Re: [PATCH v2] xen/x86: public: add TSC defines for cpuid leaf 4

[Jan Beulich <jbeulich@xxxxxxxx>]

On 26.01.2023 19:02, Krister Johansen wrote:
> On Thu, Jan 26, 2023 at 10:57:01AM +0100, Jan Beulich wrote:
>> On 25.01.2023 19:45, Krister Johansen wrote:
>>> --- a/xen/include/public/arch-x86/cpuid.h
>>> +++ b/xen/include/public/arch-x86/cpuid.h
>>> @@ -72,6 +72,14 @@
>>>   * Sub-leaf 2: EAX: host tsc frequency in kHz
>>>   */
>>>  
>>> +#define XEN_CPUID_TSC_EMULATED               (1u << 0)
>>> +#define XEN_CPUID_HOST_TSC_RELIABLE          (1u << 1)
>>> +#define XEN_CPUID_RDTSCP_INSTR_AVAIL         (1u << 2)
>>> +#define XEN_CPUID_TSC_MODE_DEFAULT           (0)
>>> +#define XEN_CPUID_TSC_MODE_EMULATE           (1u)
>>> +#define XEN_CPUID_TSC_MODE_NOEMULATE         (2u)
>>> +#define XEN_CPUID_TSC_MODE_NOEMULATE_TSC_AUX (3u)
>>
>> Actually I think we'd better stick to the names found in asm/time.h
>> (and then replace their uses, dropping the #define-s there). If you
>> agree, I'd be happy to make the adjustment while committing.
> 
> Just to confirm, this would be moving these:
> 
>    #define TSC_MODE_DEFAULT          0
>    #define TSC_MODE_ALWAYS_EMULATE   1
>    #define TSC_MODE_NEVER_EMULATE    2
>    
> To cpuid.h?  I'm generally fine with this.  I don't see anything in
> Linux that's using these names.  The only question I have is whether
> we'd still want to prefix the names with XEN so that if they're pulled
> in to Linux it's clear that the define is Xen specific?  E.g. something
> like this perhaps?
> 
>    #define XEN_TSC_MODE_DEFAULT          0
>    #define XEN_TSC_MODE_ALWAYS_EMULATE   1
>    #define XEN_TSC_MODE_NEVER_EMULATE    2
> 
> That does increase the number of files we'd need to touch to make the
> change, though. (And the other defines in that file all start with
> XEN_CPUID).
> 
> Though, if you mean doing it this way:
> 
>    #define XEN_CPUID_TSC_MODE_DEFAULT          0
>    #define XEN_CPUID_TSC_MODE_ALWAYS_EMULATE   1
>    #define XEN_CPUID_TSC_MODE_NEVER_EMULATE    2
>  
> then no objection to that at all.  Apologies for overlooking the naming
> overlap when I put this together the first time.

Yes, it's the last variant you list that I was after. And I'd be okay to
leave dropping the so far private constants to a separate follow-on patch.

Jan