From 3547d90a0f4fe476721aac2295da0db736071711 Mon Sep 17 00:00:00 2001 From: Liav A Date: Sat, 27 Mar 2021 17:11:08 +0300 Subject: [PATCH] Kernel/Storage: Select the drive before working with busmaster register This is a "quirk" I've observed on a Intel ICH7 test machine. Apparently we need to select the device (master or slave) before starting to work with the bus master register. It's very possible that other machines are requiring this step to happen before the DMA transfer can occur correctly. Also, when reading with DMA, we should set the transfer direction before clearing the interrupt status. For the sake of completeness, I added a few lines in places that I deemed it to be reasonable to clear the interrupt status there. --- Kernel/Storage/BMIDEChannel.cpp | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/Kernel/Storage/BMIDEChannel.cpp b/Kernel/Storage/BMIDEChannel.cpp index 42f6d68c4e..e6214680c1 100644 --- a/Kernel/Storage/BMIDEChannel.cpp +++ b/Kernel/Storage/BMIDEChannel.cpp @@ -44,6 +44,7 @@ UNMAP_AFTER_INIT BMIDEChannel::BMIDEChannel(const IDEController& controller, IDE UNMAP_AFTER_INIT void BMIDEChannel::initialize() { + VERIFY(m_io_group.bus_master_base().has_value()); // Let's try to set up DMA transfers. PCI::enable_bus_mastering(m_parent_controller->pci_address()); m_prdt_page = MM.allocate_supervisor_physical_page(); @@ -53,6 +54,9 @@ UNMAP_AFTER_INIT void BMIDEChannel::initialize() m_prdt_region = MM.allocate_kernel_region(m_prdt_page->paddr(), PAGE_SIZE, "IDE PRDT", Region::Access::Read | Region::Access::Write); m_dma_buffer_region = MM.allocate_kernel_region(m_dma_buffer_page->paddr(), PAGE_SIZE, "IDE DMA region", Region::Access::Read | Region::Access::Write); prdt().end_of_table = 0x8000; + + // clear bus master interrupt status + m_io_group.bus_master_base().value().offset(2).out(m_io_group.bus_master_base().value().offset(2).in() | 4); } static void print_ide_status(u8 status) @@ -81,6 +85,8 @@ void BMIDEChannel::handle_irq(const RegisterState&) dbgln_if(PATA_DEBUG, "BMIDEChannel: ignore interrupt"); return; } + // clear bus master interrupt status + m_io_group.bus_master_base().value().offset(2).out(m_io_group.bus_master_base().value().offset(2).in() | 4); ScopedSpinLock lock(m_request_lock); dbgln_if(PATA_DEBUG, "BMIDEChannel: interrupt: DRQ={}, BSY={}, DRDY={}", @@ -148,7 +154,7 @@ void BMIDEChannel::ata_write_sectors(bool slave_request, u16 capabilities) VERIFY(m_current_request->block_count() <= 256); ScopedSpinLock m_lock(m_request_lock); - dbgln_if(PATA_DEBUG, "BMIDEChannel::ata_write_sectors_with_dma ({} x {})", m_current_request->block_index(), m_current_request->block_count()); + dbgln_if(PATA_DEBUG, "BMIDEChannel::ata_write_sectors ({} x {})", m_current_request->block_index(), m_current_request->block_count()); prdt().offset = m_dma_buffer_page->paddr().get(); prdt().size = 512 * m_current_request->block_count(); @@ -158,6 +164,11 @@ void BMIDEChannel::ata_write_sectors(bool slave_request, u16 capabilities) return; } + // Note: This is a fix for a quirk for an IDE controller on ICH7 machine. + // We need to select the drive and then we wait 10 microseconds... and it doesn't hurt anything + m_io_group.io_base().offset(ATA_REG_HDDEVSEL).out(0xA0 | ((slave_request ? 1 : 0) << 4)); + IO::delay(10); + VERIFY(prdt().size <= PAGE_SIZE); VERIFY(m_io_group.bus_master_base().has_value()); // Stop bus master @@ -191,7 +202,12 @@ void BMIDEChannel::ata_read_sectors(bool slave_request, u16 capabilities) VERIFY(m_current_request->block_count() <= 256); ScopedSpinLock m_lock(m_request_lock); - dbgln_if(PATA_DEBUG, "BMIDEChannel::ata_read_sectors_with_dma ({} x {})", m_current_request->block_index(), m_current_request->block_count()); + dbgln_if(PATA_DEBUG, "BMIDEChannel::ata_read_sectors ({} x {})", m_current_request->block_index(), m_current_request->block_count()); + + // Note: This is a fix for a quirk for an IDE controller on ICH7 machine. + // We need to select the drive and then we wait 10 microseconds... and it doesn't hurt anything + m_io_group.io_base().offset(ATA_REG_HDDEVSEL).out(0xA0 | ((slave_request ? 1 : 0) << 4)); + IO::delay(10); prdt().offset = m_dma_buffer_page->paddr().get(); prdt().size = 512 * m_current_request->block_count(); @@ -205,12 +221,12 @@ void BMIDEChannel::ata_read_sectors(bool slave_request, u16 capabilities) // Write the PRDT location m_io_group.bus_master_base().value().offset(4).out(m_prdt_page->paddr().get()); - // Turn on "Interrupt" and "Error" flag. The error flag should be cleared by hardware. - m_io_group.bus_master_base().value().offset(2).out(m_io_group.bus_master_base().value().offset(2).in() | 0x6); - // Set transfer direction m_io_group.bus_master_base().value().out(0x8); + // Turn on "Interrupt" and "Error" flag. The error flag should be cleared by hardware. + m_io_group.bus_master_base().value().offset(2).out(m_io_group.bus_master_base().value().offset(2).in() | 0x6); + ata_access(Direction::Read, slave_request, m_current_request->block_index(), m_current_request->block_count(), capabilities); // Start bus master