2022-07-29 17:15:16

by Rob Clark

[permalink] [raw]
Subject: [PATCH 0/3] dma-buf: map-info support

From: Rob Clark <[email protected]>

See 1/3 for motivation.

Rob Clark (3):
dma-buf: Add ioctl to query mmap info
drm/prime: Wire up mmap_info support
drm/msm/prime: Add mmap_info support

drivers/dma-buf/dma-buf.c | 26 ++++++++++++++++++++++++++
drivers/gpu/drm/drm_prime.c | 12 ++++++++++++
drivers/gpu/drm/msm/msm_drv.c | 1 +
drivers/gpu/drm/msm/msm_drv.h | 1 +
drivers/gpu/drm/msm/msm_gem_prime.c | 11 +++++++++++
include/drm/drm_drv.h | 7 +++++++
include/linux/dma-buf.h | 7 +++++++
include/uapi/linux/dma-buf.h | 28 ++++++++++++++++++++++++++++
8 files changed, 93 insertions(+)

--
2.36.1


2022-07-29 17:15:39

by Rob Clark

[permalink] [raw]
Subject: [PATCH 3/3] drm/msm/prime: Add mmap_info support

From: Rob Clark <[email protected]>

Signed-off-by: Rob Clark <[email protected]>
---
drivers/gpu/drm/msm/msm_drv.c | 1 +
drivers/gpu/drm/msm/msm_drv.h | 1 +
drivers/gpu/drm/msm/msm_gem_prime.c | 11 +++++++++++
3 files changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index 1ca4a92ba96e..4979aa8187ec 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -1044,6 +1044,7 @@ static const struct drm_driver msm_driver = {
.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
.gem_prime_import_sg_table = msm_gem_prime_import_sg_table,
.gem_prime_mmap = msm_gem_prime_mmap,
+ .gem_prime_mmap_info= msm_gem_prime_mmap_info,
#ifdef CONFIG_DEBUG_FS
.debugfs_init = msm_debugfs_init,
#endif
diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
index 7330d7b5de8e..b4ace34ec889 100644
--- a/drivers/gpu/drm/msm/msm_drv.h
+++ b/drivers/gpu/drm/msm/msm_drv.h
@@ -271,6 +271,7 @@ void msm_gem_shrinker_init(struct drm_device *dev);
void msm_gem_shrinker_cleanup(struct drm_device *dev);

int msm_gem_prime_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma);
+int msm_gem_prime_mmap_info(struct drm_gem_object *obj);
struct sg_table *msm_gem_prime_get_sg_table(struct drm_gem_object *obj);
int msm_gem_prime_vmap(struct drm_gem_object *obj, struct iosys_map *map);
void msm_gem_prime_vunmap(struct drm_gem_object *obj, struct iosys_map *map);
diff --git a/drivers/gpu/drm/msm/msm_gem_prime.c b/drivers/gpu/drm/msm/msm_gem_prime.c
index c1d91863df05..2bacab7a1921 100644
--- a/drivers/gpu/drm/msm/msm_gem_prime.c
+++ b/drivers/gpu/drm/msm/msm_gem_prime.c
@@ -5,6 +5,7 @@
*/

#include <linux/dma-buf.h>
+#include <uapi/linux/dma-buf.h>

#include <drm/drm_prime.h>

@@ -26,6 +27,16 @@ int msm_gem_prime_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma)
return drm_gem_prime_mmap(obj, vma);
}

+int msm_gem_prime_mmap_info(struct drm_gem_object *obj)
+{
+ struct msm_gem_object *msm_obj = to_msm_bo(obj);
+
+ if (msm_obj->flags & MSM_BO_WC)
+ return DMA_BUF_VM_PROT_WC;
+
+ return DMA_BUF_VM_PROT_CACHED;
+}
+
struct sg_table *msm_gem_prime_get_sg_table(struct drm_gem_object *obj)
{
struct msm_gem_object *msm_obj = to_msm_bo(obj);
--
2.36.1

2022-07-29 17:24:30

by Rob Clark

[permalink] [raw]
Subject: [PATCH 2/3] drm/prime: Wire up mmap_info support

From: Rob Clark <[email protected]>

Just plumbing the thing thru an extra layer.

Signed-off-by: Rob Clark <[email protected]>
---
drivers/gpu/drm/drm_prime.c | 12 ++++++++++++
include/drm/drm_drv.h | 7 +++++++
2 files changed, 19 insertions(+)

diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index e3f09f18110c..f58586e131c5 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -784,6 +784,17 @@ int drm_gem_dmabuf_mmap(struct dma_buf *dma_buf, struct vm_area_struct *vma)
}
EXPORT_SYMBOL(drm_gem_dmabuf_mmap);

+static int drm_gem_dmabuf_mmap_info(struct dma_buf *dma_buf)
+{
+ struct drm_gem_object *obj = dma_buf->priv;
+ struct drm_device *dev = obj->dev;
+
+ if (!dev->driver->gem_prime_mmap_info)
+ return -ENOSYS;
+
+ return dev->driver->gem_prime_mmap_info(obj);
+}
+
static const struct dma_buf_ops drm_gem_prime_dmabuf_ops = {
.cache_sgt_mapping = true,
.attach = drm_gem_map_attach,
@@ -792,6 +803,7 @@ static const struct dma_buf_ops drm_gem_prime_dmabuf_ops = {
.unmap_dma_buf = drm_gem_unmap_dma_buf,
.release = drm_gem_dmabuf_release,
.mmap = drm_gem_dmabuf_mmap,
+ .mmap_info = drm_gem_dmabuf_mmap_info,
.vmap = drm_gem_dmabuf_vmap,
.vunmap = drm_gem_dmabuf_vunmap,
};
diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
index f6159acb8856..797c0f8c2dd0 100644
--- a/include/drm/drm_drv.h
+++ b/include/drm/drm_drv.h
@@ -355,6 +355,13 @@ struct drm_driver {
*/
int (*gem_prime_mmap)(struct drm_gem_object *obj, struct vm_area_struct *vma);

+ /**
+ * @gem_prim_mmap_info:
+ *
+ * Get info about mmap setup by gem_prime_mmap. See dma_buf_ops:mmap_info.
+ */
+ int (*gem_prime_mmap_info)(struct drm_gem_object *obj);
+
/**
* @dumb_create:
*
--
2.36.1

2022-07-29 17:37:31

by Rob Clark

[permalink] [raw]
Subject: [PATCH 1/3] dma-buf: Add ioctl to query mmap info

From: Rob Clark <[email protected]>

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 <[email protected]>
---
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
--
2.36.1

2022-07-29 21:33:24

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 1/3] dma-buf: Add ioctl to query mmap info

Hi Rob,

I love your patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v5.19-rc8]
[cannot apply to drm-misc/drm-misc-next drm/drm-next drm-exynos/exynos-drm-next drm-intel/for-linux-next drm-tip/drm-tip next-20220728]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Rob-Clark/dma-buf-map-info-support/20220730-010844
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 6e2c0490769ef8a95b61304389116ccc85c53e12
config: arc-randconfig-r043-20220729 (https://download.01.org/0day-ci/archive/20220730/[email protected]/config)
compiler: arceb-elf-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/203f14a73a179d6c5fbfa4813e45fde2a9ae9860
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Rob-Clark/dma-buf-map-info-support/20220730-010844
git checkout 203f14a73a179d6c5fbfa4813e45fde2a9ae9860
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arc SHELL=/bin/bash drivers/dma-buf/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <[email protected]>

All warnings (new ones prefixed by >>):

drivers/dma-buf/dma-buf.c: In function 'dma_buf_info':
>> drivers/dma-buf/dma-buf.c:346:26: warning: passing argument 1 of 'copy_to_user' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
346 | if (copy_to_user(uarg, &arg, sizeof(arg)))
| ^~~~
In file included from include/linux/sched/task.h:11,
from include/linux/sched/signal.h:9,
from include/linux/rcuwait.h:6,
from include/linux/percpu-rwsem.h:7,
from include/linux/fs.h:33,
from drivers/dma-buf/dma-buf.c:14:
include/linux/uaccess.h:157:27: note: expected 'void *' but argument is of type 'const void *'
157 | copy_to_user(void __user *to, const void *from, unsigned long n)
| ~~~~~~~~~~~~~^~


vim +346 drivers/dma-buf/dma-buf.c

328
329 static long dma_buf_info(struct dma_buf *dmabuf, const void __user *uarg)
330 {
331 struct dma_buf_info arg;
332
333 if (copy_from_user(&arg, uarg, sizeof(arg)))
334 return -EFAULT;
335
336 switch (arg.param) {
337 case DMA_BUF_INFO_VM_PROT:
338 if (!dmabuf->ops->mmap_info)
339 return -ENOSYS;
340 arg.value = dmabuf->ops->mmap_info(dmabuf);
341 break;
342 default:
343 return -EINVAL;
344 }
345
> 346 if (copy_to_user(uarg, &arg, sizeof(arg)))
347 return -EFAULT;
348
349 return 0;
350 }
351

--
0-DAY CI Kernel Test Service
https://01.org/lkp

2022-07-30 02:53:52

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 1/3] dma-buf: Add ioctl to query mmap info

Hi Rob,

I love your patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v5.19-rc8]
[cannot apply to drm-misc/drm-misc-next drm/drm-next drm-exynos/exynos-drm-next drm-intel/for-linux-next drm-tip/drm-tip next-20220728]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Rob-Clark/dma-buf-map-info-support/20220730-010844
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 6e2c0490769ef8a95b61304389116ccc85c53e12
config: m68k-randconfig-s032-20220729 (https://download.01.org/0day-ci/archive/20220730/[email protected]/config)
compiler: m68k-linux-gcc (GCC) 12.1.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# apt-get install sparse
# sparse version: v0.6.4-39-gce1a6720-dirty
# https://github.com/intel-lab-lkp/linux/commit/203f14a73a179d6c5fbfa4813e45fde2a9ae9860
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Rob-Clark/dma-buf-map-info-support/20220730-010844
git checkout 203f14a73a179d6c5fbfa4813e45fde2a9ae9860
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=m68k SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <[email protected]>

sparse warnings: (new ones prefixed by >>)
>> drivers/dma-buf/dma-buf.c:346:26: sparse: sparse: incorrect type in argument 1 (different modifiers) @@ expected void [noderef] __user *to @@ got void const [noderef] __user *uarg @@
drivers/dma-buf/dma-buf.c:346:26: sparse: expected void [noderef] __user *to
drivers/dma-buf/dma-buf.c:346:26: sparse: got void const [noderef] __user *uarg

vim +346 drivers/dma-buf/dma-buf.c

328
329 static long dma_buf_info(struct dma_buf *dmabuf, const void __user *uarg)
330 {
331 struct dma_buf_info arg;
332
333 if (copy_from_user(&arg, uarg, sizeof(arg)))
334 return -EFAULT;
335
336 switch (arg.param) {
337 case DMA_BUF_INFO_VM_PROT:
338 if (!dmabuf->ops->mmap_info)
339 return -ENOSYS;
340 arg.value = dmabuf->ops->mmap_info(dmabuf);
341 break;
342 default:
343 return -EINVAL;
344 }
345
> 346 if (copy_to_user(uarg, &arg, sizeof(arg)))
347 return -EFAULT;
348
349 return 0;
350 }
351

--
0-DAY CI Kernel Test Service
https://01.org/lkp

2022-07-30 06:45:25

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 1/3] dma-buf: Add ioctl to query mmap info

Hi Rob,

I love your patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v5.19-rc8]
[cannot apply to drm-misc/drm-misc-next drm/drm-next drm-exynos/exynos-drm-next drm-intel/for-linux-next drm-tip/drm-tip next-20220728]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Rob-Clark/dma-buf-map-info-support/20220730-010844
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 6e2c0490769ef8a95b61304389116ccc85c53e12
config: arm-randconfig-r033-20220729 (https://download.01.org/0day-ci/archive/20220730/[email protected]/config)
compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 52cd00cabf479aa7eb6dbb063b7ba41ea57bce9e)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install arm cross compiling tool for clang build
# apt-get install binutils-arm-linux-gnueabi
# https://github.com/intel-lab-lkp/linux/commit/203f14a73a179d6c5fbfa4813e45fde2a9ae9860
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Rob-Clark/dma-buf-map-info-support/20220730-010844
git checkout 203f14a73a179d6c5fbfa4813e45fde2a9ae9860
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash drivers/dma-buf/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <[email protected]>

All errors (new ones prefixed by >>):

>> drivers/dma-buf/dma-buf.c:346:19: error: passing 'const void *' to parameter of type 'void *' discards qualifiers [-Werror,-Wincompatible-pointer-types-discards-qualifiers]
if (copy_to_user(uarg, &arg, sizeof(arg)))
^~~~
include/linux/uaccess.h:157:27: note: passing argument to parameter 'to' here
copy_to_user(void __user *to, const void *from, unsigned long n)
^
1 error generated.


vim +346 drivers/dma-buf/dma-buf.c

328
329 static long dma_buf_info(struct dma_buf *dmabuf, const void __user *uarg)
330 {
331 struct dma_buf_info arg;
332
333 if (copy_from_user(&arg, uarg, sizeof(arg)))
334 return -EFAULT;
335
336 switch (arg.param) {
337 case DMA_BUF_INFO_VM_PROT:
338 if (!dmabuf->ops->mmap_info)
339 return -ENOSYS;
340 arg.value = dmabuf->ops->mmap_info(dmabuf);
341 break;
342 default:
343 return -EINVAL;
344 }
345
> 346 if (copy_to_user(uarg, &arg, sizeof(arg)))
347 return -EFAULT;
348
349 return 0;
350 }
351

--
0-DAY CI Kernel Test Service
https://01.org/lkp

2022-07-30 23:47:02

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 1/3] dma-buf: Add ioctl to query mmap info

Hi Rob,

I love your patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v5.19-rc8]
[cannot apply to drm-misc/drm-misc-next drm/drm-next drm-exynos/exynos-drm-next drm-intel/for-linux-next drm-tip/drm-tip next-20220728]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Rob-Clark/dma-buf-map-info-support/20220730-010844
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 6e2c0490769ef8a95b61304389116ccc85c53e12
config: hexagon-randconfig-r032-20220729 (https://download.01.org/0day-ci/archive/20220731/[email protected]/config)
compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 52cd00cabf479aa7eb6dbb063b7ba41ea57bce9e)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/203f14a73a179d6c5fbfa4813e45fde2a9ae9860
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Rob-Clark/dma-buf-map-info-support/20220730-010844
git checkout 203f14a73a179d6c5fbfa4813e45fde2a9ae9860
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash drivers/dma-buf/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <[email protected]>

All errors (new ones prefixed by >>):

>> drivers/dma-buf/dma-buf.c:346:19: error: passing 'const void btf_type_tag(user)*' (aka 'const void *') to parameter of type 'void btf_type_tag(user)*' (aka 'void *') discards qualifiers [-Werror,-Wincompatible-pointer-types-discards-qualifiers]
if (copy_to_user(uarg, &arg, sizeof(arg)))
^~~~
include/linux/uaccess.h:157:27: note: passing argument to parameter 'to' here
copy_to_user(void __user *to, const void *from, unsigned long n)
^
1 error generated.


vim +346 drivers/dma-buf/dma-buf.c

328
329 static long dma_buf_info(struct dma_buf *dmabuf, const void __user *uarg)
330 {
331 struct dma_buf_info arg;
332
333 if (copy_from_user(&arg, uarg, sizeof(arg)))
334 return -EFAULT;
335
336 switch (arg.param) {
337 case DMA_BUF_INFO_VM_PROT:
338 if (!dmabuf->ops->mmap_info)
339 return -ENOSYS;
340 arg.value = dmabuf->ops->mmap_info(dmabuf);
341 break;
342 default:
343 return -EINVAL;
344 }
345
> 346 if (copy_to_user(uarg, &arg, sizeof(arg)))
347 return -EFAULT;
348
349 return 0;
350 }
351

--
0-DAY CI Kernel Test Service
https://01.org/lkp

2022-08-07 16:50:55

by Christian König

[permalink] [raw]
Subject: Re: [Linaro-mm-sig] [PATCH 1/3] dma-buf: Add ioctl to query mmap info

Am 29.07.22 um 19:07 schrieb Rob Clark:
> From: Rob Clark <[email protected]>
>
> 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?

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.

@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 <[email protected]>
> ---
> 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

2022-08-07 17:14:46

by Rob Clark

[permalink] [raw]
Subject: Re: [Linaro-mm-sig] [PATCH 1/3] dma-buf: Add ioctl to query mmap info

On Sun, Aug 7, 2022 at 9:09 AM Christian König
<[email protected]> wrote:
>
> Am 29.07.22 um 19:07 schrieb Rob Clark:
> > From: Rob Clark <[email protected]>
> >
> > 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 <[email protected]>
> > ---
> > 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
>

2022-08-07 17:24:35

by Christian König

[permalink] [raw]
Subject: Re: [Linaro-mm-sig] [PATCH 1/3] dma-buf: Add ioctl to query mmap info

Am 07.08.22 um 19:02 schrieb Rob Clark:
> On Sun, Aug 7, 2022 at 9:09 AM Christian König
> <[email protected]> wrote:
>> Am 29.07.22 um 19:07 schrieb Rob Clark:
>>> From: Rob Clark <[email protected]>
>>>
>>> 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 <[email protected]>
>>> ---
>>> 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

2022-08-07 17:55:24

by Rob Clark

[permalink] [raw]
Subject: Re: [Linaro-mm-sig] [PATCH 1/3] dma-buf: Add ioctl to query mmap info

On Sun, Aug 7, 2022 at 10:14 AM Christian König
<[email protected]> wrote:
>
> Am 07.08.22 um 19:02 schrieb Rob Clark:
> > On Sun, Aug 7, 2022 at 9:09 AM Christian König
> > <[email protected]> wrote:
> >> Am 29.07.22 um 19:07 schrieb Rob Clark:
> >>> From: Rob Clark <[email protected]>
> >>>
> >>> 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.

except the exporting driver is in the host ;-)

> 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?

There are multiple levels to this, but in short mapping to guest
userspace happens via drm/virtio (aka "virtio_gpu" or "virtgpu"), the
cache attributes are set via "map_info" attribute returned from the
host VMM (host userspace, hence the need for this ioctl).

In the host, the host kernel driver mmaps to host userspace (VMM).
Here the exporting driver is performing the mmap with correct cache
attributes. The VMM uses KVM to map these pages into the guest so
they appear as physical pages to the guest kernel. The guest kernel
(virtgpu) in turn maps them to guest userspace.

BR,
-R

>
> 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 <[email protected]>
> >>> ---
> >>> 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
>

2022-08-07 22:01:27

by Akhil P Oommen

[permalink] [raw]
Subject: Re: [Freedreno] [PATCH 1/3] dma-buf: Add ioctl to query mmap info

On 7/29/2022 10:37 PM, Rob Clark wrote:
> From: Rob Clark <[email protected]>
>
> 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 <[email protected]>
> ---
> 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:
Is there a typo here? BM -> VM ?

-Akhil.
> + * 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

2022-08-08 13:28:53

by Rob Clark

[permalink] [raw]
Subject: Re: [Freedreno] [PATCH 1/3] dma-buf: Add ioctl to query mmap info

On Sun, Aug 7, 2022 at 1:25 PM Akhil P Oommen <[email protected]> wrote:
>
> On 7/29/2022 10:37 PM, Rob Clark wrote:
> > From: Rob Clark <[email protected]>
> >
> > 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 <[email protected]>
> > ---
> > 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:
> Is there a typo here? BM -> VM ?

yes, fixed locally

>
> -Akhil.
> > + * 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
>