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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 20/12/2016 14:45, Jan Beulich wrote:
>>>> 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 ...

Our policies along these lines are a mess.  We really should separate
the guest ABI/API from the toolstack ABI/API, and by this, I mean having
separate directory structures.

In the meantime, all of the content in this file is only useful to
entities which can make DOMCTLs to start with, so the entire file should
be restricted to Xen/tools only.

~Andrew

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