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

[Andrew Cooper <andrew.cooper3@citrix.com>]

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@citrix.com> wrote:
>>>> On 28/04/16 11:22, Jan Beulich wrote:
>>>>>>>> On 28.04.16 at 10:29, <yu.c.zhang@linux.intel.com> 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.

I already had to fix two examples of this to get clang support working
in the past.

(It is a real shame that p2m_invalid doesn't have the value 0)

~Andrew

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