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

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: <2fc0a230d79414ab2c2027ec02a022e3@kernel.org>
Date:   Wed, 07 Jun 2023 14:13:02 +0100
From:   Marc Zyngier <maz@...nel.org>
To:     "Gowans, James" <jgowans@...zon.com>
Cc:     tglx@...utronix.de, "Raslan, KarimAllah" <karahmed@...zon.com>,
        liaochang1@...wei.com, linux-kernel@...r.kernel.org,
        zouyipeng@...wei.com, chris.zjh@...wei.com
Subject: Re: [PATCH v3 2/2] genirq: fasteoi supports resend on concurrent
 invoke

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@...zon.com> 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...