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

From: Krister Johansen <kjlx@templeofstupid.com>
To: Jan Beulich <jbeulich@suse.com>
Cc: "Krister Johansen" <kjlx@templeofstupid.com>,
	"Andrew Cooper" <andrew.cooper3@citrix.com>,
	"Roger Pau Monné" <roger.pau@citrix.com>, "Wei Liu" <wl@xen.org>,
	"Juergen Gross" <jgross@suse.com>,
	"Boris Ostrovsky" <boris.ostrovsky@oracle.com>,
	"David Reaver" <me@davidreaver.com>,
	xen-devel@lists.xenproject.org
Subject: Re: [PATCH v2] xen/x86: public: add TSC defines for cpuid leaf 4
Date: Thu, 26 Jan 2023 10:02:44 -0800	[thread overview]
Message-ID: <20230126180244.GB1959@templeofstupid.com> (raw)
In-Reply-To: <77576aab-93bf-5f6a-9b04-17eaf1d84ffb@suse.com>

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.

-K