diff mbox

[v2,03/27] ARM: GICv3 ITS: allocate device and collection table

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

Commit Message

Andre Przywara March 16, 2017, 11:20 a.m. UTC
Each ITS maps a pair of a DeviceID (for instance derived from a PCI
b/d/f triplet) and an EventID (the MSI payload or interrupt ID) to a
pair of LPI number and collection ID, which points to the target CPU.
This mapping is stored in the device and collection tables, which software
has to provide for the ITS to use.
Allocate the required memory and hand it to the ITS.
The maximum number of devices is limited to a compile-time constant
exposed in Kconfig.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 docs/misc/xen-command-line.markdown |   8 ++
 xen/arch/arm/Kconfig                |  14 ++++
 xen/arch/arm/gic-v3-its.c           | 163 ++++++++++++++++++++++++++++++++++++
 xen/arch/arm/gic-v3.c               |   3 +
 xen/include/asm-arm/gic_v3_its.h    |  63 +++++++++++++-
 5 files changed, 250 insertions(+), 1 deletion(-)

Comments

Stefano Stabellini March 21, 2017, 11:29 p.m. UTC | #1
On Thu, 16 Mar 2017, Andre Przywara wrote:
> Each ITS maps a pair of a DeviceID (for instance derived from a PCI
> b/d/f triplet) and an EventID (the MSI payload or interrupt ID) to a
> pair of LPI number and collection ID, which points to the target CPU.
> This mapping is stored in the device and collection tables, which software
> has to provide for the ITS to use.
> Allocate the required memory and hand it to the ITS.
> The maximum number of devices is limited to a compile-time constant
> exposed in Kconfig.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  docs/misc/xen-command-line.markdown |   8 ++
>  xen/arch/arm/Kconfig                |  14 ++++
>  xen/arch/arm/gic-v3-its.c           | 163 ++++++++++++++++++++++++++++++++++++
>  xen/arch/arm/gic-v3.c               |   3 +
>  xen/include/asm-arm/gic_v3_its.h    |  63 +++++++++++++-
>  5 files changed, 250 insertions(+), 1 deletion(-)
> 
> diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown
> index 619016d..068d116 100644
> --- a/docs/misc/xen-command-line.markdown
> +++ b/docs/misc/xen-command-line.markdown
> @@ -1158,6 +1158,14 @@ based interrupts. Any higher IRQs will be available for use via PCI MSI.
>  ### maxcpus
>  > `= <integer>`
>  
> +### max\_its\_device\_bits
> +> `= <integer>`
> +
> +Specifies the maximum number of devices using MSIs on the ARM GICv3 ITS
> +controller to allocate table entries for. Each table entry uses a hardware
> +specific size, typically 8 or 16 bytes.
> +Defaults to 10 bits (to cover at most 1024 devices).
> +
>  ### max\_lpi\_bits
>  > `= <integer>`
>  
> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> index 86f7b53..0d50156 100644
> --- a/xen/arch/arm/Kconfig
> +++ b/xen/arch/arm/Kconfig
> @@ -64,6 +64,20 @@ config MAX_PHYS_LPI_BITS
>            This can be overridden on the command line with the max_lpi_bits
>            parameter.
>  
> +config MAX_PHYS_ITS_DEVICE_BITS
> +        depends on HAS_ITS
> +        int "Number of device bits the ITS supports"
> +        range 1 32
> +        default "10"
> +        help
> +          Specifies the maximum number of devices which want to use the ITS.
> +          Xen needs to allocates memory for the whole range very early.
> +          The allocation scheme may be sparse, so a much larger number must
> +          be supported to cover devices with a high bus number or those on
> +          separate bus segments.
> +          This can be overridden on the command line with the
> +          max_its_device_bits parameter.
> +
>  endmenu
>  
>  menu "ARM errata workaround via the alternative framework"
> diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
> index 4056e5b..9982fe9 100644
> --- a/xen/arch/arm/gic-v3-its.c
> +++ b/xen/arch/arm/gic-v3-its.c
> @@ -19,8 +19,10 @@
>   */
>  
>  #include <xen/lib.h>
> +#include <xen/mm.h>
>  #include <asm/gic_v3_defs.h>
>  #include <asm/gic_v3_its.h>
> +#include <asm/io.h>
>  
>  LIST_HEAD(host_its_list);
>  
> @@ -29,6 +31,167 @@ bool gicv3_its_host_has_its(void)
>      return !list_empty(&host_its_list);
>  }
>  
> +#define BASER_ATTR_MASK                                           \
> +        ((0x3UL << GITS_BASER_SHAREABILITY_SHIFT)               | \
> +         (0x7UL << GITS_BASER_OUTER_CACHEABILITY_SHIFT)         | \
> +         (0x7UL << GITS_BASER_INNER_CACHEABILITY_SHIFT))
> +#define BASER_RO_MASK   (GENMASK(58, 56) | GENMASK(52, 48))
> +
> +/* Check that the physical address can be encoded in the PROPBASER register. */
> +static bool check_propbaser_phys_addr(void *vaddr, unsigned int page_bits)
> +{
> +    paddr_t paddr = virt_to_maddr(vaddr);
> +
> +    return (!(paddr & ~GENMASK(page_bits < 16 ? 47 : 51, page_bits)));
> +}
> +
> +static uint64_t encode_propbaser_phys_addr(paddr_t addr, unsigned int page_bits)
> +{
> +    uint64_t ret = addr & GENMASK(47, page_bits);
> +
> +    if ( page_bits < 16 )
> +        return ret;
> +
> +    /* For 64K pages address bits 51-48 are encoded in bits 15-12. */
> +    return ret | ((addr & GENMASK(51, 48)) >> (48 - 12));
> +}
> +
> +/* The ITS BASE registers work with page sizes of 4K, 16K or 64K. */
> +#define BASER_PAGE_BITS(sz) ((sz) * 2 + 12)
> +
> +static int its_map_baser(void __iomem *basereg, uint64_t regc,
> +                         unsigned int nr_items)
> +{
> +    uint64_t attr, reg;
> +    unsigned int entry_size = GITS_BASER_ENTRY_SIZE(regc);
> +    unsigned int pagesz = 2, order, table_size;
> +    void *buffer;
> +
> +    attr  = GIC_BASER_InnerShareable << GITS_BASER_SHAREABILITY_SHIFT;
> +    attr |= GIC_BASER_CACHE_SameAsInner << GITS_BASER_OUTER_CACHEABILITY_SHIFT;
> +    attr |= GIC_BASER_CACHE_RaWaWb << GITS_BASER_INNER_CACHEABILITY_SHIFT;
> +
> +    /*
> +     * Setup the BASE register with the attributes that we like. Then read
> +     * it back and see what sticks (page size, cacheability and shareability
> +     * attributes), retrying if necessary.
> +     */
> +retry:
> +    table_size = ROUNDUP(nr_items * entry_size, BIT(BASER_PAGE_BITS(pagesz)));
> +    /* The BASE registers support at most 256 pages. */
> +    table_size = min(table_size, 256U << BASER_PAGE_BITS(pagesz));
> +    /* The memory block must be aligned to the requested page size. */
> +    order = max(get_order_from_bytes(table_size), pagesz * 2);
> +
> +    buffer = alloc_xenheap_pages(order, 0);
> +    if ( !buffer )
> +        return -ENOMEM;
> +
> +    if ( !check_propbaser_phys_addr(buffer, BASER_PAGE_BITS(pagesz)) )
> +    {
> +        free_xenheap_pages(buffer, 0);
> +        return -ERANGE;
> +    }
> +    memset(buffer, 0, table_size);
> +
> +    reg  = attr;
> +    reg |= (pagesz << GITS_BASER_PAGE_SIZE_SHIFT);
> +    reg |= (table_size >> BASER_PAGE_BITS(pagesz)) - 1;
> +    reg |= regc & BASER_RO_MASK;
> +    reg |= GITS_VALID_BIT;
> +    reg |= encode_propbaser_phys_addr(virt_to_maddr(buffer),
> +                                      BASER_PAGE_BITS(pagesz));
> +
> +    writeq_relaxed(reg, basereg);
> +    regc = readq_relaxed(basereg);
> +
> +    /* The host didn't like our attributes, just use what it returned. */
> +    if ( (regc & BASER_ATTR_MASK) != attr )
> +    {
> +        /* If we can't map it shareable, drop cacheability as well. */
> +        if ( (regc & GITS_BASER_SHAREABILITY_MASK) == GIC_BASER_NonShareable )
> +        {
> +            regc &= ~GITS_BASER_INNER_CACHEABILITY_MASK;
> +            writeq_relaxed(regc, basereg);
> +        }
> +        attr = regc & BASER_ATTR_MASK;
> +    }
> +    if ( (regc & GITS_BASER_INNER_CACHEABILITY_MASK) <= GIC_BASER_CACHE_nC )
> +        clean_and_invalidate_dcache_va_range(buffer, table_size);
> +
> +    /* If the host accepted our page size, we are done. */
> +    if ( ((regc >> GITS_BASER_PAGE_SIZE_SHIFT) & 0x3UL) == pagesz )
> +        return 0;
> +
> +    free_xenheap_pages(buffer, order);
> +
> +    if ( pagesz-- > 0 )
> +        goto retry;
> +
> +    /* None of the page sizes was accepted, give up */
> +    return -EINVAL;
> +}
> +
> +static unsigned int max_its_device_bits = CONFIG_MAX_PHYS_ITS_DEVICE_BITS;
> +integer_param("max_its_device_bits", max_its_device_bits);
> +
> +static int gicv3_its_init_single_its(struct host_its *hw_its)
> +{
> +    uint64_t reg;
> +    int i;
> +    unsigned int devid_bits;
> +
> +    hw_its->its_base = ioremap_nocache(hw_its->addr, hw_its->size);
> +    if ( !hw_its->its_base )
> +        return -ENOMEM;
> +
> +    reg = readq_relaxed(hw_its->its_base + GITS_TYPER);
> +    devid_bits = GITS_TYPER_DEVICE_ID_BITS(reg);
> +    devid_bits = min(devid_bits, max_its_device_bits);
> +
> +    for ( i = 0; i < GITS_BASER_NR_REGS; i++ )
> +    {
> +        void __iomem *basereg = hw_its->its_base + GITS_BASER0 + i * 8;
> +        unsigned int type;
> +
> +        reg = readq_relaxed(basereg);
> +        type = (reg & GITS_BASER_TYPE_MASK) >> GITS_BASER_TYPE_SHIFT;
> +        switch ( type )
> +        {
> +        case GITS_BASER_TYPE_NONE:
> +            continue;
> +        case GITS_BASER_TYPE_DEVICE:
> +            its_map_baser(basereg, reg, BIT(devid_bits));

Check return value of its_map_baser?


> +            break;

> +        case GITS_BASER_TYPE_COLLECTION:
> +            its_map_baser(basereg, reg, NR_CPUS);

We should use num_possible_cpus() instead of NR_CPUS here


> +            break;
> +        /* In case this is a GICv4, provide a (dummy) vPE table as well. */
> +        case GITS_BASER_TYPE_VCPU:
> +            its_map_baser(basereg, reg, 1);
> +            break;
> +        default:
> +            continue;
> +        }
> +    }
> +
> +    return 0;
> +}
> +
> +int gicv3_its_init(void)
> +{
> +    struct host_its *hw_its;
> +    int ret;
> +
> +    list_for_each_entry(hw_its, &host_its_list, entry) {
> +        ret = gicv3_its_init_single_its(hw_its);
> +        if ( ret )
> +            return ret;
> +    }
> +
> +    return 0;
> +}
> +
>  /* 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)
>  {
> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
> index ed78363..cc1e219 100644
> --- a/xen/arch/arm/gic-v3.c
> +++ b/xen/arch/arm/gic-v3.c
> @@ -1590,6 +1590,9 @@ static int __init gicv3_init(void)
>      spin_lock(&gicv3.lock);
>  
>      gicv3_dist_init();
> +    res = gicv3_its_init();
> +    if ( res )
> +        printk(XENLOG_WARNING "GICv3: ITS: initialization failed: %d\n", res);
>      res = gicv3_cpu_init();
>      gicv3_hyp_init();
>  
> diff --git a/xen/include/asm-arm/gic_v3_its.h b/xen/include/asm-arm/gic_v3_its.h
> index 219d109..a6c0acc 100644
> --- a/xen/include/asm-arm/gic_v3_its.h
> +++ b/xen/include/asm-arm/gic_v3_its.h
> @@ -20,6 +20,60 @@
>  #ifndef __ASM_ARM_ITS_H__
>  #define __ASM_ARM_ITS_H__
>  
> +#define GITS_CTLR                       0x000
> +#define GITS_IIDR                       0x004
> +#define GITS_TYPER                      0x008
> +#define GITS_CBASER                     0x080
> +#define GITS_CWRITER                    0x088
> +#define GITS_CREADR                     0x090
> +#define GITS_BASER_NR_REGS              8
> +#define GITS_BASER0                     0x100
> +#define GITS_BASER1                     0x108
> +#define GITS_BASER2                     0x110
> +#define GITS_BASER3                     0x118
> +#define GITS_BASER4                     0x120
> +#define GITS_BASER5                     0x128
> +#define GITS_BASER6                     0x130
> +#define GITS_BASER7                     0x138
> +
> +/* Register bits */
> +#define GITS_VALID_BIT                  BIT_ULL(63)
> +
> +#define GITS_CTLR_QUIESCENT             BIT(31)
> +#define GITS_CTLR_ENABLE                BIT(0)
> +
> +#define GITS_TYPER_DEVIDS_SHIFT         13
> +#define GITS_TYPER_DEVIDS_MASK          (0x1fUL << GITS_TYPER_DEVIDS_SHIFT)
> +#define GITS_TYPER_DEVICE_ID_BITS(r)    (((r & GITS_TYPER_DEVIDS_MASK) >> \
> +                                               GITS_TYPER_DEVIDS_SHIFT) + 1)
> +
> +#define GITS_IIDR_VALUE                 0x34c
> +
> +#define GITS_BASER_INDIRECT             BIT_ULL(62)
> +#define GITS_BASER_INNER_CACHEABILITY_SHIFT        59
> +#define GITS_BASER_TYPE_SHIFT           56
> +#define GITS_BASER_TYPE_MASK            (7ULL << GITS_BASER_TYPE_SHIFT)
> +#define GITS_BASER_OUTER_CACHEABILITY_SHIFT        53
> +#define GITS_BASER_TYPE_NONE            0UL
> +#define GITS_BASER_TYPE_DEVICE          1UL
> +#define GITS_BASER_TYPE_VCPU            2UL
> +#define GITS_BASER_TYPE_CPU             3UL
> +#define GITS_BASER_TYPE_COLLECTION      4UL
> +#define GITS_BASER_TYPE_RESERVED5       5UL
> +#define GITS_BASER_TYPE_RESERVED6       6UL
> +#define GITS_BASER_TYPE_RESERVED7       7UL
> +#define GITS_BASER_ENTRY_SIZE_SHIFT     48
> +#define GITS_BASER_ENTRY_SIZE(reg)                                       \
> +                        (((reg >> GITS_BASER_ENTRY_SIZE_SHIFT) & 0x1f) + 1)
> +#define GITS_BASER_SHAREABILITY_SHIFT   10
> +#define GITS_BASER_PAGE_SIZE_SHIFT      8
> +#define GITS_BASER_RO_MASK              (GITS_BASER_TYPE_MASK | \
> +                                        (31UL << GITS_BASER_ENTRY_SIZE_SHIFT) |\
> +                                        GITS_BASER_INDIRECT)
> +#define GITS_BASER_SHAREABILITY_MASK   (0x3ULL << GITS_BASER_SHAREABILITY_SHIFT)
> +#define GITS_BASER_OUTER_CACHEABILITY_MASK   (0x7ULL << GITS_BASER_OUTER_CACHEABILITY_SHIFT)
> +#define GITS_BASER_INNER_CACHEABILITY_MASK   (0x7ULL << GITS_BASER_INNER_CACHEABILITY_SHIFT)
> +
>  #include <xen/device_tree.h>
>  
>  /* data structure for each hardware ITS */
> @@ -28,6 +82,7 @@ struct host_its {
>      const struct dt_device_node *dt_node;
>      paddr_t addr;
>      paddr_t size;
> +    void __iomem *its_base;
>  };
>  
>  
> @@ -42,8 +97,9 @@ bool gicv3_its_host_has_its(void);
>  
>  int gicv3_lpi_init_rdist(void __iomem * rdist_base);
>  
> -/* Initialize the host structures for LPIs. */
> +/* Initialize the host structures for LPIs and the host ITSes. */
>  int gicv3_lpi_init_host_lpis(unsigned int nr_lpis);
> +int gicv3_its_init(void);
>  
>  #else
>  
> @@ -67,6 +123,11 @@ static inline int gicv3_lpi_init_host_lpis(unsigned int nr_lpis)
>  {
>      return 0;
>  }
> +
> +static inline int gicv3_its_init(void)
> +{
> +    return 0;
> +}
>  #endif /* CONFIG_HAS_ITS */
>  
>  #endif
> -- 
> 2.9.0
>
Julien Grall March 22, 2017, 1:52 p.m. UTC | #2
Hi Andre,

On 03/16/2017 11:20 AM, Andre Przywara wrote:
> Each ITS maps a pair of a DeviceID (for instance derived from a PCI
> b/d/f triplet) and an EventID (the MSI payload or interrupt ID) to a
> pair of LPI number and collection ID, which points to the target CPU.
> This mapping is stored in the device and collection tables, which software
> has to provide for the ITS to use.
> Allocate the required memory and hand it to the ITS.
> The maximum number of devices is limited to a compile-time constant
> exposed in Kconfig.
>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  docs/misc/xen-command-line.markdown |   8 ++
>  xen/arch/arm/Kconfig                |  14 ++++
>  xen/arch/arm/gic-v3-its.c           | 163 ++++++++++++++++++++++++++++++++++++
>  xen/arch/arm/gic-v3.c               |   3 +
>  xen/include/asm-arm/gic_v3_its.h    |  63 +++++++++++++-
>  5 files changed, 250 insertions(+), 1 deletion(-)
>
> diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown
> index 619016d..068d116 100644
> --- a/docs/misc/xen-command-line.markdown
> +++ b/docs/misc/xen-command-line.markdown
> @@ -1158,6 +1158,14 @@ based interrupts. Any higher IRQs will be available for use via PCI MSI.
>  ### maxcpus
>  > `= <integer>`
>
> +### max\_its\_device\_bits
> +> `= <integer>`
> +
> +Specifies the maximum number of devices using MSIs on the ARM GICv3 ITS
> +controller to allocate table entries for. Each table entry uses a hardware
> +specific size, typically 8 or 16 bytes.
> +Defaults to 10 bits (to cover at most 1024 devices).

The description is misleading. Even if the platform has less than 1024
devices, 10 may not be enough if the Device ID are not contiguous.

However, I don't think this is useful. A user may not know the DevIDs in
use of the platform and hence will not be able to choose a sensible value.

I still think that we should allocate what the hardware asked us and if
it is necessary to limit this should be done per-platform.

> +
>  ### max\_lpi\_bits
>  > `= <integer>`
>
> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> index 86f7b53..0d50156 100644
> --- a/xen/arch/arm/Kconfig
> +++ b/xen/arch/arm/Kconfig
> @@ -64,6 +64,20 @@ config MAX_PHYS_LPI_BITS
>            This can be overridden on the command line with the max_lpi_bits
>            parameter.
>
> +config MAX_PHYS_ITS_DEVICE_BITS
> +        depends on HAS_ITS
> +        int "Number of device bits the ITS supports"
> +        range 1 32
> +        default "10"
> +        help
> +          Specifies the maximum number of devices which want to use the ITS.
> +          Xen needs to allocates memory for the whole range very early.
> +          The allocation scheme may be sparse, so a much larger number must
> +          be supported to cover devices with a high bus number or those on
> +          separate bus segments.
> +          This can be overridden on the command line with the
> +          max_its_device_bits parameter.

 From what I understand the default you suggested fit neither Cavium nor
Qualcomm platform. It is also hard to see how a Distribution will be
able to choose a sensible value here.

> +
>  endmenu
>
>  menu "ARM errata workaround via the alternative framework"
> diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
> index 4056e5b..9982fe9 100644
> --- a/xen/arch/arm/gic-v3-its.c
> +++ b/xen/arch/arm/gic-v3-its.c

[...]

> +static int its_map_baser(void __iomem *basereg, uint64_t regc,
> +                         unsigned int nr_items)
> +{
> +    uint64_t attr, reg;
> +    unsigned int entry_size = GITS_BASER_ENTRY_SIZE(regc);
> +    unsigned int pagesz = 2, order, table_size;

Please document what mean 2.

> +    void *buffer;
> +
> +    attr  = GIC_BASER_InnerShareable << GITS_BASER_SHAREABILITY_SHIFT;
> +    attr |= GIC_BASER_CACHE_SameAsInner << GITS_BASER_OUTER_CACHEABILITY_SHIFT;
> +    attr |= GIC_BASER_CACHE_RaWaWb << GITS_BASER_INNER_CACHEABILITY_SHIFT;
> +
> +    /*
> +     * Setup the BASE register with the attributes that we like. Then read
> +     * it back and see what sticks (page size, cacheability and shareability
> +     * attributes), retrying if necessary.
> +     */
> +retry:
> +    table_size = ROUNDUP(nr_items * entry_size, BIT(BASER_PAGE_BITS(pagesz)));
> +    /* The BASE registers support at most 256 pages. */
> +    table_size = min(table_size, 256U << BASER_PAGE_BITS(pagesz));
> +    /* The memory block must be aligned to the requested page size. */
> +    order = max(get_order_from_bytes(table_size), pagesz * 2);
> +
> +    buffer = alloc_xenheap_pages(order, 0);

Why don't you use _zalloc(...)? This would avoid to compute the order
and drop few lines.

> +    if ( !buffer )
> +        return -ENOMEM;
> +
> +    if ( !check_propbaser_phys_addr(buffer, BASER_PAGE_BITS(pagesz)) )

The name of the function does not make sense. You check an address for
the register BASER and not PROPBASER.

> +    {
> +        free_xenheap_pages(buffer, 0);
> +        return -ERANGE;
> +    }
> +    memset(buffer, 0, table_size);

[...]

> +int gicv3_its_init(void)
> +{
> +    struct host_its *hw_its;
> +    int ret;
> +
> +    list_for_each_entry(hw_its, &host_its_list, entry) {

Coding style:

list_for_each_entry(....)
{

> +        ret = gicv3_its_init_single_its(hw_its);
> +        if ( ret )
> +            return ret;
> +    }
> +
> +    return 0;
> +}
> +
>  /* 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)
>  {
> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
> index ed78363..cc1e219 100644
> --- a/xen/arch/arm/gic-v3.c
> +++ b/xen/arch/arm/gic-v3.c
> @@ -1590,6 +1590,9 @@ static int __init gicv3_init(void)
>      spin_lock(&gicv3.lock);
>
>      gicv3_dist_init();
> +    res = gicv3_its_init();
> +    if ( res )
> +        printk(XENLOG_WARNING "GICv3: ITS: initialization failed: %d\n", res);

I would have expect a panic here because the ITS subsystem could be half
initialized and it is not safe to continue.

>      res = gicv3_cpu_init();
>      gicv3_hyp_init();
>

Cheers,

--
Julien Grall
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
Andre Przywara March 22, 2017, 4:08 p.m. UTC | #3
Hi,

On 22/03/17 13:52, Julien Grall wrote:
> Hi Andre,
> 
> On 03/16/2017 11:20 AM, Andre Przywara wrote:
>> Each ITS maps a pair of a DeviceID (for instance derived from a PCI
>> b/d/f triplet) and an EventID (the MSI payload or interrupt ID) to a
>> pair of LPI number and collection ID, which points to the target CPU.
>> This mapping is stored in the device and collection tables, which
>> software
>> has to provide for the ITS to use.
>> Allocate the required memory and hand it to the ITS.
>> The maximum number of devices is limited to a compile-time constant
>> exposed in Kconfig.
>>
>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>> ---
>>  docs/misc/xen-command-line.markdown |   8 ++
>>  xen/arch/arm/Kconfig                |  14 ++++
>>  xen/arch/arm/gic-v3-its.c           | 163
>> ++++++++++++++++++++++++++++++++++++
>>  xen/arch/arm/gic-v3.c               |   3 +
>>  xen/include/asm-arm/gic_v3_its.h    |  63 +++++++++++++-
>>  5 files changed, 250 insertions(+), 1 deletion(-)
>>
>> diff --git a/docs/misc/xen-command-line.markdown
>> b/docs/misc/xen-command-line.markdown
>> index 619016d..068d116 100644
>> --- a/docs/misc/xen-command-line.markdown
>> +++ b/docs/misc/xen-command-line.markdown
>> @@ -1158,6 +1158,14 @@ based interrupts. Any higher IRQs will be
>> available for use via PCI MSI.
>>  ### maxcpus
>>  > `= <integer>`
>>
>> +### max\_its\_device\_bits
>> +> `= <integer>`
>> +
>> +Specifies the maximum number of devices using MSIs on the ARM GICv3 ITS
>> +controller to allocate table entries for. Each table entry uses a
>> hardware
>> +specific size, typically 8 or 16 bytes.
>> +Defaults to 10 bits (to cover at most 1024 devices).
> 
> The description is misleading. Even if the platform has less than 1024
> devices, 10 may not be enough if the Device ID are not contiguous.

Right.

> However, I don't think this is useful. A user may not know the DevIDs in
> use of the platform and hence will not be able to choose a sensible value.
> 
> I still think that we should allocate what the hardware asked us and if
> it is necessary to limit this should be done per-platform.

I think you are right, a configurable limit does not make much sense. I
was wondering if we should have a hard coded *memory* limit instead, to
prevent ridiculously high allocations, say entry_size=8 and 32 bits of
device IDs, which would result in 32GB of memory for a flat device table.

Or if we don't want to arbitrarily limit this, we always print the
actual allocated size, maybe with a extra WARNING if it exceeds sane levels.

>> +
>>  ### max\_lpi\_bits
>>  > `= <integer>`
>>
>> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
>> index 86f7b53..0d50156 100644
>> --- a/xen/arch/arm/Kconfig
>> +++ b/xen/arch/arm/Kconfig
>> @@ -64,6 +64,20 @@ config MAX_PHYS_LPI_BITS
>>            This can be overridden on the command line with the
>> max_lpi_bits
>>            parameter.
>>
>> +config MAX_PHYS_ITS_DEVICE_BITS
>> +        depends on HAS_ITS
>> +        int "Number of device bits the ITS supports"
>> +        range 1 32
>> +        default "10"
>> +        help
>> +          Specifies the maximum number of devices which want to use
>> the ITS.
>> +          Xen needs to allocates memory for the whole range very early.
>> +          The allocation scheme may be sparse, so a much larger
>> number must
>> +          be supported to cover devices with a high bus number or
>> those on
>> +          separate bus segments.
>> +          This can be overridden on the command line with the
>> +          max_its_device_bits parameter.
> 
> From what I understand the default you suggested fit neither Cavium nor
> Qualcomm platform. It is also hard to see how a Distribution will be
> able to choose a sensible value here.

But it works for me (TM) ;-)
I was hoping that people would scream and suggest usable values instead
(because I don't know their limits). But indeed lets just remove that in
favor of something more future proof.

>> +
>>  endmenu
>>
>>  menu "ARM errata workaround via the alternative framework"
>> diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
>> index 4056e5b..9982fe9 100644
>> --- a/xen/arch/arm/gic-v3-its.c
>> +++ b/xen/arch/arm/gic-v3-its.c
> 
> [...]
> 
>> +static int its_map_baser(void __iomem *basereg, uint64_t regc,
>> +                         unsigned int nr_items)
>> +{
>> +    uint64_t attr, reg;
>> +    unsigned int entry_size = GITS_BASER_ENTRY_SIZE(regc);
>> +    unsigned int pagesz = 2, order, table_size;
> 
> Please document what mean 2.
> 
>> +    void *buffer;
>> +
>> +    attr  = GIC_BASER_InnerShareable << GITS_BASER_SHAREABILITY_SHIFT;
>> +    attr |= GIC_BASER_CACHE_SameAsInner <<
>> GITS_BASER_OUTER_CACHEABILITY_SHIFT;
>> +    attr |= GIC_BASER_CACHE_RaWaWb <<
>> GITS_BASER_INNER_CACHEABILITY_SHIFT;
>> +
>> +    /*
>> +     * Setup the BASE register with the attributes that we like. Then
>> read
>> +     * it back and see what sticks (page size, cacheability and
>> shareability
>> +     * attributes), retrying if necessary.
>> +     */
>> +retry:
>> +    table_size = ROUNDUP(nr_items * entry_size,
>> BIT(BASER_PAGE_BITS(pagesz)));
>> +    /* The BASE registers support at most 256 pages. */
>> +    table_size = min(table_size, 256U << BASER_PAGE_BITS(pagesz));
>> +    /* The memory block must be aligned to the requested page size. */
>> +    order = max(get_order_from_bytes(table_size), pagesz * 2);
>> +
>> +    buffer = alloc_xenheap_pages(order, 0);
> 
> Why don't you use _zalloc(...)? This would avoid to compute the order
> and drop few lines.

Because they need to be physically contiguous.
Please correct me if I am wrong here, but the normal *alloc functions do
not guarantee this (from checking the implementation).

>> +    if ( !buffer )
>> +        return -ENOMEM;
>> +
>> +    if ( !check_propbaser_phys_addr(buffer, BASER_PAGE_BITS(pagesz)) )
> 
> The name of the function does not make sense. You check an address for
> the register BASER and not PROPBASER.

Indeed.

>> +    {
>> +        free_xenheap_pages(buffer, 0);
>> +        return -ERANGE;
>> +    }
>> +    memset(buffer, 0, table_size);
> 
> [...]
> 
>> +int gicv3_its_init(void)
>> +{
>> +    struct host_its *hw_its;
>> +    int ret;
>> +
>> +    list_for_each_entry(hw_its, &host_its_list, entry) {
> 
> Coding style:
> 
> list_for_each_entry(....)
> {

Yeah, I was looking at the Linux driver at the same time ;-)

>> +        ret = gicv3_its_init_single_its(hw_its);
>> +        if ( ret )
>> +            return ret;
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>>  /* 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)
>>  {
>> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
>> index ed78363..cc1e219 100644
>> --- a/xen/arch/arm/gic-v3.c
>> +++ b/xen/arch/arm/gic-v3.c
>> @@ -1590,6 +1590,9 @@ static int __init gicv3_init(void)
>>      spin_lock(&gicv3.lock);
>>
>>      gicv3_dist_init();
>> +    res = gicv3_its_init();
>> +    if ( res )
>> +        printk(XENLOG_WARNING "GICv3: ITS: initialization failed:
>> %d\n", res);
> 
> I would have expect a panic here because the ITS subsystem could be half
> initialized and it is not safe to continue.

OK, let me check what actually happens here if there is no ITS ;-)

Cheers,
Andre

> 
>>      res = gicv3_cpu_init();
>>      gicv3_hyp_init();
>>
> 
> Cheers,
>
Julien Grall March 22, 2017, 4:33 p.m. UTC | #4
On 22/03/17 16:08, André Przywara wrote:
> Hi,

Hi Andre,

>
> On 22/03/17 13:52, Julien Grall wrote:
>> Hi Andre,
>>
>> On 03/16/2017 11:20 AM, Andre Przywara wrote:
>>> Each ITS maps a pair of a DeviceID (for instance derived from a PCI
>>> b/d/f triplet) and an EventID (the MSI payload or interrupt ID) to a
>>> pair of LPI number and collection ID, which points to the target CPU.
>>> This mapping is stored in the device and collection tables, which
>>> software
>>> has to provide for the ITS to use.
>>> Allocate the required memory and hand it to the ITS.
>>> The maximum number of devices is limited to a compile-time constant
>>> exposed in Kconfig.
>>>
>>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>>> ---
>>>  docs/misc/xen-command-line.markdown |   8 ++
>>>  xen/arch/arm/Kconfig                |  14 ++++
>>>  xen/arch/arm/gic-v3-its.c           | 163
>>> ++++++++++++++++++++++++++++++++++++
>>>  xen/arch/arm/gic-v3.c               |   3 +
>>>  xen/include/asm-arm/gic_v3_its.h    |  63 +++++++++++++-
>>>  5 files changed, 250 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/docs/misc/xen-command-line.markdown
>>> b/docs/misc/xen-command-line.markdown
>>> index 619016d..068d116 100644
>>> --- a/docs/misc/xen-command-line.markdown
>>> +++ b/docs/misc/xen-command-line.markdown
>>> @@ -1158,6 +1158,14 @@ based interrupts. Any higher IRQs will be
>>> available for use via PCI MSI.
>>>  ### maxcpus
>>>  > `= <integer>`
>>>
>>> +### max\_its\_device\_bits
>>> +> `= <integer>`
>>> +
>>> +Specifies the maximum number of devices using MSIs on the ARM GICv3 ITS
>>> +controller to allocate table entries for. Each table entry uses a
>>> hardware
>>> +specific size, typically 8 or 16 bytes.
>>> +Defaults to 10 bits (to cover at most 1024 devices).
>>
>> The description is misleading. Even if the platform has less than 1024
>> devices, 10 may not be enough if the Device ID are not contiguous.
>
> Right.
>
>> However, I don't think this is useful. A user may not know the DevIDs in
>> use of the platform and hence will not be able to choose a sensible value.
>>
>> I still think that we should allocate what the hardware asked us and if
>> it is necessary to limit this should be done per-platform.
>
> I think you are right, a configurable limit does not make much sense. I
> was wondering if we should have a hard coded *memory* limit instead, to
> prevent ridiculously high allocations, say entry_size=8 and 32 bits of
> device IDs, which would result in 32GB of memory for a flat device table.
>
> Or if we don't want to arbitrarily limit this, we always print the
> actual allocated size, maybe with a extra WARNING if it exceeds sane levels.

But what is a sane level? To be fair, I don't think this is the business 
of the common code to care about high allocations. If the platform ask 
32bits and not able to cope, then it should be a workaround for the 
platform.

[...]

>>> +    void *buffer;
>>> +
>>> +    attr  = GIC_BASER_InnerShareable << GITS_BASER_SHAREABILITY_SHIFT;
>>> +    attr |= GIC_BASER_CACHE_SameAsInner <<
>>> GITS_BASER_OUTER_CACHEABILITY_SHIFT;
>>> +    attr |= GIC_BASER_CACHE_RaWaWb <<
>>> GITS_BASER_INNER_CACHEABILITY_SHIFT;
>>> +
>>> +    /*
>>> +     * Setup the BASE register with the attributes that we like. Then
>>> read
>>> +     * it back and see what sticks (page size, cacheability and
>>> shareability
>>> +     * attributes), retrying if necessary.
>>> +     */
>>> +retry:
>>> +    table_size = ROUNDUP(nr_items * entry_size,
>>> BIT(BASER_PAGE_BITS(pagesz)));
>>> +    /* The BASE registers support at most 256 pages. */
>>> +    table_size = min(table_size, 256U << BASER_PAGE_BITS(pagesz));
>>> +    /* The memory block must be aligned to the requested page size. */
>>> +    order = max(get_order_from_bytes(table_size), pagesz * 2);
>>> +
>>> +    buffer = alloc_xenheap_pages(order, 0);
>>
>> Why don't you use _zalloc(...)? This would avoid to compute the order
>> and drop few lines.
>
> Because they need to be physically contiguous.
> Please correct me if I am wrong here, but the normal *alloc functions do
> not guarantee this (from checking the implementation).

_x*alloc will work the same way as k*alloc in Linux. The memory will be 
contiguous. Regarding the implementation details, _x*alloc is an overlay 
of alloc_xenheap_pages. For small size (i.e < PAGE_SIZE), it will 
coalesce in the same page. For bigger size, alloc_xenheap_pages will be 
used. Although, the resulting memory usage will differ because _x*alloc 
will free unused pages.

This is because alloc_xenheap_pages is working in term of order. So if 
you request 12K, alloc_xenheap_pages will allocate 16K. The 
implementation of _x*alloc will free the last 4K to avoid wasting space.

So effectively, _x*alloc will result in lower memory usage.

>>>      gicv3_dist_init();
>>> +    res = gicv3_its_init();
>>> +    if ( res )
>>> +        printk(XENLOG_WARNING "GICv3: ITS: initialization failed:
>>> %d\n", res);
>>
>> I would have expect a panic here because the ITS subsystem could be half
>> initialized and it is not safe to continue.
>
> OK, let me check what actually happens here if there is no ITS ;-)

Technically, this message should not happen when there is no ITS because 
it is not mandatory to have one on the platform.

So this would be an coding error for me.

Cheers,
Andre Przywara March 29, 2017, 1:58 p.m. UTC | #5
Hi,

On 22/03/17 16:33, Julien Grall wrote:
[ ... ]
>>>>      gicv3_dist_init();
>>>> +    res = gicv3_its_init();
>>>> +    if ( res )
>>>> +        printk(XENLOG_WARNING "GICv3: ITS: initialization failed:
>>>> %d\n", res);
>>>
>>> I would have expect a panic here because the ITS subsystem could be half
>>> initialized and it is not safe to continue.
>>
>> OK, let me check what actually happens here if there is no ITS ;-)
> 
> Technically, this message should not happen when there is no ITS because
> it is not mandatory to have one on the platform.
> 
> So this would be an coding error for me.

Having no ITS (node in the DT) would result in a empty host_its_list and
a "0" return, so doesn't raise any issues.

Technically we could cope with one or all ITSes to not initialize (by
not propagating them to any guests).
But for now I can just panic here, I guess, because it should point to
some serious issue.
If there is a use case, we can always add a more relaxed behavior later.

Cheers,
Andre.
diff mbox

Patch

diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown
index 619016d..068d116 100644
--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -1158,6 +1158,14 @@  based interrupts. Any higher IRQs will be available for use via PCI MSI.
 ### maxcpus
 > `= <integer>`
 
+### max\_its\_device\_bits
+> `= <integer>`
+
+Specifies the maximum number of devices using MSIs on the ARM GICv3 ITS
+controller to allocate table entries for. Each table entry uses a hardware
+specific size, typically 8 or 16 bytes.
+Defaults to 10 bits (to cover at most 1024 devices).
+
 ### max\_lpi\_bits
 > `= <integer>`
 
diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index 86f7b53..0d50156 100644
--- a/xen/arch/arm/Kconfig
+++ b/xen/arch/arm/Kconfig
@@ -64,6 +64,20 @@  config MAX_PHYS_LPI_BITS
           This can be overridden on the command line with the max_lpi_bits
           parameter.
 
+config MAX_PHYS_ITS_DEVICE_BITS
+        depends on HAS_ITS
+        int "Number of device bits the ITS supports"
+        range 1 32
+        default "10"
+        help
+          Specifies the maximum number of devices which want to use the ITS.
+          Xen needs to allocates memory for the whole range very early.
+          The allocation scheme may be sparse, so a much larger number must
+          be supported to cover devices with a high bus number or those on
+          separate bus segments.
+          This can be overridden on the command line with the
+          max_its_device_bits parameter.
+
 endmenu
 
 menu "ARM errata workaround via the alternative framework"
diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
index 4056e5b..9982fe9 100644
--- a/xen/arch/arm/gic-v3-its.c
+++ b/xen/arch/arm/gic-v3-its.c
@@ -19,8 +19,10 @@ 
  */
 
 #include <xen/lib.h>
+#include <xen/mm.h>
 #include <asm/gic_v3_defs.h>
 #include <asm/gic_v3_its.h>
+#include <asm/io.h>
 
 LIST_HEAD(host_its_list);
 
@@ -29,6 +31,167 @@  bool gicv3_its_host_has_its(void)
     return !list_empty(&host_its_list);
 }
 
+#define BASER_ATTR_MASK                                           \
+        ((0x3UL << GITS_BASER_SHAREABILITY_SHIFT)               | \
+         (0x7UL << GITS_BASER_OUTER_CACHEABILITY_SHIFT)         | \
+         (0x7UL << GITS_BASER_INNER_CACHEABILITY_SHIFT))
+#define BASER_RO_MASK   (GENMASK(58, 56) | GENMASK(52, 48))
+
+/* Check that the physical address can be encoded in the PROPBASER register. */
+static bool check_propbaser_phys_addr(void *vaddr, unsigned int page_bits)
+{
+    paddr_t paddr = virt_to_maddr(vaddr);
+
+    return (!(paddr & ~GENMASK(page_bits < 16 ? 47 : 51, page_bits)));
+}
+
+static uint64_t encode_propbaser_phys_addr(paddr_t addr, unsigned int page_bits)
+{
+    uint64_t ret = addr & GENMASK(47, page_bits);
+
+    if ( page_bits < 16 )
+        return ret;
+
+    /* For 64K pages address bits 51-48 are encoded in bits 15-12. */
+    return ret | ((addr & GENMASK(51, 48)) >> (48 - 12));
+}
+
+/* The ITS BASE registers work with page sizes of 4K, 16K or 64K. */
+#define BASER_PAGE_BITS(sz) ((sz) * 2 + 12)
+
+static int its_map_baser(void __iomem *basereg, uint64_t regc,
+                         unsigned int nr_items)
+{
+    uint64_t attr, reg;
+    unsigned int entry_size = GITS_BASER_ENTRY_SIZE(regc);
+    unsigned int pagesz = 2, order, table_size;
+    void *buffer;
+
+    attr  = GIC_BASER_InnerShareable << GITS_BASER_SHAREABILITY_SHIFT;
+    attr |= GIC_BASER_CACHE_SameAsInner << GITS_BASER_OUTER_CACHEABILITY_SHIFT;
+    attr |= GIC_BASER_CACHE_RaWaWb << GITS_BASER_INNER_CACHEABILITY_SHIFT;
+
+    /*
+     * Setup the BASE register with the attributes that we like. Then read
+     * it back and see what sticks (page size, cacheability and shareability
+     * attributes), retrying if necessary.
+     */
+retry:
+    table_size = ROUNDUP(nr_items * entry_size, BIT(BASER_PAGE_BITS(pagesz)));
+    /* The BASE registers support at most 256 pages. */
+    table_size = min(table_size, 256U << BASER_PAGE_BITS(pagesz));
+    /* The memory block must be aligned to the requested page size. */
+    order = max(get_order_from_bytes(table_size), pagesz * 2);
+
+    buffer = alloc_xenheap_pages(order, 0);
+    if ( !buffer )
+        return -ENOMEM;
+
+    if ( !check_propbaser_phys_addr(buffer, BASER_PAGE_BITS(pagesz)) )
+    {
+        free_xenheap_pages(buffer, 0);
+        return -ERANGE;
+    }
+    memset(buffer, 0, table_size);
+
+    reg  = attr;
+    reg |= (pagesz << GITS_BASER_PAGE_SIZE_SHIFT);
+    reg |= (table_size >> BASER_PAGE_BITS(pagesz)) - 1;
+    reg |= regc & BASER_RO_MASK;
+    reg |= GITS_VALID_BIT;
+    reg |= encode_propbaser_phys_addr(virt_to_maddr(buffer),
+                                      BASER_PAGE_BITS(pagesz));
+
+    writeq_relaxed(reg, basereg);
+    regc = readq_relaxed(basereg);
+
+    /* The host didn't like our attributes, just use what it returned. */
+    if ( (regc & BASER_ATTR_MASK) != attr )
+    {
+        /* If we can't map it shareable, drop cacheability as well. */
+        if ( (regc & GITS_BASER_SHAREABILITY_MASK) == GIC_BASER_NonShareable )
+        {
+            regc &= ~GITS_BASER_INNER_CACHEABILITY_MASK;
+            writeq_relaxed(regc, basereg);
+        }
+        attr = regc & BASER_ATTR_MASK;
+    }
+    if ( (regc & GITS_BASER_INNER_CACHEABILITY_MASK) <= GIC_BASER_CACHE_nC )
+        clean_and_invalidate_dcache_va_range(buffer, table_size);
+
+    /* If the host accepted our page size, we are done. */
+    if ( ((regc >> GITS_BASER_PAGE_SIZE_SHIFT) & 0x3UL) == pagesz )
+        return 0;
+
+    free_xenheap_pages(buffer, order);
+
+    if ( pagesz-- > 0 )
+        goto retry;
+
+    /* None of the page sizes was accepted, give up */
+    return -EINVAL;
+}
+
+static unsigned int max_its_device_bits = CONFIG_MAX_PHYS_ITS_DEVICE_BITS;
+integer_param("max_its_device_bits", max_its_device_bits);
+
+static int gicv3_its_init_single_its(struct host_its *hw_its)
+{
+    uint64_t reg;
+    int i;
+    unsigned int devid_bits;
+
+    hw_its->its_base = ioremap_nocache(hw_its->addr, hw_its->size);
+    if ( !hw_its->its_base )
+        return -ENOMEM;
+
+    reg = readq_relaxed(hw_its->its_base + GITS_TYPER);
+    devid_bits = GITS_TYPER_DEVICE_ID_BITS(reg);
+    devid_bits = min(devid_bits, max_its_device_bits);
+
+    for ( i = 0; i < GITS_BASER_NR_REGS; i++ )
+    {
+        void __iomem *basereg = hw_its->its_base + GITS_BASER0 + i * 8;
+        unsigned int type;
+
+        reg = readq_relaxed(basereg);
+        type = (reg & GITS_BASER_TYPE_MASK) >> GITS_BASER_TYPE_SHIFT;
+        switch ( type )
+        {
+        case GITS_BASER_TYPE_NONE:
+            continue;
+        case GITS_BASER_TYPE_DEVICE:
+            its_map_baser(basereg, reg, BIT(devid_bits));
+            break;
+        case GITS_BASER_TYPE_COLLECTION:
+            its_map_baser(basereg, reg, NR_CPUS);
+            break;
+        /* In case this is a GICv4, provide a (dummy) vPE table as well. */
+        case GITS_BASER_TYPE_VCPU:
+            its_map_baser(basereg, reg, 1);
+            break;
+        default:
+            continue;
+        }
+    }
+
+    return 0;
+}
+
+int gicv3_its_init(void)
+{
+    struct host_its *hw_its;
+    int ret;
+
+    list_for_each_entry(hw_its, &host_its_list, entry) {
+        ret = gicv3_its_init_single_its(hw_its);
+        if ( ret )
+            return ret;
+    }
+
+    return 0;
+}
+
 /* 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)
 {
diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index ed78363..cc1e219 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -1590,6 +1590,9 @@  static int __init gicv3_init(void)
     spin_lock(&gicv3.lock);
 
     gicv3_dist_init();
+    res = gicv3_its_init();
+    if ( res )
+        printk(XENLOG_WARNING "GICv3: ITS: initialization failed: %d\n", res);
     res = gicv3_cpu_init();
     gicv3_hyp_init();
 
diff --git a/xen/include/asm-arm/gic_v3_its.h b/xen/include/asm-arm/gic_v3_its.h
index 219d109..a6c0acc 100644
--- a/xen/include/asm-arm/gic_v3_its.h
+++ b/xen/include/asm-arm/gic_v3_its.h
@@ -20,6 +20,60 @@ 
 #ifndef __ASM_ARM_ITS_H__
 #define __ASM_ARM_ITS_H__
 
+#define GITS_CTLR                       0x000
+#define GITS_IIDR                       0x004
+#define GITS_TYPER                      0x008
+#define GITS_CBASER                     0x080
+#define GITS_CWRITER                    0x088
+#define GITS_CREADR                     0x090
+#define GITS_BASER_NR_REGS              8
+#define GITS_BASER0                     0x100
+#define GITS_BASER1                     0x108
+#define GITS_BASER2                     0x110
+#define GITS_BASER3                     0x118
+#define GITS_BASER4                     0x120
+#define GITS_BASER5                     0x128
+#define GITS_BASER6                     0x130
+#define GITS_BASER7                     0x138
+
+/* Register bits */
+#define GITS_VALID_BIT                  BIT_ULL(63)
+
+#define GITS_CTLR_QUIESCENT             BIT(31)
+#define GITS_CTLR_ENABLE                BIT(0)
+
+#define GITS_TYPER_DEVIDS_SHIFT         13
+#define GITS_TYPER_DEVIDS_MASK          (0x1fUL << GITS_TYPER_DEVIDS_SHIFT)
+#define GITS_TYPER_DEVICE_ID_BITS(r)    (((r & GITS_TYPER_DEVIDS_MASK) >> \
+                                               GITS_TYPER_DEVIDS_SHIFT) + 1)
+
+#define GITS_IIDR_VALUE                 0x34c
+
+#define GITS_BASER_INDIRECT             BIT_ULL(62)
+#define GITS_BASER_INNER_CACHEABILITY_SHIFT        59
+#define GITS_BASER_TYPE_SHIFT           56
+#define GITS_BASER_TYPE_MASK            (7ULL << GITS_BASER_TYPE_SHIFT)
+#define GITS_BASER_OUTER_CACHEABILITY_SHIFT        53
+#define GITS_BASER_TYPE_NONE            0UL
+#define GITS_BASER_TYPE_DEVICE          1UL
+#define GITS_BASER_TYPE_VCPU            2UL
+#define GITS_BASER_TYPE_CPU             3UL
+#define GITS_BASER_TYPE_COLLECTION      4UL
+#define GITS_BASER_TYPE_RESERVED5       5UL
+#define GITS_BASER_TYPE_RESERVED6       6UL
+#define GITS_BASER_TYPE_RESERVED7       7UL
+#define GITS_BASER_ENTRY_SIZE_SHIFT     48
+#define GITS_BASER_ENTRY_SIZE(reg)                                       \
+                        (((reg >> GITS_BASER_ENTRY_SIZE_SHIFT) & 0x1f) + 1)
+#define GITS_BASER_SHAREABILITY_SHIFT   10
+#define GITS_BASER_PAGE_SIZE_SHIFT      8
+#define GITS_BASER_RO_MASK              (GITS_BASER_TYPE_MASK | \
+                                        (31UL << GITS_BASER_ENTRY_SIZE_SHIFT) |\
+                                        GITS_BASER_INDIRECT)
+#define GITS_BASER_SHAREABILITY_MASK   (0x3ULL << GITS_BASER_SHAREABILITY_SHIFT)
+#define GITS_BASER_OUTER_CACHEABILITY_MASK   (0x7ULL << GITS_BASER_OUTER_CACHEABILITY_SHIFT)
+#define GITS_BASER_INNER_CACHEABILITY_MASK   (0x7ULL << GITS_BASER_INNER_CACHEABILITY_SHIFT)
+
 #include <xen/device_tree.h>
 
 /* data structure for each hardware ITS */
@@ -28,6 +82,7 @@  struct host_its {
     const struct dt_device_node *dt_node;
     paddr_t addr;
     paddr_t size;
+    void __iomem *its_base;
 };
 
 
@@ -42,8 +97,9 @@  bool gicv3_its_host_has_its(void);
 
 int gicv3_lpi_init_rdist(void __iomem * rdist_base);
 
-/* Initialize the host structures for LPIs. */
+/* Initialize the host structures for LPIs and the host ITSes. */
 int gicv3_lpi_init_host_lpis(unsigned int nr_lpis);
+int gicv3_its_init(void);
 
 #else
 
@@ -67,6 +123,11 @@  static inline int gicv3_lpi_init_host_lpis(unsigned int nr_lpis)
 {
     return 0;
 }
+
+static inline int gicv3_its_init(void)
+{
+    return 0;
+}
 #endif /* CONFIG_HAS_ITS */
 
 #endif