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

quoted

quoted

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

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().

quoted

quoted

quoted

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.

quoted

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