| 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: <SA0PR12MB4510FEE100D5555F655C7EA0E2669@SA0PR12MB4510.namprd12.prod.outlook.com>
Date: Mon, 29 Nov 2021 18:53:01 +0000
From: "Limonciello, Mario" <Mario.Limonciello@....com>
To: Sasha Levin <sashal@...nel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"stable@...r.kernel.org" <stable@...r.kernel.org>
CC: Joerie de Gram <j.de.gram@...il.com>,
"Natikar, Basavaraj" <Basavaraj.Natikar@....com>,
Linus Walleij <linus.walleij@...aro.org>,
"S-k, Shyam-sundar" <Shyam-sundar.S-k@....com>,
"linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>
Subject: RE: [PATCH AUTOSEL 5.10 10/28] pinctrl: amd: Fix wakeups when IRQ is
shared with SCI
> -----Original Message-----
> From: Limonciello, Mario
> Sent: Monday, November 29, 2021 08:48
> To: Sasha Levin <sashal@...nel.org>; linux-kernel@...r.kernel.org;
> stable@...r.kernel.org
> Cc: Joerie de Gram <j.de.gram@...il.com>; Natikar, Basavaraj
> <Basavaraj.Natikar@....com>; Linus Walleij <linus.walleij@...aro.org>; S-k,
> Shyam-sundar <Shyam-sundar.S-k@....com>; linux-gpio@...r.kernel.org
> Subject: RE: [PATCH AUTOSEL 5.10 10/28] pinctrl: amd: Fix wakeups when IRQ
> is shared with SCI
>
>
>
> > -----Original Message-----
> > From: Sasha Levin <sashal@...nel.org>
> > Sent: Thursday, November 25, 2021 20:33
> > To: linux-kernel@...r.kernel.org; stable@...r.kernel.org
> > Cc: Limonciello, Mario <Mario.Limonciello@....com>; Joerie de Gram
> > <j.de.gram@...il.com>; Natikar, Basavaraj
> <Basavaraj.Natikar@....com>;
> > Linus Walleij <linus.walleij@...aro.org>; Sasha Levin <sashal@...nel.org>;
> S-
> > k, Shyam-sundar <Shyam-sundar.S-k@....com>; linux-
> > gpio@...r.kernel.org
> > Subject: [PATCH AUTOSEL 5.10 10/28] pinctrl: amd: Fix wakeups when IRQ
> is
> > shared with SCI
> >
> > From: Mario Limonciello <mario.limonciello@....com>
> >
> > [ Upstream commit 2d54067fcd23aae61e23508425ae5b29e973573d ]
> >
> > On some Lenovo AMD Gen2 platforms the IRQ for the SCI and pinctrl
> drivers
> > are shared. Due to how the s2idle loop handling works, this case needs
> > an extra explicit check whether the interrupt was caused by SCI or by
> > the GPIO controller.
> >
> > To fix this rework the existing IRQ handler function to function as a
> > checker and an IRQ handler depending on the calling arguments.
> >
> > BugLink:
> >
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitla
> > b.freedesktop.org%2Fdrm%2Famd%2F-
> >
> %2Fissues%2F1738&data=04%7C01%7Cmario.limonciello%40amd.com%
> >
> 7Ce14b7ddf8d1143b6649208d9b0853519%7C3dd8961fe4884e608e11a82d994
> >
> e183d%7C0%7C0%7C637734908448086367%7CUnknown%7CTWFpbGZsb3d8
> >
> eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3
> >
> D%7C3000&sdata=BHHY3gLu2pQIJ1nvSE0VNQOaioTC0QdBM44ze3HXrtk
> > %3D&reserved=0
> > Reported-by: Joerie de Gram <j.de.gram@...il.com>
> > Signed-off-by: Mario Limonciello <mario.limonciello@....com>
> > Acked-by: Basavaraj Natikar <Basavaraj.Natikar@....com>
> > Link:
> >
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.
> > kernel.org%2Fr%2F20211101014853.6177-2-
> >
> mario.limonciello%40amd.com&data=04%7C01%7Cmario.limonciello%4
> >
> 0amd.com%7Ce14b7ddf8d1143b6649208d9b0853519%7C3dd8961fe4884e608
> >
> e11a82d994e183d%7C0%7C0%7C637734908448086367%7CUnknown%7CTWF
> >
> pbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXV
> >
> CI6Mn0%3D%7C3000&sdata=zUkTF851CdUmrY1z3YZbYrnVrjHQaVfb1%
> > 2Bg2dx28ZNA%3D&reserved=0
> > Signed-off-by: Linus Walleij <linus.walleij@...aro.org>
> > Signed-off-by: Sasha Levin <sashal@...nel.org>
> > ---
> > drivers/pinctrl/pinctrl-amd.c | 29 ++++++++++++++++++++++++++---
> > 1 file changed, 26 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/pinctrl/pinctrl-amd.c b/drivers/pinctrl/pinctrl-amd.c
> > index e20bcc835d6a8..54dfa0244422c 100644
> > --- a/drivers/pinctrl/pinctrl-amd.c
> > +++ b/drivers/pinctrl/pinctrl-amd.c
> > @@ -520,14 +520,14 @@ static struct irq_chip amd_gpio_irqchip = {
> >
> > #define PIN_IRQ_PENDING (BIT(INTERRUPT_STS_OFF) |
> > BIT(WAKE_STS_OFF))
> >
> > -static irqreturn_t amd_gpio_irq_handler(int irq, void *dev_id)
> > +static bool do_amd_gpio_irq_handler(int irq, void *dev_id)
> > {
> > struct amd_gpio *gpio_dev = dev_id;
> > struct gpio_chip *gc = &gpio_dev->gc;
> > - irqreturn_t ret = IRQ_NONE;
> > unsigned int i, irqnr;
> > unsigned long flags;
> > u32 __iomem *regs;
> > + bool ret = false;
> > u32 regval;
> > u64 status, mask;
> >
> > @@ -549,6 +549,14 @@ static irqreturn_t amd_gpio_irq_handler(int irq,
> void
> > *dev_id)
> > /* Each status bit covers four pins */
> > for (i = 0; i < 4; i++) {
> > regval = readl(regs + i);
> > + /* caused wake on resume context for shared IRQ */
> > + if (irq < 0 && (regval & BIT(WAKE_STS_OFF))) {
> > + dev_dbg(&gpio_dev->pdev->dev,
> > + "Waking due to GPIO %d: 0x%x",
> > + irqnr + i, regval);
> > + return true;
> > + }
> > +
> > if (!(regval & PIN_IRQ_PENDING) ||
> > !(regval & BIT(INTERRUPT_MASK_OFF)))
> > continue;
> > @@ -574,9 +582,12 @@ static irqreturn_t amd_gpio_irq_handler(int irq,
> void
> > *dev_id)
> > }
> > writel(regval, regs + i);
> > raw_spin_unlock_irqrestore(&gpio_dev->lock, flags);
> > - ret = IRQ_HANDLED;
> > + ret = true;
> > }
> > }
> > + /* did not cause wake on resume context for shared IRQ */
> > + if (irq < 0)
> > + return false;
> >
> > /* Signal EOI to the GPIO unit */
> > raw_spin_lock_irqsave(&gpio_dev->lock, flags);
> > @@ -588,6 +599,16 @@ static irqreturn_t amd_gpio_irq_handler(int irq,
> void
> > *dev_id)
> > return ret;
> > }
> >
> > +static irqreturn_t amd_gpio_irq_handler(int irq, void *dev_id)
> > +{
> > + return IRQ_RETVAL(do_amd_gpio_irq_handler(irq, dev_id));
> > +}
> > +
> > +static bool __maybe_unused amd_gpio_check_wake(void *dev_id)
> > +{
> > + return do_amd_gpio_irq_handler(-1, dev_id);
> > +}
> > +
> > static int amd_get_groups_count(struct pinctrl_dev *pctldev)
> > {
> > struct amd_gpio *gpio_dev = pinctrl_dev_get_drvdata(pctldev);
> > @@ -958,6 +979,7 @@ static int amd_gpio_probe(struct platform_device
> > *pdev)
> > goto out2;
> >
> > platform_set_drvdata(pdev, gpio_dev);
> > + acpi_register_wakeup_handler(gpio_dev->irq,
> > amd_gpio_check_wake, gpio_dev);
> >
> > dev_dbg(&pdev->dev, "amd gpio driver loaded\n");
> > return ret;
> > @@ -975,6 +997,7 @@ static int amd_gpio_remove(struct platform_device
> > *pdev)
> > gpio_dev = platform_get_drvdata(pdev);
> >
> > gpiochip_remove(&gpio_dev->gc);
> > + acpi_unregister_wakeup_handler(amd_gpio_check_wake,
> > gpio_dev);
> >
> > return 0;
> > }
> > --
> > 2.33.0
>
> Sasha,
>
> No concerns for me to 5.10, but would you mind also sending this to 5.14.y
> and 5.15.y too? I didn't see it sent up for either.
One more thought on this - please also take this (which wasn't part of autosel)
when this comes back:
e9380df85187 ACPI: Add stubs for wakeup handler functions
That prevents some compile errors in certain configurations.
>
> Thanks,
Powered by blists - more mailing lists