* [PATCH v2 1/9] Kconfig: use tab instead of space
2016-10-10 9:40 [PATCH v2 0/9] Rework gcov support in Xen Wei Liu
@ 2016-10-10 9:40 ` Wei Liu
2016-10-10 11:21 ` Jan Beulich
2016-10-10 9:40 ` [PATCH v2 2/9] Kconfig: add BROKEN config Wei Liu
` (8 subsequent siblings)
9 siblings, 1 reply; 58+ messages in thread
From: Wei Liu @ 2016-10-10 9:40 UTC (permalink / raw)
To: Xen-devel
Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper,
Ian Jackson, Tim Deegan, philippe.gabriel, Jan Beulich
Previously in d6be2cfc ("xen: make clear gcov support limitation in
Kconfig") and db6c2264 ("xen: add a gcov Kconfig option"), space was
used to indent Kconfig text. Change that to use tab instead.
No functional change.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
v2: new
Would like to have this in 4.8
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>
---
xen/Kconfig.debug | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/xen/Kconfig.debug b/xen/Kconfig.debug
index 12a1193..e9f7dcd 100644
--- a/xen/Kconfig.debug
+++ b/xen/Kconfig.debug
@@ -29,15 +29,15 @@ config FRAME_POINTER
in case of any Xen bugs.
config GCOV
- bool "Gcov Support"
- ---help---
- Enable gcov (a test coverage program in GCC) support.
+ bool "Gcov Support"
+ ---help---
+ Enable gcov (a test coverage program in GCC) support.
- Currently the data structure and hypercall interface are tied
- to GCC 3.4 gcov format. You need to have a version of GCC
- that is compatible with that format to make gcov work.
+ Currently the data structure and hypercall interface are tied
+ to GCC 3.4 gcov format. You need to have a version of GCC
+ that is compatible with that format to make gcov work.
- If unsure, say N here.
+ If unsure, say N here.
config LOCK_PROFILE
bool "Lock Profiling"
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 58+ messages in thread* Re: [PATCH v2 1/9] Kconfig: use tab instead of space
2016-10-10 9:40 ` [PATCH v2 1/9] Kconfig: use tab instead of space Wei Liu
@ 2016-10-10 11:21 ` Jan Beulich
0 siblings, 0 replies; 58+ messages in thread
From: Jan Beulich @ 2016-10-10 11:21 UTC (permalink / raw)
To: Wei Liu
Cc: Stefano Stabellini, George Dunlap, AndrewCooper, Ian Jackson,
Tim Deegan, philippe.gabriel, Xen-devel
>>> On 10.10.16 at 11:40, <wei.liu2@citrix.com> wrote:
> Previously in d6be2cfc ("xen: make clear gcov support limitation in
> Kconfig") and db6c2264 ("xen: add a gcov Kconfig option"), space was
> used to indent Kconfig text. Change that to use tab instead.
>
> No functional change.
>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 58+ messages in thread
* [PATCH v2 2/9] Kconfig: add BROKEN config
2016-10-10 9:40 [PATCH v2 0/9] Rework gcov support in Xen Wei Liu
2016-10-10 9:40 ` [PATCH v2 1/9] Kconfig: use tab instead of space Wei Liu
@ 2016-10-10 9:40 ` Wei Liu
2016-10-10 11:22 ` Jan Beulich
2016-10-10 9:40 ` [PATCH v2 3/9] xen: delete gcno files in clean target Wei Liu
` (7 subsequent siblings)
9 siblings, 1 reply; 58+ messages in thread
From: Wei Liu @ 2016-10-10 9:40 UTC (permalink / raw)
To: Xen-devel
Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper,
Ian Jackson, Tim Deegan, philippe.gabriel, Jan Beulich,
Doug Goldstein
Used to hide feature that is completely broken.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
v2: use tab
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: Doug Goldstein <cardoe@cardoe.com>
---
xen/Kconfig | 3 +++
1 file changed, 3 insertions(+)
diff --git a/xen/Kconfig b/xen/Kconfig
index 0fe7a1a..5515fe9 100644
--- a/xen/Kconfig
+++ b/xen/Kconfig
@@ -12,6 +12,9 @@ config ARCH
string
option env="ARCH"
+config BROKEN
+ bool
+
source "arch/$SRCARCH/Kconfig"
config XEN_FULLVERSION
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 58+ messages in thread* [PATCH v2 3/9] xen: delete gcno files in clean target
2016-10-10 9:40 [PATCH v2 0/9] Rework gcov support in Xen Wei Liu
2016-10-10 9:40 ` [PATCH v2 1/9] Kconfig: use tab instead of space Wei Liu
2016-10-10 9:40 ` [PATCH v2 2/9] Kconfig: add BROKEN config Wei Liu
@ 2016-10-10 9:40 ` Wei Liu
2016-10-10 11:23 ` Jan Beulich
2016-10-10 9:40 ` [PATCH v2 4/9] xen, tools: rip out old gcov implementation Wei Liu
` (6 subsequent siblings)
9 siblings, 1 reply; 58+ messages in thread
From: Wei Liu @ 2016-10-10 9:40 UTC (permalink / raw)
To: Xen-devel
Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper,
Ian Jackson, Tim Deegan, philippe.gabriel, Jan Beulich
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>
---
xen/Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/xen/Makefile b/xen/Makefile
index c511330..48dbbba 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -116,7 +116,7 @@ _clean: delete-unfresh-files
$(MAKE) -f $(BASEDIR)/Rules.mk -C arch/$(TARGET_ARCH) clean
$(MAKE) -f $(BASEDIR)/Rules.mk -C test clean
$(MAKE) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig ARCH=$(ARCH) SRCARCH=$(SRCARCH) clean
- find . \( -name "*.o" -o -name ".*.d" \) -exec rm -f {} \;
+ find . \( -name "*.o" -o -name ".*.d" -o -name "*.gcno" \) -exec rm -f {} \;
rm -f include/asm $(TARGET) $(TARGET).gz $(TARGET).efi $(TARGET).efi.map $(TARGET)-syms $(TARGET)-syms.map *~ core
rm -f include/asm-*/asm-offsets.h
rm -f .banner
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 58+ messages in thread* [PATCH v2 4/9] xen, tools: rip out old gcov implementation
2016-10-10 9:40 [PATCH v2 0/9] Rework gcov support in Xen Wei Liu
` (2 preceding siblings ...)
2016-10-10 9:40 ` [PATCH v2 3/9] xen: delete gcno files in clean target Wei Liu
@ 2016-10-10 9:40 ` Wei Liu
2016-10-10 11:24 ` Jan Beulich
2016-10-10 9:40 ` [PATCH v2 5/9] gcov: add new interface and 3.4 and 4.7 format support Wei Liu
` (5 subsequent siblings)
9 siblings, 1 reply; 58+ messages in thread
From: Wei Liu @ 2016-10-10 9:40 UTC (permalink / raw)
To: Xen-devel
Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper,
Ian Jackson, Tim Deegan, philippe.gabriel, Jan Beulich
The internal data structure and code are tied to an old gcov format.
It's easier to just redo everything from scratch.
Salvage the reusable parts: leave xen/common/gcov and an empty Makefile
there, leave gcov support in Kconfig but mark that as broken. Also
reserve the sysctl number for later use (but delete relevant sysctl
structures).
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>
---
tools/misc/Makefile | 6 --
tools/misc/xencov.c | 147 -----------------------------
tools/misc/xencov_split | 188 ------------------------------------
xen/Kconfig.debug | 1 +
xen/common/gcov/Makefile | 2 -
xen/common/gcov/gcov.c | 225 --------------------------------------------
xen/common/sysctl.c | 7 --
xen/include/public/gcov.h | 115 ----------------------
xen/include/public/sysctl.h | 38 +-------
xen/include/xen/gcov.h | 93 ------------------
10 files changed, 2 insertions(+), 820 deletions(-)
delete mode 100644 tools/misc/xencov.c
delete mode 100755 tools/misc/xencov_split
delete mode 100644 xen/common/gcov/gcov.c
delete mode 100644 xen/include/public/gcov.h
delete mode 100644 xen/include/xen/gcov.h
diff --git a/tools/misc/Makefile b/tools/misc/Makefile
index 8152f7b..5ba6672 100644
--- a/tools/misc/Makefile
+++ b/tools/misc/Makefile
@@ -13,7 +13,6 @@ CFLAGS += $(CFLAGS_libxenstore)
INSTALL_BIN-$(CONFIG_X86) += xen-cpuid
INSTALL_BIN-$(CONFIG_X86) += xen-detect
INSTALL_BIN += xencons
-INSTALL_BIN += xencov_split
INSTALL_BIN += $(INSTALL_BIN-y)
# Everything to be installed in regular sbin/
@@ -25,7 +24,6 @@ INSTALL_SBIN-$(CONFIG_X86) += xen-lowmemd
INSTALL_SBIN-$(CONFIG_X86) += xen-mfndump
INSTALL_SBIN += xen-ringwatch
INSTALL_SBIN += xen-tmem-list-parse
-INSTALL_SBIN += xencov
INSTALL_SBIN += xenlockprof
INSTALL_SBIN += xenperf
INSTALL_SBIN += xenpm
@@ -43,7 +41,6 @@ TARGETS_ALL := $(INSTALL_BIN) $(INSTALL_SBIN) $(INSTALL_PRIVBIN)
TARGETS_COPY += xen-bugtool
TARGETS_COPY += xen-ringwatch
TARGETS_COPY += xencons
-TARGETS_COPY += xencov_split
TARGETS_COPY += xenpvnetboot
# Everything which needs to be built
@@ -105,7 +102,4 @@ xen-livepatch: xen-livepatch.o
xen-lowmemd: xen-lowmemd.o
$(CC) $(LDFLAGS) -o $@ $< $(LDLIBS_libxenevtchn) $(LDLIBS_libxenctrl) $(LDLIBS_libxenstore) $(APPEND_LDFLAGS)
-xencov: xencov.o
- $(CC) $(LDFLAGS) -o $@ $< $(LDLIBS_libxenctrl) $(APPEND_LDFLAGS)
-
-include $(DEPS)
diff --git a/tools/misc/xencov.c b/tools/misc/xencov.c
deleted file mode 100644
index 2aafb1d..0000000
--- a/tools/misc/xencov.c
+++ /dev/null
@@ -1,147 +0,0 @@
-/*
- * xencov: handle test coverage information from Xen.
- *
- * Copyright (c) 2013, Citrix Systems R&D Ltd.
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms and conditions of the GNU General Public License,
- * version 2, as published by the Free Software Foundation.
- *
- * This program is distributed in the hope it will be useful, but WITHOUT
- * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
- * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
- * more details.
- *
- * You should have received a copy of the GNU General Public License along with
- * this program; If not, see <http://www.gnu.org/licenses/>.
- */
-
-#include <xenctrl.h>
-#include <stdio.h>
-#include <stdlib.h>
-#include <string.h>
-#include <unistd.h>
-#include <errno.h>
-#include <err.h>
-
-static xc_interface *gcov_xch = NULL;
-
-static void gcov_init(void)
-{
- gcov_xch = xc_interface_open(NULL, NULL, 0);
- if ( !gcov_xch )
- err(1, "opening interface");
-}
-
-int gcov_get_info(int op, struct xen_sysctl *sys, struct xc_hypercall_buffer *ptr)
-{
- struct xen_sysctl_coverage_op *cov;
- DECLARE_HYPERCALL_BUFFER_ARGUMENT(ptr);
-
- memset(sys, 0, sizeof(*sys));
- sys->cmd = XEN_SYSCTL_coverage_op;
-
- cov = &sys->u.coverage_op;
- cov->cmd = op;
- set_xen_guest_handle(cov->u.raw_info, ptr);
-
- return xc_sysctl(gcov_xch, sys);
-}
-
-static void gcov_read(const char *fn, int reset)
-{
- struct xen_sysctl sys;
- uint32_t total_len;
- DECLARE_HYPERCALL_BUFFER(uint8_t, p);
- FILE *f;
- int op = reset ? XEN_SYSCTL_COVERAGE_read_and_reset :
- XEN_SYSCTL_COVERAGE_read;
-
- /* get total length */
- if ( gcov_get_info(XEN_SYSCTL_COVERAGE_get_total_size, &sys, NULL) < 0 )
- err(1, "getting total length");
- total_len = sys.u.coverage_op.u.total_size;
- fprintf(stderr, "returned %u bytes\n", (unsigned) total_len);
-
- /* safe check */
- if ( total_len > 16u * 1024u * 1024u )
- errx(1, "coverage size too big %u bytes\n", total_len);
-
- /* allocate */
- p = xc_hypercall_buffer_alloc(gcov_xch, p, total_len);
- if ( p == NULL )
- err(1, "allocating memory for coverage");
-
- /* get data */
- memset(p, 0, total_len);
- if ( gcov_get_info(op, &sys, HYPERCALL_BUFFER(p)) < 0 )
- err(1, "getting coverage information");
-
- /* write to a file */
- if ( strcmp(fn, "-") == 0 )
- f = stdout;
- else
- f = fopen(fn, "w");
- if ( !f )
- err(1, "opening output file");
- if ( fwrite(p, 1, total_len, f) != total_len )
- err(1, "writing coverage to file");
- if (f != stdout)
- fclose(f);
- xc_hypercall_buffer_free(gcov_xch, p);
-}
-
-static void gcov_reset(void)
-{
- struct xen_sysctl sys;
-
- if ( gcov_get_info(XEN_SYSCTL_COVERAGE_reset, &sys, NULL) < 0 )
- err(1, "resetting coverage information");
-}
-
-static void usage(int exit_code)
-{
- FILE *out = exit_code ? stderr : stdout;
-
- fprintf(out, "xencov {reset|read|read-reset} [<filename>]\n"
- "\treset reset information\n"
- "\tread read information from xen to filename\n"
- "\tread-reset read and reset information from xen to filename\n"
- "\tfilename optional filename (default output)\n"
- );
- exit(exit_code);
-}
-
-int main(int argc, char **argv)
-{
- int opt;
-
- while ((opt = getopt(argc, argv, "h")) != -1) {
- switch (opt) {
- case 'h':
- usage(0);
- break;
- default:
- usage(1);
- }
- }
-
- argv += optind;
- argc -= optind;
- if (argc <= 0)
- usage(1);
-
- gcov_init();
-
- if ( strcmp(argv[0], "reset") == 0 )
- gcov_reset();
- else if ( strcmp(argv[0], "read") == 0 )
- gcov_read(argc > 1 ? argv[1] : "-", 0);
- else if ( strcmp(argv[0], "read-reset") == 0 )
- gcov_read(argc > 1 ? argv[1] : "-", 1);
- else
- usage(1);
-
- return 0;
-}
-
diff --git a/tools/misc/xencov_split b/tools/misc/xencov_split
deleted file mode 100755
index b11f27c..0000000
--- a/tools/misc/xencov_split
+++ /dev/null
@@ -1,188 +0,0 @@
-#!/usr/bin/perl
-
-# xencov_split - split coverage information from Xen
-#
-# Copyright (C) 2013 - Citrix Systems
-# -----
-#
-# This program is free software; you can redistribute it and/or modify
-# it under the terms of the GNU General Public License as published by
-# the Free Software Foundation; either version 2 of the License, or
-# (at your option) any later version.
-#
-# This program is distributed in the hope that it will be useful,
-# but WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
-# GNU General Public License for more details.
-#
-# You should have received a copy of the GNU General Public License
-# along with this program; If not, see <http://www.gnu.org/licenses/>.
-
-use strict;
-use File::Path qw(mkpath);
-
-# some magic constants
-my $magic = 0x67636461;
-my $ctrBase = 0x01a10000;
-
-my $xenMagic = 0x58544346; # file header
-my $xenTagFunc = 0x58544366; # functions tag
-my $xenTagCount0 = 0x58544330; # counter 0 tag
-my $xenTagEnd = 0x5854432e; # end file
-
-# open input file
-if ($ARGV[0]) {
- my $fn = $ARGV[0];
- open(IN, '<', $fn) or die "opening file \"$fn\"";
-} else {
- open(IN, '<&STDIN') or die "redirecting input";
-}
-
-my $pos = 0;
-
-sub getRaw($)
-{
- my $l = shift;
- die 'got no data to read' if $l < 0;
- my $res = '';
- do {
- my $data;
- my $r = read(IN, $data, $l);
- die "error $! reading data from input at position $pos" if !defined($r);
- die "unexpected end of file at position $pos" if !$r;
- $l -= $r;
- $pos += $r;
- $res .= $data;
- } while ($l > 0);
- return $res;
-}
-
-sub get32()
-{
- return unpack('V', getRaw(4));
-}
-
-sub get64()
-{
- # This is returned as raw data as some Perl version could not
- # support 64 bit integer
- # This is ok for little endian machines
- return getRaw(8);
-}
-
-sub align()
-{
- my $l = $pos & 7;
- getRaw(8-$l) if $l;
-}
-
-# read a string prefixed by length
-sub getS()
-{
- my $l = get32();
- my $res = getRaw($l);
- align();
- return $res;
-}
-
-sub parseFunctions($)
-{
- my $numCounters = shift;
- my $num = get32();
-
- my @funcs;
- for my $n (1..$num) {
- my @data;
- my $ident = get32();
- my $checksum = get32();
- for my $n (1..$numCounters) {
- push @data, get32(); # number of counters for a type
- }
- push @funcs, [$ident, $checksum, \@data];
- }
- align();
- return @funcs;
-}
-
-sub parseCounters($)
-{
- my $tag = shift;
- die sprintf("wrong tag 0x%08x pos $pos (0x%08x)", $tag, $pos) if $tag < $xenTagCount0;
- $tag -= $xenTagCount0;
- die sprintf('wrong tag 0x%08x', $tag) if $tag > 5;
- my $data = '';
-
- my $num = get32();
- for my $n (1..$num) {
- $data .= get64();
- }
- align();
- return [$tag, $data];
-}
-
-
-sub parseFile()
-{
- my $ver = get32();
- my $stamp = get32();
- my $fn = getS();
- align();
-
- my $numCounters;
-
- print "got file $fn\n";
- die if $fn !~ m,^(/.*?)[^/]+\.gcda$,;
- mkpath(".$1");
- open(OUT, '>', ".$fn") or die;
-
- print OUT pack('VVV', $magic, $ver, $stamp);
-
- # read counters of file
- my @ctrs;
- my @funcs;
- my $tag;
- for (;;) {
- $tag = get32();
- last if ($tag == $xenMagic || $tag == $xenTagEnd);
- if ($tag == $xenTagFunc) {
- die if scalar(@funcs);
- @funcs = parseFunctions(scalar(@ctrs));
- next;
- }
-
- # must be a counter
- push @ctrs, parseCounters($tag);
- ++$numCounters;
- }
-
- # print all functions
- for my $f (@funcs) {
- # tag tag_len ident checksum
- print OUT pack('VVVV', 0x01000000, 2, $f->[0], $f->[1]);
- # all counts
- my $n = 0;
- for my $c (@{$f->[2]}) {
- my ($type, $data) = @{$ctrs[$n]};
- print OUT pack('VV', $ctrBase + 0x20000 * $type, $c*2);
- die "--$c--$type--$data--" if length($data) < $c * 8;
- print OUT substr($data, 0, $c * 8);
- $ctrs[$n] = [$type, substr($data, $c * 8)];
- ++$n;
- }
- }
- close(OUT);
-
- return $tag;
-}
-
-my $tag = get32();
-die 'no coverage or wrong file format' if $tag != $xenMagic;
-for (;;) {
- if ($tag == $xenMagic) {
- $tag = parseFile();
- } elsif ($tag == $xenTagEnd) {
- last;
- } else {
- die "wrong tag $tag";
- }
-}
diff --git a/xen/Kconfig.debug b/xen/Kconfig.debug
index e9f7dcd..ef863dc 100644
--- a/xen/Kconfig.debug
+++ b/xen/Kconfig.debug
@@ -30,6 +30,7 @@ config FRAME_POINTER
config GCOV
bool "Gcov Support"
+ depends on BROKEN
---help---
Enable gcov (a test coverage program in GCC) support.
diff --git a/xen/common/gcov/Makefile b/xen/common/gcov/Makefile
index c9efe6c..e69de29 100644
--- a/xen/common/gcov/Makefile
+++ b/xen/common/gcov/Makefile
@@ -1,2 +0,0 @@
-obj-y += gcov.o
-
diff --git a/xen/common/gcov/gcov.c b/xen/common/gcov/gcov.c
deleted file mode 100644
index b5717b9..0000000
--- a/xen/common/gcov/gcov.c
+++ /dev/null
@@ -1,225 +0,0 @@
-/*
- * 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>
-#include <xen/guest_access.h>
-#include <public/xen.h>
-#include <public/gcov.h>
-
-static struct gcov_info *info_list;
-
-/*
- * __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;
-}
-
-/*
- * 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. */
-}
-
-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 += data_len;
- 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 inline void align_iter(write_iter_t *iter)
-{
- iter->write_offset =
- (iter->write_offset + sizeof(uint64_t) - 1) & -sizeof(uint64_t);
-}
-
-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);
-
- align_iter(iter);
- chk(write32(iter, XENCOV_TAG_FILE));
- chk(write32(iter, info->version));
- chk(write32(iter, info->stamp));
- chk(write_string(iter, info->filename));
-
- /* dump counters */
- ctr = info->counts;
- for ( type = -1; next_type(info, &type) < XENCOV_COUNTERS; ++ctr )
- {
- align_iter(iter);
- 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 */
- align_iter(iter);
- 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 */
- align_iter(iter);
- 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)
-{
- int ret = -EINVAL;
- write_iter_t iter;
-
- switch ( op->cmd )
- {
- case XEN_SYSCTL_COVERAGE_get_total_size:
- iter.real = 0;
-
- write_gcov(&iter);
- op->u.total_size = iter.write_offset;
- ret = 0;
- break;
-
- case XEN_SYSCTL_COVERAGE_read_and_reset:
- case XEN_SYSCTL_COVERAGE_read:
- iter.ptr = op->u.raw_info;
- iter.real = 1;
-
- ret = write_gcov(&iter);
- if ( ret || op->cmd != XEN_SYSCTL_COVERAGE_read_and_reset )
- break;
-
- /* fall through */
- case XEN_SYSCTL_COVERAGE_reset:
- ret = reset_counters();
- break;
- }
- return ret;
-}
-
-/*
- * Local variables:
- * mode: C
- * c-file-style: "BSD"
- * c-basic-offset: 4
- * tab-width: 4
- * indent-tabs-mode: nil
- * End:
- */
diff --git a/xen/common/sysctl.c b/xen/common/sysctl.c
index 8aea6ef..2cac451 100644
--- a/xen/common/sysctl.c
+++ b/xen/common/sysctl.c
@@ -27,7 +27,6 @@
#include <xen/nodemask.h>
#include <xsm/xsm.h>
#include <xen/pmstat.h>
-#include <xen/gcov.h>
#include <xen/livepatch.h>
long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
@@ -396,12 +395,6 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
}
break;
-#ifdef CONFIG_GCOV
- case XEN_SYSCTL_coverage_op:
- ret = sysctl_coverage_op(&op->u.coverage_op);
- break;
-#endif
-
#ifdef CONFIG_HAS_PCI
case XEN_SYSCTL_pcitopoinfo:
{
diff --git a/xen/include/public/gcov.h b/xen/include/public/gcov.h
deleted file mode 100644
index 1b29b48..0000000
--- a/xen/include/public/gcov.h
+++ /dev/null
@@ -1,115 +0,0 @@
-/******************************************************************************
- * 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, Citrix Systems R&D Ltd.
- */
-
-#ifndef __XEN_PUBLIC_GCOV_H__
-#define __XEN_PUBLIC_GCOV_H__ __XEN_PUBLIC_GCOV_H__
-
-#define XENCOV_COUNTERS 5
-#define XENCOV_TAG_BASE 0x58544300u
-#define XENCOV_TAG_FILE (XENCOV_TAG_BASE+0x46u)
-#define XENCOV_TAG_FUNC (XENCOV_TAG_BASE+0x66u)
-#define XENCOV_TAG_COUNTER(n) (XENCOV_TAG_BASE+0x30u+((n)&0xfu))
-#define XENCOV_TAG_END (XENCOV_TAG_BASE+0x2eu)
-#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 VERSION STAMP FILENAME 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 FUNCTION..
- * FUNCTION := IDENT CHECKSUM NUM_COUNTERS
- *
- * All tagged structures are aligned to 8 bytes
- */
-
-/**
- * File information
- * Prefixed with XENCOV_TAG_FILE and a string with filename
- * Aligned to 8 bytes
- */
-struct xencov_file
-{
- uint32_t tag; /* XENCOV_TAG_FILE */
- uint32_t version;
- uint32_t stamp;
- uint32_t fn_len;
- char filename[1];
-};
-
-
-/**
- * Counters information
- * Prefixed with XENCOV_TAG_COUNTER(n) where n is 0..(XENCOV_COUNTERS-1)
- * Aligned to 8 bytes
- */
-struct xencov_counter
-{
- uint32_t tag; /* XENCOV_TAG_COUNTER(n) */
- uint32_t num;
- uint64_t values[1];
-};
-
-/**
- * Information for each function
- * Number of counter is equal to the number of counter structures got before
- */
-struct xencov_function
-{
- uint32_t ident;
- uint32_t checksum;
- uint32_t num_counters[1];
-};
-
-/**
- * Information for all functions
- * Aligned to 8 bytes
- */
-struct xencov_functions
-{
- uint32_t tag; /* XENCOV_TAG_FUNC */
- uint32_t num;
- struct xencov_function xencov_function[1];
-};
-
-/**
- * Terminator
- */
-struct xencov_end
-{
- uint32_t tag; /* XENCOV_TAG_END */
-};
-
-#endif /* __XEN_PUBLIC_GCOV_H__ */
-
diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
index 418f4bb..a939fa9 100644
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -665,41 +665,6 @@ 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 */
-/*
- * 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 0
-
-/*
- * Read coverage information in a single run
- * You must use a tool to split them.
- */
-#define XEN_SYSCTL_COVERAGE_read 1
-
-/*
- * Reset all the coverage counters to 0
- * No parameters.
- */
-#define XEN_SYSCTL_COVERAGE_reset 2
-
-/*
- * Like XEN_SYSCTL_COVERAGE_read but reset also
- * counters to 0 in a single call.
- */
-#define XEN_SYSCTL_COVERAGE_read_and_reset 3
-
-struct xen_sysctl_coverage_op {
- uint32_t cmd; /* XEN_SYSCTL_COVERAGE_* */
- union {
- uint32_t 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);
-
#define XEN_SYSCTL_PSR_CMT_get_total_rmid 0
#define XEN_SYSCTL_PSR_CMT_get_l3_upscaling_factor 1
/* The L3 cache size is returned in KB unit */
@@ -1108,7 +1073,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
+/* #define XEN_SYSCTL_coverage_op 20 */
#define XEN_SYSCTL_psr_cmt_op 21
#define XEN_SYSCTL_pcitopoinfo 22
#define XEN_SYSCTL_psr_cat_op 23
@@ -1137,7 +1102,6 @@ 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;
struct xen_sysctl_psr_cmt_op psr_cmt_op;
struct xen_sysctl_psr_cat_op psr_cat_op;
struct xen_sysctl_tmem_op tmem_op;
diff --git a/xen/include/xen/gcov.h b/xen/include/xen/gcov.h
deleted file mode 100644
index a7d4a35..0000000
--- a/xen/include/xen/gcov.h
+++ /dev/null
@@ -1,93 +0,0 @@
-/*
- * 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__
-
-#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
- * 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];
-};
-
-
-/**
- * Sysctl operations for coverage
- */
-#ifdef CONFIG_GCOV
-int sysctl_coverage_op(xen_sysctl_coverage_op_t *op);
-#endif
-
-#endif /* __XEN_GCOV_H__ */
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 58+ messages in thread* Re: [PATCH v2 4/9] xen, tools: rip out old gcov implementation
2016-10-10 9:40 ` [PATCH v2 4/9] xen, tools: rip out old gcov implementation Wei Liu
@ 2016-10-10 11:24 ` Jan Beulich
0 siblings, 0 replies; 58+ messages in thread
From: Jan Beulich @ 2016-10-10 11:24 UTC (permalink / raw)
To: Wei Liu
Cc: Stefano Stabellini, George Dunlap, AndrewCooper, Ian Jackson,
Tim Deegan, philippe.gabriel, Xen-devel
>>> On 10.10.16 at 11:40, <wei.liu2@citrix.com> wrote:
> The internal data structure and code are tied to an old gcov format.
> It's easier to just redo everything from scratch.
>
> Salvage the reusable parts: leave xen/common/gcov and an empty Makefile
> there, leave gcov support in Kconfig but mark that as broken. Also
> reserve the sysctl number for later use (but delete relevant sysctl
> structures).
>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 58+ messages in thread
* [PATCH v2 5/9] gcov: add new interface and 3.4 and 4.7 format support
2016-10-10 9:40 [PATCH v2 0/9] Rework gcov support in Xen Wei Liu
` (3 preceding siblings ...)
2016-10-10 9:40 ` [PATCH v2 4/9] xen, tools: rip out old gcov implementation Wei Liu
@ 2016-10-10 9:40 ` Wei Liu
2016-10-10 11:56 ` Jan Beulich
2016-10-11 10:31 ` [PATCH v3] " Wei Liu
2016-10-10 9:40 ` [PATCH v2 6/9] gcov: userspace tools to extract and split gcov data Wei Liu
` (4 subsequent siblings)
9 siblings, 2 replies; 58+ messages in thread
From: Wei Liu @ 2016-10-10 9:40 UTC (permalink / raw)
To: Xen-devel
Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper,
Ian Jackson, Tim Deegan, philippe.gabriel, Jan Beulich
A new sysctl interface for passing gcov data back to userspace. The new
interface uses a customised record file format. The new sysctl reuses
original sysctl number but renames the op to gcov_op.
Both gcc 3.4 and 4.7 format are supported. The code is rewritten so that
a new format can be easily added in the future. Version specific code
is grouped into different files. The format one needs to use can be
picked via Kconfig. The default format is 4.7 format.
Userspace programs to handle extracted data will come in a later patch.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
v2:
1. Use tab in Kconfig and indent help text properly.
2. Bump XEN_SYSCTL_INTERFACE_VERSION.
3. Fix cosmetic issues.
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>
---
xen/Kconfig.debug | 24 ++-
xen/common/gcov/Makefile | 3 +
xen/common/gcov/gcc_3_4.c | 363 ++++++++++++++++++++++++++++++++++++++++++++
xen/common/gcov/gcc_4_7.c | 201 ++++++++++++++++++++++++
xen/common/gcov/gcov.c | 257 +++++++++++++++++++++++++++++++
xen/common/gcov/gcov.h | 36 +++++
xen/common/gcov/gcov_base.c | 64 ++++++++
xen/common/sysctl.c | 8 +
xen/include/public/sysctl.h | 39 ++++-
xen/include/xen/gcov.h | 9 ++
10 files changed, 997 insertions(+), 7 deletions(-)
create mode 100644 xen/common/gcov/gcc_3_4.c
create mode 100644 xen/common/gcov/gcc_4_7.c
create mode 100644 xen/common/gcov/gcov.c
create mode 100644 xen/common/gcov/gcov.h
create mode 100644 xen/common/gcov/gcov_base.c
create mode 100644 xen/include/xen/gcov.h
diff --git a/xen/Kconfig.debug b/xen/Kconfig.debug
index ef863dc..c65e543 100644
--- a/xen/Kconfig.debug
+++ b/xen/Kconfig.debug
@@ -30,16 +30,30 @@ config FRAME_POINTER
config GCOV
bool "Gcov Support"
- depends on BROKEN
---help---
Enable gcov (a test coverage program in GCC) support.
- Currently the data structure and hypercall interface are tied
- to GCC 3.4 gcov format. You need to have a version of GCC
- that is compatible with that format to make gcov work.
-
If unsure, say N here.
+choice
+ prompt "Specify Gcov format"
+ depends on GCOV
+ default GCOV_FORMAT_4_7
+ ---help---
+ The gcov format is determined by gcc version.
+
+config GCOV_FORMAT_4_7
+ bool "GCC 4.7 format"
+ ---help---
+ Select this option to use the format specified in GCC 4.7.
+
+config GCOV_FORMAT_3_4
+ bool "GCC 3.4 format"
+ ---help---
+ Select this option to use the format specified in GCC 3.4.
+
+endchoice
+
config LOCK_PROFILE
bool "Lock Profiling"
---help---
diff --git a/xen/common/gcov/Makefile b/xen/common/gcov/Makefile
index e69de29..03ac1e5 100644
--- a/xen/common/gcov/Makefile
+++ b/xen/common/gcov/Makefile
@@ -0,0 +1,3 @@
+obj-y += gcov_base.o gcov.o
+obj-$(CONFIG_GCOV_FORMAT_3_4) += gcc_3_4.o
+obj-$(CONFIG_GCOV_FORMAT_4_7) += gcc_4_7.o
diff --git a/xen/common/gcov/gcc_3_4.c b/xen/common/gcov/gcc_3_4.c
new file mode 100644
index 0000000..2b62b3f
--- /dev/null
+++ b/xen/common/gcov/gcc_3_4.c
@@ -0,0 +1,363 @@
+/*
+ * This code provides functions to handle gcc's profiling data format
+ * introduced with gcc 3.4. Future versions of gcc may change the gcov
+ * format (as happened before), so all format-specific information needs
+ * to be kept modular and easily exchangeable.
+ *
+ * This file is based on gcc-internal definitions. Functions and 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.
+ *
+ * Imported from Linux and modified for Xen by
+ * Wei Liu <wei.liu2@citrix.com>
+ */
+
+
+#include <xen/lib.h>
+
+#include "gcov.h"
+
+#define GCOV_COUNTERS 5
+
+static struct gcov_info *gcov_info_head;
+
+/**
+ * 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];
+};
+
+/**
+ * struct type_info - iterator helper array
+ * @ctr_type: counter type
+ * @offset: index of the first value of the current function for this type
+ *
+ * This array is needed to convert the in-memory data format into the in-file
+ * data format:
+ *
+ * In-memory:
+ * for each counter type
+ * for each function
+ * values
+ *
+ * In-file:
+ * for each function
+ * for each counter type
+ * values
+ *
+ * See gcc source gcc/gcov-io.h for more information on data organization.
+ */
+struct type_info {
+ int ctr_type;
+ unsigned int offset;
+};
+
+/**
+ * struct gcov_iterator - specifies current file position in logical records
+ * @info: associated profiling data
+ * @record: record type
+ * @function: function number
+ * @type: counter type
+ * @count: index into values array
+ * @num_types: number of counter types
+ * @type_info: helper array to get values-array offset for current function
+ */
+struct gcov_iterator {
+ const struct gcov_info *info;
+
+ int record;
+ unsigned int function;
+ unsigned int type;
+ unsigned int count;
+
+ int num_types;
+ struct type_info type_info[GCOV_COUNTERS];
+};
+
+/* Mapping of logical record number to actual file content. */
+#define RECORD_FILE_MAGIC 0
+#define RECORD_GCOV_VERSION 1
+#define RECORD_TIME_STAMP 2
+#define RECORD_FUNCTION_TAG 3
+#define RECORD_FUNCTON_TAG_LEN 4
+#define RECORD_FUNCTION_IDENT 5
+#define RECORD_FUNCTION_CHECK 6
+#define RECORD_COUNT_TAG 7
+#define RECORD_COUNT_LEN 8
+#define RECORD_COUNT 9
+
+static int counter_active(const struct gcov_info *info, unsigned int type)
+{
+ return (1 << type) & info->ctr_mask;
+}
+
+static unsigned int num_counter_active(const struct gcov_info *info)
+{
+ unsigned int i;
+ unsigned int result = 0;
+
+ for ( i = 0; i < GCOV_COUNTERS; i++ )
+ if ( counter_active(info, i) )
+ result++;
+
+ return result;
+}
+
+void gcov_info_link(struct gcov_info *info)
+{
+ info->next = gcov_info_head;
+ gcov_info_head = info;
+}
+
+struct gcov_info *gcov_info_next(const struct gcov_info *info)
+{
+ if ( !info )
+ return gcov_info_head;
+
+ return info->next;
+}
+
+const char *gcov_info_filename(const struct gcov_info *info)
+{
+ return info->filename;
+}
+
+void gcov_info_reset(struct gcov_info *info)
+{
+ unsigned int active = num_counter_active(info);
+ unsigned int i;
+
+ for ( i = 0; i < active; i++ )
+ memset(info->counts[i].values, 0,
+ info->counts[i].num * sizeof(gcov_type));
+}
+
+static size_t get_fn_size(const struct gcov_info *info)
+{
+ size_t size;
+
+ size = sizeof(struct gcov_fn_info) + num_counter_active(info) *
+ sizeof(unsigned int);
+ if ( __alignof__(struct gcov_fn_info) > sizeof(unsigned int) )
+ size = ROUNDUP(size, __alignof__(struct gcov_fn_info));
+ return size;
+}
+
+static struct gcov_fn_info *get_fn_info(const struct gcov_info *info,
+ unsigned int fn)
+{
+ return (struct gcov_fn_info *)
+ ((char *) info->functions + fn * get_fn_size(info));
+}
+
+static struct gcov_fn_info *get_func(struct gcov_iterator *iter)
+{
+ return get_fn_info(iter->info, iter->function);
+}
+
+static struct type_info *get_type(struct gcov_iterator *iter)
+{
+ return &iter->type_info[iter->type];
+}
+
+/**
+ * gcov_iter_next - advance file iterator to next logical record
+ * @iter: file iterator
+ *
+ * Return zero if new position is valid, non-zero if iterator has reached end.
+ */
+static int gcov_iter_next(struct gcov_iterator *iter)
+{
+ switch ( iter->record )
+ {
+ case RECORD_FILE_MAGIC:
+ case RECORD_GCOV_VERSION:
+ case RECORD_FUNCTION_TAG:
+ case RECORD_FUNCTON_TAG_LEN:
+ case RECORD_FUNCTION_IDENT:
+ case RECORD_COUNT_TAG:
+ /* Advance to next record */
+ iter->record++;
+ break;
+ case RECORD_COUNT:
+ /* Advance to next count */
+ iter->count++;
+ /* fall through */
+ case RECORD_COUNT_LEN:
+ if ( iter->count < get_func(iter)->n_ctrs[iter->type] )
+ {
+ iter->record = 9;
+ break;
+ }
+ /* Advance to next counter type */
+ get_type(iter)->offset += iter->count;
+ iter->count = 0;
+ iter->type++;
+ /* fall through */
+ case RECORD_FUNCTION_CHECK:
+ if ( iter->type < iter->num_types )
+ {
+ iter->record = 7;
+ break;
+ }
+ /* Advance to next function */
+ iter->type = 0;
+ iter->function++;
+ /* fall through */
+ case RECORD_TIME_STAMP:
+ if ( iter->function < iter->info->n_functions )
+ iter->record = 3;
+ else
+ iter->record = -1;
+ break;
+ }
+ /* Check for EOF. */
+ if ( iter->record == -1 )
+ return -EINVAL;
+ else
+ return 0;
+}
+
+/**
+ * gcov_iter_write - write data to buffer
+ * @iter: file iterator
+ * @buf: buffer to write to, if it is NULL, nothing is written
+ * @pos: position inside buffer to start writing
+ *
+ * Return number of bytes written into buffer.
+ */
+static size_t gcov_iter_write(struct gcov_iterator *iter, char *buf,
+ size_t pos)
+{
+ size_t ret = 0;
+
+ switch ( iter->record )
+ {
+ case RECORD_FILE_MAGIC:
+ ret = gcov_store_u32(buf, pos, GCOV_DATA_MAGIC);
+ break;
+ case RECORD_GCOV_VERSION:
+ ret = gcov_store_u32(buf, pos, iter->info->version);
+ break;
+ case RECORD_TIME_STAMP:
+ ret = gcov_store_u32(buf, pos, iter->info->stamp);
+ break;
+ case RECORD_FUNCTION_TAG:
+ ret = gcov_store_u32(buf, pos, GCOV_TAG_FUNCTION);
+ break;
+ case RECORD_FUNCTON_TAG_LEN:
+ ret = gcov_store_u32(buf, pos, 2);
+ break;
+ case RECORD_FUNCTION_IDENT:
+ ret = gcov_store_u32(buf, pos, get_func(iter)->ident);
+ break;
+ case RECORD_FUNCTION_CHECK:
+ ret = gcov_store_u32(buf, pos, get_func(iter)->checksum);
+ break;
+ case RECORD_COUNT_TAG:
+ ret = gcov_store_u32(buf, pos,
+ GCOV_TAG_FOR_COUNTER(get_type(iter)->ctr_type));
+ break;
+ case RECORD_COUNT_LEN:
+ ret = gcov_store_u32(buf, pos,
+ get_func(iter)->n_ctrs[iter->type] * 2);
+ break;
+ case RECORD_COUNT:
+ ret = gcov_store_u64(buf, pos, iter->info->counts[iter->type].
+ values[iter->count + get_type(iter)->offset]);
+ break;
+ }
+
+ return ret;
+}
+
+/* If buffer is NULL, no data is written. */
+size_t gcov_info_to_gcda(char *buffer, const struct gcov_info *info)
+{
+ struct gcov_iterator iter = { .info = info };
+ unsigned int i;
+ size_t pos = 0;
+
+ for ( i = 0; i < GCOV_COUNTERS; i++ )
+ {
+ if ( counter_active(info, i) )
+ {
+ iter.type_info[iter.num_types].ctr_type = i;
+ iter.type_info[iter.num_types].offset = 0;
+ iter.num_types++;
+ }
+ }
+
+ do {
+ pos += gcov_iter_write(&iter, buffer, pos);
+ } while ( gcov_iter_next(&iter) == 0 );
+
+ return pos;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/common/gcov/gcc_4_7.c b/xen/common/gcov/gcc_4_7.c
new file mode 100644
index 0000000..0906360
--- /dev/null
+++ b/xen/common/gcov/gcc_4_7.c
@@ -0,0 +1,201 @@
+/*
+ * This code provides functions to handle gcc's profiling data format
+ * introduced with gcc 4.7.
+ *
+ * This file is based heavily on gcc_3_4.c file.
+ *
+ * For a better understanding, refer to gcc source:
+ * gcc/gcov-io.h
+ * libgcc/libgcov.c
+ *
+ * Uses gcc-internal data definitions.
+ *
+ * Imported from Linux and modified for Xen by
+ * Wei Liu <wei.liu2@citrix.com>
+ */
+
+#include <xen/string.h>
+
+#include "gcov.h"
+
+#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
+
+#define GCOV_TAG_FUNCTION_LENGTH 3
+
+static struct gcov_info *gcov_info_head;
+
+/**
+ * struct gcov_ctr_info - information about counters for a single function
+ * @num: number of counter values for this type
+ * @values: array of counter values for this type
+ *
+ * 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;
+};
+
+/**
+ * struct gcov_fn_info - profiling meta data per function
+ * @key: comdat key
+ * @ident: unique ident of function
+ * @lineno_checksum: function lineo_checksum
+ * @cfg_checksum: function cfg checksum
+ * @ctrs: instrumented counters
+ *
+ * This data is generated by gcc during compilation and doesn't change
+ * at run-time.
+ *
+ * Information about a single function. This uses the trailing array
+ * idiom. The number of counters is determined from the merge pointer
+ * array in gcov_info. The key is used to detect which of a set of
+ * comdat functions was selected -- it points to the gcov_info object
+ * of the object file containing the selected comdat function.
+ */
+struct gcov_fn_info {
+ const struct gcov_info *key;
+ unsigned int ident;
+ unsigned int lineno_checksum;
+ unsigned int cfg_checksum;
+ struct gcov_ctr_info ctrs[0];
+};
+
+/**
+ * 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: uniquifying time stamp
+ * @filename: name of the associated gcov data file
+ * @merge: merge functions (null for unused counter type)
+ * @n_functions: number of instrumented functions
+ * @functions: pointer to pointers to function information
+ *
+ * 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;
+ void (*merge[GCOV_COUNTERS])(gcov_type *, unsigned int);
+ unsigned int n_functions;
+ struct gcov_fn_info **functions;
+};
+
+static int counter_active(const struct gcov_info *info, unsigned int type)
+{
+ return info->merge[type] ? 1 : 0;
+}
+
+void gcov_info_link(struct gcov_info *info)
+{
+ info->next = gcov_info_head;
+ gcov_info_head = info;
+}
+
+struct gcov_info *gcov_info_next(const struct gcov_info *info)
+{
+ if ( !info )
+ return gcov_info_head;
+ return info->next;
+}
+
+void gcov_info_reset(struct gcov_info *info)
+{
+ struct gcov_ctr_info *ci_ptr;
+ unsigned int fi_idx;
+ unsigned int ct_idx;
+
+ for ( fi_idx = 0; fi_idx < info->n_functions; fi_idx++ )
+ {
+ ci_ptr = info->functions[fi_idx]->ctrs;
+
+ for ( ct_idx = 0; ct_idx < GCOV_COUNTERS; ct_idx++ )
+ {
+ if ( !counter_active(info, ct_idx) )
+ continue;
+
+ memset(ci_ptr->values, 0, sizeof(gcov_type) * ci_ptr->num);
+ ci_ptr++;
+ }
+ }
+}
+
+const char *gcov_info_filename(const struct gcov_info *info)
+{
+ return info->filename;
+}
+
+
+/**
+ * gcov_info_to_gcda - convert profiling data set to gcda file format
+ * @buffer: the buffer to store file data or %NULL if no data should be stored
+ * @info: profiling data set to be converted
+ *
+ * Returns the number of bytes that were/would have been stored into the buffer.
+ */
+size_t gcov_info_to_gcda(char *buffer, const struct gcov_info *info)
+{
+ struct gcov_fn_info *fi_ptr;
+ struct gcov_ctr_info *ci_ptr;
+ unsigned int fi_idx;
+ unsigned int ct_idx;
+ unsigned int cv_idx;
+ size_t pos = 0;
+
+ /* File header. */
+ pos += gcov_store_u32(buffer, pos, GCOV_DATA_MAGIC);
+ pos += gcov_store_u32(buffer, pos, info->version);
+ pos += gcov_store_u32(buffer, pos, info->stamp);
+
+ for ( fi_idx = 0; fi_idx < info->n_functions; fi_idx++ )
+ {
+ fi_ptr = info->functions[fi_idx];
+
+ /* Function record. */
+ pos += gcov_store_u32(buffer, pos, GCOV_TAG_FUNCTION);
+ pos += gcov_store_u32(buffer, pos, GCOV_TAG_FUNCTION_LENGTH);
+ pos += gcov_store_u32(buffer, pos, fi_ptr->ident);
+ pos += gcov_store_u32(buffer, pos, fi_ptr->lineno_checksum);
+ pos += gcov_store_u32(buffer, pos, fi_ptr->cfg_checksum);
+
+ ci_ptr = fi_ptr->ctrs;
+
+ for ( ct_idx = 0; ct_idx < GCOV_COUNTERS; ct_idx++ )
+ {
+ if (! counter_active(info, ct_idx) )
+ continue;
+
+ /* Counter record. */
+ pos += gcov_store_u32(buffer, pos,
+ GCOV_TAG_FOR_COUNTER(ct_idx));
+ pos += gcov_store_u32(buffer, pos, ci_ptr->num * 2);
+
+ for ( cv_idx = 0; cv_idx < ci_ptr->num; cv_idx++ )
+ pos += gcov_store_u64(buffer, pos, ci_ptr->values[cv_idx]);
+
+ ci_ptr++;
+ }
+ }
+
+ return pos;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/common/gcov/gcov.c b/xen/common/gcov/gcov.c
new file mode 100644
index 0000000..88df36c
--- /dev/null
+++ b/xen/common/gcov/gcov.c
@@ -0,0 +1,257 @@
+/*
+ * 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
+ *
+ * Modified for Xen by:
+ * Wei Liu <wei.liu2@citrix.com>
+ */
+
+#include <xen/errno.h>
+#include <xen/guest_access.h>
+#include <xen/types.h>
+
+#include <public/sysctl.h>
+
+#include "gcov.h"
+
+/**
+ * gcov_store_u32 - store 32 bit number in gcov format to buffer
+ * @buffer: target buffer or NULL
+ * @off: offset into the buffer
+ * @v: value to be stored
+ *
+ * Number format defined by gcc: numbers are recorded in the 32 bit
+ * unsigned binary form of the endianness of the machine generating the
+ * file. Returns the number of bytes stored. If @buffer is %NULL, doesn't
+ * store anything.
+ */
+size_t gcov_store_u32(void *buffer, size_t off, u32 v)
+{
+ u32 *data;
+
+ if ( buffer )
+ {
+ data = buffer + off;
+ *data = v;
+ }
+
+ return sizeof(*data);
+}
+
+/**
+ * gcov_store_u64 - store 64 bit number in gcov format to buffer
+ * @buffer: target buffer or NULL
+ * @off: offset into the buffer
+ * @v: value to be stored
+ *
+ * Number format defined by gcc: numbers are recorded in the 32 bit
+ * unsigned binary form of the endianness of the machine generating the
+ * file. 64 bit numbers are stored as two 32 bit numbers, the low part
+ * first. Returns the number of bytes stored. If @buffer is %NULL, doesn't store
+ * anything.
+ */
+size_t gcov_store_u64(void *buffer, size_t off, u64 v)
+{
+ u32 *data;
+
+ if ( buffer )
+ {
+ data = buffer + off;
+
+ data[0] = (v & 0xffffffffUL);
+ data[1] = (v >> 32);
+ }
+
+ return sizeof(*data) * 2;
+}
+
+static size_t gcov_info_payload_size(const struct gcov_info *info)
+{
+ return gcov_info_to_gcda(NULL, info);
+}
+
+static int gcov_info_dump_payload(const struct gcov_info *info,
+ XEN_GUEST_HANDLE_PARAM(char) buffer,
+ uint32_t *off)
+{
+ char *buf;
+ uint32_t buf_size;
+ int ret;
+
+ /*
+ * Allocate a buffer and dump payload there. This helps us to not
+ * have copy_to_guest in other functions and retain their simple
+ * semantics.
+ */
+
+ buf_size = gcov_info_payload_size(info);
+ buf = xmalloc_array(char, buf_size);
+
+ if ( !buf )
+ {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ gcov_info_to_gcda(buf, info);
+
+ if ( copy_to_guest_offset(buffer, *off, buf, buf_size) )
+ {
+ ret = -EFAULT;
+ goto out;
+ }
+ *off += buf_size;
+
+ ret = 0;
+ out:
+ xfree(buf);
+ return ret;
+
+}
+
+static uint32_t gcov_get_size(void)
+{
+ uint32_t total_size;
+ struct gcov_info *info = NULL;
+
+ /* Magic number XCOV */
+ total_size = sizeof(uint32_t);
+
+ while ( (info = gcov_info_next(info)) )
+ {
+ /* File name length, including trailing \0 */
+ total_size += strlen(gcov_info_filename(info)) + 1;
+
+ /* Payload size field */
+ total_size += sizeof(uint32_t);
+
+ /* Payload itself */
+ total_size += gcov_info_payload_size(info);
+ }
+
+ return total_size;
+}
+
+static void gcov_reset_all_counters(void)
+{
+ struct gcov_info *info = NULL;
+
+ while ( (info = gcov_info_next(info)) )
+ gcov_info_reset(info);
+}
+
+static int gcov_dump_one_record(const struct gcov_info *info,
+ XEN_GUEST_HANDLE_PARAM(char) buffer,
+ uint32_t *off)
+{
+ uint32_t payload_size;
+ uint32_t len;
+
+ /* File name, including trailing \0 */
+ len = strlen(gcov_info_filename(info)) + 1;
+ if ( copy_to_guest_offset(buffer, *off, gcov_info_filename(info), len) )
+ return -EFAULT;
+ *off += len;
+
+ payload_size = gcov_info_payload_size(info);
+ /* Payload size */
+ if ( copy_to_guest_offset(buffer, *off, (char*)&payload_size,
+ sizeof(uint32_t)) )
+ return -EFAULT;
+ *off += sizeof(uint32_t);
+
+ /* Payload itself */
+ return gcov_info_dump_payload(info, buffer, off);
+}
+
+static int gcov_dump_all(XEN_GUEST_HANDLE_PARAM(char) buffer,
+ uint32_t *buffer_size)
+{
+ uint32_t off;
+ uint32_t magic = XEN_GCOV_FORMAT_MAGIC;
+ struct gcov_info *info = NULL;
+ int ret;
+
+ if ( *buffer_size < gcov_get_size() )
+ {
+ ret = -ENOBUFS;
+ goto out;
+ }
+
+ off = 0;
+
+ /* Magic number */
+ if ( copy_to_guest_offset(buffer, off, (char *)&magic, sizeof(magic)) )
+ {
+ ret = -EFAULT;
+ goto out;
+ }
+ off += sizeof(magic);
+
+ while ( (info = gcov_info_next(info)) )
+ {
+ ret = gcov_dump_one_record(info, buffer, &off);
+ if ( ret )
+ goto out;
+ }
+
+ *buffer_size = off;
+
+ ret = 0;
+ out:
+ return ret;
+}
+
+int sysctl_gcov_op(xen_sysctl_gcov_op_t *op)
+{
+ int ret;
+
+ switch ( op->cmd )
+ {
+ case XEN_SYSCTL_GCOV_get_size:
+ op->size = gcov_get_size();
+ ret = 0;
+ break;
+ case XEN_SYSCTL_GCOV_read:
+ {
+ XEN_GUEST_HANDLE_PARAM(char) buf;
+ uint32_t size = op->size;
+
+ buf = guest_handle_cast(op->buffer, char);
+
+ ret = gcov_dump_all(buf, &size);
+ op->size = size;
+
+ break;
+ }
+ case XEN_SYSCTL_GCOV_reset:
+ gcov_reset_all_counters();
+ ret = 0;
+ break;
+ default:
+ ret = -EINVAL;
+ break;
+ }
+
+ return ret;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/common/gcov/gcov.h b/xen/common/gcov/gcov.h
new file mode 100644
index 0000000..9a29763
--- /dev/null
+++ b/xen/common/gcov/gcov.h
@@ -0,0 +1,36 @@
+#ifndef _GCOV_H_
+#define _GCOV_H_
+
+#include <xen/guest_access.h>
+#include <xen/types.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.
+ */
+#define GCOV_DATA_MAGIC ((unsigned int) 0x67636461)
+#define GCOV_TAG_FUNCTION ((unsigned int) 0x01000000)
+#define GCOV_TAG_COUNTER_BASE ((unsigned int) 0x01a10000)
+#define GCOV_TAG_FOR_COUNTER(count) \
+ _TAG_COUNTER_BASE + ((unsigned int) (count) << 17))
+
+#if BITS_PER_LONG >= 64
+typedef long gcov_type;
+#else
+typedef long long gcov_type;
+#endif
+
+/* Opaque gcov_info -- tied to specific gcc gcov formats */
+struct gcov_info;
+
+void gcov_info_link(struct gcov_info *info);
+struct gcov_info *gcov_info_next(const struct gcov_info *info);
+void gcov_info_reset(struct gcov_info *info);
+const char *gcov_info_filename(const struct gcov_info *info);
+size_t gcov_info_to_gcda(char *buffer, const struct gcov_info *info);
+
+size_t gcov_store_u32(void *buffer, size_t off, u32 v);
+size_t gcov_store_u64(void *buffer, size_t off, u64 v);
+
+#endif /* _GCOV_H_ */
diff --git a/xen/common/gcov/gcov_base.c b/xen/common/gcov/gcov_base.c
new file mode 100644
index 0000000..d46a1b9
--- /dev/null
+++ b/xen/common/gcov/gcov_base.c
@@ -0,0 +1,64 @@
+/*
+ * Common code across gcov implementations
+ *
+ * Copyright Citrix Systems R&D UK
+ * Author(s): Wei Liu <wei.liu2@citrix.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 "gcov.h"
+
+/*
+ * __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)
+{
+ /* Link all gcov info together. */
+ gcov_info_link(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. */
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/common/sysctl.c b/xen/common/sysctl.c
index 2cac451..03cca5c 100644
--- a/xen/common/sysctl.c
+++ b/xen/common/sysctl.c
@@ -28,6 +28,7 @@
#include <xsm/xsm.h>
#include <xen/pmstat.h>
#include <xen/livepatch.h>
+#include <xen/gcov.h>
long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
{
@@ -395,6 +396,13 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
}
break;
+#ifdef CONFIG_GCOV
+ case XEN_SYSCTL_gcov_op:
+ ret = sysctl_gcov_op(&op->u.gcov_op);
+ copyback = 1;
+ break;
+#endif
+
#ifdef CONFIG_HAS_PCI
case XEN_SYSCTL_pcitopoinfo:
{
diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
index a939fa9..28ac56c 100644
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -36,7 +36,7 @@
#include "physdev.h"
#include "tmem.h"
-#define XEN_SYSCTL_INTERFACE_VERSION 0x0000000E
+#define XEN_SYSCTL_INTERFACE_VERSION 0x0000000F
/*
* Read console content from Xen buffer ring.
@@ -665,6 +665,40 @@ 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);
+/*
+ * Output format of gcov data:
+ *
+ * XEN_GCOV_FORMAT_MAGIC XEN_GCOV_RECORD ... XEN_GCOV_RECORD
+ *
+ * That is, one magic number followed by 0 or more record.
+ *
+ * The magic number is stored as an uint32_t field.
+ *
+ * The record is packed and variable in length. It has the form:
+ *
+ * filename: a NULL terminated path name extracted from gcov, used to
+ * create the name of gcda file.
+ * size: a uint32_t field indicating the size of the payload, the
+ * unit is byte.
+ * payload: the actual payload, length is `size' bytes.
+ *
+ * Userspace tool will split the record to different files.
+ */
+
+#define XEN_GCOV_FORMAT_MAGIC 0x58434f56 /* XCOV */
+
+#define XEN_SYSCTL_GCOV_get_size 0 /* Get total size of output data */
+#define XEN_SYSCTL_GCOV_read 1 /* Read output data */
+#define XEN_SYSCTL_GCOV_reset 2 /* Reset all counters */
+
+struct xen_sysctl_gcov_op {
+ uint32_t cmd;
+ uint32_t size; /* IN/OUT: size of the buffer */
+ XEN_GUEST_HANDLE_64(char) buffer; /* OUT */
+};
+typedef struct xen_sysctl_gcov_op xen_sysctl_gcov_op_t;
+DEFINE_XEN_GUEST_HANDLE(xen_sysctl_gcov_op_t);
+
#define XEN_SYSCTL_PSR_CMT_get_total_rmid 0
#define XEN_SYSCTL_PSR_CMT_get_l3_upscaling_factor 1
/* The L3 cache size is returned in KB unit */
@@ -1073,7 +1107,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 */
+#define XEN_SYSCTL_gcov_op 20
#define XEN_SYSCTL_psr_cmt_op 21
#define XEN_SYSCTL_pcitopoinfo 22
#define XEN_SYSCTL_psr_cat_op 23
@@ -1102,6 +1136,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_gcov_op gcov_op;
struct xen_sysctl_psr_cmt_op psr_cmt_op;
struct xen_sysctl_psr_cat_op psr_cat_op;
struct xen_sysctl_tmem_op tmem_op;
diff --git a/xen/include/xen/gcov.h b/xen/include/xen/gcov.h
new file mode 100644
index 0000000..8b5d914
--- /dev/null
+++ b/xen/include/xen/gcov.h
@@ -0,0 +1,9 @@
+#ifndef _XEN_GCOV_H
+#define _XEN_GCOV_H
+
+#ifdef CONFIG_GCOV
+#include <public/sysctl.h>
+int sysctl_gcov_op(xen_sysctl_gcov_op_t *op);
+#endif
+
+#endif /* _XEN_GCOV_H */
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 58+ messages in thread* Re: [PATCH v2 5/9] gcov: add new interface and 3.4 and 4.7 format support
2016-10-10 9:40 ` [PATCH v2 5/9] gcov: add new interface and 3.4 and 4.7 format support Wei Liu
@ 2016-10-10 11:56 ` Jan Beulich
2016-10-10 12:23 ` Andrew Cooper
2016-10-10 13:11 ` Wei Liu
2016-10-11 10:31 ` [PATCH v3] " Wei Liu
1 sibling, 2 replies; 58+ messages in thread
From: Jan Beulich @ 2016-10-10 11:56 UTC (permalink / raw)
To: Wei Liu
Cc: Stefano Stabellini, George Dunlap, AndrewCooper, Ian Jackson,
Tim Deegan, philippe.gabriel, Xen-devel
>>> On 10.10.16 at 11:40, <wei.liu2@citrix.com> wrote:
> +struct type_info {
> + int ctr_type;
Can this be negative?
> + unsigned int offset;
> +};
> +
> +/**
> + * struct gcov_iterator - specifies current file position in logical records
> + * @info: associated profiling data
> + * @record: record type
> + * @function: function number
> + * @type: counter type
> + * @count: index into values array
> + * @num_types: number of counter types
> + * @type_info: helper array to get values-array offset for current function
> + */
> +struct gcov_iterator {
> + const struct gcov_info *info;
> +
> + int record;
This one indeed looks like it can be.
> + unsigned int function;
> + unsigned int type;
> + unsigned int count;
> +
> + int num_types;
But judging by its name, this surely can't be.
> + struct type_info type_info[GCOV_COUNTERS];
> +};
> +
> +/* Mapping of logical record number to actual file content. */
> +#define RECORD_FILE_MAGIC 0
> +#define RECORD_GCOV_VERSION 1
> +#define RECORD_TIME_STAMP 2
> +#define RECORD_FUNCTION_TAG 3
> +#define RECORD_FUNCTON_TAG_LEN 4
> +#define RECORD_FUNCTION_IDENT 5
> +#define RECORD_FUNCTION_CHECK 6
> +#define RECORD_COUNT_TAG 7
> +#define RECORD_COUNT_LEN 8
> +#define RECORD_COUNT 9
> +
> +static int counter_active(const struct gcov_info *info, unsigned int type)
> +{
> + return (1 << type) & info->ctr_mask;
If the function return type is really meant to be a bitmask, then it
should be unsigned int (and 1u). However, I suspect this really
wants to be bool.
> +static size_t get_fn_size(const struct gcov_info *info)
> +{
> + size_t size;
> +
> + size = sizeof(struct gcov_fn_info) + num_counter_active(info) *
> + sizeof(unsigned int);
Any reason this is not the variable's initializer? Also please use
sizeof(*info) and alignof(*info) (below here).
> +static struct gcov_fn_info *get_fn_info(const struct gcov_info *info,
> + unsigned int fn)
> +{
> + return (struct gcov_fn_info *)
> + ((char *) info->functions + fn * get_fn_size(info));
Stray blank after (char *) and sub-optimal indentation.
> +static int gcov_iter_next(struct gcov_iterator *iter)
> +{
> + switch ( iter->record )
> + {
> + case RECORD_FILE_MAGIC:
> + case RECORD_GCOV_VERSION:
> + case RECORD_FUNCTION_TAG:
> + case RECORD_FUNCTON_TAG_LEN:
> + case RECORD_FUNCTION_IDENT:
> + case RECORD_COUNT_TAG:
> + /* Advance to next record */
> + iter->record++;
> + break;
> + case RECORD_COUNT:
> + /* Advance to next count */
> + iter->count++;
> + /* fall through */
> + case RECORD_COUNT_LEN:
> + if ( iter->count < get_func(iter)->n_ctrs[iter->type] )
> + {
> + iter->record = 9;
Who is 9 (more similar ones below)?
> +static int counter_active(const struct gcov_info *info, unsigned int type)
> +{
> + return info->merge[type] ? 1 : 0;
Return type bool and preferably with !! instead of conditional
expression.
> +size_t gcov_store_u32(void *buffer, size_t off, u32 v)
> +{
> + u32 *data;
Please be consistent wrt u32 vs ...
> +static int gcov_info_dump_payload(const struct gcov_info *info,
> + XEN_GUEST_HANDLE_PARAM(char) buffer,
> + uint32_t *off)
> +{
> + char *buf;
> + uint32_t buf_size;
... uint32_t (and alike; I'd suggest using only the latter, and I think
we should get rid of u32 / __u32 and their siblings eventually).
> +static uint32_t gcov_get_size(void)
> +{
> + uint32_t total_size;
> + struct gcov_info *info = NULL;
> +
> + /* Magic number XCOV */
> + total_size = sizeof(uint32_t);
Again - can't this be the initializer?
> +int sysctl_gcov_op(xen_sysctl_gcov_op_t *op)
> +{
> + int ret;
> +
> + switch ( op->cmd )
> + {
> + case XEN_SYSCTL_GCOV_get_size:
> + op->size = gcov_get_size();
> + ret = 0;
> + break;
> + case XEN_SYSCTL_GCOV_read:
Blank lines between non-fall-through case statements please.
> --- /dev/null
> +++ b/xen/common/gcov/gcov.h
> @@ -0,0 +1,36 @@
> +#ifndef _GCOV_H_
> +#define _GCOV_H_
> +
> +#include <xen/guest_access.h>
> +#include <xen/types.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.
> + */
> +#define GCOV_DATA_MAGIC ((unsigned int) 0x67636461)
> +#define GCOV_TAG_FUNCTION ((unsigned int) 0x01000000)
> +#define GCOV_TAG_COUNTER_BASE ((unsigned int) 0x01a10000)
> +#define GCOV_TAG_FOR_COUNTER(count) \
> + _TAG_COUNTER_BASE + ((unsigned int) (count) << 17))
Stray blanks after casts.
> --- /dev/null
> +++ b/xen/common/gcov/gcov_base.c
> @@ -0,0 +1,64 @@
> +/*
> + * Common code across gcov implementations
> + *
> + * Copyright Citrix Systems R&D UK
> + * Author(s): Wei Liu <wei.liu2@citrix.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 "gcov.h"
> +
> +/*
> + * __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.
I don't follow - we do such references elsewhere, so I can't see a
reason not to follow that model here too as long the call here
happens before .init.* get discarded.
Having reached the end here - what if the user gets the Kconfig setting
wrong? Wouldn't it be helpful if the gcov_<major>_<minor>.c files
#error-ed upon an out of range gcc version?
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 58+ messages in thread* Re: [PATCH v2 5/9] gcov: add new interface and 3.4 and 4.7 format support
2016-10-10 11:56 ` Jan Beulich
@ 2016-10-10 12:23 ` Andrew Cooper
2016-10-10 12:56 ` Jan Beulich
2016-10-10 13:11 ` Wei Liu
1 sibling, 1 reply; 58+ messages in thread
From: Andrew Cooper @ 2016-10-10 12:23 UTC (permalink / raw)
To: Jan Beulich, Wei Liu
Cc: Stefano Stabellini, George Dunlap, Tim Deegan, Ian Jackson,
philippe.gabriel, Xen-devel
On 10/10/16 12:56, Jan Beulich wrote:
>>>> On 10.10.16 at 11:40, <wei.liu2@citrix.com> wrote:
>> +struct type_info {
>> + int ctr_type;
> Can this be negative?
This code is largely imported straight from Linux. We should not
needlessly deviate.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 58+ messages in thread* Re: [PATCH v2 5/9] gcov: add new interface and 3.4 and 4.7 format support
2016-10-10 12:23 ` Andrew Cooper
@ 2016-10-10 12:56 ` Jan Beulich
2016-10-10 13:12 ` Wei Liu
0 siblings, 1 reply; 58+ messages in thread
From: Jan Beulich @ 2016-10-10 12:56 UTC (permalink / raw)
To: Andrew Cooper, Wei Liu
Cc: Stefano Stabellini, George Dunlap, Tim Deegan, Ian Jackson,
philippe.gabriel, Xen-devel
>>> On 10.10.16 at 14:23, <andrew.cooper3@citrix.com> wrote:
> On 10/10/16 12:56, Jan Beulich wrote:
>>>>> On 10.10.16 at 11:40, <wei.liu2@citrix.com> wrote:
>>> +struct type_info {
>>> + int ctr_type;
>> Can this be negative?
>
> This code is largely imported straight from Linux. We should not
> needlessly deviate.
Hmm, in that case some of the constification I did ask for on v1 and
which is now there should also be undone? Perhaps for such a
purpose it would help if the original Linux files got first imported
verbatim (without any review comments, and without getting wired
up), in order to then be modified minimally to build/work on Xen?
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 58+ messages in thread* Re: [PATCH v2 5/9] gcov: add new interface and 3.4 and 4.7 format support
2016-10-10 12:56 ` Jan Beulich
@ 2016-10-10 13:12 ` Wei Liu
0 siblings, 0 replies; 58+ messages in thread
From: Wei Liu @ 2016-10-10 13:12 UTC (permalink / raw)
To: Jan Beulich
Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper,
Ian Jackson, Tim Deegan, philippe.gabriel, Xen-devel
On Mon, Oct 10, 2016 at 06:56:52AM -0600, Jan Beulich wrote:
> >>> On 10.10.16 at 14:23, <andrew.cooper3@citrix.com> wrote:
> > On 10/10/16 12:56, Jan Beulich wrote:
> >>>>> On 10.10.16 at 11:40, <wei.liu2@citrix.com> wrote:
> >>> +struct type_info {
> >>> + int ctr_type;
> >> Can this be negative?
> >
> > This code is largely imported straight from Linux. We should not
> > needlessly deviate.
>
> Hmm, in that case some of the constification I did ask for on v1 and
> which is now there should also be undone? Perhaps for such a
I'm fine with either way. Since the modification is quite heavy, adding
some constification is just minor issue.
> purpose it would help if the original Linux files got first imported
> verbatim (without any review comments, and without getting wired
> up), in order to then be modified minimally to build/work on Xen?
>
This would be ugly because the code used is only a small portion of what
Linux has (we would end up deleting most of the code -- memory
allocation, seqfile ops etc), and the modification to adapt it to Xen is
quite heavy because the interface is entirely different (procfs vs
hypercall, customised packed file).
Wei.
> Jan
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH v2 5/9] gcov: add new interface and 3.4 and 4.7 format support
2016-10-10 11:56 ` Jan Beulich
2016-10-10 12:23 ` Andrew Cooper
@ 2016-10-10 13:11 ` Wei Liu
2016-10-10 14:43 ` Wei Liu
1 sibling, 1 reply; 58+ messages in thread
From: Wei Liu @ 2016-10-10 13:11 UTC (permalink / raw)
To: Jan Beulich
Cc: Stefano Stabellini, Wei Liu, George Dunlap, AndrewCooper,
Ian Jackson, Tim Deegan, philippe.gabriel, Xen-devel
On Mon, Oct 10, 2016 at 05:56:35AM -0600, Jan Beulich wrote:
[...]
> Who is 9 (more similar ones below)?
>
> > +static int counter_active(const struct gcov_info *info, unsigned int type)
> > +{
> > + return info->merge[type] ? 1 : 0;
>
> Return type bool and preferably with !! instead of conditional
> expression.
>
Andrew beat me to it: the code and data structures above are mostly
imported from Linux.
> > +size_t gcov_store_u32(void *buffer, size_t off, u32 v)
> > +{
> > + u32 *data;
>
> Please be consistent wrt u32 vs ...
>
> > +static int gcov_info_dump_payload(const struct gcov_info *info,
> > + XEN_GUEST_HANDLE_PARAM(char) buffer,
> > + uint32_t *off)
> > +{
> > + char *buf;
> > + uint32_t buf_size;
>
> ... uint32_t (and alike; I'd suggest using only the latter, and I think
> we should get rid of u32 / __u32 and their siblings eventually).
>
Right. I will switch to using uint32_t.
> > +static uint32_t gcov_get_size(void)
> > +{
> > + uint32_t total_size;
> > + struct gcov_info *info = NULL;
> > +
> > + /* Magic number XCOV */
> > + total_size = sizeof(uint32_t);
>
> Again - can't this be the initializer?
>
Sure. I will move it up.
> > +int sysctl_gcov_op(xen_sysctl_gcov_op_t *op)
> > +{
> > + int ret;
> > +
> > + switch ( op->cmd )
> > + {
> > + case XEN_SYSCTL_GCOV_get_size:
> > + op->size = gcov_get_size();
> > + ret = 0;
> > + break;
> > + case XEN_SYSCTL_GCOV_read:
>
> Blank lines between non-fall-through case statements please.
>
OK.
> > --- /dev/null
> > +++ b/xen/common/gcov/gcov.h
> > @@ -0,0 +1,36 @@
> > +#ifndef _GCOV_H_
> > +#define _GCOV_H_
> > +
> > +#include <xen/guest_access.h>
> > +#include <xen/types.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.
> > + */
> > +#define GCOV_DATA_MAGIC ((unsigned int) 0x67636461)
> > +#define GCOV_TAG_FUNCTION ((unsigned int) 0x01000000)
> > +#define GCOV_TAG_COUNTER_BASE ((unsigned int) 0x01a10000)
> > +#define GCOV_TAG_FOR_COUNTER(count) \
> > + _TAG_COUNTER_BASE + ((unsigned int) (count) << 17))
>
> Stray blanks after casts.
>
Will fix.
> > --- /dev/null
> > +++ b/xen/common/gcov/gcov_base.c
> > @@ -0,0 +1,64 @@
> > +/*
> > + * Common code across gcov implementations
> > + *
> > + * Copyright Citrix Systems R&D UK
> > + * Author(s): Wei Liu <wei.liu2@citrix.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 "gcov.h"
> > +
> > +/*
> > + * __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.
>
> I don't follow - we do such references elsewhere, so I can't see a
> reason not to follow that model here too as long the call here
> happens before .init.* get discarded.
>
This is residual from previous implementation. I haven't checked if the
statement is true or not. If this statement is not true I'm happy to
make corresponding adjustments.
> Having reached the end here - what if the user gets the Kconfig setting
> wrong? Wouldn't it be helpful if the gcov_<major>_<minor>.c files
> #error-ed upon an out of range gcc version?
>
That's a good idea.
Wei.
> Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 58+ messages in thread* Re: [PATCH v2 5/9] gcov: add new interface and 3.4 and 4.7 format support
2016-10-10 13:11 ` Wei Liu
@ 2016-10-10 14:43 ` Wei Liu
0 siblings, 0 replies; 58+ messages in thread
From: Wei Liu @ 2016-10-10 14:43 UTC (permalink / raw)
To: Jan Beulich
Cc: Stefano Stabellini, Wei Liu, George Dunlap, AndrewCooper,
Ian Jackson, Tim Deegan, philippe.gabriel, Xen-devel
On Mon, Oct 10, 2016 at 02:11:58PM +0100, Wei Liu wrote:
[...]
> > > +
> > > +#include "gcov.h"
> > > +
> > > +/*
> > > + * __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.
> >
> > I don't follow - we do such references elsewhere, so I can't see a
> > reason not to follow that model here too as long the call here
> > happens before .init.* get discarded.
> >
>
> This is residual from previous implementation. I haven't checked if the
> statement is true or not. If this statement is not true I'm happy to
> make corresponding adjustments.
It turns out that annotating __gcov_init as __init works. I will make
that adjustment in next version.
Wei.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 58+ messages in thread
* [PATCH v3] gcov: add new interface and 3.4 and 4.7 format support
2016-10-10 9:40 ` [PATCH v2 5/9] gcov: add new interface and 3.4 and 4.7 format support Wei Liu
2016-10-10 11:56 ` Jan Beulich
@ 2016-10-11 10:31 ` Wei Liu
2016-10-12 12:42 ` Jan Beulich
1 sibling, 1 reply; 58+ messages in thread
From: Wei Liu @ 2016-10-11 10:31 UTC (permalink / raw)
To: Xen-devel
Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper,
Ian Jackson, Tim Deegan, philippe.gabriel, Jan Beulich
A new sysctl interface for passing gcov data back to userspace. The new
interface uses a customised record file format. The new sysctl reuses
original sysctl number but renames the op to gcov_op.
Both gcc 3.4 and 4.7 format are supported. The code is rewritten so that
a new format can be easily added in the future. Version specific code
is grouped into different files. The format one needs to use can be
picked via Kconfig. The default format is 4.7 format.
Userspace programs to handle extracted data will come in a later patch.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
As always, v3 in my xenbits xen.git tree. Only sending this unacked
patch.
v3:
1. Check gcc version in gcc_*.c files.
2. Eliminate u32 / u64.
3. Mark __gcov_init as __init.
4. Fix a rebase error in v2.
5. Some other cosmetic fixes.
v2:
1. Use tab in Kconfig and indent help text properly.
2. Bump XEN_SYSCTL_INTERFACE_VERSION.
3. Fix cosmetic issues.
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>
---
xen/Kconfig.debug | 24 ++-
xen/common/gcov/Makefile | 3 +
xen/common/gcov/gcc_3_4.c | 367 ++++++++++++++++++++++++++++++++++++++++++++
xen/common/gcov/gcc_4_7.c | 205 +++++++++++++++++++++++++
xen/common/gcov/gcov.c | 257 +++++++++++++++++++++++++++++++
xen/common/gcov/gcov.h | 40 +++++
xen/common/gcov/gcov_base.c | 60 ++++++++
xen/common/sysctl.c | 8 +
xen/include/public/sysctl.h | 39 ++++-
xen/include/xen/gcov.h | 9 ++
10 files changed, 1005 insertions(+), 7 deletions(-)
create mode 100644 xen/common/gcov/gcc_3_4.c
create mode 100644 xen/common/gcov/gcc_4_7.c
create mode 100644 xen/common/gcov/gcov.c
create mode 100644 xen/common/gcov/gcov.h
create mode 100644 xen/common/gcov/gcov_base.c
create mode 100644 xen/include/xen/gcov.h
diff --git a/xen/Kconfig.debug b/xen/Kconfig.debug
index ef863dc..c65e543 100644
--- a/xen/Kconfig.debug
+++ b/xen/Kconfig.debug
@@ -30,16 +30,30 @@ config FRAME_POINTER
config GCOV
bool "Gcov Support"
- depends on BROKEN
---help---
Enable gcov (a test coverage program in GCC) support.
- Currently the data structure and hypercall interface are tied
- to GCC 3.4 gcov format. You need to have a version of GCC
- that is compatible with that format to make gcov work.
-
If unsure, say N here.
+choice
+ prompt "Specify Gcov format"
+ depends on GCOV
+ default GCOV_FORMAT_4_7
+ ---help---
+ The gcov format is determined by gcc version.
+
+config GCOV_FORMAT_4_7
+ bool "GCC 4.7 format"
+ ---help---
+ Select this option to use the format specified in GCC 4.7.
+
+config GCOV_FORMAT_3_4
+ bool "GCC 3.4 format"
+ ---help---
+ Select this option to use the format specified in GCC 3.4.
+
+endchoice
+
config LOCK_PROFILE
bool "Lock Profiling"
---help---
diff --git a/xen/common/gcov/Makefile b/xen/common/gcov/Makefile
index e69de29..03ac1e5 100644
--- a/xen/common/gcov/Makefile
+++ b/xen/common/gcov/Makefile
@@ -0,0 +1,3 @@
+obj-y += gcov_base.o gcov.o
+obj-$(CONFIG_GCOV_FORMAT_3_4) += gcc_3_4.o
+obj-$(CONFIG_GCOV_FORMAT_4_7) += gcc_4_7.o
diff --git a/xen/common/gcov/gcc_3_4.c b/xen/common/gcov/gcc_3_4.c
new file mode 100644
index 0000000..3631f4b
--- /dev/null
+++ b/xen/common/gcov/gcc_3_4.c
@@ -0,0 +1,367 @@
+/*
+ * This code provides functions to handle gcc's profiling data format
+ * introduced with gcc 3.4. Future versions of gcc may change the gcov
+ * format (as happened before), so all format-specific information needs
+ * to be kept modular and easily exchangeable.
+ *
+ * This file is based on gcc-internal definitions. Functions and 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.
+ *
+ * Imported from Linux and modified for Xen by
+ * Wei Liu <wei.liu2@citrix.com>
+ */
+
+
+#include <xen/lib.h>
+
+#include "gcov.h"
+
+#if !(GCC_VERSION >= 30400 && GCC_VERSION < 40700)
+#error "Wrong version of GCC used to compile gcov"
+#endif
+
+#define GCOV_COUNTERS 5
+
+static struct gcov_info *gcov_info_head;
+
+/**
+ * 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];
+};
+
+/**
+ * struct type_info - iterator helper array
+ * @ctr_type: counter type
+ * @offset: index of the first value of the current function for this type
+ *
+ * This array is needed to convert the in-memory data format into the in-file
+ * data format:
+ *
+ * In-memory:
+ * for each counter type
+ * for each function
+ * values
+ *
+ * In-file:
+ * for each function
+ * for each counter type
+ * values
+ *
+ * See gcc source gcc/gcov-io.h for more information on data organization.
+ */
+struct type_info {
+ int ctr_type;
+ unsigned int offset;
+};
+
+/**
+ * struct gcov_iterator - specifies current file position in logical records
+ * @info: associated profiling data
+ * @record: record type
+ * @function: function number
+ * @type: counter type
+ * @count: index into values array
+ * @num_types: number of counter types
+ * @type_info: helper array to get values-array offset for current function
+ */
+struct gcov_iterator {
+ const struct gcov_info *info;
+
+ int record;
+ unsigned int function;
+ unsigned int type;
+ unsigned int count;
+
+ int num_types;
+ struct type_info type_info[GCOV_COUNTERS];
+};
+
+/* Mapping of logical record number to actual file content. */
+#define RECORD_FILE_MAGIC 0
+#define RECORD_GCOV_VERSION 1
+#define RECORD_TIME_STAMP 2
+#define RECORD_FUNCTION_TAG 3
+#define RECORD_FUNCTON_TAG_LEN 4
+#define RECORD_FUNCTION_IDENT 5
+#define RECORD_FUNCTION_CHECK 6
+#define RECORD_COUNT_TAG 7
+#define RECORD_COUNT_LEN 8
+#define RECORD_COUNT 9
+
+static int counter_active(const struct gcov_info *info, unsigned int type)
+{
+ return (1 << type) & info->ctr_mask;
+}
+
+static unsigned int num_counter_active(const struct gcov_info *info)
+{
+ unsigned int i;
+ unsigned int result = 0;
+
+ for ( i = 0; i < GCOV_COUNTERS; i++ )
+ if ( counter_active(info, i) )
+ result++;
+
+ return result;
+}
+
+void gcov_info_link(struct gcov_info *info)
+{
+ info->next = gcov_info_head;
+ gcov_info_head = info;
+}
+
+struct gcov_info *gcov_info_next(const struct gcov_info *info)
+{
+ if ( !info )
+ return gcov_info_head;
+
+ return info->next;
+}
+
+const char *gcov_info_filename(const struct gcov_info *info)
+{
+ return info->filename;
+}
+
+void gcov_info_reset(struct gcov_info *info)
+{
+ unsigned int active = num_counter_active(info);
+ unsigned int i;
+
+ for ( i = 0; i < active; i++ )
+ memset(info->counts[i].values, 0,
+ info->counts[i].num * sizeof(gcov_type));
+}
+
+static size_t get_fn_size(const struct gcov_info *info)
+{
+ size_t size;
+
+ size = sizeof(struct gcov_fn_info) + num_counter_active(info) *
+ sizeof(unsigned int);
+ if ( __alignof__(struct gcov_fn_info) > sizeof(unsigned int) )
+ size = ROUNDUP(size, __alignof__(struct gcov_fn_info));
+ return size;
+}
+
+static struct gcov_fn_info *get_fn_info(const struct gcov_info *info,
+ unsigned int fn)
+{
+ return (struct gcov_fn_info *)
+ ((char *) info->functions + fn * get_fn_size(info));
+}
+
+static struct gcov_fn_info *get_func(struct gcov_iterator *iter)
+{
+ return get_fn_info(iter->info, iter->function);
+}
+
+static struct type_info *get_type(struct gcov_iterator *iter)
+{
+ return &iter->type_info[iter->type];
+}
+
+/**
+ * gcov_iter_next - advance file iterator to next logical record
+ * @iter: file iterator
+ *
+ * Return zero if new position is valid, non-zero if iterator has reached end.
+ */
+static int gcov_iter_next(struct gcov_iterator *iter)
+{
+ switch ( iter->record )
+ {
+ case RECORD_FILE_MAGIC:
+ case RECORD_GCOV_VERSION:
+ case RECORD_FUNCTION_TAG:
+ case RECORD_FUNCTON_TAG_LEN:
+ case RECORD_FUNCTION_IDENT:
+ case RECORD_COUNT_TAG:
+ /* Advance to next record */
+ iter->record++;
+ break;
+ case RECORD_COUNT:
+ /* Advance to next count */
+ iter->count++;
+ /* fall through */
+ case RECORD_COUNT_LEN:
+ if ( iter->count < get_func(iter)->n_ctrs[iter->type] )
+ {
+ iter->record = 9;
+ break;
+ }
+ /* Advance to next counter type */
+ get_type(iter)->offset += iter->count;
+ iter->count = 0;
+ iter->type++;
+ /* fall through */
+ case RECORD_FUNCTION_CHECK:
+ if ( iter->type < iter->num_types )
+ {
+ iter->record = 7;
+ break;
+ }
+ /* Advance to next function */
+ iter->type = 0;
+ iter->function++;
+ /* fall through */
+ case RECORD_TIME_STAMP:
+ if ( iter->function < iter->info->n_functions )
+ iter->record = 3;
+ else
+ iter->record = -1;
+ break;
+ }
+ /* Check for EOF. */
+ if ( iter->record == -1 )
+ return -EINVAL;
+ else
+ return 0;
+}
+
+/**
+ * gcov_iter_write - write data to buffer
+ * @iter: file iterator
+ * @buf: buffer to write to, if it is NULL, nothing is written
+ * @pos: position inside buffer to start writing
+ *
+ * Return number of bytes written into buffer.
+ */
+static size_t gcov_iter_write(struct gcov_iterator *iter, char *buf,
+ size_t pos)
+{
+ size_t ret = 0;
+
+ switch ( iter->record )
+ {
+ case RECORD_FILE_MAGIC:
+ ret = gcov_store_uint32(buf, pos, GCOV_DATA_MAGIC);
+ break;
+ case RECORD_GCOV_VERSION:
+ ret = gcov_store_uint32(buf, pos, iter->info->version);
+ break;
+ case RECORD_TIME_STAMP:
+ ret = gcov_store_uint32(buf, pos, iter->info->stamp);
+ break;
+ case RECORD_FUNCTION_TAG:
+ ret = gcov_store_uint32(buf, pos, GCOV_TAG_FUNCTION);
+ break;
+ case RECORD_FUNCTON_TAG_LEN:
+ ret = gcov_store_uint32(buf, pos, 2);
+ break;
+ case RECORD_FUNCTION_IDENT:
+ ret = gcov_store_uint32(buf, pos, get_func(iter)->ident);
+ break;
+ case RECORD_FUNCTION_CHECK:
+ ret = gcov_store_uint32(buf, pos, get_func(iter)->checksum);
+ break;
+ case RECORD_COUNT_TAG:
+ ret = gcov_store_uint32(buf, pos,
+ GCOV_TAG_FOR_COUNTER(get_type(iter)->ctr_type));
+ break;
+ case RECORD_COUNT_LEN:
+ ret = gcov_store_uint32(buf, pos,
+ get_func(iter)->n_ctrs[iter->type] * 2);
+ break;
+ case RECORD_COUNT:
+ ret = gcov_store_uint64(buf, pos, iter->info->counts[iter->type].
+ values[iter->count + get_type(iter)->offset]);
+ break;
+ }
+
+ return ret;
+}
+
+/* If buffer is NULL, no data is written. */
+size_t gcov_info_to_gcda(char *buffer, const struct gcov_info *info)
+{
+ struct gcov_iterator iter = { .info = info };
+ unsigned int i;
+ size_t pos = 0;
+
+ for ( i = 0; i < GCOV_COUNTERS; i++ )
+ {
+ if ( counter_active(info, i) )
+ {
+ iter.type_info[iter.num_types].ctr_type = i;
+ iter.type_info[iter.num_types].offset = 0;
+ iter.num_types++;
+ }
+ }
+
+ do {
+ pos += gcov_iter_write(&iter, buffer, pos);
+ } while ( gcov_iter_next(&iter) == 0 );
+
+ return pos;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/common/gcov/gcc_4_7.c b/xen/common/gcov/gcc_4_7.c
new file mode 100644
index 0000000..0fcce43
--- /dev/null
+++ b/xen/common/gcov/gcc_4_7.c
@@ -0,0 +1,205 @@
+/*
+ * This code provides functions to handle gcc's profiling data format
+ * introduced with gcc 4.7.
+ *
+ * This file is based heavily on gcc_3_4.c file.
+ *
+ * For a better understanding, refer to gcc source:
+ * gcc/gcov-io.h
+ * libgcc/libgcov.c
+ *
+ * Uses gcc-internal data definitions.
+ *
+ * Imported from Linux and modified for Xen by
+ * Wei Liu <wei.liu2@citrix.com>
+ */
+
+#include <xen/string.h>
+
+#include "gcov.h"
+
+#if GCC_VERSION < 40700
+#error "Wrong version of GCC used to compile gcov"
+#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
+
+#define GCOV_TAG_FUNCTION_LENGTH 3
+
+static struct gcov_info *gcov_info_head;
+
+/**
+ * struct gcov_ctr_info - information about counters for a single function
+ * @num: number of counter values for this type
+ * @values: array of counter values for this type
+ *
+ * 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;
+};
+
+/**
+ * struct gcov_fn_info - profiling meta data per function
+ * @key: comdat key
+ * @ident: unique ident of function
+ * @lineno_checksum: function lineo_checksum
+ * @cfg_checksum: function cfg checksum
+ * @ctrs: instrumented counters
+ *
+ * This data is generated by gcc during compilation and doesn't change
+ * at run-time.
+ *
+ * Information about a single function. This uses the trailing array
+ * idiom. The number of counters is determined from the merge pointer
+ * array in gcov_info. The key is used to detect which of a set of
+ * comdat functions was selected -- it points to the gcov_info object
+ * of the object file containing the selected comdat function.
+ */
+struct gcov_fn_info {
+ const struct gcov_info *key;
+ unsigned int ident;
+ unsigned int lineno_checksum;
+ unsigned int cfg_checksum;
+ struct gcov_ctr_info ctrs[0];
+};
+
+/**
+ * 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: uniquifying time stamp
+ * @filename: name of the associated gcov data file
+ * @merge: merge functions (null for unused counter type)
+ * @n_functions: number of instrumented functions
+ * @functions: pointer to pointers to function information
+ *
+ * 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;
+ void (*merge[GCOV_COUNTERS])(gcov_type *, unsigned int);
+ unsigned int n_functions;
+ struct gcov_fn_info **functions;
+};
+
+static int counter_active(const struct gcov_info *info, unsigned int type)
+{
+ return info->merge[type] ? 1 : 0;
+}
+
+void gcov_info_link(struct gcov_info *info)
+{
+ info->next = gcov_info_head;
+ gcov_info_head = info;
+}
+
+struct gcov_info *gcov_info_next(const struct gcov_info *info)
+{
+ if ( !info )
+ return gcov_info_head;
+ return info->next;
+}
+
+void gcov_info_reset(struct gcov_info *info)
+{
+ struct gcov_ctr_info *ci_ptr;
+ unsigned int fi_idx;
+ unsigned int ct_idx;
+
+ for ( fi_idx = 0; fi_idx < info->n_functions; fi_idx++ )
+ {
+ ci_ptr = info->functions[fi_idx]->ctrs;
+
+ for ( ct_idx = 0; ct_idx < GCOV_COUNTERS; ct_idx++ )
+ {
+ if ( !counter_active(info, ct_idx) )
+ continue;
+
+ memset(ci_ptr->values, 0, sizeof(gcov_type) * ci_ptr->num);
+ ci_ptr++;
+ }
+ }
+}
+
+const char *gcov_info_filename(const struct gcov_info *info)
+{
+ return info->filename;
+}
+
+
+/**
+ * gcov_info_to_gcda - convert profiling data set to gcda file format
+ * @buffer: the buffer to store file data or %NULL if no data should be stored
+ * @info: profiling data set to be converted
+ *
+ * Returns the number of bytes that were/would have been stored into the buffer.
+ */
+size_t gcov_info_to_gcda(char *buffer, const struct gcov_info *info)
+{
+ struct gcov_fn_info *fi_ptr;
+ struct gcov_ctr_info *ci_ptr;
+ unsigned int fi_idx;
+ unsigned int ct_idx;
+ unsigned int cv_idx;
+ size_t pos = 0;
+
+ /* File header. */
+ pos += gcov_store_uint32(buffer, pos, GCOV_DATA_MAGIC);
+ pos += gcov_store_uint32(buffer, pos, info->version);
+ pos += gcov_store_uint32(buffer, pos, info->stamp);
+
+ for ( fi_idx = 0; fi_idx < info->n_functions; fi_idx++ )
+ {
+ fi_ptr = info->functions[fi_idx];
+
+ /* Function record. */
+ pos += gcov_store_uint32(buffer, pos, GCOV_TAG_FUNCTION);
+ pos += gcov_store_uint32(buffer, pos, GCOV_TAG_FUNCTION_LENGTH);
+ pos += gcov_store_uint32(buffer, pos, fi_ptr->ident);
+ pos += gcov_store_uint32(buffer, pos, fi_ptr->lineno_checksum);
+ pos += gcov_store_uint32(buffer, pos, fi_ptr->cfg_checksum);
+
+ ci_ptr = fi_ptr->ctrs;
+
+ for ( ct_idx = 0; ct_idx < GCOV_COUNTERS; ct_idx++ )
+ {
+ if (! counter_active(info, ct_idx) )
+ continue;
+
+ /* Counter record. */
+ pos += gcov_store_uint32(buffer, pos,
+ GCOV_TAG_FOR_COUNTER(ct_idx));
+ pos += gcov_store_uint32(buffer, pos, ci_ptr->num * 2);
+
+ for ( cv_idx = 0; cv_idx < ci_ptr->num; cv_idx++ )
+ pos += gcov_store_uint64(buffer, pos, ci_ptr->values[cv_idx]);
+
+ ci_ptr++;
+ }
+ }
+
+ return pos;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/common/gcov/gcov.c b/xen/common/gcov/gcov.c
new file mode 100644
index 0000000..58b143d
--- /dev/null
+++ b/xen/common/gcov/gcov.c
@@ -0,0 +1,257 @@
+/*
+ * 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
+ *
+ * Modified for Xen by:
+ * Wei Liu <wei.liu2@citrix.com>
+ */
+
+#include <xen/errno.h>
+#include <xen/guest_access.h>
+#include <xen/types.h>
+
+#include <public/sysctl.h>
+
+#include "gcov.h"
+
+/**
+ * gcov_store_uint32 - store 32 bit number in gcov format to buffer
+ * @buffer: target buffer or NULL
+ * @off: offset into the buffer
+ * @v: value to be stored
+ *
+ * Number format defined by gcc: numbers are recorded in the 32 bit
+ * unsigned binary form of the endianness of the machine generating the
+ * file. Returns the number of bytes stored. If @buffer is %NULL, doesn't
+ * store anything.
+ */
+size_t gcov_store_uint32(void *buffer, size_t off, uint32_t v)
+{
+ uint32_t *data;
+
+ if ( buffer )
+ {
+ data = buffer + off;
+ *data = v;
+ }
+
+ return sizeof(*data);
+}
+
+/**
+ * gcov_store_uint64 - store 64 bit number in gcov format to buffer
+ * @buffer: target buffer or NULL
+ * @off: offset into the buffer
+ * @v: value to be stored
+ *
+ * Number format defined by gcc: numbers are recorded in the 32 bit
+ * unsigned binary form of the endianness of the machine generating the
+ * file. 64 bit numbers are stored as two 32 bit numbers, the low part
+ * first. Returns the number of bytes stored. If @buffer is %NULL, doesn't store
+ * anything.
+ */
+size_t gcov_store_uint64(void *buffer, size_t off, uint64_t v)
+{
+ uint32_t *data;
+
+ if ( buffer )
+ {
+ data = buffer + off;
+
+ data[0] = (v & 0xffffffffUL);
+ data[1] = (v >> 32);
+ }
+
+ return sizeof(*data) * 2;
+}
+
+static size_t gcov_info_payload_size(const struct gcov_info *info)
+{
+ return gcov_info_to_gcda(NULL, info);
+}
+
+static int gcov_info_dump_payload(const struct gcov_info *info,
+ XEN_GUEST_HANDLE_PARAM(char) buffer,
+ uint32_t *off)
+{
+ char *buf;
+ uint32_t buf_size;
+ int ret;
+
+ /*
+ * Allocate a buffer and dump payload there. This helps us to not
+ * have copy_to_guest in other functions and retain their simple
+ * semantics.
+ */
+
+ buf_size = gcov_info_payload_size(info);
+ buf = xmalloc_array(char, buf_size);
+
+ if ( !buf )
+ {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ gcov_info_to_gcda(buf, info);
+
+ if ( copy_to_guest_offset(buffer, *off, buf, buf_size) )
+ {
+ ret = -EFAULT;
+ goto out;
+ }
+ *off += buf_size;
+
+ ret = 0;
+ out:
+ xfree(buf);
+ return ret;
+
+}
+
+static uint32_t gcov_get_size(void)
+{
+ uint32_t total_size = sizeof(uint32_t); /* Magic number XCOV */
+ struct gcov_info *info = NULL;
+
+ while ( (info = gcov_info_next(info)) )
+ {
+ /* File name length, including trailing \0 */
+ total_size += strlen(gcov_info_filename(info)) + 1;
+
+ /* Payload size field */
+ total_size += sizeof(uint32_t);
+
+ /* Payload itself */
+ total_size += gcov_info_payload_size(info);
+ }
+
+ return total_size;
+}
+
+static void gcov_reset_all_counters(void)
+{
+ struct gcov_info *info = NULL;
+
+ while ( (info = gcov_info_next(info)) )
+ gcov_info_reset(info);
+}
+
+static int gcov_dump_one_record(const struct gcov_info *info,
+ XEN_GUEST_HANDLE_PARAM(char) buffer,
+ uint32_t *off)
+{
+ uint32_t payload_size;
+ uint32_t len;
+
+ /* File name, including trailing \0 */
+ len = strlen(gcov_info_filename(info)) + 1;
+ if ( copy_to_guest_offset(buffer, *off, gcov_info_filename(info), len) )
+ return -EFAULT;
+ *off += len;
+
+ payload_size = gcov_info_payload_size(info);
+ /* Payload size */
+ if ( copy_to_guest_offset(buffer, *off, (char*)&payload_size,
+ sizeof(uint32_t)) )
+ return -EFAULT;
+ *off += sizeof(uint32_t);
+
+ /* Payload itself */
+ return gcov_info_dump_payload(info, buffer, off);
+}
+
+static int gcov_dump_all(XEN_GUEST_HANDLE_PARAM(char) buffer,
+ uint32_t *buffer_size)
+{
+ uint32_t off;
+ uint32_t magic = XEN_GCOV_FORMAT_MAGIC;
+ struct gcov_info *info = NULL;
+ int ret;
+
+ if ( *buffer_size < gcov_get_size() )
+ {
+ ret = -ENOBUFS;
+ goto out;
+ }
+
+ off = 0;
+
+ /* Magic number */
+ if ( copy_to_guest_offset(buffer, off, (char *)&magic, sizeof(magic)) )
+ {
+ ret = -EFAULT;
+ goto out;
+ }
+ off += sizeof(magic);
+
+ while ( (info = gcov_info_next(info)) )
+ {
+ ret = gcov_dump_one_record(info, buffer, &off);
+ if ( ret )
+ goto out;
+ }
+
+ *buffer_size = off;
+
+ ret = 0;
+ out:
+ return ret;
+}
+
+int sysctl_gcov_op(xen_sysctl_gcov_op_t *op)
+{
+ int ret;
+
+ switch ( op->cmd )
+ {
+ case XEN_SYSCTL_GCOV_get_size:
+ op->size = gcov_get_size();
+ ret = 0;
+ break;
+
+ case XEN_SYSCTL_GCOV_read:
+ {
+ XEN_GUEST_HANDLE_PARAM(char) buf;
+ uint32_t size = op->size;
+
+ buf = guest_handle_cast(op->buffer, char);
+
+ ret = gcov_dump_all(buf, &size);
+ op->size = size;
+
+ break;
+ }
+
+ case XEN_SYSCTL_GCOV_reset:
+ gcov_reset_all_counters();
+ ret = 0;
+ break;
+
+ default:
+ ret = -EINVAL;
+ break;
+ }
+
+ return ret;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/common/gcov/gcov.h b/xen/common/gcov/gcov.h
new file mode 100644
index 0000000..efc1fd7
--- /dev/null
+++ b/xen/common/gcov/gcov.h
@@ -0,0 +1,40 @@
+#ifndef _GCOV_H_
+#define _GCOV_H_
+
+#include <xen/guest_access.h>
+#include <xen/types.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.
+ */
+#define GCOV_DATA_MAGIC ((unsigned int)0x67636461)
+#define GCOV_TAG_FUNCTION ((unsigned int)0x01000000)
+#define GCOV_TAG_COUNTER_BASE ((unsigned int)0x01a10000)
+#define GCOV_TAG_FOR_COUNTER(count) \
+ GCOV_TAG_COUNTER_BASE + ((unsigned int)(count) << 17)
+
+#define GCC_VERSION (__GNUC__ * 10000 \
+ + __GNUC_MINOR__ * 100 \
+ + __GNUC_PATCHLEVEL__)
+
+#if BITS_PER_LONG >= 64
+typedef long gcov_type;
+#else
+typedef long long gcov_type;
+#endif
+
+/* Opaque gcov_info -- tied to specific gcc gcov formats */
+struct gcov_info;
+
+void gcov_info_link(struct gcov_info *info);
+struct gcov_info *gcov_info_next(const struct gcov_info *info);
+void gcov_info_reset(struct gcov_info *info);
+const char *gcov_info_filename(const struct gcov_info *info);
+size_t gcov_info_to_gcda(char *buffer, const struct gcov_info *info);
+
+size_t gcov_store_uint32(void *buffer, size_t off, uint32_t v);
+size_t gcov_store_uint64(void *buffer, size_t off, uint64_t v);
+
+#endif /* _GCOV_H_ */
diff --git a/xen/common/gcov/gcov_base.c b/xen/common/gcov/gcov_base.c
new file mode 100644
index 0000000..e5065db
--- /dev/null
+++ b/xen/common/gcov/gcov_base.c
@@ -0,0 +1,60 @@
+/*
+ * Common code across gcov implementations
+ *
+ * Copyright Citrix Systems R&D UK
+ * Author(s): Wei Liu <wei.liu2@citrix.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 "gcov.h"
+
+/*
+ * __gcov_init is called by gcc-generated constructor code for each object
+ * file compiled with -fprofile-arcs.
+ */
+void __init __gcov_init(struct gcov_info *info)
+{
+ /* Link all gcov info together. */
+ gcov_info_link(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. */
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/common/sysctl.c b/xen/common/sysctl.c
index 2cac451..03cca5c 100644
--- a/xen/common/sysctl.c
+++ b/xen/common/sysctl.c
@@ -28,6 +28,7 @@
#include <xsm/xsm.h>
#include <xen/pmstat.h>
#include <xen/livepatch.h>
+#include <xen/gcov.h>
long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
{
@@ -395,6 +396,13 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
}
break;
+#ifdef CONFIG_GCOV
+ case XEN_SYSCTL_gcov_op:
+ ret = sysctl_gcov_op(&op->u.gcov_op);
+ copyback = 1;
+ break;
+#endif
+
#ifdef CONFIG_HAS_PCI
case XEN_SYSCTL_pcitopoinfo:
{
diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
index a939fa9..28ac56c 100644
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -36,7 +36,7 @@
#include "physdev.h"
#include "tmem.h"
-#define XEN_SYSCTL_INTERFACE_VERSION 0x0000000E
+#define XEN_SYSCTL_INTERFACE_VERSION 0x0000000F
/*
* Read console content from Xen buffer ring.
@@ -665,6 +665,40 @@ 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);
+/*
+ * Output format of gcov data:
+ *
+ * XEN_GCOV_FORMAT_MAGIC XEN_GCOV_RECORD ... XEN_GCOV_RECORD
+ *
+ * That is, one magic number followed by 0 or more record.
+ *
+ * The magic number is stored as an uint32_t field.
+ *
+ * The record is packed and variable in length. It has the form:
+ *
+ * filename: a NULL terminated path name extracted from gcov, used to
+ * create the name of gcda file.
+ * size: a uint32_t field indicating the size of the payload, the
+ * unit is byte.
+ * payload: the actual payload, length is `size' bytes.
+ *
+ * Userspace tool will split the record to different files.
+ */
+
+#define XEN_GCOV_FORMAT_MAGIC 0x58434f56 /* XCOV */
+
+#define XEN_SYSCTL_GCOV_get_size 0 /* Get total size of output data */
+#define XEN_SYSCTL_GCOV_read 1 /* Read output data */
+#define XEN_SYSCTL_GCOV_reset 2 /* Reset all counters */
+
+struct xen_sysctl_gcov_op {
+ uint32_t cmd;
+ uint32_t size; /* IN/OUT: size of the buffer */
+ XEN_GUEST_HANDLE_64(char) buffer; /* OUT */
+};
+typedef struct xen_sysctl_gcov_op xen_sysctl_gcov_op_t;
+DEFINE_XEN_GUEST_HANDLE(xen_sysctl_gcov_op_t);
+
#define XEN_SYSCTL_PSR_CMT_get_total_rmid 0
#define XEN_SYSCTL_PSR_CMT_get_l3_upscaling_factor 1
/* The L3 cache size is returned in KB unit */
@@ -1073,7 +1107,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 */
+#define XEN_SYSCTL_gcov_op 20
#define XEN_SYSCTL_psr_cmt_op 21
#define XEN_SYSCTL_pcitopoinfo 22
#define XEN_SYSCTL_psr_cat_op 23
@@ -1102,6 +1136,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_gcov_op gcov_op;
struct xen_sysctl_psr_cmt_op psr_cmt_op;
struct xen_sysctl_psr_cat_op psr_cat_op;
struct xen_sysctl_tmem_op tmem_op;
diff --git a/xen/include/xen/gcov.h b/xen/include/xen/gcov.h
new file mode 100644
index 0000000..8b5d914
--- /dev/null
+++ b/xen/include/xen/gcov.h
@@ -0,0 +1,9 @@
+#ifndef _XEN_GCOV_H
+#define _XEN_GCOV_H
+
+#ifdef CONFIG_GCOV
+#include <public/sysctl.h>
+int sysctl_gcov_op(xen_sysctl_gcov_op_t *op);
+#endif
+
+#endif /* _XEN_GCOV_H */
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 58+ messages in thread* Re: [PATCH v3] gcov: add new interface and 3.4 and 4.7 format support
2016-10-11 10:31 ` [PATCH v3] " Wei Liu
@ 2016-10-12 12:42 ` Jan Beulich
2016-10-12 13:06 ` Wei Liu
` (2 more replies)
0 siblings, 3 replies; 58+ messages in thread
From: Jan Beulich @ 2016-10-12 12:42 UTC (permalink / raw)
To: Wei Liu
Cc: Stefano Stabellini, George Dunlap, AndrewCooper, Ian Jackson,
Tim Deegan, philippe.gabriel, Xen-devel
>>> On 11.10.16 at 12:31, <wei.liu2@citrix.com> wrote:
> --- /dev/null
> +++ b/xen/common/gcov/gcc_4_7.c
> @@ -0,0 +1,205 @@
> +/*
> + * This code provides functions to handle gcc's profiling data format
> + * introduced with gcc 4.7.
> + *
> + * This file is based heavily on gcc_3_4.c file.
> + *
> + * For a better understanding, refer to gcc source:
> + * gcc/gcov-io.h
> + * libgcc/libgcov.c
> + *
> + * Uses gcc-internal data definitions.
> + *
> + * Imported from Linux and modified for Xen by
> + * Wei Liu <wei.liu2@citrix.com>
> + */
> +
> +#include <xen/string.h>
> +
> +#include "gcov.h"
> +
> +#if GCC_VERSION < 40700
> +#error "Wrong version of GCC used to compile gcov"
> +#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
I'm sorry for not having pointed this out on v2 (I had noticed it,
but then didn't finish analyzing the situation), but I'm afraid this
together with ...
> +struct gcov_info {
> + unsigned int version;
> + struct gcov_info *next;
> + unsigned int stamp;
> + const char *filename;
> + void (*merge[GCOV_COUNTERS])(gcov_type *, unsigned int);
> + unsigned int n_functions;
> + struct gcov_fn_info **functions;
> +};
... this structure's trailing fields actually getting used by the code
won't work well when changing compiler versions without cleaning
the tree. I think instead you need thin gcc_5.c and gcc_4_9.c
#define-ing their GCOV_COUNTERS and then #include-ing this
shared source file. Plus btw, I don't think gcc 5.0.x (the
development variant of 5.x) would use anything different from
5.1.x or 5.2.x; in fact use of __GNUC_MINOR__ should not
normally be necessary anymore with gcc 5+.
And then - how is all of this supposed to be working in conjucntion
with live patching, where the patch may have been created by yet
another compiler version?
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 58+ messages in thread* Re: [PATCH v3] gcov: add new interface and 3.4 and 4.7 format support
2016-10-12 12:42 ` Jan Beulich
@ 2016-10-12 13:06 ` Wei Liu
2016-10-12 13:11 ` Jan Beulich
2016-10-12 13:24 ` George Dunlap
2016-10-12 13:23 ` Konrad Rzeszutek Wilk
2016-10-12 15:33 ` Wei Liu
2 siblings, 2 replies; 58+ messages in thread
From: Wei Liu @ 2016-10-12 13:06 UTC (permalink / raw)
To: Jan Beulich
Cc: Stefano Stabellini, Wei Liu, George Dunlap, AndrewCooper,
Ian Jackson, Tim Deegan, philippe.gabriel, Xen-devel
On Wed, Oct 12, 2016 at 06:42:53AM -0600, Jan Beulich wrote:
> >>> On 11.10.16 at 12:31, <wei.liu2@citrix.com> wrote:
> > --- /dev/null
> > +++ b/xen/common/gcov/gcc_4_7.c
> > @@ -0,0 +1,205 @@
> > +/*
> > + * This code provides functions to handle gcc's profiling data format
> > + * introduced with gcc 4.7.
> > + *
> > + * This file is based heavily on gcc_3_4.c file.
> > + *
> > + * For a better understanding, refer to gcc source:
> > + * gcc/gcov-io.h
> > + * libgcc/libgcov.c
> > + *
> > + * Uses gcc-internal data definitions.
> > + *
> > + * Imported from Linux and modified for Xen by
> > + * Wei Liu <wei.liu2@citrix.com>
> > + */
> > +
> > +#include <xen/string.h>
> > +
> > +#include "gcov.h"
> > +
> > +#if GCC_VERSION < 40700
> > +#error "Wrong version of GCC used to compile gcov"
> > +#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
>
> I'm sorry for not having pointed this out on v2 (I had noticed it,
> but then didn't finish analyzing the situation), but I'm afraid this
> together with ...
>
> > +struct gcov_info {
> > + unsigned int version;
> > + struct gcov_info *next;
> > + unsigned int stamp;
> > + const char *filename;
> > + void (*merge[GCOV_COUNTERS])(gcov_type *, unsigned int);
> > + unsigned int n_functions;
> > + struct gcov_fn_info **functions;
> > +};
>
> ... this structure's trailing fields actually getting used by the code
> won't work well when changing compiler versions without cleaning
> the tree. I think instead you need thin gcc_5.c and gcc_4_9.c
> #define-ing their GCOV_COUNTERS and then #include-ing this
> shared source file. Plus btw, I don't think gcc 5.0.x (the
> development variant of 5.x) would use anything different from
> 5.1.x or 5.2.x; in fact use of __GNUC_MINOR__ should not
> normally be necessary anymore with gcc 5+.
>
Right. I will do something about this. Thanks for catching this.
> And then - how is all of this supposed to be working in conjucntion
> with live patching, where the patch may have been created by yet
> another compiler version?
>
There is a version field in gcov_info, so we can compare that and reject
incompatible version.
We need to use hooks in livepatching to call the constructor /
destructor when applying / reverting a live-patch. We might need to be
cautious about locks or whatever, but I'm sure it can be figured out.
But I have no idea how useful it would be to use gcov and livepatching
together. For now the easiest thing to do is to
depends on !LIVEPATCH
in Kconfig.
Wei.
> Jan
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 58+ messages in thread* Re: [PATCH v3] gcov: add new interface and 3.4 and 4.7 format support
2016-10-12 13:06 ` Wei Liu
@ 2016-10-12 13:11 ` Jan Beulich
2016-10-12 13:24 ` George Dunlap
1 sibling, 0 replies; 58+ messages in thread
From: Jan Beulich @ 2016-10-12 13:11 UTC (permalink / raw)
To: Wei Liu
Cc: Stefano Stabellini, George Dunlap, AndrewCooper, Ian Jackson,
Tim Deegan, philippe.gabriel, Xen-devel
>>> On 12.10.16 at 15:06, <wei.liu2@citrix.com> wrote:
> But I have no idea how useful it would be to use gcov and livepatching
> together. For now the easiest thing to do is to
>
> depends on !LIVEPATCH
>
> in Kconfig.
I agree.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH v3] gcov: add new interface and 3.4 and 4.7 format support
2016-10-12 13:06 ` Wei Liu
2016-10-12 13:11 ` Jan Beulich
@ 2016-10-12 13:24 ` George Dunlap
2016-10-12 13:26 ` George Dunlap
2016-10-12 13:29 ` Konrad Rzeszutek Wilk
1 sibling, 2 replies; 58+ messages in thread
From: George Dunlap @ 2016-10-12 13:24 UTC (permalink / raw)
To: Wei Liu, Jan Beulich
Cc: Stefano Stabellini, George Dunlap, AndrewCooper, Ian Jackson,
Tim Deegan, philippe.gabriel, Xen-devel
On 12/10/16 14:06, Wei Liu wrote:
> On Wed, Oct 12, 2016 at 06:42:53AM -0600, Jan Beulich wrote:
>>>>> On 11.10.16 at 12:31, <wei.liu2@citrix.com> wrote:
>>> --- /dev/null
>>> +++ b/xen/common/gcov/gcc_4_7.c
>>> @@ -0,0 +1,205 @@
>>> +/*
>>> + * This code provides functions to handle gcc's profiling data format
>>> + * introduced with gcc 4.7.
>>> + *
>>> + * This file is based heavily on gcc_3_4.c file.
>>> + *
>>> + * For a better understanding, refer to gcc source:
>>> + * gcc/gcov-io.h
>>> + * libgcc/libgcov.c
>>> + *
>>> + * Uses gcc-internal data definitions.
>>> + *
>>> + * Imported from Linux and modified for Xen by
>>> + * Wei Liu <wei.liu2@citrix.com>
>>> + */
>>> +
>>> +#include <xen/string.h>
>>> +
>>> +#include "gcov.h"
>>> +
>>> +#if GCC_VERSION < 40700
>>> +#error "Wrong version of GCC used to compile gcov"
>>> +#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
>>
>> I'm sorry for not having pointed this out on v2 (I had noticed it,
>> but then didn't finish analyzing the situation), but I'm afraid this
>> together with ...
>>
>>> +struct gcov_info {
>>> + unsigned int version;
>>> + struct gcov_info *next;
>>> + unsigned int stamp;
>>> + const char *filename;
>>> + void (*merge[GCOV_COUNTERS])(gcov_type *, unsigned int);
>>> + unsigned int n_functions;
>>> + struct gcov_fn_info **functions;
>>> +};
>>
>> ... this structure's trailing fields actually getting used by the code
>> won't work well when changing compiler versions without cleaning
>> the tree. I think instead you need thin gcc_5.c and gcc_4_9.c
>> #define-ing their GCOV_COUNTERS and then #include-ing this
>> shared source file. Plus btw, I don't think gcc 5.0.x (the
>> development variant of 5.x) would use anything different from
>> 5.1.x or 5.2.x; in fact use of __GNUC_MINOR__ should not
>> normally be necessary anymore with gcc 5+.
>>
>
> Right. I will do something about this. Thanks for catching this.
>
>> And then - how is all of this supposed to be working in conjucntion
>> with live patching, where the patch may have been created by yet
>> another compiler version?
>>
>
> There is a version field in gcov_info, so we can compare that and reject
> incompatible version.
>
> We need to use hooks in livepatching to call the constructor /
> destructor when applying / reverting a live-patch. We might need to be
> cautious about locks or whatever, but I'm sure it can be figured out.
>
> But I have no idea how useful it would be to use gcov and livepatching
> together. For now the easiest thing to do is to
>
> depends on !LIVEPATCH
>
> in Kconfig.
Wouldn't it be just as easy, and more useful, to set a "has been
livepatched" flag, and return errors for all gcov hypercalls if its' set?
I would expect most users to want to build a single hypervisor that can
be used for both gcov testing and live patching (under different
circumstances).
-George
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 58+ messages in thread* Re: [PATCH v3] gcov: add new interface and 3.4 and 4.7 format support
2016-10-12 13:24 ` George Dunlap
@ 2016-10-12 13:26 ` George Dunlap
2016-10-12 13:31 ` Wei Liu
` (2 more replies)
2016-10-12 13:29 ` Konrad Rzeszutek Wilk
1 sibling, 3 replies; 58+ messages in thread
From: George Dunlap @ 2016-10-12 13:26 UTC (permalink / raw)
To: Wei Liu, Jan Beulich
Cc: Stefano Stabellini, George Dunlap, AndrewCooper, Ian Jackson,
Tim Deegan, philippe.gabriel, Xen-devel
On 12/10/16 14:24, George Dunlap wrote:
> On 12/10/16 14:06, Wei Liu wrote:
>> On Wed, Oct 12, 2016 at 06:42:53AM -0600, Jan Beulich wrote:
>>>>>> On 11.10.16 at 12:31, <wei.liu2@citrix.com> wrote:
>>>> --- /dev/null
>>>> +++ b/xen/common/gcov/gcc_4_7.c
>>>> @@ -0,0 +1,205 @@
>>>> +/*
>>>> + * This code provides functions to handle gcc's profiling data format
>>>> + * introduced with gcc 4.7.
>>>> + *
>>>> + * This file is based heavily on gcc_3_4.c file.
>>>> + *
>>>> + * For a better understanding, refer to gcc source:
>>>> + * gcc/gcov-io.h
>>>> + * libgcc/libgcov.c
>>>> + *
>>>> + * Uses gcc-internal data definitions.
>>>> + *
>>>> + * Imported from Linux and modified for Xen by
>>>> + * Wei Liu <wei.liu2@citrix.com>
>>>> + */
>>>> +
>>>> +#include <xen/string.h>
>>>> +
>>>> +#include "gcov.h"
>>>> +
>>>> +#if GCC_VERSION < 40700
>>>> +#error "Wrong version of GCC used to compile gcov"
>>>> +#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
>>>
>>> I'm sorry for not having pointed this out on v2 (I had noticed it,
>>> but then didn't finish analyzing the situation), but I'm afraid this
>>> together with ...
>>>
>>>> +struct gcov_info {
>>>> + unsigned int version;
>>>> + struct gcov_info *next;
>>>> + unsigned int stamp;
>>>> + const char *filename;
>>>> + void (*merge[GCOV_COUNTERS])(gcov_type *, unsigned int);
>>>> + unsigned int n_functions;
>>>> + struct gcov_fn_info **functions;
>>>> +};
>>>
>>> ... this structure's trailing fields actually getting used by the code
>>> won't work well when changing compiler versions without cleaning
>>> the tree. I think instead you need thin gcc_5.c and gcc_4_9.c
>>> #define-ing their GCOV_COUNTERS and then #include-ing this
>>> shared source file. Plus btw, I don't think gcc 5.0.x (the
>>> development variant of 5.x) would use anything different from
>>> 5.1.x or 5.2.x; in fact use of __GNUC_MINOR__ should not
>>> normally be necessary anymore with gcc 5+.
>>>
>>
>> Right. I will do something about this. Thanks for catching this.
>>
>>> And then - how is all of this supposed to be working in conjucntion
>>> with live patching, where the patch may have been created by yet
>>> another compiler version?
>>>
>>
>> There is a version field in gcov_info, so we can compare that and reject
>> incompatible version.
>>
>> We need to use hooks in livepatching to call the constructor /
>> destructor when applying / reverting a live-patch. We might need to be
>> cautious about locks or whatever, but I'm sure it can be figured out.
>>
>> But I have no idea how useful it would be to use gcov and livepatching
>> together. For now the easiest thing to do is to
>>
>> depends on !LIVEPATCH
>>
>> in Kconfig.
>
> Wouldn't it be just as easy, and more useful, to set a "has been
> livepatched" flag, and return errors for all gcov hypercalls if its' set?
>
> I would expect most users to want to build a single hypervisor that can
> be used for both gcov testing and live patching (under different
> circumstances).
I mean software provider, not user, of course. That's what I would want
for CentOS, and I'm sure that's what the XenServer (and probably Oracle)
guys want as well.
-George
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 58+ messages in thread* Re: [PATCH v3] gcov: add new interface and 3.4 and 4.7 format support
2016-10-12 13:26 ` George Dunlap
@ 2016-10-12 13:31 ` Wei Liu
2016-10-12 13:33 ` Jan Beulich
2016-10-12 13:34 ` Andrew Cooper
2 siblings, 0 replies; 58+ messages in thread
From: Wei Liu @ 2016-10-12 13:31 UTC (permalink / raw)
To: George Dunlap
Cc: Stefano Stabellini, Wei Liu, George Dunlap, AndrewCooper,
Ian Jackson, Tim Deegan, philippe.gabriel, Jan Beulich,
Xen-devel
On Wed, Oct 12, 2016 at 02:26:26PM +0100, George Dunlap wrote:
[...]
> >>
> >> There is a version field in gcov_info, so we can compare that and reject
> >> incompatible version.
> >>
> >> We need to use hooks in livepatching to call the constructor /
> >> destructor when applying / reverting a live-patch. We might need to be
> >> cautious about locks or whatever, but I'm sure it can be figured out.
> >>
> >> But I have no idea how useful it would be to use gcov and livepatching
> >> together. For now the easiest thing to do is to
> >>
> >> depends on !LIVEPATCH
> >>
> >> in Kconfig.
> >
> > Wouldn't it be just as easy, and more useful, to set a "has been
> > livepatched" flag, and return errors for all gcov hypercalls if its' set?
> >
> > I would expect most users to want to build a single hypervisor that can
> > be used for both gcov testing and live patching (under different
> > circumstances).
>
> I mean software provider, not user, of course. That's what I would want
> for CentOS, and I'm sure that's what the XenServer (and probably Oracle)
> guys want as well.
>
The usefulness of such build is not as big as you might think I'm
afraid -- it wouldn't be useful until we figure out how to apply a
livepatch generated with gcov support built in.
Wei.
> -George
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH v3] gcov: add new interface and 3.4 and 4.7 format support
2016-10-12 13:26 ` George Dunlap
2016-10-12 13:31 ` Wei Liu
@ 2016-10-12 13:33 ` Jan Beulich
2016-10-12 13:34 ` Andrew Cooper
2 siblings, 0 replies; 58+ messages in thread
From: Jan Beulich @ 2016-10-12 13:33 UTC (permalink / raw)
To: George Dunlap
Cc: Stefano Stabellini, Wei Liu, George Dunlap, AndrewCooper,
Ian Jackson, Tim Deegan, philippe.gabriel, Xen-devel
>>> On 12.10.16 at 15:26, <george.dunlap@citrix.com> wrote:
> On 12/10/16 14:24, George Dunlap wrote:
>> I would expect most users to want to build a single hypervisor that can
>> be used for both gcov testing and live patching (under different
>> circumstances).
>
> I mean software provider, not user, of course. That's what I would want
> for CentOS, and I'm sure that's what the XenServer (and probably Oracle)
> guys want as well.
But gcov is explicitly a non-production feature, iirc.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH v3] gcov: add new interface and 3.4 and 4.7 format support
2016-10-12 13:26 ` George Dunlap
2016-10-12 13:31 ` Wei Liu
2016-10-12 13:33 ` Jan Beulich
@ 2016-10-12 13:34 ` Andrew Cooper
2016-10-12 13:41 ` George Dunlap
2 siblings, 1 reply; 58+ messages in thread
From: Andrew Cooper @ 2016-10-12 13:34 UTC (permalink / raw)
To: George Dunlap, Wei Liu, Jan Beulich
Cc: Stefano Stabellini, George Dunlap, Tim Deegan, Ian Jackson,
philippe.gabriel, Xen-devel
On 12/10/16 14:26, George Dunlap wrote:
> On 12/10/16 14:24, George Dunlap wrote:
>> On 12/10/16 14:06, Wei Liu wrote:
>>> On Wed, Oct 12, 2016 at 06:42:53AM -0600, Jan Beulich wrote:
>>>>>>> On 11.10.16 at 12:31, <wei.liu2@citrix.com> wrote:
>>>>> --- /dev/null
>>>>> +++ b/xen/common/gcov/gcc_4_7.c
>>>>> @@ -0,0 +1,205 @@
>>>>> +/*
>>>>> + * This code provides functions to handle gcc's profiling data format
>>>>> + * introduced with gcc 4.7.
>>>>> + *
>>>>> + * This file is based heavily on gcc_3_4.c file.
>>>>> + *
>>>>> + * For a better understanding, refer to gcc source:
>>>>> + * gcc/gcov-io.h
>>>>> + * libgcc/libgcov.c
>>>>> + *
>>>>> + * Uses gcc-internal data definitions.
>>>>> + *
>>>>> + * Imported from Linux and modified for Xen by
>>>>> + * Wei Liu <wei.liu2@citrix.com>
>>>>> + */
>>>>> +
>>>>> +#include <xen/string.h>
>>>>> +
>>>>> +#include "gcov.h"
>>>>> +
>>>>> +#if GCC_VERSION < 40700
>>>>> +#error "Wrong version of GCC used to compile gcov"
>>>>> +#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
>>>> I'm sorry for not having pointed this out on v2 (I had noticed it,
>>>> but then didn't finish analyzing the situation), but I'm afraid this
>>>> together with ...
>>>>
>>>>> +struct gcov_info {
>>>>> + unsigned int version;
>>>>> + struct gcov_info *next;
>>>>> + unsigned int stamp;
>>>>> + const char *filename;
>>>>> + void (*merge[GCOV_COUNTERS])(gcov_type *, unsigned int);
>>>>> + unsigned int n_functions;
>>>>> + struct gcov_fn_info **functions;
>>>>> +};
>>>> ... this structure's trailing fields actually getting used by the code
>>>> won't work well when changing compiler versions without cleaning
>>>> the tree. I think instead you need thin gcc_5.c and gcc_4_9.c
>>>> #define-ing their GCOV_COUNTERS and then #include-ing this
>>>> shared source file. Plus btw, I don't think gcc 5.0.x (the
>>>> development variant of 5.x) would use anything different from
>>>> 5.1.x or 5.2.x; in fact use of __GNUC_MINOR__ should not
>>>> normally be necessary anymore with gcc 5+.
>>>>
>>> Right. I will do something about this. Thanks for catching this.
>>>
>>>> And then - how is all of this supposed to be working in conjucntion
>>>> with live patching, where the patch may have been created by yet
>>>> another compiler version?
>>>>
>>> There is a version field in gcov_info, so we can compare that and reject
>>> incompatible version.
>>>
>>> We need to use hooks in livepatching to call the constructor /
>>> destructor when applying / reverting a live-patch. We might need to be
>>> cautious about locks or whatever, but I'm sure it can be figured out.
>>>
>>> But I have no idea how useful it would be to use gcov and livepatching
>>> together. For now the easiest thing to do is to
>>>
>>> depends on !LIVEPATCH
>>>
>>> in Kconfig.
>> Wouldn't it be just as easy, and more useful, to set a "has been
>> livepatched" flag, and return errors for all gcov hypercalls if its' set?
>>
>> I would expect most users to want to build a single hypervisor that can
>> be used for both gcov testing and live patching (under different
>> circumstances).
> I mean software provider, not user, of course. That's what I would want
> for CentOS, and I'm sure that's what the XenServer (and probably Oracle)
> guys want as well.
GCOV is majority invasive, both in terms of performance (every basic
block needs to do a locked increment of a counter) and data size
(metadata for all basic blocks).
No software vendor is ever going to have it enabled in a production
scenario.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 58+ messages in thread* Re: [PATCH v3] gcov: add new interface and 3.4 and 4.7 format support
2016-10-12 13:34 ` Andrew Cooper
@ 2016-10-12 13:41 ` George Dunlap
2016-10-12 13:43 ` Andrew Cooper
0 siblings, 1 reply; 58+ messages in thread
From: George Dunlap @ 2016-10-12 13:41 UTC (permalink / raw)
To: Andrew Cooper, Wei Liu, Jan Beulich
Cc: Stefano Stabellini, George Dunlap, Tim Deegan, Ian Jackson,
philippe.gabriel, Xen-devel
On 12/10/16 14:34, Andrew Cooper wrote:
> On 12/10/16 14:26, George Dunlap wrote:
>> On 12/10/16 14:24, George Dunlap wrote:
>>> On 12/10/16 14:06, Wei Liu wrote:
>>>> On Wed, Oct 12, 2016 at 06:42:53AM -0600, Jan Beulich wrote:
>>>>>>>> On 11.10.16 at 12:31, <wei.liu2@citrix.com> wrote:
>>>>>> --- /dev/null
>>>>>> +++ b/xen/common/gcov/gcc_4_7.c
>>>>>> @@ -0,0 +1,205 @@
>>>>>> +/*
>>>>>> + * This code provides functions to handle gcc's profiling data format
>>>>>> + * introduced with gcc 4.7.
>>>>>> + *
>>>>>> + * This file is based heavily on gcc_3_4.c file.
>>>>>> + *
>>>>>> + * For a better understanding, refer to gcc source:
>>>>>> + * gcc/gcov-io.h
>>>>>> + * libgcc/libgcov.c
>>>>>> + *
>>>>>> + * Uses gcc-internal data definitions.
>>>>>> + *
>>>>>> + * Imported from Linux and modified for Xen by
>>>>>> + * Wei Liu <wei.liu2@citrix.com>
>>>>>> + */
>>>>>> +
>>>>>> +#include <xen/string.h>
>>>>>> +
>>>>>> +#include "gcov.h"
>>>>>> +
>>>>>> +#if GCC_VERSION < 40700
>>>>>> +#error "Wrong version of GCC used to compile gcov"
>>>>>> +#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
>>>>> I'm sorry for not having pointed this out on v2 (I had noticed it,
>>>>> but then didn't finish analyzing the situation), but I'm afraid this
>>>>> together with ...
>>>>>
>>>>>> +struct gcov_info {
>>>>>> + unsigned int version;
>>>>>> + struct gcov_info *next;
>>>>>> + unsigned int stamp;
>>>>>> + const char *filename;
>>>>>> + void (*merge[GCOV_COUNTERS])(gcov_type *, unsigned int);
>>>>>> + unsigned int n_functions;
>>>>>> + struct gcov_fn_info **functions;
>>>>>> +};
>>>>> ... this structure's trailing fields actually getting used by the code
>>>>> won't work well when changing compiler versions without cleaning
>>>>> the tree. I think instead you need thin gcc_5.c and gcc_4_9.c
>>>>> #define-ing their GCOV_COUNTERS and then #include-ing this
>>>>> shared source file. Plus btw, I don't think gcc 5.0.x (the
>>>>> development variant of 5.x) would use anything different from
>>>>> 5.1.x or 5.2.x; in fact use of __GNUC_MINOR__ should not
>>>>> normally be necessary anymore with gcc 5+.
>>>>>
>>>> Right. I will do something about this. Thanks for catching this.
>>>>
>>>>> And then - how is all of this supposed to be working in conjucntion
>>>>> with live patching, where the patch may have been created by yet
>>>>> another compiler version?
>>>>>
>>>> There is a version field in gcov_info, so we can compare that and reject
>>>> incompatible version.
>>>>
>>>> We need to use hooks in livepatching to call the constructor /
>>>> destructor when applying / reverting a live-patch. We might need to be
>>>> cautious about locks or whatever, but I'm sure it can be figured out.
>>>>
>>>> But I have no idea how useful it would be to use gcov and livepatching
>>>> together. For now the easiest thing to do is to
>>>>
>>>> depends on !LIVEPATCH
>>>>
>>>> in Kconfig.
>>> Wouldn't it be just as easy, and more useful, to set a "has been
>>> livepatched" flag, and return errors for all gcov hypercalls if its' set?
>>>
>>> I would expect most users to want to build a single hypervisor that can
>>> be used for both gcov testing and live patching (under different
>>> circumstances).
>> I mean software provider, not user, of course. That's what I would want
>> for CentOS, and I'm sure that's what the XenServer (and probably Oracle)
>> guys want as well.
>
> GCOV is majority invasive, both in terms of performance (every basic
> block needs to do a locked increment of a counter) and data size
> (metadata for all basic blocks).
>
> No software vendor is ever going to have it enabled in a production
> scenario.
You're right, I wasn't thinking.
But also presumably you'd like to be able to minimize the difference
between the thing you're testing and the thing you ship; having to
disable LivePatch increases the delta slightly.
Anyway, I still think having them both Kconfig-ured is a good idea, but
not so much that I'd spend any more time arguing for it.
-George
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 58+ messages in thread* Re: [PATCH v3] gcov: add new interface and 3.4 and 4.7 format support
2016-10-12 13:41 ` George Dunlap
@ 2016-10-12 13:43 ` Andrew Cooper
0 siblings, 0 replies; 58+ messages in thread
From: Andrew Cooper @ 2016-10-12 13:43 UTC (permalink / raw)
To: George Dunlap, Wei Liu, Jan Beulich
Cc: Stefano Stabellini, George Dunlap, Tim Deegan, Ian Jackson,
philippe.gabriel, Xen-devel
On 12/10/16 14:41, George Dunlap wrote:
> On 12/10/16 14:34, Andrew Cooper wrote:
>> On 12/10/16 14:26, George Dunlap wrote:
>>> On 12/10/16 14:24, George Dunlap wrote:
>>>> On 12/10/16 14:06, Wei Liu wrote:
>>>>> On Wed, Oct 12, 2016 at 06:42:53AM -0600, Jan Beulich wrote:
>>>>>>>>> On 11.10.16 at 12:31, <wei.liu2@citrix.com> wrote:
>>>>>>> --- /dev/null
>>>>>>> +++ b/xen/common/gcov/gcc_4_7.c
>>>>>>> @@ -0,0 +1,205 @@
>>>>>>> +/*
>>>>>>> + * This code provides functions to handle gcc's profiling data format
>>>>>>> + * introduced with gcc 4.7.
>>>>>>> + *
>>>>>>> + * This file is based heavily on gcc_3_4.c file.
>>>>>>> + *
>>>>>>> + * For a better understanding, refer to gcc source:
>>>>>>> + * gcc/gcov-io.h
>>>>>>> + * libgcc/libgcov.c
>>>>>>> + *
>>>>>>> + * Uses gcc-internal data definitions.
>>>>>>> + *
>>>>>>> + * Imported from Linux and modified for Xen by
>>>>>>> + * Wei Liu <wei.liu2@citrix.com>
>>>>>>> + */
>>>>>>> +
>>>>>>> +#include <xen/string.h>
>>>>>>> +
>>>>>>> +#include "gcov.h"
>>>>>>> +
>>>>>>> +#if GCC_VERSION < 40700
>>>>>>> +#error "Wrong version of GCC used to compile gcov"
>>>>>>> +#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
>>>>>> I'm sorry for not having pointed this out on v2 (I had noticed it,
>>>>>> but then didn't finish analyzing the situation), but I'm afraid this
>>>>>> together with ...
>>>>>>
>>>>>>> +struct gcov_info {
>>>>>>> + unsigned int version;
>>>>>>> + struct gcov_info *next;
>>>>>>> + unsigned int stamp;
>>>>>>> + const char *filename;
>>>>>>> + void (*merge[GCOV_COUNTERS])(gcov_type *, unsigned int);
>>>>>>> + unsigned int n_functions;
>>>>>>> + struct gcov_fn_info **functions;
>>>>>>> +};
>>>>>> ... this structure's trailing fields actually getting used by the code
>>>>>> won't work well when changing compiler versions without cleaning
>>>>>> the tree. I think instead you need thin gcc_5.c and gcc_4_9.c
>>>>>> #define-ing their GCOV_COUNTERS and then #include-ing this
>>>>>> shared source file. Plus btw, I don't think gcc 5.0.x (the
>>>>>> development variant of 5.x) would use anything different from
>>>>>> 5.1.x or 5.2.x; in fact use of __GNUC_MINOR__ should not
>>>>>> normally be necessary anymore with gcc 5+.
>>>>>>
>>>>> Right. I will do something about this. Thanks for catching this.
>>>>>
>>>>>> And then - how is all of this supposed to be working in conjucntion
>>>>>> with live patching, where the patch may have been created by yet
>>>>>> another compiler version?
>>>>>>
>>>>> There is a version field in gcov_info, so we can compare that and reject
>>>>> incompatible version.
>>>>>
>>>>> We need to use hooks in livepatching to call the constructor /
>>>>> destructor when applying / reverting a live-patch. We might need to be
>>>>> cautious about locks or whatever, but I'm sure it can be figured out.
>>>>>
>>>>> But I have no idea how useful it would be to use gcov and livepatching
>>>>> together. For now the easiest thing to do is to
>>>>>
>>>>> depends on !LIVEPATCH
>>>>>
>>>>> in Kconfig.
>>>> Wouldn't it be just as easy, and more useful, to set a "has been
>>>> livepatched" flag, and return errors for all gcov hypercalls if its' set?
>>>>
>>>> I would expect most users to want to build a single hypervisor that can
>>>> be used for both gcov testing and live patching (under different
>>>> circumstances).
>>> I mean software provider, not user, of course. That's what I would want
>>> for CentOS, and I'm sure that's what the XenServer (and probably Oracle)
>>> guys want as well.
>> GCOV is majority invasive, both in terms of performance (every basic
>> block needs to do a locked increment of a counter) and data size
>> (metadata for all basic blocks).
>>
>> No software vendor is ever going to have it enabled in a production
>> scenario.
> You're right, I wasn't thinking.
>
> But also presumably you'd like to be able to minimize the difference
> between the thing you're testing and the thing you ship; having to
> disable LivePatch increases the delta slightly.
>
> Anyway, I still think having them both Kconfig-ured is a good idea, but
> not so much that I'd spend any more time arguing for it.
It is a testing feature. It would certainly be nice to get code
coverage of the livepatching parts, but that absolutely shouldn't block
making gcov usable in the first place.
It might be worth sticking a #TODO make GCOV worth with Livepatching in
the kconfig file (for anyone who stumbles across this dependency).
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH v3] gcov: add new interface and 3.4 and 4.7 format support
2016-10-12 13:24 ` George Dunlap
2016-10-12 13:26 ` George Dunlap
@ 2016-10-12 13:29 ` Konrad Rzeszutek Wilk
2016-10-12 13:40 ` Wei Liu
1 sibling, 1 reply; 58+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-10-12 13:29 UTC (permalink / raw)
To: George Dunlap
Cc: Stefano Stabellini, Wei Liu, George Dunlap, AndrewCooper,
Ian Jackson, Tim Deegan, philippe.gabriel, Jan Beulich,
Xen-devel
On Wed, Oct 12, 2016 at 02:24:51PM +0100, George Dunlap wrote:
> On 12/10/16 14:06, Wei Liu wrote:
> > On Wed, Oct 12, 2016 at 06:42:53AM -0600, Jan Beulich wrote:
> >>>>> On 11.10.16 at 12:31, <wei.liu2@citrix.com> wrote:
> >>> --- /dev/null
> >>> +++ b/xen/common/gcov/gcc_4_7.c
> >>> @@ -0,0 +1,205 @@
> >>> +/*
> >>> + * This code provides functions to handle gcc's profiling data format
> >>> + * introduced with gcc 4.7.
> >>> + *
> >>> + * This file is based heavily on gcc_3_4.c file.
> >>> + *
> >>> + * For a better understanding, refer to gcc source:
> >>> + * gcc/gcov-io.h
> >>> + * libgcc/libgcov.c
> >>> + *
> >>> + * Uses gcc-internal data definitions.
> >>> + *
> >>> + * Imported from Linux and modified for Xen by
> >>> + * Wei Liu <wei.liu2@citrix.com>
> >>> + */
> >>> +
> >>> +#include <xen/string.h>
> >>> +
> >>> +#include "gcov.h"
> >>> +
> >>> +#if GCC_VERSION < 40700
> >>> +#error "Wrong version of GCC used to compile gcov"
> >>> +#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
> >>
> >> I'm sorry for not having pointed this out on v2 (I had noticed it,
> >> but then didn't finish analyzing the situation), but I'm afraid this
> >> together with ...
> >>
> >>> +struct gcov_info {
> >>> + unsigned int version;
> >>> + struct gcov_info *next;
> >>> + unsigned int stamp;
> >>> + const char *filename;
> >>> + void (*merge[GCOV_COUNTERS])(gcov_type *, unsigned int);
> >>> + unsigned int n_functions;
> >>> + struct gcov_fn_info **functions;
> >>> +};
> >>
> >> ... this structure's trailing fields actually getting used by the code
> >> won't work well when changing compiler versions without cleaning
> >> the tree. I think instead you need thin gcc_5.c and gcc_4_9.c
> >> #define-ing their GCOV_COUNTERS and then #include-ing this
> >> shared source file. Plus btw, I don't think gcc 5.0.x (the
> >> development variant of 5.x) would use anything different from
> >> 5.1.x or 5.2.x; in fact use of __GNUC_MINOR__ should not
> >> normally be necessary anymore with gcc 5+.
> >>
> >
> > Right. I will do something about this. Thanks for catching this.
> >
> >> And then - how is all of this supposed to be working in conjucntion
> >> with live patching, where the patch may have been created by yet
> >> another compiler version?
> >>
> >
> > There is a version field in gcov_info, so we can compare that and reject
> > incompatible version.
> >
> > We need to use hooks in livepatching to call the constructor /
> > destructor when applying / reverting a live-patch. We might need to be
> > cautious about locks or whatever, but I'm sure it can be figured out.
> >
> > But I have no idea how useful it would be to use gcov and livepatching
> > together. For now the easiest thing to do is to
> >
> > depends on !LIVEPATCH
> >
> > in Kconfig.
>
> Wouldn't it be just as easy, and more useful, to set a "has been
> livepatched" flag, and return errors for all gcov hypercalls if its' set?
>
> I would expect most users to want to build a single hypervisor that can
> be used for both gcov testing and live patching (under different
> circumstances).
I actually would welcome livepatching and gcov to see if the test-cases I wrote
cover most of the code.
Adding in checking livepatch (common/livepatch.c: prepare_payload) to examine
the .gcov_info and see if it matches the hypervisor one, is fine too.
>
> -George
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 58+ messages in thread* Re: [PATCH v3] gcov: add new interface and 3.4 and 4.7 format support
2016-10-12 13:29 ` Konrad Rzeszutek Wilk
@ 2016-10-12 13:40 ` Wei Liu
2016-10-12 13:46 ` Konrad Rzeszutek Wilk
0 siblings, 1 reply; 58+ messages in thread
From: Wei Liu @ 2016-10-12 13:40 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: Stefano Stabellini, Wei Liu, George Dunlap, AndrewCooper,
Ian Jackson, George Dunlap, Tim Deegan, philippe.gabriel,
Jan Beulich, Xen-devel
On Wed, Oct 12, 2016 at 09:29:04AM -0400, Konrad Rzeszutek Wilk wrote:
[...]
> >
> > Wouldn't it be just as easy, and more useful, to set a "has been
> > livepatched" flag, and return errors for all gcov hypercalls if its' set?
> >
> > I would expect most users to want to build a single hypervisor that can
> > be used for both gcov testing and live patching (under different
> > circumstances).
>
> I actually would welcome livepatching and gcov to see if the test-cases I wrote
> cover most of the code.
>
I don't follow. Gcov doesn't give you a call graph -- if that's what
you're after.
> Adding in checking livepatch (common/livepatch.c: prepare_payload) to examine
> the .gcov_info and see if it matches the hypervisor one, is fine too.
>
This then involves a non-trivial amount of work to figure out all the
corner cases. It's better to defer that to a later stage.
Wei.
> >
> > -George
> >
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH v3] gcov: add new interface and 3.4 and 4.7 format support
2016-10-12 13:40 ` Wei Liu
@ 2016-10-12 13:46 ` Konrad Rzeszutek Wilk
2016-10-12 13:50 ` Wei Liu
0 siblings, 1 reply; 58+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-10-12 13:46 UTC (permalink / raw)
To: Wei Liu
Cc: Stefano Stabellini, George Dunlap, AndrewCooper, Ian Jackson,
George Dunlap, Tim Deegan, philippe.gabriel, Jan Beulich,
Xen-devel
On Wed, Oct 12, 2016 at 02:40:08PM +0100, Wei Liu wrote:
> On Wed, Oct 12, 2016 at 09:29:04AM -0400, Konrad Rzeszutek Wilk wrote:
> [...]
> > >
> > > Wouldn't it be just as easy, and more useful, to set a "has been
> > > livepatched" flag, and return errors for all gcov hypercalls if its' set?
> > >
> > > I would expect most users to want to build a single hypervisor that can
> > > be used for both gcov testing and live patching (under different
> > > circumstances).
> >
> > I actually would welcome livepatching and gcov to see if the test-cases I wrote
> > cover most of the code.
> >
>
> I don't follow. Gcov doesn't give you a call graph -- if that's what
> you're after.
It gives me an idea which functions/branches have run (not the livepatch
itself - just the infrastructure around adding a livepatch, doing ELF checks, etc).
And more importantly - which ones haven't run and need some more test-cases.
>
> > Adding in checking livepatch (common/livepatch.c: prepare_payload) to examine
> > the .gcov_info and see if it matches the hypervisor one, is fine too.
> >
>
> This then involves a non-trivial amount of work to figure out all the
> corner cases. It's better to defer that to a later stage.
Sure thing. And the !LIVEPATCH is fine for now. I just need to get an idea
of what this would entail so it can get done.
>
> Wei.
>
> > >
> > > -George
> > >
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH v3] gcov: add new interface and 3.4 and 4.7 format support
2016-10-12 13:46 ` Konrad Rzeszutek Wilk
@ 2016-10-12 13:50 ` Wei Liu
0 siblings, 0 replies; 58+ messages in thread
From: Wei Liu @ 2016-10-12 13:50 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: Stefano Stabellini, Wei Liu, George Dunlap, AndrewCooper,
Ian Jackson, George Dunlap, Tim Deegan, philippe.gabriel,
Jan Beulich, Xen-devel
On Wed, Oct 12, 2016 at 09:46:51AM -0400, Konrad Rzeszutek Wilk wrote:
> On Wed, Oct 12, 2016 at 02:40:08PM +0100, Wei Liu wrote:
> > On Wed, Oct 12, 2016 at 09:29:04AM -0400, Konrad Rzeszutek Wilk wrote:
> > [...]
> > > >
> > > > Wouldn't it be just as easy, and more useful, to set a "has been
> > > > livepatched" flag, and return errors for all gcov hypercalls if its' set?
> > > >
> > > > I would expect most users to want to build a single hypervisor that can
> > > > be used for both gcov testing and live patching (under different
> > > > circumstances).
> > >
> > > I actually would welcome livepatching and gcov to see if the test-cases I wrote
> > > cover most of the code.
> > >
> >
> > I don't follow. Gcov doesn't give you a call graph -- if that's what
> > you're after.
>
> It gives me an idea which functions/branches have run (not the livepatch
> itself - just the infrastructure around adding a livepatch, doing ELF checks, etc).
>
> And more importantly - which ones haven't run and need some more test-cases.
>
Right. Thanks for the explanation. That would indeed be useful.
Wei.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH v3] gcov: add new interface and 3.4 and 4.7 format support
2016-10-12 12:42 ` Jan Beulich
2016-10-12 13:06 ` Wei Liu
@ 2016-10-12 13:23 ` Konrad Rzeszutek Wilk
2016-10-12 13:31 ` Jan Beulich
2016-10-12 15:33 ` Wei Liu
2 siblings, 1 reply; 58+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-10-12 13:23 UTC (permalink / raw)
To: Jan Beulich
Cc: Stefano Stabellini, Wei Liu, George Dunlap, AndrewCooper,
Ian Jackson, Tim Deegan, philippe.gabriel, Xen-devel
> And then - how is all of this supposed to be working in conjucntion
> with live patching, where the patch may have been created by yet
> another compiler version?
Uh, I hope one does not create a livepatch patch with another compiler version!
Let me put on the TODO to make livepatch-build-tools check gcc against
compile.h so that one does not try this.
>
> Jan
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH v3] gcov: add new interface and 3.4 and 4.7 format support
2016-10-12 13:23 ` Konrad Rzeszutek Wilk
@ 2016-10-12 13:31 ` Jan Beulich
2016-10-12 13:44 ` Konrad Rzeszutek Wilk
0 siblings, 1 reply; 58+ messages in thread
From: Jan Beulich @ 2016-10-12 13:31 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: Stefano Stabellini, Wei Liu, George Dunlap, AndrewCooper,
Ian Jackson, Tim Deegan, philippe.gabriel, Xen-devel
>>> On 12.10.16 at 15:23, <konrad.wilk@oracle.com> wrote:
>> And then - how is all of this supposed to be working in conjucntion
>> with live patching, where the patch may have been created by yet
>> another compiler version?
>
> Uh, I hope one does not create a livepatch patch with another compiler
> version!
>
> Let me put on the TODO to make livepatch-build-tools check gcc against
> compile.h so that one does not try this.
What's wrong with mixing compiler versions in general?
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH v3] gcov: add new interface and 3.4 and 4.7 format support
2016-10-12 13:31 ` Jan Beulich
@ 2016-10-12 13:44 ` Konrad Rzeszutek Wilk
2016-10-12 14:08 ` Jan Beulich
2016-10-12 14:17 ` Martin Pohlack
0 siblings, 2 replies; 58+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-10-12 13:44 UTC (permalink / raw)
To: Jan Beulich, mpohlack
Cc: Stefano Stabellini, Wei Liu, George Dunlap, AndrewCooper,
Ian Jackson, Tim Deegan, philippe.gabriel, Xen-devel
On Wed, Oct 12, 2016 at 07:31:52AM -0600, Jan Beulich wrote:
> >>> On 12.10.16 at 15:23, <konrad.wilk@oracle.com> wrote:
> >> And then - how is all of this supposed to be working in conjucntion
> >> with live patching, where the patch may have been created by yet
> >> another compiler version?
> >
> > Uh, I hope one does not create a livepatch patch with another compiler
> > version!
> >
> > Let me put on the TODO to make livepatch-build-tools check gcc against
> > compile.h so that one does not try this.
>
> What's wrong with mixing compiler versions in general?
Besides scaring me?
The one issue we had encountered was with compilers generating random named
symbols for the switch tables. Those end up being called "CSWTCH.XYZ"
where the XYZ depends on the position of the moon along with how many
goats you have sacrificied to the altar of GCC gods.
Older compilers don't neccessarily do it, newer ones do, and every time
you build an livepatch the naming is different. Frustrating.
It maybe that newer versions of GCC are more predictable about this
naming.
Maybe Martin can share some of his experience? CC-ing him.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH v3] gcov: add new interface and 3.4 and 4.7 format support
2016-10-12 13:44 ` Konrad Rzeszutek Wilk
@ 2016-10-12 14:08 ` Jan Beulich
2016-10-12 14:17 ` Martin Pohlack
1 sibling, 0 replies; 58+ messages in thread
From: Jan Beulich @ 2016-10-12 14:08 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: Stefano Stabellini, Wei Liu, George Dunlap, AndrewCooper,
Ian Jackson, Tim Deegan, mpohlack, philippe.gabriel, Xen-devel
>>> On 12.10.16 at 15:44, <konrad.wilk@oracle.com> wrote:
> On Wed, Oct 12, 2016 at 07:31:52AM -0600, Jan Beulich wrote:
>> >>> On 12.10.16 at 15:23, <konrad.wilk@oracle.com> wrote:
>> >> And then - how is all of this supposed to be working in conjucntion
>> >> with live patching, where the patch may have been created by yet
>> >> another compiler version?
>> >
>> > Uh, I hope one does not create a livepatch patch with another compiler
>> > version!
>> >
>> > Let me put on the TODO to make livepatch-build-tools check gcc against
>> > compile.h so that one does not try this.
>>
>> What's wrong with mixing compiler versions in general?
>
> Besides scaring me?
What is it that scares you?
> The one issue we had encountered was with compilers generating random named
> symbols for the switch tables. Those end up being called "CSWTCH.XYZ"
> where the XYZ depends on the position of the moon along with how many
> goats you have sacrificied to the altar of GCC gods.
>
> Older compilers don't neccessarily do it, newer ones do, and every time
> you build an livepatch the naming is different. Frustrating.
But this would mean you don't just depend on gcc version, but even
on the specific build (as the numbering you refer to may change with
whatever patches a distro applies on top of the upstream tarball, as
well as perhaps with configure and build options).
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH v3] gcov: add new interface and 3.4 and 4.7 format support
2016-10-12 13:44 ` Konrad Rzeszutek Wilk
2016-10-12 14:08 ` Jan Beulich
@ 2016-10-12 14:17 ` Martin Pohlack
2016-10-12 16:21 ` Konrad Rzeszutek Wilk
1 sibling, 1 reply; 58+ messages in thread
From: Martin Pohlack @ 2016-10-12 14:17 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk, Jan Beulich, mpohlack
Cc: Stefano Stabellini, Wei Liu, George Dunlap, AndrewCooper,
Ian Jackson, Tim Deegan, philippe.gabriel, Xen-devel
On 12.10.2016 15:44, Konrad Rzeszutek Wilk wrote:
> On Wed, Oct 12, 2016 at 07:31:52AM -0600, Jan Beulich wrote:
>>>>> On 12.10.16 at 15:23, <konrad.wilk@oracle.com> wrote:
>>>> And then - how is all of this supposed to be working in conjucntion
>>>> with live patching, where the patch may have been created by yet
>>>> another compiler version?
>>>
>>> Uh, I hope one does not create a livepatch patch with another compiler
>>> version!
>>>
>>> Let me put on the TODO to make livepatch-build-tools check gcc against
>>> compile.h so that one does not try this.
>>
>> What's wrong with mixing compiler versions in general?
>
> Besides scaring me?
>
> The one issue we had encountered was with compilers generating random named
> symbols for the switch tables. Those end up being called "CSWTCH.XYZ"
> where the XYZ depends on the position of the moon along with how many
> goats you have sacrificied to the altar of GCC gods.
>
> Older compilers don't neccessarily do it, newer ones do, and every time
> you build an livepatch the naming is different. Frustrating.
>
> It maybe that newer versions of GCC are more predictable about this
> naming.
>
> Maybe Martin can share some of his experience? CC-ing him.
There are a couple of naming conventions for internal symbols and also
static symbols where you basically have to pray that gcc implementation
does not change. Interestingly, icc has some conventions that make
those symbol names a bit more stable.
The tricky thing is matchmaking between the existing build and the new
build to construct the binary diff and to match up symbols for which you
want to provide replacement code.
We use a reproducible build environment to construct hotpatches for an
existing build in the absolutely same environment (gcc version, lib
versions, gas version, binutils etc.). This sidesteps most of the problems.
Martin
Amazon Development Center Germany GmbH
Berlin - Dresden - Aachen
main office: Krausenstr. 38, 10117 Berlin
Geschaeftsfuehrer: Dr. Ralf Herbrich, Christian Schlaeger
Ust-ID: DE289237879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH v3] gcov: add new interface and 3.4 and 4.7 format support
2016-10-12 14:17 ` Martin Pohlack
@ 2016-10-12 16:21 ` Konrad Rzeszutek Wilk
2016-10-13 8:05 ` Martin Pohlack
0 siblings, 1 reply; 58+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-10-12 16:21 UTC (permalink / raw)
To: Martin Pohlack
Cc: Stefano Stabellini, Wei Liu, George Dunlap, AndrewCooper,
Ian Jackson, Tim Deegan, mpohlack, philippe.gabriel, Jan Beulich,
Xen-devel
On Wed, Oct 12, 2016 at 04:17:57PM +0200, Martin Pohlack wrote:
> On 12.10.2016 15:44, Konrad Rzeszutek Wilk wrote:
> > On Wed, Oct 12, 2016 at 07:31:52AM -0600, Jan Beulich wrote:
> > > > > > On 12.10.16 at 15:23, <konrad.wilk@oracle.com> wrote:
> > > > > And then - how is all of this supposed to be working in conjucntion
> > > > > with live patching, where the patch may have been created by yet
> > > > > another compiler version?
> > > >
> > > > Uh, I hope one does not create a livepatch patch with another compiler
> > > > version!
> > > >
> > > > Let me put on the TODO to make livepatch-build-tools check gcc against
> > > > compile.h so that one does not try this.
> > >
> > > What's wrong with mixing compiler versions in general?
> >
> > Besides scaring me?
> >
> > The one issue we had encountered was with compilers generating random named
> > symbols for the switch tables. Those end up being called "CSWTCH.XYZ"
> > where the XYZ depends on the position of the moon along with how many
> > goats you have sacrificied to the altar of GCC gods.
> >
> > Older compilers don't neccessarily do it, newer ones do, and every time
> > you build an livepatch the naming is different. Frustrating.
> >
> > It maybe that newer versions of GCC are more predictable about this
> > naming.
> >
> > Maybe Martin can share some of his experience? CC-ing him.
>
> There are a couple of naming conventions for internal symbols and also
> static symbols where you basically have to pray that gcc implementation does
> not change. Interestingly, icc has some conventions that make those symbol
> names a bit more stable.
>
> The tricky thing is matchmaking between the existing build and the new build
> to construct the binary diff and to match up symbols for which you want to
> provide replacement code.
>
> We use a reproducible build environment to construct hotpatches for an
> existing build in the absolutely same environment (gcc version, lib
> versions, gas version, binutils etc.). This sidesteps most of the problems.
I think the matchmaking process does not solve per say some tricky CSWTCH issues.
If a patch mucks with a switch statement (e.g. add a new case) we are pretty much
guaranteed to get in trouble. And really a change in any control structure may cause
gcc to take different code path, causing it to renumber CSWTCH. Or worst, renumber
it to the one that the hypervisor is using for some other switch statements.
I think the size of the symbol vs the one in the hypervisor is different
so one can check for this. Bad things happen if it is the same size, but bcmp
can come in handy there.
Are there any ways to make GCC be predictable or some patches
to make GCC be less random. Perhaps instead of XYZ it would use the function name..
P.S.
GCC scares me because the code comes in these big patches with not much explanation
on how it suppose to work. It probably is a piece of cake for folks who
have been marinating in compilers but for a newbie like me it is hardcore
black magic.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH v3] gcov: add new interface and 3.4 and 4.7 format support
2016-10-12 16:21 ` Konrad Rzeszutek Wilk
@ 2016-10-13 8:05 ` Martin Pohlack
0 siblings, 0 replies; 58+ messages in thread
From: Martin Pohlack @ 2016-10-13 8:05 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: Stefano Stabellini, Wei Liu, George Dunlap, AndrewCooper,
Ian Jackson, Tim Deegan, mpohlack, philippe.gabriel, Jan Beulich,
Xen-devel
On 12.10.2016 18:21, Konrad Rzeszutek Wilk wrote:
> On Wed, Oct 12, 2016 at 04:17:57PM +0200, Martin Pohlack wrote:
>> On 12.10.2016 15:44, Konrad Rzeszutek Wilk wrote:
>>> On Wed, Oct 12, 2016 at 07:31:52AM -0600, Jan Beulich wrote:
>>>>>>> On 12.10.16 at 15:23, <konrad.wilk@oracle.com> wrote:
>>>>>> And then - how is all of this supposed to be working in conjucntion
>>>>>> with live patching, where the patch may have been created by yet
>>>>>> another compiler version?
>>>>>
>>>>> Uh, I hope one does not create a livepatch patch with another compiler
>>>>> version!
>>>>>
>>>>> Let me put on the TODO to make livepatch-build-tools check gcc against
>>>>> compile.h so that one does not try this.
>>>>
>>>> What's wrong with mixing compiler versions in general?
>>>
>>> Besides scaring me?
>>>
>>> The one issue we had encountered was with compilers generating random named
>>> symbols for the switch tables. Those end up being called "CSWTCH.XYZ"
>>> where the XYZ depends on the position of the moon along with how many
>>> goats you have sacrificied to the altar of GCC gods.
>>>
>>> Older compilers don't neccessarily do it, newer ones do, and every time
>>> you build an livepatch the naming is different. Frustrating.
>>>
>>> It maybe that newer versions of GCC are more predictable about this
>>> naming.
>>>
>>> Maybe Martin can share some of his experience? CC-ing him.
>>
>> There are a couple of naming conventions for internal symbols and also
>> static symbols where you basically have to pray that gcc implementation does
>> not change. Interestingly, icc has some conventions that make those symbol
>> names a bit more stable.
>>
>> The tricky thing is matchmaking between the existing build and the new build
>> to construct the binary diff and to match up symbols for which you want to
>> provide replacement code.
>>
>> We use a reproducible build environment to construct hotpatches for an
>> existing build in the absolutely same environment (gcc version, lib
>> versions, gas version, binutils etc.). This sidesteps most of the problems.
>
> I think the matchmaking process does not solve per say some tricky CSWTCH issues.
>
> If a patch mucks with a switch statement (e.g. add a new case) we are pretty much
> guaranteed to get in trouble. And really a change in any control structure may cause
> gcc to take different code path, causing it to renumber CSWTCH. Or worst, renumber
> it to the one that the hypervisor is using for some other switch statements.
> I think the size of the symbol vs the one in the hypervisor is different
> so one can check for this. Bad things happen if it is the same size, but bcmp
> can come in handy there.
If you change a switch statement, the containing function's binary
representation will change (+ inlining effects). This means you will
have to ship a new version of this function with the hotpatch. I have
seen gcc put jump tables for switch statements into dedicated .rodata*
sections, at least with -ffunction-sections and -fdata-sections. You
need to treat those as belonging to the corresponding function and also
ship them with the hotpatch. If you restrict the match-making to
function-level symbols and retain references between such a function and
its rodata section, you should be fine.
> Are there any ways to make GCC be predictable or some patches
> to make GCC be less random. Perhaps instead of XYZ it would use the function name..
You sidestep some issues by making source-code patches as line-neutral
as possible and introducing new symbols and definitions close to usage
instead of in header files for hotpatches. This reduces cascading
effects for such renames. Some luck, tweaking, and inlining of
definitions is sometimes required.
Martin
> P.S.
> GCC scares me because the code comes in these big patches with not much explanation
> on how it suppose to work. It probably is a piece of cake for folks who
> have been marinating in compilers but for a newbie like me it is hardcore
> black magic.
Amazon Development Center Germany GmbH
Berlin - Dresden - Aachen
main office: Krausenstr. 38, 10117 Berlin
Geschaeftsfuehrer: Dr. Ralf Herbrich, Christian Schlaeger
Ust-ID: DE289237879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH v3] gcov: add new interface and 3.4 and 4.7 format support
2016-10-12 12:42 ` Jan Beulich
2016-10-12 13:06 ` Wei Liu
2016-10-12 13:23 ` Konrad Rzeszutek Wilk
@ 2016-10-12 15:33 ` Wei Liu
2016-10-12 15:42 ` Jan Beulich
2 siblings, 1 reply; 58+ messages in thread
From: Wei Liu @ 2016-10-12 15:33 UTC (permalink / raw)
To: Jan Beulich
Cc: Stefano Stabellini, Wei Liu, George Dunlap, AndrewCooper,
Ian Jackson, Tim Deegan, philippe.gabriel, Xen-devel
On Wed, Oct 12, 2016 at 06:42:53AM -0600, Jan Beulich wrote:
> >>> On 11.10.16 at 12:31, <wei.liu2@citrix.com> wrote:
> > --- /dev/null
> > +++ b/xen/common/gcov/gcc_4_7.c
> > @@ -0,0 +1,205 @@
> > +/*
> > + * This code provides functions to handle gcc's profiling data format
> > + * introduced with gcc 4.7.
> > + *
> > + * This file is based heavily on gcc_3_4.c file.
> > + *
> > + * For a better understanding, refer to gcc source:
> > + * gcc/gcov-io.h
> > + * libgcc/libgcov.c
> > + *
> > + * Uses gcc-internal data definitions.
> > + *
> > + * Imported from Linux and modified for Xen by
> > + * Wei Liu <wei.liu2@citrix.com>
> > + */
> > +
> > +#include <xen/string.h>
> > +
> > +#include "gcov.h"
> > +
> > +#if GCC_VERSION < 40700
> > +#error "Wrong version of GCC used to compile gcov"
> > +#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
>
> I'm sorry for not having pointed this out on v2 (I had noticed it,
> but then didn't finish analyzing the situation), but I'm afraid this
> together with ...
>
> > +struct gcov_info {
> > + unsigned int version;
> > + struct gcov_info *next;
> > + unsigned int stamp;
> > + const char *filename;
> > + void (*merge[GCOV_COUNTERS])(gcov_type *, unsigned int);
> > + unsigned int n_functions;
> > + struct gcov_fn_info **functions;
> > +};
>
> ... this structure's trailing fields actually getting used by the code
> won't work well when changing compiler versions without cleaning
> the tree. I think instead you need thin gcc_5.c and gcc_4_9.c
> #define-ing their GCOV_COUNTERS and then #include-ing this
> shared source file. Plus btw, I don't think gcc 5.0.x (the
> development variant of 5.x) would use anything different from
> 5.1.x or 5.2.x; in fact use of __GNUC_MINOR__ should not
> normally be necessary anymore with gcc 5+.
>
I think you misread here: __GNUC_MINOR__ is the "x" part of 5.x.y, the
"y" part is __GNUC_PATCHLEVEL__.
I've broken down things into several files as well as provided
corresponding Kconfig options:
gcc_4_7_base.c: the body of what is now gcc_4_7.c, better name is
welcome
gcc_4_7.c: gcov format in gcc version [4.7, 4,9), nr counters 8
gcc_4_9.c: gcov format in gcc version [4.9, 5.1), nr counters 9
gcc_5_1.c: gcov format in gcc version [5.1, ...), nr counters 10
Wei.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 58+ messages in thread* Re: [PATCH v3] gcov: add new interface and 3.4 and 4.7 format support
2016-10-12 15:33 ` Wei Liu
@ 2016-10-12 15:42 ` Jan Beulich
2016-10-12 17:07 ` Wei Liu
0 siblings, 1 reply; 58+ messages in thread
From: Jan Beulich @ 2016-10-12 15:42 UTC (permalink / raw)
To: Wei Liu
Cc: Stefano Stabellini, George Dunlap, AndrewCooper, Ian Jackson,
Tim Deegan, philippe.gabriel, Xen-devel
>>> On 12.10.16 at 17:33, <wei.liu2@citrix.com> wrote:
> On Wed, Oct 12, 2016 at 06:42:53AM -0600, Jan Beulich wrote:
>> >>> On 11.10.16 at 12:31, <wei.liu2@citrix.com> wrote:
>> > --- /dev/null
>> > +++ b/xen/common/gcov/gcc_4_7.c
>> > @@ -0,0 +1,205 @@
>> > +/*
>> > + * This code provides functions to handle gcc's profiling data format
>> > + * introduced with gcc 4.7.
>> > + *
>> > + * This file is based heavily on gcc_3_4.c file.
>> > + *
>> > + * For a better understanding, refer to gcc source:
>> > + * gcc/gcov-io.h
>> > + * libgcc/libgcov.c
>> > + *
>> > + * Uses gcc-internal data definitions.
>> > + *
>> > + * Imported from Linux and modified for Xen by
>> > + * Wei Liu <wei.liu2@citrix.com>
>> > + */
>> > +
>> > +#include <xen/string.h>
>> > +
>> > +#include "gcov.h"
>> > +
>> > +#if GCC_VERSION < 40700
>> > +#error "Wrong version of GCC used to compile gcov"
>> > +#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
>>
>> I'm sorry for not having pointed this out on v2 (I had noticed it,
>> but then didn't finish analyzing the situation), but I'm afraid this
>> together with ...
>>
>> > +struct gcov_info {
>> > + unsigned int version;
>> > + struct gcov_info *next;
>> > + unsigned int stamp;
>> > + const char *filename;
>> > + void (*merge[GCOV_COUNTERS])(gcov_type *, unsigned int);
>> > + unsigned int n_functions;
>> > + struct gcov_fn_info **functions;
>> > +};
>>
>> ... this structure's trailing fields actually getting used by the code
>> won't work well when changing compiler versions without cleaning
>> the tree. I think instead you need thin gcc_5.c and gcc_4_9.c
>> #define-ing their GCOV_COUNTERS and then #include-ing this
>> shared source file. Plus btw, I don't think gcc 5.0.x (the
>> development variant of 5.x) would use anything different from
>> 5.1.x or 5.2.x; in fact use of __GNUC_MINOR__ should not
>> normally be necessary anymore with gcc 5+.
>>
>
> I think you misread here: __GNUC_MINOR__ is the "x" part of 5.x.y, the
> "y" part is __GNUC_PATCHLEVEL__.
No, I didn't. From 5.x onwards the information previously carried in
__GNUC_PATCHLEVEL__ is now in __GNUC_MINOR__. And as much
as previously you would not normally need to look at the former,
with newer gcc you shouldn't need to look at the latter.
> I've broken down things into several files as well as provided
> corresponding Kconfig options:
>
> gcc_4_7_base.c: the body of what is now gcc_4_7.c, better name is
> welcome
Why don't you keep it gcc_4_7.c, with its counter definition being
conditional upon the symbol not already being defined?
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 58+ messages in thread* Re: [PATCH v3] gcov: add new interface and 3.4 and 4.7 format support
2016-10-12 15:42 ` Jan Beulich
@ 2016-10-12 17:07 ` Wei Liu
2016-10-13 8:29 ` Jan Beulich
0 siblings, 1 reply; 58+ messages in thread
From: Wei Liu @ 2016-10-12 17:07 UTC (permalink / raw)
To: Jan Beulich
Cc: Stefano Stabellini, Wei Liu, George Dunlap, AndrewCooper,
Ian Jackson, Tim Deegan, philippe.gabriel, Xen-devel
On Wed, Oct 12, 2016 at 09:42:17AM -0600, Jan Beulich wrote:
> >>> On 12.10.16 at 17:33, <wei.liu2@citrix.com> wrote:
> > On Wed, Oct 12, 2016 at 06:42:53AM -0600, Jan Beulich wrote:
> >> >>> On 11.10.16 at 12:31, <wei.liu2@citrix.com> wrote:
> >> > --- /dev/null
> >> > +++ b/xen/common/gcov/gcc_4_7.c
> >> > @@ -0,0 +1,205 @@
> >> > +/*
> >> > + * This code provides functions to handle gcc's profiling data format
> >> > + * introduced with gcc 4.7.
> >> > + *
> >> > + * This file is based heavily on gcc_3_4.c file.
> >> > + *
> >> > + * For a better understanding, refer to gcc source:
> >> > + * gcc/gcov-io.h
> >> > + * libgcc/libgcov.c
> >> > + *
> >> > + * Uses gcc-internal data definitions.
> >> > + *
> >> > + * Imported from Linux and modified for Xen by
> >> > + * Wei Liu <wei.liu2@citrix.com>
> >> > + */
> >> > +
> >> > +#include <xen/string.h>
> >> > +
> >> > +#include "gcov.h"
> >> > +
> >> > +#if GCC_VERSION < 40700
> >> > +#error "Wrong version of GCC used to compile gcov"
> >> > +#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
> >>
> >> I'm sorry for not having pointed this out on v2 (I had noticed it,
> >> but then didn't finish analyzing the situation), but I'm afraid this
> >> together with ...
> >>
> >> > +struct gcov_info {
> >> > + unsigned int version;
> >> > + struct gcov_info *next;
> >> > + unsigned int stamp;
> >> > + const char *filename;
> >> > + void (*merge[GCOV_COUNTERS])(gcov_type *, unsigned int);
> >> > + unsigned int n_functions;
> >> > + struct gcov_fn_info **functions;
> >> > +};
> >>
> >> ... this structure's trailing fields actually getting used by the code
> >> won't work well when changing compiler versions without cleaning
> >> the tree. I think instead you need thin gcc_5.c and gcc_4_9.c
> >> #define-ing their GCOV_COUNTERS and then #include-ing this
> >> shared source file. Plus btw, I don't think gcc 5.0.x (the
> >> development variant of 5.x) would use anything different from
> >> 5.1.x or 5.2.x; in fact use of __GNUC_MINOR__ should not
> >> normally be necessary anymore with gcc 5+.
> >>
> >
> > I think you misread here: __GNUC_MINOR__ is the "x" part of 5.x.y, the
> > "y" part is __GNUC_PATCHLEVEL__.
>
> No, I didn't. From 5.x onwards the information previously carried in
> __GNUC_PATCHLEVEL__ is now in __GNUC_MINOR__. And as much
> as previously you would not normally need to look at the former,
> with newer gcc you shouldn't need to look at the latter.
>
I can't find relevant information in GCC cpp manual.
Specifically, I look at 4.9.4 and 5.4.0 doc:
https://gcc.gnu.org/onlinedocs/gcc-4.9.4/cpp/Common-Predefined-Macros.html#Common-Predefined-Macros
https://gcc.gnu.org/onlinedocs/gcc-5.4.0/cpp/Common-Predefined-Macros.html#Common-Predefined-Macros
The sections about __GNUC_* macros are identical, their semantics stay
the same.
What did I miss?
> > I've broken down things into several files as well as provided
> > corresponding Kconfig options:
> >
> > gcc_4_7_base.c: the body of what is now gcc_4_7.c, better name is
> > welcome
>
> Why don't you keep it gcc_4_7.c, with its counter definition being
> conditional upon the symbol not already being defined?
>
Fixed as discussed on IRC.
Wei.
> Jan
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 58+ messages in thread* Re: [PATCH v3] gcov: add new interface and 3.4 and 4.7 format support
2016-10-12 17:07 ` Wei Liu
@ 2016-10-13 8:29 ` Jan Beulich
2016-10-13 8:49 ` Wei Liu
0 siblings, 1 reply; 58+ messages in thread
From: Jan Beulich @ 2016-10-13 8:29 UTC (permalink / raw)
To: Wei Liu
Cc: Stefano Stabellini, George Dunlap, AndrewCooper, Ian Jackson,
Tim Deegan, philippe.gabriel, Xen-devel
>>> On 12.10.16 at 19:07, <wei.liu2@citrix.com> wrote:
> On Wed, Oct 12, 2016 at 09:42:17AM -0600, Jan Beulich wrote:
>> >>> On 12.10.16 at 17:33, <wei.liu2@citrix.com> wrote:
>> > On Wed, Oct 12, 2016 at 06:42:53AM -0600, Jan Beulich wrote:
>> >> >>> On 11.10.16 at 12:31, <wei.liu2@citrix.com> wrote:
>> >> > --- /dev/null
>> >> > +++ b/xen/common/gcov/gcc_4_7.c
>> >> > @@ -0,0 +1,205 @@
>> >> > +/*
>> >> > + * This code provides functions to handle gcc's profiling data format
>> >> > + * introduced with gcc 4.7.
>> >> > + *
>> >> > + * This file is based heavily on gcc_3_4.c file.
>> >> > + *
>> >> > + * For a better understanding, refer to gcc source:
>> >> > + * gcc/gcov-io.h
>> >> > + * libgcc/libgcov.c
>> >> > + *
>> >> > + * Uses gcc-internal data definitions.
>> >> > + *
>> >> > + * Imported from Linux and modified for Xen by
>> >> > + * Wei Liu <wei.liu2@citrix.com>
>> >> > + */
>> >> > +
>> >> > +#include <xen/string.h>
>> >> > +
>> >> > +#include "gcov.h"
>> >> > +
>> >> > +#if GCC_VERSION < 40700
>> >> > +#error "Wrong version of GCC used to compile gcov"
>> >> > +#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
>> >>
>> >> I'm sorry for not having pointed this out on v2 (I had noticed it,
>> >> but then didn't finish analyzing the situation), but I'm afraid this
>> >> together with ...
>> >>
>> >> > +struct gcov_info {
>> >> > + unsigned int version;
>> >> > + struct gcov_info *next;
>> >> > + unsigned int stamp;
>> >> > + const char *filename;
>> >> > + void (*merge[GCOV_COUNTERS])(gcov_type *, unsigned int);
>> >> > + unsigned int n_functions;
>> >> > + struct gcov_fn_info **functions;
>> >> > +};
>> >>
>> >> ... this structure's trailing fields actually getting used by the code
>> >> won't work well when changing compiler versions without cleaning
>> >> the tree. I think instead you need thin gcc_5.c and gcc_4_9.c
>> >> #define-ing their GCOV_COUNTERS and then #include-ing this
>> >> shared source file. Plus btw, I don't think gcc 5.0.x (the
>> >> development variant of 5.x) would use anything different from
>> >> 5.1.x or 5.2.x; in fact use of __GNUC_MINOR__ should not
>> >> normally be necessary anymore with gcc 5+.
>> >>
>> >
>> > I think you misread here: __GNUC_MINOR__ is the "x" part of 5.x.y, the
>> > "y" part is __GNUC_PATCHLEVEL__.
>>
>> No, I didn't. From 5.x onwards the information previously carried in
>> __GNUC_PATCHLEVEL__ is now in __GNUC_MINOR__. And as much
>> as previously you would not normally need to look at the former,
>> with newer gcc you shouldn't need to look at the latter.
>>
>
> I can't find relevant information in GCC cpp manual.
>
> Specifically, I look at 4.9.4 and 5.4.0 doc:
>
> https://gcc.gnu.org/onlinedocs/gcc-4.9.4/cpp/Common-Predefined-Macros.html#Comm
> on-Predefined-Macros
> https://gcc.gnu.org/onlinedocs/gcc-5.4.0/cpp/Common-Predefined-Macros.html#Comm
> on-Predefined-Macros
>
> The sections about __GNUC_* macros are identical, their semantics stay
> the same.
>
> What did I miss?
Their change in how version numbers get used. I'm sure you've noticed
there never was a released 5.0.0 or 6.0.0, and that the stable updates
following 5.1.0 were 5.2.0, 5.3.0, etc.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 58+ messages in thread* Re: [PATCH v3] gcov: add new interface and 3.4 and 4.7 format support
2016-10-13 8:29 ` Jan Beulich
@ 2016-10-13 8:49 ` Wei Liu
2016-10-13 9:15 ` Jan Beulich
0 siblings, 1 reply; 58+ messages in thread
From: Wei Liu @ 2016-10-13 8:49 UTC (permalink / raw)
To: Jan Beulich
Cc: Stefano Stabellini, Wei Liu, George Dunlap, AndrewCooper,
Ian Jackson, Tim Deegan, philippe.gabriel, Xen-devel
On Thu, Oct 13, 2016 at 02:29:08AM -0600, Jan Beulich wrote:
[...]
> >> >> ... this structure's trailing fields actually getting used by the code
> >> >> won't work well when changing compiler versions without cleaning
> >> >> the tree. I think instead you need thin gcc_5.c and gcc_4_9.c
> >> >> #define-ing their GCOV_COUNTERS and then #include-ing this
> >> >> shared source file. Plus btw, I don't think gcc 5.0.x (the
> >> >> development variant of 5.x) would use anything different from
> >> >> 5.1.x or 5.2.x; in fact use of __GNUC_MINOR__ should not
> >> >> normally be necessary anymore with gcc 5+.
> >> >>
> >> >
> >> > I think you misread here: __GNUC_MINOR__ is the "x" part of 5.x.y, the
> >> > "y" part is __GNUC_PATCHLEVEL__.
> >>
> >> No, I didn't. From 5.x onwards the information previously carried in
> >> __GNUC_PATCHLEVEL__ is now in __GNUC_MINOR__. And as much
> >> as previously you would not normally need to look at the former,
> >> with newer gcc you shouldn't need to look at the latter.
> >>
> >
> > I can't find relevant information in GCC cpp manual.
> >
> > Specifically, I look at 4.9.4 and 5.4.0 doc:
> >
> > https://gcc.gnu.org/onlinedocs/gcc-4.9.4/cpp/Common-Predefined-Macros.html#Comm
> > on-Predefined-Macros
> > https://gcc.gnu.org/onlinedocs/gcc-5.4.0/cpp/Common-Predefined-Macros.html#Comm
> > on-Predefined-Macros
> >
> > The sections about __GNUC_* macros are identical, their semantics stay
> > the same.
> >
> > What did I miss?
>
> Their change in how version numbers get used. I'm sure you've noticed
> there never was a released 5.0.0 or 6.0.0, and that the stable updates
> following 5.1.0 were 5.2.0, 5.3.0, etc.
>
OK. I found the bits at https://gcc.gnu.org/develop.html. I see what you
meant previously.
It doesn't seem to be a problem to me to compare to 5.1 though -- that's
the first release of gcc 5, which should be what people use anyway.
If it is the complexity of the macro that concerns you, now it has been
changed to use GCC_VERSION macro in gcov.h, which is a lot simpler to
reason about. Are you happy with such arrangement?
If you feel strongly about this version comparison thing, I'm fine with
just comparing it to the major number, too.
Wei.
> Jan
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH v3] gcov: add new interface and 3.4 and 4.7 format support
2016-10-13 8:49 ` Wei Liu
@ 2016-10-13 9:15 ` Jan Beulich
2016-10-13 9:20 ` Wei Liu
0 siblings, 1 reply; 58+ messages in thread
From: Jan Beulich @ 2016-10-13 9:15 UTC (permalink / raw)
To: Wei Liu
Cc: Stefano Stabellini, George Dunlap, AndrewCooper, Ian Jackson,
Tim Deegan, philippe.gabriel, Xen-devel
>>> On 13.10.16 at 10:49, <wei.liu2@citrix.com> wrote:
> On Thu, Oct 13, 2016 at 02:29:08AM -0600, Jan Beulich wrote:
> [...]
>> >> >> ... this structure's trailing fields actually getting used by the code
>> >> >> won't work well when changing compiler versions without cleaning
>> >> >> the tree. I think instead you need thin gcc_5.c and gcc_4_9.c
>> >> >> #define-ing their GCOV_COUNTERS and then #include-ing this
>> >> >> shared source file. Plus btw, I don't think gcc 5.0.x (the
>> >> >> development variant of 5.x) would use anything different from
>> >> >> 5.1.x or 5.2.x; in fact use of __GNUC_MINOR__ should not
>> >> >> normally be necessary anymore with gcc 5+.
>> >> >>
>> >> >
>> >> > I think you misread here: __GNUC_MINOR__ is the "x" part of 5.x.y, the
>> >> > "y" part is __GNUC_PATCHLEVEL__.
>> >>
>> >> No, I didn't. From 5.x onwards the information previously carried in
>> >> __GNUC_PATCHLEVEL__ is now in __GNUC_MINOR__. And as much
>> >> as previously you would not normally need to look at the former,
>> >> with newer gcc you shouldn't need to look at the latter.
>> >>
>> >
>> > I can't find relevant information in GCC cpp manual.
>> >
>> > Specifically, I look at 4.9.4 and 5.4.0 doc:
>> >
>> >
> https://gcc.gnu.org/onlinedocs/gcc-4.9.4/cpp/Common-Predefined-Macros.html#Comm
>
>> > on-Predefined-Macros
>> >
> https://gcc.gnu.org/onlinedocs/gcc-5.4.0/cpp/Common-Predefined-Macros.html#Comm
>
>> > on-Predefined-Macros
>> >
>> > The sections about __GNUC_* macros are identical, their semantics stay
>> > the same.
>> >
>> > What did I miss?
>>
>> Their change in how version numbers get used. I'm sure you've noticed
>> there never was a released 5.0.0 or 6.0.0, and that the stable updates
>> following 5.1.0 were 5.2.0, 5.3.0, etc.
>>
>
> OK. I found the bits at https://gcc.gnu.org/develop.html. I see what you
> meant previously.
>
> It doesn't seem to be a problem to me to compare to 5.1 though -- that's
> the first release of gcc 5, which should be what people use anyway.
But your check should cover the introduction point of the feature,
which is 5.0.0 imo.
> If it is the complexity of the macro that concerns you, now it has been
> changed to use GCC_VERSION macro in gcov.h, which is a lot simpler to
> reason about. Are you happy with such arrangement?
If you mean this to be an adjustment newer than v3, then I think
I'd be fine with that, provided you cover the full range (as indicated
above), i.e. starting at 5.0.0.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH v3] gcov: add new interface and 3.4 and 4.7 format support
2016-10-13 9:15 ` Jan Beulich
@ 2016-10-13 9:20 ` Wei Liu
0 siblings, 0 replies; 58+ messages in thread
From: Wei Liu @ 2016-10-13 9:20 UTC (permalink / raw)
To: Jan Beulich
Cc: Stefano Stabellini, Wei Liu, George Dunlap, AndrewCooper,
Ian Jackson, Tim Deegan, philippe.gabriel, Xen-devel
On Thu, Oct 13, 2016 at 03:15:09AM -0600, Jan Beulich wrote:
> >>> On 13.10.16 at 10:49, <wei.liu2@citrix.com> wrote:
> > On Thu, Oct 13, 2016 at 02:29:08AM -0600, Jan Beulich wrote:
> > [...]
> >> >> >> ... this structure's trailing fields actually getting used by the code
> >> >> >> won't work well when changing compiler versions without cleaning
> >> >> >> the tree. I think instead you need thin gcc_5.c and gcc_4_9.c
> >> >> >> #define-ing their GCOV_COUNTERS and then #include-ing this
> >> >> >> shared source file. Plus btw, I don't think gcc 5.0.x (the
> >> >> >> development variant of 5.x) would use anything different from
> >> >> >> 5.1.x or 5.2.x; in fact use of __GNUC_MINOR__ should not
> >> >> >> normally be necessary anymore with gcc 5+.
> >> >> >>
> >> >> >
> >> >> > I think you misread here: __GNUC_MINOR__ is the "x" part of 5.x.y, the
> >> >> > "y" part is __GNUC_PATCHLEVEL__.
> >> >>
> >> >> No, I didn't. From 5.x onwards the information previously carried in
> >> >> __GNUC_PATCHLEVEL__ is now in __GNUC_MINOR__. And as much
> >> >> as previously you would not normally need to look at the former,
> >> >> with newer gcc you shouldn't need to look at the latter.
> >> >>
> >> >
> >> > I can't find relevant information in GCC cpp manual.
> >> >
> >> > Specifically, I look at 4.9.4 and 5.4.0 doc:
> >> >
> >> >
> > https://gcc.gnu.org/onlinedocs/gcc-4.9.4/cpp/Common-Predefined-Macros.html#Comm
> >
> >> > on-Predefined-Macros
> >> >
> > https://gcc.gnu.org/onlinedocs/gcc-5.4.0/cpp/Common-Predefined-Macros.html#Comm
> >
> >> > on-Predefined-Macros
> >> >
> >> > The sections about __GNUC_* macros are identical, their semantics stay
> >> > the same.
> >> >
> >> > What did I miss?
> >>
> >> Their change in how version numbers get used. I'm sure you've noticed
> >> there never was a released 5.0.0 or 6.0.0, and that the stable updates
> >> following 5.1.0 were 5.2.0, 5.3.0, etc.
> >>
> >
> > OK. I found the bits at https://gcc.gnu.org/develop.html. I see what you
> > meant previously.
> >
> > It doesn't seem to be a problem to me to compare to 5.1 though -- that's
> > the first release of gcc 5, which should be what people use anyway.
>
> But your check should cover the introduction point of the feature,
> which is 5.0.0 imo.
>
> > If it is the complexity of the macro that concerns you, now it has been
> > changed to use GCC_VERSION macro in gcov.h, which is a lot simpler to
> > reason about. Are you happy with such arrangement?
>
> If you mean this to be an adjustment newer than v3, then I think
> I'd be fine with that, provided you cover the full range (as indicated
> above), i.e. starting at 5.0.0.
>
OK. The check is now like:
#if GCC_VERSION < 50000
#error "Wrong version of GCC used to compile gcov"
#endif
GCC_VERSION is a convenient marco that you can find in this patch.
Then all reference to gcc 5.1 will be changed to gcc 5.
Wei.
> Jan
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 58+ messages in thread
* [PATCH v2 6/9] gcov: userspace tools to extract and split gcov data
2016-10-10 9:40 [PATCH v2 0/9] Rework gcov support in Xen Wei Liu
` (4 preceding siblings ...)
2016-10-10 9:40 ` [PATCH v2 5/9] gcov: add new interface and 3.4 and 4.7 format support Wei Liu
@ 2016-10-10 9:40 ` Wei Liu
2016-10-10 15:44 ` Ian Jackson
2016-10-10 9:40 ` [PATCH v2 7/9] Config.mk: expand cc-ver a bit Wei Liu
` (3 subsequent siblings)
9 siblings, 1 reply; 58+ messages in thread
From: Wei Liu @ 2016-10-10 9:40 UTC (permalink / raw)
To: Xen-devel; +Cc: Wei Liu, philippe.gabriel, Ian Jackson
Provide two tools: a small C program to extract data from hypervisor and
a python script to split data into multiple files.
The file xencov.c is salvaged and modified from the original xencov.c.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
v2: fix INSTALL_BIN typo
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
---
tools/misc/Makefile | 6 ++
tools/misc/xencov.c | 148 ++++++++++++++++++++++++++++++++++++++++++++++++
tools/misc/xencov_split | 100 ++++++++++++++++++++++++++++++++
3 files changed, 254 insertions(+)
create mode 100644 tools/misc/xencov.c
create mode 100755 tools/misc/xencov_split
diff --git a/tools/misc/Makefile b/tools/misc/Makefile
index 5ba6672..8152f7b 100644
--- a/tools/misc/Makefile
+++ b/tools/misc/Makefile
@@ -13,6 +13,7 @@ CFLAGS += $(CFLAGS_libxenstore)
INSTALL_BIN-$(CONFIG_X86) += xen-cpuid
INSTALL_BIN-$(CONFIG_X86) += xen-detect
INSTALL_BIN += xencons
+INSTALL_BIN += xencov_split
INSTALL_BIN += $(INSTALL_BIN-y)
# Everything to be installed in regular sbin/
@@ -24,6 +25,7 @@ INSTALL_SBIN-$(CONFIG_X86) += xen-lowmemd
INSTALL_SBIN-$(CONFIG_X86) += xen-mfndump
INSTALL_SBIN += xen-ringwatch
INSTALL_SBIN += xen-tmem-list-parse
+INSTALL_SBIN += xencov
INSTALL_SBIN += xenlockprof
INSTALL_SBIN += xenperf
INSTALL_SBIN += xenpm
@@ -41,6 +43,7 @@ TARGETS_ALL := $(INSTALL_BIN) $(INSTALL_SBIN) $(INSTALL_PRIVBIN)
TARGETS_COPY += xen-bugtool
TARGETS_COPY += xen-ringwatch
TARGETS_COPY += xencons
+TARGETS_COPY += xencov_split
TARGETS_COPY += xenpvnetboot
# Everything which needs to be built
@@ -102,4 +105,7 @@ xen-livepatch: xen-livepatch.o
xen-lowmemd: xen-lowmemd.o
$(CC) $(LDFLAGS) -o $@ $< $(LDLIBS_libxenevtchn) $(LDLIBS_libxenctrl) $(LDLIBS_libxenstore) $(APPEND_LDFLAGS)
+xencov: xencov.o
+ $(CC) $(LDFLAGS) -o $@ $< $(LDLIBS_libxenctrl) $(APPEND_LDFLAGS)
+
-include $(DEPS)
diff --git a/tools/misc/xencov.c b/tools/misc/xencov.c
new file mode 100644
index 0000000..d83bc66
--- /dev/null
+++ b/tools/misc/xencov.c
@@ -0,0 +1,148 @@
+/*
+ * xencov: extract test coverage information from Xen.
+ *
+ * Copyright (c) 2013, 2016, Citrix Systems R&D Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program; If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <err.h>
+#include <errno.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <xenctrl.h>
+
+static xc_interface *xch = NULL;
+
+int gcov_sysctl(int op, struct xen_sysctl *sysctl,
+ struct xc_hypercall_buffer *buf, uint32_t buf_size)
+{
+ DECLARE_HYPERCALL_BUFFER_ARGUMENT(buf);
+
+ memset(sysctl, 0, sizeof(*sysctl));
+ sysctl->cmd = XEN_SYSCTL_gcov_op;
+
+ sysctl->u.gcov_op.cmd = op;
+ sysctl->u.gcov_op.size = buf_size;
+ set_xen_guest_handle(sysctl->u.gcov_op.buffer, buf);
+
+ return xc_sysctl(xch, sysctl);
+}
+
+static void gcov_read(const char *fn)
+{
+ struct xen_sysctl sys;
+ uint32_t total_len;
+ DECLARE_HYPERCALL_BUFFER(uint8_t, p);
+ FILE *f;
+
+ if (gcov_sysctl(XEN_SYSCTL_GCOV_get_size, &sys, NULL, 0) < 0)
+ err(1, "getting total length");
+ total_len = sys.u.gcov_op.size;
+
+ /* Shouldn't exceed a few hundred kilobytes */
+ if (total_len > 8u * 1024u * 1024u)
+ errx(1, "gcov data too big %u bytes\n", total_len);
+
+ p = xc_hypercall_buffer_alloc(xch, p, total_len);
+ if (!p)
+ err(1, "allocating buffer");
+
+ memset(p, 0, total_len);
+ if (gcov_sysctl(XEN_SYSCTL_GCOV_read, &sys, HYPERCALL_BUFFER(p),
+ total_len) < 0)
+ err(1, "getting gcov data");
+
+ if (!strcmp(fn, "-"))
+ f = stdout;
+ else
+ f = fopen(fn, "w");
+
+ if (!f)
+ err(1, "opening output file");
+
+ if (fwrite(p, 1, total_len, f) != total_len)
+ err(1, "writing gcov data to file");
+
+ if (f != stdout)
+ fclose(f);
+
+ xc_hypercall_buffer_free(xch, p);
+}
+
+static void gcov_reset(void)
+{
+ struct xen_sysctl sys;
+
+ if (gcov_sysctl(XEN_SYSCTL_GCOV_reset, &sys, NULL, 0) < 0)
+ err(1, "resetting gcov information");
+}
+
+static void usage(int exit_code)
+{
+ FILE *out = exit_code ? stderr : stdout;
+
+ fprintf(out, "xencov {reset|read} [<filename>]\n"
+ "\treset reset information\n"
+ "\tread read information from xen to filename\n"
+ "\tfilename optional filename (default output)\n"
+ );
+ exit(exit_code);
+}
+
+int main(int argc, char **argv)
+{
+ int opt;
+
+ while ((opt = getopt(argc, argv, "h")) != -1) {
+ switch (opt) {
+ case 'h':
+ usage(0);
+ break;
+ default:
+ usage(1);
+ }
+ }
+
+ argv += optind;
+ argc -= optind;
+ if (argc <= 0)
+ usage(1);
+
+ xch = xc_interface_open(NULL, NULL, 0);
+ if (!xch)
+ err(1, "opening xc interface");
+
+ if (strcmp(argv[0], "reset") == 0)
+ gcov_reset();
+ else if ( strcmp(argv[0], "read") == 0 )
+ gcov_read(argc > 1 ? argv[1] : "-");
+ else
+ usage(1);
+
+ xc_interface_close(xch);
+
+ return 0;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/tools/misc/xencov_split b/tools/misc/xencov_split
new file mode 100755
index 0000000..2c06f49
--- /dev/null
+++ b/tools/misc/xencov_split
@@ -0,0 +1,100 @@
+#!/usr/bin/python
+
+import sys, os, os.path as path, struct, errno
+from optparse import OptionParser
+
+def xencov_split(opts):
+ """Split input into multiple gcda files"""
+
+ # Check native byte order and explicitly specify it. The "native"
+ # byte order in struct module takes into account padding while the
+ # data is always packed.
+ if sys.byteorder == 'little':
+ bo_prefix = '<'
+ else:
+ bo_prefix = '>'
+
+ input_file = opts.args[0]
+
+ f = open(input_file)
+
+ # Magic number
+ s = f.read(4)
+ magic, = struct.unpack(bo_prefix + "I", s)
+ # See public/sysctl.h for magic number -- "XCOV"
+ if magic != 0x58434f56:
+ raise Exception("Invalid magic number")
+
+ # The rest is zero or more records
+ content = f.read()
+
+ f.close()
+
+ while content:
+ off = content.find('\x00')
+ fmt = bo_prefix + str(off) + 's'
+ fn, = struct.unpack_from(fmt, content)
+ content = content[off+1:]
+
+ fmt = bo_prefix + 'I'
+ sz, = struct.unpack_from(fmt, content)
+ content = content[struct.calcsize(fmt):]
+
+ fmt = bo_prefix + str(sz) + 's'
+ payload, = struct.unpack_from(fmt, content)
+ content = content[sz:]
+
+ # Create and store files
+ if opts.output_dir == '.':
+ opts.output_dir = os.getcwd()
+
+ dir = opts.output_dir + path.dirname(fn)
+ try:
+ os.makedirs(dir)
+ except OSError, e:
+ if e.errno == errno.EEXIST and os.path.isdir(dir):
+ pass
+ else:
+ raise
+
+ full_path = dir + '/' + path.basename(fn)
+ f = open(full_path, "w")
+ f.write(payload)
+ f.close()
+
+def main():
+ """ Main entrypoint """
+
+ # Change stdout to be line-buffered.
+ sys.stdout = os.fdopen(sys.stdout.fileno(), 'w', 1)
+
+ # Normalise $CWD to the directory this script is in
+ # os.chdir(path.dirname(path.abspath(sys.argv[0])))
+
+ parser = OptionParser(
+ usage = "%prog [OPTIONS] <INPUT>",
+ description = "Utility to split xencov data file",
+ )
+
+ parser.add_option("--output-dir", action = "store",
+ dest = "output_dir", default = ".",
+ type = "string",
+ help = ('Specify the directory to place output files, '
+ 'defaults to current directory'),
+ )
+
+ opts, args = parser.parse_args()
+ opts.args = args
+
+ xencov_split(opts)
+
+
+if __name__ == "__main__":
+ try:
+ sys.exit(main())
+ except Exception, e:
+ print >>sys.stderr, "Error:", e
+ sys.exit(1)
+ except KeyboardInterrupt:
+ sys.exit(1)
+
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 58+ messages in thread* [PATCH v2 7/9] Config.mk: expand cc-ver a bit
2016-10-10 9:40 [PATCH v2 0/9] Rework gcov support in Xen Wei Liu
` (5 preceding siblings ...)
2016-10-10 9:40 ` [PATCH v2 6/9] gcov: userspace tools to extract and split gcov data Wei Liu
@ 2016-10-10 9:40 ` Wei Liu
2016-10-10 11:57 ` Jan Beulich
2016-10-10 9:40 ` [PATCH v2 8/9] Config.mk: introduce cc-ifversion Wei Liu
` (2 subsequent siblings)
9 siblings, 1 reply; 58+ messages in thread
From: Wei Liu @ 2016-10-10 9:40 UTC (permalink / raw)
To: Xen-devel
Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper,
Ian Jackson, Tim Deegan, philippe.gabriel, Jan Beulich
... so that we can do other comparisons as well.
No functional change.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
v2: move dash into macro.
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>
---
Config.mk | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/Config.mk b/Config.mk
index 3c3ff68..af50137 100644
--- a/Config.mk
+++ b/Config.mk
@@ -112,17 +112,17 @@ endef
cc-options-add = $(foreach o,$(3),$(call cc-option-add,$(1),$(2),$(o)))
-# cc-ver: Check compiler is at least specified version. Return boolean 'y'/'n'.
-# Usage: ifeq ($(call cc-ver,$(CC),0x030400),y)
+# cc-ver: Check compiler against the version requirement. Return boolean 'y'/'n'.
+# Usage: ifeq ($(call cc-ver,$(CC),ge,0x030400),y)
cc-ver = $(shell if [ $$((`$(1) -dumpversion | awk -F. \
- '{ printf "0x%02x%02x%02x", $$1, $$2, $$3}'`)) -ge $$(($(2))) ]; \
+ '{ printf "0x%02x%02x%02x", $$1, $$2, $$3}'`)) -$(2) $$(($(3))) ]; \
then echo y; else echo n; fi ;)
# cc-ver-check: Check compiler is at least specified version, else fail.
# Usage: $(call cc-ver-check,CC,0x030400,"Require at least gcc-3.4")
cc-ver-check = $(eval $(call cc-ver-check-closure,$(1),$(2),$(3)))
define cc-ver-check-closure
- ifeq ($$(call cc-ver,$$($(1)),$(2)),n)
+ ifeq ($$(call cc-ver,$$($(1)),ge,$(2)),n)
override $(1) = echo "*** FATAL BUILD ERROR: "$(3) >&2; exit 1;
cc-option := n
endif
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 58+ messages in thread* [PATCH v2 8/9] Config.mk: introduce cc-ifversion
2016-10-10 9:40 [PATCH v2 0/9] Rework gcov support in Xen Wei Liu
` (6 preceding siblings ...)
2016-10-10 9:40 ` [PATCH v2 7/9] Config.mk: expand cc-ver a bit Wei Liu
@ 2016-10-10 9:40 ` Wei Liu
2016-10-10 12:00 ` Jan Beulich
2016-10-10 9:40 ` [PATCH v2 9/9] gcov: provide the capability to select gcov format automatically Wei Liu
2016-10-10 15:47 ` [PATCH v2 0/9] Rework gcov support in Xen Ian Jackson
9 siblings, 1 reply; 58+ messages in thread
From: Wei Liu @ 2016-10-10 9:40 UTC (permalink / raw)
To: Xen-devel
Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper,
Ian Jackson, Tim Deegan, philippe.gabriel, Jan Beulich
It returns different string depending on compiler version.
No user yet.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>
---
Config.mk | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/Config.mk b/Config.mk
index af50137..844be0a 100644
--- a/Config.mk
+++ b/Config.mk
@@ -128,6 +128,11 @@ define cc-ver-check-closure
endif
endef
+# cc-ifversion: Check compiler version and take branch accordingly
+# Usage $(call cc-ifversion,lt,0x040700,string_if_y,string_if_n)
+cc-ifversion = $(shell [ $(call cc-ver,$(CC),$(1),$(2)) = "y" ] \
+ && echo $(3) || echo $(4))
+
# Require GCC v4.1+
check-$(gcc) = $(call cc-ver-check,CC,0x040100,"Xen requires at least gcc-4.1")
$(eval $(check-y))
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 58+ messages in thread* Re: [PATCH v2 8/9] Config.mk: introduce cc-ifversion
2016-10-10 9:40 ` [PATCH v2 8/9] Config.mk: introduce cc-ifversion Wei Liu
@ 2016-10-10 12:00 ` Jan Beulich
2016-10-10 13:18 ` Wei Liu
0 siblings, 1 reply; 58+ messages in thread
From: Jan Beulich @ 2016-10-10 12:00 UTC (permalink / raw)
To: Wei Liu
Cc: Stefano Stabellini, George Dunlap, AndrewCooper, Ian Jackson,
Tim Deegan, philippe.gabriel, Xen-devel
>>> On 10.10.16 at 11:40, <wei.liu2@citrix.com> wrote:
> It returns different string depending on compiler version.
>
> No user yet.
>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
albeit I wonder whether ...
> --- a/Config.mk
> +++ b/Config.mk
> @@ -128,6 +128,11 @@ define cc-ver-check-closure
> endif
> endef
>
> +# cc-ifversion: Check compiler version and take branch accordingly
> +# Usage $(call cc-ifversion,lt,0x040700,string_if_y,string_if_n)
> +cc-ifversion = $(shell [ $(call cc-ver,$(CC),$(1),$(2)) = "y" ] \
> + && echo $(3) || echo $(4))
... if-cc-version wouldn't be the better name.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH v2 8/9] Config.mk: introduce cc-ifversion
2016-10-10 12:00 ` Jan Beulich
@ 2016-10-10 13:18 ` Wei Liu
2016-10-10 13:22 ` Jan Beulich
0 siblings, 1 reply; 58+ messages in thread
From: Wei Liu @ 2016-10-10 13:18 UTC (permalink / raw)
To: Jan Beulich
Cc: Stefano Stabellini, Wei Liu, George Dunlap, AndrewCooper,
Ian Jackson, Tim Deegan, philippe.gabriel, Xen-devel
On Mon, Oct 10, 2016 at 06:00:03AM -0600, Jan Beulich wrote:
> >>> On 10.10.16 at 11:40, <wei.liu2@citrix.com> wrote:
> > It returns different string depending on compiler version.
> >
> > No user yet.
> >
> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
>
> Acked-by: Jan Beulich <jbeulich@suse.com>
> albeit I wonder whether ...
>
> > --- a/Config.mk
> > +++ b/Config.mk
> > @@ -128,6 +128,11 @@ define cc-ver-check-closure
> > endif
> > endef
> >
> > +# cc-ifversion: Check compiler version and take branch accordingly
> > +# Usage $(call cc-ifversion,lt,0x040700,string_if_y,string_if_n)
> > +cc-ifversion = $(shell [ $(call cc-ver,$(CC),$(1),$(2)) = "y" ] \
> > + && echo $(3) || echo $(4))
>
> ... if-cc-version wouldn't be the better name.
>
Linux uses cc-ifversion. That's the naming scheme I followed.
Wei.
> Jan
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH v2 8/9] Config.mk: introduce cc-ifversion
2016-10-10 13:18 ` Wei Liu
@ 2016-10-10 13:22 ` Jan Beulich
2016-10-10 13:24 ` Wei Liu
0 siblings, 1 reply; 58+ messages in thread
From: Jan Beulich @ 2016-10-10 13:22 UTC (permalink / raw)
To: Wei Liu
Cc: Stefano Stabellini, George Dunlap, AndrewCooper, Ian Jackson,
Tim Deegan, philippe.gabriel, Xen-devel
>>> On 10.10.16 at 15:18, <wei.liu2@citrix.com> wrote:
> On Mon, Oct 10, 2016 at 06:00:03AM -0600, Jan Beulich wrote:
>> >>> On 10.10.16 at 11:40, <wei.liu2@citrix.com> wrote:
>> > It returns different string depending on compiler version.
>> >
>> > No user yet.
>> >
>> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
>>
>> Acked-by: Jan Beulich <jbeulich@suse.com>
>> albeit I wonder whether ...
>>
>> > --- a/Config.mk
>> > +++ b/Config.mk
>> > @@ -128,6 +128,11 @@ define cc-ver-check-closure
>> > endif
>> > endef
>> >
>> > +# cc-ifversion: Check compiler version and take branch accordingly
>> > +# Usage $(call cc-ifversion,lt,0x040700,string_if_y,string_if_n)
>> > +cc-ifversion = $(shell [ $(call cc-ver,$(CC),$(1),$(2)) = "y" ] \
>> > + && echo $(3) || echo $(4))
>>
>> ... if-cc-version wouldn't be the better name.
>
> Linux uses cc-ifversion. That's the naming scheme I followed.
Oh, I didn't realize you've taken it from elsewhere.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 58+ messages in thread
* Re: [PATCH v2 8/9] Config.mk: introduce cc-ifversion
2016-10-10 13:22 ` Jan Beulich
@ 2016-10-10 13:24 ` Wei Liu
0 siblings, 0 replies; 58+ messages in thread
From: Wei Liu @ 2016-10-10 13:24 UTC (permalink / raw)
To: Jan Beulich
Cc: Stefano Stabellini, Wei Liu, George Dunlap, AndrewCooper,
Ian Jackson, Tim Deegan, philippe.gabriel, Xen-devel
On Mon, Oct 10, 2016 at 07:22:57AM -0600, Jan Beulich wrote:
> >>> On 10.10.16 at 15:18, <wei.liu2@citrix.com> wrote:
> > On Mon, Oct 10, 2016 at 06:00:03AM -0600, Jan Beulich wrote:
> >> >>> On 10.10.16 at 11:40, <wei.liu2@citrix.com> wrote:
> >> > It returns different string depending on compiler version.
> >> >
> >> > No user yet.
> >> >
> >> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> >>
> >> Acked-by: Jan Beulich <jbeulich@suse.com>
> >> albeit I wonder whether ...
> >>
> >> > --- a/Config.mk
> >> > +++ b/Config.mk
> >> > @@ -128,6 +128,11 @@ define cc-ver-check-closure
> >> > endif
> >> > endef
> >> >
> >> > +# cc-ifversion: Check compiler version and take branch accordingly
> >> > +# Usage $(call cc-ifversion,lt,0x040700,string_if_y,string_if_n)
> >> > +cc-ifversion = $(shell [ $(call cc-ver,$(CC),$(1),$(2)) = "y" ] \
> >> > + && echo $(3) || echo $(4))
> >>
> >> ... if-cc-version wouldn't be the better name.
> >
> > Linux uses cc-ifversion. That's the naming scheme I followed.
>
> Oh, I didn't realize you've taken it from elsewhere.
>
The name, yes. Implementation, no.
I thought we always tried to stay close to Linux naming though...
Wei.
> Jan
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 58+ messages in thread
* [PATCH v2 9/9] gcov: provide the capability to select gcov format automatically
2016-10-10 9:40 [PATCH v2 0/9] Rework gcov support in Xen Wei Liu
` (7 preceding siblings ...)
2016-10-10 9:40 ` [PATCH v2 8/9] Config.mk: introduce cc-ifversion Wei Liu
@ 2016-10-10 9:40 ` Wei Liu
2016-10-10 12:00 ` Jan Beulich
2016-10-10 15:47 ` [PATCH v2 0/9] Rework gcov support in Xen Ian Jackson
9 siblings, 1 reply; 58+ messages in thread
From: Wei Liu @ 2016-10-10 9:40 UTC (permalink / raw)
To: Xen-devel
Cc: Stefano Stabellini, Wei Liu, George Dunlap, Andrew Cooper,
Ian Jackson, Tim Deegan, philippe.gabriel, Jan Beulich
And make it the default in Kconfig.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
v2: space -> tab and indent help text
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>
---
xen/Kconfig.debug | 9 ++++++++-
xen/common/gcov/Makefile | 2 ++
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/xen/Kconfig.debug b/xen/Kconfig.debug
index c65e543..2ffee02 100644
--- a/xen/Kconfig.debug
+++ b/xen/Kconfig.debug
@@ -38,10 +38,17 @@ config GCOV
choice
prompt "Specify Gcov format"
depends on GCOV
- default GCOV_FORMAT_4_7
+ default GCOV_FORMAT_AUTODETECT
---help---
The gcov format is determined by gcc version.
+ If unsure, choose "Autodetect".
+
+config GCOV_FORMAT_AUTODETECT
+ bool "Autodetect"
+ ---help---
+ Automatically select gcov format based on GCC version.
+
config GCOV_FORMAT_4_7
bool "GCC 4.7 format"
---help---
diff --git a/xen/common/gcov/Makefile b/xen/common/gcov/Makefile
index 03ac1e5..85f38e0 100644
--- a/xen/common/gcov/Makefile
+++ b/xen/common/gcov/Makefile
@@ -1,3 +1,5 @@
obj-y += gcov_base.o gcov.o
obj-$(CONFIG_GCOV_FORMAT_3_4) += gcc_3_4.o
obj-$(CONFIG_GCOV_FORMAT_4_7) += gcc_4_7.o
+obj-$(CONFIG_GCOV_FORMAT_AUTODETECT) += $(call cc-ifversion,ge,0x040700, \
+ gcc_4_7.o, gcc_3_4.o)
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 58+ messages in thread* Re: [PATCH v2 0/9] Rework gcov support in Xen
2016-10-10 9:40 [PATCH v2 0/9] Rework gcov support in Xen Wei Liu
` (8 preceding siblings ...)
2016-10-10 9:40 ` [PATCH v2 9/9] gcov: provide the capability to select gcov format automatically Wei Liu
@ 2016-10-10 15:47 ` Ian Jackson
2016-10-10 15:58 ` Jan Beulich
9 siblings, 1 reply; 58+ messages in thread
From: Ian Jackson @ 2016-10-10 15:47 UTC (permalink / raw)
To: Wei Liu
Cc: Stefano Stabellini, George Dunlap, Andrew Cooper, Tim Deegan,
philippe.gabriel, Jan Beulich, Xen-devel
Wei Liu writes ("[PATCH v2 0/9] Rework gcov support in Xen"):
> Since the hypervisor interface is sysctl, we have the liberty to not
> care about backward compatibility. This series completely rewrites the
> gcov support inside Xen.
I have looked at these patches. One of them seemed in my bailiwick -
the libxc tools patch, which I have acked.
If you feel my ack is needed for any of the build system parts, please
feel free to add it. I've looked at those and they don't seem
controversial to me.
Can someone hypervisor-side confirm that this functionality is really
disabled when the Kconfig is off, and so we don't need to worry about
its security status ?
Thanks,
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 58+ messages in thread* Re: [PATCH v2 0/9] Rework gcov support in Xen
2016-10-10 15:47 ` [PATCH v2 0/9] Rework gcov support in Xen Ian Jackson
@ 2016-10-10 15:58 ` Jan Beulich
0 siblings, 0 replies; 58+ messages in thread
From: Jan Beulich @ 2016-10-10 15:58 UTC (permalink / raw)
To: Wei Liu, Ian Jackson
Cc: Stefano Stabellini, George Dunlap, Andrew Cooper, Tim Deegan,
philippe.gabriel, Xen-devel
>>> On 10.10.16 at 17:47, <ian.jackson@eu.citrix.com> wrote:
> Wei Liu writes ("[PATCH v2 0/9] Rework gcov support in Xen"):
>> Since the hypervisor interface is sysctl, we have the liberty to not
>> care about backward compatibility. This series completely rewrites the
>> gcov support inside Xen.
>
> I have looked at these patches. One of them seemed in my bailiwick -
> the libxc tools patch, which I have acked.
>
> If you feel my ack is needed for any of the build system parts, please
> feel free to add it. I've looked at those and they don't seem
> controversial to me.
>
> Can someone hypervisor-side confirm that this functionality is really
> disabled when the Kconfig is off, and so we don't need to worry about
> its security status ?
Well, I think I've done so by acking most of the patches. But in
any event this
subdir-$(CONFIG_GCOV) += gcov
in xen/common/Makefile ensures none of the code in that directory
would get built.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 58+ messages in thread