feat(hle): parameterize SoundDriver RPC service SID + subcommands per game - #154
Open
smmathews wants to merge 3 commits into
Open
feat(hle): parameterize SoundDriver RPC service SID + subcommands per game#154smmathews wants to merge 3 commits into
smmathews wants to merge 3 commits into
Conversation
smmathews
force-pushed
the
feature/07-sounddriver-sid-subcommand
branch
from
July 7, 2026 18:52
58f6c00 to
d6c325f
Compare
…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. Upstream already parameterized the per-game
addresses; this extends the layout to the SID and fno numbers too.
- PS2SoundDriverCompatLayout gains commandSid/stateSid + a servesSid() helper,
the submit/getStatus/getAddrTable/streamOpen/channelConfig/stop fno fields,
benignStatusValue (default 0xffffff9b), and the stream/channel/stop address
fields.
- handleSoundDriverRpcServiceImpl snapshots the layout under g_rpc_mutex,
returns false without touching guest memory when unconfigured
(commandSid == stateSid == 0), gates on servesSid, then dispatches by matching
rpcNum against the layout's fno fields (each nonzero-guarded) -- submit /
getStatus / getAddrTable (existing semantics, re-keyed) plus new streamOpen /
channelConfig / stop writes. 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. IOP_SID_LIBSD is kept (LIBSD fast path).
- Migrate the RE:CVX (slus_201.84) override, which relied on the deleted
state-SID path (sid=1, fno 0x12/0x13) to provision the sound-driver
status/addr-table pool: applyRecvxSoundDriverCompat now sets stateSid=1,
getStatusFno=0x12, getAddrTableFno=0x13. Without this the handler's
unconfigured guard returns false, statusAddr is never provisioned, and the
sceSifGetOtherData checksum backfill (fires only when srcAddr==statusAddr)
silently stops. The old submit path used placeholder SID 0 / fno 0 (a non-real
service) and is left unconfigured. LotR override audited: uses only
completionCallbacks (a separate, non-SID-gated path) and needs no migration.
0-sentinel limitation: 0 is the "unused" value for every SID/fno field, so a
service whose SID is literally 0 or a subcommand whose fno is literally 0 cannot
be expressed. Deliberate tradeoff -- real SIF-RPC services are nonzero, and the
deleted placeholder constants that were 0 were never live services.
Tests: migrate the existing snddrv-state RPC unit tests to register a layout;
add regression tests covering every subcommand semantic, the benign unknown-fno
fall-through, the inert unconfigured layout, two games routing independently
through the single global layout slot, and a real-path test provisioning the
layout solely via applyMatching("slus_201.84") and driving the actual
SifCallRpc(getStatus) -> sceSifGetOtherData backfill (fails if the override
migration is reverted).
ps2x_tests: 299 passed, 0 failed.
smmathews
force-pushed
the
feature/07-sounddriver-sid-subcommand
branch
from
July 7, 2026 18:58
d6c325f to
56c23d0
Compare
smmathews
marked this pull request as ready for review
July 7, 2026 19:01
Owner
|
Hey, sorry but I broke your PR |
Contributor
Author
no worries, I'm on it |
# Conflicts: # ps2xRuntime/include/ps2_runtime.h # ps2xRuntime/include/runtime/ps2_iop.h # ps2xRuntime/src/lib/Kernel/Syscalls/RPC.cpp # ps2xRuntime/src/lib/game_overrides.cpp # ps2xRuntime/src/lib/ps2_debug_panel.cpp # ps2xTest/src/ps2_sif_dma_tests.cpp # ps2xTest/src/ps2_sif_rpc_tests.cpp
…e new TSNDDRV module Upstream's IOP refactor (ran-j#170) deleted the ps2_iop.h/RPC.cpp SoundDriver RPC implementation this PR was built on and replaced it with a plugin-style ps2xIOP module system, moving RE:CVX's sound-driver handling into ps2xIOP/src/modules/tsnddrv.cpp with the SID/subcommand numbers hardcoded as module constants. Port this PR's feature onto that module instead of the deleted code: - TsnddrvBindings gains commandSid/stateSid/submitFunction/getStatusFunction/ getAddrTableFunction (defaults preserve the previous hardcoded RE:CVX values) plus optional streamOpenFunction/channelConfigFunction/stopFunction subcommands, their target addresses, and benignStatusValue -- mirroring the PS2SoundDriverCompatLayout fields this PR added before the refactor. - TsnddrvService::handleRpc dispatches off the bindings instead of fixed constants, supports muxing command+state on a single SID, serves the three optional subcommands when configured, and writes benignStatusValue (without marking the RPC handled) for an unrecognized subcommand on a served SID so a guest-registered server function at that SID can still run. - createTsnddrvService validates getStatusFunction != getAddrTableFunction and that the optional subcommands don't alias submit/getStatus/getAddrTable or each other. - builtin_profiles.cpp's RE:CVX binding now sets these fields explicitly instead of relying on module-internal constants. - Also includes <cstdint> explicitly in elf_parser.h: ELFIO 3.12's elf_types.hpp uses fixed-width integer types without including it, which a newer standard library no longer pulls in transitively. Test coverage for the old RPC.cpp-level implementation (ps2_sif_rpc_tests.cpp, ps2_sif_dma_tests.cpp) is removed since that code no longer exists; new module-level coverage for the parameterization and the three optional subcommands is added in ps2_iop_tests.cpp instead. ps2x_tests: 335 passed, 0 failed.
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.
Problem
The
TSNDDRVmodule answers a fixed pair of service ids and a fixed set of subcommandnumbers, held as file-local
constexprvalues intsnddrv.cpp(kCommandSid,kStateSid,kSubmitFunction,kGetStatusAddressFunction,kGetAddressTableFunction). Its other title-specific numbers already arrive throughTsnddrvBindingsfrom the game's entry inbuiltin_profiles.cpp: arena geometry,checksum candidates, busy flag address, completion rules. A title whose driver registers
a different service id, or numbers its subcommands differently, cannot reuse the module
without editing it.
Fix
TsnddrvBindingsgainscommandSid,stateSid,submitFunction,getStatusFunction,getAddrTableFunction. Defaults are the deleted constants' values,so the shipped profile behaves as before.
handleRpcdispatches against the bindings, andsids()is built at construction, so adriver that muxes command and state onto one service id registers it once, not twice.
0:streamOpenFunctionwritesstreamReadyValuetostreamStateAddress;channelConfigFunctionsetschannelAllocFlagTableAddress[channel] = 1, takingchannelfrom send word 0 whenbelow 16;
stopFunctionwrites1tostopCompletionFlagAddress. Each zeroes thereceive word and signals nowait completion.
benignStatusValue(default0xffffff9b) to the receive word and stays unhandled,so a guest-registered server function at that service id still runs.
createTsnddrvServicethrowsstd::invalid_argumentwhengetStatusFunctionequalsgetAddrTableFunction, or when an enabled optional subcommand aliases submit,getStatus or getAddrTable — either would make dispatch ambiguous.
recvx-usprofile (slus_201.84) states its service ids and subcommand numbersexplicitly rather than inheriting them.
Basis
The numbers are the deleted constants, carried over unchanged: the removed
constexprblock and the profile's new fields are both in this diff.
TsnddrvBindingsis alreadyhow the module takes its per-title addresses.
Testing
New cases in
ps2_iop_tests.cpp:0x80007701with subcommands0x30/0x31/0x32:sids()reports that id once, the custom getStatus returns anonzero status address, and a request on
1is refused.0x40/0x41/0x42: byte-level checks thatstreamReadyValuereaches
streamStateAddress, that channel 5's slot inchannelAllocFlagTableAddressbecomes
1, and thatstopCompletionFlagAddressbecomes1. Subcommand0x77returns unhandled with
benignStatusValuein the receive word.The sound-status cases in
ps2_sif_rpc_tests.cppspell1,0x12and0x13at thepoint of use instead of that file's
IOP_SID_SNDDRV_*/IOP_RPC_SNDDRV_*aliases,which no longer track the module.
ps2xTest/CMakeLists.txtaddsps2xIOP/srcto thetest include path.
Risk and not in scope
slus_201.84: an RPC on service id0or1carrying anunrecognized subcommand, with a receive buffer of at least four bytes, now gets
0xffffff9bwritten to its receive word before falling through.SifCallRpc'sfinalization in
Kernel/Syscalls/RPC.cppthen overwrites that word — copy from send,or zero — because the call stays unhandled. It survives only when a guest server
function at that id is dispatched and returns
0with the server buffer at0,leaving the result pointer equal to the receive buffer.
service ids the call arrived on. Distinct numbers disambiguate in real drivers, and the
muxed single-id case depends on it.
submitFunctioncolliding withgetStatusFunctionorgetAddrTableFunctionis notrejected; they sit on different service ids in the shipped profile.
descriptor a second title would need have no subcommand slot, no trigger and no tests.
ps2xRecomp/include/ps2recomp/elf_parser.hgains#include <cstdint>, unrelated tosound. That fix landed separately in fix(recomp): include <cstdint> before elfio in elf_parser.h #199, so the line drops out of this branch on the
next rebase.
TsnddrvBindings SID and subcommand fields — name, default, effect — and the values the recvx-us (slus_201.84) profile sets
commandSid0x00000000stateSid0x00000001submitFunction0x00000000getStatusFunction0x00000012statusAddressgetAddrTableFunction0x00000013addressTableAddressstreamOpenFunction0(off)streamReadyValuetostreamStateAddresschannelConfigFunction0(off)channelAllocFlagTableAddress[channel] = 1stopFunction0(off)1tostopCompletionFlagAddressbenignStatusValue0xffffff9bstreamStateAddress0streamReadyValue0channelAllocFlagTableAddress0stopCompletionFlagAddress00disables the three optional subcommands. It does not disable a service id:0is areal service id here, and
slus_201.84uses it.recvx-us(slus_201.84) sets:It leaves the optional subcommands and their addresses at their defaults, so its
dispatch is unchanged apart from the benign-status write noted above.