All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: Kai Huang <kai.huang@intel.com>
Cc: "pbonzini@redhat.com" <pbonzini@redhat.com>,
	"vkuznets@redhat.com" <vkuznets@redhat.com>,
	 "dwmw2@infradead.org" <dwmw2@infradead.org>,
	"paul@xen.org" <paul@xen.org>,
	 "kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	 "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"yosry@kernel.org" <yosry@kernel.org>
Subject: Re: [PATCH 04/11] KVM: VMX: Read 32-bit GPR values for ENCLS instructions outside of 64-bit mode
Date: Wed, 15 Apr 2026 14:37:55 -0700	[thread overview]
Message-ID: <aeAFMwBTUCdeq1JS@google.com> (raw)
In-Reply-To: <dea50f27311d88dcb09ea98261ca280853396a42.camel@intel.com>

On Mon, Apr 13, 2026, Kai Huang wrote:
> On Thu, 2026-04-09 at 16:56 -0700, Sean Christopherson wrote:
> > When getting register values for ENCLS emulation, use kvm_register_read()
> > instead of kvm_<reg>_read() so that bits 63:32 of the register are dropped
> > if the guest is in 32-bit mode.
> > 
> > Note, the misleading/surprising behavior of kvm_<reg>_read() being "raw"
> > variants under the hood will be addressed once all non-benign bugs are
> > fixed.
> > 
> > Fixes: 70210c044b4e ("KVM: VMX: Add SGX ENCLS[ECREATE] handler to enforce CPUID restrictions")
> > Fixes: b6f084ca5538 ("KVM: VMX: Add ENCLS[EINIT] handler to support SGX Launch Control (LC)")
> > Signed-off-by: Sean Christopherson <seanjc@google.com>
> > ---
> >  arch/x86/kvm/vmx/sgx.c | 10 +++++-----
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> > 
> > diff --git a/arch/x86/kvm/vmx/sgx.c b/arch/x86/kvm/vmx/sgx.c
> > index df1d0cf76947..4c61fc33f764 100644
> > --- a/arch/x86/kvm/vmx/sgx.c
> > +++ b/arch/x86/kvm/vmx/sgx.c
> > @@ -225,8 +225,8 @@ static int handle_encls_ecreate(struct kvm_vcpu *vcpu)
> >  	struct x86_exception ex;
> >  	int r;
> >  
> > -	if (sgx_get_encls_gva(vcpu, kvm_rbx_read(vcpu), 32, 32, &pageinfo_gva) ||
> > -	    sgx_get_encls_gva(vcpu, kvm_rcx_read(vcpu), 4096, 4096, &secs_gva))
> > +	if (sgx_get_encls_gva(vcpu, kvm_register_read(vcpu, VCPU_REGS_RBX), 32, 32, &pageinfo_gva) ||
> > +	    sgx_get_encls_gva(vcpu, kvm_register_read(vcpu, VCPU_REGS_RCX), 4096, 4096, &secs_gva))
> >  		return 1;
> >  
> >  	/*
> > @@ -302,9 +302,9 @@ static int handle_encls_einit(struct kvm_vcpu *vcpu)
> >  	gpa_t sig_gpa, secs_gpa, token_gpa;
> >  	int ret, trapnr;
> >  
> > -	if (sgx_get_encls_gva(vcpu, kvm_rbx_read(vcpu), 1808, 4096, &sig_gva) ||
> > -	    sgx_get_encls_gva(vcpu, kvm_rcx_read(vcpu), 4096, 4096, &secs_gva) ||
> > -	    sgx_get_encls_gva(vcpu, kvm_rdx_read(vcpu), 304, 512, &token_gva))
> > +	if (sgx_get_encls_gva(vcpu, kvm_register_read(vcpu, VCPU_REGS_RBX), 1808, 4096, &sig_gva) ||
> > +	    sgx_get_encls_gva(vcpu, kvm_register_read(vcpu, VCPU_REGS_RCX), 4096, 4096, &secs_gva) ||
> > +	    sgx_get_encls_gva(vcpu, kvm_register_read(vcpu, VCPU_REGS_RDX), 304, 512, &token_gva))
> >  		return 1;
> > 
> 
> Is there any case where bits 63:32 can have non-zero value?

Yes, GPR values aren't modified on transitions to/from 64-bit mode.  E.g. if
software loads 64-bit values in 64-bit mode, under the hood those values will
still be there while the CPU is in 32-bit/compat mode.

Well, that's not strictly true, per the SDM.  Values are only preseverd for R8-R15
on compat<=>64-bit transitions:

  Registers only available in 64-bit mode (R8-R15 and XMM8-XMM15) are preserved
  across transitions from 64-bit mode into compatibility mode then back into
  64-bit mode. However, values of R8-R15 and XMM8-XMM15 are unde- fined after
  transitions from 64-bit mode through compatibility mode to legacy or real mode
  and then back through compatibility mode to 64-bit mode.

And "legacy" GPRs are never preserved:

  Because the upper 32 bits of 64-bit general-purpose registers are undefined in
  32-bit modes, the upper 32 bits of any general-purpose register are not preserved
  when switching from 64-bit mode to a 32-bit mode (to protected mode or
  compatibility mode). Software must not depend on these bits to maintain a value
  after a 64-bit to 32-bit mode switch.

But IIRC, that's "just" the architectural behavior.  Hardware implementations may
choose to preserve values.

> If vCPU is in 32-bit mode then it should not be able to access 64-bit GPR?

Yes and no.  Mostly no.  Architecturally, they're all off limits.  But, again
going from memory that's ~15 years old at this point, IIRC the behavior is that
writes in 32-bit modes zero bits 63:32, same as 32-bit writes in 64-bit mode.

Take all of my memory with a huge grain of salt, it's very possible I'm
mis-remembering hallway discussions from a long time ago.

  reply	other threads:[~2026-04-15 21:37 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-09 23:56 [PATCH 00/11] KVM: x86: Clean up kvm_<reg>_{read,write}() mess Sean Christopherson
2026-04-09 23:56 ` [PATCH 01/11] KVM: SVM: Truncate INVLPGA address in compatibility mode Sean Christopherson
2026-04-21 23:26   ` Yosry Ahmed
2026-04-09 23:56 ` [PATCH 02/11] KVM: x86/xen: Bug the VM if 32-bit KVM observes a 64-bit mode hypercall Sean Christopherson
2026-04-09 23:56 ` [PATCH 03/11] KVM: x86/xen: Don't truncate RAX when handling hypercall from protected guest Sean Christopherson
2026-04-13 10:36   ` Binbin Wu
2026-04-15 21:29     ` Sean Christopherson
2026-04-09 23:56 ` [PATCH 04/11] KVM: VMX: Read 32-bit GPR values for ENCLS instructions outside of 64-bit mode Sean Christopherson
2026-04-13 12:19   ` Huang, Kai
2026-04-15 21:37     ` Sean Christopherson [this message]
2026-04-15 23:32       ` Huang, Kai
2026-04-16  0:27         ` Sean Christopherson
2026-04-16  1:40           ` Huang, Kai
2026-04-09 23:56 ` [PATCH 05/11] KVM: x86: Trace hypercall register *after* truncating values for 32-bit Sean Christopherson
2026-04-21 23:27   ` Yosry Ahmed
2026-04-09 23:56 ` [PATCH 06/11] KVM: x86: Move kvm_<reg>_{read,write}() definitions to x86.h Sean Christopherson
2026-04-21 23:32   ` Yosry Ahmed
2026-04-22  0:40     ` Sean Christopherson
2026-04-22 20:03       ` Yosry Ahmed
2026-04-23 19:17         ` Sean Christopherson
2026-04-09 23:56 ` [PATCH 07/11] KVM: x86: Add mode-aware versions of kvm_<reg>_{read,write}() helpers Sean Christopherson
2026-04-14  8:26   ` Huang, Kai
2026-04-14 15:42     ` Sean Christopherson
2026-04-14 22:40       ` Huang, Kai
2026-04-14  9:02   ` Binbin Wu
2026-04-23 22:12     ` Sean Christopherson
2026-04-09 23:56 ` [PATCH 08/11] KVM: x86: Drop non-raw kvm_<reg>_write() helpers Sean Christopherson
2026-04-09 23:56 ` [PATCH 09/11] KVM: nSVM: Use kvm_rax_read() now that it's mode-aware Sean Christopherson
2026-04-21 23:19   ` Yosry Ahmed
2026-04-09 23:56 ` [PATCH 10/11] Revert "KVM: VMX: Read 32-bit GPR values for ENCLS instructions outside of 64-bit mode" Sean Christopherson
2026-04-16  1:42   ` Huang, Kai
2026-04-09 23:56 ` [PATCH 11/11] KVM: x86: Harden is_64_bit_hypercall() against bugs on 32-bit kernels Sean Christopherson
2026-04-16  1:43   ` Huang, Kai

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=aeAFMwBTUCdeq1JS@google.com \
    --to=seanjc@google.com \
    --cc=dwmw2@infradead.org \
    --cc=kai.huang@intel.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=paul@xen.org \
    --cc=pbonzini@redhat.com \
    --cc=vkuznets@redhat.com \
    --cc=yosry@kernel.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.