Re: Report from 2013 ARM kernel summit

Re: Report from 2013 ARM kernel summit

[Hiroshi Doyu]

Missed some MLs. Resending....

Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote @ Fri, 8 Nov 2013 14:20:46 +0100:

> Early Init, Deferred Probe and Init Order
...
> The big problem with deferring to initcalls is that the kernel doesn?t
> have any information about dependencies between devices and so doesn?t
> know what order to call modules in. Right now driver probe order is
> roughly determined by kernel link order first, and registration order
> second for devices populated during or after initcalls. Olof commented
> that the current driver model is based on the assumption that devices
> live in a single hierarchy. There is no easy way to add in dependency
> data with the current structure, and in most cases dependencies aren?t
> specifically on other devices, but rather on the service provided by
> another device (ie, a GPIO line).

I think that an IOMMU device is also the service provider for other
devices, where an single IOMMU device resides in the bus to provide
the address translation to the bus children.

> Kumar is concerned that deferred probe won?t work in all situations. For
> instance, what about a device that has an optional dependency? Does it
> fail to probe in the hope that the dependency will show up later? It was

Other devices don't know if the system has IOMMU or not. IOMMU'able
devices' population can be deferred till IOMMU device is populated,
but they don't know if there's really IOMMU device exists or not at
their population time.

....
> The suggestion was made that it would be a really nice to have the core
> kernel sort out probe order rather than using deferred probe. Grant
> replied that deferred probe was designed as the simplest possible
> solution to the problem, and by no means is he attached to it if someone
> can come up with a better approach. He did consider other options that
> put dependency resolution into the core, but it ended up pulling all
> kinds of GPIO, IRQ, Clock and DMA details into the core which made it
> quite complex.

I have a problem to control the poplution order for IOMMU in Tegra
SoC, and currently I haven't found any framework so that I inserted an
IOMMU hook in the driver core[1] since I discussed with Stephen Warren
and we thought that the number of those subsystem which requires this
kind of control isn't so many. This patch depends on a certain DT
bindings("#stream-id-cells") if a device has an dependency on IOMMU or
not. But at least this doesn't require anything for a client driver
side at all. The exsiting driver can be used as they are. The current
design is:

1, When a device is populated, it checks if that device is IOMMU'able
   or not. This is identified by "#stream-id-cells" in DT. If
   a device is normal(non IOMMU), a device is populated. If a device
   is IOMMU'able, it continues to be checked.

2, When an IOMMU device has been already populated,
   "dev->bus->iommu_ops" is set. If so a device(client) is registered
   as IOMMU'able, and the rest of population process continues. If not
   set yet, a device population would be deferred til
   "dev->bus->iommu_ops" is set.

3, After an IOMMU device has been already populated, deferred
   population will be continued.

I'm not so sure if this dependecy on "#stream-id-cells" is acceptable
or not, but I haven't any better idea right now.

Any suggestion/hint would be really appreciated.

[1]
http://lists.linuxfoundation.org/pipermail/iommu/2013-November/006933.html

Re: Report from 2013 ARM kernel summit

[Grant Likely]

On Mon, 11 Nov 2013 10:47:23 +0100, Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
> Missed some MLs. Resending....
> 
> Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote @ Fri, 8 Nov 2013 14:20:46 +0100:
> 
> > Early Init, Deferred Probe and Init Order
> ...
> > The big problem with deferring to initcalls is that the kernel doesn?t
> > have any information about dependencies between devices and so doesn?t
> > know what order to call modules in. Right now driver probe order is
> > roughly determined by kernel link order first, and registration order
> > second for devices populated during or after initcalls. Olof commented
> > that the current driver model is based on the assumption that devices
> > live in a single hierarchy. There is no easy way to add in dependency
> > data with the current structure, and in most cases dependencies aren?t
> > specifically on other devices, but rather on the service provided by
> > another device (ie, a GPIO line).
> 
> I think that an IOMMU device is also the service provider for other
> devices, where an single IOMMU device resides in the bus to provide
> the address translation to the bus children.
> 
> > Kumar is concerned that deferred probe won?t work in all situations. For
> > instance, what about a device that has an optional dependency? Does it
> > fail to probe in the hope that the dependency will show up later? It was
> 
> Other devices don't know if the system has IOMMU or not. IOMMU'able
> devices' population can be deferred till IOMMU device is populated,
> but they don't know if there's really IOMMU device exists or not at
> their population time.

Deferring the population might complicate the common code by changing
the time that devices get created. Right now the code is oriented around
doing it all at once.

However, it would be possible to cause a probe deferal on devices that
requires an IOMMU in a way that it would be triggered by the bus instead
of by the individual device drivers.

I don't really know the details well enough to comment intelliently.
Craft up the patches and see what it looks like.

> > The suggestion was made that it would be a really nice to have the core
> > kernel sort out probe order rather than using deferred probe. Grant
> > replied that deferred probe was designed as the simplest possible
> > solution to the problem, and by no means is he attached to it if someone
> > can come up with a better approach. He did consider other options that
> > put dependency resolution into the core, but it ended up pulling all
> > kinds of GPIO, IRQ, Clock and DMA details into the core which made it
> > quite complex.
> 
> I have a problem to control the poplution order for IOMMU in Tegra
> SoC, and currently I haven't found any framework so that I inserted an
> IOMMU hook in the driver core[1] since I discussed with Stephen Warren
> and we thought that the number of those subsystem which requires this
> kind of control isn't so many. This patch depends on a certain DT
> bindings("#stream-id-cells") if a device has an dependency on IOMMU or
> not. But at least this doesn't require anything for a client driver
> side at all. The exsiting driver can be used as they are. The current
> design is:
> 
> 1, When a device is populated, it checks if that device is IOMMU'able
>    or not. This is identified by "#stream-id-cells" in DT. If
>    a device is normal(non IOMMU), a device is populated. If a device
>    is IOMMU'able, it continues to be checked.
> 
> 2, When an IOMMU device has been already populated,
>    "dev->bus->iommu_ops" is set. If so a device(client) is registered
>    as IOMMU'able, and the rest of population process continues. If not
>    set yet, a device population would be deferred til
>    "dev->bus->iommu_ops" is set.
> 
> 3, After an IOMMU device has been already populated, deferred
>    population will be continued.
> 
> I'm not so sure if this dependecy on "#stream-id-cells" is acceptable
> or not, but I haven't any better idea right now.

