linux-kernel - Re: [PATCH v3] thunderbolt: Explicitly enable lane adapter hotplug events at startup

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <Yy6n56H09ct76GTJ@black.fi.intel.com>
Date:   Sat, 24 Sep 2022 09:47:03 +0300
From:   Mika Westerberg <mika.westerberg@...ux.intel.com>
To:     Mario Limonciello <mario.limonciello@....com>
Cc:     Andreas Noever <andreas.noever@...il.com>,
        Michael Jamet <michael.jamet@...el.com>,
        Yehezkel Bernat <YehezkelShB@...il.com>,
        Mehta Sanju <Sanju.Mehta@....com>, stable@...r.kernel.org,
        linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3] thunderbolt: Explicitly enable lane adapter hotplug
 events at startup

Hi Mario,

On Fri, Sep 23, 2022 at 01:39:44PM -0500, Mario Limonciello wrote:
> Software that has run before the USB4 CM in Linux runs may have disabled
> hotplug events for a given lane adapter.
> 
> Other CMs such as that one distributed with Windows 11 will enable hotplug
> events. Do the same thing in the Linux CM which fixes hotplug events on
> "AMD Pink Sardine".
> 
> Cc: stable@...r.kernel.org
> Signed-off-by: Mario Limonciello <mario.limonciello@....com>
> ---
> v2->v3:
>  * Guard with tb_switch_is_icm to avoid risk to Intel FW CM case
> v1->v2:
>  * s/usb4_enable_hotplug/usb4_port_hotplug_enable/
>  * Clarify intended users in documentation comment
>  * Only call for lane adapters
>  * Add stable tag
> 
> Note: v2 it was suggested to move this to tb_switch_configure, but
> port->config is not yet read at that time.  This should be the right
> time to call it, so this version of the patch just guards against the
> code running on Intel's controllers that have a FW CM.

Can we put it in a separate function that is called from
tb_switch_add()? I explained in the previous reply (to v2) that the
tb_init_port() is pretty much just reading stuff and therefore I would
not like to add there anything that does writing.

While at it, the USB4 v2 spec (not yet public but can be found from the
working group site) adds a CM requirement that forbids writing lane 1
adapter configuration registers so in addition to that please check
port->cap_usb4 there so we know we deal with the lane 0 adapter only (as
->cap_usb4 is only set for the lane 0 adapters).

So something like this:

static void tb_switch_port_hotplug_enable(struct tb_switch *sw)
{
	if (tb_switch_is_icm(sw))
		return;

	tb_switch_for_each_port(sw, port) {
		if (!port->cap_usb4)
			continue;

		// here enable the hotplug
	}
}

int tb_switch_add(struct tb_switch *sw)
{
	... // init ports and stuff happens here

	tb_switch_default_link_ports(sw);
	tb_switch_port_hotplug_enable(sw);
}

(we should probably create a new macro tb_switch_for_each_usb4_port()
that just enumerates all USB4 ports).