Signed-off-by: Nikolay Nikolov nickysn@users.sourceforge.net --- src/hw/floppy.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/src/hw/floppy.c b/src/hw/floppy.c index 9e6647d..1120d63 100644 --- a/src/hw/floppy.c +++ b/src/hw/floppy.c @@ -220,11 +220,18 @@ floppy_disable_controller(void) floppy_dor_mask(FLOPPY_DOR_IRQ | FLOPPY_DOR_RESET, 0); }
-static int -floppy_wait_irq(void) +static inline void +floppy_clear_irq_flag(void) { u8 frs = GET_BDA(floppy_recalibration_status); SET_BDA(floppy_recalibration_status, frs & ~FRS_IRQ); +} + +static int +floppy_wait_irq(void) +{ + floppy_clear_irq_flag(); + u8 frs; u32 end = timer_calc(FLOPPY_IRQ_TIMEOUT); for (;;) { if (timer_check(end)) {
This ensures that we safely receive the IRQ even if the floppy controller manages to be faster than our code and sends the IRQ before our code enters floppy_wait_irq(). Although unlikely, this is possible to happen on emulators (which emulate floppy operations extremely fast) as well as real hardware (during reset).
Signed-off-by: Nikolay Nikolov nickysn@users.sourceforge.net --- src/hw/floppy.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/hw/floppy.c b/src/hw/floppy.c index 1120d63..c85615b 100644 --- a/src/hw/floppy.c +++ b/src/hw/floppy.c @@ -230,7 +230,6 @@ floppy_clear_irq_flag(void) static int floppy_wait_irq(void) { - floppy_clear_irq_flag(); u8 frs; u32 end = timer_calc(FLOPPY_IRQ_TIMEOUT); for (;;) { @@ -267,6 +266,9 @@ static int floppy_pio(int command, u8 *param) { dprintf(9, "Floppy pio command %x\n", command); + // Clear the IRQ flag before issuing the command. + if (command & FCF_WAITIRQ) + floppy_clear_irq_flag(); // Send command and parameters to controller. u32 end = timer_calc(FLOPPY_PIO_TIMEOUT); int send = (command >> 8) & 0xf; @@ -341,6 +343,8 @@ floppy_enable_controller(void) floppy_dor_mask(FLOPPY_DOR_RESET, FLOPPY_DOR_IRQ); // Real hardware needs a 4 microsecond delay usleep(4); + // Clear the IRQ flag right before leaving the reset state. + floppy_clear_irq_flag(); // Set the reset bit (normal operation) and keep 'enable IRQ and DMA' on floppy_dor_mask(0, FLOPPY_DOR_IRQ | FLOPPY_DOR_RESET); int ret = floppy_wait_irq();
On Sun, Feb 25, 2018 at 10:38:09PM +0200, Nikolay Nikolov wrote:
This ensures that we safely receive the IRQ even if the floppy controller manages to be faster than our code and sends the IRQ before our code enters floppy_wait_irq(). Although unlikely, this is possible to happen on emulators (which emulate floppy operations extremely fast) as well as real hardware (during reset).
This doesn't look correct to me. The SeaBIOS C code always runs with interrupts disabled. The FRS_IRQ flag is only set from the handle_0e() hardware interrupt handler. So, I don't think there is a race condition with the clearing and checking of that flag.
-Kevin
On Sun, 2018-02-25 at 18:33 -0500, Kevin O'Connor wrote:
On Sun, Feb 25, 2018 at 10:38:09PM +0200, Nikolay Nikolov wrote:
This ensures that we safely receive the IRQ even if the floppy controller manages to be faster than our code and sends the IRQ before our code enters floppy_wait_irq(). Although unlikely, this is possible to happen on emulators (which emulate floppy operations extremely fast) as well as real hardware (during reset).
This doesn't look correct to me. The SeaBIOS C code always runs with interrupts disabled. The FRS_IRQ flag is only set from the handle_0e() hardware interrupt handler. So, I don't think there is a race condition with the clearing and checking of that flag.
Sorry, I didn't know that the SeaBIOS C code runs with interrupts disabled. I assumed they were enabled. Please disregard this patch.
Nikolay
On Mon, Feb 26, 2018 at 11:05:29PM +0200, Nikolay Nikolov wrote:
On Sun, 2018-02-25 at 18:33 -0500, Kevin O'Connor wrote:
On Sun, Feb 25, 2018 at 10:38:09PM +0200, Nikolay Nikolov wrote:
This ensures that we safely receive the IRQ even if the floppy controller manages to be faster than our code and sends the IRQ before our code enters floppy_wait_irq(). Although unlikely, this is possible to happen on emulators (which emulate floppy operations extremely fast) as well as real hardware (during reset).
This doesn't look correct to me. The SeaBIOS C code always runs with interrupts disabled. The FRS_IRQ flag is only set from the handle_0e() hardware interrupt handler. So, I don't think there is a race condition with the clearing and checking of that flag.
Sorry, I didn't know that the SeaBIOS C code runs with interrupts disabled. I assumed they were enabled. Please disregard this patch.
No problem. Be aware that yield() or anything that calls yield() (such as sleep() ) will allow irqs to occur. There's some documentation on this in docs/Execution_and_code_flow.md.
-Kevin
-
Kevin O'Connor -
Nikolay Nikolov -
Nikolay Nikolov