[PATCH v4 1/2] acpi:iort: Add an IORT helper function to reserve HW ITS address regions for IOMMU drivers

[Shameerali Kolothum Thodi]

> -----Original Message-----
> From: Lorenzo Pieralisi [mailto:lorenzo.pieralisi at arm.com]
> Sent: Friday, July 28, 2017 10:58 AM
> To: Shameerali Kolothum Thodi
> Cc: Robin Murphy; Guohanjun (Hanjun Guo); Gabriele Paoloni;
> marc.zyngier at arm.com; John Garry; will.deacon at arm.com; Linuxarm; linux-
> acpi at vger.kernel.org; iommu at lists.linux-foundation.org;
> hanjun.guo at linaro.org; Wangzhou (B); sudeep.holla at arm.com; linux-arm-
> kernel at lists.infradead.org; devel at acpica.org
> 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 at arm.com]
> > > Sent: Thursday, July 27, 2017 12:13 PM
> > > To: Shameerali Kolothum Thodi; Lorenzo Pieralisi
> > > Cc: Guohanjun (Hanjun Guo); Gabriele Paoloni; marc.zyngier at arm.com;
> > > John Garry; will.deacon at arm.com; Linuxarm;
> > > linux-acpi at vger.kernel.org; iommu at lists.linux-foundation.org;
> > > hanjun.guo at linaro.org; Wangzhou (B); sudeep.holla at arm.com;
> > > linux-arm-kernel at lists.infradead.org;
> > > devel at acpica.org
> > > 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