When a PCI addreess range is empty PCI Library Programs Bridges with errouneous address range

[Jeffrey Hill]

It looks like defensive code as below should be added to the top of pci_set_bar in pci_cfg_auto.c so that, when the bridge address range is empty, erroneous address ranges are not programmed into the bridge device.

@@ -703,47 +707,49 @@ static void pci_set_bar(struct pci_dev *dev, int residx)
 	if ((res->flags == 0) || (res->flags & PCI_RES_FAIL))
 		return;
 
+  if ( res->end <= res->start )
+		return;
+

[Jeffrey Hill]

Considering this further, if it was a plug-and-play PCI system, then one would also need to make certain that the address range was disabled in the bridge if the topology changed in way that caused the address range in a particular bridge to become empty in a 2nd or 3rd scan after power on.

The scenario that I am concerned about would be if someone, for example, removed a card with the power on and then soft rebooted RTEMS. That might happen for example if a PCI Express over cable device was turned off.

This sounds like a very legitimate concern in our situation so I will submit another suggested path. standby...

[Jeffrey Hill]

I had a closer look and I believe that there is no need to be concerned about plug-and-play. This conclusion is based on identifying the following code.

line 309, the memory and IO spaces are initially disabled when the device or bridge is located.

PCI_CFG_W16(pcidev, PCIR_COMMAND, 0);

Line 342, the bridge's memory and IO space base addresses are cleared to zero when the bridge device is initially located

PCI_CFG_W32(pcidev, 0x28, 0);
PCI_CFG_W32(pcidev, 0x2C, 0);

[Jeffrey Hill]

This change still appears to be needed

@@ -703,47 +707,49 @@ static void pci_set_bar(struct pci_dev *dev, int residx)
 	if ((res->flags == 0) || (res->flags & PCI_RES_FAIL))
 		return;
 
+  if ( res->end <= res->start )
+		return;
+

[Jeffrey Hill]

I modified code starting at line 740 as follows, and now DEBUG diagnostics are much easier to follow.

-
-		/* Limit and Base */
-		tmp = ((res->end-1) & 0xfff00000) | (res->start >> 16);
-
-		DBG("PCI[%x:%x:%x]: BRIDGE BAR 0x%x: 0x%08x\n",
-			PCI_DEV_EXPAND(pcidev),
-			0x20 + (res->bar-BRIDGE_RES_MEMIO)*4, tmp);
-		PCI_CFG_W32(pcidev, 0x20+(res->bar-BRIDGE_RES_MEMIO)*4, tmp);
+    /* Limit and Base */
+    tmp = ((res->end-1) & 0xfff00000) | (res->start >> 16);
+#ifdef DEBUG
+    char * pName = "BOGOSITY";
+    if ( res->bar == BRIDGE_RES_MEMIO ) {
+      pName = "MEM IO";
+    }
+    else if ( res->bar == PCI_RES_MEM_PREFETCH ) {
+      pName = "MEM PREFETCH";
+    }
+    DBG("PCI[%x:%x:%x]: BRIDGE %s BAR: 0x%08x-0x%08x\n",
+      PCI_DEV_EXPAND(pcidev),
+      pName,
+      (tmp & 0xfff0 ) << 16u,
+      (tmp & 0xfff00000) | 0x000fffff);
+#endif
+    PCI_CFG_W32(pcidev, 0x20+(res->bar-BRIDGE_RES_MEMIO)*4, tmp);

[Jeffrey Hill]

One is fairly quickly convinced that there will be an issue when "end <= start" if we look at this line in the source. Refer to the PCI Bridge specification to fully understand this part of the code. The bridge window position and size registers at offset 0x20 and 0x24 are being programmed.

line 741:

    tmp = ((res->end-1) & 0xfff00000) | (res->start >> 16);

[Jeffrey Hill]

Certainly, in the above snippet a very wide window would be opened if res->end and res->start were both zero, and perhaps I recall correctly that it is normal to use those values for PCI hierarchical address spaces that are discovered to be unused.

[Jeffrey Hill]

At line 309 the memory and IO spaces are initially disabled when the device or bridge is located.

    PCI_CFG_W16(pcidev, PCIR_COMMAND, 0);

At Line 342 the bridge's 64 bit memory and memory IO space base addresses are cleared to zero when the bridge device is initially located

    PCI_CFG_W32(pcidev, 0x28, 0);
    PCI_CFG_W32(pcidev, 0x2C, 0);

However, I don't see that registers 0x1c, 0x20, and 0x24 are receiving initialization in pci_find_devs, and therefore they will need to be set later

And therefore my suggested change below isn’t going to perform the necessary initialization for the unused address spaces in a bridge.

@@ -703,47 +707,49 @@ static void pci_set_bar(struct pci_dev *dev, int residx)
 	if ((res->flags == 0) || (res->flags & PCI_RES_FAIL))
 		return;
 
+  if ( res->end <= res->start )
+		return;
+

I will need to watch it run again and verify that res->end and res->start are actually being used when there values are both zero, as I fear

[Jeffrey Hill]

Ok, res->end isnt used when it is zero so I will close out the request

[Jeffrey Hill]

Sorry, some confusion here when bringing up a new FPGA PCIe hardware designed led to some incorrect conclusions about issues in the software, and so I must retract this request.