Re: [PATCH 4/9] irqchip/gic-v3: Add ESPI range support

Lokesh Vutla
2019-07-23T12:51:13Z

On 23/07/19 4:14 PM, Marc Zyngier wrote:
> Add the required support for the ESPI range, which behave exactly like
> the SPIs of old, only with new funky INTIDs.
>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
> drivers/irqchip/irq-gic-v3.c | 85 ++++++++++++++++++++++++------
> include/linux/irqchip/arm-gic-v3.h | 17 +++++-
> 2 files changed, 85 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
> index 2371e0a70215..d328a8de533f 100644
> --- a/drivers/irqchip/irq-gic-v3.c
> +++ b/drivers/irqchip/irq-gic-v3.c
> @@ -51,13 +51,16 @@ struct gic_chip_data {
> u32 nr_redist_regions;
> u64 flags;
> bool has_rss;
> - unsigned int irq_nr;
> struct partition_desc *ppi_descs[16];
> };
>
> static struct gic_chip_data gic_data __read_mostly;
> static DEFINE_STATIC_KEY_TRUE(supports_deactivate_key);
>
> +#define GIC_ID_NR (1U << GICD_TYPER_ID_BITS(gic_data.rdists.gicd_typer))
> +#define GIC_LINE_NR GICD_TYPER_SPIS(gic_data.rdists.gicd_typer)
> +#define GIC_ESPI_NR GICD_TYPER_ESPIS(gic_data.rdists.gicd_typer)
> +
> /*
> * The behaviours of RPR and PMR registers differ depending on the value of
> * SCR_EL3.FIQ, and the behaviour of non-secure priority registers of the
> @@ -100,6 +103,7 @@ static DEFINE_PER_CPU(bool, has_rss);
> enum gic_intid_range {
> PPI_RANGE,
> SPI_RANGE,
> + ESPI_RANGE,
> LPI_RANGE,
> __INVALID_RANGE__
> };
> @@ -111,6 +115,8 @@ static enum gic_intid_range __get_intid_range(irq_hw_number_t hwirq)
> return PPI_RANGE;
> case 32 ... 1019:
> return SPI_RANGE;
> + case ESPI_BASE_INTID ... 8191:

as per dt documentation, shouldn't the range be
case ESPI_BASE_INTID ... 5119:

> + return ESPI_RANGE;
> case 8192 ... GENMASK(23, 0):
> return LPI_RANGE;
> default:
> @@ -141,6 +147,7 @@ static inline void __iomem *gic_dist_base(struct irq_data *d)
> return gic_data_rdist_sgi_base();
>
> case SPI_RANGE:
> + case ESPI_RANGE:
> /* SPI -> dist_base */
> return gic_data.dist_base;
>
> @@ -234,6 +241,31 @@ static u32 convert_offset_index(struct irq_data *d, u32 offset, u32 *index)
> case SPI_RANGE:
> *index = d->hwirq;
> return offset;
> + case ESPI_RANGE:
> + *index = d->hwirq - ESPI_BASE_INTID;
> + switch (offset) {
> + case GICD_ISENABLER:
> + return GICD_ISENABLERnE;
> + case GICD_ICENABLER:
> + return GICD_ICENABLERnE;
> + case GICD_ISPENDR:
> + return GICD_ISPENDRnE;
> + case GICD_ICPENDR:
> + return GICD_ICPENDRnE;
> + case GICD_ISACTIVER:
> + return GICD_ISACTIVERnE;
> + case GICD_ICACTIVER:
> + return GICD_ICACTIVERnE;
> + case GICD_IPRIORITYR:
> + return GICD_IPRIORITYRnE;
> + case GICD_ICFGR:
> + return GICD_ICFGRnE;
> + case GICD_IROUTER:
> + return GICD_IROUTERnE;
> + default:
> + break;
> + }
> + break;
> default:
> break;
> }
> @@ -316,7 +348,7 @@ static int gic_irq_set_irqchip_state(struct irq_data *d,
> {
> u32 reg;
>
> - if (d->hwirq >= gic_data.irq_nr) /* PPI/SPI only */
> + if (d->hwirq >= 8192) /* PPI/SPI only */
> return -EINVAL;
>
> switch (which) {
> @@ -343,7 +375,7 @@ static int gic_irq_set_irqchip_state(struct irq_data *d,
> static int gic_irq_get_irqchip_state(struct irq_data *d,
> enum irqchip_irq_state which, bool *val)
> {
> - if (d->hwirq >= gic_data.irq_nr) /* PPI/SPI only */
> + if (d->hwirq >= 8192) /* PPI/SPI only */
> return -EINVAL;
>
> switch (which) {
> @@ -567,7 +599,12 @@ static asmlinkage void __exception_irq_entry gic_handle_irq(struct pt_regs *regs
> gic_arch_enable_irqs();
> }
>
> - if (likely(irqnr > 15 && irqnr < 1020) || irqnr >= 8192) {
> + /* Check for special IDs first */
> + if ((irqnr >= 1020 && irqnr <= 1023))
> + return;

May be I am missing something here, what is special about these 4 interrupts? or
you meant to check for reserved range here?

Thanks and regards,
Lokesh