On 4/14/25 2:21 PM, Thomas Petazzoni wrote:
> Hello Andrew,
>
> On Mon, 14 Apr 2025 12:08:44 -0500
> Andrew Davis <afd(a)ti.com> wrote:
>
>> "UIO is a broken legacy mess, so let's add more broken things
>> to it as broken + broken => still broken, so no harm done", am I
>> getting that right?
>
> Who says UIO is a "broken legacy mess"? Only you says so. I don't see
> any indication anywhere in the kernel tree suggesting that UIO is
> considered a broken legacy mess.
>
I'm not saying that*, I'm pointing out your argument is that even
though what you are trying to do is broken and unsafe, it is okay to
do because it isn't any "more "broken and unsafe" than UIO already is."
*It is, but that is an argument to have outside of this thread :)
> Keep in mind that when you're running code as root, you can load a
> kernel module, which can do anything on the system security-wise. So
> letting UIO expose MMIO registers of devices to userspace applications
> running as root is not any worse than that.
>
You can take your computer out back and shoot it too, but we shouldn't
encourage that either :) According to the original docs, UIO was created
to support "industrial I/O cards", think old one-off custom ISA cards by
vendors that had no intention of ever writing a proper driver and just
wanted to poke registers and wait on an IRQ.
IMHO we shouldn't be encouraging that, and trying to modernize UIO does just
that. It gives the impression that is how drivers should still be written.
If you setup your FPGA card to go blink an LED, sure UIO driver it is,
anything more complex, then writing a proper driver is the way to go.
>> If your FPGA IP can do DMA then you should not be using UIO in
>> the first place, see UIO docs:
>>
>>> Please note that UIO is not an universal driver interface. Devices that
>>> are already handled well by other kernel subsystems (like networking or
>>> serial or USB) are no candidates for an UIO driver.
>>
>> The DMA subsystem already handles DMA devices, so write a DMA driver.
>
> My FPGA IP block is not a DMA controller that would fit the dmaengine
> kernel subsystem. It's a weird custom device that doesn't fit in any
> existing subsystem, and that happens to do "peripheral DMA" (i.e the IP
> block is DMA-capable itself, without relying on a separate DMA
> controller). So this (very valid) recommendation from the UIO
> documentation doesn't apply to my device.
Peripheral DMA is the much more common case, nothing new here. Could
you give a hint as to what this device does that doesn't fit *any*
current subsystem? Or are we talking a hypothetical device (which
for the sake of argument is a valid thing to say, I'm sure with an
FPGA card I could make something that doesn't fit any current
framework too). Just want to know if you are trying to solve a
specific issue or a generic issue here.
Andrew
>
> Best regards,
>
> Thomas
Hi,
This series is the follow-up of the discussion that John and I had some
time ago here:
https://lore.kernel.org/all/CANDhNCquJn6bH3KxKf65BWiTYLVqSd9892-xtFDHHqqyrr…
The initial problem we were discussing was that I'm currently working on
a platform which has a memory layout with ECC enabled. However, enabling
the ECC has a number of drawbacks on that platform: lower performance,
increased memory usage, etc. So for things like framebuffers, the
trade-off isn't great and thus there's a memory region with ECC disabled
to allocate from for such use cases.
After a suggestion from John, I chose to first start using heap
allocations flags to allow for userspace to ask for a particular ECC
setup. This is then backed by a new heap type that runs from reserved
memory chunks flagged as such, and the existing DT properties to specify
the ECC properties.
After further discussion, it was considered that flags were not the
right solution, and relying on the names of the heaps would be enough to
let userspace know the kind of buffer it deals with.
Thus, even though the uAPI part of it has been dropped in this second
version, we still need a driver to create heaps out of carved-out memory
regions. In addition to the original usecase, a similar driver can be
found in BSPs from most vendors, so I believe it would be a useful
addition to the kernel.
I submitted a draft PR to the DT schema for the bindings used in this
PR:
https://github.com/devicetree-org/dt-schema/pull/138
Let me know what you think,
Maxime
Signed-off-by: Maxime Ripard <mripard(a)kernel.org>
---
Changes in v3:
- Reworked global variable patch
- Link to v2: https://lore.kernel.org/r/20250401-dma-buf-ecc-heap-v2-0-043fd006a1af@kerne…
Changes in v2:
- Add vmap/vunmap operations
- Drop ECC flags uapi
- Rebase on top of 6.14
- Link to v1: https://lore.kernel.org/r/20240515-dma-buf-ecc-heap-v1-0-54cbbd049511@kerne…
---
Maxime Ripard (2):
dma-buf: heaps: system: Remove global variable
dma-buf: heaps: Introduce a new heap for reserved memory
drivers/dma-buf/heaps/Kconfig | 8 +
drivers/dma-buf/heaps/Makefile | 1 +
drivers/dma-buf/heaps/carveout_heap.c | 360 ++++++++++++++++++++++++++++++++++
drivers/dma-buf/heaps/system_heap.c | 3 +-
4 files changed, 370 insertions(+), 2 deletions(-)
---
base-commit: fcbf30774e82a441890b722bf0c26542fb82150f
change-id: 20240515-dma-buf-ecc-heap-28a311d2c94e
Best regards,
--
Maxime Ripard <mripard(a)kernel.org>
On 4/14/25 6:48 AM, Thomas Petazzoni wrote:
> Hello Christoph,
>
> On Mon, 14 Apr 2025 04:24:21 -0700
> Christoph Hellwig <hch(a)infradead.org> wrote:
>
>> On Mon, Apr 14, 2025 at 10:24:55AM +0200, Thomas Petazzoni wrote:
>>> What this patch series is about is to add new user-space interface to
>>> extend the existing UIO subsystem.
>>
>> Which as I explained to you is fundamentally broken and unsafe. If you
>> need to do DMA from userspae you need to use vfio/iommufd.
>
> I'm still unclear as to why it is more "broken and unsafe" than UIO
> already is. As I already replied in this thread: UIO allows to remap
> MMIO registers into a user-space application, which can then do
> whatever it wants with the IP block behind those MMIO registers. If
> this IP block supports DMA, it already means that _today_ with the
> current UIO subsystem as it is, the user-space application can program
> a DMA transfer to read/write to any location in memory.
>
> Therefore, providing a way to cleanly allocate DMA buffers and get
> their physical address will not make things any better or worse in
> terms of safety.
>
> The fact that it is reasonably safe is solely based on access control
> to the UIO device, done using usual Unix permissions, and that is
> already the case today.
>
>>> I am not sure how this can work in our use-case. We have a very simple
>>> set of IP blocks implemented in a FPGA, some of those IP blocks are
>>> able to perform DMA operations. The register of those IP blocks are
>>> mapped into a user-space application using the existing, accepted
>>> upstream, UIO subsystem. Some of those registers allow to program DMA
>>> transfers. So far, we can do all what we need, except program those DMA
>>> transfers. Lots of people are having the same issue, and zillions of
>>> ugly out-of-tree solutions flourish all over, and we're trying to see
>>> if we can constructively find a solution that would be acceptable
>>> upstream to resolve this use-case. Our platform is an old PowerPC with
>>> no IOMMU.
>>
>> Then your driver design can't work and you need to replace it with a
>> proper in-kernel driver.
>
> See above: your point is moot because providing capabilities to
> allocate a buffer and get its physical address so that a UIO-based
> user-space application can do DMA transfer does not make things any
> more unsafe than they already are.
>
"UIO is a broken legacy mess, so let's add more broken things
to it as broken + broken => still broken, so no harm done", am I
getting that right?
If your FPGA IP can do DMA then you should not be using UIO in
the first place, see UIO docs:
> Please note that UIO is not an universal driver interface. Devices that
> are already handled well by other kernel subsystems (like networking or
> serial or USB) are no candidates for an UIO driver.
The DMA subsystem already handles DMA devices, so write a DMA driver.
Andrew
> Best regards,
>
> Thomas
Hi Adrián,
kernel test robot noticed the following build warnings:
[auto build test WARNING on linus/master]
[also build test WARNING on v6.15-rc2 next-20250414]
[cannot apply to drm-misc/drm-misc-next]
[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/Adri-n-Larumbe/drm-panthor-I…
base: linus/master
patch link: https://lore.kernel.org/r/20250411150357.3308921-4-adrian.larumbe%40collabo…
patch subject: [PATCH v7 3/4] drm/panthor: Label all kernel BO's
config: i386-buildonly-randconfig-006-20250414 (https://download.01.org/0day-ci/archive/20250414/202504142148.NBAyzLuE-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/20250414/202504142148.NBAyzLuE-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/202504142148.NBAyzLuE-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> drivers/gpu/drm/panthor/panthor_gem.c:86: warning: Function parameter or struct member 'name' not described in 'panthor_kernel_bo_create'
vim +86 drivers/gpu/drm/panthor/panthor_gem.c
8a1cc07578bf42 Boris Brezillon 2024-02-29 67
8a1cc07578bf42 Boris Brezillon 2024-02-29 68 /**
8a1cc07578bf42 Boris Brezillon 2024-02-29 69 * panthor_kernel_bo_create() - Create and map a GEM object to a VM
8a1cc07578bf42 Boris Brezillon 2024-02-29 70 * @ptdev: Device.
8a1cc07578bf42 Boris Brezillon 2024-02-29 71 * @vm: VM to map the GEM to. If NULL, the kernel object is not GPU mapped.
8a1cc07578bf42 Boris Brezillon 2024-02-29 72 * @size: Size of the buffer object.
8a1cc07578bf42 Boris Brezillon 2024-02-29 73 * @bo_flags: Combination of drm_panthor_bo_flags flags.
8a1cc07578bf42 Boris Brezillon 2024-02-29 74 * @vm_map_flags: Combination of drm_panthor_vm_bind_op_flags (only those
8a1cc07578bf42 Boris Brezillon 2024-02-29 75 * that are related to map operations).
8a1cc07578bf42 Boris Brezillon 2024-02-29 76 * @gpu_va: GPU address assigned when mapping to the VM.
8a1cc07578bf42 Boris Brezillon 2024-02-29 77 * If gpu_va == PANTHOR_VM_KERNEL_AUTO_VA, the virtual address will be
8a1cc07578bf42 Boris Brezillon 2024-02-29 78 * automatically allocated.
8a1cc07578bf42 Boris Brezillon 2024-02-29 79 *
8a1cc07578bf42 Boris Brezillon 2024-02-29 80 * Return: A valid pointer in case of success, an ERR_PTR() otherwise.
8a1cc07578bf42 Boris Brezillon 2024-02-29 81 */
8a1cc07578bf42 Boris Brezillon 2024-02-29 82 struct panthor_kernel_bo *
8a1cc07578bf42 Boris Brezillon 2024-02-29 83 panthor_kernel_bo_create(struct panthor_device *ptdev, struct panthor_vm *vm,
8a1cc07578bf42 Boris Brezillon 2024-02-29 84 size_t size, u32 bo_flags, u32 vm_map_flags,
f48f05d54f7696 Adrián Larumbe 2025-04-11 85 u64 gpu_va, const char *name)
8a1cc07578bf42 Boris Brezillon 2024-02-29 @86 {
8a1cc07578bf42 Boris Brezillon 2024-02-29 87 struct drm_gem_shmem_object *obj;
8a1cc07578bf42 Boris Brezillon 2024-02-29 88 struct panthor_kernel_bo *kbo;
8a1cc07578bf42 Boris Brezillon 2024-02-29 89 struct panthor_gem_object *bo;
8a1cc07578bf42 Boris Brezillon 2024-02-29 90 int ret;
8a1cc07578bf42 Boris Brezillon 2024-02-29 91
8a1cc07578bf42 Boris Brezillon 2024-02-29 92 if (drm_WARN_ON(&ptdev->base, !vm))
8a1cc07578bf42 Boris Brezillon 2024-02-29 93 return ERR_PTR(-EINVAL);
8a1cc07578bf42 Boris Brezillon 2024-02-29 94
8a1cc07578bf42 Boris Brezillon 2024-02-29 95 kbo = kzalloc(sizeof(*kbo), GFP_KERNEL);
8a1cc07578bf42 Boris Brezillon 2024-02-29 96 if (!kbo)
8a1cc07578bf42 Boris Brezillon 2024-02-29 97 return ERR_PTR(-ENOMEM);
8a1cc07578bf42 Boris Brezillon 2024-02-29 98
8a1cc07578bf42 Boris Brezillon 2024-02-29 99 obj = drm_gem_shmem_create(&ptdev->base, size);
8a1cc07578bf42 Boris Brezillon 2024-02-29 100 if (IS_ERR(obj)) {
8a1cc07578bf42 Boris Brezillon 2024-02-29 101 ret = PTR_ERR(obj);
8a1cc07578bf42 Boris Brezillon 2024-02-29 102 goto err_free_bo;
8a1cc07578bf42 Boris Brezillon 2024-02-29 103 }
8a1cc07578bf42 Boris Brezillon 2024-02-29 104
8a1cc07578bf42 Boris Brezillon 2024-02-29 105 bo = to_panthor_bo(&obj->base);
8a1cc07578bf42 Boris Brezillon 2024-02-29 106 kbo->obj = &obj->base;
8a1cc07578bf42 Boris Brezillon 2024-02-29 107 bo->flags = bo_flags;
8a1cc07578bf42 Boris Brezillon 2024-02-29 108
f48f05d54f7696 Adrián Larumbe 2025-04-11 109 panthor_gem_kernel_bo_set_label(kbo, name);
f48f05d54f7696 Adrián Larumbe 2025-04-11 110
5d01b56f0518d8 Boris Brezillon 2024-10-30 111 /* The system and GPU MMU page size might differ, which becomes a
5d01b56f0518d8 Boris Brezillon 2024-10-30 112 * problem for FW sections that need to be mapped at explicit address
5d01b56f0518d8 Boris Brezillon 2024-10-30 113 * since our PAGE_SIZE alignment might cover a VA range that's
5d01b56f0518d8 Boris Brezillon 2024-10-30 114 * expected to be used for another section.
5d01b56f0518d8 Boris Brezillon 2024-10-30 115 * Make sure we never map more than we need.
5d01b56f0518d8 Boris Brezillon 2024-10-30 116 */
5d01b56f0518d8 Boris Brezillon 2024-10-30 117 size = ALIGN(size, panthor_vm_page_size(vm));
8a1cc07578bf42 Boris Brezillon 2024-02-29 118 ret = panthor_vm_alloc_va(vm, gpu_va, size, &kbo->va_node);
8a1cc07578bf42 Boris Brezillon 2024-02-29 119 if (ret)
8a1cc07578bf42 Boris Brezillon 2024-02-29 120 goto err_put_obj;
8a1cc07578bf42 Boris Brezillon 2024-02-29 121
8a1cc07578bf42 Boris Brezillon 2024-02-29 122 ret = panthor_vm_map_bo_range(vm, bo, 0, size, kbo->va_node.start, vm_map_flags);
8a1cc07578bf42 Boris Brezillon 2024-02-29 123 if (ret)
8a1cc07578bf42 Boris Brezillon 2024-02-29 124 goto err_free_va;
8a1cc07578bf42 Boris Brezillon 2024-02-29 125
ff60c8da0aaf7e Boris Brezillon 2024-05-02 126 kbo->vm = panthor_vm_get(vm);
8a1cc07578bf42 Boris Brezillon 2024-02-29 127 bo->exclusive_vm_root_gem = panthor_vm_root_gem(vm);
8a1cc07578bf42 Boris Brezillon 2024-02-29 128 drm_gem_object_get(bo->exclusive_vm_root_gem);
8a1cc07578bf42 Boris Brezillon 2024-02-29 129 bo->base.base.resv = bo->exclusive_vm_root_gem->resv;
8a1cc07578bf42 Boris Brezillon 2024-02-29 130 return kbo;
8a1cc07578bf42 Boris Brezillon 2024-02-29 131
8a1cc07578bf42 Boris Brezillon 2024-02-29 132 err_free_va:
8a1cc07578bf42 Boris Brezillon 2024-02-29 133 panthor_vm_free_va(vm, &kbo->va_node);
8a1cc07578bf42 Boris Brezillon 2024-02-29 134
8a1cc07578bf42 Boris Brezillon 2024-02-29 135 err_put_obj:
8a1cc07578bf42 Boris Brezillon 2024-02-29 136 drm_gem_object_put(&obj->base);
8a1cc07578bf42 Boris Brezillon 2024-02-29 137
8a1cc07578bf42 Boris Brezillon 2024-02-29 138 err_free_bo:
8a1cc07578bf42 Boris Brezillon 2024-02-29 139 kfree(kbo);
8a1cc07578bf42 Boris Brezillon 2024-02-29 140 return ERR_PTR(ret);
8a1cc07578bf42 Boris Brezillon 2024-02-29 141 }
8a1cc07578bf42 Boris Brezillon 2024-02-29 142
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki