Re: [RFC PATCH 6/7] x86/kexec: Debugging support: Dump registers on exception

[H. Peter Anvin]

On November 5, 2024 6:43:44 PM PST, David Woodhouse <dwmw2 at infradead.org> wrote:
>On Tue, 2024-11-05 at 13:37 -0800, H. Peter Anvin wrote:
>> 
>> Looking at your code, you have a much bigger problem here:
>> 
>> +/*
>> + * This allows other types of serial ports to be used.
>> + *  - %al: Character to be printed (no clobber %rax)
>> + *  - %rdx: MMIO address or port.
>> + */
>> +.macro pr_char
>> +       outb    %al, %dx
>> +.endm
>> +
>> 
>> This will overflow your UART buffer very quickly since you are now 
>> dumping a whole bunch of data. The URT buffer -- if you even have one
>> and it is enabled -- is only 16 bytes in a standard 16550A UART. In 
>> older UARTs (or emulated older UARTs) you might not have a buffer at 
>> all. To print more than a handful of bytes, you need to poll for the 
>> THRE bit=1 (bit 5 of register 5).
>
>Emulated UARTs are generally fine because they don't really emulate the
>buffer at all. And when I originally wrote this it was purely a hack to
>debug an issue for myself, and used a different type of logging device
>altogether.
>
>But yeah, if this were to be used on bare metal 16550A it would indeed
>need to wait for space in the FIFO/THR.
>
>> What is the point of writing this code in assembly in the first place? A 
>> much more logical thing to do is to just push the registers you haven't 
>> pushed already onto the stack and call a C function to do the actual 
>> dumping? It isn't like it is in any shape, way or form performance critical.
>
>If we fix it up to use a proper linker script, that's slightly more
>feasible. As things stand, it's only really possible to do it in the
>existing asm file. 
>
>And it's only the core of the exception handler "function" which could
>be moved out to C; it didn't seem particularly worth bothering. Would
>be nice to have the IDT generated from C code *before* calling
>relocate_kernel() instead of inside relocate_kernel itself, perhaps,
>but I was also trying to keep the #define DEBUG version of the code
>fairly self-contained.
>
>

Yes, the linker script needs to happen. 

This is a case of doing it right vs doing it quickly.