Re: [PATCH v3 00/27] Thunderbolt security levels and NVM firmware upgrade

From: Mika Westerberg <mika.westerberg@linux.intel.com>
To: Andreas Noever <andreas.noever@gmail.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Michael Jamet <michael.jamet@intel.com>,
	Yehezkel Bernat <yehezkel.bernat@intel.com>,
	Lukas Wunner <lukas@wunner.de>,
	Amir Levy <amir.jer.levy@intel.com>,
	Andy Lutomirski <luto@kernel.org>,
	Mario.Limonciello@dell.com, Jared.Dominguez@dell.com,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v3 00/27] Thunderbolt security levels and NVM firmware upgrade
Date: Mon, 5 Jun 2017 22:43:16 +0300	[thread overview]
Message-ID: <20170605194316.GU3454@lahna.fi.intel.com> (raw)
In-Reply-To: <CAMxnaaXp2ch4foOvRc6F4SLzyN1uSxHAFBknQVzJXMjKpPW+Vw@mail.gmail.com>

On Mon, Jun 05, 2017 at 07:01:10PM +0200, Andreas Noever wrote:
> On Mon, Jun 5, 2017 at 9:18 AM, Mika Westerberg
> <mika.westerberg@linux.intel.com> wrote:
> > On Sat, Jun 03, 2017 at 06:17:04PM +0900, Greg Kroah-Hartman wrote:
> >> On Fri, Jun 02, 2017 at 05:04:57PM +0300, Mika Westerberg wrote:
> >> > Hi,
> >> >
> >> > This is a third version of the patch series adding support for Thunderbolt
> >> > security levels and NVM firmware upgrade. PCs running Intel Falcon Ridge or
> >> > newer need these in order to connect devices if the security level is set
> >> > to "user(SL1) or secure(SL2)" from BIOS.
> >>
> >> All looks good to me, very nice work.
> >
> > Thanks!
> >
> >> I don't know what tree it should go in through, but if Andreas wants me
> >> to take it, I will if I can get his signed-off-by.
> >
> > That would be perfect.
> >
> > Andreas, do you have any objections?
> No, Thanks a lot.
> 
> Signed-off-by: Andreas Noever <andreas.noever@gmail.com>

Thanks!

> Greg, can you take this through your tree?
> 
> 
> 
> Mika, I have a quick question regarding the pci side of things (your
> "pci=hpbussize=10,hpmemsize=2M" workaround). Does that work for nested
> hotplug or just on the first level? Back when I was having a look at
> enabling chaining in the native driver I could not get pci to properly
> assign bus numbers to nested bridges. It always ran out of bus number
> after one level (irregardless of hpbussize). Has the pci behaviour
> changed or does the ICM somehow preconfigure the bridges before
> handing them of to linux?

I don't think ICM does any preconfiguration when native PCIe hotplug is
used. For PCs we typically use BIOS assisted ACPI hotplug and the SMI
handler configures the bridges before it notifies the OS.

In case of native PCIe hotplug, it still runs out of bus space, that's
why it is workaround. It works to some depth though, because it adds 10
to all bridges that it finds when the scan is done. When next device is
added you have that 10 which it then allocates to bridges downstream.

Anyway it should be fixed properly and it is on my todo list once I get
still missing parts finished. Unless someone else fixes it first, that
is ;-)

One solution that I've been thinking is to introduce some sort of
resource allocation policies to root ports depending what is connected
to them, and in case of Thunderbolt we follow the BIOS assisted hotplug
way so that we assign the remaining resources to the downstream port
where the chain is extended (this can be figured out from the registers,
I think).

There is also another PCI/ACPI related issue that Mario reported where
we execute _INI() methods before the initial PCI scan on boot when
Thunderbolt device is connected causing Linux to accidentally find the
upstream port of the Thunderbolt host controller before it is configured
properly by the BIOS ACPI hotplug handler. I've discussed this with
Rafael and he has an idea how we could fix it but it probably requires
some changes to ACPICA first. Also on my todo list :)