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

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Thu, Mar 24, 2016 at 11:48:19AM -0500, Doug Goldstein 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>
> ---
> This is an example of using the debug menu.
> 
> 
> CC: Keir Fraser <keir@xen.org>
> CC: Jan Beulich <jbeulich@suse.com>
> CC: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
>  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 +-
>  8 files changed, 21 insertions(+), 10 deletions(-)
> 
> 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 36890bd..3336a64 100644
> --- a/xen/Kconfig.debug
> +++ b/xen/Kconfig.debug
> @@ -4,3 +4,14 @@ menuconfig DEBUG
>  	---help---
>  	  If you want to debug Xen say Y and select any additional debugging
>  	  support options.
> +
> +if DEBUG

Perhaps if !defined then atuomatically enable it? Looking at Config.mk
it seems you could do crash_debug without debug=y?

Why is this called crash_debug and not 'crash_gdb' ..? Ah [after reading the docs]
it can't do breakpoints or any of that. Just to pour over the data after a crash.
Ah, you can resume the hypervisor after this. OK, definitly not for field
analysis.

And kexec/kdump is much more powerful than this.


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

Should it have a link to the docs ? On how to use it?

And maybe mention that kexec/kdump is better suited for capturing the whole
machine and one can do post-mortem analysis much more intensly?

> +
> +endif # DEBUG
> diff --git a/xen/Rules.mk b/xen/Rules.mk
> index f29491e..b5d8d33 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)
> @@ -53,7 +55,6 @@ CFLAGS += -pipe -g -D__XEN__ -include $(BASEDIR)/include/xen/config.h
>  CFLAGS += '-D__OBJECT_FILE__="$@"'
>  
>  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 1bcb08b..21d7e5f 100644
> --- a/xen/arch/x86/Makefile
> +++ b/xen/arch/x86/Makefile
> @@ -24,6 +24,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
> @@ -62,8 +63,6 @@ obj-y += hpet.o
>  obj-y += vm_event.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 77de27e..39a83fc 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
> @@ -61,7 +62,6 @@ obj-y += xmalloc_tlsf.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

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