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

[Vivek Gautam <vivek.gautam@codeaurora.org>]

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@chromium.org> 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.
>