* [Xenomai-core] [PATCH] kgdb-ipipe for 2.6.16
@ 2006-05-22 19:06 Jan Kiszka
2006-06-01 8:00 ` Jan Kiszka
0 siblings, 1 reply; 11+ messages in thread
From: Jan Kiszka @ 2006-05-22 19:06 UTC (permalink / raw)
To: xenomai-core
[-- Attachment #1.1: Type: text/plain, Size: 856 bytes --]
Hi,
here comes another round of patches to make kgdb work over Ipipe/Xenomai
(still only x86). This series is based on latest kgdb CVS [1], which
mostly work over 2.6.16 kernels. As usual, the application series file
is attached, and should explain the usage.
It "mostly works" means that you may encounter problems in combination
with gcc 4.1 [2]. For now, as a workaround, add "CFLAGS_kgdb.o +=
-fno-unit-at-a-time" to kernel/Makefile. I will have to post this issue
to kgdb-bugreport as well and see if they prefer a cleaner solution.
I also submit a new version of the required Xenomai extension.
Hopefully, I managed to integrate it more cleanly this time.
Jan
[1] cvs -d :pserver:anonymous@domain.hid \
co kgdb-2
(server seems to be down right now...)
[2] http://gcc.gnu.org/ml/gcc-help/2006-05/msg00202.html
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: prepare-kgdb-ipipe-x86.patch --]
[-- Type: text/x-patch; name="prepare-kgdb-ipipe-x86.patch", Size: 842 bytes --]
Index: linux-2.6.16.16/arch/i386/kernel/entry.S
===================================================================
--- linux-2.6.16.16.orig/arch/i386/kernel/entry.S
+++ linux-2.6.16.16/arch/i386/kernel/entry.S
@@ -123,7 +123,7 @@ VM_MASK = 0x00020000
.previous
-KPROBE_ENTRY(ret_from_fork)
+ENTRY(ret_from_fork)
pushl %eax
call schedule_tail
GET_THREAD_INFO(%ebp)
@@ -472,7 +472,7 @@ ENTRY(simd_coprocessor_error)
pushl $do_simd_coprocessor_error
jmp error_code
-KPROBE_ENTRY(device_not_available)
+ENTRY(device_not_available)
pushl $-1 # mark this as an int
SAVE_ALL
movl %cr0, %eax
@@ -654,7 +654,7 @@ ENTRY(machine_check)
jmp error_code
#endif
-KPROBE_ENTRY(spurious_interrupt_bug)
+ENTRY(spurious_interrupt_bug)
pushl $0
pushl $do_spurious_interrupt_bug
jmp error_code
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.3: kgdb-ipipe.patch --]
[-- Type: text/x-patch; name="kgdb-ipipe.patch", Size: 8542 bytes --]
Index: linux-2.6.16.16/drivers/serial/8250_kgdb.c
===================================================================
--- linux-2.6.16.16.orig/drivers/serial/8250_kgdb.c
+++ linux-2.6.16.16/drivers/serial/8250_kgdb.c
@@ -301,6 +301,10 @@ static void __init kgdb8250_late_init(vo
"GDB-stub", current_port) < 0)
printk(KERN_ERR "KGDB failed to request the serial IRQ (%d)\n",
current_port->irq);
+#ifdef CONFIG_IPIPE
+ ipipe_control_irq(current_port->irq, 0,
+ IPIPE_HANDLE_MASK|IPIPE_STICKY_MASK|IPIPE_SYSTEM_MASK);
+#endif /* CONFIG_IPIPE */
}
static __init int kgdb_init_io(void)
Index: linux-2.6.16.16/include/linux/kgdb.h
===================================================================
--- linux-2.6.16.16.orig/include/linux/kgdb.h
+++ linux-2.6.16.16/include/linux/kgdb.h
@@ -270,6 +270,9 @@ extern int kgdb_handle_exception(int ex_
extern void kgdb_nmihook(int cpu, void *regs);
extern int debugger_step;
extern atomic_t debugger_active;
+
+struct task_struct *ipipe_default_current(void);
+extern struct task_struct *(*ipipe_safe_current)(void);
#else
/* Stubs for when KGDB is not set. */
static const atomic_t debugger_active = ATOMIC_INIT(0);
Index: linux-2.6.16.16/kernel/kgdb.c
===================================================================
--- linux-2.6.16.16.orig/kernel/kgdb.c
+++ linux-2.6.16.16/kernel/kgdb.c
@@ -740,12 +740,12 @@ static void kgdb_wait(struct pt_regs *re
unsigned long flags;
int processor;
- local_irq_save(flags);
- processor = smp_processor_id();
+ local_irq_save_hw(flags);
+ processor = ipipe_processor_id();
kgdb_info[processor].debuggerinfo = regs;
- kgdb_info[processor].task = current;
+ kgdb_info[processor].task = ipipe_safe_current();
atomic_set(&procindebug[processor], 1);
- atomic_set(&kgdb_sync_softlockup[smp_processor_id()], 1);
+ atomic_set(&kgdb_sync_softlockup[processor], 1);
/* Wait till master processor goes completely into the debugger.
* FIXME: this looks racy */
@@ -770,7 +770,7 @@ static void kgdb_wait(struct pt_regs *re
/* Signal the master processor that we are done */
atomic_set(&procindebug[processor], 0);
spin_unlock(&slavecpulocks[processor]);
- local_irq_restore(flags);
+ local_irq_restore_hw(flags);
}
#endif
@@ -821,7 +821,8 @@ int kgdb_activate_sw_breakpoints(void)
return error;
if (CACHE_FLUSH_IS_SAFE) {
- if (current->mm && addr < TASK_SIZE)
+ if (ipipe_safe_current() == current &&
+ current->mm && addr < TASK_SIZE)
flush_cache_range(current->mm->mmap_cache,
addr, addr + BREAK_INSTR_SIZE);
else
@@ -884,8 +885,8 @@ int kgdb_deactivate_sw_breakpoints(void)
kgdb_break[i].saved_instr)))
return error;
- if (CACHE_FLUSH_IS_SAFE && current->mm &&
- addr < TASK_SIZE)
+ if (CACHE_FLUSH_IS_SAFE && ipipe_safe_current() == current &&
+ current->mm && addr < TASK_SIZE)
flush_cache_range(current->mm->mmap_cache,
addr, addr + BREAK_INSTR_SIZE);
else if (CACHE_FLUSH_IS_SAFE)
@@ -950,7 +951,7 @@ static inline int shadow_pid(int realpid
if (realpid) {
return realpid;
}
- return pid_max + smp_processor_id();
+ return pid_max + ipipe_processor_id();
}
static char gdbmsgbuf[BUFMAX + 1];
@@ -1014,11 +1015,11 @@ int kgdb_handle_exception(int ex_vector,
long kgdb_usethreadid = 0;
int error = 0, all_cpus_synced = 0;
struct pt_regs *shadowregs;
- int processor = smp_processor_id();
+ int processor = ipipe_processor_id();
void *local_debuggerinfo;
/* Panic on recursive debugger calls. */
- if (atomic_read(&debugger_active) == smp_processor_id() + 1)
+ if (atomic_read(&debugger_active) == ipipe_processor_id() + 1)
return 0;
acquirelock:
@@ -1033,10 +1034,10 @@ int kgdb_handle_exception(int ex_vector,
* Interrupts will be restored by the 'trap return' code, except when
* single stepping.
*/
- local_irq_save(flags);
+ local_irq_save_hw(flags);
/* Hold debugger_active */
- procid = smp_processor_id();
+ procid = ipipe_processor_id();
while (cmpxchg(&atomic_read(&debugger_active), 0, (procid + 1)) != 0) {
int i = 25; /* an arbitrary number */
@@ -1049,7 +1050,7 @@ int kgdb_handle_exception(int ex_vector,
udelay(1);
}
- atomic_set(&kgdb_sync_softlockup[smp_processor_id()], 1);
+ atomic_set(&kgdb_sync_softlockup[procid], 1);
/*
* Don't enter if the last instance of the exception handler wanted to
@@ -1058,7 +1059,7 @@ int kgdb_handle_exception(int ex_vector,
if (atomic_read(&cpu_doing_single_step) != -1 &&
atomic_read(&cpu_doing_single_step) != procid) {
atomic_set(&debugger_active, 0);
- local_irq_restore(flags);
+ local_irq_restore_hw(flags);
goto acquirelock;
}
@@ -1069,7 +1070,7 @@ int kgdb_handle_exception(int ex_vector,
goto kgdb_restore;
kgdb_info[processor].debuggerinfo = linux_regs;
- kgdb_info[processor].task = current;
+ kgdb_info[processor].task = ipipe_safe_current();
kgdb_disable_hw_debug(linux_regs);
@@ -1121,7 +1122,8 @@ int kgdb_handle_exception(int ex_vector,
*ptr++ = hexchars[(signo >> 4) % 16];
*ptr++ = hexchars[signo % 16];
ptr += strlen(strcpy(ptr, "thread:"));
- int_to_threadref(&thref, shadow_pid(current->pid));
+ int_to_threadref(&thref,
+ shadow_pid(ipipe_safe_current()->pid));
ptr = pack_threadid(ptr, &thref);
*ptr++ = ';';
@@ -1213,7 +1215,8 @@ int kgdb_handle_exception(int ex_vector,
kgdb_hex2mem(&remcom_in_buffer[1], (char *)gdb_regs,
NUMREGBYTES);
- if (kgdb_usethread && kgdb_usethread != current)
+ if (kgdb_usethread &&
+ kgdb_usethread != ipipe_safe_current())
error_packet(remcom_out_buffer, -EINVAL);
else {
gdb_regs_to_regs(gdb_regs, linux_regs);
@@ -1334,7 +1337,8 @@ int kgdb_handle_exception(int ex_vector,
/* Current thread id */
strcpy(remcom_out_buffer, "QC");
- threadid = shadow_pid(current->pid);
+ threadid =
+ shadow_pid(ipipe_safe_current()->pid);
int_to_threadref(&thref, threadid);
pack_threadid(remcom_out_buffer + 2, &thref);
@@ -1488,7 +1492,8 @@ int kgdb_handle_exception(int ex_vector,
break;
case 'c':
case 's':
- if (kgdb_contthread && kgdb_contthread != current) {
+ if (kgdb_contthread &&
+ kgdb_contthread != ipipe_safe_current()) {
/* Can't switch threads in kgdb */
error_packet(remcom_out_buffer, -EINVAL);
break;
@@ -1556,7 +1561,7 @@ int kgdb_handle_exception(int ex_vector,
kgdb_restore:
/* Free debugger_active */
atomic_set(&debugger_active, 0);
- local_irq_restore(flags);
+ local_irq_restore_hw(flags);
return error;
}
@@ -1925,9 +1930,9 @@ static int kgdb_notify_reboot(struct not
if (!kgdb_connected || atomic_read(&debugger_active) != 0)
return 0;
if ((code == SYS_RESTART) || (code == SYS_HALT) || (code == SYS_POWER_OFF)){
- local_irq_save(flags);
+ local_irq_save_hw(flags);
put_packet("X00");
- local_irq_restore(flags);
+ local_irq_restore_hw(flags);
}
return NOTIFY_DONE;
}
@@ -1942,9 +1947,9 @@ void kgdb_console_write(struct console *
if (!kgdb_connected || atomic_read(&debugger_active) != 0)
return;
- local_irq_save(flags);
+ local_irq_save_hw(flags);
kgdb_msg_write(s, count);
- local_irq_restore(flags);
+ local_irq_restore_hw(flags);
}
struct console kgdbcons = {
@@ -1974,3 +1979,12 @@ static int __init opt_kgdb_enter(char *s
}
early_param("kgdbwait", opt_kgdb_enter);
+
+struct task_struct *ipipe_default_current(void)
+{
+ return current;
+}
+EXPORT_SYMBOL(ipipe_default_current);
+
+struct task_struct *(*ipipe_safe_current)(void) = ipipe_default_current;
+EXPORT_SYMBOL(ipipe_safe_current);
Index: linux-2.6.16.16/lib/Kconfig.debug
===================================================================
--- linux-2.6.16.16.orig/lib/Kconfig.debug
+++ linux-2.6.16.16/lib/Kconfig.debug
@@ -273,7 +273,7 @@ choice
config KGDB_ONLY_MODULES
bool "KGDB: Use only kernel modules for I/O"
- depends on MODULES
+ depends on MODULES && !IPIPE
help
Use only kernel modules to configure KGDB I/O after the
kernel is booted.
@@ -318,7 +318,7 @@ config KGDB_SIBYTE
endchoice
config KGDBOE
- tristate "KGDB: On ethernet" if !KGDBOE_NOMODULE
+ tristate "KGDB: On ethernet" if !KGDBOE_NOMODULE && !IPIPE
depends on m && KGDB
select NETPOLL
select NETPOLL_TRAP
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.4: kgdb-ipipe-x86.patch --]
[-- Type: text/x-patch; name="kgdb-ipipe-x86.patch", Size: 4575 bytes --]
Index: linux-2.6.16.16/arch/i386/kernel/entry.S
===================================================================
--- linux-2.6.16.16.orig/arch/i386/kernel/entry.S
+++ linux-2.6.16.16/arch/i386/kernel/entry.S
@@ -194,7 +194,7 @@ VM_MASK = 0x00020000
.previous
-ENTRY(ret_from_fork)
+KPROBE_ENTRY(ret_from_fork)
STI_COND_HW
pushl %eax
call schedule_tail
@@ -584,7 +584,7 @@ ENTRY(simd_coprocessor_error)
PUSH_XCODE(do_simd_coprocessor_error)
jmp error_code
-ENTRY(device_not_available)
+KPROBE_ENTRY(device_not_available)
pushl $-1 # mark this as an int
SAVE_ALL
DIVERT_EXCEPTION(device_not_available)
@@ -769,7 +769,7 @@ ENTRY(machine_check)
jmp error_code
#endif
-ENTRY(spurious_interrupt_bug)
+KPROBE_ENTRY(spurious_interrupt_bug)
pushl $0
PUSH_XCODE(do_spurious_interrupt_bug)
jmp error_code
Index: linux-2.6.16.16/arch/i386/kernel/ipipe-root.c
===================================================================
--- linux-2.6.16.16.orig/arch/i386/kernel/ipipe-root.c
+++ linux-2.6.16.16/arch/i386/kernel/ipipe-root.c
@@ -34,6 +34,9 @@
#include <asm/irq.h>
#include <asm/desc.h>
#include <asm/io.h>
+#ifdef CONFIG_KGDB
+#include <linux/kgdb.h>
+#endif /* CONFIG_KGDB */
#ifdef CONFIG_X86_LOCAL_APIC
#include <asm/tlbflush.h>
#include <asm/fixmap.h>
@@ -422,8 +425,44 @@ static __ipipe_exptr __ipipe_std_extable
[ex_do_iret_error] = &do_iret_error,
};
+#ifdef CONFIG_KGDB
+static int __ipipe_xlate_signo[] = {
+
+ [ex_do_divide_error] = SIGFPE,
+ [ex_do_debug] = SIGTRAP,
+ [2] = -1,
+ [ex_do_int3] = SIGTRAP,
+ [ex_do_overflow] = SIGSEGV,
+ [ex_do_bounds] = SIGSEGV,
+ [ex_do_invalid_op] = SIGILL,
+ [ex_device_not_available] = -1,
+ [8] = -1,
+ [ex_do_coprocessor_segment_overrun] = SIGFPE,
+ [ex_do_invalid_TSS] = SIGSEGV,
+ [ex_do_segment_not_present] = SIGBUS,
+ [ex_do_stack_segment] = SIGBUS,
+ [ex_do_general_protection] = SIGSEGV,
+ [ex_do_page_fault] = SIGSEGV,
+ [ex_do_spurious_interrupt_bug] = -1,
+ [ex_do_coprocessor_error] = -1,
+ [ex_do_alignment_check] = SIGBUS,
+ [ex_machine_check_vector] = -1,
+ [ex_do_simd_coprocessor_error] = -1,
+ [20 ... 31] = -1,
+ [ex_do_iret_error] = SIGSEGV,
+};
+#endif /* CONFIG_KGDB */
+
fastcall int __ipipe_handle_exception(struct pt_regs *regs, long error_code, int vector)
{
+#ifdef CONFIG_KGDB
+ /* catch exception KGDB is interested in over non-root domains */
+ if ((ipipe_current_domain != ipipe_root_domain) &&
+ (__ipipe_xlate_signo[vector] >= 0) &&
+ !kgdb_handle_exception(vector, __ipipe_xlate_signo[vector], error_code, regs))
+ return 1;
+#endif /* CONFIG_KGDB */
+
if (!__ipipe_event_pipelined_p(vector) ||
__ipipe_dispatch_event(vector,regs) == 0) {
__ipipe_exptr handler = __ipipe_std_extable[vector];
@@ -437,6 +476,19 @@ fastcall int __ipipe_handle_exception(st
fastcall int __ipipe_divert_exception(struct pt_regs *regs, int vector)
{
+#ifdef CONFIG_KGDB
+ /* catch int1 and int3 over non-root domains */
+ if ((ipipe_current_domain != ipipe_root_domain) &&
+ (vector != ex_device_not_available)) {
+ unsigned int condition = 0;
+
+ if (vector == 1)
+ get_debugreg(condition, 6);
+ if (!kgdb_handle_exception(vector, SIGTRAP, condition, regs))
+ return 1;
+ }
+#endif /* CONFIG_KGDB */
+
if (__ipipe_event_pipelined_p(vector) &&
__ipipe_dispatch_event(vector,regs) != 0)
return 1;
Index: linux-2.6.16.16/arch/i386/kernel/kgdb.c
===================================================================
--- linux-2.6.16.16.orig/arch/i386/kernel/kgdb.c
+++ linux-2.6.16.16/arch/i386/kernel/kgdb.c
@@ -276,7 +276,8 @@ int kgdb_arch_handle_exception(int e_vec
if (remcom_in_buffer[0] == 's') {
linux_regs->eflags |= TF_MASK;
debugger_step = 1;
- atomic_set(&cpu_doing_single_step,smp_processor_id());
+ atomic_set(&cpu_doing_single_step,
+ ipipe_processor_id());
}
asm volatile ("movl %%db6, %0\n":"=r" (dr6));
@@ -319,7 +320,7 @@ static int kgdb_notify(struct notifier_b
else if ((cmd == DIE_NMI || cmd == DIE_NMI_IPI ||
cmd == DIE_NMIWATCHDOG) && atomic_read(&debugger_active)) {
/* CPU roundup */
- kgdb_nmihook(smp_processor_id(), regs);
+ kgdb_nmihook(ipipe_processor_id(), regs);
return NOTIFY_STOP;
} else if (cmd == DIE_NMI_IPI || cmd == DIE_NMI || user_mode(regs) ||
(cmd == DIE_DEBUG && atomic_read(&debugger_active)))
[-- Attachment #1.5: series --]
[-- Type: text/plain, Size: 442 bytes --]
core-lite.patch
8250.patch
netpoll_pass_skb_to_rx_hook.patch
eth.patch
i386-lite.patch
powerpc-lite.patch
mips-lite.patch
ia64-lite.patch
x86_64-no_context_hook.patch
x86_64-lite.patch
sh-lite.patch
arm-lite.patch
cfi_annotations.patch
sysrq_bugfix.patch
module.patch
core.patch
i386.patch
powerpc.patch
ia64.patch
prepare-kgdb-ipipe-x86.patch
adeos-ipipe-2.6.16-i386-1.3-04.patch
kgdb-ipipe.patch
kgdb-ipipe-x86.patch
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.6: xeno-kgdb-v2.patch --]
[-- Type: text/x-patch; name="xeno-kgdb-v2.patch", Size: 2255 bytes --]
Index: ksrc/nucleus/pod.c
===================================================================
--- ksrc/nucleus/pod.c (Revision 1126)
+++ ksrc/nucleus/pod.c (Arbeitskopie)
@@ -234,6 +234,13 @@ static void xnpod_flush_heap(xnheap_t *
xnarch_sysfree(extaddr, extsize);
}
+static inline struct task_struct * do_safe_current(void)
+{
+ return xnpod_userspace_p() ? current : &init_task;
+}
+
+RTHAL_DECLARE_SAFE_CURRENT(safe_current);
+
/*!
* \fn int xnpod_init(xnpod_t *pod,int minpri,int maxpri,xnflags_t flags)
* \brief Initialize a new pod.
@@ -373,6 +380,7 @@ int xnpod_init(xnpod_t *pod, int minpri,
the remaining operations. */
nkpod = pod;
+ rthal_hook_safe_current(safe_current);
/* No direct handler here since the host timer processing is
postponed to xnintr_irq_handler(), as part of the interrupt
@@ -477,6 +485,7 @@ int xnpod_init(xnpod_t *pod, int minpri,
if (err) {
fail:
+ rthal_release_safe_current();
nkpod = NULL;
return err;
}
@@ -607,6 +616,7 @@ void xnpod_shutdown(int xtype)
xnheap_destroy(&kheap, &xnpod_flush_heap, NULL);
+ rthal_release_safe_current();
nkpod = NULL;
unlock_and_exit:
Index: include/asm-generic/hal.h
===================================================================
--- include/asm-generic/hal.h (Revision 1126)
+++ include/asm-generic/hal.h (Arbeitskopie)
@@ -573,6 +573,25 @@ struct proc_dir_entry *__rthal_add_proc_
struct proc_dir_entry *parent);
#endif /* CONFIG_PROC_FS */
+#ifdef CONFIG_KGDB
+#include <linux/kgdb.h>
+
+#define rthal_hook_safe_current(handler) \
+ ipipe_safe_current = handler
+#define rthal_release_safe_current() \
+ ipipe_safe_current = ipipe_default_current
+
+#define RTHAL_DECLARE_SAFE_CURRENT(handler) \
+static struct task_struct *handler(void) \
+{ \
+ return do_##handler(); \
+}
+#else /* !CONFIG_KGDB */
+#define rthal_hook_safe_current(handler) do { } while (0)
+#define rthal_release_safe_current() do { } while (0)
+#define RTHAL_DECLARE_SAFE_CURRENT(handler)
+#endif /* CONFIG_KGDB */
+
#ifdef __cplusplus
}
#endif /* __cplusplus */
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 252 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [Xenomai-core] [PATCH] kgdb-ipipe for 2.6.16
2006-05-22 19:06 [Xenomai-core] [PATCH] kgdb-ipipe for 2.6.16 Jan Kiszka
@ 2006-06-01 8:00 ` Jan Kiszka
2006-06-07 16:19 ` [Xenomai-core] [PATCH] kgdb/x86 over I-pipe Philippe Gerum
0 siblings, 1 reply; 11+ messages in thread
From: Jan Kiszka @ 2006-06-01 8:00 UTC (permalink / raw)
To: xenomai-core
[-- Attachment #1.1: Type: text/plain, Size: 316 bytes --]
Jan Kiszka wrote:
> ...
> I also submit a new version of the required Xenomai extension.
> Hopefully, I managed to integrate it more cleanly this time.
>
I did not meet this goal as I found out on my own. So here is revision 3
of the Xenomai kgdb patch, now no longer breaking uvm or the simulator.
Jan
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: xeno-kgdb-v3.patch --]
[-- Type: text/x-patch; name="xeno-kgdb-v3.patch", Size: 4028 bytes --]
Index: include/asm-uvm/system.h
===================================================================
--- include/asm-uvm/system.h (Revision 1142)
+++ include/asm-uvm/system.h (Arbeitskopie)
@@ -787,4 +787,9 @@ static inline void xnarch_sysfree (void
/* Ipipe-tracer */
#define ipipe_trace_panic_freeze()
+/* kgdb hooks */
+#define xnarch_hook_safe_current(handler) do { } while (0)
+#define xnarch_release_safe_current() do { } while (0)
+#define XNARCH_DECLARE_SAFE_CURRENT(handler)
+
#endif /* !_XENO_ASM_UVM_SYSTEM_H */
Index: include/asm-generic/hal.h
===================================================================
--- include/asm-generic/hal.h (Revision 1142)
+++ include/asm-generic/hal.h (Arbeitskopie)
@@ -573,6 +573,25 @@ struct proc_dir_entry *__rthal_add_proc_
struct proc_dir_entry *parent);
#endif /* CONFIG_PROC_FS */
+#ifdef CONFIG_KGDB
+#include <linux/kgdb.h>
+
+#define rthal_hook_safe_current(handler) \
+ ipipe_safe_current = handler
+#define rthal_release_safe_current() \
+ ipipe_safe_current = ipipe_default_current
+
+#define RTHAL_DECLARE_SAFE_CURRENT(handler) \
+static struct task_struct *handler(void) \
+{ \
+ return do_##handler(); \
+}
+#else /* !CONFIG_KGDB */
+#define rthal_hook_safe_current(handler) do { } while (0)
+#define rthal_release_safe_current() do { } while (0)
+#define RTHAL_DECLARE_SAFE_CURRENT(handler)
+#endif /* CONFIG_KGDB */
+
#ifdef __cplusplus
}
#endif /* __cplusplus */
Index: include/asm-generic/system.h
===================================================================
--- include/asm-generic/system.h (Revision 1142)
+++ include/asm-generic/system.h (Arbeitskopie)
@@ -669,4 +669,10 @@ static inline void xnarch_init_heapcb (x
#define xnarch_post_graph(obj,state)
#define xnarch_post_graph_if(obj,state,cond)
+/* kgdb hooks */
+#define xnarch_hook_safe_current(handler) rthal_hook_safe_current(handler)
+#define xnarch_release_safe_current rthal_release_safe_current
+#define XNARCH_DECLARE_SAFE_CURRENT(handler) \
+ RTHAL_DECLARE_SAFE_CURRENT(handler)
+
#endif /* !_XENO_ASM_GENERIC_SYSTEM_H */
Index: include/asm-sim/system.h
===================================================================
--- include/asm-sim/system.h (Revision 1142)
+++ include/asm-sim/system.h (Arbeitskopie)
@@ -765,6 +765,11 @@ while(0)
/* Ipipe-tracer */
#define ipipe_trace_panic_freeze()
+/* kgdb hooks */
+#define xnarch_hook_safe_current(handler) do { } while (0)
+#define xnarch_release_safe_current() do { } while (0)
+#define XNARCH_DECLARE_SAFE_CURRENT(handler)
+
#ifndef PAGE_SIZE
#define PAGE_SIZE sysconf(_SC_PAGESIZE)
#endif /* !PAGE_SIZE */
Index: ksrc/nucleus/pod.c
===================================================================
--- ksrc/nucleus/pod.c (Revision 1142)
+++ ksrc/nucleus/pod.c (Arbeitskopie)
@@ -234,6 +234,13 @@ static void xnpod_flush_heap(xnheap_t *
xnarch_sysfree(extaddr, extsize);
}
+#define do_safe_current() \
+({ \
+ xnpod_userspace_p() ? current : &init_task; \
+}}
+
+XNARCH_DECLARE_SAFE_CURRENT(safe_current);
+
/*!
* \fn int xnpod_init(xnpod_t *pod,int minpri,int maxpri,xnflags_t flags)
* \brief Initialize a new pod.
@@ -373,6 +380,7 @@ int xnpod_init(xnpod_t *pod, int minpri,
the remaining operations. */
nkpod = pod;
+ xnarch_hook_safe_current(safe_current);
/* No direct handler here since the host timer processing is
postponed to xnintr_irq_handler(), as part of the interrupt
@@ -477,6 +485,7 @@ int xnpod_init(xnpod_t *pod, int minpri,
if (err) {
fail:
+ xnarch_release_safe_current();
nkpod = NULL;
return err;
}
@@ -607,6 +616,7 @@ void xnpod_shutdown(int xtype)
xnheap_destroy(&kheap, &xnpod_flush_heap, NULL);
+ xnarch_release_safe_current();
nkpod = NULL;
unlock_and_exit:
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 249 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread* [Xenomai-core] [PATCH] kgdb/x86 over I-pipe
2006-06-01 8:00 ` Jan Kiszka
@ 2006-06-07 16:19 ` Philippe Gerum
2006-06-07 16:50 ` [Xenomai-core] " Jan Kiszka
0 siblings, 1 reply; 11+ messages in thread
From: Philippe Gerum @ 2006-06-07 16:19 UTC (permalink / raw)
To: Jan Kiszka; +Cc: xenomai-core
[-- Attachment #1: Type: text/plain, Size: 1866 bytes --]
Hi Jan,
Based on your previous work, here is a set of patches coupling KGDB and
the I-pipe. Basically, I've attempted to shrink the extra patches needed
against the original KGDB + I-pipe ones to the bare minimum. This has
been obtained by having the I-pipe provide ipipe_current_safe(), and
drastically reduce the amount of fiddling with smp_processor_id().
The key difference with the former implementation is that a domain (e.g.
Xenomai) is now expected to tell the I-pipe when it's switching to a
non-Linux stack, and the I-pipe makes good use of this information to
return the proper "current" value when asked to through
ipipe_safe_current() from the KGDB code. The issue of swapping
smp_processor_id() with ipipe_processor_id() has been addressed the hard
way: smp_processor_id() is simply defined as ipipe_processor_id() when
CONFIG_IPIPE and CONFIG_KGDB are both enabled in include/linux/smp.h.
This approach was actually used during the old Adeos times when pipeline
domain had their own separate stack. I take for granted that the CPU
penalty taken in doing this is perfectly acceptable, since well, we are
debugging after all.
Aside of the small patches attached, you will need the latest I-pipe
1.3-05 patch for x86, adding the foreign stack notifier and the
ipipe_safe_current() support:
http://download.gna.org/adeos/patches/v2.6/i386/adeos-ipipe-2.6.16-i386-1.3-05.patch
Patches should be applied in this order on a vanilla 2.6.16 kernel:
- KGDB 2.4 patch series over 2.6.16 (quilt)
- pre-kgdb-ipipe-i386.patch
- adeos-ipipe-2.6.16-i386-1.3-05.patch
- kgdb-ipipe.patch
- post-kgdb-ipipe-i386.patch
Xenomai's trunk/ should be used. Older code won't work and likely crash
since the I-pipe would not be notified about foreign stack switches.
Now the surprise: I did not test this stuff, I mean, at all. Eh. :o)
--
Philippe.
[-- Attachment #2: kgdb-ipipe.patch --]
[-- Type: text/x-patch, Size: 5760 bytes --]
--- 2.6.16-base/kernel/kgdb.c 2006-06-07 16:40:21.000000000 +0200
+++ 2.6.16-kgdb/kernel/kgdb.c 2006-06-07 16:43:36.000000000 +0200
@@ -740,10 +740,10 @@
unsigned long flags;
int processor;
- local_irq_save(flags);
+ local_irq_save_hw(flags);
processor = smp_processor_id();
kgdb_info[processor].debuggerinfo = regs;
- kgdb_info[processor].task = current;
+ kgdb_info[processor].task = ipipe_safe_current();
atomic_set(&procindebug[processor], 1);
/* Wait till master processor goes completely into the debugger.
@@ -769,7 +769,7 @@
/* Signal the master processor that we are done */
atomic_set(&procindebug[processor], 0);
spin_unlock(&slavecpulocks[processor]);
- local_irq_restore(flags);
+ local_irq_restore_hw(flags);
}
#endif
@@ -883,8 +883,8 @@
kgdb_break[i].saved_instr)))
return error;
- if (CACHE_FLUSH_IS_SAFE && current->mm &&
- addr < TASK_SIZE)
+ if (CACHE_FLUSH_IS_SAFE && ipipe_safe_current() == current &&
+ current->mm && addr < TASK_SIZE)
flush_cache_range(current->mm->mmap_cache,
addr, addr + BREAK_INSTR_SIZE);
else if (CACHE_FLUSH_IS_SAFE)
@@ -1032,7 +1032,7 @@
* Interrupts will be restored by the 'trap return' code, except when
* single stepping.
*/
- local_irq_save(flags);
+ local_irq_save_hw(flags);
/* Hold debugger_active */
procid = smp_processor_id();
@@ -1055,7 +1055,7 @@
if (atomic_read(&cpu_doing_single_step) != -1 &&
atomic_read(&cpu_doing_single_step) != procid) {
atomic_set(&debugger_active, 0);
- local_irq_restore(flags);
+ local_irq_restore_hw(flags);
goto acquirelock;
}
@@ -1068,7 +1068,7 @@
goto kgdb_restore;
kgdb_info[processor].debuggerinfo = linux_regs;
- kgdb_info[processor].task = current;
+ kgdb_info[processor].task = ipipe_safe_current();
kgdb_disable_hw_debug(linux_regs);
@@ -1120,7 +1120,8 @@
*ptr++ = hexchars[(signo >> 4) % 16];
*ptr++ = hexchars[signo % 16];
ptr += strlen(strcpy(ptr, "thread:"));
- int_to_threadref(&thref, shadow_pid(current->pid));
+ int_to_threadref(&thref,
+ shadow_pid(ipipe_safe_current()->pid));
ptr = pack_threadid(ptr, &thref);
*ptr++ = ';';
@@ -1212,7 +1213,8 @@
kgdb_hex2mem(&remcom_in_buffer[1], (char *)gdb_regs,
NUMREGBYTES);
- if (kgdb_usethread && kgdb_usethread != current)
+ if (kgdb_usethread &&
+ kgdb_usethread != ipipe_safe_current())
error_packet(remcom_out_buffer, -EINVAL);
else {
gdb_regs_to_regs(gdb_regs, linux_regs);
@@ -1333,7 +1335,8 @@
/* Current thread id */
strcpy(remcom_out_buffer, "QC");
- threadid = shadow_pid(current->pid);
+ threadid =
+ shadow_pid(ipipe_safe_current()->pid);
int_to_threadref(&thref, threadid);
pack_threadid(remcom_out_buffer + 2, &thref);
@@ -1487,7 +1490,8 @@
break;
case 'c':
case 's':
- if (kgdb_contthread && kgdb_contthread != current) {
+ if (kgdb_contthread &&
+ kgdb_contthread != ipipe_safe_current()) {
/* Can't switch threads in kgdb */
error_packet(remcom_out_buffer, -EINVAL);
break;
@@ -1555,7 +1559,7 @@
kgdb_restore:
/* Free debugger_active */
atomic_set(&debugger_active, 0);
- local_irq_restore(flags);
+ local_irq_restore_hw(flags);
return error;
}
@@ -1924,9 +1928,9 @@
if (!kgdb_connected || atomic_read(&debugger_active) != 0)
return 0;
if ((code == SYS_RESTART) || (code == SYS_HALT) || (code == SYS_POWER_OFF)){
- local_irq_save(flags);
+ local_irq_save_hw(flags);
put_packet("X00");
- local_irq_restore(flags);
+ local_irq_restore_hw(flags);
}
return NOTIFY_DONE;
}
@@ -1941,10 +1945,10 @@
if (!kgdb_connected || atomic_read(&debugger_active) != 0)
return;
- local_irq_save(flags);
+ local_irq_save_hw(flags);
kgdb_msg_write(s, count);
- local_irq_restore(flags);
+ local_irq_restore_hw(flags);
}
static struct console kgdbcons = {
--- 2.6.16-base/lib/Kconfig.debug 2006-06-07 16:40:21.000000000 +0200
+++ 2.6.16-kgdb/lib/Kconfig.debug 2006-06-06 16:01:26.000000000 +0200
@@ -250,7 +250,7 @@
config KGDB_ONLY_MODULES
bool "KGDB: Use only kernel modules for I/O"
- depends on MODULES
+ depends on MODULES && !IPIPE
help
Use only kernel modules to configure KGDB I/O after the
kernel is booted.
@@ -295,8 +295,8 @@
endchoice
config KGDBOE
- tristate "KGDB: On ethernet" if !KGDBOE_NOMODULE
+ tristate "KGDB: On ethernet" if !KGDBOE_NOMODULE && !IPIPE
depends on m && KGDB
select NETPOLL
select NETPOLL_TRAP
--- 2.6.16-base/drivers/serial/8250_kgdb.c 2006-06-07 16:40:20.000000000 +0200
+++ 2.6.16-kgdb/drivers/serial/8250_kgdb.c 2006-06-06 16:01:26.000000000 +0200
@@ -301,6 +301,10 @@
"GDB-stub", current_port) < 0)
printk(KERN_ERR "KGDB failed to request the serial IRQ (%d)\n",
current_port->irq);
+#ifdef CONFIG_IPIPE
+ ipipe_control_irq(current_port->irq, 0,
+ IPIPE_HANDLE_MASK|IPIPE_STICKY_MASK|IPIPE_SYSTEM_MASK);
+#endif /* CONFIG_IPIPE */
}
static __init int kgdb_init_io(void)
--- 2.6.16/include/linux/smp.h 2006-03-20 06:53:29.000000000 +0100
+++ 2.6.16-kgdb/include/linux/smp.h 2006-06-07 17:59:13.000000000 +0200
@@ -116,12 +116,16 @@
* which use for some reason is legal). Don't use this to hack around
* the warning message, as your code might not work under PREEMPT.
*/
+#if defined(CONFIG_KGDB) && defined(CONFIG_IPIPE)
+#define smp_processor_id() ipipe_processor_id()
+#else /* CONFIG_KGDB && CONFIG_IPIPE */
#ifdef CONFIG_DEBUG_PREEMPT
extern unsigned int debug_smp_processor_id(void);
# define smp_processor_id() debug_smp_processor_id()
#else
# define smp_processor_id() raw_smp_processor_id()
#endif
+#endif
#define get_cpu() ({ preempt_disable(); smp_processor_id(); })
#define put_cpu() preempt_enable()
[-- Attachment #3: post-kgdb-ipipe-i386.patch --]
[-- Type: text/x-patch, Size: 834 bytes --]
Index: linux-2.6.16.16/arch/i386/kernel/entry.S
===================================================================
--- linux-2.6.16.16.orig/arch/i386/kernel/entry.S
+++ linux-2.6.16.16/arch/i386/kernel/entry.S
@@ -194,7 +194,7 @@ VM_MASK = 0x00020000
.previous
-ENTRY(ret_from_fork)
+KPROBE_ENTRY(ret_from_fork)
STI_COND_HW
pushl %eax
call schedule_tail
@@ -584,7 +584,7 @@ ENTRY(simd_coprocessor_error)
PUSH_XCODE(do_simd_coprocessor_error)
jmp error_code
-ENTRY(device_not_available)
+KPROBE_ENTRY(device_not_available)
pushl $-1 # mark this as an int
SAVE_ALL
DIVERT_EXCEPTION(device_not_available)
@@ -769,7 +769,7 @@ ENTRY(machine_check)
jmp error_code
#endif
-ENTRY(spurious_interrupt_bug)
+KPROBE_ENTRY(spurious_interrupt_bug)
pushl $0
PUSH_XCODE(do_spurious_interrupt_bug)
jmp error_code
[-- Attachment #4: pre-kgdb-ipipe-i386.patch --]
[-- Type: text/x-patch, Size: 811 bytes --]
Index: linux-2.6.16.16/arch/i386/kernel/entry.S
===================================================================
--- linux-2.6.16.16.orig/arch/i386/kernel/entry.S
+++ linux-2.6.16.16/arch/i386/kernel/entry.S
@@ -123,7 +123,7 @@ VM_MASK = 0x00020000
.previous
-KPROBE_ENTRY(ret_from_fork)
+ENTRY(ret_from_fork)
pushl %eax
call schedule_tail
GET_THREAD_INFO(%ebp)
@@ -472,7 +472,7 @@ ENTRY(simd_coprocessor_error)
pushl $do_simd_coprocessor_error
jmp error_code
-KPROBE_ENTRY(device_not_available)
+ENTRY(device_not_available)
pushl $-1 # mark this as an int
SAVE_ALL
movl %cr0, %eax
@@ -654,7 +654,7 @@ ENTRY(machine_check)
jmp error_code
#endif
-KPROBE_ENTRY(spurious_interrupt_bug)
+ENTRY(spurious_interrupt_bug)
pushl $0
pushl $do_spurious_interrupt_bug
jmp error_code
^ permalink raw reply [flat|nested] 11+ messages in thread* [Xenomai-core] Re: [PATCH] kgdb/x86 over I-pipe
2006-06-07 16:19 ` [Xenomai-core] [PATCH] kgdb/x86 over I-pipe Philippe Gerum
@ 2006-06-07 16:50 ` Jan Kiszka
2006-06-08 8:24 ` Philippe Gerum
0 siblings, 1 reply; 11+ messages in thread
From: Jan Kiszka @ 2006-06-07 16:50 UTC (permalink / raw)
To: Philippe Gerum; +Cc: xenomai-core
[-- Attachment #1: Type: text/plain, Size: 2592 bytes --]
Philippe Gerum wrote:
>
> Hi Jan,
>
> Based on your previous work, here is a set of patches coupling KGDB and
> the I-pipe. Basically, I've attempted to shrink the extra patches needed
> against the original KGDB + I-pipe ones to the bare minimum. This has
> been obtained by having the I-pipe provide ipipe_current_safe(), and
> drastically reduce the amount of fiddling with smp_processor_id().
>
> The key difference with the former implementation is that a domain (e.g.
> Xenomai) is now expected to tell the I-pipe when it's switching to a
> non-Linux stack, and the I-pipe makes good use of this information to
> return the proper "current" value when asked to through
> ipipe_safe_current() from the KGDB code. The issue of swapping
This looks nice.
> smp_processor_id() with ipipe_processor_id() has been addressed the hard
> way: smp_processor_id() is simply defined as ipipe_processor_id() when
> CONFIG_IPIPE and CONFIG_KGDB are both enabled in include/linux/smp.h.
> This approach was actually used during the old Adeos times when pipeline
> domain had their own separate stack. I take for granted that the CPU
> penalty taken in doing this is perfectly acceptable, since well, we are
> debugging after all.
There is only one drawback: we will not be able to debug
smp_processor_id-related bugs in ipipe/Xenomai anymore...
>
> Aside of the small patches attached, you will need the latest I-pipe
> 1.3-05 patch for x86, adding the foreign stack notifier and the
> ipipe_safe_current() support:
> http://download.gna.org/adeos/patches/v2.6/i386/adeos-ipipe-2.6.16-i386-1.3-05.patch
>
>
> Patches should be applied in this order on a vanilla 2.6.16 kernel:
>
> - KGDB 2.4 patch series over 2.6.16 (quilt)
> - pre-kgdb-ipipe-i386.patch
> - adeos-ipipe-2.6.16-i386-1.3-05.patch
> - kgdb-ipipe.patch
> - post-kgdb-ipipe-i386.patch
>
> Xenomai's trunk/ should be used. Older code won't work and likely crash
> since the I-pipe would not be notified about foreign stack switches.
>
> Now the surprise: I did not test this stuff, I mean, at all. Eh. :o)
That's fair: leave the head banging while debugging debuggers (*) up to
me. ;)
Will try to let this fly, likely not before the weekend.
BTW, there is one pending issue of gcc-4.1 which popped up under kgdb,
see [1] (also for a workaround).
Jan
(*) Actually, that's feasible: kgdb-patched kernel inside qemu - already
done this (to find [1]), it's just a bit sloooow.
[1]http://sourceforge.net/mailarchive/forum.php?thread_id=10452132&forum_id=5557
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 250 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Xenomai-core] Re: [PATCH] kgdb/x86 over I-pipe
2006-06-07 16:50 ` [Xenomai-core] " Jan Kiszka
@ 2006-06-08 8:24 ` Philippe Gerum
2006-06-10 22:19 ` Jan Kiszka
0 siblings, 1 reply; 11+ messages in thread
From: Philippe Gerum @ 2006-06-08 8:24 UTC (permalink / raw)
To: Jan Kiszka; +Cc: xenomai-core
[-- Attachment #1: Type: text/plain, Size: 2730 bytes --]
Jan Kiszka wrote:
> Philippe Gerum wrote:
>
>>Hi Jan,
>>
>>Based on your previous work, here is a set of patches coupling KGDB and
>>the I-pipe. Basically, I've attempted to shrink the extra patches needed
>>against the original KGDB + I-pipe ones to the bare minimum. This has
>>been obtained by having the I-pipe provide ipipe_current_safe(), and
>>drastically reduce the amount of fiddling with smp_processor_id().
>>
>>The key difference with the former implementation is that a domain (e.g.
>>Xenomai) is now expected to tell the I-pipe when it's switching to a
>>non-Linux stack, and the I-pipe makes good use of this information to
>>return the proper "current" value when asked to through
>>ipipe_safe_current() from the KGDB code. The issue of swapping
>
>
> This looks nice.
>
>
>>smp_processor_id() with ipipe_processor_id() has been addressed the hard
>>way: smp_processor_id() is simply defined as ipipe_processor_id() when
>>CONFIG_IPIPE and CONFIG_KGDB are both enabled in include/linux/smp.h.
>>This approach was actually used during the old Adeos times when pipeline
>>domain had their own separate stack. I take for granted that the CPU
>>penalty taken in doing this is perfectly acceptable, since well, we are
>>debugging after all.
>
>
> There is only one drawback: we will not be able to debug
> smp_processor_id-related bugs in ipipe/Xenomai anymore...
>
Good point. Here is an updated patch.
>
>>Aside of the small patches attached, you will need the latest I-pipe
>>1.3-05 patch for x86, adding the foreign stack notifier and the
>>ipipe_safe_current() support:
>>http://download.gna.org/adeos/patches/v2.6/i386/adeos-ipipe-2.6.16-i386-1.3-05.patch
>>
>>
>>Patches should be applied in this order on a vanilla 2.6.16 kernel:
>>
>>- KGDB 2.4 patch series over 2.6.16 (quilt)
>>- pre-kgdb-ipipe-i386.patch
>>- adeos-ipipe-2.6.16-i386-1.3-05.patch
>>- kgdb-ipipe.patch
>>- post-kgdb-ipipe-i386.patch
>>
>>Xenomai's trunk/ should be used. Older code won't work and likely crash
>>since the I-pipe would not be notified about foreign stack switches.
>>
>>Now the surprise: I did not test this stuff, I mean, at all. Eh. :o)
>
>
> That's fair: leave the head banging while debugging debuggers (*) up to
> me. ;)
>
> Will try to let this fly, likely not before the weekend.
>
> BTW, there is one pending issue of gcc-4.1 which popped up under kgdb,
> see [1] (also for a workaround).
>
Mm, I'll keep on working with gcc3.4 and 8k stacks on x86 then.
> Jan
>
>
> (*) Actually, that's feasible: kgdb-patched kernel inside qemu - already
> done this (to find [1]), it's just a bit sloooow.
>
> [1]http://sourceforge.net/mailarchive/forum.php?thread_id=10452132&forum_id=5557
>
--
Philippe.
[-- Attachment #2: kgdb-ipipe.patch --]
[-- Type: text/x-patch, Size: 5539 bytes --]
--- 2.6.16-base/kernel/kgdb.c 2006-06-07 16:40:21.000000000 +0200
+++ 2.6.16-kgdb/kernel/kgdb.c 2006-06-07 16:43:36.000000000 +0200
@@ -740,10 +740,10 @@
unsigned long flags;
int processor;
- local_irq_save(flags);
+ local_irq_save_hw(flags);
processor = smp_processor_id();
kgdb_info[processor].debuggerinfo = regs;
- kgdb_info[processor].task = current;
+ kgdb_info[processor].task = ipipe_safe_current();
atomic_set(&procindebug[processor], 1);
/* Wait till master processor goes completely into the debugger.
@@ -769,7 +769,7 @@
/* Signal the master processor that we are done */
atomic_set(&procindebug[processor], 0);
spin_unlock(&slavecpulocks[processor]);
- local_irq_restore(flags);
+ local_irq_restore_hw(flags);
}
#endif
@@ -883,8 +883,8 @@
kgdb_break[i].saved_instr)))
return error;
- if (CACHE_FLUSH_IS_SAFE && current->mm &&
- addr < TASK_SIZE)
+ if (CACHE_FLUSH_IS_SAFE && ipipe_safe_current() == current &&
+ current->mm && addr < TASK_SIZE)
flush_cache_range(current->mm->mmap_cache,
addr, addr + BREAK_INSTR_SIZE);
else if (CACHE_FLUSH_IS_SAFE)
@@ -1032,7 +1032,7 @@
* Interrupts will be restored by the 'trap return' code, except when
* single stepping.
*/
- local_irq_save(flags);
+ local_irq_save_hw(flags);
/* Hold debugger_active */
procid = smp_processor_id();
@@ -1055,7 +1055,7 @@
if (atomic_read(&cpu_doing_single_step) != -1 &&
atomic_read(&cpu_doing_single_step) != procid) {
atomic_set(&debugger_active, 0);
- local_irq_restore(flags);
+ local_irq_restore_hw(flags);
goto acquirelock;
}
@@ -1068,7 +1068,7 @@
goto kgdb_restore;
kgdb_info[processor].debuggerinfo = linux_regs;
- kgdb_info[processor].task = current;
+ kgdb_info[processor].task = ipipe_safe_current();
kgdb_disable_hw_debug(linux_regs);
@@ -1120,7 +1120,8 @@
*ptr++ = hexchars[(signo >> 4) % 16];
*ptr++ = hexchars[signo % 16];
ptr += strlen(strcpy(ptr, "thread:"));
- int_to_threadref(&thref, shadow_pid(current->pid));
+ int_to_threadref(&thref,
+ shadow_pid(ipipe_safe_current()->pid));
ptr = pack_threadid(ptr, &thref);
*ptr++ = ';';
@@ -1212,7 +1213,8 @@
kgdb_hex2mem(&remcom_in_buffer[1], (char *)gdb_regs,
NUMREGBYTES);
- if (kgdb_usethread && kgdb_usethread != current)
+ if (kgdb_usethread &&
+ kgdb_usethread != ipipe_safe_current())
error_packet(remcom_out_buffer, -EINVAL);
else {
gdb_regs_to_regs(gdb_regs, linux_regs);
@@ -1333,7 +1335,8 @@
/* Current thread id */
strcpy(remcom_out_buffer, "QC");
- threadid = shadow_pid(current->pid);
+ threadid =
+ shadow_pid(ipipe_safe_current()->pid);
int_to_threadref(&thref, threadid);
pack_threadid(remcom_out_buffer + 2, &thref);
@@ -1487,7 +1490,8 @@
break;
case 'c':
case 's':
- if (kgdb_contthread && kgdb_contthread != current) {
+ if (kgdb_contthread &&
+ kgdb_contthread != ipipe_safe_current()) {
/* Can't switch threads in kgdb */
error_packet(remcom_out_buffer, -EINVAL);
break;
@@ -1555,7 +1559,7 @@
kgdb_restore:
/* Free debugger_active */
atomic_set(&debugger_active, 0);
- local_irq_restore(flags);
+ local_irq_restore_hw(flags);
return error;
}
@@ -1924,9 +1928,9 @@
if (!kgdb_connected || atomic_read(&debugger_active) != 0)
return 0;
if ((code == SYS_RESTART) || (code == SYS_HALT) || (code == SYS_POWER_OFF)){
- local_irq_save(flags);
+ local_irq_save_hw(flags);
put_packet("X00");
- local_irq_restore(flags);
+ local_irq_restore_hw(flags);
}
return NOTIFY_DONE;
}
@@ -1941,10 +1945,10 @@
if (!kgdb_connected || atomic_read(&debugger_active) != 0)
return;
- local_irq_save(flags);
+ local_irq_save_hw(flags);
kgdb_msg_write(s, count);
- local_irq_restore(flags);
+ local_irq_restore_hw(flags);
}
static struct console kgdbcons = {
--- 2.6.16-base/lib/Kconfig.debug 2006-06-07 16:40:21.000000000 +0200
+++ 2.6.16-kgdb/lib/Kconfig.debug 2006-06-06 16:01:26.000000000 +0200
@@ -250,7 +250,7 @@
config KGDB_ONLY_MODULES
bool "KGDB: Use only kernel modules for I/O"
- depends on MODULES
+ depends on MODULES && !IPIPE
help
Use only kernel modules to configure KGDB I/O after the
kernel is booted.
@@ -295,8 +295,8 @@
endchoice
config KGDBOE
- tristate "KGDB: On ethernet" if !KGDBOE_NOMODULE
+ tristate "KGDB: On ethernet" if !KGDBOE_NOMODULE && !IPIPE
depends on m && KGDB
select NETPOLL
select NETPOLL_TRAP
--- 2.6.16-base/drivers/serial/8250_kgdb.c 2006-06-07 16:40:20.000000000 +0200
+++ 2.6.16-kgdb/drivers/serial/8250_kgdb.c 2006-06-06 16:01:26.000000000 +0200
@@ -301,6 +301,10 @@
"GDB-stub", current_port) < 0)
printk(KERN_ERR "KGDB failed to request the serial IRQ (%d)\n",
current_port->irq);
+#ifdef CONFIG_IPIPE
+ ipipe_control_irq(current_port->irq, 0,
+ IPIPE_HANDLE_MASK|IPIPE_STICKY_MASK|IPIPE_SYSTEM_MASK);
+#endif /* CONFIG_IPIPE */
}
static __init int kgdb_init_io(void)
--- 2.6.16-base/include/asm-i386/smp.h 2006-03-20 06:53:29.000000000 +0100
+++ 2.6.16-kgdb/include/asm-i386/smp.h 2006-06-08 09:49:07.000000000 +0200
@@ -57,7 +57,11 @@
* from the initial startup. We map APIC_BASE very early in page_setup(),
* so this is correct in the x86 case.
*/
+#if defined(CONFIG_KGDB) && defined(CONFIG_IPIPE)
+#define raw_smp_processor_id() ipipe_processor_id()
+#else
#define raw_smp_processor_id() (current_thread_info()->cpu)
+#endif
extern cpumask_t cpu_callout_map;
extern cpumask_t cpu_callin_map;
^ permalink raw reply [flat|nested] 11+ messages in thread* [Xenomai-core] Re: [PATCH] kgdb/x86 over I-pipe
2006-06-08 8:24 ` Philippe Gerum
@ 2006-06-10 22:19 ` Jan Kiszka
2006-06-12 8:27 ` Philippe Gerum
0 siblings, 1 reply; 11+ messages in thread
From: Jan Kiszka @ 2006-06-10 22:19 UTC (permalink / raw)
To: Philippe Gerum; +Cc: xenomai-core
[-- Attachment #1: Type: text/plain, Size: 2317 bytes --]
Philippe Gerum wrote:
> Jan Kiszka wrote:
>> Philippe Gerum wrote:
>>
>>> Hi Jan,
>>>
>>> Based on your previous work, here is a set of patches coupling KGDB and
>>> the I-pipe. Basically, I've attempted to shrink the extra patches needed
>>> against the original KGDB + I-pipe ones to the bare minimum. This has
>>> been obtained by having the I-pipe provide ipipe_current_safe(), and
>>> drastically reduce the amount of fiddling with smp_processor_id().
>>>
>>> The key difference with the former implementation is that a domain (e.g.
>>> Xenomai) is now expected to tell the I-pipe when it's switching to a
>>> non-Linux stack, and the I-pipe makes good use of this information to
>>> return the proper "current" value when asked to through
>>> ipipe_safe_current() from the KGDB code. The issue of swapping
>>
>>
>> This looks nice.
>>
>>
>>> smp_processor_id() with ipipe_processor_id() has been addressed the hard
>>> way: smp_processor_id() is simply defined as ipipe_processor_id() when
>>> CONFIG_IPIPE and CONFIG_KGDB are both enabled in include/linux/smp.h.
>>> This approach was actually used during the old Adeos times when pipeline
>>> domain had their own separate stack. I take for granted that the CPU
>>> penalty taken in doing this is perfectly acceptable, since well, we are
>>> debugging after all.
>>
>>
>> There is only one drawback: we will not be able to debug
>> smp_processor_id-related bugs in ipipe/Xenomai anymore...
>>
>
> Good point. Here is an updated patch.
>
Nope that's not the point I meant. I was referring to bugs like the
missing smp_processor_id patch in asm-i386/mmu_context.h. Your way makes
such problems disappear once you switch on the debugger. Remember that
we spotted the mmu_context issue via kgdb.
The attached kgdb-ipipe patch (which also updates to kgdb-CVS head)
addresses maintenance and smp-safeness by redefining the involved
services only per source file, not kernel-wide. What's do you think
about this approach?
I'm also posting two fixes for ipipe itself to 1) make kgdb work over
SMP and 2) fix a compiler warning.
This version works fine for me, at least in qemu. I tried to check SMP
as well, but qemu obviously lacks support for NMIs raised via IPI, and
this is used by kgdb to sync all CPUs on debugging events (vanilla kgdb
suffers too).
Jan
[-- Attachment #2: kgdb-ipipe.patch --]
[-- Type: text/x-patch, Size: 4738 bytes --]
Index: linux/kernel/kgdb.c
===================================================================
--- linux.orig/kernel/kgdb.c
+++ linux/kernel/kgdb.c
@@ -49,6 +49,15 @@
#include <linux/console.h>
#include <asm/byteorder.h>
+#if defined(CONFIG_KGDB) && defined(CONFIG_IPIPE)
+#undef smp_processor_id
+#undef local_irq_save
+#undef local_irq_restore
+#define smp_processor_id ipipe_processor_id
+#define local_irq_save local_irq_save_hw
+#define local_irq_restore local_irq_restore_hw
+#endif
+
extern int pid_max;
extern int pidhash_init_done;
@@ -743,7 +752,7 @@ static void kgdb_wait(struct pt_regs *re
local_irq_save(flags);
processor = smp_processor_id();
kgdb_info[processor].debuggerinfo = regs;
- kgdb_info[processor].task = current;
+ kgdb_info[processor].task = ipipe_safe_current();
atomic_set(&procindebug[processor], 1);
atomic_set(&kgdb_sync_softlockup[smp_processor_id()], 1);
@@ -884,8 +893,8 @@ int kgdb_deactivate_sw_breakpoints(void)
kgdb_break[i].saved_instr)))
return error;
- if (CACHE_FLUSH_IS_SAFE && current->mm &&
- addr < TASK_SIZE)
+ if (CACHE_FLUSH_IS_SAFE && ipipe_safe_current() == current &&
+ current->mm && addr < TASK_SIZE)
flush_cache_range(current->mm->mmap_cache,
addr, addr + BREAK_INSTR_SIZE);
else if (CACHE_FLUSH_IS_SAFE)
@@ -1069,7 +1078,7 @@ int kgdb_handle_exception(int ex_vector,
goto kgdb_restore;
kgdb_info[processor].debuggerinfo = linux_regs;
- kgdb_info[processor].task = current;
+ kgdb_info[processor].task = ipipe_safe_current();
kgdb_disable_hw_debug(linux_regs);
@@ -1121,7 +1130,8 @@ int kgdb_handle_exception(int ex_vector,
*ptr++ = hexchars[(signo >> 4) % 16];
*ptr++ = hexchars[signo % 16];
ptr += strlen(strcpy(ptr, "thread:"));
- int_to_threadref(&thref, shadow_pid(current->pid));
+ int_to_threadref(&thref,
+ shadow_pid(ipipe_safe_current()->pid));
ptr = pack_threadid(ptr, &thref);
*ptr++ = ';';
@@ -1213,7 +1223,8 @@ int kgdb_handle_exception(int ex_vector,
kgdb_hex2mem(&remcom_in_buffer[1], (char *)gdb_regs,
NUMREGBYTES);
- if (kgdb_usethread && kgdb_usethread != current)
+ if (kgdb_usethread &&
+ kgdb_usethread != ipipe_safe_current())
error_packet(remcom_out_buffer, -EINVAL);
else {
gdb_regs_to_regs(gdb_regs, linux_regs);
@@ -1334,7 +1345,8 @@ int kgdb_handle_exception(int ex_vector,
/* Current thread id */
strcpy(remcom_out_buffer, "QC");
- threadid = shadow_pid(current->pid);
+ threadid =
+ shadow_pid(ipipe_safe_current()->pid);
int_to_threadref(&thref, threadid);
pack_threadid(remcom_out_buffer + 2, &thref);
@@ -1488,7 +1500,8 @@ int kgdb_handle_exception(int ex_vector,
break;
case 'c':
case 's':
- if (kgdb_contthread && kgdb_contthread != current) {
+ if (kgdb_contthread &&
+ kgdb_contthread != ipipe_safe_current()) {
/* Can't switch threads in kgdb */
error_packet(remcom_out_buffer, -EINVAL);
break;
Index: linux/lib/Kconfig.debug
===================================================================
--- linux.orig/lib/Kconfig.debug
+++ linux/lib/Kconfig.debug
@@ -273,7 +273,7 @@ choice
config KGDB_ONLY_MODULES
bool "KGDB: Use only kernel modules for I/O"
- depends on MODULES
+ depends on MODULES && !IPIPE
help
Use only kernel modules to configure KGDB I/O after the
kernel is booted.
@@ -318,7 +318,7 @@ config KGDB_SIBYTE
endchoice
config KGDBOE
- tristate "KGDB: On ethernet" if !KGDBOE_NOMODULE
+ tristate "KGDB: On ethernet" if !KGDBOE_NOMODULE && !IPIPE
depends on m && KGDB
select NETPOLL
select NETPOLL_TRAP
Index: linux/drivers/serial/8250_kgdb.c
===================================================================
--- linux.orig/drivers/serial/8250_kgdb.c
+++ linux/drivers/serial/8250_kgdb.c
@@ -301,6 +301,10 @@ static void __init kgdb8250_late_init(vo
"GDB-stub", current_port) < 0)
printk(KERN_ERR "KGDB failed to request the serial IRQ (%d)\n",
current_port->irq);
+#ifdef CONFIG_IPIPE
+ ipipe_control_irq(current_port->irq, 0,
+ IPIPE_HANDLE_MASK|IPIPE_STICKY_MASK|IPIPE_SYSTEM_MASK);
+#endif /* CONFIG_IPIPE */
}
static __init int kgdb_init_io(void)
Index: linux/arch/i386/kernel/kgdb.c
===================================================================
--- linux.orig/arch/i386/kernel/kgdb.c
+++ linux/arch/i386/kernel/kgdb.c
@@ -43,6 +43,11 @@
#include "mach_ipi.h"
+#if defined(CONFIG_KGDB) && defined(CONFIG_IPIPE)
+#undef smp_processor_id
+#define smp_processor_id ipipe_processor_id
+#endif
+
/* Put the error code here just in case the user cares. */
int gdb_i386errcode;
/* Likewise, the vector number here (since GDB only gets the signal
[-- Attachment #3: load-cpuid-in-foreign_stack.patch --]
[-- Type: text/x-patch, Size: 640 bytes --]
Index: linux-2.6.16.16/include/linux/ipipe.h
===================================================================
--- linux-2.6.16.16.orig/include/linux/ipipe.h
+++ linux-2.6.16.16/include/linux/ipipe.h
@@ -617,6 +617,7 @@ static inline void ipipe_set_foreign_sta
{
/* Must be called hw interrupts off. */
ipipe_declare_cpuid;
+ ipipe_load_cpuid();
__set_bit(IPIPE_NOSTACK_FLAG, &ipd->cpudata[cpuid].status);
}
@@ -624,6 +625,7 @@ static inline void ipipe_clear_foreign_s
{
/* Must be called hw interrupts off. */
ipipe_declare_cpuid;
+ ipipe_load_cpuid();
__clear_bit(IPIPE_NOSTACK_FLAG, &ipd->cpudata[cpuid].status);
}
[-- Attachment #4: add-kgdb-include.patch --]
[-- Type: text/x-patch, Size: 413 bytes --]
Index: linux-2.6.16.16/arch/i386/kernel/ipipe-root.c
===================================================================
--- linux-2.6.16.16.orig/arch/i386/kernel/ipipe-root.c
+++ linux-2.6.16.16/arch/i386/kernel/ipipe-root.c
@@ -423,6 +423,8 @@ static __ipipe_exptr __ipipe_std_extable
};
#ifdef CONFIG_KGDB
+#include <linux/kgdb.h>
+
static int __ipipe_xlate_signo[] = {
[ex_do_divide_error] = SIGFPE,
^ permalink raw reply [flat|nested] 11+ messages in thread* [Xenomai-core] Re: [PATCH] kgdb/x86 over I-pipe
2006-06-10 22:19 ` Jan Kiszka
@ 2006-06-12 8:27 ` Philippe Gerum
2006-06-12 8:40 ` Jan Kiszka
0 siblings, 1 reply; 11+ messages in thread
From: Philippe Gerum @ 2006-06-12 8:27 UTC (permalink / raw)
To: Jan Kiszka; +Cc: xenomai-core
Jan Kiszka wrote:
> Philippe Gerum wrote:
>
>>Jan Kiszka wrote:
>>
>>>Philippe Gerum wrote:
>>>
>>>
>>>>Hi Jan,
>>>>
>>>>Based on your previous work, here is a set of patches coupling KGDB and
>>>>the I-pipe. Basically, I've attempted to shrink the extra patches needed
>>>>against the original KGDB + I-pipe ones to the bare minimum. This has
>>>>been obtained by having the I-pipe provide ipipe_current_safe(), and
>>>>drastically reduce the amount of fiddling with smp_processor_id().
>>>>
>>>>The key difference with the former implementation is that a domain (e.g.
>>>>Xenomai) is now expected to tell the I-pipe when it's switching to a
>>>>non-Linux stack, and the I-pipe makes good use of this information to
>>>>return the proper "current" value when asked to through
>>>>ipipe_safe_current() from the KGDB code. The issue of swapping
>>>
>>>
>>>This looks nice.
>>>
>>>
>>>
>>>>smp_processor_id() with ipipe_processor_id() has been addressed the hard
>>>>way: smp_processor_id() is simply defined as ipipe_processor_id() when
>>>>CONFIG_IPIPE and CONFIG_KGDB are both enabled in include/linux/smp.h.
>>>>This approach was actually used during the old Adeos times when pipeline
>>>>domain had their own separate stack. I take for granted that the CPU
>>>>penalty taken in doing this is perfectly acceptable, since well, we are
>>>>debugging after all.
>>>
>>>
>>>There is only one drawback: we will not be able to debug
>>>smp_processor_id-related bugs in ipipe/Xenomai anymore...
>>>
>>
>>Good point. Here is an updated patch.
>>
>
>
> Nope that's not the point I meant. I was referring to bugs like the
> missing smp_processor_id patch in asm-i386/mmu_context.h. Your way makes
> such problems disappear once you switch on the debugger. Remember that
> we spotted the mmu_context issue via kgdb.
>
Got it now. Indeed, making such kind of bugs disappear would be a very
undesirable side-effect of enabling KGDB.
> The attached kgdb-ipipe patch (which also updates to kgdb-CVS head)
> addresses maintenance and smp-safeness by redefining the involved
> services only per source file, not kernel-wide. What's do you think
> about this approach?
>
Mixed feelings. On one hand, it locally fixes the issue without masking
any bugs and that's good. On the other hand, I've been bitten using this
kind of tricks in earlier Adeos releases for patching printk() the same
way, substituting it with spin_lock_irqsave_hw. At some point,
spin_lock_irqsave became local_irq_save + spin_lock in the vanilla code,
making the substitution of the former pointless, and opening a
preemption hole in the code. This said, no spinlock macros are involved
in the substitution you are suggesting, so there is no such risk, and in
any case, the risk is confined to one file and not spread all over the
place like with the raw_smp_processor_id() substitution. IOW, let's go
your way.
While we are at it,
#define current ipipe_safe_current() /* ? */
> I'm also posting two fixes for ipipe itself to 1) make kgdb work over
> SMP and 2) fix a compiler warning.
>
> This version works fine for me, at least in qemu. I tried to check SMP
> as well, but qemu obviously lacks support for NMIs raised via IPI, and
> this is used by kgdb to sync all CPUs on debugging events (vanilla kgdb
> suffers too).
>
--
Philippe.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Xenomai-core] Re: [PATCH] kgdb/x86 over I-pipe
2006-06-12 8:27 ` Philippe Gerum
@ 2006-06-12 8:40 ` Jan Kiszka
2006-06-12 9:27 ` Philippe Gerum
0 siblings, 1 reply; 11+ messages in thread
From: Jan Kiszka @ 2006-06-12 8:40 UTC (permalink / raw)
To: Philippe Gerum; +Cc: xenomai-core
[-- Attachment #1: Type: text/plain, Size: 3927 bytes --]
Philippe Gerum wrote:
> Jan Kiszka wrote:
>> Philippe Gerum wrote:
>>
>>> Jan Kiszka wrote:
>>>
>>>> Philippe Gerum wrote:
>>>>
>>>>
>>>>> Hi Jan,
>>>>>
>>>>> Based on your previous work, here is a set of patches coupling KGDB
>>>>> and
>>>>> the I-pipe. Basically, I've attempted to shrink the extra patches
>>>>> needed
>>>>> against the original KGDB + I-pipe ones to the bare minimum. This has
>>>>> been obtained by having the I-pipe provide ipipe_current_safe(), and
>>>>> drastically reduce the amount of fiddling with smp_processor_id().
>>>>>
>>>>> The key difference with the former implementation is that a domain
>>>>> (e.g.
>>>>> Xenomai) is now expected to tell the I-pipe when it's switching to a
>>>>> non-Linux stack, and the I-pipe makes good use of this information to
>>>>> return the proper "current" value when asked to through
>>>>> ipipe_safe_current() from the KGDB code. The issue of swapping
>>>>
>>>>
>>>> This looks nice.
>>>>
>>>>
>>>>
>>>>> smp_processor_id() with ipipe_processor_id() has been addressed the
>>>>> hard
>>>>> way: smp_processor_id() is simply defined as ipipe_processor_id() when
>>>>> CONFIG_IPIPE and CONFIG_KGDB are both enabled in include/linux/smp.h.
>>>>> This approach was actually used during the old Adeos times when
>>>>> pipeline
>>>>> domain had their own separate stack. I take for granted that the CPU
>>>>> penalty taken in doing this is perfectly acceptable, since well, we
>>>>> are
>>>>> debugging after all.
>>>>
>>>>
>>>> There is only one drawback: we will not be able to debug
>>>> smp_processor_id-related bugs in ipipe/Xenomai anymore...
>>>>
>>>
>>> Good point. Here is an updated patch.
>>>
>>
>>
>> Nope that's not the point I meant. I was referring to bugs like the
>> missing smp_processor_id patch in asm-i386/mmu_context.h. Your way makes
>> such problems disappear once you switch on the debugger. Remember that
>> we spotted the mmu_context issue via kgdb.
>>
>
> Got it now. Indeed, making such kind of bugs disappear would be a very
> undesirable side-effect of enabling KGDB.
>
>> The attached kgdb-ipipe patch (which also updates to kgdb-CVS head)
>> addresses maintenance and smp-safeness by redefining the involved
>> services only per source file, not kernel-wide. What's do you think
>> about this approach?
>>
>
> Mixed feelings. On one hand, it locally fixes the issue without masking
> any bugs and that's good. On the other hand, I've been bitten using this
> kind of tricks in earlier Adeos releases for patching printk() the same
> way, substituting it with spin_lock_irqsave_hw. At some point,
> spin_lock_irqsave became local_irq_save + spin_lock in the vanilla code,
> making the substitution of the former pointless, and opening a
> preemption hole in the code. This said, no spinlock macros are involved
> in the substitution you are suggesting, so there is no such risk, and in
> any case, the risk is confined to one file and not spread all over the
> place like with the raw_smp_processor_id() substitution. IOW, let's go
> your way.
I agree that it's kind of fragile, but we need to check on kgdb updates
anyway. I hope for kgdb becoming mainline one day. Then the code should
stabilise (if it hasn't already), and we can track it via ipipe directly
- without tricks. So far, this should help to keep efforts lower for us.
>
> While we are at it,
> #define current ipipe_safe_current() /* ? */
Nope, there is the need for some special changes.
>
>> I'm also posting two fixes for ipipe itself to 1) make kgdb work over
>> SMP and 2) fix a compiler warning.
>>
>> This version works fine for me, at least in qemu. I tried to check SMP
>> as well, but qemu obviously lacks support for NMIs raised via IPI, and
>> this is used by kgdb to sync all CPUs on debugging events (vanilla kgdb
>> suffers too).
>>
>
Jan
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 250 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Xenomai-core] Re: [PATCH] kgdb/x86 over I-pipe
2006-06-12 8:40 ` Jan Kiszka
@ 2006-06-12 9:27 ` Philippe Gerum
2006-06-15 12:42 ` Jan Kiszka
0 siblings, 1 reply; 11+ messages in thread
From: Philippe Gerum @ 2006-06-12 9:27 UTC (permalink / raw)
To: Jan Kiszka; +Cc: xenomai-core
Jan Kiszka wrote:
> Philippe Gerum wrote:
>
>>Jan Kiszka wrote:
>>
>>>Philippe Gerum wrote:
>>>
>>>
>>>>Jan Kiszka wrote:
>>>>
>>>>
>>>>>Philippe Gerum wrote:
>>>>>
>>>>>
>>>>>
>>>>>>Hi Jan,
>>>>>>
>>>>>>Based on your previous work, here is a set of patches coupling KGDB
>>>>>>and
>>>>>>the I-pipe. Basically, I've attempted to shrink the extra patches
>>>>>>needed
>>>>>>against the original KGDB + I-pipe ones to the bare minimum. This has
>>>>>>been obtained by having the I-pipe provide ipipe_current_safe(), and
>>>>>>drastically reduce the amount of fiddling with smp_processor_id().
>>>>>>
>>>>>>The key difference with the former implementation is that a domain
>>>>>>(e.g.
>>>>>>Xenomai) is now expected to tell the I-pipe when it's switching to a
>>>>>>non-Linux stack, and the I-pipe makes good use of this information to
>>>>>>return the proper "current" value when asked to through
>>>>>>ipipe_safe_current() from the KGDB code. The issue of swapping
>>>>>
>>>>>
>>>>>This looks nice.
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>>smp_processor_id() with ipipe_processor_id() has been addressed the
>>>>>>hard
>>>>>>way: smp_processor_id() is simply defined as ipipe_processor_id() when
>>>>>>CONFIG_IPIPE and CONFIG_KGDB are both enabled in include/linux/smp.h.
>>>>>>This approach was actually used during the old Adeos times when
>>>>>>pipeline
>>>>>>domain had their own separate stack. I take for granted that the CPU
>>>>>>penalty taken in doing this is perfectly acceptable, since well, we
>>>>>>are
>>>>>>debugging after all.
>>>>>
>>>>>
>>>>>There is only one drawback: we will not be able to debug
>>>>>smp_processor_id-related bugs in ipipe/Xenomai anymore...
>>>>>
>>>>
>>>>Good point. Here is an updated patch.
>>>>
>>>
>>>
>>>Nope that's not the point I meant. I was referring to bugs like the
>>>missing smp_processor_id patch in asm-i386/mmu_context.h. Your way makes
>>>such problems disappear once you switch on the debugger. Remember that
>>>we spotted the mmu_context issue via kgdb.
>>>
>>
>>Got it now. Indeed, making such kind of bugs disappear would be a very
>>undesirable side-effect of enabling KGDB.
>>
>>
>>>The attached kgdb-ipipe patch (which also updates to kgdb-CVS head)
>>>addresses maintenance and smp-safeness by redefining the involved
>>>services only per source file, not kernel-wide. What's do you think
>>>about this approach?
>>>
>>
>>Mixed feelings. On one hand, it locally fixes the issue without masking
>>any bugs and that's good. On the other hand, I've been bitten using this
>>kind of tricks in earlier Adeos releases for patching printk() the same
>>way, substituting it with spin_lock_irqsave_hw. At some point,
>>spin_lock_irqsave became local_irq_save + spin_lock in the vanilla code,
>>making the substitution of the former pointless, and opening a
>>preemption hole in the code. This said, no spinlock macros are involved
>>in the substitution you are suggesting, so there is no such risk, and in
>>any case, the risk is confined to one file and not spread all over the
>>place like with the raw_smp_processor_id() substitution. IOW, let's go
>>your way.
>
>
> I agree that it's kind of fragile, but we need to check on kgdb updates
> anyway. I hope for kgdb becoming mainline one day. Then the code should
> stabilise (if it hasn't already), and we can track it via ipipe directly
> - without tricks. So far, this should help to keep efforts lower for us.
>
>
>>While we are at it,
>>#define current ipipe_safe_current() /* ? */
>
>
> Nope, there is the need for some special changes.
>
If you refer to the cache flushing issue, then it would be better to
actually check for foreign stacks explicitely, so that you could
substitute current globally:
- if (CACHE_FLUSH_IS_SAFE && current->mm &&
- addr < TASK_SIZE)
+ if (CACHE_FLUSH_IS_SAFE && !testbit(IPIPE_NOSTACK_FLAG, +
&ipipe_percpu_domain[cpuid]->cpudata[cpuid].status) &&
+ current->mm && addr < TASK_SIZE)
--
Philippe.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Xenomai-core] Re: [PATCH] kgdb/x86 over I-pipe
2006-06-12 9:27 ` Philippe Gerum
@ 2006-06-15 12:42 ` Jan Kiszka
2006-06-15 14:24 ` Philippe Gerum
0 siblings, 1 reply; 11+ messages in thread
From: Jan Kiszka @ 2006-06-15 12:42 UTC (permalink / raw)
To: Philippe Gerum; +Cc: xenomai-core
[-- Attachment #1: Type: text/plain, Size: 1010 bytes --]
Philippe Gerum wrote:
> Jan Kiszka wrote:
>> Philippe Gerum wrote:
>>> While we are at it,
>>> #define current ipipe_safe_current() /* ? */
>>
>>
>> Nope, there is the need for some special changes.
>>
>
> If you refer to the cache flushing issue, then it would be better to
> actually check for foreign stacks explicitely, so that you could
> substitute current globally:
>
> - if (CACHE_FLUSH_IS_SAFE && current->mm &&
> - addr < TASK_SIZE)
> + if (CACHE_FLUSH_IS_SAFE && !testbit(IPIPE_NOSTACK_FLAG,
> + &ipipe_percpu_domain[cpuid]->cpudata[cpuid].status) &&
> + current->mm && addr < TASK_SIZE)
>
Right - on first sight. I tried to redefine current, but the
ipipe_safe_current macro requires that symbol itself, ugh. Turning
ipipe_safe_current into a static inline doesn't work due to circular
dependencies on linux/sched.h.
So I guess it's best to keep it as it is (though kgdb-ipipe.patch would
have become really cute).
Jan
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 250 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Xenomai-core] Re: [PATCH] kgdb/x86 over I-pipe
2006-06-15 12:42 ` Jan Kiszka
@ 2006-06-15 14:24 ` Philippe Gerum
0 siblings, 0 replies; 11+ messages in thread
From: Philippe Gerum @ 2006-06-15 14:24 UTC (permalink / raw)
To: Jan Kiszka; +Cc: xenomai-core
Jan Kiszka wrote:
> Philippe Gerum wrote:
>
>>Jan Kiszka wrote:
>>
>>>Philippe Gerum wrote:
>>>
>>>>While we are at it,
>>>>#define current ipipe_safe_current() /* ? */
>>>
>>>
>>>Nope, there is the need for some special changes.
>>>
>>
>>If you refer to the cache flushing issue, then it would be better to
>>actually check for foreign stacks explicitely, so that you could
>>substitute current globally:
>>
>>- if (CACHE_FLUSH_IS_SAFE && current->mm &&
>>- addr < TASK_SIZE)
>>+ if (CACHE_FLUSH_IS_SAFE && !testbit(IPIPE_NOSTACK_FLAG,
>>+ &ipipe_percpu_domain[cpuid]->cpudata[cpuid].status) &&
>>+ current->mm && addr < TASK_SIZE)
>>
>
>
> Right - on first sight. I tried to redefine current, but the
> ipipe_safe_current macro requires that symbol itself, ugh. Turning
> ipipe_safe_current into a static inline doesn't work due to circular
> dependencies on linux/sched.h.
>
> So I guess it's best to keep it as it is (though kgdb-ipipe.patch would
> have become really cute).
>
Ack.
--
Philippe.
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2006-06-15 14:24 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-05-22 19:06 [Xenomai-core] [PATCH] kgdb-ipipe for 2.6.16 Jan Kiszka
2006-06-01 8:00 ` Jan Kiszka
2006-06-07 16:19 ` [Xenomai-core] [PATCH] kgdb/x86 over I-pipe Philippe Gerum
2006-06-07 16:50 ` [Xenomai-core] " Jan Kiszka
2006-06-08 8:24 ` Philippe Gerum
2006-06-10 22:19 ` Jan Kiszka
2006-06-12 8:27 ` Philippe Gerum
2006-06-12 8:40 ` Jan Kiszka
2006-06-12 9:27 ` Philippe Gerum
2006-06-15 12:42 ` Jan Kiszka
2006-06-15 14:24 ` Philippe Gerum
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.