From: Roman Bolshakov <r.bolshakov@yadro.com>
To: Hill Ma <maahiuzeon@gmail.com>
Cc: qemu-devel@nongnu.org, dirty@apple.com
Subject: Re: [PATCH] hvf: guard xgetbv call.
Date: Sat, 9 Jan 2021 08:48:19 +0300 [thread overview]
Message-ID: <X/lDozXFWfR4AZAU@SPB-NB-133.local> (raw)
In-Reply-To: <X91h2yoy7qVrO1kv@Hills-Mac-Pro.local>
On Fri, Dec 18, 2020 at 06:13:47PM -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 <maahiuzeon@gmail.com>
> ---
> target/i386/hvf/x86_cpuid.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
Hi Hill,
I'm sorry for delay with the review.
> diff --git a/target/i386/hvf/x86_cpuid.c b/target/i386/hvf/x86_cpuid.c
> index a6842912f5..b4b7111fc3 100644
> --- a/target/i386/hvf/x86_cpuid.c
> +++ b/target/i386/hvf/x86_cpuid.c
> @@ -100,11 +100,16 @@ 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 |
> + uint64_t supp_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);
> + XSTATE_ZMM_Hi256_MASK | XSTATE_Hi16_ZMM_MASK;
> + if ((ecx & CPUID_EXT_AVX) &&
> + (ecx & CPUID_EXT_XSAVE) &&
> + (ecx & CPUID_EXT_OSXSAVE)) {
It's sufficient to check only CPUID_EXT_OSXSAVE to ensure xgetbv
presence (per SDM Vol. 1 13-5):
Software operating with CPL > 0 may need to determine whether the
XSAVE feature set and certain XSAVE-enabled features have been
enabled. If CPL > 0, execution of the MOV from CR4 instruction causes
a general-protection fault (#GP). The following alternative mechanisms
allow software to discover the enabling of the XSAVE feature set
regardless of CPL:
* The value of CR4.OSXSAVE is returned in CPUID.1:ECX.OSXSAVE[bit 27].
If software determines that CPUID.1:ECX.OSXSAVE = 1, the processor
supports the XSAVE feature set and the feature set has been enabled in
CR4.
* Executing the XGETBV instruction with ECX = 0 returns the value of
XCR0 in EDX:EAX. XGETBV can be executed if CR4.OSXSAVE = 1 (if
CPUID.1:ECX.OSXSAVE = 1), regardless of CPL.
> + uint64_t host_xcr0 = xgetbv(0);
> + supp_xcr0 &= host_xcr0;
> + }
> eax &= supp_xcr0;
I think instead of the patch you can do:
- if (idx == 0) {
+ if (idx == 0 && (ecx & CPUID_EXT_OSXSAVE)) {
That'd keep host values returned from CPUID on platforms that don't
support XSAVE.
Thanks,
Roman
> } else if (idx == 1) {
> hv_vmx_read_capability(HV_VMX_CAP_PROCBASED2, &cap);
> --
> 2.20.1 (Apple Git-117)
>
next prev parent reply other threads:[~2021-01-09 5:48 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-12-19 2:13 [PATCH] hvf: guard xgetbv call Hill Ma
2021-01-09 5:48 ` Roman Bolshakov [this message]
2021-01-09 11:42 ` Peter Maydell
2021-01-10 1:46 ` Roman Bolshakov
2021-01-10 18:34 ` Richard Henderson
2021-01-10 18:38 ` Richard Henderson
2021-01-11 4:31 ` Roman Bolshakov
2021-01-11 17:06 ` Richard Henderson
2021-01-12 7:49 ` Roman Bolshakov
2021-01-12 15:28 ` Richard Henderson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=X/lDozXFWfR4AZAU@SPB-NB-133.local \
--to=r.bolshakov@yadro.com \
--cc=dirty@apple.com \
--cc=maahiuzeon@gmail.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.