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
