|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [RFC PATCH 02/24] ARM: GICv3: allocate LPI pending and property table
Hi Andre,
Sorry for the late reply, I'll try to be faster for the next rounds of
review. The patch looks good for a first iteration. Some comments below.
On Wed, 28 Sep 2016, Andre Przywara wrote:
> The ARM GICv3 ITS provides a new kind of interrupt called LPIs.
> The pending bits and the configuration data (priority, enable bits) for
> those LPIs are stored in tables in normal memory, which software has to
> provide to the hardware.
> Allocate the required memory, initialize it and hand it over to each
> ITS. We limit the number of LPIs we use with a compile time constant to
> avoid wasting memory.
>
> Signed-off-by: Andre Przywara <andre.przywara@xxxxxxx>
> ---
> xen/arch/arm/Kconfig | 6 ++++
> xen/arch/arm/efi/efi-boot.h | 1 -
> xen/arch/arm/gic-its.c | 76
> +++++++++++++++++++++++++++++++++++++++
> xen/arch/arm/gic-v3.c | 27 ++++++++++++++
> xen/include/asm-arm/cache.h | 4 +++
> xen/include/asm-arm/gic-its.h | 22 +++++++++++-
> xen/include/asm-arm/gic_v3_defs.h | 48 ++++++++++++++++++++++++-
> 7 files changed, 181 insertions(+), 3 deletions(-)
>
> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> index 9fe3b8e..66e2bb8 100644
> --- a/xen/arch/arm/Kconfig
> +++ b/xen/arch/arm/Kconfig
> @@ -50,6 +50,12 @@ config HAS_ITS
> depends on ARM_64
> depends on HAS_GICV3
>
> +config HOST_LPI_BITS
> + depends on HAS_ITS
> + int "Maximum bits for GICv3 host LPIs (14-32)"
> + range 14 32
> + default "20"
> +
> config ALTERNATIVE
> bool
>
> diff --git a/xen/arch/arm/efi/efi-boot.h b/xen/arch/arm/efi/efi-boot.h
> index 045d6ce..dc64aec 100644
> --- a/xen/arch/arm/efi/efi-boot.h
> +++ b/xen/arch/arm/efi/efi-boot.h
> @@ -10,7 +10,6 @@
> #include "efi-dom0.h"
>
> void noreturn efi_xen_start(void *fdt_ptr, uint32_t fdt_size);
> -void __flush_dcache_area(const void *vaddr, unsigned long size);
>
> #define DEVICE_TREE_GUID \
> {0xb1b621d5, 0xf19c, 0x41a5, {0x83, 0x0b, 0xd9, 0x15, 0x2c, 0x69, 0xaa,
> 0xe0}}
> diff --git a/xen/arch/arm/gic-its.c b/xen/arch/arm/gic-its.c
> index 0f42a77..b52dff3 100644
> --- a/xen/arch/arm/gic-its.c
> +++ b/xen/arch/arm/gic-its.c
> @@ -20,10 +20,86 @@
> #include <xen/lib.h>
> #include <xen/device_tree.h>
> #include <xen/libfdt/libfdt.h>
> +#include <asm/p2m.h>
> #include <asm/gic.h>
> #include <asm/gic_v3_defs.h>
> #include <asm/gic-its.h>
>
> +/* Global state */
> +static struct {
> + uint8_t *lpi_property;
> + int host_lpi_bits;
> +} lpi_data;
> +
> +/* Pending table for each redistributor */
> +static DEFINE_PER_CPU(void *, pending_table);
> +
> +#define MAX_HOST_LPI_BITS \
To avoid confusion, I would call this MAX_PHYS_LPI_BITS
> + min_t(unsigned int, lpi_data.host_lpi_bits, CONFIG_HOST_LPI_BITS)
> +#define MAX_HOST_LPIS (BIT(MAX_HOST_LPI_BITS) - 8192)
And this MAX_PHYS_LPIS
> +uint64_t gicv3_lpi_allocate_pendtable(void)
> +{
> + uint64_t reg, attr;
> + void *pendtable;
I would introduce a check to make sure that this_cpu(pending_table) == NULL.
> + attr = GIC_BASER_CACHE_RaWaWb <<
> GICR_PENDBASER_INNER_CACHEABILITY_SHIFT;
> + attr |= GIC_BASER_CACHE_SameAsInner <<
> GICR_PENDBASER_OUTER_CACHEABILITY_SHIFT;
> + attr |= GIC_BASER_InnerShareable << GICR_PENDBASER_SHAREABILITY_SHIFT;
> +
> + /*
> + * The pending table holds one bit per LPI, so we need three bits less
> + * than the number of LPI_BITs.
Why 3 bit less? Please add more info on how you came up with 3.
> But the alignment requirement from the
> + * ITS is 64K, so make order at least 16 (-12).
Does it need to be 64K aligned or does it need to be at least 64K in
size? That makes a big difference. If it just needs to be 64K aligned,
you can do that with xmalloc.
> + */
> + pendtable = alloc_xenheap_pages(MAX(lpi_data.host_lpi_bits - 3, 16) -
> 12, 0);
Shouldn't we be using MAX_HOST_LPI_BITS instead of
lpi_data.host_lpi_bits to make this calculation?
> + if ( !pendtable )
> + return 0;
> +
> + memset(pendtable, 0, BIT(lpi_data.host_lpi_bits - 3));
flush_dcache?
> + this_cpu(pending_table) = pendtable;
> +
> + reg = attr | GICR_PENDBASER_PTZ;
> + reg |= virt_to_maddr(pendtable) & GENMASK(51, 16);
> +
> + return reg;
> +}
> +
> +uint64_t gicv3_lpi_get_proptable()
> +{
> + uint64_t attr;
> + static uint64_t reg = 0;
> +
> + /* The property table is shared across all redistributors. */
> + if ( reg )
> + return reg;
Can't you just use lpi_data.lpi_property != NULL instead of introducing
a new static local variable?
> + attr = GIC_BASER_CACHE_RaWaWb <<
> GICR_PENDBASER_INNER_CACHEABILITY_SHIFT;
> + attr |= GIC_BASER_CACHE_SameAsInner <<
> GICR_PENDBASER_OUTER_CACHEABILITY_SHIFT;
> + attr |= GIC_BASER_InnerShareable << GICR_PENDBASER_SHAREABILITY_SHIFT;
> +
> + lpi_data.lpi_property = alloc_xenheap_pages(MAX_HOST_LPI_BITS - 12, 0);
Please add a comment on how the order is calculated.
> + if ( !lpi_data.lpi_property )
> + return 0;
> +
> + memset(lpi_data.lpi_property, GIC_PRI_IRQ | LPI_PROP_RES1,
> MAX_HOST_LPIS);
> + __flush_dcache_area(lpi_data.lpi_property, MAX_HOST_LPIS);
> +
> + reg = attr | ((MAX_HOST_LPI_BITS - 1) << 0);
> + reg |= virt_to_maddr(lpi_data.lpi_property) & GENMASK(51, 12);
> +
> + return reg;
> +}
> +
> +int gicv3_lpi_init_host_lpis(int lpi_bits)
> +{
> + lpi_data.host_lpi_bits = lpi_bits;
> +
> + printk("GICv3: using at most %ld LPIs on the host.\n", MAX_HOST_LPIS);
> +
> + return 0;
> +}
> +
> void gicv3_its_dt_init(const struct dt_device_node *node)
> {
> const struct dt_device_node *its = NULL;
> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
> index 238da84..2534aa5 100644
> --- a/xen/arch/arm/gic-v3.c
> +++ b/xen/arch/arm/gic-v3.c
> @@ -546,6 +546,9 @@ static void __init gicv3_dist_init(void)
> type = readl_relaxed(GICD + GICD_TYPER);
> nr_lines = 32 * ((type & GICD_TYPE_LINES) + 1);
>
> + if ( type & GICD_TYPE_LPIS )
> + gicv3_lpi_init_host_lpis(((type >> GICD_TYPE_ID_BITS_SHIFT) & 0x1f)
> + 1);
Please #define a mask instead of using 0x1f
> +
> printk("GICv3: %d lines, (IID %8.8x).\n",
> nr_lines, readl_relaxed(GICD + GICD_IIDR));
>
> @@ -615,6 +618,26 @@ static int gicv3_enable_redist(void)
>
> return 0;
> }
> +static void gicv3_rdist_init_lpis(void __iomem * rdist_base)
> +{
> + uint32_t reg;
> + uint64_t table_reg;
> +
> + if ( list_empty(&host_its_list) )
> + return;
> +
> + /* Make sure LPIs are disabled before setting up the BASERs. */
> + reg = readl_relaxed(rdist_base + GICR_CTLR);
> + writel_relaxed(reg & ~GICR_CTLR_ENABLE_LPIS, rdist_base + GICR_CTLR);
> +
> + table_reg = gicv3_lpi_allocate_pendtable();
> + if ( table_reg )
> + writeq_relaxed(table_reg, rdist_base + GICR_PENDBASER);
Maybe we want to return in case table_reg == NULL ?
> + table_reg = gicv3_lpi_get_proptable();
> + if ( table_reg )
> + writeq_relaxed(table_reg, rdist_base + GICR_PROPBASER);
> +}
>
> static int __init gicv3_populate_rdist(void)
> {
> @@ -658,6 +681,10 @@ static int __init gicv3_populate_rdist(void)
> if ( (typer >> 32) == aff )
> {
> this_cpu(rbase) = ptr;
> +
> + if ( typer & GICR_TYPER_PLPIS )
> + gicv3_rdist_init_lpis(ptr);
> +
> printk("GICv3: CPU%d: Found redistributor in region %d
> @%p\n",
> smp_processor_id(), i, ptr);
> return 0;
> diff --git a/xen/include/asm-arm/cache.h b/xen/include/asm-arm/cache.h
> index 2de6564..af96eee 100644
> --- a/xen/include/asm-arm/cache.h
> +++ b/xen/include/asm-arm/cache.h
> @@ -7,6 +7,10 @@
> #define L1_CACHE_SHIFT (CONFIG_ARM_L1_CACHE_SHIFT)
> #define L1_CACHE_BYTES (1 << L1_CACHE_SHIFT)
>
> +#ifndef __ASSEMBLY__
> +void __flush_dcache_area(const void *vaddr, unsigned long size);
> +#endif
> +
> #define __read_mostly __section(".data.read_mostly")
>
> #endif
> diff --git a/xen/include/asm-arm/gic-its.h b/xen/include/asm-arm/gic-its.h
> index 2f5c51c..48c6c78 100644
> --- a/xen/include/asm-arm/gic-its.h
> +++ b/xen/include/asm-arm/gic-its.h
> @@ -36,12 +36,32 @@ extern struct list_head host_its_list;
> /* Parse the host DT and pick up all host ITSes. */
> void gicv3_its_dt_init(const struct dt_device_node *node);
>
> +/* Allocate and initialize tables for each host redistributor.
> + * Returns the respective {PROP,PEND}BASER register value.
> + */
> +uint64_t gicv3_lpi_get_proptable(void);
> +uint64_t gicv3_lpi_allocate_pendtable(void);
> +
> +/* Initialize the host structures for LPIs. */
> +int gicv3_lpi_init_host_lpis(int nr_lpis);
> +
> #else
>
> static inline void gicv3_its_dt_init(const struct dt_device_node *node)
> {
> }
> -
> +static inline uint64_t gicv3_lpi_get_proptable(void)
> +{
> + return 0;
> +}
> +static inline uint64_t gicv3_lpi_allocate_pendtable(void)
> +{
> + return 0;
> +}
> +static inline int gicv3_lpi_init_host_lpis(int nr_lpis)
> +{
> + return 0;
> +}
> #endif /* CONFIG_HAS_ITS */
>
> #endif /* __ASSEMBLY__ */
> diff --git a/xen/include/asm-arm/gic_v3_defs.h
> b/xen/include/asm-arm/gic_v3_defs.h
> index 6bd25a5..da5fb77 100644
> --- a/xen/include/asm-arm/gic_v3_defs.h
> +++ b/xen/include/asm-arm/gic_v3_defs.h
> @@ -44,7 +44,8 @@
> #define GICC_SRE_EL2_ENEL1 (1UL << 3)
>
> /* Additional bits in GICD_TYPER defined by GICv3 */
> -#define GICD_TYPE_ID_BITS_SHIFT 19
> +#define GICD_TYPE_ID_BITS_SHIFT 19
> +#define GICD_TYPE_LPIS (1U << 17)
>
> #define GICD_CTLR_RWP (1UL << 31)
> #define GICD_CTLR_ARE_NS (1U << 4)
> @@ -95,12 +96,57 @@
> #define GICR_IGRPMODR0 (0x0D00)
> #define GICR_NSACR (0x0E00)
>
> +#define GICR_CTLR_ENABLE_LPIS (1U << 0)
> #define GICR_TYPER_PLPIS (1U << 0)
> #define GICR_TYPER_VLPIS (1U << 1)
> #define GICR_TYPER_LAST (1U << 4)
>
> +#define GIC_BASER_CACHE_nCnB 0ULL
> +#define GIC_BASER_CACHE_SameAsInner 0ULL
> +#define GIC_BASER_CACHE_nC 1ULL
> +#define GIC_BASER_CACHE_RaWt 2ULL
> +#define GIC_BASER_CACHE_RaWb 3ULL
> +#define GIC_BASER_CACHE_WaWt 4ULL
> +#define GIC_BASER_CACHE_WaWb 5ULL
> +#define GIC_BASER_CACHE_RaWaWt 6ULL
> +#define GIC_BASER_CACHE_RaWaWb 7ULL
> +#define GIC_BASER_CACHE_MASK 7ULL
> +#define GIC_BASER_NonShareable 0ULL
> +#define GIC_BASER_InnerShareable 1ULL
> +#define GIC_BASER_OuterShareable 2ULL
> +
> +#define GICR_PROPBASER_SHAREABILITY_SHIFT 10
> +#define GICR_PROPBASER_INNER_CACHEABILITY_SHIFT 7
> +#define GICR_PROPBASER_OUTER_CACHEABILITY_SHIFT 56
> +#define GICR_PROPBASER_SHAREABILITY_MASK \
> + (3UL << GICR_PROPBASER_SHAREABILITY_SHIFT)
> +#define GICR_PROPBASER_INNER_CACHEABILITY_MASK \
> + (7UL << GICR_PROPBASER_INNER_CACHEABILITY_SHIFT)
> +#define GICR_PROPBASER_OUTER_CACHEABILITY_MASK \
> + (7UL << GICR_PROPBASER_OUTER_CACHEABILITY_SHIFT)
> +#define PROPBASER_RES0_MASK \
> + (GENMASK(63, 59) | GENMASK(55, 52) | GENMASK(6, 5))
> +
> +#define GICR_PENDBASER_SHAREABILITY_SHIFT 10
> +#define GICR_PENDBASER_INNER_CACHEABILITY_SHIFT 7
> +#define GICR_PENDBASER_OUTER_CACHEABILITY_SHIFT 56
> +#define GICR_PENDBASER_SHAREABILITY_MASK \
> + (3UL << GICR_PENDBASER_SHAREABILITY_SHIFT)
> +#define GICR_PENDBASER_INNER_CACHEABILITY_MASK \
> + (7UL << GICR_PENDBASER_INNER_CACHEABILITY_SHIFT)
> +#define GICR_PENDBASER_OUTER_CACHEABILITY_MASK \
> + (7UL << GICR_PENDBASER_OUTER_CACHEABILITY_SHIFT)
> +#define GICR_PENDBASER_PTZ BIT(62)
> +#define PENDBASER_RES0_MASK \
> + (BIT(63) | GENMASK(61, 59) | GENMASK(55, 52) | \
> + GENMASK(15, 12) | GENMASK(6, 0))
> +
> #define DEFAULT_PMR_VALUE 0xff
>
> +#define LPI_PROP_DEFAULT_PRIO 0xa0
> +#define LPI_PROP_RES1 (1 << 1)
> +#define LPI_PROP_ENABLED (1 << 0)
> +
> #define GICH_VMCR_EOI (1 << 9)
> #define GICH_VMCR_VENG1 (1 << 1)
>
> --
> 2.9.0
>
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |