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>,
	"kas@kernel.org" <kas@kernel.org>,
	 "vkuznets@redhat.com" <vkuznets@redhat.com>,
	"dwmw2@infradead.org" <dwmw2@infradead.org>,
	"paul@xen.org" <paul@xen.org>,
	 Rick P Edgecombe <rick.p.edgecombe@intel.com>,
	"x86@kernel.org" <x86@kernel.org>,
	 "binbin.wu@linux.intel.com" <binbin.wu@linux.intel.com>,
	 "dave.hansen@linux.intel.com" <dave.hansen@linux.intel.com>,
	 "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"yosry@kernel.org" <yosry@kernel.org>,
	 "kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	 "linux-coco@lists.linux.dev" <linux-coco@lists.linux.dev>
Subject: Re: [PATCH v2 08/15] KVM: x86: Add mode-aware versions of kvm_<reg>_{read,write}() helpers
Date: Mon, 18 May 2026 13:51:10 -0700	[thread overview]
Message-ID: <agt7vnVL5rJJUVzo@google.com> (raw)
In-Reply-To: <136d277dba2ac681ed7607a436f55e2fd1975ec5.camel@intel.com>

On Mon, May 18, 2026, Kai Huang wrote:
> 
> > @@ -10413,29 +10413,30 @@ static int complete_hypercall_exit(struct kvm_vcpu *vcpu)
> >  
> >  	if (!is_64_bit_hypercall(vcpu))
> >  		ret = (u32)ret;
> > -	kvm_rax_write(vcpu, ret);
> > +	kvm_rax_write_raw(vcpu, ret);
> >  	return kvm_skip_emulated_instruction(vcpu);
> >  }
> > 
> 
> Nit:  AFAICT if we use kvm_rax_write(vcpu, ret) instead of the "raw" version
> here, we can then remove the
> 
> 	if (!is_64_bit_hypercall(vcpu))
> 		ret = (u32)ret;

No, because sneakily, is_64_bit_hypercall() != is_64_bit_mode(vcpu).  And because
we also need to avoid calling is_64_bit_mode().  If we use kvm_rax_write(), then
the unpacked code will be:

	WARN_ON_ONCE(vcpu->arch.guest_state_protected);

	if (is_long_mode(vcpu))
		kvm_x86_call(get_cs_db_l_bits)(vcpu, &cs_db, &cs_l);
	else
		cs_l = 0;

	if (cs_l)
		vcpu->arch.regs[VCPU_REGS_RAX] = ret;
	else	
		vcpu->arch.regs[VCPU_REGS_RAX] = (u32)ret;

whereas the (correct) behavior here is:

	if (vcpu->arch.guest_state_protected)
		cs_l = 1;
	else if (is_long_mode(vcpu))
		kvm_x86_call(get_cs_db_l_bits)(vcpu, &cs_db, &cs_l);
	else
		cs_l = 0;

	if (cs_l)
		vcpu->arch.regs[VCPU_REGS_RAX] = ret;
	else	
		vcpu->arch.regs[VCPU_REGS_RAX] = (u32)ret;

I.e. using the non-raw version will trigger the WARN_ON_ONCE(), and will incorrectly
truncate "ret" whenever cs_l is stale (which might be always?).

  reply	other threads:[~2026-05-18 20:51 UTC|newest]

Thread overview: 52+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-14 21:53 [PATCH v2 00/15] KVM: x86: Clean up kvm_<reg>_{read,write}() mess Sean Christopherson
2026-05-14 21:53 ` [PATCH v2 01/15] KVM: SVM: Truncate INVLPGA address in compatibility mode Sean Christopherson
2026-05-15  6:36   ` Binbin Wu
2026-05-14 21:53 ` [PATCH v2 02/15] KVM: x86/xen: Bug the VM if 32-bit KVM observes a 64-bit mode hypercall Sean Christopherson
2026-05-15  6:46   ` Binbin Wu
2026-05-14 21:53 ` [PATCH v2 03/15] KVM: x86/xen: Don't truncate RAX when handling hypercall from protected guest Sean Christopherson
2026-05-15  7:21   ` Binbin Wu
2026-05-15 12:55     ` Sean Christopherson
2026-05-18  2:19       ` Binbin Wu
2026-05-18  7:15         ` David Woodhouse
2026-05-18  9:43           ` Binbin Wu
2026-05-18  9:50             ` David Woodhouse
2026-05-18  9:55               ` Binbin Wu
2026-05-20  5:02                 ` Binbin Wu
2026-05-20  8:27                   ` David Woodhouse
2026-06-04 21:48                 ` David Woodhouse
2026-05-20  8:32   ` David Woodhouse
2026-05-14 21:53 ` [PATCH v2 04/15] KVM: VMX: Read 32-bit GPR values for ENCLS instructions outside of 64-bit mode Sean Christopherson
2026-05-15  7:26   ` Binbin Wu
2026-05-14 21:53 ` [PATCH v2 05/15] KVM: x86: Trace hypercall register *after* truncating values for 32-bit Sean Christopherson
2026-05-15  7:32   ` Binbin Wu
2026-05-14 21:53 ` [PATCH v2 06/15] KVM: x86: Rename kvm_cache_regs.h => regs.h Sean Christopherson
2026-05-14 22:28   ` Yosry Ahmed
2026-05-15  7:45   ` Binbin Wu
2026-05-14 21:53 ` [PATCH v2 07/15] KVM: x86: Move inlined CR and DR helpers from x86.h to regs.h Sean Christopherson
2026-05-14 22:30   ` Yosry Ahmed
2026-05-15  8:07   ` Binbin Wu
2026-05-14 21:53 ` [PATCH v2 08/15] KVM: x86: Add mode-aware versions of kvm_<reg>_{read,write}() helpers Sean Christopherson
2026-05-15  8:46   ` Binbin Wu
2026-05-18 11:31   ` Huang, Kai
2026-05-18 20:51     ` Sean Christopherson [this message]
2026-05-18 22:29       ` Huang, Kai
2026-05-18 23:44       ` Huang, Kai
2026-05-14 21:53 ` [PATCH v2 09/15] KVM: x86: Drop non-raw kvm_<reg>_write() helpers Sean Christopherson
2026-05-15  9:11   ` Binbin Wu
2026-05-14 21:53 ` [PATCH v2 10/15] KVM: nSVM: Use kvm_rax_read() now that it's mode-aware Sean Christopherson
2026-05-14 21:53 ` [PATCH v2 11/15] Revert "KVM: VMX: Read 32-bit GPR values for ENCLS instructions outside of 64-bit mode" Sean Christopherson
2026-05-15  9:26   ` Binbin Wu
2026-05-14 21:53 ` [PATCH v2 12/15] KVM: x86: Harden is_64_bit_hypercall() against bugs on 32-bit kernels Sean Christopherson
2026-05-15  9:31   ` Binbin Wu
2026-05-14 21:53 ` [PATCH v2 13/15] KVM: x86: Move update_cr8_intercept() to lapic.c Sean Christopherson
2026-05-14 21:53 ` [PATCH v2 14/15] KVM: x86: Move kvm_pv_async_pf_enabled() to x86.h (as an inline) Sean Christopherson
2026-05-14 21:53 ` [PATCH v2 15/15] KVM: x86: Move the bulk of register specific code from x86.c to regs.c Sean Christopherson
2026-05-19 12:16   ` Huang, Kai
2026-05-19 15:04     ` Sean Christopherson
2026-05-20  0:59       ` Huang, Kai
2026-05-20  1:25         ` Sean Christopherson
2026-05-20  2:29           ` Huang, Kai
2026-05-20 18:11             ` Sean Christopherson
2026-05-20 22:22               ` Huang, Kai
2026-05-21 18:47                 ` Sean Christopherson
2026-05-14 22:31 ` [PATCH v2 00/15] KVM: x86: Clean up kvm_<reg>_{read,write}() mess Yosry Ahmed

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=agt7vnVL5rJJUVzo@google.com \
    --to=seanjc@google.com \
    --cc=binbin.wu@linux.intel.com \
    --cc=dave.hansen@linux.intel.com \
    --cc=dwmw2@infradead.org \
    --cc=kai.huang@intel.com \
    --cc=kas@kernel.org \
    --cc=kvm@vger.kernel.org \
    --cc=linux-coco@lists.linux.dev \
    --cc=linux-kernel@vger.kernel.org \
    --cc=paul@xen.org \
    --cc=pbonzini@redhat.com \
    --cc=rick.p.edgecombe@intel.com \
    --cc=vkuznets@redhat.com \
    --cc=x86@kernel.org \
    --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.