From 5f7ec50055c25aebae43b8dd80d2448c74e977b8 Mon Sep 17 00:00:00 2001 From: "Justin C. Miller" Date: Tue, 15 May 2018 21:37:27 -0700 Subject: [PATCH] Add fixes I made while looking for VBox bug --- src/kernel/ahci/hba.cpp | 26 +++++++++++++++++++++++--- src/kernel/ahci/hba.h | 3 +++ src/kernel/ahci/port.cpp | 39 +++++++++++++++++++++++++++++++++++---- src/kernel/ahci/port.h | 14 ++++++++++---- src/kernel/pci.cpp | 37 ++++++++++++++++++++++++++++++------- 5 files changed, 101 insertions(+), 18 deletions(-) diff --git a/src/kernel/ahci/hba.cpp b/src/kernel/ahci/hba.cpp index 8d725b9..9182137 100644 --- a/src/kernel/ahci/hba.cpp +++ b/src/kernel/ahci/hba.cpp @@ -1,6 +1,7 @@ #include #include "ahci/ata.h" #include "ahci/hba.h" +#include "console.h" #include "device_manager.h" #include "log.h" #include "page_manager.h" @@ -81,7 +82,7 @@ hba::hba(pci_device *device) m_ports.ensure_capacity(ports); for (unsigned i = 0; i < ports; ++i) { bool impl = ((m_data->port_impl & (1 << i)) != 0); - port &p = m_ports.emplace(i, kutil::offset_pointer(pd, 0x80 * i), impl); + port &p = m_ports.emplace(this, i, kutil::offset_pointer(pd, 0x80 * i), impl); if (p.get_state() == port::state::active) needs_interrupt = true; @@ -98,12 +99,31 @@ hba::hba(pci_device *device) void hba::handle_interrupt() { + uint32_t status = m_data->int_status; for (auto &port : m_ports) { - if (m_data->int_status & (1 << port.index())) { + if (status & (1 << port.index())) { port.handle_interrupt(); } } - m_data->int_status = 0; + // Write 1 to the handled interrupts + m_data->int_status = status; +} + +void +hba::dump() +{ + console *cons = console::get(); + static const char *regs[] = { + " CAP", " GHC", " IS", " PI", " VS", " C3C", + " C3P", " EML", " EMC", "CAP2", "BOHC" + }; + + cons->printf("HBA Registers:\n"); + uint32_t *data = reinterpret_cast(m_data); + for (int i = 0; i < 11; ++i) { + cons->printf(" %s: %08x\n", regs[i], data[i]); + } + cons->putc('\n'); } } // namespace ahci diff --git a/src/kernel/ahci/hba.h b/src/kernel/ahci/hba.h index 6bdbe02..2e95043 100644 --- a/src/kernel/ahci/hba.h +++ b/src/kernel/ahci/hba.h @@ -25,6 +25,9 @@ public: /// Interrupt handler. void handle_interrupt(); + /// Dump the HBA registers to the console + void dump(); + private: pci_device *m_device; hba_data *m_data; diff --git a/src/kernel/ahci/port.cpp b/src/kernel/ahci/port.cpp index efa90ea..f81030c 100644 --- a/src/kernel/ahci/port.cpp +++ b/src/kernel/ahci/port.cpp @@ -2,8 +2,10 @@ #include "kutil/assert.h" #include "kutil/enum_bitfields.h" #include "ahci/ata.h" +#include "ahci/hba.h" #include "ahci/fis.h" #include "ahci/port.h" +#include "console.h" #include "io.h" #include "log.h" #include "page_manager.h" @@ -110,10 +112,11 @@ struct port_data } __attribute__ ((packed)); -port::port(uint8_t index, port_data *data, bool impl) : +port::port(hba *device, uint8_t index, port_data *data, bool impl) : m_index(index), m_type(sata_signature::none), m_state(state::unimpl), + m_hba(device), m_data(data), m_fis(nullptr), m_cmd_list(nullptr), @@ -158,7 +161,9 @@ port::update() pend.type = command_type::none; } - m_data->interrupt_enable = 1; + // Clear any old pending interrupts and enable interrupts + m_data->interrupt_status = m_data->interrupt_status; + m_data->interrupt_enable = 0xffffffff; } else { m_state = state::inactive; } @@ -197,7 +202,11 @@ int port::make_command(size_t length) { int slot = -1; - uint32_t used_slots = (m_data->serial_active | m_data->cmd_issue); + uint32_t used_slots = + m_data->serial_active | + m_data->cmd_issue | + m_data->interrupt_status; + for (int i = 0; i < 32; ++i) { if (used_slots & (1 << i)) continue; @@ -292,8 +301,12 @@ size_t port::read(uint64_t offset, size_t length, void *dest) { int slot = read_async(offset, length, dest); - while (m_pending[slot].type == command_type::read) + + int timeout = 0; + while (m_pending[slot].type == command_type::read) { + if (timeout++ > 10) return 0; asm("hlt"); + } kassert(m_pending[slot].type == command_type::finished, "Read got unexpected command type"); @@ -465,4 +478,22 @@ port::rebase() start_commands(); } +void +port::dump() +{ + console *cons = console::get(); + static const char *regs[] = { + " CLB", "+CLB", " FB", " +FB", " IS", " IE", + " CMD", nullptr, " TFD", " SIG", "SSTS", "SCTL", "SERR", + "SACT", " CI", "SNTF", " FBS", "DEVS" + }; + + cons->printf("Port Registers:\n"); + uint32_t *data = reinterpret_cast(m_data); + for (int i = 0; i < 18; ++i) { + if (regs[i]) cons->printf(" %s: %08x\n", regs[i], data[i]); + } + cons->putc('\n'); +} + } // namespace ahci diff --git a/src/kernel/ahci/port.h b/src/kernel/ahci/port.h index 70841fb..a645941 100644 --- a/src/kernel/ahci/port.h +++ b/src/kernel/ahci/port.h @@ -10,6 +10,7 @@ namespace ahci { struct cmd_list_entry; struct cmd_table; +class hba; enum class sata_signature : uint32_t; enum class port_cmd : uint32_t; struct port_data; @@ -21,10 +22,11 @@ class port : { public: /// Constructor. - /// \arg index Index of the port on its HBA - /// \arg data Pointer to the device's registers for this port - /// \arg impl Whether this port is marked as implemented in the HBA - port(uint8_t index, port_data *data, bool impl); + /// \arg device The HBA device this port belongs to + /// \arg index Index of the port on its HBA + /// \arg data Pointer to the device's registers for this port + /// \arg impl Whether this port is marked as implemented in the HBA + port(hba *device, uint8_t index, port_data *data, bool impl); /// Destructor ~port(); @@ -72,6 +74,9 @@ public: /// Handle an incoming interrupt void handle_interrupt(); + /// Dump the port registers to the console + void dump(); + private: /// Rebase the port command structures to a new location in system /// memory, to be allocated from the page manager. @@ -101,6 +106,7 @@ private: uint8_t m_index; state m_state; + hba *m_hba; port_data *m_data; void *m_fis; cmd_list_entry *m_cmd_list; diff --git a/src/kernel/pci.cpp b/src/kernel/pci.cpp index 1ddc7c9..cb5338b 100644 --- a/src/kernel/pci.cpp +++ b/src/kernel/pci.cpp @@ -1,4 +1,5 @@ #include "kutil/assert.h" +#include "console.h" #include "log.h" #include "interrupts.h" #include "pci.h" @@ -35,6 +36,33 @@ struct pci_cap_msi64 } __attribute__ ((packed)); +void dump_msi(pci_cap_msi *cap) +{ + auto cons = console::get(); + cons->printf("MSI Cap:\n"); + cons->printf(" id: %02x\n", cap->id); + cons->printf(" next: %02x\n", cap->next); + cons->printf("control: %04x\n", cap->control); + if (cap->control & 0x0080) { + pci_cap_msi64 *cap64 = reinterpret_cast(cap); + cons->printf("address: %016x\n", cap64->address); + cons->printf(" data: %04x\n", cap64->data); + if (cap->control & 0x100) { + cons->printf(" mask: %08x\n", cap64->mask); + cons->printf("pending: %08x\n", cap64->pending); + } + } else { + pci_cap_msi32 *cap32 = reinterpret_cast(cap); + cons->printf("address: %08x\n", cap32->address); + cons->printf(" data: %04x\n", cap32->data); + if (cap->control & 0x100) { + cons->printf(" mask: %08x\n", cap32->mask); + cons->printf("pending: %08x\n", cap32->pending); + } + } + cons->putc('\n'); +}; + pci_device::pci_device() : m_base(nullptr), m_bus_addr(0), @@ -82,8 +110,8 @@ pci_device::pci_device(pci_group &group, uint8_t bus, uint8_t device, uint8_t fu if (cap->id == pci_cap::type::msi) { m_msi = cap; pci_cap_msi *mcap = reinterpret_cast(cap); - mcap->control |= ~0x1; // Mask interrupts - log::debug(logs::device, " - MSI control %08x", mcap->control); + mcap->control &= ~0x70; // at most 1 vector allocated + mcap->control |= 0x01; // Enable interrupts, at most 1 vector allocated } } } @@ -126,20 +154,15 @@ pci_device::write_msi_regs(addr_t address, uint16_t data) pci_cap_msi64 *mcap64 = reinterpret_cast(m_msi); mcap64->address = address; mcap64->data = data; - if (mcap64->control & 0x0100) - log::debug(logs::device, " - MSI mask %08x pending %08x", mcap64->mask, mcap64->pending); } else { pci_cap_msi32 *mcap32 = reinterpret_cast(m_msi); mcap32->address = address; mcap32->data = data; - if (mcap32->control & 0x0100) - log::debug(logs::device, " - MSI mask %08x pending %08x", mcap32->mask, mcap32->pending); } uint16_t control = mcap->control; control &= 0xff8f; // We're allocating one vector, clear 6::4 control |= 0x0001; // Enable MSI mcap->control = control; - } else { kassert(0, "MIS-X is NYI"); }