On Tue, Aug 26, 2025, Peter Zijlstra wrote:
> On Tue, Aug 26, 2025 at 05:00:31PM +0530, Naman Jain wrote:
> >
> >
> > On 8/25/2025 3:12 PM, Peter Zijlstra wrote:
> > > On Mon, Aug 25, 2025 at 11:22:08AM +0530, Naman Jain wrote:
> > > > With commit 0e20f1f4c2cb ("x86/hyperv: Clean up hv_do_hypercall()"),
> > > > config checks were added to conditionally restrict export
> > > > of hv_hypercall_pg symbol at the same time when a usage of that symbol
> > > > was added in mshv_vtl_main.c driver. This results in missing symbol
> > > > warning when mshv_vtl_main is compiled. Change the logic to
> > > > export it unconditionally.
> > > >
> > > > Fixes: 96a1d2495c2f ("Drivers: hv: Introduce mshv_vtl driver")
> > > > Signed-off-by: Naman Jain <[email protected]>
> > >
> > > Oh gawd, that commit is terrible and adds yet another hypercall
> > > interface.
> > >
> > > I would argue the proper fix is moving the whole of mshv_vtl_return()
> > > into the kernel proper and doing it like hv_std_hypercall() on x86_64.
> >
> > Thanks for the review comments.
> >
> > This is doable, I can move the hypercall part of it to
> > arch/x86/hyperv/hv_init.c if I understand correctly.
> >
> > >
> > > Additionally, how is that function not utterly broken? What happens if
> > > an interrupt or NMI comes in after native_write_cr2() and before the
> > > actual hypercall does VMEXIT and trips a #PF?
> >
> > mshv_vtl driver is used for OpenHCL paravisor. The interrupts are
> > disabled, and NMIs aren't sent to the paravisor by the virt stack.
>
> I do not know what OpenHCL is. Nor is it clear from the code what NMIs
> can't happen.
FWIW, NMIs likely aren't a problem because the NMI handler saves/restores CR2
specifically to guard against #PFs in NMI context clobbering guest state. AMD
CPUs can block NMIs via GIF=0, but blocking NMIs on Intel for the VM-Entry =>
VM-Exit would require worse hacks than saving/restoring CR2 in the NMI handler.
:-(
> Anyway, same can be achieved with breakpoints / kprobes. You can get a trap
> after setting CR2 and scribble it.
Ya, KVM marks everything for vmx_vcpu_enter_exit() to/from VM-Enter/VM-Exit as
noinstr (no instrumentation) to prevent breakpoints/kprobes from clobbering CR2
and other state (and DR7 is zero for good measure).
