Re: constant_tsc support for SVM guest

[Wei Huang]

On 4/23/21 4:27 PM, 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.
>
> Oh.  This seems to defeat the purpose of the invtsc migration
> blocker we have.
>
> Do we know when this behavior was introduced in Linux?

This code has existed in the kernel for a long time:

  commit 2b16a2353814a513cdb5c5c739b76a19d7ea39ce
  Author: Andi Kleen <ak@linux.intel.com>
  Date:   Wed Jan 30 13:32:40 2008 +0100

     x86: move X86_FEATURE_CONSTANT_TSC into early cpu feature detection

There was another related commit which might explain the reasoning of 
turning on CONSTANT_TSC based on CPU family on Intel:

  commit 40fb17152c50a69dc304dd632131c2f41281ce44
  Author: Venki Pallipadi <venkatesh.pallipadi@intel.com>
  Date:   Mon Nov 17 16:11:37 2008 -0800

     x86: support always running TSC on Intel CPUs

According to the commit above, there are two kernel features: 
CONSTANT_TSC and NONSTOP_TSC:

  * CONSTANT_TSC: TSC runs at constant rate
  * NONSTOP_TSC: TSC not stop in deep C-states

If CPUID_Fn80000007_EDX[8] == 1, both CONSTANT_TSC and NONSTOP_TSC are 
turned on. This applies to all x86 vendors. For Intel CPU with certain 
CPU families (i.e. c->x86 == 0x6 && c->x86_model >= 0x0e), it will turn 
on CONSTANT_TSC (but no NONSTOP_TSC) with CPUID_Fn80000007_EDX[8]=0.

I believe the migration blocker was created for the CONSTANT_TSC case: 
if vCPU claims to have a constant TSC rate, we have to make sure 
src/dest are matched with each other (having the same CPU frequency or 
have tsc_ratio). NONSTOP_TSC doesn't matter in this scope.

> > 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).
> >
> > [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.
>
> > 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).

Will fix

> > +                    { "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 */ }
> > +        }