[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PATCH v7 0/8] Generalize start-powered-off property from ARM
From: |
Thiago Jung Bauermann |
Subject: |
[PATCH v7 0/8] Generalize start-powered-off property from ARM |
Date: |
Wed, 26 Aug 2020 02:55:27 -0300 |
This version fixes `make check` failures in ppc/e500.c, mips/cps.c and
sparc/sun4m.c. This was done by moving the qdev_realize_and_unref() call as
close as possible to the object_new() call, in order to keep the CPU object
construction as similar as possible to the earlier version which used
cpu_create().
I also had to change the patch which removed the main_cpu_reset() function
from sparc/sun4m.c. It was causing a `make check` failure but I can't
really explain why. See this message for a few more details:
https://lists.nongnu.org/archive/html/qemu-ppc/2020-08/msg00419.html
I dropped the Reviewed-by's on the changed patches because of these
changes.
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 v6:
Patch "ppc/e500: Use start-powered-off CPUState property"
Patch "mips/cps: Use start-powered-off CPUState property"
- Moved setting of start-powered-off property and qdev_realize_and_unref()
to right after object_new(machine->cpu_type).
- Dropped Philippe's Reviewed-by due to this change.
Patch "sparc/sun4m: Remove main_cpu_reset()"
- Dropped.
Patch "sparc/sun4m: Don't set cs->halted = 0 in main_cpu_reset()"
- New patch.
Patch "sparc/sun4m: Use start-powered-off CPUState property"
- Merged secondary_cpu_reset() with main_cpu_reset().
- Dropped Philippe's Reviewed-by due to this change.
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: Don't set cs->halted = 0 in 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 | 15 +++++++++++----
hw/ppc/e500.c | 13 +++++++++----
hw/ppc/spapr_cpu_core.c | 10 +++++-----
hw/sparc/sun4m.c | 26 ++++++--------------------
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, 41 insertions(+), 43 deletions(-)
- [PATCH v7 0/8] Generalize start-powered-off property from ARM,
Thiago Jung Bauermann <=
- [PATCH v7 2/8] target/arm: Move setting of CPU halted state to generic code, Thiago Jung Bauermann, 2020/08/26
- [PATCH v7 1/8] target/arm: Move start-powered-off property to generic CPUState, Thiago Jung Bauermann, 2020/08/26
- [PATCH v7 3/8] ppc/spapr: Use start-powered-off CPUState property, Thiago Jung Bauermann, 2020/08/26
- [PATCH v7 4/8] ppc/e500: Use start-powered-off CPUState property, Thiago Jung Bauermann, 2020/08/26
- [PATCH v7 5/8] mips/cps: Use start-powered-off CPUState property, Thiago Jung Bauermann, 2020/08/26
- [PATCH v7 6/8] sparc/sun4m: Don't set cs->halted = 0 in main_cpu_reset(), Thiago Jung Bauermann, 2020/08/26
- [PATCH v7 7/8] sparc/sun4m: Use start-powered-off CPUState property, Thiago Jung Bauermann, 2020/08/26
- [PATCH v7 8/8] target/s390x: Use start-powered-off CPUState property, Thiago Jung Bauermann, 2020/08/26
- Re: [PATCH v7 0/8] Generalize start-powered-off property from ARM, David Gibson, 2020/08/31