diff mbox series

[6/6] remoteproc: qcom: Enable map/unmap and SHM bridge support

Message ID 20241004212359.2263502-7-quic_mojha@quicinc.com
State New
Headers show
Series Peripheral Image Loader support for Qualcomm SoCs | expand

Commit Message

Mukesh Ojha Oct. 4, 2024, 9:23 p.m. UTC
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(-)

Comments

Neil Armstrong Oct. 7, 2024, 8:05 a.m. UTC | #1
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;
Mukesh Ojha Oct. 7, 2024, 2:52 p.m. UTC | #2
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
Mukesh Ojha Oct. 8, 2024, 6:21 a.m. UTC | #3
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
>
Neil Armstrong Oct. 10, 2024, 6:57 a.m. UTC | #4
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
>>
Dmitry Baryshkov Oct. 11, 2024, 6:23 a.m. UTC | #5
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?
Shiraz Hashim Oct. 11, 2024, 7:09 a.m. UTC | #6
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
Mukesh Ojha Oct. 11, 2024, 7:11 a.m. UTC | #7
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
Dmitry Baryshkov Oct. 11, 2024, 7:12 a.m. UTC | #8
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.
Shiraz Hashim Oct. 14, 2024, 12:29 p.m. UTC | #9
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
Shiraz Hashim Oct. 14, 2024, 12:31 p.m. UTC | #10
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
Dmitry Baryshkov Oct. 14, 2024, 12:38 p.m. UTC | #11
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.
diff mbox series

Patch

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;