| Message ID | 20230911023038.30649-6-yong.wu@mediatek.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/9] dma-buf: heaps: Deduplicate docs and adopt common format | expand |
On Mon, 2023-09-11 at 11:29 +0200, AngeloGioacchino Del Regno wrote: > Il 11/09/23 04:30, Yong Wu ha scritto: > > The TEE probe later than dma-buf heap, and PROBE_DEDER doesn't work > > here since this is not a platform driver, therefore initialise the > > TEE > > context/session while we allocate the first secure buffer. > > > > Signed-off-by: Yong Wu <yong.wu@mediatek.com> > > --- > > drivers/dma-buf/heaps/mtk_secure_heap.c | 61 > > +++++++++++++++++++++++++ > > 1 file changed, 61 insertions(+) > > > > diff --git a/drivers/dma-buf/heaps/mtk_secure_heap.c b/drivers/dma- > > buf/heaps/mtk_secure_heap.c > > index bbf1c8dce23e..e3da33a3d083 100644 > > --- a/drivers/dma-buf/heaps/mtk_secure_heap.c > > +++ b/drivers/dma-buf/heaps/mtk_secure_heap.c > > @@ -10,6 +10,12 @@ > > #include <linux/err.h> > > #include <linux/module.h> > > #include <linux/slab.h> > > +#include <linux/tee_drv.h> > > +#include <linux/uuid.h> > > + > > +#define TZ_TA_MEM_UUID "4477588a-8476-11e2-ad15- > > e41f1390d676" > > + > > Is this UUID the same for all SoCs and all TZ versions? Yes. It is the same for all SoCs and all TZ versions currently. > > Thanks, > Angelo > > > > +#define MTK_TEE_PARAM_NUM 4 > > > > /* > > * MediaTek secure (chunk) memory type > > @@ -28,17 +34,72 @@ struct mtk_secure_heap_buffer { > > struct mtk_secure_heap { > > const char *name; > > const enum kree_mem_type mem_type; > > + u32 mem_session; > > + struct tee_context *tee_ctx; > > }; > > > > +static int mtk_optee_ctx_match(struct tee_ioctl_version_data *ver, > > const void *data) > > +{ > > + return ver->impl_id == TEE_IMPL_ID_OPTEE; > > +} > > + > > +static int mtk_kree_secure_session_init(struct mtk_secure_heap > > *sec_heap) > > +{ > > + struct tee_param t_param[MTK_TEE_PARAM_NUM] = {0}; > > + struct tee_ioctl_open_session_arg arg = {0}; > > + uuid_t ta_mem_uuid; > > + int ret; > > + > > + sec_heap->tee_ctx = tee_client_open_context(NULL, > > mtk_optee_ctx_match, > > + NULL, NULL); > > + if (IS_ERR(sec_heap->tee_ctx)) { > > + pr_err("%s: open context failed, ret=%ld\n", sec_heap- > > >name, > > + PTR_ERR(sec_heap->tee_ctx)); > > + return -ENODEV; > > + } > > + > > + arg.num_params = MTK_TEE_PARAM_NUM; > > + arg.clnt_login = TEE_IOCTL_LOGIN_PUBLIC; > > + ret = uuid_parse(TZ_TA_MEM_UUID, &ta_mem_uuid); > > + if (ret) > > + goto close_context; > > + memcpy(&arg.uuid, &ta_mem_uuid.b, sizeof(ta_mem_uuid)); > > + > > + ret = tee_client_open_session(sec_heap->tee_ctx, &arg, > > t_param); > > + if (ret < 0 || arg.ret) { > > + pr_err("%s: open session failed, ret=%d:%d\n", > > + sec_heap->name, ret, arg.ret); > > + ret = -EINVAL; > > + goto close_context; > > + } > > + sec_heap->mem_session = arg.session; > > + return 0; > > + > > +close_context: > > + tee_client_close_context(sec_heap->tee_ctx); > > + return ret; > > +} > > + > > static struct dma_buf * > > mtk_sec_heap_allocate(struct dma_heap *heap, size_t size, > > unsigned long fd_flags, unsigned long heap_flags) > > { > > + struct mtk_secure_heap *sec_heap = dma_heap_get_drvdata(heap); > > struct mtk_secure_heap_buffer *sec_buf; > > DEFINE_DMA_BUF_EXPORT_INFO(exp_info); > > struct dma_buf *dmabuf; > > int ret; > > > > + /* > > + * TEE probe may be late. Initialise the secure session in the > > first > > + * allocating secure buffer. > > + */ > > + if (!sec_heap->mem_session) { > > + ret = mtk_kree_secure_session_init(sec_heap); > > + if (ret) > > + return ERR_PTR(ret); > > + } > > + > > sec_buf = kzalloc(sizeof(*sec_buf), GFP_KERNEL); > > if (!sec_buf) > > return ERR_PTR(-ENOMEM); > >
On Mon, Sep 25, 2023 at 12:49:50PM +0000, Yong Wu (吴勇) wrote: > On Tue, 2023-09-12 at 11:32 +0200, AngeloGioacchino Del Regno wrote: > > Il 12/09/23 08:17, Yong Wu (吴勇) ha scritto: > > > On Mon, 2023-09-11 at 11:29 +0200, AngeloGioacchino Del Regno > > > wrote: > > > > Il 11/09/23 04:30, Yong Wu ha scritto: > > > > > The TEE probe later than dma-buf heap, and PROBE_DEDER doesn't > > > > > work > > > > > here since this is not a platform driver, therefore initialise > > > > > the > > > > > TEE > > > > > context/session while we allocate the first secure buffer. > > > > > > > > > > Signed-off-by: Yong Wu <yong.wu@mediatek.com> > > > > > --- > > > > > drivers/dma-buf/heaps/mtk_secure_heap.c | 61 > > > > > +++++++++++++++++++++++++ > > > > > 1 file changed, 61 insertions(+) > > > > > > > > > > diff --git a/drivers/dma-buf/heaps/mtk_secure_heap.c > > > > > b/drivers/dma- > > > > > buf/heaps/mtk_secure_heap.c > > > > > index bbf1c8dce23e..e3da33a3d083 100644 > > > > > --- a/drivers/dma-buf/heaps/mtk_secure_heap.c > > > > > +++ b/drivers/dma-buf/heaps/mtk_secure_heap.c > > > > > @@ -10,6 +10,12 @@ > > > > > #include <linux/err.h> > > > > > #include <linux/module.h> > > > > > #include <linux/slab.h> > > > > > +#include <linux/tee_drv.h> > > > > > +#include <linux/uuid.h> > > > > > + > > > > > +#define TZ_TA_MEM_UUID "4477588a-8476-11e2-ad15- > > > > > e41f1390d676" > > > > > + > > > > > > > > Is this UUID the same for all SoCs and all TZ versions? > > > > > > Yes. It is the same for all SoCs and all TZ versions currently. > > > > > > > That's good news! > > > > Is this UUID used in any userspace component? (example: Android > > HALs?) > > No. Userspace never use it. If userspace would like to allocate this > secure buffer, it can achieve through the existing dmabuf IOCTL via > /dev/dma_heap/mtk_svp node. > In general I think as mentioned elsewhere in comments, that there isn't that much here that seems to be unique for MediaTek in this patch series, so I think it worth to see whether this whole patch set can be made more generic. Having said that, the UUID is always unique for a certain Trusted Application. So, it's not entirely true saying that the UUID is the same for all SoCs and all TrustZone versions. It might be true for a family of MediaTek devices and the TEE in use, but not generically. So, if we need to differentiate between different TA implementations, then we need different UUIDs. If it would be possible to make this patch set generic, then it sounds like a single UUID would be sufficient, but that would imply that all TA's supporting such a generic UUID would be implemented the same from an API point of view. Which also means that for example Trusted Application function ID's needs to be the same etc. Not impossible to achieve, but still not easy (different TEE follows different specifications) and it's not typically something we've done in the past. Unfortunately there is no standardized database of TA's describing what they implement and support. As an alternative, we could implement a query call in the TEE answering, "What UUID does your TA have that implements secure unmapped heap?". I.e., something that reminds of a lookup table. Then we wouldn't have to carry this in UAPI, DT or anywhere else.
Le 27/09/2023 à 15:46, Joakim Bech a écrit : > On Mon, Sep 25, 2023 at 12:49:50PM +0000, Yong Wu (吴勇) wrote: >> On Tue, 2023-09-12 at 11:32 +0200, AngeloGioacchino Del Regno wrote: >>> Il 12/09/23 08:17, Yong Wu (吴勇) ha scritto: >>>> On Mon, 2023-09-11 at 11:29 +0200, AngeloGioacchino Del Regno >>>> wrote: >>>>> Il 11/09/23 04:30, Yong Wu ha scritto: >>>>>> The TEE probe later than dma-buf heap, and PROBE_DEDER doesn't >>>>>> work >>>>>> here since this is not a platform driver, therefore initialise >>>>>> the >>>>>> TEE >>>>>> context/session while we allocate the first secure buffer. >>>>>> >>>>>> Signed-off-by: Yong Wu <yong.wu@mediatek.com> >>>>>> --- >>>>>> drivers/dma-buf/heaps/mtk_secure_heap.c | 61 >>>>>> +++++++++++++++++++++++++ >>>>>> 1 file changed, 61 insertions(+) >>>>>> >>>>>> diff --git a/drivers/dma-buf/heaps/mtk_secure_heap.c >>>>>> b/drivers/dma- >>>>>> buf/heaps/mtk_secure_heap.c >>>>>> index bbf1c8dce23e..e3da33a3d083 100644 >>>>>> --- a/drivers/dma-buf/heaps/mtk_secure_heap.c >>>>>> +++ b/drivers/dma-buf/heaps/mtk_secure_heap.c >>>>>> @@ -10,6 +10,12 @@ >>>>>> #include <linux/err.h> >>>>>> #include <linux/module.h> >>>>>> #include <linux/slab.h> >>>>>> +#include <linux/tee_drv.h> >>>>>> +#include <linux/uuid.h> >>>>>> + >>>>>> +#define TZ_TA_MEM_UUID "4477588a-8476-11e2-ad15- >>>>>> e41f1390d676" >>>>>> + >>>>> Is this UUID the same for all SoCs and all TZ versions? >>>> Yes. It is the same for all SoCs and all TZ versions currently. >>>> >>> That's good news! >>> >>> Is this UUID used in any userspace component? (example: Android >>> HALs?) >> No. Userspace never use it. If userspace would like to allocate this >> secure buffer, it can achieve through the existing dmabuf IOCTL via >> /dev/dma_heap/mtk_svp node. >> > In general I think as mentioned elsewhere in comments, that there isn't > that much here that seems to be unique for MediaTek in this patch > series, so I think it worth to see whether this whole patch set can be > made more generic. Having said that, the UUID is always unique for a > certain Trusted Application. So, it's not entirely true saying that the > UUID is the same for all SoCs and all TrustZone versions. It might be > true for a family of MediaTek devices and the TEE in use, but not > generically. > > So, if we need to differentiate between different TA implementations, > then we need different UUIDs. If it would be possible to make this patch > set generic, then it sounds like a single UUID would be sufficient, but > that would imply that all TA's supporting such a generic UUID would be > implemented the same from an API point of view. Which also means that > for example Trusted Application function ID's needs to be the same etc. > Not impossible to achieve, but still not easy (different TEE follows > different specifications) and it's not typically something we've done in > the past. > > Unfortunately there is no standardized database of TA's describing what > they implement and support. > > As an alternative, we could implement a query call in the TEE answering, > "What UUID does your TA have that implements secure unmapped heap?". > I.e., something that reminds of a lookup table. Then we wouldn't have to > carry this in UAPI, DT or anywhere else. Joakim does a TA could offer a generic API and hide the hardware specific details (like kernel uAPI does for drivers) ? Aside that question I wonder what are the needs to perform a 'secure' playback. I have in mind 2 requirements: - secure memory regions, which means configure the hardware to ensure that only dedicated hardware blocks and read or write into it. - set hardware blocks in secure modes so they access to secure memory. Do you see something else ? Regards, Benjamin >
On Wed, Sep 27, 2023 at 6:46 AM Joakim Bech <joakim.bech@linaro.org> wrote: > > On Mon, Sep 25, 2023 at 12:49:50PM +0000, Yong Wu (吴勇) wrote: > > On Tue, 2023-09-12 at 11:32 +0200, AngeloGioacchino Del Regno wrote: > > > Il 12/09/23 08:17, Yong Wu (吴勇) ha scritto: > > > > On Mon, 2023-09-11 at 11:29 +0200, AngeloGioacchino Del Regno > > > > wrote: > > > > > Il 11/09/23 04:30, Yong Wu ha scritto: > > > > > > The TEE probe later than dma-buf heap, and PROBE_DEDER doesn't > > > > > > work > > > > > > here since this is not a platform driver, therefore initialise > > > > > > the > > > > > > TEE > > > > > > context/session while we allocate the first secure buffer. > > > > > > > > > > > > Signed-off-by: Yong Wu <yong.wu@mediatek.com> > > > > > > --- > > > > > > drivers/dma-buf/heaps/mtk_secure_heap.c | 61 > > > > > > +++++++++++++++++++++++++ > > > > > > 1 file changed, 61 insertions(+) > > > > > > > > > > > > diff --git a/drivers/dma-buf/heaps/mtk_secure_heap.c > > > > > > b/drivers/dma- > > > > > > buf/heaps/mtk_secure_heap.c > > > > > > index bbf1c8dce23e..e3da33a3d083 100644 > > > > > > --- a/drivers/dma-buf/heaps/mtk_secure_heap.c > > > > > > +++ b/drivers/dma-buf/heaps/mtk_secure_heap.c > > > > > > @@ -10,6 +10,12 @@ > > > > > > #include <linux/err.h> > > > > > > #include <linux/module.h> > > > > > > #include <linux/slab.h> > > > > > > +#include <linux/tee_drv.h> > > > > > > +#include <linux/uuid.h> > > > > > > + > > > > > > +#define TZ_TA_MEM_UUID "4477588a-8476-11e2-ad15- > > > > > > e41f1390d676" > > > > > > + > > > > > > > > > > Is this UUID the same for all SoCs and all TZ versions? > > > > > > > > Yes. It is the same for all SoCs and all TZ versions currently. > > > > > > > > > > That's good news! > > > > > > Is this UUID used in any userspace component? (example: Android > > > HALs?) > > > > No. Userspace never use it. If userspace would like to allocate this > > secure buffer, it can achieve through the existing dmabuf IOCTL via > > /dev/dma_heap/mtk_svp node. > > > In general I think as mentioned elsewhere in comments, that there isn't > that much here that seems to be unique for MediaTek in this patch > series, so I think it worth to see whether this whole patch set can be > made more generic. Having said that, the UUID is always unique for a > certain Trusted Application. So, it's not entirely true saying that the > UUID is the same for all SoCs and all TrustZone versions. It might be > true for a family of MediaTek devices and the TEE in use, but not > generically. > > So, if we need to differentiate between different TA implementations, > then we need different UUIDs. If it would be possible to make this patch > set generic, then it sounds like a single UUID would be sufficient, but > that would imply that all TA's supporting such a generic UUID would be > implemented the same from an API point of view. Which also means that > for example Trusted Application function ID's needs to be the same etc. > Not impossible to achieve, but still not easy (different TEE follows > different specifications) and it's not typically something we've done in > the past. > > Unfortunately there is no standardized database of TA's describing what > they implement and support. > > As an alternative, we could implement a query call in the TEE answering, > "What UUID does your TA have that implements secure unmapped heap?". > I.e., something that reminds of a lookup table. Then we wouldn't have to > carry this in UAPI, DT or anywhere else. > I think that's a good idea. If we add kernel APIs to the tee for opening a session for secure memory allocation and for performing the allocation, then the UUID, TA commands and TA parameters can all be decided upon in the TEE specific driver and the code in dma-heap becomes generic. > -- > // Regards > Joakim > > > > > > If it is (and I somehow expect that it is), then this definition > > > should go > > > to a UAPI header, as suggested by Christian. > > > > > > Cheers! > > > > > > > > > > > > > Thanks, > > > > > Angelo > > > > > > > > > > > > > > > > +#define MTK_TEE_PARAM_NUM 4 > > > > > > > > > > > > /* > > > > > > * MediaTek secure (chunk) memory type > > > > > > @@ -28,17 +34,72 @@ struct mtk_secure_heap_buffer { > > > > > > struct mtk_secure_heap { > > > > > > const char *name; > > > > > > const enum kree_mem_type mem_type; > > > > > > + u32 mem_session; > > > > > > + struct tee_context *tee_ctx; > > > > > > }; > > > > > > > > > > > > +static int mtk_optee_ctx_match(struct tee_ioctl_version_data > > > > > > *ver, > > > > > > const void *data) > > > > > > +{ > > > > > > + return ver->impl_id == TEE_IMPL_ID_OPTEE; > > > > > > +} > > > > > > + > > > > > > +static int mtk_kree_secure_session_init(struct mtk_secure_heap > > > > > > *sec_heap) > > > > > > +{ > > > > > > + struct tee_param t_param[MTK_TEE_PARAM_NUM] = {0}; > > > > > > + struct tee_ioctl_open_session_arg arg = {0}; > > > > > > + uuid_t ta_mem_uuid; > > > > > > + int ret; > > > > > > + > > > > > > + sec_heap->tee_ctx = tee_client_open_context(NULL, > > > > > > mtk_optee_ctx_match, > > > > > > + NULL, > > > > > > NULL); > > > > > > + if (IS_ERR(sec_heap->tee_ctx)) { > > > > > > + pr_err("%s: open context failed, ret=%ld\n", > > > > > > sec_heap- > > > > > > > name, > > > > > > > > > > > > + PTR_ERR(sec_heap->tee_ctx)); > > > > > > + return -ENODEV; > > > > > > + } > > > > > > + > > > > > > + arg.num_params = MTK_TEE_PARAM_NUM; > > > > > > + arg.clnt_login = TEE_IOCTL_LOGIN_PUBLIC; > > > > > > + ret = uuid_parse(TZ_TA_MEM_UUID, &ta_mem_uuid); > > > > > > + if (ret) > > > > > > + goto close_context; > > > > > > + memcpy(&arg.uuid, &ta_mem_uuid.b, sizeof(ta_mem_uuid)); > > > > > > + > > > > > > + ret = tee_client_open_session(sec_heap->tee_ctx, &arg, > > > > > > t_param); > > > > > > + if (ret < 0 || arg.ret) { > > > > > > + pr_err("%s: open session failed, ret=%d:%d\n", > > > > > > + sec_heap->name, ret, arg.ret); > > > > > > + ret = -EINVAL; > > > > > > + goto close_context; > > > > > > + } > > > > > > + sec_heap->mem_session = arg.session; > > > > > > + return 0; > > > > > > + > > > > > > +close_context: > > > > > > + tee_client_close_context(sec_heap->tee_ctx); > > > > > > + return ret; > > > > > > +} > > > > > > + > > > > > > static struct dma_buf * > > > > > > mtk_sec_heap_allocate(struct dma_heap *heap, size_t size, > > > > > > unsigned long fd_flags, unsigned long > > > > > > heap_flags) > > > > > > { > > > > > > + struct mtk_secure_heap *sec_heap = > > > > > > dma_heap_get_drvdata(heap); > > > > > > struct mtk_secure_heap_buffer *sec_buf; > > > > > > DEFINE_DMA_BUF_EXPORT_INFO(exp_info); > > > > > > struct dma_buf *dmabuf; > > > > > > int ret; > > > > > > > > > > > > + /* > > > > > > + * TEE probe may be late. Initialise the secure session > > > > > > in the > > > > > > first > > > > > > + * allocating secure buffer. > > > > > > + */ > > > > > > + if (!sec_heap->mem_session) { > > > > > > + ret = mtk_kree_secure_session_init(sec_heap); > > > > > > + if (ret) > > > > > > + return ERR_PTR(ret); > > > > > > + } > > > > > > + > > > > > > sec_buf = kzalloc(sizeof(*sec_buf), GFP_KERNEL); > > > > > > if (!sec_buf) > > > > > > return ERR_PTR(-ENOMEM); > > > > > > > > > > > > > > > >
On Wed, Sep 27, 2023 at 8:18 AM Benjamin Gaignard <benjamin.gaignard@collabora.com> wrote: > > > Le 27/09/2023 à 15:46, Joakim Bech a écrit : > > On Mon, Sep 25, 2023 at 12:49:50PM +0000, Yong Wu (吴勇) wrote: > >> On Tue, 2023-09-12 at 11:32 +0200, AngeloGioacchino Del Regno wrote: > >>> Il 12/09/23 08:17, Yong Wu (吴勇) ha scritto: > >>>> On Mon, 2023-09-11 at 11:29 +0200, AngeloGioacchino Del Regno > >>>> wrote: > >>>>> Il 11/09/23 04:30, Yong Wu ha scritto: > >>>>>> The TEE probe later than dma-buf heap, and PROBE_DEDER doesn't > >>>>>> work > >>>>>> here since this is not a platform driver, therefore initialise > >>>>>> the > >>>>>> TEE > >>>>>> context/session while we allocate the first secure buffer. > >>>>>> > >>>>>> Signed-off-by: Yong Wu <yong.wu@mediatek.com> > >>>>>> --- > >>>>>> drivers/dma-buf/heaps/mtk_secure_heap.c | 61 > >>>>>> +++++++++++++++++++++++++ > >>>>>> 1 file changed, 61 insertions(+) > >>>>>> > >>>>>> diff --git a/drivers/dma-buf/heaps/mtk_secure_heap.c > >>>>>> b/drivers/dma- > >>>>>> buf/heaps/mtk_secure_heap.c > >>>>>> index bbf1c8dce23e..e3da33a3d083 100644 > >>>>>> --- a/drivers/dma-buf/heaps/mtk_secure_heap.c > >>>>>> +++ b/drivers/dma-buf/heaps/mtk_secure_heap.c > >>>>>> @@ -10,6 +10,12 @@ > >>>>>> #include <linux/err.h> > >>>>>> #include <linux/module.h> > >>>>>> #include <linux/slab.h> > >>>>>> +#include <linux/tee_drv.h> > >>>>>> +#include <linux/uuid.h> > >>>>>> + > >>>>>> +#define TZ_TA_MEM_UUID "4477588a-8476-11e2-ad15- > >>>>>> e41f1390d676" > >>>>>> + > >>>>> Is this UUID the same for all SoCs and all TZ versions? > >>>> Yes. It is the same for all SoCs and all TZ versions currently. > >>>> > >>> That's good news! > >>> > >>> Is this UUID used in any userspace component? (example: Android > >>> HALs?) > >> No. Userspace never use it. If userspace would like to allocate this > >> secure buffer, it can achieve through the existing dmabuf IOCTL via > >> /dev/dma_heap/mtk_svp node. > >> > > In general I think as mentioned elsewhere in comments, that there isn't > > that much here that seems to be unique for MediaTek in this patch > > series, so I think it worth to see whether this whole patch set can be > > made more generic. Having said that, the UUID is always unique for a > > certain Trusted Application. So, it's not entirely true saying that the > > UUID is the same for all SoCs and all TrustZone versions. It might be > > true for a family of MediaTek devices and the TEE in use, but not > > generically. > > > > So, if we need to differentiate between different TA implementations, > > then we need different UUIDs. If it would be possible to make this patch > > set generic, then it sounds like a single UUID would be sufficient, but > > that would imply that all TA's supporting such a generic UUID would be > > implemented the same from an API point of view. Which also means that > > for example Trusted Application function ID's needs to be the same etc. > > Not impossible to achieve, but still not easy (different TEE follows > > different specifications) and it's not typically something we've done in > > the past. > > > > Unfortunately there is no standardized database of TA's describing what > > they implement and support. > > > > As an alternative, we could implement a query call in the TEE answering, > > "What UUID does your TA have that implements secure unmapped heap?". > > I.e., something that reminds of a lookup table. Then we wouldn't have to > > carry this in UAPI, DT or anywhere else. > > Joakim does a TA could offer a generic API and hide the hardware specific > details (like kernel uAPI does for drivers) ? It would have to go through another layer (like the tee driver) to be a generic API. The main issue with TAs is that they have UUIDs you need to connect to and specific codes for each function; so we should abstract at a layer above where those exist in the dma-heap code. > > Aside that question I wonder what are the needs to perform a 'secure' playback. > I have in mind 2 requirements: > - secure memory regions, which means configure the hardware to ensure that only > dedicated hardware blocks and read or write into it. > - set hardware blocks in secure modes so they access to secure memory. > Do you see something else ? This is more or less what is required, but this is out of scope for the Linux kernel since it can't be trusted to do these things...this is all done in firmware or the TEE itself. > > Regards, > Benjamin > > >
Le 27/09/2023 à 20:56, Jeffrey Kardatzke a écrit : > On Wed, Sep 27, 2023 at 8:18 AM Benjamin Gaignard > <benjamin.gaignard@collabora.com> wrote: >> >> Le 27/09/2023 à 15:46, Joakim Bech a écrit : >>> On Mon, Sep 25, 2023 at 12:49:50PM +0000, Yong Wu (吴勇) wrote: >>>> On Tue, 2023-09-12 at 11:32 +0200, AngeloGioacchino Del Regno wrote: >>>>> Il 12/09/23 08:17, Yong Wu (吴勇) ha scritto: >>>>>> On Mon, 2023-09-11 at 11:29 +0200, AngeloGioacchino Del Regno >>>>>> wrote: >>>>>>> Il 11/09/23 04:30, Yong Wu ha scritto: >>>>>>>> The TEE probe later than dma-buf heap, and PROBE_DEDER doesn't >>>>>>>> work >>>>>>>> here since this is not a platform driver, therefore initialise >>>>>>>> the >>>>>>>> TEE >>>>>>>> context/session while we allocate the first secure buffer. >>>>>>>> >>>>>>>> Signed-off-by: Yong Wu <yong.wu@mediatek.com> >>>>>>>> --- >>>>>>>> drivers/dma-buf/heaps/mtk_secure_heap.c | 61 >>>>>>>> +++++++++++++++++++++++++ >>>>>>>> 1 file changed, 61 insertions(+) >>>>>>>> >>>>>>>> diff --git a/drivers/dma-buf/heaps/mtk_secure_heap.c >>>>>>>> b/drivers/dma- >>>>>>>> buf/heaps/mtk_secure_heap.c >>>>>>>> index bbf1c8dce23e..e3da33a3d083 100644 >>>>>>>> --- a/drivers/dma-buf/heaps/mtk_secure_heap.c >>>>>>>> +++ b/drivers/dma-buf/heaps/mtk_secure_heap.c >>>>>>>> @@ -10,6 +10,12 @@ >>>>>>>> #include <linux/err.h> >>>>>>>> #include <linux/module.h> >>>>>>>> #include <linux/slab.h> >>>>>>>> +#include <linux/tee_drv.h> >>>>>>>> +#include <linux/uuid.h> >>>>>>>> + >>>>>>>> +#define TZ_TA_MEM_UUID "4477588a-8476-11e2-ad15- >>>>>>>> e41f1390d676" >>>>>>>> + >>>>>>> Is this UUID the same for all SoCs and all TZ versions? >>>>>> Yes. It is the same for all SoCs and all TZ versions currently. >>>>>> >>>>> That's good news! >>>>> >>>>> Is this UUID used in any userspace component? (example: Android >>>>> HALs?) >>>> No. Userspace never use it. If userspace would like to allocate this >>>> secure buffer, it can achieve through the existing dmabuf IOCTL via >>>> /dev/dma_heap/mtk_svp node. >>>> >>> In general I think as mentioned elsewhere in comments, that there isn't >>> that much here that seems to be unique for MediaTek in this patch >>> series, so I think it worth to see whether this whole patch set can be >>> made more generic. Having said that, the UUID is always unique for a >>> certain Trusted Application. So, it's not entirely true saying that the >>> UUID is the same for all SoCs and all TrustZone versions. It might be >>> true for a family of MediaTek devices and the TEE in use, but not >>> generically. >>> >>> So, if we need to differentiate between different TA implementations, >>> then we need different UUIDs. If it would be possible to make this patch >>> set generic, then it sounds like a single UUID would be sufficient, but >>> that would imply that all TA's supporting such a generic UUID would be >>> implemented the same from an API point of view. Which also means that >>> for example Trusted Application function ID's needs to be the same etc. >>> Not impossible to achieve, but still not easy (different TEE follows >>> different specifications) and it's not typically something we've done in >>> the past. >>> >>> Unfortunately there is no standardized database of TA's describing what >>> they implement and support. >>> >>> As an alternative, we could implement a query call in the TEE answering, >>> "What UUID does your TA have that implements secure unmapped heap?". >>> I.e., something that reminds of a lookup table. Then we wouldn't have to >>> carry this in UAPI, DT or anywhere else. >> Joakim does a TA could offer a generic API and hide the hardware specific >> details (like kernel uAPI does for drivers) ? > It would have to go through another layer (like the tee driver) to be > a generic API. The main issue with TAs is that they have UUIDs you > need to connect to and specific codes for each function; so we should > abstract at a layer above where those exist in the dma-heap code. >> Aside that question I wonder what are the needs to perform a 'secure' playback. >> I have in mind 2 requirements: >> - secure memory regions, which means configure the hardware to ensure that only >> dedicated hardware blocks and read or write into it. >> - set hardware blocks in secure modes so they access to secure memory. >> Do you see something else ? > This is more or less what is required, but this is out of scope for > the Linux kernel since it can't be trusted to do these things...this > is all done in firmware or the TEE itself. Yes kernel can't be trusted to do these things but know what we need could help to define a API for a generic TA. Just to brainstorm on mailing list: What about a TA API like TA_secure_memory_region() and TA_unsecure_memory_region() with parameters like: - device identifier (an ID or compatible string maybe) - memory region (physical address, size, offset) - requested access rights (read, write) and on kernel side a IOMMU driver because it basically have all this information already (device attachment, kernel map/unmap). In my mind it sound like a solution to limit the impact (new controls, new memory type) inside v4l2. Probably we won't need new heap either. All hardware dedicated implementations could live inside the TA which can offer a generic API. >> Regards, >> Benjamin >>
On Thu, Sep 28, 2023 at 1:30 AM Benjamin Gaignard <benjamin.gaignard@collabora.com> wrote: > > > Le 27/09/2023 à 20:56, Jeffrey Kardatzke a écrit : > > On Wed, Sep 27, 2023 at 8:18 AM Benjamin Gaignard > > <benjamin.gaignard@collabora.com> wrote: > >> > >> Le 27/09/2023 à 15:46, Joakim Bech a écrit : > >>> On Mon, Sep 25, 2023 at 12:49:50PM +0000, Yong Wu (吴勇) wrote: > >>>> On Tue, 2023-09-12 at 11:32 +0200, AngeloGioacchino Del Regno wrote: > >>>>> Il 12/09/23 08:17, Yong Wu (吴勇) ha scritto: > >>>>>> On Mon, 2023-09-11 at 11:29 +0200, AngeloGioacchino Del Regno > >>>>>> wrote: > >>>>>>> Il 11/09/23 04:30, Yong Wu ha scritto: > >>>>>>>> The TEE probe later than dma-buf heap, and PROBE_DEDER doesn't > >>>>>>>> work > >>>>>>>> here since this is not a platform driver, therefore initialise > >>>>>>>> the > >>>>>>>> TEE > >>>>>>>> context/session while we allocate the first secure buffer. > >>>>>>>> > >>>>>>>> Signed-off-by: Yong Wu <yong.wu@mediatek.com> > >>>>>>>> --- > >>>>>>>> drivers/dma-buf/heaps/mtk_secure_heap.c | 61 > >>>>>>>> +++++++++++++++++++++++++ > >>>>>>>> 1 file changed, 61 insertions(+) > >>>>>>>> > >>>>>>>> diff --git a/drivers/dma-buf/heaps/mtk_secure_heap.c > >>>>>>>> b/drivers/dma- > >>>>>>>> buf/heaps/mtk_secure_heap.c > >>>>>>>> index bbf1c8dce23e..e3da33a3d083 100644 > >>>>>>>> --- a/drivers/dma-buf/heaps/mtk_secure_heap.c > >>>>>>>> +++ b/drivers/dma-buf/heaps/mtk_secure_heap.c > >>>>>>>> @@ -10,6 +10,12 @@ > >>>>>>>> #include <linux/err.h> > >>>>>>>> #include <linux/module.h> > >>>>>>>> #include <linux/slab.h> > >>>>>>>> +#include <linux/tee_drv.h> > >>>>>>>> +#include <linux/uuid.h> > >>>>>>>> + > >>>>>>>> +#define TZ_TA_MEM_UUID "4477588a-8476-11e2-ad15- > >>>>>>>> e41f1390d676" > >>>>>>>> + > >>>>>>> Is this UUID the same for all SoCs and all TZ versions? > >>>>>> Yes. It is the same for all SoCs and all TZ versions currently. > >>>>>> > >>>>> That's good news! > >>>>> > >>>>> Is this UUID used in any userspace component? (example: Android > >>>>> HALs?) > >>>> No. Userspace never use it. If userspace would like to allocate this > >>>> secure buffer, it can achieve through the existing dmabuf IOCTL via > >>>> /dev/dma_heap/mtk_svp node. > >>>> > >>> In general I think as mentioned elsewhere in comments, that there isn't > >>> that much here that seems to be unique for MediaTek in this patch > >>> series, so I think it worth to see whether this whole patch set can be > >>> made more generic. Having said that, the UUID is always unique for a > >>> certain Trusted Application. So, it's not entirely true saying that the > >>> UUID is the same for all SoCs and all TrustZone versions. It might be > >>> true for a family of MediaTek devices and the TEE in use, but not > >>> generically. > >>> > >>> So, if we need to differentiate between different TA implementations, > >>> then we need different UUIDs. If it would be possible to make this patch > >>> set generic, then it sounds like a single UUID would be sufficient, but > >>> that would imply that all TA's supporting such a generic UUID would be > >>> implemented the same from an API point of view. Which also means that > >>> for example Trusted Application function ID's needs to be the same etc. > >>> Not impossible to achieve, but still not easy (different TEE follows > >>> different specifications) and it's not typically something we've done in > >>> the past. > >>> > >>> Unfortunately there is no standardized database of TA's describing what > >>> they implement and support. > >>> > >>> As an alternative, we could implement a query call in the TEE answering, > >>> "What UUID does your TA have that implements secure unmapped heap?". > >>> I.e., something that reminds of a lookup table. Then we wouldn't have to > >>> carry this in UAPI, DT or anywhere else. > >> Joakim does a TA could offer a generic API and hide the hardware specific > >> details (like kernel uAPI does for drivers) ? > > It would have to go through another layer (like the tee driver) to be > > a generic API. The main issue with TAs is that they have UUIDs you > > need to connect to and specific codes for each function; so we should > > abstract at a layer above where those exist in the dma-heap code. > >> Aside that question I wonder what are the needs to perform a 'secure' playback. > >> I have in mind 2 requirements: > >> - secure memory regions, which means configure the hardware to ensure that only > >> dedicated hardware blocks and read or write into it. > >> - set hardware blocks in secure modes so they access to secure memory. > >> Do you see something else ? > > This is more or less what is required, but this is out of scope for > > the Linux kernel since it can't be trusted to do these things...this > > is all done in firmware or the TEE itself. > > Yes kernel can't be trusted to do these things but know what we need could help > to define a API for a generic TA. > > Just to brainstorm on mailing list: > What about a TA API like > TA_secure_memory_region() and TA_unsecure_memory_region() with parameters like: > - device identifier (an ID or compatible string maybe) > - memory region (physical address, size, offset) > - requested access rights (read, write) > > and on kernel side a IOMMU driver because it basically have all this information already > (device attachment, kernel map/unmap). > > In my mind it sound like a solution to limit the impact (new controls, new memory type) > inside v4l2. Probably we won't need new heap either. > All hardware dedicated implementations could live inside the TA which can offer a generic > API. The main problem with that type of design is the limitations of TrustZone memory protection. Usually there is a limit to the number of regions you can define for memory protection (and there is on Mediatek). So you can't pass an arbitrary memory region and mark it protected/unprotected at a given time. You need to establish these regions in the firmware instead and then configure those regions for protection in the firmware or the TEE. > > >> Regards, > >> Benjamin > >>
Le 28/09/2023 à 19:48, Jeffrey Kardatzke a écrit : > On Thu, Sep 28, 2023 at 1:30 AM Benjamin Gaignard > <benjamin.gaignard@collabora.com> wrote: >> >> Le 27/09/2023 à 20:56, Jeffrey Kardatzke a écrit : >>> On Wed, Sep 27, 2023 at 8:18 AM Benjamin Gaignard >>> <benjamin.gaignard@collabora.com> wrote: >>>> Le 27/09/2023 à 15:46, Joakim Bech a écrit : >>>>> On Mon, Sep 25, 2023 at 12:49:50PM +0000, Yong Wu (吴勇) wrote: >>>>>> On Tue, 2023-09-12 at 11:32 +0200, AngeloGioacchino Del Regno wrote: >>>>>>> Il 12/09/23 08:17, Yong Wu (吴勇) ha scritto: >>>>>>>> On Mon, 2023-09-11 at 11:29 +0200, AngeloGioacchino Del Regno >>>>>>>> wrote: >>>>>>>>> Il 11/09/23 04:30, Yong Wu ha scritto: >>>>>>>>>> The TEE probe later than dma-buf heap, and PROBE_DEDER doesn't >>>>>>>>>> work >>>>>>>>>> here since this is not a platform driver, therefore initialise >>>>>>>>>> the >>>>>>>>>> TEE >>>>>>>>>> context/session while we allocate the first secure buffer. >>>>>>>>>> >>>>>>>>>> Signed-off-by: Yong Wu <yong.wu@mediatek.com> >>>>>>>>>> --- >>>>>>>>>> drivers/dma-buf/heaps/mtk_secure_heap.c | 61 >>>>>>>>>> +++++++++++++++++++++++++ >>>>>>>>>> 1 file changed, 61 insertions(+) >>>>>>>>>> >>>>>>>>>> diff --git a/drivers/dma-buf/heaps/mtk_secure_heap.c >>>>>>>>>> b/drivers/dma- >>>>>>>>>> buf/heaps/mtk_secure_heap.c >>>>>>>>>> index bbf1c8dce23e..e3da33a3d083 100644 >>>>>>>>>> --- a/drivers/dma-buf/heaps/mtk_secure_heap.c >>>>>>>>>> +++ b/drivers/dma-buf/heaps/mtk_secure_heap.c >>>>>>>>>> @@ -10,6 +10,12 @@ >>>>>>>>>> #include <linux/err.h> >>>>>>>>>> #include <linux/module.h> >>>>>>>>>> #include <linux/slab.h> >>>>>>>>>> +#include <linux/tee_drv.h> >>>>>>>>>> +#include <linux/uuid.h> >>>>>>>>>> + >>>>>>>>>> +#define TZ_TA_MEM_UUID "4477588a-8476-11e2-ad15- >>>>>>>>>> e41f1390d676" >>>>>>>>>> + >>>>>>>>> Is this UUID the same for all SoCs and all TZ versions? >>>>>>>> Yes. It is the same for all SoCs and all TZ versions currently. >>>>>>>> >>>>>>> That's good news! >>>>>>> >>>>>>> Is this UUID used in any userspace component? (example: Android >>>>>>> HALs?) >>>>>> No. Userspace never use it. If userspace would like to allocate this >>>>>> secure buffer, it can achieve through the existing dmabuf IOCTL via >>>>>> /dev/dma_heap/mtk_svp node. >>>>>> >>>>> In general I think as mentioned elsewhere in comments, that there isn't >>>>> that much here that seems to be unique for MediaTek in this patch >>>>> series, so I think it worth to see whether this whole patch set can be >>>>> made more generic. Having said that, the UUID is always unique for a >>>>> certain Trusted Application. So, it's not entirely true saying that the >>>>> UUID is the same for all SoCs and all TrustZone versions. It might be >>>>> true for a family of MediaTek devices and the TEE in use, but not >>>>> generically. >>>>> >>>>> So, if we need to differentiate between different TA implementations, >>>>> then we need different UUIDs. If it would be possible to make this patch >>>>> set generic, then it sounds like a single UUID would be sufficient, but >>>>> that would imply that all TA's supporting such a generic UUID would be >>>>> implemented the same from an API point of view. Which also means that >>>>> for example Trusted Application function ID's needs to be the same etc. >>>>> Not impossible to achieve, but still not easy (different TEE follows >>>>> different specifications) and it's not typically something we've done in >>>>> the past. >>>>> >>>>> Unfortunately there is no standardized database of TA's describing what >>>>> they implement and support. >>>>> >>>>> As an alternative, we could implement a query call in the TEE answering, >>>>> "What UUID does your TA have that implements secure unmapped heap?". >>>>> I.e., something that reminds of a lookup table. Then we wouldn't have to >>>>> carry this in UAPI, DT or anywhere else. >>>> Joakim does a TA could offer a generic API and hide the hardware specific >>>> details (like kernel uAPI does for drivers) ? >>> It would have to go through another layer (like the tee driver) to be >>> a generic API. The main issue with TAs is that they have UUIDs you >>> need to connect to and specific codes for each function; so we should >>> abstract at a layer above where those exist in the dma-heap code. >>>> Aside that question I wonder what are the needs to perform a 'secure' playback. >>>> I have in mind 2 requirements: >>>> - secure memory regions, which means configure the hardware to ensure that only >>>> dedicated hardware blocks and read or write into it. >>>> - set hardware blocks in secure modes so they access to secure memory. >>>> Do you see something else ? >>> This is more or less what is required, but this is out of scope for >>> the Linux kernel since it can't be trusted to do these things...this >>> is all done in firmware or the TEE itself. >> Yes kernel can't be trusted to do these things but know what we need could help >> to define a API for a generic TA. >> >> Just to brainstorm on mailing list: >> What about a TA API like >> TA_secure_memory_region() and TA_unsecure_memory_region() with parameters like: >> - device identifier (an ID or compatible string maybe) >> - memory region (physical address, size, offset) >> - requested access rights (read, write) >> >> and on kernel side a IOMMU driver because it basically have all this information already >> (device attachment, kernel map/unmap). >> >> In my mind it sound like a solution to limit the impact (new controls, new memory type) >> inside v4l2. Probably we won't need new heap either. >> All hardware dedicated implementations could live inside the TA which can offer a generic >> API. > The main problem with that type of design is the limitations of > TrustZone memory protection. Usually there is a limit to the number of > regions you can define for memory protection (and there is on > Mediatek). So you can't pass an arbitrary memory region and mark it > protected/unprotected at a given time. You need to establish these > regions in the firmware instead and then configure those regions for > protection in the firmware or the TEE. The TEE iommu could be aware of these limitations and merge the regions when possible plus we can define a CMA region for each device to limit the secured memory fragmentation. > >>>> Regards, >>>> Benjamin >>>>
Sorry for the delayed reply, needed to get some more info. This really wouldn't be possible due to the limitation on the number of regions...for example only 32 regions can be defined on some SoCs, and you'd run out of regions really fast trying to do this. That's why this is creating heaps for those regions and then allocations are performed within the defined region is the preferred strategy. On Thu, Sep 28, 2023 at 11:54 PM Benjamin Gaignard <benjamin.gaignard@collabora.com> wrote: > > > Le 28/09/2023 à 19:48, Jeffrey Kardatzke a écrit : > > On Thu, Sep 28, 2023 at 1:30 AM Benjamin Gaignard > > <benjamin.gaignard@collabora.com> wrote: > >> > >> Le 27/09/2023 à 20:56, Jeffrey Kardatzke a écrit : > >>> On Wed, Sep 27, 2023 at 8:18 AM Benjamin Gaignard > >>> <benjamin.gaignard@collabora.com> wrote: > >>>> Le 27/09/2023 à 15:46, Joakim Bech a écrit : > >>>>> On Mon, Sep 25, 2023 at 12:49:50PM +0000, Yong Wu (吴勇) wrote: > >>>>>> On Tue, 2023-09-12 at 11:32 +0200, AngeloGioacchino Del Regno wrote: > >>>>>>> Il 12/09/23 08:17, Yong Wu (吴勇) ha scritto: > >>>>>>>> On Mon, 2023-09-11 at 11:29 +0200, AngeloGioacchino Del Regno > >>>>>>>> wrote: > >>>>>>>>> Il 11/09/23 04:30, Yong Wu ha scritto: > >>>>>>>>>> The TEE probe later than dma-buf heap, and PROBE_DEDER doesn't > >>>>>>>>>> work > >>>>>>>>>> here since this is not a platform driver, therefore initialise > >>>>>>>>>> the > >>>>>>>>>> TEE > >>>>>>>>>> context/session while we allocate the first secure buffer. > >>>>>>>>>> > >>>>>>>>>> Signed-off-by: Yong Wu <yong.wu@mediatek.com> > >>>>>>>>>> --- > >>>>>>>>>> drivers/dma-buf/heaps/mtk_secure_heap.c | 61 > >>>>>>>>>> +++++++++++++++++++++++++ > >>>>>>>>>> 1 file changed, 61 insertions(+) > >>>>>>>>>> > >>>>>>>>>> diff --git a/drivers/dma-buf/heaps/mtk_secure_heap.c > >>>>>>>>>> b/drivers/dma- > >>>>>>>>>> buf/heaps/mtk_secure_heap.c > >>>>>>>>>> index bbf1c8dce23e..e3da33a3d083 100644 > >>>>>>>>>> --- a/drivers/dma-buf/heaps/mtk_secure_heap.c > >>>>>>>>>> +++ b/drivers/dma-buf/heaps/mtk_secure_heap.c > >>>>>>>>>> @@ -10,6 +10,12 @@ > >>>>>>>>>> #include <linux/err.h> > >>>>>>>>>> #include <linux/module.h> > >>>>>>>>>> #include <linux/slab.h> > >>>>>>>>>> +#include <linux/tee_drv.h> > >>>>>>>>>> +#include <linux/uuid.h> > >>>>>>>>>> + > >>>>>>>>>> +#define TZ_TA_MEM_UUID "4477588a-8476-11e2-ad15- > >>>>>>>>>> e41f1390d676" > >>>>>>>>>> + > >>>>>>>>> Is this UUID the same for all SoCs and all TZ versions? > >>>>>>>> Yes. It is the same for all SoCs and all TZ versions currently. > >>>>>>>> > >>>>>>> That's good news! > >>>>>>> > >>>>>>> Is this UUID used in any userspace component? (example: Android > >>>>>>> HALs?) > >>>>>> No. Userspace never use it. If userspace would like to allocate this > >>>>>> secure buffer, it can achieve through the existing dmabuf IOCTL via > >>>>>> /dev/dma_heap/mtk_svp node. > >>>>>> > >>>>> In general I think as mentioned elsewhere in comments, that there isn't > >>>>> that much here that seems to be unique for MediaTek in this patch > >>>>> series, so I think it worth to see whether this whole patch set can be > >>>>> made more generic. Having said that, the UUID is always unique for a > >>>>> certain Trusted Application. So, it's not entirely true saying that the > >>>>> UUID is the same for all SoCs and all TrustZone versions. It might be > >>>>> true for a family of MediaTek devices and the TEE in use, but not > >>>>> generically. > >>>>> > >>>>> So, if we need to differentiate between different TA implementations, > >>>>> then we need different UUIDs. If it would be possible to make this patch > >>>>> set generic, then it sounds like a single UUID would be sufficient, but > >>>>> that would imply that all TA's supporting such a generic UUID would be > >>>>> implemented the same from an API point of view. Which also means that > >>>>> for example Trusted Application function ID's needs to be the same etc. > >>>>> Not impossible to achieve, but still not easy (different TEE follows > >>>>> different specifications) and it's not typically something we've done in > >>>>> the past. > >>>>> > >>>>> Unfortunately there is no standardized database of TA's describing what > >>>>> they implement and support. > >>>>> > >>>>> As an alternative, we could implement a query call in the TEE answering, > >>>>> "What UUID does your TA have that implements secure unmapped heap?". > >>>>> I.e., something that reminds of a lookup table. Then we wouldn't have to > >>>>> carry this in UAPI, DT or anywhere else. > >>>> Joakim does a TA could offer a generic API and hide the hardware specific > >>>> details (like kernel uAPI does for drivers) ? > >>> It would have to go through another layer (like the tee driver) to be > >>> a generic API. The main issue with TAs is that they have UUIDs you > >>> need to connect to and specific codes for each function; so we should > >>> abstract at a layer above where those exist in the dma-heap code. > >>>> Aside that question I wonder what are the needs to perform a 'secure' playback. > >>>> I have in mind 2 requirements: > >>>> - secure memory regions, which means configure the hardware to ensure that only > >>>> dedicated hardware blocks and read or write into it. > >>>> - set hardware blocks in secure modes so they access to secure memory. > >>>> Do you see something else ? > >>> This is more or less what is required, but this is out of scope for > >>> the Linux kernel since it can't be trusted to do these things...this > >>> is all done in firmware or the TEE itself. > >> Yes kernel can't be trusted to do these things but know what we need could help > >> to define a API for a generic TA. > >> > >> Just to brainstorm on mailing list: > >> What about a TA API like > >> TA_secure_memory_region() and TA_unsecure_memory_region() with parameters like: > >> - device identifier (an ID or compatible string maybe) > >> - memory region (physical address, size, offset) > >> - requested access rights (read, write) > >> > >> and on kernel side a IOMMU driver because it basically have all this information already > >> (device attachment, kernel map/unmap). > >> > >> In my mind it sound like a solution to limit the impact (new controls, new memory type) > >> inside v4l2. Probably we won't need new heap either. > >> All hardware dedicated implementations could live inside the TA which can offer a generic > >> API. > > The main problem with that type of design is the limitations of > > TrustZone memory protection. Usually there is a limit to the number of > > regions you can define for memory protection (and there is on > > Mediatek). So you can't pass an arbitrary memory region and mark it > > protected/unprotected at a given time. You need to establish these > > regions in the firmware instead and then configure those regions for > > protection in the firmware or the TEE. > > The TEE iommu could be aware of these limitations and merge the regions when possible > plus we can define a CMA region for each device to limit the secured memory fragmentation. > > > > >>>> Regards, > >>>> Benjamin > >>>>
diff --git a/drivers/dma-buf/heaps/mtk_secure_heap.c b/drivers/dma-buf/heaps/mtk_secure_heap.c index bbf1c8dce23e..e3da33a3d083 100644 --- a/drivers/dma-buf/heaps/mtk_secure_heap.c +++ b/drivers/dma-buf/heaps/mtk_secure_heap.c @@ -10,6 +10,12 @@ #include <linux/err.h> #include <linux/module.h> #include <linux/slab.h> +#include <linux/tee_drv.h> +#include <linux/uuid.h> + +#define TZ_TA_MEM_UUID "4477588a-8476-11e2-ad15-e41f1390d676" + +#define MTK_TEE_PARAM_NUM 4 /* * MediaTek secure (chunk) memory type @@ -28,17 +34,72 @@ struct mtk_secure_heap_buffer { struct mtk_secure_heap { const char *name; const enum kree_mem_type mem_type; + u32 mem_session; + struct tee_context *tee_ctx; }; +static int mtk_optee_ctx_match(struct tee_ioctl_version_data *ver, const void *data) +{ + return ver->impl_id == TEE_IMPL_ID_OPTEE; +} + +static int mtk_kree_secure_session_init(struct mtk_secure_heap *sec_heap) +{ + struct tee_param t_param[MTK_TEE_PARAM_NUM] = {0}; + struct tee_ioctl_open_session_arg arg = {0}; + uuid_t ta_mem_uuid; + int ret; + + sec_heap->tee_ctx = tee_client_open_context(NULL, mtk_optee_ctx_match, + NULL, NULL); + if (IS_ERR(sec_heap->tee_ctx)) { + pr_err("%s: open context failed, ret=%ld\n", sec_heap->name, + PTR_ERR(sec_heap->tee_ctx)); + return -ENODEV; + } + + arg.num_params = MTK_TEE_PARAM_NUM; + arg.clnt_login = TEE_IOCTL_LOGIN_PUBLIC; + ret = uuid_parse(TZ_TA_MEM_UUID, &ta_mem_uuid); + if (ret) + goto close_context; + memcpy(&arg.uuid, &ta_mem_uuid.b, sizeof(ta_mem_uuid)); + + ret = tee_client_open_session(sec_heap->tee_ctx, &arg, t_param); + if (ret < 0 || arg.ret) { + pr_err("%s: open session failed, ret=%d:%d\n", + sec_heap->name, ret, arg.ret); + ret = -EINVAL; + goto close_context; + } + sec_heap->mem_session = arg.session; + return 0; + +close_context: + tee_client_close_context(sec_heap->tee_ctx); + return ret; +} + static struct dma_buf * mtk_sec_heap_allocate(struct dma_heap *heap, size_t size, unsigned long fd_flags, unsigned long heap_flags) { + struct mtk_secure_heap *sec_heap = dma_heap_get_drvdata(heap); struct mtk_secure_heap_buffer *sec_buf; DEFINE_DMA_BUF_EXPORT_INFO(exp_info); struct dma_buf *dmabuf; int ret; + /* + * TEE probe may be late. Initialise the secure session in the first + * allocating secure buffer. + */ + if (!sec_heap->mem_session) { + ret = mtk_kree_secure_session_init(sec_heap); + if (ret) + return ERR_PTR(ret); + } + sec_buf = kzalloc(sizeof(*sec_buf), GFP_KERNEL); if (!sec_buf) return ERR_PTR(-ENOMEM);
The TEE probe later than dma-buf heap, and PROBE_DEDER doesn't work here since this is not a platform driver, therefore initialise the TEE context/session while we allocate the first secure buffer. Signed-off-by: Yong Wu <yong.wu@mediatek.com> --- drivers/dma-buf/heaps/mtk_secure_heap.c | 61 +++++++++++++++++++++++++ 1 file changed, 61 insertions(+)