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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <de12cbf0-22ec-83d0-a034-9872f302101b@kernel.org>
Date:   Tue, 23 Jul 2019 14:04:06 +0100
From:   Marc Zyngier <maz@...nel.org>
To:     Lokesh Vutla <lokeshvutla@...com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Jason Cooper <jason@...edaemon.net>,
        Julien Thierry <julien.thierry.kdev@...il.com>,
        Rob Herring <robh+dt@...nel.org>
Cc:     linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH 4/9] irqchip/gic-v3: Add ESPI range support

On 23/07/2019 13:50, Lokesh Vutla wrote:
> 
> 
> 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@...nel.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:

Yes, you're right. Although I strongly suspect that someone will bump
this limit up again at some point (there is enough space in the new
register space for 4k interrupts -- madness).

> 
>> +		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?

What's so special about the special INTID range is that these are not
interrupts at all. That's how the GIC signals that *something else* is
happening (see 2.2.1 "Special INTIDs" in the architecture spec). In
practice, and as long as you run Linux in non-secure mode, only 1023 can
happen.

Thanks,

	M.
-- 
Jazz is not dead, it just smells funny...