From 27f045e2d52ee49ddbb375b5593d4fc387aad4b0 Mon Sep 17 00:00:00 2001 From: Jordan Niethe Date: Tue, 29 Aug 2023 10:12:01 +1000 Subject: [PATCH 2/2] virtio-serial: Do not close stdout on quiesce RH-Author: Thomas Huth RH-MergeRequest: 1: Fix SLOF crash when only using virtio console (without spapr-vty) RH-Jira: RHEL-3709 RH-Acked-by: David Gibson (Red Hat) RH-Acked-by: Laurent Vivier RH-Commit: [2/2] cbde239703a11aa2676a12b8415281d37cba54a5 Commit 76fee95 ("slof: Only close stdout for virtio-serial devices") says that commit cf28264 ("virtio-serial: Rework shutdown sequence") fixed a hang. The problem was believed to be that it was necessary to close stdout to shutdown the underlying virtio device. Commit cf28264 ("virtio-serial: Rework shutdown sequence") closed stdout on quiesce. This meant when prom_init() called write on stdout after quiesce, there is a use after free so this is unreliable, and can also hang (especially after reboots). Quiescing is intended to put hardware into a safe state for the client to take over. It is incorrect for SLOF to close ihandles that the client could still be using, even after a quiesce. Rather than closing the stdout device, all that needs to happen is to ensure virtio-serial-shutdown gets called. On quiesce, close the virtio device, but leave the stdout device itself open. Commit 8174acd ("virtio-serial: Close device completely") handles reads and writes as no-ops if the underlying virtio device is closed so there is no problem with the client calling "write" on stdout after this, but no output will be displayed. Fixes: cf28264 ("virtio-serial: Rework shutdown sequence") Debugged-by: Kautuk Consul Co-developed-by: Kautuk Consul Signed-off-by: Kautuk Consul Signed-off-by: Jordan Niethe Reviewed-by: Thomas Huth Signed-off-by: Alexey Kardashevskiy (cherry picked from commit dd4d4ea0add97df078d571b48192adaf7c4b0d87) Jira: https://issues.redhat.com/browse/RHEL-3709 Signed-off-by: Thomas Huth --- board-qemu/slof/virtio-serial.fs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/board-qemu/slof/virtio-serial.fs b/board-qemu/slof/virtio-serial.fs index 41e2e04..de42cc7 100644 --- a/board-qemu/slof/virtio-serial.fs +++ b/board-qemu/slof/virtio-serial.fs @@ -33,16 +33,14 @@ virtio-setup-vd VALUE virtiodev : virtio-serial-term-key? virtiodev virtio-serial-haschar ; : virtio-serial-term-key BEGIN virtio-serial-term-key? UNTIL virtiodev virtio-serial-getchar ; -: virtio-serial-close-stdout s" stdout" get-chosen IF decode-int nip nip close-dev THEN ; - \ Basic device initialization - which has only to be done once : init ( -- ) virtiodev virtio-serial-init drop TRUE to initialized? - \ Linux closes stdin at some point in prom_init(). This internally triggers a - \ quiesce in SLOF. We must ensure stdout gets closed as well otherwise the - \ device cannot be reset properly and the boot will hang. - ['] virtio-serial-close-stdout add-quiesce-xt + \ virtiodev must be shutdown at quiesce so the device is reset properly. + \ The read and write methods can be called after quiesce so must handle + \ virtiodev being closed. + ['] shutdown add-quiesce-xt ; 0 VALUE open-count @@ -62,8 +60,8 @@ virtiodev virtio-serial-init drop open-count 0> IF open-count 1 - dup to open-count 0= IF shutdown THEN + close THEN - close ; : write ( addr len -- actual ) -- 2.41.0