* [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
* [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 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 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 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 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 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 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
* 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
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;
as well as URLs for NNTP newsgroup(s).