[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v7 11/12] xen/arm: translate virtual PCI bus topology for guests
On 13.06.2023 12:32, Volodymyr Babchuk wrote: > --- a/xen/drivers/vpci/vpci.c > +++ b/xen/drivers/vpci/vpci.c > @@ -180,6 +180,44 @@ static void vpci_remove_virtual_device(const struct > pci_dev *pdev) > write_unlock(&pdev->domain->vpci_rwlock); > } > > +/* > + * Find the physical device which is mapped to the virtual device > + * and translate virtual SBDF to the physical one. > + */ > +bool vpci_translate_virtual_device(struct domain *d, pci_sbdf_t *sbdf) > +{ > + struct pci_dev *pdev; > + > + ASSERT(!is_hardware_domain(d)); > + > + read_lock(&d->vpci_rwlock); > + pcidevs_lock(); > + for_each_pdev( d, pdev ) > + { > + bool found; > + > + if ( !pdev->vpci ) > + continue; > + > + spin_lock(&pdev->vpci->lock); Following the description of patch 1, why does this lock need acquiring here? The r/w lock acquired above should already guard against modification. > + found = pdev->vpci && (pdev->vpci->guest_sbdf.sbdf == sbdf->sbdf); The LHS of the && is pointless here - when acquiring the lock you have already de-referenced pdev->vpci. > + spin_unlock(&pdev->vpci->lock); > + > + if ( found ) > + { > + /* Replace guest SBDF with the physical one. */ > + *sbdf = pdev->sbdf; > + pcidevs_unlock(); > + read_unlock(&d->vpci_rwlock); > + return true; What use is the result to the caller with all locks dropped, and hence state possibly having changed before the caller gets a chance to look at the result? If there are reason why this is okay, you want to (a) spell out those reasons in the description and (b) document this restriction in a comment, to warn people who may want to (re)use this function. But really I think the caller needs to hold a suitable lock until after the result from here was consumed. Jan
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |