| Message ID | 20161020065912.16132-4-npiggin@gmail.com |
|---|---|
| State | New |
| Headers | show |
On Thu, 20 Oct 2016 11:21:22 +0200 Thomas Huth <thuth@redhat.com> wrote: > On 20.10.2016 08:59, Nicholas Piggin wrote: > > The H_SIGNAL_SYS_RESET hcall allows a guest CPU to raise a system > > reset exception on other CPUs in the same guest. > > > > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> > > --- > > hw/ppc/spapr_hcall.c | 42 ++++++++++++++++++++++++++++++++++++++++++ > > include/hw/ppc/spapr.h | 8 +++++++- > > 2 files changed, 49 insertions(+), 1 deletion(-) > ... > > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h > > index aeaba3e..a28538b 100644 > > --- a/include/hw/ppc/spapr.h > > +++ b/include/hw/ppc/spapr.h > > @@ -339,7 +339,13 @@ struct sPAPRMachineState { > > #define H_XIRR_X 0x2FC > > #define H_RANDOM 0x300 > > #define H_SET_MODE 0x31C > > -#define MAX_HCALL_OPCODE H_SET_MODE > > +#define H_SIGNAL_SYS_RESET 0x380 > > +#define MAX_HCALL_OPCODE H_SIGNAL_SYS_RESET > > + > > +/* Parameters to H_SIGNAL_SYS_RESET */ > > +#define H_SIGNAL_SYS_RESET_ALL -1 > > +#define H_SIGNAL_SYS_RESET_ALLBUTSELF -2 > > + > > > > /* The hcalls above are standardized in PAPR and implemented by pHyp > > * as well. > > Is there a spec for this hypercall? I can't find it in LoPAPR v1.1 ? Oh sorry, I should have said that this is going through an internal process at moment and not in a released document yet. I'll try to get a more satisfying answer on that one. Thanks, Nick
On Thu, 20 Oct 2016 17:59:12 +1100 Nicholas Piggin <npiggin@gmail.com> wrote: > The H_SIGNAL_SYS_RESET hcall allows a guest CPU to raise a system > reset exception on other CPUs in the same guest. > Actually on all CPUs or all-but-self CPUs or a specific CPU (including self). > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> > --- > hw/ppc/spapr_hcall.c | 42 ++++++++++++++++++++++++++++++++++++++++++ > include/hw/ppc/spapr.h | 8 +++++++- > 2 files changed, 49 insertions(+), 1 deletion(-) > > diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c > index c5e7e8c..5ae84f0 100644 > --- a/hw/ppc/spapr_hcall.c > +++ b/hw/ppc/spapr_hcall.c > @@ -880,6 +880,47 @@ static target_ulong h_set_mode(PowerPCCPU *cpu, sPAPRMachineState *spapr, > return ret; > } > > +static void do_sys_reset(CPUState *cs, void *arg) > +{ > + cpu_synchronize_state(cs); > + ppc_cpu_do_system_reset(cs); > +} > + We already have the following function in hw/ppc/spapr.c, which serves the same purpose: static void ppc_cpu_do_nmi_on_cpu(CPUState *cs, void *arg) { cpu_synchronize_state(cs); ppc_cpu_do_system_reset(cs); } What about using it ? > +static target_ulong h_signal_sys_reset(PowerPCCPU *cpu, > + sPAPRMachineState *spapr, > + target_ulong opcode, target_ulong *args) > +{ > + target_long target = args[0]; > + CPUState *cs; > + > + if (target < H_SIGNAL_SYS_RESET_ALLBUTSELF) { > + return H_PARAMETER; > + } > + > + CPU_FOREACH(cs) { > + PowerPCCPU *c = POWERPC_CPU(cs); > + > + if (cpu->cpu_dt_id == target) { > + run_on_cpu(cs, do_sys_reset, NULL); > + return H_SUCCESS; > + } > + > + if (target == H_SIGNAL_SYS_RESET_ALLBUTSELF) { > + if (c == cpu) { > + continue; > + } > + } > + > + run_on_cpu(cs, do_sys_reset, NULL); > + } > + > + if (target >= 0) { > + return H_PARAMETER; > + } > + > + return H_SUCCESS; > +} > + > /* > * Return the offset to the requested option vector @vector in the > * option vector table @table. > @@ -1113,6 +1154,7 @@ static void hypercall_register_types(void) > /* hcall-splpar */ > spapr_register_hypercall(H_REGISTER_VPA, h_register_vpa); > spapr_register_hypercall(H_CEDE, h_cede); > + spapr_register_hypercall(H_SIGNAL_SYS_RESET, h_signal_sys_reset); > > /* processor register resource access h-calls */ > spapr_register_hypercall(H_SET_SPRG0, h_set_sprg0); > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h > index aeaba3e..a28538b 100644 > --- a/include/hw/ppc/spapr.h > +++ b/include/hw/ppc/spapr.h > @@ -339,7 +339,13 @@ struct sPAPRMachineState { > #define H_XIRR_X 0x2FC > #define H_RANDOM 0x300 > #define H_SET_MODE 0x31C > -#define MAX_HCALL_OPCODE H_SET_MODE > +#define H_SIGNAL_SYS_RESET 0x380 > +#define MAX_HCALL_OPCODE H_SIGNAL_SYS_RESET > + > +/* Parameters to H_SIGNAL_SYS_RESET */ > +#define H_SIGNAL_SYS_RESET_ALL -1 > +#define H_SIGNAL_SYS_RESET_ALLBUTSELF -2 > + I'd rather move these to hw/ppc/spapr_hcall.c just above h_signal_sys_reset(), the same way it is done for h_bulk_remove(). > > /* The hcalls above are standardized in PAPR and implemented by pHyp > * as well.
On Thu, 20 Oct 2016 18:49:22 +0200 Greg Kurz <groug@kaod.org> wrote: > On Thu, 20 Oct 2016 17:59:12 +1100 > Nicholas Piggin <npiggin@gmail.com> wrote: > > > The H_SIGNAL_SYS_RESET hcall allows a guest CPU to raise a system > > reset exception on other CPUs in the same guest. > > > > Actually on all CPUs or all-but-self CPUs or a specific CPU (including self). Exactly right. I'll improve the changelog and try to add something more official. > > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> > > --- > > hw/ppc/spapr_hcall.c | 42 ++++++++++++++++++++++++++++++++++++++++++ > > include/hw/ppc/spapr.h | 8 +++++++- > > 2 files changed, 49 insertions(+), 1 deletion(-) > > > > diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c > > index c5e7e8c..5ae84f0 100644 > > --- a/hw/ppc/spapr_hcall.c > > +++ b/hw/ppc/spapr_hcall.c > > @@ -880,6 +880,47 @@ static target_ulong h_set_mode(PowerPCCPU *cpu, sPAPRMachineState *spapr, > > return ret; > > } > > > > +static void do_sys_reset(CPUState *cs, void *arg) > > +{ > > + cpu_synchronize_state(cs); > > + ppc_cpu_do_system_reset(cs); > > +} > > + > > We already have the following function in hw/ppc/spapr.c, which serves the > same purpose: > > static void ppc_cpu_do_nmi_on_cpu(CPUState *cs, void *arg) > { > cpu_synchronize_state(cs); > ppc_cpu_do_system_reset(cs); > } > > What about using it ? I suppose. "nmi" isn't really architected, whereas system reset is. I was considering ppc_cpu_do_nmi_on_cpu call do_sys_reset, but it didn't seem like much of an improvement. I'll use a single function for both if you prefer. > > --- a/include/hw/ppc/spapr.h > > +++ b/include/hw/ppc/spapr.h > > @@ -339,7 +339,13 @@ struct sPAPRMachineState { > > #define H_XIRR_X 0x2FC > > #define H_RANDOM 0x300 > > #define H_SET_MODE 0x31C > > -#define MAX_HCALL_OPCODE H_SET_MODE > > +#define H_SIGNAL_SYS_RESET 0x380 > > +#define MAX_HCALL_OPCODE H_SIGNAL_SYS_RESET > > + > > +/* Parameters to H_SIGNAL_SYS_RESET */ > > +#define H_SIGNAL_SYS_RESET_ALL -1 > > +#define H_SIGNAL_SYS_RESET_ALLBUTSELF -2 > > + > > I'd rather move these to hw/ppc/spapr_hcall.c just above h_signal_sys_reset(), > the same way it is done for h_bulk_remove(). Will do. Thanks, Nick
On Fri, Oct 21, 2016 at 11:56:34AM +1100, Nicholas Piggin wrote: > On Thu, 20 Oct 2016 18:49:22 +0200 > Greg Kurz <groug@kaod.org> wrote: > > > On Thu, 20 Oct 2016 17:59:12 +1100 > > Nicholas Piggin <npiggin@gmail.com> wrote: > > > > > The H_SIGNAL_SYS_RESET hcall allows a guest CPU to raise a system > > > reset exception on other CPUs in the same guest. > > > > > > > Actually on all CPUs or all-but-self CPUs or a specific CPU (including self). > > Exactly right. I'll improve the changelog and try to add something more > official. > > > > > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> > > > --- > > > hw/ppc/spapr_hcall.c | 42 ++++++++++++++++++++++++++++++++++++++++++ > > > include/hw/ppc/spapr.h | 8 +++++++- > > > 2 files changed, 49 insertions(+), 1 deletion(-) > > > > > > diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c > > > index c5e7e8c..5ae84f0 100644 > > > --- a/hw/ppc/spapr_hcall.c > > > +++ b/hw/ppc/spapr_hcall.c > > > @@ -880,6 +880,47 @@ static target_ulong h_set_mode(PowerPCCPU *cpu, sPAPRMachineState *spapr, > > > return ret; > > > } > > > > > > +static void do_sys_reset(CPUState *cs, void *arg) > > > +{ > > > + cpu_synchronize_state(cs); > > > + ppc_cpu_do_system_reset(cs); > > > +} > > > + > > > > We already have the following function in hw/ppc/spapr.c, which serves the > > same purpose: > > > > static void ppc_cpu_do_nmi_on_cpu(CPUState *cs, void *arg) > > { > > cpu_synchronize_state(cs); > > ppc_cpu_do_system_reset(cs); > > } > > > > What about using it ? > > I suppose. "nmi" isn't really architected, whereas system reset is. > I was considering ppc_cpu_do_nmi_on_cpu call do_sys_reset, but it > didn't seem like much of an improvement. I'll use a single function > for both if you prefer. I think system_reset is a better name, so it's probably best to rename the existing function. > > > > --- a/include/hw/ppc/spapr.h > > > +++ b/include/hw/ppc/spapr.h > > > @@ -339,7 +339,13 @@ struct sPAPRMachineState { > > > #define H_XIRR_X 0x2FC > > > #define H_RANDOM 0x300 > > > #define H_SET_MODE 0x31C > > > -#define MAX_HCALL_OPCODE H_SET_MODE > > > +#define H_SIGNAL_SYS_RESET 0x380 > > > +#define MAX_HCALL_OPCODE H_SIGNAL_SYS_RESET > > > + > > > +/* Parameters to H_SIGNAL_SYS_RESET */ > > > +#define H_SIGNAL_SYS_RESET_ALL -1 > > > +#define H_SIGNAL_SYS_RESET_ALLBUTSELF -2 > > > + > > > > I'd rather move these to hw/ppc/spapr_hcall.c just above h_signal_sys_reset(), > > the same way it is done for h_bulk_remove(). > > Will do. > > Thanks, > Nick >
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c index c5e7e8c..5ae84f0 100644 --- a/hw/ppc/spapr_hcall.c +++ b/hw/ppc/spapr_hcall.c @@ -880,6 +880,47 @@ static target_ulong h_set_mode(PowerPCCPU *cpu, sPAPRMachineState *spapr, return ret; } +static void do_sys_reset(CPUState *cs, void *arg) +{ + cpu_synchronize_state(cs); + ppc_cpu_do_system_reset(cs); +} + +static target_ulong h_signal_sys_reset(PowerPCCPU *cpu, + sPAPRMachineState *spapr, + target_ulong opcode, target_ulong *args) +{ + target_long target = args[0]; + CPUState *cs; + + if (target < H_SIGNAL_SYS_RESET_ALLBUTSELF) { + return H_PARAMETER; + } + + CPU_FOREACH(cs) { + PowerPCCPU *c = POWERPC_CPU(cs); + + if (cpu->cpu_dt_id == target) { + run_on_cpu(cs, do_sys_reset, NULL); + return H_SUCCESS; + } + + if (target == H_SIGNAL_SYS_RESET_ALLBUTSELF) { + if (c == cpu) { + continue; + } + } + + run_on_cpu(cs, do_sys_reset, NULL); + } + + if (target >= 0) { + return H_PARAMETER; + } + + return H_SUCCESS; +} + /* * Return the offset to the requested option vector @vector in the * option vector table @table. @@ -1113,6 +1154,7 @@ static void hypercall_register_types(void) /* hcall-splpar */ spapr_register_hypercall(H_REGISTER_VPA, h_register_vpa); spapr_register_hypercall(H_CEDE, h_cede); + spapr_register_hypercall(H_SIGNAL_SYS_RESET, h_signal_sys_reset); /* processor register resource access h-calls */ spapr_register_hypercall(H_SET_SPRG0, h_set_sprg0); diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h index aeaba3e..a28538b 100644 --- a/include/hw/ppc/spapr.h +++ b/include/hw/ppc/spapr.h @@ -339,7 +339,13 @@ struct sPAPRMachineState { #define H_XIRR_X 0x2FC #define H_RANDOM 0x300 #define H_SET_MODE 0x31C -#define MAX_HCALL_OPCODE H_SET_MODE +#define H_SIGNAL_SYS_RESET 0x380 +#define MAX_HCALL_OPCODE H_SIGNAL_SYS_RESET + +/* Parameters to H_SIGNAL_SYS_RESET */ +#define H_SIGNAL_SYS_RESET_ALL -1 +#define H_SIGNAL_SYS_RESET_ALLBUTSELF -2 + /* The hcalls above are standardized in PAPR and implemented by pHyp * as well.
The H_SIGNAL_SYS_RESET hcall allows a guest CPU to raise a system reset exception on other CPUs in the same guest. Signed-off-by: Nicholas Piggin <npiggin@gmail.com> --- hw/ppc/spapr_hcall.c | 42 ++++++++++++++++++++++++++++++++++++++++++ include/hw/ppc/spapr.h | 8 +++++++- 2 files changed, 49 insertions(+), 1 deletion(-)