Xen project Mailing List

On Sun, Jul 29, 2018 at 10:53 AM, Florian Schmidt <florian@xxxxxxxxx> wrote:

Hi Dafna,

thanks for the patch. I have some minimal comments, but they are all about formatting, and I would just change those in your patch before applying it. Unless you of course want to do a v4.

Other than that:

Reviewed-by: Florian Schmidt <florian.schmidt@xxxxxxxxx>

On 07/28/2018 08:50 AM, Dafna Hirschfeld wrote:

Add an option in the KVM configuration to use VGA console.
The VGA dimensions are 25x80. Once the screen is full, it
is cleared before new prints arrive.

Control Characters implementation:
'\a' - ascii bell (0x07) is ignored
'\b' - ascii backspace (0x08) supported
'\r' - ascii carriage return (0x0d) supported
'\n' - ascii new line (0x0a) interpreted as '\r\n'
'\t' - ascii horizontal tab (0x09) add spaces until the
next column that is a multiple of 8 or until riching the VGA width

There's a typo here (reaching).

Signed-off-by: Dafna Hirschfeld <dafna3@xxxxxxxxx>
---
plat/kvm/Config.uk | 32 +++++
plat/kvm/Makefile.uk | 6 +
plat/kvm/include/kvm-x86/serial_console.h | 29 +++++
plat/kvm/include/kvm-x86/vga_console.h | 28 +++++
plat/kvm/x86/console.c | 97 ++++++---------
plat/kvm/x86/serial_console.c | 85 +++++++++++++
plat/kvm/x86/vga_console.c | 145 ++++++++++++++++++++++
7 files changed, 363 insertions(+), 59 deletions(-)
create mode 100644 plat/kvm/include/kvm-x86/serial_console.h
create mode 100644 plat/kvm/include/kvm-x86/vga_console.h
create mode 100644 plat/kvm/x86/serial_console.c
create mode 100644 plat/kvm/x86/vga_console.c

