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 14:06, <wei.liu2@xxxxxxxxxx> wrote:
> On Thu, Apr 28, 2016 at 01:00:57PM +0100, Andrew Cooper wrote:
>> On 28/04/16 12:59, Wei Liu wrote:
>> > On Thu, Apr 28, 2016 at 07:40:45PM +0800, Yu, Zhang wrote:
>> >> Thanks Jan. And I admire your rigorous thought. :)
>> >>
>> >> On 4/28/2016 6:57 PM, Jan Beulich wrote:
>> >>>>>> 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?
>> >> Well, I did not delete the old line, because in my coming patch(the
>> >> p2m renaming code), I'm planning to introduce the HVMMEM_ioreq_server,
>> >> which is HVMMEM_unused+1. And I do not want the check of a.hvmmem_type
>> >> against HVMMEN_unused later in this routine appear in that patch.
>> >>
>> >>>> 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,
>> >> Not sure if this will compile? Can have a try. :)
>> >>
>> > To answer your question this can compile with gcc but not probably not
>> > with clang. This syntax is gcc extension.
>> >
>> > See: https://gcc.gnu.org/onlinedocs/gcc/Designated-Inits.html 
>> 
>> That syntax works in Clang, but will subsequent entries in the list will
>> suffer a -Werror,-Winitializer-overrides and fail to compile.
>> 
> 
> This can easily be fixed :-)
> 
>  [ 0 ... <first-upper-bound> ] = p2m_inavlid;
>  [ <second-lower-bound> ...  <second-upper-bound> ] = p2m_invalid;
> 
> But I'm not sure whether you guys think this is pretty or ugly.

What if multiple holes show up in the future? The goal really is to
deal with all holes in one line, once and for all.

Jan


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