RE: [PATCH 4/4] mshv: Pre-deposit pages for SLAT creation

[<mhklkml@zohomail.com>]

From: Michael Kelley <mhklinux@outlook.com> Sent: Thursday, March 5, 2026 11:45 AM
> 
> From: Stanislav Kinsburskii <skinsburskii@linux.microsoft.com> Sent: Tuesday, March
3, 2026 4:24 PM
> >
> > Deposit enough pages up front to avoid guest address space region creation
> > failures due to low memory. This also speeds up guest creation.
> >
> > Calculate the required number of pages based on the guest's physical
> > address space size, rounded up to 1 GB chunks. Even the smallest guests are
> > assumed to need at least 1 GB worth of deposits. This is because every
> > guest requires tens of megabytes of deposited pages for hypervisor
> > overhead, making smaller deposits impractical.
> >
> > Estimating in 1 GB chunks prevents over-depositing for larger guests while
> > accepting some over-deposit for smaller ones. This trade-off keeps the
> > estimate close to actual needs for larger guests.
> >
> > Also withdraw the deposited pages if address space region creation fails.
> >
> > Signed-off-by: Stanislav Kinsburskii <skinsburskii@linux.microsoft.com>
> > ---
> >  drivers/hv/mshv_root_main.c |   25 +++++++++++++++++++++++--
> >  1 file changed, 23 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/hv/mshv_root_main.c b/drivers/hv/mshv_root_main.c
> > index 48c842b6938d..cb5b4505f8eb 100644
> > --- a/drivers/hv/mshv_root_main.c
> > +++ b/drivers/hv/mshv_root_main.c
> > @@ -39,6 +39,7 @@
> >  #define MSHV_PARTITION_DEPOSIT_PAGES		(SZ_512K >> PAGE_SHIFT)
> >  #define MSHV_PARTITION_DEPOSIT_PAGES_NESTED	(20 * SZ_1M >> PAGE_SHIFT)
> >  #define MSHV_VP_DEPOSIT_PAGES			(1 * SZ_1M >> PAGE_SHIFT)
> > +#define MSHV_1G_DEPOSIT_PAGES			(6 * SZ_1M >> PAGE_SHIFT)
> >
> >  MODULE_AUTHOR("Microsoft");
> >  MODULE_LICENSE("GPL");
> > @@ -1324,6 +1325,18 @@ static int mshv_prepare_pinned_region(struct
mshv_mem_region *region)
> >  	return ret;
> >  }
> >
> > +static u64
> > +mshv_region_deposit_slat_pages(struct mshv_mem_region *region)
> 
> Same nit about the function name. This one seems like it will "deposit slat pages".
> 
> > +{
> > +	u64 region_in_gbs, slat_pages;
> > +
> > +	/* SLAT needs 6 MB per 1 GB of address space. */
> > +	region_in_gbs = DIV_ROUND_UP(region->nr_pages << HV_HYP_PAGE_SHIFT, SZ_1G);
> 
> This local variable "region_in_gbs" is computed in units of bytes.

Ignore this comment and the following one in this function. I saw the
ROUND_UP(), but somehow failed to see that it was DIV_ROUND_UP().  :-(

Michael

> 
> > +	slat_pages = region_in_gbs * MSHV_1G_DEPOSIT_PAGES;
> 
> But here region_in_gbs is used as if it were in units of Gbytes.  So the
> slat_pages return value is much larger than intended.
> 
> > +
> > +	return slat_pages;
> > +}
> > +
> >  /*
> >   * This maps two things: guest RAM and for pci passthru mmio space.
> >   *
> > @@ -1364,6 +1377,11 @@ mshv_map_user_memory(struct mshv_partition *partition,
> >  	if (ret)
> >  		return ret;
> >
> > +	ret = hv_call_deposit_pages(NUMA_NO_NODE, partition->pt_id,
> > +				    mshv_region_deposit_slat_pages(region));
> > +	if (ret)
> > +		goto free_region;
> > +
> >  	switch (region->mreg_type) {
> >  	case MSHV_REGION_TYPE_MEM_PINNED:
> >  		ret = mshv_prepare_pinned_region(region);
> > @@ -1392,7 +1410,7 @@ mshv_map_user_memory(struct mshv_partition *partition,
> >  				   region->hv_map_flags, ret);
> >
> >  	if (ret)
> > -		goto errout;
> > +		goto withdraw_memory;
> >
> >  	spin_lock(&partition->pt_mem_regions_lock);
> >  	hlist_add_head(&region->hnode, &partition->pt_mem_regions);
> > @@ -1400,7 +1418,10 @@ mshv_map_user_memory(struct mshv_partition *partition,
> >
> >  	return 0;
> >
> > -errout:
> > +withdraw_memory:
> > +	hv_call_withdraw_memory(mshv_region_deposit_slat_pages(region),
> > +				NUMA_NO_NODE, partition->pt_id);
> 
> Again, for an L1VH partition, the actual number of pages deposited would
> be 2x what mshv_region_deposit_slat_pages() returns.
> 
> > +free_region:
> >  	vfree(region);
> >  	return ret;
> >  }
> >
> >
>