Re: [Kgdb-bugreport] [KGDB PATCH][2/7] Serial updates, take 2

George Anzinger
2004-02-27T23:54:06Z

Tom Rini wrote:
> On Fri, Feb 27, 2004 at 02:44:33PM -0800, George Anzinger wrote:
>
>
>>Couple of comments below...
>>
>>Tom Rini wrote:
>
> [snip]
>
>>>- spin_unlock(&uart_interrupt_lock);
>>>- local_irq_restore(flags);
>>>-
>>
>>I think you need at least this (especially in SMP, but works in all):
>> char iir = serial_inb(kgdb8250_port + (UART_IIR << reg_shift));
>> if(iir & UART_IIR_RDI){
>>
>>>+ kgdb_schedule_breakpoint();
>>
>> }
>>
>>> return IRQ_HANDLED;
>
>
> Would this be to ensure that we only schedule one breakpoint not 2?
Well, that too, but the notion is to take care of an interrupt on cpu 1 while
doing console output on cpu 0. If cpu 0 doesn't grab the '+' fast enough to
prevent the interrupt cpu 1 may get an interrupt with no characters in the fifo.
This code just ignores that.
>
> [snip]
>
>>>+ /* If we get the start of another packet, this means
>>>+ * that GDB is attempting to reconnect. We will NAK
>>>+ * the packet being sent, and stop trying to send this
>>>+ * packet. */
>>>+ if (ch == '$') {
>>>+ kgdb_serial->write_char('-');
>>> if (kgdb_serial->flush)
>>> kgdb_serial->flush();
>>>- breakpoint();
>>
>>Flags go up in my mind here about recursion... What if we are already
>>handling a breakpoint??? This may all be cool, but, as I said, alarms are
>>ringing.
>
>
> There's two cases here, IMHO. If GDB is connected, the only time we'll
> get a '$' when we're sending a packet is if we're out-of-sync (in
> regular gdb/kgdb communication) or we're prempting a console message
> (i.e. we're trying to send something while gdb wants to break in).

I am a little unclear about this. Assuming that the user has not been so dumb
as to put a breakpoint in kgdb's console handling code, I suspect that the entry
code should allow the console message to complete prior to sending the first
message to gdb. I made a rather lame attempt to do this in the -mm kgdb. What
I think should happen is that, on entry kgdb should send an nmi to all but self.
The kgdb nmi code (at in_kgdb() in the -mm patch) should check to see if the
CURRENT cpu is in the kgdb console code and, if so, set a flag for the console
code that a kgdb entry is pending and then return. The console code should
check this flag on exit, after clearing the "i am in console" flag and if set,
do a send nmi self. This would allow the console code to complete prior to the
kgdb entry while still rounding up all the other cpus with the nmi.

In
> the latter case, it's OK to give up on the console packet (it's not
> critical) and trying to send a console message during a breakpoint is
> asking for trouble.

Not only that, it is forbidden by the protocol (dam :(.

If things get out-of-sync in normal gdb/kgdb
> communication, what will happen w/o this change is that we get stuck in
> a "Packet instead of ACK" loop on gdb, and we get stuck trying to
> transmit that same packet on the kgdb side. I think that if we call
> breakpoint() here we can try and start over...

The problem is that you are now doing a breakpoint from inside kgdb while
handling a prior breakpoint. At the very least you would need to consider very
carefully what happens after the breakpoint. It is not clear that you can
recover from this condition... but you may be able to look around.
>
> The other case is what the comment describes, and in that case we quite
> quickly get to the ponit of acting like gdb is connected again.
>

--
George Anzinger george@mvista.com
High-res-timers: http://sourceforge.net/projects/high-res-timers/
Preemption patch: http://www.kernel.org/pub/linux/kernel/people/rml