[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH] vixen: transmit NUL characters received from guest serial port



On Fri, Jan 12, 2018 at 3:39 PM, Sarah Newman <srn@xxxxxxxxx> wrote:
> Certain programs, such as the NetBSD installer, include NUL characters
> in their output. Using null-terminated strings for transmitting data
> from the guest to the L0 hypervisor meant the output was being corrupted.
>
> This makes only the required changes for vixen to work properly.
> Future work could include generally switching away from null-terminated
> strings within the xen console code.
>
> Signed-off-by: Sarah Newman <srn@xxxxxxxxx>
> ---
>  xen/arch/x86/guest/vixen.c |  4 ++--
>  xen/drivers/char/console.c | 14 +++++++++++---
>  xen/drivers/char/serial.c  | 35 +++++++++++++++++++++++++++++++++++
>  xen/include/xen/lib.h      |  2 +-
>  xen/include/xen/serial.h   |  3 +++
>  5 files changed, 52 insertions(+), 6 deletions(-)
>
> diff --git a/xen/arch/x86/guest/vixen.c b/xen/arch/x86/guest/vixen.c
> index 9cb1a80..1b1d2e0 100644
> --- a/xen/arch/x86/guest/vixen.c
> +++ b/xen/arch/x86/guest/vixen.c
> @@ -304,7 +304,7 @@ bool vixen_ring_process(uint16_t port)
>          char ch = r->out[MASK_XENCONS_IDX(r->out_cons, r->out)];
>          if (n == sizeof(buffer) - 1) {
>              buffer[n] = 0;
> -            guest_puts(hardware_domain, buffer);
> +            guest_putsn(hardware_domain, buffer, n);
>              n = 0;
>          }
>          buffer[n++] = ch;
> @@ -314,7 +314,7 @@ bool vixen_ring_process(uint16_t port)
>
>      if (n) {
>          buffer[n] = 0;
> -        guest_puts(hardware_domain, buffer);
> +        guest_putsn(hardware_domain, buffer, n);
>      }
>
>      spin_unlock(&vixen_xencons_lock);
> diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
> index 38a5d67..ba22cdf 100644
> --- a/xen/drivers/char/console.c
> +++ b/xen/drivers/char/console.c
> @@ -341,6 +341,14 @@ static void sercon_puts(const char *s)
>          serial_puts(sercon_handle, s);
>  }
>
> +static void sercon_putsn(const char *s, unsigned int count)
> +{
> +    if ( serial_steal_fn != NULL )
> +        (*serial_steal_fn)(s);
> +    else
> +        serial_putsn(sercon_handle, s, count);
> +}
> +
>  static void dump_console_ring_key(unsigned char key)
>  {
>      uint32_t idx, len, sofar, c;
> @@ -461,7 +469,7 @@ static long 
> guest_console_write(XEN_GUEST_HANDLE_PARAM(char) buffer, int count)
>              /* Use direct console output as it could be interactive */
>              spin_lock_irq(&console_lock);
>
> -            sercon_puts(kbuf);
> +            sercon_putsn(kbuf, count);
>              video_puts(kbuf);
>
>              if ( opt_console_to_ring )
> @@ -761,11 +769,11 @@ void guest_printk(const struct domain *d, const char 
> *fmt, ...)
>      va_end(args);
>  }
>
> -void guest_puts(const struct domain *d, const char *kbuf)
> +void guest_putsn(const struct domain *d, const char *kbuf, unsigned int 
> count)
>  {
>      spin_lock_irq(&console_lock);
>
> -    sercon_puts(kbuf);
> +    sercon_putsn(kbuf, count);
>      video_puts(kbuf);
>
>      if ( opt_console_to_ring )
> diff --git a/xen/drivers/char/serial.c b/xen/drivers/char/serial.c
> index 09a20ac..e90378a 100644
> --- a/xen/drivers/char/serial.c
> +++ b/xen/drivers/char/serial.c
> @@ -223,6 +223,41 @@ void serial_putc(int handle, char c)
>      spin_unlock_irqrestore(&port->tx_lock, flags);
>  }
>
> +void serial_putsn(int handle, const char *s, unsigned int count)
> +{
> +    struct serial_port *port;
> +    unsigned long flags;
> +    char c;
> +
> +    if ( handle == -1 )
> +        return;
> +
> +    port = &com[handle & SERHND_IDX];
> +    if ( !port->driver || !port->driver->putc )
> +        return;
> +
> +    spin_lock_irqsave(&port->tx_lock, flags);
> +
> +    for (unsigned int i = 0; i < count; i++)
> +    {
> +        c = *s++;
> +        if ( (c == '\n') && (handle & SERHND_COOKED) )
> +            __serial_putc(port, '\r' | ((handle & SERHND_HI) ? 0x80 : 0x00));
> +
> +        if ( handle & SERHND_HI )
> +            c |= 0x80;
> +        else if ( handle & SERHND_LO )
> +            c &= 0x7f;
> +
> +        __serial_putc(port, c);
> +    }
> +
> +    if ( port->driver->flush )
> +        port->driver->flush(port);
> +
> +    spin_unlock_irqrestore(&port->tx_lock, flags);
> +}
> +

It looks like you are duplicating serial_puts() more or less.

I would also change serial_puts() to call serial_putsn() with strlen()
as a parameter to avoid the duplication.

Regards,

Anthony Liguori

>  void serial_puts(int handle, const char *s)
>  {
>      struct serial_port *port;
> diff --git a/xen/include/xen/lib.h b/xen/include/xen/lib.h
> index 5359fa4..baccae0 100644
> --- a/xen/include/xen/lib.h
> +++ b/xen/include/xen/lib.h
> @@ -91,7 +91,7 @@ extern void printk(const char *format, ...)
>      __attribute__ ((format (printf, 1, 2)));
>  extern void guest_printk(const struct domain *d, const char *format, ...)
>      __attribute__ ((format (printf, 2, 3)));
> -extern void guest_puts(const struct domain *d, const char *message);
> +extern void guest_putsn(const struct domain *d, const char *message, 
> unsigned int count);
>  extern void noreturn panic(const char *format, ...)
>      __attribute__ ((format (printf, 1, 2)));
>  extern long vm_assist(struct domain *, unsigned int cmd, unsigned int type,
> diff --git a/xen/include/xen/serial.h b/xen/include/xen/serial.h
> index 1212a12..87dbdcf 100644
> --- a/xen/include/xen/serial.h
> +++ b/xen/include/xen/serial.h
> @@ -115,6 +115,9 @@ void serial_putc(int handle, char c);
>  /* Transmit a NULL-terminated string via the specified COM port. */
>  void serial_puts(int handle, const char *s);
>
> +/* Transmit "count" characters via the specified COM port. */
> +void serial_putsn(int handle, const char *s, unsigned int count);
> +
>  /*
>   * An alternative to registering a character-receive hook. This function
>   * will not return until a character is available. It can safely be
> --
> 1.9.1
>

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.