[v3,1/2] trap signals for "-serial mon:stdio"

From: Paolo Bonzini <pbonzini@redhat.com>

With mon:stdio you can exit the VM by switching to the monitor and
sending the "quit" command.  It is then useful to pass Ctrl-C to the
VM instead of exiting.

This in turn lets us stop tying the default signal handling behavior
to -nographic, removing gratuitous differences between "-display none"
and "-nographic".

This patch changes behavior for "-display none -serial mon:stdio", as
expected, but not for "-display none -serial stdio".

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
---
V2: added code comments and documentation fixes by mjt
  (hopefully the s-o-b stands still)
V3: documentation fix, no code changed

 qemu-char.c     |   13 +++++++++----
 qemu-options.hx |    8 +++++---
 2 files changed, 14 insertions(+), 7 deletions(-)

Ping?

03.07.2013 20:29, Michael Tokarev wrote:
> From: Paolo Bonzini <pbonzini@redhat.com>
> 
> With mon:stdio you can exit the VM by switching to the monitor and
> sending the "quit" command.  It is then useful to pass Ctrl-C to the
> VM instead of exiting.
> 
> This in turn lets us stop tying the default signal handling behavior
> to -nographic, removing gratuitous differences between "-display none"
> and "-nographic".
> 
> This patch changes behavior for "-display none -serial mon:stdio", as
> expected, but not for "-display none -serial stdio".
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
> ---
> V2: added code comments and documentation fixes by mjt
>   (hopefully the s-o-b stands still)
> V3: documentation fix, no code changed
> 
>  qemu-char.c     |   13 +++++++++----
>  qemu-options.hx |    8 +++++---
>  2 files changed, 14 insertions(+), 7 deletions(-)
> 
> diff --git a/qemu-char.c b/qemu-char.c
> index 6cec5d7..18c42a3 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -926,7 +926,6 @@ static void qemu_chr_set_echo_stdio(CharDriverState *chr, bool echo)
>          tty.c_cc[VMIN] = 1;
>          tty.c_cc[VTIME] = 0;
>      }
> -    /* if graphical mode, we allow Ctrl-C handling */
>      if (!stdio_allow_signal)
>          tty.c_lflag &= ~ISIG;
>  
> @@ -955,7 +954,6 @@ static CharDriverState *qemu_chr_open_stdio(ChardevStdio *opts)
>      chr = qemu_chr_open_fd(0, 1);
>      chr->chr_close = qemu_chr_close_stdio;
>      chr->chr_set_echo = qemu_chr_set_echo_stdio;
> -    stdio_allow_signal = display_type != DT_NOGRAPHIC;
>      if (opts->has_signal) {
>          stdio_allow_signal = opts->signal;
>      }
> @@ -2932,6 +2930,14 @@ QemuOpts *qemu_chr_parse_compat(const char *label, const char *filename)
>      if (strstart(filename, "mon:", &p)) {
>          filename = p;
>          qemu_opt_set(opts, "mux", "on");
> +        if (strcmp(filename, "stdio") == 0) {
> +            /* Monitor is muxed to stdio: do not exit on Ctrl+C by default
> +             * but pass it to the guest.  Handle this only for compat syntax,
> +             * for -chardev syntax we have special option for this.
> +             * This is what -nographic did, redirecting+muxing serial+monitor
> +             * to stdio causing Ctrl+C to be passed to guest. */
> +            qemu_opt_set(opts, "signal", "off");
> +        }
>      }
>  
>      if (strcmp(filename, "null")    == 0 ||
> @@ -3060,8 +3066,7 @@ static void qemu_chr_parse_stdio(QemuOpts *opts, ChardevBackend *backend,
>  {
>      backend->stdio = g_new0(ChardevStdio, 1);
>      backend->stdio->has_signal = true;
> -    backend->stdio->signal =
> -        qemu_opt_get_bool(opts, "signal", display_type != DT_NOGRAPHIC);
> +    backend->stdio->signal = qemu_opt_get_bool(opts, "signal", true);
>  }
>  
>  static void qemu_chr_parse_serial(QemuOpts *opts, ChardevBackend *backend,
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 137a39b..7cc4d8e 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -842,7 +842,8 @@ STEXI
>  Normally, QEMU uses SDL to display the VGA output. With this option,
>  you can totally disable graphical output so that QEMU is a simple
>  command line application. The emulated serial port is redirected on
> -the console. Therefore, you can still use QEMU to debug a Linux kernel
> +the console and muxed with the monitor (unless redirected elsewhere
> +explicitly). Therefore, you can still use QEMU to debug a Linux kernel
>  with a serial console.
>  ETEXI
>  
> @@ -2485,14 +2486,15 @@ same as if you had specified @code{-serial tcp} except the unix domain socket
>  @item mon:@var{dev_string}
>  This is a special option to allow the monitor to be multiplexed onto
>  another serial port.  The monitor is accessed with key sequence of
> -@key{Control-a} and then pressing @key{c}. See monitor access
> -@ref{pcsys_keys} in the -nographic section for more keys.
> +@key{Control-a} and then pressing @key{c}.
>  @var{dev_string} should be any one of the serial devices specified
>  above.  An example to multiplex the monitor onto a telnet server
>  listening on port 4444 would be:
>  @table @code
>  @item -serial mon:telnet::4444,server,nowait
>  @end table
> +When monitor is multiplexed to stdio this way, Ctrl+C will not terminate
> +QEMU anymore but will be passed to the guest instead.
>  
>  @item braille
>  Braille device.  This will use BrlAPI to display the braille output on a real
>

Am 09.07.2013 09:39, schrieb Michael Tokarev:
> Ping?

Have you tested -semihosting? The xtensa test image on the Wiki uses
-nographic, but semihosting content always seems to go to stdout without
going through a chardev IIUC...

Andreas

09.07.2013 12:42, Andreas Färber wrote:
> Am 09.07.2013 09:39, schrieb Michael Tokarev:
>> Ping?
> 
> Have you tested -semihosting? The xtensa test image on the Wiki uses
> -nographic, but semihosting content always seems to go to stdout without
> going through a chardev IIUC...

Thanks for the info. I've no idea what is -semihosting, what is xtensa
and that we have xtensa test image on the wiki.  I'll try for sure,
once I can find that image (googling for xtensa image site:qemu.org
does not help much, after failing searching in qemu wiki).  Maybe
you can give some hint?

/mjt

On Tue, Jul 9, 2013 at 12:42 PM, Andreas Färber <afaerber@suse.de> wrote:
> Am 09.07.2013 09:39, schrieb Michael Tokarev:
>> Ping?
>
> Have you tested -semihosting? The xtensa test image on the Wiki uses
> -nographic, but semihosting content always seems to go to stdout without
> going through a chardev IIUC...

Andreas,

xtensa semihosting allows guest privileged code to work with host's file
descriptors doing open/read/write/lseek/close. Linux sim machine uses it
to implement console (with standard 0/1 file descriptors), simulated disks
and networking. Do you think it's worth tying these descriptors with a
chardev? If so, I'm probably the right person to do it (:

Hi Max,

Am 09.07.2013 13:00, schrieb Max Filippov:
> On Tue, Jul 9, 2013 at 12:42 PM, Andreas Färber <afaerber@suse.de> wrote:
>> Am 09.07.2013 09:39, schrieb Michael Tokarev:
>>> Ping?
>>
>> Have you tested -semihosting? The xtensa test image on the Wiki uses
>> -nographic, but semihosting content always seems to go to stdout without
>> going through a chardev IIUC...
> 
> xtensa semihosting allows guest privileged code to work with host's file
> descriptors doing open/read/write/lseek/close. Linux sim machine uses it
> to implement console (with standard 0/1 file descriptors), simulated disks
> and networking. Do you think it's worth tying these descriptors with a
> chardev? If so, I'm probably the right person to do it (:

Paolo's/Michael's patches are relying on -serial stdio,signal=on as a
replacement implementation for -nographic, so I raised the question of
whether that would work as expected for -semihosting in general. I do
not have test images for arm or m68k semihosting, so I cannot tell
whether xtensa should be doing something differently.

Peter and me seemed to agree that unicore32 instructions shouldn't mess
with the host's ncurses directly, but Blue apparently didn't agree and
applied it (and I guess neither of us has access to a test image to see
it in action).

stdio seems less intrusive compared to that, but I did have issues with
input at the xtensa login prompt going to the monitor instead (I use
-qmp for all my test machines to inspect the QOM tree, so I needed to
specify -monitor mon:stdio manually); -display none -monitor none -qmp
unix:... worked better for me.

Cheers,
Andreas

09.07.2013 12:42, Andreas Färber wrote:
> Am 09.07.2013 09:39, schrieb Michael Tokarev:
>> Ping?
> 
> Have you tested -semihosting? The xtensa test image on the Wiki uses
> -nographic, but semihosting content always seems to go to stdout without
> going through a chardev IIUC...

Ok.  I tried the xtensa image from wiki.qemu.org/Testing , and it
appears to work with or without the changes in question.  Which,
I think, is wrong, because it is a bug of the same theme - with
-daemonize we can't do -semihosting due to std file descriptors
being redirected to /dev/null.  So it looks like there's another
bug of waiting to hunt, but I really am a wrong person to do that,
maybe Max Filippov can take a look.

Thanks,

/mjt

On 9 July 2013 13:02, Michael Tokarev <mjt@tls.msk.ru> wrote:
> 09.07.2013 12:42, Andreas Färber wrote:
>> Am 09.07.2013 09:39, schrieb Michael Tokarev:
>>> Ping?
>>
>> Have you tested -semihosting? The xtensa test image on the Wiki uses
>> -nographic, but semihosting content always seems to go to stdout without
>> going through a chardev IIUC...

This is I think correct, or at least not worth trying to
change to make it talk to a chardev. The semihosting ABI
in this area is intended primarily for debugging and similar
purposes. I don't think this patch should adversely affect
semihosting users.

> Ok.  I tried the xtensa image from wiki.qemu.org/Testing , and it
> appears to work with or without the changes in question.  Which,
> I think, is wrong, because it is a bug of the same theme - with
> -daemonize we can't do -semihosting due to std file descriptors
> being redirected to /dev/null.  So it looks like there's another
> bug of waiting to hunt, but I really am a wrong person to do that,
> maybe Max Filippov can take a look.

daemonize + semihosting is a bit of an odd combination, but in
theory I guess it might be ok if the guest restricts its use of
the semihosting ABI.

-- PMM

Am 09.07.2013 14:37, schrieb Peter Maydell:
> On 9 July 2013 13:02, Michael Tokarev <mjt@tls.msk.ru> wrote:
>> 09.07.2013 12:42, Andreas Färber wrote:
>>> Am 09.07.2013 09:39, schrieb Michael Tokarev:
>>>> Ping?
>>>
>>> Have you tested -semihosting? The xtensa test image on the Wiki uses
>>> -nographic, but semihosting content always seems to go to stdout without
>>> going through a chardev IIUC...
> 
> This is I think correct, or at least not worth trying to
> change to make it talk to a chardev. The semihosting ABI
> in this area is intended primarily for debugging and similar
> purposes. I don't think this patch should adversely affect
> semihosting users.

The way I interpreted it (without having tested it yet) is that we would
not be able to use Ctrl+C in -semihosting mode, no? At least that's how
-display none is like today IIRC.

Andreas

On 9 July 2013 13:40, Andreas Färber <afaerber@suse.de> wrote:
> The way I interpreted it (without having tested it yet) is that we would
> not be able to use Ctrl+C in -semihosting mode, no? At least that's how
> -display none is like today IIRC.

I don't think anybody expects ctrl-C to interact with the semihosting
ABI. It should either kill QEMU or go to QEMU's console, depending
on whether you were using -display none or -nographic, as usual.

I really ought to test this patchset...

-- PMM

09.07.2013 16:40, Andreas Färber wrote:
> The way I interpreted it (without having tested it yet) is that we would
> not be able to use Ctrl+C in -semihosting mode, no? At least that's how
> -display none is like today IIRC.

With -display none qemu terminates when you hit Ctrl+C, with or
without the changes in question.

Only in case you use -serial mux:stdio (with the patch in question,
regardless of -display) or you use -nographic (without the patch
in question) Ctrl+C is forwarded to the guest.

Thanks,

/mjt

On 3 July 2013 17:29, Michael Tokarev <mjt@tls.msk.ru> wrote:
> +When monitor is multiplexed to stdio this way, Ctrl+C will not terminate
> +QEMU anymore but will be passed to the guest instead.

"the monitor", "in this way", "any more".
Otherwise
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

-- PMM

09.07.2013 18:04, Peter Maydell wrote:
> On 3 July 2013 17:29, Michael Tokarev <mjt@tls.msk.ru> wrote:
>> +When monitor is multiplexed to stdio this way, Ctrl+C will not terminate
>> +QEMU anymore but will be passed to the guest instead.
> 
> "the monitor", "in this way", "any more".

Grr.  This is 3rd change for the same sentence. Can we go without a v4?

> Otherwise
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

Thank you for the review!

/mjt

Applied.  Thanks.

Regards,

Anthony Liguori