- Linaro-mm-sig - lists.linaro.org

Re: [PATCH v3 3/4] udmabuf: Implement udmabuf rw_file callback

by kernel test robot

Hi wangtao, kernel test robot noticed the following build errors: [auto build test ERROR on brauner-vfs/vfs.all] [also build test ERROR on next-20250530] [cannot apply to linus/master v6.15] [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/wangtao/fs-allow-cross-FS-co… base: https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git vfs.all patch link: https://lore.kernel.org/r/20250530103941.11092-4-tao.wangtao%40honor.com patch subject: [PATCH v3 3/4] udmabuf: Implement udmabuf rw_file callback config: sparc64-randconfig-002-20250530 (https://download.01.org/0day-ci/archive/20250530/202505302235.mDzENMSm-lkp@…) compiler: sparc64-linux-gcc (GCC) 15.1.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250530/202505302235.mDzENMSm-lkp@…) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp(a)intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202505302235.mDzENMSm-lkp@intel.com/ All error/warnings (new ones prefixed by >>): drivers/dma-buf/udmabuf.c: In function 'udmabuf_rw_file': >> drivers/dma-buf/udmabuf.c:298:25: error: storage size of 'iter' isn't known 298 | struct iov_iter iter; | ^~~~ >> drivers/dma-buf/udmabuf.c:299:45: error: 'ITER_SOURCE' undeclared (first use in this function) 299 | unsigned int direction = is_write ? ITER_SOURCE : ITER_DEST; | ^~~~~~~~~~~ drivers/dma-buf/udmabuf.c:299:45: note: each undeclared identifier is reported only once for each function it appears in >> drivers/dma-buf/udmabuf.c:299:59: error: 'ITER_DEST' undeclared (first use in this function) 299 | unsigned int direction = is_write ? ITER_SOURCE : ITER_DEST; | ^~~~~~~~~ >> drivers/dma-buf/udmabuf.c:327:17: error: implicit declaration of function 'iov_iter_bvec'; did you mean 'bvec_iter_bvec'? [-Wimplicit-function-declaration] 327 | iov_iter_bvec(&iter, direction, bvec, bv_idx, bv_total); | ^~~~~~~~~~~~~ | bvec_iter_bvec >> drivers/dma-buf/udmabuf.c:298:25: warning: unused variable 'iter' [-Wunused-variable] 298 | struct iov_iter iter; | ^~~~ vim +298 drivers/dma-buf/udmabuf.c 286 287 static ssize_t udmabuf_rw_file(struct dma_buf *dmabuf, loff_t my_pos, 288 struct file *other, loff_t pos, 289 size_t count, bool is_write) 290 { 291 struct udmabuf *ubuf = dmabuf->priv; 292 loff_t my_end = my_pos + count, bv_beg, bv_end = 0; 293 pgoff_t pg_idx = my_pos / PAGE_SIZE; 294 pgoff_t pg_end = DIV_ROUND_UP(my_end, PAGE_SIZE); 295 size_t i, bv_off, bv_len, bv_num, bv_idx = 0, bv_total = 0; 296 struct bio_vec *bvec; 297 struct kiocb kiocb; > 298 struct iov_iter iter; > 299 unsigned int direction = is_write ? ITER_SOURCE : ITER_DEST; 300 ssize_t ret = 0, rw_total = 0; 301 struct folio *folio; 302 303 bv_num = min_t(size_t, pg_end - pg_idx + 1, 1024); 304 bvec = kvcalloc(bv_num, sizeof(*bvec), GFP_KERNEL); 305 if (!bvec) 306 return -ENOMEM; 307 308 init_sync_kiocb(&kiocb, other); 309 kiocb.ki_pos = pos; 310 311 for (i = 0; i < ubuf->nr_pinned && my_pos < my_end; i++) { 312 folio = ubuf->pinned_folios[i]; 313 bv_beg = bv_end; 314 bv_end += folio_size(folio); 315 if (bv_end <= my_pos) 316 continue; 317 318 bv_len = min(bv_end, my_end) - my_pos; 319 bv_off = my_pos - bv_beg; 320 my_pos += bv_len; 321 bv_total += bv_len; 322 bvec_set_page(&bvec[bv_idx], &folio->page, bv_len, bv_off); 323 if (++bv_idx < bv_num && my_pos < my_end) 324 continue; 325 326 /* start R/W if bvec is full or count reaches zero. */ > 327 iov_iter_bvec(&iter, direction, bvec, bv_idx, bv_total); 328 if (is_write) 329 ret = other->f_op->write_iter(&kiocb, &iter); 330 else 331 ret = other->f_op->read_iter(&kiocb, &iter); 332 if (ret <= 0) 333 break; 334 rw_total += ret; 335 if (ret < bv_total || fatal_signal_pending(current)) 336 break; 337 338 bv_idx = bv_total = 0; 339 } 340 kvfree(bvec); 341 342 return rw_total > 0 ? rw_total : ret; 343 } 344 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki

1 week, 1 day

1
0
0 0

Re: [RFC PATCH 00/12] Private MMIO support for private assigned dev

by Jason Gunthorpe

On Thu, May 29, 2025 at 10:41:15PM +0800, Xu Yilun wrote: > > On AMD, the host can "revoke" at any time, at worst it'll see RMP > > events from IOMMU. Thanks, > > Is the RMP event firstly detected by host or guest? If by host, > host could fool guest by just suppress the event. Guest thought the > DMA writting is successful but it is not and may cause security issue. Is that in scope of the threat model though? Host must not be able to change DMAs or target them to different memory, but the host can stop DMA and loose it, surely? Host controls the PCI memory enable bit, doesn't it? Jason

1 week, 1 day

1
0
0 0

Re: [PATCH v5 01/10] dt-bindings: npu: rockchip,rknn: Add bindings

by Rob Herring

On Tue, May 20, 2025 at 5:27 AM Tomeu Vizoso <tomeu(a)tomeuvizoso.net> wrote: > > Add the bindings for the Neural Processing Unit IP from Rockchip. > > v2: > - Adapt to new node structure (one node per core, each with its own > IOMMU) > - Several misc. fixes from Sebastian Reichel > > v3: > - Split register block in its constituent subblocks, and only require > the ones that the kernel would ever use (Nicolas Frattaroli) > - Group supplies (Rob Herring) > - Explain the way in which the top core is special (Rob Herring) > > v4: > - Change required node name to npu@ (Rob Herring and Krzysztof Kozlowski) > - Remove unneeded items: (Krzysztof Kozlowski) > - Fix use of minItems/maxItems (Krzysztof Kozlowski) > - Add reg-names to list of required properties (Krzysztof Kozlowski) > - Fix example (Krzysztof Kozlowski) > > v5: > - Rename file to rockchip,rk3588-rknn-core.yaml (Krzysztof Kozlowski) > - Streamline compatible property (Krzysztof Kozlowski) > > Signed-off-by: Sebastian Reichel <sebastian.reichel(a)collabora.com> > Signed-off-by: Tomeu Vizoso <tomeu(a)tomeuvizoso.net> > --- > .../bindings/npu/rockchip,rk3588-rknn-core.yaml | 147 +++++++++++++++++++++ > 1 file changed, 147 insertions(+) > > diff --git a/Documentation/devicetree/bindings/npu/rockchip,rk3588-rknn-core.yaml b/Documentation/devicetree/bindings/npu/rockchip,rk3588-rknn-core.yaml > new file mode 100644 > index 0000000000000000000000000000000000000000..9eb426367afcbc03c387d43c4b8250cdd1b9ee86 > --- /dev/null > +++ b/Documentation/devicetree/bindings/npu/rockchip,rk3588-rknn-core.yaml > @@ -0,0 +1,147 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/npu/rockchip,rk3588-rknn-core.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Neural Processing Unit IP from Rockchip > + > +maintainers: > + - Tomeu Vizoso <tomeu(a)tomeuvizoso.net> > + > +description: > + Rockchip IP for accelerating inference of neural networks, based on NVIDIA's > + open source NVDLA IP. > + > + There is to be a node per each core in the NPU. In Rockchip's design there > + will be one core that is special and needs to be powered on before any of the > + other cores can be used. This special core is called the top core and should > + have the compatible string that corresponds to top cores. Is this really a distinction in the h/w? If you change which core is the top one in the DT, does it still work? > + > +properties: > + $nodename: > + pattern: '^npu@[a-f0-9]+$' > + > + compatible: > + enum: > + - rockchip,rk3588-rknn-core-top > + - rockchip,rk3588-rknn-core > + > + reg: > + maxItems: 3 > + > + reg-names: > + items: > + - const: pc > + - const: cna > + - const: core > + > + clocks: > + minItems: 2 > + maxItems: 4 > + > + clock-names: > + items: > + - const: aclk > + - const: hclk > + - const: npu > + - const: pclk > + minItems: 2 It is odd that the non-top cores only have bus clocks and no module clock. But based on the clock names, I'm guessing the aclk/hclk are not shared, but the npu and pclk are shared. Since you make the top core probe first, then it will enable the shared clocks and the non-top cores don't have to worry about them. If so, that is wrong as it is letting the software design define the bindings. Rob

1 week, 3 days

1
0
0 0

Re: [PATCH v5 09/12] tee: add Qualcomm TEE driver

by Dan Carpenter

Hi Amirreza, kernel test robot noticed the following build warnings: url: https://github.com/intel-lab-lkp/linux/commits/Amirreza-Zarrabi/tee-allow-a… base: 3be1a7a31fbda82f3604b6c31e4f390110de1b46 patch link: https://lore.kernel.org/r/20250526-qcom-tee-using-tee-ss-without-mem-obj-v5… patch subject: [PATCH v5 09/12] tee: add Qualcomm TEE driver config: x86_64-randconfig-161-20250528 (https://download.01.org/0day-ci/archive/20250528/202505280653.Y79JKqDd-lkp@…) compiler: gcc-12 (Debian 12.2.0-14) 12.2.0 If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp(a)intel.com> | Reported-by: Dan Carpenter <dan.carpenter(a)linaro.org> | Closes: https://lore.kernel.org/r/202505280653.Y79JKqDd-lkp@intel.com/ smatch warnings: drivers/tee/qcomtee/call.c:748 qcomtee_probe() warn: missing error code 'err' vim +/err +748 drivers/tee/qcomtee/call.c accd33ce59c3367 Amirreza Zarrabi 2025-05-26 711 static int qcomtee_probe(struct platform_device *pdev) accd33ce59c3367 Amirreza Zarrabi 2025-05-26 712 { accd33ce59c3367 Amirreza Zarrabi 2025-05-26 713 struct workqueue_struct *async_wq; accd33ce59c3367 Amirreza Zarrabi 2025-05-26 714 struct tee_device *teedev; accd33ce59c3367 Amirreza Zarrabi 2025-05-26 715 struct tee_shm_pool *pool; accd33ce59c3367 Amirreza Zarrabi 2025-05-26 716 struct tee_context *ctx; accd33ce59c3367 Amirreza Zarrabi 2025-05-26 717 struct qcomtee *qcomtee; accd33ce59c3367 Amirreza Zarrabi 2025-05-26 718 int err; accd33ce59c3367 Amirreza Zarrabi 2025-05-26 719 accd33ce59c3367 Amirreza Zarrabi 2025-05-26 720 qcomtee = kzalloc(sizeof(*qcomtee), GFP_KERNEL); accd33ce59c3367 Amirreza Zarrabi 2025-05-26 721 if (!qcomtee) accd33ce59c3367 Amirreza Zarrabi 2025-05-26 722 return -ENOMEM; accd33ce59c3367 Amirreza Zarrabi 2025-05-26 723 accd33ce59c3367 Amirreza Zarrabi 2025-05-26 724 pool = qcomtee_shm_pool_alloc(); accd33ce59c3367 Amirreza Zarrabi 2025-05-26 725 if (IS_ERR(pool)) { accd33ce59c3367 Amirreza Zarrabi 2025-05-26 726 err = PTR_ERR(pool); accd33ce59c3367 Amirreza Zarrabi 2025-05-26 727 accd33ce59c3367 Amirreza Zarrabi 2025-05-26 728 goto err_free_qcomtee; accd33ce59c3367 Amirreza Zarrabi 2025-05-26 729 } accd33ce59c3367 Amirreza Zarrabi 2025-05-26 730 accd33ce59c3367 Amirreza Zarrabi 2025-05-26 731 teedev = tee_device_alloc(&qcomtee_desc, NULL, pool, qcomtee); accd33ce59c3367 Amirreza Zarrabi 2025-05-26 732 if (IS_ERR(teedev)) { accd33ce59c3367 Amirreza Zarrabi 2025-05-26 733 err = PTR_ERR(teedev); accd33ce59c3367 Amirreza Zarrabi 2025-05-26 734 accd33ce59c3367 Amirreza Zarrabi 2025-05-26 735 goto err_pool_destroy; accd33ce59c3367 Amirreza Zarrabi 2025-05-26 736 } accd33ce59c3367 Amirreza Zarrabi 2025-05-26 737 accd33ce59c3367 Amirreza Zarrabi 2025-05-26 738 qcomtee->teedev = teedev; accd33ce59c3367 Amirreza Zarrabi 2025-05-26 739 qcomtee->pool = pool; accd33ce59c3367 Amirreza Zarrabi 2025-05-26 740 err = tee_device_register(qcomtee->teedev); accd33ce59c3367 Amirreza Zarrabi 2025-05-26 741 if (err) accd33ce59c3367 Amirreza Zarrabi 2025-05-26 742 goto err_unreg_teedev; accd33ce59c3367 Amirreza Zarrabi 2025-05-26 743 accd33ce59c3367 Amirreza Zarrabi 2025-05-26 744 platform_set_drvdata(pdev, qcomtee); accd33ce59c3367 Amirreza Zarrabi 2025-05-26 745 /* Start async wq. */ accd33ce59c3367 Amirreza Zarrabi 2025-05-26 746 async_wq = alloc_ordered_workqueue("qcomtee_wq", 0); accd33ce59c3367 Amirreza Zarrabi 2025-05-26 747 if (!async_wq) accd33ce59c3367 Amirreza Zarrabi 2025-05-26 @748 goto err_unreg_teedev; err = -ENOMEM; accd33ce59c3367 Amirreza Zarrabi 2025-05-26 749 accd33ce59c3367 Amirreza Zarrabi 2025-05-26 750 qcomtee->wq = async_wq; accd33ce59c3367 Amirreza Zarrabi 2025-05-26 751 /* Driver context used for async operations of teedev. */ accd33ce59c3367 Amirreza Zarrabi 2025-05-26 752 ctx = teedev_open(qcomtee->teedev); accd33ce59c3367 Amirreza Zarrabi 2025-05-26 753 if (IS_ERR(ctx)) { accd33ce59c3367 Amirreza Zarrabi 2025-05-26 754 err = PTR_ERR(ctx); accd33ce59c3367 Amirreza Zarrabi 2025-05-26 755 accd33ce59c3367 Amirreza Zarrabi 2025-05-26 756 goto err_dest_wq; accd33ce59c3367 Amirreza Zarrabi 2025-05-26 757 } accd33ce59c3367 Amirreza Zarrabi 2025-05-26 758 accd33ce59c3367 Amirreza Zarrabi 2025-05-26 759 qcomtee->ctx = ctx; accd33ce59c3367 Amirreza Zarrabi 2025-05-26 760 /* Init Object table. */ accd33ce59c3367 Amirreza Zarrabi 2025-05-26 761 qcomtee->xa_last_id = 0; accd33ce59c3367 Amirreza Zarrabi 2025-05-26 762 xa_init_flags(&qcomtee->xa_local_objects, XA_FLAGS_ALLOC); accd33ce59c3367 Amirreza Zarrabi 2025-05-26 763 /* Get QTEE verion. */ accd33ce59c3367 Amirreza Zarrabi 2025-05-26 764 qcomtee_get_qtee_feature_list(qcomtee->ctx, accd33ce59c3367 Amirreza Zarrabi 2025-05-26 765 QCOMTEE_FEATURE_VER_OP_GET_QTEE_ID, accd33ce59c3367 Amirreza Zarrabi 2025-05-26 766 &qtee_version); accd33ce59c3367 Amirreza Zarrabi 2025-05-26 767 accd33ce59c3367 Amirreza Zarrabi 2025-05-26 768 pr_info("QTEE version %u.%u.%u\n", accd33ce59c3367 Amirreza Zarrabi 2025-05-26 769 QTEE_VERSION_GET_MAJOR(qtee_version), accd33ce59c3367 Amirreza Zarrabi 2025-05-26 770 QTEE_VERSION_GET_MINOR(qtee_version), accd33ce59c3367 Amirreza Zarrabi 2025-05-26 771 QTEE_VERSION_GET_PATCH(qtee_version)); accd33ce59c3367 Amirreza Zarrabi 2025-05-26 772 accd33ce59c3367 Amirreza Zarrabi 2025-05-26 773 return 0; accd33ce59c3367 Amirreza Zarrabi 2025-05-26 774 accd33ce59c3367 Amirreza Zarrabi 2025-05-26 775 err_dest_wq: accd33ce59c3367 Amirreza Zarrabi 2025-05-26 776 destroy_workqueue(qcomtee->wq); accd33ce59c3367 Amirreza Zarrabi 2025-05-26 777 err_unreg_teedev: accd33ce59c3367 Amirreza Zarrabi 2025-05-26 778 tee_device_unregister(qcomtee->teedev); accd33ce59c3367 Amirreza Zarrabi 2025-05-26 779 err_pool_destroy: accd33ce59c3367 Amirreza Zarrabi 2025-05-26 780 tee_shm_pool_free(pool); accd33ce59c3367 Amirreza Zarrabi 2025-05-26 781 err_free_qcomtee: accd33ce59c3367 Amirreza Zarrabi 2025-05-26 782 kfree(qcomtee); accd33ce59c3367 Amirreza Zarrabi 2025-05-26 783 accd33ce59c3367 Amirreza Zarrabi 2025-05-26 784 return err; accd33ce59c3367 Amirreza Zarrabi 2025-05-26 785 } -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki

1 week, 3 days

1
0
0 0

Re: [PATCH v5 03/12] tee: add TEE_IOCTL_PARAM_ATTR_TYPE_UBUF

by kernel test robot

Hi Amirreza, kernel test robot noticed the following build warnings: [auto build test WARNING on 3be1a7a31fbda82f3604b6c31e4f390110de1b46] url: https://github.com/intel-lab-lkp/linux/commits/Amirreza-Zarrabi/tee-allow-a… base: 3be1a7a31fbda82f3604b6c31e4f390110de1b46 patch link: https://lore.kernel.org/r/20250526-qcom-tee-using-tee-ss-without-mem-obj-v5… patch subject: [PATCH v5 03/12] tee: add TEE_IOCTL_PARAM_ATTR_TYPE_UBUF config: arm64-randconfig-r121-20250527 (https://download.01.org/0day-ci/archive/20250528/202505280721.abBn0GaE-lkp@…) compiler: aarch64-linux-gcc (GCC) 8.5.0 reproduce: (https://download.01.org/0day-ci/archive/20250528/202505280721.abBn0GaE-lkp@…) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp(a)intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202505280721.abBn0GaE-lkp@intel.com/ sparse warnings: (new ones prefixed by >>) drivers/tee/tee_core.c:393:48: sparse: sparse: incorrect type in assignment (different address spaces) @@ expected void *[noderef] uaddr @@ got void [noderef] __user * @@ drivers/tee/tee_core.c:393:48: sparse: expected void *[noderef] uaddr drivers/tee/tee_core.c:393:48: sparse: got void [noderef] __user * >> drivers/tee/tee_core.c:396:56: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected void const [noderef] __user *addr @@ got void *[noderef] uaddr @@ drivers/tee/tee_core.c:396:56: sparse: expected void const [noderef] __user *addr drivers/tee/tee_core.c:396:56: sparse: got void *[noderef] uaddr drivers/tee/tee_core.c:785:41: sparse: sparse: incorrect type in assignment (different address spaces) @@ expected void *[noderef] uaddr @@ got void [noderef] __user * @@ drivers/tee/tee_core.c:785:41: sparse: expected void *[noderef] uaddr drivers/tee/tee_core.c:785:41: sparse: got void [noderef] __user * drivers/tee/tee_core.c:788:56: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected void const [noderef] __user *addr @@ got void *[noderef] uaddr @@ drivers/tee/tee_core.c:788:56: sparse: expected void const [noderef] __user *addr drivers/tee/tee_core.c:788:56: sparse: got void *[noderef] uaddr drivers/tee/tee_core.c:396:46: sparse: sparse: dereference of noderef expression drivers/tee/tee_core.c:396:46: sparse: sparse: dereference of noderef expression drivers/tee/tee_core.c:677:37: sparse: sparse: dereference of noderef expression drivers/tee/tee_core.c:788:46: sparse: sparse: dereference of noderef expression drivers/tee/tee_core.c:788:46: sparse: sparse: dereference of noderef expression vim +396 drivers/tee/tee_core.c 361 362 static int params_from_user(struct tee_context *ctx, struct tee_param *params, 363 size_t num_params, 364 struct tee_ioctl_param __user *uparams) 365 { 366 size_t n; 367 368 for (n = 0; n < num_params; n++) { 369 struct tee_shm *shm; 370 struct tee_ioctl_param ip; 371 372 if (copy_from_user(&ip, uparams + n, sizeof(ip))) 373 return -EFAULT; 374 375 /* All unused attribute bits has to be zero */ 376 if (ip.attr & ~TEE_IOCTL_PARAM_ATTR_MASK) 377 return -EINVAL; 378 379 params[n].attr = ip.attr; 380 switch (ip.attr & TEE_IOCTL_PARAM_ATTR_TYPE_MASK) { 381 case TEE_IOCTL_PARAM_ATTR_TYPE_NONE: 382 case TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_OUTPUT: 383 break; 384 case TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INPUT: 385 case TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INOUT: 386 params[n].u.value.a = ip.a; 387 params[n].u.value.b = ip.b; 388 params[n].u.value.c = ip.c; 389 break; 390 case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INPUT: 391 case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_OUTPUT: 392 case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INOUT: 393 params[n].u.ubuf.uaddr = u64_to_user_ptr(ip.a); 394 params[n].u.ubuf.size = ip.b; 395 > 396 if (!access_ok(params[n].u.ubuf.uaddr, 397 params[n].u.ubuf.size)) 398 return -EFAULT; 399 400 break; 401 case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INPUT: 402 case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT: 403 case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT: 404 /* 405 * If a NULL pointer is passed to a TA in the TEE, 406 * the ip.c IOCTL parameters is set to TEE_MEMREF_NULL 407 * indicating a NULL memory reference. 408 */ 409 if (ip.c != TEE_MEMREF_NULL) { 410 /* 411 * If we fail to get a pointer to a shared 412 * memory object (and increase the ref count) 413 * from an identifier we return an error. All 414 * pointers that has been added in params have 415 * an increased ref count. It's the callers 416 * responibility to do tee_shm_put() on all 417 * resolved pointers. 418 */ 419 shm = tee_shm_get_from_id(ctx, ip.c); 420 if (IS_ERR(shm)) 421 return PTR_ERR(shm); 422 423 /* 424 * Ensure offset + size does not overflow 425 * offset and does not overflow the size of 426 * the referred shared memory object. 427 */ 428 if ((ip.a + ip.b) < ip.a || 429 (ip.a + ip.b) > shm->size) { 430 tee_shm_put(shm); 431 return -EINVAL; 432 } 433 } else if (ctx->cap_memref_null) { 434 /* Pass NULL pointer to OP-TEE */ 435 shm = NULL; 436 } else { 437 return -EINVAL; 438 } 439 440 params[n].u.memref.shm_offs = ip.a; 441 params[n].u.memref.size = ip.b; 442 params[n].u.memref.shm = shm; 443 break; 444 default: 445 /* Unknown attribute */ 446 return -EINVAL; 447 } 448 } 449 return 0; 450 } 451 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki

1 week, 3 days

1
0
0 0

Re: [PATCH v5 09/12] tee: add Qualcomm TEE driver

by kernel test robot

Hi Amirreza, kernel test robot noticed the following build warnings: [auto build test WARNING on 3be1a7a31fbda82f3604b6c31e4f390110de1b46] url: https://github.com/intel-lab-lkp/linux/commits/Amirreza-Zarrabi/tee-allow-a… base: 3be1a7a31fbda82f3604b6c31e4f390110de1b46 patch link: https://lore.kernel.org/r/20250526-qcom-tee-using-tee-ss-without-mem-obj-v5… patch subject: [PATCH v5 09/12] tee: add Qualcomm TEE driver config: i386-randconfig-062-20250528 (https://download.01.org/0day-ci/archive/20250528/202505280538.DVSrdWK7-lkp@…) compiler: gcc-12 (Debian 12.2.0-14) 12.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250528/202505280538.DVSrdWK7-lkp@…) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp(a)intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202505280538.DVSrdWK7-lkp@intel.com/ sparse warnings: (new ones prefixed by >>) >> drivers/tee/qcomtee/call.c:227:38: sparse: sparse: incorrect type in assignment (different address spaces) @@ expected void [noderef] __user *uaddr @@ got void *[noderef] uaddr @@ drivers/tee/qcomtee/call.c:227:38: sparse: expected void [noderef] __user *uaddr drivers/tee/qcomtee/call.c:227:38: sparse: got void *[noderef] uaddr vim +227 drivers/tee/qcomtee/call.c 203 204 /** 205 * qcomtee_params_to_args() - Convert TEE parameters to QTEE arguments. 206 * @u: QTEE arguments. 207 * @params: TEE parameters. 208 * @num_params: number of elements in the parameter array. 209 * @ctx: context in which the conversion should happen. 210 * 211 * It assumes @u has at least @num_params + 1 entries and has been initialized 212 * with %QCOMTEE_ARG_TYPE_INV as &struct qcomtee_arg.type. 213 * 214 * Return: On success, returns 0; on failure, returns < 0. 215 */ 216 static int qcomtee_params_to_args(struct qcomtee_arg *u, 217 struct tee_param *params, int num_params, 218 struct tee_context *ctx) 219 { 220 int i; 221 222 for (i = 0; i < num_params; i++) { 223 switch (params[i].attr) { 224 case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INPUT: 225 case TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_OUTPUT: 226 u[i].flags = QCOMTEE_ARG_FLAGS_UADDR; > 227 u[i].b.uaddr = params[i].u.ubuf.uaddr; 228 u[i].b.size = params[i].u.ubuf.size; 229 230 if (params[i].attr == 231 TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_INPUT) 232 u[i].type = QCOMTEE_ARG_TYPE_IB; 233 else /* TEE_IOCTL_PARAM_ATTR_TYPE_UBUF_OUTPUT */ 234 u[i].type = QCOMTEE_ARG_TYPE_OB; 235 236 break; 237 case TEE_IOCTL_PARAM_ATTR_TYPE_OBJREF_INPUT: 238 u[i].type = QCOMTEE_ARG_TYPE_IO; 239 if (qcomtee_objref_to_arg(&u[i], &params[i], ctx)) 240 goto out_failed; 241 242 break; 243 case TEE_IOCTL_PARAM_ATTR_TYPE_OBJREF_OUTPUT: 244 u[i].type = QCOMTEE_ARG_TYPE_OO; 245 u[i].o = NULL_QCOMTEE_OBJECT; 246 break; 247 default: 248 goto out_failed; 249 } 250 } 251 252 return 0; 253 254 out_failed: 255 /* Undo qcomtee_objref_to_arg(). */ 256 for (i--; i >= 0; i--) { 257 if (u[i].type != QCOMTEE_ARG_TYPE_IO) 258 continue; 259 260 qcomtee_user_object_set_notify(u[i].o, false); 261 if (typeof_qcomtee_object(u[i].o) == QCOMTEE_OBJECT_TYPE_CB) 262 qcomtee_object_put(u[i].o); 263 264 qcomtee_object_put(u[i].o); 265 } 266 267 return -EINVAL; 268 } 269 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki

1 week, 3 days

1
0
0 0

[PATCH bpf-next v7 0/5] Replace CONFIG_DMABUF_SYSFS_STATS with BPF

by T.J. Mercier

Until CONFIG_DMABUF_SYSFS_STATS was added [1] it was only possible to perform per-buffer accounting with debugfs which is not suitable for production environments. Eventually we discovered the overhead with per-buffer sysfs file creation/removal was significantly impacting allocation and free times, and exacerbated kernfs lock contention. [2] dma_buf_stats_setup() is responsible for 39% of single-page buffer creation duration, or 74% of single-page dma_buf_export() duration when stressing dmabuf allocations and frees. I prototyped a change from per-buffer to per-exporter statistics with a RCU protected list of exporter allocations that accommodates most (but not all) of our use-cases and avoids almost all of the sysfs overhead. While that adds less overhead than per-buffer sysfs, and less even than the maintenance of the dmabuf debugfs_list, it's still *additional* overhead on top of the debugfs_list and doesn't give us per-buffer info. This series uses the existing dmabuf debugfs_list to implement a BPF dmabuf iterator, which adds no overhead to buffer allocation/free and provides per-buffer info. The list has been moved outside of CONFIG_DEBUG_FS scope so that it is always populated. The BPF program loaded by userspace that extracts per-buffer information gets to define its own interface which avoids the lack of ABI stability with debugfs. This will allow us to replace our use of CONFIG_DMABUF_SYSFS_STATS, and the plan is to remove it from the kernel after the next longterm stable release. [1] https://lore.kernel.org/linux-media/20201210044400.1080308-1-hridya@google.… [2] https://lore.kernel.org/all/20220516171315.2400578-1-tjmercier@google.com v1: https://lore.kernel.org/all/20250414225227.3642618-1-tjmercier@google.com v1 -> v2: Make the DMA buffer list independent of CONFIG_DEBUG_FS per Christian König Add CONFIG_DMA_SHARED_BUFFER check to kernel/bpf/Makefile per kernel test robot Use BTF_ID_LIST_SINGLE instead of BTF_ID_LIST_GLOBAL_SINGLE per Song Liu Fixup comment style, mixing code/declarations, and use ASSERT_OK_FD in selftest per Song Liu Add BPF_ITER_RESCHED feature to bpf_dmabuf_reg_info per Alexei Starovoitov Add open-coded iterator and selftest per Alexei Starovoitov Add a second test buffer from the system dmabuf heap to selftests Use the BPF program we'll use in production for selftest per Alexei Starovoitov https://r.android.com/c/platform/system/bpfprogs/+/3616123/2/dmabufIter.c https://r.android.com/c/platform/system/memory/libmeminfo/+/3614259/1/libdm… v2: https://lore.kernel.org/all/20250504224149.1033867-1-tjmercier@google.com v2 -> v3: Rebase onto bpf-next/master Move get_next_dmabuf() into drivers/dma-buf/dma-buf.c, along with the new get_first_dmabuf(). This avoids having to expose the dmabuf list and mutex to the rest of the kernel, and keeps the dmabuf mutex operations near each other in the same file. (Christian König) Add Christian's RB to dma-buf: Rename debugfs symbols Drop RFC: dma-buf: Remove DMA-BUF statistics v3: https://lore.kernel.org/all/20250507001036.2278781-1-tjmercier@google.com v3 -> v4: Fix selftest BPF program comment style (not kdoc) per Alexei Starovoitov Fix dma-buf.c kdoc comment style per Alexei Starovoitov Rename get_first_dmabuf / get_next_dmabuf to dma_buf_iter_begin / dma_buf_iter_next per Christian König Add Christian's RB to bpf: Add dmabuf iterator v4: https://lore.kernel.org/all/20250508182025.2961555-1-tjmercier@google.com v4 -> v5: Add Christian's Acks to all patches Add Song Liu's Acks Move BTF_ID_LIST_SINGLE and DEFINE_BPF_ITER_FUNC closer to usage per Song Liu Fix open-coded iterator comment style per Song Liu Move iterator termination check to its own subtest per Song Liu Rework selftest buffer creation per Song Liu Fix spacing in sanitize_string per BPF CI v5: https://lore.kernel.org/all/20250512174036.266796-1-tjmercier@google.com v5 -> v6: Song Liu: Init test buffer FDs to -1 Zero-init udmabuf_create for future proofing Bail early for iterator fd/FILE creation failure Dereference char ptr to check for NUL in sanitize_string() Move map insertion from create_test_buffers() to test_dmabuf_iter() Add ACK to selftests/bpf: Add test for open coded dmabuf_iter v6: https://lore.kernel.org/all/20250513163601.812317-1-tjmercier@google.com v6 -> v7: Zero uninitialized name bytes following the end of name strings per s390x BPF CI Reorder sanitize_string bounds checks per Song Liu Add Song's Ack to: selftests/bpf: Add test for dmabuf_iter Rebase onto bpf-next/master per BPF CI T.J. Mercier (5): dma-buf: Rename debugfs symbols bpf: Add dmabuf iterator bpf: Add open coded dmabuf iterator selftests/bpf: Add test for dmabuf_iter selftests/bpf: Add test for open coded dmabuf_iter drivers/dma-buf/dma-buf.c | 98 ++++-- include/linux/dma-buf.h | 4 +- kernel/bpf/Makefile | 3 + kernel/bpf/dmabuf_iter.c | 150 +++++++++ kernel/bpf/helpers.c | 5 + .../testing/selftests/bpf/bpf_experimental.h | 5 + tools/testing/selftests/bpf/config | 3 + .../selftests/bpf/prog_tests/dmabuf_iter.c | 285 ++++++++++++++++++ .../testing/selftests/bpf/progs/dmabuf_iter.c | 101 +++++++ 9 files changed, 632 insertions(+), 22 deletions(-) create mode 100644 kernel/bpf/dmabuf_iter.c create mode 100644 tools/testing/selftests/bpf/prog_tests/dmabuf_iter.c create mode 100644 tools/testing/selftests/bpf/progs/dmabuf_iter.c base-commit: 6888a036cfc3d617d0843ecc9bd8504e91fb9de6 -- 2.49.0.1151.ga128411c76-goog

1 week, 3 days

2
6
0 0

[PATCH 6.12 626/626] drm/gem: Internally test import_attach for imported objects

by Greg Kroah-Hartman

6.12-stable review patch. If anyone has any objections, please let me know. ------------------ From: Thomas Zimmermann <tzimmermann(a)suse.de> commit 8260731ccad0451207b45844bb66eb161a209218 upstream. Test struct drm_gem_object.import_attach to detect imported objects. During object clenanup, the dma_buf field might be NULL. Testing it in an object's free callback then incorrectly does a cleanup as for native objects. Happens for calls to drm_mode_destroy_dumb_ioctl() that clears the dma_buf field in drm_gem_object_exported_dma_buf_free(). v3: - only test for import_attach (Boris) v2: - use import_attach.dmabuf instead of dma_buf (Christian) Signed-off-by: Thomas Zimmermann <tzimmermann(a)suse.de> Fixes: b57aa47d39e9 ("drm/gem: Test for imported GEM buffers with helper") Reported-by: Andy Yan <andyshrk(a)163.com> Closes: https://lore.kernel.org/dri-devel/38d09d34.4354.196379aa560.Coremail.andysh… Tested-by: Andy Yan <andyshrk(a)163.com> Cc: Thomas Zimmermann <tzimmermann(a)suse.de> Cc: Anusha Srivatsa <asrivats(a)redhat.com> Cc: Christian König <christian.koenig(a)amd.com> Cc: Maarten Lankhorst <maarten.lankhorst(a)linux.intel.com> Cc: Maxime Ripard <mripard(a)kernel.org> Cc: David Airlie <airlied(a)gmail.com> Cc: Simona Vetter <simona(a)ffwll.ch> Cc: Sumit Semwal <sumit.semwal(a)linaro.org> Cc: "Christian König" <christian.koenig(a)amd.com> Cc: dri-devel(a)lists.freedesktop.org Cc: linux-media(a)vger.kernel.org Cc: linaro-mm-sig(a)lists.linaro.org Reviewed-by: Boris Brezillon <boris.brezillon(a)collabora.com> Reviewed-by: Simona Vetter <simona.vetter(a)ffwll.ch> Link: https://lore.kernel.org/r/20250416065820.26076-1-tzimmermann@suse.de Signed-off-by: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org> --- include/drm/drm_gem.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) --- a/include/drm/drm_gem.h +++ b/include/drm/drm_gem.h @@ -580,8 +580,7 @@ static inline bool drm_gem_object_is_sha */ static inline bool drm_gem_is_imported(const struct drm_gem_object *obj) { - /* The dma-buf's priv field points to the original GEM object. */ - return obj->dma_buf && (obj->dma_buf->priv != obj); + return !!obj->import_attach; } #ifdef CONFIG_LOCKDEP

