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

[Krister Johansen <kjlx@xxxxxxxxxxxxxxxxxx>]

On Fri, Jan 27, 2023 at 08:16:18AM +0100, Jan Beulich wrote:
> 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.

Ok, thanks. I'll send you a v3 that makes these changes, unless you've
already fixed this up and committed the v2.  In that case, feel free to
disregard.

-K