Kip Macy
2005-May-16 01:25 UTC
[Xen-devel] [PATCH] make domu_debug run-time option + fix int3 handling for MP
I''m not entirely thrilled with the interfaces used to get the domu_debug option into xen, but adding yet another argument xc_linux_build and/or adding a separate handler for domu_debug didn''t seem that clean either. The debug support doesn''t seem like something Ron would use, so I didn''t update the Plan 9 builder. Feedback would be appreciated. # This is a BitKeeper generated diff -Nru style patch. # # ChangeSet # 2005/05/15 18:19:15-07:00 kmacy@octopus.eng.netapp.com # make domu_debug domain-builder/run-time option # pause all vcpus on int3 / trace trap # make domu_debug and cdb not be mutually exclusive # Signed-off-by: Kip Macy <kmacy@netapp.com> # # xen/include/xen/sched.h # 2005/05/15 18:19:13-07:00 kmacy@octopus.eng.netapp.com +3 -0 # add DOMF_debug flag # # xen/include/public/dom0_ops.h # 2005/05/15 18:19:13-07:00 kmacy@octopus.eng.netapp.com +2 -0 # add flags field to setdomaininfo to enable domu_debug # # xen/include/asm-x86/debugger.h # 2005/05/15 18:19:13-07:00 kmacy@octopus.eng.netapp.com +24 -24 # pause vcpus on int3 or trace trap # enable domu_debug by default # make cdb and domu_debug not be mutually exclusive # # xen/common/domain.c # 2005/05/15 18:19:13-07:00 kmacy@octopus.eng.netapp.com +5 -0 # toggle DOMF_debug depending on value passed with # setdomaininfo # # xen/Rules.mk # 2005/05/15 18:19:12-07:00 kmacy@octopus.eng.netapp.com +0 -5 # remove domu_debug from compile # # tools/python/xen/xm/create.py # 2005/05/15 18:19:12-07:00 kmacy@octopus.eng.netapp.com +7 -0 # add domu_debug option # # tools/python/xen/xend/XendDomainInfo.py # 2005/05/15 18:19:12-07:00 kmacy@octopus.eng.netapp.com +7 -0 # pass debug flag to domain builder if option set # # tools/libxc/xc_vmx_build.c # 2005/05/15 18:19:12-07:00 kmacy@octopus.eng.netapp.com +7 -1 # pass debug flags to setdomaininfo # # tools/libxc/xc_linux_build.c # 2005/05/15 18:19:12-07:00 kmacy@octopus.eng.netapp.com +7 -2 # pass debug flags to setdomaininfo # diff -Nru a/tools/libxc/xc_linux_build.c b/tools/libxc/xc_linux_build.c --- a/tools/libxc/xc_linux_build.c 2005-05-14 18:24:04 -07:00 +++ b/tools/libxc/xc_linux_build.c 2005-05-14 18:24:04 -07:00 @@ -316,12 +316,16 @@ int initrd_fd = -1; gzFile initrd_gfd = NULL; int rc, i; + unsigned int debug_flags; vcpu_guest_context_t st_ctxt, *ctxt = &st_ctxt; unsigned long nr_pages; char *image = NULL; - unsigned long image_size, initrd_size=0; + unsigned long si_flags, image_size, initrd_size=0; unsigned long vstartinfo_start, vkern_entry; + si_flags = flags & ~DOMFLAGS_DOMU_DEBUG; + debug_flags = flags & DOMFLAGS_DOMU_DEBUG; + if ( (nr_pages = xc_get_tot_pages(xc_handle, domid)) < 0 ) { PERROR("Could not find total pages for domain"); @@ -381,7 +385,7 @@ &vstartinfo_start, &vkern_entry, ctxt, cmdline, op.u.getdomaininfo.shared_info_frame, - control_evtchn, flags, vcpus) < 0 ) + control_evtchn, si_flags, vcpus) < 0 ) { ERROR("Error constructing guest OS"); goto error_out; @@ -458,6 +462,7 @@ memset( &launch_op, 0, sizeof(launch_op) ); launch_op.u.setdomaininfo.domain = (domid_t)domid; + launch_op.u.setdomaininfo.flags = debug_flags; launch_op.u.setdomaininfo.vcpu = 0; launch_op.u.setdomaininfo.ctxt = ctxt; diff -Nru a/tools/libxc/xc_vmx_build.c b/tools/libxc/xc_vmx_build.c --- a/tools/libxc/xc_vmx_build.c 2005-05-14 18:24:04 -07:00 +++ b/tools/libxc/xc_vmx_build.c 2005-05-14 18:24:04 -07:00 @@ -498,11 +498,16 @@ int initrd_fd = -1; gzFile initrd_gfd = NULL; int rc, i; + unsigned long si_flags; + unsigned int debug_flags; vcpu_guest_context_t st_ctxt, *ctxt = &st_ctxt; unsigned long nr_pages; char *image = NULL; unsigned long image_size, initrd_size=0; + debug_flags = flags & DOMFLAGS_DOMU_DEBUG; + si_flags = flags & ~DOMFLAGS_DOMU_DEBUG; + if ( vmx_identify() < 0 ) { PERROR("CPU doesn''t support VMX Extensions"); @@ -567,7 +572,7 @@ initrd_gfd, initrd_size, nr_pages, ctxt, cmdline, op.u.getdomaininfo.shared_info_frame, - control_evtchn, flags, mem_mapp) < 0 ) + control_evtchn, si_flags, mem_mapp) < 0 ) { ERROR("Error constructing guest OS"); goto error_out; @@ -624,6 +629,7 @@ memset( &launch_op, 0, sizeof(launch_op) ); launch_op.u.setdomaininfo.domain = (domid_t)domid; + launch_op.u.setdomaininfo.flags = debug_flags; launch_op.u.setdomaininfo.vcpu = 0; launch_op.u.setdomaininfo.ctxt = ctxt; diff -Nru a/tools/python/xen/xend/XendDomainInfo.py b/tools/python/xen/xend/XendDomainInfo.py --- a/tools/python/xen/xend/XendDomainInfo.py 2005-05-14 18:24:04 -07:00 +++ b/tools/python/xen/xend/XendDomainInfo.py 2005-05-14 18:24:04 -07:00 @@ -36,6 +36,9 @@ """Flag for a net device backend domain.""" SIF_NET_BE_DOMAIN = (1<<5) +"""Flag for enable unprivileged domain debugging.""" +DOMFLAGS_DOMU_DEBUG = (1<<6) + """Shutdown code for poweroff.""" DOMAIN_POWEROFF = 0 @@ -272,6 +275,7 @@ self.ramdisk = None self.cmdline = None + self.domu_debug = 0 self.channel = None self.controllers = {} @@ -711,6 +715,8 @@ flags = 0 if self.netif_backend: flags |= SIF_NET_BE_DOMAIN if self.blkif_backend: flags |= SIF_BLK_BE_DOMAIN + if self.domu_debug: flags |= DOMFLAGS_DOMU_DEBUG + #todo generalise this if ostype == "vmx": err = buildfn(dom = dom, @@ -1114,6 +1120,7 @@ if args: cmdline += " " + args ramdisk = sxp.child_value(image, "ramdisk", '''') + vm.domu_debug = sxp.child_value(image, "domu_debug", 0) log.debug("creating linux domain with cmdline: %s" %(cmdline,)) vm.create_domain("linux", kernel, ramdisk, cmdline) return vm diff -Nru a/tools/python/xen/xm/create.py b/tools/python/xen/xm/create.py --- a/tools/python/xen/xm/create.py 2005-05-14 18:24:04 -07:00 +++ b/tools/python/xen/xm/create.py 2005-05-14 18:24:04 -07:00 @@ -40,6 +40,10 @@ fn=set_true, default=0, use="Quiet.") +gopts.opt(''domu_debug'', short=''g'', + fn=set_true, default=0, + use="Debug.") + gopts.opt(''path'', val=''PATH'', fn=set_value, default=''.:/etc/xen'', use="""Search path for configuration scripts. @@ -279,6 +283,9 @@ config_image.append([''args'', vals.extra]) if vals.vcpus: config_image.append([''vcpus'', vals.vcpus]) + if vals.domu_debug: + config_image.append([''domu_debug'', vals.domu_debug]) + config.append([''image'', config_image ]) diff -Nru a/xen/Rules.mk b/xen/Rules.mk --- a/xen/Rules.mk 2005-05-14 18:24:04 -07:00 +++ b/xen/Rules.mk 2005-05-14 18:24:04 -07:00 @@ -5,7 +5,6 @@ perfc_arrays?= n trace ?= n optimize ?= y -domu_debug ?= n crash_debug ?= n include $(BASEDIR)/../Config.mk @@ -43,10 +42,6 @@ endif else CFLAGS += -g -DVERBOSE -endif - -ifeq ($(domu_debug),y) -CFLAGS += -DDOMU_DEBUG endif ifeq ($(crash_debug),y) diff -Nru a/xen/common/domain.c b/xen/common/domain.c --- a/xen/common/domain.c 2005-05-14 18:24:04 -07:00 +++ b/xen/common/domain.c 2005-05-14 18:24:04 -07:00 @@ -227,6 +227,11 @@ if ( (vcpu >= MAX_VIRT_CPUS) || ((ed = d->exec_domain[vcpu]) == NULL) ) return -EINVAL; + if (setdomaininfo->flags & DOMFLAGS_DOMU_DEBUG) + set_bit(_DOMF_debug, &d->domain_flags); + else + clear_bit(_DOMF_debug, &d->domain_flags); + if (test_bit(_DOMF_constructed, &d->domain_flags) && !test_bit(_VCPUF_ctrl_pause, &ed->vcpu_flags)) return -EINVAL; diff -Nru a/xen/include/asm-x86/debugger.h b/xen/include/asm-x86/debugger.h --- a/xen/include/asm-x86/debugger.h 2005-05-14 18:24:04 -07:00 +++ b/xen/include/asm-x86/debugger.h 2005-05-14 18:24:04 -07:00 @@ -38,38 +38,26 @@ #define DEBUGGER_trap_fatal(_v, _r) \ if ( debugger_trap_fatal(_v, _r) ) return EXCRET_fault_fixed; -#if defined(CRASH_DEBUG) -extern int __trap_to_cdb(struct cpu_user_regs *r); -#define debugger_trap_entry(_v, _r) (0) -static inline int debugger_trap_fatal( - unsigned int vector, struct cpu_user_regs *regs) -{ - (void)__trap_to_cdb(regs); - return (vector == TRAP_int3); /* int3 is harmless */ -} - -/* Int3 is a trivial way to gather cpu_user_regs context. */ -#define debugger_trap_immediate() __asm__ __volatile__ ( "int3" ); - -#elif defined(DOMU_DEBUG) #include <xen/softirq.h> static inline int debugger_trap_entry( unsigned int vector, struct cpu_user_regs *regs) { - struct exec_domain *ed = current; + struct exec_domain *ted, *ed = current; - if ( !KERNEL_MODE(ed, regs) || (ed->domain->domain_id == 0) ) + if ( !KERNEL_MODE(ed, regs) || (ed->domain->domain_id == 0) || + !test_bit(_DOMF_debug, &ed->domain->domain_flags) ) return 0; switch ( vector ) { case TRAP_int3: case TRAP_debug: - set_bit(_VCPUF_ctrl_pause, &ed->vcpu_flags); + for_each_exec_domain ( ed->domain, ted ) + set_bit(_VCPUF_ctrl_pause, &ted->vcpu_flags); raise_softirq(SCHEDULE_SOFTIRQ); return 1; } @@ -77,11 +65,29 @@ return 0; } + +#if defined(CRASH_DEBUG) + +extern int __trap_to_cdb(struct cpu_user_regs *r); + +static inline int debugger_trap_fatal( + unsigned int vector, struct cpu_user_regs *regs) +{ + (void)__trap_to_cdb(regs); + return (vector == TRAP_int3); /* int3 is harmless */ +} + +/* Int3 is a trivial way to gather cpu_user_regs context. */ +#define debugger_trap_immediate() __asm__ __volatile__ ( "int3" ); + +#else + #define debugger_trap_fatal(_v, _r) (0) #define debugger_trap_immediate() +#endif -#elif 0 +#if 0 extern int kdb_trap(int, int, struct cpu_user_regs *); @@ -99,12 +105,6 @@ /* Int3 is a trivial way to gather cpu_user_regs context. */ #define debugger_trap_immediate() __asm__ __volatile__ ( "int3" ); - -#else - -#define debugger_trap_entry(_v, _r) (0) -#define debugger_trap_fatal(_v, _r) (0) -#define debugger_trap_immediate() #endif diff -Nru a/xen/include/public/dom0_ops.h b/xen/include/public/dom0_ops.h --- a/xen/include/public/dom0_ops.h 2005-05-14 18:24:04 -07:00 +++ b/xen/include/public/dom0_ops.h 2005-05-14 18:24:04 -07:00 @@ -77,6 +77,7 @@ #define DOMFLAGS_PAUSED (1<<3) /* Currently paused by control software. */ #define DOMFLAGS_BLOCKED (1<<4) /* Currently blocked pending an event. */ #define DOMFLAGS_RUNNING (1<<5) /* Domain is currently running. */ +#define DOMFLAGS_DOMU_DEBUG (1<<6) /* Pause domain on int3 or trace trap. */ #define DOMFLAGS_CPUMASK 255 /* CPU to which this domain is bound. */ #define DOMFLAGS_CPUSHIFT 8 #define DOMFLAGS_SHUTDOWNMASK 255 /* DOMFLAGS_SHUTDOWN guest-supplied code. */ @@ -95,6 +96,7 @@ typedef struct { /* IN variables. */ domid_t domain; + u32 flags; u16 vcpu; /* IN/OUT parameters */ vcpu_guest_context_t *ctxt; diff -Nru a/xen/include/xen/sched.h b/xen/include/xen/sched.h --- a/xen/include/xen/sched.h 2005-05-14 18:24:04 -07:00 +++ b/xen/include/xen/sched.h 2005-05-14 18:24:04 -07:00 @@ -383,6 +383,9 @@ /* Death rattle. */ #define _DOMF_dying 6 #define DOMF_dying (1UL<<_DOMF_dying) + /* PAUSE on int3 or trace trap. */ +#define _DOMF_debug 7 +#define DOMF_debug (1UL<<_DOMF_debug) static inline int domain_runnable(struct exec_domain *ed) { _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Kip Macy
2005-May-16 19:07 UTC
[Xen-devel] possible changes was Re: [PATCH] make domu_debug run-time option + fix int3 handling for MP
I can chunk this into 3 or 4 patches and make enabling domu_debug a separate DOM0 operation. Would this be more palatable? On Sun, 15 May 2005, Kip Macy wrote:> I''m not entirely thrilled with the interfaces used to get the domu_debug option > into xen, but adding yet another argument xc_linux_build and/or adding a > separate handler for domu_debug didn''t seem that clean either. The debug support > doesn''t seem like something Ron would use, so I didn''t update the Plan 9 > builder. > > Feedback would be appreciated. > > > # This is a BitKeeper generated diff -Nru style patch. > # > # ChangeSet > # 2005/05/15 18:19:15-07:00 kmacy@octopus.eng.netapp.com > # make domu_debug domain-builder/run-time option > # pause all vcpus on int3 / trace trap > # make domu_debug and cdb not be mutually exclusive > # Signed-off-by: Kip Macy <kmacy@netapp.com> > # > # xen/include/xen/sched.h > # 2005/05/15 18:19:13-07:00 kmacy@octopus.eng.netapp.com +3 -0 > # add DOMF_debug flag > # > # xen/include/public/dom0_ops.h > # 2005/05/15 18:19:13-07:00 kmacy@octopus.eng.netapp.com +2 -0 > # add flags field to setdomaininfo to enable domu_debug > # > # xen/include/asm-x86/debugger.h > # 2005/05/15 18:19:13-07:00 kmacy@octopus.eng.netapp.com +24 -24 > # pause vcpus on int3 or trace trap > # enable domu_debug by default > # make cdb and domu_debug not be mutually exclusive > # > # xen/common/domain.c > # 2005/05/15 18:19:13-07:00 kmacy@octopus.eng.netapp.com +5 -0 > # toggle DOMF_debug depending on value passed with > # setdomaininfo > # > # xen/Rules.mk > # 2005/05/15 18:19:12-07:00 kmacy@octopus.eng.netapp.com +0 -5 > # remove domu_debug from compile > # > # tools/python/xen/xm/create.py > # 2005/05/15 18:19:12-07:00 kmacy@octopus.eng.netapp.com +7 -0 > # add domu_debug option > # > # tools/python/xen/xend/XendDomainInfo.py > # 2005/05/15 18:19:12-07:00 kmacy@octopus.eng.netapp.com +7 -0 > # pass debug flag to domain builder if option set > # > # tools/libxc/xc_vmx_build.c > # 2005/05/15 18:19:12-07:00 kmacy@octopus.eng.netapp.com +7 -1 > # pass debug flags to setdomaininfo > # > # tools/libxc/xc_linux_build.c > # 2005/05/15 18:19:12-07:00 kmacy@octopus.eng.netapp.com +7 -2 > # pass debug flags to setdomaininfo > # > diff -Nru a/tools/libxc/xc_linux_build.c b/tools/libxc/xc_linux_build.c > --- a/tools/libxc/xc_linux_build.c 2005-05-14 18:24:04 -07:00 > +++ b/tools/libxc/xc_linux_build.c 2005-05-14 18:24:04 -07:00 > @@ -316,12 +316,16 @@ > int initrd_fd = -1; > gzFile initrd_gfd = NULL; > int rc, i; > + unsigned int debug_flags; > vcpu_guest_context_t st_ctxt, *ctxt = &st_ctxt; > unsigned long nr_pages; > char *image = NULL; > - unsigned long image_size, initrd_size=0; > + unsigned long si_flags, image_size, initrd_size=0; > unsigned long vstartinfo_start, vkern_entry; > > + si_flags = flags & ~DOMFLAGS_DOMU_DEBUG; > + debug_flags = flags & DOMFLAGS_DOMU_DEBUG; > + > if ( (nr_pages = xc_get_tot_pages(xc_handle, domid)) < 0 ) > { > PERROR("Could not find total pages for domain"); > @@ -381,7 +385,7 @@ > &vstartinfo_start, &vkern_entry, > ctxt, cmdline, > op.u.getdomaininfo.shared_info_frame, > - control_evtchn, flags, vcpus) < 0 ) > + control_evtchn, si_flags, vcpus) < 0 ) > { > ERROR("Error constructing guest OS"); > goto error_out; > @@ -458,6 +462,7 @@ > memset( &launch_op, 0, sizeof(launch_op) ); > > launch_op.u.setdomaininfo.domain = (domid_t)domid; > + launch_op.u.setdomaininfo.flags = debug_flags; > launch_op.u.setdomaininfo.vcpu = 0; > launch_op.u.setdomaininfo.ctxt = ctxt; > > diff -Nru a/tools/libxc/xc_vmx_build.c b/tools/libxc/xc_vmx_build.c > --- a/tools/libxc/xc_vmx_build.c 2005-05-14 18:24:04 -07:00 > +++ b/tools/libxc/xc_vmx_build.c 2005-05-14 18:24:04 -07:00 > @@ -498,11 +498,16 @@ > int initrd_fd = -1; > gzFile initrd_gfd = NULL; > int rc, i; > + unsigned long si_flags; > + unsigned int debug_flags; > vcpu_guest_context_t st_ctxt, *ctxt = &st_ctxt; > unsigned long nr_pages; > char *image = NULL; > unsigned long image_size, initrd_size=0; > > + debug_flags = flags & DOMFLAGS_DOMU_DEBUG; > + si_flags = flags & ~DOMFLAGS_DOMU_DEBUG; > + > if ( vmx_identify() < 0 ) > { > PERROR("CPU doesn''t support VMX Extensions"); > @@ -567,7 +572,7 @@ > initrd_gfd, initrd_size, nr_pages, > ctxt, cmdline, > op.u.getdomaininfo.shared_info_frame, > - control_evtchn, flags, mem_mapp) < 0 ) > + control_evtchn, si_flags, mem_mapp) < 0 ) > { > ERROR("Error constructing guest OS"); > goto error_out; > @@ -624,6 +629,7 @@ > memset( &launch_op, 0, sizeof(launch_op) ); > > launch_op.u.setdomaininfo.domain = (domid_t)domid; > + launch_op.u.setdomaininfo.flags = debug_flags; > launch_op.u.setdomaininfo.vcpu = 0; > launch_op.u.setdomaininfo.ctxt = ctxt; > > diff -Nru a/tools/python/xen/xend/XendDomainInfo.py b/tools/python/xen/xend/XendDomainInfo.py > --- a/tools/python/xen/xend/XendDomainInfo.py 2005-05-14 18:24:04 -07:00 > +++ b/tools/python/xen/xend/XendDomainInfo.py 2005-05-14 18:24:04 -07:00 > @@ -36,6 +36,9 @@ > """Flag for a net device backend domain.""" > SIF_NET_BE_DOMAIN = (1<<5) > > +"""Flag for enable unprivileged domain debugging.""" > +DOMFLAGS_DOMU_DEBUG = (1<<6) > + > """Shutdown code for poweroff.""" > DOMAIN_POWEROFF = 0 > > @@ -272,6 +275,7 @@ > self.ramdisk = None > self.cmdline = None > > + self.domu_debug = 0 > self.channel = None > self.controllers = {} > > @@ -711,6 +715,8 @@ > flags = 0 > if self.netif_backend: flags |= SIF_NET_BE_DOMAIN > if self.blkif_backend: flags |= SIF_BLK_BE_DOMAIN > + if self.domu_debug: flags |= DOMFLAGS_DOMU_DEBUG > + > #todo generalise this > if ostype == "vmx": > err = buildfn(dom = dom, > @@ -1114,6 +1120,7 @@ > if args: > cmdline += " " + args > ramdisk = sxp.child_value(image, "ramdisk", '''') > + vm.domu_debug = sxp.child_value(image, "domu_debug", 0) > log.debug("creating linux domain with cmdline: %s" %(cmdline,)) > vm.create_domain("linux", kernel, ramdisk, cmdline) > return vm > diff -Nru a/tools/python/xen/xm/create.py b/tools/python/xen/xm/create.py > --- a/tools/python/xen/xm/create.py 2005-05-14 18:24:04 -07:00 > +++ b/tools/python/xen/xm/create.py 2005-05-14 18:24:04 -07:00 > @@ -40,6 +40,10 @@ > fn=set_true, default=0, > use="Quiet.") > > +gopts.opt(''domu_debug'', short=''g'', > + fn=set_true, default=0, > + use="Debug.") > + > gopts.opt(''path'', val=''PATH'', > fn=set_value, default=''.:/etc/xen'', > use="""Search path for configuration scripts. > @@ -279,6 +283,9 @@ > config_image.append([''args'', vals.extra]) > if vals.vcpus: > config_image.append([''vcpus'', vals.vcpus]) > + if vals.domu_debug: > + config_image.append([''domu_debug'', vals.domu_debug]) > + > config.append([''image'', config_image ]) > > > diff -Nru a/xen/Rules.mk b/xen/Rules.mk > --- a/xen/Rules.mk 2005-05-14 18:24:04 -07:00 > +++ b/xen/Rules.mk 2005-05-14 18:24:04 -07:00 > @@ -5,7 +5,6 @@ > perfc_arrays?= n > trace ?= n > optimize ?= y > -domu_debug ?= n > crash_debug ?= n > > include $(BASEDIR)/../Config.mk > @@ -43,10 +42,6 @@ > endif > else > CFLAGS += -g -DVERBOSE > -endif > - > -ifeq ($(domu_debug),y) > -CFLAGS += -DDOMU_DEBUG > endif > > ifeq ($(crash_debug),y) > diff -Nru a/xen/common/domain.c b/xen/common/domain.c > --- a/xen/common/domain.c 2005-05-14 18:24:04 -07:00 > +++ b/xen/common/domain.c 2005-05-14 18:24:04 -07:00 > @@ -227,6 +227,11 @@ > if ( (vcpu >= MAX_VIRT_CPUS) || ((ed = d->exec_domain[vcpu]) == NULL) ) > return -EINVAL; > > + if (setdomaininfo->flags & DOMFLAGS_DOMU_DEBUG) > + set_bit(_DOMF_debug, &d->domain_flags); > + else > + clear_bit(_DOMF_debug, &d->domain_flags); > + > if (test_bit(_DOMF_constructed, &d->domain_flags) && > !test_bit(_VCPUF_ctrl_pause, &ed->vcpu_flags)) > return -EINVAL; > diff -Nru a/xen/include/asm-x86/debugger.h b/xen/include/asm-x86/debugger.h > --- a/xen/include/asm-x86/debugger.h 2005-05-14 18:24:04 -07:00 > +++ b/xen/include/asm-x86/debugger.h 2005-05-14 18:24:04 -07:00 > @@ -38,38 +38,26 @@ > #define DEBUGGER_trap_fatal(_v, _r) \ > if ( debugger_trap_fatal(_v, _r) ) return EXCRET_fault_fixed; > > -#if defined(CRASH_DEBUG) > > -extern int __trap_to_cdb(struct cpu_user_regs *r); > -#define debugger_trap_entry(_v, _r) (0) > > -static inline int debugger_trap_fatal( > - unsigned int vector, struct cpu_user_regs *regs) > -{ > - (void)__trap_to_cdb(regs); > - return (vector == TRAP_int3); /* int3 is harmless */ > -} > - > -/* Int3 is a trivial way to gather cpu_user_regs context. */ > -#define debugger_trap_immediate() __asm__ __volatile__ ( "int3" ); > - > -#elif defined(DOMU_DEBUG) > > #include <xen/softirq.h> > > static inline int debugger_trap_entry( > unsigned int vector, struct cpu_user_regs *regs) > { > - struct exec_domain *ed = current; > + struct exec_domain *ted, *ed = current; > > - if ( !KERNEL_MODE(ed, regs) || (ed->domain->domain_id == 0) ) > + if ( !KERNEL_MODE(ed, regs) || (ed->domain->domain_id == 0) || > + !test_bit(_DOMF_debug, &ed->domain->domain_flags) ) > return 0; > > switch ( vector ) > { > case TRAP_int3: > case TRAP_debug: > - set_bit(_VCPUF_ctrl_pause, &ed->vcpu_flags); > + for_each_exec_domain ( ed->domain, ted ) > + set_bit(_VCPUF_ctrl_pause, &ted->vcpu_flags); > raise_softirq(SCHEDULE_SOFTIRQ); > return 1; > } > @@ -77,11 +65,29 @@ > return 0; > } > > + > +#if defined(CRASH_DEBUG) > + > +extern int __trap_to_cdb(struct cpu_user_regs *r); > + > +static inline int debugger_trap_fatal( > + unsigned int vector, struct cpu_user_regs *regs) > +{ > + (void)__trap_to_cdb(regs); > + return (vector == TRAP_int3); /* int3 is harmless */ > +} > + > +/* Int3 is a trivial way to gather cpu_user_regs context. */ > +#define debugger_trap_immediate() __asm__ __volatile__ ( "int3" ); > + > +#else > + > #define debugger_trap_fatal(_v, _r) (0) > #define debugger_trap_immediate() > > +#endif > > -#elif 0 > +#if 0 > > extern int kdb_trap(int, int, struct cpu_user_regs *); > > @@ -99,12 +105,6 @@ > > /* Int3 is a trivial way to gather cpu_user_regs context. */ > #define debugger_trap_immediate() __asm__ __volatile__ ( "int3" ); > - > -#else > - > -#define debugger_trap_entry(_v, _r) (0) > -#define debugger_trap_fatal(_v, _r) (0) > -#define debugger_trap_immediate() > > #endif > > diff -Nru a/xen/include/public/dom0_ops.h b/xen/include/public/dom0_ops.h > --- a/xen/include/public/dom0_ops.h 2005-05-14 18:24:04 -07:00 > +++ b/xen/include/public/dom0_ops.h 2005-05-14 18:24:04 -07:00 > @@ -77,6 +77,7 @@ > #define DOMFLAGS_PAUSED (1<<3) /* Currently paused by control software. */ > #define DOMFLAGS_BLOCKED (1<<4) /* Currently blocked pending an event. */ > #define DOMFLAGS_RUNNING (1<<5) /* Domain is currently running. */ > +#define DOMFLAGS_DOMU_DEBUG (1<<6) /* Pause domain on int3 or trace trap. */ > #define DOMFLAGS_CPUMASK 255 /* CPU to which this domain is bound. */ > #define DOMFLAGS_CPUSHIFT 8 > #define DOMFLAGS_SHUTDOWNMASK 255 /* DOMFLAGS_SHUTDOWN guest-supplied code. */ > @@ -95,6 +96,7 @@ > typedef struct { > /* IN variables. */ > domid_t domain; > + u32 flags; > u16 vcpu; > /* IN/OUT parameters */ > vcpu_guest_context_t *ctxt; > diff -Nru a/xen/include/xen/sched.h b/xen/include/xen/sched.h > --- a/xen/include/xen/sched.h 2005-05-14 18:24:04 -07:00 > +++ b/xen/include/xen/sched.h 2005-05-14 18:24:04 -07:00 > @@ -383,6 +383,9 @@ > /* Death rattle. */ > #define _DOMF_dying 6 > #define DOMF_dying (1UL<<_DOMF_dying) > + /* PAUSE on int3 or trace trap. */ > +#define _DOMF_debug 7 > +#define DOMF_debug (1UL<<_DOMF_debug) > > static inline int domain_runnable(struct exec_domain *ed) > { > >-- "Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it." - Brian W. Kernighan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2005-May-16 20:10 UTC
[Xen-devel] Re: possible changes was Re: [PATCH] make domu_debug run-time option + fix int3 handling for MP
On 16 May 2005, at 20:07, Kip Macy wrote:> I can chunk this into 3 or 4 patches and make enabling domu_debug a > separate > DOM0 operation. Would this be more palatable?Not if I understand the purpose of this patch. As I understand it, the sole reason for making domu_debug configurable per-domain is to distinguish the cause of an ''int3'' when you take an int3 trap. If a domain is a domu_debug domain then you assume that any int3 is for the domu debugger; otherwise, none are. Apart from not being a very nice extension of the domain-building interface, I think the correct solution is to have a breakpoint-setting API within Xen that is shared by all of the debugger stubs. This would allow each debugger to set and clear breakpoints (i.e., add and remove int3 instructions), and would automatically demux int3 traps to the correct debugger(s), and/or propagate the trap up to the guest kernel if appropriate. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Kip Macy
2005-May-16 20:26 UTC
[Xen-devel] Re: possible changes was Re: [PATCH] make domu_debug run-time option + fix int3 handling for MP
> > Not if I understand the purpose of this patch. As I understand it, the > sole reason for making domu_debug configurable per-domain is to > distinguish the cause of an ''int3'' when you take an int3 trap. If a > domain is a domu_debug domain then you assume that any int3 is for the > domu debugger; otherwise, none are.Correct.> Apart from not being a very nice extension of the domain-building > interface,I won''t argue that enabling it belongs in the domain-building interface. So lets not get stuck on that aspect.> I think the correct solution is to have a breakpoint-setting > API within Xen that is shared by all of the debugger stubs.This is a very kernel-stub centric point of view.> This would > allow each debugger to set and clear breakpoints (i.e., add and remove > int3 instructions), and would automatically demux int3 traps to the > correct debugger(s), and/or propagate the trap up to the guest kernel > if appropriate.No differentiation is made between setting / removing int3 and other modifications to memory in ptrace, /proc or the GDB stub protocol. What you''re basically asking is to move debugger state into xen by having xen know about who put what breakpoints where. This is a non-trivial imposition and incompatible with the tools with which I am acquainted. -Kip _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2005-May-16 20:28 UTC
[Xen-devel] Re: possible changes was Re: [PATCH] make domu_debug run-time option + fix int3 handling for MP
> No differentiation is made between setting / removing int3 and other > modifications to memory in ptrace, /proc or the GDB stub protocol. > What you''re > basically asking is to move debugger state into xen by having xen know > about who > put what breakpoints where. This is a non-trivial imposition and > incompatible > with the tools with which I am acquainted.How else can we be sure what to do with int3 traps? Unless Xen can account for debugger-inserted int3''s, any ''solution'' will be a kludge. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Kip Macy
2005-May-16 20:33 UTC
Re: [Xen-devel] Re: possible changes was Re: [PATCH] make domu_debug run-time option + fix int3 handling for MP
I guess I don''t see how this is any different from the per-process ptrace flag on UN*X. Why is this any more of a kludge? -Kip On 5/16/05, Keir Fraser <Keir.Fraser@cl.cam.ac.uk> wrote:> > > No differentiation is made between setting / removing int3 and other > > modifications to memory in ptrace, /proc or the GDB stub protocol. > > What you''re > > basically asking is to move debugger state into xen by having xen know > > about who > > put what breakpoints where. This is a non-trivial imposition and > > incompatible > > with the tools with which I am acquainted. > > How else can we be sure what to do with int3 traps? Unless Xen can > account for debugger-inserted int3''s, any ''solution'' will be a kludge. > > -- Keir > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2005-May-16 20:40 UTC
Re: [Xen-devel] Re: possible changes was Re: [PATCH] make domu_debug run-time option + fix int3 handling for MP
On 16 May 2005, at 21:33, Kip Macy wrote:> I guess I don''t see how this is any different from the per-process > ptrace flag on UN*X. Why is this any more of a kludge?Hmm, yeah, I guess. It''s fair to say that noone would expect to be able to have multiple debuggers setting breakpoints and single-stepping on the same entity simultaneously, so having a per-domain mode, or having a notifier chain for debug events, is I guess okay. But please kill the changes to the domain builders, and change the flag to something like DOMF_debug_mode (I hope to expunge the name ''domu_debug'' :-) ). -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Kip Macy
2005-May-16 20:46 UTC
Re: [Xen-devel] Re: possible changes was Re: [PATCH] make domu_debug run-time option + fix int3 handling for MP
> > I guess I don''t see how this is any different from the per-process > > ptrace flag on UN*X. Why is this any more of a kludge? > > Hmm, yeah, I guess. It''s fair to say that noone would expect to be able > to have multiple debuggers setting breakpoints and single-stepping on > the same entity simultaneously, so having a per-domain mode, or having > a notifier chain for debug events, is I guess okay. > > But please kill the changes to the domain builders, and change the flag > to something like DOMF_debug_mode (I hope to expunge the name > ''domu_debug'' :-) ).Cool. I''m glad to see we''re back on the same page. -Kip _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Christian Limpach
2005-May-16 21:55 UTC
Re: [Xen-devel] [PATCH] make domu_debug run-time option + fix int3 handling for MP
On 5/16/05, Kip Macy <kmacy@netapp.com> wrote:> I''m not entirely thrilled with the interfaces used to get the domu_debug option > into xen, but adding yet another argument xc_linux_build and/or adding a > separate handler for domu_debug didn''t seem that clean either. The debug support > doesn''t seem like something Ron would use, so I didn''t update the Plan 9 > builder.Looks allright -- except that the debugger.h changes break the non-domu_debug usages. Even if you fix that, I''m not sure we want debugger_trap_entry to be domu_debug only. Could you please explain what the intent is and resend as separate patches? Maybe a debug_trap_entry wrapper which tests the DOMF_debug flag and either calls the domu_debug function or the other debugger function would be better? Please also leave the #if/#elif construct for easy switching between the different debugger stubs... christian _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Kip Macy
2005-May-16 22:08 UTC
Re: [Xen-devel] [PATCH] make domu_debug run-time option + fix int3 handling for MP
"domu_debug" was mutually exclusive with crash_debug, which doesn''t make sense to me. The other stub already had a #if 0 as a pre-processor guard around it. So the end effect with the patch is that domu_debug would be on by default and cdb could be enabled at compile-time. As I recall, there is no working functionality that this changes. I''ll go back and look and recant if I am in error. I will make debugger_trap_entry call my code only if the domain flag is set, but right now this is the only debugger type that has a real debugger_trap_entry - so multiplexing seems a bit silly. There are hundreds of other places in the code where we could generalize for future extensibility. -Kip> > Looks allright -- except that the debugger.h changes break the > non-domu_debug usages. Even if you fix that, I''m not sure we want > debugger_trap_entry to be domu_debug only. Could you please explain > what the intent is and resend as separate patches? > Maybe a debug_trap_entry wrapper which tests the DOMF_debug flag and > either calls the domu_debug function or the other debugger function > would be better? Please also leave the #if/#elif construct for easy > switching between the different debugger stubs... > > christian >-- "Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it." - Brian W. Kernighan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Christian Limpach
2005-May-16 22:18 UTC
Re: [Xen-devel] [PATCH] make domu_debug run-time option + fix int3 handling for MP
On Mon, May 16, 2005 at 03:08:24PM -0700, Kip Macy wrote:> "domu_debug" was mutually exclusive with crash_debug, which doesn''t make sense > to me. The other stub already had a #if 0 as a pre-processor guard around it. So > the end effect with the patch is that domu_debug would be on by default and cdb > could be enabled at compile-time. As I recall, there is no working functionality > that this changes. I''ll go back and look and recant if I am in error.Unless I misread the patch (didn''t apply it), you''d end up with unconditional definitions of debugger_trap_fatal and debugger_trap_immediate? I usually just toggle the #elif 0 to enable the kdb stubs, I don''t see how that would work now, without having to do more changes?> I will make debugger_trap_entry call my code only if the domain flag is set, but > right now this is the only debugger type that has a real debugger_trap_entry - > so multiplexing seems a bit silly. There are hundreds of other places in the > code where we could generalize for future extensibility.Ok, fair enough. Put a comment on it then ;-) christian _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Kip Macy
2005-May-16 22:23 UTC
Re: [Xen-devel] [PATCH] make domu_debug run-time option + fix int3 handling for MP
> I usually just toggle the #elif 0 to enable the kdb stubs, I don''t see > how that would work now, without having to do more changes?Ouch. Therein lies my mistake. I interpreted "#if 0" to mean broken functionality, not "currently not enabled". Not that I don''t do that all the time in my code. I just hold you to a higher standard ;-). -Kip _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel