Michael Brown
2017-Mar-19 12:23 UTC
[syslinux] [PATCH] pxe: Never chain to the original ISR
The behaviour of default ISRs as provided by the BIOS varies wildly
between platforms. Some will simply iret, some will send EOI, some
will send EOI and disable the interrupt at the PIC, some will crash
the machine due to single-bit errors in the ISR address.
When PXENV_UNDI_ISR_IN_START returns PXENV_UNDI_ISR_OUT_NOT_OURS, send
the EOI ourselves rather than risking the unpredictable behaviour of
chaining to the original ISR.
Signed-off-by: Michael Brown <mcb30 at ipxe.org>
---
core/fs/pxe/isr.c | 5 +++--
core/fs/pxe/pxe.h | 1 -
core/pxeisr.inc | 17 ++++++-----------
3 files changed, 9 insertions(+), 14 deletions(-)
diff --git a/core/fs/pxe/isr.c b/core/fs/pxe/isr.c
index 7da0cc7..e14b953 100644
--- a/core/fs/pxe/isr.c
+++ b/core/fs/pxe/isr.c
@@ -17,6 +17,7 @@ extern volatile uint8_t pxe_need_poll;
static DECLARE_INIT_SEMAPHORE(pxe_receive_thread_sem, 0);
static DECLARE_INIT_SEMAPHORE(pxe_poll_thread_sem, 0);
static struct thread *pxe_thread, *poll_thread;
+static far_ptr_t old_pxe_isr;
#ifndef PXE_POLL_FORCE
# define PXE_POLL_FORCE 0
@@ -252,7 +253,7 @@ void pxe_start_isr(void)
pxe_irq_vector = irq;
if (irq) {
- if (!install_irq_vector(irq, pxe_isr, &pxe_irq_chain))
+ if (!install_irq_vector(irq, pxe_isr, &old_pxe_isr))
irq = 0; /* Install failed or stuck interrupt */
}
@@ -290,7 +291,7 @@ int reset_pxe(void)
printf("PXENV_UNDI_CLOSE failed: 0x%x\n", undi_close.Status);
if (pxe_irq_vector)
- uninstall_irq_vector(pxe_irq_vector, pxe_isr, &pxe_irq_chain);
+ uninstall_irq_vector(pxe_irq_vector, pxe_isr, &old_pxe_isr);
if (poll_thread)
kill_thread(poll_thread);
diff --git a/core/fs/pxe/pxe.h b/core/fs/pxe/pxe.h
index 19664f9..449391b 100644
--- a/core/fs/pxe/pxe.h
+++ b/core/fs/pxe/pxe.h
@@ -212,7 +212,6 @@ static inline uint32_t gateway(uint32_t ip)
/* pxeisr.inc */
extern uint8_t pxe_irq_vector;
extern void pxe_isr(void);
-extern far_ptr_t pxe_irq_chain;
extern void pxe_poll(void);
/* isr.c */
diff --git a/core/pxeisr.inc b/core/pxeisr.inc
index 93c73ed..36e83f2 100644
--- a/core/pxeisr.inc
+++ b/core/pxeisr.inc
@@ -40,7 +40,7 @@ pxe_isr:
; leftover BC doesn't get control.
mov byte [pxe_irq_pending],1
inc dword [pxe_irq_count]
-
+.eoi:
cmp byte [pxe_irq_vector], 8
mov al,0x20 ; Non-specific EOI
jb .pri_pic
@@ -66,20 +66,15 @@ pxe_isr:
dec word [pxeirq_deadman]
jz .timeout
-.chain:
- pop gs
- pop fs
- pop es
- pop ds
- popa
- jmp 0:0
- global pxe_irq_chain
-pxe_irq_chain equ $-4
+ ; Even though it isn't ours, we still need to send the
+ ; EOI ourselves, since chaining to the original
+ ; handler is too risky.
+ jmp .eoi
.reset_timeout:
mov [pxeirq_last],ax
mov word [pxeirq_deadman],PXEIRQ_MAX
- jmp .chain
+ jmp .eoi
; Too many spurious interrupts, shut off the interrupts
; and go to polling mode
--
2.9.3
Retransmit On Sun, Mar 19, 2017 at 12:23:20PM +0000, Michael Brown via Syslinux wrote:> The behaviour of default ISRs as provided by the BIOS varies wildly > between platforms. Some will simply iret, some will send EOI, some > will send EOI and disable the interrupt at the PIC, some will crash > the machine due to single-bit errors in the ISR address. > > When PXENV_UNDI_ISR_IN_START returns PXENV_UNDI_ISR_OUT_NOT_OURS, send > the EOI ourselves rather than risking the unpredictable behaviour of > chaining to the original ISR. > > Signed-off-by: Michael Brown <mcb30 at ipxe.org> > --- > core/fs/pxe/isr.c | 5 +++-- > core/fs/pxe/pxe.h | 1 - > core/pxeisr.inc | 17 ++++++----------- > 3 files changed, 9 insertions(+), 14 deletions(-) > > diff --git a/core/fs/pxe/isr.c b/core/fs/pxe/isr.c > index 7da0cc7..e14b953 100644 > --- a/core/fs/pxe/isr.c > +++ b/core/fs/pxe/isr.c > @@ -17,6 +17,7 @@ extern volatile uint8_t pxe_need_poll; > static DECLARE_INIT_SEMAPHORE(pxe_receive_thread_sem, 0); > static DECLARE_INIT_SEMAPHORE(pxe_poll_thread_sem, 0); > static struct thread *pxe_thread, *poll_thread; > +static far_ptr_t old_pxe_isr; > > #ifndef PXE_POLL_FORCE > # define PXE_POLL_FORCE 0 > @@ -252,7 +253,7 @@ void pxe_start_isr(void) > pxe_irq_vector = irq; > > if (irq) { > - if (!install_irq_vector(irq, pxe_isr, &pxe_irq_chain)) > + if (!install_irq_vector(irq, pxe_isr, &old_pxe_isr)) > irq = 0; /* Install failed or stuck interrupt */ > } > > @@ -290,7 +291,7 @@ int reset_pxe(void) > printf("PXENV_UNDI_CLOSE failed: 0x%x\n", undi_close.Status); > > if (pxe_irq_vector) > - uninstall_irq_vector(pxe_irq_vector, pxe_isr, &pxe_irq_chain); > + uninstall_irq_vector(pxe_irq_vector, pxe_isr, &old_pxe_isr); > if (poll_thread) > kill_thread(poll_thread); > > diff --git a/core/fs/pxe/pxe.h b/core/fs/pxe/pxe.h > index 19664f9..449391b 100644 > --- a/core/fs/pxe/pxe.h > +++ b/core/fs/pxe/pxe.h > @@ -212,7 +212,6 @@ static inline uint32_t gateway(uint32_t ip) > /* pxeisr.inc */ > extern uint8_t pxe_irq_vector; > extern void pxe_isr(void); > -extern far_ptr_t pxe_irq_chain; > extern void pxe_poll(void); > > /* isr.c */ > diff --git a/core/pxeisr.inc b/core/pxeisr.inc > index 93c73ed..36e83f2 100644 > --- a/core/pxeisr.inc > +++ b/core/pxeisr.inc > @@ -40,7 +40,7 @@ pxe_isr: > ; leftover BC doesn't get control. > mov byte [pxe_irq_pending],1 > inc dword [pxe_irq_count] > - > +.eoi: > cmp byte [pxe_irq_vector], 8 > mov al,0x20 ; Non-specific EOI > jb .pri_pic > @@ -66,20 +66,15 @@ pxe_isr: > dec word [pxeirq_deadman] > jz .timeout > > -.chain: > - pop gs > - pop fs > - pop es > - pop ds > - popa > - jmp 0:0 > - global pxe_irq_chain > -pxe_irq_chain equ $-4 > + ; Even though it isn't ours, we still need to send the > + ; EOI ourselves, since chaining to the original > + ; handler is too risky. > + jmp .eoi > > .reset_timeout: > mov [pxeirq_last],ax > mov word [pxeirq_deadman],PXEIRQ_MAX > - jmp .chain > + jmp .eoi > > ; Too many spurious interrupts, shut off the interrupts > ; and go to polling mode > -- > 2.9.3 > > _______________________________________________ > Syslinux mailing list > Submissions to Syslinux at zytor.com > Unsubscribe or set options at: > http://www.zytor.com/mailman/listinfo/syslinux