[v3] Generalize start-powered-off property from ARM

[PATCH v3 1/8] target/arm: Move start-powered-off property to generic CPUState

Posted by Thiago Jung Bauermann 5 years, 6 months ago

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>
Reviewed-by: Philippe Mathieu-Daudé <philmd@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)) {

Re: [PATCH v3 1/8] target/arm: Move start-powered-off property to generic CPUState

Posted by David Gibson 5 years, 6 months ago

On Wed, Jul 22, 2020 at 11:56:50PM -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>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@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

Re: [PATCH v3 1/8] target/arm: Move start-powered-off property to generic CPUState

Posted by Thiago Jung Bauermann 5 years, 6 months ago

David Gibson <david@gibson.dropbear.id.au> writes:

> On Wed, Jul 22, 2020 at 11:56:50PM -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>
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
>
> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

Thanks! Sory about the extra work.

-- 
Thiago Jung Bauermann
IBM Linux Technology Center

Re: [PATCH v3 1/8] target/arm: Move start-powered-off property to generic CPUState

Posted by Greg Kurz 5 years, 6 months ago

On Thu, 23 Jul 2020 13:06:09 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Wed, Jul 22, 2020 at 11:56:50PM -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>
> > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> > Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
> 
> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> 

Reviewed-by: Greg Kurz <groug@kaod.org>

> > ---
> >  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)) {
> > 
>

Re: [PATCH v3 1/8] target/arm: Move start-powered-off property to generic CPUState

Posted by Thiago Jung Bauermann 5 years, 6 months ago

Greg Kurz <groug@kaod.org> writes:

> On Thu, 23 Jul 2020 13:06:09 +1000
> David Gibson <david@gibson.dropbear.id.au> wrote:
>
>> On Wed, Jul 22, 2020 at 11:56:50PM -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>
>> > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> > Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
>> 
>> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
>> 
>
> Reviewed-by: Greg Kurz <groug@kaod.org>

Thanks!

-- 
Thiago Jung Bauermann
IBM Linux Technology Center

[PATCH v3 1/8] target/arm: Move start-powered-off property to generic CPUState
[PATCH v3 2/8] target/arm: Move setting of CPU halted state to generic code
[PATCH v3 3/8] ppc/spapr: Use start-powered-off CPUState property
[PATCH v3 4/8] ppc/e500: Use start-powered-off CPUState property
[PATCH v3 5/8] mips/cps: Use start-powered-off CPUState property
[PATCH v3 6/8] sparc/sun4m: Remove main_cpu_reset()
[PATCH v3 7/8] sparc/sun4m: Use start-powered-off CPUState property
[RFC PATCH v3 8/8] target/s390x: Use start-powered-off CPUState property