On 2016-11-29 16:55, Ralf Ramsauer wrote: > > > 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.
Yes, sounds all reasonable. Thanks, Jan -- Siemens AG, Corporate Technology, CT RDA ITP SES-DE Corporate Competence Center Embedded Linux -- 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.
