From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53990) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bxO8n-0001Vy-9y for qemu-devel@nongnu.org; Thu, 20 Oct 2016 20:56:58 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bxO8k-0005l5-7A for qemu-devel@nongnu.org; Thu, 20 Oct 2016 20:56:57 -0400 Date: Fri, 21 Oct 2016 11:56:34 +1100 From: Nicholas Piggin Message-ID: <20161021115634.4db238bc@roar.ozlabs.ibm.com> In-Reply-To: <20161020184922.518a2e44@bahia> References: <20161020065912.16132-1-npiggin@gmail.com> <20161020065912.16132-4-npiggin@gmail.com> <20161020184922.518a2e44@bahia> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [Qemu-ppc] [PATCH 3/3] ppc/spapr: implement H_SIGNAL_SYS_RESET List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Greg Kurz Cc: qemu-devel@nongnu.org, qemu-ppc@nongnu.org, David Gibson On Thu, 20 Oct 2016 18:49:22 +0200 Greg Kurz wrote: > On Thu, 20 Oct 2016 17:59:12 +1100 > 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. > > > > 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 > > --- > > 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