Re: [PATCH 09/13] powerpc/xive: Remove P9 DD1 flag XIVE_IRQ_FLAG_SHIFT_BUG
On 12/8/20 6:39 PM, Greg Kurz wrote:
> On Tue, 8 Dec 2020 16:11:20 +0100
> Cédric Le Goater 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
>> ---
>
> Reviewed-by: Greg Kurz
>
> 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 = 0x0001,
>> OPAL_XIVE_IRQ_STORE_EOI = 0x0002,
>> OPAL_XIVE_IRQ_LSI = 0x0004,
>> -OPAL_XIVE_IRQ_SHIFT_BUG = 0x0008,
>> +OPAL_XIVE_IRQ_SHIFT_BUG = 0x0008, /* P9 DD1.0 workaround */
>
> Maybe you can even comment the entire line so that any future
> tentative to use that flag breaks build ?
This file is "copied" from OPAL. I think it is best to keep them
in sync.
>
>> OPAL_XIVE_IRQ_MASK_VIA_FW = 0x0010,
>> OPAL_XIVE_IRQ_EOI_VIA_FW= 0x0020,
>> };
>> 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.
Yes. Since this is an internal flag, we can simply remove it.
Thanks,
C.
>
>> #define XIVE_IRQ_FLAG_MASK_FW 0x08
>> #define XIVE_IRQ_FLAG_EOI_FW0x10
>> #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
>> ---
Re: [PATCH 09/13] powerpc/xive: Remove P9 DD1 flag XIVE_IRQ_FLAG_SHIFT_BUG
On Tue, 8 Dec 2020 16:11:20 +0100
Cédric Le Goater 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
> ---
Reviewed-by: Greg Kurz
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 = 0x0001,
> OPAL_XIVE_IRQ_STORE_EOI = 0x0002,
> OPAL_XIVE_IRQ_LSI = 0x0004,
> - OPAL_XIVE_IRQ_SHIFT_BUG = 0x0008,
> + OPAL_XIVE_IRQ_SHIFT_BUG = 0x0008, /* 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 = 0x0010,
> OPAL_XIVE_IRQ_EOI_VIA_FW= 0x0020,
> };
> 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_LSI0x02
> -#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_FW0x08
> #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 |=
[PATCH 09/13] powerpc/xive: Remove P9 DD1 flag XIVE_IRQ_FLAG_SHIFT_BUG
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
---
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 = 0x0001,
OPAL_XIVE_IRQ_STORE_EOI = 0x0002,
OPAL_XIVE_IRQ_LSI = 0x0004,
- OPAL_XIVE_IRQ_SHIFT_BUG = 0x0008,
+ OPAL_XIVE_IRQ_SHIFT_BUG = 0x0008, /* P9 DD1.0 workaround */
OPAL_XIVE_IRQ_MASK_VIA_FW = 0x0010,
OPAL_XIVE_IRQ_EOI_VIA_FW= 0x0020,
};
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_EOI0x01
#define XIVE_IRQ_FLAG_LSI 0x02
-#define XIVE_IRQ_FLAG_SHIFT_BUG0x04
+#define XIVE_IRQ_FLAG_SHIFT_BUG0x04 /* P9 DD1.0 workaround */
#define XIVE_IRQ_FLAG_MASK_FW 0x08
#define XIVE_IRQ_FLAG_EOI_FW 0x10
#define XIVE_IRQ_FLAG_H_INT_ESB0x20
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)
--
2.26.2
