Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/62951 )
Change subject: pcidev: Construct a API to handle phantom pci controllers ......................................................................
pcidev: Construct a API to handle phantom pci controllers
On Intel the SPI PCI device can habe hiden PCI vendor and device IDs. So we need a PCI access method that works even when the OS doesn't know the PCI device.
This however does not generalise since it would bring along other constraints (e.g. on PCI domains, extended PCIe config space).
Fortunately the logic exists inside chipset_enable.c however the interaction with the rest of the pcidev workers are quite invasive and hard to reason about. Therefore here we implement a definable API to manage this special casing by having methods define on a handle with a known life-time.
BUG=b:220950271 TEST=`sudo ./flashrom -p internal -r /tmp/bios` on Found chipset "Intel Kaby Lake U w/ iHDCP2.2 Prem.".
Change-Id: Ic75573a14c9997a2d0347cd683ffc0c78d1b3a4f Signed-off-by: Edward O'Callaghan quasisec@google.com --- M chipset_enable.c M pcidev.c M programmer.h 3 files changed, 77 insertions(+), 37 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/51/62951/1
diff --git a/chipset_enable.c b/chipset_enable.c index 0484299..5e5763e 100644 --- a/chipset_enable.c +++ b/chipset_enable.c @@ -903,9 +903,9 @@ }
/* Sunrise Point */ -static int enable_flash_pch100_shutdown(void *const pci_acc) +static int enable_flash_pch100_shutdown(void *const ph_spidev) { - pci_cleanup(pci_acc); + pcidev_phantom_release(ph_spidev); return 0; }
@@ -915,31 +915,13 @@ { int ret = ERROR_FATAL;
- /* - * The SPI PCI device is usually hidden (by hiding PCI vendor - * and device IDs). So we need a PCI access method that works - * even when the OS doesn't know the PCI device. We can't use - * this method globally since it would bring along other con- - * straints (e.g. on PCI domains, extended PCIe config space). - */ - struct pci_access *const pci_acc = pci_alloc(); - if (!pci_acc) { - msg_perr("Can't allocate PCI accessor.\n"); + struct pcidev_phantom_spi *const ph_spidev = pcidev_phantom_new_spidev(dev, slot, func); + if (!ph_spidev) { + msg_perr("Can't allocate Phanom SPI device.\n"); return ret; } - pci_acc->method = PCI_ACCESS_I386_TYPE1; - pci_init(pci_acc); - register_shutdown(enable_flash_pch100_shutdown, pci_acc); + struct pci_dev *const spi_dev = pcidev_phantom_spidev(ph_spidev);
- struct pci_dev *const spi_dev = pci_get_dev(pci_acc, dev->domain, dev->bus, slot, func); - if (!spi_dev) { - msg_perr("Can't allocate PCI device.\n"); - return ret; - } - - /* Modify pacc so the rpci_write can register the undo callback with a - * device using the correct pci_access */ - struct pci_access *const saved_pacc = pcidev_override_pacc(pci_acc); const enum chipbustype boot_buses = enable_flash_ich_report_gcs(spi_dev, pch_generation, NULL);
const int ret_bc = enable_flash_ich_bios_cntl_config_space(spi_dev, pch_generation, 0xdc); @@ -953,6 +935,7 @@ msg_pdbg("SPIBAR = 0x%0*" PRIxPTR " (phys = 0x%08x)\n", PRIxPTR_WIDTH, (uintptr_t)spibar, phys_spibar);
/* This adds BUS_SPI */ + register_shutdown(enable_flash_pch100_shutdown, ph_spidev); const int ret_spi = ich_init_spi(spibar, pch_generation); if (ret_spi != ERROR_FATAL) { if (ret_bc || ret_spi) @@ -966,8 +949,8 @@ laptop_ok = 1;
_freepci_ret: - pci_free_dev(spi_dev); - pcidev_override_pacc(saved_pacc); + if (ret == ERROR_FATAL) + pcidev_phantom_release(ph_spidev); return ret; }
diff --git a/pcidev.c b/pcidev.c index 5673b02..29ae698 100644 --- a/pcidev.c +++ b/pcidev.c @@ -30,11 +30,70 @@ TYPE_UNKNOWN };
-struct pci_access *pcidev_override_pacc(struct pci_access *new_pacc) +struct pcidev_phantom_spi { + struct pci_access *pci_acc_backup; + struct pci_access *pci_acc; + struct pci_dev *pci_dev; +}; + +/** + * @brief The SPI PCI device is usually hidden (by hiding PCI vendor + * and device IDs). So we need a PCI access method that works + * even when the OS doesn't know the PCI device. We can't use + * this method globally since it would bring along other con- + * straints (e.g. on PCI domains, extended PCIe config space). + * + * The returned device handle contains a backup of the previous + * pci_access pointer to restore. + * + * @param pci_dev pointer + * @param pci slot number + * @param pci func number + * @return phantom device handle. + */ +struct pcidev_phantom_spi * pcidev_phantom_new_spidev( + struct pci_dev *const dev, const int slot, const int func) { - struct pci_access *const saved_pacc = pacc; - pacc = new_pacc; - return saved_pacc; + struct pci_access *const pci_acc = pci_alloc(); + if (!pci_acc) { + msg_perr("Can't allocate PCI accessor.\n"); + return NULL; + } + pci_acc->method = PCI_ACCESS_I386_TYPE1; + pci_init(pci_acc); + + struct pci_dev *const spi_dev = pci_get_dev(pci_acc, dev->domain, dev->bus, slot, func); + if (!spi_dev) { + msg_perr("Can't allocate PCI device.\n"); + return NULL; + } + + struct pcidev_phantom_spi *ph_spi = calloc(1, sizeof(struct pcidev_phantom_spi)); + if (!ph_spi) + return NULL; + ph_spi->pci_acc_backup = pacc; + ph_spi->pci_acc = pci_acc; + ph_spi->pci_dev = spi_dev; + + /* Modify pacc so the rpci_write can register the undo callback with a + * device using the correct pci_access */ + pacc = pci_acc; + + return ph_spi; +} + +struct pci_dev * pcidev_phantom_spidev(struct pcidev_phantom_spi *const ph_spidev) +{ + return ph_spidev->pci_dev; +} + +void pcidev_phantom_release(struct pcidev_phantom_spi *const ph_spidev) +{ + pacc = ph_spidev->pci_acc_backup; /* restore pacc */ + + pci_free_dev(ph_spidev->pci_dev); + pci_cleanup(ph_spidev->pci_acc); + free(ph_spidev); }
uintptr_t pcidev_readbar(struct pci_dev *dev, int bar) diff --git a/programmer.h b/programmer.h index e744077..8141809 100644 --- a/programmer.h +++ b/programmer.h @@ -128,13 +128,11 @@ struct pci_dev *pcidev_find_vendorclass(uint16_t vendor, uint16_t devclass); struct pci_dev *pcidev_card_find(uint16_t vendor, uint16_t device, uint16_t card_vendor, uint16_t card_device); struct pci_dev *pcidev_find(uint16_t vendor, uint16_t device); -/** - * @brief pcidev_override_pacc() sets a new pacc and returns a backup. - * FIXME: Fix chipset_enable.c life-cycle to remove the need for this override logic. - * @param pci_access pointer to override with. - * @return previous pci_access pointer to restore. - */ -struct pci_access *pcidev_override_pacc(struct pci_access *new_pacc); + +struct pcidev_phantom_spi; +struct pcidev_phantom_spi * pcidev_phantom_new_spidev(struct pci_dev *const dev, const int slot, const int func); +struct pci_dev * pcidev_phantom_spidev(struct pcidev_phantom_spi *const ph_spidev); +void pcidev_phantom_release(struct pcidev_phantom_spi *const ph_spidev);
/* rpci_write_* are reversible writes. The original PCI config space register * contents will be restored on shutdown.