[PATCH 0/5] gcov: more cleanup

[PATCH 0/5] gcov: more cleanup

[Wei Liu]

This series further cleans up existing gcov code. It should be now clear that
Xen's gcov is using gcc 3.4 format.

I have tried to integrate gcc 4.7 format. The amount of work is moderate in
terms of coding effort, but I now believe it is a mistake to have invented xen
specific format which is in fact tied to gcc 3.4. I can't map gcc 4.7 format to
Xen's own format, so I stop here.

Wei.

Wei Liu (5):
  xen: add tainted state and show warning is gcov is enabled
  gcov: collect more sections to constructor list
  xen: replace TEST_COVERAGE with CONFIG_GCOV
  gcov: add option to determine gcov format
  gcov: split out gcc version specific stuff

 xen/Kconfig.debug          | 13 +++++++
 xen/Rules.mk               |  2 +-
 xen/arch/arm/xen.lds.S     |  2 ++
 xen/arch/x86/xen.lds.S     |  2 ++
 xen/common/gcov/gcov.c     | 69 ++++++++++++++++++++++++-------------
 xen/common/gcov/gcov_3_4.h | 84 ++++++++++++++++++++++++++++++++++++++++++++++
 xen/common/kernel.c        |  6 ++--
 xen/common/sysctl.c        |  2 +-
 xen/drivers/char/console.c |  9 +++++
 xen/include/xen/gcov.h     | 82 +-------------------------------------------
 xen/include/xen/lib.h      |  1 +
 11 files changed, 164 insertions(+), 108 deletions(-)
 create mode 100644 xen/common/gcov/gcov_3_4.h

-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH 1/5] xen: add tainted state and show warning is gcov is enabled

[Wei Liu]

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
The location of warning_add() is a bit arbitrary because gcov doesn't
have an initialisation routine.

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/common/gcov/gcov.c     | 5 +++++
 xen/common/kernel.c        | 6 ++++--
 xen/drivers/char/console.c | 9 +++++++++
 xen/include/xen/lib.h      | 1 +
 4 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/xen/common/gcov/gcov.c b/xen/common/gcov/gcov.c
index b5717b9..6d18729 100644
--- a/xen/common/gcov/gcov.c
+++ b/xen/common/gcov/gcov.c
@@ -23,6 +23,11 @@
 #include <public/xen.h>
 #include <public/gcov.h>
 
