RE: [PATCH 02/11] Drivers: hv: Move hv_vp_assist_page to common files

[Michael Kelley]

From: Naman Jain <namjain@xxxxxxxxxxxxxxxxxxx> Sent: Monday, March 16, 2026 5:13 AM
> 
> Move the logic to initialize and export hv_vp_assist_page from x86
> architecture code to Hyper-V common code to allow it to be used for
> upcoming arm64 support in MSHV_VTL driver.
> Note: This change also improves error handling - if VP assist page
> allocation fails, hyperv_init() now returns early instead of
> continuing with partial initialization.
> 
> Signed-off-by: Roman Kisel <romank@xxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Naman Jain <namjain@xxxxxxxxxxxxxxxxxxx>
> ---
>  arch/x86/hyperv/hv_init.c      | 88 +---------------------------------
>  drivers/hv/hv_common.c         | 88 ++++++++++++++++++++++++++++++++++
>  include/asm-generic/mshyperv.h |  4 ++
>  include/hyperv/hvgdk_mini.h    |  2 +
>  4 files changed, 95 insertions(+), 87 deletions(-)
> 
> diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
> index 323adc93f2dc..75a98b5e451b 100644
> --- a/arch/x86/hyperv/hv_init.c
> +++ b/arch/x86/hyperv/hv_init.c
> @@ -81,9 +81,6 @@ union hv_ghcb * __percpu *hv_ghcb_pg;
>  /* Storage to save the hypercall page temporarily for hibernation */
>  static void *hv_hypercall_pg_saved;
> 
> -struct hv_vp_assist_page **hv_vp_assist_page;
> -EXPORT_SYMBOL_GPL(hv_vp_assist_page);
> -
>  static int hyperv_init_ghcb(void)
>  {
>  	u64 ghcb_gpa;
> @@ -117,59 +114,12 @@ static int hyperv_init_ghcb(void)
> 
>  static int hv_cpu_init(unsigned int cpu)
>  {
> -	union hv_vp_assist_msr_contents msr = { 0 };
> -	struct hv_vp_assist_page **hvp;
>  	int ret;
> 
>  	ret = hv_common_cpu_init(cpu);
>  	if (ret)
>  		return ret;
> 
> -	if (!hv_vp_assist_page)
> -		return 0;
> -
> -	hvp = &hv_vp_assist_page[cpu];
> -	if (hv_root_partition()) {
> -		/*
> -		 * For root partition we get the hypervisor provided VP assist
> -		 * page, instead of allocating a new page.
> -		 */
> -		rdmsrq(HV_X64_MSR_VP_ASSIST_PAGE, msr.as_uint64);
> -		*hvp = memremap(msr.pfn << HV_X64_MSR_VP_ASSIST_PAGE_ADDRESS_SHIFT,
> -				PAGE_SIZE, MEMREMAP_WB);
> -	} else {
> -		/*
> -		 * The VP assist page is an "overlay" page (see Hyper-V TLFS's
> -		 * Section 5.2.1 "GPA Overlay Pages"). Here it must be zeroed
> -		 * out to make sure we always write the EOI MSR in
> -		 * hv_apic_eoi_write() *after* the EOI optimization is disabled
> -		 * in hv_cpu_die(), otherwise a CPU may not be stopped in the
> -		 * case of CPU offlining and the VM will hang.
> -		 */
> -		if (!*hvp) {
> -			*hvp = __vmalloc(PAGE_SIZE, GFP_KERNEL | __GFP_ZERO);
> -
> -			/*
> -			 * Hyper-V should never specify a VM that is a Confidential
> -			 * VM and also running in the root partition. Root partition
> -			 * is blocked to run in Confidential VM. So only decrypt assist
> -			 * page in non-root partition here.
> -			 */
> -			if (*hvp && !ms_hyperv.paravisor_present && hv_isolation_type_snp()) {
> -				WARN_ON_ONCE(set_memory_decrypted((unsigned long)(*hvp), 1));
> -				memset(*hvp, 0, PAGE_SIZE);
> -			}
> -		}
> -
> -		if (*hvp)
> -			msr.pfn = vmalloc_to_pfn(*hvp);
> -
> -	}
> -	if (!WARN_ON(!(*hvp))) {
> -		msr.enable = 1;
> -		wrmsrq(HV_X64_MSR_VP_ASSIST_PAGE, msr.as_uint64);
> -	}
> -
>  	/* Allow Hyper-V stimer vector to be injected from Hypervisor. */
>  	if (ms_hyperv.misc_features & HV_STIMER_DIRECT_MODE_AVAILABLE)
>  		apic_update_vector(cpu, HYPERV_STIMER0_VECTOR, true);
> @@ -286,23 +236,6 @@ static int hv_cpu_die(unsigned int cpu)
> 
>  	hv_common_cpu_die(cpu);
> 
> -	if (hv_vp_assist_page && hv_vp_assist_page[cpu]) {
> -		union hv_vp_assist_msr_contents msr = { 0 };
> -		if (hv_root_partition()) {
> -			/*
> -			 * For root partition the VP assist page is mapped to
> -			 * hypervisor provided page, and thus we unmap the
> -			 * page here and nullify it, so that in future we have
> -			 * correct page address mapped in hv_cpu_init.
> -			 */
> -			memunmap(hv_vp_assist_page[cpu]);
> -			hv_vp_assist_page[cpu] = NULL;
> -			rdmsrq(HV_X64_MSR_VP_ASSIST_PAGE, msr.as_uint64);
> -			msr.enable = 0;
> -		}
> -		wrmsrq(HV_X64_MSR_VP_ASSIST_PAGE, msr.as_uint64);
> -	}
> -
>  	if (hv_reenlightenment_cb == NULL)
>  		return 0;
> 
> @@ -460,21 +393,6 @@ void __init hyperv_init(void)
>  	if (hv_common_init())
>  		return;
> 
> -	/*
> -	 * The VP assist page is useless to a TDX guest: the only use we
> -	 * would have for it is lazy EOI, which can not be used with TDX.
> -	 */
> -	if (hv_isolation_type_tdx())
> -		hv_vp_assist_page = NULL;
> -	else
> -		hv_vp_assist_page = kzalloc_objs(*hv_vp_assist_page, nr_cpu_ids);
> -	if (!hv_vp_assist_page) {
> -		ms_hyperv.hints &= ~HV_X64_ENLIGHTENED_VMCS_RECOMMENDED;
> -
> -		if (!hv_isolation_type_tdx())
> -			goto common_free;
> -	}
> -
>  	if (ms_hyperv.paravisor_present && hv_isolation_type_snp()) {
>  		/* Negotiate GHCB Version. */
>  		if (!hv_ghcb_negotiate_protocol())
> @@ -483,7 +401,7 @@ void __init hyperv_init(void)
> 
>  		hv_ghcb_pg = alloc_percpu(union hv_ghcb *);
>  		if (!hv_ghcb_pg)
> -			goto free_vp_assist_page;
> +			goto free_ghcb_page;
>  	}
> 
>  	cpuhp = cpuhp_setup_state(CPUHP_AP_HYPERV_ONLINE, "x86/hyperv_init:online",
> @@ -613,10 +531,6 @@ void __init hyperv_init(void)
>  	cpuhp_remove_state(CPUHP_AP_HYPERV_ONLINE);
>  free_ghcb_page:
>  	free_percpu(hv_ghcb_pg);
> -free_vp_assist_page:
> -	kfree(hv_vp_assist_page);
> -	hv_vp_assist_page = NULL;
> -common_free:
>  	hv_common_free();
>  }
> 
> diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c
> index 6b67ac616789..d1ebc0ebd08f 100644
> --- a/drivers/hv/hv_common.c
> +++ b/drivers/hv/hv_common.c
> @@ -28,7 +28,9 @@
>  #include <linux/slab.h>
>  #include <linux/dma-map-ops.h>
>  #include <linux/set_memory.h>
> +#include <linux/vmalloc.h>
>  #include <hyperv/hvhdk.h>
> +#include <hyperv/hvgdk.h>
>  #include <asm/mshyperv.h>

Need to add

#include <linux/io.h>

because of the memremap() and related calls that have been added.
io.h is probably being #include'd indirectly, but it is better to #include
it directly.

> 
>  u64 hv_current_partition_id = HV_PARTITION_ID_SELF;
> @@ -78,6 +80,8 @@ static struct ctl_table_header *hv_ctl_table_hdr;
>  u8 * __percpu *hv_synic_eventring_tail;
>  EXPORT_SYMBOL_GPL(hv_synic_eventring_tail);
> 
> +struct hv_vp_assist_page **hv_vp_assist_page;
> +EXPORT_SYMBOL_GPL(hv_vp_assist_page);
>  /*
>   * Hyper-V specific initialization and shutdown code that is
>   * common across all architectures.  Called from architecture
> @@ -92,6 +96,9 @@ void __init hv_common_free(void)
>  	if (ms_hyperv.misc_features & HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE)
>  		hv_kmsg_dump_unregister();
> 
> +	kfree(hv_vp_assist_page);
> +	hv_vp_assist_page = NULL;
> +
>  	kfree(hv_vp_index);
>  	hv_vp_index = NULL;
> 
> @@ -394,6 +401,23 @@ int __init hv_common_init(void)
>  	for (i = 0; i < nr_cpu_ids; i++)
>  		hv_vp_index[i] = VP_INVAL;
> 
> +	/*
> +	 * The VP assist page is useless to a TDX guest: the only use we
> +	 * would have for it is lazy EOI, which can not be used with TDX.
> +	 */
> +	if (hv_isolation_type_tdx()) {
> +		hv_vp_assist_page = NULL;
> +	} else {
> +		hv_vp_assist_page = kzalloc_objs(*hv_vp_assist_page, nr_cpu_ids);
> +		if (!hv_vp_assist_page) {
> +#ifdef CONFIG_X86_64
> +			ms_hyperv.hints &= ~HV_X64_ENLIGHTENED_VMCS_RECOMMENDED;
> +#endif
> +			hv_common_free();
> +			return -ENOMEM;

Given that "failure to allocate memory" now returns an error that is
essentially fatal to hyperv_init(), is it still necessary to clear the flag in
ms_hyperv.hints?  I'd love to see that #ifdef go away. It's the only
#ifdef in hv_common.c, and I had worked hard in the past to avoid
such #ifdef's. :-)

> +		}
> +	}
> +
>  	return 0;
>  }
> 
> @@ -471,6 +495,8 @@ void __init ms_hyperv_late_init(void)
> 
>  int hv_common_cpu_init(unsigned int cpu)
>  {
> +	union hv_vp_assist_msr_contents msr = { 0 };
> +	struct hv_vp_assist_page **hvp;
>  	void **inputarg, **outputarg;
>  	u8 **synic_eventring_tail;
>  	u64 msr_vp_index;
> @@ -542,6 +568,50 @@ int hv_common_cpu_init(unsigned int cpu)
>  			ret = -ENOMEM;

The Sashiko AI comment here about a bug when ret is set to -ENOMEM
seems valid to me.

>  	}
> 
> +	if (!hv_vp_assist_page)
> +		return ret;
> +
> +	hvp = &hv_vp_assist_page[cpu];
> +	if (hv_root_partition()) {
> +		/*
> +		 * For root partition we get the hypervisor provided VP assist
> +		 * page, instead of allocating a new page.
> +		 */
> +		msr.as_uint64 = hv_get_msr(HV_SYN_REG_VP_ASSIST_PAGE);
> +		*hvp = memremap(msr.pfn << HV_VP_ASSIST_PAGE_ADDRESS_SHIFT,
> +				PAGE_SIZE, MEMREMAP_WB);

The Sashiko AI comment about potentially memremap'ing 64K instead of 4K can
be ignored. We know that the root partition can only run with a 4K page size,
and that is enforced in drivers/hv/Kconfig.

HV_VP_ASSIST_PAGE_ADDRESS_SHIFT is defined in asm-generic/mshyperv.h.
But there is also HV_X64_MSR_VP_ASSIST_PAGE_ADDRESS_SHIFT in hvgdk_mini.h.
Is there a clean way to eliminate the duplication?

> +	} else {
> +		/*
> +		 * The VP assist page is an "overlay" page (see Hyper-V TLFS's
> +		 * Section 5.2.1 "GPA Overlay Pages"). Here it must be zeroed
> +		 * out to make sure we always write the EOI MSR in
> +		 * hv_apic_eoi_write() *after* the EOI optimization is disabled
> +		 * in hv_cpu_die(), otherwise a CPU may not be stopped in the
> +		 * case of CPU offlining and the VM will hang.
> +		 */

Somewhere in the comment above, I'd suggest adding a short "on x86/x64"
qualifier, as the comment doesn't apply on arm64 since it doesn't support
the AutoEOI optimization.  Maybe "Here it must be zeroed out to make sure
that on x86/x64 we always write the EOI MSR in ....".   

> +		if (!*hvp) {
> +			*hvp = __vmalloc(PAGE_SIZE, GFP_KERNEL | __GFP_ZERO);

The Sashiko AI comment about using "flags" instead of GFP_KERNEL seems valid.

> +
> +			/*
> +			 * Hyper-V should never specify a VM that is a Confidential
> +			 * VM and also running in the root partition. Root partition
> +			 * is blocked to run in Confidential VM. So only decrypt assist
> +			 * page in non-root partition here.
> +			 */
> +			if (*hvp && !ms_hyperv.paravisor_present && hv_isolation_type_snp()) {
> +				WARN_ON_ONCE(set_memory_decrypted((unsigned long)(*hvp), 1));
> +				memset(*hvp, 0, PAGE_SIZE);
> +			}
> +		}
> +
> +		if (*hvp)
> +			msr.pfn = vmalloc_to_pfn(*hvp);

The Sashiko AI comment about page size here seems valid. But what are the rules
about arm64 page sizes that are supported for VTL2, and how does they relate
to VTL0 allowing 4K, 16K, and 64K page size? What combinations are allowed?
For example, can a VTL2 built with 4K page size run with a VTL0 built with
64K page size? It would be nice to have the rules recorded somewhere in a
code comment, but I'm not sure of the best place.

But regardless of the rules, I'd suggest future-proofing by using
"page_to_hvpfn(vmalloc_to_page(*hvp))" so that the PFN generated is always
in terms of 4K page size as the Hyper-V host expects.

> +	}
> +	if (!WARN_ON(!(*hvp))) {
> +		msr.enable = 1;
> +		hv_set_msr(HV_SYN_REG_VP_ASSIST_PAGE, msr.as_uint64);
> +	}
> +
>  	return ret;
>  }
> 
> @@ -566,6 +636,24 @@ int hv_common_cpu_die(unsigned int cpu)
>  		*synic_eventring_tail = NULL;
>  	}
> 
> +	if (hv_vp_assist_page && hv_vp_assist_page[cpu]) {
> +		union hv_vp_assist_msr_contents msr = { 0 };
> +
> +		if (hv_root_partition()) {
> +			/*
> +			 * For root partition the VP assist page is mapped to
> +			 * hypervisor provided page, and thus we unmap the
> +			 * page here and nullify it, so that in future we have
> +			 * correct page address mapped in hv_cpu_init.
> +			 */
> +			memunmap(hv_vp_assist_page[cpu]);
> +			hv_vp_assist_page[cpu] = NULL;
> +			msr.as_uint64 = hv_get_msr(HV_SYN_REG_VP_ASSIST_PAGE);
> +			msr.enable = 0;
> +		}
> +		hv_set_msr(HV_SYN_REG_VP_ASSIST_PAGE, msr.as_uint64);
> +	}
> +
>  	return 0;
>  }
> 
> diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h
> index d37b68238c97..108f135d4fd9 100644
> --- a/include/asm-generic/mshyperv.h
> +++ b/include/asm-generic/mshyperv.h
> @@ -25,6 +25,7 @@
>  #include <linux/nmi.h>
>  #include <asm/ptrace.h>
>  #include <hyperv/hvhdk.h>
> +#include <hyperv/hvgdk.h>
> 
>  #define VTPM_BASE_ADDRESS 0xfed40000
> 
> @@ -299,6 +300,8 @@ do { \
>  #define hv_status_debug(status, fmt, ...) \
>  	hv_status_printk(debug, status, fmt, ##__VA_ARGS__)
> 
> +extern struct hv_vp_assist_page **hv_vp_assist_page;

This "extern" statement is added here so it is visible to both x86/x64 and arm64.
And that's correct.

But there is still some VP assist page stuff that has been left in the arch/x86
version of mshyperv.h.  That other stuff, including the inline function
hv_get_vp_assist_page(), should also be moved to asm-generic/mshyperv.h.
Given that the VP assist page support is now fully generic and not x86/x64
specific, it shouldn't occur anywhere in the arch/x86 version of mshyperv.h.

> +
>  const char *hv_result_to_string(u64 hv_status);
>  int hv_result_to_errno(u64 status);
>  void hyperv_report_panic(struct pt_regs *regs, long err, bool in_die);
> @@ -377,6 +380,7 @@ static inline int hv_deposit_memory(u64 partition_id, u64 status)
>  	return hv_deposit_memory_node(NUMA_NO_NODE, partition_id, status);
>  }
> 
> +#define HV_VP_ASSIST_PAGE_ADDRESS_SHIFT	12
>  #if IS_ENABLED(CONFIG_HYPERV_VTL_MODE)
>  u8 __init get_vtl(void);
>  #else
> diff --git a/include/hyperv/hvgdk_mini.h b/include/hyperv/hvgdk_mini.h
> index 056ef7b6b360..be697ddb211a 100644
> --- a/include/hyperv/hvgdk_mini.h
> +++ b/include/hyperv/hvgdk_mini.h
> @@ -149,6 +149,7 @@ struct hv_u128 {
>  #define HV_X64_MSR_VP_ASSIST_PAGE_ADDRESS_SHIFT	12
>  #define HV_X64_MSR_VP_ASSIST_PAGE_ADDRESS_MASK	\
>  		(~((1ull << HV_X64_MSR_VP_ASSIST_PAGE_ADDRESS_SHIFT) - 1))
> +#define HV_SYN_REG_VP_ASSIST_PAGE              (HV_X64_MSR_VP_ASSIST_PAGE)
> 
>  /* Hyper-V Enlightened VMCS version mask in nested features CPUID */
>  #define HV_X64_ENLIGHTENED_VMCS_VERSION		0xff
> @@ -1185,6 +1186,7 @@ enum hv_register_name {
> 
>  #define HV_MSR_STIMER0_CONFIG	(HV_REGISTER_STIMER0_CONFIG)
>  #define HV_MSR_STIMER0_COUNT	(HV_REGISTER_STIMER0_COUNT)
> +#define HV_SYN_REG_VP_ASSIST_PAGE    (HV_REGISTER_VP_ASSIST_PAGE)

This defines a new register name prefix "HV_SYN_REG_" that isn't used
anywhere else. The prefixes for Hyper-V register names are already complex
to account to x86/x64 and arm64 differences, and the fact the x86/x64 has
synthetic MSRs, while arm64 does not. So introducing another prefix is
undesirable. Couldn't this just be HV_MSR_VP_ASSIST_PAGE using the
same structure as HV_MSR_STIMER0_COUNT (for example)?

> 
>  #endif /* CONFIG_ARM64 */
> 
> --
> 2.43.0
>