* [PATCH v2 0/9] Generalize start-powered-off property from ARM
@ 2020-07-22 3:50 Thiago Jung Bauermann
2020-07-22 3:50 ` [PATCH v2 1/9] target/arm: Move start-powered-off property to generic CPUState Thiago Jung Bauermann
` (8 more replies)
0 siblings, 9 replies; 32+ messages in thread
From: Thiago Jung Bauermann @ 2020-07-22 3:50 UTC (permalink / raw)
To: qemu-ppc
Cc: Thiago Jung Bauermann, Peter Maydell, Aleksandar Rikalo,
Eduardo Habkost, Aleksandar Markovic, Cornelia Huck,
Mark Cave-Ayland, qemu-devel, qemu-s390x, qemu-arm,
Artyom Tarasenko, Thomas Huth, Paolo Bonzini, David Hildenbrand,
Richard Henderson, Aurelien Jarno, David Gibson
The ARM code has a start-powered-off property in ARMCPU, which is a
subclass of CPUState. This property causes arm_cpu_reset() to set
CPUState::halted to 1, signalling that the CPU should start in a halted
state. Other architectures also have code which aim to achieve the same
effect, but without using a property.
The ppc/spapr version has a bug where QEMU does a KVM_RUN on the vcpu
before cs->halted is set to 1, causing the vcpu to run while it's still in
an unitialized state (more details in patch 3).
Peter Maydell mentioned the ARM start-powered-off property and
Eduardo Habkost suggested making it generic, so this patch series does
that, for all cases which I was able to find via grep in the code.
The only problem is that I was only able to test these changes on a ppc64le
pseries KVM guest, so except for patches 2 and 3, all others are only
build-tested. Also, my grasp of QOM lifecycle is basically non-existant so
please be aware of that when reviewing this series.
The last 3 patches I think are good cleanups but I'm even less confident in
their correctness compared to the other patches, so I marked them as RFC.
Applies cleanly on yesterday's master.
Thiago Jung Bauermann (9):
target/arm: Move start-powered-off property to generic CPUState
target/arm: Move setting of CPU halted state to generic code
ppc/spapr: Use start-powered-off CPUState property
ppc/e500: Use start-powered-off CPUState property
mips/cps: Use start-powered-off CPUState property
sparc/sun4m: Use start-powered-off CPUState property
sparc/sun4m: Don't set CPUState::halted in cpu_devinit()
sparc/sun4m: Use one cpu_reset() function for main and secondary CPUs
target/s390x: Use start-powered-off CPUState property
exec.c | 1 +
hw/core/cpu.c | 2 +-
hw/mips/cps.c | 6 +++---
hw/ppc/e500.c | 10 +++++++---
hw/ppc/spapr_cpu_core.c | 12 +++++++-----
hw/sparc/sun4m.c | 23 +++++------------------
include/hw/core/cpu.h | 4 ++++
target/arm/cpu.c | 4 +---
target/arm/cpu.h | 3 ---
target/arm/kvm32.c | 2 +-
target/arm/kvm64.c | 2 +-
target/s390x/cpu.c | 3 ++-
12 files changed, 33 insertions(+), 39 deletions(-)
^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 1/9] target/arm: Move start-powered-off property to generic CPUState
2020-07-22 3:50 [PATCH v2 0/9] Generalize start-powered-off property from ARM Thiago Jung Bauermann
@ 2020-07-22 3:50 ` Thiago Jung Bauermann
2020-07-22 6:52 ` Philippe Mathieu-Daudé
2020-07-23 0:55 ` David Gibson
2020-07-22 3:50 ` [PATCH v2 2/9] target/arm: Move setting of CPU halted state to generic code Thiago Jung Bauermann
` (7 subsequent siblings)
8 siblings, 2 replies; 32+ messages in thread
From: Thiago Jung Bauermann @ 2020-07-22 3:50 UTC (permalink / raw)
To: qemu-ppc
Cc: Thiago Jung Bauermann, Peter Maydell, Aleksandar Rikalo,
Eduardo Habkost, Aleksandar Markovic, Cornelia Huck,
Mark Cave-Ayland, qemu-devel, qemu-s390x, qemu-arm,
Artyom Tarasenko, Thomas Huth, Paolo Bonzini, David Hildenbrand,
Richard Henderson, Aurelien Jarno, David Gibson
There are other platforms which also have CPUs that start powered off, so
generalize the start-powered-off property so that it can be used by them.
Note that ARMv7MState also has a property of the same name but this patch
doesn't change it because that class isn't a subclass of CPUState so it
wouldn't be a trivial change.
This change should not cause any change in behavior.
Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
---
exec.c | 1 +
include/hw/core/cpu.h | 4 ++++
target/arm/cpu.c | 5 ++---
target/arm/cpu.h | 3 ---
target/arm/kvm32.c | 2 +-
target/arm/kvm64.c | 2 +-
6 files changed, 9 insertions(+), 8 deletions(-)
NB: I was only able to test that this patch builds. I wasn't able to
run it.
diff --git a/exec.c b/exec.c
index 6f381f98e2..82e82fab09 100644
--- a/exec.c
+++ b/exec.c
@@ -899,6 +899,7 @@ Property cpu_common_props[] = {
DEFINE_PROP_LINK("memory", CPUState, memory, TYPE_MEMORY_REGION,
MemoryRegion *),
#endif
+ DEFINE_PROP_BOOL("start-powered-off", CPUState, start_powered_off, false),
DEFINE_PROP_END_OF_LIST(),
};
diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 8f145733ce..9fc2696db5 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -374,6 +374,10 @@ struct CPUState {
bool created;
bool stop;
bool stopped;
+
+ /* Should CPU start in powered-off state? */
+ bool start_powered_off;
+
bool unplug;
bool crash_occurred;
bool exit_request;
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 111579554f..ec65c7653f 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -174,8 +174,8 @@ static void arm_cpu_reset(DeviceState *dev)
env->vfp.xregs[ARM_VFP_MVFR1] = cpu->isar.mvfr1;
env->vfp.xregs[ARM_VFP_MVFR2] = cpu->isar.mvfr2;
- cpu->power_state = cpu->start_powered_off ? PSCI_OFF : PSCI_ON;
- s->halted = cpu->start_powered_off;
+ cpu->power_state = s->start_powered_off ? PSCI_OFF : PSCI_ON;
+ s->halted = s->start_powered_off;
if (arm_feature(env, ARM_FEATURE_IWMMXT)) {
env->iwmmxt.cregs[ARM_IWMMXT_wCID] = 0x69051000 | 'Q';
@@ -2182,7 +2182,6 @@ static const ARMCPUInfo arm_cpus[] = {
};
static Property arm_cpu_properties[] = {
- DEFINE_PROP_BOOL("start-powered-off", ARMCPU, start_powered_off, false),
DEFINE_PROP_UINT32("psci-conduit", ARMCPU, psci_conduit, 0),
DEFINE_PROP_UINT64("midr", ARMCPU, midr, 0),
DEFINE_PROP_UINT64("mp-affinity", ARMCPU,
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 9e8ed423ea..a925d26996 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -810,9 +810,6 @@ struct ARMCPU {
*/
uint32_t psci_version;
- /* Should CPU start in PSCI powered-off state? */
- bool start_powered_off;
-
/* Current power state, access guarded by BQL */
ARMPSCIState power_state;
diff --git a/target/arm/kvm32.c b/target/arm/kvm32.c
index 0af46b41c8..1f2b8f8b7a 100644
--- a/target/arm/kvm32.c
+++ b/target/arm/kvm32.c
@@ -218,7 +218,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
/* Determine init features for this CPU */
memset(cpu->kvm_init_features, 0, sizeof(cpu->kvm_init_features));
- if (cpu->start_powered_off) {
+ if (cs->start_powered_off) {
cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_POWER_OFF;
}
if (kvm_check_extension(cs->kvm_state, KVM_CAP_ARM_PSCI_0_2)) {
diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
index 1169237905..f8a6d905fb 100644
--- a/target/arm/kvm64.c
+++ b/target/arm/kvm64.c
@@ -775,7 +775,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
/* Determine init features for this CPU */
memset(cpu->kvm_init_features, 0, sizeof(cpu->kvm_init_features));
- if (cpu->start_powered_off) {
+ if (cs->start_powered_off) {
cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_POWER_OFF;
}
if (kvm_check_extension(cs->kvm_state, KVM_CAP_ARM_PSCI_0_2)) {
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v2 2/9] target/arm: Move setting of CPU halted state to generic code
2020-07-22 3:50 [PATCH v2 0/9] Generalize start-powered-off property from ARM Thiago Jung Bauermann
2020-07-22 3:50 ` [PATCH v2 1/9] target/arm: Move start-powered-off property to generic CPUState Thiago Jung Bauermann
@ 2020-07-22 3:50 ` Thiago Jung Bauermann
2020-07-22 6:54 ` Philippe Mathieu-Daudé
2020-07-23 0:55 ` David Gibson
2020-07-22 3:50 ` [PATCH v2 3/9] ppc/spapr: Use start-powered-off CPUState property Thiago Jung Bauermann
` (6 subsequent siblings)
8 siblings, 2 replies; 32+ messages in thread
From: Thiago Jung Bauermann @ 2020-07-22 3:50 UTC (permalink / raw)
To: qemu-ppc
Cc: Thiago Jung Bauermann, Peter Maydell, Aleksandar Rikalo,
Eduardo Habkost, Aleksandar Markovic, Cornelia Huck,
Mark Cave-Ayland, qemu-devel, qemu-s390x, qemu-arm,
Artyom Tarasenko, Thomas Huth, Paolo Bonzini, David Hildenbrand,
Richard Henderson, Aurelien Jarno, David Gibson
This change is in a separate patch because it's not so obvious that it
won't cause a regression.
Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
---
hw/core/cpu.c | 2 +-
target/arm/cpu.c | 1 -
2 files changed, 1 insertion(+), 2 deletions(-)
NB: I was only able to test that this patch builds. I wasn't able to
run it on an ARM machine. I did on a ppc64le pseries KVM guest.
diff --git a/hw/core/cpu.c b/hw/core/cpu.c
index 594441a150..71bb7859f1 100644
--- a/hw/core/cpu.c
+++ b/hw/core/cpu.c
@@ -258,7 +258,7 @@ static void cpu_common_reset(DeviceState *dev)
}
cpu->interrupt_request = 0;
- cpu->halted = 0;
+ cpu->halted = cpu->start_powered_off;
cpu->mem_io_pc = 0;
cpu->icount_extra = 0;
atomic_set(&cpu->icount_decr_ptr->u32, 0);
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index ec65c7653f..b6c65e4df6 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -175,7 +175,6 @@ static void arm_cpu_reset(DeviceState *dev)
env->vfp.xregs[ARM_VFP_MVFR2] = cpu->isar.mvfr2;
cpu->power_state = s->start_powered_off ? PSCI_OFF : PSCI_ON;
- s->halted = s->start_powered_off;
if (arm_feature(env, ARM_FEATURE_IWMMXT)) {
env->iwmmxt.cregs[ARM_IWMMXT_wCID] = 0x69051000 | 'Q';
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v2 3/9] ppc/spapr: Use start-powered-off CPUState property
2020-07-22 3:50 [PATCH v2 0/9] Generalize start-powered-off property from ARM Thiago Jung Bauermann
2020-07-22 3:50 ` [PATCH v2 1/9] target/arm: Move start-powered-off property to generic CPUState Thiago Jung Bauermann
2020-07-22 3:50 ` [PATCH v2 2/9] target/arm: Move setting of CPU halted state to generic code Thiago Jung Bauermann
@ 2020-07-22 3:50 ` Thiago Jung Bauermann
2020-07-22 7:13 ` Philippe Mathieu-Daudé
2020-07-22 3:50 ` [PATCH v2 4/9] ppc/e500: " Thiago Jung Bauermann
` (5 subsequent siblings)
8 siblings, 1 reply; 32+ messages in thread
From: Thiago Jung Bauermann @ 2020-07-22 3:50 UTC (permalink / raw)
To: qemu-ppc
Cc: Thiago Jung Bauermann, Peter Maydell, Aleksandar Rikalo,
Eduardo Habkost, Aleksandar Markovic, Cornelia Huck,
Mark Cave-Ayland, qemu-devel, qemu-s390x, qemu-arm,
Artyom Tarasenko, Thomas Huth, Paolo Bonzini, David Hildenbrand,
Richard Henderson, Aurelien Jarno, David Gibson
PowerPC sPAPR CPUs start in the halted state, and spapr_reset_vcpu()
attempts to implement this by setting CPUState::halted to 1. But that's too
late for the case of hotplugged CPUs in a machine configure with 2 or more
threads per core.
By then, other parts of QEMU have already caused the vCPU to run in an
unitialized state a couple of times. For example, ppc_cpu_reset() calls
ppc_tlb_invalidate_all(), which ends up calling async_run_on_cpu(). This
kicks the new vCPU while it has CPUState::halted = 0, causing QEMU to issue
a KVM_RUN ioctl on the new vCPU before the guest is able to make the
start-cpu RTAS call to initialize its register state.
This problem doesn't seem to cause visible issues for regular guests, but
on a secure guest running under the Ultravisor it does. The Ultravisor
relies on being able to snoop on the start-cpu RTAS call to map vCPUs to
guests, and this issue causes it to see a stray vCPU that doesn't belong to
any guest.
Fix by setting the start-powered-off CPUState property in
spapr_create_vcpu(), which makes cpu_common_reset() initialize
CPUState::halted to 1 at an earlier moment.
Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
---
hw/ppc/spapr_cpu_core.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
NB: Tested on ppc64le pseries KVM guest with two threads per core.
Hot-plugging additional cores doesn't cause the bug described above
anymore.
diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index c4f47dcc04..09feeb5f8f 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -36,11 +36,6 @@ static void spapr_reset_vcpu(PowerPCCPU *cpu)
cpu_reset(cs);
- /* 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;
-
env->spr[SPR_HIOR] = 0;
lpcr = env->spr[SPR_LPCR];
@@ -288,6 +283,13 @@ static PowerPCCPU *spapr_create_vcpu(SpaprCpuCore *sc, int i, Error **errp)
cpu->machine_data = g_new0(SpaprCpuState, 1);
+ /*
+ * 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.
+ */
+ object_property_set_bool(OBJECT(cs), "start-powered-off", true,
+ &error_abort);
+
object_unref(obj);
return cpu;
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v2 4/9] ppc/e500: Use start-powered-off CPUState property
2020-07-22 3:50 [PATCH v2 0/9] Generalize start-powered-off property from ARM Thiago Jung Bauermann
` (2 preceding siblings ...)
2020-07-22 3:50 ` [PATCH v2 3/9] ppc/spapr: Use start-powered-off CPUState property Thiago Jung Bauermann
@ 2020-07-22 3:50 ` Thiago Jung Bauermann
2020-07-22 6:55 ` Philippe Mathieu-Daudé
2020-07-23 0:56 ` David Gibson
2020-07-22 3:50 ` [PATCH v2 5/9] mips/cps: " Thiago Jung Bauermann
` (4 subsequent siblings)
8 siblings, 2 replies; 32+ messages in thread
From: Thiago Jung Bauermann @ 2020-07-22 3:50 UTC (permalink / raw)
To: qemu-ppc
Cc: Thiago Jung Bauermann, Peter Maydell, Aleksandar Rikalo,
Eduardo Habkost, Aleksandar Markovic, Cornelia Huck,
Mark Cave-Ayland, qemu-devel, qemu-s390x, qemu-arm,
Artyom Tarasenko, Thomas Huth, Paolo Bonzini, David Hildenbrand,
Richard Henderson, Aurelien Jarno, David Gibson
Instead of setting CPUState::halted to 1 in ppce500_cpu_reset_sec(), use
the start-powered-off property which makes cpu_common_reset() initialize it
to 1 in common code.
Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
---
hw/ppc/e500.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
NB: I was only able to test that this patch builds. I wasn't able to
run it.
diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index ab9884e315..dda71bc05d 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -704,9 +704,6 @@ static void ppce500_cpu_reset_sec(void *opaque)
cpu_reset(cs);
- /* Secondary CPU starts in halted state for now. Needs to change when
- implementing non-kernel boot. */
- cs->halted = 1;
cs->exception_index = EXCP_HLT;
}
@@ -897,6 +894,13 @@ void ppce500_init(MachineState *machine)
} else {
/* Secondary CPUs */
qemu_register_reset(ppce500_cpu_reset_sec, cpu);
+
+ /*
+ * Secondary CPU starts in halted state for now. Needs to change
+ * when implementing non-kernel boot.
+ */
+ object_property_set_bool(OBJECT(cs), "start-powered-off", true,
+ &error_abort);
}
}
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v2 5/9] mips/cps: Use start-powered-off CPUState property
2020-07-22 3:50 [PATCH v2 0/9] Generalize start-powered-off property from ARM Thiago Jung Bauermann
` (3 preceding siblings ...)
2020-07-22 3:50 ` [PATCH v2 4/9] ppc/e500: " Thiago Jung Bauermann
@ 2020-07-22 3:50 ` Thiago Jung Bauermann
2020-07-22 7:09 ` Philippe Mathieu-Daudé
2020-07-23 0:57 ` David Gibson
2020-07-22 3:50 ` [PATCH v2 6/9] sparc/sun4m: " Thiago Jung Bauermann
` (3 subsequent siblings)
8 siblings, 2 replies; 32+ messages in thread
From: Thiago Jung Bauermann @ 2020-07-22 3:50 UTC (permalink / raw)
To: qemu-ppc
Cc: Thiago Jung Bauermann, Peter Maydell, Aleksandar Rikalo,
Eduardo Habkost, Aleksandar Markovic, Cornelia Huck,
Mark Cave-Ayland, qemu-devel, qemu-s390x, qemu-arm,
Artyom Tarasenko, Thomas Huth, Paolo Bonzini, David Hildenbrand,
Richard Henderson, Aurelien Jarno, David Gibson
Instead of setting CPUState::halted to 1 in main_cpu_reset(), use the
start-powered-off property which makes cpu_common_reset() initialize it
to 1 in common code.
Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
---
hw/mips/cps.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
NB: I was only able to test that this patch builds. I wasn't able to
run it.
diff --git a/hw/mips/cps.c b/hw/mips/cps.c
index 615e1a1ad2..d5b6c78019 100644
--- a/hw/mips/cps.c
+++ b/hw/mips/cps.c
@@ -52,9 +52,6 @@ static void main_cpu_reset(void *opaque)
CPUState *cs = CPU(cpu);
cpu_reset(cs);
-
- /* All VPs are halted on reset. Leave powering up to CPC. */
- cs->halted = 1;
}
static bool cpu_mips_itu_supported(CPUMIPSState *env)
@@ -89,6 +86,9 @@ static void mips_cps_realize(DeviceState *dev, Error **errp)
env->itc_tag = mips_itu_get_tag_region(&s->itu);
env->itu = &s->itu;
}
+ /* All VPs are halted on reset. Leave powering up to CPC. */
+ object_property_set_bool(OBJECT(cpu), "start-powered-off", true,
+ &error_abort);
qemu_register_reset(main_cpu_reset, cpu);
}
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v2 6/9] sparc/sun4m: Use start-powered-off CPUState property
2020-07-22 3:50 [PATCH v2 0/9] Generalize start-powered-off property from ARM Thiago Jung Bauermann
` (4 preceding siblings ...)
2020-07-22 3:50 ` [PATCH v2 5/9] mips/cps: " Thiago Jung Bauermann
@ 2020-07-22 3:50 ` Thiago Jung Bauermann
2020-07-23 0:57 ` David Gibson
2020-07-22 3:50 ` [RFC PATCH v2 7/9] sparc/sun4m: Don't set CPUState::halted in cpu_devinit() Thiago Jung Bauermann
` (2 subsequent siblings)
8 siblings, 1 reply; 32+ messages in thread
From: Thiago Jung Bauermann @ 2020-07-22 3:50 UTC (permalink / raw)
To: qemu-ppc
Cc: Thiago Jung Bauermann, Peter Maydell, Aleksandar Rikalo,
Eduardo Habkost, Aleksandar Markovic, Cornelia Huck,
Mark Cave-Ayland, qemu-devel, qemu-s390x, qemu-arm,
Artyom Tarasenko, Thomas Huth, Paolo Bonzini, David Hildenbrand,
Richard Henderson, Aurelien Jarno, David Gibson
Instead of setting CPUState::halted to 1 in secondary_cpu_reset(), use the
start-powered-off property which makes cpu_common_reset() initialize it
to 1 in common code.
Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
---
hw/sparc/sun4m.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
NB: I was only able to test that this patch builds. I wasn't able to
run it.
diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
index 9be930415f..766e79bb5e 100644
--- a/hw/sparc/sun4m.c
+++ b/hw/sparc/sun4m.c
@@ -233,7 +233,6 @@ static void secondary_cpu_reset(void *opaque)
CPUState *cs = CPU(cpu);
cpu_reset(cs);
- cs->halted = 1;
}
static void cpu_halt_signal(void *opaque, int irq, int level)
@@ -833,6 +832,8 @@ static void cpu_devinit(const char *cpu_type, unsigned int id,
qemu_register_reset(secondary_cpu_reset, cpu);
cs = CPU(cpu);
cs->halted = 1;
+ object_property_set_bool(OBJECT(cs), "start-powered-off", true,
+ &error_abort);
}
*cpu_irqs = qemu_allocate_irqs(cpu_set_irq, cpu, MAX_PILS);
env->prom_addr = prom_addr;
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [RFC PATCH v2 7/9] sparc/sun4m: Don't set CPUState::halted in cpu_devinit()
2020-07-22 3:50 [PATCH v2 0/9] Generalize start-powered-off property from ARM Thiago Jung Bauermann
` (5 preceding siblings ...)
2020-07-22 3:50 ` [PATCH v2 6/9] sparc/sun4m: " Thiago Jung Bauermann
@ 2020-07-22 3:50 ` Thiago Jung Bauermann
2020-07-22 7:02 ` Philippe Mathieu-Daudé
2020-07-22 3:50 ` [RFC PATCH v2 8/9] sparc/sun4m: Use one cpu_reset() function for main and secondary CPUs Thiago Jung Bauermann
2020-07-22 3:50 ` [RFC PATCH v2 9/9] target/s390x: Use start-powered-off CPUState property Thiago Jung Bauermann
8 siblings, 1 reply; 32+ messages in thread
From: Thiago Jung Bauermann @ 2020-07-22 3:50 UTC (permalink / raw)
To: qemu-ppc
Cc: Thiago Jung Bauermann, Peter Maydell, Aleksandar Rikalo,
Eduardo Habkost, Aleksandar Markovic, Cornelia Huck,
Mark Cave-Ayland, qemu-devel, qemu-s390x, qemu-arm,
Artyom Tarasenko, Thomas Huth, Paolo Bonzini, David Hildenbrand,
Richard Henderson, Aurelien Jarno, David Gibson
Remove setting of cs->halted from cpu_devinit(), which seems out of place
when compared to similar code in other architectures (e.g., ppce500_init()
in hw/ppc/e500.c).
Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
---
hw/sparc/sun4m.c | 1 -
1 file changed, 1 deletion(-)
NB: I was only able to test that this patch builds. I wasn't able to
run it.
diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
index 766e79bb5e..7b3042a801 100644
--- a/hw/sparc/sun4m.c
+++ b/hw/sparc/sun4m.c
@@ -831,7 +831,6 @@ static void cpu_devinit(const char *cpu_type, unsigned int id,
} else {
qemu_register_reset(secondary_cpu_reset, cpu);
cs = CPU(cpu);
- cs->halted = 1;
object_property_set_bool(OBJECT(cs), "start-powered-off", true,
&error_abort);
}
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [RFC PATCH v2 8/9] sparc/sun4m: Use one cpu_reset() function for main and secondary CPUs
2020-07-22 3:50 [PATCH v2 0/9] Generalize start-powered-off property from ARM Thiago Jung Bauermann
` (6 preceding siblings ...)
2020-07-22 3:50 ` [RFC PATCH v2 7/9] sparc/sun4m: Don't set CPUState::halted in cpu_devinit() Thiago Jung Bauermann
@ 2020-07-22 3:50 ` Thiago Jung Bauermann
2020-07-22 7:22 ` Philippe Mathieu-Daudé
2020-07-22 3:50 ` [RFC PATCH v2 9/9] target/s390x: Use start-powered-off CPUState property Thiago Jung Bauermann
8 siblings, 1 reply; 32+ messages in thread
From: Thiago Jung Bauermann @ 2020-07-22 3:50 UTC (permalink / raw)
To: qemu-ppc
Cc: Thiago Jung Bauermann, Peter Maydell, Aleksandar Rikalo,
Eduardo Habkost, Aleksandar Markovic, Cornelia Huck,
Mark Cave-Ayland, qemu-devel, qemu-s390x, qemu-arm,
Artyom Tarasenko, Thomas Huth, Paolo Bonzini, David Hildenbrand,
Richard Henderson, Aurelien Jarno, David Gibson
If we rely on cpu_common_reset() setting CPUState::halted according to the
start-powered-off property, both reset functions become equivalent and we
can use only one.
Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
---
hw/sparc/sun4m.c | 21 ++++-----------------
1 file changed, 4 insertions(+), 17 deletions(-)
NB: I was only able to test that this patch builds. I wasn't able to
run it.
diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
index 7b3042a801..deb5e9f027 100644
--- a/hw/sparc/sun4m.c
+++ b/hw/sparc/sun4m.c
@@ -218,16 +218,7 @@ static void dummy_cpu_set_irq(void *opaque, int irq, int level)
{
}
-static void main_cpu_reset(void *opaque)
-{
- SPARCCPU *cpu = opaque;
- CPUState *cs = CPU(cpu);
-
- cpu_reset(cs);
- cs->halted = 0;
-}
-
-static void secondary_cpu_reset(void *opaque)
+static void sun4m_cpu_reset(void *opaque)
{
SPARCCPU *cpu = opaque;
CPUState *cs = CPU(cpu);
@@ -818,7 +809,6 @@ static const TypeInfo ram_info = {
static void cpu_devinit(const char *cpu_type, unsigned int id,
uint64_t prom_addr, qemu_irq **cpu_irqs)
{
- CPUState *cs;
SPARCCPU *cpu;
CPUSPARCState *env;
@@ -826,12 +816,9 @@ static void cpu_devinit(const char *cpu_type, unsigned int id,
env = &cpu->env;
cpu_sparc_set_id(env, id);
- if (id == 0) {
- qemu_register_reset(main_cpu_reset, cpu);
- } else {
- qemu_register_reset(secondary_cpu_reset, cpu);
- cs = CPU(cpu);
- object_property_set_bool(OBJECT(cs), "start-powered-off", true,
+ qemu_register_reset(sun4m_cpu_reset, cpu);
+ if (id != 0) {
+ object_property_set_bool(OBJECT(cpu), "start-powered-off", true,
&error_abort);
}
*cpu_irqs = qemu_allocate_irqs(cpu_set_irq, cpu, MAX_PILS);
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [RFC PATCH v2 9/9] target/s390x: Use start-powered-off CPUState property
2020-07-22 3:50 [PATCH v2 0/9] Generalize start-powered-off property from ARM Thiago Jung Bauermann
` (7 preceding siblings ...)
2020-07-22 3:50 ` [RFC PATCH v2 8/9] sparc/sun4m: Use one cpu_reset() function for main and secondary CPUs Thiago Jung Bauermann
@ 2020-07-22 3:50 ` Thiago Jung Bauermann
2020-07-22 7:06 ` Philippe Mathieu-Daudé
2020-07-22 17:00 ` Eduardo Habkost
8 siblings, 2 replies; 32+ messages in thread
From: Thiago Jung Bauermann @ 2020-07-22 3:50 UTC (permalink / raw)
To: qemu-ppc
Cc: Thiago Jung Bauermann, Peter Maydell, Aleksandar Rikalo,
Eduardo Habkost, Aleksandar Markovic, Cornelia Huck,
Mark Cave-Ayland, qemu-devel, qemu-s390x, qemu-arm,
Artyom Tarasenko, Thomas Huth, Paolo Bonzini, David Hildenbrand,
Richard Henderson, Aurelien Jarno, David Gibson
Instead of setting CPUState::halted to 1 in s390_cpu_initfn(), use the
start-powered-off property which makes cpu_common_reset() initialize it to
1 in common code.
Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
---
target/s390x/cpu.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
NB: I was only able to test that this patch builds. I wasn't able to
run it.
diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
index 08eb674d22..d3a14af1d9 100644
--- a/target/s390x/cpu.c
+++ b/target/s390x/cpu.c
@@ -291,7 +291,8 @@ static void s390_cpu_initfn(Object *obj)
S390CPU *cpu = S390_CPU(obj);
cpu_set_cpustate_pointers(cpu);
- cs->halted = 1;
+ object_property_set_bool(OBJECT(cs), "start-powered-off", true,
+ &error_abort);
cs->exception_index = EXCP_HLT;
#if !defined(CONFIG_USER_ONLY)
object_property_add(obj, "crash-information", "GuestPanicInformation",
^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [PATCH v2 1/9] target/arm: Move start-powered-off property to generic CPUState
2020-07-22 3:50 ` [PATCH v2 1/9] target/arm: Move start-powered-off property to generic CPUState Thiago Jung Bauermann
@ 2020-07-22 6:52 ` Philippe Mathieu-Daudé
2020-07-23 0:38 ` Thiago Jung Bauermann
2020-07-23 0:55 ` David Gibson
1 sibling, 1 reply; 32+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-07-22 6:52 UTC (permalink / raw)
To: Thiago Jung Bauermann, qemu-ppc
Cc: Peter Maydell, Aleksandar Rikalo, Eduardo Habkost,
David Hildenbrand, qemu-s390x, Cornelia Huck, Mark Cave-Ayland,
qemu-devel, Aleksandar Markovic, qemu-arm, Aurelien Jarno,
Thomas Huth, Paolo Bonzini, David Gibson, Artyom Tarasenko,
Richard Henderson
Hi Thiago,
On 7/22/20 5:50 AM, Thiago Jung Bauermann wrote:
> There are other platforms which also have CPUs that start powered off, so
> generalize the start-powered-off property so that it can be used by them.
>
> Note that ARMv7MState also has a property of the same name but this patch
> doesn't change it because that class isn't a subclass of CPUState so it
> wouldn't be a trivial change.
>
> This change should not cause any change in behavior.
>
> Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
As I participated in reviewing your v1, I'd have appreciated
being Cc'ed for v2.
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> exec.c | 1 +
> include/hw/core/cpu.h | 4 ++++
> target/arm/cpu.c | 5 ++---
> target/arm/cpu.h | 3 ---
> target/arm/kvm32.c | 2 +-
> target/arm/kvm64.c | 2 +-
> 6 files changed, 9 insertions(+), 8 deletions(-)
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 2/9] target/arm: Move setting of CPU halted state to generic code
2020-07-22 3:50 ` [PATCH v2 2/9] target/arm: Move setting of CPU halted state to generic code Thiago Jung Bauermann
@ 2020-07-22 6:54 ` Philippe Mathieu-Daudé
2020-07-23 0:55 ` David Gibson
1 sibling, 0 replies; 32+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-07-22 6:54 UTC (permalink / raw)
To: Thiago Jung Bauermann, qemu-ppc
Cc: Peter Maydell, Aleksandar Rikalo, Eduardo Habkost,
David Hildenbrand, qemu-s390x, Cornelia Huck, Mark Cave-Ayland,
qemu-devel, Aleksandar Markovic, qemu-arm, Aurelien Jarno,
Thomas Huth, Paolo Bonzini, David Gibson, Artyom Tarasenko,
Richard Henderson
On 7/22/20 5:50 AM, Thiago Jung Bauermann wrote:
> This change is in a separate patch because it's not so obvious that it
> won't cause a regression.
>
> Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
> ---
> hw/core/cpu.c | 2 +-
> target/arm/cpu.c | 1 -
> 2 files changed, 1 insertion(+), 2 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 4/9] ppc/e500: Use start-powered-off CPUState property
2020-07-22 3:50 ` [PATCH v2 4/9] ppc/e500: " Thiago Jung Bauermann
@ 2020-07-22 6:55 ` Philippe Mathieu-Daudé
2020-07-23 0:56 ` David Gibson
1 sibling, 0 replies; 32+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-07-22 6:55 UTC (permalink / raw)
To: Thiago Jung Bauermann, qemu-ppc
Cc: Peter Maydell, Aleksandar Rikalo, Eduardo Habkost,
David Hildenbrand, qemu-s390x, Cornelia Huck, Mark Cave-Ayland,
qemu-devel, Aleksandar Markovic, qemu-arm, Aurelien Jarno,
Thomas Huth, Paolo Bonzini, David Gibson, Artyom Tarasenko,
Richard Henderson
On 7/22/20 5:50 AM, Thiago Jung Bauermann wrote:
> Instead of setting CPUState::halted to 1 in ppce500_cpu_reset_sec(), use
> the start-powered-off property which makes cpu_common_reset() initialize it
> to 1 in common code.
>
> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
> ---
> hw/ppc/e500.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> NB: I was only able to test that this patch builds. I wasn't able to
> run it.
>
> diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
> index ab9884e315..dda71bc05d 100644
> --- a/hw/ppc/e500.c
> +++ b/hw/ppc/e500.c
> @@ -704,9 +704,6 @@ static void ppce500_cpu_reset_sec(void *opaque)
>
> cpu_reset(cs);
>
> - /* Secondary CPU starts in halted state for now. Needs to change when
> - implementing non-kernel boot. */
> - cs->halted = 1;
> cs->exception_index = EXCP_HLT;
> }
>
> @@ -897,6 +894,13 @@ void ppce500_init(MachineState *machine)
> } else {
> /* Secondary CPUs */
> qemu_register_reset(ppce500_cpu_reset_sec, cpu);
> +
> + /*
> + * Secondary CPU starts in halted state for now. Needs to change
> + * when implementing non-kernel boot.
> + */
> + object_property_set_bool(OBJECT(cs), "start-powered-off", true,
> + &error_abort);
> }
> }
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC PATCH v2 7/9] sparc/sun4m: Don't set CPUState::halted in cpu_devinit()
2020-07-22 3:50 ` [RFC PATCH v2 7/9] sparc/sun4m: Don't set CPUState::halted in cpu_devinit() Thiago Jung Bauermann
@ 2020-07-22 7:02 ` Philippe Mathieu-Daudé
2020-07-23 0:45 ` Thiago Jung Bauermann
0 siblings, 1 reply; 32+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-07-22 7:02 UTC (permalink / raw)
To: Thiago Jung Bauermann, qemu-ppc
Cc: Peter Maydell, Aleksandar Rikalo, Eduardo Habkost,
David Hildenbrand, qemu-s390x, Cornelia Huck, Mark Cave-Ayland,
qemu-devel, Aleksandar Markovic, qemu-arm, Aurelien Jarno,
Thomas Huth, Paolo Bonzini, David Gibson, Artyom Tarasenko,
Richard Henderson
On 7/22/20 5:50 AM, Thiago Jung Bauermann wrote:
> Remove setting of cs->halted from cpu_devinit(), which seems out of place
> when compared to similar code in other architectures (e.g., ppce500_init()
> in hw/ppc/e500.c).
>
> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
> ---
> hw/sparc/sun4m.c | 1 -
> 1 file changed, 1 deletion(-)
>
> NB: I was only able to test that this patch builds. I wasn't able to
> run it.
>
> diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
> index 766e79bb5e..7b3042a801 100644
> --- a/hw/sparc/sun4m.c
> +++ b/hw/sparc/sun4m.c
> @@ -831,7 +831,6 @@ static void cpu_devinit(const char *cpu_type, unsigned int id,
> } else {
> qemu_register_reset(secondary_cpu_reset, cpu);
> cs = CPU(cpu);
> - cs->halted = 1;
> object_property_set_bool(OBJECT(cs), "start-powered-off", true,
> &error_abort);
> }
>
Why not squash with previous patch?
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC PATCH v2 9/9] target/s390x: Use start-powered-off CPUState property
2020-07-22 3:50 ` [RFC PATCH v2 9/9] target/s390x: Use start-powered-off CPUState property Thiago Jung Bauermann
@ 2020-07-22 7:06 ` Philippe Mathieu-Daudé
2020-07-23 0:50 ` Thiago Jung Bauermann
2020-07-22 17:00 ` Eduardo Habkost
1 sibling, 1 reply; 32+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-07-22 7:06 UTC (permalink / raw)
To: Thiago Jung Bauermann, qemu-ppc
Cc: Peter Maydell, Aleksandar Rikalo, Eduardo Habkost,
David Hildenbrand, qemu-s390x, Cornelia Huck, Mark Cave-Ayland,
qemu-devel, Aleksandar Markovic, qemu-arm, Aurelien Jarno,
Thomas Huth, Paolo Bonzini, David Gibson, Artyom Tarasenko,
Richard Henderson
On 7/22/20 5:50 AM, Thiago Jung Bauermann wrote:
> Instead of setting CPUState::halted to 1 in s390_cpu_initfn(), use the
> start-powered-off property which makes cpu_common_reset() initialize it to
> 1 in common code.
>
> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
> ---
> target/s390x/cpu.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> NB: I was only able to test that this patch builds. I wasn't able to
> run it.
>
> diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
> index 08eb674d22..d3a14af1d9 100644
> --- a/target/s390x/cpu.c
> +++ b/target/s390x/cpu.c
> @@ -291,7 +291,8 @@ static void s390_cpu_initfn(Object *obj)
> S390CPU *cpu = S390_CPU(obj);
>
> cpu_set_cpustate_pointers(cpu);
> - cs->halted = 1;
> + object_property_set_bool(OBJECT(cs), "start-powered-off", true,
> + &error_abort);
Here this seems overkill since this is the same object, so you can
directly do:
+ cs->start_powered_off = true;
> cs->exception_index = EXCP_HLT;
> #if !defined(CONFIG_USER_ONLY)
> object_property_add(obj, "crash-information", "GuestPanicInformation",
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 5/9] mips/cps: Use start-powered-off CPUState property
2020-07-22 3:50 ` [PATCH v2 5/9] mips/cps: " Thiago Jung Bauermann
@ 2020-07-22 7:09 ` Philippe Mathieu-Daudé
2020-07-23 0:42 ` Thiago Jung Bauermann
2020-07-23 0:57 ` David Gibson
1 sibling, 1 reply; 32+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-07-22 7:09 UTC (permalink / raw)
To: Thiago Jung Bauermann, qemu-ppc
Cc: Peter Maydell, Aleksandar Rikalo, Eduardo Habkost,
David Hildenbrand, qemu-s390x, Cornelia Huck, Mark Cave-Ayland,
qemu-devel, Aleksandar Markovic, qemu-arm, Aurelien Jarno,
Thomas Huth, Paolo Bonzini, David Gibson, Artyom Tarasenko,
Richard Henderson
On 7/22/20 5:50 AM, Thiago Jung Bauermann wrote:
> Instead of setting CPUState::halted to 1 in main_cpu_reset(), use the
> start-powered-off property which makes cpu_common_reset() initialize it
> to 1 in common code.
>
> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
> ---
> hw/mips/cps.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> NB: I was only able to test that this patch builds. I wasn't able to
> run it.
>
> diff --git a/hw/mips/cps.c b/hw/mips/cps.c
> index 615e1a1ad2..d5b6c78019 100644
> --- a/hw/mips/cps.c
> +++ b/hw/mips/cps.c
> @@ -52,9 +52,6 @@ static void main_cpu_reset(void *opaque)
> CPUState *cs = CPU(cpu);
>
> cpu_reset(cs);
> -
> - /* All VPs are halted on reset. Leave powering up to CPC. */
> - cs->halted = 1;
> }
>
> static bool cpu_mips_itu_supported(CPUMIPSState *env)
> @@ -89,6 +86,9 @@ static void mips_cps_realize(DeviceState *dev, Error **errp)
> env->itc_tag = mips_itu_get_tag_region(&s->itu);
> env->itu = &s->itu;
> }
> + /* All VPs are halted on reset. Leave powering up to CPC. */
> + object_property_set_bool(OBJECT(cpu), "start-powered-off", true,
> + &error_abort);
This is indeed better as now the property is set once, *after* realize
but *before* reset.
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> qemu_register_reset(main_cpu_reset, cpu);
> }
>
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 3/9] ppc/spapr: Use start-powered-off CPUState property
2020-07-22 3:50 ` [PATCH v2 3/9] ppc/spapr: Use start-powered-off CPUState property Thiago Jung Bauermann
@ 2020-07-22 7:13 ` Philippe Mathieu-Daudé
2020-07-23 0:40 ` Thiago Jung Bauermann
0 siblings, 1 reply; 32+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-07-22 7:13 UTC (permalink / raw)
To: Thiago Jung Bauermann, qemu-ppc
Cc: Peter Maydell, Aleksandar Rikalo, Eduardo Habkost,
David Hildenbrand, qemu-s390x, Cornelia Huck, Mark Cave-Ayland,
qemu-devel, Aleksandar Markovic, qemu-arm, Aurelien Jarno,
Thomas Huth, Paolo Bonzini, David Gibson, Artyom Tarasenko,
Richard Henderson
On 7/22/20 5:50 AM, Thiago Jung Bauermann wrote:
> PowerPC sPAPR CPUs start in the halted state, and spapr_reset_vcpu()
> attempts to implement this by setting CPUState::halted to 1. But that's too
> late for the case of hotplugged CPUs in a machine configure with 2 or more
> threads per core.
>
> By then, other parts of QEMU have already caused the vCPU to run in an
> unitialized state a couple of times. For example, ppc_cpu_reset() calls
> ppc_tlb_invalidate_all(), which ends up calling async_run_on_cpu(). This
> kicks the new vCPU while it has CPUState::halted = 0, causing QEMU to issue
> a KVM_RUN ioctl on the new vCPU before the guest is able to make the
> start-cpu RTAS call to initialize its register state.
>
> This problem doesn't seem to cause visible issues for regular guests, but
> on a secure guest running under the Ultravisor it does. The Ultravisor
> relies on being able to snoop on the start-cpu RTAS call to map vCPUs to
> guests, and this issue causes it to see a stray vCPU that doesn't belong to
> any guest.
>
> Fix by setting the start-powered-off CPUState property in
> spapr_create_vcpu(), which makes cpu_common_reset() initialize
> CPUState::halted to 1 at an earlier moment.
>
> Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
> ---
> hw/ppc/spapr_cpu_core.c | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> NB: Tested on ppc64le pseries KVM guest with two threads per core.
> Hot-plugging additional cores doesn't cause the bug described above
> anymore.
>
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index c4f47dcc04..09feeb5f8f 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -36,11 +36,6 @@ static void spapr_reset_vcpu(PowerPCCPU *cpu)
>
> cpu_reset(cs);
>
> - /* 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;
> -
> env->spr[SPR_HIOR] = 0;
>
> lpcr = env->spr[SPR_LPCR];
> @@ -288,6 +283,13 @@ static PowerPCCPU *spapr_create_vcpu(SpaprCpuCore *sc, int i, Error **errp)
>
> cpu->machine_data = g_new0(SpaprCpuState, 1);
>
> + /*
> + * 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.
> + */
> + object_property_set_bool(OBJECT(cs), "start-powered-off", true,
> + &error_abort);
Since here object_new() is used, it is simpler to set the field before
the object is realized, similarly to cs->cpu_index:
-- >8 --
@@ -275,6 +275,11 @@ static PowerPCCPU *spapr_create_vcpu(SpaprCpuCore
*sc, int i, Error **errp)
cs = CPU(obj);
cpu = POWERPC_CPU(obj);
cs->cpu_index = cc->core_id + i;
+ /*
+ * 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->start_powered_off = true;
spapr_set_vcpu_id(cpu, cs->cpu_index, &local_err);
if (local_err) {
goto err;
---
> +
> object_unref(obj);
> return cpu;
>
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC PATCH v2 8/9] sparc/sun4m: Use one cpu_reset() function for main and secondary CPUs
2020-07-22 3:50 ` [RFC PATCH v2 8/9] sparc/sun4m: Use one cpu_reset() function for main and secondary CPUs Thiago Jung Bauermann
@ 2020-07-22 7:22 ` Philippe Mathieu-Daudé
2020-07-23 0:48 ` Thiago Jung Bauermann
0 siblings, 1 reply; 32+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-07-22 7:22 UTC (permalink / raw)
To: Thiago Jung Bauermann, qemu-ppc
Cc: Peter Maydell, Aleksandar Rikalo, Eduardo Habkost,
David Hildenbrand, qemu-s390x, Cornelia Huck, Mark Cave-Ayland,
qemu-devel, Aleksandar Markovic, qemu-arm, Aurelien Jarno,
Thomas Huth, Paolo Bonzini, David Gibson, Artyom Tarasenko,
Richard Henderson
On 7/22/20 5:50 AM, Thiago Jung Bauermann wrote:
> If we rely on cpu_common_reset() setting CPUState::halted according to the
> start-powered-off property, both reset functions become equivalent and we
> can use only one.
>
> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
> ---
> hw/sparc/sun4m.c | 21 ++++-----------------
> 1 file changed, 4 insertions(+), 17 deletions(-)
>
> NB: I was only able to test that this patch builds. I wasn't able to
> run it.
>
> diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
> index 7b3042a801..deb5e9f027 100644
> --- a/hw/sparc/sun4m.c
> +++ b/hw/sparc/sun4m.c
> @@ -218,16 +218,7 @@ static void dummy_cpu_set_irq(void *opaque, int irq, int level)
> {
> }
>
> -static void main_cpu_reset(void *opaque)
> -{
> - SPARCCPU *cpu = opaque;
> - CPUState *cs = CPU(cpu);
> -
> - cpu_reset(cs);
> - cs->halted = 0;
> -}
> -
> -static void secondary_cpu_reset(void *opaque)
> +static void sun4m_cpu_reset(void *opaque)
> {
> SPARCCPU *cpu = opaque;
> CPUState *cs = CPU(cpu);
> @@ -818,7 +809,6 @@ static const TypeInfo ram_info = {
> static void cpu_devinit(const char *cpu_type, unsigned int id,
> uint64_t prom_addr, qemu_irq **cpu_irqs)
> {
> - CPUState *cs;
> SPARCCPU *cpu;
> CPUSPARCState *env;
>
> @@ -826,12 +816,9 @@ static void cpu_devinit(const char *cpu_type, unsigned int id,
> env = &cpu->env;
>
> cpu_sparc_set_id(env, id);
> - if (id == 0) {
> - qemu_register_reset(main_cpu_reset, cpu);
IMO it is easier to review this patch in 2, first drop main_cpu_reset
as it is pointless (we rely on cpu_common_reset), then set the
"start-powered-off" property and drop secondary_cpu_reset().
> - } else {
> - qemu_register_reset(secondary_cpu_reset, cpu);
> - cs = CPU(cpu);
> - object_property_set_bool(OBJECT(cs), "start-powered-off", true,
> + qemu_register_reset(sun4m_cpu_reset, cpu);
Why do you still keep it?
> + if (id != 0) {
> + object_property_set_bool(OBJECT(cpu), "start-powered-off", true,
> &error_abort);
At this point the CPU is realized, so this is correct.
I'd use directly:
object_property_set_bool(OBJECT(cpu), "start-powered-off", !!id,
&error_abort);
> }
> *cpu_irqs = qemu_allocate_irqs(cpu_set_irq, cpu, MAX_PILS);
>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC PATCH v2 9/9] target/s390x: Use start-powered-off CPUState property
2020-07-22 3:50 ` [RFC PATCH v2 9/9] target/s390x: Use start-powered-off CPUState property Thiago Jung Bauermann
2020-07-22 7:06 ` Philippe Mathieu-Daudé
@ 2020-07-22 17:00 ` Eduardo Habkost
2020-07-23 0:51 ` Thiago Jung Bauermann
1 sibling, 1 reply; 32+ messages in thread
From: Eduardo Habkost @ 2020-07-22 17:00 UTC (permalink / raw)
To: Thiago Jung Bauermann
Cc: Peter Maydell, Aleksandar Rikalo, Aleksandar Markovic,
Cornelia Huck, Mark Cave-Ayland, qemu-devel, qemu-s390x,
qemu-arm, qemu-ppc, Artyom Tarasenko, Thomas Huth, Paolo Bonzini,
David Hildenbrand, Richard Henderson, Aurelien Jarno,
David Gibson
On Wed, Jul 22, 2020 at 12:50:16AM -0300, Thiago Jung Bauermann wrote:
> Instead of setting CPUState::halted to 1 in s390_cpu_initfn(), use the
> start-powered-off property which makes cpu_common_reset() initialize it to
> 1 in common code.
>
> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
> ---
> target/s390x/cpu.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> NB: I was only able to test that this patch builds. I wasn't able to
> run it.
>
> diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
> index 08eb674d22..d3a14af1d9 100644
> --- a/target/s390x/cpu.c
> +++ b/target/s390x/cpu.c
> @@ -291,7 +291,8 @@ static void s390_cpu_initfn(Object *obj)
> S390CPU *cpu = S390_CPU(obj);
>
> cpu_set_cpustate_pointers(cpu);
> - cs->halted = 1;
> + object_property_set_bool(OBJECT(cs), "start-powered-off", true,
> + &error_abort);
Is this really OK? s390 CPUs don't seem to set halted=1 on reset
today.
> cs->exception_index = EXCP_HLT;
> #if !defined(CONFIG_USER_ONLY)
> object_property_add(obj, "crash-information", "GuestPanicInformation",
>
--
Eduardo
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 1/9] target/arm: Move start-powered-off property to generic CPUState
2020-07-22 6:52 ` Philippe Mathieu-Daudé
@ 2020-07-23 0:38 ` Thiago Jung Bauermann
0 siblings, 0 replies; 32+ messages in thread
From: Thiago Jung Bauermann @ 2020-07-23 0:38 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Peter Maydell, Aleksandar Rikalo, Eduardo Habkost,
David Hildenbrand, qemu-s390x, Cornelia Huck, Mark Cave-Ayland,
qemu-devel, Aleksandar Markovic, qemu-arm, qemu-ppc,
Aurelien Jarno, Thomas Huth, Paolo Bonzini, David Gibson,
Artyom Tarasenko, Richard Henderson
Philippe Mathieu-Daudé <philmd@redhat.com> writes:
> Hi Thiago,
>
> On 7/22/20 5:50 AM, Thiago Jung Bauermann wrote:
>> There are other platforms which also have CPUs that start powered off, so
>> generalize the start-powered-off property so that it can be used by them.
>>
>> Note that ARMv7MState also has a property of the same name but this patch
>> doesn't change it because that class isn't a subclass of CPUState so it
>> wouldn't be a trivial change.
>>
>> This change should not cause any change in behavior.
>>
>> Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
>> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
>
> As I participated in reviewing your v1, I'd have appreciated
> being Cc'ed for v2.
I'm sorry about this. I fixed the Cc list for the next version.
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Thank you very much for your prompt review, and suggestions.
I will post a new version addressing your comments shortly.
>> ---
>> exec.c | 1 +
>> include/hw/core/cpu.h | 4 ++++
>> target/arm/cpu.c | 5 ++---
>> target/arm/cpu.h | 3 ---
>> target/arm/kvm32.c | 2 +-
>> target/arm/kvm64.c | 2 +-
>> 6 files changed, 9 insertions(+), 8 deletions(-)
--
Thiago Jung Bauermann
IBM Linux Technology Center
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 3/9] ppc/spapr: Use start-powered-off CPUState property
2020-07-22 7:13 ` Philippe Mathieu-Daudé
@ 2020-07-23 0:40 ` Thiago Jung Bauermann
0 siblings, 0 replies; 32+ messages in thread
From: Thiago Jung Bauermann @ 2020-07-23 0:40 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Peter Maydell, Aleksandar Rikalo, Eduardo Habkost,
David Hildenbrand, qemu-s390x, Cornelia Huck, Mark Cave-Ayland,
qemu-devel, Aleksandar Markovic, qemu-arm, qemu-ppc,
Aurelien Jarno, Thomas Huth, Paolo Bonzini, David Gibson,
Artyom Tarasenko, Richard Henderson
Philippe Mathieu-Daudé <philmd@redhat.com> writes:
> On 7/22/20 5:50 AM, Thiago Jung Bauermann wrote:
>> PowerPC sPAPR CPUs start in the halted state, and spapr_reset_vcpu()
>> attempts to implement this by setting CPUState::halted to 1. But that's too
>> late for the case of hotplugged CPUs in a machine configure with 2 or more
>> threads per core.
>>
>> By then, other parts of QEMU have already caused the vCPU to run in an
>> unitialized state a couple of times. For example, ppc_cpu_reset() calls
>> ppc_tlb_invalidate_all(), which ends up calling async_run_on_cpu(). This
>> kicks the new vCPU while it has CPUState::halted = 0, causing QEMU to issue
>> a KVM_RUN ioctl on the new vCPU before the guest is able to make the
>> start-cpu RTAS call to initialize its register state.
>>
>> This problem doesn't seem to cause visible issues for regular guests, but
>> on a secure guest running under the Ultravisor it does. The Ultravisor
>> relies on being able to snoop on the start-cpu RTAS call to map vCPUs to
>> guests, and this issue causes it to see a stray vCPU that doesn't belong to
>> any guest.
>>
>> Fix by setting the start-powered-off CPUState property in
>> spapr_create_vcpu(), which makes cpu_common_reset() initialize
>> CPUState::halted to 1 at an earlier moment.
>>
>> Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
>> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
>> ---
>> hw/ppc/spapr_cpu_core.c | 12 +++++++-----
>> 1 file changed, 7 insertions(+), 5 deletions(-)
>>
>> NB: Tested on ppc64le pseries KVM guest with two threads per core.
>> Hot-plugging additional cores doesn't cause the bug described above
>> anymore.
>>
>> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
>> index c4f47dcc04..09feeb5f8f 100644
>> --- a/hw/ppc/spapr_cpu_core.c
>> +++ b/hw/ppc/spapr_cpu_core.c
>> @@ -36,11 +36,6 @@ static void spapr_reset_vcpu(PowerPCCPU *cpu)
>>
>> cpu_reset(cs);
>>
>> - /* 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;
>> -
>> env->spr[SPR_HIOR] = 0;
>>
>> lpcr = env->spr[SPR_LPCR];
>> @@ -288,6 +283,13 @@ static PowerPCCPU *spapr_create_vcpu(SpaprCpuCore *sc, int i, Error **errp)
>>
>> cpu->machine_data = g_new0(SpaprCpuState, 1);
>>
>> + /*
>> + * 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.
>> + */
>> + object_property_set_bool(OBJECT(cs), "start-powered-off", true,
>> + &error_abort);
>
> Since here object_new() is used, it is simpler to set the field before
> the object is realized, similarly to cs->cpu_index:
>
> -- >8 --
> @@ -275,6 +275,11 @@ static PowerPCCPU *spapr_create_vcpu(SpaprCpuCore
> *sc, int i, Error **errp)
> cs = CPU(obj);
> cpu = POWERPC_CPU(obj);
> cs->cpu_index = cc->core_id + i;
> + /*
> + * 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->start_powered_off = true;
> spapr_set_vcpu_id(cpu, cs->cpu_index, &local_err);
> if (local_err) {
> goto err;
> ---
Good point. I adopted your suggestion.
--
Thiago Jung Bauermann
IBM Linux Technology Center
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 5/9] mips/cps: Use start-powered-off CPUState property
2020-07-22 7:09 ` Philippe Mathieu-Daudé
@ 2020-07-23 0:42 ` Thiago Jung Bauermann
0 siblings, 0 replies; 32+ messages in thread
From: Thiago Jung Bauermann @ 2020-07-23 0:42 UTC (permalink / raw)
To: qemu-ppc
Cc: Peter Maydell, Aleksandar Rikalo, Eduardo Habkost,
David Hildenbrand, Cornelia Huck, qemu-devel,
Aleksandar Markovic, qemu-s390x, qemu-arm, Artyom Tarasenko,
Thomas Huth, Paolo Bonzini, Richard Henderson, Aurelien Jarno,
David Gibson
Philippe Mathieu-Daudé <philmd@redhat.com> writes:
> On 7/22/20 5:50 AM, Thiago Jung Bauermann wrote:
>> Instead of setting CPUState::halted to 1 in main_cpu_reset(), use the
>> start-powered-off property which makes cpu_common_reset() initialize it
>> to 1 in common code.
>>
>> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
>> ---
>> hw/mips/cps.c | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> NB: I was only able to test that this patch builds. I wasn't able to
>> run it.
>>
>> diff --git a/hw/mips/cps.c b/hw/mips/cps.c
>> index 615e1a1ad2..d5b6c78019 100644
>> --- a/hw/mips/cps.c
>> +++ b/hw/mips/cps.c
>> @@ -52,9 +52,6 @@ static void main_cpu_reset(void *opaque)
>> CPUState *cs = CPU(cpu);
>>
>> cpu_reset(cs);
>> -
>> - /* All VPs are halted on reset. Leave powering up to CPC. */
>> - cs->halted = 1;
>> }
>>
>> static bool cpu_mips_itu_supported(CPUMIPSState *env)
>> @@ -89,6 +86,9 @@ static void mips_cps_realize(DeviceState *dev, Error **errp)
>> env->itc_tag = mips_itu_get_tag_region(&s->itu);
>> env->itu = &s->itu;
>> }
>> + /* All VPs are halted on reset. Leave powering up to CPC. */
>> + object_property_set_bool(OBJECT(cpu), "start-powered-off", true,
>> + &error_abort);
>
> This is indeed better as now the property is set once, *after* realize
> but *before* reset.
>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Thanks for confirming!
>> qemu_register_reset(main_cpu_reset, cpu);
>> }
>>
>>
--
Thiago Jung Bauermann
IBM Linux Technology Center
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC PATCH v2 7/9] sparc/sun4m: Don't set CPUState::halted in cpu_devinit()
2020-07-22 7:02 ` Philippe Mathieu-Daudé
@ 2020-07-23 0:45 ` Thiago Jung Bauermann
0 siblings, 0 replies; 32+ messages in thread
From: Thiago Jung Bauermann @ 2020-07-23 0:45 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Peter Maydell, Aleksandar Rikalo, Eduardo Habkost,
David Hildenbrand, qemu-s390x, Cornelia Huck, Mark Cave-Ayland,
qemu-devel, Aleksandar Markovic, qemu-arm, qemu-ppc,
Aurelien Jarno, Thomas Huth, Paolo Bonzini, David Gibson,
Artyom Tarasenko, Richard Henderson
Philippe Mathieu-Daudé <philmd@redhat.com> writes:
> On 7/22/20 5:50 AM, Thiago Jung Bauermann wrote:
>> Remove setting of cs->halted from cpu_devinit(), which seems out of place
>> when compared to similar code in other architectures (e.g., ppce500_init()
>> in hw/ppc/e500.c).
>>
>> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
>> ---
>> hw/sparc/sun4m.c | 1 -
>> 1 file changed, 1 deletion(-)
>>
>> NB: I was only able to test that this patch builds. I wasn't able to
>> run it.
>>
>> diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
>> index 766e79bb5e..7b3042a801 100644
>> --- a/hw/sparc/sun4m.c
>> +++ b/hw/sparc/sun4m.c
>> @@ -831,7 +831,6 @@ static void cpu_devinit(const char *cpu_type, unsigned int id,
>> } else {
>> qemu_register_reset(secondary_cpu_reset, cpu);
>> cs = CPU(cpu);
>> - cs->halted = 1;
>> object_property_set_bool(OBJECT(cs), "start-powered-off", true,
>> &error_abort);
>> }
>>
>
> Why not squash with previous patch?
I wasn't sure about this change, and it's also not strictly necessary
for this patch set so I wanted to make it easy for maintainers to not
apply it.
I squashed it for v3.
--
Thiago Jung Bauermann
IBM Linux Technology Center
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC PATCH v2 8/9] sparc/sun4m: Use one cpu_reset() function for main and secondary CPUs
2020-07-22 7:22 ` Philippe Mathieu-Daudé
@ 2020-07-23 0:48 ` Thiago Jung Bauermann
0 siblings, 0 replies; 32+ messages in thread
From: Thiago Jung Bauermann @ 2020-07-23 0:48 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Peter Maydell, Aleksandar Rikalo, Eduardo Habkost,
David Hildenbrand, qemu-s390x, Cornelia Huck, Mark Cave-Ayland,
qemu-devel, Aleksandar Markovic, qemu-arm, qemu-ppc,
Aurelien Jarno, Thomas Huth, Paolo Bonzini, David Gibson,
Artyom Tarasenko, Richard Henderson
Philippe Mathieu-Daudé <philmd@redhat.com> writes:
> On 7/22/20 5:50 AM, Thiago Jung Bauermann wrote:
>> If we rely on cpu_common_reset() setting CPUState::halted according to the
>> start-powered-off property, both reset functions become equivalent and we
>> can use only one.
>>
>> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
>> ---
>> hw/sparc/sun4m.c | 21 ++++-----------------
>> 1 file changed, 4 insertions(+), 17 deletions(-)
>>
>> NB: I was only able to test that this patch builds. I wasn't able to
>> run it.
>>
>> diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
>> index 7b3042a801..deb5e9f027 100644
>> --- a/hw/sparc/sun4m.c
>> +++ b/hw/sparc/sun4m.c
>> @@ -218,16 +218,7 @@ static void dummy_cpu_set_irq(void *opaque, int irq, int level)
>> {
>> }
>>
>> -static void main_cpu_reset(void *opaque)
>> -{
>> - SPARCCPU *cpu = opaque;
>> - CPUState *cs = CPU(cpu);
>> -
>> - cpu_reset(cs);
>> - cs->halted = 0;
>> -}
>> -
>> -static void secondary_cpu_reset(void *opaque)
>> +static void sun4m_cpu_reset(void *opaque)
>> {
>> SPARCCPU *cpu = opaque;
>> CPUState *cs = CPU(cpu);
>> @@ -818,7 +809,6 @@ static const TypeInfo ram_info = {
>> static void cpu_devinit(const char *cpu_type, unsigned int id,
>> uint64_t prom_addr, qemu_irq **cpu_irqs)
>> {
>> - CPUState *cs;
>> SPARCCPU *cpu;
>> CPUSPARCState *env;
>>
>> @@ -826,12 +816,9 @@ static void cpu_devinit(const char *cpu_type, unsigned int id,
>> env = &cpu->env;
>>
>> cpu_sparc_set_id(env, id);
>> - if (id == 0) {
>> - qemu_register_reset(main_cpu_reset, cpu);
>
> IMO it is easier to review this patch in 2, first drop main_cpu_reset
> as it is pointless (we rely on cpu_common_reset), then set the
> "start-powered-off" property and drop secondary_cpu_reset().
That's a good idea. I made those patches for v3.
>> - } else {
>> - qemu_register_reset(secondary_cpu_reset, cpu);
>> - cs = CPU(cpu);
>> - object_property_set_bool(OBJECT(cs), "start-powered-off", true,
>> + qemu_register_reset(sun4m_cpu_reset, cpu);
>
> Why do you still keep it?
I didn't know that not registering a reset function would cause
cpu_reset() to be caused.
>> + if (id != 0) {
>> + object_property_set_bool(OBJECT(cpu), "start-powered-off", true,
>> &error_abort);
>
> At this point the CPU is realized, so this is correct.
Great. Thanks for confirming!
> I'd use directly:
>
> object_property_set_bool(OBJECT(cpu), "start-powered-off", !!id,
> &error_abort);
I used a slight variation of your suggestion, with `id != 0` instead of
`!!id` because I think it makes the code easier to read.
--
Thiago Jung Bauermann
IBM Linux Technology Center
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC PATCH v2 9/9] target/s390x: Use start-powered-off CPUState property
2020-07-22 7:06 ` Philippe Mathieu-Daudé
@ 2020-07-23 0:50 ` Thiago Jung Bauermann
0 siblings, 0 replies; 32+ messages in thread
From: Thiago Jung Bauermann @ 2020-07-23 0:50 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Peter Maydell, Aleksandar Rikalo, Eduardo Habkost,
David Hildenbrand, qemu-s390x, Cornelia Huck, Mark Cave-Ayland,
qemu-devel, Aleksandar Markovic, qemu-arm, qemu-ppc,
Aurelien Jarno, Thomas Huth, Paolo Bonzini, David Gibson,
Artyom Tarasenko, Richard Henderson
Philippe Mathieu-Daudé <philmd@redhat.com> writes:
> On 7/22/20 5:50 AM, Thiago Jung Bauermann wrote:
>> Instead of setting CPUState::halted to 1 in s390_cpu_initfn(), use the
>> start-powered-off property which makes cpu_common_reset() initialize it to
>> 1 in common code.
>>
>> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
>> ---
>> target/s390x/cpu.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> NB: I was only able to test that this patch builds. I wasn't able to
>> run it.
>>
>> diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
>> index 08eb674d22..d3a14af1d9 100644
>> --- a/target/s390x/cpu.c
>> +++ b/target/s390x/cpu.c
>> @@ -291,7 +291,8 @@ static void s390_cpu_initfn(Object *obj)
>> S390CPU *cpu = S390_CPU(obj);
>>
>> cpu_set_cpustate_pointers(cpu);
>> - cs->halted = 1;
>> + object_property_set_bool(OBJECT(cs), "start-powered-off", true,
>> + &error_abort);
>
> Here this seems overkill since this is the same object, so you can
> directly do:
>
> + cs->start_powered_off = true;
I adopted your suggestion.
--
Thiago Jung Bauermann
IBM Linux Technology Center
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC PATCH v2 9/9] target/s390x: Use start-powered-off CPUState property
2020-07-22 17:00 ` Eduardo Habkost
@ 2020-07-23 0:51 ` Thiago Jung Bauermann
0 siblings, 0 replies; 32+ messages in thread
From: Thiago Jung Bauermann @ 2020-07-23 0:51 UTC (permalink / raw)
To: Eduardo Habkost
Cc: Peter Maydell, Aleksandar Rikalo, Aleksandar Markovic,
Cornelia Huck, Mark Cave-Ayland, qemu-devel, qemu-s390x,
qemu-arm, qemu-ppc, Artyom Tarasenko, Thomas Huth, Paolo Bonzini,
David Hildenbrand, Richard Henderson, Aurelien Jarno,
David Gibson
Eduardo Habkost <ehabkost@redhat.com> writes:
> On Wed, Jul 22, 2020 at 12:50:16AM -0300, Thiago Jung Bauermann wrote:
>> Instead of setting CPUState::halted to 1 in s390_cpu_initfn(), use the
>> start-powered-off property which makes cpu_common_reset() initialize it to
>> 1 in common code.
>>
>> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
>> ---
>> target/s390x/cpu.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> NB: I was only able to test that this patch builds. I wasn't able to
>> run it.
>>
>> diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
>> index 08eb674d22..d3a14af1d9 100644
>> --- a/target/s390x/cpu.c
>> +++ b/target/s390x/cpu.c
>> @@ -291,7 +291,8 @@ static void s390_cpu_initfn(Object *obj)
>> S390CPU *cpu = S390_CPU(obj);
>>
>> cpu_set_cpustate_pointers(cpu);
>> - cs->halted = 1;
>> + object_property_set_bool(OBJECT(cs), "start-powered-off", true,
>> + &error_abort);
>
> Is this really OK? s390 CPUs don't seem to set halted=1 on reset
> today.
Hm, good point. That is indeed a behavior change that this patch
introduces. I'll point it out in the description for v3, and if it's
wrong then this patch can simply be dropped.
--
Thiago Jung Bauermann
IBM Linux Technology Center
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 1/9] target/arm: Move start-powered-off property to generic CPUState
2020-07-22 3:50 ` [PATCH v2 1/9] target/arm: Move start-powered-off property to generic CPUState Thiago Jung Bauermann
2020-07-22 6:52 ` Philippe Mathieu-Daudé
@ 2020-07-23 0:55 ` David Gibson
2020-07-23 3:17 ` Thiago Jung Bauermann
1 sibling, 1 reply; 32+ messages in thread
From: David Gibson @ 2020-07-23 0:55 UTC (permalink / raw)
To: Thiago Jung Bauermann
Cc: Peter Maydell, Aleksandar Rikalo, Eduardo Habkost,
Aleksandar Markovic, Cornelia Huck, Mark Cave-Ayland, qemu-devel,
qemu-s390x, qemu-arm, qemu-ppc, Artyom Tarasenko, Thomas Huth,
Paolo Bonzini, David Hildenbrand, Aurelien Jarno,
Richard Henderson
[-- Attachment #1: Type: text/plain, Size: 4682 bytes --]
On Wed, Jul 22, 2020 at 12:50:08AM -0300, Thiago Jung Bauermann wrote:
> There are other platforms which also have CPUs that start powered off, so
> generalize the start-powered-off property so that it can be used by them.
>
> Note that ARMv7MState also has a property of the same name but this patch
> doesn't change it because that class isn't a subclass of CPUState so it
> wouldn't be a trivial change.
>
> This change should not cause any change in behavior.
>
> Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> exec.c | 1 +
> include/hw/core/cpu.h | 4 ++++
> target/arm/cpu.c | 5 ++---
> target/arm/cpu.h | 3 ---
> target/arm/kvm32.c | 2 +-
> target/arm/kvm64.c | 2 +-
> 6 files changed, 9 insertions(+), 8 deletions(-)
>
> NB: I was only able to test that this patch builds. I wasn't able to
> run it.
>
> diff --git a/exec.c b/exec.c
> index 6f381f98e2..82e82fab09 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -899,6 +899,7 @@ Property cpu_common_props[] = {
> DEFINE_PROP_LINK("memory", CPUState, memory, TYPE_MEMORY_REGION,
> MemoryRegion *),
> #endif
> + DEFINE_PROP_BOOL("start-powered-off", CPUState, start_powered_off, false),
> DEFINE_PROP_END_OF_LIST(),
> };
>
> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
> index 8f145733ce..9fc2696db5 100644
> --- a/include/hw/core/cpu.h
> +++ b/include/hw/core/cpu.h
> @@ -374,6 +374,10 @@ struct CPUState {
> bool created;
> bool stop;
> bool stopped;
> +
> + /* Should CPU start in powered-off state? */
> + bool start_powered_off;
> +
> bool unplug;
> bool crash_occurred;
> bool exit_request;
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index 111579554f..ec65c7653f 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -174,8 +174,8 @@ static void arm_cpu_reset(DeviceState *dev)
> env->vfp.xregs[ARM_VFP_MVFR1] = cpu->isar.mvfr1;
> env->vfp.xregs[ARM_VFP_MVFR2] = cpu->isar.mvfr2;
>
> - cpu->power_state = cpu->start_powered_off ? PSCI_OFF : PSCI_ON;
> - s->halted = cpu->start_powered_off;
> + cpu->power_state = s->start_powered_off ? PSCI_OFF : PSCI_ON;
> + s->halted = s->start_powered_off;
>
> if (arm_feature(env, ARM_FEATURE_IWMMXT)) {
> env->iwmmxt.cregs[ARM_IWMMXT_wCID] = 0x69051000 | 'Q';
> @@ -2182,7 +2182,6 @@ static const ARMCPUInfo arm_cpus[] = {
> };
>
> static Property arm_cpu_properties[] = {
> - DEFINE_PROP_BOOL("start-powered-off", ARMCPU, start_powered_off, false),
> DEFINE_PROP_UINT32("psci-conduit", ARMCPU, psci_conduit, 0),
> DEFINE_PROP_UINT64("midr", ARMCPU, midr, 0),
> DEFINE_PROP_UINT64("mp-affinity", ARMCPU,
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index 9e8ed423ea..a925d26996 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -810,9 +810,6 @@ struct ARMCPU {
> */
> uint32_t psci_version;
>
> - /* Should CPU start in PSCI powered-off state? */
> - bool start_powered_off;
> -
> /* Current power state, access guarded by BQL */
> ARMPSCIState power_state;
>
> diff --git a/target/arm/kvm32.c b/target/arm/kvm32.c
> index 0af46b41c8..1f2b8f8b7a 100644
> --- a/target/arm/kvm32.c
> +++ b/target/arm/kvm32.c
> @@ -218,7 +218,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
>
> /* Determine init features for this CPU */
> memset(cpu->kvm_init_features, 0, sizeof(cpu->kvm_init_features));
> - if (cpu->start_powered_off) {
> + if (cs->start_powered_off) {
> cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_POWER_OFF;
> }
> if (kvm_check_extension(cs->kvm_state, KVM_CAP_ARM_PSCI_0_2)) {
> diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
> index 1169237905..f8a6d905fb 100644
> --- a/target/arm/kvm64.c
> +++ b/target/arm/kvm64.c
> @@ -775,7 +775,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
>
> /* Determine init features for this CPU */
> memset(cpu->kvm_init_features, 0, sizeof(cpu->kvm_init_features));
> - if (cpu->start_powered_off) {
> + if (cs->start_powered_off) {
> cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_POWER_OFF;
> }
> if (kvm_check_extension(cs->kvm_state, KVM_CAP_ARM_PSCI_0_2)) {
>
--
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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 2/9] target/arm: Move setting of CPU halted state to generic code
2020-07-22 3:50 ` [PATCH v2 2/9] target/arm: Move setting of CPU halted state to generic code Thiago Jung Bauermann
2020-07-22 6:54 ` Philippe Mathieu-Daudé
@ 2020-07-23 0:55 ` David Gibson
1 sibling, 0 replies; 32+ messages in thread
From: David Gibson @ 2020-07-23 0:55 UTC (permalink / raw)
To: Thiago Jung Bauermann
Cc: Peter Maydell, Aleksandar Rikalo, Eduardo Habkost,
Aleksandar Markovic, Cornelia Huck, Mark Cave-Ayland, qemu-devel,
qemu-s390x, qemu-arm, qemu-ppc, Artyom Tarasenko, Thomas Huth,
Paolo Bonzini, David Hildenbrand, Aurelien Jarno,
Richard Henderson
[-- Attachment #1: Type: text/plain, Size: 1762 bytes --]
On Wed, Jul 22, 2020 at 12:50:09AM -0300, Thiago Jung Bauermann wrote:
> This change is in a separate patch because it's not so obvious that it
> won't cause a regression.
>
> Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> hw/core/cpu.c | 2 +-
> target/arm/cpu.c | 1 -
> 2 files changed, 1 insertion(+), 2 deletions(-)
>
> NB: I was only able to test that this patch builds. I wasn't able to
> run it on an ARM machine. I did on a ppc64le pseries KVM guest.
>
> diff --git a/hw/core/cpu.c b/hw/core/cpu.c
> index 594441a150..71bb7859f1 100644
> --- a/hw/core/cpu.c
> +++ b/hw/core/cpu.c
> @@ -258,7 +258,7 @@ static void cpu_common_reset(DeviceState *dev)
> }
>
> cpu->interrupt_request = 0;
> - cpu->halted = 0;
> + cpu->halted = cpu->start_powered_off;
> cpu->mem_io_pc = 0;
> cpu->icount_extra = 0;
> atomic_set(&cpu->icount_decr_ptr->u32, 0);
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index ec65c7653f..b6c65e4df6 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -175,7 +175,6 @@ static void arm_cpu_reset(DeviceState *dev)
> env->vfp.xregs[ARM_VFP_MVFR2] = cpu->isar.mvfr2;
>
> cpu->power_state = s->start_powered_off ? PSCI_OFF : PSCI_ON;
> - s->halted = s->start_powered_off;
>
> if (arm_feature(env, ARM_FEATURE_IWMMXT)) {
> env->iwmmxt.cregs[ARM_IWMMXT_wCID] = 0x69051000 | 'Q';
>
--
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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 4/9] ppc/e500: Use start-powered-off CPUState property
2020-07-22 3:50 ` [PATCH v2 4/9] ppc/e500: " Thiago Jung Bauermann
2020-07-22 6:55 ` Philippe Mathieu-Daudé
@ 2020-07-23 0:56 ` David Gibson
1 sibling, 0 replies; 32+ messages in thread
From: David Gibson @ 2020-07-23 0:56 UTC (permalink / raw)
To: Thiago Jung Bauermann
Cc: Peter Maydell, Aleksandar Rikalo, Eduardo Habkost,
Aleksandar Markovic, Cornelia Huck, Mark Cave-Ayland, qemu-devel,
qemu-s390x, qemu-arm, qemu-ppc, Artyom Tarasenko, Thomas Huth,
Paolo Bonzini, David Hildenbrand, Aurelien Jarno,
Richard Henderson
[-- Attachment #1: Type: text/plain, Size: 1729 bytes --]
On Wed, Jul 22, 2020 at 12:50:11AM -0300, Thiago Jung Bauermann wrote:
> Instead of setting CPUState::halted to 1 in ppce500_cpu_reset_sec(), use
> the start-powered-off property which makes cpu_common_reset() initialize it
> to 1 in common code.
>
> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> hw/ppc/e500.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> NB: I was only able to test that this patch builds. I wasn't able to
> run it.
>
> diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
> index ab9884e315..dda71bc05d 100644
> --- a/hw/ppc/e500.c
> +++ b/hw/ppc/e500.c
> @@ -704,9 +704,6 @@ static void ppce500_cpu_reset_sec(void *opaque)
>
> cpu_reset(cs);
>
> - /* Secondary CPU starts in halted state for now. Needs to change when
> - implementing non-kernel boot. */
> - cs->halted = 1;
> cs->exception_index = EXCP_HLT;
> }
>
> @@ -897,6 +894,13 @@ void ppce500_init(MachineState *machine)
> } else {
> /* Secondary CPUs */
> qemu_register_reset(ppce500_cpu_reset_sec, cpu);
> +
> + /*
> + * Secondary CPU starts in halted state for now. Needs to change
> + * when implementing non-kernel boot.
> + */
> + object_property_set_bool(OBJECT(cs), "start-powered-off", true,
> + &error_abort);
> }
> }
>
>
--
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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 5/9] mips/cps: Use start-powered-off CPUState property
2020-07-22 3:50 ` [PATCH v2 5/9] mips/cps: " Thiago Jung Bauermann
2020-07-22 7:09 ` Philippe Mathieu-Daudé
@ 2020-07-23 0:57 ` David Gibson
1 sibling, 0 replies; 32+ messages in thread
From: David Gibson @ 2020-07-23 0:57 UTC (permalink / raw)
To: Thiago Jung Bauermann
Cc: Peter Maydell, Aleksandar Rikalo, Eduardo Habkost,
Aleksandar Markovic, Cornelia Huck, Mark Cave-Ayland, qemu-devel,
qemu-s390x, qemu-arm, qemu-ppc, Artyom Tarasenko, Thomas Huth,
Paolo Bonzini, David Hildenbrand, Aurelien Jarno,
Richard Henderson
[-- Attachment #1: Type: text/plain, Size: 1642 bytes --]
On Wed, Jul 22, 2020 at 12:50:12AM -0300, Thiago Jung Bauermann wrote:
> Instead of setting CPUState::halted to 1 in main_cpu_reset(), use the
> start-powered-off property which makes cpu_common_reset() initialize it
> to 1 in common code.
>
> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> hw/mips/cps.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> NB: I was only able to test that this patch builds. I wasn't able to
> run it.
>
> diff --git a/hw/mips/cps.c b/hw/mips/cps.c
> index 615e1a1ad2..d5b6c78019 100644
> --- a/hw/mips/cps.c
> +++ b/hw/mips/cps.c
> @@ -52,9 +52,6 @@ static void main_cpu_reset(void *opaque)
> CPUState *cs = CPU(cpu);
>
> cpu_reset(cs);
> -
> - /* All VPs are halted on reset. Leave powering up to CPC. */
> - cs->halted = 1;
> }
>
> static bool cpu_mips_itu_supported(CPUMIPSState *env)
> @@ -89,6 +86,9 @@ static void mips_cps_realize(DeviceState *dev, Error **errp)
> env->itc_tag = mips_itu_get_tag_region(&s->itu);
> env->itu = &s->itu;
> }
> + /* All VPs are halted on reset. Leave powering up to CPC. */
> + object_property_set_bool(OBJECT(cpu), "start-powered-off", true,
> + &error_abort);
> qemu_register_reset(main_cpu_reset, cpu);
> }
>
>
--
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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 6/9] sparc/sun4m: Use start-powered-off CPUState property
2020-07-22 3:50 ` [PATCH v2 6/9] sparc/sun4m: " Thiago Jung Bauermann
@ 2020-07-23 0:57 ` David Gibson
0 siblings, 0 replies; 32+ messages in thread
From: David Gibson @ 2020-07-23 0:57 UTC (permalink / raw)
To: Thiago Jung Bauermann
Cc: Peter Maydell, Aleksandar Rikalo, Eduardo Habkost,
Aleksandar Markovic, Cornelia Huck, Mark Cave-Ayland, qemu-devel,
qemu-s390x, qemu-arm, qemu-ppc, Artyom Tarasenko, Thomas Huth,
Paolo Bonzini, David Hildenbrand, Aurelien Jarno,
Richard Henderson
[-- Attachment #1: Type: text/plain, Size: 1573 bytes --]
On Wed, Jul 22, 2020 at 12:50:13AM -0300, Thiago Jung Bauermann wrote:
> Instead of setting CPUState::halted to 1 in secondary_cpu_reset(), use the
> start-powered-off property which makes cpu_common_reset() initialize it
> to 1 in common code.
>
> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> hw/sparc/sun4m.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> NB: I was only able to test that this patch builds. I wasn't able to
> run it.
>
> diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
> index 9be930415f..766e79bb5e 100644
> --- a/hw/sparc/sun4m.c
> +++ b/hw/sparc/sun4m.c
> @@ -233,7 +233,6 @@ static void secondary_cpu_reset(void *opaque)
> CPUState *cs = CPU(cpu);
>
> cpu_reset(cs);
> - cs->halted = 1;
> }
>
> static void cpu_halt_signal(void *opaque, int irq, int level)
> @@ -833,6 +832,8 @@ static void cpu_devinit(const char *cpu_type, unsigned int id,
> qemu_register_reset(secondary_cpu_reset, cpu);
> cs = CPU(cpu);
> cs->halted = 1;
> + object_property_set_bool(OBJECT(cs), "start-powered-off", true,
> + &error_abort);
> }
> *cpu_irqs = qemu_allocate_irqs(cpu_set_irq, cpu, MAX_PILS);
> env->prom_addr = prom_addr;
>
--
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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 1/9] target/arm: Move start-powered-off property to generic CPUState
2020-07-23 0:55 ` David Gibson
@ 2020-07-23 3:17 ` Thiago Jung Bauermann
0 siblings, 0 replies; 32+ messages in thread
From: Thiago Jung Bauermann @ 2020-07-23 3:17 UTC (permalink / raw)
To: David Gibson
Cc: Peter Maydell, Aleksandar Rikalo, Eduardo Habkost,
Aleksandar Markovic, Cornelia Huck, Mark Cave-Ayland, qemu-devel,
qemu-s390x, qemu-arm, qemu-ppc, Artyom Tarasenko, Thomas Huth,
Paolo Bonzini, David Hildenbrand, Aurelien Jarno,
Richard Henderson
Hello David,
David Gibson <david@gibson.dropbear.id.au> writes:
> On Wed, Jul 22, 2020 at 12:50:08AM -0300, Thiago Jung Bauermann wrote:
>> There are other platforms which also have CPUs that start powered off, so
>> generalize the start-powered-off property so that it can be used by them.
>>
>> Note that ARMv7MState also has a property of the same name but this patch
>> doesn't change it because that class isn't a subclass of CPUState so it
>> wouldn't be a trivial change.
>>
>> This change should not cause any change in behavior.
>>
>> Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
>> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
>
> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Thank you very much for your review!
Unfortunately I apparently had a minor email mishap and only got your
reviews after I sent the v3 patches, so I wasn't able to put your
Reviewed-by's in them. Sorry about that.
--
Thiago Jung Bauermann
IBM Linux Technology Center
^ permalink raw reply [flat|nested] 32+ messages in thread
end of thread, other threads:[~2020-07-23 3:19 UTC | newest]
Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-22 3:50 [PATCH v2 0/9] Generalize start-powered-off property from ARM Thiago Jung Bauermann
2020-07-22 3:50 ` [PATCH v2 1/9] target/arm: Move start-powered-off property to generic CPUState Thiago Jung Bauermann
2020-07-22 6:52 ` Philippe Mathieu-Daudé
2020-07-23 0:38 ` Thiago Jung Bauermann
2020-07-23 0:55 ` David Gibson
2020-07-23 3:17 ` Thiago Jung Bauermann
2020-07-22 3:50 ` [PATCH v2 2/9] target/arm: Move setting of CPU halted state to generic code Thiago Jung Bauermann
2020-07-22 6:54 ` Philippe Mathieu-Daudé
2020-07-23 0:55 ` David Gibson
2020-07-22 3:50 ` [PATCH v2 3/9] ppc/spapr: Use start-powered-off CPUState property Thiago Jung Bauermann
2020-07-22 7:13 ` Philippe Mathieu-Daudé
2020-07-23 0:40 ` Thiago Jung Bauermann
2020-07-22 3:50 ` [PATCH v2 4/9] ppc/e500: " Thiago Jung Bauermann
2020-07-22 6:55 ` Philippe Mathieu-Daudé
2020-07-23 0:56 ` David Gibson
2020-07-22 3:50 ` [PATCH v2 5/9] mips/cps: " Thiago Jung Bauermann
2020-07-22 7:09 ` Philippe Mathieu-Daudé
2020-07-23 0:42 ` Thiago Jung Bauermann
2020-07-23 0:57 ` David Gibson
2020-07-22 3:50 ` [PATCH v2 6/9] sparc/sun4m: " Thiago Jung Bauermann
2020-07-23 0:57 ` David Gibson
2020-07-22 3:50 ` [RFC PATCH v2 7/9] sparc/sun4m: Don't set CPUState::halted in cpu_devinit() Thiago Jung Bauermann
2020-07-22 7:02 ` Philippe Mathieu-Daudé
2020-07-23 0:45 ` Thiago Jung Bauermann
2020-07-22 3:50 ` [RFC PATCH v2 8/9] sparc/sun4m: Use one cpu_reset() function for main and secondary CPUs Thiago Jung Bauermann
2020-07-22 7:22 ` Philippe Mathieu-Daudé
2020-07-23 0:48 ` Thiago Jung Bauermann
2020-07-22 3:50 ` [RFC PATCH v2 9/9] target/s390x: Use start-powered-off CPUState property Thiago Jung Bauermann
2020-07-22 7:06 ` Philippe Mathieu-Daudé
2020-07-23 0:50 ` Thiago Jung Bauermann
2020-07-22 17:00 ` Eduardo Habkost
2020-07-23 0:51 ` Thiago Jung Bauermann
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.