linux-hyperv.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] Fix uio_hv_generic on 64k page systems
@ 2025-04-18  0:43 longli
  2025-04-18  0:43 ` [PATCH 1/2] Drivers: hv: Allocate interrupt and monitor pages aligned to system page boundary longli
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: longli @ 2025-04-18  0:43 UTC (permalink / raw)
  To: K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
	Greg Kroah-Hartman, linux-hyperv, linux-kernel
  Cc: Long Li

From: Long Li <longli@microsoft.com>

UIO framework requires the device memory aligned to page boundary.
Hyper-V may allocate some memory that is Hyper-V page aligned (4k)
but not system page aligned.

Fix this by having Hyper-V always allocate those pages on system page
boundary and expose them to user-mode.

Long Li (2):
  Drivers: hv: Allocate interrupt and monitor pages aligned to system
    page boundary
  uio_hv_generic: Use correct size for interrupt and monitor pages

 drivers/hv/hv_common.c       | 29 +++++++----------------------
 drivers/uio/uio_hv_generic.c |  4 ++--
 2 files changed, 9 insertions(+), 24 deletions(-)

-- 
2.34.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/2] Drivers: hv: Allocate interrupt and monitor pages aligned to system page boundary
  2025-04-18  0:43 [PATCH 0/2] Fix uio_hv_generic on 64k page systems longli
@ 2025-04-18  0:43 ` longli
  2025-04-20 23:57   ` Michael Kelley
  2025-04-18  0:43 ` [PATCH 2/2] uio_hv_generic: Use correct size for interrupt and monitor pages longli
  2025-04-20 23:57 ` [PATCH 0/2] Fix uio_hv_generic on 64k page systems Michael Kelley
  2 siblings, 1 reply; 11+ messages in thread
From: longli @ 2025-04-18  0:43 UTC (permalink / raw)
  To: K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
	Greg Kroah-Hartman, linux-hyperv, linux-kernel
  Cc: Long Li, stable

From: Long Li <longli@microsoft.com>

There are use cases that interrupt and monitor pages are mapped to
user-mode through UIO, they need to be system page aligned. Some Hyper-V
allocation APIs introduced earlier broke those requirements.

Fix those APIs by always allocating Hyper-V page at system page boundaries.

Cc: stable@vger.kernel.org
Fixes: ca48739e59df ("Drivers: hv: vmbus: Move Hyper-V page allocator to arch neutral code")
Signed-off-by: Long Li <longli@microsoft.com>
---
 drivers/hv/hv_common.c | 29 +++++++----------------------
 1 file changed, 7 insertions(+), 22 deletions(-)

diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c
index a7d7494feaca..f426aaa9b8f9 100644
--- a/drivers/hv/hv_common.c
+++ b/drivers/hv/hv_common.c
@@ -106,41 +106,26 @@ void __init hv_common_free(void)
 }
 
 /*
- * Functions for allocating and freeing memory with size and
- * alignment HV_HYP_PAGE_SIZE. These functions are needed because
- * the guest page size may not be the same as the Hyper-V page
- * size. We depend upon kmalloc() aligning power-of-two size
- * allocations to the allocation size boundary, so that the
- * allocated memory appears to Hyper-V as a page of the size
- * it expects.
+ * A Hyper-V page can be used by UIO for mapping to user-space, it should
+ * always be allocated on system page boundaries.
  */
-
 void *hv_alloc_hyperv_page(void)
 {
-	BUILD_BUG_ON(PAGE_SIZE <  HV_HYP_PAGE_SIZE);
-
-	if (PAGE_SIZE == HV_HYP_PAGE_SIZE)
-		return (void *)__get_free_page(GFP_KERNEL);
-	else
-		return kmalloc(HV_HYP_PAGE_SIZE, GFP_KERNEL);
+	BUILD_BUG_ON(PAGE_SIZE < HV_HYP_PAGE_SIZE);
+	return (void *)__get_free_page(GFP_KERNEL);
 }
 EXPORT_SYMBOL_GPL(hv_alloc_hyperv_page);
 
 void *hv_alloc_hyperv_zeroed_page(void)
 {
-	if (PAGE_SIZE == HV_HYP_PAGE_SIZE)
-		return (void *)__get_free_page(GFP_KERNEL | __GFP_ZERO);
-	else
-		return kzalloc(HV_HYP_PAGE_SIZE, GFP_KERNEL);
+	BUILD_BUG_ON(PAGE_SIZE < HV_HYP_PAGE_SIZE);
+	return (void *)__get_free_page(GFP_KERNEL | __GFP_ZERO);
 }
 EXPORT_SYMBOL_GPL(hv_alloc_hyperv_zeroed_page);
 
 void hv_free_hyperv_page(void *addr)
 {
-	if (PAGE_SIZE == HV_HYP_PAGE_SIZE)
-		free_page((unsigned long)addr);
-	else
-		kfree(addr);
+	free_page((unsigned long)addr);
 }
 EXPORT_SYMBOL_GPL(hv_free_hyperv_page);
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/2] uio_hv_generic: Use correct size for interrupt and monitor pages
  2025-04-18  0:43 [PATCH 0/2] Fix uio_hv_generic on 64k page systems longli
  2025-04-18  0:43 ` [PATCH 1/2] Drivers: hv: Allocate interrupt and monitor pages aligned to system page boundary longli
@ 2025-04-18  0:43 ` longli
  2025-04-20 23:58   ` Michael Kelley
  2025-04-20 23:57 ` [PATCH 0/2] Fix uio_hv_generic on 64k page systems Michael Kelley
  2 siblings, 1 reply; 11+ messages in thread
From: longli @ 2025-04-18  0:43 UTC (permalink / raw)
  To: K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
	Greg Kroah-Hartman, linux-hyperv, linux-kernel
  Cc: Long Li, stable

From: Long Li <longli@microsoft.com>

Interrupt and monitor pages should be in Hyper-V page size (4k bytes).
This can be different to the system page size.

Cc: stable@vger.kernel.org
Fixes: 95096f2fbd10 ("uio-hv-generic: new userspace i/o driver for VMBus")
Signed-off-by: Long Li <longli@microsoft.com>
---
 drivers/uio/uio_hv_generic.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/uio/uio_hv_generic.c b/drivers/uio/uio_hv_generic.c
index 1b19b5647495..08385b04c4ab 100644
--- a/drivers/uio/uio_hv_generic.c
+++ b/drivers/uio/uio_hv_generic.c
@@ -287,13 +287,13 @@ hv_uio_probe(struct hv_device *dev,
 	pdata->info.mem[INT_PAGE_MAP].name = "int_page";
 	pdata->info.mem[INT_PAGE_MAP].addr
 		= (uintptr_t)vmbus_connection.int_page;
-	pdata->info.mem[INT_PAGE_MAP].size = PAGE_SIZE;
+	pdata->info.mem[INT_PAGE_MAP].size = HV_HYP_PAGE_SIZE;
 	pdata->info.mem[INT_PAGE_MAP].memtype = UIO_MEM_LOGICAL;
 
 	pdata->info.mem[MON_PAGE_MAP].name = "monitor_page";
 	pdata->info.mem[MON_PAGE_MAP].addr
 		= (uintptr_t)vmbus_connection.monitor_pages[1];
-	pdata->info.mem[MON_PAGE_MAP].size = PAGE_SIZE;
+	pdata->info.mem[MON_PAGE_MAP].size = HV_HYP_PAGE_SIZE;
 	pdata->info.mem[MON_PAGE_MAP].memtype = UIO_MEM_LOGICAL;
 
 	if (channel->device_id == HV_NIC) {
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* RE: [PATCH 0/2] Fix uio_hv_generic on 64k page systems
  2025-04-18  0:43 [PATCH 0/2] Fix uio_hv_generic on 64k page systems longli
  2025-04-18  0:43 ` [PATCH 1/2] Drivers: hv: Allocate interrupt and monitor pages aligned to system page boundary longli
  2025-04-18  0:43 ` [PATCH 2/2] uio_hv_generic: Use correct size for interrupt and monitor pages longli
@ 2025-04-20 23:57 ` Michael Kelley
  2025-04-23 18:30   ` Long Li
  2 siblings, 1 reply; 11+ messages in thread
From: Michael Kelley @ 2025-04-20 23:57 UTC (permalink / raw)
  To: longli@linuxonhyperv.com, K. Y. Srinivasan, Haiyang Zhang,
	Wei Liu, Dexuan Cui, Greg Kroah-Hartman,
	linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org
  Cc: Long Li

From: longli@linuxonhyperv.com <longli@linuxonhyperv.com>
> 

The Subject line of this cover letter is a bit too narrow in scope. The scope
should be any page size larger than 4K. For example, the arm64 architecture
permits a page size of 16K, and Linux kernels built that way work just fine
on Hyper-V arm64 hosts. Perhaps:

    Fix uio_hv_generic for guests with page size > 4 KiB

> UIO framework requires the device memory aligned to page boundary.
> Hyper-V may allocate some memory that is Hyper-V page aligned (4k)
> but not system page aligned.
> 
> Fix this by having Hyper-V always allocate those pages on system page
> boundary and expose them to user-mode.

Also within the scope of making uio_hv_generic work with page size > 4KiB,
there's an issue with the ring size. When hv_dev_ring_size() returns 0,
hv_uio_probe() uses 2 MiB as the ring size. That works OK with the larger
page sizes. But when hv_dev_ring_size() returns a specific value, it might
not work. The fcopy device returns 16 KiB, which will fail. hv_uio_probe()
needs to use the VMBUS_RING_SIZE() macro to increase the ring size if
necessary to handle the larger ring header that results if the page size
is > 4 KiB.  You might want to include such a patch in this series.

Separately, tools/hv/vmbus_bufring.c needs work to operate correctly on
arm64 and with page sizes > 4 KiB. But that's probably a different patch
series.

Michael

> 
> Long Li (2):
>   Drivers: hv: Allocate interrupt and monitor pages aligned to system
>     page boundary
>   uio_hv_generic: Use correct size for interrupt and monitor pages
> 
>  drivers/hv/hv_common.c       | 29 +++++++----------------------
>  drivers/uio/uio_hv_generic.c |  4 ++--
>  2 files changed, 9 insertions(+), 24 deletions(-)
> 
> --
> 2.34.1
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH 1/2] Drivers: hv: Allocate interrupt and monitor pages aligned to system page boundary
  2025-04-18  0:43 ` [PATCH 1/2] Drivers: hv: Allocate interrupt and monitor pages aligned to system page boundary longli
@ 2025-04-20 23:57   ` Michael Kelley
  2025-04-23 18:39     ` Long Li
  0 siblings, 1 reply; 11+ messages in thread
From: Michael Kelley @ 2025-04-20 23:57 UTC (permalink / raw)
  To: longli@linuxonhyperv.com, K. Y. Srinivasan, Haiyang Zhang,
	Wei Liu, Dexuan Cui, Greg Kroah-Hartman,
	linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org
  Cc: Long Li, stable@vger.kernel.org

From: longli@linuxonhyperv.com <longli@linuxonhyperv.com> Sent: Thursday, April 17, 2025 5:43 PM
> 
> There are use cases that interrupt and monitor pages are mapped to
> user-mode through UIO, they need to be system page aligned. Some Hyper-V
> allocation APIs introduced earlier broke those requirements.
> 
> Fix those APIs by always allocating Hyper-V page at system page boundaries.

I'd suggest doing away with the hv_alloc/free_*() functions entirely since
they are now reduced to just being a wrapper around __get_free_pages(),
which doesn't add any value. Once all the arm64 support and CoCo VM
code settled out, it turned out that these functions to allocate Hyper-V
size pages had dwindling usage.

Allocation of the interrupt and monitor pages can use __get_free_pages()
directly, and that properly captures the need for those allocations to be a
full page. Just add a comment that this wastes space when PAGE_SIZE
> HV_HYP_PAGE_SIZE, but is necessary because the page may be mapped
into user space by uio_hv_generic.

The only other use is in hv_kmsg_dump_register(), and it can do
kzalloc(HV_HYP_PAGE_SIZE), since that case really is tied to the Hyper-V
page size, not PAGE_SIZE. There's no need to waste space by allocating
a full page.

Michael

> 
> Cc: stable@vger.kernel.org
> Fixes: ca48739e59df ("Drivers: hv: vmbus: Move Hyper-V page allocator to arch neutral
> code")
> Signed-off-by: Long Li <longli@microsoft.com>
> ---
>  drivers/hv/hv_common.c | 29 +++++++----------------------
>  1 file changed, 7 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c
> index a7d7494feaca..f426aaa9b8f9 100644
> --- a/drivers/hv/hv_common.c
> +++ b/drivers/hv/hv_common.c
> @@ -106,41 +106,26 @@ void __init hv_common_free(void)
>  }
> 
>  /*
> - * Functions for allocating and freeing memory with size and
> - * alignment HV_HYP_PAGE_SIZE. These functions are needed because
> - * the guest page size may not be the same as the Hyper-V page
> - * size. We depend upon kmalloc() aligning power-of-two size
> - * allocations to the allocation size boundary, so that the
> - * allocated memory appears to Hyper-V as a page of the size
> - * it expects.
> + * A Hyper-V page can be used by UIO for mapping to user-space, it should
> + * always be allocated on system page boundaries.
>   */
> -
>  void *hv_alloc_hyperv_page(void)
>  {
> -	BUILD_BUG_ON(PAGE_SIZE <  HV_HYP_PAGE_SIZE);
> -
> -	if (PAGE_SIZE == HV_HYP_PAGE_SIZE)
> -		return (void *)__get_free_page(GFP_KERNEL);
> -	else
> -		return kmalloc(HV_HYP_PAGE_SIZE, GFP_KERNEL);
> +	BUILD_BUG_ON(PAGE_SIZE < HV_HYP_PAGE_SIZE);
> +	return (void *)__get_free_page(GFP_KERNEL);
>  }
>  EXPORT_SYMBOL_GPL(hv_alloc_hyperv_page);
> 
>  void *hv_alloc_hyperv_zeroed_page(void)
>  {
> -	if (PAGE_SIZE == HV_HYP_PAGE_SIZE)
> -		return (void *)__get_free_page(GFP_KERNEL | __GFP_ZERO);
> -	else
> -		return kzalloc(HV_HYP_PAGE_SIZE, GFP_KERNEL);
> +	BUILD_BUG_ON(PAGE_SIZE < HV_HYP_PAGE_SIZE);
> +	return (void *)__get_free_page(GFP_KERNEL | __GFP_ZERO);
>  }
>  EXPORT_SYMBOL_GPL(hv_alloc_hyperv_zeroed_page);
> 
>  void hv_free_hyperv_page(void *addr)
>  {
> -	if (PAGE_SIZE == HV_HYP_PAGE_SIZE)
> -		free_page((unsigned long)addr);
> -	else
> -		kfree(addr);
> +	free_page((unsigned long)addr);
>  }
>  EXPORT_SYMBOL_GPL(hv_free_hyperv_page);
> 
> --
> 2.34.1
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH 2/2] uio_hv_generic: Use correct size for interrupt and monitor pages
  2025-04-18  0:43 ` [PATCH 2/2] uio_hv_generic: Use correct size for interrupt and monitor pages longli
@ 2025-04-20 23:58   ` Michael Kelley
  2025-04-23 18:49     ` Long Li
  0 siblings, 1 reply; 11+ messages in thread
From: Michael Kelley @ 2025-04-20 23:58 UTC (permalink / raw)
  To: longli@linuxonhyperv.com, K. Y. Srinivasan, Haiyang Zhang,
	Wei Liu, Dexuan Cui, Greg Kroah-Hartman,
	linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org
  Cc: Long Li, stable@vger.kernel.org

From: longli@linuxonhyperv.com <longli@linuxonhyperv.com>
> 
> Interrupt and monitor pages should be in Hyper-V page size (4k bytes).
> This can be different to the system page size.

I'm curious about this change. Since Patch 1 of the series changes
the allocations to be the full PAGE_SIZE, what does it mean to set
the mapping size to less than a full page (in the case PAGE_SIZE >
HV_HYP_PAGE_SIZE)? mmap can only map full PAGE_SIZE pages,
so uio_mmap() rounds up the INT_PAGE_MAP and MON_PAGE_MAP
sizes to PAGE_SIZE when checking the validity of the mmap
parameters.

The changes in this patch do ensure that the INT_PAGE_MAP and
MON_PAGE_MAP "maps" entries in sysfs always show the size as
4096 even if the full PAGE_SIZE is actually mapped, but I'm not sure
if that difference is good or bad.

Michael

> 
> Cc: stable@vger.kernel.org
> Fixes: 95096f2fbd10 ("uio-hv-generic: new userspace i/o driver for VMBus")
> Signed-off-by: Long Li <longli@microsoft.com>
> ---
>  drivers/uio/uio_hv_generic.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/uio/uio_hv_generic.c b/drivers/uio/uio_hv_generic.c
> index 1b19b5647495..08385b04c4ab 100644
> --- a/drivers/uio/uio_hv_generic.c
> +++ b/drivers/uio/uio_hv_generic.c
> @@ -287,13 +287,13 @@ hv_uio_probe(struct hv_device *dev,
>  	pdata->info.mem[INT_PAGE_MAP].name = "int_page";
>  	pdata->info.mem[INT_PAGE_MAP].addr
>  		= (uintptr_t)vmbus_connection.int_page;
> -	pdata->info.mem[INT_PAGE_MAP].size = PAGE_SIZE;
> +	pdata->info.mem[INT_PAGE_MAP].size = HV_HYP_PAGE_SIZE;
>  	pdata->info.mem[INT_PAGE_MAP].memtype = UIO_MEM_LOGICAL;
> 
>  	pdata->info.mem[MON_PAGE_MAP].name = "monitor_page";
>  	pdata->info.mem[MON_PAGE_MAP].addr
>  		= (uintptr_t)vmbus_connection.monitor_pages[1];
> -	pdata->info.mem[MON_PAGE_MAP].size = PAGE_SIZE;
> +	pdata->info.mem[MON_PAGE_MAP].size = HV_HYP_PAGE_SIZE;
>  	pdata->info.mem[MON_PAGE_MAP].memtype = UIO_MEM_LOGICAL;
> 
>  	if (channel->device_id == HV_NIC) {
> --
> 2.34.1
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH 0/2] Fix uio_hv_generic on 64k page systems
  2025-04-20 23:57 ` [PATCH 0/2] Fix uio_hv_generic on 64k page systems Michael Kelley
@ 2025-04-23 18:30   ` Long Li
  0 siblings, 0 replies; 11+ messages in thread
From: Long Li @ 2025-04-23 18:30 UTC (permalink / raw)
  To: Michael Kelley, longli@linuxonhyperv.com, KY Srinivasan,
	Haiyang Zhang, Wei Liu, Dexuan Cui, Greg Kroah-Hartman,
	linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org



> -----Original Message-----
> From: Michael Kelley <mhklinux@outlook.com>
> Sent: Sunday, April 20, 2025 4:57 PM
> To: longli@linuxonhyperv.com; KY Srinivasan <kys@microsoft.com>; Haiyang
> Zhang <haiyangz@microsoft.com>; Wei Liu <wei.liu@kernel.org>; Dexuan Cui
> <decui@microsoft.com>; Greg Kroah-Hartman <gregkh@linuxfoundation.org>;
> linux-hyperv@vger.kernel.org; linux-kernel@vger.kernel.org
> Cc: Long Li <longli@microsoft.com>
> Subject: [EXTERNAL] RE: [PATCH 0/2] Fix uio_hv_generic on 64k page systems
> 
> From: longli@linuxonhyperv.com <longli@linuxonhyperv.com>
> >
> 
> The Subject line of this cover letter is a bit too narrow in scope. The scope should
> be any page size larger than 4K. For example, the arm64 architecture permits a
> page size of 16K, and Linux kernels built that way work just fine on Hyper-V arm64
> hosts. Perhaps:
> 
>     Fix uio_hv_generic for guests with page size > 4 KiB
> 
> > UIO framework requires the device memory aligned to page boundary.
> > Hyper-V may allocate some memory that is Hyper-V page aligned (4k) but
> > not system page aligned.
> >
> > Fix this by having Hyper-V always allocate those pages on system page
> > boundary and expose them to user-mode.
> 
> Also within the scope of making uio_hv_generic work with page size > 4KiB,
> there's an issue with the ring size. When hv_dev_ring_size() returns 0,
> hv_uio_probe() uses 2 MiB as the ring size. That works OK with the larger page
> sizes. But when hv_dev_ring_size() returns a specific value, it might not work. The
> fcopy device returns 16 KiB, which will fail. hv_uio_probe() needs to use the
> VMBUS_RING_SIZE() macro to increase the ring size if necessary to handle the
> larger ring header that results if the page size is > 4 KiB.  You might want to
> include such a patch in this series.

Sure, will use VMBUS_RING_SIZE() to increase ring size if necessary.

Long

> 
> Separately, tools/hv/vmbus_bufring.c needs work to operate correctly on
> arm64 and with page sizes > 4 KiB. But that's probably a different patch series.
> 
> Michael
> 
> >
> > Long Li (2):
> >   Drivers: hv: Allocate interrupt and monitor pages aligned to system
> >     page boundary
> >   uio_hv_generic: Use correct size for interrupt and monitor pages
> >
> >  drivers/hv/hv_common.c       | 29 +++++++----------------------
> >  drivers/uio/uio_hv_generic.c |  4 ++--
> >  2 files changed, 9 insertions(+), 24 deletions(-)
> >
> > --
> > 2.34.1
> >

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH 1/2] Drivers: hv: Allocate interrupt and monitor pages aligned to system page boundary
  2025-04-20 23:57   ` Michael Kelley
@ 2025-04-23 18:39     ` Long Li
  2025-04-23 20:14       ` Michael Kelley
  0 siblings, 1 reply; 11+ messages in thread
From: Long Li @ 2025-04-23 18:39 UTC (permalink / raw)
  To: Michael Kelley, longli@linuxonhyperv.com, KY Srinivasan,
	Haiyang Zhang, Wei Liu, Dexuan Cui, Greg Kroah-Hartman,
	linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org
  Cc: stable@vger.kernel.org

> Subject: [EXTERNAL] RE: [PATCH 1/2] Drivers: hv: Allocate interrupt and monitor
> pages aligned to system page boundary
> 
> From: longli@linuxonhyperv.com <longli@linuxonhyperv.com> Sent: Thursday,
> April 17, 2025 5:43 PM
> >
> > There are use cases that interrupt and monitor pages are mapped to
> > user-mode through UIO, they need to be system page aligned. Some
> > Hyper-V allocation APIs introduced earlier broke those requirements.
> >
> > Fix those APIs by always allocating Hyper-V page at system page boundaries.
> 
> I'd suggest doing away with the hv_alloc/free_*() functions entirely since they are
> now reduced to just being a wrapper around __get_free_pages(), which doesn't
> add any value. Once all the arm64 support and CoCo VM code settled out, it
> turned out that these functions to allocate Hyper-V size pages had dwindling
> usage.

There is a BUILD_BUG_ON(PAGE_SIZE < HV_HYP_PAGE_SIZE) in those functions, but it probably doesn't do anything.

If there is no objection, I can remove these functions.

Long

> 
> Allocation of the interrupt and monitor pages can use __get_free_pages() directly,
> and that properly captures the need for those allocations to be a full page. Just
> add a comment that this wastes space when PAGE_SIZE
> > HV_HYP_PAGE_SIZE, but is necessary because the page may be mapped
> into user space by uio_hv_generic.
> 
> The only other use is in hv_kmsg_dump_register(), and it can do
> kzalloc(HV_HYP_PAGE_SIZE), since that case really is tied to the Hyper-V page
> size, not PAGE_SIZE. There's no need to waste space by allocating a full page.
> 
> Michael
> 
> >
> > Cc: stable@vger.kernel.org
> > Fixes: ca48739e59df ("Drivers: hv: vmbus: Move Hyper-V page allocator
> > to arch neutral
> > code")
> > Signed-off-by: Long Li <longli@microsoft.com>
> > ---
> >  drivers/hv/hv_common.c | 29 +++++++----------------------
> >  1 file changed, 7 insertions(+), 22 deletions(-)
> >
> > diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c index
> > a7d7494feaca..f426aaa9b8f9 100644
> > --- a/drivers/hv/hv_common.c
> > +++ b/drivers/hv/hv_common.c
> > @@ -106,41 +106,26 @@ void __init hv_common_free(void)  }
> >
> >  /*
> > - * Functions for allocating and freeing memory with size and
> > - * alignment HV_HYP_PAGE_SIZE. These functions are needed because
> > - * the guest page size may not be the same as the Hyper-V page
> > - * size. We depend upon kmalloc() aligning power-of-two size
> > - * allocations to the allocation size boundary, so that the
> > - * allocated memory appears to Hyper-V as a page of the size
> > - * it expects.
> > + * A Hyper-V page can be used by UIO for mapping to user-space, it
> > + should
> > + * always be allocated on system page boundaries.
> >   */
> > -
> >  void *hv_alloc_hyperv_page(void)
> >  {
> > -	BUILD_BUG_ON(PAGE_SIZE <  HV_HYP_PAGE_SIZE);
> > -
> > -	if (PAGE_SIZE == HV_HYP_PAGE_SIZE)
> > -		return (void *)__get_free_page(GFP_KERNEL);
> > -	else
> > -		return kmalloc(HV_HYP_PAGE_SIZE, GFP_KERNEL);
> > +	BUILD_BUG_ON(PAGE_SIZE < HV_HYP_PAGE_SIZE);
> > +	return (void *)__get_free_page(GFP_KERNEL);
> >  }
> >  EXPORT_SYMBOL_GPL(hv_alloc_hyperv_page);
> >
> >  void *hv_alloc_hyperv_zeroed_page(void)
> >  {
> > -	if (PAGE_SIZE == HV_HYP_PAGE_SIZE)
> > -		return (void *)__get_free_page(GFP_KERNEL | __GFP_ZERO);
> > -	else
> > -		return kzalloc(HV_HYP_PAGE_SIZE, GFP_KERNEL);
> > +	BUILD_BUG_ON(PAGE_SIZE < HV_HYP_PAGE_SIZE);
> > +	return (void *)__get_free_page(GFP_KERNEL | __GFP_ZERO);
> >  }
> >  EXPORT_SYMBOL_GPL(hv_alloc_hyperv_zeroed_page);
> >
> >  void hv_free_hyperv_page(void *addr)
> >  {
> > -	if (PAGE_SIZE == HV_HYP_PAGE_SIZE)
> > -		free_page((unsigned long)addr);
> > -	else
> > -		kfree(addr);
> > +	free_page((unsigned long)addr);
> >  }
> >  EXPORT_SYMBOL_GPL(hv_free_hyperv_page);
> >
> > --
> > 2.34.1
> >


^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH 2/2] uio_hv_generic: Use correct size for interrupt and monitor pages
  2025-04-20 23:58   ` Michael Kelley
@ 2025-04-23 18:49     ` Long Li
  2025-04-23 20:07       ` Michael Kelley
  0 siblings, 1 reply; 11+ messages in thread
From: Long Li @ 2025-04-23 18:49 UTC (permalink / raw)
  To: Michael Kelley, longli@linuxonhyperv.com, KY Srinivasan,
	Haiyang Zhang, Wei Liu, Dexuan Cui, Greg Kroah-Hartman,
	linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org
  Cc: stable@vger.kernel.org

> Subject: [EXTERNAL] RE: [PATCH 2/2] uio_hv_generic: Use correct size for
> interrupt and monitor pages
> 
> From: longli@linuxonhyperv.com <longli@linuxonhyperv.com>
> >
> > Interrupt and monitor pages should be in Hyper-V page size (4k bytes).
> > This can be different to the system page size.
> 
> I'm curious about this change. Since Patch 1 of the series changes the allocations
> to be the full PAGE_SIZE, what does it mean to set the mapping size to less than a
> full page (in the case PAGE_SIZE > HV_HYP_PAGE_SIZE)? mmap can only map full
> PAGE_SIZE pages, so uio_mmap() rounds up the INT_PAGE_MAP and
> MON_PAGE_MAP sizes to PAGE_SIZE when checking the validity of the mmap
> parameters.
> 
> The changes in this patch do ensure that the INT_PAGE_MAP and
> MON_PAGE_MAP "maps" entries in sysfs always show the size as
> 4096 even if the full PAGE_SIZE is actually mapped, but I'm not sure if that
> difference is good or bad.

Kernel needs to tell the user-mode the correct length of the map. In this case, 4096 bytes are usable data regardless of what is actual mapped as long as it's >4096.

The DPDK vmbus driver uses this length to setup checks for accessing the page.

Long

> 
> Michael
> 
> >
> > Cc: stable@vger.kernel.org
> > Fixes: 95096f2fbd10 ("uio-hv-generic: new userspace i/o driver for
> > VMBus")
> > Signed-off-by: Long Li <longli@microsoft.com>
> > ---
> >  drivers/uio/uio_hv_generic.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/uio/uio_hv_generic.c
> > b/drivers/uio/uio_hv_generic.c index 1b19b5647495..08385b04c4ab 100644
> > --- a/drivers/uio/uio_hv_generic.c
> > +++ b/drivers/uio/uio_hv_generic.c
> > @@ -287,13 +287,13 @@ hv_uio_probe(struct hv_device *dev,
> >  	pdata->info.mem[INT_PAGE_MAP].name = "int_page";
> >  	pdata->info.mem[INT_PAGE_MAP].addr
> >  		= (uintptr_t)vmbus_connection.int_page;
> > -	pdata->info.mem[INT_PAGE_MAP].size = PAGE_SIZE;
> > +	pdata->info.mem[INT_PAGE_MAP].size = HV_HYP_PAGE_SIZE;
> >  	pdata->info.mem[INT_PAGE_MAP].memtype = UIO_MEM_LOGICAL;
> >
> >  	pdata->info.mem[MON_PAGE_MAP].name = "monitor_page";
> >  	pdata->info.mem[MON_PAGE_MAP].addr
> >  		= (uintptr_t)vmbus_connection.monitor_pages[1];
> > -	pdata->info.mem[MON_PAGE_MAP].size = PAGE_SIZE;
> > +	pdata->info.mem[MON_PAGE_MAP].size = HV_HYP_PAGE_SIZE;
> >  	pdata->info.mem[MON_PAGE_MAP].memtype = UIO_MEM_LOGICAL;
> >
> >  	if (channel->device_id == HV_NIC) {
> > --
> > 2.34.1
> >


^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH 2/2] uio_hv_generic: Use correct size for interrupt and monitor pages
  2025-04-23 18:49     ` Long Li
