Linux-HyperV List
 help / color / mirror / Atom feed
* Re: [PATCH] Drivers: hv: Use -ETIMEDOUT for HV_STATUS_TIME_OUT instead of -EIO
From: Easwar Hariharan @ 2025-10-02 23:06 UTC (permalink / raw)
  To: Stanislav Kinsburskii
  Cc: easwar.hariharan, K. Y. Srinivasan, Haiyang Zhang, Wei Liu,
	Dexuan Cui, Nuno Das Neves,
	open list:Hyper-V/Azure CORE AND DRIVERS, open list
In-Reply-To: <aN7-ryUqFNpgsIzI@skinsburskii.localdomain>

On 10/2/2025 3:37 PM, Stanislav Kinsburskii wrote:
> On Thu, Oct 02, 2025 at 10:13:46PM +0000, Easwar Hariharan wrote:
>> Use the -ETIMEDOUT errno value as the correct 1:1 match for the
>> hypervisor timeout status
>>
> 
> The commit message should answer the question why this change is being
> made.
> Is there a practical reason for this or is it only for consistensy?
> 
<snip>

It's for consistency and because it's a better match. See my sibling response
for other error codes that I can also fix in a v2.

Thanks,
Easwar (he/him)

^ permalink raw reply

* Re: [PATCH] Drivers: hv: Use -ETIMEDOUT for HV_STATUS_TIME_OUT instead of -EIO
From: Stanislav Kinsburskii @ 2025-10-02 22:37 UTC (permalink / raw)
  To: Easwar Hariharan
  Cc: K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
	Nuno Das Neves, open list:Hyper-V/Azure CORE AND DRIVERS,
	open list
In-Reply-To: <20251002221347.402320-1-easwar.hariharan@linux.microsoft.com>

On Thu, Oct 02, 2025 at 10:13:46PM +0000, Easwar Hariharan wrote:
> Use the -ETIMEDOUT errno value as the correct 1:1 match for the
> hypervisor timeout status
> 

The commit message should answer the question why this change is being
made.
Is there a practical reason for this or is it only for consistensy?

Thanks,
Stanislav

