[PATCH v3 00/12] Implement virtual hypervisor console

59 views
Skip to first unread message

Ralf Ramsauer

unread,
Jan 16, 2017, 11:07:37 AM1/16/17
to Jan Kiszka, jailho...@googlegroups.com, Ralf Ramsauer
Hi,

I implemented a sysfs interface for the hypervisor console. This is useful for
debugging devices without UARTs. At least as long as the root-cell doesn't
crash...

Additionally, I implemented continuous reading of the hypervisor console via
/dev/jailhouse, which is a pretty nice and easy way to follow the hypervisor
console from Linux.

I use a page in the hypervisor's memory space as ringbuffer. If the root cell
has a certain permission enabled in its config, the hypervisor won't back that
page with empty_page. The Linux driver may then read from that page without any
interactions with the hypervisor. We do this without any signalling or locking.
Otherwise we would risk starvation of the hypervisor. The producer (hypervisor
debug write) only indicates ongoing writes by setting a flag.

tree:
https://github.com/lfd/jailhouse/tree/hvconsoleV3

PS: maybe we could think of enabling JAILHOUSE_CELL_DEBUG_CONSOLE as a default
flag for our demo inmate cell configuration. As Jan already pointed out
earlier, none of them is permissive.

Ralf

since v2:
- support continuous reading on /dev/jailhouse as well as non-blocking reads
- support overflows of the tail pointer
- adopt userspace tool to dump the console (either by switching a command
line flag or automatically on errors)
- ring buffer size must be a power of two (otherwise modulo fails on tail
pointer overflows)
- successfully tested on Qemu and Tegra [XK]1

since v1:
- refactor JAILHOUSE_CON_TYPE to JAILHOUSE_CON1_TYPE
- use JAILHOUSE_CON2_TYPE_ROOTPAGE as permission flag for sysfs console
- no extra section in linker file required

Ralf Ramsauer (12):
configs, core: Refactor JAILHOUSE_CON_TYPE to JAILHOUSE_CON1_TYPE
core: declare and define structures used for virtual console
core: don't hide console page
driver, core: propagate offset of console page
core: print console messages to the console page
driver: add console sysfs attribute
Documentation: add documentation for sysfs console
configs: activate sysfs console
driver: implement continous console reading on /dev/jailhouse
driver: copy the console_page after the hypervisor is disabled
tools: move helper function to the top
tools: allow to follow console on jailhouse enable

Documentation/debug-output.md | 37 +++--
Documentation/sysfs-entries.txt | 3 +
Documentation/vga-console.txt | 2 +-
TODO.md | 1 -
configs/amd-seattle.c | 5 +-
configs/bananapi.c | 5 +-
configs/f2a88xm-hd3.c | 5 +-
configs/foundation-v8.c | 5 +-
configs/hikey.c | 4 +-
configs/imb-a180.c | 5 +-
configs/jetson-tk1.c | 5 +-
configs/jetson-tx1.c | 5 +-
configs/orangepi0.c | 5 +-
configs/qemu-vm.c | 5 +-
configs/vexpress.c | 5 +-
configs/zynqmp-zcu102.c | 5 +-
driver/main.c | 230 ++++++++++++++++++++++++++++-
driver/main.h | 3 +
driver/sysfs.c | 20 +++
hypervisor/arch/arm-common/dbg-write.c | 10 +-
hypervisor/arch/x86/dbg-write.c | 6 +-
hypervisor/arch/x86/uart.c | 2 +-
hypervisor/hypervisor.lds.S | 6 +
hypervisor/include/jailhouse/cell-config.h | 31 ++--
hypervisor/include/jailhouse/header.h | 11 ++
hypervisor/include/jailhouse/printk.h | 3 +
hypervisor/paging.c | 2 +-
hypervisor/printk.c | 22 ++-
hypervisor/setup.c | 13 ++
tools/jailhouse-cell-linux | 2 +-
tools/jailhouse-hardware-check | 2 +-
tools/jailhouse.c | 78 ++++++++--
tools/root-cell-config.c.tmpl | 5 +-
33 files changed, 466 insertions(+), 82 deletions(-)

--
2.11.0

Ralf Ramsauer

unread,
Jan 16, 2017, 11:07:37 AM1/16/17
to Jan Kiszka, jailho...@googlegroups.com, Ralf Ramsauer
If the console is enabled. Don't write messages to the console page if
it is disabled. Otherwise we would leak information if we disable the
hypervisor.

Signed-off-by: Ralf Ramsauer <ralf.r...@oth-regensburg.de>
---
hypervisor/printk.c | 19 ++++++++++++++++++-
1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/hypervisor/printk.c b/hypervisor/printk.c
index aad6dadc..7b9cfd2a 100644
--- a/hypervisor/printk.c
+++ b/hypervisor/printk.c
@@ -23,7 +23,24 @@ struct jailhouse_console console __attribute__((section(".console")));

static DEFINE_SPINLOCK(printk_lock);

-#define console_write(msg) arch_dbg_write(msg)
+static void console_write(const char *msg)
+{
+ arch_dbg_write(msg);
+
+ if (!console_print)
+ return;
+
+ console.lock = true;
+ memory_barrier();
+ while (*msg) {
+ console.content[console.tail % sizeof(console.content)] =
+ *msg++;
+ console.tail++;
+ }
+ console.lock = false;
+ memory_barrier();
+}
+
#include "printk-core.c"

static void dbg_write_stub(const char *msg)
--
2.11.0

Ralf Ramsauer

unread,
Jan 16, 2017, 11:07:37 AM1/16/17
to Jan Kiszka, jailho...@googlegroups.com, Ralf Ramsauer
Driver must know the location of the console_page. Use the
jailhouse_header for propagating the offset.

Signed-off-by: Ralf Ramsauer <ralf.r...@oth-regensburg.de>
---
hypervisor/include/jailhouse/header.h | 3 +++
hypervisor/setup.c | 1 +
2 files changed, 4 insertions(+)

