> -----Original Message----- > From: Lorenzo Pieralisi [mailto:lorenzo.pieralisi@xxxxxxx] > Sent: Friday, July 28, 2017 10:58 AM > To: Shameerali Kolothum Thodi > Cc: Robin Murphy; Guohanjun (Hanjun Guo); Gabriele Paoloni; > marc.zyngier@xxxxxxx; John Garry; will.deacon@xxxxxxx; Linuxarm; linux- > acpi@xxxxxxxxxxxxxxx; iommu@xxxxxxxxxxxxxxxxxxxxxxxxxx; > hanjun.guo@xxxxxxxxxx; Wangzhou (B); sudeep.holla@xxxxxxx; linux-arm- > kernel@xxxxxxxxxxxxxxxxxxx; devel@xxxxxxxxxx > Subject: Re: [PATCH v4 1/2] acpi:iort: Add an IORT helper function to reserve > HW ITS address regions for IOMMU drivers > > On Thu, Jul 27, 2017 at 01:26:14PM +0000, Shameerali Kolothum Thodi wrote: > > > > > > > -----Original Message----- > > > From: Robin Murphy [mailto:robin.murphy@xxxxxxx] > > > Sent: Thursday, July 27, 2017 12:13 PM > > > To: Shameerali Kolothum Thodi; Lorenzo Pieralisi > > > Cc: Guohanjun (Hanjun Guo); Gabriele Paoloni; marc.zyngier@xxxxxxx; > > > John Garry; will.deacon@xxxxxxx; Linuxarm; > > > linux-acpi@xxxxxxxxxxxxxxx; iommu@xxxxxxxxxxxxxxxxxxxxxxxxxx; > > > hanjun.guo@xxxxxxxxxx; Wangzhou (B); sudeep.holla@xxxxxxx; > > > linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; > > > devel@xxxxxxxxxx > > > Subject: Re: [PATCH v4 1/2] acpi:iort: Add an IORT helper function > > > to reserve HW ITS address regions for IOMMU drivers > > > > > [...] > > > > > >>>> I can make these changes but I suspect this series will go via > > > >>>> IOMMU tree, let me know how you want to handle it. > > > >>>> > > > >>>> Lorenzo > > > >>>> > > > >>>>> + node = iort_find_dev_node(dev); > > > >>>>> + if (!node) > > > >>>>> + return -ENODEV; > > > >>>>> + > > > >>> > > > >>> I'd suggest we also want a comment here to clarify that we're > > > >>> currently assuming straightforward topologies where all mappings > > > >>> for a given root complex/named component target the same ITS > > > >>> group. Otherwise > > > we're > > > >> going > > > >>> to need somewhat more logic to iterate the its_node processing > > > >>> over every mapping (or every alias in the PCI case), but avoid > > > >>> creating duplicate entries. > > > >> > > > >> You have a point and we have time to update the code. Short of > > > >> reserving all ITS regions for every device that maps to one at > > > >> least, we could (even pre-compute instead of looking it up on the > > > >> fly) create a list of ITS identifiers a given IORT node may map > > > >> to and use that to reserve the regions. > > > > > > > > I am trying to understand the use case scenario discussed here. > > > > Apologies if it is a dumb query. > > > > > > > > My understanding is that, it is possible to have a PCI RC iort > > > > node mapped > > > to > > > > multiple ITS group nodes. That is perfectly fine and given a dev > > > > input RID > > > we > > > > can identify the ITS group the device points to using - > iort_node_map_id(). > > > > > > > > But the above discussion seems to suggest that there might be > > > > situations > > > where > > > > we have to go through all the mapped ITS groups and identify all > > > > the ITSs > > > associated > > > > with the RC. Clearly I am missing something. > > > > > > I was mostly thinking of a situation like this: > > > > > > +----Node 0-----+ +----Node 1-----+ > > > | [CPU 0..n] | | [CPU n+1..] | > > > | [ITS group 0] | | [ITS group 1] | > > > +---------------+ +---------------+ > > > ^ ^ > > > \_______ _______/ > > > \/ > > > +--Node 2--+ > > > | [SMMU] | > > > | ^ | > > > | | | > > > | [Device] | > > > +----------+ > > > > > > where the (named component) device has IDs for both ITS groups (to > > > help optimise affining, or allow physically hotplugging CPU nodes, > > > or whatever - I'm hypothesising here ;)). A generic IORT function > > > isn't in a position to decide *which* ITS region the device may be > > > targeting at any given time, so can only correctly describe both. > > > > Thanks Robin. That makes it clear. > > > > > I'm perfectly happy not to even try to support such crazy > > > configurations until they actually exist, if ever; I'd just prefer > > > to document whatever assumptions we do make, so that we don't have > > > to remember or re-derive them when looking at the code in future. > > > > So I think the conclusion here is we will document the assumption that > > we are only taking care of the straightforward topologies for now. > > > > Hi Lorenzo, > > If you are ok with the above, please let me know if it make sense to > > send out a v5 with this and your other comments or you can take care > > of them. I am fine either way. > > I added below what should be the final patch - please have a look test and > post it as part of v5 that should hopefully be final. Thanks Lorenzo. Will do that. Shameer -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
