Re: [Xen-devel] [PATCH v7 00/34] arm64: Dom0 ITS emulation

[Vijay Kilari]

Hi Andre,

On Sat, Apr 8, 2017 at 3:37 AM, Andre Przywara  wrote:
> Hi,
>
> an only slightly modified repost of the last version.
> I added the Reviewed-by: and Acked-by: tags from Stefano and Julien
> and rebased on top of the latest staging tree:
> commit 89216c7999eb5b8558bfac7d61ae0d5ab844ce3f
> Author: Dario Faggioli 
> Date:   Fri Apr 7 18:57:14 2017 +0200
>
> xen: credit1: treat pCPUs more evenly during balancing.
>
> Other than that and one typo and comment fix the first 10 patches have
> not been changed.
> I dropped the addition of the GIC_IRQ_GUEST_LPI_PENDING bit in patch 12/36
> and followed Stefano's suggestion, which led to the removal of former
> patches 17/36 and 23/36 and some simplification in later patches.
> I haven't been able to address most review comments from the last part of
> v5 yet, but will definitely still fix them.
> Detailed changelog below.
>
> Cheers,
> Andre
>
> --
> This series adds support for emulation of an ARM GICv3 ITS interrupt
> controller. For hardware which relies on the ITS to provide interrupts for
> its peripherals this code is needed to get a machine booted into Dom0 at
> all. ITS emulation for DomUs is only really useful with PCI passthrough,
> which is not yet available for ARM. It is expected that this feature
> will be co-developed with the ITS DomU code. However this code drop here
> considered DomU emulation already, to keep later architectural changes
> to a minimum.
>
> This is technical preview version to allow early testing of the feature.
> Things not (properly) addressed in this release:
> - The MOVALL command is not emulated. In our case there is really nothing
> to do here. We might need to revisit this in the future for DomU support.
> - The INVALL command might need some rework to be more efficient. Currently
> we iterate over all mapped LPIs, which might take a bit longer.
> - Indirect tables are not supported. This affects both the host and the
> virtual side.
> - The command queue locking is currently suboptimal and should be made more
> fine-grained in the future, if possible.
> - We need to properly investigate the possible interaction when devices get
> removed. This requires to properly clean up and remove any associated
> resources like pending or in-flight LPIs, for instance.
>
>
> Some generic design principles:
>
> * The current GIC code statically allocates structures for each supported
> IRQ (both for the host and the guest), which due to the potentially
> millions of LPI interrupts is not feasible to copy for the ITS.
> So we refrain from introducing the ITS as a first class Xen interrupt
> controller, also we don't hold struct irq_desc's or struct pending_irq's
> for each possible LPI.
> Fortunately LPIs are only interesting to guests, so we get away with
> storing only the virtual IRQ number and the guest VCPU for each allocated
> host LPI, which can be stashed into one uint64_t. This data is stored in
> a two-level table, which is both memory efficient and quick to access.
> We hook into the existing IRQ handling and VGIC code to avoid accessing
> the normal structures, providing alternative methods for getting the
> needed information (priority, is enabled?) for LPIs.
> Whenever a guest maps a device, we allocate the maximum required number
> of struct pending_irq's, so that any triggering LPI can find its data
> structure. Upon the guest actually mapping the LPI, this pointer to the
> corresponding pending_irq gets entered into a radix tree, so that it can
> be quickly looked up.
>
> * On the guest side we (later will) have to deal with malicious guests
> trying to hog Xen with mapping requests for a lot of LPIs, for instance.
> As the ITS actually uses system memory for storing status information,
> we use this memory (which the guest has to provide) to naturally limit
> a guest. Whenever we need information from any of the ITS tables, we
> temporarily map them (which is cheap on arm64) and copy the required data.
> * An obvious approach to handling some guest ITS commands would be to
> propagate them to the host, for instance to map devices and LPIs and
> to enable or disable LPIs.
> However this (later with DomU support) will create an attack vector, as
> a malicious guest could try to fill the host command queue with
> propagated commands.
> So we try to avoid this situation: Dom0 sending a device mapping (MAPD)
> command is the only time we allow queuing commands to the host ITS command
> queue, as this seems to be the only reliable way of getting the
> required information at the moment. However at the same time we map all
> events to LPIs already, also enable them. This avoids sending commands
> later at runtime, as we can deal with mappings and LPI enabling/disabling
> internally.
>
> To accomodate the tech preview nature of this feature at the moment, there
> is a Kconfig option to enable it. Also it is supported on arm64 only, 

Re: [Xen-devel] [PATCH v7 00/34] arm64: Dom0 ITS emulation

[Julien Grall]

Hi,

Andre mentioned this was only slightly modified. So I am going to 
continue to comment on v6 to keep all comments in same version making 
easier to track.


I would encourage Stefano to do the same.

Cheers,

--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v7 00/34] arm64: Dom0 ITS emulation

[Stefano Stabellini]

Thanks Andre, I have applied patches 1-9.

On Fri, 7 Apr 2017, Andre Przywara wrote:
> Hi,
> 
> an only slightly modified repost of the last version.
> I added the Reviewed-by: and Acked-by: tags from Stefano and Julien
> and rebased on top of the latest staging tree:
> commit 89216c7999eb5b8558bfac7d61ae0d5ab844ce3f
> Author: Dario Faggioli 
> Date:   Fri Apr 7 18:57:14 2017 +0200
> 
> xen: credit1: treat pCPUs more evenly during balancing.
> 
> Other than that and one typo and comment fix the first 10 patches have
> not been changed.
> I dropped the addition of the GIC_IRQ_GUEST_LPI_PENDING bit in patch 12/36
> and followed Stefano's suggestion, which led to the removal of former
> patches 17/36 and 23/36 and some simplification in later patches.
> I haven't been able to address most review comments from the last part of
> v5 yet, but will definitely still fix them.
> Detailed changelog below.
> 
> Cheers,
> Andre
> 
> --
> This series adds support for emulation of an ARM GICv3 ITS interrupt
> controller. For hardware which relies on the ITS to provide interrupts for
> its peripherals this code is needed to get a machine booted into Dom0 at
> all. ITS emulation for DomUs is only really useful with PCI passthrough,
> which is not yet available for ARM. It is expected that this feature
> will be co-developed with the ITS DomU code. However this code drop here
> considered DomU emulation already, to keep later architectural changes
> to a minimum.
> 
> This is technical preview version to allow early testing of the feature.
> Things not (properly) addressed in this release:
> - The MOVALL command is not emulated. In our case there is really nothing
> to do here. We might need to revisit this in the future for DomU support.
> - The INVALL command might need some rework to be more efficient. Currently
> we iterate over all mapped LPIs, which might take a bit longer.
> - Indirect tables are not supported. This affects both the host and the
> virtual side.
> - The command queue locking is currently suboptimal and should be made more
> fine-grained in the future, if possible.
> - We need to properly investigate the possible interaction when devices get
> removed. This requires to properly clean up and remove any associated
> resources like pending or in-flight LPIs, for instance.
> 
> 
> Some generic design principles:
> 
> * The current GIC code statically allocates structures for each supported
> IRQ (both for the host and the guest), which due to the potentially
> millions of LPI interrupts is not feasible to copy for the ITS.
> So we refrain from introducing the ITS as a first class Xen interrupt
> controller, also we don't hold struct irq_desc's or struct pending_irq's
> for each possible LPI.
> Fortunately LPIs are only interesting to guests, so we get away with
> storing only the virtual IRQ number and the guest VCPU for each allocated
> host LPI, which can be stashed into one uint64_t. This data is stored in
> a two-level table, which is both memory efficient and quick to access.
> We hook into the existing IRQ handling and VGIC code to avoid accessing
> the normal structures, providing alternative methods for getting the
> needed information (priority, is enabled?) for LPIs.
> Whenever a guest maps a device, we allocate the maximum required number
> of struct pending_irq's, so that any triggering LPI can find its data
> structure. Upon the guest actually mapping the LPI, this pointer to the
> corresponding pending_irq gets entered into a radix tree, so that it can
> be quickly looked up.
> 
> * On the guest side we (later will) have to deal with malicious guests
> trying to hog Xen with mapping requests for a lot of LPIs, for instance.
> As the ITS actually uses system memory for storing status information,
> we use this memory (which the guest has to provide) to naturally limit
> a guest. Whenever we need information from any of the ITS tables, we
> temporarily map them (which is cheap on arm64) and copy the required data.
> * An obvious approach to handling some guest ITS commands would be to
> propagate them to the host, for instance to map devices and LPIs and
> to enable or disable LPIs.
> However this (later with DomU support) will create an attack vector, as
> a malicious guest could try to fill the host command queue with
> propagated commands.
> So we try to avoid this situation: Dom0 sending a device mapping (MAPD)
> command is the only time we allow queuing commands to the host ITS command
> queue, as this seems to be the only reliable way of getting the
> required information at the moment. However at the same time we map all
> events to LPIs already, also enable them. This avoids sending commands
> later at runtime, as we can deal with mappings and LPI enabling/disabling
> internally.
> 
> To accomodate the tech preview nature of this feature at the moment, there
> is a Kconfig option to enable it. Also it is supported on arm64 

