[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v3 02/26] ARM: GICv3: allocate LPI pending and property table
Hi Andre, On 31/03/17 19:05, Andre Przywara wrote: diff --git a/xen/arch/arm/gic-v3-lpi.c b/xen/arch/arm/gic-v3-lpi.c new file mode 100644 index 0000000..77f6009 --- /dev/null +++ b/xen/arch/arm/gic-v3-lpi.c @@ -0,0 +1,209 @@ [...] +#include <xen/lib.h> +#include <xen/mm.h> +#include <xen/sizes.h> +#include <asm/gic.h> +#include <asm/gic_v3_defs.h> +#include <asm/gic_v3_its.h> +#include <asm/io.h> +#include <asm/page.h> + +#define LPI_PROPTABLE_NEEDS_FLUSHING (1U << 0) NIT: newline here. +/* Global state */ +static struct { + /* The global LPI property table, shared by all redistributors. */ + uint8_t *lpi_property; + /* + * Number of physical LPIs the host supports. This is a property of + * the GIC hardware. We depart from the habit of naming these things + * "physical" in Xen, as the GICv3/4 spec uses the term "physical LPI" + * in a different context to differentiate them from "virtual LPIs". + */ + unsigned long int nr_host_lpis; On v2, you said you will rename this variable to max_host_lpi_ids and ... + unsigned int flags; +} lpi_data; + +struct lpi_redist_data { + void *pending_table; +}; + +static DEFINE_PER_CPU(struct lpi_redist_data, lpi_redist); + +#define MAX_PHYS_LPIS (lpi_data.nr_host_lpis - LPI_OFFSET) ... this one to MAX_NR_PHYS_LPIS or even MAX_NR_HOST_LPIS to stay consistent. So please do it. + +static int gicv3_lpi_allocate_pendtable(uint64_t *reg) +{ + uint64_t val; + void *pendtable; + + if ( this_cpu(lpi_redist).pending_table ) + return -EBUSY; + + val = GIC_BASER_CACHE_RaWaWb << GICR_PENDBASER_INNER_CACHEABILITY_SHIFT; + val |= GIC_BASER_CACHE_SameAsInner << GICR_PENDBASER_OUTER_CACHEABILITY_SHIFT; + val |= GIC_BASER_InnerShareable << GICR_PENDBASER_SHAREABILITY_SHIFT; + + /* + * The pending table holds one bit per LPI and even covers bits for + * interrupt IDs below 8192, so we allocate the full range. + * The GICv3 imposes a 64KB alignment requirement, also requires + * physically contiguous memory. + */ + pendtable = _xzalloc(lpi_data.nr_host_lpis / 8, SZ_64K); + if ( !pendtable ) + return -ENOMEM; + + /* Make sure the physical address can be encoded in the register. */ + if ( (virt_to_maddr(pendtable) & ~GENMASK_ULL(51, 16)) ) NIT the middle ( ... ) are not necessary. [...] +static int gicv3_lpi_set_proptable(void __iomem * rdist_base) +{ + uint64_t reg; + + reg = GIC_BASER_CACHE_RaWaWb << GICR_PROPBASER_INNER_CACHEABILITY_SHIFT; + reg |= GIC_BASER_CACHE_SameAsInner << GICR_PROPBASER_OUTER_CACHEABILITY_SHIFT; + reg |= GIC_BASER_InnerShareable << GICR_PROPBASER_SHAREABILITY_SHIFT; + + /* + * The property table is shared across all redistributors, so allocate + * this only once, but return the same value on subsequent calls. + */ + if ( !lpi_data.lpi_property ) + { + /* The property table holds one byte per LPI. */ + void *table = _xmalloc(lpi_data.nr_host_lpis, SZ_4K); + + if ( !table ) + return -ENOMEM; + + /* Make sure the physical address can be encoded in the register. */ + if ( (virt_to_maddr(table) & ~GENMASK_ULL(51, 12)) ) + { + xfree(table); + return -ERANGE; + } + memset(table, GIC_PRI_IRQ | LPI_PROP_RES1, MAX_PHYS_LPIS); + clean_and_invalidate_dcache_va_range(table, MAX_PHYS_LPIS); + lpi_data.lpi_property = table; + } + + /* Encode the number of bits needed, minus one */ + reg |= (fls(lpi_data.nr_host_lpis - 1) - 1); NIT: The outer ( ... ) are not necessary. [...] +int gicv3_lpi_init_rdist(void __iomem * rdist_base) +{ + uint32_t reg; + uint64_t table_reg; + int ret; + + /* We don't support LPIs without an ITS. */ + if ( !gicv3_its_host_has_its() ) + return -ENODEV; + + /* Make sure LPIs are disabled before setting up the tables. */ + reg = readl_relaxed(rdist_base + GICR_CTLR); + if ( reg & GICR_CTLR_ENABLE_LPIS ) + return -EBUSY; + + ret = gicv3_lpi_allocate_pendtable(&table_reg); + if (ret) Coding style: if ( ... ) [...] +static unsigned int max_lpi_bits = 20; +integer_param("max_lpi_bits", max_lpi_bits); + +int gicv3_lpi_init_host_lpis(unsigned int hw_lpi_bits) +{ Again, please add a comment to explain why you don't sanitize the value from the command line. + lpi_data.nr_host_lpis = BIT_ULL(min(hw_lpi_bits, max_lpi_bits)); Again, nr_host_lpis is "unsigned long" so why are you using BIT_ULL? Looking at the introduction of GENMASK_ULL, it likely means nr_host_lpis should be unsigned long long. + + printk("GICv3: using at most %lu LPIs on the host.\n", MAX_PHYS_LPIS); + + return 0; +} [...] diff --git a/xen/include/asm-arm/gic_v3_defs.h b/xen/include/asm-arm/gic_v3_defs.h index 6bd25a5..7cdebc5 100644 --- a/xen/include/asm-arm/gic_v3_defs.h +++ b/xen/include/asm-arm/gic_v3_defs.h @@ -44,7 +44,10 @@ #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_ID_BITS(r) ((((r) >> GICD_TYPE_ID_BITS_SHIFT) & 0x1f) + 1) Please align with the rest. [...] diff --git a/xen/include/asm-arm/gic_v3_its.h b/xen/include/asm-arm/gic_v3_its.h index 765a655..219d109 100644 --- a/xen/include/asm-arm/gic_v3_its.h +++ b/xen/include/asm-arm/gic_v3_its.h @@ -40,6 +40,11 @@ void gicv3_its_dt_init(const struct dt_device_node *node); bool gicv3_its_host_has_its(void); +int gicv3_lpi_init_rdist(void __iomem * rdist_base); + +/* Initialize the host structures for LPIs. */ +int gicv3_lpi_init_host_lpis(unsigned int nr_lpis); Again, in the implementation, the parameter is called "hw_lpi_bits". Please stay consistent. + #else static LIST_HEAD(host_its_list); @@ -53,6 +58,15 @@ static inline bool gicv3_its_host_has_its(void) return false; } +static inline int gicv3_lpi_init_rdist(void __iomem * rdist_base) +{ + return -ENODEV; +} + +static inline int gicv3_lpi_init_host_lpis(unsigned int nr_lpis) Ditto. +{ + return 0; +} #endif /* CONFIG_HAS_ITS */ #endif diff --git a/xen/include/asm-arm/irq.h b/xen/include/asm-arm/irq.h index 8f7a167..13528c0 100644 --- a/xen/include/asm-arm/irq.h +++ b/xen/include/asm-arm/irq.h @@ -19,8 +19,16 @@ struct arch_irq_desc { }; #define NR_LOCAL_IRQS 32 + +/* + * This only covers the interrupts that Xen cares about, so SGIs, PPIs and + * SPIs. LPIs are too numerous, also only propagated to guests, so they are + * not included in this number. + */ #define NR_IRQS 1024 +#define LPI_OFFSET 8192 + #define nr_irqs NR_IRQS #define nr_static_irqs NR_IRQS #define arch_hwdom_irqs(domid) NR_IRQS diff --git a/xen/include/xen/bitops.h b/xen/include/xen/bitops.h index bd0883a..9261e06 100644 --- a/xen/include/xen/bitops.h +++ b/xen/include/xen/bitops.h @@ -5,11 +5,14 @@ /* * Create a contiguous bitmask starting at bit position @l and ending at * position @h. For example - * GENMASK(30, 21) gives us the 32bit vector 0x01fe00000. + * GENMASK(30, 21) gives us the 32bit vector 0x7fe00000. This is a new addition in this series and should really be a separate patch with the appropriate maintainers CCed. */ #define GENMASK(h, l) \ (((~0UL) << (l)) & (~0UL >> (BITS_PER_LONG - 1 - (h)))) +#define GENMASK_ULL(h, l) \ + (((~0ULL) << (l)) & (~0ULL >> (BITS_PER_LONG_LONG - 1 - (h)))) This should also be a separate patch with BITS_PER_LONG_LONG too. + /* * ffs: find first bit set. This is defined the same way as * the libc and compiler builtin ffs routines, therefore Cheers, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx https://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |