| Message ID | 20241004212359.2263502-7-quic_mojha@quicinc.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Peripheral Image Loader support for Qualcomm SoCs | expand |
On 04/10/2024 23:23, Mukesh Ojha wrote: > For Qualcomm SoCs runnning with Qualcomm EL2 hypervisor(QHEE), IOMMU > translation for remote processors is managed by QHEE and if the same SoC > run under KVM, remoteproc carveout and devmem region should be IOMMU > mapped from Linux PAS driver before remoteproc is brought up and > unmapped once it is tear down and apart from this, SHM bridge also need > to set up to enable memory protection on both remoteproc meta data > memory as well as for the carveout region. > > Enable the support required to run Qualcomm remoteprocs on non-QHEE > hypervisors. > > Signed-off-by: Mukesh Ojha <quic_mojha@quicinc.com> > --- > drivers/remoteproc/qcom_q6v5_pas.c | 41 +++++++++++++++++++++++++++++- > 1 file changed, 40 insertions(+), 1 deletion(-) > > diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c > index ac339145e072..13bd13f1b989 100644 > --- a/drivers/remoteproc/qcom_q6v5_pas.c > +++ b/drivers/remoteproc/qcom_q6v5_pas.c > @@ -122,6 +122,7 @@ struct qcom_adsp { > > struct qcom_devmem_table *devmem; > struct qcom_tzmem_area *tzmem; > + unsigned long sid; > }; > > static void adsp_segment_dump(struct rproc *rproc, struct rproc_dump_segment *segment, > @@ -310,9 +311,21 @@ static int adsp_start(struct rproc *rproc) > if (ret) > return ret; > > + ret = qcom_map_unmap_carveout(rproc, adsp->mem_phys, adsp->mem_size, true, true, adsp->sid); > + if (ret) { > + dev_err(adsp->dev, "iommu mapping failed, ret: %d\n", ret); > + goto disable_irqs; > + } > + > + ret = qcom_map_devmem(rproc, adsp->devmem, true, adsp->sid); > + if (ret) { > + dev_err(adsp->dev, "devmem iommu mapping failed, ret: %d\n", ret); > + goto unmap_carveout; > + } > + > ret = adsp_pds_enable(adsp, adsp->proxy_pds, adsp->proxy_pd_count); > if (ret < 0) > - goto disable_irqs; > + goto unmap_devmem; > > ret = clk_prepare_enable(adsp->xo); > if (ret) > @@ -400,6 +413,10 @@ static int adsp_start(struct rproc *rproc) > clk_disable_unprepare(adsp->xo); > disable_proxy_pds: > adsp_pds_disable(adsp, adsp->proxy_pds, adsp->proxy_pd_count); > +unmap_devmem: > + qcom_unmap_devmem(rproc, adsp->devmem, adsp->sid); > +unmap_carveout: > + qcom_map_unmap_carveout(rproc, adsp->mem_phys, adsp->mem_size, false, true, adsp->sid); > disable_irqs: > qcom_q6v5_unprepare(&adsp->q6v5); > > @@ -445,6 +462,9 @@ static int adsp_stop(struct rproc *rproc) > dev_err(adsp->dev, "failed to shutdown dtb: %d\n", ret); > } > > + qcom_unmap_devmem(rproc, adsp->devmem, adsp->sid); > + qcom_map_unmap_carveout(rproc, adsp->mem_phys, adsp->mem_size, false, true, adsp->sid); > + > handover = qcom_q6v5_unprepare(&adsp->q6v5); > if (handover) > qcom_pas_handover(&adsp->q6v5); > @@ -844,6 +864,25 @@ static int adsp_probe(struct platform_device *pdev) > } > platform_set_drvdata(pdev, adsp); > > + if (of_property_present(pdev->dev.of_node, "iommus")) { > + struct of_phandle_args args; > + > + ret = of_parse_phandle_with_args(pdev->dev.of_node, "iommus", "#iommu-cells", 0, &args); > + if (ret < 0) > + return ret; > + > + rproc->has_iommu = true; > + adsp->sid = args.args[0]; > + of_node_put(args.np); > + ret = adsp_devmem_init(adsp); > + if (ret) > + return ret; Why don't you get this table from the firmware like presumably QHEE does ? Neil > + > + adsp->pas_metadata.shm_bridge_needed = true; > + } else { > + rproc->has_iommu = false; > + } > + > ret = device_init_wakeup(adsp->dev, true); > if (ret) > goto free_rproc;
On Mon, Oct 07, 2024 at 10:05:08AM +0200, neil.armstrong@linaro.org wrote: > On 04/10/2024 23:23, Mukesh Ojha wrote: > > For Qualcomm SoCs runnning with Qualcomm EL2 hypervisor(QHEE), IOMMU > > translation for remote processors is managed by QHEE and if the same SoC > > run under KVM, remoteproc carveout and devmem region should be IOMMU > > mapped from Linux PAS driver before remoteproc is brought up and > > unmapped once it is tear down and apart from this, SHM bridge also need > > to set up to enable memory protection on both remoteproc meta data > > memory as well as for the carveout region. > > > > Enable the support required to run Qualcomm remoteprocs on non-QHEE > > hypervisors. > > > > Signed-off-by: Mukesh Ojha <quic_mojha@quicinc.com> > > --- > > drivers/remoteproc/qcom_q6v5_pas.c | 41 +++++++++++++++++++++++++++++- > > 1 file changed, 40 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c > > index ac339145e072..13bd13f1b989 100644 > > --- a/drivers/remoteproc/qcom_q6v5_pas.c > > +++ b/drivers/remoteproc/qcom_q6v5_pas.c > > @@ -122,6 +122,7 @@ struct qcom_adsp { > > struct qcom_devmem_table *devmem; > > struct qcom_tzmem_area *tzmem; > > + unsigned long sid; > > }; > > static void adsp_segment_dump(struct rproc *rproc, struct rproc_dump_segment *segment, > > @@ -310,9 +311,21 @@ static int adsp_start(struct rproc *rproc) > > if (ret) > > return ret; > > + ret = qcom_map_unmap_carveout(rproc, adsp->mem_phys, adsp->mem_size, true, true, adsp->sid); > > + if (ret) { > > + dev_err(adsp->dev, "iommu mapping failed, ret: %d\n", ret); > > + goto disable_irqs; > > + } > > + > > + ret = qcom_map_devmem(rproc, adsp->devmem, true, adsp->sid); > > + if (ret) { > > + dev_err(adsp->dev, "devmem iommu mapping failed, ret: %d\n", ret); > > + goto unmap_carveout; > > + } > > + > > ret = adsp_pds_enable(adsp, adsp->proxy_pds, adsp->proxy_pd_count); > > if (ret < 0) > > - goto disable_irqs; > > + goto unmap_devmem; > > ret = clk_prepare_enable(adsp->xo); > > if (ret) > > @@ -400,6 +413,10 @@ static int adsp_start(struct rproc *rproc) > > clk_disable_unprepare(adsp->xo); > > disable_proxy_pds: > > adsp_pds_disable(adsp, adsp->proxy_pds, adsp->proxy_pd_count); > > +unmap_devmem: > > + qcom_unmap_devmem(rproc, adsp->devmem, adsp->sid); > > +unmap_carveout: > > + qcom_map_unmap_carveout(rproc, adsp->mem_phys, adsp->mem_size, false, true, adsp->sid); > > disable_irqs: > > qcom_q6v5_unprepare(&adsp->q6v5); > > @@ -445,6 +462,9 @@ static int adsp_stop(struct rproc *rproc) > > dev_err(adsp->dev, "failed to shutdown dtb: %d\n", ret); > > } > > + qcom_unmap_devmem(rproc, adsp->devmem, adsp->sid); > > + qcom_map_unmap_carveout(rproc, adsp->mem_phys, adsp->mem_size, false, true, adsp->sid); > > + > > handover = qcom_q6v5_unprepare(&adsp->q6v5); > > if (handover) > > qcom_pas_handover(&adsp->q6v5); > > @@ -844,6 +864,25 @@ static int adsp_probe(struct platform_device *pdev) > > } > > platform_set_drvdata(pdev, adsp); > > + if (of_property_present(pdev->dev.of_node, "iommus")) { > > + struct of_phandle_args args; > > + > > + ret = of_parse_phandle_with_args(pdev->dev.of_node, "iommus", "#iommu-cells", 0, &args); > > + if (ret < 0) > > + return ret; > > + > > + rproc->has_iommu = true; > > + adsp->sid = args.args[0]; > > + of_node_put(args.np); > > + ret = adsp_devmem_init(adsp); > > + if (ret) > > + return ret; > > Why don't you get this table from the firmware like presumably QHEE does ? Well, AFAIK, QHEE(EL2) has this information statically present and does not get it from anywhere., but will confirm this twice.. -Mukesh
On Mon, Oct 07, 2024 at 08:22:39PM +0530, Mukesh Ojha wrote: > On Mon, Oct 07, 2024 at 10:05:08AM +0200, neil.armstrong@linaro.org wrote: > > On 04/10/2024 23:23, Mukesh Ojha wrote: > > > For Qualcomm SoCs runnning with Qualcomm EL2 hypervisor(QHEE), IOMMU > > > translation for remote processors is managed by QHEE and if the same SoC > > > run under KVM, remoteproc carveout and devmem region should be IOMMU > > > mapped from Linux PAS driver before remoteproc is brought up and > > > unmapped once it is tear down and apart from this, SHM bridge also need > > > to set up to enable memory protection on both remoteproc meta data > > > memory as well as for the carveout region. > > > > > > Enable the support required to run Qualcomm remoteprocs on non-QHEE > > > hypervisors. > > > > > > Signed-off-by: Mukesh Ojha <quic_mojha@quicinc.com> > > > --- > > > drivers/remoteproc/qcom_q6v5_pas.c | 41 +++++++++++++++++++++++++++++- > > > 1 file changed, 40 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c > > > index ac339145e072..13bd13f1b989 100644 > > > --- a/drivers/remoteproc/qcom_q6v5_pas.c > > > +++ b/drivers/remoteproc/qcom_q6v5_pas.c > > > @@ -122,6 +122,7 @@ struct qcom_adsp { > > > struct qcom_devmem_table *devmem; > > > struct qcom_tzmem_area *tzmem; > > > + unsigned long sid; > > > }; > > > static void adsp_segment_dump(struct rproc *rproc, struct rproc_dump_segment *segment, > > > @@ -310,9 +311,21 @@ static int adsp_start(struct rproc *rproc) > > > if (ret) > > > return ret; > > > + ret = qcom_map_unmap_carveout(rproc, adsp->mem_phys, adsp->mem_size, true, true, adsp->sid); > > > + if (ret) { > > > + dev_err(adsp->dev, "iommu mapping failed, ret: %d\n", ret); > > > + goto disable_irqs; > > > + } > > > + > > > + ret = qcom_map_devmem(rproc, adsp->devmem, true, adsp->sid); > > > + if (ret) { > > > + dev_err(adsp->dev, "devmem iommu mapping failed, ret: %d\n", ret); > > > + goto unmap_carveout; > > > + } > > > + > > > ret = adsp_pds_enable(adsp, adsp->proxy_pds, adsp->proxy_pd_count); > > > if (ret < 0) > > > - goto disable_irqs; > > > + goto unmap_devmem; > > > ret = clk_prepare_enable(adsp->xo); > > > if (ret) > > > @@ -400,6 +413,10 @@ static int adsp_start(struct rproc *rproc) > > > clk_disable_unprepare(adsp->xo); > > > disable_proxy_pds: > > > adsp_pds_disable(adsp, adsp->proxy_pds, adsp->proxy_pd_count); > > > +unmap_devmem: > > > + qcom_unmap_devmem(rproc, adsp->devmem, adsp->sid); > > > +unmap_carveout: > > > + qcom_map_unmap_carveout(rproc, adsp->mem_phys, adsp->mem_size, false, true, adsp->sid); > > > disable_irqs: > > > qcom_q6v5_unprepare(&adsp->q6v5); > > > @@ -445,6 +462,9 @@ static int adsp_stop(struct rproc *rproc) > > > dev_err(adsp->dev, "failed to shutdown dtb: %d\n", ret); > > > } > > > + qcom_unmap_devmem(rproc, adsp->devmem, adsp->sid); > > > + qcom_map_unmap_carveout(rproc, adsp->mem_phys, adsp->mem_size, false, true, adsp->sid); > > > + > > > handover = qcom_q6v5_unprepare(&adsp->q6v5); > > > if (handover) > > > qcom_pas_handover(&adsp->q6v5); > > > @@ -844,6 +864,25 @@ static int adsp_probe(struct platform_device *pdev) > > > } > > > platform_set_drvdata(pdev, adsp); > > > + if (of_property_present(pdev->dev.of_node, "iommus")) { > > > + struct of_phandle_args args; > > > + > > > + ret = of_parse_phandle_with_args(pdev->dev.of_node, "iommus", "#iommu-cells", 0, &args); > > > + if (ret < 0) > > > + return ret; > > > + > > > + rproc->has_iommu = true; > > > + adsp->sid = args.args[0]; > > > + of_node_put(args.np); > > > + ret = adsp_devmem_init(adsp); > > > + if (ret) > > > + return ret; > > > > Why don't you get this table from the firmware like presumably QHEE does ? > > Well, AFAIK, QHEE(EL2) has this information statically present and does > not get it from anywhere., but will confirm this twice.. Double confirmed, device memory region required by remoteproc is statically present with QHEE. -Mukesh > > -Mukesh >
On 08/10/2024 08:21, Mukesh Ojha wrote: > On Mon, Oct 07, 2024 at 08:22:39PM +0530, Mukesh Ojha wrote: >> On Mon, Oct 07, 2024 at 10:05:08AM +0200, neil.armstrong@linaro.org wrote: >>> On 04/10/2024 23:23, Mukesh Ojha wrote: >>>> For Qualcomm SoCs runnning with Qualcomm EL2 hypervisor(QHEE), IOMMU >>>> translation for remote processors is managed by QHEE and if the same SoC >>>> run under KVM, remoteproc carveout and devmem region should be IOMMU >>>> mapped from Linux PAS driver before remoteproc is brought up and >>>> unmapped once it is tear down and apart from this, SHM bridge also need >>>> to set up to enable memory protection on both remoteproc meta data >>>> memory as well as for the carveout region. >>>> >>>> Enable the support required to run Qualcomm remoteprocs on non-QHEE >>>> hypervisors. >>>> >>>> Signed-off-by: Mukesh Ojha <quic_mojha@quicinc.com> >>>> --- >>>> drivers/remoteproc/qcom_q6v5_pas.c | 41 +++++++++++++++++++++++++++++- >>>> 1 file changed, 40 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c >>>> index ac339145e072..13bd13f1b989 100644 >>>> --- a/drivers/remoteproc/qcom_q6v5_pas.c >>>> +++ b/drivers/remoteproc/qcom_q6v5_pas.c >>>> @@ -122,6 +122,7 @@ struct qcom_adsp { >>>> struct qcom_devmem_table *devmem; >>>> struct qcom_tzmem_area *tzmem; >>>> + unsigned long sid; >>>> }; >>>> static void adsp_segment_dump(struct rproc *rproc, struct rproc_dump_segment *segment, >>>> @@ -310,9 +311,21 @@ static int adsp_start(struct rproc *rproc) >>>> if (ret) >>>> return ret; >>>> + ret = qcom_map_unmap_carveout(rproc, adsp->mem_phys, adsp->mem_size, true, true, adsp->sid); >>>> + if (ret) { >>>> + dev_err(adsp->dev, "iommu mapping failed, ret: %d\n", ret); >>>> + goto disable_irqs; >>>> + } >>>> + >>>> + ret = qcom_map_devmem(rproc, adsp->devmem, true, adsp->sid); >>>> + if (ret) { >>>> + dev_err(adsp->dev, "devmem iommu mapping failed, ret: %d\n", ret); >>>> + goto unmap_carveout; >>>> + } >>>> + >>>> ret = adsp_pds_enable(adsp, adsp->proxy_pds, adsp->proxy_pd_count); >>>> if (ret < 0) >>>> - goto disable_irqs; >>>> + goto unmap_devmem; >>>> ret = clk_prepare_enable(adsp->xo); >>>> if (ret) >>>> @@ -400,6 +413,10 @@ static int adsp_start(struct rproc *rproc) >>>> clk_disable_unprepare(adsp->xo); >>>> disable_proxy_pds: >>>> adsp_pds_disable(adsp, adsp->proxy_pds, adsp->proxy_pd_count); >>>> +unmap_devmem: >>>> + qcom_unmap_devmem(rproc, adsp->devmem, adsp->sid); >>>> +unmap_carveout: >>>> + qcom_map_unmap_carveout(rproc, adsp->mem_phys, adsp->mem_size, false, true, adsp->sid); >>>> disable_irqs: >>>> qcom_q6v5_unprepare(&adsp->q6v5); >>>> @@ -445,6 +462,9 @@ static int adsp_stop(struct rproc *rproc) >>>> dev_err(adsp->dev, "failed to shutdown dtb: %d\n", ret); >>>> } >>>> + qcom_unmap_devmem(rproc, adsp->devmem, adsp->sid); >>>> + qcom_map_unmap_carveout(rproc, adsp->mem_phys, adsp->mem_size, false, true, adsp->sid); >>>> + >>>> handover = qcom_q6v5_unprepare(&adsp->q6v5); >>>> if (handover) >>>> qcom_pas_handover(&adsp->q6v5); >>>> @@ -844,6 +864,25 @@ static int adsp_probe(struct platform_device *pdev) >>>> } >>>> platform_set_drvdata(pdev, adsp); >>>> + if (of_property_present(pdev->dev.of_node, "iommus")) { >>>> + struct of_phandle_args args; >>>> + >>>> + ret = of_parse_phandle_with_args(pdev->dev.of_node, "iommus", "#iommu-cells", 0, &args); >>>> + if (ret < 0) >>>> + return ret; >>>> + >>>> + rproc->has_iommu = true; >>>> + adsp->sid = args.args[0]; >>>> + of_node_put(args.np); >>>> + ret = adsp_devmem_init(adsp); >>>> + if (ret) >>>> + return ret; >>> >>> Why don't you get this table from the firmware like presumably QHEE does ? >> >> Well, AFAIK, QHEE(EL2) has this information statically present and does >> not get it from anywhere., but will confirm this twice.. > > Double confirmed, device memory region required by remoteproc is > statically present with QHEE. Right, in this case why those tables can't be embedded in the elf .resource_table like it's done with qcom_q6v5_adsp.c by calling rproc_elf_load_rsc_table() and let the remoteproc framework load the resource table and setup the devmem ssmu_map ? Neil > > -Mukesh > >> >> -Mukesh >>
On Thu, Oct 10, 2024 at 08:57:56AM +0200, neil.armstrong@linaro.org wrote: > On 08/10/2024 08:21, Mukesh Ojha wrote: > > On Mon, Oct 07, 2024 at 08:22:39PM +0530, Mukesh Ojha wrote: > > > On Mon, Oct 07, 2024 at 10:05:08AM +0200, neil.armstrong@linaro.org wrote: > > > > On 04/10/2024 23:23, Mukesh Ojha wrote: > > > > > For Qualcomm SoCs runnning with Qualcomm EL2 hypervisor(QHEE), IOMMU > > > > > translation for remote processors is managed by QHEE and if the same SoC > > > > > run under KVM, remoteproc carveout and devmem region should be IOMMU > > > > > mapped from Linux PAS driver before remoteproc is brought up and > > > > > unmapped once it is tear down and apart from this, SHM bridge also need > > > > > to set up to enable memory protection on both remoteproc meta data > > > > > memory as well as for the carveout region. > > > > > > > > > > Enable the support required to run Qualcomm remoteprocs on non-QHEE > > > > > hypervisors. > > > > > > > > > > Signed-off-by: Mukesh Ojha <quic_mojha@quicinc.com> > > > > > --- > > > > > drivers/remoteproc/qcom_q6v5_pas.c | 41 +++++++++++++++++++++++++++++- > > > > > 1 file changed, 40 insertions(+), 1 deletion(-) > > > > > > > > > > diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c > > > > > index ac339145e072..13bd13f1b989 100644 > > > > > --- a/drivers/remoteproc/qcom_q6v5_pas.c > > > > > +++ b/drivers/remoteproc/qcom_q6v5_pas.c <snip> > > > > > + struct of_phandle_args args; > > > > > + > > > > > + ret = of_parse_phandle_with_args(pdev->dev.of_node, "iommus", "#iommu-cells", 0, &args); > > > > > + if (ret < 0) > > > > > + return ret; > > > > > + > > > > > + rproc->has_iommu = true; > > > > > + adsp->sid = args.args[0]; > > > > > + of_node_put(args.np); > > > > > + ret = adsp_devmem_init(adsp); > > > > > + if (ret) > > > > > + return ret; > > > > > > > > Why don't you get this table from the firmware like presumably > > > > QHEE does ? > > > > > > Well, AFAIK, QHEE(EL2) has this information statically present > > > and does not get it from anywhere., but will confirm this > > > twice.. > > > > Double confirmed, device memory region required by remoteproc is > > statically present with QHEE. > > Right, in this case why those tables can't be embedded in the elf > .resource_table like it's done with qcom_q6v5_adsp.c by calling > rproc_elf_load_rsc_table() and let the remoteproc framework load the > resource table and setup the devmem ssmu_map ? Mainly for two reasons - firmware images on platforms where we like to bring additional no-qhee support do not have resource table. QCOM PAS implementation for secure remoteproc supports single TZ call of auth_and_rest that authenticates and brings remoteproc out of reset. And we don't have provision to authenticate resource table before it is used for devmem/iommu setup. regards Shiraz
On Fri, Oct 11, 2024 at 10:35:18AM GMT, Shiraz Hashim wrote: > On Thu, Oct 10, 2024 at 08:57:56AM +0200, neil.armstrong@linaro.org wrote: > > On 08/10/2024 08:21, Mukesh Ojha wrote: > > > On Mon, Oct 07, 2024 at 08:22:39PM +0530, Mukesh Ojha wrote: > > > > On Mon, Oct 07, 2024 at 10:05:08AM +0200, neil.armstrong@linaro.org wrote: > > > > > On 04/10/2024 23:23, Mukesh Ojha wrote: > > > > > > For Qualcomm SoCs runnning with Qualcomm EL2 hypervisor(QHEE), IOMMU > > > > > > translation for remote processors is managed by QHEE and if the same SoC > > > > > > run under KVM, remoteproc carveout and devmem region should be IOMMU > > > > > > mapped from Linux PAS driver before remoteproc is brought up and > > > > > > unmapped once it is tear down and apart from this, SHM bridge also need > > > > > > to set up to enable memory protection on both remoteproc meta data > > > > > > memory as well as for the carveout region. > > > > > > > > > > > > Enable the support required to run Qualcomm remoteprocs on non-QHEE > > > > > > hypervisors. > > > > > > > > > > > > Signed-off-by: Mukesh Ojha <quic_mojha@quicinc.com> > > > > > > --- > > > > > > drivers/remoteproc/qcom_q6v5_pas.c | 41 +++++++++++++++++++++++++++++- > > > > > > 1 file changed, 40 insertions(+), 1 deletion(-) > > > > > > > > > > > > diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c > > > > > > index ac339145e072..13bd13f1b989 100644 > > > > > > --- a/drivers/remoteproc/qcom_q6v5_pas.c > > > > > > +++ b/drivers/remoteproc/qcom_q6v5_pas.c > > <snip> > > > > > > > + struct of_phandle_args args; > > > > > > + > > > > > > + ret = of_parse_phandle_with_args(pdev->dev.of_node, "iommus", "#iommu-cells", 0, &args); > > > > > > + if (ret < 0) > > > > > > + return ret; > > > > > > + > > > > > > + rproc->has_iommu = true; > > > > > > + adsp->sid = args.args[0]; > > > > > > + of_node_put(args.np); > > > > > > + ret = adsp_devmem_init(adsp); > > > > > > + if (ret) > > > > > > + return ret; > > > > > > > > > > Why don't you get this table from the firmware like presumably > > > > > QHEE does ? > > > > > > > > Well, AFAIK, QHEE(EL2) has this information statically present > > > > and does not get it from anywhere., but will confirm this > > > > twice.. > > > > > > Double confirmed, device memory region required by remoteproc is > > > statically present with QHEE. > > > > Right, in this case why those tables can't be embedded in the elf > > .resource_table like it's done with qcom_q6v5_adsp.c by calling > > rproc_elf_load_rsc_table() and let the remoteproc framework load the > > resource table and setup the devmem ssmu_map ? > > Mainly for two reasons - > > firmware images on platforms where we like to bring additional no-qhee > support do not have resource table. > > QCOM PAS implementation for secure remoteproc supports single TZ call > of auth_and_rest that authenticates and brings remoteproc out of > reset. And we don't have provision to authenticate resource table > before it is used for devmem/iommu setup. So normally TZ / QHEE have the platform-specific resource table? Isn't it tied to the firmware binary?
On Fri, Oct 11, 2024 at 09:23:05AM +0300, Dmitry Baryshkov wrote: > On Fri, Oct 11, 2024 at 10:35:18AM GMT, Shiraz Hashim wrote: > > On Thu, Oct 10, 2024 at 08:57:56AM +0200, neil.armstrong@linaro.org wrote: > > > On 08/10/2024 08:21, Mukesh Ojha wrote: > > > > On Mon, Oct 07, 2024 at 08:22:39PM +0530, Mukesh Ojha wrote: > > > > > On Mon, Oct 07, 2024 at 10:05:08AM +0200, neil.armstrong@linaro.org wrote: > > > > > > On 04/10/2024 23:23, Mukesh Ojha wrote: > > > > > > > For Qualcomm SoCs runnning with Qualcomm EL2 hypervisor(QHEE), IOMMU > > > > > > > translation for remote processors is managed by QHEE and if the same SoC > > > > > > > run under KVM, remoteproc carveout and devmem region should be IOMMU > > > > > > > mapped from Linux PAS driver before remoteproc is brought up and > > > > > > > unmapped once it is tear down and apart from this, SHM bridge also need > > > > > > > to set up to enable memory protection on both remoteproc meta data > > > > > > > memory as well as for the carveout region. > > > > > > > > > > > > > > Enable the support required to run Qualcomm remoteprocs on non-QHEE > > > > > > > hypervisors. > > > > > > > > > > > > > > Signed-off-by: Mukesh Ojha <quic_mojha@quicinc.com> > > > > > > > --- > > > > > > > drivers/remoteproc/qcom_q6v5_pas.c | 41 +++++++++++++++++++++++++++++- > > > > > > > 1 file changed, 40 insertions(+), 1 deletion(-) > > > > > > > > > > > > > > diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c > > > > > > > index ac339145e072..13bd13f1b989 100644 > > > > > > > --- a/drivers/remoteproc/qcom_q6v5_pas.c > > > > > > > +++ b/drivers/remoteproc/qcom_q6v5_pas.c > > > > <snip> > > > > > > > > > + struct of_phandle_args args; > > > > > > > + > > > > > > > + ret = of_parse_phandle_with_args(pdev->dev.of_node, "iommus", "#iommu-cells", 0, &args); > > > > > > > + if (ret < 0) > > > > > > > + return ret; > > > > > > > + > > > > > > > + rproc->has_iommu = true; > > > > > > > + adsp->sid = args.args[0]; > > > > > > > + of_node_put(args.np); > > > > > > > + ret = adsp_devmem_init(adsp); > > > > > > > + if (ret) > > > > > > > + return ret; > > > > > > > > > > > > Why don't you get this table from the firmware like presumably > > > > > > QHEE does ? > > > > > > > > > > Well, AFAIK, QHEE(EL2) has this information statically present > > > > > and does not get it from anywhere., but will confirm this > > > > > twice.. > > > > > > > > Double confirmed, device memory region required by remoteproc is > > > > statically present with QHEE. > > > > > > Right, in this case why those tables can't be embedded in the elf > > > .resource_table like it's done with qcom_q6v5_adsp.c by calling > > > rproc_elf_load_rsc_table() and let the remoteproc framework load the > > > resource table and setup the devmem ssmu_map ? > > > > Mainly for two reasons - > > > > firmware images on platforms where we like to bring additional no-qhee > > support do not have resource table. > > > > QCOM PAS implementation for secure remoteproc supports single TZ call > > of auth_and_rest that authenticates and brings remoteproc out of > > reset. And we don't have provision to authenticate resource table > > before it is used for devmem/iommu setup. > > So normally TZ / QHEE have the platform-specific resource table? Isn't > it tied to the firmware binary? Yes this table is with QHEE and not firmware binary. Now with no-qhee case, this patch series is proposing to get it from device tree. regards Shiraz
On 11/10/2024 07:05, Shiraz Hashim wrote: > On Thu, Oct 10, 2024 at 08:57:56AM +0200, neil.armstrong@linaro.org wrote: >> On 08/10/2024 08:21, Mukesh Ojha wrote: >>> On Mon, Oct 07, 2024 at 08:22:39PM +0530, Mukesh Ojha wrote: >>>> On Mon, Oct 07, 2024 at 10:05:08AM +0200, neil.armstrong@linaro.org wrote: >>>>> On 04/10/2024 23:23, Mukesh Ojha wrote: >>>>>> For Qualcomm SoCs runnning with Qualcomm EL2 hypervisor(QHEE), IOMMU >>>>>> translation for remote processors is managed by QHEE and if the same SoC >>>>>> run under KVM, remoteproc carveout and devmem region should be IOMMU >>>>>> mapped from Linux PAS driver before remoteproc is brought up and >>>>>> unmapped once it is tear down and apart from this, SHM bridge also need >>>>>> to set up to enable memory protection on both remoteproc meta data >>>>>> memory as well as for the carveout region. >>>>>> >>>>>> Enable the support required to run Qualcomm remoteprocs on non-QHEE >>>>>> hypervisors. >>>>>> >>>>>> Signed-off-by: Mukesh Ojha <quic_mojha@quicinc.com> >>>>>> --- >>>>>> drivers/remoteproc/qcom_q6v5_pas.c | 41 +++++++++++++++++++++++++++++- >>>>>> 1 file changed, 40 insertions(+), 1 deletion(-) >>>>>> >>>>>> diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c >>>>>> index ac339145e072..13bd13f1b989 100644 >>>>>> --- a/drivers/remoteproc/qcom_q6v5_pas.c >>>>>> +++ b/drivers/remoteproc/qcom_q6v5_pas.c > > <snip> > >>>>>> + struct of_phandle_args args; >>>>>> + >>>>>> + ret = of_parse_phandle_with_args(pdev->dev.of_node, "iommus", "#iommu-cells", 0, &args); >>>>>> + if (ret < 0) >>>>>> + return ret; >>>>>> + >>>>>> + rproc->has_iommu = true; >>>>>> + adsp->sid = args.args[0]; >>>>>> + of_node_put(args.np); >>>>>> + ret = adsp_devmem_init(adsp); >>>>>> + if (ret) >>>>>> + return ret; >>>>> >>>>> Why don't you get this table from the firmware like presumably >>>>> QHEE does ? >>>> >>>> Well, AFAIK, QHEE(EL2) has this information statically present >>>> and does not get it from anywhere., but will confirm this >>>> twice.. >>> >>> Double confirmed, device memory region required by remoteproc is >>> statically present with QHEE. >> >> Right, in this case why those tables can't be embedded in the elf >> .resource_table like it's done with qcom_q6v5_adsp.c by calling >> rproc_elf_load_rsc_table() and let the remoteproc framework load the >> resource table and setup the devmem ssmu_map ? > > Mainly for two reasons - > > firmware images on platforms where we like to bring additional no-qhee > support do not have resource table. > > QCOM PAS implementation for secure remoteproc supports single TZ call > of auth_and_rest that authenticates and brings remoteproc out of > reset. And we don't have provision to authenticate resource table > before it is used for devmem/iommu setup. Why not authenticate a separate binary containing the resource table ? Adding the resources to DT is a no go since it's clearly related to what the firmare will be using at runtime, so either it should go in a .resource_table section or can be moved in a signed .mbn that can be authenticated by TZ. Remoteproc doesn't dictate how to load the resource table, there's helpers to load it from an elf, but it can be loaded by other means. Neil > > regards > Shiraz
On Fri, Oct 11, 2024 at 09:23:05AM +0300, Dmitry Baryshkov wrote: > On Fri, Oct 11, 2024 at 10:35:18AM GMT, Shiraz Hashim wrote: > > On Thu, Oct 10, 2024 at 08:57:56AM +0200, neil.armstrong@linaro.org wrote: > > > On 08/10/2024 08:21, Mukesh Ojha wrote: > > > > On Mon, Oct 07, 2024 at 08:22:39PM +0530, Mukesh Ojha wrote: > > > > > On Mon, Oct 07, 2024 at 10:05:08AM +0200, neil.armstrong@linaro.org wrote: > > > > > > On 04/10/2024 23:23, Mukesh Ojha wrote: > > > > > > > For Qualcomm SoCs runnning with Qualcomm EL2 hypervisor(QHEE), IOMMU > > > > > > > translation for remote processors is managed by QHEE and if the same SoC > > > > > > > run under KVM, remoteproc carveout and devmem region should be IOMMU > > > > > > > mapped from Linux PAS driver before remoteproc is brought up and > > > > > > > unmapped once it is tear down and apart from this, SHM bridge also need > > > > > > > to set up to enable memory protection on both remoteproc meta data > > > > > > > memory as well as for the carveout region. > > > > > > > > > > > > > > Enable the support required to run Qualcomm remoteprocs on non-QHEE > > > > > > > hypervisors. > > > > > > > > > > > > > > Signed-off-by: Mukesh Ojha <quic_mojha@quicinc.com> > > > > > > > --- > > > > > > > drivers/remoteproc/qcom_q6v5_pas.c | 41 +++++++++++++++++++++++++++++- > > > > > > > 1 file changed, 40 insertions(+), 1 deletion(-) > > > > > > > > > > > > > > diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c > > > > > > > index ac339145e072..13bd13f1b989 100644 > > > > > > > --- a/drivers/remoteproc/qcom_q6v5_pas.c > > > > > > > +++ b/drivers/remoteproc/qcom_q6v5_pas.c > > > > <snip> > > > > > > > > > + struct of_phandle_args args; > > > > > > > + > > > > > > > + ret = of_parse_phandle_with_args(pdev->dev.of_node, "iommus", "#iommu-cells", 0, &args); > > > > > > > + if (ret < 0) > > > > > > > + return ret; > > > > > > > + > > > > > > > + rproc->has_iommu = true; > > > > > > > + adsp->sid = args.args[0]; > > > > > > > + of_node_put(args.np); > > > > > > > + ret = adsp_devmem_init(adsp); > > > > > > > + if (ret) > > > > > > > + return ret; > > > > > > > > > > > > Why don't you get this table from the firmware like presumably > > > > > > QHEE does ? > > > > > > > > > > Well, AFAIK, QHEE(EL2) has this information statically present > > > > > and does not get it from anywhere., but will confirm this > > > > > twice.. > > > > > > > > Double confirmed, device memory region required by remoteproc is > > > > statically present with QHEE. > > > > > > Right, in this case why those tables can't be embedded in the elf > > > .resource_table like it's done with qcom_q6v5_adsp.c by calling > > > rproc_elf_load_rsc_table() and let the remoteproc framework load the > > > resource table and setup the devmem ssmu_map ? > > > > Mainly for two reasons - > > > > firmware images on platforms where we like to bring additional no-qhee > > support do not have resource table. > > > > QCOM PAS implementation for secure remoteproc supports single TZ call > > of auth_and_rest that authenticates and brings remoteproc out of > > reset. And we don't have provision to authenticate resource table > > before it is used for devmem/iommu setup. > > So normally TZ / QHEE have the platform-specific resource table? Isn't > it tied to the firmware binary? It is not TZ, but QHEE that statically keeps remoteproc resources with it. -Mukesh
On Fri, 11 Oct 2024 at 10:09, Shiraz Hashim <quic_shashim@quicinc.com> wrote: > > On Fri, Oct 11, 2024 at 09:23:05AM +0300, Dmitry Baryshkov wrote: > > On Fri, Oct 11, 2024 at 10:35:18AM GMT, Shiraz Hashim wrote: > > > On Thu, Oct 10, 2024 at 08:57:56AM +0200, neil.armstrong@linaro.org wrote: > > > > On 08/10/2024 08:21, Mukesh Ojha wrote: > > > > > On Mon, Oct 07, 2024 at 08:22:39PM +0530, Mukesh Ojha wrote: > > > > > > On Mon, Oct 07, 2024 at 10:05:08AM +0200, neil.armstrong@linaro.org wrote: > > > > > > > On 04/10/2024 23:23, Mukesh Ojha wrote: > > > > > > > > For Qualcomm SoCs runnning with Qualcomm EL2 hypervisor(QHEE), IOMMU > > > > > > > > translation for remote processors is managed by QHEE and if the same SoC > > > > > > > > run under KVM, remoteproc carveout and devmem region should be IOMMU > > > > > > > > mapped from Linux PAS driver before remoteproc is brought up and > > > > > > > > unmapped once it is tear down and apart from this, SHM bridge also need > > > > > > > > to set up to enable memory protection on both remoteproc meta data > > > > > > > > memory as well as for the carveout region. > > > > > > > > > > > > > > > > Enable the support required to run Qualcomm remoteprocs on non-QHEE > > > > > > > > hypervisors. > > > > > > > > > > > > > > > > Signed-off-by: Mukesh Ojha <quic_mojha@quicinc.com> > > > > > > > > --- > > > > > > > > drivers/remoteproc/qcom_q6v5_pas.c | 41 +++++++++++++++++++++++++++++- > > > > > > > > 1 file changed, 40 insertions(+), 1 deletion(-) > > > > > > > > > > > > > > > > diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c > > > > > > > > index ac339145e072..13bd13f1b989 100644 > > > > > > > > --- a/drivers/remoteproc/qcom_q6v5_pas.c > > > > > > > > +++ b/drivers/remoteproc/qcom_q6v5_pas.c > > > > > > <snip> > > > > > > > > > > > + struct of_phandle_args args; > > > > > > > > + > > > > > > > > + ret = of_parse_phandle_with_args(pdev->dev.of_node, "iommus", "#iommu-cells", 0, &args); > > > > > > > > + if (ret < 0) > > > > > > > > + return ret; > > > > > > > > + > > > > > > > > + rproc->has_iommu = true; > > > > > > > > + adsp->sid = args.args[0]; > > > > > > > > + of_node_put(args.np); > > > > > > > > + ret = adsp_devmem_init(adsp); > > > > > > > > + if (ret) > > > > > > > > + return ret; > > > > > > > > > > > > > > Why don't you get this table from the firmware like presumably > > > > > > > QHEE does ? > > > > > > > > > > > > Well, AFAIK, QHEE(EL2) has this information statically present > > > > > > and does not get it from anywhere., but will confirm this > > > > > > twice.. > > > > > > > > > > Double confirmed, device memory region required by remoteproc is > > > > > statically present with QHEE. > > > > > > > > Right, in this case why those tables can't be embedded in the elf > > > > .resource_table like it's done with qcom_q6v5_adsp.c by calling > > > > rproc_elf_load_rsc_table() and let the remoteproc framework load the > > > > resource table and setup the devmem ssmu_map ? > > > > > > Mainly for two reasons - > > > > > > firmware images on platforms where we like to bring additional no-qhee > > > support do not have resource table. > > > > > > QCOM PAS implementation for secure remoteproc supports single TZ call > > > of auth_and_rest that authenticates and brings remoteproc out of > > > reset. And we don't have provision to authenticate resource table > > > before it is used for devmem/iommu setup. > > > > So normally TZ / QHEE have the platform-specific resource table? Isn't > > it tied to the firmware binary? > > Yes this table is with QHEE and not firmware binary. Now with no-qhee > case, this patch series is proposing to get it from device tree. If it is platform-specific (rather than being device-specific), then it should go to the driver, not the DT.
On Fri, Oct 11, 2024 at 09:09:08AM +0200, neil.armstrong@linaro.org wrote: > On 11/10/2024 07:05, Shiraz Hashim wrote: > > On Thu, Oct 10, 2024 at 08:57:56AM +0200, neil.armstrong@linaro.org wrote: > > > On 08/10/2024 08:21, Mukesh Ojha wrote: > > > > On Mon, Oct 07, 2024 at 08:22:39PM +0530, Mukesh Ojha wrote: > > > > > On Mon, Oct 07, 2024 at 10:05:08AM +0200, neil.armstrong@linaro.org wrote: > > > > > > On 04/10/2024 23:23, Mukesh Ojha wrote: > > > > > > > For Qualcomm SoCs runnning with Qualcomm EL2 hypervisor(QHEE), IOMMU > > > > > > > translation for remote processors is managed by QHEE and if the same SoC > > > > > > > run under KVM, remoteproc carveout and devmem region should be IOMMU > > > > > > > mapped from Linux PAS driver before remoteproc is brought up and > > > > > > > unmapped once it is tear down and apart from this, SHM bridge also need > > > > > > > to set up to enable memory protection on both remoteproc meta data > > > > > > > memory as well as for the carveout region. > > > > > > > > > > > > > > Enable the support required to run Qualcomm remoteprocs on non-QHEE > > > > > > > hypervisors. > > > > > > > > > > > > > > Signed-off-by: Mukesh Ojha <quic_mojha@quicinc.com> > > > > > > > --- > > > > > > > drivers/remoteproc/qcom_q6v5_pas.c | 41 +++++++++++++++++++++++++++++- > > > > > > > 1 file changed, 40 insertions(+), 1 deletion(-) > > > > > > > > > > > > > > diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c > > > > > > > index ac339145e072..13bd13f1b989 100644 > > > > > > > --- a/drivers/remoteproc/qcom_q6v5_pas.c > > > > > > > +++ b/drivers/remoteproc/qcom_q6v5_pas.c > > > > <snip> > > > > > > > > > + struct of_phandle_args args; > > > > > > > + > > > > > > > + ret = of_parse_phandle_with_args(pdev->dev.of_node, "iommus", "#iommu-cells", 0, &args); > > > > > > > + if (ret < 0) > > > > > > > + return ret; > > > > > > > + > > > > > > > + rproc->has_iommu = true; > > > > > > > + adsp->sid = args.args[0]; > > > > > > > + of_node_put(args.np); > > > > > > > + ret = adsp_devmem_init(adsp); > > > > > > > + if (ret) > > > > > > > + return ret; > > > > > > > > > > > > Why don't you get this table from the firmware like presumably > > > > > > QHEE does ? > > > > > > > > > > Well, AFAIK, QHEE(EL2) has this information statically present > > > > > and does not get it from anywhere., but will confirm this > > > > > twice.. > > > > > > > > Double confirmed, device memory region required by remoteproc is > > > > statically present with QHEE. > > > > > > Right, in this case why those tables can't be embedded in the elf > > > .resource_table like it's done with qcom_q6v5_adsp.c by calling > > > rproc_elf_load_rsc_table() and let the remoteproc framework load the > > > resource table and setup the devmem ssmu_map ? > > > > Mainly for two reasons - > > > > firmware images on platforms where we like to bring additional no-qhee > > support do not have resource table. > > > > QCOM PAS implementation for secure remoteproc supports single TZ call > > of auth_and_rest that authenticates and brings remoteproc out of > > reset. And we don't have provision to authenticate resource table > > before it is used for devmem/iommu setup. > > Why not authenticate a separate binary containing the resource table ? > > Adding the resources to DT is a no go since it's clearly related to what > the firmare will be using at runtime, Sorry didn't understand how is it classified as runtime. Similar to resources required to bring up a device, these correspond to resources required to be handled before bringing up a remoteproc. > so either it should go in a .resource_table section or can be moved > in a signed .mbn that can be authenticated by TZ. TZ doesn't have a separate authentication call as of now. If DT is strictly a no go, would moving it to driver itself be an acceptable option ? inline with what Dmitry suggesting. regards Shiraz
On Fri, Oct 11, 2024 at 10:12:09AM +0300, Dmitry Baryshkov wrote: > On Fri, 11 Oct 2024 at 10:09, Shiraz Hashim <quic_shashim@quicinc.com> wrote: > > > > On Fri, Oct 11, 2024 at 09:23:05AM +0300, Dmitry Baryshkov wrote: > > > On Fri, Oct 11, 2024 at 10:35:18AM GMT, Shiraz Hashim wrote: > > > > On Thu, Oct 10, 2024 at 08:57:56AM +0200, neil.armstrong@linaro.org wrote: > > > > > On 08/10/2024 08:21, Mukesh Ojha wrote: > > > > > > On Mon, Oct 07, 2024 at 08:22:39PM +0530, Mukesh Ojha wrote: > > > > > > > On Mon, Oct 07, 2024 at 10:05:08AM +0200, neil.armstrong@linaro.org wrote: > > > > > > > > On 04/10/2024 23:23, Mukesh Ojha wrote: > > > > > > > > > For Qualcomm SoCs runnning with Qualcomm EL2 hypervisor(QHEE), IOMMU > > > > > > > > > translation for remote processors is managed by QHEE and if the same SoC > > > > > > > > > run under KVM, remoteproc carveout and devmem region should be IOMMU > > > > > > > > > mapped from Linux PAS driver before remoteproc is brought up and > > > > > > > > > unmapped once it is tear down and apart from this, SHM bridge also need > > > > > > > > > to set up to enable memory protection on both remoteproc meta data > > > > > > > > > memory as well as for the carveout region. > > > > > > > > > > > > > > > > > > Enable the support required to run Qualcomm remoteprocs on non-QHEE > > > > > > > > > hypervisors. > > > > > > > > > > > > > > > > > > Signed-off-by: Mukesh Ojha <quic_mojha@quicinc.com> > > > > > > > > > --- > > > > > > > > > drivers/remoteproc/qcom_q6v5_pas.c | 41 +++++++++++++++++++++++++++++- > > > > > > > > > 1 file changed, 40 insertions(+), 1 deletion(-) > > > > > > > > > > > > > > > > > > diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c > > > > > > > > > index ac339145e072..13bd13f1b989 100644 > > > > > > > > > --- a/drivers/remoteproc/qcom_q6v5_pas.c > > > > > > > > > +++ b/drivers/remoteproc/qcom_q6v5_pas.c > > > > > > > > <snip> > > > > > > > > > > > > > + struct of_phandle_args args; > > > > > > > > > + > > > > > > > > > + ret = of_parse_phandle_with_args(pdev->dev.of_node, "iommus", "#iommu-cells", 0, &args); > > > > > > > > > + if (ret < 0) > > > > > > > > > + return ret; > > > > > > > > > + > > > > > > > > > + rproc->has_iommu = true; > > > > > > > > > + adsp->sid = args.args[0]; > > > > > > > > > + of_node_put(args.np); > > > > > > > > > + ret = adsp_devmem_init(adsp); > > > > > > > > > + if (ret) > > > > > > > > > + return ret; > > > > > > > > > > > > > > > > Why don't you get this table from the firmware like presumably > > > > > > > > QHEE does ? > > > > > > > > > > > > > > Well, AFAIK, QHEE(EL2) has this information statically present > > > > > > > and does not get it from anywhere., but will confirm this > > > > > > > twice.. > > > > > > > > > > > > Double confirmed, device memory region required by remoteproc is > > > > > > statically present with QHEE. > > > > > > > > > > Right, in this case why those tables can't be embedded in the elf > > > > > .resource_table like it's done with qcom_q6v5_adsp.c by calling > > > > > rproc_elf_load_rsc_table() and let the remoteproc framework load the > > > > > resource table and setup the devmem ssmu_map ? > > > > > > > > Mainly for two reasons - > > > > > > > > firmware images on platforms where we like to bring additional no-qhee > > > > support do not have resource table. > > > > > > > > QCOM PAS implementation for secure remoteproc supports single TZ call > > > > of auth_and_rest that authenticates and brings remoteproc out of > > > > reset. And we don't have provision to authenticate resource table > > > > before it is used for devmem/iommu setup. > > > > > > So normally TZ / QHEE have the platform-specific resource table? Isn't > > > it tied to the firmware binary? > > > > Yes this table is with QHEE and not firmware binary. Now with no-qhee > > case, this patch series is proposing to get it from device tree. > > If it is platform-specific (rather than being device-specific), then > it should go to the driver, not the DT. Just to be clear, your reference to platform is SoC specific and device is board ? regards Shiraz
On Mon, 14 Oct 2024 at 15:31, Shiraz Hashim <quic_shashim@quicinc.com> wrote: > > On Fri, Oct 11, 2024 at 10:12:09AM +0300, Dmitry Baryshkov wrote: > > On Fri, 11 Oct 2024 at 10:09, Shiraz Hashim <quic_shashim@quicinc.com> wrote: > > > > > > On Fri, Oct 11, 2024 at 09:23:05AM +0300, Dmitry Baryshkov wrote: > > > > On Fri, Oct 11, 2024 at 10:35:18AM GMT, Shiraz Hashim wrote: > > > > > On Thu, Oct 10, 2024 at 08:57:56AM +0200, neil.armstrong@linaro.org wrote: > > > > > > On 08/10/2024 08:21, Mukesh Ojha wrote: > > > > > > > On Mon, Oct 07, 2024 at 08:22:39PM +0530, Mukesh Ojha wrote: > > > > > > > > On Mon, Oct 07, 2024 at 10:05:08AM +0200, neil.armstrong@linaro.org wrote: > > > > > > > > > On 04/10/2024 23:23, Mukesh Ojha wrote: > > > > > > > > > > For Qualcomm SoCs runnning with Qualcomm EL2 hypervisor(QHEE), IOMMU > > > > > > > > > > translation for remote processors is managed by QHEE and if the same SoC > > > > > > > > > > run under KVM, remoteproc carveout and devmem region should be IOMMU > > > > > > > > > > mapped from Linux PAS driver before remoteproc is brought up and > > > > > > > > > > unmapped once it is tear down and apart from this, SHM bridge also need > > > > > > > > > > to set up to enable memory protection on both remoteproc meta data > > > > > > > > > > memory as well as for the carveout region. > > > > > > > > > > > > > > > > > > > > Enable the support required to run Qualcomm remoteprocs on non-QHEE > > > > > > > > > > hypervisors. > > > > > > > > > > > > > > > > > > > > Signed-off-by: Mukesh Ojha <quic_mojha@quicinc.com> > > > > > > > > > > --- > > > > > > > > > > drivers/remoteproc/qcom_q6v5_pas.c | 41 +++++++++++++++++++++++++++++- > > > > > > > > > > 1 file changed, 40 insertions(+), 1 deletion(-) > > > > > > > > > > > > > > > > > > > > diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c > > > > > > > > > > index ac339145e072..13bd13f1b989 100644 > > > > > > > > > > --- a/drivers/remoteproc/qcom_q6v5_pas.c > > > > > > > > > > +++ b/drivers/remoteproc/qcom_q6v5_pas.c > > > > > > > > > > <snip> > > > > > > > > > > > > > > > + struct of_phandle_args args; > > > > > > > > > > + > > > > > > > > > > + ret = of_parse_phandle_with_args(pdev->dev.of_node, "iommus", "#iommu-cells", 0, &args); > > > > > > > > > > + if (ret < 0) > > > > > > > > > > + return ret; > > > > > > > > > > + > > > > > > > > > > + rproc->has_iommu = true; > > > > > > > > > > + adsp->sid = args.args[0]; > > > > > > > > > > + of_node_put(args.np); > > > > > > > > > > + ret = adsp_devmem_init(adsp); > > > > > > > > > > + if (ret) > > > > > > > > > > + return ret; > > > > > > > > > > > > > > > > > > Why don't you get this table from the firmware like presumably > > > > > > > > > QHEE does ? > > > > > > > > > > > > > > > > Well, AFAIK, QHEE(EL2) has this information statically present > > > > > > > > and does not get it from anywhere., but will confirm this > > > > > > > > twice.. > > > > > > > > > > > > > > Double confirmed, device memory region required by remoteproc is > > > > > > > statically present with QHEE. > > > > > > > > > > > > Right, in this case why those tables can't be embedded in the elf > > > > > > .resource_table like it's done with qcom_q6v5_adsp.c by calling > > > > > > rproc_elf_load_rsc_table() and let the remoteproc framework load the > > > > > > resource table and setup the devmem ssmu_map ? > > > > > > > > > > Mainly for two reasons - > > > > > > > > > > firmware images on platforms where we like to bring additional no-qhee > > > > > support do not have resource table. > > > > > > > > > > QCOM PAS implementation for secure remoteproc supports single TZ call > > > > > of auth_and_rest that authenticates and brings remoteproc out of > > > > > reset. And we don't have provision to authenticate resource table > > > > > before it is used for devmem/iommu setup. > > > > > > > > So normally TZ / QHEE have the platform-specific resource table? Isn't > > > > it tied to the firmware binary? > > > > > > Yes this table is with QHEE and not firmware binary. Now with no-qhee > > > case, this patch series is proposing to get it from device tree. > > > > If it is platform-specific (rather than being device-specific), then > > it should go to the driver, not the DT. > > Just to be clear, your reference to platform is SoC specific and > device is board ? Yes.
On 4.10.2024 11:23 PM, Mukesh Ojha wrote: > For Qualcomm SoCs runnning with Qualcomm EL2 hypervisor(QHEE), IOMMU > translation for remote processors is managed by QHEE and if the same SoC > run under KVM, remoteproc carveout and devmem region should be IOMMU > mapped from Linux PAS driver before remoteproc is brought up and > unmapped once it is tear down and apart from this, SHM bridge also need > to set up to enable memory protection on both remoteproc meta data > memory as well as for the carveout region. !Gunyah != KVM > Enable the support required to run Qualcomm remoteprocs on non-QHEE > hypervisors. > > Signed-off-by: Mukesh Ojha <quic_mojha@quicinc.com> > --- [...] > > + if (of_property_present(pdev->dev.of_node, "iommus")) { > + struct of_phandle_args args; > + > + ret = of_parse_phandle_with_args(pdev->dev.of_node, "iommus", "#iommu-cells", 0, &args); > + if (ret < 0) > + return ret; > + > + rproc->has_iommu = true; > + adsp->sid = args.args[0]; Do we ignore the SMR mask completely? This ties the implementation very closely to arm-smmu-v2. While I don't expect any changes in there, this is something to keep in mind.. Konrad
diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c index ac339145e072..13bd13f1b989 100644 --- a/drivers/remoteproc/qcom_q6v5_pas.c +++ b/drivers/remoteproc/qcom_q6v5_pas.c @@ -122,6 +122,7 @@ struct qcom_adsp { struct qcom_devmem_table *devmem; struct qcom_tzmem_area *tzmem; + unsigned long sid; }; static void adsp_segment_dump(struct rproc *rproc, struct rproc_dump_segment *segment, @@ -310,9 +311,21 @@ static int adsp_start(struct rproc *rproc) if (ret) return ret; + ret = qcom_map_unmap_carveout(rproc, adsp->mem_phys, adsp->mem_size, true, true, adsp->sid); + if (ret) { + dev_err(adsp->dev, "iommu mapping failed, ret: %d\n", ret); + goto disable_irqs; + } + + ret = qcom_map_devmem(rproc, adsp->devmem, true, adsp->sid); + if (ret) { + dev_err(adsp->dev, "devmem iommu mapping failed, ret: %d\n", ret); + goto unmap_carveout; + } + ret = adsp_pds_enable(adsp, adsp->proxy_pds, adsp->proxy_pd_count); if (ret < 0) - goto disable_irqs; + goto unmap_devmem; ret = clk_prepare_enable(adsp->xo); if (ret) @@ -400,6 +413,10 @@ static int adsp_start(struct rproc *rproc) clk_disable_unprepare(adsp->xo); disable_proxy_pds: adsp_pds_disable(adsp, adsp->proxy_pds, adsp->proxy_pd_count); +unmap_devmem: + qcom_unmap_devmem(rproc, adsp->devmem, adsp->sid); +unmap_carveout: + qcom_map_unmap_carveout(rproc, adsp->mem_phys, adsp->mem_size, false, true, adsp->sid); disable_irqs: qcom_q6v5_unprepare(&adsp->q6v5); @@ -445,6 +462,9 @@ static int adsp_stop(struct rproc *rproc) dev_err(adsp->dev, "failed to shutdown dtb: %d\n", ret); } + qcom_unmap_devmem(rproc, adsp->devmem, adsp->sid); + qcom_map_unmap_carveout(rproc, adsp->mem_phys, adsp->mem_size, false, true, adsp->sid); + handover = qcom_q6v5_unprepare(&adsp->q6v5); if (handover) qcom_pas_handover(&adsp->q6v5); @@ -844,6 +864,25 @@ static int adsp_probe(struct platform_device *pdev) } platform_set_drvdata(pdev, adsp); + if (of_property_present(pdev->dev.of_node, "iommus")) { + struct of_phandle_args args; + + ret = of_parse_phandle_with_args(pdev->dev.of_node, "iommus", "#iommu-cells", 0, &args); + if (ret < 0) + return ret; + + rproc->has_iommu = true; + adsp->sid = args.args[0]; + of_node_put(args.np); + ret = adsp_devmem_init(adsp); + if (ret) + return ret; + + adsp->pas_metadata.shm_bridge_needed = true; + } else { + rproc->has_iommu = false; + } + ret = device_init_wakeup(adsp->dev, true); if (ret) goto free_rproc;
For Qualcomm SoCs runnning with Qualcomm EL2 hypervisor(QHEE), IOMMU translation for remote processors is managed by QHEE and if the same SoC run under KVM, remoteproc carveout and devmem region should be IOMMU mapped from Linux PAS driver before remoteproc is brought up and unmapped once it is tear down and apart from this, SHM bridge also need to set up to enable memory protection on both remoteproc meta data memory as well as for the carveout region. Enable the support required to run Qualcomm remoteprocs on non-QHEE hypervisors. Signed-off-by: Mukesh Ojha <quic_mojha@quicinc.com> --- drivers/remoteproc/qcom_q6v5_pas.c | 41 +++++++++++++++++++++++++++++- 1 file changed, 40 insertions(+), 1 deletion(-)