linux-kernel - Re: [PATCH v2 16/28] thunderbolt: Discover preboot PCIe paths the boot firmware established

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]

Date:   Wed, 13 Feb 2019 10:30:18 +0200
From:   Mika Westerberg <mika.westerberg@...ux.intel.com>
To:     Lukas Wunner <lukas@...ner.de>
Cc:     linux-kernel@...r.kernel.org,
        Michael Jamet <michael.jamet@...el.com>,
        Yehezkel Bernat <YehezkelShB@...il.com>,
        Andreas Noever <andreas.noever@...il.com>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Subject: Re: [PATCH v2 16/28] thunderbolt: Discover preboot PCIe paths the
 boot firmware established

On Tue, Feb 12, 2019 at 08:42:49PM +0100, Lukas Wunner wrote:
> On Wed, Feb 06, 2019 at 04:17:26PM +0300, Mika Westerberg wrote:
> > +static struct tb_port *tb_port_remote(struct tb_port *port)
> > +{
> > +	struct tb_port *remote = port->remote;
> > +
> > +	/*
> > +	 * If we have a dual link, the remote is available through the
> > +	 * primary link.
> > +	 */
> > +	if (!remote && port->dual_link_port && port->dual_link_port->remote)
> > +		return port->dual_link_port->remote->dual_link_port;
> > +	return remote;
> > +}
> 
> Yet more special-casing for dual-link ports. :-(
> 
> 
> > +	if (tunnel->dst_port->config.type != TB_TYPE_PCIE_UP) {
> > +		tb_port_warn(tunnel->dst_port,
> > +			     "path does not end to a PCIe adapter\n");
> 
> Nit: I think the proper proposition is "on" or "at", not "to".
> 
> The tunnel discovery algorithm looks solid to me, so:
> Reviewed-by: Lukas Wunner <lukas@...ner.de>

Thanks!

> When the module is unloaded, tb_stop() currently deactivates all PCI
> tunnels.  Is this still a good idea now that tunnels are discovered
> on probe?  We could just leave the tunnels in place and rediscover
> them when the module is reloaded.  If something was unplugged in the
> meantime, pciehp will have disconnected the devices and we should
> notice on reprobe that certain tunnels cannot be rediscovered, so no
> harm no foul.  Thoughts?

I agree it makes sense and that's actually what we already do with the
firmware CM.