* [PATCH 1/2] Drivers: hv: Allocate interrupt and monitor pages aligned to system page boundary [not found] <1744936997-7844-1-git-send-email-longli@linuxonhyperv.com> @ 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 1 sibling, 1 reply; 8+ 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] 8+ 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; 8+ 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] 8+ 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; 8+ 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] 8+ 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; 8+ 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] 8+ messages in thread
* [PATCH 2/2] uio_hv_generic: Use correct size for interrupt and monitor pages [not found] <1744936997-7844-1-git-send-email-longli@linuxonhyperv.com> 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 1 sibling, 1 reply; 8+ 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] 8+ 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; 8+ 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] 8+ 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; 8+ 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] 8+ 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; 8+ 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] 8+ messages in thread
end of thread, other threads:[~2025-04-23 20:14 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1744936997-7844-1-git-send-email-longli@linuxonhyperv.com>
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
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox