slof: Only close stdout for virtio-serial devices

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@linux.ibm.com>
Fixes: cf28264196e5 "virtio-serial: Rework shutdown sequence"
Signed-off-by: Greg Kurz <groug@kaod.org>
---

David,

Loosing so much output with spapr-vty is an annoying regression IMHO.
I really thing we should have this fixed in QEMU 5.0.
---
 board-qemu/slof/virtio-serial.fs |   14 +++++++++++---
 slof/fs/client.fs                |    5 -----
 2 files changed, 11 insertions(+), 8 deletions(-)

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@linux.ibm.com>
> Fixes: cf28264196e5 "virtio-serial: Rework shutdown sequence"
> Signed-off-by: Greg Kurz <groug@kaod.org>
> ---
> 
> David,
> 
> Loosing so much output with spapr-vty is an annoying regression IMHO.
> I really thing we should have this fixed in QEMU 5.0.



Thanks, applied.


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
prom_hold_cpus: skipped
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 ...
 -> fw_cmo_feature_init()
CMO not available
 <- fw_cmo_feature_init()
 <- pseries_init()
===

Thanks,


> ---
>  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
>  	    close-dev
> -	    \ 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
>  	    quiesce
>  	ELSE
>  	    close-dev
>