| Message ID | 20200716082019.215316-3-xiaoyao.li@intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | Two Fixes for xsave compoent features | expand |
On Thu, Jul 16, 2020 at 04:20:19PM +0800, Xiaoyao Li wrote: > When setting up XSAVE components, it needs to mask off those unsupported > by KVM. > > Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com> We must never disable CPUID features silently based on host capabilities, otherwise we can't guarantee guest ABI stability when migrating to another host. Filtering of features should involve a call to mark_unavailable_features() (or some equivalent mechanism) so we can report the missing features properly through QMP. Could you explain what's the bug you are trying to fix? The loop at x86_cpu_filter_features() is already supposed to disable features unsupported by the host. > --- > target/i386/cpu.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/target/i386/cpu.c b/target/i386/cpu.c > index f5f11603e805..efc92334b7b1 100644 > --- a/target/i386/cpu.c > +++ b/target/i386/cpu.c > @@ -6274,8 +6274,10 @@ static void x86_cpu_enable_xsave_components(X86CPU *cpu) > } > } > > - env->features[FEAT_XSAVE_COMP_LO] = mask; > - env->features[FEAT_XSAVE_COMP_HI] = mask >> 32; > + env->features[FEAT_XSAVE_COMP_LO] = mask & > + x86_cpu_get_supported_feature_word(FEAT_XSAVE_COMP_LO, cpu->migratable); > + env->features[FEAT_XSAVE_COMP_HI] = (mask >> 32) & > + x86_cpu_get_supported_feature_word(FEAT_XSAVE_COMP_HI, cpu->migratable); > } > > /***** Steps involved on loading and filtering CPUID data > -- > 2.18.4 >
On 7/16/2020 11:14 PM, Eduardo Habkost wrote: > On Thu, Jul 16, 2020 at 04:20:19PM +0800, Xiaoyao Li wrote: >> When setting up XSAVE components, it needs to mask off those unsupported >> by KVM. >> >> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com> > > We must never disable CPUID features silently based on host > capabilities, otherwise we can't guarantee guest ABI stability > when migrating to another host. Filtering of features should > involve a call to mark_unavailable_features() (or some equivalent > mechanism) so we can report the missing features properly through > QMP. > > Could you explain what's the bug you are trying to fix? The loop > at x86_cpu_filter_features() is already supposed to disable > features unsupported by the host. Sorry, I forgot x86_cpu_filter_features() totally when code inspection. >> --- >> target/i386/cpu.c | 6 ++++-- >> 1 file changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/target/i386/cpu.c b/target/i386/cpu.c >> index f5f11603e805..efc92334b7b1 100644 >> --- a/target/i386/cpu.c >> +++ b/target/i386/cpu.c >> @@ -6274,8 +6274,10 @@ static void x86_cpu_enable_xsave_components(X86CPU *cpu) >> } >> } >> >> - env->features[FEAT_XSAVE_COMP_LO] = mask; >> - env->features[FEAT_XSAVE_COMP_HI] = mask >> 32; >> + env->features[FEAT_XSAVE_COMP_LO] = mask & >> + x86_cpu_get_supported_feature_word(FEAT_XSAVE_COMP_LO, cpu->migratable); >> + env->features[FEAT_XSAVE_COMP_HI] = (mask >> 32) & >> + x86_cpu_get_supported_feature_word(FEAT_XSAVE_COMP_HI, cpu->migratable); >> } >> >> /***** Steps involved on loading and filtering CPUID data >> -- >> 2.18.4 >> >
diff --git a/target/i386/cpu.c b/target/i386/cpu.c index f5f11603e805..efc92334b7b1 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -6274,8 +6274,10 @@ static void x86_cpu_enable_xsave_components(X86CPU *cpu) } } - env->features[FEAT_XSAVE_COMP_LO] = mask; - env->features[FEAT_XSAVE_COMP_HI] = mask >> 32; + env->features[FEAT_XSAVE_COMP_LO] = mask & + x86_cpu_get_supported_feature_word(FEAT_XSAVE_COMP_LO, cpu->migratable); + env->features[FEAT_XSAVE_COMP_HI] = (mask >> 32) & + x86_cpu_get_supported_feature_word(FEAT_XSAVE_COMP_HI, cpu->migratable); } /***** Steps involved on loading and filtering CPUID data
When setting up XSAVE components, it needs to mask off those unsupported by KVM. Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com> --- target/i386/cpu.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)