Re: [PATCH v4 00/27] arm64: Dom0 ITS emulation

[Julien Grall <julien.grall@arm.com>]

Hi Andre,

Despite having asked multiple times a list of missing items to support 
guest, the list is still not there.

Please provide a full list of what is missing in the cover letter of the 
next version.

Cheers,

On 03/04/17 21:28, Andre Przywara wrote:
> Hi,
>
> I managed to go over the remaining emails I couldn't finish on Friday.
> This series is the result of this and has about 30 smaller fixes, see
> the changelog below. This should address all comments Stefano had.
> There are a few things my brain cannot cope with today anymore, so I
> will address them with Julien face-to-face tomorrow:
> - Move lpi_get_priority() and do_LPI() into gic_ops
> - check MOVI behavior in our special case
> - check LPI state changes if already in an LR
> - implement indirect table
> - move GENMASK_ULL and other helpers into separate patches
> - re-check issue reported by Cavium
> - agree on having a command line for the devices or not
> - rebasing artifacts
> - anything not mentioned here ;-)
>
> 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.
>
> 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.
> For interrupts which are queued to or are actually in a guest we
> allocate struct pending_irq's on demand. As it is expected that only a
> very small number of interrupts is ever on a VCPU at the same time, this
> seems like the best approach. For now allocated structs are re-used and
> held in a linked list. Should it emerge that traversing a linked list
> is a performance issue, this can be changed to use a hash table.
>
> * 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. For those tables which are page sized (devices, collections (CPUs),
> LPI properties) we map those pages into Xen, so we can easily access
> them from the virtual GIC code.
> Unfortunately the actual interrupt mapping tables are not necessarily
> page aligned, also can be much smaller than a page, so mapping all of
> them permanently is fiddly. As ITS commands in need to iterate those
> tables are pretty rare after all, we for now map them on demand upon
> emulating a virtual ITS command. This is acceptable because "mapping"
> them is actually very cheap on arm64. Also as we can't properly protect
> those areas due to their sub-page-size property, we validate the data
> in there before actually using it. The vITS code basically just stores
> the data in there which the guest has actually transferred via the
> virtual ITS command queue before, so there is no secret revealed nor
> does it create an attack vector for a malicious guest.
>
> * 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.
>
> As it is expected that the ITS support will become a tech preview in the
> first release, 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, I have some hope we can still clean this up.
> Also some parameters are config options which can be overridden on the
> Xen commandline. This is to support experimentation and adaption to
> various platforms, ideally we find either one-size-fits-all values or
> find another way of getting rid of this.
>
> This code boots Dom0 on an ARM Fast Model with ITS support. I tried to
> address the issues seen by people running the previous version on real
> hardware, though couldn't verify this here for myself.
> So any testing, bug reports (and possibly even fixes) are very welcome.
>
> The code can also be found on the its/v4 branch here:
> git://linux-arm.org/xen-ap.git
> http://www.linux-arm.org/git?p=xen-ap.git;a=shortlog;h=refs/heads/its/v4
>
> Cheers,
> Andre
>
> Changelog v3 .. v4:
> - make HAS_ITS depend on EXPERT
> - introduce new patch 02 to initialize host ITS early
> - swap "host LPI array" and "device mapping" patch
> - fix cmd_lock init position
> - introduce warning on high number of LPI allocations
> - various int -> unsigned fixes
> - adding and improving comments
> - rate limit ITS command queue full msg
> - drop unneeded checks
> - validate against allowed number of device IDs
> - avoid memory leaks when removing devices
> - improve algorithm for finding free host LPI
> - convert unmap_all_devices from goto to while loop
> - add message on remapping ITS device
> - name virtual device / event IDs properly
> - use atomic read when reading ITT entry
>
> Changelog v2 .. v3:
> - preallocate struct pending_irq's
> - map ITS and redistributor tables only on demand
> - store property, enable and pending bit in struct pending_irq
> - improve error checking and handling
> - add comments
>
> Changelog v1 .. v2:
> - clean up header file inclusion
> - rework host ITS table allocation: observe attributes, many fixes
> - remove patch 1 to export __flush_dcache_area, use existing function instead
> - use number of LPIs internally instead of number of bits
> - keep host_its_list as private as possible
> - keep struct its_devices private
> - rework gicv3_its_map_guest_devices
> - fix rbtree issues
> - more error handling and propagation
> - cope with GICv4 implementations (but no virtual LPI features!)
> - abstract host and guest ITSes by using doorbell addresses
> - join per-redistributor variables into one per-CPU structure
> - fix data types (unsigned int)
> - many minor bug fixes
>
> (Rough) changelog RFC-v2 .. v1:
> - split host ITS driver into gic-v3-lpi.c and gic-v3-its.c part
> - rename virtual ITS driver file to vgic-v3-its.c
> - use macros and named constants for all magic numbers
> - use atomic accessors for accessing the host LPI data
> - remove leftovers from connecting virtual and host ITSes
> - bail out if host ITS is disabled in the DT
> - rework map/unmap_guest_pages():
>     - split off p2m part as get/put_guest_pages (to be done on allocation)
>     - get rid of vmap, using map_domain_page() instead
> - delay allocation of virtual tables until actual LPI/ITS enablement
> - properly size both virtual and physical tables upon allocation
> - fix put_domain() locking issues in physdev_op and LPI handling code
> - add and extend comments in various areas
> - fix lotsa coding style and white space issues, including comment style
> - add locking to data structures not yet covered
> - fix various locking issues
> - use an rbtree to deal with ITS devices (instead of a list)
> - properly handle memory attributes for ITS tables
> - handle cacheable/non-cacheable ITS table mappings
> - sanitize guest provided ITS/LPI table attributes
> - fix breakage on non-GICv2 compatible host GICv3 controllers
> - add command line parameters on top of Kconfig options
> - properly wait for an ITS to become quiescient before enabling it
> - handle host ITS command queue errors
> - actually wait for host ITS command completion (READR==WRITER)
> - fix ARM32 compilation
> - various patch splits and reorderings
>
> Andre Przywara (27):
>   ARM: GICv3 ITS: parse and store ITS subnodes from hardware DT
>   ARM: GICv3 ITS: initialize host ITS
>   ARM: GICv3: allocate LPI pending and property table
>   ARM: GICv3 ITS: allocate device and collection table
>   ARM: GICv3 ITS: map ITS command buffer
>   ARM: GICv3 ITS: introduce ITS command handling
>   ARM: GICv3 ITS: introduce host LPI array
>   ARM: GICv3 ITS: introduce device mapping
>   ARM: GICv3: introduce separate pending_irq structs for LPIs
>   ARM: GICv3: forward pending LPIs to guests
>   ARM: GICv3: enable ITS and LPIs on the host
>   ARM: vGICv3: handle virtual LPI pending and property tables
>   ARM: vGICv3: Handle disabled LPIs
>   ARM: vGICv3: introduce basic ITS emulation bits
>   ARM: vITS: introduce translation table walks
>   ARM: vITS: handle CLEAR command
>   ARM: vITS: handle INT command
>   ARM: vITS: handle MAPC command
>   ARM: vITS: handle MAPD command
>   ARM: vITS: handle MAPTI command
>   ARM: vITS: handle MOVI command
>   ARM: vITS: handle DISCARD command
>   ARM: vITS: handle INV command
>   ARM: vITS: handle INVALL command
>   ARM: vITS: create and initialize virtual ITSes for Dom0
>   ARM: vITS: create ITS subnodes for Dom0 DT
>   ARM: vGIC: advertise LPI support
>
>  docs/misc/xen-command-line.markdown |   18 +
>  xen/arch/arm/Kconfig                |    5 +
>  xen/arch/arm/Makefile               |    3 +
>  xen/arch/arm/gic-v3-its.c           |  981 ++++++++++++++++++++++++++++++
>  xen/arch/arm/gic-v3-lpi.c           |  545 +++++++++++++++++
>  xen/arch/arm/gic-v3.c               |   80 ++-
>  xen/arch/arm/gic.c                  |   20 +-
>  xen/arch/arm/vgic-v3-its.c          | 1122 +++++++++++++++++++++++++++++++++++
>  xen/arch/arm/vgic-v3.c              |  272 ++++++++-
>  xen/arch/arm/vgic.c                 |   16 +-
>  xen/common/memory.c                 |   61 ++
>  xen/include/asm-arm/bitops.h        |    1 +
>  xen/include/asm-arm/config.h        |    2 +
>  xen/include/asm-arm/domain.h        |   12 +-
>  xen/include/asm-arm/gic.h           |    2 +
>  xen/include/asm-arm/gic_v3_defs.h   |   75 ++-
>  xen/include/asm-arm/gic_v3_its.h    |  262 ++++++++
>  xen/include/asm-arm/irq.h           |   15 +
>  xen/include/asm-arm/vgic.h          |    6 +
>  xen/include/xen/bitops.h            |    5 +-
>  xen/include/xen/mm.h                |    8 +
>  21 files changed, 3468 insertions(+), 43 deletions(-)
>  create mode 100644 xen/arch/arm/gic-v3-its.c
>  create mode 100644 xen/arch/arm/gic-v3-lpi.c
>  create mode 100644 xen/arch/arm/vgic-v3-its.c
>  create mode 100644 xen/include/asm-arm/gic_v3_its.h
>

-- 
Julien Grall

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