On 11/22/19 1:25 PM, David Hildenbrand wrote: > On 22.11.19 13:22, Janosch Frank wrote: >> On 11/22/19 1:10 PM, Cornelia Huck wrote: >>> On Fri, 22 Nov 2019 12:47:44 +0100 >>> David Hildenbrand <da...@redhat.com> wrote: >>> >>>> On 22.11.19 12:46, Janosch Frank wrote: >>>>> 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 <fran...@linux.ibm.com> >>>>>>> --- >>>>>>> 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? >>>>> >>>> >>>> Because it is not a 1:1 mapping: S390_RESET_EXTERNAL >>>> >>> >>> Tack the explanation onto the definitions of S390_RESET_, then? >>> Probably still quicker than consulting the POP :) >>> >> >> Does it really bother you that much, that I add some explanations to the >> things we're doing. The external reset also gets a comment so Conni >> won't need that much coffee anymore to understand the code :-) >> > > I'm really sorry, but I fail to see how "Subcode 0" is *any* better than > S390_RESET_MODIFIED_CLEAR (and avoids consulting the PoP) and why the > order should matter at all here to make it easier to understand. > > I don't NACK this, I just find it *completely* useless :) >
Ugh, time for some rebase conflicts...
signature.asc
Description: OpenPGP digital signature