From 465217b0f872602b4084a1b0fa2ef75377cb3589 Mon Sep 17 00:00:00 2001 From: Jan Beulich Date: Wed, 31 May 2023 12:01:11 +0200 Subject: [PATCH] vPCI: account for hidden devices MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Hidden devices (e.g. an add-in PCI serial card used for Xen's serial console) are associated with DomXEN, not Dom0. This means that while looking for overlapping BARs such devices cannot be found on Dom0's list of devices; DomXEN's list also needs to be scanned. Suppress vPCI init altogether for r/o devices (which constitute a subset of hidden ones). Signed-off-by: Jan Beulich Reviewed-by: Roger Pau Monné Tested-by: Stefano Stabellini --- xen/drivers/vpci/header.c | 96 ++++++++++++++++++++++----------------- xen/drivers/vpci/vpci.c | 30 ++++++++---- 2 files changed, 75 insertions(+), 51 deletions(-) diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c index 766fd98b21..b41556d007 100644 --- a/xen/drivers/vpci/header.c +++ b/xen/drivers/vpci/header.c @@ -218,6 +218,7 @@ static int modify_bars(const struct pci_dev *pdev, uint16_t cmd, bool rom_only) struct vpci_header *header = &pdev->vpci->header; struct rangeset *mem = rangeset_new(NULL, NULL, 0); struct pci_dev *tmp, *dev = NULL; + const struct domain *d; const struct vpci_msix *msix = pdev->vpci->msix; unsigned int i; int rc; @@ -285,58 +286,69 @@ static int modify_bars(const struct pci_dev *pdev, uint16_t cmd, bool rom_only) /* * Check for overlaps with other BARs. Note that only BARs that are - * currently mapped (enabled) are checked for overlaps. + * currently mapped (enabled) are checked for overlaps. Note also that + * for hwdom we also need to include hidden, i.e. DomXEN's, devices. */ - for_each_pdev ( pdev->domain, tmp ) + for ( d = pdev->domain != dom_xen ? pdev->domain : hardware_domain; ; ) { - if ( !tmp->vpci ) - /* - * For the hardware domain it's possible to have devices assigned - * to it that are not handled by vPCI, either because those are - * read-only devices, or because vPCI setup has failed. - */ - continue; - - if ( tmp == pdev ) + for_each_pdev ( d, tmp ) { - /* - * Need to store the device so it's not constified and defer_map - * can modify it in case of error. - */ - dev = tmp; - if ( !rom_only ) + if ( !tmp->vpci ) /* - * If memory decoding is toggled avoid checking against the - * same device, or else all regions will be removed from the - * memory map in the unmap case. + * For the hardware domain it's possible to have devices + * assigned to it that are not handled by vPCI, either because + * those are read-only devices, or because vPCI setup has + * failed. */ continue; - } - for ( i = 0; i < ARRAY_SIZE(tmp->vpci->header.bars); i++ ) - { - const struct vpci_bar *bar = &tmp->vpci->header.bars[i]; - unsigned long start = PFN_DOWN(bar->addr); - unsigned long end = PFN_DOWN(bar->addr + bar->size - 1); - - if ( !bar->enabled || !rangeset_overlaps_range(mem, start, end) || - /* - * If only the ROM enable bit is toggled check against other - * BARs in the same device for overlaps, but not against the - * same ROM BAR. - */ - (rom_only && tmp == pdev && bar->type == VPCI_BAR_ROM) ) - continue; - - rc = rangeset_remove_range(mem, start, end); - if ( rc ) + if ( tmp == pdev ) { - printk(XENLOG_G_WARNING "Failed to remove [%lx, %lx]: %d\n", - start, end, rc); - rangeset_destroy(mem); - return rc; + /* + * Need to store the device so it's not constified and defer_map + * can modify it in case of error. + */ + dev = tmp; + if ( !rom_only ) + /* + * If memory decoding is toggled avoid checking against the + * same device, or else all regions will be removed from the + * memory map in the unmap case. + */ + continue; + } + + for ( i = 0; i < ARRAY_SIZE(tmp->vpci->header.bars); i++ ) + { + const struct vpci_bar *bar = &tmp->vpci->header.bars[i]; + unsigned long start = PFN_DOWN(bar->addr); + unsigned long end = PFN_DOWN(bar->addr + bar->size - 1); + + if ( !bar->enabled || + !rangeset_overlaps_range(mem, start, end) || + /* + * If only the ROM enable bit is toggled check against + * other BARs in the same device for overlaps, but not + * against the same ROM BAR. + */ + (rom_only && tmp == pdev && bar->type == VPCI_BAR_ROM) ) + continue; + + rc = rangeset_remove_range(mem, start, end); + if ( rc ) + { + printk(XENLOG_G_WARNING "Failed to remove [%lx, %lx]: %d\n", + start, end, rc); + rangeset_destroy(mem); + return rc; + } } } + + if ( !is_hardware_domain(d) ) + break; + + d = dom_xen; } ASSERT(dev); diff --git a/xen/drivers/vpci/vpci.c b/xen/drivers/vpci/vpci.c index 652807a4a4..d73fa76302 100644 --- a/xen/drivers/vpci/vpci.c +++ b/xen/drivers/vpci/vpci.c @@ -70,6 +70,7 @@ void vpci_remove_device(struct pci_dev *pdev) int vpci_add_handlers(struct pci_dev *pdev) { unsigned int i; + const unsigned long *ro_map; int rc = 0; if ( !has_vpci(pdev->domain) ) @@ -78,6 +79,11 @@ int vpci_add_handlers(struct pci_dev *pdev) /* We should not get here twice for the same device. */ ASSERT(!pdev->vpci); + /* No vPCI for r/o devices. */ + ro_map = pci_get_ro_map(pdev->sbdf.seg); + if ( ro_map && test_bit(pdev->sbdf.bdf, ro_map) ) + return 0; + pdev->vpci = xzalloc(struct vpci); if ( !pdev->vpci ) return -ENOMEM; @@ -332,8 +338,13 @@ uint32_t vpci_read(pci_sbdf_t sbdf, unsigned int reg, unsigned int size) return data; } - /* Find the PCI dev matching the address. */ + /* + * Find the PCI dev matching the address, which for hwdom also requires + * consulting DomXEN. Passthrough everything that's not trapped. + */ pdev = pci_get_pdev(d, sbdf); + if ( !pdev && is_hardware_domain(d) ) + pdev = pci_get_pdev(dom_xen, sbdf); if ( !pdev || !pdev->vpci ) return vpci_read_hw(sbdf, reg, size); @@ -427,7 +438,6 @@ void vpci_write(pci_sbdf_t sbdf, unsigned int reg, unsigned int size, const struct pci_dev *pdev; const struct vpci_register *r; unsigned int data_offset = 0; - const unsigned long *ro_map = pci_get_ro_map(sbdf.seg); if ( !size ) { @@ -435,18 +445,20 @@ void vpci_write(pci_sbdf_t sbdf, unsigned int reg, unsigned int size, return; } - if ( ro_map && test_bit(sbdf.bdf, ro_map) ) - /* Ignore writes to read-only devices. */ - return; - /* - * Find the PCI dev matching the address. - * Passthrough everything that's not trapped. + * Find the PCI dev matching the address, which for hwdom also requires + * consulting DomXEN. Passthrough everything that's not trapped. */ pdev = pci_get_pdev(d, sbdf); + if ( !pdev && is_hardware_domain(d) ) + pdev = pci_get_pdev(dom_xen, sbdf); if ( !pdev || !pdev->vpci ) { - vpci_write_hw(sbdf, reg, size, data); + /* Ignore writes to read-only devices, which have no ->vpci. */ + const unsigned long *ro_map = pci_get_ro_map(sbdf.seg); + + if ( !ro_map || !test_bit(sbdf.bdf, ro_map) ) + vpci_write_hw(sbdf, reg, size, data); return; }