feat(hle): parameterize SoundDriver RPC service SID + subcommands per game#10
Closed
smmathews wants to merge 2 commits into
Closed
feat(hle): parameterize SoundDriver RPC service SID + subcommands per game#10smmathews wants to merge 2 commits into
smmathews wants to merge 2 commits into
Conversation
…er game Move the hardcoded SoundDriver service SID(s) and subcommand (fno) numbers out of ps2_iop.h and into the per-game PS2SoundDriverCompatLayout struct, so a title whose sound driver registers a different SID or different subcommand numbers can be served without editing deps. - PS2SoundDriverCompatLayout gains commandSid/stateSid + a servesSid() helper, the submit/getStatus/getAddrTable/streamOpen/channelConfig/stop fno fields, benignStatusValue, and the stream/channel/stop address fields. - handleSoundDriverRpcServiceImpl snapshots the layout under g_rpc_mutex, returns false without touching guest memory when unconfigured, gates on servesSid, then dispatches by looked-up fno semantic. Unknown fno on a served SID writes benignStatusValue to recv[0] and falls through (returns false) so LIBSD/game handlers still run. - Remove the IOP_SID_SNDDRV_* / IOP_RPC_SNDDRV_* placeholder constants and their references in the debug panel. - Migrate existing snddrv-state RPC tests to register a layout; add regression tests covering every subcommand semantic, the benign unknown-fno fall-through, the inert unconfigured layout, and two games routing independently through a single global layout slot.
The SoundDriver SID/fno parameterization deleted the hardcoded placeholder
constants but did not migrate the RE:CVX (slus_201.84) game override, which
relied on the old hardcoded state-SID path (sid=1, fno 0x12/0x13) to provision
the sound-driver status/addr-table pool. Post-parameterization the handler's
unconfigured guard returned false, so g_soundDriverRpcState.statusAddr was never
provisioned and the sceSifGetOtherData checksum backfill (which only fires when
srcAddr==statusAddr) silently stopped running.
- game_overrides.cpp: applyRecvxSoundDriverCompat now sets stateSid=1,
getStatusFno=0x12, getAddrTableFno=0x13 (the values of the deleted
IOP_SID_SNDDRV_STATE / IOP_RPC_SNDDRV_GET_STATUS_ADDR / _GET_ADDR_TABLE
constants). The old submit path used placeholder SID 0 / fno 0 (a non-real
service) and is intentionally left unconfigured. LotR override audited: it
only uses completionCallbacks (a separate, non-SID-gated path) and needs no
migration.
- ps2_sif_dma_tests.cpp: add a real-path regression test that provisions the
layout solely via ps2_game_overrides::applyMatching("slus_201.84") and drives
the actual SifCallRpc(getStatus) -> sceSifGetOtherData backfill, asserting the
check-array -> status backfill ran. Verified to fail if the override migration
is reverted.
- ps2_runtime.h: document the 0-sentinel limitation (SID 0 / fno 0 are
inexpressible; spec-endorsed, real services are nonzero).
- ps2_sif_rpc_tests.cpp: document that the unknown-fno benignStatusValue is a
handler-local recv[0] write, overwritten by the outer recv finalization in the
real SifCallRpc path (not guest-visible via that path).
smmathews
force-pushed
the
feature/07-sounddriver-sid-subcommand
branch
from
July 7, 2026 01:23
b6c8327 to
58f6c00
Compare
Owner
Author
|
Superseded by the upstream draft PR against ran-j/PS2Recomp: ran-j#154 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Moves the SoundDriver RPC HLE's service SID and subcommand (fno) numbers out of hardcoded constants into the per-game compat layout. Upstream already parameterized the per-game addresses via
PS2SoundDriverCompatLayout; the service SID and subcommand (fno) numbers were still hardcoded placeholder constants inps2_iop.h. This change moves the SID(s) and subcommand numbers into the compat layout so a title whose sound driver registers a different SID or different subcommand numbers is served without editing deps.What changed
PS2SoundDriverCompatLayout(ps2_runtime.h) gainscommandSid/stateSid+ aservesSid()helper, the subcommand fno fields (submitFno,getStatusFno,getAddrTableFno,streamOpenFno,channelConfigFno,stopFno),benignStatusValue(default0xffffff9b), and the extra address fields (streamStateAddr,streamReadyValue,channelAllocFlagTableAddr,stopCompletionFlagAddr).handleSoundDriverRpcServiceImpl(RPC.cpp) now:g_rpc_mutex(no fiber/scheduler interaction);falseimmediately without touching guest memory when the layout is unconfigured (commandSid == 0 && stateSid == 0);servesSid(sid), then dispatches by matchingrpcNumagainst the layout's fno fields (each match nonzero-guarded) — submit / getStatus / getAddrTable (existing semantics, re-keyed) plus the new streamOpen / channelConfig / stop writes;benignStatusValuetorecv[0]and returnsfalse(falls through so LIBSD / game handlers still run).IOP_SID_SNDDRV_*/IOP_RPC_SNDDRV_*placeholder constants fromps2_iop.hand their references inps2_debug_panel.cpp.IOP_SID_LIBSDis kept (still used by the LIBSD fast path).slus_201.84) game override (game_overrides.cpp) now carries the SID/fno numbers it relies on: it setsstateSid = 1,getStatusFno = 0x12,getAddrTableFno = 0x13(the values of the deletedIOP_SID_SNDDRV_STATE/IOP_RPC_SNDDRV_GET_STATUS_ADDR/_GET_ADDR_TABLEconstants). RE:CVX's getStatus RPC (sid=1,fno=0x12) provisionsg_soundDriverRpcState.statusAddr, and thesceSifGetOtherDatachecksum backfill fires only whensrcAddr == statusAddr; without these values the layout would be unconfigured (commandSid == stateSid == 0), the handler's unconfigured guard would returnfalse,statusAddrwould never be provisioned, and the checksum backfill would silently stop. This is the only production override that speaks the sound-driver RPC service; the LotR override uses onlycompletionCallbacks(a separate, non-SID-gated path) and correctly needs no SID/fno configuration. (RE:CVX's old submit path used placeholder SID0/ fno0— a non-real service that never matched a live call — and is intentionally left unconfigured.)ps2_sif_rpc_tests.cpp,ps2_sif_dma_tests.cpp): migrated the pre-existing snddrv-state / sound-status unit tests (which relied on the deleted placeholder SID1and fnos0x12/0x13) to register a layout that maps those numbers. Added regression tests: one exercising every subcommand semantic + the benign unknown-fno fall-through; one proving the inert unconfigured layout plus two games routing independently through the single global layout slot; and a real-path test that provisions the layout solely throughps2_game_overrides::applyMatching("slus_201.84")and drives the actualSifCallRpc(getStatus)→sceSifGetOtherDatapath, asserting the check-array → status backfill ran and that getStatus returns a nonzero status address.Known limitations
0is the "unused" sentinel for every SID and fno field, so a real service whose SID is literally0or a subcommand whose fno is literally0cannot be expressed. This is a deliberate tradeoff and adequate in practice — real SIF-RPC services are nonzero, and the deleted placeholder constants that were0(IOP_SID_SNDDRV_COMMAND/_SUBMIT) were never live services. Documented in a code comment on the struct; if a title ever needs SID 0 / fno 0, add explicit has-value flags rather than overloading the sentinel.benignStatusValueis handler-local, not guest-visible on the normal path. For an unknown fno on a served SID the handler writesbenignStatusValuetorecv[0]and returnsfalse; in the realSifCallRpcpath the outer recv finalization then overwritesrecv[0](copy-from-send / zero) before the guest observes it. The unknown-fno unit test asserts the handler-local write directly and is annotated to say so — it locks the handler's documented contract, not guest-observable state.Verification
ps2x_testssuite: 299 passed / 0 failed. Rebuilt clean (GCC 16) and re-run.streamReadyValuetostreamStateAddrand signals nowait; channelConfig setschannelAllocFlagTableAddr[channel]=1(channel from send word 0,<16); stop writes1tostopCompletionFlagAddr; getStatus/getAddrTable return distinct nonzero addresses; unknown fno returnsfalsewith handler-localrecv[0]==0xffffff9b; unconfigured layout leaves guest memory untouched and returnsfalse.game_overrides.cppoverride migration makes the new real-path regression test fail (getStatusreturns 0, backfill does not run); restoring it returns the suite to green.Scope notes
game_overrides.cpp(the RE:CVX override migration, above) andps2_sif_dma_tests.cpp(two existing tests issuedSifCallRpcwithsid=1, rpcNum=0x12relying on the old hardcoded constants and never configured a layout; updated to setstateSid=1/getStatusFno=0x12, plus the new real-path regression test). Necessary collateral of the layout-gated dispatch. The debug-panel edits are likewise required collateral of deleting the constants.0x410b64/0x410b78) and the stream descriptor (0x41b550) are not implemented — they have no enumerated*Fnosemantic slot, no defined trigger, and no test coverage. A real DQ8 port would need those added.Known interactions
benignStatusValue(0xffffff9b, the documented "no audio work required" discard sentinel) torecv[0]and then falls through. For a game that muxes its driver onto the LIBSD SID, this means every unknown-fno LIBSD call transiently gets0xffffff9binrecv[0]before the LIBSD backend runs and overwrites it. This is the intended fall-through design; flagged here for whoever later wires a real game onto that SID.servesSidpasses,rpcNumis matched against all*Fnofields regardless of whether the call arrived oncommandSidvsstateSid. This is by design (servesSid passes, thenrpcNumis compared to the*Fnofields); distinct fno numbers disambiguate in real drivers, and the single-SID muxed case relies on it.🤖 Generated with Claude Code