[PATCH 0/6] Fix gen_*_exception error codes

[PATCH 0/6] Fix gen_*_exception error codes

[Matheus Ferst]

The first patch of this series is the RFC of [1] (hence the r-b in v1).
Patches 2~4 follow the other problems that Laurent pointed out, and
patches 5-6 fix similar problems that I found.

[1] https://lists.gnu.org/archive/html/qemu-ppc/2022-01/msg00400.html

Matheus Ferst (6):
  target/ppc: Fix gen_priv_exception error value in mfspr/mtspr
  target/ppc: fix exception error value in slbfee
  target/ppc: remove mfdcrux and mtdcrux
  target/ppc: fix exception error code in helper_{load,store}_dcr
  target/ppc: fix PMU Group A register read/write exceptions
  target/ppc: fix exception error code in spr_write_excp_vector

 target/ppc/cpu.h                 |  6 ++----
 target/ppc/helper.h              |  2 +-
 target/ppc/power8-pmu-regs.c.inc | 10 ++++-----
 target/ppc/timebase_helper.c     |  6 +++---
 target/ppc/translate.c           | 36 ++++++++------------------------
 5 files changed, 20 insertions(+), 40 deletions(-)

-- 
2.25.1

[PATCH 1/6] target/ppc: Fix gen_priv_exception error value in mfspr/mtspr

[Matheus Ferst]

The code in linux-user/ppc/cpu_loop.c expects POWERPC_EXCP_PRIV
exception with error POWERPC_EXCP_PRIV_OPC or POWERPC_EXCP_PRIV_REG,
while POWERPC_EXCP_INVAL_SPR is expected in POWERPC_EXCP_INVAL
exceptions. This mismatch caused an EXCP_DUMP with the message "Unknown
privilege violation (03)", as seen in [1].

[1] https://gitlab.com/qemu-project/qemu/-/issues/588

Fixes: 9b2fadda3e01 ("ppc: Rework generation of priv and inval interrupts")
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/588
Reviewed-by: Fabiano Rosas <farosas@linux.ibm.com>
Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
---
This patch was split from
https://lists.gnu.org/archive/html/qemu-ppc/2022-01/msg00400.html
---
 target/ppc/translate.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 1d6daa4608..55f34eb490 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -4789,11 +4789,11 @@ static inline void gen_op_mfspr(DisasContext *ctx)
          */
         if (sprn & 0x10) {
             if (ctx->pr) {
-                gen_priv_exception(ctx, POWERPC_EXCP_INVAL_SPR);
+                gen_priv_exception(ctx, POWERPC_EXCP_PRIV_REG);
             }
         } else {
             if (ctx->pr || sprn == 0 || sprn == 4 || sprn == 5 || sprn == 6) {
-                gen_hvpriv_exception(ctx, POWERPC_EXCP_INVAL_SPR);
+                gen_hvpriv_exception(ctx, POWERPC_EXCP_PRIV_REG);
             }
         }
     }
@@ -4976,11 +4976,11 @@ static void gen_mtspr(DisasContext *ctx)
          */
         if (sprn & 0x10) {
             if (ctx->pr) {
-                gen_priv_exception(ctx, POWERPC_EXCP_INVAL_SPR);
+                gen_priv_exception(ctx, POWERPC_EXCP_PRIV_REG);
             }
         } else {
             if (ctx->pr || sprn == 0) {
-                gen_hvpriv_exception(ctx, POWERPC_EXCP_INVAL_SPR);
+                gen_hvpriv_exception(ctx, POWERPC_EXCP_PRIV_REG);
             }
         }
     }
-- 
2.25.1

[PATCH 2/6] target/ppc: fix exception error value in slbfee

[Matheus Ferst]

Testing on a POWER9 DD2.3, we observed that the Linux kernel delivers a
signal with si_code ILL_PRVOPC (5) when a userspace application tries to
use slbfee. To obtain this behavior on linux-user, we should use
POWERPC_EXCP_PRIV with POWERPC_EXCP_PRIV_OPC.

