-
Notifications
You must be signed in to change notification settings - Fork 8.1k
dma: New syscalls #97652
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
base: main
Are you sure you want to change the base?
dma: New syscalls #97652
Conversation
DMA syscalls as they were implemented were unsafe. Accepting a void* was never acceptable as many things could not be verified about it. Accepting a channel identifier meant that a user mode thread could start/stop any DMA channel which in theory could be owned by any other driver. This shouldn't be possible. Signed-off-by: Tom Burdick <[email protected]>
ffe5bbb
to
94a6eee
Compare
Adds structs for representing a DMA channel and DMA hardware fifo that are newly added kernel object types. These kernel objects can then be used to finely allow access control to user mode threads specific DMA channels as well as specific hardware FIFOs. This enables DMA to be used, with careful configuration, from a user mode thread. Some rules around the reload API need to be followed *by the DMA* driver to fully comply with the user space API. That is, the hardware FIFO address is validated against a well known set of addresses. That the FIFO source or destination address is not incremented by the transfer (write to preceeding or succeeding addresses). Signed-off-by: Tom Burdick <[email protected]>
94a6eee
to
b0b2d5d
Compare
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @teburd , this looks very good. Comments inline. I probably need to try using this to be able to give more feedback.
struct dma_channel { | ||
const struct device *dma; | ||
uint32_t channel; | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@teburd Ok this looks something I could work with. I have my draft code that implements the syscalls on SOF side. I did this better understand the pros and cons. Code here:
https://github.com/kv2019i/sof/commits/202510-userspace-sof-dma-test/
So I could perhaps try to use this PR as a basis.
You did now add more interfaces (like reload) to user-space? Were you thinking to add the Zephyr side API to also cover calls like the get dma_config() and dma_get_attribute()? dma_config() is a bit problematic as well as the generic interface has lot of configuration options, including passing a callback function. These obviously won't work from user-space. For SOF, we really use a subset of the interface, so I could expose a more limited version of dma_config() (as sof_dma_config()) and verify the allowed fields the vrfy function. Aside those two interfaces, I don't see much missing.
static inline int z_impl_dma_channel_start(const struct dma_channel *dma_chan) | ||
{ | ||
return dma_start(dma_chan->dma, dma_chan->channel); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Btw, I don't see any dma handlers in this commit (e.g. z_vrfy_dma_channel_start()). Did you forget this from the second commit?
Builds on #97058 and works to replace the existing available syscalls with new ones.
Note
Draft until tests are done and passing, here for first review on the API in dma.h for folks interested
Adds structs for representing a DMA channel and DMA hardware fifo that
are newly added kernel object types. These kernel objects can then be
used to finely allow access control to user mode threads specific DMA
channels as well as specific hardware FIFOs.
This enables DMA to be used, with careful configuration, from a user
mode thread.
Some rules around the reload API need to be followed by the DMA driver
to fully comply with the user space API. That is, the hardware FIFO
address is validated against a well known set of addresses. That the
FIFO source or destination address is not incremented by the transfer
(write to preceeding or succeeding addresses).