[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aS14E-QpVccRc6Gz@anirudh-surface.localdomain>
Date: Mon, 1 Dec 2025 11:12:19 +0000
From: Anirudh Rayabharam <anirudh@...rudhrb.com>
To: Stanislav Kinsburskii <skinsburskii@...ux.microsoft.com>
Cc: kys@...rosoft.com, haiyangz@...rosoft.com, wei.liu@...nel.org,
decui@...rosoft.com, linux-hyperv@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v7 2/7] Drivers: hv: Centralize guest memory region
destruction
On Wed, Nov 26, 2025 at 02:08:57AM +0000, Stanislav Kinsburskii wrote:
> Centralize guest memory region destruction to prevent resource leaks and
> inconsistent cleanup across unmap and partition destruction paths.
>
> Unify region removal, encrypted partition access recovery, and region
> invalidation to improve maintainability and reliability. Reduce code
> duplication and make future updates less error-prone by encapsulating
> cleanup logic in a single helper.
>
> Signed-off-by: Stanislav Kinsburskii <skinsburskii@...ux.microsoft.com>
> Reviewed-by: Nuno Das Neves <nunodasneves@...ux.microsoft.com>
> ---
> drivers/hv/mshv_root_main.c | 65 ++++++++++++++++++++++---------------------
> 1 file changed, 34 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/hv/mshv_root_main.c b/drivers/hv/mshv_root_main.c
> index fec82619684a..ec18984c3f2d 100644
> --- a/drivers/hv/mshv_root_main.c
> +++ b/drivers/hv/mshv_root_main.c
> @@ -1356,13 +1356,42 @@ mshv_map_user_memory(struct mshv_partition *partition,
> return ret;
> }
>
> +static void mshv_partition_destroy_region(struct mshv_mem_region *region)
> +{
> + struct mshv_partition *partition = region->partition;
> + u32 unmap_flags = 0;
> + int ret;
> +
> + hlist_del(®ion->hnode);
> +
> + if (mshv_partition_encrypted(partition)) {
> + ret = mshv_partition_region_share(region);
> + if (ret) {
> + pt_err(partition,
> + "Failed to regain access to memory, unpinning user pages will fail and crash the host error: %d\n",
> + ret);
> + return;
> + }
> + }
> +
> + if (region->flags.large_pages)
> + unmap_flags |= HV_UNMAP_GPA_LARGE_PAGE;
> +
> + /* ignore unmap failures and continue as process may be exiting */
> + hv_call_unmap_gpa_pages(partition->pt_id, region->start_gfn,
> + region->nr_pages, unmap_flags);
> +
> + mshv_region_invalidate(region);
> +
> + vfree(region);
> +}
> +
> /* Called for unmapping both the guest ram and the mmio space */
> static long
> mshv_unmap_user_memory(struct mshv_partition *partition,
> struct mshv_user_mem_region mem)
> {
> struct mshv_mem_region *region;
> - u32 unmap_flags = 0;
>
> if (!(mem.flags & BIT(MSHV_SET_MEM_BIT_UNMAP)))
> return -EINVAL;
> @@ -1377,18 +1406,8 @@ mshv_unmap_user_memory(struct mshv_partition *partition,
> region->nr_pages != HVPFN_DOWN(mem.size))
> return -EINVAL;
>
> - hlist_del(®ion->hnode);
> + mshv_partition_destroy_region(region);
>
> - if (region->flags.large_pages)
> - unmap_flags |= HV_UNMAP_GPA_LARGE_PAGE;
> -
> - /* ignore unmap failures and continue as process may be exiting */
> - hv_call_unmap_gpa_pages(partition->pt_id, region->start_gfn,
> - region->nr_pages, unmap_flags);
> -
> - mshv_region_invalidate(region);
> -
> - vfree(region);
> return 0;
> }
>
> @@ -1724,8 +1743,8 @@ static void destroy_partition(struct mshv_partition *partition)
> {
> struct mshv_vp *vp;
> struct mshv_mem_region *region;
> - int i, ret;
> struct hlist_node *n;
> + int i;
>
> if (refcount_read(&partition->pt_ref_count)) {
> pt_err(partition,
> @@ -1789,25 +1808,9 @@ static void destroy_partition(struct mshv_partition *partition)
>
> remove_partition(partition);
>
> - /* Remove regions, regain access to the memory and unpin the pages */
> hlist_for_each_entry_safe(region, n, &partition->pt_mem_regions,
> - hnode) {
> - hlist_del(®ion->hnode);
> -
> - if (mshv_partition_encrypted(partition)) {
> - ret = mshv_partition_region_share(region);
> - if (ret) {
> - pt_err(partition,
> - "Failed to regain access to memory, unpinning user pages will fail and crash the host error: %d\n",
> - ret);
> - return;
> - }
> - }
> -
> - mshv_region_invalidate(region);
> -
> - vfree(region);
> - }
> + hnode)
> + mshv_partition_destroy_region(region);
>
> /* Withdraw and free all pages we deposited */
> hv_call_withdraw_memory(U64_MAX, NUMA_NO_NODE, partition->pt_id);
>
>
Reviewed-by: Anirudh Rayabharam (Microsoft) <anirudh@...rudhrb.com>
Powered by blists - more mailing lists