On 11/21/2016 11:05 PM, Jan Kiszka wrote:
> On 2016-11-21 14:04, Ralf Ramsauer wrote:
>> This patch exchanges the type of the debug_console.
>>
>> So far, the jailhouse debug console has been a simple
>> jailhouse_memory_region. This makes it hard to differentiate between
>> different kinds of debug outputs.
>>
>> This patch introduces the new struct jailhouse_debug_console which makes
>> the debug_console much more extensible.
>>
>> Signed-off-by: Ralf Ramsauer <[email protected]>
>> ---
>> Documentation/vga-console.txt | 4 ++--
>> configs/amd-seattle.c | 6 ++++--
>> configs/bananapi.c | 6 ++++--
>> configs/f2a88xm-hd3.c | 5 ++++-
>> configs/foundation-v8.c | 6 ++++--
>> configs/h87i.c | 5 ++++-
>> configs/hikey.c | 6 ++++--
>> configs/imb-a180.c | 5 ++++-
>> configs/jetson-tk1.c | 6 ++++--
>> configs/qemu-vm.c | 5 ++++-
>> configs/vexpress.c | 6 ++++--
>> driver/main.c | 6 +++---
>> hypervisor/arch/arm64/asm-defines.c | 2 +-
>> hypervisor/arch/x86/uart.c | 12 +++++++-----
>> hypervisor/arch/x86/vga.c | 2 +-
>> hypervisor/include/jailhouse/cell-config.h | 26 +++++++++++++++++++++++++-
>> hypervisor/paging.c | 4 ++--
>> tools/root-cell-config.c.tmpl | 4 +++-
>> 18 files changed, 84 insertions(+), 32 deletions(-)
>>
>> diff --git a/Documentation/vga-console.txt b/Documentation/vga-console.txt
>> index 64506fadfd5f..3157aef01b6d 100644
>> --- a/Documentation/vga-console.txt
>> +++ b/Documentation/vga-console.txt
>> @@ -18,9 +18,9 @@ Usage
>> Add the following to the header section of your root cell's config:
>>
>> .debug_console = {
>> - .phys_start = 0xb8000,
>> + .address = 0xb8000,
>> .size = 0x1000,
>> - .flags = JAILHOUSE_MEM_IO,
>> + .flags = JAILHOUSE_DBG_TYPE_VGA | JAILHOUSE_DBG_FLAG_MMIO,
>> },
>>
>> Boot using the following kernel parameters:
>> diff --git a/configs/amd-seattle.c b/configs/amd-seattle.c
>> index 7298bec0c34f..1b70d80103af 100644
>> --- a/configs/amd-seattle.c
>> +++ b/configs/amd-seattle.c
>> @@ -28,9 +28,11 @@ struct {
>> .size = 0x4000000,
>> },
>> .debug_console = {
>> - .phys_start = 0xe1010000,
>> + .address = 0xe1010000,
>> .size = 0x1000,
>> - .flags = JAILHOUSE_MEM_IO,
>> + .flags = JAILHOUSE_DBG_TYPE_UART_ARM |
>> + JAILHOUSE_DBG_FLAG_MMIO |
>> + JAILHOUSE_DBG_FLAG_REG_WIDTH_32,
>
> Let's limit explicit register distance specifications to default
> deviations. 32 bit is the default for MMIO.
>
>> },
>> .platform_info.arm = {
>> .gicd_base = 0xe1110000,
>> diff --git a/configs/bananapi.c b/configs/bananapi.c
>> index 6f1103faf4fd..95424588f814 100644
>> --- a/configs/bananapi.c
>> +++ b/configs/bananapi.c
>> @@ -30,9 +30,11 @@ struct {
>> .size = 0x4000000,
>> },
>> .debug_console = {
>> - .phys_start = 0x01c28000,
>> + .address = 0x01c28000,
>> .size = 0x1000,
>> - .flags = JAILHOUSE_MEM_IO,
>> + .flags = JAILHOUSE_DBG_TYPE_UART_ARM |
>> + JAILHOUSE_DBG_FLAG_MMIO |
>> + JAILHOUSE_DBG_FLAG_REG_WIDTH_32,
>> },
>> .platform_info.arm = {
>> .gicd_base = 0x01c81000,
>> diff --git a/configs/f2a88xm-hd3.c b/configs/f2a88xm-hd3.c
>> index b7926bc41222..ba1e2f0d8b85 100644
>> --- a/configs/f2a88xm-hd3.c
>> +++ b/configs/f2a88xm-hd3.c
>> @@ -38,7 +38,10 @@ struct {
>> .size = 0x4000000,
>> },
>> .debug_console = {
>> - .phys_start = 0x3f8,
>> + .address = 0x3f8,
>> + .flags = JAILHOUSE_DBG_TYPE_UART_X86 |
>> + JAILHOUSE_DBG_FLAG_PIO |
>> + JAILHOUSE_DBG_FLAG_REG_WIDTH_8,
>
> ...and 8 bit for PIO.
>
>> },
>> .platform_info.x86 = {
>> .mmconfig_base = 0xe0000000,
>> diff --git a/configs/foundation-v8.c b/configs/foundation-v8.c
>> index a7f402169e16..479021fe0167 100644
>> --- a/configs/foundation-v8.c
>> +++ b/configs/foundation-v8.c
>> @@ -28,9 +28,11 @@ struct {
>> .size = 0x4000000,
>> },
>> .debug_console = {
>> - .phys_start = 0x1c090000,
>> + .address = 0x1c090000,
>> .size = 0x1000,
>> - .flags = JAILHOUSE_MEM_IO,
>> + .flags = JAILHOUSE_DBG_TYPE_UART_ARM |
>> + JAILHOUSE_DBG_FLAG_MMIO |
>> + JAILHOUSE_DBG_FLAG_REG_WIDTH_32,
>> },
>> .platform_info.arm = {
>> #ifdef CONFIG_ARM_GIC_V3
>> diff --git a/configs/h87i.c b/configs/h87i.c
>> index 7f0407cc9ca9..1cbdc33113dc 100644
>> --- a/configs/h87i.c
>> +++ b/configs/h87i.c
>> @@ -33,7 +33,10 @@ struct {
>> .size = 0x4000000,
>> },
>> .debug_console = {
>> - .phys_start = 0xe010,
>> + .address = 0xe010,
>> + .flags = JAILHOUSE_DBG_TYPE_UART_X86 |
>> + JAILHOUSE_DBG_FLAG_PIO |
>> + JAILHOUSE_DBG_FLAG_REG_WIDTH_8,
>> },
>> .platform_info.x86 = {
>> .mmconfig_base = 0xf8000000,
>> diff --git a/configs/hikey.c b/configs/hikey.c
>> index 3c16c25a5f38..8da01d55a2b8 100644
>> --- a/configs/hikey.c
>> +++ b/configs/hikey.c
>> @@ -28,9 +28,11 @@ struct {
>> .size = 0x04000000,
>> },
>> .debug_console = {
>> - .phys_start = 0xf7113000,
>> + .address = 0xf7113000,
>> .size = 0x1000,
>> - .flags = JAILHOUSE_MEM_IO,
>> + .flags = JAILHOUSE_DBG_TYPE_UART_ARM |
>> + JAILHOUSE_DBG_FLAG_MMIO |
>> + JAILHOUSE_DBG_FLAG_REG_WIDTH_32,
>> },
>> .platform_info.arm = {
>> .gicd_base = 0xf6801000,
>> diff --git a/configs/imb-a180.c b/configs/imb-a180.c
>> index 0b23dae7e35c..f944fe35a6d8 100644
>> --- a/configs/imb-a180.c
>> +++ b/configs/imb-a180.c
>> @@ -37,7 +37,10 @@ struct {
>> .size = 0x4000000,
>> },
>> .debug_console = {
>> - .phys_start = 0x3f8,
>> + .address = 0x3f8,
>> + .flags = JAILHOUSE_DBG_TYPE_UART_X86 |
>> + JAILHOUSE_DBG_FLAG_PIO |
>> + JAILHOUSE_DBG_FLAG_REG_WIDTH_8,
>> },
>> .platform_info.x86 = {
>> .mmconfig_base = 0xe0000000,
>> diff --git a/configs/jetson-tk1.c b/configs/jetson-tk1.c
>> index 37fc46ea573d..cc24ad0cc05b 100644
>> --- a/configs/jetson-tk1.c
>> +++ b/configs/jetson-tk1.c
>> @@ -33,9 +33,11 @@ struct {
>> .size = 0x4000000 - 0x100000, /* -1MB (PSCI) */
>> },
>> .debug_console = {
>> - .phys_start = 0x70006000,
>> + .address = 0x70006000,
>> .size = 0x1000,
>> - .flags = JAILHOUSE_MEM_IO,
>> + .flags = JAILHOUSE_DBG_TYPE_UART_ARM |
>> + JAILHOUSE_DBG_FLAG_MMIO |
>> + JAILHOUSE_DBG_FLAG_REG_WIDTH_32,
>> },
>> .platform_info.arm = {
>> .gicd_base = 0x50041000,
>> diff --git a/configs/qemu-vm.c b/configs/qemu-vm.c
>> index 5f5b0b6a1852..532aa3d07720 100644
>> --- a/configs/qemu-vm.c
>> +++ b/configs/qemu-vm.c
>> @@ -47,7 +47,10 @@ struct {
>> .size = 0x600000,
>> },
>> .debug_console = {
>> - .phys_start = 0x3f8,
>> + .address = 0x3f8,
>> + .flags = JAILHOUSE_DBG_TYPE_UART_X86 |
>> + JAILHOUSE_DBG_FLAG_PIO |
>> + JAILHOUSE_DBG_FLAG_REG_WIDTH_8,
>> },
>> .platform_info.x86 = {
>> .mmconfig_base = 0xb0000000,
>> diff --git a/configs/vexpress.c b/configs/vexpress.c
>> index 319bb58e7977..b407ca6cfe79 100644
>> --- a/configs/vexpress.c
>> +++ b/configs/vexpress.c
>> @@ -28,9 +28,11 @@ struct {
>> .size = 0x4000000,
>> },
>> .debug_console = {
>> - .phys_start = 0x1c090000,
>> + .address = 0x1c090000,
>> .size = 0x1000,
>> - .flags = JAILHOUSE_MEM_IO,
>> + .flags = JAILHOUSE_DBG_TYPE_UART_ARM |
>> + JAILHOUSE_DBG_FLAG_MMIO |
>> + JAILHOUSE_DBG_FLAG_REG_WIDTH_32,
>> },
>> .platform_info.arm = {
>> #ifdef CONFIG_ARM_GIC_V3
>> diff --git a/driver/main.c b/driver/main.c
>> index d6834ef507d2..15294f954139 100644
>> --- a/driver/main.c
>> +++ b/driver/main.c
>> @@ -297,14 +297,14 @@ static int jailhouse_cmd_enable(struct
>> jailhouse_system __user *arg)
>> }
>>
>> #ifdef JAILHOUSE_BORROW_ROOT_PT
>> - if (config->debug_console.flags & JAILHOUSE_MEM_IO) {
>> - console = ioremap(config->debug_console.phys_start,
>> + if (DBG_IS_MMIO(config->debug_console.flags)) {
>> + console = ioremap(config->debug_console.address,
>> config->debug_console.size);
>> if (!console) {
>> err = -EINVAL;
>> pr_err("jailhouse: Unable to map hypervisor debug "
>> "console at %08lx\n",
>> - (unsigned long)config->debug_console.phys_start);
>> + (unsigned long)config->debug_console.address);
>> goto error_unmap;
>> }
>> /* The hypervisor has no notion of address spaces, so we need
>> diff --git a/hypervisor/arch/arm64/asm-defines.c
>> b/hypervisor/arch/arm64/asm-defines.c
>> index 2e9373de73d5..5a92b00a6ab8 100644
>> --- a/hypervisor/arch/arm64/asm-defines.c
>> +++ b/hypervisor/arch/arm64/asm-defines.c
>> @@ -23,7 +23,7 @@ void common(void)
>> OFFSET(HEADER_MAX_CPUS, jailhouse_header, max_cpus);
>> OFFSET(HEADER_DEBUG_CONSOLE_VIRT, jailhouse_header, debug_console_base);
>> OFFSET(SYSCONFIG_DEBUG_CONSOLE_PHYS, jailhouse_system,
>> - debug_console.phys_start);
>> + debug_console.address);
>> OFFSET(SYSCONFIG_HYPERVISOR_PHYS, jailhouse_system,
>> hypervisor_memory.phys_start);
>> BLANK();
>> diff --git a/hypervisor/arch/x86/uart.c b/hypervisor/arch/x86/uart.c
>> index 09c66f01a7c3..0913d2b4a1e8 100644
>> --- a/hypervisor/arch/x86/uart.c
>> +++ b/hypervisor/arch/x86/uart.c
>> @@ -66,14 +66,16 @@ void uart_init(void)
>> {
>> u64 flags = system_config->debug_console.flags;
>>
>> - if (system_config->debug_console.phys_start == 0)
>> + if (system_config->debug_console.address == 0)
>> return;
>>
>> - if (flags & JAILHOUSE_MEM_IO) {
>> - if (system_config->debug_console.phys_start < VGA_LIMIT)
>> + if (DBG_IS_MMIO(flags)) {
>> + if (system_config->debug_console.address < VGA_LIMIT)
>> return; /* VGA memory */
>>
>> - if (flags & JAILHOUSE_MEM_IO_32) {
>> + /* Use 32-bit MMIO access width if desired, and 8-bit MMIO
>> + * access width by default */
>
> Register distance - not access width. And we should use 32-bit distance
> for MMIO irrespective of the arch.
>
>> + if (flags & JAILHOUSE_DBG_FLAG_REG_WIDTH_32) {
>> uart_reg_out = uart_mmio32_out;
>> uart_reg_in = uart_mmio32_in;
>> } else {
>> @@ -82,7 +84,7 @@ void uart_init(void)
>> }
>> uart_base = (u64)hypervisor_header.debug_console_base;
>> } else {
>> - uart_base = system_config->debug_console.phys_start;
>> + uart_base = system_config->debug_console.address;
>> }
>>
>> uart_reg_out(UART_LCR, UART_LCR_DLAB);
>> diff --git a/hypervisor/arch/x86/vga.c b/hypervisor/arch/x86/vga.c
>> index 53a2f8e00c93..e784448c5d2b 100644
>> --- a/hypervisor/arch/x86/vga.c
>> +++ b/hypervisor/arch/x86/vga.c
>> @@ -30,7 +30,7 @@ u16 *vga_mem;
>>
>> void vga_init(void)
>> {
>> - if (system_config->debug_console.phys_start < VGA_LIMIT)
>> + if (system_config->debug_console.address < VGA_LIMIT)
>> vga_mem = hypervisor_header.debug_console_base;
>> }
>>
>> diff --git a/hypervisor/include/jailhouse/cell-config.h
>> b/hypervisor/include/jailhouse/cell-config.h
>> index a139f97a7397..82c29495deaa 100644
>> --- a/hypervisor/include/jailhouse/cell-config.h
>> +++ b/hypervisor/include/jailhouse/cell-config.h
>> @@ -164,6 +164,30 @@ struct jailhouse_iommu {
>> __u32 amd_features;
>> } __attribute__((packed));
>>
>> +/* Bits 0..3 are used to select the particular driver */
>> +#define JAILHOUSE_DBG_TYPE_UART_X86 0x0001
>> +#define JAILHOUSE_DBG_TYPE_UART_ARM 0x0002
>> +#define JAILHOUSE_DBG_TYPE_VGA 0x0003
>> +#define JAILHOUSE_DBG_TYPE_MASK 0x000f
>> +
>> +#define DBG_TYPE(flags) ((flags) & JAILHOUSE_DBG_TYPE_MASK)
>> +
>> +/* We use bit 4..5 to differentiate between PIO and MMIO access */
>> +#define JAILHOUSE_DBG_FLAG_PIO 0x0010
>> +#define JAILHOUSE_DBG_FLAG_MMIO 0x0020
>> +
>> +#define DBG_IS_MMIO(flags) ((flags) & JAILHOUSE_DBG_FLAG_MMIO)
>> +
>> +/* Bits 8..9 select the MMIO access width */
>> +#define JAILHOUSE_DBG_FLAG_REG_WIDTH_8 0x0100
>> +#define JAILHOUSE_DBG_FLAG_REG_WIDTH_32 0x0200
>
> Sorry, send you down the wrong road: As said above, we actually specify
> the register distance here. The access width is specific to the UART and
> encoded into the driver. So, ...REG_DIST_1 and ...REG_DIST_4 would be an
> option, encoding things in bytes now as that is more natural for addresses.
Ok, this clarifies things... I really thought you were talking about
access width and not register distance.
Only for my understanding: given a driver with let's say a single 32 bit
width access in the code, isn't this a naturally given minimum of 4 byte
register distance?
In this case, 8 bit distance together with such a driver would not make
any sense at all, right? But we still want to force-support that?
Anyway, let me introduce those flags in this patch and respect them
later in the series...
Ralf
>
>> +
>> +struct jailhouse_debug_console {
>> + __u64 address;
>> + __u32 size;
>> + __u32 flags;
>> +} __attribute__((packed));
>> +
>> #define JAILHOUSE_SYSTEM_SIGNATURE "JAILSYST"
>>
>> /**
>> @@ -174,7 +198,7 @@ struct jailhouse_system {
>>
>> /** Jailhouse's location in memory */
>> struct jailhouse_memory hypervisor_memory;
>> - struct jailhouse_memory debug_console;
>> + struct jailhouse_debug_console debug_console;
>> union {
>> struct {
>> __u64 mmconfig_base;
>> diff --git a/hypervisor/paging.c b/hypervisor/paging.c
>> index 5ecee52881c9..3b030d6b1089 100644
>> --- a/hypervisor/paging.c
>> +++ b/hypervisor/paging.c
>> @@ -595,7 +595,7 @@ int paging_init(void)
>> if (err)
>> return err;
>>
>> - if (system_config->debug_console.flags & JAILHOUSE_MEM_IO) {
>> + if (DBG_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 &&
>> @@ -603,7 +603,7 @@ int paging_init(void)
>> return trace_error(-EINVAL);
>>
>> err = paging_create(&hv_paging_structs,
>> - system_config->debug_console.phys_start,
>> + system_config->debug_console.address,
>> system_config->debug_console.size, vaddr,
>> PAGE_DEFAULT_FLAGS | PAGE_FLAG_DEVICE,
>> PAGING_NON_COHERENT);
>> diff --git a/tools/root-cell-config.c.tmpl b/tools/root-cell-config.c.tmpl
>> index b4522efd7377..3e668273e091 100644
>> --- a/tools/root-cell-config.c.tmpl
>> +++ b/tools/root-cell-config.c.tmpl
>> @@ -60,7 +60,9 @@ struct {
>> .size = ${hex(hvmem[1])},
>> },
>> .debug_console = {
>> - .phys_start = 0x3f8,
>> + .address = 0x3f8,
>> + .flags = JAILHOUSE_DBG_TYPE_UART_X86 | \
>> + JAILHOUSE_DBG_FLAG_PIO
>> },
>> .platform_info.x86 = {
>> .mmconfig_base = ${hex(mmconfig.base)},
>>
>
> Patches 1-3 look fine.
>
> Thanks,
> Jan
>
--
Ralf Ramsauer
GPG: 0x8F10049B
--
You received this message because you are subscribed to the Google Groups
"Jailhouse" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
