Re: [Xen-devel] [XenARM] XEN tools for ARM with Virtualization Extensions - Patch #2

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Tue, 16 Jul 2013, Eric Trudeau wrote:
> > -----Original Message-----
> > From: Julien Grall [mailto:julien.grall@xxxxxxxxxx]
> > Sent: Monday, July 15, 2013 7:14 PM
> > To: Eric Trudeau
> > Cc: xen-devel; Stefano.Stabellini@xxxxxxxxxx; Ian Campbell
> > Subject: Re: [Xen-devel] [XenARM] XEN tools for ARM with Virtualization
> > Extensions - Patch #2
> > 
> > On 12 July 2013 20:27, Eric Trudeau <etrudeau@xxxxxxxxxxxx> wrote:
> > > Second patch submitted with changes based on comments on first patch.
> > >
> > >> > From 551f174c9653def01890fadd2dd769ab8a9acde7 Mon Sep 17 00:00:00
> > >> 2001
> > >> > From: Eric Trudeau <etrudeau@xxxxxxxxxxxx>
> > >> > Date: Thu, 11 Jul 2013 20:03:51 -0400
> > >> > Subject: [PATCH] Add support for Guest physdev irqs
> > >> >
> > >> > ---
> > >> >  xen/arch/arm/domain.c  | 16 ++++++++++++++++
> > >> >  xen/arch/arm/gic.c     | 15 ++++++++++-----
> > >> >  xen/arch/arm/physdev.c | 48
> > >> ++++++++++++++++++++++++++++++++++++++++++++++--
> > >> >  xen/arch/arm/vgic.c    |  5 +----
> > >> >  4 files changed, 73 insertions(+), 11 deletions(-)
> > >> >
> > >> > diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> > >> > index 4c434a1..52d3429 100644
> > >> > --- a/xen/arch/arm/domain.c
> > >> > +++ b/xen/arch/arm/domain.c
> > >> > @@ -31,6 +31,8 @@
> > >> >  #include <asm/gic.h>
> > >> >  #include "vtimer.h"
> > >> >  #include "vpl011.h"
> > >> > +#include <xen/iocap.h>
> > >> > +#include <xen/irq.h>
> > >> >
> > >> >  DEFINE_PER_CPU(struct vcpu *, curr_vcpu);
> > >> >
> > >> > @@ -513,8 +515,22 @@ fail:
> > >> >      return rc;
> > >> >  }
> > >> >
> > >> > +static int release_domain_irqs(struct domain *d)
> > >> > +{
> > >> > +    int i;
> > >> > +    for (i = 0; i <= d->nr_pirqs; i++) {
> > >> > +        if (irq_access_permitted(d, i)) {
> > >> > +            release_irq(i);
> > >> > +        }
> > >> > +    }
> > >> > +    return 0;
> > >> > +}
> > >>
> > >> As you may know, release_irq will spin until the flags IRQ_INPROGRESS
> > >> is unset. This is done my the maintenance handler.
> > >>
> > >> Now, let say the domain has crashed and the IRQ is not yet EOIed, then 
> > >> you
> > >> will spin forever.
> > >>
> > >
> > > I put a timeout in the IRQ_INPROGRESS check in release_irq() of 100 
> > > attempts
> > > with a 1 msec delay per loop iteration.
> > 
> > If we plan to only use release_irq when a domain is destroyed, this
> > check is useless,
> > so it should be removed.
> > 
> > An IRQ stays with the flag IRQ_INPROGRESS until Xen will eoi it.
> > If the domain has crashed or received an hard shutdown (ie xl
> > destroy), the IRQ will
> > remain "inflight" and can never come up again.
> > 
> > You need to check if the IRQ is still inflight and, if yes, eoi it.
> >

I think that Julien is right: we need to call something similar to
xen/arch/arm/gic.c:maintenance_interrupt. I would refactor the code
there into a separate function that ends up being called by
maintenance_interrupt and release_irq.


> I will have to research this implementation as I am not familiar with the flow
> and functions in the IRQ handler code.  If you have any suggestions, please
> let me know.  Is it simply a matter of checking IRQ_INFLIGHT bit in the
> desc->status word and then calling a function to EOI the interrupt?

First we need to make sure that the domain is paused and about to be
destroyed otherwise we risk breaking the gic/vgic interface.  Then we
need to check whether the irq is currently inflight, looking at the
inflight queue, see for example
xen/arch/arm/vgic.c:vgic_vcpu_inject_irq.  If the irq is inflight we
need to check whether it's actually in one of the LR registers or just
in the lr_queue. See xen/arch/arm/gic.c:gic_set_guest_irq to see how the
lr_queue works. If the irq is queued there, just remove it from the
lr_queue, EOI it and remove it from the inflight queue. If the irq is
not present in lr_queue, it means it's in one of the LR registers. In
that case we can call something similar to maintenance_interrupt to
remove it from the LR, EOI the interrupt and remove it from the inflight
queue.

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