Change in ...coreboot[master]: device/hypertransport: Fix regression on dev_find_slot() removal

[Kyösti Mälkki (Code Review)]

Kyösti Mälkki has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/30703

Change subject: device/hypertransport: Fix regression on dev_find_slot() removal
......................................................................

device/hypertransport: Fix regression on dev_find_slot() removal

The regression is with the implementation of the functions
introduced in commit
   ad7674e device: Introduce pcidev_on_root() and friends

For these platforms, the first PCI node on devicetree
is not the root of PCI bus hierarchy, and the topology
(bus->children and dev->sibling links) are being
manipulated during HyperTransport enumeration.

This workaround reverts to old method of using dev_find_slot()
while keeping track of whether the new method would give
equivalent result.

Change-Id: I05a54de542f97159266a1c127da32665957f58f9
Signed-off-by: Kyösti Mälkki 
---
M Makefile.inc
M src/device/device.c
M src/device/device_const.c
M src/device/hypertransport.c
M src/include/device/device.h
M src/include/device/pci.h
6 files changed, 87 insertions(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/03/30703/1

diff --git a/Makefile.inc b/Makefile.inc
index 2741b19..24b5164 100644
--- a/Makefile.inc
+++ b/Makefile.inc
@@ -577,7 +577,10 @@
 verstage-y+=$(DEVICETREE_STATIC_C)
 bootblock-y+=$(DEVICETREE_STATIC_C)
 postcar-y+=$(DEVICETREE_STATIC_C)
+
+ifneq ($(CONFIG_HYPERTRANSPORT_PLUGIN_SUPPORT),y)
 smm-y+=$(DEVICETREE_STATIC_C)
+endif

#######################################################################
 # Clean up rules
diff --git a/src/device/device.c b/src/device/device.c
index 7836af1..20a4077 100644
--- a/src/device/device.c
+++ b/src/device/device.c
@@ -1253,4 +1253,7 @@
    	final_link(link);

printk(BIOS_INFO, "Devices finalized\n");
+
+	if (IS_ENABLED(CONFIG_HYPERTRANSPORT_PLUGIN_SUPPORT))
+		ht_report_devtree_sanity();
 }
diff --git a/src/device/device_const.c b/src/device/device_const.c
index ec128e8..480ac76 100644
--- a/src/device/device_const.c
+++ b/src/device/device_const.c
@@ -173,6 +173,43 @@
    return child;
 }

+/***
+ * Work around the devicetree topology being manipulated on-the-fly
+ * on systems with HyperTransport Support.
+ */
+
+static int ht_tree_needs_fixing, ht_tree_fixup_failed;
+
+void ht_report_devtree_sanity(void)
+{
+	printk(BIOS_INFO, "HT fixup counters %d / %d (rqrd/failed)\n",
+			ht_tree_needs_fixing, ht_tree_fixup_failed);
+}
+
+static const struct bus *ht_bus_reloc(const struct bus *parent,
+					      pci_devfn_t devfn)
+{
+	DEVTREE_CONST struct device *dev;
+	DEVTREE_CONST struct bus *bus;
+
+	/* HyperTransport enumeration fixes up the whacky
+	 * topology present in static devicetree.
+	 */
+	if (ENV_RAMSTAGE && (SCAN_COMPLETE == ht_scanning_status()))
+		return parent;
+
+	if (devfn >= PCI_DEVFN(0x18, 0))
+		return parent;
+
+	/* One of the links is bus for devices 0:00.x to 0:17.x. */
+	dev = pcidev_path_behind(parent, PCI_DEVFN(0x18, 0));
+	for (bus = dev->link_list; bus; bus = bus->next)
+		if (bus->children)
+			return bus;
+
+	return NULL;
+}
+
 DEVTREE_CONST struct device *pcidev_path_behind(
    const struct bus *parent, pci_devfn_t devfn)
 {
@@ -185,12 +222,34 @@

DEVTREE_CONST struct device *pcidev_path_on_root(pci_devfn_t devfn)
 {
+	DEVTREE_CONST struct device *dev;
    DEVTREE_CONST struct device *pci_domain;

pci_domain = dev_find_path(NULL, DEVICE_PATH_DOMAIN);
    if (!pci_domain)
    	return NULL;
-	return pcidev_path_behind(pci_domain->link_list, devfn);
+
+	dev = pcidev_path_behind(pci_domain->link_list, devfn);
+
+	/* Static devicetree with HyperTransport has whacky topology. */
+	if (!IS_ENABLED(CONFIG_HYPERTRANSPORT_PLUGIN_SUPPORT))
+		return dev;
+
+	DEVTREE_CONST struct device *dev_fixup, *dev_old_method;
+
+	const struct bus *root = ht_bus_reloc(pci_domain->link_list, devfn);
+	dev_fixup = pcidev_path_behind(root, devfn);
+	dev_old_method = dev_find_slot(0, devfn);
+
+	if (ENV_RAMSTAGE) {
+		if (dev != dev_old_method)
+			ht_tree_needs_fixing++;
+
+		if (dev_fixup != dev_old_method)
+			ht_tree_fixup_failed++;
+	}
+
+	return dev_old_method;
 }

DEVTREE_CONST struct device *pcidev_on_root(uint8_t dev, uint8_t fn)
diff --git a/src/device/hypertransport.c b/src/device/hypertransport.c
index ed6b2dd..fd4961e 100644
--- a/src/device/hypertransport.c
+++ b/src/device/hypertransport.c
@@ -244,6 +244,13 @@
    }
 }

