Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement support for APU DSP Memory access (fix EP, add GP) #11

Closed
wants to merge 3 commits into from

Conversation

JayFoxRox
Copy link
Owner

@JayFoxRox JayFoxRox commented Jul 19, 2018

This is required to read-back results of MCPX APU DSP calculations more easily, so without this change, debugging the DSP from the outside is a lot harder.

This is code from 2017, but it has gotten a lot of additional unit testing and research on actual hardware the past weeks.

Once this patch is applied, we can use xboxpy and my existing dsp-homebrew script to send custom code to the DSP. This allows us to verify instruction behaviour, as the script also works with a real Xbox.

I was unsure about the DSP memory sizes, but then decided to confirm them:

I have written a program to measure the DSP data memory sizes (Click to unfold)

Here is the code for measuring Y-Memory size:

start
  move y:$<address>, a0
  move a0, x:$000000
  jmp start

Where <address> was raised using a step-size.
I then wrote a random value to y:$<address> using the CPU and checked if it appeared in x:$000000. I did this for 2 random values per <address>. When any of these checks failed, the previous <address> was reverted and a smaller step-size was used.

For measuring the size of X-Memory instead of Y-Memory I have simply swapped x: and y: (and respective lookups on the CPU).
For measuring the size of the MIXBUF, I set a higher start offset (0x1400) in X-Memory and re-ran the same program as before for measuring X-Size. Strangely, I got 0x7E0 words here. The last 0x20 words were not accessible. See known problem below.

I also wrote a program to measure the DSP program memory size (Click to unfold)

Here is the code for measuring P-Memory size:

start
  move y:$000000, a0
  <nops>
  move a0, x:$000000
  jmp start

Where <nops> is an increasing number of nop instructions. By raising the number of nop instructions, the write to x:$000000 will eventually be outside of program memory. This will result in either the value in y:$000000 not updating or the Xbox crashing.
Keep in mind that the other instructions take up some memory (6 words) so the number of nop instructions (1 word each) is not the same as the program memory size.


The results of these tests have been added to XboxDevWiki.

According to existing code, the last 0x400 words of GP X-Memory (so, offset 0xC00) should be MIXBUF. My tests could not confirm this behaviour. I have observed MIXBUF at x:$001400, 0x380 words (See problem regarding size below).
I assume that DirectSound uses the last 0x400 words of X-Memory RAM for the DMA from the actual MIXBUF; but X-Memory RAM has no relation to MIXBUF in hardware.

Known problems

Only 32 bit access works

Also see FIXME 91. This might cause trouble in games, so this PR should be tested with a handful of games.
I did not test a single game yet.

MIXBUF size not known

I was unable to access the last 0x20 words of MIXBUF with my unit test.

I assume this might have to do with VP or EP having control and overwriting my data. I still assume a size of 0x400 bytes, not 0x380; so I implemented 0x400 in XQEMU, but documented 0x380 for now. To be fair, I was surprised that I had write access to MIXBUF at all, as I had assumed the VP would instantly overwrite data.

EP and GP use the same memory map in XQEMU

Memory size defines are used by both DSP instances, so we use the same memory map for GP and EP.

This is not so much a problem with this PR, but XQEMU in general. This even means that the EP currently has MIXBUF access which is not possible on real hardware.

I'll create an issue after merge. I did not fix it here because it leads to design decisions and what we have works (our EP is just more powerful than on real hardware).


This patch could fix bugs in the following games:

  • Top Gear RPM Tuning
  • Grand Theft Auto: San Andreas

These games previously ran into the following issue in dsp_cpu: assert(address < DSP_XRAM_SIZE);.

However, I did not test those games because it's not the motivation for this patch: I just want to be able to debug the DSP.


