Re: [PATCH v3 2/2] genirq: fasteoi supports resend on concurrent invoke

From: Marc Zyngier
Date: Wed Jun 07 2023 - 09:13:16 EST


On 2023-06-07 13:21, Gowans, James wrote:
On Tue, 2023-06-06 at 18:05 +0100, Marc Zyngier wrote:

On Mon, 05 Jun 2023 16:57:23 +0100,
James Gowans <jgowans@xxxxxxxxxx> wrote:
> ... and enable that functionality for GIC-v3 only.

nit: drop the multi-line subject.

Would you prefer two commits - one to introduce the functionality and one
to enable it for GIC-v3?

I'd prefer that. It is in general better to decouple driver stuff from
core code.

diff --git a/include/linux/irq.h b/include/linux/irq.h
> index b1b28affb32a..b76cc90faebd 100644
> --- a/include/linux/irq.h
> +++ b/include/linux/irq.h
> @@ -223,6 +223,8 @@ struct irq_data {
> * irq_chip::irq_set_affinity() when deactivated.
> * IRQD_IRQ_ENABLED_ON_SUSPEND - Interrupt is enabled on suspend by irq pm if
> * irqchip have flag IRQCHIP_ENABLE_WAKEUP_ON_SUSPEND set.
> + * RQD_RESEND_WHEN_IN_PROGRESS - Interrupt may fire when already in progress in which
> + * case it must be resent at the next available opportunity.
> */
> enum {
> IRQD_TRIGGER_MASK = 0xf,
> @@ -249,6 +251,7 @@ enum {
> IRQD_HANDLE_ENFORCE_IRQCTX = (1 << 28),
> IRQD_AFFINITY_ON_ACTIVATE = (1 << 29),
> IRQD_IRQ_ENABLED_ON_SUSPEND = (1 << 30),
> + IRQD_RESEND_WHEN_IN_PROGRESS = (1 << 31),

Make this unsigned, as it otherwise has the potential to sign-extend
and lead to "fun to debug" issues.

Ack, doing this change:

@@ -251,7 +251,7 @@ enum {
IRQD_HANDLE_ENFORCE_IRQCTX = (1 << 28),
IRQD_AFFINITY_ON_ACTIVATE = (1 << 29),
IRQD_IRQ_ENABLED_ON_SUSPEND = (1 << 30),
- IRQD_RESEND_WHEN_IN_PROGRESS = (1 << 31),
+ IRQD_RESEND_WHEN_IN_PROGRESS = (1U << 31),
};

(looks a bit odd having only this one unsigned though...)

Eventually, someone will bite the bullet and use BIT() everywhere.

Thanks,

M.
--
Jazz is not dead. It just smells funny...