Re: [PATCH 4/4] s390x: Beautify machine reset

[Janosch Frank]

On 11/22/19 11:59 AM, David Hildenbrand wrote:
> On 22.11.19 08:52, Janosch Frank wrote:
>> * Add comments that tell you which diag308 subcode caused the reset
>> * Sort by diag308 reset subcode
>>
>> Signed-off-by: Janosch Frank <address@hidden>
>> ---
>>   hw/s390x/s390-virtio-ccw.c | 20 ++++++++++----------
>>   1 file changed, 10 insertions(+), 10 deletions(-)
>>
>> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
>> index c1d1440272..88f7758721 100644
>> --- a/hw/s390x/s390-virtio-ccw.c
>> +++ b/hw/s390x/s390-virtio-ccw.c
>> @@ -330,15 +330,7 @@ static void s390_machine_reset(MachineState *machine)
>>       s390_cmma_reset();
>>   
>>       switch (reset_type) {
>> -    case S390_RESET_EXTERNAL:
>> -    case S390_RESET_REIPL:
>> -        qemu_devices_reset();
>> -        s390_crypto_reset();
>> -
>> -        /* configure and start the ipl CPU only */
>> -        run_on_cpu(cs, s390_do_cpu_ipl, RUN_ON_CPU_NULL);
>> -        break;
>> -    case S390_RESET_MODIFIED_CLEAR:
>> +    case S390_RESET_MODIFIED_CLEAR: /* Subcode 0 */
> 
> IMHO "Subcode X" isn't of much help here. We're out of diag handling.
> 
> I'd suggest to just document the subcodes along with the definitions, if 
> really needed, and drop this patch, at least I don't quite see the value 
> of moving code around here... or is the code shuffling of any value on 
> your prot virt patches?
> 

It keeps me from consulting the POP every time I need to change things
in the machine resets. This is basically a 1:1 mapping of diag 308
subcodes to machine resets, so why don't we want to make that obvious
and order them by the subcodes?

signature.asc
Description: OpenPGP digital signature