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

Re: [PATCH for-4.14 1/2] x86/passthrough: do not assert edge triggered GSIs for PVH dom0


  • To: <paul@xxxxxxx>
  • From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Date: Wed, 10 Jun 2020 14:40:43 +0200
  • Authentication-results: esa2.hc3370-68.iphmx.com; dkim=none (message not signed) header.i=none
  • Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx, 'Wei Liu' <wl@xxxxxxx>, 'Jan Beulich' <jbeulich@xxxxxxxx>, 'Andrew Cooper' <andrew.cooper3@xxxxxxxxxx>
  • Delivery-date: Wed, 10 Jun 2020 12:40:57 +0000
  • Ironport-sdr: nNU+aj6yplQrqlTAqeK4CfMv+rI/KyzFg+pXfgzkQnX5EWIp934YHUa4GCOE003PI7fC5sz7dx Fe+eHPRYZd7cTGWfITkTezmZf67kvYbCl24OzIKFYI324xS3lIshY/m3FpjQ7axbalgFGbvcCi khmYnTYAAWOx/T910Fnfmcx0D9OPpX1Y6bOCqxmkivZy9KuCFuvItBb2DETWE0yb9DrG1nkQvG eA3Yva/xaoPSOy5Ha+tkCAcp4CR6OJnTRWIFPcxmb0DiPl5AEFLGvORS+OREGzMTBmT2JJ+x8n pQU=
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On Wed, Jun 10, 2020 at 01:33:46PM +0100, Paul Durrant wrote:
> > -----Original Message-----
> > From: Roger Pau Monne <roger.pau@xxxxxxxxxx>
> > Sent: 10 June 2020 12:51
> > To: xen-devel@xxxxxxxxxxxxxxxxxxxx
> > Cc: paul@xxxxxxx; Roger Pau Monne <roger.pau@xxxxxxxxxx>; Jan Beulich 
> > <jbeulich@xxxxxxxx>; Andrew
> > Cooper <andrew.cooper3@xxxxxxxxxx>; Wei Liu <wl@xxxxxxx>
> > Subject: [PATCH for-4.14 1/2] x86/passthrough: do not assert edge triggered 
> > GSIs for PVH dom0
> > 
> > Edge triggered interrupts do not assert the line, so the handling done
> > in Xen should also avoid asserting it. Asserting the line prevents
> > further edge triggered interrupts on the same vIO-APIC pin from being
> > delivered, since the line is not de-asserted.
> > 
> > One case of such kind of interrupt is the RTC timer, which is edge
> > triggered and available to a PVH dom0. Note this should not affect
> > domUs, as it only modifies the behavior of IDENTITY_GSI kind of passed
> > through interrupts.
> > 
> > Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> > ---
> >  xen/arch/x86/hvm/irq.c | 13 ++++++++-----
> >  1 file changed, 8 insertions(+), 5 deletions(-)
> > 
> > diff --git a/xen/arch/x86/hvm/irq.c b/xen/arch/x86/hvm/irq.c
> > index 9c8adbc495..9a56543c1b 100644
> > --- a/xen/arch/x86/hvm/irq.c
> > +++ b/xen/arch/x86/hvm/irq.c
> > @@ -169,9 +169,10 @@ void hvm_pci_intx_deassert(
> > 
> >  void hvm_gsi_assert(struct domain *d, unsigned int gsi)
> >  {
> > +    int level = vioapic_get_trigger_mode(d, gsi);
> >      struct hvm_irq *hvm_irq = hvm_domain_irq(d);
> > 
> > -    if ( gsi >= hvm_irq->nr_gsis )
> > +    if ( gsi >= hvm_irq->nr_gsis || level < 0 )
> >      {
> >          ASSERT_UNREACHABLE();
> >          return;
> > @@ -186,9 +187,10 @@ void hvm_gsi_assert(struct domain *d, unsigned int gsi)
> >       * to know if the GSI is pending or not.
> >       */
> >      spin_lock(&d->arch.hvm.irq_lock);
> > -    if ( !hvm_irq->gsi_assert_count[gsi] )
> > +    if ( !level || !hvm_irq->gsi_assert_count[gsi] )
> 
> We have definitions of VIOAPIC_EDGE_TRIG and VIOAPIC_LEVEL_TRIG. It seems odd 
> not to use them here.

Urg, completely forgot about those. I think there are many places
where the triggering is treated as a boolean. I will rename the local
variable to 'trig' and compare against VIOAPIC_EDGE_TRIG.

Thanks, Roger.



 


Rackspace

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