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 :)