[PATCH v2 0/7] Kconfig debug options

[PATCH v2 0/7] Kconfig debug options

[Doug Goldstein]

This converts the debug options from xen/Rules.mk to Kconfig. I'm unsure
if I properly described PERF_ARRAYS but otherwise the other descriptions
have either been provided by maintainers or improved by maintainers so
I am confident about those.

The big departure from Rules.mk is how NDEBUG is turned on (or isn't).
Basically if you enable the debug menu at all it will not turn on NDEBUG.
Previously this was only done when you supplied 'debug=n'. The inverse,
'debug=y' did 'verbose=y' and 'frame_pointer=y' so they were linked but
differently.

Doug Goldstein (7):
  build: add debug menu to Kconfig
  build: convert crash_debug to Kconfig
  build: convert verbose to Kconfig
  build: convert frame_pointer to Kconfig
  build: wire up pre-existing debug build flag
  build: convert perfc{,_arrays} to Kconfig
  build: convert lock_profile to Kconfig

 INSTALL                           |  6 ----
 docs/misc/crashdb.txt             |  4 +--
 xen/Kconfig                       |  2 ++
 xen/Kconfig.debug                 | 61 +++++++++++++++++++++++++++++++++++++++
 xen/Makefile                      |  1 +
 xen/Rules.mk                      | 28 +++---------------
 xen/arch/arm/kernel.c             |  2 +-
 xen/arch/arm/xen.lds.S            |  2 +-
 xen/arch/x86/Makefile             |  3 +-
 xen/arch/x86/domain.c             |  2 +-
 xen/arch/x86/domain_build.c       |  2 +-
 xen/arch/x86/hvm/hvm.c            |  2 +-
 xen/arch/x86/time.c               |  4 +--
 xen/arch/x86/x86_64/Makefile      |  2 +-
 xen/arch/x86/x86_64/asm-offsets.c |  2 +-
 xen/arch/x86/xen.lds.S            |  2 +-
 xen/common/Makefile               |  4 +--
 xen/common/keyhandler.c           |  4 +--
 xen/common/perfc.c                |  2 +-
 xen/common/spinlock.c             | 10 +++----
 xen/common/sysctl.c               |  4 +--
 xen/include/asm-x86/asm_defns.h   |  2 +-
 xen/include/asm-x86/debugger.h    |  2 +-
 xen/include/asm-x86/domain.h      |  2 +-
 xen/include/xen/config.h          |  4 +++
 xen/include/xen/gdbstub.h         |  2 +-
 xen/include/xen/perfc.h           |  8 ++---
 xen/include/xen/sched.h           |  2 +-
 xen/include/xen/spinlock.h        |  4 +--
 xen/include/xsm/dummy.h           |  2 +-
 30 files changed, 109 insertions(+), 68 deletions(-)
 create mode 100644 xen/Kconfig.debug

-- 
2.7.3


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

[PATCH v2 1/7] build: add debug menu to Kconfig

[Doug Goldstein]

There are a number of debugging options for Xen so the idea is to have a
menu to group them all together. Enabling this menu item will also
disable NDEBUG which will result in more debug prints. This was
previously wired into the 'debug=y' command line option.

Signed-off-by: Doug Goldstein <cardoe@cardoe.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              | 2 ++
 xen/Kconfig.debug        | 7 +++++++
 xen/include/xen/config.h | 4 ++++
 3 files changed, 13 insertions(+)
 create mode 100644 xen/Kconfig.debug

diff --git a/xen/Kconfig b/xen/Kconfig
index fa8b27c..0fe7a1a 100644
--- a/xen/Kconfig
+++ b/xen/Kconfig
@@ -26,3 +26,5 @@ config DEFCONFIG_LIST
 config EXPERT
 	string
 	option env="XEN_CONFIG_EXPERT"
+
+source "Kconfig.debug"
diff --git a/xen/Kconfig.debug b/xen/Kconfig.debug
new file mode 100644
index 0000000..e5179f4
--- /dev/null
+++ b/xen/Kconfig.debug
@@ -0,0 +1,7 @@
+
+menuconfig DEBUG
+	bool "Debugging Options"
+	---help---
+	  If you want to debug Xen say Y and select any additional debugging
+	  support options. Enabling this option is intended for development
+	  purposes only, and not for production use.
diff --git a/xen/include/xen/config.h b/xen/include/xen/config.h
index ef6e5ee..473c5e8 100644
--- a/xen/include/xen/config.h
+++ b/xen/include/xen/config.h
@@ -81,4 +81,8 @@
 /* allow existing code to work with Kconfig variable */
 #define NR_CPUS CONFIG_NR_CPUS
 
+#ifndef CONFIG_DEBUG
+#define NDEBUG
+#endif
+
 #endif /* __XEN_CONFIG_H__ */
-- 
2.7.3


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

[PATCH v2 2/7] build: convert crash_debug to Kconfig

[Doug Goldstein]

Convert the crash_debug option to Kconfig as CONFIG_CRASH_DEBUG. This
was previously togglable on the command line so this adds a message for
users enabling it from the command line to tell them to enable it from
make menuconfig.

Signed-off-by: Doug Goldstein <cardoe@cardoe.com>
---
CC: Jan Beulich <jbeulich@suse.com>
CC: Andrew Cooper <andrew.cooper3@citrix.com>
---
 INSTALL                        |  1 -
 docs/misc/crashdb.txt          |  4 ++--
 xen/Kconfig.debug              | 11 +++++++++++
 xen/Rules.mk                   |  5 +++--
 xen/arch/x86/Makefile          |  3 +--
 xen/arch/x86/x86_64/Makefile   |  2 +-
 xen/common/Makefile            |  2 +-
 xen/include/asm-x86/debugger.h |  2 +-
 xen/include/xen/gdbstub.h      |  2 +-
 9 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/INSTALL b/INSTALL
index 95fa94d..2974b9b 100644
--- a/INSTALL
+++ b/INSTALL
@@ -231,7 +231,6 @@ verbose=y
 perfc=y
 perfc_arrays=y
 lock_profile=y
-crash_debug=y
 frame_pointer=y
 lto=y
 
diff --git a/docs/misc/crashdb.txt b/docs/misc/crashdb.txt
index b41a538..9733666 100644
--- a/docs/misc/crashdb.txt
+++ b/docs/misc/crashdb.txt
@@ -5,7 +5,7 @@ Xen has a simple gdb stub for doing post-mortem debugging i.e. once
 you've crashed it, you get to poke around and find out why.  There's
 also a special key handler for making it crash, which is handy.
 
-You need to have crash_debug=y set when compiling , and you also need
+You need to have CRASH_DEBUG=y set when compiling, and you also need
 to enable it on the Xen command line, eg by gdb=com1.
 
 If you need to have a serial port shared between gdb and the console,
@@ -19,7 +19,7 @@ if you have a simple null modem connection between the test box and
 the workstation, and aren't using a H/L split console:
 
   * Set debug=y in Config.mk
-  * Set crash_debug=y in xen/Rules.mk
+  * Set CRASH_DEBUG=y with `make -C xen menuconfig`
   * Make the changes in the attached patch, and build.
   * Arrange to pass gdb=com1 as a hypervisor command line argument
     (I already have com1=38400,8n1 console=com1,vga sync_console)
diff --git a/xen/Kconfig.debug b/xen/Kconfig.debug
index e5179f4..04f672f 100644
--- a/xen/Kconfig.debug
+++ b/xen/Kconfig.debug
@@ -5,3 +5,14 @@ menuconfig DEBUG
 	  If you want to debug Xen say Y and select any additional debugging
 	  support options. Enabling this option is intended for development
 	  purposes only, and not for production use.
+
+if DEBUG
+
+config CRASH_DEBUG
+	bool "Crash Debugging Support"
+	depends on X86
+	---help---
+	  If you want to be able to attach gdb to Xen to be able to debug
+	  Xen if it crashes then say Y.
+
+endif # DEBUG
diff --git a/xen/Rules.mk b/xen/Rules.mk
index 961d533..c044fd1 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -7,7 +7,6 @@ verbose       ?= n
 perfc         ?= n
 perfc_arrays  ?= n
 lock_profile  ?= n
-crash_debug   ?= n
 frame_pointer ?= n
 lto           ?= n
 
@@ -30,6 +29,9 @@ endif
 ifneq ($(origin kexec),undefined)
 $(error "You must use 'make menuconfig' to enable/disable kexec now.")
 endif
+ifneq ($(origin crash_debug),undefined)
+$(error "You must use 'make menuconfig' to enable/disable crash_debug now.")
+endif
 
 # Set ARCH/SUBARCH appropriately.
 override TARGET_SUBARCH  := $(XEN_TARGET_ARCH)
@@ -58,7 +60,6 @@ CFLAGS += -Wa,--strip-local-absolute
 endif
 
 CFLAGS-$(verbose)       += -DVERBOSE
-CFLAGS-$(crash_debug)   += -DCRASH_DEBUG
 CFLAGS-$(perfc)         += -DPERF_COUNTERS
 CFLAGS-$(perfc_arrays)  += -DPERF_ARRAYS
 CFLAGS-$(lock_profile)  += -DLOCK_PROFILE
diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index 4665a68..4ccef4a 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -27,6 +27,7 @@ obj-y += domain_page.o
 obj-y += e820.o
 obj-y += extable.o
 obj-y += flushtlb.o
+obj-$(CONFIG_CRASH_DEBUG) += gdbstub.o
 obj-y += i387.o
 obj-y += i8259.o
 obj-y += io_apic.o
@@ -66,8 +67,6 @@ obj-y += vm_event.o
 obj-$(CONFIG_XSPLICE) += alternative.o xsplice.o
 obj-y += xstate.o
 
-obj-$(crash_debug) += gdbstub.o
-
 x86_emulate.o: x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h
 
 efi-y := $(shell if [ ! -r $(BASEDIR)/include/xen/compile.h -o \
diff --git a/xen/arch/x86/x86_64/Makefile b/xen/arch/x86/x86_64/Makefile
index 5b54c16..d8815e7 100644
--- a/xen/arch/x86/x86_64/Makefile
+++ b/xen/arch/x86/x86_64/Makefile
@@ -14,4 +14,4 @@ obj-y += cpu_idle.o
 obj-y += cpufreq.o
 obj-bin-$(CONFIG_KEXEC) += kexec_reloc.o
 
-obj-$(crash_debug)   += gdbstub.o
+obj-$(CONFIG_CRASH_DEBUG)   += gdbstub.o
diff --git a/xen/common/Makefile b/xen/common/Makefile
index afd84b6..a98bcc2 100644
--- a/xen/common/Makefile
+++ b/xen/common/Makefile
@@ -8,6 +8,7 @@ obj-y += domain.o
 obj-y += event_2l.o
 obj-y += event_channel.o
 obj-y += event_fifo.o
+obj-$(CONFIG_CRASH_DEBUG) += gdbstub.o
 obj-y += grant_table.o
 obj-y += guestcopy.o
 obj-bin-y += gunzip.init.o
@@ -64,7 +65,6 @@ obj-$(CONFIG_XSPLICE) += xsplice_elf.o
 obj-bin-$(CONFIG_X86) += $(foreach n,decompress bunzip2 unxz unlzma unlzo unlz4 earlycpio,$(n).init.o)
 
 obj-$(perfc)       += perfc.o
-obj-$(crash_debug) += gdbstub.o
 
 obj-$(CONFIG_COMPAT) += $(addprefix compat/,domain.o kernel.o memory.o multicall.o xlat.o)
 
diff --git a/xen/include/asm-x86/debugger.h b/xen/include/asm-x86/debugger.h
index 33f4700..fb92ee4 100644
--- a/xen/include/asm-x86/debugger.h
+++ b/xen/include/asm-x86/debugger.h
@@ -39,7 +39,7 @@
 #define DEBUGGER_trap_fatal(_v, _r) \
     if ( debugger_trap_fatal(_v, _r) ) return;
 
-#if defined(CRASH_DEBUG)
+#if defined(CONFIG_CRASH_DEBUG)
 
 #include <xen/gdbstub.h>
 
diff --git a/xen/include/xen/gdbstub.h b/xen/include/xen/gdbstub.h
index ab710da..a5e6714 100644
--- a/xen/include/xen/gdbstub.h
+++ b/xen/include/xen/gdbstub.h
@@ -23,7 +23,7 @@
 #include <asm/atomic.h>
 #include <asm/page.h>
 
-#ifdef CRASH_DEBUG
+#ifdef CONFIG_CRASH_DEBUG
 
 struct gdb_context {
     int                 serhnd;           /* handle on our serial line */
-- 
2.7.3


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

[PATCH v2 3/7] build: convert verbose to Kconfig

[Doug Goldstein]

Convert 'verbose', which was enabled by 'debug=y' to Kconfig as
CONFIG_VERBOSE_DEBUG which is enabled by default when CONFIG_DEBUG is
enabled.

Signed-off-by: Doug Goldstein <cardoe@cardoe.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>
CC: Daniel De Graaf <dgdegra@tycho.nsa.gov>
---
 INSTALL                     | 1 -
 xen/Kconfig.debug           | 7 +++++++
 xen/Rules.mk                | 5 -----
 xen/arch/arm/kernel.c       | 2 +-
 xen/arch/x86/domain_build.c | 2 +-
 xen/include/xsm/dummy.h     | 2 +-
 6 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/INSTALL b/INSTALL
index 2974b9b..35668bd 100644
--- a/INSTALL
+++ b/INSTALL
@@ -227,7 +227,6 @@ VGABIOS_REL_DATE="dd Mon yyyy"
 
 The following variables can be used to tweak some aspects of the
 hypervisor build.
-verbose=y
 perfc=y
 perfc_arrays=y
 lock_profile=y
diff --git a/xen/Kconfig.debug b/xen/Kconfig.debug
index 04f672f..89e26fe 100644
--- a/xen/Kconfig.debug
+++ b/xen/Kconfig.debug
@@ -15,4 +15,11 @@ config CRASH_DEBUG
 	  If you want to be able to attach gdb to Xen to be able to debug
 	  Xen if it crashes then say Y.
 
+config VERBOSE_DEBUG
+	bool "Verbose debug messages"
+	default y
+	---help---
+	  Guest output from HYPERVISOR_console_io and hypervisor parsing
+	  ELF images (dom0) is logged in the Xen ring buffer.
+
 endif # DEBUG
diff --git a/xen/Rules.mk b/xen/Rules.mk
index c044fd1..b159451 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -3,7 +3,6 @@
 # If you change any of these configuration options then you must
 # 'make clean' before rebuilding.
 #
-verbose       ?= n
 perfc         ?= n
 perfc_arrays  ?= n
 lock_profile  ?= n
@@ -17,10 +16,7 @@ include $(XEN_ROOT)/Config.mk
 # Hardcoded configuration implications and dependencies.
 # Do this is a neater way if it becomes unwieldy.
 ifeq ($(debug),y)
-verbose       := y
 frame_pointer := y
-else
-CFLAGS += -DNDEBUG
 endif
 ifeq ($(perfc_arrays),y)
 perfc := y
@@ -59,7 +55,6 @@ ifneq ($(clang),y)
 CFLAGS += -Wa,--strip-local-absolute
 endif
 
-CFLAGS-$(verbose)       += -DVERBOSE
 CFLAGS-$(perfc)         += -DPERF_COUNTERS
 CFLAGS-$(perfc_arrays)  += -DPERF_ARRAYS
 CFLAGS-$(lock_profile)  += -DLOCK_PROFILE
diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
index 9871bd9..3f6cce3 100644
--- a/xen/arch/arm/kernel.c
+++ b/xen/arch/arm/kernel.c
@@ -472,7 +472,7 @@ static int kernel_elf_probe(struct kernel_info *info,
 
     if ( (rc = elf_init(&info->elf.elf, info->elf.kernel_img, size )) != 0 )
         goto err;
-#ifdef VERBOSE
+#ifdef CONFIG_VERBOSE_DEBUG
     elf_set_verbose(&info->elf.elf);
 #endif
     elf_parse_binary(&info->elf.elf);
diff --git a/xen/arch/x86/domain_build.c b/xen/arch/x86/domain_build.c
index f9a3eca..b29c377 100644
--- a/xen/arch/x86/domain_build.c
+++ b/xen/arch/x86/domain_build.c
@@ -942,7 +942,7 @@ int __init construct_dom0(
 
     if ( (rc = elf_init(&elf, image_start, image_len)) != 0 )
         return rc;
-#ifdef VERBOSE
+#ifdef CONFIG_VERBOSE_DEBUG
     elf_set_verbose(&elf);
 #endif
     elf_parse_binary(&elf);
diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h
index abbe282..406cd18 100644
--- a/xen/include/xsm/dummy.h
+++ b/xen/include/xsm/dummy.h
@@ -215,7 +215,7 @@ static XSM_INLINE int xsm_memory_stat_reservation(XSM_DEFAULT_ARG struct domain
 static XSM_INLINE int xsm_console_io(XSM_DEFAULT_ARG struct domain *d, int cmd)
 {
     XSM_ASSERT_ACTION(XSM_OTHER);
-#ifdef VERBOSE
+#ifdef CONFIG_VERBOSE_DEBUG
     if ( cmd == CONSOLEIO_write )
         return xsm_default_action(XSM_HOOK, d, NULL);
 #endif
-- 
2.7.3


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

[PATCH v2 4/7] build: convert frame_pointer to Kconfig

[Doug Goldstein]

Converts the frame_pointer option to a Kconfig option.

Signed-off-by: Doug Goldstein <cardoe@cardoe.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>
---
 INSTALL           | 1 -
 xen/Kconfig.debug | 8 ++++++++
 xen/Rules.mk      | 6 +-----
 3 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/INSTALL b/INSTALL
index 35668bd..f55d42c 100644
--- a/INSTALL
+++ b/INSTALL
@@ -230,7 +230,6 @@ hypervisor build.
 perfc=y
 perfc_arrays=y
 lock_profile=y
-frame_pointer=y
 lto=y
 
 During tools build external repos will be cloned into the source tree.
diff --git a/xen/Kconfig.debug b/xen/Kconfig.debug
index 89e26fe..c75cf1d 100644
--- a/xen/Kconfig.debug
+++ b/xen/Kconfig.debug
@@ -15,6 +15,14 @@ config CRASH_DEBUG
 	  If you want to be able to attach gdb to Xen to be able to debug
 	  Xen if it crashes then say Y.
 
+config FRAME_POINTER
+	bool "Compile Xen with frame pointers"
+	default y
+	---help---
+	  If you say Y here the resulting Xen will be slightly larger and
+	  maybe slower, but it gives very useful debugging information
+	  in case of any Xen bugs.
+
 config VERBOSE_DEBUG
 	bool "Verbose debug messages"
 	default y
diff --git a/xen/Rules.mk b/xen/Rules.mk
index b159451..84b9d81 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -6,7 +6,6 @@
 perfc         ?= n
 perfc_arrays  ?= n
 lock_profile  ?= n
-frame_pointer ?= n
 lto           ?= n
 
 -include $(BASEDIR)/include/config/auto.conf
@@ -15,9 +14,6 @@ include $(XEN_ROOT)/Config.mk
 
 # Hardcoded configuration implications and dependencies.
 # Do this is a neater way if it becomes unwieldy.
-ifeq ($(debug),y)
-frame_pointer := y
-endif
 ifeq ($(perfc_arrays),y)
 perfc := y
 endif
@@ -58,7 +54,7 @@ endif
 CFLAGS-$(perfc)         += -DPERF_COUNTERS
 CFLAGS-$(perfc_arrays)  += -DPERF_ARRAYS
 CFLAGS-$(lock_profile)  += -DLOCK_PROFILE
-CFLAGS-$(frame_pointer) += -fno-omit-frame-pointer -DCONFIG_FRAME_POINTER
+CFLAGS-$(CONFIG_FRAME_POINTER) += -fno-omit-frame-pointer
 
 ifneq ($(max_phys_irqs),)
 CFLAGS-y                += -DMAX_PHYS_IRQS=$(max_phys_irqs)
-- 
2.7.3


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

[PATCH v2 5/7] build: wire up pre-existing debug build flag

[Doug Goldstein]

This allows 'make debug=n' and 'make debug=y' work as it did previously
but only in the case of the user not having an existing .config file
from a 'make menuconfig'. This is because the command line 'debug' flag
can only be used to set the default value and if the user has already
built up a config with their real preference set.

Signed-off-by: Doug Goldstein <cardoe@cardoe.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 | 5 +++++
 xen/Makefile      | 1 +
 2 files changed, 6 insertions(+)

diff --git a/xen/Kconfig.debug b/xen/Kconfig.debug
index c75cf1d..9aefa16 100644
--- a/xen/Kconfig.debug
+++ b/xen/Kconfig.debug
@@ -1,6 +1,11 @@
+config DEBUG_ENV
+	bool
+	option env="debug"
 
 menuconfig DEBUG
 	bool "Debugging Options"
+	default y if DEBUG_ENV = "y"
+	default n
 	---help---
 	  If you want to debug Xen say Y and select any additional debugging
 	  support options. Enabling this option is intended for development
diff --git a/xen/Makefile b/xen/Makefile
index 0d5f240..31cca71 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -27,6 +27,7 @@ SRCARCH=$(shell echo $(ARCH) | sed -e 's/x86.*/x86/' -e s'/arm\(32\|64\)/arm/g')
 # Don't break if the build process wasn't called from the top level
 # we need XEN_TARGET_ARCH to generate the proper config
 include $(XEN_ROOT)/Config.mk
+export debug
 
 # Allow someone to change their config file
 export KCONFIG_CONFIG ?= .config
-- 
2.7.3


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

[PATCH v2 6/7] build: convert perfc{, _arrays} to Kconfig

[Doug Goldstein]

Convert the 'perfc' and 'perfc_arrays' options to Kconfig as
CONFIG_PERF_COUNTERS and CONFIG_PERF_ARRAYS to minimize code changes.

Signed-off-by: Doug Goldstein <cardoe@cardoe.com>
---
CC: Jan Beulich <jbeulich@suse.com>
CC: Andrew Cooper <andrew.cooper3@citrix.com>
---
 INSTALL                           |  2 --
 xen/Kconfig.debug                 | 15 +++++++++++++++
 xen/Rules.mk                      | 10 ----------
 xen/arch/x86/hvm/hvm.c            |  2 +-
 xen/arch/x86/time.c               |  4 ++--
 xen/arch/x86/x86_64/asm-offsets.c |  2 +-
 xen/common/Makefile               |  2 +-
 xen/common/keyhandler.c           |  2 +-
 xen/common/perfc.c                |  2 +-
 xen/common/sysctl.c               |  2 +-
 xen/include/asm-x86/asm_defns.h   |  2 +-
 xen/include/asm-x86/domain.h      |  2 +-
 xen/include/xen/perfc.h           |  8 ++++----
 xen/include/xen/sched.h           |  2 +-
 14 files changed, 30 insertions(+), 27 deletions(-)

diff --git a/INSTALL b/INSTALL
index f55d42c..623887d 100644
--- a/INSTALL
+++ b/INSTALL
@@ -227,8 +227,6 @@ VGABIOS_REL_DATE="dd Mon yyyy"
 
 The following variables can be used to tweak some aspects of the
 hypervisor build.
-perfc=y
-perfc_arrays=y
 lock_profile=y
 lto=y
 
diff --git a/xen/Kconfig.debug b/xen/Kconfig.debug
index 9aefa16..3ea717e 100644
--- a/xen/Kconfig.debug
+++ b/xen/Kconfig.debug
@@ -28,6 +28,21 @@ config FRAME_POINTER
 	  maybe slower, but it gives very useful debugging information
 	  in case of any Xen bugs.
 
+config PERF_ARRAYS
+	bool "Performance Counter Array Histograms"
+	default n
+	depends on PERF_COUNTERS
+	---help---
+	  Enables software performance counter array histograms.
+
+config PERF_COUNTERS
+	bool "Performance Counters"
+	default n
+	---help---
+	  Enables software performance counters that allows you to analyze
+	  bottlenecks in the system. To access this data you must use the
+	  'xenperf' tool.
+
 config VERBOSE_DEBUG
 	bool "Verbose debug messages"
 	default y
diff --git a/xen/Rules.mk b/xen/Rules.mk
index 84b9d81..7e2cb5f 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -3,8 +3,6 @@
 # If you change any of these configuration options then you must
 # 'make clean' before rebuilding.
 #
-perfc         ?= n
-perfc_arrays  ?= n
 lock_profile  ?= n
 lto           ?= n
 
@@ -12,12 +10,6 @@ lto           ?= n
 
 include $(XEN_ROOT)/Config.mk
 
-# Hardcoded configuration implications and dependencies.
-# Do this is a neater way if it becomes unwieldy.
-ifeq ($(perfc_arrays),y)
-perfc := y
-endif
-
 ifneq ($(origin kexec),undefined)
 $(error "You must use 'make menuconfig' to enable/disable kexec now.")
 endif
@@ -51,8 +43,6 @@ ifneq ($(clang),y)
 CFLAGS += -Wa,--strip-local-absolute
 endif
 
-CFLAGS-$(perfc)         += -DPERF_COUNTERS
-CFLAGS-$(perfc_arrays)  += -DPERF_ARRAYS
 CFLAGS-$(lock_profile)  += -DLOCK_PROFILE
 CFLAGS-$(CONFIG_FRAME_POINTER) += -fno-omit-frame-pointer
 
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 82e2ed1..8519dc9 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -3526,7 +3526,7 @@ void hvm_cpuid(unsigned int input, unsigned int *eax, unsigned int *ebx,
 static uint64_t _hvm_rdtsc_intercept(void)
 {
     struct vcpu *curr = current;
-#if !defined(NDEBUG) || defined(PERF_COUNTERS)
+#if !defined(NDEBUG) || defined(CONFIG_PERF_COUNTERS)
     struct domain *currd = curr->domain;
 
     if ( currd->arch.vtsc )
diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
index 6438b47..3928a5f 100644
--- a/xen/arch/x86/time.c
+++ b/xen/arch/x86/time.c
@@ -1688,7 +1688,7 @@ void pv_soft_rdtsc(struct vcpu *v, struct cpu_user_regs *regs, int rdtscp)
 
     spin_lock(&d->arch.vtsc_lock);
 
-#if !defined(NDEBUG) || defined(PERF_COUNTERS)
+#if !defined(NDEBUG) || defined(CONFIG_PERF_COUNTERS)
     if ( guest_kernel_mode(v, regs) )
         d->arch.vtsc_kerncount++;
     else
@@ -1959,7 +1959,7 @@ static void dump_softtsc(unsigned char key)
             printk(",khz=%"PRIu32, d->arch.tsc_khz);
         if ( d->arch.incarnation )
             printk(",inc=%"PRIu32, d->arch.incarnation);
-#if !defined(NDEBUG) || defined(PERF_COUNTERS)
+#if !defined(NDEBUG) || defined(CONFIG_PERF_COUNTERS)
         if ( !(d->arch.vtsc_kerncount | d->arch.vtsc_usercount) )
             printk("\n");
         else
diff --git a/xen/arch/x86/x86_64/asm-offsets.c b/xen/arch/x86/x86_64/asm-offsets.c
index fa37ee0..891ddc8 100644
--- a/xen/arch/x86/x86_64/asm-offsets.c
+++ b/xen/arch/x86/x86_64/asm-offsets.c
@@ -151,7 +151,7 @@ void __dummy__(void)
     OFFSET(TRAPBOUNCE_eip, struct trap_bounce, eip);
     BLANK();
 
-#if PERF_COUNTERS
+#if CONFIG_PERF_COUNTERS
     DEFINE(ASM_PERFC_hypercalls, PERFC_hypercalls);
     DEFINE(ASM_PERFC_exceptions, PERFC_exceptions);
     BLANK();
diff --git a/xen/common/Makefile b/xen/common/Makefile
index a98bcc2..5891b1c 100644
--- a/xen/common/Makefile
+++ b/xen/common/Makefile
@@ -64,7 +64,7 @@ obj-$(CONFIG_XSPLICE) += xsplice_elf.o
 
 obj-bin-$(CONFIG_X86) += $(foreach n,decompress bunzip2 unxz unlzma unlzo unlz4 earlycpio,$(n).init.o)
 
-obj-$(perfc)       += perfc.o
+obj-$(CONFIG_PERF_COUNTERS)       += perfc.o
 
 obj-$(CONFIG_COMPAT) += $(addprefix compat/,domain.o kernel.o memory.o multicall.o xlat.o)
 
diff --git a/xen/common/keyhandler.c b/xen/common/keyhandler.c
index 4ff90f6..65b70ce 100644
--- a/xen/common/keyhandler.c
+++ b/xen/common/keyhandler.c
@@ -59,7 +59,7 @@ static struct keyhandler {
     IRQ_KEYHANDLER('%', do_debug_key, "trap to xendbg", 0),
     IRQ_KEYHANDLER('*', run_all_keyhandlers, "print all diagnostics", 0),
 
-#ifdef PERF_COUNTERS
+#ifdef CONFIG_PERF_COUNTERS
     KEYHANDLER('p', perfc_printall, "print performance counters", 1),
     KEYHANDLER('P', perfc_reset, "reset performance counters", 0),
 #endif
diff --git a/xen/common/perfc.c b/xen/common/perfc.c
index 9f078e1..3da4c96 100644
--- a/xen/common/perfc.c
+++ b/xen/common/perfc.c
@@ -78,7 +78,7 @@ void perfc_printall(unsigned char key)
             printk("TOTAL[%12Lu]", sum);
             if (sum)
             {
-#ifdef PERF_ARRAYS
+#ifdef CONFIG_PERF_ARRAYS
                 for ( k = 0; k < perfc_info[i].nr_elements; k++ )
                 {
                     sum = 0;
diff --git a/xen/common/sysctl.c b/xen/common/sysctl.c
index 9a4cc1f..11bef0e 100644
--- a/xen/common/sysctl.c
+++ b/xen/common/sysctl.c
@@ -115,7 +115,7 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
     }
     break;
 
-#ifdef PERF_COUNTERS
+#ifdef CONFIG_PERF_COUNTERS
     case XEN_SYSCTL_perfc_op:
         ret = perfc_control(&op->u.perfc_op);
         break;
diff --git a/xen/include/asm-x86/asm_defns.h b/xen/include/asm-x86/asm_defns.h
index b5f79d8..24f5404 100644
--- a/xen/include/asm-x86/asm_defns.h
+++ b/xen/include/asm-x86/asm_defns.h
@@ -363,7 +363,7 @@ static always_inline void stac(void)
 
 #endif
 
-#ifdef PERF_COUNTERS
+#ifdef CONFIG_PERF_COUNTERS
 #define PERFC_INCR(_name,_idx,_cur)             \
         pushq _cur;                             \
         movslq VCPU_processor(_cur),_cur;       \
diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
index 165e533..783fa4f 100644
--- a/xen/include/asm-x86/domain.h
+++ b/xen/include/asm-x86/domain.h
@@ -377,7 +377,7 @@ struct arch_domain
                                      hardware TSC scaling cases */
     uint32_t incarnation;    /* incremented every restore or live migrate
                                 (possibly other cases in the future */
-#if !defined(NDEBUG) || defined(PERF_COUNTERS)
+#if !defined(NDEBUG) || defined(CONFIG_PERF_COUNTERS)
     uint64_t vtsc_kerncount;
     uint64_t vtsc_usercount;
 #endif
diff --git a/xen/include/xen/perfc.h b/xen/include/xen/perfc.h
index 6cb0cd1..6846e71 100644
--- a/xen/include/xen/perfc.h
+++ b/xen/include/xen/perfc.h
@@ -1,7 +1,7 @@
 #ifndef __XEN_PERFC_H__
 #define __XEN_PERFC_H__
 
-#ifdef PERF_COUNTERS
+#ifdef CONFIG_PERF_COUNTERS
 
 #include <xen/lib.h>
 #include <xen/smp.h>
@@ -76,7 +76,7 @@ DECLARE_PER_CPU(perfc_t[NUM_PERFCOUNTERS], perfcounters);
  * Histogram: special treatment for 0 and 1 count. After that equally spaced 
  * with last bucket taking the rest.
  */
-#ifdef PERF_ARRAYS
+#ifdef CONFIG_PERF_ARRAYS
 #define perfc_incr_histo(x,v)                                           \
     do {                                                                \
         if ( (v) == 0 )                                                 \
@@ -100,7 +100,7 @@ extern void perfc_printall(unsigned char key);
 extern void perfc_reset(unsigned char key);
 
     
-#else /* PERF_COUNTERS */
+#else /* CONFIG_PERF_COUNTERS */
 
 #define perfc_value(x)    (0)
 #define perfc_valuea(x,y) (0)
@@ -114,6 +114,6 @@ extern void perfc_reset(unsigned char key);
 #define perfc_adda(x,y,z) ((void)0)
 #define perfc_incr_histo(x,y,z) ((void)0)
 
-#endif /* PERF_COUNTERS */
+#endif /* CONFIG_PERF_COUNTERS */
 
 #endif /* __XEN_PERFC_H__ */
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 09fff5f..651cd96 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -38,7 +38,7 @@ DEFINE_XEN_GUEST_HANDLE(vcpu_runstate_info_compat_t);
  * Enable and ease the use of scheduling related performance counters.
  *
  */
-#ifdef PERF_COUNTERS
+#ifdef CONFIG_PERF_COUNTERS
 #define SCHED_STATS
 #endif
 
-- 
2.7.3


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

[PATCH v2 7/7] build: convert lock_profile to Kconfig

[Doug Goldstein]

Convert the 'lock_profile' option to Kconfig as CONFIG_LOCK_PROFILE.

Signed-off-by: Doug Goldstein <cardoe@cardoe.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>
---
 INSTALL                    |  1 -
 xen/Kconfig.debug          |  8 ++++++++
 xen/Rules.mk               |  2 --
 xen/arch/arm/xen.lds.S     |  2 +-
 xen/arch/x86/domain.c      |  2 +-
 xen/arch/x86/xen.lds.S     |  2 +-
 xen/common/keyhandler.c    |  2 +-
 xen/common/spinlock.c      | 10 +++++-----
 xen/common/sysctl.c        |  2 +-
 xen/include/xen/spinlock.h |  4 ++--
 10 files changed, 20 insertions(+), 15 deletions(-)

diff --git a/INSTALL b/INSTALL
index 623887d..616a67a 100644
--- a/INSTALL
+++ b/INSTALL
@@ -227,7 +227,6 @@ VGABIOS_REL_DATE="dd Mon yyyy"
 
 The following variables can be used to tweak some aspects of the
 hypervisor build.
-lock_profile=y
 lto=y
 
 During tools build external repos will be cloned into the source tree.
diff --git a/xen/Kconfig.debug b/xen/Kconfig.debug
index 3ea717e..a03aca9 100644
--- a/xen/Kconfig.debug
+++ b/xen/Kconfig.debug
@@ -28,6 +28,14 @@ config FRAME_POINTER
 	  maybe slower, but it gives very useful debugging information
 	  in case of any Xen bugs.
 
+config LOCK_PROFILE
+	bool "Lock Profiiling"
+	default n
+	---help---
+	  Lock profiling allows you to see how often locks are taken and blocked.
+	  You can use serial console to print (and reset) using 'l' and 'L'
+	  respectively, or the 'xenlockprof' tool.
+
 config PERF_ARRAYS
 	bool "Performance Counter Array Histograms"
 	default n
diff --git a/xen/Rules.mk b/xen/Rules.mk
index 7e2cb5f..3015c5b 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -3,7 +3,6 @@
 # If you change any of these configuration options then you must
 # 'make clean' before rebuilding.
 #
-lock_profile  ?= n
 lto           ?= n
 
 -include $(BASEDIR)/include/config/auto.conf
@@ -43,7 +42,6 @@ ifneq ($(clang),y)
 CFLAGS += -Wa,--strip-local-absolute
 endif
 
-CFLAGS-$(lock_profile)  += -DLOCK_PROFILE
 CFLAGS-$(CONFIG_FRAME_POINTER) += -fno-omit-frame-pointer
 
 ifneq ($(max_phys_irqs),)
diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S
index 1f010bd..76982b2 100644
--- a/xen/arch/arm/xen.lds.S
+++ b/xen/arch/arm/xen.lds.S
@@ -54,7 +54,7 @@ SECTIONS
        *(.rodata)
        *(.rodata.*)
 
-#ifdef LOCK_PROFILE
+#ifdef CONFIG_LOCK_PROFILE
        . = ALIGN(POINTER_ALIGN);
        __lock_profile_start = .;
        *(.lockprofile.data)
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 5af2cc5..978ec3a 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -251,7 +251,7 @@ struct domain *alloc_domain_struct(void)
 #endif
 
 
-#ifndef LOCK_PROFILE
+#ifndef CONFIG_LOCK_PROFILE
     BUILD_BUG_ON(sizeof(*d) > PAGE_SIZE);
 #endif
     d = alloc_xenheap_pages(order, MEMF_bits(bits));
diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
index b14bcd2..a43b29d 100644
--- a/xen/arch/x86/xen.lds.S
+++ b/xen/arch/x86/xen.lds.S
@@ -103,7 +103,7 @@ SECTIONS
        *(.ex_table.pre)
        __stop___pre_ex_table = .;
 
-#ifdef LOCK_PROFILE
+#ifdef CONFIG_LOCK_PROFILE
        . = ALIGN(POINTER_ALIGN);
        __lock_profile_start = .;
        *(.lockprofile.data)
diff --git a/xen/common/keyhandler.c b/xen/common/keyhandler.c
index 65b70ce..16de6e8 100644
--- a/xen/common/keyhandler.c
+++ b/xen/common/keyhandler.c
@@ -64,7 +64,7 @@ static struct keyhandler {
     KEYHANDLER('P', perfc_reset, "reset performance counters", 0),
 #endif
 
-#ifdef LOCK_PROFILE
+#ifdef CONFIG_LOCK_PROFILE
     KEYHANDLER('l', spinlock_profile_printall, "print lock profile info", 1),
     KEYHANDLER('L', spinlock_profile_reset, "reset lock profile info", 0),
 #endif
diff --git a/xen/common/spinlock.c b/xen/common/spinlock.c
index b377bb9..017bdf3 100644
--- a/xen/common/spinlock.c
+++ b/xen/common/spinlock.c
@@ -85,7 +85,7 @@ void spin_debug_disable(void)
 
 #endif
 
-#ifdef LOCK_PROFILE
+#ifdef CONFIG_LOCK_PROFILE
 
 #define LOCK_PROFILE_REL                                                     \
     if (lock->profile)                                                       \
@@ -212,7 +212,7 @@ int _spin_trylock(spinlock_t *lock)
     if ( cmpxchg(&lock->tickets.head_tail,
                  old.head_tail, new.head_tail) != old.head_tail )
         return 0;
-#ifdef LOCK_PROFILE
+#ifdef CONFIG_LOCK_PROFILE
     if (lock->profile)
         lock->profile->time_locked = NOW();
 #endif
@@ -227,7 +227,7 @@ int _spin_trylock(spinlock_t *lock)
 void _spin_barrier(spinlock_t *lock)
 {
     spinlock_tickets_t sample;
-#ifdef LOCK_PROFILE
+#ifdef CONFIG_LOCK_PROFILE
     s_time_t block = NOW();
 #endif
 
@@ -238,7 +238,7 @@ void _spin_barrier(spinlock_t *lock)
     {
         while ( observe_head(&lock->tickets) == sample.head )
             arch_lock_relax();
-#ifdef LOCK_PROFILE
+#ifdef CONFIG_LOCK_PROFILE
         if ( lock->profile )
         {
             lock->profile->time_block += NOW() - block;
@@ -296,7 +296,7 @@ void _spin_unlock_recursive(spinlock_t *lock)
     }
 }
 
-#ifdef LOCK_PROFILE
+#ifdef CONFIG_LOCK_PROFILE
 
 struct lock_profile_anc {
     struct lock_profile_qhead *head_q;   /* first head of this type */
diff --git a/xen/common/sysctl.c b/xen/common/sysctl.c
index 11bef0e..b4f235e 100644
--- a/xen/common/sysctl.c
+++ b/xen/common/sysctl.c
@@ -121,7 +121,7 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
         break;
 #endif
 
-#ifdef LOCK_PROFILE
+#ifdef CONFIG_LOCK_PROFILE
     case XEN_SYSCTL_lockprof_op:
         ret = spinlock_profile_control(&op->u.lockprof_op);
         break;
diff --git a/xen/include/xen/spinlock.h b/xen/include/xen/spinlock.h
index 88b53f9..c1883bd 100644
--- a/xen/include/xen/spinlock.h
+++ b/xen/include/xen/spinlock.h
@@ -20,7 +20,7 @@ struct lock_debug { };
 #define spin_debug_disable() ((void)0)
 #endif
 
-#ifdef LOCK_PROFILE
+#ifdef CONFIG_LOCK_PROFILE
 
 #include <public/sysctl.h>
 
@@ -144,7 +144,7 @@ typedef struct spinlock {
     u16 recurse_cnt:4;
 #define SPINLOCK_MAX_RECURSE 0xfu
     struct lock_debug debug;
-#ifdef LOCK_PROFILE
+#ifdef CONFIG_LOCK_PROFILE
     struct lock_profile *profile;
 #endif
 } spinlock_t;
-- 
2.7.3


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

Re: [PATCH v2 1/7] build: add debug menu to Kconfig

[Jan Beulich]

>>> On 03.05.16 at 16:29, <cardoe@cardoe.com> wrote:
> --- a/xen/include/xen/config.h
> +++ b/xen/include/xen/config.h
> @@ -81,4 +81,8 @@
>  /* allow existing code to work with Kconfig variable */
>  #define NR_CPUS CONFIG_NR_CPUS
>  
> +#ifndef CONFIG_DEBUG
> +#define NDEBUG
> +#endif

At the same time you should delete the -DNDEBUG from xen/Rules.mk.
There shouldn't be two places controlling the same thing.

Jan


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

Re: [PATCH v2 1/7] build: add debug menu to Kconfig

[Doug Goldstein]

[-- Attachment #1.1.1: Type: text/plain, Size: 618 bytes --]

On 5/3/16 9:38 AM, Jan Beulich wrote:
>>>> On 03.05.16 at 16:29, <cardoe@cardoe.com> wrote:
>> --- a/xen/include/xen/config.h
>> +++ b/xen/include/xen/config.h
>> @@ -81,4 +81,8 @@
>>  /* allow existing code to work with Kconfig variable */
>>  #define NR_CPUS CONFIG_NR_CPUS
>>  
>> +#ifndef CONFIG_DEBUG
>> +#define NDEBUG
>> +#endif
> 
> At the same time you should delete the -DNDEBUG from xen/Rules.mk.
> There shouldn't be two places controlling the same thing.
> 
> Jan
> 

You're right. That hunk slipped to 3/7 during a rebase. I'll repost with
that moved to 1/7.

-- 
Doug Goldstein


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 959 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

Re: [PATCH v2 2/7] build: convert crash_debug to Kconfig

[Jan Beulich]

>>> On 03.05.16 at 16:29, <cardoe@cardoe.com> wrote:
> Convert the crash_debug option to Kconfig as CONFIG_CRASH_DEBUG. This
> was previously togglable on the command line so this adds a message for
> users enabling it from the command line to tell them to enable it from
> make menuconfig.
> 
> Signed-off-by: Doug Goldstein <cardoe@cardoe.com>
> ---
> CC: Jan Beulich <jbeulich@suse.com>
> CC: Andrew Cooper <andrew.cooper3@citrix.com>

I think the Cc list should be quite a bit wider here.

> --- a/xen/Kconfig.debug
> +++ b/xen/Kconfig.debug
> @@ -5,3 +5,14 @@ menuconfig DEBUG
>  	  If you want to debug Xen say Y and select any additional debugging
>  	  support options. Enabling this option is intended for development
>  	  purposes only, and not for production use.
> +
> +if DEBUG

Didn't we agree on "DEBUG || EXPERT" at the hackathon? With tha
Reviewed-by: Jan Beulich <jbeulich@suse.com>

But one more suggestion:

> --- a/xen/include/asm-x86/debugger.h
> +++ b/xen/include/asm-x86/debugger.h
> @@ -39,7 +39,7 @@
>  #define DEBUGGER_trap_fatal(_v, _r) \
>      if ( debugger_trap_fatal(_v, _r) ) return;
>  
> -#if defined(CRASH_DEBUG)
> +#if defined(CONFIG_CRASH_DEBUG)

#ifdef

Jan


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

Re: [PATCH v2 2/7] build: convert crash_debug to Kconfig

[Andrew Cooper]

On 03/05/16 15:29, Doug Goldstein wrote:
> diff --git a/xen/Kconfig.debug b/xen/Kconfig.debug
> index e5179f4..04f672f 100644
> --- a/xen/Kconfig.debug
> +++ b/xen/Kconfig.debug
> @@ -5,3 +5,14 @@ menuconfig DEBUG
>  	  If you want to debug Xen say Y and select any additional debugging
>  	  support options. Enabling this option is intended for development
>  	  purposes only, and not for production use.
> +
> +if DEBUG
> +
> +config CRASH_DEBUG
> +	bool "Crash Debugging Support"
> +	depends on X86

default n

Even with debugging enabled before, this wasn't available by default
(see the hunk below).

~Andrew

> +	---help---
> +	  If you want to be able to attach gdb to Xen to be able to debug
> +	  Xen if it crashes then say Y.
> +
> +endif # DEBUG
> diff --git a/xen/Rules.mk b/xen/Rules.mk
> index 961d533..c044fd1 100644
> --- a/xen/Rules.mk
> +++ b/xen/Rules.mk
> @@ -7,7 +7,6 @@ verbose       ?= n
>  perfc         ?= n
>  perfc_arrays  ?= n
>  lock_profile  ?= n
> -crash_debug   ?= n
>  frame_pointer ?= n
>  lto           ?= n


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

Re: [PATCH v2 3/7] build: convert verbose to Kconfig

[Jan Beulich]

>>> On 03.05.16 at 16:29, <cardoe@cardoe.com> wrote:
> --- a/xen/Kconfig.debug
> +++ b/xen/Kconfig.debug
> @@ -15,4 +15,11 @@ config CRASH_DEBUG
>  	  If you want to be able to attach gdb to Xen to be able to debug
>  	  Xen if it crashes then say Y.
>  
> +config VERBOSE_DEBUG
> +	bool "Verbose debug messages"
> +	default y

With the enclosing "if" adjusted, this would need to become
"default DEBUG", and I think that's a good idea anyway (i.e. even
if that other change was rejected by someone else, this would still
make the code prepared to _some_ future adjustment to that "if").

> @@ -17,10 +16,7 @@ include $(XEN_ROOT)/Config.mk
>  # Hardcoded configuration implications and dependencies.
>  # Do this is a neater way if it becomes unwieldy.
>  ifeq ($(debug),y)
> -verbose       := y
>  frame_pointer := y
> -else
> -CFLAGS += -DNDEBUG

Ah, here's the change that belongs into patch 1.

Again, with those adjustments
Reviewed-by: Jan Beulich <jbeulich@suse.com>


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

Re: [PATCH v2 2/7] build: convert crash_debug to Kconfig

[Doug Goldstein]

[-- Attachment #1.1.1: Type: text/plain, Size: 810 bytes --]

On 5/3/16 9:43 AM, Jan Beulich wrote:
>>>> On 03.05.16 at 16:29, <cardoe@cardoe.com> wrote:
>> Convert the crash_debug option to Kconfig as CONFIG_CRASH_DEBUG. This
>> was previously togglable on the command line so this adds a message for
>> users enabling it from the command line to tell them to enable it from
>> make menuconfig.
>>
>> Signed-off-by: Doug Goldstein <cardoe@cardoe.com>
>> ---
>> CC: Jan Beulich <jbeulich@suse.com>
>> CC: Andrew Cooper <andrew.cooper3@citrix.com>
> 
> I think the Cc list should be quite a bit wider here.

$ ./scripts/get_maintainer.pl
patches/v2-0002-build-convert-crash_debug-to-Kconfig.patch
Jan Beulich <jbeulich@suse.com>
Andrew Cooper <andrew.cooper3@citrix.com>
xen-devel@lists.xen.org

Do you want me to CC THE REST?

-- 
Doug Goldstein


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 959 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

Re: [PATCH v2 4/7] build: convert frame_pointer to Kconfig

[Jan Beulich]

>>> On 03.05.16 at 16:29, <cardoe@cardoe.com> wrote:
> --- a/xen/Kconfig.debug
> +++ b/xen/Kconfig.debug
> @@ -15,6 +15,14 @@ config CRASH_DEBUG
>  	  If you want to be able to attach gdb to Xen to be able to debug
>  	  Xen if it crashes then say Y.
>  
> +config FRAME_POINTER
> +	bool "Compile Xen with frame pointers"
> +	default y

Same as for patch 3, including the R-b.

Jan


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

Re: [PATCH v2 5/7] build: wire up pre-existing debug build flag

[Jan Beulich]

>>> On 03.05.16 at 16:29, <cardoe@cardoe.com> wrote:
> This allows 'make debug=n' and 'make debug=y' work as it did previously
> but only in the case of the user not having an existing .config file
> from a 'make menuconfig'. This is because the command line 'debug' flag
> can only be used to set the default value and if the user has already
> built up a config with their real preference set.

Do we really need this?

Jan


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

Re: [PATCH v2 6/7] build: convert perfc{, _arrays} to Kconfig

[Jan Beulich]

>>> On 03.05.16 at 16:29, <cardoe@cardoe.com> wrote:
> Convert the 'perfc' and 'perfc_arrays' options to Kconfig as
> CONFIG_PERF_COUNTERS and CONFIG_PERF_ARRAYS to minimize code changes.
> 
> Signed-off-by: Doug Goldstein <cardoe@cardoe.com>
> ---
> CC: Jan Beulich <jbeulich@suse.com>
> CC: Andrew Cooper <andrew.cooper3@citrix.com>

Again too narrow a Cc list.

> --- a/xen/Kconfig.debug
> +++ b/xen/Kconfig.debug
> @@ -28,6 +28,21 @@ config FRAME_POINTER
>  	  maybe slower, but it gives very useful debugging information
>  	  in case of any Xen bugs.
>  
> +config PERF_ARRAYS
> +	bool "Performance Counter Array Histograms"
> +	default n
> +	depends on PERF_COUNTERS
> +	---help---
> +	  Enables software performance counter array histograms.
> +
> +config PERF_COUNTERS
> +	bool "Performance Counters"
> +	default n
> +	---help---
> +	  Enables software performance counters that allows you to analyze
> +	  bottlenecks in the system. To access this data you must use the
> +	  'xenperf' tool.

I guess you put them in this order because that's how they sort.
But for any of the menu config interfaces this will result in the
former not properly becoming a sub-item of the latter. IOW
dependent should always (directly) follow their master options.

Also - is "default n" really useful here? In one of the earlier
patches you had omitted it, and that's what I'd prefer unless
doing so has some bad side effect.

>  ifneq ($(origin kexec),undefined)
>  $(error "You must use 'make menuconfig' to enable/disable kexec now.")
>  endif

Considering this context - no similar addition here?

> --- a/xen/arch/x86/x86_64/asm-offsets.c
> +++ b/xen/arch/x86/x86_64/asm-offsets.c
> @@ -151,7 +151,7 @@ void __dummy__(void)
>      OFFSET(TRAPBOUNCE_eip, struct trap_bounce, eip);
>      BLANK();
>  
> -#if PERF_COUNTERS
> +#if CONFIG_PERF_COUNTERS

#ifdef

> --- a/xen/common/Makefile
> +++ b/xen/common/Makefile
> @@ -64,7 +64,7 @@ obj-$(CONFIG_XSPLICE) += xsplice_elf.o
>  
>  obj-bin-$(CONFIG_X86) += $(foreach n,decompress bunzip2 unxz unlzma unlzo unlz4 earlycpio,$(n).init.o)
>  
> -obj-$(perfc)       += perfc.o
> +obj-$(CONFIG_PERF_COUNTERS)       += perfc.o

Please move this up at once, too, like you did in one of the earlier
patches.

Jan


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

Re: [PATCH v2 7/7] build: convert lock_profile to Kconfig

[Jan Beulich]

>>> On 03.05.16 at 16:29, <cardoe@cardoe.com> wrote:
> Convert the 'lock_profile' option to Kconfig as CONFIG_LOCK_PROFILE.
> 
> Signed-off-by: Doug Goldstein <cardoe@cardoe.com>

Subject to transformations paralleling such requested on earlier
patches:
Reviewed-by: Jan Beulich <jbeulich@suse.com>


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

Re: [PATCH v2 2/7] build: convert crash_debug to Kconfig

[Jan Beulich]

>>> On 03.05.16 at 16:47, <andrew.cooper3@citrix.com> wrote:
> On 03/05/16 15:29, Doug Goldstein wrote:
>> diff --git a/xen/Kconfig.debug b/xen/Kconfig.debug
>> index e5179f4..04f672f 100644
>> --- a/xen/Kconfig.debug
>> +++ b/xen/Kconfig.debug
>> @@ -5,3 +5,14 @@ menuconfig DEBUG
>>  	  If you want to debug Xen say Y and select any additional debugging
>>  	  support options. Enabling this option is intended for development
>>  	  purposes only, and not for production use.
>> +
>> +if DEBUG
>> +
>> +config CRASH_DEBUG
>> +	bool "Crash Debugging Support"
>> +	depends on X86
> 
> default n

See my reply to a later patch: Ideally we'd go without this (the default
is n anyway). The only thing I'm not sure is whether omitting the
defaults in such cases leads to questions getting asked during some
intended silent "make *config".

Jan


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

Re: [PATCH v2 1/7] build: add debug menu to Kconfig

[Jan Beulich]

>>> On 03.05.16 at 16:29, <cardoe@cardoe.com> wrote:
> --- /dev/null
> +++ b/xen/Kconfig.debug
> @@ -0,0 +1,7 @@
> +
> +menuconfig DEBUG
> +	bool "Debugging Options"

One more thing: In the unstable branch this should really default to
y, and the release check list should be adjusted to say that this
default needs to be dropped (or inverted) while preparing a release.

And obviously the "debug=" also needs to go away from ./Config.mk.

Jan


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

Re: [PATCH v2 1/7] build: add debug menu to Kconfig

[Andrew Cooper]

On 03/05/16 16:05, Jan Beulich wrote:
>>>> On 03.05.16 at 16:29, <cardoe@cardoe.com> wrote:
>> --- /dev/null
>> +++ b/xen/Kconfig.debug
>> @@ -0,0 +1,7 @@
>> +
>> +menuconfig DEBUG
>> +	bool "Debugging Options"
> One more thing: In the unstable branch this should really default to
> y, and the release check list should be adjusted to say that this
> default needs to be dropped (or inverted) while preparing a release.
>
> And obviously the "debug=" also needs to go away from ./Config.mk.

Things other than Xen make use of debug= in the root Config.mk. 
Valgrind client requests for example in libxc.

That option can't be moved without providing an alternative.

~Andrew

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

Re: [PATCH v2 1/7] build: add debug menu to Kconfig

[Jan Beulich]

>>> On 03.05.16 at 17:10, <andrew.cooper3@citrix.com> wrote:
> On 03/05/16 16:05, Jan Beulich wrote:
>>>>> On 03.05.16 at 16:29, <cardoe@cardoe.com> wrote:
>>> --- /dev/null
>>> +++ b/xen/Kconfig.debug
>>> @@ -0,0 +1,7 @@
>>> +
>>> +menuconfig DEBUG
>>> +	bool "Debugging Options"
>> One more thing: In the unstable branch this should really default to
>> y, and the release check list should be adjusted to say that this
>> default needs to be dropped (or inverted) while preparing a release.
>>
>> And obviously the "debug=" also needs to go away from ./Config.mk.
> 
> Things other than Xen make use of debug= in the root Config.mk. 
> Valgrind client requests for example in libxc.
> 
> That option can't be moved without providing an alternative.

Why can't it be moved into tools/ if that's where it is still of interest?

Jan


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

Re: [PATCH v2 2/7] build: convert crash_debug to Kconfig

[Jan Beulich]

>>> On 03.05.16 at 16:48, <cardoe@cardoe.com> wrote:
> On 5/3/16 9:43 AM, Jan Beulich wrote:
>>>>> On 03.05.16 at 16:29, <cardoe@cardoe.com> wrote:
>>> Convert the crash_debug option to Kconfig as CONFIG_CRASH_DEBUG. This
>>> was previously togglable on the command line so this adds a message for
>>> users enabling it from the command line to tell them to enable it from
>>> make menuconfig.
>>>
>>> Signed-off-by: Doug Goldstein <cardoe@cardoe.com>
>>> ---
>>> CC: Jan Beulich <jbeulich@suse.com>
>>> CC: Andrew Cooper <andrew.cooper3@citrix.com>
>> 
>> I think the Cc list should be quite a bit wider here.
> 
> $ ./scripts/get_maintainer.pl
> patches/v2-0002-build-convert-crash_debug-to-Kconfig.patch
> Jan Beulich <jbeulich@suse.com>
> Andrew Cooper <andrew.cooper3@citrix.com>
> xen-devel@lists.xen.org 
> 
> Do you want me to CC THE REST?

With files belonging to REST being touched - yes. (And yes, we all
know the script isn't perfect.)

Jan


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

Re: [PATCH v2 1/7] build: add debug menu to Kconfig

[Doug Goldstein]

[-- Attachment #1.1.1: Type: text/plain, Size: 1150 bytes --]

On 5/3/16 10:18 AM, Jan Beulich wrote:
>>>> On 03.05.16 at 17:10, <andrew.cooper3@citrix.com> wrote:
>> On 03/05/16 16:05, Jan Beulich wrote:
>>>>>> On 03.05.16 at 16:29, <cardoe@cardoe.com> wrote:
>>>> --- /dev/null
>>>> +++ b/xen/Kconfig.debug
>>>> @@ -0,0 +1,7 @@
>>>> +
>>>> +menuconfig DEBUG
>>>> +	bool "Debugging Options"
>>> One more thing: In the unstable branch this should really default to
>>> y, and the release check list should be adjusted to say that this
>>> default needs to be dropped (or inverted) while preparing a release.
>>>
>>> And obviously the "debug=" also needs to go away from ./Config.mk.
>>
>> Things other than Xen make use of debug= in the root Config.mk. 
>> Valgrind client requests for example in libxc.
>>
>> That option can't be moved without providing an alternative.
> 
> Why can't it be moved into tools/ if that's where it is still of interest?
> 
> Jan
> 

You could argue it could then move into the autoconf script which would
make the knobs more consistent for each part of the tree. The tools/
bits always as autoconf and the xen/ bits as Kconfig.

-- 
Doug Goldstein


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 959 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

Re: [PATCH v2 2/7] build: convert crash_debug to Kconfig

[Doug Goldstein]

[-- Attachment #1.1.1: Type: text/plain, Size: 1500 bytes --]

On 5/3/16 9:43 AM, Jan Beulich wrote:
>>>> On 03.05.16 at 16:29, <cardoe@cardoe.com> wrote:
>> Convert the crash_debug option to Kconfig as CONFIG_CRASH_DEBUG. This
>> was previously togglable on the command line so this adds a message for
>> users enabling it from the command line to tell them to enable it from
>> make menuconfig.
>>
>> Signed-off-by: Doug Goldstein <cardoe@cardoe.com>
>> ---
>> CC: Jan Beulich <jbeulich@suse.com>
>> CC: Andrew Cooper <andrew.cooper3@citrix.com>
> 
> I think the Cc list should be quite a bit wider here.
> 
>> --- a/xen/Kconfig.debug
>> +++ b/xen/Kconfig.debug
>> @@ -5,3 +5,14 @@ menuconfig DEBUG
>>  	  If you want to debug Xen say Y and select any additional debugging
>>  	  support options. Enabling this option is intended for development
>>  	  purposes only, and not for production use.
>> +
>> +if DEBUG
> 
> Didn't we agree on "DEBUG || EXPERT" at the hackathon? With tha
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> 

Well this will actually fundamentally change the patch. Because the
DEBUG flag is a menuconfig so I can't just do "DEBUG || EXPERT". I guess
the concern here is that I tied DEBUG to #undef NDEBUG? I can disconnect
that and make the operation of NDEBUG its own thing.

I can rename the CONFIG_VERBOSE_DEBUG option to something like
CONFIG_CONSOLE_DEBUG because its HYPERVISOR_console_io and ELF parsing.
Then create a CONFIG_VERBOSE_DEBUG option that controls NDEBUG?

-- 
Doug Goldstein


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 959 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

Re: [PATCH v2 5/7] build: wire up pre-existing debug build flag

[Doug Goldstein]

[-- Attachment #1.1.1: Type: text/plain, Size: 666 bytes --]

On 5/3/16 9:50 AM, Jan Beulich wrote:
>>>> On 03.05.16 at 16:29, <cardoe@cardoe.com> wrote:
>> This allows 'make debug=n' and 'make debug=y' work as it did previously
>> but only in the case of the user not having an existing .config file
>> from a 'make menuconfig'. This is because the command line 'debug' flag
>> can only be used to set the default value and if the user has already
>> built up a config with their real preference set.
> 
> Do we really need this?
> 
> Jan
> 

Honestly I did this because previously people didn't want to alter their
workflow, I am more than happy to drop this if others concur with you.

-- 
Doug Goldstein


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 959 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

Re: [PATCH v2 1/7] build: add debug menu to Kconfig

[Jan Beulich]

>>> On 04.05.16 at 04:22, <cardoe@cardoe.com> wrote:
> On 5/3/16 10:18 AM, Jan Beulich wrote:
>>>>> On 03.05.16 at 17:10, <andrew.cooper3@citrix.com> wrote:
>>> On 03/05/16 16:05, Jan Beulich wrote:
>>>>>>> On 03.05.16 at 16:29, <cardoe@cardoe.com> wrote:
>>>>> --- /dev/null
>>>>> +++ b/xen/Kconfig.debug
>>>>> @@ -0,0 +1,7 @@
>>>>> +
>>>>> +menuconfig DEBUG
>>>>> +	bool "Debugging Options"
>>>> One more thing: In the unstable branch this should really default to
>>>> y, and the release check list should be adjusted to say that this
>>>> default needs to be dropped (or inverted) while preparing a release.
>>>>
>>>> And obviously the "debug=" also needs to go away from ./Config.mk.
>>>
>>> Things other than Xen make use of debug= in the root Config.mk. 
>>> Valgrind client requests for example in libxc.
>>>
>>> That option can't be moved without providing an alternative.
>> 
>> Why can't it be moved into tools/ if that's where it is still of interest?
> 
> You could argue it could then move into the autoconf script which would
> make the knobs more consistent for each part of the tree. The tools/
> bits always as autoconf and the xen/ bits as Kconfig.

I agree, but perhaps as a separate step.

Jan


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

Re: [PATCH v2 2/7] build: convert crash_debug to Kconfig

[Jan Beulich]

>>> On 04.05.16 at 05:10, <cardoe@cardoe.com> wrote:
> On 5/3/16 9:43 AM, Jan Beulich wrote:
>>>>> On 03.05.16 at 16:29, <cardoe@cardoe.com> wrote:
>>> Convert the crash_debug option to Kconfig as CONFIG_CRASH_DEBUG. This
>>> was previously togglable on the command line so this adds a message for
>>> users enabling it from the command line to tell them to enable it from
>>> make menuconfig.
>>>
>>> Signed-off-by: Doug Goldstein <cardoe@cardoe.com>
>>> ---
>>> CC: Jan Beulich <jbeulich@suse.com>
>>> CC: Andrew Cooper <andrew.cooper3@citrix.com>
>> 
>> I think the Cc list should be quite a bit wider here.
>> 
>>> --- a/xen/Kconfig.debug
>>> +++ b/xen/Kconfig.debug
>>> @@ -5,3 +5,14 @@ menuconfig DEBUG
>>>  	  If you want to debug Xen say Y and select any additional debugging
>>>  	  support options. Enabling this option is intended for development
>>>  	  purposes only, and not for production use.
>>> +
>>> +if DEBUG
>> 
>> Didn't we agree on "DEBUG || EXPERT" at the hackathon? With tha
>> Reviewed-by: Jan Beulich <jbeulich@suse.com>
>> 
> 
> Well this will actually fundamentally change the patch. Because the
> DEBUG flag is a menuconfig so I can't just do "DEBUG || EXPERT".

I don't understand. I didn't say to change the config option, all I
asked for was to change the conditional. And I don't see why that
can't become "if DEBUG || EXPERT".

> I guess
> the concern here is that I tied DEBUG to #undef NDEBUG? I can disconnect
> that and make the operation of NDEBUG its own thing.

No, that's not the concern. The concern is that the sub-options
should be separately controllable even when DEBUG is off.

Jan


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