Re: [PATCH v3] hvf: guard xgetbv call.
Reviewed-by: Cameron Esfahani mailto:di...@apple.com>> Cameron Esfahani di...@apple.com > On Jan 12, 2021, at 10:07 PM, Hill Ma wrote: > > This prevents illegal instruction on cpus do not support xgetbv. > > Buglink: https://bugs.launchpad.net/qemu/+bug/1758819 > Signed-off-by: Hill Ma > --- > v3: addressed feedback. > v2: xgetbv() modified based on feedback. > > target/i386/hvf/x86_cpuid.c | 34 ++ > 1 file changed, 22 insertions(+), 12 deletions(-) > > diff --git a/target/i386/hvf/x86_cpuid.c b/target/i386/hvf/x86_cpuid.c > index a6842912f5..32b0d131df 100644 > --- a/target/i386/hvf/x86_cpuid.c > +++ b/target/i386/hvf/x86_cpuid.c > @@ -27,15 +27,22 @@ > #include "vmx.h" > #include "sysemu/hvf.h" > > -static uint64_t xgetbv(uint32_t xcr) > +static bool xgetbv(uint32_t cpuid_ecx, uint32_t idx, uint64_t *xcr) > { > -uint32_t eax, edx; > +uint32_t xcrl, xcrh; > > -__asm__ volatile ("xgetbv" > - : "=a" (eax), "=d" (edx) > - : "c" (xcr)); > +if (cpuid_ecx & CPUID_EXT_OSXSAVE) { > +/* > + * The xgetbv instruction is not available to older versions of > + * the assembler, so we encode the instruction manually. > + */ > +asm(".byte 0x0f, 0x01, 0xd0" : "=a" (xcrl), "=d" (xcrh) : "c" (idx)); > > -return (((uint64_t)edx) << 32) | eax; > +*xcr = (((uint64_t)xcrh) << 32) | xcrl; > +return true; > +} > + > +return false; > } > > uint32_t hvf_get_supported_cpuid(uint32_t func, uint32_t idx, > @@ -100,12 +107,15 @@ uint32_t hvf_get_supported_cpuid(uint32_t func, > uint32_t idx, > break; > case 0xD: > if (idx == 0) { > -uint64_t host_xcr0 = xgetbv(0); > -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; > +uint64_t host_xcr0; > +if (xgetbv(ecx, 0, &host_xcr0)) { > +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; > +} > } else if (idx == 1) { > hv_vmx_read_capability(HV_VMX_CAP_PROCBASED2, &cap); > eax &= CPUID_XSAVE_XSAVEOPT | CPUID_XSAVE_XGETBV1; > -- > 2.20.1 (Apple Git-117) >
Re: [PATCH v3] hvf: guard xgetbv call.
On Tue, Jan 12, 2021 at 10:07:35PM -0800, Hill Ma wrote: > This prevents illegal instruction on cpus do not support xgetbv. > > Buglink: https://bugs.launchpad.net/qemu/+bug/1758819 > Signed-off-by: Hill Ma > --- > v3: addressed feedback. > v2: xgetbv() modified based on feedback. > > target/i386/hvf/x86_cpuid.c | 34 ++ > 1 file changed, 22 insertions(+), 12 deletions(-) > Reviewed-by: Roman Bolshakov Tested-by: Roman Bolshakov Thanks, Roman > diff --git a/target/i386/hvf/x86_cpuid.c b/target/i386/hvf/x86_cpuid.c > index a6842912f5..32b0d131df 100644 > --- a/target/i386/hvf/x86_cpuid.c > +++ b/target/i386/hvf/x86_cpuid.c > @@ -27,15 +27,22 @@ > #include "vmx.h" > #include "sysemu/hvf.h" > > -static uint64_t xgetbv(uint32_t xcr) > +static bool xgetbv(uint32_t cpuid_ecx, uint32_t idx, uint64_t *xcr) > { > -uint32_t eax, edx; > +uint32_t xcrl, xcrh; > > -__asm__ volatile ("xgetbv" > - : "=a" (eax), "=d" (edx) > - : "c" (xcr)); > +if (cpuid_ecx & CPUID_EXT_OSXSAVE) { > +/* > + * The xgetbv instruction is not available to older versions of > + * the assembler, so we encode the instruction manually. > + */ > +asm(".byte 0x0f, 0x01, 0xd0" : "=a" (xcrl), "=d" (xcrh) : "c" (idx)); > > -return (((uint64_t)edx) << 32) | eax; > +*xcr = (((uint64_t)xcrh) << 32) | xcrl; > +return true; > +} > + > +return false; > } > > uint32_t hvf_get_supported_cpuid(uint32_t func, uint32_t idx, > @@ -100,12 +107,15 @@ uint32_t hvf_get_supported_cpuid(uint32_t func, > uint32_t idx, > break; > case 0xD: > if (idx == 0) { > -uint64_t host_xcr0 = xgetbv(0); > -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; > +uint64_t host_xcr0; > +if (xgetbv(ecx, 0, &host_xcr0)) { > +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; > +} > } else if (idx == 1) { > hv_vmx_read_capability(HV_VMX_CAP_PROCBASED2, &cap); > eax &= CPUID_XSAVE_XSAVEOPT | CPUID_XSAVE_XGETBV1; > -- > 2.20.1 (Apple Git-117) >
Re: [PATCH v3] hvf: guard xgetbv call.
gentle bump :) On Tue, Jan 12, 2021 at 10:07 PM Hill Ma wrote: > > This prevents illegal instruction on cpus do not support xgetbv. > > Buglink: https://bugs.launchpad.net/qemu/+bug/1758819 > Signed-off-by: Hill Ma > --- > v3: addressed feedback. > v2: xgetbv() modified based on feedback. > > target/i386/hvf/x86_cpuid.c | 34 ++ > 1 file changed, 22 insertions(+), 12 deletions(-) > > diff --git a/target/i386/hvf/x86_cpuid.c b/target/i386/hvf/x86_cpuid.c > index a6842912f5..32b0d131df 100644 > --- a/target/i386/hvf/x86_cpuid.c > +++ b/target/i386/hvf/x86_cpuid.c > @@ -27,15 +27,22 @@ > #include "vmx.h" > #include "sysemu/hvf.h" > > -static uint64_t xgetbv(uint32_t xcr) > +static bool xgetbv(uint32_t cpuid_ecx, uint32_t idx, uint64_t *xcr) > { > -uint32_t eax, edx; > +uint32_t xcrl, xcrh; > > -__asm__ volatile ("xgetbv" > - : "=a" (eax), "=d" (edx) > - : "c" (xcr)); > +if (cpuid_ecx & CPUID_EXT_OSXSAVE) { > +/* > + * The xgetbv instruction is not available to older versions of > + * the assembler, so we encode the instruction manually. > + */ > +asm(".byte 0x0f, 0x01, 0xd0" : "=a" (xcrl), "=d" (xcrh) : "c" (idx)); > > -return (((uint64_t)edx) << 32) | eax; > +*xcr = (((uint64_t)xcrh) << 32) | xcrl; > +return true; > +} > + > +return false; > } > > uint32_t hvf_get_supported_cpuid(uint32_t func, uint32_t idx, > @@ -100,12 +107,15 @@ uint32_t hvf_get_supported_cpuid(uint32_t func, > uint32_t idx, > break; > case 0xD: > if (idx == 0) { > -uint64_t host_xcr0 = xgetbv(0); > -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; > +uint64_t host_xcr0; > +if (xgetbv(ecx, 0, &host_xcr0)) { > +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; > +} > } else if (idx == 1) { > hv_vmx_read_capability(HV_VMX_CAP_PROCBASED2, &cap); > eax &= CPUID_XSAVE_XSAVEOPT | CPUID_XSAVE_XGETBV1; > -- > 2.20.1 (Apple Git-117) >
[PATCH v3] hvf: guard xgetbv call.
This prevents illegal instruction on cpus do not support xgetbv. Buglink: https://bugs.launchpad.net/qemu/+bug/1758819 Signed-off-by: Hill Ma --- v3: addressed feedback. v2: xgetbv() modified based on feedback. target/i386/hvf/x86_cpuid.c | 34 ++ 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/target/i386/hvf/x86_cpuid.c b/target/i386/hvf/x86_cpuid.c index a6842912f5..32b0d131df 100644 --- a/target/i386/hvf/x86_cpuid.c +++ b/target/i386/hvf/x86_cpuid.c @@ -27,15 +27,22 @@ #include "vmx.h" #include "sysemu/hvf.h" -static uint64_t xgetbv(uint32_t xcr) +static bool xgetbv(uint32_t cpuid_ecx, uint32_t idx, uint64_t *xcr) { -uint32_t eax, edx; +uint32_t xcrl, xcrh; -__asm__ volatile ("xgetbv" - : "=a" (eax), "=d" (edx) - : "c" (xcr)); +if (cpuid_ecx & CPUID_EXT_OSXSAVE) { +/* + * The xgetbv instruction is not available to older versions of + * the assembler, so we encode the instruction manually. + */ +asm(".byte 0x0f, 0x01, 0xd0" : "=a" (xcrl), "=d" (xcrh) : "c" (idx)); -return (((uint64_t)edx) << 32) | eax; +*xcr = (((uint64_t)xcrh) << 32) | xcrl; +return true; +} + +return false; } uint32_t hvf_get_supported_cpuid(uint32_t func, uint32_t idx, @@ -100,12 +107,15 @@ uint32_t hvf_get_supported_cpuid(uint32_t func, uint32_t idx, break; case 0xD: if (idx == 0) { -uint64_t host_xcr0 = xgetbv(0); -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; +uint64_t host_xcr0; +if (xgetbv(ecx, 0, &host_xcr0)) { +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; +} } else if (idx == 1) { hv_vmx_read_capability(HV_VMX_CAP_PROCBASED2, &cap); eax &= CPUID_XSAVE_XSAVEOPT | CPUID_XSAVE_XGETBV1; -- 2.20.1 (Apple Git-117)