[PATCH 0/4] Fix gcov build

[PATCH 0/4] Fix gcov build

[Wei Liu]

A small series to fix gcov build for both x86 and arm.

Wei Liu (4):
  arm: acpi/boot.c is only used during initialisation
  arm64: use "b" to branch to start_xen
  xen: fix gcov compilation
  xen: add a gcov Kconfig option

 Config.mk                  | 3 ---
 xen/Kconfig.debug          | 5 +++++
 xen/Rules.mk               | 4 +++-
 xen/arch/arm/acpi/Makefile | 2 +-
 xen/arch/arm/acpi/boot.c   | 2 +-
 xen/arch/arm/arm64/head.S  | 4 +++-
 xen/arch/x86/efi/Makefile  | 1 +
 xen/common/Makefile        | 2 +-
 8 files changed, 15 insertions(+), 8 deletions(-)

-- 
2.1.4


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

[PATCH 1/4] arm: acpi/boot.c is only used during initialisation

[Wei Liu]

That file should contain code and data used during initialisation only.

Mark it as such in build system and correctly annotate enabled_cpus.
This is required to enable gcov support on ARM.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
---
 xen/arch/arm/acpi/Makefile | 2 +-
 xen/arch/arm/acpi/boot.c   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/acpi/Makefile b/xen/arch/arm/acpi/Makefile
index 196c40a..23963f8 100644
--- a/xen/arch/arm/acpi/Makefile
+++ b/xen/arch/arm/acpi/Makefile
@@ -1,2 +1,2 @@
 obj-y += lib.o
-obj-y += boot.o
+obj-y += boot.init.o
diff --git a/xen/arch/arm/acpi/boot.c b/xen/arch/arm/acpi/boot.c
index 28b3450..c3242a0 100644
--- a/xen/arch/arm/acpi/boot.c
+++ b/xen/arch/arm/acpi/boot.c
@@ -37,7 +37,7 @@
 #include <asm/setup.h>
 
 /* Processors with enabled flag and sane MPIDR */
-static unsigned int enabled_cpus = 1;
+static unsigned int __initdata enabled_cpus = 1;
 static bool __initdata bootcpu_valid;
 
 /* total number of cpus in this system */
-- 
2.1.4


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

Re: [PATCH 1/4] arm: acpi/boot.c is only used during initialisation

[Julien Grall]

Hi Wei,

On 30/08/16 13:01, Wei Liu wrote:
> That file should contain code and data used during initialisation only.
>
> Mark it as such in build system and correctly annotate enabled_cpus.
> This is required to enable gcov support on ARM.

I am not sure to understand the requirement here. Does it mean it is not 
possible to have a mix of init code and runtime code?

>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

Regardless the questions:

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

> ---
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Julien Grall <julien.grall@arm.com>
> ---
>  xen/arch/arm/acpi/Makefile | 2 +-
>  xen/arch/arm/acpi/boot.c   | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/xen/arch/arm/acpi/Makefile b/xen/arch/arm/acpi/Makefile
> index 196c40a..23963f8 100644
> --- a/xen/arch/arm/acpi/Makefile
> +++ b/xen/arch/arm/acpi/Makefile
> @@ -1,2 +1,2 @@
>  obj-y += lib.o
> -obj-y += boot.o
> +obj-y += boot.init.o
> diff --git a/xen/arch/arm/acpi/boot.c b/xen/arch/arm/acpi/boot.c
> index 28b3450..c3242a0 100644
> --- a/xen/arch/arm/acpi/boot.c
> +++ b/xen/arch/arm/acpi/boot.c
> @@ -37,7 +37,7 @@
>  #include <asm/setup.h>
>
>  /* Processors with enabled flag and sane MPIDR */
> -static unsigned int enabled_cpus = 1;
> +static unsigned int __initdata enabled_cpus = 1;
>  static bool __initdata bootcpu_valid;
>
>  /* total number of cpus in this system */
>

Regards,

-- 
Julien Grall

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

Re: [PATCH 1/4] arm: acpi/boot.c is only used during initialisation

[Wei Liu]

On Wed, Aug 31, 2016 at 02:25:59PM +0100, Julien Grall wrote:
> Hi Wei,
> 
> On 30/08/16 13:01, Wei Liu wrote:
> >That file should contain code and data used during initialisation only.
> >
> >Mark it as such in build system and correctly annotate enabled_cpus.
> >This is required to enable gcov support on ARM.
> 
> I am not sure to understand the requirement here. Does it mean it is not
> possible to have a mix of init code and runtime code?
> 

No. That's still allowed.

"This is required to enable gcov support on ARM" is remnant that should
be deleted, because strictly speaking if we don't change boot.o to
boot.init.o, the resulting binary should still work.

Please consider this patch a general cleanup.

Can I keep your RoB?

> >
> >Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> 
> Regardless the questions:
> 
> Reviewed-by: Julien Grall <julien.grall@arm.com>
> 
> >---
> >Cc: Stefano Stabellini <sstabellini@kernel.org>
> >Cc: Julien Grall <julien.grall@arm.com>
> >---
> > xen/arch/arm/acpi/Makefile | 2 +-
> > xen/arch/arm/acpi/boot.c   | 2 +-
> > 2 files changed, 2 insertions(+), 2 deletions(-)
> >
> >diff --git a/xen/arch/arm/acpi/Makefile b/xen/arch/arm/acpi/Makefile
> >index 196c40a..23963f8 100644
> >--- a/xen/arch/arm/acpi/Makefile
> >+++ b/xen/arch/arm/acpi/Makefile
> >@@ -1,2 +1,2 @@
> > obj-y += lib.o
> >-obj-y += boot.o
> >+obj-y += boot.init.o
> >diff --git a/xen/arch/arm/acpi/boot.c b/xen/arch/arm/acpi/boot.c
> >index 28b3450..c3242a0 100644
> >--- a/xen/arch/arm/acpi/boot.c
> >+++ b/xen/arch/arm/acpi/boot.c
> >@@ -37,7 +37,7 @@
> > #include <asm/setup.h>
> >
> > /* Processors with enabled flag and sane MPIDR */
> >-static unsigned int enabled_cpus = 1;
> >+static unsigned int __initdata enabled_cpus = 1;
> > static bool __initdata bootcpu_valid;
> >
> > /* total number of cpus in this system */
> >
> 
> Regards,
> 
> -- 
> Julien Grall

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

Re: [PATCH 1/4] arm: acpi/boot.c is only used during initialisation

[Julien Grall]

On 31/08/16 14:45, Wei Liu wrote:
> On Wed, Aug 31, 2016 at 02:25:59PM +0100, Julien Grall wrote:
>> Hi Wei,
>>
>> On 30/08/16 13:01, Wei Liu wrote:
>>> That file should contain code and data used during initialisation only.
>>>
>>> Mark it as such in build system and correctly annotate enabled_cpus.
>>> This is required to enable gcov support on ARM.
>>
>> I am not sure to understand the requirement here. Does it mean it is not
>> possible to have a mix of init code and runtime code?
>>
>
> No. That's still allowed.
>
> "This is required to enable gcov support on ARM" is remnant that should
> be deleted, because strictly speaking if we don't change boot.o to
> boot.init.o, the resulting binary should still work.
>
> Please consider this patch a general cleanup.

I had a similar clean up in my todo list :).

> Can I keep your RoB?

Sure.

Regards,

-- 
Julien Grall

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

[PATCH 2/4] arm64: use "b" to branch to start_xen

[Wei Liu]

The cbz instruction has range limitation. When compiled with gcov
support the object is larger so cbz can't handle that anymore. The error
message is like:

aarch64-linux-gnu-ld    -EL  -T xen.lds -N prelink.o \
    /local/work/xen.git/xen/common/symbols-dummy.o -o /local/work/xen.git/xen/.xen-syms.0
prelink.o: In function `launch':
/local/work/xen.git/xen/arch/arm/arm64/head.S:602:(.text+0x408): relocation truncated to fit: R_AARCH64_CONDBR19 against symbol `start_xen' defined in .init.text section in prelink.o

Use "b" instead.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
Compile test only.

Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
---
 xen/arch/arm/arm64/head.S | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
