Re: [Xen-devel] [PATCH for 4.7] Remove HVMMEM_mmio_write_dm from the public interface.

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 28.04.16 at 12:42, <george.dunlap@xxxxxxxxxx> wrote:
> On 28/04/16 11:22, Jan Beulich wrote:
>>>>> On 28.04.16 at 10:29, <yu.c.zhang@xxxxxxxxxxxxxxx> wrote:
>>> @@ -5529,7 +5527,7 @@ long do_hvm_op(unsigned long op, 
> XEN_GUEST_HANDLE_PARAM(void) arg)
>>>              [HVMMEM_ram_rw]  = p2m_ram_rw,
>>>              [HVMMEM_ram_ro]  = p2m_ram_ro,
>>>              [HVMMEM_mmio_dm] = p2m_mmio_dm,
>>> -            [HVMMEM_mmio_write_dm] = p2m_mmio_write_dm
>>> +            [HVMMEM_unused] = p2m_invalid
>> 
>> Why don't you simply delete the old line, without replacement?
> 
> That might have been slightly cleaner; but we're going to have to put it
> back as soon as the development window opens anyway, so I don't really
> see the point of going through the effort of respinning the patch again.
> 
> Would you be willing to ack this version anyway?

I have no problem doing so (and in fact I have it on my to by
committed list already), it is just looked slightly confusing (and
I had already typed half a reply that this isn't what was discussed
until I properly looked at the next hunk), and hence I wanted to
understand the motivation. And btw., I'm not convinced it would
need to be put there anyway later: I don't view the used
mechanism as a good (read: extensible) one to deal with what
would be holes in the array above. Indeed we can't leave them
uninitialized (as that would mean p2m_ram_rw), but I think we
should better find a way to initialize _all_ unused slots without
requiring an initializer for each of them. Sadly the desire to allow
compilation with clang prohibits the most natural solution:

        static const p2m_type_t memtype[] = {
            [0 ... <upper-bound> - 1] = p2m_invalid,
            [HVMMEM_ram_rw]  = p2m_ram_rw,
            [HVMMEM_ram_ro]  = p2m_ram_ro,
            [HVMMEM_mmio_dm] = p2m_mmio_dm,
        };

Maybe we could do (altering the second hunk of this patch)

@@ -5553,7 +5551,10 @@ long do_hvm_op(unsigned long op, 
XEN_GUEST_HANDLE_PARAM(void) arg)
              ((a.first_pfn + a.nr - 1) > domain_get_maximum_gpfn(d)) )
             goto setmemtype_fail;
             
-        if ( a.hvmmem_type >= ARRAY_SIZE(memtype) )
+        BUILD_BUG_ON(p2m_ram_rw);
+        BUILD_BUG_ON(HVMMEM_ram_rw);
+        if ( a.hvmmem_type >= ARRAY_SIZE(memtype) ||
+             (a.hvmmem_type && !memtype[a.hvmmem_type]) )
             goto setmemtype_fail;
 
         while ( a.nr > start_iter )

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel