Philipp Deppenwiese has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/34285 )
Change subject: soc/intel: Fix regression with hidden PCI devices ......................................................................
soc/intel: Fix regression with hidden PCI devices
Fix regression with commit 903b40a soc/intel: Replace uses of dev_find_slot()
Platforms where FSP hides PCI devices before enumeration may halt with error message 'PCI: dev is NULL!'.
The workaround here is to print an error message revealing the faulty source code function and revert to old behaviour of dev_find_slot().
Change-Id: I5eab3e7f1993b686103eaa257aacda379dc259fa Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/34285 Reviewed-by: Matt DeVillier matt.devillier@gmail.com Reviewed-by: Maxim Polyakov max.senia.poliak@gmail.com Reviewed-by: Angel Pons th3fanbus@gmail.com Reviewed-by: Christian Walter christian.walter@9elements.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/device/device_const.c M src/include/device/device.h M src/soc/intel/apollolake/include/soc/pci_devs.h M src/soc/intel/broadwell/include/soc/pci_devs.h M src/soc/intel/cannonlake/include/soc/pci_devs.h M src/soc/intel/denverton_ns/include/soc/pci_devs.h M src/soc/intel/icelake/include/soc/pci_devs.h M src/soc/intel/skylake/include/soc/pci_devs.h 8 files changed, 29 insertions(+), 15 deletions(-)
Approvals: build bot (Jenkins): Verified Matt DeVillier: Looks good to me, but someone else must approve Angel Pons: Looks good to me, approved Maxim Polyakov: Looks good to me, but someone else must approve Christian Walter: Looks good to me, but someone else must approve
diff --git a/src/device/device_const.c b/src/device/device_const.c index c1b0b06..f2f0177 100644 --- a/src/device/device_const.c +++ b/src/device/device_const.c @@ -234,6 +234,18 @@ return pcidev_path_on_root(PCI_DEVFN(dev, fn)); }
+DEVTREE_CONST struct device *pcidev_path_on_root_debug(pci_devfn_t devfn, const char *func) +{ + DEVTREE_CONST struct device *dev = pcidev_path_on_root(devfn); + if (dev) + return dev; + + printk(BIOS_ERR, "BUG: %s requests hidden 00:%02x.%u\n", func, devfn >> 3, devfn & 7); + + /* FIXME: This can return wrong device. */ + return dev_find_slot(0, devfn); +} + /** * Given an SMBus bus and a device number, find the device structure. * diff --git a/src/include/device/device.h b/src/include/device/device.h index 676da65..d6f80ce 100644 --- a/src/include/device/device.h +++ b/src/include/device/device.h @@ -275,8 +275,6 @@ void tolm_test(void *gp, struct device *dev, struct resource *new); u32 find_pci_tolm(struct bus *bus);
-DEVTREE_CONST struct device *dev_find_slot(unsigned int bus, - unsigned int devfn); DEVTREE_CONST struct device *dev_find_next_pci_device( DEVTREE_CONST struct device *previous_dev); DEVTREE_CONST struct device *dev_find_slot_on_smbus(unsigned int bus, @@ -292,6 +290,10 @@ DEVTREE_CONST struct device *pcidev_on_root(uint8_t dev, uint8_t fn); DEVTREE_CONST struct bus *pci_root_bus(void);
+/* To be deprecated, avoid using. */ +DEVTREE_CONST struct device *dev_find_slot(unsigned int bus, unsigned int devfn); +DEVTREE_CONST struct device *pcidev_path_on_root_debug(pci_devfn_t devfn, const char *func); + void scan_smbus(struct device *bus); void scan_generic_bus(struct device *bus); void scan_static_bus(struct device *bus); diff --git a/src/soc/intel/apollolake/include/soc/pci_devs.h b/src/soc/intel/apollolake/include/soc/pci_devs.h index a334f63..c5eaf4c 100644 --- a/src/soc/intel/apollolake/include/soc/pci_devs.h +++ b/src/soc/intel/apollolake/include/soc/pci_devs.h @@ -22,8 +22,8 @@
#if !defined(__SIMPLE_DEVICE__) #include <device/device.h> -#define _SA_DEV(slot) pcidev_path_on_root(_SA_DEVFN(slot)) -#define _PCH_DEV(slot, func) pcidev_path_on_root(_PCH_DEVFN(slot, func)) +#define _SA_DEV(slot) pcidev_path_on_root_debug(_SA_DEVFN(slot), __func__) +#define _PCH_DEV(slot, func) pcidev_path_on_root_debug(_PCH_DEVFN(slot, func), __func__) #else #define _SA_DEV(slot) PCI_DEV(0, SA_DEV_SLOT_ ## slot, 0) #define _PCH_DEV(slot, func) PCI_DEV(0, PCH_DEV_SLOT_ ## slot, func) diff --git a/src/soc/intel/broadwell/include/soc/pci_devs.h b/src/soc/intel/broadwell/include/soc/pci_devs.h index 522e3eb..ae3e08f 100644 --- a/src/soc/intel/broadwell/include/soc/pci_devs.h +++ b/src/soc/intel/broadwell/include/soc/pci_devs.h @@ -25,8 +25,8 @@ #else #include <device/device.h> #include <device/pci_def.h> -#define _SA_DEV(slot) pcidev_path_on_root(_SA_DEVFN(slot)) -#define _PCH_DEV(slot, func) pcidev_path_on_root(_PCH_DEVFN(slot, func)) +#define _SA_DEV(slot) pcidev_path_on_root_debug(_SA_DEVFN(slot), __func__) +#define _PCH_DEV(slot, func) pcidev_path_on_root_debug(_PCH_DEVFN(slot, func), __func__) #endif
/* System Agent Devices */ diff --git a/src/soc/intel/cannonlake/include/soc/pci_devs.h b/src/soc/intel/cannonlake/include/soc/pci_devs.h index 1bc028f..33814b0 100644 --- a/src/soc/intel/cannonlake/include/soc/pci_devs.h +++ b/src/soc/intel/cannonlake/include/soc/pci_devs.h @@ -24,8 +24,8 @@
#if !defined(__SIMPLE_DEVICE__) #include <device/device.h> -#define _SA_DEV(slot) pcidev_path_on_root(_SA_DEVFN(slot)) -#define _PCH_DEV(slot, func) pcidev_path_on_root(_PCH_DEVFN(slot, func)) +#define _SA_DEV(slot) pcidev_path_on_root_debug(_SA_DEVFN(slot), __func__) +#define _PCH_DEV(slot, func) pcidev_path_on_root_debug(_PCH_DEVFN(slot, func), __func__) #else #define _SA_DEV(slot) PCI_DEV(0, SA_DEV_SLOT_ ## slot, 0) #define _PCH_DEV(slot, func) PCI_DEV(0, PCH_DEV_SLOT_ ## slot, func) diff --git a/src/soc/intel/denverton_ns/include/soc/pci_devs.h b/src/soc/intel/denverton_ns/include/soc/pci_devs.h index 303ba67..27f9e35 100644 --- a/src/soc/intel/denverton_ns/include/soc/pci_devs.h +++ b/src/soc/intel/denverton_ns/include/soc/pci_devs.h @@ -27,8 +27,8 @@ #if ENV_RAMSTAGE #include <device/device.h> #include <device/pci_def.h> -#define _SA_DEV(slot) pcidev_path_on_root(_SA_DEVFN(slot)) -#define _PCH_DEV(slot, func) pcidev_path_on_root(_PCH_DEVFN(slot, func)) +#define _SA_DEV(slot) pcidev_path_on_root_debug(_SA_DEVFN(slot), __func__) +#define _PCH_DEV(slot, func) pcidev_path_on_root_debug(_PCH_DEVFN(slot, func), __func__) #else #define _SA_DEV(slot) PCI_DEV(0, SA_DEV_SLOT_##slot, 0) #define _PCH_DEV(slot, func) PCI_DEV(0, PCH_DEV_SLOT_##slot, func) diff --git a/src/soc/intel/icelake/include/soc/pci_devs.h b/src/soc/intel/icelake/include/soc/pci_devs.h index 1348f23b..0eddee2 100644 --- a/src/soc/intel/icelake/include/soc/pci_devs.h +++ b/src/soc/intel/icelake/include/soc/pci_devs.h @@ -23,8 +23,8 @@
#if !defined(__SIMPLE_DEVICE__) #include <device/device.h> -#define _SA_DEV(slot) pcidev_path_on_root(_SA_DEVFN(slot)) -#define _PCH_DEV(slot, func) pcidev_path_on_root(_PCH_DEVFN(slot, func)) +#define _SA_DEV(slot) pcidev_path_on_root_debug(_SA_DEVFN(slot), __func__) +#define _PCH_DEV(slot, func) pcidev_path_on_root_debug(_PCH_DEVFN(slot, func), __func__) #else #define _SA_DEV(slot) PCI_DEV(0, SA_DEV_SLOT_ ## slot, 0) #define _PCH_DEV(slot, func) PCI_DEV(0, PCH_DEV_SLOT_ ## slot, func) diff --git a/src/soc/intel/skylake/include/soc/pci_devs.h b/src/soc/intel/skylake/include/soc/pci_devs.h index 5acaaeb..0669ced 100644 --- a/src/soc/intel/skylake/include/soc/pci_devs.h +++ b/src/soc/intel/skylake/include/soc/pci_devs.h @@ -24,8 +24,8 @@
#if !defined(__SIMPLE_DEVICE__) #include <device/device.h> -#define _SA_DEV(slot) pcidev_path_on_root(_SA_DEVFN(slot)) -#define _PCH_DEV(slot, func) pcidev_path_on_root(_PCH_DEVFN(slot, func)) +#define _SA_DEV(slot) pcidev_path_on_root_debug(_SA_DEVFN(slot), __func__) +#define _PCH_DEV(slot, func) pcidev_path_on_root_debug(_PCH_DEVFN(slot, func), __func__) #else #define _SA_DEV(slot) PCI_DEV(0, SA_DEV_SLOT_ ## slot, 0) #define _PCH_DEV(slot, func) PCI_DEV(0, PCH_DEV_SLOT_ ## slot, func) @@ -39,7 +39,7 @@
#define SA_DEV_SLOT_PEG 0x01 #define SA_DEVFN_PEG(func) PCI_DEVFN(SA_DEV_SLOT_PEG, func) -#define SA_DEV_PEG(func) pcidev_path_on_root(SA_DEVFN_PEG(func)) +#define SA_DEV_PEG(func) pcidev_path_on_root_debug(SA_DEVFN_PEG(func), __func__) #define SA_DEV_PEG0 SA_DEV_PEG(0) #define SA_DEV_PEG1 SA_DEV_PEG(1) #define SA_DEV_PEG2 SA_DEV_PEG(2)