diff --git a/plat/kvm/Config.uk b/plat/kvm/Config.uk
index 118954d..1042a04 100644
--- a/plat/kvm/Config.uk
+++ b/plat/kvm/Config.uk
@@ -10,6 +10,38 @@ menuconfig PLAT_KVM
Create a Unikraft image that runs as a KVM guest
if (PLAT_KVM)
+
+menu "Console Options"
+
+config KVM_KERNEL_SERIAL_CONSOLE
+ bool "Serial console for the kernel prints"
+ default y
+ depends on (LIBUKDEBUG_PRINTK && ARCH_X86_64)
+ help
+ Choose serial console for the kernel printing
+
+config KVM_KERNEL_VGA_CONSOLE
+ bool "VGA console for the kernel prints"
+ default y
+ depends on (LIBUKDEBUG_PRINTK && ARCH_X86_64)
+ help
+ Choose VGA console for the kernel printing
+
+config KVM_DEBUG_SERIAL_CONSOLE
+ bool "Serial console for the debug prints"
+ default y
+ depends on (LIBUKDEBUG_PRINTD && ARCH_X86_64)
+ help
+ Choose serial console for the debug printing
+
+config KVM_DEBUG_VGA_CONSOLE
+ bool "VGA console for the debug prints"
+ default y
+ depends on (LIBUKDEBUG_PRINTD && ARCH_X86_64)
+ help
+ Choose VGA console for the debug printing
+endmenu
+
config KVM_PCI
bool "PCI Bus Driver"
default y
diff --git a/plat/kvm/Makefile.uk b/plat/kvm/Makefile.uk
index e379c83..7e5a865 100644
--- a/plat/kvm/Makefile.uk
+++ b/plat/kvm/Makefile.uk
@@ -33,6 +33,12 @@ LIBKVMPLAT_SRCS-$(CONFIG_ARCH_X86_64) += $(LIBKVMPLAT_BASE)/x86/setup.c
LIBKVMPLAT_SRCS-$(CONFIG_ARCH_X86_64) += $(LIBKVMPLAT_BASE)/x86/console.c
LIBKVMPLAT_SRCS-$(CONFIG_ARCH_X86_64) += $(LIBKVMPLAT_BASE)/x86/lcpu.c
LIBKVMPLAT_SRCS-$(CONFIG_ARCH_X86_64) += $(LIBKVMPLAT_BASE)/x86/intctrl.c
+ifeq ($(findstring y,$(CONFIG_KVM_KERNEL_VGA_CONSOLE) $(CONFIG_KVM_DEBUG_VGA_CONSOLE)),y)
+LIBKVMPLAT_SRCS-$(CONFIG_ARCH_X86_64) += $(LIBKVMPLAT_BASE)/x86/vga_console.c
+endif
+ifeq ($(findstring y,$(CONFIG_KVM_KERNEL_SERIAL_CONSOLE) $(CONFIG_KVM_DEBUG_SERIAL_CONSOLE)),y)
+LIBKVMPLAT_SRCS-$(CONFIG_ARCH_X86_64) += $(LIBKVMPLAT_BASE)/x86/serial_console.c
+endif
LIBKVMPLAT_SRCS-y += $(LIBKVMPLAT_BASE)/shutdown.c
LIBKVMPLAT_SRCS-y += $(LIBKVMPLAT_BASE)/memory.c
LIBKVMPLAT_SRCS-y += $(LIBKVMPLAT_BASE)/irq.c
diff --git a/plat/kvm/include/kvm-x86/serial_console.h b/plat/kvm/include/kvm-x86/serial_console.h
new file mode 100644
index 0000000..490e7aa
--- /dev/null
+++ b/plat/kvm/include/kvm-x86/serial_console.h
@@ -0,0 +1,29 @@
+/* SPDX-License-Identifier: ISC */
+/*
+ * Authors: Dafna Hirschfeld <dafna3@xxxxxxxxx>
+ *
+ * Copyright (c) 2018 Dafna Hirschfeld <dafna3@xxxxxxxxx>
+ *
+ * Permission to use, copy, modify, and/or distribute this software
+ * for any purpose with or without fee is hereby granted, provided
+ * that the above copyright notice and this permission notice appear
+ * in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL
+ * WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED
+ * WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE
+ * AUTHOR BE LIABLE FOR ANY SPECIAL, DIRECT, INDIRECT, OR
+ * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS
+ * OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
+ * NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN
+ * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#ifndef __KVM_SERIAL_CONSOLE__
+#define __KVM_SERIAL_CONSOLE__
+
+void _libkvmplat_init_serial_console(void);
+void _libkvmplat_serial_putc(char a);

You have an extra space here.

+int _libkvmplat_serial_getc(void);
+
+#endif /* __KVM_SERIAL_CONSOLE__ */
diff --git a/plat/kvm/include/kvm-x86/vga_console.h b/plat/kvm/include/kvm-x86/vga_console.h
new file mode 100644
index 0000000..f4227f6
--- /dev/null
+++ b/plat/kvm/include/kvm-x86/vga_console.h
@@ -0,0 +1,28 @@
+/* SPDX-License-Identifier: ISC */
+/*
+ * Authors: Dafna Hirschfeld
+ *
+ * Copyright (c) 2018 Dafna Hirschfeld <dafna3@xxxxxxxxx>
+ *
+ * Permission to use, copy, modify, and/or distribute this software
+ * for any purpose with or without fee is hereby granted, provided
+ * that the above copyright notice and this permission notice appear
+ * in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL
+ * WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED
+ * WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE
+ * AUTHOR BE LIABLE FOR ANY SPECIAL, DIRECT, INDIRECT, OR
+ * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS
+ * OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
+ * NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN
+ * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#ifndef __KVM_VGA_CONSOLE__
+#define __KVM_VGA_CONSOLE__
+
+void _libkvmplat_init_vga_console(void);
+void _libkvmplat_vga_putc(char a);
+
+#endif /* __KVM_VGA_CONSOLE__ */
diff --git a/plat/kvm/x86/console.c b/plat/kvm/x86/console.c
index 5ec03b0..071cc70 100644
--- a/plat/kvm/x86/console.c
+++ b/plat/kvm/x86/console.c
@@ -25,89 +25,68 @@
* CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
*/
-#include <x86/cpu.h>
-#include <kvm/console.h>
#include <uk/plat/console.h>
+#include <uk/config.h>
#include <uk/essentials.h>
-#include <uk/print.h>
-#define COM1 0x3f8
+#if (CONFIG_KVM_DEBUG_VGA_CONSOLE || CONFIG_KVM_KERNEL_VGA_CONSOLE)
+#include <kvm-x86/vga_console.h>
+#endif
-#define COM1_DATA (COM1 + 0)
-#define COM1_INTR (COM1 + 1)
-#define COM1_CTRL (COM1 + 3)
-#define COM1_STATUS (COM1 + 5)
-
-/* only when DLAB is set */
-#define COM1_DIV_LO (COM1 + 0)
-#define COM1_DIV_HI (COM1 + 1)
-
-#define DLAB 0x80
-#define PROT 0x03 /* 8N1 (8 bits, no parity, one stop bit) */
+#if (CONFIG_KVM_DEBUG_SERIAL_CONSOLE || CONFIG_KVM_KERNEL_SERIAL_CONSOLE)
+#include <kvm-x86/serial_console.h>
+#endif