index 91e2817..3f63d2a 100644
--- a/xen/arch/arm/arm64/head.S
+++ b/xen/arch/arm/arm64/head.S
@@ -599,7 +599,9 @@ launch:
         mov   x0, x20                /* Marshal args: - phys_offset */
         mov   x1, x21                /*               - FDT */
         mov   x2, x24                /*               - CPU ID */
-        cbz   x22, start_xen         /* and disappear into the land of C */
+        cbnz  x22, 1f
+        b     start_xen              /* and disappear into the land of C */
+1:
         b     start_secondary        /* (to the appropriate entry point) */
 
 /* Fail-stop */
-- 
2.1.4


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

Re: [PATCH 2/4] arm64: use "b" to branch to start_xen

[Konrad Rzeszutek Wilk]

On Tue, Aug 30, 2016 at 01:01:30PM +0100, Wei Liu wrote:
> The cbz instruction has range limitation. When compiled with gcov
> support the object is larger so cbz can't handle that anymore. The error
> message is like:
> 
> aarch64-linux-gnu-ld    -EL  -T xen.lds -N prelink.o \
>     /local/work/xen.git/xen/common/symbols-dummy.o -o /local/work/xen.git/xen/.xen-syms.0
> prelink.o: In function `launch':
> /local/work/xen.git/xen/arch/arm/arm64/head.S:602:(.text+0x408): relocation truncated to fit: R_AARCH64_CONDBR19 against symbol `start_xen' defined in .init.text section in prelink.o
> 
> Use "b" instead.
> 
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
> Compile test only.

Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> 
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Julien Grall <julien.grall@arm.com>
> ---
>  xen/arch/arm/arm64/head.S | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
> index 91e2817..3f63d2a 100644
> --- a/xen/arch/arm/arm64/head.S
> +++ b/xen/arch/arm/arm64/head.S
> @@ -599,7 +599,9 @@ launch:
>          mov   x0, x20                /* Marshal args: - phys_offset */
>          mov   x1, x21                /*               - FDT */
>          mov   x2, x24                /*               - CPU ID */
> -        cbz   x22, start_xen         /* and disappear into the land of C */
> +        cbnz  x22, 1f
> +        b     start_xen              /* and disappear into the land of C */
> +1:
>          b     start_secondary        /* (to the appropriate entry point) */
>  
>  /* Fail-stop */
> -- 
> 2.1.4
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> https://lists.xen.org/xen-devel

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

Re: [PATCH 2/4] arm64: use "b" to branch to start_xen

[Julien Grall]

Hi Wei,

On 30/08/16 13:01, Wei Liu wrote:
> The cbz instruction has range limitation. When compiled with gcov
> support the object is larger so cbz can't handle that anymore. The error
> message is like:
>
> aarch64-linux-gnu-ld    -EL  -T xen.lds -N prelink.o \
>     /local/work/xen.git/xen/common/symbols-dummy.o -o /local/work/xen.git/xen/.xen-syms.0
> prelink.o: In function `launch':
> /local/work/xen.git/xen/arch/arm/arm64/head.S:602:(.text+0x408): relocation truncated to fit: R_AARCH64_CONDBR19 against symbol `start_xen' defined in .init.text section in prelink.o
>
> Use "b" instead.

 From a quick look the size of xen doubled (776K -> 1.4M) with GCov. Xen 
on ARM assumes the binary will always fit in 2MB, it might be time to 
get rid of this limit.

I know that enabling UBSAN [1] will lead to a binary bigger than 2MB and 
Xen will explode in early boot.

> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

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

Regards,

[1] 
http://developers.redhat.com/blog/2014/10/16/gcc-undefined-behavior-sanitizer-ubsan/

-- 
Julien Grall

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

[PATCH 3/4] xen: fix gcov compilation

[Wei Liu]

Currently enabling gcov in hypervisor won't build because although
26c9d03d ("gcov: Adding support for coverage information") claimed that
%.init.o files were excluded from applying compilation options, it was
in fact not true.

Fix that by filtering out the options correctly. Due to the dependency
of stub.o in x86 EFI build can't be eliminated easily and we prefer a
generalised method going forward, we introduce nogov-y to explicitly
mark objects that don't need to build with gcov support.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Signed-off-by: Jan Beulich <JBeulich@suse.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              | 4 +++-
 xen/arch/x86/efi/Makefile | 1 +
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/xen/Rules.mk b/xen/Rules.mk
index a190ff0..22aca0a 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -115,7 +115,9 @@ subdir-all := $(subdir-y) $(subdir-n)
 
 $(filter %.init.o,$(obj-y) $(obj-bin-y) $(extra-y)): CFLAGS += -DINIT_SECTIONS_ONLY
 
-$(obj-$(coverage)): CFLAGS += -fprofile-arcs -ftest-coverage -DTEST_COVERAGE
+ifeq ($(coverage),y)
+$(filter-out %.init.o $(nogcov-y),$(obj-y) $(obj-bin-y) $(extra-y)): CFLAGS += -fprofile-arcs -ftest-coverage -DTEST_COVERAGE
+endif
 
 ifeq ($(lto),y)
 # Would like to handle all object files as bitcode, but objects made from
diff --git a/xen/arch/x86/efi/Makefile b/xen/arch/x86/efi/Makefile
index 5099430..d62b14f 100644
--- a/xen/arch/x86/efi/Makefile
+++ b/xen/arch/x86/efi/Makefile
@@ -12,3 +12,4 @@ efi := $(if $(efi),$(shell rm disabled)y,$(shell $(call create,boot.init.o); $(c
 extra-$(efi) += boot.init.o relocs-dummy.o runtime.o compat.o
 
 stub.o: $(extra-y)
+nogcov-$(efi) += stub.o
-- 
2.1.4


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

Re: [PATCH 3/4] xen: fix gcov compilation

[Ian Jackson]

Wei Liu writes ("[PATCH 3/4] xen: fix gcov compilation"):
> Currently enabling gcov in hypervisor won't build because although
> 26c9d03d ("gcov: Adding support for coverage information") claimed that
> %.init.o files were excluded from applying compilation options, it was
> in fact not true.
> 
> Fix that by filtering out the options correctly. Due to the dependency
> of stub.o in x86 EFI build can't be eliminated easily and we prefer a
> generalised method going forward, we introduce nogov-y to explicitly
                                                    ^
> mark objects that don't need to build with gcov support.

And I'm afraid the second sentence there is not comprehensible.  It
seesm to be "Due to X, Y" but I can't quite figure out which parts of
the sentence are X and which are Y.

I'm not unhappy with the approach in the Makefile, though.

Ian.

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

Re: [PATCH 3/4] xen: fix gcov compilation

[Jan Beulich]

>>> On 30.08.16 at 14:57, <ian.jackson@eu.citrix.com> wrote:
> Wei Liu writes ("[PATCH 3/4] xen: fix gcov compilation"):
>> Currently enabling gcov in hypervisor won't build because although
>> 26c9d03d ("gcov: Adding support for coverage information") claimed that
>> %.init.o files were excluded from applying compilation options, it was
>> in fact not true.
>> 
>> Fix that by filtering out the options correctly. Due to the dependency
>> of stub.o in x86 EFI build can't be eliminated easily and we prefer a
>> generalised method going forward, we introduce nogov-y to explicitly
>                                                     ^
>> mark objects that don't need to build with gcov support.
> 
> And I'm afraid the second sentence there is not comprehensible.  It
> seesm to be "Due to X, Y" but I can't quite figure out which parts of
> the sentence are X and which are Y.

Does s/Due to/Since/ in the original sentence help?

Jan


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

Re: [PATCH 3/4] xen: fix gcov compilation

[Wei Liu]

On Tue, Aug 30, 2016 at 01:57:06PM +0100, Ian Jackson wrote:
> Wei Liu writes ("[PATCH 3/4] xen: fix gcov compilation"):
> > Currently enabling gcov in hypervisor won't build because although
> > 26c9d03d ("gcov: Adding support for coverage information") claimed that
> > %.init.o files were excluded from applying compilation options, it was
> > in fact not true.
> > 
> > Fix that by filtering out the options correctly. Due to the dependency
> > of stub.o in x86 EFI build can't be eliminated easily and we prefer a
> > generalised method going forward, we introduce nogov-y to explicitly
>                                                     ^
> > mark objects that don't need to build with gcov support.
> 
> And I'm afraid the second sentence there is not comprehensible.  It
> seesm to be "Due to X, Y" but I can't quite figure out which parts of
> the sentence are X and which are Y.
> 

Let's rewrite it as such:

Due to 1) the dependency of stub.o in x86 EFI build can't be eliminated
easily and 2) we prefer a generalised method going forward, we introduce
nogov-y to explicitly.

Is this clearer?

> I'm not unhappy with the approach in the Makefile, though.
> 
> Ian.

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

[PATCH 4/4] xen: add a gcov Kconfig option

[Wei Liu]

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: Doug Goldstein <cardoe@cardoe.com>

Would like to have it default to DEBUG, but we also need to check
compiler to be gcc. I couldn't figure out how to check compiler to be
gcc in Kconfig.
---
 Config.mk           | 3 ---
 xen/Kconfig.debug   | 5 +++++
 xen/Rules.mk        | 2 +-
 xen/common/Makefile | 2 +-
 4 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/Config.mk b/Config.mk
index 9c60896..081ff69 100644
--- a/Config.mk
+++ b/Config.mk
@@ -20,9 +20,6 @@ or       = $(if $(strip $(1)),$(1),$(if $(strip $(2)),$(2),$(if $(strip $(3)),$(
 debug ?= y
 debug_symbols ?= $(debug)
 
-# Test coverage support
-coverage ?= n
-
 XEN_COMPILE_ARCH    ?= $(shell uname -m | sed -e s/i.86/x86_32/ \
                          -e s/i86pc/x86_32/ -e s/amd64/x86_64/ \
                          -e s/armv7.*/arm32/ -e s/armv8.*/arm64/ \
diff --git a/xen/Kconfig.debug b/xen/Kconfig.debug
index 1be6344..a0bd043 100644
--- a/xen/Kconfig.debug
+++ b/xen/Kconfig.debug
@@ -13,6 +13,11 @@ config DEBUG
 
 if DEBUG || EXPERT = "y"
 
+config GCOV
+       bool "Gcov Support"
+       ---help---
+         Enable gcov (a test coverage program in GCC) support.
+
 config CRASH_DEBUG
 	bool "Crash Debugging Support"
 	depends on X86
diff --git a/xen/Rules.mk b/xen/Rules.mk
index 22aca0a..696aaa8 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -115,7 +115,7 @@ subdir-all := $(subdir-y) $(subdir-n)
 
 $(filter %.init.o,$(obj-y) $(obj-bin-y) $(extra-y)): CFLAGS += -DINIT_SECTIONS_ONLY
 
-ifeq ($(coverage),y)
+ifeq ($(CONFIG_GCOV),y)
 $(filter-out %.init.o $(nogcov-y),$(obj-y) $(obj-bin-y) $(extra-y)): CFLAGS += -fprofile-arcs -ftest-coverage -DTEST_COVERAGE
 endif
 
diff --git a/xen/common/Makefile b/xen/common/Makefile
index c2e6846..0fed30b 100644
--- a/xen/common/Makefile
+++ b/xen/common/Makefile
@@ -75,7 +75,7 @@ obj-$(CONFIG_TMEM) += $(tmem-y)
 
 subdir-$(CONFIG_X86) += hvm
 
-subdir-$(coverage) += gcov
+subdir-$(CONFIG_GCOV) += gcov
 
 subdir-y += libelf
 subdir-$(CONFIG_HAS_DEVICE_TREE) += libfdt
-- 
2.1.4


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

Re: [PATCH 4/4] xen: add a gcov Kconfig option

[Jan Beulich]

>>> On 30.08.16 at 14:01, <wei.liu2@citrix.com> wrote:
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

Acked-by: Jan Beulich <jbeulich@suse.com>

with one adjustment (see below).

> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Tim Deegan <tim@xen.org>
> Cc: Doug Goldstein <cardoe@cardoe.com>
> 
> Would like to have it default to DEBUG, but we also need to check
> compiler to be gcc. I couldn't figure out how to check compiler to be
> gcc in Kconfig.

Actually I don't think defaulting to DEBUG would have been a good
idea. Why would you want everyone to carry that extra baggage
regardless of whether they actually mean to make use of it? And
anyway, conversion to Kconfig should retain current behavior, which
clearly was ...

> --- a/Config.mk
> +++ b/Config.mk
> @@ -20,9 +20,6 @@ or       = $(if $(strip $(1)),$(1),$(if $(strip 
> $(2)),$(2),$(if $(strip $(3)),$(
>  debug ?= y
>  debug_symbols ?= $(debug)
>  
> -# Test coverage support
> -coverage ?= n

... off by default.

> --- a/xen/Kconfig.debug
> +++ b/xen/Kconfig.debug
> @@ -13,6 +13,11 @@ config DEBUG
>  
>  if DEBUG || EXPERT = "y"
>  
> +config GCOV
> +       bool "Gcov Support"
> +       ---help---
> +         Enable gcov (a test coverage program in GCC) support.
> +
>  config CRASH_DEBUG
>  	bool "Crash Debugging Support"
>  	depends on X86

I now see that someone prior to you already managed to break the
alphabetical ordering (DEVICE_TREE_DEBUG got added to the end),
but please don't introduce further issues like this. The only exception
here are entries which have dependencies on others in this same
section (like PERF_ARRAYS depends on PERF_COUNTERS).

Jan


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

Re: [PATCH 4/4] xen: add a gcov Kconfig option

[Wei Liu]

On Tue, Aug 30, 2016 at 06:56:36AM -0600, Jan Beulich wrote:
> >>> On 30.08.16 at 14:01, <wei.liu2@citrix.com> wrote:
> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> 
> Acked-by: Jan Beulich <jbeulich@suse.com>
> 
> with one adjustment (see below).
> 
> > Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> > Cc: Stefano Stabellini <sstabellini@kernel.org>
> > Cc: Tim Deegan <tim@xen.org>
> > Cc: Doug Goldstein <cardoe@cardoe.com>
> > 
> > Would like to have it default to DEBUG, but we also need to check
> > compiler to be gcc. I couldn't figure out how to check compiler to be
> > gcc in Kconfig.
> 
> Actually I don't think defaulting to DEBUG would have been a good
> idea. Why would you want everyone to carry that extra baggage
> regardless of whether they actually mean to make use of it? And
> anyway, conversion to Kconfig should retain current behavior, which
> clearly was ...
> 
> > --- a/Config.mk
> > +++ b/Config.mk
> > @@ -20,9 +20,6 @@ or       = $(if $(strip $(1)),$(1),$(if $(strip 
> > $(2)),$(2),$(if $(strip $(3)),$(
> >  debug ?= y
> >  debug_symbols ?= $(debug)
> >  
> > -# Test coverage support
> > -coverage ?= n
> 
> ... off by default.
> 

OK. Makes sense to me.

> > --- a/xen/Kconfig.debug
> > +++ b/xen/Kconfig.debug
> > @@ -13,6 +13,11 @@ config DEBUG
> >  
> >  if DEBUG || EXPERT = "y"
> >  
> > +config GCOV
> > +       bool "Gcov Support"
> > +       ---help---
> > +         Enable gcov (a test coverage program in GCC) support.
> > +
> >  config CRASH_DEBUG
> >  	bool "Crash Debugging Support"
> >  	depends on X86
> 
> I now see that someone prior to you already managed to break the
> alphabetical ordering (DEVICE_TREE_DEBUG got added to the end),
> but please don't introduce further issues like this. The only exception
> here are entries which have dependencies on others in this same
> section (like PERF_ARRAYS depends on PERF_COUNTERS).
> 

OK. I will move this option to correct location.

Wei.

> Jan
> 

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

Re: [PATCH 4/4] xen: add a gcov Kconfig option

[Doug Goldstein]

> On Aug 30, 2016, at 8:06 AM, Wei Liu <wei.liu2@citrix.com> wrote:
> 
> On Tue, Aug 30, 2016 at 06:56:36AM -0600, Jan Beulich wrote:
>>>>> On 30.08.16 at 14:01, <wei.liu2@citrix.com> wrote:
>>> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
>> 
>> Acked-by: Jan Beulich <jbeulich@suse.com>
>> 
>> with one adjustment (see below).

Thanks for doing this Wei. I agree with Jan's reviews. You can toss my Reviewed-by: Doug Goldstein <cardoe@cardoe.com> on there if you'd like as well.

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