[PATCH v4 0/6] Kconfig debug options

[PATCH v4 0/6] Kconfig debug options

[Doug Goldstein]

This converts the debug options from xen/Rules.mk to Kconfig. Hopefully
I haven't missed anything in the back and forth.

change since v3:
- wrap all options in 'if DEBUG || EXPERT' (except DEBUG)
- wording update to DEBUG option and some commit messages
- all old command line options now complain
change since v2:
- dropped patch 5 as it was unwanted
- remove 'default n'
- redid patch 1


*** BLURB HERE ***

Doug Goldstein (6):
  build: convert debug to Kconfig
  build: convert crash_debug to Kconfig
  build: convert verbose to Kconfig
  build: convert frame_pointer to Kconfig
  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                 | 60 +++++++++++++++++++++++++++++++++++++++
 xen/Rules.mk                      | 40 ++++++++++++--------------
 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 +-
 29 files changed, 121 insertions(+), 66 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 v4 1/6] build: convert debug to Kconfig

[Doug Goldstein]

Enabling debug will disable NDEBUG which will result in more debug
prints.  There are a number of debugging options for Xen so place the
debug option under a menu for different debugging options to have a way
to group them all together.

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        | 13 +++++++++++++
 xen/Rules.mk             |  5 +++--
 xen/include/xen/config.h |  4 ++++
 4 files changed, 22 insertions(+), 2 deletions(-)
 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..b446027
--- /dev/null
+++ b/xen/Kconfig.debug
@@ -0,0 +1,13 @@
+
+menu "Debugging Options"
+
+config DEBUG
+	bool "Developer Checks"
+	---help---
+	  Enables developer checks such as asserts and extra printks, this
+	  option is intended for development purposes only, and not for
+	  production use.
+
+	  You probably want to say 'N' here.
+
+endmenu
diff --git a/xen/Rules.mk b/xen/Rules.mk
index 961d533..86c1e0d 100644
--- a/xen/Rules.mk
+++ b/xen/Rules.mk
@@ -20,13 +20,14 @@ include $(XEN_ROOT)/Config.mk
 ifeq ($(debug),y)
 verbose       := y
 frame_pointer := y
-else
-CFLAGS += -DNDEBUG
 endif
 ifeq ($(perfc_arrays),y)
 perfc := y
 endif
 
+ifeq ($(origin debug),command line)
+$(error "You must use 'make menuconfig' to enable/disable debug now.")
+endif
 ifneq ($(origin kexec),undefined)
 $(error "You must use 'make menuconfig' to enable/disable kexec now.")
 endif
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 v4 2/6] 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: 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 -
 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 b446027..308911a 100644
--- a/xen/Kconfig.debug
+++ b/xen/Kconfig.debug
@@ -10,4 +10,15 @@ config DEBUG
 
 	  You probably want to say 'N' here.
 
+if DEBUG || EXPERT = "y"
+
+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 || EXPERT
+
 endmenu
diff --git a/xen/Rules.mk b/xen/Rules.mk
index 86c1e0d..45aad61 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
 
@@ -25,6 +24,9 @@ ifeq ($(perfc_arrays),y)
 perfc := y
 endif
 
+ifneq ($(origin crash_debug),undefined)
+$(error "You must use 'make menuconfig' to enable/disable crash_debug now.")
+endif
 ifeq ($(origin debug),command line)
 $(error "You must use 'make menuconfig' to enable/disable debug now.")
 endif
@@ -59,7 +61,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..271cbaa 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)
+#ifdef 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 v4 3/6] 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                | 6 +++---
 xen/arch/arm/kernel.c       | 2 +-
 xen/arch/x86/domain_build.c | 2 +-
 xen/include/xsm/dummy.h     | 2 +-
 6 files changed, 13 insertions(+), 7 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 308911a..7c115b0 100644
--- a/xen/Kconfig.debug
+++ b/xen/Kconfig.debug
@@ -19,6 +19,13 @@ 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 DEBUG
+	---help---
+	  Guest output from HYPERVISOR_console_io and hypervisor parsing
+	  ELF images (dom0) is logged in the Xen ring buffer.
+
 endif # DEBUG || EXPERT
 
 endmenu
