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

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Thu, Apr 28, 2016 at 06:34:48AM -0600, Jan Beulich wrote:
> >>> 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.
> 

It's up to you to decide what to do. I don't have further suggestions
really.

Wei.

> Jan
> 

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