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

[neil . armstrong]

On 11/10/2024 07:05, Shiraz Hashim wrote:
> On Thu, Oct 10, 2024 at 08:57:56AM +0200, neil.armstrong@xxxxxxxxxx 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@xxxxxxxxxx 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@xxxxxxxxxxx>
> > > > > > ---
> > > > > >     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