PCI: generic: remove dependency on hw_pci

The current code in pci-host-generic.c uses pci_common_init_dev() from the arch/arm/ to do a part of the PCI initialization, and this prevents it from being used on arm64.

The initialization done by pci_common_init_dev() that is really needed by pci-host-generic.c can be done in the same file without using the hw_pci API of ARM.

The ARM platform requires a pci_sys_data as sysdata for the PCI bus, this is be handled by setting up 'struct gen_pci' to embed a pci_sys_data variable as the first element on the ARM platform.

Signed-off-by Jayachandran C <[email protected]>

---
Here's v2 of the patches, this enables use of pci-host-generic on arm64.

This has been tested on both qemu and fast model for arm64, and on qemu for arm32.

v1->v2
- Address comments from Arnd Bergmann and Lorenzo Pieralisi
- move contents of gen_pci_init to gen_pci_probe
- assign resources only when !probe_only
- tested on ARM32 with qemu option -M virt

Quoted 17 lines · Jayachandran C

The current code in pci-host-generic.c uses pci_common_init_dev() from the arch/arm/ to do a part of the PCI initialization, and this prevents it from being used on arm64.

The initialization done by pci_common_init_dev() that is really needed by pci-host-generic.c can be done in the same file without using the hw_pci API of ARM.

The ARM platform requires a pci_sys_data as sysdata for the PCI bus, this is be handled by setting up 'struct gen_pci' to embed a pci_sys_data variable as the first element on the ARM platform.

Signed-off-by Jayachandran C <[email protected]>

---
Here's v2 of the patches, this enables use of pci-host-generic on arm64.

Quoted 8 lines

This has been tested on both qemu and fast model for arm64, and on qemu for arm32.

v1->v2
- Address comments from Arnd Bergmann and Lorenzo Pieralisi
- move contents of gen_pci_init to gen_pci_probe
- assign resources only when !probe_only
- tested on ARM32 with qemu option -M virt

I tried this with an arm64 kernel running under kvmtool, but I get the following errors (a 32-bit ARM kernel does seem to work):

PCI host bridge /pci ranges:

     IO 0x00000000..0x0000ffff -> 0x00000000
    MEM 0x41000000..0x7fffffff -> 0x41000000

pci-host-generic 40000000.pci: PCI host bridge to bus 0000:00 pci_bus 0000:00: root bus resource [bus 00-01] pci_bus 0000:00: root bus resource [io 0x0000-0xffff] pci_bus 0000:00: root bus resource [mem 0x41000000-0x7fffffff] pci_bus 0000:00: scanning bus
pci 0000:00:00.0: [1af4:1009] type 00 class 0xff0000 pci 0000:00:00.0: reg 0x10: [mem 0x41000000-0x410003ff] pci 0000:00:00.0: reg 0x14: [io 0x6200-0x65ff]
pci 0000:00:00.0: reg 0x18: [mem 0x41000400-0x410005ff] pci 0000:00:01.0: [1af4:1009] type 00 class 0xff0000 pci 0000:00:01.0: reg 0x10: [mem 0x41000800-0x41000bff] pci 0000:00:01.0: reg 0x14: [io 0x6600-0x69ff]
pci 0000:00:01.0: reg 0x18: [mem 0x41000c00-0x41000dff] pci_bus 0000:00: fixups for bus
pci_bus 0000:00: bus scan returning with max=00

pci 0000:00:00.0: fixup irq: got 10
pci 0000:00:00.0: assigning IRQ 10
pci 0000:00:01.0: fixup irq: got 11
pci 0000:00:01.0: assigning IRQ 11
virtio-pci 0000:00:00.0: can't enable device: BAR 0 [mem 0x41000000-0x410003ff] not claimed
virtio-pci: probe of 0000:00:00.0 failed with error -22
virtio-pci 0000:00:01.0: can't enable device: BAR 0 [mem 0x41000800-0x41000bff] not claimed
virtio-pci: probe of 0000:00:01.0 failed with error -22

Quoted 17 lines · Jayachandran C

The current code in pci-host-generic.c uses pci_common_init_dev() from the arch/arm/ to do a part of the PCI initialization, and this prevents it from being used on arm64.

The initialization done by pci_common_init_dev() that is really needed by pci-host-generic.c can be done in the same file without using the hw_pci API of ARM.

The ARM platform requires a pci_sys_data as sysdata for the PCI bus, this is be handled by setting up 'struct gen_pci' to embed a pci_sys_data variable as the first element on the ARM platform.

Signed-off-by Jayachandran C <[email protected]>

---
Here's v2 of the patches, this enables use of pci-host-generic on arm64.

Quoted 8 lines

This has been tested on both qemu and fast model for arm64, and on qemu for arm32.

v1->v2
- Address comments from Arnd Bergmann and Lorenzo Pieralisi
- move contents of gen_pci_init to gen_pci_probe
- assign resources only when !probe_only
- tested on ARM32 with qemu option -M virt

I tried this with an arm64 kernel running under kvmtool, but I get the following errors (a 32-bit ARM kernel does seem to work):

PCI host bridge /pci ranges:

     IO 0x00000000..0x0000ffff -> 0x00000000
    MEM 0x41000000..0x7fffffff -> 0x41000000

pci-host-generic 40000000.pci: PCI host bridge to bus 0000:00 pci_bus 0000:00: root bus resource [bus 00-01] pci_bus 0000:00: root bus resource [io 0x0000-0xffff] pci_bus 0000:00: root bus resource [mem 0x41000000-0x7fffffff] pci_bus 0000:00: scanning bus
pci 0000:00:00.0: [1af4:1009] type 00 class 0xff0000 pci 0000:00:00.0: reg 0x10: [mem 0x41000000-0x410003ff] pci 0000:00:00.0: reg 0x14: [io 0x6200-0x65ff]
pci 0000:00:00.0: reg 0x18: [mem 0x41000400-0x410005ff] pci 0000:00:01.0: [1af4:1009] type 00 class 0xff0000 pci 0000:00:01.0: reg 0x10: [mem 0x41000800-0x41000bff] pci 0000:00:01.0: reg 0x14: [io 0x6600-0x69ff]
pci 0000:00:01.0: reg 0x18: [mem 0x41000c00-0x41000dff] pci_bus 0000:00: fixups for bus
pci_bus 0000:00: bus scan returning with max=00

pci 0000:00:00.0: fixup irq: got 10
pci 0000:00:00.0: assigning IRQ 10
pci 0000:00:01.0: fixup irq: got 11
pci 0000:00:01.0: assigning IRQ 11
virtio-pci 0000:00:00.0: can't enable device: BAR 0 [mem 0x41000000-0x410003ff] not claimed
virtio-pci: probe of 0000:00:00.0 failed with error -22
virtio-pci 0000:00:01.0: can't enable device: BAR 0 [mem 0x41000800-0x41000bff] not claimed
virtio-pci: probe of 0000:00:01.0 failed with error -22

Quoted 17 lines · Jayachandran C

The current code in pci-host-generic.c uses pci_common_init_dev() from the arch/arm/ to do a part of the PCI initialization, and this prevents it from being used on arm64.

The initialization done by pci_common_init_dev() that is really needed by pci-host-generic.c can be done in the same file without using the hw_pci API of ARM.

The ARM platform requires a pci_sys_data as sysdata for the PCI bus, this is be handled by setting up 'struct gen_pci' to embed a pci_sys_data variable as the first element on the ARM platform.

Signed-off-by Jayachandran C <[email protected]>

---
Here's v2 of the patches, this enables use of pci-host-generic on arm64.

Quoted 8 lines

This has been tested on both qemu and fast model for arm64, and on qemu for arm32.

v1->v2
- Address comments from Arnd Bergmann and Lorenzo Pieralisi
- move contents of gen_pci_init to gen_pci_probe
- assign resources only when !probe_only
- tested on ARM32 with qemu option -M virt

I tried this with an arm64 kernel running under kvmtool, but I get the following errors (a 32-bit ARM kernel does seem to work):

PCI host bridge /pci ranges:

     IO 0x00000000..0x0000ffff -> 0x00000000
    MEM 0x41000000..0x7fffffff -> 0x41000000

pci-host-generic 40000000.pci: PCI host bridge to bus 0000:00 pci_bus 0000:00: root bus resource [bus 00-01] pci_bus 0000:00: root bus resource [io 0x0000-0xffff] pci_bus 0000:00: root bus resource [mem 0x41000000-0x7fffffff] pci_bus 0000:00: scanning bus
pci 0000:00:00.0: [1af4:1009] type 00 class 0xff0000 pci 0000:00:00.0: reg 0x10: [mem 0x41000000-0x410003ff] pci 0000:00:00.0: reg 0x14: [io 0x6200-0x65ff]
pci 0000:00:00.0: reg 0x18: [mem 0x41000400-0x410005ff] pci 0000:00:01.0: [1af4:1009] type 00 class 0xff0000 pci 0000:00:01.0: reg 0x10: [mem 0x41000800-0x41000bff] pci 0000:00:01.0: reg 0x14: [io 0x6600-0x69ff]
pci 0000:00:01.0: reg 0x18: [mem 0x41000c00-0x41000dff] pci_bus 0000:00: fixups for bus
pci_bus 0000:00: bus scan returning with max=00

pci 0000:00:00.0: fixup irq: got 10
pci 0000:00:00.0: assigning IRQ 10
pci 0000:00:01.0: fixup irq: got 11
pci 0000:00:01.0: assigning IRQ 11
virtio-pci 0000:00:00.0: can't enable device: BAR 0 [mem 0x41000000-0x410003ff] not claimed
virtio-pci: probe of 0000:00:00.0 failed with error -22
virtio-pci 0000:00:01.0: can't enable device: BAR 0 [mem 0x41000800-0x41000bff] not claimed
virtio-pci: probe of 0000:00:01.0 failed with error -22

Quoted 48 lines · Will Deacon

Quoted 17 lines · Jayachandran C

The current code in pci-host-generic.c uses pci_common_init_dev() from the arch/arm/ to do a part of the PCI initialization, and this prevents it from being used on arm64.

The initialization done by pci_common_init_dev() that is really needed by pci-host-generic.c can be done in the same file without using the hw_pci API of ARM.

The ARM platform requires a pci_sys_data as sysdata for the PCI bus, this is be handled by setting up 'struct gen_pci' to embed a pci_sys_data variable as the first element on the ARM platform.

Signed-off-by Jayachandran C <[email protected]>

---
Here's v2 of the patches, this enables use of pci-host-generic on arm64.

Quoted 8 lines

This has been tested on both qemu and fast model for arm64, and on qemu for arm32.

v1->v2
- Address comments from Arnd Bergmann and Lorenzo Pieralisi
- move contents of gen_pci_init to gen_pci_probe
- assign resources only when !probe_only
- tested on ARM32 with qemu option -M virt

I tried this with an arm64 kernel running under kvmtool, but I get the following errors (a 32-bit ARM kernel does seem to work):

PCI host bridge /pci ranges:

     IO 0x00000000..0x0000ffff -> 0x00000000
    MEM 0x41000000..0x7fffffff -> 0x41000000

pci-host-generic 40000000.pci: PCI host bridge to bus 0000:00 pci_bus 0000:00: root bus resource [bus 00-01] pci_bus 0000:00: root bus resource [io 0x0000-0xffff] pci_bus 0000:00: root bus resource [mem 0x41000000-0x7fffffff] pci_bus 0000:00: scanning bus
pci 0000:00:00.0: [1af4:1009] type 00 class 0xff0000 pci 0000:00:00.0: reg 0x10: [mem 0x41000000-0x410003ff] pci 0000:00:00.0: reg 0x14: [io 0x6200-0x65ff]
pci 0000:00:00.0: reg 0x18: [mem 0x41000400-0x410005ff] pci 0000:00:01.0: [1af4:1009] type 00 class 0xff0000 pci 0000:00:01.0: reg 0x10: [mem 0x41000800-0x41000bff] pci 0000:00:01.0: reg 0x14: [io 0x6600-0x69ff]
pci 0000:00:01.0: reg 0x18: [mem 0x41000c00-0x41000dff] pci_bus 0000:00: fixups for bus
pci_bus 0000:00: bus scan returning with max=00

Quoted 8 lines

pci 0000:00:00.0: fixup irq: got 10
pci 0000:00:00.0: assigning IRQ 10
pci 0000:00:01.0: fixup irq: got 11
pci 0000:00:01.0: assigning IRQ 11
virtio-pci 0000:00:00.0: can't enable device: BAR 0 [mem 0x41000000-0x410003ff] not claimed
virtio-pci: probe of 0000:00:00.0 failed with error -22
virtio-pci 0000:00:01.0: can't enable device: BAR 0 [mem 0x41000800-0x41000bff] not claimed
virtio-pci: probe of 0000:00:01.0 failed with error -22

Ok, had a further look.

Referring to this thread:

https://lkml.org/lkml/2014/9/29/557

By looking at other architectures code, resources should be claimed (ie requested) even when PCI_PROBE_ONLY is set. Alpha, Sparc and PowerPC seem to do that, in slightly different fashions.

I do not think, as Bjorn mentioned, that PCI_PROBE_ONLY should be used to prevent enabling resources through a PCI command, which is what pci_enable_resources does.

What we can do, is providing a generic PCI layer API that allows claiming resources for a specific PCI bus, something similar if not identical to what is done on alpha:

arch/alpha/kernel/pci.c pcibios_claim_one_bus()

that is not alpha specific at all. That way, we can use the API to claim bus resources instead of assigning them on PCI_PROBE_ONLY (I *think* that alpha calls pci_assign_unassigned_resources() even if PCI_PROBE_ONLY is set, it should be safe since resources are claimed first so IIUC the PCI layer would revert to FW BAR configuration on assignment failure).

Bjorn, any opinion on this ? Putting together a patch is easy when we agree on the solution.

Quoted 85 lines · Lorenzo Pieralisi

Quoted 48 lines · Will Deacon

Quoted 17 lines · Jayachandran C

The current code in pci-host-generic.c uses pci_common_init_dev() from the arch/arm/ to do a part of the PCI initialization, and this prevents it from being used on arm64.

The initialization done by pci_common_init_dev() that is really needed by pci-host-generic.c can be done in the same file without using the hw_pci API of ARM.

The ARM platform requires a pci_sys_data as sysdata for the PCI bus, this is be handled by setting up 'struct gen_pci' to embed a pci_sys_data variable as the first element on the ARM platform.

Signed-off-by Jayachandran C <[email protected]>

---
Here's v2 of the patches, this enables use of pci-host-generic on arm64.

Quoted 8 lines

This has been tested on both qemu and fast model for arm64, and on qemu for arm32.

v1->v2
- Address comments from Arnd Bergmann and Lorenzo Pieralisi
- move contents of gen_pci_init to gen_pci_probe
- assign resources only when !probe_only
- tested on ARM32 with qemu option -M virt

I tried this with an arm64 kernel running under kvmtool, but I get the following errors (a 32-bit ARM kernel does seem to work):

   PCI host bridge /pci ranges:
      IO 0x00000000..0x0000ffff -> 0x00000000
     MEM 0x41000000..0x7fffffff -> 0x41000000
   pci-host-generic 40000000.pci: PCI host bridge to bus 0000:00
   pci_bus 0000:00: root bus resource [bus 00-01]
   pci_bus 0000:00: root bus resource [io  0x0000-0xffff]
   pci_bus 0000:00: root bus resource [mem 0x41000000-0x7fffffff]
   pci_bus 0000:00: scanning bus
   pci 0000:00:00.0: [1af4:1009] type 00 class 0xff0000
   pci 0000:00:00.0: reg 0x10: [mem 0x41000000-0x410003ff]
   pci 0000:00:00.0: reg 0x14: [io  0x6200-0x65ff]
   pci 0000:00:00.0: reg 0x18: [mem 0x41000400-0x410005ff]
   pci 0000:00:01.0: [1af4:1009] type 00 class 0xff0000
   pci 0000:00:01.0: reg 0x10: [mem 0x41000800-0x41000bff]
   pci 0000:00:01.0: reg 0x14: [io  0x6600-0x69ff]
   pci 0000:00:01.0: reg 0x18: [mem 0x41000c00-0x41000dff]
   pci_bus 0000:00: fixups for bus
   pci_bus 0000:00: bus scan returning with max=00

