Re: [PATCH] vpci: introduce per-domain lock to protect vpci structure

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

On Fri, Feb 11, 2022 at 12:13:38PM +0000, Oleksandr Andrushchenko wrote:
> 
> 
> On 11.02.22 13:40, Roger Pau Monné wrote:
> > On Fri, Feb 11, 2022 at 07:27:39AM +0000, Oleksandr Andrushchenko wrote:
> >> Hi, Roger!
> >>
> >> On 10.02.22 18:16, Roger Pau Monné wrote:
> >>> On Wed, Feb 09, 2022 at 03:36:27PM +0200, Oleksandr Andrushchenko wrote:
> >>>> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>
> >>>>
> >>>> Introduce a per-domain read/write lock to check whether vpci is present,
> >>>> so we are sure there are no accesses to the contents of the vpci struct
> >>>> if not. This lock can be used (and in a few cases is used right away)
> >>>> so that vpci removal can be performed while holding the lock in write
> >>>> mode. Previously such removal could race with vpci_read for example.
> >>> Sadly there's still a race in the usage of pci_get_pdev_by_domain wrt
> >>> pci_remove_device, and likely when vPCI gets also used in
> >>> {de}assign_device I think.
> >> Yes, this is indeed an issue, but I was not trying to solve it in
> >> context of vPCI locking yet. I think we should discuss how do
> >> we approach pdev locking, so I can create a patch for that.
> >> that being said, I would like not to solve pdev in  this patch yet
> >>
> >> ...I do understand we do want to avoid that, but at the moment
> >> a single reliable way for making sure pdev is alive seems to
> >> be pcidevs_lock....
> > I think we will need to make pcidevs_lock a rwlock and take it in read
> > mode for pci_get_pdev_by_domain.
> >
> > We didn't have this scenario before where PCI emulation is done in the
> > hypervisor, and hence the locking around those data structures has not
> > been designed for those use-cases.
> Yes, I do understand that.
> I hope pcidevs lock move to rwlock can be done as a separate
> patch. While this is not done, do you think we can proceed with
> vPCI series and pcidevs locking re-work being done after?

Ideally we would like to sort out the locking once and for all. I
would like to be sure that what we introduce now doesn't turn out to
interact badly when we decide to look at the pcidevs locking issue.

Thanks, Roger.