Subject: Re: [PATCH 11/13] powerpc/xive: Remove P9 DD1 flag XIVE_IRQ_FLAG_EOI_FW

From: Greg Kurz <[email-masked]>
Date: Wed, 9 Dec 2020 16:24:35 +0100

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

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

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

Same suggestion as with previous patch.

>  arch/powerpc/include/asm/opal-api.h      |  2 +-
>  arch/powerpc/include/asm/xive.h          |  2 +-
>  arch/powerpc/sysdev/xive/xive-internal.h |  1 -
>  arch/powerpc/kvm/book3s_xive_template.c  |  2 --
>  arch/powerpc/sysdev/xive/common.c        | 13 +------------
>  arch/powerpc/sysdev/xive/native.c        | 12 ------------
>  arch/powerpc/sysdev/xive/spapr.c         |  6 ------
>  7 files changed, 3 insertions(+), 35 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/opal-api.h b/arch/powerpc/include/asm/opal-api.h
> index 0455b679c050..0b63ba7d5917 100644
> --- a/arch/powerpc/include/asm/opal-api.h
> +++ b/arch/powerpc/include/asm/opal-api.h
> @@ -1093,7 +1093,7 @@ enum {
>  	OPAL_XIVE_IRQ_LSI		= 0x00000004,
>  	OPAL_XIVE_IRQ_SHIFT_BUG		= 0x00000008, /* P9 DD1.0 workaround */
>  	OPAL_XIVE_IRQ_MASK_VIA_FW	= 0x00000010, /* P9 DD1.0 workaround */
> -	OPAL_XIVE_IRQ_EOI_VIA_FW	= 0x00000020,
> +	OPAL_XIVE_IRQ_EOI_VIA_FW	= 0x00000020, /* P9 DD1.0 workaround */
>  };
>  
>  /* Flags for OPAL_XIVE_GET/SET_QUEUE_INFO */
> diff --git a/arch/powerpc/include/asm/xive.h b/arch/powerpc/include/asm/xive.h
> index d62368d0ba91..f6150d7a757a 100644
> --- a/arch/powerpc/include/asm/xive.h
> +++ b/arch/powerpc/include/asm/xive.h
> @@ -62,7 +62,7 @@ struct xive_irq_data {
>  #define XIVE_IRQ_FLAG_LSI	0x02
>  #define XIVE_IRQ_FLAG_SHIFT_BUG	0x04 /* P9 DD1.0 workaround */
>  #define XIVE_IRQ_FLAG_MASK_FW	0x08 /* P9 DD1.0 workaround */
> -#define XIVE_IRQ_FLAG_EOI_FW	0x10
> +#define XIVE_IRQ_FLAG_EOI_FW	0x10 /* P9 DD1.0 workaround */
>  #define XIVE_IRQ_FLAG_H_INT_ESB	0x20
>  
>  /* Special flag set by KVM for excalation interrupts */
> diff --git a/arch/powerpc/sysdev/xive/xive-internal.h b/arch/powerpc/sysdev/xive/xive-internal.h
> index 066d6fe3dc1d..3b7dd2cba9db 100644
> --- a/arch/powerpc/sysdev/xive/xive-internal.h
> +++ b/arch/powerpc/sysdev/xive/xive-internal.h
> @@ -52,7 +52,6 @@ struct xive_ops {
>  	void	(*shutdown)(void);
>  
>  	void	(*update_pending)(struct xive_cpu *xc);
> -	void	(*eoi)(u32 hw_irq);
>  	void	(*sync_source)(u32 hw_irq);
>  	u64	(*esb_rw)(u32 hw_irq, u32 offset, u64 data, bool write);
>  #ifdef CONFIG_SMP
> diff --git a/arch/powerpc/kvm/book3s_xive_template.c b/arch/powerpc/kvm/book3s_xive_template.c
> index ece36e024a8f..b0015e05d99a 100644
> --- a/arch/powerpc/kvm/book3s_xive_template.c
> +++ b/arch/powerpc/kvm/book3s_xive_template.c
> @@ -74,8 +74,6 @@ static void GLUE(X_PFX,source_eoi)(u32 hw_irq, struct xive_irq_data *xd)
>  	/* If the XIVE supports the new "store EOI facility, use it */
>  	if (xd->flags & XIVE_IRQ_FLAG_STORE_EOI)
>  		__x_writeq(0, __x_eoi_page(xd) + XIVE_ESB_STORE_EOI);
> -	else if (hw_irq && xd->flags & XIVE_IRQ_FLAG_EOI_FW)
> -		opal_int_eoi(hw_irq);
>  	else if (xd->flags & XIVE_IRQ_FLAG_LSI) {
>  		/*
>  		 * For LSIs the HW EOI cycle is used rather than PQ bits,
> diff --git a/arch/powerpc/sysdev/xive/common.c b/arch/powerpc/sysdev/xive/common.c
> index a71412fefb65..fe6229dd3241 100644
> --- a/arch/powerpc/sysdev/xive/common.c
> +++ b/arch/powerpc/sysdev/xive/common.c
> @@ -354,18 +354,7 @@ static void xive_do_source_eoi(u32 hw_irq, struct xive_irq_data *xd)
>  	/* If the XIVE supports the new "store EOI facility, use it */
>  	if (xd->flags & XIVE_IRQ_FLAG_STORE_EOI)
>  		xive_esb_write(xd, XIVE_ESB_STORE_EOI, 0);
> -	else if (hw_irq && xd->flags & XIVE_IRQ_FLAG_EOI_FW) {
> -		/*
> -		 * The FW told us to call it. This happens for some
> -		 * interrupt sources that need additional HW whacking
> -		 * beyond the ESB manipulation. For example LPC interrupts
> -		 * on P9 DD1.0 needed a latch to be clared in the LPC bridge
> -		 * itself. The Firmware will take care of it.
> -		 */
> -		if (WARN_ON_ONCE(!xive_ops->eoi))
> -			return;
> -		xive_ops->eoi(hw_irq);
> -	} else {
> +	else {
>  		u8 eoi_val;
>  
>  		/*
> diff --git a/arch/powerpc/sysdev/xive/native.c b/arch/powerpc/sysdev/xive/native.c
> index deb97ad25d62..4902d05ebbd1 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_EOI_VIA_FW)
> -		data->flags |= XIVE_IRQ_FLAG_EOI_FW;
>  	data->eoi_page = be64_to_cpu(eoi_page);
>  	data->trig_page = be64_to_cpu(trig_page);
>  	data->esb_shift = be32_to_cpu(esb_shift);
> @@ -380,15 +378,6 @@ static void xive_native_update_pending(struct xive_cpu *xc)
>  	}
>  }
>  
> -static void xive_native_eoi(u32 hw_irq)
> -{
> -	/*
> -	 * Not normally used except if specific interrupts need
> -	 * a workaround on EOI.
> -	 */
> -	opal_int_eoi(hw_irq);
> -}
> -
>  static void xive_native_setup_cpu(unsigned int cpu, struct xive_cpu *xc)
>  {
>  	s64 rc;
> @@ -471,7 +460,6 @@ static const struct xive_ops xive_native_ops = {
>  	.match			= xive_native_match,
>  	.shutdown		= xive_native_shutdown,
>  	.update_pending		= xive_native_update_pending,
> -	.eoi			= xive_native_eoi,
>  	.setup_cpu		= xive_native_setup_cpu,
>  	.teardown_cpu		= xive_native_teardown_cpu,
>  	.sync_source		= xive_native_sync_source,
> diff --git a/arch/powerpc/sysdev/xive/spapr.c b/arch/powerpc/sysdev/xive/spapr.c
> index 6610e5149d5a..01ccc0786ada 100644
> --- a/arch/powerpc/sysdev/xive/spapr.c
> +++ b/arch/powerpc/sysdev/xive/spapr.c
> @@ -628,11 +628,6 @@ static void xive_spapr_update_pending(struct xive_cpu *xc)
>  	}
>  }
>  
> -static void xive_spapr_eoi(u32 hw_irq)
> -{
> -	/* Not used */;
> -}
> -
>  static void xive_spapr_setup_cpu(unsigned int cpu, struct xive_cpu *xc)
>  {
>  	/* Only some debug on the TIMA settings */
> @@ -677,7 +672,6 @@ static const struct xive_ops xive_spapr_ops = {
>  	.match			= xive_spapr_match,
>  	.shutdown		= xive_spapr_shutdown,
>  	.update_pending		= xive_spapr_update_pending,
> -	.eoi			= xive_spapr_eoi,
>  	.setup_cpu		= xive_spapr_setup_cpu,
>  	.teardown_cpu		= xive_spapr_teardown_cpu,
>  	.sync_source		= xive_spapr_sync_source,