Updated set of patches for coverage. Changes: - now public header contain information only of Xen export format - use sysctl instead of a specific hypercall Tools require smaller changes, now export one use libxc, I''ll attach ASAP.
This patch introduce coverage support to Xen.
Currently it allows to compile Xen with coverage support but there is no
way to extract them.
The declarations came from Linux source files (as you can see from file
headers).
The idea is to have some operations mainly
- get coverage information size
- read coverage information
- reset coverage counters
Linux use a file system to export these information. The information will
be a blob to handle with some tools (as usually tools require a bunch of
files but Xen does not handle files at all). I''ll pack them to make
things
simpler as possible.
These information cannot be put in a specific section (allowing a safe
mapping) as gcc use .rodata, .data, .text and .ctors sections.
I added code to handle constructors used in this case to initialize a
linked list of files.
I excluded %.init.o files as they are used before Xen start and should
not have section like .text or .data.
I used a "coverage" configuration option to mimic the
"debug" one.
Signed-off-by: Frediano Ziglio <frediano.ziglio@citrix.com>
---
.gitignore | 2 +
.hgignore | 2 +
Config.mk | 3 ++
xen/Rules.mk | 5 +++
xen/arch/x86/setup.c | 3 ++
xen/arch/x86/xen.lds.S | 7 ++++
xen/common/Makefile | 2 +
xen/common/gcov/Makefile | 2 +
xen/common/gcov/gcov.c | 94 ++++++++++++++++++++++++++++++++++++++++++++
xen/include/xen/gcov.h | 97 ++++++++++++++++++++++++++++++++++++++++++++++
10 files changed, 217 insertions(+)
create mode 100644 xen/common/gcov/Makefile
create mode 100644 xen/common/gcov/gcov.c
create mode 100644 xen/include/xen/gcov.h
diff --git a/.gitignore b/.gitignore
index 462e291..af48492 100644
--- a/.gitignore
+++ b/.gitignore
@@ -13,6 +13,8 @@
*.tmp
*.spot
*.spit
+*.gcno
+*.gcda
TAGS
GTAGS
GRTAGS
diff --git a/.hgignore b/.hgignore
index 3024ef1..146d8d4 100644
--- a/.hgignore
+++ b/.hgignore
@@ -17,6 +17,8 @@
.*\.rej$
.*\.spot$
.*\.spit$
+.*\.gcno$
+.*\.gcda$
.*/a\.out$
.*/Modules\.symvers$
.*/cscope\..*$
diff --git a/Config.mk b/Config.mk
index 64541c8..8e60f5e 100644
--- a/Config.mk
+++ b/Config.mk
@@ -13,6 +13,9 @@ realpath = $(wildcard $(foreach file,$(1),$(shell cd -P $(dir
$(file)) && echo "
debug ?= y
debug_symbols ?= $(debug)
+# Test coverage support
+coverage ?= n
+
XEN_COMPILE_ARCH ?= $(shell uname -m | sed -e s/i.86/x86_32/ \
-e s/i86pc/x86_32/ -e s/amd64/x86_64/ \
-e s/armv7.*/arm32/)
diff --git a/xen/Rules.mk b/xen/Rules.mk
index c2db449..c9044f5 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -103,6 +103,11 @@ subdir-all := $(subdir-y) $(subdir-n)
$(filter %.init.o,$(obj-y) $(obj-bin-y)): CFLAGS += -DINIT_SECTIONS_ONLY
+
+ifeq ($(coverage),y)
+$(filter-out %.init.o,$(obj-y) $(obj-bin-y)): CFLAGS += -fprofile-arcs
-ftest-coverage -DTEST_COVERAGE
+endif
+
ifeq ($(lto),y)
# Would like to handle all object files as bitcode, but objects made from
# pure asm are in a different format and have to be collected separately.
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index f4d3788..ddf5c4d 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -46,6 +46,7 @@
#include <asm/setup.h>
#include <xen/cpu.h>
#include <asm/nmi.h>
+#include <xen/gcov.h>
/* opt_nosmp: If true, secondary processors are ignored. */
static bool_t __initdata opt_nosmp;
@@ -1313,6 +1314,8 @@ void __init __start_xen(unsigned long mbi_p)
init_trace_bufs();
+ init_coverage();
+
console_endboot();
/* Hide UART from DOM0 if we''re using it */
diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
index d324afd..5570389 100644
--- a/xen/arch/x86/xen.lds.S
+++ b/xen/arch/x86/xen.lds.S
@@ -108,6 +108,13 @@ SECTIONS
__trampoline_seg_start = .;
*(.trampoline_seg)
__trampoline_seg_stop = .;
+
+ . = ALIGN(8);
+ __CTOR_LIST__ = .;
+ QUAD((__CTOR_END__ - __CTOR_LIST__) / 8 - 2)
+ *(.ctors)
+ QUAD(0)
+ __CTOR_END__ = .;
} :text
. = ALIGN(32);
.init.setup : {
diff --git a/xen/common/Makefile b/xen/common/Makefile
index 1677342..8a0c506 100644
--- a/xen/common/Makefile
+++ b/xen/common/Makefile
@@ -59,5 +59,7 @@ subdir-$(CONFIG_COMPAT) += compat
subdir-$(x86_64) += hvm
+subdir-$(coverage) += gcov
+
subdir-y += libelf
subdir-$(HAS_DEVICE_TREE) += libfdt
diff --git a/xen/common/gcov/Makefile b/xen/common/gcov/Makefile
new file mode 100644
index 0000000..c9efe6c
--- /dev/null
+++ b/xen/common/gcov/Makefile
@@ -0,0 +1,2 @@
+obj-y += gcov.o
+
diff --git a/xen/common/gcov/gcov.c b/xen/common/gcov/gcov.c
new file mode 100644
index 0000000..ab75fdf
--- /dev/null
+++ b/xen/common/gcov/gcov.c
@@ -0,0 +1,94 @@
+/*
+ * This code maintains a list of active profiling data structures.
+ *
+ * Copyright IBM Corp. 2009
+ * Author(s): Peter Oberparleiter <oberpar@linux.vnet.ibm.com>
+ *
+ * Uses gcc-internal data definitions.
+ * Based on the gcov-kernel patch by:
+ * Hubertus Franke <frankeh@us.ibm.com>
+ * Nigel Hinds <nhinds@us.ibm.com>
+ * Rajan Ravindran <rajancr@us.ibm.com>
+ * Peter Oberparleiter <oberpar@linux.vnet.ibm.com>
+ * Paul Larson
+ */
+
+#include <xen/config.h>
+#include <xen/init.h>
+#include <xen/lib.h>
+#include <xen/hypercall.h>
+#include <xen/gcov.h>
+#include <xen/errno.h>
+
+static struct gcov_info *info_list;
+static unsigned num_info = 0;
+
+/*
+ * __gcov_init is called by gcc-generated constructor code for each object
+ * file compiled with -fprofile-arcs.
+ *
+ * Although this function is called only during initialization is called from
+ * a .text section which is still present after initialization so not declare
+ * as __init.
+ */
+void __gcov_init(struct gcov_info *info)
+{
+ /* add new profiling data structure to list */
+ info->next = info_list;
+ info_list = info;
+ ++num_info;
+}
+
+/*
+ * These functions may be referenced by gcc-generated profiling code but serve
+ * no function for Xen.
+ */
+void __gcov_flush(void)
+{
+ /* Unused. */
+}
+
+void __gcov_merge_add(gcov_type *counters, unsigned int n_counters)
+{
+ /* Unused. */
+}
+
+void __gcov_merge_single(gcov_type *counters, unsigned int n_counters)
+{
+ /* Unused. */
+}
+
+void __gcov_merge_delta(gcov_type *counters, unsigned int n_counters)
+{
+ /* Unused. */
+}
+
+typedef void (*ctor_func_t)(void);
+extern const struct
+{
+ unsigned long count;
+ ctor_func_t funcs[1];
+} __CTOR_LIST__;
+
+void init_coverage(void)
+{
+ unsigned long n;
+ for ( n = 0; n < __CTOR_LIST__.count; ++n )
+ __CTOR_LIST__.funcs[n]();
+
+#ifndef NDEBUG
+ printk(XENLOG_INFO "Initialized %u coverage strucures\n",
num_info);
+ if ( info_list )
+ printk(XENLOG_INFO "First coverage file %s\n",
info_list->filename);
+#endif
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-set-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/include/xen/gcov.h b/xen/include/xen/gcov.h
new file mode 100644
index 0000000..7b374b8
--- /dev/null
+++ b/xen/include/xen/gcov.h
@@ -0,0 +1,97 @@
+/*
+ * Profiling infrastructure declarations.
+ *
+ * This file is based on gcc-internal definitions. Data structures are
+ * defined to be compatible with gcc counterparts. For a better
+ * understanding, refer to gcc source: gcc/gcov-io.h.
+ *
+ * Copyright IBM Corp. 2009
+ * Author(s): Peter Oberparleiter <oberpar@linux.vnet.ibm.com>
+ *
+ * Uses gcc-internal data definitions.
+ */
+
+#ifndef __XEN_GCOV_H__
+#define __XEN_GCOV_H__ __XEN_GCOV_H__
+
+/*
+ * Profiling data types used for gcc 3.4 and above - these are defined by
+ * gcc and need to be kept as close to the original definition as possible to
+ * remain compatible.
+ */
+
+typedef uint64_t gcov_type;
+
+/**
+ * struct gcov_fn_info - profiling meta data per function
+ * @ident: object file-unique function identifier
+ * @checksum: function checksum
+ * @n_ctrs: number of values per counter type belonging to this function
+ *
+ * This data is generated by gcc during compilation and doesn''t change
+ * at run-time.
+ */
+struct gcov_fn_info
+{
+ unsigned int ident;
+ unsigned int checksum;
+ unsigned int n_ctrs[0];
+};
+
+/**
+ * struct gcov_ctr_info - profiling data per counter type
+ * @num: number of counter values for this type
+ * @values: array of counter values for this type
+ * @merge: merge function for counter values of this type (unused)
+ *
+ * This data is generated by gcc during compilation and doesn''t change
+ * at run-time with the exception of the values array.
+ */
+struct gcov_ctr_info
+{
+ unsigned int num;
+ gcov_type *values;
+ void (*merge)(gcov_type *, unsigned int);
+};
+
+/**
+ * struct gcov_info - profiling data per object file
+ * @version: gcov version magic indicating the gcc version used for compilation
+ * @next: list head for a singly-linked list
+ * @stamp: time stamp
+ * @filename: name of the associated gcov data file
+ * @n_functions: number of instrumented functions
+ * @functions: function data
+ * @ctr_mask: mask specifying which counter types are active
+ * @counts: counter data per counter type
+ *
+ * This data is generated by gcc during compilation and doesn''t change
+ * at run-time with the exception of the next pointer.
+ */
+struct gcov_info
+{
+ unsigned int version;
+ struct gcov_info *next;
+ unsigned int stamp;
+ const char *filename;
+ unsigned int n_functions;
+ const struct gcov_fn_info *functions;
+ unsigned int ctr_mask;
+ struct gcov_ctr_info counts[0];
+};
+
+
+/**
+ * Initialize coverage informations
+ * Currently create a linked list of all files compiled in with
+ * coverage informations.
+ */
+#ifdef TEST_COVERAGE
+void __init init_coverage(void);
+#else
+static inline void init_coverage(void)
+{
+}
+#endif
+
+#endif /* __XEN_GCOV_H__ */
--
1.7.9.5
Frediano Ziglio
2013-Feb-04 17:08 UTC
[PATCH 2/2] Implement code to read coverage informations
Operations are handled by a sysctl specific operation.
Implement 4 operations
- check if coverage is compiled in
- read size of coverage information
- read coverage information
- reset coverage counters
Information are stored in a single blob in a format specific to Xen designed
to make code that generate coverage as small as possible.
This patch add a public header with the structure of blob exported by Xen.
This avoid problems distributing header which is GPL2.
Signed-off-by: Frediano Ziglio <frediano.ziglio@citrix.com>
---
xen/common/gcov/gcov.c | 151 ++++++++++++++++++++++++++++++++++++++++++-
xen/common/sysctl.c | 5 ++
xen/include/public/gcov.h | 92 ++++++++++++++++++++++++++
xen/include/public/sysctl.h | 38 +++++++++++
xen/include/xen/gcov.h | 14 ++++
5 files changed, 297 insertions(+), 3 deletions(-)
create mode 100644 xen/include/public/gcov.h
diff --git a/xen/common/gcov/gcov.c b/xen/common/gcov/gcov.c
index ab75fdf..378866d 100644
--- a/xen/common/gcov/gcov.c
+++ b/xen/common/gcov/gcov.c
@@ -19,9 +19,11 @@
#include <xen/hypercall.h>
#include <xen/gcov.h>
#include <xen/errno.h>
+#include <xen/guest_access.h>
+#include <public/xen.h>
+#include <public/gcov.h>
static struct gcov_info *info_list;
-static unsigned num_info = 0;
/*
* __gcov_init is called by gcc-generated constructor code for each object
@@ -36,7 +38,6 @@ void __gcov_init(struct gcov_info *info)
/* add new profiling data structure to list */
info->next = info_list;
info_list = info;
- ++num_info;
}
/*
@@ -77,12 +78,156 @@ void init_coverage(void)
__CTOR_LIST__.funcs[n]();
#ifndef NDEBUG
- printk(XENLOG_INFO "Initialized %u coverage strucures\n",
num_info);
if ( info_list )
printk(XENLOG_INFO "First coverage file %s\n",
info_list->filename);
#endif
}
+static inline int counter_active(const struct gcov_info *info, unsigned int
type)
+{
+ return (1 << type) & info->ctr_mask;
+}
+
+typedef struct write_iter_t
+{
+ XEN_GUEST_HANDLE(uint8) ptr;
+ int real;
+ uint32_t write_offset;
+} write_iter_t;
+
+static int write_raw(struct write_iter_t *iter, const void *data,
+ size_t data_len)
+{
+ if ( iter->real &&
+ copy_to_guest_offset(iter->ptr, iter->write_offset,
+ (const unsigned char *) data, data_len) )
+ return -EFAULT;
+
+ iter->write_offset + (iter->write_offset + data_len +
(sizeof(uint32_t) - 1)) &
+ -sizeof(uint32_t);
+ return 0;
+}
+
+#define chk(v) do { ret=(v); if ( ret ) return ret; } while(0)
+
+static inline int write32(write_iter_t *iter, uint32_t val)
+{
+ return write_raw(iter, &val, sizeof(val));
+}
+
+static int write_string(write_iter_t *iter, const char *s)
+{
+ int ret;
+ size_t len = strlen(s);
+
+ chk(write32(iter, len));
+ return write_raw(iter, s, len);
+}
+
+static inline int next_type(const struct gcov_info *info, int *type)
+{
+ while ( ++*type < XENCOV_COUNTERS && !counter_active(info,
*type) )
+ continue;
+ return *type;
+}
+
+static int write_gcov(write_iter_t *iter)
+{
+ struct gcov_info *info;
+ int ret;
+
+ /* reset offset */
+ iter->write_offset = 0;
+
+ /* dump all files */
+ for ( info = info_list ; info; info = info->next )
+ {
+ const struct gcov_ctr_info *ctr;
+ int type;
+ size_t size_fn = sizeof(struct gcov_fn_info);
+
+ chk(write32(iter, XENCOV_TAG_FILE));
+ chk(write_string(iter, info->filename));
+ chk(write32(iter, info->version));
+ chk(write32(iter, info->stamp));
+
+ /* dump counters */
+ ctr = info->counts;
+ for ( type = -1; next_type(info, &type) < XENCOV_COUNTERS; ++ctr
)
+ {
+ chk(write32(iter, XENCOV_TAG_COUNTER(type)));
+ chk(write32(iter, ctr->num));
+ chk(write_raw(iter, ctr->values,
+ ctr->num * sizeof(ctr->values[0])));
+
+ size_fn += sizeof(unsigned);
+ }
+
+ /* dump all functions together */
+ chk(write32(iter, XENCOV_TAG_FUNC));
+ chk(write32(iter, info->n_functions));
+ chk(write_raw(iter, info->functions, info->n_functions *
size_fn));
+ }
+
+ /* stop tag */
+ chk(write32(iter, XENCOV_TAG_END));
+ return 0;
+}
+
+static int reset_counters(void)
+{
+ struct gcov_info *info;
+
+ for ( info = info_list ; info; info = info->next )
+ {
+ const struct gcov_ctr_info *ctr;
+ int type;
+
+ /* reset counters */
+ ctr = info->counts;
+ for ( type = -1; next_type(info, &type) < XENCOV_COUNTERS; ++ctr
)
+ memset(ctr->values, 0, ctr->num * sizeof(ctr->values[0]));
+ }
+
+ return 0;
+}
+
+int sysctl_coverage_op(xen_sysctl_coverage_op_t *op)
+{
+ long ret = 0;
+ write_iter_t iter;
+
+ switch ( op->cmd )
+ {
+ case XEN_SYSCTL_COVERAGE_enabled:
+ break;
+
+ case XEN_SYSCTL_COVERAGE_get_total_size:
+ iter.real = 0;
+
+ write_gcov(&iter);
+ if ( copy_to_guest(op->u.total_size, &iter.write_offset, 1) )
+ ret = -EFAULT;
+ break;
+
+ case XEN_SYSCTL_COVERAGE_read:
+ iter.ptr = op->u.raw_info;
+ iter.real = 1;
+
+ ret = write_gcov(&iter);
+ break;
+
+ case XEN_SYSCTL_COVERAGE_reset:
+ ret = reset_counters();
+ break;
+
+ default:
+ ret = -EINVAL;
+ }
+ return ret;
+}
+
/*
* Local variables:
* mode: C
diff --git a/xen/common/sysctl.c b/xen/common/sysctl.c
index d663ed7..fa3ef0a 100644
--- a/xen/common/sysctl.c
+++ b/xen/common/sysctl.c
@@ -26,6 +26,7 @@
#include <xen/nodemask.h>
#include <xsm/xsm.h>
#include <xen/pmstat.h>
+#include <xen/gcov.h>
long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
{
@@ -249,6 +250,10 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t)
u_sysctl)
ret = sched_adjust_global(&op->u.scheduler_op);
break;
+ case XEN_SYSCTL_coverage_op:
+ ret = sysctl_coverage_op(&op->u.coverage_op);
+ break;
+
default:
ret = arch_do_sysctl(op, u_sysctl);
copyback = 0;
diff --git a/xen/include/public/gcov.h b/xen/include/public/gcov.h
new file mode 100644
index 0000000..5224249
--- /dev/null
+++ b/xen/include/public/gcov.h
@@ -0,0 +1,92 @@
+/******************************************************************************
+ * gcov.h
+ *
+ * Coverage structures exported by Xen.
+ * Structure is different from Gcc one.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the
"Software"), to
+ * deal in the Software without restriction, including without limitation the
+ * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
+ * sell copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ *
+ * Copyright (c) 2013, Frediano Ziglio
+ */
+
+#ifndef __XEN_PUBLIC_GCOV_H__
+#define __XEN_PUBLIC_GCOV_H__ __XEN_PUBLIC_GCOV_H__
+
+#define XENCOV_COUNTERS 5
+#define XENCOV_TAG_BASE 0x58430000u
+#define XENCOV_TAG_FILE (XENCOV_TAG_BASE+0x0100u)
+#define XENCOV_TAG_FUNC (XENCOV_TAG_BASE+0x0200u)
+#define XENCOV_TAG_COUNTER(n) (XENCOV_TAG_BASE+0x0300u+((n)&0xffu))
+#define XENCOV_TAG_END (XENCOV_TAG_BASE+0xff00u)
+#define XENCOV_IS_TAG_COUNTER(n) \
+ ((n) >= XENCOV_TAG_COUNTER(0) && (n) <
XENCOV_TAG_COUNTER(XENCOV_COUNTERS))
+#define XENCOV_COUNTER_NUM(n) ((n)-XENCOV_TAG_COUNTER(0))
+
+/*
+ * The main structure for the blob is
+ * BLOB := FILE.. END
+ * FILE := TAG_FILE FILENAME xencov_file COUNTERS FUNCTIONS
+ * FILENAME := LEN characters
+ * characters are padded to 32 bit
+ * LEN := 32 bit value
+ * COUNTERS := TAG_COUNTER(n) NUM COUNTER..
+ * NUM := 32 bit valie
+ * COUNTER := 64 bit value
+ * FUNCTIONS := TAG_FUNC NUM xencov_function..
+ */
+
+/**
+ * File information
+ * Prefixed with XENCOV_TAG_FILE and a string with filename
+ */
+struct xencov_file
+{
+ uint32_t version;
+ uint32_t stamp;
+} __attribute__((packed));
+
+/**
+ * Counters information
+ * Prefixed with XENCOV_TAG_COUNTER(n) where n is 0..(XENCOV_COUNTERS-1)
+ */
+struct xencov_counter
+{
+ uint32_t num;
+ uint64_t values[0];
+} __attribute__((packed));
+
+/**
+ * Information for each function
+ * Prefixed with XENCOV_TAG_FUNC
+ * Number of counter is equal to the number of counter got before
+ */
+struct xencov_function
+{
+ uint32_t ident;
+ uint32_t checksum;
+ uint32_t n_ctrs[0];
+} __attribute__((packed));
+
+struct xencov_functions
+{
+ uint32_t num;
+ uint32_t xencov_function[0];
+} __attribute__((packed));
+
+#endif /* __XEN_PUBLIC_GCOV_H__ */
diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
index 3225b2a..5e80400 100644
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -596,6 +596,42 @@ struct xen_sysctl_scheduler_op {
typedef struct xen_sysctl_scheduler_op xen_sysctl_scheduler_op_t;
DEFINE_XEN_GUEST_HANDLE(xen_sysctl_scheduler_op_t);
+/* XEN_SYSCTL_coverage_op */
+/*
+ * Check if coverage informations are available
+ * return just success or error, no parameters
+ */
+#define XEN_SYSCTL_COVERAGE_enabled 0
+
+/*
+ * Get total size of information, to help allocate
+ * the buffer. The pointer points to a 32 bit value.
+ */
+#define XEN_SYSCTL_COVERAGE_get_total_size 1
+
+/*
+ * Read coverage information in a single run
+ * You must use a tool to split them
+ */
+#define XEN_SYSCTL_COVERAGE_read 2
+
+/*
+ * Reset all the coverage counters to 0
+ * No parameters.
+ */
+#define XEN_SYSCTL_COVERAGE_reset 3
+
+struct xen_sysctl_coverage_op {
+ uint32_t cmd; /* XEN_SYSCTL_COVERAGE_* */
+ union {
+ XEN_GUEST_HANDLE_64(uint32) total_size; /* OUT */
+ XEN_GUEST_HANDLE_64(uint8) raw_info; /* OUT */
+ } u;
+};
+typedef struct xen_sysctl_coverage_op xen_sysctl_coverage_op_t;
+DEFINE_XEN_GUEST_HANDLE(xen_sysctl_coverage_op_t);
+
+
struct xen_sysctl {
uint32_t cmd;
#define XEN_SYSCTL_readconsole 1
@@ -616,6 +652,7 @@ struct xen_sysctl {
#define XEN_SYSCTL_numainfo 17
#define XEN_SYSCTL_cpupool_op 18
#define XEN_SYSCTL_scheduler_op 19
+#define XEN_SYSCTL_coverage_op 20
uint32_t interface_version; /* XEN_SYSCTL_INTERFACE_VERSION */
union {
struct xen_sysctl_readconsole readconsole;
@@ -636,6 +673,7 @@ struct xen_sysctl {
struct xen_sysctl_lockprof_op lockprof_op;
struct xen_sysctl_cpupool_op cpupool_op;
struct xen_sysctl_scheduler_op scheduler_op;
+ struct xen_sysctl_coverage_op coverage_op;
uint8_t pad[128];
} u;
};
diff --git a/xen/include/xen/gcov.h b/xen/include/xen/gcov.h
index 7b374b8..d47a3c5 100644
--- a/xen/include/xen/gcov.h
+++ b/xen/include/xen/gcov.h
@@ -14,6 +14,8 @@
#ifndef __XEN_GCOV_H__
#define __XEN_GCOV_H__ __XEN_GCOV_H__
+#include <public/sysctl.h>
+
/*
* Profiling data types used for gcc 3.4 and above - these are defined by
* gcc and need to be kept as close to the original definition as possible to
@@ -94,4 +96,16 @@ static inline void init_coverage(void)
}
#endif
+/**
+ * Sysctl operations for coverage
+ */
+#ifdef TEST_COVERAGE
+int sysctl_coverage_op(xen_sysctl_coverage_op_t *op);
+#else
+static inline int sysctl_coverage_op(xen_sysctl_coverage_op_t *op)
+{
+ return -ENOSYS;
+}
+#endif
+
#endif /* __XEN_GCOV_H__ */
--
1.7.9.5
>>> On 04.02.13 at 18:08, Frediano Ziglio <frediano.ziglio@citrix.com> wrote: > --- a/xen/Rules.mk > +++ b/xen/Rules.mk > @@ -103,6 +103,11 @@ subdir-all := $(subdir-y) $(subdir-n) > > $(filter %.init.o,$(obj-y) $(obj-bin-y)): CFLAGS += -DINIT_SECTIONS_ONLY > > + > +ifeq ($(coverage),y) > +$(filter-out %.init.o,$(obj-y) $(obj-bin-y)): CFLAGS += -fprofile-arcs -ftest-coverage -DTEST_COVERAGE > +endifSo you had tried out the simplified version of this that I had suggested, it worked, yet you still don''t use it?> +void init_coverage(void)As already said previously: __init!> +#ifdef TEST_COVERAGE > +void __init init_coverage(void);And as also already said previously - no __init on declarations! Please, don''t waste reviewers'' time by claiming you incorporated review comments when you really didn''t. Jan
Jan Beulich
2013-Feb-05 08:42 UTC
Re: [PATCH 2/2] Implement code to read coverage informations
>>> On 04.02.13 at 18:08, Frediano Ziglio <frediano.ziglio@citrix.com> wrote: > +/** > + * File information > + * Prefixed with XENCOV_TAG_FILE and a string with filename > + */ > +struct xencov_file > +{ > + uint32_t version; > + uint32_t stamp; > +} __attribute__((packed));No (unconditional) use of compiler extensions in public headers. Even more so that you really don''t need it here ...> + > +/** > + * Counters information > + * Prefixed with XENCOV_TAG_COUNTER(n) where n is 0..(XENCOV_COUNTERS-1) > + */ > +struct xencov_counter > +{ > + uint32_t num; > + uint64_t values[0]; > +} __attribute__((packed));... and you can easily avoid it here, and it''s pointless again further down. The [0] btw also is a compiler extension, and hence you''ll also need to replace it. Jan
Frediano Ziglio
2013-Feb-05 10:15 UTC
Re: [PATCH 1/2] Adding support for coverage information
On Tue, 2013-02-05 at 08:38 +0000, Jan Beulich wrote:> >>> On 04.02.13 at 18:08, Frediano Ziglio <frediano.ziglio@citrix.com> wrote: > > --- a/xen/Rules.mk > > +++ b/xen/Rules.mk > > @@ -103,6 +103,11 @@ subdir-all := $(subdir-y) $(subdir-n) > > > > $(filter %.init.o,$(obj-y) $(obj-bin-y)): CFLAGS += -DINIT_SECTIONS_ONLY > > > > + > > +ifeq ($(coverage),y) > > +$(filter-out %.init.o,$(obj-y) $(obj-bin-y)): CFLAGS += -fprofile-arcs -ftest-coverage -DTEST_COVERAGE > > +endif > > So you had tried out the simplified version of this that I had > suggested, it worked, yet you still don''t use it? >Yes, I forget to explain why I didn''t change this (apart I forgot to remove the extra blank line). It seems that if I change this code as you suggested the code compile correctly but if I extract the informations there are less data. Now I don''t understand if this is just an error or something more critical. Currently I''m trying to understand which file change and why. It seems that these files are compiled with different options xen/common/libelf/libelf-loader.o xen/common/libelf/libelf.o xen/common/libelf/libelf-dominfo.o xen/common/libelf/built_in.o xen/common/libelf/libelf-tools.o xen/common/libelf/libelf-temp.o I''ll try to understand better what''s going on.> > +void init_coverage(void) > > As already said previously: __init! > > > +#ifdef TEST_COVERAGE > > +void __init init_coverage(void); > > And as also already said previously - no __init on declarations! >Got it, I checked code and understood. First patch the __init was correctly in the .c file and somebody complaints so I moved to .h (or probably I misunderstood and the solution was only to remove from header).> Please, don''t waste reviewers'' time by claiming you incorporated > review comments when you really didn''t. > > Jan >Frediano