Re: [Qemu-devel] [RFC v3 10/56] ppc: convert to cpu_halted

2018-10-22 Thread Emilio G. Cota 
On Sun, Oct 21, 2018 at 13:56:59 +0100, Richard Henderson wrote:
> On 10/19/18 2:05 AM, Emilio G. Cota wrote:
> > @@ -1088,11 +1088,13 @@ static target_ulong h_cede(PowerPCCPU *cpu, 
> > sPAPRMachineState *spapr,
> >  
> >  env->msr |= (1ULL << MSR_EE);
> >  hreg_compute_hflags(env);
> > +cpu_mutex_lock(cs);
> >  if (!cpu_has_work(cs)) {
> > -cs->halted = 1;
> > +cpu_halted_set(cs, 1);
> >  cs->exception_index = EXCP_HLT;
> >  cs->exit_request = 1;
> >  }
> > +cpu_mutex_unlock(cs);
> >  return H_SUCCESS;
> 
> Why does this one get extra locking?

It's taking into account that later in the series we
expand the CPU lock to cpu_has_work. I've added the
following note to this patch's commit log:

> In hw/ppc/spapr_hcall.c, acquire the lock just once to
> update cpu->halted and call cpu_has_work, since later
> in the series we'll acquire the BQL (if not already held)
> from cpu_has_work.

Thanks,

Emilio

Re: [Qemu-devel] [RFC v3 10/56] ppc: convert to cpu_halted

2018-10-21 Thread Richard Henderson 
On 10/19/18 2:05 AM, Emilio G. Cota wrote:
> @@ -1088,11 +1088,13 @@ static target_ulong h_cede(PowerPCCPU *cpu, 
> sPAPRMachineState *spapr,
>  
>  env->msr |= (1ULL << MSR_EE);
>  hreg_compute_hflags(env);
> +cpu_mutex_lock(cs);
>  if (!cpu_has_work(cs)) {
> -cs->halted = 1;
> +cpu_halted_set(cs, 1);
>  cs->exception_index = EXCP_HLT;
>  cs->exit_request = 1;
>  }
> +cpu_mutex_unlock(cs);
>  return H_SUCCESS;

Why does this one get extra locking?


r~

[Qemu-devel] [RFC v3 10/56] ppc: convert to cpu_halted

2018-10-18 Thread Emilio G. Cota 
In ppce500_spin.c, acquire the lock just once to update
both cpu->halted and cpu->stopped.

Cc: David Gibson 
Cc: Alexander Graf 
Cc: qemu-...@nongnu.org
Signed-off-by: Emilio G. Cota 
---
 target/ppc/helper_regs.h|  2 +-
 hw/ppc/e500.c   |  4 ++--
 hw/ppc/ppc.c| 10 +-
 hw/ppc/ppce500_spin.c   |  6 --
 hw/ppc/spapr_cpu_core.c |  4 ++--
 hw/ppc/spapr_hcall.c|  4 +++-
 hw/ppc/spapr_rtas.c |  6 +++---
 target/ppc/excp_helper.c|  4 ++--
 target/ppc/kvm.c|  4 ++--
 target/ppc/translate_init.inc.c |  6 +++---
 10 files changed, 27 insertions(+), 23 deletions(-)

diff --git a/target/ppc/helper_regs.h b/target/ppc/helper_regs.h
index 5efd18049e..9298052ac5 100644
--- a/target/ppc/helper_regs.h
+++ b/target/ppc/helper_regs.h
@@ -161,7 +161,7 @@ static inline int hreg_store_msr(CPUPPCState *env, 
target_ulong value,
 #if !defined(CONFIG_USER_ONLY)
 if (unlikely(msr_pow == 1)) {
 if (!env->pending_interrupts && (*env->check_pow)(env)) {
-cs->halted = 1;
+cpu_halted_set(cs, 1);
 excp = EXCP_HALTED;
 }
 }
diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index e6747fce28..6843c545b7 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -657,7 +657,7 @@ static void ppce500_cpu_reset_sec(void *opaque)
 
 /* Secondary CPU starts in halted state for now. Needs to change when
implementing non-kernel boot. */
-cs->halted = 1;
+cpu_halted_set(cs, 1);
 cs->exception_index = EXCP_HLT;
 }
 
@@ -671,7 +671,7 @@ static void ppce500_cpu_reset(void *opaque)
 cpu_reset(cs);
 
 /* Set initial guest state. */
-cs->halted = 0;
+cpu_halted_set(cs, 0);
 env->gpr[1] = (16 * MiB) - 8;
 env->gpr[3] = bi->dt_base;
 env->gpr[4] = 0;
diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
index ec4be25f49..d1a5a0b877 100644
--- a/hw/ppc/ppc.c
+++ b/hw/ppc/ppc.c
@@ -151,7 +151,7 @@ static void ppc6xx_set_irq(void *opaque, int pin, int level)
 /* XXX: Note that the only way to restart the CPU is to reset it */
 if (level) {
 LOG_IRQ("%s: stop the CPU\n", __func__);
-cs->halted = 1;
+cpu_halted_set(cs, 1);
 }
 break;
 case PPC6xx_INPUT_HRESET:
@@ -230,10 +230,10 @@ static void ppc970_set_irq(void *opaque, int pin, int 
level)
 /* XXX: TODO: relay the signal to CKSTP_OUT pin */
 if (level) {
 LOG_IRQ("%s: stop the CPU\n", __func__);
-cs->halted = 1;
+cpu_halted_set(cs, 1);
 } else {
 LOG_IRQ("%s: restart the CPU\n", __func__);
-cs->halted = 0;
+cpu_halted_set(cs, 0);
 qemu_cpu_kick(cs);
 }
 break;
@@ -361,10 +361,10 @@ static void ppc40x_set_irq(void *opaque, int pin, int 
level)
 /* Level sensitive - active low */
 if (level) {
 LOG_IRQ("%s: stop the CPU\n", __func__);
-cs->halted = 1;
+cpu_halted_set(cs, 1);
 } else {
 LOG_IRQ("%s: restart the CPU\n", __func__);
-cs->halted = 0;
+cpu_halted_set(cs, 0);
 qemu_cpu_kick(cs);
 }
 break;
diff --git a/hw/ppc/ppce500_spin.c b/hw/ppc/ppce500_spin.c
index c45fc858de..4b3532730f 100644
--- a/hw/ppc/ppce500_spin.c
+++ b/hw/ppc/ppce500_spin.c
@@ -107,9 +107,11 @@ static void spin_kick(CPUState *cs, run_on_cpu_data data)
 map_start = ldq_p(>addr) & ~(map_size - 1);
 mmubooke_create_initial_mapping(env, 0, map_start, map_size);
 
-cs->halted = 0;
-cs->exception_index = -1;
+cpu_mutex_lock(cs);
+cpu_halted_set(cs, 0);
 cs->stopped = false;
+cpu_mutex_unlock(cs);
+cs->exception_index = -1;
 qemu_cpu_kick(cs);
 }
 
diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index 2398ce62c0..4c9c60b53b 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -37,7 +37,7 @@ static void spapr_cpu_reset(void *opaque)
 /* All CPUs start halted.  CPU0 is unhalted from the machine level
  * reset code and the rest are explicitly started up by the guest
  * using an RTAS call */
-cs->halted = 1;
+cpu_halted_set(cs, 1);
 
 /* Set compatibility mode to match the boot CPU, which was either set
  * by the machine reset code or by CAS. This should never fail.
@@ -91,7 +91,7 @@ void spapr_cpu_set_entry_state(PowerPCCPU *cpu, target_ulong 
nip, target_ulong r
 env->nip = nip;
 env->gpr[3] = r3;
 kvmppc_set_reg_ppc_online(cpu, 1);
-CPU(cpu)->halted = 0;
+cpu_halted_set(CPU(cpu), 0);
 /* Enable Power-saving mode Exit Cause exceptions */
 ppc_store_lpcr(cpu, env->spr[SPR_LPCR] | pcc->lpcr_pm);
 }
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index