Subject: -> "introduce" On 2017-02-27 16:40, Henning Schild wrote: > This introduces pretty minimal GCOV support. The hypervisor does not > need to understand much of that. It just needs to call the init > functions to gather the addresses of the gcov related data structures. > These are just linked together so something outside the hypervisor can > look at them later. > > Signed-off-by: Henning Schild <[email protected]> > > diff --git a/driver/main.c b/driver/main.c > --- a/driver/main.c > +++ b/driver/main.c > @@ -419,6 +419,7 @@ > > header = (struct jailhouse_header *)hypervisor_mem; > header->max_cpus = max_cpus; > + header->jailhouse_base = (void *)JAILHOUSE_BASE;
Can be done during hypervisor compile time. Then you also do not need patch 7. > > hypervisor_size = hypervisor->size; > err = jailhouse_sysfs_firmware_init(jailhouse_dev); > diff --git a/hypervisor/Makefile b/hypervisor/Makefile > --- a/hypervisor/Makefile > +++ b/hypervisor/Makefile > @@ -34,6 +34,14 @@ > > CORE_OBJECTS = setup.o printk.o paging.o control.o lib.o mmio.o pci.o > ivshmem.o > > +## for now "make CONFIG_JAILHOUSE_GCOV=y" > +## using config.(h|mk) does not work on this level > +## probably move config.h->mk up one dir > +ifdef CONFIG_JAILHOUSE_GCOV > +CORE_OBJECTS += gcov.o > +endif > +ccflags-$(CONFIG_JAILHOUSE_GCOV) += --coverage -DCONFIG_JAILHOUSE_GCOV=1 > + > define filechk_config_mk > ( \ > echo "\$$(foreach config,\$$(filter CONFIG_%, \ > diff --git a/hypervisor/arch/arm-common/Kbuild > b/hypervisor/arch/arm-common/Kbuild > --- a/hypervisor/arch/arm-common/Kbuild > +++ b/hypervisor/arch/arm-common/Kbuild > @@ -13,6 +13,7 @@ > include $(CONFIG_MK) > > GCOV_PROFILE := n > +ccflags-$(CONFIG_JAILHOUSE_GCOV) += --coverage -DCONFIG_JAILHOUSE_GCOV=1 > > OBJS-y += dbg-write.o lib.o psci.o control.o paging.o mmu_cell.o > OBJS-y += irqchip.o pci.o ivshmem.o uart-pl011.o uart-8250.o uart-xuartps.o > diff --git a/hypervisor/arch/x86/Kbuild b/hypervisor/arch/x86/Kbuild > --- a/hypervisor/arch/x86/Kbuild > +++ b/hypervisor/arch/x86/Kbuild > @@ -13,6 +13,7 @@ > # > > GCOV_PROFILE := n > +ccflags-$(CONFIG_JAILHOUSE_GCOV) += --coverage -DCONFIG_JAILHOUSE_GCOV=1 > > BUILT_IN_OBJECTS := built-in-amd.o built-in-intel.o > COMMON_OBJECTS := apic.o dbg-write.o entry.o setup.o control.o mmio.o > iommu.o \ > diff --git a/hypervisor/gcov.c b/hypervisor/gcov.c > new file mode 100644 > --- /dev/null > +++ b/hypervisor/gcov.c > @@ -0,0 +1,35 @@ > +/* > + * Jailhouse, a Linux-based partitioning hypervisor > + * > + * Copyright (c) Siemens AG, 2017 > + * > + * Authors: > + * Henning Schild <[email protected]> > + * > + * This work is licensed under the terms of the GNU GPL, version 2. See > + * the COPYING file in the top-level directory. > + */ > +#include <jailhouse/entry.h> > + > +/* the actual data structure is bigger but we just need to know the version > + * independant beginning to link the element to a list */ independent "element" or "elements"? > +struct gcov_min_info { > + unsigned int version; > + struct gcov_min_info *next; > +}; Blank line. > +void __gcov_init(struct gcov_min_info *info); > +void __gcov_merge_add(void *counters, unsigned int n_counters); > + > + > +/* just link them all together and leave the head in the header > + * where a processing tool can find it */ > +void __gcov_init(struct gcov_min_info *info) > +{ > + info->next = (struct gcov_min_info*)hypervisor_header.gcov_info_head; (struct gcov_min_info *) > + hypervisor_header.gcov_info_head = info; > +} > + > +/* Satisfy the linker, never called */ > +void __gcov_merge_add(void *counters, unsigned int n_counters) > +{ > +} > diff --git a/hypervisor/hypervisor.lds.S b/hypervisor/hypervisor.lds.S > --- a/hypervisor/hypervisor.lds.S > +++ b/hypervisor/hypervisor.lds.S > @@ -30,6 +30,11 @@ > . = ALIGN(16); > .data : { *(.data) } > > + . = ALIGN(8); > + __init_array_start = .; > + .init_array : { *(SORT(.init_array.*)) *(.init_array) } > + __init_array_end = .; > + > ARCH_SECTIONS > > . = ALIGN(16); > diff --git a/hypervisor/include/jailhouse/header.h > b/hypervisor/include/jailhouse/header.h > --- a/hypervisor/include/jailhouse/header.h > +++ b/hypervisor/include/jailhouse/header.h > @@ -56,7 +56,19 @@ > /** Offset of the console page inside the hypervisor memory > * @note Filled at build time. */ > unsigned long console_page; > + // TODO we could also printk those two values or put them in sysfs > + // but putting them here the tools will find everything it needs > + // in one place I don't get the comment: How can we put that in sysfs without telling the driver first what the value is? Then we do need this header field, no? Please clarify, then remove the comment. > + /** Pointer to the first struct gcov_info > + * @note Filled at build time */ > + void *gcov_info_head; > > + // TODO would it be cleaner to do fill this in from the hypervisor > + // would make arm64 export patch redundant Exactly. Please fix. > + /** JAILHOUSE_BASE, keep it here so debugging tools can know > + * the firmware image was mapped > + * @note Filled by Linux loader driver before entry */ > + void *jailhouse_base; > /** Configured maximum logical CPU ID + 1. > * @note Filled by Linux loader driver before entry. */ > unsigned int max_cpus; > diff --git a/hypervisor/setup.c b/hypervisor/setup.c > --- a/hypervisor/setup.c > +++ b/hypervisor/setup.c > @@ -23,7 +23,9 @@ > extern u8 __text_start[], __page_pool[]; The __init_arrays should go here. > > static const __attribute__((aligned(PAGE_SIZE))) u8 empty_page[PAGE_SIZE]; > - > +#ifdef CONFIG_JAILHOUSE_GCOV > +extern unsigned long __init_array_start[], __init_array_end[]; > +#endif Unneeded #ifdef. > static DEFINE_SPINLOCK(init_lock); > static unsigned int master_cpu_id = -1; > static volatile unsigned int initialized_cpus; > @@ -216,7 +218,19 @@ > arch_cpu_restore(cpu_data, error); > return error; > } > - > +// TODO is there a better time to do that Yes, early in init_early, maybe after the first printk but before the first return on error. > +#ifdef CONFIG_JAILHOUSE_GCOV > + if (master) { > + unsigned long *iarray = __init_array_start; > + unsigned long *iarray_end = __init_array_end; > + void (*__init_func)(void); Blank line. > + while (iarray < iarray_end) { > + __init_func = (void(*)(void))(iarray[0]); The braces around iarray[0] are unneeded. > + iarray++; Could be folded into the statement above, but I also don't mind keeping it separate. > + (*__init_func)(); __init_func(); > + } > + } > +#endif > if (master) > printk("Activating hypervisor\n"); > > 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.
