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)) {
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(-)
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
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
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
© 2016 - 2025 Red Hat, Inc.