Legacy PCI devices don't have any extended config space. Reading any part thereof may return all ones or other arbitrary data, e.g. in some cases base config space contents repeatedly.
Logic follows Linux 6.19-rc's pci_cfg_space_size(), albeit leveraging our determination of device type; in particular some comments are taken verbatim from there. Signed-off-by: Jan Beulich <[email protected]> --- Note that alloc_pdev()'s switch doesn't handle DEV_TYPE_PCI2PCIe_BRIDGE at all. Such bridges will therefore not have ->ext_cfg set (which is likely wrong). Shouldn't we handle them like DEV_TYPE_LEGACY_PCI_BRIDGE (or DEV_TYPE_PCI?) anyway (just like VT-d's set_msi_source_id() does)? Linux also has CONFIG_PCI_QUIRKS, allowing to compile out the slightly risky code (as reads may in principle have side effects). Should we gain such, too? --- a/xen/drivers/passthrough/pci.c +++ b/xen/drivers/passthrough/pci.c @@ -310,6 +310,41 @@ static void apply_quirks(struct pci_dev * from trying to size the BARs or add handlers to trap accesses. */ pdev->ignore_bars = true; + + if ( pdev->ext_cfg ) + { + unsigned int pos; + + /* + * PCI Express to PCI/PCI-X Bridge Specification, rev 1.0, 4.1.4 says + * that when forwarding a type1 configuration request the bridge must + * check that the extended register address field is zero. The bridge + * is not permitted to forward the transactions and must handle it as + * an Unsupported Request. Some bridges do not follow this rule and + * simply drop the extended register bits, resulting in the standard + * config space being aliased, every 256 bytes across the entire + * configuration space. Test for this condition by comparing the first + * dword of each potential alias to the vendor/device ID. + * Known offenders: + * ASM1083/1085 PCIe-to-PCI Reversible Bridge (1b21:1080, rev 01 & 03) + * AMD/ATI SBx00 PCI to PCI Bridge (1002:4384, rev 40) + */ + for ( pos = PCI_CFG_SPACE_SIZE; + pos < PCI_CFG_SPACE_EXP_SIZE; pos += PCI_CFG_SPACE_SIZE ) + { + if ( pci_conf_read16(pdev->sbdf, pos) != vendor || + pci_conf_read16(pdev->sbdf, pos + 2) != device ) + break; + } + + if ( pos >= PCI_CFG_SPACE_EXP_SIZE ) + { + printk(XENLOG_WARNING + "%pp: extended config space aliases base one\n", + &pdev->sbdf); + pdev->ext_cfg = false; + } + } } static struct pci_dev *alloc_pdev(struct pci_seg *pseg, u8 bus, u8 devfn) @@ -351,6 +386,8 @@ static struct pci_dev *alloc_pdev(struct unsigned long flags; case DEV_TYPE_PCIe2PCI_BRIDGE: + pdev->ext_cfg = true; + fallthrough; case DEV_TYPE_LEGACY_PCI_BRIDGE: sec_bus = pci_conf_read8(pdev->sbdf, PCI_SECONDARY_BUS); sub_bus = pci_conf_read8(pdev->sbdf, PCI_SUBORDINATE_BUS); @@ -363,9 +400,19 @@ static struct pci_dev *alloc_pdev(struct pseg->bus2bridge[sec_bus].devfn = devfn; } spin_unlock_irqrestore(&pseg->bus2bridge_lock, flags); + + fallthrough; + case DEV_TYPE_PCI: + pos = pci_find_cap_offset(pdev->sbdf, PCI_CAP_ID_PCIX); + if ( pos && + (pci_conf_read32(pdev->sbdf, pos + PCI_X_STATUS) & + (PCI_X_STATUS_266MHZ | PCI_X_STATUS_533MHZ)) ) + pdev->ext_cfg = true; break; case DEV_TYPE_PCIe_ENDPOINT: + pdev->ext_cfg = true; + pos = pci_find_cap_offset(pdev->sbdf, PCI_CAP_ID_EXP); BUG_ON(!pos); cap = pci_conf_read16(pdev->sbdf, pos + PCI_EXP_DEVCAP); @@ -409,9 +456,9 @@ static struct pci_dev *alloc_pdev(struct } break; - case DEV_TYPE_PCI: case DEV_TYPE_PCIe_BRIDGE: case DEV_TYPE_PCI_HOST_BRIDGE: + pdev->ext_cfg = true; break; default: @@ -420,6 +467,19 @@ static struct pci_dev *alloc_pdev(struct break; } + if ( pdev->ext_cfg && + /* + * Regular PCI devices have 256 bytes, but PCI-X 2 and PCI Express + * devices have 4096 bytes. Even if the device is capable, that + * doesn't mean we can access it. Maybe we don't have a way to + * generate extended config space accesses, or the device is behind a + * reverse Express bridge. So we try reading the dword at + * PCI_CFG_SPACE_SIZE which must either be 0 or a valid extended + * capability header. + */ + pci_conf_read32(pdev->sbdf, PCI_CFG_SPACE_SIZE) == 0xffffffffU ) + pdev->ext_cfg = false; + apply_quirks(pdev); check_pdev(pdev); @@ -717,6 +777,11 @@ int pci_add_device(u16 seg, u8 bus, u8 d list_add(&pdev->vf_list, &pf_pdev->vf_list); } + + if ( !pdev->ext_cfg ) + printk(XENLOG_WARNING + "%pp: VF without extended config space?\n", + &pdev->sbdf); } } --- a/xen/include/xen/pci.h +++ b/xen/include/xen/pci.h @@ -126,6 +126,9 @@ struct pci_dev { nodeid_t node; /* NUMA node */ + /* Whether the device has extended config space. */ + bool ext_cfg; + /* Device to be quarantined, don't automatically re-assign to dom0 */ bool quarantine;
