Re: [Xen-devel] [PATCH v2 02/11] vpci/msix: add lock to protect the list of MSIX regions

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 17.07.18 at 11:48, <roger.pau@xxxxxxxxxx> wrote:
> --- a/xen/drivers/vpci/msix.c
> +++ b/xen/drivers/vpci/msix.c
> @@ -148,10 +148,11 @@ static void control_write(const struct pci_dev *pdev, 
> unsigned int reg,
>          pci_conf_write16(pdev->seg, pdev->bus, slot, func, reg, val);
>  }
>  
> -static struct vpci_msix *msix_find(const struct domain *d, unsigned long 
> addr)
> +static struct vpci_msix *msix_find(struct domain *d, unsigned long addr)
>  {
>      struct vpci_msix *msix;
>  
> +    read_lock(&d->arch.hvm_domain.msix_lock);
>      list_for_each_entry ( msix, &d->arch.hvm_domain.msix_tables, next )
>      {
>          const struct vpci_bar *bars = msix->pdev->vpci->header.bars;
> @@ -160,8 +161,12 @@ static struct vpci_msix *msix_find(const struct domain 
> *d, unsigned long addr)
>          for ( i = 0; i < ARRAY_SIZE(msix->tables); i++ )
>              if ( bars[msix->tables[i] & PCI_MSIX_BIRMASK].enabled &&
>                   VMSIX_ADDR_IN_RANGE(addr, msix->pdev->vpci, i) )
> +            {
> +                read_unlock(&d->arch.hvm_domain.msix_lock);
>                  return msix;
> +            }
>      }
> +    read_unlock(&d->arch.hvm_domain.msix_lock);
>  
>      return NULL;
>  }

Don't you rather need the caller to acquire the lock, so that the return
value is guaranteed non-stale by the time the caller looks at it?

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel