On 2017-04-03 14:00, Henning Schild wrote:
> With the new feature to download the used firmware out of sysfs and
> a way to get GCOV data into that. It is now time to extract this
> information and generate .gcda files for higher level tools to work
> with.
> Instead of dealing with the details of that just use what the
> compiler gives us when we link to its gcov lib.
> 
> Signed-off-by: Henning Schild <[email protected]>
> 
> diff --git a/.gitignore b/.gitignore
> --- a/.gitignore
> +++ b/.gitignore
> @@ -4,6 +4,7 @@
>  *.cmd
>  *.bin
>  *.gcno
> +*.gcda
>  .tmp_versions
>  *.dtb
>  *.dtb.S
> @@ -14,6 +15,7 @@ driver/jailhouse.ko
>  hypervisor/include/jailhouse/config.h
>  hypervisor/hypervisor.lds
>  tools/jailhouse
> +tools/jailhouse-gcov-extract
>  tools/jailhouse-config-collect
>  configs/*.cell
>  Documentation/generated
> diff --git a/Kbuild b/Kbuild
> --- a/Kbuild
> +++ b/Kbuild
> @@ -45,6 +45,6 @@ subdir-ccflags-y := -Werror
>  
>  # directory dependencies on generated files
>  $(obj)/driver $(obj)/hypervisor: $(VERSION_H)
> -$(obj)/hypervisor $(obj)/inmates $(obj)/driver: $(CONFIG_MK)
> +$(obj)/hypervisor $(obj)/inmates $(obj)/driver $(obj)/tools: $(CONFIG_MK)
>  
>  clean-dirs := Documentation/generated hypervisor/include/generated
> diff --git a/Makefile b/Makefile
> --- a/Makefile
> +++ b/Makefile
> @@ -49,7 +49,7 @@ tool_inmates_install: $(DESTDIR)$(libexe
>       $(INSTALL_DATA) inmates/tools/$(ARCH)/*.bin $<
>  
>  install: modules_install firmware_install tool_inmates_install
> -     $(Q)$(MAKE) -C tools $@ src=.
> +     $(Q)$(MAKE) -C tools $@ src=. obj=.
>  
>  .PHONY: modules_install install clean firmware_install modules tools docs \
>       docs_clean
> diff --git a/tools/Makefile b/tools/Makefile
> --- a/tools/Makefile
> +++ b/tools/Makefile
> @@ -12,6 +12,7 @@
>  
>  # includes installation-related variables and definitions
>  include $(src)/../scripts/include.mk
> +include $(obj)/../hypervisor/include/generated/config.mk
>  
>  LD = $(CC)
>  NOSTDINC_FLAGS :=
> @@ -23,6 +24,7 @@ LDFLAGS :=
>  GCOV_PROFILE := n
>  
>  BINARIES := jailhouse
> +
>  HELPERS := \
>       jailhouse-cell-linux \
>       jailhouse-cell-stats \
> @@ -58,6 +60,20 @@ targets += jailhouse.o
>  $(obj)/jailhouse: $(obj)/jailhouse.o
>       $(call if_changed,ld)
>  
> +ifdef CONFIG_JAILHOUSE_GCOV

Did I ask this before? What speaks against building this tool
unconditionally?

> +CFLAGS_jailhouse-gcov-extract.o      := -I$(src)/../hypervisor/include \
> +     -I$(src)/../hypervisor/arch/$(SRCARCH)/include
> +# add that just to LDFLAGS, otherwise we would profile the tool as well
> +LDFLAGS_jailhouse-gcov-extract := --coverage

Would that be a problem? Do we want that for the main jailhouse tool?

> +
> +targets += jailhouse-gcov-extract.o
> +BINARIES += jailhouse-gcov-extract
> +always += jailhouse-gcov-extract
> +
> +$(obj)/jailhouse-gcov-extract: $(obj)/jailhouse-gcov-extract.o
> +     $(call if_changed,ld)
> +endif
> +
>  $(obj)/jailhouse-config-collect: $(src)/jailhouse-config-create 
> $(src)/jailhouse-config-collect.tmpl
>       $(call if_changed,gen_collect)
>  
> diff --git a/tools/jailhouse-gcov-extract.c b/tools/jailhouse-gcov-extract.c
> new file mode 100644
> --- /dev/null
> +++ b/tools/jailhouse-gcov-extract.c
> @@ -0,0 +1,191 @@
> +/*
> + * 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 <stdio.h>
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <sys/mman.h>
> +#include <fcntl.h>
> +#include <errno.h>
> +#include <error.h>
> +#include <unistd.h>
> +#include <assert.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <jailhouse/header.h>
> +#include <asm/jailhouse_header.h>
> +
> +#if (__GNUC__ < 4) || (__GNUC__ == 4 && __GNUC_MINOR__ < 7)
> +// does anyone still care about that ?

At least we care about bailing out when building with an unsupported
toolchain - just remove the comment.

> +#error "Gcov format of gcc < 4.7 is not supported!"
> +#endif
> +
> +/*
> + * the following bits are heavily inspired by linux/kernel/gcov/gcc_4.7.c
> + * with some slight modification
> + */
> +#if BITS_PER_LONG >= 64
> +typedef long gcov_type;
> +#else
> +typedef long long gcov_type;
> +#endif
> +
> +#if (__GNUC__ > 5) || (__GNUC__ == 5 && __GNUC_MINOR__ >= 1)
> +#define GCOV_COUNTERS                        10
> +#elif __GNUC__ == 4 && __GNUC_MINOR__ >= 9
> +#define GCOV_COUNTERS                        9
> +#else
> +#define GCOV_COUNTERS                        8
> +#endif
> +
> +struct gcov_ctr_info {
> +     unsigned int num;
> +     gcov_type *values;
> +};
> +
> +struct gcov_fn_info {
> +     struct gcov_info *key;
> +     unsigned int ident;
> +     unsigned int lineno_checksum;
> +     unsigned int cfg_checksum;
> +     struct gcov_ctr_info ctrs[0];
> +};
> +
> +struct gcov_info {
> +     unsigned int version;
> +     struct gcov_info *next;
> +     unsigned int stamp;
> +     char *filename;
> +     void (*merge[GCOV_COUNTERS])(gcov_type *, unsigned int);
> +     unsigned int n_functions;
> +     struct gcov_fn_info **functions;
> +};
> +/*
> + * end of linux/kernel/gcov/gcc_4.7.c
> + */
> +
> +static void *hypervisor;
> +static size_t hypervisor_size;
> +// TODO
> +// do we care about the other possible merge functions from libgcc?

Can you clarify this? Ideally before merging.

> +extern void __gcov_merge_add(gcov_type *counters, unsigned int n_counters);
> +extern void __gcov_init(struct gcov_info *);
> +extern void __gcov_dump();
> +
> +
> +static void *hypervisor2current(void *hvp)
> +{
> +     unsigned long hvaddr = (unsigned long)hvp;
> +     void *ret;
> +
> +     if(hvp == NULL)

Coding style: if (hyp...

> +             return NULL;
> +     assert(hvaddr >= JAILHOUSE_BASE &&
> +            hvaddr < JAILHOUSE_BASE + hypervisor_size);
> +     ret = (void *)(hvaddr - JAILHOUSE_BASE + (unsigned long)hypervisor);
> +     
> +     return ret;
> +}
> +
> +/*
> + * translate one gcov-"tree" from the hypervisor address space to the current
> + * addresses
> + */
> +static void translate_all_pointers(struct gcov_info *info)
> +{
> +     struct gcov_fn_info *fn_info;
> +     struct gcov_ctr_info *ctr_info;
> +     unsigned int i, j, active;
> +
> +     info->next = hypervisor2current(info->next);
> +     info->filename = hypervisor2current(info->filename);
> +     active = 0;
> +     for (i = 0; i < GCOV_COUNTERS; i++) {
> +             if (info->merge[i]) {
> +                     active++;
> +                     info->merge[i] = &__gcov_merge_add;
> +             } else
> +                     break;

if (!info->merge[i])
        break;
...

> +     }
> +     info->functions = hypervisor2current(info->functions);
> +     for (i = 0; i < info->n_functions; i++) {
> +             info->functions[i] = hypervisor2current(info->functions[i]);
> +             fn_info = info->functions[i];
> +             if (fn_info) {
> +                     fn_info->key = hypervisor2current(fn_info->key);
> +                     assert(fn_info->key == info);
> +                     for (j = 0; j < active; j++) {
> +                             ctr_info = fn_info->ctrs + j;
> +                             ctr_info->values =
> +                                     hypervisor2current(ctr_info->values);
> +                     }
> +             }
> +     }
> +}
> +
> +int main(int argc, char **argv)
> +{
> +     struct gcov_info *gcov_info_head, *info, *next;
> +     struct jailhouse_header *header;
> +     struct stat sbuf;
> +     char *filename;
> +     int fd, ret;
> +
> +     if (argc != 2) {
> +             errno = EINVAL;
> +             goto out;
> +     }
> +     filename = argv[1];
> +     fd = open(filename, O_RDONLY);
> +     if (fd < 1)
> +             goto out;
> +     
> +     ret = fstat(fd, &sbuf);
> +     if (ret)
> +             goto out;
> +     hypervisor_size = sbuf.st_size;
> +     hypervisor = mmap(NULL, hypervisor_size, PROT_READ | PROT_WRITE,
> +                       MAP_PRIVATE, fd, 0);

If mmap is not supported on sysfs, how about reading the file into a
buffer here? Would remove the need to copy it around. We are talking
about less than a megabyte.

> +     if (hypervisor == MAP_FAILED)
> +             goto out_fd;
> +
> +     header = (struct jailhouse_header *)hypervisor;
> +     if (strcmp(header->signature, JAILHOUSE_SIGNATURE)) {
> +             errno = EINVAL;
> +             error(0, 0, "%s does not seem to be a hypervisor image",
> +                   filename);
> +             goto out_m;
> +     }
> +
> +     gcov_info_head = hypervisor2current(header->gcov_info_head);
> +     info = gcov_info_head;
> +     for (info = gcov_info_head; info; info = info->next)
> +             translate_all_pointers(info);
> +     
> +     for (info = gcov_info_head; info;)
> +     {

Coding style.

> +             // remember next because __gcov_init changes it

Is that a TODO? If not, please avoid //-style comments.

> +             next = info->next;
> +             __gcov_init(info);
> +             info = next;
> +     }
> +     __gcov_dump();
> +
> +out_m:
> +     munmap(hypervisor, hypervisor_size);
> +out_fd:      
> +     close(fd);
> +out:
> +     if (errno)
> +             error(errno, errno, " ");
> +     return 0;
> +}
> 

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.

Reply via email to