+const char __initconst warning_gcov[] =
+    "WARNING: GCOV SUPPORT IS ENABLED\n"
+    "This option is *ONLY* intended to aid testing of Xen.\n"
+    "Please *DO NOT* use this in production.\n";
+
 static struct gcov_info *info_list;
 
 /*
diff --git a/xen/common/kernel.c b/xen/common/kernel.c
index 2d3db64..324cc24 100644
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -173,6 +173,7 @@ unsigned int tainted;
  *
  *  'C' - Console output is synchronous.
  *  'E' - An error (e.g. a machine check exceptions) has been injected.
+ *  'G' - GCov support is enabled.
  *  'H' - HVM forced emulation prefix is permitted.
  *  'M' - Machine had a machine check experience.
  *
@@ -182,11 +183,12 @@ char *print_tainted(char *str)
 {
     if ( tainted )
     {
-        snprintf(str, TAINT_STRING_MAX_LEN, "Tainted: %c%c%c%c",
+        snprintf(str, TAINT_STRING_MAX_LEN, "Tainted: %c%c%c%c%c",
                  tainted & TAINT_MACHINE_CHECK ? 'M' : ' ',
                  tainted & TAINT_SYNC_CONSOLE ? 'C' : ' ',
                  tainted & TAINT_ERROR_INJECT ? 'E' : ' ',
-                 tainted & TAINT_HVM_FEP ? 'H' : ' ');
+                 tainted & TAINT_HVM_FEP ? 'H' : ' ',
+                 tainted & TAINT_GCOV ? 'G' : ' ');
     }
     else
     {
diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index 650035d..77604f9 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -792,6 +792,10 @@ void __init console_init_postirq(void)
     console_init_ring();
 }
 
+#ifdef CONFIG_GCOV
+extern char warning_gcov[];
+#endif
+
 void __init console_endboot(void)
 {
     printk("Std. Loglevel: %s", loglvl_str(xenlog_lower_thresh));
@@ -802,6 +806,11 @@ void __init console_endboot(void)
         printk(" (Rate-limited: %s)", loglvl_str(xenlog_guest_upper_thresh));
     printk("\n");
 
+#ifdef CONFIG_GCOV
+    warning_add(warning_gcov);
+    add_taint(TAINT_GCOV);
+#endif
+
     warning_print();
 
     video_endboot();
diff --git a/xen/include/xen/lib.h b/xen/include/xen/lib.h
index e518adc..7bcc910 100644
--- a/xen/include/xen/lib.h
+++ b/xen/include/xen/lib.h
@@ -143,6 +143,7 @@ uint64_t muldiv64(uint64_t a, uint32_t b, uint32_t c);
 #define TAINT_MACHINE_CHECK             (1u << 1)
 #define TAINT_ERROR_INJECT              (1u << 2)
 #define TAINT_HVM_FEP                   (1u << 3)
+#define TAINT_GCOV                      (1u << 4)
 extern unsigned int tainted;
 #define TAINT_STRING_MAX_LEN            20
 extern char *print_tainted(char *str);
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH 2/5] gcov: collect more sections to constructor list

[Wei Liu]

The version of gcc (4.9.2) I use put constructors into .init_array*
section(s). Collect those sections into constructor list as well.

Modify both arm and x86 scripts to keep them in sync.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
---
 xen/arch/arm/xen.lds.S | 2 ++
 xen/arch/x86/xen.lds.S | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S
index b24e93b..ffff13a 100644
--- a/xen/arch/arm/xen.lds.S
+++ b/xen/arch/arm/xen.lds.S
@@ -166,6 +166,8 @@ SECTIONS
 
        . = ALIGN(8);
        __ctors_start = .;
+       *(.ctors)
+       *(SORT(.init_array.*))
        *(.init_array)
        __ctors_end = .;
   } :text
diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
index 67cfda1..8957efd 100644
--- a/xen/arch/x86/xen.lds.S
+++ b/xen/arch/x86/xen.lds.S
@@ -205,6 +205,8 @@ SECTIONS
        . = ALIGN(8);
        __ctors_start = .;
        *(.ctors)
+       *(SORT(.init_array.*))
+       *(.init_array)
        __ctors_end = .;
   } :text
 
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH 3/5] xen: replace TEST_COVERAGE with CONFIG_GCOV

[Wei Liu]

The sole purpose of TEST_COVERAGE macro is to guard the availability of
gcov sysctl. Now we have a proper CONFIG_GCOV, use it.

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/Rules.mk           | 2 +-
 xen/common/sysctl.c    | 2 +-
 xen/include/xen/gcov.h | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/xen/Rules.mk b/xen/Rules.mk
index 696aaa8..a9fda71 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -116,7 +116,7 @@ subdir-all := $(subdir-y) $(subdir-n)
 $(filter %.init.o,$(obj-y) $(obj-bin-y) $(extra-y)): CFLAGS += -DINIT_SECTIONS_ONLY
 
 ifeq ($(CONFIG_GCOV),y)
-$(filter-out %.init.o $(nogcov-y),$(obj-y) $(obj-bin-y) $(extra-y)): CFLAGS += -fprofile-arcs -ftest-coverage -DTEST_COVERAGE
+$(filter-out %.init.o $(nogcov-y),$(obj-y) $(obj-bin-y) $(extra-y)): CFLAGS += -fprofile-arcs -ftest-coverage
 endif
 
 ifeq ($(lto),y)
diff --git a/xen/common/sysctl.c b/xen/common/sysctl.c
index 55f2077..8aea6ef 100644
--- a/xen/common/sysctl.c
+++ b/xen/common/sysctl.c
@@ -396,7 +396,7 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
     }
     break;
 
-#ifdef TEST_COVERAGE
+#ifdef CONFIG_GCOV
     case XEN_SYSCTL_coverage_op:
         ret = sysctl_coverage_op(&op->u.coverage_op);
         break;
diff --git a/xen/include/xen/gcov.h b/xen/include/xen/gcov.h
index 27c5c37..a7d4a35 100644
--- a/xen/include/xen/gcov.h
+++ b/xen/include/xen/gcov.h
@@ -86,7 +86,7 @@ struct gcov_info
 /**
  * Sysctl operations for coverage
  */
-#ifdef TEST_COVERAGE
+#ifdef CONFIG_GCOV
 int sysctl_coverage_op(xen_sysctl_coverage_op_t *op);
 #endif
 
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH 4/5] gcov: add option to determine gcov format

[Wei Liu]

