[PATCH 1/1] mshv: Add comment about huge page mappings in guest physical address space

[PATCH 1/1] mshv: Add comment about huge page mappings in guest physical address space

[mhkelley58]

From: Michael Kelley <mhklinux@outlook.com>

Huge page mappings in the guest physical address space depend on having
matching alignment of the userspace address in the parent partition and
of the guest physical address. Add a comment that captures this
information. See the link to the mailing list thread.

No code or functional change.

Link: https://lore.kernel.org/linux-hyperv/aUrC94YvscoqBzh3@skinsburskii.localdomain/T/#m0871d2cae9b297fd397ddb8459e534981307c7dc
Signed-off-by: Michael Kelley <mhklinux@outlook.com>
---
 drivers/hv/mshv_root_main.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/hv/mshv_root_main.c b/drivers/hv/mshv_root_main.c
index 681b58154d5e..bc738ff4508e 100644
--- a/drivers/hv/mshv_root_main.c
+++ b/drivers/hv/mshv_root_main.c
@@ -1389,6 +1389,20 @@ mshv_partition_ioctl_set_memory(struct mshv_partition *partition,
 	if (mem.flags & BIT(MSHV_SET_MEM_BIT_UNMAP))
 		return mshv_unmap_user_memory(partition, mem);
 
+	/*
+	 * If the userspace_addr and the guest physical address (as derived
+	 * from the guest_pfn) have the same alignment modulo PMD huge page
+	 * size, the MSHV driver can map any PMD huge pages to the guest
+	 * physical address space as PMD huge pages. If the alignments do
+	 * not match, PMD huge pages must be mapped as single pages in the
+	 * guest physical address space. The MSHV driver does not enforce
+	 * that the alignments match, and it invokes the hypervisor to set
+	 * up correct functional mappings either way. See mshv_chunk_stride().
+	 * The caller of the ioctl is responsible for providing userspace_addr
+	 * and guest_pfn values with matching alignments if it wants the guest
+	 * to get the performance benefits of PMD huge page mappings of its
+	 * physical address space to real system memory.
+	 */
 	return mshv_map_user_memory(partition, mem);
 }
 
-- 
2.25.1

Re: [PATCH 1/1] mshv: Add comment about huge page mappings in guest physical address space

[Stanislav Kinsburskii]

On Mon, Feb 02, 2026 at 08:51:01AM -0800, mhkelley58@gmail.com wrote:
> From: Michael Kelley <mhklinux@outlook.com>
> 
> Huge page mappings in the guest physical address space depend on having
> matching alignment of the userspace address in the parent partition and
> of the guest physical address. Add a comment that captures this
> information. See the link to the mailing list thread.
> 
> No code or functional change.
> 
> Link: https://lore.kernel.org/linux-hyperv/aUrC94YvscoqBzh3@skinsburskii.localdomain/T/#m0871d2cae9b297fd397ddb8459e534981307c7dc
> Signed-off-by: Michael Kelley <mhklinux@outlook.com>
> ---
>  drivers/hv/mshv_root_main.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/hv/mshv_root_main.c b/drivers/hv/mshv_root_main.c
> index 681b58154d5e..bc738ff4508e 100644
> --- a/drivers/hv/mshv_root_main.c
> +++ b/drivers/hv/mshv_root_main.c
> @@ -1389,6 +1389,20 @@ mshv_partition_ioctl_set_memory(struct mshv_partition *partition,
>  	if (mem.flags & BIT(MSHV_SET_MEM_BIT_UNMAP))
>  		return mshv_unmap_user_memory(partition, mem);
>  
> +	/*
> +	 * If the userspace_addr and the guest physical address (as derived
> +	 * from the guest_pfn) have the same alignment modulo PMD huge page
> +	 * size, the MSHV driver can map any PMD huge pages to the guest
> +	 * physical address space as PMD huge pages. If the alignments do
> +	 * not match, PMD huge pages must be mapped as single pages in the
> +	 * guest physical address space. The MSHV driver does not enforce
> +	 * that the alignments match, and it invokes the hypervisor to set
> +	 * up correct functional mappings either way. See mshv_chunk_stride().
> +	 * The caller of the ioctl is responsible for providing userspace_addr
> +	 * and guest_pfn values with matching alignments if it wants the guest
> +	 * to get the performance benefits of PMD huge page mappings of its
> +	 * physical address space to real system memory.
> +	 */

Thanks. However, I'd suggest to reduce this commet a lot and put the
details into the commit message instead. Also, why this place? Why not a
part of the function description instead, for example?

Thanks,
Stanislav

>  	return mshv_map_user_memory(partition, mem);
>  }
>  
> -- 
> 2.25.1

RE: [PATCH 1/1] mshv: Add comment about huge page mappings in guest physical address space

[Michael Kelley]

From: Stanislav Kinsburskii <skinsburskii@linux.microsoft.com> Sent: Monday, February 2, 2026 9:18 AM
> 
> On Mon, Feb 02, 2026 at 08:51:01AM -0800, mhkelley58@gmail.com wrote:
> > From: Michael Kelley <mhklinux@outlook.com>
> >
> > Huge page mappings in the guest physical address space depend on having
> > matching alignment of the userspace address in the parent partition and
> > of the guest physical address. Add a comment that captures this
> > information. See the link to the mailing list thread.
> >
> > No code or functional change.
> >
> > Link: https://lore.kernel.org/linux-hyperv/aUrC94YvscoqBzh3@skinsburskii.localdomain/T/#m0871d2cae9b297fd397ddb8459e534981307c7dc
> > Signed-off-by: Michael Kelley <mhklinux@outlook.com>
> > ---
> >  drivers/hv/mshv_root_main.c | 14 ++++++++++++++
> >  1 file changed, 14 insertions(+)
> >
> > diff --git a/drivers/hv/mshv_root_main.c b/drivers/hv/mshv_root_main.c
> > index 681b58154d5e..bc738ff4508e 100644
> > --- a/drivers/hv/mshv_root_main.c
> > +++ b/drivers/hv/mshv_root_main.c
> > @@ -1389,6 +1389,20 @@ mshv_partition_ioctl_set_memory(struct mshv_partition *partition,
> >  	if (mem.flags & BIT(MSHV_SET_MEM_BIT_UNMAP))
> >  		return mshv_unmap_user_memory(partition, mem);
> >
> > +	/*
> > +	 * If the userspace_addr and the guest physical address (as derived
> > +	 * from the guest_pfn) have the same alignment modulo PMD huge page
> > +	 * size, the MSHV driver can map any PMD huge pages to the guest
> > +	 * physical address space as PMD huge pages. If the alignments do
> > +	 * not match, PMD huge pages must be mapped as single pages in the
> > +	 * guest physical address space. The MSHV driver does not enforce
> > +	 * that the alignments match, and it invokes the hypervisor to set
> > +	 * up correct functional mappings either way. See mshv_chunk_stride().
> > +	 * The caller of the ioctl is responsible for providing userspace_addr
> > +	 * and guest_pfn values with matching alignments if it wants the guest
> > +	 * to get the performance benefits of PMD huge page mappings of its
> > +	 * physical address space to real system memory.
> > +	 */
> 
> Thanks. However, I'd suggest to reduce this commet a lot and put the
> details into the commit message instead. Also, why this place? Why not a
> part of the function description instead, for example?

In general, I'm very much an advocate of putting a bit more detail into code
comments, so that someone new reading the code has a chance of figuring
out what's going on without having to search through the commit history
and read commit messages. The commit history is certainly useful for the
historical record, and especially how things have changed over time. But for
"how non-obvious things work now", I like to see that in the code comments.

As for where to put the comment, I'm flexible. I thought about placing it
outside the function as a "header" (which is what I think you mean by the
"function description"), but the function handles both "map" and "unmap"
operations, and this comment applies only to "map".  Hence I put it after
the test for whether we're doing "map" vs. "unmap".  But I wouldn't object
to it being placed as a function description, though the text would need to be
enhanced to more broadly be a function description instead of just a comment
about a specific aspect of "map" behavior.

Michael

> 
> Thanks,
> Stanislav
> 
> >  	return mshv_map_user_memory(partition, mem);
> >  }
> >
> > --
> > 2.25.1

Re: [PATCH 1/1] mshv: Add comment about huge page mappings in guest physical address space

[Stanislav Kinsburskii]

