From f5090ab8105c36c28addd30aaa9c387cb35d12e4 Mon Sep 17 00:00:00 2001 From: Liav A Date: Tue, 14 Apr 2020 22:48:02 +0300 Subject: [PATCH] Kernel: Restore ATA PIO functionality First, before this change, specifying 'force_pio' in the kernel commandline was meaningless because we nevertheless set the DMA flag to be enabled. Also, we had a problem in which we used IO::repeated_out16() in PIO write method. This might work on buggy emulators, but I suspect that on real hardware this code will fail. The most difficult problem was to restore the PIO read operation. Apparently, it seems that we can't use IO::repeated_in16() here because it will read zeroed data. Currently we rely on a simple loop that invokes IO::in16() to a buffer. Also, the interrupt handling stage in the PIO read method is moved to be handled inside the loop of reading the requested sectors. --- Kernel/Devices/PATAChannel.cpp | 82 +++++++++++++++++++--------------- 1 file changed, 47 insertions(+), 35 deletions(-) diff --git a/Kernel/Devices/PATAChannel.cpp b/Kernel/Devices/PATAChannel.cpp index 982ba85b5f..a00fec9211 100644 --- a/Kernel/Devices/PATAChannel.cpp +++ b/Kernel/Devices/PATAChannel.cpp @@ -136,10 +136,9 @@ PATAChannel::PATAChannel(PCI::Address address, ChannelType type, bool force_pio) { disable_irq(); - m_dma_enabled.resource() = true; + m_dma_enabled.resource() = !force_pio; ProcFS::add_sys_bool("ide_dma", m_dma_enabled); - m_prdt_page = MM.allocate_supervisor_physical_page(); initialize(force_pio); detect_disks(); disable_irq(); @@ -157,14 +156,15 @@ void PATAChannel::prepare_for_irq() void PATAChannel::initialize(bool force_pio) { - + PCI::enable_interrupt_line(pci_address()); if (force_pio) { klog() << "PATAChannel: Requested to force PIO mode; not setting up DMA"; return; } + // Let's try to set up DMA transfers. PCI::enable_bus_mastering(pci_address()); - PCI::enable_interrupt_line(pci_address()); + m_prdt_page = MM.allocate_supervisor_physical_page(); prdt().end_of_table = 0x8000; m_dma_buffer_page = MM.allocate_supervisor_physical_page(); klog() << "PATAChannel: Bus master IDE: " << m_bus_master_base; @@ -402,57 +402,66 @@ bool PATAChannel::ata_write_sectors_with_dma(u32 lba, u16 count, const u8* inbuf return true; } -bool PATAChannel::ata_read_sectors(u32 start_sector, u16 count, u8* outbuf, bool slave_request) +bool PATAChannel::ata_read_sectors(u32 lba, u16 count, u8* outbuf, bool slave_request) { ASSERT(count <= 256); LOCKER(s_lock()); #ifdef PATA_DEBUG - dbg() << "PATAChannel::ata_read_sectors request (" << count << " sector(s) @ " << start_sector << " into " << outbuf << ")"; + dbg() << "PATAChannel::ata_read_sectors request (" << count << " sector(s) @ " << lba << " into " << outbuf << ")"; #endif while (m_io_base.offset(ATA_REG_STATUS).in() & ATA_SR_BSY) ; #ifdef PATA_DEBUG - klog() << "PATAChannel: Reading " << count << " sector(s) @ LBA " << start_sector; + klog() << "PATAChannel: Reading " << count << " sector(s) @ LBA " << lba; #endif u8 devsel = 0xe0; if (slave_request) devsel |= 0x10; - m_io_base.offset(ATA_REG_SECCOUNT0).out(count == 256 ? 0 : LSB(count)); - m_io_base.offset(ATA_REG_LBA0).out(start_sector & 0xff); - m_io_base.offset(ATA_REG_LBA1).out((start_sector >> 8) & 0xff); - m_io_base.offset(ATA_REG_LBA2).out((start_sector >> 16) & 0xff); - m_io_base.offset(ATA_REG_HDDEVSEL).out(devsel | ((start_sector >> 24) & 0xf)); + m_control_base.offset(ATA_CTL_CONTROL).out(0); + m_io_base.offset(ATA_REG_HDDEVSEL).out(devsel | (static_cast(slave_request) << 4) | 0x40); + io_delay(); - IO::out8(0x3F6, 0x08); - while (!(m_io_base.offset(ATA_REG_STATUS).in() & ATA_SR_DRDY)) - ; + m_io_base.offset(ATA_REG_FEATURES).out(0); - prepare_for_irq(); - m_io_base.offset(ATA_REG_COMMAND).out(ATA_CMD_READ_PIO); - wait_for_irq(); + m_io_base.offset(ATA_REG_SECCOUNT0).out(0); + m_io_base.offset(ATA_REG_LBA0).out(0); + m_io_base.offset(ATA_REG_LBA1).out(0); + m_io_base.offset(ATA_REG_LBA2).out(0); - if (m_device_error) - return false; + m_io_base.offset(ATA_REG_SECCOUNT0).out(count); + m_io_base.offset(ATA_REG_LBA0).out((lba & 0x000000ff) >> 0); + m_io_base.offset(ATA_REG_LBA1).out((lba & 0x0000ff00) >> 8); + m_io_base.offset(ATA_REG_LBA2).out((lba & 0x00ff0000) >> 16); - for (int i = 0; i < count; i++) { - io_delay(); - - while (m_io_base.offset(ATA_REG_STATUS).in() & ATA_SR_BSY) - ; - - u8 status = m_io_base.offset(ATA_REG_STATUS).in(); - ASSERT(status & ATA_SR_DRQ); -#ifdef PATA_DEBUG - dbg() << "PATAChannel: Retrieving 512 bytes (part " << i << ") (status=" << String::format("%b", status) << "), outbuf=(" << (outbuf + (512 * i)) << ")..."; -#endif - - IO::repeated_in16(m_io_base.offset(ATA_REG_DATA).get(), outbuf + (512 * i), 256); + for (;;) { + auto status = m_io_base.offset(ATA_REG_STATUS).in(); + if (!(status & ATA_SR_BSY) && (status & ATA_SR_DRDY)) + break; } + m_io_base.offset(ATA_REG_COMMAND).out(ATA_CMD_READ_PIO); + + for (int i = 0; i < count; i++) { + prepare_for_irq(); + wait_for_irq(); + if (m_device_error) + return false; + + u8 status = m_control_base.offset(ATA_CTL_ALTSTATUS).in(); + ASSERT(!(status & ATA_SR_BSY)); + + auto* buffer = (u16*)(outbuf + i * 512); +#ifdef PATA_DEBUG + dbg() << "PATAChannel: Retrieving 512 bytes (part " << i << ") (status=" << String::format("%b", status) << "), outbuf=(" << buffer << ")..."; +#endif + for (int i = 0; i < 256; i++) { + buffer[i] = IO::in16(m_io_base.offset(ATA_REG_DATA).get()); + } + } return true; } @@ -489,7 +498,7 @@ bool PATAChannel::ata_write_sectors(u32 start_sector, u16 count, const u8* inbuf for (int i = 0; i < count; i++) { io_delay(); - while (m_io_base.offset(ATA_REG_STATUS).in() & ATA_SR_BSY) + while ((m_io_base.offset(ATA_REG_STATUS).in() & ATA_SR_BSY) || !(m_io_base.offset(ATA_REG_STATUS).in() & ATA_SR_DRQ)) ; u8 status = m_io_base.offset(ATA_REG_STATUS).in(); @@ -499,7 +508,10 @@ bool PATAChannel::ata_write_sectors(u32 start_sector, u16 count, const u8* inbuf dbg() << "PATAChannel: Writing 512 bytes (part " << i << ") (status=" << String::format("%b", status) << "), inbuf=(" << (inbuf + (512 * i)) << ")..."; #endif - IO::repeated_out16(m_io_base.offset(ATA_REG_DATA).get(), inbuf + (512 * i), 256); + auto* buffer = (u16*)(const_cast(inbuf) + i * 512); + for (int i = 0; i < 256; i++) { + IO::out16(m_io_base.offset(ATA_REG_DATA).get(), buffer[i]); + } prepare_for_irq(); wait_for_irq(); status = m_io_base.offset(ATA_REG_STATUS).in();