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

Re: [Xen-devel] [RFC PATCH v2 03/26] ARM: GICv3 ITS: allocate device and collection table



Hi Stefano,

On 04/01/17 21:47, Stefano Stabellini wrote:
> On Thu, 22 Dec 2016, Andre Przywara wrote:
>> Each ITS maps a pair of a DeviceID (usually the 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 the ITS.
>> The maximum number of devices is limited to a compile-time constant
>> exposed in Kconfig.
>>
>> Signed-off-by: Andre Przywara <andre.przywara@xxxxxxx>
>> ---
>>  xen/arch/arm/Kconfig          |   6 +++
>>  xen/arch/arm/gic-its.c        | 114 
>> ++++++++++++++++++++++++++++++++++++++++++
>>  xen/arch/arm/gic-v3.c         |   5 ++
>>  xen/include/asm-arm/bitops.h  |   1 +
>>  xen/include/asm-arm/gic-its.h |  51 ++++++++++++++++++-
>>  5 files changed, 176 insertions(+), 1 deletion(-)
>>
>> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
>> index a7d941c..a369305 100644
>> --- a/xen/arch/arm/Kconfig
>> +++ b/xen/arch/arm/Kconfig
>> @@ -59,6 +59,12 @@ config MAX_HOST_LPI_BITS
>>            This can be overriden on the command line with the max_lpi_bits
>>            parameter.
>>  
>> +config MAX_ITS_PCI_BUSSES
>> +        depends on HAS_ITS
>> +        int "Number of PCI busses the ITS supports (4)"
>> +        range 1 1024
>> +        default "4"
> 
> Given that any kind of devices can be behind an ITS, including non-PCI
> devices, I think it is best to call this MAX_PHYS_ITS_DEVICES.

Good point.

>>  endmenu
>>  
>>  menu "ARM errata workaround via the alternative framework"
>> diff --git a/xen/arch/arm/gic-its.c b/xen/arch/arm/gic-its.c
>> index 6c3a35d..f1540b2 100644
>> --- a/xen/arch/arm/gic-its.c
>> +++ b/xen/arch/arm/gic-its.c
>> @@ -22,6 +22,7 @@
>>  #include <xen/libfdt/libfdt.h>
>>  #include <xen/sizes.h>
>>  #include <asm/p2m.h>
>> +#include <asm/io.h>
>>  #include <asm/gic.h>
>>  #include <asm/gic_v3_defs.h>
>>  #include <asm/gic-its.h>
>> @@ -37,6 +38,119 @@ static DEFINE_PER_CPU(void *, pending_table);
>>  
>>  #define MAX_PHYS_LPIS   (BIT_ULL(lpi_data.host_lpi_bits) - 8192)
>>  
>> +#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))
>> +
>> +static uint64_t encode_phys_addr(paddr_t addr, int page_bits)
>> +{
>> +    uint64_t ret;
>> +
>> +    if ( page_bits < 16)
>> +        return (uint64_t)addr & GENMASK(47, page_bits);
>> +
>> +    ret = addr & GENMASK(47, 16);
>> +    return ret | (addr & GENMASK(51, 48)) >> (48 - 12);
>> +}
>> +
>> +static int its_map_baser(void __iomem *basereg, uint64_t regc, int nr_items)
>> +{
>> +    uint64_t attr;
>> +    int entry_size = ((regc >> GITS_BASER_ENTRY_SIZE_SHIFT) & 0x1f) + 1;
>> +    int pagesz;
>> +    int order;
>> +    void *buffer = NULL;
>> +
>> +    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;
>> +
>> +    /*
>> +     * Loop over the page sizes (4K, 16K, 64K) to find out what the host
>> +     * supports.
>> +     */
>> +    for ( pagesz = 0; pagesz < 3; pagesz++ )
>> +    {
>> +        uint64_t reg;
>> +        int nr_bytes;
>> +
>> +        nr_bytes = ROUNDUP(nr_items * entry_size, BIT(pagesz * 2 + 12));
> 
> 12/PAGE_SHIFT

Will use a macro to convert the loop counter into the number of bits.

>> +        order = get_order_from_bytes(nr_bytes);
>> +
>> +        if ( !buffer )
>> +            buffer = alloc_xenheap_pages(order, 0);
>> +        if ( !buffer )
>> +            return -ENOMEM;
>> +
>> +        reg  = attr;
>> +        reg |= (pagesz << GITS_BASER_PAGE_SIZE_SHIFT);
>> +        reg |= nr_bytes >> (pagesz * 2 + 12);
>> +        reg |= regc & BASER_RO_MASK;
>> +        reg |= GITS_BASER_VALID;
>> +        reg |= encode_phys_addr(virt_to_maddr(buffer), pagesz * 2 + 12);
>> +
>> +        writeq_relaxed(reg, basereg);
>> +        regc = readl_relaxed(basereg);
>> +
>> +        /* The host didn't like our attributes, just use what it returned. 
>> */
>> +        if ( (regc & BASER_ATTR_MASK) != attr )
>> +            attr = regc & BASER_ATTR_MASK;
>> +
>> +        /* If the host accepted our page size, we are done. */
>> +        if ( (reg & (3UL << GITS_BASER_PAGE_SIZE_SHIFT)) == pagesz )
>> +            return 0;
>> +
>> +        /* Check whether our buffer is aligned to the next page size 
>> already. */
>> +        if ( !(virt_to_maddr(buffer) & (BIT(pagesz * 2 + 12 + 2) - 1)) )
>> +        {
>> +            free_xenheap_pages(buffer, order);
>> +            buffer = NULL;
>> +        }
> 
> Regardless of alignment, should we always free buffer here, given that
> we need to allocate the right size for the next attempt?

This was an attempt to (a premature) optimization to avoid repetitive
allocations in case the first allocation was already 64K aligned.
Looking at it again I see that the order depends on the page size as
well, so this is broken. I will unconditionally free the buffer here.

Cheers,
Andre.

>> +    }
>> +
>> +    if ( buffer )
>> +        free_xenheap_pages(buffer, order);
>> +
>> +    return -EINVAL;
>> +}
>> +
>> +int gicv3_its_init(struct host_its *hw_its)
>> +{
>> +    uint64_t reg;
>> +    int i;
>> + 
>> +    hw_its->its_base = ioremap_nocache(hw_its->addr, hw_its->size);
>> +    if ( !hw_its->its_base )
>> +        return -ENOMEM;
>> +
>> +    for ( i = 0; i < GITS_BASER_NR_REGS; i++ )
>> +    {
>> +        void __iomem *basereg = hw_its->its_base + GITS_BASER0 + i * 8;
>> +        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:
>> +            /* TODO: find some better way of limiting the number of devices 
>> */
>> +            its_map_baser(basereg, reg, CONFIG_MAX_ITS_PCI_BUSSES * 256);
>> +            break;
>> +        case GITS_BASER_TYPE_COLLECTION:
>> +            its_map_baser(basereg, reg, NR_CPUS);
>> +            break;
>> +        default:
>> +            continue;
>> +        }
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>>  uint64_t gicv3_lpi_allocate_pendtable(void)
>>  {
>>      uint64_t reg, attr;
>> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
>> index 1c6869d..8ca7da2 100644
>> --- a/xen/arch/arm/gic-v3.c
>> +++ b/xen/arch/arm/gic-v3.c
>> @@ -29,6 +29,7 @@
>>  #include <xen/irq.h>
>>  #include <xen/iocap.h>
>>  #include <xen/sched.h>
>> +#include <xen/err.h>
>>  #include <xen/errno.h>
>>  #include <xen/delay.h>
>>  #include <xen/device_tree.h>
>> @@ -1564,6 +1565,7 @@ static int __init gicv3_init(void)
>>  {
>>      int res, i;
>>      uint32_t reg;
>> +    struct host_its *hw_its;
>>  
>>      if ( !cpu_has_gicv3 )
>>      {
>> @@ -1619,6 +1621,9 @@ static int __init gicv3_init(void)
>>      res = gicv3_cpu_init();
>>      gicv3_hyp_init();
>>  
>> +    list_for_each_entry(hw_its, &host_its_list, entry)
>> +        gicv3_its_init(hw_its);
>> +
>>      spin_unlock(&gicv3.lock);
>>  
>>      return res;
>> diff --git a/xen/include/asm-arm/bitops.h b/xen/include/asm-arm/bitops.h
>> index c9adf79..4ccb444 100644
>> --- a/xen/include/asm-arm/bitops.h
>> +++ b/xen/include/asm-arm/bitops.h
>> @@ -24,6 +24,7 @@
>>  #define BIT(nr)                 (1UL << (nr))
>>  #define BIT_MASK(nr)            (1UL << ((nr) % BITS_PER_WORD))
>>  #define BIT_WORD(nr)            ((nr) / BITS_PER_WORD)
>> +#define BIT_ULL(nr)             (1ULL << (nr))
>>  #define BITS_PER_BYTE           8
>>  #define BIT_ULL(nr)             (1ULL << (nr))
>>  
>> diff --git a/xen/include/asm-arm/gic-its.h b/xen/include/asm-arm/gic-its.h
>> index a66b6be..26b0584 100644
>> --- a/xen/include/asm-arm/gic-its.h
>> +++ b/xen/include/asm-arm/gic-its.h
>> @@ -18,6 +18,49 @@
>>  #ifndef __ASM_ARM_ITS_H__
>>  #define __ASM_ARM_ITS_H__
>>  
>> +#define LPI_OFFSET                      8192
>> +
>> +#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_CTLR_ENABLE                BIT_ULL(0)
>> +#define GITS_IIDR_VALUE                 0x34c
>> +
>> +#define GITS_BASER_VALID                BIT_ULL(63)
>> +#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_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)
>> +
>>  #ifndef __ASSEMBLY__
>>  #include <xen/device_tree.h>
>>  
>> @@ -27,6 +70,7 @@ struct host_its {
>>      const struct dt_device_node *dt_node;
>>      paddr_t addr;
>>      paddr_t size;
>> +    void __iomem *its_base;
>>  };
>>  
>>  extern struct list_head host_its_list;
>> @@ -42,8 +86,9 @@ void gicv3_its_dt_init(const struct dt_device_node *node);
>>  uint64_t gicv3_lpi_get_proptable(void);
>>  uint64_t gicv3_lpi_allocate_pendtable(void);
>>  
>> -/* 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(struct host_its *hw_its);
>>  
>>  #else
>>  
>> @@ -62,6 +107,10 @@ static inline int gicv3_lpi_init_host_lpis(unsigned int 
>> nr_lpis)
>>  {
>>      return 0;
>>  }
>> +static inline int gicv3_its_init(struct host_its *hw_its)
>> +{
>> +    return 0;
>> +}
>>  #endif /* CONFIG_HAS_ITS */
>>  
>>  #endif /* __ASSEMBLY__ */
>> -- 
>> 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®.