On Sun, Aug 7, 2022 at 9:09 AM Christian König <ckoenig.leichtzumerken@xxxxxxxxx> wrote: > > Am 29.07.22 um 19:07 schrieb Rob Clark: > > From: Rob Clark <robdclark@xxxxxxxxxxxx> > > > > 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 ;-) 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. 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@xxxxxxxxxxxx> > > --- > > 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 >