[PATCH for-4.8 0/2] Disable debug build for hypervisor

[PATCH for-4.8 0/2] Disable debug build for hypervisor

[Wei Liu]

I think this is new in this release. Please check if this is the correct
approach.

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>  

Wei Liu (2):
  xen: disable debug build
  Config.mk: fix comment for debug option

 Config.mk         | 3 ++-
 xen/Kconfig.debug | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

-- 
2.1.4


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

[PATCH for-4.8 1/2] xen: disable debug build

[Wei Liu]

Xen debug build is controlled by Kconfig.

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 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/Kconfig.debug b/xen/Kconfig.debug
index e9f7dcd..b3bb085 100644
--- a/xen/Kconfig.debug
+++ b/xen/Kconfig.debug
@@ -3,7 +3,7 @@ menu "Debugging Options"
 
 config DEBUG
 	bool "Developer Checks"
-	default y
+	default n
 	---help---
 	  If you say Y here this will enable developer checks such as asserts
 	  and extra printks. This option is intended for development purposes
-- 
2.1.4


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

[PATCH for-4.8 2/2] Config.mk: fix comment for debug 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: Wei Liu <wei.liu2@citrix.com>
---
 Config.mk | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Config.mk b/Config.mk
index ebbd9c0..fb836a4 100644
--- a/Config.mk
+++ b/Config.mk
@@ -16,7 +16,8 @@ or       = $(if $(strip $(1)),$(1),$(if $(strip $(2)),$(2),$(if $(strip $(3)),$(
 
 -include $(XEN_ROOT)/.config
 
-# A debug build of Xen and tools?
+# A debug build of tools?
+# Hypervisor debug build is controlled by Kconfig.
 debug ?= n
 debug_symbols ?= $(debug)
 
-- 
2.1.4


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

Re: [PATCH for-4.8 0/2] Disable debug build for hypervisor

[Andrew Cooper]

On 31/10/16 17:09, Wei Liu wrote:
> I think this is new in this release.

I believe so.

>  Please check if this is the correct
> approach.
>
> 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>  
>
> Wei Liu (2):
>   xen: disable debug build
>   Config.mk: fix comment for debug option

Both Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

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

Re: [PATCH for-4.8 0/2] Disable debug build for hypervisor

[Jan Beulich]

>>> On 31.10.16 at 18:11, <andrew.cooper3@citrix.com> wrote:
> On 31/10/16 17:09, Wei Liu wrote:
>> I think this is new in this release.
> 
> I believe so.

Perhaps an adjustment to the release check list is needed?

Jan

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

Re: [PATCH for-4.8 0/2] Disable debug build for hypervisor

[Konrad Rzeszutek Wilk]

On Mon, Oct 31, 2016 at 05:11:19PM +0000, Andrew Cooper wrote:
> On 31/10/16 17:09, Wei Liu wrote:
> > I think this is new in this release.
> 
> I believe so.
> 
> >  Please check if this is the correct
> > approach.
> >
> > 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>  
> >
> > Wei Liu (2):
> >   xen: disable debug build
> >   Config.mk: fix comment for debug option
> 
> Both Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

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

Re: [PATCH for-4.8 2/2] Config.mk: fix comment for debug option

[Wei Liu]

On Mon, Oct 31, 2016 at 05:09:46PM +0000, Wei Liu wrote:
> 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 | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/Config.mk b/Config.mk
> index ebbd9c0..fb836a4 100644
> --- a/Config.mk
> +++ b/Config.mk
> @@ -16,7 +16,8 @@ or       = $(if $(strip $(1)),$(1),$(if $(strip $(2)),$(2),$(if $(strip $(3)),$(
>  
>  -include $(XEN_ROOT)/.config
>  
> -# A debug build of Xen and tools?
> +# A debug build of tools?

For this to hold true, a patch like this is needed:

Please let me know what you think.

---8<---
From 0a96ff9f3610622bc4f7114d6e094bf45ca9305f Mon Sep 17 00:00:00 2001
From: Wei Liu <wei.liu2@citrix.com>
Date: Mon, 31 Oct 2016 17:42:25 +0000
Subject: [PATCH] build: make debug option affect tools only

The debug option in Config.mk affects hypervisor, tools and stubdom by
appending different flags to CFLAGS.

It is undesirable because now hypervisor build is affect by both Kconfig
and debug.

Disentangle the semantics of debug by pushing relevant options to
individual sub-systems.

For hypervisor, the flags previously added by debug option is now
controlled by CONFIG_DEBUG.

For tools, flags are moved from config/*.mk into tools/Rules.mk.

For stubdom, it is a bit special because it unilaterally sets debug all
the time, and it also inherits CFLAGS from the source package it tries
to build. It should be fine to not inherit any flags from Xen build
system because they will be overridden by source packages anyway.

Specifically there are some considerations on how the flags are picked:

1. we don't need -fno-optimize-sibling-calls anymore because gcc doc
   indicates that it is not enabled for -O1, which we already set in the
   debug build.
2. for tools we use -O0 -g3 in Rules.mk because they already take
   precedence over the flags set in config/*.mk.
3. for hypervisor we don't add -fno-omit-frame-pointer to debug build
   because that's controlled by CONFIG_FRAME_POINTER.

The debug option in Config.mk will only affect tools components after
this patch is applied.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 config/StdGNU.mk | 8 --------
 config/SunOS.mk  | 7 -------
 tools/Rules.mk   | 4 +++-
 xen/Rules.mk     | 6 ++++++
 4 files changed, 9 insertions(+), 16 deletions(-)

diff --git a/config/StdGNU.mk b/config/StdGNU.mk
index 39d36b2..6be8233 100644
--- a/config/StdGNU.mk
+++ b/config/StdGNU.mk
@@ -35,14 +35,6 @@ UTIL_LIBS = -lutil
 SONAME_LDFLAG = -soname
 SHLIB_LDFLAGS = -shared
 
-ifneq ($(debug),y)
-CFLAGS += -O2 -fomit-frame-pointer
-else
-# Less than -O1 produces bad code and large stack frames
-CFLAGS += -O1 -fno-omit-frame-pointer
-CFLAGS-$(gcc) += -fno-optimize-sibling-calls
-endif
-
 ifeq ($(lto),y)
 CFLAGS += -flto
 LDFLAGS-$(clang) += -plugin LLVMgold.so
diff --git a/config/SunOS.mk b/config/SunOS.mk
index 86a384d..0fe5f45 100644
--- a/config/SunOS.mk
+++ b/config/SunOS.mk
@@ -31,12 +31,5 @@ UTIL_LIBS =
 SONAME_LDFLAG = -h
 SHLIB_LDFLAGS = -R $(SunOS_LIBDIR) -shared
 
-ifneq ($(debug),y)
-CFLAGS += -O2 -fno-omit-frame-pointer
-else
-# Less than -O1 produces bad code and large stack frames
-CFLAGS += -O1 -fno-omit-frame-pointer
-endif
-
 CFLAGS += -Wa,--divide -D_POSIX_C_SOURCE=200112L -D__EXTENSIONS__
 
diff --git a/tools/Rules.mk b/tools/Rules.mk
index 5a80fec..0e73690 100644
--- a/tools/Rules.mk
+++ b/tools/Rules.mk
@@ -138,9 +138,11 @@ SHLIB_libxenvchan  = $(SHDEPS_libxenvchan) -Wl,-rpath-link=$(XEN_LIBVCHAN)
 
 ifeq ($(debug),y)
 # Disable optimizations and enable debugging information for macros
-CFLAGS += -O0 -g3
+CFLAGS += -O0 -g3 -fno-omit-frame-pointer
 # But allow an override to -O0 in case Python enforces -D_FORTIFY_SOURCE=<n>.
 PY_CFLAGS += $(PY_NOOPT_CFLAGS)
+else
+CFLAGS += -O2 -fomit-frame-pointer
 endif
 
 LIBXL_BLKTAP ?= $(CONFIG_BLKTAP2)
diff --git a/xen/Rules.mk b/xen/Rules.mk
index a9fda71..08cc776 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -46,6 +46,12 @@ ALL_OBJS-y               += $(BASEDIR)/xsm/built_in.o
 ALL_OBJS-y               += $(BASEDIR)/arch/$(TARGET_ARCH)/built_in.o
 ALL_OBJS-$(CONFIG_CRYPTO)   += $(BASEDIR)/crypto/built_in.o
 
+ifeq ($(CONFIG_DEBUG),y)
+CFLAGS += -O1
+else
+CFLAGS += -O2 -fomit-frame-pointer
+endif
+
 CFLAGS += -nostdinc -fno-builtin -fno-common
 CFLAGS += -Werror -Wredundant-decls -Wno-pointer-arith
 CFLAGS += -pipe -g -D__XEN__ -include $(BASEDIR)/include/xen/config.h
-- 
2.1.4


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

Re: [PATCH for-4.8 2/2] Config.mk: fix comment for debug option

[Andrew Cooper]

On 01/11/16 13:47, Wei Liu wrote:
> On Mon, Oct 31, 2016 at 05:09:46PM +0000, Wei Liu wrote:
>> 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 | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/Config.mk b/Config.mk
>> index ebbd9c0..fb836a4 100644
>> --- a/Config.mk
>> +++ b/Config.mk
>> @@ -16,7 +16,8 @@ or       = $(if $(strip $(1)),$(1),$(if $(strip $(2)),$(2),$(if $(strip $(3)),$(
>>  
>>  -include $(XEN_ROOT)/.config
>>  
>> -# A debug build of Xen and tools?
>> +# A debug build of tools?
> For this to hold true, a patch like this is needed:
>
> Please let me know what you think.

Looks like another swamp :s

>
> ---8<---
> From 0a96ff9f3610622bc4f7114d6e094bf45ca9305f Mon Sep 17 00:00:00 2001
> From: Wei Liu <wei.liu2@citrix.com>
> Date: Mon, 31 Oct 2016 17:42:25 +0000
> Subject: [PATCH] build: make debug option affect tools only
>
> The debug option in Config.mk affects hypervisor, tools and stubdom by
> appending different flags to CFLAGS.
>
> It is undesirable because now hypervisor build is affect by both Kconfig
> and debug.
>
> Disentangle the semantics of debug by pushing relevant options to
> individual sub-systems.
>
> For hypervisor, the flags previously added by debug option is now
> controlled by CONFIG_DEBUG.
>
> For tools, flags are moved from config/*.mk into tools/Rules.mk.
>
> For stubdom, it is a bit special because it unilaterally sets debug all
> the time, and it also inherits CFLAGS from the source package it tries
> to build. It should be fine to not inherit any flags from Xen build
> system because they will be overridden by source packages anyway.
>
> Specifically there are some considerations on how the flags are picked:
>
> 1. we don't need -fno-optimize-sibling-calls anymore because gcc doc
>    indicates that it is not enabled for -O1, which we already set in the
>    debug build.
> 2. for tools we use -O0 -g3 in Rules.mk because they already take
>    precedence over the flags set in config/*.mk.
> 3. for hypervisor we don't add -fno-omit-frame-pointer to debug build
>    because that's controlled by CONFIG_FRAME_POINTER.
>
> The debug option in Config.mk will only affect tools components after
> this patch is applied.
>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
>  config/StdGNU.mk | 8 --------
>  config/SunOS.mk  | 7 -------
>  tools/Rules.mk   | 4 +++-
>  xen/Rules.mk     | 6 ++++++
>  4 files changed, 9 insertions(+), 16 deletions(-)
>
> diff --git a/config/StdGNU.mk b/config/StdGNU.mk
> index 39d36b2..6be8233 100644
> --- a/config/StdGNU.mk
> +++ b/config/StdGNU.mk
> @@ -35,14 +35,6 @@ UTIL_LIBS = -lutil
>  SONAME_LDFLAG = -soname
>  SHLIB_LDFLAGS = -shared
>  
> -ifneq ($(debug),y)
> -CFLAGS += -O2 -fomit-frame-pointer
> -else
> -# Less than -O1 produces bad code and large stack frames
> -CFLAGS += -O1 -fno-omit-frame-pointer
> -CFLAGS-$(gcc) += -fno-optimize-sibling-calls
> -endif
> -
>  ifeq ($(lto),y)
>  CFLAGS += -flto
>  LDFLAGS-$(clang) += -plugin LLVMgold.so
> diff --git a/config/SunOS.mk b/config/SunOS.mk
> index 86a384d..0fe5f45 100644
> --- a/config/SunOS.mk
> +++ b/config/SunOS.mk
> @@ -31,12 +31,5 @@ UTIL_LIBS =
>  SONAME_LDFLAG = -h
>  SHLIB_LDFLAGS = -R $(SunOS_LIBDIR) -shared
>  
> -ifneq ($(debug),y)
> -CFLAGS += -O2 -fno-omit-frame-pointer
> -else
> -# Less than -O1 produces bad code and large stack frames
> -CFLAGS += -O1 -fno-omit-frame-pointer
> -endif
> -
>  CFLAGS += -Wa,--divide -D_POSIX_C_SOURCE=200112L -D__EXTENSIONS__
>  
> diff --git a/tools/Rules.mk b/tools/Rules.mk
> index 5a80fec..0e73690 100644
> --- a/tools/Rules.mk
> +++ b/tools/Rules.mk
> @@ -138,9 +138,11 @@ SHLIB_libxenvchan  = $(SHDEPS_libxenvchan) -Wl,-rpath-link=$(XEN_LIBVCHAN)
>  
>  ifeq ($(debug),y)
>  # Disable optimizations and enable debugging information for macros
> -CFLAGS += -O0 -g3
> +CFLAGS += -O0 -g3 -fno-omit-frame-pointer

Perhaps this a suggestion better left for a later patch, but the use of
-O0 is still bad here.

Debug builds should use -Og if available, and -O1 otherwise.  As
identified immediately below, a number of options are incompatible with -O0.

>  # But allow an override to -O0 in case Python enforces -D_FORTIFY_SOURCE=<n>.
>  PY_CFLAGS += $(PY_NOOPT_CFLAGS)
> +else
> +CFLAGS += -O2 -fomit-frame-pointer
>  endif
>  
>  LIBXL_BLKTAP ?= $(CONFIG_BLKTAP2)
> diff --git a/xen/Rules.mk b/xen/Rules.mk
> index a9fda71..08cc776 100644
> --- a/xen/Rules.mk
> +++ b/xen/Rules.mk
> @@ -46,6 +46,12 @@ ALL_OBJS-y               += $(BASEDIR)/xsm/built_in.o
>  ALL_OBJS-y               += $(BASEDIR)/arch/$(TARGET_ARCH)/built_in.o
>  ALL_OBJS-$(CONFIG_CRYPTO)   += $(BASEDIR)/crypto/built_in.o
>  
> +ifeq ($(CONFIG_DEBUG),y)
> +CFLAGS += -O1
> +else
> +CFLAGS += -O2 -fomit-frame-pointer

The frame pointer option should be omitted entirely.  A user should be
able to control debug and frame pointer entirely independently with Kconfig.

There have been two times where I have specifically needed a release
build with frame pointers to track down bugs.

~Andrew

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

Re: [PATCH for-4.8 2/2] Config.mk: fix comment for debug option

[Wei Liu]

On Tue, Nov 01, 2016 at 02:58:22PM +0000, Andrew Cooper wrote:
> On 01/11/16 13:47, Wei Liu wrote:
> > On Mon, Oct 31, 2016 at 05:09:46PM +0000, Wei Liu wrote:
> >> 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 | 3 ++-
> >>  1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/Config.mk b/Config.mk
> >> index ebbd9c0..fb836a4 100644
> >> --- a/Config.mk
> >> +++ b/Config.mk
> >> @@ -16,7 +16,8 @@ or       = $(if $(strip $(1)),$(1),$(if $(strip $(2)),$(2),$(if $(strip $(3)),$(
> >>  
> >>  -include $(XEN_ROOT)/.config
> >>  
> >> -# A debug build of Xen and tools?
> >> +# A debug build of tools?
> > For this to hold true, a patch like this is needed:
> >
> > Please let me know what you think.
> 
> Looks like another swamp :s
> 

Indeed. This is something we overlooked early in the release.

> >
> > ---8<---
> > From 0a96ff9f3610622bc4f7114d6e094bf45ca9305f Mon Sep 17 00:00:00 2001
> > From: Wei Liu <wei.liu2@citrix.com>
> > Date: Mon, 31 Oct 2016 17:42:25 +0000
> > Subject: [PATCH] build: make debug option affect tools only
> >
> > The debug option in Config.mk affects hypervisor, tools and stubdom by
> > appending different flags to CFLAGS.
> >
> > It is undesirable because now hypervisor build is affect by both Kconfig
> > and debug.
> >

^
Specifically because of this, I really want to fix this whole thing
properly. Otherwise it is going to be rather confusing to downstream.

> > Disentangle the semantics of debug by pushing relevant options to
> > individual sub-systems.
> >
> > For hypervisor, the flags previously added by debug option is now
> > controlled by CONFIG_DEBUG.
> >
> > For tools, flags are moved from config/*.mk into tools/Rules.mk.
> >
[...]
> > diff --git a/tools/Rules.mk b/tools/Rules.mk
> > index 5a80fec..0e73690 100644
> > --- a/tools/Rules.mk
> > +++ b/tools/Rules.mk
> > @@ -138,9 +138,11 @@ SHLIB_libxenvchan  = $(SHDEPS_libxenvchan) -Wl,-rpath-link=$(XEN_LIBVCHAN)
> >  
> >  ifeq ($(debug),y)
> >  # Disable optimizations and enable debugging information for macros
> > -CFLAGS += -O0 -g3
> > +CFLAGS += -O0 -g3 -fno-omit-frame-pointer
> 
> Perhaps this a suggestion better left for a later patch, but the use of
> -O0 is still bad here.
> 
> Debug builds should use -Og if available, and -O1 otherwise.  As
> identified immediately below, a number of options are incompatible with -O0.
> 
> >  # But allow an override to -O0 in case Python enforces -D_FORTIFY_SOURCE=<n>.
> >  PY_CFLAGS += $(PY_NOOPT_CFLAGS)
> > +else
> > +CFLAGS += -O2 -fomit-frame-pointer
> >  endif
> >  
> >  LIBXL_BLKTAP ?= $(CONFIG_BLKTAP2)
> > diff --git a/xen/Rules.mk b/xen/Rules.mk
> > index a9fda71..08cc776 100644
> > --- a/xen/Rules.mk
> > +++ b/xen/Rules.mk
> > @@ -46,6 +46,12 @@ ALL_OBJS-y               += $(BASEDIR)/xsm/built_in.o
> >  ALL_OBJS-y               += $(BASEDIR)/arch/$(TARGET_ARCH)/built_in.o
> >  ALL_OBJS-$(CONFIG_CRYPTO)   += $(BASEDIR)/crypto/built_in.o
> >  
> > +ifeq ($(CONFIG_DEBUG),y)
> > +CFLAGS += -O1
> > +else
> > +CFLAGS += -O2 -fomit-frame-pointer
> 
> The frame pointer option should be omitted entirely.  A user should be
> able to control debug and frame pointer entirely independently with Kconfig.
> 
> There have been two times where I have specifically needed a release
> build with frame pointers to track down bugs.
> 

I think both are fine suggestions.

I would like to (hopefully) not introduce noticeable changes of the
effect of all combined flags in this patch and adjust the flags in a
later patch.

Wei.

> ~Andrew

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

Re: [PATCH for-4.8 2/2] Config.mk: fix comment for debug option

[Jan Beulich]

>>> Wei Liu <wei.liu2@citrix.com> 11/01/16 2:48 PM >>>
>config/StdGNU.mk | 8 --------
>config/SunOS.mk  | 7 -------
>tools/Rules.mk   | 4 +++-
>xen/Rules.mk     | 6 ++++++
>4 files changed, 9 insertions(+), 16 deletions(-)

Considering this diffstat - did the original global settings not get inherited
by any of the other subtrees, namely stubdom/ and/or extras/ ?

Jan

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

Re: [PATCH for-4.8 2/2] Config.mk: fix comment for debug option

[Wei Liu]

On Tue, Nov 01, 2016 at 10:18:46AM -0600, Jan Beulich wrote:
> >>> Wei Liu <wei.liu2@citrix.com> 11/01/16 2:48 PM >>>
> >config/StdGNU.mk | 8 --------
> >config/SunOS.mk  | 7 -------
> >tools/Rules.mk   | 4 +++-
> >xen/Rules.mk     | 6 ++++++
> >4 files changed, 9 insertions(+), 16 deletions(-)
> 
> Considering this diffstat - did the original global settings not get inherited
> by any of the other subtrees, namely stubdom/ and/or extras/ ?
> 

I mentioned this in commit message and deliberately made the decision to
not inherit these flags for stubdom build.

Wei.

> Jan
> 

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

Re: [PATCH for-4.8 2/2] Config.mk: fix comment for debug option

[Jan Beulich]

>>> Wei Liu <wei.liu2@citrix.com> 11/01/16 5:20 PM >>>
>On Tue, Nov 01, 2016 at 10:18:46AM -0600, Jan Beulich wrote:
>> >>> Wei Liu <wei.liu2@citrix.com> 11/01/16 2:48 PM >>>
>> >config/StdGNU.mk | 8 --------
>> >config/SunOS.mk  | 7 -------
>> >tools/Rules.mk   | 4 +++-
>> >xen/Rules.mk     | 6 ++++++
>> >4 files changed, 9 insertions(+), 16 deletions(-)
>> 
>> Considering this diffstat - did the original global settings not get inherited
>> by any of the other subtrees, namely stubdom/ and/or extras/ ?
> 
>I mentioned this in commit message and deliberately made the decision to
>not inherit these flags for stubdom build.

Indeed, I've overlooked that part, but I'm not fully convinced. And then
extras/ and possible other subtrees don't get mentioned (at the very
least I'd expect a sentence clarifying that no other subtrees are
affected, but as said at least for extras/ I'm not sure: That's where
mini-os gets linked in, and I don't recall whether its build machinery
got fully separated).

Jan

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

Re: [PATCH for-4.8 2/2] Config.mk: fix comment for debug option

[Wei Liu]

On Tue, Nov 01, 2016 at 10:33:54AM -0600, Jan Beulich wrote:
> >>> Wei Liu <wei.liu2@citrix.com> 11/01/16 5:20 PM >>>
> >On Tue, Nov 01, 2016 at 10:18:46AM -0600, Jan Beulich wrote:
> >> >>> Wei Liu <wei.liu2@citrix.com> 11/01/16 2:48 PM >>>
> >> >config/StdGNU.mk | 8 --------
> >> >config/SunOS.mk  | 7 -------
> >> >tools/Rules.mk   | 4 +++-
> >> >xen/Rules.mk     | 6 ++++++
> >> >4 files changed, 9 insertions(+), 16 deletions(-)
> >> 
> >> Considering this diffstat - did the original global settings not get inherited
> >> by any of the other subtrees, namely stubdom/ and/or extras/ ?
> > 
> >I mentioned this in commit message and deliberately made the decision to
> >not inherit these flags for stubdom build.
> 
> Indeed, I've overlooked that part, but I'm not fully convinced. And then

Not fully convinced because? Can you be more specific?

> extras/ and possible other subtrees don't get mentioned (at the very
> least I'd expect a sentence clarifying that no other subtrees are
> affected, but as said at least for extras/ I'm not sure: That's where
> mini-os gets linked in, and I don't recall whether its build machinery
> got fully separated).
> 

Mini-os got its own build system after I separated it out from Xen.git.
It should be dealt with separately if necessary.  I can mention that in
commit message as well.

Wei.

> Jan
> 

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

Re: [PATCH for-4.8 2/2] Config.mk: fix comment for debug option

[Jan Beulich]

>>> Wei Liu <wei.liu2@citrix.com> 11/01/16 5:38 PM >>>
>On Tue, Nov 01, 2016 at 10:33:54AM -0600, Jan Beulich wrote:
>> >>> Wei Liu <wei.liu2@citrix.com> 11/01/16 5:20 PM >>>
>> >On Tue, Nov 01, 2016 at 10:18:46AM -0600, Jan Beulich wrote:
>> >> >>> Wei Liu <wei.liu2@citrix.com> 11/01/16 2:48 PM >>>
>> >> >config/StdGNU.mk | 8 --------
>> >> >config/SunOS.mk  | 7 -------
>> >> >tools/Rules.mk   | 4 +++-
>> >> >xen/Rules.mk     | 6 ++++++
>> >> >4 files changed, 9 insertions(+), 16 deletions(-)
>> >> 
>> >> Considering this diffstat - did the original global settings not get inherited
>> >> by any of the other subtrees, namely stubdom/ and/or extras/ ?
>> > 
>> >I mentioned this in commit message and deliberately made the decision to
>> >not inherit these flags for stubdom build.
>> 
>> Indeed, I've overlooked that part, but I'm not fully convinced. And then
>
>Not fully convinced because? Can you be more specific?

I don't know very much about the stubdom build machinery, but I could
easily imaging some top level setting to have made it through to some
of the sub-components, in which case this change would have a
functional impact.

Jan


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

Re: [PATCH for-4.8 2/2] Config.mk: fix comment for debug option

[Wei Liu]

On Tue, Nov 01, 2016 at 10:42:37AM -0600, Jan Beulich wrote:
> >>> Wei Liu <wei.liu2@citrix.com> 11/01/16 5:38 PM >>>
> >On Tue, Nov 01, 2016 at 10:33:54AM -0600, Jan Beulich wrote:
> >> >>> Wei Liu <wei.liu2@citrix.com> 11/01/16 5:20 PM >>>
> >> >On Tue, Nov 01, 2016 at 10:18:46AM -0600, Jan Beulich wrote:
> >> >> >>> Wei Liu <wei.liu2@citrix.com> 11/01/16 2:48 PM >>>
> >> >> >config/StdGNU.mk | 8 --------
> >> >> >config/SunOS.mk  | 7 -------
> >> >> >tools/Rules.mk   | 4 +++-
> >> >> >xen/Rules.mk     | 6 ++++++
> >> >> >4 files changed, 9 insertions(+), 16 deletions(-)
> >> >> 
> >> >> Considering this diffstat - did the original global settings not get inherited
> >> >> by any of the other subtrees, namely stubdom/ and/or extras/ ?
> >> > 
> >> >I mentioned this in commit message and deliberately made the decision to
> >> >not inherit these flags for stubdom build.
> >> 
> >> Indeed, I've overlooked that part, but I'm not fully convinced. And then
> >
> >Not fully convinced because? Can you be more specific?
> 
> I don't know very much about the stubdom build machinery, but I could
> easily imaging some top level setting to have made it through to some
> of the sub-components, in which case this change would have a
> functional impact.
> 

I don't feel strongly about this. I can move some flags into stubdom
build system as well, just to err on the safe side.

Wei.

> Jan
> 

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

Re: [PATCH for-4.8 2/2] Config.mk: fix comment for debug option

[Ian Jackson]

Wei Liu writes ("Re: [PATCH for-4.8 2/2] Config.mk: fix comment for debug option"):
> On Mon, Oct 31, 2016 at 05:09:46PM +0000, Wei Liu wrote:
> > -# A debug build of Xen and tools?
> > +# A debug build of tools?
> 
> For this to hold true, a patch like this is needed:
> 
> Please let me know what you think.
...
> For hypervisor, the flags previously added by debug option is now
> controlled by CONFIG_DEBUG.

So there will no longer be one place where debug is enabled.  That's a
shame.  It would need to come with an osstest patch...

Ian.

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

Re: [PATCH for-4.8 2/2] Config.mk: fix comment for debug option

[Andrew Cooper]

On 01/11/16 16:56, Ian Jackson wrote:
> Wei Liu writes ("Re: [PATCH for-4.8 2/2] Config.mk: fix comment for debug option"):
>> On Mon, Oct 31, 2016 at 05:09:46PM +0000, Wei Liu wrote:
>>> -# A debug build of Xen and tools?
>>> +# A debug build of tools?
>> For this to hold true, a patch like this is needed:
>>
>> Please let me know what you think.
> ...
>> For hypervisor, the flags previously added by debug option is now
>> controlled by CONFIG_DEBUG.
> So there will no longer be one place where debug is enabled.  That's a
> shame.  It would need to come with an osstest patch...

It hasn't been the case for almost all of 4.8

Attempting to build Xen with an explicit debug= set yields "You must use
'make menuconfig' to enable/disable debug now."

I personally would prefer if debug= still worked, but apparently that is
prohibitively difficult to do.

~Andrew

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

[OSSTEST PATCH] ts-xen-build: set CONFIG_DEBUG for KConfig

[Wei Liu]

Starting from Xen 4.8 the hypervisor debug build is controlled by
Kconfig. Set it correctly in xen/.config.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 ts-xen-build | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ts-xen-build b/ts-xen-build
index 07a69ec..3e53d74 100755
--- a/ts-xen-build
+++ b/ts-xen-build
@@ -86,6 +86,7 @@ END
 	if test -f xen/Kconfig; then
 		echo >>xen/.config CONFIG_XSM='${build_xsm}'
 		echo >>xen/.config CONFIG_FLASK='${build_xsm}'
+		echo >>xen/.config CONFIG_DEBUG=$debug_build
 	fi
 	echo >>.config XSM_ENABLE='${build_xsm}'
 END
-- 
2.1.4


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

Re: [OSSTEST PATCH] ts-xen-build: set CONFIG_DEBUG for KConfig

[Ian Jackson]

Wei Liu writes ("[OSSTEST PATCH] ts-xen-build: set CONFIG_DEBUG for KConfig"):
> Starting from Xen 4.8 the hypervisor debug build is controlled by
> Kconfig. Set it correctly in xen/.config.
> 
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

Thanks.  Pushed to osstest pretest.

Ian.

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