* [PATCH 0/2] Fix udmabuf vmap error @ 2025-04-15 3:15 Huan Yang 2025-04-15 3:15 ` [PATCH 1/2] Revert "udmabuf: fix vmap_udmabuf error page set" Huan Yang 2025-04-15 3:15 ` [PATCH 2/2] udmabuf: fix vmap missed offset page Huan Yang 0 siblings, 2 replies; 11+ messages in thread From: Huan Yang @ 2025-04-15 3:15 UTC (permalink / raw) To: Sumit Semwal, Christian König, Gerd Hoffmann, Vivek Kasireddy, Andrew Morton, Dave Airlie, linux-media, dri-devel, linaro-mm-sig, linux-kernel Cc: opensource.kernel, Huan Yang In [1] Bingbu report an error that vmap_udmabuf invoke failed due to use vmap_pfn and blocked by !pfn_valid. Due to misunderstand it, I change vmap_pfn check of !pfn_valid to pfn_valid, then send [2], then Christoph point that: vmap_pfn is entirely for memory not backed by pages or folios, i.e. PCIe BARs and similar memory. This must not be mixed with proper folio backed memory. So, I given a misuse of vmap_pfn. But to fix vmap_udmabuf, and consider HVO effect, I offer a RFC patchset [3], which simple copy vmap_pfn to vmap_udmabuf, other implement folio range based vmap. But in [3], Muchun point that I misunderstand HVO, which do not released any page struct pointer in vmemmap, only change this VA's PTE point to hugetlb's **HEAD** page frame, and release remainned **tail** page frame. So any page struct pointer do exist, and folio_page, folio_pfn or other api still can work. By this, we can fix this error simpliy: Patch 1 revert this vmap_pfn misuse patch. Patch 2 fix missed offset page set. [1] https://lore.kernel.org/all/9172a601-c360-0d5b-ba1b-33deba430455@linux.intel.com/ [2] https://lore.kernel.org/all/20250312061513.1126496-1-link@vivo.com/ [3] https://lore.kernel.org/all/20250327092922.536-1-link@vivo.com/ Huan Yang (2): Revert "udmabuf: fix vmap_udmabuf error page set" udmabuf: fix vmap missed offset page drivers/dma-buf/Kconfig | 1 - drivers/dma-buf/udmabuf.c | 23 ++++++++--------------- 2 files changed, 8 insertions(+), 16 deletions(-) base-commit: b425262c07a6a643ebeed91046e161e20b944164 -- 2.48.1 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/2] Revert "udmabuf: fix vmap_udmabuf error page set" 2025-04-15 3:15 [PATCH 0/2] Fix udmabuf vmap error Huan Yang @ 2025-04-15 3:15 ` Huan Yang 2025-04-22 5:14 ` Kasireddy, Vivek 2025-04-15 3:15 ` [PATCH 2/2] udmabuf: fix vmap missed offset page Huan Yang 1 sibling, 1 reply; 11+ messages in thread From: Huan Yang @ 2025-04-15 3:15 UTC (permalink / raw) To: Sumit Semwal, Christian König, Gerd Hoffmann, Vivek Kasireddy, Andrew Morton, Dave Airlie, linux-media, dri-devel, linaro-mm-sig, linux-kernel Cc: opensource.kernel, Huan Yang, Bingbu Cao This reverts commit 18d7de823b7150344d242c3677e65d68c5271b04. This given a misuse of vmap_pfn, vmap_pfn only allow none-page based user invoke, i.e. PCIe BARs and other. Signed-off-by: Huan Yang <link@vivo.com> Reported-by: Bingbu Cao <bingbu.cao@linux.intel.com> Closes: https://lore.kernel.org/dri-devel/eb7e0137-3508-4287-98c4-816c5fd98e10@vivo.com/T/#mbda4f64a3532b32e061f4e8763bc8e307bea3ca8 --- drivers/dma-buf/Kconfig | 1 - drivers/dma-buf/udmabuf.c | 22 +++++++--------------- 2 files changed, 7 insertions(+), 16 deletions(-) diff --git a/drivers/dma-buf/Kconfig b/drivers/dma-buf/Kconfig index fee04fdb0822..b46eb8a552d7 100644 --- a/drivers/dma-buf/Kconfig +++ b/drivers/dma-buf/Kconfig @@ -36,7 +36,6 @@ config UDMABUF depends on DMA_SHARED_BUFFER depends on MEMFD_CREATE || COMPILE_TEST depends on MMU - select VMAP_PFN help A driver to let userspace turn memfd regions into dma-bufs. Qemu can use this to create host dmabufs for guest framebuffers. diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c index 7eee3eb47a8e..79845565089d 100644 --- a/drivers/dma-buf/udmabuf.c +++ b/drivers/dma-buf/udmabuf.c @@ -109,29 +109,21 @@ static int mmap_udmabuf(struct dma_buf *buf, struct vm_area_struct *vma) static int vmap_udmabuf(struct dma_buf *buf, struct iosys_map *map) { struct udmabuf *ubuf = buf->priv; - unsigned long *pfns; + struct page **pages; void *vaddr; pgoff_t pg; dma_resv_assert_held(buf->resv); - /** - * HVO may free tail pages, so just use pfn to map each folio - * into vmalloc area. - */ - pfns = kvmalloc_array(ubuf->pagecount, sizeof(*pfns), GFP_KERNEL); - if (!pfns) + pages = kvmalloc_array(ubuf->pagecount, sizeof(*pages), GFP_KERNEL); + if (!pages) return -ENOMEM; - for (pg = 0; pg < ubuf->pagecount; pg++) { - unsigned long pfn = folio_pfn(ubuf->folios[pg]); - - pfn += ubuf->offsets[pg] >> PAGE_SHIFT; - pfns[pg] = pfn; - } + for (pg = 0; pg < ubuf->pagecount; pg++) + pages[pg] = &ubuf->folios[pg]->page; - vaddr = vmap_pfn(pfns, ubuf->pagecount, PAGE_KERNEL); - kvfree(pfns); + vaddr = vm_map_ram(pages, ubuf->pagecount, -1); + kvfree(pages); if (!vaddr) return -EINVAL; -- 2.48.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* RE: [PATCH 1/2] Revert "udmabuf: fix vmap_udmabuf error page set" 2025-04-15 3:15 ` [PATCH 1/2] Revert "udmabuf: fix vmap_udmabuf error page set" Huan Yang @ 2025-04-22 5:14 ` Kasireddy, Vivek 2025-04-23 7:22 ` Huan Yang 0 siblings, 1 reply; 11+ messages in thread From: Kasireddy, Vivek @ 2025-04-22 5:14 UTC (permalink / raw) To: Huan Yang, Sumit Semwal, Christian König, Gerd Hoffmann, Andrew Morton, Dave Airlie, linux-media@vger.kernel.org, dri-devel@lists.freedesktop.org, linaro-mm-sig@lists.linaro.org, linux-kernel@vger.kernel.org Cc: opensource.kernel@vivo.com, Bingbu Cao Hi Huan, > Subject: [PATCH 1/2] Revert "udmabuf: fix vmap_udmabuf error page set" > > This reverts commit 18d7de823b7150344d242c3677e65d68c5271b04. > > This given a misuse of vmap_pfn, vmap_pfn only allow none-page based > user invoke, i.e. PCIe BARs and other. The commit message can be improved a little bit to briefly explain why vmap_pfn() would not work for this use-case. Acked-by: Vivek Kasireddy <vivek.kasireddy@intel.com> Thanks, Vivek > > Signed-off-by: Huan Yang <link@vivo.com> > Reported-by: Bingbu Cao <bingbu.cao@linux.intel.com> > Closes: https://lore.kernel.org/dri-devel/eb7e0137-3508-4287-98c4- > 816c5fd98e10@vivo.com/T/#mbda4f64a3532b32e061f4e8763bc8e307bea3ca > 8 > --- > drivers/dma-buf/Kconfig | 1 - > drivers/dma-buf/udmabuf.c | 22 +++++++--------------- > 2 files changed, 7 insertions(+), 16 deletions(-) > > diff --git a/drivers/dma-buf/Kconfig b/drivers/dma-buf/Kconfig > index fee04fdb0822..b46eb8a552d7 100644 > --- a/drivers/dma-buf/Kconfig > +++ b/drivers/dma-buf/Kconfig > @@ -36,7 +36,6 @@ config UDMABUF > depends on DMA_SHARED_BUFFER > depends on MEMFD_CREATE || COMPILE_TEST > depends on MMU > - select VMAP_PFN > help > A driver to let userspace turn memfd regions into dma-bufs. > Qemu can use this to create host dmabufs for guest framebuffers. > diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c > index 7eee3eb47a8e..79845565089d 100644 > --- a/drivers/dma-buf/udmabuf.c > +++ b/drivers/dma-buf/udmabuf.c > @@ -109,29 +109,21 @@ static int mmap_udmabuf(struct dma_buf *buf, > struct vm_area_struct *vma) > static int vmap_udmabuf(struct dma_buf *buf, struct iosys_map *map) > { > struct udmabuf *ubuf = buf->priv; > - unsigned long *pfns; > + struct page **pages; > void *vaddr; > pgoff_t pg; > > dma_resv_assert_held(buf->resv); > > - /** > - * HVO may free tail pages, so just use pfn to map each folio > - * into vmalloc area. > - */ > - pfns = kvmalloc_array(ubuf->pagecount, sizeof(*pfns), GFP_KERNEL); > - if (!pfns) > + pages = kvmalloc_array(ubuf->pagecount, sizeof(*pages), > GFP_KERNEL); > + if (!pages) > return -ENOMEM; > > - for (pg = 0; pg < ubuf->pagecount; pg++) { > - unsigned long pfn = folio_pfn(ubuf->folios[pg]); > - > - pfn += ubuf->offsets[pg] >> PAGE_SHIFT; > - pfns[pg] = pfn; > - } > + for (pg = 0; pg < ubuf->pagecount; pg++) > + pages[pg] = &ubuf->folios[pg]->page; > > - vaddr = vmap_pfn(pfns, ubuf->pagecount, PAGE_KERNEL); > - kvfree(pfns); > + vaddr = vm_map_ram(pages, ubuf->pagecount, -1); > + kvfree(pages); > if (!vaddr) > return -EINVAL; > > -- > 2.48.1 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] Revert "udmabuf: fix vmap_udmabuf error page set" 2025-04-22 5:14 ` Kasireddy, Vivek @ 2025-04-23 7:22 ` Huan Yang 2025-04-23 7:26 ` Huan Yang 0 siblings, 1 reply; 11+ messages in thread From: Huan Yang @ 2025-04-23 7:22 UTC (permalink / raw) To: Kasireddy, Vivek, Sumit Semwal, Christian König, Gerd Hoffmann, Andrew Morton, Dave Airlie, linux-media@vger.kernel.org, dri-devel@lists.freedesktop.org, linaro-mm-sig@lists.linaro.org, linux-kernel@vger.kernel.org Cc: opensource.kernel@vivo.com, Bingbu Cao Hi Vivek, Thanks for your ack. 在 2025/4/22 13:14, Kasireddy, Vivek 写道: > Hi Huan, > >> Subject: [PATCH 1/2] Revert "udmabuf: fix vmap_udmabuf error page set" >> >> This reverts commit 18d7de823b7150344d242c3677e65d68c5271b04. >> >> This given a misuse of vmap_pfn, vmap_pfn only allow none-page based >> user invoke, i.e. PCIe BARs and other. > The commit message can be improved a little bit to briefly explain why vmap_pfn() > would not work for this use-case. OK, I will update patch direct in later reply.:) Thanks, Huan > > Acked-by: Vivek Kasireddy <vivek.kasireddy@intel.com> > > Thanks, > Vivek > >> Signed-off-by: Huan Yang <link@vivo.com> >> Reported-by: Bingbu Cao <bingbu.cao@linux.intel.com> >> Closes: https://lore.kernel.org/dri-devel/eb7e0137-3508-4287-98c4- >> 816c5fd98e10@vivo.com/T/#mbda4f64a3532b32e061f4e8763bc8e307bea3ca >> 8 >> --- >> drivers/dma-buf/Kconfig | 1 - >> drivers/dma-buf/udmabuf.c | 22 +++++++--------------- >> 2 files changed, 7 insertions(+), 16 deletions(-) >> >> diff --git a/drivers/dma-buf/Kconfig b/drivers/dma-buf/Kconfig >> index fee04fdb0822..b46eb8a552d7 100644 >> --- a/drivers/dma-buf/Kconfig >> +++ b/drivers/dma-buf/Kconfig >> @@ -36,7 +36,6 @@ config UDMABUF >> depends on DMA_SHARED_BUFFER >> depends on MEMFD_CREATE || COMPILE_TEST >> depends on MMU >> - select VMAP_PFN >> help >> A driver to let userspace turn memfd regions into dma-bufs. >> Qemu can use this to create host dmabufs for guest framebuffers. >> diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c >> index 7eee3eb47a8e..79845565089d 100644 >> --- a/drivers/dma-buf/udmabuf.c >> +++ b/drivers/dma-buf/udmabuf.c >> @@ -109,29 +109,21 @@ static int mmap_udmabuf(struct dma_buf *buf, >> struct vm_area_struct *vma) >> static int vmap_udmabuf(struct dma_buf *buf, struct iosys_map *map) >> { >> struct udmabuf *ubuf = buf->priv; >> - unsigned long *pfns; >> + struct page **pages; >> void *vaddr; >> pgoff_t pg; >> >> dma_resv_assert_held(buf->resv); >> >> - /** >> - * HVO may free tail pages, so just use pfn to map each folio >> - * into vmalloc area. >> - */ >> - pfns = kvmalloc_array(ubuf->pagecount, sizeof(*pfns), GFP_KERNEL); >> - if (!pfns) >> + pages = kvmalloc_array(ubuf->pagecount, sizeof(*pages), >> GFP_KERNEL); >> + if (!pages) >> return -ENOMEM; >> >> - for (pg = 0; pg < ubuf->pagecount; pg++) { >> - unsigned long pfn = folio_pfn(ubuf->folios[pg]); >> - >> - pfn += ubuf->offsets[pg] >> PAGE_SHIFT; >> - pfns[pg] = pfn; >> - } >> + for (pg = 0; pg < ubuf->pagecount; pg++) >> + pages[pg] = &ubuf->folios[pg]->page; >> >> - vaddr = vmap_pfn(pfns, ubuf->pagecount, PAGE_KERNEL); >> - kvfree(pfns); >> + vaddr = vm_map_ram(pages, ubuf->pagecount, -1); >> + kvfree(pages); >> if (!vaddr) >> return -EINVAL; >> >> -- >> 2.48.1 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] Revert "udmabuf: fix vmap_udmabuf error page set" 2025-04-23 7:22 ` Huan Yang @ 2025-04-23 7:26 ` Huan Yang 2025-04-28 4:16 ` Kasireddy, Vivek 0 siblings, 1 reply; 11+ messages in thread From: Huan Yang @ 2025-04-23 7:26 UTC (permalink / raw) To: Kasireddy, Vivek, Sumit Semwal, Christian König, Gerd Hoffmann, Andrew Morton, Dave Airlie, linux-media@vger.kernel.org, dri-devel@lists.freedesktop.org, linaro-mm-sig@lists.linaro.org, linux-kernel@vger.kernel.org Cc: opensource.kernel@vivo.com, Bingbu Cao From 38aa11d92f209e7529736f3e11e08dfc804bdfae Mon Sep 17 00:00:00 2001 From: Huan Yang <link@vivo.com> Date: Tue, 15 Apr 2025 10:04:18 +0800 Subject: [PATCH 1/2] Revert "udmabuf: fix vmap_udmabuf error page set" This reverts commit 18d7de823b7150344d242c3677e65d68c5271b04. This given a misuse of vmap_pfn, vmap_pfn give a !pfn_valid check to avoid user miss use it. This API design to only for none-page struct based user invoke, i.e. PCIe BARs and other. So any page based will inject by !pfn_valid check. udmabuf used shmem or hugetlb as folio src, hence, page/folio based, can't use it. Signed-off-by: Huan Yang <link@vivo.com> Reported-by: Bingbu Cao <bingbu.cao@linux.intel.com> Closes: https://lore.kernel.org/dri-devel/eb7e0137-3508-4287-98c4-816c5fd98e10@vivo.com/T/#mbda4f64a3532b32e061f4e8763bc8e307bea3ca8 Acked-by: Vivek Kasireddy <vivek.kasireddy@intel.com> --- drivers/dma-buf/Kconfig | 1 - drivers/dma-buf/udmabuf.c | 22 +++++++--------------- 2 files changed, 7 insertions(+), 16 deletions(-) diff --git a/drivers/dma-buf/Kconfig b/drivers/dma-buf/Kconfig index fee04fdb0822..b46eb8a552d7 100644 --- a/drivers/dma-buf/Kconfig +++ b/drivers/dma-buf/Kconfig @@ -36,7 +36,6 @@ config UDMABUF depends on DMA_SHARED_BUFFER depends on MEMFD_CREATE || COMPILE_TEST depends on MMU - select VMAP_PFN help A driver to let userspace turn memfd regions into dma-bufs. Qemu can use this to create host dmabufs for guest framebuffers. diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c index 7eee3eb47a8e..79845565089d 100644 --- a/drivers/dma-buf/udmabuf.c +++ b/drivers/dma-buf/udmabuf.c @@ -109,29 +109,21 @@ static int mmap_udmabuf(struct dma_buf *buf, struct vm_area_struct *vma) static int vmap_udmabuf(struct dma_buf *buf, struct iosys_map *map) { struct udmabuf *ubuf = buf->priv; - unsigned long *pfns; + struct page **pages; void *vaddr; pgoff_t pg; dma_resv_assert_held(buf->resv); - /** - * HVO may free tail pages, so just use pfn to map each folio - * into vmalloc area. - */ - pfns = kvmalloc_array(ubuf->pagecount, sizeof(*pfns), GFP_KERNEL); - if (!pfns) + pages = kvmalloc_array(ubuf->pagecount, sizeof(*pages), GFP_KERNEL); + if (!pages) return -ENOMEM; - for (pg = 0; pg < ubuf->pagecount; pg++) { - unsigned long pfn = folio_pfn(ubuf->folios[pg]); - - pfn += ubuf->offsets[pg] >> PAGE_SHIFT; - pfns[pg] = pfn; - } + for (pg = 0; pg < ubuf->pagecount; pg++) + pages[pg] = &ubuf->folios[pg]->page; - vaddr = vmap_pfn(pfns, ubuf->pagecount, PAGE_KERNEL); - kvfree(pfns); + vaddr = vm_map_ram(pages, ubuf->pagecount, -1); + kvfree(pages); if (!vaddr) return -EINVAL; -- 2.48.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* RE: [PATCH 1/2] Revert "udmabuf: fix vmap_udmabuf error page set" 2025-04-23 7:26 ` Huan Yang @ 2025-04-28 4:16 ` Kasireddy, Vivek 2025-04-28 7:03 ` Huan Yang 0 siblings, 1 reply; 11+ messages in thread From: Kasireddy, Vivek @ 2025-04-28 4:16 UTC (permalink / raw) To: Huan Yang, Sumit Semwal, Christian König, Gerd Hoffmann, Andrew Morton, Dave Airlie, linux-media@vger.kernel.org, dri-devel@lists.freedesktop.org, linaro-mm-sig@lists.linaro.org, linux-kernel@vger.kernel.org Cc: opensource.kernel@vivo.com, Bingbu Cao Hi Huan, > Subject: Re: [PATCH 1/2] Revert "udmabuf: fix vmap_udmabuf error page set" > > From 38aa11d92f209e7529736f3e11e08dfc804bdfae Mon Sep 17 00:00:00 > 2001 > From: Huan Yang <link@vivo.com> > Date: Tue, 15 Apr 2025 10:04:18 +0800 > Subject: [PATCH 1/2] Revert "udmabuf: fix vmap_udmabuf error page set" > > This reverts commit 18d7de823b7150344d242c3677e65d68c5271b04. > > This given a misuse of vmap_pfn, vmap_pfn give a !pfn_valid check > to avoid user miss use it. This API design to only for none-page struct > based user invoke, i.e. PCIe BARs and other. So any page based will > inject by !pfn_valid check. > > udmabuf used shmem or hugetlb as folio src, hence, page/folio based, > can't use it. Please consider having a commit message like below and resend both patches: "We cannot use vmap_pfn() in vmap_udmabuf() as it would fail the pfn_valid() check in vmap_pfn_apply(). This is because vmap_pfn() is intended to be used for mapping non-struct-page memory such as PCIe BARs. Since, udmabuf mostly works with pages/folios backed by shmem/hugetlbfs/THP, vmap_pfn() is not the right tool or API to invoke for implementing vmap." Thanks, Vivek > > Signed-off-by: Huan Yang <link@vivo.com> > Reported-by: Bingbu Cao <bingbu.cao@linux.intel.com> > Closes: https://lore.kernel.org/dri-devel/eb7e0137-3508-4287-98c4- > 816c5fd98e10@vivo.com/T/#mbda4f64a3532b32e061f4e8763bc8e307bea3ca > 8 > Acked-by: Vivek Kasireddy <vivek.kasireddy@intel.com> > --- > drivers/dma-buf/Kconfig | 1 - > drivers/dma-buf/udmabuf.c | 22 +++++++--------------- > 2 files changed, 7 insertions(+), 16 deletions(-) > > diff --git a/drivers/dma-buf/Kconfig b/drivers/dma-buf/Kconfig > index fee04fdb0822..b46eb8a552d7 100644 > --- a/drivers/dma-buf/Kconfig > +++ b/drivers/dma-buf/Kconfig > @@ -36,7 +36,6 @@ config UDMABUF > depends on DMA_SHARED_BUFFER > depends on MEMFD_CREATE || COMPILE_TEST > depends on MMU > - select VMAP_PFN > help > A driver to let userspace turn memfd regions into dma-bufs. > Qemu can use this to create host dmabufs for guest framebuffers. > diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c > index 7eee3eb47a8e..79845565089d 100644 > --- a/drivers/dma-buf/udmabuf.c > +++ b/drivers/dma-buf/udmabuf.c > @@ -109,29 +109,21 @@ static int mmap_udmabuf(struct dma_buf *buf, > struct vm_area_struct *vma) > static int vmap_udmabuf(struct dma_buf *buf, struct iosys_map *map) > { > struct udmabuf *ubuf = buf->priv; > - unsigned long *pfns; > + struct page **pages; > void *vaddr; > pgoff_t pg; > > dma_resv_assert_held(buf->resv); > > - /** > - * HVO may free tail pages, so just use pfn to map each folio > - * into vmalloc area. > - */ > - pfns = kvmalloc_array(ubuf->pagecount, sizeof(*pfns), GFP_KERNEL); > - if (!pfns) > + pages = kvmalloc_array(ubuf->pagecount, sizeof(*pages), GFP_KERNEL); > + if (!pages) > return -ENOMEM; > > - for (pg = 0; pg < ubuf->pagecount; pg++) { > - unsigned long pfn = folio_pfn(ubuf->folios[pg]); > - > - pfn += ubuf->offsets[pg] >> PAGE_SHIFT; > - pfns[pg] = pfn; > - } > + for (pg = 0; pg < ubuf->pagecount; pg++) > + pages[pg] = &ubuf->folios[pg]->page; > > - vaddr = vmap_pfn(pfns, ubuf->pagecount, PAGE_KERNEL); > - kvfree(pfns); > + vaddr = vm_map_ram(pages, ubuf->pagecount, -1); > + kvfree(pages); > if (!vaddr) > return -EINVAL; > > -- > 2.48.1 > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] Revert "udmabuf: fix vmap_udmabuf error page set" 2025-04-28 4:16 ` Kasireddy, Vivek @ 2025-04-28 7:03 ` Huan Yang 0 siblings, 0 replies; 11+ messages in thread From: Huan Yang @ 2025-04-28 7:03 UTC (permalink / raw) To: Kasireddy, Vivek, Sumit Semwal, Christian König, Gerd Hoffmann, Andrew Morton, Dave Airlie, linux-media@vger.kernel.org, dri-devel@lists.freedesktop.org, linaro-mm-sig@lists.linaro.org, linux-kernel@vger.kernel.org Cc: opensource.kernel@vivo.com, Bingbu Cao 在 2025/4/28 12:16, Kasireddy, Vivek 写道: > Hi Huan, > >> Subject: Re: [PATCH 1/2] Revert "udmabuf: fix vmap_udmabuf error page set" >> >> From 38aa11d92f209e7529736f3e11e08dfc804bdfae Mon Sep 17 00:00:00 >> 2001 >> From: Huan Yang <link@vivo.com> >> Date: Tue, 15 Apr 2025 10:04:18 +0800 >> Subject: [PATCH 1/2] Revert "udmabuf: fix vmap_udmabuf error page set" >> >> This reverts commit 18d7de823b7150344d242c3677e65d68c5271b04. >> >> This given a misuse of vmap_pfn, vmap_pfn give a !pfn_valid check >> to avoid user miss use it. This API design to only for none-page struct >> based user invoke, i.e. PCIe BARs and other. So any page based will >> inject by !pfn_valid check. >> >> udmabuf used shmem or hugetlb as folio src, hence, page/folio based, >> can't use it. > Please consider having a commit message like below and resend both patches: > "We cannot use vmap_pfn() in vmap_udmabuf() as it would fail the pfn_valid() > check in vmap_pfn_apply(). This is because vmap_pfn() is intended to be > used for mapping non-struct-page memory such as PCIe BARs. Since, udmabuf > mostly works with pages/folios backed by shmem/hugetlbfs/THP, vmap_pfn() > is not the right tool or API to invoke for implementing vmap." Thanks, that's clearer, I'll update in v2 version. :) Huan > > Thanks, > Vivek > >> Signed-off-by: Huan Yang <link@vivo.com> >> Reported-by: Bingbu Cao <bingbu.cao@linux.intel.com> >> Closes: https://lore.kernel.org/dri-devel/eb7e0137-3508-4287-98c4- >> 816c5fd98e10@vivo.com/T/#mbda4f64a3532b32e061f4e8763bc8e307bea3ca >> 8 >> Acked-by: Vivek Kasireddy <vivek.kasireddy@intel.com> >> --- >> drivers/dma-buf/Kconfig | 1 - >> drivers/dma-buf/udmabuf.c | 22 +++++++--------------- >> 2 files changed, 7 insertions(+), 16 deletions(-) >> >> diff --git a/drivers/dma-buf/Kconfig b/drivers/dma-buf/Kconfig >> index fee04fdb0822..b46eb8a552d7 100644 >> --- a/drivers/dma-buf/Kconfig >> +++ b/drivers/dma-buf/Kconfig >> @@ -36,7 +36,6 @@ config UDMABUF >> depends on DMA_SHARED_BUFFER >> depends on MEMFD_CREATE || COMPILE_TEST >> depends on MMU >> - select VMAP_PFN >> help >> A driver to let userspace turn memfd regions into dma-bufs. >> Qemu can use this to create host dmabufs for guest framebuffers. >> diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c >> index 7eee3eb47a8e..79845565089d 100644 >> --- a/drivers/dma-buf/udmabuf.c >> +++ b/drivers/dma-buf/udmabuf.c >> @@ -109,29 +109,21 @@ static int mmap_udmabuf(struct dma_buf *buf, >> struct vm_area_struct *vma) >> static int vmap_udmabuf(struct dma_buf *buf, struct iosys_map *map) >> { >> struct udmabuf *ubuf = buf->priv; >> - unsigned long *pfns; >> + struct page **pages; >> void *vaddr; >> pgoff_t pg; >> >> dma_resv_assert_held(buf->resv); >> >> - /** >> - * HVO may free tail pages, so just use pfn to map each folio >> - * into vmalloc area. >> - */ >> - pfns = kvmalloc_array(ubuf->pagecount, sizeof(*pfns), GFP_KERNEL); >> - if (!pfns) >> + pages = kvmalloc_array(ubuf->pagecount, sizeof(*pages), GFP_KERNEL); >> + if (!pages) >> return -ENOMEM; >> >> - for (pg = 0; pg < ubuf->pagecount; pg++) { >> - unsigned long pfn = folio_pfn(ubuf->folios[pg]); >> - >> - pfn += ubuf->offsets[pg] >> PAGE_SHIFT; >> - pfns[pg] = pfn; >> - } >> + for (pg = 0; pg < ubuf->pagecount; pg++) >> + pages[pg] = &ubuf->folios[pg]->page; >> >> - vaddr = vmap_pfn(pfns, ubuf->pagecount, PAGE_KERNEL); >> - kvfree(pfns); >> + vaddr = vm_map_ram(pages, ubuf->pagecount, -1); >> + kvfree(pages); >> if (!vaddr) >> return -EINVAL; >> >> -- >> 2.48.1 >> ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/2] udmabuf: fix vmap missed offset page 2025-04-15 3:15 [PATCH 0/2] Fix udmabuf vmap error Huan Yang 2025-04-15 3:15 ` [PATCH 1/2] Revert "udmabuf: fix vmap_udmabuf error page set" Huan Yang @ 2025-04-15 3:15 ` Huan Yang 2025-04-22 5:22 ` Kasireddy, Vivek 1 sibling, 1 reply; 11+ messages in thread From: Huan Yang @ 2025-04-15 3:15 UTC (permalink / raw) To: Sumit Semwal, Christian König, Gerd Hoffmann, Vivek Kasireddy, Andrew Morton, Dave Airlie, linux-media, dri-devel, linaro-mm-sig, linux-kernel Cc: opensource.kernel, Huan Yang Before invoke vmap, we need offer a pages pointer array which each page need to map in vmalloc area. But currently vmap_udmabuf only set each folio's head page into pages, missed each offset pages when iter. This patch set the correctly offset page in each folio into array. Signed-off-by: Huan Yang <link@vivo.com> Fixes: 5e72b2b41a21 ("udmabuf: convert udmabuf driver to use folios") --- drivers/dma-buf/udmabuf.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c index 79845565089d..af5200e360a6 100644 --- a/drivers/dma-buf/udmabuf.c +++ b/drivers/dma-buf/udmabuf.c @@ -120,7 +120,8 @@ static int vmap_udmabuf(struct dma_buf *buf, struct iosys_map *map) return -ENOMEM; for (pg = 0; pg < ubuf->pagecount; pg++) - pages[pg] = &ubuf->folios[pg]->page; + pages[pg] = folio_page(ubuf->folios[pg], + ubuf->offsets[pg] >> PAGE_SHIFT); vaddr = vm_map_ram(pages, ubuf->pagecount, -1); kvfree(pages); -- 2.48.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* RE: [PATCH 2/2] udmabuf: fix vmap missed offset page 2025-04-15 3:15 ` [PATCH 2/2] udmabuf: fix vmap missed offset page Huan Yang @ 2025-04-22 5:22 ` Kasireddy, Vivek 2025-04-23 7:25 ` Huan Yang 2025-04-25 2:59 ` Bingbu Cao 0 siblings, 2 replies; 11+ messages in thread From: Kasireddy, Vivek @ 2025-04-22 5:22 UTC (permalink / raw) To: Huan Yang, Sumit Semwal, Christian König, Gerd Hoffmann, Andrew Morton, Dave Airlie, linux-media@vger.kernel.org, dri-devel@lists.freedesktop.org, linaro-mm-sig@lists.linaro.org, linux-kernel@vger.kernel.org Cc: opensource.kernel@vivo.com, bingbu.cao@linux.intel.com Hi Huan, > Subject: [PATCH 2/2] udmabuf: fix vmap missed offset page > > Before invoke vmap, we need offer a pages pointer array which each page > need to map in vmalloc area. > > But currently vmap_udmabuf only set each folio's head page into pages, > missed each offset pages when iter. > > This patch set the correctly offset page in each folio into array. > > Signed-off-by: Huan Yang <link@vivo.com> > Fixes: 5e72b2b41a21 ("udmabuf: convert udmabuf driver to use folios") > --- > drivers/dma-buf/udmabuf.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c > index 79845565089d..af5200e360a6 100644 > --- a/drivers/dma-buf/udmabuf.c > +++ b/drivers/dma-buf/udmabuf.c > @@ -120,7 +120,8 @@ static int vmap_udmabuf(struct dma_buf *buf, struct > iosys_map *map) > return -ENOMEM; > > for (pg = 0; pg < ubuf->pagecount; pg++) > - pages[pg] = &ubuf->folios[pg]->page; > + pages[pg] = folio_page(ubuf->folios[pg], > + ubuf->offsets[pg] >> PAGE_SHIFT); IIUC, it does not look like vm_map_ram() or the other functions it calls would write to these tail page pointers (struct page*), which should be safe even when HVO is enabled (based on your conversations with Muchun). However, I am wondering whether Bingbu can test this out with HVO enabled? Regardless, Acked-by: Vivek Kasireddy <vivek.kasireddy@intel.com> Thanks, Vivek > > vaddr = vm_map_ram(pages, ubuf->pagecount, -1); > kvfree(pages); > -- > 2.48.1 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] udmabuf: fix vmap missed offset page 2025-04-22 5:22 ` Kasireddy, Vivek @ 2025-04-23 7:25 ` Huan Yang 2025-04-25 2:59 ` Bingbu Cao 1 sibling, 0 replies; 11+ messages in thread From: Huan Yang @ 2025-04-23 7:25 UTC (permalink / raw) To: Kasireddy, Vivek, Sumit Semwal, Christian König, Gerd Hoffmann, Andrew Morton, Dave Airlie, linux-media@vger.kernel.org, dri-devel@lists.freedesktop.org, linaro-mm-sig@lists.linaro.org, linux-kernel@vger.kernel.org Cc: opensource.kernel@vivo.com, bingbu.cao@linux.intel.com Hi Vivek 在 2025/4/22 13:22, Kasireddy, Vivek 写道: > Hi Huan, > >> Subject: [PATCH 2/2] udmabuf: fix vmap missed offset page >> >> Before invoke vmap, we need offer a pages pointer array which each page >> need to map in vmalloc area. >> >> But currently vmap_udmabuf only set each folio's head page into pages, >> missed each offset pages when iter. >> >> This patch set the correctly offset page in each folio into array. >> >> Signed-off-by: Huan Yang <link@vivo.com> >> Fixes: 5e72b2b41a21 ("udmabuf: convert udmabuf driver to use folios") >> --- >> drivers/dma-buf/udmabuf.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c >> index 79845565089d..af5200e360a6 100644 >> --- a/drivers/dma-buf/udmabuf.c >> +++ b/drivers/dma-buf/udmabuf.c >> @@ -120,7 +120,8 @@ static int vmap_udmabuf(struct dma_buf *buf, struct >> iosys_map *map) >> return -ENOMEM; >> >> for (pg = 0; pg < ubuf->pagecount; pg++) >> - pages[pg] = &ubuf->folios[pg]->page; >> + pages[pg] = folio_page(ubuf->folios[pg], >> + ubuf->offsets[pg] >> PAGE_SHIFT); > IIUC, it does not look like vm_map_ram() or the other functions it calls would > write to these tail page pointers (struct page*), which should be safe even > when HVO is enabled (based on your conversations with Muchun). However, Yes, and need point, each write to tail page's va pointer is no permit.(HVO changed this va in vmemmap's prot into RO), so, easy to observe. And I see each vmap-api, only turn to read it, no write. :) Thanks, Huan > I am wondering whether Bingbu can test this out with HVO enabled? > > Regardless, > Acked-by: Vivek Kasireddy <vivek.kasireddy@intel.com> > > Thanks, > Vivek > >> vaddr = vm_map_ram(pages, ubuf->pagecount, -1); >> kvfree(pages); >> -- >> 2.48.1 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] udmabuf: fix vmap missed offset page 2025-04-22 5:22 ` Kasireddy, Vivek 2025-04-23 7:25 ` Huan Yang @ 2025-04-25 2:59 ` Bingbu Cao 1 sibling, 0 replies; 11+ messages in thread From: Bingbu Cao @ 2025-04-25 2:59 UTC (permalink / raw) To: Kasireddy, Vivek, Huan Yang, Sumit Semwal, Christian König, Gerd Hoffmann, Andrew Morton, Dave Airlie, linux-media@vger.kernel.org, dri-devel@lists.freedesktop.org, linaro-mm-sig@lists.linaro.org, linux-kernel@vger.kernel.org Cc: opensource.kernel@vivo.com On 4/22/25 1:22 PM, Kasireddy, Vivek wrote: > Hi Huan, > >> Subject: [PATCH 2/2] udmabuf: fix vmap missed offset page >> >> Before invoke vmap, we need offer a pages pointer array which each page >> need to map in vmalloc area. >> >> But currently vmap_udmabuf only set each folio's head page into pages, >> missed each offset pages when iter. >> >> This patch set the correctly offset page in each folio into array. >> >> Signed-off-by: Huan Yang <link@vivo.com> >> Fixes: 5e72b2b41a21 ("udmabuf: convert udmabuf driver to use folios") >> --- >> drivers/dma-buf/udmabuf.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c >> index 79845565089d..af5200e360a6 100644 >> --- a/drivers/dma-buf/udmabuf.c >> +++ b/drivers/dma-buf/udmabuf.c >> @@ -120,7 +120,8 @@ static int vmap_udmabuf(struct dma_buf *buf, struct >> iosys_map *map) >> return -ENOMEM; >> >> for (pg = 0; pg < ubuf->pagecount; pg++) >> - pages[pg] = &ubuf->folios[pg]->page; >> + pages[pg] = folio_page(ubuf->folios[pg], >> + ubuf->offsets[pg] >> PAGE_SHIFT); > IIUC, it does not look like vm_map_ram() or the other functions it calls would > write to these tail page pointers (struct page*), which should be safe even > when HVO is enabled (based on your conversations with Muchun). However, > I am wondering whether Bingbu can test this out with HVO enabled? Sorry, I cannot test HVO enabled case. I was running my case with local revert patch. :) > > Regardless, > Acked-by: Vivek Kasireddy <vivek.kasireddy@intel.com> > > Thanks, > Vivek > >> >> vaddr = vm_map_ram(pages, ubuf->pagecount, -1); >> kvfree(pages); >> -- >> 2.48.1 > -- Best regards, Bingbu Cao ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2025-04-28 7:04 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-04-15 3:15 [PATCH 0/2] Fix udmabuf vmap error Huan Yang 2025-04-15 3:15 ` [PATCH 1/2] Revert "udmabuf: fix vmap_udmabuf error page set" Huan Yang 2025-04-22 5:14 ` Kasireddy, Vivek 2025-04-23 7:22 ` Huan Yang 2025-04-23 7:26 ` Huan Yang 2025-04-28 4:16 ` Kasireddy, Vivek 2025-04-28 7:03 ` Huan Yang 2025-04-15 3:15 ` [PATCH 2/2] udmabuf: fix vmap missed offset page Huan Yang 2025-04-22 5:22 ` Kasireddy, Vivek 2025-04-23 7:25 ` Huan Yang 2025-04-25 2:59 ` Bingbu Cao
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox