From: Sean Christopherson <sean.j.christopherson@intel.com>
To: Xiaoyao Li <xiaoyao.li@intel.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
Vitaly Kuznetsov <vkuznets@redhat.com>,
Wanpeng Li <wanpengli@tencent.com>,
Jim Mattson <jmattson@google.com>, Joerg Roedel <joro@8bytes.org>,
kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
Pu Wen <puwen@hygon.cn>
Subject: Re: [PATCH v2 2/7] KVM: x86: Add helpers to perform CPUID-based guest vendor check
Date: Thu, 5 Mar 2020 11:58:26 -0800 [thread overview]
Message-ID: <20200305195826.GP11500@linux.intel.com> (raw)
In-Reply-To: <b752a4d4-b469-1a1f-c064-bf98a0467d49@intel.com>
On Thu, Mar 05, 2020 at 11:48:20AM +0800, Xiaoyao Li wrote:
> On 3/5/2020 9:34 AM, Sean Christopherson wrote:
> >Add helpers to provide CPUID-based guest vendor checks, i.e. to do the
> >ugly register comparisons. Use the new helpers to check for an AMD
> >guest vendor in guest_cpuid_is_amd() as well as in the existing emulator
> >flows.
> >
> >Using the new helpers fixes a _very_ theoretical bug where
> >guest_cpuid_is_amd() would get a false positive on a non-AMD virtual CPU
> >with a vendor string beginning with "Auth" due to the previous logic
> >only checking EBX. It also fixes a marginally less theoretically bug
> >where guest_cpuid_is_amd() would incorrectly return false for a guest
> >CPU with "AMDisbetter!" as its vendor string.
> >
> >Fixes: a0c0feb57992c ("KVM: x86: reserve bit 8 of non-leaf PDPEs and PML4Es in 64-bit mode on AMD")
> >Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
> >---
> > arch/x86/include/asm/kvm_emulate.h | 24 ++++++++++++++++++++
> > arch/x86/kvm/cpuid.h | 2 +-
> > arch/x86/kvm/emulate.c | 36 +++++++-----------------------
> > 3 files changed, 33 insertions(+), 29 deletions(-)
> >
> >diff --git a/arch/x86/include/asm/kvm_emulate.h b/arch/x86/include/asm/kvm_emulate.h
> >index bf5f5e476f65..2754972c36e6 100644
> >--- a/arch/x86/include/asm/kvm_emulate.h
> >+++ b/arch/x86/include/asm/kvm_emulate.h
> >@@ -393,6 +393,30 @@ struct x86_emulate_ctxt {
> > #define X86EMUL_CPUID_VENDOR_GenuineIntel_ecx 0x6c65746e
> > #define X86EMUL_CPUID_VENDOR_GenuineIntel_edx 0x49656e69
> >+static inline bool is_guest_vendor_intel(u32 ebx, u32 ecx, u32 edx)
> >+{
> >+ return ebx == X86EMUL_CPUID_VENDOR_GenuineIntel_ebx &&
> >+ ecx == X86EMUL_CPUID_VENDOR_GenuineIntel_ecx &&
> >+ edx == X86EMUL_CPUID_VENDOR_GenuineIntel_edx;
> >+}
> >+
> >+static inline bool is_guest_vendor_amd(u32 ebx, u32 ecx, u32 edx)
> >+{
> >+ return (ebx == X86EMUL_CPUID_VENDOR_AuthenticAMD_ebx &&
> >+ ecx == X86EMUL_CPUID_VENDOR_AuthenticAMD_ecx &&
> >+ edx == X86EMUL_CPUID_VENDOR_AuthenticAMD_edx) ||
> >+ (ebx == X86EMUL_CPUID_VENDOR_AMDisbetterI_ebx &&
> >+ ecx == X86EMUL_CPUID_VENDOR_AMDisbetterI_ecx &&
> >+ edx == X86EMUL_CPUID_VENDOR_AMDisbetterI_edx);
> >+}
> >+
> >+static inline bool is_guest_vendor_hygon(u32 ebx, u32 ecx, u32 edx)
> >+{
> >+ return ebx == X86EMUL_CPUID_VENDOR_HygonGenuine_ebx &&
> >+ ecx == X86EMUL_CPUID_VENDOR_HygonGenuine_ecx &&
> >+ edx == X86EMUL_CPUID_VENDOR_HygonGenuine_edx;
> >+}
> >+
>
> Why not define those in cpuid.h ?
> And also move X86EMUL_CPUID_VENDOR_* to cpuid.h and remove the "EMUL"
> prefix.
To avoid pulling cpuid.h into the emulator. Ideally, the emulator would
only use KVM APIs defined in kvm_emulate.h. Obviously that's a bit of a
pipe dream at the moment :-)
#include <linux/kvm_host.h>
#include "kvm_cache_regs.h"
#include <asm/kvm_emulate.h>
#include <linux/stringify.h>
#include <asm/fpu/api.h>
#include <asm/debugreg.h>
#include <asm/nospec-branch.h>
#include "x86.h"
#include "tss.h"
#include "mmu.h"
#include "pmu.h"
> > enum x86_intercept_stage {
> > X86_ICTP_NONE = 0, /* Allow zero-init to not match anything */
> > X86_ICPT_PRE_EXCEPT,
> >diff --git a/arch/x86/kvm/cpuid.h b/arch/x86/kvm/cpuid.h
> >index 7366c618aa04..13eb3e92c6a9 100644
> >--- a/arch/x86/kvm/cpuid.h
> >+++ b/arch/x86/kvm/cpuid.h
> >@@ -145,7 +145,7 @@ static inline bool guest_cpuid_is_amd(struct kvm_vcpu *vcpu)
> > struct kvm_cpuid_entry2 *best;
> > best = kvm_find_cpuid_entry(vcpu, 0, 0);
> >- return best && best->ebx == X86EMUL_CPUID_VENDOR_AuthenticAMD_ebx;
> >+ return best && is_guest_vendor_amd(best->ebx, best->ecx, best->edx);
> > }
> > static inline int guest_cpuid_family(struct kvm_vcpu *vcpu)
> >diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> >index dd19fb3539e0..9cf303984fe5 100644
> >--- a/arch/x86/kvm/emulate.c
> >+++ b/arch/x86/kvm/emulate.c
> >@@ -2712,9 +2712,7 @@ static bool vendor_intel(struct x86_emulate_ctxt *ctxt)
> > eax = ecx = 0;
> > ctxt->ops->get_cpuid(ctxt, &eax, &ebx, &ecx, &edx, false);
> >- return ebx == X86EMUL_CPUID_VENDOR_GenuineIntel_ebx
> >- && ecx == X86EMUL_CPUID_VENDOR_GenuineIntel_ecx
> >- && edx == X86EMUL_CPUID_VENDOR_GenuineIntel_edx;
> >+ return is_guest_vendor_intel(ebx, ecx, edx);
> > }
> > static bool em_syscall_is_enabled(struct x86_emulate_ctxt *ctxt)
> >@@ -2733,34 +2731,16 @@ static bool em_syscall_is_enabled(struct x86_emulate_ctxt *ctxt)
> > ecx = 0x00000000;
> > ops->get_cpuid(ctxt, &eax, &ebx, &ecx, &edx, false);
> > /*
> >- * Intel ("GenuineIntel")
> >- * remark: Intel CPUs only support "syscall" in 64bit
> >- * longmode. Also an 64bit guest with a
> >- * 32bit compat-app running will #UD !! While this
> >- * behaviour can be fixed (by emulating) into AMD
> >- * response - CPUs of AMD can't behave like Intel.
> >+ * remark: Intel CPUs only support "syscall" in 64bit longmode. Also a
> >+ * 64bit guest with a 32bit compat-app running will #UD !! While this
> >+ * behaviour can be fixed (by emulating) into AMD response - CPUs of
> >+ * AMD can't behave like Intel.
> > */
> >- if (ebx == X86EMUL_CPUID_VENDOR_GenuineIntel_ebx &&
> >- ecx == X86EMUL_CPUID_VENDOR_GenuineIntel_ecx &&
> >- edx == X86EMUL_CPUID_VENDOR_GenuineIntel_edx)
> >+ if (is_guest_vendor_intel(ebx, ecx, edx))
> > return false;
> >- /* AMD ("AuthenticAMD") */
> >- if (ebx == X86EMUL_CPUID_VENDOR_AuthenticAMD_ebx &&
> >- ecx == X86EMUL_CPUID_VENDOR_AuthenticAMD_ecx &&
> >- edx == X86EMUL_CPUID_VENDOR_AuthenticAMD_edx)
> >- return true;
> >-
> >- /* AMD ("AMDisbetter!") */
> >- if (ebx == X86EMUL_CPUID_VENDOR_AMDisbetterI_ebx &&
> >- ecx == X86EMUL_CPUID_VENDOR_AMDisbetterI_ecx &&
> >- edx == X86EMUL_CPUID_VENDOR_AMDisbetterI_edx)
> >- return true;
> >-
> >- /* Hygon ("HygonGenuine") */
> >- if (ebx == X86EMUL_CPUID_VENDOR_HygonGenuine_ebx &&
> >- ecx == X86EMUL_CPUID_VENDOR_HygonGenuine_ecx &&
> >- edx == X86EMUL_CPUID_VENDOR_HygonGenuine_edx)
> >+ if (is_guest_vendor_amd(ebx, ecx, edx) ||
> >+ is_guest_vendor_hygon(ebx, ecx, edx))
> > return true;
> > /*
> >
>
next prev parent reply other threads:[~2020-03-05 19:58 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-05 1:34 [PATCH v2 0/7] KVM: x86: CPUID emulation and tracing fixes Sean Christopherson
2020-03-05 1:34 ` [PATCH v2 1/7] KVM: x86: Trace the original requested CPUID function in kvm_cpuid() Sean Christopherson
2020-03-05 17:50 ` Jim Mattson
2020-03-05 1:34 ` [PATCH v2 2/7] KVM: x86: Add helpers to perform CPUID-based guest vendor check Sean Christopherson
2020-03-05 3:48 ` Xiaoyao Li
2020-03-05 19:58 ` Sean Christopherson [this message]
2020-03-05 18:07 ` Jim Mattson
2020-03-05 1:34 ` [PATCH v2 3/7] KVM x86: Extend AMD specific guest behavior to Hygon virtual CPUs Sean Christopherson
2020-03-05 1:34 ` [PATCH v2 4/7] KVM: x86: Fix CPUID range checks for Hypervisor and Centaur classes Sean Christopherson
2020-03-05 18:43 ` Jim Mattson
2020-03-05 19:25 ` Sean Christopherson
2020-03-05 21:10 ` Jim Mattson
2020-03-05 21:51 ` Sean Christopherson
2020-03-06 9:03 ` Paolo Bonzini
2020-03-10 17:10 ` Sean Christopherson
2020-03-10 17:23 ` Jim Mattson
2020-03-05 1:34 ` [PATCH v2 5/7] KVM: x86: Add build-time assertions on validity of vendor strings Sean Christopherson
2020-03-05 1:34 ` [PATCH v2 6/7] KVM: x86: Refactor out-of-range logic to contain the madness Sean Christopherson
2020-03-05 1:34 ` [PATCH v2 7/7] KVM: x86: Refactor kvm_cpuid() param that controls out-of-range logic Sean Christopherson
2020-03-05 16:42 ` [PATCH v2 0/7] KVM: x86: CPUID emulation and tracing fixes Paolo Bonzini
2020-03-05 17:12 ` Sean Christopherson
2020-03-06 8:45 ` Paolo Bonzini
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=20200305195826.GP11500@linux.intel.com \
--to=sean.j.christopherson@intel.com \
--cc=jmattson@google.com \
--cc=joro@8bytes.org \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=pbonzini@redhat.com \
--cc=puwen@hygon.cn \
--cc=vkuznets@redhat.com \
--cc=wanpengli@tencent.com \
--cc=xiaoyao.li@intel.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox