Re: [PATCH RFC] vPCI: account for hidden devices in modify_bars()

[Oleksandr Andrushchenko <andr2000@xxxxxxxxx>]

Hello, Jan!

On 30.08.21 16:04, 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.
>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> ---
> RFC: Patch intentionally mis-formatted, as the necessary re-indentation
>       would make the diff difficult to read. At this point I'd merely
>       like to gather input towards possible better approaches to solve
>       the issue (not the least because quite possibly there are further
>       places needing changing).
>
> --- a/xen/drivers/vpci/header.c
> +++ b/xen/drivers/vpci/header.c
> @@ -206,6 +206,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;
> @@ -265,7 +266,8 @@ 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.
>        */
> -    for_each_pdev ( pdev->domain, tmp )
> +for ( d = pdev->domain; ; d = dom_xen ) {//todo
I am not quite sure this will be correct for the cases where pdev->domain != 
dom0,

e.g. in the series for PCI passthrough for Arm this can be any guest. For such 
cases

we'll force running the loop for dom_xen which I am not sure is desirable.

Another question is why such a hidden device has its pdev->domain not set 
correctly,

so we need to work this around?

Thank you,

Oleksandr

> +    for_each_pdev ( d, tmp )
>       {
>           if ( tmp == pdev )
>           {
> @@ -282,6 +284,7 @@ static int modify_bars(const struct pci_
>                    */
>                   continue;
>           }
> +if ( !tmp->vpci ) { ASSERT(d == dom_xen && system_state < SYS_STATE_active); 
> continue; }//todo
>   
>           for ( i = 0; i < ARRAY_SIZE(tmp->vpci->header.bars); i++ )
>           {
> @@ -308,6 +311,7 @@ static int modify_bars(const struct pci_
>               }
>           }
>       }
> +if ( !is_hardware_domain(d) ) break; }//todo
>   
>       ASSERT(dev);
>