Currently only gcc 3.4 format is supported.

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/Kconfig.debug | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/xen/Kconfig.debug b/xen/Kconfig.debug
index 06afd80..2366b06 100644
--- a/xen/Kconfig.debug
+++ b/xen/Kconfig.debug
@@ -33,6 +33,19 @@ config GCOV
        ---help---
          Enable gcov (a test coverage program in GCC) support.
 
+choice
+	prompt "Specify Gcov format"
+	depends on GCOV
+	---help---
+	The gcov format is determined by gcc version.
+
+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---
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH 5/5] gcov: split out gcc version specific stuff

[Wei Liu]

Gcov record format is tied to specific gcc versions. The current code in
tree conforms to gcc 3.4 format.

Move structure definitions to a specific file and factor out some
version specific functions.

No functional change.

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/common/gcov/gcov.c     | 64 ++++++++++++++++++++++-------------
 xen/common/gcov/gcov_3_4.h | 84 ++++++++++++++++++++++++++++++++++++++++++++++
 xen/include/xen/gcov.h     | 80 -------------------------------------------
 3 files changed, 125 insertions(+), 103 deletions(-)
 create mode 100644 xen/common/gcov/gcov_3_4.h

diff --git a/xen/common/gcov/gcov.c b/xen/common/gcov/gcov.c
index 6d18729..2c2f05a 100644
--- a/xen/common/gcov/gcov.c
+++ b/xen/common/gcov/gcov.c
@@ -22,6 +22,11 @@
 #include <xen/guest_access.h>
 #include <public/xen.h>
 #include <public/gcov.h>
+#if defined(CONFIG_GCOV_FORMAT_3_4)
+#include "gcov_3_4.h"
+#else
+# error Gcov format not supported
+#endif
 
 const char __initconst warning_gcov[] =
     "WARNING: GCOV SUPPORT IS ENABLED\n"
@@ -69,10 +74,13 @@ 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)
+#if defined(CONFIG_GCOV_FORMAT_3_4)
+static inline int counter_active(const struct gcov_info *info,
+                                 unsigned int type)
 {
     return (1 << type) & info->ctr_mask;
 }
+#endif
 
 typedef struct write_iter_t
 {
@@ -122,6 +130,37 @@ static inline void align_iter(write_iter_t *iter)
         (iter->write_offset + sizeof(uint64_t) - 1) & -sizeof(uint64_t);
 }
 
+#if defined(CONFIG_GCOV_FORMAT_3_4)
+static int dump(write_iter_t *iter, const struct gcov_info *info)
+{
+    const struct gcov_ctr_info *ctr;
+    size_t size_fn = sizeof(struct gcov_fn_info);
+    int type;
+    int ret;
+
+    /* 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));
+
+    return 0;
+}
+#endif
+
 static int write_gcov(write_iter_t *iter)
 {
     struct gcov_info *info;
@@ -133,34 +172,13 @@ static int write_gcov(write_iter_t *iter)
     /* 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));
+        chk(dump(iter, info));
     }
 
     /* stop tag */
diff --git a/xen/common/gcov/gcov_3_4.h b/xen/common/gcov/gcov_3_4.h
new file mode 100644
index 0000000..e10c568
--- /dev/null
+++ b/xen/common/gcov/gcov_3_4.h
@@ -0,0 +1,84 @@
+/*
+ *  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_3_4_H__
+#define __XEN_GCOV_3_4_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];
+};
+
+
+#endif /* __XEN_GCOV_3_4_H__ */
diff --git a/xen/include/xen/gcov.h b/xen/include/xen/gcov.h
index a7d4a35..f6a1ac7 100644
--- a/xen/include/xen/gcov.h
+++ b/xen/include/xen/gcov.h
@@ -1,88 +1,8 @@
-/*
- *  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
  */
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 1/5] xen: add tainted state and show warning is gcov is enabled

[Jan Beulich]

>>> On 02.09.16 at 13:47, <wei.liu2@citrix.com> wrote:

Since this is a config option - why bother issuing a warning and
tainting the hypervisor?

> --- a/xen/common/gcov/gcov.c
> +++ b/xen/common/gcov/gcov.c
> @@ -23,6 +23,11 @@
>  #include <public/xen.h>
>  #include <public/gcov.h>
>  
> +const char __initconst warning_gcov[] =
> +    "WARNING: GCOV SUPPORT IS ENABLED\n"
> +    "This option is *ONLY* intended to aid testing of Xen.\n"
> +    "Please *DO NOT* use this in production.\n";

