Fix PCI config r/w of byte and word and unaligned

dingusppc could not read bytes from offset 1,2,3 or words from offset 2.
dingusppc did not read words from offset 1,3 and longs from offset 1,2,3 in the same way as a real Power Mac 8600 or B&W G3.
This commit fixes those issues.

- Added pci_cfg_rev_read. It takes a 32 bit value from offset 0 and returns a value of the specified size using bytes starting from the specified offset. Offsets 4,5, & 6 wrap around to 0,1, & 2 respectively. The result bytes are in flipped order as required by the pci_cfg_read method (so a value of 0x12345678 is returned as 0x78563412)
A real Power Mac 8600 might return a random byte for offset 4, 5, 6 for vci0 but usually not for pci1. A B&W G3 seems to always wrap around correctly. We won't read random bytes, and we won't read a default such as 00 or FF. We'll do the wrap around which makes the most sense because writing 0x12345678 to any offset and reading from the same offset should produce the value that was written.

- Added pci_cfg_rev_write. It takes a 32 bit value from offset 0, and modifies a specified number of bytes starting at a specified offset with the offset wrapping around to 0 if it exceeds 3. The modified bytes take their new values from the flipped bytes passed to pci_cfg_write. When size is 4, the original value is not used since all bytes will be modified.

Basically, those two functions handle all the sizes and all the offsets and replace calls to BYTESWAP_32, read_mem or read_mem_rev, and write_mem or write_mem_rev.
read_mem_rev, as it was used by pcidevice and some other places, could read beyond offset 3 if it were ever passed a reg_offs value that did not have offset as 0. Since the offset was always zero, it would always read the wrong byte or word if they were not at offset 0. Same for read_mem as used by mpc106.
write_mem_rev, as it was used by pcidevice and some other places, could write beyond offset 3 if it were ever passed a reg_offs value that did not have offset as 0. Since the offset was always zero, it would always write the wrong byte or word if they were not at offset 0. Same for write_mem as used by mpc106.

The PCI controllers (bandit, chaos, mpc106) need to encode the offset (0,1,2,3) into the reg_offs parameter passed to pci_cfg_read and pci_cfg_write so they can return or modify the correct bytes of the dword at reg_offs & 3.

The pci_cfg_read and pci_cfg_write methods extract the offset from reg_offs and report unaligned accesses.

pci_cfg_read uses pci_cfg_rev_read to read from the reg using the size and offset to determine which bytes to read.

pci_cfg_write uses pci_cfg_rev_write to write to the reg using the size and offset to determine which bytes to modify.

Other changes:
- for unimplemented config register reads and writes, bandit and ATIRage now includes offset and size (and value in the case of writes) in log warnings.
- for unimplemented config register reads and writes, pcidevice now includes offset in log warnings.
- pci_read and pci_write of mpc106 require an offset parameter since config_addr does not contain the offset (it is always a multiple of 4). The offset is included in the log warninings for non-existent PCI devices.
- ATIRage uses pci_cfg_rev_read and pci_cfg_rev_write which correctly places user_cfg at byte 0x40 instead of 0x43 and writes the correct byte depending on size and offset.

Notes:
- pci_cfg_read calls READ_DWORD_LE_A and pci_cfg_write calls WRITE_DWORD_LE_A. When reading or writing memory that is organized as little endian dwords, such as my_pci_cfg_hdr of mpc106, the function should explicitly state that it's little endian so that the emulator may be ported one day to a CPU architecture that is not little endian.
This commit is contained in:
joevt 2022-09-02 03:32:28 -07:00
parent d91e14abc6
commit b654424465
6 changed files with 198 additions and 68 deletions

View File

