Subject: Re: [PATCH 09/13] powerpc/xive: Remove P9 DD1 flag XIVE_IRQ_FLAG_SHIFT_BUG

From: Greg Kurz <[email-masked]>
Date: Tue, 8 Dec 2020 18:39:06 +0100

On Tue, 8 Dec 2020 16:11:20 +0100
Cédric Le Goater <[email-masked]> wrote:

> This flag was used to support the PHB4 LSIs on P9 DD1 and we have
> stopped supporting this CPU when DD2 came out. See skiboot commit:
> 
>   https://github.com/open-power/skiboot/commit/0b0d15e3c170
> 
> Signed-off-by: Cédric Le Goater <[email-masked]>
> ---

Reviewed-by: Greg Kurz <[email-masked]>

Just a minor suggestion in case you need to post a v2. See below.

>  arch/powerpc/include/asm/opal-api.h     | 2 +-
>  arch/powerpc/include/asm/xive.h         | 2 +-
>  arch/powerpc/kvm/book3s_xive_native.c   | 3 ---
>  arch/powerpc/kvm/book3s_xive_template.c | 3 ---
>  arch/powerpc/sysdev/xive/common.c       | 8 --------
>  arch/powerpc/sysdev/xive/native.c       | 2 --
>  6 files changed, 2 insertions(+), 18 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/opal-api.h b/arch/powerpc/include/asm/opal-api.h
> index 1dffa3cb16ba..48ee604ca39a 100644
> --- a/arch/powerpc/include/asm/opal-api.h
> +++ b/arch/powerpc/include/asm/opal-api.h
> @@ -1091,7 +1091,7 @@ enum {
>  	OPAL_XIVE_IRQ_TRIGGER_PAGE	= 0x00000001,
>  	OPAL_XIVE_IRQ_STORE_EOI		= 0x00000002,
>  	OPAL_XIVE_IRQ_LSI		= 0x00000004,
> -	OPAL_XIVE_IRQ_SHIFT_BUG		= 0x00000008,
> +	OPAL_XIVE_IRQ_SHIFT_BUG		= 0x00000008, /* P9 DD1.0 workaround */

Maybe you can even comment the entire line so that any future
tentative to use that flag breaks build ?

>  	OPAL_XIVE_IRQ_MASK_VIA_FW	= 0x00000010,
>  	OPAL_XIVE_IRQ_EOI_VIA_FW	= 0x00000020,
>  };
> diff --git a/arch/powerpc/include/asm/xive.h b/arch/powerpc/include/asm/xive.h
> index d332dd9a18de..ff805885a028 100644
> --- a/arch/powerpc/include/asm/xive.h
> +++ b/arch/powerpc/include/asm/xive.h
> @@ -60,7 +60,7 @@ struct xive_irq_data {
>  };
>  #define XIVE_IRQ_FLAG_STORE_EOI	0x01
>  #define XIVE_IRQ_FLAG_LSI	0x02
> -#define XIVE_IRQ_FLAG_SHIFT_BUG	0x04
> +#define XIVE_IRQ_FLAG_SHIFT_BUG	0x04 /* P9 DD1.0 workaround */

Same here, with an extra cleanup to stop using it when initializing 
xive_irq_flags[] in common.c.

>  #define XIVE_IRQ_FLAG_MASK_FW	0x08
>  #define XIVE_IRQ_FLAG_EOI_FW	0x10
>  #define XIVE_IRQ_FLAG_H_INT_ESB	0x20
> diff --git a/arch/powerpc/kvm/book3s_xive_native.c b/arch/powerpc/kvm/book3s_xive_native.c
> index 9b395381179d..170d1d04e1d1 100644
> --- a/arch/powerpc/kvm/book3s_xive_native.c
> +++ b/arch/powerpc/kvm/book3s_xive_native.c
> @@ -37,9 +37,6 @@ static u8 xive_vm_esb_load(struct xive_irq_data *xd, u32 offset)
>  	 * ordering.
>  	 */
>  
> -	if (xd->flags & XIVE_IRQ_FLAG_SHIFT_BUG)
> -		offset |= offset << 4;
> -
>  	val = in_be64(xd->eoi_mmio + offset);
>  	return (u8)val;
>  }
> diff --git a/arch/powerpc/kvm/book3s_xive_template.c b/arch/powerpc/kvm/book3s_xive_template.c
> index 4ad3c0279458..ece36e024a8f 100644
> --- a/arch/powerpc/kvm/book3s_xive_template.c
> +++ b/arch/powerpc/kvm/book3s_xive_template.c
> @@ -61,9 +61,6 @@ static u8 GLUE(X_PFX,esb_load)(struct xive_irq_data *xd, u32 offset)
>  	if (offset == XIVE_ESB_SET_PQ_10 && xd->flags & XIVE_IRQ_FLAG_STORE_EOI)
>  		offset |= XIVE_ESB_LD_ST_MO;
>  
> -	if (xd->flags & XIVE_IRQ_FLAG_SHIFT_BUG)
> -		offset |= offset << 4;
> -
>  	val =__x_readq(__x_eoi_page(xd) + offset);
>  #ifdef __LITTLE_ENDIAN__
>  	val >>= 64-8;
> diff --git a/arch/powerpc/sysdev/xive/common.c b/arch/powerpc/sysdev/xive/common.c
> index 411cba12d73b..a9259470bf9f 100644
> --- a/arch/powerpc/sysdev/xive/common.c
> +++ b/arch/powerpc/sysdev/xive/common.c
> @@ -200,10 +200,6 @@ static notrace u8 xive_esb_read(struct xive_irq_data *xd, u32 offset)
>  	if (offset == XIVE_ESB_SET_PQ_10 && xd->flags & XIVE_IRQ_FLAG_STORE_EOI)
>  		offset |= XIVE_ESB_LD_ST_MO;
>  
> -	/* Handle HW errata */
> -	if (xd->flags & XIVE_IRQ_FLAG_SHIFT_BUG)
> -		offset |= offset << 4;
> -
>  	if ((xd->flags & XIVE_IRQ_FLAG_H_INT_ESB) && xive_ops->esb_rw)
>  		val = xive_ops->esb_rw(xd->hw_irq, offset, 0, 0);
>  	else
> @@ -214,10 +210,6 @@ static notrace u8 xive_esb_read(struct xive_irq_data *xd, u32 offset)
>  
>  static void xive_esb_write(struct xive_irq_data *xd, u32 offset, u64 data)
>  {
> -	/* Handle HW errata */
> -	if (xd->flags & XIVE_IRQ_FLAG_SHIFT_BUG)
> -		offset |= offset << 4;
> -
>  	if ((xd->flags & XIVE_IRQ_FLAG_H_INT_ESB) && xive_ops->esb_rw)
>  		xive_ops->esb_rw(xd->hw_irq, offset, data, 1);
>  	else
> diff --git a/arch/powerpc/sysdev/xive/native.c b/arch/powerpc/sysdev/xive/native.c
> index 5f1e5aed8ab4..0310783241b5 100644
> --- a/arch/powerpc/sysdev/xive/native.c
> +++ b/arch/powerpc/sysdev/xive/native.c
> @@ -64,8 +64,6 @@ int xive_native_populate_irq_data(u32 hw_irq, struct xive_irq_data *data)
>  		data->flags |= XIVE_IRQ_FLAG_STORE_EOI;
>  	if (opal_flags & OPAL_XIVE_IRQ_LSI)
>  		data->flags |= XIVE_IRQ_FLAG_LSI;
> -	if (opal_flags & OPAL_XIVE_IRQ_SHIFT_BUG)
> -		data->flags |= XIVE_IRQ_FLAG_SHIFT_BUG;
>  	if (opal_flags & OPAL_XIVE_IRQ_MASK_VIA_FW)
>  		data->flags |= XIVE_IRQ_FLAG_MASK_FW;
>  	if (opal_flags & OPAL_XIVE_IRQ_EOI_VIA_FW)