* [PATCH] powerpc/xive: Fix offset for store EOI MMIOs
@ 2017-06-14 0:19 Benjamin Herrenschmidt
2017-06-14 4:44 ` Michael Ellerman
2017-06-15 22:50 ` Michael Ellerman
0 siblings, 2 replies; 4+ messages in thread
From: Benjamin Herrenschmidt @ 2017-06-14 0:19 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Michael Ellerman
Architecturally we should apply a 0x400 offset for these. Not doing
it will break future HW implementations.
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
arch/powerpc/include/asm/xive.h | 12 +++++++-----
arch/powerpc/kvm/book3s_xive_template.c | 4 ++--
arch/powerpc/sysdev/xive/common.c | 2 +-
3 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/include/asm/xive.h b/arch/powerpc/include/asm/xive.h
index c8a822a..c23ff43 100644
--- a/arch/powerpc/include/asm/xive.h
+++ b/arch/powerpc/include/asm/xive.h
@@ -94,11 +94,13 @@ struct xive_q {
* store at 0 and some ESBs support doing a trigger via a
* separate trigger page.
*/
-#define XIVE_ESB_GET 0x800
-#define XIVE_ESB_SET_PQ_00 0xc00
-#define XIVE_ESB_SET_PQ_01 0xd00
-#define XIVE_ESB_SET_PQ_10 0xe00
-#define XIVE_ESB_SET_PQ_11 0xf00
+#define XIVE_ESB_STORE_EOI 0x400 /* Store */
+#define XIVE_ESB_LOAD_EOI 0x000 /* Load */
+#define XIVE_ESB_GET 0x800 /* Load */
+#define XIVE_ESB_SET_PQ_00 0xc00 /* Load */
+#define XIVE_ESB_SET_PQ_01 0xd00 /* Load */
+#define XIVE_ESB_SET_PQ_10 0xe00 /* Load */
+#define XIVE_ESB_SET_PQ_11 0xf00 /* Load */
#define XIVE_ESB_VAL_P 0x2
#define XIVE_ESB_VAL_Q 0x1
diff --git a/arch/powerpc/kvm/book3s_xive_template.c b/arch/powerpc/kvm/book3s_xive_template.c
index 023a311..4636ca6 100644
--- a/arch/powerpc/kvm/book3s_xive_template.c
+++ b/arch/powerpc/kvm/book3s_xive_template.c
@@ -69,7 +69,7 @@ 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));
+ __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 {
@@ -89,7 +89,7 @@ static void GLUE(X_PFX,source_eoi)(u32 hw_irq, struct xive_irq_data *xd)
* properly.
*/
if (xd->flags & XIVE_IRQ_FLAG_LSI)
- __x_readq(__x_eoi_page(xd));
+ __x_readq(__x_eoi_page(xd) + XIVE_ESB_LOAD_EOI);
else {
eoi_val = GLUE(X_PFX,esb_load)(xd, XIVE_ESB_SET_PQ_00);
diff --git a/arch/powerpc/sysdev/xive/common.c b/arch/powerpc/sysdev/xive/common.c
index 9138250..8f5e303 100644
--- a/arch/powerpc/sysdev/xive/common.c
+++ b/arch/powerpc/sysdev/xive/common.c
@@ -297,7 +297,7 @@ 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)
- out_be64(xd->eoi_mmio, 0);
+ out_be64(xd->eoi_mmio + 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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] powerpc/xive: Fix offset for store EOI MMIOs
2017-06-14 0:19 [PATCH] powerpc/xive: Fix offset for store EOI MMIOs Benjamin Herrenschmidt
@ 2017-06-14 4:44 ` Michael Ellerman
2017-06-14 14:38 ` Benjamin Herrenschmidt
2017-06-15 22:50 ` Michael Ellerman
1 sibling, 1 reply; 4+ messages in thread
From: Michael Ellerman @ 2017-06-14 4:44 UTC (permalink / raw)
To: Benjamin Herrenschmidt, linuxppc-dev
Benjamin Herrenschmidt <benh@kernel.crashing.org> writes:
> Architecturally we should apply a 0x400 offset for these. Not doing
> it will break future HW implementations.
Can you elaborate a bit?
You're changing a write to 0x0 to be a write to 0x400, which at face
value appears like it breaks something, or is already broken?
cheers
> diff --git a/arch/powerpc/include/asm/xive.h b/arch/powerpc/include/asm/xive.h
> index c8a822a..c23ff43 100644
> --- a/arch/powerpc/include/asm/xive.h
> +++ b/arch/powerpc/include/asm/xive.h
> @@ -94,11 +94,13 @@ struct xive_q {
> * store at 0 and some ESBs support doing a trigger via a
> * separate trigger page.
> */
> -#define XIVE_ESB_GET 0x800
> -#define XIVE_ESB_SET_PQ_00 0xc00
> -#define XIVE_ESB_SET_PQ_01 0xd00
> -#define XIVE_ESB_SET_PQ_10 0xe00
> -#define XIVE_ESB_SET_PQ_11 0xf00
> +#define XIVE_ESB_STORE_EOI 0x400 /* Store */
> +#define XIVE_ESB_LOAD_EOI 0x000 /* Load */
> +#define XIVE_ESB_GET 0x800 /* Load */
> +#define XIVE_ESB_SET_PQ_00 0xc00 /* Load */
> +#define XIVE_ESB_SET_PQ_01 0xd00 /* Load */
> +#define XIVE_ESB_SET_PQ_10 0xe00 /* Load */
> +#define XIVE_ESB_SET_PQ_11 0xf00 /* Load */
>
> #define XIVE_ESB_VAL_P 0x2
> #define XIVE_ESB_VAL_Q 0x1
> diff --git a/arch/powerpc/kvm/book3s_xive_template.c b/arch/powerpc/kvm/book3s_xive_template.c
> index 023a311..4636ca6 100644
> --- a/arch/powerpc/kvm/book3s_xive_template.c
> +++ b/arch/powerpc/kvm/book3s_xive_template.c
> @@ -69,7 +69,7 @@ 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));
> + __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 {
> @@ -89,7 +89,7 @@ static void GLUE(X_PFX,source_eoi)(u32 hw_irq, struct xive_irq_data *xd)
> * properly.
> */
> if (xd->flags & XIVE_IRQ_FLAG_LSI)
> - __x_readq(__x_eoi_page(xd));
> + __x_readq(__x_eoi_page(xd) + XIVE_ESB_LOAD_EOI);
> else {
> eoi_val = GLUE(X_PFX,esb_load)(xd, XIVE_ESB_SET_PQ_00);
>
> diff --git a/arch/powerpc/sysdev/xive/common.c b/arch/powerpc/sysdev/xive/common.c
> index 9138250..8f5e303 100644
> --- a/arch/powerpc/sysdev/xive/common.c
> +++ b/arch/powerpc/sysdev/xive/common.c
> @@ -297,7 +297,7 @@ 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)
> - out_be64(xd->eoi_mmio, 0);
> + out_be64(xd->eoi_mmio + 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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] powerpc/xive: Fix offset for store EOI MMIOs
2017-06-14 4:44 ` Michael Ellerman
@ 2017-06-14 14:38 ` Benjamin Herrenschmidt
0 siblings, 0 replies; 4+ messages in thread
From: Benjamin Herrenschmidt @ 2017-06-14 14:38 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev
On Wed, 2017-06-14 at 14:44 +1000, Michael Ellerman wrote:
> Benjamin Herrenschmidt <benh@kernel.crashing.org> writes:
>
> > Architecturally we should apply a 0x400 offset for these. Not doing
> > it will break future HW implementations.
>
> Can you elaborate a bit?
>
> You're changing a write to 0x0 to be a write to 0x400, which at face
> value appears like it breaks something, or is already broken?
The code is broken today, I didn't read the spec properly. We never had
a proper DD2 model to test until recently. The offset of 0 is supposed
to remain for "triggers" though not all sources support both trigger
and store EOI, and in P9 specifically, some sources will treat 0 as a
store EOI :-) But future chips will not. So this makes us use the
properly architected offset which should work always.
Cheers,
Ben.
>
> cheers
>
> > diff --git a/arch/powerpc/include/asm/xive.h b/arch/powerpc/include/asm/xive.h
> > index c8a822a..c23ff43 100644
> > --- a/arch/powerpc/include/asm/xive.h
> > +++ b/arch/powerpc/include/asm/xive.h
> > @@ -94,11 +94,13 @@ struct xive_q {
> > * store at 0 and some ESBs support doing a trigger via a
> > * separate trigger page.
> > */
> > -#define XIVE_ESB_GET 0x800
> > -#define XIVE_ESB_SET_PQ_00 0xc00
> > -#define XIVE_ESB_SET_PQ_01 0xd00
> > -#define XIVE_ESB_SET_PQ_10 0xe00
> > -#define XIVE_ESB_SET_PQ_11 0xf00
> > +#define XIVE_ESB_STORE_EOI 0x400 /* Store */
> > +#define XIVE_ESB_LOAD_EOI 0x000 /* Load */
> > +#define XIVE_ESB_GET 0x800 /* Load */
> > +#define XIVE_ESB_SET_PQ_00 0xc00 /* Load */
> > +#define XIVE_ESB_SET_PQ_01 0xd00 /* Load */
> > +#define XIVE_ESB_SET_PQ_10 0xe00 /* Load */
> > +#define XIVE_ESB_SET_PQ_11 0xf00 /* Load */
> >
> > #define XIVE_ESB_VAL_P 0x2
> > #define XIVE_ESB_VAL_Q 0x1
> > diff --git a/arch/powerpc/kvm/book3s_xive_template.c b/arch/powerpc/kvm/book3s_xive_template.c
> > index 023a311..4636ca6 100644
> > --- a/arch/powerpc/kvm/book3s_xive_template.c
> > +++ b/arch/powerpc/kvm/book3s_xive_template.c
> > @@ -69,7 +69,7 @@ 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));
> > + __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 {
> > @@ -89,7 +89,7 @@ static void GLUE(X_PFX,source_eoi)(u32 hw_irq, struct xive_irq_data *xd)
> > * properly.
> > */
> > if (xd->flags & XIVE_IRQ_FLAG_LSI)
> > - __x_readq(__x_eoi_page(xd));
> > + __x_readq(__x_eoi_page(xd) + XIVE_ESB_LOAD_EOI);
> > else {
> > eoi_val = GLUE(X_PFX,esb_load)(xd, XIVE_ESB_SET_PQ_00);
> >
> > diff --git a/arch/powerpc/sysdev/xive/common.c b/arch/powerpc/sysdev/xive/common.c
> > index 9138250..8f5e303 100644
> > --- a/arch/powerpc/sysdev/xive/common.c
> > +++ b/arch/powerpc/sysdev/xive/common.c
> > @@ -297,7 +297,7 @@ 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)
> > - out_be64(xd->eoi_mmio, 0);
> > + out_be64(xd->eoi_mmio + 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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: powerpc/xive: Fix offset for store EOI MMIOs
2017-06-14 0:19 [PATCH] powerpc/xive: Fix offset for store EOI MMIOs Benjamin Herrenschmidt
2017-06-14 4:44 ` Michael Ellerman
@ 2017-06-15 22:50 ` Michael Ellerman
1 sibling, 0 replies; 4+ messages in thread
From: Michael Ellerman @ 2017-06-15 22:50 UTC (permalink / raw)
To: Benjamin Herrenschmidt, linuxppc-dev; +Cc: Michael Ellerman
On Wed, 2017-06-14 at 00:19:25 UTC, Benjamin Herrenschmidt wrote:
> Architecturally we should apply a 0x400 offset for these. Not doing
> it will break future HW implementations.
>
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Applied to powerpc fixes, thanks.
https://git.kernel.org/powerpc/c/25642705b2359a705784bbbf1655c2
cheers
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-06-15 22:50 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-14 0:19 [PATCH] powerpc/xive: Fix offset for store EOI MMIOs Benjamin Herrenschmidt
2017-06-14 4:44 ` Michael Ellerman
2017-06-14 14:38 ` Benjamin Herrenschmidt
2017-06-15 22:50 ` Michael Ellerman
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).