| Message ID | 20241029130401.525297-1-pbonzini@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | target/i386/hvf: hide MPX states from XCR0 | expand |
On Tue, 29 Oct 2024 at 14:05, Paolo Bonzini <pbonzini@redhat.com> wrote: > > QEMU does not show availability of MPX in CPUID when running under > Hypervisor.framework. Therefore, in the unlikely chance that the host > has MPX enabled, hide those bits from leaf 0xD as well. To clarify: is there some kind of issue with MPX in Qemu in general? Or is this a consistency effort - normal Macs don't expose this feature, so we have no idea if it were to work if someone did manage to hack up some frankensteinian host system that somehow does have those bits set? > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > target/i386/hvf/x86_cpuid.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/target/i386/hvf/x86_cpuid.c b/target/i386/hvf/x86_cpuid.c > index e56cd8411ba..4b184767f4a 100644 > --- a/target/i386/hvf/x86_cpuid.c > +++ b/target/i386/hvf/x86_cpuid.c > @@ -110,9 +110,9 @@ uint32_t hvf_get_supported_cpuid(uint32_t func, uint32_t idx, > if (idx == 0) { > uint64_t host_xcr0; > if (xgetbv(ecx, 0, &host_xcr0)) { > + /* Only show xcr0 bits corresponding to usable features. */ > uint64_t supp_xcr0 = host_xcr0 & (XSTATE_FP_MASK | > XSTATE_SSE_MASK | XSTATE_YMM_MASK | > - XSTATE_BNDREGS_MASK | XSTATE_BNDCSR_MASK | > XSTATE_OPMASK_MASK | XSTATE_ZMM_Hi256_MASK | > XSTATE_Hi16_ZMM_MASK); > eax &= supp_xcr0; > -- > 2.47.0 > >
On 10/30/24 13:30, Phil Dennis-Jordan wrote: > On Tue, 29 Oct 2024 at 14:05, Paolo Bonzini <pbonzini@redhat.com> wrote: >> >> QEMU does not show availability of MPX in CPUID when running under >> Hypervisor.framework. Therefore, in the unlikely chance that the host >> has MPX enabled, hide those bits from leaf 0xD as well. > > To clarify: is there some kind of issue with MPX in Qemu in general? > Or is this a consistency effort - normal Macs don't expose this > feature, so we have no idea if it were to work if someone did manage > to hack up some frankensteinian host system that somehow does have > those bits set? That, and also that real hardware will only show XSTATE_BNDREGS_MASK and XSTATE_BNDCSR_MASK if the MPX bit is set in CPUID; which it isn't in hvf_get_supported_cpuid(). In fact, for completeness it should also go the other way: if XSTATE_YMM_MASK is not set in the result of XGETBV, AVX should be hidden. And if any of OPMASK, ZMM_Hi256 and Hi16_ZMM are not set in the result of XGETBV, AVX512F (and AVX10 eventually) should be hidden in hvf_get_supported_cpuid(). By the way, could you check if Macs set the PKRU bit of XCR0 (bit 9) and/or the OSPKE bit in CPUID (that's bit 4 of CPUID[EAX=7, ECX=0].ECX)? Thanks, Paolo > >> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> >> --- >> target/i386/hvf/x86_cpuid.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/target/i386/hvf/x86_cpuid.c b/target/i386/hvf/x86_cpuid.c >> index e56cd8411ba..4b184767f4a 100644 >> --- a/target/i386/hvf/x86_cpuid.c >> +++ b/target/i386/hvf/x86_cpuid.c >> @@ -110,9 +110,9 @@ uint32_t hvf_get_supported_cpuid(uint32_t func, uint32_t idx, >> if (idx == 0) { >> uint64_t host_xcr0; >> if (xgetbv(ecx, 0, &host_xcr0)) { >> + /* Only show xcr0 bits corresponding to usable features. */ >> uint64_t supp_xcr0 = host_xcr0 & (XSTATE_FP_MASK | >> XSTATE_SSE_MASK | XSTATE_YMM_MASK | >> - XSTATE_BNDREGS_MASK | XSTATE_BNDCSR_MASK | >> XSTATE_OPMASK_MASK | XSTATE_ZMM_Hi256_MASK | >> XSTATE_Hi16_ZMM_MASK); >> eax &= supp_xcr0; >> -- >> 2.47.0 >> >> > >
On Wed, 30 Oct 2024 at 14:35, Paolo Bonzini <pbonzini@redhat.com> wrote: > > On 10/30/24 13:30, Phil Dennis-Jordan wrote: > > On Tue, 29 Oct 2024 at 14:05, Paolo Bonzini <pbonzini@redhat.com> wrote: > >> > >> QEMU does not show availability of MPX in CPUID when running under > >> Hypervisor.framework. Therefore, in the unlikely chance that the host > >> has MPX enabled, hide those bits from leaf 0xD as well. > > > > To clarify: is there some kind of issue with MPX in Qemu in general? > > Or is this a consistency effort - normal Macs don't expose this > > feature, so we have no idea if it were to work if someone did manage > > to hack up some frankensteinian host system that somehow does have > > those bits set? > > That, and also that real hardware will only show XSTATE_BNDREGS_MASK and > XSTATE_BNDCSR_MASK if the MPX bit is set in CPUID; which it isn't in > hvf_get_supported_cpuid(). Understood, and makes sense. Perhaps you could add this to the commit message as well? > In fact, for completeness it should also go the other way: if > XSTATE_YMM_MASK is not set in the result of XGETBV, AVX should be > hidden. And if any of OPMASK, ZMM_Hi256 and Hi16_ZMM are not set in the > result of XGETBV, AVX512F (and AVX10 eventually) should be hidden in > hvf_get_supported_cpuid(). Indeed. macOS does also have AVX512 support, and I believe there were two Mac models which shipped with Xeon CPUs that exposed some version of the feature, the iMac Pro and the 2019 Mac Pro. I don't have access to such a machine however, so I’m not sure exactly what the host support entails. (I don't know if HVF guests theoretically could support AVX512 features supported by the CPU but not the host OS as long as it only uses register state correctly saved by the host OS?) > By the way, could you check if Macs set the PKRU bit of XCR0 (bit 9) > and/or the OSPKE bit in CPUID (that's bit 4 of CPUID[EAX=7, ECX=0].ECX)? On a 2019 MacBook Pro 15" with a Coffee Lake Intel Core i9 9880H CPU, the OSPKE bit is not set. (cpuid 7,0 returns: ebx = 0x029c67af, ecx = 0x40000000, edx = 0xbc000e00) Assuming I'm doing this right, xcr0 on this machine appears to be 0x7, so there's no PKRU bit set. While trying to log xcr0, I noticed that xgetbv checks the passed ecx value against CPUID_EXT_OSXSAVE. Except as far as I'm aware, OSXSAVE is in cpuid function 1's ecx, whereas the code path in hvf_get_supported_cpuid() is, by my reading, passing the ecx value for cpuid function 0xD, index 0. The xgetbv call thus returns false on my machine (ecx = 0x440), and eax is not modified. (stays 0x1f rather than 0x7) It seems like we ought to either fetch the CPUID_EXT_OSXSAVE bit in another cpuid call, or cache it. Or do I have this wrong? Thanks, Phil
diff --git a/target/i386/hvf/x86_cpuid.c b/target/i386/hvf/x86_cpuid.c index e56cd8411ba..4b184767f4a 100644 --- a/target/i386/hvf/x86_cpuid.c +++ b/target/i386/hvf/x86_cpuid.c @@ -110,9 +110,9 @@ uint32_t hvf_get_supported_cpuid(uint32_t func, uint32_t idx, if (idx == 0) { uint64_t host_xcr0; if (xgetbv(ecx, 0, &host_xcr0)) { + /* Only show xcr0 bits corresponding to usable features. */ uint64_t supp_xcr0 = host_xcr0 & (XSTATE_FP_MASK | XSTATE_SSE_MASK | XSTATE_YMM_MASK | - XSTATE_BNDREGS_MASK | XSTATE_BNDCSR_MASK | XSTATE_OPMASK_MASK | XSTATE_ZMM_Hi256_MASK | XSTATE_Hi16_ZMM_MASK); eax &= supp_xcr0;
QEMU does not show availability of MPX in CPUID when running under Hypervisor.framework. Therefore, in the unlikely chance that the host has MPX enabled, hide those bits from leaf 0xD as well. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- target/i386/hvf/x86_cpuid.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)