It seems a little fragile to me too. I'd rather the IOMMU requirement be
described more explicitly.

g.

Re: Report from 2013 ARM kernel summit

[Hiroshi Doyu]

Hi Grant,

Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote @ Fri, 15 Nov 2013 08:06:27 +0100:

> On Mon, 11 Nov 2013 10:47:23 +0100, Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
> > Missed some MLs. Resending....
> > 
> > Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote @ Fri, 8 Nov 2013 14:20:46 +0100:
> > 
> > > Early Init, Deferred Probe and Init Order
> > ...
> > > The big problem with deferring to initcalls is that the kernel doesn?t
> > > have any information about dependencies between devices and so doesn?t
> > > know what order to call modules in. Right now driver probe order is
> > > roughly determined by kernel link order first, and registration order
> > > second for devices populated during or after initcalls. Olof commented
> > > that the current driver model is based on the assumption that devices
> > > live in a single hierarchy. There is no easy way to add in dependency
> > > data with the current structure, and in most cases dependencies aren?t
> > > specifically on other devices, but rather on the service provided by
> > > another device (ie, a GPIO line).
> > 
> > I think that an IOMMU device is also the service provider for other
> > devices, where an single IOMMU device resides in the bus to provide
> > the address translation to the bus children.
> > 
> > > Kumar is concerned that deferred probe won?t work in all situations. For
> > > instance, what about a device that has an optional dependency? Does it
> > > fail to probe in the hope that the dependency will show up later? It was
> > 
> > Other devices don't know if the system has IOMMU or not. IOMMU'able
> > devices' population can be deferred till IOMMU device is populated,
> > but they don't know if there's really IOMMU device exists or not at
> > their population time.
> 
> Deferring the population might complicate the common code by changing
> the time that devices get created. Right now the code is oriented around
> doing it all at once.
> 
> However, it would be possible to cause a probe deferal on devices that
> requires an IOMMU in a way that it would be triggered by the bus instead
> of by the individual device drivers.
> 
> I don't really know the details well enough to comment intelliently.
> Craft up the patches and see what it looks like.

I have just sent v5 as below:
  [PATCHv5 0/9] Unifying Tegra IOMMU(SMMU) driver among Tegra SoCs
  http://lists.linuxfoundation.org/pipermail/iommu/2013-November/007004.html

Driver core part is:
  [PATCHv5 2/9] driver/core: populate devices in order for IOMMUs
  http://lists.linuxfoundation.org/pipermail/iommu/2013-November/007003.html

Any comment would be really appreciated.

> > > The suggestion was made that it would be a really nice to have the core
> > > kernel sort out probe order rather than using deferred probe. Grant
> > > replied that deferred probe was designed as the simplest possible
> > > solution to the problem, and by no means is he attached to it if someone
> > > can come up with a better approach. He did consider other options that
> > > put dependency resolution into the core, but it ended up pulling all
> > > kinds of GPIO, IRQ, Clock and DMA details into the core which made it
> > > quite complex.
> > 
> > I have a problem to control the poplution order for IOMMU in Tegra
> > SoC, and currently I haven't found any framework so that I inserted an
> > IOMMU hook in the driver core[1] since I discussed with Stephen Warren
> > and we thought that the number of those subsystem which requires this
> > kind of control isn't so many. This patch depends on a certain DT
> > bindings("#stream-id-cells") if a device has an dependency on IOMMU or
> > not. But at least this doesn't require anything for a client driver
> > side at all. The exsiting driver can be used as they are. The current
> > design is:
> > 
> > 1, When a device is populated, it checks if that device is IOMMU'able
> >    or not. This is identified by "#stream-id-cells" in DT. If
> >    a device is normal(non IOMMU), a device is populated. If a device
> >    is IOMMU'able, it continues to be checked.
> > 
> > 2, When an IOMMU device has been already populated,
> >    "dev->bus->iommu_ops" is set. If so a device(client) is registered
> >    as IOMMU'able, and the rest of population process continues. If not
> >    set yet, a device population would be deferred til
> >    "dev->bus->iommu_ops" is set.
> > 
> > 3, After an IOMMU device has been already populated, deferred
> >    population will be continued.
> > 
> > I'm not so sure if this dependecy on "#stream-id-cells" is acceptable
> > or not, but I haven't any better idea right now.
> 
> It seems a little fragile to me too. I'd rather the IOMMU requirement be
> described more explicitly.

I think that Will Deacon can do better than I.

Re: Report from 2013 ARM kernel summit

[Will Deacon]

On Tue, Nov 19, 2013 at 09:40:54AM +0000, Hiroshi Doyu wrote:
> Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote @ Fri, 15 Nov 2013 08:06:27 +0100:
> > On Mon, 11 Nov 2013 10:47:23 +0100, Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
> > > 1, When a device is populated, it checks if that device is IOMMU'able
> > >    or not. This is identified by "#stream-id-cells" in DT. If
> > >    a device is normal(non IOMMU), a device is populated. If a device
> > >    is IOMMU'able, it continues to be checked.

[...]

> > > I'm not so sure if this dependecy on "#stream-id-cells" is acceptable
> > > or not, but I haven't any better idea right now.
> > 
> > It seems a little fragile to me too. I'd rather the IOMMU requirement be
> > described more explicitly.
> 
> I think that Will Deacon can do better than I.

I already commented briefly here:

  http://www.spinics.net/lists/devicetree/msg11513.html

basically deferring to DT people :)

Anyway, I'm happy to tighten up the IOMMU requirement description but
*not* at the expense of breaking what we currently have for the ARM SMMU,
which is being used by Calxeda.

Adding Andreas and Rob for input on potential binding additions to the SMMU.

Will

Re: Report from 2013 ARM kernel summit

[Rob Herring]