diff --git a/xen/Rules.mk b/xen/Rules.mk
index 45aad61..55096d3 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,7 +16,6 @@ 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
 endif
 ifeq ($(perfc_arrays),y)
@@ -33,6 +31,9 @@ endif
 ifneq ($(origin kexec),undefined)
 $(error "You must use 'make menuconfig' to enable/disable kexec now.")
 endif
+ifneq ($(origin verbose),undefined)
+$(error "You must use 'make menuconfig' to enable/disable verbose now.")
+endif
 
 # Set ARCH/SUBARCH appropriately.
 override TARGET_SUBARCH  := $(XEN_TARGET_ARCH)
@@ -60,7 +61,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 v4 4/6] 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      | 9 ++++-----
 3 files changed, 12 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 7c115b0..f4eb815 100644
--- a/xen/Kconfig.debug
+++ b/xen/Kconfig.debug
@@ -19,6 +19,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 DEBUG
+	---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 DEBUG
diff --git a/xen/Rules.mk b/xen/Rules.mk
index 55096d3..ab6f7fc 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
@@ -28,6 +24,9 @@ endif
 ifeq ($(origin debug),command line)
 $(error "You must use 'make menuconfig' to enable/disable debug now.")
 endif
+ifneq ($(origin frame_pointer),undefined)
+$(error "You must use 'make menuconfig' to enable/disable frame_pointer now.")
+endif
 ifneq ($(origin kexec),undefined)
 $(error "You must use 'make menuconfig' to enable/disable kexec now.")
 endif
@@ -64,7 +63,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 v4 5/6] 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.

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                           |  2 --
 xen/Kconfig.debug                 | 14 ++++++++++++++
 xen/Rules.mk                      | 12 +++---------
 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, 32 insertions(+), 26 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 f4eb815..0678049 100644
--- a/xen/Kconfig.debug
+++ b/xen/Kconfig.debug
@@ -27,6 +27,20 @@ config FRAME_POINTER
 	  maybe slower, but it gives very useful debugging information
 	  in case of any Xen bugs.
 
+config PERF_COUNTERS
+	bool "Performance Counters"
+	---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 PERF_ARRAYS
+	bool "Performance Counter Array Histograms"
+	depends on PERF_COUNTERS
+	---help---
+	  Enables software performance counter array histograms.
+
+
 config VERBOSE_DEBUG
 	bool "Verbose debug messages"
 	default DEBUG
diff --git a/xen/Rules.mk b/xen/Rules.mk
index ab6f7fc..6a756f6 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,11 +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 crash_debug),undefined)
 $(error "You must use 'make menuconfig' to enable/disable crash_debug now.")
@@ -30,6 +23,9 @@ endif
 ifneq ($(origin kexec),undefined)
 $(error "You must use 'make menuconfig' to enable/disable kexec now.")
 endif
+ifneq ($(origin perfc),undefined)
+$(error "You must use 'make menuconfig' to enable/disable perfc now.")
+endif
 ifneq ($(origin verbose),undefined)
 $(error "You must use 'make menuconfig' to enable/disable verbose now.")
 endif
@@ -60,8 +56,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 5040a5c..a4c20cd 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -3536,7 +3536,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 a3ae7a4..05d2b85 100644
--- a/xen/arch/x86/x86_64/asm-offsets.c
+++ b/xen/arch/x86/x86_64/asm-offsets.c
@@ -152,7 +152,7 @@ void __dummy__(void)
     OFFSET(TRAPBOUNCE_eip, struct trap_bounce, eip);
     BLANK();
 
-#if PERF_COUNTERS
+#ifdef 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..3baddb4 100644
--- a/xen/common/Makefile
+++ b/xen/common/Makefile
@@ -26,6 +26,7 @@ obj-y += multicall.o
 obj-y += notifier.o
 obj-y += page_alloc.o
 obj-$(CONFIG_HAS_PDX) += pdx.o
+obj-$(CONFIG_PERF_COUNTERS) += perfc.o
 obj-y += preempt.o
 obj-y += random.o
 obj-y += rangeset.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-$(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 279d702..e36e78f 100644
--- a/xen/include/asm-x86/asm_defns.h
+++ b/xen/include/asm-x86/asm_defns.h
@@ -385,7 +385,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 fe15e9c..46c82e7 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -39,7 +39,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 v4 6/6] 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          |  7 +++++++
 xen/Rules.mk               |  5 +++--
 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, 22 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 0678049..303bf36 100644
--- a/xen/Kconfig.debug
+++ b/xen/Kconfig.debug
@@ -27,6 +27,13 @@ config FRAME_POINTER
 	  maybe slower, but it gives very useful debugging information
 	  in case of any Xen bugs.
 
+config LOCK_PROFILE
+	bool "Lock Profiiling"
+	---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_COUNTERS
 	bool "Performance Counters"
 	---help---
diff --git a/xen/Rules.mk b/xen/Rules.mk
index 6a756f6..23c9181 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
@@ -23,6 +22,9 @@ endif
 ifneq ($(origin kexec),undefined)
 $(error "You must use 'make menuconfig' to enable/disable kexec now.")
 endif
+ifneq ($(origin lock_profile),undefined)
+$(error "You must use 'make menuconfig' to enable/disable lock_profile now.")
+endif
 ifneq ($(origin perfc),undefined)
 $(error "You must use 'make menuconfig' to enable/disable perfc now.")
 endif
@@ -56,7 +58,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 v4 0/6] Kconfig debug options

[Andrew Cooper]

On 22/05/16 06:01, Doug Goldstein wrote:
> This converts the debug options from xen/Rules.mk to Kconfig. Hopefully
> I haven't missed anything in the back and forth.
>
> change since v3:
> - wrap all options in 'if DEBUG || EXPERT' (except DEBUG)
> - wording update to DEBUG option and some commit messages
> - all old command line options now complain
> change since v2:
> - dropped patch 5 as it was unwanted
> - remove 'default n'
> - redid patch 1
>
>
> *** BLURB HERE ***
>
> Doug Goldstein (6):
>   build: convert debug to Kconfig
>   build: convert crash_debug to Kconfig
>   build: convert verbose to Kconfig
>   build: convert frame_pointer to Kconfig
>   build: convert perfc{,_arrays} to Kconfig
>   build: convert lock_profile to Kconfig

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

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

Re: [PATCH v4 1/6] build: convert debug to Kconfig

[Doug Goldstein]

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

On 5/22/16 12:01 AM, Doug Goldstein wrote:
> Enabling debug will disable NDEBUG which will result in more debug
> prints.  There are a number of debugging options for Xen so place the
> debug option under a menu for different debugging options to have a way
> to group them all together.
> 
> Signed-off-by: Doug Goldstein <cardoe@cardoe.com>

... snip ...

> diff --git a/xen/Kconfig.debug b/xen/Kconfig.debug
> new file mode 100644
> index 0000000..b446027
> --- /dev/null
> +++ b/xen/Kconfig.debug
> @@ -0,0 +1,13 @@
> +
> +menu "Debugging Options"
> +
> +config DEBUG
> +	bool "Developer Checks"

Add the following when committing:

	default y

if you want debug to be on by default.

> +	---help---
> +	  Enables developer checks such as asserts and extra printks, this
> +	  option is intended for development purposes only, and not for
> +	  production use.
> +
> +	  You probably want to say 'N' here.



-- 
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 v4 1/6] build: convert debug to Kconfig

[Jan Beulich]

>>> On 22.05.16 at 21:04, <cardoe@cardoe.com> wrote:
> On 5/22/16 12:01 AM, Doug Goldstein wrote:
>> --- /dev/null
>> +++ b/xen/Kconfig.debug
>> @@ -0,0 +1,13 @@
>> +
>> +menu "Debugging Options"
>> +
>> +config DEBUG
>> +	bool "Developer Checks"
> 
> Add the following when committing:
> 
> 	default y
> 
> if you want debug to be on by default.

Indeed we definitely want this in other than stable (or late -rc) trees.

Jan


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

Re: [PATCH v4 1/6] build: convert debug to Kconfig

[Jan Beulich]

