| Message ID | 1391799378-31664-3-git-send-email-stefano.stabellini@eu.citrix.com |
|---|---|
| State | New |
| Headers | show |
Hi Stefano, On 07/02/14 18:56, Stefano Stabellini wrote: > Do not set GICH_LR_MAINTENANCE_IRQ for every interrupt with set in the > GICH_LR registers. > > Introduce a new function, gic_clear_lrs, that goes over the GICH_LR > registers, clear the invalid ones and free the corresponding interrupts > from the inflight queue if appropriate. Add the interrupt to lr_pending > if the GIC_IRQ_GUEST_PENDING is still set. > > Call gic_clear_lrs from gic_restore_state and on return to guest > (gic_inject). > > Remove the now unused code in maintenance_interrupts and gic_irq_eoi. > > In vgic_vcpu_inject_irq, if the target is a vcpu running on another cpu, > send and SGI to it to interrupt it and force it to clear the old LRs. > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > --- > xen/arch/arm/gic.c | 126 ++++++++++++++++++++++----------------------------- > xen/arch/arm/vgic.c | 3 +- > 2 files changed, 56 insertions(+), 73 deletions(-) > > diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c > index 215b679..87bd5d3 100644 > --- a/xen/arch/arm/gic.c > +++ b/xen/arch/arm/gic.c > +static void gic_clear_lrs(struct vcpu *v) > +{ > + struct pending_irq *p; > + int i = 0, irq; > + uint32_t lr; > + bool_t inflight; > + > + ASSERT(!local_irq_is_enabled()); > + > + while ((i = find_next_bit((const long unsigned int *) &this_cpu(lr_mask), > + nr_lrs, i)) < nr_lrs) { > + lr = GICH[GICH_LR + i]; > + if ( !(lr & (GICH_LR_PENDING|GICH_LR_ACTIVE)) ) > + { > + if ( lr & GICH_LR_HW ) > + irq = (lr >> GICH_LR_PHYSICAL_SHIFT) & GICH_LR_PHYSICAL_MASK; > + else > + irq = (lr >> GICH_LR_VIRTUAL_SHIFT) & GICH_LR_VIRTUAL_MASK; > + The if sentence can be simply by: irq = (lr >> GICH_LR_VIRTUAL_SHIFT) & GICH_LR_VIRTUAL_MASK; > + inflight = 0; > + GICH[GICH_LR + i] = 0; > + clear_bit(i, &this_cpu(lr_mask)); > + > + spin_lock(&gic.lock); > + p = irq_to_pending(v, irq); > + if ( p->desc != NULL ) > + p->desc->status &= ~IRQ_INPROGRESS; > + clear_bit(GIC_IRQ_GUEST_VISIBLE, &p->status); > + if ( test_bit(GIC_IRQ_GUEST_PENDING, &p->status) && > + test_bit(GIC_IRQ_GUEST_ENABLED, &p->status)) > + { I would add a WARN_ON(p->desc != NULL) here. AFAIK, this code path shouldn't be used for physical IRQ.
On 07/02/14 18:56, Stefano Stabellini wrote: > +static void gic_clear_lrs(struct vcpu *v) > +{ > + struct pending_irq *p; > + int i = 0, irq; > + uint32_t lr; > + bool_t inflight; > + > + ASSERT(!local_irq_is_enabled()); > + > + while ((i = find_next_bit((const long unsigned int *) &this_cpu(lr_mask), > + nr_lrs, i)) < nr_lrs) { Did you look at to ELRSR{0,1} registers which list the usable LRs? I think you can use it with the this_cpu(lr_mask) to avoid browsing every LRs.
On Fri, 7 Feb 2014, Julien Grall wrote: > Hi Stefano, > > On 07/02/14 18:56, Stefano Stabellini wrote: > > Do not set GICH_LR_MAINTENANCE_IRQ for every interrupt with set in the > > GICH_LR registers. > > > > Introduce a new function, gic_clear_lrs, that goes over the GICH_LR > > registers, clear the invalid ones and free the corresponding interrupts > > from the inflight queue if appropriate. Add the interrupt to lr_pending > > if the GIC_IRQ_GUEST_PENDING is still set. > > > > Call gic_clear_lrs from gic_restore_state and on return to guest > > (gic_inject). > > > > Remove the now unused code in maintenance_interrupts and gic_irq_eoi. > > > > In vgic_vcpu_inject_irq, if the target is a vcpu running on another cpu, > > send and SGI to it to interrupt it and force it to clear the old LRs. > > > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > --- > > xen/arch/arm/gic.c | 126 > > ++++++++++++++++++++++----------------------------- > > xen/arch/arm/vgic.c | 3 +- > > 2 files changed, 56 insertions(+), 73 deletions(-) > > > > diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c > > index 215b679..87bd5d3 100644 > > --- a/xen/arch/arm/gic.c > > +++ b/xen/arch/arm/gic.c > > +static void gic_clear_lrs(struct vcpu *v) > > +{ > > + struct pending_irq *p; > > + int i = 0, irq; > > + uint32_t lr; > > + bool_t inflight; > > + > > + ASSERT(!local_irq_is_enabled()); > > + > > + while ((i = find_next_bit((const long unsigned int *) > > &this_cpu(lr_mask), > > + nr_lrs, i)) < nr_lrs) { > > + lr = GICH[GICH_LR + i]; > > + if ( !(lr & (GICH_LR_PENDING|GICH_LR_ACTIVE)) ) > > + { > > + if ( lr & GICH_LR_HW ) > > + irq = (lr >> GICH_LR_PHYSICAL_SHIFT) & > > GICH_LR_PHYSICAL_MASK; > > + else > > + irq = (lr >> GICH_LR_VIRTUAL_SHIFT) & GICH_LR_VIRTUAL_MASK; > > + > > The if sentence can be simply by: > > irq = (lr >> GICH_LR_VIRTUAL_SHIFT) & GICH_LR_VIRTUAL_MASK; right > > + inflight = 0; > > + GICH[GICH_LR + i] = 0; > > + clear_bit(i, &this_cpu(lr_mask)); > > + > > + spin_lock(&gic.lock); > > + p = irq_to_pending(v, irq); > > + if ( p->desc != NULL ) > > + p->desc->status &= ~IRQ_INPROGRESS; > > + clear_bit(GIC_IRQ_GUEST_VISIBLE, &p->status); > > + if ( test_bit(GIC_IRQ_GUEST_PENDING, &p->status) && > > + test_bit(GIC_IRQ_GUEST_ENABLED, &p->status)) > > + { > > I would add a WARN_ON(p->desc != NULL) here. AFAIK, this code path shouldn't > be used for physical IRQ. That's not true: an edge physical irq can come through while another one of the same type is being handled. In fact pending and active bits exist even on the physical GIC interface.
On Fri, 7 Feb 2014, Julien Grall wrote: > On 07/02/14 18:56, Stefano Stabellini wrote: > > +static void gic_clear_lrs(struct vcpu *v) > > +{ > > + struct pending_irq *p; > > + int i = 0, irq; > > + uint32_t lr; > > + bool_t inflight; > > + > > + ASSERT(!local_irq_is_enabled()); > > + > > + while ((i = find_next_bit((const long unsigned int *) > > &this_cpu(lr_mask), > > + nr_lrs, i)) < nr_lrs) { > > Did you look at to ELRSR{0,1} registers which list the usable LRs? I think you > can use it with the this_cpu(lr_mask) to avoid browsing every LRs. Given that we only have 4 LR registers, I think that unconditionally reading 2 ELRSR registers would cost more than simply checking lr_mask on average.
On Mon, 2014-02-10 at 17:06 +0000, Stefano Stabellini wrote: > On Fri, 7 Feb 2014, Julien Grall wrote: > > On 07/02/14 18:56, Stefano Stabellini wrote: > > > +static void gic_clear_lrs(struct vcpu *v) > > > +{ > > > + struct pending_irq *p; > > > + int i = 0, irq; > > > + uint32_t lr; > > > + bool_t inflight; > > > + > > > + ASSERT(!local_irq_is_enabled()); > > > + > > > + while ((i = find_next_bit((const long unsigned int *) > > > &this_cpu(lr_mask), > > > + nr_lrs, i)) < nr_lrs) { > > > > Did you look at to ELRSR{0,1} registers which list the usable LRs? I think you > > can use it with the this_cpu(lr_mask) to avoid browsing every LRs. > > Given that we only have 4 LR registers, I think that unconditionally > reading 2 ELRSR registers would cost more than simply checking lr_mask > on average. You also need to actually read the LR and do some bit masking etc don't you? What about implementations with >4 LRs? Ian.
On 02/10/2014 05:06 PM, Stefano Stabellini wrote: > On Fri, 7 Feb 2014, Julien Grall wrote: >> On 07/02/14 18:56, Stefano Stabellini wrote: >> > +static void gic_clear_lrs(struct vcpu *v) >>> +{ >>> + struct pending_irq *p; >>> + int i = 0, irq; >>> + uint32_t lr; >>> + bool_t inflight; >>> + >>> + ASSERT(!local_irq_is_enabled()); >>> + >>> + while ((i = find_next_bit((const long unsigned int *) >>> &this_cpu(lr_mask), >>> + nr_lrs, i)) < nr_lrs) { >> >> Did you look at to ELRSR{0,1} registers which list the usable LRs? I think you >> can use it with the this_cpu(lr_mask) to avoid browsing every LRs. > > Given that we only have 4 LR registers, I think that unconditionally > reading 2 ELRSR registers would cost more than simply checking lr_mask > on average. The maximum number of LR registers is 64. I agree that the current hardwares only handle 4 ... but we should think about future hardware.
On Mon, 10 Feb 2014, Ian Campbell wrote: > On Mon, 2014-02-10 at 17:06 +0000, Stefano Stabellini wrote: > > On Fri, 7 Feb 2014, Julien Grall wrote: > > > On 07/02/14 18:56, Stefano Stabellini wrote: > > > > +static void gic_clear_lrs(struct vcpu *v) > > > > +{ > > > > + struct pending_irq *p; > > > > + int i = 0, irq; > > > > + uint32_t lr; > > > > + bool_t inflight; > > > > + > > > > + ASSERT(!local_irq_is_enabled()); > > > > + > > > > + while ((i = find_next_bit((const long unsigned int *) > > > > &this_cpu(lr_mask), > > > > + nr_lrs, i)) < nr_lrs) { > > > > > > Did you look at to ELRSR{0,1} registers which list the usable LRs? I think you > > > can use it with the this_cpu(lr_mask) to avoid browsing every LRs. > > > > Given that we only have 4 LR registers, I think that unconditionally > > reading 2 ELRSR registers would cost more than simply checking lr_mask > > on average. > > You also need to actually read the LR and do some bit masking etc don't > you? No bit masking but I need to read the LRs, that are just 4. > What about implementations with >4 LRs? I could read ELRSR only if nr_lrs > 8 or something like that.
On Mon, 2014-02-10 at 17:16 +0000, Stefano Stabellini wrote: > On Mon, 10 Feb 2014, Ian Campbell wrote: > > On Mon, 2014-02-10 at 17:06 +0000, Stefano Stabellini wrote: > > > On Fri, 7 Feb 2014, Julien Grall wrote: > > > > On 07/02/14 18:56, Stefano Stabellini wrote: > > > > > +static void gic_clear_lrs(struct vcpu *v) > > > > > +{ > > > > > + struct pending_irq *p; > > > > > + int i = 0, irq; > > > > > + uint32_t lr; > > > > > + bool_t inflight; > > > > > + > > > > > + ASSERT(!local_irq_is_enabled()); > > > > > + > > > > > + while ((i = find_next_bit((const long unsigned int *) > > > > > &this_cpu(lr_mask), > > > > > + nr_lrs, i)) < nr_lrs) { > > > > > > > > Did you look at to ELRSR{0,1} registers which list the usable LRs? I think you > > > > can use it with the this_cpu(lr_mask) to avoid browsing every LRs. > > > > > > Given that we only have 4 LR registers, I think that unconditionally > > > reading 2 ELRSR registers would cost more than simply checking lr_mask > > > on average. > > > > You also need to actually read the LR and do some bit masking etc don't > > you? > > No bit masking but I need to read the LRs, that are just 4. Having read them then what do you do with the result? Surely you need to examine some or all of the bits to determine if it is free or not? > > What about implementations with >4 LRs? > > I could read ELRSR only if nr_lrs > 8 or something like that. That would be the worst of both worlds IMHO. Ian.
On 02/10/2014 05:03 PM, Stefano Stabellini wrote: > >>> + inflight = 0; >>> + GICH[GICH_LR + i] = 0; >>> + clear_bit(i, &this_cpu(lr_mask)); >>> + >>> + spin_lock(&gic.lock); >>> + p = irq_to_pending(v, irq); >>> + if ( p->desc != NULL ) >>> + p->desc->status &= ~IRQ_INPROGRESS; >>> + clear_bit(GIC_IRQ_GUEST_VISIBLE, &p->status); >>> + if ( test_bit(GIC_IRQ_GUEST_PENDING, &p->status) && >>> + test_bit(GIC_IRQ_GUEST_ENABLED, &p->status)) >>> + { >> >> I would add a WARN_ON(p->desc != NULL) here. AFAIK, this code path shouldn't >> be used for physical IRQ. > > That's not true: an edge physical irq can come through while another one > of the same type is being handled. In fact pending and active bits exist > even on the physical GIC interface. > It won't be fired until the previous one is EOIed. The physical GIC interface will keep it internally. But ... after thinking the WARN is stupid here because we can have this following case: IRQ A fired -> inject IRQ A IRQ A eoied IRQ A fired -> set pending bits clear lrs -> re-inject IRQ A
On Mon, 10 Feb 2014, Ian Campbell wrote: > On Mon, 2014-02-10 at 17:16 +0000, Stefano Stabellini wrote: > > On Mon, 10 Feb 2014, Ian Campbell wrote: > > > On Mon, 2014-02-10 at 17:06 +0000, Stefano Stabellini wrote: > > > > On Fri, 7 Feb 2014, Julien Grall wrote: > > > > > On 07/02/14 18:56, Stefano Stabellini wrote: > > > > > > +static void gic_clear_lrs(struct vcpu *v) > > > > > > +{ > > > > > > + struct pending_irq *p; > > > > > > + int i = 0, irq; > > > > > > + uint32_t lr; > > > > > > + bool_t inflight; > > > > > > + > > > > > > + ASSERT(!local_irq_is_enabled()); > > > > > > + > > > > > > + while ((i = find_next_bit((const long unsigned int *) > > > > > > &this_cpu(lr_mask), > > > > > > + nr_lrs, i)) < nr_lrs) { > > > > > > > > > > Did you look at to ELRSR{0,1} registers which list the usable LRs? I think you > > > > > can use it with the this_cpu(lr_mask) to avoid browsing every LRs. > > > > > > > > Given that we only have 4 LR registers, I think that unconditionally > > > > reading 2 ELRSR registers would cost more than simply checking lr_mask > > > > on average. > > > > > > You also need to actually read the LR and do some bit masking etc don't > > > you? > > > > No bit masking but I need to read the LRs, that are just 4. > > Having read them then what do you do with the result? Surely you need to > examine some or all of the bits to determine if it is free or not? Right: if ( !(lr & (GICH_LR_PENDING|GICH_LR_ACTIVE)) ) I thought you meant we need to write back a masked value. > > > What about implementations with >4 LRs? > > > > I could read ELRSR only if nr_lrs > 8 or something like that. > > That would be the worst of both worlds IMHO. On second thought you are right: if we always clear the LRs, the logic is simpler and may be slower with an high number of LRs and concurrent irqs. If we don't clear all the LRs on return to guest and use ELRSR to filter them, we are slower with a small number of LRs / concurrent irqs, faster with an high number of LRs and concurrent irqs but the code would be much more complex. Overall checking for nr_lrs > 8 would add too much complexity for little benefit.
On Mon, 2014-02-10 at 17:24 +0000, Stefano Stabellini wrote: > On Mon, 10 Feb 2014, Ian Campbell wrote: > > On Mon, 2014-02-10 at 17:16 +0000, Stefano Stabellini wrote: > > > On Mon, 10 Feb 2014, Ian Campbell wrote: > > > > On Mon, 2014-02-10 at 17:06 +0000, Stefano Stabellini wrote: > > > > > On Fri, 7 Feb 2014, Julien Grall wrote: > > > > > > On 07/02/14 18:56, Stefano Stabellini wrote: > > > > > > > +static void gic_clear_lrs(struct vcpu *v) > > > > > > > +{ > > > > > > > + struct pending_irq *p; > > > > > > > + int i = 0, irq; > > > > > > > + uint32_t lr; > > > > > > > + bool_t inflight; > > > > > > > + > > > > > > > + ASSERT(!local_irq_is_enabled()); > > > > > > > + > > > > > > > + while ((i = find_next_bit((const long unsigned int *) > > > > > > > &this_cpu(lr_mask), > > > > > > > + nr_lrs, i)) < nr_lrs) { > > > > > > > > > > > > Did you look at to ELRSR{0,1} registers which list the usable LRs? I think you > > > > > > can use it with the this_cpu(lr_mask) to avoid browsing every LRs. > > > > > > > > > > Given that we only have 4 LR registers, I think that unconditionally > > > > > reading 2 ELRSR registers would cost more than simply checking lr_mask > > > > > on average. > > > > > > > > You also need to actually read the LR and do some bit masking etc don't > > > > you? > > > > > > No bit masking but I need to read the LRs, that are just 4. > > > > Having read them then what do you do with the result? Surely you need to > > examine some or all of the bits to determine if it is free or not? > > Right: > > if ( !(lr & (GICH_LR_PENDING|GICH_LR_ACTIVE)) ) This is basically exactly what the ELRSR registers do for you. Why not just use the h/w feature which gives you exactly what you need instead of reinventing it yourself because you think it might be faster? I think there is a certain amount of premature optimisation going on here. Ian.
diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c index 215b679..87bd5d3 100644 --- a/xen/arch/arm/gic.c +++ b/xen/arch/arm/gic.c @@ -67,6 +67,8 @@ static DEFINE_PER_CPU(u8, gic_cpu_id); /* Maximum cpu interface per GIC */ #define NR_GIC_CPU_IF 8 +static void gic_clear_lrs(struct vcpu *v); + static unsigned int gic_cpu_mask(const cpumask_t *cpumask) { unsigned int cpu; @@ -126,6 +128,7 @@ void gic_restore_state(struct vcpu *v) GICH[GICH_HCR] = GICH_HCR_EN; isb(); + gic_clear_lrs(v); gic_restore_pending_irqs(v); } @@ -628,12 +631,12 @@ static inline void gic_set_lr(struct vcpu *v, int lr, unsigned int irq, BUG_ON(state & ~(GICH_LR_STATE_MASK<<GICH_LR_STATE_SHIFT)); if ( p->desc != NULL ) - GICH[GICH_LR + lr] = GICH_LR_HW | state | GICH_LR_MAINTENANCE_IRQ | + GICH[GICH_LR + lr] = GICH_LR_HW | state | ((priority >> 3) << GICH_LR_PRIORITY_SHIFT) | ((irq & GICH_LR_PHYSICAL_MASK) << GICH_LR_PHYSICAL_SHIFT) | ((irq & GICH_LR_VIRTUAL_MASK) << GICH_LR_VIRTUAL_SHIFT); else - GICH[GICH_LR + lr] = state | GICH_LR_MAINTENANCE_IRQ | + GICH[GICH_LR + lr] = state | ((priority >> 3) << GICH_LR_PRIORITY_SHIFT) | ((irq & GICH_LR_VIRTUAL_MASK) << GICH_LR_VIRTUAL_SHIFT); @@ -695,6 +698,54 @@ out: return; } +static void gic_clear_lrs(struct vcpu *v) +{ + struct pending_irq *p; + int i = 0, irq; + uint32_t lr; + bool_t inflight; + + ASSERT(!local_irq_is_enabled()); + + while ((i = find_next_bit((const long unsigned int *) &this_cpu(lr_mask), + nr_lrs, i)) < nr_lrs) { + lr = GICH[GICH_LR + i]; + if ( !(lr & (GICH_LR_PENDING|GICH_LR_ACTIVE)) ) + { + if ( lr & GICH_LR_HW ) + irq = (lr >> GICH_LR_PHYSICAL_SHIFT) & GICH_LR_PHYSICAL_MASK; + else + irq = (lr >> GICH_LR_VIRTUAL_SHIFT) & GICH_LR_VIRTUAL_MASK; + + inflight = 0; + GICH[GICH_LR + i] = 0; + clear_bit(i, &this_cpu(lr_mask)); + + spin_lock(&gic.lock); + p = irq_to_pending(v, irq); + if ( p->desc != NULL ) + p->desc->status &= ~IRQ_INPROGRESS; + clear_bit(GIC_IRQ_GUEST_VISIBLE, &p->status); + if ( test_bit(GIC_IRQ_GUEST_PENDING, &p->status) && + test_bit(GIC_IRQ_GUEST_ENABLED, &p->status)) + { + inflight = 1; + gic_add_to_lr_pending(v, irq, p->priority); + } + spin_unlock(&gic.lock); + if ( !inflight ) + { + spin_lock(&v->arch.vgic.lock); + list_del_init(&p->inflight); + spin_unlock(&v->arch.vgic.lock); + } + + } + + i++; + } +} + static void gic_restore_pending_irqs(struct vcpu *v) { int i; @@ -751,6 +802,8 @@ int gic_events_need_delivery(void) void gic_inject(void) { + gic_clear_lrs(current); + if ( vcpu_info(current, evtchn_upcall_pending) ) vgic_vcpu_inject_irq(current, current->domain->arch.evtchn_irq); @@ -908,77 +961,8 @@ int gicv_setup(struct domain *d) } -static void gic_irq_eoi(void *info) -{ - int virq = (uintptr_t) info; - GICC[GICC_DIR] = virq; -} - static void maintenance_interrupt(int irq, void *dev_id, struct cpu_user_regs *regs) { - int i = 0, virq, pirq = -1; - uint32_t lr; - struct vcpu *v = current; - uint64_t eisr = GICH[GICH_EISR0] | (((uint64_t) GICH[GICH_EISR1]) << 32); - - while ((i = find_next_bit((const long unsigned int *) &eisr, - 64, i)) < 64) { - struct pending_irq *p, *p2; - int cpu; - bool_t inflight; - - cpu = -1; - inflight = 0; - - spin_lock_irq(&gic.lock); - lr = GICH[GICH_LR + i]; - virq = lr & GICH_LR_VIRTUAL_MASK; - GICH[GICH_LR + i] = 0; - clear_bit(i, &this_cpu(lr_mask)); - - p = irq_to_pending(v, virq); - if ( p->desc != NULL ) { - p->desc->status &= ~IRQ_INPROGRESS; - /* Assume only one pcpu needs to EOI the irq */ - cpu = p->desc->arch.eoi_cpu; - pirq = p->desc->irq; - } - if ( test_bit(GIC_IRQ_GUEST_PENDING, &p->status) && - test_bit(GIC_IRQ_GUEST_ENABLED, &p->status)) - { - inflight = 1; - gic_add_to_lr_pending(v, virq, p->priority); - } - - clear_bit(GIC_IRQ_GUEST_VISIBLE, &p->status); - - if ( !list_empty(&v->arch.vgic.lr_pending) ) { - p2 = list_entry(v->arch.vgic.lr_pending.next, typeof(*p2), lr_queue); - gic_set_lr(v, i, p2->irq, GICH_LR_PENDING, p2->priority); - list_del_init(&p2->lr_queue); - set_bit(i, &this_cpu(lr_mask)); - } - spin_unlock_irq(&gic.lock); - - if ( !inflight ) - { - spin_lock_irq(&v->arch.vgic.lock); - list_del_init(&p->inflight); - spin_unlock_irq(&v->arch.vgic.lock); - } - - if ( p->desc != NULL ) { - /* this is not racy because we can't receive another irq of the - * same type until we EOI it. */ - if ( cpu == smp_processor_id() ) - gic_irq_eoi((void*)(uintptr_t)pirq); - else - on_selected_cpus(cpumask_of(cpu), - gic_irq_eoi, (void*)(uintptr_t)pirq, 0); - } - - i++; - } } void gic_dump_info(struct vcpu *v) diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c index 7d10227..da15f4d 100644 --- a/xen/arch/arm/vgic.c +++ b/xen/arch/arm/vgic.c @@ -699,8 +699,7 @@ void vgic_vcpu_inject_irq(struct vcpu *v, unsigned int irq) if ( (irq != current->domain->arch.evtchn_irq) || (!test_bit(GIC_IRQ_GUEST_VISIBLE, &n->status)) ) set_bit(GIC_IRQ_GUEST_PENDING, &n->status); - spin_unlock_irqrestore(&v->arch.vgic.lock, flags); - return; + goto out; } /* vcpu offline */
Do not set GICH_LR_MAINTENANCE_IRQ for every interrupt with set in the GICH_LR registers. Introduce a new function, gic_clear_lrs, that goes over the GICH_LR registers, clear the invalid ones and free the corresponding interrupts from the inflight queue if appropriate. Add the interrupt to lr_pending if the GIC_IRQ_GUEST_PENDING is still set. Call gic_clear_lrs from gic_restore_state and on return to guest (gic_inject). Remove the now unused code in maintenance_interrupts and gic_irq_eoi. In vgic_vcpu_inject_irq, if the target is a vcpu running on another cpu, send and SGI to it to interrupt it and force it to clear the old LRs. Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> --- xen/arch/arm/gic.c | 126 ++++++++++++++++++++++----------------------------- xen/arch/arm/vgic.c | 3 +- 2 files changed, 56 insertions(+), 73 deletions(-)