The 405 Program Interrupt does not set SRR1 with any diagnostic bits,
just a clean copy of the MSR.

We're using the BookE Exception Syndrome Register which is different
from the 405.

Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
---
 target/ppc/excp_helper.c | 16 ----------------
 1 file changed, 16 deletions(-)

diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 13674a102f..2efec6d13b 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -484,30 +484,14 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
                 env->error_code = 0;
                 return;
             }
-
-            /*
-             * FP exceptions always have NIP pointing to the faulting
-             * instruction, so always use store_next and claim we are
-             * precise in the MSR.
-             */
-            msr |= 0x00100000;
-            env->spr[SPR_BOOKE_ESR] = ESR_FP;
             break;
         case POWERPC_EXCP_INVAL:
             trace_ppc_excp_inval(env->nip);
-            msr |= 0x00080000;
-            env->spr[SPR_BOOKE_ESR] = ESR_PIL;
             break;
         case POWERPC_EXCP_PRIV:
-            msr |= 0x00040000;
-            env->spr[SPR_BOOKE_ESR] = ESR_PPR;
-            break;
         case POWERPC_EXCP_TRAP:
-            msr |= 0x00020000;
-            env->spr[SPR_BOOKE_ESR] = ESR_PTR;
             break;
         default:
-            /* Should never occur */
             cpu_abort(cs, "Invalid program exception %d. Aborting\n",
                       env->error_code);
             break;
-- 
2.33.1

On Tue, Jan 18, 2022 at 03:44:47PM -0300, Fabiano Rosas wrote:
> The 405 Program Interrupt does not set SRR1 with any diagnostic bits,
> just a clean copy of the MSR.
> 
> We're using the BookE Exception Syndrome Register which is different
> from the 405.

Hrm.  We really do want to set the 40x ESR bits here, though.

> 
> Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
> ---
>  target/ppc/excp_helper.c | 16 ----------------
>  1 file changed, 16 deletions(-)
> 
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index 13674a102f..2efec6d13b 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -484,30 +484,14 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
>                  env->error_code = 0;
>                  return;
>              }
> -
> -            /*
> -             * FP exceptions always have NIP pointing to the faulting
> -             * instruction, so always use store_next and claim we are
> -             * precise in the MSR.
> -             */
> -            msr |= 0x00100000;
> -            env->spr[SPR_BOOKE_ESR] = ESR_FP;
>              break;
>          case POWERPC_EXCP_INVAL:
>              trace_ppc_excp_inval(env->nip);
> -            msr |= 0x00080000;
> -            env->spr[SPR_BOOKE_ESR] = ESR_PIL;
>              break;
>          case POWERPC_EXCP_PRIV:
> -            msr |= 0x00040000;
> -            env->spr[SPR_BOOKE_ESR] = ESR_PPR;
> -            break;
>          case POWERPC_EXCP_TRAP:
> -            msr |= 0x00020000;
> -            env->spr[SPR_BOOKE_ESR] = ESR_PTR;
>              break;
>          default:
> -            /* Should never occur */
>              cpu_abort(cs, "Invalid program exception %d. Aborting\n",
>                        env->error_code);
>              break;

-- 
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

David Gibson <david@gibson.dropbear.id.au> writes:

> On Tue, Jan 18, 2022 at 03:44:47PM -0300, Fabiano Rosas wrote:
>> The 405 Program Interrupt does not set SRR1 with any diagnostic bits,
>> just a clean copy of the MSR.
>> 
>> We're using the BookE Exception Syndrome Register which is different
>> from the 405.
>
> Hrm.  We really do want to set the 40x ESR bits here, though.

Well I wrote the code and nothing changed so I dropped it. Not sure if
we are even raising these properly in the translation code. I'll take
another look.

On 1/19/22 13:54, Fabiano Rosas wrote:
> David Gibson <david@gibson.dropbear.id.au> writes:
> 
>> On Tue, Jan 18, 2022 at 03:44:47PM -0300, Fabiano Rosas wrote:
>>> The 405 Program Interrupt does not set SRR1 with any diagnostic bits,
>>> just a clean copy of the MSR.
>>>
>>> We're using the BookE Exception Syndrome Register which is different
>>> from the 405.
>>
>> Hrm.  We really do want to set the 40x ESR bits here, though.
> 
> Well I wrote the code and nothing changed so I dropped it. Not sure if
> we are even raising these properly in the translation code. I'll take
> another look.
> 

For instance, this ESR bit allows Linux to handle traps correctly in
some cases, like when CONFIG_DEBUG_VM=y :

@@ -488,7 +488,9 @@ static void powerpc_excp_40x(PowerPCCPU
              trace_ppc_excp_inval(env->nip);
              break;
          case POWERPC_EXCP_PRIV:
+            break;
          case POWERPC_EXCP_TRAP:
+            env->spr[SPR_40x_ESR] = ESR_PTR;
              break;
          default:
              cpu_abort(cs, "Invalid program exception %d. Aborting\n",


These could be reported to Linux :

/* On 4xx, the reason for the machine check or program exception
    is in the ESR. */
#define get_reason(regs)	((regs)->esr)
#define REASON_FP		ESR_FP
#define REASON_ILLEGAL		(ESR_PIL | ESR_PUO)
#define REASON_PRIVILEGED	ESR_PPR
#define REASON_TRAP		ESR_PTR
#define REASON_PREFIXED		0
#define REASON_BOUNDARY		0


Thanks,

C.

On 1/18/22 19:44, Fabiano Rosas wrote:
> The 405 Program Interrupt does not set SRR1 with any diagnostic bits,
> just a clean copy of the MSR.
> 
> We're using the BookE Exception Syndrome Register which is different
> from the 405.

I restored the setting of SPR_40x_ESR.

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

Reviewed-by: Cédric Le Goater <clg@kaod.org>


Thanks,

C.


> ---
>   target/ppc/excp_helper.c | 16 ----------------
>   1 file changed, 16 deletions(-)
> 
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index 13674a102f..2efec6d13b 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -484,30 +484,14 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
>                   env->error_code = 0;
>                   return;
>               }
> -
> -            /*
> -             * FP exceptions always have NIP pointing to the faulting
> -             * instruction, so always use store_next and claim we are
> -             * precise in the MSR.
> -             */
> -            msr |= 0x00100000;
> -            env->spr[SPR_BOOKE_ESR] = ESR_FP;
>               break;
>           case POWERPC_EXCP_INVAL:
>               trace_ppc_excp_inval(env->nip);
> -            msr |= 0x00080000;
> -            env->spr[SPR_BOOKE_ESR] = ESR_PIL;
>               break;
>           case POWERPC_EXCP_PRIV:
> -            msr |= 0x00040000;
> -            env->spr[SPR_BOOKE_ESR] = ESR_PPR;
> -            break;
>           case POWERPC_EXCP_TRAP:
> -            msr |= 0x00020000;
> -            env->spr[SPR_BOOKE_ESR] = ESR_PTR;
>               break;
>           default:
> -            /* Should never occur */
>               cpu_abort(cs, "Invalid program exception %d. Aborting\n",
>                         env->error_code);
>               break;
>