From 08d7250882dcd73b95dc86892db04c6393fb43db Mon Sep 17 00:00:00 2001 From: Utkarsh Adhran Date: Tue, 7 Jul 2026 10:05:33 +0530 Subject: [PATCH 1/4] hw: use enum class for PCI config registers Replace PCI config register #defines with typed enum classes (config_reg, command, cap_id). Updates in pci_device.cpp and pci_msi.cpp. Related: #2333 --- api/hw/pci_device.hpp | 67 ++++++++++++++++++++++++++----------------- src/hw/pci_device.cpp | 20 +++++++------ src/hw/pci_msi.cpp | 14 +++------ 3 files changed, 55 insertions(+), 46 deletions(-) diff --git a/api/hw/pci_device.hpp b/api/hw/pci_device.hpp index 40921708c1..c0cdaf2682 100644 --- a/api/hw/pci_device.hpp +++ b/api/hw/pci_device.hpp @@ -22,34 +22,47 @@ #include #include -/* PCI Register Config Space */ -#define PCI_DEV_VEND_REG 0x00 /* for the 32 bit read of dev/vend */ -#define PCI_VENDID_REG 0x00 -#define PCI_DEVID_REG 0x02 -#define PCI_CMD_REG 0x04 -#define PCI_STATUS_REG 0x06 -#define PCI_REVID_REG 0x08 -#define PCI_PROGIF_REG 0x09 -#define PCI_SUBCLASS_REG 0x0a -#define PCI_CLASS_REG 0x0b -#define PCI_CLSZ_REG 0x0c -#define PCI_LATTIM_REG 0x0d -#define PCI_HEADER_REG 0x0e -#define PCI_BIST_REG 0x0f -#define PCI_CAPABILITY_REG 0x34 - -#define PCI_COMMAND_IO 0x01 -#define PCI_COMMAND_MEM 0x02 -#define PCI_COMMAND_MASTER 0x04 - -#define PCI_CAP_ID_VNDR 0x09 -#define PCI_CAP_ID_AF 0x13 /* PCI Advanced Features */ -#define PCI_CAP_ID_MAX PCI_CAP_ID_AF -#define PCI_EXT_CAP_ID_PASID 0x1B /* Process Address Space ID */ -#define PCI_EXT_CAP_ID_MAX PCI_EXT_CAP_ID_PASID - namespace PCI { + /** PCI configuration space register offsets */ + enum class config_reg : uint8_t { + DEV_VEND = 0x00, /**< 32-bit device/vendor ID */ + DEVID = 0x02, + CMD = 0x04, + STATUS = 0x06, + REVID = 0x08, + PROGIF = 0x09, + SUBCLASS = 0x0a, + CLASS = 0x0b, + CLSZ = 0x0c, + LATTIM = 0x0d, + HEADER = 0x0e, + BIST = 0x0f, + CAPABILITY = 0x34 + }; + + /** PCI command register flags */ + enum class command : uint16_t { + IO = 0x01, + MEM = 0x02, + MASTER = 0x04 + }; + + /** Standard PCI capability IDs */ + enum class cap_id : uint8_t { + MSI = 0x05, + VNDR = 0x09, + MSIX = 0x11, + AF = 0x13, /**< PCI Advanced Features */ + MAX = AF + }; + + /** Extended PCI capability IDs */ + enum class ext_cap_id : uint8_t { + PASID = 0x1B, /**< Process Address Space ID */ + MAX = PASID + }; + static const uint16_t CONFIG_ADDR {0xCF8U}; static const uint16_t CONFIG_DATA {0xCFCU}; static const uint8_t CONFIG_INTR {0x3CU}; @@ -308,7 +321,7 @@ struct msix_t; int m_iobase = -1; std::array m_resources; - std::array caps; + std::array(PCI::cap_id::MAX)+1> caps; // has msix support if not null msix_t* msix = nullptr; diff --git a/src/hw/pci_device.cpp b/src/hw/pci_device.cpp index 508ffea81d..1dab1bb736 100644 --- a/src/hw/pci_device.cpp +++ b/src/hw/pci_device.cpp @@ -90,9 +90,11 @@ namespace hw { : pci_addr_{pci_addr}, device_id_{device_id} { // set master, mem and io flags - uint32_t cmd = read32(PCI_CMD_REG); - cmd |= PCI_COMMAND_MASTER | PCI_COMMAND_MEM | PCI_COMMAND_IO; - write_dword(PCI_CMD_REG, cmd); + uint32_t cmd = read32(static_cast(PCI::config_reg::CMD)); + cmd |= static_cast(PCI::command::MASTER) + | static_cast(PCI::command::MEM) + | static_cast(PCI::command::IO); + write_dword(static_cast(PCI::config_reg::CMD), cmd); // device class info is coming from pci manager to save a PCI read this->devtype_.reg = devclass; @@ -167,11 +169,11 @@ namespace hw { caps = {}; // the capability list is only available if bit 4 // in the status register is set - uint16_t status = read16(PCI_STATUS_REG); + uint16_t status = read16(static_cast(PCI::config_reg::STATUS)); //printf("read16 %#x status %#x\n", PCI_STATUS_REG, status); if ((status & 0x10) == 0) return; // this offset works for non-cardbus bridges - uint32_t offset = PCI_CAPABILITY_REG; + uint32_t offset = static_cast(PCI::config_reg::CAPABILITY); // read first capability offset = read16(offset) & 0xff; offset &= ~0x3; // lower 2 bits reserved @@ -189,19 +191,19 @@ namespace hw { void PCI_Device::deactivate() { // disables device (except for configuration) - write_dword(PCI_CMD_REG, 0); + write_dword(static_cast(PCI::config_reg::CMD), 0); } void PCI_Device::intx_enable() { - auto cmd = read16(PCI_CMD_REG); - write16(PCI_CMD_REG, cmd & ~(1 << 10)); + auto cmd = read16(static_cast(PCI::config_reg::CMD)); + write16(static_cast(PCI::config_reg::CMD), cmd & ~(1 << 10)); // delete msi-x if (this->msix) delete this->msix; } bool PCI_Device::intx_status() { - auto stat = read16(PCI_STATUS_REG); + auto stat = read16(static_cast(PCI::config_reg::STATUS)); return stat & (1 << 3); } diff --git a/src/hw/pci_msi.cpp b/src/hw/pci_msi.cpp index 358aa3e253..de7c34b2ee 100644 --- a/src/hw/pci_msi.cpp +++ b/src/hw/pci_msi.cpp @@ -2,12 +2,6 @@ #include #include -#define PCI_CMD_REG 0x04 - -// MSI and MSI-X capability registers -#define PCI_CAP_ID_MSI 0x05 /* Message Signalled Interrupts */ -#define PCI_CAP_ID_MSIX 0x11 /* MSI-X */ - // Message Signalled Interrupts registers #define PCI_MSI_FLAGS_ENABLE 0x0001 /* MSI feature enabled */ #define PCI_MSI_FLAGS_QMASK 0x000e /* Maximum queue size available */ @@ -30,20 +24,20 @@ namespace hw { int PCI_Device::msi_cap() { - return caps[PCI_CAP_ID_MSI]; + return caps[static_cast(PCI::cap_id::MSI)]; } int PCI_Device::msix_cap() { - return caps[PCI_CAP_ID_MSIX]; + return caps[static_cast(PCI::cap_id::MSIX)]; } void PCI_Device::init_msix() { assert(this->msix == nullptr); // disable intx - auto cmd = read16(PCI_CMD_REG); - write16(PCI_CMD_REG, cmd | (1 << 10)); + auto cmd = read16(static_cast(PCI::config_reg::CMD)); + write16(static_cast(PCI::config_reg::CMD), cmd | (1 << 10)); // enable MSI-X this->msix = new msix_t(*this, msix_cap()); // deallocate if it failed From e47ca607188ffefe8c78814196ddba276b106352 Mon Sep 17 00:00:00 2001 From: Utkarsh Adhran Date: Wed, 8 Jul 2026 15:45:28 +0530 Subject: [PATCH 2/4] hw: typed PCI config accessors and command bitmask ops - Add read/write overloads taking PCI::config_reg; uint8_t overloads delegate - Enable enable_bitmask_ops for IO | MEM | MASTER - Document enum names against Linux pci_regs.h / PCI Local Bus Spec - Use command::INTX_DISABLE instead of raw bit 10 in intx/msix paths --- api/hw/pci_device.hpp | 70 +++++++++++++++++++++++++++++-------------- src/hw/pci_device.cpp | 45 +++++++++++++++++++++------- src/hw/pci_msi.cpp | 5 ++-- 3 files changed, 84 insertions(+), 36 deletions(-) diff --git a/api/hw/pci_device.hpp b/api/hw/pci_device.hpp index c0cdaf2682..65819f9dd7 100644 --- a/api/hw/pci_device.hpp +++ b/api/hw/pci_device.hpp @@ -21,39 +21,47 @@ #include #include #include +#include namespace PCI { - /** PCI configuration space register offsets */ + /** + * PCI configuration space register offsets. + * + * Names follow IncludeOS PCI_* macros; see Linux include/linux/pci_regs.h + * (PCI_COMMAND, PCI_CACHE_LINE_SIZE, PCI_CAPABILITY_LIST, etc.) and the + * PCI Local Bus Specification: https://www.pcisig.com/specifications + */ enum class config_reg : uint8_t { - DEV_VEND = 0x00, /**< 32-bit device/vendor ID */ - DEVID = 0x02, - CMD = 0x04, - STATUS = 0x06, - REVID = 0x08, - PROGIF = 0x09, - SUBCLASS = 0x0a, - CLASS = 0x0b, - CLSZ = 0x0c, - LATTIM = 0x0d, - HEADER = 0x0e, - BIST = 0x0f, - CAPABILITY = 0x34 + DEV_VEND = 0x00, /**< PCI_VENDOR_ID — 32-bit device/vendor ID */ + DEVID = 0x02, /**< PCI_DEVICE_ID */ + CMD = 0x04, /**< PCI_COMMAND */ + STATUS = 0x06, /**< PCI_STATUS */ + REVID = 0x08, /**< PCI_REVISION_ID */ + PROGIF = 0x09, /**< PCI_CLASS_PROG */ + SUBCLASS = 0x0a, /**< PCI_CLASS_DEVICE */ + CLASS = 0x0b, /**< Class code byte */ + CLSZ = 0x0c, /**< PCI_CACHE_LINE_SIZE */ + LATTIM = 0x0d, /**< PCI_LATENCY_TIMER */ + HEADER = 0x0e, /**< PCI_HEADER_TYPE */ + BIST = 0x0f, /**< PCI_BIST */ + CAPABILITY = 0x34 /**< PCI_CAPABILITY_LIST */ }; - /** PCI command register flags */ + /** PCI command register flags (PCI_COMMAND_*) */ enum class command : uint16_t { - IO = 0x01, - MEM = 0x02, - MASTER = 0x04 + IO = 0x01, /**< PCI_COMMAND_IO */ + MEM = 0x02, /**< PCI_COMMAND_MEMORY */ + MASTER = 0x04, /**< PCI_COMMAND_MASTER */ + INTX_DISABLE = 0x400, /**< PCI_COMMAND_INTX_DISABLE */ }; - /** Standard PCI capability IDs */ + /** Standard PCI capability IDs (PCI_CAP_ID_*) */ enum class cap_id : uint8_t { - MSI = 0x05, - VNDR = 0x09, - MSIX = 0x11, - AF = 0x13, /**< PCI Advanced Features */ + MSI = 0x05, /**< PCI_CAP_ID_MSI — Message Signalled Interrupts */ + VNDR = 0x09, /**< PCI_CAP_ID_VNDR — Vendor-specific */ + MSIX = 0x11, /**< PCI_CAP_ID_MSIX — MSI-X */ + AF = 0x13, /**< PCI_CAP_ID_AF — PCI Advanced Features */ MAX = AF }; @@ -183,15 +191,20 @@ struct msix_t; explicit PCI_Device(const uint16_t pci_addr, const uint32_t, const uint32_t); //! @brief Read from device with implicit pci_address (e.g. used by Nic) + uint32_t read32(PCI::config_reg reg) noexcept; uint32_t read32(const uint8_t reg) noexcept; //! @brief Read from device with explicit pci_addr + static uint32_t read_dword(const uint16_t pci_addr, PCI::config_reg reg) noexcept; static uint32_t read_dword(const uint16_t pci_addr, const uint8_t reg) noexcept; //! @brief Write to device with implicit pci_address (e.g. used by Nic) + void write_dword(PCI::config_reg reg, const uint32_t value) noexcept; void write_dword(const uint8_t reg, const uint32_t value) noexcept; + uint16_t read16(PCI::config_reg reg) noexcept; uint16_t read16(const uint8_t reg) noexcept; + void write16(PCI::config_reg reg, const uint16_t value) noexcept; void write16(const uint8_t reg, const uint16_t value) noexcept; /** A descriptive name */ @@ -374,4 +387,15 @@ static const char* PCI::vendor_str(uint16_t code){ return it == classcodes.end() ? "Unknown vendor" : it->second; } +/** Enable bitmask operators for PCI command register flags */ +namespace util { +inline namespace bitops { +template<> +struct enable_bitmask_ops { + using type = std::underlying_type::type; + static constexpr bool enable = true; +}; +} +} + #endif //< HW_PCI_DEVICE_HPP diff --git a/src/hw/pci_device.cpp b/src/hw/pci_device.cpp index 1dab1bb736..09cf922ced 100644 --- a/src/hw/pci_device.cpp +++ b/src/hw/pci_device.cpp @@ -20,9 +20,12 @@ #include #include #include +#include namespace hw { + using namespace util::bitops; + static constexpr std::array bridge_subclasses { "Host", "ISA", @@ -90,16 +93,36 @@ namespace hw { : pci_addr_{pci_addr}, device_id_{device_id} { // set master, mem and io flags - uint32_t cmd = read32(static_cast(PCI::config_reg::CMD)); - cmd |= static_cast(PCI::command::MASTER) - | static_cast(PCI::command::MEM) - | static_cast(PCI::command::IO); - write_dword(static_cast(PCI::config_reg::CMD), cmd); + uint32_t cmd = read32(PCI::config_reg::CMD); + cmd |= static_cast(PCI::command::MASTER + | PCI::command::MEM + | PCI::command::IO); + write_dword(PCI::config_reg::CMD, cmd); // device class info is coming from pci manager to save a PCI read this->devtype_.reg = devclass; } + uint32_t PCI_Device::read32(PCI::config_reg reg) noexcept { + return read32(static_cast(reg)); + } + + uint32_t PCI_Device::read_dword(const uint16_t pci_addr, PCI::config_reg reg) noexcept { + return read_dword(pci_addr, static_cast(reg)); + } + + void PCI_Device::write_dword(PCI::config_reg reg, const uint32_t value) noexcept { + write_dword(static_cast(reg), value); + } + + uint16_t PCI_Device::read16(PCI::config_reg reg) noexcept { + return read16(static_cast(reg)); + } + + void PCI_Device::write16(PCI::config_reg reg, const uint16_t value) noexcept { + write16(static_cast(reg), value); + } + uint32_t PCI_Device::read_dword(const uint16_t pci_addr, const uint8_t reg) noexcept { PCI::msg req; @@ -169,8 +192,7 @@ namespace hw { caps = {}; // the capability list is only available if bit 4 // in the status register is set - uint16_t status = read16(static_cast(PCI::config_reg::STATUS)); - //printf("read16 %#x status %#x\n", PCI_STATUS_REG, status); + uint16_t status = read16(PCI::config_reg::STATUS); if ((status & 0x10) == 0) return; // this offset works for non-cardbus bridges uint32_t offset = static_cast(PCI::config_reg::CAPABILITY); @@ -191,19 +213,20 @@ namespace hw { void PCI_Device::deactivate() { // disables device (except for configuration) - write_dword(static_cast(PCI::config_reg::CMD), 0); + write_dword(PCI::config_reg::CMD, 0); } void PCI_Device::intx_enable() { - auto cmd = read16(static_cast(PCI::config_reg::CMD)); - write16(static_cast(PCI::config_reg::CMD), cmd & ~(1 << 10)); + auto cmd = read16(PCI::config_reg::CMD); + write16(PCI::config_reg::CMD, + cmd & ~static_cast(PCI::command::INTX_DISABLE)); // delete msi-x if (this->msix) delete this->msix; } bool PCI_Device::intx_status() { - auto stat = read16(static_cast(PCI::config_reg::STATUS)); + auto stat = read16(PCI::config_reg::STATUS); return stat & (1 << 3); } diff --git a/src/hw/pci_msi.cpp b/src/hw/pci_msi.cpp index de7c34b2ee..45be321589 100644 --- a/src/hw/pci_msi.cpp +++ b/src/hw/pci_msi.cpp @@ -36,8 +36,9 @@ namespace hw { assert(this->msix == nullptr); // disable intx - auto cmd = read16(static_cast(PCI::config_reg::CMD)); - write16(static_cast(PCI::config_reg::CMD), cmd | (1 << 10)); + auto cmd = read16(PCI::config_reg::CMD); + write16(PCI::config_reg::CMD, + cmd | static_cast(PCI::command::INTX_DISABLE)); // enable MSI-X this->msix = new msix_t(*this, msix_cap()); // deallocate if it failed From 9d21426c65ca5096e402a47e73ee7d42c64ee332 Mon Sep 17 00:00:00 2001 From: Utkarsh Adhran Date: Thu, 9 Jul 2026 17:29:16 +0530 Subject: [PATCH 3/4] hw: make config_reg the primary PCI config accessor path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Own the HW path in the typed overloads and keep uint8_t as a thin fallback for dynamic offsets. Operate on PCI::command with bitops and only cast at the wire boundary. Tested: nix-build unittests.nix — 85/85 passed --- api/hw/pci_device.hpp | 6 +++ src/hw/pci_device.cpp | 95 ++++++++++++++++++++++--------------------- src/hw/pci_msi.cpp | 7 ++-- 3 files changed, 58 insertions(+), 50 deletions(-) diff --git a/api/hw/pci_device.hpp b/api/hw/pci_device.hpp index 65819f9dd7..c5b5f00757 100644 --- a/api/hw/pci_device.hpp +++ b/api/hw/pci_device.hpp @@ -190,6 +190,12 @@ struct msix_t; */ explicit PCI_Device(const uint16_t pci_addr, const uint32_t, const uint32_t); + /** + * Config-space accessors. + * + * Prefer PCI::config_reg overloads (own the HW path). uint8_t overloads + * are thin fallbacks for dynamic offsets (BARs, capability chain, etc.). + */ //! @brief Read from device with implicit pci_address (e.g. used by Nic) uint32_t read32(PCI::config_reg reg) noexcept; uint32_t read32(const uint8_t reg) noexcept; diff --git a/src/hw/pci_device.cpp b/src/hw/pci_device.cpp index 09cf922ced..3220f89ea9 100644 --- a/src/hw/pci_device.cpp +++ b/src/hw/pci_device.cpp @@ -92,88 +92,91 @@ namespace hw { const uint32_t devclass) : pci_addr_{pci_addr}, device_id_{device_id} { - // set master, mem and io flags - uint32_t cmd = read32(PCI::config_reg::CMD); - cmd |= static_cast(PCI::command::MASTER - | PCI::command::MEM - | PCI::command::IO); - write_dword(PCI::config_reg::CMD, cmd); + // set master, mem and io flags (16-bit command register only) + auto cmd = static_cast(read16(PCI::config_reg::CMD)); + cmd |= PCI::command::MASTER | PCI::command::MEM | PCI::command::IO; + write16(PCI::config_reg::CMD, static_cast(cmd)); // device class info is coming from pci manager to save a PCI read this->devtype_.reg = devclass; } + // Typed config-space accessors own the HW path; cast only at the wire boundary. uint32_t PCI_Device::read32(PCI::config_reg reg) noexcept { - return read32(static_cast(reg)); - } - - uint32_t PCI_Device::read_dword(const uint16_t pci_addr, PCI::config_reg reg) noexcept { - return read_dword(pci_addr, static_cast(reg)); - } - - void PCI_Device::write_dword(PCI::config_reg reg, const uint32_t value) noexcept { - write_dword(static_cast(reg), value); - } + PCI::msg req; - uint16_t PCI_Device::read16(PCI::config_reg reg) noexcept { - return read16(static_cast(reg)); - } + req.data = 0x80000000; + req.addr = pci_addr_; + req.reg = static_cast(reg); - void PCI_Device::write16(PCI::config_reg reg, const uint16_t value) noexcept { - write16(static_cast(reg), value); + outpd(PCI::CONFIG_ADDR, 0x80000000 | req.data); + return inpd(PCI::CONFIG_DATA); } - uint32_t PCI_Device::read_dword(const uint16_t pci_addr, const uint8_t reg) noexcept { + uint32_t PCI_Device::read_dword(const uint16_t pci_addr, PCI::config_reg reg) noexcept { PCI::msg req; req.data = 0x80000000; req.addr = pci_addr; - req.reg = reg; + req.reg = static_cast(reg); outpd(PCI::CONFIG_ADDR, 0x80000000 | req.data); return inpd(PCI::CONFIG_DATA); } - void PCI_Device::write_dword(const uint8_t reg, const uint32_t value) noexcept { + + void PCI_Device::write_dword(PCI::config_reg reg, const uint32_t value) noexcept { PCI::msg req; req.data = 0x80000000; req.addr = pci_addr_; - req.reg = reg; + req.reg = static_cast(reg); outpd(PCI::CONFIG_ADDR, 0x80000000 | req.data); outpd(PCI::CONFIG_DATA, value); } - uint32_t PCI_Device::read32(const uint8_t reg) noexcept { + __attribute__((noinline)) + uint16_t PCI_Device::read16(PCI::config_reg reg) noexcept { + const auto off = static_cast(reg); PCI::msg req; - req.data = 0x80000000; req.addr = pci_addr_; - req.reg = reg; + req.reg = off; outpd(PCI::CONFIG_ADDR, 0x80000000 | req.data); - return inpd(PCI::CONFIG_DATA); + return inpw(PCI::CONFIG_DATA + (off & 2)); } - __attribute__((noinline)) - uint16_t PCI_Device::read16(const uint8_t reg) noexcept { + void PCI_Device::write16(PCI::config_reg reg, const uint16_t value) noexcept { + const auto off = static_cast(reg); PCI::msg req; req.data = 0x80000000; req.addr = pci_addr_; - req.reg = reg; + req.reg = off; outpd(PCI::CONFIG_ADDR, 0x80000000 | req.data); - uint16_t data = inpw(PCI::CONFIG_DATA + (reg & 2)); - return data; + outpw(PCI::CONFIG_DATA + (off & 2), value); } - void PCI_Device::write16(const uint8_t reg, const uint16_t value) noexcept { - PCI::msg req; - req.data = 0x80000000; - req.addr = pci_addr_; - req.reg = reg; - outpd(PCI::CONFIG_ADDR, 0x80000000 | req.data); - outpw(PCI::CONFIG_DATA + (reg & 2), value); + // Raw-offset fallbacks for dynamic addresses (BARs, capability chain, CONFIG_INTR). + uint32_t PCI_Device::read_dword(const uint16_t pci_addr, const uint8_t reg) noexcept { + return read_dword(pci_addr, static_cast(reg)); + } + + void PCI_Device::write_dword(const uint8_t reg, const uint32_t value) noexcept { + write_dword(static_cast(reg), value); + } + + uint32_t PCI_Device::read32(const uint8_t reg) noexcept { + return read32(static_cast(reg)); + } + + uint16_t PCI_Device::read16(const uint8_t reg) noexcept { + return read16(static_cast(reg)); + } + + void PCI_Device::write16(const uint8_t reg, const uint16_t value) noexcept { + write16(static_cast(reg), value); } union capability_t @@ -195,9 +198,7 @@ namespace hw { uint16_t status = read16(PCI::config_reg::STATUS); if ((status & 0x10) == 0) return; // this offset works for non-cardbus bridges - uint32_t offset = static_cast(PCI::config_reg::CAPABILITY); - // read first capability - offset = read16(offset) & 0xff; + uint32_t offset = read16(PCI::config_reg::CAPABILITY) & 0xff; offset &= ~0x3; // lower 2 bits reserved while (offset) { @@ -218,9 +219,9 @@ namespace hw { void PCI_Device::intx_enable() { - auto cmd = read16(PCI::config_reg::CMD); - write16(PCI::config_reg::CMD, - cmd & ~static_cast(PCI::command::INTX_DISABLE)); + auto cmd = static_cast(read16(PCI::config_reg::CMD)); + cmd &= ~PCI::command::INTX_DISABLE; + write16(PCI::config_reg::CMD, static_cast(cmd)); // delete msi-x if (this->msix) delete this->msix; } diff --git a/src/hw/pci_msi.cpp b/src/hw/pci_msi.cpp index 45be321589..1ff8ecd22d 100644 --- a/src/hw/pci_msi.cpp +++ b/src/hw/pci_msi.cpp @@ -36,9 +36,10 @@ namespace hw { assert(this->msix == nullptr); // disable intx - auto cmd = read16(PCI::config_reg::CMD); - write16(PCI::config_reg::CMD, - cmd | static_cast(PCI::command::INTX_DISABLE)); + using namespace util::bitops; + auto cmd = static_cast(read16(PCI::config_reg::CMD)); + cmd |= PCI::command::INTX_DISABLE; + write16(PCI::config_reg::CMD, static_cast(cmd)); // enable MSI-X this->msix = new msix_t(*this, msix_cap()); // deallocate if it failed From a502cf91369d8bbc27de3647c92e126e76031e97 Mon Sep 17 00:00:00 2001 From: Utkarsh Adhran Date: Fri, 10 Jul 2026 17:52:01 +0530 Subject: [PATCH 4/4] hw: drop unused ext_cap_id and trim PCI enum docs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tested: nix-build unittests.nix — 85/85 passed --- api/hw/pci_device.hpp | 65 +++++++++++++++---------------------------- 1 file changed, 23 insertions(+), 42 deletions(-) diff --git a/api/hw/pci_device.hpp b/api/hw/pci_device.hpp index c5b5f00757..b4b6b472d7 100644 --- a/api/hw/pci_device.hpp +++ b/api/hw/pci_device.hpp @@ -25,52 +25,38 @@ namespace PCI { - /** - * PCI configuration space register offsets. - * - * Names follow IncludeOS PCI_* macros; see Linux include/linux/pci_regs.h - * (PCI_COMMAND, PCI_CACHE_LINE_SIZE, PCI_CAPABILITY_LIST, etc.) and the - * PCI Local Bus Specification: https://www.pcisig.com/specifications - */ + // Names/values match Linux include/linux/pci_regs.h (PCI Local Bus Spec). enum class config_reg : uint8_t { - DEV_VEND = 0x00, /**< PCI_VENDOR_ID — 32-bit device/vendor ID */ - DEVID = 0x02, /**< PCI_DEVICE_ID */ - CMD = 0x04, /**< PCI_COMMAND */ - STATUS = 0x06, /**< PCI_STATUS */ - REVID = 0x08, /**< PCI_REVISION_ID */ - PROGIF = 0x09, /**< PCI_CLASS_PROG */ - SUBCLASS = 0x0a, /**< PCI_CLASS_DEVICE */ - CLASS = 0x0b, /**< Class code byte */ - CLSZ = 0x0c, /**< PCI_CACHE_LINE_SIZE */ - LATTIM = 0x0d, /**< PCI_LATENCY_TIMER */ - HEADER = 0x0e, /**< PCI_HEADER_TYPE */ - BIST = 0x0f, /**< PCI_BIST */ - CAPABILITY = 0x34 /**< PCI_CAPABILITY_LIST */ + DEV_VEND = 0x00, + DEVID = 0x02, + CMD = 0x04, + STATUS = 0x06, + REVID = 0x08, + PROGIF = 0x09, + SUBCLASS = 0x0a, + CLASS = 0x0b, + CLSZ = 0x0c, + LATTIM = 0x0d, + HEADER = 0x0e, + BIST = 0x0f, + CAPABILITY = 0x34 }; - /** PCI command register flags (PCI_COMMAND_*) */ enum class command : uint16_t { - IO = 0x01, /**< PCI_COMMAND_IO */ - MEM = 0x02, /**< PCI_COMMAND_MEMORY */ - MASTER = 0x04, /**< PCI_COMMAND_MASTER */ - INTX_DISABLE = 0x400, /**< PCI_COMMAND_INTX_DISABLE */ + IO = 0x01, + MEM = 0x02, + MASTER = 0x04, + INTX_DISABLE = 0x400, }; - /** Standard PCI capability IDs (PCI_CAP_ID_*) */ enum class cap_id : uint8_t { - MSI = 0x05, /**< PCI_CAP_ID_MSI — Message Signalled Interrupts */ - VNDR = 0x09, /**< PCI_CAP_ID_VNDR — Vendor-specific */ - MSIX = 0x11, /**< PCI_CAP_ID_MSIX — MSI-X */ - AF = 0x13, /**< PCI_CAP_ID_AF — PCI Advanced Features */ + MSI = 0x05, + VNDR = 0x09, + MSIX = 0x11, + AF = 0x13, MAX = AF }; - /** Extended PCI capability IDs */ - enum class ext_cap_id : uint8_t { - PASID = 0x1B, /**< Process Address Space ID */ - MAX = PASID - }; - static const uint16_t CONFIG_ADDR {0xCF8U}; static const uint16_t CONFIG_DATA {0xCFCU}; static const uint8_t CONFIG_INTR {0x3CU}; @@ -190,12 +176,7 @@ struct msix_t; */ explicit PCI_Device(const uint16_t pci_addr, const uint32_t, const uint32_t); - /** - * Config-space accessors. - * - * Prefer PCI::config_reg overloads (own the HW path). uint8_t overloads - * are thin fallbacks for dynamic offsets (BARs, capability chain, etc.). - */ + // config_reg overloads own the HW path; uint8_t is for dynamic offsets. //! @brief Read from device with implicit pci_address (e.g. used by Nic) uint32_t read32(PCI::config_reg reg) noexcept; uint32_t read32(const uint8_t reg) noexcept;