On 14.08.2025 18:03, Roger Pau Monne wrote:
> Avoid multiple calls to pci_check_bar() for the same memory decoding
> related operation, as each call can possibly print a warning message avoid
> a BAR being in an invalid position.
s/avoid/about/ ?
> @@ -217,13 +179,13 @@ bool vpci_process_pending(struct vcpu *v)
> * rangeset_consume_ranges() itself doesn't generate any errors.
> */
> rangeset_purge(bar->mem);
> +
> + next:
> + if ( bar->valid )
> + bar->enabled = v->vpci.cmd & PCI_COMMAND_MEMORY;
Isn't it at least latently risky to possibly leave ->enabled set to true
when ->valid is false?
> @@ -243,10 +205,8 @@ static int __init apply_map(struct domain *d, const
> struct pci_dev *pdev,
> struct vpci_bar *bar = &header->bars[i];
> struct map_data data = { .d = d, .map = true, .bar = bar };
>
> - if ( rangeset_is_empty(bar->mem) )
> - continue;
> -
> - while ( (rc = rangeset_consume_ranges(bar->mem, map_range,
> + while ( bar->mem &&
> + (rc = rangeset_consume_ranges(bar->mem, map_range,
> &data)) == -ERESTART )
> {
> /*
> @@ -258,9 +218,10 @@ static int __init apply_map(struct domain *d, const
> struct pci_dev *pdev,
> process_pending_softirqs();
> write_lock(&d->pci_lock);
> }
> +
> + if ( bar->valid )
> + bar->enabled = true;
> }
> - if ( !rc )
> - modify_decoding(pdev, cmd, false);
>
> return rc;
> }
> @@ -326,6 +287,9 @@ static int modify_bars(const struct pci_dev *pdev,
> uint16_t cmd, bool rom_only)
> */
> rangeset_purge(bar->mem);
>
> + /* Reset whether the BAR is valid, will be checked below. */
> + bar->valid = false;
Just that I can't spot any check further down. It's only ...
> @@ -341,6 +305,8 @@ static int modify_bars(const struct pci_dev *pdev,
> uint16_t cmd, bool rom_only)
> continue;
> }
>
> + bar->valid = true;
... this setting to true.
> @@ -539,6 +505,7 @@ static void cf_check cmd_write(
> if ( (cmd & PCI_COMMAND_MEMORY) && vpci_make_msix_hole(pdev) )
> return;
> #endif
> +
> /*
> * FIXME: for domUs we don't want the guest toggling the memory decoding
> * bit. It should be set in vpci_init_header() and guest attempts to
This change was probably meant to go into an earlier patch?
Jan
