target/ppc/cpu.h | 5 ++++- target/ppc/translate.c | 19 +++++++++++++++++-- target/ppc/translate_init.inc.c | 3 ++- 3 files changed, 23 insertions(+), 4 deletions(-)
POWER9 introduced a new variant of the eieio instruction using bit 6
as a hint to tell the CPU it is a store-forwarding barrier.
The usage of this eieio extension was recently added in Linux 4.17
which activated the "support for a store forwarding barrier at kernel
entry/exit".
Unfortunately, it is not possible to insert this new eieio instruction
without considerable change in ppc_tr_translate_insn(). So instead we
loosen the QEMU eieio instruction mask. The gen_eieio() helper is
modified to test for bit6 and a custom instruction flag to catch
invalid eieio opcodes on non-POWER9 CPUs.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
target/ppc/cpu.h | 5 ++++-
target/ppc/translate.c | 19 +++++++++++++++++--
target/ppc/translate_init.inc.c | 3 ++-
3 files changed, 23 insertions(+), 4 deletions(-)
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 0247c1f04c37..021c9b2f10d1 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -2206,6 +2206,9 @@ enum {
/* POWER ISA 3.0 */
PPC2_ISA300 = 0x0000000000080000ULL,
+ /* POWER ISA 3.0 eieio variants */
+ PPC2_MEM_EIEIO2 = 0x0000000000100000ULL,
+
#define PPC_TCG_INSNS2 (PPC2_BOOKE206 | PPC2_VSX | PPC2_PRCNTL | PPC2_DBRX | \
PPC2_ISA205 | PPC2_VSX207 | PPC2_PERM_ISA206 | \
PPC2_DIVE_ISA206 | PPC2_ATOMIC_ISA206 | \
@@ -2213,7 +2216,7 @@ enum {
PPC2_BCTAR_ISA207 | PPC2_LSQ_ISA207 | \
PPC2_ALTIVEC_207 | PPC2_ISA207S | PPC2_DFP | \
PPC2_FP_CVT_S64 | PPC2_TM | PPC2_PM_ISA206 | \
- PPC2_ISA300)
+ PPC2_ISA300 | PPC2_MEM_EIEIO2)
};
/*****************************************************************************/
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 8ba8f67dc513..a73ef02aef1d 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -2967,7 +2967,22 @@ static void gen_stswx(DisasContext *ctx)
/* eieio */
static void gen_eieio(DisasContext *ctx)
{
- tcg_gen_mb(TCG_MO_LD_ST | TCG_BAR_SC);
+ TCGBar bar = TCG_MO_LD_ST;
+
+ /*
+ * POWER9 has a eieio instruction variant using bit 6 as a hint to
+ * tell the CPU it is a store-forwarding barrier.
+ */
+ if (ctx->opcode & 0x2000000) {
+ if (!(ctx->insns_flags2 & PPC2_MEM_EIEIO2)) {
+ qemu_log_mask(LOG_GUEST_ERROR, "invalid eieio using bit 6 at @"
+ TARGET_FMT_lx "\n", ctx->base.pc_next - 4);
+ return;
+ }
+ bar = TCG_MO_ST_LD;
+ }
+
+ tcg_gen_mb(bar | TCG_BAR_SC);
}
#if !defined(CONFIG_USER_ONLY)
@@ -6483,7 +6498,7 @@ GEN_HANDLER(lswi, 0x1F, 0x15, 0x12, 0x00000001, PPC_STRING),
GEN_HANDLER(lswx, 0x1F, 0x15, 0x10, 0x00000001, PPC_STRING),
GEN_HANDLER(stswi, 0x1F, 0x15, 0x16, 0x00000001, PPC_STRING),
GEN_HANDLER(stswx, 0x1F, 0x15, 0x14, 0x00000001, PPC_STRING),
-GEN_HANDLER(eieio, 0x1F, 0x16, 0x1A, 0x03FFF801, PPC_MEM_EIEIO),
+GEN_HANDLER(eieio, 0x1F, 0x16, 0x1A, 0x01FFF801, PPC_MEM_EIEIO),
GEN_HANDLER(isync, 0x13, 0x16, 0x04, 0x03FFF801, PPC_MEM),
GEN_HANDLER_E(lbarx, 0x1F, 0x14, 0x01, 0, PPC_NONE, PPC2_ATOMIC_ISA206),
GEN_HANDLER_E(lharx, 0x1F, 0x14, 0x03, 0, PPC_NONE, PPC2_ATOMIC_ISA206),
diff --git a/target/ppc/translate_init.inc.c b/target/ppc/translate_init.inc.c
index 1a89017ddea8..6b70f6dc5a2c 100644
--- a/target/ppc/translate_init.inc.c
+++ b/target/ppc/translate_init.inc.c
@@ -8847,7 +8847,8 @@ POWERPC_FAMILY(POWER9)(ObjectClass *oc, void *data)
PPC2_FP_TST_ISA206 | PPC2_BCTAR_ISA207 |
PPC2_LSQ_ISA207 | PPC2_ALTIVEC_207 |
PPC2_ISA205 | PPC2_ISA207S | PPC2_FP_CVT_S64 |
- PPC2_TM | PPC2_PM_ISA206 | PPC2_ISA300 | PPC2_PRCNTL;
+ PPC2_TM | PPC2_PM_ISA206 | PPC2_ISA300 | PPC2_PRCNTL |
+ PPC2_MEM_EIEIO2;
pcc->msr_mask = (1ull << MSR_SF) |
(1ull << MSR_TM) |
(1ull << MSR_VR) |
--
2.13.6
On Wed, Jun 06, 2018 at 08:08:30AM +0200, Cédric Le Goater wrote:
> POWER9 introduced a new variant of the eieio instruction using bit 6
> as a hint to tell the CPU it is a store-forwarding barrier.
>
> The usage of this eieio extension was recently added in Linux 4.17
> which activated the "support for a store forwarding barrier at kernel
> entry/exit".
>
> Unfortunately, it is not possible to insert this new eieio instruction
> without considerable change in ppc_tr_translate_insn(). So instead we
> loosen the QEMU eieio instruction mask. The gen_eieio() helper is
> modified to test for bit6 and a custom instruction flag to catch
> invalid eieio opcodes on non-POWER9 CPUs.
>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
> target/ppc/cpu.h | 5 ++++-
> target/ppc/translate.c | 19 +++++++++++++++++--
> target/ppc/translate_init.inc.c | 3 ++-
> 3 files changed, 23 insertions(+), 4 deletions(-)
>
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index 0247c1f04c37..021c9b2f10d1 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -2206,6 +2206,9 @@ enum {
> /* POWER ISA 3.0 */
> PPC2_ISA300 = 0x0000000000080000ULL,
>
> + /* POWER ISA 3.0 eieio variants */
> + PPC2_MEM_EIEIO2 = 0x0000000000100000ULL,
> +
> #define PPC_TCG_INSNS2 (PPC2_BOOKE206 | PPC2_VSX | PPC2_PRCNTL | PPC2_DBRX | \
> PPC2_ISA205 | PPC2_VSX207 | PPC2_PERM_ISA206 | \
> PPC2_DIVE_ISA206 | PPC2_ATOMIC_ISA206 | \
> @@ -2213,7 +2216,7 @@ enum {
> PPC2_BCTAR_ISA207 | PPC2_LSQ_ISA207 | \
> PPC2_ALTIVEC_207 | PPC2_ISA207S | PPC2_DFP | \
> PPC2_FP_CVT_S64 | PPC2_TM | PPC2_PM_ISA206 | \
> - PPC2_ISA300)
> + PPC2_ISA300 | PPC2_MEM_EIEIO2)
> };
>
> /*****************************************************************************/
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index 8ba8f67dc513..a73ef02aef1d 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -2967,7 +2967,22 @@ static void gen_stswx(DisasContext *ctx)
> /* eieio */
> static void gen_eieio(DisasContext *ctx)
> {
> - tcg_gen_mb(TCG_MO_LD_ST | TCG_BAR_SC);
> + TCGBar bar = TCG_MO_LD_ST;
> +
> + /*
> + * POWER9 has a eieio instruction variant using bit 6 as a hint to
> + * tell the CPU it is a store-forwarding barrier.
> + */
> + if (ctx->opcode & 0x2000000) {
> + if (!(ctx->insns_flags2 & PPC2_MEM_EIEIO2)) {
Since we have to adapt the gen_eieio code anyway we don't really need
the new instruction flag - we can just reuse PPC2_ISA300 for it.
> + qemu_log_mask(LOG_GUEST_ERROR, "invalid eieio using bit 6 at @"
> + TARGET_FMT_lx "\n", ctx->base.pc_next - 4);
Logging is ok, but it's not enough in this context - that will turn
the variant eieio into a no-op. You need to actually generate an
invalid instruction exception here, which is what I assume will happen
if you try to execute the new variant on a POWER8:
gen_inval_exception(ctx, POWERPC_EXCP_INVAL_INVAL);
> + return;
> + }
> + bar = TCG_MO_ST_LD;
> + }
> +
> + tcg_gen_mb(bar | TCG_BAR_SC);
> }
>
> #if !defined(CONFIG_USER_ONLY)
> @@ -6483,7 +6498,7 @@ GEN_HANDLER(lswi, 0x1F, 0x15, 0x12, 0x00000001, PPC_STRING),
> GEN_HANDLER(lswx, 0x1F, 0x15, 0x10, 0x00000001, PPC_STRING),
> GEN_HANDLER(stswi, 0x1F, 0x15, 0x16, 0x00000001, PPC_STRING),
> GEN_HANDLER(stswx, 0x1F, 0x15, 0x14, 0x00000001, PPC_STRING),
> -GEN_HANDLER(eieio, 0x1F, 0x16, 0x1A, 0x03FFF801, PPC_MEM_EIEIO),
> +GEN_HANDLER(eieio, 0x1F, 0x16, 0x1A, 0x01FFF801, PPC_MEM_EIEIO),
> GEN_HANDLER(isync, 0x13, 0x16, 0x04, 0x03FFF801, PPC_MEM),
> GEN_HANDLER_E(lbarx, 0x1F, 0x14, 0x01, 0, PPC_NONE, PPC2_ATOMIC_ISA206),
> GEN_HANDLER_E(lharx, 0x1F, 0x14, 0x03, 0, PPC_NONE, PPC2_ATOMIC_ISA206),
> diff --git a/target/ppc/translate_init.inc.c b/target/ppc/translate_init.inc.c
> index 1a89017ddea8..6b70f6dc5a2c 100644
> --- a/target/ppc/translate_init.inc.c
> +++ b/target/ppc/translate_init.inc.c
> @@ -8847,7 +8847,8 @@ POWERPC_FAMILY(POWER9)(ObjectClass *oc, void *data)
> PPC2_FP_TST_ISA206 | PPC2_BCTAR_ISA207 |
> PPC2_LSQ_ISA207 | PPC2_ALTIVEC_207 |
> PPC2_ISA205 | PPC2_ISA207S | PPC2_FP_CVT_S64 |
> - PPC2_TM | PPC2_PM_ISA206 | PPC2_ISA300 | PPC2_PRCNTL;
> + PPC2_TM | PPC2_PM_ISA206 | PPC2_ISA300 | PPC2_PRCNTL |
> + PPC2_MEM_EIEIO2;
> pcc->msr_mask = (1ull << MSR_SF) |
> (1ull << MSR_TM) |
> (1ull << MSR_VR) |
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
On 06/06/2018 08:32 AM, David Gibson wrote:
> On Wed, Jun 06, 2018 at 08:08:30AM +0200, Cédric Le Goater wrote:
>> POWER9 introduced a new variant of the eieio instruction using bit 6
>> as a hint to tell the CPU it is a store-forwarding barrier.
>>
>> The usage of this eieio extension was recently added in Linux 4.17
>> which activated the "support for a store forwarding barrier at kernel
>> entry/exit".
>>
>> Unfortunately, it is not possible to insert this new eieio instruction
>> without considerable change in ppc_tr_translate_insn(). So instead we
>> loosen the QEMU eieio instruction mask. The gen_eieio() helper is
>> modified to test for bit6 and a custom instruction flag to catch
>> invalid eieio opcodes on non-POWER9 CPUs.
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>> target/ppc/cpu.h | 5 ++++-
>> target/ppc/translate.c | 19 +++++++++++++++++--
>> target/ppc/translate_init.inc.c | 3 ++-
>> 3 files changed, 23 insertions(+), 4 deletions(-)
>>
>> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
>> index 0247c1f04c37..021c9b2f10d1 100644
>> --- a/target/ppc/cpu.h
>> +++ b/target/ppc/cpu.h
>> @@ -2206,6 +2206,9 @@ enum {
>> /* POWER ISA 3.0 */
>> PPC2_ISA300 = 0x0000000000080000ULL,
>>
>> + /* POWER ISA 3.0 eieio variants */
>> + PPC2_MEM_EIEIO2 = 0x0000000000100000ULL,
>> +
>> #define PPC_TCG_INSNS2 (PPC2_BOOKE206 | PPC2_VSX | PPC2_PRCNTL | PPC2_DBRX | \
>> PPC2_ISA205 | PPC2_VSX207 | PPC2_PERM_ISA206 | \
>> PPC2_DIVE_ISA206 | PPC2_ATOMIC_ISA206 | \
>> @@ -2213,7 +2216,7 @@ enum {
>> PPC2_BCTAR_ISA207 | PPC2_LSQ_ISA207 | \
>> PPC2_ALTIVEC_207 | PPC2_ISA207S | PPC2_DFP | \
>> PPC2_FP_CVT_S64 | PPC2_TM | PPC2_PM_ISA206 | \
>> - PPC2_ISA300)
>> + PPC2_ISA300 | PPC2_MEM_EIEIO2)
>> };
>>
>> /*****************************************************************************/
>> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
>> index 8ba8f67dc513..a73ef02aef1d 100644
>> --- a/target/ppc/translate.c
>> +++ b/target/ppc/translate.c
>> @@ -2967,7 +2967,22 @@ static void gen_stswx(DisasContext *ctx)
>> /* eieio */
>> static void gen_eieio(DisasContext *ctx)
>> {
>> - tcg_gen_mb(TCG_MO_LD_ST | TCG_BAR_SC);
>> + TCGBar bar = TCG_MO_LD_ST;
>> +
>> + /*
>> + * POWER9 has a eieio instruction variant using bit 6 as a hint to
>> + * tell the CPU it is a store-forwarding barrier.
>> + */
>> + if (ctx->opcode & 0x2000000) {
>> + if (!(ctx->insns_flags2 & PPC2_MEM_EIEIO2)) {
>
> Since we have to adapt the gen_eieio code anyway we don't really need
> the new instruction flag - we can just reuse PPC2_ISA300 for it.
OK. That seems now a little superfluous, indeed. Unless there are more
or these instructions which depend on CPU revision. I don't know so
let's stick to PPC2_ISA300.
>
>> + qemu_log_mask(LOG_GUEST_ERROR, "invalid eieio using bit 6 at @"
>> + TARGET_FMT_lx "\n", ctx->base.pc_next - 4);
>
> Logging is ok, but it's not enough in this context - that will turn
> the variant eieio into a no-op. You need to actually generate an
> invalid instruction exception here, which is what I assume will happen
> if you try to execute the new variant on a POWER8:
> gen_inval_exception(ctx, POWERPC_EXCP_INVAL_INVAL);
well, I didn't try but ISA says that :
"Reserved fields in instructions are ignored by the processor."
So it should be a no-op.
C.
>> + return;
>> + }
>> + bar = TCG_MO_ST_LD;
>> + }
>> +
>> + tcg_gen_mb(bar | TCG_BAR_SC);
>> }
>>
>> #if !defined(CONFIG_USER_ONLY)
>> @@ -6483,7 +6498,7 @@ GEN_HANDLER(lswi, 0x1F, 0x15, 0x12, 0x00000001, PPC_STRING),
>> GEN_HANDLER(lswx, 0x1F, 0x15, 0x10, 0x00000001, PPC_STRING),
>> GEN_HANDLER(stswi, 0x1F, 0x15, 0x16, 0x00000001, PPC_STRING),
>> GEN_HANDLER(stswx, 0x1F, 0x15, 0x14, 0x00000001, PPC_STRING),
>> -GEN_HANDLER(eieio, 0x1F, 0x16, 0x1A, 0x03FFF801, PPC_MEM_EIEIO),
>> +GEN_HANDLER(eieio, 0x1F, 0x16, 0x1A, 0x01FFF801, PPC_MEM_EIEIO),
>> GEN_HANDLER(isync, 0x13, 0x16, 0x04, 0x03FFF801, PPC_MEM),
>> GEN_HANDLER_E(lbarx, 0x1F, 0x14, 0x01, 0, PPC_NONE, PPC2_ATOMIC_ISA206),
>> GEN_HANDLER_E(lharx, 0x1F, 0x14, 0x03, 0, PPC_NONE, PPC2_ATOMIC_ISA206),
>> diff --git a/target/ppc/translate_init.inc.c b/target/ppc/translate_init.inc.c
>> index 1a89017ddea8..6b70f6dc5a2c 100644
>> --- a/target/ppc/translate_init.inc.c
>> +++ b/target/ppc/translate_init.inc.c
>> @@ -8847,7 +8847,8 @@ POWERPC_FAMILY(POWER9)(ObjectClass *oc, void *data)
>> PPC2_FP_TST_ISA206 | PPC2_BCTAR_ISA207 |
>> PPC2_LSQ_ISA207 | PPC2_ALTIVEC_207 |
>> PPC2_ISA205 | PPC2_ISA207S | PPC2_FP_CVT_S64 |
>> - PPC2_TM | PPC2_PM_ISA206 | PPC2_ISA300 | PPC2_PRCNTL;
>> + PPC2_TM | PPC2_PM_ISA206 | PPC2_ISA300 | PPC2_PRCNTL |
>> + PPC2_MEM_EIEIO2;
>> pcc->msr_mask = (1ull << MSR_SF) |
>> (1ull << MSR_TM) |
>> (1ull << MSR_VR) |
>
On Wed, Jun 06, 2018 at 08:42:08AM +0200, Cédric Le Goater wrote:
> On 06/06/2018 08:32 AM, David Gibson wrote:
> > On Wed, Jun 06, 2018 at 08:08:30AM +0200, Cédric Le Goater wrote:
> >> POWER9 introduced a new variant of the eieio instruction using bit 6
> >> as a hint to tell the CPU it is a store-forwarding barrier.
> >>
> >> The usage of this eieio extension was recently added in Linux 4.17
> >> which activated the "support for a store forwarding barrier at kernel
> >> entry/exit".
> >>
> >> Unfortunately, it is not possible to insert this new eieio instruction
> >> without considerable change in ppc_tr_translate_insn(). So instead we
> >> loosen the QEMU eieio instruction mask. The gen_eieio() helper is
> >> modified to test for bit6 and a custom instruction flag to catch
> >> invalid eieio opcodes on non-POWER9 CPUs.
> >>
> >> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> >> ---
> >> target/ppc/cpu.h | 5 ++++-
> >> target/ppc/translate.c | 19 +++++++++++++++++--
> >> target/ppc/translate_init.inc.c | 3 ++-
> >> 3 files changed, 23 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> >> index 0247c1f04c37..021c9b2f10d1 100644
> >> --- a/target/ppc/cpu.h
> >> +++ b/target/ppc/cpu.h
> >> @@ -2206,6 +2206,9 @@ enum {
> >> /* POWER ISA 3.0 */
> >> PPC2_ISA300 = 0x0000000000080000ULL,
> >>
> >> + /* POWER ISA 3.0 eieio variants */
> >> + PPC2_MEM_EIEIO2 = 0x0000000000100000ULL,
> >> +
> >> #define PPC_TCG_INSNS2 (PPC2_BOOKE206 | PPC2_VSX | PPC2_PRCNTL | PPC2_DBRX | \
> >> PPC2_ISA205 | PPC2_VSX207 | PPC2_PERM_ISA206 | \
> >> PPC2_DIVE_ISA206 | PPC2_ATOMIC_ISA206 | \
> >> @@ -2213,7 +2216,7 @@ enum {
> >> PPC2_BCTAR_ISA207 | PPC2_LSQ_ISA207 | \
> >> PPC2_ALTIVEC_207 | PPC2_ISA207S | PPC2_DFP | \
> >> PPC2_FP_CVT_S64 | PPC2_TM | PPC2_PM_ISA206 | \
> >> - PPC2_ISA300)
> >> + PPC2_ISA300 | PPC2_MEM_EIEIO2)
> >> };
> >>
> >> /*****************************************************************************/
> >> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> >> index 8ba8f67dc513..a73ef02aef1d 100644
> >> --- a/target/ppc/translate.c
> >> +++ b/target/ppc/translate.c
> >> @@ -2967,7 +2967,22 @@ static void gen_stswx(DisasContext *ctx)
> >> /* eieio */
> >> static void gen_eieio(DisasContext *ctx)
> >> {
> >> - tcg_gen_mb(TCG_MO_LD_ST | TCG_BAR_SC);
> >> + TCGBar bar = TCG_MO_LD_ST;
> >> +
> >> + /*
> >> + * POWER9 has a eieio instruction variant using bit 6 as a hint to
> >> + * tell the CPU it is a store-forwarding barrier.
> >> + */
> >> + if (ctx->opcode & 0x2000000) {
> >> + if (!(ctx->insns_flags2 & PPC2_MEM_EIEIO2)) {
> >
> > Since we have to adapt the gen_eieio code anyway we don't really need
> > the new instruction flag - we can just reuse PPC2_ISA300 for it.
>
> OK. That seems now a little superfluous, indeed. Unless there are more
> or these instructions which depend on CPU revision. I don't know so
> let's stick to PPC2_ISA300.
>
> >
> >> + qemu_log_mask(LOG_GUEST_ERROR, "invalid eieio using bit 6 at @"
> >> + TARGET_FMT_lx "\n", ctx->base.pc_next - 4);
> >
> > Logging is ok, but it's not enough in this context - that will turn
> > the variant eieio into a no-op. You need to actually generate an
> > invalid instruction exception here, which is what I assume will happen
> > if you try to execute the new variant on a POWER8:
> > gen_inval_exception(ctx, POWERPC_EXCP_INVAL_INVAL);
>
> well, I didn't try but ISA says that :
>
> "Reserved fields in instructions are ignored by the processor."
>
> So it should be a no-op.
Ah, ok. So, no 0x700, but.. that doesn't mean a no-op, that means it
should act as a normal eieio.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
© 2016 - 2024 Red Hat, Inc.