On 16/12/16 14:21, Stuart Yoder wrote: > > >> -----Original Message----- >> From: Mark Rutland [mailto:[email protected]] >> Sent: Friday, December 16, 2016 5:39 AM >> To: Stuart Yoder <[email protected]> >> Cc: [email protected]; [email protected]; [email protected]; Bharat >> Bhushan >> <[email protected]>; Nipun Gupta <[email protected]>; Diana Madalina >> Craciun >> <[email protected]>; [email protected]; >> [email protected] >> Subject: Re: RFC: extend iommu-map binding to support #iommu-cells > 1 >> >> On Fri, Dec 16, 2016 at 02:36:57AM +0000, Stuart Yoder wrote: >>> For context, please see the thread: >>> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.spinics.net%2Flists%2Farm- >> kernel%2Fmsg539066.html&data=01%7C01%7Cstuart.yoder%40nxp.com%7C0464e12bddfd42e0f0a508d425a847cb%7C686ea >> 1d3bc2b4c6fa92cd99c5c301635%7C0&sdata=KnzeOlbts271GwD1cpx2FLMsKnxw%2FOdiGVTp6%2BKFrbM%3D&reserved=0 >>> >>> The existing iommu-map binding did not account for the situation where >>> #iommu-cells == 2, as permitted in the ARM SMMU binding. The 2nd cell >>> of the IOMMU specifier being the SMR mask. The existing binding defines >>> the mapping as: >>> Any RID r in the interval [rid-base, rid-base + length) is associated >>> with >>> the listed IOMMU, with the iommu-specifier (r - rid-base + iommu-base). >>> >>> ...and that does not work if iommu-base is 2 cells, the second being the >>> SMR mask. >>> >>> While this can be worked around by always having length=1, it seems we >>> can get this cleaned up by updating the binding definition for iommu-map. >> >> To reiterate, I'm very much not keen on the pci-iommu binding having >> knowledge of the decomposition of iommu-specifiers from other bindings. > > With the current definition of iommu-map we already have baked in an > assumption that an iommu-specifier is a value that can be incremented > by 1 to get to the next sequential specifier. So the binding already > has "knowledge" that applies in most, but not all cases. > > The generic iommu binding also defines a case where #iommu-cells=4 > for some IOMMUs. > > Since the ARM SMMU is a special case, perhaps the intepretation > of an iommu-specifier in the context of iommu-map should be moved > into the SMMU binding. > >> As mentioned previously, there's an intended interpretation [1] that we >> need to fix up the pci-iommu binding with. With that, I don't think it's >> even necessary to bodge iommu-cells = <1> AFAICT. > > You had said in the previous thread: > > > I had intended that the offset was added to the final cell of the > > iommu-specifier (i.e. that the iommu-specifier was treated as a large > > number). > > > You can handle this case by adding additional entries in the map table, > > with a single entry length. > > I understand that, and it works, but I don't see why the definition has > to be that the offset is added to the "final cell".
Because the cells of a specifier form a single opaque big-endian value. Were DT little-endian it would be the "first cell". To be pedantic, both of those descriptions are technically wrong because they fail to account for overflow and carry up into the next cell (in whichever direction). > Why can't it be > an iommu specific definition that makes sense for a given IOMMU? Because the implementation would then become a nightmarish rabbit-warren of special-cases, largely defeating the point of a *generic* binding. At the very least it'd have to chase every phandle and determine its compatible just to work out what to do with any given specifier - merely thinking about the complexity of the error handling for the number of additional failure modes that introduces is enough to put me off. Robin. > > Stuart > _______________________________________________ iommu mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/iommu
