Re: [Xen-devel] [PATCH v2 for-4.5] xen/arm: clear UIE on hypervisor entry

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Thu, 20 Nov 2014, Julien Grall wrote:
> Hi Stefano,
> 
> On 11/20/2014 10:53 AM, Stefano Stabellini wrote:
> > UIE being set can cause maintenance interrupts to occur when Xen writes
> > to one or more LR registers. The effect is a busy loop around the
> > interrupt handler in Xen
> > (http://marc.info/?l=xen-devel&m=141597517132682): everything gets stuck.
> > 
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
> > Reported-and-Tested-by: Andrii Tseglytskyi 
> > <andrii.tseglytskyi@xxxxxxxxxxxxxxx>
> > CC: konrad.wilk@xxxxxxxxxx
> > ---
> > 
> > Konrad, this fixes an actual bug, at least on OMAP5. It should have no
> > bad side effects on any other platforms as far as I can tell. It should
> > go in 4.5.
> 
> >From Andrii's mail, there is a bad side effect. We can receive spurious
> interrupt.

The spurious interrupt is the maintenance interrupt.
There is not one more spurious interrupt notification in addition to the
maintenance interrupt.


> On V1, you said that you tried this patch on midway. I would prefer if
> you give a try on  platform that would really be used (such as xgene or
> seattle). As we know Midway won't be used in prod.

OK


> Regards,
> 
> > Changes in v2:
> > 
> > - add an in-code comment about maintenance_interrupt not being called.
> > 
> > diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
> > index 70d10d6..c6c11d3 100644
> > --- a/xen/arch/arm/gic.c
> > +++ b/xen/arch/arm/gic.c
> > @@ -403,6 +403,8 @@ void gic_clear_lrs(struct vcpu *v)
> >      if ( is_idle_vcpu(v) )
> >          return;
> >  
> > +    gic_hw_ops->update_hcr_status(GICH_HCR_UIE, 0);
> > +
> >      spin_lock_irqsave(&v->arch.vgic.lock, flags);
> >  
> >      while ((i = find_next_bit((const unsigned long *) &this_cpu(lr_mask),
> > @@ -527,8 +529,6 @@ void gic_inject(void)
> >  
> >      if ( !list_empty(&current->arch.vgic.lr_pending) && lr_all_full() )
> >          gic_hw_ops->update_hcr_status(GICH_HCR_UIE, 1);
> > -    else
> > -        gic_hw_ops->update_hcr_status(GICH_HCR_UIE, 0);
> >  }
> >  
> >  static void do_sgi(struct cpu_user_regs *regs, enum gic_sgi sgi)
> > @@ -598,6 +598,10 @@ static void maintenance_interrupt(int irq, void 
> > *dev_id, struct cpu_user_regs *r
> >       * Receiving the interrupt is going to cause gic_inject to be called
> >       * on return to guest that is going to clear the old LRs and inject
> >       * new interrupts.
> > +     *
> > +     * Do not add code here: maintenance interrupts caused by setting
> > +     * GICH_HCR_UIE, might read as spurious interrupts (1023). As a
> > +     * consequence sometimes this handler might not be called.
> >       */
> >  }
> >  
> > 
> 
> 
> -- 
> Julien Grall
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel