Re: [PATCH v11 2/2] Drivers: hv: Introduce mshv_vtl driver

[Naman Jain <namjain@linux.microsoft.com>]

On 11/10/2025 8:08 PM, Peter Zijlstra wrote:
> On Mon, Nov 10, 2025 at 05:08:35AM +0000, Naman Jain wrote:
>> Provide an interface for Virtual Machine Monitor like OpenVMM and its
>> use as OpenHCL paravisor to control VTL0 (Virtual trust Level).
>> Expose devices and support IOCTLs for features like VTL creation,
>> VTL0 memory management, context switch, making hypercalls,
>> mapping VTL0 address space to VTL2 userspace, getting new VMBus
>> messages and channel events in VTL2 etc.
> 
>> diff --git a/arch/x86/hyperv/hv_vtl.c b/arch/x86/hyperv/hv_vtl.c
>> index 042e8712d8de..dba27e1bcc10 100644
>> --- a/arch/x86/hyperv/hv_vtl.c
>> +++ b/arch/x86/hyperv/hv_vtl.c
>> @@ -249,3 +253,42 @@ int __init hv_vtl_early_init(void)
>>   
>>   	return 0;
>>   }
>> +
>> +DEFINE_STATIC_CALL_NULL(__mshv_vtl_return_hypercall, void (*)(void));
>> +
>> +noinstr void mshv_vtl_return_hypercall(void)
>> +{
>> +	asm volatile ("call " STATIC_CALL_TRAMP_STR(__mshv_vtl_return_hypercall));
>> +}
>> +
>> +/*
>> + * ASM_CALL_CONSTRAINT is intentionally not used in above asm block before making a call to
>> + * __mshv_vtl_return_hypercall, to avoid rbp clobbering before actual VTL return happens.
>> + * This however leads to objtool complain about "call without frame pointer save/setup".
>> + * To ignore that warning, and inform objtool about this non-standard function,
>> + * STACK_FRAME_NON_STANDARD_FP is used.
>> + */
>> +STACK_FRAME_NON_STANDARD_FP(mshv_vtl_return_hypercall);
> 
>> --- /dev/null
>> +++ b/arch/x86/hyperv/mshv_vtl_asm.S
>> @@ -0,0 +1,98 @@
>> +/* SPDX-License-Identifier: GPL-2.0
>> + *
>> + * Assembly level code for mshv_vtl VTL transition
>> + *
>> + * Copyright (c) 2025, Microsoft Corporation.
>> + *
>> + * Author:
>> + *   Naman Jain <namjain@microsoft.com>
>> + */
>> +
>> +#include <linux/linkage.h>
>> +#include <asm/asm.h>
>> +#include <asm/asm-offsets.h>
>> +#include <asm/frame.h>
>> +#include "mshv-asm-offsets.h"
>> +
>> +	.text
>> +	.section .noinstr.text, "ax"
>> +/*
>> + * void __mshv_vtl_return_call(struct mshv_vtl_cpu_context *vtl0)
> 
> Can we please get a few words on the magical context here? Like no NMIs
> and #DB traps and the like. Because if any of them were possible this
> code would be horribly broken.
Sure, I can add below comment:

This function is marked as 'noinstr' to prevent against instrumentation 
and debugging facilities. NMIs aren't a problem because the NMI handler 
saves/restores CR2 specifically to guard against #PFs in NMI context 
clobbering the guest state.
> 
>> + */
>> +SYM_FUNC_START(__mshv_vtl_return_call)
>> +	/* Push callee save registers */
>> +	pushq %rbp
>> +	mov %rsp, %rbp
>> +	pushq %r12
>> +	pushq %r13
>> +	pushq %r14
>> +	pushq %r15
>> +	pushq %rbx
>> +
>> +	/* register switch to VTL0 clobbers all registers except rax/rcx */
>> +	mov %_ASM_ARG1, %rax
>> +
>> +	/* grab rbx/rbp/rsi/rdi/r8-r15 */
>> +	mov MSHV_VTL_CPU_CONTEXT_rbx(%rax), %rbx
>> +	mov MSHV_VTL_CPU_CONTEXT_rbp(%rax), %rbp
>> +	mov MSHV_VTL_CPU_CONTEXT_rsi(%rax), %rsi
>> +	mov MSHV_VTL_CPU_CONTEXT_rdi(%rax), %rdi
>> +	mov MSHV_VTL_CPU_CONTEXT_r8(%rax), %r8
>> +	mov MSHV_VTL_CPU_CONTEXT_r9(%rax), %r9
>> +	mov MSHV_VTL_CPU_CONTEXT_r10(%rax), %r10
>> +	mov MSHV_VTL_CPU_CONTEXT_r11(%rax), %r11
>> +	mov MSHV_VTL_CPU_CONTEXT_r12(%rax), %r12
>> +	mov MSHV_VTL_CPU_CONTEXT_r13(%rax), %r13
>> +	mov MSHV_VTL_CPU_CONTEXT_r14(%rax), %r14
>> +	mov MSHV_VTL_CPU_CONTEXT_r15(%rax), %r15
>> +
>> +	mov MSHV_VTL_CPU_CONTEXT_cr2(%rax), %rdx
>> +	mov %rdx, %cr2
>> +	mov MSHV_VTL_CPU_CONTEXT_rdx(%rax), %rdx
>> +
>> +	/* stash host registers on stack */
>> +	pushq %rax
>> +	pushq %rcx
>> +
>> +	xor %ecx, %ecx
>> +
>> +	/* make a hypercall to switch VTL */
>> +	call mshv_vtl_return_hypercall
> 
> Yuck!
> 
> This seems to build for me.

