| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <8accca79ebcde0b78a9e3026155543e67df3ed7a.camel@mediatek.com>
Date: Wed, 25 Oct 2023 02:08:46 +0000
From: Jason-JH Lin (林睿祥)
<Jason-JH.Lin@...iatek.com>
To: "fshao@...omium.org" <fshao@...omium.org>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"robh+dt@...nel.org" <robh+dt@...nel.org>,
Singo Chang (張興國)
<Singo.Chang@...iatek.com>,
"linux-mediatek@...ts.infradead.org"
<linux-mediatek@...ts.infradead.org>,
Johnson Wang (王聖鑫)
<Johnson.Wang@...iatek.com>,
"chunkuang.hu@...nel.org" <chunkuang.hu@...nel.org>,
Jason-ch Chen (陳建豪)
<Jason-ch.Chen@...iatek.com>,
Shawn Sung (宋孝謙)
<Shawn.Sung@...iatek.com>,
Nancy Lin (林欣螢) <Nancy.Lin@...iatek.com>,
"jkardatzke@...gle.com" <jkardatzke@...gle.com>,
"conor+dt@...nel.org" <conor+dt@...nel.org>,
Project_Global_Chrome_Upstream_Group
<Project_Global_Chrome_Upstream_Group@...iatek.com>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>,
"krzysztof.kozlowski+dt@...aro.org"
<krzysztof.kozlowski+dt@...aro.org>,
"matthias.bgg@...il.com" <matthias.bgg@...il.com>,
"jassisinghbrar@...il.com" <jassisinghbrar@...il.com>,
"angelogioacchino.delregno@...labora.com"
<angelogioacchino.delregno@...labora.com>
Subject: Re: [PATCH v2 7/9] mailbox: mediatek: Add secure CMDQ driver support
for CMDQ driver
Hi Fei,
Thanks for the reviews.
On Mon, 2023-10-23 at 18:47 +0800, Fei Shao wrote:
>
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> Hi Jason,
>
> On Mon, Oct 23, 2023 at 12:39 PM Jason-JH.Lin <
> jason-jh.lin@...iatek.com> wrote:
> >
> > CMDQ driver will probe a secure CMDQ driver when has_sec flag
> > in platform data is true and its device node in dts has defined a
> > event id of CMDQ_SYNC_TOKEN_SEC_EOF.
> >
> > Secure CMDQ driver support on mt8188 and mt8195 currently.
> > So add a has_sec flag to their driver data to probe it.
> >
> > Signed-off-by: Jason-JH.Lin <jason-jh.lin@...iatek.com>
> > ---
> > drivers/mailbox/mtk-cmdq-mailbox.c | 42
> ++++++++++++++++++++++--
> > include/linux/mailbox/mtk-cmdq-mailbox.h | 11 +++++++
> > 2 files changed, 51 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c
> b/drivers/mailbox/mtk-cmdq-mailbox.c
> > index 3bdfb9a60614..4db5eb76f353 100644
> > --- a/drivers/mailbox/mtk-cmdq-mailbox.c
> > +++ b/drivers/mailbox/mtk-cmdq-mailbox.c
> > @@ -87,6 +87,7 @@ struct gce_plat {
> > u8 shift;
> > bool control_by_sw;
> > bool sw_ddr_en;
> > + bool has_sec;
> > u32 gce_num;
> > };
> >
> > @@ -560,14 +561,23 @@ static int cmdq_probe(struct platform_device
> *pdev)
> > int alias_id = 0;
> > static const char * const clk_name = "gce";
> > static const char * const clk_names[] = { "gce0", "gce1" };
> > + struct resource *res;
> > + struct platform_device *mtk_cmdq_sec;
> > + u32 hwid = 0;
> >
> > cmdq = devm_kzalloc(dev, sizeof(*cmdq), GFP_KERNEL);
> > if (!cmdq)
> > return -ENOMEM;
> >
> > - cmdq->base = devm_platform_ioremap_resource(pdev, 0);
> > - if (IS_ERR(cmdq->base))
> > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> The devm_platform_ioremap_resource() helper was added on purpose [1].
> Please stick to it unless you have a strong reason to re-split it.
>
> [1]: a04f30356e75 ("mailbox: mtk-cmdq: Make use of the helper
> function
> devm_platform_ioremap_resource()")
>
OK, because want to get the pa of GCE by resource like this:
gce_sec_plat.base_pa = res->start;
I'll find another way to do that.
> > + if (!res)
> > + return -EINVAL;
> > +
> > + cmdq->base = devm_ioremap_resource(dev, res);
> > + if (IS_ERR(cmdq->base)) {
> > + dev_err(dev, "failed to ioremap cmdq\n");
> > return PTR_ERR(cmdq->base);
> > + }
> >
> > cmdq->irq = platform_get_irq(pdev, 0);
> > if (cmdq->irq < 0)
> > @@ -585,6 +595,8 @@ static int cmdq_probe(struct platform_device
> *pdev)
> > dev, cmdq->base, cmdq->irq);
> >
> > if (cmdq->pdata->gce_num > 1) {
> > + hwid = of_alias_get_id(dev->of_node, clk_name);
> Why get hwid here while it's only used in the has_sec branch?
Because I want to pass the hwid to secure world and let it know what
GCE pa should use in TEE rather than passing GCE pa.
>
> > +
> > for_each_child_of_node(phandle->parent, node) {
> > alias_id = of_alias_get_id(node, clk_name);
> > if (alias_id >= 0 && alias_id < cmdq-
> >pdata->gce_num) {
> > @@ -653,6 +665,30 @@ static int cmdq_probe(struct platform_device
> *pdev)
> > return err;
> > }
> >
> > + if (cmdq->pdata->has_sec) {
> > + struct cmdq_sec_plat gce_sec_plat;
> > +
> > + if (of_property_read_u32_index(dev->of_node,
> "mediatek,gce-events", 0,
> >
> + &gce_sec_plat.cmdq_eve
> nt) == 0) {
> > + gce_sec_plat.gce_dev = dev;
> > + gce_sec_plat.base = cmdq->base;
> > + gce_sec_plat.base_pa = res->start;
> > + gce_sec_plat.hwid = hwid;
> > + gce_sec_plat.gce_num = cmdq->pdata-
> >gce_num;
> > + gce_sec_plat.clocks = cmdq->clocks;
> > + gce_sec_plat.thread_nr = cmdq->pdata-
> >thread_nr;
> > +
> > + mtk_cmdq_sec =
> platform_device_register_data(dev, "mtk_cmdq_sec",
> >
> +
> PLATFORM_DEVID_AUTO,
> >
> +
> &gce_sec_plat,
> >
> +
> sizeof(gce_sec_plat));
> > + if (IS_ERR(mtk_cmdq_sec)) {
> > + dev_err(dev, "failed to register
> platform_device mtk_cmdq_sec\n");
> > + return PTR_ERR(mtk_cmdq_sec);
> > + }
> > + }
> > + }
> > +
> > return 0;
> > }
> >
> > @@ -693,6 +729,7 @@ static const struct gce_plat gce_plat_v6 = {
> > .thread_nr = 24,
> > .shift = 3,
> > .control_by_sw = true,
> > + .has_sec = true,
> > .gce_num = 2
> > };
> >
> > @@ -708,6 +745,7 @@ static const struct gce_plat gce_plat_v8 = {
> > .thread_nr = 32,
> > .shift = 3,
> > .control_by_sw = true,
> > + .has_sec = true,
> > .gce_num = 2
> > };
> >
> > diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h
> b/include/linux/mailbox/mtk-cmdq-mailbox.h
> > index f78a08e7c6ed..fdda995a69ce 100644
> > --- a/include/linux/mailbox/mtk-cmdq-mailbox.h
> > +++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
> > @@ -79,6 +79,17 @@ struct cmdq_pkt {
> > bool loop;
> > };
> >
> > +struct cmdq_sec_plat {
> > + struct device *gce_dev;
> > + void __iomem *base;
> > + dma_addr_t base_pa;
> > + u32 hwid;
> > + u32 gce_num;
> > + struct clk_bulk_data *clocks;
> > + u32 thread_nr;
> > + u32 cmdq_event;
> > +};
> I feel this should be in mtk-cmdq-sec-mailbox.h.
>
That means we have to include <mtk-cmdq-sec-mailbox.h> in this file,
but actually mtk-cmdq-sec-mailbox.h have to include <mtk-cmdq-
mailbox.h>. So I add this here finally.
But I agree with you, I'll try to move this struct into mtk-cmdq-sec-
mailbox.h and include <mtk-cmdq-sec-mailbox.h> in mtk-cmdq-mailbox.c.
Regards,
Jason-JH.Lin
> Regards,
> Fei
>
>
>
> > +
> > u8 cmdq_get_shift_pa(struct mbox_chan *chan);
> >
> > #endif /* __MTK_CMDQ_MAILBOX_H__ */
> > --
> > 2.18.0
> >
> >
>
Powered by blists - more mailing lists