diff mbox series

[v2,6/8] target/ppc: allow ppc_cpu_do_system_reset to take an alternate vector

Message ID 20200316142613.121089-7-npiggin@gmail.com
State New
Headers show
Series FWNMI fixes / changes | expand

Commit Message

Nicholas Piggin March 16, 2020, 2:26 p.m. UTC
Provide for an alternate delivery location, -1 defaults to the
architected address.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 hw/ppc/spapr.c           | 2 +-
 target/ppc/cpu.h         | 2 +-
 target/ppc/excp_helper.c | 5 ++++-
 3 files changed, 6 insertions(+), 3 deletions(-)

Comments

Greg Kurz March 16, 2020, 6:04 p.m. UTC | #1
On Tue, 17 Mar 2020 00:26:11 +1000
Nicholas Piggin <npiggin@gmail.com> wrote:

> Provide for an alternate delivery location, -1 defaults to the
> architected address.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---

Reviewed-by: Greg Kurz <groug@kaod.org>

>  hw/ppc/spapr.c           | 2 +-
>  target/ppc/cpu.h         | 2 +-
>  target/ppc/excp_helper.c | 5 ++++-
>  3 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 5f93c49706..25221d843c 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -3400,7 +3400,7 @@ static void spapr_machine_finalizefn(Object *obj)
>  void spapr_do_system_reset_on_cpu(CPUState *cs, run_on_cpu_data arg)
>  {
>      cpu_synchronize_state(cs);
> -    ppc_cpu_do_system_reset(cs);
> +    ppc_cpu_do_system_reset(cs, -1);
>  }
>  
>  static void spapr_nmi(NMIState *n, int cpu_index, Error **errp)
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index 3953680534..f8c7d6f19c 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -1220,7 +1220,7 @@ int ppc64_cpu_write_elf64_note(WriteCoreDumpFunction f, CPUState *cs,
>  int ppc32_cpu_write_elf32_note(WriteCoreDumpFunction f, CPUState *cs,
>                                 int cpuid, void *opaque);
>  #ifndef CONFIG_USER_ONLY
> -void ppc_cpu_do_system_reset(CPUState *cs);
> +void ppc_cpu_do_system_reset(CPUState *cs, target_ulong vector);
>  void ppc_cpu_do_fwnmi_machine_check(CPUState *cs, target_ulong vector);
>  extern const VMStateDescription vmstate_ppc_cpu;
>  #endif
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index 7f2b5899d3..08bc885ca6 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -961,12 +961,15 @@ static void ppc_hw_interrupt(CPUPPCState *env)
>      }
>  }
>  
> -void ppc_cpu_do_system_reset(CPUState *cs)
> +void ppc_cpu_do_system_reset(CPUState *cs, target_ulong vector)
>  {
>      PowerPCCPU *cpu = POWERPC_CPU(cs);
>      CPUPPCState *env = &cpu->env;
>  
>      powerpc_excp(cpu, env->excp_model, POWERPC_EXCP_RESET);
> +    if (vector != -1) {
> +        env->nip = vector;
> +    }
>  }
>  
>  void ppc_cpu_do_fwnmi_machine_check(CPUState *cs, target_ulong vector)
Cédric Le Goater March 16, 2020, 6:15 p.m. UTC | #2
On 3/16/20 3:26 PM, Nicholas Piggin wrote:
> Provide for an alternate delivery location, -1 defaults to the
> architected address.

I don't know what is the best approach, to override the vector addr
computed by powerpc_excp() or use a machine class handler with 
cpu->vhyp.

> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>  hw/ppc/spapr.c           | 2 +-
>  target/ppc/cpu.h         | 2 +-
>  target/ppc/excp_helper.c | 5 ++++-
>  3 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 5f93c49706..25221d843c 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -3400,7 +3400,7 @@ static void spapr_machine_finalizefn(Object *obj)
>  void spapr_do_system_reset_on_cpu(CPUState *cs, run_on_cpu_data arg)
>  {
>      cpu_synchronize_state(cs);
> -    ppc_cpu_do_system_reset(cs);
> +    ppc_cpu_do_system_reset(cs, -1);
>  }
>  
>  static void spapr_nmi(NMIState *n, int cpu_index, Error **errp)
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index 3953680534..f8c7d6f19c 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -1220,7 +1220,7 @@ int ppc64_cpu_write_elf64_note(WriteCoreDumpFunction f, CPUState *cs,
>  int ppc32_cpu_write_elf32_note(WriteCoreDumpFunction f, CPUState *cs,
>                                 int cpuid, void *opaque);
>  #ifndef CONFIG_USER_ONLY
> -void ppc_cpu_do_system_reset(CPUState *cs);
> +void ppc_cpu_do_system_reset(CPUState *cs, target_ulong vector);
>  void ppc_cpu_do_fwnmi_machine_check(CPUState *cs, target_ulong vector);
>  extern const VMStateDescription vmstate_ppc_cpu;
>  #endif
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index 7f2b5899d3..08bc885ca6 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -961,12 +961,15 @@ static void ppc_hw_interrupt(CPUPPCState *env)
>      }
>  }
>  
> -void ppc_cpu_do_system_reset(CPUState *cs)
> +void ppc_cpu_do_system_reset(CPUState *cs, target_ulong vector)
>  {
>      PowerPCCPU *cpu = POWERPC_CPU(cs);
>      CPUPPCState *env = &cpu->env;
>  
>      powerpc_excp(cpu, env->excp_model, POWERPC_EXCP_RESET);
> +    if (vector != -1) {
> +        env->nip = vector;
> +    }
>  }
>  
>  void ppc_cpu_do_fwnmi_machine_check(CPUState *cs, target_ulong vector)
>
Nicholas Piggin March 16, 2020, 11:28 p.m. UTC | #3
Cédric Le Goater's on March 17, 2020 4:15 am:
> On 3/16/20 3:26 PM, Nicholas Piggin wrote:
>> Provide for an alternate delivery location, -1 defaults to the
>> architected address.
> 
> I don't know what is the best approach, to override the vector addr
> computed by powerpc_excp() or use a machine class handler with 
> cpu->vhyp.

Yeah it's getting a bit ad hoc and inconsistent with machine check
etc, I just figured get something minimal in there now. The whole
exception delivery needs a spring clean though.

Thanks,
Nick
David Gibson March 16, 2020, 11:28 p.m. UTC | #4
On Mon, Mar 16, 2020 at 07:15:14PM +0100, Cédric Le Goater wrote:
> On 3/16/20 3:26 PM, Nicholas Piggin wrote:
> > Provide for an alternate delivery location, -1 defaults to the
> > architected address.
> 
> I don't know what is the best approach, to override the vector addr
> computed by powerpc_excp() or use a machine class handler with 
> cpu->vhyp.

Again, in the interests of getting this in for the soft freeze, I've
applied this now.  We can clean it up later.

> 
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > ---
> >  hw/ppc/spapr.c           | 2 +-
> >  target/ppc/cpu.h         | 2 +-
> >  target/ppc/excp_helper.c | 5 ++++-
> >  3 files changed, 6 insertions(+), 3 deletions(-)
> > 
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index 5f93c49706..25221d843c 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -3400,7 +3400,7 @@ static void spapr_machine_finalizefn(Object *obj)
> >  void spapr_do_system_reset_on_cpu(CPUState *cs, run_on_cpu_data arg)
> >  {
> >      cpu_synchronize_state(cs);
> > -    ppc_cpu_do_system_reset(cs);
> > +    ppc_cpu_do_system_reset(cs, -1);
> >  }
> >  
> >  static void spapr_nmi(NMIState *n, int cpu_index, Error **errp)
> > diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> > index 3953680534..f8c7d6f19c 100644
> > --- a/target/ppc/cpu.h
> > +++ b/target/ppc/cpu.h
> > @@ -1220,7 +1220,7 @@ int ppc64_cpu_write_elf64_note(WriteCoreDumpFunction f, CPUState *cs,
> >  int ppc32_cpu_write_elf32_note(WriteCoreDumpFunction f, CPUState *cs,
> >                                 int cpuid, void *opaque);
> >  #ifndef CONFIG_USER_ONLY
> > -void ppc_cpu_do_system_reset(CPUState *cs);
> > +void ppc_cpu_do_system_reset(CPUState *cs, target_ulong vector);
> >  void ppc_cpu_do_fwnmi_machine_check(CPUState *cs, target_ulong vector);
> >  extern const VMStateDescription vmstate_ppc_cpu;
> >  #endif
> > diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> > index 7f2b5899d3..08bc885ca6 100644
> > --- a/target/ppc/excp_helper.c
> > +++ b/target/ppc/excp_helper.c
> > @@ -961,12 +961,15 @@ static void ppc_hw_interrupt(CPUPPCState *env)
> >      }
> >  }
> >  
> > -void ppc_cpu_do_system_reset(CPUState *cs)
> > +void ppc_cpu_do_system_reset(CPUState *cs, target_ulong vector)
> >  {
> >      PowerPCCPU *cpu = POWERPC_CPU(cs);
> >      CPUPPCState *env = &cpu->env;
> >  
> >      powerpc_excp(cpu, env->excp_model, POWERPC_EXCP_RESET);
> > +    if (vector != -1) {
> > +        env->nip = vector;
> > +    }
> >  }
> >  
> >  void ppc_cpu_do_fwnmi_machine_check(CPUState *cs, target_ulong vector)
> > 
>
David Gibson March 16, 2020, 11:34 p.m. UTC | #5
On Tue, Mar 17, 2020 at 09:28:24AM +1000, Nicholas Piggin wrote:
> Cédric Le Goater's on March 17, 2020 4:15 am:
> > On 3/16/20 3:26 PM, Nicholas Piggin wrote:
> >> Provide for an alternate delivery location, -1 defaults to the
> >> architected address.
> > 
> > I don't know what is the best approach, to override the vector addr
> > computed by powerpc_excp() or use a machine class handler with 
> > cpu->vhyp.
> 
> Yeah it's getting a bit ad hoc and inconsistent with machine check
> etc, I just figured get something minimal in there now. The whole
> exception delivery needs a spring clean though.

Yeah, there's a huge amount of cruft in nearly all the softmmu code.
It's such a big task that I don't really have any plans to tackle it
specifically.  Instead I've been cleaning up little pieces as they
impinge on things I actually care about.
Cédric Le Goater March 17, 2020, 10:47 a.m. UTC | #6
On 3/17/20 12:34 AM, David Gibson wrote:
> On Tue, Mar 17, 2020 at 09:28:24AM +1000, Nicholas Piggin wrote:
>> Cédric Le Goater's on March 17, 2020 4:15 am:
>>> On 3/16/20 3:26 PM, Nicholas Piggin wrote:
>>>> Provide for an alternate delivery location, -1 defaults to the
>>>> architected address.
>>>
>>> I don't know what is the best approach, to override the vector addr
>>> computed by powerpc_excp() or use a machine class handler with 
>>> cpu->vhyp.
>>
>> Yeah it's getting a bit ad hoc and inconsistent with machine check
>> etc, I just figured get something minimal in there now. The whole
>> exception delivery needs a spring clean though.
> 
> Yeah, there's a huge amount of cruft in nearly all the softmmu code.

The MMU emulation is not that bad to read. However, the exception model 
is hideous as one would say. powerpc_excp() is my favorite. 

> It's such a big task that I don't really have any plans to tackle it
> specifically.  Instead I've been cleaning up little pieces as they
> impinge on things I actually care about.

Maybe we should extract book3s to start with.

C.
diff mbox series

Patch

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 5f93c49706..25221d843c 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -3400,7 +3400,7 @@  static void spapr_machine_finalizefn(Object *obj)
 void spapr_do_system_reset_on_cpu(CPUState *cs, run_on_cpu_data arg)
 {
     cpu_synchronize_state(cs);
-    ppc_cpu_do_system_reset(cs);
+    ppc_cpu_do_system_reset(cs, -1);
 }
 
 static void spapr_nmi(NMIState *n, int cpu_index, Error **errp)
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 3953680534..f8c7d6f19c 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -1220,7 +1220,7 @@  int ppc64_cpu_write_elf64_note(WriteCoreDumpFunction f, CPUState *cs,
 int ppc32_cpu_write_elf32_note(WriteCoreDumpFunction f, CPUState *cs,
                                int cpuid, void *opaque);
 #ifndef CONFIG_USER_ONLY
-void ppc_cpu_do_system_reset(CPUState *cs);
+void ppc_cpu_do_system_reset(CPUState *cs, target_ulong vector);
 void ppc_cpu_do_fwnmi_machine_check(CPUState *cs, target_ulong vector);
 extern const VMStateDescription vmstate_ppc_cpu;
 #endif
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 7f2b5899d3..08bc885ca6 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -961,12 +961,15 @@  static void ppc_hw_interrupt(CPUPPCState *env)
     }
 }
 
-void ppc_cpu_do_system_reset(CPUState *cs)
+void ppc_cpu_do_system_reset(CPUState *cs, target_ulong vector)
 {
     PowerPCCPU *cpu = POWERPC_CPU(cs);
     CPUPPCState *env = &cpu->env;
 
     powerpc_excp(cpu, env->excp_model, POWERPC_EXCP_RESET);
+    if (vector != -1) {
+        env->nip = vector;
+    }
 }
 
 void ppc_cpu_do_fwnmi_machine_check(CPUState *cs, target_ulong vector)