Re: [PATCH v3 04/11] vpci/header: Emulate extended capability list for dom0

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Mon, Apr 21, 2025 at 02:18:56PM +0800, Jiqian Chen wrote:
> Add a new function to emulate extended capability list for dom0,
> and call it in init_header(). So that it will be easy to hide a
> extended capability whose initialization fails.
> 
> As for the extended capability list of domU, just move the logic
> into above function and keep hiding it for domU.
> 
> Signed-off-by: Jiqian Chen <Jiqian.Chen@xxxxxxx>
> ---
> cc: "Roger Pau Monné" <roger.pau@xxxxxxxxxx>
> ---
> v2->v3 changes:
> * In vpci_init_ext_capability_list(), when domain is domU, directly return 
> after adding a handler(hiding all extended capability for domU).
> * In vpci_init_ext_capability_list(), change condition to be "while ( pos >= 
> 0x100U && ttl-- )" instead of "while ( pos && ttl-- )".
> * Add new function vpci_hw_write32, and pass it to extended capability 
> handler for dom0.
> 
> v1->v2 changes:
> new patch
> 
> Best regards,
> Jiqian Chen.
> ---
>  xen/drivers/vpci/header.c | 36 ++++++++++++++++++++++++++++--------
>  xen/drivers/vpci/vpci.c   |  6 ++++++
>  xen/include/xen/vpci.h    |  2 ++
>  3 files changed, 36 insertions(+), 8 deletions(-)
> 
> diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c
> index c98cd211d9d7..ee94ad8e5037 100644
> --- a/xen/drivers/vpci/header.c
> +++ b/xen/drivers/vpci/header.c
> @@ -817,6 +817,31 @@ 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;
> +
> +    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;

I'm thinking it might be helpful to avoid setting the handler for the
last capability on the list, or simply for devices that have no
extended capabilities at all:

if ( PCI_EXT_CAP_NEXT(header) >= PCI_CFG_SPACE_SIZE )
{
    int rc = vpci_add_register(pdev->vpci, vpci_read_val, vpci_hw_write32,
                               pos, 4, (void *)(uintptr_t)header);

    if ( rc )
        return rc;
}

Otherwise on systems with a lot of devices it can be quite wasteful to
set a handler to just return the next capability as 0.

Thanks, Roger.