Note the type (const) difference between this and ...

> --- a/xen/drivers/char/console.c
> +++ b/xen/drivers/char/console.c
> @@ -792,6 +792,10 @@ void __init console_init_postirq(void)
>      console_init_ring();
>  }
>  
> +#ifdef CONFIG_GCOV
> +extern char warning_gcov[];
> +#endif

... this. That's one of the reasons declarations of stuff defined in
C sources should be put in a header, which then gets included by
both producer and consumer(s). But ...

> @@ -802,6 +806,11 @@ void __init console_endboot(void)
>          printk(" (Rate-limited: %s)", loglvl_str(xenlog_guest_upper_thresh));
>      printk("\n");
>  
> +#ifdef CONFIG_GCOV
> +    warning_add(warning_gcov);
> +    add_taint(TAINT_GCOV);
> +#endif

... (if we want this in the first place) how about

#ifdef CONFIG_GCOV
    {
        static const char __initconst warning_gcov[] = "...";

        warning_add(warning_gcov);
        add_taint(TAINT_GCOV);
    }
#endif

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 2/5] gcov: collect more sections to constructor list

[Jan Beulich]

>>> On 02.09.16 at 13:47, <wei.liu2@citrix.com> wrote:
> --- a/xen/arch/x86/xen.lds.S
> +++ b/xen/arch/x86/xen.lds.S
> @@ -205,6 +205,8 @@ SECTIONS
>         . = ALIGN(8);
>         __ctors_start = .;
>         *(.ctors)
> +       *(SORT(.init_array.*))
> +       *(.init_array)
>         __ctors_end = .;
>    } :text

Is the ordering meaningful? If so, a comment is warranted. If not,
elsewhere we have <section> precede <section>.* .

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 3/5] xen: replace TEST_COVERAGE with CONFIG_GCOV

[Jan Beulich]

>>> On 02.09.16 at 13:47, <wei.liu2@citrix.com> wrote:
> The sole purpose of TEST_COVERAGE macro is to guard the availability of
> gcov sysctl. Now we have a proper CONFIG_GCOV, use it.
> 
> 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

Re: [PATCH 1/5] xen: add tainted state and show warning is gcov is enabled

[Wei Liu]

On Fri, Sep 02, 2016 at 05:56:49AM -0600, Jan Beulich wrote:
> >>> On 02.09.16 at 13:47, <wei.liu2@citrix.com> wrote:
> 
> Since this is a config option - why bother issuing a warning and
> tainting the hypervisor?
> 

Because there isn't a clear indicator if gcov is enabled.

I think it would be valuable to just tell from the backtrace or console
log that gcov is enabled, then we can legitimately refuse to provide
(security) support for such builds.

> > --- a/xen/common/gcov/gcov.c
> > +++ b/xen/common/gcov/gcov.c
> > @@ -23,6 +23,11 @@
> >  #include <public/xen.h>
> >  #include <public/gcov.h>
> >  
> > +const char __initconst warning_gcov[] =
> > +    "WARNING: GCOV SUPPORT IS ENABLED\n"
> > +    "This option is *ONLY* intended to aid testing of Xen.\n"
> > +    "Please *DO NOT* use this in production.\n";
> 
> Note the type (const) difference between this and ...
> 
> > --- a/xen/drivers/char/console.c
> > +++ b/xen/drivers/char/console.c
> > @@ -792,6 +792,10 @@ void __init console_init_postirq(void)
> >      console_init_ring();
> >  }
> >  
> > +#ifdef CONFIG_GCOV
> > +extern char warning_gcov[];
> > +#endif
> 
> ... this. That's one of the reasons declarations of stuff defined in
> C sources should be put in a header, which then gets included by
> both producer and consumer(s). But ...
> 
> > @@ -802,6 +806,11 @@ void __init console_endboot(void)
> >          printk(" (Rate-limited: %s)", loglvl_str(xenlog_guest_upper_thresh));
> >      printk("\n");
> >  
> > +#ifdef CONFIG_GCOV
> > +    warning_add(warning_gcov);
> > +    add_taint(TAINT_GCOV);
> > +#endif
> 
> ... (if we want this in the first place) how about
> 
> #ifdef CONFIG_GCOV
>     {
>         static const char __initconst warning_gcov[] = "...";
> 
>         warning_add(warning_gcov);
>         add_taint(TAINT_GCOV);
>     }
> #endif
> 

