dingusdev / dingusppc

An experimental emulator
GNU General Public License v3.0
200 stars 21 forks source link

more fixes #30

Closed joevt closed 1 year ago

joevt commented 1 year ago

Fixes for stuff I found while trying to make Nvidia 6200 GPU emulation for Open Firmware 2.4 and while testing my lspci for Open Firmware command.

maximumspatium commented 1 year ago

PCI Expansion ROM size should be multiple of 2 commit can be simplified as follows:

uint32_t exp_rom_image_size;

...
// determine image size
img_file.seekg(0, std::ios::end);
exp_rom_image_size = img_file.tellg();

// ensure ROM size is aligned on a 2KB boundary
this->exp_rom_size = (exp_rom_image_size + 0x7FFU) & 0xFFFFF800UL;

// ROM image ok - go ahead and load it
this->exp_rom_data = std::unique_ptr<uint8_t[]> (new uint8_t[this->exp_rom_size]);
std::memset(this->exp_rom_data, 0xff, this->exp_rom_size);
img_file.seekg(0, std::ios::beg);
img_file.read((char *)this->exp_rom_data.get(), exp_rom_image_size);
joevt commented 1 year ago

The commit comment should say "power of 2"

maximumspatium commented 1 year ago

I suggest to rename all reg_start params to rgn_start to avoid ambiguity. The reg abbreviation is associated mostly with register, not region. This param actually indicates the base address of a memory region so rgn_start is more appropriate.

maximumspatium commented 1 year ago

Static tables should not be in headers...

joevt commented 1 year ago

I'm ok with either. reg could refer to base address register, but then again, not all regions are pointed to be a base address register.

joevt commented 1 year ago

Static tables should not be in headers...

I thought it looked weird but didn't get a compiler error. I'll fix it.

maximumspatium commented 1 year ago

The point is that the emulation does not need to be limited to 5 slots.

Well, a real Beige G3 desktop has only three physical PCI slots. Two internal PCI slots are hardwired to the build-in GPU and the PERCH slot. device X @X comments add no value IMHO and can be deleted. I'm not sure whether the source code is the proper place for documenting HW. Anything that isn't obvious from code flow should go to some kind of Wiki.

joevt commented 1 year ago

I think I fixed everything so far. Don't know how to fix the commit comment.

joevt commented 1 year ago

I'm going to try some rebase to clean up the commits. Ignore changes until further notice.

joevt commented 1 year ago

I used interactive rebase in Fork.app to merge the pull request update commits into the original commits, split the nvedit commit from the setenv/printenv commit, and reworded the power of 2 rom size commit. So the 15 commits in the pull request commit list should be all good.