Redesign memory access functions

[JayFoxRox]

This redesigns the memory read / write functionality.
By giving it a deterministic data read/write behaviour it becomes useful for MMIO.

---

The proposed behaviour of the reads / writes is:

• You can write any size you want
• All writes are in order, from lowest to highest address
• While there is more than 4 bytes left: read/write 4 bytes
• While there is more than 2 bytes left: read/write 2 bytes
• While there is more than 1 bytes left: read/write 1 byte

Logically, it's not possible to get more than one 2 byte write and one 1 byte write.
The while loops were still kept to make the repeating pattern in the code more obvious + someone might want to disable the 4 byte write or extend the function in the future.

I did consider design alternatives, but this setup works for most cases. Most MMIO registers are 4 byte on Xbox, so we can write a bunch of them at once (using a single packet / more or less atomic operation) if need-be. We can also read and write large blocks of data at okish-speeds, still.

Known problem

The protobuf bytes type probably doesn't free the allocated data anywhere. Therefore, this probably leaks memory.
An issue should be created after merge, so someone should look into this in the future.
Fixed.

---

The second commit is a design decision: Instead of using the existing xbox.mem function, this turns it into xbox.mem_read and xbox.mem_write.
This reflects the actual protobuf function names and avoids possible conflicts (where size and data can both be supplied for example).

I did consider naming the new functions just read and write to be more like the Python stream (file/socket) operations. However, we might want xbox.io_write and xbox.io_read later, so I decided to keep the protobuf names with the mem_ prefix.

---

Closes #2

[JayFoxRox]

I have reviewed the design (this was written over a year ago) and I dislike some of the choices:

• Leaks memory, where master didn't. Fixed.
• Forces 32 bit access even when trying to stream large amounts of memory. This could have performance implications. It's therefore probably a better idea to not always enforce this rule. For now, just assume that a 1, 2 and 4 byte access is guaranteed if any of those sizes is given. Anything else might do something different (such as copying data through SSE).

However, this allows MMIO and we still have almost 10k leaky-allocations before memory reaches 30MB+.
I'd rather have buggy software than non-functional software, so I'm fine with these drawbacks - we can iterate later.

I've also fixed a small issue in dbg.py with the first commit.
Otherwise I think the code looks fine.

Both commits have been tested and worked fine.
I believe I only tested 4 byte MMIO (with my timer-script).
I can't think of a good use-case for 1 or 2 byte right now either.
My test did run into some occasional issues, but these could also stem from #5, bugs in xboxpy or simply the leak issue I explained above.
For the majority of the time, this patch behaved nicely, and a simple reboot always fixed all problems.

---

I think we should merge this as-is and then address the issues later.
However, even with this PR, nobody should rely on guaranteed access sizes except for explicit access for 1, 2 or 4 bytes (see reason above).

[mborgerson]

free?

[JayFoxRox]

See "Known problems" in first post. We discussed this in 2017 when I prototyped this PR but I never got a response.

I know this is bad code, but without this feature (MMIO compatible read/write) nxdk-rdt is pretty useless. I don't really want to spend another few hours to rewrite this differently - this code already exists and at least makes nxdk-rdt functional.
We can iterate and make nxdk-rdt less leaky and more stable later.

[mborgerson]

I like this PR but prefer not to merge it until the leak is fixed.

If memory serves me correct malloc is allocating 4K size pages per alloc. So you get about 64M/4K ~= 16k calls until you exhaust memory entirely (not counting stuff that's already in memory), and I could easily see us making 16k requests.

[JayFoxRox]

We do run into this issue (see my previous post, also contains the same 4k logic).

But I currently don't have the time to redesign nxdk-rdt to support free()
I'm not motivated to spend much time on nxdk-rdt right now either because there's more important issues to fix and we might even redesign a lot of nxdk-rdt soon anyway (See #16).

I already have a long backlog for XQEMU.
If this PR does not happen all my DSP and other XQEMU work is delayed because that requires MMIO (this PR) and CALL (planned PR). It also delays work on nv2a-trace and other tools.

[JayFoxRox]

I added a simple work-around by using a single growing buffer.

As the buffer is never free'd, you have to be careful still when doing large reads (as memory fills up and isn't available for other allocations).
You also have to be careful when implementing functions, because the transfer buffer can move when it is resized (and all contents are lost; I avoided realloc because it would just reduce performance).

My tests still reliably crashed within the first minute - but so does running:

# Print something to the screen
while True:
	xbox.debug_print("Hello!")

(So this is probably not the PRs fault, but #5 or another issue)

I did not confirm if the code works correctly (so I did not add debug-prints to trace the current buffer address / size / malloc / free), so please review that function.

I also did not try to read(addr, size=0) and I'm not sure what will happen.
Same goes for write(addr, data=bytes([])).

[PatrickvL]

Be aware using static variables kills multithreaded use.

[JayFoxRox]

This was more of a workaround to avoid a leak and get this merged quickly.
I agree that we should eventually have an object for each connection.

I'll create an issue about improving this after merge. Thanks for pointing it out.