Fine with me.

Wei.

> Jan
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 4/5] gcov: add option to determine gcov format

[Jan Beulich]

>>> On 02.09.16 at 13:47, <wei.liu2@citrix.com> wrote:
> Currently only gcc 3.4 format is supported.

Doesn't this patch contradict your coverage letter? Here you provide
means to add support for further formats, but there you said there's
no obvious route to that goal.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 0/5] gcov: more cleanup

[Andrew Cooper]

On 02/09/16 12:47, Wei Liu wrote:
> This series further cleans up existing gcov code. It should be now clear that
> Xen's gcov is using gcc 3.4 format.
>
> I have tried to integrate gcc 4.7 format. The amount of work is moderate in
> terms of coding effort, but I now believe it is a mistake to have invented xen
> specific format which is in fact tied to gcc 3.4. I can't map gcc 4.7 format to
> Xen's own format, so I stop here.

Given that the interface has clearly bitrotten since it was first
introduced, I think it is reasonable to declare amnesty to fix it properly.

As the format is chosen at configure time, I think it is reasonable for
the Xen interface to just hand back blobs, and require the tools in dom0
to know how to interpret them.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 0/5] gcov: more cleanup

[Wei Liu]

On Fri, Sep 02, 2016 at 01:04:45PM +0100, Andrew Cooper wrote:
> On 02/09/16 12:47, Wei Liu wrote:
> > This series further cleans up existing gcov code. It should be now clear that
> > Xen's gcov is using gcc 3.4 format.
> >
> > I have tried to integrate gcc 4.7 format. The amount of work is moderate in
> > terms of coding effort, but I now believe it is a mistake to have invented xen
> > specific format which is in fact tied to gcc 3.4. I can't map gcc 4.7 format to
> > Xen's own format, so I stop here.
> 
> Given that the interface has clearly bitrotten since it was first
> introduced, I think it is reasonable to declare amnesty to fix it properly.
> 
> As the format is chosen at configure time, I think it is reasonable for
> the Xen interface to just hand back blobs, and require the tools in dom0
> to know how to interpret them.
> 

Yes. I think that's how I would do it. But I don't have time for it now.

Wei.

> ~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 1/5] xen: add tainted state and show warning is gcov is enabled

[Jan Beulich]

>>> On 02.09.16 at 14:01, <wei.liu2@citrix.com> wrote:
> On Fri, Sep 02, 2016 at 05:56:49AM -0600, Jan Beulich wrote:
>> >>> On 02.09.16 at 13:47, <wei.liu2@citrix.com> wrote:
>> 
>> Since this is a config option - why bother issuing a warning and
>> tainting the hypervisor?
>> 
> 
> Because there isn't a clear indicator if gcov is enabled.
> 
> I think it would be valuable to just tell from the backtrace or console
> log that gcov is enabled, then we can legitimately refuse to provide
> (security) support for such builds.

Then perhaps making it match the "debug=" would be the better
approach for a feature not controlled on the command line?

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 4/5] gcov: add option to determine gcov format

[Wei Liu]

On Fri, Sep 02, 2016 at 06:01:22AM -0600, Jan Beulich wrote:
> >>> On 02.09.16 at 13:47, <wei.liu2@citrix.com> wrote:
> > Currently only gcc 3.4 format is supported.
> 
> Doesn't this patch contradict your coverage letter? Here you provide
> means to add support for further formats, but there you said there's
> no obvious route to that goal.
> 

There is a way, we can ditch the old interface and just hand back the
blob.

Wei.

> Jan
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 2/5] gcov: collect more sections to constructor list

[Wei Liu]

On Fri, Sep 02, 2016 at 05:58:43AM -0600, Jan Beulich wrote:
> >>> On 02.09.16 at 13:47, <wei.liu2@citrix.com> wrote:
> > --- a/xen/arch/x86/xen.lds.S
> > +++ b/xen/arch/x86/xen.lds.S
> > @@ -205,6 +205,8 @@ SECTIONS
> >         . = ALIGN(8);
> >         __ctors_start = .;
> >         *(.ctors)
> > +       *(SORT(.init_array.*))
> > +       *(.init_array)
> >         __ctors_end = .;
> >    } :text
> 
> Is the ordering meaningful? If so, a comment is warranted. If not,
> elsewhere we have <section> precede <section>.* .
> 

I don't think the order matters.

I will swap the position of the two entries.

Wei.

> Jan
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 1/5] xen: add tainted state and show warning is gcov is enabled

[Andrew Cooper]

On 02/09/16 13:06, Jan Beulich wrote:
>>>> On 02.09.16 at 14:01, <wei.liu2@citrix.com> wrote:
>> On Fri, Sep 02, 2016 at 05:56:49AM -0600, Jan Beulich wrote:
>>>>>> On 02.09.16 at 13:47, <wei.liu2@citrix.com> wrote:
>>> Since this is a config option - why bother issuing a warning and
>>> tainting the hypervisor?
>>>
>> Because there isn't a clear indicator if gcov is enabled.
>>
>> I think it would be valuable to just tell from the backtrace or console
>> log that gcov is enabled, then we can legitimately refuse to provide
>> (security) support for such builds.
> Then perhaps making it match the "debug=" would be the better
> approach for a feature not controlled on the command line?

I would prefer not to make it depend on debug=

Coverage on a release hypervisor is equally important, and will be
different from a debug hypervisor.

I am on the fence as to whether a taint is right to use, but I do think
that a "with GCOV" is needed somewhere obvious on the banner line.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 0/5] gcov: more cleanup

[Andrew Cooper]

On 02/09/16 13:06, Wei Liu wrote:
> On Fri, Sep 02, 2016 at 01:04:45PM +0100, Andrew Cooper wrote:
>> On 02/09/16 12:47, Wei Liu wrote:
>>> This series further cleans up existing gcov code. It should be now clear that
>>> Xen's gcov is using gcc 3.4 format.
>>>
>>> I have tried to integrate gcc 4.7 format. The amount of work is moderate in
>>> terms of coding effort, but I now believe it is a mistake to have invented xen
>>> specific format which is in fact tied to gcc 3.4. I can't map gcc 4.7 format to
>>> Xen's own format, so I stop here.
>> Given that the interface has clearly bitrotten since it was first
>> introduced, I think it is reasonable to declare amnesty to fix it properly.
>>
>> As the format is chosen at configure time, I think it is reasonable for
>> the Xen interface to just hand back blobs, and require the tools in dom0
>> to know how to interpret them.
>>
> Yes. I think that's how I would do it. But I don't have time for it now.

That's perfectly ok.  Thanks for getting this far.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 1/5] xen: add tainted state and show warning is gcov is enabled

[Jan Beulich]

>>> On 02.09.16 at 14:13, <andrew.cooper3@citrix.com> wrote:
> On 02/09/16 13:06, Jan Beulich wrote:
>>>>> On 02.09.16 at 14:01, <wei.liu2@citrix.com> wrote:
>>> On Fri, Sep 02, 2016 at 05:56:49AM -0600, Jan Beulich wrote:
>>>>>>> On 02.09.16 at 13:47, <wei.liu2@citrix.com> wrote:
>>>> Since this is a config option - why bother issuing a warning and
>>>> tainting the hypervisor?
>>>>
>>> Because there isn't a clear indicator if gcov is enabled.
>>>
>>> I think it would be valuable to just tell from the backtrace or console
>>> log that gcov is enabled, then we can legitimately refuse to provide
>>> (security) support for such builds.
>> Then perhaps making it match the "debug=" would be the better
>> approach for a feature not controlled on the command line?
> 
> I would prefer not to make it depend on debug=
> 
> Coverage on a release hypervisor is equally important, and will be
> different from a debug hypervisor.

I didn't say "depend on", but "match" (which I mean just logging wise).

> I am on the fence as to whether a taint is right to use, but I do think
> that a "with GCOV" is needed somewhere obvious on the banner line.

Right, hence the matching goal with "debug=".

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 1/5] xen: add tainted state and show warning is gcov is enabled

[Andrew Cooper]

On 02/09/16 13:26, Jan Beulich wrote:
>>>> On 02.09.16 at 14:13, <andrew.cooper3@citrix.com> wrote:
>> On 02/09/16 13:06, Jan Beulich wrote:
>>>>>> On 02.09.16 at 14:01, <wei.liu2@citrix.com> wrote:
>>>> On Fri, Sep 02, 2016 at 05:56:49AM -0600, Jan Beulich wrote:
>>>>>>>> On 02.09.16 at 13:47, <wei.liu2@citrix.com> wrote:
>>>>> Since this is a config option - why bother issuing a warning and
>>>>> tainting the hypervisor?
>>>>>
>>>> Because there isn't a clear indicator if gcov is enabled.
>>>>
>>>> I think it would be valuable to just tell from the backtrace or console
>>>> log that gcov is enabled, then we can legitimately refuse to provide
>>>> (security) support for such builds.
>>> Then perhaps making it match the "debug=" would be the better
>>> approach for a feature not controlled on the command line?
>> I would prefer not to make it depend on debug=
>>
>> Coverage on a release hypervisor is equally important, and will be
>> different from a debug hypervisor.
> I didn't say "depend on", but "match" (which I mean just logging wise).
>
>> I am on the fence as to whether a taint is right to use, but I do think
>> that a "with GCOV" is needed somewhere obvious on the banner line.
> Right, hence the matching goal with "debug=".

