Hi,
I started to review it, but it's probably best to discuss it here.
On Thu, Jan 30, 2025 at 01:08:56PM +0000, Florent Tomasin wrote:
Hi,
This is a patch series covering the support for protected mode execution in Mali Panthor CSF kernel driver.
The Mali CSF GPUs come with the support for protected mode execution at the HW level. This feature requires two main changes in the kernel driver:
- Configure the GPU with a protected buffer. The system must provide a DMA heap from which the driver can allocate a protected buffer. It can be a carved-out memory or dynamically allocated protected memory region. Some system includes a trusted FW which is in charge of the protected memory. Since this problem is integration specific, the Mali Panthor CSF kernel driver must import the protected memory from a device specific exporter.
Why do you need a heap for it in the first place? My understanding of your series is that you have a carved out memory region somewhere, and you want to allocate from that carved out memory region your buffers.
How is that any different from using a reserved-memory region, adding the reserved-memory property to the GPU device and doing all your allocation through the usual dma_alloc_* API?
Or do you expect to have another dma-buf heap that would call into the firmware to perform the allocations?
The semantics of the CMA heap allocations is a concern too.
Another question is how would you expect something like a secure video-playback pipeline to operate with that kind of interface. Assuming you have a secure codec, you would likely get that protected buffer from the codec, right? So the most likely scenario would be to then import that dma-buf into the GPU driver, but not allocate the buffer from it.
Overall, I think a better abstraction would be to have a heap indeed to allocate your protected buffers from, and then import them in the devices that need them. The responsibility would be on the userspace to do so, but it already kind of does with your design anyway.
Maxime
Le jeudi 30 janvier 2025 à 14:46 +0100, Maxime Ripard a écrit :
Hi,
I started to review it, but it's probably best to discuss it here.
On Thu, Jan 30, 2025 at 01:08:56PM +0000, Florent Tomasin wrote:
Hi,
This is a patch series covering the support for protected mode execution in Mali Panthor CSF kernel driver.
The Mali CSF GPUs come with the support for protected mode execution at the HW level. This feature requires two main changes in the kernel driver:
- Configure the GPU with a protected buffer. The system must provide a DMA heap from which the driver can allocate a protected buffer. It can be a carved-out memory or dynamically allocated protected memory region. Some system includes a trusted FW which is in charge of the protected memory. Since this problem is integration specific, the Mali Panthor CSF kernel driver must import the protected memory from a device specific exporter.
Why do you need a heap for it in the first place? My understanding of your series is that you have a carved out memory region somewhere, and you want to allocate from that carved out memory region your buffers.
How is that any different from using a reserved-memory region, adding the reserved-memory property to the GPU device and doing all your allocation through the usual dma_alloc_* API?
How do you then multiplex this region so it can be shared between GPU/Camera/Display/Codec drivers and also userspace ? Also, how the secure memory is allocted / obtained is a process that can vary a lot between SoC, so implementation details assumption should not be coded in the driver.
Nicolas
Or do you expect to have another dma-buf heap that would call into the firmware to perform the allocations?
The semantics of the CMA heap allocations is a concern too.
Another question is how would you expect something like a secure video-playback pipeline to operate with that kind of interface. Assuming you have a secure codec, you would likely get that protected buffer from the codec, right? So the most likely scenario would be to then import that dma-buf into the GPU driver, but not allocate the buffer from it.
Overall, I think a better abstraction would be to have a heap indeed to allocate your protected buffers from, and then import them in the devices that need them. The responsibility would be on the userspace to do so, but it already kind of does with your design anyway.
Maxime
Hi Nicolas,
On Thu, Jan 30, 2025 at 10:59:56AM -0500, Nicolas Dufresne wrote:
Le jeudi 30 janvier 2025 à 14:46 +0100, Maxime Ripard a écrit :
Hi,
I started to review it, but it's probably best to discuss it here.
On Thu, Jan 30, 2025 at 01:08:56PM +0000, Florent Tomasin wrote:
Hi,
This is a patch series covering the support for protected mode execution in Mali Panthor CSF kernel driver.
The Mali CSF GPUs come with the support for protected mode execution at the HW level. This feature requires two main changes in the kernel driver:
- Configure the GPU with a protected buffer. The system must provide a DMA heap from which the driver can allocate a protected buffer. It can be a carved-out memory or dynamically allocated protected memory region. Some system includes a trusted FW which is in charge of the protected memory. Since this problem is integration specific, the Mali Panthor CSF kernel driver must import the protected memory from a device specific exporter.
Why do you need a heap for it in the first place? My understanding of your series is that you have a carved out memory region somewhere, and you want to allocate from that carved out memory region your buffers.
How is that any different from using a reserved-memory region, adding the reserved-memory property to the GPU device and doing all your allocation through the usual dma_alloc_* API?
How do you then multiplex this region so it can be shared between GPU/Camera/Display/Codec drivers and also userspace ?
You could point all the devices to the same reserved memory region, and they would all allocate from there, including for their userspace-facing allocations.
Also, how the secure memory is allocted / obtained is a process that can vary a lot between SoC, so implementation details assumption should not be coded in the driver.
But yeah, we agree there, it's also the point I was trying to make :)
Maxime
Le jeudi 30 janvier 2025 à 17:38 +0100, Maxime Ripard a écrit :
Hi Nicolas,
On Thu, Jan 30, 2025 at 10:59:56AM -0500, Nicolas Dufresne wrote:
Le jeudi 30 janvier 2025 à 14:46 +0100, Maxime Ripard a écrit :
Hi,
I started to review it, but it's probably best to discuss it here.
On Thu, Jan 30, 2025 at 01:08:56PM +0000, Florent Tomasin wrote:
Hi,
This is a patch series covering the support for protected mode execution in Mali Panthor CSF kernel driver.
The Mali CSF GPUs come with the support for protected mode execution at the HW level. This feature requires two main changes in the kernel driver:
- Configure the GPU with a protected buffer. The system must provide a DMA heap from which the driver can allocate a protected buffer. It can be a carved-out memory or dynamically allocated protected memory region. Some system includes a trusted FW which is in charge of the protected memory. Since this problem is integration specific, the Mali Panthor CSF kernel driver must import the protected memory from a device specific exporter.
Why do you need a heap for it in the first place? My understanding of your series is that you have a carved out memory region somewhere, and you want to allocate from that carved out memory region your buffers.
How is that any different from using a reserved-memory region, adding the reserved-memory property to the GPU device and doing all your allocation through the usual dma_alloc_* API?
How do you then multiplex this region so it can be shared between GPU/Camera/Display/Codec drivers and also userspace ?
You could point all the devices to the same reserved memory region, and they would all allocate from there, including for their userspace-facing allocations.
I get that using memory region is somewhat more of an HW description, and aligned with what a DT is supposed to describe. One of the challenge is that Mediatek heap proposal endup calling into their TEE, meaning knowing the region is not that useful. You actually need the TEE APP guid and its IPC protocol. If we can dell drivers to use a head instead, we can abstract that SoC specific complexity. I believe each allocated addressed has to be mapped to a zone, and that can only be done in the secure application. I can imagine similar needs when the protection is done using some sort of a VM / hypervisor.
Nicolas
Also, how the secure memory is allocted / obtained is a process that can vary a lot between SoC, so implementation details assumption should not be coded in the driver.
But yeah, we agree there, it's also the point I was trying to make :)
Maxime
linaro-mm-sig@lists.linaro.org
-
Maxime Ripard -
Nicolas Dufresne