This ends up with two newlines before the #ifs. I'd remove those to make the #include part a bit more compact.

void _libkvmplat_init_console(void)
{
- outb(COM1_INTR, 0x00); /* Disable all interrupts */
- outb(COM1_CTRL, DLAB); /* Enable DLAB (set baudrate divisor) */
- outb(COM1_DIV_LO, 0x01);/* Set div to 1 (lo byte) 115200 baud */
- outb(COM1_DIV_HI, 0x00);/* (hi byte) */
- outb(COM1_CTRL, PROT); /* Set 8N1, clear DLAB */
-}
-
-int ukplat_coutd(const char *str, unsigned int len)
-{
- return ukplat_coutk(str, len);
-}
-
-static int serial_tx_empty(void)
-{
- return inb(COM1_STATUS) & 0x20;
-}
-
-static void serial_write(char a)
-{
- while (!serial_tx_empty())
- ;
+#if (CONFIG_KVM_DEBUG_VGA_CONSOLE || CONFIG_KVM_KERNEL_VGA_CONSOLE)
+ _libkvmplat_init_vga_console();
+#endif
+#if (CONFIG_KVM_DEBUG_SERIAL_CONSOLE || CONFIG_KVM_KERNEL_SERIAL_CONSOLE)
+ _libkvmplat_init_serial_console();
+#endif
- outb(COM1_DATA, a);
}
-static void serial_putc(char a)
+int ukplat_coutd(const char *buf __maybe_unused, unsigned int len)
{
- if (a == '\n')
- serial_write('\r');
- serial_write(a);
-}
-
-static int serial_rx_ready(void)
-{
- return inb(COM1_STATUS) & 0x01;
+ for (unsigned int i = 0; i < len; i++) {
+#if CONFIG_KVM_DEBUG_SERIAL_CONSOLE
+ _libkvmplat_serial_putc(buf[i]);
+#endif
+#if CONFIG_KVM_DEBUG_VGA_CONSOLE
+ _libkvmplat_vga_putc(buf[i]);
+#endif
+ }
+ return len;
}
-static int serial_getc(void)
-{
- if (!serial_rx_ready())
- return -1;
- return (int) inb(COM1_DATA);
-}
-int ukplat_coutk(const char *buf, unsigned int len)
+int ukplat_coutk(const char *buf __maybe_unused, unsigned int len)
{
- for (unsigned int i = 0; i < len; i++)
- serial_putc(buf[i]);
+ for (unsigned int i = 0; i < len; i++) {
+#if CONFIG_KVM_KERNEL_SERIAL_CONSOLE
+ _libkvmplat_serial_putc(buf[i]);
+#endif
+#if CONFIG_KVM_KERNEL_VGA_CONSOLE
+ _libkvmplat_vga_putc(buf[i]);
+#endif
+ }
return len;
}
-int ukplat_cink(char *buf, unsigned int maxlen)
+int ukplat_cink(char *buf __maybe_unused, unsigned int maxlen __maybe_unused)
{
- int ret;
+ int ret __maybe_unused;
unsigned int num = 0;
+#if CONFIG_KVM_KERNEL_SERIAL_CONSOLE
+

Same here with that extra newline.

while (num < maxlen
- && (ret = serial_getc()) >= 0) {
+ && (ret = _libkvmplat_serial_getc()) >= 0) {
*(buf++) = (char) ret;
num++;
}
-
+#endif
return (int) num;
}
diff --git a/plat/kvm/x86/serial_console.c b/plat/kvm/x86/serial_console.c
new file mode 100644
index 0000000..2cf9ce6
--- /dev/null
+++ b/plat/kvm/x86/serial_console.c
@@ -0,0 +1,85 @@
+/* SPDX-License-Identifier: ISC */
+/*
+ * Authors: Dan Williams
+ * Martin Lucina
+ * Felipe Huici <felipe.huici@xxxxxxxxx>
+ * Florian Schmidt <florian.schmidt@xxxxxxxxx>
+ * Simon Kuenzer <simon.kuenzer@xxxxxxxxx>
+ *
+ * Copyright (c) 2015-2017 IBM
+ * Copyright (c) 2016-2017 Docker, Inc.
+ * Copyright (c) 2017 NEC Europe Ltd., NEC Corporation
+ *
+ * Permission to use, copy, modify, and/or distribute this software
+ * for any purpose with or without fee is hereby granted, provided
+ * that the above copyright notice and this permission notice appear
+ * in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL
+ * WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED
+ * WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE
+ * AUTHOR BE LIABLE FOR ANY SPECIAL, DIRECT, INDIRECT, OR
+ * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS
+ * OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
+ * NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN
+ * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#include <kvm-x86/serial_console.h>
+
+#include <x86/cpu.h>

And here.

+
+#define COM1 0x3f8
+
+#define COM1_DATA (COM1 + 0)
+#define COM1_INTR (COM1 + 1)
+#define COM1_CTRL (COM1 + 3)
+#define COM1_STATUS (COM1 + 5)
+
+/* only when DLAB is set */
+#define COM1_DIV_LO (COM1 + 0)
+#define COM1_DIV_HI (COM1 + 1)
+
+#define DLAB 0x80
+#define PROT 0x03 /* 8N1 (8 bits, no parity, one stop bit) */
+
+void _libkvmplat_init_serial_console(void)
+{
+ outb(COM1_INTR, 0x00); /* Disable all interrupts */
+ outb(COM1_CTRL, DLAB); /* Enable DLAB (set baudrate divisor) */
+ outb(COM1_DIV_LO, 0x01);/* Set div to 1 (lo byte) 115200 baud */
+ outb(COM1_DIV_HI, 0x00);/* (hi byte) */
+ outb(COM1_CTRL, PROT); /* Set 8N1, clear DLAB */
+}
+
+static int serial_tx_empty(void)
+{
+ return inb(COM1_STATUS) & 0x20;
+}
+
+static void serial_write(char a)
+{
+ while (!serial_tx_empty())
+ ;
+
+ outb(COM1_DATA, a);
+}
+
+void _libkvmplat_serial_putc(char a)
+{
+ if (a == '\n')
+ serial_write('\r');
+ serial_write(a);
+}
+
+static int serial_rx_ready(void)
+{
+ return inb(COM1_STATUS) & 0x01;
+}
+
+int _libkvmplat_serial_getc(void)
+{
+ if (!serial_rx_ready())
+ return -1;
+ return (int) inb(COM1_DATA);
+}
diff --git a/plat/kvm/x86/vga_console.c b/plat/kvm/x86/vga_console.c
new file mode 100644
index 0000000..aad61c9
--- /dev/null
+++ b/plat/kvm/x86/vga_console.c
@@ -0,0 +1,145 @@
+/* SPDX-License-Identifier: ISC */
+/*
+ * Authors: Dan Williams
+ * Martin Lucina
+ * Felipe Huici <felipe.huici@xxxxxxxxx>
+ * Florian Schmidt <florian.schmidt@xxxxxxxxx>
+ * Simon Kuenzer <simon.kuenzer@xxxxxxxxx>
+ * Dafna Hirschfeld <dafna3@xxxxxxxxx>
+ *
+ * Copyright (c) 2015-2017 IBM
+ * Copyright (c) 2016-2017 Docker, Inc.
+ * Copyright (c) 2017 NEC Europe Ltd., NEC Corporation
+ *
+ * Permission to use, copy, modify, and/or distribute this software
+ * for any purpose with or without fee is hereby granted, provided
+ * that the above copyright notice and this permission notice appear
+ * in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL
+ * WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED
+ * WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE
+ * AUTHOR BE LIABLE FOR ANY SPECIAL, DIRECT, INDIRECT, OR
+ * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS
+ * OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
+ * NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN
+ * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#include <sys/types.h>
+#include <stdint.h>
+#include <kvm-x86/vga_console.h>
+
+/* Hardware text mode color constants. */
+enum vga_color {
+ VGA_COLOR_BLACK = 0,
+ VGA_COLOR_BLUE = 1,
+ VGA_COLOR_GREEN = 2,
+ VGA_COLOR_CYAN = 3,
+ VGA_COLOR_RED = 4,
+ VGA_COLOR_MAGENTA = 5,
+ VGA_COLOR_BROWN = 6,
+ VGA_COLOR_LIGHT_GREY = 7,
+ VGA_COLOR_DARK_GREY = 8,
+ VGA_COLOR_LIGHT_BLUE = 9,
+ VGA_COLOR_LIGHT_GREEN = 10,
+ VGA_COLOR_LIGHT_CYAN = 11,
+ VGA_COLOR_LIGHT_RED = 12,
+ VGA_COLOR_LIGHT_MAGENTA = 13,
+ VGA_COLOR_LIGHT_BROWN = 14,
+ VGA_COLOR_WHITE = 15,
+};
+
+static inline uint8_t vga_entry_color(enum vga_color fg, enum vga_color bg)
+{
+ return fg | bg << 4;
+}
+
+static inline uint16_t vga_entry(unsigned char uc, uint8_t color)
+{
+ return (uint16_t) uc | (uint16_t) color << 8;
+}
+
+#define TAB_ALIGNMENT 8
+#define VGA_WIDTH 80
+#define VGA_HEIGHT 25
+
+static size_t terminal_row;
+static size_t terminal_column;
+static uint8_t terminal_color;
+static uint16_t *terminal_buffer;
+
+static void clear_terminal(void)
+{
+ for (size_t y = 0; y < VGA_HEIGHT; y++) {
+ for (size_t x = 0; x < VGA_WIDTH; x++) {
+ const size_t index = y * VGA_WIDTH + x;
+
+ terminal_buffer[index] = vga_entry(' ', terminal_color);
+ }
+ }
+}
+
+void _libkvmplat_init_vga_console(void)
+{
+ terminal_row = 0;
+ terminal_column = 0;
+ terminal_color = vga_entry_color(VGA_COLOR_LIGHT_GREY, VGA_COLOR_BLACK);
+ terminal_buffer = (uint16_t *) 0xB8000;
+ clear_terminal();
+}
+
+static void terminal_putentryat(char c, uint8_t color, size_t x, size_t y)
+{
+ const size_t index = y * VGA_WIDTH + x;
+
+ terminal_buffer[index] = vga_entry(c, color);
+}
+
+void _libkvmplat_vga_putc(char c)
+{
+ if (terminal_column == 0 && terminal_row == 0)
+ clear_terminal();
+
+ switch (c) {
+ case '\a':
+ break; //ascii bel (0x07) - ignore
+ case '\b':
+ if (terminal_column > 0) {
+ terminal_column--;
+ } else if (terminal_row > 0) {
+ terminal_column = VGA_WIDTH - 1;
+ terminal_row--;
+ }
+ break;
+ case '\n':
+ _libkvmplat_vga_putc('\r');
+ if (++terminal_row == VGA_HEIGHT)
+ terminal_row = 0;
+ break;
+ case '\r':
+ terminal_column = 0;
+ break;
+ case '\t':
+ do {
+ terminal_column++;
+ } while (terminal_column % TAB_ALIGNMENT != 0
+ && terminal_column != VGA_WIDTH);
+
+ if (terminal_column == VGA_WIDTH) {
+ terminal_column = 0;
+ if (++terminal_row == VGA_HEIGHT)
+ terminal_row = 0;
+ }
+ break;
+ default:
+ terminal_putentryat(c, terminal_color,
+ terminal_column, terminal_row);
+ if (++terminal_column == VGA_WIDTH) {
+ terminal_column = 0;
+ if (++terminal_row == VGA_HEIGHT)
+ terminal_row = 0;
+ }
+ break;
+ }
+}

Re: [Minios-devel] [UNIKRAFT PATCH/v3] plat/kvm: Add VGA textmode console support.