Raul Rangel has posted comments on this change. ( https://review.coreboot.org/28100 )
Change subject: libpayload/x86/exception: Add ability to handle user defined interrupts ......................................................................
Patch Set 2:
(3 comments)
https://review.coreboot.org/#/c/28100/1/payloads/libpayload/arch/x86/excepti... File payloads/libpayload/arch/x86/exception.c:
https://review.coreboot.org/#/c/28100/1/payloads/libpayload/arch/x86/excepti... PS1, Line 165: /* Handle user defined vectors */
Not opposed to adding this capability in general, but I don't think it would be a good fit for the A […]
Ah, I haven't tested with a gdb build yet, so I haven't seen it break. In the spirit of separation of concerns, I would like to avoid hard coding the interrupt number here. Instead of having a linked list for exception_install_hook. What if we kept a lookup table of handlers and added anew method set_interrupt_handler(void (*handler)(u32 vector)). This way we don't need to iterate over all the hooks checking the return status and we don't get into cases where we have two hooks that handle the same interrupt.
https://review.coreboot.org/#/c/28100/1/payloads/libpayload/arch/x86/excepti... PS1, Line 167: die_if(!hook || !hook(vec),
I don't think this does what you want, since even if hook(vec) returns true, you'll keep running thr […]
lol, thanks. I had it there originally, but when I went to make the commit, I forgot why I had it and removed it.
https://review.coreboot.org/#/c/28100/1/payloads/libpayload/arch/x86/excepti... File payloads/libpayload/arch/x86/exception_asm.S:
https://review.coreboot.org/#/c/28100/1/payloads/libpayload/arch/x86/excepti... PS1, Line 73: stub \from : .if \to-\from
You might want to just review tabs/spaces in the changes
good catch