If you want to test this PR with my DSP homebrew, keep in mind that nxdk-rdt does not support CALL in master yet (meaning it can't remote control the DSP using existing code).
You will need to look for nxdk-rdt and xboxpy PRs to work around this.
Testing using XBDM should work fine.

@JayFoxRox JayFoxRox changed the title Add write support for DSP registers (fix EP, add GP) Add support for DSP registers access (fix EP, add GP) Jul 19, 2018
@JayFoxRox JayFoxRox changed the title Add support for DSP registers access (fix EP, add GP) Add support for APU DSP registers access (fix EP, add GP) Jul 19, 2018
@JayFoxRox JayFoxRox mentioned this pull request Jul 19, 2018
38 tasks
@JayFoxRox JayFoxRox added the wip label Jul 19, 2018
@JayFoxRox JayFoxRox changed the title Add support for APU DSP registers access (fix EP, add GP) Implement support for APU DSP Memory access (fix EP, add GP) Jul 21, 2018
{
int space;

switch (space_id) {
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Swap names for space_id and space; also for dsp_read_memory

@JayFoxRox JayFoxRox force-pushed the dsp-regs branch 2 times, most recently from 72113f2 to 1455480 Compare July 24, 2018 17:24
@JayFoxRox
Copy link
Owner Author

Upstreamed.

@JayFoxRox JayFoxRox closed this Jul 24, 2018
JayFoxRox pushed a commit that referenced this pull request Sep 11, 2018
With a Spice port chardev, it is possible to reenter
monitor_qapi_event_queue() (when the client disconnects for
example). This will dead-lock on monitor_lock.

Instead, use some TLS variables to check for recursion and queue the
events.

Fixes:
 (gdb) bt
 #0  0x00007fa69e7217fd in __lll_lock_wait () at /lib64/libpthread.so.0
 #1  0x00007fa69e71acf4 in pthread_mutex_lock () at /lib64/libpthread.so.0
 #2  0x0000563303567619 in qemu_mutex_lock_impl (mutex=0x563303d3e220 <monitor_lock>, file=0x5633036589a8 "/home/elmarco/src/qq/monitor.c", line=645) at /home/elmarco/src/qq/util/qemu-thread-posix.c:66
 #3  0x0000563302fa6c25 in monitor_qapi_event_queue (event=QAPI_EVENT_SPICE_DISCONNECTED, qdict=0x56330602bde0, errp=0x7ffc6ab5e728) at /home/elmarco/src/qq/monitor.c:645
 #4  0x0000563303549aca in qapi_event_send_spice_disconnected (server=0x563305afd630, client=0x563305745360, errp=0x563303d8d0f0 <error_abort>) at qapi/qapi-events-ui.c:149
 #5  0x00005633033e600f in channel_event (event=3, info=0x5633061b0050) at /home/elmarco/src/qq/ui/spice-core.c:235
 #6  0x00007fa69f6c86bb in reds_handle_channel_event (reds=<optimized out>, event=3, info=0x5633061b0050) at reds.c:316
 #7  0x00007fa69f6b193b in main_dispatcher_self_handle_channel_event (info=0x5633061b0050, event=3, self=0x563304e088c0) at main-dispatcher.c:197
 #8  0x00007fa69f6b193b in main_dispatcher_channel_event (self=0x563304e088c0, event=event@entry=3, info=0x5633061b0050) at main-dispatcher.c:197
 #9  0x00007fa69f6d0833 in red_stream_push_channel_event (s=s@entry=0x563305ad8f50, event=event@entry=3) at red-stream.c:414
 #10 0x00007fa69f6d086b in red_stream_free (s=0x563305ad8f50) at red-stream.c:388
 #11 0x00007fa69f6b7ddc in red_channel_client_finalize (object=0x563304df2360) at red-channel-client.c:347
 #12 0x00007fa6a56b7fb9 in g_object_unref () at /lib64/libgobject-2.0.so.0
 #13 0x00007fa69f6ba212 in red_channel_client_push (rcc=0x563304df2360) at red-channel-client.c:1341
 #14 0x00007fa69f68b259 in red_char_device_send_msg_to_client (client=<optimized out>, msg=0x5633059b6310, dev=0x563304e08bc0) at char-device.c:305
 #15 0x00007fa69f68b259 in red_char_device_send_msg_to_clients (msg=0x5633059b6310, dev=0x563304e08bc0) at char-device.c:305
 #16 0x00007fa69f68b259 in red_char_device_read_from_device (dev=0x563304e08bc0) at char-device.c:353
 #17 0x000056330317d01d in spice_chr_write (chr=0x563304cafe20, buf=0x563304cc50b0 "{\"timestamp\": {\"seconds\": 1532944763, \"microseconds\": 326636}, \"event\": \"SHUTDOWN\", \"data\": {\"guest\": false}}\r\n", len=111) at /home/elmarco/src/qq/chardev/spice.c:199
 #18 0x00005633034deee7 in qemu_chr_write_buffer (s=0x563304cafe20, buf=0x563304cc50b0 "{\"timestamp\": {\"seconds\": 1532944763, \"microseconds\": 326636}, \"event\": \"SHUTDOWN\", \"data\": {\"guest\": false}}\r\n", len=111, offset=0x7ffc6ab5ea70, write_all=false) at /home/elmarco/src/qq/chardev/char.c:112
 #19 0x00005633034df054 in qemu_chr_write (s=0x563304cafe20, buf=0x563304cc50b0 "{\"timestamp\": {\"seconds\": 1532944763, \"microseconds\": 326636}, \"event\": \"SHUTDOWN\", \"data\": {\"guest\": false}}\r\n", len=111, write_all=false) at /home/elmarco/src/qq/chardev/char.c:147
 #20 0x00005633034e1e13 in qemu_chr_fe_write (be=0x563304dbb800, buf=0x563304cc50b0 "{\"timestamp\": {\"seconds\": 1532944763, \"microseconds\": 326636}, \"event\": \"SHUTDOWN\", \"data\": {\"guest\": false}}\r\n", len=111) at /home/elmarco/src/qq/chardev/char-fe.c:42
 #21 0x0000563302fa6334 in monitor_flush_locked (mon=0x563304dbb800) at /home/elmarco/src/qq/monitor.c:425
 #22 0x0000563302fa6520 in monitor_puts (mon=0x563304dbb800, str=0x563305de7e9e "") at /home/elmarco/src/qq/monitor.c:468
 #23 0x0000563302fa680c in qmp_send_response (mon=0x563304dbb800, rsp=0x563304df5730) at /home/elmarco/src/qq/monitor.c:517
 #24 0x0000563302fa6905 in qmp_queue_response (mon=0x563304dbb800, rsp=0x563304df5730) at /home/elmarco/src/qq/monitor.c:538
 #25 0x0000563302fa6b5b in monitor_qapi_event_emit (event=QAPI_EVENT_SHUTDOWN, qdict=0x563304df5730) at /home/elmarco/src/qq/monitor.c:624
 #26 0x0000563302fa6c4b in monitor_qapi_event_queue (event=QAPI_EVENT_SHUTDOWN, qdict=0x563304df5730, errp=0x7ffc6ab5ed00) at /home/elmarco/src/qq/monitor.c:649
 #27 0x0000563303548cce in qapi_event_send_shutdown (guest=false, errp=0x563303d8d0f0 <error_abort>) at qapi/qapi-events-run-state.c:58
 #28 0x000056330313bcd7 in main_loop_should_exit () at /home/elmarco/src/qq/vl.c:1822
 #29 0x000056330313bde3 in main_loop () at /home/elmarco/src/qq/vl.c:1862
 #30 0x0000563303143781 in main (argc=3, argv=0x7ffc6ab5f068, envp=0x7ffc6ab5f088) at /home/elmarco/src/qq/vl.c:4644

Note that error report is now moved to the first caller, which may
receive an error for a recursed event. This is probably fine (95% of
callers use &error_abort, the rest have NULL error and ignore it)

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20180731150144.14022-1-marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
[*_no_recurse renamed to *_no_reenter, local variables reordered]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant