On Tue, 2024-08-13 at 17:52 +0100, Peter Maydell wrote: > Convert the s390 CPU to the Resettable interface. This is slightly > more involved than the other CPU types were (see commits > 9130cade5fc22..d66e64dd006df) because S390 has its own set of > different kinds of reset with different behaviours that it needs to > trigger. > > We handle this by adding these reset types to the Resettable > ResetType enum. Now instead of having an underlying implementation > of reset that is s390-specific and which might be called either > directly or via the DeviceClass::reset method, we can implement only > the Resettable hold phase method, and have the places that need to > trigger an s390-specific reset type do so by calling > resettable_reset(). > > The other option would have been to smuggle in the s390 reset > type via, for instance, a field in the CPU state that we set > in s390_do_cpu_initial_reset() etc and then examined in the > reset method, but doing it this way seems cleaner. > > The motivation for this change is that this is the last caller > of the legacy device_class_set_parent_reset() function, and > removing that will let us clean up some glue code that we added > for the transition to three-phase reset. > > Signed-off-by: Peter Maydell <[email protected]> > --- > Tested with 'make check' and 'make check-avocado' only. The > descriptions of the reset types are borrowed from the commit > message of f5ae2a4fd8d573cfeba; please check them as I haven't > got a clue what s390 does ;-) > ---
With the already mentioned fix: Reviewed-by: Nina Schoetterl-Glausch <[email protected]> > diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c > index 0fbfcd35d83..4e41a3dff59 100644 > --- a/target/s390x/cpu.c > +++ b/target/s390x/cpu.c > [...] > -/* S390CPUClass::reset() */ > -static void s390_cpu_reset(CPUState *s, cpu_reset_type type) > +/* S390CPUClass Resettable reset_hold phase method */ > +static void s390_cpu_reset_hold(Object *obj, ResetType type) > { [...] > > switch (type) { > - case S390_CPU_RESET_CLEAR: > + default: > + /* RESET_TYPE_COLD: power on or "clear" reset */ I'd prefer case RESET_TYPE_COLD: case RESET_TYPE_SNAPSHOT_LOAD: and keeping the default unreachable assert. > memset(env, 0, offsetof(CPUS390XState, start_initial_reset_fields)); > /* fall through */ > - case S390_CPU_RESET_INITIAL: > + case RESET_TYPE_S390_CPU_INITIAL: > /* initial reset does not clear everything! */ > memset(&env->start_initial_reset_fields, 0, > offsetof(CPUS390XState, start_normal_reset_fields) - > @@ -203,7 +206,7 @@ static void s390_cpu_reset(CPUState *s, cpu_reset_type > type) > set_float_detect_tininess(float_tininess_before_rounding, > &env->fpu_status); > /* fall through */ > - case S390_CPU_RESET_NORMAL: > + case RESET_TYPE_S390_CPU_NORMAL: > env->psw.mask &= ~PSW_MASK_RI; > memset(&env->start_normal_reset_fields, 0, > offsetof(CPUS390XState, end_reset_fields) - > @@ -212,20 +215,18 @@ static void s390_cpu_reset(CPUState *s, cpu_reset_type > type) > env->pfault_token = -1UL; > env->bpbc = false; > break; > - default: > - g_assert_not_reached(); > } > > /* Reset state inside the kernel that we cannot access yet from QEMU. */ > if (kvm_enabled()) { > switch (type) { > - case S390_CPU_RESET_CLEAR: > + default: And the same here. > kvm_s390_reset_vcpu_clear(cpu); > break; > - case S390_CPU_RESET_INITIAL: > + case RESET_TYPE_S390_CPU_INITIAL: > kvm_s390_reset_vcpu_initial(cpu); > break; > - case S390_CPU_RESET_NORMAL: > + case RESET_TYPE_S390_CPU_NORMAL: > kvm_s390_reset_vcpu_normal(cpu); > break; > } > @@ -315,12 +316,6 @@ static Property s390x_cpu_properties[] = { > DEFINE_PROP_END_OF_LIST() > }; [...]
