[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <aXqZSKmRdJMc6x5u@skinsburskii.localdomain>
Date: Wed, 28 Jan 2026 15:18:32 -0800
From: Stanislav Kinsburskii <skinsburskii@...ux.microsoft.com>
To: Mukesh R <mrathor@...ux.microsoft.com>
Cc: kys@...rosoft.com, haiyangz@...rosoft.com, wei.liu@...nel.org,
decui@...rosoft.com, longli@...rosoft.com,
linux-hyperv@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/4] mshv: Introduce hv_deposit_memory helper functions
On Tue, Jan 27, 2026 at 11:44:25AM -0800, Mukesh R wrote:
> On 1/27/26 10:30, Stanislav Kinsburskii wrote:
> > On Mon, Jan 26, 2026 at 06:06:23PM -0800, Mukesh R wrote:
> > > On 1/25/26 14:41, Stanislav Kinsburskii wrote:
> > > > On Fri, Jan 23, 2026 at 04:33:39PM -0800, Mukesh R wrote:
> > > > > On 1/22/26 17:35, Stanislav Kinsburskii wrote:
> > > > > > Introduce hv_deposit_memory_node() and hv_deposit_memory() helper
> > > > > > functions to handle memory deposition with proper error handling.
> > > > > >
> > > > > > The new hv_deposit_memory_node() function takes the hypervisor status
> > > > > > as a parameter and validates it before depositing pages. It checks for
> > > > > > HV_STATUS_INSUFFICIENT_MEMORY specifically and returns an error for
> > > > > > unexpected status codes.
> > > > > >
> > > > > > This is a precursor patch to new out-of-memory error codes support.
> > > > > > No functional changes intended.
> > > > > >
> > > > > > Signed-off-by: Stanislav Kinsburskii <skinsburskii@...ux.microsoft.com>
> > > > > > ---
> > > > > > drivers/hv/hv_proc.c | 22 ++++++++++++++++++++--
> > > > > > drivers/hv/mshv_root_hv_call.c | 25 +++++++++----------------
> > > > > > drivers/hv/mshv_root_main.c | 3 +--
> > > > > > include/asm-generic/mshyperv.h | 10 ++++++++++
> > > > > > 4 files changed, 40 insertions(+), 20 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/hv/hv_proc.c b/drivers/hv/hv_proc.c
> > > > > > index 80c66d1c74d5..c0c2bfc80d77 100644
> > > > > > --- a/drivers/hv/hv_proc.c
> > > > > > +++ b/drivers/hv/hv_proc.c
> > > > > > @@ -110,6 +110,23 @@ int hv_call_deposit_pages(int node, u64 partition_id, u32 num_pages)
> > > > > > }
> > > > > > EXPORT_SYMBOL_GPL(hv_call_deposit_pages);
> > > > > > +int hv_deposit_memory_node(int node, u64 partition_id,
> > > > > > + u64 hv_status)
> > > > > > +{
> > > > > > + u32 num_pages;
> > > > > > +
> > > > > > + switch (hv_result(hv_status)) {
> > > > > > + case HV_STATUS_INSUFFICIENT_MEMORY:
> > > > > > + num_pages = 1;
> > > > > > + break;
> > > > > > + default:
> > > > > > + hv_status_err(hv_status, "Unexpected!\n");
> > > > > > + return -ENOMEM;
> > > > > > + }
> > > > > > + return hv_call_deposit_pages(node, partition_id, num_pages);
> > > > > > +}
> > > > > > +EXPORT_SYMBOL_GPL(hv_deposit_memory_node);
> > > > > > +
> > > > >
> > > > > Different hypercalls may want to deposit different number of pages in one
> > > > > shot. As feature evolves, page sizes get mixed, we'd almost need that
> > > > > flexibility. So, imo, either we just don't do this for now, or add num pages
> > > > > parameter to be passed down.
> > > > >
> > > >
> > > > What you do mean by "page sizes get mixed"?
> > > > A helper to deposit num pages already exists: its
> > > > hv_call_deposit_pages().
> > >
> > > My point, you are removing number of pages, and we may want to keep
> > > that so one can quickly play around and change them.
> > >
> > > - ret = hv_call_deposit_pages(NUMA_NO_NODE,
> > > - pt_id, 1);
> > > + ret = hv_deposit_memory(pt_id, status);
> > >
> > > For example, in hv_call_initialize_partition() we may realize after
> > > some analysis that depositing 2 pages or 4 pages is much better.
> > >
> >
> > We have been using this 1-page deposit logic from the beginning. To
> > change the number of pages, simply replace hv_deposit_memory with
> > hv_call_deposit_pages and specify the desired number of pages.
>
> You could perhaps rename it to hv_deposit_page().
>
Yes, this would be a good name, but unfortunately we can now receive
statuses like HV_STATUS_INSUFFICIENT_CONTIGUOUS_MEMORY, where we need to
deposit at least 8 consecutive pages. There is also another pair of
status codes for required root pages, even when a guest partition-related
hypercall is performed (see the next patch for details).
This new helper is intended to cover all such cases, instead of branching
for all these different cases in every function.
Thanks,
Stanislav
> > The proposed approach reduces code duplication and is less error-prone,
> > as there are multiple error codes to handle. Consolidating the logic
> > also makes the driver more robust.
> >
> >
> > Thanks, Stanislav
> >
> > > > Thanks,
> > > > Stanislav
> > > >
> > > > > Thanks,
> > > > > -Mukesh
> > > > >
> > > > >
> > > > >
> > > > > > bool hv_result_oom(u64 status)
> > > > > > {
> > > > > > switch (hv_result(status)) {
> > > > > > @@ -155,7 +172,8 @@ int hv_call_add_logical_proc(int node, u32 lp_index, u32 apic_id)
> > > > > > }
> > > > > > break;
> > > > > > }
> > > > > > - ret = hv_call_deposit_pages(node, hv_current_partition_id, 1);
> > > > > > + ret = hv_deposit_memory_node(node, hv_current_partition_id,
> > > > > > + status);
> > > > > > } while (!ret);
> > > > > > return ret;
> > > > > > @@ -197,7 +215,7 @@ int hv_call_create_vp(int node, u64 partition_id, u32 vp_index, u32 flags)
> > > > > > }
> > > > > > break;
> > > > > > }
> > > > > > - ret = hv_call_deposit_pages(node, partition_id, 1);
> > > > > > + ret = hv_deposit_memory_node(node, partition_id, status);
> > > > > > } while (!ret);
> > > > > > diff --git a/drivers/hv/mshv_root_hv_call.c b/drivers/hv/mshv_root_hv_call.c
> > > > > > index 58c5cbf2e567..06f2bac8039d 100644
> > > > > > --- a/drivers/hv/mshv_root_hv_call.c
> > > > > > +++ b/drivers/hv/mshv_root_hv_call.c
> > > > > > @@ -123,8 +123,7 @@ int hv_call_create_partition(u64 flags,
> > > > > > break;
> > > > > > }
> > > > > > local_irq_restore(irq_flags);
> > > > > > - ret = hv_call_deposit_pages(NUMA_NO_NODE,
> > > > > > - hv_current_partition_id, 1);
> > > > > > + ret = hv_deposit_memory(hv_current_partition_id, status);
> > > > > > } while (!ret);
> > > > > > return ret;
> > > > > > @@ -151,7 +150,7 @@ int hv_call_initialize_partition(u64 partition_id)
> > > > > > ret = hv_result_to_errno(status);
> > > > > > break;
> > > > > > }
> > > > > > - ret = hv_call_deposit_pages(NUMA_NO_NODE, partition_id, 1);
> > > > > > + ret = hv_deposit_memory(partition_id, status);
> > > > > > } while (!ret);
> > > > > > return ret;
> > > > > > @@ -465,8 +464,7 @@ int hv_call_get_vp_state(u32 vp_index, u64 partition_id,
> > > > > > }
> > > > > > local_irq_restore(flags);
> > > > > > - ret = hv_call_deposit_pages(NUMA_NO_NODE,
> > > > > > - partition_id, 1);
> > > > > > + ret = hv_deposit_memory(partition_id, status);
> > > > > > } while (!ret);
> > > > > > return ret;
> > > > > > @@ -525,8 +523,7 @@ int hv_call_set_vp_state(u32 vp_index, u64 partition_id,
> > > > > > }
> > > > > > local_irq_restore(flags);
> > > > > > - ret = hv_call_deposit_pages(NUMA_NO_NODE,
> > > > > > - partition_id, 1);
> > > > > > + ret = hv_deposit_memory(partition_id, status);
> > > > > > } while (!ret);
> > > > > > return ret;
> > > > > > @@ -573,7 +570,7 @@ static int hv_call_map_vp_state_page(u64 partition_id, u32 vp_index, u32 type,
> > > > > > local_irq_restore(flags);
> > > > > > - ret = hv_call_deposit_pages(NUMA_NO_NODE, partition_id, 1);
> > > > > > + ret = hv_deposit_memory(partition_id, status);
> > > > > > } while (!ret);
> > > > > > return ret;
> > > > > > @@ -722,8 +719,7 @@ hv_call_create_port(u64 port_partition_id, union hv_port_id port_id,
> > > > > > ret = hv_result_to_errno(status);
> > > > > > break;
> > > > > > }
> > > > > > - ret = hv_call_deposit_pages(NUMA_NO_NODE, port_partition_id, 1);
> > > > > > -
> > > > > > + ret = hv_deposit_memory(port_partition_id, status);
> > > > > > } while (!ret);
> > > > > > return ret;
> > > > > > @@ -776,8 +772,7 @@ hv_call_connect_port(u64 port_partition_id, union hv_port_id port_id,
> > > > > > ret = hv_result_to_errno(status);
> > > > > > break;
> > > > > > }
> > > > > > - ret = hv_call_deposit_pages(NUMA_NO_NODE,
> > > > > > - connection_partition_id, 1);
> > > > > > + ret = hv_deposit_memory(connection_partition_id, status);
> > > > > > } while (!ret);
> > > > > > return ret;
> > > > > > @@ -848,8 +843,7 @@ static int hv_call_map_stats_page2(enum hv_stats_object_type type,
> > > > > > break;
> > > > > > }
> > > > > > - ret = hv_call_deposit_pages(NUMA_NO_NODE,
> > > > > > - hv_current_partition_id, 1);
> > > > > > + ret = hv_deposit_memory(hv_current_partition_id, status);
> > > > > > } while (!ret);
> > > > > > return ret;
> > > > > > @@ -885,8 +879,7 @@ static int hv_call_map_stats_page(enum hv_stats_object_type type,
> > > > > > return ret;
> > > > > > }
> > > > > > - ret = hv_call_deposit_pages(NUMA_NO_NODE,
> > > > > > - hv_current_partition_id, 1);
> > > > > > + ret = hv_deposit_memory(hv_current_partition_id, status);
> > > > > > if (ret)
> > > > > > return ret;
> > > > > > } while (!ret);
> > > > > > diff --git a/drivers/hv/mshv_root_main.c b/drivers/hv/mshv_root_main.c
> > > > > > index f4697497f83e..5fc572e31cd7 100644
> > > > > > --- a/drivers/hv/mshv_root_main.c
> > > > > > +++ b/drivers/hv/mshv_root_main.c
> > > > > > @@ -264,8 +264,7 @@ static int mshv_ioctl_passthru_hvcall(struct mshv_partition *partition,
> > > > > > if (!hv_result_oom(status))
> > > > > > ret = hv_result_to_errno(status);
> > > > > > else
> > > > > > - ret = hv_call_deposit_pages(NUMA_NO_NODE,
> > > > > > - pt_id, 1);
> > > > > > + ret = hv_deposit_memory(pt_id, status);
> > > > > > } while (!ret);
> > > > > > args.status = hv_result(status);
> > > > > > diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h
> > > > > > index b73352a7fc9e..c8e8976839f8 100644
> > > > > > --- a/include/asm-generic/mshyperv.h
> > > > > > +++ b/include/asm-generic/mshyperv.h
> > > > > > @@ -344,6 +344,7 @@ static inline bool hv_parent_partition(void)
> > > > > > }
> > > > > > bool hv_result_oom(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_create_vp(int node, u64 partition_id, u32 vp_index, u32 flags);
> > > > > > @@ -353,6 +354,10 @@ static inline bool hv_root_partition(void) { return false; }
> > > > > > static inline bool hv_l1vh_partition(void) { return false; }
> > > > > > static inline bool hv_parent_partition(void) { return false; }
> > > > > > static inline bool hv_result_oom(u64 status) { return false; }
> > > > > > +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;
> > > > > > @@ -367,6 +372,11 @@ static inline int hv_call_create_vp(int node, u64 partition_id, u32 vp_index, u3
> > > > > > }
> > > > > > #endif /* CONFIG_MSHV_ROOT */
> > > > > > +static inline int hv_deposit_memory(u64 partition_id, u64 status)
> > > > > > +{
> > > > > > + return hv_deposit_memory_node(NUMA_NO_NODE, partition_id, status);
> > > > > > +}
> > > > > > +
> > > > > > #if IS_ENABLED(CONFIG_HYPERV_VTL_MODE)
> > > > > > u8 __init get_vtl(void);
> > > > > > #else
> > > > > >
> > > > > >
>
Powered by blists - more mailing lists