linux-kernel - RE: [PATCH v2] arm64: kexec: Check if IRQ is already masked before masking

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>] [day] [month] [year] [list]

Message-ID: <3a499ecdfa5042f8b6e8834c47cdb2d9@amazon.com>
Date: Thu, 28 Nov 2024 20:07:57 +0000
From: "Farber, Eliav" <farbere@...zon.com>
To: Thomas Gleixner <tglx@...utronix.de>, "linux@...linux.org.uk"
	<linux@...linux.org.uk>, "catalin.marinas@....com" <catalin.marinas@....com>,
	"will@...nel.org" <will@...nel.org>, "mpe@...erman.id.au"
	<mpe@...erman.id.au>, "npiggin@...il.com" <npiggin@...il.com>,
	"christophe.leroy@...roup.eu" <christophe.leroy@...roup.eu>,
	"naveen@...nel.org" <naveen@...nel.org>, "maddy@...ux.ibm.com"
	<maddy@...ux.ibm.com>, "paul.walmsley@...ive.com" <paul.walmsley@...ive.com>,
	"palmer@...belt.com" <palmer@...belt.com>, "aou@...s.berkeley.edu"
	<aou@...s.berkeley.edu>, "akpm@...ux-foundation.org"
	<akpm@...ux-foundation.org>, "bhe@...hat.com" <bhe@...hat.com>,
	"hbathini@...ux.ibm.com" <hbathini@...ux.ibm.com>, "adityag@...ux.ibm.com"
	<adityag@...ux.ibm.com>, "songshuaishuai@...ylab.org"
	<songshuaishuai@...ylab.org>, "takakura@...inux.co.jp"
	<takakura@...inux.co.jp>, "linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "linuxppc-dev@...ts.ozlabs.org"
	<linuxppc-dev@...ts.ozlabs.org>, "linux-riscv@...ts.infradead.org"
	<linux-riscv@...ts.infradead.org>
CC: "Chocron, Jonathan" <jonnyc@...zon.com>
Subject: RE: [PATCH v2] arm64: kexec: Check if IRQ is already masked before
 masking

On 11/28/2024 12:39 PM, Thomas Gleixner wrote:
> This is just wrong. If the interrupt was torn down, then its state is deactivated and it was masked already. So the EOI handling and the mask/disable dance are neither required nor make sense.
>
> So this whole thing should be:
>
>                 chip = irq_desc_get_chip(desc);
> -               if (!chip)
> +               if (!chip || !irqd_is_started(&desc->irq_data))
>                         continue;
ACK. Will be done this way in V3.

> But what's worse is that we have 4 almost identical variants of the same code.
>
> So instead of exposing core functionality and "fixing" up four variants, can we please have a consolidated version of this function in the core
> code:
>                 struct irq_chip *chip;
>                 int check_eoi = 1;
>
>                 chip = irq_desc_get_chip(desc);
>                 if (!chip || !irqd_is_started(&desc->irq_data))
>                         continue;
>
>                 if (IS_ENABLED(CONFIG_.....)) {
>                         /*
>                          * Add a sensible comment which explains this.
>                          */
>                         check_eoi = irq_set_irqchip_state(....);
>                 }
>
>                 if (check_eoi && ....)
>                         chip->irq_eoi(&desc->irq_data);
>
>                 irq_shutdown(desc);
>
> No?
In V3 I will add a preliminary patch that will remove the four variants
and instead add a common implementations to the kexec core.

Thanks, Eliav