Re: [PATCH 08/11] Drivers: hv: mshv_vtl: Move register page config to arch-specific files

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

On 4/1/2026 10:28 PM, Michael Kelley wrote:
> From: Naman Jain <namjain@linux.microsoft.com> Sent: Monday, March 16, 2026 5:13 AM
>>
>> Move mshv_vtl_configure_reg_page() implementation from
>> drivers/hv/mshv_vtl_main.c to arch-specific files:
>> - arch/x86/hyperv/hv_vtl.c: full implementation with register page setup
>> - arch/arm64/hyperv/hv_vtl.c: stub implementation (unsupported)
>>
>> Move common type definitions to include/asm-generic/mshyperv.h:
>> - struct mshv_vtl_per_cpu
>> - union hv_synic_overlay_page_msr
>>
>> Move hv_call_get_vp_registers() and hv_call_set_vp_registers()
>> declarations to include/asm-generic/mshyperv.h since these functions
>> are used by multiple modules.
>>
>> While at it, remove the unnecessary stub implementations in #else
>> case for mshv_vtl_return* functions in arch/x86/include/asm/mshyperv.h.
> 
> Seems like this patch is doing multiple things. The reg page configuration
> changes are more substantial and should probably be in a patch by
> themselves. The other changes are more trivial and maybe are OK
> grouped into a single patch, but you could also consider breaking them
> out.

I will split this patch into 3 patches.