>>> On 22.05.16 at 07:01, <cardoe@cardoe.com> wrote:
> --- /dev/null
> +++ b/xen/Kconfig.debug
> @@ -0,0 +1,13 @@
> +
> +menu "Debugging Options"
> +
> +config DEBUG
> +	bool "Developer Checks"
> +	---help---
> +	  Enables developer checks such as asserts and extra printks, this
> +	  option is intended for development purposes only, and not for
> +	  production use.

I'm not a native speaker, but it seems to me that either this wants to
start "Enabling ..." or the first comma wants to become a stop.

> --- a/xen/Rules.mk
> +++ b/xen/Rules.mk
> @@ -20,13 +20,14 @@ include $(XEN_ROOT)/Config.mk
>  ifeq ($(debug),y)

This being left in place suggests ...

>  verbose       := y
>  frame_pointer := y
> -else
> -CFLAGS += -DNDEBUG
>  endif
>  ifeq ($(perfc_arrays),y)
>  perfc := y
>  endif
>  
> +ifeq ($(origin debug),command line)
> +$(error "You must use 'make menuconfig' to enable/disable debug now.")
> +endif

... that this is at least premature. "At least" because: Didn't you or
someone else point out that this is then still used for controlling the
tool stack part of the build?

Jan


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

Re: [PATCH v4 0/6] Kconfig debug options

[Jan Beulich]

>>> On 22.05.16 at 07:01, <cardoe@cardoe.com> wrote:
> This converts the debug options from xen/Rules.mk to Kconfig. Hopefully
> I haven't missed anything in the back and forth.
> 
> change since v3:
> - wrap all options in 'if DEBUG || EXPERT' (except DEBUG)
> - wording update to DEBUG option and some commit messages
> - all old command line options now complain

Patches 2...5:
Reviewed-by: Jan Beulich <jbeulich@suse.com>


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

Re: [PATCH v4 3/6] build: convert verbose to Kconfig

[Konrad Rzeszutek Wilk]

>  
> +config VERBOSE_DEBUG
> +	bool "Verbose debug messages"
> +	default DEBUG
> +	---help---
> +	  Guest output from HYPERVISOR_console_io and hypervisor parsing
> +	  ELF images (dom0) is logged in the Xen ring buffer.

is logged?

or will be logged?


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

Re: [PATCH v4 1/6] build: convert debug to Kconfig

[Andrew Cooper]

On 23/05/16 13:58, Jan Beulich wrote:
>>>> On 22.05.16 at 07:01, <cardoe@cardoe.com> wrote:
>> --- /dev/null
>> +++ b/xen/Kconfig.debug
>> @@ -0,0 +1,13 @@
>> +
>> +menu "Debugging Options"
>> +
>> +config DEBUG
>> +	bool "Developer Checks"
>> +	---help---
>> +	  Enables developer checks such as asserts and extra printks, this
>> +	  option is intended for development purposes only, and not for
>> +	  production use.
> I'm not a native speaker, but it seems to me that either this wants to
> start "Enabling ..." or the first comma wants to become a stop.

The first comma should become a full stop.

Changing Enables to Enabling still results in a particularly odd sentence.

~Andrew

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

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

[Konrad Rzeszutek Wilk]

> +config PERF_COUNTERS
> +	bool "Performance Counters"
> +	---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 would reword this as :

"To access this data you can use serial console to print (and reset) using 'p' and 'P'
respectively, or the 'xenperf' tool."

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

Re: [PATCH v4 2/6] build: convert crash_debug to Kconfig

[Konrad Rzeszutek Wilk]

> +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

s/be able//

> +	  Xen if it crashes then say Y.
> +

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

Re: [PATCH v4 0/6] Kconfig debug options

[Konrad Rzeszutek Wilk]

On Sun, May 22, 2016 at 12:33:23PM +0100, Andrew Cooper wrote:
> On 22/05/16 06:01, Doug Goldstein wrote:
> > This converts the debug options from xen/Rules.mk to Kconfig. Hopefully
> > I haven't missed anything in the back and forth.
> >
> > change since v3:
> > - wrap all options in 'if DEBUG || EXPERT' (except DEBUG)
> > - wording update to DEBUG option and some commit messages
> > - all old command line options now complain
> > change since v2:
> > - dropped patch 5 as it was unwanted
> > - remove 'default n'
> > - redid patch 1
> >
> >
> > *** BLURB HERE ***
> >
> > Doug Goldstein (6):
> >   build: convert debug to Kconfig
> >   build: convert crash_debug to Kconfig
> >   build: convert verbose to Kconfig
> >   build: convert frame_pointer to Kconfig
> >   build: convert perfc{,_arrays} to Kconfig
> >   build: convert lock_profile to Kconfig
> 
> All Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

