Re: [PATCH] cpu/i386: update xsave components after CPUID filtering

[Paolo Bonzini <pbonzini@redhat.com>]

On 11/23/22 19:17, Huanyu ZHAI wrote:
> Subject: [PATCH] cpu/i386: update xsave components after CPUID filtering
> 
> On i386 platform, CPUID data are setup through three consecutive steps: 
> CPU model definition, expansion and filtering.
> 
> XSAVE components are enabled during the expansion stage, by checking if 
> they are enabled in CPUID. However, it is still
> 
> probable that some XSAVE features will be enabled/disabled during the 
> filtering stage and the XSAVE components left unchanged.
> 
> Inconsistency between XSAVE features and enabled XSAVE components can 
> lead to problems on some Linux guests in the absence of

The patch is correct, but I think you can also remove the existing call 
to x86_cpu_enable_xsave_components().  Can you test that?

Thanks,

Paolo

> the following patch in the kernel:
> 
> https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1452368.html <https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1452368.html>
> 
> A simple case to reproduce this problem is to start a SUSE 12 SP3 guest 
> with cpu model set to Skylake-Server:
> 
> $ qemu-system-x86_64 -cpu Skylake-Server ...
> 
> In the SUSE 12 SP3 guest, one can observe that PKRU will be enabled 
> without Intel PKU's presence.
> 
> That's because on platform with Skylake-Server cpus, Intel PKU is 
> disabled during x86_cpu_filter_features(),
> 
> but the XSAVE PKRU bit was enabled by x86_cpu_expand_features().
> 
> Signed-off-by: Huanyu ZHAI zhaihuanyu@huawei.com 
> <mailto:zhaihuanyu@huawei.com>
> 
> Signed-off-by: Xin Wang wangxinxin.wang@huawei.com 
> <mailto:wangxinxin.wang@huawei.com>
> 
> ---
> 
> target/i386/cpu.c | 3 +++
> 
> 1 file changed, 3 insertions(+)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> 
> index 22b681ca37..2ee574cf05 100644
> 
> --- a/target/i386/cpu.c
> 
> +++ b/target/i386/cpu.c
> 
> @@ -6362,6 +6362,9 @@ static void x86_cpu_filter_features(X86CPU *cpu, 
> bool verbose)
> 
>               mark_unavailable_features(cpu, FEAT_7_0_EBX, 
> CPUID_7_0_EBX_INTEL_PT, prefix);
> 
>           }
> 
>       }
> 
> +
> 
> +    /* Update XSAVE components again based on the filtered CPU feature 
> flags */
> 
> +    x86_cpu_enable_xsave_components(cpu);
> 
> }
> 
> static void x86_cpu_hyperv_realize(X86CPU *cpu)
> 
> -- 
> 
> 2.27.0
>