No functional change is intended for softmmu targets as
gen_hvpriv_exception uses the same 'exception' argument
(POWERPC_EXCP_HV_EMU) for raise_exception_*, and the powerpc_excp_*
methods do not use lower bits of the exception error code when handling
POWERPC_EXCP_{INVAL,PRIV}.

Reported-by: Laurent Vivier <laurent@vivier.eu>
Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
---
 target/ppc/translate.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 55f34eb490..d7e5670c20 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -5386,12 +5386,12 @@ static void gen_slbmfev(DisasContext *ctx)
 static void gen_slbfee_(DisasContext *ctx)
 {
 #if defined(CONFIG_USER_ONLY)
-    gen_inval_exception(ctx, POWERPC_EXCP_PRIV_REG);
+    gen_hvpriv_exception(ctx, POWERPC_EXCP_PRIV_OPC);
 #else
     TCGLabel *l1, *l2;
 
     if (unlikely(ctx->pr)) {
-        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_REG);
+        gen_hvpriv_exception(ctx, POWERPC_EXCP_PRIV_OPC);
         return;
     }
     gen_helper_find_slb_vsid(cpu_gpr[rS(ctx->opcode)], cpu_env,
-- 
2.25.1

Re: [PATCH 2/6] target/ppc: fix exception error value in slbfee

[Daniel Henrique Barboza]

On 6/27/22 11:11, Matheus Ferst wrote:
> Testing on a POWER9 DD2.3, we observed that the Linux kernel delivers a
> signal with si_code ILL_PRVOPC (5) when a userspace application tries to
> use slbfee. To obtain this behavior on linux-user, we should use
> POWERPC_EXCP_PRIV with POWERPC_EXCP_PRIV_OPC.
> 
> No functional change is intended for softmmu targets as
> gen_hvpriv_exception uses the same 'exception' argument
> (POWERPC_EXCP_HV_EMU) for raise_exception_*, and the powerpc_excp_*
> methods do not use lower bits of the exception error code when handling
> POWERPC_EXCP_{INVAL,PRIV}.
> 
> Reported-by: Laurent Vivier <laurent@vivier.eu>
> Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
> ---

Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>

>   target/ppc/translate.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index 55f34eb490..d7e5670c20 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -5386,12 +5386,12 @@ static void gen_slbmfev(DisasContext *ctx)
>   static void gen_slbfee_(DisasContext *ctx)
>   {
>   #if defined(CONFIG_USER_ONLY)
> -    gen_inval_exception(ctx, POWERPC_EXCP_PRIV_REG);
> +    gen_hvpriv_exception(ctx, POWERPC_EXCP_PRIV_OPC);
>   #else
>       TCGLabel *l1, *l2;
>   
>       if (unlikely(ctx->pr)) {
> -        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_REG);
> +        gen_hvpriv_exception(ctx, POWERPC_EXCP_PRIV_OPC);
>           return;
>       }
>       gen_helper_find_slb_vsid(cpu_gpr[rS(ctx->opcode)], cpu_env,

[PATCH 3/6] target/ppc: remove mfdcrux and mtdcrux

[Matheus Ferst]

The only PowerPC implementations with these insns were the 460 and 460F,
which had their definitions removed in [1].

[1] 7ff26aa6c657 ("target/ppc: Remove unused PPC 460 and 460F definitions")

Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
---
 target/ppc/cpu.h       |  6 ++----
 target/ppc/translate.c | 18 ------------------
 2 files changed, 2 insertions(+), 22 deletions(-)

diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 6d78078f37..80664446e6 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -2203,8 +2203,6 @@ enum {
     PPC_DCR            = 0x1000000000000000ULL,
     /* DCR extended accesse                                                  */
     PPC_DCRX           = 0x2000000000000000ULL,
-    /* user-mode DCR access, implemented in PowerPC 460                      */
-    PPC_DCRUX          = 0x4000000000000000ULL,
     /* popcntw and popcntd instructions                                      */
     PPC_POPCNTWD       = 0x8000000000000000ULL,
 
@@ -2228,8 +2226,8 @@ enum {
                         | PPC_405_MAC | PPC_440_SPEC | PPC_BOOKE \
                         | PPC_MFAPIDI | PPC_TLBIVA | PPC_TLBIVAX \
                         | PPC_4xx_COMMON | PPC_40x_ICBT | PPC_RFMCI \
-                        | PPC_RFDI | PPC_DCR | PPC_DCRX | PPC_DCRUX \
-                        | PPC_POPCNTWD | PPC_CILDST)
+                        | PPC_RFDI | PPC_DCR | PPC_DCRX | PPC_POPCNTWD \
+                        | PPC_CILDST)
 
     /* extended type values */
 
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index d7e5670c20..30dd524959 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -5907,22 +5907,6 @@ static void gen_mtdcrx(DisasContext *ctx)
 #endif /* defined(CONFIG_USER_ONLY) */
 }
 
-/* mfdcrux (PPC 460) : user-mode access to DCR */
-static void gen_mfdcrux(DisasContext *ctx)
-{
-    gen_helper_load_dcr(cpu_gpr[rD(ctx->opcode)], cpu_env,
-                        cpu_gpr[rA(ctx->opcode)]);
-    /* Note: Rc update flag set leads to undefined state of Rc0 */
-}
-
-/* mtdcrux (PPC 460) : user-mode access to DCR */
-static void gen_mtdcrux(DisasContext *ctx)
-{
-    gen_helper_store_dcr(cpu_env, cpu_gpr[rA(ctx->opcode)],
-                         cpu_gpr[rS(ctx->opcode)]);
-    /* Note: Rc update flag set leads to undefined state of Rc0 */
-}
-
 /* dccci */
 static void gen_dccci(DisasContext *ctx)
 {
@@ -6958,8 +6942,6 @@ GEN_HANDLER(mfdcr, 0x1F, 0x03, 0x0A, 0x00000001, PPC_DCR),
 GEN_HANDLER(mtdcr, 0x1F, 0x03, 0x0E, 0x00000001, PPC_DCR),
 GEN_HANDLER(mfdcrx, 0x1F, 0x03, 0x08, 0x00000000, PPC_DCRX),
 GEN_HANDLER(mtdcrx, 0x1F, 0x03, 0x0C, 0x00000000, PPC_DCRX),
-GEN_HANDLER(mfdcrux, 0x1F, 0x03, 0x09, 0x00000000, PPC_DCRUX),
-GEN_HANDLER(mtdcrux, 0x1F, 0x03, 0x0D, 0x00000000, PPC_DCRUX),
 GEN_HANDLER(dccci, 0x1F, 0x06, 0x0E, 0x03E00001, PPC_4xx_COMMON),
 GEN_HANDLER(dcread, 0x1F, 0x06, 0x0F, 0x00000001, PPC_4xx_COMMON),
 GEN_HANDLER2(icbt_40x, "icbt", 0x1F, 0x06, 0x08, 0x03E00001, PPC_40x_ICBT),
-- 
2.25.1

Re: [PATCH 3/6] target/ppc: remove mfdcrux and mtdcrux

[Fabiano Rosas]

Matheus Ferst <matheus.ferst@eldorado.org.br> writes:

> The only PowerPC implementations with these insns were the 460 and 460F,
> which had their definitions removed in [1].
>
> [1] 7ff26aa6c657 ("target/ppc: Remove unused PPC 460 and 460F definitions")
>
> Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>

Reviewed-by: Fabiano Rosas <farosas@linux.ibm.com>

> ---
>  target/ppc/cpu.h       |  6 ++----
>  target/ppc/translate.c | 18 ------------------
>  2 files changed, 2 insertions(+), 22 deletions(-)
>
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index 6d78078f37..80664446e6 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -2203,8 +2203,6 @@ enum {
>      PPC_DCR            = 0x1000000000000000ULL,
>      /* DCR extended accesse                                                  */
>      PPC_DCRX           = 0x2000000000000000ULL,
> -    /* user-mode DCR access, implemented in PowerPC 460                      */
> -    PPC_DCRUX          = 0x4000000000000000ULL,
>      /* popcntw and popcntd instructions                                      */
>      PPC_POPCNTWD       = 0x8000000000000000ULL,
>  
> @@ -2228,8 +2226,8 @@ enum {
>                          | PPC_405_MAC | PPC_440_SPEC | PPC_BOOKE \
>                          | PPC_MFAPIDI | PPC_TLBIVA | PPC_TLBIVAX \
>                          | PPC_4xx_COMMON | PPC_40x_ICBT | PPC_RFMCI \
> -                        | PPC_RFDI | PPC_DCR | PPC_DCRX | PPC_DCRUX \
> -                        | PPC_POPCNTWD | PPC_CILDST)
> +                        | PPC_RFDI | PPC_DCR | PPC_DCRX | PPC_POPCNTWD \
> +                        | PPC_CILDST)
>  
>      /* extended type values */
>  
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index d7e5670c20..30dd524959 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -5907,22 +5907,6 @@ static void gen_mtdcrx(DisasContext *ctx)
>  #endif /* defined(CONFIG_USER_ONLY) */
>  }
>  
> -/* mfdcrux (PPC 460) : user-mode access to DCR */
> -static void gen_mfdcrux(DisasContext *ctx)
> -{
> -    gen_helper_load_dcr(cpu_gpr[rD(ctx->opcode)], cpu_env,
> -                        cpu_gpr[rA(ctx->opcode)]);
> -    /* Note: Rc update flag set leads to undefined state of Rc0 */
> -}
> -
> -/* mtdcrux (PPC 460) : user-mode access to DCR */
> -static void gen_mtdcrux(DisasContext *ctx)
> -{
> -    gen_helper_store_dcr(cpu_env, cpu_gpr[rA(ctx->opcode)],
> -                         cpu_gpr[rS(ctx->opcode)]);
> -    /* Note: Rc update flag set leads to undefined state of Rc0 */
> -}
> -
>  /* dccci */
>  static void gen_dccci(DisasContext *ctx)
>  {
> @@ -6958,8 +6942,6 @@ GEN_HANDLER(mfdcr, 0x1F, 0x03, 0x0A, 0x00000001, PPC_DCR),
>  GEN_HANDLER(mtdcr, 0x1F, 0x03, 0x0E, 0x00000001, PPC_DCR),
>  GEN_HANDLER(mfdcrx, 0x1F, 0x03, 0x08, 0x00000000, PPC_DCRX),
>  GEN_HANDLER(mtdcrx, 0x1F, 0x03, 0x0C, 0x00000000, PPC_DCRX),
> -GEN_HANDLER(mfdcrux, 0x1F, 0x03, 0x09, 0x00000000, PPC_DCRUX),
> -GEN_HANDLER(mtdcrux, 0x1F, 0x03, 0x0D, 0x00000000, PPC_DCRUX),
>  GEN_HANDLER(dccci, 0x1F, 0x06, 0x0E, 0x03E00001, PPC_4xx_COMMON),
>  GEN_HANDLER(dcread, 0x1F, 0x06, 0x0F, 0x00000001, PPC_4xx_COMMON),
>  GEN_HANDLER2(icbt_40x, "icbt", 0x1F, 0x06, 0x08, 0x03E00001, PPC_40x_ICBT),

[PATCH 4/6] target/ppc: fix exception error code in helper_{load, store}_dcr

[Matheus Ferst]

POWERPC_EXCP_INVAL should only be or-ed with other constants prefixed
with POWERPC_EXCP_INVAL_. Also, take the opportunity to move both
helpers under #if !defined(CONFIG_USER_ONLY) as the instructions that
use them are privileged.

No functional change is intended, the lower 4 bits of the error code are
ignored by all powerpc_excp_* methods on POWERPC_EXCP_INVAL exceptions.

Reported-by: Laurent Vivier <laurent@vivier.eu>
Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
---
 target/ppc/helper.h          | 2 +-
 target/ppc/timebase_helper.c | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/target/ppc/helper.h b/target/ppc/helper.h
index 6233e28d85..c6895f2f99 100644
--- a/target/ppc/helper.h
+++ b/target/ppc/helper.h
@@ -684,10 +684,10 @@ DEF_HELPER_2(book3s_msgclr, void, env, tl)
 DEF_HELPER_4(dlmzb, tl, env, tl, tl, i32)
 #if !defined(CONFIG_USER_ONLY)
 DEF_HELPER_2(rac, tl, env, tl)
-#endif
 
 DEF_HELPER_2(load_dcr, tl, env, tl)
 DEF_HELPER_3(store_dcr, void, env, tl, tl)
+#endif
 
 DEF_HELPER_2(load_dump_spr, void, env, i32)
 DEF_HELPER_2(store_dump_spr, void, env, i32)
diff --git a/target/ppc/timebase_helper.c b/target/ppc/timebase_helper.c
index 86d01d6e4e..b80f56af7e 100644
--- a/target/ppc/timebase_helper.c
+++ b/target/ppc/timebase_helper.c
@@ -143,7 +143,6 @@ void helper_store_booke_tsr(CPUPPCState *env, target_ulong val)
 {
     store_booke_tsr(env, val);
 }
-#endif
 
 /*****************************************************************************/
 /* Embedded PowerPC specific helpers */
@@ -169,7 +168,7 @@ target_ulong helper_load_dcr(CPUPPCState *env, target_ulong dcrn)
                           (uint32_t)dcrn, (uint32_t)dcrn);
             raise_exception_err_ra(env, POWERPC_EXCP_PROGRAM,
                                    POWERPC_EXCP_INVAL |
-                                   POWERPC_EXCP_PRIV_REG, GETPC());
+                                   POWERPC_EXCP_INVAL_INVAL, GETPC());
         }
     }
     return val;
@@ -192,7 +191,8 @@ void helper_store_dcr(CPUPPCState *env, target_ulong dcrn, target_ulong val)
                           (uint32_t)dcrn, (uint32_t)dcrn);
             raise_exception_err_ra(env, POWERPC_EXCP_PROGRAM,
                                    POWERPC_EXCP_INVAL |
-                                   POWERPC_EXCP_PRIV_REG, GETPC());
+                                   POWERPC_EXCP_INVAL_INVAL, GETPC());
         }
     }
 }
+#endif
-- 
2.25.1

Re: [PATCH 4/6] target/ppc: fix exception error code in helper_{load,store}_dcr

[Daniel Henrique Barboza]

On 6/27/22 11:11, Matheus Ferst wrote:
> POWERPC_EXCP_INVAL should only be or-ed with other constants prefixed
> with POWERPC_EXCP_INVAL_. Also, take the opportunity to move both
> helpers under #if !defined(CONFIG_USER_ONLY) as the instructions that
> use them are privileged.
> 
> No functional change is intended, the lower 4 bits of the error code are
> ignored by all powerpc_excp_* methods on POWERPC_EXCP_INVAL exceptions.
> 
> Reported-by: Laurent Vivier <laurent@vivier.eu>
> Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
> ---
>   target/ppc/helper.h          | 2 +-
>   target/ppc/timebase_helper.c | 6 +++---
>   2 files changed, 4 insertions(+), 4 deletions(-)

Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>

