On Tue, Sep 6, 2022 at 12:46 PM Daniel Vetter <daniel@xxxxxxxx> wrote: > > On Mon, Aug 01, 2022 at 10:04:55AM -0700, Rob Clark wrote: > > 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). 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> > > --- > > v2. fix compiler warning > > I think I bikeshedded this on irc already, here for the record too. You should look at v3 (which I confusingly titled as v2, sry): https://patchwork.freedesktop.org/patch/497799/?series=106847&rev=3 > - this wont work for buffers which do change the mapping when they move > (ttm can do that). And cros does make noises about discrete gpus I've > heard, this matters even for you :-) Correct, in v3 you could use DMA_BUF_MAP_INCOHERENT for this case (or we could add additional enum values.. DMA_BUF_MAP_IDK or whatever) re: dgpu, I guess those will be pass-thru so not so relevant for this issue > - I'm pretty sure this will put is even more onto the nasty people list > that dma-api folks maintain, especially with passing this all to > userspace > - follow_pte() can figure this out internally in the kernel and kvm is > already using this, and I think doing this all internally with mmu > notifier and what not to make sure it all stays in sync is the right > approach. So your kvm/whatever combo should be able to figure out wth > it's supposed to be doing. This doesn't help, because the VMM is in userspace.. it is the VMM which needs this information. > I think if you make this a virtio special case like we've done with the > magic uuid stuff, then that would make sense. Making it a full dma-buf > interface doesn't imo. IMHO we can consider this (at least in case of v3) as a virtio special, at least in the sense that it is opt-in for the exporting driver, and exporting drivers are free to not play along BR, -R > > Cheers, Daniel > > > > > 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..87c52f080274 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, 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, (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 > > -- > > 2.36.1 > > > > _______________________________________________ > > Linaro-mm-sig mailing list -- linaro-mm-sig@xxxxxxxxxxxxxxxx > > To unsubscribe send an email to linaro-mm-sig-leave@xxxxxxxxxxxxxxxx > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch
