| Message ID | 1659536480-5176-8-git-send-email-quic_srivasam@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | Update ADSP pil loader for SC7280 platform | expand |
On 03/08/2022 17:21, Srinivasa Rao Mandadapu wrote: > Add memory sandbox support for ADSP based platforms secure booting. This repeats commit subject. Please replace it with proper commit message text describing what is done and why. > > Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com> > --- > drivers/remoteproc/qcom_q6v5_adsp.c | 101 +++++++++++++++++++++++++++++++++++- > 1 file changed, 99 insertions(+), 2 deletions(-) > > diff --git a/drivers/remoteproc/qcom_q6v5_adsp.c b/drivers/remoteproc/qcom_q6v5_adsp.c > index 3dbd035..f81da47 100644 > --- a/drivers/remoteproc/qcom_q6v5_adsp.c > +++ b/drivers/remoteproc/qcom_q6v5_adsp.c > @@ -9,6 +9,7 @@ > #include <linux/firmware.h> > #include <linux/interrupt.h> > #include <linux/io.h> > +#include <linux/iommu.h> > #include <linux/iopoll.h> > #include <linux/kernel.h> > #include <linux/mfd/syscon.h> > @@ -48,6 +49,8 @@ > #define LPASS_PWR_ON_REG 0x10 > #define LPASS_HALTREQ_REG 0x0 > > +#define SID_MASK_DEFAULT 0xF > + > #define QDSP6SS_XO_CBCR 0x38 > #define QDSP6SS_CORE_CBCR 0x20 > #define QDSP6SS_SLEEP_CBCR 0x3c > @@ -77,7 +80,7 @@ struct adsp_pil_data { > struct qcom_adsp { > struct device *dev; > struct rproc *rproc; > - > + struct iommu_domain *iommu_dom; > struct qcom_q6v5 q6v5; > > struct clk *xo; > @@ -332,6 +335,91 @@ static int adsp_load(struct rproc *rproc, const struct firmware *fw) > return 0; > } > > +static int adsp_map_smmu(struct qcom_adsp *adsp, struct rproc *rproc) > +{ > + struct of_phandle_args args; > + int ret, rc, i; > + long long sid; > + > + unsigned long mem_phys; > + unsigned long iova; > + const __be32 *prop; > + int access_level; > + uint32_t len, flag, mem_size; > + int offset; > + struct fw_rsc_hdr *hdr; > + struct fw_rsc_devmem *rsc_fw; > + > + rc = of_parse_phandle_with_fixed_args(adsp->dev->of_node, "iommus", 1, 0, &args); Please do not add implicit dependency on #iommu-cells value. > + if (rc < 0) > + sid = -1; > + else > + sid = args.args[0] & SID_MASK_DEFAULT; > + > + adsp->iommu_dom = iommu_domain_alloc(&platform_bus_type); please use adsp->dev->bus instead of platform_bus_type here. > + if (!adsp->iommu_dom) { > + dev_err(adsp->dev, "failed to allocate iommu domain\n"); > + return -ENOMEM; > + } > + > + ret = iommu_attach_device(adsp->iommu_dom, adsp->dev); > + if (ret) { > + dev_err(adsp->dev, "could not attach device ret = %d\n", ret); > + return -EBUSY; > + } > + > + /* Add SID configuration for ADSP Firmware to SMMU */ > + adsp->mem_phys = adsp->mem_phys | (sid << 32); > + > + ret = iommu_map(adsp->iommu_dom, adsp->mem_phys, adsp->mem_phys, > + adsp->mem_size, IOMMU_READ | IOMMU_WRITE); > + if (ret) { > + dev_err(adsp->dev, "Unable to map ADSP Physical Memory\n"); > + return ret; > + } > + > + prop = of_get_property(adsp->dev->of_node, "qcom,adsp-memory", &len); Non-documented property. So, this chunk is not acceptable. > + if (prop) { > + len /= sizeof(__be32); > + for (i = 0; i < len; i++) { > + iova = be32_to_cpu(prop[i++]); > + mem_phys = be32_to_cpu(prop[i++]); > + mem_size = be32_to_cpu(prop[i++]); > + access_level = be32_to_cpu(prop[i]); > + > + if (access_level) > + flag = IOMMU_READ | IOMMU_WRITE; > + else > + flag = IOMMU_READ; > + > + ret = iommu_map(adsp->iommu_dom, iova, mem_phys, mem_size, flag); > + if (ret) { > + dev_err(adsp->dev, "failed to map addr = %p mem_size = %x\n", > + &(mem_phys), mem_size); > + return ret; > + } > + } > + } else { > + if (!rproc->table_ptr) > + return 0; > + > + for (i = 0; i < rproc->table_ptr->num; i++) { > + offset = rproc->table_ptr->offset[i]; > + hdr = (void *)rproc->table_ptr + offset; > + rsc_fw = (struct fw_rsc_devmem *)hdr + sizeof(*hdr); > + > + ret = iommu_map(rproc->domain, rsc_fw->da, rsc_fw->pa, > + rsc_fw->len, rsc_fw->flags); What about filling an sgtable instead and using it? > + if (ret) { > + pr_err("%s; unable to map adsp memory address\n", __func__); > + return ret; > + } > + } > + } > + return 0; > +} > + > + > static int adsp_start(struct rproc *rproc) > { > struct qcom_adsp *adsp = (struct qcom_adsp *)rproc->priv; > @@ -341,7 +429,13 @@ static int adsp_start(struct rproc *rproc) > ret = qcom_q6v5_prepare(&adsp->q6v5); > if (ret) > return ret; > - > + if (!adsp->is_wpss) { > + ret = adsp_map_smmu(adsp, rproc); Is this also applicable to cDSP? To sdm845 adsp? > + if (ret) { > + dev_err(adsp->dev, "ADSP smmu mapping failed\n"); > + goto adsp_smmu_unmap; > + } > + } > ret = clk_prepare_enable(adsp->xo); > if (ret) > goto disable_irqs; > @@ -402,6 +496,9 @@ static int adsp_start(struct rproc *rproc) > clk_disable_unprepare(adsp->xo); > disable_irqs: > qcom_q6v5_unprepare(&adsp->q6v5); > +adsp_smmu_unmap: > + iommu_unmap(adsp->iommu_dom, adsp->mem_phys, adsp->mem_size); > + iommu_domain_free(adsp->iommu_dom); > > return ret; > }
Hi, the error handling below looks odd. Le 03/08/2022 à 16:21, Srinivasa Rao Mandadapu a écrit : > Add memory sandbox support for ADSP based platforms secure booting. > > Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam-jfJNa2p1gH1BDgjK7y7TUQ@public.gmane.org> > --- > drivers/remoteproc/qcom_q6v5_adsp.c | 101 +++++++++++++++++++++++++++++++++++- > 1 file changed, 99 insertions(+), 2 deletions(-) > > diff --git a/drivers/remoteproc/qcom_q6v5_adsp.c b/drivers/remoteproc/qcom_q6v5_adsp.c > index 3dbd035..f81da47 100644 > --- a/drivers/remoteproc/qcom_q6v5_adsp.c > +++ b/drivers/remoteproc/qcom_q6v5_adsp.c > static int adsp_start(struct rproc *rproc) > { > struct qcom_adsp *adsp = (struct qcom_adsp *)rproc->priv; > @@ -341,7 +429,13 @@ static int adsp_start(struct rproc *rproc) > ret = qcom_q6v5_prepare(&adsp->q6v5); > if (ret) > return ret; > - > + if (!adsp->is_wpss) { > + ret = adsp_map_smmu(adsp, rproc); > + if (ret) { > + dev_err(adsp->dev, "ADSP smmu mapping failed\n"); > + goto adsp_smmu_unmap; goto disable_irqs;? > + } > + } > ret = clk_prepare_enable(adsp->xo); > if (ret) > goto disable_irqs; goto adsp_smmu_unmap;? > @@ -402,6 +496,9 @@ static int adsp_start(struct rproc *rproc) > clk_disable_unprepare(adsp->xo); > disable_irqs: > qcom_q6v5_unprepare(&adsp->q6v5); > +adsp_smmu_unmap: > + iommu_unmap(adsp->iommu_dom, adsp->mem_phys, adsp->mem_size); > + iommu_domain_free(adsp->iommu_dom); Should this new hunk be above disable_irqs? And I think that it should be guardd with a "if (!adsp->is_wpss)". CJ > > return ret; > }
On 8/7/2022 2:09 AM, Christophe JAILLET wrote: Thanks for Your Time CJ!!! > Hi, > > the error handling below looks odd. > > Le 03/08/2022 à 16:21, Srinivasa Rao Mandadapu a écrit : >> Add memory sandbox support for ADSP based platforms secure booting. >> >> Signed-off-by: Srinivasa Rao Mandadapu >> <quic_srivasam-jfJNa2p1gH1BDgjK7y7TUQ@public.gmane.org> >> --- >> drivers/remoteproc/qcom_q6v5_adsp.c | 101 >> +++++++++++++++++++++++++++++++++++- >> 1 file changed, 99 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/remoteproc/qcom_q6v5_adsp.c >> b/drivers/remoteproc/qcom_q6v5_adsp.c >> index 3dbd035..f81da47 100644 >> --- a/drivers/remoteproc/qcom_q6v5_adsp.c >> +++ b/drivers/remoteproc/qcom_q6v5_adsp.c > >> static int adsp_start(struct rproc *rproc) >> { >> struct qcom_adsp *adsp = (struct qcom_adsp *)rproc->priv; >> @@ -341,7 +429,13 @@ static int adsp_start(struct rproc *rproc) >> ret = qcom_q6v5_prepare(&adsp->q6v5); >> if (ret) >> return ret; >> - >> + if (!adsp->is_wpss) { >> + ret = adsp_map_smmu(adsp, rproc); >> + if (ret) { >> + dev_err(adsp->dev, "ADSP smmu mapping failed\n"); >> + goto adsp_smmu_unmap; > goto disable_irqs;? Yes. will correct it accordingly. > >> + } >> + } >> ret = clk_prepare_enable(adsp->xo); >> if (ret) >> goto disable_irqs; > > goto adsp_smmu_unmap;? Yes. will correct it accordingly. > >> @@ -402,6 +496,9 @@ static int adsp_start(struct rproc *rproc) >> clk_disable_unprepare(adsp->xo); >> disable_irqs: >> qcom_q6v5_unprepare(&adsp->q6v5); >> +adsp_smmu_unmap: >> + iommu_unmap(adsp->iommu_dom, adsp->mem_phys, adsp->mem_size); >> + iommu_domain_free(adsp->iommu_dom); > > Should this new hunk be above disable_irqs? > And I think that it should be guardd with a "if (!adsp->is_wpss)". Yes. will correct it accordingly. > > CJ > >> return ret; >> } >
On 8/7/2022 2:04 AM, Dmitry Baryshkov wrote: Thanks for your time and Valuable inputs Dmitry!!! > On 03/08/2022 17:21, Srinivasa Rao Mandadapu wrote: >> Add memory sandbox support for ADSP based platforms secure booting. > > This repeats commit subject. Please replace it with proper commit > message text describing what is done and why. Okay. Will update it. > >> >> Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com> >> --- >> drivers/remoteproc/qcom_q6v5_adsp.c | 101 >> +++++++++++++++++++++++++++++++++++- >> 1 file changed, 99 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/remoteproc/qcom_q6v5_adsp.c >> b/drivers/remoteproc/qcom_q6v5_adsp.c >> index 3dbd035..f81da47 100644 >> --- a/drivers/remoteproc/qcom_q6v5_adsp.c >> +++ b/drivers/remoteproc/qcom_q6v5_adsp.c >> @@ -9,6 +9,7 @@ >> #include <linux/firmware.h> >> #include <linux/interrupt.h> >> #include <linux/io.h> >> +#include <linux/iommu.h> >> #include <linux/iopoll.h> >> #include <linux/kernel.h> >> #include <linux/mfd/syscon.h> >> @@ -48,6 +49,8 @@ >> #define LPASS_PWR_ON_REG 0x10 >> #define LPASS_HALTREQ_REG 0x0 >> +#define SID_MASK_DEFAULT 0xF >> + >> #define QDSP6SS_XO_CBCR 0x38 >> #define QDSP6SS_CORE_CBCR 0x20 >> #define QDSP6SS_SLEEP_CBCR 0x3c >> @@ -77,7 +80,7 @@ struct adsp_pil_data { >> struct qcom_adsp { >> struct device *dev; >> struct rproc *rproc; >> - >> + struct iommu_domain *iommu_dom; >> struct qcom_q6v5 q6v5; >> struct clk *xo; >> @@ -332,6 +335,91 @@ static int adsp_load(struct rproc *rproc, const >> struct firmware *fw) >> return 0; >> } >> +static int adsp_map_smmu(struct qcom_adsp *adsp, struct rproc *rproc) >> +{ >> + struct of_phandle_args args; >> + int ret, rc, i; >> + long long sid; >> + >> + unsigned long mem_phys; >> + unsigned long iova; >> + const __be32 *prop; >> + int access_level; >> + uint32_t len, flag, mem_size; >> + int offset; >> + struct fw_rsc_hdr *hdr; >> + struct fw_rsc_devmem *rsc_fw; >> + >> + rc = of_parse_phandle_with_fixed_args(adsp->dev->of_node, >> "iommus", 1, 0, &args); > > Please do not add implicit dependency on #iommu-cells value. Okay. Will change it to "of_parse_phandle_with_args()" > >> + if (rc < 0) >> + sid = -1; >> + else >> + sid = args.args[0] & SID_MASK_DEFAULT; >> + >> + adsp->iommu_dom = iommu_domain_alloc(&platform_bus_type); > > please use adsp->dev->bus instead of platform_bus_type here. Okay. will update it. > >> + if (!adsp->iommu_dom) { >> + dev_err(adsp->dev, "failed to allocate iommu domain\n"); >> + return -ENOMEM; >> + } >> + >> + ret = iommu_attach_device(adsp->iommu_dom, adsp->dev); >> + if (ret) { >> + dev_err(adsp->dev, "could not attach device ret = %d\n", ret); >> + return -EBUSY; >> + } >> + >> + /* Add SID configuration for ADSP Firmware to SMMU */ >> + adsp->mem_phys = adsp->mem_phys | (sid << 32); >> + >> + ret = iommu_map(adsp->iommu_dom, adsp->mem_phys, adsp->mem_phys, >> + adsp->mem_size, IOMMU_READ | IOMMU_WRITE); >> + if (ret) { >> + dev_err(adsp->dev, "Unable to map ADSP Physical Memory\n"); >> + return ret; >> + } >> + >> + prop = of_get_property(adsp->dev->of_node, "qcom,adsp-memory", >> &len); > > Non-documented property. So, this chunk is not acceptable. Okay. Will add it in dt-bindings too. > >> + if (prop) { >> + len /= sizeof(__be32); >> + for (i = 0; i < len; i++) { >> + iova = be32_to_cpu(prop[i++]); >> + mem_phys = be32_to_cpu(prop[i++]); >> + mem_size = be32_to_cpu(prop[i++]); >> + access_level = be32_to_cpu(prop[i]); >> + >> + if (access_level) >> + flag = IOMMU_READ | IOMMU_WRITE; >> + else >> + flag = IOMMU_READ; >> + >> + ret = iommu_map(adsp->iommu_dom, iova, mem_phys, >> mem_size, flag); >> + if (ret) { >> + dev_err(adsp->dev, "failed to map addr = %p mem_size >> = %x\n", >> + &(mem_phys), mem_size); >> + return ret; >> + } >> + } >> + } else { >> + if (!rproc->table_ptr) >> + return 0; >> + >> + for (i = 0; i < rproc->table_ptr->num; i++) { >> + offset = rproc->table_ptr->offset[i]; >> + hdr = (void *)rproc->table_ptr + offset; >> + rsc_fw = (struct fw_rsc_devmem *)hdr + sizeof(*hdr); >> + >> + ret = iommu_map(rproc->domain, rsc_fw->da, rsc_fw->pa, >> + rsc_fw->len, rsc_fw->flags); > > What about filling an sgtable instead and using it? Here we are just doing IO mapping and allowing ADSP to access the specified memory. I am not sure, sg_table applicable here or not as it's not any DMA activity. Please correct me if my understanding is not enough and It would help me a lot, if any good example shared. > >> + if (ret) { >> + pr_err("%s; unable to map adsp memory address\n", >> __func__); >> + return ret; >> + } >> + } >> + } >> + return 0; >> +} >> + >> + >> static int adsp_start(struct rproc *rproc) >> { >> struct qcom_adsp *adsp = (struct qcom_adsp *)rproc->priv; >> @@ -341,7 +429,13 @@ static int adsp_start(struct rproc *rproc) >> ret = qcom_q6v5_prepare(&adsp->q6v5); >> if (ret) >> return ret; >> - >> + if (!adsp->is_wpss) { >> + ret = adsp_map_smmu(adsp, rproc); > > Is this also applicable to cDSP? To sdm845 adsp? It's applicable to all ADSP SoC variants. I think it's better to add adsp flag("is_adsp") for distinguishing adsp use cases. Please suggest here. > >> + if (ret) { >> + dev_err(adsp->dev, "ADSP smmu mapping failed\n"); >> + goto adsp_smmu_unmap; >> + } >> + } >> ret = clk_prepare_enable(adsp->xo); >> if (ret) >> goto disable_irqs; >> @@ -402,6 +496,9 @@ static int adsp_start(struct rproc *rproc) >> clk_disable_unprepare(adsp->xo); >> disable_irqs: >> qcom_q6v5_unprepare(&adsp->q6v5); >> +adsp_smmu_unmap: >> + iommu_unmap(adsp->iommu_dom, adsp->mem_phys, adsp->mem_size); >> + iommu_domain_free(adsp->iommu_dom); >> return ret; >> } > >
On 8/9/2022 1:52 PM, Srinivasa Rao Mandadapu wrote: > > On 8/7/2022 2:04 AM, Dmitry Baryshkov wrote: > Thanks for your time and Valuable inputs Dmitry!!! >> On 03/08/2022 17:21, Srinivasa Rao Mandadapu wrote: >>> Add memory sandbox support for ADSP based platforms secure booting. >> >> This repeats commit subject. Please replace it with proper commit >> message text describing what is done and why. > Okay. Will update it. >> >>> >>> Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com> >>> --- >>> drivers/remoteproc/qcom_q6v5_adsp.c | 101 >>> +++++++++++++++++++++++++++++++++++- >>> 1 file changed, 99 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/remoteproc/qcom_q6v5_adsp.c >>> b/drivers/remoteproc/qcom_q6v5_adsp.c >>> index 3dbd035..f81da47 100644 >>> --- a/drivers/remoteproc/qcom_q6v5_adsp.c >>> +++ b/drivers/remoteproc/qcom_q6v5_adsp.c >>> @@ -9,6 +9,7 @@ >>> #include <linux/firmware.h> >>> #include <linux/interrupt.h> >>> #include <linux/io.h> >>> +#include <linux/iommu.h> >>> #include <linux/iopoll.h> >>> #include <linux/kernel.h> >>> #include <linux/mfd/syscon.h> >>> @@ -48,6 +49,8 @@ >>> #define LPASS_PWR_ON_REG 0x10 >>> #define LPASS_HALTREQ_REG 0x0 >>> +#define SID_MASK_DEFAULT 0xF >>> + >>> #define QDSP6SS_XO_CBCR 0x38 >>> #define QDSP6SS_CORE_CBCR 0x20 >>> #define QDSP6SS_SLEEP_CBCR 0x3c >>> @@ -77,7 +80,7 @@ struct adsp_pil_data { >>> struct qcom_adsp { >>> struct device *dev; >>> struct rproc *rproc; >>> - >>> + struct iommu_domain *iommu_dom; >>> struct qcom_q6v5 q6v5; >>> struct clk *xo; >>> @@ -332,6 +335,91 @@ static int adsp_load(struct rproc *rproc, const >>> struct firmware *fw) >>> return 0; >>> } >>> +static int adsp_map_smmu(struct qcom_adsp *adsp, struct rproc >>> *rproc) >>> +{ >>> + struct of_phandle_args args; >>> + int ret, rc, i; >>> + long long sid; >>> + >>> + unsigned long mem_phys; >>> + unsigned long iova; >>> + const __be32 *prop; >>> + int access_level; >>> + uint32_t len, flag, mem_size; >>> + int offset; >>> + struct fw_rsc_hdr *hdr; >>> + struct fw_rsc_devmem *rsc_fw; >>> + >>> + rc = of_parse_phandle_with_fixed_args(adsp->dev->of_node, >>> "iommus", 1, 0, &args); >> >> Please do not add implicit dependency on #iommu-cells value. > Okay. Will change it to "of_parse_phandle_with_args()" >> >>> + if (rc < 0) >>> + sid = -1; >>> + else >>> + sid = args.args[0] & SID_MASK_DEFAULT; >>> + >>> + adsp->iommu_dom = iommu_domain_alloc(&platform_bus_type); >> >> please use adsp->dev->bus instead of platform_bus_type here. > Okay. will update it. >> >>> + if (!adsp->iommu_dom) { >>> + dev_err(adsp->dev, "failed to allocate iommu domain\n"); >>> + return -ENOMEM; >>> + } >>> + >>> + ret = iommu_attach_device(adsp->iommu_dom, adsp->dev); >>> + if (ret) { >>> + dev_err(adsp->dev, "could not attach device ret = %d\n", ret); >>> + return -EBUSY; >>> + } >>> + >>> + /* Add SID configuration for ADSP Firmware to SMMU */ >>> + adsp->mem_phys = adsp->mem_phys | (sid << 32); >>> + >>> + ret = iommu_map(adsp->iommu_dom, adsp->mem_phys, adsp->mem_phys, >>> + adsp->mem_size, IOMMU_READ | IOMMU_WRITE); >>> + if (ret) { >>> + dev_err(adsp->dev, "Unable to map ADSP Physical Memory\n"); >>> + return ret; >>> + } >>> + >>> + prop = of_get_property(adsp->dev->of_node, "qcom,adsp-memory", >>> &len); >> >> Non-documented property. So, this chunk is not acceptable. > Okay. Will add it in dt-bindings too. >> >>> + if (prop) { >>> + len /= sizeof(__be32); >>> + for (i = 0; i < len; i++) { >>> + iova = be32_to_cpu(prop[i++]); >>> + mem_phys = be32_to_cpu(prop[i++]); >>> + mem_size = be32_to_cpu(prop[i++]); >>> + access_level = be32_to_cpu(prop[i]); >>> + >>> + if (access_level) >>> + flag = IOMMU_READ | IOMMU_WRITE; >>> + else >>> + flag = IOMMU_READ; >>> + >>> + ret = iommu_map(adsp->iommu_dom, iova, mem_phys, >>> mem_size, flag); >>> + if (ret) { >>> + dev_err(adsp->dev, "failed to map addr = %p >>> mem_size = %x\n", >>> + &(mem_phys), mem_size); >>> + return ret; >>> + } >>> + } >>> + } else { >>> + if (!rproc->table_ptr) >>> + return 0; >>> + >>> + for (i = 0; i < rproc->table_ptr->num; i++) { >>> + offset = rproc->table_ptr->offset[i]; >>> + hdr = (void *)rproc->table_ptr + offset; >>> + rsc_fw = (struct fw_rsc_devmem *)hdr + sizeof(*hdr); >>> + >>> + ret = iommu_map(rproc->domain, rsc_fw->da, rsc_fw->pa, >>> + rsc_fw->len, rsc_fw->flags); >> >> What about filling an sgtable instead and using it? > > Here we are just doing IO mapping and allowing ADSP to access the > specified memory. > > I am not sure, sg_table applicable here or not as it's not any DMA > activity. > > Please correct me if my understanding is not enough and It would help > me a lot, if any good example shared. > >> >>> + if (ret) { >>> + pr_err("%s; unable to map adsp memory address\n", >>> __func__); >>> + return ret; >>> + } >>> + } >>> + } >>> + return 0; >>> +} >>> + >>> + >>> static int adsp_start(struct rproc *rproc) >>> { >>> struct qcom_adsp *adsp = (struct qcom_adsp *)rproc->priv; >>> @@ -341,7 +429,13 @@ static int adsp_start(struct rproc *rproc) >>> ret = qcom_q6v5_prepare(&adsp->q6v5); >>> if (ret) >>> return ret; >>> - >>> + if (!adsp->is_wpss) { >>> + ret = adsp_map_smmu(adsp, rproc); >> >> Is this also applicable to cDSP? To sdm845 adsp? > > It's applicable to all ADSP SoC variants. I think it's better to add > adsp flag("is_adsp") for > > distinguishing adsp use cases. Please suggest here. Verified with sdm845 developer, and got to know that, even though it's applicable there too, Somehow for them it was working without memory sandboxing. Maybe, it was due to SMMU security levels were different. > >> >>> + if (ret) { >>> + dev_err(adsp->dev, "ADSP smmu mapping failed\n"); >>> + goto adsp_smmu_unmap; >>> + } >>> + } >>> ret = clk_prepare_enable(adsp->xo); >>> if (ret) >>> goto disable_irqs; >>> @@ -402,6 +496,9 @@ static int adsp_start(struct rproc *rproc) >>> clk_disable_unprepare(adsp->xo); >>> disable_irqs: >>> qcom_q6v5_unprepare(&adsp->q6v5); >>> +adsp_smmu_unmap: >>> + iommu_unmap(adsp->iommu_dom, adsp->mem_phys, adsp->mem_size); >>> + iommu_domain_free(adsp->iommu_dom); >>> return ret; >>> } >> >>
diff --git a/drivers/remoteproc/qcom_q6v5_adsp.c b/drivers/remoteproc/qcom_q6v5_adsp.c index 3dbd035..f81da47 100644 --- a/drivers/remoteproc/qcom_q6v5_adsp.c +++ b/drivers/remoteproc/qcom_q6v5_adsp.c @@ -9,6 +9,7 @@ #include <linux/firmware.h> #include <linux/interrupt.h> #include <linux/io.h> +#include <linux/iommu.h> #include <linux/iopoll.h> #include <linux/kernel.h> #include <linux/mfd/syscon.h> @@ -48,6 +49,8 @@ #define LPASS_PWR_ON_REG 0x10 #define LPASS_HALTREQ_REG 0x0 +#define SID_MASK_DEFAULT 0xF + #define QDSP6SS_XO_CBCR 0x38 #define QDSP6SS_CORE_CBCR 0x20 #define QDSP6SS_SLEEP_CBCR 0x3c @@ -77,7 +80,7 @@ struct adsp_pil_data { struct qcom_adsp { struct device *dev; struct rproc *rproc; - + struct iommu_domain *iommu_dom; struct qcom_q6v5 q6v5; struct clk *xo; @@ -332,6 +335,91 @@ static int adsp_load(struct rproc *rproc, const struct firmware *fw) return 0; } +static int adsp_map_smmu(struct qcom_adsp *adsp, struct rproc *rproc) +{ + struct of_phandle_args args; + int ret, rc, i; + long long sid; + + unsigned long mem_phys; + unsigned long iova; + const __be32 *prop; + int access_level; + uint32_t len, flag, mem_size; + int offset; + struct fw_rsc_hdr *hdr; + struct fw_rsc_devmem *rsc_fw; + + rc = of_parse_phandle_with_fixed_args(adsp->dev->of_node, "iommus", 1, 0, &args); + if (rc < 0) + sid = -1; + else + sid = args.args[0] & SID_MASK_DEFAULT; + + adsp->iommu_dom = iommu_domain_alloc(&platform_bus_type); + if (!adsp->iommu_dom) { + dev_err(adsp->dev, "failed to allocate iommu domain\n"); + return -ENOMEM; + } + + ret = iommu_attach_device(adsp->iommu_dom, adsp->dev); + if (ret) { + dev_err(adsp->dev, "could not attach device ret = %d\n", ret); + return -EBUSY; + } + + /* Add SID configuration for ADSP Firmware to SMMU */ + adsp->mem_phys = adsp->mem_phys | (sid << 32); + + ret = iommu_map(adsp->iommu_dom, adsp->mem_phys, adsp->mem_phys, + adsp->mem_size, IOMMU_READ | IOMMU_WRITE); + if (ret) { + dev_err(adsp->dev, "Unable to map ADSP Physical Memory\n"); + return ret; + } + + prop = of_get_property(adsp->dev->of_node, "qcom,adsp-memory", &len); + if (prop) { + len /= sizeof(__be32); + for (i = 0; i < len; i++) { + iova = be32_to_cpu(prop[i++]); + mem_phys = be32_to_cpu(prop[i++]); + mem_size = be32_to_cpu(prop[i++]); + access_level = be32_to_cpu(prop[i]); + + if (access_level) + flag = IOMMU_READ | IOMMU_WRITE; + else + flag = IOMMU_READ; + + ret = iommu_map(adsp->iommu_dom, iova, mem_phys, mem_size, flag); + if (ret) { + dev_err(adsp->dev, "failed to map addr = %p mem_size = %x\n", + &(mem_phys), mem_size); + return ret; + } + } + } else { + if (!rproc->table_ptr) + return 0; + + for (i = 0; i < rproc->table_ptr->num; i++) { + offset = rproc->table_ptr->offset[i]; + hdr = (void *)rproc->table_ptr + offset; + rsc_fw = (struct fw_rsc_devmem *)hdr + sizeof(*hdr); + + ret = iommu_map(rproc->domain, rsc_fw->da, rsc_fw->pa, + rsc_fw->len, rsc_fw->flags); + if (ret) { + pr_err("%s; unable to map adsp memory address\n", __func__); + return ret; + } + } + } + return 0; +} + + static int adsp_start(struct rproc *rproc) { struct qcom_adsp *adsp = (struct qcom_adsp *)rproc->priv; @@ -341,7 +429,13 @@ static int adsp_start(struct rproc *rproc) ret = qcom_q6v5_prepare(&adsp->q6v5); if (ret) return ret; - + if (!adsp->is_wpss) { + ret = adsp_map_smmu(adsp, rproc); + if (ret) { + dev_err(adsp->dev, "ADSP smmu mapping failed\n"); + goto adsp_smmu_unmap; + } + } ret = clk_prepare_enable(adsp->xo); if (ret) goto disable_irqs; @@ -402,6 +496,9 @@ static int adsp_start(struct rproc *rproc) clk_disable_unprepare(adsp->xo); disable_irqs: qcom_q6v5_unprepare(&adsp->q6v5); +adsp_smmu_unmap: + iommu_unmap(adsp->iommu_dom, adsp->mem_phys, adsp->mem_size); + iommu_domain_free(adsp->iommu_dom); return ret; }
Add memory sandbox support for ADSP based platforms secure booting. Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com> --- drivers/remoteproc/qcom_q6v5_adsp.c | 101 +++++++++++++++++++++++++++++++++++- 1 file changed, 99 insertions(+), 2 deletions(-)