Re: [Xen-devel] [PATCH v3 5/6] vpt: add support for level interrupts

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

Ping?

On Mon, Jun 25, 2018 at 01:19:19PM +0200, Roger Pau Monné wrote:
> On Fri, Jun 22, 2018 at 08:23:02AM -0600, Jan Beulich wrote:
> > >>> On 08.06.18 at 17:07, <roger.pau@xxxxxxxxxx> wrote:
> > > @@ -316,7 +317,9 @@ int pt_update_irq(struct vcpu *v)
> > >          if ( pt->pending_intr_nr )
> > >          {
> > >              /* RTC code takes care of disabling the timer itself. */
> > > -            if ( (pt->irq != RTC_IRQ || !pt->priv) && pt_irq_masked(pt) )
> > > +            if ( (pt->irq != RTC_IRQ || !pt->priv) && pt_irq_masked(pt) 
> > > &&
> > > +                 /* Level interrupts should be asserted even if masked. 
> > > */
> > > +                 !pt->level )
> > >              {
> > >                  /* suspend timer emulation */
> > 
> > Especially with this comment I'm not convinced this change is fully
> > correct: Once a level triggered interrupt is latched in IRR, no
> > further assertions are meaningful, and hence emulation could (and
> > hence should) still be stopped to reduce resource consumption.
> 
> I've been thinking about this and I think the above comment is not
> fully correct. Assertion of latched level interrupts is meaningful
> because gsi_assert_count should be increased regardless of the state
> of IRR.
> 
> Your comment made me realize that periodic level triggered interrupts
> don't make much sense in vpt. IO-APIC level triggered interrupts will
> require external deassertion of the line, in which case such
> deassertion should also take care of reprogramming the timer if
> required (like it's done in the next patch for vhpet when clearing
> ISR).
> 
> > > @@ -374,13 +378,36 @@ int pt_update_irq(struct vcpu *v)
> > >          break;
> > >  
> > >      case PTSRC_ioapic:
> > > -        /*
> > > -         * NB: At the moment IO-APIC routed interrupts generated by vpt 
> > > devices
> > > -         * (HPET) are edge-triggered.
> > > -         */
> > > -        pt_vector = hvm_ioapic_assert(v->domain, irq, false);
> > > +        pt_vector = hvm_ioapic_assert(v->domain, irq, level);
> > >          if ( pt_vector < 0 || !vlapic_test_irq(vcpu_vlapic(v), 
> > > pt_vector) )
> > > +        {
> > >              pt_vector = -1;
> > > +            if ( level )
> > > +            {
> > > +                /*
> > > +                 * Level interrupts are asserted even if the interrupt is
> > > +                 * masked, so also execute the callback associated with 
> > > the
> > > +                 * timer.
> > > +                 */
> > > +                time_cb *cb = NULL;
> > > +                void *cb_priv;
> > > +
> > > +                spin_lock(&v->arch.hvm_vcpu.tm_lock);
> > > +                /* Make sure the timer is still on the list. */
> > > +                list_for_each_entry ( pt, &v->arch.hvm_vcpu.tm_list, 
> > > list )
> > > +                    if ( pt == earliest_pt )
> > > +                    {
> > > +                        pt_irq_fired(v, pt);
> > > +                        cb = pt->cb;
> > > +                        cb_priv = pt->priv;
> > > +                        break;
> > > +                    }
> > > +                spin_unlock(&v->arch.hvm_vcpu.tm_lock);
> > > +
> > > +                if ( cb != NULL )
> > > +                    cb(v, cb_priv);
> > > +            }
> > > +        }
> > >          break;
> > 
> > I'm not fully convinced, especially in the case that hvm_ioapic_assert()
> > returned a negative value: Either the callback needs to be called in all
> > cases (even if an edge triggered interrupt was not successfully asserted),
> > or only when an interrupt really gets surfaced to the guest.
> 
> Even if hvm_ioapic_assert returns -1 gsi_assert_count will be
> increased, so I think it makes sense to also call the callback in this
> case, because the line has been asserted.
> 
> Roger.
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxxx
> https://lists.xenproject.org/mailman/listinfo/xen-devel

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel