| Message ID | 1404315229-31047-1-git-send-email-stefano.stabellini@eu.citrix.com |
|---|---|
| State | New |
| Headers | show |
Hi Stefano, Sorry I didn't think about it before. Ian has pushed Vijay GICv2 rework today. So you patch won't apply correctly. On 07/02/2014 04:33 PM, Stefano Stabellini wrote: > GICH_LR_HW doesn't work as expected on X-Gene: request maintenance > interrupts and perform EOIs in the hypervisor as a workaround. > Trigger this behaviour with a per platform option. > > This patch assumes that GICC_DIR can be written on any pcpu for a given > IRQ, not matter where GICC_IAR has been read before. I would replace IRQ by SGIs as if it's an SPI/PPI we are screwed. In practice it doesn't happen :). Regards,
On Wed, 2 Jul 2014, Stefano Stabellini wrote: > GICH_LR_HW doesn't work as expected on X-Gene: request maintenance > interrupts and perform EOIs in the hypervisor as a workaround. > Trigger this behaviour with a per platform option. > > This patch assumes that GICC_DIR can be written on any pcpu for a given > IRQ, not matter where GICC_IAR has been read before. > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > CC: psawargaonkar@apm.com > CC: apatel@apm.com > --- > > Changes in v2: > - s/PLATFORM_QUIRK_NEED_EOI/PLATFORM_QUIRK_GUEST_PIRQ_NEED_EOI/g; > - improved commit message. > > --- > > xen/arch/arm/gic.c | 13 ++++++++++++- > xen/arch/arm/platforms/xgene-storm.c | 2 +- > xen/include/asm-arm/platform.h | 5 +++++ > 3 files changed, 18 insertions(+), 2 deletions(-) > > diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c > index 1965f86..3e58c9c 100644 > --- a/xen/arch/arm/gic.c > +++ b/xen/arch/arm/gic.c > @@ -577,7 +577,9 @@ static inline void gic_set_lr(int lr, struct pending_irq *p, > > lr_val = state | (GIC_PRI_TO_GUEST(p->priority) << GICH_LR_PRIORITY_SHIFT) | > ((p->irq & GICH_LR_VIRTUAL_MASK) << GICH_LR_VIRTUAL_SHIFT); > - if ( p->desc != NULL ) > + if ( platform_has_quirk(PLATFORM_QUIRK_GUEST_PIRQ_NEED_EOI) ) > + lr_val |= GICH_LR_MAINTENANCE_IRQ; > + else if ( p->desc != NULL ) > lr_val |= GICH_LR_HW | (p->desc->irq << GICH_LR_PHYSICAL_SHIFT); Actually I had a better idea: we could still avoid maintenance interrupts for virtual interrupts (vtimer and evtchn_irq). I'll resubmit. > GICH[GICH_LR + lr] = lr_val; > @@ -656,6 +658,11 @@ void gic_raise_guest_irq(struct vcpu *v, unsigned int virtual_irq, > gic_add_to_lr_pending(v, irq_to_pending(v, virtual_irq)); > } > > +static void gic_irq_eoi(int irq) > +{ > + GICC[GICC_DIR] = irq; > +} > + > static void gic_update_one_lr(struct vcpu *v, int i) > { > struct pending_irq *p; > @@ -692,7 +699,11 @@ static void gic_update_one_lr(struct vcpu *v, int i) > clear_bit(i, &this_cpu(lr_mask)); > > if ( p->desc != NULL ) > + { > p->desc->status &= ~IRQ_INPROGRESS; > + if ( platform_has_quirk(PLATFORM_QUIRK_GUEST_PIRQ_NEED_EOI) ) > + gic_irq_eoi(p->irq); > + } > clear_bit(GIC_IRQ_GUEST_VISIBLE, &p->status); > clear_bit(GIC_IRQ_GUEST_ACTIVE, &p->status); > p->lr = GIC_INVALID_LR; > diff --git a/xen/arch/arm/platforms/xgene-storm.c b/xen/arch/arm/platforms/xgene-storm.c > index c9dd63c..837d8e6 100644 > --- a/xen/arch/arm/platforms/xgene-storm.c > +++ b/xen/arch/arm/platforms/xgene-storm.c > @@ -37,7 +37,7 @@ static bool reset_vals_valid = false; > > static uint32_t xgene_storm_quirks(void) > { > - return PLATFORM_QUIRK_GIC_64K_STRIDE; > + return PLATFORM_QUIRK_GIC_64K_STRIDE|PLATFORM_QUIRK_GUEST_PIRQ_NEED_EOI; > } > > static int map_one_mmio(struct domain *d, const char *what, > diff --git a/xen/include/asm-arm/platform.h b/xen/include/asm-arm/platform.h > index bcd2097..eefaca6 100644 > --- a/xen/include/asm-arm/platform.h > +++ b/xen/include/asm-arm/platform.h > @@ -55,6 +55,11 @@ struct platform_desc { > */ > #define PLATFORM_QUIRK_GIC_64K_STRIDE (1 << 0) > > +/* > + * Quirk for platforms where GICH_LR_HW does not work as expected. > + */ > +#define PLATFORM_QUIRK_GUEST_PIRQ_NEED_EOI (1 << 1) > + > void __init platform_init(void); > int __init platform_init_time(void); > int __init platform_specific_mapping(struct domain *d); > -- > 1.7.10.4 >
diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c index 1965f86..3e58c9c 100644 --- a/xen/arch/arm/gic.c +++ b/xen/arch/arm/gic.c @@ -577,7 +577,9 @@ static inline void gic_set_lr(int lr, struct pending_irq *p, lr_val = state | (GIC_PRI_TO_GUEST(p->priority) << GICH_LR_PRIORITY_SHIFT) | ((p->irq & GICH_LR_VIRTUAL_MASK) << GICH_LR_VIRTUAL_SHIFT); - if ( p->desc != NULL ) + if ( platform_has_quirk(PLATFORM_QUIRK_GUEST_PIRQ_NEED_EOI) ) + lr_val |= GICH_LR_MAINTENANCE_IRQ; + else if ( p->desc != NULL ) lr_val |= GICH_LR_HW | (p->desc->irq << GICH_LR_PHYSICAL_SHIFT); GICH[GICH_LR + lr] = lr_val; @@ -656,6 +658,11 @@ void gic_raise_guest_irq(struct vcpu *v, unsigned int virtual_irq, gic_add_to_lr_pending(v, irq_to_pending(v, virtual_irq)); } +static void gic_irq_eoi(int irq) +{ + GICC[GICC_DIR] = irq; +} + static void gic_update_one_lr(struct vcpu *v, int i) { struct pending_irq *p; @@ -692,7 +699,11 @@ static void gic_update_one_lr(struct vcpu *v, int i) clear_bit(i, &this_cpu(lr_mask)); if ( p->desc != NULL ) + { p->desc->status &= ~IRQ_INPROGRESS; + if ( platform_has_quirk(PLATFORM_QUIRK_GUEST_PIRQ_NEED_EOI) ) + gic_irq_eoi(p->irq); + } clear_bit(GIC_IRQ_GUEST_VISIBLE, &p->status); clear_bit(GIC_IRQ_GUEST_ACTIVE, &p->status); p->lr = GIC_INVALID_LR; diff --git a/xen/arch/arm/platforms/xgene-storm.c b/xen/arch/arm/platforms/xgene-storm.c index c9dd63c..837d8e6 100644 --- a/xen/arch/arm/platforms/xgene-storm.c +++ b/xen/arch/arm/platforms/xgene-storm.c @@ -37,7 +37,7 @@ static bool reset_vals_valid = false; static uint32_t xgene_storm_quirks(void) { - return PLATFORM_QUIRK_GIC_64K_STRIDE; + return PLATFORM_QUIRK_GIC_64K_STRIDE|PLATFORM_QUIRK_GUEST_PIRQ_NEED_EOI; } static int map_one_mmio(struct domain *d, const char *what, diff --git a/xen/include/asm-arm/platform.h b/xen/include/asm-arm/platform.h index bcd2097..eefaca6 100644 --- a/xen/include/asm-arm/platform.h +++ b/xen/include/asm-arm/platform.h @@ -55,6 +55,11 @@ struct platform_desc { */ #define PLATFORM_QUIRK_GIC_64K_STRIDE (1 << 0) +/* + * Quirk for platforms where GICH_LR_HW does not work as expected. + */ +#define PLATFORM_QUIRK_GUEST_PIRQ_NEED_EOI (1 << 1) + void __init platform_init(void); int __init platform_init_time(void); int __init platform_specific_mapping(struct domain *d);
GICH_LR_HW doesn't work as expected on X-Gene: request maintenance interrupts and perform EOIs in the hypervisor as a workaround. Trigger this behaviour with a per platform option. This patch assumes that GICC_DIR can be written on any pcpu for a given IRQ, not matter where GICC_IAR has been read before. Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> CC: psawargaonkar@apm.com CC: apatel@apm.com --- Changes in v2: - s/PLATFORM_QUIRK_NEED_EOI/PLATFORM_QUIRK_GUEST_PIRQ_NEED_EOI/g; - improved commit message. --- xen/arch/arm/gic.c | 13 ++++++++++++- xen/arch/arm/platforms/xgene-storm.c | 2 +- xen/include/asm-arm/platform.h | 5 +++++ 3 files changed, 18 insertions(+), 2 deletions(-)