From: Paolo Bonzini <pbonzini@redhat.com>
To: Sean Christopherson <seanjc@google.com>
Cc: 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,
Babu Moger <babu.moger@amd.com>,
Joao Martins <joao.m.martins@oracle.com>,
David Woodhouse <dwmw@amazon.co.uk>
Subject: Re: [PATCH v2 1/9] KVM: x86: Remove emulator's broken checks on CR0/CR3/CR4 loads
Date: Thu, 22 Apr 2021 08:50:36 +0200 [thread overview]
Message-ID: <856568f8-bf57-9806-a0f6-e5136a623be4@redhat.com> (raw)
In-Reply-To: <20210422022128.3464144-2-seanjc@google.com>
On 22/04/21 04:21, Sean Christopherson wrote:
> Remove the emulator's checks for illegal CR0, CR3, and CR4 values, as
> the checks are redundant, outdated, and in the case of SEV's C-bit,
> broken. The emulator manually calculates MAXPHYADDR from CPUID and
> neglects to mask off the C-bit. For all other checks, kvm_set_cr*() are
> a superset of the emulator checks, e.g. see CR4.LA57.
>
> Fixes: a780a3ea6282 ("KVM: X86: Fix reserved bits check for MOV to CR3")
> Cc: Babu Moger <babu.moger@amd.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
> arch/x86/kvm/emulate.c | 68 +-----------------------------------------
> 1 file changed, 1 insertion(+), 67 deletions(-)
This can be (opportunistically ;)) squashed on top:
diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index f4273b8e31fa..abd9a4db11a8 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -4220,15 +4220,7 @@ static bool valid_cr(int nr)
}
}
-static int check_cr_read(struct x86_emulate_ctxt *ctxt)
-{
- if (!valid_cr(ctxt->modrm_reg))
- return emulate_ud(ctxt);
-
- return X86EMUL_CONTINUE;
-}
-
-static int check_cr_write(struct x86_emulate_ctxt *ctxt)
+static int check_cr_access(struct x86_emulate_ctxt *ctxt)
{
if (!valid_cr(ctxt->modrm_reg))
return emulate_ud(ctxt);
@@ -4775,10 +4767,10 @@ static const struct opcode twobyte_table[256] = {
D(ImplicitOps | ModRM | SrcMem | NoAccess), /* 8 * reserved NOP */
D(ImplicitOps | ModRM | SrcMem | NoAccess), /* NOP + 7 * reserved NOP */
/* 0x20 - 0x2F */
- DIP(ModRM | DstMem | Priv | Op3264 | NoMod, cr_read, check_cr_read),
+ DIP(ModRM | DstMem | Priv | Op3264 | NoMod, cr_read, check_cr_access),
DIP(ModRM | DstMem | Priv | Op3264 | NoMod, dr_read, check_dr_read),
IIP(ModRM | SrcMem | Priv | Op3264 | NoMod, em_cr_write, cr_write,
- check_cr_write),
+ check_cr_access),
IIP(ModRM | SrcMem | Priv | Op3264 | NoMod, em_dr_write, dr_write,
check_dr_write),
N, N, N, N,
> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> index f7970ba6219f..f4273b8e31fa 100644
> --- a/arch/x86/kvm/emulate.c
> +++ b/arch/x86/kvm/emulate.c
> @@ -4230,75 +4230,9 @@ static int check_cr_read(struct x86_emulate_ctxt *ctxt)
>
> static int check_cr_write(struct x86_emulate_ctxt *ctxt)
> {
> - u64 new_val = ctxt->src.val64;
> - int cr = ctxt->modrm_reg;
> - u64 efer = 0;
> -
> - static u64 cr_reserved_bits[] = {
> - 0xffffffff00000000ULL,
> - 0, 0, 0, /* CR3 checked later */
> - CR4_RESERVED_BITS,
> - 0, 0, 0,
> - CR8_RESERVED_BITS,
> - };
> -
> - if (!valid_cr(cr))
> + if (!valid_cr(ctxt->modrm_reg))
> return emulate_ud(ctxt);
>
> - if (new_val & cr_reserved_bits[cr])
> - return emulate_gp(ctxt, 0);
> -
> - switch (cr) {
> - case 0: {
> - u64 cr4;
> - if (((new_val & X86_CR0_PG) && !(new_val & X86_CR0_PE)) ||
> - ((new_val & X86_CR0_NW) && !(new_val & X86_CR0_CD)))
> - return emulate_gp(ctxt, 0);
> -
> - cr4 = ctxt->ops->get_cr(ctxt, 4);
> - ctxt->ops->get_msr(ctxt, MSR_EFER, &efer);
> -
> - if ((new_val & X86_CR0_PG) && (efer & EFER_LME) &&
> - !(cr4 & X86_CR4_PAE))
> - return emulate_gp(ctxt, 0);
> -
> - break;
> - }
> - case 3: {
> - u64 rsvd = 0;
> -
> - ctxt->ops->get_msr(ctxt, MSR_EFER, &efer);
> - if (efer & EFER_LMA) {
> - u64 maxphyaddr;
> - u32 eax, ebx, ecx, edx;
> -
> - eax = 0x80000008;
> - ecx = 0;
> - if (ctxt->ops->get_cpuid(ctxt, &eax, &ebx, &ecx,
> - &edx, true))
> - maxphyaddr = eax & 0xff;
> - else
> - maxphyaddr = 36;
> - rsvd = rsvd_bits(maxphyaddr, 63);
> - if (ctxt->ops->get_cr(ctxt, 4) & X86_CR4_PCIDE)
> - rsvd &= ~X86_CR3_PCID_NOFLUSH;
> - }
> -
> - if (new_val & rsvd)
> - return emulate_gp(ctxt, 0);
> -
> - break;
> - }
> - case 4: {
> - ctxt->ops->get_msr(ctxt, MSR_EFER, &efer);
> -
> - if ((efer & EFER_LMA) && !(new_val & X86_CR4_PAE))
> - return emulate_gp(ctxt, 0);
> -
> - break;
> - }
> - }
> -
> return X86EMUL_CONTINUE;
> }
>
>
next prev parent reply other threads:[~2021-04-22 6:50 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-22 2:21 [PATCH v2 0/9] KVM: x86: Fixes for (benign?) truncation bugs Sean Christopherson
2021-04-22 2:21 ` [PATCH v2 1/9] KVM: x86: Remove emulator's broken checks on CR0/CR3/CR4 loads Sean Christopherson
2021-04-22 6:50 ` Paolo Bonzini [this message]
2021-04-22 2:21 ` [PATCH v2 2/9] KVM: x86: Check CR3 GPA for validity regardless of vCPU mode Sean Christopherson
2021-04-22 6:51 ` Paolo Bonzini
2021-04-22 16:55 ` Sean Christopherson
2021-04-22 2:21 ` [PATCH v2 3/9] KVM: SVM: Truncate GPR value for DR and CR accesses in !64-bit mode Sean Christopherson
2021-04-22 2:21 ` [PATCH v2 4/9] KVM: VMX: Truncate GPR value for DR and CR reads " Sean Christopherson
2021-04-22 2:21 ` [PATCH v2 5/9] KVM: nVMX: Truncate bits 63:32 of VMCS field on nested check in !64-bit Sean Christopherson
2021-04-22 2:21 ` [PATCH v2 6/9] KVM: nVMX: Truncate base/index GPR value on address calc " Sean Christopherson
2021-04-22 2:21 ` [PATCH v2 7/9] KVM: x86/xen: Drop RAX[63:32] when processing hypercall Sean Christopherson
2021-04-22 9:51 ` Vitaly Kuznetsov
2021-04-22 10:35 ` Paolo Bonzini
2021-04-22 10:49 ` Vitaly Kuznetsov
2021-04-22 2:21 ` [PATCH v2 8/9] KVM: SVM: Use default rAX size for INVLPGA emulation Sean Christopherson
2021-04-22 2:21 ` [PATCH v2 9/9] KVM: x86: Rename GPR accessors to make mode-aware variants the defaults Sean Christopherson
2021-04-22 6:55 ` [PATCH v2 0/9] KVM: x86: Fixes for (benign?) truncation bugs 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=856568f8-bf57-9806-a0f6-e5136a623be4@redhat.com \
--to=pbonzini@redhat.com \
--cc=babu.moger@amd.com \
--cc=dwmw@amazon.co.uk \
--cc=jmattson@google.com \
--cc=joao.m.martins@oracle.com \
--cc=joro@8bytes.org \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=seanjc@google.com \
--cc=vkuznets@redhat.com \
--cc=wanpengli@tencent.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;
as well as URLs for NNTP newsgroup(s).