With the little nitpicks I pointed out - and the #1 redo as Jan
pointed out:

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

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

Re: [PATCH v4 6/6] build: convert lock_profile to Kconfig

[Julien Grall]

Hi Doug,

On 22/05/16 06:01, Doug Goldstein wrote:
> diff --git a/xen/Kconfig.debug b/xen/Kconfig.debug
> index 0678049..303bf36 100644
> --- a/xen/Kconfig.debug
> +++ b/xen/Kconfig.debug
> @@ -27,6 +27,13 @@ config FRAME_POINTER
>   	  maybe slower, but it gives very useful debugging information
>   	  in case of any Xen bugs.
>
> +config LOCK_PROFILE
> +	bool "Lock Profiiling"

NIT: s/Profiliiling/profiling/

> +	---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.
> +

Regards,

-- 
Julien Grall

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

Re: [PATCH v4 1/6] build: convert debug to Kconfig

[Doug Goldstein]

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

On 5/23/16 7:58 AM, Jan Beulich wrote:
>>>> On 22.05.16 at 07:01, <cardoe@cardoe.com> wrote:

>>  verbose       := y
>>  frame_pointer := y
>> -else
>> -CFLAGS += -DNDEBUG
>>  endif
>>  ifeq ($(perfc_arrays),y)
>>  perfc := y
>>  endif
>>  
>> +ifeq ($(origin debug),command line)
>> +$(error "You must use 'make menuconfig' to enable/disable debug now.")
>> +endif
> 
> ... that this is at least premature. "At least" because: Didn't you or
> someone else point out that this is then still used for controlling the
> tool stack part of the build?
> 
> Jan
> 

I purposefully just check for "command line" in this case. So if someone
does "make -C xen debug=[y|n]" or "cd xen && make debug=[y|n]" then that
should be the only time they get that message.

The other cases you'll see the check is different and would catch the
debug value from the top level.

If that's not what's intended please let me know and I can strip this
bit out. You had mentioned the 'debug' option in
http://lists.xenproject.org/archives/html/xen-devel/2016-05/msg01027.html
which is why I tried to come up with a message that I thought would be
more limited and then hopefully not premature.

-- 
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 v4 1/6] build: convert debug to Kconfig

[Doug Goldstein]

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

On 5/23/16 8:19 AM, Andrew Cooper wrote:
> On 23/05/16 13:58, Jan Beulich wrote:
>>>>> On 22.05.16 at 07:01, <cardoe@cardoe.com> wrote:
>>> --- /dev/null
>>> +++ b/xen/Kconfig.debug
>>> @@ -0,0 +1,13 @@
>>> +
>>> +menu "Debugging Options"
>>> +
>>> +config DEBUG
>>> +	bool "Developer Checks"
>>> +	---help---
>>> +	  Enables developer checks such as asserts and extra printks, this
>>> +	  option is intended for development purposes only, and not for
>>> +	  production use.
>> I'm not a native speaker, but it seems to me that either this wants to
>> start "Enabling ..." or the first comma wants to become a stop.
> 
> The first comma should become a full stop.
> 
> Changing Enables to Enabling still results in a particularly odd sentence.
> 
> ~Andrew
> 

Looks like I glued together a few suggestions between the different
reposts and never really read it. This is what I've got now:

If you say Y here this will enable developer checks such as asserts
and extra printks. This option is intended for development purposes
only, and not for production use.

You probably want to say 'N' here.

-- 
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 v4 1/6] build: convert debug to Kconfig

[Doug Goldstein]

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

On 5/23/16 3:39 AM, Jan Beulich wrote:
>>>> On 22.05.16 at 21:04, <cardoe@cardoe.com> wrote:
>> On 5/22/16 12:01 AM, Doug Goldstein wrote:
>>> --- /dev/null
>>> +++ b/xen/Kconfig.debug
>>> @@ -0,0 +1,13 @@
>>> +
>>> +menu "Debugging Options"
>>> +
>>> +config DEBUG
>>> +	bool "Developer Checks"
>>
>> Add the following when committing:
>>
>> 	default y
>>
>> if you want debug to be on by default.
> 
> Indeed we definitely want this in other than stable (or late -rc) trees.
> 
> Jan
> 

Since we're now for sure sticking this into 4.8, I've squashed that in.

-- 
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 v4 2/6] build: convert crash_debug to Kconfig

[Doug Goldstein]

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

On 5/23/16 8:22 AM, Konrad Rzeszutek Wilk wrote:
>> +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
> 
> s/be able//
> 

I'll drop the extra 'to' as well. ;)

"If you want to attach gdb to Xen to debug"

-- 
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 v4 3/6] build: convert verbose to Kconfig

[Doug Goldstein]

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

On 5/23/16 8:19 AM, Konrad Rzeszutek Wilk wrote:
>>  
>> +config VERBOSE_DEBUG
>> +	bool "Verbose debug messages"
>> +	default DEBUG
>> +	---help---
>> +	  Guest output from HYPERVISOR_console_io and hypervisor parsing
>> +	  ELF images (dom0) is logged in the Xen ring buffer.
> 
> is logged?
> 
> or will be logged?
> 

"will be logged" sounds better. thanks.

-- 
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 v4 6/6] build: convert lock_profile to Kconfig

[Doug Goldstein]

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

On 5/23/16 10:53 AM, Julien Grall wrote:
> Hi Doug,
> 
> On 22/05/16 06:01, Doug Goldstein wrote:
>> diff --git a/xen/Kconfig.debug b/xen/Kconfig.debug
>> index 0678049..303bf36 100644
>> --- a/xen/Kconfig.debug
>> +++ b/xen/Kconfig.debug
>> @@ -27,6 +27,13 @@ config FRAME_POINTER
>>         maybe slower, but it gives very useful debugging information
>>         in case of any Xen bugs.
>>
>> +config LOCK_PROFILE
>> +    bool "Lock Profiiling"
> 
> NIT: s/Profiliiling/profiling/
> 

Thank you. Missed that through all the rounds.

-- 
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 v4 1/6] build: convert debug to Kconfig

[Jan Beulich]

>>> On 24.05.16 at 03:03, <cardoe@cardoe.com> wrote:
> On 5/23/16 7:58 AM, Jan Beulich wrote:
>>>>> On 22.05.16 at 07:01, <cardoe@cardoe.com> wrote:
> 
>>>  verbose       := y
>>>  frame_pointer := y
>>> -else
>>> -CFLAGS += -DNDEBUG
>>>  endif
>>>  ifeq ($(perfc_arrays),y)
>>>  perfc := y
>>>  endif
>>>  
>>> +ifeq ($(origin debug),command line)
>>> +$(error "You must use 'make menuconfig' to enable/disable debug now.")
>>> +endif
>> 
>> ... that this is at least premature. "At least" because: Didn't you or
>> someone else point out that this is then still used for controlling the
>> tool stack part of the build?
> 
> I purposefully just check for "command line" in this case. So if someone
> does "make -C xen debug=[y|n]" or "cd xen && make debug=[y|n]" then that
> should be the only time they get that message.

But doesn't a "debug=[y|n]" on a top level make command line
trickle down as a command line variable too? In which case your
check would prevent someone forcing that setting to a specific
value for the tools (and other?) subtree(s).

> The other cases you'll see the check is different and would catch the
> debug value from the top level.
> 
> If that's not what's intended please let me know and I can strip this
> bit out. You had mentioned the 'debug' option in
> http://lists.xenproject.org/archives/html/xen-devel/2016-05/msg01027.html 
> which is why I tried to come up with a message that I thought would be
> more limited and then hopefully not premature.

Well, yes, I did mention debug there, but iirc the discussion about
debug having a meaning outside of xen/ was on another subthread,
and needs to be taken into account here. Bottom line - I'd be fine
with the check left in place if indeed it won't harm any possible top
level make invocation. Otherwise I think this would at least need to
be relaxed to $(warning ...).

Jan


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