[SLOF] [PATCH] slof: Only close stdout for virtio-serial devices
aik at ozlabs.ru
Fri Mar 27 12:04:00 AEDT 2020
On 26/03/2020 21:32, Greg Kurz wrote:
> Recent commit cf28264196e5 fixed an issue where a virtio-serial device
> wouldn't shutdown properly during quiesce. The fix is to close stdout
> just before quiesce. As expected this causes some messages to not
> appear anymore, like the well known ones from prom_init():
> Quiescing Open Firmware ...
> Booting Linux via __start() @ 0x0000000002000000 ...
> Actually all messages are discarded until the OS driver finally takes
> control of the device, which may represent a fair amount of logging.
> This is suboptimal but this still better than hanging in SLOF.
> The hammer is a bit too big though because the change also affects
> spapr-vty based consoles, which have no reason to stop working
> after quiesce.
> Move the hack from the common code to the virtio-serial code so that
> it doesn't affect other device types anymore. Register a quiesce hook
> that closes stdout in virtio-serial.fs.
> While here, as suggested by Segher, bring back some robustness in the
> shutdown method.
> Reported-by: Fabiano Rosas <farosas at linux.ibm.com>
> Fixes: cf28264196e5 "virtio-serial: Rework shutdown sequence"
> Signed-off-by: Greg Kurz <groug at kaod.org>
> Loosing so much output with spapr-vty is an annoying regression IMHO.
> I really thing we should have this fixed in QEMU 5.0.
I'll send a pull request shortly but to be fair only two lines with ">"
are missing without the patch in this example:
instantiating rtas at 0x000000002fff0000... done
copying OF device tree...
Building dt strings...
Building dt structure...
Device tree strings 0x000000000b030000 -> 0x000000000b030a2c
Device tree struct 0x000000000b040000 -> 0x000000000b050000
>Quiescing Open Firmware ...
>Booting Linux via __start() @ 0x0000000000400000 ...
CMO not available
> board-qemu/slof/virtio-serial.fs | 14 +++++++++++---
> slof/fs/client.fs | 5 -----
> 2 files changed, 11 insertions(+), 8 deletions(-)
> diff --git a/board-qemu/slof/virtio-serial.fs b/board-qemu/slof/virtio-serial.fs
> index ac55ffcc8ebb..06bfe762b105 100644
> --- a/board-qemu/slof/virtio-serial.fs
> +++ b/board-qemu/slof/virtio-serial.fs
> @@ -19,9 +19,11 @@ virtio-setup-vd VALUE virtiodev
> \ Quiescence the virtqueue of this device so that no more background
> \ transactions can be pending.
> : shutdown ( -- )
> - virtiodev virtio-serial-shutdown
> - FALSE to initialized?
> - 0 to virtiodev
> + initialized? IF
> + virtiodev virtio-serial-shutdown
> + FALSE to initialized?
> + 0 to virtiodev
> + THEN
> : virtio-serial-term-emit
> @@ -31,10 +33,16 @@ 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
> 0 VALUE open-count
> diff --git a/slof/fs/client.fs b/slof/fs/client.fs
> index 76231f9aef75..db7a1925792c 100644
> --- a/slof/fs/client.fs
> +++ b/slof/fs/client.fs
> @@ -203,11 +203,6 @@ ALSO client-voc DEFINITIONS
> \ End of life of SLOF now, call platform quiesce as quiesce
> \ is an undocumented extension and not everybody supports it
> - \ Some device, eg. virtio-serial, need all instances to be
> - \ closed in order to be reset properly
> - s" stdout" get-chosen IF
> - decode-int nip nip close-dev
> - THEN
More information about the SLOF