Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1458612 > unrolled thread

x86/PCI: Scan all functions during probing

Back to article view | Back to linux.kernel

Contents

  x86/PCI: Scan all functions during probing Thomas Gleixner <tglx@linutronix.de> - 2016-08-09 13:30 +0200
    Re: x86/PCI: Scan all functions during probing Lukas Wunner <lukas@wunner.de> - 2016-08-09 14:20 +0200
    Re: x86/PCI: Scan all functions during probing Bjorn Helgaas <helgaas@kernel.org> - 2016-08-09 15:50 +0200

#1458612 — x86/PCI: Scan all functions during probing

From: Benedikt Spranger <b.spranger@linutronix.de>

PCI and PCIBIOS probing only scans devices at function number 0/8/16/...
Subdevices (e.g. multiqueue) have function numbers which are not a
multiple of 8.

Simple hypervisors (e.g. Jailhouse) pass subdevices directly w/o providing
virtual PCI mappings like KVM. As a consequence a simple PCI passthrough from
Jailhouse to a linux guest is not able to detect such devices.

Changing the probe functions to scan all function numbers makes it work. This
has no side effects and there is no reason to force the 0/8/16... probing
scheme.

Signed-off-by: Benedikt Spranger <b.spranger@linutronix.de>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 arch/x86/pci/legacy.c |    2 +-
 drivers/pci/probe.c   |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

--- a/arch/x86/pci/legacy.c
+++ b/arch/x86/pci/legacy.c
@@ -42,7 +42,7 @@ void pcibios_scan_specific_bus(int busn)
 	if (pci_find_bus(0, busn))
 		return;
 
-	for (devfn = 0; devfn < 256; devfn += 8) {
+	for (devfn = 0; devfn < 256; devfn++) {
 		if (!raw_pci_read(0, busn, devfn, PCI_VENDOR_ID, 2, &l) &&
 		    l != 0x0000 && l != 0xffff) {
 			DBG("Found device at %02x:%02x [%04x]\n", busn, devfn, l);
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -2063,7 +2063,7 @@ unsigned int pci_scan_child_bus(struct p
 	dev_dbg(&bus->dev, "scanning bus\n");
 
 	/* Go find them, Rover! */
-	for (devfn = 0; devfn < 0x100; devfn += 8)
+	for (devfn = 0; devfn < 0x100; devfn++)
 		pci_scan_slot(bus, devfn);
 
 	/* Reserve buses for SR-IOV capability. */

[toc] | [next] | [standalone]

#1458661

On Tue, Aug 09, 2016 at 01:22:30PM +0200, Thomas Gleixner wrote:
> From: Benedikt Spranger <b.spranger@linutronix.de>
> 
> PCI and PCIBIOS probing only scans devices at function number 0/8/16/...
> Subdevices (e.g. multiqueue) have function numbers which are not a
> multiple of 8.
> 
> Simple hypervisors (e.g. Jailhouse) pass subdevices directly w/o providing
> virtual PCI mappings like KVM. As a consequence a simple PCI passthrough from
> Jailhouse to a linux guest is not able to detect such devices.
> 
> Changing the probe functions to scan all function numbers makes it work. This
> has no side effects and there is no reason to force the 0/8/16... probing
> scheme.

It does have the side effect that probing (and thus booting) is prolonged.
Depending on how much that is, it may be worth pondering if usage of the
smaller stride should be constrained to platforms that really need it
(assuming they can be detected/quirked).

Just claiming "has no side effects" is going out on a limb I think.

Thanks,

Lukas

> 
> Signed-off-by: Benedikt Spranger <b.spranger@linutronix.de>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> ---
>  arch/x86/pci/legacy.c |    2 +-
>  drivers/pci/probe.c   |    2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> --- a/arch/x86/pci/legacy.c
> +++ b/arch/x86/pci/legacy.c
> @@ -42,7 +42,7 @@ void pcibios_scan_specific_bus(int busn)
>  	if (pci_find_bus(0, busn))
>  		return;
>  
> -	for (devfn = 0; devfn < 256; devfn += 8) {
> +	for (devfn = 0; devfn < 256; devfn++) {
>  		if (!raw_pci_read(0, busn, devfn, PCI_VENDOR_ID, 2, &l) &&
>  		    l != 0x0000 && l != 0xffff) {
>  			DBG("Found device at %02x:%02x [%04x]\n", busn, devfn, l);
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -2063,7 +2063,7 @@ unsigned int pci_scan_child_bus(struct p
>  	dev_dbg(&bus->dev, "scanning bus\n");
>  
>  	/* Go find them, Rover! */
> -	for (devfn = 0; devfn < 0x100; devfn += 8)
> +	for (devfn = 0; devfn < 0x100; devfn++)
>  		pci_scan_slot(bus, devfn);
>  
>  	/* Reserve buses for SR-IOV capability. */

[toc] | [prev] | [next] | [standalone]

#1458757

[+cc Lukas]

On Tue, Aug 09, 2016 at 01:22:30PM +0200, Thomas Gleixner wrote:
> From: Benedikt Spranger <b.spranger@linutronix.de>
> 
> PCI and PCIBIOS probing only scans devices at function number 0/8/16/...
> Subdevices (e.g. multiqueue) have function numbers which are not a
> multiple of 8.
> 
> Simple hypervisors (e.g. Jailhouse) pass subdevices directly w/o providing
> virtual PCI mappings like KVM. As a consequence a simple PCI passthrough from
> Jailhouse to a linux guest is not able to detect such devices.
> 
> Changing the probe functions to scan all function numbers makes it work. This
> has no side effects and there is no reason to force the 0/8/16... probing
> scheme.

"devfn" here is a 8-bit field (5 bits of device number and 3 bits of
function number), so incrementing by 8 is really a way of looking at
function 0 of each device number.  I'm pretty sure this is based on
something in the spec that says a multi-function device must implement
function 0.  Please look that up and include a reference in the
changelog so we have a more complete story here.

It's possible there are other assumptions in the code about
multi-function devices always having a function 0.  It would take a
little more research to be certain that this wouldn't break anything.

As Lukas pointed out, it does increase the number of probe attempts by
a factor of 8.  I don't know how much that will affect boot time, but
it's certainly something to consider and hopefully quantify.

> Signed-off-by: Benedikt Spranger <b.spranger@linutronix.de>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> ---
>  arch/x86/pci/legacy.c |    2 +-
>  drivers/pci/probe.c   |    2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> --- a/arch/x86/pci/legacy.c
> +++ b/arch/x86/pci/legacy.c
> @@ -42,7 +42,7 @@ void pcibios_scan_specific_bus(int busn)
>  	if (pci_find_bus(0, busn))
>  		return;
>  
> -	for (devfn = 0; devfn < 256; devfn += 8) {
> +	for (devfn = 0; devfn < 256; devfn++) {
>  		if (!raw_pci_read(0, busn, devfn, PCI_VENDOR_ID, 2, &l) &&
>  		    l != 0x0000 && l != 0xffff) {
>  			DBG("Found device at %02x:%02x [%04x]\n", busn, devfn, l);
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -2063,7 +2063,7 @@ unsigned int pci_scan_child_bus(struct p
>  	dev_dbg(&bus->dev, "scanning bus\n");
>  
>  	/* Go find them, Rover! */
> -	for (devfn = 0; devfn < 0x100; devfn += 8)
> +	for (devfn = 0; devfn < 0x100; devfn++)
>  		pci_scan_slot(bus, devfn);
>  
>  	/* Reserve buses for SR-IOV capability. */
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web