target/ppc: Fix msync to do what hardware does

Message ID 20190126150102.7AABC74569F@zero.eik.bme.hu
State New
Headers show
Series
  • target/ppc: Fix msync to do what hardware does
Related show

Commit Message

BALATON Zoltan Jan. 26, 2019, 2:50 p.m.
According to BookE docs, invalid bits (while undefined behaviour) should
not raise exception but be ignored. This seems to be implementation
dependent though and QEMU currently does what e500 CPUs do and raise
exception for invalid bits. Unfortunately some versions of libstdc++
(and so all programs compiled with it) have lwsync on PPC440 which is
invalid but on real hardware it's just executed as msync ignoring the
invalid bits (maybe that's why it got undetected) but they fail on QEMU.
This patch changes invalid mask of msync to allow these programs to run
but keep generating exception on e500 cores to follow what hardware does.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
 target/ppc/translate.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

Comments

BALATON Zoltan Feb. 1, 2019, 1:03 p.m. | #1
On Sat, 26 Jan 2019, BALATON Zoltan wrote:
> According to BookE docs, invalid bits (while undefined behaviour) should
> not raise exception but be ignored. This seems to be implementation
> dependent though and QEMU currently does what e500 CPUs do and raise
> exception for invalid bits. Unfortunately some versions of libstdc++
> (and so all programs compiled with it) have lwsync on PPC440 which is
> invalid but on real hardware it's just executed as msync ignoring the
> invalid bits (maybe that's why it got undetected) but they fail on QEMU.
> This patch changes invalid mask of msync to allow these programs to run
> but keep generating exception on e500 cores to follow what hardware does.
>
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>

Ping? This seems to have been ignored so far.

> ---
> target/ppc/translate.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index e169c43643..5429ceb1ab 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -6476,7 +6476,12 @@ static void gen_mbar(DisasContext *ctx)
> /* msync replaces sync on 440 */
> static void gen_msync_4xx(DisasContext *ctx)
> {
> -    /* interpreted as no-op */
> +    /* Only e500 seems to treat reserved bits as invalid */
> +    if ((ctx->insns_flags2 & PPC2_BOOKE206) &&
> +        (ctx->opcode & 0x03FFF801)) {
> +        gen_inval_exception(ctx, POWERPC_EXCP_INVAL_INVAL);
> +    }
> +    /* otherwise interpreted as no-op */
> }
>
> /* icbt */
> @@ -7054,11 +7059,11 @@ GEN_HANDLER(wrteei, 0x1F, 0x03, 0x05, 0x000E7C01, PPC_WRTEE),
> GEN_HANDLER(dlmzb, 0x1F, 0x0E, 0x02, 0x00000000, PPC_440_SPEC),
> GEN_HANDLER_E(mbar, 0x1F, 0x16, 0x1a, 0x001FF801,
>               PPC_BOOKE, PPC2_BOOKE206),
> -GEN_HANDLER(msync_4xx, 0x1F, 0x16, 0x12, 0x03FFF801, PPC_BOOKE),
> +GEN_HANDLER(msync_4xx, 0x1F, 0x16, 0x12, 0x039FF801, PPC_BOOKE),
> GEN_HANDLER2_E(icbt_440, "icbt", 0x1F, 0x16, 0x00, 0x03E00001,
>                PPC_BOOKE, PPC2_BOOKE206),
> GEN_HANDLER2(icbt_440, "icbt", 0x1F, 0x06, 0x08, 0x03E00001,
> -               PPC_440_SPEC),
> +             PPC_440_SPEC),
> GEN_HANDLER(lvsl, 0x1f, 0x06, 0x00, 0x00000001, PPC_ALTIVEC),
> GEN_HANDLER(lvsr, 0x1f, 0x06, 0x01, 0x00000001, PPC_ALTIVEC),
> GEN_HANDLER(mfvscr, 0x04, 0x2, 0x18, 0x001ff800, PPC_ALTIVEC),
>
David Gibson Feb. 8, 2019, 5:34 a.m. | #2
On Sat, Jan 26, 2019 at 03:50:28PM +0100, BALATON Zoltan wrote:
> According to BookE docs, invalid bits (while undefined behaviour) should
> not raise exception but be ignored. This seems to be implementation
> dependent though and QEMU currently does what e500 CPUs do and raise
> exception for invalid bits. Unfortunately some versions of libstdc++
> (and so all programs compiled with it) have lwsync on PPC440 which is
> invalid but on real hardware it's just executed as msync ignoring the
> invalid bits (maybe that's why it got undetected) but they fail on QEMU.
> This patch changes invalid mask of msync to allow these programs to run
> but keep generating exception on e500 cores to follow what hardware does.
> 
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>

Applied to ppc-for-4.0, thanks.

> ---
>  target/ppc/translate.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index e169c43643..5429ceb1ab 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -6476,7 +6476,12 @@ static void gen_mbar(DisasContext *ctx)
>  /* msync replaces sync on 440 */
>  static void gen_msync_4xx(DisasContext *ctx)
>  {
> -    /* interpreted as no-op */
> +    /* Only e500 seems to treat reserved bits as invalid */
> +    if ((ctx->insns_flags2 & PPC2_BOOKE206) &&
> +        (ctx->opcode & 0x03FFF801)) {
> +        gen_inval_exception(ctx, POWERPC_EXCP_INVAL_INVAL);
> +    }
> +    /* otherwise interpreted as no-op */
>  }
>  
>  /* icbt */
> @@ -7054,11 +7059,11 @@ GEN_HANDLER(wrteei, 0x1F, 0x03, 0x05, 0x000E7C01, PPC_WRTEE),
>  GEN_HANDLER(dlmzb, 0x1F, 0x0E, 0x02, 0x00000000, PPC_440_SPEC),
>  GEN_HANDLER_E(mbar, 0x1F, 0x16, 0x1a, 0x001FF801,
>                PPC_BOOKE, PPC2_BOOKE206),
> -GEN_HANDLER(msync_4xx, 0x1F, 0x16, 0x12, 0x03FFF801, PPC_BOOKE),
> +GEN_HANDLER(msync_4xx, 0x1F, 0x16, 0x12, 0x039FF801, PPC_BOOKE),
>  GEN_HANDLER2_E(icbt_440, "icbt", 0x1F, 0x16, 0x00, 0x03E00001,
>                 PPC_BOOKE, PPC2_BOOKE206),
>  GEN_HANDLER2(icbt_440, "icbt", 0x1F, 0x06, 0x08, 0x03E00001,
> -               PPC_440_SPEC),
> +             PPC_440_SPEC),
>  GEN_HANDLER(lvsl, 0x1f, 0x06, 0x00, 0x00000001, PPC_ALTIVEC),
>  GEN_HANDLER(lvsr, 0x1f, 0x06, 0x01, 0x00000001, PPC_ALTIVEC),
>  GEN_HANDLER(mfvscr, 0x04, 0x2, 0x18, 0x001ff800, PPC_ALTIVEC),

Patch

diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index e169c43643..5429ceb1ab 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -6476,7 +6476,12 @@  static void gen_mbar(DisasContext *ctx)
 /* msync replaces sync on 440 */
 static void gen_msync_4xx(DisasContext *ctx)
 {
-    /* interpreted as no-op */
+    /* Only e500 seems to treat reserved bits as invalid */
+    if ((ctx->insns_flags2 & PPC2_BOOKE206) &&
+        (ctx->opcode & 0x03FFF801)) {
+        gen_inval_exception(ctx, POWERPC_EXCP_INVAL_INVAL);
+    }
+    /* otherwise interpreted as no-op */
 }
 
 /* icbt */
@@ -7054,11 +7059,11 @@  GEN_HANDLER(wrteei, 0x1F, 0x03, 0x05, 0x000E7C01, PPC_WRTEE),
 GEN_HANDLER(dlmzb, 0x1F, 0x0E, 0x02, 0x00000000, PPC_440_SPEC),
 GEN_HANDLER_E(mbar, 0x1F, 0x16, 0x1a, 0x001FF801,
               PPC_BOOKE, PPC2_BOOKE206),
-GEN_HANDLER(msync_4xx, 0x1F, 0x16, 0x12, 0x03FFF801, PPC_BOOKE),
+GEN_HANDLER(msync_4xx, 0x1F, 0x16, 0x12, 0x039FF801, PPC_BOOKE),
 GEN_HANDLER2_E(icbt_440, "icbt", 0x1F, 0x16, 0x00, 0x03E00001,
                PPC_BOOKE, PPC2_BOOKE206),
 GEN_HANDLER2(icbt_440, "icbt", 0x1F, 0x06, 0x08, 0x03E00001,
-               PPC_440_SPEC),
+             PPC_440_SPEC),
 GEN_HANDLER(lvsl, 0x1f, 0x06, 0x00, 0x00000001, PPC_ALTIVEC),
 GEN_HANDLER(lvsr, 0x1f, 0x06, 0x01, 0x00000001, PPC_ALTIVEC),
 GEN_HANDLER(mfvscr, 0x04, 0x2, 0x18, 0x001ff800, PPC_ALTIVEC),