Re: [PATCH] arm64: dts: sdm845: Add iommus property to qup1

From: Vivek Gautam
Date: Wed Jun 12 2019 - 05:31:01 EST



On 6/11/2019 4:51 AM, Stephen Boyd wrote:
Quoting Vivek Gautam (2019-06-06 04:17:16)
Hi Stephen,

On Thu, Jun 6, 2019 at 2:27 AM Stephen Boyd <swboyd@xxxxxxxxxxxx> wrote:
Quoting Vivek Gautam (2019-06-04 21:55:26)

Cheza will throw faults for anything that is programmed with TZ on mtp
as all of that should be handled in HLOS. The firmwares of all these
peripherals assume that the SID reservation is done (whether in TZ or HLOS).

I am inclined to moving the iommus property for all 'TZ' to board dts files.
MTP wouldn't need those SIDs. So, the SOC level dtsi will have just the
HLOS SIDs.
So you're saying you'd like to have the SID be <&apps_smmu 0x6c3 0x0> in
the sdm845.dtsi file and then override this on Cheza because our SID is
different (possibly because we don't use GSI)? Why can't we program the
SID in Cheza firmware to match the "HLOS" SID of 0x6c3?
Sorry my bad, I missed the overriding part.
May be we add the lists of SIDs in board dts only. So, cheza dts will
have all these SIDs -
<&apps_smmu 0x6c0 0x3> // for both 0x6c0 (TZ) and 0x6c3 (HLOS)
<&apps_smmu 0x6d6 0x0> // if we want to use the GSI dma.
and
MTP will have
<&apps_smmu 0x6c3 0x0>
<&apps_smmu 0x6d6 0x0>
WDUT?
I'd prefer to fix the firmware so that the HLOS SID is used even on this
board. Making Cheza use something different from MTP doesn't sound so
good. Do you know how that works? Is there some configuration register
or something that I should be looking for to see why the SID is not the
HLOS one? It's definitely generating SIDs for the TZ SID (0x6c0), but
I'd like to make sure that we can't change it because it's tied to some
hardware signal like the NS bit and/or the Execution Level. Hopefully
it's a config and then our difference from MTP can be minimized.

I don't think SMMU limits any such programming of SIDs. It's a design decision
to program few SIDs in TZ/Hyp and allocate the corresponding context banks
and create respective mappings. You should be able to see these SMR configurations
before kernel boots up. I use a simple T32 command -

smmu.add "<name>" <smmu_type> <base_address>
smmu.streammaptable <name>

e.g. for sdm845 apps_smmu

smmu.add "apps" MMU500 a:0x15000000
smmu.StreamMapTable apps

This dumps all the information regarding the smmu.


As far as I can tell, HLOS on SDM845 has always used GPI (yet another
DMA engine) to do the DMA transfers. And the GPI is the hardware block
that uses the SID of 0x6d6 above, so putting that into iommus for the
geniqup doesn't make any sense given that GPI is another node. Can you
confirm this is the case? Furthermore, the SID of 0x6c3 sounds untested?
Has it ever been generated on SDM845 MTP?

I will get back with this information.

BRs
Vivek


If we ever support GPI, I'd expect to see something like this in DT:

gpi_dma: gpi@a00000 {
reg = <0x00a00000 0x60000>;
iommus = <&apps_smmu 0x6d6 0x0>;
...
};

geniqup@ac0000 {
reg = <0x00ac0000 0x6000>;
iommus = <&apps_smmu 0x6c3 0x0>;

i2c@....{

dmas = <&gpi_dma ....>;
};

But now I'm worried that the geniqup needs the proper geniqup wrapper
clks to talk to it. Most likely the GPI is embedded inside the geniqup
wrapper and sits right next to the bus to do bus DMA mastering. From the
DT side, it means we should either put it inside the geniqup node, or we
should add the wrapper clks to the GPI node and hope things work out
with regards to clks and shared resources being used at the right time.

If we're left with trying to figure out how to express the different
SIDs depending on the CPU execution state then it may be easier to push
for GPI upstreaming and use that dma engine to "fold" the SID
numberspace into one SID for the GPI. This would avoid having to deal
with the HLOS vs. TZ SID problem by adding a whole other driver. Or we
could just rip out the non-GPI DMA support in this driver because the
SID is all confused.