From fba2ff4231345fc544da8c0ef4f6155ae1e907ff Mon Sep 17 00:00:00 2001 From: joevt Date: Thu, 2 Feb 2023 02:40:35 -0800 Subject: [PATCH 1/2] Corrections for refactors. - Macros need parenthesis to enforce operation order when expanded. - Fix device and register numbers in log messages. - Unmapped I/O space reads don't necessarily return 0xffffffff. That's only for config space reads. Just return 0. Unhandled I/O space read should probably cause a memory check (TEA - Transfer Error Acknowledge) exception as it does on a Power Mac 8600. --- devices/common/pci/bandit.cpp | 14 +++++++------- devices/common/pci/bandit.h | 8 ++++---- devices/memctrl/mpc106.cpp | 13 +++++++------ 3 files changed, 18 insertions(+), 17 deletions(-) diff --git a/devices/common/pci/bandit.cpp b/devices/common/pci/bandit.cpp index d8ec036..675faa9 100644 --- a/devices/common/pci/bandit.cpp +++ b/devices/common/pci/bandit.cpp @@ -146,7 +146,7 @@ uint32_t BanditHost::read(uint32_t rgn_start, uint32_t offset, int size) if (this->config_addr & BANDIT_CAR_TYPE) { // type 1 configuration command LOG_F( WARNING, "%s: read config cycle type 1 not supported yet %02x:%02x.%x @%02x", - this->name.c_str(), BUS_NUM(), DEV_NUM(), FUN_NUM(), offset & 0xFCU + this->name.c_str(), BUS_NUM(), DEV_NUM(), FUN_NUM(), REG_NUM() + (offset & 3) ); return 0xFFFFFFFFUL; // PCI spec §6.1 } @@ -170,7 +170,7 @@ uint32_t BanditHost::read(uint32_t rgn_start, uint32_t offset, int size) } else { LOG_F( ERROR, "%s err: read attempt from non-existing PCI device ??:%02x.%x @%02x", - this->name.c_str(), WHAT_BIT_SET(idsel), FUN_NUM(), offset + this->name.c_str(), WHAT_BIT_SET(idsel) + 11, FUN_NUM(), REG_NUM() + (offset & 3) ); return 0xFFFFFFFFUL; // PCI spec §6.1 } @@ -188,9 +188,9 @@ uint32_t BanditHost::read(uint32_t rgn_start, uint32_t offset, int size) } LOG_F(ERROR, "%s: attempt to read from unmapped PCI I/O space, offset=0x%X", this->name.c_str(), offset); + // FIXME: add machine check exception (DEFAULT CATCH!, code=FFF00200) + return 0; } - - return 0xFFFFFFFFUL; // PCI spec §6.1 } void BanditHost::write(uint32_t rgn_start, uint32_t offset, uint32_t value, int size) @@ -202,7 +202,7 @@ void BanditHost::write(uint32_t rgn_start, uint32_t offset, uint32_t value, int if (this->config_addr & BANDIT_CAR_TYPE) { // type 1 configuration command LOG_F( WARNING, "%s: write config cycle type 1 not supported yet %02x:%02x.%x @%02x", - this->name.c_str(), BUS_NUM(), DEV_NUM(), FUN_NUM(), offset & 0xFCU + this->name.c_str(), BUS_NUM(), DEV_NUM(), FUN_NUM(), REG_NUM() + (offset & 3) ); return; } @@ -231,7 +231,7 @@ void BanditHost::write(uint32_t rgn_start, uint32_t offset, uint32_t value, int } else { LOG_F( ERROR, "%s err: write attempt to non-existing PCI device ??:%02x.%x @%02x", - this->name.c_str(), WHAT_BIT_SET(idsel), FUN_NUM(), offset + this->name.c_str(), WHAT_BIT_SET(idsel) + 11, FUN_NUM(), REG_NUM() + (offset & 3) ); } break; @@ -273,7 +273,7 @@ Bandit::Bandit(int bridge_num, std::string name, int dev_id, int rev) mem_ctrl->add_mmio_region(base_addr, 0x01000000, this); // connnect Bandit PCI device - this->my_pci_device = unique_ptr( + this->my_pci_device = unique_ptr( new BanditPciDevice(bridge_num, name, dev_id, rev) ); this->pci_register_device(1, this->my_pci_device.get()); diff --git a/devices/common/pci/bandit.h b/devices/common/pci/bandit.h index 5f44630..007442e 100644 --- a/devices/common/pci/bandit.h +++ b/devices/common/pci/bandit.h @@ -49,10 +49,10 @@ along with this program. If not, see . #define BANDIT_CAR_TYPE (1 << 0) // Bandit config address type bit /* Convenient macros for parsing CONFIG_ADDR fields. */ -#define BUS_NUM() (this->config_addr >> 16) & 0xFFU -#define DEV_NUM() (this->config_addr >> 11) & 0x1FU -#define FUN_NUM() (this->config_addr >> 8) & 0x07U -#define REG_NUM() (this->config_addr ) & 0xFCU +#define BUS_NUM() ((this->config_addr >> 16) & 0xFFU) +#define DEV_NUM() ((this->config_addr >> 11) & 0x1FU) +#define FUN_NUM() ((this->config_addr >> 8) & 0x07U) +#define REG_NUM() ((this->config_addr ) & 0xFCU) /** Bandit specific configuration registers. */ enum { diff --git a/devices/memctrl/mpc106.cpp b/devices/memctrl/mpc106.cpp index e3e7875..ff28438 100644 --- a/devices/memctrl/mpc106.cpp +++ b/devices/memctrl/mpc106.cpp @@ -86,6 +86,7 @@ uint32_t MPC106::read(uint32_t rgn_start, uint32_t offset, int size) { } } LOG_F(ERROR, "Attempt to read from unmapped PCI I/O space, offset=0x%X", offset); + // FIXME: add machine check exception (DEFAULT CATCH!, code=FFF00200) } else { if (offset >= 0x200000) { if (this->config_addr & 0x80) // process only if bit E (enable) is set @@ -125,8 +126,8 @@ uint32_t MPC106::pci_read(uint32_t offset, uint32_t size) { int reg_offs = (this->config_addr >> 24) & 0xFC; if (bus_num) { - LOG_F(ERROR, "%s: read attempt from non-local PCI bus, %02x:%02x.%x @%02x", - this->name.c_str(), bus_num, dev_num, fun_num, offset & 0xFCU); + LOG_F(ERROR, "%s: read attempt from non-local PCI bus, %02x:%02x.%x @%02x", + this->name.c_str(), bus_num, dev_num, fun_num, reg_offs + (offset & 3)); return 0xFFFFFFFFUL; // PCI spec §6.1 } @@ -139,7 +140,7 @@ uint32_t MPC106::pci_read(uint32_t offset, uint32_t size) { return pci_conv_rd_data(result, details); } else { LOG_F(ERROR, "%s: read attempt from non-existing PCI device ??:%02x.%x @%02x", - this->name.c_str(), dev_num, fun_num, offset); + this->name.c_str(), dev_num, fun_num, reg_offs + (offset & 3)); } return 0xFFFFFFFFUL; // PCI spec §6.1 @@ -152,8 +153,8 @@ void MPC106::pci_write(uint32_t offset, uint32_t value, uint32_t size) { int reg_offs = (this->config_addr >> 24) & 0xFC; if (bus_num) { - LOG_F(ERROR, "%s: write attempt to non-local PCI bus, %02x:%02x.%x @%02x", - this->name.c_str(), bus_num, dev_num, fun_num, offset & 0xFCU); + LOG_F(ERROR, "%s: write attempt to non-local PCI bus, %02x:%02x.%x @%02x", + this->name.c_str(), bus_num, dev_num, fun_num, reg_offs + (offset & 3)); return; } @@ -172,7 +173,7 @@ void MPC106::pci_write(uint32_t offset, uint32_t value, uint32_t size) { } } else { LOG_F(ERROR, "%s: write attempt to non-existing PCI device ??:%02x.%x @%02x", - this->name.c_str(), dev_num, fun_num, offset); + this->name.c_str(), dev_num, fun_num, reg_offs + (offset & 3)); } } From 2a64f547cc9986500a4c40f02041f0278cfa34df Mon Sep 17 00:00:00 2001 From: joevt Date: Sun, 11 Dec 2022 13:37:14 -0800 Subject: [PATCH 2/2] Add 64-bit BAR support. While dingusppc only emulates 32-bit Macs (for now), it is possible for a 32-bit Power Mac to use a PCIe card that has 64-bit BARs. finish_config_bars is added to scan the cfg values of the BARs and determine their type. The type is stored separately so that it does not need to be determined again. The type can be I/O (16 or 32 bit) or Mem (20 or 32 or 64 bit). A 64 bit bar is two BARs, the second contains the most significant 32 bits. set_bar_value uses the stored type instead of trying to determine the type itself. It is always called even when the firmware is doing sizing. For sizing, It does the job of setting the bar value so do_bar_sizing is now just a stub. Every PCIDevice that has a BAR needs to call finish_config_bars after setting up the cfg values just as they need to setup the cfg values. Since they need to do both, maybe the cfg values should be arguments of finish_config_bars, then finish_config_bars() should be renamed config_bars(). --- devices/common/pci/pcidevice.cpp | 74 ++++++++++++++++++++++++++------ devices/common/pci/pcidevice.h | 12 ++++++ devices/ioctrl/grandcentral.cpp | 1 + devices/ioctrl/heathrow.cpp | 1 + devices/ioctrl/ohare.cpp | 1 + devices/video/atimach64gx.cpp | 1 + devices/video/atirage.cpp | 1 + devices/video/control.cpp | 1 + 8 files changed, 79 insertions(+), 13 deletions(-) diff --git a/devices/common/pci/pcidevice.cpp b/devices/common/pci/pcidevice.cpp index 615487c..016f891 100644 --- a/devices/common/pci/pcidevice.cpp +++ b/devices/common/pci/pcidevice.cpp @@ -100,11 +100,7 @@ void PCIDevice::pci_cfg_write(uint32_t reg_offs, uint32_t value, AccessDetails & case PCI_CFG_BAR3: case PCI_CFG_BAR4: case PCI_CFG_BAR5: - if (value == 0xFFFFFFFFUL) { - this->do_bar_sizing((reg_offs - 0x10) >> 2); - } else { - this->set_bar_value((reg_offs - 0x10) >> 2, value); - } + this->set_bar_value((reg_offs - 0x10) >> 2, value); break; case PCI_CFG_ROM_BAR: if ((value & this->exp_bar_cfg) == this->exp_bar_cfg) { @@ -207,23 +203,75 @@ int PCIDevice::attach_exp_rom_image(const std::string img_path) void PCIDevice::do_bar_sizing(int bar_num) { - this->bars[bar_num] = this->bars_cfg[bar_num]; } void PCIDevice::set_bar_value(int bar_num, uint32_t value) { uint32_t bar_cfg = this->bars_cfg[bar_num]; - if (bar_cfg & 1) { - this->bars[bar_num] = (value & 0xFFFFFFFCUL) | (bar_cfg & 3); - } else { - if (bar_cfg & 6) { - ABORT_F("Invalid or unsupported PCI space type: %d", (bar_cfg >> 1) & 3); - } - this->bars[bar_num] = (value & 0xFFFFFFF0UL) | (bar_cfg & 0xF); + switch (bars_typ[bar_num]) { + case BAR_Unused: + return; + + case BAR_IO_16Bit: + case BAR_IO_32Bit: + this->bars[bar_num] = (value & bar_cfg & ~3) | (bar_cfg & 3); + break; + + case BAR_MEM_20Bit: + case BAR_MEM_32Bit: + case BAR_MEM_64Bit: + this->bars[bar_num] = (value & bar_cfg & ~0xF) | (bar_cfg & 0xF); + break; + + case BAR_MEM_64BitHi: + this->bars[bar_num] = (value & bar_cfg); + break; } + + if (value == 0xFFFFFFFFUL) { + do_bar_sizing(bar_num); + return; + } + this->pci_notify_bar_change(bar_num); } +void PCIDevice::finish_config_bars() +{ + for (int bar_num = 0; bar_num < 6; bar_num++) { + uint32_t bar_cfg = this->bars_cfg[bar_num]; + if (bar_cfg & 1) { + bars_typ[bar_num] = (bar_cfg & 0xffff0000) ? BAR_IO_32Bit : BAR_IO_16Bit; + } + else if (bar_cfg != 0) { + int pci_space_type = (bar_cfg >> 1) & 3; + switch (pci_space_type) { + case 0: + bars_typ[bar_num] = BAR_MEM_32Bit; + break; + case 1: + bars_typ[bar_num] = BAR_MEM_20Bit; + break; + case 2: + if (bar_num > 4) { + ABORT_F("%s: BAR %d cannot be 64-bit", this->pci_name.c_str(), bar_num); + } + else if (this->bars_cfg[bar_num+1] == 0) { + ABORT_F("%s: 64-bit BAR %d has zero for upper 32 bits", this->pci_name.c_str(), bar_num); + } + else { + bars_typ[bar_num++] = BAR_MEM_64Bit; + bars_typ[bar_num] = BAR_MEM_64BitHi; + } + break; + case 3: + ABORT_F("%s: invalid or unsupported PCI space type %d for BAR %d", this->pci_name.c_str(), pci_space_type, bar_num); + break; + } // switch pci_space_type + } // if BAR_MEM + } // for bar_num +} + void PCIDevice::map_exp_rom_mem() { uint32_t rom_addr, rom_size; diff --git a/devices/common/pci/pcidevice.h b/devices/common/pci/pcidevice.h index 147a59c..c215ea2 100644 --- a/devices/common/pci/pcidevice.h +++ b/devices/common/pci/pcidevice.h @@ -58,6 +58,16 @@ enum { PCI_VENDOR_NVIDIA = 0x10DE, }; +/** PCI BAR types */ +enum PCIBarType { + BAR_Unused, + BAR_IO_16Bit, + BAR_IO_32Bit, + BAR_MEM_20Bit, // < 1M + BAR_MEM_32Bit, + BAR_MEM_64Bit, + BAR_MEM_64BitHi, +}; class PCIDevice : public MMIODevice { public: @@ -108,6 +118,7 @@ public: protected: void do_bar_sizing(int bar_num); void set_bar_value(int bar_num, uint32_t value); + void finish_config_bars(); void map_exp_rom_mem(); std::string pci_name; // human-readable device name @@ -132,6 +143,7 @@ protected: uint32_t bars[6] = { 0 }; // base address registers uint32_t bars_cfg[6] = { 0 }; // configuration values for base address registers + PCIBarType bars_typ[6] = { BAR_Unused }; // types for base address registers uint32_t exp_bar_cfg = 0; // expansion ROM configuration uint32_t exp_rom_bar = 0; // expansion ROM base address register diff --git a/devices/ioctrl/grandcentral.cpp b/devices/ioctrl/grandcentral.cpp index 56309d8..18fc5f2 100644 --- a/devices/ioctrl/grandcentral.cpp +++ b/devices/ioctrl/grandcentral.cpp @@ -43,6 +43,7 @@ GrandCentral::GrandCentral() : PCIDevice("mac-io/grandcentral"), InterruptCtrl() this->class_rev = 0xFF000002; this->cache_ln_sz = 8; this->bars_cfg[0] = 0xFFFE0000UL; // declare 128Kb of memory-mapped I/O space + this->finish_config_bars(); this->pci_notify_bar_change = [this](int bar_num) { this->notify_bar_change(bar_num); diff --git a/devices/ioctrl/heathrow.cpp b/devices/ioctrl/heathrow.cpp index 782af0d..9331fb0 100644 --- a/devices/ioctrl/heathrow.cpp +++ b/devices/ioctrl/heathrow.cpp @@ -55,6 +55,7 @@ HeathrowIC::HeathrowIC() : PCIDevice("mac-io/heathrow"), InterruptCtrl() this->cache_ln_sz = 8; this->lat_timer = 0x40; this->bars_cfg[0] = 0xFFF80000UL; // declare 512Kb of memory-mapped I/O space + this->finish_config_bars(); this->pci_notify_bar_change = [this](int bar_num) { this->notify_bar_change(bar_num); diff --git a/devices/ioctrl/ohare.cpp b/devices/ioctrl/ohare.cpp index 4cf8df6..50b1d03 100644 --- a/devices/ioctrl/ohare.cpp +++ b/devices/ioctrl/ohare.cpp @@ -37,6 +37,7 @@ OHare::OHare() : PCIDevice("mac-io/ohare"), InterruptCtrl() this->class_rev = 0xFF000001; this->cache_ln_sz = 8; this->bars_cfg[0] = 0xFFF80000UL; // declare 512Kb of memory-mapped I/O space + this->finish_config_bars(); this->pci_notify_bar_change = [this](int bar_num) { this->notify_bar_change(bar_num); diff --git a/devices/video/atimach64gx.cpp b/devices/video/atimach64gx.cpp index b903fcc..fa975f2 100644 --- a/devices/video/atimach64gx.cpp +++ b/devices/video/atimach64gx.cpp @@ -45,6 +45,7 @@ AtiMach64Gx::AtiMach64Gx() this->device_id = ATI_MACH64_GX_DEV_ID; this->class_rev = (0x030000 << 8) | 3; this->bars_cfg[0] = 0xFF000000UL; // declare main aperture (16MB) + this->finish_config_bars(); this->pci_notify_bar_change = [this](int bar_num) { this->notify_bar_change(bar_num); diff --git a/devices/video/atirage.cpp b/devices/video/atirage.cpp index c88bd64..f24c2c9 100644 --- a/devices/video/atirage.cpp +++ b/devices/video/atirage.cpp @@ -127,6 +127,7 @@ ATIRage::ATIRage(uint16_t dev_id) this->bars_cfg[0] = 0xFF000000UL; // declare main aperture (16MB) this->bars_cfg[1] = 0xFFFFFF01UL; // declare I/O region (256 bytes) this->bars_cfg[2] = 0xFFFFF000UL; // declare register aperture (4KB) + this->finish_config_bars(); this->pci_notify_bar_change = [this](int bar_num) { this->notify_bar_change(bar_num); diff --git a/devices/video/control.cpp b/devices/video/control.cpp index cfbcedc..30d26cd 100644 --- a/devices/video/control.cpp +++ b/devices/video/control.cpp @@ -62,6 +62,7 @@ ControlVideo::ControlVideo() this->bars_cfg[0] = 0xFFFFFFFFUL; // I/O region (4 bytes but it's weird because bit 1 is set) this->bars_cfg[1] = 0xFFFFF000UL; // base address for the HW registers (4KB) this->bars_cfg[2] = 0xFC000000UL; // base address for the VRAM (64MB) + this->finish_config_bars(); this->pci_notify_bar_change = [this](int bar_num) { this->notify_bar_change(bar_num);