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: <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

Powered by Openwall GNU/*/Linux Powered by OpenVZ