| Message ID | 1474659936-978-3-git-send-email-ehabkost@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 09/23/2016 12:45 PM, Eduardo Habkost wrote: > The code that calculates the set of supported XSAVE components on > CPUID looks at ext_save_areas to find out which components should > be enabled. However, if there are zeroed entries in the > ext_save_areas array, the > ((env->features[esa->feature] & esa->bits) == esa->bits) > check will always succeed and QEMU will unconditionally try to > enable the component. > > Luckily this never caused any problems because the only missing > entry in ext_save_areas is the PT State component (bit 8), and > KVM currently doesn't support it (so it was cleared on ena_mask). > But the code was still incorrect and would break if KVM starts > returning CPUID[EAX=0xD,ECX=0].EAX[bit 8] as supported on > GET_SUPPORTED_CPUID. > > Fix the problem by changing the code to not enable a XSAVE > component if ExtSaveArea::bits is zero. > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > --- > target-i386/cpu.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) Reviewed-by: Richard Henderson <rth@twiddle.net> r~
diff --git a/target-i386/cpu.c b/target-i386/cpu.c index cc07fdb..25ab4f8 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -2514,7 +2514,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, *ecx = 0x240; for (i = 2; i < ARRAY_SIZE(x86_ext_save_areas); i++) { const ExtSaveArea *esa = &x86_ext_save_areas[i]; - if ((env->features[esa->feature] & esa->bits) == esa->bits + if ((env->features[esa->feature] & esa->bits) && ((ena_mask >> i) & 1) != 0) { if (i < 32) { *eax |= 1u << i; @@ -2530,7 +2530,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, *eax = env->features[FEAT_XSAVE]; } else if (count < ARRAY_SIZE(x86_ext_save_areas)) { const ExtSaveArea *esa = &x86_ext_save_areas[count]; - if ((env->features[esa->feature] & esa->bits) == esa->bits + if ((env->features[esa->feature] & esa->bits) && ((ena_mask >> count) & 1) != 0) { *eax = esa->size; *ebx = esa->offset; @@ -2766,7 +2766,7 @@ static void x86_cpu_reset(CPUState *s) } for (i = 2; i < ARRAY_SIZE(x86_ext_save_areas); i++) { const ExtSaveArea *esa = &x86_ext_save_areas[i]; - if ((env->features[esa->feature] & esa->bits) == esa->bits) { + if (env->features[esa->feature] & esa->bits) { xcr0 |= 1ull << i; } }
The code that calculates the set of supported XSAVE components on CPUID looks at ext_save_areas to find out which components should be enabled. However, if there are zeroed entries in the ext_save_areas array, the ((env->features[esa->feature] & esa->bits) == esa->bits) check will always succeed and QEMU will unconditionally try to enable the component. Luckily this never caused any problems because the only missing entry in ext_save_areas is the PT State component (bit 8), and KVM currently doesn't support it (so it was cleared on ena_mask). But the code was still incorrect and would break if KVM starts returning CPUID[EAX=0xD,ECX=0].EAX[bit 8] as supported on GET_SUPPORTED_CPUID. Fix the problem by changing the code to not enable a XSAVE component if ExtSaveArea::bits is zero. Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- target-i386/cpu.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)