@ -81,15 +81,27 @@ uint32_t Bandit::pci_cfg_read(uint32_t reg_offs, uint32_t size)
if (reg_offs < 64) {
return PCIDevice::pci_cfg_read(reg_offs, size);
}
switch (reg_offs) {
case BANDIT_ADDR_MASK:
return BYTESWAP_32(this->addr_mask);
default:
LOG_F(WARNING, "%s: reading from unimplemented config register at 0x%X",
this->pci_name.c_str(), reg_offs);
uint32_t offset = reg_offs & 3;
reg_offs &= ~3;
if (~-size & offset) {
LOG_F(
WARNING, "%s: unaligned read @%02x.%c",
this->name.c_str(), reg_offs + offset,
size == 4 ? 'l' : size == 2 ? 'w' : size == 1 ? 'b' : '0' + size
);
}
if (reg_offs == BANDIT_ADDR_MASK) {
return pci_cfg_rev_read(this->addr_mask, offset, size);
}
LOG_F(
WARNING, "%s: reading from unimplemented config register @%02x.%c",
this->name.c_str(), reg_offs + offset,
size == 4 ? 'l' : size == 2 ? 'w' : size == 1 ? 'b' : '0' + size
);
return 0;
}
@ -100,15 +112,27 @@ void Bandit::pci_cfg_write(uint32_t reg_offs, uint32_t value, uint32_t size)
return;
}
switch (reg_offs) {
case BANDIT_ADDR_MASK:
this->addr_mask = BYTESWAP_32(value);
this->verbose_address_space();
break;
default:
LOG_F(WARNING, "%s: writing to unimplemented config register at 0x%X",
this->pci_name.c_str(), reg_offs);
uint32_t offset = reg_offs & 3;
reg_offs &= ~3;
if (~-size & offset) {
LOG_F(
WARNING, "%s: unaligned write @%02x.%c = %0*x",
this->name.c_str(), reg_offs + offset,
size == 4 ? 'l' : size == 2 ? 'w' : size == 1 ? 'b' : '0' + size, size * 2, flip_sized(value, size)
);
}
if (reg_offs == BANDIT_ADDR_MASK) {
this->addr_mask = pci_cfg_rev_write(this->addr_mask, offset, size, value);
this->verbose_address_space();
return;
}
LOG_F(
WARNING, "%s: writing to unimplemented config register @%02x.%c = %0*x",
this->name.c_str(), reg_offs + offset,
size == 4 ? 'l' : size == 2 ? 'w' : size == 1 ? 'b' : '0' + size, size * 2, flip_sized(value, size)
);
}
uint32_t Bandit::read(uint32_t rgn_start, uint32_t offset, int size)
@ -147,10 +171,10 @@ uint32_t Bandit::read(uint32_t rgn_start, uint32_t offset, int size)
}
if (idsel == BANDIT_ID_SEL) { // access to myself
result = this->pci_cfg_read(reg_offs, size);
result = this->pci_cfg_read(reg_offs + (offset & 3), size);
} else {
if (this->dev_map.count(idsel)) {
result = this->dev_map[idsel]->pci_cfg_read(reg_offs, size);
result = this->dev_map[idsel]->pci_cfg_read(reg_offs + (offset & 3), size);
} else {
dev_num = WHAT_BIT_SET(idsel) + 11;
LOG_F(
@ -214,12 +238,12 @@ void Bandit::write(uint32_t rgn_start, uint32_t offset, uint32_t value, int size
}
if (idsel == BANDIT_ID_SEL) { // access to myself
this->pci_cfg_write(reg_offs, value, size);
this->pci_cfg_write(reg_offs + (offset & 3), value, size);
return;
}
if (this->dev_map.count(idsel)) {
this->dev_map[idsel]->pci_cfg_write(reg_offs, value, size);
this->dev_map[idsel]->pci_cfg_write(reg_offs + (offset & 3), value, size);
} else {
dev_num = WHAT_BIT_SET(idsel) + 11;
LOG_F(
@ -320,7 +344,7 @@ uint32_t Chaos::read(uint32_t rgn_start, uint32_t offset, int size)
}
if (this->dev_map.count(idsel)) {
result = this->dev_map[idsel]->pci_cfg_read(reg_offs, size);
result = this->dev_map[idsel]->pci_cfg_read(reg_offs + (offset & 3), size);
} else {
dev_num = WHAT_BIT_SET(idsel) + 11;
LOG_F(
@ -376,7 +400,7 @@ void Chaos::write(uint32_t rgn_start, uint32_t offset, uint32_t value, int size)
}
if (this->dev_map.count(idsel)) {
this->dev_map[idsel]->pci_cfg_write(reg_offs, value, size);
this->dev_map[idsel]->pci_cfg_write(reg_offs + (offset & 3), value, size);
} else {
dev_num = WHAT_BIT_SET(idsel) + 11;
LOG_F(

View File

@ -52,6 +52,16 @@ uint32_t PCIDevice::pci_cfg_read(uint32_t reg_offs, uint32_t size)
{
uint32_t result;
uint32_t offset = reg_offs & 3;
reg_offs &= ~3;
if (~-size & offset) {
LOG_F(
WARNING, "%s: unaligned read @%02x.%c",
this->pci_name.c_str(), reg_offs + offset,
size == 4 ? 'l' : size == 2 ? 'w' : size == 1 ? 'b' : '0' + size
);
}
switch (reg_offs) {
case PCI_CFG_DEV_ID:
result = (this->device_id << 16) | (this->vendor_id);
@ -89,31 +99,32 @@ uint32_t PCIDevice::pci_cfg_read(uint32_t reg_offs, uint32_t size)
default:
LOG_F(
WARNING, "%s: attempt to read from reserved/unimplemented register @%02x.%c",
this->pci_name.c_str(), reg_offs,
this->pci_name.c_str(), reg_offs + offset,
size == 4 ? 'l' : size == 2 ? 'w' : size == 1 ? 'b' : '0' + size
);
return 0;
}
if (size == 4) {
return BYTESWAP_32(result);
} else {
return read_mem_rev(((uint8_t *)&result) + (reg_offs & 3), size);
}
return pci_cfg_rev_read(result, offset, size);
}
void PCIDevice::pci_cfg_write(uint32_t reg_offs, uint32_t value, uint32_t size)
{
uint32_t data;
if (size == 4) {
data = BYTESWAP_32(value);
} else {
// get current register content as DWORD and update it partially
data = BYTESWAP_32(this->pci_cfg_read(reg_offs, 4));
write_mem_rev(((uint8_t *)&data) + (reg_offs & 3), value, size);
uint32_t offset = reg_offs & 3;
reg_offs &= ~3;
if (~-size & offset) {
LOG_F(
WARNING, "%s: unaligned write @%02x.%c = %0*x",
this->pci_name.c_str(), reg_offs + offset,
size == 4 ? 'l' : size == 2 ? 'w' : size == 1 ? 'b' : '0' + size, size * 2, flip_sized(value, size)
);
}
// get current register content as DWORD and update it partially
data = pci_cfg_rev_write(size == 4 ? 0 : BYTESWAP_32(this->pci_cfg_read(reg_offs, 4)), offset, size, value);
switch (reg_offs) {
case PCI_CFG_STAT_CMD:
this->pci_wr_stat(data >> 16);

View File

@ -89,7 +89,7 @@ uint32_t MPC106::read(uint32_t rgn_start, uint32_t offset, int size) {
} else {
if (offset >= 0x200000) {
if (this->config_addr & 0x80) // process only if bit E (enable) is set
return pci_read(size);
return pci_read(offset & 3, size);
}
}
@ -113,39 +113,39 @@ void MPC106::write(uint32_t rgn_start, uint32_t offset, uint32_t value, int size
this->config_addr = value;
} else {
if (this->config_addr & 0x80) // process only if bit E (enable) is set
return pci_write(value, size);
return pci_write(offset & 3, value, size);
}
}
}
uint32_t MPC106::pci_read(uint32_t size) {
uint32_t MPC106::pci_read(uint32_t offset, uint32_t size) {
int bus_num, dev_num, fun_num, reg_offs;
bus_num = (this->config_addr >> 8) & 0xFF;
bus_num = (this->config_addr >> 8) & 0xFF;
dev_num = (this->config_addr >> 19) & 0x1F;
fun_num = (this->config_addr >> 16) & 0x07;
reg_offs = (this->config_addr >> 24) & 0xFC;
if (bus_num) {
LOG_F(
ERROR,
"%s err: read attempt from non-local PCI bus, config_addr = %x %02x:%02x.%x @%02x.%c",
this->name.c_str(), this->config_addr, bus_num, dev_num, fun_num, reg_offs,
size == 4 ? 'l' : size == 2 ? 'w' : size == 1 ? 'b' : '0' + size
);
LOG_F(
ERROR,
"%s err: read attempt from non-local PCI bus, config_addr = %x, offset = %x %02x:%02x.%x @%02x.%c",
this->name.c_str(), this->config_addr, offset, bus_num, dev_num, fun_num, reg_offs,
size == 4 ? 'l' : size == 2 ? 'w' : size == 1 ? 'b' : '0' + size
);
return 0xFFFFFFFFUL; // PCI spec §6.1
}
if (dev_num == 0 && fun_num == 0) { // dev_num 0 is assigned to myself
return this->pci_cfg_read(reg_offs, size);
return this->pci_cfg_read(reg_offs + offset, size);
} else {
if (this->dev_map.count(dev_num)) {
return this->dev_map[dev_num]->pci_cfg_read(reg_offs, size);
return this->dev_map[dev_num]->pci_cfg_read(reg_offs + offset, size);
} else {
LOG_F(
ERROR,
"%s err: read attempt from non-existing PCI device %02x:%02x.%x @%02x.%c",
this->name.c_str(), bus_num, dev_num, fun_num, reg_offs,
this->name.c_str(), bus_num, dev_num, fun_num, reg_offs + offset,
size == 4 ? 'l' : size == 2 ? 'w' : size == 1 ? 'b' : '0' + size
);
return 0xFFFFFFFFUL; // PCI spec §6.1
@ -155,10 +155,10 @@ uint32_t MPC106::pci_read(uint32_t size) {
return 0;
}
void MPC106::pci_write(uint32_t value, uint32_t size) {
void MPC106::pci_write(uint32_t offset, uint32_t value, uint32_t size) {
int bus_num, dev_num, fun_num, reg_offs;
bus_num = (this->config_addr >> 8) & 0xFF;
bus_num = (this->config_addr >> 8) & 0xFF;
dev_num = (this->config_addr >> 19) & 0x1F;
fun_num = (this->config_addr >> 16) & 0x07;
reg_offs = (this->config_addr >> 24) & 0xFC;
@ -166,24 +166,24 @@ void MPC106::pci_write(uint32_t value, uint32_t size) {
if (bus_num) {
LOG_F(
ERROR,
"%s err: write attempt to non-local PCI bus, config_addr = %x %02x:%02x.%x @%02x.%c = %0*x",
this->name.c_str(), this->config_addr, bus_num, dev_num, fun_num, reg_offs,
size == 4 ? 'l' : size == 2 ? 'w' : size == 1 ? 'b' : '0' + size,
"%s err: write attempt to non-local PCI bus, config_addr = %x, offset = %x %02x:%02x.%x @%02x.%c = %0*x",
this->name.c_str(), this->config_addr, offset, bus_num, dev_num, fun_num, reg_offs + offset,
size == 4 ? 'l' : size == 2 ? 'w' : size == 1 ? 'b' : '0' + size,
size * 2, flip_sized(value, size)
);
return;
}
if (dev_num == 0 && fun_num == 0) { // dev_num 0 is assigned to myself
this->pci_cfg_write(reg_offs, value, size);
this->pci_cfg_write(reg_offs + offset, value, size);
} else {
if (this->dev_map.count(dev_num)) {
this->dev_map[dev_num]->pci_cfg_write(reg_offs, value, size);
this->dev_map[dev_num]->pci_cfg_write(reg_offs + offset, value, size);
} else {
LOG_F(
ERROR,
"%s err: write attempt to non-existing PCI device %02x:%02x.%x @%02x.%c = %0*x",
this->name.c_str(), bus_num, dev_num, fun_num, reg_offs,
this->name.c_str(), bus_num, dev_num, fun_num, reg_offs + offset,
size == 4 ? 'l' : size == 2 ? 'w' : size == 1 ? 'b' : '0' + size,
size * 2, flip_sized(value, size)
);
@ -200,7 +200,17 @@ uint32_t MPC106::pci_cfg_read(uint32_t reg_offs, uint32_t size) {
return PCIDevice::pci_cfg_read(reg_offs, size);
}
return read_mem(&this->my_pci_cfg_hdr[reg_offs], size);
uint32_t offset = reg_offs & 3;
reg_offs &= ~3;
if (~-size & offset) {
LOG_F(
WARNING, "%s: unaligned read @%02x.%c",
this->pci_name.c_str(), reg_offs + offset,
size == 4 ? 'l' : size == 2 ? 'w' : size == 1 ? 'b' : '0' + size
);
}
return pci_cfg_rev_read(READ_DWORD_LE_A(&this->my_pci_cfg_hdr[reg_offs]), offset, size);
}
void MPC106::pci_cfg_write(uint32_t reg_offs, uint32_t value, uint32_t size) {
@ -213,9 +223,20 @@ void MPC106::pci_cfg_write(uint32_t reg_offs, uint32_t value, uint32_t size) {
return;
}
uint32_t offset = reg_offs & 3;
reg_offs &= ~3;
if (~-size & offset) {
LOG_F(
WARNING, "%s: unaligned write @%02x.%c = %0*x",
this->pci_name.c_str(), reg_offs + offset,
size == 4 ? 'l' : size == 2 ? 'w' : size == 1 ? 'b' : '0' + size, size * 2, flip_sized(value, size)
);
}
// FIXME: implement write-protection for read-only registers
write_mem(&this->my_pci_cfg_hdr[reg_offs], value, size);
uint32_t *addr = (uint32_t *)&this->my_pci_cfg_hdr[reg_offs];
WRITE_DWORD_LE_A(addr, pci_cfg_rev_write(READ_DWORD_LE_A(addr), offset, size, value));
if (this->my_pci_cfg_hdr[0xF2] & 8) {
#ifdef MPC106_DEBUG

View File

@ -59,8 +59,8 @@ public:
protected:
/* PCI access */
uint32_t pci_read(uint32_t size);
void pci_write(uint32_t value, uint32_t size);
uint32_t pci_read(uint32_t offset, uint32_t size);
void pci_write(uint32_t offset, uint32_t value, uint32_t size);
/* my own PCI configuration registers access */
uint32_t pci_cfg_read(uint32_t reg_offs, uint32_t size);

View File

@ -167,11 +167,25 @@ uint32_t ATIRage::pci_cfg_read(uint32_t reg_offs, uint32_t size)
return PCIDevice::pci_cfg_read(reg_offs, size);
}
uint32_t offset = reg_offs & 3;
reg_offs &= ~3;
if (~-size & offset) {
LOG_F(
WARNING, "%s: unaligned read @%02x.%c",
this->pci_name.c_str(), reg_offs + offset,
size == 4 ? 'l' : size == 2 ? 'w' : size == 1 ? 'b' : '0' + size
);
}
switch (reg_offs) {
case 0x40:
return this->user_cfg;
return pci_cfg_rev_read(this->user_cfg, offset, size);
default:
LOG_F(WARNING, "ATIRage: reading from unimplemented config register at 0x%X", reg_offs);
LOG_F(
WARNING, "%s: reading from unimplemented config register @%02x.%c",
this->pci_name.c_str(), reg_offs + offset,
size == 4 ? 'l' : size == 2 ? 'w' : size == 1 ? 'b' : '0' + size
);
}
return 0;
@ -181,14 +195,29 @@ void ATIRage::pci_cfg_write(uint32_t reg_offs, uint32_t value, uint32_t size)
{
if (reg_offs < 64) {
PCIDevice::pci_cfg_write(reg_offs, value, size);
} else {
switch (reg_offs) {
case 0x40:
this->user_cfg = value;
break;
default:
LOG_F(WARNING, "ATIRage: writing to unimplemented config register at 0x%X", reg_offs);
}
return;
}
uint32_t offset = reg_offs & 3;
reg_offs &= ~3;
if (~-size & offset) {
LOG_F(
WARNING, "%s: unaligned write @%02x.%c = %0*x",
this->pci_name.c_str(), reg_offs + offset,
size == 4 ? 'l' : size == 2 ? 'w' : size == 1 ? 'b' : '0' + size, size * 2, flip_sized(value, size)
);
}
switch (reg_offs) {
case 0x40:
this->user_cfg = pci_cfg_rev_write(this->user_cfg, offset, size, value);
break;
default:
LOG_F(
WARNING, "%s: writing to unimplemented config register @%02x.%c = %0*x",
this->pci_name.c_str(), reg_offs + offset,
size == 4 ? 'l' : size == 2 ? 'w' : size == 1 ? 'b' : '0' + size, size * 2, flip_sized(value, size)
);
}
}

View File

@ -177,6 +177,51 @@ inline uint32_t read_mem_rev(const uint8_t* buf, uint32_t size) {
}
}
/* value is dword from PCI config. MSB..LSB of value is stored in PCI config as 0:LSB..3:MSB.
result is part of value at byte offset from LSB and size bytes (with wrap around) and flipped as required for pci_cfg_read result. */
inline uint32_t pci_cfg_rev_read(uint32_t value, uint32_t offset, uint32_t size) {
switch (size << 2 | offset) {
case 0x04: return value & 0xff; // 0
case 0x05: return (value >> 8) & 0xff; // 1
case 0x06: return (value >> 16) & 0xff; // 2
case 0x07: return (value >> 24) & 0xff; // 3
case 0x08: return ((value & 0xff) << 8) | ((value >> 8) & 0xff); // 0 1
case 0x09: return ( value & 0xff00) | ((value >> 16) & 0xff); // 1 2
case 0x0a: return ((value >> 8) & 0xff00) | ((value >> 24) & 0xff); // 2 3
case 0x0b: return ((value >> 16) & 0xff00) | ( value & 0xff); // 3 0
case 0x10: return ((value & 0xff) << 24) | ((value & 0xff00) << 8) | ((value >> 8) & 0xff00) | ((value >> 24) & 0xff); // 0 1 2 3
case 0x11: return ((value & 0xff00) << 16) | ( value & 0xff0000) | ((value >> 16) & 0xff00) | ( value & 0xff); // 1 2 3 0
case 0x12: return ((value & 0xff0000) << 8) | ((value >> 8) & 0xff0000) | ((value & 0xff) << 8) | ((value >> 8) & 0xff); // 2 3 0 1
case 0x13: return ( value & 0xff000000) | ((value & 0xff) << 16) | ( value & 0xff00) | ((value >> 16) & 0xff); // 3 0 1 2
default: LOG_F(ERROR, "pci_cfg_rev: invalid offset %d for size %d!", offset, size); return 0xffffffff;
}
}
/* value is dword from PCI config. MSB..LSB of value (3.2.1.0) is stored in PCI config as 0:LSB..3:MSB.
data is flipped bytes (d0.d1.d2.d3, as passed to pci_cfg_write) to be merged into value.
result is part of value at byte offset from LSB and size bytes (with wrap around) modified by data. */
inline uint32_t pci_cfg_rev_write(uint32_t value, uint32_t offset, uint32_t size, uint32_t data) {
switch (size << 2 | offset) {
case 0x04: return (value & 0xffffff00) | (data & 0xff); // 3 2 1 d0
case 0x05: return (value & 0xffff00ff) | ((data & 0xff) << 8); // 3 2 d0 0
case 0x06: return (value & 0xff00ffff) | ((data & 0xff) << 16); // 3 d0 1 0
case 0x07: return (value & 0x00ffffff) | ((data & 0xff) << 24); // d0 2 1 0
case 0x08: return (value & 0xffff0000) | ((data >> 8) & 0xff) | ((data & 0xff) << 8); // 3 2 d1 d0
case 0x09: return (value & 0xff0000ff) | (data & 0xff00) | ((data & 0xff) << 16); // 3 d1 d0 0
case 0x0a: return (value & 0x0000ffff) | ((data & 0xff00) << 8) | ((data & 0xff) << 24); // d1 d0 1 0
case 0x0b: return (value & 0x00ffff00) | ((data & 0xff00) << 16) | (data & 0xff); // d0 2 1 d1
case 0x10: return ((data & 0xff) << 24) | ((data & 0xff00) << 8) | ((data >> 8) & 0xff00) | ((data >> 24) & 0xff); // d3 d2 d1 d0
case 0x11: return ((data & 0xff00) << 16) | ( data & 0xff0000) | ((data >> 16) & 0xff00) | ( data & 0xff); // d2 d1 d0 d3
case 0x12: return ((data & 0xff0000) << 8) | ((data >> 8) & 0xff0000) | ((data & 0xff) << 8) | ((data >> 8) & 0xff); // d1 d0 d3 d2
case 0x13: return ( data & 0xff000000) | ((data & 0xff) << 16) | ( data & 0xff00) | ((data >> 16) & 0xff); // d0 d3 d2 d1
default: LOG_F(ERROR, "pci_cfg_rev: invalid offset %d for size %d!", offset, size); return 0xffffffff;
}
}
inline uint32_t flip_sized(uint32_t value, uint32_t size) {
switch (size) {
case 1: return value;