|
[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
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |