PCI: Fixes for native PCIe and ACPI hotplug
v2 cover PCI: Fixes for native PCIe and ACPI hotplug
Hi,
When Thunderbolt controller is configured to be in native enumeration mode it actually includes two slightly different modes. First there is non-RTD3 mode where the Thunderbolt host controller is only present when there is a device connected. The second one is RTD3 mode where the controller is always present.
In non-RTD3 mode the Thunderbolt host controller (NHI) and USB host (xHCI) controller are not hotplugged using native PCIe hotplug but instead they will be hotplugged via BIOS triggered ACPI Notify() to the root port. This is done to preserve resources since the NHI and xHCI only need 1 MB of MMIO space and no additional buses. Currently Linux does not support this very well and ends up failing the hotplug in one way or another. More detailed explanation is in changelog of patch [4/5].
This series fixes this issue and in addition includes fixes for few other issues found during testing on a system that has Thunderbolt controller in non-RTD3 native PCIe enumeration mode. However, the fixes here are not in any way Thunderbolt specific and should be applicable to other systems as well.
The previous version of the patch series can be found here:
https://www.spinics.net/lists/linux-acpi/msg80607.html
Changes from the previous version:
- Drop 'cmax - max ?: 1' and use similar construct than we use in second pass loop in patch [1/5].
- Drop unnecessary parentheses in patch [1/5].
- Added Rafael's tag to patches [2-5/5].
[ I'm sending this updated patch series now because my vacation starts
tomorrow and I'll be away whole next week. If any further changes are needed, I'll do those when I'm back. ]
Mika Westerberg (5):
PCI: Make sure all bridges reserve at least one bus number
PCI: Take bridge window alignment into account when distributing resources
PCI: pciehp: Clear Presence Detect and Data Link Layer Status Changed on resume ACPI / hotplug / PCI: Do not scan all bridges when native PCIe hotplug is used ACPI / hotplug / PCI: Mark stale PCI devices disconnected
drivers/pci/hotplug/acpiphp.h | 1 + drivers/pci/hotplug/acpiphp_glue.c | 78 ++++++++++++++++++++++++++++++-------- drivers/pci/hotplug/pciehp.h | 2 +- drivers/pci/hotplug/pciehp_core.c | 2 +- drivers/pci/hotplug/pciehp_hpc.c | 13 ++++++- drivers/pci/probe.c | 11 ++++-- drivers/pci/setup-bus.c | 41 +++++++++++++++++++- 7 files changed, 126 insertions(+), 22 deletions(-)
Signature · 1 line
2.15.1
PATCH v2 1/5 PCI: Make sure all bridges reserve at least one bus number
When distributing extra buses between hotplug bridges we need to make sure each bridge reserve at least one bus number, even if there is currently nothing connected to it. For instance ACPI hotplug may bring in additional devices to non-hotplug bridges later on. To make sure we don't run out of bus numbers for non-hotplug bridges reserve one bus number for them upfront before distributing buses for hotplug bridges.
--- drivers/pci/probe.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)
drivers/pci/probe.c +8 -3
@@ -2561,7 +2561,10 @@ static unsigned int pci_scan_child_bus_extend(struct pci_bus *bus,for_each_pci_bridge (dev, bus) { cmax = max; max =pci_scan_bridge_extend (bus, dev, max,0 ,0 ); - used_buses += cmax - max; +/* Reserve one bus for each bridge */ + used_buses++; +if (cmax - max >1 ) + used_buses += cmax - max -1 ; }/* Scan bridges that need to be reconfigured */
@@ -2584,12 +2587,14 @@ static unsigned int pci_scan_child_bus_extend(struct pci_bus *bus,* bridges if any. */ buses = available_buses / hotplug_bridges; - buses =min (buses, available_buses - used_buses); + buses =min (buses, available_buses - used_buses +1 ); } cmax = max; max =pci_scan_bridge_extend (bus, dev, cmax, buses,1 ); - used_buses += max - cmax; +/* One bus is already accounted so don't add it again */ +if (max - cmax >1 ) + used_buses += max - cmax -1 ; }/*
Signature · 1 line
2.15.1
Re: [PATCH v2 1/5] PCI: Make sure all bridges reserve at least one bus number
Quoted 11 lines · Mika Westerberg
When distributing extra buses between hotplug bridges we need to make sure each bridge reserve at least one bus number, even if there is currently nothing connected to it. For instance ACPI hotplug may bring in additional devices to non-hotplug bridges later on. To make sure we don't run out of bus numbers for non-hotplug bridges reserve one bus number for them upfront before distributing buses for hotplug bridges.
Fixes 1c02ea810065 ("PCI: Distribute available buses to hotplug-capable bridges")Reported-by Mario Limonciello <[email protected]>Signed-off-by Mika Westerberg <[email protected]>
Quoted 44 lines
--- drivers/pci/probe.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)drivers/pci/probe.c +8
-4@@ -2561,7 +2561,10 @@ static unsigned int pci_scan_child_bus_extend(struct pci_bus *bus,for_each_pci_bridge (dev, bus) { cmax = max; max =pci_scan_bridge_extend (bus, dev, max,0 ,0 ); - used_buses += cmax - max; +/* Reserve one bus for each bridge */ + used_buses++; +if (cmax - max >1 ) + used_buses += cmax - max -1 ; }/* Scan bridges that need to be reconfigured */ @@ -2584,12 +2587,14 @@ static unsigned int pci_scan_child_bus_extend(struct pci_bus *bus,* bridges if any. */ buses = available_buses / hotplug_bridges; - buses =min (buses, available_buses - used_buses); + buses =min (buses, available_buses - used_buses +1 ); } cmax = max; max =pci_scan_bridge_extend (bus, dev, cmax, buses,1 ); - used_buses += max - cmax; +/* One bus is already accounted so don't add it again */ +if (max - cmax >1 ) + used_buses += max - cmax -1 ; }/* -- 2.15.1
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/5] PCI: Make sure all bridges reserve at least one bus number
In reply to Mika WesterbergQuoted 9 lines · Mika Westerberg
When distributing extra buses between hotplug bridges we need to make sure each bridge reserve at least one bus number, even if there is currently nothing connected to it. For instance ACPI hotplug may bring in additional devices to non-hotplug bridges later on. To make sure we don't run out of bus numbers for non-hotplug bridges reserve one bus number for them upfront before distributing buses for hotplug bridges.
Fixes 1c02ea810065 ("PCI: Distribute available buses to hotplug-capable bridges")Reported-by Mario Limonciello <[email protected]>
Is there a bugzilla or email URL we can include here?
Quoted 46 lines
Signed-off-by Mika Westerberg <[email protected]>--- drivers/pci/probe.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)drivers/pci/probe.c +8
-3@@ -2561,7 +2561,10 @@ static unsigned int pci_scan_child_bus_extend(struct pci_bus *bus,for_each_pci_bridge (dev, bus) { cmax = max; max =pci_scan_bridge_extend (bus, dev, max,0 ,0 ); - used_buses += cmax - max; +/* Reserve one bus for each bridge */ + used_buses++; +if (cmax - max >1 ) + used_buses += cmax - max -1 ; }/* Scan bridges that need to be reconfigured */ @@ -2584,12 +2587,14 @@ static unsigned int pci_scan_child_bus_extend(struct pci_bus *bus,* bridges if any. */ buses = available_buses / hotplug_bridges; - buses =min (buses, available_buses - used_buses); + buses =min (buses, available_buses - used_buses +1 ); } cmax = max; max =pci_scan_bridge_extend (bus, dev, cmax, buses,1 ); - used_buses += max - cmax; +/* One bus is already accounted so don't add it again */ +if (max - cmax >1 ) + used_buses += max - cmax -1 ; }/* Signature · 6 lines
2.15.1 -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/5] PCI: Make sure all bridges reserve at least one bus number
Quoted 9 lines · Bjorn Helgaas
Quoted 7 lines · Mika Westerberg
When distributing extra buses between hotplug bridges we need to make sure each bridge reserve at least one bus number, even if there is currently nothing connected to it. For instance ACPI hotplug may bring in additional devices to non-hotplug bridges later on. To make sure we don't run out of bus numbers for non-hotplug bridges reserve one bus number for them upfront before distributing buses for hotplug bridges.
Quoted 4 lines
Quoted 2 lines
Fixes 1c02ea810065 ("PCI: Distribute available buses to hotplug-capable bridges")Reported-by Mario Limonciello <[email protected]>Is there a bugzilla or email URL we can include here?
Sorry it was private communication that I believe Mika is referring to in Reported-By here.
You can remove the tag if you think it's inappropriate for this commit.
Quoted 47 lines
Quoted 47 lines
Signed-off-by Mika Westerberg <[email protected]>--- drivers/pci/probe.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)drivers/pci/probe.c +8
-4@@ -2561,7 +2561,10 @@ static unsigned int pci_scan_child_bus_extend(struct pci_bus *bus, for_each_pci_bridge(dev, bus) { cmax = max; max =pci_scan_bridge_extend (bus, dev, max,0 ,0 ); - used_buses += cmax - max; +/* Reserve one bus for each bridge */ + used_buses++; +if (cmax - max >1 ) + used_buses += cmax - max -1 ; }/* Scan bridges that need to be reconfigured */ @@ -2584,12 +2587,14 @@ static unsigned int pci_scan_child_bus_extend(struct pci_bus *bus,* bridges if any. */ buses = available_buses / hotplug_bridges; - buses =min (buses, available_buses - used_buses); + buses =min (buses, available_buses - used_buses +1 ); } cmax = max; max =pci_scan_bridge_extend (bus, dev, cmax, buses,1 ); - used_buses += max - cmax; +/* One bus is already accounted so don't add it again */ +if (max - cmax >1 ) + used_buses += max - cmax -1 ; }/* -- 2.15.1
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/5] PCI: Make sure all bridges reserve at least one bus number
Quoted 9 lines · On Fri, Feb 23, 2018 at 12:52:18AM +0000, [email protected] wrote:
Quoted 8 lines · Bjorn Helgaas
Quoted 7 lines · Mika Westerberg
When distributing extra buses between hotplug bridges we need to make sure each bridge reserve at least one bus number, even if there is currently nothing connected to it. For instance ACPI hotplug may bring in additional devices to non-hotplug bridges later on. To make sure we don't run out of bus numbers for non-hotplug bridges reserve one bus number for them upfront before distributing buses for hotplug bridges.
Quoted 8 lines
Quoted 4 lines
Quoted 2 lines
Fixes 1c02ea810065 ("PCI: Distribute available buses to hotplug-capable bridges")Reported-by Mario Limonciello <[email protected]>Is there a bugzilla or email URL we can include here?
Sorry it was private communication that I believe Mika is referring to in Reported-By here.
You can remove the tag if you think it's inappropriate for this commit.
I like to give credit whenever possible, so I wouldn't necessarily remove the Reported-by.
But it would be very useful to also have a URL to something with more details, e.g., what sort of failure the user would observe, dmesg logs, PCI topology, etc.
This information can certainly be redacted to remove any proprietary things that can't be made public. Usually that isn't of interest to structural issues like this anyway.
Quoted 47 lines
Quoted 46 lines
Quoted 46 lines
Signed-off-by Mika Westerberg <[email protected]>--- drivers/pci/probe.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)drivers/pci/probe.c +8
-4@@ -2561,7 +2561,10 @@ static unsigned int pci_scan_child_bus_extend(struct pci_bus *bus, for_each_pci_bridge(dev, bus) { cmax = max; max =pci_scan_bridge_extend (bus, dev, max,0 ,0 ); - used_buses += cmax - max; +/* Reserve one bus for each bridge */ + used_buses++; +if (cmax - max >1 ) + used_buses += cmax - max -1 ; }/* Scan bridges that need to be reconfigured */ @@ -2584,12 +2587,14 @@ static unsigned int pci_scan_child_bus_extend(struct pci_bus *bus,* bridges if any. */ buses = available_buses / hotplug_bridges; - buses =min (buses, available_buses - used_buses); + buses =min (buses, available_buses - used_buses +1 ); } cmax = max; max =pci_scan_bridge_extend (bus, dev, cmax, buses,1 ); - used_buses += max - cmax; +/* One bus is already accounted so don't add it again */ +if (max - cmax >1 ) + used_buses += max - cmax -1 ; }/* -- 2.15.1
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v2 1/5] PCI: Make sure all bridges reserve at least one bus number
Quoted 6 lines
-----Original Message-----
From: Bjorn Helgaas [mailto:[email protected]]
Sent: Friday, February 23, 2018 11:26 AM
To: Limonciello, Mario <[email protected]>
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]=
om;
Quoted 3 lines
[email protected]; [email protected]; linux- [email protected]; [email protected]
Subject: Re: [PATCH v2 1/5] PCI: Make sure all bridges reserve at least o=
ne bus
Quoted 3 lines
number
=20
On Fri, Feb 23, 2018 at 12:52:18AM +0000, [email protected] wrot=
e:
Quoted 3 lines
Quoted 3 lines · Bjorn Helgaas
Quoted 2 lines · Mika Westerberg
Quoted 1 line
When distributing extra buses between hotplug bridges we need to ma=
ke
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
sure each bridge reserve at least one bus number, even if there is currently nothing connected to it. For instance ACPI hotplug may br=
ing
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
in additional devices to non-hotplug bridges later on. To make sure=
we
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
don't run out of bus numbers for non-hotplug bridges reserve one bu=
s
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
number for them upfront before distributing buses for hotplug bridg=
es.
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Fixes 1c02ea810065 ("PCI: Distribute available buses to hotplug-ca=
pable
Quoted 6 lines
bridges")
Quoted 5 lines
Quoted 3 lines
Quoted 1 line
Reported-by Mario Limonciello <[email protected]>Is there a bugzilla or email URL we can include here?
Sorry it was private communication that I believe Mika is referring to =
in Reported-
Quoted 14 lines
By here.
Quoted 2 lines
You can remove the tag if you think it's inappropriate for this commit.
=20
I like to give credit whenever possible, so I wouldn't necessarily remove the Reported-by.
=20
But it would be very useful to also have a URL to something with more details, e.g., what sort of failure the user would observe, dmesg logs, PCI topology, etc.
=20
This information can certainly be redacted to remove any proprietary things that can't be made public. Usually that isn't of interest to structural issues like this anyway.
It was a result of a very long communication chain, but I think I pulled out the relevant details here if you would like a URL to include:
https://gist.github.com/superm1/100a2ed20449f684ebb84c392e35dbed
Thank you.
Quoted 23 lines
=20
Quoted 11 lines
Quoted 11 lines
drivers/pci/probe.c static unsigned int
Quoted 11 lines
Signed-off-by Mika Westerberg <[email protected]>--- drivers/pci/probe.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)drivers/pci/probe.c +0
-0@@ -2561,7 +2561,10 @@ static unsigned intpci_scan_child_bus_extend(struct
pci_bus*bus,Quoted 10 lines
Quoted 10 lines
Quoted 10 lines
drivers/pci/probe.c +4
-1for_each_pci_bridge(dev, bus) { cmax =3D max; max =3D pci_scan_bridge_extend(bus, dev, max, 0, 0); - used_buses +=3D cmax - max; +/* Reserve one bus for each bridge */ + used_buses++; +if (cmax - max >1 ) + used_buses +=3D cmax - max - 1; }
Quoted 24 lines
Quoted 2 lines
Quoted 2 lines
drivers/pci/probe.c static unsigned int
Quoted 2 lines
drivers/pci/probe.c +0
-0/* Scan bridges that need to be reconfigured */ @@ -2584,12 +2587,14 @@ static unsigned intpci_scan_child_bus_extend(struct
pci_bus*bus,Quoted 21 lines
Quoted 21 lines
Quoted 21 lines
drivers/pci/probe.c +4
-3* bridges if any. */ buses =3D available_buses / hotplug_bridges; - buses =3D min(buses, available_buses - used_buses); + buses =3D min(buses, available_buses - used_buses + 1); } cmax =3D max; max =3D pci_scan_bridge_extend(bus, dev, cmax, buses, 1); - used_buses +=3D max - cmax; +/* One bus is already accounted so don't add it again */ +if (max - cmax >1 ) + used_buses +=3D max - cmax - 1; }/* -- 2.15.1
--
To unsubscribe from this list: send the line "unsubscribe linux-acp=
i" in
Quoted 3 lines
Quoted 3 lines
Quoted 2 lines
Quoted 2 lines
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/5] PCI: Make sure all bridges reserve at least one bus number
Quoted 39 lines · On Fri, Feb 23, 2018 at 07:06:59PM +0000, [email protected] wrote:
Quoted 39 lines
-----Original Message-----
From: Bjorn Helgaas [mailto:[email protected]]
Sent: Friday, February 23, 2018 11:26 AM
To: Limonciello, Mario <[email protected]>
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; linux- [email protected]; [email protected]
Subject: Re: [PATCH v2 1/5] PCI: Make sure all bridges reserve at least one bus numberQuoted 10 lines · On Fri, Feb 23, 2018 at 12:52:18AM +0000, [email protected] wrote:
Quoted 9 lines · Bjorn Helgaas
Quoted 8 lines · Mika Westerberg
When distributing extra buses between hotplug bridges we need to make sure each bridge reserve at least one bus number, even if there is currently nothing connected to it. For instance ACPI hotplug may bring in additional devices to non-hotplug bridges later on. To make sure we don't run out of bus numbers for non-hotplug bridges reserve one bus number for them upfront before distributing buses for hotplug bridges.
Fixes 1c02ea810065 ("PCI: Distribute available buses to hotplug-capablebridges")
Quoted 5 lines
Quoted 3 lines
Quoted 1 line
Reported-by Mario Limonciello <[email protected]>Is there a bugzilla or email URL we can include here?
Sorry it was private communication that I believe Mika is referring to in Reported-
By here.
Quoted 2 lines
You can remove the tag if you think it's inappropriate for this commit.
I like to give credit whenever possible, so I wouldn't necessarily remove the Reported-by.
But it would be very useful to also have a URL to something with more details, e.g., what sort of failure the user would observe, dmesg logs, PCI topology, etc.
Quoted 7 lines
Quoted 3 lines
This information can certainly be redacted to remove any proprietary things that can't be made public. Usually that isn't of interest to structural issues like this anyway.
It was a result of a very long communication chain, but I think I pulled out the relevant details here if you would like a URL to include:
https://gist.github.com/superm1/100a2ed20449f684ebb84c392e35dbed
Thanks Mario.
I'll update changelog to include the above information as well.
PATCH v2 2/5 PCI: Take bridge window alignment into account when distributing resources
In reply to Mika WesterbergWhen connecting a Thunderbolt endpoint which itself has internal PCIe switch (not the integrated one) the way we currently distribute resources does not always work well because devices below non-hotplug bridges might need to have their MMIO resources aligned to something else than 1 MB boundary. As an example connecting a server grade 4-port GbE add in card adds another PCIe switch leading to GbE devices that want to have their MMIO resources aligned to 2 MB boundary instead.
Current resource distribution code does not take this alignment into account and might try to add too much resources for the extension hotplug bridges. The resulting bridge window is then too big which makes Linux to re-allocate minimal amount of resources, making future extension impossible.
Make this work better by substracting properly aligned non-hotplug downstream bridge window size from the remaining resources.
--- drivers/pci/setup-bus.c | 41 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 40 insertions(+), 1 deletion(-)
drivers/pci/setup-bus.c +40 -1
@@ -1882,6 +1882,7 @@ static void pci_bus_distribute_available_resources(struct pci_bus *bus, resource_size_t available_mmio, resource_size_t available_mmio_pref) {resource_size_t remaining_io, remaining_mmio, remaining_mmio_pref; +resource_size_t io_start, mmio_start, mmio_pref_start;unsigned int normal_bridges =0 , hotplug_bridges =0 ;struct resource *io_res, *mmio_res, *mmio_pref_res;struct pci_dev *dev, *bridge = bus->self;
@@ -1946,11 +1947,16 @@ static void pci_bus_distribute_available_resources(struct pci_bus *bus, remaining_mmio_pref -=resource_size (res); } + io_start = io_res->start; + mmio_start = mmio_res->start; + mmio_pref_start = mmio_pref_res->start; +/* * Go over devices on this bus and distribute the remaining * resource space between hotplug bridges. */ for_each_pci_bridge (dev, bus) { +resource_size_t align;struct pci_bus *b; b = dev->subordinate;
@@ -1968,7 +1974,7 @@ static void pci_bus_distribute_available_resources(struct pci_bus *bus, available_io, available_mmio, available_mmio_pref); }else if (dev->is_hotplug_bridge) { -resource_size_t align, io, mmio, mmio_pref; +resource_size_t io, mmio, mmio_pref;/* * Distribute available extra resources equally
@@ -1981,11 +1987,13 @@ static void pci_bus_distribute_available_resources(struct pci_bus *bus, io =div64_ul (available_io, hotplug_bridges); io =min (ALIGN (io, align), remaining_io); remaining_io -= io; + io_start += io; align =pci_resource_alignment (bridge, mmio_res); mmio =div64_ul (available_mmio, hotplug_bridges); mmio =min (ALIGN (mmio, align), remaining_mmio); remaining_mmio -= mmio; + mmio_start += mmio; align =pci_resource_alignment (bridge, mmio_pref_res); mmio_pref = div64_ul(available_mmio_pref,
@@ -1993,9 +2001,40 @@ static void pci_bus_distribute_available_resources(struct pci_bus *bus, mmio_pref =min (ALIGN (mmio_pref, align), remaining_mmio_pref); remaining_mmio_pref -= mmio_pref; + mmio_pref_start += mmio_pref;pci_bus_distribute_available_resources (b, add_list, io, mmio, mmio_pref); + }else { +/* +* For normal bridges, track start of the parent +* bridge window to make sure we align the +* remaining space which is distributed to the +* hotplug bridges properly. +*/ +resource_size_t aligned; +struct resource *res; + + res = &dev->resource[PCI_BRIDGE_RESOURCES +0 ]; + io_start +=resource_size (res); + aligned =ALIGN (io_start, +pci_resource_alignment (dev, res)); +if (aligned > io_start) + remaining_io -= aligned - io_start; + + res = &dev->resource[PCI_BRIDGE_RESOURCES +1 ]; + mmio_start +=resource_size (res); + aligned =ALIGN (mmio_start, +pci_resource_alignment (dev, res)); +if (aligned > mmio_start) + remaining_mmio -= aligned - mmio_start; + + res = &dev->resource[PCI_BRIDGE_RESOURCES +2 ]; + mmio_pref_start +=resource_size (res); + aligned =ALIGN (mmio_pref_start, +pci_resource_alignment (dev, res)); +if (aligned > mmio_pref_start) + remaining_mmio_pref -= aligned - mmio_pref_start; } } }
Signature · 1 line
2.15.1
PATCH v2 3/5 PCI: pciehp: Clear Presence Detect and Data Link Layer Status Changed on resume
In reply to Mika WesterbergAfter suspend/resume cycle the Presence Detect or Data Link Layer Status Changed bits might be set and if we don't clear them those events will not fire anymore and nothing happens for instance when a device is now hot-unplugged.
Fix this by clearing those bits in a newly introduced function pcie_reenable_notification(). This should be fine because immediately after we check if the adapter is still present by reading directly from the status register.
--- drivers/pci/hotplug/pciehp.h | 2 +- drivers/pci/hotplug/pciehp_core.c | 2 +- drivers/pci/hotplug/pciehp_hpc.c | 13 ++++++++++++- 3 files changed, 14 insertions(+), 3 deletions(-)
drivers/pci/hotplug/pciehp.h +1 -1
@@ -120,7 +120,7 @@ struct controller *pcie_init(struct pcie_device *dev);int pcie_init_notification (struct controller *ctrl);int pciehp_enable_slot (struct slot *p_slot);int pciehp_disable_slot (struct slot *p_slot); -void pcie_enable_notification (struct controller *ctrl); +void pcie_reenable_notification (struct controller *ctrl);int pciehp_power_on_slot (struct slot *slot);void pciehp_power_off_slot (struct slot *slot);void pciehp_get_power_status (struct slot *slot,u8 *status);
drivers/pci/hotplug/pciehp_core.c +1 -1
@@ -283,7 +283,7 @@ static int pciehp_resume(struct pcie_device *dev) ctrl =get_service_data (dev);/* reinitialize the chipset's event detection logic */ -pcie_enable_notification (ctrl); +pcie_reenable_notification (ctrl); slot = ctrl->slot;
drivers/pci/hotplug/pciehp_hpc.c +12 -1
@@ -659,7 +659,7 @@ static irqreturn_t pcie_isr(int irq, void *dev_id)return handled; } -void pcie_enable_notification (struct controller *ctrl) +static void pcie_enable_notification (struct controller *ctrl) {u16 cmd, mask;
@@ -697,6 +697,17 @@ void pcie_enable_notification(struct controller *ctrl) pci_pcie_cap(ctrl->pcie->port) + PCI_EXP_SLTCTL, cmd); } +void pcie_reenable_notification (struct controller *ctrl) +{ +/* +* Clear both Presence and Data Link Layer Changed to make sure +* those events still fire after we have re-enabled them. +*/ +pcie_capability_write_word (ctrl->pcie->port,PCI_EXP_SLTSTA , +PCI_EXP_SLTSTA_PDC |PCI_EXP_SLTSTA_DLLSC ); +pcie_enable_notification (ctrl); +} +static void pcie_disable_notification (struct controller *ctrl) {u16 mask;
Signature · 1 line
2.15.1
PATCH v2 4/5 ACPI / hotplug / PCI: Do not scan all bridges when native PCIe hotplug is used
In reply to Mika WesterbergWhen a system is using native PCIe hotplug for Thunderbolt and the controller is not enabled for full RTD3 (runtime D3) it will be only present in the system when there is a device connected. This pretty much follows the BIOS assisted hotplug behaviour.
Thunderbolt host controller integrated PCIe switch has two additional PCIe downstream ports that lead to NHI (Thunderbolt host controller) and xHCI (USB 3 host controller) respectively. These downstream ports are not marked being hotplug capable. Reason for that is to preserve resources. Otherwise the OS would distribute remaining resources between all downstream ports making these two ports consume too much resources they don't really need (both NHI and xHCI only need 1 MB of MMIO space, and no extra buses).
Now, because these ports are not marked being hotplug capable the OS will not enable hotplug interrupt for them either and will not receive interrupt when devices below them are hotplugged. Solution to this is that the BIOS sends ACPI Notify() for the root port to notify the OS it needs to rescan for added devices.
However, the current ACPI hotplug implementation scans the whole slot regardless whether native PCIe is used or not and it expects that the BIOS has configured bridge resources upfront. If that's not the case it assigns resources using minimal allocation (everything currently found just barely fit) preventing future extension. In addition to that, if there is another native PCIe hotplug going on we may find the new PCIe switch only partially ready (all links are not fully trained yet) confusing PCIehp when it finally starts to enumerate new devices.
To make this work better with the native PCIe hotplug driver (PCIehp), we let it to handle all slot management and resource allocation for hotplug bridges and restrict ACPI hotplug to non-hotplug bridges.
--- drivers/pci/hotplug/acpiphp.h | 1 + drivers/pci/hotplug/acpiphp_glue.c | 73 ++++++++++++++++++++++++++++++-------- 2 files changed, 59 insertions(+), 15 deletions(-)
drivers/pci/hotplug/acpiphp.h +1 -0
@@ -158,6 +158,7 @@ struct acpiphp_attention_info#define SLOT_ENABLED (0x00000001)#define SLOT_IS_GOING_AWAY (0x00000002) +#define SLOT_IS_NATIVE (0x00000004)/* function flags */
drivers/pci/hotplug/acpiphp_glue.c +58 -15
102 lines not on this page · raw message
Signature · 1 line
2.15.1
PATCH v2 5/5 ACPI / hotplug / PCI: Mark stale PCI devices disconnected
In reply to Mika WesterbergFollowing PCIehp mark the unplugged PCI devices disconnected. This makes sure PCI core code leaves the now missing hardware registers alone.
--- drivers/pci/hotplug/acpiphp_glue.c | 5 +++++ 1 file changed, 5 insertions(+)
drivers/pci/hotplug/acpiphp_glue.c +5 -0
@@ -651,6 +651,11 @@ static void trim_stale_devices(struct pci_dev *dev) alive =pci_device_is_present (dev);if (!alive) { +pci_dev_set_disconnected (dev,NULL ); +if (pci_has_subordinate (dev)) +pci_walk_bus (dev->subordinate, pci_dev_set_disconnected, +NULL ); +pci_stop_and_remove_bus_device (dev);if (adev)acpi_bus_trim (adev);
Signature · 1 line
2.15.1