| 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: <MN0PR12MB61019560E0A264485B46CB27E2719@MN0PR12MB6101.namprd12.prod.outlook.com>
Date: Mon, 8 May 2023 14:09:17 +0000
From: "Limonciello, Mario" <Mario.Limonciello@....com>
To: Andreas Noever <andreas.noever@...il.com>,
Michael Jamet <michael.jamet@...el.com>,
Mika Westerberg <mika.westerberg@...ux.intel.com>,
Yehezkel Bernat <YehezkelShB@...il.com>
CC: "S, Sanath" <Sanath.S@....com>,
"Gong, Richard" <Richard.Gong@....com>,
"Mehta, Sanju" <Sanju.Mehta@....com>, Takashi Iwai <tiwai@...e.de>,
"linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH v2 1/2] thunderbolt: Clear registers properly when auto
clear isn't in use
[AMD Official Use Only - General]
> -----Original Message-----
> From: Limonciello, Mario <Mario.Limonciello@....com>
> Sent: Monday, April 24, 2023 2:56 PM
> To: Andreas Noever <andreas.noever@...il.com>; Michael Jamet
> <michael.jamet@...el.com>; Mika Westerberg
> <mika.westerberg@...ux.intel.com>; Yehezkel Bernat
> <YehezkelShB@...il.com>; Limonciello, Mario
> <Mario.Limonciello@....com>
> Cc: S, Sanath <Sanath.S@....com>; Gong, Richard
> <Richard.Gong@....com>; Mehta, Sanju <Sanju.Mehta@....com>;
> Takashi Iwai <tiwai@...e.de>; linux-usb@...r.kernel.org; linux-
> kernel@...r.kernel.org
> Subject: [PATCH v2 1/2] thunderbolt: Clear registers properly when auto
> clear isn't in use
>
> When `QUIRK_AUTO_CLEAR_INT` isn't set, interrupt masking should be
> cleared by writing to Interrupt Mask Clear (IMR) and interrupt
> status should be cleared properly at shutdown/init.
>
> This fixes an error where interrupts are left enabled during resume
> from hibernation with `CONFIG_USB4=y`.
>
> Fixes: 468c49f44759 ("thunderbolt: Disable interrupt auto clear for rings")
> Reported-by: Takashi Iwai <tiwai@...e.de>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=217343
> Signed-off-by: Mario Limonciello <mario.limonciello@....com>
> ---
> v1->v2:
> * Whitespace changes
> * Add new static functions for nhi_mask_interrupt() and
> nhi_clear_interrupt()
>
> I tried to base this off thunderbolt.git/next (tag: thunderbolt-for-v6.4-rc1)
> but the following 3 commits are missing from that branch but are in 6.3-rc7:
>
> 58cdfe6f58b3 thunderbolt: Rename shadowed variables bit to interrupt_bit
> and auto_clear_bit
> 468c49f44759 thunderbolt: Disable interrupt auto clear for rings
> 1716efdb0793 thunderbolt: Use const qualifier for `ring_interrupt_index`
>
> I cherry picked them first as this patch builds on them. It's expected that
> this patch should apply on top of 6.4-rc1 properly.
Mika,
Since 6.4-rc1 is out, you should be able to test and apply this patch now.
If you need any changes I'll send a v3 dropping the second patch.
Thanks!
> ---
> drivers/thunderbolt/nhi.c | 29 ++++++++++++++++++++++++-----
> drivers/thunderbolt/nhi_regs.h | 2 ++
> 2 files changed, 26 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/thunderbolt/nhi.c b/drivers/thunderbolt/nhi.c
> index d76e923fbc6a..c0aee5dc5237 100644
> --- a/drivers/thunderbolt/nhi.c
> +++ b/drivers/thunderbolt/nhi.c
> @@ -54,6 +54,21 @@ static int ring_interrupt_index(const struct tb_ring
> *ring)
> return bit;
> }
>
> +static void nhi_mask_interrupt(struct tb_nhi *nhi, int mask, int ring)
> +{
> + if (nhi->quirks & QUIRK_AUTO_CLEAR_INT)
> + return;
> + iowrite32(mask, nhi->iobase +
> REG_RING_INTERRUPT_MASK_CLEAR_BASE + ring);
> +}
> +
> +static void nhi_clear_interrupt(struct tb_nhi *nhi, int ring)
> +{
> + if (nhi->quirks & QUIRK_AUTO_CLEAR_INT)
> + ioread32(nhi->iobase + REG_RING_NOTIFY_BASE + ring);
> + else
> + iowrite32(~0, nhi->iobase + REG_RING_INT_CLEAR + ring);
> +}
> +
> /*
> * ring_interrupt_active() - activate/deactivate interrupts for a single ring
> *
> @@ -61,8 +76,8 @@ static int ring_interrupt_index(const struct tb_ring
> *ring)
> */
> static void ring_interrupt_active(struct tb_ring *ring, bool active)
> {
> - int reg = REG_RING_INTERRUPT_BASE +
> - ring_interrupt_index(ring) / 32 * 4;
> + int index = ring_interrupt_index(ring) / 32 * 4;
> + int reg = REG_RING_INTERRUPT_BASE + index;
> int interrupt_bit = ring_interrupt_index(ring) & 31;
> int mask = 1 << interrupt_bit;
> u32 old, new;
> @@ -123,7 +138,11 @@ static void ring_interrupt_active(struct tb_ring *ring,
> bool active)
> "interrupt for %s %d is already
> %s\n",
> RING_TYPE(ring), ring->hop,
> active ? "enabled" : "disabled");
> - iowrite32(new, ring->nhi->iobase + reg);
> +
> + if (active)
> + iowrite32(new, ring->nhi->iobase + reg);
> + else
> + nhi_mask_interrupt(ring->nhi, mask, index);
> }
>
> /*
> @@ -136,11 +155,11 @@ static void nhi_disable_interrupts(struct tb_nhi
> *nhi)
> int i = 0;
> /* disable interrupts */
> for (i = 0; i < RING_INTERRUPT_REG_COUNT(nhi); i++)
> - iowrite32(0, nhi->iobase + REG_RING_INTERRUPT_BASE + 4
> * i);
> + nhi_mask_interrupt(nhi, ~0, 4 * i);
>
> /* clear interrupt status bits */
> for (i = 0; i < RING_NOTIFY_REG_COUNT(nhi); i++)
> - ioread32(nhi->iobase + REG_RING_NOTIFY_BASE + 4 * i);
> + nhi_clear_interrupt(nhi, 4 * i);
> }
>
> /* ring helper methods */
> diff --git a/drivers/thunderbolt/nhi_regs.h b/drivers/thunderbolt/nhi_regs.h
> index faef165a919c..6ba295815477 100644
> --- a/drivers/thunderbolt/nhi_regs.h
> +++ b/drivers/thunderbolt/nhi_regs.h
> @@ -93,6 +93,8 @@ struct ring_desc {
> #define REG_RING_INTERRUPT_BASE 0x38200
> #define RING_INTERRUPT_REG_COUNT(nhi) ((31 + 2 * nhi->hop_count) /
> 32)
>
> +#define REG_RING_INTERRUPT_MASK_CLEAR_BASE 0x38208
> +
> #define REG_INT_THROTTLING_RATE 0x38c00
>
> /* Interrupt Vector Allocation */
> --
> 2.34.1
Powered by blists - more mailing lists