Skip to content

fix: semaphore syscalls return sid on success instead of KE_OK#136

Open
smmathews wants to merge 3 commits into
ran-j:mainfrom
smmathews:fix/semaphore-syscall-return-sid
Open

fix: semaphore syscalls return sid on success instead of KE_OK#136
smmathews wants to merge 3 commits into
ran-j:mainfrom
smmathews:fix/semaphore-syscall-return-sid

Conversation

@smmathews

@smmathews smmathews commented Jun 19, 2026 •
edited
Loading

Copy link
Copy Markdown

What's wrong

On the real PS2 EE BIOS, the semaphore syscalls return the semaphore ID on success, not zero. Our implementations of `PollSema`, `WaitSema`, `SignalSema`, and `DeleteSema` were all returning `KE_OK` (0) on success instead.

Why it matters

DQ8 is what surfaced this. Its init code at `0x12A670` polls a mutex semaphore and compares the result against the semaphore ID (`bne $v1, $v0`). When `PollSema` returns `KE_OK` instead of the ID, that comparison never matches, so the game spins forever at `PC 0x164978` and never finishes initializing. With the correct return value it proceeds normally.

This isn't DQ8-specific. The standard EE SDK semaphore pattern relies on the success return being the sema ID, so any game built against it can hit the same problem. `CreateSema` already returned the ID on success; the other four operations were just inconsistent with it.

What changed

Five edits in `ps2xRuntime/src/lib/Kernel/Syscalls/Sync.cpp`:

  • `PollSema`, `SignalSema`, `DeleteSema` now return `sid` on success instead of `KE_OK`.
  • `WaitSema` seeds `ret = sid` instead of `ret = 0`. The error paths (`KE_WAIT_DELETE`, `KE_RELEASE_WAIT`) are negative and still override `ret`.
  • `WaitSema`'s count-decrement guard changes from `ret == 0` to `ret == sid`. This one is required, not cosmetic: once the success value is a positive `sid`, the old `== 0` check would never fire, so the semaphore count would never be decremented and token accounting would break for every subsequent wait. Using `ret == sid` rather than `ret >= 0` also makes the success condition explicit — `ret` is seeded to `sid` and only ever overwritten with negative error codes, so the guard precisely expresses "this wait acquired this semaphore."

`ReferSemaStatus` is intentionally left returning `KE_OK` — it's a query, not an acquire/release operation, and the convention only applies to the state-changing ops.

References

Public docs are thorough for `CreateSema` but sparse on the success return values of the other four. The behavior is inferred from game code plus two sources that are consistent with it:

  • DQ8's `bne $v1, $v0` at `0x12A670`, comparing the `PollSema` return directly against the sema ID, is the concrete proof — the BIOS has to be returning `sid` for that branch to make sense.
  • ps2tek EE syscall reference documents `CreateSema` returning the semaphore ID on success. The four state-changing ops follow the same convention.
  • ps2sdk `kernel.h` declares all of these with an `s32` return type, consistent with returning the ID (negative on error, positive ID on success).

Tests

Existing semaphore tests updated to expect `sid` on success paths. New cases added:

  • Blocked `WaitSema` woken by `SignalSema` returns `sid` (the DQ8 scenario directly).
  • `WaitSema` force-released via `ReleaseWaitThread` returns `KE_RELEASE_WAIT` and does not consume a token (guards the `ret == sid` change).
  • Count-decrement assertion after a successful wait confirms the guard still decrements correctly.

@smmathews
fix: semaphore syscalls return sid on success instead of KE_OK
0a5e0af 
The PS2 EE BIOS returns the semaphore ID on success for all semaphore
syscalls, not zero. PollSema, WaitSema, SignalSema, and DeleteSema were
all returning KE_OK (0) on success, which breaks games that compare the
return value against the semaphore ID.

DQ8's init code polls a mutex semaphore at 0x12A670 and checks the result
against the semaphore ID using bne. With KE_OK returned, the comparison
always fails and the game spins forever at PC 0x164978 and never starts.
Other games that follow the same EE BIOS convention would hit the same
problem. CreateSema already returned the ID correctly; this brings the
other four operations in line with the same convention.

The fix is five changes in Sync.cpp: the four return sites are updated to
return sid, and the count-decrement guard in WaitSema is changed from
ret == 0 to ret >= 0. The guard change is necessary because ret is now
seeded to sid (a positive value) on the success path, and the old equality
check would have silently stopped decrementing the semaphore count. Error
paths (KE_WAIT_DELETE, KE_RELEASE_WAIT) are negative and still correctly
bypass the decrement.

Tests are updated to expect sid instead of KE_OK on success paths, and new
test cases cover the blocked-wait-then-signal path (the actual DQ8 scenario),
force-release via ReleaseWaitThread, and the count-decrement guard directly.
@ran-j

ran-j commented Jun 20, 2026

Copy link
Copy Markdown
Owner

Hey, I will review this one later.

Also why you close your other PR ?

@smmathews

smmathews commented Jun 20, 2026 via email •
edited
Loading

Copy link
Copy Markdown
Author

@smmathews
fix: update stale KE_OK assertions in expansion and SIF RPC tests
cb28e1a 
Two test files were not updated alongside the semaphore return-value change.
PollSema and SignalSema now return sid on success; update the four assertions
that expected KE_OK on those success paths.
@ran-j

ran-j commented Jun 20, 2026 •
edited
Loading

Copy link
Copy Markdown
Owner

Nice I see https://github.com/ps2dev/ps2sdk/blob/master/ee/kernel/include/kernel.h#L423 declare s32 SignalSema(s32 sema_id); and macros just returns what BIOS put on $v0/$2

ran-j
ran-j requested changes Jun 20, 2026
View reviewed changes
Comment thread ps2xRuntime/src/lib/Kernel/Syscalls/Sync.cpp Outdated
}

if (ret == 0 && sema->count > 0)
if (ret >= 0 && sema->count > 0)

@ran-j ran-j Jun 20, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I think is better to check ret == sid

@smmathews smmathews Jun 20, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

agreed. done.

@smmathews
fix: tighten WaitSema count-decrement guard to ret == sid
e6bf596 
ret is seeded to sid on success and only ever overwritten with negative
error codes, so ret == sid precisely expresses "this wait acquired this
semaphore" — more explicit than the looser ret >= 0.

Suggested by ran-j in PR review.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ran-j ran-j ran-j requested changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@smmathews @ran-j