Re: [RFC PATCH 1/1] i386: Remove features from Epyc-Milan cpu

[David Edmondson <david.edmondson@oracle.com>]

On Saturday, 2022-01-29 at 07:23:37 -03, Leonardo Bras wrote:

> While trying to bring a VM with EPYC-Milan cpu on a host with
> EPYC-Milan cpu (EPYC 7313), the following warning can be seen:
>
> qemu-system-x86_64: warning: host doesn't support requested feature: CPUID.07H:EBX.erms [bit 9]
> qemu-system-x86_64: warning: host doesn't support requested feature: CPUID.07H:EDX.fsrm [bit 4]
>
> Even with this warning, the host goes up.
>
> Then, grep'ing cpuid output on both guest and host, outputs:
>
> extended feature flags (7):
>       enhanced REP MOVSB/STOSB                 = false
>       fast short REP MOV                       = false
>       (simple synth)  = AMD EPYC (3rd Gen) (Milan B1) [Zen 3], 7nm
>    brand = "AMD EPYC 7313 16-Core Processor               "
>
> This means that for the same -cpu model (EPYC-Milan), the vcpu may or may
> not have the above feature bits set, which is usually not a good idea for
> live migration:
> Migrating from a host with these features to a host without them can
> be troublesome for the guest.
>
> Remove the "optional" features (erms, fsrm) from Epyc-Milan, in order to
> avoid possible after-migration guest issues.
>
> Signed-off-by: Leonardo Bras <leobras@redhat.com>
> ---
>
> Does this make sense? Or maybe I am missing something here.

We have encountered some Milan CPUs (7J13) that did not initially
declare support for either ERMS or FSRM.

A firmware update caused these features to appear, which definitely
causes potential problems with migration of VMs from hosts with updated
firmware to those without.

It would be interesting to know if there is any expectation that the
features might be enabled on the 7313 CPUs that you have with a future
firmware update.

I *think* that the expectation is that Milan CPUs will have the
features, and if that is correct then they should remain present in the
EPYC-Milan definition on QEMU.

> Having a kvm guest running with a feature bit, while the host
> does not support it seems to cause a possible break the guest.

As Daniel said, this should not happen in this case.

>  target/i386/cpu.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index aa9e636800..a4bbd38ed0 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -4160,12 +4160,9 @@ static const X86CPUDefinition builtin_x86_defs[] = {
>              CPUID_7_0_EBX_FSGSBASE | CPUID_7_0_EBX_BMI1 | CPUID_7_0_EBX_AVX2 |
>              CPUID_7_0_EBX_SMEP | CPUID_7_0_EBX_BMI2 | CPUID_7_0_EBX_RDSEED |
>              CPUID_7_0_EBX_ADX | CPUID_7_0_EBX_SMAP | CPUID_7_0_EBX_CLFLUSHOPT |
> -            CPUID_7_0_EBX_SHA_NI | CPUID_7_0_EBX_CLWB | CPUID_7_0_EBX_ERMS |
> -            CPUID_7_0_EBX_INVPCID,
> +            CPUID_7_0_EBX_SHA_NI | CPUID_7_0_EBX_CLWB | CPUID_7_0_EBX_INVPCID,
>          .features[FEAT_7_0_ECX] =
>              CPUID_7_0_ECX_UMIP | CPUID_7_0_ECX_RDPID | CPUID_7_0_ECX_PKU,
> -        .features[FEAT_7_0_EDX] =
> -            CPUID_7_0_EDX_FSRM,
>          .features[FEAT_XSAVE] =
>              CPUID_XSAVE_XSAVEOPT | CPUID_XSAVE_XSAVEC |
>              CPUID_XSAVE_XGETBV1 | CPUID_XSAVE_XSAVES,

dme.
-- 
I don't care 'bout your other girls, just be good to me.