[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PATCH v6 0/8] Generalize start-powered-off property from ARM
From: |
Thiago Jung Bauermann |
Subject: |
[PATCH v6 0/8] Generalize start-powered-off property from ARM |
Date: |
Wed, 19 Aug 2020 13:42:58 -0300 |
This version has one small fix in patch 7, and adds Philippe's Reviewed-bys.
Applies cleanly on dgibson/ppc-for-5.2.
Original cover letter below, followed by changelog:
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 patch may be wrong, as pointed out by Eduardo, so I marked it as
RFC. It may make sense to drop it.
Changes since v5:
Patch "ppc/e500: Use start-powered-off CPUState property"
Patch "mips/cps: Use start-powered-off CPUState property"
Patch "sparc/sun4m: Remove main_cpu_reset()"
Patch "target/s390x: Use start-powered-off CPUState property"
- Added Philippe's Reviewed-by.
Patch "sparc/sun4m: Use start-powered-off CPUState property"
- Move call to qdev_realize_and_unref() right after object_property_set_bool(),
as suggested by Philippe.
Changes since v4:
Patch "ppc/e500: Use start-powered-off CPUState property"
Patch "sparc/sun4m: Use start-powered-off CPUState property"
- Use qdev_realize_and_unref() instead of qdev_realize(), as suggested
by Igor.
- Pass &error_fatal to qdev_realize_and_unref() instead of manually
reporting the error and exiting QEMU, as suggested by Philippe.
- Changed object_property_set_bool() to use &error_fatal instead of
&error_abort.
Patch "mips/cps: Use start-powered-off CPUState property"
- Use qdev_realize_and_unref() instead of qdev_realize(), as suggested
by Igor.
- Use existing errp argument to propagate error back to the caller, as
suggested by Philippe.
- Changed object_property_set_bool() to use existing errp argument to
propagate error back to the caller instead of using &error_abort.
Changes since v3:
General:
- Added David's, Greg's and Cornelia's Reviewed-by and Acked-by to some
of the patches.
- Rebased on top of dgibson/ppc-for-5.2.
Patch "ppc/e500: Use start-powered-off CPUState property"
Patch "mips/cps: Use start-powered-off CPUState property"
Patch "sparc/sun4m: Use start-powered-off CPUState property"
- Initialize CPU object with object_new() and qdev_realize() instead
of cpu_create().
- Removed Reviewed-by's and Acked-by's from these patches because of
these changes.
Changes since v2:
General:
- Added Philippe's Reviewed-by to some of the patches.
Patch "ppc/spapr: Use start-powered-off CPUState property"
- Set the CPUState::start_powered_off variable directly rather than using
object_property_set_bool(). Suggested by Philippe.
Patch "sparc/sun4m: Remove main_cpu_reset()"
- New patch. Suggested by Philippe.
Patch "sparc/sun4m: Use start-powered-off CPUState property"
- Remove secondary_cpu_reset(). Suggested by Philippe.
- Remove setting of `cs->halted = 1` from cpu_devinit(). Suggested by Philippe.
Patch "Don't set CPUState::halted in cpu_devinit()"
- Squashed into previous patch. Suggested by Philippe.
Patch "sparc/sun4m: Use one cpu_reset() function for main and secondary CPUs"
- Dropped.
Patch "target/s390x: Use start-powered-off CPUState property"
- Set the CPUState::start_powered_off variable directly rather than using
object_property_set_bool(). Suggested by Philippe.
- Mention in the commit message Eduardo's observation that before this
patch, the code didn't set cs->halted on reset.
Thiago Jung Bauermann (8):
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: Remove main_cpu_reset()
sparc/sun4m: Use start-powered-off CPUState property
target/s390x: Use start-powered-off CPUState property
exec.c | 1 +
hw/core/cpu.c | 2 +-
hw/mips/cps.c | 14 ++++++++++----
hw/ppc/e500.c | 14 ++++++++++----
hw/ppc/spapr_cpu_core.c | 10 +++++-----
hw/sparc/sun4m.c | 31 ++++---------------------------
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 | 2 +-
12 files changed, 39 insertions(+), 50 deletions(-)
- [PATCH v6 0/8] Generalize start-powered-off property from ARM,
Thiago Jung Bauermann <=