Gazelle fixes, PCI changes and additions, MMIO changes, etc.

[joevt]

PCI bridge support, PCI config register reading/writing fixes, etc. I also have a pci-stubs branch which includes stubs for some additional PCI devices used for Open Firmware testing.

[maximumspatium]

this->sys_id = 0x10000000;

This doesn't look right to me. All HW ID registers should have 0x3 in the upper nibble to indicate the high-end RISC design center.

[maximumspatium]

My O'Hare doc says that its I/O space is 512KB. The O'Hare successor - Heathrow - also has the I/O space of the same size.

[joevt]

this->sys_id = 0x10000000;

This doesn't look right to me. All HW ID registers should have 0x3 in the upper nibble to indicate the high-end RISC design center.

If I use the 0x30040000 value, then my nvramrc patch to change the order of PCI slot probing doesn't execute.

Check the code in my 6500/G3 compare. Here is where it sets the gazelle flag and the cpu-id value and property:

0f8000000 constant cpubase
...
cpubase ar-rl@ dup d#16 >> 0ff and
case
    0 of true to ?gazelle endof
    1 of true to ?powerbook endof
endcase
value cpu-id
...
cpu-id encode-int " AAPL,cpu-id" property

The dump-device-tree shows AAPL,cpu-id is 0x10000000.

/
PROPERTIES:
name                    device-tree
model                   Power Macintosh
compatible              AAPL,e411
                        MacRISC
AAPL,cpu-id             10000000 
#address-cells          00000001 
#size-cells             00000001 
clock-frequency         02FAF080 

[joevt]

My O'Hare doc says that its I/O space is 512KB. The O'Hare successor - Heathrow - also has the I/O space of the same size.

The dump-device-tree shows 1MB in reg and assigned-addresses.

/bandit@F2000000/ohare@10
PROPERTIES:
name                    ohare
device_type             dbdma
model                   AAPL,343S0172
reg                     00008000 00000000 00000000  00000000 00000000 
                        02008010 00000000 F3000000  00000000 00100000 
assigned-addresses      82008010 00000000 F3000000  00000000 00100000 
ranges                  00000000  02008010 00000000 F3000000  00100000 
#address-cells          00000001 
#size-cells             00000001 
vendor-id               0000106B 
device-id               00000007 
revision-id             00000002 
class-code              00FF0000 
min-grant               00000000 
max-latency             00000000 
devsel-speed            00000001 

The 1MB value is hard coded in the ohare fcode.

0       0          value_990_8000    0        0 encode-pci-reg
iocbase 0  2000010 value_990_8000 or h#100000 0 encode-pci-reg encode+
"reg" property

I am waiting for verification of the BAR size using lspci for OpenFirmware

[joevt]

You're right. The BAR is 512K even though the fcode reserves 1MB.

00:10.0         [106B:0007] [FF0000] 6B10070016000082010000FF08200000000000F300000000000000000000000000000000000000000000000000000000000000000000000000000000000
0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
000000000000000000000000000000000000000000000000000000000000000000000
                                     00008010:F3000000.FFF80000

[joevt]

• Removed the ohare BAR change. 512K is correct.
• https://github.com/dingusdev/dingusppc/pull/40/commits/19b73cc8cece004dab4148cb755fde785c601a94 still exists at the top of this pull request.
• Corrected a note in https://github.com/dingusdev/dingusppc/pull/40/commits/c5c40baebd924f95bbc52365e844ffac62d78e4b regarding number of BARs in a type 2 header.

[maximumspatium]

Bandit/PSX ERS states that the bridge as the target of a configuration access expects its operand size to be 32-bits. I therefore suggest to remove calls to pci_cfg_rev_read from Bandit::pci_cfg_read and add assert(size == 4) instead. Bandit::pci_cfg_write to be changed accordingly.

[joevt]

The size and offset passed to bandit::read and bandit::write for the CONFIG_DATA memory range are to satisfy the PowerPC instruction that is reading bytes from CDATA or writing bytes to CDATA.

A real Power Mac 8600 can use the lbz instruction (size = 1) to read from CDATA+3 (offset = 3) after setting CADDR to config-space of bandit to get the single byte from config-space + 3 for bandit. So maybe bandit always does 32-bit accesses between CDATA and the pci device, but the end result is still a byte in the case of the lbz PowerPC instruction.

0 > dev pci1/@B  ok
0 > .properties 
name                    pci106b,1
vendor-id               0000106B 
device-id               00000001 
revision-id             00000003 
class-code              00060000 
reg                     00005800 00000000 00000000  00000000 00000000 
0 > dev pci1  ok
0 > 5800 config-l@ 8 u.r 0001106B ok
0 > 5800 4 bounds do i config-b@ 2 u.r loop 6B100100 ok

I suppose we can always read 32-bits and do the byte swizzling in bandit::read and bandit::write instead of in the individual PCI devices pci_cfg_read and pci_cfg_write methods, but then we lose the information about offset and size that we can use for debugging/logging in each of those methods.

[maximumspatium]

This ERS note explicitly states that Bandit only supports DWORD accesses to its own configuration space.

[maximumspatium]

I suppose we can always read 32-bits and do the byte swizzling in bandit::read and bandit::write instead of in the individual PCI devices pci_cfg_read and pci_cfg_write methods, but then we lose the information about offset and size that we can use for debugging/logging in each of those methods.

Swizzling and wrapping around of PCI accesses is a functionality provided by bridges, not devices. Doing those operations in the device sources produces duplicated code. We all know that code duplication is evil, especially in regards to bug fixes. Moreover, relying on specific code blocks when developing new device emulators is prone to bugs. I therefore vote for performing byte swizzling and wrapping around in the bridges in a centralized fashion, that is using as few code duplication as possible.

We can still pass the original offset as well as access size to the particular device method in a structure so they can be used for logging:

uint32_t Bandit::pci_cfg_read(uint32_t reg_offs, struct& AccessDetails details) {
...
   LOG_F(
        WARNING, "%s: reading from unimplemented config register @%02x.%c",
        this->name.c_str(), details.offset,
        details.size == 4 ? 'l' : details.size == 2 ? 'w' : details.size == 1 ? 'b' : '0' + details.size
    );
}

Normal execution won't use AccessDetails but error handling will do. Further, a macro can be provided for shortening this kind of logging code.

[maximumspatium]

Could you split this PR into two PRs, the one containing all commits excluding b809421, 4bb07f5, c5c40ba and 8f770ad, and the other containing only those? Having two separate PRs would be easier to handle. That first PR containing small fixes can be merged immediately. The above mentioned four commits mix bug fixes and new features we need to separate first. Thank you in advance!

[joevt]

I'll make the changes. I'm not sure what kind of facilities GitHub has for making multiple pull requests so I might do one at a time.

[maximumspatium]

Thanks! FYI, you can move a group of commits in a new branch and make a PR from there.

[maximumspatium]

How is the PR split going? Do you want me to give it a try?

[joevt]

Got distracted by another project. Gimme a couple days to do this PR.

[joevt]

This pull request is done and ready for review.

I removed these 4 commits: Fix PCI config r/w of byte and word and unaligned. Add 64-bit BAR support. Add PCI bridge and multi-function device support. Add a PCI bridge device. plus the Allow adding multiple of the same PCI device commit which depends on one of the 4 commits. Those commits will be in a new pull request in a couple days. In that pull request I will modify the Fix PCI config r/w of byte and word and unaligned commit so that the config read/write swizzle code in all PCI devices is moved to the host bridges and the config space log messages will be changed to a macro or something less wordy. The code for producing log messages for a PCI device will have to call the swizzle code to present the value being written to config space register correctly based on size and offset.

[maximumspatium]

In that pull request I will modify the Fix PCI config r/w of byte and word and unaligned commit so that the config read/write swizzle code in all PCI devices is moved to the host bridges and the config space log messages will be changed to a macro or something less wordy. The code for producing log messages for a PCI device will have to call the swizzle code to present the value being written to config space register correctly based on size and offset.

I suggest to submit four PRs instead of one containing five commits with non-trivial changes. We won't be charged per PR :)