Message ID | 20241105155800.5461-3-phil@philjordan.eu (mailing list archive) |
---|---|
State | New |
Headers | show |
Series | i386/hvf: x2apic support and some small fixes | expand |
On Tue, Nov 05, 2024 at 04:57:57PM +0100, Phil Dennis-Jordan wrote: > The handling for CPUID function 0xD (supported XSAVE features) was > improved in a recent patch. Unfortunately, this appears to have > introduced undefined behaviour for cases where ecx > 30, as the result > of (1 << idx) is undefined if idx > 30. > > Per Intel SDM section 13.2, the behaviour for ecx values up to and > including 62 are specified. This change therefore specifically sets > all registers returned by the CPUID instruction to 0 for 63 and higher. > Furthermore, the bit shift uses uint64_t, where behaviour for the entire > range of 2..62 is safe and correct. > Thanks for correcting the regression. Reviewed-by: Roman Bolshakov <rbolshakov@ddn.com>
diff --git a/target/i386/hvf/x86_cpuid.c b/target/i386/hvf/x86_cpuid.c index ac922d7fd16..9d9ccaa815d 100644 --- a/target/i386/hvf/x86_cpuid.c +++ b/target/i386/hvf/x86_cpuid.c @@ -119,8 +119,8 @@ uint32_t hvf_get_supported_cpuid(uint32_t func, uint32_t idx, eax = 0; break; case 0xD: - if (!supported_xcr0 || - (idx > 1 && !(supported_xcr0 & (1 << idx)))) { + if (!supported_xcr0 || idx >= 63 || + (idx > 1 && !(supported_xcr0 & (UINT64_C(1) << idx)))) { eax = ebx = ecx = edx = 0; break; }
The handling for CPUID function 0xD (supported XSAVE features) was improved in a recent patch. Unfortunately, this appears to have introduced undefined behaviour for cases where ecx > 30, as the result of (1 << idx) is undefined if idx > 30. Per Intel SDM section 13.2, the behaviour for ecx values up to and including 62 are specified. This change therefore specifically sets all registers returned by the CPUID instruction to 0 for 63 and higher. Furthermore, the bit shift uses uint64_t, where behaviour for the entire range of 2..62 is safe and correct. Signed-off-by: Phil Dennis-Jordan <phil@philjordan.eu> --- target/i386/hvf/x86_cpuid.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)