On Thu, Jul 20, 2023 at 03:27:29PM +0200, Jan Beulich wrote: > On 20.07.2023 13:20, Roger Pau Monné wrote: > > On Thu, Jul 20, 2023 at 12:32:31AM +0000, Volodymyr Babchuk wrote: > >> @@ -447,8 +476,16 @@ void vpci_write(pci_sbdf_t sbdf, unsigned int reg, > >> unsigned int size, > >> > >> /* > >> * Find the PCI dev matching the address, which for hwdom also > >> requires > >> - * consulting DomXEN. Passthrough everything that's not trapped. > >> + * consulting DomXEN. Passthrough everything that's not trapped. > >> + * If this is hwdom, we need to hold locks for both domain in case if > >> + * modify_bars is called() > > > > Typo: the () wants to be at the end of modify_bars(). > > > >> */ > >> + read_lock(&d->pci_lock); > >> + > >> + /* dom_xen->pci_lock always should be taken second to prevent > >> deadlock */ > >> + if ( is_hardware_domain(d) ) > >> + read_lock(&dom_xen->pci_lock); > > > > For modify_bars() we also want the locks to be in write mode (at least > > the hw one), so that the position of the BARs can't be changed while > > modify_bars() is iterating over them. > > Isn't changing of the BARs happening under the vpci lock?
It is. > Or else I guess > I haven't understood the description correctly: My reading so far was > that it is only the presence (allocation status / pointer validity) that > is protected by this new lock. Hm, I see, yes. I guess it was a previous patch version that also took care of the modify_bars() issue by taking the lock in exclusive mode here. We can always do that later, so forget about that comment (for now). Thanks, Roger.