Quoted 8 lines

   pci 0000:00:00.0: fixup irq: got 10
   pci 0000:00:00.0: assigning IRQ 10
   pci 0000:00:01.0: fixup irq: got 11
   pci 0000:00:01.0: assigning IRQ 11
   virtio-pci 0000:00:00.0: can't enable device: BAR 0 [mem 0x41000000-0x410003ff] not claimed
   virtio-pci: probe of 0000:00:00.0 failed with error -22
   virtio-pci 0000:00:01.0: can't enable device: BAR 0 [mem 0x41000800-0x41000bff] not claimed
   virtio-pci: probe of 0000:00:01.0 failed with error -22

Ok, had a further look.

Referring to this thread:

https://lkml.org/lkml/2014/9/29/557

By looking at other architectures code, resources should be claimed (ie requested) even when PCI_PROBE_ONLY is set. Alpha, Sparc and PowerPC seem to do that, in slightly different fashions.

I do not think, as Bjorn mentioned, that PCI_PROBE_ONLY should be used to prevent enabling resources through a PCI command, which is what pci_enable_resources does.

What we can do, is providing a generic PCI layer API that allows claiming resources for a specific PCI bus, something similar if not identical to what is done on alpha:

arch/alpha/kernel/pci.c pcibios_claim_one_bus()

that is not alpha specific at all. That way, we can use the API to claim bus resources instead of assigning them on PCI_PROBE_ONLY (I *think* that alpha calls pci_assign_unassigned_resources() even if PCI_PROBE_ONLY is set, it should be safe since resources are claimed first so IIUC the PCI layer would revert to FW BAR configuration on assignment failure).

Quoted 2 lines

Bjorn, any opinion on this ? Putting together a patch is easy when we agree on the solution.

I would like claiming resources, i.e., pci_claim_resource(), to happen in the core instead of in arch code because it's not inherently arch-specific. I don't think it should depend on PCI_PROBE_ONLY.

Bjorn

...
I tested this patch series on the AMD Seattle w/o PCI_PROBE_ONLY mode and that works fine. However, w/ PCI_PROBE_ONLY, I also run into the resource not claimed issue (no surprise here).

So, I tried porting the pcibios_claim_one_bus() from arch/alpha/kernel/pci.c as Lorenzo suggested, plus the a small change in pci_claim_resource(), and it seems to work w/ PCI_PROBE_ONLY. (Please see example patch below.)

The additional while loop is needed in the pci_claim_resource() since I need to reference back to the resource in the root bus, which are defined from the DT node. Does this sounds like a reasonable approach?

drivers/pci/host/pci-host-generic.c +3 -0

index e9cc559..0dfa23d 100644
@@ -261,7 +261,10 @@ static int gen_pci_probe(struct platform_device *pdev)
 	if (!pci_has_flag(PCI_PROBE_ONLY)) {
 		pci_bus_size_bridges(bus);
 		pci_bus_assign_resources(bus);
+	} else {
+		pci_claim_one_bus(bus);
 	}
+
 	pci_bus_add_devices(bus);
 
 	/* Configure PCI Express settings */

drivers/pci/setup-res.c +1 -0

index 232f925..d4b43b3 100644
@@ -109,6 +109,7 @@ int pci_claim_resource(struct pci_dev *dev, int
 resource)
 {
 	struct resource *res = &dev->resource[resource];
 	struct resource *root, *conflict;
+	struct pci_dev *pdev = dev;
 
 	if (res->flags & IORESOURCE_UNSET) {

drivers/pci/setup-res.c +1 -1

 		dev_info(&dev->dev, "can't claim BAR %d %pR: no address assigned\n",

@@ -116,7 +117,18 @@ int pci_claim_resource(struct pci_dev *dev, int
 resource)
 		return -EINVAL;
 	}
 
-	root = pci_find_parent_resource(dev, res);
+	while (pdev) {

drivers/pci/setup-res.c +1 -0

+
 	if (!root) {
 		dev_info(&dev->dev, "can't claim BAR %d %pR: no compatible bridge

window\n",

drivers/pci/setup-res.c +19 -0

 			 resource, res);

@@ -136,6 +148,36 @@ int pci_claim_resource(struct pci_dev *dev, int
 resource)
 }
 EXPORT_SYMBOL(pci_claim_resource);
 
+void pci_claim_one_bus(struct pci_bus *b)
+{
+	struct pci_dev *pdev;
+	struct pci_bus *child_bus;
+
+	list_for_each_entry(pdev, &b->devices, bus_list) {
+		int i;
+
+		for (i = 0; i < PCI_NUM_RESOURCES; i++) {
+			struct resource *r = &pdev->resource[i];
+
+			if (r->parent || !r->start || !r->flags)
+				continue;
+
+			if (pci_has_flag(PCI_PROBE_ONLY) ||
+			    (r->flags & IORESOURCE_PCI_FIXED)) {
+				if (pci_claim_resource(pdev, i) == 0)
+					continue;
+

drivers/pci/setup-res.c +1 -0

+
 void pci_disable_bridge_window(struct pci_dev *dev)
 {

drivers/pci/setup-res.c +0 -0

 	dev_info(&dev->dev, "disabling bridge mem windows\n");

include/linux/pci.h +1 -0

index 353db8d..b59ad4b 100644
@@ -1085,6 +1085,7 @@ void
 pci_assign_unassigned_bridge_resources(struct pci_dev *bridge);
 void pci_assign_unassigned_bus_resources(struct pci_bus *bus);
 void pci_assign_unassigned_root_bus_resources(struct pci_bus *bus);
 void pdev_enable_device(struct pci_dev *);
+void pci_claim_one_bus(struct pci_bus *b);
 int pci_enable_resources(struct pci_dev *, int mask);
 void pci_fixup_irqs(u8 (*)(struct pci_dev *, u8 *),

int (*)(const struct pci_dev *, u8, u8));
-------- END PATCH ----

Thanks,

Suravee

Quoted 19 lines

(quoted diff) +19 -0

+void pci_claim_one_bus(struct pci_bus *b)
+{
+	struct pci_dev *pdev;
+	struct pci_bus *child_bus;
+
+	list_for_each_entry(pdev, &b->devices, bus_list) {
+		int i;
+
+		for (i = 0; i < PCI_NUM_RESOURCES; i++) {
+			struct resource *r = &pdev->resource[i];
+
+			if (r->parent || !r->start || !r->flags)
+				continue;
+
+			if (pci_has_flag(PCI_PROBE_ONLY) ||
+			    (r->flags & IORESOURCE_PCI_FIXED)) {
+				if (pci_claim_resource(pdev, i) == 0)
+					continue;
+

Review

Quoted 10 lines

(quoted diff) +10 -0

+				pci_claim_bridge_resource(pdev, i);
+			}
+		}
+	}
+
+	list_for_each_entry(child_bus, &b->children, node) {
+		pci_claim_one_bus(child_bus);
+	}
+}
+EXPORT_SYMBOL(pci_claim_one_bus);

I'm not a fan of pci_claim_one_bus(), on the philosophical grounds that claiming resources is a per-device thing, and I don't want to encourage people to do it on a per-bus level.

I'd rather claim them somewhere in the pci_device_add() path, as s390 does in pcibios_add_device(). In fact, I'd *like* to do it even earlier, when we read each BAR, so we could identify invalid or unassigned BARs immediately.

On Tue, May 12, 2015 at 02:34:31PM +0100, Bjorn Helgaas wrote:

[...]

Quoted 34 lines

Quoted 19 lines

(quoted diff) +19 -0

+void pci_claim_one_bus(struct pci_bus *b)
+{
+   struct pci_dev *pdev;
+   struct pci_bus *child_bus;
+
+   list_for_each_entry(pdev, &b->devices, bus_list) {
+           int i;
+
+           for (i = 0; i < PCI_NUM_RESOURCES; i++) {
+                   struct resource *r = &pdev->resource[i];
+
+                   if (r->parent || !r->start || !r->flags)
+                           continue;
+
+                   if (pci_has_flag(PCI_PROBE_ONLY) ||
+                       (r->flags & IORESOURCE_PCI_FIXED)) {
+                           if (pci_claim_resource(pdev, i) == 0)
+                                   continue;
+

Review

Quoted 10 lines

(quoted diff) +10 -0

+                           pci_claim_bridge_resource(pdev, i);
+                   }
+           }
+   }
+
+   list_for_each_entry(child_bus, &b->children, node) {
+           pci_claim_one_bus(child_bus);
+   }
+}
+EXPORT_SYMBOL(pci_claim_one_bus);

I'm not a fan of pci_claim_one_bus(), on the philosophical grounds that claiming resources is a per-device thing, and I don't want to encourage people to do it on a per-bus level.

Quoted 4 lines

I'd rather claim them somewhere in the pci_device_add() path, as s390 does in pcibios_add_device(). In fact, I'd *like* to do it even earlier, when we read each BAR, so we could identify invalid or unassigned BARs immediately.

You mean claiming the resources in __pci_read_base (and unset the resource if claiming it fails ?) regardless of PCI_PROBE_ONLY ?

We could claim the resources in pcibios_add_device on arm64, but this means arm code should be patched too since I am not happy at all to let arm and arm64 diverge even more.

Hi All,

I tested this patch series on the AMD Seattle w/o PCI_PROBE_ONLY mode and that works fine. However, w/ PCI_PROBE_ONLY, I also run into the resource not claimed issue (no surprise here).

So, I tried porting the pcibios_claim_one_bus() from arch/alpha/kernel/pci.c as Lorenzo suggested, plus the a small change in pci_claim_resource(), and it seems to work w/ PCI_PROBE_ONLY. (Please see example patch below.)

The additional while loop is needed in the pci_claim_resource() since I need to reference back to the resource in the root bus, which are defined from the DT node. Does this sounds like a reasonable approach?

drivers/pci/host/pci-host-generic.c +3 -0

index e9cc559..0dfa23d 100644
@@ -261,7 +261,10 @@ static int gen_pci_probe(struct platform_device *pdev)
      if (!pci_has_flag(PCI_PROBE_ONLY)) {
          pci_bus_size_bridges(bus);
          pci_bus_assign_resources(bus);
+    } else {
+        pci_claim_one_bus(bus);
      }
+
      pci_bus_add_devices(bus);
 
      /* Configure PCI Express settings */

drivers/pci/setup-res.c +1 -0

index 232f925..d4b43b3 100644
@@ -109,6 +109,7 @@ int pci_claim_resource(struct pci_dev *dev, int
 resource)
  {
      struct resource *res = &dev->resource[resource];
      struct resource *root, *conflict;
+    struct pci_dev *pdev = dev;
 
      if (res->flags & IORESOURCE_UNSET) {

          dev_info(&dev->dev, "can't claim BAR %d %pR: no address

assigned\n",

drivers/pci/setup-res.c +12 -1

@@ -116,7 +117,18 @@ int pci_claim_resource(struct pci_dev *dev, int
 resource)
          return -EINVAL;
      }
 
-    root = pci_find_parent_resource(dev, res);
+    while (pdev) {
+        root = pci_find_parent_resource(pdev, res);
+        if (root)
+            break;
+
+        if (pci_has_flag(PCI_PROBE_ONLY) &&
+            !pci_is_root_bus(pdev->bus))
+            pdev = pdev->bus->self;
+        else
+            break;
+    }
+
      if (!root) {
          dev_info(&dev->dev, "can't claim BAR %d %pR: no compatible

bridge window\n",

               resource, res);

Quoted 64 lines · Suravee Suthikulanit

I tested this patch series on the AMD Seattle w/o PCI_PROBE_ONLY mode and that works fine. However, w/ PCI_PROBE_ONLY, I also run into the resource not claimed issue (no surprise here).

So, I tried porting the pcibios_claim_one_bus() from arch/alpha/kernel/pci.c as Lorenzo suggested, plus the a small change in pci_claim_resource(), and it seems to work w/ PCI_PROBE_ONLY. (Please see example patch below.)

The additional while loop is needed in the pci_claim_resource() since I need to reference back to the resource in the root bus, which are defined from the DT node. Does this sounds like a reasonable approach?

drivers/pci/host/pci-host-generic.c +3 -0

index e9cc559..0dfa23d 100644
@@ -261,7 +261,10 @@ static int gen_pci_probe(struct platform_device *pdev)
     if (!pci_has_flag(PCI_PROBE_ONLY)) {
         pci_bus_size_bridges(bus);
         pci_bus_assign_resources(bus);
+    } else {
+        pci_claim_one_bus(bus);
     }
+
     pci_bus_add_devices(bus);
 
     /* Configure PCI Express settings */

drivers/pci/setup-res.c +1 -0

index 232f925..d4b43b3 100644
@@ -109,6 +109,7 @@ int pci_claim_resource(struct pci_dev *dev, int
 resource)
 {
     struct resource *res = &dev->resource[resource];
     struct resource *root, *conflict;
+    struct pci_dev *pdev = dev;
 
     if (res->flags & IORESOURCE_UNSET) {

         dev_info(&dev->dev, "can't claim BAR %d %pR: no address

assigned\n",

drivers/pci/setup-res.c +12 -1

@@ -116,7 +117,18 @@ int pci_claim_resource(struct pci_dev *dev, int
 resource)
         return -EINVAL;
     }
 
-    root = pci_find_parent_resource(dev, res);
+    while (pdev) {
+        root = pci_find_parent_resource(pdev, res);
+        if (root)
+            break;
+
+        if (pci_has_flag(PCI_PROBE_ONLY) &&
+            !pci_is_root_bus(pdev->bus))
+            pdev = pdev->bus->self;
+        else
+            break;
+    }
+
     if (!root) {
         dev_info(&dev->dev, "can't claim BAR %d %pR: no compatible

bridge window\n",

Quoted 1 line

              resource, res);

[From Bjorn]
I don't understand this new loop. Apparently you have a device BAR, and the upstream bridge doesn't have a window that contains the BAR? That sounds like a problem with the upstream bridge resources.

Do you have an example that would make this more concrete, e.g., a host bridge, P2P bridge(s), and endpoint with their resources?

[Suravee]
Here is the information you were asking for. This information is setup from the FW. In the PCI bridge (00:02.1), I see the prefetchable memory behind bridge is already setup.

OUTPUT from lspci -tv:

-[0000:00]-+-00.0  Advanced Micro Devices, Inc. [AMD] Device 1a00
           +-02.0  Advanced Micro Devices, Inc. [AMD] Device 1a01
           \-02.1  PCI Bridge -[01]--+-00.0  Intel Corporation
82599ES 10-Gigabit SFI/SFP+ Network Connection
                        \-00.1  Intel Corporation 82599ES 10-Gigabit

SFI/SFP+ Network Connection

OUTPUT from lspci -vvv:

00:00.0 Host bridge: Advanced Micro Devices, Inc. [AMD] Device 1a00
Subsystem: Advanced Micro Devices, Inc. [AMD] Device 1a00
Control: I/O- Mem- BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr-
Stepping- SERR- FastB2B- DisINTx-
Status: Cap- 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort-
<TAbort- <MAbort- >SERR- <PERR- INTx-
Latency: 0

00:02.0 Host bridge: Advanced Micro Devices, Inc. [AMD] Device 1a01
Control: I/O- Mem- BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr-
Stepping- SERR- FastB2B- DisINTx-
Status: Cap- 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort-
<TAbort- <MAbort- >SERR- <PERR- INTx-

00:02.1 PCI bridge: Advanced Micro Devices, Inc. [AMD] Device 1a02 (prog-if 00 [Normal decode])
Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr-
Stepping- SERR- FastB2B- DisINTx-
Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort-
<TAbort- <MAbort- >SERR- <PERR- INTx-

	Latency: 0
	Bus: primary=00, secondary=01, subordinate=01, sec-latency=0
	I/O behind bridge: 00fff000-00000fff
	Memory behind bridge: fff00000-000fffff
	Prefetchable memory behind bridge: 0000007fffe00000-0000007fffffffff
	Secondary status: 66MHz- FastB2B- ParErr- DEVSEL=fast >TAbort-

<TAbort- <MAbort+ <SERR- <PERR-
BridgeCtl: Parity- SERR- NoISA- VGA- MAbort- >Reset- FastB2B-
PriDiscTmr- SecDiscTmr- DiscTmrStat- DiscTmrSERREn-
.......

01:00.0 Ethernet controller: Intel Corporation 82599ES 10-Gigabit SFI/SFP+ Network Connection (rev 01)
Subsystem: Intel Corporation Ethernet Server Adapter X520-2
Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr-
Stepping- SERR- FastB2B- DisINTx+
Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort-
<TAbort- <MAbort- >SERR- <PERR- INTx-

	Latency: 0
	Interrupt: pin A routed to IRQ 46
	Region 0: Memory at 7fffe80000 (64-bit, prefetchable) [size=512K]
	Region 2: I/O ports at 0020 [size=32]
	Region 4: Memory at 7ffff04000 (64-bit, prefetchable) [size=16K]

.......

01:00.1 Ethernet controller: Intel Corporation 82599ES 10-Gigabit SFI/SFP+ Network Connection (rev 01)
Subsystem: Intel Corporation Ethernet Server Adapter X520-2
Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr-
Stepping- SERR- FastB2B- DisINTx+
Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort-
<TAbort- <MAbort- >SERR- <PERR- INTx-

	Latency: 0
	Interrupt: pin B routed to IRQ 47
	Region 0: Memory at 7fffe00000 (64-bit, prefetchable) [size=512K]
	Region 2: I/O ports at 0000 [size=32]
	Region 4: Memory at 7ffff00000 (64-bit, prefetchable) [size=16K]

........

Thanks,
Suravee

Hi Suravee,

Quoted 73 lines · Suravee Suthikulanit

Quoted 64 lines · Suravee Suthikulanit

I tested this patch series on the AMD Seattle w/o PCI_PROBE_ONLY mode and that works fine. However, w/ PCI_PROBE_ONLY, I also run into the resource not claimed issue (no surprise here).

So, I tried porting the pcibios_claim_one_bus() from arch/alpha/kernel/pci.c as Lorenzo suggested, plus the a small change in pci_claim_resource(), and it seems to work w/ PCI_PROBE_ONLY. (Please see example patch below.)

The additional while loop is needed in the pci_claim_resource() since I need to reference back to the resource in the root bus, which are defined from the DT node. Does this sounds like a reasonable approach?

drivers/pci/host/pci-host-generic.c +3 -0

index e9cc559..0dfa23d 100644
@@ -261,7 +261,10 @@ static int gen_pci_probe(struct platform_device *pdev)
      if (!pci_has_flag(PCI_PROBE_ONLY)) {
          pci_bus_size_bridges(bus);
          pci_bus_assign_resources(bus);
+    } else {
+        pci_claim_one_bus(bus);
      }
+
      pci_bus_add_devices(bus);
 
      /* Configure PCI Express settings */

drivers/pci/setup-res.c +1 -0

index 232f925..d4b43b3 100644
@@ -109,6 +109,7 @@ int pci_claim_resource(struct pci_dev *dev, int
 resource)
  {
      struct resource *res = &dev->resource[resource];
      struct resource *root, *conflict;
+    struct pci_dev *pdev = dev;
 
      if (res->flags & IORESOURCE_UNSET) {

          dev_info(&dev->dev, "can't claim BAR %d %pR: no address

assigned\n",

drivers/pci/setup-res.c +12 -1

@@ -116,7 +117,18 @@ int pci_claim_resource(struct pci_dev *dev, int
 resource)
          return -EINVAL;
      }
 
-    root = pci_find_parent_resource(dev, res);
+    while (pdev) {
+        root = pci_find_parent_resource(pdev, res);
+        if (root)
+            break;
+
+        if (pci_has_flag(PCI_PROBE_ONLY) &&
+            !pci_is_root_bus(pdev->bus))
+            pdev = pdev->bus->self;
+        else
+            break;
+    }
+
      if (!root) {
          dev_info(&dev->dev, "can't claim BAR %d %pR: no compatible

bridge window\n",

Quoted 1 line

               resource, res);

[From Bjorn]
I don't understand this new loop. Apparently you have a device BAR, and the upstream bridge doesn't have a window that contains the BAR? That sounds like a problem with the upstream bridge resources.

Quoted 7 lines

Do you have an example that would make this more concrete, e.g., a host bridge, P2P bridge(s), and endpoint with their resources?

[Suravee]
Here is the information you were asking for. This information is setup from the FW. In the PCI bridge (00:02.1), I see the prefetchable memory behind bridge is already setup.

Here's a summary:

   00:02.1: PCI bridge to [bus 01]
   00:02.1:   [io window disabled]
   00:02.1:   [mem window disabled]
   00:02.1:   bridge window [mem 0x7f_ffe0_0000-0x7f_ffff_ffff 64bit pref]
   01:00.0: BAR 0: [mem 0x7f_ffe8_0000-0x... 64bit pref]
   01:00.0: BAR 2: [io  0x0020-0x3f]
   01:00.0: BAR 4: [mem 0x7f_fff0_4000-0x... 64bit pref]

So I guess the new loop would be for the I/O resource because the 00:02.1 I/O window is disabled? This definitely seems like a problem -- we should enable that I/O window so we can claim the 01:00.0 I/O resource with the 00:02.1 I/O window as the parent. We don't want the parent to be the host bridge window. In fact, unless we enable that I/O window, the 01:00.0 I/O BAR won't even work!

It may be that the driver for 01:00.0 doesn't need the I/O BAR. We can leave the bridge I/O window disabled and let pci_claim_resource() fail, which means we'll treat the 01:00.0 I/O BAR as unassigned. That's all fine; we'll never turn on PCI_COMMAND_IO, and as long as the driver doesn't request I/O space, everything should just work. Note that the driver would have to use pci_enable_device_mem() to tell us that it doesn't need I/O space.

Quoted 101 lines · Bjorn Helgaas

Hi Suravee,

Quoted 73 lines · Suravee Suthikulanit

Quoted 64 lines · Suravee Suthikulanit

I tested this patch series on the AMD Seattle w/o PCI_PROBE_ONLY mode and that works fine. However, w/ PCI_PROBE_ONLY, I also run into the resource not claimed issue (no surprise here).

So, I tried porting the pcibios_claim_one_bus() from arch/alpha/kernel/pci.c as Lorenzo suggested, plus the a small change in pci_claim_resource(), and it seems to work w/ PCI_PROBE_ONLY. (Please see example patch below.)

The additional while loop is needed in the pci_claim_resource() since I need to reference back to the resource in the root bus, which are defined from the DT node. Does this sounds like a reasonable approach?

drivers/pci/host/pci-host-generic.c +3 -0

index e9cc559..0dfa23d 100644
@@ -261,7 +261,10 @@ static int gen_pci_probe(struct platform_device *pdev)
     if (!pci_has_flag(PCI_PROBE_ONLY)) {
         pci_bus_size_bridges(bus);
         pci_bus_assign_resources(bus);
+    } else {
+        pci_claim_one_bus(bus);
     }
+
     pci_bus_add_devices(bus);
 
     /* Configure PCI Express settings */

drivers/pci/setup-res.c +1 -0

index 232f925..d4b43b3 100644
@@ -109,6 +109,7 @@ int pci_claim_resource(struct pci_dev *dev, int
 resource)
 {
     struct resource *res = &dev->resource[resource];
     struct resource *root, *conflict;
+    struct pci_dev *pdev = dev;
 
     if (res->flags & IORESOURCE_UNSET) {

         dev_info(&dev->dev, "can't claim BAR %d %pR: no address

assigned\n",

drivers/pci/setup-res.c +12 -1

@@ -116,7 +117,18 @@ int pci_claim_resource(struct pci_dev *dev, int
 resource)
         return -EINVAL;
     }
 
-    root = pci_find_parent_resource(dev, res);
+    while (pdev) {
+        root = pci_find_parent_resource(pdev, res);
+        if (root)
+            break;
+
+        if (pci_has_flag(PCI_PROBE_ONLY) &&
+            !pci_is_root_bus(pdev->bus))
+            pdev = pdev->bus->self;
+        else
+            break;
+    }
+
     if (!root) {
         dev_info(&dev->dev, "can't claim BAR %d %pR: no compatible

bridge window\n",

Quoted 1 line

              resource, res);

[From Bjorn]
I don't understand this new loop. Apparently you have a device BAR, and the upstream bridge doesn't have a window that contains the BAR? That sounds like a problem with the upstream bridge resources.

Quoted 7 lines

Do you have an example that would make this more concrete, e.g., a host bridge, P2P bridge(s), and endpoint with their resources?

[Suravee]
Here is the information you were asking for. This information is setup from the FW. In the PCI bridge (00:02.1), I see the prefetchable memory behind bridge is already setup.

Here's a summary:

00:02.1: PCI bridge to [bus 01]

  00:02.1:   [io window disabled]
  00:02.1:   [mem window disabled]
  00:02.1:   bridge window [mem 0x7f_ffe0_0000-0x7f_ffff_ffff 64bit pref]

01:00.0: BAR 0: [mem 0x7f_ffe8_0000-0x... 64bit pref] 01:00.0: BAR 2: [io 0x0020-0x3f]
01:00.0: BAR 4: [mem 0x7f_fff0_4000-0x... 64bit pref]

So I guess the new loop would be for the I/O resource because the 00:02.1 I/O window is disabled? This definitely seems like a problem -- we should enable that I/O window so we can claim the 01:00.0 I/O resource with the 00:02.1 I/O window as the parent. We don't want the parent to be the host bridge window. In fact, unless we enable that I/O window, the 01:00.0 I/O BAR won't even work!

Quoted 8 lines

It may be that the driver for 01:00.0 doesn't need the I/O BAR. We can leave the bridge I/O window disabled and let pci_claim_resource() fail, which means we'll treat the 01:00.0 I/O BAR as unassigned. That's all fine; we'll never turn on PCI_COMMAND_IO, and as long as the driver doesn't request I/O space, everything should just work. Note that the driver would have to use pci_enable_device_mem() to tell us that it doesn't need I/O space.

I see your point on the I/O window. Let me double check the FW why this is getting setup this way. My guess is that the I/O windows are not used by ARM64 systems, therefore the FW disabled it at the bridge.

However, the loop is mainly to recursively search for parent resource for the 64bit pref resource of device 1:00.0 when calling pci_claim_one_bus() on PROBE_ONLY. It doesn't seem to find compatible bridge windows in the parent bridge (0:2.1), and I was not sure if that is where the device is supposed to be claiming from.

Quoted 17 lines · Jayachandran C

The current code in pci-host-generic.c uses pci_common_init_dev() from the arch/arm/ to do a part of the PCI initialization, and this prevents it from being used on arm64.

The initialization done by pci_common_init_dev() that is really needed by pci-host-generic.c can be done in the same file without using the hw_pci API of ARM.

The ARM platform requires a pci_sys_data as sysdata for the PCI bus, this is be handled by setting up 'struct gen_pci' to embed a pci_sys_data variable as the first element on the ARM platform.

Signed-off-by Jayachandran C <[email protected]>

---
Here's v2 of the patches, this enables use of pci-host-generic on arm64.

Quoted 8 lines

This has been tested on both qemu and fast model for arm64, and on qemu for arm32.

v1->v2
- Address comments from Arnd Bergmann and Lorenzo Pieralisi
- move contents of gen_pci_init to gen_pci_probe
- assign resources only when !probe_only
- tested on ARM32 with qemu option -M virt

I tried this with an arm64 kernel running under kvmtool, but I get the following errors (a 32-bit ARM kernel does seem to work):

PCI host bridge /pci ranges:

     IO 0x00000000..0x0000ffff -> 0x00000000
    MEM 0x41000000..0x7fffffff -> 0x41000000

pci-host-generic 40000000.pci: PCI host bridge to bus 0000:00 pci_bus 0000:00: root bus resource [bus 00-01] pci_bus 0000:00: root bus resource [io 0x0000-0xffff] pci_bus 0000:00: root bus resource [mem 0x41000000-0x7fffffff] pci_bus 0000:00: scanning bus
pci 0000:00:00.0: [1af4:1009] type 00 class 0xff0000 pci 0000:00:00.0: reg 0x10: [mem 0x41000000-0x410003ff] pci 0000:00:00.0: reg 0x14: [io 0x6200-0x65ff]
pci 0000:00:00.0: reg 0x18: [mem 0x41000400-0x410005ff] pci 0000:00:01.0: [1af4:1009] type 00 class 0xff0000 pci 0000:00:01.0: reg 0x10: [mem 0x41000800-0x41000bff] pci 0000:00:01.0: reg 0x14: [io 0x6600-0x69ff]
pci 0000:00:01.0: reg 0x18: [mem 0x41000c00-0x41000dff] pci_bus 0000:00: fixups for bus
pci_bus 0000:00: bus scan returning with max=00

pci 0000:00:00.0: fixup irq: got 10
pci 0000:00:00.0: assigning IRQ 10
pci 0000:00:01.0: fixup irq: got 11
pci 0000:00:01.0: assigning IRQ 11
virtio-pci 0000:00:00.0: can't enable device: BAR 0 [mem 0x41000000-0x410003ff] not claimed
virtio-pci: probe of 0000:00:00.0 failed with error -22
virtio-pci 0000:00:01.0: can't enable device: BAR 0 [mem 0x41000800-0x41000bff] not claimed
virtio-pci: probe of 0000:00:01.0 failed with error -22

Quoted 48 lines · Will Deacon

Quoted 17 lines · Jayachandran C

The current code in pci-host-generic.c uses pci_common_init_dev() from the arch/arm/ to do a part of the PCI initialization, and this prevents it from being used on arm64.

The initialization done by pci_common_init_dev() that is really needed by pci-host-generic.c can be done in the same file without using the hw_pci API of ARM.

The ARM platform requires a pci_sys_data as sysdata for the PCI bus, this is be handled by setting up 'struct gen_pci' to embed a pci_sys_data variable as the first element on the ARM platform.

Signed-off-by Jayachandran C <[email protected]>

---
Here's v2 of the patches, this enables use of pci-host-generic on arm64.

Quoted 8 lines

This has been tested on both qemu and fast model for arm64, and on qemu for arm32.

v1->v2
- Address comments from Arnd Bergmann and Lorenzo Pieralisi
- move contents of gen_pci_init to gen_pci_probe
- assign resources only when !probe_only
- tested on ARM32 with qemu option -M virt

I tried this with an arm64 kernel running under kvmtool, but I get the following errors (a 32-bit ARM kernel does seem to work):

PCI host bridge /pci ranges:

     IO 0x00000000..0x0000ffff -> 0x00000000
    MEM 0x41000000..0x7fffffff -> 0x41000000

pci-host-generic 40000000.pci: PCI host bridge to bus 0000:00 pci_bus 0000:00: root bus resource [bus 00-01] pci_bus 0000:00: root bus resource [io 0x0000-0xffff] pci_bus 0000:00: root bus resource [mem 0x41000000-0x7fffffff] pci_bus 0000:00: scanning bus
pci 0000:00:00.0: [1af4:1009] type 00 class 0xff0000 pci 0000:00:00.0: reg 0x10: [mem 0x41000000-0x410003ff] pci 0000:00:00.0: reg 0x14: [io 0x6200-0x65ff]
pci 0000:00:00.0: reg 0x18: [mem 0x41000400-0x410005ff] pci 0000:00:01.0: [1af4:1009] type 00 class 0xff0000 pci 0000:00:01.0: reg 0x10: [mem 0x41000800-0x41000bff] pci 0000:00:01.0: reg 0x14: [io 0x6600-0x69ff]
pci 0000:00:01.0: reg 0x18: [mem 0x41000c00-0x41000dff] pci_bus 0000:00: fixups for bus
pci_bus 0000:00: bus scan returning with max=00

Quoted 8 lines

pci 0000:00:00.0: fixup irq: got 10
pci 0000:00:00.0: assigning IRQ 10
pci 0000:00:01.0: fixup irq: got 11
pci 0000:00:01.0: assigning IRQ 11
virtio-pci 0000:00:00.0: can't enable device: BAR 0 [mem 0x41000000-0x410003ff] not claimed
virtio-pci: probe of 0000:00:00.0 failed with error -22
virtio-pci 0000:00:01.0: can't enable device: BAR 0 [mem 0x41000800-0x41000bff] not claimed
virtio-pci: probe of 0000:00:01.0 failed with error -22

PCI_PROBE_ONLY does not work yet for arm64, this will need further changes in either arm64 or pci as noted by Lorenzo. This is not due to a problem with this patch per se.

More importantly, this does not change the current arm32 behavior and adds support for arm64 for two very useful cases
- we can use "pci-host-ecam-generic" for a host bridge controller that is initialized by firmware, without adding a custom driver.
- qemu virt platform works on arm64

If you think there are any furhter changes needed in this patchset please let me know.

Quoted 68 lines · Jayachandran C.

Quoted 48 lines · Will Deacon

Quoted 17 lines · Jayachandran C

The current code in pci-host-generic.c uses pci_common_init_dev() from the arch/arm/ to do a part of the PCI initialization, and this prevents it from being used on arm64.

The initialization done by pci_common_init_dev() that is really needed by pci-host-generic.c can be done in the same file without using the hw_pci API of ARM.

The ARM platform requires a pci_sys_data as sysdata for the PCI bus, this is be handled by setting up 'struct gen_pci' to embed a pci_sys_data variable as the first element on the ARM platform.

Signed-off-by Jayachandran C <[email protected]>

---
Here's v2 of the patches, this enables use of pci-host-generic on arm64.

Quoted 8 lines

This has been tested on both qemu and fast model for arm64, and on qemu for arm32.

v1->v2
- Address comments from Arnd Bergmann and Lorenzo Pieralisi
- move contents of gen_pci_init to gen_pci_probe
- assign resources only when !probe_only
- tested on ARM32 with qemu option -M virt

I tried this with an arm64 kernel running under kvmtool, but I get the following errors (a 32-bit ARM kernel does seem to work):

PCI host bridge /pci ranges:

     IO 0x00000000..0x0000ffff -> 0x00000000
    MEM 0x41000000..0x7fffffff -> 0x41000000

pci-host-generic 40000000.pci: PCI host bridge to bus 0000:00 pci_bus 0000:00: root bus resource [bus 00-01] pci_bus 0000:00: root bus resource [io 0x0000-0xffff] pci_bus 0000:00: root bus resource [mem 0x41000000-0x7fffffff] pci_bus 0000:00: scanning bus
pci 0000:00:00.0: [1af4:1009] type 00 class 0xff0000 pci 0000:00:00.0: reg 0x10: [mem 0x41000000-0x410003ff] pci 0000:00:00.0: reg 0x14: [io 0x6200-0x65ff]
pci 0000:00:00.0: reg 0x18: [mem 0x41000400-0x410005ff] pci 0000:00:01.0: [1af4:1009] type 00 class 0xff0000 pci 0000:00:01.0: reg 0x10: [mem 0x41000800-0x41000bff] pci 0000:00:01.0: reg 0x14: [io 0x6600-0x69ff]
pci 0000:00:01.0: reg 0x18: [mem 0x41000c00-0x41000dff] pci_bus 0000:00: fixups for bus
pci_bus 0000:00: bus scan returning with max=00

Quoted 8 lines

pci 0000:00:00.0: fixup irq: got 10
pci 0000:00:00.0: assigning IRQ 10
pci 0000:00:01.0: fixup irq: got 11
pci 0000:00:01.0: assigning IRQ 11
virtio-pci 0000:00:00.0: can't enable device: BAR 0 [mem 0x41000000-0x410003ff] not claimed
virtio-pci: probe of 0000:00:00.0 failed with error -22
virtio-pci 0000:00:01.0: can't enable device: BAR 0 [mem 0x41000800-0x41000bff] not claimed
virtio-pci: probe of 0000:00:01.0 failed with error -22

PCI_PROBE_ONLY does not work yet for arm64, this will need further changes in either arm64 or pci as noted by Lorenzo. This is not due to a problem with this patch per se.

More importantly, this does not change the current arm32 behavior and adds support for arm64 for two very useful cases
- we can use "pci-host-ecam-generic" for a host bridge controller that is initialized by firmware, without adding a custom driver.
- qemu virt platform works on arm64

Quoted 2 lines

If you think there are any furhter changes needed in this patchset please let me know.

I'm waiting for at least an ack from Will.

Quoted 72 lines · Bjorn Helgaas

Quoted 70 lines · Jayachandran C.

Quoted 48 lines · Will Deacon

Quoted 17 lines · Jayachandran C

The current code in pci-host-generic.c uses pci_common_init_dev() from the arch/arm/ to do a part of the PCI initialization, and this prevents it from being used on arm64.

The initialization done by pci_common_init_dev() that is really needed by pci-host-generic.c can be done in the same file without using the hw_pci API of ARM.

The ARM platform requires a pci_sys_data as sysdata for the PCI bus, this is be handled by setting up 'struct gen_pci' to embed a pci_sys_data variable as the first element on the ARM platform.

Signed-off-by Jayachandran C <[email protected]>

---
Here's v2 of the patches, this enables use of pci-host-generic on arm64.

Quoted 8 lines

This has been tested on both qemu and fast model for arm64, and on qemu for arm32.

v1->v2
- Address comments from Arnd Bergmann and Lorenzo Pieralisi
- move contents of gen_pci_init to gen_pci_probe
- assign resources only when !probe_only
- tested on ARM32 with qemu option -M virt

I tried this with an arm64 kernel running under kvmtool, but I get the following errors (a 32-bit ARM kernel does seem to work):

PCI host bridge /pci ranges:

     IO 0x00000000..0x0000ffff -> 0x00000000
    MEM 0x41000000..0x7fffffff -> 0x41000000

pci-host-generic 40000000.pci: PCI host bridge to bus 0000:00 pci_bus 0000:00: root bus resource [bus 00-01] pci_bus 0000:00: root bus resource [io 0x0000-0xffff] pci_bus 0000:00: root bus resource [mem 0x41000000-0x7fffffff] pci_bus 0000:00: scanning bus
pci 0000:00:00.0: [1af4:1009] type 00 class 0xff0000 pci 0000:00:00.0: reg 0x10: [mem 0x41000000-0x410003ff] pci 0000:00:00.0: reg 0x14: [io 0x6200-0x65ff]
pci 0000:00:00.0: reg 0x18: [mem 0x41000400-0x410005ff] pci 0000:00:01.0: [1af4:1009] type 00 class 0xff0000 pci 0000:00:01.0: reg 0x10: [mem 0x41000800-0x41000bff] pci 0000:00:01.0: reg 0x14: [io 0x6600-0x69ff]
pci 0000:00:01.0: reg 0x18: [mem 0x41000c00-0x41000dff] pci_bus 0000:00: fixups for bus
pci_bus 0000:00: bus scan returning with max=00

Quoted 8 lines

pci 0000:00:00.0: fixup irq: got 10
pci 0000:00:00.0: assigning IRQ 10
pci 0000:00:01.0: fixup irq: got 11
pci 0000:00:01.0: assigning IRQ 11
virtio-pci 0000:00:00.0: can't enable device: BAR 0 [mem 0x41000000-0x410003ff] not claimed
virtio-pci: probe of 0000:00:00.0 failed with error -22
virtio-pci 0000:00:01.0: can't enable device: BAR 0 [mem 0x41000800-0x41000bff] not claimed
virtio-pci: probe of 0000:00:01.0 failed with error -22

PCI_PROBE_ONLY does not work yet for arm64, this will need further changes in either arm64 or pci as noted by Lorenzo. This is not due to a problem with this patch per se.

More importantly, this does not change the current arm32 behavior and adds support for arm64 for two very useful cases
- we can use "pci-host-ecam-generic" for a host bridge controller that is initialized by firmware, without adding a custom driver.
- qemu virt platform works on arm64

If you think there are any furhter changes needed in this patchset please let me know.

Quoted 1 line

I'm waiting for at least an ack from Will.

I'm not too keen on the current state of this patch. Firstly, I'd really like to see the PROBE_ONLY case addressed at the same time as this port (Lorenzo and Suravee are working on this). Secondly, the hack to put pci_sys_data as the first member of gen_pci is ugly at best and probably makes it difficult to support MSIs on ARM.

Quoted 69 lines · Bjorn Helgaas

Quoted 68 lines · Jayachandran C.

Quoted 48 lines · Will Deacon

Quoted 17 lines · Jayachandran C

The current code in pci-host-generic.c uses pci_common_init_dev() from the arch/arm/ to do a part of the PCI initialization, and this prevents it from being used on arm64.

The initialization done by pci_common_init_dev() that is really needed by pci-host-generic.c can be done in the same file without using the hw_pci API of ARM.

The ARM platform requires a pci_sys_data as sysdata for the PCI bus, this is be handled by setting up 'struct gen_pci' to embed a pci_sys_data variable as the first element on the ARM platform.

Signed-off-by Jayachandran C <[email protected]>

---
Here's v2 of the patches, this enables use of pci-host-generic on arm64.

Quoted 8 lines

This has been tested on both qemu and fast model for arm64, and on qemu for arm32.

v1->v2
- Address comments from Arnd Bergmann and Lorenzo Pieralisi
- move contents of gen_pci_init to gen_pci_probe
- assign resources only when !probe_only
- tested on ARM32 with qemu option -M virt

I tried this with an arm64 kernel running under kvmtool, but I get the following errors (a 32-bit ARM kernel does seem to work):

PCI host bridge /pci ranges:

     IO 0x00000000..0x0000ffff -> 0x00000000
    MEM 0x41000000..0x7fffffff -> 0x41000000

pci-host-generic 40000000.pci: PCI host bridge to bus 0000:00 pci_bus 0000:00: root bus resource [bus 00-01] pci_bus 0000:00: root bus resource [io 0x0000-0xffff] pci_bus 0000:00: root bus resource [mem 0x41000000-0x7fffffff] pci_bus 0000:00: scanning bus
pci 0000:00:00.0: [1af4:1009] type 00 class 0xff0000 pci 0000:00:00.0: reg 0x10: [mem 0x41000000-0x410003ff] pci 0000:00:00.0: reg 0x14: [io 0x6200-0x65ff]
pci 0000:00:00.0: reg 0x18: [mem 0x41000400-0x410005ff] pci 0000:00:01.0: [1af4:1009] type 00 class 0xff0000 pci 0000:00:01.0: reg 0x10: [mem 0x41000800-0x41000bff] pci 0000:00:01.0: reg 0x14: [io 0x6600-0x69ff]
pci 0000:00:01.0: reg 0x18: [mem 0x41000c00-0x41000dff] pci_bus 0000:00: fixups for bus
pci_bus 0000:00: bus scan returning with max=00

Quoted 8 lines

pci 0000:00:00.0: fixup irq: got 10
pci 0000:00:00.0: assigning IRQ 10
pci 0000:00:01.0: fixup irq: got 11
pci 0000:00:01.0: assigning IRQ 11
virtio-pci 0000:00:00.0: can't enable device: BAR 0 [mem 0x41000000-0x410003ff] not claimed
virtio-pci: probe of 0000:00:00.0 failed with error -22
virtio-pci 0000:00:01.0: can't enable device: BAR 0 [mem 0x41000800-0x41000bff] not claimed
virtio-pci: probe of 0000:00:01.0 failed with error -22

PCI_PROBE_ONLY does not work yet for arm64, this will need further changes in either arm64 or pci as noted by Lorenzo. This is not due to a problem with this patch per se.

More importantly, this does not change the current arm32 behavior and adds support for arm64 for two very useful cases
- we can use "pci-host-ecam-generic" for a host bridge controller that is initialized by firmware, without adding a custom driver.
- qemu virt platform works on arm64

Quoted 4 lines

Quoted 2 lines

If you think there are any furhter changes needed in this patchset please let me know.

I'm waiting for at least an ack from Will.

I'm not too keen on the current state of this patch. Firstly, I'd really like to see the PROBE_ONLY case addressed at the same time as this port (Lorenzo and Suravee are working on this).

Secondly, the hack to put pci_sys_data as the first member of gen_pci is ugly at best

and probably makes it difficult to support MSIs on ARM.

Quoted 75 lines · Will Deacon

Quoted 73 lines · Bjorn Helgaas

Quoted 62 lines · Jayachandran C.

Quoted 48 lines · Will Deacon

Quoted 17 lines · Jayachandran C

The current code in pci-host-generic.c uses pci_common_init_dev() from the arch/arm/ to do a part of the PCI initialization, and this prevents it from being used on arm64.

The initialization done by pci_common_init_dev() that is really needed by pci-host-generic.c can be done in the same file without using the hw_pci API of ARM.

The ARM platform requires a pci_sys_data as sysdata for the PCI bus, this is be handled by setting up 'struct gen_pci' to embed a pci_sys_data variable as the first element on the ARM platform.

Signed-off-by Jayachandran C <[email protected]>

---
Here's v2 of the patches, this enables use of pci-host-generic on arm64.

Quoted 8 lines

This has been tested on both qemu and fast model for arm64, and on qemu for arm32.

v1->v2
- Address comments from Arnd Bergmann and Lorenzo Pieralisi
- move contents of gen_pci_init to gen_pci_probe
- assign resources only when !probe_only
- tested on ARM32 with qemu option -M virt

I tried this with an arm64 kernel running under kvmtool, but I get the following errors (a 32-bit ARM kernel does seem to work):

PCI host bridge /pci ranges:

     IO 0x00000000..0x0000ffff -> 0x00000000
    MEM 0x41000000..0x7fffffff -> 0x41000000

pci-host-generic 40000000.pci: PCI host bridge to bus 0000:00 pci_bus 0000:00: root bus resource [bus 00-01] pci_bus 0000:00: root bus resource [io 0x0000-0xffff] pci_bus 0000:00: root bus resource [mem 0x41000000-0x7fffffff] pci_bus 0000:00: scanning bus
pci 0000:00:00.0: [1af4:1009] type 00 class 0xff0000 pci 0000:00:00.0: reg 0x10: [mem 0x41000000-0x410003ff] pci 0000:00:00.0: reg 0x14: [io 0x6200-0x65ff]
pci 0000:00:00.0: reg 0x18: [mem 0x41000400-0x410005ff] pci 0000:00:01.0: [1af4:1009] type 00 class 0xff0000 pci 0000:00:01.0: reg 0x10: [mem 0x41000800-0x41000bff] pci 0000:00:01.0: reg 0x14: [io 0x6600-0x69ff]
pci 0000:00:01.0: reg 0x18: [mem 0x41000c00-0x41000dff] pci_bus 0000:00: fixups for bus
pci_bus 0000:00: bus scan returning with max=00

Quoted 8 lines

pci 0000:00:00.0: fixup irq: got 10
pci 0000:00:00.0: assigning IRQ 10
pci 0000:00:01.0: fixup irq: got 11
pci 0000:00:01.0: assigning IRQ 11
virtio-pci 0000:00:00.0: can't enable device: BAR 0 [mem 0x41000000-0x410003ff] not claimed
virtio-pci: probe of 0000:00:00.0 failed with error -22
virtio-pci 0000:00:01.0: can't enable device: BAR 0 [mem 0x41000800-0x41000bff] not claimed
virtio-pci: probe of 0000:00:01.0 failed with error -22

PCI_PROBE_ONLY does not work yet for arm64, this will need further changes in either arm64 or pci as noted by Lorenzo. This is not due to a problem with this patch per se.

Quoted 8 lines

More importantly, this does not change the current arm32 behavior and adds support for arm64 for two very useful cases
- we can use "pci-host-ecam-generic" for a host bridge controller that is initialized by firmware, without adding a custom driver.
- qemu virt platform works on arm64

If you think there are any furhter changes needed in this patchset please let me know.

I'm waiting for at least an ack from Will.

Quoted 3 lines

I'm not too keen on the current state of this patch. Firstly, I'd really like to see the PROBE_ONLY case addressed at the same time as this port (Lorenzo and Suravee are working on this).

Fixing the PROBE-ONLY for arm64 is probably unrelated to the scope of this patch - so I would hope that we don't NACK this patch due to that issue. This patch is very useful without PROBE-ONLY like I noted above.

Quoted 2 lines

Secondly, the hack to put pci_sys_data as the first member of gen_pci is ugly at best

On balance, it removes the problem of using hw_pci interface which has its own ugliness (Arnd's mail on this http://www.spinics.net/lists/linux-pci/msg40703.html)

Quoted 1 line

and probably makes it difficult to support MSIs on ARM.

Adding parsing of msi-parent and setting it up for arm32 and arm64 can be added fairly easily to the current code, I can post an RFC patch for this.

Quoted 5 lines

Quoted 2 lines

Secondly, the hack to put pci_sys_data as the first member of gen_pci is ugly at best

On balance, it removes the problem of using hw_pci interface which has its own ugliness (Arnd's mail on this http://www.spinics.net/lists/linux-pci/msg40703.html)

To me, that sounds like we should be disposing of the hw_pci interface altogether and moving arch/arm/ over to the new architecture-independent interface.