|
[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 |