dingusdev / dingusppc

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

mmu changes #50

Closed joevt closed 10 months ago

maximumspatium commented 10 months ago

The idea implemented in 814260f is great! I think we could extend it to work together with the internal debugger later. If the debugger is running record unaligned accesses. Otherwise ignore them.

maximumspatium commented 10 months ago

I don't like the idea of splitting a 64-bit MMIO access into two consecutive 32-bit accesses. For now, only graphic cards use 64-bit accesses. Those accesses are performed for increasing drawing performance. Going through the slow MMIO interface while splitting accesses contradicts this goal. Video cards could register video memory as usual memory. That would be definitely faster.

If 64-bit MMIO is required it should be implemented by extending the return value as well as the value parameter to uint64_t. Unfortunately, it would require rewriting the existing device emulators. That's why I postponed this task to later.

maximumspatium commented 10 months ago

What's the benefit of having negative base addresses in tlb_entry->reg_va_offs? (phys_addr - reg_desc->start) calculates the register offset. Subtracting guest_va from it will produce the negative virtual base address (that unfortunately gets truncated to 32-bit in Clang 11).

What speaks against this:

tlb_entry->reg_va_offs = guest_va - (phys_addr - reg_desc->start);

This way, we will get the well readable virtual base address.

The proposed change to the TLBEntry definition is great.

joevt commented 10 months ago

If 64-bit MMIO is required it should be implemented by extending the return value as well as the value parameter to uint64_t. Unfortunately, it would require rewriting the existing device emulators. That's why I postponed this task to later.

I think including my 64-bit MMIO changes won't make rewriting the existing device emulators any more difficult. Also, some or most of the changes will be needed for that task.

What's the benefit of having negative base addresses in tlb_entry->reg_va_offs?

reg_va_offs is not a base address. It's an offset. It's calculated such that mmu_read_vmem and mmu_write_vmem only need to add it to guest_va to get the correct offset in the MMIO region.

I suppose it could be true that the offset is always negative because the MMIO read/write functions always assume the beginning to start at 0, but consider the case where the guest might map the pages in a different order or might not map the entire region. Guest page 6 could be mmio region page 8.

Anyway, host_va_offs_w and host_va_offs_r are added to guest_va, so it would be more consistent for reg_va_offs to work the same way.

(phys_addr - reg_desc->start) calculates the register offset. Subtracting guest_va from it will produce the negative virtual base address (that unfortunately gets truncated to 32-bit in Clang 11).

I suppose a (int64_t) type cast needs to be added to get the correct 64-bit representation. Since the MMIO read / write methods take a 32 bit value, the offset calculation still works. In that case, reg_va_offs could be defined as int32_t.