> 
>>
>> This is essential for adding support for ARM64 in MSHV_VTL.
>>
>> Signed-off-by: Naman Jain <namjain@linux.microsoft.com>
>> ---
>>   arch/arm64/hyperv/hv_vtl.c        |  8 +++++
>>   arch/arm64/include/asm/mshyperv.h |  3 ++
>>   arch/x86/hyperv/hv_vtl.c          | 32 ++++++++++++++++++++
>>   arch/x86/include/asm/mshyperv.h   |  7 ++---
>>   drivers/hv/mshv.h                 |  8 -----
>>   drivers/hv/mshv_vtl_main.c        | 49 +++----------------------------
>>   include/asm-generic/mshyperv.h    | 42 ++++++++++++++++++++++++++
>>   7 files changed, 92 insertions(+), 57 deletions(-)
>>
>> diff --git a/arch/arm64/hyperv/hv_vtl.c b/arch/arm64/hyperv/hv_vtl.c
>> index 66318672c242..d699138427c1 100644
>> --- a/arch/arm64/hyperv/hv_vtl.c
>> +++ b/arch/arm64/hyperv/hv_vtl.c
>> @@ -10,6 +10,7 @@
>>   #include <asm/boot.h>
>>   #include <asm/mshyperv.h>
>>   #include <asm/cpu_ops.h>
>> +#include <linux/export.h>
>>
>>   void mshv_vtl_return_call(struct mshv_vtl_cpu_context *vtl0)
>>   {
>> @@ -142,3 +143,10 @@ void mshv_vtl_return_call(struct mshv_vtl_cpu_context *vtl0)
>>   		"v24", "v25", "v26", "v27", "v28", "v29", "v30", "v31");
>>   }
>>   EXPORT_SYMBOL(mshv_vtl_return_call);
>> +
>> +bool hv_vtl_configure_reg_page(struct mshv_vtl_per_cpu *per_cpu)
>> +{
>> +	pr_debug("Register page not supported on ARM64\n");
>> +	return false;
>> +}
>> +EXPORT_SYMBOL_GPL(hv_vtl_configure_reg_page);
>> diff --git a/arch/arm64/include/asm/mshyperv.h
>> b/arch/arm64/include/asm/mshyperv.h
>> index de7f3a41a8ea..36803f0386cc 100644
>> --- a/arch/arm64/include/asm/mshyperv.h
>> +++ b/arch/arm64/include/asm/mshyperv.h
>> @@ -61,6 +61,8 @@ static inline u64 hv_get_non_nested_msr(unsigned int reg)
>>   				ARM_SMCCC_OWNER_VENDOR_HYP,	\
>>   				HV_SMCCC_FUNC_NUMBER)
>>
>> +struct mshv_vtl_per_cpu;
>> +
>>   struct mshv_vtl_cpu_context {
>>   /*
>>    * NOTE: x18 is managed by the hypervisor. It won't be reloaded from this array.
>> @@ -82,6 +84,7 @@ static inline int hv_vtl_get_set_reg(struct hv_register_assoc *regs,
>> bool set, u
>>   }
>>
>>   void mshv_vtl_return_call(struct mshv_vtl_cpu_context *vtl0);
>> +bool hv_vtl_configure_reg_page(struct mshv_vtl_per_cpu *per_cpu);
> 
> I think this declaration could be added in asm-generic/mshyperv.h so that it
> is shared by x86 and arm64. That also obviates the need for the forward
> ref to struct mshv_vtl_per_cpu that you've added here.

Acked.

> 
>>   #endif
>>
>>   #include <asm-generic/mshyperv.h>
>> diff --git a/arch/x86/hyperv/hv_vtl.c b/arch/x86/hyperv/hv_vtl.c
>> index 72a0bb4ae0c7..ede290985d41 100644
>> --- a/arch/x86/hyperv/hv_vtl.c
>> +++ b/arch/x86/hyperv/hv_vtl.c
>> @@ -20,6 +20,7 @@
>>   #include <uapi/asm/mtrr.h>
>>   #include <asm/debugreg.h>
>>   #include <linux/export.h>
>> +#include <linux/hyperv.h>
>>   #include <../kernel/smpboot.h>
>>   #include "../../kernel/fpu/legacy.h"
>>
>> @@ -259,6 +260,37 @@ int __init hv_vtl_early_init(void)
>>   	return 0;
>>   }
>>
>> +static const union hv_input_vtl input_vtl_zero;
>> +
>> +bool hv_vtl_configure_reg_page(struct mshv_vtl_per_cpu *per_cpu)
>> +{
>> +	struct hv_register_assoc reg_assoc = {};
>> +	union hv_synic_overlay_page_msr overlay = {};
>> +	struct page *reg_page;
>> +
>> +	reg_page = alloc_page(GFP_KERNEL | __GFP_ZERO | __GFP_RETRY_MAYFAIL);
>> +	if (!reg_page) {
>> +		WARN(1, "failed to allocate register page\n");
>> +		return false;
>> +	}
>> +
>> +	overlay.enabled = 1;
>> +	overlay.pfn = page_to_hvpfn(reg_page);
>> +	reg_assoc.name = HV_X64_REGISTER_REG_PAGE;
>> +	reg_assoc.value.reg64 = overlay.as_uint64;
>> +
>> +	if (hv_call_set_vp_registers(HV_VP_INDEX_SELF, HV_PARTITION_ID_SELF,
>> +				     1, input_vtl_zero, &reg_assoc)) {
>> +		WARN(1, "failed to setup register page\n");
>> +		__free_page(reg_page);
>> +		return false;
>> +	}
>> +
>> +	per_cpu->reg_page = reg_page;
>> +	return true;
> 
> As Sashiko AI noted, the memory allocated for the reg_page never gets freed.

These are present in existing code, I'll address them in a separate series.

> 
>> +}
>> +EXPORT_SYMBOL_GPL(hv_vtl_configure_reg_page);
>> +
>>   DEFINE_STATIC_CALL_NULL(__mshv_vtl_return_hypercall, void (*)(void));
>>
>>   void mshv_vtl_return_call_init(u64 vtl_return_offset)
>> diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
>> index d5355a5b7517..d592fea49cdb 100644
>> --- a/arch/x86/include/asm/mshyperv.h
>> +++ b/arch/x86/include/asm/mshyperv.h
>> @@ -271,6 +271,8 @@ static inline u64 hv_get_non_nested_msr(unsigned int reg) {
>> return 0; }
>>   static inline int hv_apicid_to_vp_index(u32 apic_id) { return -EINVAL; }
>>   #endif /* CONFIG_HYPERV */
>>
>> +struct mshv_vtl_per_cpu;
>> +
>>   struct mshv_vtl_cpu_context {
>>   	union {
>>   		struct {
>> @@ -305,13 +307,10 @@ void mshv_vtl_return_call_init(u64 vtl_return_offset);
>>   void mshv_vtl_return_hypercall(void);
>>   void __mshv_vtl_return_call(struct mshv_vtl_cpu_context *vtl0);
>>   int hv_vtl_get_set_reg(struct hv_register_assoc *regs, bool set, u64 shared);
>> +bool hv_vtl_configure_reg_page(struct mshv_vtl_per_cpu *per_cpu);
> 
> Same as for arm64. Add a shared declaration in asm-generic/mshyperv.h.

Ditto.

> 
>>   #else
>>   static inline void __init hv_vtl_init_platform(void) {}
>>   static inline int __init hv_vtl_early_init(void) { return 0; }
>> -static inline void mshv_vtl_return_call(struct mshv_vtl_cpu_context *vtl0) {}
>> -static inline void mshv_vtl_return_call_init(u64 vtl_return_offset) {}
>> -static inline void mshv_vtl_return_hypercall(void) {}
>> -static inline void __mshv_vtl_return_call(struct mshv_vtl_cpu_context *vtl0) {}
>>   #endif
>>
>>   #include <asm-generic/mshyperv.h>
>> diff --git a/drivers/hv/mshv.h b/drivers/hv/mshv.h
>> index d4813df92b9c..0fcb7f9ba6a9 100644
>> --- a/drivers/hv/mshv.h
>> +++ b/drivers/hv/mshv.h
>> @@ -14,14 +14,6 @@
>>   	memchr_inv(&((STRUCT).MEMBER), \
>>   		   0, sizeof_field(typeof(STRUCT), MEMBER))
>>
>> -int hv_call_get_vp_registers(u32 vp_index, u64 partition_id, u16 count,
>> -			     union hv_input_vtl input_vtl,
>> -			     struct hv_register_assoc *registers);
>> -
>> -int hv_call_set_vp_registers(u32 vp_index, u64 partition_id, u16 count,
>> -			     union hv_input_vtl input_vtl,
>> -			     struct hv_register_assoc *registers);
>> -
>>   int hv_call_get_partition_property(u64 partition_id, u64 property_code,
>>   				   u64 *property_value);
>>
>> diff --git a/drivers/hv/mshv_vtl_main.c b/drivers/hv/mshv_vtl_main.c
>> index 91517b45d526..c79d24317b8e 100644
>> --- a/drivers/hv/mshv_vtl_main.c
>> +++ b/drivers/hv/mshv_vtl_main.c
>> @@ -78,21 +78,6 @@ struct mshv_vtl {
>>   	u64 id;
>>   };
>>
>> -struct mshv_vtl_per_cpu {
>> -	struct mshv_vtl_run *run;
>> -	struct page *reg_page;
>> -};
>> -
>> -/* SYNIC_OVERLAY_PAGE_MSR - internal, identical to hv_synic_simp */
>> -union hv_synic_overlay_page_msr {
>> -	u64 as_uint64;
>> -	struct {
>> -		u64 enabled: 1;
>> -		u64 reserved: 11;
>> -		u64 pfn: 52;
>> -	} __packed;
>> -};
>> -
>>   static struct mutex mshv_vtl_poll_file_lock;
>>   static union hv_register_vsm_page_offsets mshv_vsm_page_offsets;
>>   static union hv_register_vsm_capabilities mshv_vsm_capabilities;
>> @@ -201,34 +186,6 @@ static struct page *mshv_vtl_cpu_reg_page(int cpu)
>>   	return *per_cpu_ptr(&mshv_vtl_per_cpu.reg_page, cpu);
>>   }
>>
>> -static void mshv_vtl_configure_reg_page(struct mshv_vtl_per_cpu *per_cpu)
>> -{
>> -	struct hv_register_assoc reg_assoc = {};
>> -	union hv_synic_overlay_page_msr overlay = {};
>> -	struct page *reg_page;
>> -
>> -	reg_page = alloc_page(GFP_KERNEL | __GFP_ZERO | __GFP_RETRY_MAYFAIL);
>> -	if (!reg_page) {
>> -		WARN(1, "failed to allocate register page\n");
>> -		return;
>> -	}
>> -
>> -	overlay.enabled = 1;
>> -	overlay.pfn = page_to_hvpfn(reg_page);
>> -	reg_assoc.name = HV_X64_REGISTER_REG_PAGE;
>> -	reg_assoc.value.reg64 = overlay.as_uint64;
>> -
>> -	if (hv_call_set_vp_registers(HV_VP_INDEX_SELF, HV_PARTITION_ID_SELF,
>> -				     1, input_vtl_zero, &reg_assoc)) {
>> -		WARN(1, "failed to setup register page\n");
>> -		__free_page(reg_page);
>> -		return;
>> -	}
>> -
>> -	per_cpu->reg_page = reg_page;
>> -	mshv_has_reg_page = true;
>> -}
>> -
>>   static void mshv_vtl_synic_enable_regs(unsigned int cpu)
>>   {
>>   	union hv_synic_sint sint;
>> @@ -329,8 +286,10 @@ static int mshv_vtl_alloc_context(unsigned int cpu)
>>   	if (!per_cpu->run)
>>   		return -ENOMEM;
>>
>> -	if (mshv_vsm_capabilities.intercept_page_available)
>> -		mshv_vtl_configure_reg_page(per_cpu);
>> +	if (mshv_vsm_capabilities.intercept_page_available) {
>> +		if (hv_vtl_configure_reg_page(per_cpu))
>> +			mshv_has_reg_page = true;
> 
> As Sashiko AI noted, it doesn't work to use the global mshv_has_reg_page
> to indicate the success of configuring the reg page, which is a per-cpu
> operation. But this bug existed before this patch set, so maybe it should
> be fixed as a preliminary patch.

Acked. Will address them in a separate series.

> 
>> +	}
>>
>>   	mshv_vtl_synic_enable_regs(cpu);
>>
>> diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h
>> index b147a12085e4..b53fcc071596 100644
>> --- a/include/asm-generic/mshyperv.h
>> +++ b/include/asm-generic/mshyperv.h
>> @@ -383,8 +383,50 @@ static inline int hv_deposit_memory(u64 partition_id, u64 status)
>>   	return hv_deposit_memory_node(NUMA_NO_NODE, partition_id, status);
>>   }
>>
>> +#if IS_ENABLED(CONFIG_MSHV_ROOT) || IS_ENABLED(CONFIG_MSHV_VTL)
>> +int hv_call_get_vp_registers(u32 vp_index, u64 partition_id, u16 count,
>> +			     union hv_input_vtl input_vtl,
>> +			     struct hv_register_assoc *registers);
>> +
>> +int hv_call_set_vp_registers(u32 vp_index, u64 partition_id, u16 count,
>> +			     union hv_input_vtl input_vtl,
>> +			     struct hv_register_assoc *registers);
>> +#else
>> +static inline int hv_call_get_vp_registers(u32 vp_index, u64 partition_id,
>> +					   u16 count,
>> +					   union hv_input_vtl input_vtl,
>> +					   struct hv_register_assoc *registers)
>> +{
>> +	return -EOPNOTSUPP;
>> +}
>> +
>> +static inline int hv_call_set_vp_registers(u32 vp_index, u64 partition_id,
>> +					   u16 count,
>> +					   union hv_input_vtl input_vtl,
>> +					   struct hv_register_assoc *registers)
>> +{
>> +	return -EOPNOTSUPP;
>> +}
>> +#endif /* CONFIG_MSHV_ROOT || CONFIG_MSHV_VTL */
>> +
>>   #define HV_VP_ASSIST_PAGE_ADDRESS_SHIFT	12
>> +
>>   #if IS_ENABLED(CONFIG_HYPERV_VTL_MODE)
>> +struct mshv_vtl_per_cpu {
>> +	struct mshv_vtl_run *run;
>> +	struct page *reg_page;
>> +};
>> +
>> +/* SYNIC_OVERLAY_PAGE_MSR - internal, identical to hv_synic_simp */
>> +union hv_synic_overlay_page_msr {
>> +	u64 as_uint64;
>> +	struct {
>> +		u64 enabled: 1;
>> +		u64 reserved: 11;
>> +		u64 pfn: 52;
>> +	} __packed;
>> +};
>> +
>>   u8 __init get_vtl(void);
>>   #else
>>   static inline u8 get_vtl(void) { return 0; }
>> --
>> 2.43.0
>>
> 
> Sashiko AI noted another existing bug in mshv_vtl_init(), which is that
> the error path does kfree(mem_dev) when it should do
> put_device(mem_dev).  See the comment in the header of
> device_initialize().


To avoid this series bloating up, I am thinking of taking up these fixes 
in a separate series.

Regards,
Naman

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv