linux-kernel - Re: realtek,rtl-intc IRQ mapping broken on 5.16-rc1

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: <fdfe6615a0ec0d4a770b04a437922956e8586078.camel@svanheule.net>
Date:   Thu, 18 Nov 2021 20:45:26 +0100
From:   Sander Vanheule <sander@...nheule.net>
To:     Marc Zyngier <maz@...nel.org>
Cc:     Rob Herring <robh+dt@...nel.org>,
        Lorenzo Pieralisi <lorenzo.pieralisi@....com>,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        Birger Koblitz <mail@...ger-koblitz.de>,
        Bert Vermeulen <bert@...t.com>,
        John Crispin <john@...ozen.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
        Frank Rowand <frowand.list@...il.com>
Subject: Re: realtek,rtl-intc IRQ mapping broken on 5.16-rc1

Hi Marc,

On Thu, 2021-11-18 at 19:19 +0000, Marc Zyngier wrote:
> Hi Sander,
> 
> On Thu, 18 Nov 2021 15:56:06 +0000,
> Sander Vanheule <sander@...nheule.net> wrote:
> > 
> > Hi everyone,
> > 
> > On 5.16-rc1, the realtek,rtl-intc interrupt controller driver for
> > Realtek RTL8380 SoCs (and related) appears broken. When booting, I
> > don't get a tty on the serial port, although serial output works.
> 
> Thanks for the heads up.
> 
> > The watchdog (currently under review) also cannot acquire the
> > required phase1 interrupt, and produces the following output:
> 
> > [    1.968228] realtek-otto-watchdog 18003150.watchdog: error -EINVAL: Failed to get
> > IRQ 4
> > for phase1
> > [    1.978404] realtek-otto-watchdog: probe of 18003150.watchdog failed with error -22
> > 
> > A bisects points to commit 041284181226 ("of/irq: Allow matching of
> > an interrupt-map local to an interrupt controller"). Reverting this
> > above commit and follow-up commit 10a20b34d735 ("of/irq: Don't
> > ignore interrupt-controller when interrupt-map failed") restores the
> > functionality from v5.15.
> 
> OK, back to square one, we need to debug this one.
> 
> [...]
> 
> >         cpuintc: cpuintc {
> >                 compatible = "mti,cpu-interrupt-controller";
> >                 #address-cells = <0>;
> >                 #interrupt-cells = <1>;
> >                 interrupt-controller;
> >         };
> > 
> 
> [...]
> 
> > 
> >                 intc: interrupt-controller@...0 {
> >                         compatible = "realtek,rtl-intc";
> >                         reg = <0x3000 0x20>;
> >                         interrupt-controller;
> >                         #interrupt-cells = <1>;
> > 
> >                         #address-cells = <0>;
> >                         interrupt-map =
> >                                 <31 &cpuintc 2>, /* UART0 */
> >                                 <20 &cpuintc 3>, /* SWCORE */
> >                                 <19 &cpuintc 4>, /* WDT IP1 */
> >                                 <18 &cpuintc 5>; /* WDT IP2 */
> >                 };
> 
> Something looks pretty odd. With 5.15, this interrupt-map would be
> completely ignored. With 5.16-rc1, we should actually honour it.
> 
> /me digs...
> 
> Gah, I see. This driver has its own interrupt-map parser and invents
> something out of thin air. I will bang my own head on the wall for
> having merged this horror.
> 
> Can you try applying the patch below and rename the interrupt-map
> property in your DT to "silly-interrupt-map" and let me know if that
> helps?

I've dropped the aforementioned reverts, and applied your suggested changes to the DTS and
irq-realtek-rtl. Interrupts now appear to work like before; UART console and watchdog work
as expected.

Best,
Sander

> 
> That's of course not the right fix, but that's just to confirm the
> extent of the damage...
> 
>         M.
> 
> diff --git a/drivers/irqchip/irq-realtek-rtl.c b/drivers/irqchip/irq-realtek-rtl.c
> index fd9f275592d2..3641cd2b1a2c 100644
> --- a/drivers/irqchip/irq-realtek-rtl.c
> +++ b/drivers/irqchip/irq-realtek-rtl.c
> @@ -114,7 +114,7 @@ static int __init map_interrupts(struct device_node *node, struct
> irq_domain *do
>         if (ret || tmp)
>                 return -EINVAL;
>  
> -       imap = of_get_property(node, "interrupt-map", &imaplen);
> +       imap = of_get_property(node, "silly-interrupt-map", &imaplen);
>         if (!imap || imaplen % 3)
>                 return -EINVAL;
>  
>