| Message ID | AS4P190MB202188766937CF16EC4F1DAFF60C9@AS4P190MB2021.EURP190.PROD.OUTLOOK.COM |
|---|---|
| State | New |
| Headers | show |
| Series | cpu/i386: update xsave components after CPUID filtering | expand |
* Huanyu ZHAI (huanyu.zhai@outlook.com) 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 following patch in the kernel: > > 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(). I just spotted this when trying to clear out my mail folder; this isn't a 'trivial'! I'm not sure if it's right or not, but I've cc'd in Paolo and Marcelo as x86 maintainers. Dave > 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 >
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 >
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)