On Mon, Feb 02, 2026 at 06:26:42PM +0000, Michael Kelley wrote:
> From: Stanislav Kinsburskii <skinsburskii@linux.microsoft.com> Sent: Monday, February 2, 2026 9:18 AM
> > 
> > On Mon, Feb 02, 2026 at 08:51:01AM -0800, mhkelley58@gmail.com wrote:
> > > From: Michael Kelley <mhklinux@outlook.com>
> > >
> > > Huge page mappings in the guest physical address space depend on having
> > > matching alignment of the userspace address in the parent partition and
> > > of the guest physical address. Add a comment that captures this
> > > information. See the link to the mailing list thread.
> > >
> > > No code or functional change.
> > >
> > > Link: https://lore.kernel.org/linux-hyperv/aUrC94YvscoqBzh3@skinsburskii.localdomain/T/#m0871d2cae9b297fd397ddb8459e534981307c7dc
> > > Signed-off-by: Michael Kelley <mhklinux@outlook.com>
> > > ---
> > >  drivers/hv/mshv_root_main.c | 14 ++++++++++++++
> > >  1 file changed, 14 insertions(+)
> > >
> > > diff --git a/drivers/hv/mshv_root_main.c b/drivers/hv/mshv_root_main.c
> > > index 681b58154d5e..bc738ff4508e 100644
> > > --- a/drivers/hv/mshv_root_main.c
> > > +++ b/drivers/hv/mshv_root_main.c
> > > @@ -1389,6 +1389,20 @@ mshv_partition_ioctl_set_memory(struct mshv_partition *partition,
> > >  	if (mem.flags & BIT(MSHV_SET_MEM_BIT_UNMAP))
> > >  		return mshv_unmap_user_memory(partition, mem);
> > >
> > > +	/*
> > > +	 * If the userspace_addr and the guest physical address (as derived
> > > +	 * from the guest_pfn) have the same alignment modulo PMD huge page
> > > +	 * size, the MSHV driver can map any PMD huge pages to the guest
> > > +	 * physical address space as PMD huge pages. If the alignments do
> > > +	 * not match, PMD huge pages must be mapped as single pages in the
> > > +	 * guest physical address space. The MSHV driver does not enforce
> > > +	 * that the alignments match, and it invokes the hypervisor to set
> > > +	 * up correct functional mappings either way. See mshv_chunk_stride().
> > > +	 * The caller of the ioctl is responsible for providing userspace_addr
> > > +	 * and guest_pfn values with matching alignments if it wants the guest
> > > +	 * to get the performance benefits of PMD huge page mappings of its
> > > +	 * physical address space to real system memory.
> > > +	 */
> > 
> > Thanks. However, I'd suggest to reduce this commet a lot and put the
> > details into the commit message instead. Also, why this place? Why not a
> > part of the function description instead, for example?
> 
> In general, I'm very much an advocate of putting a bit more detail into code
> comments, so that someone new reading the code has a chance of figuring
> out what's going on without having to search through the commit history
> and read commit messages. The commit history is certainly useful for the
> historical record, and especially how things have changed over time. But for
> "how non-obvious things work now", I like to see that in the code comments.
> 

This approach is not well aligned with the existing kernel coding style.
It is common to answer the “why” question in the commit message.
Code comments should focus on “what” the code does.

https://www.kernel.org/doc/html/latest/process/coding-style.html

For more details, it is common to use `git blame` to learn the context
of a change when needed.

> As for where to put the comment, I'm flexible. I thought about placing it
> outside the function as a "header" (which is what I think you mean by the
> "function description"), but the function handles both "map" and "unmap"
> operations, and this comment applies only to "map".  Hence I put it after
> the test for whether we're doing "map" vs. "unmap".  But I wouldn't object
> to it being placed as a function description, though the text would need to be
> enhanced to more broadly be a function description instead of just a comment
> about a specific aspect of "map" behavior.
> 

As for the location, since this documents the userspace API, I would
rather place it above the function as part of the function description.
Even though the function handles both map and unmap, unmap also deals
with huge pages.

Thanks,
Stanislav

> Michael
> 
> > 
> > Thanks,
> > Stanislav
> > 
> > >  	return mshv_map_user_memory(partition, mem);
> > >  }
> > >
> > > --
> > > 2.25.1

Re: [PATCH 1/1] mshv: Add comment about huge page mappings in guest physical address space

[Mukesh R]

On 2/2/26 09:17, Stanislav Kinsburskii wrote:
> On Mon, Feb 02, 2026 at 08:51:01AM -0800, mhkelley58@gmail.com wrote:
>> From: Michael Kelley <mhklinux@outlook.com>
>>
>> Huge page mappings in the guest physical address space depend on having
>> matching alignment of the userspace address in the parent partition and
>> of the guest physical address. Add a comment that captures this
>> information. See the link to the mailing list thread.
>>
>> No code or functional change.
>>
>> Link: https://lore.kernel.org/linux-hyperv/aUrC94YvscoqBzh3@skinsburskii.localdomain/T/#m0871d2cae9b297fd397ddb8459e534981307c7dc
>> Signed-off-by: Michael Kelley <mhklinux@outlook.com>
>> ---
>>   drivers/hv/mshv_root_main.c | 14 ++++++++++++++
>>   1 file changed, 14 insertions(+)
>>
>> diff --git a/drivers/hv/mshv_root_main.c b/drivers/hv/mshv_root_main.c
>> index 681b58154d5e..bc738ff4508e 100644
>> --- a/drivers/hv/mshv_root_main.c
>> +++ b/drivers/hv/mshv_root_main.c
>> @@ -1389,6 +1389,20 @@ mshv_partition_ioctl_set_memory(struct mshv_partition *partition,
>>   	if (mem.flags & BIT(MSHV_SET_MEM_BIT_UNMAP))
>>   		return mshv_unmap_user_memory(partition, mem);
>>   
>> +	/*
>> +	 * If the userspace_addr and the guest physical address (as derived
>> +	 * from the guest_pfn) have the same alignment modulo PMD huge page
>> +	 * size, the MSHV driver can map any PMD huge pages to the guest
>> +	 * physical address space as PMD huge pages. If the alignments do
>> +	 * not match, PMD huge pages must be mapped as single pages in the
>> +	 * guest physical address space. The MSHV driver does not enforce
>> +	 * that the alignments match, and it invokes the hypervisor to set
>> +	 * up correct functional mappings either way. See mshv_chunk_stride().
>> +	 * The caller of the ioctl is responsible for providing userspace_addr
>> +	 * and guest_pfn values with matching alignments if it wants the guest
>> +	 * to get the performance benefits of PMD huge page mappings of its
>> +	 * physical address space to real system memory.
>> +	 */
> 
> Thanks. However, I'd suggest to reduce this commet a lot and put the
> details into the commit message instead. Also, why this place? Why not a
> part of the function description instead, for example?

Fwiw, I also prefer this in the function prologue. IMO, larger comments
belong outside the function rather than inside, unless of course cases
where it has to be that way. This makes functions easier to study.

Thanks,
-Mukesh



> Thanks,
> Stanislav
> 
>>   	return mshv_map_user_memory(partition, mem);
>>   }
>>   
>> -- 
>> 2.25.1

RE: [PATCH 1/1] mshv: Add comment about huge page mappings in guest physical address space

[Michael Kelley]

From: Stanislav Kinsburskii <skinsburskii@linux.microsoft.com> Sent: Monday, February 2, 2026 10:56 AM
> 
> On Mon, Feb 02, 2026 at 06:26:42PM +0000, Michael Kelley wrote:
> > From: Stanislav Kinsburskii <skinsburskii@linux.microsoft.com> Sent: Monday, February 2, 2026 9:18 AM
> > >
> > > On Mon, Feb 02, 2026 at 08:51:01AM -0800, mhkelley58@gmail.com wrote:
> > > > From: Michael Kelley <mhklinux@outlook.com>
> > > >
> > > > Huge page mappings in the guest physical address space depend on having
> > > > matching alignment of the userspace address in the parent partition and
> > > > of the guest physical address. Add a comment that captures this
> > > > information. See the link to the mailing list thread.
> > > >
> > > > No code or functional change.
> > > >
> > > > Link: https://lore.kernel.org/linux-hyperv/aUrC94YvscoqBzh3@skinsburskii.localdomain/T/#m0871d2cae9b297fd397ddb8459e534981307c7dc
> > > > Signed-off-by: Michael Kelley <mhklinux@outlook.com>
> > > > ---
> > > >  drivers/hv/mshv_root_main.c | 14 ++++++++++++++
> > > >  1 file changed, 14 insertions(+)
> > > >
> > > > diff --git a/drivers/hv/mshv_root_main.c b/drivers/hv/mshv_root_main.c
> > > > index 681b58154d5e..bc738ff4508e 100644
> > > > --- a/drivers/hv/mshv_root_main.c
> > > > +++ b/drivers/hv/mshv_root_main.c
> > > > @@ -1389,6 +1389,20 @@ mshv_partition_ioctl_set_memory(struct mshv_partition *partition,
> > > >  	if (mem.flags & BIT(MSHV_SET_MEM_BIT_UNMAP))
> > > >  		return mshv_unmap_user_memory(partition, mem);
> > > >
> > > > +	/*
> > > > +	 * If the userspace_addr and the guest physical address (as derived
> > > > +	 * from the guest_pfn) have the same alignment modulo PMD huge page
> > > > +	 * size, the MSHV driver can map any PMD huge pages to the guest
> > > > +	 * physical address space as PMD huge pages. If the alignments do
> > > > +	 * not match, PMD huge pages must be mapped as single pages in the
> > > > +	 * guest physical address space. The MSHV driver does not enforce
> > > > +	 * that the alignments match, and it invokes the hypervisor to set
> > > > +	 * up correct functional mappings either way. See mshv_chunk_stride().
> > > > +	 * The caller of the ioctl is responsible for providing userspace_addr
> > > > +	 * and guest_pfn values with matching alignments if it wants the guest
> > > > +	 * to get the performance benefits of PMD huge page mappings of its
> > > > +	 * physical address space to real system memory.
> > > > +	 */
> > >
> > > Thanks. However, I'd suggest to reduce this commet a lot and put the
> > > details into the commit message instead. Also, why this place? Why not a
> > > part of the function description instead, for example?
> >
> > In general, I'm very much an advocate of putting a bit more detail into code
> > comments, so that someone new reading the code has a chance of figuring
> > out what's going on without having to search through the commit history
> > and read commit messages. The commit history is certainly useful for the
> > historical record, and especially how things have changed over time. But for
> > "how non-obvious things work now", I like to see that in the code comments.
> >
> 
> This approach is not well aligned with the existing kernel coding style.
> It is common to answer the "why" question in the commit message.
> Code comments should focus on "what" the code does.
> 
> https://www.kernel.org/doc/html/latest/process/coding-style.html
> 

Which says "Instead, put the comments at the head of the function,
telling people what it does, and possibly WHY it does it." I'm good with
that approach.

> For more details, it is common to use `git blame` to learn the context
> of a change when needed.

Yep, I use that all the time for the historical record.

> 
> > As for where to put the comment, I'm flexible. I thought about placing it
> > outside the function as a "header" (which is what I think you mean by the
> > "function description"), but the function handles both "map" and "unmap"
> > operations, and this comment applies only to "map".  Hence I put it after
> > the test for whether we're doing "map" vs. "unmap".  But I wouldn't object
> > to it being placed as a function description, though the text would need to be
> > enhanced to more broadly be a function description instead of just a comment
> > about a specific aspect of "map" behavior.
> >
> 
> As for the location, since this documents the userspace API, I would
> rather place it above the function as part of the function description.
> Even though the function handles both map and unmap, unmap also deals
> with huge pages.

I'll do a version written as the function description. But the full function
description will be more extensive to cover all the "what" that this function
implements:
* input parameters, and their valid values
* map and unmap
* when pinned vs. movable vs. mmio regions are created
* what is done with huge pages in the above cases (i.e., a massaged version
   of what I've already written)
* populating and pinning of pages for pinned regions

Does that match with your expectations?

Michael

Re: [PATCH 1/1] mshv: Add comment about huge page mappings in guest physical address space

[Stanislav Kinsburskii]

On Tue, Feb 03, 2026 at 06:35:40PM +0000, Michael Kelley wrote:
> From: Stanislav Kinsburskii <skinsburskii@linux.microsoft.com> Sent: Monday, February 2, 2026 10:56 AM
> > 
> > On Mon, Feb 02, 2026 at 06:26:42PM +0000, Michael Kelley wrote:
> > > From: Stanislav Kinsburskii <skinsburskii@linux.microsoft.com> Sent: Monday, February 2, 2026 9:18 AM
> > > >
> > > > On Mon, Feb 02, 2026 at 08:51:01AM -0800, mhkelley58@gmail.com wrote:
> > > > > From: Michael Kelley <mhklinux@outlook.com>
> > > > >
> > > > > Huge page mappings in the guest physical address space depend on having
> > > > > matching alignment of the userspace address in the parent partition and
> > > > > of the guest physical address. Add a comment that captures this
> > > > > information. See the link to the mailing list thread.
> > > > >
> > > > > No code or functional change.
> > > > >
> > > > > Link: https://lore.kernel.org/linux-hyperv/aUrC94YvscoqBzh3@skinsburskii.localdomain/T/#m0871d2cae9b297fd397ddb8459e534981307c7dc
> > > > > Signed-off-by: Michael Kelley <mhklinux@outlook.com>
> > > > > ---
> > > > >  drivers/hv/mshv_root_main.c | 14 ++++++++++++++
> > > > >  1 file changed, 14 insertions(+)
> > > > >
> > > > > diff --git a/drivers/hv/mshv_root_main.c b/drivers/hv/mshv_root_main.c
> > > > > index 681b58154d5e..bc738ff4508e 100644
> > > > > --- a/drivers/hv/mshv_root_main.c
> > > > > +++ b/drivers/hv/mshv_root_main.c
> > > > > @@ -1389,6 +1389,20 @@ mshv_partition_ioctl_set_memory(struct mshv_partition *partition,
> > > > >  	if (mem.flags & BIT(MSHV_SET_MEM_BIT_UNMAP))
> > > > >  		return mshv_unmap_user_memory(partition, mem);
> > > > >
> > > > > +	/*
> > > > > +	 * If the userspace_addr and the guest physical address (as derived
> > > > > +	 * from the guest_pfn) have the same alignment modulo PMD huge page
> > > > > +	 * size, the MSHV driver can map any PMD huge pages to the guest
> > > > > +	 * physical address space as PMD huge pages. If the alignments do
> > > > > +	 * not match, PMD huge pages must be mapped as single pages in the
> > > > > +	 * guest physical address space. The MSHV driver does not enforce
> > > > > +	 * that the alignments match, and it invokes the hypervisor to set
> > > > > +	 * up correct functional mappings either way. See mshv_chunk_stride().
> > > > > +	 * The caller of the ioctl is responsible for providing userspace_addr
> > > > > +	 * and guest_pfn values with matching alignments if it wants the guest
> > > > > +	 * to get the performance benefits of PMD huge page mappings of its
> > > > > +	 * physical address space to real system memory.
> > > > > +	 */
> > > >
> > > > Thanks. However, I'd suggest to reduce this commet a lot and put the
> > > > details into the commit message instead. Also, why this place? Why not a
> > > > part of the function description instead, for example?
> > >
> > > In general, I'm very much an advocate of putting a bit more detail into code
> > > comments, so that someone new reading the code has a chance of figuring
> > > out what's going on without having to search through the commit history
> > > and read commit messages. The commit history is certainly useful for the
> > > historical record, and especially how things have changed over time. But for
> > > "how non-obvious things work now", I like to see that in the code comments.
> > >
> > 
> > This approach is not well aligned with the existing kernel coding style.
> > It is common to answer the "why" question in the commit message.
> > Code comments should focus on "what" the code does.
> > 
> > https://www.kernel.org/doc/html/latest/process/coding-style.html
> > 
> 
> Which says "Instead, put the comments at the head of the function,
> telling people what it does, and possibly WHY it does it." I'm good with
> that approach.
> 
> > For more details, it is common to use `git blame` to learn the context
> > of a change when needed.
> 
> Yep, I use that all the time for the historical record.
> 
> > 
> > > As for where to put the comment, I'm flexible. I thought about placing it
> > > outside the function as a "header" (which is what I think you mean by the
> > > "function description"), but the function handles both "map" and "unmap"
> > > operations, and this comment applies only to "map".  Hence I put it after
> > > the test for whether we're doing "map" vs. "unmap".  But I wouldn't object
> > > to it being placed as a function description, though the text would need to be
> > > enhanced to more broadly be a function description instead of just a comment
> > > about a specific aspect of "map" behavior.
> > >
> > 
> > As for the location, since this documents the userspace API, I would
> > rather place it above the function as part of the function description.
> > Even though the function handles both map and unmap, unmap also deals
> > with huge pages.
> 
> I'll do a version written as the function description. But the full function
> description will be more extensive to cover all the "what" that this function
> implements:
> * input parameters, and their valid values
> * map and unmap
> * when pinned vs. movable vs. mmio regions are created
> * what is done with huge pages in the above cases (i.e., a massaged version
>    of what I've already written)
> * populating and pinning of pages for pinned regions
> 
> Does that match with your expectations?

I’d rather suggest something simpler for the function header:

* What regions are created
* What pages sizes are supported

I.e. describe what the function does, not the rationale or the
architecture behind it.

For example, something like this (suggested by AI, feel free to rewrite
completly):

 * Depending on the request, the region is created as pinned RAM, movable RAM,
 * or MMIO. PMD-sized huge page mappings are supported when the userspace
 * address and guest physical address (guest_pfn << PAGE_SHIFT) have matching
 * alignment modulo PMD_SIZE; otherwise the mapping is established using base
 * pages.

The rationale and architecture can be put into the commit message.

Thanks,
Stanislav

> Michael

RE: [PATCH 1/1] mshv: Add comment about huge page mappings in guest physical address space

[Michael Kelley]

From: Stanislav Kinsburskii <skinsburskii@linux.microsoft.com> Sent: Tuesday, February 3, 2026 4:55 PM
> 
> On Tue, Feb 03, 2026 at 06:35:40PM +0000, Michael Kelley wrote:
> > From: Stanislav Kinsburskii <skinsburskii@linux.microsoft.com> Sent: Monday,
> February 2, 2026 10:56 AM
> > >
> > > On Mon, Feb 02, 2026 at 06:26:42PM +0000, Michael Kelley wrote:
> > > > From: Stanislav Kinsburskii <skinsburskii@linux.microsoft.com> Sent: Monday, February 2, 2026 9:18 AM
> > > > >
> > > > > On Mon, Feb 02, 2026 at 08:51:01AM -0800, mhkelley58@gmail.com wrote:
> > > > > > From: Michael Kelley <mhklinux@outlook.com>
> > > > > >
> > > > > > Huge page mappings in the guest physical address space depend on having
> > > > > > matching alignment of the userspace address in the parent partition and
> > > > > > of the guest physical address. Add a comment that captures this
> > > > > > information. See the link to the mailing list thread.
> > > > > >
> > > > > > No code or functional change.
> > > > > >
> > > > > > Link: https://lore.kernel.org/linux-hyperv/aUrC94YvscoqBzh3@skinsburskii.localdomain/T/#m0871d2cae9b297fd397ddb8459e534981307c7dc
> > > > > > Signed-off-by: Michael Kelley <mhklinux@outlook.com>
> > > > > > ---
> > > > > >  drivers/hv/mshv_root_main.c | 14 ++++++++++++++
> > > > > >  1 file changed, 14 insertions(+)
> > > > > >
> > > > > > diff --git a/drivers/hv/mshv_root_main.c b/drivers/hv/mshv_root_main.c
> > > > > > index 681b58154d5e..bc738ff4508e 100644
> > > > > > --- a/drivers/hv/mshv_root_main.c
> > > > > > +++ b/drivers/hv/mshv_root_main.c
> > > > > > @@ -1389,6 +1389,20 @@ mshv_partition_ioctl_set_memory(struct mshv_partition *partition,
> > > > > >  	if (mem.flags & BIT(MSHV_SET_MEM_BIT_UNMAP))
> > > > > >  		return mshv_unmap_user_memory(partition, mem);
> > > > > >
> > > > > > +	/*
> > > > > > +	 * If the userspace_addr and the guest physical address (as derived
> > > > > > +	 * from the guest_pfn) have the same alignment modulo PMD huge page
> > > > > > +	 * size, the MSHV driver can map any PMD huge pages to the guest
> > > > > > +	 * physical address space as PMD huge pages. If the alignments do
> > > > > > +	 * not match, PMD huge pages must be mapped as single pages in the
> > > > > > +	 * guest physical address space. The MSHV driver does not enforce
> > > > > > +	 * that the alignments match, and it invokes the hypervisor to set
> > > > > > +	 * up correct functional mappings either way. See mshv_chunk_stride().
> > > > > > +	 * The caller of the ioctl is responsible for providing userspace_addr
> > > > > > +	 * and guest_pfn values with matching alignments if it wants the guest
> > > > > > +	 * to get the performance benefits of PMD huge page mappings of its
> > > > > > +	 * physical address space to real system memory.
> > > > > > +	 */
> > > > >
> > > > > Thanks. However, I'd suggest to reduce this commet a lot and put the
> > > > > details into the commit message instead. Also, why this place? Why not a
> > > > > part of the function description instead, for example?
> > > >
> > > > In general, I'm very much an advocate of putting a bit more detail into code
> > > > comments, so that someone new reading the code has a chance of figuring
> > > > out what's going on without having to search through the commit history
> > > > and read commit messages. The commit history is certainly useful for the
> > > > historical record, and especially how things have changed over time. But for
> > > > "how non-obvious things work now", I like to see that in the code comments.
> > > >
> > >
> > > This approach is not well aligned with the existing kernel coding style.
> > > It is common to answer the "why" question in the commit message.
> > > Code comments should focus on "what" the code does.
> > >
> > > https://www.kernel.org/doc/html/latest/process/coding-style.html
> > >
> >
> > Which says "Instead, put the comments at the head of the function,
> > telling people what it does, and possibly WHY it does it." I'm good with
> > that approach.
> >
> > > For more details, it is common to use `git blame` to learn the context
> > > of a change when needed.
> >
> > Yep, I use that all the time for the historical record.
> >
> > >
> > > > As for where to put the comment, I'm flexible. I thought about placing it
> > > > outside the function as a "header" (which is what I think you mean by the
> > > > "function description"), but the function handles both "map" and "unmap"
> > > > operations, and this comment applies only to "map".  Hence I put it after
> > > > the test for whether we're doing "map" vs. "unmap".  But I wouldn't object
> > > > to it being placed as a function description, though the text would need to be
> > > > enhanced to more broadly be a function description instead of just a comment
> > > > about a specific aspect of "map" behavior.
> > > >
> > >
> > > As for the location, since this documents the userspace API, I would
> > > rather place it above the function as part of the function description.
> > > Even though the function handles both map and unmap, unmap also deals
> > > with huge pages.
> >
> > I'll do a version written as the function description. But the full function
> > description will be more extensive to cover all the "what" that this function
> > implements:
> > * input parameters, and their valid values
> > * map and unmap
> > * when pinned vs. movable vs. mmio regions are created
> > * what is done with huge pages in the above cases (i.e., a massaged version
> >    of what I've already written)
> > * populating and pinning of pages for pinned regions
> >
> > Does that match with your expectations?
> 
> I'd rather suggest something simpler for the function header:
> 
> * What regions are created
> * What pages sizes are supported
> 
> I.e. describe what the function does, not the rationale or the
> architecture behind it.
> 
> For example, something like this (suggested by AI, feel free to rewrite
> completly):
> 
>  * Depending on the request, the region is created as pinned RAM, movable RAM,
>  * or MMIO. PMD-sized huge page mappings are supported when the userspace
>  * address and guest physical address (guest_pfn << PAGE_SHIFT) have matching
>  * alignment modulo PMD_SIZE; otherwise the mapping is established using base
>  * pages.
> 
> The rationale and architecture can be put into the commit message.

I really disagree with the approach you are suggesting. In my view, putting
a detailed description in the function header is completely aligned with the
"Commenting" section of the Coding Guidelines. We're not in danger of over
commenting or commenting on trivialities. My goal is always to be as helpful
as possible to whoever comes next in reviewing or updating the code. I
think that's putting the information front-and-center in the function header,
or in comments within a function to call out noteworthy aspects.

But we're at an impasse here, and further discussion is not likely to resolve it.
You and the Microsoft team are custodians of this code, so I'll stand down. I
can't do justice to the approach you prefer, so let's drop my patch. You, or
someone else who can embrace your approach, can submit a new patch that
does so, and I won't object.

Michael

Re: [PATCH 1/1] mshv: Add comment about huge page mappings in guest physical address space

[Anirudh Rayabharam]

On Tue, Feb 03, 2026 at 06:35:40PM +0000, Michael Kelley wrote:
> From: Stanislav Kinsburskii <skinsburskii@linux.microsoft.com> Sent: Monday, February 2, 2026 10:56 AM
> > 
> > On Mon, Feb 02, 2026 at 06:26:42PM +0000, Michael Kelley wrote:
> > > From: Stanislav Kinsburskii <skinsburskii@linux.microsoft.com> Sent: Monday, February 2, 2026 9:18 AM
> > > >
> > > > On Mon, Feb 02, 2026 at 08:51:01AM -0800, mhkelley58@gmail.com wrote:
> > > > > From: Michael Kelley <mhklinux@outlook.com>
> > > > >
> > > > > Huge page mappings in the guest physical address space depend on having
> > > > > matching alignment of the userspace address in the parent partition and
> > > > > of the guest physical address. Add a comment that captures this
> > > > > information. See the link to the mailing list thread.
> > > > >
> > > > > No code or functional change.
> > > > >
> > > > > Link: https://lore.kernel.org/linux-hyperv/aUrC94YvscoqBzh3@skinsburskii.localdomain/T/#m0871d2cae9b297fd397ddb8459e534981307c7dc
> > > > > Signed-off-by: Michael Kelley <mhklinux@outlook.com>
> > > > > ---
> > > > >  drivers/hv/mshv_root_main.c | 14 ++++++++++++++
> > > > >  1 file changed, 14 insertions(+)
> > > > >
> > > > > diff --git a/drivers/hv/mshv_root_main.c b/drivers/hv/mshv_root_main.c
> > > > > index 681b58154d5e..bc738ff4508e 100644
> > > > > --- a/drivers/hv/mshv_root_main.c
> > > > > +++ b/drivers/hv/mshv_root_main.c
> > > > > @@ -1389,6 +1389,20 @@ mshv_partition_ioctl_set_memory(struct mshv_partition *partition,
> > > > >  	if (mem.flags & BIT(MSHV_SET_MEM_BIT_UNMAP))
> > > > >  		return mshv_unmap_user_memory(partition, mem);
> > > > >
> > > > > +	/*
> > > > > +	 * If the userspace_addr and the guest physical address (as derived
> > > > > +	 * from the guest_pfn) have the same alignment modulo PMD huge page
> > > > > +	 * size, the MSHV driver can map any PMD huge pages to the guest
> > > > > +	 * physical address space as PMD huge pages. If the alignments do
> > > > > +	 * not match, PMD huge pages must be mapped as single pages in the
> > > > > +	 * guest physical address space. The MSHV driver does not enforce
> > > > > +	 * that the alignments match, and it invokes the hypervisor to set
> > > > > +	 * up correct functional mappings either way. See mshv_chunk_stride().
> > > > > +	 * The caller of the ioctl is responsible for providing userspace_addr
> > > > > +	 * and guest_pfn values with matching alignments if it wants the guest
> > > > > +	 * to get the performance benefits of PMD huge page mappings of its
> > > > > +	 * physical address space to real system memory.
> > > > > +	 */
> > > >
> > > > Thanks. However, I'd suggest to reduce this commet a lot and put the
> > > > details into the commit message instead. Also, why this place? Why not a
> > > > part of the function description instead, for example?
> > >
> > > In general, I'm very much an advocate of putting a bit more detail into code
> > > comments, so that someone new reading the code has a chance of figuring
> > > out what's going on without having to search through the commit history
> > > and read commit messages. The commit history is certainly useful for the
> > > historical record, and especially how things have changed over time. But for
> > > "how non-obvious things work now", I like to see that in the code comments.
> > >
> > 
> > This approach is not well aligned with the existing kernel coding style.
> > It is common to answer the "why" question in the commit message.
> > Code comments should focus on "what" the code does.
> > 
> > https://www.kernel.org/doc/html/latest/process/coding-style.html
> > 
> 
> Which says "Instead, put the comments at the head of the function,
> telling people what it does, and possibly WHY it does it." I'm good with
> that approach.
> 
> > For more details, it is common to use `git blame` to learn the context
> > of a change when needed.
> 
> Yep, I use that all the time for the historical record.
> 
> > 
> > > As for where to put the comment, I'm flexible. I thought about placing it
> > > outside the function as a "header" (which is what I think you mean by the
> > > "function description"), but the function handles both "map" and "unmap"
> > > operations, and this comment applies only to "map".  Hence I put it after
> > > the test for whether we're doing "map" vs. "unmap".  But I wouldn't object
> > > to it being placed as a function description, though the text would need to be
> > > enhanced to more broadly be a function description instead of just a comment
> > > about a specific aspect of "map" behavior.
> > >
> > 
> > As for the location, since this documents the userspace API, I would
> > rather place it above the function as part of the function description.
> > Even though the function handles both map and unmap, unmap also deals
> > with huge pages.
> 
> I'll do a version written as the function description. But the full function
> description will be more extensive to cover all the "what" that this function
> implements:
> * input parameters, and their valid values
> * map and unmap
> * when pinned vs. movable vs. mmio regions are created
> * what is done with huge pages in the above cases (i.e., a massaged version
>    of what I've already written)
> * populating and pinning of pages for pinned regions

I'm happy to approve such a version of this patch.

Also, if you want to limit yourself to the map behavior and not unmap,
you could also place this in the description of mshv_map_user_memory().
I would happily approve such a patch as well.

Overall, I think your comment is very useful and points out things that
are easy to miss while reading, modifying or reviewing this code in the
future. I also believe that this information is better as a comment here
than a commit message as has been suggested elsewhere in this thread.

Thanks,
Anirudh.