[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v8 02/13] vpci: use per-domain PCI lock to protect vpci structure


  • To: Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>
  • From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Date: Mon, 24 Jul 2023 09:59:33 +0200
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=fmbhP3f0LfUbMTQ/Yf3rDA9ENI6qRthzEso0M42rmeE=; b=W5dACPnqxkty0jhXKPzpZsM5XklOjhr5ML7xG45k79n6xbzLDq9YYA1UjOT8xJARu4vvvIgUgTpiBRgm5puZot0GWqU93YYNUZqjQZ+yreN5eJuZBvlT/9bLHHDFxdnNhvRhNPt9GV2DxhsRlInYWzNqVWzVhAdQP1lzrJsRo/1DGbQM53NsO0V314u6IqKqbgK/3UaxaFXNRLYpqSU88FHmQgYmxDrT2i3911MsSThEtrhXS4y0I8RaektxL6SgkzxZDLRCLNyA5gDMJpwdoeVrf9hnAyKv1KOkV45zkUbfrKVYnKBCLsi0DATtPAw+lVVpTFmzlad9Zu0+et9+Nw==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=nl/jr+W21KIcHlgYxVU1jQNQX1aamXEUfU3GeD2w3d5sEGnz9gnUHuwEYpf+Zli9h3tC4S+fUGY+fyS7Gz+6zjF0K21t1D8CbDc8SMUU6Ec5Gd4EAtZnIUsYOEZAKAPew5lfqKWlYwdguvIDjQZv/Equ2UdH9qaywUvpTu+Wp8vqIH8c0XhuuqyLtIHfPHS+wAmI/r05BPn7F5tnla0VNSukVHZCMgEXMoworFvwup9mogKn8N25e0HLpcNhoPWxWRRKh8DjYuezW+La/qF1xsL9bMjG0+lH0GvTLC6/MQyNeryJaBuv/EAoz3zOIGsxqWdoU0mWMmDoBR5do90hsQ==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=citrix.com;
  • Cc: Jan Beulich <jbeulich@xxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Oleksandr Andrushchenko <Oleksandr_Andrushchenko@xxxxxxxx>
  • Delivery-date: Mon, 24 Jul 2023 08:00:04 +0000
  • Ironport-data: A9a23:owGraKIKRtPX1mAVFE+RApQlxSXFcZb7ZxGr2PjKsXjdYENSg2YGx zMeXWDQPq3ba2v1eoxxao609kkD7J7Xnd9hSAJlqX01Q3x08seUXt7xwmUcnc+xBpaaEB84t ZV2hv3odp1coqr0/0/1WlTZhSAgk/rOHvykU7Ss1hlZHWdMUD0mhQ9oh9k3i4tphcnRKw6Ws Jb5rta31GWNglaYCUpKrfrawP9TlK6q4mhA4QZlPasjUGL2zBH5MrpOfcldEFOgKmVkNrbSb /rOyri/4lTY838FYj9yuu+mGqGiaue60Tmm0hK6aYD76vRxjnVaPpIAHOgdcS9qZwChxLid/ jnvWauYEm/FNoWU8AgUvoIx/ytWZcWq85efSZSzXFD6I+QrvBIAzt03ZHzaM7H09c56DEtV5 +4kMAoHRQ3d2MCqw7CEeuVV05FLwMnDZOvzu1lG5BSAVLMNZsmGRK/Ho9hFwD03m8ZCW+7EY NYUYiZuaxKGZABTPlAQC9Q1m+LAanvXKmUE7g7K4/dppTGMlGSd05C0WDbRUsaNSshP2F6Ru 0rN/njjAwFcP9uaodaA2iv12beRwHqgBer+EpWqq9l3u3CSyFYVUjYfR0qFp7qllRSxDoc3x 0s8v3BGQbIJ3HKsSt7xThipukmutxQXW8dTO+Ai4QTLwa3Riy6bDGUZSj9KaPQ9qdQ7Azct0 zehj97vQDBirrCRYXac7auP6yO/PzAPKm0PbjNCShEKi/HEpIwwlRvJQsxUOai5lMDuGTrwz jaJqwAzn7wWy8UM0s2GEUvvhjutot3CSVcz7wCOB2a9tFomPMiiepCi7kXd4bBYNoGFQ1Kdv X8C3c+D8OQJCpLLnyuIKAkQIIyUCz++GGW0qTZS81MJrWrFF6KLFWyI3AxDGQ==
  • Ironport-hdrordr: A9a23:BLOz/K9SA4TSj52X/Yluk+DWI+orL9Y04lQ7vn2ZKCY4TiX8ra uTdZsguiMc5Ax+ZJhDo7C90di7IE80nKQdieN9AV7IZniEhILHFvAG0aLShxHmBi3i5qp8+M 5bAsxD4QTLfDpHsfo=
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On Mon, Jul 24, 2023 at 12:07:48AM +0000, Volodymyr Babchuk wrote:
> 
> Hi Roger,
> 
> Roger Pau Monné <roger.pau@xxxxxxxxxx> writes:
> 
> > 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).
> 
> Are you sure? I'd rather rework the code to use write lock in the
> modify_bars(). This is why we began all this journey in the first place.

Well, I was just saying that it doesn't need to be done in this same
patch, it can be done as a followup if that's preferred, but one way
or another we need to deal with it.

I'm fine if you want to adjust the commit message and do the change in
this same patch.

Thanks, Roger.



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.