| Message ID | 20200723025657.644724-2-bauerman@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | Generalize start-powered-off property from ARM | expand |
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 <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.
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)) { > > >
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!
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)) {