From 072bf8cbb9f573288b1f001c4e95357018b50a51 Mon Sep 17 00:00:00 2001 From: Conrad Pankoff Date: Mon, 12 Aug 2019 21:05:14 +1000 Subject: [PATCH] Kernel: Fix non-DMA writes to IDE drives Our logic for using the ATA_CMD_CACHE_FLUSH functionality was a bit wrong, and now it's better. The ATA spec says these two things: > The device shall enter the interrupt pending state when: > 1) any command except a PIO data-in command reaches command completion > successfully; > ... > The device shall exit the interrupt pending state when: > 1) the device is selected, BSY is cleared to zero, and the Status > register is read; This means that our sequence of actions was probably never going to work. We were waiting in a loop checking the status register until it left the busy state, _then_ waiting for an interrupt. Unfortunately by checking the status register, we were _clearing_ the interrupt we were about to wait for. Now we just wait for the interrupt - we don't poll the status register at all. This also means that once we get our `wait_for_irq` method sorted out we'll spend a bunch less CPU time waiting for things to complete. --- Kernel/Devices/PATAChannel.cpp | 32 ++++++++++++++++++++++---------- 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/Kernel/Devices/PATAChannel.cpp b/Kernel/Devices/PATAChannel.cpp index d429254133f..60e0f469cce 100644 --- a/Kernel/Devices/PATAChannel.cpp +++ b/Kernel/Devices/PATAChannel.cpp @@ -494,27 +494,39 @@ bool PATAChannel::ata_write_sectors(u32 start_sector, u16 count, const u8* inbuf IO::out8(m_io_base + ATA_REG_HDDEVSEL, devsel | ((start_sector >> 24) & 0xf)); IO::out8(0x3F6, 0x08); + while (!(IO::in8(m_io_base + ATA_REG_STATUS) & ATA_SR_DRDY)) + ; IO::out8(m_io_base + ATA_REG_COMMAND, ATA_CMD_WRITE_PIO); - while (!(IO::in8(m_io_base + ATA_REG_STATUS) & ATA_SR_DRQ)) - ; + for (int i = 0; i < count; i++) { + wait_400ns(m_io_base); + while (IO::in8(m_io_base + ATA_REG_STATUS) & ATA_SR_BSY) + ; - u8 status = IO::in8(m_io_base + ATA_REG_STATUS); - ASSERT(status & ATA_SR_DRQ); - IO::repeated_out16(m_io_base + ATA_REG_DATA, inbuf, count * 256); + u8 status = IO::in8(m_io_base + ATA_REG_STATUS); + ASSERT(status & ATA_SR_DRQ); - m_interrupted = false; - enable_irq(); - wait_for_irq(); +#ifdef PATA_DEBUG + kprintf("PATAChannel: Writing 512 bytes (part %d) (status=%b), inbuf=%p...\n", i, status, inbuf + (512 * i)); +#endif + + disable_irq(); + IO::repeated_out16(m_io_base + ATA_REG_DATA, inbuf + (512 * i), 256); + m_interrupted = false; + enable_irq(); + wait_for_irq(); + status = IO::in8(m_io_base + ATA_REG_STATUS); + ASSERT(!(status & ATA_SR_BSY)); + } disable_irq(); IO::out8(m_io_base + ATA_REG_COMMAND, ATA_CMD_CACHE_FLUSH); - while (IO::in8(m_io_base + ATA_REG_STATUS) & ATA_SR_BSY) - ; m_interrupted = false; enable_irq(); wait_for_irq(); + u8 status = IO::in8(m_io_base + ATA_REG_STATUS); + ASSERT(!(status & ATA_SR_BSY)); return !m_device_error; }