diff mbox series

[2/8] target/ppc: 405: Add missing exception handlers

Message ID 20220110181546.4131853-3-farosas@linux.ibm.com
State New
Headers show
Series target/ppc: powerpc_excp improvements [40x] (3/n) | expand

Commit Message

Fabiano Rosas Jan. 10, 2022, 6:15 p.m. UTC
Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
---
 target/ppc/cpu_init.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

David Gibson Jan. 11, 2022, 2:10 a.m. UTC | #1
On Mon, Jan 10, 2022 at 03:15:40PM -0300, Fabiano Rosas wrote:
> Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
> ---
>  target/ppc/cpu_init.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
> index a50ddaeaae..9097948e67 100644
> --- a/target/ppc/cpu_init.c
> +++ b/target/ppc/cpu_init.c
> @@ -1951,7 +1951,9 @@ static void init_excp_4xx_softmmu(CPUPPCState *env)
>      env->excp_vectors[POWERPC_EXCP_EXTERNAL] = 0x00000500;
>      env->excp_vectors[POWERPC_EXCP_ALIGN]    = 0x00000600;
>      env->excp_vectors[POWERPC_EXCP_PROGRAM]  = 0x00000700;
> +    env->excp_vectors[POWERPC_EXCP_FPU]      = 0x00000800;

I have a vague recollection from my days of working on 405 that there
may have been something funky with FP emulation on there: e.g. FP
instructions causing 0x700 program interrupts instead of FP unavailble
interrupts or something.

I might be remembering incorrectly - the manual does seem to imply
that 0x800 FP unavailable is there as normal, but it might be worth
double checking this (against real hardware, if possible).

>      env->excp_vectors[POWERPC_EXCP_SYSCALL]  = 0x00000C00;
> +    env->excp_vectors[POWERPC_EXCP_APU]      = 0x00000F20;
>      env->excp_vectors[POWERPC_EXCP_PIT]      = 0x00001000;
>      env->excp_vectors[POWERPC_EXCP_FIT]      = 0x00001010;
>      env->excp_vectors[POWERPC_EXCP_WDT]      = 0x00001020;
Fabiano Rosas Jan. 14, 2022, 9:46 p.m. UTC | #2
David Gibson <david@gibson.dropbear.id.au> writes:

> On Mon, Jan 10, 2022 at 03:15:40PM -0300, Fabiano Rosas wrote:
>> Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
>> ---
>>  target/ppc/cpu_init.c | 2 ++
>>  1 file changed, 2 insertions(+)
>> 
>> diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
>> index a50ddaeaae..9097948e67 100644
>> --- a/target/ppc/cpu_init.c
>> +++ b/target/ppc/cpu_init.c
>> @@ -1951,7 +1951,9 @@ static void init_excp_4xx_softmmu(CPUPPCState *env)
>>      env->excp_vectors[POWERPC_EXCP_EXTERNAL] = 0x00000500;
>>      env->excp_vectors[POWERPC_EXCP_ALIGN]    = 0x00000600;
>>      env->excp_vectors[POWERPC_EXCP_PROGRAM]  = 0x00000700;
>> +    env->excp_vectors[POWERPC_EXCP_FPU]      = 0x00000800;
>
> I have a vague recollection from my days of working on 405 that there
> may have been something funky with FP emulation on there: e.g. FP
> instructions causing 0x700 program interrupts instead of FP unavailble
> interrupts or something.

Maybe this (from the manual):

  Program - causing conditions:
  
  Attempted execution of illegal instructions, TRAP instruction,
  privileged instruction in problem state, or auxiliary processor (APU)
  instruction, or unimplemented FPU instruction, or unimplemented APU
  instruction, or APU interrupt, or FPU interrupt
  
  FPU Unavailable - causing conditions:
  
  Attempted execution of an FPU instruction when MSR[FP]=0.

There's also this bit:

  Attempted execution of an APU instruction while the APUc405exception
  signal is asserted) results in a program interrupt. Similarly, attempted
  execution of an FPU instruction whilethe FPUc405exception signal is
  asserted) also results in a program interrupt. The following also result
  in program interrupts: attempted execution of an APU instruction while
  APUc405DcdAPUOp is asserted but APUC405DcdValidOp is deasserted; and
  attempted execution of an FPU instruction while APUc405DcdFpuOp but
  APUC405DcdValidOp is deasserted.

> I might be remembering incorrectly - the manual does seem to imply
> that 0x800 FP unavailable is there as normal, but it might be worth
> double checking this (against real hardware, if possible).

The Linux kernel has the vectors that I'm adding disabled:

  EXCEPTION(0x0800, Trap_08, unknown_exception) <-- FPU
  EXCEPTION(0x0900, Trap_09, unknown_exception)
  EXCEPTION(0x0A00, Trap_0A, unknown_exception) 
  EXCEPTION(0x0B00, Trap_0B, unknown_exception)
  ...
  EXCEPTION(0x0F00, Trap_0F, unknown_exception) <-- APU

(0xf20 would probably cause a crash as we'd jump to the middle of the
exception prologue)

Maybe I should drop this patch then? That way future developers won't
feel tempted to raise one of these.

It seems mostly inconsequential either way, what do you think?
David Gibson Jan. 15, 2022, 7:05 a.m. UTC | #3
On Fri, Jan 14, 2022 at 06:46:10PM -0300, Fabiano Rosas wrote:
> David Gibson <david@gibson.dropbear.id.au> writes:
> 
> > On Mon, Jan 10, 2022 at 03:15:40PM -0300, Fabiano Rosas wrote:
> >> Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
> >> ---
> >>  target/ppc/cpu_init.c | 2 ++
> >>  1 file changed, 2 insertions(+)
> >> 
> >> diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
> >> index a50ddaeaae..9097948e67 100644
> >> --- a/target/ppc/cpu_init.c
> >> +++ b/target/ppc/cpu_init.c
> >> @@ -1951,7 +1951,9 @@ static void init_excp_4xx_softmmu(CPUPPCState *env)
> >>      env->excp_vectors[POWERPC_EXCP_EXTERNAL] = 0x00000500;
> >>      env->excp_vectors[POWERPC_EXCP_ALIGN]    = 0x00000600;
> >>      env->excp_vectors[POWERPC_EXCP_PROGRAM]  = 0x00000700;
> >> +    env->excp_vectors[POWERPC_EXCP_FPU]      = 0x00000800;
> >
> > I have a vague recollection from my days of working on 405 that there
> > may have been something funky with FP emulation on there: e.g. FP
> > instructions causing 0x700 program interrupts instead of FP unavailble
> > interrupts or something.
> 
> Maybe this (from the manual):
> 
>   Program - causing conditions:
>   
>   Attempted execution of illegal instructions, TRAP instruction,
>   privileged instruction in problem state, or auxiliary processor (APU)
>   instruction, or unimplemented FPU instruction, or unimplemented APU
>   instruction, or APU interrupt, or FPU interrupt
>   
>   FPU Unavailable - causing conditions:
>   
>   Attempted execution of an FPU instruction when MSR[FP]=0.
> 
> There's also this bit:
> 
>   Attempted execution of an APU instruction while the APUc405exception
>   signal is asserted) results in a program interrupt. Similarly, attempted
>   execution of an FPU instruction whilethe FPUc405exception signal is
>   asserted) also results in a program interrupt. The following also result
>   in program interrupts: attempted execution of an APU instruction while
>   APUc405DcdAPUOp is asserted but APUC405DcdValidOp is deasserted; and
>   attempted execution of an FPU instruction while APUc405DcdFpuOp but
>   APUC405DcdValidOp is deasserted.

Right... those do seem to suggest that FP comes in as a 0x700 rather
than 0x800.  Really hard to be sure without checking an actual chip.

> > I might be remembering incorrectly - the manual does seem to imply
> > that 0x800 FP unavailable is there as normal, but it might be worth
> > double checking this (against real hardware, if possible).
> 
> The Linux kernel has the vectors that I'm adding disabled:
> 
>   EXCEPTION(0x0800, Trap_08, unknown_exception) <-- FPU
>   EXCEPTION(0x0900, Trap_09, unknown_exception)
>   EXCEPTION(0x0A00, Trap_0A, unknown_exception) 
>   EXCEPTION(0x0B00, Trap_0B, unknown_exception)
>   ...
>   EXCEPTION(0x0F00, Trap_0F, unknown_exception) <-- APU
> 
> (0xf20 would probably cause a crash as we'd jump to the middle of the
> exception prologue)

Right.. that's fairly strong evidence that those vectors don't operate
in practice.

> Maybe I should drop this patch then? That way future developers won't
> feel tempted to raise one of these.

Maybe.  Better yet would be to verify on a chip then drop a comment in
there explicitly describing the situation

> It seems mostly inconsequential either way, what do you think?

Well, yes.
diff mbox series

Patch

diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index a50ddaeaae..9097948e67 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -1951,7 +1951,9 @@  static void init_excp_4xx_softmmu(CPUPPCState *env)
     env->excp_vectors[POWERPC_EXCP_EXTERNAL] = 0x00000500;
     env->excp_vectors[POWERPC_EXCP_ALIGN]    = 0x00000600;
     env->excp_vectors[POWERPC_EXCP_PROGRAM]  = 0x00000700;
+    env->excp_vectors[POWERPC_EXCP_FPU]      = 0x00000800;
     env->excp_vectors[POWERPC_EXCP_SYSCALL]  = 0x00000C00;
+    env->excp_vectors[POWERPC_EXCP_APU]      = 0x00000F20;
     env->excp_vectors[POWERPC_EXCP_PIT]      = 0x00001000;
     env->excp_vectors[POWERPC_EXCP_FIT]      = 0x00001010;
     env->excp_vectors[POWERPC_EXCP_WDT]      = 0x00001020;