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

Re: [Xen-devel] [RFC PATCH 05/24] ARM: GICv3 ITS: introduce ITS command handling



Hi,

On 27/10/16 00:55, Stefano Stabellini wrote:
> On Wed, 28 Sep 2016, Andre Przywara wrote:
>> To be able to easily send commands to the ITS, create the respective
>> wrapper functions, which take care of the ring buffer.
>> The first two commands we implement provide methods to map a collection
>> to a redistributor (aka host core) and to flush the command queue (SYNC).
>> Start using these commands for mapping one collection to each host CPU.
>>
>> Signed-off-by: Andre Przywara <andre.przywara@xxxxxxx>
>> ---
>>  xen/arch/arm/gic-its.c        | 101 
>> ++++++++++++++++++++++++++++++++++++++++++
>>  xen/arch/arm/gic-v3.c         |  17 +++++++
>>  xen/include/asm-arm/gic-its.h |  32 +++++++++++++
>>  3 files changed, 150 insertions(+)
>>
>> diff --git a/xen/arch/arm/gic-its.c b/xen/arch/arm/gic-its.c
>> index c8a7a7e..88397bc 100644
>> --- a/xen/arch/arm/gic-its.c
>> +++ b/xen/arch/arm/gic-its.c
>> @@ -33,6 +33,10 @@ static struct {
>>      int host_lpi_bits;
>>  } lpi_data;
>>  
>> +/* Physical redistributor address */
>> +static DEFINE_PER_CPU(uint64_t, rdist_addr);
>> +/* Redistributor ID */
>> +static DEFINE_PER_CPU(uint64_t, rdist_id);
>>  /* Pending table for each redistributor */
>>  static DEFINE_PER_CPU(void *, pending_table);
>>  
>> @@ -40,6 +44,86 @@ static DEFINE_PER_CPU(void *, pending_table);
>>          min_t(unsigned int, lpi_data.host_lpi_bits, CONFIG_HOST_LPI_BITS)
>>  #define MAX_HOST_LPIS   (BIT(MAX_HOST_LPI_BITS) - 8192)
>>  
>> +#define ITS_COMMAND_SIZE        32
>> +
>> +static int its_send_command(struct host_its *hw_its, void *its_cmd)
>> +{
>> +    int readp, writep;
> 
> uint64_t

A bit overkill, but probably right type-wise.

> 
>> +    spin_lock(&hw_its->cmd_lock);
>> +
>> +    readp = readl_relaxed(hw_its->its_base + GITS_CREADR) & GENMASK(19, 5);
>> +    writep = readl_relaxed(hw_its->its_base + GITS_CWRITER) & GENMASK(19, 
>> 5);
> 
> It might be worth to
> 
>   #define ITS_CMD_RING_SIZE PAGE_SIZE
> 
> for clarity

Or revisit this to allow bigger queues than one page.

> 
>> +    if ( ((writep + ITS_COMMAND_SIZE) % PAGE_SIZE) == readp )
>> +    {
>> +        spin_unlock(&hw_its->cmd_lock);
>> +        return -EBUSY;
>> +    }
>> +
>> +    memcpy(hw_its->cmd_buf + writep, its_cmd, ITS_COMMAND_SIZE);
>> +    __flush_dcache_area(hw_its->cmd_buf + writep, ITS_COMMAND_SIZE);
>> +    writep = (writep + ITS_COMMAND_SIZE) % PAGE_SIZE;
>> +
>> +    writeq_relaxed(writep & GENMASK(19, 5), hw_its->its_base + 
>> GITS_CWRITER);
>> +
>> +    spin_unlock(&hw_its->cmd_lock);
>> +
>> +    return 0;
>> +}
>> +
>> +static uint64_t encode_rdbase(struct host_its *hw_its, int cpu, uint64_t 
>> reg)
>> +{
>> +    reg &= ~GENMASK(51, 16);
>> +
>> +    if ( hw_its->pta )
>> +        reg |= per_cpu(rdist_addr, cpu) & GENMASK(51, 16);
> 
> Again in my version of the spec is GENMASK(47, 16).
> 
> 
>> +    else
>> +        reg |= per_cpu(rdist_id, cpu) << 16;
>> +    return reg;
>> +}
>> +
>> +static int its_send_cmd_sync(struct host_its *its, int cpu)
>> +{
>> +    uint64_t cmd[4];
>> +
>> +    cmd[0] = GITS_CMD_SYNC;
>> +    cmd[1] = 0x00;
>> +    cmd[2] = encode_rdbase(its, cpu, 0x0);
>> +    cmd[3] = 0x00;
>> +
>> +    return its_send_command(its, cmd);
>> +}
>> +
>> +static int its_send_cmd_mapc(struct host_its *its, int collection_id, int 
>> cpu)
>> +{
>> +    uint64_t cmd[4];
>> +
>> +    cmd[0] = GITS_CMD_MAPC;
>> +    cmd[1] = 0x00;
>> +    cmd[2] = encode_rdbase(its, cpu, (collection_id & GENMASK(15, 0)) | 
>> BIT(63));
>> +    cmd[3] = 0x00;
>> +
>> +    return its_send_command(its, cmd);
>> +}
>> +
>> +/* Set up the (1:1) collection mapping for the given host CPU. */
>> +void gicv3_its_setup_collection(int cpu)
>> +{
>> +    struct host_its *its;
>> +
>> +    list_for_each_entry(its, &host_its_list, entry)
>> +    {
>> +        /* Only send commands to ITS that have been initialized already. */
>> +        if ( !its->cmd_buf )
>> +            continue;
>> +
>> +        its_send_cmd_mapc(its, cpu, cpu);
>> +        its_send_cmd_sync(its, cpu);
>> +    }
>> +}
>> +
>>  #define BASER_ATTR_MASK                                           \
>>          ((0x3UL << GITS_BASER_SHAREABILITY_SHIFT)               | \
>>           (0x7UL << GITS_BASER_OUTER_CACHEABILITY_SHIFT)         | \
>> @@ -147,6 +231,13 @@ int gicv3_its_init(struct host_its *hw_its)
>>      if ( !hw_its->its_base )
>>          return -ENOMEM;
>>  
>> +    /* Make sure the ITS is disabled before programming the BASE registers. 
>> */
>> +    reg = readl_relaxed(hw_its->its_base + GITS_CTLR);
>> +    writel_relaxed(reg & ~GITS_CTLR_ENABLE, hw_its->its_base + GITS_CTLR);
>> +
>> +    reg = readq_relaxed(hw_its->its_base + GITS_TYPER);
>> +    hw_its->pta = reg & GITS_TYPER_PTA;
> 
> To avoid problems:
> 
>   pta = !!(reg & GITS_TYPER_PTA);
> 
> 
>>      for (i = 0; i < 8; i++)
>>      {
>>          void __iomem *basereg = hw_its->its_base + GITS_BASER0 + i * 8;
>> @@ -174,9 +265,18 @@ int gicv3_its_init(struct host_its *hw_its)
>>      if ( IS_ERR(hw_its->cmd_buf) )
>>          return PTR_ERR(hw_its->cmd_buf);
>>  
>> +    its_send_cmd_mapc(hw_its, smp_processor_id(), smp_processor_id());
>> +    its_send_cmd_sync(hw_its, smp_processor_id());
> 
> Why do we need these two commands in addition to the ones issued by
> gicv3_its_setup_collection?

gicv3_its_setup_collection() gets called for each redistributor. On the
first CPU this is done _before_ we actually have set up the ITS, so we
can't send any commands at this time. The function actually checks for
an initialised command buffer, eventually doing nothing on the first core.
However this function _here_ is only called once (on core 0).
So we send the commands for core 0 now, actually at the earliest
possible time.

Cheers,
Andre.

>>      return 0;
>>  }
>>  
>> +void gicv3_set_redist_addr(paddr_t address, int redist_id)
>> +{
>> +    this_cpu(rdist_addr) = address;
>> +    this_cpu(rdist_id) = redist_id;
>> +}
>> +
>>  uint64_t gicv3_lpi_allocate_pendtable(void)
>>  {
>>      uint64_t reg, attr;
>> @@ -265,6 +365,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/gic-v3.c b/xen/arch/arm/gic-v3.c
>> index 5cf4618..b9387a3 100644
>> --- a/xen/arch/arm/gic-v3.c
>> +++ b/xen/arch/arm/gic-v3.c
>> @@ -638,6 +638,8 @@ static void gicv3_rdist_init_lpis(void __iomem * 
>> rdist_base)
>>      table_reg = gicv3_lpi_get_proptable();
>>      if ( table_reg )
>>          writeq_relaxed(table_reg, rdist_base + GICR_PROPBASER);
>> +
>> +    gicv3_its_setup_collection(smp_processor_id());
>>  }
>>  
>>  static int __init gicv3_populate_rdist(void)
>> @@ -684,7 +686,22 @@ static int __init gicv3_populate_rdist(void)
>>                  this_cpu(rbase) = ptr;
>>  
>>                  if ( typer & GICR_TYPER_PLPIS )
>> +                {
>> +                    paddr_t rdist_addr;
>> +
>> +                    rdist_addr = gicv3.rdist_regions[i].base;
>> +                    rdist_addr += ptr - gicv3.rdist_regions[i].map_base;
>> +
>> +                    /* The ITS refers to redistributors either by their 
>> physical
>> +                     * address or by their ID. Determine those two values 
>> and
>> +                     * let the ITS code store them in per host CPU 
>> variables to
>> +                     * later be able to address those redistributors.
>> +                     */
>> +                    gicv3_set_redist_addr(rdist_addr,
>> +                                          (typer >> 8) & GENMASK(15, 0));
> 
> Please #define the 8
> 
> 
>>                      gicv3_rdist_init_lpis(ptr);
>> +                }
>>  
>>                  printk("GICv3: CPU%d: Found redistributor in region %d 
>> @%p\n",
>>                          smp_processor_id(), i, ptr);
>> diff --git a/xen/include/asm-arm/gic-its.h b/xen/include/asm-arm/gic-its.h
>> index b2a003f..b49d274 100644
>> --- a/xen/include/asm-arm/gic-its.h
>> +++ b/xen/include/asm-arm/gic-its.h
>> @@ -37,6 +37,7 @@
>>  
>>  /* Register bits */
>>  #define GITS_CTLR_ENABLE     0x1
>> +#define GITS_TYPER_PTA       BIT(19)
>>  #define GITS_IIDR_VALUE      0x34c
>>  
>>  #define GITS_BASER_VALID                BIT(63)
>> @@ -59,6 +60,22 @@
>>                                          (31UL << 
>> GITS_BASER_ENTRY_SIZE_SHIFT) |\
>>                                          GITS_BASER_INDIRECT)
>>  
>> +/* ITS command definitions */
>> +#define ITS_CMD_SIZE                    32
>> +
>> +#define GITS_CMD_MOVI                   0x01
>> +#define GITS_CMD_INT                    0x03
>> +#define GITS_CMD_CLEAR                  0x04
>> +#define GITS_CMD_SYNC                   0x05
>> +#define GITS_CMD_MAPD                   0x08
>> +#define GITS_CMD_MAPC                   0x09
>> +#define GITS_CMD_MAPTI                  0x0a
> 
> In my version of the spec (PRD03-GENC-010745 24.0) 0x0a is MAPVI.
> 
> 
>> +#define GITS_CMD_MAPI                   0x0b
>> +#define GITS_CMD_INV                    0x0c
>> +#define GITS_CMD_INVALL                 0x0d
>> +#define GITS_CMD_MOVALL                 0x0e
>> +#define GITS_CMD_DISCARD                0x0f
>> +
>>  #ifndef __ASSEMBLY__
>>  #include <xen/device_tree.h>
>>  
>> @@ -69,7 +86,9 @@ struct host_its {
>>      paddr_t addr;
>>      paddr_t size;
>>      void __iomem *its_base;
>> +    spinlock_t cmd_lock;
>>      void *cmd_buf;
>> +    bool pta;
>>  };
>>  
>>  extern struct list_head host_its_list;
>> @@ -89,6 +108,12 @@ uint64_t gicv3_lpi_allocate_pendtable(void);
>>  int gicv3_lpi_init_host_lpis(int nr_lpis);
>>  int gicv3_its_init(struct host_its *hw_its);
>>  
>> +/* Set the physical address and ID for each redistributor as read from DT. 
>> */
>> +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);
>> +
>>  #else
>>  
>>  static inline void gicv3_its_dt_init(const struct dt_device_node *node)
>> @@ -110,6 +135,13 @@ static inline int gicv3_its_init(struct host_its 
>> *hw_its)
>>  {
>>      return 0;
>>  }
>> +static inline void gicv3_set_redist_addr(paddr_t address, int redist_id)
>> +{
>> +}
>> +static inline void gicv3_its_setup_collection(int cpu)
>> +{
>> +}
>> +
>>  #endif /* CONFIG_HAS_ITS */
>>  
>>  #endif /* __ASSEMBLY__ */
> 

_______________________________________________
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®.