diff mbox series

constant_tsc support for SVM guest

Message ID a6662944-18ac-5075-1427-f92ae8e20163@amd.com
State New
Headers show
Series constant_tsc support for SVM guest | expand

Commit Message

Wei Huang April 23, 2021, 5:32 a.m. UTC
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.

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?]

Do I miss anything here? Any comments about the proposal?

Thanks,
-Wei

Comments

Moger, Babu April 23, 2021, 4:54 p.m. UTC | #1
Hi Wei,
I dont know the background of this feature. I will let some else to
comment on that.

The patch exposes the feature TscInvariant to the guest successfully.
Tested it on my AMD box. I have few comments on your patch below.


On 4/23/21 12:32 AM, 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.
> 
> 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?]
> 
> 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",

We dont need this alias. I think this is there only for legacy purposes.

> +                .props = (PropValue[]) {
> +                    { "ibpb", "on" },
> +                    { "perfctr-core", "on" },
> +                    { "clzero", "on" },
> +                    { "xsaveerptr", "on" },
> +                    { "xsaves", "on" },

You dont need to list any of these features again. All the old features
will be added implicitly.

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

Same as above. Dont need these previous features.

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

Here is the updated patch..
=========================================================================

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index ad99cad0e7..27287c1343 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -4077,6 +4077,13 @@ static X86CPUDefinition builtin_x86_defs[] = {
                     { /* end of list */ }
                 }
             },
+            {
+              .version = 4,
+              .props = (PropValue[]) {
+                    { "invtsc", "on" },
+                    { /* end of list */ }
+                }
+            },
             { /* end of list */ }
         }
     },
@@ -4189,6 +4196,14 @@ static X86CPUDefinition builtin_x86_defs[] = {
                     { /* end of list */ }
                 }
             },
+            {
+              .version = 3,
+              .props = (PropValue[]) {
+                    { "invtsc", "on" },
+                    { /* end of list */ }
+                }
+            },
+
             { /* end of list */ }
         }
     },
@@ -4246,6 +4261,18 @@ 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 */ }
+        }
     },
 };



thanks
Eduardo Habkost April 23, 2021, 9:27 p.m. UTC | #2
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 */ }
> +        }
>
Wei Huang April 25, 2021, 5:19 a.m. UTC | #3
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 */ }
>> +        }
>>
>
Marcelo Tosatti April 26, 2021, 5:28 p.m. UTC | #4
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
> 
>
Marcelo Tosatti April 26, 2021, 6:51 p.m. UTC | #5
On Sun, Apr 25, 2021 at 12:19:11AM -0500, Wei Huang wrote:
> 
> 
> 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. 

It should be blocked, if performed to a host with a different frequency
or without TscRateMsr, if one desires the "constant TSC rate" meaning
to be maintained.

> > > 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?]

Not as far as i know.

Fact is that libvirt will set the TSC_KHZ (from the value of
KVM_GET_TSC_KHZ ioctl).

That could be done inside QEMU itself, maybe by specifying -cpu
AAA,cpu-freq=auto ?

https://www.spinics.net/linux/fedora/libvir/msg141570.html
diff mbox series

Patch

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