[PATCH V1 0/3] Redesign memory deposit logic

[PATCH V1 0/3] Redesign memory deposit logic

[Mukesh R]

This small series redesigns the memory deposit logic when Linux runs on
Hyper-V hypervisor as a privileged VM, the second patch explains why.
The series is split in three patches, the first patch renames the
functions to _old and this makes the 2 main patch much easier and nicer
to review. Last patch just removes the *_old functions from first patch.

Thanks,
-Mukesh

V1:
 o Remove hv_alloc_contig_pages() and make contiguous allocation part of
   hv_alloc_dep_pages(), thus getting rid of the kmalloc.
 o Instead of stubbing out functions, rename them to *_old.

Mukesh R (3):
  mshv: Rename memory deposit memory functions to old
  mshv: Redesign hypervisor memory deposit logic
  mshv: Remove unused *_old memory deposit functions

 drivers/hv/hv_proc.c           | 250 +++++++++++++++++++--------------
 drivers/hv/mshv_root_hv_call.c |  10 +-
 include/asm-generic/mshyperv.h |   5 -
 3 files changed, 145 insertions(+), 120 deletions(-)

-- 
2.51.2.vfs.0.1

[PATCH V1 1/3] mshv: Rename memory deposit memory functions to _old

[Mukesh R]

Rename hv_call_deposit_pages() and hv_deposit_memory_node() to _old
to make reviewing the new functions much easier.

Signed-off-by: Mukesh R <mrathor@linux.microsoft.com>
---
 drivers/hv/hv_proc.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/drivers/hv/hv_proc.c b/drivers/hv/hv_proc.c
index 57b2c64197cb..57864bb5bcd8 100644
--- a/drivers/hv/hv_proc.c
+++ b/drivers/hv/hv_proc.c
@@ -13,10 +13,10 @@
  * See struct hv_deposit_memory. The first u64 is partition ID, the rest
  * are GPAs.
  */
-#define HV_DEPOSIT_MAX (HV_HYP_PAGE_SIZE / sizeof(u64) - 1)
+#define HV_DEPOSIT_MAX_OLD (HV_HYP_PAGE_SIZE / sizeof(u64) - 1)
 
 /* Deposits exact number of pages. Must be called with interrupts enabled.  */
-int hv_call_deposit_pages(int node, u64 partition_id, u32 num_pages)
+static int hv_call_deposit_pages_old(int node, u64 partition_id, u32 num_pages)
 {
 	struct page **pages, *page;
 	int *counts;
@@ -29,7 +29,7 @@ int hv_call_deposit_pages(int node, u64 partition_id, u32 num_pages)
 	struct hv_deposit_memory *input_page;
 	unsigned long flags;
 
-	if (num_pages > HV_DEPOSIT_MAX)
+	if (num_pages > HV_DEPOSIT_MAX_OLD)
 		return -E2BIG;
 	if (!num_pages)
 		return 0;
@@ -40,7 +40,7 @@ int hv_call_deposit_pages(int node, u64 partition_id, u32 num_pages)
 		return -ENOMEM;
 	pages = page_address(page);
 
-	counts = kzalloc_objs(int, HV_DEPOSIT_MAX);
+	counts = kzalloc_objs(int, HV_DEPOSIT_MAX_OLD);
 	if (!counts) {
 		free_page((unsigned long)pages);
 		return -ENOMEM;
@@ -108,10 +108,14 @@ int hv_call_deposit_pages(int node, u64 partition_id, u32 num_pages)
 	kfree(counts);
 	return ret;
 }
+
+int hv_call_deposit_pages(int node, u64 partition_id, u32 num_pages)
+{
+	return hv_call_deposit_pages_old(node, partition_id, num_pages);
+}
 EXPORT_SYMBOL_GPL(hv_call_deposit_pages);
 
-int hv_deposit_memory_node(int node, u64 partition_id,
-			   u64 hv_status)
+static int __maybe_unused hv_deposit_memory_node_old(int node, u64 partition_id, u64 hv_status)
 {
 	u32 num_pages = 1;
 
@@ -137,7 +141,12 @@ int hv_deposit_memory_node(int node, u64 partition_id,
 		hv_status_err(hv_status, "Unexpected!\n");
 		return -ENOMEM;
 	}
-	return hv_call_deposit_pages(node, partition_id, num_pages);
+	return hv_call_deposit_pages_old(node, partition_id, num_pages);
+}
+
+int hv_deposit_memory_node(int node, u64 partition_id, u64 hv_status)
+{
+	return hv_deposit_memory_node_old(node, partition_id, hv_status);
 }
 EXPORT_SYMBOL_GPL(hv_deposit_memory_node);
 
-- 
2.51.2.vfs.0.1

RE: [PATCH V1 1/3] mshv: Rename memory deposit memory functions to _old

[Michael Kelley]

From: Mukesh R <mrathor@linux.microsoft.com> Sent: Monday, September 7, 2026 7:14 PM
> 
> Rename hv_call_deposit_pages() and hv_deposit_memory_node() to _old
> to make reviewing the new functions much easier.
> 
> Signed-off-by: Mukesh R <mrathor@linux.microsoft.com>
> ---
>  drivers/hv/hv_proc.c | 23 ++++++++++++++++-------
>  1 file changed, 16 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/hv/hv_proc.c b/drivers/hv/hv_proc.c
> index 57b2c64197cb..57864bb5bcd8 100644
> --- a/drivers/hv/hv_proc.c
> +++ b/drivers/hv/hv_proc.c
> @@ -13,10 +13,10 @@
>   * See struct hv_deposit_memory. The first u64 is partition ID, the rest
>   * are GPAs.
>   */
> -#define HV_DEPOSIT_MAX (HV_HYP_PAGE_SIZE / sizeof(u64) - 1)
> +#define HV_DEPOSIT_MAX_OLD (HV_HYP_PAGE_SIZE / sizeof(u64) - 1)
> 
>  /* Deposits exact number of pages. Must be called with interrupts enabled.  */
> -int hv_call_deposit_pages(int node, u64 partition_id, u32 num_pages)
> +static int hv_call_deposit_pages_old(int node, u64 partition_id, u32 num_pages)
>  {
>  	struct page **pages, *page;
>  	int *counts;
> @@ -29,7 +29,7 @@ int hv_call_deposit_pages(int node, u64 partition_id, u32 num_pages)
>  	struct hv_deposit_memory *input_page;
>  	unsigned long flags;
> 
> -	if (num_pages > HV_DEPOSIT_MAX)
> +	if (num_pages > HV_DEPOSIT_MAX_OLD)
>  		return -E2BIG;
>  	if (!num_pages)
>  		return 0;
> @@ -40,7 +40,7 @@ int hv_call_deposit_pages(int node, u64 partition_id, u32 num_pages)
>  		return -ENOMEM;
>  	pages = page_address(page);
> 
> -	counts = kzalloc_objs(int, HV_DEPOSIT_MAX);
> +	counts = kzalloc_objs(int, HV_DEPOSIT_MAX_OLD);
>  	if (!counts) {
>  		free_page((unsigned long)pages);
>  		return -ENOMEM;
> @@ -108,10 +108,14 @@ int hv_call_deposit_pages(int node, u64 partition_id, u32 num_pages)
>  	kfree(counts);
>  	return ret;
>  }
> +
> +int hv_call_deposit_pages(int node, u64 partition_id, u32 num_pages)
> +{
> +	return hv_call_deposit_pages_old(node, partition_id, num_pages);
> +}
>  EXPORT_SYMBOL_GPL(hv_call_deposit_pages);
> 
> -int hv_deposit_memory_node(int node, u64 partition_id,
> -			   u64 hv_status)
> +static int __maybe_unused hv_deposit_memory_node_old(int node, u64 partition_id, u64 hv_status)

I'm not clear on why there is "__maybe_unused" here. If CONFIG_MSHV_ROOT
isn't defined, this whole file isn't built.

But it doesn't really matter since Patch 3 of the series deletes the code anyway.

>  {
>  	u32 num_pages = 1;
> 
> @@ -137,7 +141,12 @@ int hv_deposit_memory_node(int node, u64 partition_id,
>  		hv_status_err(hv_status, "Unexpected!\n");
>  		return -ENOMEM;
>  	}
> -	return hv_call_deposit_pages(node, partition_id, num_pages);
> +	return hv_call_deposit_pages_old(node, partition_id, num_pages);
> +}
> +
> +int hv_deposit_memory_node(int node, u64 partition_id, u64 hv_status)
> +{
> +	return hv_deposit_memory_node_old(node, partition_id, hv_status);
>  }
>  EXPORT_SYMBOL_GPL(hv_deposit_memory_node);
> 
> --
> 2.51.2.vfs.0.1
> 

The "__maybe_unused" notwithstanding,

Reviewed-by: Michael Kelley <mhklinux@outlook.com>

Re: [PATCH V1 1/3] mshv: Rename memory deposit memory functions to _old

[Mukesh R]

On 9/8/26 11:01, Michael Kelley wrote:
> From: Mukesh R <mrathor@linux.microsoft.com> Sent: Monday, September 7, 2026 7:14 PM
>>
>> Rename hv_call_deposit_pages() and hv_deposit_memory_node() to _old
>> to make reviewing the new functions much easier.
>>
>> Signed-off-by: Mukesh R <mrathor@linux.microsoft.com>
>> ---
>>   drivers/hv/hv_proc.c | 23 ++++++++++++++++-------
>>   1 file changed, 16 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/hv/hv_proc.c b/drivers/hv/hv_proc.c
>> index 57b2c64197cb..57864bb5bcd8 100644
>> --- a/drivers/hv/hv_proc.c
>> +++ b/drivers/hv/hv_proc.c
>> @@ -13,10 +13,10 @@
>>    * See struct hv_deposit_memory. The first u64 is partition ID, the rest
>>    * are GPAs.
>>    */
>> -#define HV_DEPOSIT_MAX (HV_HYP_PAGE_SIZE / sizeof(u64) - 1)
>> +#define HV_DEPOSIT_MAX_OLD (HV_HYP_PAGE_SIZE / sizeof(u64) - 1)
>>
>>   /* Deposits exact number of pages. Must be called with interrupts enabled.  */
>> -int hv_call_deposit_pages(int node, u64 partition_id, u32 num_pages)
>> +static int hv_call_deposit_pages_old(int node, u64 partition_id, u32 num_pages)
>>   {
>>   	struct page **pages, *page;
>>   	int *counts;
>> @@ -29,7 +29,7 @@ int hv_call_deposit_pages(int node, u64 partition_id, u32 num_pages)
>>   	struct hv_deposit_memory *input_page;
>>   	unsigned long flags;
>>
>> -	if (num_pages > HV_DEPOSIT_MAX)
>> +	if (num_pages > HV_DEPOSIT_MAX_OLD)
>>   		return -E2BIG;
>>   	if (!num_pages)
>>   		return 0;
>> @@ -40,7 +40,7 @@ int hv_call_deposit_pages(int node, u64 partition_id, u32 num_pages)
>>   		return -ENOMEM;
>>   	pages = page_address(page);
>>
>> -	counts = kzalloc_objs(int, HV_DEPOSIT_MAX);
>> +	counts = kzalloc_objs(int, HV_DEPOSIT_MAX_OLD);
>>   	if (!counts) {
>>   		free_page((unsigned long)pages);
>>   		return -ENOMEM;
>> @@ -108,10 +108,14 @@ int hv_call_deposit_pages(int node, u64 partition_id, u32 num_pages)
>>   	kfree(counts);
>>   	return ret;
>>   }
>> +
>> +int hv_call_deposit_pages(int node, u64 partition_id, u32 num_pages)
>> +{
>> +	return hv_call_deposit_pages_old(node, partition_id, num_pages);
>> +}
>>   EXPORT_SYMBOL_GPL(hv_call_deposit_pages);
>>
>> -int hv_deposit_memory_node(int node, u64 partition_id,
>> -			   u64 hv_status)
>> +static int __maybe_unused hv_deposit_memory_node_old(int node, u64 partition_id, u64 hv_status)
> 
> I'm not clear on why there is "__maybe_unused" here. If CONFIG_MSHV_ROOT
> isn't defined, this whole file isn't built.

The main 2nd patch adds newer version of the function, so  this _old is
not called anymore, and will generate compiler warning.

Thanks,
-Mukesh


> But it doesn't really matter since Patch 3 of the series deletes the code anyway.
> 
>>   {
>>   	u32 num_pages = 1;
>>
>> @@ -137,7 +141,12 @@ int hv_deposit_memory_node(int node, u64 partition_id,
>>   		hv_status_err(hv_status, "Unexpected!\n");
>>   		return -ENOMEM;
>>   	}
>> -	return hv_call_deposit_pages(node, partition_id, num_pages);
>> +	return hv_call_deposit_pages_old(node, partition_id, num_pages);
>> +}
>> +
>> +int hv_deposit_memory_node(int node, u64 partition_id, u64 hv_status)
>> +{
>> +	return hv_deposit_memory_node_old(node, partition_id, hv_status);
>>   }
>>   EXPORT_SYMBOL_GPL(hv_deposit_memory_node);
>>
>> --
>> 2.51.2.vfs.0.1
>>
> 
> The "__maybe_unused" notwithstanding,
> 
> Reviewed-by: Michael Kelley <mhklinux@outlook.com>

RE: [PATCH V1 1/3] mshv: Rename memory deposit memory functions to _old

[Michael Kelley]

From: Mukesh R <mrathor@linux.microsoft.com> Sent: Tuesday, September 8, 2026 1:43 PM
> 
> On 9/8/26 11:01, Michael Kelley wrote:
> > From: Mukesh R <mrathor@linux.microsoft.com> Sent: Monday, September 7, 2026 7:14 PM

[snip]

> >> -int hv_deposit_memory_node(int node, u64 partition_id,
> >> -			   u64 hv_status)
> >> +static int __maybe_unused hv_deposit_memory_node_old(int node, u64 partition_id, u64 hv_status)
> >
> > I'm not clear on why there is "__maybe_unused" here. If CONFIG_MSHV_ROOT
> > isn't defined, this whole file isn't built.
> 
> The main 2nd patch adds newer version of the function, so  this _old is
> not called anymore, and will generate compiler warning.
> 
> Thanks,
> -Mukesh
> 

OK, right. That makes sense.

Thx,
Michael

[PATCH V1 2/3] mshv: Redesign hypervisor memory deposit logic

[Mukesh R]

The current memory deposit implementation has a few issues and bugs:
 o It is very slow
 o Contiguous range requirement is broken, and is critical bug
 o An incorrect assumption is made that contiguous memory size would
   always be power of 2.
 o Two pages are allocated, only one is really needed. This adds to
   overhead.
 o For a 512 page deposit, the allocation is split into two: one for 511
   and second for 1. Thus, an order 9 allocation never happens. A
   contiguous 2M range, if possible, significantly improves performance
   in the hypervisor.
 o Since a page is already allocated to collect the frames, there is
   not really a need to use per cpu input page, and hence avoid local
   irq disable.
 o In hv_call_deposit_pages(), in case of error, under err_free_allocations
   label, all pages are freed without checking status to see if some pages
   were deposited. This is a critical bug as it would free pages that hyp
   may be using.

All of above is addressed by:
 o Allocate 2M by default, this is the recommendation from the hypervisor
   team, and greatly improves performance.
 o Always start with a full 2M range allocation, thus getting contiguous if
   available. In cases where possible, the deposits are much faster.
 o Allocate only one page in the deposit function and collect 511 pfns
   there. Just use a local variable for last pfn.
 o Use the page as input to hypercall. Since this page is locally allocated,
   irq disable can be avoided helping speed up the deposit.
 o Fix the physical contiguous memory requirement.
 o Lastly, remove pre-deposits hv_call_create_vp() and
   hv_call_initialize_partition() as they were removed internally while
   ago, most likely because they didn't help much.

Signed-off-by: Mukesh R <mrathor@linux.microsoft.com>
---
 drivers/hv/hv_proc.c           | 198 +++++++++++++++++++++++++++++----
 drivers/hv/mshv_root_hv_call.c |  10 +-
 include/asm-generic/mshyperv.h |   5 -
 3 files changed, 179 insertions(+), 34 deletions(-)

diff --git a/drivers/hv/hv_proc.c b/drivers/hv/hv_proc.c
index 57864bb5bcd8..0ebede0bc8b4 100644
--- a/drivers/hv/hv_proc.c
+++ b/drivers/hv/hv_proc.c
@@ -9,6 +9,184 @@
 #include <linux/export.h>
 #include <asm/mshyperv.h>
 
+#define HV_DEPOSIT_MAX 512
+#define HV_DEPOSIT_INP_MAX ((HV_HYP_PAGE_SIZE -  \
+	offsetof(struct hv_deposit_memory, gpa_page_list)) / sizeof(u64))
+
+/*
+ * Allocate free pages for deposit to hypervisor. pfna[] must be large enough
+ * to hold HV_DEPOSIT_INP_MAX (511) pages. If num_pages is 512, return last
+ * pfn in lastpfn. If @single, then it must be a single allocation (not split
+ * over multiple contiguous ranges).
+ *
+ * Returns: number of pages allocated or -ENOMEM
+ */
+static int hv_alloc_dep_pages(int node, u64 *pfna, u64 *lastpfnp, int num_pages,
+			      bool single)
+{
+	struct page *page;
+	int num_allocd, count = 0;
+
+	/* Published ABI, enforce its immutability. */
+	BUILD_BUG_ON(HV_DEPOSIT_INP_MAX != 511);
+
+	if (num_pages > HV_DEPOSIT_MAX ||
+	    (num_pages == HV_DEPOSIT_MAX && lastpfnp == NULL))
+		return -EINVAL;
+
+	while (num_pages) {
+		/* Find highest order we can actually allocate */
+		int order = 31 - __builtin_clz(num_pages);
+		gfp_t gfp_flags = GFP_KERNEL;
+
+		if (!single)
+			gfp_flags |= __GFP_NOWARN;
+
+		while (1) {
+			page = alloc_pages_node(node, gfp_flags, order);
+			if (page || order == 0 || single)
+				break;
+
+			order--;
+		}
+
+		if (page == NULL)
+			break;
+
+		split_page(page, order);
+		num_allocd = 1 << order;
+		num_pages -= num_allocd;
+
+		while (num_allocd && count < HV_DEPOSIT_INP_MAX) {
+			pfna[count++] = page_to_pfn(page++);
+			num_allocd--;
+		}
+
+		if (num_allocd-- && count == HV_DEPOSIT_INP_MAX) {
+			*lastpfnp = page_to_pfn(page);
+			count++;
+			break;
+		}
+	}
+
+	return count ? count : -ENOMEM;
+}
+
+/*
+ * Deposit memory in the hypervisor. Even if @contiguous is false, a contiguous
+ * 2M worth of pfns is utmost desired for performance reasons. But short of
+ * that, we deposit whatever contiguous chunks we can get. If @contiguous is
+ * true, then the entire range has to be physically contiguous. Note, in that
+ * case, upon withdrawl, hypervisor could return any page in between the range,
+ * so we must split that also. Lastly, HV_MAX_CONTIGUOUS_ALLOCATION_PAGES is
+ * not guaranteed to always be power of 2.
+ */
+static int hv_call_deposit_pages(int node, u64 partition_id, bool contiguous)
+{
+	struct hv_deposit_memory *hc_input;
+	int i, rc, num_pages;
+	u64 status, *pfna, lastpfn = 0;
+	bool trunc_extra = false;
+
+	BUILD_BUG_ON(HV_MAX_CONTIGUOUS_ALLOCATION_PAGES > HV_DEPOSIT_MAX);
+
+	if (contiguous) {
+		num_pages = roundup_pow_of_two(
+					  HV_MAX_CONTIGUOUS_ALLOCATION_PAGES);
+		trunc_extra = num_pages != HV_MAX_CONTIGUOUS_ALLOCATION_PAGES;
+	} else {
+		num_pages = HV_DEPOSIT_MAX;
+	}
+
+	hc_input = (struct hv_deposit_memory *)get_zeroed_page(GFP_KERNEL);
+	if (hc_input == NULL)
+		return -ENOMEM;
+
+	hc_input->partition_id = partition_id;
+	pfna = hc_input->gpa_page_list;
+
+	rc = hv_alloc_dep_pages(node, pfna, &lastpfn, num_pages, contiguous);
+	if (rc < 0)
+		goto out_free;
+
+	num_pages = rc;
+	if (num_pages > HV_DEPOSIT_INP_MAX)
+		num_pages = HV_DEPOSIT_INP_MAX;
+
+	if (contiguous && trunc_extra) {
+		for (i = HV_MAX_CONTIGUOUS_ALLOCATION_PAGES; i < num_pages; i++)
+			__free_page(pfn_to_page(pfna[i]));
+
+		if (lastpfn) {
+			__free_page(pfn_to_page(lastpfn));
+			lastpfn = 0;
+		}
+
+		num_pages = HV_MAX_CONTIGUOUS_ALLOCATION_PAGES;
+	}
+
+	/* We are not using hyperv_pcpu_input_arg, so no need to disable */
+
+	status = hv_do_rep_hypercall(HVCALL_DEPOSIT_MEMORY, num_pages,
+				     0, hc_input, NULL);
+	if (!hv_result_success(status))
+		goto err_free_dep_pages;
+
+	if (lastpfn) {
+		hc_input->gpa_page_list[0] = lastpfn;
+		status = hv_do_rep_hypercall(HVCALL_DEPOSIT_MEMORY, 1, 0,
+					     hc_input, NULL);
+
+		if (!hv_result_success(status)) {
+			if (contiguous)
+				goto err_free_dep_pages;
+
+			/* We deposited lot earlier, so give it a go */
+			__free_page(pfn_to_page(lastpfn));
+		}
+	}
+
+	free_page((unsigned long)hc_input);
+	return 0;
+
+err_free_dep_pages:
+	hv_status_err(status, "\n");
+	rc = hv_result_to_errno(status);
+
+	for (i = hv_repcomp(status); i < num_pages; i++)
+		__free_page(pfn_to_page(pfna[i]));
+	if (lastpfn)
+		__free_page(pfn_to_page(lastpfn));
+
+out_free:
+	free_page((unsigned long)hc_input);
+	return rc;
+}
+
+int hv_deposit_memory_node(int node, u64 pt_id, u64 hv_status)
+{
+	int result = hv_result(hv_status);
+	bool contiguous = false;
+
+	if (result == HV_STATUS_INSUFFICIENT_ROOT_MEMORY ||
+	    result == HV_STATUS_INSUFFICIENT_CONTIGUOUS_ROOT_MEMORY) {
+		if (!hv_root_partition()) {
+			hv_status_err(hv_status,
+				      "Unexpected root memory deposit\n");
+			return -EINVAL;
+		}
+
+		pt_id = HV_PARTITION_ID_SELF;
+	}
+
+	if (result == HV_STATUS_INSUFFICIENT_CONTIGUOUS_MEMORY ||
+	    result == HV_STATUS_INSUFFICIENT_CONTIGUOUS_ROOT_MEMORY)
+		contiguous = true;
+
+	return hv_call_deposit_pages(node, pt_id, contiguous);
+}
+EXPORT_SYMBOL_GPL(hv_deposit_memory_node);
+
 /*
  * See struct hv_deposit_memory. The first u64 is partition ID, the rest
  * are GPAs.
@@ -109,12 +287,6 @@ static int hv_call_deposit_pages_old(int node, u64 partition_id, u32 num_pages)
 	return ret;
 }
 
-int hv_call_deposit_pages(int node, u64 partition_id, u32 num_pages)
-{
-	return hv_call_deposit_pages_old(node, partition_id, num_pages);
-}
-EXPORT_SYMBOL_GPL(hv_call_deposit_pages);
-
 static int __maybe_unused hv_deposit_memory_node_old(int node, u64 partition_id, u64 hv_status)
 {
 	u32 num_pages = 1;
@@ -144,12 +316,6 @@ static int __maybe_unused hv_deposit_memory_node_old(int node, u64 partition_id,
 	return hv_call_deposit_pages_old(node, partition_id, num_pages);
 }
 
-int hv_deposit_memory_node(int node, u64 partition_id, u64 hv_status)
-{
-	return hv_deposit_memory_node_old(node, partition_id, hv_status);
-}
-EXPORT_SYMBOL_GPL(hv_deposit_memory_node);
-
 bool hv_result_needs_memory(u64 status)
 {
 	switch (hv_result(status)) {
@@ -212,14 +378,6 @@ int hv_call_create_vp(int node, u64 partition_id, u32 vp_index, u32 flags)
 	unsigned long irq_flags;
 	int ret = 0;
 
-	/* Root VPs don't seem to need pages deposited */
-	if (partition_id != hv_current_partition_id) {
-		/* The value 90 is empirically determined. It may change. */
-		ret = hv_call_deposit_pages(node, partition_id, 90);
-		if (ret)
-			return ret;
-	}
-
 	do {
 		local_irq_save(irq_flags);
 
diff --git a/drivers/hv/mshv_root_hv_call.c b/drivers/hv/mshv_root_hv_call.c
index cb55d4d4be2e..b8d199f95299 100644
--- a/drivers/hv/mshv_root_hv_call.c
+++ b/drivers/hv/mshv_root_hv_call.c
@@ -15,8 +15,6 @@
 #include "mshv_root.h"
 
 /* Determined empirically */
-#define HV_INIT_PARTITION_DEPOSIT_PAGES 208
-#define HV_MAP_GPA_DEPOSIT_PAGES	256
 #define HV_UMAP_GPA_PAGES		512
 
 #define HV_PAGE_COUNT_2M_ALIGNED(pg_count) (!((pg_count) & (0x200 - 1)))
@@ -140,11 +138,6 @@ int hv_call_initialize_partition(u64 partition_id)
 
 	input.partition_id = partition_id;
 
-	ret = hv_call_deposit_pages(NUMA_NO_NODE, partition_id,
-				    HV_INIT_PARTITION_DEPOSIT_PAGES);
-	if (ret)
-		return ret;
-
 	do {
 		status = hv_do_fast_hypercall8(HVCALL_INITIALIZE_PARTITION,
 					       *(u64 *)&input);
@@ -248,8 +241,7 @@ static int hv_do_map_gpa_hcall(u64 partition_id, u64 gfn, u64 page_struct_count,
 		completed = hv_repcomp(status);
 
 		if (hv_result_needs_memory(status)) {
-			ret = hv_call_deposit_pages(NUMA_NO_NODE, partition_id,
-						    HV_MAP_GPA_DEPOSIT_PAGES);
+			ret = hv_deposit_memory(partition_id, status);
 			if (ret)
 				break;
 
diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h
index bf601d67cecb..c16abaecb65e 100644
--- a/include/asm-generic/mshyperv.h
+++ b/include/asm-generic/mshyperv.h
@@ -345,7 +345,6 @@ static inline bool hv_parent_partition(void)
 
 bool hv_result_needs_memory(u64 status);
 int hv_deposit_memory_node(int node, u64 partition_id, u64 status);
-int hv_call_deposit_pages(int node, u64 partition_id, u32 num_pages);
 int hv_call_add_logical_proc(int node, u32 lp_index, u32 acpi_id);
 int hv_call_notify_all_processors_started(void);
 bool hv_lp_exists(u32 lp_index);
@@ -360,10 +359,6 @@ static inline int hv_deposit_memory_node(int node, u64 partition_id, u64 status)
 {
 	return -EOPNOTSUPP;
 }
-static inline int hv_call_deposit_pages(int node, u64 partition_id, u32 num_pages)
-{
-	return -EOPNOTSUPP;
-}
 static inline int hv_call_add_logical_proc(int node, u32 lp_index, u32 acpi_id)
 {
 	return -EOPNOTSUPP;
-- 
2.51.2.vfs.0.1

RE: [PATCH V1 2/3] mshv: Redesign hypervisor memory deposit logic

[Michael Kelley]

From: Mukesh R <mrathor@linux.microsoft.com> Sent: Monday, September 7, 2026 7:14 PM
> 
> The current memory deposit implementation has a few issues and bugs:
>  o It is very slow
>  o Contiguous range requirement is broken, and is critical bug
>  o An incorrect assumption is made that contiguous memory size would
>    always be power of 2.

Is this HV_MAX_CONTIGUOUS_ALLOCATION_PAGES? That value is
defined as a constant in hvhdk_mini.h. So is there a possibility that
the constant will change in the future, or in some new variation of
the overall environment?

>  o Two pages are allocated, only one is really needed. This adds to
>    overhead.
>  o For a 512 page deposit, the allocation is split into two: one for 511
>    and second for 1. Thus, an order 9 allocation never happens. A
>    contiguous 2M range, if possible, significantly improves performance
>    in the hypervisor.
>  o Since a page is already allocated to collect the frames, there is
>    not really a need to use per cpu input page, and hence avoid local
>    irq disable.
>  o In hv_call_deposit_pages(), in case of error, under err_free_allocations
>    label, all pages are freed without checking status to see if some pages
>    were deposited. This is a critical bug as it would free pages that hyp
>    may be using.
> 
> All of above is addressed by:
>  o Allocate 2M by default, this is the recommendation from the hypervisor
>    team, and greatly improves performance.

Can you be more specific about "improves performance"? Is the
improvement on the guest side, or on the hypervisor side? And what's
the key leverage point in improving performance, regardless of which
side? It might be helpful to record this for future reference to prevent
a change from being made that unknowingly hurts the key leverage
point.

>  o Always start with a full 2M range allocation, thus getting contiguous if
>    available. In cases where possible, the deposits are much faster.

Is this faster because the hypervisor can consume the pages faster
if they are contiguous?

>  o Allocate only one page in the deposit function and collect 511 pfns
>    there. Just use a local variable for last pfn.
>  o Use the page as input to hypercall. Since this page is locally allocated,
>    irq disable can be avoided helping speed up the deposit.
>  o Fix the physical contiguous memory requirement.
>  o Lastly, remove pre-deposits hv_call_create_vp() and
>    hv_call_initialize_partition() as they were removed internally while
>    ago, most likely because they didn't help much.
> 
> Signed-off-by: Mukesh R <mrathor@linux.microsoft.com>
> ---
>  drivers/hv/hv_proc.c           | 198 +++++++++++++++++++++++++++++----
>  drivers/hv/mshv_root_hv_call.c |  10 +-
>  include/asm-generic/mshyperv.h |   5 -
>  3 files changed, 179 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/hv/hv_proc.c b/drivers/hv/hv_proc.c
> index 57864bb5bcd8..0ebede0bc8b4 100644
> --- a/drivers/hv/hv_proc.c
> +++ b/drivers/hv/hv_proc.c
> @@ -9,6 +9,184 @@
>  #include <linux/export.h>
>  #include <asm/mshyperv.h>
> 
> +#define HV_DEPOSIT_MAX 512
> +#define HV_DEPOSIT_INP_MAX ((HV_HYP_PAGE_SIZE -  \
> +	offsetof(struct hv_deposit_memory, gpa_page_list)) / sizeof(u64))
> +
> +/*
> + * Allocate free pages for deposit to hypervisor. pfna[] must be large enough
> + * to hold HV_DEPOSIT_INP_MAX (511) pages. If num_pages is 512, return last
> + * pfn in lastpfn. If @single, then it must be a single allocation (not split
> + * over multiple contiguous ranges).
> + *
> + * Returns: number of pages allocated or -ENOMEM
> + */
> +static int hv_alloc_dep_pages(int node, u64 *pfna, u64 *lastpfnp, int num_pages,
> +			      bool single)
> +{
> +	struct page *page;
> +	int num_allocd, count = 0;
> +
> +	/* Published ABI, enforce its immutability. */
> +	BUILD_BUG_ON(HV_DEPOSIT_INP_MAX != 511);
> +
> +	if (num_pages > HV_DEPOSIT_MAX ||
> +	    (num_pages == HV_DEPOSIT_MAX && lastpfnp == NULL))
> +		return -EINVAL;
> +
> +	while (num_pages) {
> +		/* Find highest order we can actually allocate */
> +		int order = 31 - __builtin_clz(num_pages);

Outside of the kernel source file da9062-regulator.c and the definition
of fls(), there are only about 14 direct uses of __builtin_clz() in the
kernel. By contrast fls() is used more than 700 times. Using __builtin_clz()
works, but you might consider using fls() instead just to be on the more
common path.

> +		gfp_t gfp_flags = GFP_KERNEL;
> +
> +		if (!single)
> +			gfp_flags |= __GFP_NOWARN;
> +
> +		while (1) {
> +			page = alloc_pages_node(node, gfp_flags, order);
> +			if (page || order == 0 || single)
> +				break;
> +
> +			order--;
> +		}
> +
> +		if (page == NULL)

Nit: I've always thought kernel style has a slight preference for "!page"
instead of "page == NULL". And indeed, a few lines above you test
just "page". Not a big deal, but when there are inconsistencies I wonder
if there's a reason ....

> +			break;
> +
> +		split_page(page, order);
> +		num_allocd = 1 << order;
> +		num_pages -= num_allocd;
> +
> +		while (num_allocd && count < HV_DEPOSIT_INP_MAX) {
> +			pfna[count++] = page_to_pfn(page++);
> +			num_allocd--;
> +		}
> +
> +		if (num_allocd-- && count == HV_DEPOSIT_INP_MAX) {
> +			*lastpfnp = page_to_pfn(page);
> +			count++;
> +			break;
> +		}
> +	}
> +
> +	return count ? count : -ENOMEM;
> +}
> +
> +/*
> + * Deposit memory in the hypervisor. Even if @contiguous is false, a contiguous
> + * 2M worth of pfns is utmost desired for performance reasons. But short of
> + * that, we deposit whatever contiguous chunks we can get. If @contiguous is
> + * true, then the entire range has to be physically contiguous. Note, in that
> + * case, upon withdrawl, hypervisor could return any page in between the range,
> + * so we must split that also. Lastly, HV_MAX_CONTIGUOUS_ALLOCATION_PAGES is
> + * not guaranteed to always be power of 2.
> + */
> +static int hv_call_deposit_pages(int node, u64 partition_id, bool contiguous)
> +{
> +	struct hv_deposit_memory *hc_input;
> +	int i, rc, num_pages;
> +	u64 status, *pfna, lastpfn = 0;
> +	bool trunc_extra = false;
> +
> +	BUILD_BUG_ON(HV_MAX_CONTIGUOUS_ALLOCATION_PAGES > HV_DEPOSIT_MAX);
> +
> +	if (contiguous) {
> +		num_pages = roundup_pow_of_two(
> +				 HV_MAX_CONTIGUOUS_ALLOCATION_PAGES);
> +		trunc_extra = num_pages != HV_MAX_CONTIGUOUS_ALLOCATION_PAGES;
> +	} else {
> +		num_pages = HV_DEPOSIT_MAX;
> +	}
> +
> +	hc_input = (struct hv_deposit_memory *)get_zeroed_page(GFP_KERNEL);

Mike Rapoport has a kernel-wide effort underway to replace
__get_free_page() with kmalloc() and get_zeroed_page() with kzalloc().
See [1] for one example of the many patches he has submitted. The
commit message has a short explanation. Going with kzalloc() here
would probably avoid a future change.

[1] https://lore.kernel.org/lkml/20260830-tty-v1-1-0cbe6170649b@kernel.org/

> +	if (hc_input == NULL)
> +		return -ENOMEM;
> +
> +	hc_input->partition_id = partition_id;
> +	pfna = hc_input->gpa_page_list;
> +
> +	rc = hv_alloc_dep_pages(node, pfna, &lastpfn, num_pages, contiguous);
> +	if (rc < 0)
> +		goto out_free;
> +
> +	num_pages = rc;
> +	if (num_pages > HV_DEPOSIT_INP_MAX)
> +		num_pages = HV_DEPOSIT_INP_MAX;
> +
> +	if (contiguous && trunc_extra) {
> +		for (i = HV_MAX_CONTIGUOUS_ALLOCATION_PAGES; i < num_pages; i++)
> +			__free_page(pfn_to_page(pfna[i]));
> +
> +		if (lastpfn) {
> +			__free_page(pfn_to_page(lastpfn));
> +			lastpfn = 0;
> +		}
> +
> +		num_pages = HV_MAX_CONTIGUOUS_ALLOCATION_PAGES;
> +	}
> +
> +	/* We are not using hyperv_pcpu_input_arg, so no need to disable */

"disable interrupts"?

> +
> +	status = hv_do_rep_hypercall(HVCALL_DEPOSIT_MEMORY, num_pages,
> +				     0, hc_input, NULL);
> +	if (!hv_result_success(status))
> +		goto err_free_dep_pages;
> +
> +	if (lastpfn) {
> +		hc_input->gpa_page_list[0] = lastpfn;
> +		status = hv_do_rep_hypercall(HVCALL_DEPOSIT_MEMORY, 1, 0,
> +					     hc_input, NULL);

So if a full 2 MiB contiguous is allocated, it's OK to deposit the
first 511 pages and then do a 2nd hypercall to deposit the last
page? Does the hypervisor figure out that the 2 MiB is contiguous
and map it with a single 2 MiB PTE to get the mapping perf benefit?
I realize there's not any other choice given the limitation of 1 page
of hypercall input, but I just wondered.

> +
> +		if (!hv_result_success(status)) {
> +			if (contiguous)
> +				goto err_free_dep_pages;
> +
> +			/* We deposited lot earlier, so give it a go */
> +			__free_page(pfn_to_page(lastpfn));
> +		}
> +	}
> +
> +	free_page((unsigned long)hc_input);
> +	return 0;
> +
> +err_free_dep_pages:
> +	hv_status_err(status, "\n");
> +	rc = hv_result_to_errno(status);
> +
> +	for (i = hv_repcomp(status); i < num_pages; i++)
> +		__free_page(pfn_to_page(pfna[i]));
> +	if (lastpfn)
> +		__free_page(pfn_to_page(lastpfn));
> +
> +out_free:
> +	free_page((unsigned long)hc_input);
> +	return rc;
> +}
> +
> +int hv_deposit_memory_node(int node, u64 pt_id, u64 hv_status)
> +{
> +	int result = hv_result(hv_status);
> +	bool contiguous = false;
> +
> +	if (result == HV_STATUS_INSUFFICIENT_ROOT_MEMORY ||
> +	    result == HV_STATUS_INSUFFICIENT_CONTIGUOUS_ROOT_MEMORY) {
> +		if (!hv_root_partition()) {
> +			hv_status_err(hv_status,
> +				      "Unexpected root memory deposit\n");
> +			return -EINVAL;
> +		}
> +
> +		pt_id = HV_PARTITION_ID_SELF;
> +	}
> +
> +	if (result == HV_STATUS_INSUFFICIENT_CONTIGUOUS_MEMORY ||
> +	    result == HV_STATUS_INSUFFICIENT_CONTIGUOUS_ROOT_MEMORY)
> +		contiguous = true;
> +
> +	return hv_call_deposit_pages(node, pt_id, contiguous);
> +}
> +EXPORT_SYMBOL_GPL(hv_deposit_memory_node);
> +
>  /*
>   * See struct hv_deposit_memory. The first u64 is partition ID, the rest
>   * are GPAs.
> @@ -109,12 +287,6 @@ static int hv_call_deposit_pages_old(int node, u64 partition_id, u32 num_pages)
>  	return ret;
>  }
> 
> -int hv_call_deposit_pages(int node, u64 partition_id, u32 num_pages)
> -{
> -	return hv_call_deposit_pages_old(node, partition_id, num_pages);
> -}
> -EXPORT_SYMBOL_GPL(hv_call_deposit_pages);
> -
>  static int __maybe_unused hv_deposit_memory_node_old(int node, u64 partition_id, u64 hv_status)
>  {
>  	u32 num_pages = 1;
> @@ -144,12 +316,6 @@ static int __maybe_unused hv_deposit_memory_node_old(int node, u64 partition_id,
>  	return hv_call_deposit_pages_old(node, partition_id, num_pages);
>  }
> 
> -int hv_deposit_memory_node(int node, u64 partition_id, u64 hv_status)
> -{
> -	return hv_deposit_memory_node_old(node, partition_id, hv_status);
> -}
> -EXPORT_SYMBOL_GPL(hv_deposit_memory_node);
> -
>  bool hv_result_needs_memory(u64 status)
>  {
>  	switch (hv_result(status)) {
> @@ -212,14 +378,6 @@ int hv_call_create_vp(int node, u64 partition_id, u32 vp_index, u32 flags)
>  	unsigned long irq_flags;
>  	int ret = 0;
> 
> -	/* Root VPs don't seem to need pages deposited */
> -	if (partition_id != hv_current_partition_id) {
> -		/* The value 90 is empirically determined. It may change. */
> -		ret = hv_call_deposit_pages(node, partition_id, 90);
> -		if (ret)
> -			return ret;
> -	}
> -
>  	do {
>  		local_irq_save(irq_flags);
> 
> diff --git a/drivers/hv/mshv_root_hv_call.c b/drivers/hv/mshv_root_hv_call.c
> index cb55d4d4be2e..b8d199f95299 100644
> --- a/drivers/hv/mshv_root_hv_call.c
> +++ b/drivers/hv/mshv_root_hv_call.c
> @@ -15,8 +15,6 @@
>  #include "mshv_root.h"
> 
>  /* Determined empirically */
> -#define HV_INIT_PARTITION_DEPOSIT_PAGES 208
> -#define HV_MAP_GPA_DEPOSIT_PAGES	256
>  #define HV_UMAP_GPA_PAGES		512
> 
>  #define HV_PAGE_COUNT_2M_ALIGNED(pg_count) (!((pg_count) & (0x200 - 1)))
> @@ -140,11 +138,6 @@ int hv_call_initialize_partition(u64 partition_id)
> 
>  	input.partition_id = partition_id;
> 
> -	ret = hv_call_deposit_pages(NUMA_NO_NODE, partition_id,
> -				    HV_INIT_PARTITION_DEPOSIT_PAGES);
> -	if (ret)
> -		return ret;
> -
>  	do {
>  		status = hv_do_fast_hypercall8(HVCALL_INITIALIZE_PARTITION,
>  					       *(u64 *)&input);
> @@ -248,8 +241,7 @@ static int hv_do_map_gpa_hcall(u64 partition_id, u64 gfn, u64 page_struct_count,
>  		completed = hv_repcomp(status);
> 
>  		if (hv_result_needs_memory(status)) {
> -			ret = hv_call_deposit_pages(NUMA_NO_NODE, partition_id,
> -						    HV_MAP_GPA_DEPOSIT_PAGES);
> +			ret = hv_deposit_memory(partition_id, status);
>  			if (ret)
>  				break;
> 
> diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h
> index bf601d67cecb..c16abaecb65e 100644
> --- a/include/asm-generic/mshyperv.h
> +++ b/include/asm-generic/mshyperv.h
> @@ -345,7 +345,6 @@ static inline bool hv_parent_partition(void)
> 
>  bool hv_result_needs_memory(u64 status);
>  int hv_deposit_memory_node(int node, u64 partition_id, u64 status);
> -int hv_call_deposit_pages(int node, u64 partition_id, u32 num_pages);
>  int hv_call_add_logical_proc(int node, u32 lp_index, u32 acpi_id);
>  int hv_call_notify_all_processors_started(void);
>  bool hv_lp_exists(u32 lp_index);
> @@ -360,10 +359,6 @@ static inline int hv_deposit_memory_node(int node, u64 partition_id, u64 status)
>  {
>  	return -EOPNOTSUPP;
>  }
> -static inline int hv_call_deposit_pages(int node, u64 partition_id, u32 num_pages)
> -{
> -	return -EOPNOTSUPP;
> -}
>  static inline int hv_call_add_logical_proc(int node, u32 lp_index, u32 acpi_id)
>  {
>  	return -EOPNOTSUPP;
> --
> 2.51.2.vfs.0.1
> 

Re: [PATCH V1 2/3] mshv: Redesign hypervisor memory deposit logic

[Mukesh R]

On 9/8/26 11:02, Michael Kelley wrote:
> From: Mukesh R <mrathor@linux.microsoft.com> Sent: Monday, September 7, 2026 7:14 PM
>>
>> The current memory deposit implementation has a few issues and bugs:
>>   o It is very slow
>>   o Contiguous range requirement is broken, and is critical bug
>>   o An incorrect assumption is made that contiguous memory size would
>>     always be power of 2.
> 
> Is this HV_MAX_CONTIGUOUS_ALLOCATION_PAGES? That value is
> defined as a constant in hvhdk_mini.h. So is there a possibility that
> the constant will change in the future, or in some new variation of
> the overall environment?

Yes, I was told it could change in future. It is also not guaranteed to
be power of 2 (even tho it is currently).

>>   o Two pages are allocated, only one is really needed. This adds to
>>     overhead.
>>   o For a 512 page deposit, the allocation is split into two: one for 511
>>     and second for 1. Thus, an order 9 allocation never happens. A
>>     contiguous 2M range, if possible, significantly improves performance
>>     in the hypervisor.
>>   o Since a page is already allocated to collect the frames, there is
>>     not really a need to use per cpu input page, and hence avoid local
>>     irq disable.
>>   o In hv_call_deposit_pages(), in case of error, under err_free_allocations
>>     label, all pages are freed without checking status to see if some pages
>>     were deposited. This is a critical bug as it would free pages that hyp
>>     may be using.
>>
>> All of above is addressed by:
>>   o Allocate 2M by default, this is the recommendation from the hypervisor
>>     team, and greatly improves performance.
> 
> Can you be more specific about "improves performance"? Is the
> improvement on the guest side, or on the hypervisor side? And what's
> the key leverage point in improving performance, regardless of which
> side? It might be helpful to record this for future reference to prevent
> a change from being made that unknowingly hurts the key leverage
> point.

Well, overall improvement I'd say. Lesser HV_STATUS_INSUFFICIENT_MEMORY
means lesser interruptions means faster non-deposit hypercalls. With
larger deposits, it will not come back as often with insufficient memory.
If it only uses 1M say out of the 2, next time it will reuse the remaining
meg before coming back and asking for more. IOW, incomplete hypercalls
repeatedly coming back asking for more ram add lot of overhead.
Eg. the guest setup part reduces from 8 sec to 1 sec with this, I
don't have further breakdown of which ioctls.

>>   o Always start with a full 2M range allocation, thus getting contiguous if
>>     available. In cases where possible, the deposits are much faster.
> 
> Is this faster because the hypervisor can consume the pages faster
> if they are contiguous?

I didn't ask but my guess is that it can map 2M page in its page
tables, so all the benefits of large pages: faster L2 mapping, less
tlb overhead, etc..

>>   o Allocate only one page in the deposit function and collect 511 pfns
>>     there. Just use a local variable for last pfn.
>>   o Use the page as input to hypercall. Since this page is locally allocated,
>>     irq disable can be avoided helping speed up the deposit.
>>   o Fix the physical contiguous memory requirement.
>>   o Lastly, remove pre-deposits hv_call_create_vp() and
>>     hv_call_initialize_partition() as they were removed internally while
>>     ago, most likely because they didn't help much.
>>
>> Signed-off-by: Mukesh R <mrathor@linux.microsoft.com>
>> ---
>>   drivers/hv/hv_proc.c           | 198 +++++++++++++++++++++++++++++----
>>   drivers/hv/mshv_root_hv_call.c |  10 +-
>>   include/asm-generic/mshyperv.h |   5 -
>>   3 files changed, 179 insertions(+), 34 deletions(-)
>>
>> diff --git a/drivers/hv/hv_proc.c b/drivers/hv/hv_proc.c
>> index 57864bb5bcd8..0ebede0bc8b4 100644
>> --- a/drivers/hv/hv_proc.c
>> +++ b/drivers/hv/hv_proc.c
>> @@ -9,6 +9,184 @@
>>   #include <linux/export.h>
>>   #include <asm/mshyperv.h>
>>
>> +#define HV_DEPOSIT_MAX 512
>> +#define HV_DEPOSIT_INP_MAX ((HV_HYP_PAGE_SIZE -  \
>> +	offsetof(struct hv_deposit_memory, gpa_page_list)) / sizeof(u64))
>> +
>> +/*
>> + * Allocate free pages for deposit to hypervisor. pfna[] must be large enough
>> + * to hold HV_DEPOSIT_INP_MAX (511) pages. If num_pages is 512, return last
>> + * pfn in lastpfn. If @single, then it must be a single allocation (not split
>> + * over multiple contiguous ranges).
>> + *
>> + * Returns: number of pages allocated or -ENOMEM
>> + */
>> +static int hv_alloc_dep_pages(int node, u64 *pfna, u64 *lastpfnp, int num_pages,
>> +			      bool single)
>> +{
>> +	struct page *page;
>> +	int num_allocd, count = 0;
>> +
>> +	/* Published ABI, enforce its immutability. */
>> +	BUILD_BUG_ON(HV_DEPOSIT_INP_MAX != 511);
>> +
>> +	if (num_pages > HV_DEPOSIT_MAX ||
>> +	    (num_pages == HV_DEPOSIT_MAX && lastpfnp == NULL))
>> +		return -EINVAL;
>> +
>> +	while (num_pages) {
>> +		/* Find highest order we can actually allocate */
>> +		int order = 31 - __builtin_clz(num_pages);
> 
> Outside of the kernel source file da9062-regulator.c and the definition
> of fls(), there are only about 14 direct uses of __builtin_clz() in the
> kernel. By contrast fls() is used more than 700 times. Using __builtin_clz()
> works, but you might consider using fls() instead just to be on the more
> common path.

Well, i just used things from existing code in the current function
hv_call_deposit_pages()...  So, it's not new code. Frankly, i don't
want to risk changing something that works unless it is necessary.

>> +		gfp_t gfp_flags = GFP_KERNEL;
>> +
>> +		if (!single)
>> +			gfp_flags |= __GFP_NOWARN;
>> +
>> +		while (1) {
>> +			page = alloc_pages_node(node, gfp_flags, order);
>> +			if (page || order == 0 || single)
>> +				break;
>> +
>> +			order--;
>> +		}
>> +
>> +		if (page == NULL)
> 
> Nit: I've always thought kernel style has a slight preference for "!page"
> instead of "page == NULL". And indeed, a few lines above you test
> just "page". Not a big deal, but when there are inconsistencies I wonder
> if there's a reason ....

My aging eyes and brain prefer (x == NULL) :)... Earlier one  is existing
line from previous code.

>> +			break;
>> +
>> +		split_page(page, order);
>> +		num_allocd = 1 << order;
>> +		num_pages -= num_allocd;
>> +
>> +		while (num_allocd && count < HV_DEPOSIT_INP_MAX) {
>> +			pfna[count++] = page_to_pfn(page++);
>> +			num_allocd--;
>> +		}
>> +
>> +		if (num_allocd-- && count == HV_DEPOSIT_INP_MAX) {
>> +			*lastpfnp = page_to_pfn(page);
>> +			count++;
>> +			break;
>> +		}
>> +	}
>> +
>> +	return count ? count : -ENOMEM;
>> +}
>> +
>> +/*
>> + * Deposit memory in the hypervisor. Even if @contiguous is false, a contiguous
>> + * 2M worth of pfns is utmost desired for performance reasons. But short of
>> + * that, we deposit whatever contiguous chunks we can get. If @contiguous is
>> + * true, then the entire range has to be physically contiguous. Note, in that
>> + * case, upon withdrawl, hypervisor could return any page in between the range,
>> + * so we must split that also. Lastly, HV_MAX_CONTIGUOUS_ALLOCATION_PAGES is
>> + * not guaranteed to always be power of 2.
>> + */
>> +static int hv_call_deposit_pages(int node, u64 partition_id, bool contiguous)
>> +{
>> +	struct hv_deposit_memory *hc_input;
>> +	int i, rc, num_pages;
>> +	u64 status, *pfna, lastpfn = 0;
>> +	bool trunc_extra = false;
>> +
>> +	BUILD_BUG_ON(HV_MAX_CONTIGUOUS_ALLOCATION_PAGES > HV_DEPOSIT_MAX);
>> +
>> +	if (contiguous) {
>> +		num_pages = roundup_pow_of_two(
>> +				 HV_MAX_CONTIGUOUS_ALLOCATION_PAGES);
>> +		trunc_extra = num_pages != HV_MAX_CONTIGUOUS_ALLOCATION_PAGES;
>> +	} else {
>> +		num_pages = HV_DEPOSIT_MAX;
>> +	}
>> +
>> +	hc_input = (struct hv_deposit_memory *)get_zeroed_page(GFP_KERNEL);
> 
> Mike Rapoport has a kernel-wide effort underway to replace
> __get_free_page() with kmalloc() and get_zeroed_page() with kzalloc().
> See [1] for one example of the many patches he has submitted. The
> commit message has a short explanation. Going with kzalloc() here
> would probably avoid a future change.

There might be places where a get_page may not be strictly needed, but
in this case we do for the hypercall (we use it as input page), and
this hypercall can go upto exactly one page. I don't think an operating
system can ever get away without having a get_page() api :). kmalloc()
could add an aligntment parameter, but it would just call get_page
anyways for page aligned requests.

Thanks for the review, BTW, sashiko found a bug where wrong status is
used in an error case, so I'll be sending another version.

Thanks,
-Mukesh

> [1] https://lore.kernel.org/lkml/20260830-tty-v1-1-0cbe6170649b@kernel.org/
> 
>> +	if (hc_input == NULL)
>> +		return -ENOMEM;
>> +
>> +	hc_input->partition_id = partition_id;
>> +	pfna = hc_input->gpa_page_list;
>> +
>> +	rc = hv_alloc_dep_pages(node, pfna, &lastpfn, num_pages, contiguous);
>> +	if (rc < 0)
>> +		goto out_free;
>> +
>> +	num_pages = rc;
>> +	if (num_pages > HV_DEPOSIT_INP_MAX)
>> +		num_pages = HV_DEPOSIT_INP_MAX;
>> +
>> +	if (contiguous && trunc_extra) {
>> +		for (i = HV_MAX_CONTIGUOUS_ALLOCATION_PAGES; i < num_pages; i++)
>> +			__free_page(pfn_to_page(pfna[i]));
>> +
>> +		if (lastpfn) {
>> +			__free_page(pfn_to_page(lastpfn));
>> +			lastpfn = 0;
>> +		}
>> +
>> +		num_pages = HV_MAX_CONTIGUOUS_ALLOCATION_PAGES;
>> +	}
>> +
>> +	/* We are not using hyperv_pcpu_input_arg, so no need to disable */
> 
> "disable interrupts"?
> 
>> +
>> +	status = hv_do_rep_hypercall(HVCALL_DEPOSIT_MEMORY, num_pages,
>> +				     0, hc_input, NULL);
>> +	if (!hv_result_success(status))
>> +		goto err_free_dep_pages;
>> +
>> +	if (lastpfn) {
>> +		hc_input->gpa_page_list[0] = lastpfn;
>> +		status = hv_do_rep_hypercall(HVCALL_DEPOSIT_MEMORY, 1, 0,
>> +					     hc_input, NULL);
> 
> So if a full 2 MiB contiguous is allocated, it's OK to deposit the
> first 511 pages and then do a 2nd hypercall to deposit the last
> page? Does the hypervisor figure out that the 2 MiB is contiguous
> and map it with a single 2 MiB PTE to get the mapping perf benefit?
> I realize there's not any other choice given the limitation of 1 page
> of hypercall input, but I just wondered.
> 
>> +
>> +		if (!hv_result_success(status)) {
>> +			if (contiguous)
>> +				goto err_free_dep_pages;
>> +
>> +			/* We deposited lot earlier, so give it a go */
>> +			__free_page(pfn_to_page(lastpfn));
>> +		}
>> +	}
>> +
>> +	free_page((unsigned long)hc_input);
>> +	return 0;
>> +
>> +err_free_dep_pages:
>> +	hv_status_err(status, "\n");
>> +	rc = hv_result_to_errno(status);
>> +
>> +	for (i = hv_repcomp(status); i < num_pages; i++)
>> +		__free_page(pfn_to_page(pfna[i]));
>> +	if (lastpfn)
>> +		__free_page(pfn_to_page(lastpfn));
>> +
>> +out_free:
>> +	free_page((unsigned long)hc_input);
>> +	return rc;
>> +}
>> +
>> +int hv_deposit_memory_node(int node, u64 pt_id, u64 hv_status)
>> +{
>> +	int result = hv_result(hv_status);
>> +	bool contiguous = false;
>> +
>> +	if (result == HV_STATUS_INSUFFICIENT_ROOT_MEMORY ||
>> +	    result == HV_STATUS_INSUFFICIENT_CONTIGUOUS_ROOT_MEMORY) {
>> +		if (!hv_root_partition()) {
>> +			hv_status_err(hv_status,
>> +				      "Unexpected root memory deposit\n");
>> +			return -EINVAL;
>> +		}
>> +
>> +		pt_id = HV_PARTITION_ID_SELF;
>> +	}
>> +
>> +	if (result == HV_STATUS_INSUFFICIENT_CONTIGUOUS_MEMORY ||
>> +	    result == HV_STATUS_INSUFFICIENT_CONTIGUOUS_ROOT_MEMORY)
>> +		contiguous = true;
>> +
>> +	return hv_call_deposit_pages(node, pt_id, contiguous);
>> +}
>> +EXPORT_SYMBOL_GPL(hv_deposit_memory_node);
>> +
>>   /*
>>    * See struct hv_deposit_memory. The first u64 is partition ID, the rest
>>    * are GPAs.
>> @@ -109,12 +287,6 @@ static int hv_call_deposit_pages_old(int node, u64 partition_id, u32 num_pages)
>>   	return ret;
>>   }
>>
>> -int hv_call_deposit_pages(int node, u64 partition_id, u32 num_pages)
>> -{
>> -	return hv_call_deposit_pages_old(node, partition_id, num_pages);
>> -}
>> -EXPORT_SYMBOL_GPL(hv_call_deposit_pages);
>> -
>>   static int __maybe_unused hv_deposit_memory_node_old(int node, u64 partition_id, u64 hv_status)
>>   {
>>   	u32 num_pages = 1;
>> @@ -144,12 +316,6 @@ static int __maybe_unused hv_deposit_memory_node_old(int node, u64 partition_id,
>>   	return hv_call_deposit_pages_old(node, partition_id, num_pages);
>>   }
>>
>> -int hv_deposit_memory_node(int node, u64 partition_id, u64 hv_status)
>> -{
>> -	return hv_deposit_memory_node_old(node, partition_id, hv_status);
>> -}
>> -EXPORT_SYMBOL_GPL(hv_deposit_memory_node);
>> -
>>   bool hv_result_needs_memory(u64 status)
>>   {
>>   	switch (hv_result(status)) {
>> @@ -212,14 +378,6 @@ int hv_call_create_vp(int node, u64 partition_id, u32 vp_index, u32 flags)
>>   	unsigned long irq_flags;
>>   	int ret = 0;
>>
>> -	/* Root VPs don't seem to need pages deposited */
>> -	if (partition_id != hv_current_partition_id) {
>> -		/* The value 90 is empirically determined. It may change. */
>> -		ret = hv_call_deposit_pages(node, partition_id, 90);
>> -		if (ret)
>> -			return ret;
>> -	}
>> -
>>   	do {
>>   		local_irq_save(irq_flags);
>>
>> diff --git a/drivers/hv/mshv_root_hv_call.c b/drivers/hv/mshv_root_hv_call.c
>> index cb55d4d4be2e..b8d199f95299 100644
>> --- a/drivers/hv/mshv_root_hv_call.c
>> +++ b/drivers/hv/mshv_root_hv_call.c
>> @@ -15,8 +15,6 @@
>>   #include "mshv_root.h"
>>
>>   /* Determined empirically */
>> -#define HV_INIT_PARTITION_DEPOSIT_PAGES 208
>> -#define HV_MAP_GPA_DEPOSIT_PAGES	256
>>   #define HV_UMAP_GPA_PAGES		512
>>
>>   #define HV_PAGE_COUNT_2M_ALIGNED(pg_count) (!((pg_count) & (0x200 - 1)))
>> @@ -140,11 +138,6 @@ int hv_call_initialize_partition(u64 partition_id)
>>
>>   	input.partition_id = partition_id;
>>
>> -	ret = hv_call_deposit_pages(NUMA_NO_NODE, partition_id,
>> -				    HV_INIT_PARTITION_DEPOSIT_PAGES);
>> -	if (ret)
>> -		return ret;
>> -
>>   	do {
>>   		status = hv_do_fast_hypercall8(HVCALL_INITIALIZE_PARTITION,
>>   					       *(u64 *)&input);
>> @@ -248,8 +241,7 @@ static int hv_do_map_gpa_hcall(u64 partition_id, u64 gfn, u64 page_struct_count,
>>   		completed = hv_repcomp(status);
>>
>>   		if (hv_result_needs_memory(status)) {
>> -			ret = hv_call_deposit_pages(NUMA_NO_NODE, partition_id,
>> -						    HV_MAP_GPA_DEPOSIT_PAGES);
>> +			ret = hv_deposit_memory(partition_id, status);
>>   			if (ret)
>>   				break;
>>
>> diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h
>> index bf601d67cecb..c16abaecb65e 100644
>> --- a/include/asm-generic/mshyperv.h
>> +++ b/include/asm-generic/mshyperv.h
>> @@ -345,7 +345,6 @@ static inline bool hv_parent_partition(void)
>>
>>   bool hv_result_needs_memory(u64 status);
>>   int hv_deposit_memory_node(int node, u64 partition_id, u64 status);
>> -int hv_call_deposit_pages(int node, u64 partition_id, u32 num_pages);
>>   int hv_call_add_logical_proc(int node, u32 lp_index, u32 acpi_id);
>>   int hv_call_notify_all_processors_started(void);
>>   bool hv_lp_exists(u32 lp_index);
>> @@ -360,10 +359,6 @@ static inline int hv_deposit_memory_node(int node, u64 partition_id, u64 status)
>>   {
>>   	return -EOPNOTSUPP;
>>   }
>> -static inline int hv_call_deposit_pages(int node, u64 partition_id, u32 num_pages)
>> -{
>> -	return -EOPNOTSUPP;
>> -}
>>   static inline int hv_call_add_logical_proc(int node, u32 lp_index, u32 acpi_id)
>>   {
>>   	return -EOPNOTSUPP;
>> --
>> 2.51.2.vfs.0.1
>>

RE: [PATCH V1 2/3] mshv: Redesign hypervisor memory deposit logic

[Michael Kelley]

From: Mukesh R <mrathor@linux.microsoft.com> Sent: Tuesday, September 8, 2026 3:33 PM
> 
> On 9/8/26 11:02, Michael Kelley wrote:
> > From: Mukesh R <mrathor@linux.microsoft.com> Sent: Monday, September 7, 2026 7:14 PM
> >>
> >> The current memory deposit implementation has a few issues and bugs:
> >>   o It is very slow
> >>   o Contiguous range requirement is broken, and is critical bug
> >>   o An incorrect assumption is made that contiguous memory size would
> >>     always be power of 2.
> >
> > Is this HV_MAX_CONTIGUOUS_ALLOCATION_PAGES? That value is
> > defined as a constant in hvhdk_mini.h. So is there a possibility that
> > the constant will change in the future, or in some new variation of
> > the overall environment?
> 
> Yes, I was told it could change in future. It is also not guaranteed to
> be power of 2 (even tho it is currently).
> 
> >>   o Two pages are allocated, only one is really needed. This adds to
> >>     overhead.
> >>   o For a 512 page deposit, the allocation is split into two: one for 511
> >>     and second for 1. Thus, an order 9 allocation never happens. A
> >>     contiguous 2M range, if possible, significantly improves performance
> >>     in the hypervisor.
> >>   o Since a page is already allocated to collect the frames, there is
> >>     not really a need to use per cpu input page, and hence avoid local
> >>     irq disable.
> >>   o In hv_call_deposit_pages(), in case of error, under err_free_allocations
> >>     label, all pages are freed without checking status to see if some pages
> >>     were deposited. This is a critical bug as it would free pages that hyp
> >>     may be using.
> >>
> >> All of above is addressed by:
> >>   o Allocate 2M by default, this is the recommendation from the hypervisor
> >>     team, and greatly improves performance.
> >
> > Can you be more specific about "improves performance"? Is the
> > improvement on the guest side, or on the hypervisor side? And what's
> > the key leverage point in improving performance, regardless of which
> > side? It might be helpful to record this for future reference to prevent
> > a change from being made that unknowingly hurts the key leverage
> > point.
> 
> Well, overall improvement I'd say. Lesser HV_STATUS_INSUFFICIENT_MEMORY
> means lesser interruptions means faster non-deposit hypercalls. With
> larger deposits, it will not come back as often with insufficient memory.
> If it only uses 1M say out of the 2, next time it will reuse the remaining
> meg before coming back and asking for more. IOW, incomplete hypercalls
> repeatedly coming back asking for more ram add lot of overhead.
> Eg. the guest setup part reduces from 8 sec to 1 sec with this, I
> don't have further breakdown of which ioctls.

OK -- this part wasn't clear to me from reading the commit message.
The primary goal is to reduce hypercall failures due to insufficient
memory, followed by the cycle of adding memory and trying again. By
adding memory in larger chunks in the first place, you reduce those
cycles. Improving the performance of the code for doing the deposit
helps, but that's probably not where you get the most noticeable
benefit. Could you clarify this in the commit message?

> 
> >>   o Always start with a full 2M range allocation, thus getting contiguous if
> >>     available. In cases where possible, the deposits are much faster.
> >
> > Is this faster because the hypervisor can consume the pages faster
> > if they are contiguous?
> 
> I didn't ask but my guess is that it can map 2M page in its page
> tables, so all the benefits of large pages: faster L2 mapping, less
> tlb overhead, etc..

Makes sense. This seems like it is probably a 2nd order benefit
after reducing hypercall failures due to insufficient memory.

> 
> >>   o Allocate only one page in the deposit function and collect 511 pfns
> >>     there. Just use a local variable for last pfn.
> >>   o Use the page as input to hypercall. Since this page is locally allocated,
> >>     irq disable can be avoided helping speed up the deposit.

Again, seems like a 2nd order perf improvement in the code
for doing the memory deposit.

> >>   o Fix the physical contiguous memory requirement.
> >>   o Lastly, remove pre-deposits hv_call_create_vp() and
> >>     hv_call_initialize_partition() as they were removed internally while
> >>     ago, most likely because they didn't help much.
> >>
> >> Signed-off-by: Mukesh R <mrathor@linux.microsoft.com>
> >> ---
> >>   drivers/hv/hv_proc.c           | 198 +++++++++++++++++++++++++++++----
> >>   drivers/hv/mshv_root_hv_call.c |  10 +-
> >>   include/asm-generic/mshyperv.h |   5 -
> >>   3 files changed, 179 insertions(+), 34 deletions(-)
> >>

[snip]

> >> +/*
> >> + * Deposit memory in the hypervisor. Even if @contiguous is false, a contiguous
> >> + * 2M worth of pfns is utmost desired for performance reasons. But short of

Again, this comment isn't clear on *why* the 2M helps performance.

> >> + * that, we deposit whatever contiguous chunks we can get. If @contiguous is
> >> + * true, then the entire range has to be physically contiguous. Note, in that
> >> + * case, upon withdrawl, hypervisor could return any page in between the range,
> >> + * so we must split that also. Lastly, HV_MAX_CONTIGUOUS_ALLOCATION_PAGES is
> >> + * not guaranteed to always be power of 2.
> >> + */
> >> +static int hv_call_deposit_pages(int node, u64 partition_id, bool contiguous)
> >> +{
> >> +	struct hv_deposit_memory *hc_input;
> >> +	int i, rc, num_pages;
> >> +	u64 status, *pfna, lastpfn = 0;
> >> +	bool trunc_extra = false;
> >> +
> >> +	BUILD_BUG_ON(HV_MAX_CONTIGUOUS_ALLOCATION_PAGES > HV_DEPOSIT_MAX);
> >> +
> >> +	if (contiguous) {
> >> +		num_pages = roundup_pow_of_two(
> >> +				 HV_MAX_CONTIGUOUS_ALLOCATION_PAGES);
> >> +		trunc_extra = num_pages != HV_MAX_CONTIGUOUS_ALLOCATION_PAGES;
> >> +	} else {
> >> +		num_pages = HV_DEPOSIT_MAX;
> >> +	}
> >> +
> >> +	hc_input = (struct hv_deposit_memory *)get_zeroed_page(GFP_KERNEL);
> >
> > Mike Rapoport has a kernel-wide effort underway to replace
> > __get_free_page() with kmalloc() and get_zeroed_page() with kzalloc().
> > See [1] for one example of the many patches he has submitted. The
> > commit message has a short explanation. Going with kzalloc() here
> > would probably avoid a future change.
> 
> There might be places where a get_page may not be strictly needed, but
> in this case we do for the hypercall (we use it as input page), and
> this hypercall can go upto exactly one page. I don't think an operating
> system can ever get away without having a get_page() api :). kmalloc()
> could add an aligntment parameter, but it would just call get_page
> anyways for page aligned requests.
> 

What Mike wants to do is:

	hc_input = kzalloc(PAGE_SIZE, GFP_KERNEL);

This is guaranteed to provide page-aligned memory, just like
get_zeroed_page(). See 2nd paragraph under "Description" at [1].

Michael

[1] https://www.kernel.org/doc/html/latest/core-api/mm-api.html#c.kmalloc

Re: [PATCH V1 2/3] mshv: Redesign hypervisor memory deposit logic

[Mukesh R]

On 9/9/26 10:32, Michael Kelley wrote:
> From: Mukesh R <mrathor@linux.microsoft.com> Sent: Tuesday, September 8, 2026 3:33 PM
>>
>> On 9/8/26 11:02, Michael Kelley wrote:
>>> From: Mukesh R <mrathor@linux.microsoft.com> Sent: Monday, September 7, 2026 7:14 PM
>>>>
>>>> The current memory deposit implementation has a few issues and bugs:
>>>>    o It is very slow
>>>>    o Contiguous range requirement is broken, and is critical bug
>>>>    o An incorrect assumption is made that contiguous memory size would
>>>>      always be power of 2.
>>>
>>> Is this HV_MAX_CONTIGUOUS_ALLOCATION_PAGES? That value is
>>> defined as a constant in hvhdk_mini.h. So is there a possibility that
>>> the constant will change in the future, or in some new variation of
>>> the overall environment?
>>
>> Yes, I was told it could change in future. It is also not guaranteed to
>> be power of 2 (even tho it is currently).
>>
>>>>    o Two pages are allocated, only one is really needed. This adds to
>>>>      overhead.
>>>>    o For a 512 page deposit, the allocation is split into two: one for 511
>>>>      and second for 1. Thus, an order 9 allocation never happens. A
>>>>      contiguous 2M range, if possible, significantly improves performance
>>>>      in the hypervisor.
>>>>    o Since a page is already allocated to collect the frames, there is
>>>>      not really a need to use per cpu input page, and hence avoid local
>>>>      irq disable.
>>>>    o In hv_call_deposit_pages(), in case of error, under err_free_allocations
>>>>      label, all pages are freed without checking status to see if some pages
>>>>      were deposited. This is a critical bug as it would free pages that hyp
>>>>      may be using.
>>>>
>>>> All of above is addressed by:
>>>>    o Allocate 2M by default, this is the recommendation from the hypervisor
>>>>      team, and greatly improves performance.
>>>
>>> Can you be more specific about "improves performance"? Is the
>>> improvement on the guest side, or on the hypervisor side? And what's
>>> the key leverage point in improving performance, regardless of which
>>> side? It might be helpful to record this for future reference to prevent
>>> a change from being made that unknowingly hurts the key leverage
>>> point.
>>
>> Well, overall improvement I'd say. Lesser HV_STATUS_INSUFFICIENT_MEMORY
>> means lesser interruptions means faster non-deposit hypercalls. With
>> larger deposits, it will not come back as often with insufficient memory.
>> If it only uses 1M say out of the 2, next time it will reuse the remaining
>> meg before coming back and asking for more. IOW, incomplete hypercalls
>> repeatedly coming back asking for more ram add lot of overhead.
>> Eg. the guest setup part reduces from 8 sec to 1 sec with this, I
>> don't have further breakdown of which ioctls.
> 
> OK -- this part wasn't clear to me from reading the commit message.
> The primary goal is to reduce hypercall failures due to insufficient
> memory, followed by the cycle of adding memory and trying again. By
> adding memory in larger chunks in the first place, you reduce those
> cycles. Improving the performance of the code for doing the deposit
> helps, but that's probably not where you get the most noticeable
> benefit. Could you clarify this in the commit message?
> 
>>
>>>>    o Always start with a full 2M range allocation, thus getting contiguous if
>>>>      available. In cases where possible, the deposits are much faster.
>>>
>>> Is this faster because the hypervisor can consume the pages faster
>>> if they are contiguous?
>>
>> I didn't ask but my guess is that it can map 2M page in its page
>> tables, so all the benefits of large pages: faster L2 mapping, less
>> tlb overhead, etc..
> 
> Makes sense. This seems like it is probably a 2nd order benefit
> after reducing hypercall failures due to insufficient memory.
> 
>>
>>>>    o Allocate only one page in the deposit function and collect 511 pfns
>>>>      there. Just use a local variable for last pfn.
>>>>    o Use the page as input to hypercall. Since this page is locally allocated,
>>>>      irq disable can be avoided helping speed up the deposit.
> 
> Again, seems like a 2nd order perf improvement in the code
> for doing the memory deposit.
> 
>>>>    o Fix the physical contiguous memory requirement.
>>>>    o Lastly, remove pre-deposits hv_call_create_vp() and
>>>>      hv_call_initialize_partition() as they were removed internally while
>>>>      ago, most likely because they didn't help much.
>>>>
>>>> Signed-off-by: Mukesh R <mrathor@linux.microsoft.com>
>>>> ---
>>>>    drivers/hv/hv_proc.c           | 198 +++++++++++++++++++++++++++++----
>>>>    drivers/hv/mshv_root_hv_call.c |  10 +-
>>>>    include/asm-generic/mshyperv.h |   5 -
>>>>    3 files changed, 179 insertions(+), 34 deletions(-)
>>>>
> 
> [snip]
> 
>>>> +/*
>>>> + * Deposit memory in the hypervisor. Even if @contiguous is false, a contiguous
>>>> + * 2M worth of pfns is utmost desired for performance reasons. But short of
> 
> Again, this comment isn't clear on *why* the 2M helps performance.

It can be mapped as large page in hyp page tables.

>>>> + * that, we deposit whatever contiguous chunks we can get. If @contiguous is
>>>> + * true, then the entire range has to be physically contiguous. Note, in that
>>>> + * case, upon withdrawl, hypervisor could return any page in between the range,
>>>> + * so we must split that also. Lastly, HV_MAX_CONTIGUOUS_ALLOCATION_PAGES is
>>>> + * not guaranteed to always be power of 2.
>>>> + */
>>>> +static int hv_call_deposit_pages(int node, u64 partition_id, bool contiguous)
>>>> +{
>>>> +	struct hv_deposit_memory *hc_input;
>>>> +	int i, rc, num_pages;
>>>> +	u64 status, *pfna, lastpfn = 0;
>>>> +	bool trunc_extra = false;
>>>> +
>>>> +	BUILD_BUG_ON(HV_MAX_CONTIGUOUS_ALLOCATION_PAGES > HV_DEPOSIT_MAX);
>>>> +
>>>> +	if (contiguous) {
>>>> +		num_pages = roundup_pow_of_two(
>>>> +				 HV_MAX_CONTIGUOUS_ALLOCATION_PAGES);
>>>> +		trunc_extra = num_pages != HV_MAX_CONTIGUOUS_ALLOCATION_PAGES;
>>>> +	} else {
>>>> +		num_pages = HV_DEPOSIT_MAX;
>>>> +	}
>>>> +
>>>> +	hc_input = (struct hv_deposit_memory *)get_zeroed_page(GFP_KERNEL);
>>>
>>> Mike Rapoport has a kernel-wide effort underway to replace
>>> __get_free_page() with kmalloc() and get_zeroed_page() with kzalloc().
>>> See [1] for one example of the many patches he has submitted. The
>>> commit message has a short explanation. Going with kzalloc() here
>>> would probably avoid a future change.
>>
>> There might be places where a get_page may not be strictly needed, but
>> in this case we do for the hypercall (we use it as input page), and
>> this hypercall can go upto exactly one page. I don't think an operating
>> system can ever get away without having a get_page() api :). kmalloc()
>> could add an aligntment parameter, but it would just call get_page
>> anyways for page aligned requests.
>>
> 
> What Mike wants to do is:
> 
> 	hc_input = kzalloc(PAGE_SIZE, GFP_KERNEL);
> 
> This is guaranteed to provide page-aligned memory, just like
> get_zeroed_page(). See 2nd paragraph under "Description" at [1].

Confusing... the first line after Description also says:

"kmalloc is the normal method of allocating memory for objects smaller
  than page size in the kernel."

I think it is clearer to keep get_page, it also helps keep common code
with earlier versions

Thanks,
-Mukesh



> Michael
> 
> [1] https://www.kernel.org/doc/html/latest/core-api/mm-api.html#c.kmalloc

[PATCH V1 3/3] mshv: Remove unused *_old memory deposit functions

[Mukesh R]

Remove the renamed *_old functions.

Signed-off-by: Mukesh R <mrathor@linux.microsoft.com>
---
 drivers/hv/hv_proc.c | 129 -------------------------------------------
 1 file changed, 129 deletions(-)

diff --git a/drivers/hv/hv_proc.c b/drivers/hv/hv_proc.c
index 0ebede0bc8b4..dfae4b933fa9 100644
--- a/drivers/hv/hv_proc.c
+++ b/drivers/hv/hv_proc.c
@@ -187,135 +187,6 @@ int hv_deposit_memory_node(int node, u64 pt_id, u64 hv_status)
 }
 EXPORT_SYMBOL_GPL(hv_deposit_memory_node);
 
-/*
- * See struct hv_deposit_memory. The first u64 is partition ID, the rest
- * are GPAs.
- */
-#define HV_DEPOSIT_MAX_OLD (HV_HYP_PAGE_SIZE / sizeof(u64) - 1)
-
-/* Deposits exact number of pages. Must be called with interrupts enabled.  */
-static int hv_call_deposit_pages_old(int node, u64 partition_id, u32 num_pages)
-{
-	struct page **pages, *page;
-	int *counts;
-	int num_allocations;
-	int i, j, page_count;
-	int order;
-	u64 status;
-	int ret;
-	u64 base_pfn;
-	struct hv_deposit_memory *input_page;
-	unsigned long flags;
-
-	if (num_pages > HV_DEPOSIT_MAX_OLD)
-		return -E2BIG;
-	if (!num_pages)
-		return 0;
-
-	/* One buffer for page pointers and counts */
-	page = alloc_page(GFP_KERNEL);
-	if (!page)
-		return -ENOMEM;
-	pages = page_address(page);
-
-	counts = kzalloc_objs(int, HV_DEPOSIT_MAX_OLD);
-	if (!counts) {
-		free_page((unsigned long)pages);
-		return -ENOMEM;
-	}
-
-	/* Allocate all the pages before disabling interrupts */
-	i = 0;
-
-	while (num_pages) {
-		/* Find highest order we can actually allocate */
-		order = 31 - __builtin_clz(num_pages);
-
-		while (1) {
-			pages[i] = alloc_pages_node(node, GFP_KERNEL, order);
-			if (pages[i])
-				break;
-			if (!order) {
-				ret = -ENOMEM;
-				num_allocations = i;
-				goto err_free_allocations;
-			}
-			--order;
-		}
-
-		split_page(pages[i], order);
-		counts[i] = 1 << order;
-		num_pages -= counts[i];
-		i++;
-	}
-	num_allocations = i;
-
-	local_irq_save(flags);
-
-	input_page = *this_cpu_ptr(hyperv_pcpu_input_arg);
-
-	input_page->partition_id = partition_id;
-
-	/* Populate gpa_page_list - these will fit on the input page */
-	for (i = 0, page_count = 0; i < num_allocations; ++i) {
-		base_pfn = page_to_pfn(pages[i]);
-		for (j = 0; j < counts[i]; ++j, ++page_count)
-			input_page->gpa_page_list[page_count] = base_pfn + j;
-	}
-	status = hv_do_rep_hypercall(HVCALL_DEPOSIT_MEMORY,
-				     page_count, 0, input_page, NULL);
-	local_irq_restore(flags);
-	if (!hv_result_success(status)) {
-		hv_status_err(status, "\n");
-		ret = hv_result_to_errno(status);
-		goto err_free_allocations;
-	}
-
-	ret = 0;
-	goto free_buf;
-
-err_free_allocations:
-	for (i = 0; i < num_allocations; ++i) {
-		base_pfn = page_to_pfn(pages[i]);
-		for (j = 0; j < counts[i]; ++j)
-			__free_page(pfn_to_page(base_pfn + j));
-	}
-
-free_buf:
-	free_page((unsigned long)pages);
-	kfree(counts);
-	return ret;
-}
-
-static int __maybe_unused hv_deposit_memory_node_old(int node, u64 partition_id, u64 hv_status)
-{
-	u32 num_pages = 1;
-
-	switch (hv_result(hv_status)) {
-	case HV_STATUS_INSUFFICIENT_MEMORY:
-		break;
-	case HV_STATUS_INSUFFICIENT_CONTIGUOUS_MEMORY:
-		num_pages = HV_MAX_CONTIGUOUS_ALLOCATION_PAGES;
-		break;
-
-	case HV_STATUS_INSUFFICIENT_CONTIGUOUS_ROOT_MEMORY:
-		num_pages = HV_MAX_CONTIGUOUS_ALLOCATION_PAGES;
-		fallthrough;
-	case HV_STATUS_INSUFFICIENT_ROOT_MEMORY:
-		if (!hv_root_partition()) {
-			hv_status_err(hv_status, "Unexpected root memory deposit\n");
-			return -ENOMEM;
-		}
-		partition_id = HV_PARTITION_ID_SELF;
-		break;
-
-	default:
-		hv_status_err(hv_status, "Unexpected!\n");
-		return -ENOMEM;
-	}
-	return hv_call_deposit_pages_old(node, partition_id, num_pages);
-}
-
 bool hv_result_needs_memory(u64 status)
 {
 	switch (hv_result(status)) {
-- 
2.51.2.vfs.0.1

RE: [PATCH V1 3/3] mshv: Remove unused *_old memory deposit functions

[Michael Kelley]

From: Mukesh R <mrathor@linux.microsoft.com> Sent: Monday, September 7, 2026 7:14 PM
> 
> Remove the renamed *_old functions.
> 
> Signed-off-by: Mukesh R <mrathor@linux.microsoft.com>

Reviewed-by: Michael Kelley <mhklinux@outlook.com>

> ---
>  drivers/hv/hv_proc.c | 129 -------------------------------------------
>  1 file changed, 129 deletions(-)
> 
> diff --git a/drivers/hv/hv_proc.c b/drivers/hv/hv_proc.c
> index 0ebede0bc8b4..dfae4b933fa9 100644
> --- a/drivers/hv/hv_proc.c
> +++ b/drivers/hv/hv_proc.c
> @@ -187,135 +187,6 @@ int hv_deposit_memory_node(int node, u64 pt_id, u64 hv_status)
>  }
>  EXPORT_SYMBOL_GPL(hv_deposit_memory_node);
> 
> -/*
> - * See struct hv_deposit_memory. The first u64 is partition ID, the rest
> - * are GPAs.
> - */
> -#define HV_DEPOSIT_MAX_OLD (HV_HYP_PAGE_SIZE / sizeof(u64) - 1)
> -
> -/* Deposits exact number of pages. Must be called with interrupts enabled.  */
> -static int hv_call_deposit_pages_old(int node, u64 partition_id, u32 num_pages)
> -{
> -	struct page **pages, *page;
> -	int *counts;
> -	int num_allocations;
> -	int i, j, page_count;
> -	int order;
> -	u64 status;
> -	int ret;
> -	u64 base_pfn;
> -	struct hv_deposit_memory *input_page;
> -	unsigned long flags;
> -
> -	if (num_pages > HV_DEPOSIT_MAX_OLD)
> -		return -E2BIG;
> -	if (!num_pages)
> -		return 0;
> -
> -	/* One buffer for page pointers and counts */
> -	page = alloc_page(GFP_KERNEL);
> -	if (!page)
> -		return -ENOMEM;
> -	pages = page_address(page);
> -
> -	counts = kzalloc_objs(int, HV_DEPOSIT_MAX_OLD);
> -	if (!counts) {
> -		free_page((unsigned long)pages);
> -		return -ENOMEM;
> -	}
> -
> -	/* Allocate all the pages before disabling interrupts */
> -	i = 0;
> -
> -	while (num_pages) {
> -		/* Find highest order we can actually allocate */
> -		order = 31 - __builtin_clz(num_pages);
> -
> -		while (1) {
> -			pages[i] = alloc_pages_node(node, GFP_KERNEL, order);
> -			if (pages[i])
> -				break;
> -			if (!order) {
> -				ret = -ENOMEM;
> -				num_allocations = i;
> -				goto err_free_allocations;
> -			}
> -			--order;
> -		}
> -
> -		split_page(pages[i], order);
> -		counts[i] = 1 << order;
> -		num_pages -= counts[i];
> -		i++;
> -	}
> -	num_allocations = i;
> -
> -	local_irq_save(flags);
> -
> -	input_page = *this_cpu_ptr(hyperv_pcpu_input_arg);
> -
> -	input_page->partition_id = partition_id;
> -
> -	/* Populate gpa_page_list - these will fit on the input page */
> -	for (i = 0, page_count = 0; i < num_allocations; ++i) {
> -		base_pfn = page_to_pfn(pages[i]);
> -		for (j = 0; j < counts[i]; ++j, ++page_count)
> -			input_page->gpa_page_list[page_count] = base_pfn + j;
> -	}
> -	status = hv_do_rep_hypercall(HVCALL_DEPOSIT_MEMORY,
> -				     page_count, 0, input_page, NULL);
> -	local_irq_restore(flags);
> -	if (!hv_result_success(status)) {
> -		hv_status_err(status, "\n");
> -		ret = hv_result_to_errno(status);
> -		goto err_free_allocations;
> -	}
> -
> -	ret = 0;
> -	goto free_buf;
> -
> -err_free_allocations:
> -	for (i = 0; i < num_allocations; ++i) {
> -		base_pfn = page_to_pfn(pages[i]);
> -		for (j = 0; j < counts[i]; ++j)
> -			__free_page(pfn_to_page(base_pfn + j));
> -	}
> -
> -free_buf:
> -	free_page((unsigned long)pages);
> -	kfree(counts);
> -	return ret;
> -}
> -
> -static int __maybe_unused hv_deposit_memory_node_old(int node, u64 partition_id, u64 hv_status)
> -{
> -	u32 num_pages = 1;
> -
> -	switch (hv_result(hv_status)) {
> -	case HV_STATUS_INSUFFICIENT_MEMORY:
> -		break;
> -	case HV_STATUS_INSUFFICIENT_CONTIGUOUS_MEMORY:
> -		num_pages = HV_MAX_CONTIGUOUS_ALLOCATION_PAGES;
> -		break;
> -
> -	case HV_STATUS_INSUFFICIENT_CONTIGUOUS_ROOT_MEMORY:
> -		num_pages = HV_MAX_CONTIGUOUS_ALLOCATION_PAGES;
> -		fallthrough;
> -	case HV_STATUS_INSUFFICIENT_ROOT_MEMORY:
> -		if (!hv_root_partition()) {
> -			hv_status_err(hv_status, "Unexpected root memory deposit\n");
> -			return -ENOMEM;
> -		}
> -		partition_id = HV_PARTITION_ID_SELF;
> -		break;
> -
> -	default:
> -		hv_status_err(hv_status, "Unexpected!\n");
> -		return -ENOMEM;
> -	}
> -	return hv_call_deposit_pages_old(node, partition_id, num_pages);
> -}
> -
>  bool hv_result_needs_memory(u64 status)
>  {
>  	switch (hv_result(status)) {
> --
> 2.51.2.vfs.0.1
>