linux-kernel - Re: [PATCH 1/3] Drivers: hv: Rename a few memory region related functions for clarity

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aNcMgqJrPzcGumUp@skinsburskii.localdomain>
Date: Fri, 26 Sep 2025 14:58:26 -0700
From: Stanislav Kinsburskii <skinsburskii@...ux.microsoft.com>
To: Nuno Das Neves <nunodasneves@...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 1/3] Drivers: hv: Rename a few memory region related
 functions for clarity

On Fri, Sep 26, 2025 at 10:14:25AM -0700, Nuno Das Neves wrote:
> On 9/24/2025 2:31 PM, Stanislav Kinsburskii wrote:
> > A cleanup and precursor patch.
> > 
> This line doesn't add much, I think you can remove it.
> 

It actually means something important: it explains why a change is being
made and that other changes to follow will make more sense out of this
one.

> >  
> >  static int
> > -mshv_region_populate(struct mshv_mem_region *region)
> > +mshv_region_pin(struct mshv_mem_region *region)
> >  {
> > -	return mshv_region_populate_pages(region, 0, region->nr_pages);
> > +	return mshv_region_pin_pages(region, 0, region->nr_pages);
> >  }
> Do we ever partially pin a region? Maybe we don't need a function called
> mshv_region_pin_pages() and we just have mshv_region_pin() instead.
> 

We don't and we likley won't until we support virtio-iommu.
I'll can remove mshv_region_pin_pages.

> >  
> >  static struct mshv_mem_region *
> > @@ -1264,17 +1259,25 @@ static int mshv_partition_create_region(struct mshv_partition *partition,
> >  	return 0;
> >  }
> >  
> > -/*
> > - * Map guest ram. if snp, make sure to release that from the host first
> > - * Side Effects: In case of failure, pages are unpinned when feasible.
> > +/**
> > + * mshv_handle_pinned_region - Handle pinned memory regions
> > + * @region: Pointer to the memory region structure
> > + *
> > + * This function processes memory regions that are explicitly marked as pinned.
> > + * Pinned regions are preallocated, mapped upfront, and do not rely on fault-based
> > + * population. The function ensures the region is properly populated, handles
> > + * encryption requirements for SNP partitions if applicable, maps the region,
> > + * and performs necessary sharing or eviction operations based on the mapping
> > + * result.
> > + *
> > + * Return: 0 on success, negative error code on failure.
> >   */
> > -static int
> > -mshv_partition_mem_region_map(struct mshv_mem_region *region)
> > +static int mshv_handle_pinned_region(struct mshv_mem_region *region)
> 
> Why the verb "handle"? It doesn't provide any information on what the function does,
> when it might be called etc. Maybe mshv_init_pinned_region() ?
> 

I see what you mean. Indeed, "handle" isn't goot, but "init" is quite
overloaded either. I think "mshv_prepare_pinned_region" suit better
here.
Is it okay with you?

Thanks,
Stanilav