> 
> diff --git a/target/ppc/helper.h b/target/ppc/helper.h
> index 6233e28d85..c6895f2f99 100644
> --- a/target/ppc/helper.h
> +++ b/target/ppc/helper.h
> @@ -684,10 +684,10 @@ DEF_HELPER_2(book3s_msgclr, void, env, tl)
>   DEF_HELPER_4(dlmzb, tl, env, tl, tl, i32)
>   #if !defined(CONFIG_USER_ONLY)
>   DEF_HELPER_2(rac, tl, env, tl)
> -#endif
>   
>   DEF_HELPER_2(load_dcr, tl, env, tl)
>   DEF_HELPER_3(store_dcr, void, env, tl, tl)
> +#endif
>   
>   DEF_HELPER_2(load_dump_spr, void, env, i32)
>   DEF_HELPER_2(store_dump_spr, void, env, i32)
> diff --git a/target/ppc/timebase_helper.c b/target/ppc/timebase_helper.c
> index 86d01d6e4e..b80f56af7e 100644
> --- a/target/ppc/timebase_helper.c
> +++ b/target/ppc/timebase_helper.c
> @@ -143,7 +143,6 @@ void helper_store_booke_tsr(CPUPPCState *env, target_ulong val)
>   {
>       store_booke_tsr(env, val);
>   }
> -#endif
>   
>   /*****************************************************************************/
>   /* Embedded PowerPC specific helpers */
> @@ -169,7 +168,7 @@ target_ulong helper_load_dcr(CPUPPCState *env, target_ulong dcrn)
>                             (uint32_t)dcrn, (uint32_t)dcrn);
>               raise_exception_err_ra(env, POWERPC_EXCP_PROGRAM,
>                                      POWERPC_EXCP_INVAL |
> -                                   POWERPC_EXCP_PRIV_REG, GETPC());
> +                                   POWERPC_EXCP_INVAL_INVAL, GETPC());
>           }
>       }
>       return val;
> @@ -192,7 +191,8 @@ void helper_store_dcr(CPUPPCState *env, target_ulong dcrn, target_ulong val)
>                             (uint32_t)dcrn, (uint32_t)dcrn);
>               raise_exception_err_ra(env, POWERPC_EXCP_PROGRAM,
>                                      POWERPC_EXCP_INVAL |
> -                                   POWERPC_EXCP_PRIV_REG, GETPC());
> +                                   POWERPC_EXCP_INVAL_INVAL, GETPC());
>           }
>       }
>   }
> +#endif

[PATCH 5/6] target/ppc: fix PMU Group A register read/write exceptions

[Matheus Ferst]

A call to "gen_(hv)priv_exception" should use POWERPC_EXCP_PRIV_* as the
'error' argument instead of POWERPC_EXCP_INVAL_*, and POWERPC_EXCP_FU is
an exception type, not an exception error code. To correctly set
FSCR[IC], we should raise Facility Unavailable with this exception type
and IC value as the error code.

Fixes: 565cb1096733 ("target/ppc: add user read/write functions for MMCR0")
Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
---
 target/ppc/power8-pmu-regs.c.inc | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/target/ppc/power8-pmu-regs.c.inc b/target/ppc/power8-pmu-regs.c.inc
index 2bab6cece7..c3cc919ee4 100644
--- a/target/ppc/power8-pmu-regs.c.inc
+++ b/target/ppc/power8-pmu-regs.c.inc
@@ -22,7 +22,7 @@
 static bool spr_groupA_read_allowed(DisasContext *ctx)
 {
     if (!ctx->mmcr0_pmcc0 && ctx->mmcr0_pmcc1) {
-        gen_hvpriv_exception(ctx, POWERPC_EXCP_FU);
+        gen_exception_err(ctx, POWERPC_EXCP_FU, FSCR_IC_PMU);
         return false;
     }
 
@@ -46,10 +46,10 @@ static bool spr_groupA_write_allowed(DisasContext *ctx)
 
     if (ctx->mmcr0_pmcc1) {
         /* PMCC = 0b01 */
-        gen_hvpriv_exception(ctx, POWERPC_EXCP_FU);
+        gen_exception_err(ctx, POWERPC_EXCP_FU, FSCR_IC_PMU);
     } else {
         /* PMCC = 0b00 */
-        gen_hvpriv_exception(ctx, POWERPC_EXCP_INVAL_SPR);
+        gen_hvpriv_exception(ctx, POWERPC_EXCP_PRIV_REG);
     }
 
     return false;
@@ -214,7 +214,7 @@ void spr_read_PMC56_ureg(DisasContext *ctx, int gprn, int sprn)
      * Interrupt.
      */
     if (ctx->mmcr0_pmcc0 && ctx->mmcr0_pmcc1) {
-        gen_hvpriv_exception(ctx, POWERPC_EXCP_FU);
+        gen_exception_err(ctx, POWERPC_EXCP_FU, FSCR_IC_PMU);
         return;
     }
 
@@ -249,7 +249,7 @@ void spr_write_PMC56_ureg(DisasContext *ctx, int sprn, int gprn)
      * Interrupt.
      */
     if (ctx->mmcr0_pmcc0 && ctx->mmcr0_pmcc1) {
-        gen_hvpriv_exception(ctx, POWERPC_EXCP_FU);
+        gen_exception_err(ctx, POWERPC_EXCP_FU, FSCR_IC_PMU);
         return;
     }
 
-- 
2.25.1

Re: [PATCH 5/6] target/ppc: fix PMU Group A register read/write exceptions

[Daniel Henrique Barboza]

On 6/27/22 11:11, Matheus Ferst wrote:
> A call to "gen_(hv)priv_exception" should use POWERPC_EXCP_PRIV_* as the
> 'error' argument instead of POWERPC_EXCP_INVAL_*, and POWERPC_EXCP_FU is
> an exception type, not an exception error code. To correctly set
> FSCR[IC], we should raise Facility Unavailable with this exception type
> and IC value as the error code.
> 
> Fixes: 565cb1096733 ("target/ppc: add user read/write functions for MMCR0")
> Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
> ---

Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>

>   target/ppc/power8-pmu-regs.c.inc | 10 +++++-----
>   1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/target/ppc/power8-pmu-regs.c.inc b/target/ppc/power8-pmu-regs.c.inc
> index 2bab6cece7..c3cc919ee4 100644
> --- a/target/ppc/power8-pmu-regs.c.inc
> +++ b/target/ppc/power8-pmu-regs.c.inc
> @@ -22,7 +22,7 @@
>   static bool spr_groupA_read_allowed(DisasContext *ctx)
>   {
>       if (!ctx->mmcr0_pmcc0 && ctx->mmcr0_pmcc1) {
> -        gen_hvpriv_exception(ctx, POWERPC_EXCP_FU);
> +        gen_exception_err(ctx, POWERPC_EXCP_FU, FSCR_IC_PMU);
>           return false;
>       }
>   
> @@ -46,10 +46,10 @@ static bool spr_groupA_write_allowed(DisasContext *ctx)
>   
>       if (ctx->mmcr0_pmcc1) {
>           /* PMCC = 0b01 */
> -        gen_hvpriv_exception(ctx, POWERPC_EXCP_FU);
> +        gen_exception_err(ctx, POWERPC_EXCP_FU, FSCR_IC_PMU);
>       } else {
>           /* PMCC = 0b00 */
> -        gen_hvpriv_exception(ctx, POWERPC_EXCP_INVAL_SPR);
> +        gen_hvpriv_exception(ctx, POWERPC_EXCP_PRIV_REG);
>       }
>   
>       return false;
> @@ -214,7 +214,7 @@ void spr_read_PMC56_ureg(DisasContext *ctx, int gprn, int sprn)
>        * Interrupt.
>        */
>       if (ctx->mmcr0_pmcc0 && ctx->mmcr0_pmcc1) {
> -        gen_hvpriv_exception(ctx, POWERPC_EXCP_FU);
> +        gen_exception_err(ctx, POWERPC_EXCP_FU, FSCR_IC_PMU);
>           return;
>       }
>   
> @@ -249,7 +249,7 @@ void spr_write_PMC56_ureg(DisasContext *ctx, int sprn, int gprn)
>        * Interrupt.
>        */
>       if (ctx->mmcr0_pmcc0 && ctx->mmcr0_pmcc1) {
> -        gen_hvpriv_exception(ctx, POWERPC_EXCP_FU);
> +        gen_exception_err(ctx, POWERPC_EXCP_FU, FSCR_IC_PMU);
>           return;
>       }
>   

[PATCH 6/6] target/ppc: fix exception error code in spr_write_excp_vector

[Matheus Ferst]

The 'error' argument of gen_inval_exception will be or-ed with
POWERPC_EXCP_INVAL, so it should always be a constant prefixed with
POWERPC_EXCP_INVAL_. No functional change is intended,
spr_write_excp_vector is only used by register_BookE_sprs, and
powerpc_excp_booke ignores the lower 4 bits of the error code on
POWERPC_EXCP_INVAL exceptions.

Also, take the opportunity to replace printf with qemu_log_mask.

Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
---
 target/ppc/translate.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 30dd524959..da11472877 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -907,9 +907,9 @@ void spr_write_excp_vector(DisasContext *ctx, int sprn, int gprn)
     } else if (sprn >= SPR_BOOKE_IVOR38 && sprn <= SPR_BOOKE_IVOR42) {
         sprn_offs = sprn - SPR_BOOKE_IVOR38 + 38;
     } else {
-        printf("Trying to write an unknown exception vector %d %03x\n",
-               sprn, sprn);
-        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_REG);
+        qemu_log_mask(LOG_GUEST_ERROR, "Trying to write an unknown exception"
+                      " vector 0x%03x\n", sprn);
+        gen_inval_exception(ctx, POWERPC_EXCP_INVAL_INVAL);
         return;
     }
 
-- 
2.25.1

Re: [PATCH 6/6] target/ppc: fix exception error code in spr_write_excp_vector

[Daniel Henrique Barboza]

On 6/27/22 11:11, Matheus Ferst wrote:
> The 'error' argument of gen_inval_exception will be or-ed with
> POWERPC_EXCP_INVAL, so it should always be a constant prefixed with
> POWERPC_EXCP_INVAL_. No functional change is intended,
> spr_write_excp_vector is only used by register_BookE_sprs, and
> powerpc_excp_booke ignores the lower 4 bits of the error code on
> POWERPC_EXCP_INVAL exceptions.
> 
> Also, take the opportunity to replace printf with qemu_log_mask.
> 
> Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
> ---

Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>

>   target/ppc/translate.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index 30dd524959..da11472877 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -907,9 +907,9 @@ void spr_write_excp_vector(DisasContext *ctx, int sprn, int gprn)
>       } else if (sprn >= SPR_BOOKE_IVOR38 && sprn <= SPR_BOOKE_IVOR42) {
>           sprn_offs = sprn - SPR_BOOKE_IVOR38 + 38;
>       } else {
> -        printf("Trying to write an unknown exception vector %d %03x\n",
> -               sprn, sprn);
> -        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_REG);
> +        qemu_log_mask(LOG_GUEST_ERROR, "Trying to write an unknown exception"
> +                      " vector 0x%03x\n", sprn);
> +        gen_inval_exception(ctx, POWERPC_EXCP_INVAL_INVAL);
>           return;
>       }
>   

Re: [PATCH 0/6] Fix gen_*_exception error codes

[Daniel Henrique Barboza]

Queued in gitlab.com/danielhb/qemu/tree/ppc-next. Thanks,


Daniel

On 6/27/22 11:10, Matheus Ferst wrote:
> The first patch of this series is the RFC of [1] (hence the r-b in v1).
> Patches 2~4 follow the other problems that Laurent pointed out, and
> patches 5-6 fix similar problems that I found.
> 
> [1] https://lists.gnu.org/archive/html/qemu-ppc/2022-01/msg00400.html
> 
> Matheus Ferst (6):
>    target/ppc: Fix gen_priv_exception error value in mfspr/mtspr
>    target/ppc: fix exception error value in slbfee
>    target/ppc: remove mfdcrux and mtdcrux
>    target/ppc: fix exception error code in helper_{load,store}_dcr
>    target/ppc: fix PMU Group A register read/write exceptions
>    target/ppc: fix exception error code in spr_write_excp_vector
> 
>   target/ppc/cpu.h                 |  6 ++----
>   target/ppc/helper.h              |  2 +-
>   target/ppc/power8-pmu-regs.c.inc | 10 ++++-----
>   target/ppc/timebase_helper.c     |  6 +++---
>   target/ppc/translate.c           | 36 ++++++++------------------------
>   5 files changed, 20 insertions(+), 40 deletions(-)
>