[Qemu-devel] [PATCH 2/8] spapr: Clean up rtas_start_cpu() & rtas_stop_self()

David Gibson posted 8 patches 7 years, 5 months ago
[Qemu-devel] [PATCH 2/8] spapr: Clean up rtas_start_cpu() & rtas_stop_self()
Posted by David Gibson 7 years, 5 months ago
This makes several minor cleanups to these functions:
  * Follow usual convention of an early exit on error, rather than having
    most of the body in an if
  * Clearer naming of cpu and cpu_.  Now callcpu is the cpu from which the
    RTAS call is invoked, newcpu is the cpu which we're starting
  * Use cpu_synchronize_state() instead of kvm_cpu_synchronize_state()
    directly
  * Remove pointless comment describing what cpu_synchronize_state() does
  * Use ppc_store_lpcr() instead of directly writing the register field

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/ppc/spapr_rtas.c | 66 ++++++++++++++++++++++-----------------------
 1 file changed, 32 insertions(+), 34 deletions(-)

diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index 0ec5fa4cfe..b251c130cb 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -32,7 +32,7 @@
 #include "hw/qdev.h"
 #include "sysemu/device_tree.h"
 #include "sysemu/cpus.h"
-#include "sysemu/kvm.h"
+#include "sysemu/hw_accel.h"
 
 #include "hw/ppc/spapr.h"
 #include "hw/ppc/spapr_vio.h"
@@ -45,6 +45,7 @@
 #include "qemu/cutils.h"
 #include "trace.h"
 #include "hw/ppc/fdt.h"
+#include "target/ppc/mmu-hash64.h"
 
 static void rtas_display_character(PowerPCCPU *cpu, sPAPRMachineState *spapr,
                                    uint32_t token, uint32_t nargs,
@@ -140,13 +141,15 @@ static void spapr_cpu_set_endianness(PowerPCCPU *cpu)
     }
 }
 
-static void rtas_start_cpu(PowerPCCPU *cpu_, sPAPRMachineState *spapr,
+static void rtas_start_cpu(PowerPCCPU *callcpu, sPAPRMachineState *spapr,
                            uint32_t token, uint32_t nargs,
                            target_ulong args,
                            uint32_t nret, target_ulong rets)
 {
     target_ulong id, start, r3;
-    PowerPCCPU *cpu;
+    PowerPCCPU *newcpu;
+    CPUPPCState *env;
+    PowerPCCPUClass *pcc;
 
     if (nargs != 3 || nret != 1) {
         rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
@@ -157,41 +160,37 @@ static void rtas_start_cpu(PowerPCCPU *cpu_, sPAPRMachineState *spapr,
     start = rtas_ld(args, 1);
     r3 = rtas_ld(args, 2);
 
-    cpu = spapr_find_cpu(id);
-    if (cpu != NULL) {
-        CPUState *cs = CPU(cpu);
-        CPUPPCState *env = &cpu->env;
-        PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
+    newcpu = spapr_find_cpu(id);
+    if (!newcpu) {
+        /* Didn't find a matching cpu */
+        rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
+        return;
+    }
 
-        if (!cs->halted) {
-            rtas_st(rets, 0, RTAS_OUT_HW_ERROR);
-            return;
-        }
+    env = &newcpu->env;
+    pcc = POWERPC_CPU_GET_CLASS(newcpu);
 
-        /* This will make sure qemu state is up to date with kvm, and
-         * mark it dirty so our changes get flushed back before the
-         * new cpu enters */
-        kvm_cpu_synchronize_state(cs);
+    if (!CPU(newcpu)->halted) {
+        rtas_st(rets, 0, RTAS_OUT_HW_ERROR);
+        return;
+    }
 
-        env->msr = (1ULL << MSR_SF) | (1ULL << MSR_ME);
+    cpu_synchronize_state(CPU(newcpu));
 
-        /* Enable Power-saving mode Exit Cause exceptions for the new CPU */
-        env->spr[SPR_LPCR] |= pcc->lpcr_pm;
+    env->msr = (1ULL << MSR_SF) | (1ULL << MSR_ME);
+    spapr_cpu_set_endianness(newcpu);
+    spapr_cpu_update_tb_offset(newcpu);
+    /* Enable Power-saving mode Exit Cause exceptions for the new CPU */
+    ppc_store_lpcr(newcpu, env->spr[SPR_LPCR] | pcc->lpcr_pm);
 
-        env->nip = start;
-        env->gpr[3] = r3;
-        cs->halted = 0;
-        spapr_cpu_set_endianness(cpu);
-        spapr_cpu_update_tb_offset(cpu);
+    env->nip = start;
+    env->gpr[3] = r3;
 
-        qemu_cpu_kick(cs);
+    CPU(newcpu)->halted = 0;
 
-        rtas_st(rets, 0, RTAS_OUT_SUCCESS);
-        return;
-    }
+    qemu_cpu_kick(CPU(newcpu));
 
-    /* Didn't find a matching cpu */
-    rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
+    rtas_st(rets, 0, RTAS_OUT_SUCCESS);
 }
 
 static void rtas_stop_self(PowerPCCPU *cpu, sPAPRMachineState *spapr,
@@ -203,13 +202,12 @@ static void rtas_stop_self(PowerPCCPU *cpu, sPAPRMachineState *spapr,
     CPUPPCState *env = &cpu->env;
     PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
 
-    cs->halted = 1;
-    qemu_cpu_kick(cs);
-
     /* Disable Power-saving mode Exit Cause exceptions for the CPU.
      * This could deliver an interrupt on a dying CPU and crash the
      * guest */
-    env->spr[SPR_LPCR] &= ~pcc->lpcr_pm;
+    ppc_store_lpcr(cpu, env->spr[SPR_LPCR] & ~pcc->lpcr_pm);
+    cs->halted = 1;
+    qemu_cpu_kick(cs);
 }
 
 static inline int sysparm_st(target_ulong addr, target_ulong len,
-- 
2.17.0


Re: [Qemu-devel] [PATCH 2/8] spapr: Clean up rtas_start_cpu() & rtas_stop_self()
Posted by Cédric Le Goater 7 years, 5 months ago
On 05/03/2018 08:21 AM, David Gibson wrote:
> This makes several minor cleanups to these functions:
>   * Follow usual convention of an early exit on error, rather than having
>     most of the body in an if
>   * Clearer naming of cpu and cpu_.  Now callcpu is the cpu from which the
>     RTAS call is invoked, newcpu is the cpu which we're starting
>   * Use cpu_synchronize_state() instead of kvm_cpu_synchronize_state()
>     directly
>   * Remove pointless comment describing what cpu_synchronize_state() does
>   * Use ppc_store_lpcr() instead of directly writing the register field
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>

Reviewed-by: Cédric Le Goater <clg@kaod.org>

> ---
>  hw/ppc/spapr_rtas.c | 66 ++++++++++++++++++++++-----------------------
>  1 file changed, 32 insertions(+), 34 deletions(-)
> 
> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> index 0ec5fa4cfe..b251c130cb 100644
> --- a/hw/ppc/spapr_rtas.c
> +++ b/hw/ppc/spapr_rtas.c
> @@ -32,7 +32,7 @@
>  #include "hw/qdev.h"
>  #include "sysemu/device_tree.h"
>  #include "sysemu/cpus.h"
> -#include "sysemu/kvm.h"
> +#include "sysemu/hw_accel.h"
>  
>  #include "hw/ppc/spapr.h"
>  #include "hw/ppc/spapr_vio.h"
> @@ -45,6 +45,7 @@
>  #include "qemu/cutils.h"
>  #include "trace.h"
>  #include "hw/ppc/fdt.h"
> +#include "target/ppc/mmu-hash64.h"
>  
>  static void rtas_display_character(PowerPCCPU *cpu, sPAPRMachineState *spapr,
>                                     uint32_t token, uint32_t nargs,
> @@ -140,13 +141,15 @@ static void spapr_cpu_set_endianness(PowerPCCPU *cpu)
>      }
>  }
>  
> -static void rtas_start_cpu(PowerPCCPU *cpu_, sPAPRMachineState *spapr,
> +static void rtas_start_cpu(PowerPCCPU *callcpu, sPAPRMachineState *spapr,
>                             uint32_t token, uint32_t nargs,
>                             target_ulong args,
>                             uint32_t nret, target_ulong rets)
>  {
>      target_ulong id, start, r3;
> -    PowerPCCPU *cpu;
> +    PowerPCCPU *newcpu;
> +    CPUPPCState *env;
> +    PowerPCCPUClass *pcc;
>  
>      if (nargs != 3 || nret != 1) {
>          rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
> @@ -157,41 +160,37 @@ static void rtas_start_cpu(PowerPCCPU *cpu_, sPAPRMachineState *spapr,
>      start = rtas_ld(args, 1);
>      r3 = rtas_ld(args, 2);
>  
> -    cpu = spapr_find_cpu(id);
> -    if (cpu != NULL) {
> -        CPUState *cs = CPU(cpu);
> -        CPUPPCState *env = &cpu->env;
> -        PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
> +    newcpu = spapr_find_cpu(id);
> +    if (!newcpu) {
> +        /* Didn't find a matching cpu */
> +        rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
> +        return;
> +    }
>  
> -        if (!cs->halted) {
> -            rtas_st(rets, 0, RTAS_OUT_HW_ERROR);
> -            return;
> -        }
> +    env = &newcpu->env;
> +    pcc = POWERPC_CPU_GET_CLASS(newcpu);
>  
> -        /* This will make sure qemu state is up to date with kvm, and
> -         * mark it dirty so our changes get flushed back before the
> -         * new cpu enters */
> -        kvm_cpu_synchronize_state(cs);
> +    if (!CPU(newcpu)->halted) {
> +        rtas_st(rets, 0, RTAS_OUT_HW_ERROR);
> +        return;
> +    }
>  
> -        env->msr = (1ULL << MSR_SF) | (1ULL << MSR_ME);
> +    cpu_synchronize_state(CPU(newcpu));
>  
> -        /* Enable Power-saving mode Exit Cause exceptions for the new CPU */
> -        env->spr[SPR_LPCR] |= pcc->lpcr_pm;
> +    env->msr = (1ULL << MSR_SF) | (1ULL << MSR_ME);
> +    spapr_cpu_set_endianness(newcpu);
> +    spapr_cpu_update_tb_offset(newcpu);
> +    /* Enable Power-saving mode Exit Cause exceptions for the new CPU */
> +    ppc_store_lpcr(newcpu, env->spr[SPR_LPCR] | pcc->lpcr_pm);
>  
> -        env->nip = start;
> -        env->gpr[3] = r3;
> -        cs->halted = 0;
> -        spapr_cpu_set_endianness(cpu);
> -        spapr_cpu_update_tb_offset(cpu);
> +    env->nip = start;
> +    env->gpr[3] = r3;
>  
> -        qemu_cpu_kick(cs);
> +    CPU(newcpu)->halted = 0;
>  
> -        rtas_st(rets, 0, RTAS_OUT_SUCCESS);
> -        return;
> -    }
> +    qemu_cpu_kick(CPU(newcpu));
>  
> -    /* Didn't find a matching cpu */
> -    rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
> +    rtas_st(rets, 0, RTAS_OUT_SUCCESS);
>  }
>  
>  static void rtas_stop_self(PowerPCCPU *cpu, sPAPRMachineState *spapr,
> @@ -203,13 +202,12 @@ static void rtas_stop_self(PowerPCCPU *cpu, sPAPRMachineState *spapr,
>      CPUPPCState *env = &cpu->env;
>      PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
>  
> -    cs->halted = 1;
> -    qemu_cpu_kick(cs);
> -
>      /* Disable Power-saving mode Exit Cause exceptions for the CPU.
>       * This could deliver an interrupt on a dying CPU and crash the
>       * guest */
> -    env->spr[SPR_LPCR] &= ~pcc->lpcr_pm;
> +    ppc_store_lpcr(cpu, env->spr[SPR_LPCR] & ~pcc->lpcr_pm);
> +    cs->halted = 1;
> +    qemu_cpu_kick(cs);
>  }
>  
>  static inline int sysparm_st(target_ulong addr, target_ulong len,
> 


Re: [Qemu-devel] [PATCH 2/8] spapr: Clean up rtas_start_cpu() & rtas_stop_self()
Posted by Greg Kurz 7 years, 5 months ago
On Thu,  3 May 2018 16:21:39 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> This makes several minor cleanups to these functions:
>   * Follow usual convention of an early exit on error, rather than having
>     most of the body in an if
>   * Clearer naming of cpu and cpu_.  Now callcpu is the cpu from which the
>     RTAS call is invoked, newcpu is the cpu which we're starting
>   * Use cpu_synchronize_state() instead of kvm_cpu_synchronize_state()
>     directly
>   * Remove pointless comment describing what cpu_synchronize_state() does
>   * Use ppc_store_lpcr() instead of directly writing the register field
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  hw/ppc/spapr_rtas.c | 66 ++++++++++++++++++++++-----------------------
>  1 file changed, 32 insertions(+), 34 deletions(-)
> 
> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> index 0ec5fa4cfe..b251c130cb 100644
> --- a/hw/ppc/spapr_rtas.c
> +++ b/hw/ppc/spapr_rtas.c
> @@ -32,7 +32,7 @@
>  #include "hw/qdev.h"
>  #include "sysemu/device_tree.h"
>  #include "sysemu/cpus.h"
> -#include "sysemu/kvm.h"
> +#include "sysemu/hw_accel.h"
>  
>  #include "hw/ppc/spapr.h"
>  #include "hw/ppc/spapr_vio.h"
> @@ -45,6 +45,7 @@
>  #include "qemu/cutils.h"
>  #include "trace.h"
>  #include "hw/ppc/fdt.h"
> +#include "target/ppc/mmu-hash64.h"
>  
>  static void rtas_display_character(PowerPCCPU *cpu, sPAPRMachineState *spapr,
>                                     uint32_t token, uint32_t nargs,
> @@ -140,13 +141,15 @@ static void spapr_cpu_set_endianness(PowerPCCPU *cpu)
>      }
>  }
>  
> -static void rtas_start_cpu(PowerPCCPU *cpu_, sPAPRMachineState *spapr,
> +static void rtas_start_cpu(PowerPCCPU *callcpu, sPAPRMachineState *spapr,
>                             uint32_t token, uint32_t nargs,
>                             target_ulong args,
>                             uint32_t nret, target_ulong rets)
>  {
>      target_ulong id, start, r3;
> -    PowerPCCPU *cpu;
> +    PowerPCCPU *newcpu;
> +    CPUPPCState *env;
> +    PowerPCCPUClass *pcc;
>  
>      if (nargs != 3 || nret != 1) {
>          rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
> @@ -157,41 +160,37 @@ static void rtas_start_cpu(PowerPCCPU *cpu_, sPAPRMachineState *spapr,
>      start = rtas_ld(args, 1);
>      r3 = rtas_ld(args, 2);
>  
> -    cpu = spapr_find_cpu(id);
> -    if (cpu != NULL) {
> -        CPUState *cs = CPU(cpu);

The function ends up with four invocations of CPU(newcpu), ie, we'll
go down the full type checking path four times. Not very painful since
rtas_start_cpu() is not a hot path, but it would certainly not hurt
to do it only once like the current code does.

Anyway, with or without this change:

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

> -        CPUPPCState *env = &cpu->env;
> -        PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
> +    newcpu = spapr_find_cpu(id);
> +    if (!newcpu) {
> +        /* Didn't find a matching cpu */
> +        rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
> +        return;
> +    }
>  
> -        if (!cs->halted) {
> -            rtas_st(rets, 0, RTAS_OUT_HW_ERROR);
> -            return;
> -        }
> +    env = &newcpu->env;
> +    pcc = POWERPC_CPU_GET_CLASS(newcpu);
>  
> -        /* This will make sure qemu state is up to date with kvm, and
> -         * mark it dirty so our changes get flushed back before the
> -         * new cpu enters */
> -        kvm_cpu_synchronize_state(cs);
> +    if (!CPU(newcpu)->halted) {
> +        rtas_st(rets, 0, RTAS_OUT_HW_ERROR);
> +        return;
> +    }
>  
> -        env->msr = (1ULL << MSR_SF) | (1ULL << MSR_ME);
> +    cpu_synchronize_state(CPU(newcpu));
>  
> -        /* Enable Power-saving mode Exit Cause exceptions for the new CPU */
> -        env->spr[SPR_LPCR] |= pcc->lpcr_pm;
> +    env->msr = (1ULL << MSR_SF) | (1ULL << MSR_ME);
> +    spapr_cpu_set_endianness(newcpu);
> +    spapr_cpu_update_tb_offset(newcpu);
> +    /* Enable Power-saving mode Exit Cause exceptions for the new CPU */
> +    ppc_store_lpcr(newcpu, env->spr[SPR_LPCR] | pcc->lpcr_pm);
>  
> -        env->nip = start;
> -        env->gpr[3] = r3;
> -        cs->halted = 0;
> -        spapr_cpu_set_endianness(cpu);
> -        spapr_cpu_update_tb_offset(cpu);
> +    env->nip = start;
> +    env->gpr[3] = r3;
>  
> -        qemu_cpu_kick(cs);
> +    CPU(newcpu)->halted = 0;
>  
> -        rtas_st(rets, 0, RTAS_OUT_SUCCESS);
> -        return;
> -    }
> +    qemu_cpu_kick(CPU(newcpu));
>  
> -    /* Didn't find a matching cpu */
> -    rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
> +    rtas_st(rets, 0, RTAS_OUT_SUCCESS);
>  }
>  
>  static void rtas_stop_self(PowerPCCPU *cpu, sPAPRMachineState *spapr,
> @@ -203,13 +202,12 @@ static void rtas_stop_self(PowerPCCPU *cpu, sPAPRMachineState *spapr,
>      CPUPPCState *env = &cpu->env;
>      PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
>  
> -    cs->halted = 1;
> -    qemu_cpu_kick(cs);
> -
>      /* Disable Power-saving mode Exit Cause exceptions for the CPU.
>       * This could deliver an interrupt on a dying CPU and crash the
>       * guest */
> -    env->spr[SPR_LPCR] &= ~pcc->lpcr_pm;
> +    ppc_store_lpcr(cpu, env->spr[SPR_LPCR] & ~pcc->lpcr_pm);
> +    cs->halted = 1;
> +    qemu_cpu_kick(cs);
>  }
>  
>  static inline int sysparm_st(target_ulong addr, target_ulong len,


Re: [Qemu-devel] [PATCH 2/8] spapr: Clean up rtas_start_cpu() & rtas_stop_self()
Posted by David Gibson 7 years, 5 months ago
On Thu, May 03, 2018 at 05:13:31PM +0200, Greg Kurz wrote:
> On Thu,  3 May 2018 16:21:39 +1000
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > This makes several minor cleanups to these functions:
> >   * Follow usual convention of an early exit on error, rather than having
> >     most of the body in an if
> >   * Clearer naming of cpu and cpu_.  Now callcpu is the cpu from which the
> >     RTAS call is invoked, newcpu is the cpu which we're starting
> >   * Use cpu_synchronize_state() instead of kvm_cpu_synchronize_state()
> >     directly
> >   * Remove pointless comment describing what cpu_synchronize_state() does
> >   * Use ppc_store_lpcr() instead of directly writing the register field
> > 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > ---
> >  hw/ppc/spapr_rtas.c | 66 ++++++++++++++++++++++-----------------------
> >  1 file changed, 32 insertions(+), 34 deletions(-)
> > 
> > diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> > index 0ec5fa4cfe..b251c130cb 100644
> > --- a/hw/ppc/spapr_rtas.c
> > +++ b/hw/ppc/spapr_rtas.c
> > @@ -32,7 +32,7 @@
> >  #include "hw/qdev.h"
> >  #include "sysemu/device_tree.h"
> >  #include "sysemu/cpus.h"
> > -#include "sysemu/kvm.h"
> > +#include "sysemu/hw_accel.h"
> >  
> >  #include "hw/ppc/spapr.h"
> >  #include "hw/ppc/spapr_vio.h"
> > @@ -45,6 +45,7 @@
> >  #include "qemu/cutils.h"
> >  #include "trace.h"
> >  #include "hw/ppc/fdt.h"
> > +#include "target/ppc/mmu-hash64.h"
> >  
> >  static void rtas_display_character(PowerPCCPU *cpu, sPAPRMachineState *spapr,
> >                                     uint32_t token, uint32_t nargs,
> > @@ -140,13 +141,15 @@ static void spapr_cpu_set_endianness(PowerPCCPU *cpu)
> >      }
> >  }
> >  
> > -static void rtas_start_cpu(PowerPCCPU *cpu_, sPAPRMachineState *spapr,
> > +static void rtas_start_cpu(PowerPCCPU *callcpu, sPAPRMachineState *spapr,
> >                             uint32_t token, uint32_t nargs,
> >                             target_ulong args,
> >                             uint32_t nret, target_ulong rets)
> >  {
> >      target_ulong id, start, r3;
> > -    PowerPCCPU *cpu;
> > +    PowerPCCPU *newcpu;
> > +    CPUPPCState *env;
> > +    PowerPCCPUClass *pcc;
> >  
> >      if (nargs != 3 || nret != 1) {
> >          rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
> > @@ -157,41 +160,37 @@ static void rtas_start_cpu(PowerPCCPU *cpu_, sPAPRMachineState *spapr,
> >      start = rtas_ld(args, 1);
> >      r3 = rtas_ld(args, 2);
> >  
> > -    cpu = spapr_find_cpu(id);
> > -    if (cpu != NULL) {
> > -        CPUState *cs = CPU(cpu);
> 
> The function ends up with four invocations of CPU(newcpu), ie, we'll
> go down the full type checking path four times. Not very painful since
> rtas_start_cpu() is not a hot path, but it would certainly not hurt
> to do it only once like the current code does.
> 
> Anyway, with or without this change:

Ah, yeah, I was thinking of CPU() as a trivial offset, and forgot it
also did type checking.

Eh, as you say it's not a hot path and I don't really want to
introduce a fourth cpu parameter/variable so let's leave it as is.

> 
> Reviewed-by: Greg Kurz <groug@kaod.org>
> 
> > -        CPUPPCState *env = &cpu->env;
> > -        PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
> > +    newcpu = spapr_find_cpu(id);
> > +    if (!newcpu) {
> > +        /* Didn't find a matching cpu */
> > +        rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
> > +        return;
> > +    }
> >  
> > -        if (!cs->halted) {
> > -            rtas_st(rets, 0, RTAS_OUT_HW_ERROR);
> > -            return;
> > -        }
> > +    env = &newcpu->env;
> > +    pcc = POWERPC_CPU_GET_CLASS(newcpu);
> >  
> > -        /* This will make sure qemu state is up to date with kvm, and
> > -         * mark it dirty so our changes get flushed back before the
> > -         * new cpu enters */
> > -        kvm_cpu_synchronize_state(cs);
> > +    if (!CPU(newcpu)->halted) {
> > +        rtas_st(rets, 0, RTAS_OUT_HW_ERROR);
> > +        return;
> > +    }
> >  
> > -        env->msr = (1ULL << MSR_SF) | (1ULL << MSR_ME);
> > +    cpu_synchronize_state(CPU(newcpu));
> >  
> > -        /* Enable Power-saving mode Exit Cause exceptions for the new CPU */
> > -        env->spr[SPR_LPCR] |= pcc->lpcr_pm;
> > +    env->msr = (1ULL << MSR_SF) | (1ULL << MSR_ME);
> > +    spapr_cpu_set_endianness(newcpu);
> > +    spapr_cpu_update_tb_offset(newcpu);
> > +    /* Enable Power-saving mode Exit Cause exceptions for the new CPU */
> > +    ppc_store_lpcr(newcpu, env->spr[SPR_LPCR] | pcc->lpcr_pm);
> >  
> > -        env->nip = start;
> > -        env->gpr[3] = r3;
> > -        cs->halted = 0;
> > -        spapr_cpu_set_endianness(cpu);
> > -        spapr_cpu_update_tb_offset(cpu);
> > +    env->nip = start;
> > +    env->gpr[3] = r3;
> >  
> > -        qemu_cpu_kick(cs);
> > +    CPU(newcpu)->halted = 0;
> >  
> > -        rtas_st(rets, 0, RTAS_OUT_SUCCESS);
> > -        return;
> > -    }
> > +    qemu_cpu_kick(CPU(newcpu));
> >  
> > -    /* Didn't find a matching cpu */
> > -    rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
> > +    rtas_st(rets, 0, RTAS_OUT_SUCCESS);
> >  }
> >  
> >  static void rtas_stop_self(PowerPCCPU *cpu, sPAPRMachineState *spapr,
> > @@ -203,13 +202,12 @@ static void rtas_stop_self(PowerPCCPU *cpu, sPAPRMachineState *spapr,
> >      CPUPPCState *env = &cpu->env;
> >      PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
> >  
> > -    cs->halted = 1;
> > -    qemu_cpu_kick(cs);
> > -
> >      /* Disable Power-saving mode Exit Cause exceptions for the CPU.
> >       * This could deliver an interrupt on a dying CPU and crash the
> >       * guest */
> > -    env->spr[SPR_LPCR] &= ~pcc->lpcr_pm;
> > +    ppc_store_lpcr(cpu, env->spr[SPR_LPCR] & ~pcc->lpcr_pm);
> > +    cs->halted = 1;
> > +    qemu_cpu_kick(cs);
> >  }
> >  
> >  static inline int sysparm_st(target_ulong addr, target_ulong len,
> 

-- 
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