1 week, 3 days

1
0
0 0

[PATCH 6.14 635/783] drm/gem: Internally test import_attach for imported objects

by Greg Kroah-Hartman

6.14-stable review patch. If anyone has any objections, please let me know. ------------------ From: Thomas Zimmermann <tzimmermann(a)suse.de> [ Upstream commit 8260731ccad0451207b45844bb66eb161a209218 ] Test struct drm_gem_object.import_attach to detect imported objects. During object clenanup, the dma_buf field might be NULL. Testing it in an object's free callback then incorrectly does a cleanup as for native objects. Happens for calls to drm_mode_destroy_dumb_ioctl() that clears the dma_buf field in drm_gem_object_exported_dma_buf_free(). v3: - only test for import_attach (Boris) v2: - use import_attach.dmabuf instead of dma_buf (Christian) Signed-off-by: Thomas Zimmermann <tzimmermann(a)suse.de> Fixes: b57aa47d39e9 ("drm/gem: Test for imported GEM buffers with helper") Reported-by: Andy Yan <andyshrk(a)163.com> Closes: https://lore.kernel.org/dri-devel/38d09d34.4354.196379aa560.Coremail.andysh… Tested-by: Andy Yan <andyshrk(a)163.com> Cc: Thomas Zimmermann <tzimmermann(a)suse.de> Cc: Anusha Srivatsa <asrivats(a)redhat.com> Cc: Christian König <christian.koenig(a)amd.com> Cc: Maarten Lankhorst <maarten.lankhorst(a)linux.intel.com> Cc: Maxime Ripard <mripard(a)kernel.org> Cc: David Airlie <airlied(a)gmail.com> Cc: Simona Vetter <simona(a)ffwll.ch> Cc: Sumit Semwal <sumit.semwal(a)linaro.org> Cc: "Christian König" <christian.koenig(a)amd.com> Cc: dri-devel(a)lists.freedesktop.org Cc: linux-media(a)vger.kernel.org Cc: linaro-mm-sig(a)lists.linaro.org Reviewed-by: Boris Brezillon <boris.brezillon(a)collabora.com> Reviewed-by: Simona Vetter <simona.vetter(a)ffwll.ch> Link: https://lore.kernel.org/r/20250416065820.26076-1-tzimmermann@suse.de Signed-off-by: Sasha Levin <sashal(a)kernel.org> --- include/drm/drm_gem.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h index 2bf893eabb4b2..bcd54020d6ba5 100644 --- a/include/drm/drm_gem.h +++ b/include/drm/drm_gem.h @@ -585,8 +585,7 @@ static inline bool drm_gem_object_is_shared_for_memory_stats(struct drm_gem_obje */ static inline bool drm_gem_is_imported(const struct drm_gem_object *obj) { - /* The dma-buf's priv field points to the original GEM object. */ - return obj->dma_buf && (obj->dma_buf->priv != obj); + return !!obj->import_attach; } #ifdef CONFIG_LOCKDEP -- 2.39.5

