On 11/21/19 12:10 PM, Cornelia Huck wrote: > On Wed, 20 Nov 2019 06:43:20 -0500 > Janosch Frank <[email protected]> wrote: > >> Let's move the resets into one function and switch by type, so we can >> use fallthroughs for shared reset actions. > > Doing that makes sense. > >> >> Signed-off-by: Janosch Frank <[email protected]> >> --- >> hw/s390x/s390-virtio-ccw.c | 3 + >> target/s390x/cpu.c | 111 ++++++++++++++++--------------------- >> 2 files changed, 52 insertions(+), 62 deletions(-) >> >> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c >> index d3edeef0ad..c1d1440272 100644 >> --- a/hw/s390x/s390-virtio-ccw.c >> +++ b/hw/s390x/s390-virtio-ccw.c >> @@ -348,6 +348,9 @@ static void s390_machine_reset(MachineState *machine) >> break; >> case S390_RESET_LOAD_NORMAL: >> CPU_FOREACH(t) { >> + if (t == cs) { >> + continue; >> + } > > Hm, why is this needed now?
The Ultravisor checks which reset is done to which cpu.
So blindly resetting the calling cpu with a normal reset to then do a
clear/initial reset will return an error.
>
>> run_on_cpu(t, s390_do_cpu_reset, RUN_ON_CPU_NULL);
>> }
>> subsystem_reset();
>> diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
>> index 3abe7e80fd..10d5b915d8 100644
>> --- a/target/s390x/cpu.c
>> +++ b/target/s390x/cpu.c
>> @@ -82,67 +82,53 @@ static void s390_cpu_load_normal(CPUState *s)
>> }
>> #endif
>>
>> -/* S390CPUClass::cpu_reset() */
>
> Not sure if it would be worth keeping these comments near by the
> calling functions.
>
>> -static void s390_cpu_reset(CPUState *s)
>> +enum {
>> + S390_CPU_RESET_NORMAL,
>> + S390_CPU_RESET_INITIAL,
>> + S390_CPU_RESET_CLEAR,
>> +};
>
> Maybe make this into a proper type, so you can use type checking?
Ok
>
> (...)
>
> The diff is a bit hard to read, but the change seems fine at a glance.
>
signature.asc
Description: OpenPGP digital signature
