Use of GICv3/ITS with PCIe host-generic driver - resizing ITS MAPD?

Use of GICv3/ITS with PCIe host-generic driver - resizing ITS MAPD?

[Alan Douglas]

I am using a Cadence PCIe Root Port in an ECAM setup with A53 and GICv3 with ITS, and have configured the host-generic driver to use MSI, using Device Tree.
Setup works well and the bus is correctly enumerated.  However, to get MSI/MSI-X working correctly I needed to make a change in drivers/irqchip/irq-gic-v3-its.c

The PCI setup I am currently testing is:
# lspci -t
-[0000:00]---00.0-[01-04]----00.0-[02-04]--+-03.0-[03]----00.0
                                           \-07.0-[04]----00.0

Device 00:00.0 is a pci-bridge, and claims 1 MSI interrupt.
       01:00.0 is a pci-bridge, 1 MSI interrupt
       02:03.0 is a pci-bridge, 1 MSI interrupt
       02:03.0 is a pci-bridge, 1 MSI interrupt
       03:00.0 is a USB controller with 2 MSI-X interrupts
       04:00.0 is a SATA controller with 1 MSI interrupt

When setting up bus 0, the ITS device is created, and its_build_map_cmd() sets the size of the ITS MAPD based on the number of interrupts claimed by bus 0.  When subsequent buses are enumerated, the ITS device will be reused, however we do not increase the number of supported interrupts to allow for the additional interrupts claimed by the additional devices being enumerated.  (This can be seen in its_msi_prepare(), which is called for each device which has MSI/MSI-X enabled, and will reuse an existing ITS. )

The solution I have implemented is in its_alloc_device_irq(), if the offset into the LPI table for the allocated interrupt is greater than the ITS MAPD, I reallocate the itt area, and resize the ITS MAPD.  I'm looking for comments as to whether this is a suitable solution and I should submit as a patch, is there some other recommendation or am I missing something regarding reuse of the ITS?

Regards
Alan Douglas

Use of GICv3/ITS with PCIe host-generic driver - resizing ITS MAPD?

[Marc Zyngier]

Hi Alan,

On Tue, Nov 01 2016 at 11:47:46 AM, Alan Douglas <adouglas@cadence.com> wrote:
> I am using a Cadence PCIe Root Port in an ECAM setup with A53 and
> GICv3 with ITS, and have configured the host-generic driver to use
> MSI, using Device Tree.  Setup works well and the bus is correctly
> enumerated.  However, to get MSI/MSI-X working correctly I needed to
> make a change in drivers/irqchip/irq-gic-v3-its.c
>
> The PCI setup I am currently testing is:
> # lspci -t
> -[0000:00]---00.0-[01-04]----00.0-[02-04]--+-03.0-[03]----00.0
>                                            \-07.0-[04]----00.0
>
> Device 00:00.0 is a pci-bridge, and claims 1 MSI interrupt.
>        01:00.0 is a pci-bridge, 1 MSI interrupt
>        02:03.0 is a pci-bridge, 1 MSI interrupt
>        02:03.0 is a pci-bridge, 1 MSI interrupt
>        03:00.0 is a USB controller with 2 MSI-X interrupts
>        04:00.0 is a SATA controller with 1 MSI interrupt
>
> When setting up bus 0, the ITS device is created, and
> its_build_map_cmd() sets the size of the ITS MAPD based on the number
> of interrupts claimed by bus 0.  When subsequent buses are enumerated,
> the ITS device will be reused, however we do not increase the number
> of supported interrupts to allow for the additional interrupts claimed
> by the additional devices being enumerated.  (This can be seen in
> its_msi_prepare(), which is called for each device which has MSI/MSI-X
> enabled, and will reuse an existing ITS. )

Am I right in understanding that all the PCIe devices in your system
end-up aliasing to the same RequesterID? If so, that's a major
issue. The ITS is designed so that each device exposes its *own* RID,
and have its own Interrupt Translation Table (ITT).

In your case, you seem to first discover the root port, which is not
upstream of anything, so it doesn't alias with anything at that
point. We allocate the corresponding ITT, and it's all fine. Until we
start probing the rest, and ugly things happen.

> The solution I have implemented is in its_alloc_device_irq(), if the
> offset into the LPI table for the allocated interrupt is greater than
> the ITS MAPD, I reallocate the itt area, and resize the ITS MAPD.  I'm
> looking for comments as to whether this is a suitable solution and I
> should submit as a patch, is there some other recommendation or am I
> missing something regarding reuse of the ITS?

Not seeing the patch doesn't help, but in general reallocating an ITT
implies unmapping everything at the ITS level (LPIs, devices),
recreating the ITT, remapping everything, and then replaying fake
interrupts to cater for anything that you;ve missed in between. Not
exactly fun.

So before we have to introduce all of this, it would be good to find out
*why* are all the devices aliased together. Because this completely
screws all the ITS isolation, and makes device assignment to guest VM
completely insecure.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny.

Use of GICv3/ITS with PCIe host-generic driver - resizing ITS MAPD?

[Alan Douglas]

Hi Marc,

