[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] spapr: Implement better workaround in spapr-vty
From: |
David Gibson |
Subject: |
Re: [Qemu-devel] [PATCH] spapr: Implement better workaround in spapr-vty device |
Date: |
Wed, 31 Jul 2019 16:51:20 +1000 |
User-agent: |
Mutt/1.12.0 (2019-05-25) |
On Wed, Jul 31, 2019 at 02:36:54PM +1000, Paul Mackerras wrote:
> Linux guest kernels have code which scans the string of characters
> returned from the H_GET_TERM_CHAR hypercall and removes any \0
> character which comes immediately after a \r character. This is to
> work around a bug which was present in some ancient versions of
> PowerVM. In order to avoid the corruption of the console byte stream
> that this introduced, commit 6c3bc244d3cb ("spapr: Implement bug in
> spapr-vty device to be compatible with PowerVM") added a workaround
> which adds a \0 character after every \r character. Unfortunately,
> this corrupts the console byte stream for those operating systems,
> such as AIX, which don't remove the null bytes.
>
> We can avoid triggering the Linux kernel workaround if we avoid
> returning a buffer which contains a \0 after a \r. We can do that by
> breaking out of the loop in vty_getchars() if we are about to insert a
> \0 and the previous character in the buffer is a \r. That means we
> return the characters up to the \r for the current H_GET_TERM_CHAR,
> and the characters starting with the \0 for the next one.
>
> With this workaround, we don't insert any spurious characters and we
> avoid triggering the Linux kernel workaround, so the guest will
> receive an uncorrupted stream whether or not they have the workaround.
>
> Fixes: 6c3bc244d3cb ("spapr: Implement bug in spapr-vty device to be
> compatible with PowerVM")
> Signed-off-by: Paul Mackerras <address@hidden>
Applied to ppc-for-4.2, thanks.
> ---
> hw/char/spapr_vty.c | 30 ++++++++++++------------------
> 1 file changed, 12 insertions(+), 18 deletions(-)
>
> diff --git a/hw/char/spapr_vty.c b/hw/char/spapr_vty.c
> index 617303dbaf..65a7c2ffbd 100644
> --- a/hw/char/spapr_vty.c
> +++ b/hw/char/spapr_vty.c
> @@ -57,25 +57,19 @@ static int vty_getchars(SpaprVioDevice *sdev, uint8_t
> *buf, int max)
> int n = 0;
>
> while ((n < max) && (dev->out != dev->in)) {
> - buf[n++] = dev->buf[dev->out++ % VTERM_BUFSIZE];
> -
> - /* PowerVM's vty implementation has a bug where it inserts a
> - * \0 after every \r going to the guest. Existing guests have
> - * a workaround for this which removes every \0 immediately
> - * following a \r, so here we make ourselves bug-for-bug
> - * compatible, so that the guest won't drop a real \0-after-\r
> - * that happens to occur in a binary stream. */
> - if (buf[n - 1] == '\r') {
> - if (n < max) {
> - buf[n++] = '\0';
> - } else {
> - /* No room for the extra \0, roll back and try again
> - * next time */
> - dev->out--;
> - n--;
> - break;
> - }
> + /*
> + * Long ago, PowerVM's vty implementation had a bug where it
> + * inserted a \0 after every \r going to the guest. Existing
> + * guests have a workaround for this which removes every \0
> + * immediately following a \r. To avoid triggering this
> + * workaround, we stop before inserting a \0 if the preceding
> + * character in the output buffer is a \r.
> + */
> + if (n > 0 && (buf[n - 1] == '\r') &&
> + (dev->buf[dev->out % VTERM_BUFSIZE] == '\0')) {
> + break;
> }
> + buf[n++] = dev->buf[dev->out++ % VTERM_BUFSIZE];
> }
>
> qemu_chr_fe_accept_input(&dev->chardev);
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature