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 <pau...@ozlabs.org>
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