@ 2025-04-23 20:07       ` Michael Kelley
  0 siblings, 0 replies; 11+ messages in thread
From: Michael Kelley @ 2025-04-23 20:07 UTC (permalink / raw)
  To: Long Li, longli@linuxonhyperv.com, KY Srinivasan, Haiyang Zhang,
	Wei Liu, Dexuan Cui, Greg Kroah-Hartman,
	linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org
  Cc: stable@vger.kernel.org

From: Long Li <longli@microsoft.com> Sent: Wednesday, April 23, 2025 11:49 AM
> > >
> > > Interrupt and monitor pages should be in Hyper-V page size (4k bytes).
> > > This can be different to the system page size.
> >
> > I'm curious about this change. Since Patch 1 of the series changes the allocations
> > to be the full PAGE_SIZE, what does it mean to set the mapping size to less than a
> > full page (in the case PAGE_SIZE > HV_HYP_PAGE_SIZE)? mmap can only map full
> > PAGE_SIZE pages, so uio_mmap() rounds up the INT_PAGE_MAP and
> > MON_PAGE_MAP sizes to PAGE_SIZE when checking the validity of the mmap
> > parameters.
> >
> > The changes in this patch do ensure that the INT_PAGE_MAP and
> > MON_PAGE_MAP "maps" entries in sysfs always show the size as
> > 4096 even if the full PAGE_SIZE is actually mapped, but I'm not sure if that
> > difference is good or bad.
> 
> Kernel needs to tell the user-mode the correct length of the map. In this case, 4096
> bytes are usable data regardless of what is actual mapped as long as it's >4096.
> 
> The DPDK vmbus driver uses this length to setup checks for accessing the page.

OK, so the "maps" entry needs to show a size of 4096 so the DPDK VMBus
driver does the right thing.

At a minimum, mention in the commit message that the reason for the change
is to always show 4096 as the size of the "maps" entries, even if PAGE_SIZE
and the actual mapped area is larger. Perhaps also mention that the DPDK
VMBus driver is one known consumer of that detail.

Michael

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH 1/2] Drivers: hv: Allocate interrupt and monitor pages aligned to system page boundary
  2025-04-23 18:39     ` Long Li