On 11/19/2013 11:35 AM, Will Deacon wrote:
> On Tue, Nov 19, 2013 at 09:40:54AM +0000, Hiroshi Doyu wrote:
>> Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote @ Fri, 15 Nov 2013 08:06:27 +0100:
>>> On Mon, 11 Nov 2013 10:47:23 +0100, Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
>>>> 1, When a device is populated, it checks if that device is IOMMU'able
>>>>    or not. This is identified by "#stream-id-cells" in DT. If
>>>>    a device is normal(non IOMMU), a device is populated. If a device
>>>>    is IOMMU'able, it continues to be checked.
> 
> [...]
> 
>>>> I'm not so sure if this dependecy on "#stream-id-cells" is acceptable
>>>> or not, but I haven't any better idea right now.
>>>
>>> It seems a little fragile to me too. I'd rather the IOMMU requirement be
>>> described more explicitly.

I don't see how this can work. Typically you find a property and then
read the relevant #*-cells to determine the size. Having multiple cell
properties is asking for errors.

>>
>> I think that Will Deacon can do better than I.
> 
> I already commented briefly here:
> 
>   http://www.spinics.net/lists/devicetree/msg11513.html
> 
> basically deferring to DT people :)
> 
> Anyway, I'm happy to tighten up the IOMMU requirement description but
> *not* at the expense of breaking what we currently have for the ARM SMMU,
> which is being used by Calxeda.
> 
> Adding Andreas and Rob for input on potential binding additions to the SMMU.

The above proposal would be an incompatible change. However, I think we
could still deal with a change in this binding at this stage.

One way approach to handle this without changing the binding would be to
scan the DT for all iommu's up front and create a list of all nodes and
their iommu parent. The fact that the hierarchy is described in a way
that doesn't fit Linux well is really a Linux implementation detail.

If changing the binding, a simple approach would be to allow
'smmu-parent' to be a bus and/or device property and not just for
chained iommu's. This could be a global or bus property that is
inherited. Like interrupt-parent, you would have to deal with the parent
being itself. Also, perhaps iommu-parent would be a better name. In any
case, I'd like to see this all be a generic iommu binding.

Rob

Re: Report from 2013 ARM kernel summit

[Hiroshi Doyu]

Hi Rob,

Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote @ Tue, 19 Nov 2013 21:45:02 +0100:

> On 11/19/2013 11:35 AM, Will Deacon wrote:
> > On Tue, Nov 19, 2013 at 09:40:54AM +0000, Hiroshi Doyu wrote:
> >> Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote @ Fri, 15 Nov 2013 08:06:27 +0100:
> >>> On Mon, 11 Nov 2013 10:47:23 +0100, Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
> >>>> 1, When a device is populated, it checks if that device is IOMMU'able
> >>>>    or not. This is identified by "#stream-id-cells" in DT. If
> >>>>    a device is normal(non IOMMU), a device is populated. If a device
> >>>>    is IOMMU'able, it continues to be checked.
> > 
> > [...]
> > 
> >>>> I'm not so sure if this dependecy on "#stream-id-cells" is acceptable
> >>>> or not, but I haven't any better idea right now.
> >>>
> >>> It seems a little fragile to me too. I'd rather the IOMMU requirement be
> >>> described more explicitly.
> 
> I don't see how this can work. Typically you find a property and then
> read the relevant #*-cells to determine the size. Having multiple cell
> properties is asking for errors.

The above was mentioned for PATCHv4 series[1], which used the arm,smmu
DT bindings,  "#stream-id-cells" in clients and "mmu-masters" in
iommu.

In PATCHv5[2], we took the following DT binding where multiple cell
properties seem to work ok.

  smmu_a: iommu@xxxxxxxx {
  	#iommu-cells = <2>;
  	....
  };
  
  smmu_b: iommu@xxxxxxxx {
  	#iommu-cells = <3>;
  	....
  };
  
  device_a {
  	 iommus = <&smmu_a param1 param2>,
  	 	  <&smmu_b param1 param2 param3>;
  };

This can describe the relation between a device and an iommu
independently. The number of params needed for each IOMMU can be
sepcified by #iommu-cells in its iommu entry.

  device_a <-> smmu_a, needs 2 params for a device
  device_a <-> smmu_b, needs 3 params for a device

For example, "smmu_a" can be an bus level global IOMMU where all child
devices can be an master of "smmu_a", and "smmu_b" is a local IOMMU
only for "device_a".

"memory controller"---"smmu_a"---bus--+--"smmu_b"--"device_a"
                                      |
                                      |
                                      +--"device_b"

> >> I think that Will Deacon can do better than I.
> > 
> > I already commented briefly here:
> > 
> >   http://www.spinics.net/lists/devicetree/msg11513.html
> > 
> > basically deferring to DT people :)
> > 
> > Anyway, I'm happy to tighten up the IOMMU requirement description but
> > *not* at the expense of breaking what we currently have for the ARM SMMU,
> > which is being used by Calxeda.
> > 
> > Adding Andreas and Rob for input on potential binding additions to the SMMU.
> 
> The above proposal would be an incompatible change. However, I think we
> could still deal with a change in this binding at this stage.
> 
> One way approach to handle this without changing the binding would be to
> scan the DT for all iommu's up front and create a list of all nodes and
> their iommu parent. The fact that the hierarchy is described in a way
> that doesn't fit Linux well is really a Linux implementation detail.

I may need some implementation to understand this further.

> If changing the binding, a simple approach would be to allow
> 'smmu-parent' to be a bus and/or device property and not just for
> chained iommu's. This could be a global or bus property that is
> inherited. Like interrupt-parent, you would have to deal with the parent
> being itself. Also, perhaps iommu-parent would be a better name. In any
> case, I'd like to see this all be a generic iommu binding.

I guess that this would work.

One concern to this may be that there might be the case where IOMMU
hierarchy doesn't always follow the bus's one, I guess. I'm not so
sure if it's an good example, but, platform_bus itself doesn't follow
the actual bus hierarchy at all. But if we start to describe the bus
hierarchy(adding a specific bus) we'll loose the benefit of
platform_bus, where we can share almost all devices in a single
driver.

OTOH, do you see any limitations of point-to-point connections between
a device and IOMMUs, described in the above "device_a" and "smmu_[a|b]"?

[1] http://lists.linuxfoundation.org/pipermail/iommu/2013-November/006931.html
[2] http://lists.linuxfoundation.org/pipermail/iommu/2013-November/007004.html

Re: Report from 2013 ARM kernel summit

[Rob Herring]

On Wed, Nov 20, 2013 at 12:39 AM, Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
> Hi Rob,
>
> Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote @ Tue, 19 Nov 2013 21:45:02 +0100:
>
>> On 11/19/2013 11:35 AM, Will Deacon wrote:
>> > On Tue, Nov 19, 2013 at 09:40:54AM +0000, Hiroshi Doyu wrote:
>> >> Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote @ Fri, 15 Nov 2013 08:06:27 +0100:
>> >>> On Mon, 11 Nov 2013 10:47:23 +0100, Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
>> >>>> 1, When a device is populated, it checks if that device is IOMMU'able
>> >>>>    or not. This is identified by "#stream-id-cells" in DT. If
>> >>>>    a device is normal(non IOMMU), a device is populated. If a device
>> >>>>    is IOMMU'able, it continues to be checked.
>> >
>> > [...]
>> >
>> >>>> I'm not so sure if this dependecy on "#stream-id-cells" is acceptable
>> >>>> or not, but I haven't any better idea right now.
>> >>>
>> >>> It seems a little fragile to me too. I'd rather the IOMMU requirement be
>> >>> described more explicitly.
>>
>> I don't see how this can work. Typically you find a property and then
>> read the relevant #*-cells to determine the size. Having multiple cell
>> properties is asking for errors.
>
> The above was mentioned for PATCHv4 series[1], which used the arm,smmu
> DT bindings,  "#stream-id-cells" in clients and "mmu-masters" in
> iommu.
>
> In PATCHv5[2], we took the following DT binding where multiple cell
> properties seem to work ok.
>
>   smmu_a: iommu@xxxxxxxx {
>         #iommu-cells = <2>;
>         ....
>   };
>
>   smmu_b: iommu@xxxxxxxx {
>         #iommu-cells = <3>;
>         ....
>   };
>
>   device_a {
>          iommus = <&smmu_a param1 param2>,
>                   <&smmu_b param1 param2 param3>;
>   };
>
> This can describe the relation between a device and an iommu
> independently. The number of params needed for each IOMMU can be
> sepcified by #iommu-cells in its iommu entry.
>
>   device_a <-> smmu_a, needs 2 params for a device
>   device_a <-> smmu_b, needs 3 params for a device
>
> For example, "smmu_a" can be an bus level global IOMMU where all child
> devices can be an master of "smmu_a", and "smmu_b" is a local IOMMU
> only for "device_a".
>
> "memory controller"---"smmu_a"---bus--+--"smmu_b"--"device_a"
>                                       |
>                                       |
>                                       +--"device_b"

I think the above binding would be the correct way to describe things
if you have 1 device connected to 2 IOMMUs (directly rather than
chained). IIUC, that is something you have on tegra?

For the topology above where you are chaining iommu's, I think
something like this is more accurately describing the hierarchy:

  smmu_b: iommu@xxxxxxxx {
        #iommu-cells = <3>;
         iommus = <&smmu_a param1 param2>;
       ....
  };
  device_a {
         iommus = <&smmu_b param1 param2 param3>;
  };

I remember discussing this with Will and seem to recall some issue
with describing things this way. But looking at it now, I don't see
what that was.

>> >> I think that Will Deacon can do better than I.
>> >
>> > I already commented briefly here:
>> >
>> >   http://www.spinics.net/lists/devicetree/msg11513.html
>> >
>> > basically deferring to DT people :)
>> >
>> > Anyway, I'm happy to tighten up the IOMMU requirement description but
>> > *not* at the expense of breaking what we currently have for the ARM SMMU,
>> > which is being used by Calxeda.
>> >
>> > Adding Andreas and Rob for input on potential binding additions to the SMMU.
>>
>> The above proposal would be an incompatible change. However, I think we
>> could still deal with a change in this binding at this stage.
>>
>> One way approach to handle this without changing the binding would be to
>> scan the DT for all iommu's up front and create a list of all nodes and
>> their iommu parent. The fact that the hierarchy is described in a way
>> that doesn't fit Linux well is really a Linux implementation detail.
>
> I may need some implementation to understand this further.
>
>> If changing the binding, a simple approach would be to allow
>> 'smmu-parent' to be a bus and/or device property and not just for
>> chained iommu's. This could be a global or bus property that is
>> inherited. Like interrupt-parent, you would have to deal with the parent
>> being itself. Also, perhaps iommu-parent would be a better name. In any
>> case, I'd like to see this all be a generic iommu binding.
>
> I guess that this would work.
>
> One concern to this may be that there might be the case where IOMMU
> hierarchy doesn't always follow the bus's one, I guess. I'm not so
> sure if it's an good example, but, platform_bus itself doesn't follow
> the actual bus hierarchy at all. But if we start to describe the bus
> hierarchy(adding a specific bus) we'll loose the benefit of
> platform_bus, where we can share almost all devices in a single
> driver.

It is not trying to follow the bus hierarchy. Inheritance is just a
convenience. Interrupts don't follow the bus hierarchy either.

Rob

>
> OTOH, do you see any limitations of point-to-point connections between
> a device and IOMMUs, described in the above "device_a" and "smmu_[a|b]"?
>
> [1] http://lists.linuxfoundation.org/pipermail/iommu/2013-November/006931.html
> [2] http://lists.linuxfoundation.org/pipermail/iommu/2013-November/007004.html

Re: Report from 2013 ARM kernel summit

[Stephen Warren]

