Re: [Xen-devel] [PATCH] xen/arm: Drop support of platform where GICH_LR_HW is not working correctly

[Pranavkumar Sawargaonkar <pranavkumar@xxxxxxxxxx>]

Hi Julien,

On 15 May 2015 at 21:21, Julien Grall <julien.grall@xxxxxxxxxx> wrote:
> Until recently, a maintenance IRQ was requested for any IRQ injected to
> the guest on x-gene platform.
>
> The commit 50dcb3de603927db2fd87ba09e29c817415aaa44 "xen: arm: X-Gene Storm
> check GIC DIST address for EOI quirk" disable the quirk for boards using
> the new firmware.
>
> It is known that the quirk is fragile and cause performance regression.
> Given that firmware is available for people having the board, I think
> we can request the user to upgrade to the latest version.
>
> Signed-off-by: Julien Grall <julien.grall@xxxxxxxxxx>
> Cc: Pranavkumar Sawargaonkar <pranavkumar@xxxxxxxxxx>
>
> ---
>
> Ref: http://lists.xen.org/archives/html/xen-devel/2015-05/msg00801.html
> ---
>  xen/arch/arm/gic-hip04.c             |  9 ++-------
>  xen/arch/arm/gic-v2.c                |  9 ++-------
>  xen/arch/arm/gic.c                   |  4 ----
>  xen/arch/arm/platforms/xgene-storm.c | 15 ++++++---------
>  xen/include/asm-arm/platform.h       |  5 -----
>  5 files changed, 10 insertions(+), 32 deletions(-)
>
> diff --git a/xen/arch/arm/gic-hip04.c b/xen/arch/arm/gic-hip04.c
> index 5dcd894..ee693e7 100644
> --- a/xen/arch/arm/gic-hip04.c
> +++ b/xen/arch/arm/gic-hip04.c
> @@ -423,13 +423,8 @@ static void hip04gic_update_lr(int lr, const struct 
> pending_irq *p,
>                ((p->irq & GICH_V2_LR_VIRTUAL_MASK) << 
> GICH_V2_LR_VIRTUAL_SHIFT));
>
>      if ( p->desc != NULL )
> -    {
> -        if ( platform_has_quirk(PLATFORM_QUIRK_GUEST_PIRQ_NEED_EOI) )
> -            lr_reg |= GICH_V2_LR_MAINTENANCE_IRQ;
> -        else
> -            lr_reg |= GICH_V2_LR_HW | ((p->desc->irq & 
> GICH_V2_LR_PHYSICAL_MASK )
> -                            << GICH_V2_LR_PHYSICAL_SHIFT);
> -    }
> +        lr_reg |= GICH_V2_LR_HW | ((p->desc->irq & GICH_V2_LR_PHYSICAL_MASK )
> +                                   << GICH_V2_LR_PHYSICAL_SHIFT);
>
>      writel_gich(lr_reg, HIP04_GICH_LR + lr * 4);
>  }
> diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
> index 80acc62..edf659b 100644
> --- a/xen/arch/arm/gic-v2.c
> +++ b/xen/arch/arm/gic-v2.c
> @@ -413,13 +413,8 @@ static void gicv2_update_lr(int lr, const struct 
> pending_irq *p,
>                ((p->irq & GICH_V2_LR_VIRTUAL_MASK) << 
> GICH_V2_LR_VIRTUAL_SHIFT));
>
>      if ( p->desc != NULL )
> -    {
> -        if ( platform_has_quirk(PLATFORM_QUIRK_GUEST_PIRQ_NEED_EOI) )
> -            lr_reg |= GICH_V2_LR_MAINTENANCE_IRQ;
> -        else
> -            lr_reg |= GICH_V2_LR_HW | ((p->desc->irq & 
> GICH_V2_LR_PHYSICAL_MASK )
> -                            << GICH_V2_LR_PHYSICAL_SHIFT);
> -    }
> +        lr_reg |= GICH_V2_LR_HW | ((p->desc->irq & GICH_V2_LR_PHYSICAL_MASK )
> +                                   << GICH_V2_LR_PHYSICAL_SHIFT);
>
>      writel_gich(lr_reg, GICH_LR + lr * 4);
>  }
> diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
> index 125298c..c41e82e 100644
> --- a/xen/arch/arm/gic.c
> +++ b/xen/arch/arm/gic.c
> @@ -445,11 +445,7 @@ static void gic_update_one_lr(struct vcpu *v, int i)
>          clear_bit(i, &this_cpu(lr_mask));
>
>          if ( p->desc != NULL )
> -        {
>              clear_bit(_IRQ_INPROGRESS, &p->desc->status);
> -            if ( platform_has_quirk(PLATFORM_QUIRK_GUEST_PIRQ_NEED_EOI) )
> -                gic_hw_ops->deactivate_irq(p->desc);
> -        }
>          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 c717360..1362bea 100644
> --- a/xen/arch/arm/platforms/xgene-storm.c
> +++ b/xen/arch/arm/platforms/xgene-storm.c
> @@ -37,7 +37,6 @@ static u32 reset_mask;
>  static bool reset_vals_valid = false;
>
>  #define XGENE_SEC_GICV2_DIST_ADDR    0x78010000
> -static u32 __read_mostly xgene_quirks = PLATFORM_QUIRK_GIC_64K_STRIDE;
>
>  static void __init xgene_check_pirq_eoi(void)
>  {
> @@ -60,20 +59,18 @@ static void __init xgene_check_pirq_eoi(void)
>
>      /*
>       * In old X-Gene Storm firmware and DT, secure mode addresses have
> -     * been mentioned in GICv2 node. We have to use maintenance interrupt
> -     * instead of EOI HW in this case. We check the GIC Distributor Base
> -     * Address to maintain compatibility with older firmware.
> +     * been mentioned in GICv2 node. EOI HW won't work in this case.
> +     * We check the GIC Distributor Base Address to deny Xen booting
> +     * with older firmware.
>       */
>      if ( dbase == XGENE_SEC_GICV2_DIST_ADDR )
> -    {
> -        xgene_quirks |= PLATFORM_QUIRK_GUEST_PIRQ_NEED_EOI;
> -        printk("Xen: WARNING: OLD X-Gene Firmware, disabling PIRQ EOI 
> mode\n");
> -    }
> +        panic("OLD X-Gene Firmware is not supported by Xen.\n"
> +              "Please upgrade your firmware to the latest version");
>  }
>
>  static uint32_t xgene_storm_quirks(void)
>  {
> -    return xgene_quirks;
> +    return PLATFORM_QUIRK_GIC_64K_STRIDE;
>  }
>
>  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 4eba37b..746e126 100644
> --- a/xen/include/asm-arm/platform.h
> +++ b/xen/include/asm-arm/platform.h
> @@ -51,11 +51,6 @@ 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);
> --
> 2.1.4
>

Thanks for this patch.
Acked-by: Pranavkumar Sawargaonkar <pranavkumar@xxxxxxxxxx>

Thanks,
Pranav

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