+static int ht_scan_chain_process = SCAN_NOT_STARTED;
+
+int ht_scanning_status(void)
+{
+	return ht_scan_chain_process;
+}
+
 static unsigned int do_hypertransport_scan_chain(struct bus *bus, unsigned min_devfn,
    			       unsigned max_devfn,
    			       unsigned *ht_unitid_base,
@@ -276,6 +283,9 @@
    /* Restore the hypertransport chain to it's uninitialized state. */
    ht_collapse_early_enumeration(bus, offset_unitid);

+	if (bus->secondary == 0)
+		ht_scan_chain_process = SCAN_IN_PROGRESS;
+
    /* See which static device nodes I have. */
    old_devices = bus->children;
    bus->children = 0;
@@ -472,6 +482,9 @@
    		last_func->sibling = old_devices;
    }

+	if (bus->secondary == 0)
+		ht_scan_chain_process = SCAN_COMPLETE;
+
    return next_unitid;
 }

diff --git a/src/include/device/device.h b/src/include/device/device.h
index 540b7e3..40a0f58 100644
--- a/src/include/device/device.h
+++ b/src/include/device/device.h
@@ -227,6 +227,7 @@
 void show_one_resource(int debug_level, struct device *dev,
    	       struct resource *resource, const char *comment);
 void show_all_devs_resources(int debug_level, const char *msg);
+void ht_report_devtree_sanity(void);

/* Rounding for boundaries.
  * Due to some chip bugs, go ahead and round IO to 16
diff --git a/src/include/device/pci.h b/src/include/device/pci.h
index 7cf7e06..41ad281 100644
--- a/src/include/device/pci.h
+++ b/src/include/device/pci.h
@@ -122,6 +122,13 @@
 int pci_msix_table_bar(struct device *dev, u32 *offset, u8 *idx);
 struct msix_entry *pci_msix_get_table(struct device *dev);

+enum {
+	SCAN_NOT_STARTED = 0,
+	SCAN_IN_PROGRESS,
+	SCAN_COMPLETE
+};
+int ht_scanning_status(void);
+
 #define PCI_IO_BRIDGE_ALIGN 4096
 #define PCI_MEM_BRIDGE_ALIGN (1024*1024)

Change in coreboot[master]: [WIP] device/hypertransport: Fix regression on dev_find_slot() removal

[Kyösti Mälkki (Code Review)]

Kyösti Mälkki has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/30703 )

Change subject: [WIP] device/hypertransport: Fix regression on dev_find_slot() removal
......................................................................

Abandoned