Ah - I see what you mean.  Yes - that would be fine by me.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 1/5] xen: add tainted state and show warning is gcov is enabled

[Julien Grall]

Hi Wei,

On 02/09/16 13:01, Wei Liu wrote:
> On Fri, Sep 02, 2016 at 05:56:49AM -0600, Jan Beulich wrote:
>>>>> On 02.09.16 at 13:47, <wei.liu2@citrix.com> wrote:
>>
>> Since this is a config option - why bother issuing a warning and
>> tainting the hypervisor?
>>
>
> Because there isn't a clear indicator if gcov is enabled.

I think this is an issue to all .config option. If I am not mistaken, 
currently you are not able to know whether option FOO has been enabled 
for your kernel.

Maybe we should integrate the .config in the binary?

Regards,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 1/5] xen: add tainted state and show warning is gcov is enabled

[Wei Liu]

On Fri, Sep 02, 2016 at 01:30:40PM +0100, Andrew Cooper wrote:
> On 02/09/16 13:26, Jan Beulich wrote:
> >>>> On 02.09.16 at 14:13, <andrew.cooper3@citrix.com> wrote:
> >> On 02/09/16 13:06, Jan Beulich wrote:
> >>>>>> On 02.09.16 at 14:01, <wei.liu2@citrix.com> wrote:
> >>>> On Fri, Sep 02, 2016 at 05:56:49AM -0600, Jan Beulich wrote:
> >>>>>>>> On 02.09.16 at 13:47, <wei.liu2@citrix.com> wrote:
> >>>>> Since this is a config option - why bother issuing a warning and
> >>>>> tainting the hypervisor?
> >>>>>
> >>>> Because there isn't a clear indicator if gcov is enabled.
> >>>>
> >>>> I think it would be valuable to just tell from the backtrace or console
> >>>> log that gcov is enabled, then we can legitimately refuse to provide
> >>>> (security) support for such builds.
> >>> Then perhaps making it match the "debug=" would be the better
> >>> approach for a feature not controlled on the command line?
> >> I would prefer not to make it depend on debug=
> >>
> >> Coverage on a release hypervisor is equally important, and will be
> >> different from a debug hypervisor.
> > I didn't say "depend on", but "match" (which I mean just logging wise).
> >
> >> I am on the fence as to whether a taint is right to use, but I do think
> >> that a "with GCOV" is needed somewhere obvious on the banner line.
> > Right, hence the matching goal with "debug=".
> 
> Ah - I see what you mean.  Yes - that would be fine by me.
> 

Fine by me, too.

I will replace this patch with a new one.

Wei.

> ~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 1/5] xen: add tainted state and show warning is gcov is enabled

[Wei Liu]

On Fri, Sep 02, 2016 at 02:21:28PM +0100, Julien Grall wrote:
> Hi Wei,
> 
> On 02/09/16 13:01, Wei Liu wrote:
> >On Fri, Sep 02, 2016 at 05:56:49AM -0600, Jan Beulich wrote:
> >>>>>On 02.09.16 at 13:47, <wei.liu2@citrix.com> wrote:
> >>
> >>Since this is a config option - why bother issuing a warning and
> >>tainting the hypervisor?
> >>
> >
> >Because there isn't a clear indicator if gcov is enabled.
> 
> I think this is an issue to all .config option. If I am not mistaken,
> currently you are not able to know whether option FOO has been enabled for
> your kernel.
> 
> Maybe we should integrate the .config in the binary?
> 

I think that would be a good idea.  It is orthogonal to what I'm trying
to do here though.

Wei.

> Regards,
> 
> -- 
> Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 4/5] gcov: add option to determine gcov format

[Wei Liu]

