-----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

[...]

quoted

quoted

quoted

quoted

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

quoted

+	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

quoted

quoted

going

quoted

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

quoted

multiple ITS group nodes.  That is perfectly fine and given a dev input RID

we

quoted

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

quoted

we have to go through all the mapped ITS groups and identify all the ITSs

associated

quoted

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.

Many thanks,
Shameer