[coreboot-gerrit] Change in coreboot[master]: libpayload/x86/exception: Add ability to handle user defined interrupts

[Raul Rangel (Code Review)]

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/exception.c
File payloads/libpayload/arch/x86/exception.c:

https://review.coreboot.org/#/c/28100/1/payloads/libpayload/arch/x86/exception.c@165
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/exception.c@167
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/exception_asm.S
File payloads/libpayload/arch/x86/exception_asm.S:

https://review.coreboot.org/#/c/28100/1/payloads/libpayload/arch/x86/exception_asm.S@73
PS1, Line 73: 	stub	\from
            : 	.if	\to-\from
> You might want to just review tabs/spaces in the changes
good catch



-- 
To view, visit https://review.coreboot.org/28100
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id9c2583c7c3d9be4a06a25e546e64399f2b0620c
Gerrit-Change-Number: 28100
Gerrit-PatchSet: 2
Gerrit-Owner: Raul Rangel <rrangel at chromium.org>
Gerrit-Reviewer: Raul Rangel <rrangel at chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply at coreboot.org>
Gerrit-CC: Julius Werner <jwerner at chromium.org>
Gerrit-CC: Martin Roth <martinroth at google.com>
Gerrit-Comment-Date: Thu, 16 Aug 2018 14:38:20 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: No
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.coreboot.org/pipermail/coreboot-gerrit/attachments/20180816/dc96f843/attachment.html>