Re: constant_tsc support for SVM guest

[Eduardo Habkost]

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?

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

> +                    { "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