This would have been the cleanest approach. We discussed this before and 
unfortunately it didn't work. Please find the link to this discussion:

https://lore.kernel.org/all/9f8007a3-f810-4b60-8942-e721cd6a32c4@linux.microsoft.com/

To summarize above discussion, I see below compilation error with this 
from objtool. You may have CONFIG_X86_KERNEL_IBT enabled in your 
workspace, which would have masked this.


   AS      arch/x86/hyperv/mshv_vtl_asm.o
arch/x86/hyperv/mshv_vtl_asm.o: error: objtool: static_call: can't find 
static_call_key symbol: __SCK____mshv_vtl_return_hypercall
make[4]: *** [scripts/Makefile.build:430: 
arch/x86/hyperv/mshv_vtl_asm.o] Error 255
make[4]: *** Deleting file 'arch/x86/hyperv/mshv_vtl_asm.o'
make[3]: *** [scripts/Makefile.build:556: arch/x86/hyperv] Error 2


Because of this compilation error, I had to implement this wrapper 
function (mshv_vtl_return_hypercall) which makes this static call in C.

> 
> ---
> --- a/arch/x86/hyperv/hv_vtl.c
> +++ b/arch/x86/hyperv/hv_vtl.c
> @@ -256,20 +256,6 @@ int __init hv_vtl_early_init(void)
>   
>   DEFINE_STATIC_CALL_NULL(__mshv_vtl_return_hypercall, void (*)(void));
>   
> -noinstr void mshv_vtl_return_hypercall(void)
> -{
> -	asm volatile ("call " STATIC_CALL_TRAMP_STR(__mshv_vtl_return_hypercall));
> -}
> -
> -/*
> - * ASM_CALL_CONSTRAINT is intentionally not used in above asm block before making a call to
> - * __mshv_vtl_return_hypercall, to avoid rbp clobbering before actual VTL return happens.
> - * This however leads to objtool complain about "call without frame pointer save/setup".
> - * To ignore that warning, and inform objtool about this non-standard function,
> - * STACK_FRAME_NON_STANDARD_FP is used.
> - */
> -STACK_FRAME_NON_STANDARD_FP(mshv_vtl_return_hypercall)
> -
>   void mshv_vtl_return_call_init(u64 vtl_return_offset)
>   {
>   	static_call_update(__mshv_vtl_return_hypercall,
> --- a/arch/x86/hyperv/mshv_vtl_asm.S
> +++ b/arch/x86/hyperv/mshv_vtl_asm.S
> @@ -9,6 +9,7 @@
>    */

<snip>

Regards,
Naman