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

[Andre Przywara <andre.przywara@arm.com>]

Hi,

On 24/10/16 16:31, Vijay Kilari wrote:
> On Wed, Sep 28, 2016 at 11:54 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 a list 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-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);
>     xfree() should be done after list_del()

Oh, indeed.

>> +            list_del(&dev->entry);
>> +
>> +            return 0;
>> +        }
>> +
>> +        reuse_dev = true;
>> +        break;
>> +    }
>> +
>> +    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);
>           check on itt_addr is redundant

Ah, yes, this is an artifact of an earlier version. Thanks for spotting
this.

> 
>> +}
>> +
>> +/* 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);
> 
> xfree() should be done after list_del(
>> +            list_del(&dev->entry);
>> +        }
>         This code is same as above. Can be moved to a separate function?

Probably. Will take a look.

Thanks,
Andre.

>> +    }
>> +
>> +    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;
>       can define a macro for these constants
>>
>>      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@lists.xen.org
>> https://lists.xen.org/xen-devel
> 

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