[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: David Hildenbrand
Subject: Re: [PATCH 4/4] s390x: Beautify machine reset
Date: Fri, 22 Nov 2019 13:25:25 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.1.1

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 <address@hidden> 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 <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)
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: /* 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 :)



David / dhildenb

reply via email to

[Prev in Thread] Current Thread [Next in Thread]