diff mbox

[v3,06/26] ARM: GICv3 ITS: introduce device mapping

Message ID 20170331180525.30038-7-andre.przywara@arm.com (mailing list archive)
State New, archived
Headers show

Commit Message

Andre Przywara March 31, 2017, 6:05 p.m. UTC
The ITS uses device IDs to map LPIs to a device. Dom0 will later use
those IDs, which we directly pass on to the host.
For this we have to map each device that Dom0 may request to a host
ITS device with the same identifier.
Allocate the respective memory and enter each device into an rbtree to
later be able to iterate over it or to easily teardown guests.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 xen/arch/arm/gic-v3-its.c        | 227 +++++++++++++++++++++++++++++++++++++++
 xen/arch/arm/vgic-v3.c           |   4 +
 xen/include/asm-arm/domain.h     |   3 +
 xen/include/asm-arm/gic_v3_its.h |  23 ++++
 4 files changed, 257 insertions(+)

Comments

Stefano Stabellini March 31, 2017, 11:20 p.m. UTC | #1
On Fri, 31 Mar 2017, Andre Przywara wrote:
> The ITS uses device IDs to map LPIs to a device. Dom0 will later use
> those IDs, which we directly pass on to the host.
> For this we have to map each device that Dom0 may request to a host
> ITS device with the same identifier.
> Allocate the respective memory and enter each device into an rbtree to
> later be able to iterate over it or to easily teardown guests.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>

See 3a044833-4b61-d807-600c-cf88d6e1901a@arm.com and
alpine.DEB.2.10.1703211718190.11679@sstabellini-ThinkPad-X260 


> ---
>  xen/arch/arm/gic-v3-its.c        | 227 +++++++++++++++++++++++++++++++++++++++
>  xen/arch/arm/vgic-v3.c           |   4 +
>  xen/include/asm-arm/domain.h     |   3 +
>  xen/include/asm-arm/gic_v3_its.h |  23 ++++
>  4 files changed, 257 insertions(+)
> 
> diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
> index 1ac598f..295f7dc 100644
> --- a/xen/arch/arm/gic-v3-its.c
> +++ b/xen/arch/arm/gic-v3-its.c
> @@ -21,6 +21,8 @@
>  #include <xen/lib.h>
>  #include <xen/delay.h>
>  #include <xen/mm.h>
> +#include <xen/rbtree.h>
> +#include <xen/sched.h>
>  #include <xen/sizes.h>
>  #include <asm/gic.h>
>  #include <asm/gic_v3_defs.h>
> @@ -32,6 +34,18 @@
>  
>  LIST_HEAD(host_its_list);
>  
> +struct its_devices {
> +    struct rb_node rbnode;
> +    struct host_its *hw_its;
> +    void *itt_addr;
> +    paddr_t guest_doorbell;             /* Identifies the virtual ITS */
> +    uint32_t host_devid;
> +    uint32_t guest_devid;
> +    uint32_t eventids;                  /* Number of event IDs (MSIs) */
> +    uint32_t *host_lpi_blocks;          /* Which LPIs are used on the host */
> +    struct pending_irq *pend_irqs;      /* One struct per event */
> +};
> +
>  bool gicv3_its_host_has_its(void)
>  {
>      return !list_empty(&host_its_list);
> @@ -151,6 +165,26 @@ static int its_send_cmd_mapc(struct host_its *its, uint32_t collection_id,
>      return its_send_command(its, cmd);
>  }
>  
> +static int its_send_cmd_mapd(struct host_its *its, uint32_t deviceid,
> +                             uint8_t size_bits, paddr_t itt_addr, bool valid)
> +{
> +    uint64_t cmd[4];
> +
> +    if ( valid )
> +    {
> +        ASSERT(size_bits < 32);
> +        ASSERT(!(itt_addr & ~GENMASK_ULL(51, 8)));
> +    }
> +    cmd[0] = GITS_CMD_MAPD | ((uint64_t)deviceid << 32);
> +    cmd[1] = size_bits;
> +    cmd[2] = itt_addr;
> +    if ( valid )
> +        cmd[2] |= GITS_VALID_BIT;
> +    cmd[3] = 0x00;
> +
> +    return its_send_command(its, cmd);
> +}
> +
>  /* Set up the (1:1) collection mapping for the given host CPU. */
>  int gicv3_its_setup_collection(unsigned int cpu)
>  {
> @@ -376,6 +410,7 @@ static int gicv3_its_init_single_its(struct host_its *hw_its)
>      hw_its->devid_bits = min(hw_its->devid_bits, max_its_device_bits);
>      if ( reg & GITS_TYPER_PTA )
>          hw_its->flags |= HOST_ITS_USES_PTA;
> +    hw_its->itte_size = GITS_TYPER_ITT_SIZE(reg);
>  
>      for ( i = 0; i < GITS_BASER_NR_REGS; i++ )
>      {
> @@ -432,6 +467,197 @@ int gicv3_its_init(void)
>      return 0;
>  }
>  
> +static int remove_mapped_guest_device(struct its_devices *dev)
> +{
> +    int ret;
> +
> +    if ( dev->hw_its )
> +    {
> +        /* MAPD also discards all events with this device ID. */
> +        int ret = its_send_cmd_mapd(dev->hw_its, dev->host_devid, 0, 0, false);
> +        if ( ret )
> +            return ret;
> +    }
> +
> +    ret = gicv3_its_wait_commands(dev->hw_its);
> +    if ( ret )
> +        return ret;
> +
> +    xfree(dev->itt_addr);
> +    xfree(dev->pend_irqs);
> +    xfree(dev);
> +
> +    return 0;
> +}
> +
> +static struct host_its *gicv3_its_find_by_doorbell(paddr_t doorbell_address)
> +{
> +    struct host_its *hw_its;
> +
> +    list_for_each_entry(hw_its, &host_its_list, entry)
> +    {
> +        if ( hw_its->addr + ITS_DOORBELL_OFFSET == doorbell_address )
> +            return hw_its;
> +    }
> +
> +    return NULL;
> +}
> +
> +static int compare_its_guest_devices(struct its_devices *dev,
> +                                     paddr_t doorbell, uint32_t devid)
> +{
> +    if ( dev->guest_doorbell < doorbell )
> +        return -1;
> +
> +    if ( dev->guest_doorbell > doorbell )
> +        return 1;
> +
> +    if ( dev->guest_devid < devid )
> +        return -1;
> +
> +    if ( dev->guest_devid > devid )
> +        return 1;
> +
> +    return 0;
> +}
> +
> +/*
> + * Map a hardware device, identified by a certain host ITS and its device ID
> + * to domain d, a guest ITS (identified by its doorbell address) and device ID.
> + * Also provide the number of events (MSIs) needed for that device.
> + * This does not check if this particular hardware device is already mapped
> + * at another domain, it is expected that this would be done by the caller.
> + */
> +int gicv3_its_map_guest_device(struct domain *d,
> +                               paddr_t host_doorbell, uint32_t host_devid,
> +                               paddr_t guest_doorbell, uint32_t guest_devid,
> +                               uint32_t nr_events, bool valid)
> +{
> +    void *itt_addr = NULL;
> +    struct host_its *hw_its;
> +    struct its_devices *dev = NULL;
> +    struct rb_node **new = &d->arch.vgic.its_devices.rb_node, *parent = NULL;
> +    int ret = -ENOENT;
> +
> +    hw_its = gicv3_its_find_by_doorbell(host_doorbell);
> +    if ( !hw_its )
> +        return ret;
> +
> +    /* check for already existing mappings */
> +    spin_lock(&d->arch.vgic.its_devices_lock);
> +    while ( *new )
> +    {
> +        struct its_devices *temp;
> +        int cmp;
> +
> +        temp = rb_entry(*new, struct its_devices, rbnode);
> +
> +        parent = *new;
> +        cmp = compare_its_guest_devices(temp, guest_doorbell, guest_devid);
> +        if ( !cmp )
> +        {
> +            if ( !valid )
> +                rb_erase(&temp->rbnode, &d->arch.vgic.its_devices);
> +
> +            spin_unlock(&d->arch.vgic.its_devices_lock);
> +
> +            if ( valid )
> +                return -EBUSY;
> +
> +            return remove_mapped_guest_device(temp);
> +        }
> +
> +        if ( cmp > 0 )
> +            new = &((*new)->rb_left);
> +        else
> +            new = &((*new)->rb_right);
> +    }
> +
> +    if ( !valid )
> +        goto out_unlock;
> +
> +    ret = -ENOMEM;
> +
> +    /* An Interrupt Translation Table needs to be 256-byte aligned. */
> +    itt_addr = _xzalloc(nr_events * hw_its->itte_size, 256);
> +    if ( !itt_addr )
> +        goto out_unlock;
> +
> +    dev = xzalloc(struct its_devices);
> +    if ( !dev )
> +        goto out_unlock;
> +
> +    /*
> +     * Allocate the pending_irqs for each virtual LPI. They will be put
> +     * into the domain's radix tree upon the guest's MAPTI command.
> +     */
> +    dev->pend_irqs = xzalloc_array(struct pending_irq, nr_events);
> +    if ( !dev->pend_irqs )
> +        goto out_unlock;
> +
> +    ret = its_send_cmd_mapd(hw_its, host_devid,
> +                            fls(ROUNDUP(nr_events, LPI_BLOCK) - 1) - 1,
> +                            virt_to_maddr(itt_addr), true);
> +    if ( ret )
> +        goto out_unlock;
> +
> +    dev->itt_addr = itt_addr;
> +    dev->hw_its = hw_its;
> +    dev->guest_doorbell = guest_doorbell;
> +    dev->guest_devid = guest_devid;
> +    dev->host_devid = host_devid;
> +    dev->eventids = nr_events;
> +
> +    rb_link_node(&dev->rbnode, parent, new);
> +    rb_insert_color(&dev->rbnode, &d->arch.vgic.its_devices);
> +
> +    spin_unlock(&d->arch.vgic.its_devices_lock);
> +
> +    return 0;
> +
> +out_unlock:
> +    spin_unlock(&d->arch.vgic.its_devices_lock);
> +    if ( dev )
> +    {
> +        xfree(dev->pend_irqs);
> +        xfree(dev->host_lpi_blocks);
> +    }
> +    xfree(itt_addr);
> +    xfree(dev);
> +    return ret;
> +}
> +
> +/* Removing any connections a domain had to any ITS in the system. */
> +void gicv3_its_unmap_all_devices(struct domain *d)
> +{
> +    struct rb_node *victim;
> +    struct its_devices *dev;
> +
> +    /*
> +     * This is an easily readable, but suboptimal implementation.
> +     * It uses the provided iteration wrapper and erases each node, which
> +     * possibly triggers rebalancing.
> +     * This seems overkill since we are going to abolish the whole tree, but
> +     * avoids an open-coded re-implementation of the traversal functions with
> +     * some recursive function calls.
> +     */
> +restart:
> +    spin_lock(&d->arch.vgic.its_devices_lock);
> +    if ( (victim = rb_first(&d->arch.vgic.its_devices)) )
> +    {
> +        dev = rb_entry(victim, struct its_devices, rbnode);
> +        rb_erase(victim, &d->arch.vgic.its_devices);
> +
> +        spin_unlock(&d->arch.vgic.its_devices_lock);
> +
> +        remove_mapped_guest_device(dev);
> +
> +        goto restart;
> +    }
> +
> +    spin_unlock(&d->arch.vgic.its_devices_lock);
> +}
> +
>  /* Scan the DT for any ITS nodes and create a list of host ITSes out of it. */
>  void gicv3_its_dt_init(const struct dt_device_node *node)
>  {
> @@ -459,6 +685,7 @@ void gicv3_its_dt_init(const struct dt_device_node *node)
>          its_data->addr = addr;
>          its_data->size = size;
>          its_data->dt_node = its;
> +        spin_lock_init(&its_data->cmd_lock);
>  
>          printk("GICv3: Found ITS @0x%lx\n", addr);
>  
> diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
> index d61479d..6242252 100644
> --- a/xen/arch/arm/vgic-v3.c
> +++ b/xen/arch/arm/vgic-v3.c
> @@ -1450,6 +1450,9 @@ static int vgic_v3_domain_init(struct domain *d)
>      d->arch.vgic.nr_regions = rdist_count;
>      d->arch.vgic.rdist_regions = rdist_regions;
>  
> +    spin_lock_init(&d->arch.vgic.its_devices_lock);
> +    d->arch.vgic.its_devices = RB_ROOT;
> +
>      /*
>       * Domain 0 gets the hardware address.
>       * Guests get the virtual platform layout.
> @@ -1522,6 +1525,7 @@ static int vgic_v3_domain_init(struct domain *d)
>  
>  static void vgic_v3_domain_free(struct domain *d)
>  {
> +    gicv3_its_unmap_all_devices(d);
>      xfree(d->arch.vgic.rdist_regions);
>  }
>  
> diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
> index 2d6fbb1..e559027 100644
> --- a/xen/include/asm-arm/domain.h
> +++ b/xen/include/asm-arm/domain.h
> @@ -11,6 +11,7 @@
>  #include <asm/gic.h>
>  #include <public/hvm/params.h>
>  #include <xen/serial.h>
> +#include <xen/rbtree.h>
>  
>  struct hvm_domain
>  {
> @@ -109,6 +110,8 @@ struct arch_domain
>          } *rdist_regions;
>          int nr_regions;                     /* Number of rdist regions */
>          uint32_t rdist_stride;              /* Re-Distributor stride */
> +        struct rb_root its_devices;         /* Devices mapped to an ITS */
> +        spinlock_t its_devices_lock;        /* Protects the its_devices tree */
>  #endif
>      } vgic;
>  
> diff --git a/xen/include/asm-arm/gic_v3_its.h b/xen/include/asm-arm/gic_v3_its.h
> index 4c2ae1c..4ade5f6 100644
> --- a/xen/include/asm-arm/gic_v3_its.h
> +++ b/xen/include/asm-arm/gic_v3_its.h
> @@ -48,6 +48,10 @@
>  #define GITS_TYPER_DEVICE_ID_BITS(r)    (((r & GITS_TYPER_DEVIDS_MASK) >> \
>                                                 GITS_TYPER_DEVIDS_SHIFT) + 1)
>  #define GITS_TYPER_IDBITS_SHIFT         8
> +#define GITS_TYPER_ITT_SIZE_SHIFT       4
> +#define GITS_TYPER_ITT_SIZE_MASK        (0xfUL << GITS_TYPER_ITT_SIZE_SHIFT)
> +#define GITS_TYPER_ITT_SIZE(r)          ((((r) & GITS_TYPER_ITT_SIZE_MASK) >> \
> +                                                GITS_TYPER_ITT_SIZE_SHIFT) + 1)
>  
>  #define GITS_IIDR_VALUE                 0x34c
>  
> @@ -94,7 +98,10 @@
>  #define GITS_CMD_MOVALL                 0x0e
>  #define GITS_CMD_DISCARD                0x0f
>  
> +#define ITS_DOORBELL_OFFSET             0x10040
> +
>  #include <xen/device_tree.h>
> +#include <xen/rbtree.h>
>  
>  #define HOST_ITS_FLUSH_CMD_QUEUE        (1U << 0)
>  #define HOST_ITS_USES_PTA               (1U << 1)
> @@ -109,6 +116,7 @@ struct host_its {
>      unsigned int devid_bits;
>      spinlock_t cmd_lock;
>      void *cmd_buf;
> +    unsigned int itte_size;
>      unsigned int flags;
>  };
>  
> @@ -135,6 +143,17 @@ uint64_t gicv3_get_redist_address(unsigned int cpu, bool use_pta);
>  /* Map a collection for this host CPU to each host ITS. */
>  int gicv3_its_setup_collection(unsigned int cpu);
>  
> +/*
> + * Map a device on the host by allocating an ITT on the host (ITS).
> + * "nr_event" specifies how many events (interrupts) this device will need.
> + * Setting "valid" to false deallocates the device.
> + */
> +int gicv3_its_map_guest_device(struct domain *d,
> +                               paddr_t host_doorbell, uint32_t host_devid,
> +                               paddr_t guest_doorbell, uint32_t guest_devid,
> +                               uint32_t nr_events, bool valid);
> +void gicv3_its_unmap_all_devices(struct domain *d);
> +
>  #else
>  
>  static LIST_HEAD(host_its_list);
> @@ -173,6 +192,10 @@ static inline int gicv3_its_setup_collection(unsigned int cpu)
>      return 0;
>  }
>  
> +static inline void gicv3_its_unmap_all_devices(struct domain *d)
> +{
> +}
> +
>  #endif /* CONFIG_HAS_ITS */
>  
>  #endif
> -- 
> 2.9.0
>
Vijay Kilari April 1, 2017, 8:01 a.m. UTC | #2
Hi Andre,

On Fri, Mar 31, 2017 at 11:35 PM, Andre Przywara <andre.przywara@arm.com> wrote:
> The ITS uses device IDs to map LPIs to a device. Dom0 will later use
> those IDs, which we directly pass on to the host.
> For this we have to map each device that Dom0 may request to a host
> ITS device with the same identifier.
> Allocate the respective memory and enter each device into an rbtree to
> later be able to iterate over it or to easily teardown guests.
>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  xen/arch/arm/gic-v3-its.c        | 227 +++++++++++++++++++++++++++++++++++++++
>  xen/arch/arm/vgic-v3.c           |   4 +
>  xen/include/asm-arm/domain.h     |   3 +
>  xen/include/asm-arm/gic_v3_its.h |  23 ++++
>  4 files changed, 257 insertions(+)
>
> diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
> index 1ac598f..295f7dc 100644
> --- a/xen/arch/arm/gic-v3-its.c
> +++ b/xen/arch/arm/gic-v3-its.c
> @@ -21,6 +21,8 @@
>  #include <xen/lib.h>
>  #include <xen/delay.h>
>  #include <xen/mm.h>
> +#include <xen/rbtree.h>
> +#include <xen/sched.h>
>  #include <xen/sizes.h>
>  #include <asm/gic.h>
>  #include <asm/gic_v3_defs.h>
> @@ -32,6 +34,18 @@
>
>  LIST_HEAD(host_its_list);
>
> +struct its_devices {
> +    struct rb_node rbnode;
> +    struct host_its *hw_its;
> +    void *itt_addr;
> +    paddr_t guest_doorbell;             /* Identifies the virtual ITS */
> +    uint32_t host_devid;
> +    uint32_t guest_devid;
> +    uint32_t eventids;                  /* Number of event IDs (MSIs) */
> +    uint32_t *host_lpi_blocks;          /* Which LPIs are used on the host */
> +    struct pending_irq *pend_irqs;      /* One struct per event */
> +};
> +
>  bool gicv3_its_host_has_its(void)
>  {
>      return !list_empty(&host_its_list);
> @@ -151,6 +165,26 @@ static int its_send_cmd_mapc(struct host_its *its, uint32_t collection_id,
>      return its_send_command(its, cmd);
>  }
>
> +static int its_send_cmd_mapd(struct host_its *its, uint32_t deviceid,
> +                             uint8_t size_bits, paddr_t itt_addr, bool valid)
> +{
> +    uint64_t cmd[4];
> +
> +    if ( valid )
> +    {
> +        ASSERT(size_bits < 32);
> +        ASSERT(!(itt_addr & ~GENMASK_ULL(51, 8)));
> +    }
> +    cmd[0] = GITS_CMD_MAPD | ((uint64_t)deviceid << 32);
> +    cmd[1] = size_bits;
> +    cmd[2] = itt_addr;
> +    if ( valid )
> +        cmd[2] |= GITS_VALID_BIT;
> +    cmd[3] = 0x00;
> +
> +    return its_send_command(its, cmd);
> +}
> +
>  /* Set up the (1:1) collection mapping for the given host CPU. */
>  int gicv3_its_setup_collection(unsigned int cpu)
>  {
> @@ -376,6 +410,7 @@ static int gicv3_its_init_single_its(struct host_its *hw_its)
>      hw_its->devid_bits = min(hw_its->devid_bits, max_its_device_bits);
>      if ( reg & GITS_TYPER_PTA )
>          hw_its->flags |= HOST_ITS_USES_PTA;
> +    hw_its->itte_size = GITS_TYPER_ITT_SIZE(reg);
>
>      for ( i = 0; i < GITS_BASER_NR_REGS; i++ )
>      {
> @@ -432,6 +467,197 @@ int gicv3_its_init(void)
>      return 0;
>  }
>
> +static int remove_mapped_guest_device(struct its_devices *dev)
> +{
> +    int ret;
> +
> +    if ( dev->hw_its )
> +    {
> +        /* MAPD also discards all events with this device ID. */
> +        int ret = its_send_cmd_mapd(dev->hw_its, dev->host_devid, 0, 0, false);
> +        if ( ret )
> +            return ret;
> +    }
> +
> +    ret = gicv3_its_wait_commands(dev->hw_its);
> +    if ( ret )
> +        return ret;
> +
> +    xfree(dev->itt_addr);
> +    xfree(dev->pend_irqs);
> +    xfree(dev);
> +
> +    return 0;
> +}
> +
> +static struct host_its *gicv3_its_find_by_doorbell(paddr_t doorbell_address)
> +{
> +    struct host_its *hw_its;
> +
> +    list_for_each_entry(hw_its, &host_its_list, entry)
> +    {
> +        if ( hw_its->addr + ITS_DOORBELL_OFFSET == doorbell_address )
> +            return hw_its;
> +    }
> +
> +    return NULL;
> +}
> +
> +static int compare_its_guest_devices(struct its_devices *dev,
> +                                     paddr_t doorbell, uint32_t devid)
> +{
> +    if ( dev->guest_doorbell < doorbell )
> +        return -1;
> +
> +    if ( dev->guest_doorbell > doorbell )
> +        return 1;
> +
> +    if ( dev->guest_devid < devid )
> +        return -1;
> +
> +    if ( dev->guest_devid > devid )
> +        return 1;
> +
> +    return 0;
> +}
> +
> +/*
> + * Map a hardware device, identified by a certain host ITS and its device ID
> + * to domain d, a guest ITS (identified by its doorbell address) and device ID.
> + * Also provide the number of events (MSIs) needed for that device.
> + * This does not check if this particular hardware device is already mapped
> + * at another domain, it is expected that this would be done by the caller.
> + */
> +int gicv3_its_map_guest_device(struct domain *d,
> +                               paddr_t host_doorbell, uint32_t host_devid,
> +                               paddr_t guest_doorbell, uint32_t guest_devid,
> +                               uint32_t nr_events, bool valid)
> +{
> +    void *itt_addr = NULL;
> +    struct host_its *hw_its;
> +    struct its_devices *dev = NULL;
> +    struct rb_node **new = &d->arch.vgic.its_devices.rb_node, *parent = NULL;
> +    int ret = -ENOENT;
> +
> +    hw_its = gicv3_its_find_by_doorbell(host_doorbell);
> +    if ( !hw_its )
> +        return ret;
> +
> +    /* check for already existing mappings */
> +    spin_lock(&d->arch.vgic.its_devices_lock);
> +    while ( *new )
> +    {
> +        struct its_devices *temp;
> +        int cmp;
> +
> +        temp = rb_entry(*new, struct its_devices, rbnode);
> +
> +        parent = *new;
> +        cmp = compare_its_guest_devices(temp, guest_doorbell, guest_devid);
> +        if ( !cmp )
> +        {
> +            if ( !valid )
> +                rb_erase(&temp->rbnode, &d->arch.vgic.its_devices);
> +
> +            spin_unlock(&d->arch.vgic.its_devices_lock);
> +
> +            if ( valid )
> +                return -EBUSY;
> +
> +            return remove_mapped_guest_device(temp);
> +        }
> +
> +        if ( cmp > 0 )
> +            new = &((*new)->rb_left);
> +        else
> +            new = &((*new)->rb_right);
> +    }
> +
> +    if ( !valid )
> +        goto out_unlock;
> +
> +    ret = -ENOMEM;
> +
> +    /* An Interrupt Translation Table needs to be 256-byte aligned. */
> +    itt_addr = _xzalloc(nr_events * hw_its->itte_size, 256);

      As I mentioned, in previous version, if itt_addr is not enough size,
ITS would overwrite and corrupt memory.
Similar to size passed in MAPD cmd, itt_addr should also be allocated of size
ROUNDUP(nr_events, LPI_BLOCK).

> +    if ( !itt_addr )
> +        goto out_unlock;
> +
> +    dev = xzalloc(struct its_devices);
> +    if ( !dev )
> +        goto out_unlock;
> +
> +    /*
> +     * Allocate the pending_irqs for each virtual LPI. They will be put
> +     * into the domain's radix tree upon the guest's MAPTI command.
> +     */
> +    dev->pend_irqs = xzalloc_array(struct pending_irq, nr_events);
> +    if ( !dev->pend_irqs )
> +        goto out_unlock;
> +
> +    ret = its_send_cmd_mapd(hw_its, host_devid,
> +                            fls(ROUNDUP(nr_events, LPI_BLOCK) - 1) - 1,
> +                            virt_to_maddr(itt_addr), true);
> +    if ( ret )
> +        goto out_unlock;
> +
> +    dev->itt_addr = itt_addr;
> +    dev->hw_its = hw_its;
> +    dev->guest_doorbell = guest_doorbell;
> +    dev->guest_devid = guest_devid;
> +    dev->host_devid = host_devid;
> +    dev->eventids = nr_events;
> +
> +    rb_link_node(&dev->rbnode, parent, new);
> +    rb_insert_color(&dev->rbnode, &d->arch.vgic.its_devices);
> +
> +    spin_unlock(&d->arch.vgic.its_devices_lock);
> +
> +    return 0;
> +
> +out_unlock:
> +    spin_unlock(&d->arch.vgic.its_devices_lock);
> +    if ( dev )
> +    {
> +        xfree(dev->pend_irqs);
> +        xfree(dev->host_lpi_blocks);
> +    }
> +    xfree(itt_addr);
> +    xfree(dev);
> +    return ret;
> +}
> +
> +/* Removing any connections a domain had to any ITS in the system. */
> +void gicv3_its_unmap_all_devices(struct domain *d)
> +{
> +    struct rb_node *victim;
> +    struct its_devices *dev;
> +
> +    /*
> +     * This is an easily readable, but suboptimal implementation.
> +     * It uses the provided iteration wrapper and erases each node, which
> +     * possibly triggers rebalancing.
> +     * This seems overkill since we are going to abolish the whole tree, but
> +     * avoids an open-coded re-implementation of the traversal functions with
> +     * some recursive function calls.
> +     */
> +restart:
> +    spin_lock(&d->arch.vgic.its_devices_lock);
> +    if ( (victim = rb_first(&d->arch.vgic.its_devices)) )
> +    {
> +        dev = rb_entry(victim, struct its_devices, rbnode);
> +        rb_erase(victim, &d->arch.vgic.its_devices);
> +
> +        spin_unlock(&d->arch.vgic.its_devices_lock);
> +
> +        remove_mapped_guest_device(dev);
> +
> +        goto restart;
> +    }
> +
> +    spin_unlock(&d->arch.vgic.its_devices_lock);
> +}
> +
>  /* Scan the DT for any ITS nodes and create a list of host ITSes out of it. */
>  void gicv3_its_dt_init(const struct dt_device_node *node)
>  {
> @@ -459,6 +685,7 @@ void gicv3_its_dt_init(const struct dt_device_node *node)
>          its_data->addr = addr;
>          its_data->size = size;
>          its_data->dt_node = its;
> +        spin_lock_init(&its_data->cmd_lock);
>
>          printk("GICv3: Found ITS @0x%lx\n", addr);
>
> diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
> index d61479d..6242252 100644
> --- a/xen/arch/arm/vgic-v3.c
> +++ b/xen/arch/arm/vgic-v3.c
> @@ -1450,6 +1450,9 @@ static int vgic_v3_domain_init(struct domain *d)
>      d->arch.vgic.nr_regions = rdist_count;
>      d->arch.vgic.rdist_regions = rdist_regions;
>
> +    spin_lock_init(&d->arch.vgic.its_devices_lock);
> +    d->arch.vgic.its_devices = RB_ROOT;
> +
>      /*
>       * Domain 0 gets the hardware address.
>       * Guests get the virtual platform layout.
> @@ -1522,6 +1525,7 @@ static int vgic_v3_domain_init(struct domain *d)
>
>  static void vgic_v3_domain_free(struct domain *d)
>  {
> +    gicv3_its_unmap_all_devices(d);
>      xfree(d->arch.vgic.rdist_regions);
>  }
>
> diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
> index 2d6fbb1..e559027 100644
> --- a/xen/include/asm-arm/domain.h
> +++ b/xen/include/asm-arm/domain.h
> @@ -11,6 +11,7 @@
>  #include <asm/gic.h>
>  #include <public/hvm/params.h>
>  #include <xen/serial.h>
> +#include <xen/rbtree.h>
>
>  struct hvm_domain
>  {
> @@ -109,6 +110,8 @@ struct arch_domain
>          } *rdist_regions;
>          int nr_regions;                     /* Number of rdist regions */
>          uint32_t rdist_stride;              /* Re-Distributor stride */
> +        struct rb_root its_devices;         /* Devices mapped to an ITS */
> +        spinlock_t its_devices_lock;        /* Protects the its_devices tree */
>  #endif
>      } vgic;
>
> diff --git a/xen/include/asm-arm/gic_v3_its.h b/xen/include/asm-arm/gic_v3_its.h
> index 4c2ae1c..4ade5f6 100644
> --- a/xen/include/asm-arm/gic_v3_its.h
> +++ b/xen/include/asm-arm/gic_v3_its.h
> @@ -48,6 +48,10 @@
>  #define GITS_TYPER_DEVICE_ID_BITS(r)    (((r & GITS_TYPER_DEVIDS_MASK) >> \
>                                                 GITS_TYPER_DEVIDS_SHIFT) + 1)
>  #define GITS_TYPER_IDBITS_SHIFT         8
> +#define GITS_TYPER_ITT_SIZE_SHIFT       4
> +#define GITS_TYPER_ITT_SIZE_MASK        (0xfUL << GITS_TYPER_ITT_SIZE_SHIFT)
> +#define GITS_TYPER_ITT_SIZE(r)          ((((r) & GITS_TYPER_ITT_SIZE_MASK) >> \
> +                                                GITS_TYPER_ITT_SIZE_SHIFT) + 1)
>
>  #define GITS_IIDR_VALUE                 0x34c
>
> @@ -94,7 +98,10 @@
>  #define GITS_CMD_MOVALL                 0x0e
>  #define GITS_CMD_DISCARD                0x0f
>
> +#define ITS_DOORBELL_OFFSET             0x10040
> +
>  #include <xen/device_tree.h>
> +#include <xen/rbtree.h>
>
>  #define HOST_ITS_FLUSH_CMD_QUEUE        (1U << 0)
>  #define HOST_ITS_USES_PTA               (1U << 1)
> @@ -109,6 +116,7 @@ struct host_its {
>      unsigned int devid_bits;
>      spinlock_t cmd_lock;
>      void *cmd_buf;
> +    unsigned int itte_size;
>      unsigned int flags;
>  };
>
> @@ -135,6 +143,17 @@ uint64_t gicv3_get_redist_address(unsigned int cpu, bool use_pta);
>  /* Map a collection for this host CPU to each host ITS. */
>  int gicv3_its_setup_collection(unsigned int cpu);
>
> +/*
> + * Map a device on the host by allocating an ITT on the host (ITS).
> + * "nr_event" specifies how many events (interrupts) this device will need.
> + * Setting "valid" to false deallocates the device.
> + */
> +int gicv3_its_map_guest_device(struct domain *d,
> +                               paddr_t host_doorbell, uint32_t host_devid,
> +                               paddr_t guest_doorbell, uint32_t guest_devid,
> +                               uint32_t nr_events, bool valid);
> +void gicv3_its_unmap_all_devices(struct domain *d);
> +
>  #else
>
>  static LIST_HEAD(host_its_list);
> @@ -173,6 +192,10 @@ static inline int gicv3_its_setup_collection(unsigned int cpu)
>      return 0;
>  }
>
> +static inline void gicv3_its_unmap_all_devices(struct domain *d)
> +{
> +}
> +
>  #endif /* CONFIG_HAS_ITS */
>
>  #endif
> --
> 2.9.0
>
Julien Grall April 3, 2017, 6:33 p.m. UTC | #3
On 01/04/17 09:01, Vijay Kilari wrote:
> Hi Andre,

Hi Vijay,

> On Fri, Mar 31, 2017 at 11:35 PM, Andre Przywara <andre.przywara@arm.com> wrote:
>> +    /* An Interrupt Translation Table needs to be 256-byte aligned. */
>> +    itt_addr = _xzalloc(nr_events * hw_its->itte_size, 256);
>
>       As I mentioned, in previous version, if itt_addr is not enough size,
> ITS would overwrite and corrupt memory.
> Similar to size passed in MAPD cmd, itt_addr should also be allocated of size
> ROUNDUP(nr_events, LPI_BLOCK).

ROUNDUP(nr_events, LPI_BLOCK) would still be wrong as the MAPD command 
works in term of bits. You have to round up to the next bit.

Cheers,
Julien Grall April 3, 2017, 6:56 p.m. UTC | #4
Hi Andre,

Mostly repeating my comments from the previous version.

On 31/03/17 19:05, Andre Przywara wrote:

[...]

> +static int its_send_cmd_mapd(struct host_its *its, uint32_t deviceid,
> +                             uint8_t size_bits, paddr_t itt_addr, bool valid)
> +{
> +    uint64_t cmd[4];
> +
> +    if ( valid )
> +    {
> +        ASSERT(size_bits < 32);

Again, it would be better if you do the check against the real number in 
hardware (i.e GITS_TYPER.ID_bits).

> +        ASSERT(!(itt_addr & ~GENMASK_ULL(51, 8)));
> +    }
> +    cmd[0] = GITS_CMD_MAPD | ((uint64_t)deviceid << 32);
> +    cmd[1] = size_bits;

I would have expected to see size_bits - 1 to accommodate all the 
helpers rather than relying on them.

[...]

> +static int remove_mapped_guest_device(struct its_devices *dev)
> +{
> +    int ret;
> +
> +    if ( dev->hw_its )
> +    {
> +        /* MAPD also discards all events with this device ID. */
> +        int ret = its_send_cmd_mapd(dev->hw_its, dev->host_devid, 0, 0, false);

You are re-defining ret. Why?

[...]

> +static struct host_its *gicv3_its_find_by_doorbell(paddr_t doorbell_address)
> +{
> +    struct host_its *hw_its;
> +
> +    list_for_each_entry(hw_its, &host_its_list, entry)
> +    {
> +        if ( hw_its->addr + ITS_DOORBELL_OFFSET == doorbell_address )

Again, why not storing the ITS address rather than the doorbell to avoid 
"+ ITS_DOORBELL_OFFSET" ?

[...]

> +/*
> + * Map a hardware device, identified by a certain host ITS and its device ID
> + * to domain d, a guest ITS (identified by its doorbell address) and device ID.
> + * Also provide the number of events (MSIs) needed for that device.
> + * This does not check if this particular hardware device is already mapped
> + * at another domain, it is expected that this would be done by the caller.
> + */
> +int gicv3_its_map_guest_device(struct domain *d,
> +                               paddr_t host_doorbell, uint32_t host_devid,
> +                               paddr_t guest_doorbell, uint32_t guest_devid,
> +                               uint32_t nr_events, bool valid)

I am sure I said it somewhere in this series, nr_events likely needs to 
be sanitized against the hardware value. Same for host_devid.

[...]

> +        parent = *new;
> +        cmp = compare_its_guest_devices(temp, guest_doorbell, guest_devid);
> +        if ( !cmp )
> +        {
> +            if ( !valid )
> +                rb_erase(&temp->rbnode, &d->arch.vgic.its_devices);
> +
> +            spin_unlock(&d->arch.vgic.its_devices_lock);
> +
> +            if ( valid )

Again, a printk(XENLOG_GUEST...) here would be useful to know which host 
DeviceID was associated to the guest DeviceID.

> +                return -EBUSY;
> +
> +            return remove_mapped_guest_device(temp);

Again, just above you removed the device from the RB-tree but this 
function may fail and never free the memory. This means that memory will 
be leaked leading to a potential denial of service.

> +        }
> +
> +        if ( cmp > 0 )
> +            new = &((*new)->rb_left);
> +        else
> +            new = &((*new)->rb_right);
> +    }
> +
> +    if ( !valid )
> +        goto out_unlock;
> +
> +    ret = -ENOMEM;
> +
> +    /* An Interrupt Translation Table needs to be 256-byte aligned. */
> +    itt_addr = _xzalloc(nr_events * hw_its->itte_size, 256);

See Vijay's comment. But why don't you round up nr_events at the 
beginning once for all rather than doing it in the middle?

[...]

> +out_unlock:
> +    spin_unlock(&d->arch.vgic.its_devices_lock);
> +    if ( dev )
> +    {
> +        xfree(dev->pend_irqs);
> +        xfree(dev->host_lpi_blocks);

Where is host_lpi_blocks allocated? Why is it freed here?

> +    }
> +    xfree(itt_addr);
> +    xfree(dev);
> +    return ret;
> +}
> +
> +/* Removing any connections a domain had to any ITS in the system. */
> +void gicv3_its_unmap_all_devices(struct domain *d)
> +{
> +    struct rb_node *victim;
> +    struct its_devices *dev;
> +
> +    /*
> +     * This is an easily readable, but suboptimal implementation.
> +     * It uses the provided iteration wrapper and erases each node, which
> +     * possibly triggers rebalancing.
> +     * This seems overkill since we are going to abolish the whole tree, but
> +     * avoids an open-coded re-implementation of the traversal functions with
> +     * some recursive function calls.
> +     */

Well, you updated the comment but it does not make the performance 
problem going away... Xen cannot be preempted, so if it takes too long, 
you will have an impact on the overall system.

As said previously, I think it would be fair to assume that all devices 
will be deassigned before the ITS is destroyed. So I would just drop 
this function. Not that we have the same assumption in the SMMU driver.

If you disagree please say why. But ignoring comments will not help here.

> +restart:
> +    spin_lock(&d->arch.vgic.its_devices_lock);
> +    if ( (victim = rb_first(&d->arch.vgic.its_devices)) )
> +    {
> +        dev = rb_entry(victim, struct its_devices, rbnode);
> +        rb_erase(victim, &d->arch.vgic.its_devices);
> +
> +        spin_unlock(&d->arch.vgic.its_devices_lock);
> +
> +        remove_mapped_guest_device(dev);
> +
> +        goto restart;
> +    }
> +
> +    spin_unlock(&d->arch.vgic.its_devices_lock);
> +}
> +
>  /* Scan the DT for any ITS nodes and create a list of host ITSes out of it. */
>  void gicv3_its_dt_init(const struct dt_device_node *node)
>  {
> @@ -459,6 +685,7 @@ void gicv3_its_dt_init(const struct dt_device_node *node)
>          its_data->addr = addr;
>          its_data->size = size;
>          its_data->dt_node = its;
> +        spin_lock_init(&its_data->cmd_lock);

This should be in patch #5.

>
>          printk("GICv3: Found ITS @0x%lx\n", addr);
>
> diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
> index d61479d..6242252 100644
> --- a/xen/arch/arm/vgic-v3.c
> +++ b/xen/arch/arm/vgic-v3.c
> @@ -1450,6 +1450,9 @@ static int vgic_v3_domain_init(struct domain *d)
>      d->arch.vgic.nr_regions = rdist_count;
>      d->arch.vgic.rdist_regions = rdist_regions;
>
> +    spin_lock_init(&d->arch.vgic.its_devices_lock);
> +    d->arch.vgic.its_devices = RB_ROOT;
> +

Again, the placement of those 2 lines are likely wrong. This should 
belong to the vITS and not the vgic-v3.

I think it would make sense to get a patch that introduces a skeleton 
for the vITS before this patch and start plumbing through.

>      /*
>       * Domain 0 gets the hardware address.
>       * Guests get the virtual platform layout.
> @@ -1522,6 +1525,7 @@ static int vgic_v3_domain_init(struct domain *d)
>
>  static void vgic_v3_domain_free(struct domain *d)
>  {
> +    gicv3_its_unmap_all_devices(d);

See my comment above regarding this function.

>      xfree(d->arch.vgic.rdist_regions);
>  }


[...]

> diff --git a/xen/include/asm-arm/gic_v3_its.h b/xen/include/asm-arm/gic_v3_its.h
> index 4c2ae1c..4ade5f6 100644
> --- a/xen/include/asm-arm/gic_v3_its.h
> +++ b/xen/include/asm-arm/gic_v3_its.h
> @@ -48,6 +48,10 @@
>  #define GITS_TYPER_DEVICE_ID_BITS(r)    (((r & GITS_TYPER_DEVIDS_MASK) >> \
>                                                 GITS_TYPER_DEVIDS_SHIFT) + 1)
>  #define GITS_TYPER_IDBITS_SHIFT         8
> +#define GITS_TYPER_ITT_SIZE_SHIFT       4
> +#define GITS_TYPER_ITT_SIZE_MASK        (0xfUL << GITS_TYPER_ITT_SIZE_SHIFT)
> +#define GITS_TYPER_ITT_SIZE(r)          ((((r) & GITS_TYPER_ITT_SIZE_MASK) >> \
> +                                                GITS_TYPER_ITT_SIZE_SHIFT) + 1)
>
>  #define GITS_IIDR_VALUE                 0x34c
>
> @@ -94,7 +98,10 @@
>  #define GITS_CMD_MOVALL                 0x0e
>  #define GITS_CMD_DISCARD                0x0f
>
> +#define ITS_DOORBELL_OFFSET             0x10040
> +
>  #include <xen/device_tree.h>
> +#include <xen/rbtree.h>
>
>  #define HOST_ITS_FLUSH_CMD_QUEUE        (1U << 0)
>  #define HOST_ITS_USES_PTA               (1U << 1)
> @@ -109,6 +116,7 @@ struct host_its {
>      unsigned int devid_bits;
>      spinlock_t cmd_lock;
>      void *cmd_buf;
> +    unsigned int itte_size;

Stefano mentioned:
"I would move itte_size and its initialization to the patch that
introduced struct host_its."

>      unsigned int flags;
>  };

Cheers,
diff mbox

Patch

diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
index 1ac598f..295f7dc 100644
--- a/xen/arch/arm/gic-v3-its.c
+++ b/xen/arch/arm/gic-v3-its.c
@@ -21,6 +21,8 @@ 
 #include <xen/lib.h>
 #include <xen/delay.h>
 #include <xen/mm.h>
+#include <xen/rbtree.h>
+#include <xen/sched.h>
 #include <xen/sizes.h>
 #include <asm/gic.h>
 #include <asm/gic_v3_defs.h>
@@ -32,6 +34,18 @@ 
 
 LIST_HEAD(host_its_list);
 
+struct its_devices {
+    struct rb_node rbnode;
+    struct host_its *hw_its;
+    void *itt_addr;
+    paddr_t guest_doorbell;             /* Identifies the virtual ITS */
+    uint32_t host_devid;
+    uint32_t guest_devid;
+    uint32_t eventids;                  /* Number of event IDs (MSIs) */
+    uint32_t *host_lpi_blocks;          /* Which LPIs are used on the host */
+    struct pending_irq *pend_irqs;      /* One struct per event */
+};
+
 bool gicv3_its_host_has_its(void)
 {
     return !list_empty(&host_its_list);
@@ -151,6 +165,26 @@  static int its_send_cmd_mapc(struct host_its *its, uint32_t collection_id,
     return its_send_command(its, cmd);
 }
 
+static int its_send_cmd_mapd(struct host_its *its, uint32_t deviceid,
+                             uint8_t size_bits, paddr_t itt_addr, bool valid)
+{
+    uint64_t cmd[4];
+
+    if ( valid )
+    {
+        ASSERT(size_bits < 32);
+        ASSERT(!(itt_addr & ~GENMASK_ULL(51, 8)));
+    }
+    cmd[0] = GITS_CMD_MAPD | ((uint64_t)deviceid << 32);
+    cmd[1] = size_bits;
+    cmd[2] = itt_addr;
+    if ( valid )
+        cmd[2] |= GITS_VALID_BIT;
+    cmd[3] = 0x00;
+
+    return its_send_command(its, cmd);
+}
+
 /* Set up the (1:1) collection mapping for the given host CPU. */
 int gicv3_its_setup_collection(unsigned int cpu)
 {
@@ -376,6 +410,7 @@  static int gicv3_its_init_single_its(struct host_its *hw_its)
     hw_its->devid_bits = min(hw_its->devid_bits, max_its_device_bits);
     if ( reg & GITS_TYPER_PTA )
         hw_its->flags |= HOST_ITS_USES_PTA;
+    hw_its->itte_size = GITS_TYPER_ITT_SIZE(reg);
 
     for ( i = 0; i < GITS_BASER_NR_REGS; i++ )
     {
@@ -432,6 +467,197 @@  int gicv3_its_init(void)
     return 0;
 }
 
+static int remove_mapped_guest_device(struct its_devices *dev)
+{
+    int ret;
+
+    if ( dev->hw_its )
+    {
+        /* MAPD also discards all events with this device ID. */
+        int ret = its_send_cmd_mapd(dev->hw_its, dev->host_devid, 0, 0, false);
+        if ( ret )
+            return ret;
+    }
+
+    ret = gicv3_its_wait_commands(dev->hw_its);
+    if ( ret )
+        return ret;
+
+    xfree(dev->itt_addr);
+    xfree(dev->pend_irqs);
+    xfree(dev);
+
+    return 0;
+}
+
+static struct host_its *gicv3_its_find_by_doorbell(paddr_t doorbell_address)
+{
+    struct host_its *hw_its;
+
+    list_for_each_entry(hw_its, &host_its_list, entry)
+    {
+        if ( hw_its->addr + ITS_DOORBELL_OFFSET == doorbell_address )
+            return hw_its;
+    }
+
+    return NULL;
+}
+
+static int compare_its_guest_devices(struct its_devices *dev,
+                                     paddr_t doorbell, uint32_t devid)
+{
+    if ( dev->guest_doorbell < doorbell )
+        return -1;
+
+    if ( dev->guest_doorbell > doorbell )
+        return 1;
+
+    if ( dev->guest_devid < devid )
+        return -1;
+
+    if ( dev->guest_devid > devid )
+        return 1;
+
+    return 0;
+}
+
+/*
+ * Map a hardware device, identified by a certain host ITS and its device ID
+ * to domain d, a guest ITS (identified by its doorbell address) and device ID.
+ * Also provide the number of events (MSIs) needed for that device.
+ * This does not check if this particular hardware device is already mapped
+ * at another domain, it is expected that this would be done by the caller.
+ */
+int gicv3_its_map_guest_device(struct domain *d,
+                               paddr_t host_doorbell, uint32_t host_devid,
+                               paddr_t guest_doorbell, uint32_t guest_devid,
+                               uint32_t nr_events, bool valid)
+{
+    void *itt_addr = NULL;
+    struct host_its *hw_its;
+    struct its_devices *dev = NULL;
+    struct rb_node **new = &d->arch.vgic.its_devices.rb_node, *parent = NULL;
+    int ret = -ENOENT;
+
+    hw_its = gicv3_its_find_by_doorbell(host_doorbell);
+    if ( !hw_its )
+        return ret;
+
+    /* check for already existing mappings */
+    spin_lock(&d->arch.vgic.its_devices_lock);
+    while ( *new )
+    {
+        struct its_devices *temp;
+        int cmp;
+
+        temp = rb_entry(*new, struct its_devices, rbnode);
+
+        parent = *new;
+        cmp = compare_its_guest_devices(temp, guest_doorbell, guest_devid);
+        if ( !cmp )
+        {
+            if ( !valid )
+                rb_erase(&temp->rbnode, &d->arch.vgic.its_devices);
+
+            spin_unlock(&d->arch.vgic.its_devices_lock);
+
+            if ( valid )
+                return -EBUSY;
+
+            return remove_mapped_guest_device(temp);
+        }
+
+        if ( cmp > 0 )
+            new = &((*new)->rb_left);
+        else
+            new = &((*new)->rb_right);
+    }
+
+    if ( !valid )
+        goto out_unlock;
+
+    ret = -ENOMEM;
+
+    /* An Interrupt Translation Table needs to be 256-byte aligned. */
+    itt_addr = _xzalloc(nr_events * hw_its->itte_size, 256);
+    if ( !itt_addr )
+        goto out_unlock;
+
+    dev = xzalloc(struct its_devices);
+    if ( !dev )
+        goto out_unlock;
+
+    /*
+     * Allocate the pending_irqs for each virtual LPI. They will be put
+     * into the domain's radix tree upon the guest's MAPTI command.
+     */
+    dev->pend_irqs = xzalloc_array(struct pending_irq, nr_events);
+    if ( !dev->pend_irqs )
+        goto out_unlock;
+
+    ret = its_send_cmd_mapd(hw_its, host_devid,
+                            fls(ROUNDUP(nr_events, LPI_BLOCK) - 1) - 1,
+                            virt_to_maddr(itt_addr), true);
+    if ( ret )
+        goto out_unlock;
+
+    dev->itt_addr = itt_addr;
+    dev->hw_its = hw_its;
+    dev->guest_doorbell = guest_doorbell;
+    dev->guest_devid = guest_devid;
+    dev->host_devid = host_devid;
+    dev->eventids = nr_events;
+
+    rb_link_node(&dev->rbnode, parent, new);
+    rb_insert_color(&dev->rbnode, &d->arch.vgic.its_devices);
+
+    spin_unlock(&d->arch.vgic.its_devices_lock);
+
+    return 0;
+
+out_unlock:
+    spin_unlock(&d->arch.vgic.its_devices_lock);
+    if ( dev )
+    {
+        xfree(dev->pend_irqs);
+        xfree(dev->host_lpi_blocks);
+    }
+    xfree(itt_addr);
+    xfree(dev);
+    return ret;
+}
+
+/* Removing any connections a domain had to any ITS in the system. */
+void gicv3_its_unmap_all_devices(struct domain *d)
+{
+    struct rb_node *victim;
+    struct its_devices *dev;
+
+    /*
+     * This is an easily readable, but suboptimal implementation.
+     * It uses the provided iteration wrapper and erases each node, which
+     * possibly triggers rebalancing.
+     * This seems overkill since we are going to abolish the whole tree, but
+     * avoids an open-coded re-implementation of the traversal functions with
+     * some recursive function calls.
+     */
+restart:
+    spin_lock(&d->arch.vgic.its_devices_lock);
+    if ( (victim = rb_first(&d->arch.vgic.its_devices)) )
+    {
+        dev = rb_entry(victim, struct its_devices, rbnode);
+        rb_erase(victim, &d->arch.vgic.its_devices);
+
+        spin_unlock(&d->arch.vgic.its_devices_lock);
+
+        remove_mapped_guest_device(dev);
+
+        goto restart;
+    }
+
+    spin_unlock(&d->arch.vgic.its_devices_lock);
+}
+
 /* Scan the DT for any ITS nodes and create a list of host ITSes out of it. */
 void gicv3_its_dt_init(const struct dt_device_node *node)
 {
@@ -459,6 +685,7 @@  void gicv3_its_dt_init(const struct dt_device_node *node)
         its_data->addr = addr;
         its_data->size = size;
         its_data->dt_node = its;
+        spin_lock_init(&its_data->cmd_lock);
 
         printk("GICv3: Found ITS @0x%lx\n", addr);
 
diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
index d61479d..6242252 100644
--- a/xen/arch/arm/vgic-v3.c
+++ b/xen/arch/arm/vgic-v3.c
@@ -1450,6 +1450,9 @@  static int vgic_v3_domain_init(struct domain *d)
     d->arch.vgic.nr_regions = rdist_count;
     d->arch.vgic.rdist_regions = rdist_regions;
 
+    spin_lock_init(&d->arch.vgic.its_devices_lock);
+    d->arch.vgic.its_devices = RB_ROOT;
+
     /*
      * Domain 0 gets the hardware address.
      * Guests get the virtual platform layout.
@@ -1522,6 +1525,7 @@  static int vgic_v3_domain_init(struct domain *d)
 
 static void vgic_v3_domain_free(struct domain *d)
 {
+    gicv3_its_unmap_all_devices(d);
     xfree(d->arch.vgic.rdist_regions);
 }
 
diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
index 2d6fbb1..e559027 100644
--- a/xen/include/asm-arm/domain.h
+++ b/xen/include/asm-arm/domain.h
@@ -11,6 +11,7 @@ 
 #include <asm/gic.h>
 #include <public/hvm/params.h>
 #include <xen/serial.h>
+#include <xen/rbtree.h>
 
 struct hvm_domain
 {
@@ -109,6 +110,8 @@  struct arch_domain
         } *rdist_regions;
         int nr_regions;                     /* Number of rdist regions */
         uint32_t rdist_stride;              /* Re-Distributor stride */
+        struct rb_root its_devices;         /* Devices mapped to an ITS */
+        spinlock_t its_devices_lock;        /* Protects the its_devices tree */
 #endif
     } vgic;
 
diff --git a/xen/include/asm-arm/gic_v3_its.h b/xen/include/asm-arm/gic_v3_its.h
index 4c2ae1c..4ade5f6 100644
--- a/xen/include/asm-arm/gic_v3_its.h
+++ b/xen/include/asm-arm/gic_v3_its.h
@@ -48,6 +48,10 @@ 
 #define GITS_TYPER_DEVICE_ID_BITS(r)    (((r & GITS_TYPER_DEVIDS_MASK) >> \
                                                GITS_TYPER_DEVIDS_SHIFT) + 1)
 #define GITS_TYPER_IDBITS_SHIFT         8
+#define GITS_TYPER_ITT_SIZE_SHIFT       4
+#define GITS_TYPER_ITT_SIZE_MASK        (0xfUL << GITS_TYPER_ITT_SIZE_SHIFT)
+#define GITS_TYPER_ITT_SIZE(r)          ((((r) & GITS_TYPER_ITT_SIZE_MASK) >> \
+                                                GITS_TYPER_ITT_SIZE_SHIFT) + 1)
 
 #define GITS_IIDR_VALUE                 0x34c
 
@@ -94,7 +98,10 @@ 
 #define GITS_CMD_MOVALL                 0x0e
 #define GITS_CMD_DISCARD                0x0f
 
+#define ITS_DOORBELL_OFFSET             0x10040
+
 #include <xen/device_tree.h>
+#include <xen/rbtree.h>
 
 #define HOST_ITS_FLUSH_CMD_QUEUE        (1U << 0)
 #define HOST_ITS_USES_PTA               (1U << 1)
@@ -109,6 +116,7 @@  struct host_its {
     unsigned int devid_bits;
     spinlock_t cmd_lock;
     void *cmd_buf;
+    unsigned int itte_size;
     unsigned int flags;
 };
 
@@ -135,6 +143,17 @@  uint64_t gicv3_get_redist_address(unsigned int cpu, bool use_pta);
 /* Map a collection for this host CPU to each host ITS. */
 int gicv3_its_setup_collection(unsigned int cpu);
 
+/*
+ * Map a device on the host by allocating an ITT on the host (ITS).
+ * "nr_event" specifies how many events (interrupts) this device will need.
+ * Setting "valid" to false deallocates the device.
+ */
+int gicv3_its_map_guest_device(struct domain *d,
+                               paddr_t host_doorbell, uint32_t host_devid,
+                               paddr_t guest_doorbell, uint32_t guest_devid,
+                               uint32_t nr_events, bool valid);
+void gicv3_its_unmap_all_devices(struct domain *d);
+
 #else
 
 static LIST_HEAD(host_its_list);
@@ -173,6 +192,10 @@  static inline int gicv3_its_setup_collection(unsigned int cpu)
     return 0;
 }
 
+static inline void gicv3_its_unmap_all_devices(struct domain *d)
+{
+}
+
 #endif /* CONFIG_HAS_ITS */
 
 #endif