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 >> >> Am Wed, 25 Jan 2017 13:11:37 +0100 >> schrieb Ralf Ramsauer <[email protected]>: >> >>> The console page is implemented as ring buffer, and will _not_ >>> contain a trailing \0 for string termination. >>> >>> Signed-off-by: Ralf Ramsauer <[email protected]> >>> --- >>> 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) >> > -- 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.