On Fri, Sep 02, 2016 at 01:08:27PM +0100, Wei Liu wrote:
> On Fri, Sep 02, 2016 at 06:01:22AM -0600, Jan Beulich wrote:
> > >>> On 02.09.16 at 13:47, <wei.liu2@citrix.com> wrote:
> > > Currently only gcc 3.4 format is supported.
> > 
> > Doesn't this patch contradict your coverage letter? Here you provide
> > means to add support for further formats, but there you said there's
> > no obvious route to that goal.
> > 
> 
> There is a way, we can ditch the old interface and just hand back the
> blob.
> 

Let me try to make myself clearer because now I reread my reply it
doesn't seem to convey my thought.

There are two issues:

1. The sysctl interface is tied to gcc 3.4 format.
2. The implementation inside Xen is tied to gcc 3.4 format.

My cover letter was referring to #1 because there is no way to fit newer
gcc format into the existing Xen sysctl coverage interface. To solve #1
I'm afraid we need to design new interfaces.

#2 is independent of #1. Regardless of what the sysctl interface looks
like, Xen needs to know which format to use (size of the structure,
offset of fields etc) in order to extract information.

This patch (sorta) deals with #2 and is one step towards dealing with
#1.

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 2/5] gcov: collect more sections to constructor list

[Julien Grall]

Hi Wei,

On 02/09/2016 12:47, Wei Liu wrote:
> The version of gcc (4.9.2) I use put constructors into .init_array*
> section(s). Collect those sections into constructor list as well.
>
> Modify both arm and x86 scripts to keep them in sync.

With Jan's comment handled:

Acked-by: Julien Grall <julien.grall@arm.com>

Regards,

>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Julien Grall <julien.grall@arm.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
>  xen/arch/arm/xen.lds.S | 2 ++
>  xen/arch/x86/xen.lds.S | 2 ++
>  2 files changed, 4 insertions(+)
>
> diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S
> index b24e93b..ffff13a 100644
> --- a/xen/arch/arm/xen.lds.S
> +++ b/xen/arch/arm/xen.lds.S
> @@ -166,6 +166,8 @@ SECTIONS
>
>         . = ALIGN(8);
>         __ctors_start = .;
> +       *(.ctors)
> +       *(SORT(.init_array.*))
>         *(.init_array)
>         __ctors_end = .;
>    } :text
> diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
> index 67cfda1..8957efd 100644
> --- a/xen/arch/x86/xen.lds.S
> +++ b/xen/arch/x86/xen.lds.S
> @@ -205,6 +205,8 @@ SECTIONS
>         . = ALIGN(8);
>         __ctors_start = .;
>         *(.ctors)
> +       *(SORT(.init_array.*))
> +       *(.init_array)
>         __ctors_end = .;
>    } :text
>
>

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 4/5] gcov: add option to determine gcov format

[Wei Liu]

On Fri, Sep 02, 2016 at 12:47:08PM +0100, Wei Liu wrote:
> Currently only gcc 3.4 format is supported.
> 
> 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>

What should I do with this patch and the next one?

I'm fine with dropping both and replace them with a patch to
Kconfig.debug to make clear the format is currently 3.4, if people feel
strongly about that.

Wei.

> ---
>  xen/Kconfig.debug | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/xen/Kconfig.debug b/xen/Kconfig.debug
> index 06afd80..2366b06 100644
> --- a/xen/Kconfig.debug
> +++ b/xen/Kconfig.debug
> @@ -33,6 +33,19 @@ config GCOV
>         ---help---
>           Enable gcov (a test coverage program in GCC) support.
>  
> +choice
> +	prompt "Specify Gcov format"
> +	depends on GCOV
> +	---help---
> +	The gcov format is determined by gcc version.
> +
> +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---
> -- 
> 2.1.4
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH 3/5] xen: replace TEST_COVERAGE with CONFIG_GCOV

[Ian Jackson]

Wei Liu writes ("[PATCH 3/5] xen: replace TEST_COVERAGE with CONFIG_GCOV"):
> The sole purpose of TEST_COVERAGE macro is to guard the availability of
> gcov sysctl. Now we have a proper CONFIG_GCOV, use it.

FAOD my reading of xen/Kconfig.debug is that CONFIG_GCOV depends on
CONFIG_DEBUG which says

         This option is intended for development purposes
         only, and not for production use.

and is therefore out of security support.

Accordingly I think the only security review needed for the hypervisor
part is to check that everything is indeed disabled without
CONFIG_GCOV.

Thanks,
Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel