On Tue, Aug 16, 2022 at 11:20:50AM +0000, Olivier Masse wrote:
On ven., 2022-08-12 at 17:39 +0100, Brian Starkey wrote:
On Mon, Aug 08, 2022 at 02:39:53PM +0000, Olivier Masse wrote:
On ven., 2022-08-05 at 16:41 +0100, Brian Starkey wrote:
On Fri, Aug 05, 2022 at 03:53:28PM +0200, Olivier Masse wrote:
.. snip
+static struct sg_table *secure_heap_map_dma_buf(struct dma_buf_attachment *attachment,
enumdma_data_direction direction) +{
struct secure_heap_attachment *a = attachment->priv;return a->table;I think you still need to implement mapping and unmapping using the DMA APIs. For example devices might be behind IOMMUs and the buffer will need mapping into the IOMMU.
Devices that will need access to the buffer must be in secure. The tee driver will only need the scatter-list table to get dma address and len. Mapping will be done in the TEE. Please find tee_shm_register_fd in the following commit
https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com...
This patch need to be up-streamed as well.
Interesting, that's not how the devices I've worked on operated.
Are you saying that you have to have a display controller driver running in the TEE to display one of these buffers?
In fact the display controller is managing 3 plans : UI, PiP and video. The video plan is protected in secure as you can see on slide 11: https://static.linaro.org/connect/san19/presentations/san19-107.pdf
The DCSS (display controller) is able to read from the protected secure heap and composition result is send directly to the HDMI/HDCP port.
But it sounds like the DCSS driver is running in non-trusted Linux?
If everything needs to be in the TEE, then why even have these buffers allocated by non-secure Linux at all?
The TEE is only doing decryption using the HW Crypto Accelerator (CAAM). The CAAM will read from a non protected encrypted buffer to write clear content to a secure buffer allocated with DMABUF and mapped in secure by OPTEE OS.
I would have expected there to be HW enforcement of buffer access, but for the display driver to be in non-secure Linux. That's how TZMP1 works: https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fstatic.lin...
Looking at this SDP presentation, that also seems to be the case there: https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fstatic.lin...
Indeed, TZMP1 is similar to our implementation.
Based on those presentations, I think this heap should be implementing map_dma_buf in the "normal" way, using the DMA API to map the buffer to the device. It's up to the TEE and HW firewall to prevent access to those mappings from non-secure devices.
In fact, our devices (VPU and DCSS) do not need any mapping, but only the physical address of buffers which need to be contiguous.
That's not how dma-buf or the DMA APIs work though - you should use dma_map_sgtable and let the DMA API take care of whether a mapping is needed or not.
The VPU decoder, run by the CPU, read video meta data from a non protected buffer and send physical memory address of encoded buffer to the VPU HW. As well, the DCSS get physical address of contiguous decoded video buffer to do the composition.
Can you share the DCSS side of this? Maybe that will help to clear it up.
Thanks, -Brian
My understanding is:
- The memory region should never be mapped or accessed from the host CPU. This is not a security requirement - the CPU will be denied access by whatever hardware is enforcing security - but any CPU accesses will fail, so there is no point in ever having a CPU mapping.
agree with that.
- The allocated buffers _should_ be mapped to devices via
map_dma_buf. Again the HW enforcement will prevent access from devices which aren't permitted access, but for example a display controller may be allowed to read the secure buffer, composite it with other buffers, and display it on the screen.
yes, in could be done for a more generic implementation.
Am I wrong? Even if SDP doesn't work this way, I think we should make the heap as generic as possible so that it can work with different secure video implementations.
.. snip
alright, I get your point
+RESERVEDMEM_OF_DECLARE(secure_heap, "linaro,secure-heap", rmem_secure_heap_setup);
Is there anything linaro-specific about this? Could it be linux,secure-heap?
for now, it's specific to Linaro OPTEE OS. but in a more generic way, it could be linux,unmapped-heap ?
If these buffers can never be mapped, not to the CPU nor to devices, then actually I don't see why it should be a dma-buf heap at all.
If this is just an interface to associate some identifier (in this case an fd) with a region of physical address space, then why is it useful to pretend that it's a dma-buf, if none of the dma-buf operations actually do anything?
in our previous implementation, we were using unmapped ION buffer to be able to send an opaque fd to the TEE driver which could then be mapped in secure by OPTEE. Transitioning from ION to DMABUF heaps, our retaining option was to create a new heap type.
Best regards, Olivier