| Message ID | 20200316142613.121089-7-npiggin@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | FWNMI fixes / changes | expand |
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)
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) >
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
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) > > >
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.
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 --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)
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(-)