Re: [PATCH v4 03/10] vpci/header: Emulate extended capability list for dom0

[Jan Beulich <jbeulich@xxxxxxxx>]

On 09.05.2025 11:05, Jiqian Chen wrote:
> @@ -827,6 +827,34 @@ static int vpci_init_capability_list(struct pci_dev 
> *pdev)
>                                                   PCI_STATUS_RSVDZ_MASK);
>  }
>  
> +static int vpci_init_ext_capability_list(struct pci_dev *pdev)
> +{
> +    unsigned int pos = PCI_CFG_SPACE_SIZE, ttl = 480;

The ttl value exists (in the function you took it from) to make sure
the loop below eventually ends. That is, to be able to kind of
gracefully deal with loops in the linked list. Such loops, however,
would ...

> +    if ( !is_hardware_domain(pdev->domain) )
> +        /* Extended capabilities read as zero, write ignore for guest */
> +        return vpci_add_register(pdev->vpci, vpci_read_val, NULL,
> +                                 pos, 4, (void *)0);
> +
> +    while ( pos >= PCI_CFG_SPACE_SIZE && ttl-- )
> +    {
> +        uint32_t header = pci_conf_read32(pdev->sbdf, pos);
> +        int rc;
> +
> +        if ( !header )
> +            return 0;
> +
> +        rc = vpci_add_register(pdev->vpci, vpci_read_val, vpci_hw_write32,
> +                               pos, 4, (void *)(uintptr_t)header);

... mean we may invoke this twice for the same capability. Such
a secondary invocation would fail with -EEXIST, causing device init
to fail altogether. Which is kind of against our aim of exposing
(in a controlled manner) as much of the PCI hardware as possible.

Imo we ought to be using a bitmap to detect the situation earlier
and hence to be able to avoid redundant register addition. Thoughts?

Jan