Re: constant_tsc support for SVM guest

[Marcelo Tosatti]

Hi Wei, Eduardo,

On Fri, Apr 23, 2021 at 05:27:44PM -0400, Eduardo Habkost wrote:
> On Fri, Apr 23, 2021 at 12:32:00AM -0500, Wei Huang wrote:
> > There was a customer request for const_tsc support on AMD guests. Right now
> > this feature is turned off by default for QEMU x86 CPU types (in
> > CPUID_Fn80000007_EDX[8]). However we are seeing a discrepancy in guest VM
> > behavior between Intel and AMD.
> > 
> > In Linux kernel, Intel x86 code enables X86_FEATURE_CONSTANT_TSC based on
> > vCPU's family & model. So it ignores CPUID_Fn80000007_EDX[8] and guest VMs
> > have const_tsc enabled. On AMD, however, the kernel checks
> > CPUID_Fn80000007_EDX[8]. So const_tsc is disabled on AMD by default.

EAX=80000007h: Advanced Power Management Information
This function provides advanced power management feature identifiers. 
EDX bit 8 indicates support for invariant TSC. 

Intel documentation states:

"The time stamp counter in newer processors may support an enhancement,
referred to as invariant TSC. Processor's support for invariant TSC
is indicated by CPUID.80000007H:EDX[8]. The invariant TSC will run
at a constant rate in all ACPI P-, C-. and T-states. This is the
architectural behavior moving forward. On processors with invariant TSC
support, the OS may use the TSC for wall clock timer services (instead
of ACPI or HPET timers). TSC reads are much more efficient and do not
incur the overhead associated with a ring transition or access to a
platform resource."

X86_FEATURE_NONSTOP_TSC is enabled (on both Intel and AMD) by checking
the CPUID_Fn80000007_EDX[8] bit.

> Oh.  This seems to defeat the purpose of the invtsc migration
> blocker we have.
> 
> Do we know when this behavior was introduced in Linux?
> 
> > 
> > I am thinking turning on invtsc for EPYC CPU types (see example below). Most
> > AMD server CPUs have supported invariant TSC for a long time. So this change
> > is compatible with the hardware behavior. The only problem is live migration
> > support, which will be blocked because of invtsc. However this problem
> > should be considered very minor because most server CPUs support TscRateMsr
> > (see CPUID_Fn8000000A_EDX[4]), allowing VMs to migrate among CPUs with
> > different TSC rates. This live migration restriction can be lifted as long
> > as the destination supports TscRateMsr or has the same frequency as the
> > source (QEMU/libvirt do it).

Yes.

> > [BTW I believe this migration limitation might be unnecessary because it is
> > apparently OK for Intel guests to ignore invtsc while claiming const_tsc.
> > Have anyone reported issues?]
> 
> CCing Marcelo, who originally added the migration blocker in QEMU.

The reasoning behind the migration blocker was to ensure that 
the invariant TSC meaning as defined:

"The invariant TSC will run at a constant rate in all ACPI P-, C-. and T-states"

Would be maintained across migration.

> > 
> > Do I miss anything here? Any comments about the proposal?
> > 
> > Thanks,
> > -Wei
> > 
> > diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> > index ad99cad0e7..3c48266884 100644
> > --- a/target/i386/cpu.c
> > +++ b/target/i386/cpu.c
> > @@ -4077,6 +4076,21 @@ static X86CPUDefinition builtin_x86_defs[] = {
> >                      { /* end of list */ }
> >                  }
> >              },
> > +            {
> > +                .version = 4,
> > +                .alias = "EPYC-IBPB",
> > +                .props = (PropValue[]) {
> > +                    { "ibpb", "on" },
> > +                    { "perfctr-core", "on" },
> > +                    { "clzero", "on" },
> > +                    { "xsaveerptr", "on" },
> > +                    { "xsaves", "on" },
> 
> You don't need to copy the properties from the previous version.
> The properties of version N are applied on top of the properties
> of version (N-1).
> 
> > +                    { "invtsc", "on" },
> > +                    { "model-id",
> > +                      "AMD EPYC Processor" },
> > +                    { /* end of list */ }
> > +                }
> > +            },
> >              { /* end of list */ }
> >          }
> >      },
> > @@ -4189,6 +4203,15 @@ static X86CPUDefinition builtin_x86_defs[] = {
> >                      { /* end of list */ }
> >                  }
> >              },
> > +            {
> > +                .version = 3,
> > +                .props = (PropValue[]) {
> > +                    { "ibrs", "on" },
> > +                    { "amd-ssbd", "on" },
> > +                    { "invtsc", "on" },
> > +                    { /* end of list */ }
> > +                }
> > +            },
> >              { /* end of list */ }
> >          }
> >      },
> > @@ -4246,6 +4269,17 @@ static X86CPUDefinition builtin_x86_defs[] = {
> >          .xlevel = 0x8000001E,
> >          .model_id = "AMD EPYC-Milan Processor",
> >          .cache_info = &epyc_milan_cache_info,
> > +        .versions = (X86CPUVersionDefinition[]) {
> > +            { .version = 1 },
> > +            {
> > +                .version = 2,
> > +                .props = (PropValue[]) {
> > +                    { "invtsc", "on" },
> > +                    { /* end of list */ }
> > +                }
> > +            },
> > +            { /* end of list */ }
> > +        }
> > 
> 
> -- 
> Eduardo
> 
>