> Fixes: 3817854ba89201 ("hyperv: Log hypercall status codes as strings")
> Signed-off-by: Easwar Hariharan <easwar.hariharan@linux.microsoft.com>
> ---
>  drivers/hv/hv_common.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c
> index 49898d10fafff..9b51b67d54cc8 100644
> --- a/drivers/hv/hv_common.c
> +++ b/drivers/hv/hv_common.c
> @@ -781,7 +781,7 @@ static const struct hv_status_info hv_status_infos[] = {
>  	_STATUS_INFO(HV_STATUS_INVALID_LP_INDEX,		-EIO),
>  	_STATUS_INFO(HV_STATUS_INVALID_REGISTER_VALUE,		-EIO),
>  	_STATUS_INFO(HV_STATUS_OPERATION_FAILED,		-EIO),
> -	_STATUS_INFO(HV_STATUS_TIME_OUT,			-EIO),
> +	_STATUS_INFO(HV_STATUS_TIME_OUT,			-ETIMEDOUT),
>  	_STATUS_INFO(HV_STATUS_CALL_PENDING,			-EIO),
>  	_STATUS_INFO(HV_STATUS_VTL_ALREADY_ENABLED,		-EIO),
>  #undef _STATUS_INFO
> -- 
> 2.43.0

^ permalink raw reply

* Re: [PATCH] Drivers: hv: Use -ETIMEDOUT for HV_STATUS_TIME_OUT instead of -EIO
From: Easwar Hariharan @ 2025-10-02 22:30 UTC (permalink / raw)
  To: K. Y. Srinivasan
  Cc: Haiyang Zhang, Wei Liu, Dexuan Cui, Nuno Das Neves,
	Stanislav Kinsburskii, open list:Hyper-V/Azure CORE AND DRIVERS,
	open list, easwar.hariharan
In-Reply-To: <20251002221347.402320-1-easwar.hariharan@linux.microsoft.com>

On 10/2/2025 3:13 PM, Easwar Hariharan wrote:
> Use the -ETIMEDOUT errno value as the correct 1:1 match for the
> hypervisor timeout status
> 
> Fixes: 3817854ba89201 ("hyperv: Log hypercall status codes as strings")
> Signed-off-by: Easwar Hariharan <easwar.hariharan@linux.microsoft.com>
> ---
>  drivers/hv/hv_common.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c
> index 49898d10fafff..9b51b67d54cc8 100644
> --- a/drivers/hv/hv_common.c
> +++ b/drivers/hv/hv_common.c
> @@ -781,7 +781,7 @@ static const struct hv_status_info hv_status_infos[] = {
>  	_STATUS_INFO(HV_STATUS_INVALID_LP_INDEX,		-EIO),
>  	_STATUS_INFO(HV_STATUS_INVALID_REGISTER_VALUE,		-EIO),
>  	_STATUS_INFO(HV_STATUS_OPERATION_FAILED,		-EIO),
> -	_STATUS_INFO(HV_STATUS_TIME_OUT,			-EIO),
> +	_STATUS_INFO(HV_STATUS_TIME_OUT,			-ETIMEDOUT),
>  	_STATUS_INFO(HV_STATUS_CALL_PENDING,			-EIO),
>  	_STATUS_INFO(HV_STATUS_VTL_ALREADY_ENABLED,		-EIO),
>  #undef _STATUS_INFO

Actually looking at the whole struct, it may be useful to also change at least some of
 the following codes?

        _STATUS_INFO(HV_STATUS_INVALID_ALIGNMENT,               -EIO), -> EINVAL
        _STATUS_INFO(HV_STATUS_ACCESS_DENIED,                   -EIO), -> EACCES
        _STATUS_INFO(HV_STATUS_INVALID_PARTITION_STATE,         -EIO), -> EINVAL
        _STATUS_INFO(HV_STATUS_OPERATION_DENIED,                -EIO), -> EACCES
        _STATUS_INFO(HV_STATUS_PROPERTY_VALUE_OUT_OF_RANGE,     -EIO), -> ERANGE
        _STATUS_INFO(HV_STATUS_INSUFFICIENT_BUFFERS,            -EIO), -> ENOBUFS
        _STATUS_INFO(HV_STATUS_NOT_ACKNOWLEDGED,                -EIO), -> EBUSY
        _STATUS_INFO(HV_STATUS_INVALID_VP_STATE,                -EIO), -> EINVAL
        _STATUS_INFO(HV_STATUS_INVALID_LP_INDEX,                -EIO), -> EINVAL
        _STATUS_INFO(HV_STATUS_INVALID_REGISTER_VALUE,          -EIO), -> EINVAL
        _STATUS_INFO(HV_STATUS_VTL_ALREADY_ENABLED,             -EIO), -> EBUSY

Nuno, Stas, others, what do you think?

Thanks,
Easwar (he/him)

^ permalink raw reply

* [PATCH] Drivers: hv: Use -ETIMEDOUT for HV_STATUS_TIME_OUT instead of -EIO
From: Easwar Hariharan @ 2025-10-02 22:13 UTC (permalink / raw)
  To: K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
	Nuno Das Neves, Stanislav Kinsburskii,
	open list:Hyper-V/Azure CORE AND DRIVERS, open list
  Cc: Easwar Hariharan

Use the -ETIMEDOUT errno value as the correct 1:1 match for the
hypervisor timeout status

Fixes: 3817854ba89201 ("hyperv: Log hypercall status codes as strings")
Signed-off-by: Easwar Hariharan <easwar.hariharan@linux.microsoft.com>
---
 drivers/hv/hv_common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c
index 49898d10fafff..9b51b67d54cc8 100644
--- a/drivers/hv/hv_common.c
+++ b/drivers/hv/hv_common.c
@@ -781,7 +781,7 @@ static const struct hv_status_info hv_status_infos[] = {
 	_STATUS_INFO(HV_STATUS_INVALID_LP_INDEX,		-EIO),
 	_STATUS_INFO(HV_STATUS_INVALID_REGISTER_VALUE,		-EIO),
 	_STATUS_INFO(HV_STATUS_OPERATION_FAILED,		-EIO),
-	_STATUS_INFO(HV_STATUS_TIME_OUT,			-EIO),
+	_STATUS_INFO(HV_STATUS_TIME_OUT,			-ETIMEDOUT),
 	_STATUS_INFO(HV_STATUS_CALL_PENDING,			-EIO),
 	_STATUS_INFO(HV_STATUS_VTL_ALREADY_ENABLED,		-EIO),
 #undef _STATUS_INFO
-- 
2.43.0


^ permalink raw reply related

* Re: [PATCH v2 5/6] x86/hyperv: Implement hypervisor RAM collection into vmcore
From: Mukesh R @ 2025-10-02 22:07 UTC (permalink / raw)
  To: Wei Liu
  Cc: linux-hyperv, linux-kernel, linux-arch, kys, haiyangz, decui,
	tglx, mingo, bp, dave.hansen, x86, hpa, arnd
In-Reply-To: <20251002214236.GC925245@liuwe-devbox-debian-v2.local>

On 10/2/25 14:42, Wei Liu wrote:
> On Tue, Sep 23, 2025 at 02:46:08PM -0700, Mukesh Rathor wrote:
> [...]
>> +
>> +/*
>> + * This is the C entry point from the asm glue code after the devirt hypercall.
> 
> devirt -> devirtualization
> 
>> + * We enter here in IA32-e long mode, ie, full 64bit mode running on kernel
>> + * page tables with our below 4G page identity mapped, but using a temporary
>> + * GDT. ds/fs/gs/es are null. ss is not usable. bp is null. stack is not
>> + * available. We restore kernel GDT, and rest of the context, and continue
>> + * to kexec.
>> + */
> [...]
>> +
>> +static noinline __noclone void crash_nmi_callback(struct pt_regs *regs)
>> +{
>> +	struct hv_input_disable_hyp_ex *input;
>> +	u64 status;
>> +	int msecs = 1000, ccpu = smp_processor_id();
>> +
>> +	if (ccpu == 0) {
>> +		/* crash_save_cpu() will be done in the kexec path */
>> +		cpu_emergency_stop_pt();	/* disable performance trace */
>> +		atomic_inc(&crash_cpus_wait);
>> +	} else {
>> +		crash_save_cpu(regs, ccpu);
>> +		cpu_emergency_stop_pt();	/* disable performance trace */
>> +		atomic_inc(&crash_cpus_wait);
>> +		for (;;)
>> +			cpu_relax();
>> +	}
>> +
>> +	while (atomic_read(&crash_cpus_wait) < num_online_cpus() && msecs--)
>> +		mdelay(1);
>> +
>> +	stop_nmi();
>> +	if (!hv_has_crashed)
>> +		hv_notify_prepare_hyp();
>> +
>> +	if (crashing_cpu == -1)
>> +		crashing_cpu = ccpu;		/* crash cmd uses this */
>> +
>> +	hv_hvcrash_ctxt_save();
>> +	hv_mark_tss_not_busy();
>> +	hv_crash_fixup_kernpt();
>> +
>> +	input = *this_cpu_ptr(hyperv_pcpu_input_arg);
>> +	memset(input, 0, sizeof(*input));
>> +	input->rip = trampoline_pa;
>> +	input->arg = devirt_arg;
>> +
>> +	status = hv_do_hypercall(HVCALL_DISABLE_HYP_EX, input, NULL);
>> +
> 
> If I understand this correctly, after this call, upon return from the
> hypervisor, Linux will start executing the trampoline code.

correct.


>> +	hv_panic_timeout_reboot();
> 
> Why is this needed? Is it to catch the case when the hypercall fails?
  
correct.
 

> [...]
>> +static void __noclone hv_crash_stop_other_cpus(void)
>> +{
>> +	static bool crash_stop_done;
>> +	struct pt_regs lregs;
>> +	int ccpu = smp_processor_id();
>> +
>> +	if (hv_has_crashed)
>> +		return;		/* all cpus already in NMI handler path */
>> +
>> +	if (!kexec_crash_loaded()) {
>> +		hv_notify_prepare_hyp();
>> +		hv_panic_timeout_reboot();	/* no return */
>> +	}
>> +
>> +	/* If hyp crashes also, we could come here again before cpus_stopped is
> 
> hypervisor or hv (given the same term is used in the function)
> 
>> +	 * set in crash_smp_send_stop(). So use our own check.
>> +	 */
>> +	if (crash_stop_done)
>> +		return;
>> +	crash_stop_done = true;
>> +
>> +	/* Linux has crashed: hv is healthy, we can ipi safely */
> 
> IPI.
> 
>> +
>> +err_out:
>> +	unregister_nmi_handler(NMI_LOCAL, "hv_crash_nmi");
>> +	pr_err("Hyper-V: only linux (but not hyp) kdump support enabled\n");
> 
> hypervisor not hyp. This is a message for the user so we should be as
> clear as possible.

> Wei
> 
>> +}
>> -- 
>> 2.36.1.vfs.0.0
>>


^ permalink raw reply

* Re: [PATCH v2 5/6] x86/hyperv: Implement hypervisor RAM collection into vmcore
From: Wei Liu @ 2025-10-02 21:42 UTC (permalink / raw)
  To: Mukesh Rathor
  Cc: linux-hyperv, linux-kernel, linux-arch, kys, haiyangz, wei.liu,
	decui, tglx, mingo, bp, dave.hansen, x86, hpa, arnd
In-Reply-To: <20250923214609.4101554-6-mrathor@linux.microsoft.com>

On Tue, Sep 23, 2025 at 02:46:08PM -0700, Mukesh Rathor wrote:
[...]
> +
> +/*
> + * This is the C entry point from the asm glue code after the devirt hypercall.

devirt -> devirtualization

> + * We enter here in IA32-e long mode, ie, full 64bit mode running on kernel
> + * page tables with our below 4G page identity mapped, but using a temporary
> + * GDT. ds/fs/gs/es are null. ss is not usable. bp is null. stack is not
> + * available. We restore kernel GDT, and rest of the context, and continue
> + * to kexec.
> + */
[...]
> +
> +static noinline __noclone void crash_nmi_callback(struct pt_regs *regs)
> +{
> +	struct hv_input_disable_hyp_ex *input;
> +	u64 status;
> +	int msecs = 1000, ccpu = smp_processor_id();
> +
> +	if (ccpu == 0) {
> +		/* crash_save_cpu() will be done in the kexec path */
> +		cpu_emergency_stop_pt();	/* disable performance trace */
> +		atomic_inc(&crash_cpus_wait);
> +	} else {
> +		crash_save_cpu(regs, ccpu);
> +		cpu_emergency_stop_pt();	/* disable performance trace */
> +		atomic_inc(&crash_cpus_wait);
> +		for (;;)
> +			cpu_relax();
> +	}
> +
> +	while (atomic_read(&crash_cpus_wait) < num_online_cpus() && msecs--)
> +		mdelay(1);
> +
> +	stop_nmi();
> +	if (!hv_has_crashed)
> +		hv_notify_prepare_hyp();
> +
> +	if (crashing_cpu == -1)
> +		crashing_cpu = ccpu;		/* crash cmd uses this */
> +
> +	hv_hvcrash_ctxt_save();
> +	hv_mark_tss_not_busy();
> +	hv_crash_fixup_kernpt();
> +
> +	input = *this_cpu_ptr(hyperv_pcpu_input_arg);
> +	memset(input, 0, sizeof(*input));
> +	input->rip = trampoline_pa;
> +	input->arg = devirt_arg;
> +
> +	status = hv_do_hypercall(HVCALL_DISABLE_HYP_EX, input, NULL);
> +

If I understand this correctly, after this call, upon return from the
hypervisor, Linux will start executing the trampoline code.

> +	hv_panic_timeout_reboot();

Why is this needed? Is it to catch the case when the hypercall fails?


[...]
> +static void __noclone hv_crash_stop_other_cpus(void)
> +{
> +	static bool crash_stop_done;
> +	struct pt_regs lregs;
> +	int ccpu = smp_processor_id();
> +
> +	if (hv_has_crashed)
> +		return;		/* all cpus already in NMI handler path */
> +
> +	if (!kexec_crash_loaded()) {
> +		hv_notify_prepare_hyp();
> +		hv_panic_timeout_reboot();	/* no return */
> +	}
> +
> +	/* If hyp crashes also, we could come here again before cpus_stopped is

hypervisor or hv (given the same term is used in the function)

> +	 * set in crash_smp_send_stop(). So use our own check.
> +	 */
> +	if (crash_stop_done)
> +		return;
> +	crash_stop_done = true;
> +
> +	/* Linux has crashed: hv is healthy, we can ipi safely */

IPI.

> +
> +err_out:
> +	unregister_nmi_handler(NMI_LOCAL, "hv_crash_nmi");
> +	pr_err("Hyper-V: only linux (but not hyp) kdump support enabled\n");

hypervisor not hyp. This is a message for the user so we should be as
clear as possible.

Wei

> +}
> -- 
> 2.36.1.vfs.0.0
> 

^ permalink raw reply

* Re: [PATCH hyperv-next] hyperv: Remove the spurious null directive line
From: Wei Liu @ 2025-10-02 21:21 UTC (permalink / raw)
  To: Easwar Hariharan
  Cc: Roman Kisel, decui, eahariha, haiyangz, kys, mhklinux,
	nunodasneves, wei.liu, linux-hyperv, linux-kernel, benhill,
	bperkins, sunilmut
In-Reply-To: <089d6be9-20be-4985-a761-51bfb9879b92@linux.microsoft.com>

On Thu, Oct 02, 2025 at 09:04:21AM -0700, Easwar Hariharan wrote:
> On 10/1/2025 4:08 PM, Roman Kisel wrote:
> > The file contains a line that consists of the lone # symbol
> > followed by a newline. While that is a valid syntax as
> > defined by the C99+ grammar (6.10.7 "Null directive"), it
> > serves no apparent purpose in this case.
> > 
> > Remove the null preprocessor directive. No functional changes.
> > 
> > Fixes: e68bda71a238 ("hyperv: Add new Hyper-V headers in include/hyperv")
> > Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
> > ---
> >  include/hyperv/hvgdk_mini.h | 2 --
> >  1 file changed, 2 deletions(-)
> > 
> > diff --git a/include/hyperv/hvgdk_mini.h b/include/hyperv/hvgdk_mini.h
> > index 1be7f6a02304..77abddfc750e 100644
> > --- a/include/hyperv/hvgdk_mini.h
> > +++ b/include/hyperv/hvgdk_mini.h
> > @@ -597,8 +597,6 @@ struct ms_hyperv_tsc_page {	 /* HV_REFERENCE_TSC_PAGE */
> >  #define HV_SYNIC_SINT_AUTO_EOI		(1ULL << 17)
> >  #define HV_SYNIC_SINT_VECTOR_MASK	(0xFF)
> >  
> > -#
> > -
> >  /* Hyper-V defined statically assigned SINTs */
> >  #define HV_SYNIC_INTERCEPTION_SINT_INDEX 0x00000000
> >  #define HV_SYNIC_IOMMU_FAULT_SINT_INDEX  0x00000001
> > 
> > base-commit: e3ec97c3abaf2fb68cc755cae3229288696b9f3d
> 
> Reviewed-by: Easwar Hariharan <easwar.hariharan@linux.microsoft.com>

Applied.

^ permalink raw reply

* Re: [PATCH v2 0/4] Introduce movable pages for Hyper-V guests
From: Stanislav Kinsburskii @ 2025-10-02 16:36 UTC (permalink / raw)
  To: Wei Liu; +Cc: kys, haiyangz, decui, linux-hyperv, linux-kernel
In-Reply-To: <aNyoK8xfgy2zpKCf@liuwe-devbox-ubuntu-v2.lamzopl0uupeniq2etz1fddiyg.xx.internal.cloudapp.net>

On Wed, Oct 01, 2025 at 04:03:55AM +0000, Wei Liu wrote:
> On Mon, Sep 29, 2025 at 04:02:59PM +0000, Stanislav Kinsburskii wrote:
> > From the start, the root-partition driver allocates, pins, and maps all
> > guest memory into the hypervisor at guest creation. This is simple: Linux
> > cannot move the pages, so the guest’s view in Linux and in Microsoft
> > Hypervisor never diverges.
> > 
> > However, this approach has major drawbacks:
> > - NUMA: affinity can’t be changed at runtime, so you can’t migrate guest memory closer to the CPUs running it → performance hit.
> > - Memory management: unused guest memory can’t be swapped out, compacted, or merged.
> > - Provisioning time: upfront allocation/pinning slows guest create/destroy.
> > - Overcommit: no memory overcommit on hosts with pinned-guest memory.
> > 
> > This series adds movable memory pages for Hyper-V child partitions. Guest
> > pages are no longer allocated upfront; they’re allocated and mapped into
> > the hypervisor on demand (i.e., when the guest touches a GFN that isn’t yet
> > backed by a host PFN).
> > When a page is moved, Linux no longer holds it and it is unmapped from the hypervisor.
> > As a result, Hyper-V guests behave like regular Linux processes, enabling standard Linux memory features to apply to guests.
> > 
> > Exceptions (still pinned):
> >   1. Encrypted guests (explicit).
> >   2 Guests with passthrough devices (implicitly pinned by the VFIO framework).
> > 
> > v2:
> > - Split unmap batching into a separate patch.
> > - Fixed commit messages from v1 review.
> > - Renamed a few functions for clarity.
> > 
> > ---
> > 
> > Stanislav Kinsburskii (4):
> >       Drivers: hv: Refactor and rename memory region handling functions
> >       Drivers: hv: Centralize guest memory region destruction
> >       Drivers: hv: Batch GPA unmap operations to improve large region performance
> >       Drivers: hv: Add support for movable memory regions
> > 
> 
> Our internal test discovered an issue which caused a kernel panic. Has
> that been fixed in this series?
> 
> Without fixing that bug or ruling out problems in the code I don't think
> I can merge this. Once this feature goes into other people's systems it
> will be extremely difficult to debug.
> 
> Wei
> 

The issue in fixed in v3.

Thanks,
Stanislav.

> 
> > 
> >  drivers/hv/Kconfig             |    1 
> >  drivers/hv/mshv_root.h         |   10 +
> >  drivers/hv/mshv_root_hv_call.c |    2 
> >  drivers/hv/mshv_root_main.c    |  472 +++++++++++++++++++++++++++++++++-------
> >  4 files changed, 403 insertions(+), 82 deletions(-)
> > 

^ permalink raw reply

* [PATCH v3 5/5] Drivers: hv: Add support for movable memory regions
From: Stanislav Kinsburskii @ 2025-10-02 16:36 UTC (permalink / raw)
  To: kys, haiyangz, wei.liu, decui; +Cc: linux-hyperv, linux-kernel
In-Reply-To: <175942263069.128138.16103948906881178993.stgit@skinsburskii-cloud-desktop.internal.cloudapp.net>

Introduce support for movable memory regions in the Hyper-V root partition
driver, thus improving memory management flexibility and preparing the
driver for advanced use cases such as dynamic memory remapping.

Integrate mmu_interval_notifier for movable regions, implement functions to
handle HMM faults and memory invalidation, and update memory region mapping
logic to support movable regions.

While MMU notifiers are commonly used in virtualization drivers, this
implementation leverages HMM (Heterogeneous Memory Management) for its
tailored functionality. HMM provides a ready-made framework for mirroring,
invalidation, and fault handling, avoiding the need to reimplement these
mechanisms for a single callback. Although MMU notifiers are more generic,
using HMM reduces boilerplate and ensures maintainability by utilizing a
mechanism specifically designed for such use cases.

Signed-off-by: Anirudh Rayabharam <anrayabh@linux.microsoft.com>
Signed-off-by: Stanislav Kinsburskii <skinsburskii@linux.microsoft.com>
---
 drivers/hv/Kconfig          |    1 
 drivers/hv/mshv_root.h      |    8 +
 drivers/hv/mshv_root_main.c |  328 ++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 327 insertions(+), 10 deletions(-)

diff --git a/drivers/hv/Kconfig b/drivers/hv/Kconfig
index e24f6299c3760..9d24a8c8c52e3 100644
--- a/drivers/hv/Kconfig
+++ b/drivers/hv/Kconfig
@@ -68,6 +68,7 @@ config MSHV_ROOT
 	depends on PAGE_SIZE_4KB
 	select EVENTFD
 	select VIRT_XFER_TO_GUEST_WORK
+	select HMM_MIRROR
 	default n
 	help
 	  Select this option to enable support for booting and running as root
diff --git a/drivers/hv/mshv_root.h b/drivers/hv/mshv_root.h
index 97e64d5341b6e..13367c84497c0 100644
--- a/drivers/hv/mshv_root.h
+++ b/drivers/hv/mshv_root.h
@@ -15,6 +15,7 @@
 #include <linux/hashtable.h>
 #include <linux/dev_printk.h>
 #include <linux/build_bug.h>
+#include <linux/mmu_notifier.h>
 #include <uapi/linux/mshv.h>
 
 /*
@@ -81,9 +82,14 @@ struct mshv_mem_region {
 	struct {
 		u64 large_pages:  1; /* 2MiB */
 		u64 range_pinned: 1;
-		u64 reserved:	 62;
+		u64 is_ram	: 1; /* mem region can be ram or mmio */
+		u64 reserved:	 61;
 	} flags;
 	struct mshv_partition *partition;
+#if defined(CONFIG_MMU_NOTIFIER)
+	struct mmu_interval_notifier mni;
+	struct mutex mutex;	/* protects region pages remapping */
+#endif
 	struct page *pages[];
 };
 
diff --git a/drivers/hv/mshv_root_main.c b/drivers/hv/mshv_root_main.c
index 9547833a04681..d31a79e1bc3e6 100644
--- a/drivers/hv/mshv_root_main.c
+++ b/drivers/hv/mshv_root_main.c
@@ -29,6 +29,7 @@
 #include <linux/crash_dump.h>
 #include <linux/panic_notifier.h>
 #include <linux/vmalloc.h>
+#include <linux/hmm.h>
 
 #include "mshv_eventfd.h"
 #include "mshv.h"
@@ -36,6 +37,8 @@
 
 #define VALUE_PMD_ALIGNED(c)			(!((c) & (PTRS_PER_PMD - 1)))
 
+#define MSHV_MAP_FAULT_IN_PAGES			HPAGE_PMD_NR
+
 MODULE_AUTHOR("Microsoft");
 MODULE_LICENSE("GPL");
 MODULE_DESCRIPTION("Microsoft Hyper-V root partition VMM interface /dev/mshv");
@@ -76,6 +79,11 @@ static int mshv_vp_mmap(struct file *file, struct vm_area_struct *vma);
 static vm_fault_t mshv_vp_fault(struct vm_fault *vmf);
 static int mshv_init_async_handler(struct mshv_partition *partition);
 static void mshv_async_hvcall_handler(void *data, u64 *status);
+static struct mshv_mem_region
+	*mshv_partition_region_by_gfn(struct mshv_partition *pt, u64 gfn);
+static int mshv_region_remap_pages(struct mshv_mem_region *region,
+				   u32 map_flags, u64 page_offset,
+				   u64 page_count);
 
 static const union hv_input_vtl input_vtl_zero;
 static const union hv_input_vtl input_vtl_normal = {
@@ -602,14 +610,197 @@ static long mshv_run_vp_with_root_scheduler(struct mshv_vp *vp)
 static_assert(sizeof(struct hv_message) <= MSHV_RUN_VP_BUF_SZ,
 	      "sizeof(struct hv_message) must not exceed MSHV_RUN_VP_BUF_SZ");
 
+#ifdef CONFIG_X86_64
+
+#if defined(CONFIG_MMU_NOTIFIER)
+/**
+ * mshv_region_hmm_fault_and_lock - Handle HMM faults and lock the memory region
+ * @region: Pointer to the memory region structure
+ * @range: Pointer to the HMM range structure
+ *
+ * This function performs the following steps:
+ * 1. Reads the notifier sequence for the HMM range.
+ * 2. Acquires a read lock on the memory map.
+ * 3. Handles HMM faults for the specified range.
+ * 4. Releases the read lock on the memory map.
+ * 5. If successful, locks the memory region mutex.
+ * 6. Verifies if the notifier sequence has changed during the operation.
+ *    If it has, releases the mutex and returns -EBUSY to match with
+ *    hmm_range_fault() return code for repeating.
+ *
+ * Return: 0 on success, a negative error code otherwise.
+ */
+static int mshv_region_hmm_fault_and_lock(struct mshv_mem_region *region,
+					  struct hmm_range *range)
+{
+	int ret;
+
+	range->notifier_seq = mmu_interval_read_begin(range->notifier);
+	mmap_read_lock(region->mni.mm);
+	ret = hmm_range_fault(range);
+	mmap_read_unlock(region->mni.mm);
+	if (ret)
+		return ret;
+
+	mutex_lock(&region->mutex);
+
+	if (mmu_interval_read_retry(range->notifier, range->notifier_seq)) {
+		mutex_unlock(&region->mutex);
+		cond_resched();
+		return -EBUSY;
+	}
+
+	return 0;
+}
+
+/**
+ * mshv_region_range_fault - Handle memory range faults for a given region.
+ * @region: Pointer to the memory region structure.
+ * @page_offset: Offset of the page within the region.
+ * @page_count: Number of pages to handle.
+ *
+ * This function resolves memory faults for a specified range of pages
+ * within a memory region. It uses HMM (Heterogeneous Memory Management)
+ * to fault in the required pages and updates the region's page array.
+ *
+ * Return: 0 on success, negative error code on failure.
+ */
+static int mshv_region_range_fault(struct mshv_mem_region *region,
+				   u64 page_offset, u64 page_count)
+{
+	struct hmm_range range = {
+		.notifier = &region->mni,
+		.default_flags = HMM_PFN_REQ_FAULT | HMM_PFN_REQ_WRITE,
+	};
+	unsigned long *pfns;
+	int ret;
+	u64 i;
+
+	pfns = kmalloc_array(page_count, sizeof(unsigned long), GFP_KERNEL);
+	if (!pfns)
+		return -ENOMEM;
+
+	range.hmm_pfns = pfns;
+	range.start = region->start_uaddr + page_offset * HV_HYP_PAGE_SIZE;
+	range.end = range.start + page_count * HV_HYP_PAGE_SIZE;
+
+	do {
+		ret = mshv_region_hmm_fault_and_lock(region, &range);
+	} while (ret == -EBUSY);
+
+	if (ret)
+		goto out;
+
+	for (i = 0; i < page_count; i++)
+		region->pages[page_offset + i] = hmm_pfn_to_page(pfns[i]);
+
+	if (PageHuge(region->pages[page_offset]))
+		region->flags.large_pages = true;
+
+	ret = mshv_region_remap_pages(region, region->hv_map_flags,
+				      page_offset, page_count);
+
+	mutex_unlock(&region->mutex);
+out:
+	kfree(pfns);
+	return ret;
+}
+#else /* CONFIG_MMU_NOTIFIER */
+static int mshv_region_range_fault(struct mshv_mem_region *region,
+				   u64 page_offset, u64 page_count)
+{
+	return -ENODEV;
+}
+#endif /* CONFIG_MMU_NOTIFIER */
+
+static bool mshv_region_handle_gfn_fault(struct mshv_mem_region *region, u64 gfn)
+{
+	u64 page_offset, page_count;
+	int ret;
+
+	if (WARN_ON_ONCE(region->flags.range_pinned))
+		return false;
+
+	/* Align the page offset to the nearest MSHV_MAP_FAULT_IN_PAGES. */
+	page_offset = ALIGN_DOWN(gfn - region->start_gfn,
+				 MSHV_MAP_FAULT_IN_PAGES);
+
+	/* Map more pages than requested to reduce the number of faults. */
+	page_count = min(region->nr_pages - page_offset,
+			 MSHV_MAP_FAULT_IN_PAGES);
+
+	ret = mshv_region_range_fault(region, page_offset, page_count);
+
+	WARN_ONCE(ret,
+		  "p%llu: GPA intercept failed: region %#llx-%#llx, gfn %#llx, page_offset %llu, page_count %llu\n",
+		  region->partition->pt_id, region->start_uaddr,
+		  region->start_uaddr + (region->nr_pages << HV_HYP_PAGE_SHIFT),
+		  gfn, page_offset, page_count);
+
+	return !ret;
+}
+
+/**
+ * mshv_handle_gpa_intercept - Handle GPA (Guest Physical Address) intercepts.
+ * @vp: Pointer to the virtual processor structure.
+ *
+ * This function processes GPA intercepts by identifying the memory region
+ * corresponding to the intercepted GPA, aligning the page offset, and
+ * mapping the required pages. It ensures that the region is valid and
+ * handles faults efficiently by mapping multiple pages at once.
+ *
+ * Return: true if the intercept was handled successfully, false otherwise.
+ */
+static bool mshv_handle_gpa_intercept(struct mshv_vp *vp)
+{
+	struct mshv_partition *p = vp->vp_partition;
+	struct mshv_mem_region *region;
+	struct hv_x64_memory_intercept_message *msg;
+	u64 gfn;
+
+	msg = (struct hv_x64_memory_intercept_message *)
+		vp->vp_intercept_msg_page->u.payload;
+
+	gfn = HVPFN_DOWN(msg->guest_physical_address);
+
+	region = mshv_partition_region_by_gfn(p, gfn);
+	if (!region)
+		return false;
+
+	if (WARN_ON_ONCE(!region->flags.is_ram))
+		return false;
+
+	if (WARN_ON_ONCE(region->flags.range_pinned))
+		return false;
+
+	return mshv_region_handle_gfn_fault(region, gfn);
+}
+
+#else	/* CONFIG_X86_64 */
+
+static bool mshv_handle_gpa_intercept(struct mshv_vp *vp) { return false; }
+
+#endif	/* CONFIG_X86_64 */
+
+static bool mshv_vp_handle_intercept(struct mshv_vp *vp)
+{
+	switch (vp->vp_intercept_msg_page->header.message_type) {
+	case HVMSG_GPA_INTERCEPT:
+		return mshv_handle_gpa_intercept(vp);
+	}
+	return false;
+}
+
 static long mshv_vp_ioctl_run_vp(struct mshv_vp *vp, void __user *ret_msg)
 {
 	long rc;
 
-	if (hv_scheduler_type == HV_SCHEDULER_TYPE_ROOT)
-		rc = mshv_run_vp_with_root_scheduler(vp);
-	else
-		rc = mshv_run_vp_with_hyp_scheduler(vp);
+	do {
+		if (hv_scheduler_type == HV_SCHEDULER_TYPE_ROOT)
+			rc = mshv_run_vp_with_root_scheduler(vp);
+		else
+			rc = mshv_run_vp_with_hyp_scheduler(vp);
+	} while (rc == 0 && mshv_vp_handle_intercept(vp));
 
 	if (rc)
 		return rc;
@@ -1209,6 +1400,110 @@ mshv_partition_region_by_uaddr(struct mshv_partition *partition, u64 uaddr)
 	return NULL;
 }
 
+#if defined(CONFIG_MMU_NOTIFIER)
+static void mshv_region_movable_fini(struct mshv_mem_region *region)
+{
+	if (region->flags.range_pinned)
+		return;
+
+	mmu_interval_notifier_remove(&region->mni);
+}
+
+/**
+ * mshv_region_interval_invalidate - Invalidate a range of memory region
+ * @mni: Pointer to the mmu_interval_notifier structure
+ * @range: Pointer to the mmu_notifier_range structure
+ * @cur_seq: Current sequence number for the interval notifier
+ *
+ * This function invalidates a memory region by remapping its pages with
+ * no access permissions. It locks the region's mutex to ensure thread safety
+ * and updates the sequence number for the interval notifier. If the range
+ * is blockable, it uses a blocking lock; otherwise, it attempts a non-blocking
+ * lock and returns false if unsuccessful.
+ *
+ * NOTE: Failure to invalidate a region is a serious error, as the pages will
+ * be considered freed while they are still mapped by the hypervisor.
+ * Any attempt to access such pages will likely crash the system.
+ *
+ * Return: true if the region was successfully invalidated, false otherwise.
+ */
+static bool
+mshv_region_interval_invalidate(struct mmu_interval_notifier *mni,
+				const struct mmu_notifier_range *range,
+				unsigned long cur_seq)
+{
+	struct mshv_mem_region *region = container_of(mni,
+						struct mshv_mem_region,
+						mni);
+	u64 page_offset, page_count;
+	unsigned long mstart, mend;
+	int ret;
+
+	if (mmu_notifier_range_blockable(range))
+		mutex_lock(&region->mutex);
+	else if (!mutex_trylock(&region->mutex))
+		goto out_fail;
+
+	mmu_interval_set_seq(mni, cur_seq);
+
+	mstart = max(range->start, region->start_uaddr);
+	mend = min(range->end, region->start_uaddr +
+		   (region->nr_pages << HV_HYP_PAGE_SHIFT));
+
+	page_offset = HVPFN_DOWN(mstart - region->start_uaddr);
+	page_count = HVPFN_DOWN(mend - mstart);
+
+	ret = mshv_region_remap_pages(region, HV_MAP_GPA_NO_ACCESS,
+				      page_offset, page_count);
+	if (ret)
+		goto out_fail;
+
+	mshv_region_invalidate_pages(region, page_offset, page_count);
+
+	mutex_unlock(&region->mutex);
+
+	return true;
+
+out_fail:
+	WARN_ONCE(ret,
+		  "Failed to invalidate region %#llx-%#llx (range %#lx-%#lx, event: %u, pages %#llx-%#llx, mm: %#llx): %d\n",
+		  region->start_uaddr,
+		  region->start_uaddr + (region->nr_pages << HV_HYP_PAGE_SHIFT),
+		  range->start, range->end, range->event,
+		  page_offset, page_offset + page_count - 1, (u64)range->mm, ret);
+	return false;
+}
+
+static const struct mmu_interval_notifier_ops mshv_region_mni_ops = {
+	.invalidate = mshv_region_interval_invalidate,
+};
+
+static bool mshv_region_movable_init(struct mshv_mem_region *region)
+{
+	int ret;
+
+	ret = mmu_interval_notifier_insert(&region->mni, current->mm,
+					   region->start_uaddr,
+					   region->nr_pages << HV_HYP_PAGE_SHIFT,
+					   &mshv_region_mni_ops);
+	if (ret)
+		return false;
+
+	mutex_init(&region->mutex);
+
+	return true;
+}
+#else
+static inline void mshv_region_movable_fini(struct mshv_mem_region *region)
+{
+}
+
+static inline bool mshv_region_movable_init(struct mshv_mem_region *region)
+{
+	return false;
+}
+#endif
+
 /*
  * NB: caller checks and makes sure mem->size is page aligned
  * Returns: 0 with regionpp updated on success, or -errno
@@ -1241,9 +1536,14 @@ static int mshv_partition_create_region(struct mshv_partition *partition,
 	if (mem->flags & BIT(MSHV_SET_MEM_BIT_EXECUTABLE))
 		region->hv_map_flags |= HV_MAP_GPA_EXECUTABLE;
 
-	/* Note: large_pages flag populated when we pin the pages */
-	if (!is_mmio)
-		region->flags.range_pinned = true;
+	/* Note: large_pages flag populated when pages are allocated. */
+	if (!is_mmio) {
+		region->flags.is_ram = true;
+
+		if (mshv_partition_encrypted(partition) ||
+		    !mshv_region_movable_init(region))
+			region->flags.range_pinned = true;
+	}
 
 	region->partition = partition;
 
@@ -1363,9 +1663,16 @@ mshv_map_user_memory(struct mshv_partition *partition,
 	if (is_mmio)
 		ret = hv_call_map_mmio_pages(partition->pt_id, mem.guest_pfn,
 					     mmio_pfn, HVPFN_DOWN(mem.size));
-	else
+	else if (region->flags.range_pinned)
 		ret = mshv_prepare_pinned_region(region);
-
+	else
+		/*
+		 * For non-pinned regions, remap with no access to let the
+		 * hypervisor track dirty pages, enabling pre-copy live
+		 * migration.
+		 */
+		ret = mshv_region_remap_pages(region, HV_MAP_GPA_NO_ACCESS,
+					      0, region->nr_pages);
 	if (ret)
 		goto errout;
 
@@ -1388,6 +1695,9 @@ static void mshv_partition_destroy_region(struct mshv_mem_region *region)
 
 	hlist_del(&region->hnode);
 
+	if (region->flags.is_ram)
+		mshv_region_movable_fini(region);
+
 	if (mshv_partition_encrypted(partition)) {
 		ret = mshv_partition_region_share(region);
 		if (ret) {



^ permalink raw reply related

* [PATCH v3 4/5] Drivers: hv: Ensure large page GPA mapping is PMD-aligned
From: Stanislav Kinsburskii @ 2025-10-02 16:36 UTC (permalink / raw)
  To: kys, haiyangz, wei.liu, decui; +Cc: linux-hyperv, linux-kernel
In-Reply-To: <175942263069.128138.16103948906881178993.stgit@skinsburskii-cloud-desktop.internal.cloudapp.net>

With the upcoming introduction of movable pages, a region doesn't guarantee
always having large pages mapped. Both mapping on fault and unmapping
during PTE invalidation may not be 2M-aligned, while the hypervisor
requires both the GFN and page count to be 2M-aligned to use the large page
flag.

Update the logic for large page mapping in mshv_region_remap_pages() to
require both page_offset and page_count to be PMD-aligned.

Signed-off-by: Stanislav Kinsburskii <skinsburskii@linux.microsoft.com>
---
 drivers/hv/mshv_root_main.c |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/hv/mshv_root_main.c b/drivers/hv/mshv_root_main.c
index 8bf0b5af25802..9547833a04681 100644
--- a/drivers/hv/mshv_root_main.c
+++ b/drivers/hv/mshv_root_main.c
@@ -34,6 +34,8 @@
 #include "mshv.h"
 #include "mshv_root.h"
 
+#define VALUE_PMD_ALIGNED(c)			(!((c) & (PTRS_PER_PMD - 1)))
+
 MODULE_AUTHOR("Microsoft");
 MODULE_LICENSE("GPL");
 MODULE_DESCRIPTION("Microsoft Hyper-V root partition VMM interface /dev/mshv");
@@ -1100,7 +1102,9 @@ mshv_region_remap_pages(struct mshv_mem_region *region, u32 map_flags,
 	if (page_offset + page_count > region->nr_pages)
 		return -EINVAL;
 
-	if (region->flags.large_pages)
+	if (region->flags.large_pages &&
+	    VALUE_PMD_ALIGNED(page_offset) &&
+	    VALUE_PMD_ALIGNED(page_count))
 		map_flags |= HV_MAP_GPA_LARGE_PAGE;
 
 	/* ask the hypervisor to map guest ram */



^ permalink raw reply related

* [PATCH v3 3/5] Drivers: hv: Batch GPA unmap operations to improve large region performance
From: Stanislav Kinsburskii @ 2025-10-02 16:36 UTC (permalink / raw)
  To: kys, haiyangz, wei.liu, decui; +Cc: linux-hyperv, linux-kernel
In-Reply-To: <175942263069.128138.16103948906881178993.stgit@skinsburskii-cloud-desktop.internal.cloudapp.net>

Reduce overhead when unmapping large memory regions by batching GPA unmap
operations in 2MB-aligned chunks.

Use a dedicated constant for batch size to improve code clarity and
maintainability.

Signed-off-by: Stanislav Kinsburskii <skinsburskii@linux.microsoft.com>
---
 drivers/hv/mshv_root.h         |    2 ++
 drivers/hv/mshv_root_hv_call.c |    2 +-
 drivers/hv/mshv_root_main.c    |   15 ++++++++++++---
 3 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/drivers/hv/mshv_root.h b/drivers/hv/mshv_root.h
index e3931b0f12693..97e64d5341b6e 100644
--- a/drivers/hv/mshv_root.h
+++ b/drivers/hv/mshv_root.h
@@ -32,6 +32,8 @@ static_assert(HV_HYP_PAGE_SIZE == MSHV_HV_PAGE_SIZE);
 
 #define MSHV_PIN_PAGES_BATCH_SIZE	(0x10000000ULL / HV_HYP_PAGE_SIZE)
 
+#define MSHV_MAX_UNMAP_GPA_PAGES	512
+
 struct mshv_vp {
 	u32 vp_index;
 	struct mshv_partition *vp_partition;
diff --git a/drivers/hv/mshv_root_hv_call.c b/drivers/hv/mshv_root_hv_call.c
index c9c274f29c3c6..0696024ccfe31 100644
--- a/drivers/hv/mshv_root_hv_call.c
+++ b/drivers/hv/mshv_root_hv_call.c
@@ -17,7 +17,7 @@
 /* Determined empirically */
 #define HV_INIT_PARTITION_DEPOSIT_PAGES 208
 #define HV_MAP_GPA_DEPOSIT_PAGES	256
-#define HV_UMAP_GPA_PAGES		512
+#define HV_UMAP_GPA_PAGES		MSHV_MAX_UNMAP_GPA_PAGES
 
 #define HV_PAGE_COUNT_2M_ALIGNED(pg_count) (!((pg_count) & (0x200 - 1)))
 
diff --git a/drivers/hv/mshv_root_main.c b/drivers/hv/mshv_root_main.c
index 7ef66c43e1515..8bf0b5af25802 100644
--- a/drivers/hv/mshv_root_main.c
+++ b/drivers/hv/mshv_root_main.c
@@ -1378,6 +1378,7 @@ mshv_map_user_memory(struct mshv_partition *partition,
 static void mshv_partition_destroy_region(struct mshv_mem_region *region)
 {
 	struct mshv_partition *partition = region->partition;
+	u64 page_offset;
 	u32 unmap_flags = 0;
 	int ret;
 
@@ -1396,9 +1397,17 @@ static void mshv_partition_destroy_region(struct mshv_mem_region *region)
 	if (region->flags.large_pages)
 		unmap_flags |= HV_UNMAP_GPA_LARGE_PAGE;
 
-	/* ignore unmap failures and continue as process may be exiting */
-	hv_call_unmap_gpa_pages(partition->pt_id, region->start_gfn,
-				region->nr_pages, unmap_flags);
+	for (page_offset = 0; page_offset < region->nr_pages; page_offset++) {
+		if (!region->pages[page_offset])
+			continue;
+
+		hv_call_unmap_gpa_pages(partition->pt_id,
+					ALIGN(region->start_gfn + page_offset,
+					      MSHV_MAX_UNMAP_GPA_PAGES),
+					MSHV_MAX_UNMAP_GPA_PAGES, unmap_flags);
+
+		page_offset += MSHV_MAX_UNMAP_GPA_PAGES - 1;
+	}
 
 	mshv_region_invalidate(region);
 



^ permalink raw reply related

* [PATCH v3 2/5] Drivers: hv: Centralize guest memory region destruction
From: Stanislav Kinsburskii @ 2025-10-02 16:35 UTC (permalink / raw)
  To: kys, haiyangz, wei.liu, decui; +Cc: linux-hyperv, linux-kernel
In-Reply-To: <175942263069.128138.16103948906881178993.stgit@skinsburskii-cloud-desktop.internal.cloudapp.net>

Centralize guest memory region destruction to prevent resource leaks and
inconsistent cleanup across unmap and partition destruction paths.

Unify region removal, encrypted partition access recovery, and region
invalidation to improve maintainability and reliability. Reduce code
duplication and make future updates less error-prone by encapsulating
cleanup logic in a single helper.

Signed-off-by: Stanislav Kinsburskii <skinsburskii@linux.microsoft.com>
---
 drivers/hv/mshv_root_main.c |   65 ++++++++++++++++++++++---------------------
 1 file changed, 34 insertions(+), 31 deletions(-)

diff --git a/drivers/hv/mshv_root_main.c b/drivers/hv/mshv_root_main.c
index 29d0c2c9ae4c8..7ef66c43e1515 100644
--- a/drivers/hv/mshv_root_main.c
+++ b/drivers/hv/mshv_root_main.c
@@ -1375,13 +1375,42 @@ mshv_map_user_memory(struct mshv_partition *partition,
 	return ret;
 }
 
+static void mshv_partition_destroy_region(struct mshv_mem_region *region)
+{
+	struct mshv_partition *partition = region->partition;
+	u32 unmap_flags = 0;
+	int ret;
+
+	hlist_del(&region->hnode);
+
+	if (mshv_partition_encrypted(partition)) {
+		ret = mshv_partition_region_share(region);
+		if (ret) {
+			pt_err(partition,
+			       "Failed to regain access to memory, unpinning user pages will fail and crash the host error: %d\n",
+			       ret);
+			return;
+		}
+	}
+
+	if (region->flags.large_pages)
+		unmap_flags |= HV_UNMAP_GPA_LARGE_PAGE;
+
+	/* ignore unmap failures and continue as process may be exiting */
+	hv_call_unmap_gpa_pages(partition->pt_id, region->start_gfn,
+				region->nr_pages, unmap_flags);
+
+	mshv_region_invalidate(region);
+
+	vfree(region);
+}
+
 /* Called for unmapping both the guest ram and the mmio space */
 static long
 mshv_unmap_user_memory(struct mshv_partition *partition,
 		       struct mshv_user_mem_region mem)
 {
 	struct mshv_mem_region *region;
-	u32 unmap_flags = 0;
 
 	if (!(mem.flags & BIT(MSHV_SET_MEM_BIT_UNMAP)))
 		return -EINVAL;
@@ -1396,18 +1425,8 @@ mshv_unmap_user_memory(struct mshv_partition *partition,
 	    region->nr_pages != HVPFN_DOWN(mem.size))
 		return -EINVAL;
 
-	hlist_del(&region->hnode);
+	mshv_partition_destroy_region(region);
 
-	if (region->flags.large_pages)
-		unmap_flags |= HV_UNMAP_GPA_LARGE_PAGE;
-
-	/* ignore unmap failures and continue as process may be exiting */
-	hv_call_unmap_gpa_pages(partition->pt_id, region->start_gfn,
-				region->nr_pages, unmap_flags);
-
-	mshv_region_invalidate(region);
-
-	vfree(region);
 	return 0;
 }
 
@@ -1743,8 +1762,8 @@ static void destroy_partition(struct mshv_partition *partition)
 {
 	struct mshv_vp *vp;
 	struct mshv_mem_region *region;
-	int i, ret;
 	struct hlist_node *n;
+	int i;
 
 	if (refcount_read(&partition->pt_ref_count)) {
 		pt_err(partition,
@@ -1804,25 +1823,9 @@ static void destroy_partition(struct mshv_partition *partition)
 
 	remove_partition(partition);
 
-	/* Remove regions, regain access to the memory and unpin the pages */
 	hlist_for_each_entry_safe(region, n, &partition->pt_mem_regions,
-				  hnode) {
-		hlist_del(&region->hnode);
-
-		if (mshv_partition_encrypted(partition)) {
-			ret = mshv_partition_region_share(region);
-			if (ret) {
-				pt_err(partition,
-				       "Failed to regain access to memory, unpinning user pages will fail and crash the host error: %d\n",
-				      ret);
-				return;
-			}
-		}
-
-		mshv_region_invalidate(region);
-
-		vfree(region);
-	}
+				  hnode)
+		mshv_partition_destroy_region(region);
 
 	/* Withdraw and free all pages we deposited */
 	hv_call_withdraw_memory(U64_MAX, NUMA_NO_NODE, partition->pt_id);



^ permalink raw reply related

* [PATCH v3 1/5] Drivers: hv: Refactor and rename memory region handling functions
From: Stanislav Kinsburskii @ 2025-10-02 16:35 UTC (permalink / raw)
  To: kys, haiyangz, wei.liu, decui; +Cc: linux-hyperv, linux-kernel
In-Reply-To: <175942263069.128138.16103948906881178993.stgit@skinsburskii-cloud-desktop.internal.cloudapp.net>

Simplify and unify memory region management to improve code clarity and
reliability. Consolidate pinning and invalidation logic, adopt consistent
naming, and remove redundant checks to reduce complexity.

Enhance documentation and update call sites for maintainability.

Signed-off-by: Stanislav Kinsburskii <skinsburskii@linux.microsoft.com>
---
 drivers/hv/mshv_root_main.c |   78 +++++++++++++++++++------------------------
 1 file changed, 35 insertions(+), 43 deletions(-)

diff --git a/drivers/hv/mshv_root_main.c b/drivers/hv/mshv_root_main.c
index fa42c40e1e02f..29d0c2c9ae4c8 100644
--- a/drivers/hv/mshv_root_main.c
+++ b/drivers/hv/mshv_root_main.c
@@ -1120,8 +1120,8 @@ mshv_region_map(struct mshv_mem_region *region)
 }
 
 static void
-mshv_region_evict_pages(struct mshv_mem_region *region,
-			u64 page_offset, u64 page_count)
+mshv_region_invalidate_pages(struct mshv_mem_region *region,
+			     u64 page_offset, u64 page_count)
 {
 	if (region->flags.range_pinned)
 		unpin_user_pages(region->pages + page_offset, page_count);
@@ -1131,29 +1131,24 @@ mshv_region_evict_pages(struct mshv_mem_region *region,
 }
 
 static void
-mshv_region_evict(struct mshv_mem_region *region)
+mshv_region_invalidate(struct mshv_mem_region *region)
 {
-	mshv_region_evict_pages(region, 0, region->nr_pages);
+	mshv_region_invalidate_pages(region, 0, region->nr_pages);
 }
 
 static int
-mshv_region_populate_pages(struct mshv_mem_region *region,
-			   u64 page_offset, u64 page_count)
+mshv_region_pin(struct mshv_mem_region *region)
 {
 	u64 done_count, nr_pages;
 	struct page **pages;
 	__u64 userspace_addr;
 	int ret;
 
-	if (page_offset + page_count > region->nr_pages)
-		return -EINVAL;
-
-	for (done_count = 0; done_count < page_count; done_count += ret) {
-		pages = region->pages + page_offset + done_count;
+	for (done_count = 0; done_count < region->nr_pages; done_count += ret) {
+		pages = region->pages + done_count;
 		userspace_addr = region->start_uaddr +
-				(page_offset + done_count) *
-				HV_HYP_PAGE_SIZE;
-		nr_pages = min(page_count - done_count,
+				 done_count * HV_HYP_PAGE_SIZE;
+		nr_pages = min(region->nr_pages - done_count,
 			       MSHV_PIN_PAGES_BATCH_SIZE);
 
 		/*
@@ -1164,34 +1159,23 @@ mshv_region_populate_pages(struct mshv_mem_region *region,
 		 * with the FOLL_LONGTERM flag does a large temporary
 		 * allocation of contiguous memory.
 		 */
-		if (region->flags.range_pinned)
-			ret = pin_user_pages_fast(userspace_addr,
-						  nr_pages,
-						  FOLL_WRITE | FOLL_LONGTERM,
-						  pages);
-		else
-			ret = -EOPNOTSUPP;
-
+		ret = pin_user_pages_fast(userspace_addr, nr_pages,
+					  FOLL_WRITE | FOLL_LONGTERM,
+					  pages);
 		if (ret < 0)
 			goto release_pages;
 	}
 
-	if (PageHuge(region->pages[page_offset]))
+	if (PageHuge(region->pages[0]))
 		region->flags.large_pages = true;
 
 	return 0;
 
 release_pages:
-	mshv_region_evict_pages(region, page_offset, done_count);
+	mshv_region_invalidate_pages(region, 0, done_count);
 	return ret;
 }
 
-static int
-mshv_region_populate(struct mshv_mem_region *region)
-{
-	return mshv_region_populate_pages(region, 0, region->nr_pages);
-}
-
 static struct mshv_mem_region *
 mshv_partition_region_by_gfn(struct mshv_partition *partition, u64 gfn)
 {
@@ -1264,17 +1248,25 @@ static int mshv_partition_create_region(struct mshv_partition *partition,
 	return 0;
 }
 
-/*
- * Map guest ram. if snp, make sure to release that from the host first
- * Side Effects: In case of failure, pages are unpinned when feasible.
+/**
+ * mshv_prepare_pinned_region - Pin and map memory regions
+ * @region: Pointer to the memory region structure
+ *
+ * This function processes memory regions that are explicitly marked as pinned.
+ * Pinned regions are preallocated, mapped upfront, and do not rely on fault-based
+ * population. The function ensures the region is properly populated, handles
+ * encryption requirements for SNP partitions if applicable, maps the region,
+ * and performs necessary sharing or eviction operations based on the mapping
+ * result.
+ *
+ * Return: 0 on success, negative error code on failure.
  */
-static int
-mshv_partition_mem_region_map(struct mshv_mem_region *region)
+static int mshv_prepare_pinned_region(struct mshv_mem_region *region)
 {
 	struct mshv_partition *partition = region->partition;
 	int ret;
 
-	ret = mshv_region_populate(region);
+	ret = mshv_region_pin(region);
 	if (ret) {
 		pt_err(partition, "Failed to populate memory region: %d\n",
 		       ret);
@@ -1294,7 +1286,7 @@ mshv_partition_mem_region_map(struct mshv_mem_region *region)
 			pt_err(partition,
 			       "Failed to unshare memory region (guest_pfn: %llu): %d\n",
 			       region->start_gfn, ret);
-			goto evict_region;
+			goto invalidate_region;
 		}
 	}
 
@@ -1304,7 +1296,7 @@ mshv_partition_mem_region_map(struct mshv_mem_region *region)
 
 		shrc = mshv_partition_region_share(region);
 		if (!shrc)
-			goto evict_region;
+			goto invalidate_region;
 
 		pt_err(partition,
 		       "Failed to share memory region (guest_pfn: %llu): %d\n",
@@ -1318,8 +1310,8 @@ mshv_partition_mem_region_map(struct mshv_mem_region *region)
 
 	return 0;
 
-evict_region:
-	mshv_region_evict(region);
+invalidate_region:
+	mshv_region_invalidate(region);
 err_out:
 	return ret;
 }
@@ -1368,7 +1360,7 @@ mshv_map_user_memory(struct mshv_partition *partition,
 		ret = hv_call_map_mmio_pages(partition->pt_id, mem.guest_pfn,
 					     mmio_pfn, HVPFN_DOWN(mem.size));
 	else
-		ret = mshv_partition_mem_region_map(region);
+		ret = mshv_prepare_pinned_region(region);
 
 	if (ret)
 		goto errout;
@@ -1413,7 +1405,7 @@ mshv_unmap_user_memory(struct mshv_partition *partition,
 	hv_call_unmap_gpa_pages(partition->pt_id, region->start_gfn,
 				region->nr_pages, unmap_flags);
 
-	mshv_region_evict(region);
+	mshv_region_invalidate(region);
 
 	vfree(region);
 	return 0;
@@ -1827,7 +1819,7 @@ static void destroy_partition(struct mshv_partition *partition)
 			}
 		}
 
-		mshv_region_evict(region);
+		mshv_region_invalidate(region);
 
 		vfree(region);
 	}



^ permalink raw reply related

* [PATCH v3 0/5] Introduce movable pages for Hyper-V guests
From: Stanislav Kinsburskii @ 2025-10-02 16:35 UTC (permalink / raw)
  To: kys, haiyangz, wei.liu, decui; +Cc: linux-hyperv, linux-kernel

From the start, the root-partition driver allocates, pins, and maps all
guest memory into the hypervisor at guest creation. This is simple: Linux
cannot move the pages, so the guest’s view in Linux and in Microsoft
Hypervisor never diverges.

However, this approach has major drawbacks:
- NUMA: affinity can’t be changed at runtime, so you can’t migrate guest memory closer to the CPUs running it → performance hit.
- Memory management: unused guest memory can’t be swapped out, compacted, or merged.
- Provisioning time: upfront allocation/pinning slows guest create/destroy.
- Overcommit: no memory overcommit on hosts with pinned-guest memory.

This series adds movable memory pages for Hyper-V child partitions. Guest
pages are no longer allocated upfront; they’re allocated and mapped into
the hypervisor on demand (i.e., when the guest touches a GFN that isn’t yet
backed by a host PFN).
When a page is moved, Linux no longer holds it and it is unmapped from the hypervisor.
As a result, Hyper-V guests behave like regular Linux processes, enabling standard Linux memory features to apply to guests.

Exceptions (still pinned):
  1. Encrypted guests (explicit).
  2. Guests with passthrough devices (implicitly pinned by the VFIO framework).

v3:
- Region is invalidated even if the mm has no users.
- Page remapping logic is updated to support 2M-unaligned remappings for
  regions that are PMD-aligned, which can occur during both faults and
  invalidations.

v2:
- Split unmap batching into a separate patch.
- Fixed commit messages from v1 review.
- Renamed a few functions for clarity.

---

Stanislav Kinsburskii (5):
      Drivers: hv: Refactor and rename memory region handling functions
      Drivers: hv: Centralize guest memory region destruction
      Drivers: hv: Batch GPA unmap operations to improve large region performance
      Drivers: hv: Ensure large page GPA mapping is PMD-aligned
      Drivers: hv: Add support for movable memory regions


 drivers/hv/Kconfig             |    1 
 drivers/hv/mshv_root.h         |   10 +
 drivers/hv/mshv_root_hv_call.c |    2 
 drivers/hv/mshv_root_main.c    |  480 +++++++++++++++++++++++++++++++++-------
 4 files changed, 410 insertions(+), 83 deletions(-)


^ permalink raw reply

* Re: [PATCH hyperv-next] hyperv: Remove the spurious null directive line
From: Easwar Hariharan @ 2025-10-02 16:04 UTC (permalink / raw)
  To: Roman Kisel
  Cc: decui, eahariha, haiyangz, kys, mhklinux, nunodasneves, wei.liu,
	linux-hyperv, linux-kernel, easwar.hariharan, benhill, bperkins,
	sunilmut
In-Reply-To: <20251001230847.5002-1-romank@linux.microsoft.com>

On 10/1/2025 4:08 PM, Roman Kisel wrote:
> The file contains a line that consists of the lone # symbol
> followed by a newline. While that is a valid syntax as
> defined by the C99+ grammar (6.10.7 "Null directive"), it
> serves no apparent purpose in this case.
> 
> Remove the null preprocessor directive. No functional changes.
> 
> Fixes: e68bda71a238 ("hyperv: Add new Hyper-V headers in include/hyperv")
> Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
> ---
>  include/hyperv/hvgdk_mini.h | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/include/hyperv/hvgdk_mini.h b/include/hyperv/hvgdk_mini.h
> index 1be7f6a02304..77abddfc750e 100644
> --- a/include/hyperv/hvgdk_mini.h
> +++ b/include/hyperv/hvgdk_mini.h
> @@ -597,8 +597,6 @@ struct ms_hyperv_tsc_page {	 /* HV_REFERENCE_TSC_PAGE */
>  #define HV_SYNIC_SINT_AUTO_EOI		(1ULL << 17)
>  #define HV_SYNIC_SINT_VECTOR_MASK	(0xFF)
>  
> -#
> -
>  /* Hyper-V defined statically assigned SINTs */
>  #define HV_SYNIC_INTERCEPTION_SINT_INDEX 0x00000000
>  #define HV_SYNIC_IOMMU_FAULT_SINT_INDEX  0x00000001
> 
> base-commit: e3ec97c3abaf2fb68cc755cae3229288696b9f3d

Reviewed-by: Easwar Hariharan <easwar.hariharan@linux.microsoft.com>

^ permalink raw reply

* [PATCH] scsi: storvsc: Prefer returning channel with the same CPU as on the I/O issuing CPU
From: longli @ 2025-10-02  5:05 UTC (permalink / raw)
  To: K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
	James E.J. Bottomley, Martin K. Petersen, James Bottomley,
	linux-hyperv, linux-scsi, linux-kernel
  Cc: Long Li

From: Long Li <longli@microsoft.com>

When selecting an outgoing channel for I/O, storvsc tries to select a
channel with a returning CPU that is not the same as issuing CPU. This
worked well in the past, however it doesn't work well when the Hyper-V
exposes a large number of channels (up to the number of all CPUs). Use
a different CPU for returning channel is not efficient on Hyper-V.

Change this behavior by preferring to the channel with the same CPU
as the current I/O issuing CPU whenever possible.

Tests have shown improvements in newer Hyper-V/Azure environment, and
no regression with older Hyper-V/Azure environments.

Tested-by: Raheel Abdul Faizy <rabdulfaizy@microsoft.com>
Signed-off-by: Long Li <longli@microsoft.com>
---
 drivers/scsi/storvsc_drv.c | 96 ++++++++++++++++++--------------------
 1 file changed, 45 insertions(+), 51 deletions(-)

diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index d9e59204a9c3..092939791ea0 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -1406,14 +1406,19 @@ static struct vmbus_channel *get_og_chn(struct storvsc_device *stor_device,
 	}
 
 	/*
-	 * Our channel array is sparsley populated and we
+	 * Our channel array could be sparsley populated and we
 	 * initiated I/O on a processor/hw-q that does not
 	 * currently have a designated channel. Fix this.
 	 * The strategy is simple:
-	 * I. Ensure NUMA locality
-	 * II. Distribute evenly (best effort)
+	 * I. Prefer the channel associated with the current CPU
+	 * II. Ensure NUMA locality
+	 * III. Distribute evenly (best effort)
 	 */
 
+	/* Prefer the channel on the I/O issuing processor/hw-q */
+	if (cpumask_test_cpu(q_num, &stor_device->alloced_cpus))
+		return stor_device->stor_chns[q_num];
+
 	node_mask = cpumask_of_node(cpu_to_node(q_num));
 
 	num_channels = 0;
@@ -1469,59 +1474,48 @@ static int storvsc_do_io(struct hv_device *device,
 	/* See storvsc_change_target_cpu(). */
 	outgoing_channel = READ_ONCE(stor_device->stor_chns[q_num]);
 	if (outgoing_channel != NULL) {
-		if (outgoing_channel->target_cpu == q_num) {
-			/*
-			 * Ideally, we want to pick a different channel if
-			 * available on the same NUMA node.
-			 */
-			node_mask = cpumask_of_node(cpu_to_node(q_num));
-			for_each_cpu_wrap(tgt_cpu,
-				 &stor_device->alloced_cpus, q_num + 1) {
-				if (!cpumask_test_cpu(tgt_cpu, node_mask))
-					continue;
-				if (tgt_cpu == q_num)
-					continue;
-				channel = READ_ONCE(
-					stor_device->stor_chns[tgt_cpu]);
-				if (channel == NULL)
-					continue;
-				if (hv_get_avail_to_write_percent(
-							&channel->outbound)
-						> ring_avail_percent_lowater) {
-					outgoing_channel = channel;
-					goto found_channel;
-				}
-			}
+		if (hv_get_avail_to_write_percent(&outgoing_channel->outbound)
+				> ring_avail_percent_lowater)
+			goto found_channel;
 
-			/*
-			 * All the other channels on the same NUMA node are
-			 * busy. Try to use the channel on the current CPU
-			 */
-			if (hv_get_avail_to_write_percent(
-						&outgoing_channel->outbound)
-					> ring_avail_percent_lowater)
+		/*
+		 * Channel is busy, try to find a channel on the same NUMA node
+		 */
+		node_mask = cpumask_of_node(cpu_to_node(q_num));
+		for_each_cpu_wrap(tgt_cpu, &stor_device->alloced_cpus,
+				  q_num + 1) {
+			if (!cpumask_test_cpu(tgt_cpu, node_mask))
+				continue;
+			channel = READ_ONCE(stor_device->stor_chns[tgt_cpu]);
+			if (!channel)
+				continue;
+			if (hv_get_avail_to_write_percent(&channel->outbound)
+					> ring_avail_percent_lowater) {
+				outgoing_channel = channel;
 				goto found_channel;
+			}
+		}
 
-			/*
-			 * If we reach here, all the channels on the current
-			 * NUMA node are busy. Try to find a channel in
-			 * other NUMA nodes
-			 */
-			for_each_cpu(tgt_cpu, &stor_device->alloced_cpus) {
-				if (cpumask_test_cpu(tgt_cpu, node_mask))
-					continue;
-				channel = READ_ONCE(
-					stor_device->stor_chns[tgt_cpu]);
-				if (channel == NULL)
-					continue;
-				if (hv_get_avail_to_write_percent(
-							&channel->outbound)
-						> ring_avail_percent_lowater) {
-					outgoing_channel = channel;
-					goto found_channel;
-				}
+		/*
+		 * If we reach here, all the channels on the current
+		 * NUMA node are busy. Try to find a channel in
+		 * all NUMA nodes
+		 */
+		for_each_cpu_wrap(tgt_cpu, &stor_device->alloced_cpus,
+				  q_num + 1) {
+			channel = READ_ONCE(stor_device->stor_chns[tgt_cpu]);
+			if (!channel)
+				continue;
+			if (hv_get_avail_to_write_percent(&channel->outbound)
+					> ring_avail_percent_lowater) {
+				outgoing_channel = channel;
+				goto found_channel;
 			}
 		}
+		/*
+		 * If we reach here, all the channels are busy. Use the
+		 * original channel found.
+		 */
 	} else {
 		spin_lock_irqsave(&stor_device->lock, flags);
 		outgoing_channel = stor_device->stor_chns[q_num];
-- 
2.34.1


^ permalink raw reply related

* RE: [PATCH v4 5/5] mshv: Introduce new hypercall to map stats page for L1VH partitions
From: Michael Kelley @ 2025-10-02  0:16 UTC (permalink / raw)
  To: Nuno Das Neves, linux-hyperv@vger.kernel.org,
	linux-kernel@vger.kernel.org, prapal@linux.microsoft.com,
	easwar.hariharan@linux.microsoft.com, tiala@microsoft.com,
	anirudh@anirudhrb.com, paekkaladevi@linux.microsoft.com,
	skinsburskii@linux.microsoft.com
  Cc: kys@microsoft.com, haiyangz@microsoft.com, wei.liu@kernel.org,
	decui@microsoft.com, Jinank Jain
In-Reply-To: <33868cb1-4cb1-40bb-a773-8f15ec75f679@linux.microsoft.com>

From: Nuno Das Neves <nunodasneves@linux.microsoft.com> Sent: Wednesday, October 1, 2025 4:03 PM
> 
> On 9/29/2025 10:57 AM, Michael Kelley wrote:
> > From: Nuno Das Neves <nunodasneves@linux.microsoft.com> Sent: Friday, September 26, 2025 9:23 AM
> >>
> >> From: Jinank Jain <jinankjain@linux.microsoft.com>
> >>

[snip]

> >> diff --git a/drivers/hv/mshv_root_hv_call.c b/drivers/hv/mshv_root_hv_call.c
> >> index 98c6278ff151..5a805b3dec0b 100644
> >> --- a/drivers/hv/mshv_root_hv_call.c
> >> +++ b/drivers/hv/mshv_root_hv_call.c
> >> @@ -804,9 +804,51 @@ hv_call_notify_port_ring_empty(u32 sint_index)
> >>  	return hv_result_to_errno(status);
> >>  }
> >>
> >> -int hv_call_map_stat_page(enum hv_stats_object_type type,
> >> -			  const union hv_stats_object_identity *identity,
> >> -			  void **addr)
> >> +static int hv_call_map_stats_page2(enum hv_stats_object_type type,
> >> +				   const union hv_stats_object_identity *identity,
> >> +				   u64 map_location)
> >> +{
> >> +	unsigned long flags;
> >> +	struct hv_input_map_stats_page2 *input;
> >> +	u64 status;
> >> +	int ret;
> >> +
> >> +	if (!map_location || !mshv_use_overlay_gpfn())
> >> +		return -EINVAL;
> >> +
> >> +	do {
> >> +		local_irq_save(flags);
> >> +		input = *this_cpu_ptr(hyperv_pcpu_input_arg);
> >> +
> >> +		memset(input, 0, sizeof(*input));
> >> +		input->type = type;
> >> +		input->identity = *identity;
> >> +		input->map_location = map_location;
> >> +
> >> +		status = hv_do_hypercall(HVCALL_MAP_STATS_PAGE2, input, NULL);
> >> +
> >> +		local_irq_restore(flags);
> >> +
> >> +		ret = hv_result_to_errno(status);
> >> +
> >> +		if (!ret)
> >> +			break;
> >
> > This logic is incorrect. If the hypercall returns
> > HV_STATUS_INSUFFICIENT_MEMORY, then errno -ENOMEM is immediately
> > returned to the caller without any opportunity to do hv_call_deposit_pages().
> >
> The loop breaks if (!ret), i.e. on success. Maybe you misread it as `if (ret)`?

Pfft!  Indeed I had it wrong.

I'll note that hv_call_map_stats_page2() and the existing hv_call_map_stats_page()
have different code for sorting out success vs. insufficient memory vs. other errors.
And there are additional code variations in other MSHV places that must check for
insufficient memory. Some consistency might be nice, but that's out of scope
for this patch set.

> >> +
> >> +		if (hv_result(status) != HV_STATUS_INSUFFICIENT_MEMORY) {
> >> +			hv_status_debug(status, "\n");
> >> +			break;
> >> +		}
> >> +
> >> +		ret = hv_call_deposit_pages(NUMA_NO_NODE,
> >> +					    hv_current_partition_id, 1);
> >> +	} while (!ret);
> >> +
> >> +	return ret;
> >> +}
> >> +
> >> +static int hv_call_map_stats_page(enum hv_stats_object_type type,
> >> +				  const union hv_stats_object_identity *identity,
> >> +				  void **addr)
> >>  {
> >>  	unsigned long flags;
> >>  	struct hv_input_map_stats_page *input;
> >> @@ -845,8 +887,36 @@ int hv_call_map_stat_page(enum hv_stats_object_type type,
> >>  	return ret;
> >>  }
> >>
> >> -int hv_call_unmap_stat_page(enum hv_stats_object_type type,
> >> -			    const union hv_stats_object_identity *identity)
> >> +int hv_map_stats_page(enum hv_stats_object_type type,
> >> +		      const union hv_stats_object_identity *identity,
> >> +		      void **addr)
> >> +{
> >> +	int ret;
> >> +	struct page *allocated_page = NULL;
> >> +
> >> +	if (!addr)
> >> +		return -EINVAL;
> >> +
> >> +	if (mshv_use_overlay_gpfn()) {
> >> +		allocated_page = alloc_page(GFP_KERNEL);
> >> +		if (!allocated_page)
> >> +			return -ENOMEM;
> >> +
> >> +		ret = hv_call_map_stats_page2(type, identity,
> >> +					      page_to_pfn(allocated_page));
> >
> > This should use page_to_hvpfn() per my comments in Patch 4 of this series.
> >
> I could change it, but as mentioned in my reply to Patch 4 I'm not sure it's
> worth doing without addressing the whole issue which is a much bigger ask.

Agreed.

Michael

> 
> >> +		*addr = page_address(allocated_page);
> >> +	} else {
> >> +		ret = hv_call_map_stats_page(type, identity, addr);
> >> +	}
> >> +
> >> +	if (ret && allocated_page)
> >> +		__free_page(allocated_page);
> >
> > Might want to do *addr = NULL after freeing the page so that the caller
> > can't erroneously reference the free page. Again, the current caller doesn't
> > do that, so current code isn't broken.
> >
> Yep, I can add it to provide a little more robustness against bugs in callers.
> 
> >> +
> >> +	return ret;
> >> +}
> >> +
> >> +static int hv_call_unmap_stats_page(enum hv_stats_object_type type,
> >> +				    const union hv_stats_object_identity *identity)
> >>  {
> >>  	unsigned long flags;
> >>  	struct hv_input_unmap_stats_page *input;
> >> @@ -865,6 +935,19 @@ int hv_call_unmap_stat_page(enum hv_stats_object_type
> type,
> >>  	return hv_result_to_errno(status);
> >>  }
> >>
> >> +int hv_unmap_stats_page(enum hv_stats_object_type type, void *page_addr,
> >> +			const union hv_stats_object_identity *identity)
> >> +{
> >> +	int ret;
> >> +
> >> +	ret = hv_call_unmap_stats_page(type, identity);
> >> +
> >> +	if (mshv_use_overlay_gpfn() && page_addr)
> >> +		__free_page(virt_to_page(page_addr));
> >> +
> >> +	return ret;
> >> +}
> >> +
> >>  int hv_call_modify_spa_host_access(u64 partition_id, struct page **pages,
> >>  				   u64 page_struct_count, u32 host_access,
> >>  				   u32 flags, u8 acquire)
> >> diff --git a/drivers/hv/mshv_root_main.c b/drivers/hv/mshv_root_main.c
> >> index 2d0ad17acde6..71a8ab5db3b8 100644
> >> --- a/drivers/hv/mshv_root_main.c
> >> +++ b/drivers/hv/mshv_root_main.c
> >> @@ -841,7 +841,8 @@ mshv_vp_release(struct inode *inode, struct file *filp)
> >>  	return 0;
> >>  }
> >>
> >> -static void mshv_vp_stats_unmap(u64 partition_id, u32 vp_index)
> >> +static void mshv_vp_stats_unmap(u64 partition_id, u32 vp_index,
> >> +				void *stats_pages[])
> >>  {
> >>  	union hv_stats_object_identity identity = {
> >>  		.vp.partition_id = partition_id,
> >> @@ -849,10 +850,10 @@ static void mshv_vp_stats_unmap(u64 partition_id, u32
> vp_index)
> >>  	};
> >>
> >>  	identity.vp.stats_area_type = HV_STATS_AREA_SELF;
> >> -	hv_call_unmap_stat_page(HV_STATS_OBJECT_VP, &identity);
> >> +	hv_unmap_stats_page(HV_STATS_OBJECT_VP, NULL, &identity);
> >>
> >>  	identity.vp.stats_area_type = HV_STATS_AREA_PARENT;
> >> -	hv_call_unmap_stat_page(HV_STATS_OBJECT_VP, &identity);
> >> +	hv_unmap_stats_page(HV_STATS_OBJECT_VP, NULL, &identity);
> >>  }
> >>
> >>  static int mshv_vp_stats_map(u64 partition_id, u32 vp_index,
> >> @@ -865,14 +866,14 @@ static int mshv_vp_stats_map(u64 partition_id, u32
> vp_index,
> >>  	int err;
> >>
> >>  	identity.vp.stats_area_type = HV_STATS_AREA_SELF;
> >> -	err = hv_call_map_stat_page(HV_STATS_OBJECT_VP, &identity,
> >> -				    &stats_pages[HV_STATS_AREA_SELF]);
> >> +	err = hv_map_stats_page(HV_STATS_OBJECT_VP, &identity,
> >> +				&stats_pages[HV_STATS_AREA_SELF]);
> >>  	if (err)
> >>  		return err;
> >>
> >>  	identity.vp.stats_area_type = HV_STATS_AREA_PARENT;
> >> -	err = hv_call_map_stat_page(HV_STATS_OBJECT_VP, &identity,
> >> -				    &stats_pages[HV_STATS_AREA_PARENT]);
> >> +	err = hv_map_stats_page(HV_STATS_OBJECT_VP, &identity,
> >> +				&stats_pages[HV_STATS_AREA_PARENT]);
> >>  	if (err)
> >>  		goto unmap_self;
> >>
> >> @@ -880,7 +881,7 @@ static int mshv_vp_stats_map(u64 partition_id, u32
> vp_index,
> >>
> >>  unmap_self:
> >>  	identity.vp.stats_area_type = HV_STATS_AREA_SELF;
> >> -	hv_call_unmap_stat_page(HV_STATS_OBJECT_VP, &identity);
> >> +	hv_unmap_stats_page(HV_STATS_OBJECT_VP, NULL, &identity);
> >>  	return err;
> >>  }
> >>
> >> @@ -988,7 +989,7 @@ mshv_partition_ioctl_create_vp(struct mshv_partition
> *partition,
> >>  	kfree(vp);
> >>  unmap_stats_pages:
> >>  	if (hv_scheduler_type == HV_SCHEDULER_TYPE_ROOT)
> >> -		mshv_vp_stats_unmap(partition->pt_id, args.vp_index);
> >> +		mshv_vp_stats_unmap(partition->pt_id, args.vp_index, stats_pages);
> >>  unmap_ghcb_page:
> >>  	if (mshv_partition_encrypted(partition) && is_ghcb_mapping_available())
> >>  		hv_unmap_vp_state_page(partition->pt_id, args.vp_index,
> >> @@ -1740,7 +1741,8 @@ static void destroy_partition(struct mshv_partition
> *partition)
> >>  				continue;
> >>
> >>  			if (hv_scheduler_type == HV_SCHEDULER_TYPE_ROOT)
> >> -				mshv_vp_stats_unmap(partition->pt_id, vp-
> >vp_index);
> >> +				mshv_vp_stats_unmap(partition->pt_id, vp->vp_index,
> >> +						    (void **)vp->vp_stats_pages);
> >>
> >>  			if (vp->vp_register_page) {
> >>  				(void)hv_unmap_vp_state_page(partition->pt_id,
> >> diff --git a/include/hyperv/hvgdk_mini.h b/include/hyperv/hvgdk_mini.h
> >> index ff4325fb623a..f66565106d21 100644
> >> --- a/include/hyperv/hvgdk_mini.h
> >> +++ b/include/hyperv/hvgdk_mini.h
> >> @@ -493,6 +493,7 @@ union hv_vp_assist_msr_contents {	 /*
> >> HV_REGISTER_VP_ASSIST_PAGE */
> >>  #define HVCALL_GET_PARTITION_PROPERTY_EX		0x0101
> >>  #define HVCALL_MMIO_READ				0x0106
> >>  #define HVCALL_MMIO_WRITE				0x0107
> >> +#define HVCALL_MAP_STATS_PAGE2				0x0131
> >>
> >>  /* HV_HYPERCALL_INPUT */
> >>  #define HV_HYPERCALL_RESULT_MASK	GENMASK_ULL(15, 0)
> >> diff --git a/include/hyperv/hvhdk_mini.h b/include/hyperv/hvhdk_mini.h
> >> index bf2ce27dfcc5..064bf735cab6 100644
> >> --- a/include/hyperv/hvhdk_mini.h
> >> +++ b/include/hyperv/hvhdk_mini.h
> >> @@ -177,6 +177,13 @@ struct hv_input_map_stats_page {
> >>  	union hv_stats_object_identity identity;
> >>  } __packed;
> >>
> >> +struct hv_input_map_stats_page2 {
> >> +	u32 type; /* enum hv_stats_object_type */
> >> +	u32 padding;
> >> +	union hv_stats_object_identity identity;
> >> +	u64 map_location;
> >> +} __packed;
> >> +
> >>  struct hv_output_map_stats_page {
> >>  	u64 map_location;
> >>  } __packed;
> >> --
> >> 2.34.1
> >>


^ permalink raw reply

* RE: [PATCH v4 4/5] mshv: Allocate vp state page for HVCALL_MAP_VP_STATE_PAGE on L1VH
From: Michael Kelley @ 2025-10-02  0:02 UTC (permalink / raw)
  To: Nuno Das Neves, linux-hyperv@vger.kernel.org,
	linux-kernel@vger.kernel.org, prapal@linux.microsoft.com,
	easwar.hariharan@linux.microsoft.com, tiala@microsoft.com,
	anirudh@anirudhrb.com, paekkaladevi@linux.microsoft.com,
	skinsburskii@linux.microsoft.com
  Cc: kys@microsoft.com, haiyangz@microsoft.com, wei.liu@kernel.org,
	decui@microsoft.com, Jinank Jain
In-Reply-To: <8bacde85-6406-4b47-b11d-ed5054b270f2@linux.microsoft.com>

From: Nuno Das Neves <nunodasneves@linux.microsoft.com> Sent: Wednesday, October 1, 2025 3:56 PM
> 
> On 9/29/2025 10:56 AM, Michael Kelley wrote:
> > From: Nuno Das Neves <nunodasneves@linux.microsoft.com> Sent: Friday, September 26, 2025 9:23 AM
> >>
> >> From: Jinank Jain <jinankjain@linux.microsoft.com>
> >>
> >> Introduce mshv_use_overlay_gpfn() to check if a page needs to be
> >> allocated and passed to the hypervisor to map VP state pages. This is
> >> only needed on L1VH, and only on some (newer) versions of the
> >> hypervisor, hence the need to check vmm_capabilities.
> >>
> >> Introduce functions hv_map/unmap_vp_state_page() to handle the
> >> allocation and freeing.
> >>
> >> Signed-off-by: Jinank Jain <jinankjain@linux.microsoft.com>
> >> Signed-off-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>
> >> Reviewed-by: Praveen K Paladugu <prapal@linux.microsoft.com>
> >> Reviewed-by: Easwar Hariharan <easwar.hariharan@linux.microsoft.com>
> >> Reviewed-by: Stanislav Kinsburskii <skinsburskii@linux.microsoft.com>
> >> Reviewed-by: Anirudh Rayabharam <anirudh@anirudhrb.com>
> >> ---
> >>  drivers/hv/mshv_root.h         | 11 ++---
> >>  drivers/hv/mshv_root_hv_call.c | 61 ++++++++++++++++++++++++---
> >>  drivers/hv/mshv_root_main.c    | 76 +++++++++++++++++-----------------
> >>  3 files changed, 98 insertions(+), 50 deletions(-)
> >>
> >> diff --git a/drivers/hv/mshv_root.h b/drivers/hv/mshv_root.h
> >> index 0cb1e2589fe1..dbe2d1d0b22f 100644
> >> --- a/drivers/hv/mshv_root.h
> >> +++ b/drivers/hv/mshv_root.h
> >> @@ -279,11 +279,12 @@ int hv_call_set_vp_state(u32 vp_index, u64 partition_id,
> >>  			 /* Choose between pages and bytes */
> >>  			 struct hv_vp_state_data state_data, u64 page_count,
> >>  			 struct page **pages, u32 num_bytes, u8 *bytes);
> >> -int hv_call_map_vp_state_page(u64 partition_id, u32 vp_index, u32 type,
> >> -			      union hv_input_vtl input_vtl,
> >> -			      struct page **state_page);
> >> -int hv_call_unmap_vp_state_page(u64 partition_id, u32 vp_index, u32 type,
> >> -				union hv_input_vtl input_vtl);
> >> +int hv_map_vp_state_page(u64 partition_id, u32 vp_index, u32 type,
> >> +			 union hv_input_vtl input_vtl,
> >> +			 struct page **state_page);
> >> +int hv_unmap_vp_state_page(u64 partition_id, u32 vp_index, u32 type,
> >> +			   struct page *state_page,
> >> +			   union hv_input_vtl input_vtl);
> >>  int hv_call_create_port(u64 port_partition_id, union hv_port_id port_id,
> >>  			u64 connection_partition_id, struct hv_port_info *port_info,
> >>  			u8 port_vtl, u8 min_connection_vtl, int node);
> >> diff --git a/drivers/hv/mshv_root_hv_call.c b/drivers/hv/mshv_root_hv_call.c
> >> index 3fd3cce23f69..98c6278ff151 100644
> >> --- a/drivers/hv/mshv_root_hv_call.c
> >> +++ b/drivers/hv/mshv_root_hv_call.c
> >> @@ -526,9 +526,9 @@ int hv_call_set_vp_state(u32 vp_index, u64 partition_id,
> >>  	return ret;
> >>  }
> >>
> >> -int hv_call_map_vp_state_page(u64 partition_id, u32 vp_index, u32 type,
> >> -			      union hv_input_vtl input_vtl,
> >> -			      struct page **state_page)
> >> +static int hv_call_map_vp_state_page(u64 partition_id, u32 vp_index, u32 type,
> >> +				     union hv_input_vtl input_vtl,
> >> +				     struct page **state_page)
> >>  {
> >>  	struct hv_input_map_vp_state_page *input;
> >>  	struct hv_output_map_vp_state_page *output;
> >> @@ -547,7 +547,14 @@ int hv_call_map_vp_state_page(u64 partition_id, u32
> vp_index, u32 type,
> >>  		input->type = type;
> >>  		input->input_vtl = input_vtl;
> >>
> >> -		status = hv_do_hypercall(HVCALL_MAP_VP_STATE_PAGE, input, output);
> >
> > This function must zero the input area before using it. Otherwise,
> > flags.map_location_provided is uninitialized when *state_page is NULL. It will
> > have whatever value was left by the previous user of hyperv_pcpu_input_arg,
> > potentially producing bizarre results. And there's a reserved field that won't be
> > set to zero.
> >
> Good catch, will add a memset().
> 
> >> +		if (*state_page) {
> >> +			input->flags.map_location_provided = 1;
> >> +			input->requested_map_location =
> >> +				page_to_pfn(*state_page);
> >
> > Technically, this should be page_to_hvpfn() since the PFN value is being sent to
> > Hyper-V. I know root (and L1VH?) partitions must run with the same page size
> > as the Hyper-V host, but it's better to not leave code buried here that will blow
> > up if the "same page size requirement" should ever change.
> >
> Good point...I could change these calls, but the other way doesn't work, see below.
> 
> > And after making the hypercall, there's an invocation of pfn_to_page(), which
> > should account for the same. Unfortunately, there's not an existing hvpfn_to_page()
> > function.
> >
> This seems like a tricky scenario to get right. In the root partition case, the
> hypervisor allocates the page. That pfn could be some page within a larger Linux page.
> Converting that to a Linux pfn (naively) means losing the original hvpfn since it gets
> truncated, which is no good if we want to unmap it later. Also page_address() would
> give the wrong virtual address.
> 
> In other words, we'd have to completely change how we track these pages in order to
> support this scenario, and the same goes for various other hypervisor APIs where the
> hypervisor does the allocating. I think it's out of scope to try and address that
> here, even in part, especially since we will be making assumptions about something
> that may never happen.

OK, yes the hypervisor allocating the page is a problem when Linux tracks it
as a struct page. I'll agree it's out of current scope to change this.

It makes me think about hv_synic_enable_regs() where the paravisor or hypervisor
allocates the synic_message_page and synic_event_page. But that case should work
OK with a regular guest with page size greater than 4K because the pages are tracked
based on the guest kernel virtual address, not the PFN. So hv_synic_enable_regs()
should work on ARM64 Linux guests with 64K page size and a paravisor, as well as
for my postulated root partition with page size greater than 4K.

When it matters, cases where the hypervisor or paravisor allocate pages to give
to the guest will require careful handling to ensure they work for guest page sizes
greater than 4K. That's useful information for future consideration. Thanks for the
discussion.

Michael

> 
> >> +		}
> >> +
> >> +		status = hv_do_hypercall(HVCALL_MAP_VP_STATE_PAGE, input,
> >> +					 output);
> >>
> >>  		if (hv_result(status) != HV_STATUS_INSUFFICIENT_MEMORY) {
> >>  			if (hv_result_success(status))
> >> @@ -565,8 +572,39 @@ int hv_call_map_vp_state_page(u64 partition_id, u32 vp_index, u32 type,
> >>  	return ret;
> >>  }
> >>
> >> -int hv_call_unmap_vp_state_page(u64 partition_id, u32 vp_index, u32 type,
> >> -				union hv_input_vtl input_vtl)
> >> +static bool mshv_use_overlay_gpfn(void)
> >> +{
> >> +	return hv_l1vh_partition() &&
> >> +	       mshv_root.vmm_caps.vmm_can_provide_overlay_gpfn;
> >> +}
> >> +
> >> +int hv_map_vp_state_page(u64 partition_id, u32 vp_index, u32 type,
> >> +			 union hv_input_vtl input_vtl,
> >> +			 struct page **state_page)
> >> +{
> >> +	int ret = 0;
> >> +	struct page *allocated_page = NULL;
> >> +
> >> +	if (mshv_use_overlay_gpfn()) {
> >> +		allocated_page = alloc_page(GFP_KERNEL);
> >> +		if (!allocated_page)
> >> +			return -ENOMEM;
> >> +		*state_page = allocated_page;
> >> +	} else {
> >> +		*state_page = NULL;
> >> +	}
> >> +
> >> +	ret = hv_call_map_vp_state_page(partition_id, vp_index, type, input_vtl,
> >> +					state_page);
> >> +
> >> +	if (ret && allocated_page)
> >> +		__free_page(allocated_page);
> >
> > For robustness, you might want to set *state_page = NULL here so the
> > caller doesn't have a reference to the page that has been freed. I didn't
> > see any cases where the caller incorrectly checks the returned
> > *state_page value after an error, so the current code isn't broken.
> >
> Sure, I can add it.
> 
> >> +
> >> +	return ret;
> >> +}
> >> +
> >> +static int hv_call_unmap_vp_state_page(u64 partition_id, u32 vp_index, u32 type,
> >> +				       union hv_input_vtl input_vtl)
> >>  {
> >>  	unsigned long flags;
> >>  	u64 status;
> >> @@ -590,6 +628,17 @@ int hv_call_unmap_vp_state_page(u64 partition_id, u32 vp_index, u32 type,
> >>  	return hv_result_to_errno(status);
> >>  }
> >>
> >> +int hv_unmap_vp_state_page(u64 partition_id, u32 vp_index, u32 type,
> >> +			   struct page *state_page, union hv_input_vtl input_vtl)
> >> +{
> >> +	int ret = hv_call_unmap_vp_state_page(partition_id, vp_index, type, input_vtl);
> >> +
> >> +	if (mshv_use_overlay_gpfn() && state_page)
> >> +		__free_page(state_page);
> >> +
> >> +	return ret;
> >> +}
> >> +
> >>  int hv_call_get_partition_property_ex(u64 partition_id, u64 property_code,
> >>  				      u64 arg, void *property_value,
> >>  				      size_t property_value_sz)
> >> diff --git a/drivers/hv/mshv_root_main.c b/drivers/hv/mshv_root_main.c
> >> index e199770ecdfa..2d0ad17acde6 100644
> >> --- a/drivers/hv/mshv_root_main.c
> >> +++ b/drivers/hv/mshv_root_main.c
> >> @@ -890,7 +890,7 @@ mshv_partition_ioctl_create_vp(struct mshv_partition
> >> *partition,
> >>  {
> >>  	struct mshv_create_vp args;
> >>  	struct mshv_vp *vp;
> >> -	struct page *intercept_message_page, *register_page, *ghcb_page;
> >> +	struct page *intercept_msg_page, *register_page, *ghcb_page;
> >>  	void *stats_pages[2];
> >>  	long ret;
> >>
> >> @@ -908,28 +908,25 @@ mshv_partition_ioctl_create_vp(struct mshv_partition *partition,
> >>  	if (ret)
> >>  		return ret;
> >>
> >> -	ret = hv_call_map_vp_state_page(partition->pt_id, args.vp_index,
> >> -					HV_VP_STATE_PAGE_INTERCEPT_MESSAGE,
> >> -					input_vtl_zero,
> >> -					&intercept_message_page);
> >> +	ret = hv_map_vp_state_page(partition->pt_id, args.vp_index,
> >> +				   HV_VP_STATE_PAGE_INTERCEPT_MESSAGE,
> >> +				   input_vtl_zero, &intercept_msg_page);
> >>  	if (ret)
> >>  		goto destroy_vp;
> >>
> >>  	if (!mshv_partition_encrypted(partition)) {
> >> -		ret = hv_call_map_vp_state_page(partition->pt_id, args.vp_index,
> >> -						HV_VP_STATE_PAGE_REGISTERS,
> >> -						input_vtl_zero,
> >> -						&register_page);
> >> +		ret = hv_map_vp_state_page(partition->pt_id, args.vp_index,
> >> +					   HV_VP_STATE_PAGE_REGISTERS,
> >> +					   input_vtl_zero, &register_page);
> >>  		if (ret)
> >>  			goto unmap_intercept_message_page;
> >>  	}
> >>
> >>  	if (mshv_partition_encrypted(partition) &&
> >>  	    is_ghcb_mapping_available()) {
> >> -		ret = hv_call_map_vp_state_page(partition->pt_id, args.vp_index,
> >> -						HV_VP_STATE_PAGE_GHCB,
> >> -						input_vtl_normal,
> >> -						&ghcb_page);
> >> +		ret = hv_map_vp_state_page(partition->pt_id, args.vp_index,
> >> +					   HV_VP_STATE_PAGE_GHCB,
> >> +					   input_vtl_normal, &ghcb_page);
> >>  		if (ret)
> >>  			goto unmap_register_page;
> >>  	}
> >> @@ -960,7 +957,7 @@ mshv_partition_ioctl_create_vp(struct mshv_partition *partition,
> >>  	atomic64_set(&vp->run.vp_signaled_count, 0);
> >>
> >>  	vp->vp_index = args.vp_index;
> >> -	vp->vp_intercept_msg_page = page_to_virt(intercept_message_page);
> >> +	vp->vp_intercept_msg_page = page_to_virt(intercept_msg_page);
> >>  	if (!mshv_partition_encrypted(partition))
> >>  		vp->vp_register_page = page_to_virt(register_page);
> >>
> >> @@ -993,21 +990,19 @@ mshv_partition_ioctl_create_vp(struct mshv_partition *partition,
> >>  	if (hv_scheduler_type == HV_SCHEDULER_TYPE_ROOT)
> >>  		mshv_vp_stats_unmap(partition->pt_id, args.vp_index);
> >>  unmap_ghcb_page:
> >> -	if (mshv_partition_encrypted(partition) && is_ghcb_mapping_available()) {
> >> -		hv_call_unmap_vp_state_page(partition->pt_id, args.vp_index,
> >> -					    HV_VP_STATE_PAGE_GHCB,
> >> -					    input_vtl_normal);
> >> -	}
> >> +	if (mshv_partition_encrypted(partition) && is_ghcb_mapping_available())
> >> +		hv_unmap_vp_state_page(partition->pt_id, args.vp_index,
> >> +				       HV_VP_STATE_PAGE_GHCB, ghcb_page,
> >> +				       input_vtl_normal);
> >>  unmap_register_page:
> >> -	if (!mshv_partition_encrypted(partition)) {
> >> -		hv_call_unmap_vp_state_page(partition->pt_id, args.vp_index,
> >> -					    HV_VP_STATE_PAGE_REGISTERS,
> >> -					    input_vtl_zero);
> >> -	}
> >> +	if (!mshv_partition_encrypted(partition))
> >> +		hv_unmap_vp_state_page(partition->pt_id, args.vp_index,
> >> +				       HV_VP_STATE_PAGE_REGISTERS,
> >> +				       register_page, input_vtl_zero);
> >>  unmap_intercept_message_page:
> >> -	hv_call_unmap_vp_state_page(partition->pt_id, args.vp_index,
> >> -				    HV_VP_STATE_PAGE_INTERCEPT_MESSAGE,
> >> -				    input_vtl_zero);
> >> +	hv_unmap_vp_state_page(partition->pt_id, args.vp_index,
> >> +			       HV_VP_STATE_PAGE_INTERCEPT_MESSAGE,
> >> +			       intercept_msg_page, input_vtl_zero);
> >>  destroy_vp:
> >>  	hv_call_delete_vp(partition->pt_id, args.vp_index);
> >>  	return ret;
> >> @@ -1748,24 +1743,27 @@ static void destroy_partition(struct mshv_partition *partition)
> >>  				mshv_vp_stats_unmap(partition->pt_id, vp->vp_index);
> >>
> >>  			if (vp->vp_register_page) {
> >> -				(void)hv_call_unmap_vp_state_page(partition->pt_id,
> >> -								  vp->vp_index,
> >> -								  HV_VP_STATE_PAGE_REGISTERS,
> >> -								  input_vtl_zero);
> >> +				(void)hv_unmap_vp_state_page(partition->pt_id,
> >> +							     vp->vp_index,
> >> +							     HV_VP_STATE_PAGE_REGISTERS,
> >> +							     virt_to_page(vp->vp_register_page),
> >> +							     input_vtl_zero);
> >>  				vp->vp_register_page = NULL;
> >>  			}
> >>
> >> -			(void)hv_call_unmap_vp_state_page(partition->pt_id,
> >> -							  vp->vp_index,
> >> -							  HV_VP_STATE_PAGE_INTERCEPT_MESSAGE,
> >> -							  input_vtl_zero);
> >> +			(void)hv_unmap_vp_state_page(partition->pt_id,
> >> +						     vp->vp_index,
> >> +						     HV_VP_STATE_PAGE_INTERCEPT_MESSAGE,
> >> +						     virt_to_page(vp->vp_intercept_msg_page),
> >> +						     input_vtl_zero);
> >>  			vp->vp_intercept_msg_page = NULL;
> >>
> >>  			if (vp->vp_ghcb_page) {
> >> -				(void)hv_call_unmap_vp_state_page(partition->pt_id,
> >> -								  vp->vp_index,
> >> -								  HV_VP_STATE_PAGE_GHCB,
> >> -								  input_vtl_normal);
> >> +				(void)hv_unmap_vp_state_page(partition->pt_id,
> >> +							     vp->vp_index,
> >> +							     HV_VP_STATE_PAGE_GHCB,
> >> +							     virt_to_page(vp->vp_ghcb_page),
> >> +							     input_vtl_normal);
> >>  				vp->vp_ghcb_page = NULL;
> >>  			}
> >>
> >> --
> >> 2.34.1
> >>


^ permalink raw reply

* RE: [PATCH v4 2/5] mshv: Add the HVCALL_GET_PARTITION_PROPERTY_EX hypercall
From: Michael Kelley @ 2025-10-02  0:01 UTC (permalink / raw)
  To: Nuno Das Neves, linux-hyperv@vger.kernel.org,
	linux-kernel@vger.kernel.org, prapal@linux.microsoft.com,
	easwar.hariharan@linux.microsoft.com, tiala@microsoft.com,
	anirudh@anirudhrb.com, paekkaladevi@linux.microsoft.com,
	skinsburskii@linux.microsoft.com
  Cc: kys@microsoft.com, haiyangz@microsoft.com, wei.liu@kernel.org,
	decui@microsoft.com
In-Reply-To: <e12f4d2a-8e58-40fa-930d-5575cd097b04@linux.microsoft.com>

From: Nuno Das Neves <nunodasneves@linux.microsoft.com> Sent: Wednesday, October 1, 2025 3:33 PM
> 
> On 9/29/2025 10:55 AM, Michael Kelley wrote:
> > From: Nuno Das Neves <nunodasneves@linux.microsoft.com> Sent: Friday, September 26, 2025 9:23 AM
> >>
> >> From: Purna Pavan Chandra Aekkaladevi <paekkaladevi@linux.microsoft.com>
> >>

[snip]

> >> diff --git a/include/hyperv/hvhdk.h b/include/hyperv/hvhdk.h
> >> index b4067ada02cf..416c0d45b793 100644
> >> --- a/include/hyperv/hvhdk.h
> >> +++ b/include/hyperv/hvhdk.h
> >> @@ -376,6 +376,46 @@ struct hv_input_set_partition_property {
> >>  	u64 property_value;
> >>  } __packed;
> >>
> >> +union hv_partition_property_arg {
> >> +	u64 as_uint64;
> >> +	struct {
> >> +		union {
> >> +			u32 arg;
> >> +			u32 vp_index;
> >> +		};
> >> +		u16 reserved0;
> >> +		u8 reserved1;
> >> +		u8 object_type;
> >> +	} __packed;
> >> +};
> >> +
> >> +struct hv_input_get_partition_property_ex {
> >> +	u64 partition_id;
> >> +	u32 property_code; /* enum hv_partition_property_code */
> >> +	u32 padding;
> >> +	union {
> >> +		union hv_partition_property_arg arg_data;
> >> +		u64 arg;
> >
> > This union, and the "u64 arg" member, seems redundant since
> > union hv_partition_property_arg already has a member "as_uint64".
> >
> > Maybe this is just being copied from the Windows versions of these
> > structures, in which case I realize there are constraints on what you
> > want to change or fix, and you can ignore my comment.
> >
> Yeah, this is just due to it coming from the Windows code. I prefer to
> keep it as close as possible unless it is actually a bug (like missing
> padding which has happened a couple of times).

Makes sense.

> 
> >> +	};
> >> +} __packed;
> >> +
> >> +/*
> >> + * NOTE: Should use hv_input_set_partition_property_ex_header to compute this
> >> + * size, but hv_input_get_partition_property_ex is identical so it suffices
> >> + */
> >> +#define HV_PARTITION_PROPERTY_EX_MAX_VAR_SIZE \
> >> +	(HV_HYP_PAGE_SIZE - sizeof(struct hv_input_get_partition_property_ex))
> >> +
> >> +union hv_partition_property_ex {
> >> +	u8 buffer[HV_PARTITION_PROPERTY_EX_MAX_VAR_SIZE];
> >
> > It's unclear what this "buffer" field is trying to do, and particularly its size.
> > The comment above references hv_input_set_partition_property_ex_header,
> > which doesn't exist in the Linux code.  And why is the max size of the output
> > buffer reduced by the size of the header of the input to "set partition property"?
> >
> Yes, hv_input_set_partition_property_ex_header doesn't exist here, it's from the
> Windows code (although in CAPs). It's identical to hv_input_get_partition_property_ex.
> I decided to just use hv_input_get_partition_property_ex instead of introducing the
> unused header struct, for now.
> 
> I'm not certain what the buffer is used for, but I suspect it's for data that doesn't
> fit with the other partition property structs. Maybe some raw/unstructured data.
> 
> hv_partition_property_ex has to be that size because it is also used (not yet, in linux)
> for setting properties, and the set hypercall has a header as mentioned above, so the
> entire struct mustn't exceed a page in size - i.e.:
> 
> struct hv_input_set_partition_property_ex {
> 	hv_input_set_partition_property_ex_header header;
> 	hv_partition_property_ex property_value;
> };

OK, that makes sense. Thanks.

Michael

^ permalink raw reply

* [PATCH hyperv-next] hyperv: Remove the spurious null directive line
From: Roman Kisel @ 2025-10-01 23:08 UTC (permalink / raw)
  To: decui, eahariha, haiyangz, kys, mhklinux, nunodasneves, wei.liu,
	linux-hyperv, linux-kernel
  Cc: benhill, bperkins, sunilmut, romank

The file contains a line that consists of the lone # symbol
followed by a newline. While that is a valid syntax as
defined by the C99+ grammar (6.10.7 "Null directive"), it
serves no apparent purpose in this case.

Remove the null preprocessor directive. No functional changes.

Fixes: e68bda71a238 ("hyperv: Add new Hyper-V headers in include/hyperv")
Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
---
 include/hyperv/hvgdk_mini.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/include/hyperv/hvgdk_mini.h b/include/hyperv/hvgdk_mini.h
index 1be7f6a02304..77abddfc750e 100644
--- a/include/hyperv/hvgdk_mini.h
+++ b/include/hyperv/hvgdk_mini.h
@@ -597,8 +597,6 @@ struct ms_hyperv_tsc_page {	 /* HV_REFERENCE_TSC_PAGE */
 #define HV_SYNIC_SINT_AUTO_EOI		(1ULL << 17)
 #define HV_SYNIC_SINT_VECTOR_MASK	(0xFF)
 
-#
-
 /* Hyper-V defined statically assigned SINTs */
 #define HV_SYNIC_INTERCEPTION_SINT_INDEX 0x00000000
 #define HV_SYNIC_IOMMU_FAULT_SINT_INDEX  0x00000001

base-commit: e3ec97c3abaf2fb68cc755cae3229288696b9f3d
-- 
2.43.0


^ permalink raw reply related

* Re: [PATCH v4 5/5] mshv: Introduce new hypercall to map stats page for L1VH partitions
From: Nuno Das Neves @ 2025-10-01 23:03 UTC (permalink / raw)
  To: Michael Kelley, linux-hyperv@vger.kernel.org,
	linux-kernel@vger.kernel.org, prapal@linux.microsoft.com,
	easwar.hariharan@linux.microsoft.com, tiala@microsoft.com,
	anirudh@anirudhrb.com, paekkaladevi@linux.microsoft.com,
	skinsburskii@linux.microsoft.com
  Cc: kys@microsoft.com, haiyangz@microsoft.com, wei.liu@kernel.org,
	decui@microsoft.com, Jinank Jain
In-Reply-To: <SN6PR02MB41571CB74AAAB29181443EE9D41BA@SN6PR02MB4157.namprd02.prod.outlook.com>

On 9/29/2025 10:57 AM, Michael Kelley wrote:
> From: Nuno Das Neves <nunodasneves@linux.microsoft.com> Sent: Friday, September 26, 2025 9:23 AM
>>
>> From: Jinank Jain <jinankjain@linux.microsoft.com>
>>
>> Introduce HVCALL_MAP_STATS_PAGE2 which provides a map location (GPFN)
>> to map the stats to. This hypercall is required for L1VH partitions,
>> depending on the hypervisor version. This uses the same check as the
>> state page map location; mshv_use_overlay_gpfn().
>>
>> Add mshv_map_vp_state_page() helpers to use this new hypercall or the
>> old one depending on availability.
>>
>> For unmapping, the original HVCALL_UNMAP_STATS_PAGE works for both
>> cases.
>>
>> Signed-off-by: Jinank Jain <jinankjain@linux.microsoft.com>
>> Signed-off-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>
>> Reviewed-by: Easwar Hariharan <easwar.hariharan@linux.microsoft.com>
>> ---
>>  drivers/hv/mshv_root.h         | 10 ++--
>>  drivers/hv/mshv_root_hv_call.c | 93 ++++++++++++++++++++++++++++++++--
>>  drivers/hv/mshv_root_main.c    | 22 ++++----
>>  include/hyperv/hvgdk_mini.h    |  1 +
>>  include/hyperv/hvhdk_mini.h    |  7 +++
>>  5 files changed, 113 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/hv/mshv_root.h b/drivers/hv/mshv_root.h
>> index dbe2d1d0b22f..0dfccfbe6123 100644
>> --- a/drivers/hv/mshv_root.h
>> +++ b/drivers/hv/mshv_root.h
>> @@ -297,11 +297,11 @@ int hv_call_connect_port(u64 port_partition_id, union hv_port_id port_id,
>>  int hv_call_disconnect_port(u64 connection_partition_id,
>>  			    union hv_connection_id connection_id);
>>  int hv_call_notify_port_ring_empty(u32 sint_index);
>> -int hv_call_map_stat_page(enum hv_stats_object_type type,
>> -			  const union hv_stats_object_identity *identity,
>> -			  void **addr);
>> -int hv_call_unmap_stat_page(enum hv_stats_object_type type,
>> -			    const union hv_stats_object_identity *identity);
>> +int hv_map_stats_page(enum hv_stats_object_type type,
>> +		      const union hv_stats_object_identity *identity,
>> +		      void **addr);
>> +int hv_unmap_stats_page(enum hv_stats_object_type type, void *page_addr,
>> +			const union hv_stats_object_identity *identity);
>>  int hv_call_modify_spa_host_access(u64 partition_id, struct page **pages,
>>  				   u64 page_struct_count, u32 host_access,
>>  				   u32 flags, u8 acquire);
>> diff --git a/drivers/hv/mshv_root_hv_call.c b/drivers/hv/mshv_root_hv_call.c
>> index 98c6278ff151..5a805b3dec0b 100644
>> --- a/drivers/hv/mshv_root_hv_call.c
>> +++ b/drivers/hv/mshv_root_hv_call.c
>> @@ -804,9 +804,51 @@ hv_call_notify_port_ring_empty(u32 sint_index)
>>  	return hv_result_to_errno(status);
>>  }
>>
>> -int hv_call_map_stat_page(enum hv_stats_object_type type,
>> -			  const union hv_stats_object_identity *identity,
>> -			  void **addr)
>> +static int hv_call_map_stats_page2(enum hv_stats_object_type type,
>> +				   const union hv_stats_object_identity *identity,
>> +				   u64 map_location)
>> +{
>> +	unsigned long flags;
>> +	struct hv_input_map_stats_page2 *input;
>> +	u64 status;
>> +	int ret;
>> +
>> +	if (!map_location || !mshv_use_overlay_gpfn())
>> +		return -EINVAL;
>> +
>> +	do {
>> +		local_irq_save(flags);
>> +		input = *this_cpu_ptr(hyperv_pcpu_input_arg);
>> +
>> +		memset(input, 0, sizeof(*input));
>> +		input->type = type;
>> +		input->identity = *identity;
>> +		input->map_location = map_location;
>> +
>> +		status = hv_do_hypercall(HVCALL_MAP_STATS_PAGE2, input, NULL);
>> +
>> +		local_irq_restore(flags);
>> +
>> +		ret = hv_result_to_errno(status);
>> +
>> +		if (!ret)
>> +			break;
> 
> This logic is incorrect. If the hypercall returns 
> HV_STATUS_INSUFFICIENT_MEMORY, then errno -ENOMEM is immediately
> returned to the caller without any opportunity to do hv_call_deposit_pages().
> 
The loop breaks if (!ret), i.e. on success. Maybe you misread it as `if (ret)`?
>> +
>> +		if (hv_result(status) != HV_STATUS_INSUFFICIENT_MEMORY) {
>> +			hv_status_debug(status, "\n");
>> +			break;
>> +		}
>> +
>> +		ret = hv_call_deposit_pages(NUMA_NO_NODE,
>> +					    hv_current_partition_id, 1);
>> +	} while (!ret);
>> +
>> +	return ret;
>> +}
>> +
>> +static int hv_call_map_stats_page(enum hv_stats_object_type type,
>> +				  const union hv_stats_object_identity *identity,
>> +				  void **addr)
>>  {
>>  	unsigned long flags;
>>  	struct hv_input_map_stats_page *input;
>> @@ -845,8 +887,36 @@ int hv_call_map_stat_page(enum hv_stats_object_type type,
>>  	return ret;
>>  }
>>
>> -int hv_call_unmap_stat_page(enum hv_stats_object_type type,
>> -			    const union hv_stats_object_identity *identity)
>> +int hv_map_stats_page(enum hv_stats_object_type type,
>> +		      const union hv_stats_object_identity *identity,
>> +		      void **addr)
>> +{
>> +	int ret;
>> +	struct page *allocated_page = NULL;
>> +
>> +	if (!addr)
>> +		return -EINVAL;
>> +
>> +	if (mshv_use_overlay_gpfn()) {
>> +		allocated_page = alloc_page(GFP_KERNEL);
>> +		if (!allocated_page)
>> +			return -ENOMEM;
>> +
>> +		ret = hv_call_map_stats_page2(type, identity,
>> +					      page_to_pfn(allocated_page));
> 
> This should use page_to_hvpfn() per my comments in Patch 4 of this series.
> 
I could change it, but as mentioned in my reply to Patch 4 I'm not sure it's
worth doing without addressing the whole issue which is a much bigger ask.

>> +		*addr = page_address(allocated_page);
>> +	} else {
>> +		ret = hv_call_map_stats_page(type, identity, addr);
>> +	}
>> +
>> +	if (ret && allocated_page)
>> +		__free_page(allocated_page);
> 
> Might want to do *addr = NULL after freeing the page so that the caller
> can't erroneously reference the free page. Again, the current caller doesn't
> do that, so current code isn't broken.
> 
Yep, I can add it to provide a little more robustness against bugs in callers.

>> +
>> +	return ret;
>> +}
>> +
>> +static int hv_call_unmap_stats_page(enum hv_stats_object_type type,
>> +				    const union hv_stats_object_identity *identity)
>>  {
>>  	unsigned long flags;
>>  	struct hv_input_unmap_stats_page *input;
>> @@ -865,6 +935,19 @@ int hv_call_unmap_stat_page(enum hv_stats_object_type type,
>>  	return hv_result_to_errno(status);
>>  }
>>
>> +int hv_unmap_stats_page(enum hv_stats_object_type type, void *page_addr,
>> +			const union hv_stats_object_identity *identity)
>> +{
>> +	int ret;
>> +
>> +	ret = hv_call_unmap_stats_page(type, identity);
>> +
>> +	if (mshv_use_overlay_gpfn() && page_addr)
>> +		__free_page(virt_to_page(page_addr));
>> +
>> +	return ret;
>> +}
>> +
>>  int hv_call_modify_spa_host_access(u64 partition_id, struct page **pages,
>>  				   u64 page_struct_count, u32 host_access,
>>  				   u32 flags, u8 acquire)
>> diff --git a/drivers/hv/mshv_root_main.c b/drivers/hv/mshv_root_main.c
>> index 2d0ad17acde6..71a8ab5db3b8 100644
>> --- a/drivers/hv/mshv_root_main.c
>> +++ b/drivers/hv/mshv_root_main.c
>> @@ -841,7 +841,8 @@ mshv_vp_release(struct inode *inode, struct file *filp)
>>  	return 0;
>>  }
>>
>> -static void mshv_vp_stats_unmap(u64 partition_id, u32 vp_index)
>> +static void mshv_vp_stats_unmap(u64 partition_id, u32 vp_index,
>> +				void *stats_pages[])
>>  {
>>  	union hv_stats_object_identity identity = {
>>  		.vp.partition_id = partition_id,
>> @@ -849,10 +850,10 @@ static void mshv_vp_stats_unmap(u64 partition_id, u32 vp_index)
>>  	};
>>
>>  	identity.vp.stats_area_type = HV_STATS_AREA_SELF;
>> -	hv_call_unmap_stat_page(HV_STATS_OBJECT_VP, &identity);
>> +	hv_unmap_stats_page(HV_STATS_OBJECT_VP, NULL, &identity);
>>
>>  	identity.vp.stats_area_type = HV_STATS_AREA_PARENT;
>> -	hv_call_unmap_stat_page(HV_STATS_OBJECT_VP, &identity);
>> +	hv_unmap_stats_page(HV_STATS_OBJECT_VP, NULL, &identity);
>>  }
>>
>>  static int mshv_vp_stats_map(u64 partition_id, u32 vp_index,
>> @@ -865,14 +866,14 @@ static int mshv_vp_stats_map(u64 partition_id, u32 vp_index,
>>  	int err;
>>
>>  	identity.vp.stats_area_type = HV_STATS_AREA_SELF;
>> -	err = hv_call_map_stat_page(HV_STATS_OBJECT_VP, &identity,
>> -				    &stats_pages[HV_STATS_AREA_SELF]);
>> +	err = hv_map_stats_page(HV_STATS_OBJECT_VP, &identity,
>> +				&stats_pages[HV_STATS_AREA_SELF]);
>>  	if (err)
>>  		return err;
>>
>>  	identity.vp.stats_area_type = HV_STATS_AREA_PARENT;
>> -	err = hv_call_map_stat_page(HV_STATS_OBJECT_VP, &identity,
>> -				    &stats_pages[HV_STATS_AREA_PARENT]);
>> +	err = hv_map_stats_page(HV_STATS_OBJECT_VP, &identity,
>> +				&stats_pages[HV_STATS_AREA_PARENT]);
>>  	if (err)
>>  		goto unmap_self;
>>
>> @@ -880,7 +881,7 @@ static int mshv_vp_stats_map(u64 partition_id, u32 vp_index,
>>
>>  unmap_self:
>>  	identity.vp.stats_area_type = HV_STATS_AREA_SELF;
>> -	hv_call_unmap_stat_page(HV_STATS_OBJECT_VP, &identity);
>> +	hv_unmap_stats_page(HV_STATS_OBJECT_VP, NULL, &identity);
>>  	return err;
>>  }
>>
>> @@ -988,7 +989,7 @@ mshv_partition_ioctl_create_vp(struct mshv_partition *partition,
>>  	kfree(vp);
>>  unmap_stats_pages:
>>  	if (hv_scheduler_type == HV_SCHEDULER_TYPE_ROOT)
>> -		mshv_vp_stats_unmap(partition->pt_id, args.vp_index);
>> +		mshv_vp_stats_unmap(partition->pt_id, args.vp_index, stats_pages);
>>  unmap_ghcb_page:
>>  	if (mshv_partition_encrypted(partition) && is_ghcb_mapping_available())
>>  		hv_unmap_vp_state_page(partition->pt_id, args.vp_index,
>> @@ -1740,7 +1741,8 @@ static void destroy_partition(struct mshv_partition *partition)
>>  				continue;
>>
>>  			if (hv_scheduler_type == HV_SCHEDULER_TYPE_ROOT)
>> -				mshv_vp_stats_unmap(partition->pt_id, vp->vp_index);
>> +				mshv_vp_stats_unmap(partition->pt_id, vp->vp_index,
>> +						    (void **)vp->vp_stats_pages);
>>
>>  			if (vp->vp_register_page) {
>>  				(void)hv_unmap_vp_state_page(partition->pt_id,
>> diff --git a/include/hyperv/hvgdk_mini.h b/include/hyperv/hvgdk_mini.h
>> index ff4325fb623a..f66565106d21 100644
>> --- a/include/hyperv/hvgdk_mini.h
>> +++ b/include/hyperv/hvgdk_mini.h
>> @@ -493,6 +493,7 @@ union hv_vp_assist_msr_contents {	 /*
>> HV_REGISTER_VP_ASSIST_PAGE */
>>  #define HVCALL_GET_PARTITION_PROPERTY_EX		0x0101
>>  #define HVCALL_MMIO_READ				0x0106
>>  #define HVCALL_MMIO_WRITE				0x0107
>> +#define HVCALL_MAP_STATS_PAGE2				0x0131
>>
>>  /* HV_HYPERCALL_INPUT */
>>  #define HV_HYPERCALL_RESULT_MASK	GENMASK_ULL(15, 0)
>> diff --git a/include/hyperv/hvhdk_mini.h b/include/hyperv/hvhdk_mini.h
>> index bf2ce27dfcc5..064bf735cab6 100644
>> --- a/include/hyperv/hvhdk_mini.h
>> +++ b/include/hyperv/hvhdk_mini.h
>> @@ -177,6 +177,13 @@ struct hv_input_map_stats_page {
>>  	union hv_stats_object_identity identity;
>>  } __packed;
>>
>> +struct hv_input_map_stats_page2 {
>> +	u32 type; /* enum hv_stats_object_type */
>> +	u32 padding;
>> +	union hv_stats_object_identity identity;
>> +	u64 map_location;
>> +} __packed;
>> +
>>  struct hv_output_map_stats_page {
>>  	u64 map_location;
>>  } __packed;
>> --
>> 2.34.1
>>


^ permalink raw reply

* Re: [PATCH v4 4/5] mshv: Allocate vp state page for HVCALL_MAP_VP_STATE_PAGE on L1VH
From: Nuno Das Neves @ 2025-10-01 22:56 UTC (permalink / raw)
  To: Michael Kelley, linux-hyperv@vger.kernel.org,
	linux-kernel@vger.kernel.org, prapal@linux.microsoft.com,
	easwar.hariharan@linux.microsoft.com, tiala@microsoft.com,
	anirudh@anirudhrb.com, paekkaladevi@linux.microsoft.com,
	skinsburskii@linux.microsoft.com
  Cc: kys@microsoft.com, haiyangz@microsoft.com, wei.liu@kernel.org,
	decui@microsoft.com, Jinank Jain
In-Reply-To: <SN6PR02MB4157E95BDC70A1F91228DAF7D41BA@SN6PR02MB4157.namprd02.prod.outlook.com>

On 9/29/2025 10:56 AM, Michael Kelley wrote:
> From: Nuno Das Neves <nunodasneves@linux.microsoft.com> Sent: Friday, September 26, 2025 9:23 AM
>>
>> From: Jinank Jain <jinankjain@linux.microsoft.com>
>>
>> Introduce mshv_use_overlay_gpfn() to check if a page needs to be
>> allocated and passed to the hypervisor to map VP state pages. This is
>> only needed on L1VH, and only on some (newer) versions of the
>> hypervisor, hence the need to check vmm_capabilities.
>>
>> Introduce functions hv_map/unmap_vp_state_page() to handle the
>> allocation and freeing.
>>
>> Signed-off-by: Jinank Jain <jinankjain@linux.microsoft.com>
>> Signed-off-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>
>> Reviewed-by: Praveen K Paladugu <prapal@linux.microsoft.com>
>> Reviewed-by: Easwar Hariharan <easwar.hariharan@linux.microsoft.com>
>> Reviewed-by: Stanislav Kinsburskii <skinsburskii@linux.microsoft.com>
>> Reviewed-by: Anirudh Rayabharam <anirudh@anirudhrb.com>
>> ---
>>  drivers/hv/mshv_root.h         | 11 ++---
>>  drivers/hv/mshv_root_hv_call.c | 61 ++++++++++++++++++++++++---
>>  drivers/hv/mshv_root_main.c    | 76 +++++++++++++++++-----------------
>>  3 files changed, 98 insertions(+), 50 deletions(-)
>>
>> diff --git a/drivers/hv/mshv_root.h b/drivers/hv/mshv_root.h
>> index 0cb1e2589fe1..dbe2d1d0b22f 100644
>> --- a/drivers/hv/mshv_root.h
>> +++ b/drivers/hv/mshv_root.h
>> @@ -279,11 +279,12 @@ int hv_call_set_vp_state(u32 vp_index, u64 partition_id,
>>  			 /* Choose between pages and bytes */
>>  			 struct hv_vp_state_data state_data, u64 page_count,
>>  			 struct page **pages, u32 num_bytes, u8 *bytes);
>> -int hv_call_map_vp_state_page(u64 partition_id, u32 vp_index, u32 type,
>> -			      union hv_input_vtl input_vtl,
>> -			      struct page **state_page);
>> -int hv_call_unmap_vp_state_page(u64 partition_id, u32 vp_index, u32 type,
>> -				union hv_input_vtl input_vtl);
>> +int hv_map_vp_state_page(u64 partition_id, u32 vp_index, u32 type,
>> +			 union hv_input_vtl input_vtl,
>> +			 struct page **state_page);
>> +int hv_unmap_vp_state_page(u64 partition_id, u32 vp_index, u32 type,
>> +			   struct page *state_page,
>> +			   union hv_input_vtl input_vtl);
>>  int hv_call_create_port(u64 port_partition_id, union hv_port_id port_id,
>>  			u64 connection_partition_id, struct hv_port_info *port_info,
>>  			u8 port_vtl, u8 min_connection_vtl, int node);
>> diff --git a/drivers/hv/mshv_root_hv_call.c b/drivers/hv/mshv_root_hv_call.c
>> index 3fd3cce23f69..98c6278ff151 100644
>> --- a/drivers/hv/mshv_root_hv_call.c
>> +++ b/drivers/hv/mshv_root_hv_call.c
>> @@ -526,9 +526,9 @@ int hv_call_set_vp_state(u32 vp_index, u64 partition_id,
>>  	return ret;
>>  }
>>
>> -int hv_call_map_vp_state_page(u64 partition_id, u32 vp_index, u32 type,
>> -			      union hv_input_vtl input_vtl,
>> -			      struct page **state_page)
>> +static int hv_call_map_vp_state_page(u64 partition_id, u32 vp_index, u32 type,
>> +				     union hv_input_vtl input_vtl,
>> +				     struct page **state_page)
>>  {
>>  	struct hv_input_map_vp_state_page *input;
>>  	struct hv_output_map_vp_state_page *output;
>> @@ -547,7 +547,14 @@ int hv_call_map_vp_state_page(u64 partition_id, u32 vp_index, u32 type,
>>  		input->type = type;
>>  		input->input_vtl = input_vtl;
>>
>> -		status = hv_do_hypercall(HVCALL_MAP_VP_STATE_PAGE, input, output);
> 
> This function must zero the input area before using it. Otherwise,
> flags.map_location_provided is uninitialized when *state_page is NULL. It will
> have whatever value was left by the previous user of hyperv_pcpu_input_arg,
> potentially producing bizarre results. And there's a reserved field that won't be
> set to zero.
> 
Good catch, will add a memset().

>> +		if (*state_page) {
>> +			input->flags.map_location_provided = 1;
>> +			input->requested_map_location =
>> +				page_to_pfn(*state_page);
> 
> Technically, this should be page_to_hvpfn() since the PFN value is being sent to
> Hyper-V. I know root (and L1VH?) partitions must run with the same page size
> as the Hyper-V host, but it's better to not leave code buried here that will blow
> up if the "same page size requirement" should ever change.
> 
Good point...I could change these calls, but the other way doesn't work, see below.

> And after making the hypercall, there's an invocation of pfn_to_page(), which
> should account for the same. Unfortunately, there's not an existing hvpfn_to_page()
> function.
> 
This seems like a tricky scenario to get right. In the root partition case, the
hypervisor allocates the page. That pfn could be some page within a larger Linux page.
Converting that to a Linux pfn (naively) means losing the original hvpfn since it gets
truncated, which is no good if we want to unmap it later. Also page_address() would
give the wrong virtual address.

In other words, we'd have to completely change how we track these pages in order to
support this scenario, and the same goes for various other hypervisor APIs where the
hypervisor does the allocating. I think it's out of scope to try and address that
here, even in part, especially since we will be making assumptions about something
that may never happen.

>> +		}
>> +
>> +		status = hv_do_hypercall(HVCALL_MAP_VP_STATE_PAGE, input,
>> +					 output);
>>
>>  		if (hv_result(status) != HV_STATUS_INSUFFICIENT_MEMORY) {
>>  			if (hv_result_success(status))
>> @@ -565,8 +572,39 @@ int hv_call_map_vp_state_page(u64 partition_id, u32 vp_index, u32 type,
>>  	return ret;
>>  }
>>
>> -int hv_call_unmap_vp_state_page(u64 partition_id, u32 vp_index, u32 type,
>> -				union hv_input_vtl input_vtl)
>> +static bool mshv_use_overlay_gpfn(void)
>> +{
>> +	return hv_l1vh_partition() &&
>> +	       mshv_root.vmm_caps.vmm_can_provide_overlay_gpfn;
>> +}
>> +
>> +int hv_map_vp_state_page(u64 partition_id, u32 vp_index, u32 type,
>> +			 union hv_input_vtl input_vtl,
>> +			 struct page **state_page)
>> +{
>> +	int ret = 0;
>> +	struct page *allocated_page = NULL;
>> +
>> +	if (mshv_use_overlay_gpfn()) {
>> +		allocated_page = alloc_page(GFP_KERNEL);
>> +		if (!allocated_page)
>> +			return -ENOMEM;
>> +		*state_page = allocated_page;
>> +	} else {
>> +		*state_page = NULL;
>> +	}
>> +
>> +	ret = hv_call_map_vp_state_page(partition_id, vp_index, type, input_vtl,
>> +					state_page);
>> +
>> +	if (ret && allocated_page)
>> +		__free_page(allocated_page);
> 
> For robustness, you might want to set *state_page = NULL here so the
> caller doesn't have a reference to the page that has been freed. I didn't
> see any cases where the caller incorrectly checks the returned
> *state_page value after an error, so the current code isn't broken.
> 
Sure, I can add it.

>> +
>> +	return ret;
>> +}
>> +
>> +static int hv_call_unmap_vp_state_page(u64 partition_id, u32 vp_index, u32 type,
>> +				       union hv_input_vtl input_vtl)
>>  {
>>  	unsigned long flags;
>>  	u64 status;
>> @@ -590,6 +628,17 @@ int hv_call_unmap_vp_state_page(u64 partition_id, u32 vp_index, u32 type,
>>  	return hv_result_to_errno(status);
>>  }
>>
>> +int hv_unmap_vp_state_page(u64 partition_id, u32 vp_index, u32 type,
>> +			   struct page *state_page, union hv_input_vtl input_vtl)
>> +{
>> +	int ret = hv_call_unmap_vp_state_page(partition_id, vp_index, type, input_vtl);
>> +
>> +	if (mshv_use_overlay_gpfn() && state_page)
>> +		__free_page(state_page);
>> +
>> +	return ret;
>> +}
>> +
>>  int hv_call_get_partition_property_ex(u64 partition_id, u64 property_code,
>>  				      u64 arg, void *property_value,
>>  				      size_t property_value_sz)
>> diff --git a/drivers/hv/mshv_root_main.c b/drivers/hv/mshv_root_main.c
>> index e199770ecdfa..2d0ad17acde6 100644
>> --- a/drivers/hv/mshv_root_main.c
>> +++ b/drivers/hv/mshv_root_main.c
>> @@ -890,7 +890,7 @@ mshv_partition_ioctl_create_vp(struct mshv_partition
>> *partition,
>>  {
>>  	struct mshv_create_vp args;
>>  	struct mshv_vp *vp;
>> -	struct page *intercept_message_page, *register_page, *ghcb_page;
>> +	struct page *intercept_msg_page, *register_page, *ghcb_page;
>>  	void *stats_pages[2];
>>  	long ret;
>>
>> @@ -908,28 +908,25 @@ mshv_partition_ioctl_create_vp(struct mshv_partition *partition,
>>  	if (ret)
>>  		return ret;
>>
>> -	ret = hv_call_map_vp_state_page(partition->pt_id, args.vp_index,
>> -					HV_VP_STATE_PAGE_INTERCEPT_MESSAGE,
>> -					input_vtl_zero,
>> -					&intercept_message_page);
>> +	ret = hv_map_vp_state_page(partition->pt_id, args.vp_index,
>> +				   HV_VP_STATE_PAGE_INTERCEPT_MESSAGE,
>> +				   input_vtl_zero, &intercept_msg_page);
>>  	if (ret)
>>  		goto destroy_vp;
>>
>>  	if (!mshv_partition_encrypted(partition)) {
>> -		ret = hv_call_map_vp_state_page(partition->pt_id, args.vp_index,
>> -						HV_VP_STATE_PAGE_REGISTERS,
>> -						input_vtl_zero,
>> -						&register_page);
>> +		ret = hv_map_vp_state_page(partition->pt_id, args.vp_index,
>> +					   HV_VP_STATE_PAGE_REGISTERS,
>> +					   input_vtl_zero, &register_page);
>>  		if (ret)
>>  			goto unmap_intercept_message_page;
>>  	}
>>
>>  	if (mshv_partition_encrypted(partition) &&
>>  	    is_ghcb_mapping_available()) {
>> -		ret = hv_call_map_vp_state_page(partition->pt_id, args.vp_index,
>> -						HV_VP_STATE_PAGE_GHCB,
>> -						input_vtl_normal,
>> -						&ghcb_page);
>> +		ret = hv_map_vp_state_page(partition->pt_id, args.vp_index,
>> +					   HV_VP_STATE_PAGE_GHCB,
>> +					   input_vtl_normal, &ghcb_page);
>>  		if (ret)
>>  			goto unmap_register_page;
>>  	}
>> @@ -960,7 +957,7 @@ mshv_partition_ioctl_create_vp(struct mshv_partition *partition,
>>  	atomic64_set(&vp->run.vp_signaled_count, 0);
>>
>>  	vp->vp_index = args.vp_index;
>> -	vp->vp_intercept_msg_page = page_to_virt(intercept_message_page);
>> +	vp->vp_intercept_msg_page = page_to_virt(intercept_msg_page);
>>  	if (!mshv_partition_encrypted(partition))
>>  		vp->vp_register_page = page_to_virt(register_page);
>>
>> @@ -993,21 +990,19 @@ mshv_partition_ioctl_create_vp(struct mshv_partition *partition,
>>  	if (hv_scheduler_type == HV_SCHEDULER_TYPE_ROOT)
>>  		mshv_vp_stats_unmap(partition->pt_id, args.vp_index);
>>  unmap_ghcb_page:
>> -	if (mshv_partition_encrypted(partition) && is_ghcb_mapping_available()) {
>> -		hv_call_unmap_vp_state_page(partition->pt_id, args.vp_index,
>> -					    HV_VP_STATE_PAGE_GHCB,
>> -					    input_vtl_normal);
>> -	}
>> +	if (mshv_partition_encrypted(partition) && is_ghcb_mapping_available())
>> +		hv_unmap_vp_state_page(partition->pt_id, args.vp_index,
>> +				       HV_VP_STATE_PAGE_GHCB, ghcb_page,
>> +				       input_vtl_normal);
>>  unmap_register_page:
>> -	if (!mshv_partition_encrypted(partition)) {
>> -		hv_call_unmap_vp_state_page(partition->pt_id, args.vp_index,
>> -					    HV_VP_STATE_PAGE_REGISTERS,
>> -					    input_vtl_zero);
>> -	}
>> +	if (!mshv_partition_encrypted(partition))
>> +		hv_unmap_vp_state_page(partition->pt_id, args.vp_index,
>> +				       HV_VP_STATE_PAGE_REGISTERS,
>> +				       register_page, input_vtl_zero);
>>  unmap_intercept_message_page:
>> -	hv_call_unmap_vp_state_page(partition->pt_id, args.vp_index,
>> -				    HV_VP_STATE_PAGE_INTERCEPT_MESSAGE,
>> -				    input_vtl_zero);
>> +	hv_unmap_vp_state_page(partition->pt_id, args.vp_index,
>> +			       HV_VP_STATE_PAGE_INTERCEPT_MESSAGE,
>> +			       intercept_msg_page, input_vtl_zero);
>>  destroy_vp:
>>  	hv_call_delete_vp(partition->pt_id, args.vp_index);
>>  	return ret;
>> @@ -1748,24 +1743,27 @@ static void destroy_partition(struct mshv_partition *partition)
>>  				mshv_vp_stats_unmap(partition->pt_id, vp->vp_index);
>>
>>  			if (vp->vp_register_page) {
>> -				(void)hv_call_unmap_vp_state_page(partition->pt_id,
>> -								  vp->vp_index,
>> - 								  HV_VP_STATE_PAGE_REGISTERS,
>> -								  input_vtl_zero);
>> +				(void)hv_unmap_vp_state_page(partition->pt_id,
>> +							     vp->vp_index,
>> +							     HV_VP_STATE_PAGE_REGISTERS,
>> +							     virt_to_page(vp->vp_register_page),
>> +							     input_vtl_zero);
>>  				vp->vp_register_page = NULL;
>>  			}
>>
>> -			(void)hv_call_unmap_vp_state_page(partition->pt_id,
>> -							  vp->vp_index,
>> -							  HV_VP_STATE_PAGE_INTERCEPT_MESSAGE,
>> -							  input_vtl_zero);
>> +			(void)hv_unmap_vp_state_page(partition->pt_id,
>> +						     vp->vp_index,
>> +						     HV_VP_STATE_PAGE_INTERCEPT_MESSAGE,
>> +						     virt_to_page(vp->vp_intercept_msg_page),
>> +						     input_vtl_zero);
>>  			vp->vp_intercept_msg_page = NULL;
>>
>>  			if (vp->vp_ghcb_page) {
>> -				(void)hv_call_unmap_vp_state_page(partition->pt_id,
>> -								  vp->vp_index,
>> -								  HV_VP_STATE_PAGE_GHCB,
>> -								  input_vtl_normal);
>> +				(void)hv_unmap_vp_state_page(partition->pt_id,
>> +							     vp->vp_index,
>> +							     HV_VP_STATE_PAGE_GHCB,
>> +							     virt_to_page(vp->vp_ghcb_page),
>> +							     input_vtl_normal);
>>  				vp->vp_ghcb_page = NULL;
>>  			}
>>
>> --
>> 2.34.1
>>


^ permalink raw reply

* Re: [PATCH v4 2/5] mshv: Add the HVCALL_GET_PARTITION_PROPERTY_EX hypercall
From: Nuno Das Neves @ 2025-10-01 22:32 UTC (permalink / raw)
  To: Michael Kelley, linux-hyperv@vger.kernel.org,
	linux-kernel@vger.kernel.org, prapal@linux.microsoft.com,
	easwar.hariharan@linux.microsoft.com, tiala@microsoft.com,
	anirudh@anirudhrb.com, paekkaladevi@linux.microsoft.com,
	skinsburskii@linux.microsoft.com
  Cc: kys@microsoft.com, haiyangz@microsoft.com, wei.liu@kernel.org,
	decui@microsoft.com
In-Reply-To: <SN6PR02MB415781A21E3F8CF52AB2A579D41BA@SN6PR02MB4157.namprd02.prod.outlook.com>

On 9/29/2025 10:55 AM, Michael Kelley wrote:
> From: Nuno Das Neves <nunodasneves@linux.microsoft.com> Sent: Friday, September 26, 2025 9:23 AM
>>
>> From: Purna Pavan Chandra Aekkaladevi <paekkaladevi@linux.microsoft.com>
>>
>> This hypercall can be used to fetch extended properties of a
>> partition. Extended properties are properties with values larger than
>> a u64. Some of these also need additional input arguments.
>>
>> Add helper function for using the hypercall in the mshv_root driver.
>>
>> Signed-off-by: Purna Pavan Chandra Aekkaladevi <paekkaladevi@linux.microsoft.com>
>> Signed-off-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>
>> Reviewed-by: Anirudh Rayabharam <anirudh@anirudhrb.com>
>> Reviewed-by: Praveen K Paladugu <prapal@linux.microsoft.com>
>> Reviewed-by: Easwar Hariharan <easwar.hariharan@linux.microsoft.com>
>> ---
>>  drivers/hv/mshv_root.h         |  2 ++
>>  drivers/hv/mshv_root_hv_call.c | 31 ++++++++++++++++++++++++++
>>  include/hyperv/hvgdk_mini.h    |  1 +
>>  include/hyperv/hvhdk.h         | 40 ++++++++++++++++++++++++++++++++++
>>  include/hyperv/hvhdk_mini.h    | 26 ++++++++++++++++++++++
>>  5 files changed, 100 insertions(+)
>>
>> diff --git a/drivers/hv/mshv_root.h b/drivers/hv/mshv_root.h
>> index e3931b0f1269..4aeb03bea6b6 100644
>> --- a/drivers/hv/mshv_root.h
>> +++ b/drivers/hv/mshv_root.h
>> @@ -303,6 +303,8 @@ int hv_call_unmap_stat_page(enum hv_stats_object_type type,
>>  int hv_call_modify_spa_host_access(u64 partition_id, struct page **pages,
>>  				   u64 page_struct_count, u32 host_access,
>>  				   u32 flags, u8 acquire);
>> +int hv_call_get_partition_property_ex(u64 partition_id, u64 property_code, u64 arg,
>> +				      void *property_value, size_t property_value_sz);
>>
>>  extern struct mshv_root mshv_root;
>>  extern enum hv_scheduler_type hv_scheduler_type;
>> diff --git a/drivers/hv/mshv_root_hv_call.c b/drivers/hv/mshv_root_hv_call.c
>> index c9c274f29c3c..3fd3cce23f69 100644
>> --- a/drivers/hv/mshv_root_hv_call.c
>> +++ b/drivers/hv/mshv_root_hv_call.c
>> @@ -590,6 +590,37 @@ int hv_call_unmap_vp_state_page(u64 partition_id, u32 vp_index, u32 type,
>>  	return hv_result_to_errno(status);
>>  }
>>
>> +int hv_call_get_partition_property_ex(u64 partition_id, u64 property_code,
>> +				      u64 arg, void *property_value,
>> +				      size_t property_value_sz)
>> +{
>> +	u64 status;
>> +	unsigned long flags;
>> +	struct hv_input_get_partition_property_ex *input;
>> +	struct hv_output_get_partition_property_ex *output;
>> +
>> +	local_irq_save(flags);
>> +	input = *this_cpu_ptr(hyperv_pcpu_input_arg);
>> +	output = *this_cpu_ptr(hyperv_pcpu_output_arg);
>> +
>> +	memset(input, 0, sizeof(*input));
>> +	input->partition_id = partition_id;
>> +	input->property_code = property_code;
>> +	input->arg = arg;
>> +	status = hv_do_hypercall(HVCALL_GET_PARTITION_PROPERTY_EX, input, output);
>> +
>> +	if (!hv_result_success(status)) {
>> +		hv_status_debug(status, "\n");
>> +		local_irq_restore(flags);
> 
> Nit: It would be marginally better to do the local_irq_restore() first, and then
> output the error message so that interrupts are not disabled any longer than
> needed.
> 
Good point

>> +		return hv_result_to_errno(status);
>> +	}
>> +	memcpy(property_value, &output->property_value, property_value_sz);
>> +
>> +	local_irq_restore(flags);
>> +
>> +	return 0;
>> +}
>> +
>>  int
>>  hv_call_clear_virtual_interrupt(u64 partition_id)
>>  {
>> diff --git a/include/hyperv/hvgdk_mini.h b/include/hyperv/hvgdk_mini.h
>> index 1be7f6a02304..ff4325fb623a 100644
>> --- a/include/hyperv/hvgdk_mini.h
>> +++ b/include/hyperv/hvgdk_mini.h
>> @@ -490,6 +490,7 @@ union hv_vp_assist_msr_contents {	 /*
>> HV_REGISTER_VP_ASSIST_PAGE */
>>  #define HVCALL_GET_VP_STATE				0x00e3
>>  #define HVCALL_SET_VP_STATE				0x00e4
>>  #define HVCALL_GET_VP_CPUID_VALUES			0x00f4
>> +#define HVCALL_GET_PARTITION_PROPERTY_EX		0x0101
>>  #define HVCALL_MMIO_READ				0x0106
>>  #define HVCALL_MMIO_WRITE				0x0107
>>
>> diff --git a/include/hyperv/hvhdk.h b/include/hyperv/hvhdk.h
>> index b4067ada02cf..416c0d45b793 100644
>> --- a/include/hyperv/hvhdk.h
>> +++ b/include/hyperv/hvhdk.h
>> @@ -376,6 +376,46 @@ struct hv_input_set_partition_property {
>>  	u64 property_value;
>>  } __packed;
>>
>> +union hv_partition_property_arg {
>> +	u64 as_uint64;
>> +	struct {
>> +		union {
>> +			u32 arg;
>> +			u32 vp_index;
>> +		};
>> +		u16 reserved0;
>> +		u8 reserved1;
>> +		u8 object_type;
>> +	} __packed;
>> +};
>> +
>> +struct hv_input_get_partition_property_ex {
>> +	u64 partition_id;
>> +	u32 property_code; /* enum hv_partition_property_code */
>> +	u32 padding;
>> +	union {
>> +		union hv_partition_property_arg arg_data;
>> +		u64 arg;
> 
> This union, and the "u64 arg" member, seems redundant since
> union hv_partition_property_arg already has a member "as_uint64".
> 
> Maybe this is just being copied from the Windows versions of these
> structures, in which case I realize there are constraints on what you
> want to change or fix, and you can ignore my comment.
> 
Yeah, this is just due to it coming from the Windows code. I prefer to
keep it as close as possible unless it is actually a bug (like missing
padding which has happened a couple of times).

>> +	};
>> +} __packed;
>> +
>> +/*
>> + * NOTE: Should use hv_input_set_partition_property_ex_header to compute this
>> + * size, but hv_input_get_partition_property_ex is identical so it suffices
>> + */
>> +#define HV_PARTITION_PROPERTY_EX_MAX_VAR_SIZE \
>> +	(HV_HYP_PAGE_SIZE - sizeof(struct hv_input_get_partition_property_ex))
>> +
>> +union hv_partition_property_ex {
>> +	u8 buffer[HV_PARTITION_PROPERTY_EX_MAX_VAR_SIZE];
> 
> It's unclear what this "buffer" field is trying to do, and particularly its size.
> The comment above references hv_input_set_partition_property_ex_header,
> which doesn't exist in the Linux code.  And why is the max size of the output
> buffer reduced by the size of the header of the input to "set partition property"?
> 
Yes, hv_input_set_partition_property_ex_header doesn't exist here, it's from the
Windows code (although in CAPs). It's identical to hv_input_get_partition_property_ex.
I decided to just use hv_input_get_partition_property_ex instead of introducing the
unused header struct, for now.

I'm not certain what the buffer is used for, but I suspect it's for data that doesn't
fit with the other partition property structs. Maybe some raw/unstructured data.

hv_partition_property_ex has to be that size because it is also used (not yet, in linux)
for setting properties, and the set hypercall has a header as mentioned above, so the
entire struct mustn't exceed a page in size - i.e.:

struct hv_input_set_partition_property_ex {
	hv_input_set_partition_property_ex_header header;
	hv_partition_property_ex property_value;
};

But we don't need this yet.

Thanks
Nuno

>> +	struct hv_partition_property_vmm_capabilities vmm_capabilities;
>> +	/* More fields to be filled in when needed */
>> +};
>> +
>> +struct hv_output_get_partition_property_ex {
>> +	union hv_partition_property_ex property_value;
>> +} __packed;
>> +
>>  enum hv_vp_state_page_type {
>>  	HV_VP_STATE_PAGE_REGISTERS = 0,
>>  	HV_VP_STATE_PAGE_INTERCEPT_MESSAGE = 1,
>> diff --git a/include/hyperv/hvhdk_mini.h b/include/hyperv/hvhdk_mini.h
>> index 858f6a3925b3..bf2ce27dfcc5 100644
>> --- a/include/hyperv/hvhdk_mini.h
>> +++ b/include/hyperv/hvhdk_mini.h
>> @@ -96,8 +96,34 @@ enum hv_partition_property_code {
>>  	HV_PARTITION_PROPERTY_XSAVE_STATES                      = 0x00060007,
>>  	HV_PARTITION_PROPERTY_MAX_XSAVE_DATA_SIZE		= 0x00060008,
>>  	HV_PARTITION_PROPERTY_PROCESSOR_CLOCK_FREQUENCY		= 0x00060009,
>> +
>> +	/* Extended properties with larger property values */
>> +	HV_PARTITION_PROPERTY_VMM_CAPABILITIES			= 0x00090007,
>>  };
>>
>> +#define HV_PARTITION_VMM_CAPABILITIES_BANK_COUNT		1
>> +#define HV_PARTITION_VMM_CAPABILITIES_RESERVED_BITFIELD_COUNT	59
>> +
>> +struct hv_partition_property_vmm_capabilities {
>> +	u16 bank_count;
>> +	u16 reserved[3];
>> +	union {
>> +		u64 as_uint64[HV_PARTITION_VMM_CAPABILITIES_BANK_COUNT];
>> +		struct {
>> +			u64 map_gpa_preserve_adjustable: 1;
>> +			u64 vmm_can_provide_overlay_gpfn: 1;
>> +			u64 vp_affinity_property: 1;
>> +#if IS_ENABLED(CONFIG_ARM64)
>> +			u64 vmm_can_provide_gic_overlay_locations: 1;
>> +#else
>> +			u64 reservedbit3: 1;
>> +#endif
>> +			u64 assignable_synthetic_proc_features: 1;
>> +			u64 reserved0: HV_PARTITION_VMM_CAPABILITIES_RESERVED_BITFIELD_COUNT;
>> +		} __packed;
>> +	};
>> +} __packed;
>> +
>>  enum hv_snp_status {
>>  	HV_SNP_STATUS_NONE = 0,
>>  	HV_SNP_STATUS_AVAILABLE = 1,
>> --
>> 2.34.1
>>


^ permalink raw reply

* Re: [PATCH v2 4/6] x86/hyperv: Add trampoline asm code to transition from hypervisor
From: Mukesh R @ 2025-10-01 21:07 UTC (permalink / raw)
  To: Wei Liu
  Cc: linux-hyperv, linux-kernel, linux-arch, kys, haiyangz, decui,
	tglx, mingo, bp, dave.hansen, x86, hpa, arnd
In-Reply-To: <20251001060002.GA603271@liuwe-devbox-debian-v2.local>

On 9/30/25 23:00, Wei Liu wrote:
> On Tue, Sep 23, 2025 at 02:46:07PM -0700, Mukesh Rathor wrote:
>> Introduce a small asm stub to transition from the hypervisor to Linux
>> after devirtualization. Devirtualization means disabling hypervisor on
>> the fly, so after it is done, the code is running on physical processor
>> instead of virtual, and hypervisor is gone. This can be done by a
>> root/dom0 vm only.
> 
> I want to scrub "dom0" from comments and commit messages. We drew
> parallels to Xen when we first wrote this code, but it's not a useful
> term externally. "root" or "root partition" should be sufficient.
> 
>>
>> At a high level, during panic of either the hypervisor or the dom0 (aka
>> root), the NMI handler asks hypervisor to devirtualize. As part of that,
>> the arguments include an entry point to return back to Linux. This asm
>> stub implements that entry point.
>>
>> The stub is entered in protected mode, uses temporary gdt and page table
>> to enable long mode and get to kernel entry point which then restores full
>> kernel context to resume execution to kexec.
>>
>> Signed-off-by: Mukesh Rathor <mrathor@linux.microsoft.com>
>> ---
>>  arch/x86/hyperv/hv_trampoline.S | 101 ++++++++++++++++++++++++++++++++
>>  1 file changed, 101 insertions(+)
>>  create mode 100644 arch/x86/hyperv/hv_trampoline.S
>>
>> diff --git a/arch/x86/hyperv/hv_trampoline.S b/arch/x86/hyperv/hv_trampoline.S
>> new file mode 100644
>> index 000000000000..25f02ff12286
>> --- /dev/null
>> +++ b/arch/x86/hyperv/hv_trampoline.S
>> @@ -0,0 +1,101 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +/*
>> + * X86 specific Hyper-V kdump/crash related code.
>> + *
>> + * Copyright (C) 2025, Microsoft, Inc.
>> + *
>> + */
>> +#include <linux/linkage.h>
>> +#include <asm/alternative.h>
>> +#include <asm/msr.h>
>> +#include <asm/processor-flags.h>
>> +#include <asm/nospec-branch.h>
>> +
>> +/*
>> + * void noreturn hv_crash_asm32(arg1)
>> + *    arg1 == edi == 32bit PA of struct hv_crash_tramp_data
>> + *
>> + * The hypervisor jumps here upon devirtualization in protected mode. This
>> + * code gets copied to a page in the low 4G ie, 32bit space so it can run
>> + * in the protected mode. Hence we cannot use any compile/link time offsets or
>> + * addresses. It restores long mode via temporary gdt and page tables and
>> + * eventually jumps to kernel code entry at HV_CRASHDATA_OFFS_C_entry.
>> + *
>> + * PreCondition (ie, Hypervisor call back ABI):
>> + *  o CR0 is set to 0x0021: PE(prot mode) and NE are set, paging is disabled
>> + *  o CR4 is set to 0x0
>> + *  o IA32_EFER is set to 0x901 (SCE and NXE are set)
>> + *  o EDI is set to the Arg passed to HVCALL_DISABLE_HYP_EX.
>> + *  o CS, DS, ES, FS, GS are all initialized with a base of 0 and limit 0xFFFF
>> + *  o IDTR, TR and GDTR are initialized with a base of 0 and limit of 0xFFFF
>> + *  o LDTR is initialized as invalid (limit of 0)
>> + *  o MSR PAT is power on default.
>> + *  o Other state/registers are cleared. All TLBs flushed.
>> + */
>> +
>> +#define HV_CRASHDATA_OFFS_TRAMPCR3    0x0    /*  0 */
>> +#define HV_CRASHDATA_OFFS_KERNCR3     0x8    /*  8 */
>> +#define HV_CRASHDATA_OFFS_GDTRLIMIT  0x12    /* 18 */
>> +#define HV_CRASHDATA_OFFS_CS_JMPTGT  0x28    /* 40 */
>> +#define HV_CRASHDATA_OFFS_C_entry    0x30    /* 48 */
>> +
>> +	.text
>> +	.code32
>> +
> 
> I recently learned that instrumentation may be problematic for context
> switching code. I have not studied this code and noinstr usage in tree
> extensively so cannot make a judgement here.
> 
> It is worth checking out the recent discussion on the VTL transition
> code.
> 
> https://lore.kernel.org/linux-hyperv/27e50bb7-7f0e-48fb-bdbc-6c6d606e7113@redhat.com/
> 
> And check out the in-tree document Documentation/core-api/entry.rst.

Thanks, we should be ok here because this is actually copied to another
below 4G page for protected mode transfer. It is then executed from there,
and not the default section it is linked in. For example, 

arch/x86/kernel/relocate_kernel_64.S

does not have .noinstr. 

Thanks,
-Mukesh


> Wei
> 
>> +SYM_CODE_START(hv_crash_asm32)
>> +	UNWIND_HINT_UNDEFINED
>> +	ENDBR
>> +	movl	$X86_CR4_PAE, %ecx
>> +	movl	%ecx, %cr4
>> +
>> +	movl %edi, %ebx
>> +	add $HV_CRASHDATA_OFFS_TRAMPCR3, %ebx
>> +	movl %cs:(%ebx), %eax
>> +	movl %eax, %cr3
>> +
>> +	/* Setup EFER for long mode now */
>> +	movl	$MSR_EFER, %ecx
>> +	rdmsr
>> +	btsl	$_EFER_LME, %eax
>> +	wrmsr
>> +
>> +	/* Turn paging on using the temp 32bit trampoline page table */
>> +	movl %cr0, %eax
>> +	orl $(X86_CR0_PG), %eax
>> +	movl %eax, %cr0
>> +
>> +	/* since kernel cr3 could be above 4G, we need to be in the long mode
>> +	 * before we can load 64bits of the kernel cr3. We use a temp gdt for
>> +	 * that with CS.L=1 and CS.D=0 */
>> +	mov %edi, %eax
>> +	add $HV_CRASHDATA_OFFS_GDTRLIMIT, %eax
>> +	lgdtl %cs:(%eax)
>> +
>> +	/* not done yet, restore CS now to switch to CS.L=1 */
>> +	mov %edi, %eax
>> +	add $HV_CRASHDATA_OFFS_CS_JMPTGT, %eax
>> +	ljmp %cs:*(%eax)
>> +SYM_CODE_END(hv_crash_asm32)
>> +
>> +	/* we now run in full 64bit IA32-e long mode, CS.L=1 and CS.D=0 */
>> +	.code64
>> +	.balign 8
>> +SYM_CODE_START(hv_crash_asm64)
>> +	UNWIND_HINT_UNDEFINED
>> +	ENDBR
>> +	/* restore kernel page tables so we can jump to kernel code */
>> +	mov %edi, %eax
>> +	add $HV_CRASHDATA_OFFS_KERNCR3, %eax
>> +	movq %cs:(%eax), %rbx
>> +	movq %rbx, %cr3
>> +
>> +	mov %edi, %eax
>> +	add $HV_CRASHDATA_OFFS_C_entry, %eax
>> +	movq %cs:(%eax), %rbx
>> +	ANNOTATE_RETPOLINE_SAFE
>> +	jmp *%rbx
>> +
>> +	int $3
>> +
>> +SYM_INNER_LABEL(hv_crash_asm_end, SYM_L_GLOBAL)
>> +SYM_CODE_END(hv_crash_asm64)
>> -- 
>> 2.36.1.vfs.0.0
>>
>>


^ permalink raw reply

* Re: [PATCH 0/3] Introduce movable pages for Hyper-V guests
From: Mike Rapoport @ 2025-10-01 16:39 UTC (permalink / raw)
  To: Wei Liu
  Cc: Mukesh R, Stanislav Kinsburskii, kys, haiyangz, decui,
	linux-hyperv, linux-kernel
In-Reply-To: <aNyrlijDzaosdWxa@liuwe-devbox-ubuntu-v2.lamzopl0uupeniq2etz1fddiyg.xx.internal.cloudapp.net>

On Wed, Oct 01, 2025 at 04:18:30AM +0000, Wei Liu wrote:
> +Mike Rapoport, our resident memory management expert.
> 
> On Fri, Sep 26, 2025 at 07:02:02PM -0700, Mukesh R wrote:
> > On 9/24/25 14:30, Stanislav Kinsburskii wrote:
> > >>From the start, the root-partition driver allocates, pins, and maps all
> > > guest memory into the hypervisor at guest creation. This is simple: Linux
> > > cannot move the pages, so the guest?s view in Linux and in Microsoft
> > > Hypervisor never diverges.
> > > 
> > > However, this approach has major drawbacks:
> > > - NUMA: affinity can?t be changed at runtime, so you can?t migrate guest memory closer to the CPUs running it ? performance hit.
> > > - Memory management: unused guest memory can?t be swapped out, compacted, or merged.
> > > - Provisioning time: upfront allocation/pinning slows guest create/destroy.
> > > - Overcommit: no memory overcommit on hosts with pinned-guest memory.
> > > 
> > > This series adds movable memory pages for Hyper-V child partitions. Guest
> > > pages are no longer allocated upfront; they?re allocated and mapped into
> > > the hypervisor on demand (i.e., when the guest touches a GFN that isn?t yet
> > > backed by a host PFN).
> > > When a page is moved, Linux no longer holds it and it is unmapped from the hypervisor.
> > > As a result, Hyper-V guests behave like regular Linux processes, enabling standard Linux memory features to apply to guests.
> > > 
> > > Exceptions (still pinned):
> > >   1. Encrypted guests (explicit).
> > >   2 Guests with passthrough devices (implicitly pinned by the VFIO framework).
> > 
> > 
> > As I had commented internally, I am not fully comfortable about the
> > approach here, specially around use of HMM, and the correctness of
> > locking for shared memory regions, but my knowledge is from 4.15 and
> > maybe outdated, and don't have time right now. So I won't object to it
> > if other hard core mmu developers think there are no issues.
> > 
> 
> Mike, I seem to remember you had a discussion with Stanislav about this?
> Can you confirm that this is a reasonable approach?
>
> Better yet, if you have time to review the code, that would be great.
> Note that there is a v2 on linux-hyperv.  But I would like to close
> Mukesh's question first.

I only had time to skip through the patches and yes, this is a reasonable
approach. I also confirmed privately with HMM maintainer a while ago that
the use of HMM and MMU notifiers is correct.

I don't know enough about mshv to see if there are corner cases that these
patches don't cover, but conceptually they make memory model follow KVM
best practices.
 
> > However, we won't be using this for minkernel, so would like a driver
> > boot option to disable it upon boot that we can just set in minkernel
> > init path. This option can also be used to disable it if problems are
> > observed on the field. Minkernel design is still being worked on, so I
> > cannot provide much details on it yet.

The usual way we do things in the kernel is to add functionality when it
has users, so a boot option can be added later when minkernel design will
be more mature and ready for upstream.

-- 
Sincerely yours,
Mike.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox