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 <peter.mayd...@linaro.org>
> ---
> 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 <n...@linux.ibm.com>
> 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()
> };
[...]
