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

Re: [PATCH for-4.14 2/2] x86/passthrough: introduce a flag for GSIs not requiring an EOI or unmask


  • To: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
  • From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Date: Wed, 10 Jun 2020 14:46:45 +0200
  • Authentication-results: esa1.hc3370-68.iphmx.com; dkim=none (message not signed) header.i=none
  • Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx, Wei Liu <wl@xxxxxxx>, Jan Beulich <jbeulich@xxxxxxxx>, paul@xxxxxxx
  • Delivery-date: Wed, 10 Jun 2020 12:47:03 +0000
  • Ironport-sdr: IOxINSA+UxuA5GreV127XwoCoD48f/pHDw4nhs5FlCzjJN+fnZRg3DoGI9fIinbZ0GU26qw81a C7mvgSlkpKBHHA5o0UQ4SsJlTFNDAj1PLegxtAZURnbW7jPVRWhf/Xs9y0sPQzEgybBfPP3FGP 98vHRjvsXTcqm+Ii6stmcB6WMRK/sSatu9+5An6pxvLipjD845Sf18LS4e2TUJq21jaUUvURIY AbMAmMEmDU9w2geST+3rETiXtX9JBmUPrYWbxrRBBbtSPgKWET8+4n2A1B6F0h7UIxEN9/XZro 2bs=
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On Wed, Jun 10, 2020 at 01:37:15PM +0100, Andrew Cooper wrote:
> On 10/06/2020 12:51, Roger Pau Monne wrote:
> > @@ -920,6 +927,11 @@ static void hvm_dirq_assist(struct domain *d, struct 
> > hvm_pirq_dpci *pirq_dpci)
> >          if ( pirq_dpci->flags & HVM_IRQ_DPCI_IDENTITY_GSI )
> >          {
> >              hvm_gsi_assert(d, pirq->pirq);
> > +            if ( pirq_dpci->flags & HVM_IRQ_DPCI_NO_EOI )
> > +            {
> > +                spin_unlock(&d->event_lock);
> > +                return;
> > +            }
> 
> Urgh.  Could I possibly talk you into fixing hvm_dirq_assist() to have a
> "goto out;" and a single unlock path ?  (How far are you expecting this
> to be backported?)

I was very tempted to go that way but didn't want to introduce more
churn. Since you agree I will do so.

> I'm also totally unconvinced that the atomic test_and_clear() needs to
> be done with the event lock held (it should either be non-atomic, or the
> locking should be inside the if() condition), but that is probably not a
> can of worms wanting opening right now...

There's some reasoning about all this in 104072fc1c7e6ed. I also think
naming it masked is confusing, since the underlying interrupt might
not be masked. Anyway, this seems like something I don't really want
to get into now, as it seems quite fragile.

Thanks, Roger.



 


Rackspace

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