Re: [Xen-devel] [PATCH 06/13] libx86: Introduce a helper to serialise a cpuid_policy object

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 05.07.18 at 15:39, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 05/07/18 09:40, Jan Beulich wrote:
>>>>> On 04.07.18 at 18:23, <andrew.cooper3@xxxxxxxxxx> wrote:
>>> On 04/07/18 09:51, Jan Beulich wrote:
>>>>>>> On 04.07.18 at 10:42, <wei.liu2@xxxxxxxxxx> wrote:
>>>>> On Tue, Jul 03, 2018 at 09:55:19PM +0100, Andrew Cooper wrote:
>>>>>> --- a/xen/include/public/arch-x86/xen.h
>>>>>> +++ b/xen/include/public/arch-x86/xen.h
>>>>>> @@ -314,6 +314,17 @@ struct xen_arch_domainconfig {
>>>>>>  #define XEN_ACPI_GPE0_CPUHP_BIT      2
>>>>>>  #endif
>>>>>>  
>>>>>> +/*
>>>>>> + * Representations of architectural CPUID information.  Used as the
>>>>>> + * serialised version of Xen's internal representation.
>>>>>> + */
>>>>>> +typedef struct xen_cpuid_leaf {
>>>>>> +#define XEN_CPUID_NO_SUBLEAF 0xffffffffu
>>>>>> +    uint32_t leaf, subleaf;
>>>>>> +    uint32_t a, b, c, d;
>>>>>> +} xen_cpuid_leaf_t;
>>>>>> +DEFINE_XEN_GUEST_HANDLE(xen_cpuid_leaf_t);
>>>>>> +
>>>>> Is putting this in public header while leaving
>>>>> CPUID_MAX_SERIALISED_LEAVES in a private header useful?
>>>>>
>>>>> How does an external user know how big a buffer it needs to provide to
>>>>> accommodate the return structure?
>>>> Yeah, this is indeed hard to tell from this patch alone. And going
>>>> over the titles of the remainder of the series it's also not clear
>>>> where a use of this structure in a domctl/sysctl would appear.
>>> An array of (some number of) xen_cpuid_leaf_t objects is the externally
>>> visible representation of the policy, which is why this definition is
>>> externally visible.
>>>
>>> Content of this nature will (eventually) exist in the migration stream,
>>> and it is also the interface by which toolstacks (libxl and Xapi) can
>>> make under-specified modifications to the policies.
>> But then how can a merge be as simple as the one done here?
>> Shouldn't you e.g. match CPUID entries by leaf/subleaf, rather than
>> relying on a given ordering within the arrays?
> 
> I'm confused as to where you think there is any implied ordering within
> the array.
> 
> When serialising the object (this patch), we write the values out in a
> fixed order because that is the natural way to do it.
> 
> When deserialising the object (patch 13), we walk the array linearly
> filling in the cpuid_policy object based on the leaf/subleaf.  The array
> can have the leaves in any order and this won't affect the resulting
> deserialisation.  (The array can also contain duplicate leaves, which we
> can't reasonably be detect, and why there is a sanity upper bounds check).

Oh, yes, I see now - I must have been pretty blind. I'm sorry for the
noise.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel