Re: [PATCH v2 2/2] irqchip/gic-v2, v3: Prevent SW resends entirely

From: Valentin Schneider <valentin.schneider@arm.com>
To: Marc Zyngier <maz@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>,
	Lorenzo Pieralisi <Lorenzo.Pieralisi@arm.com>,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	Jason Cooper <jason@lakedaemon.net>
Subject: Re: [PATCH v2 2/2] irqchip/gic-v2, v3: Prevent SW resends entirely
Date: Fri, 31 Jul 2020 01:08:47 +0100	[thread overview]
Message-ID: <jhjmu3gim0g.mognet@arm.com> (raw)
In-Reply-To: <ba26464de5e82eace97924121d7bcd1d@kernel.org>

Hi Marc,

On 30/07/20 19:10, Marc Zyngier wrote:
[...]
>> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
>> index e2b4cae88bce..a91ce1e73bd2 100644
>> --- a/drivers/irqchip/irq-gic.c
>> +++ b/drivers/irqchip/irq-gic.c
>> @@ -983,6 +983,7 @@ static int gic_irq_domain_map(struct irq_domain
>> *d, unsigned int irq,
>>                              irq_hw_number_t hw)
>>  {
>>      struct gic_chip_data *gic = d->host_data;
>> +	struct irq_data *irqd = irq_desc_get_irq_data(irq_to_desc(irq));
>>
>>      if (hw < 32) {
>>              irq_set_percpu_devid(irq);
>> @@ -992,8 +993,11 @@ static int gic_irq_domain_map(struct irq_domain
>> *d, unsigned int irq,
>>              irq_domain_set_info(d, irq, hw, &gic->chip, d->host_data,
>>                                  handle_fasteoi_irq, NULL, NULL);
>>              irq_set_probe(irq);
>> -		irqd_set_single_target(irq_desc_get_irq_data(irq_to_desc(irq)));
>> +		irqd_set_single_target(irqd);
>>      }
>> +
>> +	/* Prevents SW retriggers which mess up the ACK/EOI ordering */
>> +	irqd_set_handle_enforce_irqctx(irqd);
>>      return 0;
>>  }
>
> I'm OK with this in principle, but this requires additional changes
> in the rest of the GIC universe. The ITS driver needs to provide its own
> retrigger function for LPIs (queuing an INT command), and any of the
> SPI generating widgets that can be stacked on top of a GIC (GICv3-MBI,
> GICv2m, and all the other Annapurna/Marvell/NVDIA wonders need to gain
> directly or indirectly a call to irq_chip_retrigger_hierarchy().
>

Eep, yes indeed... I didn't see that can was full of worms, though even if
it only really matters for eoimode=0 I think it might still be worth it
(if only to respect the spec).

> We can probably avoid changing the MSI widgets by teaching the MSI
> code about the HW retrigger, but a number of other non-MSI drivers
> will need some help...
>
> I'll have a look tomorrow.
>

For LPIs AFAICT we could directly reuse its_irq_set_irqchip_state() - I see
the VPE side of things already has a HW retrigger callback.

For gicv2m, I *think* we'd want irq_chip_retrigger_hierarchy() on both MSI
domains (which IIUC you suggest might be doable by adding the retrigger as
a default MSI chip op).

I'm not very familiar with the rest of the fauna, so I'll have to do some
reading tomorrow as well; it's probably high time for me to actually read
up on LPIs & ITS while I'm at it...

> Thanks,
>
>          M.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel