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

[David Hildenbrand]

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 <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?

Because it is not a 1:1 mapping: S390_RESET_EXTERNAL

--

Thanks,

David / dhildenb