SMR masking and PCI

[Stuart Yoder]

> -----Original Message-----
> From: Robin Murphy [mailto:robin.murphy at arm.com]
> Sent: Friday, October 28, 2016 11:17 AM
> To: Stuart Yoder <stuart.yoder at nxp.com>; Mark Rutland <mark.rutland at arm.com>
> Cc: linux-arm-kernel at lists.infradead.org; Will Deacon <will.deacon at arm.com>; Diana Madalina Craciun
> <diana.craciun at nxp.com>; Nipun Gupta <nipun.gupta at nxp.com>; iommu at lists.linux-foundation.org
> Subject: Re: SMR masking and PCI
> 
> Hi Stuart,
> 
> On 27/10/16 18:10, Stuart Yoder wrote:
> > Hi Robin,
> >
> > A question about how the SMR masking defined in the arm,smmu binding
> > relates to the PCI iommu-map.
> >
> > The #iommu-cells property defines the number of cells an "IOMMU specifier"
> > takes and 2 is specified to be:
> >
> >    SMMUs with stream matching support and complex masters
> >    may use a value of 2, where the second cell represents
> >    an SMR mask to combine with the ID in the first cell.
> >
> > An iommu-map entry is defined as:
> >
> >    (rid-base,iommu,iommu-base,length)
> >
> > What seems to be currently missing in the iommu-map support is
> > the possibility the case where #iommu-cells=<2>.
> 
> Indeed. The bindings have so far rather implicitly assumed the case of
> #{msi,iommu}-cells = 1, and the code has followed suit.
> 
> > In this case iommu-base which is an IOMMU specifier should
> > occupy 2 cells.  For example on an ls2085a we would want:
> >
> > 	iommu-map = <0x0   0x6 0x7 0x3ff 0x1
> > 		       0x100 0x6 0x8 0x3ff 0x1>;
> >
> > ...to mask our stream IDs to 10 bits.
> >
> > This should work in theory and comply with the bindings, no?
> 
> In theory, but now consider:
> 
> 	iommu-map = <0x0 0x6 0x7 0x3ff 0x2>;
> 
> faced with ID 1. The input base is 0, the output base is the 2-cell
> value 0x7000003ff, so the final ID value should be 0x700000400, right?

No.  The second cell as per the SMMU binding is the SMR mask...applied
by the SMMU before matching stream IDs.

In our case we want to mask off the upper TBU ID bits that the SMMU tags
onto the stream ID in our RID->SID LUT table.

 RID = 0
 SID in LUT and seen by SMMU = 7
 SMMU-500 TBU appends bits, making SID something like: 0xC07
 SMR mask of 0x3ff should be applied making the SID: 0x7

> > of_pci_map_rid() seems to have a hardcoded assumption that
> > each field in the map is 4 bytes.
> 
> It does. I guess we should explicitly check that #{msi,iommu}-cells = 1
> on the target node, and maybe clarify in the binding that that cell
> should represent a plain linear ID value (although that's pretty obvious
> from context IMO).
> 
> > (Also, I guess that msi-map is not affected by this since it
> > is not related to the IOMMU...but we do have common code
> > handling both maps.)
> 
> I'd say it's definitely affected, since #msi-cells can equally be more
> than 1, and encodes an equally opaque value.
> 
> It seems pretty reasonable to me that we could extend the binding to
> accommodate #cells > 1 iff length == 1. Mark?

I'm not following why the length matters.

> That said, is there a concrete need for this, i.e. do you actually have
> one device with a single requester ID, which maps to multiple output IDs
> (which differ only in the upper bits) in a non-predictable manner?

Devices behind an fsl-mc bus instance share the same stream ID (and GIC
ITS device ID) but may be behind different TBUs.

(see drivers/staging/fsl-mc/README.txt for overview info on fsl-mc)

So we could have a bus instance with all devices having a stream ID
of 0x5.  But the TBU ID appending causes the TCU to see incoming 
stream IDs from different devices with values of 0x105 and 0x205.
We need to get those upper bits masked off.

I don't see the above being an issue for PCI and platform devices.  But
we don't want to expose TBU topography and IDs.  Those are treated as
a microarchitecture detail in our SoC and are not even documented.

The SMMU programming model does not expose the TCU/TBU split.
Just treating the stream ID namespace as a clean linear space is
much easier to understand without having to grasp the existence
of TBUs at all.  Masking SMRs keeps things clean.

As far as msi-map goes, I don't think there is an issue there.  I
don't think the TBU appended bits are propagated to the GIC ITS
(as the device ID) so I don't think any masking is needed.  Perhaps
you or Marc know.

It seems like perhaps what we need is a new argument passed to
of_pci_map_rid() that tells the function how many cells the
base iommu/msi identifier is.  The caller supplies it.

Thanks,
Stuart