1 week, 3 days

1
0
0 0

Patch "drm/gem: Internally test import_attach for imported objects" has been added to the 6.6-stable tree

by gregkh＠linuxfoundation.org

This is a note to let you know that I've just added the patch titled drm/gem: Internally test import_attach for imported objects to the 6.6-stable tree which can be found at: http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=sum… The filename of the patch is: drm-gem-internally-test-import_attach-for-imported-objects.patch and it can be found in the queue-6.6 subdirectory. If you, or anyone else, feels it should not be added to the stable tree, please let <stable(a)vger.kernel.org> know about it. From 8260731ccad0451207b45844bb66eb161a209218 Mon Sep 17 00:00:00 2001 From: Thomas Zimmermann <tzimmermann(a)suse.de> Date: Wed, 16 Apr 2025 08:57:45 +0200 Subject: drm/gem: Internally test import_attach for imported objects MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit From: Thomas Zimmermann <tzimmermann(a)suse.de> commit 8260731ccad0451207b45844bb66eb161a209218 upstream. Test struct drm_gem_object.import_attach to detect imported objects. During object clenanup, the dma_buf field might be NULL. Testing it in an object's free callback then incorrectly does a cleanup as for native objects. Happens for calls to drm_mode_destroy_dumb_ioctl() that clears the dma_buf field in drm_gem_object_exported_dma_buf_free(). v3: - only test for import_attach (Boris) v2: - use import_attach.dmabuf instead of dma_buf (Christian) Signed-off-by: Thomas Zimmermann <tzimmermann(a)suse.de> Fixes: b57aa47d39e9 ("drm/gem: Test for imported GEM buffers with helper") Reported-by: Andy Yan <andyshrk(a)163.com> Closes: https://lore.kernel.org/dri-devel/38d09d34.4354.196379aa560.Coremail.andysh… Tested-by: Andy Yan <andyshrk(a)163.com> Cc: Thomas Zimmermann <tzimmermann(a)suse.de> Cc: Anusha Srivatsa <asrivats(a)redhat.com> Cc: Christian König <christian.koenig(a)amd.com> Cc: Maarten Lankhorst <maarten.lankhorst(a)linux.intel.com> Cc: Maxime Ripard <mripard(a)kernel.org> Cc: David Airlie <airlied(a)gmail.com> Cc: Simona Vetter <simona(a)ffwll.ch> Cc: Sumit Semwal <sumit.semwal(a)linaro.org> Cc: "Christian König" <christian.koenig(a)amd.com> Cc: dri-devel(a)lists.freedesktop.org Cc: linux-media(a)vger.kernel.org Cc: linaro-mm-sig(a)lists.linaro.org Reviewed-by: Boris Brezillon <boris.brezillon(a)collabora.com> Reviewed-by: Simona Vetter <simona.vetter(a)ffwll.ch> Link: https://lore.kernel.org/r/20250416065820.26076-1-tzimmermann@suse.de Signed-off-by: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org> --- include/drm/drm_gem.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) --- a/include/drm/drm_gem.h +++ b/include/drm/drm_gem.h @@ -567,8 +567,7 @@ static inline bool drm_gem_object_is_sha */ static inline bool drm_gem_is_imported(const struct drm_gem_object *obj) { - /* The dma-buf's priv field points to the original GEM object. */ - return obj->dma_buf && (obj->dma_buf->priv != obj); + return !!obj->import_attach; } #ifdef CONFIG_LOCKDEP Patches currently in stable-queue which might be from tzimmermann(a)suse.de are queue-6.6/drm-gem-internally-test-import_attach-for-imported-objects.patch queue-6.6/drm-ast-find-vbios-mode-from-regular-display-size.patch queue-6.6/drm-gem-test-for-imported-gem-buffers-with-helper.patch queue-6.6/drm-atomic-clarify-the-rules-around-drm_atomic_state.patch

1 week, 3 days

1
0
0 0

Patch "drm/gem: Internally test import_attach for imported objects" has been added to the 6.12-stable tree

by gregkh＠linuxfoundation.org

This is a note to let you know that I've just added the patch titled drm/gem: Internally test import_attach for imported objects to the 6.12-stable tree which can be found at: http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=sum… The filename of the patch is: drm-gem-internally-test-import_attach-for-imported-objects.patch and it can be found in the queue-6.12 subdirectory. If you, or anyone else, feels it should not be added to the stable tree, please let <stable(a)vger.kernel.org> know about it. From 8260731ccad0451207b45844bb66eb161a209218 Mon Sep 17 00:00:00 2001 From: Thomas Zimmermann <tzimmermann(a)suse.de> Date: Wed, 16 Apr 2025 08:57:45 +0200 Subject: drm/gem: Internally test import_attach for imported objects MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit From: Thomas Zimmermann <tzimmermann(a)suse.de> commit 8260731ccad0451207b45844bb66eb161a209218 upstream. Test struct drm_gem_object.import_attach to detect imported objects. During object clenanup, the dma_buf field might be NULL. Testing it in an object's free callback then incorrectly does a cleanup as for native objects. Happens for calls to drm_mode_destroy_dumb_ioctl() that clears the dma_buf field in drm_gem_object_exported_dma_buf_free(). v3: - only test for import_attach (Boris) v2: - use import_attach.dmabuf instead of dma_buf (Christian) Signed-off-by: Thomas Zimmermann <tzimmermann(a)suse.de> Fixes: b57aa47d39e9 ("drm/gem: Test for imported GEM buffers with helper") Reported-by: Andy Yan <andyshrk(a)163.com> Closes: https://lore.kernel.org/dri-devel/38d09d34.4354.196379aa560.Coremail.andysh… Tested-by: Andy Yan <andyshrk(a)163.com> Cc: Thomas Zimmermann <tzimmermann(a)suse.de> Cc: Anusha Srivatsa <asrivats(a)redhat.com> Cc: Christian König <christian.koenig(a)amd.com> Cc: Maarten Lankhorst <maarten.lankhorst(a)linux.intel.com> Cc: Maxime Ripard <mripard(a)kernel.org> Cc: David Airlie <airlied(a)gmail.com> Cc: Simona Vetter <simona(a)ffwll.ch> Cc: Sumit Semwal <sumit.semwal(a)linaro.org> Cc: "Christian König" <christian.koenig(a)amd.com> Cc: dri-devel(a)lists.freedesktop.org Cc: linux-media(a)vger.kernel.org Cc: linaro-mm-sig(a)lists.linaro.org Reviewed-by: Boris Brezillon <boris.brezillon(a)collabora.com> Reviewed-by: Simona Vetter <simona.vetter(a)ffwll.ch> Link: https://lore.kernel.org/r/20250416065820.26076-1-tzimmermann@suse.de Signed-off-by: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org> --- include/drm/drm_gem.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) --- a/include/drm/drm_gem.h +++ b/include/drm/drm_gem.h @@ -580,8 +580,7 @@ static inline bool drm_gem_object_is_sha */ static inline bool drm_gem_is_imported(const struct drm_gem_object *obj) { - /* The dma-buf's priv field points to the original GEM object. */ - return obj->dma_buf && (obj->dma_buf->priv != obj); + return !!obj->import_attach; } #ifdef CONFIG_LOCKDEP Patches currently in stable-queue which might be from tzimmermann(a)suse.de are queue-6.12/drm-gem-internally-test-import_attach-for-imported-objects.patch queue-6.12/drm-ast-find-vbios-mode-from-regular-display-size.patch queue-6.12/drm-gem-test-for-imported-gem-buffers-with-helper.patch queue-6.12/drm-atomic-clarify-the-rules-around-drm_atomic_state.patch

1 week, 3 days

1
0
0 0

Re: [PATCH 2/2] dmabuf/heaps: implement DMA_BUF_IOCTL_RW_FILE for system_heap

by Christian König

On 5/27/25 16:35, wangtao wrote: >> -----Original Message----- >> From: Christian König <christian.koenig(a)amd.com> >> Sent: Thursday, May 22, 2025 7:58 PM >> To: wangtao <tao.wangtao(a)honor.com>; T.J. Mercier >> <tjmercier(a)google.com> >> Cc: sumit.semwal(a)linaro.org; benjamin.gaignard(a)collabora.com; >> Brian.Starkey(a)arm.com; jstultz(a)google.com; linux-media(a)vger.kernel.org; >> dri-devel(a)lists.freedesktop.org; linaro-mm-sig(a)lists.linaro.org; linux- >> kernel(a)vger.kernel.org; wangbintian(BintianWang) >> <bintian.wang(a)honor.com>; yipengxiang <yipengxiang(a)honor.com>; liulu >> 00013167 <liulu.liu(a)honor.com>; hanfeng 00012985 <feng.han(a)honor.com>; >> amir73il(a)gmail.com >> Subject: Re: [PATCH 2/2] dmabuf/heaps: implement >> DMA_BUF_IOCTL_RW_FILE for system_heap >> >> On 5/22/25 10:02, wangtao wrote: >>>> -----Original Message----- >>>> From: Christian König <christian.koenig(a)amd.com> >>>> Sent: Wednesday, May 21, 2025 7:57 PM >>>> To: wangtao <tao.wangtao(a)honor.com>; T.J. Mercier >>>> <tjmercier(a)google.com> >>>> Cc: sumit.semwal(a)linaro.org; benjamin.gaignard(a)collabora.com; >>>> Brian.Starkey(a)arm.com; jstultz(a)google.com; >>>> linux-media(a)vger.kernel.org; dri-devel(a)lists.freedesktop.org; >>>> linaro-mm-sig(a)lists.linaro.org; linux- kernel(a)vger.kernel.org; >>>> wangbintian(BintianWang) <bintian.wang(a)honor.com>; yipengxiang >>>> <yipengxiang(a)honor.com>; liulu >>>> 00013167 <liulu.liu(a)honor.com>; hanfeng 00012985 >>>> <feng.han(a)honor.com>; amir73il(a)gmail.com >>>> Subject: Re: [PATCH 2/2] dmabuf/heaps: implement >>>> DMA_BUF_IOCTL_RW_FILE for system_heap >>>> >>>> On 5/21/25 12:25, wangtao wrote: >>>>> [wangtao] I previously explained that >>>>> read/sendfile/splice/copy_file_range >>>>> syscalls can't achieve dmabuf direct IO zero-copy. >>>> >>>> And why can't you work on improving those syscalls instead of >>>> creating a new IOCTL? >>>> >>> [wangtao] As I mentioned in previous emails, these syscalls cannot >>> achieve dmabuf zero-copy due to technical constraints. >> >> Yeah, and why can't you work on removing those technical constrains? >> >> What is blocking you from improving the sendfile system call or proposing a >> patch to remove the copy_file_range restrictions? > [wangtao] Since sendfile/splice can't eliminate CPU copies, I skipped cross-FS checks > in copy_file_range when copying memory/disk files. It will probably be a longer discussion, but I think that having the FS people take a look as well is clearly mandatory. If Linus or anybody else of those maintainers then say that this isn't going to fly either we can still look into alternatives. Thanks, Christian. > Will send new patches after completing shmem/udmabuf callback. > Thank you for your attention to this issue. > > UFS 4.0 device @4GB/s, Arm64 CPU @1GHz: > | Metrics |Creat(us)|Close(us)| I/O(us) |I/O(MB/s)| Vs.% > |--------------------------|---------|---------|---------|---------|------- > | 0) dmabuf buffer read | 46898 | 4804 | 1173661 | 914 | 100% > | 1) udmabuf buffer read | 593844 | 337111 | 2144681 | 500 | 54% > | 2) memfd buffer read | 1029 | 305322 | 2215859 | 484 | 52% > | 3) memfd direct read | 562 | 295239 | 1019913 | 1052 | 115% > | 4) memfd buffer sendfile | 785 | 299026 | 1431304 | 750 | 82% > | 5) memfd direct sendfile | 718 | 296307 | 2622270 | 409 | 44% > | 6) memfd buffer splice | 981 | 299694 | 1573710 | 682 | 74% > | 7) memfd direct splice | 890 | 302509 | 1269757 | 845 | 92% > | 8) memfd buffer c_f_r | 33 | 4432 | N/A | N/A | N/A > | 9) memfd direct c_f_r | 27 | 4421 | N/A | N/A | N/A > |10) memfd buffer sendfile | 595797 | 423105 | 1242494 | 864 | 94% > |11) memfd direct sendfile | 593758 | 357921 | 2344001 | 458 | 50% > |12) memfd buffer splice | 623221 | 356212 | 1117507 | 960 | 105% > |13) memfd direct splice | 587059 | 345484 | 857103 | 1252 | 136% > |14) udmabuf buffer c_f_r | 22725 | 10248 | N/A | N/A | N/A > |15) udmabuf direct c_f_r | 20120 | 9952 | N/A | N/A | N/A > |16) dmabuf buffer c_f_r | 46517 | 4708 | 857587 | 1252 | 136% > |17) dmabuf direct c_f_r | 47339 | 4661 | 284023 | 3780 | 413% > >> >> Regards, >> Christian. >> >> Could you >>> specify the technical points, code, or principles that need >>> optimization? >>> >>> Let me explain again why these syscalls can't work: >>> 1. read() syscall >>> - dmabuf fops lacks read callback implementation. Even if implemented, >>> file_fd info cannot be transferred >>> - read(file_fd, dmabuf_ptr, len) with remap_pfn_range-based mmap >>> cannot access dmabuf_buf pages, forcing buffer-mode reads >>> >>> 2. sendfile() syscall >>> - Requires CPU copy from page cache to memory file(tmpfs/shmem): >>> [DISK] --DMA--> [page cache] --CPU copy--> [MEMORY file] >>> - CPU overhead (both buffer/direct modes involve copies): >>> 55.08% do_sendfile >>> |- 55.08% do_splice_direct >>> |-|- 55.08% splice_direct_to_actor >>> |-|-|- 22.51% copy_splice_read >>> |-|-|-|- 16.57% f2fs_file_read_iter >>> |-|-|-|-|- 15.12% __iomap_dio_rw >>> |-|-|- 32.33% direct_splice_actor >>> |-|-|-|- 32.11% iter_file_splice_write >>> |-|-|-|-|- 28.42% vfs_iter_write >>> |-|-|-|-|-|- 28.42% do_iter_write >>> |-|-|-|-|-|-|- 28.39% shmem_file_write_iter >>> |-|-|-|-|-|-|-|- 24.62% generic_perform_write >>> |-|-|-|-|-|-|-|-|- 18.75% __pi_memmove >>> >>> 3. splice() requires one end to be a pipe, incompatible with regular files or >> dmabuf. >>> >>> 4. copy_file_range() >>> - Blocked by cross-FS restrictions (Amir's commit 868f9f2f8e00) >>> - Even without this restriction, Even without restrictions, implementing >>> the copy_file_range callback in dmabuf fops would only allow dmabuf >> read >>> from regular files. This is because copy_file_range relies on >>> file_out->f_op->copy_file_range, which cannot support dmabuf >> write >>> operations to regular files. >>> >>> Test results confirm these limitations: >>> T.J. Mercier's 1G from ext4 on 6.12.20 | read/sendfile (ms) w/ 3 > >>> drop_caches >>> ------------------------|------------------- >>> udmabuf buffer read | 1210 >>> udmabuf direct read | 671 >>> udmabuf buffer sendfile | 1096 >>> udmabuf direct sendfile | 2340 >>> >>> My 3GHz CPU tests (cache cleared): >>> Method | alloc | read | vs. (%) >>> ----------------------------------------------- >>> udmabuf buffer read | 135 | 546 | 180% >>> udmabuf direct read | 159 | 300 | 99% >>> udmabuf buffer sendfile | 134 | 303 | 100% >>> udmabuf direct sendfile | 141 | 912 | 301% >>> dmabuf buffer read | 22 | 362 | 119% >>> my patch direct read | 29 | 265 | 87% >>> >>> My 1GHz CPU tests (cache cleared): >>> Method | alloc | read | vs. (%) >>> ----------------------------------------------- >>> udmabuf buffer read | 552 | 2067 | 198% >>> udmabuf direct read | 540 | 627 | 60% >>> udmabuf buffer sendfile | 497 | 1045 | 100% udmabuf direct sendfile | >>> 527 | 2330 | 223% >>> dmabuf buffer read | 40 | 1111 | 106% >>> patch direct read | 44 | 310 | 30% >>> >>> Test observations align with expectations: >>> 1. dmabuf buffer read requires slow CPU copies 2. udmabuf direct read >>> achieves zero-copy but has page retrieval >>> latency from vaddr >>> 3. udmabuf buffer sendfile suffers CPU copy overhead 4. udmabuf direct >>> sendfile combines CPU copies with frequent DMA >>> operations due to small pipe buffers 5. dmabuf buffer read also >>> requires CPU copies 6. My direct read patch enables zero-copy with >>> better performance >>> on low-power CPUs >>> 7. udmabuf creation time remains problematic (as you’ve noted). >>> >>>>> My focus is enabling dmabuf direct I/O for [regular file] <--DMA--> >>>>> [dmabuf] zero-copy. >>>> >>>> Yeah and that focus is wrong. You need to work on a general solution >>>> to the issue and not specific to your problem. >>>> >>>>> Any API achieving this would work. Are there other uAPIs you think >>>>> could help? Could you recommend experts who might offer suggestions? >>>> >>>> Well once more: Either work on sendfile or copy_file_range or >>>> eventually splice to make it what you want to do. >>>> >>>> When that is done we can discuss with the VFS people if that approach >>>> is feasible. >>>> >>>> But just bypassing the VFS review by implementing a DMA-buf specific >>>> IOCTL is a NO-GO. That is clearly not something you can do in any way. >>> [wangtao] The issue is that only dmabuf lacks Direct I/O zero-copy >>> support. Tmpfs/shmem already work with Direct I/O zero-copy. As >>> explained, existing syscalls or generic methods can't enable dmabuf >>> direct I/O zero-copy, which is why I propose adding an IOCTL command. >>> >>> I respect your perspective. Could you clarify specific technical >>> aspects, code requirements, or implementation principles for modifying >>> sendfile() or copy_file_range()? This would help advance our discussion. >>> >>> Thank you for engaging in this dialogue. >>> >>>> >>>> Regards, >>>> Christian. >

1 week, 3 days

1
0
0 0

Re: [PATCH v9 8/9] optee: FF-A: dynamic protected memory allocation

by Jens Wiklander

On Mon, May 26, 2025 at 10:09 AM Sumit Garg <sumit.garg(a)kernel.org> wrote: > > On Tue, May 20, 2025 at 05:16:51PM +0200, Jens Wiklander wrote: > > Add support in the OP-TEE backend driver dynamic protected memory > > allocation with FF-A. > > > > The protected memory pools for dynamically allocated protected memory > > are instantiated when requested by user-space. This instantiation can > > fail if OP-TEE doesn't support the requested use-case of protected > > memory. > > > > Restricted memory pools based on a static carveout or dynamic allocation > > can coexist for different use-cases. We use only dynamic allocation with > > FF-A. > > > > Signed-off-by: Jens Wiklander <jens.wiklander(a)linaro.org> > > --- [...] > > +static int optee_ffa_protmem_pool_init(struct optee *optee, u32 sec_caps) > > +{ > > + enum tee_dma_heap_id id = TEE_DMA_HEAP_SECURE_VIDEO_PLAY; > > + struct tee_protmem_pool *pool; > > + int rc = 0; > > + > > + if (sec_caps & OPTEE_FFA_SEC_CAP_PROTMEM) { > > + pool = optee_protmem_alloc_dyn_pool(optee, id); > > + if (IS_ERR(pool)) > > + return PTR_ERR(pool); > > + > > + rc = tee_device_register_dma_heap(optee->teedev, id, pool); > > + if (rc) > > + pool->ops->destroy_pool(pool); > > + } > > + > > + return rc; > > +} > > + > > static int optee_ffa_probe(struct ffa_device *ffa_dev) > > { > > const struct ffa_notifier_ops *notif_ops; > > @@ -918,7 +1057,7 @@ static int optee_ffa_probe(struct ffa_device *ffa_dev) > > optee); > > if (IS_ERR(teedev)) { > > rc = PTR_ERR(teedev); > > - goto err_free_pool; > > + goto err_free_shm_pool; > > } > > optee->teedev = teedev; > > > > @@ -965,6 +1104,9 @@ static int optee_ffa_probe(struct ffa_device *ffa_dev) > > rc); > > } > > > > + if (optee_ffa_protmem_pool_init(optee, sec_caps)) > > Let's add a Kconfig check for DMABUF heaps support here as well. I prefer complaining in the log if there's something wrong with the configuration. > > > + pr_info("Protected memory service not available\n"); > > + [...] > > +static int init_dyn_protmem(struct optee_protmem_dyn_pool *rp) > > +{ > > + int rc; > > + > > + rp->protmem = tee_shm_alloc_dma_mem(rp->optee->ctx, rp->page_count); > > + if (IS_ERR(rp->protmem)) { > > + rc = PTR_ERR(rp->protmem); > > + goto err_null_protmem; > > + } > > + > > + /* > > + * TODO unmap the memory range since the physical memory will > > + * become inaccesible after the lend_protmem() call. > > Let's ellaborate this comment to also say that unmap isn't strictly > needed here in case a platform supports hypervisor in EL2 which can > perform unmapping as part for memory lending to secure world as that > will avoid any cache pre-fetch of memory lent to secure world. > > With that I can live with this as a ToDo in kernel which can be > implemented once we see platforms requiring this change to happen. OK, I'll add something. [...] > > + > > +struct tee_protmem_pool *optee_protmem_alloc_dyn_pool(struct optee *optee, > > + enum tee_dma_heap_id id) > > +{ > > + struct optee_protmem_dyn_pool *rp; > > + u32 use_case = id; > > Here we can get rid of redundant extra variable with s/id/use_case/. OK, I'll update. Cheers, Jens

1 week, 3 days

1
0
0 0

Re: [PATCH v9 9/9] optee: smc abi: dynamic protected memory allocation

by Jens Wiklander

On Mon, May 26, 2025 at 10:13 AM Sumit Garg <sumit.garg(a)kernel.org> wrote: > > On Tue, May 20, 2025 at 05:16:52PM +0200, Jens Wiklander wrote: > > Add support in the OP-TEE backend driver for dynamic protected memory > > allocation using the SMC ABI. > > > > Signed-off-by: Jens Wiklander <jens.wiklander(a)linaro.org> > > --- > > drivers/tee/optee/smc_abi.c | 102 ++++++++++++++++++++++++++++++------ > > 1 file changed, 85 insertions(+), 17 deletions(-) > > > > diff --git a/drivers/tee/optee/smc_abi.c b/drivers/tee/optee/smc_abi.c > > index f3cae8243785..6b3fbe7f0909 100644 > > --- a/drivers/tee/optee/smc_abi.c > > +++ b/drivers/tee/optee/smc_abi.c > > @@ -965,6 +965,70 @@ static int optee_smc_do_call_with_arg(struct tee_context *ctx, > > return rc; > > } > > > > +static int optee_smc_lend_protmem(struct optee *optee, struct tee_shm *protmem, > > + u16 *end_points, unsigned int ep_count, > > + u32 use_case) > > +{ > > + struct optee_shm_arg_entry *entry; > > + struct optee_msg_arg *msg_arg; > > + struct tee_shm *shm; > > + u_int offs; > > + int rc; > > + > > + msg_arg = optee_get_msg_arg(optee->ctx, 2, &entry, &shm, &offs); > > + if (IS_ERR(msg_arg)) > > + return PTR_ERR(msg_arg); > > + > > + msg_arg->cmd = OPTEE_MSG_CMD_LEND_PROTMEM; > > + msg_arg->params[0].attr = OPTEE_MSG_ATTR_TYPE_VALUE_INPUT; > > + msg_arg->params[0].u.value.a = use_case; > > + msg_arg->params[1].attr = OPTEE_MSG_ATTR_TYPE_TMEM_INPUT; > > + msg_arg->params[1].u.tmem.buf_ptr = protmem->paddr; > > + msg_arg->params[1].u.tmem.size = protmem->size; > > + msg_arg->params[1].u.tmem.shm_ref = (u_long)protmem; > > + > > + rc = optee->ops->do_call_with_arg(optee->ctx, shm, offs, false); > > + if (rc) > > + goto out; > > + if (msg_arg->ret != TEEC_SUCCESS) { > > + rc = -EINVAL; > > + goto out; > > + } > > + protmem->sec_world_id = (u_long)protmem; > > + > > +out: > > + optee_free_msg_arg(optee->ctx, entry, offs); > > + return rc; > > +} > > + > > +static int optee_smc_reclaim_protmem(struct optee *optee, > > + struct tee_shm *protmem) > > +{ > > + struct optee_shm_arg_entry *entry; > > + struct optee_msg_arg *msg_arg; > > + struct tee_shm *shm; > > + u_int offs; > > + int rc; > > + > > + msg_arg = optee_get_msg_arg(optee->ctx, 1, &entry, &shm, &offs); > > + if (IS_ERR(msg_arg)) > > + return PTR_ERR(msg_arg); > > + > > + msg_arg->cmd = OPTEE_MSG_CMD_RECLAIM_PROTMEM; > > + msg_arg->params[0].attr = OPTEE_MSG_ATTR_TYPE_RMEM_INPUT; > > + msg_arg->params[0].u.rmem.shm_ref = (u_long)protmem; > > + > > + rc = optee->ops->do_call_with_arg(optee->ctx, shm, offs, false); > > + if (rc) > > + goto out; > > + if (msg_arg->ret != TEEC_SUCCESS) > > + rc = -EINVAL; > > + > > +out: > > + optee_free_msg_arg(optee->ctx, entry, offs); > > + return rc; > > +} > > + > > /* > > * 5. Asynchronous notification > > */ > > @@ -1216,6 +1280,8 @@ static const struct optee_ops optee_ops = { > > .do_call_with_arg = optee_smc_do_call_with_arg, > > .to_msg_param = optee_to_msg_param, > > .from_msg_param = optee_from_msg_param, > > + .lend_protmem = optee_smc_lend_protmem, > > + .reclaim_protmem = optee_smc_reclaim_protmem, > > }; > > > > static int enable_async_notif(optee_invoke_fn *invoke_fn) > > @@ -1586,11 +1652,14 @@ static inline int optee_load_fw(struct platform_device *pdev, > > > > static int optee_protmem_pool_init(struct optee *optee) > > { > > + bool protm = optee->smc.sec_caps & OPTEE_SMC_SEC_CAP_PROTMEM; > > + bool dyn_protm = optee->smc.sec_caps & > > + OPTEE_SMC_SEC_CAP_DYNAMIC_PROTMEM; > > enum tee_dma_heap_id heap_id = TEE_DMA_HEAP_SECURE_VIDEO_PLAY; > > - struct tee_protmem_pool *pool; > > - int rc; > > + struct tee_protmem_pool *pool = ERR_PTR(-EINVAL); > > + int rc = -EINVAL; > > > > - if (optee->smc.sec_caps & OPTEE_SMC_SEC_CAP_PROTMEM) { > > + if (protm) { > > union { > > struct arm_smccc_res smccc; > > struct optee_smc_get_protmem_config_result result; > > @@ -1598,26 +1667,26 @@ static int optee_protmem_pool_init(struct optee *optee) > > > > optee->smc.invoke_fn(OPTEE_SMC_GET_PROTMEM_CONFIG, 0, 0, 0, 0, > > 0, 0, 0, &res.smccc); > > - if (res.result.status != OPTEE_SMC_RETURN_OK) { > > - pr_err("Secure Data Path service not available\n"); > > - return 0; > > - } > > - rc = optee_set_dma_mask(optee, res.result.pa_width); > > + if (res.result.status == OPTEE_SMC_RETURN_OK) > > + rc = optee_set_dma_mask(optee, res.result.pa_width); > > This change should be folded in patch 7/9. OK > > > if (!rc) > > pool = tee_protmem_static_pool_alloc(res.result.start, > > res.result.size); > > - if (IS_ERR(pool)) > > - return PTR_ERR(pool); > > + } > > > > + if (dyn_protm && IS_ERR(pool)) > > + pool = optee_protmem_alloc_dyn_pool(optee, heap_id); > > + > > + if (!IS_ERR(pool)) { > > rc = tee_device_register_dma_heap(optee->teedev, heap_id, pool); > > if (rc) > > - goto err; > > + pool->ops->destroy_pool(pool); > > } > > > > + if (protm || dyn_protm) > > + return rc; > > + > > return 0; > > -err: > > - pool->ops->destroy_pool(pool); > > - return rc; > > } > > > > static int optee_probe(struct platform_device *pdev) > > @@ -1788,9 +1857,8 @@ static int optee_probe(struct platform_device *pdev) > > pr_info("Asynchronous notifications enabled\n"); > > } > > > > - rc = optee_protmem_pool_init(optee); > > - if (rc) > > - goto err_notif_uninit; > > + if (optee_protmem_pool_init(optee)) > > + pr_info("Protected memory service not available\n"); > > This change can be folded in patch 7/9. Yes, that makes sense. Cheers, Jens > > Rest looks good to me. > > -Sumit > > > > > /* > > * Ensure that there are no pre-existing shm objects before enabling > > -- > > 2.43.0 > >

1 week, 3 days

1
0
0 0

Re: [PATCH v9 7/9] optee: support protected memory allocation

by Jens Wiklander

On Mon, May 26, 2025 at 9:33 AM Sumit Garg <sumit.garg(a)kernel.org> wrote: > > On Tue, May 20, 2025 at 05:16:50PM +0200, Jens Wiklander wrote: > > Add support in the OP-TEE backend driver for protected memory > > allocation. The support is limited to only the SMC ABI and for secure > > video buffers. > > > > OP-TEE is probed for the range of protected physical memory and a > > memory pool allocator is initialized if OP-TEE have support for such > > memory. > > > > Signed-off-by: Jens Wiklander <jens.wiklander(a)linaro.org> > > --- > > drivers/tee/optee/core.c | 10 +++++++ > > drivers/tee/optee/optee_private.h | 2 ++ > > drivers/tee/optee/smc_abi.c | 45 +++++++++++++++++++++++++++++-- > > 3 files changed, 55 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c > > index c75fddc83576..4b14a7ac56f9 100644 > > --- a/drivers/tee/optee/core.c > > +++ b/drivers/tee/optee/core.c > > @@ -56,6 +56,15 @@ int optee_rpmb_intf_rdev(struct notifier_block *intf, unsigned long action, > > return 0; > > } > > > > +int optee_set_dma_mask(struct optee *optee, u_int pa_width) > > +{ > > + u64 mask = DMA_BIT_MASK(min(64, pa_width)); > > + > > + optee->teedev->dev.dma_mask = &optee->teedev->dev.coherent_dma_mask; > > + > > + return dma_set_mask_and_coherent(&optee->teedev->dev, mask); > > +} > > + > > static void optee_bus_scan(struct work_struct *work) > > { > > WARN_ON(optee_enumerate_devices(PTA_CMD_GET_DEVICES_SUPP)); > > @@ -181,6 +190,7 @@ void optee_remove_common(struct optee *optee) > > tee_device_unregister(optee->supp_teedev); > > tee_device_unregister(optee->teedev); > > > > + tee_device_unregister_all_dma_heaps(optee->teedev); > > tee_shm_pool_free(optee->pool); > > optee_supp_uninit(&optee->supp); > > mutex_destroy(&optee->call_queue.mutex); > > diff --git a/drivers/tee/optee/optee_private.h b/drivers/tee/optee/optee_private.h > > index dc0f355ef72a..5e3c34802121 100644 > > --- a/drivers/tee/optee/optee_private.h > > +++ b/drivers/tee/optee/optee_private.h > > @@ -272,6 +272,8 @@ struct optee_call_ctx { > > > > extern struct blocking_notifier_head optee_rpmb_intf_added; > > > > +int optee_set_dma_mask(struct optee *optee, u_int pa_width); > > + > > int optee_notif_init(struct optee *optee, u_int max_key); > > void optee_notif_uninit(struct optee *optee); > > int optee_notif_wait(struct optee *optee, u_int key, u32 timeout); > > diff --git a/drivers/tee/optee/smc_abi.c b/drivers/tee/optee/smc_abi.c > > index f0c3ac1103bb..f3cae8243785 100644 > > --- a/drivers/tee/optee/smc_abi.c > > +++ b/drivers/tee/optee/smc_abi.c > > @@ -1584,6 +1584,42 @@ static inline int optee_load_fw(struct platform_device *pdev, > > } > > #endif > > > > +static int optee_protmem_pool_init(struct optee *optee) > > +{ > > + enum tee_dma_heap_id heap_id = TEE_DMA_HEAP_SECURE_VIDEO_PLAY; > > + struct tee_protmem_pool *pool; > > + int rc; > > + > > + if (optee->smc.sec_caps & OPTEE_SMC_SEC_CAP_PROTMEM) { > > + union { > > + struct arm_smccc_res smccc; > > + struct optee_smc_get_protmem_config_result result; > > + } res; > > + > > + optee->smc.invoke_fn(OPTEE_SMC_GET_PROTMEM_CONFIG, 0, 0, 0, 0, > > + 0, 0, 0, &res.smccc); > > + if (res.result.status != OPTEE_SMC_RETURN_OK) { > > + pr_err("Secure Data Path service not available\n"); > > + return 0; > > + } > > + rc = optee_set_dma_mask(optee, res.result.pa_width); > > + if (!rc) > > + pool = tee_protmem_static_pool_alloc(res.result.start, > > + res.result.size); > > + if (IS_ERR(pool)) > > + return PTR_ERR(pool); > > + > > + rc = tee_device_register_dma_heap(optee->teedev, heap_id, pool); > > + if (rc) > > + goto err; > > + } > > + > > + return 0; > > +err: > > + pool->ops->destroy_pool(pool); > > + return rc; > > +} > > + > > static int optee_probe(struct platform_device *pdev) > > { > > optee_invoke_fn *invoke_fn; > > @@ -1679,7 +1715,7 @@ static int optee_probe(struct platform_device *pdev) > > optee = kzalloc(sizeof(*optee), GFP_KERNEL); > > if (!optee) { > > rc = -ENOMEM; > > - goto err_free_pool; > > + goto err_free_shm_pool; > > } > > > > optee->ops = &optee_ops; > > @@ -1752,6 +1788,10 @@ static int optee_probe(struct platform_device *pdev) > > pr_info("Asynchronous notifications enabled\n"); > > } > > > > + rc = optee_protmem_pool_init(optee); > > Here we should do a Kconfig check for CONFIG_DMABUF_HEAPS so that we > don't proceed any further with initialization. Why? If OP-TEE is configured for protected memory but the kernel isn't, something isn't right, and a print could be useful. Cheers, Jens > > Rest looks good to me. > > -Sumit > > > + if (rc) > > + goto err_notif_uninit; > > + > > /* > > * Ensure that there are no pre-existing shm objects before enabling > > * the shm cache so that there's no chance of receiving an invalid > > @@ -1787,6 +1827,7 @@ static int optee_probe(struct platform_device *pdev) > > optee_disable_shm_cache(optee); > > optee_smc_notif_uninit_irq(optee); > > optee_unregister_devices(); > > + tee_device_unregister_all_dma_heaps(optee->teedev); > > err_notif_uninit: > > optee_notif_uninit(optee); > > err_close_ctx: > > @@ -1803,7 +1844,7 @@ static int optee_probe(struct platform_device *pdev) > > tee_device_unregister(optee->teedev); > > err_free_optee: > > kfree(optee); > > -err_free_pool: > > +err_free_shm_pool: > > tee_shm_pool_free(pool); > > if (memremaped_shm) > > memunmap(memremaped_shm); > > -- > > 2.43.0 > >

1 week, 3 days

1
0
0 0

Re: [PATCH v9 6/9] tee: add tee_shm_alloc_dma_mem()

by Jens Wiklander

On Mon, May 26, 2025 at 11:33 AM Sumit Garg <sumit.garg(a)kernel.org> wrote: > > On Mon, May 26, 2025 at 11:21:47AM +0200, Jens Wiklander wrote: > > On Mon, May 26, 2025 at 9:22 AM Sumit Garg <sumit.garg(a)kernel.org> wrote: > > > > > > On Tue, May 20, 2025 at 05:16:49PM +0200, Jens Wiklander wrote: > > > > Add tee_shm_alloc_dma_mem() to allocate DMA memory. The memory is > > > > represented by a tee_shm object using the new flag TEE_SHM_DMA_MEM to > > > > identify it as DMA memory. The allocated memory will later be lent to > > > > the TEE to be used as protected memory. > > > > > > > > Signed-off-by: Jens Wiklander <jens.wiklander(a)linaro.org> > > > > --- > > > > drivers/tee/tee_shm.c | 74 ++++++++++++++++++++++++++++++++++++++-- > > > > include/linux/tee_core.h | 5 +++ > > > > 2 files changed, 77 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/drivers/tee/tee_shm.c b/drivers/tee/tee_shm.c > > > > index e1ed52ee0a16..92a6a35e1a1e 100644 > > > > --- a/drivers/tee/tee_shm.c > > > > +++ b/drivers/tee/tee_shm.c > > > > @@ -5,6 +5,8 @@ > > > > #include <linux/anon_inodes.h> > > > > #include <linux/device.h> > > > > #include <linux/dma-buf.h> > > > > +#include <linux/dma-mapping.h> > > > > +#include <linux/highmem.h> > > > > #include <linux/idr.h> > > > > #include <linux/io.h> > > > > #include <linux/mm.h> > > > > @@ -13,9 +15,14 @@ > > > > #include <linux/tee_core.h> > > > > #include <linux/uaccess.h> > > > > #include <linux/uio.h> > > > > -#include <linux/highmem.h> > > > > #include "tee_private.h" > > > > > > > > +struct tee_shm_dma_mem { > > > > + struct tee_shm shm; > > > > + dma_addr_t dma_addr; > > > > + struct page *page; > > > > +}; > > > > + > > > > static void shm_put_kernel_pages(struct page **pages, size_t page_count) > > > > { > > > > size_t n; > > > > @@ -49,7 +56,14 @@ static void tee_shm_release(struct tee_device *teedev, struct tee_shm *shm) > > > > struct tee_shm *parent_shm = NULL; > > > > void *p = shm; > > > > > > > > - if (shm->flags & TEE_SHM_DMA_BUF) { > > > > + if (shm->flags & TEE_SHM_DMA_MEM) { > > > > + struct tee_shm_dma_mem *dma_mem; > > > > + > > > > + dma_mem = container_of(shm, struct tee_shm_dma_mem, shm); > > > > + p = dma_mem; > > > > + dma_free_pages(&teedev->dev, shm->size, dma_mem->page, > > > > + dma_mem->dma_addr, DMA_BIDIRECTIONAL); > > > > > > Although the kernel bot already found a randconfig issue, it looks like > > > we need to add Kconfig dependencies like HAS_DMA, DMA_CMA etc. > > > > > > Also, I was thinking if we should rather add a new TEE subsystem > > > specific Kconfig option like: TEE_DMABUF_HEAPS which can then be used to > > > select whatever dependency is needed as well as act as a gating Kconfig > > > for relevant features. > > > > You mean something like this? > > > > --- a/drivers/tee/Kconfig > > +++ b/drivers/tee/Kconfig > > @@ -13,6 +13,14 @@ menuconfig TEE > > > > if TEE > > > > +config TEE_DMABUF_HEAPS > > + bool > > + depends on HAS_DMA && DMABUF_HEAPS > > Yeah this looks fine to me but needs to be tested if DMA_CMA is a > dependency here too. Why? It can work without CMA for small allocations. > > > + > > +config TEE_STATIC_PROTMEM_POOL > > + bool > > + depends on HAS_IOMEM && TEE_DMABUF_HEAPS > > The static and dynamic protected memory pools should get auto enabled if > TEE_DMABUF_HEAPS is enabled since they are pre-requisite to provide the > protected heaps support. Something like: > > +config TEE_STATIC_PROTMEM_POOL > + bool > + default y if TEE_DMABUF_HEAPS > + depends on HAS_IOMEM Right, I'll update as needed. Cheers, Jens

1 week, 4 days

1
0
0 0

Re: [PATCH v3 3/3] dma-buf: heaps: Give default CMA heap a fixed name

by Maxime Ripard

Hi, On Thu, May 22, 2025 at 12:14:18PM -0700, Jared Kangas wrote: > The CMA heap's name in devtmpfs can vary depending on how the heap is > defined. Its name defaults to "reserved", but if a CMA area is defined > in the devicetree, the heap takes on the devicetree node's name, such as > "default-pool" or "linux,cma". To simplify naming, unconditionally name > it "default_cma_region", but keep a legacy node in place backed by the > same underlying allocator for backwards compatibility. > > Signed-off-by: Jared Kangas <jkangas(a)redhat.com> > --- > Documentation/userspace-api/dma-buf-heaps.rst | 7 +++++-- > drivers/dma-buf/heaps/Kconfig | 10 ++++++++++ > drivers/dma-buf/heaps/cma_heap.c | 20 ++++++++++++++++++- > 3 files changed, 34 insertions(+), 3 deletions(-) > > diff --git a/Documentation/userspace-api/dma-buf-heaps.rst b/Documentation/userspace-api/dma-buf-heaps.rst > index 23bd0bd7b0654..1dfe5e7acd5a3 100644 > --- a/Documentation/userspace-api/dma-buf-heaps.rst > +++ b/Documentation/userspace-api/dma-buf-heaps.rst > @@ -21,5 +21,8 @@ following heaps: > usually created either through the kernel commandline through the > ``cma`` parameter, a memory region Device-Tree node with the > ``linux,cma-default`` property set, or through the ``CMA_SIZE_MBYTES`` or > - ``CMA_SIZE_PERCENTAGE`` Kconfig options. Depending on the platform, it > - might be called ``reserved``, ``linux,cma``, or ``default-pool``. > + ``CMA_SIZE_PERCENTAGE`` Kconfig options. The heap's name in devtmpfs is > + ``default_cma_region``. For backwards compatibility, when the > + ``DMABUF_HEAPS_CMA_LEGACY`` Kconfig option is set, a duplicate node is > + created following legacy naming conventions; the legacy name might be > + ``reserved``, ``linux,cma``, or ``default-pool``. > diff --git a/drivers/dma-buf/heaps/Kconfig b/drivers/dma-buf/heaps/Kconfig > index a5eef06c42264..bb369b38b001a 100644 > --- a/drivers/dma-buf/heaps/Kconfig > +++ b/drivers/dma-buf/heaps/Kconfig > @@ -12,3 +12,13 @@ config DMABUF_HEAPS_CMA > Choose this option to enable dma-buf CMA heap. This heap is backed > by the Contiguous Memory Allocator (CMA). If your system has these > regions, you should say Y here. > + > +config DMABUF_HEAPS_CMA_LEGACY > + bool "Legacy DMA-BUF CMA Heap" > + default y > + depends on DMABUF_HEAPS_CMA > + help > + Add a duplicate CMA-backed dma-buf heap with legacy naming derived > + from the CMA area's devicetree node, or "reserved" if the area is not > + defined in the devicetree. This uses the same underlying allocator as > + CONFIG_DMABUF_HEAPS_CMA. > diff --git a/drivers/dma-buf/heaps/cma_heap.c b/drivers/dma-buf/heaps/cma_heap.c > index e998d8ccd1dc6..dfeccafc6ae3c 100644 > --- a/drivers/dma-buf/heaps/cma_heap.c > +++ b/drivers/dma-buf/heaps/cma_heap.c > @@ -9,6 +9,9 @@ > * Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com/ > * Andrew F. Davis <afd(a)ti.com> > */ > + > +#define pr_fmt(fmt) "cma_heap: " fmt > + > #include <linux/cma.h> > #include <linux/dma-buf.h> > #include <linux/dma-heap.h> > @@ -22,6 +25,7 @@ > #include <linux/slab.h> > #include <linux/vmalloc.h> > > +#define DEFAULT_CMA_NAME "default_cma_region" > > struct cma_heap { > struct dma_heap *heap; > @@ -394,15 +398,29 @@ static int __init __add_cma_heap(struct cma *cma, const char *name) > static int __init add_default_cma_heap(void) > { > struct cma *default_cma = dev_get_cma_area(NULL); > + const char *legacy_cma_name; > int ret; > > if (!default_cma) > return 0; > > - ret = __add_cma_heap(default_cma, cma_get_name(default_cma)); > + ret = __add_cma_heap(default_cma, DEFAULT_CMA_NAME); > if (ret) > return ret; > > + if (IS_ENABLED(CONFIG_DMABUF_HEAPS_CMA_LEGACY)) { > + legacy_cma_name = cma_get_name(default_cma); > + if (!strcmp(legacy_cma_name, DEFAULT_CMA_NAME)) { > + pr_warn("legacy name and default name are the same, skipping legacy heap\n"); > + return 0; > + } > + > + ret = __add_cma_heap(default_cma, legacy_cma_name); > + if (ret) > + pr_warn("failed to add legacy heap: %pe\n", > + ERR_PTR(-ret)); Are you sure about the -ret? ret should already be a negative number if it failed? With that fixed, Reviewed-by: Maxime Ripard <mripard(a)kernel.org> Maxime

1 week, 4 days

1
0
0 0

Re: [PATCH v3 2/3] dma-buf: heaps: Parameterize heap name in __add_cma_heap()

by Maxime Ripard

On Thu, 22 May 2025 12:14:17 -0700, Jared Kangas wrote: > Prepare for the introduction of a fixed-name CMA heap by replacing the > unused void pointer parameter in __add_cma_heap() with the heap name. > > Signed-off-by: Jared Kangas <jkangas(a)redhat.com> Reviewed-by: Maxime Ripard <mripard(a)kernel.org> Thanks! Maxime

1 week, 4 days

1
0
0 0

Re: [PATCH v3 1/3] Documentation: dma-buf: heaps: Fix code markup

by Maxime Ripard

On Thu, 22 May 2025 12:14:16 -0700, Jared Kangas wrote: > Code snippets should be wrapped in double backticks to follow > reStructuredText semantics; the use of single backticks uses the > :title-reference: role by default, which isn't quite what we want. > Add double backticks to code snippets to fix this. > > > [ ... ] Reviewed-by: Maxime Ripard <mripard(a)kernel.org> Thanks! Maxime

1 week, 4 days

1
0
0 0

Re: [PATCH v9 6/9] tee: add tee_shm_alloc_dma_mem()

by Jens Wiklander

On Mon, May 26, 2025 at 9:22 AM Sumit Garg <sumit.garg(a)kernel.org> wrote: > > On Tue, May 20, 2025 at 05:16:49PM +0200, Jens Wiklander wrote: > > Add tee_shm_alloc_dma_mem() to allocate DMA memory. The memory is > > represented by a tee_shm object using the new flag TEE_SHM_DMA_MEM to > > identify it as DMA memory. The allocated memory will later be lent to > > the TEE to be used as protected memory. > > > > Signed-off-by: Jens Wiklander <jens.wiklander(a)linaro.org> > > --- > > drivers/tee/tee_shm.c | 74 ++++++++++++++++++++++++++++++++++++++-- > > include/linux/tee_core.h | 5 +++ > > 2 files changed, 77 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/tee/tee_shm.c b/drivers/tee/tee_shm.c > > index e1ed52ee0a16..92a6a35e1a1e 100644 > > --- a/drivers/tee/tee_shm.c > > +++ b/drivers/tee/tee_shm.c > > @@ -5,6 +5,8 @@ > > #include <linux/anon_inodes.h> > > #include <linux/device.h> > > #include <linux/dma-buf.h> > > +#include <linux/dma-mapping.h> > > +#include <linux/highmem.h> > > #include <linux/idr.h> > > #include <linux/io.h> > > #include <linux/mm.h> > > @@ -13,9 +15,14 @@ > > #include <linux/tee_core.h> > > #include <linux/uaccess.h> > > #include <linux/uio.h> > > -#include <linux/highmem.h> > > #include "tee_private.h" > > > > +struct tee_shm_dma_mem { > > + struct tee_shm shm; > > + dma_addr_t dma_addr; > > + struct page *page; > > +}; > > + > > static void shm_put_kernel_pages(struct page **pages, size_t page_count) > > { > > size_t n; > > @@ -49,7 +56,14 @@ static void tee_shm_release(struct tee_device *teedev, struct tee_shm *shm) > > struct tee_shm *parent_shm = NULL; > > void *p = shm; > > > > - if (shm->flags & TEE_SHM_DMA_BUF) { > > + if (shm->flags & TEE_SHM_DMA_MEM) { > > + struct tee_shm_dma_mem *dma_mem; > > + > > + dma_mem = container_of(shm, struct tee_shm_dma_mem, shm); > > + p = dma_mem; > > + dma_free_pages(&teedev->dev, shm->size, dma_mem->page, > > + dma_mem->dma_addr, DMA_BIDIRECTIONAL); > > Although the kernel bot already found a randconfig issue, it looks like > we need to add Kconfig dependencies like HAS_DMA, DMA_CMA etc. > > Also, I was thinking if we should rather add a new TEE subsystem > specific Kconfig option like: TEE_DMABUF_HEAPS which can then be used to > select whatever dependency is needed as well as act as a gating Kconfig > for relevant features. You mean something like this? --- a/drivers/tee/Kconfig +++ b/drivers/tee/Kconfig @@ -13,6 +13,14 @@ menuconfig TEE if TEE +config TEE_DMABUF_HEAPS + bool + depends on HAS_DMA && DMABUF_HEAPS + +config TEE_STATIC_PROTMEM_POOL + bool + depends on HAS_IOMEM && TEE_DMABUF_HEAPS + Cheers, Jens > > -Sumit > > > + } else if (shm->flags & TEE_SHM_DMA_BUF) { > > struct tee_shm_dmabuf_ref *ref; > > > > ref = container_of(shm, struct tee_shm_dmabuf_ref, shm); > > @@ -306,6 +320,62 @@ struct tee_shm *tee_shm_alloc_priv_buf(struct tee_context *ctx, size_t size) > > } > > EXPORT_SYMBOL_GPL(tee_shm_alloc_priv_buf); > > > > +/** > > + * tee_shm_alloc_dma_mem() - Allocate DMA memory as shared memory object > > + * @ctx: Context that allocates the shared memory > > + * @page_count: Number of pages > > + * > > + * The allocated memory is expected to be lent (made inaccessible to the > > + * kernel) to the TEE while it's used and returned (accessible to the > > + * kernel again) before it's freed. > > + * > > + * This function should normally only be used internally in the TEE > > + * drivers. > > + * > > + * @returns a pointer to 'struct tee_shm' > > + */ > > +struct tee_shm *tee_shm_alloc_dma_mem(struct tee_context *ctx, > > + size_t page_count) > > +{ > > + struct tee_device *teedev = ctx->teedev; > > + struct tee_shm_dma_mem *dma_mem; > > + dma_addr_t dma_addr; > > + struct page *page; > > + > > + if (!tee_device_get(teedev)) > > + return ERR_PTR(-EINVAL); > > + > > + page = dma_alloc_pages(&teedev->dev, page_count * PAGE_SIZE, > > + &dma_addr, DMA_BIDIRECTIONAL, GFP_KERNEL); > > + if (!page) > > + goto err_put_teedev; > > + > > + dma_mem = kzalloc(sizeof(*dma_mem), GFP_KERNEL); > > + if (!dma_mem) > > + goto err_free_pages; > > + > > + refcount_set(&dma_mem->shm.refcount, 1); > > + dma_mem->shm.ctx = ctx; > > + dma_mem->shm.paddr = page_to_phys(page); > > + dma_mem->dma_addr = dma_addr; > > + dma_mem->page = page; > > + dma_mem->shm.size = page_count * PAGE_SIZE; > > + dma_mem->shm.flags = TEE_SHM_DMA_MEM; > > + > > + teedev_ctx_get(ctx); > > + > > + return &dma_mem->shm; > > + > > +err_free_pages: > > + dma_free_pages(&teedev->dev, page_count * PAGE_SIZE, page, dma_addr, > > + DMA_BIDIRECTIONAL); > > +err_put_teedev: > > + tee_device_put(teedev); > > + > > + return ERR_PTR(-ENOMEM); > > +} > > +EXPORT_SYMBOL_GPL(tee_shm_alloc_dma_mem); > > + > > int tee_dyn_shm_alloc_helper(struct tee_shm *shm, size_t size, size_t align, > > int (*shm_register)(struct tee_context *ctx, > > struct tee_shm *shm, > > diff --git a/include/linux/tee_core.h b/include/linux/tee_core.h > > index 02c07f661349..925690e1020b 100644 > > --- a/include/linux/tee_core.h > > +++ b/include/linux/tee_core.h > > @@ -29,6 +29,8 @@ > > #define TEE_SHM_POOL BIT(2) /* Memory allocated from pool */ > > #define TEE_SHM_PRIV BIT(3) /* Memory private to TEE driver */ > > #define TEE_SHM_DMA_BUF BIT(4) /* Memory with dma-buf handle */ > > +#define TEE_SHM_DMA_MEM BIT(5) /* Memory allocated with */ > > + /* dma_alloc_pages() */ > > > > #define TEE_DEVICE_FLAG_REGISTERED 0x1 > > #define TEE_MAX_DEV_NAME_LEN 32 > > @@ -310,6 +312,9 @@ void *tee_get_drvdata(struct tee_device *teedev); > > */ > > struct tee_shm *tee_shm_alloc_priv_buf(struct tee_context *ctx, size_t size); > > > > +struct tee_shm *tee_shm_alloc_dma_mem(struct tee_context *ctx, > > + size_t page_count); > > + > > int tee_dyn_shm_alloc_helper(struct tee_shm *shm, size_t size, size_t align, > > int (*shm_register)(struct tee_context *ctx, > > struct tee_shm *shm, > > -- > > 2.43.0 > >

1 week, 5 days

1
0
0 0

Re: [PATCH v9 5/9] tee: new ioctl to a register tee_shm from a dmabuf file descriptor

by Jens Wiklander

On Fri, May 23, 2025 at 3:31 PM Sumit Garg <sumit.garg(a)kernel.org> wrote: > > On Tue, May 20, 2025 at 05:16:48PM +0200, Jens Wiklander wrote: > > From: Etienne Carriere <etienne.carriere(a)foss.st.com> > > > > Add a userspace API to create a tee_shm object that refers to a dmabuf > > reference. > > > > Userspace registers the dmabuf file descriptor as in a tee_shm object. > > The registration is completed with a tee_shm returned file descriptor. > > > > Userspace is free to close the dmabuf file descriptor after it has been > > registered since all the resources are now held via the new tee_shm > > object. > > > > Closing the tee_shm file descriptor will eventually release all > > resources used by the tee_shm object when all references are released. > > > > The new IOCTL, TEE_IOC_SHM_REGISTER_FD, supports dmabuf references to > > physically contiguous memory buffers. Dmabuf references acquired from > > the TEE DMA-heap can be used as protected memory for Secure Video Path > > and such use cases. It depends on the TEE and the TEE driver if dmabuf > > references acquired by other means can be used. > > > > A new tee_shm flag is added to identify tee_shm objects built from a > > registered dmabuf, TEE_SHM_DMA_BUF. > > > > Signed-off-by: Etienne Carriere <etienne.carriere(a)foss.st.com> > > Signed-off-by: Olivier Masse <olivier.masse(a)nxp.com> > > Signed-off-by: Jens Wiklander <jens.wiklander(a)linaro.org> > > --- > > drivers/tee/tee_core.c | 63 +++++++++++++++++++++- > > drivers/tee/tee_private.h | 10 ++++ > > drivers/tee/tee_shm.c | 111 ++++++++++++++++++++++++++++++++++++-- > > include/linux/tee_core.h | 1 + > > include/linux/tee_drv.h | 10 ++++ > > include/uapi/linux/tee.h | 31 +++++++++++ > > 6 files changed, 221 insertions(+), 5 deletions(-) > > With below minor comments fixed, feel free to add: > > Reviewed-by: Sumit Garg <sumit.garg(a)oss.qualcomm.com> > > > > > diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c > > index 5259b8223c27..0e9d9e5872a4 100644 > > --- a/drivers/tee/tee_core.c > > +++ b/drivers/tee/tee_core.c > > @@ -353,11 +353,49 @@ tee_ioctl_shm_register(struct tee_context *ctx, > > return ret; > > } > > > > +static int > > +tee_ioctl_shm_register_fd(struct tee_context *ctx, > > + struct tee_ioctl_shm_register_fd_data __user *udata) > > +{ > > + struct tee_ioctl_shm_register_fd_data data; > > + struct tee_shm *shm; > > + long ret; > > + > > + if (copy_from_user(&data, udata, sizeof(data))) > > + return -EFAULT; > > + > > + /* Currently no input flags are supported */ > > + if (data.flags) > > + return -EINVAL; > > + > > + shm = tee_shm_register_fd(ctx, data.fd); > > + if (IS_ERR(shm)) > > + return -EINVAL; > > + > > + data.id = shm->id; > > + data.flags = shm->flags; > > + data.size = shm->size; > > + > > + if (copy_to_user(udata, &data, sizeof(data))) > > + ret = -EFAULT; > > + else > > + ret = tee_shm_get_fd(shm); > > + > > + /* > > + * When user space closes the file descriptor the shared memory > > + * should be freed or if tee_shm_get_fd() failed then it will > > + * be freed immediately. > > + */ > > + tee_shm_put(shm); > > + return ret; > > +} > > + > > static int param_from_user_memref(struct tee_context *ctx, > > struct tee_param_memref *memref, > > struct tee_ioctl_param *ip) > > { > > struct tee_shm *shm; > > + size_t offs = 0; > > > > /* > > * If a NULL pointer is passed to a TA in the TEE, > > @@ -388,6 +426,26 @@ static int param_from_user_memref(struct tee_context *ctx, > > tee_shm_put(shm); > > return -EINVAL; > > } > > + > > + if (shm->flags & TEE_SHM_DMA_BUF) { > > + struct tee_shm_dmabuf_ref *ref; > > + > > + ref = container_of(shm, struct tee_shm_dmabuf_ref, shm); > > + if (ref->parent_shm) { > > + /* > > + * The shm already has one reference to > > + * ref->parent_shm so we are clear of 0. > > + * We're getting another reference since > > + * this shm will be used in the parameter > > + * list instead of the shm we got with > > + * tee_shm_get_from_id() above. > > + */ > > + refcount_inc(&ref->parent_shm->refcount); > > + tee_shm_put(shm); > > + shm = ref->parent_shm; > > + offs = ref->offset; > > + } > > + } > > } else if (ctx->cap_memref_null) { > > /* Pass NULL pointer to OP-TEE */ > > shm = NULL; > > @@ -395,7 +453,7 @@ static int param_from_user_memref(struct tee_context *ctx, > > return -EINVAL; > > } > > > > - memref->shm_offs = ip->a; > > + memref->shm_offs = ip->a + offs; > > Is this an issue being detected now? If yes then shouldn't it be a > separate fix eligible for backport? No, this is to add an eventual offset for a TEE_SHM_DMA_BUF, introduced above. > > > memref->size = ip->b; > > memref->shm = shm; > > > > @@ -841,6 +899,8 @@ static long tee_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) > > return tee_ioctl_shm_alloc(ctx, uarg); > > case TEE_IOC_SHM_REGISTER: > > return tee_ioctl_shm_register(ctx, uarg); > > + case TEE_IOC_SHM_REGISTER_FD: > > + return tee_ioctl_shm_register_fd(ctx, uarg); > > case TEE_IOC_OPEN_SESSION: > > return tee_ioctl_open_session(ctx, uarg); > > case TEE_IOC_INVOKE: > > @@ -1300,3 +1360,4 @@ MODULE_AUTHOR("Linaro"); > > MODULE_DESCRIPTION("TEE Driver"); > > MODULE_VERSION("1.0"); > > MODULE_LICENSE("GPL v2"); > > +MODULE_IMPORT_NS("DMA_BUF"); > > diff --git a/drivers/tee/tee_private.h b/drivers/tee/tee_private.h > > index 6c6ff5d5eed2..308467705da6 100644 > > --- a/drivers/tee/tee_private.h > > +++ b/drivers/tee/tee_private.h > > @@ -13,6 +13,16 @@ > > #include <linux/mutex.h> > > #include <linux/types.h> > > > > +/* extra references appended to shm object for registered shared memory */ > > +struct tee_shm_dmabuf_ref { > > + struct tee_shm shm; > > + size_t offset; > > + struct dma_buf *dmabuf; > > + struct dma_buf_attachment *attach; > > + struct sg_table *sgt; > > + struct tee_shm *parent_shm; > > +}; > > + > > int tee_shm_get_fd(struct tee_shm *shm); > > > > bool tee_device_get(struct tee_device *teedev); > > diff --git a/drivers/tee/tee_shm.c b/drivers/tee/tee_shm.c > > index daf6e5cfd59a..e1ed52ee0a16 100644 > > --- a/drivers/tee/tee_shm.c > > +++ b/drivers/tee/tee_shm.c > > @@ -4,6 +4,7 @@ > > */ > > #include <linux/anon_inodes.h> > > #include <linux/device.h> > > +#include <linux/dma-buf.h> > > #include <linux/idr.h> > > #include <linux/io.h> > > #include <linux/mm.h> > > @@ -45,7 +46,23 @@ static void release_registered_pages(struct tee_shm *shm) > > > > static void tee_shm_release(struct tee_device *teedev, struct tee_shm *shm) > > { > > - if (shm->flags & TEE_SHM_POOL) { > > + struct tee_shm *parent_shm = NULL; > > + void *p = shm; > > + > > + if (shm->flags & TEE_SHM_DMA_BUF) { > > + struct tee_shm_dmabuf_ref *ref; > > + > > + ref = container_of(shm, struct tee_shm_dmabuf_ref, shm); > > + parent_shm = ref->parent_shm; > > + p = ref; > > + if (ref->attach) { > > + dma_buf_unmap_attachment(ref->attach, ref->sgt, > > + DMA_BIDIRECTIONAL); > > + > > + dma_buf_detach(ref->dmabuf, ref->attach); > > + } > > + dma_buf_put(ref->dmabuf); > > + } else if (shm->flags & TEE_SHM_POOL) { > > teedev->pool->ops->free(teedev->pool, shm); > > } else if (shm->flags & TEE_SHM_DYNAMIC) { > > int rc = teedev->desc->ops->shm_unregister(shm->ctx, shm); > > @@ -57,9 +74,10 @@ static void tee_shm_release(struct tee_device *teedev, struct tee_shm *shm) > > release_registered_pages(shm); > > } > > > > - teedev_ctx_put(shm->ctx); > > + if (shm->ctx) > > + teedev_ctx_put(shm->ctx); > > redundant change? Yes, I'll remove it. > > > > > - kfree(shm); > > + kfree(p); > > > > tee_device_put(teedev); > > } > > @@ -169,7 +187,7 @@ struct tee_shm *tee_shm_alloc_user_buf(struct tee_context *ctx, size_t size) > > * tee_client_invoke_func(). The memory allocated is later freed with a > > * call to tee_shm_free(). > > * > > - * @returns a pointer to 'struct tee_shm' > > + * @returns a pointer to 'struct tee_shm' on success, and ERR_PTR on failure > > */ > > struct tee_shm *tee_shm_alloc_kernel_buf(struct tee_context *ctx, size_t size) > > { > > @@ -179,6 +197,91 @@ struct tee_shm *tee_shm_alloc_kernel_buf(struct tee_context *ctx, size_t size) > > } > > EXPORT_SYMBOL_GPL(tee_shm_alloc_kernel_buf); > > > > +struct tee_shm *tee_shm_register_fd(struct tee_context *ctx, int fd) > > +{ > > + struct tee_shm_dmabuf_ref *ref; > > + int rc; > > + > > + if (!tee_device_get(ctx->teedev)) > > + return ERR_PTR(-EINVAL); > > + > > + teedev_ctx_get(ctx); > > + > > + ref = kzalloc(sizeof(*ref), GFP_KERNEL); > > + if (!ref) { > > + rc = -ENOMEM; > > + goto err_put_tee; > > + } > > + > > + refcount_set(&ref->shm.refcount, 1); > > + ref->shm.ctx = ctx; > > + ref->shm.id = -1; > > + ref->shm.flags = TEE_SHM_DMA_BUF; > > + > > + ref->dmabuf = dma_buf_get(fd); > > + if (IS_ERR(ref->dmabuf)) { > > + rc = PTR_ERR(ref->dmabuf); > > + goto err_kfree_ref; > > + } > > + > > + rc = tee_heap_update_from_dma_buf(ctx->teedev, ref->dmabuf, > > + &ref->offset, &ref->shm, > > + &ref->parent_shm); > > + if (!rc) > > + goto out; > > + if (rc != -EINVAL) > > + goto err_put_dmabuf; > > + > > + ref->attach = dma_buf_attach(ref->dmabuf, &ctx->teedev->dev); > > + if (IS_ERR(ref->attach)) { > > + rc = PTR_ERR(ref->attach); > > + goto err_put_dmabuf; > > + } > > + > > + ref->sgt = dma_buf_map_attachment(ref->attach, DMA_BIDIRECTIONAL); > > + if (IS_ERR(ref->sgt)) { > > + rc = PTR_ERR(ref->sgt); > > + goto err_detach; > > + } > > + > > + if (sg_nents(ref->sgt->sgl) != 1) { > > + rc = -EINVAL; > > + goto err_unmap_attachement; > > + } > > + > > + ref->shm.paddr = page_to_phys(sg_page(ref->sgt->sgl)); > > + ref->shm.size = ref->sgt->sgl->length; > > + > > +out: > > + mutex_lock(&ref->shm.ctx->teedev->mutex); > > + ref->shm.id = idr_alloc(&ref->shm.ctx->teedev->idr, &ref->shm, > > + 1, 0, GFP_KERNEL); > > + mutex_unlock(&ref->shm.ctx->teedev->mutex); > > + if (ref->shm.id < 0) { > > + rc = ref->shm.id; > > + if (ref->attach) > > + goto err_unmap_attachement; > > + goto err_put_dmabuf; > > + } > > + > > + return &ref->shm; > > + > > +err_unmap_attachement: > > + dma_buf_unmap_attachment(ref->attach, ref->sgt, DMA_BIDIRECTIONAL); > > +err_detach: > > + dma_buf_detach(ref->dmabuf, ref->attach); > > +err_put_dmabuf: > > + dma_buf_put(ref->dmabuf); > > +err_kfree_ref: > > + kfree(ref); > > +err_put_tee: > > + teedev_ctx_put(ctx); > > + tee_device_put(ctx->teedev); > > + > > + return ERR_PTR(rc); > > +} > > +EXPORT_SYMBOL_GPL(tee_shm_register_fd); > > + > > /** > > * tee_shm_alloc_priv_buf() - Allocate shared memory for a privately shared > > * kernel buffer > > diff --git a/include/linux/tee_core.h b/include/linux/tee_core.h > > index b8b99c97e00c..02c07f661349 100644 > > --- a/include/linux/tee_core.h > > +++ b/include/linux/tee_core.h > > @@ -28,6 +28,7 @@ > > #define TEE_SHM_USER_MAPPED BIT(1) /* Memory mapped in user space */ > > #define TEE_SHM_POOL BIT(2) /* Memory allocated from pool */ > > #define TEE_SHM_PRIV BIT(3) /* Memory private to TEE driver */ > > +#define TEE_SHM_DMA_BUF BIT(4) /* Memory with dma-buf handle */ > > > > #define TEE_DEVICE_FLAG_REGISTERED 0x1 > > #define TEE_MAX_DEV_NAME_LEN 32 > > diff --git a/include/linux/tee_drv.h b/include/linux/tee_drv.h > > index a54c203000ed..824f1251de60 100644 > > --- a/include/linux/tee_drv.h > > +++ b/include/linux/tee_drv.h > > @@ -116,6 +116,16 @@ struct tee_shm *tee_shm_alloc_kernel_buf(struct tee_context *ctx, size_t size); > > struct tee_shm *tee_shm_register_kernel_buf(struct tee_context *ctx, > > void *addr, size_t length); > > > > +/** > > + * tee_shm_register_fd() - Register shared memory from file descriptor > > + * > > + * @ctx: Context that allocates the shared memory > > + * @fd: Shared memory file descriptor reference > > + * > > + * @returns a pointer to 'struct tee_shm' on success, and ERR_PTR on failure > > + */ > > +struct tee_shm *tee_shm_register_fd(struct tee_context *ctx, int fd); > > + > > /** > > * tee_shm_free() - Free shared memory > > * @shm: Handle to shared memory to free > > diff --git a/include/uapi/linux/tee.h b/include/uapi/linux/tee.h > > index d0430bee8292..8ec5f46fbfbe 100644 > > --- a/include/uapi/linux/tee.h > > +++ b/include/uapi/linux/tee.h > > @@ -118,6 +118,37 @@ struct tee_ioctl_shm_alloc_data { > > #define TEE_IOC_SHM_ALLOC _IOWR(TEE_IOC_MAGIC, TEE_IOC_BASE + 1, \ > > struct tee_ioctl_shm_alloc_data) > > > > +/** > > + * struct tee_ioctl_shm_register_fd_data - Shared memory registering argument > > + * @fd: [in] File descriptor identifying dmabuf reference > > + * @size: [out] Size of referenced memory > > + * @flags: [in] Flags to/from allocation. > > + * @id: [out] Identifier of the shared memory > > + * > > + * The flags field should currently be zero as input. Updated by the call > > + * with actual flags as defined by TEE_IOCTL_SHM_* above. > > + * This structure is used as argument for TEE_IOC_SHM_REGISTER_FD below. > > + */ > > +struct tee_ioctl_shm_register_fd_data { > > + __s64 fd; > > + __u64 size; > > + __u32 flags; > > + __s32 id; > > +}; > > + > > +/** > > + * TEE_IOC_SHM_REGISTER_FD - register a shared memory from a file descriptor > > + * > > + * Returns a file descriptor on success or < 0 on failure > > + * > > + * The returned file descriptor refers to the shared memory object in the > > + * kernel. The supplied file deccriptor can be closed if it's not needed > > + * for other purposes. The shared memory is freed when the descriptor is > > + * closed. > > + */ > > +#define TEE_IOC_SHM_REGISTER_FD _IOWR(TEE_IOC_MAGIC, TEE_IOC_BASE + 8, \ > > + struct tee_ioctl_shm_register_fd_data) > > Please add this IOCTL in correct sequence order. OK, I'll fix it. Thanks, Jens > > -Sumit > > > + > > /** > > * struct tee_ioctl_buf_data - Variable sized buffer > > * @buf_ptr: [in] A __user pointer to a buffer > > -- > > 2.43.0 > >

1 week, 5 days

1
0
0 0

Re: [PATCH v9 3/9] tee: implement protected DMA-heap

by Jens Wiklander

On Fri, May 23, 2025 at 3:03 PM Sumit Garg <sumit.garg(a)kernel.org> wrote: > > + Robin > > Jens, > > I suppose you missed to add Robin who has earlier reviewed this patch. Yes, you're right, sorry. Thanks for catching this. > > On Tue, May 20, 2025 at 05:16:46PM +0200, Jens Wiklander wrote: > > Implement DMA heap for protected DMA-buf allocation in the TEE > > subsystem. > > > > Restricted memory refers to memory buffers behind a hardware enforced > > s/Restricted/Protected/ > > > firewall. It is not accessible to the kernel during normal circumstances > > but rather only accessible to certain hardware IPs or CPUs executing in > > higher or differently privileged mode than the kernel itself. This > > interface allows to allocate and manage such protected memory buffers > > via interaction with a TEE implementation. > > > > The protected memory is allocated for a specific use-case, like Secure > > Video Playback, Trusted UI, or Secure Video Recording where certain > > hardware devices can access the memory. > > > > The DMA-heaps are enabled explicitly by the TEE backend driver. The TEE > > backend drivers needs to implement protected memory pool to manage the > > protected memory. > > > > Signed-off-by: Jens Wiklander <jens.wiklander(a)linaro.org> > > --- > > drivers/tee/Makefile | 1 + > > drivers/tee/tee_heap.c | 487 ++++++++++++++++++++++++++++++++++++++ > > drivers/tee/tee_private.h | 6 + > > include/linux/tee_core.h | 65 +++++ > > 4 files changed, 559 insertions(+) > > create mode 100644 drivers/tee/tee_heap.c > > > > diff --git a/drivers/tee/Makefile b/drivers/tee/Makefile > > index 5488cba30bd2..949a6a79fb06 100644 > > --- a/drivers/tee/Makefile > > +++ b/drivers/tee/Makefile > > @@ -1,6 +1,7 @@ > > # SPDX-License-Identifier: GPL-2.0 > > obj-$(CONFIG_TEE) += tee.o > > tee-objs += tee_core.o > > +tee-objs += tee_heap.o > > tee-objs += tee_shm.o > > tee-objs += tee_shm_pool.o > > obj-$(CONFIG_OPTEE) += optee/ > > diff --git a/drivers/tee/tee_heap.c b/drivers/tee/tee_heap.c > > new file mode 100644 > > index 000000000000..a332805f9f26 > > --- /dev/null > > +++ b/drivers/tee/tee_heap.c > > @@ -0,0 +1,487 @@ > > +// SPDX-License-Identifier: GPL-2.0-only > > +/* > > + * Copyright (c) 2025, Linaro Limited > > + */ > > + > > +#include <linux/dma-buf.h> > > +#include <linux/dma-heap.h> > > +#include <linux/genalloc.h> > > +#include <linux/module.h> > > +#include <linux/scatterlist.h> > > +#include <linux/slab.h> > > +#include <linux/tee_core.h> > > +#include <linux/xarray.h> > > + > > +#include "tee_private.h" > > + > > +struct tee_dma_heap { > > + struct dma_heap *heap; > > + enum tee_dma_heap_id id; > > + struct tee_protmem_pool *pool; > > + struct tee_device *teedev; > > + /* Protects pool and teedev above */ > > + struct mutex mu; > > +}; > > + > > +struct tee_heap_buffer { > > + struct tee_protmem_pool *pool; > > + struct tee_device *teedev; > > + size_t size; > > + size_t offs; > > + struct sg_table table; > > +}; > > + > > +struct tee_heap_attachment { > > + struct sg_table table; > > + struct device *dev; > > +}; > > + > > +struct tee_protmem_static_pool { > > + struct tee_protmem_pool pool; > > + struct gen_pool *gen_pool; > > + phys_addr_t pa_base; > > + void *base; > > +}; > > + > > +#if IS_ENABLED(CONFIG_DMABUF_HEAPS) > > +static DEFINE_XARRAY_ALLOC(tee_dma_heap); > > + > > +static int copy_sg_table(struct sg_table *dst, struct sg_table *src) > > +{ > > + struct scatterlist *dst_sg; > > + struct scatterlist *src_sg; > > + int ret; > > + int i; > > + > > + ret = sg_alloc_table(dst, src->orig_nents, GFP_KERNEL); > > + if (ret) > > + return ret; > > + > > + dst_sg = dst->sgl; > > + for_each_sgtable_sg(src, src_sg, i) { > > + sg_set_page(dst_sg, sg_page(src_sg), src_sg->length, > > + src_sg->offset); > > + dst_sg = sg_next(dst_sg); > > + } > > + > > + return 0; > > +} > > + > > +static int tee_heap_attach(struct dma_buf *dmabuf, > > + struct dma_buf_attachment *attachment) > > +{ > > + struct tee_heap_buffer *buf = dmabuf->priv; > > + struct tee_heap_attachment *a; > > + int ret; > > + > > + a = kzalloc(sizeof(*a), GFP_KERNEL); > > + if (!a) > > + return -ENOMEM; > > + > > + ret = copy_sg_table(&a->table, &buf->table); > > + if (ret) { > > + kfree(a); > > + return ret; > > + } > > + > > + a->dev = attachment->dev; > > + attachment->priv = a; > > + > > + return 0; > > +} > > + > > +static void tee_heap_detach(struct dma_buf *dmabuf, > > + struct dma_buf_attachment *attachment) > > +{ > > + struct tee_heap_attachment *a = attachment->priv; > > + > > + sg_free_table(&a->table); > > + kfree(a); > > +} > > + > > +static struct sg_table * > > +tee_heap_map_dma_buf(struct dma_buf_attachment *attachment, > > + enum dma_data_direction direction) > > +{ > > + struct tee_heap_attachment *a = attachment->priv; > > + int ret; > > + > > + ret = dma_map_sgtable(attachment->dev, &a->table, direction, > > + DMA_ATTR_SKIP_CPU_SYNC); > > + if (ret) > > + return ERR_PTR(ret); > > + > > + return &a->table; > > +} > > + > > +static void tee_heap_unmap_dma_buf(struct dma_buf_attachment *attachment, > > + struct sg_table *table, > > + enum dma_data_direction direction) > > +{ > > + struct tee_heap_attachment *a = attachment->priv; > > + > > + WARN_ON(&a->table != table); > > + > > + dma_unmap_sgtable(attachment->dev, table, direction, > > + DMA_ATTR_SKIP_CPU_SYNC); > > +} > > + > > +static void tee_heap_buf_free(struct dma_buf *dmabuf) > > +{ > > + struct tee_heap_buffer *buf = dmabuf->priv; > > + struct tee_device *teedev = buf->teedev; > > + > > + buf->pool->ops->free(buf->pool, &buf->table); > > + tee_device_put(teedev); > > +} > > + > > +static const struct dma_buf_ops tee_heap_buf_ops = { > > + .attach = tee_heap_attach, > > + .detach = tee_heap_detach, > > + .map_dma_buf = tee_heap_map_dma_buf, > > + .unmap_dma_buf = tee_heap_unmap_dma_buf, > > + .release = tee_heap_buf_free, > > +}; > > + > > +static struct dma_buf *tee_dma_heap_alloc(struct dma_heap *heap, > > + unsigned long len, u32 fd_flags, > > + u64 heap_flags) > > +{ > > + struct tee_dma_heap *h = dma_heap_get_drvdata(heap); > > + DEFINE_DMA_BUF_EXPORT_INFO(exp_info); > > + struct tee_device *teedev = NULL; > > + struct tee_heap_buffer *buf; > > + struct tee_protmem_pool *pool; > > + struct dma_buf *dmabuf; > > + int rc; > > + > > + mutex_lock(&h->mu); > > + if (tee_device_get(h->teedev)) { > > + teedev = h->teedev; > > + pool = h->pool; > > + } > > + mutex_unlock(&h->mu); > > + > > + if (!teedev) > > + return ERR_PTR(-EINVAL); > > + > > + buf = kzalloc(sizeof(*buf), GFP_KERNEL); > > + if (!buf) { > > + dmabuf = ERR_PTR(-ENOMEM); > > + goto err; > > + } > > + buf->size = len; > > + buf->pool = pool; > > + buf->teedev = teedev; > > + > > + rc = pool->ops->alloc(pool, &buf->table, len, &buf->offs); > > + if (rc) { > > + dmabuf = ERR_PTR(rc); > > + goto err_kfree; > > + } > > + > > + exp_info.ops = &tee_heap_buf_ops; > > + exp_info.size = len; > > + exp_info.priv = buf; > > + exp_info.flags = fd_flags; > > + dmabuf = dma_buf_export(&exp_info); > > + if (IS_ERR(dmabuf)) > > + goto err_protmem_free; > > + > > + return dmabuf; > > + > > +err_protmem_free: > > + pool->ops->free(pool, &buf->table); > > +err_kfree: > > + kfree(buf); > > +err: > > + tee_device_put(h->teedev); > > + return dmabuf; > > +} > > + > > +static const struct dma_heap_ops tee_dma_heap_ops = { > > + .allocate = tee_dma_heap_alloc, > > +}; > > + > > +static const char *heap_id_2_name(enum tee_dma_heap_id id) > > +{ > > + switch (id) { > > + case TEE_DMA_HEAP_SECURE_VIDEO_PLAY: > > + return "protected,secure-video"; > > + case TEE_DMA_HEAP_TRUSTED_UI: > > + return "protected,trusted-ui"; > > + case TEE_DMA_HEAP_SECURE_VIDEO_RECORD: > > + return "protected,secure-video-record"; > > + default: > > + return NULL; > > + } > > +} > > + > > +static int alloc_dma_heap(struct tee_device *teedev, enum tee_dma_heap_id id, > > + struct tee_protmem_pool *pool) > > +{ > > + struct dma_heap_export_info exp_info = { > > + .ops = &tee_dma_heap_ops, > > + .name = heap_id_2_name(id), > > + }; > > + struct tee_dma_heap *h; > > + int rc; > > + > > + if (!exp_info.name) > > + return -EINVAL; > > + > > + if (xa_reserve(&tee_dma_heap, id, GFP_KERNEL)) { > > + if (!xa_load(&tee_dma_heap, id)) > > + return -EEXIST; > > + return -ENOMEM; > > + } > > + > > + h = kzalloc(sizeof(*h), GFP_KERNEL); > > + if (!h) > > + return -ENOMEM; > > + h->id = id; > > + h->teedev = teedev; > > + h->pool = pool; > > + mutex_init(&h->mu); > > + > > + exp_info.priv = h; > > + h->heap = dma_heap_add(&exp_info); > > + if (IS_ERR(h->heap)) { > > + rc = PTR_ERR(h->heap); > > + kfree(h); > > + > > + return rc; > > + } > > + > > + /* "can't fail" due to the call to xa_reserve() above */ > > + return WARN(xa_store(&tee_dma_heap, id, h, GFP_KERNEL), > > + "xa_store() failed"); > > I think this can rather be simplified to: > > return WARN_ON(xa_is_err(xa_store(&tee_dma_heap, id, h, GFP_KERNEL))); OK > > > +} > > + > > +int tee_device_register_dma_heap(struct tee_device *teedev, > > + enum tee_dma_heap_id id, > > + struct tee_protmem_pool *pool) > > +{ > > + struct tee_dma_heap *h; > > + int rc; > > + > > + h = xa_load(&tee_dma_heap, id); > > + if (h) { > > + mutex_lock(&h->mu); > > + if (h->teedev) { > > + rc = -EBUSY; > > + } else { > > + h->teedev = teedev; > > + h->pool = pool; > > + rc = 0; > > + } > > + mutex_unlock(&h->mu); > > + } else { > > + rc = alloc_dma_heap(teedev, id, pool); > > + } > > + > > + if (rc) > > + dev_err(&teedev->dev, "can't register DMA heap id %d (%s)\n", > > + id, heap_id_2_name(id)); > > + > > + return rc; > > +} > > +EXPORT_SYMBOL_GPL(tee_device_register_dma_heap); > > + > > +void tee_device_unregister_all_dma_heaps(struct tee_device *teedev) > > +{ > > + struct tee_protmem_pool *pool; > > + struct tee_dma_heap *h; > > + u_long i; > > + > > + xa_for_each(&tee_dma_heap, i, h) { > > + if (h) { > > + pool = NULL; > > + mutex_lock(&h->mu); > > + if (h->teedev == teedev) { > > + pool = h->pool; > > + h->teedev = NULL; > > + h->pool = NULL; > > + } > > + mutex_unlock(&h->mu); > > + if (pool) > > + pool->ops->destroy_pool(pool); > > + } > > + } > > +} > > +EXPORT_SYMBOL_GPL(tee_device_unregister_all_dma_heaps); > > + > > +int tee_heap_update_from_dma_buf(struct tee_device *teedev, > > + struct dma_buf *dmabuf, size_t *offset, > > + struct tee_shm *shm, > > + struct tee_shm **parent_shm) > > +{ > > + struct tee_heap_buffer *buf; > > + int rc; > > + > > + /* The DMA-buf must be from our heap */ > > + if (dmabuf->ops != &tee_heap_buf_ops) > > + return -EINVAL; > > + > > + buf = dmabuf->priv; > > + /* The buffer must be from the same teedev */ > > + if (buf->teedev != teedev) > > + return -EINVAL; > > + > > + shm->size = buf->size; > > + > > + rc = buf->pool->ops->update_shm(buf->pool, &buf->table, buf->offs, shm, > > + parent_shm); > > + if (!rc && *parent_shm) > > + *offset = buf->offs; > > + > > + return rc; > > +} > > +#else > > +int tee_device_register_dma_heap(struct tee_device *teedev __always_unused, > > + enum tee_dma_heap_id id __always_unused, > > + struct tee_protmem_pool *pool __always_unused) > > +{ > > + return -EINVAL; > > +} > > +EXPORT_SYMBOL_GPL(tee_device_register_dma_heap); > > + > > +void > > +tee_device_unregister_all_dma_heaps(struct tee_device *teedev __always_unused) > > +{ > > +} > > +EXPORT_SYMBOL_GPL(tee_device_unregister_all_dma_heaps); > > + > > +int tee_heap_update_from_dma_buf(struct tee_device *teedev __always_unused, > > + struct dma_buf *dmabuf __always_unused, > > + size_t *offset __always_unused, > > + struct tee_shm *shm __always_unused, > > + struct tee_shm **parent_shm __always_unused) > > +{ > > + return -EINVAL; > > +} > > +#endif > > + > > +static struct tee_protmem_static_pool * > > +to_protmem_static_pool(struct tee_protmem_pool *pool) > > +{ > > + return container_of(pool, struct tee_protmem_static_pool, pool); > > +} > > + > > +static int protmem_pool_op_static_alloc(struct tee_protmem_pool *pool, > > + struct sg_table *sgt, size_t size, > > + size_t *offs) > > +{ > > + struct tee_protmem_static_pool *stp = to_protmem_static_pool(pool); > > + phys_addr_t pa; > > + int ret; > > + > > + pa = gen_pool_alloc(stp->gen_pool, size); > > + if (!pa) > > + return -ENOMEM; > > + > > + ret = sg_alloc_table(sgt, 1, GFP_KERNEL); > > + if (ret) { > > + gen_pool_free(stp->gen_pool, pa, size); > > + return ret; > > + } > > + > > + sg_set_page(sgt->sgl, phys_to_page(pa), size, 0); > > Did you missed pfn_valid() check from prior v8 review comments? No, I tried it, but pfn_valid() didn't like the address, so I had to find a way to fix it. tee_protmem_static_pool_alloc(), below, calls memremap() on the range, and that should make pfn_valid() redundant. > > > + *offs = pa - stp->pa_base; > > + > > + return 0; > > +} > > + > > +static void protmem_pool_op_static_free(struct tee_protmem_pool *pool, > > + struct sg_table *sgt) > > +{ > > + struct tee_protmem_static_pool *stp = to_protmem_static_pool(pool); > > + struct scatterlist *sg; > > + int i; > > + > > + for_each_sgtable_sg(sgt, sg, i) > > + gen_pool_free(stp->gen_pool, sg_phys(sg), sg->length); > > + sg_free_table(sgt); > > +} > > + > > +static int protmem_pool_op_static_update_shm(struct tee_protmem_pool *pool, > > + struct sg_table *sgt, size_t offs, > > + struct tee_shm *shm, > > + struct tee_shm **parent_shm) > > +{ > > + struct tee_protmem_static_pool *stp = to_protmem_static_pool(pool); > > + > > + shm->paddr = stp->pa_base + offs; > > + *parent_shm = NULL; > > + > > + return 0; > > +} > > + > > +static void protmem_pool_op_static_destroy_pool(struct tee_protmem_pool *pool) > > +{ > > + struct tee_protmem_static_pool *stp = to_protmem_static_pool(pool); > > + > > + gen_pool_destroy(stp->gen_pool); > > + memunmap(stp->base); > > + kfree(stp); > > +} > > + > > +static struct tee_protmem_pool_ops protmem_pool_ops_static = { > > + .alloc = protmem_pool_op_static_alloc, > > + .free = protmem_pool_op_static_free, > > + .update_shm = protmem_pool_op_static_update_shm, > > + .destroy_pool = protmem_pool_op_static_destroy_pool, > > +}; > > + > > +struct tee_protmem_pool *tee_protmem_static_pool_alloc(phys_addr_t paddr, > > + size_t size) > > +{ > > + const size_t page_mask = PAGE_SIZE - 1; > > + struct tee_protmem_static_pool *stp; > > + int rc; > > + > > + /* Check it's page aligned */ > > + if ((paddr | size) & page_mask) > > + return ERR_PTR(-EINVAL); > > + > > + stp = kzalloc(sizeof(*stp), GFP_KERNEL); > > + if (!stp) > > + return ERR_PTR(-ENOMEM); > > + > > + /* > > + * Map the memory as uncached to make sure the kernel can work with > > + * __pfn_to_page() and friends since that's needed when passing the > > + * protected DMA-buf to a device. The memory should otherwise not > > + * be touched by the kernel since it's likely to cause an external > > + * abort due to the protection status. > > + */ > > + stp->base = memremap(paddr, size, MEMREMAP_WC); > > + if (!stp->base) { > > + rc = -EINVAL; > > + goto err_free; > > + } > > + > > + stp->gen_pool = gen_pool_create(PAGE_SHIFT, -1); > > + if (!stp->gen_pool) { > > + rc = -ENOMEM; > > + goto err_unmap; > > + } > > + > > + rc = gen_pool_add(stp->gen_pool, paddr, size, -1); > > + if (rc) > > + goto err_free_pool; > > + > > + stp->pool.ops = &protmem_pool_ops_static; > > + stp->pa_base = paddr; > > + return &stp->pool; > > + > > +err_free_pool: > > + gen_pool_destroy(stp->gen_pool); > > +err_unmap: > > + memunmap(stp->base); > > +err_free: > > + kfree(stp); > > + > > + return ERR_PTR(rc); > > +} > > +EXPORT_SYMBOL_GPL(tee_protmem_static_pool_alloc); > > diff --git a/drivers/tee/tee_private.h b/drivers/tee/tee_private.h > > index 9bc50605227c..6c6ff5d5eed2 100644 > > --- a/drivers/tee/tee_private.h > > +++ b/drivers/tee/tee_private.h > > @@ -8,6 +8,7 @@ > > #include <linux/cdev.h> > > #include <linux/completion.h> > > #include <linux/device.h> > > +#include <linux/dma-buf.h> > > #include <linux/kref.h> > > #include <linux/mutex.h> > > #include <linux/types.h> > > @@ -24,4 +25,9 @@ struct tee_shm *tee_shm_alloc_user_buf(struct tee_context *ctx, size_t size); > > struct tee_shm *tee_shm_register_user_buf(struct tee_context *ctx, > > unsigned long addr, size_t length); > > > > +int tee_heap_update_from_dma_buf(struct tee_device *teedev, > > + struct dma_buf *dmabuf, size_t *offset, > > + struct tee_shm *shm, > > + struct tee_shm **parent_shm); > > + > > #endif /*TEE_PRIVATE_H*/ > > diff --git a/include/linux/tee_core.h b/include/linux/tee_core.h > > index a38494d6b5f4..b8b99c97e00c 100644 > > --- a/include/linux/tee_core.h > > +++ b/include/linux/tee_core.h > > @@ -8,9 +8,11 @@ > > > > #include <linux/cdev.h> > > #include <linux/device.h> > > +#include <linux/dma-buf.h> > > #include <linux/idr.h> > > #include <linux/kref.h> > > #include <linux/list.h> > > +#include <linux/scatterlist.h> > > #include <linux/tee.h> > > #include <linux/tee_drv.h> > > #include <linux/types.h> > > @@ -30,6 +32,12 @@ > > #define TEE_DEVICE_FLAG_REGISTERED 0x1 > > #define TEE_MAX_DEV_NAME_LEN 32 > > > > +enum tee_dma_heap_id { > > + TEE_DMA_HEAP_SECURE_VIDEO_PLAY = 1, > > + TEE_DMA_HEAP_TRUSTED_UI, > > + TEE_DMA_HEAP_SECURE_VIDEO_RECORD, > > +}; > > + > > /** > > * struct tee_device - TEE Device representation > > * @name: name of device > > @@ -116,6 +124,36 @@ struct tee_desc { > > u32 flags; > > }; > > > > +/** > > + * struct tee_protmem_pool - protected memory pool > > + * @ops: operations > > + * > > + * This is an abstract interface where this struct is expected to be > > + * embedded in another struct specific to the implementation. > > + */ > > +struct tee_protmem_pool { > > + const struct tee_protmem_pool_ops *ops; > > +}; > > + > > +/** > > + * struct tee_protmem_pool_ops - protected memory pool operations > > + * @alloc: called when allocating protected memory > > + * @free: called when freeing protected memory > > + * @update_shm: called when registering a dma-buf to update the @shm > > + * with physical address of the buffer or to return the > > + * @parent_shm of the memory pool > > + * @destroy_pool: called when destroying the pool > > + */ > > +struct tee_protmem_pool_ops { > > + int (*alloc)(struct tee_protmem_pool *pool, struct sg_table *sgt, > > + size_t size, size_t *offs); > > + void (*free)(struct tee_protmem_pool *pool, struct sg_table *sgt); > > + int (*update_shm)(struct tee_protmem_pool *pool, struct sg_table *sgt, > > + size_t offs, struct tee_shm *shm, > > + struct tee_shm **parent_shm); > > + void (*destroy_pool)(struct tee_protmem_pool *pool); > > +}; > > + > > /** > > * tee_device_alloc() - Allocate a new struct tee_device instance > > * @teedesc: Descriptor for this driver > > @@ -154,6 +192,11 @@ int tee_device_register(struct tee_device *teedev); > > */ > > void tee_device_unregister(struct tee_device *teedev); > > > > +int tee_device_register_dma_heap(struct tee_device *teedev, > > + enum tee_dma_heap_id id, > > + struct tee_protmem_pool *pool); > > +void tee_device_unregister_all_dma_heaps(struct tee_device *teedev); > > + > > /** > > * tee_device_set_dev_groups() - Set device attribute groups > > * @teedev: Device to register > > @@ -229,6 +272,28 @@ static inline void tee_shm_pool_free(struct tee_shm_pool *pool) > > pool->ops->destroy_pool(pool); > > } > > > > +/** > > + * tee_protmem_static_pool_alloc() - Create a protected memory manager > > + * @paddr: Physical address of start of pool > > + * @size: Size in bytes of the pool > > + * > > + * @returns pointer to a 'struct tee_shm_pool' or an ERR_PTR on failure. > > s/tee_shm_pool/tee_protmem_pool/ I'll fix it. > > Rest looks fine to me. Thanks, Jens > > -Sumit > > > + */ > > +struct tee_protmem_pool *tee_protmem_static_pool_alloc(phys_addr_t paddr, > > + size_t size); > > + > > +/** > > + * tee_protmem_pool_free() - Free a protected memory pool > > + * @pool: The protected memory pool to free > > + * > > + * There must be no remaining protected memory allocated from this pool > > + * when this function is called. > > + */ > > +static inline void tee_protmem_pool_free(struct tee_protmem_pool *pool) > > +{ > > + pool->ops->destroy_pool(pool); > > +} > > + > > /** > > * tee_get_drvdata() - Return driver_data pointer > > * @returns the driver_data pointer supplied to tee_register(). > > -- > > 2.43.0 > >

1 week, 5 days

1
0
0 0

Re: [PATCH v3 01/10] dt-bindings: npu: rockchip,rknn: Add bindings

by Rob Herring (Arm)

On Fri, 16 May 2025 18:53:15 +0200, Tomeu Vizoso wrote: > Add the bindings for the Neural Processing Unit IP from Rockchip. > > v2: > - Adapt to new node structure (one node per core, each with its own > IOMMU) > - Several misc. fixes from Sebastian Reichel > > v3: > - Split register block in its constituent subblocks, and only require > the ones that the kernel would ever use (Nicolas Frattaroli) > - Group supplies (Rob Herring) > - Explain the way in which the top core is special (Rob Herring) > > Signed-off-by: Tomeu Vizoso <tomeu(a)tomeuvizoso.net> > Signed-off-by: Sebastian Reichel <sebastian.reichel(a)collabora.com> > --- > .../bindings/npu/rockchip,rknn-core.yaml | 162 +++++++++++++++++++++ > 1 file changed, 162 insertions(+) > My bot found errors running 'make dt_binding_check' on your patch: yamllint warnings/errors: dtschema/dtc warnings/errors: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/npu/rockchip,rknn-core.yaml: properties:reg-names: 'oneOf' conditional failed, one must be fixed: [{'const': 'pc'}, {'const': 'cna'}, {'const': 'core'}] is too long [{'const': 'pc'}, {'const': 'cna'}, {'const': 'core'}] is too short False schema does not allow 3 1 was expected 3 is greater than the maximum of 2 hint: "minItems" is only needed if less than the "items" list length from schema $id: http://devicetree.org/meta-schemas/items.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/npu/rockchip,rknn-core.example.dtb: npu-core@fdab0000 (rockchip,rk3588-rknn-core-top): compatible: 'oneOf' conditional failed, one must be fixed: ['rockchip,rk3588-rknn-core-top', 'rockchip,rknn-core-top'] is too long 'rockchip,rk3588-rknn-core-top' is not one of ['rockchip,rk3588-rknn-core'] from schema $id: http://devicetree.org/schemas/npu/rockchip,rknn-core.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/npu/rockchip,rknn-core.example.dtb: npu-core@fdab0000 (rockchip,rk3588-rknn-core-top): reg: [[0, 4255842304, 0, 36864]] is too short from schema $id: http://devicetree.org/schemas/npu/rockchip,rknn-core.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/npu/rockchip,rknn-core.example.dtb: npu-core@fdac0000 (rockchip,rk3588-rknn-core): compatible: 'oneOf' conditional failed, one must be fixed: ['rockchip,rk3588-rknn-core', 'rockchip,rknn-core'] is too long 'rockchip,rk3588-rknn-core' is not one of ['rockchip,rk3588-rknn-core-top'] from schema $id: http://devicetree.org/schemas/npu/rockchip,rknn-core.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/npu/rockchip,rknn-core.example.dtb: npu-core@fdac0000 (rockchip,rk3588-rknn-core): reg: [[0, 4255907840, 0, 36864]] is too short from schema $id: http://devicetree.org/schemas/npu/rockchip,rknn-core.yaml# doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250516-6-1… The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.

2 weeks, 1 day

2
1
0 0

[PATCH v9 0/9] TEE subsystem for protected dma-buf allocations

by Jens Wiklander

Hi, This patch set allocates the protected DMA-bufs from a DMA-heap instantiated from the TEE subsystem. The TEE subsystem handles the DMA-buf allocations since it is the TEE (OP-TEE, AMD-TEE, TS-TEE, or perhaps a future QTEE) which sets up the protection for the memory used for the DMA-bufs. The DMA-heap uses a protected memory pool provided by the backend TEE driver, allowing it to choose how to allocate the protected physical memory. The allocated DMA-bufs must be imported with a new TEE_IOC_SHM_REGISTER_FD before they can be passed as arguments when requesting services from the secure world. Three use-cases (Secure Video Playback, Trusted UI, and Secure Video Recording) have been identified so far to serve as examples of what can be expected. The use-cases have predefined DMA-heap names, "protected,secure-video", "protected,trusted-ui", and "protected,secure-video-record". The backend driver registers protected memory pools for the use-cases it supports. Each use-case has its own protected memory pool since different use-cases require isolation from different parts of the system. A protected memory pool can be based on a static carveout instantiated while probing the TEE backend driver, or dynamically allocated from CMA (dma_alloc_pages()) and made protected as needed by the TEE. This can be tested on a RockPi 4B+ with the following steps: repo init -u https://github.com/jenswi-linaro/manifest.git -m rockpi4.xml \ -b prototype/sdp-v9 repo sync -j8 cd build make toolchains -j$(nproc) make all -j$(nproc) # Copy ../out/rockpi4.img to an SD card and boot the RockPi from that # Connect a monitor to the RockPi # login and at the prompt: gst-launch-1.0 videotestsrc ! \ aesenc key=1f9423681beb9a79215820f6bda73d0f \ iv=e9aa8e834d8d70b7e0d254ff670dd718 serialize-iv=true ! \ aesdec key=1f9423681beb9a79215820f6bda73d0f ! \ kmssink The aesdec module has been hacked to use an OP-TEE TA to decrypt the stream into protected DMA-bufs which are consumed by the kmssink. The primitive QEMU tests from previous patch sets can be tested on RockPi in the same way using: xtest --sdp-basic The primitive tests are tested on QEMU with the following steps: repo init -u https://github.com/jenswi-linaro/manifest.git -m qemu_v8.xml \ -b prototype/sdp-v9 repo sync -j8 cd build make toolchains -j$(nproc) make SPMC_AT_EL=1 all -j$(nproc) make SPMC_AT_EL=1 run-only # login and at the prompt: xtest --sdp-basic The SPMC_AT_EL=1 parameter configures the build with FF-A and an SPMC at S-EL1 inside OP-TEE. The parameter can be changed to SPMC_AT_EL=n to test without FF-A using the original SMC ABI instead. Please remember to do %make arm-tf-clean for TF-A to be rebuilt properly using the new configuration. https://optee.readthedocs.io/en/latest/building/prerequisites.html list dependencies required to build the above. The primitive tests are pretty basic, mostly checking that a Trusted Application in the secure world can access and manipulate the memory. There are also some negative tests for out of bounds buffers, etc. Thanks, Jens Changes since V8: * Using dma_alloc_pages() instead of cma_alloc() so the direct dependency on CMA can be removed together with the patches "cma: export cma_alloc() and cma_release()" and "dma-contiguous: export dma_contiguous_default_area". The patch * Renaming the patch "tee: add tee_shm_alloc_cma_phys_mem()" to "tee: add tee_shm_alloc_dma_mem()" * Setting DMA mask for the OP-TEE TEE device based on input from the secure world instead of relying on the parent device so following patches are removed: "tee: tee_device_alloc(): copy dma_mask from parent device" and "optee: pass parent device to tee_device_alloc()". * Adding Sumit Garg's R-B to "tee: refactor params_from_user()" * In the patch "tee: implement protected DMA-heap", map the physical memory passed to tee_protmem_static_pool_alloc(). Changes since V7: * Adding "dma-buf: dma-heap: export declared functions", "cma: export cma_alloc() and cma_release()", and "dma-contiguous: export dma_contiguous_default_area" to export the symbols needed to keep the TEE subsystem as a load module. * Removing CONFIG_TEE_DMABUF_HEAP and CONFIG_TEE_CMA since they aren't needed any longer. * Addressing review comments in "optee: sync secure world ABI headers" * Better align protected memory pool initialization between the smc-abi and ffa-abi parts of the optee driver. * Removing the patch "optee: account for direction while converting parameters" Changes since V6: * Restricted memory is now known as protected memory since to use the same term as https://docs.vulkan.org/guide/latest/protected.html. Update all patches to consistently use protected memory. * In "tee: implement protected DMA-heap" add the hidden config option TEE_DMABUF_HEAP to tell if the DMABUF_HEAPS functions are available for the TEE subsystem * Adding "tee: refactor params_from_user()", broken out from the patch "tee: new ioctl to a register tee_shm from a dmabuf file descriptor" * For "tee: new ioctl to a register tee_shm from a dmabuf file descriptor": - Update commit message to mention protected memory - Remove and open code tee_shm_get_parent_shm() in param_from_user_memref() * In "tee: add tee_shm_alloc_cma_phys_mem" add the hidden config option TEE_CMA to tell if the CMA functions are available for the TEE subsystem * For "tee: tee_device_alloc(): copy dma_mask from parent device" and "optee: pass parent device to tee_device_alloc", added Reviewed-by: Sumit Garg <sumit.garg(a)kernel.org> Changes since V5: * Removing "tee: add restricted memory allocation" and "tee: add TEE_IOC_RSTMEM_FD_INFO" * Adding "tee: implement restricted DMA-heap", "tee: new ioctl to a register tee_shm from a dmabuf file descriptor", "tee: add tee_shm_alloc_cma_phys_mem()", "optee: pass parent device to tee_device_alloc()", and "tee: tee_device_alloc(): copy dma_mask from parent device" * The two TEE driver OPs "rstmem_alloc()" and "rstmem_free()" are replaced with a struct tee_rstmem_pool abstraction. * Replaced the the TEE_IOC_RSTMEM_ALLOC user space API with the DMA-heap API Changes since V4: * Adding the patch "tee: add TEE_IOC_RSTMEM_FD_INFO" needed by the GStreamer demo * Removing the dummy CPU access and mmap functions from the dma_buf_ops * Fixing a compile error in "optee: FF-A: dynamic restricted memory allocation" reported by kernel test robot <lkp(a)intel.com> Changes since V3: * Make the use_case and flags field in struct tee_shm u32's instead of u16's * Add more description for TEE_IOC_RSTMEM_ALLOC in the header file * Import namespace DMA_BUF in module tee, reported by lkp(a)intel.com * Added a note in the commit message for "optee: account for direction while converting parameters" why it's needed * Factor out dynamic restricted memory allocation from "optee: support restricted memory allocation" into two new commits "optee: FF-A: dynamic restricted memory allocation" and "optee: smc abi: dynamic restricted memory allocation" * Guard CMA usage with #ifdef CONFIG_CMA, effectively disabling dynamic restricted memory allocate if CMA isn't configured Changes since the V2 RFC: * Based on v6.12 * Replaced the flags for SVP and Trusted UID memory with a u32 field with unique id for each use case * Added dynamic allocation of restricted memory pools * Added OP-TEE ABI both with and without FF-A for dynamic restricted memory * Added support for FF-A with FFA_LEND Changes since the V1 RFC: * Based on v6.11 * Complete rewrite, replacing the restricted heap with TEE_IOC_RSTMEM_ALLOC Changes since Olivier's post [2]: * Based on Yong Wu's post [1] where much of dma-buf handling is done in the generic restricted heap * Simplifications and cleanup * New commit message for "dma-buf: heaps: add Linaro restricted dmabuf heap support" * Replaced the word "secure" with "restricted" where applicable Etienne Carriere (1): tee: new ioctl to a register tee_shm from a dmabuf file descriptor Jens Wiklander (8): optee: sync secure world ABI headers dma-buf: dma-heap: export declared functions tee: implement protected DMA-heap tee: refactor params_from_user() tee: add tee_shm_alloc_dma_mem() optee: support protected memory allocation optee: FF-A: dynamic protected memory allocation optee: smc abi: dynamic protected memory allocation drivers/dma-buf/dma-heap.c | 3 + drivers/tee/Makefile | 1 + drivers/tee/optee/Makefile | 1 + drivers/tee/optee/core.c | 10 + drivers/tee/optee/ffa_abi.c | 147 ++++++++- drivers/tee/optee/optee_ffa.h | 27 +- drivers/tee/optee/optee_msg.h | 84 +++++- drivers/tee/optee/optee_private.h | 15 +- drivers/tee/optee/optee_smc.h | 37 ++- drivers/tee/optee/protmem.c | 332 ++++++++++++++++++++ drivers/tee/optee/smc_abi.c | 113 ++++++- drivers/tee/tee_core.c | 155 +++++++--- drivers/tee/tee_heap.c | 487 ++++++++++++++++++++++++++++++ drivers/tee/tee_private.h | 16 + drivers/tee/tee_shm.c | 183 ++++++++++- include/linux/tee_core.h | 71 +++++ include/linux/tee_drv.h | 10 + include/uapi/linux/tee.h | 31 ++ 18 files changed, 1655 insertions(+), 68 deletions(-) create mode 100644 drivers/tee/optee/protmem.c create mode 100644 drivers/tee/tee_heap.c base-commit: b4432656b36e5cc1d50a1f2dc15357543add530e -- 2.43.0

2 weeks, 1 day

3
14
0 0

Re: [PATCH 2/2] dmabuf/heaps: implement DMA_BUF_IOCTL_RW_FILE for system_heap

by Christian König

On 5/22/25 10:02, wangtao wrote: >> -----Original Message----- >> From: Christian König <christian.koenig(a)amd.com> >> Sent: Wednesday, May 21, 2025 7:57 PM >> To: wangtao <tao.wangtao(a)honor.com>; T.J. Mercier >> <tjmercier(a)google.com> >> Cc: sumit.semwal(a)linaro.org; benjamin.gaignard(a)collabora.com; >> Brian.Starkey(a)arm.com; jstultz(a)google.com; linux-media(a)vger.kernel.org; >> dri-devel(a)lists.freedesktop.org; linaro-mm-sig(a)lists.linaro.org; linux- >> kernel(a)vger.kernel.org; wangbintian(BintianWang) >> <bintian.wang(a)honor.com>; yipengxiang <yipengxiang(a)honor.com>; liulu >> 00013167 <liulu.liu(a)honor.com>; hanfeng 00012985 <feng.han(a)honor.com>; >> amir73il(a)gmail.com >> Subject: Re: [PATCH 2/2] dmabuf/heaps: implement >> DMA_BUF_IOCTL_RW_FILE for system_heap >> >> On 5/21/25 12:25, wangtao wrote: >>> [wangtao] I previously explained that >>> read/sendfile/splice/copy_file_range >>> syscalls can't achieve dmabuf direct IO zero-copy. >> >> And why can't you work on improving those syscalls instead of creating a new >> IOCTL? >> > [wangtao] As I mentioned in previous emails, these syscalls cannot > achieve dmabuf zero-copy due to technical constraints. Yeah, and why can't you work on removing those technical constrains? What is blocking you from improving the sendfile system call or proposing a patch to remove the copy_file_range restrictions? Regards, Christian. Could you > specify the technical points, code, or principles that need > optimization? > > Let me explain again why these syscalls can't work: > 1. read() syscall > - dmabuf fops lacks read callback implementation. Even if implemented, > file_fd info cannot be transferred > - read(file_fd, dmabuf_ptr, len) with remap_pfn_range-based mmap > cannot access dmabuf_buf pages, forcing buffer-mode reads > > 2. sendfile() syscall > - Requires CPU copy from page cache to memory file(tmpfs/shmem): > [DISK] --DMA--> [page cache] --CPU copy--> [MEMORY file] > - CPU overhead (both buffer/direct modes involve copies): > 55.08% do_sendfile > |- 55.08% do_splice_direct > |-|- 55.08% splice_direct_to_actor > |-|-|- 22.51% copy_splice_read > |-|-|-|- 16.57% f2fs_file_read_iter > |-|-|-|-|- 15.12% __iomap_dio_rw > |-|-|- 32.33% direct_splice_actor > |-|-|-|- 32.11% iter_file_splice_write > |-|-|-|-|- 28.42% vfs_iter_write > |-|-|-|-|-|- 28.42% do_iter_write > |-|-|-|-|-|-|- 28.39% shmem_file_write_iter > |-|-|-|-|-|-|-|- 24.62% generic_perform_write > |-|-|-|-|-|-|-|-|- 18.75% __pi_memmove > > 3. splice() requires one end to be a pipe, incompatible with regular files or dmabuf. > > 4. copy_file_range() > - Blocked by cross-FS restrictions (Amir's commit 868f9f2f8e00) > - Even without this restriction, Even without restrictions, implementing > the copy_file_range callback in dmabuf fops would only allow dmabuf read > from regular files. This is because copy_file_range relies on > file_out->f_op->copy_file_range, which cannot support dmabuf write > operations to regular files. > > Test results confirm these limitations: > T.J. Mercier's 1G from ext4 on 6.12.20 | read/sendfile (ms) w/ 3 > drop_caches > ------------------------|------------------- > udmabuf buffer read | 1210 > udmabuf direct read | 671 > udmabuf buffer sendfile | 1096 > udmabuf direct sendfile | 2340 > > My 3GHz CPU tests (cache cleared): > Method | alloc | read | vs. (%) > ----------------------------------------------- > udmabuf buffer read | 135 | 546 | 180% > udmabuf direct read | 159 | 300 | 99% > udmabuf buffer sendfile | 134 | 303 | 100% > udmabuf direct sendfile | 141 | 912 | 301% > dmabuf buffer read | 22 | 362 | 119% > my patch direct read | 29 | 265 | 87% > > My 1GHz CPU tests (cache cleared): > Method | alloc | read | vs. (%) > ----------------------------------------------- > udmabuf buffer read | 552 | 2067 | 198% > udmabuf direct read | 540 | 627 | 60% > udmabuf buffer sendfile | 497 | 1045 | 100% > udmabuf direct sendfile | 527 | 2330 | 223% > dmabuf buffer read | 40 | 1111 | 106% > patch direct read | 44 | 310 | 30% > > Test observations align with expectations: > 1. dmabuf buffer read requires slow CPU copies > 2. udmabuf direct read achieves zero-copy but has page retrieval > latency from vaddr > 3. udmabuf buffer sendfile suffers CPU copy overhead > 4. udmabuf direct sendfile combines CPU copies with frequent DMA > operations due to small pipe buffers > 5. dmabuf buffer read also requires CPU copies > 6. My direct read patch enables zero-copy with better performance > on low-power CPUs > 7. udmabuf creation time remains problematic (as you’ve noted). > >>> My focus is enabling dmabuf direct I/O for [regular file] <--DMA--> >>> [dmabuf] zero-copy. >> >> Yeah and that focus is wrong. You need to work on a general solution to the >> issue and not specific to your problem. >> >>> Any API achieving this would work. Are there other uAPIs you think >>> could help? Could you recommend experts who might offer suggestions? >> >> Well once more: Either work on sendfile or copy_file_range or eventually >> splice to make it what you want to do. >> >> When that is done we can discuss with the VFS people if that approach is >> feasible. >> >> But just bypassing the VFS review by implementing a DMA-buf specific IOCTL >> is a NO-GO. That is clearly not something you can do in any way. > [wangtao] The issue is that only dmabuf lacks Direct I/O zero-copy support. Tmpfs/shmem > already work with Direct I/O zero-copy. As explained, existing syscalls or > generic methods can't enable dmabuf direct I/O zero-copy, which is why I > propose adding an IOCTL command. > > I respect your perspective. Could you clarify specific technical aspects, > code requirements, or implementation principles for modifying sendfile() > or copy_file_range()? This would help advance our discussion. > > Thank you for engaging in this dialogue. > >> >> Regards, >> Christian.

2 weeks, 2 days

1
0
0 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

Linaro-mm-sig