Am Tue, 11 Apr 2017 16:09:16 +0200 schrieb Ralf Ramsauer <[email protected]>:
> 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 > >> > >> 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.
