On 11/29/2016 03:25 PM, Jan Kiszka wrote: > On 2016-11-29 12:56, Jan Kiszka wrote: >> On 2016-11-25 16:26, Ralf Ramsauer wrote: >>> Add dbg-types PIO and MMIO for x86 inmates. >>> >>> Signed-off-by: Ralf Ramsauer <[email protected]> >>> --- >>> inmates/lib/x86/printk.c | 52 >>> +++++++++++++++++++++++++++++++++++++++++------- >>> 1 file changed, 45 insertions(+), 7 deletions(-) >>> >>> diff --git a/inmates/lib/x86/printk.c b/inmates/lib/x86/printk.c >>> index 9939e61158..f7c75815f0 100644 >>> --- a/inmates/lib/x86/printk.c >>> +++ b/inmates/lib/x86/printk.c >>> @@ -13,6 +13,7 @@ >>> #include <stdarg.h> >>> #include <inmate.h> >>> >>> +#define DEBUG_TYPE "PIO" >>> #define UART_BASE 0x3f8 >>> #define UART_TX 0x0 >>> #define UART_DLL 0x0 >>> @@ -23,7 +24,31 @@ >>> #define UART_LSR 0x5 >>> #define UART_LSR_THRE 0x20 >>> >>> -static unsigned int printk_uart_base; >>> +static long unsigned int printk_uart_base; >>> + >>> +static void uart_pio_out(unsigned int reg, u8 value) >>> +{ >>> + outb(value, printk_uart_base + reg); >>> +} >>> + >>> +static u8 uart_pio_in(unsigned int reg) >>> +{ >>> + return inb(printk_uart_base + reg); >>> +} >>> + >>> +static void uart_mmio32_out(unsigned int reg, u8 value) >>> +{ >>> + mmio_write32((void *)printk_uart_base + reg * 4, value); >>> +} >>> + >>> +static u8 uart_mmio32_in(unsigned int reg) >>> +{ >>> + return mmio_read32((void *)printk_uart_base + reg * 4); >>> +} >>> + >>> +/* choose 8 bit PIO by default */ >>> +static void (*uart_reg_out)(unsigned int, u8) = uart_pio_out; >>> +static u8 (*uart_reg_in)(unsigned int) = uart_pio_in; >>> >>> static void uart_write(const char *msg) >>> { >>> @@ -39,9 +64,9 @@ static void uart_write(const char *msg) >>> c = *msg++; >>> if (!c) >>> break; >>> - while (!(inb(printk_uart_base + UART_LSR) & UART_LSR_THRE)) >>> + while (!(uart_reg_in(UART_LSR) & UART_LSR_THRE)) >>> cpu_relax(); >>> - outb(c, printk_uart_base + UART_TX); >>> + uart_reg_out(UART_TX, c); >>> } >>> } >>> >>> @@ -50,18 +75,28 @@ static void uart_write(const char *msg) >>> >>> static void dbg_init(void) >>> { >>> + const char *type; >>> + char buf[32]; >>> unsigned int divider; >>> >>> printk_uart_base = cmdline_parse_int("dbg-base", UART_BASE); >>> divider = cmdline_parse_int("dbg-divider", 0); >>> + type = cmdline_parse_str("dbg-type", buf, sizeof(buf), DEBUG_TYPE); >>> + >>> + if (strncmp(type, "MMIO", 4) == 0) { >>> + uart_reg_out = uart_mmio32_out; >>> + uart_reg_in = uart_mmio32_in; >> >> You also need map_range(base, size, MAP_UNCACHED) in order to do this >> mmio on x86. >> > > ...which we only provide for 64 bit so far. > > OK, good news is that I successfully tested hypervisor and apic-demo > with this patch on an Apollo Lake board: > > diff --git a/configs/apic-demo.c b/configs/apic-demo.c > index db599c3..ebc157b 100644 > --- a/configs/apic-demo.c > +++ b/configs/apic-demo.c > @@ -20,7 +20,7 @@ > struct { > struct jailhouse_cell_desc cell; > __u64 cpus[1]; > - struct jailhouse_memory mem_regions[2]; > + struct jailhouse_memory mem_regions[3]; > struct jailhouse_cache cache_regions[1]; > __u8 pio_bitmap[0x2000]; > } __attribute__((packed)) config = { > @@ -55,6 +55,13 @@ struct { > .flags = JAILHOUSE_MEM_READ | JAILHOUSE_MEM_WRITE | > JAILHOUSE_MEM_COMM_REGION, > }, > + { > + .phys_start = 0x91230000, > + .virt_start = 0x91230000, > + .size = 0x1000, > + .flags = JAILHOUSE_MEM_READ | JAILHOUSE_MEM_WRITE | > + JAILHOUSE_MEM_ROOTSHARED, > + }, > }, > > .cache_regions = { > diff --git a/inmates/lib/x86/printk.c b/inmates/lib/x86/printk.c > index f7c7581..ccdc732 100644 > --- a/inmates/lib/x86/printk.c > +++ b/inmates/lib/x86/printk.c > @@ -86,6 +86,9 @@ static void dbg_init(void) > if (strncmp(type, "MMIO", 4) == 0) { > uart_reg_out = uart_mmio32_out; > uart_reg_in = uart_mmio32_in; > +#ifdef __x86_64__ > + map_range((void *)printk_uart_base, 0x1000, MAP_UNCACHED); > +#endif > } else if (strncmp(type, "none", 4) == 0) { > printk_uart_base = 0; > } > > > While playing with it, I realized another thing, though this is now > really cosmetic: tagging the inmate parameters with "dbg-" is not fully > accurate. For our inmates, this channel is actually their primary and > only console. So, either "con-" or even "uart-" would be better. What do > you think? Right, true...
So what about "con-"? Sounds more generic than "uart-". Do you also want me to s/JAILHOUSE_DBG_TYPE_/JAILHOUSE_CON_TYPE_/? And we should increase the config revision counter. Think it's better to resend the whole series again after those changes. Ralf > > Jan > -- Ralf Ramsauer PGP: 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.