> > When setting up bus 0, the ITS device is created, and
> > its_build_map_cmd() sets the size of the ITS MAPD based on the number
> > of interrupts claimed by bus 0.  When subsequent buses are enumerated,
> > the ITS device will be reused, however we do not increase the number
> > of supported interrupts to allow for the additional interrupts claimed
> > by the additional devices being enumerated.  (This can be seen in
> > its_msi_prepare(), which is called for each device which has MSI/MSI-X
> > enabled, and will reuse an existing ITS. )
> 
> Am I right in understanding that all the PCIe devices in your system end-up
> aliasing to the same RequesterID? If so, that's a major issue. The ITS is
> designed so that each device exposes its *own* RID, and have its own
> Interrupt Translation Table (ITT).
> 
> In your case, you seem to first discover the root port, which is not upstream
> of anything, so it doesn't alias with anything at that point. We allocate the
> corresponding ITT, and it's all fine. Until we start probing the rest, and ugly
> things happen.
> 
Yes, your understanding is correct.  I will dig into this a bit further to see what
is wrong then send an update.  I suspect my DTS msi mapping.

> > The solution I have implemented is in its_alloc_device_irq(), if the
> > offset into the LPI table for the allocated interrupt is greater than
> > the ITS MAPD, I reallocate the itt area, and resize the ITS MAPD.  I'm
> > looking for comments as to whether this is a suitable solution and I
> > should submit as a patch, is there some other recommendation or am I
> > missing something regarding reuse of the ITS?
> 
> Not seeing the patch doesn't help, but in general reallocating an ITT implies
> unmapping everything at the ITS level (LPIs, devices), recreating the ITT,
> remapping everything, and then replaying fake interrupts to cater for
> anything that you;ve missed in between. Not exactly fun.
> 
> So before we have to introduce all of this, it would be good to find out
> *why* are all the devices aliased together. Because this completely screws
> all the ITS isolation, and makes device assignment to guest VM completely
> insecure.
OK, understood.  My current patch doesn't deal with all the issues you point out,
but I will concentrate on avoiding the re-use instead.   Thanks for clarifying that
this re-use is not expected in my case.

Thanks,
Alan

Use of GICv3/ITS with PCIe host-generic driver - resizing ITS MAPD?

[Marc Zyngier]

On Wed, Nov 02 2016 at 09:36:14 AM, Alan Douglas <adouglas@cadence.com> wrote:
> Hi Marc,
>
>> > When setting up bus 0, the ITS device is created, and
>> > its_build_map_cmd() sets the size of the ITS MAPD based on the number
>> > of interrupts claimed by bus 0.  When subsequent buses are enumerated,
>> > the ITS device will be reused, however we do not increase the number
>> > of supported interrupts to allow for the additional interrupts claimed
>> > by the additional devices being enumerated.  (This can be seen in
>> > its_msi_prepare(), which is called for each device which has MSI/MSI-X
>> > enabled, and will reuse an existing ITS. )
>> 
>> Am I right in understanding that all the PCIe devices in your system end-up
>> aliasing to the same RequesterID? If so, that's a major issue. The ITS is
>> designed so that each device exposes its *own* RID, and have its own
>> Interrupt Translation Table (ITT).
>> 
>> In your case, you seem to first discover the root port, which is not upstream
>> of anything, so it doesn't alias with anything at that point. We allocate the
>> corresponding ITT, and it's all fine. Until we start probing the rest, and ugly
>> things happen.
>> 
> Yes, your understanding is correct.  I will dig into this a bit
> further to see what is wrong then send an update.  I suspect my DTS
> msi mapping.

Right. That would explain a lot of what you're seeing. In general, and
unless you have some funky remapping going on, you're better off having
a very straightforward msi-map property in your RC node (such as example
#1 in Documentation/devicetree/bindings/pci/pci-msi.txt).

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny.

Use of GICv3/ITS with PCIe host-generic driver - resizing ITS MAPD?

[Alan Douglas]

Hi Marc,
> >> > When setting up bus 0, the ITS device is created, and
> >> > its_build_map_cmd() sets the size of the ITS MAPD based on the
> >> > number of interrupts claimed by bus 0.  When subsequent buses are
> >> > enumerated, the ITS device will be reused, however we do not
> >> > increase the number of supported interrupts to allow for the
> >> > additional interrupts claimed by the additional devices being
> >> > enumerated.  (This can be seen in its_msi_prepare(), which is
> >> > called for each device which has MSI/MSI-X enabled, and will reuse
> >> > an existing ITS. )
> >>
> >> Am I right in understanding that all the PCIe devices in your system
> >> end-up aliasing to the same RequesterID? If so, that's a major issue.
> >> The ITS is designed so that each device exposes its *own* RID, and
> >> have its own Interrupt Translation Table (ITT).
> >>
> >> In your case, you seem to first discover the root port, which is not
> >> upstream of anything, so it doesn't alias with anything at that
> >> point. We allocate the corresponding ITT, and it's all fine. Until we
> >> start probing the rest, and ugly things happen.
> >>
> > Yes, your understanding is correct.  I will dig into this a bit
> > further to see what is wrong then send an update.  I suspect my DTS
> > msi mapping.
> 
> Right. That would explain a lot of what you're seeing. In general, and unless
> you have some funky remapping going on, you're better off having a very
> straightforward msi-map property in your RC node (such as example
> #1 in Documentation/devicetree/bindings/pci/pci-msi.txt).
The problem was that I had an msi-map-mask specified such that all
requester IDs were seen as zero.  Now fixed that and checked that the
requester ID for each device is being correctly provided by HW to the GIC, 
and all is now working correctly with without any patches required.
Thanks for your help.

Alan