On Jan 13, 2006, at 5:28 AM, Isaku Yamahata wrote:
attached the updated one. I tested only very roughly.
Hollis, is this patch ok for ppc version?
Thanks Isaku! This whole patch has whitespace issues though; please
replace your tabs with 4 spaces.
+ . = ALIGN(32);
+ __gdbstub_text_start = . ;
+ *(.gdbstub.text)
+ __gdbstub_text_end = . ;
+ . = ALIGN(32) ;
...
+/* Append this to the declaration of any function needed to be used by
+ * the gdb stub. Any functions marked with this will not be allowed
+ * to have a breakpoint set in them. */
+#define __gdbstub __attribute__((section(".gdbstub.text")))
These make sense if we want to avoid setting a breakpoint in GDB stub
code or code called by the GDB stub, such as serial output. However,
these symbols aren't currently used. (You probably inherited this from
the PPC stub.) At least it's worth a TODO comment, possibly in
gdb_cmd_write_mem(): we shouldn't be writing to memory between those
two symbols.
+/* XXX move to some shared location... */
+char
+hex2char(unsigned long x)
With hex2char, char2hex, str2hex, str2ulong, I expect that they will be
needed in the arch-specific gdb backends. Actually I guess they just
need to be added to gdbstub.h; I'm not sure why I added that comment.
+ for (ctx->in_bytes = 0; ctx->in_bytes < sizeof(ctx->in_buf);
ctx->in_bytes++) {
Exceeds 80 chars.
+#ifdef CONFIG_GDB_NO_CSUM
+ return 0;
+#else
+ if (received_csum == csum) {
+ return 0;
+ } else {
+ return -1;
+ }
+#endif
What is this about? As far as I know, the GDB protocol unconditionally
requires the checksum.
+ //XXX optimization to use best native load size
...
+ //XXX optimization to use best native store size
I think it's reasonable to leave it up to the arch code to decide what
size access to use, so these comments could go.
+ .signum = 1,
+
+ .in_bytes = 0,
+
+ .out_offset = 0,
Blank lines.
+ /* Try to make things a little more stable by disabling
+ interrupts while we're here. */
+ local_irq_save(flags);
...
+ local_irq_restore(flags);
Shouldn't we enter __trap_to_gdb with interrupts disabled already?
That's how it's done on PPC, I believe x86 can choose in the IDT, and
I'm sure IA64 can too?
Hmm, actually, how does this interact with interrupt-driven ns16550?
--
Hollis Blanchard
IBM Linux Technology Center
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
| 
Home