From: "Roger Pau Monné" <roger.pau@citrix.com>
To: Stefano Stabellini <sstabellini@kernel.org>
Cc: Stefano Stabellini <stefano.stabellini@amd.com>,
xen-devel@lists.xenproject.org, jbeulich@suse.com,
andrew.cooper3@citrix.com, jason.andryuk@amd.com,
alejandro.garciavallejo@amd.com
Subject: Re: [PATCH v2] x86/hvm: Add Kconfig option to disable nested virtualization
Date: Mon, 16 Feb 2026 10:08:39 +0100 [thread overview]
Message-ID: <aZLel_W_1B6684zC@Mac.lan> (raw)
In-Reply-To: <alpine.DEB.2.22.394.2602131350040.6031@ubuntu-linux-20-04-desktop>
On Fri, Feb 13, 2026 at 01:56:34PM -0800, Stefano Stabellini wrote:
> I address all other comments
>
>
> On Mon, 9 Feb 2026, Roger Pau Monné wrote:
> > > +static inline int nvmx_msr_read_intercept(unsigned int msr, u64
> > > *msr_content)
> > > +{
> > > + ASSERT_UNREACHABLE();
> > > + return 0;
> > > +}
> > >
> > I think this function is reachable even when nested virt is not
> > enabled:
> >
> > vmx_msr_read_intercept() -> case MSR_IA32_VMX_BASIC...MSR_IA32_VMX_VMFUNC -> nvmx_msr_read_intercept()
> >
> > I'm also confused about why the function returns 0 instead of an error
> > when !nestedhvm_enabled(). We should probably make it return -ENODEV
> > when nested virt is not available or enabled.
Oh, I see. The return type of that function is weird. It should be
adjusted to bool (not that you need to do it here).
> I agree on the way of thinking but if we return zero it will goto gp_fault.
> So I'll just remove ASSERT_UNREACHABLE.
Ack.
>
> > > +static inline int nvmx_handle_vmx_insn(struct cpu_user_regs *regs,
> > > + unsigned int exit_reason)
> > > +{
> > > + ASSERT_UNREACHABLE();
> > > + return 0;
> > > +}
> >
> > Same here, I think this is likely reachable from vmx_vmexit_handler(),
> > and shouldn't assert?
> >
> > It should also do something like:
> >
> > hvm_inject_hw_exception(X86_EXC_UD, X86_EVENT_NO_EC);
> > return X86EMUL_EXCEPTION;
> >
> > So it mimics what the function itself does when !nestedhvm_enabled().
>
> hvm_inject_hw_exception cannot be easily called here because it is not
> available at this point in the header. But actually this function should
> be unreachable because when !CONFIG_NESTED_VIRT, CR4.VMXE is not
> a valid guest CR4 bit, so nested VMX instructions should cause #UD?
I'm not sure about nvmx_handle_vmx_insn() being unreachable when
CR4.VMXE is not set, the Intel SDM states:
"26.1.2 Instructions That Cause VM Exits Unconditionally
The following instructions cause VM exits when they are executed in
VMX non-root operation: CPUID, GETSEC,1 INVD, and XSETBV. This is also
true of instructions introduced with VMX, which include: INVEPT,
INVVPID, VMCALL,2 VMCLEAR, VMLAUNCH, VMPTRLD, VMPTRST, VMRESUME,
VMXOFF, and VMXON."
My reading is that regardless of the value of CR4.VMXE the execution
of VMX instructions will cause a VMEXIT, and it's the hypervisor task
to figure out what to do.
>
> I changed it to:
>
> ASSERT_UNREACHABLE();
> return X86EMUL_EXCEPTION;
If the above is correct, the ASSERT_UNREACHABLE() is reachable.
Returning X86EMUL_EXCEPTION without actually having injected the
exception will result in a loop I think, as vmx_vmexit_handler() will
resume guest execution without having advanced the instruction
pointer, and without having injected an exception, and hence another
VMEXIT will trigger.
IMO the best way to solve this is to make vmx_vmexit_handler() inject
#UD when nvmx_handle_vmx_insn() returns X86EMUL_EXCEPTION. You will
need to then also adjust the non-stub version of
nvmx_handle_vmx_insn(), so it doesn't inject #UD itself.
Thanks, Roger.
next prev parent reply other threads:[~2026-02-16 9:09 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-02-06 21:05 [PATCH v2] x86/hvm: Add Kconfig option to disable nested virtualization Stefano Stabellini
2026-02-08 2:31 ` Demi Marie Obenour
2026-02-09 11:08 ` Roger Pau Monné
2026-02-13 21:56 ` Stefano Stabellini
2026-02-16 9:08 ` Roger Pau Monné [this message]
2026-02-09 14:55 ` Alejandro Vallejo
2026-02-09 15:01 ` Jan Beulich
2026-02-09 15:09 ` Jan Beulich
2026-02-13 21:46 ` Stefano Stabellini
2026-02-16 8:57 ` Jan Beulich
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=aZLel_W_1B6684zC@Mac.lan \
--to=roger.pau@citrix.com \
--cc=alejandro.garciavallejo@amd.com \
--cc=andrew.cooper3@citrix.com \
--cc=jason.andryuk@amd.com \
--cc=jbeulich@suse.com \
--cc=sstabellini@kernel.org \
--cc=stefano.stabellini@amd.com \
--cc=xen-devel@lists.xenproject.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.