Re: [Xen-devel] [PATCH RFC 01/31] xen/public: Export featureset information in the public API

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 23.12.15 at 12:26, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 23/12/2015 10:24, Jan Beulich wrote:
>>>>> Andrew Cooper <andrew.cooper3@xxxxxxxxxx> 12/23/15 11:05 AM >>>
>>> On 22/12/2015 16:59, Jan Beulich wrote:
>>>>>>> On 22.12.15 at 17:42, <andrew.cooper3@xxxxxxxxxx> wrote:
>>>>> On 22/12/15 16:28, Jan Beulich wrote:
>>>>>>>>> On 16.12.15 at 22:24, <andrew.cooper3@xxxxxxxxxx> wrote:
>>>>>>> +#define XEN_FEATURESET_1d     0 /* 0x00000001.edx      */
>>>>>>> +#define XEN_FEATURESET_1c     1 /* 0x00000001.ecx      */
>>>>>>> +#define XEN_FEATURESET_e1d    2 /* 0x80000001.edx      */
>>>>>>> +#define XEN_FEATURESET_e1c    3 /* 0x80000001.ecx      */
>>>>>>> +#define XEN_FEATURESET_Da1    4 /* 0x0000000d:1.eax    */
>>>>>>> +#define XEN_FEATURESET_7b0    5 /* 0x00000007:0.ebx    */
>>>>>> This ends up being pretty cryptic.
>>>>> Perhaps at a first glance, but there are so many uses that a shorthand
>>>>> really is needed.  See especially the MSR masking patches towards the
>>>>> end of the series.
>>>> I understand that something short is needed. But as the main
>>>> identifier in the ABI?
>>> It really comes down to whom is intended to do what with a featureset.
>>>
>>> These values are derivable as functions of the feature values
>>> themselves, but that requires an assumption that the featureset bitmap
>>> mirrors hardware precisely.  Providing this (names not withstanding) as
>>> a part of the ABI is intended to provide that assurance.
>>>
>>> I fully intend higher levels of the toolstack to deal with a featureset
>>> as an arbitrary bitmap, but lower levels like libxc do need a greater
>>> level of understanding of its layout.
>> All understood. The question just is whether the main identifiers in the 
>> public
>> header should be this cryptic, or whether consumers should instead introduce
>> shortcuts of their liking (even more so that, just like noted elsewhere, 
>> these
>> identifiers also lack "CPU" and perhaps "x86" in their names - in the end
>> feature leveling isn't something that's needed on x86 only).
> 
> For now, I can move them along with XEN_NR_FEATURESET_ENTRIES, because
> the only bits of libxc which need them are the bits which are already
> using the shared library.
> 
> 
> The slight conceptual issue I am concerned about is how this will fit in
> with extended work to fix the rest of cpuid handling in Xen.
> 
> Other quantities, such as max_leaf and max_phys_addr need levelling
> across hosts.  At the moment, all of this information is derived from
> dom0's view and is not kept consistent across migrate.  (Surprisingly,
> windows notices this, doesn't crash, and decides to install a new CPU
> driver.)
> 
> Long term, the toolstack needs to level a complete idea of the cpuid
> policy for the guest, and the featureset is just one part of this. 
> However, I am expecting that the featureset on its own will still be a
> useful entity, which is why I progressed down this route.

I have to admit that I don't see how all this relates to the naming issue:
I don't mind the constants being publicly advertised (and in fact I agree
they need to be), just that their names should imo be readable /
understandable without deciphering.

Jan


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