<p><a href="https://review.coreboot.org/28100">View Change</a></p><p>3 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="https://review.coreboot.org/#/c/28100/1/payloads/libpayload/arch/x86/exception.c">File payloads/libpayload/arch/x86/exception.c:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/28100/1/payloads/libpayload/arch/x86/exception.c@165">Patch Set #1, Line 165:</a> <code style="font-family:monospace,monospace">       /* Handle user defined vectors */</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Not opposed to adding this capability in general, but I don't think it would be a good fit for the A […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">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.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/28100/1/payloads/libpayload/arch/x86/exception.c@167">Patch Set #1, Line 167:</a> <code style="font-family:monospace,monospace">         die_if(!hook || !hook(vec),</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">I don't think this does what you want, since even if hook(vec) returns true, you'll keep running thr […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">lol, thanks. I had it there originally, but when I went to make the commit, I forgot why I had it and removed it.</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://review.coreboot.org/#/c/28100/1/payloads/libpayload/arch/x86/exception_asm.S">File payloads/libpayload/arch/x86/exception_asm.S:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://review.coreboot.org/#/c/28100/1/payloads/libpayload/arch/x86/exception_asm.S@73">Patch Set #1, Line 73:</a> </p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><pre style="font-family: monospace,monospace; white-space: pre-wrap;">   stub    \from<br> .if     \to-\from<br></pre></blockquote></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">You might want to just review tabs/spaces in the changes</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">good catch</p></li></ul></li></ul><p>To view, visit <a href="https://review.coreboot.org/28100">change 28100</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://review.coreboot.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://review.coreboot.org/28100"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: coreboot </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: comment </div>
<div style="display:none"> Gerrit-Change-Id: Id9c2583c7c3d9be4a06a25e546e64399f2b0620c </div>
<div style="display:none"> Gerrit-Change-Number: 28100 </div>
<div style="display:none"> Gerrit-PatchSet: 2 </div>
<div style="display:none"> Gerrit-Owner: Raul Rangel <rrangel@chromium.org> </div>
<div style="display:none"> Gerrit-Reviewer: Raul Rangel <rrangel@chromium.org> </div>
<div style="display:none"> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> </div>
<div style="display:none"> Gerrit-CC: Julius Werner <jwerner@chromium.org> </div>
<div style="display:none"> Gerrit-CC: Martin Roth <martinroth@google.com> </div>
<div style="display:none"> Gerrit-Comment-Date: Thu, 16 Aug 2018 14:38:20 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-HasLabels: No </div>