Re: [Xen-devel] [PATCH v5 04/13] pvh/acpi: Install handlers for ACPI-related PVH IO accesses

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 20.12.16 at 15:16, <boris.ostrovsky@xxxxxxxxxx> wrote:
> On 12/20/2016 09:10 AM, Jan Beulich wrote:
>>>>> On 20.12.16 at 15:03, <boris.ostrovsky@xxxxxxxxxx> wrote:
>>> On 12/20/2016 06:24 AM, Jan Beulich wrote:
>>>>>>> On 17.12.16 at 00:18, <boris.ostrovsky@xxxxxxxxxx> wrote:
>>>>> --- a/xen/include/public/arch-x86/hvm/save.h
>>>>> +++ b/xen/include/public/arch-x86/hvm/save.h
>>>>> @@ -527,7 +527,37 @@ DECLARE_HVM_SAVE_TYPE(HPET, 12, struct hvm_hw_hpet);
>>>>>  /*
>>>>>   * PM timer
>>>>>   */
>>>>> +#if __XEN_INTERFACE_VERSION__ >= 0x00040800
>>>>> +struct hvm_hw_pmtimer {
>>>>> +    uint32_t tmr_val;   /* PM_TMR_BLK.TMR_VAL: 32bit free-running 
>>>>> counter */
>>>>> +    uint16_t pm1a_sts;  /* PM1a_EVT_BLK.PM1a_STS: status register */
>>>>> +    uint16_t pm1a_en;   /* PM1a_EVT_BLK.PM1a_EN: enable register */
>>>>> +#if defined(__XEN__) || defined(__XEN_TOOLS__)
>>>>> +    uint16_t gpe0_sts;
>>>>> +    uint16_t gpe0_en;
>>>>> +#endif
>>>> Why inside another #ifdef? There's no other example in this file
>>>> which might have suggested to you that it needs doing this way.
>>>> In fact there are also no pre-existing uses of
>>>> __XEN_INTERFACE_VERSION__ in this header, so I also don't see
>>>> why you added one (and then with a slightly off value check).
>>> Don't we want users of old interface to continue using original
>>> definition of hvm_hw_timer? And not to expose them to the fix routine below?
>> There shouldn't be any such old users, because of ...
>>
>>>> If anything the _whole_ header would need to become Xen/tools
>>>> only.
>> ... this.
> 
> Is this file is not supposed to be used by anyone outside of the Xen tree?

I don't think so, no. In any event - prior additions did not do
any precautions to guard possible foreign consumers. Maybe
Andrew has an opinion here ...

Jan


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