diff --git a/hypervisor/include/jailhouse/header.h b/hypervisor/include/jailhouse/header.h
index 072a37b2..655d4c91 100644
--- a/hypervisor/include/jailhouse/header.h
+++ b/hypervisor/include/jailhouse/header.h
@@ -53,6 +53,9 @@ struct jailhouse_header {
/** Entry point (arch_entry()).
* @note Filled at build time. */
int (*entry)(unsigned int);
+ /** Offset of the console page inside the hypervisor memory
+ * @note Filled at build time. */
+ unsigned long console_page;

/** Configured maximum logical CPU ID + 1.
* @note Filled by Linux loader driver before entry. */
diff --git a/hypervisor/setup.c b/hypervisor/setup.c
index 12e563ba..791341b1 100644
--- a/hypervisor/setup.c
+++ b/hypervisor/setup.c
@@ -232,4 +232,5 @@ hypervisor_header = {
.core_size = (unsigned long)__page_pool - JAILHOUSE_BASE,
.percpu_size = sizeof(struct per_cpu),
.entry = arch_entry - JAILHOUSE_BASE,
+ .console_page = (unsigned long)&console - JAILHOUSE_BASE,
};
--
2.11.0

Ralf Ramsauer

unread,
Jan 16, 2017, 11:07:37 AM1/16/17
to Jan Kiszka, jailho...@googlegroups.com, Ralf Ramsauer
We're backing hypervisor pages with an empty page for the root cell.
This commit excludes the console page and allows RO access to it, but
only if the debug console of system configuration has the flag
JAILHOUSE_CON2_TYPE_ROOTPAGE set.

This flag is not activated by default and has to be set manually.

In this way, the Linux driver may easily read out the console without
any interactions with the hypervisor.

Signed-off-by: Ralf Ramsauer <ralf.r...@oth-regensburg.de>
---
hypervisor/include/jailhouse/printk.h | 1 +
hypervisor/printk.c | 1 +
hypervisor/setup.c | 12 ++++++++++++
3 files changed, 14 insertions(+)

diff --git a/hypervisor/include/jailhouse/printk.h b/hypervisor/include/jailhouse/printk.h
index 68596e84..4c8e5224 100644
--- a/hypervisor/include/jailhouse/printk.h
+++ b/hypervisor/include/jailhouse/printk.h
@@ -28,4 +28,5 @@ void __attribute__((format(printf, 1, 2))) panic_printk(const char *fmt, ...);
void arch_dbg_write_init(void);
extern void (*arch_dbg_write)(const char *msg);

+extern bool console_print;
extern struct jailhouse_console console;
diff --git a/hypervisor/printk.c b/hypervisor/printk.c
index 994a0432..aad6dadc 100644
--- a/hypervisor/printk.c
+++ b/hypervisor/printk.c
@@ -18,6 +18,7 @@
#include <asm/bitops.h>
#include <asm/spinlock.h>

+bool console_print = false;
struct jailhouse_console console __attribute__((section(".console")));

static DEFINE_SPINLOCK(printk_lock);
diff --git a/hypervisor/setup.c b/hypervisor/setup.c
index 8cfa4555..12e563ba 100644
--- a/hypervisor/setup.c
+++ b/hypervisor/setup.c
@@ -41,6 +41,10 @@ static void init_early(unsigned int cpu_id)
system_config = (struct jailhouse_system *)
(JAILHOUSE_BASE + core_and_percpu_size);

+ if (CON2_TYPE(system_config->debug_console.flags) ==
+ JAILHOUSE_CON2_TYPE_ROOTPAGE)
+ console_print = true;
+
arch_dbg_write_init();

printk("\nInitializing Jailhouse hypervisor %s on CPU %d\n",
@@ -65,6 +69,9 @@ static void init_early(unsigned int cpu_id)
* Back the region of the hypervisor core and per-CPU page with empty
* pages for Linux. This allows to fault-in the hypervisor region into
* Linux' page table before shutdown without triggering violations.
+ *
+ * Allow read access to the console page, if the hypervisor has the
+ * debug console flag JAILHOUSE_CON2_TYPE_ROOTPAGE set.
*/
hyp_phys_start = system_config->hypervisor_memory.phys_start;
hyp_phys_end = hyp_phys_start + system_config->hypervisor_memory.size;
@@ -74,6 +81,11 @@ static void init_early(unsigned int cpu_id)
hv_page.size = PAGE_SIZE;
hv_page.flags = JAILHOUSE_MEM_READ;
while (hv_page.virt_start < hyp_phys_end) {
+ if (console_print &&
+ hv_page.virt_start == paging_hvirt2phys(&console))
+ hv_page.phys_start = paging_hvirt2phys(&console);
+ else
+ hv_page.phys_start = paging_hvirt2phys(empty_page);
error = arch_map_memory_region(&root_cell, &hv_page);
if (error)
return;
--
2.11.0

Ralf Ramsauer

unread,
Jan 16, 2017, 11:07:37 AM1/16/17
to Jan Kiszka, jailho...@googlegroups.com, Ralf Ramsauer
The console page is implemented as ring buffer, and will _not_ contain a
trailing \0 for string termination.

We're changing the semantics of the configuration: increment revision
counter.

Signed-off-by: Ralf Ramsauer <ralf.r...@oth-regensburg.de>
---
hypervisor/hypervisor.lds.S | 6 ++++++
hypervisor/include/jailhouse/cell-config.h | 13 +++++++++----
hypervisor/include/jailhouse/header.h | 8 ++++++++
hypervisor/include/jailhouse/printk.h | 2 ++
hypervisor/printk.c | 2 ++
tools/jailhouse-cell-linux | 2 +-
tools/jailhouse-hardware-check | 2 +-
7 files changed, 29 insertions(+), 6 deletions(-)

diff --git a/hypervisor/hypervisor.lds.S b/hypervisor/hypervisor.lds.S
index 09b1a0b2..f8178282 100644
--- a/hypervisor/hypervisor.lds.S
+++ b/hypervisor/hypervisor.lds.S
@@ -35,6 +35,12 @@ SECTIONS
. = ALIGN(16);
.bss : { *(.bss) }

+ /* The console section shall only contain the hypervisor console. This
+ * section and the next section must be aligned to PAGE_SIZE, as we
+ * want the console section to reserve a whole page */
+ . = ALIGN(PAGE_SIZE);
+ .console : { *(.console) }
+
. = ALIGN(PAGE_SIZE);
__page_pool = .;

diff --git a/hypervisor/include/jailhouse/cell-config.h b/hypervisor/include/jailhouse/cell-config.h
index 80fa5a78..ad894cb6 100644
--- a/hypervisor/include/jailhouse/cell-config.h
+++ b/hypervisor/include/jailhouse/cell-config.h
@@ -40,7 +40,7 @@
#define _JAILHOUSE_CELL_CONFIG_H

/* Incremented on any layout or semantic change of system or cell config. */
-#define JAILHOUSE_CONFIG_REVISION 3
+#define JAILHOUSE_CONFIG_REVISION 4

#define JAILHOUSE_CELL_NAME_MAXLEN 31

@@ -186,9 +186,14 @@ struct jailhouse_iommu {

#define CON1_TYPE(flags) ((flags) & JAILHOUSE_CON1_TYPE_MASK)

-/* We use bit 4..5 to differentiate between PIO and MMIO access */
-#define JAILHOUSE_CON1_FLAG_PIO 0x0010
-#define JAILHOUSE_CON1_FLAG_MMIO 0x0020
+#define JAILHOUSE_CON2_TYPE_ROOTPAGE 0x0010
+#define JAILHOUSE_CON2_TYPE_MASK 0x00f0
+
+#define CON2_TYPE(flags) ((flags) & JAILHOUSE_CON2_TYPE_MASK)
+
+/* We use bit 8..9 to differentiate between PIO and MMIO access */
+#define JAILHOUSE_CON1_FLAG_PIO 0x0100
+#define JAILHOUSE_CON1_FLAG_MMIO 0x0200

#define CON1_IS_MMIO(flags) ((flags) & JAILHOUSE_CON1_FLAG_MMIO)

diff --git a/hypervisor/include/jailhouse/header.h b/hypervisor/include/jailhouse/header.h
index 4fe159c6..072a37b2 100644
--- a/hypervisor/include/jailhouse/header.h
+++ b/hypervisor/include/jailhouse/header.h
@@ -24,6 +24,14 @@
*/
typedef int (*jailhouse_entry)(unsigned int);

+#define CONSOLE_SIZE 0x1000
+
+struct jailhouse_console {
+ volatile unsigned int lock;
+ unsigned int tail;
+ char content[2048];
+} __attribute__((aligned(CONSOLE_SIZE)));
+
/**
* Hypervisor description.
* Located at the beginning of the hypervisor binary image and loaded by
diff --git a/hypervisor/include/jailhouse/printk.h b/hypervisor/include/jailhouse/printk.h
index a506c0fd..68596e84 100644
--- a/hypervisor/include/jailhouse/printk.h
+++ b/hypervisor/include/jailhouse/printk.h
@@ -27,3 +27,5 @@ void __attribute__((format(printf, 1, 2))) panic_printk(const char *fmt, ...);

void arch_dbg_write_init(void);
extern void (*arch_dbg_write)(const char *msg);
+
+extern struct jailhouse_console console;
diff --git a/hypervisor/printk.c b/hypervisor/printk.c
index e8f5ffe2..994a0432 100644
--- a/hypervisor/printk.c
+++ b/hypervisor/printk.c
@@ -18,6 +18,8 @@
#include <asm/bitops.h>
#include <asm/spinlock.h>

+struct jailhouse_console console __attribute__((section(".console")));
+
static DEFINE_SPINLOCK(printk_lock);

#define console_write(msg) arch_dbg_write(msg)
diff --git a/tools/jailhouse-cell-linux b/tools/jailhouse-cell-linux
index 1643a731..dea6c3ef 100755
--- a/tools/jailhouse-cell-linux
+++ b/tools/jailhouse-cell-linux
@@ -512,7 +512,7 @@ class MemoryRegion:

class Config:
_HEADER_FORMAT = '6sH32s4xIIIIIIIII'
- _HEADER_REVISION = 3
+ _HEADER_REVISION = 4

def __init__(self, config_file):
self.data = config_file.read()
diff --git a/tools/jailhouse-hardware-check b/tools/jailhouse-hardware-check
index 67314af2..fb820120 100755
--- a/tools/jailhouse-hardware-check
+++ b/tools/jailhouse-hardware-check
@@ -113,7 +113,7 @@ class Sysconfig:
X86_MAX_IOMMU_UNITS = 8
X86_IOMMU_SIZE = 20

- HEADER_REVISION = 3
+ HEADER_REVISION = 4
HEADER_FORMAT = '6sH'

def __init__(self, path):
--
2.11.0

Ralf Ramsauer

unread,
Jan 16, 2017, 11:07:38 AM1/16/17
to Jan Kiszka, jailho...@googlegroups.com, Ralf Ramsauer
This allows us to dump messages if the hypervisor failed to start or the
last few messages after the hypervisor was disabled. This needs some
special treatment, as the hypervisor_memory region is unmapped after
failures.

Hook into disable an failure paths and copy the page to a dedicated
variable. Indicate with a boolean flag that data is present.

Signed-off-by: Ralf Ramsauer <ralf.r...@oth-regensburg.de>
---
driver/main.c | 40 +++++++++++++++++++++++++++++++++++++---
1 file changed, 37 insertions(+), 3 deletions(-)

diff --git a/driver/main.c b/driver/main.c
index f98e63c6..807514bc 100644
--- a/driver/main.c
+++ b/driver/main.c
@@ -84,6 +84,16 @@ static int error_code;
static struct jailhouse_console *console_page;
static bool console_available;

+/* dump_last_console_page will be true in two cases:
+ * - the jailhouse console page when disabling the hypervisor
+ * - the jailhouse console page when enabling the hypervisor fails
+ *
+ * In these cases, we must copy the console_page to last_console_page before
+ * the hypervisor_mem region is unmapped.
+ */
+static bool dump_last_console_page;
+static struct jailhouse_console last_console_page;
+
#ifdef CONFIG_X86
bool jailhouse_use_vmcall;

@@ -298,6 +308,9 @@ static int jailhouse_cmd_enable(struct jailhouse_system __user *arg)
long max_cpus;
int err;

+ /* unset the dump_last_console_page to drop pending content */
+ dump_last_console_page = false;
+
fw_name = jailhouse_get_fw_name();
if (!fw_name) {
pr_err("jailhouse: Missing or unsupported HVM technology\n");
@@ -477,6 +490,10 @@ static int jailhouse_cmd_enable(struct jailhouse_system __user *arg)
return 0;

error_free_cell:
+ if (console_available) {
+ copy_console_page(&last_console_page);
+ dump_last_console_page = true;
+ }
jailhouse_cell_delete_root();

error_unmap:
@@ -576,6 +593,11 @@ static int jailhouse_cmd_disable(void)
if (err)
goto unlock_out;

+ if (console_available) {
+ copy_console_page(&last_console_page);
+ dump_last_console_page = true;
+ }
+
vunmap(hypervisor_mem);

jailhouse_cell_delete_root();
@@ -660,9 +682,21 @@ static ssize_t jailhouse_console_read(struct file *file, char __user *out,

/* wait for new data */
while (1) {
- ret = jailhouse_console_page_delta(content, user->head, &miss);
- if ((!ret || ret == -EAGAIN) && file->f_flags & O_NONBLOCK)
- goto console_free_out;
+ if (dump_last_console_page) {
+ ret = jailhouse_console_delta(&last_console_page,
+ content, user->head,
+ &miss);
+ if (ret == 0) {
+ dump_last_console_page = false;
+ goto console_free_out;
+ }
+ } else {
+ ret = jailhouse_console_page_delta(content, user->head,
+ &miss);
+ if ((!ret || ret == -EAGAIN) &&
+ file->f_flags & O_NONBLOCK)
+ goto console_free_out;
+ }

if (ret == -EAGAIN)
/* Reset the user head, if jailhouse is not enabled. We
--
2.11.0

Ralf Ramsauer

unread,
Jan 16, 2017, 11:07:38 AM1/16/17
to Jan Kiszka, jailho...@googlegroups.com, Ralf Ramsauer
Signed-off-by: Ralf Ramsauer <ralf.r...@oth-regensburg.de>
---
Documentation/sysfs-entries.txt | 3 ++
TODO.md | 1 -
driver/main.c | 105 +++++++++++++++++++++++++++++++++++++++-
driver/main.h | 3 ++
driver/sysfs.c | 20 ++++++++
5 files changed, 130 insertions(+), 2 deletions(-)

diff --git a/Documentation/sysfs-entries.txt b/Documentation/sysfs-entries.txt
index 6b483bba..58d1f5a2 100644
--- a/Documentation/sysfs-entries.txt
+++ b/Documentation/sysfs-entries.txt
@@ -5,6 +5,7 @@ The following sysfs entries are provided by the Jailhouse Linux driver. These
can be used for monitoring the state of the hypervisor and its cells.

/sys/devices/jailhouse
+|- console - hypervisor console (see [1])
|- enabled - 1 if Jailhouse is enabled, 0 otherwise
|- mem_pool_size - number of pages in hypervisor memory pool
|- mem_pool_used - used pages of hypervisor memory pool
@@ -30,3 +31,5 @@ may not reflect a fully consistent state. The existence and semantics of VM
exit reason values are architecture-dependent and may change in future
versions. In general statistics shall only be considered as a first hint when
analyzing cell behavior.
+
+[1] Documentation/debug-output.md
diff --git a/TODO.md b/TODO.md
index e452d306..4e93b1d7 100644
--- a/TODO.md
+++ b/TODO.md
@@ -89,7 +89,6 @@ Hardware error handling

Monitoring
- report error-triggering devices behind IOMMUs via sysfs
- - hypervisor console via debugfs?
- cell software watchdog via comm region messages
-> time out pending comm region messages and kill failing cells
(includes timeouts of unanswered shutdown requests)
diff --git a/driver/main.c b/driver/main.c
index 676794be..8a4b929f 100644
--- a/driver/main.c
+++ b/driver/main.c
@@ -19,6 +19,7 @@
#include <linux/miscdevice.h>
#include <linux/firmware.h>
#include <linux/mm.h>
+#include <linux/slab.h>
#include <linux/smp.h>
#include <linux/uaccess.h>
#include <linux/reboot.h>
@@ -37,7 +38,6 @@
#include "pci.h"
#include "sysfs.h"

-#include <jailhouse/header.h>
#include <jailhouse/hypercall.h>
#include <generated/version.h>

@@ -77,6 +77,8 @@ static void *hypervisor_mem;
static unsigned long hv_core_and_percpu_size;
static atomic_t call_done;
static int error_code;
+static struct jailhouse_console *console_page;
+static bool console_available;

#ifdef CONFIG_X86
bool jailhouse_use_vmcall;
@@ -91,6 +93,18 @@ static void init_hypercall(void)
}
#endif

+static void copy_console_page(struct jailhouse_console *dst)
+{
+retry:
+ /* spin while hypervisor is writing to console */
+ while (console_page->lock);
+ /* copy console page */
+ memcpy(dst, console_page, sizeof(struct jailhouse_console));
+ /* check consistency of console_page */
+ if (dst->tail != console_page->tail)
+ goto retry;
+}
+
static long get_max_cpus(u32 cpu_set_size,
const struct jailhouse_system __user *system_config)
{
@@ -182,6 +196,89 @@ static inline const char * jailhouse_get_fw_name(void)
#endif
}

+static int jailhouse_console_delta(struct jailhouse_console *console,
+ char *dst, unsigned int head,
+ unsigned int *miss)
+{
+ int ret;
+ unsigned int head_mod, tail_mod;
+ unsigned int delta;
+
+ if (miss)
+ *miss = 0;
+
+ /* check if tail overflowed */
+ if (console->tail < head)
+ delta = (0 - head) + console->tail;
+ else
+ delta = console->tail - head;
+
+ /* check if we have misses */
+ if (delta > sizeof(console->content)) {
+ if (miss)
+ *miss = delta - sizeof(console->content);
+ head = console->tail - sizeof(console->content);
+ delta = sizeof(console->content);
+ }
+
+ head_mod = head % sizeof(console->content);
+ tail_mod = console->tail % sizeof(console->content);
+
+ if (head_mod + delta > sizeof(console->content)) {
+ ret = sizeof(console->content) - head_mod;
+ memcpy(dst, console->content + head_mod, ret);
+ delta -= ret;
+ memcpy(dst + ret, console->content, delta);
+ ret += delta;
+ } else {
+ ret = delta;
+ memcpy(dst, console->content + head_mod, delta);
+ }
+
+ return ret;
+}
+
+int jailhouse_console_page_delta(char *dst, unsigned int head,
+ unsigned int *miss)
+{
+ int ret;
+ struct jailhouse_console *console;
+
+ if (mutex_lock_interruptible(&jailhouse_lock) != 0)
+ return -EINTR;
+
+ if (!jailhouse_enabled) {
+ ret = -EAGAIN;
+ goto unlock_out;
+ }
+
+ if (!console_available) {
+ ret = -EPERM;
+ goto unlock_out;
+ }
+
+ console = kmalloc(sizeof(struct jailhouse_console), GFP_KERNEL);
+ if (console == NULL) {
+ ret = -ENOMEM;
+ goto unlock_out;
+ }
+
+ copy_console_page(console);
+ if (console->tail == head) {
+ ret = 0;
+ goto console_free_out;
+ }
+
+ ret = jailhouse_console_delta(console, dst, head, miss);
+
+console_free_out:
+ kfree(console);
+
+unlock_out:
+ mutex_unlock(&jailhouse_lock);
+ return ret;
+}
+
/* See Documentation/bootstrap-interface.txt */
static int jailhouse_cmd_enable(struct jailhouse_system __user *arg)
{
@@ -273,6 +370,9 @@ static int jailhouse_cmd_enable(struct jailhouse_system __user *arg)
goto error_release_fw;
}

+ console_page = (struct jailhouse_console*)
+ (hypervisor_mem + header->console_page);
+
/* Copy hypervisor's binary image at beginning of the memory region
* and clear the rest to zero. */
memcpy(hypervisor_mem, hypervisor->data, hypervisor->size);
@@ -329,6 +429,9 @@ static int jailhouse_cmd_enable(struct jailhouse_system __user *arg)
}
#endif

+ console_available = CON2_TYPE(config->debug_console.flags) ==
+ JAILHOUSE_CON2_TYPE_ROOTPAGE;
+
err = jailhouse_cell_prepare_root(&config->root_cell);
if (err)
goto error_unmap;
diff --git a/driver/main.h b/driver/main.h
index e01ca5b5..8a92c59e 100644
--- a/driver/main.h
+++ b/driver/main.h
@@ -14,6 +14,7 @@
#define _JAILHOUSE_DRIVER_MAIN_H

#include <linux/mutex.h>
+#include <jailhouse/header.h>

#include "cell.h"

@@ -22,5 +23,7 @@ extern bool jailhouse_enabled;

void *jailhouse_ioremap(phys_addr_t phys, unsigned long virt,
unsigned long size);
+int jailhouse_console_page_delta(char *dst, unsigned int head,
+ unsigned int *miss);

#endif /* !_JAILHOUSE_DRIVER_MAIN_H */
diff --git a/driver/sysfs.c b/driver/sysfs.c
index 8da1f766..08c15d6d 100644
--- a/driver/sysfs.c
+++ b/driver/sysfs.c
@@ -309,6 +309,24 @@ void jailhouse_sysfs_cell_delete(struct cell *cell)
kobject_put(&cell->kobj);
}

+static ssize_t console_show(struct device *dev, struct device_attribute *attr,
+ char *buffer)
+{
+ ssize_t ret;
+
+ ret = jailhouse_console_page_delta(buffer, 0, NULL);
+ if (ret > 0)
+ /* We can safely fill the whole buffer and append a terminating
+ * \0, as buffer comes preallocated with PAGE_SIZE and
+ * the content is smaller than PAGE_SIZE */
+ buffer[ret++] = 0;
+
+ /* don't return error if jailhouse is not enabled */
+ if (ret == -EAGAIN)
+ ret = 0;
+ return ret;
+}
+
static ssize_t enabled_show(struct device *dev, struct device_attribute *attr,
char *buffer)
{
@@ -361,6 +379,7 @@ static ssize_t remap_pool_used_show(struct device *dev,
return info_show(dev, buffer, JAILHOUSE_INFO_REMAP_POOL_USED);
}

+static DEVICE_ATTR_RO(console);
static DEVICE_ATTR_RO(enabled);
static DEVICE_ATTR_RO(mem_pool_size);
static DEVICE_ATTR_RO(mem_pool_used);
@@ -368,6 +387,7 @@ static DEVICE_ATTR_RO(remap_pool_size);
static DEVICE_ATTR_RO(remap_pool_used);

static struct attribute *jailhouse_sysfs_entries[] = {
+ &dev_attr_console.attr,
&dev_attr_enabled.attr,
&dev_attr_mem_pool_size.attr,
&dev_attr_mem_pool_used.attr,
--
2.11.0

Ralf Ramsauer

unread,
Jan 16, 2017, 11:07:38 AM1/16/17
to Jan Kiszka, jailho...@googlegroups.com, Ralf Ramsauer
Signed-off-by: Ralf Ramsauer <ralf.r...@oth-regensburg.de>
---
Documentation/debug-output.md | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/Documentation/debug-output.md b/Documentation/debug-output.md
index 6367fbcd..1c11ca18 100644
--- a/Documentation/debug-output.md
+++ b/Documentation/debug-output.md
@@ -84,6 +84,13 @@ Example configuration for disabled debug output (architecture independent):
}
```

+Hypervisor Console via sysfs
+----------------------------
+
+If the debug console of root cell has the flag JAILHOUSE_CON2_TYPE_ROOTPAGE
+set, the hypervisor console is available through
+/sys/devices/jailhouse/console.
+
Inmates
-------

--
2.11.0

Ralf Ramsauer

unread,
Jan 16, 2017, 11:07:38 AM1/16/17
to Jan Kiszka, jailho...@googlegroups.com, Ralf Ramsauer
We will need this one earlier in the next patch. No functional change.

Signed-off-by: Ralf Ramsauer <ralf.r...@oth-regensburg.de>
---
tools/jailhouse.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/tools/jailhouse.c b/tools/jailhouse.c
index 84e5fe3d..ab495fda 100644
--- a/tools/jailhouse.c
+++ b/tools/jailhouse.c
@@ -79,6 +79,13 @@ static void __attribute__((noreturn)) help(char *prog, int exit_status)
exit(exit_status);
}

+static bool match_opt(const char *argv, const char *short_opt,
+ const char *long_opt)
+{
+ return strcmp(argv, short_opt) == 0 ||
+ strcmp(argv, long_opt) == 0;
+}
+
static void call_extension_script(const char *cmd, int argc, char *argv[])
{
const struct extension *ext;
@@ -277,13 +284,6 @@ static int parse_cell_id(struct jailhouse_cell_id *cell_id, int argc,
return arg_pos + 1;
}

-static bool match_opt(const char *argv, const char *short_opt,
- const char *long_opt)
-{
- return strcmp(argv, short_opt) == 0 ||
- strcmp(argv, long_opt) == 0;
-}
-
static struct jailhouse_cell_info *get_cell_info(const unsigned int id)
{
struct jailhouse_cell_info *cinfo;
--
2.11.0

Ralf Ramsauer

unread,
Jan 16, 2017, 11:07:38 AM1/16/17
to Jan Kiszka, jailho...@googlegroups.com, Ralf Ramsauer
for all existing system configuration. Activate it in the root cell
template as well.

Signed-off-by: Ralf Ramsauer <ralf.r...@oth-regensburg.de>
---
configs/amd-seattle.c | 3 ++-
configs/bananapi.c | 3 ++-
configs/f2a88xm-hd3.c | 3 ++-
configs/foundation-v8.c | 3 ++-
configs/imb-a180.c | 3 ++-
configs/jetson-tk1.c | 3 ++-
configs/jetson-tx1.c | 3 ++-
configs/orangepi0.c | 3 ++-
configs/qemu-vm.c | 3 ++-
configs/vexpress.c | 3 ++-
configs/zynqmp-zcu102.c | 3 ++-
tools/root-cell-config.c.tmpl | 3 ++-
12 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/configs/amd-seattle.c b/configs/amd-seattle.c
index 66e3fdcd..81bc6208 100644
--- a/configs/amd-seattle.c
+++ b/configs/amd-seattle.c
@@ -33,7 +33,8 @@ struct {
.address = 0xe1010000,
.size = 0x1000,
.flags = JAILHOUSE_CON1_TYPE_PL011 |
- JAILHOUSE_CON1_FLAG_MMIO,
+ JAILHOUSE_CON1_FLAG_MMIO |
+ JAILHOUSE_CON2_TYPE_ROOTPAGE,
},
.platform_info = {
.pci_mmconfig_base = 0xf0000000,
diff --git a/configs/bananapi.c b/configs/bananapi.c
index 484d808a..f289a3e3 100644
--- a/configs/bananapi.c
+++ b/configs/bananapi.c
@@ -38,7 +38,8 @@ struct {
/* .gate_nr = 16 */
/* .divider = 0x0d, */
.flags = JAILHOUSE_CON1_TYPE_8250 |
- JAILHOUSE_CON1_FLAG_MMIO,
+ JAILHOUSE_CON1_FLAG_MMIO |
+ JAILHOUSE_CON2_TYPE_ROOTPAGE,
},
.platform_info = {
.pci_mmconfig_base = 0x2000000,
diff --git a/configs/f2a88xm-hd3.c b/configs/f2a88xm-hd3.c
index 62a9117b..33973bcf 100644
--- a/configs/f2a88xm-hd3.c
+++ b/configs/f2a88xm-hd3.c
@@ -42,7 +42,8 @@ struct {
.address = 0x3f8,
/* .divider = 0x1, */
.flags = JAILHOUSE_CON1_TYPE_UART_X86 |
- JAILHOUSE_CON1_FLAG_PIO,
+ JAILHOUSE_CON1_FLAG_PIO |
+ JAILHOUSE_CON2_TYPE_ROOTPAGE,
},
.platform_info = {
.pci_mmconfig_base = 0xe0000000,
diff --git a/configs/foundation-v8.c b/configs/foundation-v8.c
index c4112004..56e4816d 100644
--- a/configs/foundation-v8.c
+++ b/configs/foundation-v8.c
@@ -32,7 +32,8 @@ struct {
.address = 0x1c090000,
.size = 0x1000,
.flags = JAILHOUSE_CON1_TYPE_PL011 |
- JAILHOUSE_CON1_FLAG_MMIO,
+ JAILHOUSE_CON1_FLAG_MMIO |
+ JAILHOUSE_CON2_TYPE_ROOTPAGE,
},
.platform_info.arm = {
#ifdef CONFIG_ARM_GIC_V3
diff --git a/configs/imb-a180.c b/configs/imb-a180.c
index fa36d791..a6b9012c 100644
--- a/configs/imb-a180.c
+++ b/configs/imb-a180.c
@@ -41,7 +41,8 @@ struct {
.address = 0x3f8,
/* .divider = 0x1, */
.flags = JAILHOUSE_CON1_TYPE_UART_X86 |
- JAILHOUSE_CON1_FLAG_PIO,
+ JAILHOUSE_CON1_FLAG_PIO |
+ JAILHOUSE_CON2_TYPE_ROOTPAGE,
},
.platform_info = {
.pci_mmconfig_base = 0xe0000000,
diff --git a/configs/jetson-tk1.c b/configs/jetson-tk1.c
index d6713405..2cdb4924 100644
--- a/configs/jetson-tk1.c
+++ b/configs/jetson-tk1.c
@@ -41,7 +41,8 @@ struct {
/* .gate_nr = (65 % 32), */
/* .divider = 0xdd, */
.flags = JAILHOUSE_CON1_TYPE_8250 |
- JAILHOUSE_CON1_FLAG_MMIO,
+ JAILHOUSE_CON1_FLAG_MMIO |
+ JAILHOUSE_CON2_TYPE_ROOTPAGE,
},
.platform_info = {
.pci_mmconfig_base = 0x48000000,
diff --git a/configs/jetson-tx1.c b/configs/jetson-tx1.c
index af1f5577..f47ebfd4 100644
--- a/configs/jetson-tx1.c
+++ b/configs/jetson-tx1.c
@@ -35,7 +35,8 @@ struct {
.address = 0x70006000,
.size = 0x0040,
.flags = JAILHOUSE_CON1_TYPE_8250 |
- JAILHOUSE_CON1_FLAG_MMIO,
+ JAILHOUSE_CON1_FLAG_MMIO |
+ JAILHOUSE_CON2_TYPE_ROOTPAGE,
},
.platform_info.arm = {
.gicd_base = 0x50041000,
diff --git a/configs/orangepi0.c b/configs/orangepi0.c
index 14f1a0e0..ae1da186 100644
--- a/configs/orangepi0.c
+++ b/configs/orangepi0.c
@@ -35,7 +35,8 @@ struct {
.address = 0x01c28000,
.size = 0x1000,
.flags = JAILHOUSE_CON1_TYPE_8250 |
- JAILHOUSE_CON1_FLAG_MMIO,
+ JAILHOUSE_CON1_FLAG_MMIO |
+ JAILHOUSE_CON2_TYPE_ROOTPAGE,
},
.platform_info = {
.pci_mmconfig_base = 0x2000000,
diff --git a/configs/qemu-vm.c b/configs/qemu-vm.c
index 0ce34eea..f091834e 100644
--- a/configs/qemu-vm.c
+++ b/configs/qemu-vm.c
@@ -41,7 +41,8 @@ struct {
.address = 0x3f8,
/* .divider = 0x1, */
.flags = JAILHOUSE_CON1_TYPE_UART_X86 |
- JAILHOUSE_CON1_FLAG_PIO,
+ JAILHOUSE_CON1_FLAG_PIO |
+ JAILHOUSE_CON2_TYPE_ROOTPAGE,
},
.platform_info = {
.pci_mmconfig_base = 0xb0000000,
diff --git a/configs/vexpress.c b/configs/vexpress.c
index 61c99b9b..065db656 100644
--- a/configs/vexpress.c
+++ b/configs/vexpress.c
@@ -32,7 +32,8 @@ struct {
.address = 0x1c090000,
.size = 0x1000,
.flags = JAILHOUSE_CON1_TYPE_PL011 |
- JAILHOUSE_CON1_FLAG_MMIO,
+ JAILHOUSE_CON1_FLAG_MMIO |
+ JAILHOUSE_CON2_TYPE_ROOTPAGE,
},
.platform_info.arm = {
#ifdef CONFIG_ARM_GIC_V3
diff --git a/configs/zynqmp-zcu102.c b/configs/zynqmp-zcu102.c
index 120a73dc..458fc24f 100644
--- a/configs/zynqmp-zcu102.c
+++ b/configs/zynqmp-zcu102.c
@@ -35,7 +35,8 @@ struct {
.address = 0xff000000,
.size = 0x1000,
.flags = JAILHOUSE_CON1_TYPE_XUARTPS |
- JAILHOUSE_CON1_FLAG_MMIO,
+ JAILHOUSE_CON1_FLAG_MMIO |
+ JAILHOUSE_CON2_TYPE_ROOTPAGE,
},
.platform_info = {
.pci_mmconfig_base = 0xfc000000,
diff --git a/tools/root-cell-config.c.tmpl b/tools/root-cell-config.c.tmpl
index 85c1834a..11956b74 100644
--- a/tools/root-cell-config.c.tmpl
+++ b/tools/root-cell-config.c.tmpl
@@ -63,7 +63,8 @@ struct {
.debug_console = {
.address = 0x3f8,
.flags = JAILHOUSE_CON1_TYPE_UART_X86 |
- JAILHOUSE_CON1_FLAG_PIO,
+ JAILHOUSE_CON1_FLAG_PIO |
+ JAILHOUSE_CON2_TYPE_ROOTPAGE,
},
.platform_info = {
.pci_mmconfig_base = ${hex(mmconfig.base)},
--
2.11.0

Ralf Ramsauer

unread,
Jan 16, 2017, 11:07:38 AM1/16/17
to Jan Kiszka, jailho...@googlegroups.com, Ralf Ramsauer
We're about to implement a second HV console channel: a 'virtual'
console available for the root cell via sysfs. In order to encode its
configuration in struct jailhouse_debug_console, rename
JAILHOUSE_CON_TYPE to JAILHOUSE_CON1_TYPE.

Most of the work was done automatically with sed.

Signed-off-by: Ralf Ramsauer <ralf.r...@oth-regensburg.de>
---
Documentation/debug-output.md | 24 ++++++++++++------------
Documentation/vga-console.txt | 2 +-
configs/amd-seattle.c | 4 ++--
configs/bananapi.c | 4 ++--
configs/f2a88xm-hd3.c | 4 ++--
configs/foundation-v8.c | 4 ++--
configs/hikey.c | 4 ++--
configs/imb-a180.c | 4 ++--
configs/jetson-tk1.c | 4 ++--
configs/jetson-tx1.c | 4 ++--
configs/orangepi0.c | 4 ++--
configs/qemu-vm.c | 4 ++--
configs/vexpress.c | 4 ++--
configs/zynqmp-zcu102.c | 4 ++--
driver/main.c | 2 +-
hypervisor/arch/arm-common/dbg-write.c | 10 +++++-----
hypervisor/arch/x86/dbg-write.c | 6 +++---
hypervisor/arch/x86/uart.c | 2 +-
hypervisor/include/jailhouse/cell-config.h | 22 +++++++++++-----------
hypervisor/paging.c | 2 +-
tools/root-cell-config.c.tmpl | 4 ++--
21 files changed, 61 insertions(+), 61 deletions(-)

diff --git a/Documentation/debug-output.md b/Documentation/debug-output.md
index 1e5888fc..6367fbcd 100644
--- a/Documentation/debug-output.md
+++ b/Documentation/debug-output.md
@@ -12,22 +12,22 @@ options.
### .flags
All architectures support the empty debug output driver, which is selected by
default if nothing else is chosen:
- - JAILHOUSE_CON_TYPE_NONE
+ - JAILHOUSE_CON1_TYPE_NONE

Possible debug outputs for x86:
- - JAILHOUSE_CON_TYPE_UART_X86 /* generic X86 PIO/MMIO UART driver */
- - JAILHOUSE_CON_TYPE_VGA /* VGA console */
+ - JAILHOUSE_CON1_TYPE_UART_X86 /* generic X86 PIO/MMIO UART driver */
+ - JAILHOUSE_CON1_TYPE_VGA /* VGA console */

VGA output is only available for x86. For further documentation on VGA output
see [vga-console.txt](vga-console.txt).

Possible debug outputs for arm and arm64:
- - JAILHOUSE_CON_TYPE_8250 /* 8250 compatible UART */
- - JAILHOUSE_CON_TYPE_PL011 /* AMBA PL011 UART */
+ - JAILHOUSE_CON1_TYPE_8250 /* 8250 compatible UART */
+ - JAILHOUSE_CON1_TYPE_PL011 /* AMBA PL011 UART */

Additional flags that can be or'ed:
- - JAILHOUSE_CON_FLAG_PIO /* x86 only */
- - JAILHOUSE_CON_FLAG_MMIO /* x86 and ARM. Should always be selected for
+ - JAILHOUSE_CON1_FLAG_PIO /* x86 only */
+ - JAILHOUSE_CON1_FLAG_MMIO /* x86 and ARM. Should always be selected for
* ARM. */

### .address and .size
@@ -59,8 +59,8 @@ Example configuration for PIO based debug output on x86:
.debug_console = {
.address = 0x3f8, /* PIO address */
.divider = 0x1, /* 115200 Baud */
- .flags = JAILHOUSE_CON_TYPE_UART_X86 | /* generic x86 UART driver */
- JAILHOUSE_CON_FLAG_PIO, /* use PIO instead of MMIO */
+ .flags = JAILHOUSE_CON1_TYPE_UART_X86 | /* generic x86 UART driver */
+ JAILHOUSE_CON1_FLAG_PIO, /* use PIO instead of MMIO */
},
```

@@ -72,15 +72,15 @@ Example configuration for MMIO based debug output on ARM (8250 UART):
.clock_reg = 0x60006000 + 0x330, /* Optional: Debug Clock Register */
.gate_nr = (65 % 32), /* Optional: Debug Clock Gate Nr */
.divider = 0xdd, /* 115200 */
- .flags = JAILHOUSE_CON_TYPE_8250 | /* choose the 8250 driver */
- JAILHOUSE_CON_FLAG_MMIO, /* choose MMIO register access */
+ .flags = JAILHOUSE_CON1_TYPE_8250 | /* choose the 8250 driver */
+ JAILHOUSE_CON1_FLAG_MMIO, /* choose MMIO register access */
},
```

Example configuration for disabled debug output (architecture independent):
```
.debug_console = {
- .flags = JAILHOUSE_CON_TYPE_NONE,
+ .flags = JAILHOUSE_CON1_TYPE_NONE,
}
```

diff --git a/Documentation/vga-console.txt b/Documentation/vga-console.txt
index 7b378264..81dcc403 100644
--- a/Documentation/vga-console.txt
+++ b/Documentation/vga-console.txt
@@ -20,7 +20,7 @@ Add the following to the header section of your root cell's config:
.debug_console = {
.address = 0xb8000,
.size = 0x1000,
- .flags = JAILHOUSE_CON_TYPE_VGA | JAILHOUSE_CON_FLAG_MMIO,
+ .flags = JAILHOUSE_CON1_TYPE_VGA | JAILHOUSE_CON1_FLAG_MMIO,
},

Boot using the following kernel parameters:
diff --git a/configs/amd-seattle.c b/configs/amd-seattle.c
index 7d32eacc..66e3fdcd 100644
--- a/configs/amd-seattle.c
+++ b/configs/amd-seattle.c
@@ -32,8 +32,8 @@ struct {
.debug_console = {
.address = 0xe1010000,
.size = 0x1000,
- .flags = JAILHOUSE_CON_TYPE_PL011 |
- JAILHOUSE_CON_FLAG_MMIO,
+ .flags = JAILHOUSE_CON1_TYPE_PL011 |
+ JAILHOUSE_CON1_FLAG_MMIO,
},
.platform_info = {
.pci_mmconfig_base = 0xf0000000,
diff --git a/configs/bananapi.c b/configs/bananapi.c
index ef7f8240..484d808a 100644
--- a/configs/bananapi.c
+++ b/configs/bananapi.c
@@ -37,8 +37,8 @@ struct {
/* .clock_reg = 0x01c2006c, */
/* .gate_nr = 16 */
/* .divider = 0x0d, */
- .flags = JAILHOUSE_CON_TYPE_8250 |
- JAILHOUSE_CON_FLAG_MMIO,
+ .flags = JAILHOUSE_CON1_TYPE_8250 |
+ JAILHOUSE_CON1_FLAG_MMIO,
},
.platform_info = {
.pci_mmconfig_base = 0x2000000,
diff --git a/configs/f2a88xm-hd3.c b/configs/f2a88xm-hd3.c
index 7c57407b..62a9117b 100644
--- a/configs/f2a88xm-hd3.c
+++ b/configs/f2a88xm-hd3.c
@@ -41,8 +41,8 @@ struct {
.debug_console = {
.address = 0x3f8,
/* .divider = 0x1, */
- .flags = JAILHOUSE_CON_TYPE_UART_X86 |
- JAILHOUSE_CON_FLAG_PIO,
+ .flags = JAILHOUSE_CON1_TYPE_UART_X86 |
+ JAILHOUSE_CON1_FLAG_PIO,
},
.platform_info = {
.pci_mmconfig_base = 0xe0000000,
diff --git a/configs/foundation-v8.c b/configs/foundation-v8.c
index 9ae97556..c4112004 100644
--- a/configs/foundation-v8.c
+++ b/configs/foundation-v8.c
@@ -31,8 +31,8 @@ struct {
.debug_console = {
.address = 0x1c090000,
.size = 0x1000,
- .flags = JAILHOUSE_CON_TYPE_PL011 |
- JAILHOUSE_CON_FLAG_MMIO,
+ .flags = JAILHOUSE_CON1_TYPE_PL011 |
+ JAILHOUSE_CON1_FLAG_MMIO,
},
.platform_info.arm = {
#ifdef CONFIG_ARM_GIC_V3
diff --git a/configs/hikey.c b/configs/hikey.c
index c45fc96e..a739222b 100644
--- a/configs/hikey.c
+++ b/configs/hikey.c
@@ -32,8 +32,8 @@ struct {
.debug_console = {
.address = 0xf7113000,
.size = 0x1000,
- .flags = JAILHOUSE_CON_TYPE_PL011 |
- JAILHOUSE_CON_FLAG_MMIO,
+ .flags = JAILHOUSE_CON1_TYPE_PL011 |
+ JAILHOUSE_CON1_FLAG_MMIO,
},
.platform_info = {
.pci_mmconfig_base = 0xf6000000,
diff --git a/configs/imb-a180.c b/configs/imb-a180.c
index d5987bbe..fa36d791 100644
--- a/configs/imb-a180.c
+++ b/configs/imb-a180.c
@@ -40,8 +40,8 @@ struct {
.debug_console = {
.address = 0x3f8,
/* .divider = 0x1, */
- .flags = JAILHOUSE_CON_TYPE_UART_X86 |
- JAILHOUSE_CON_FLAG_PIO,
+ .flags = JAILHOUSE_CON1_TYPE_UART_X86 |
+ JAILHOUSE_CON1_FLAG_PIO,
},
.platform_info = {
.pci_mmconfig_base = 0xe0000000,
diff --git a/configs/jetson-tk1.c b/configs/jetson-tk1.c
index b8176dee..d6713405 100644
--- a/configs/jetson-tk1.c
+++ b/configs/jetson-tk1.c
@@ -40,8 +40,8 @@ struct {
/* .clock_reg = 0x60006000 + 0x330, */
/* .gate_nr = (65 % 32), */
/* .divider = 0xdd, */
- .flags = JAILHOUSE_CON_TYPE_8250 |
- JAILHOUSE_CON_FLAG_MMIO,
+ .flags = JAILHOUSE_CON1_TYPE_8250 |
+ JAILHOUSE_CON1_FLAG_MMIO,
},
.platform_info = {
.pci_mmconfig_base = 0x48000000,
diff --git a/configs/jetson-tx1.c b/configs/jetson-tx1.c
index cf80aa47..af1f5577 100644
--- a/configs/jetson-tx1.c
+++ b/configs/jetson-tx1.c
@@ -34,8 +34,8 @@ struct {
.debug_console = {
.address = 0x70006000,
.size = 0x0040,
- .flags = JAILHOUSE_CON_TYPE_8250 |
- JAILHOUSE_CON_FLAG_MMIO,
+ .flags = JAILHOUSE_CON1_TYPE_8250 |
+ JAILHOUSE_CON1_FLAG_MMIO,
},
.platform_info.arm = {
.gicd_base = 0x50041000,
diff --git a/configs/orangepi0.c b/configs/orangepi0.c
index a37e5d49..14f1a0e0 100644
--- a/configs/orangepi0.c
+++ b/configs/orangepi0.c
@@ -34,8 +34,8 @@ struct {
.debug_console = {
.address = 0x01c28000,
.size = 0x1000,
- .flags = JAILHOUSE_CON_TYPE_8250 |
- JAILHOUSE_CON_FLAG_MMIO,
+ .flags = JAILHOUSE_CON1_TYPE_8250 |
+ JAILHOUSE_CON1_FLAG_MMIO,
},
.platform_info = {
.pci_mmconfig_base = 0x2000000,
diff --git a/configs/qemu-vm.c b/configs/qemu-vm.c
index 04d0c947..0ce34eea 100644
--- a/configs/qemu-vm.c
+++ b/configs/qemu-vm.c
@@ -40,8 +40,8 @@ struct {
.debug_console = {
.address = 0x3f8,
/* .divider = 0x1, */
- .flags = JAILHOUSE_CON_TYPE_UART_X86 |
- JAILHOUSE_CON_FLAG_PIO,
+ .flags = JAILHOUSE_CON1_TYPE_UART_X86 |
+ JAILHOUSE_CON1_FLAG_PIO,
},
.platform_info = {
.pci_mmconfig_base = 0xb0000000,
diff --git a/configs/vexpress.c b/configs/vexpress.c
index 15202970..61c99b9b 100644
--- a/configs/vexpress.c
+++ b/configs/vexpress.c
@@ -31,8 +31,8 @@ struct {
.debug_console = {
.address = 0x1c090000,
.size = 0x1000,
- .flags = JAILHOUSE_CON_TYPE_PL011 |
- JAILHOUSE_CON_FLAG_MMIO,
+ .flags = JAILHOUSE_CON1_TYPE_PL011 |
+ JAILHOUSE_CON1_FLAG_MMIO,
},
.platform_info.arm = {
#ifdef CONFIG_ARM_GIC_V3
diff --git a/configs/zynqmp-zcu102.c b/configs/zynqmp-zcu102.c
index d236ae6f..120a73dc 100644
--- a/configs/zynqmp-zcu102.c
+++ b/configs/zynqmp-zcu102.c
@@ -34,8 +34,8 @@ struct {
.debug_console = {
.address = 0xff000000,
.size = 0x1000,
- .flags = JAILHOUSE_CON_TYPE_XUARTPS |
- JAILHOUSE_CON_FLAG_MMIO,
+ .flags = JAILHOUSE_CON1_TYPE_XUARTPS |
+ JAILHOUSE_CON1_FLAG_MMIO,
},
.platform_info = {
.pci_mmconfig_base = 0xfc000000,
diff --git a/driver/main.c b/driver/main.c
index e793d9d7..676794be 100644
--- a/driver/main.c
+++ b/driver/main.c
@@ -301,7 +301,7 @@ static int jailhouse_cmd_enable(struct jailhouse_system __user *arg)
}

#ifdef JAILHOUSE_BORROW_ROOT_PT
- if (CON_IS_MMIO(config->debug_console.flags)) {
+ if (CON1_IS_MMIO(config->debug_console.flags)) {
console = ioremap(config->debug_console.address,
config->debug_console.size);
if (!console) {
diff --git a/hypervisor/arch/arm-common/dbg-write.c b/hypervisor/arch/arm-common/dbg-write.c
index e88b18b9..e9e51a32 100644
--- a/hypervisor/arch/arm-common/dbg-write.c
+++ b/hypervisor/arch/arm-common/dbg-write.c
@@ -42,16 +42,16 @@ static void arm_uart_write(const char *msg)

void arch_dbg_write_init(void)
{
- unsigned char con_type = CON_TYPE(system_config->debug_console.flags);
+ unsigned char con_type = CON1_TYPE(system_config->debug_console.flags);

- if (!CON_IS_MMIO(system_config->debug_console.flags))
+ if (!CON1_IS_MMIO(system_config->debug_console.flags))
return;

- if (con_type == JAILHOUSE_CON_TYPE_PL011)
+ if (con_type == JAILHOUSE_CON1_TYPE_PL011)
uart = &uart_pl011_ops;
- else if (con_type == JAILHOUSE_CON_TYPE_8250)
+ else if (con_type == JAILHOUSE_CON1_TYPE_8250)
uart = &uart_8250_ops;
- else if (con_type == JAILHOUSE_CON_TYPE_XUARTPS)
+ else if (con_type == JAILHOUSE_CON1_TYPE_XUARTPS)
uart = &uart_xuartps_ops;

if (uart) {
diff --git a/hypervisor/arch/x86/dbg-write.c b/hypervisor/arch/x86/dbg-write.c
index 60e8adb4..30ddc908 100644
--- a/hypervisor/arch/x86/dbg-write.c
+++ b/hypervisor/arch/x86/dbg-write.c
@@ -17,13 +17,13 @@

void arch_dbg_write_init(void)
{
- unsigned char dbg_type = CON_TYPE(system_config->debug_console.flags);
+ unsigned char dbg_type = CON1_TYPE(system_config->debug_console.flags);

/* PIO / MMIO differentiation is done inside the driver code */
- if (dbg_type == JAILHOUSE_CON_TYPE_UART_X86) {
+ if (dbg_type == JAILHOUSE_CON1_TYPE_UART_X86) {
uart_init();
arch_dbg_write = uart_write;
- } else if (dbg_type == JAILHOUSE_CON_TYPE_VGA) {
+ } else if (dbg_type == JAILHOUSE_CON1_TYPE_VGA) {
vga_init();
arch_dbg_write = vga_write;
}
diff --git a/hypervisor/arch/x86/uart.c b/hypervisor/arch/x86/uart.c
index 736cf3b2..09b9e84f 100644
--- a/hypervisor/arch/x86/uart.c
+++ b/hypervisor/arch/x86/uart.c
@@ -57,7 +57,7 @@ void uart_init(void)
u32 flags = system_config->debug_console.flags;
u32 divider = system_config->debug_console.divider;

- if (CON_IS_MMIO(flags)) {
+ if (CON1_IS_MMIO(flags)) {
uart_reg_out = uart_mmio32_out;
uart_reg_in = uart_mmio32_in;
uart_base = (u64)hypervisor_header.debug_console_base;
diff --git a/hypervisor/include/jailhouse/cell-config.h b/hypervisor/include/jailhouse/cell-config.h
index 2f8c7cb9..80fa5a78 100644
--- a/hypervisor/include/jailhouse/cell-config.h
+++ b/hypervisor/include/jailhouse/cell-config.h
@@ -176,21 +176,21 @@ struct jailhouse_iommu {
} __attribute__((packed));

/* Bits 0..3 are used to select the particular driver */
-#define JAILHOUSE_CON_TYPE_NONE 0x0000
-#define JAILHOUSE_CON_TYPE_UART_X86 0x0001
-#define JAILHOUSE_CON_TYPE_VGA 0x0002
-#define JAILHOUSE_CON_TYPE_8250 0x0003
-#define JAILHOUSE_CON_TYPE_PL011 0x0004
-#define JAILHOUSE_CON_TYPE_XUARTPS 0x0005
-#define JAILHOUSE_CON_TYPE_MASK 0x000f
+#define JAILHOUSE_CON1_TYPE_NONE 0x0000
+#define JAILHOUSE_CON1_TYPE_UART_X86 0x0001
+#define JAILHOUSE_CON1_TYPE_VGA 0x0002
+#define JAILHOUSE_CON1_TYPE_8250 0x0003
+#define JAILHOUSE_CON1_TYPE_PL011 0x0004
+#define JAILHOUSE_CON1_TYPE_XUARTPS 0x0005
+#define JAILHOUSE_CON1_TYPE_MASK 0x000f

-#define CON_TYPE(flags) ((flags) & JAILHOUSE_CON_TYPE_MASK)
+#define CON1_TYPE(flags) ((flags) & JAILHOUSE_CON1_TYPE_MASK)

/* We use bit 4..5 to differentiate between PIO and MMIO access */
-#define JAILHOUSE_CON_FLAG_PIO 0x0010
-#define JAILHOUSE_CON_FLAG_MMIO 0x0020
+#define JAILHOUSE_CON1_FLAG_PIO 0x0010
+#define JAILHOUSE_CON1_FLAG_MMIO 0x0020

-#define CON_IS_MMIO(flags) ((flags) & JAILHOUSE_CON_FLAG_MMIO)
+#define CON1_IS_MMIO(flags) ((flags) & JAILHOUSE_CON1_FLAG_MMIO)

struct jailhouse_debug_console {
__u64 address;
diff --git a/hypervisor/paging.c b/hypervisor/paging.c
index 67ed63f4..a431550f 100644
--- a/hypervisor/paging.c
+++ b/hypervisor/paging.c
@@ -595,7 +595,7 @@ int paging_init(void)
if (err)
return err;

- if (CON_IS_MMIO(system_config->debug_console.flags)) {
+ if (CON1_IS_MMIO(system_config->debug_console.flags)) {
vaddr = (unsigned long)hypervisor_header.debug_console_base;
/* check if console overlaps remapping region */
if (vaddr + system_config->debug_console.size >= REMAP_BASE &&
diff --git a/tools/root-cell-config.c.tmpl b/tools/root-cell-config.c.tmpl
index b7be8b74..85c1834a 100644
--- a/tools/root-cell-config.c.tmpl
+++ b/tools/root-cell-config.c.tmpl
@@ -62,8 +62,8 @@ struct {
},
.debug_console = {
.address = 0x3f8,
- .flags = JAILHOUSE_CON_TYPE_UART_X86 |
- JAILHOUSE_CON_FLAG_PIO,
+ .flags = JAILHOUSE_CON1_TYPE_UART_X86 |
+ JAILHOUSE_CON1_FLAG_PIO,

Ralf Ramsauer

unread,
Jan 16, 2017, 11:07:38 AM1/16/17
to Jan Kiszka, jailho...@googlegroups.com, Ralf Ramsauer
Signed-off-by: Ralf Ramsauer <ralf.r...@oth-regensburg.de>
---
Documentation/debug-output.md | 8 +++-
driver/main.c | 89 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 96 insertions(+), 1 deletion(-)

diff --git a/Documentation/debug-output.md b/Documentation/debug-output.md
index 1c11ca18..5d0610a1 100644
--- a/Documentation/debug-output.md
+++ b/Documentation/debug-output.md
@@ -89,7 +89,13 @@ Hypervisor Console via sysfs

If the debug console of root cell has the flag JAILHOUSE_CON2_TYPE_ROOTPAGE
set, the hypervisor console is available through
-/sys/devices/jailhouse/console.
+/sys/devices/jailhouse/console. Continuous reading of the hypervisor console
+is available through /dev/jailhouse.
+
+Example
+```
+cat /dev/jailhouse
+```

Inmates
-------
diff --git a/driver/main.c b/driver/main.c
index 8a4b929f..f98e63c6 100644
--- a/driver/main.c
+++ b/driver/main.c
@@ -69,6 +69,10 @@ MODULE_FIRMWARE(JAILHOUSE_FW_NAME);
#endif
MODULE_VERSION(JAILHOUSE_VERSION);

+struct console_userdata {
+ unsigned int head;
+};
+
DEFINE_MUTEX(jailhouse_lock);
bool jailhouse_enabled;

@@ -621,11 +625,96 @@ static long jailhouse_ioctl(struct file *file, unsigned int ioctl,
return err;
}

+static int jailhouse_console_open(struct inode *inode, struct file *file)
+{
+ struct console_userdata *user;
+
+ file->private_data = NULL;
+ user = kzalloc(sizeof(struct console_userdata), GFP_KERNEL);
+ if (!user)
+ return -ENOMEM;
+
+ file->private_data = user;
+ return 0;
+}
+
+static int jailhouse_console_release(struct inode *inode, struct file *file)
+{
+ struct console_userdata *user = file->private_data;
+ if (user)
+ kfree(user);
+ return 0;
+}
+
+static ssize_t jailhouse_console_read(struct file *file, char __user *out,
+ size_t size, loff_t *off)
+{
+ struct console_userdata *user = file->private_data;
+ char *content;
+ unsigned int miss;
+ int ret;
+
+ content = kmalloc(sizeof(console_page->content), GFP_KERNEL);
+ if (content == NULL)
+ return -ENOMEM;
+
+ /* wait for new data */
+ while (1) {
+ ret = jailhouse_console_page_delta(content, user->head, &miss);
+ if ((!ret || ret == -EAGAIN) && file->f_flags & O_NONBLOCK)
+ goto console_free_out;
+
+ if (ret == -EAGAIN)
+ /* Reset the user head, if jailhouse is not enabled. We
+ * have to do this, as jailhouse might be reenabled and
+ * the file handle was kept open in the meanwhile */
+ user->head = 0;
+ else if (ret < 0)
+ goto console_free_out;
+ else if (ret)
+ break;
+
+ schedule_timeout(HZ / 10);
+ set_current_state(TASK_UNINTERRUPTIBLE);
+ if (signal_pending(current)) {
+ ret = -EINTR;
+ goto console_free_out;
+ }
+ }
+
+ if (miss) {
+ /* If we missed anything, warn user. We will dump the actual
+ * content in the next call. */
+ ret = snprintf(content, sizeof(console_page->content),
+ "NOTE: missing %u byte console log\n",
+ miss);
+ user->head += miss;
+ if (size < ret)
+ ret = size;
+ } else {
+ if (size < ret)
+ ret = size;
+ user->head += ret;
+ }
+
+ if (copy_to_user(out, content, ret))
+ ret = -EFAULT;
+
+console_free_out:
+ set_current_state(TASK_RUNNING);
+ kfree(content);
+ return ret;
+}
+
+
static const struct file_operations jailhouse_fops = {
.owner = THIS_MODULE,
.unlocked_ioctl = jailhouse_ioctl,
.compat_ioctl = jailhouse_ioctl,
.llseek = noop_llseek,
+ .open = jailhouse_console_open,
+ .release = jailhouse_console_release,
+ .read = jailhouse_console_read,
};

static struct miscdevice jailhouse_misc_dev = {
--
2.11.0

Ralf Ramsauer

unread,
Jan 16, 2017, 11:07:38 AM1/16/17
to Jan Kiszka, jailho...@googlegroups.com, Ralf Ramsauer
Similar to other tools, like 'tail -f', allow 'jailhosue enable' to
follow the console output when jailhouse gets enabled.

On failure, we will print the console as well (if available).

'jailhouse enable CELL -v' will print the hypervisor's console content
upon enabling jailhouse and terminate.

Signed-off-by: Ralf Ramsauer <ralf.r...@oth-regensburg.de>
---
tools/jailhouse.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++++------
1 file changed, 57 insertions(+), 7 deletions(-)

diff --git a/tools/jailhouse.c b/tools/jailhouse.c
index ab495fda..5760b087 100644
--- a/tools/jailhouse.c
+++ b/tools/jailhouse.c
@@ -62,7 +62,8 @@ static void __attribute__((noreturn)) help(char *prog, int exit_status)

printf("Usage: %s { COMMAND | --help || --version }\n"
"\nAvailable commands:\n"
- " enable SYSCONFIG\n"
+ " enable SYSCONFIG"
+ " { [ -f | --follow ] | [ -v | --verbose] }\n"
" disable\n"
" cell create CELLCONFIG\n"
" cell list\n"
@@ -86,6 +87,21 @@ static bool match_opt(const char *argv, const char *short_opt,
strcmp(argv, long_opt) == 0;
}

+static int fd_set_nonblock(int fd)
+{
+ int ret;
+
+ ret = fcntl(fd, F_GETFL, 0);
+ if (ret == -1)
+ return -errno;
+
+ ret |= O_NONBLOCK;
+ ret = fcntl(fd, F_SETFL, ret);
+ if (ret == -1)
+ return -errno;
+ return 0;
+}
+
static void call_extension_script(const char *cmd, int argc, char *argv[])
{
const struct extension *ext;
@@ -207,21 +223,55 @@ static int enable(int argc, char *argv[])
{
void *config;
int err, fd;
+ char console_buffer[128];
+ ssize_t r;
+ bool dump_console = false;

- if (argc != 3)
+ fd = open_dev();
+
+ if (argc < 3 || argc > 4)
help(argv[0], 1);

- config = read_file(argv[2], NULL);
+ if (argc == 4 && match_opt(argv[3], "-f", "--follow"))
+ dump_console = true;

- fd = open_dev();
+ if (argc == 4 && match_opt(argv[3], "-v", "--verbose")) {
+ dump_console = true;
+ err = fd_set_nonblock(fd);
+ if (err) {
+ perror("FD_SET_NONBLOCK");
+ goto fd_close;
+ }
+ }
+
+ config = read_file(argv[2], NULL);

err = ioctl(fd, JAILHOUSE_ENABLE, config);
- if (err)
+ if (err) {
perror("JAILHOUSE_ENABLE");
+ err = fd_set_nonblock(fd);
+ if (err) {
+ perror("FD_SET_NONBLOCK");
+ goto config_free;
+ }
+ dump_console = true;
+ }

- close(fd);
- free(config);
+ if (dump_console) {
+ do {
+ r = read(fd, console_buffer, sizeof(console_buffer));
+ if (r < 0)
+ break;
+ r = write(STDOUT_FILENO, console_buffer, r);
+ } while(r);
+ if (r < 0)
+ err = r;
+ }

+config_free:
+ free(config);
+fd_close:
+ close(fd);
return err;
}

--
2.11.0

Jan Kiszka

unread,
Jan 16, 2017, 11:40:30 AM1/16/17
to Ralf Ramsauer, jailho...@googlegroups.com
If you do not shift these flags around, there is also no need for the
revision bump. Actually, it would make at least equal sense to keep all
CON1 related bits together, maybe moving the CON2 related bits up to 16..31.

>
> #define CON1_IS_MMIO(flags) ((flags) & JAILHOUSE_CON1_FLAG_MMIO)
>
> diff --git a/hypervisor/include/jailhouse/header.h b/hypervisor/include/jailhouse/header.h
> index 4fe159c6..072a37b2 100644
> --- a/hypervisor/include/jailhouse/header.h
> +++ b/hypervisor/include/jailhouse/header.h
> @@ -24,6 +24,14 @@
> */
> typedef int (*jailhouse_entry)(unsigned int);
>
> +#define CONSOLE_SIZE 0x1000

This is no longer the actual size, is it?

> +
> +struct jailhouse_console {
> + volatile unsigned int lock;
> + unsigned int tail;
> + char content[2048];
> +} __attribute__((aligned(CONSOLE_SIZE)));

But do we still need explicit alignment at all, with the hypervisor
placing the struct in an aligned section?
Jan

--
Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Corporate Competence Center Embedded Linux

Jan Kiszka

unread,
Jan 16, 2017, 11:49:00 AM1/16/17
to Ralf Ramsauer, jailho...@googlegroups.com
Oh, and tail is volatile as well, isn't it?

Jan

Jan Kiszka

unread,
Jan 16, 2017, 12:52:18 PM1/16/17
to Ralf Ramsauer, jailho...@googlegroups.com
On 2017-01-16 17:07, Ralf Ramsauer wrote:
> We're backing hypervisor pages with an empty page for the root cell.
> This commit excludes the console page and allows RO access to it, but
> only if the debug console of system configuration has the flag
> JAILHOUSE_CON2_TYPE_ROOTPAGE set.
>
> This flag is not activated by default and has to be set manually.
>
> In this way, the Linux driver may easily read out the console without
> any interactions with the hypervisor.
>
> Signed-off-by: Ralf Ramsauer <ralf.r...@oth-regensburg.de>
> ---
> hypervisor/include/jailhouse/printk.h | 1 +
> hypervisor/printk.c | 1 +
> hypervisor/setup.c | 12 ++++++++++++
> 3 files changed, 14 insertions(+)
>
> diff --git a/hypervisor/include/jailhouse/printk.h b/hypervisor/include/jailhouse/printk.h
> index 68596e84..4c8e5224 100644
> --- a/hypervisor/include/jailhouse/printk.h
> +++ b/hypervisor/include/jailhouse/printk.h
> @@ -28,4 +28,5 @@ void __attribute__((format(printf, 1, 2))) panic_printk(const char *fmt, ...);
> void arch_dbg_write_init(void);
> extern void (*arch_dbg_write)(const char *msg);
>
> +extern bool console_print;

Should probably be called something like "virtual_console", to
differentiate from the UARTs.

Ralf Ramsauer

unread,
Jan 16, 2017, 1:04:43 PM1/16/17
to Jan Kiszka, jailho...@googlegroups.com
This is the code where I copy the console page:

+static void copy_console_page(struct jailhouse_console *dst)
+{
+retry:
+ /* spin while hypervisor is writing to console */
+ while (console_page->lock);
+ /* copy console page */
+ memcpy(dst, console_page, sizeof(struct jailhouse_console));
+ /* check consistency of console_page */
+ if (dst->tail != console_page->tail)
+ goto retry;
+}

I'm spinning on lock, copy the whole page and comparing tails. So is it
really necessary that the tail gets volatile? But sure, it won't hurt,
if it is volatile.

Ralf Ramsauer

unread,
Jan 16, 2017, 1:10:03 PM1/16/17
to Jan Kiszka, jailho...@googlegroups.com
Yep, you're right. Same thing just came into my mind when looking at the
code again.
Ralf

Jan Kiszka

unread,
Jan 16, 2017, 1:26:33 PM1/16/17
to Ralf Ramsauer, jailho...@googlegroups.com
The avoid is not using any compiler barrier (typically via cpu_relax,
which is a good idea anyway), and then the volatile is definitely
missing because the compiler could theoretically get the idea to read
console_page-> tail only once. With barriers, the volatile is not
strictly required, right.

Jan Kiszka

unread,
Jan 16, 2017, 1:52:11 PM1/16/17
to Ralf Ramsauer, jailho...@googlegroups.com
On 2017-01-16 17:07, Ralf Ramsauer wrote:
Barrier has to come before the lock update.

You should leave comments why we need the barriers: The first one, to
ensure the lock is visible prior to starting any content or tail update.
The second, to make sure that those updates are committed before unlocking.

> +}
> +
> #include "printk-core.c"
>
> static void dbg_write_stub(const char *msg)
>

Jan Kiszka

unread,
Jan 16, 2017, 1:59:34 PM1/16/17
to Ralf Ramsauer, jailho...@googlegroups.com
On 2017-01-16 17:07, Ralf Ramsauer wrote:
As commented elsewhere: spinning should issue cpu_relax.

> + /* copy console page */
> + memcpy(dst, console_page, sizeof(struct jailhouse_console));
> + /* check consistency of console_page */
> + if (dst->tail != console_page->tail)
> + goto retry;

do { } while would work here.

But the logic isn't correct yet: you need to read lock and tail
atomically, only proceed if the lock is not set. Memory read barrier.
Then copy the page. Memory read barrier. Now fetch lock and tail from
memory again and check that they still match.

Jan Kiszka

unread,
Jan 16, 2017, 2:27:10 PM1/16/17
to Ralf Ramsauer, jailho...@googlegroups.com
On 2017-01-16 17:07, Ralf Ramsauer wrote:
> Similar to other tools, like 'tail -f', allow 'jailhosue enable' to
> follow the console output when jailhouse gets enabled.
>
> On failure, we will print the console as well (if available).
>
> 'jailhouse enable CELL -v' will print the hypervisor's console content
> upon enabling jailhouse and terminate.

I wonder if we should have some "jailhouse console" as well or instead
of enable -f/-v. Would allow to attach and print the content even after
a non-verbose enable.

OK, let's keep it simple: separate "jailhouse console [-f | --follow]",
callable at any time (provided the driver exists).

>
> Signed-off-by: Ralf Ramsauer <ralf.r...@oth-regensburg.de>
> ---
> tools/jailhouse.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++++------
> 1 file changed, 57 insertions(+), 7 deletions(-)
>
> diff --git a/tools/jailhouse.c b/tools/jailhouse.c
> index ab495fda..5760b087 100644
> --- a/tools/jailhouse.c
> +++ b/tools/jailhouse.c
> @@ -62,7 +62,8 @@ static void __attribute__((noreturn)) help(char *prog, int exit_status)
>
> printf("Usage: %s { COMMAND | --help || --version }\n"

(hmm, looks like the is one "|" too much after --help)

> "\nAvailable commands:\n"
> - " enable SYSCONFIG\n"
> + " enable SYSCONFIG"
> + " { [ -f | --follow ] | [ -v | --verbose] }\n"

No unneeded line-wrap, please.
while (r)

> + if (r < 0)
> + err = r;
> + }
>
> +config_free:
> + free(config);
> +fd_close:
> + close(fd);
> return err;
> }
>
>

Ralf Ramsauer

unread,
Jan 16, 2017, 6:19:34 PM1/16/17
to Jan Kiszka, jailho...@googlegroups.com
On 01/16/2017 08:27 PM, Jan Kiszka wrote:
> On 2017-01-16 17:07, Ralf Ramsauer wrote:
>> Similar to other tools, like 'tail -f', allow 'jailhosue enable' to
>> follow the console output when jailhouse gets enabled.
>>
>> On failure, we will print the console as well (if available).
>>
>> 'jailhouse enable CELL -v' will print the hypervisor's console content
>> upon enabling jailhouse and terminate.
>
> I wonder if we should have some "jailhouse console" as well or instead
> of enable -f/-v. Would allow to attach and print the content even after
> a non-verbose enable.
>
> OK, let's keep it simple: separate "jailhouse console [-f | --follow]",
> callable at any time (provided the driver exists).
Ok, but still dump the console content if enabling the hypervisor fails?
>
>>
>> Signed-off-by: Ralf Ramsauer <ralf.r...@oth-regensburg.de>
>> ---
>> tools/jailhouse.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++++------
>> 1 file changed, 57 insertions(+), 7 deletions(-)
>>
>> diff --git a/tools/jailhouse.c b/tools/jailhouse.c
>> index ab495fda..5760b087 100644
>> --- a/tools/jailhouse.c
>> +++ b/tools/jailhouse.c
>> @@ -62,7 +62,8 @@ static void __attribute__((noreturn)) help(char *prog, int exit_status)
>>
>> printf("Usage: %s { COMMAND | --help || --version }\n"
>
> (hmm, looks like the is one "|" too much after --help)
let me pick this up and squash it to this commit.
>
>> "\nAvailable commands:\n"
>> - " enable SYSCONFIG\n"
>> + " enable SYSCONFIG"
>> + " { [ -f | --follow ] | [ -v | --verbose] }\n"
>
> No unneeded line-wrap, please.
Oh, it's exactly 80 characters! Must have happened during writing. The
other day, it was longer :)
You mean replacing the do {} while() with a while() {}?

Then I would have to set r first.

Thanks for comments
Ralf

Jan Kiszka

unread,
Jan 17, 2017, 3:32:47 AM1/17/17
to Ralf Ramsauer, jailho...@googlegroups.com
On 2017-01-16 17:07, Ralf Ramsauer wrote:
Unneeded - this instance will die if you return an error, and otherwise
you overwrite it.

> + user = kzalloc(sizeof(struct console_userdata), GFP_KERNEL);
> + if (!user)
> + return -ENOMEM;
> +
> + file->private_data = user;
> + return 0;
> +}
> +
> +static int jailhouse_console_release(struct inode *inode, struct file *file)
> +{
> + struct console_userdata *user = file->private_data;
> + if (user)

Unneeded (but add a blank line) - kfree(NULL) is allowed.
Wrong ordering: first set yourself uninterruptible, then schedule. Or
just used schedule_timeout_interruptible, that does this for you.

> + if (signal_pending(current)) {
> + ret = -EINTR;
> + goto console_free_out;
> + }
> + }
> +
> + if (miss) {
> + /* If we missed anything, warn user. We will dump the actual
> + * content in the next call. */
> + ret = snprintf(content, sizeof(console_page->content),
> + "NOTE: missing %u byte console log\n",

Should better be in some <> or [] because the note may be added to a
previous output without a line-break.

Jan Kiszka

unread,
Jan 17, 2017, 3:37:20 AM1/17/17
to Ralf Ramsauer, jailho...@googlegroups.com
On 2017-01-17 00:18, Ralf Ramsauer wrote:
> On 01/16/2017 08:27 PM, Jan Kiszka wrote:
>> On 2017-01-16 17:07, Ralf Ramsauer wrote:
>>> Similar to other tools, like 'tail -f', allow 'jailhosue enable' to
>>> follow the console output when jailhouse gets enabled.
>>>
>>> On failure, we will print the console as well (if available).
>>>
>>> 'jailhouse enable CELL -v' will print the hypervisor's console content
>>> upon enabling jailhouse and terminate.
>>
>> I wonder if we should have some "jailhouse console" as well or instead
>> of enable -f/-v. Would allow to attach and print the content even after
>> a non-verbose enable.
>>
>> OK, let's keep it simple: separate "jailhouse console [-f | --follow]",
>> callable at any time (provided the driver exists).
> Ok, but still dump the console content if enabling the hypervisor fails?

That dump won't interfer with some "jailhouse console" running in
parallel, right? Then it might be useful. But keep this as separate patch.

I was thinking about allowing "-v" for all commands (jailhouse [-v |
--verbose] <command>) but interpreting this as a dump only in those that
change the hypervisor configuration. Would be an alternative to
unconditional dumping on error. Undecided yet.
No, I mean having a blank between while and (condition) - while is not a
function.

Ralf Ramsauer

unread,
Jan 17, 2017, 7:21:55 AM1/17/17
to Jan Kiszka, jailho...@googlegroups.com
Hi,

On 01/17/2017 09:37 AM, Jan Kiszka wrote:
> On 2017-01-17 00:18, Ralf Ramsauer wrote:
>> On 01/16/2017 08:27 PM, Jan Kiszka wrote:
>>> On 2017-01-16 17:07, Ralf Ramsauer wrote:
>>>> Similar to other tools, like 'tail -f', allow 'jailhosue enable' to
>>>> follow the console output when jailhouse gets enabled.
>>>>
>>>> On failure, we will print the console as well (if available).
>>>>
>>>> 'jailhouse enable CELL -v' will print the hypervisor's console content
>>>> upon enabling jailhouse and terminate.
>>>
>>> I wonder if we should have some "jailhouse console" as well or instead
>>> of enable -f/-v. Would allow to attach and print the content even after
>>> a non-verbose enable.
>>>
>>> OK, let's keep it simple: separate "jailhouse console [-f | --follow]",
>>> callable at any time (provided the driver exists).
>> Ok, but still dump the console content if enabling the hypervisor fails?
>
> That dump won't interfer with some "jailhouse console" running in
> parallel, right? Then it might be useful. But keep this as separate patch.
No, it won't. Parallel reads are fine, besides the fact that they are
unperformant, as each reader holds and copies its own state of the console.

Actually it would be better to spawn a kernel thread when the first
reader arrives that periodically tries to consume deltas from the
console page and writes them to a more generic structure (internal
kernel ring buffer?) that can be consumed by any reader. We can exit the
thread when there are no more readers. I'll drop another comment in
Patch 10/12 to explain why this would really make sense, though it's
more complex.

Ralf

Ralf Ramsauer

unread,
Jan 17, 2017, 7:25:38 AM1/17/17
to Jan Kiszka, jailho...@googlegroups.com
Just found a little regression here:

If there is more than one reader, only one of them will dump the
last_console_page. FCFS...

This would not be the case if we would have a threaded periodical common
consumer of the (last_)console_page.

Ralf

Jan Kiszka

unread,
Jan 17, 2017, 10:10:01 AM1/17/17
to Ralf Ramsauer, jailho...@googlegroups.com
Actually, I like the ability to read without interference and loss via
separate fds.

Can't we mirror the last console page into a shared buffer that is
invalidated on next hypervisor enabling?

Ralf Ramsauer

unread,
Jan 17, 2017, 10:12:46 AM1/17/17
to Jan Kiszka, jailho...@googlegroups.com
Ok.
>
> Can't we mirror the last console page into a shared buffer that is
> invalidated on next hypervisor enabling?
That's possible. Then I only have to store a per-fd flag that indicates
if the last console page has been read. That's it.

Ralf
>
> Jan
>

Ralf Ramsauer

unread,
Jan 19, 2017, 3:12:30 PM1/19/17
to Jan Kiszka, jailho...@googlegroups.com, Ralf Ramsauer
The console page is implemented as ring buffer, and will _not_ contain a
trailing \0 for string termination.

Signed-off-by: Ralf Ramsauer <ralf.r...@oth-regensburg.de>
---
hypervisor/hypervisor.lds.S | 6 ++++++
hypervisor/include/jailhouse/cell-config.h | 6 ++++++
hypervisor/include/jailhouse/header.h | 8 ++++++++
hypervisor/include/jailhouse/printk.h | 2 ++
hypervisor/printk.c | 2 ++
5 files changed, 24 insertions(+)

diff --git a/hypervisor/hypervisor.lds.S b/hypervisor/hypervisor.lds.S
index 09b1a0b2e3..f81782820d 100644
--- a/hypervisor/hypervisor.lds.S
+++ b/hypervisor/hypervisor.lds.S
@@ -35,6 +35,12 @@ SECTIONS
. = ALIGN(16);
.bss : { *(.bss) }

+ /* The console section shall only contain the hypervisor console. This
+ * section and the next section must be aligned to PAGE_SIZE, as we
+ * want the console section to reserve a whole page */
+ . = ALIGN(PAGE_SIZE);
+ .console : { *(.console) }
+
. = ALIGN(PAGE_SIZE);
__page_pool = .;

diff --git a/hypervisor/include/jailhouse/cell-config.h b/hypervisor/include/jailhouse/cell-config.h
index 80fa5a78b4..67b3319eca 100644
--- a/hypervisor/include/jailhouse/cell-config.h
+++ b/hypervisor/include/jailhouse/cell-config.h
@@ -192,6 +192,12 @@ struct jailhouse_iommu {

#define CON1_IS_MMIO(flags) ((flags) & JAILHOUSE_CON1_FLAG_MMIO)

+/* Bits 16..19 are used to select the second console driver */
+#define JAILHOUSE_CON2_TYPE_ROOTPAGE 0x0100
+#define JAILHOUSE_CON2_TYPE_MASK 0x0f00
+
+#define CON2_TYPE(flags) ((flags) & JAILHOUSE_CON2_TYPE_MASK)
+
struct jailhouse_debug_console {
__u64 address;
__u32 size;
diff --git a/hypervisor/include/jailhouse/header.h b/hypervisor/include/jailhouse/header.h
index 4fe159c621..ccb9684243 100644
--- a/hypervisor/include/jailhouse/header.h
+++ b/hypervisor/include/jailhouse/header.h
@@ -24,6 +24,14 @@
*/
typedef int (*jailhouse_entry)(unsigned int);

+struct jailhouse_console {
+ unsigned int lock;
+ unsigned int tail;
+ /* current implementation requires the size of the content to be a
+ * power of two */
+ char content[2048];
+};
+
/**
* Hypervisor description.
* Located at the beginning of the hypervisor binary image and loaded by
diff --git a/hypervisor/include/jailhouse/printk.h b/hypervisor/include/jailhouse/printk.h
index a506c0fddf..f2bed6e292 100644
--- a/hypervisor/include/jailhouse/printk.h
+++ b/hypervisor/include/jailhouse/printk.h
@@ -27,3 +27,5 @@ void __attribute__((format(printf, 1, 2))) panic_printk(const char *fmt, ...);

void arch_dbg_write_init(void);
extern void (*arch_dbg_write)(const char *msg);
+
+extern volatile struct jailhouse_console console;
diff --git a/hypervisor/printk.c b/hypervisor/printk.c
index e8f5ffe289..ca54b60dce 100644
--- a/hypervisor/printk.c
+++ b/hypervisor/printk.c
@@ -18,6 +18,8 @@
#include <asm/bitops.h>
#include <asm/spinlock.h>

+volatile struct jailhouse_console console __attribute__((section(".console")));
+
static DEFINE_SPINLOCK(printk_lock);

#define console_write(msg) arch_dbg_write(msg)
--
2.11.0

Ralf Ramsauer

unread,
Jan 19, 2017, 3:12:30 PM1/19/17
to Jan Kiszka, jailho...@googlegroups.com, Ralf Ramsauer
Hi,

I implemented a sysfs interface for the hypervisor console. This is useful for
debugging devices without UARTs. At least as long as the root-cell doesn't
crash...

Additionally, I implemented continuous reading of the hypervisor console via
/dev/jailhouse, which is a pretty nice and easy way to follow the hypervisor
console from Linux.

I use a page in the hypervisor's memory space as ringbuffer. If the root cell
has a certain permission enabled in its config, the hypervisor won't back that
page with empty_page. The Linux driver may then read from that page without any
interactions with the hypervisor. We do this without any signalling or locking.
Otherwise we would risk starvation of the hypervisor. The producer (hypervisor
debug write) only indicates ongoing writes by setting a flag.

tree:
https://github.com/lfd/jailhouse/tree/hvconsoleV4

PS: maybe we could think of enabling JAILHOUSE_CELL_DEBUG_CONSOLE as a default
flag for our demo inmate cell configuration. As Jan already pointed out
earlier, none of them is permissive.

Ralf

since v3:
- fix memory barriers
- improve handling of last_page
- don't increment config file revision: we using upper bits for CON2_TYPE
- minor improvements

since v2:
- support continuous reading on /dev/jailhouse as well as non-blocking reads
- support overflows of the tail pointer
- adopt userspace tool to dump the console (either by switching a command
line flag or automatically on errors)
- ring buffer size must be a power of two (otherwise modulo fails on tail
pointer overflows)
- successfully tested on Qemu and Tegra [XK]1

since v1:
- refactor JAILHOUSE_CON_TYPE to JAILHOUSE_CON1_TYPE
- use JAILHOUSE_CON2_TYPE_ROOTPAGE as permission flag for sysfs console
- no extra section in linker file required


Ralf Ramsauer (12):
configs, core: Refactor JAILHOUSE_CON_TYPE to JAILHOUSE_CON1_TYPE
core: declare and define structures used for virtual console
core: don't hide console page
driver, core: propagate offset of console page
core: print console messages to the console page
driver: add console sysfs attribute
Documentation: add documentation for sysfs console
configs: activate sysfs console
driver: implement continous console reading on /dev/jailhouse
driver: copy the console_page after the hypervisor is disabled
tools: implement 'jailhouse console [-f]'
tools: dump error console on failures

Documentation/debug-output.md | 37 +++--
Documentation/sysfs-entries.txt | 3 +
Documentation/vga-console.txt | 2 +-
TODO.md | 1 -
configs/amd-seattle.c | 5 +-
configs/bananapi.c | 5 +-
configs/f2a88xm-hd3.c | 5 +-
configs/foundation-v8.c | 5 +-
configs/hikey.c | 4 +-
configs/imb-a180.c | 5 +-
configs/jetson-tk1.c | 5 +-
configs/jetson-tx1.c | 5 +-
configs/orangepi0.c | 5 +-
configs/qemu-vm.c | 5 +-
configs/vexpress.c | 5 +-
configs/zynqmp-zcu102.c | 5 +-
driver/main.c | 241 ++++++++++++++++++++++++++++-
driver/main.h | 2 +
driver/sysfs.c | 26 ++++
hypervisor/arch/arm-common/dbg-write.c | 10 +-
hypervisor/arch/x86/dbg-write.c | 6 +-
hypervisor/arch/x86/uart.c | 2 +-
hypervisor/hypervisor.lds.S | 6 +
hypervisor/include/jailhouse/cell-config.h | 28 ++--
hypervisor/include/jailhouse/header.h | 11 ++
hypervisor/include/jailhouse/printk.h | 3 +
hypervisor/paging.c | 2 +-
hypervisor/printk.c | 24 ++-
hypervisor/setup.c | 13 ++
tools/jailhouse.c | 51 +++++-
tools/root-cell-config.c.tmpl | 5 +-
31 files changed, 466 insertions(+), 66 deletions(-)

--
2.11.0

Ralf Ramsauer

unread,
Jan 19, 2017, 3:12:31 PM1/19/17
to Jan Kiszka, jailho...@googlegroups.com, Ralf Ramsauer
We're backing hypervisor pages with an empty page for the root cell.
This commit excludes the console page and allows RO access to it, but
only if the debug console of system configuration has the flag
JAILHOUSE_CON2_TYPE_ROOTPAGE set.

This flag is not activated by default and has to be set manually.

In this way, the Linux driver may easily read out the console without
any interactions with the hypervisor.

Signed-off-by: Ralf Ramsauer <ralf.r...@oth-regensburg.de>
---
hypervisor/include/jailhouse/printk.h | 1 +
hypervisor/printk.c | 1 +
hypervisor/setup.c | 12 ++++++++++++
3 files changed, 14 insertions(+)

diff --git a/hypervisor/include/jailhouse/printk.h b/hypervisor/include/jailhouse/printk.h
index f2bed6e292..e775e97564 100644
--- a/hypervisor/include/jailhouse/printk.h
+++ b/hypervisor/include/jailhouse/printk.h
@@ -28,4 +28,5 @@ void __attribute__((format(printf, 1, 2))) panic_printk(const char *fmt, ...);
void arch_dbg_write_init(void);
extern void (*arch_dbg_write)(const char *msg);

+extern bool virtual_console;
extern volatile struct jailhouse_console console;
diff --git a/hypervisor/printk.c b/hypervisor/printk.c
index ca54b60dce..117fe74c96 100644
--- a/hypervisor/printk.c
+++ b/hypervisor/printk.c
@@ -18,6 +18,7 @@
#include <asm/bitops.h>
#include <asm/spinlock.h>

+bool virtual_console = false;
volatile struct jailhouse_console console __attribute__((section(".console")));

static DEFINE_SPINLOCK(printk_lock);
diff --git a/hypervisor/setup.c b/hypervisor/setup.c
index 8cfa455510..9b1c0c1df7 100644
--- a/hypervisor/setup.c
+++ b/hypervisor/setup.c
@@ -41,6 +41,10 @@ static void init_early(unsigned int cpu_id)
system_config = (struct jailhouse_system *)
(JAILHOUSE_BASE + core_and_percpu_size);

+ if (CON2_TYPE(system_config->debug_console.flags) ==
+ JAILHOUSE_CON2_TYPE_ROOTPAGE)
+ virtual_console = true;
+
arch_dbg_write_init();

printk("\nInitializing Jailhouse hypervisor %s on CPU %d\n",
@@ -65,6 +69,9 @@ static void init_early(unsigned int cpu_id)
* Back the region of the hypervisor core and per-CPU page with empty
* pages for Linux. This allows to fault-in the hypervisor region into
* Linux' page table before shutdown without triggering violations.
+ *
+ * Allow read access to the console page, if the hypervisor has the
+ * debug console flag JAILHOUSE_CON2_TYPE_ROOTPAGE set.
*/
hyp_phys_start = system_config->hypervisor_memory.phys_start;
hyp_phys_end = hyp_phys_start + system_config->hypervisor_memory.size;
@@ -74,6 +81,11 @@ static void init_early(unsigned int cpu_id)
hv_page.size = PAGE_SIZE;
hv_page.flags = JAILHOUSE_MEM_READ;
while (hv_page.virt_start < hyp_phys_end) {
+ if (virtual_console &&
+ hv_page.virt_start == paging_hvirt2phys(&console))
+ hv_page.phys_start = paging_hvirt2phys(&console);
+ else
+ hv_page.phys_start = paging_hvirt2phys(empty_page);
error = arch_map_memory_region(&root_cell, &hv_page);
if (error)
return;
--
2.11.0

Ralf Ramsauer

unread,
Jan 19, 2017, 3:12:31 PM1/19/17
to Jan Kiszka, jailho...@googlegroups.com, Ralf Ramsauer
We're about to implement a second HV console channel: a 'virtual'
console available for the root cell via sysfs. In order to encode its
configuration in struct jailhouse_debug_console, rename
JAILHOUSE_CON_TYPE to JAILHOUSE_CON1_TYPE.

Most of the work was done automatically with sed.

Signed-off-by: Ralf Ramsauer <ralf.r...@oth-regensburg.de>
---
Documentation/debug-output.md | 24 ++++++++++++------------
Documentation/vga-console.txt | 2 +-
configs/amd-seattle.c | 4 ++--
configs/bananapi.c | 4 ++--
configs/f2a88xm-hd3.c | 4 ++--
configs/foundation-v8.c | 4 ++--
configs/hikey.c | 4 ++--
configs/imb-a180.c | 4 ++--
configs/jetson-tk1.c | 4 ++--
configs/jetson-tx1.c | 4 ++--
configs/orangepi0.c | 4 ++--
configs/qemu-vm.c | 4 ++--
configs/vexpress.c | 4 ++--
configs/zynqmp-zcu102.c | 4 ++--
driver/main.c | 2 +-
hypervisor/arch/arm-common/dbg-write.c | 10 +++++-----
hypervisor/arch/x86/dbg-write.c | 6 +++---
hypervisor/arch/x86/uart.c | 2 +-
hypervisor/include/jailhouse/cell-config.h | 22 +++++++++++-----------
hypervisor/paging.c | 2 +-
tools/root-cell-config.c.tmpl | 4 ++--
21 files changed, 61 insertions(+), 61 deletions(-)

diff --git a/Documentation/debug-output.md b/Documentation/debug-output.md
index 1e5888fc23..6367fbcd93 100644
--- a/Documentation/debug-output.md
+++ b/Documentation/debug-output.md
index 7b37826488..81dcc403b0 100644
--- a/Documentation/vga-console.txt
+++ b/Documentation/vga-console.txt
@@ -20,7 +20,7 @@ Add the following to the header section of your root cell's config:
.debug_console = {
.address = 0xb8000,
.size = 0x1000,
- .flags = JAILHOUSE_CON_TYPE_VGA | JAILHOUSE_CON_FLAG_MMIO,
+ .flags = JAILHOUSE_CON1_TYPE_VGA | JAILHOUSE_CON1_FLAG_MMIO,
},

Boot using the following kernel parameters:
diff --git a/configs/amd-seattle.c b/configs/amd-seattle.c
index 7d32eacc01..66e3fdcd82 100644
--- a/configs/amd-seattle.c
+++ b/configs/amd-seattle.c
@@ -32,8 +32,8 @@ struct {
.debug_console = {
.address = 0xe1010000,
.size = 0x1000,
- .flags = JAILHOUSE_CON_TYPE_PL011 |
- JAILHOUSE_CON_FLAG_MMIO,
+ .flags = JAILHOUSE_CON1_TYPE_PL011 |
+ JAILHOUSE_CON1_FLAG_MMIO,
},
.platform_info = {
.pci_mmconfig_base = 0xf0000000,
diff --git a/configs/bananapi.c b/configs/bananapi.c
index ef7f824060..484d808ac3 100644
--- a/configs/bananapi.c
+++ b/configs/bananapi.c
@@ -37,8 +37,8 @@ struct {
/* .clock_reg = 0x01c2006c, */
/* .gate_nr = 16 */
/* .divider = 0x0d, */
- .flags = JAILHOUSE_CON_TYPE_8250 |
- JAILHOUSE_CON_FLAG_MMIO,
+ .flags = JAILHOUSE_CON1_TYPE_8250 |
+ JAILHOUSE_CON1_FLAG_MMIO,
},
.platform_info = {
.pci_mmconfig_base = 0x2000000,
diff --git a/configs/f2a88xm-hd3.c b/configs/f2a88xm-hd3.c
index 7c57407bbf..62a9117b99 100644
--- a/configs/f2a88xm-hd3.c
+++ b/configs/f2a88xm-hd3.c
@@ -41,8 +41,8 @@ struct {
.debug_console = {
.address = 0x3f8,
/* .divider = 0x1, */
- .flags = JAILHOUSE_CON_TYPE_UART_X86 |
- JAILHOUSE_CON_FLAG_PIO,
+ .flags = JAILHOUSE_CON1_TYPE_UART_X86 |
+ JAILHOUSE_CON1_FLAG_PIO,
},
.platform_info = {
.pci_mmconfig_base = 0xe0000000,
diff --git a/configs/foundation-v8.c b/configs/foundation-v8.c
index 9ae975567d..c4112004d9 100644
--- a/configs/foundation-v8.c
+++ b/configs/foundation-v8.c
@@ -31,8 +31,8 @@ struct {
.debug_console = {
.address = 0x1c090000,
.size = 0x1000,
- .flags = JAILHOUSE_CON_TYPE_PL011 |
- JAILHOUSE_CON_FLAG_MMIO,
+ .flags = JAILHOUSE_CON1_TYPE_PL011 |
+ JAILHOUSE_CON1_FLAG_MMIO,
},
.platform_info.arm = {
#ifdef CONFIG_ARM_GIC_V3
diff --git a/configs/hikey.c b/configs/hikey.c
index c45fc96e74..a739222b93 100644
--- a/configs/hikey.c
+++ b/configs/hikey.c
@@ -32,8 +32,8 @@ struct {
.debug_console = {
.address = 0xf7113000,
.size = 0x1000,
- .flags = JAILHOUSE_CON_TYPE_PL011 |
- JAILHOUSE_CON_FLAG_MMIO,
+ .flags = JAILHOUSE_CON1_TYPE_PL011 |
+ JAILHOUSE_CON1_FLAG_MMIO,
},
.platform_info = {
.pci_mmconfig_base = 0xf6000000,
diff --git a/configs/imb-a180.c b/configs/imb-a180.c
index d5987bbe71..fa36d791d3 100644
--- a/configs/imb-a180.c
+++ b/configs/imb-a180.c
@@ -40,8 +40,8 @@ struct {
.debug_console = {
.address = 0x3f8,
/* .divider = 0x1, */
- .flags = JAILHOUSE_CON_TYPE_UART_X86 |
- JAILHOUSE_CON_FLAG_PIO,
+ .flags = JAILHOUSE_CON1_TYPE_UART_X86 |
+ JAILHOUSE_CON1_FLAG_PIO,
},
.platform_info = {
.pci_mmconfig_base = 0xe0000000,
diff --git a/configs/jetson-tk1.c b/configs/jetson-tk1.c
index b8176dee79..d6713405bf 100644
--- a/configs/jetson-tk1.c
+++ b/configs/jetson-tk1.c
@@ -40,8 +40,8 @@ struct {
/* .clock_reg = 0x60006000 + 0x330, */
/* .gate_nr = (65 % 32), */
/* .divider = 0xdd, */
- .flags = JAILHOUSE_CON_TYPE_8250 |
- JAILHOUSE_CON_FLAG_MMIO,
+ .flags = JAILHOUSE_CON1_TYPE_8250 |
+ JAILHOUSE_CON1_FLAG_MMIO,
},
.platform_info = {
.pci_mmconfig_base = 0x48000000,
diff --git a/configs/jetson-tx1.c b/configs/jetson-tx1.c
index cf80aa4786..af1f55775b 100644
--- a/configs/jetson-tx1.c
+++ b/configs/jetson-tx1.c
@@ -34,8 +34,8 @@ struct {
.debug_console = {
.address = 0x70006000,
.size = 0x0040,
- .flags = JAILHOUSE_CON_TYPE_8250 |
- JAILHOUSE_CON_FLAG_MMIO,
+ .flags = JAILHOUSE_CON1_TYPE_8250 |
+ JAILHOUSE_CON1_FLAG_MMIO,
},
.platform_info.arm = {
.gicd_base = 0x50041000,
diff --git a/configs/orangepi0.c b/configs/orangepi0.c
index a37e5d49d8..14f1a0e0dd 100644
--- a/configs/orangepi0.c
+++ b/configs/orangepi0.c
@@ -34,8 +34,8 @@ struct {
.debug_console = {
.address = 0x01c28000,
.size = 0x1000,
- .flags = JAILHOUSE_CON_TYPE_8250 |
- JAILHOUSE_CON_FLAG_MMIO,
+ .flags = JAILHOUSE_CON1_TYPE_8250 |
+ JAILHOUSE_CON1_FLAG_MMIO,
},
.platform_info = {
.pci_mmconfig_base = 0x2000000,
diff --git a/configs/qemu-vm.c b/configs/qemu-vm.c
index 04d0c947f5..0ce34eeab6 100644
--- a/configs/qemu-vm.c
+++ b/configs/qemu-vm.c
@@ -40,8 +40,8 @@ struct {
.debug_console = {
.address = 0x3f8,
/* .divider = 0x1, */
- .flags = JAILHOUSE_CON_TYPE_UART_X86 |
- JAILHOUSE_CON_FLAG_PIO,
+ .flags = JAILHOUSE_CON1_TYPE_UART_X86 |
+ JAILHOUSE_CON1_FLAG_PIO,
},
.platform_info = {
.pci_mmconfig_base = 0xb0000000,
diff --git a/configs/vexpress.c b/configs/vexpress.c
index 15202970f8..61c99b9b73 100644
--- a/configs/vexpress.c
+++ b/configs/vexpress.c
@@ -31,8 +31,8 @@ struct {
.debug_console = {
.address = 0x1c090000,
.size = 0x1000,
- .flags = JAILHOUSE_CON_TYPE_PL011 |
- JAILHOUSE_CON_FLAG_MMIO,
+ .flags = JAILHOUSE_CON1_TYPE_PL011 |
+ JAILHOUSE_CON1_FLAG_MMIO,
},
.platform_info.arm = {
#ifdef CONFIG_ARM_GIC_V3
diff --git a/configs/zynqmp-zcu102.c b/configs/zynqmp-zcu102.c
index d236ae6f49..120a73dc6d 100644
--- a/configs/zynqmp-zcu102.c
+++ b/configs/zynqmp-zcu102.c
@@ -34,8 +34,8 @@ struct {
.debug_console = {
.address = 0xff000000,
.size = 0x1000,
- .flags = JAILHOUSE_CON_TYPE_XUARTPS |
- JAILHOUSE_CON_FLAG_MMIO,
+ .flags = JAILHOUSE_CON1_TYPE_XUARTPS |
+ JAILHOUSE_CON1_FLAG_MMIO,
},
.platform_info = {
.pci_mmconfig_base = 0xfc000000,
diff --git a/driver/main.c b/driver/main.c
index e793d9d7a3..676794bef1 100644
--- a/driver/main.c
+++ b/driver/main.c
@@ -301,7 +301,7 @@ static int jailhouse_cmd_enable(struct jailhouse_system __user *arg)
}

#ifdef JAILHOUSE_BORROW_ROOT_PT
- if (CON_IS_MMIO(config->debug_console.flags)) {
+ if (CON1_IS_MMIO(config->debug_console.flags)) {
console = ioremap(config->debug_console.address,
config->debug_console.size);
if (!console) {
diff --git a/hypervisor/arch/arm-common/dbg-write.c b/hypervisor/arch/arm-common/dbg-write.c
index e88b18b916..e9e51a3242 100644
--- a/hypervisor/arch/arm-common/dbg-write.c
+++ b/hypervisor/arch/arm-common/dbg-write.c
@@ -42,16 +42,16 @@ static void arm_uart_write(const char *msg)

void arch_dbg_write_init(void)
{
- unsigned char con_type = CON_TYPE(system_config->debug_console.flags);
+ unsigned char con_type = CON1_TYPE(system_config->debug_console.flags);

- if (!CON_IS_MMIO(system_config->debug_console.flags))
+ if (!CON1_IS_MMIO(system_config->debug_console.flags))
return;

- if (con_type == JAILHOUSE_CON_TYPE_PL011)
+ if (con_type == JAILHOUSE_CON1_TYPE_PL011)
uart = &uart_pl011_ops;
- else if (con_type == JAILHOUSE_CON_TYPE_8250)
+ else if (con_type == JAILHOUSE_CON1_TYPE_8250)
uart = &uart_8250_ops;
- else if (con_type == JAILHOUSE_CON_TYPE_XUARTPS)
+ else if (con_type == JAILHOUSE_CON1_TYPE_XUARTPS)
uart = &uart_xuartps_ops;

if (uart) {
diff --git a/hypervisor/arch/x86/dbg-write.c b/hypervisor/arch/x86/dbg-write.c
index 60e8adb43a..30ddc9087a 100644
--- a/hypervisor/arch/x86/dbg-write.c
+++ b/hypervisor/arch/x86/dbg-write.c
@@ -17,13 +17,13 @@

void arch_dbg_write_init(void)
{
- unsigned char dbg_type = CON_TYPE(system_config->debug_console.flags);
+ unsigned char dbg_type = CON1_TYPE(system_config->debug_console.flags);

/* PIO / MMIO differentiation is done inside the driver code */
- if (dbg_type == JAILHOUSE_CON_TYPE_UART_X86) {
+ if (dbg_type == JAILHOUSE_CON1_TYPE_UART_X86) {
uart_init();
arch_dbg_write = uart_write;
- } else if (dbg_type == JAILHOUSE_CON_TYPE_VGA) {
+ } else if (dbg_type == JAILHOUSE_CON1_TYPE_VGA) {
vga_init();
arch_dbg_write = vga_write;
}
diff --git a/hypervisor/arch/x86/uart.c b/hypervisor/arch/x86/uart.c
index 736cf3b2d2..09b9e84f7c 100644
--- a/hypervisor/arch/x86/uart.c
+++ b/hypervisor/arch/x86/uart.c
@@ -57,7 +57,7 @@ void uart_init(void)
u32 flags = system_config->debug_console.flags;
u32 divider = system_config->debug_console.divider;

- if (CON_IS_MMIO(flags)) {
+ if (CON1_IS_MMIO(flags)) {
uart_reg_out = uart_mmio32_out;
uart_reg_in = uart_mmio32_in;
uart_base = (u64)hypervisor_header.debug_console_base;
diff --git a/hypervisor/include/jailhouse/cell-config.h b/hypervisor/include/jailhouse/cell-config.h
index 2f8c7cb901..80fa5a78b4 100644
--- a/hypervisor/include/jailhouse/cell-config.h
+++ b/hypervisor/include/jailhouse/cell-config.h
@@ -176,21 +176,21 @@ struct jailhouse_iommu {
} __attribute__((packed));

/* Bits 0..3 are used to select the particular driver */
-#define JAILHOUSE_CON_TYPE_NONE 0x0000
-#define JAILHOUSE_CON_TYPE_UART_X86 0x0001
-#define JAILHOUSE_CON_TYPE_VGA 0x0002
-#define JAILHOUSE_CON_TYPE_8250 0x0003
-#define JAILHOUSE_CON_TYPE_PL011 0x0004
-#define JAILHOUSE_CON_TYPE_XUARTPS 0x0005
-#define JAILHOUSE_CON_TYPE_MASK 0x000f
+#define JAILHOUSE_CON1_TYPE_NONE 0x0000
+#define JAILHOUSE_CON1_TYPE_UART_X86 0x0001
+#define JAILHOUSE_CON1_TYPE_VGA 0x0002
+#define JAILHOUSE_CON1_TYPE_8250 0x0003
+#define JAILHOUSE_CON1_TYPE_PL011 0x0004
+#define JAILHOUSE_CON1_TYPE_XUARTPS 0x0005
+#define JAILHOUSE_CON1_TYPE_MASK 0x000f

-#define CON_TYPE(flags) ((flags) & JAILHOUSE_CON_TYPE_MASK)
+#define CON1_TYPE(flags) ((flags) & JAILHOUSE_CON1_TYPE_MASK)

/* We use bit 4..5 to differentiate between PIO and MMIO access */
-#define JAILHOUSE_CON_FLAG_PIO 0x0010
-#define JAILHOUSE_CON_FLAG_MMIO 0x0020
+#define JAILHOUSE_CON1_FLAG_PIO 0x0010
+#define JAILHOUSE_CON1_FLAG_MMIO 0x0020

-#define CON_IS_MMIO(flags) ((flags) & JAILHOUSE_CON_FLAG_MMIO)
+#define CON1_IS_MMIO(flags) ((flags) & JAILHOUSE_CON1_FLAG_MMIO)

struct jailhouse_debug_console {
__u64 address;
diff --git a/hypervisor/paging.c b/hypervisor/paging.c
index 67ed63f415..a431550f56 100644
--- a/hypervisor/paging.c
+++ b/hypervisor/paging.c
@@ -595,7 +595,7 @@ int paging_init(void)
if (err)
return err;

- if (CON_IS_MMIO(system_config->debug_console.flags)) {
+ if (CON1_IS_MMIO(system_config->debug_console.flags)) {
vaddr = (unsigned long)hypervisor_header.debug_console_base;
/* check if console overlaps remapping region */
if (vaddr + system_config->debug_console.size >= REMAP_BASE &&
diff --git a/tools/root-cell-config.c.tmpl b/tools/root-cell-config.c.tmpl
index b7be8b7406..85c1834ac3 100644

Ralf Ramsauer

unread,
Jan 19, 2017, 3:12:31 PM1/19/17
to Jan Kiszka, jailho...@googlegroups.com, Ralf Ramsauer
Signed-off-by: Ralf Ramsauer <ralf.r...@oth-regensburg.de>
---
Documentation/sysfs-entries.txt | 3 ++
TODO.md | 1 -
driver/main.c | 98 +++++++++++++++++++++++++++++++++++++++++
driver/main.h | 2 +
driver/sysfs.c | 26 +++++++++++
5 files changed, 129 insertions(+), 1 deletion(-)

diff --git a/Documentation/sysfs-entries.txt b/Documentation/sysfs-entries.txt
index 6b483bba20..58d1f5a2df 100644
--- a/Documentation/sysfs-entries.txt
+++ b/Documentation/sysfs-entries.txt
@@ -5,6 +5,7 @@ The following sysfs entries are provided by the Jailhouse Linux driver. These
can be used for monitoring the state of the hypervisor and its cells.

/sys/devices/jailhouse
+|- console - hypervisor console (see [1])
|- enabled - 1 if Jailhouse is enabled, 0 otherwise
|- mem_pool_size - number of pages in hypervisor memory pool
|- mem_pool_used - used pages of hypervisor memory pool
@@ -30,3 +31,5 @@ may not reflect a fully consistent state. The existence and semantics of VM
exit reason values are architecture-dependent and may change in future
versions. In general statistics shall only be considered as a first hint when
analyzing cell behavior.
+
+[1] Documentation/debug-output.md
diff --git a/TODO.md b/TODO.md
index e452d30630..4e93b1d7af 100644
--- a/TODO.md
+++ b/TODO.md
@@ -89,7 +89,6 @@ Hardware error handling

Monitoring
- report error-triggering devices behind IOMMUs via sysfs
- - hypervisor console via debugfs?
- cell software watchdog via comm region messages
-> time out pending comm region messages and kill failing cells
(includes timeouts of unanswered shutdown requests)
diff --git a/driver/main.c b/driver/main.c
index 676794bef1..2a86842448 100644
--- a/driver/main.c
+++ b/driver/main.c
@@ -19,11 +19,13 @@
#include <linux/miscdevice.h>
#include <linux/firmware.h>
#include <linux/mm.h>
+#include <linux/slab.h>
#include <linux/smp.h>
#include <linux/uaccess.h>
#include <linux/reboot.h>
#include <linux/vmalloc.h>
#include <linux/io.h>
+#include <asm/barrier.h>
#include <asm/smp.h>
#include <asm/cacheflush.h>
#include <asm/tlbflush.h>
@@ -77,6 +79,8 @@ static void *hypervisor_mem;
static unsigned long hv_core_and_percpu_size;
static atomic_t call_done;
static int error_code;
+static struct jailhouse_console* volatile console_page;
+static bool console_available;

#ifdef CONFIG_X86
bool jailhouse_use_vmcall;
@@ -91,6 +95,23 @@ static void init_hypercall(void)
}
#endif

+static void copy_console_page(struct jailhouse_console *dst)
+{
+ unsigned int tail;
+
+ do {
+ /* spin while hypervisor is writing to console */
+ while (console_page->lock)
+ cpu_relax();
+ tail = console_page->tail;
+ rmb();
+
+ /* copy console page */
+ memcpy(dst, console_page, sizeof(struct jailhouse_console));
+ rmb();
+ } while (console_page->tail != tail || console_page->lock);
+}
+
static long get_max_cpus(u32 cpu_set_size,
const struct jailhouse_system __user *system_config)
{
@@ -182,6 +203,77 @@ static inline const char * jailhouse_get_fw_name(void)
#endif
}

+static int jailhouse_console_delta(struct jailhouse_console *console,
+ char *dst, unsigned int head,
+ unsigned int *miss)
+{
+ int ret;
+ unsigned int head_mod, tail_mod;
+ unsigned int delta;
+
+ if (miss)
+ *miss = 0;
+
+ /* check if tail overflowed */
+ if (console->tail < head)
+ delta = (0 - head) + console->tail;
+ else
+ delta = console->tail - head;
+
+ /* check if we have misses */
+ if (delta > sizeof(console->content)) {
+ if (miss)
+ *miss = delta - sizeof(console->content);
+ head = console->tail - sizeof(console->content);
+ delta = sizeof(console->content);
+ }
+
+ head_mod = head % sizeof(console->content);
+ tail_mod = console->tail % sizeof(console->content);
+
+ if (head_mod + delta > sizeof(console->content)) {
+ ret = sizeof(console->content) - head_mod;
+ memcpy(dst, console->content + head_mod, ret);
+ delta -= ret;
+ memcpy(dst + ret, console->content, delta);
+ ret += delta;
+ } else {
+ ret = delta;
+ memcpy(dst, console->content + head_mod, delta);
+ }
+
+ return ret;
+}
+
+int jailhouse_console_page_delta(char *dst, unsigned int head,
+ unsigned int *miss)
+{
+ int ret;
+ struct jailhouse_console *console;
+
+ if (!jailhouse_enabled)
+ return -EAGAIN;
+
+ if (!console_available)
+ return -EPERM;
+
+ console = kmalloc(sizeof(struct jailhouse_console), GFP_KERNEL);
+ if (console == NULL)
+ return -ENOMEM;
+
+ copy_console_page(console);
+ if (console->tail == head) {
+ ret = 0;
+ goto console_free_out;
+ }
+
+ ret = jailhouse_console_delta(console, dst, head, miss);
+
+console_free_out:
+ kfree(console);
+ return ret;
+}
+
/* See Documentation/bootstrap-interface.txt */
static int jailhouse_cmd_enable(struct jailhouse_system __user *arg)
{
@@ -273,6 +365,9 @@ static int jailhouse_cmd_enable(struct jailhouse_system __user *arg)
goto error_release_fw;
}

+ console_page = (struct jailhouse_console*)
+ (hypervisor_mem + header->console_page);
+
/* Copy hypervisor's binary image at beginning of the memory region
* and clear the rest to zero. */
memcpy(hypervisor_mem, hypervisor->data, hypervisor->size);
@@ -329,6 +424,9 @@ static int jailhouse_cmd_enable(struct jailhouse_system __user *arg)
}
#endif

+ console_available = CON2_TYPE(config->debug_console.flags) ==
+ JAILHOUSE_CON2_TYPE_ROOTPAGE;
+
err = jailhouse_cell_prepare_root(&config->root_cell);
if (err)
goto error_unmap;
diff --git a/driver/main.h b/driver/main.h
index e01ca5b52f..fe322ead18 100644
--- a/driver/main.h
+++ b/driver/main.h
@@ -22,5 +22,7 @@ extern bool jailhouse_enabled;

void *jailhouse_ioremap(phys_addr_t phys, unsigned long virt,
unsigned long size);
+int jailhouse_console_page_delta(char *dst, unsigned int head,
+ unsigned int *miss);

#endif /* !_JAILHOUSE_DRIVER_MAIN_H */
diff --git a/driver/sysfs.c b/driver/sysfs.c
index 8da1f766ce..151a5bf46a 100644
--- a/driver/sysfs.c
+++ b/driver/sysfs.c
@@ -309,6 +309,30 @@ void jailhouse_sysfs_cell_delete(struct cell *cell)
kobject_put(&cell->kobj);
}

+static ssize_t console_show(struct device *dev, struct device_attribute *attr,
+ char *buffer)
+{
+ ssize_t ret;
+
+ if (mutex_lock_interruptible(&jailhouse_lock) != 0)
+ return -EINTR;
+
+ ret = jailhouse_console_page_delta(buffer, 0, NULL);
+ if (ret > 0)
+ /* We can safely fill the whole buffer and append a terminating
+ * \0, as buffer comes preallocated with PAGE_SIZE and
+ * the content is smaller than PAGE_SIZE */
+ buffer[ret++] = 0;
+
+ /* don't return error if jailhouse is not enabled */
+ if (ret == -EAGAIN)
+ ret = 0;
+
+ mutex_unlock(&jailhouse_lock);
+
+ return ret;
+}
+
static ssize_t enabled_show(struct device *dev, struct device_attribute *attr,
char *buffer)
{
@@ -361,6 +385,7 @@ static ssize_t remap_pool_used_show(struct device *dev,
return info_show(dev, buffer, JAILHOUSE_INFO_REMAP_POOL_USED);
}

+static DEVICE_ATTR_RO(console);
static DEVICE_ATTR_RO(enabled);
static DEVICE_ATTR_RO(mem_pool_size);
static DEVICE_ATTR_RO(mem_pool_used);
@@ -368,6 +393,7 @@ static DEVICE_ATTR_RO(remap_pool_size);
static DEVICE_ATTR_RO(remap_pool_used);

static struct attribute *jailhouse_sysfs_entries[] = {
+ &dev_attr_console.attr,
&dev_attr_enabled.attr,
&dev_attr_mem_pool_size.attr,
&dev_attr_mem_pool_used.attr,
--
2.11.0

Ralf Ramsauer

unread,
Jan 19, 2017, 3:12:31 PM1/19/17
to Jan Kiszka, jailho...@googlegroups.com, Ralf Ramsauer
If the console is enabled. Don't write messages to the console page if
it is disabled. Otherwise we would leak information if we disable the
hypervisor.

Signed-off-by: Ralf Ramsauer <ralf.r...@oth-regensburg.de>
---
hypervisor/printk.c | 21 ++++++++++++++++++++-
1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/hypervisor/printk.c b/hypervisor/printk.c
index 117fe74c96..63faed1d39 100644
--- a/hypervisor/printk.c
+++ b/hypervisor/printk.c
@@ -23,7 +23,26 @@ volatile struct jailhouse_console console __attribute__((section(".console")));

static DEFINE_SPINLOCK(printk_lock);

-#define console_write(msg) arch_dbg_write(msg)
+static void console_write(const char *msg)
+{
+ arch_dbg_write(msg);
+
+ if (!virtual_console)
+ return;
+
+ console.lock = true;
+ /* ensure the lock is visible prior to updates of the content */
+ memory_barrier();
+ while (*msg) {
+ console.content[console.tail % sizeof(console.content)] =
+ *msg++;
+ console.tail++;
+ }
+ /* ensure that all updates are committed before releasing the lock */
+ memory_barrier();
+ console.lock = false;
+}
+
#include "printk-core.c"

static void dbg_write_stub(const char *msg)
--
2.11.0

Ralf Ramsauer

unread,
Jan 19, 2017, 3:12:31 PM1/19/17
to Jan Kiszka, jailho...@googlegroups.com, Ralf Ramsauer
Driver must know the location of the console_page. Use the
jailhouse_header for propagating the offset.

Signed-off-by: Ralf Ramsauer <ralf.r...@oth-regensburg.de>
---
hypervisor/include/jailhouse/header.h | 3 +++
hypervisor/setup.c | 1 +
2 files changed, 4 insertions(+)

diff --git a/hypervisor/include/jailhouse/header.h b/hypervisor/include/jailhouse/header.h
index ccb9684243..132877a4a7 100644
--- a/hypervisor/include/jailhouse/header.h
+++ b/hypervisor/include/jailhouse/header.h
@@ -53,6 +53,9 @@ struct jailhouse_header {
/** Entry point (arch_entry()).
* @note Filled at build time. */
int (*entry)(unsigned int);
+ /** Offset of the console page inside the hypervisor memory
+ * @note Filled at build time. */
+ unsigned long console_page;

/** Configured maximum logical CPU ID + 1.
* @note Filled by Linux loader driver before entry. */
diff --git a/hypervisor/setup.c b/hypervisor/setup.c
index 9b1c0c1df7..3a8c188024 100644
--- a/hypervisor/setup.c
+++ b/hypervisor/setup.c
@@ -232,4 +232,5 @@ hypervisor_header = {
.core_size = (unsigned long)__page_pool - JAILHOUSE_BASE,
.percpu_size = sizeof(struct per_cpu),
.entry = arch_entry - JAILHOUSE_BASE,
+ .console_page = (unsigned long)&console - JAILHOUSE_BASE,
};
--
2.11.0

Ralf Ramsauer

unread,
Jan 19, 2017, 3:12:31 PM1/19/17
to Jan Kiszka, jailho...@googlegroups.com, Ralf Ramsauer
Signed-off-by: Ralf Ramsauer <ralf.r...@oth-regensburg.de>
---
Documentation/debug-output.md | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/Documentation/debug-output.md b/Documentation/debug-output.md
index 6367fbcd93..1c11ca180a 100644
--- a/Documentation/debug-output.md
+++ b/Documentation/debug-output.md
@@ -84,6 +84,13 @@ Example configuration for disabled debug output (architecture independent):
}
```

+Hypervisor Console via sysfs
+----------------------------
+
+If the debug console of root cell has the flag JAILHOUSE_CON2_TYPE_ROOTPAGE
+set, the hypervisor console is available through
+/sys/devices/jailhouse/console.
+
Inmates
-------

--
2.11.0

Ralf Ramsauer

unread,
Jan 19, 2017, 3:12:32 PM1/19/17
to Jan Kiszka, jailho...@googlegroups.com, Ralf Ramsauer
Signed-off-by: Ralf Ramsauer <ralf.r...@oth-regensburg.de>
---
Documentation/debug-output.md | 8 +++-
driver/main.c | 97 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 104 insertions(+), 1 deletion(-)

diff --git a/Documentation/debug-output.md b/Documentation/debug-output.md
index 1c11ca180a..5d0610a154 100644
--- a/Documentation/debug-output.md
+++ b/Documentation/debug-output.md
@@ -89,7 +89,13 @@ Hypervisor Console via sysfs

If the debug console of root cell has the flag JAILHOUSE_CON2_TYPE_ROOTPAGE
set, the hypervisor console is available through
-/sys/devices/jailhouse/console.
+/sys/devices/jailhouse/console. Continuous reading of the hypervisor console
+is available through /dev/jailhouse.
+
+Example
+```
+cat /dev/jailhouse
+```

Inmates
-------
diff --git a/driver/main.c b/driver/main.c
index 2a86842448..4eade5724d 100644
--- a/driver/main.c
+++ b/driver/main.c
@@ -71,6 +71,10 @@ MODULE_FIRMWARE(JAILHOUSE_FW_NAME);
#endif
MODULE_VERSION(JAILHOUSE_VERSION);

+struct console_userdata {
+ unsigned int head;
+};
+
DEFINE_MUTEX(jailhouse_lock);
bool jailhouse_enabled;

@@ -616,11 +620,104 @@ static long jailhouse_ioctl(struct file *file, unsigned int ioctl,
return err;
}

+static int jailhouse_console_open(struct inode *inode, struct file *file)
+{
+ struct console_userdata *user;
+
+ user = kzalloc(sizeof(struct console_userdata), GFP_KERNEL);
+ if (!user)
+ return -ENOMEM;
+
+ file->private_data = user;
+
+ return 0;
+}
+
+static int jailhouse_console_release(struct inode *inode, struct file *file)
+{
+ struct console_userdata *user = file->private_data;
+
+ kfree(user);
+
+ return 0;
+}
+
+static ssize_t jailhouse_console_read(struct file *file, char __user *out,
+ size_t size, loff_t *off)
+{
+ struct console_userdata *user = file->private_data;
+ char *content;
+ unsigned int miss;
+ int ret;
+
+ content = kmalloc(sizeof(console_page->content), GFP_KERNEL);
+ if (content == NULL)
+ return -ENOMEM;
+
+ /* wait for new data */
+ while (1) {
+ if (mutex_lock_interruptible(&jailhouse_lock) != 0) {
+ ret = -EINTR;
+ goto console_free_out;
+ }
+
+ ret = jailhouse_console_page_delta(content, user->head, &miss);
+
+ mutex_unlock(&jailhouse_lock);
+
+ if ((!ret || ret == -EAGAIN) && file->f_flags & O_NONBLOCK)
+ goto console_free_out;
+
+ if (ret == -EAGAIN)
+ /* Reset the user head, if jailhouse is not enabled. We
+ * have to do this, as jailhouse might be reenabled and
+ * the file handle was kept open in the meanwhile */
+ user->head = 0;
+ else if (ret < 0)
+ goto console_free_out;
+ else if (ret)
+ break;
+
+ schedule_timeout_uninterruptible(HZ / 10);
+ if (signal_pending(current)) {
+ ret = -EINTR;
+ goto console_free_out;
+ }
+ }
+
+ if (miss) {
+ /* If we missed anything, warn user. We will dump the actual
+ * content in the next call. */
+ ret = snprintf(content, sizeof(console_page->content),
+ "<NOTE: missing %u byte console log>\n",
+ miss);
+ user->head += miss;
+ if (size < ret)
+ ret = size;
+ } else {
+ if (size < ret)
+ ret = size;
+ user->head += ret;
+ }
+
+ if (copy_to_user(out, content, ret))
+ ret = -EFAULT;
+
+console_free_out:
+ set_current_state(TASK_RUNNING);
+ kfree(content);
+ return ret;
+}
+
+
static const struct file_operations jailhouse_fops = {
.owner = THIS_MODULE,
.unlocked_ioctl = jailhouse_ioctl,
.compat_ioctl = jailhouse_ioctl,
.llseek = noop_llseek,
+ .open = jailhouse_console_open,
+ .release = jailhouse_console_release,
+ .read = jailhouse_console_read,
};

static struct miscdevice jailhouse_misc_dev = {
--
2.11.0

Ralf Ramsauer

unread,
Jan 19, 2017, 3:12:32 PM1/19/17
to Jan Kiszka, jailho...@googlegroups.com, Ralf Ramsauer
for all existing system configuration. Activate it in the root cell
template as well.

Signed-off-by: Ralf Ramsauer <ralf.r...@oth-regensburg.de>
---
configs/amd-seattle.c | 3 ++-
configs/bananapi.c | 3 ++-
configs/f2a88xm-hd3.c | 3 ++-
configs/foundation-v8.c | 3 ++-
configs/imb-a180.c | 3 ++-
configs/jetson-tk1.c | 3 ++-
configs/jetson-tx1.c | 3 ++-
configs/orangepi0.c | 3 ++-
configs/qemu-vm.c | 3 ++-
configs/vexpress.c | 3 ++-
configs/zynqmp-zcu102.c | 3 ++-
tools/root-cell-config.c.tmpl | 3 ++-
12 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/configs/amd-seattle.c b/configs/amd-seattle.c
index 66e3fdcd82..81bc620813 100644
--- a/configs/amd-seattle.c
+++ b/configs/amd-seattle.c
@@ -33,7 +33,8 @@ struct {
.address = 0xe1010000,
.size = 0x1000,
.flags = JAILHOUSE_CON1_TYPE_PL011 |
- JAILHOUSE_CON1_FLAG_MMIO,
+ JAILHOUSE_CON1_FLAG_MMIO |
+ JAILHOUSE_CON2_TYPE_ROOTPAGE,
},
.platform_info = {
.pci_mmconfig_base = 0xf0000000,
diff --git a/configs/bananapi.c b/configs/bananapi.c
index 484d808ac3..f289a3e3ef 100644
--- a/configs/bananapi.c
+++ b/configs/bananapi.c
@@ -38,7 +38,8 @@ struct {
/* .gate_nr = 16 */
/* .divider = 0x0d, */
.flags = JAILHOUSE_CON1_TYPE_8250 |
- JAILHOUSE_CON1_FLAG_MMIO,
+ JAILHOUSE_CON1_FLAG_MMIO |
+ JAILHOUSE_CON2_TYPE_ROOTPAGE,
},
.platform_info = {
.pci_mmconfig_base = 0x2000000,
diff --git a/configs/f2a88xm-hd3.c b/configs/f2a88xm-hd3.c
index 62a9117b99..33973bcf2b 100644
--- a/configs/f2a88xm-hd3.c
+++ b/configs/f2a88xm-hd3.c
@@ -42,7 +42,8 @@ struct {
.address = 0x3f8,
/* .divider = 0x1, */
.flags = JAILHOUSE_CON1_TYPE_UART_X86 |
- JAILHOUSE_CON1_FLAG_PIO,
+ JAILHOUSE_CON1_FLAG_PIO |
+ JAILHOUSE_CON2_TYPE_ROOTPAGE,
},
.platform_info = {
.pci_mmconfig_base = 0xe0000000,
diff --git a/configs/foundation-v8.c b/configs/foundation-v8.c
index c4112004d9..56e4816d38 100644
--- a/configs/foundation-v8.c
+++ b/configs/foundation-v8.c
@@ -32,7 +32,8 @@ struct {
.address = 0x1c090000,
.size = 0x1000,
.flags = JAILHOUSE_CON1_TYPE_PL011 |
- JAILHOUSE_CON1_FLAG_MMIO,
+ JAILHOUSE_CON1_FLAG_MMIO |
+ JAILHOUSE_CON2_TYPE_ROOTPAGE,
},
.platform_info.arm = {
#ifdef CONFIG_ARM_GIC_V3
diff --git a/configs/imb-a180.c b/configs/imb-a180.c
index fa36d791d3..a6b9012ce5 100644
--- a/configs/imb-a180.c
+++ b/configs/imb-a180.c
@@ -41,7 +41,8 @@ struct {
.address = 0x3f8,
/* .divider = 0x1, */
.flags = JAILHOUSE_CON1_TYPE_UART_X86 |
- JAILHOUSE_CON1_FLAG_PIO,
+ JAILHOUSE_CON1_FLAG_PIO |
+ JAILHOUSE_CON2_TYPE_ROOTPAGE,
},
.platform_info = {
.pci_mmconfig_base = 0xe0000000,
diff --git a/configs/jetson-tk1.c b/configs/jetson-tk1.c
index d6713405bf..2cdb492423 100644
--- a/configs/jetson-tk1.c
+++ b/configs/jetson-tk1.c
@@ -41,7 +41,8 @@ struct {
/* .gate_nr = (65 % 32), */
/* .divider = 0xdd, */
.flags = JAILHOUSE_CON1_TYPE_8250 |
- JAILHOUSE_CON1_FLAG_MMIO,
+ JAILHOUSE_CON1_FLAG_MMIO |
+ JAILHOUSE_CON2_TYPE_ROOTPAGE,
},
.platform_info = {
.pci_mmconfig_base = 0x48000000,
diff --git a/configs/jetson-tx1.c b/configs/jetson-tx1.c
index af1f55775b..f47ebfd4aa 100644
--- a/configs/jetson-tx1.c
+++ b/configs/jetson-tx1.c
@@ -35,7 +35,8 @@ struct {
.address = 0x70006000,
.size = 0x0040,
.flags = JAILHOUSE_CON1_TYPE_8250 |
- JAILHOUSE_CON1_FLAG_MMIO,
+ JAILHOUSE_CON1_FLAG_MMIO |
+ JAILHOUSE_CON2_TYPE_ROOTPAGE,
},
.platform_info.arm = {
.gicd_base = 0x50041000,
diff --git a/configs/orangepi0.c b/configs/orangepi0.c
index 14f1a0e0dd..ae1da18601 100644
--- a/configs/orangepi0.c
+++ b/configs/orangepi0.c
@@ -35,7 +35,8 @@ struct {
.address = 0x01c28000,
.size = 0x1000,
.flags = JAILHOUSE_CON1_TYPE_8250 |
- JAILHOUSE_CON1_FLAG_MMIO,
+ JAILHOUSE_CON1_FLAG_MMIO |
+ JAILHOUSE_CON2_TYPE_ROOTPAGE,
},
.platform_info = {
.pci_mmconfig_base = 0x2000000,
diff --git a/configs/qemu-vm.c b/configs/qemu-vm.c
index 0ce34eeab6..f091834e3c 100644
--- a/configs/qemu-vm.c
+++ b/configs/qemu-vm.c
@@ -41,7 +41,8 @@ struct {
.address = 0x3f8,
/* .divider = 0x1, */
.flags = JAILHOUSE_CON1_TYPE_UART_X86 |
- JAILHOUSE_CON1_FLAG_PIO,
+ JAILHOUSE_CON1_FLAG_PIO |
+ JAILHOUSE_CON2_TYPE_ROOTPAGE,
},
.platform_info = {
.pci_mmconfig_base = 0xb0000000,
diff --git a/configs/vexpress.c b/configs/vexpress.c
index 61c99b9b73..065db65600 100644
--- a/configs/vexpress.c
+++ b/configs/vexpress.c
@@ -32,7 +32,8 @@ struct {
.address = 0x1c090000,
.size = 0x1000,
.flags = JAILHOUSE_CON1_TYPE_PL011 |
- JAILHOUSE_CON1_FLAG_MMIO,
+ JAILHOUSE_CON1_FLAG_MMIO |
+ JAILHOUSE_CON2_TYPE_ROOTPAGE,
},
.platform_info.arm = {
#ifdef CONFIG_ARM_GIC_V3
diff --git a/configs/zynqmp-zcu102.c b/configs/zynqmp-zcu102.c
index 120a73dc6d..458fc24faa 100644
--- a/configs/zynqmp-zcu102.c
+++ b/configs/zynqmp-zcu102.c
@@ -35,7 +35,8 @@ struct {
.address = 0xff000000,
.size = 0x1000,
.flags = JAILHOUSE_CON1_TYPE_XUARTPS |
- JAILHOUSE_CON1_FLAG_MMIO,
+ JAILHOUSE_CON1_FLAG_MMIO |
+ JAILHOUSE_CON2_TYPE_ROOTPAGE,
},
.platform_info = {
.pci_mmconfig_base = 0xfc000000,
diff --git a/tools/root-cell-config.c.tmpl b/tools/root-cell-config.c.tmpl
index 85c1834ac3..11956b742b 100644
--- a/tools/root-cell-config.c.tmpl
+++ b/tools/root-cell-config.c.tmpl
@@ -63,7 +63,8 @@ struct {
.debug_console = {
.address = 0x3f8,
.flags = JAILHOUSE_CON1_TYPE_UART_X86 |
- JAILHOUSE_CON1_FLAG_PIO,
+ JAILHOUSE_CON1_FLAG_PIO |
+ JAILHOUSE_CON2_TYPE_ROOTPAGE,

Ralf Ramsauer

unread,
Jan 19, 2017, 3:12:32 PM1/19/17
to Jan Kiszka, jailho...@googlegroups.com, Ralf Ramsauer
This allows us to dump messages if the hypervisor failed to start or the
last few messages after the hypervisor was disabled. This needs some
special treatment, as the hypervisor_memory region is unmapped after
failures.

Hook into disable an failure paths and copy the page to a dedicated
variable. Indicate with a boolean flag that data is present.

Signed-off-by: Ralf Ramsauer <ralf.r...@oth-regensburg.de>
---
driver/main.c | 46 +++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 45 insertions(+), 1 deletion(-)

diff --git a/driver/main.c b/driver/main.c
index 4eade5724d..439ff9b1b2 100644
--- a/driver/main.c
+++ b/driver/main.c
@@ -73,6 +73,7 @@ MODULE_VERSION(JAILHOUSE_VERSION);

struct console_userdata {
unsigned int head;
+ unsigned int last_console_id;
};

DEFINE_MUTEX(jailhouse_lock);
@@ -86,6 +87,24 @@ static int error_code;
static struct jailhouse_console* volatile console_page;
static bool console_available;

+/* last_console contains three members:
+ * - valid: indicates if content in the page member is present
+ * - id: hint for the consumer if it already consumed the content
+ * - page: actual content
+ *
+ * Those members are updated in following cases:
+ * - on disabling the hypervisor to print last messages
+ * - on failures when enabling the hypervisor
+ *
+ * We need this structure, as in those cases the hypervisor memory gets
+ * unmapped.
+ */
+static struct {
+ bool valid;
+ unsigned int id;
+ struct jailhouse_console page;
+} last_console;
+
#ifdef CONFIG_X86
bool jailhouse_use_vmcall;

@@ -116,6 +135,16 @@ static void copy_console_page(struct jailhouse_console *dst)
} while (console_page->tail != tail || console_page->lock);
}

+static inline void update_last_console(void)
+{
+ if (!console_available)
+ return;
+
+ copy_console_page(&last_console.page);
+ last_console.id++;
+ last_console.valid = true;
+}
+
static long get_max_cpus(u32 cpu_set_size,
const struct jailhouse_system __user *system_config)
{
@@ -371,6 +400,7 @@ static int jailhouse_cmd_enable(struct jailhouse_system __user *arg)

console_page = (struct jailhouse_console*)
(hypervisor_mem + header->console_page);
+ last_console.valid = false;

/* Copy hypervisor's binary image at beginning of the memory region
* and clear the rest to zero. */
@@ -472,6 +502,7 @@ static int jailhouse_cmd_enable(struct jailhouse_system __user *arg)
return 0;

error_free_cell:
+ update_last_console();
jailhouse_cell_delete_root();

error_unmap:
@@ -571,6 +602,8 @@ static int jailhouse_cmd_disable(void)
if (err)
goto unlock_out;

+ update_last_console();
+
vunmap(hypervisor_mem);

jailhouse_cell_delete_root();
@@ -661,7 +694,18 @@ static ssize_t jailhouse_console_read(struct file *file, char __user *out,
goto console_free_out;
}

- ret = jailhouse_console_page_delta(content, user->head, &miss);
+ if (last_console.id != user->last_console_id &&
+ last_console.valid) {
+ ret = jailhouse_console_delta(&last_console.page,
+ content, user->head,
+ &miss);
+ if (!ret)
+ user->last_console_id =
+ last_console.id;
+ } else {
+ ret = jailhouse_console_page_delta(content, user->head,
+ &miss);
+ }

mutex_unlock(&jailhouse_lock);

--
2.11.0

Ralf Ramsauer

unread,
Jan 19, 2017, 3:12:32 PM1/19/17
to Jan Kiszka, jailho...@googlegroups.com, Ralf Ramsauer
From now on, 'jailhouse enable' will dump the console if an error occurs
during initialisation of the hypervisor.

Signed-off-by: Ralf Ramsauer <ralf.r...@oth-regensburg.de>
---
tools/jailhouse.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/jailhouse.c b/tools/jailhouse.c
index ede6ad4412..904c0658f3 100644
--- a/tools/jailhouse.c
+++ b/tools/jailhouse.c
@@ -234,8 +234,10 @@ static int enable(int argc, char *argv[])
fd = open_dev();

err = ioctl(fd, JAILHOUSE_ENABLE, config);
- if (err)
+ if (err) {
perror("JAILHOUSE_ENABLE");
+ dump_console(fd, true);
+ }

close(fd);
free(config);
--
2.11.0

Ralf Ramsauer

unread,
Jan 19, 2017, 3:12:32 PM1/19/17
to Jan Kiszka, jailho...@googlegroups.com, Ralf Ramsauer
Similar to other tools, like 'tail -f', allow 'jailhosue console' to
follow the console output when jailhouse gets enabled.

'jailhouse console' will print all available console log of the
hypervisor.

'jailhouse console -f' will print all available console log of the
hypervisor and append new data as it arrives.

Use this patch to remove a redundant '|' in the usage string of the
tool.

Signed-off-by: Ralf Ramsauer <ralf.r...@oth-regensburg.de>
---
tools/jailhouse.c | 47 +++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 45 insertions(+), 2 deletions(-)

diff --git a/tools/jailhouse.c b/tools/jailhouse.c
index 84e5fe3d64..ede6ad4412 100644
--- a/tools/jailhouse.c
+++ b/tools/jailhouse.c
@@ -60,7 +60,7 @@ static void __attribute__((noreturn)) help(char *prog, int exit_status)
{
const struct extension *ext;

- printf("Usage: %s { COMMAND | --help || --version }\n"
+ printf("Usage: %s { COMMAND | --help | --version }\n"
"\nAvailable commands:\n"
" enable SYSCONFIG\n"
" disable\n"
@@ -71,7 +71,8 @@ static void __attribute__((noreturn)) help(char *prog, int exit_status)
" [-a | --address ADDRESS] ...\n"
" cell start { ID | [--name] NAME }\n"
" cell shutdown { ID | [--name] NAME }\n"
- " cell destroy { ID | [--name] NAME }\n",
+ " cell destroy { ID | [--name] NAME }\n"
+ " console [-f | --follow]\n",
basename(prog));
for (ext = extensions; ext->cmd; ext++)
printf(" %s %s %s\n", ext->cmd, ext->subcmd, ext->help);
@@ -79,6 +80,30 @@ static void __attribute__((noreturn)) help(char *prog, int exit_status)
exit(exit_status);
}

+static int dump_console(int fd, bool non_block)
+{
+ int ret;
+ char buffer[128];
+
+ if (non_block) {
+ ret = fcntl(fd, F_GETFL, 0);
+ if (ret == -1)
+ return -errno;
+ ret = fcntl(fd, F_SETFL, ret | O_NONBLOCK);
+ if (ret == -1)
+ return -errno;
+ }
+
+ do {
+ ret = read(fd, buffer, sizeof(buffer));
+ if (ret < 0)
+ break;
+ ret = write(STDOUT_FILENO, buffer, ret);
+ } while (ret > 0);
+
+ return ret;
+}
+
static void call_extension_script(const char *cmd, int argc, char *argv[])
{
const struct extension *ext;
@@ -504,6 +529,22 @@ static int cell_management(int argc, char *argv[])
return err;
}

+static int console(int argc, char *argv[])
+{
+ int fd;
+ ssize_t ret;
+ bool non_block = false;
+
+ if (!(argc == 3 && match_opt(argv[2], "-f", "--follow")))
+ non_block = true;
+
+ fd = open_dev();
+ ret = dump_console(fd, non_block);
+ close(fd);
+
+ return ret;
+}
+
int main(int argc, char *argv[])
{
int fd;
@@ -522,6 +563,8 @@ int main(int argc, char *argv[])
close(fd);
} else if (strcmp(argv[1], "cell") == 0) {
err = cell_management(argc, argv);
+ } else if (strcmp(argv[1], "console") == 0) {
+ err = console(argc, argv);
} else if (strcmp(argv[1], "config") == 0 ||
strcmp(argv[1], "hardware") == 0) {
call_extension_script(argv[1], argc, argv);
--
2.11.0

Jan Kiszka

unread,
Jan 21, 2017, 2:37:16 AM1/21/17
to Ralf Ramsauer, jailho...@googlegroups.com
On 2017-01-19 21:11, Ralf Ramsauer wrote:
> Hi,
>
> I implemented a sysfs interface for the hypervisor console. This is useful for
> debugging devices without UARTs. At least as long as the root-cell doesn't
> crash...
>
> Additionally, I implemented continuous reading of the hypervisor console via
> /dev/jailhouse, which is a pretty nice and easy way to follow the hypervisor
> console from Linux.
>
> I use a page in the hypervisor's memory space as ringbuffer. If the root cell
> has a certain permission enabled in its config, the hypervisor won't back that
> page with empty_page. The Linux driver may then read from that page without any
> interactions with the hypervisor. We do this without any signalling or locking.
> Otherwise we would risk starvation of the hypervisor. The producer (hypervisor
> debug write) only indicates ongoing writes by setting a flag.
>
> tree:
> https://github.com/lfd/jailhouse/tree/hvconsoleV4

Started to play with it, looks good, but I need to check in more details
again the next days or so.

>
> PS: maybe we could think of enabling JAILHOUSE_CELL_DEBUG_CONSOLE as a default
> flag for our demo inmate cell configuration. As Jan already pointed out
> earlier, none of them is permissive.

I don't get this yet: which configs do you want to enable, and why? The
linux inmates?

Jan

Ralf Ramsauer

unread,
Jan 21, 2017, 9:19:55 AM1/21/17
to Jan Kiszka, jailho...@googlegroups.com
On 01/21/2017 08:37 AM, Jan Kiszka wrote:
> On 2017-01-19 21:11, Ralf Ramsauer wrote:
>> Hi,
>>
>> I implemented a sysfs interface for the hypervisor console. This is useful for
>> debugging devices without UARTs. At least as long as the root-cell doesn't
>> crash...
>>
>> Additionally, I implemented continuous reading of the hypervisor console via
>> /dev/jailhouse, which is a pretty nice and easy way to follow the hypervisor
>> console from Linux.
>>
>> I use a page in the hypervisor's memory space as ringbuffer. If the root cell
>> has a certain permission enabled in its config, the hypervisor won't back that
>> page with empty_page. The Linux driver may then read from that page without any
>> interactions with the hypervisor. We do this without any signalling or locking.
>> Otherwise we would risk starvation of the hypervisor. The producer (hypervisor
>> debug write) only indicates ongoing writes by setting a flag.
>>
>> tree:
>> https://github.com/lfd/jailhouse/tree/hvconsoleV4
>
> Started to play with it, looks good, but I need to check in more details
> again the next days or so.
Great, thanks.
>
>>
>> PS: maybe we could think of enabling JAILHOUSE_CELL_DEBUG_CONSOLE as a default
>> flag for our demo inmate cell configuration. As Jan already pointed out
>> earlier, none of them is permissive.
>
> I don't get this yet: which configs do you want to enable, and why? The
> linux inmates?
Yes, and other inmates as well. At the moment, inmates are not allowed
to use the debug console by default. The virtual jailhouse console is
kind of a debug feature as well. Currently we only get Jailhouse messages.

Ralf
>
> Jan
>

Jan Kiszka

unread,
Jan 24, 2017, 10:23:43 AM1/24/17
to Ralf Ramsauer, jailho...@googlegroups.com
On 2017-01-19 21:11, Ralf Ramsauer wrote:
Reviewed-by: Jan Kiszka <jan.k...@siemens.com>


Jan Kiszka

unread,
Jan 24, 2017, 10:23:51 AM1/24/17
to Ralf Ramsauer, jailho...@googlegroups.com
On 2017-01-19 21:11, Ralf Ramsauer wrote:
> The console page is implemented as ring buffer, and will _not_ contain a
> trailing \0 for string termination.
>
> Signed-off-by: Ralf Ramsauer <ralf.r...@oth-regensburg.de>
> ---
> hypervisor/hypervisor.lds.S | 6 ++++++
> hypervisor/include/jailhouse/cell-config.h | 6 ++++++
> hypervisor/include/jailhouse/header.h | 8 ++++++++
> hypervisor/include/jailhouse/printk.h | 2 ++
> hypervisor/printk.c | 2 ++
> 5 files changed, 24 insertions(+)
>
> diff --git a/hypervisor/hypervisor.lds.S b/hypervisor/hypervisor.lds.S
> index 09b1a0b2e3..f81782820d 100644
> --- a/hypervisor/hypervisor.lds.S
> +++ b/hypervisor/hypervisor.lds.S
> @@ -35,6 +35,12 @@ SECTIONS
> . = ALIGN(16);
> .bss : { *(.bss) }
>
> + /* The console section shall only contain the hypervisor console. This
> + * section and the next section must be aligned to PAGE_SIZE, as we
> + * want the console section to reserve a whole page */

"...as we will map the console section, and only that section, as a
whole page to the root cell."
Maybe something like "busy" is more appropriate here as we neither lock
writers nor require readers to take that as a lock on access.

Jan

Jan Kiszka

unread,
Jan 24, 2017, 10:24:36 AM1/24/17
to Ralf Ramsauer, jailho...@googlegroups.com
On 2017-01-19 21:11, Ralf Ramsauer wrote:
Reviewed-by: Jan Kiszka <jan.k...@siemens.com>


Jan Kiszka

unread,
Jan 24, 2017, 10:24:56 AM1/24/17
to Ralf Ramsauer, jailho...@googlegroups.com
On 2017-01-19 21:11, Ralf Ramsauer wrote:
Reviewed-by: Jan Kiszka <jan.k...@siemens.com>


Jan Kiszka

unread,
Jan 24, 2017, 10:25:08 AM1/24/17
to Ralf Ramsauer, jailho...@googlegroups.com
On 2017-01-19 21:11, Ralf Ramsauer wrote:
Reviewed-by: Jan Kiszka <jan.k...@siemens.com>


Jan Kiszka

unread,
Jan 24, 2017, 10:41:23 AM1/24/17
to Ralf Ramsauer, jailho...@googlegroups.com
On 2017-01-19 21:11, Ralf Ramsauer wrote:
I would use a local var for "missed", check at the end if the passed
pointer is non-NULL, and only then write back.

> +
> + /* check if tail overflowed */
> + if (console->tail < head)
> + delta = (0 - head) + console->tail;
> + else
> + delta = console->tail - head;

Maybe me mind is still frozen, but these two statements should generate
the same instructions, shouldn't they...?
Can you find some more telling names for this one vs.
"jailhouse_console_delta"? Both are missing some verb, and it's not
obvious in what they are different?
The "why" for terminating the buffer with 0 should be mentioned. It's
not clear to me at least
Jan

Jan Kiszka

unread,
Jan 24, 2017, 10:43:56 AM1/24/17
to Ralf Ramsauer, jailho...@googlegroups.com
On 2017-01-19 21:11, Ralf Ramsauer wrote:
...and the "jailhouse console" command later on as well. Maybe worth
adding that and moving the patch down the queue.

Jan

> +
> Inmates
> -------
>
>

Jan Kiszka

unread,
Jan 24, 2017, 10:45:47 AM1/24/17
to Ralf Ramsauer, jailho...@googlegroups.com
On 2017-01-19 21:11, Ralf Ramsauer wrote:
The hikey root cell is missing.

Jan

Jan Kiszka

unread,
Jan 24, 2017, 10:47:21 AM1/24/17
to Ralf Ramsauer, jailho...@googlegroups.com
Ah, the file is extended later on. Unintuitive split-up, but that's minor.

Reviewed-by: Jan Kiszka <jan.k...@siemens.com>

> Jan
>
>> +
>> Inmates
>> -------
>>
>>
>

Ralf Ramsauer

unread,
Jan 24, 2017, 11:03:30 AM1/24/17
to Jan Kiszka, jailho...@googlegroups.com
Let me squash documentation and put it to the end, that's in deed more
intuitive.

Ralf
>
>> Jan
>>
>>> +
>>> Inmates
>>> -------
>>>
>>>
>>
>

Jan Kiszka

unread,
Jan 24, 2017, 11:04:18 AM1/24/17
to Ralf Ramsauer, jailho...@googlegroups.com
On 2017-01-19 21:11, Ralf Ramsauer wrote:
How about "console_state"?
"missed %u bytes of console log"

> + miss);
> + user->head += miss;
> + if (size < ret)
> + ret = size;

Why is user->head only update by "miss" here? ...

> + } else {
> + if (size < ret)
> + ret = size;
> + user->head += ret;

...And shouldn't the last line be pulled out and executed
unconditionally? Actually, that would allow to pull the "if size < ret"
as well.

> + }
> +
> + if (copy_to_user(out, content, ret))
> + ret = -EFAULT;
> +
> +console_free_out:
> + set_current_state(TASK_RUNNING);
> + kfree(content);
> + return ret;
> +}
> +
> +
> static const struct file_operations jailhouse_fops = {
> .owner = THIS_MODULE,
> .unlocked_ioctl = jailhouse_ioctl,
> .compat_ioctl = jailhouse_ioctl,
> .llseek = noop_llseek,
> + .open = jailhouse_console_open,
> + .release = jailhouse_console_release,
> + .read = jailhouse_console_read,
> };
>
> static struct miscdevice jailhouse_misc_dev = {
>

Jan

Jan Kiszka

unread,
Jan 24, 2017, 11:12:51 AM1/24/17
to Ralf Ramsauer, jailho...@googlegroups.com
On 2017-01-19 21:11, Ralf Ramsauer wrote:
Read and writes are synchronized by the jailhouse_lock, right?
What if ret is 0, but there would be something to read from the new
console? Or isn't that possible? Hmm, we declare the last_console
invalid on next enable. So enable will be a reset, ok.

> + } else {
> + ret = jailhouse_console_page_delta(content, user->head,
> + &miss);
> + }
>
> mutex_unlock(&jailhouse_lock);
>
>

Jan

Jan Kiszka

unread,
Jan 24, 2017, 11:23:52 AM1/24/17
to Ralf Ramsauer, jailho...@googlegroups.com
On 2017-01-19 21:11, Ralf Ramsauer wrote:
Let's move that line before the first "cell" command - the extension
scripts will add further "cell" entries, and that breaks ordering.

> basename(prog));
> for (ext = extensions; ext->cmd; ext++)
> printf(" %s %s %s\n", ext->cmd, ext->subcmd, ext->help);
> @@ -79,6 +80,30 @@ static void __attribute__((noreturn)) help(char *prog, int exit_status)
> exit(exit_status);
> }
>
> +static int dump_console(int fd, bool non_block)
> +{
> + int ret;
> + char buffer[128];
> +
> + if (non_block) {
> + ret = fcntl(fd, F_GETFL, 0);
> + if (ret == -1)
> + return -errno;
> + ret = fcntl(fd, F_SETFL, ret | O_NONBLOCK);

We can set O_NONBLOCK without reading the current flags - we know that
there are none set.
You should handle invalid options as well and dump help() in that case.

> +
> + fd = open_dev();
> + ret = dump_console(fd, non_block);
> + close(fd);
> +
> + return ret;
> +}
> +
> int main(int argc, char *argv[])
> {
> int fd;
> @@ -522,6 +563,8 @@ int main(int argc, char *argv[])
> close(fd);
> } else if (strcmp(argv[1], "cell") == 0) {
> err = cell_management(argc, argv);
> + } else if (strcmp(argv[1], "console") == 0) {
> + err = console(argc, argv);
> } else if (strcmp(argv[1], "config") == 0 ||
> strcmp(argv[1], "hardware") == 0) {
> call_extension_script(argv[1], argc, argv);
>

Jan

Jan Kiszka

unread,
Jan 24, 2017, 11:29:05 AM1/24/17
to Ralf Ramsauer, jailho...@googlegroups.com
On 2017-01-19 21:11, Ralf Ramsauer wrote:
Wondering if we should do the same on errors of other commands, but then
only the delta...

In its simple form, this patch is

Ralf Ramsauer

unread,
Jan 24, 2017, 11:35:11 AM1/24/17
to Jan Kiszka, jailho...@googlegroups.com
All-clear, no chilblains. Had to think twice, but it's the same, yes. We
don't even need to check for this case.

This must be an artifact when i was using a content size that was not a
power of two, because then you need to check for this case.
Mmhm. My intention was:
- jailhouse_console_delta: dumps the delta of an arbitrary struct
jailhouse_console
- jailhouse_console_page_delta: dumps the delta of THE console_page
does locking, checks its availability, copies content, ...

What about:
jailhouse_dump_console_delta()
jailhouse_dump_console_{log/page}_delta()
... wait -- we don't even need to do any string termination here.
Returning the size of the content inside the buffer should be sufficient.

Ralf

Ralf Ramsauer

unread,
Jan 24, 2017, 11:54:47 AM1/24/17
to Jan Kiszka, jailho...@googlegroups.com
Without brackets and without \n? Didn't you propose brackets?

>
>> + miss);
>> + user->head += miss;
>> + if (size < ret)
>> + ret = size;
>
> Why is user->head only update by "miss" here? ...
In this round we're only going to return 'missed %u byte console log' to
userspace and forward the head to the latest valid position.

Rest of the content is caught in the next round, right afterwards.
Otherwise we have a significant amount of error checking and rewinding
and string composition, if we don't want to loose any content.
>
>> + } else {
>> + if (size < ret)
>> + ret = size;
>> + user->head += ret;
>
> ...And shouldn't the last line be pulled out and executed
> unconditionally? Actually, that would allow to pull the "if size < ret"
> as well.
No, in case of misses, we don't want to increase the head by the length
of the 'missed %u byte console log' string.

"user->head += ret" forwards the head by the amount of bytes that were
successfully read from the ring buffer and fit into userspace buffer.


"user->head += miss" forwards the head to the next valid byte inside the
ringbuffer, that will be caught in the next round. In this case, ret
countains the number of bytes that are returned by the function and
represent the length of the 'missing foo bytes' message.

Ralf

Ralf Ramsauer

unread,
Jan 24, 2017, 12:02:13 PM1/24/17
to Jan Kiszka, jailho...@googlegroups.com
Yes, we're calling update_last_console() only in contexts where we
already hold the lock.

We're only reading from jailhouse_console_read() by calling either
jailhouse_console_delta() or jailhouse_console_page_delta(). This short
critical section is protected by the jailhouse_lock as well.

Everything should be fine here.
Yep, and again: protected by jailhouse_lock.

Ralf

Ralf Ramsauer

unread,
Jan 24, 2017, 2:29:36 PM1/24/17
to Jan Kiszka, jailho...@googlegroups.com
Yes, would be easy. But the hypervisor console output isn't really
helpful in those cases right now.
>
> In its simple form, this patch is
>
> Reviewed-by: Jan Kiszka <jan.k...@siemens.com>
Again, thanks for the review.

Most of your comments are already addressed, please find v5 here:
https://github.com/lfd/jailhouse/tree/hvconsoleV5

(still have to find a proper name for jailhouse_console_delta)

Ralf
>
> Jan
>

Jan Kiszka

unread,
Jan 25, 2017, 4:17:00 AM1/25/17
to Ralf Ramsauer, jailho...@googlegroups.com
One is inner/lower the other is outer/higher level. It's not clear from
the name which is which, but maybe describing the functions can help.
Perfect. The simpler, the better.

Jan

Jan Kiszka

unread,
Jan 25, 2017, 4:18:16 AM1/25/17
to Ralf Ramsauer, jailho...@googlegroups.com
I just meant to adjust the message text, not the formatting.

>
>>
>>> + miss);
>>> + user->head += miss;
>>> + if (size < ret)
>>> + ret = size;
>>
>> Why is user->head only update by "miss" here? ...
> In this round we're only going to return 'missed %u byte console log' to
> userspace and forward the head to the latest valid position.
>
> Rest of the content is caught in the next round, right afterwards.
> Otherwise we have a significant amount of error checking and rewinding
> and string composition, if we don't want to loose any content.

OK, now I see.

>>
>>> + } else {
>>> + if (size < ret)
>>> + ret = size;
>>> + user->head += ret;
>>
>> ...And shouldn't the last line be pulled out and executed
>> unconditionally? Actually, that would allow to pull the "if size < ret"
>> as well.
> No, in case of misses, we don't want to increase the head by the length
> of the 'missed %u byte console log' string.
>
> "user->head += ret" forwards the head by the amount of bytes that were
> successfully read from the ring buffer and fit into userspace buffer.
>
>
> "user->head += miss" forwards the head to the next valid byte inside the
> ringbuffer, that will be caught in the next round. In this case, ret
> countains the number of bytes that are returned by the function and
> represent the length of the 'missing foo bytes' message.

Makes sense, thanks.

Jan

Ralf Ramsauer

unread,
Jan 25, 2017, 7:11:51 AM1/25/17
to Jan Kiszka, jailho...@googlegroups.com, Ralf Ramsauer
Hi,

I implemented a sysfs interface for the hypervisor console. This is useful for
debugging devices without UARTs. At least as long as the root-cell doesn't
crash...

Additionally, I implemented continuous reading of the hypervisor console via
/dev/jailhouse, which is a pretty nice and easy way to follow the hypervisor
console from Linux.

I use a page in the hypervisor's memory space as ringbuffer. If the root cell
has a certain permission enabled in its config, the hypervisor won't back that
page with empty_page. The Linux driver may then read from that page without any
interactions with the hypervisor. We do this without any signalling or locking.
Otherwise we would risk starvation of the hypervisor. The producer (hypervisor
debug write) only indicates ongoing writes by setting a flag.

tree:
https://github.com/lfd/jailhouse/tree/hvconsoleV5

Ralf

since v4:
- add proper function names
- minor fixes and cleanups
- move documentation to the end

since v3:
- fix memory barriers
- improve handling of last_page
- don't increment config file revision: we using upper bits for CON2_TYPE
- minor improvements

since v2:
- support continuous reading on /dev/jailhouse as well as non-blocking reads
- support overflows of the tail pointer
- adopt userspace tool to dump the console (either by switching a command
line flag or automatically on errors)
- ring buffer size must be a power of two (otherwise modulo fails on tail
pointer overflows)
- successfully tested on Qemu and Tegra [XK]1

since v1:
- refactor JAILHOUSE_CON_TYPE to JAILHOUSE_CON1_TYPE
- use JAILHOUSE_CON2_TYPE_ROOTPAGE as permission flag for sysfs console
- no extra section in linker file required

Ralf Ramsauer (12):
configs, core: Refactor JAILHOUSE_CON_TYPE to JAILHOUSE_CON1_TYPE
core: declare and define structures used for virtual console
core: don't hide console page
driver, core: propagate offset of console page
core: print console messages to the console page
driver: add console sysfs attribute
configs: activate sysfs console
driver: implement continous console reading on /dev/jailhouse
driver: copy the console_page after the hypervisor is disabled
tools: implement 'jailhouse console [-f]'
tools: dump error console on failures
Documentation: add documentation for sysfs console

Documentation/debug-output.md | 37 +++--
Documentation/sysfs-entries.txt | 3 +
Documentation/vga-console.txt | 2 +-
TODO.md | 1 -
configs/amd-seattle.c | 5 +-
configs/bananapi.c | 5 +-
configs/f2a88xm-hd3.c | 5 +-
configs/foundation-v8.c | 5 +-
configs/hikey.c | 5 +-
configs/imb-a180.c | 5 +-
configs/jetson-tk1.c | 5 +-
configs/jetson-tx1.c | 5 +-
configs/orangepi0.c | 5 +-
configs/qemu-vm.c | 5 +-
configs/vexpress.c | 5 +-
configs/zynqmp-zcu102.c | 5 +-
driver/main.c | 238 ++++++++++++++++++++++++++++-
driver/main.h | 2 +
driver/sysfs.c | 20 +++
hypervisor/arch/arm-common/dbg-write.c | 10 +-
hypervisor/arch/x86/dbg-write.c | 6 +-
hypervisor/arch/x86/uart.c | 2 +-
hypervisor/hypervisor.lds.S | 9 ++
hypervisor/include/jailhouse/cell-config.h | 28 ++--
hypervisor/include/jailhouse/header.h | 11 ++
hypervisor/include/jailhouse/printk.h | 3 +
hypervisor/paging.c | 2 +-
hypervisor/printk.c | 24 ++-
hypervisor/setup.c | 13 ++
tools/jailhouse.c | 47 +++++-
tools/root-cell-config.c.tmpl | 5 +-
31 files changed, 458 insertions(+), 65 deletions(-)

--
2.11.0

Ralf Ramsauer

unread,
Jan 25, 2017, 7:11:51 AM1/25/17
to Jan Kiszka, jailho...@googlegroups.com, Ralf Ramsauer
We're about to implement a second HV console channel: a 'virtual'
console available for the root cell via sysfs. In order to encode its
configuration in struct jailhouse_debug_console, rename
JAILHOUSE_CON_TYPE to JAILHOUSE_CON1_TYPE.

Most of the work was done automatically with sed.

Signed-off-by: Ralf Ramsauer <ralf.r...@oth-regensburg.de>
Reviewed-by: Jan Kiszka <jan.k...@siemens.com>
---
Documentation/debug-output.md | 24 ++++++++++++------------
Documentation/vga-console.txt | 2 +-
configs/amd-seattle.c | 4 ++--
configs/bananapi.c | 4 ++--
configs/f2a88xm-hd3.c | 4 ++--
configs/foundation-v8.c | 4 ++--
configs/hikey.c | 4 ++--
configs/imb-a180.c | 4 ++--
configs/jetson-tk1.c | 4 ++--
configs/jetson-tx1.c | 4 ++--
configs/orangepi0.c | 4 ++--
configs/qemu-vm.c | 4 ++--
configs/vexpress.c | 4 ++--
configs/zynqmp-zcu102.c | 4 ++--
driver/main.c | 2 +-
hypervisor/arch/arm-common/dbg-write.c | 10 +++++-----
hypervisor/arch/x86/dbg-write.c | 6 +++---
hypervisor/arch/x86/uart.c | 2 +-
hypervisor/include/jailhouse/cell-config.h | 22 +++++++++++-----------
hypervisor/paging.c | 2 +-
tools/root-cell-config.c.tmpl | 4 ++--
21 files changed, 61 insertions(+), 61 deletions(-)

diff --git a/Documentation/debug-output.md b/Documentation/debug-output.md
index 1e5888fc..6367fbcd 100644
--- a/Documentation/debug-output.md
+++ b/Documentation/debug-output.md
.debug_console = {
.address = 0x3f8, /* PIO address */
.divider = 0x1, /* 115200 Baud */
- .flags = JAILHOUSE_CON_TYPE_UART_X86 | /* generic x86 UART driver */
- JAILHOUSE_CON_FLAG_PIO, /* use PIO instead of MMIO */
+ .flags = JAILHOUSE_CON1_TYPE_UART_X86 | /* generic x86 UART driver */
+ JAILHOUSE_CON1_FLAG_PIO, /* use PIO instead of MMIO */
},
```

@@ -72,15 +72,15 @@ Example configuration for MMIO based debug output on ARM (8250 UART):
.clock_reg = 0x60006000 + 0x330, /* Optional: Debug Clock Register */
.gate_nr = (65 % 32), /* Optional: Debug Clock Gate Nr */
.divider = 0xdd, /* 115200 */
- .flags = JAILHOUSE_CON_TYPE_8250 | /* choose the 8250 driver */
- JAILHOUSE_CON_FLAG_MMIO, /* choose MMIO register access */
+ .flags = JAILHOUSE_CON1_TYPE_8250 | /* choose the 8250 driver */
+ JAILHOUSE_CON1_FLAG_MMIO, /* choose MMIO register access */
},
```

Example configuration for disabled debug output (architecture independent):
```
.debug_console = {
- .flags = JAILHOUSE_CON_TYPE_NONE,
+ .flags = JAILHOUSE_CON1_TYPE_NONE,
}
```

diff --git a/Documentation/vga-console.txt b/Documentation/vga-console.txt
index 7b378264..81dcc403 100644
--- a/Documentation/vga-console.txt
+++ b/Documentation/vga-console.txt
@@ -20,7 +20,7 @@ Add the following to the header section of your root cell's config:
.debug_console = {
.address = 0xb8000,
.size = 0x1000,
- .flags = JAILHOUSE_CON_TYPE_VGA | JAILHOUSE_CON_FLAG_MMIO,
+ .flags = JAILHOUSE_CON1_TYPE_VGA | JAILHOUSE_CON1_FLAG_MMIO,
},

Boot using the following kernel parameters:
diff --git a/configs/amd-seattle.c b/configs/amd-seattle.c
index 7d32eacc..66e3fdcd 100644
--- a/configs/amd-seattle.c
+++ b/configs/amd-seattle.c
@@ -32,8 +32,8 @@ struct {
.debug_console = {
.address = 0xe1010000,
.size = 0x1000,
- .flags = JAILHOUSE_CON_TYPE_PL011 |
- JAILHOUSE_CON_FLAG_MMIO,
+ .flags = JAILHOUSE_CON1_TYPE_PL011 |
+ JAILHOUSE_CON1_FLAG_MMIO,
},
.platform_info = {
.pci_mmconfig_base = 0xf0000000,
diff --git a/configs/bananapi.c b/configs/bananapi.c
index ef7f8240..484d808a 100644
--- a/configs/bananapi.c
+++ b/configs/bananapi.c
@@ -37,8 +37,8 @@ struct {
/* .clock_reg = 0x01c2006c, */
/* .gate_nr = 16 */
/* .divider = 0x0d, */
- .flags = JAILHOUSE_CON_TYPE_8250 |
- JAILHOUSE_CON_FLAG_MMIO,
+ .flags = JAILHOUSE_CON1_TYPE_8250 |
+ JAILHOUSE_CON1_FLAG_MMIO,
},
.platform_info = {
.pci_mmconfig_base = 0x2000000,
diff --git a/configs/f2a88xm-hd3.c b/configs/f2a88xm-hd3.c
index 7c57407b..62a9117b 100644
--- a/configs/f2a88xm-hd3.c
+++ b/configs/f2a88xm-hd3.c
@@ -41,8 +41,8 @@ struct {
.debug_console = {
.address = 0x3f8,
/* .divider = 0x1, */
- .flags = JAILHOUSE_CON_TYPE_UART_X86 |
- JAILHOUSE_CON_FLAG_PIO,
+ .flags = JAILHOUSE_CON1_TYPE_UART_X86 |
+ JAILHOUSE_CON1_FLAG_PIO,
},
.platform_info = {
.pci_mmconfig_base = 0xe0000000,
diff --git a/configs/foundation-v8.c b/configs/foundation-v8.c
index 9ae97556..c4112004 100644
--- a/configs/foundation-v8.c
+++ b/configs/foundation-v8.c
@@ -31,8 +31,8 @@ struct {
.debug_console = {
.address = 0x1c090000,
.size = 0x1000,
- .flags = JAILHOUSE_CON_TYPE_PL011 |
- JAILHOUSE_CON_FLAG_MMIO,
+ .flags = JAILHOUSE_CON1_TYPE_PL011 |
+ JAILHOUSE_CON1_FLAG_MMIO,
},
.platform_info.arm = {
#ifdef CONFIG_ARM_GIC_V3
diff --git a/configs/hikey.c b/configs/hikey.c
index c45fc96e..a739222b 100644
--- a/configs/hikey.c
+++ b/configs/hikey.c
@@ -32,8 +32,8 @@ struct {
.debug_console = {
.address = 0xf7113000,
.size = 0x1000,
- .flags = JAILHOUSE_CON_TYPE_PL011 |
- JAILHOUSE_CON_FLAG_MMIO,
+ .flags = JAILHOUSE_CON1_TYPE_PL011 |
+ JAILHOUSE_CON1_FLAG_MMIO,
},
.platform_info = {
.pci_mmconfig_base = 0xf6000000,
diff --git a/configs/imb-a180.c b/configs/imb-a180.c
index d5987bbe..fa36d791 100644
--- a/configs/imb-a180.c
+++ b/configs/imb-a180.c
@@ -40,8 +40,8 @@ struct {
.debug_console = {
.address = 0x3f8,
/* .divider = 0x1, */
- .flags = JAILHOUSE_CON_TYPE_UART_X86 |
- JAILHOUSE_CON_FLAG_PIO,
+ .flags = JAILHOUSE_CON1_TYPE_UART_X86 |
+ JAILHOUSE_CON1_FLAG_PIO,
},
.platform_info = {
.pci_mmconfig_base = 0xe0000000,
diff --git a/configs/jetson-tk1.c b/configs/jetson-tk1.c
index b8176dee..d6713405 100644
--- a/configs/jetson-tk1.c
+++ b/configs/jetson-tk1.c
@@ -40,8 +40,8 @@ struct {
/* .clock_reg = 0x60006000 + 0x330, */
/* .gate_nr = (65 % 32), */
/* .divider = 0xdd, */
- .flags = JAILHOUSE_CON_TYPE_8250 |
- JAILHOUSE_CON_FLAG_MMIO,
+ .flags = JAILHOUSE_CON1_TYPE_8250 |
+ JAILHOUSE_CON1_FLAG_MMIO,
},
.platform_info = {
.pci_mmconfig_base = 0x48000000,
diff --git a/configs/jetson-tx1.c b/configs/jetson-tx1.c
index cf80aa47..af1f5577 100644
--- a/configs/jetson-tx1.c
+++ b/configs/jetson-tx1.c
@@ -34,8 +34,8 @@ struct {
.debug_console = {
.address = 0x70006000,
.size = 0x0040,
- .flags = JAILHOUSE_CON_TYPE_8250 |
- JAILHOUSE_CON_FLAG_MMIO,
+ .flags = JAILHOUSE_CON1_TYPE_8250 |
+ JAILHOUSE_CON1_FLAG_MMIO,
},
.platform_info.arm = {
.gicd_base = 0x50041000,
diff --git a/configs/orangepi0.c b/configs/orangepi0.c
index a37e5d49..14f1a0e0 100644
--- a/configs/orangepi0.c
+++ b/configs/orangepi0.c
@@ -34,8 +34,8 @@ struct {
.debug_console = {
.address = 0x01c28000,
.size = 0x1000,
- .flags = JAILHOUSE_CON_TYPE_8250 |
- JAILHOUSE_CON_FLAG_MMIO,
+ .flags = JAILHOUSE_CON1_TYPE_8250 |
+ JAILHOUSE_CON1_FLAG_MMIO,
},
.platform_info = {
.pci_mmconfig_base = 0x2000000,
diff --git a/configs/qemu-vm.c b/configs/qemu-vm.c
index 04d0c947..0ce34eea 100644
--- a/configs/qemu-vm.c
+++ b/configs/qemu-vm.c
@@ -40,8 +40,8 @@ struct {
.debug_console = {
.address = 0x3f8,
/* .divider = 0x1, */
- .flags = JAILHOUSE_CON_TYPE_UART_X86 |
- JAILHOUSE_CON_FLAG_PIO,
+ .flags = JAILHOUSE_CON1_TYPE_UART_X86 |
+ JAILHOUSE_CON1_FLAG_PIO,
},
.platform_info = {
.pci_mmconfig_base = 0xb0000000,
diff --git a/configs/vexpress.c b/configs/vexpress.c
index 15202970..61c99b9b 100644
--- a/configs/vexpress.c
+++ b/configs/vexpress.c
@@ -31,8 +31,8 @@ struct {
.debug_console = {
.address = 0x1c090000,
.size = 0x1000,
- .flags = JAILHOUSE_CON_TYPE_PL011 |
- JAILHOUSE_CON_FLAG_MMIO,
+ .flags = JAILHOUSE_CON1_TYPE_PL011 |
+ JAILHOUSE_CON1_FLAG_MMIO,
},
.platform_info.arm = {
#ifdef CONFIG_ARM_GIC_V3
diff --git a/configs/zynqmp-zcu102.c b/configs/zynqmp-zcu102.c
index d236ae6f..120a73dc 100644
--- a/configs/zynqmp-zcu102.c
+++ b/configs/zynqmp-zcu102.c
@@ -34,8 +34,8 @@ struct {
.debug_console = {
.address = 0xff000000,
.size = 0x1000,
- .flags = JAILHOUSE_CON_TYPE_XUARTPS |
- JAILHOUSE_CON_FLAG_MMIO,
+ .flags = JAILHOUSE_CON1_TYPE_XUARTPS |
+ JAILHOUSE_CON1_FLAG_MMIO,
},
.platform_info = {
.pci_mmconfig_base = 0xfc000000,
diff --git a/driver/main.c b/driver/main.c
index e793d9d7..676794be 100644
--- a/driver/main.c
+++ b/driver/main.c
@@ -301,7 +301,7 @@ static int jailhouse_cmd_enable(struct jailhouse_system __user *arg)
}

#ifdef JAILHOUSE_BORROW_ROOT_PT
- if (CON_IS_MMIO(config->debug_console.flags)) {
+ if (CON1_IS_MMIO(config->debug_console.flags)) {
console = ioremap(config->debug_console.address,
config->debug_console.size);
if (!console) {
diff --git a/hypervisor/arch/arm-common/dbg-write.c b/hypervisor/arch/arm-common/dbg-write.c
index e88b18b9..e9e51a32 100644
--- a/hypervisor/arch/arm-common/dbg-write.c
+++ b/hypervisor/arch/arm-common/dbg-write.c
@@ -42,16 +42,16 @@ static void arm_uart_write(const char *msg)

void arch_dbg_write_init(void)
{
- unsigned char con_type = CON_TYPE(system_config->debug_console.flags);
+ unsigned char con_type = CON1_TYPE(system_config->debug_console.flags);

- if (!CON_IS_MMIO(system_config->debug_console.flags))
+ if (!CON1_IS_MMIO(system_config->debug_console.flags))
return;

- if (con_type == JAILHOUSE_CON_TYPE_PL011)
+ if (con_type == JAILHOUSE_CON1_TYPE_PL011)
uart = &uart_pl011_ops;
- else if (con_type == JAILHOUSE_CON_TYPE_8250)
+ else if (con_type == JAILHOUSE_CON1_TYPE_8250)
uart = &uart_8250_ops;
- else if (con_type == JAILHOUSE_CON_TYPE_XUARTPS)
+ else if (con_type == JAILHOUSE_CON1_TYPE_XUARTPS)
uart = &uart_xuartps_ops;

if (uart) {
diff --git a/hypervisor/arch/x86/dbg-write.c b/hypervisor/arch/x86/dbg-write.c
index 60e8adb4..30ddc908 100644
--- a/hypervisor/arch/x86/dbg-write.c
+++ b/hypervisor/arch/x86/dbg-write.c
@@ -17,13 +17,13 @@

void arch_dbg_write_init(void)
{
- unsigned char dbg_type = CON_TYPE(system_config->debug_console.flags);
+ unsigned char dbg_type = CON1_TYPE(system_config->debug_console.flags);

/* PIO / MMIO differentiation is done inside the driver code */
- if (dbg_type == JAILHOUSE_CON_TYPE_UART_X86) {
+ if (dbg_type == JAILHOUSE_CON1_TYPE_UART_X86) {
uart_init();
arch_dbg_write = uart_write;
- } else if (dbg_type == JAILHOUSE_CON_TYPE_VGA) {
+ } else if (dbg_type == JAILHOUSE_CON1_TYPE_VGA) {
vga_init();
arch_dbg_write = vga_write;
}
diff --git a/hypervisor/arch/x86/uart.c b/hypervisor/arch/x86/uart.c
index 736cf3b2..09b9e84f 100644
--- a/hypervisor/arch/x86/uart.c
+++ b/hypervisor/arch/x86/uart.c
@@ -57,7 +57,7 @@ void uart_init(void)
u32 flags = system_config->debug_console.flags;
u32 divider = system_config->debug_console.divider;

- if (CON_IS_MMIO(flags)) {
+ if (CON1_IS_MMIO(flags)) {
uart_reg_out = uart_mmio32_out;
uart_reg_in = uart_mmio32_in;
uart_base = (u64)hypervisor_header.debug_console_base;
diff --git a/hypervisor/include/jailhouse/cell-config.h b/hypervisor/include/jailhouse/cell-config.h
index 2f8c7cb9..80fa5a78 100644
--- a/hypervisor/include/jailhouse/cell-config.h
+++ b/hypervisor/include/jailhouse/cell-config.h
index 67ed63f4..a431550f 100644
--- a/hypervisor/paging.c
+++ b/hypervisor/paging.c
@@ -595,7 +595,7 @@ int paging_init(void)
if (err)
return err;

- if (CON_IS_MMIO(system_config->debug_console.flags)) {
+ if (CON1_IS_MMIO(system_config->debug_console.flags)) {
vaddr = (unsigned long)hypervisor_header.debug_console_base;
/* check if console overlaps remapping region */
if (vaddr + system_config->debug_console.size >= REMAP_BASE &&
diff --git a/tools/root-cell-config.c.tmpl b/tools/root-cell-config.c.tmpl
index b7be8b74..85c1834a 100644
--- a/tools/root-cell-config.c.tmpl
+++ b/tools/root-cell-config.c.tmpl
@@ -62,8 +62,8 @@ struct {
},
.debug_console = {
.address = 0x3f8,
- .flags = JAILHOUSE_CON_TYPE_UART_X86 |
- JAILHOUSE_CON_FLAG_PIO,
+ .flags = JAILHOUSE_CON1_TYPE_UART_X86 |
+ JAILHOUSE_CON1_FLAG_PIO,
},
.platform_info = {
.pci_mmconfig_base = ${hex(mmconfig.base)},
--
2.11.0

Ralf Ramsauer

unread,
Jan 25, 2017, 7:11:51 AM1/25/17
to Jan Kiszka, jailho...@googlegroups.com, Ralf Ramsauer
If the console is enabled. Don't write messages to the console page if
it is disabled. Otherwise we would leak information if we disable the
hypervisor.

Signed-off-by: Ralf Ramsauer <ralf.r...@oth-regensburg.de>
Reviewed-by: Jan Kiszka <jan.k...@siemens.com>
---
hypervisor/printk.c | 21 ++++++++++++++++++++-
1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/hypervisor/printk.c b/hypervisor/printk.c
index 117fe74c..3a550474 100644
--- a/hypervisor/printk.c
+++ b/hypervisor/printk.c
@@ -23,7 +23,26 @@ volatile struct jailhouse_console console __attribute__((section(".console")));

static DEFINE_SPINLOCK(printk_lock);

-#define console_write(msg) arch_dbg_write(msg)
+static void console_write(const char *msg)
+{
+ arch_dbg_write(msg);
+
+ if (!virtual_console)
+ return;
+
+ console.busy = true;
+ /* ensure the lock is visible prior to updates of the content */
+ memory_barrier();
+ while (*msg) {
+ console.content[console.tail % sizeof(console.content)] =
+ *msg++;
+ console.tail++;
+ }
+ /* ensure that all updates are committed before releasing the lock */
+ memory_barrier();
+ console.busy = false;
+}
+
#include "printk-core.c"

static void dbg_write_stub(const char *msg)
--
2.11.0

Ralf Ramsauer

unread,
Jan 25, 2017, 7:11:51 AM1/25/17
to Jan Kiszka, jailho...@googlegroups.com, Ralf Ramsauer
We're backing hypervisor pages with an empty page for the root cell.
This commit excludes the console page and allows RO access to it, but
only if the debug console of system configuration has the flag
JAILHOUSE_CON2_TYPE_ROOTPAGE set.

This flag is not activated by default and has to be set manually.

In this way, the Linux driver may easily read out the console without
any interactions with the hypervisor.

Signed-off-by: Ralf Ramsauer <ralf.r...@oth-regensburg.de>
Reviewed-by: Jan Kiszka <jan.k...@siemens.com>
---
hypervisor/include/jailhouse/printk.h | 1 +
hypervisor/printk.c | 1 +
hypervisor/setup.c | 12 ++++++++++++
3 files changed, 14 insertions(+)

diff --git a/hypervisor/include/jailhouse/printk.h b/hypervisor/include/jailhouse/printk.h
index f2bed6e2..e775e975 100644
--- a/hypervisor/include/jailhouse/printk.h
+++ b/hypervisor/include/jailhouse/printk.h
@@ -28,4 +28,5 @@ void __attribute__((format(printf, 1, 2))) panic_printk(const char *fmt, ...);
void arch_dbg_write_init(void);
extern void (*arch_dbg_write)(const char *msg);

+extern bool virtual_console;
extern volatile struct jailhouse_console console;
diff --git a/hypervisor/printk.c b/hypervisor/printk.c
index ca54b60d..117fe74c 100644
--- a/hypervisor/printk.c
+++ b/hypervisor/printk.c
@@ -18,6 +18,7 @@
#include <asm/bitops.h>
#include <asm/spinlock.h>

+bool virtual_console = false;
volatile struct jailhouse_console console __attribute__((section(".console")));

static DEFINE_SPINLOCK(printk_lock);
diff --git a/hypervisor/setup.c b/hypervisor/setup.c
index 8cfa4555..9b1c0c1d 100644
--- a/hypervisor/setup.c
+++ b/hypervisor/setup.c
--
2.11.0

Ralf Ramsauer

unread,
Jan 25, 2017, 7:11:51 AM1/25/17
to Jan Kiszka, jailho...@googlegroups.com, Ralf Ramsauer
Driver must know the location of the console_page. Use the
jailhouse_header for propagating the offset.

Signed-off-by: Ralf Ramsauer <ralf.r...@oth-regensburg.de>
Reviewed-by: Jan Kiszka <jan.k...@siemens.com>
---
hypervisor/include/jailhouse/header.h | 3 +++
hypervisor/setup.c | 1 +
2 files changed, 4 insertions(+)

diff --git a/hypervisor/include/jailhouse/header.h b/hypervisor/include/jailhouse/header.h
index dd852370..d1f1508b 100644
--- a/hypervisor/include/jailhouse/header.h
+++ b/hypervisor/include/jailhouse/header.h
@@ -53,6 +53,9 @@ struct jailhouse_header {
/** Entry point (arch_entry()).
* @note Filled at build time. */
int (*entry)(unsigned int);
+ /** Offset of the console page inside the hypervisor memory
+ * @note Filled at build time. */
+ unsigned long console_page;

/** Configured maximum logical CPU ID + 1.
* @note Filled by Linux loader driver before entry. */
diff --git a/hypervisor/setup.c b/hypervisor/setup.c
index 9b1c0c1d..3a8c1880 100644
--- a/hypervisor/setup.c
+++ b/hypervisor/setup.c
@@ -232,4 +232,5 @@ hypervisor_header = {
.core_size = (unsigned long)__page_pool - JAILHOUSE_BASE,
.percpu_size = sizeof(struct per_cpu),
.entry = arch_entry - JAILHOUSE_BASE,
+ .console_page = (unsigned long)&console - JAILHOUSE_BASE,
};
--
2.11.0

Ralf Ramsauer

unread,
Jan 25, 2017, 7:11:51 AM1/25/17
to Jan Kiszka, jailho...@googlegroups.com, Ralf Ramsauer
The console page is implemented as ring buffer, and will _not_ contain a
trailing \0 for string termination.

Signed-off-by: Ralf Ramsauer <ralf.r...@oth-regensburg.de>
---
hypervisor/hypervisor.lds.S | 9 +++++++++
hypervisor/include/jailhouse/cell-config.h | 6 ++++++
hypervisor/include/jailhouse/header.h | 8 ++++++++
hypervisor/include/jailhouse/printk.h | 2 ++
hypervisor/printk.c | 2 ++
5 files changed, 27 insertions(+)

diff --git a/hypervisor/hypervisor.lds.S b/hypervisor/hypervisor.lds.S
index 09b1a0b2..b0154b5a 100644
--- a/hypervisor/hypervisor.lds.S
+++ b/hypervisor/hypervisor.lds.S
@@ -35,6 +35,15 @@ SECTIONS
. = ALIGN(16);
.bss : { *(.bss) }

+ /* The console section shall only contain the hypervisor console. This
+ * section and the next section must be aligned to PAGE_SIZE, as we
+ * will map the console section, and only that section, as a whole page
+ * to the root cell. */
+
+
+ . = ALIGN(PAGE_SIZE);
+ .console : { *(.console) }
+
. = ALIGN(PAGE_SIZE);
__page_pool = .;

diff --git a/hypervisor/include/jailhouse/cell-config.h b/hypervisor/include/jailhouse/cell-config.h
index 80fa5a78..67b3319e 100644
--- a/hypervisor/include/jailhouse/cell-config.h
+++ b/hypervisor/include/jailhouse/cell-config.h
@@ -192,6 +192,12 @@ struct jailhouse_iommu {

#define CON1_IS_MMIO(flags) ((flags) & JAILHOUSE_CON1_FLAG_MMIO)

+/* Bits 16..19 are used to select the second console driver */
+#define JAILHOUSE_CON2_TYPE_ROOTPAGE 0x0100
+#define JAILHOUSE_CON2_TYPE_MASK 0x0f00
+
+#define CON2_TYPE(flags) ((flags) & JAILHOUSE_CON2_TYPE_MASK)
+
struct jailhouse_debug_console {
__u64 address;
__u32 size;
diff --git a/hypervisor/include/jailhouse/header.h b/hypervisor/include/jailhouse/header.h
index 4fe159c6..dd852370 100644
--- a/hypervisor/include/jailhouse/header.h
+++ b/hypervisor/include/jailhouse/header.h
@@ -24,6 +24,14 @@
*/
typedef int (*jailhouse_entry)(unsigned int);

+struct jailhouse_console {
+ unsigned int busy;
+ unsigned int tail;
+ /* current implementation requires the size of the content to be a
+ * power of two */
+ char content[2048];
+};
+
/**
* Hypervisor description.
* Located at the beginning of the hypervisor binary image and loaded by
diff --git a/hypervisor/include/jailhouse/printk.h b/hypervisor/include/jailhouse/printk.h
index a506c0fd..f2bed6e2 100644
--- a/hypervisor/include/jailhouse/printk.h
+++ b/hypervisor/include/jailhouse/printk.h
@@ -27,3 +27,5 @@ void __attribute__((format(printf, 1, 2))) panic_printk(const char *fmt, ...);

void arch_dbg_write_init(void);
extern void (*arch_dbg_write)(const char *msg);
+
+extern volatile struct jailhouse_console console;
diff --git a/hypervisor/printk.c b/hypervisor/printk.c
index e8f5ffe2..ca54b60d 100644
--- a/hypervisor/printk.c
+++ b/hypervisor/printk.c
@@ -18,6 +18,8 @@
#include <asm/bitops.h>
#include <asm/spinlock.h>

+volatile struct jailhouse_console console __attribute__((section(".console")));
+
static DEFINE_SPINLOCK(printk_lock);

#define console_write(msg) arch_dbg_write(msg)
--
2.11.0

Ralf Ramsauer

unread,
Jan 25, 2017, 7:11:51 AM1/25/17
to Jan Kiszka, jailho...@googlegroups.com, Ralf Ramsauer
Signed-off-by: Ralf Ramsauer <ralf.r...@oth-regensburg.de>
---
driver/main.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 97 insertions(+)

diff --git a/driver/main.c b/driver/main.c
index 98bb7d38..9045281e 100644
--- a/driver/main.c
+++ b/driver/main.c
@@ -71,6 +71,10 @@ MODULE_FIRMWARE(JAILHOUSE_FW_NAME);
#endif
MODULE_VERSION(JAILHOUSE_VERSION);

+struct console_state {
+ unsigned int head;
+};
+
DEFINE_MUTEX(jailhouse_lock);
bool jailhouse_enabled;

@@ -612,11 +616,104 @@ static long jailhouse_ioctl(struct file *file, unsigned int ioctl,
return err;
}

+static int jailhouse_console_open(struct inode *inode, struct file *file)
+{
+ struct console_state *user;
+
+ user = kzalloc(sizeof(struct console_state), GFP_KERNEL);
+ if (!user)
+ return -ENOMEM;
+
+ file->private_data = user;
+
+ return 0;
+}
+
+static int jailhouse_console_release(struct inode *inode, struct file *file)
+{
+ struct console_state *user = file->private_data;
+
+ kfree(user);
+
+ return 0;
+}
+
+static ssize_t jailhouse_console_read(struct file *file, char __user *out,
+ size_t size, loff_t *off)
+{
+ struct console_state *user = file->private_data;
+ char *content;
+ unsigned int miss;
+ int ret;
+
+ content = kmalloc(sizeof(console_page->content), GFP_KERNEL);
+ if (content == NULL)
+ return -ENOMEM;
+
+ /* wait for new data */
+ while (1) {
+ if (mutex_lock_interruptible(&jailhouse_lock) != 0) {
+ ret = -EINTR;
+ goto console_free_out;
+ }
+
+ ret = jailhouse_console_dump_delta(content, user->head, &miss);
+ "<missed %u bytes of console log>\n",
+ miss);
+ user->head += miss;
+ if (size < ret)
+ ret = size;
+ } else {
+ if (size < ret)
+ ret = size;
+ user->head += ret;
+ }
+
+ if (copy_to_user(out, content, ret))
+ ret = -EFAULT;
+
+console_free_out:
+ set_current_state(TASK_RUNNING);
+ kfree(content);
+ return ret;
+}
+
+
static const struct file_operations jailhouse_fops = {
.owner = THIS_MODULE,
.unlocked_ioctl = jailhouse_ioctl,
.compat_ioctl = jailhouse_ioctl,
.llseek = noop_llseek,
+ .open = jailhouse_console_open,
+ .release = jailhouse_console_release,
+ .read = jailhouse_console_read,
};

static struct miscdevice jailhouse_misc_dev = {
--
2.11.0

Ralf Ramsauer

unread,
Jan 25, 2017, 7:11:51 AM1/25/17
to Jan Kiszka, jailho...@googlegroups.com, Ralf Ramsauer
Signed-off-by: Ralf Ramsauer <ralf.r...@oth-regensburg.de>
---
Documentation/sysfs-entries.txt | 3 ++
TODO.md | 1 -
driver/main.c | 94 +++++++++++++++++++++++++++++++++++++++++
driver/main.h | 2 +
driver/sysfs.c | 20 +++++++++
5 files changed, 119 insertions(+), 1 deletion(-)

diff --git a/Documentation/sysfs-entries.txt b/Documentation/sysfs-entries.txt
index 6b483bba..58d1f5a2 100644
--- a/Documentation/sysfs-entries.txt
+++ b/Documentation/sysfs-entries.txt
@@ -5,6 +5,7 @@ The following sysfs entries are provided by the Jailhouse Linux driver. These
can be used for monitoring the state of the hypervisor and its cells.

/sys/devices/jailhouse
+|- console - hypervisor console (see [1])
|- enabled - 1 if Jailhouse is enabled, 0 otherwise
|- mem_pool_size - number of pages in hypervisor memory pool
|- mem_pool_used - used pages of hypervisor memory pool
@@ -30,3 +31,5 @@ may not reflect a fully consistent state. The existence and semantics of VM
exit reason values are architecture-dependent and may change in future
versions. In general statistics shall only be considered as a first hint when
analyzing cell behavior.
+
+[1] Documentation/debug-output.md
diff --git a/TODO.md b/TODO.md
index e452d306..4e93b1d7 100644
--- a/TODO.md
+++ b/TODO.md
@@ -89,7 +89,6 @@ Hardware error handling

Monitoring
- report error-triggering devices behind IOMMUs via sysfs
- - hypervisor console via debugfs?
- cell software watchdog via comm region messages
-> time out pending comm region messages and kill failing cells
(includes timeouts of unanswered shutdown requests)
diff --git a/driver/main.c b/driver/main.c
index 676794be..98bb7d38 100644
--- a/driver/main.c
+++ b/driver/main.c
+ unsigned int tail;
+
+ do {
+ /* spin while hypervisor is writing to console */
+ while (console_page->busy)
+ cpu_relax();
+ tail = console_page->tail;
+ rmb();
+
+ /* copy console page */
+ memcpy(dst, console_page, sizeof(struct jailhouse_console));
+ rmb();
+ } while (console_page->tail != tail || console_page->busy);
+}
+
static long get_max_cpus(u32 cpu_set_size,
const struct jailhouse_system __user *system_config)
{
@@ -182,6 +203,73 @@ static inline const char * jailhouse_get_fw_name(void)
#endif
}

+static int _jailhouse_console_dump_delta(struct jailhouse_console *console,
+ char *dst, unsigned int head,
+ unsigned int *miss)
+{
+ int ret;
+ unsigned int head_mod, tail_mod;
+ unsigned int delta, missed = 0;
+
+ /* we might underflow here intentionally */
+ delta = console->tail - head;
+
+ /* check if we have misses */
+ if (delta > sizeof(console->content)) {
+ missed = delta - sizeof(console->content);
+ head = console->tail - sizeof(console->content);
+ delta = sizeof(console->content);
+ }
+
+ head_mod = head % sizeof(console->content);
+ tail_mod = console->tail % sizeof(console->content);
+
+ if (head_mod + delta > sizeof(console->content)) {
+ ret = sizeof(console->content) - head_mod;
+ memcpy(dst, console->content + head_mod, ret);
+ delta -= ret;
+ memcpy(dst + ret, console->content, delta);
+ ret += delta;
+ } else {
+ ret = delta;
+ memcpy(dst, console->content + head_mod, delta);
+ }
+
+ if (miss)
+ *miss = missed;
+
+ return ret;
+}
+
+int jailhouse_console_dump_delta(char *dst, unsigned int head,
+ unsigned int *miss)
+{
+ int ret;
+ struct jailhouse_console *console;
+
+ if (!jailhouse_enabled)
+ return -EAGAIN;
+
+ if (!console_available)
+ return -EPERM;
+
+ console = kmalloc(sizeof(struct jailhouse_console), GFP_KERNEL);
+ if (console == NULL)
+ return -ENOMEM;
+
+ copy_console_page(console);
+ if (console->tail == head) {
+ ret = 0;
+ goto console_free_out;
+ }
+
+ ret = _jailhouse_console_dump_delta(console, dst, head, miss);
+
+console_free_out:
+ kfree(console);
+ return ret;
+}
+
/* See Documentation/bootstrap-interface.txt */
static int jailhouse_cmd_enable(struct jailhouse_system __user *arg)
{
@@ -273,6 +361,9 @@ static int jailhouse_cmd_enable(struct jailhouse_system __user *arg)
goto error_release_fw;
}

+ console_page = (struct jailhouse_console*)
+ (hypervisor_mem + header->console_page);
+
/* Copy hypervisor's binary image at beginning of the memory region
* and clear the rest to zero. */
memcpy(hypervisor_mem, hypervisor->data, hypervisor->size);
@@ -329,6 +420,9 @@ static int jailhouse_cmd_enable(struct jailhouse_system __user *arg)
}
#endif

+ console_available = CON2_TYPE(config->debug_console.flags) ==
+ JAILHOUSE_CON2_TYPE_ROOTPAGE;
+
err = jailhouse_cell_prepare_root(&config->root_cell);
if (err)
goto error_unmap;
diff --git a/driver/main.h b/driver/main.h
index e01ca5b5..00ceab4f 100644
--- a/driver/main.h
+++ b/driver/main.h
@@ -22,5 +22,7 @@ extern bool jailhouse_enabled;

void *jailhouse_ioremap(phys_addr_t phys, unsigned long virt,
unsigned long size);
+int jailhouse_console_dump_delta(char *dst, unsigned int head,
+ unsigned int *miss);

#endif /* !_JAILHOUSE_DRIVER_MAIN_H */
diff --git a/driver/sysfs.c b/driver/sysfs.c
index 8da1f766..6a01cbff 100644
--- a/driver/sysfs.c
+++ b/driver/sysfs.c
@@ -309,6 +309,24 @@ void jailhouse_sysfs_cell_delete(struct cell *cell)
kobject_put(&cell->kobj);
}

+static ssize_t console_show(struct device *dev, struct device_attribute *attr,
+ char *buffer)
+{
+ ssize_t ret;
+
+ if (mutex_lock_interruptible(&jailhouse_lock) != 0)
+ return -EINTR;
+
+ ret = jailhouse_console_dump_delta(buffer, 0, NULL);
+ /* don't return error if jailhouse is not enabled */
+ if (ret == -EAGAIN)
+ ret = 0;
+
+ mutex_unlock(&jailhouse_lock);
+
+ return ret;
+}
+
static ssize_t enabled_show(struct device *dev, struct device_attribute *attr,
char *buffer)
{
@@ -361,6 +379,7 @@ static ssize_t remap_pool_used_show(struct device *dev,
return info_show(dev, buffer, JAILHOUSE_INFO_REMAP_POOL_USED);
}

+static DEVICE_ATTR_RO(console);
static DEVICE_ATTR_RO(enabled);
static DEVICE_ATTR_RO(mem_pool_size);
static DEVICE_ATTR_RO(mem_pool_used);
@@ -368,6 +387,7 @@ static DEVICE_ATTR_RO(remap_pool_size);
static DEVICE_ATTR_RO(remap_pool_used);

static struct attribute *jailhouse_sysfs_entries[] = {
+ &dev_attr_console.attr,
&dev_attr_enabled.attr,
&dev_attr_mem_pool_size.attr,
&dev_attr_mem_pool_used.attr,
--
2.11.0

Ralf Ramsauer

unread,
Jan 25, 2017, 7:11:52 AM1/25/17
to Jan Kiszka, jailho...@googlegroups.com, Ralf Ramsauer
From now on, 'jailhouse enable' will dump the console if an error occurs
during initialisation of the hypervisor.

Signed-off-by: Ralf Ramsauer <ralf.r...@oth-regensburg.de>
Reviewed-by: Jan Kiszka <jan.k...@siemens.com>
---
tools/jailhouse.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/jailhouse.c b/tools/jailhouse.c
index 46f82553..54ff4b17 100644
--- a/tools/jailhouse.c
+++ b/tools/jailhouse.c
@@ -228,8 +228,10 @@ static int enable(int argc, char *argv[])
fd = open_dev();

err = ioctl(fd, JAILHOUSE_ENABLE, config);
- if (err)
+ if (err) {
perror("JAILHOUSE_ENABLE");
+ dump_console(fd, true);
+ }

close(fd);
free(config);
--
2.11.0

Ralf Ramsauer

unread,
Jan 25, 2017, 7:11:52 AM1/25/17
to Jan Kiszka, jailho...@googlegroups.com, Ralf Ramsauer
for all existing system configuration. Activate it in the root cell
template as well.

Signed-off-by: Ralf Ramsauer <ralf.r...@oth-regensburg.de>
---
configs/amd-seattle.c | 3 ++-
configs/bananapi.c | 3 ++-
configs/f2a88xm-hd3.c | 3 ++-
configs/foundation-v8.c | 3 ++-
configs/hikey.c | 3 ++-
configs/imb-a180.c | 3 ++-
configs/jetson-tk1.c | 3 ++-
configs/jetson-tx1.c | 3 ++-
configs/orangepi0.c | 3 ++-
configs/qemu-vm.c | 3 ++-
configs/vexpress.c | 3 ++-
configs/zynqmp-zcu102.c | 3 ++-
tools/root-cell-config.c.tmpl | 3 ++-
13 files changed, 26 insertions(+), 13 deletions(-)

diff --git a/configs/amd-seattle.c b/configs/amd-seattle.c
index 66e3fdcd..81bc6208 100644
--- a/configs/amd-seattle.c
+++ b/configs/amd-seattle.c
@@ -33,7 +33,8 @@ struct {
.address = 0xe1010000,
.size = 0x1000,
.flags = JAILHOUSE_CON1_TYPE_PL011 |
- JAILHOUSE_CON1_FLAG_MMIO,
+ JAILHOUSE_CON1_FLAG_MMIO |
+ JAILHOUSE_CON2_TYPE_ROOTPAGE,
},
.platform_info = {
.pci_mmconfig_base = 0xf0000000,
diff --git a/configs/bananapi.c b/configs/bananapi.c
index 484d808a..f289a3e3 100644
--- a/configs/bananapi.c
+++ b/configs/bananapi.c
@@ -38,7 +38,8 @@ struct {
/* .gate_nr = 16 */
/* .divider = 0x0d, */
.flags = JAILHOUSE_CON1_TYPE_8250 |
- JAILHOUSE_CON1_FLAG_MMIO,
+ JAILHOUSE_CON1_FLAG_MMIO |
+ JAILHOUSE_CON2_TYPE_ROOTPAGE,
},
.platform_info = {
.pci_mmconfig_base = 0x2000000,
diff --git a/configs/f2a88xm-hd3.c b/configs/f2a88xm-hd3.c
index 62a9117b..33973bcf 100644
--- a/configs/f2a88xm-hd3.c
+++ b/configs/f2a88xm-hd3.c
@@ -42,7 +42,8 @@ struct {
.address = 0x3f8,
/* .divider = 0x1, */
.flags = JAILHOUSE_CON1_TYPE_UART_X86 |
- JAILHOUSE_CON1_FLAG_PIO,
+ JAILHOUSE_CON1_FLAG_PIO |
+ JAILHOUSE_CON2_TYPE_ROOTPAGE,
},
.platform_info = {
.pci_mmconfig_base = 0xe0000000,
diff --git a/configs/foundation-v8.c b/configs/foundation-v8.c
index c4112004..56e4816d 100644
--- a/configs/foundation-v8.c
+++ b/configs/foundation-v8.c
@@ -32,7 +32,8 @@ struct {
.address = 0x1c090000,
.size = 0x1000,
.flags = JAILHOUSE_CON1_TYPE_PL011 |
- JAILHOUSE_CON1_FLAG_MMIO,
+ JAILHOUSE_CON1_FLAG_MMIO |
+ JAILHOUSE_CON2_TYPE_ROOTPAGE,
},
.platform_info.arm = {
#ifdef CONFIG_ARM_GIC_V3
diff --git a/configs/hikey.c b/configs/hikey.c
index a739222b..2c859a09 100644
--- a/configs/hikey.c
+++ b/configs/hikey.c
@@ -33,7 +33,8 @@ struct {
.address = 0xf7113000,
.size = 0x1000,
.flags = JAILHOUSE_CON1_TYPE_PL011 |
- JAILHOUSE_CON1_FLAG_MMIO,
+ JAILHOUSE_CON1_FLAG_MMIO |
+ JAILHOUSE_CON2_TYPE_ROOTPAGE,
},
.platform_info = {
.pci_mmconfig_base = 0xf6000000,
diff --git a/configs/imb-a180.c b/configs/imb-a180.c
index fa36d791..a6b9012c 100644
--- a/configs/imb-a180.c
+++ b/configs/imb-a180.c
@@ -41,7 +41,8 @@ struct {
.address = 0x3f8,
/* .divider = 0x1, */
.flags = JAILHOUSE_CON1_TYPE_UART_X86 |
- JAILHOUSE_CON1_FLAG_PIO,
+ JAILHOUSE_CON1_FLAG_PIO |
+ JAILHOUSE_CON2_TYPE_ROOTPAGE,
},
.platform_info = {
.pci_mmconfig_base = 0xe0000000,
diff --git a/configs/jetson-tk1.c b/configs/jetson-tk1.c
index d6713405..2cdb4924 100644
--- a/configs/jetson-tk1.c
+++ b/configs/jetson-tk1.c
@@ -41,7 +41,8 @@ struct {
/* .gate_nr = (65 % 32), */
/* .divider = 0xdd, */
.flags = JAILHOUSE_CON1_TYPE_8250 |
- JAILHOUSE_CON1_FLAG_MMIO,
+ JAILHOUSE_CON1_FLAG_MMIO |
+ JAILHOUSE_CON2_TYPE_ROOTPAGE,
},
.platform_info = {
.pci_mmconfig_base = 0x48000000,
diff --git a/configs/jetson-tx1.c b/configs/jetson-tx1.c
index af1f5577..f47ebfd4 100644
--- a/configs/jetson-tx1.c
+++ b/configs/jetson-tx1.c
@@ -35,7 +35,8 @@ struct {
.address = 0x70006000,
.size = 0x0040,
.flags = JAILHOUSE_CON1_TYPE_8250 |
- JAILHOUSE_CON1_FLAG_MMIO,
+ JAILHOUSE_CON1_FLAG_MMIO |
+ JAILHOUSE_CON2_TYPE_ROOTPAGE,
},
.platform_info.arm = {
.gicd_base = 0x50041000,
diff --git a/configs/orangepi0.c b/configs/orangepi0.c
index 14f1a0e0..ae1da186 100644
--- a/configs/orangepi0.c
+++ b/configs/orangepi0.c
@@ -35,7 +35,8 @@ struct {
.address = 0x01c28000,
.size = 0x1000,
.flags = JAILHOUSE_CON1_TYPE_8250 |
- JAILHOUSE_CON1_FLAG_MMIO,
+ JAILHOUSE_CON1_FLAG_MMIO |
+ JAILHOUSE_CON2_TYPE_ROOTPAGE,
},
.platform_info = {
.pci_mmconfig_base = 0x2000000,
diff --git a/configs/qemu-vm.c b/configs/qemu-vm.c
index 0ce34eea..f091834e 100644
--- a/configs/qemu-vm.c
+++ b/configs/qemu-vm.c
@@ -41,7 +41,8 @@ struct {
.address = 0x3f8,
/* .divider = 0x1, */
.flags = JAILHOUSE_CON1_TYPE_UART_X86 |
- JAILHOUSE_CON1_FLAG_PIO,
+ JAILHOUSE_CON1_FLAG_PIO |
+ JAILHOUSE_CON2_TYPE_ROOTPAGE,
},
.platform_info = {
.pci_mmconfig_base = 0xb0000000,
diff --git a/configs/vexpress.c b/configs/vexpress.c
index 61c99b9b..065db656 100644
--- a/configs/vexpress.c
+++ b/configs/vexpress.c
@@ -32,7 +32,8 @@ struct {
.address = 0x1c090000,
.size = 0x1000,
.flags = JAILHOUSE_CON1_TYPE_PL011 |
- JAILHOUSE_CON1_FLAG_MMIO,
+ JAILHOUSE_CON1_FLAG_MMIO |
+ JAILHOUSE_CON2_TYPE_ROOTPAGE,
},
.platform_info.arm = {
#ifdef CONFIG_ARM_GIC_V3
diff --git a/configs/zynqmp-zcu102.c b/configs/zynqmp-zcu102.c
index 120a73dc..458fc24f 100644
--- a/configs/zynqmp-zcu102.c
+++ b/configs/zynqmp-zcu102.c
@@ -35,7 +35,8 @@ struct {
.address = 0xff000000,
.size = 0x1000,
.flags = JAILHOUSE_CON1_TYPE_XUARTPS |
- JAILHOUSE_CON1_FLAG_MMIO,
+ JAILHOUSE_CON1_FLAG_MMIO |
+ JAILHOUSE_CON2_TYPE_ROOTPAGE,
},
.platform_info = {
.pci_mmconfig_base = 0xfc000000,
diff --git a/tools/root-cell-config.c.tmpl b/tools/root-cell-config.c.tmpl
index 85c1834a..11956b74 100644
--- a/tools/root-cell-config.c.tmpl
+++ b/tools/root-cell-config.c.tmpl
@@ -63,7 +63,8 @@ struct {
.debug_console = {
.address = 0x3f8,
.flags = JAILHOUSE_CON1_TYPE_UART_X86 |
- JAILHOUSE_CON1_FLAG_PIO,
+ JAILHOUSE_CON1_FLAG_PIO |
+ JAILHOUSE_CON2_TYPE_ROOTPAGE,

Ralf Ramsauer

unread,
Jan 25, 2017, 7:11:52 AM1/25/17
to Jan Kiszka, jailho...@googlegroups.com, Ralf Ramsauer
Signed-off-by: Ralf Ramsauer <ralf.r...@oth-regensburg.de>
Reviewed-by: Jan Kiszka <jan.k...@siemens.com>
---
Documentation/debug-output.md | 13 +++++++++++++
1 file changed, 13 insertions(+)

diff --git a/Documentation/debug-output.md b/Documentation/debug-output.md
index 6367fbcd..5d0610a1 100644
--- a/Documentation/debug-output.md
+++ b/Documentation/debug-output.md
@@ -84,6 +84,19 @@ Example configuration for disabled debug output (architecture independent):
}
```

+Hypervisor Console via sysfs
+----------------------------
+
+If the debug console of root cell has the flag JAILHOUSE_CON2_TYPE_ROOTPAGE
+set, the hypervisor console is available through
+/sys/devices/jailhouse/console. Continuous reading of the hypervisor console
+is available through /dev/jailhouse.
+
+Example
+```
+cat /dev/jailhouse
+```
+
Inmates
-------

--
2.11.0

Ralf Ramsauer

unread,
Jan 25, 2017, 7:11:52 AM1/25/17
to Jan Kiszka, jailho...@googlegroups.com, Ralf Ramsauer
This allows us to dump messages if the hypervisor failed to start or the
last few messages after the hypervisor was disabled. This needs some
special treatment, as the hypervisor_memory region is unmapped after
failures.

Hook into disable an failure paths and copy the page to a dedicated
variable. Indicate with a boolean flag that data is present.

Signed-off-by: Ralf Ramsauer <ralf.r...@oth-regensburg.de>
---
driver/main.c | 47 ++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 46 insertions(+), 1 deletion(-)

diff --git a/driver/main.c b/driver/main.c
index 9045281e..8ff69e19 100644
--- a/driver/main.c
+++ b/driver/main.c
@@ -73,6 +73,7 @@ MODULE_VERSION(JAILHOUSE_VERSION);

struct console_state {
unsigned int head;
+ unsigned int last_console_id;
};

DEFINE_MUTEX(jailhouse_lock);
@@ -86,6 +87,24 @@ static int error_code;
static struct jailhouse_console* volatile console_page;
static bool console_available;

+/* last_console contains three members:
+ * - valid: indicates if content in the page member is present
+ * - id: hint for the consumer if it already consumed the content
+ * - page: actual content
+ *
+ * Those members are updated in following cases:
+ * - on disabling the hypervisor to print last messages
+ * - on failures when enabling the hypervisor
+ *
+ * We need this structure, as in those cases the hypervisor memory gets
+ * unmapped.
+ */
+static struct {
+ bool valid;
+ unsigned int id;
+ struct jailhouse_console page;
+} last_console;
+
#ifdef CONFIG_X86
bool jailhouse_use_vmcall;

@@ -116,6 +135,16 @@ static void copy_console_page(struct jailhouse_console *dst)
} while (console_page->tail != tail || console_page->busy);
}

+static inline void update_last_console(void)
+{
+ if (!console_available)
+ return;
+
+ copy_console_page(&last_console.page);
+ last_console.id++;
+ last_console.valid = true;
+}
+
static long get_max_cpus(u32 cpu_set_size,
const struct jailhouse_system __user *system_config)
{
@@ -367,6 +396,7 @@ static int jailhouse_cmd_enable(struct jailhouse_system __user *arg)

console_page = (struct jailhouse_console*)
(hypervisor_mem + header->console_page);
+ last_console.valid = false;

/* Copy hypervisor's binary image at beginning of the memory region
* and clear the rest to zero. */
@@ -468,6 +498,7 @@ static int jailhouse_cmd_enable(struct jailhouse_system __user *arg)
return 0;

error_free_cell:
+ update_last_console();
jailhouse_cell_delete_root();

error_unmap:
@@ -567,6 +598,8 @@ static int jailhouse_cmd_disable(void)
if (err)
goto unlock_out;

+ update_last_console();
+
vunmap(hypervisor_mem);

jailhouse_cell_delete_root();
@@ -657,7 +690,19 @@ static ssize_t jailhouse_console_read(struct file *file, char __user *out,
goto console_free_out;
}

- ret = jailhouse_console_dump_delta(content, user->head, &miss);
+ if (last_console.id != user->last_console_id &&
+ last_console.valid) {
+ ret = _jailhouse_console_dump_delta(&last_console.page,
+ content,
+ user->head,
+ &miss);
+ if (!ret)
+ user->last_console_id =
+ last_console.id;
+ } else {
+ ret = jailhouse_console_dump_delta(content, user->head,
+ &miss);
+ }

mutex_unlock(&jailhouse_lock);

--
2.11.0

Ralf Ramsauer

unread,
Jan 25, 2017, 7:11:52 AM1/25/17
to Jan Kiszka, jailho...@googlegroups.com, Ralf Ramsauer
Similar to other tools, like 'tail -f', allow 'jailhosue console' to
follow the console output when jailhouse gets enabled.

'jailhouse console' will print all available console log of the
hypervisor.

'jailhouse console -f' will print all available console log of the
hypervisor and append new data as it arrives.

Use this patch to remove a redundant '|' in the usage string of the
tool.

Signed-off-by: Ralf Ramsauer <ralf.r...@oth-regensburg.de>
---
tools/jailhouse.c | 43 ++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 42 insertions(+), 1 deletion(-)

diff --git a/tools/jailhouse.c b/tools/jailhouse.c
index 84e5fe3d..46f82553 100644
--- a/tools/jailhouse.c
+++ b/tools/jailhouse.c
@@ -60,10 +60,11 @@ static void __attribute__((noreturn)) help(char *prog, int exit_status)
{
const struct extension *ext;

- printf("Usage: %s { COMMAND | --help || --version }\n"
+ printf("Usage: %s { COMMAND | --help | --version }\n"
"\nAvailable commands:\n"
" enable SYSCONFIG\n"
" disable\n"
+ " console [-f | --follow]\n"
" cell create CELLCONFIG\n"
" cell list\n"
" cell load { ID | [--name] NAME } "
@@ -79,6 +80,24 @@ static void __attribute__((noreturn)) help(char *prog, int exit_status)
exit(exit_status);
}

+static int dump_console(int fd, bool non_block)
+{
+ int ret;
+ char buffer[128];
+
+ if (non_block && fcntl(fd, F_SETFL, O_NONBLOCK) == -1)
+ return -errno;
+
+ do {
+ ret = read(fd, buffer, sizeof(buffer));
+ if (ret < 0)
+ break;
+ ret = write(STDOUT_FILENO, buffer, ret);
+ } while (ret > 0);
+
+ return ret;
+}
+
static void call_extension_script(const char *cmd, int argc, char *argv[])
{
const struct extension *ext;
@@ -504,6 +523,26 @@ static int cell_management(int argc, char *argv[])
return err;
}

+static int console(int argc, char *argv[])
+{
+ int fd;
+ ssize_t ret;
+ bool non_block = true;
+
+ if (argc == 3) {
+ if (match_opt(argv[2], "-f", "--follow"))
+ non_block = false;
+ else
+ help(argv[0], 1);
+ }
+
+ fd = open_dev();
+ ret = dump_console(fd, non_block);
+ close(fd);
+
+ return ret;
+}
+
int main(int argc, char *argv[])
{
int fd;
@@ -522,6 +561,8 @@ int main(int argc, char *argv[])
close(fd);
} else if (strcmp(argv[1], "cell") == 0) {
err = cell_management(argc, argv);
+ } else if (strcmp(argv[1], "console") == 0) {
+ err = console(argc, argv);
} else if (strcmp(argv[1], "config") == 0 ||
strcmp(argv[1], "hardware") == 0) {
call_extension_script(argv[1], argc, argv);
--
2.11.0

Jan Kiszka

unread,
Jan 28, 2017, 7:45:52 AM1/28/17
to Ralf Ramsauer, jailho...@googlegroups.com
Thanks, applied to next with two minor adjustment (comment on
console.busy and "__jailhouse_console_dump_delta").

Jan

Henning Schild

unread,
Apr 11, 2017, 9:26:36 AM4/11/17
to Ralf Ramsauer, Jan Kiszka, jailho...@googlegroups.com
Hey guys,

i backported my gcov patches to a jailhouse version before this commit
(4adf31d6f88cf8df6e47675ccc7fa446b00b9284) to test them on my ARM
target.
Turns out this patch makes the hypervisor firmware image grow by .bss
and .console, that is around 30kB of zeros on arm32.
check "objdump -x hypervisor/hypervisor.o" and "hexdump -C
hypervisor/jailhouse.bin | tail -5"

Maybe the console page can become part of .bss or at least can be
like .bss in the sense that all those zeros do not need to be part of
the binary. But i am actually not sure whether anyone really cares
about 30kB, even MBs would not be a problem really.

In addition the patch introduced an inconsitency with
Documentation/memory-layout.txt.

Henning

Am Wed, 25 Jan 2017 13:11:37 +0100
schrieb Ralf Ramsauer <ralf.r...@oth-regensburg.de>:

Jan Kiszka

unread,
Apr 11, 2017, 9:34:47 AM4/11/17
to Henning Schild, Ralf Ramsauer, jailho...@googlegroups.com
On 2017-04-11 15:27, Henning Schild wrote:
> Hey guys,
>
> i backported my gcov patches to a jailhouse version before this commit
> (4adf31d6f88cf8df6e47675ccc7fa446b00b9284) to test them on my ARM
> target.
> Turns out this patch makes the hypervisor firmware image grow by .bss
> and .console, that is around 30kB of zeros on arm32.
> check "objdump -x hypervisor/hypervisor.o" and "hexdump -C
> hypervisor/jailhouse.bin | tail -5"

Good catch.

>
> Maybe the console page can become part of .bss or at least can be
> like .bss in the sense that all those zeros do not need to be part of
> the binary. But i am actually not sure whether anyone really cares
> about 30kB, even MBs would not be a problem really.

Well, we never know how .bss may grow. As long as we already zero-out
all hypervisor memory during loading, I don't see a point in loading zeros.

Jan
--
Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Corporate Competence Center Embedded Linux

Ralf Ramsauer

unread,
Apr 11, 2017, 10:09:17 AM4/11/17
to Jan Kiszka, Henning Schild, jailho...@googlegroups.com
On 04/11/2017 03:34 PM, Jan Kiszka wrote:
> On 2017-04-11 15:27, Henning Schild wrote:
>> Hey guys,
>>
>> i backported my gcov patches to a jailhouse version before this commit
>> (4adf31d6f88cf8df6e47675ccc7fa446b00b9284) to test them on my ARM
>> target.
>> Turns out this patch makes the hypervisor firmware image grow by .bss
>> and .console, that is around 30kB of zeros on arm32.
>> check "objdump -x hypervisor/hypervisor.o" and "hexdump -C
>> hypervisor/jailhouse.bin | tail -5"
>
> Good catch.
In deed... Ok -- but I hope this patch is not related to your issue :)
>
>>
>> Maybe the console page can become part of .bss or at least can be
>> like .bss in the sense that all those zeros do not need to be part of
>> the binary. But i am actually not sure whether anyone really cares
>> about 30kB, even MBs would not be a problem really.
>
> Well, we never know how .bss may grow. As long as we already zero-out
> all hypervisor memory during loading, I don't see a point in loading zeros.
Yes, in the end of the day it doesn't really matter but I prefer the
smaller size as well.

It could also be mitigated if we place the .console section before the
.bss. Then only .console will be initialised and we waste one page
instead of 30kB.

Why is .console actually after ARCH_SECTIONS? I remember some off-list
discussions why we chose to place .console at this very location, but I
don't remember details... There must have been some reasons...
>
> Jan
>
>>
>> In addition the patch introduced an inconsitency with
>> Documentation/memory-layout.txt.
I'll follow up with a patch. Thanks!

Ralf

Henning Schild

unread,
Apr 11, 2017, 11:45:33 AM4/11/17
to Ralf Ramsauer, Jan Kiszka, jailho...@googlegroups.com
Am Tue, 11 Apr 2017 16:09:16 +0200
schrieb Ralf Ramsauer <ralf.r...@oth-regensburg.de>:

> On 04/11/2017 03:34 PM, Jan Kiszka wrote:
> > On 2017-04-11 15:27, Henning Schild wrote:
> >> Hey guys,
> >>
> >> i backported my gcov patches to a jailhouse version before this
> >> commit (4adf31d6f88cf8df6e47675ccc7fa446b00b9284) to test them on
> >> my ARM target.
> >> Turns out this patch makes the hypervisor firmware image grow
> >> by .bss and .console, that is around 30kB of zeros on arm32.
> >> check "objdump -x hypervisor/hypervisor.o" and "hexdump -C
> >> hypervisor/jailhouse.bin | tail -5"
> >
> > Good catch.
> In deed... Ok -- but I hope this patch is not related to your issue :)

No, it actually helped me find another issue ;).

Henning
Reply all
Reply to author
Forward
0 new messages