[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v3] vPCI: account for hidden devices
On Tue, 30 May 2023, Jan Beulich wrote: > 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 <jbeulich@xxxxxxxx> Tested-by: Stefano Stabellini <sstabellini@xxxxxxxxxx> > --- > v3: Also consider pdev being DomXEN's in modify_bars(). Also consult > DomXEN in vpci_{read,write}(). Move vpci_write()'s check of the r/o > map out of mainline code. Re-base over the standalone addition of > the loop continuation in modify_bars(), and finally make the code > change there well-formed. > v2: Extend existing comment. Relax assertion. Don't initialize vPCI for > r/o devices. > > --- a/xen/drivers/vpci/header.c > +++ b/xen/drivers/vpci/header.c > @@ -218,6 +218,7 @@ static int modify_bars(const struct pci_ > 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_ > > /* > * 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; > + if ( tmp == pdev ) > + { > + /* > + * 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; > + } > > - rc = rangeset_remove_range(mem, start, end); > - if ( rc ) > + for ( i = 0; i < ARRAY_SIZE(tmp->vpci->header.bars); i++ ) > { > - printk(XENLOG_G_WARNING "Failed to remove [%lx, %lx]: %d\n", > - start, end, rc); > - rangeset_destroy(mem); > - return rc; > + 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); > --- a/xen/drivers/vpci/vpci.c > +++ b/xen/drivers/vpci/vpci.c > @@ -70,6 +70,7 @@ void vpci_remove_device(struct pci_dev * > 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 *pd > /* 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, unsi > 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, unsigne > 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, unsigne > 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; > } > >
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |