The reason behind that patch is associated with videobuf2 subsystem (or more genrally with v4l2 framework) and user created dma buffers (udmabuf). In some circumstances when dealing with V4L2_MEMORY_DMABUF buffers videobuf2 subsystem wants to use dma_buf_vmap() method on the attached dma buffer. As udmabuf does not have .vmap operation implemented, such dma_buf_vmap() natually fails.
videobuf2_common: [cap-000000003473b2f1] __vb2_queue_alloc: allocated 3 buffers, 1 plane(s) each videobuf2_common: [cap-000000003473b2f1] __prepare_dmabuf: buffer for plane 0 changed videobuf2_common: [cap-000000003473b2f1] __prepare_dmabuf: failed to map dmabuf for plane 0 videobuf2_common: [cap-000000003473b2f1] __buf_prepare: buffer preparation failed: -14
The patch itself seems to be strighforward. It adds implementation of .vmap method to 'struct dma_buf_ops udmabuf_ops'. .vmap method itself uses vm_map_ram() to map pages linearly into the kernel virtual address space (only if such mapping hasn't been created yet).
Signed-off-by: Lukasz Wiecaszek lukasz.wiecaszek@gmail.com Reported-by: kernel test robot lkp@intel.com --- v1: https://lore.kernel.org/linux-media/202211120352.G7WPASoP-lkp@intel.com/T/#t
v1 -> v2: Patch prepared and tested against 6.1.0-rc2+
drivers/dma-buf/udmabuf.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)
diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c index 2bcdb935a3ac..2ca0e3639360 100644 --- a/drivers/dma-buf/udmabuf.c +++ b/drivers/dma-buf/udmabuf.c @@ -12,6 +12,8 @@ #include <linux/slab.h> #include <linux/udmabuf.h> #include <linux/hugetlb.h> +#include <linux/vmalloc.h> +#include <linux/iosys-map.h>
static int list_limit = 1024; module_param(list_limit, int, 0644); @@ -26,6 +28,7 @@ struct udmabuf { struct page **pages; struct sg_table *sg; struct miscdevice *device; + void *vaddr; };
static vm_fault_t udmabuf_vm_fault(struct vm_fault *vmf) @@ -57,6 +60,21 @@ static int mmap_udmabuf(struct dma_buf *buf, struct vm_area_struct *vma) return 0; }
+static int vmap_udmabuf(struct dma_buf *buf, struct iosys_map *map) +{ + struct udmabuf *ubuf = buf->priv; + + if (!ubuf->vaddr) { + ubuf->vaddr = vm_map_ram(ubuf->pages, ubuf->pagecount, -1); + if (!ubuf->vaddr) + return -EINVAL; + } + + iosys_map_set_vaddr(map, ubuf->vaddr); + + return 0; +} + static struct sg_table *get_sg_table(struct device *dev, struct dma_buf *buf, enum dma_data_direction direction) { @@ -159,6 +177,7 @@ static const struct dma_buf_ops udmabuf_ops = { .unmap_dma_buf = unmap_udmabuf, .release = release_udmabuf, .mmap = mmap_udmabuf, + .vmap = vmap_udmabuf, .begin_cpu_access = begin_cpu_udmabuf, .end_cpu_access = end_cpu_udmabuf, };
On 11/13/22 18:05, Lukasz Wiecaszek wrote:
The reason behind that patch is associated with videobuf2 subsystem (or more genrally with v4l2 framework) and user created dma buffers (udmabuf). In some circumstances when dealing with V4L2_MEMORY_DMABUF buffers videobuf2 subsystem wants to use dma_buf_vmap() method on the attached dma buffer. As udmabuf does not have .vmap operation implemented, such dma_buf_vmap() natually fails.
videobuf2_common: [cap-000000003473b2f1] __vb2_queue_alloc: allocated 3 buffers, 1 plane(s) each videobuf2_common: [cap-000000003473b2f1] __prepare_dmabuf: buffer for plane 0 changed videobuf2_common: [cap-000000003473b2f1] __prepare_dmabuf: failed to map dmabuf for plane 0 videobuf2_common: [cap-000000003473b2f1] __buf_prepare: buffer preparation failed: -14
The patch itself seems to be strighforward. It adds implementation of .vmap method to 'struct dma_buf_ops udmabuf_ops'. .vmap method itself uses vm_map_ram() to map pages linearly into the kernel virtual address space (only if such mapping hasn't been created yet).
Signed-off-by: Lukasz Wiecaszek lukasz.wiecaszek@gmail.com Reported-by: kernel test robot lkp@intel.com
v1: https://lore.kernel.org/linux-media/202211120352.G7WPASoP-lkp@intel.com/T/#t
v1 -> v2: Patch prepared and tested against 6.1.0-rc2+
drivers/dma-buf/udmabuf.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)
diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c index 2bcdb935a3ac..2ca0e3639360 100644 --- a/drivers/dma-buf/udmabuf.c +++ b/drivers/dma-buf/udmabuf.c @@ -12,6 +12,8 @@ #include <linux/slab.h> #include <linux/udmabuf.h> #include <linux/hugetlb.h> +#include <linux/vmalloc.h> +#include <linux/iosys-map.h> static int list_limit = 1024; module_param(list_limit, int, 0644); @@ -26,6 +28,7 @@ struct udmabuf { struct page **pages; struct sg_table *sg; struct miscdevice *device;
- void *vaddr;
}; static vm_fault_t udmabuf_vm_fault(struct vm_fault *vmf) @@ -57,6 +60,21 @@ static int mmap_udmabuf(struct dma_buf *buf, struct vm_area_struct *vma) return 0; } +static int vmap_udmabuf(struct dma_buf *buf, struct iosys_map *map) +{
- struct udmabuf *ubuf = buf->priv;
- if (!ubuf->vaddr) {
ubuf->vaddr = vm_map_ram(ubuf->pages, ubuf->pagecount, -1);
if (!ubuf->vaddr)
return -EINVAL;
- }
- iosys_map_set_vaddr(map, ubuf->vaddr);
- return 0;
+}
static struct sg_table *get_sg_table(struct device *dev, struct dma_buf *buf, enum dma_data_direction direction) { @@ -159,6 +177,7 @@ static const struct dma_buf_ops udmabuf_ops = { .unmap_dma_buf = unmap_udmabuf, .release = release_udmabuf, .mmap = mmap_udmabuf,
- .vmap = vmap_udmabuf, .begin_cpu_access = begin_cpu_udmabuf, .end_cpu_access = end_cpu_udmabuf,
};
Where is vunmap?
On 11/13/22 18:05, Lukasz Wiecaszek wrote:
+static int vmap_udmabuf(struct dma_buf *buf, struct iosys_map *map) +{
- struct udmabuf *ubuf = buf->priv;
- if (!ubuf->vaddr) {
ubuf->vaddr = vm_map_ram(ubuf->pages, ubuf->pagecount, -1);
if (!ubuf->vaddr)
return -EINVAL;
- }
Create a new mapping on each vmap_udmabuf() and add the corresponding vunmap.
Otherwise persistent vmapping shall be released together with udmabuf. It doesn't look that persistent vmapping is needed for udmabufs.
On Sun, Nov 13, 2022 at 07:35:20PM +0300, Dmitry Osipenko wrote:
On 11/13/22 18:05, Lukasz Wiecaszek wrote:
+static int vmap_udmabuf(struct dma_buf *buf, struct iosys_map *map) +{
- struct udmabuf *ubuf = buf->priv;
- if (!ubuf->vaddr) {
ubuf->vaddr = vm_map_ram(ubuf->pages, ubuf->pagecount, -1);
if (!ubuf->vaddr)
return -EINVAL;
- }
Create a new mapping on each vmap_udmabuf() and add the corresponding vunmap.
Otherwise persistent vmapping shall be released together with udmabuf. It doesn't look that persistent vmapping is needed for udmabufs.
-- Best regards, Dmitry
Right. Thanks for review and remarks. Adding vunmap sounds reasonable to me. Will add it somehow this week.
Regards, Lukasz
linaro-mm-sig@lists.linaro.org