PCI/shpchp: fix a bus speed issue on hotplug
PATCH 1/1 PCI/shpchp: fix a bus speed issue on hotplug
When a board is added, the shpchp driver checks if there
is a mismatch between the bridge's adapter and the bus speed. If there is, it sets the subordinate speed (if there is no device on it).
However, it takes the reference of the board speed from the primary bus and not from the subordinate. If the primary bus is PCI and not PCIX/PCIe, its speed is not updated and remains 0xff. As a result hotplug fails with error: "Speed of bus ff and adapter 0 mismatch".
Fixed that by checking the speed against the subordinate bus.
--- drivers/pci/hotplug/shpchp_ctrl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/pci/hotplug/shpchp_ctrl.c +2 -2
@@ -282,8 +282,8 @@ static int board_added(struct slot *p_slot)return WRONG_BUS_FREQUENCY ; } - bsp = ctrl->pci_dev->bus->cur_bus_speed; - msp = ctrl->pci_dev->bus->max_bus_speed; + bsp = ctrl->pci_dev->subordinate->cur_bus_speed; + msp = ctrl->pci_dev->subordinate->max_bus_speed;/* Check if there are other slots or devices on the same bus */ if (!list_empty (&ctrl->pci_dev->subordinate->devices))
Signature · 1 line
1.8.3.1
Quoted 9 lines · Marcel Apfelbaum
When a board is added, the shpchp driver checks if there
is a mismatch between the bridge's adapter and the bus speed. If there is, it sets the subordinate speed (if there is no device on it).However, it takes the reference of the board speed from the primary bus and not from the subordinate. If the primary bus is PCI and not PCIX/PCIe, its speed is not updated and remains
0xff. As a result hotplug fails with error: "Speed of bus ff and adapter 0 mismatch".
Quoted 4 lines
Fixed that by checking the speed against the subordinate bus.
Signed-off-by Marcel Apfelbaum <[email protected]>
Looking at git history, I suspect this is a regression introduced by
commit 3749c51ac6c1560aa1cb1520066bed84c6f8152a Author: Matthew Wilcox <[email protected]> Date: Sun Dec 13 08:11:32 2009 -0500 PCI: Make current and maximum bus speeds part of the PCI core
At least I remember this configuration worked for me early in 2009...
Quoted 21 lines
--- drivers/pci/hotplug/shpchp_ctrl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)drivers/pci/hotplug/shpchp_ctrl.c +2
-2@@ -282,8 +282,8 @@ static int board_added(struct slot *p_slot)return WRONG_BUS_FREQUENCY ; } - bsp = ctrl->pci_dev->bus->cur_bus_speed; - msp = ctrl->pci_dev->bus->max_bus_speed; + bsp = ctrl->pci_dev->subordinate->cur_bus_speed; + msp = ctrl->pci_dev->subordinate->max_bus_speed;/* Check if there are other slots or devices on the same bus */ if (!list_empty (&ctrl->pci_dev->subordinate->devices))Signature · 1 line
1.8.3.1
Bjorn Helgaas
maintainerQuoted 8 lines · Marcel Apfelbaum
When a board is added, the shpchp driver checks if there
is a mismatch between the bridge's adapter and the bus speed. If there is, it sets the subordinate speed (if there is no device on it).However, it takes the reference of the board speed from the primary bus and not from the subordinate. If the primary bus is PCI and not PCIX/PCIe, its speed is not updated and remains
0xff. As a result hotplug fails with error: "Speed of bus ff and adapter 0 mismatch".
It'd be cool to have a bugzilla for this with lspci and dmesg output. I'll also have to check the other hotplug drivers for similar issues, unless you've already done that.
Quoted 26 lines
Fixed that by checking the speed against the subordinate bus.
Signed-off-by Marcel Apfelbaum <[email protected]>--- drivers/pci/hotplug/shpchp_ctrl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)drivers/pci/hotplug/shpchp_ctrl.c +2
-3@@ -282,8 +282,8 @@ static int board_added(struct slot *p_slot)return WRONG_BUS_FREQUENCY ; } - bsp = ctrl->pci_dev->bus->cur_bus_speed; - msp = ctrl->pci_dev->bus->max_bus_speed; + bsp = ctrl->pci_dev->subordinate->cur_bus_speed; + msp = ctrl->pci_dev->subordinate->max_bus_speed;/* Check if there are other slots or devices on the same bus */ if (!list_empty (&ctrl->pci_dev->subordinate->devices)) --1.8.3.1
Quoted 5 lines · Bjorn Helgaas
Quoted 4 lines · Marcel Apfelbaum
When a board is added, the shpchp driver checks if there
is a mismatch between the bridge's adapter and the bus speed. If there is, it sets the subordinate speed (if there is no device on it).
Quoted 8 lines
Quoted 4 lines
However, it takes the reference of the board speed from the primary bus and not from the subordinate. If the primary bus is PCI and not PCIX/PCIe, its speed is not updated and remains
0xff. As a result hotplug fails with error: "Speed of bus ff and adapter 0 mismatch".It'd be cool to have a bugzilla for this with lspci and dmesg output. I'll also have to check the other hotplug drivers for similar issues, unless you've already done that.
I'll open a BZ with the details, sure.
I didn't check the other hotplug drivers, but I do plan to look into PCIe driver as part of QEMU's PCIe hotplug feature.
Thanks,
Marcel
Quoted 27 lines
Quoted 27 lines
Fixed that by checking the speed against the subordinate bus.
Signed-off-by Marcel Apfelbaum <[email protected]>--- drivers/pci/hotplug/shpchp_ctrl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)drivers/pci/hotplug/shpchp_ctrl.c +2
-3@@ -282,8 +282,8 @@ static int board_added(struct slot *p_slot)return WRONG_BUS_FREQUENCY ; } - bsp = ctrl->pci_dev->bus->cur_bus_speed; - msp = ctrl->pci_dev->bus->max_bus_speed; + bsp = ctrl->pci_dev->subordinate->cur_bus_speed; + msp = ctrl->pci_dev->subordinate->max_bus_speed;/* Check if there are other slots or devices on the same bus */ if (!list_empty (&ctrl->pci_dev->subordinate->devices)) --1.8.3.1
Quoted 11 lines · Marcel Apfelbaum
Quoted 10 lines · Bjorn Helgaas
Quoted 8 lines · Marcel Apfelbaum
When a board is added, the shpchp driver checks if there
is a mismatch between the bridge's adapter and the bus speed. If there is, it sets the subordinate speed (if there is no device on it).However, it takes the reference of the board speed from the primary bus and not from the subordinate. If the primary bus is PCI and not PCIX/PCIe, its speed is not updated and remains
0xff. As a result hotplug fails with error: "Speed of bus ff and adapter 0 mismatch".
Quoted 4 lines
Quoted 3 lines
It'd be cool to have a bugzilla for this with lspci and dmesg output. I'll also have to check the other hotplug drivers for similar issues, unless you've already done that.
I'll open a BZ with the details, sure.
Done: https://bugzilla.kernel.org/show_bug.cgi?id=75251
If you need further details, please let me know.
Thanks,
Marcel
Quoted 41 lines
I didn't check the other hotplug drivers, but I do plan to look into PCIe driver as part of QEMU's PCIe hotplug feature.
Thanks,
MarcelQuoted 22 lines
Quoted 22 lines
Fixed that by checking the speed against the subordinate bus.
Signed-off-by Marcel Apfelbaum <[email protected]>--- drivers/pci/hotplug/shpchp_ctrl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)drivers/pci/hotplug/shpchp_ctrl.c +2
-2@@ -282,8 +282,8 @@ static int board_added(struct slot *p_slot)return WRONG_BUS_FREQUENCY ; } - bsp = ctrl->pci_dev->bus->cur_bus_speed; - msp = ctrl->pci_dev->bus->max_bus_speed; + bsp = ctrl->pci_dev->subordinate->cur_bus_speed; + msp = ctrl->pci_dev->subordinate->max_bus_speed;Quoted 5 lines
Quoted 5 lines
drivers/pci/hotplug/shpchp_ctrl.c +0
-1/* Check if there are other slots or devices on the same bus */ if (!list_empty (&ctrl->pci_dev->subordinate->devices)) --1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Bjorn Helgaas
maintainerQuoted 12 lines · Marcel Apfelbaum
Quoted 11 lines · Marcel Apfelbaum
Quoted 10 lines · Bjorn Helgaas
Quoted 8 lines · Marcel Apfelbaum
When a board is added, the shpchp driver checks if there
is a mismatch between the bridge's adapter and the bus speed. If there is, it sets the subordinate speed (if there is no device on it).However, it takes the reference of the board speed from the primary bus and not from the subordinate. If the primary bus is PCI and not PCIX/PCIe, its speed is not updated and remains
0xff. As a result hotplug fails with error: "Speed of bus ff and adapter 0 mismatch".
Quoted 6 lines
Quoted 4 lines
Quoted 3 lines
It'd be cool to have a bugzilla for this with lspci and dmesg output. I'll also have to check the other hotplug drivers for similar issues, unless you've already done that.
I'll open a BZ with the details, sure.
Done: https://bugzilla.kernel.org/show_bug.cgi?id=75251
If you need further details, please let me know.
Thanks. Would you mind attaching the "lspci -vv" output? That should show more details, including the information used to compute the bus speed.
Also, you checked the "regression" box. Can you confirm that and identify a known-working kernel? If we know which kernels are broken, we can potentially mark the fix to be backported to them.
Bjorn
Quoted 17 lines · Bjorn Helgaas
Quoted 15 lines · Marcel Apfelbaum
Quoted 14 lines · Marcel Apfelbaum
Quoted 13 lines · Bjorn Helgaas
Quoted 8 lines · Marcel Apfelbaum
When a board is added, the shpchp driver checks if there
is a mismatch between the bridge's adapter and the bus speed. If there is, it sets the subordinate speed (if there is no device on it).However, it takes the reference of the board speed from the primary bus and not from the subordinate. If the primary bus is PCI and not PCIX/PCIe, its speed is not updated and remains
0xff. As a result hotplug fails with error: "Speed of bus ff and adapter 0 mismatch".It'd be cool to have a bugzilla for this with lspci and dmesg output. I'll also have to check the other hotplug drivers for similar issues, unless you've already done that.
Quoted 7 lines
Quoted 3 lines
Quoted 1 line
I'll open a BZ with the details, sure.
Done: https://bugzilla.kernel.org/show_bug.cgi?id=75251
If you need further details, please let me know.Thanks. Would you mind attaching the "lspci -vv" output? That should show more details, including the information used to compute the bus speed.
Sure, done.
Quoted 4 lines
Also, you checked the "regression" box. Can you confirm that and identify a known-working kernel? If we know which kernels are broken, we can potentially mark the fix to be backported to them.
I checked the regression box based on Michael's comment that in early 2009 it did work. He pointed out a commit he thinks that caused this regression.
I think it can be backported to all the relevant stable versions since the commit that introduced the code mentioned in the patch.
Thanks,
Marcel
Quoted 2 lines
Bjorn
Quoted 21 lines · Bjorn Helgaas
Quoted 12 lines · Marcel Apfelbaum
Quoted 11 lines · Marcel Apfelbaum
Quoted 10 lines · Bjorn Helgaas
Quoted 8 lines · Marcel Apfelbaum
When a board is added, the shpchp driver checks if there
is a mismatch between the bridge's adapter and the bus speed. If there is, it sets the subordinate speed (if there is no device on it).However, it takes the reference of the board speed from the primary bus and not from the subordinate. If the primary bus is PCI and not PCIX/PCIe, its speed is not updated and remains
0xff. As a result hotplug fails with error: "Speed of bus ff and adapter 0 mismatch".Quoted 6 lines
Quoted 4 lines
Quoted 3 lines
It'd be cool to have a bugzilla for this with lspci and dmesg output. I'll also have to check the other hotplug drivers for similar issues, unless you've already done that.
I'll open a BZ with the details, sure.
Done: https://bugzilla.kernel.org/show_bug.cgi?id=75251
If you need further details, please let me know.
Quoted 7 lines
Thanks. Would you mind attaching the "lspci -vv" output? That should show more details, including the information used to compute the bus speed.
Also, you checked the "regression" box. Can you confirm that and identify a known-working kernel? If we know which kernels are broken, we can potentially mark the fix to be backported to them.
I checked 2.6.24-26-generic that comes with Ubuntu 8.04 and it works OK. So we have a working version.
I hope it helps,
Marcel
Quoted 2 lines
Bjorn
Quoted 3 lines · Marcel Apfelbaum
When a board is added, the shpchp driver checks if there
is a mismatch between the bridge's adapter and the bus speed. If there is, it sets the subordinate speed (if there is no device on it).
Since the speed is irrelevant when running in a VM, I suggest that you either ignore it altogether, or "normalize" the speed of all the devices/bridges/buses in QEMU in order to avoid any conflicts in the first place.
Thanks, Ronen.
Quoted 29 lines
However, it takes the reference of the board speed from the primary bus and not from the subordinate. If the primary bus is PCI and not PCIX/PCIe, its speed is not updated and remains
0xff. As a result hotplug fails with error: "Speed of bus ff and adapter 0 mismatch".Fixed that by checking the speed against the subordinate bus.
Signed-off-by Marcel Apfelbaum <[email protected]>--- drivers/pci/hotplug/shpchp_ctrl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)drivers/pci/hotplug/shpchp_ctrl.c +2
-2@@ -282,8 +282,8 @@ static int board_added(struct slot *p_slot)return WRONG_BUS_FREQUENCY ; } - bsp = ctrl->pci_dev->bus->cur_bus_speed; - msp = ctrl->pci_dev->bus->max_bus_speed; + bsp = ctrl->pci_dev->subordinate->cur_bus_speed; + msp = ctrl->pci_dev->subordinate->max_bus_speed;/* Check if there are other slots or devices on the same bus */ if (!list_empty (&ctrl->pci_dev->subordinate->devices))
Quoted 9 lines · Ronen Hod
Quoted 3 lines · Marcel Apfelbaum
When a board is added, the shpchp driver checks if there
is a mismatch between the bridge's adapter and the bus speed. If there is, it sets the subordinate speed (if there is no device on it).Since the speed is irrelevant when running in a VM, I suggest that you either ignore it altogether, or "normalize" the speed of all the devices/bridges/buses in QEMU in order to avoid any conflicts in the first place.
Before this patch, this would not help since the primary bus speed is not even set by the kernel.
After this patch, normalizing the QEMU device/bus/... speed may help avoiding future conflicts, but we should discuss it in qemu mailing list :).
Ignoring emulated/para-virt device's speed when running in a VM it is\
a good question, but I don't know if such a distinction would be feasible.
Any thoughts would be appreciated,
Thanks,
Marcel
Quoted 33 lines
Thanks, Ronen.
Quoted 29 lines
However, it takes the reference of the board speed from the primary bus and not from the subordinate. If the primary bus is PCI and not PCIX/PCIe, its speed is not updated and remains
0xff. As a result hotplug fails with error: "Speed of bus ff and adapter 0 mismatch".Fixed that by checking the speed against the subordinate bus.
Signed-off-by Marcel Apfelbaum <[email protected]>--- drivers/pci/hotplug/shpchp_ctrl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)drivers/pci/hotplug/shpchp_ctrl.c +2
-2@@ -282,8 +282,8 @@ static int board_added(struct slot *p_slot)return WRONG_BUS_FREQUENCY ; } - bsp = ctrl->pci_dev->bus->cur_bus_speed; - msp = ctrl->pci_dev->bus->max_bus_speed; + bsp = ctrl->pci_dev->subordinate->cur_bus_speed; + msp = ctrl->pci_dev->subordinate->max_bus_speed;/* Check if there are other slots or devices on the same bus */ if (!list_empty (&ctrl->pci_dev->subordinate->devices))
Quoted 9 lines · Marcel Apfelbaum
When a board is added, the shpchp driver checks if there
is a mismatch between the bridge's adapter and the bus speed. If there is, it sets the subordinate speed (if there is no device on it).However, it takes the reference of the board speed from the primary bus and not from the subordinate. If the primary bus is PCI and not PCIX/PCIe, its speed is not updated and remains
0xff. As a result hotplug fails with error: "Speed of bus ff and adapter 0 mismatch".
Quoted 4 lines
Fixed that by checking the speed against the subordinate bus.
Signed-off-by Marcel Apfelbaum <[email protected]>
I think cpqphp has similar issues. But I can't test it and I don't know if anybody even uses it any more, so I'm just going to leave it alone.
I reworked the changelog because I got confused about the bridge, adapter, board, and what was what. Let me know if the one below is inaccurate.
I'll include this for v3.15 since it's a regression fix.
Thanks,
Bjorn
commit be219b7f518a36f62d67f057e9d31ebe9674814f
Author: Marcel Apfelbaum <[email protected]> Date: Thu May 1 17:35:48 2014 +0300
PCI: shpchp: Check bridge's secondary (not primary) bus speed
When a new device is added below a hotplug bridge, the bridge's secondary bus speed and the device's bus speed must match. The shpchp driver previously checked the bridge's *primary* bus speed, not the secondary bus speed.
This caused hot-add errors like:
shpchp 0000:00:03.0: Speed of bus ff and adapter 0 mismatch
Check the secondary bus speed instead.
drivers/pci/hotplug/shpchp_ctrl.c +2 -2
@@ -282,8 +282,8 @@ static int board_added(struct slot *p_slot)return WRONG_BUS_FREQUENCY ; } - bsp = ctrl->pci_dev->bus->cur_bus_speed; - msp = ctrl->pci_dev->bus->max_bus_speed; + bsp = ctrl->pci_dev->subordinate->cur_bus_speed; + msp = ctrl->pci_dev->subordinate->max_bus_speed;/* Check if there are other slots or devices on the same bus */ if (!list_empty (&ctrl->pci_dev->subordinate->devices))
Quoted 12 lines · Bjorn Helgaas
Quoted 11 lines · Marcel Apfelbaum
When a board is added, the shpchp driver checks if there
is a mismatch between the bridge's adapter and the bus speed. If there is, it sets the subordinate speed (if there is no device on it).However, it takes the reference of the board speed from the primary bus and not from the subordinate. If the primary bus is PCI and not PCIX/PCIe, its speed is not updated and remains
0xff. As a result hotplug fails with error: "Speed of bus ff and adapter 0 mismatch".Fixed that by checking the speed against the subordinate bus.
Quoted 8 lines
Quoted 2 lines
Signed-off-by Marcel Apfelbaum <[email protected]>I think cpqphp has similar issues. But I can't test it and I don't know if anybody even uses it any more, so I'm just going to leave it alone.
I reworked the changelog because I got confused about the bridge, adapter, board, and what was what. Let me know if the one below is inaccurate.
It's accurate I think. I only wish someone reworked the actual shpchp code to avoid the confusing adapter/board terminology.
Quoted 45 lines
I'll include this for v3.15 since it's a regression fix.
Thanks,
Bjorncommit
be219b7f518a36f62d67f057e9d31ebe9674814fAuthor: Marcel Apfelbaum <[email protected]> Date: Thu May 1 17:35:48 2014 +0300PCI: shpchp: Check bridge's secondary (not primary) bus speed
When a new device is added below a hotplug bridge, the bridge's secondary bus speed and the device's bus speed must match. The shpchp driver previously checked the bridge's *primary* bus speed, not the secondary bus speed.
This caused hot-add errors like:
shpchp 0000:00:03.0: Speed of bus ff and adapter 0 mismatch
Check the secondary bus speed instead.
[bhelgaas: changelog]Fixes 3749c51ac6c1 ("PCI: Make current and maximum bus speeds part of the PCI core")Signed-off-by Marcel Apfelbaum <[email protected]>Signed-off-by Bjorn Helgaas <[email protected]>CC [email protected] # v2.6.34+drivers/pci/hotplug/shpchp_ctrl.c +2
-2@@ -282,8 +282,8 @@ static int board_added(struct slot *p_slot)return WRONG_BUS_FREQUENCY ; } - bsp = ctrl->pci_dev->bus->cur_bus_speed; - msp = ctrl->pci_dev->bus->max_bus_speed; + bsp = ctrl->pci_dev->subordinate->cur_bus_speed; + msp = ctrl->pci_dev->subordinate->max_bus_speed;/* Check if there are other slots or devices on the same bus */ if (!list_empty (&ctrl->pci_dev->subordinate->devices))