[Xen-devel] [PATCH v7 00/34] arm64: Dom0 ITS emulation

[Andre Przywara]

Hi,

an only slightly modified repost of the last version.
I added the Reviewed-by: and Acked-by: tags from Stefano and Julien
and rebased on top of the latest staging tree:
commit 89216c7999eb5b8558bfac7d61ae0d5ab844ce3f
Author: Dario Faggioli 
Date:   Fri Apr 7 18:57:14 2017 +0200

xen: credit1: treat pCPUs more evenly during balancing.

Other than that and one typo and comment fix the first 10 patches have
not been changed.
I dropped the addition of the GIC_IRQ_GUEST_LPI_PENDING bit in patch 12/36
and followed Stefano's suggestion, which led to the removal of former
patches 17/36 and 23/36 and some simplification in later patches.
I haven't been able to address most review comments from the last part of
v5 yet, but will definitely still fix them.
Detailed changelog below.

Cheers,
Andre

--
This series adds support for emulation of an ARM GICv3 ITS interrupt
controller. For hardware which relies on the ITS to provide interrupts for
its peripherals this code is needed to get a machine booted into Dom0 at
all. ITS emulation for DomUs is only really useful with PCI passthrough,
which is not yet available for ARM. It is expected that this feature
will be co-developed with the ITS DomU code. However this code drop here
considered DomU emulation already, to keep later architectural changes
to a minimum.

This is technical preview version to allow early testing of the feature.
Things not (properly) addressed in this release:
- The MOVALL command is not emulated. In our case there is really nothing
to do here. We might need to revisit this in the future for DomU support.
- The INVALL command might need some rework to be more efficient. Currently
we iterate over all mapped LPIs, which might take a bit longer.
- Indirect tables are not supported. This affects both the host and the
virtual side.
- The command queue locking is currently suboptimal and should be made more
fine-grained in the future, if possible.
- We need to properly investigate the possible interaction when devices get
removed. This requires to properly clean up and remove any associated
resources like pending or in-flight LPIs, for instance.


Some generic design principles:

* The current GIC code statically allocates structures for each supported
IRQ (both for the host and the guest), which due to the potentially
millions of LPI interrupts is not feasible to copy for the ITS.
So we refrain from introducing the ITS as a first class Xen interrupt
controller, also we don't hold struct irq_desc's or struct pending_irq's
for each possible LPI.
Fortunately LPIs are only interesting to guests, so we get away with
storing only the virtual IRQ number and the guest VCPU for each allocated
host LPI, which can be stashed into one uint64_t. This data is stored in
a two-level table, which is both memory efficient and quick to access.
We hook into the existing IRQ handling and VGIC code to avoid accessing
the normal structures, providing alternative methods for getting the
needed information (priority, is enabled?) for LPIs.
Whenever a guest maps a device, we allocate the maximum required number
of struct pending_irq's, so that any triggering LPI can find its data
structure. Upon the guest actually mapping the LPI, this pointer to the
corresponding pending_irq gets entered into a radix tree, so that it can
be quickly looked up.

* On the guest side we (later will) have to deal with malicious guests
trying to hog Xen with mapping requests for a lot of LPIs, for instance.
As the ITS actually uses system memory for storing status information,
we use this memory (which the guest has to provide) to naturally limit
a guest. Whenever we need information from any of the ITS tables, we
temporarily map them (which is cheap on arm64) and copy the required data.
* An obvious approach to handling some guest ITS commands would be to
propagate them to the host, for instance to map devices and LPIs and
to enable or disable LPIs.
However this (later with DomU support) will create an attack vector, as
a malicious guest could try to fill the host command queue with
propagated commands.
So we try to avoid this situation: Dom0 sending a device mapping (MAPD)
command is the only time we allow queuing commands to the host ITS command
queue, as this seems to be the only reliable way of getting the
required information at the moment. However at the same time we map all
events to LPIs already, also enable them. This avoids sending commands
later at runtime, as we can deal with mappings and LPI enabling/disabling
internally.

To accomodate the tech preview nature of this feature at the moment, there
is a Kconfig option to enable it. Also it is supported on arm64 only, which
will most likely not change in the future.
This leads to some hideous constructs like an #ifdef'ed header file with
empty function stubs to accomodate arm32 and non-ITS builds, which share
some generic code paths with the ITS emulation.
The number of