Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

uaccess tag by default for /dev/udmabuf #32662

Open
rmader opened this issue May 6, 2024 · 7 comments
Open

uaccess tag by default for /dev/udmabuf #32662

rmader opened this issue May 6, 2024 · 7 comments
Labels
RFE 🎁 Request for Enhancement, i.e. a feature request udev

Comments

@rmader
Copy link

rmader commented May 6, 2024

Component

udev rule files

Is your feature request related to a problem? Please describe

This came up in a downstream discussion in https://gitlab.alpinelinux.org/alpine/aports/-/merge_requests/65223#note_401039:

The dma_heap (/dev/dma_heap) allows apps to allocate generic DMABufs that can be directly imported to GL / VK or even KMS (if certain conditions are met - such as using the CMA / contiguous allocations) and was inspired by Androids ION. One prominent user of it is the upcoming libcamera softwareISP that is used to enable "complex" cameras on newer laptops and phones (to be released in libcamera 0.3 soon).

Describe the solution you'd like

libcamera / pipewire / the user needs access to /dev/dma_heap/* - and it would thus be great if systemd based distros allow access to it by default - i.e. something along the lines of

SUBSYSTEM=="dma_heap",TAG+="uaccess"

Note that I'm not an expert on this at all, so this is just a suggestion that "works for me".

Describe alternatives you've considered

postmarketOS - given that it doesn't use systemd by default yet - will give members of video group permissions to use it for now, see https://gitlab.alpinelinux.org/alpine/aports/-/merge_requests/65223

The systemd version you checked that didn't have the feature you are asking for

No response

@rmader rmader added the RFE 🎁 Request for Enhancement, i.e. a feature request label May 6, 2024
@github-actions github-actions bot added the udev label May 6, 2024
@poettering
Copy link
Member

We can certainly do something like this, but only if the relevant kernel subsystem maintainers sign off on this, that this is a safe thing to do.

@jwrdegoede iirc you are involved in the libcamera work, or did i mix that up? Any chance you can comment on this? Or CC the people who could?

@jwrdegoede
Copy link
Contributor

I actually had sending out an email to discuss doing this with the dma-buf maintainers on my TODO list. I have just send out that email:

https://lore.kernel.org/all/bb372250-e8b8-4458-bc99-dd8365b06991@redhat.com/

So lets await the dam-buf maintainers reply and discuss this further in that email thread.

@ndufresne
Copy link

ndufresne commented May 7, 2024

So far, it seems like a bad idea until dmabuf allocated from heaps can be accounted and under user quotas. I've let floating the idea for libcamera softISP to enable memfd + udmabuf (Laurent said memfd out of libcamera is a no-go). (but then you need kernels that enable udmabuf in the first place)

@jwrdegoede
Copy link
Contributor

The upstream kernel list discussion has come to the conclusion that opening up DMA heaps is a bad idea. But opening up /dev/udmabuf like this should be acceptable.

I have posted a patch upstream for libcamera to try udmabuf of the DMA heaps are not available:

https://lists.libcamera.org/pipermail/libcamera-devel/2024-May/042085.html

I think we should change the title of this issue to "uaccess tag by default for /dev/udmabuf". @rmader can you update this.

And/or maybe we should just submit a pull-request with this change ?

@rmader rmader changed the title uaccess tag by default for dma_heap subsystem uaccess tag by default for /dev/udmabuf May 28, 2024
@rmader
Copy link
Author

rmader commented May 28, 2024

I think we should change the title of this issue to "uaccess tag by default for /dev/udmabuf". @rmader can you update this.

Agreed, looks like it will be a while until the accounting issues around dma heaps will have been resolved - done.

And/or maybe we should just submit a pull-request with this change ?

Sounds good! Won't get to it at least in the next two weeks, so please go ahead or I'll pick it up later.

@jwrdegoede
Copy link
Contributor

Sounds good! Won't get to it at least in the next two weeks, so please go ahead or I'll pick it up later.

Any idea/suggestion which existing systemd udev rules file we should add this to ?

@keszybz
Copy link
Member

keszybz commented May 28, 2024

rules.d/70-uaccess.rules.in

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RFE 🎁 Request for Enhancement, i.e. a feature request udev
Development

No branches or pull requests

5 participants