[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [RFC PATCH 07/24] ARM: GICv3 ITS: introduce device mapping



On Wed, 28 Sep 2016, 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 a list to
> later be able to iterate over it or to easily teardown guests.
> 
> Signed-off-by: Andre Przywara <andre.przywara@xxxxxxx>
> ---
>  xen/arch/arm/gic-its.c        | 90 
> +++++++++++++++++++++++++++++++++++++++++++
>  xen/include/asm-arm/gic-its.h | 16 ++++++++
>  2 files changed, 106 insertions(+)
> 
> diff --git a/xen/arch/arm/gic-its.c b/xen/arch/arm/gic-its.c
> index 2140e4a..bf1f5b5 100644
> --- a/xen/arch/arm/gic-its.c
> +++ b/xen/arch/arm/gic-its.c
> @@ -168,6 +168,94 @@ static int its_send_cmd_mapc(struct host_its *its, int 
> collection_id, int cpu)
>      return its_send_command(its, cmd);
>  }
>  
> +static int its_send_cmd_mapd(struct host_its *its, uint32_t deviceid,
> +                             int size, uint64_t itt_addr, bool valid)
> +{
> +    uint64_t cmd[4];
> +
> +    cmd[0] = GITS_CMD_MAPD | ((uint64_t)deviceid << 32);
> +    cmd[1] = size & GENMASK(4, 0);
> +    cmd[2] = itt_addr & GENMASK(51, 8);
> +    if ( valid )
> +        cmd[2] |= BIT(63);
> +    cmd[3] = 0x00;
> +
> +    return its_send_command(its, cmd);
> +}
> +
> +int gicv3_its_map_device(struct host_its *hw_its, struct domain *d,
> +                         int devid, int bits, bool valid)
> +{
> +    void *itt_addr = NULL;
> +    struct its_devices *dev, *temp;
> +    bool reuse_dev = false;
> +
> +    list_for_each_entry_safe(dev, temp, &hw_its->its_devices, entry)
> +    {
> +        if ( (dev->d->domain_id != d->domain_id) || (dev->devid != devid) )
> +            continue;
> +
> +        its_send_cmd_mapd(hw_its, dev->devid, 0, 0, false);
> +        xfree(dev->itt_addr);
> +        if ( !valid )
> +        {
> +            xfree(dev);
> +            list_del(&dev->entry);
> +
> +            return 0;
> +        }
> +
> +        reuse_dev = true;
> +        break;
> +    }

I don't think we want to go through the whole list every time this
function is called. Devices can be thousands. I would split it in two:
one to retrieve existing mappings and another to allocate new ones. We
need to make sure we don't call the function to retrieve existing
mappings often.

We can also consider using a rbtree instead of a list, or if devids are
a dense numeric group, we could use an array and direct access.


> +    if ( !valid )
> +        return 0;
> +
> +    itt_addr = _xmalloc(BIT(bits) * hw_its->itte_size, 256);
> +    if ( !itt_addr )
> +        return -ENOMEM;
> +
> +    if ( !reuse_dev )
> +    {
> +        dev = xmalloc(struct its_devices);
> +        if ( !dev )
> +            return -ENOMEM;
> +
> +        list_add_tail(&dev->entry, &hw_its->its_devices);
> +    }
> +
> +    dev->itt_addr = itt_addr;
> +    dev->d = d;
> +    dev->devid = devid;
> +
> +    return its_send_cmd_mapd(hw_its, devid, bits - 1,
> +                             itt_addr ? virt_to_maddr(itt_addr) : 0, true);
> +}
> +
> +/* Removing any connections a domain had to any ITS in the system. */
> +int its_remove_domain(struct domain *d)
> +{
> +    struct host_its *its;
> +    struct its_devices *dev, *temp;
> +
> +    list_for_each_entry(its, &host_its_list, entry)
> +    {
> +        list_for_each_entry_safe(dev, temp, &its->its_devices, entry)
> +        {
> +            if ( dev->d->domain_id != d->domain_id )
> +                continue;
> +
> +            its_send_cmd_mapd(its, dev->devid, 0, 0, false);
> +            xfree(dev->itt_addr);
> +            xfree(dev);
> +            list_del(&dev->entry);
> +        }
> +    }

Again scanning the full list on every domain destruction is not good.
This is easy to work around, even without completely reworking the data
structures, because we could add a second per-domain list to link all
devices that belong to the same domain.


> +    return 0;
> +}
> +
>  /* Set up the (1:1) collection mapping for the given host CPU. */
>  void gicv3_its_setup_collection(int cpu)
>  {
> @@ -297,6 +385,7 @@ int gicv3_its_init(struct host_its *hw_its)
>  
>      reg = readq_relaxed(hw_its->its_base + GITS_TYPER);
>      hw_its->pta = reg & GITS_TYPER_PTA;
> +    hw_its->itte_size = ((reg >> 4) & 0xf) + 1;

Please add some #define


>      for (i = 0; i < 8; i++)
>      {
> @@ -520,6 +609,7 @@ void gicv3_its_dt_init(const struct dt_device_node *node)
>          its_data->size = size;
>          its_data->dt_node = its;
>          spin_lock_init(&its_data->cmd_lock);
> +        INIT_LIST_HEAD(&its_data->its_devices);
>  
>          printk("GICv3: Found ITS @0x%lx\n", addr);
>  
> diff --git a/xen/include/asm-arm/gic-its.h b/xen/include/asm-arm/gic-its.h
> index 512a388..4e9841a 100644
> --- a/xen/include/asm-arm/gic-its.h
> +++ b/xen/include/asm-arm/gic-its.h
> @@ -79,6 +79,13 @@
>  #ifndef __ASSEMBLY__
>  #include <xen/device_tree.h>
>  
> +struct its_devices {
> +    struct list_head entry;
> +    struct domain *d;
> +    void *itt_addr;
> +    int devid;
> +};
> +
>  /* data structure for each hardware ITS */
>  struct host_its {
>      struct list_head entry;
> @@ -88,6 +95,8 @@ struct host_its {
>      void __iomem *its_base;
>      spinlock_t cmd_lock;
>      void *cmd_buf;
> +    struct list_head its_devices;
> +    int itte_size;
>      bool pta;
>  };
>  
> @@ -114,6 +123,13 @@ void gicv3_set_redist_addr(paddr_t address, int 
> redist_id);
>  /* Map a collection for this host CPU to each host ITS. */
>  void gicv3_its_setup_collection(int cpu);
>  
> +/* Map a device on the host by allocating an ITT on the host (ITS).
> + * "bits" specifies how many events (interrupts) this device will need.
> + * Setting "valid" to false deallocates the device.
> + */
> +int gicv3_its_map_device(struct host_its *hw_its, struct domain *d,
> +                         int devid, int bits, bool valid);
> +
>  int gicv3_lpi_allocate_host_lpi(struct host_its *its,
>                                  uint32_t devid, uint32_t eventid,
>                                  struct vcpu *v, int virt_lpi);
> -- 
> 2.9.0
> 

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.