[pci] little bit of code cleanup in the pci bus driver

Mostly just suggestions by clang-tidy. No functional change.
This commit is contained in:
Travis Geiselbrecht
2025-09-23 23:16:55 -07:00
parent 5a17519e54
commit 72112c0676
10 changed files with 55 additions and 50 deletions

View File

@@ -142,7 +142,7 @@ void bridge::extend_subordinate_range(uint8_t new_secondary_bus) {
DEBUG_ASSERT(subordinate_bus() == new_secondary_bus);
// tell the parent bridge that we have a new range
auto *parent_bridge = bus_->get_bridge();
auto *parent_bridge = parent_bus()->get_bridge();
if (parent_bridge) {
parent_bridge->extend_subordinate_range(new_secondary_bus);
}
@@ -154,12 +154,12 @@ void bridge::assign_bus_numbers(uint8_t primary, uint8_t secondary, uint8_t subo
uint32_t temp;
pci_read_config_word(loc_, 0x18, &temp);
pci_read_config_word(loc(), 0x18, &temp);
temp &= 0xff000000; // leave latency timer alone
temp |= subordinate << 16;
temp |= secondary << 8;
temp |= primary << 0;
pci_write_config_word(loc_, 0x18, temp);
pci_write_config_word(loc(), 0x18, temp);
// reread the config
load_config();
@@ -174,7 +174,7 @@ void bridge::dump(size_t indent) {
};
char str[14];
scoot();
printf("bridge %s %04hx:%04hx primary bus %d child busses [%d..%d]\n", pci_loc_string(loc_, str),
printf("bridge %s %04hx:%04hx primary bus %d child busses [%d..%d]\n", pci_loc_string(loc(), str),
vendor_id(), device_id(), primary_bus(),
secondary_bus(), subordinate_bus());
@@ -186,11 +186,11 @@ void bridge::dump(size_t indent) {
mr.base, mr.limit, ir.base, ir.limit, pr.base, pr.limit);
for (size_t b = 0; b < 2; b++) {
if (bars_[b].valid) {
if (bar(b).valid) {
for (size_t i = 0; i < indent + 1; i++) {
printf(" ");
}
printf("BAR %zu: addr %#" PRIx64 " size %#zx io %d valid %d\n", b, bars_[b].addr, bars_[b].size, bars_[b].io, bars_[b].valid);
printf("BAR %zu: addr %#" PRIx64 " size %#zx io %d valid %d\n", b, bar(b).addr, bar(b).size, bar(b).io, bar(b).valid);
}
}
@@ -201,39 +201,39 @@ void bridge::dump(size_t indent) {
// accessors to compute the io and memory range of the bridge
bridge::range<uint32_t> bridge::io_range() {
if (config_.type1.io_limit < config_.type1.io_base) {
if (config().type1.io_limit < config().type1.io_base) {
return { 0, 0 };
} else {
// TODO: handle 32bit io (does this really exist?)
return { ((uint32_t)config_.type1.io_base >> 4) << 12,
(((uint32_t)config_.type1.io_limit >> 4) << 12) | 0xfff };
return { ((uint32_t)config().type1.io_base >> 4) << 12,
(((uint32_t)config().type1.io_limit >> 4) << 12) | 0xfff };
}
}
bridge::range<uint32_t> bridge::mem_range() {
if (config_.type1.memory_limit < config_.type1.memory_base) {
if (config().type1.memory_limit < config().type1.memory_base) {
return { 0, 0 };
} else {
return { ((uint32_t)config_.type1.memory_base >> 4) << 20,
(((uint32_t)config_.type1.memory_limit >> 4) << 20) | 0xfffff };
return { ((uint32_t)config().type1.memory_base >> 4) << 20,
(((uint32_t)config().type1.memory_limit >> 4) << 20) | 0xfffff };
}
}
bridge::range<uint64_t> bridge::prefetch_range() {
if (config_.type1.prefetchable_memory_limit < config_.type1.prefetchable_memory_base) {
if (config().type1.prefetchable_memory_limit < config().type1.prefetchable_memory_base) {
return { 0, 0 };
} else {
bool is_64 = (config_.type1.prefetchable_memory_base & 0xf) == 1;
bool is_64 = (config().type1.prefetchable_memory_base & 0xf) == 1;
uint64_t base, limit;
base = (((uint64_t)config_.type1.prefetchable_memory_base >> 4) << 20);
base = (((uint64_t)config().type1.prefetchable_memory_base >> 4) << 20);
if (is_64) {
base |= (uint64_t)config_.type1.prefetchable_base_upper << 32;
base |= (uint64_t)config().type1.prefetchable_base_upper << 32;
}
limit = (((uint64_t)config_.type1.prefetchable_memory_limit >> 4) << 20) | 0xfffff;
limit = (((uint64_t)config().type1.prefetchable_memory_limit >> 4) << 20) | 0xfffff;
if (is_64) {
limit |= (uint64_t)config_.type1.prefetchable_limit_upper << 32;
limit |= (uint64_t)config().type1.prefetchable_limit_upper << 32;
}
return { base, limit };
}
@@ -403,7 +403,7 @@ status_t bridge::assign_resource(bar_alloc_request *request, uint64_t address) {
return ERR_NOT_SUPPORTED;
}
// assert that the device supports 64bit addresses
DEBUG_ASSERT((config_.type1.prefetchable_memory_base & 0xf) == 1);
DEBUG_ASSERT((config().type1.prefetchable_memory_base & 0xf) == 1);
DEBUG_ASSERT(IS_ALIGNED(address, (1UL << 20)));
DEBUG_ASSERT(IS_ALIGNED(request->size, (1UL << 20)));

View File

@@ -45,9 +45,9 @@ public:
void dump(size_t indent = 0) override;
// config accessors
uint8_t primary_bus() const { return config_.type1.primary_bus; }
uint8_t secondary_bus() const { return config_.type1.secondary_bus; }
uint8_t subordinate_bus() const { return config_.type1.subordinate_bus; }
uint8_t primary_bus() const { return config().type1.primary_bus; }
uint8_t secondary_bus() const { return config().type1.secondary_bus; }
uint8_t subordinate_bus() const { return config().type1.subordinate_bus; }
template <typename T>
struct range {

View File

@@ -34,12 +34,12 @@ namespace pci {
bus *root = nullptr;
list_node bus_list = LIST_INITIAL_VALUE(bus_list);
namespace {
uint8_t last_bus = 0;
resource_allocator resources;
namespace {
// local helper routines
// iterate all devices on all busses with the functor
template <typename F>

View File

@@ -10,7 +10,6 @@
#include <lk/cpp.h>
#include <lk/err.h>
#include <lk/trace.h>
#include <assert.h>
#include <sys/types.h>
#include <dev/bus/pci.h>

View File

@@ -120,9 +120,10 @@ void device::dump(size_t indent) {
printf(" ");
}
char str[14];
printf("dev %s vid:pid %04hx:%04hx base:sub:intr %hhu:%hhu:%hhu %s%s\n",
printf("dev %s vid:pid %04hx:%04hx base:sub:intr %hhu:%hhu:%hhu int %u %s%s\n",
pci_loc_string(loc_, str), config_.vendor_id, config_.device_id,
base_class(), sub_class(), interface(),
config_.type0.interrupt_line,
has_msi() ? "msi " : "",
has_msix() ? "msix " : "");
for (size_t b = 0; b < countof(bars_); b++) {
@@ -394,7 +395,7 @@ status_t device::load_bars() {
bars_[i].prefetchable = bar_addr & (1<<3);
bars_[i].size_64 = true;
bars_[i].addr = bar_addr & ~0xf;
bars_[i].addr |= (uint64_t)config_.type0.base_addresses[i + 1] << 32;
bars_[i].addr |= static_cast<uint64_t>(config_.type0.base_addresses[i + 1]) << 32;
// probe size by writing all 1s and seeing what bits are masked
uint64_t size;
@@ -404,12 +405,12 @@ status_t device::load_bars() {
size = size32;
pci_write_config_word(loc_, PCI_CONFIG_BASE_ADDRESSES + i * 4 + 4, 0xffffffff);
pci_read_config_word(loc_, PCI_CONFIG_BASE_ADDRESSES + i * 4 + 4, &size32);
size |= (uint64_t)size32 << 32;
size |= static_cast<uint64_t>(size32) << 32;
pci_write_config_word(loc_, PCI_CONFIG_BASE_ADDRESSES + i * 4, bars_[i].addr);
pci_write_config_word(loc_, PCI_CONFIG_BASE_ADDRESSES + i * 4 + 4, bars_[i].addr >> 32);
// mask out bottom bits, invert and add 1 to compute size
bars_[i].size = (~(size & ~(uint64_t)0b1111)) + 1;
bars_[i].size = (~(size & ~static_cast<uint64_t>(0b1111))) + 1;
bars_[i].valid = (bars_[i].size != 0);

View File

@@ -97,6 +97,14 @@ public:
virtual void dump(size_t indent = 0);
protected:
const pci_config_t &config() const { return config_; }
const pci_bar_t &bar(size_t index) const {
DEBUG_ASSERT(index < 6);
return bars_[index];
}
bus *parent_bus() const { return bus_; }
private:
// let the bus device directly manipulate our list node
friend class bus;
list_node node = LIST_INITIAL_CLEARED_VALUE;

View File

@@ -19,7 +19,7 @@ struct resource_range {
uint64_t base;
uint64_t size;
void dump() {
void dump() const {
printf("resource type %d: base %#llx size %#llx\n",
type, base, size);
}
@@ -32,7 +32,7 @@ struct resource_range_set {
resource_range mmio_prefetchable;
resource_range mmio64_prefetchable;
void dump() {
void dump() const {
printf("resource range set:\n");
io.dump();
mmio.dump();

View File

@@ -17,19 +17,20 @@
#include <dev/bus/pci.h>
#include <lk/console_cmd.h>
namespace {
/*
* enumerates pci devices
*/
static void pci_list(void) {
void pci_list() {
pci_location_t state;
int busses = 0, devices = 0, ret;
printf("Scanning (brute force)...\n");
for (int segment = 0; segment <= (int)pci_get_last_segment(); segment++) {
for (int segment = 0; segment <= pci_get_last_segment(); segment++) {
state.segment = segment;
for (int bus = 0; bus <= (int)pci_get_last_bus(); bus++) {
for (int bus = 0; bus <= pci_get_last_bus(); bus++) {
state.bus = bus;
busses++;
@@ -83,7 +84,7 @@ error:
* a somewhat fugly pci config space examine/modify command. this should probably
* be broken up a bit.
*/
static int pci_config(int argc, const console_cmd_args *argv) {
int pci_config(int argc, const console_cmd_args *argv) {
pci_location_t loc;
pci_config_t config;
uint32_t offset;
@@ -146,12 +147,11 @@ static int pci_config(int argc, const console_cmd_args *argv) {
break;
case 1: // 64 bit prefetchable addressing
printf("prefetchable base=%llx prefetchable limit=%llx\n",
((uint64_t)config.type1.prefetchable_memory_base & 0xfff0) << 16 |
((uint64_t)config.type1.prefetchable_base_upper << 32),
((uint64_t)config.type1.prefetchable_memory_limit & 0xfff0) << 16 | 0xf'ffff |
((uint64_t)config.type1.prefetchable_limit_upper << 32));
(static_cast<uint64_t>(config.type1.prefetchable_memory_base) & 0xfff0) << 16 |
(static_cast<uint64_t>(config.type1.prefetchable_base_upper) << 32),
(static_cast<uint64_t>(config.type1.prefetchable_memory_limit) & 0xfff0) << 16 | 0xf'ffff |
(static_cast<uint64_t>(config.type1.prefetchable_limit_upper) << 32));
break;
}
}
hexdump8_ex(&config, sizeof(config), 0);
@@ -257,7 +257,7 @@ error:
return -2;
}
static int pci_cmd(int argc, const console_cmd_args *argv) {
int pci_cmd(int argc, const console_cmd_args *argv) {
if (argc < 2) {
printf("pci commands:\n");
usage:
@@ -287,3 +287,5 @@ STATIC_COMMAND_START
STATIC_COMMAND("pci", "pci toolbox", &pci_cmd)
STATIC_COMMAND_END(pcitests);
} // namespace

View File

@@ -8,7 +8,6 @@
*/
#pragma once
#include <assert.h>
#include <sys/types.h>
#include <lk/compiler.h>
@@ -87,16 +86,16 @@ status_t pci_bus_mgr_find_device(pci_location_t *state, uint16_t device_id, uint
status_t pci_bus_mgr_find_device_by_class(pci_location_t *state, uint8_t base_class, uint8_t subclass, uint8_t interface, size_t index);
// set io and mem enable on the device
status_t pci_bus_mgr_enable_device(const pci_location_t loc);
status_t pci_bus_mgr_enable_device(pci_location_t loc);
// read a list of up to 6 bars out of the device. each is marked with a valid bit
status_t pci_bus_mgr_read_bars(const pci_location_t loc, pci_bar_t bar[6]);
status_t pci_bus_mgr_read_bars(pci_location_t loc, pci_bar_t bar[6]);
// try to allocate one or more msi vectors for this device
status_t pci_bus_mgr_allocate_msi(const pci_location_t loc, size_t num_requested, uint *irqbase);
status_t pci_bus_mgr_allocate_msi(pci_location_t loc, size_t num_requested, uint *irqbase);
// allocate a regular irq for this device and return it in irqbase
status_t pci_bus_mgr_allocate_irq(const pci_location_t loc, uint *irqbase);
status_t pci_bus_mgr_allocate_irq(pci_location_t loc, uint *irqbase);
// return a pointer to a formatted string
const char *pci_loc_string(pci_location_t loc, char out_str[14]);

View File

@@ -8,13 +8,9 @@
*/
#pragma once
#include <inttypes.h>
#include <lk/compiler.h>
#include <dev/bus/pci.h>
#include "backend/bios32.h"
#include "backend/ecam.h"
#include "backend/type1.h"
#include "bus_mgr/bus_mgr.h"