Messages in this thread | | | Date | Sun, 7 Aug 2022 19:14:43 +0200 | Subject | Re: [Linaro-mm-sig] [PATCH 1/3] dma-buf: Add ioctl to query mmap info | From | Christian König <> |
| |
Am 07.08.22 um 19:02 schrieb Rob Clark: > On Sun, Aug 7, 2022 at 9:09 AM Christian König > <ckoenig.leichtzumerken@gmail.com> wrote: >> Am 29.07.22 um 19:07 schrieb Rob Clark: >>> From: Rob Clark <robdclark@chromium.org> >>> >>> This is a fairly narrowly focused interface, providing a way for a VMM >>> in userspace to tell the guest kernel what pgprot settings to use when >>> mapping a buffer to guest userspace. >>> >>> For buffers that get mapped into guest userspace, virglrenderer returns >>> a dma-buf fd to the VMM (crosvm or qemu). >> Wow, wait a second. Who is giving whom the DMA-buf fd here? > Not sure I understand the question.. the dma-buf fd could come from > EGL_MESA_image_dma_buf_export, gbm, or similar. > >> My last status was that this design was illegal and couldn't be >> implemented because it requires internal knowledge only the exporting >> driver can have. > This ioctl provides that information from the exporting driver so that > a VMM doesn't have to make assumptions ;-)
And exactly that was NAKed the last time it came up. Only the exporting driver is allowed to mmap() the DMA-buf into the guest.
This way you also don't need to transport any caching information anywhere.
> Currently crosvm assumes if (drivername == "i915") then it is a cached > mapping, otherwise it is wc. I'm trying to find a way to fix this. > Suggestions welcome, but because of how mapping to a guest VM works, a > VMM is a somewhat special case where this information is needed in > userspace.
Ok that leaves me completely puzzled. How does that work in the first place?
In other words how does the mapping into the guest page tables happen?
Regards, Christian.
> > BR, > -R > >> @Daniel has anything changed on that is or my status still valid? >> >> Regards, >> Christian. >> >>> In addition to mapping the >>> pages into the guest VM, it needs to report to drm/virtio in the guest >>> the cache settings to use for guest userspace. In particular, on some >>> architectures, creating aliased mappings with different cache attributes >>> is frowned upon, so it is important that the guest mappings have the >>> same cache attributes as any potential host mappings. >>> >>> Signed-off-by: Rob Clark <robdclark@chromium.org> >>> --- >>> drivers/dma-buf/dma-buf.c | 26 ++++++++++++++++++++++++++ >>> include/linux/dma-buf.h | 7 +++++++ >>> include/uapi/linux/dma-buf.h | 28 ++++++++++++++++++++++++++++ >>> 3 files changed, 61 insertions(+) >>> >>> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c >>> index 32f55640890c..d02d6c2a3b49 100644 >>> --- a/drivers/dma-buf/dma-buf.c >>> +++ b/drivers/dma-buf/dma-buf.c >>> @@ -326,6 +326,29 @@ static long dma_buf_set_name(struct dma_buf *dmabuf, const char __user *buf) >>> return 0; >>> } >>> >>> +static long dma_buf_info(struct dma_buf *dmabuf, const void __user *uarg) >>> +{ >>> + struct dma_buf_info arg; >>> + >>> + if (copy_from_user(&arg, uarg, sizeof(arg))) >>> + return -EFAULT; >>> + >>> + switch (arg.param) { >>> + case DMA_BUF_INFO_VM_PROT: >>> + if (!dmabuf->ops->mmap_info) >>> + return -ENOSYS; >>> + arg.value = dmabuf->ops->mmap_info(dmabuf); >>> + break; >>> + default: >>> + return -EINVAL; >>> + } >>> + >>> + if (copy_to_user(uarg, &arg, sizeof(arg))) >>> + return -EFAULT; >>> + >>> + return 0; >>> +} >>> + >>> static long dma_buf_ioctl(struct file *file, >>> unsigned int cmd, unsigned long arg) >>> { >>> @@ -369,6 +392,9 @@ static long dma_buf_ioctl(struct file *file, >>> case DMA_BUF_SET_NAME_B: >>> return dma_buf_set_name(dmabuf, (const char __user *)arg); >>> >>> + case DMA_BUF_IOCTL_INFO: >>> + return dma_buf_info(dmabuf, (const void __user *)arg); >>> + >>> default: >>> return -ENOTTY; >>> } >>> diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h >>> index 71731796c8c3..6f4de64a5937 100644 >>> --- a/include/linux/dma-buf.h >>> +++ b/include/linux/dma-buf.h >>> @@ -283,6 +283,13 @@ struct dma_buf_ops { >>> */ >>> int (*mmap)(struct dma_buf *, struct vm_area_struct *vma); >>> >>> + /** >>> + * @mmap_info: >>> + * >>> + * Return mmapping info for the buffer. See DMA_BUF_INFO_VM_PROT. >>> + */ >>> + int (*mmap_info)(struct dma_buf *); >>> + >>> int (*vmap)(struct dma_buf *dmabuf, struct iosys_map *map); >>> void (*vunmap)(struct dma_buf *dmabuf, struct iosys_map *map); >>> }; >>> diff --git a/include/uapi/linux/dma-buf.h b/include/uapi/linux/dma-buf.h >>> index b1523cb8ab30..a41adac0f46a 100644 >>> --- a/include/uapi/linux/dma-buf.h >>> +++ b/include/uapi/linux/dma-buf.h >>> @@ -85,6 +85,32 @@ struct dma_buf_sync { >>> >>> #define DMA_BUF_NAME_LEN 32 >>> >>> + >>> +/** >>> + * struct dma_buf_info - Query info about the buffer. >>> + */ >>> +struct dma_buf_info { >>> + >>> +#define DMA_BUF_INFO_VM_PROT 1 >>> +# define DMA_BUF_VM_PROT_WC 0 >>> +# define DMA_BUF_VM_PROT_CACHED 1 >>> + >>> + /** >>> + * @param: Which param to query >>> + * >>> + * DMA_BUF_INFO_BM_PROT: >>> + * Query the access permissions of userspace mmap's of this buffer. >>> + * Returns one of DMA_BUF_VM_PROT_x >>> + */ >>> + __u32 param; >>> + __u32 pad; >>> + >>> + /** >>> + * @value: Return value of the query. >>> + */ >>> + __u64 value; >>> +}; >>> + >>> #define DMA_BUF_BASE 'b' >>> #define DMA_BUF_IOCTL_SYNC _IOW(DMA_BUF_BASE, 0, struct dma_buf_sync) >>> >>> @@ -95,4 +121,6 @@ struct dma_buf_sync { >>> #define DMA_BUF_SET_NAME_A _IOW(DMA_BUF_BASE, 1, __u32) >>> #define DMA_BUF_SET_NAME_B _IOW(DMA_BUF_BASE, 1, __u64) >>> >>> +#define DMA_BUF_IOCTL_INFO _IOWR(DMA_BUF_BASE, 2, struct dma_buf_info) >>> + >>> #endif
| |