On 11/20/2013 01:02 PM, Rob Herring wrote:
> On Wed, Nov 20, 2013 at 12:39 AM, Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
>> Hi Rob,
>>
>> Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote @ Tue, 19 Nov 2013 21:45:02 +0100:
>>
>>> On 11/19/2013 11:35 AM, Will Deacon wrote:
>>>> On Tue, Nov 19, 2013 at 09:40:54AM +0000, Hiroshi Doyu wrote:
>>>>> Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote @ Fri, 15 Nov 2013 08:06:27 +0100:
>>>>>> On Mon, 11 Nov 2013 10:47:23 +0100, Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
>>>>>>> 1, When a device is populated, it checks if that device is IOMMU'able
>>>>>>>    or not. This is identified by "#stream-id-cells" in DT. If
>>>>>>>    a device is normal(non IOMMU), a device is populated. If a device
>>>>>>>    is IOMMU'able, it continues to be checked.
>>>>
>>>> [...]
>>>>
>>>>>>> I'm not so sure if this dependecy on "#stream-id-cells" is acceptable
>>>>>>> or not, but I haven't any better idea right now.
>>>>>>
>>>>>> It seems a little fragile to me too. I'd rather the IOMMU requirement be
>>>>>> described more explicitly.
>>>
>>> I don't see how this can work. Typically you find a property and then
>>> read the relevant #*-cells to determine the size. Having multiple cell
>>> properties is asking for errors.
>>
>> The above was mentioned for PATCHv4 series[1], which used the arm,smmu
>> DT bindings,  "#stream-id-cells" in clients and "mmu-masters" in
>> iommu.
>>
>> In PATCHv5[2], we took the following DT binding where multiple cell
>> properties seem to work ok.
>>
>>   smmu_a: iommu@xxxxxxxx {
>>         #iommu-cells = <2>;
>>         ....
>>   };
>>
>>   smmu_b: iommu@xxxxxxxx {
>>         #iommu-cells = <3>;
>>         ....
>>   };
>>
>>   device_a {
>>          iommus = <&smmu_a param1 param2>,
>>                   <&smmu_b param1 param2 param3>;
>>   };
>>
>> This can describe the relation between a device and an iommu
>> independently. The number of params needed for each IOMMU can be
>> sepcified by #iommu-cells in its iommu entry.
>>
>>   device_a <-> smmu_a, needs 2 params for a device
>>   device_a <-> smmu_b, needs 3 params for a device
>>
>> For example, "smmu_a" can be an bus level global IOMMU where all child
>> devices can be an master of "smmu_a", and "smmu_b" is a local IOMMU
>> only for "device_a".
>>
>> "memory controller"---"smmu_a"---bus--+--"smmu_b"--"device_a"
>>                                       |
>>                                       |
>>                                       +--"device_b"
> 
> I think the above binding would be the correct way to describe things
> if you have 1 device connected to 2 IOMMUs (directly rather than
> chained). IIUC, that is something you have on tegra?

I'm not sure that we do, but perhaps; Hiroshi will have to answer.
Certainly whenever I personally mentioned chained IOMMUs it was simply
in the context of making sure the bindings allowed for any potential
arbitrary HW configuration, not because I had specific knowledge of one
that actually exists.

> For the topology above where you are chaining iommu's, I think
> something like this is more accurately describing the hierarchy:
> 
>   smmu_b: iommu@xxxxxxxx {
>         #iommu-cells = <3>;
>          iommus = <&smmu_a param1 param2>;
>        ....
>   };
>   device_a {
>          iommus = <&smmu_b param1 param2 param3>;
>   };
> 
> I remember discussing this with Will and seem to recall some issue
> with describing things this way. But looking at it now, I don't see
> what that was.

That's the DT content I would expect for that scenario.

The syntax:

>>          iommus = <&smmu_a param1 param2>,
>>                   <&smmu_b param1 param2 param3>;

... I would expect to be useful if a single device was a bus-master on
multiple buses, and each bus had a path to memory via a different IOMMU.
One example might be a DMA controller that bridges two buses. We
certainly have such a DMA controller on Tegra, although IIRC, the
upstream path to memory would pass through the same IOMMU from both
buses on current HW that I'm familiar with.

Re: Report from 2013 ARM kernel summit

[Hiroshi Doyu]

On Wed, 20 Nov 2013 21:22:39 +0100
Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> wrote:
...
> >> In PATCHv5[2], we took the following DT binding where multiple cell
> >> properties seem to work ok.
> >>
> >>   smmu_a: iommu@xxxxxxxx {
> >>         #iommu-cells = <2>;
> >>         ....
> >>   };
> >>
> >>   smmu_b: iommu@xxxxxxxx {
> >>         #iommu-cells = <3>;
> >>         ....
> >>   };
> >>
> >>   device_a {
> >>          iommus = <&smmu_a param1 param2>,
> >>                   <&smmu_b param1 param2 param3>;
> >>   };
> >>
> >> This can describe the relation between a device and an iommu
> >> independently. The number of params needed for each IOMMU can be
> >> sepcified by #iommu-cells in its iommu entry.
> >>
> >>   device_a <-> smmu_a, needs 2 params for a device
> >>   device_a <-> smmu_b, needs 3 params for a device
> >>
> >> For example, "smmu_a" can be an bus level global IOMMU where all child
> >> devices can be an master of "smmu_a", and "smmu_b" is a local IOMMU
> >> only for "device_a".
> >>
> >> "memory controller"---"smmu_a"---bus--+--"smmu_b"--"device_a"
> >>                                       |
> >>                                       |
> >>                                       +--"device_b"
> > 
> > I think the above binding would be the correct way to describe things
> > if you have 1 device connected to 2 IOMMUs (directly rather than
> > chained). IIUC, that is something you have on tegra?
> 
> I'm not sure that we do, but perhaps; Hiroshi will have to answer.
> Certainly whenever I personally mentioned chained IOMMUs it was simply
> in the context of making sure the bindings allowed for any potential
> arbitrary HW configuration, not because I had specific knowledge of one
> that actually exists.

In tegra, IOMMUs are chained in a way that:

- "device_a" run in "smmu_b"'s virtual address space.
- "device_a"'s requesting address is transted into "smmu_a"'s virtual
  address via "smmu_b".
- This "smmu_a"'s virtual address is translated into physical address
  via "smmu_a".

"smmu_a" and "smmu_b" are different driver although we haven't
converted "smmu_b" as the standard IOMMU API yet.

> > For the topology above where you are chaining iommu's, I think
> > something like this is more accurately describing the hierarchy:
> > 
> >   smmu_b: iommu@xxxxxxxx {
> >         #iommu-cells = <3>;
> >          iommus = <&smmu_a param1 param2>;
> >        ....
> >   };
> >   device_a {
> >          iommus = <&smmu_b param1 param2 param3>;
> >   };

This describes the dependency between IOMMUs in Tegra. We can populate
"smmu_a" always earlier than "smmu_b". Those dependency/hierarchy can
be stored in IOMMU class(global IOMMU list), which Thierry suggested.

Re: Report from 2013 ARM kernel summit

[Will Deacon]

