On 11/21/19 12:10 PM, Cornelia Huck wrote:
> On Wed, 20 Nov 2019 06:43:20 -0500
> Janosch Frank <fran...@linux.ibm.com> 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 <fran...@linux.ibm.com>
>> ---
>> 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
