| 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
| ||
|
Message-ID: <CANBLGcwwrqkYS2cxX5dYAaoWdj5pRp9c+qBDAMb3=0D5oBD+Zg@mail.gmail.com>
Date: Thu, 10 Feb 2022 13:59:59 +0100
From: Emil Renner Berthing <kernel@...il.dk>
To: Marc Zyngier <maz@...nel.org>
Cc: Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
"open list:GPIO SUBSYSTEM" <linux-gpio@...r.kernel.org>,
Linus Walleij <linus.walleij@...aro.org>,
Bartosz Golaszewski <brgl@...ev.pl>,
Matthias Brugger <matthias.bgg@...il.com>,
Grygorii Strashko <grygorii.strashko@...com>,
Santosh Shilimkar <ssantosh@...nel.org>,
Kevin Hilman <khilman@...nel.org>,
Tony Lindgren <tony@...mide.com>,
Thomas Gleixner <tglx@...utronix.de>,
Vladimir Zapolskiy <vz@...ia.com>,
Andrew Lunn <andrew@...n.ch>,
Gregory Clement <gregory.clement@...tlin.com>,
Sebastian Hesselbarth <sebastian.hesselbarth@...il.com>,
kernel-team@...roid.com
Subject: Re: [PATCH 10/10] pinctrl: starfive: Switch to dynamic chip name output
On Thu, 10 Feb 2022 at 10:06, Marc Zyngier <maz@...nel.org> wrote:
> On Wed, 09 Feb 2022 23:30:55 +0000,
> Emil Renner Berthing <kernel@...il.dk> wrote:
> >
> > On Wed, 9 Feb 2022 at 17:49, Marc Zyngier <maz@...nel.org> wrote:
> > >
> > > Instead of overloading the name field, use the relevant callback to
> > > output the device name.
> > >
> > > Signed-off-by: Marc Zyngier <maz@...nel.org>
> > > ---
> > > drivers/pinctrl/pinctrl-starfive.c | 11 +++++++++--
> > > 1 file changed, 9 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/pinctrl/pinctrl-starfive.c b/drivers/pinctrl/pinctrl-starfive.c
> > > index 5be9866c2b3c..f29d9ccf858b 100644
> > > --- a/drivers/pinctrl/pinctrl-starfive.c
> > > +++ b/drivers/pinctrl/pinctrl-starfive.c
> > > @@ -15,6 +15,7 @@
> > > #include <linux/of.h>
> > > #include <linux/platform_device.h>
> > > #include <linux/reset.h>
> > > +#include <linux/seq_file.h>
> > > #include <linux/spinlock.h>
> > >
> > > #include <linux/pinctrl/pinctrl.h>
> > > @@ -1163,12 +1164,20 @@ static int starfive_irq_set_type(struct irq_data *d, unsigned int trigger)
> > > return 0;
> > > }
> > >
> > > +static void starfive_irq_print_chip(struct irq_data *d, struct seq_file *p)
> > > +{
> > > + struct starfive_pinctrl *sfp = starfive_from_irq_data(d);
> > > +
> > > + seq_printf(p, sfp->gc.label);
> > > +}
> > > +
> > > static struct irq_chip starfive_irq_chip = {
> > > .irq_ack = starfive_irq_ack,
> > > .irq_mask = starfive_irq_mask,
> > > .irq_mask_ack = starfive_irq_mask_ack,
> > > .irq_unmask = starfive_irq_unmask,
> > > .irq_set_type = starfive_irq_set_type,
> > > + .irq_print_chip = starfive_irq_print_chip,
> > > .flags = IRQCHIP_SET_TYPE_MASKED,
> > > };
> >
> > The parent interrupt doesn't show up in /proc/interrupts anyway, so if
> > setting the name is considered abuse we can just drop the addition
> > above and just delete the two lines below.
>
> Are you sure this never appears? Is there a another irqchip stacked on
> top of this one? Could you please dump /sys/kernel/debug/irq/irqs/XX,
> where XX is an interrupt number using one of these GPIO pins? Please
> run it without this patch, as I just noticed that debugfs blindly
> uses the name.
Yes, the old gpio driver this derives from used to set
sfp->gc.irq.parent_handler = NULL
and then register its own irq handler, then the parent would show up
in /proc/interrupts. But after switching to letting the gpio framework
register the handler it stopped showing up.
root@...ionfive~# cat /proc/interrupts
CPU0 CPU1
5: 5035 4907 RISC-V INTC 5 Edge riscv-timer
6: 960 0 SiFive PLIC 4 Edge dw-mci
7: 4384 0 SiFive PLIC 5 Edge dw-mci
8: 562 0 SiFive PLIC 6 Edge eth0
10: 1 0 SiFive PLIC 7 Edge eth0
11: 0 0 SiFive PLIC 2 Edge dw_axi_dmac_platform
15: 2690 0 SiFive PLIC 44 Edge xhci-hcd:usb1
17: 0 0 SiFive PLIC 43 Edge 104c0000.usb
18: 0 0 SiFive PLIC 1 Edge dw_axi_dmac_platform
20: 234 0 SiFive PLIC 96 Edge 118b0000.i2c
21: 0 0 SiFive PLIC 97 Edge 118c0000.i2c
22: 7 0 SiFive PLIC 98 Edge 118d0000.trng
28: 0 0 SiFive PLIC 101 Edge sf_lcdc
29: 0 0 SiFive PLIC 103 Edge sf_vpp1
30: 0 0 SiFive PLIC 70 Edge 12410000.spi
31: 205 0 SiFive PLIC 73 Edge ttyS0
32: 0 0 SiFive PLIC 74 Edge 12450000.i2c
33: 0 0 SiFive PLIC 80 Edge 12480000.watchdog
34: 28 0 SiFive PLIC 122 Edge 124a0000.tmon
37: 0 0 11910000.pinctrl 35 Edge gpiomon
IPI0: 52 133 Rescheduling interrupts
IPI1: 2591 6924 Function call interrupts
IPI2: 0 0 CPU stop interrupts
IPI3: 0 0 IRQ work interrupts
IPI4: 0 0 Timer broadcast interrupts
root@...ionfive~# cat /sys/kernel/debug/irq/irqs/26
handler: starfive_gpio_irq_handler
device: (null)
status: 0x00010c00
_IRQ_NOPROBE
_IRQ_NOREQUEST
_IRQ_NOTHREAD
istate: 0x00000000
ddepth: 0
wdepth: 0
dstate: 0x02401200
IRQD_ACTIVATED
IRQD_IRQ_STARTED
IRQD_AFFINITY_SET
IRQD_DEFAULT_TRIGGER_SET
node: 0
affinity: 0-1
domain: :soc:interrupt-controller@...0000
hwirq: 0x20
chip: SiFive PLIC
flags: 0x0
root@...ionfive~# cat /sys/kernel/debug/irq/irqs/37
handler: handle_edge_irq
device: (null)
status: 0x00000403
_IRQ_NOPROBE
istate: 0x00000020
IRQS_ONESHOT
ddepth: 0
wdepth: 0
dstate: 0x00400203
IRQ_TYPE_EDGE_RISING
IRQ_TYPE_EDGE_FALLING
IRQD_ACTIVATED
IRQD_IRQ_STARTED
node: 0
affinity: 0-1
domain: :soc:pinctrl@...10000
hwirq: 0x23
chip: 11910000.pinctrl
flags: 0x1
IRQCHIP_SET_TYPE_MASKED
> > The gpio framework seems to fill in default handlers in the struct
> > above, so unfortunately it can't yet be made const. Is this something
> > you intend to fix in the future?
>
> This is next on my list of things to address. The whole 'let's copy a
> whole irqchip structure and hijack random pointers' should not have
> happened, and it certainly is going to be an interesting ride.
Sounds great, thanks.
/Emil
Powered by blists - more mailing lists