On 16/12/16 14:21, Stuart Yoder wrote: > > >> -----Original Message----- >> From: Mark Rutland [mailto:mark.rutland@xxxxxxx] >> Sent: Friday, December 16, 2016 5:39 AM >> To: Stuart Yoder <stuart.yoder@xxxxxxx> >> Cc: robin.murphy@xxxxxxx; will.deacon@xxxxxxx; robh+dt@xxxxxxxxxx; Bharat Bhushan >> <bharat.bhushan@xxxxxxx>; Nipun Gupta <nipun.gupta@xxxxxxx>; Diana Madalina Craciun >> <diana.craciun@xxxxxxx>; devicetree@xxxxxxxxxxxxxxx; iommu@xxxxxxxxxxxxxxxxxxxxxxxxxx >> 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 > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