On Wed, Nov 20, 2013 at 08:02:10PM +0000, Rob Herring wrote:
> On Wed, Nov 20, 2013 at 12:39 AM, Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
> >   smmu_a: iommu@xxxxxxxx {
> >         #iommu-cells = <2>;
> >         ....
> >   };
> >
> >   smmu_b: iommu@xxxxxxxx {
> >         #iommu-cells = <3>;
> >         ....
> >   };
> >
> >   device_a {
> >          iommus = <&smmu_a param1 param2>,
> >                   <&smmu_b param1 param2 param3>;
> >   };
> >
> > This can describe the relation between a device and an iommu
> > independently. The number of params needed for each IOMMU can be
> > sepcified by #iommu-cells in its iommu entry.
> >
> >   device_a <-> smmu_a, needs 2 params for a device
> >   device_a <-> smmu_b, needs 3 params for a device
> >
> > For example, "smmu_a" can be an bus level global IOMMU where all child
> > devices can be an master of "smmu_a", and "smmu_b" is a local IOMMU
> > only for "device_a".
> >
> > "memory controller"---"smmu_a"---bus--+--"smmu_b"--"device_a"
> >                                       |
> >                                       |
> >                                       +--"device_b"
> 
> I think the above binding would be the correct way to describe things
> if you have 1 device connected to 2 IOMMUs (directly rather than
> chained). IIUC, that is something you have on tegra?
> 
> For the topology above where you are chaining iommu's, I think
> something like this is more accurately describing the hierarchy:
> 
>   smmu_b: iommu@xxxxxxxx {
>         #iommu-cells = <3>;
>          iommus = <&smmu_a param1 param2>;
>        ....
>   };
>   device_a {
>          iommus = <&smmu_b param1 param2 param3>;
>   };
> 
> I remember discussing this with Will and seem to recall some issue
> with describing things this way. But looking at it now, I don't see
> what that was.

I think it was the usual issue with StreamID remastering. For example, if
you had a device going into two chained IOMMUs, you need a way to relate the
StreamIDs of the device to the remastered IDs coming out of the first SMMU.

Will

Re: Report from 2013 ARM kernel summit

[Stephen Warren]

On 11/21/2013 04:00 AM, Will Deacon wrote:
> On Wed, Nov 20, 2013 at 08:02:10PM +0000, Rob Herring wrote:
>> On Wed, Nov 20, 2013 at 12:39 AM, Hiroshi Doyu <hdoyu-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
>>>   smmu_a: iommu@xxxxxxxx {
>>>         #iommu-cells = <2>;
>>>         ....
>>>   };
>>>
>>>   smmu_b: iommu@xxxxxxxx {
>>>         #iommu-cells = <3>;
>>>         ....
>>>   };
>>>
>>>   device_a {
>>>          iommus = <&smmu_a param1 param2>,
>>>                   <&smmu_b param1 param2 param3>;
>>>   };
>>>
>>> This can describe the relation between a device and an iommu
>>> independently. The number of params needed for each IOMMU can be
>>> sepcified by #iommu-cells in its iommu entry.
>>>
>>>   device_a <-> smmu_a, needs 2 params for a device
>>>   device_a <-> smmu_b, needs 3 params for a device
>>>
>>> For example, "smmu_a" can be an bus level global IOMMU where all child
>>> devices can be an master of "smmu_a", and "smmu_b" is a local IOMMU
>>> only for "device_a".
>>>
>>> "memory controller"---"smmu_a"---bus--+--"smmu_b"--"device_a"
>>>                                       |
>>>                                       |
>>>                                       +--"device_b"
>>
>> I think the above binding would be the correct way to describe things
>> if you have 1 device connected to 2 IOMMUs (directly rather than
>> chained). IIUC, that is something you have on tegra?
>>
>> For the topology above where you are chaining iommu's, I think
>> something like this is more accurately describing the hierarchy:
>>
>>   smmu_b: iommu@xxxxxxxx {
>>         #iommu-cells = <3>;
>>          iommus = <&smmu_a param1 param2>;
>>        ....
>>   };
>>   device_a {
>>          iommus = <&smmu_b param1 param2 param3>;
>>   };
>>
>> I remember discussing this with Will and seem to recall some issue
>> with describing things this way. But looking at it now, I don't see
>> what that was.
> 
> I think it was the usual issue with StreamID remastering. For example, if
             ^^^ s/was/had/ I assume
> you had a device going into two chained IOMMUs, you need a way to relate the
> StreamIDs of the device to the remastered IDs coming out of the first SMMU.

That seems pretty easy to deal with. Am I missing something. Here's how
I assume that would work:

1)

If the mapping is static in HW, then you simply have a table that
describes the remapping. A table representation could be in the DT or
the driver itself. Either way, any static mapping is a property of the
HW itself, so shouldn't need to leak into the client DT nodes at all, right?

That might look like:

smmu_a: iommu@xxxxxxxx {
	#iommu-cells = <1>;
};
smmu_b: iommu@xxxxxxxx {
	#iommu-cells = <1>;

	// a) All stream IDs squashed down to 1 ID
	iommus = <&smmu_a SMMU_B's_STREAM_ID_IN_SMMU_A>;

	// OR b) stream IDs translated:
	iommus = <&smmu_a 0>,
		 <&smmu_a 1>,
		 <&smmu_a 2> ...;
	// this table could be in the driver instead
	iommu-stream-id-map =
		<DEV_A's_STREAM_ID_IN_SMMU_B 0>,
		<DEV_B's_STREAM_ID_IN_SMMU_B 1>,
		<DEV_C's_STREAM_ID_IN_SMMU_B 2>,
};
device_a {
	iommus = <&smmu_b DEV_A's_STREAM_ID_IN_SMMU_B>;
};

or 2)

If the remapping is entirely dynamic, then you'd either need a table in
the DT to describe how to set it up for the particular use-case (similar
to the above), or a function in the driver to manage the mapping at
run-time, probably hooked into the of_xlate() of the parent IOMMU, which
would call into some function of the source IOMMU.

In other words, of_xlate for smmu_a would do both:

a) Parse the SMMU specifier in the client's iommus property

b) If the parse specifier represents a "dynamic stream ID device/IOMMU",
call some function in the client driver to find out which stream ID(s)
the parse iommus entry (specifier) represents.

Re: Report from 2013 ARM kernel summit

[Will Deacon]

Hi Stephen,

On Thu, Nov 21, 2013 at 06:40:28PM +0000, Stephen Warren wrote:
> On 11/21/2013 04:00 AM, Will Deacon wrote:
> > On Wed, Nov 20, 2013 at 08:02:10PM +0000, Rob Herring wrote:
> >> For the topology above where you are chaining iommu's, I think
> >> something like this is more accurately describing the hierarchy:
> >>
> >>   smmu_b: iommu@xxxxxxxx {
> >>         #iommu-cells = <3>;
> >>          iommus = <&smmu_a param1 param2>;
> >>        ....
> >>   };
> >>   device_a {
> >>          iommus = <&smmu_b param1 param2 param3>;
> >>   };
> >>
> >> I remember discussing this with Will and seem to recall some issue
> >> with describing things this way. But looking at it now, I don't see
> >> what that was.
> > 
> > I think it was the usual issue with StreamID remastering. For example, if
>              ^^^ s/was/had/ I assume

Take your pick!

> > you had a device going into two chained IOMMUs, you need a way to relate the
> > StreamIDs of the device to the remastered IDs coming out of the first SMMU.
> 
> That seems pretty easy to deal with. Am I missing something. Here's how
> I assume that would work:
> 
> 1)
> 
> If the mapping is static in HW, then you simply have a table that
> describes the remapping. A table representation could be in the DT or
> the driver itself. Either way, any static mapping is a property of the
> HW itself, so shouldn't need to leak into the client DT nodes at all, right?
> 
> That might look like:
> 
> smmu_a: iommu@xxxxxxxx {
> 	#iommu-cells = <1>;
> };
> smmu_b: iommu@xxxxxxxx {
> 	#iommu-cells = <1>;
> 
> 	// a) All stream IDs squashed down to 1 ID
> 	iommus = <&smmu_a SMMU_B's_STREAM_ID_IN_SMMU_A>;
> 
> 	// OR b) stream IDs translated:
> 	iommus = <&smmu_a 0>,
> 		 <&smmu_a 1>,
> 		 <&smmu_a 2> ...;
> 	// this table could be in the driver instead
> 	iommu-stream-id-map =
> 		<DEV_A's_STREAM_ID_IN_SMMU_B 0>,
> 		<DEV_B's_STREAM_ID_IN_SMMU_B 1>,
> 		<DEV_C's_STREAM_ID_IN_SMMU_B 2>,

Actually, something like this could work. I think we should also support the
case where a range of IDs are `windowed' (i.e. there is a constant offset
applied to incoming IDs to get the outgoing IDs) to avoid huge tables in
some cases.

> };
> device_a {
> 	iommus = <&smmu_b DEV_A's_STREAM_ID_IN_SMMU_B>;
> };
> 
> or 2)
> 
> If the remapping is entirely dynamic, then you'd either need a table in
> the DT to describe how to set it up for the particular use-case (similar
> to the above), or a function in the driver to manage the mapping at
> run-time, probably hooked into the of_xlate() of the parent IOMMU, which
> would call into some function of the source IOMMU.
> 
> In other words, of_xlate for smmu_a would do both:
> 
> a) Parse the SMMU specifier in the client's iommus property
> 
> b) If the parse specifier represents a "dynamic stream ID device/IOMMU",
> call some function in the client driver to find out which stream ID(s)
> the parse iommus entry (specifier) represents.

Sure. We just need to get the binding right so we're prepared when these
platforms turn up. There's also the usual problem of continuing to support
what we already do for existing SMMU bindings.

Will

Re: Report from 2013 ARM kernel summit

[Will Deacon]

On Tue, Nov 19, 2013 at 08:45:02PM +0000, Rob Herring wrote:
> On 11/19/2013 11:35 AM, Will Deacon wrote:
> > Adding Andreas and Rob for input on potential binding additions to the SMMU.
> 
> The above proposal would be an incompatible change. However, I think we
> could still deal with a change in this binding at this stage.
> 
> One way approach to handle this without changing the binding would be to
> scan the DT for all iommu's up front and create a list of all nodes and
> their iommu parent. The fact that the hierarchy is described in a way
> that doesn't fit Linux well is really a Linux implementation detail.
> 
> If changing the binding, a simple approach would be to allow
> 'smmu-parent' to be a bus and/or device property and not just for
> chained iommu's. This could be a global or bus property that is
> inherited. Like interrupt-parent, you would have to deal with the parent
> being itself. Also, perhaps iommu-parent would be a better name. In any
> case, I'd like to see this all be a generic iommu binding.

I like that idea. I've recently been toying with removing the chained IOMMU
support, since I don't think anybody is using it who is interested in
mainline. However, making it more general sounds like a better idea.

One potential issue is that I think the nvidia guys want to describe masters
that master via multiple SMMUs (which I believe was the motivation for
moving the stream-ids out into the master nodes, rather than keeping them in
the SMMU). Again, that's not something we can easily add to the arm-smmu,
because the incoming stream-ids are a property of the SMMU node.

So the question is: do we actually need to describe masters that master
through multiple SMMUs as a single node in the devicetree?

Will

Re: Report from 2013 ARM kernel summit

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 2769 bytes --]

On Wed, Nov 20, 2013 at 10:31:11AM +0000, Will Deacon wrote:
> On Tue, Nov 19, 2013 at 08:45:02PM +0000, Rob Herring wrote:
> > On 11/19/2013 11:35 AM, Will Deacon wrote:
> > > Adding Andreas and Rob for input on potential binding additions to the SMMU.
> > 
> > The above proposal would be an incompatible change. However, I think we
> > could still deal with a change in this binding at this stage.
> > 
> > One way approach to handle this without changing the binding would be to
> > scan the DT for all iommu's up front and create a list of all nodes and
> > their iommu parent. The fact that the hierarchy is described in a way
> > that doesn't fit Linux well is really a Linux implementation detail.
> > 
> > If changing the binding, a simple approach would be to allow
> > 'smmu-parent' to be a bus and/or device property and not just for
> > chained iommu's. This could be a global or bus property that is
> > inherited. Like interrupt-parent, you would have to deal with the parent
> > being itself. Also, perhaps iommu-parent would be a better name. In any
> > case, I'd like to see this all be a generic iommu binding.
> 
> I like that idea. I've recently been toying with removing the chained IOMMU
> support, since I don't think anybody is using it who is interested in
> mainline. However, making it more general sounds like a better idea.
> 
> One potential issue is that I think the nvidia guys want to describe masters
> that master via multiple SMMUs (which I believe was the motivation for
> moving the stream-ids out into the master nodes, rather than keeping them in
> the SMMU). Again, that's not something we can easily add to the arm-smmu,
> because the incoming stream-ids are a property of the SMMU node.

If I remember correctly, one of the reasons for the proposal was also
that the interrupt-parent property turned out to be insufficient for
some use-cases, which lead to Grant's proposal of the new interrupts-
extended property. Since that comparison has already been drawn, I think
we can agree that both are used in similar ways. Therefore we should
consider what we've learned from interrupt-parent when designing this
generic IOMMU binding to avoid having to introduce iommu-extended at
some point.

> So the question is: do we actually need to describe masters that master
> through multiple SMMUs as a single node in the devicetree?

I would think so, yes. The alternative would be to have several nodes
that describe the same device, and that conflicts on a different level.

Perhaps it could be done by having separate sub-nodes that each use a
different IOMMU, but that sounds like a much grosser solution. That
pretty much boils down to interrupt-parent/interrupt-map.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: Report from 2013 ARM kernel summit

[Rob Herring]

On Wed, Nov 20, 2013 at 7:53 AM, Thierry Reding
<thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Wed, Nov 20, 2013 at 10:31:11AM +0000, Will Deacon wrote:
>> On Tue, Nov 19, 2013 at 08:45:02PM +0000, Rob Herring wrote:
>> > On 11/19/2013 11:35 AM, Will Deacon wrote:
>> > > Adding Andreas and Rob for input on potential binding additions to the SMMU.
>> >
>> > The above proposal would be an incompatible change. However, I think we
>> > could still deal with a change in this binding at this stage.
>> >
>> > One way approach to handle this without changing the binding would be to
>> > scan the DT for all iommu's up front and create a list of all nodes and
>> > their iommu parent. The fact that the hierarchy is described in a way
>> > that doesn't fit Linux well is really a Linux implementation detail.
>> >
>> > If changing the binding, a simple approach would be to allow
>> > 'smmu-parent' to be a bus and/or device property and not just for
>> > chained iommu's. This could be a global or bus property that is
>> > inherited. Like interrupt-parent, you would have to deal with the parent
>> > being itself. Also, perhaps iommu-parent would be a better name. In any
>> > case, I'd like to see this all be a generic iommu binding.
>>
>> I like that idea. I've recently been toying with removing the chained IOMMU
>> support, since I don't think anybody is using it who is interested in
>> mainline. However, making it more general sounds like a better idea.
>>
>> One potential issue is that I think the nvidia guys want to describe masters
>> that master via multiple SMMUs (which I believe was the motivation for
>> moving the stream-ids out into the master nodes, rather than keeping them in
>> the SMMU). Again, that's not something we can easily add to the arm-smmu,
>> because the incoming stream-ids are a property of the SMMU node.
>
> If I remember correctly, one of the reasons for the proposal was also
> that the interrupt-parent property turned out to be insufficient for
> some use-cases, which lead to Grant's proposal of the new interrupts-
> extended property. Since that comparison has already been drawn, I think
> we can agree that both are used in similar ways. Therefore we should
> consider what we've learned from interrupt-parent when designing this
> generic IOMMU binding to avoid having to introduce iommu-extended at
> some point.

The problem in that case is really the interrupts property not having
a phandle. This case would be a bit different because there is not an
analogous property to interrupts.

However, I don't really like connections described via phandles being
done differently for different bindings. And this one has been a bit
unusual.

>> So the question is: do we actually need to describe masters that master
>> through multiple SMMUs as a single node in the devicetree?
>
> I would think so, yes. The alternative would be to have several nodes
> that describe the same device, and that conflicts on a different level.
>
> Perhaps it could be done by having separate sub-nodes that each use a
> different IOMMU, but that sounds like a much grosser solution. That
> pretty much boils down to interrupt-parent/interrupt-map.

Perhaps not within the DT, but we could have this case with IOMMUs
within PCIe bus and the PCIe RC itself behind an IOMMU.

I'll reply to Hiroshi's email with more on a variation of his proposal.

Rob

Re: Report from 2013 ARM kernel summit

[Rob Herring]

On Wed, Nov 20, 2013 at 4:31 AM, Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org> wrote:
> On Tue, Nov 19, 2013 at 08:45:02PM +0000, Rob Herring wrote:
>> On 11/19/2013 11:35 AM, Will Deacon wrote:
>> > Adding Andreas and Rob for input on potential binding additions to the SMMU.
>>
>> The above proposal would be an incompatible change. However, I think we
>> could still deal with a change in this binding at this stage.
>>
>> One way approach to handle this without changing the binding would be to
>> scan the DT for all iommu's up front and create a list of all nodes and
>> their iommu parent. The fact that the hierarchy is described in a way
>> that doesn't fit Linux well is really a Linux implementation detail.
>>
>> If changing the binding, a simple approach would be to allow
>> 'smmu-parent' to be a bus and/or device property and not just for
>> chained iommu's. This could be a global or bus property that is
>> inherited. Like interrupt-parent, you would have to deal with the parent
>> being itself. Also, perhaps iommu-parent would be a better name. In any
>> case, I'd like to see this all be a generic iommu binding.
>
> I like that idea. I've recently been toying with removing the chained IOMMU
> support, since I don't think anybody is using it who is interested in
> mainline. However, making it more general sounds like a better idea.
>
> One potential issue is that I think the nvidia guys want to describe masters
> that master via multiple SMMUs (which I believe was the motivation for
> moving the stream-ids out into the master nodes, rather than keeping them in
> the SMMU). Again, that's not something we can easily add to the arm-smmu,
> because the incoming stream-ids are a property of the SMMU node.

For stream-ids, I think that is debatable. It is not a property of the
SMMU. The AXI ID is determined by the master. Yes, AXI ID is only part
of it and the SMMU determines the full stream-id, but that doesn't
really matter. Look at interrupts properties as a comparison. The
number of cells and the meaning of the cell values is determined by
the interrupt controller, not the node with the interrupts property.

Rob

> So the question is: do we actually need to describe masters that master
> through multiple SMMUs as a single node in the devicetree?
>
> Will