Re: [PATCH v2 6/8] target/ppc: allow ppc_cpu_do_system_reset to take an alternate vector
> On Mar 17, 2020, at 6:46 PM, David Gibson wrote: > > On Tue, Mar 17, 2020 at 11:06:15AM -0400, Programmingkid wrote: >> >>> On Mar 17, 2020, at 7:01 AM, qemu-ppc-requ...@nongnu.org wrote: >>> >>> Message: 3 >>> Date: Tue, 17 Mar 2020 11:47:32 +0100 >>> From: Cédric Le Goater >>> To: David Gibson , Nicholas Piggin >>> >>> Cc: qemu-...@nongnu.org, Aravinda Prasad , >>> Ganesh Goudar , Greg Kurz , >>> qemu-devel@nongnu.org >>> Subject: Re: [PATCH v2 6/8] target/ppc: allow ppc_cpu_do_system_reset >>> to take an alternate vector >>> Message-ID: <097148e5-78be-a294-236d-160fb5c29...@kaod.org> >>> Content-Type: text/plain; charset=windows-1252 >>> >>> 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. >>>> >> >> Currently Mac OS 9 will not restart. When someone goes to restart it >> the screen will turn black and stay that way. Could this patch solve >> this problem? > > No. It's unlikely to be related, and at this stage is used > exclusively to implement the FWNMI stuff for the pseries machine. > > -- > 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 Ok. Thank you.
Re: [PATCH v2 6/8] target/ppc: allow ppc_cpu_do_system_reset to take an alternate vector
On Tue, Mar 17, 2020 at 11:06:15AM -0400, Programmingkid wrote: > > > On Mar 17, 2020, at 7:01 AM, qemu-ppc-requ...@nongnu.org wrote: > > > > Message: 3 > > Date: Tue, 17 Mar 2020 11:47:32 +0100 > > From: Cédric Le Goater > > To: David Gibson , Nicholas Piggin > > > > Cc: qemu-...@nongnu.org, Aravinda Prasad , > > Ganesh Goudar , Greg Kurz , > > qemu-devel@nongnu.org > > Subject: Re: [PATCH v2 6/8] target/ppc: allow ppc_cpu_do_system_reset > > to take an alternate vector > > Message-ID: <097148e5-78be-a294-236d-160fb5c29...@kaod.org> > > Content-Type: text/plain; charset=windows-1252 > > > > 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. > >> > > Currently Mac OS 9 will not restart. When someone goes to restart it > the screen will turn black and stay that way. Could this patch solve > this problem? No. It's unlikely to be related, and at this stage is used exclusively to implement the FWNMI stuff for the pseries machine. -- 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 signature.asc Description: PGP signature
Re: [PATCH v2 6/8] target/ppc: allow ppc_cpu_do_system_reset to take an alternate vector
> On Mar 17, 2020, at 7:01 AM, qemu-ppc-requ...@nongnu.org wrote: > > Message: 3 > Date: Tue, 17 Mar 2020 11:47:32 +0100 > From: Cédric Le Goater > To: David Gibson , Nicholas Piggin > > Cc: qemu-...@nongnu.org, Aravinda Prasad , > Ganesh Goudar , Greg Kurz , > qemu-devel@nongnu.org > Subject: Re: [PATCH v2 6/8] target/ppc: allow ppc_cpu_do_system_reset > to take an alternate vector > Message-ID: <097148e5-78be-a294-236d-160fb5c29...@kaod.org> > Content-Type: text/plain; charset=windows-1252 > > 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. >> Currently Mac OS 9 will not restart. When someone goes to restart it the screen will turn black and stay that way. Could this patch solve this problem?
Re: [PATCH v2 6/8] target/ppc: allow ppc_cpu_do_system_reset to take an alternate vector
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.
Re: [PATCH v2 6/8] target/ppc: allow ppc_cpu_do_system_reset to take an alternate 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. -- 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 signature.asc Description: PGP signature
Re: [PATCH v2 6/8] target/ppc: allow ppc_cpu_do_system_reset to take an alternate vector
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
> > ---
> > 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 = >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| 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
signature.asc
Description: PGP signature
Re: [PATCH v2 6/8] target/ppc: allow ppc_cpu_do_system_reset to take an alternate 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
Re: [PATCH v2 6/8] target/ppc: allow ppc_cpu_do_system_reset to take an alternate vector
On Tue, 17 Mar 2020 00:26:11 +1000
Nicholas Piggin wrote:
> Provide for an alternate delivery location, -1 defaults to the
> architected address.
>
> Signed-off-by: Nicholas Piggin
> ---
Reviewed-by: Greg Kurz
> 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 = >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)
Re: [PATCH v2 6/8] target/ppc: allow ppc_cpu_do_system_reset to take an alternate 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
> ---
> 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 = >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)
>
[PATCH v2 6/8] target/ppc: allow ppc_cpu_do_system_reset to take an alternate vector
Provide for an alternate delivery location, -1 defaults to the
architected address.
Signed-off-by: Nicholas Piggin
---
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 = >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)
--
2.23.0