@ 2025-04-23 20:14       ` Michael Kelley
  0 siblings, 0 replies; 11+ messages in thread
From: Michael Kelley @ 2025-04-23 20:14 UTC (permalink / raw)
  To: Long Li, longli@linuxonhyperv.com, KY Srinivasan, Haiyang Zhang,
	Wei Liu, Dexuan Cui, Greg Kroah-Hartman,
	linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org
  Cc: stable@vger.kernel.org

From: Long Li <longli@microsoft.com> Sent: Wednesday, April 23, 2025 11:40 AM
> > >
> > > There are use cases that interrupt and monitor pages are mapped to
> > > user-mode through UIO, they need to be system page aligned. Some
> > > Hyper-V allocation APIs introduced earlier broke those requirements.
> > >
> > > Fix those APIs by always allocating Hyper-V page at system page boundaries.
> >
> > I'd suggest doing away with the hv_alloc/free_*() functions entirely since they are
> > now reduced to just being a wrapper around __get_free_pages(), which doesn't
> > add any value. Once all the arm64 support and CoCo VM code settled out, it
> > turned out that these functions to allocate Hyper-V size pages had dwindling
> > usage.
> 
> There is a BUILD_BUG_ON(PAGE_SIZE < HV_HYP_PAGE_SIZE) in those functions, but it
> probably doesn't do anything.

You could move the BUILD_BUG_ON() to vmbus_connection() where
one of the calls to __get_free_pages() is made. That would codify the
assumption that __get_free_pages() returns memory at least as large as
HV_HYP_PAGE_SIZE.

Michael

> 
> If there is no objection, I can remove these functions.
> 
> Long
> 
> >
> > Allocation of the interrupt and monitor pages can use __get_free_pages() directly,
> > and that properly captures the need for those allocations to be a full page. Just
> > add a comment that this wastes space when PAGE_SIZE
> > > HV_HYP_PAGE_SIZE, but is necessary because the page may be mapped
> > into user space by uio_hv_generic.
> >
> > The only other use is in hv_kmsg_dump_register(), and it can do
> > kzalloc(HV_HYP_PAGE_SIZE), since that case really is tied to the Hyper-V page
> > size, not PAGE_SIZE. There's no need to waste space by allocating a full page.
> >
> > Michael
> >
> > >
> > > Cc: stable@vger.kernel.org
> > > Fixes: ca48739e59df ("Drivers: hv: vmbus: Move Hyper-V page allocator
> > > to arch neutral
> > > code")
> > > Signed-off-by: Long Li <longli@microsoft.com>
> > > ---
> > >  drivers/hv/hv_common.c | 29 +++++++----------------------
> > >  1 file changed, 7 insertions(+), 22 deletions(-)
> > >
> > > diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c index
> > > a7d7494feaca..f426aaa9b8f9 100644
> > > --- a/drivers/hv/hv_common.c
> > > +++ b/drivers/hv/hv_common.c
> > > @@ -106,41 +106,26 @@ void __init hv_common_free(void)  }
> > >
> > >  /*
> > > - * Functions for allocating and freeing memory with size and
> > > - * alignment HV_HYP_PAGE_SIZE. These functions are needed because
> > > - * the guest page size may not be the same as the Hyper-V page
> > > - * size. We depend upon kmalloc() aligning power-of-two size
> > > - * allocations to the allocation size boundary, so that the
> > > - * allocated memory appears to Hyper-V as a page of the size
> > > - * it expects.
> > > + * A Hyper-V page can be used by UIO for mapping to user-space, it
> > > + should
> > > + * always be allocated on system page boundaries.
> > >   */
> > > -
> > >  void *hv_alloc_hyperv_page(void)
> > >  {
> > > -	BUILD_BUG_ON(PAGE_SIZE <  HV_HYP_PAGE_SIZE);
> > > -
> > > -	if (PAGE_SIZE == HV_HYP_PAGE_SIZE)
> > > -		return (void *)__get_free_page(GFP_KERNEL);
> > > -	else
> > > -		return kmalloc(HV_HYP_PAGE_SIZE, GFP_KERNEL);
> > > +	BUILD_BUG_ON(PAGE_SIZE < HV_HYP_PAGE_SIZE);
> > > +	return (void *)__get_free_page(GFP_KERNEL);
> > >  }
> > >  EXPORT_SYMBOL_GPL(hv_alloc_hyperv_page);
> > >
> > >  void *hv_alloc_hyperv_zeroed_page(void)
> > >  {
> > > -	if (PAGE_SIZE == HV_HYP_PAGE_SIZE)
> > > -		return (void *)__get_free_page(GFP_KERNEL | __GFP_ZERO);
> > > -	else
> > > -		return kzalloc(HV_HYP_PAGE_SIZE, GFP_KERNEL);
> > > +	BUILD_BUG_ON(PAGE_SIZE < HV_HYP_PAGE_SIZE);
> > > +	return (void *)__get_free_page(GFP_KERNEL | __GFP_ZERO);
> > >  }
> > >  EXPORT_SYMBOL_GPL(hv_alloc_hyperv_zeroed_page);
> > >
> > >  void hv_free_hyperv_page(void *addr)
> > >  {
> > > -	if (PAGE_SIZE == HV_HYP_PAGE_SIZE)
> > > -		free_page((unsigned long)addr);
> > > -	else
> > > -		kfree(addr);
> > > +	free_page((unsigned long)addr);
> > >  }
> > >  EXPORT_SYMBOL_GPL(hv_free_hyperv_page);
> > >
> > > --
> > > 2.34.1
> > >
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2025-04-23 20:14 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-18  0:43 [PATCH 0/2] Fix uio_hv_generic on 64k page systems longli
2025-04-18  0:43 ` [PATCH 1/2] Drivers: hv: Allocate interrupt and monitor pages aligned to system page boundary longli
2025-04-20 23:57   ` Michael Kelley
2025-04-23 18:39     ` Long Li
2025-04-23 20:14       ` Michael Kelley
2025-04-18  0:43 ` [PATCH 2/2] uio_hv_generic: Use correct size for interrupt and monitor pages longli
2025-04-20 23:58   ` Michael Kelley
2025-04-23 18:49     ` Long Li
2025-04-23 20:07       ` Michael Kelley
2025-04-20 23:57 ` [PATCH 0/2] Fix uio_hv_generic on 64k page systems Michael Kelley
2025-04-23 18:30   ` Long Li

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).