| 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: <17d3adc7d7d329cab65b54ce71db05bc070872d1.camel@nxp.com>
Date: Wed, 08 Jun 2022 19:29:31 +0800
From: Liu Ying <victor.liu@....com>
To: Lucas Stach <l.stach@...gutronix.de>, linux-kernel@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org
Cc: Thomas Gleixner <tglx@...utronix.de>,
Marc Zyngier <maz@...nel.org>, Shawn Guo <shawnguo@...nel.org>,
Sascha Hauer <s.hauer@...gutronix.de>,
Pengutronix Kernel Team <kernel@...gutronix.de>,
Fabio Estevam <festevam@...il.com>,
NXP Linux Team <linux-imx@....com>
Subject: Re: [PATCH] irqchip/irq-imx-irqsteer: Get/put PM runtime in
->irq_unmask()/irq_mask()
On Wed, 2022-06-08 at 12:56 +0200, Lucas Stach wrote:
> Am Mittwoch, dem 08.06.2022 um 18:50 +0800 schrieb Liu Ying:
> > Now that runtime PM support was added in this driver, we have
> > to enable power before accessing irqchip registers. And, after
> > the access is done, we should disable power. This patch calls
> > pm_runtime_get_sync() in ->irq_unmask() and pm_runtime_put() in
> > ->irq_mask() to make sure power is managed for the register access.
> >
>
> Can you tell me in which case this is necessary? IIRC the IRQ core
With the i.MX8qxp DPU driver[1], I see below synchronous external
abort:
[ 1.207270] Internal error: synchronous external abort: 96000210
[#1] PREEMPT SMP
[ 1.207287] Modules linked in:
[ 1.207299] CPU: 1 PID: 64 Comm: kworker/u8:2 Not tainted 5.18.0-
rc6-next-20220509-00053-gf01f74ee1c18 #272
[ 1.207311] Hardware name: Freescale i.MX8QXP MEK (DT)
[ 1.207319] Workqueue: events_unbound deferred_probe_work_func
[ 1.207339] pstate: 400000c5 (nZcv daIF -PAN -UAO -TCO -DIT -SSBS
BTYPE=--)
[ 1.207349] pc : imx_irqsteer_irq_unmask+0x48/0x80
[ 1.207360] lr : imx_irqsteer_irq_unmask+0x38/0x80
[ 1.207368] sp : ffff80000a88b900
[ 1.207372] x29: ffff80000a88b900 x28: ffff8000080fed90 x27:
ffff8000080fefe0
[ 1.207388] x26: ffff8000080fef40 x25: ffff0008012538d4 x24:
ffff8000092fe388
[ 1.207407] x23: 0000000000000001 x22: ffff0008013295b4 x21:
ffff000801329580
[ 1.207425] x20: ffff0008003faa60 x19: 000000000000000e x18:
0000000000000000
[ 1.207443] x17: 0000000000000003 x16: 0000000000000162 x15:
0000000000000001
[ 1.207459] x14: 0000000000000002 x13: 0000000000000018 x12:
0000000000000040
[ 1.207477] x11: ffff000800682480 x10: ffff000800682482 x9 :
ffff80000a072678
[ 1.207495] x8 : ffff0008006a64a8 x7 : 0000000000000000 x6 :
ffff0008006a6608
[ 1.207513] x5 : ffff800009070a18 x4 : 0000000000000000 x3 :
ffff80000b240000
[ 1.207529] x2 : ffff80000b240038 x1 : 00000000000000c0 x0 :
00000000000000c0
[ 1.207549] Call trace:
[ 1.207553] imx_irqsteer_irq_unmask+0x48/0x80
[ 1.207562] irq_enable+0x40/0x8c
[ 1.207575] __irq_startup+0x78/0xa4
[ 1.207588] irq_startup+0x78/0x16c
[ 1.207601] irq_activate_and_startup+0x38/0x70
[ 1.207612] __irq_do_set_handler+0xcc/0x1e0
[ 1.207626] irq_set_chained_handler_and_data+0x58/0xa0
[ 1.207642] dpu_core_probe+0x368/0xbd4
[ 1.207653] platform_probe+0x68/0xe0
[ 1.207667] really_probe.part.0+0x9c/0x28c
[ 1.207678] __driver_probe_device+0x98/0x144
[ 1.207692] driver_probe_device+0xac/0x140
[ 1.207704] __device_attach_driver+0xb4/0x120
[ 1.207716] bus_for_each_drv+0x78/0xd0
[ 1.207727] __device_attach+0xdc/0x184
[ 1.207739] device_initial_probe+0x14/0x20
[ 1.207749] bus_probe_device+0x9c/0xa4
[ 1.207762] deferred_probe_work_func+0x88/0xc0
[ 1.207774] process_one_work+0x1d0/0x320
[ 1.207788] worker_thread+0x2c8/0x444
[ 1.207799] kthread+0x10c/0x110
[ 1.207812] ret_from_fork+0x10/0x20
[ 1.207829] Code: f94002a3 531e7662 aa0003e1 8b22c062 (b9400040)
[ 1.207839] ---[ end trace 0000000000000000 ]---
DPU DT node references an imx-irqsteer DT node as the interrupt parent.
The DPU driver adds an irq domain by itself.
[1]
https://patchwork.kernel.org/project/dri-devel/patch/20220407091156.1211923-6-victor.liu@nxp.com/
Regards,
Liu Ying
> already keeps the chip runtime resumed as soon as a IRQ is requested,
> so why would it be in runtime suspend at mask/unmask?
>
> Regards,
> Lucas
>
> > Fixes: 4730d2233311 ("irqchip/imx-irqsteer: Add runtime PM
> > support")
> > Cc: Thomas Gleixner <tglx@...utronix.de>
> > Cc: Marc Zyngier <maz@...nel.org>
> > Cc: Shawn Guo <shawnguo@...nel.org>
> > Cc: Sascha Hauer <s.hauer@...gutronix.de>
> > Cc: Pengutronix Kernel Team <kernel@...gutronix.de>
> > Cc: Fabio Estevam <festevam@...il.com>
> > Cc: NXP Linux Team <linux-imx@....com>
> > Cc: Lucas Stach <l.stach@...gutronix.de>
> > Signed-off-by: Liu Ying <victor.liu@....com>
> > ---
> > drivers/irqchip/irq-imx-irqsteer.c | 6 ++++++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/drivers/irqchip/irq-imx-irqsteer.c
> > b/drivers/irqchip/irq-imx-irqsteer.c
> > index 96230a04ec23..a5eabe71e8ab 100644
> > --- a/drivers/irqchip/irq-imx-irqsteer.c
> > +++ b/drivers/irqchip/irq-imx-irqsteer.c
> > @@ -45,11 +45,14 @@ static int imx_irqsteer_get_reg_index(struct
> > irqsteer_data *data,
> >
> > static void imx_irqsteer_irq_unmask(struct irq_data *d)
> > {
> > + struct device *dev = d->domain->dev;
> > struct irqsteer_data *data = d->chip_data;
> > int idx = imx_irqsteer_get_reg_index(data, d->hwirq);
> > unsigned long flags;
> > u32 val;
> >
> > + pm_runtime_get_sync(dev);
> > +
> > raw_spin_lock_irqsave(&data->lock, flags);
> > val = readl_relaxed(data->regs + CHANMASK(idx, data->reg_num));
> > val |= BIT(d->hwirq % 32);
> > @@ -59,6 +62,7 @@ static void imx_irqsteer_irq_unmask(struct
> > irq_data *d)
> >
> > static void imx_irqsteer_irq_mask(struct irq_data *d)
> > {
> > + struct device *dev = d->domain->dev;
> > struct irqsteer_data *data = d->chip_data;
> > int idx = imx_irqsteer_get_reg_index(data, d->hwirq);
> > unsigned long flags;
> > @@ -69,6 +73,8 @@ static void imx_irqsteer_irq_mask(struct irq_data
> > *d)
> > val &= ~BIT(d->hwirq % 32);
> > writel_relaxed(val, data->regs + CHANMASK(idx, data->reg_num));
> > raw_spin_unlock_irqrestore(&data->lock, flags);
> > +
> > + pm_runtime_put(dev);
> > }
> >
> > static const struct irq_chip imx_irqsteer_irq_chip = {
>
>
Powered by blists - more mailing lists