On Fri, Sep 11, 2026 at 05:03:17PM -0700, Mukesh R wrote: > The current memory deposit implementation has a few issues and bugs: > o It is very slow: one of the main contributions to slow VM creations > and boot is many hypercalls repeatedly coming back with insufficient > memory. On Hyper-V, a hypercall returns with such status whenever it > cannot complete due to lack of memory in the hypervisor and needs more. > 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. Extra alloc adds > to the 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, greatly improves performance > in the hypervisor as it can be mapped as large page whenever possible. > Also, removing extra allocation reduces overhead in linux. > 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. Depositing one or few pages > at a time results in lot of insufficient memory returns from hypercalls. > 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 enforcement.. > 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 <[email protected]> Reviewed-by: Souradeep Chakrabarti <[email protected]> > --- > drivers/hv/hv_proc.c | 194 +++++++++++++++++++++++++++++---- > drivers/hv/mshv_root_hv_call.c | 10 +- > include/asm-generic/mshyperv.h | 5 - > 3 files changed, 175 insertions(+), 34 deletions(-) > > diff --git a/drivers/hv/hv_proc.c b/drivers/hv/hv_proc.c > index 57864bb5bcd8..df39a5c587ca 100644 > --- a/drivers/hv/hv_proc.c > +++ b/drivers/hv/hv_proc.c > @@ -9,6 +9,180 @@ > #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; > + } > + > + /* Not using hyperv_pcpu_input_arg, so no need to 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); > + > + if (!hv_result_success(status) && hv_repcomp(status) == 0) > + /* 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 +283,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 +312,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 +374,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 >
