Skip to content

Revert "feat(vex): hide RDRAND and RDSEED from CPUID with --deterministic-rng" - #55

Open
not-matthias wants to merge 1 commit into
masterfrom
cod-3957-determinstic-rng-2
Open

not-matthias wants to merge 1 commit into
masterfrom
cod-3957-determinstic-rng-2

Conversation

@not-matthias

@not-matthias not-matthias commented Oct 29, 2026 •

Copy link
Copy Markdown
Member

As discussed, to avoid timing differences keep the instruction feature flag enabled.

@not-matthias
not-matthias requested a review from art049 October 29, 2026 15:38
@not-matthias
not-matthias marked this pull request as ready for review October 29, 2026 15:39
@greptile-apps

greptile-apps Bot commented Oct 29, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 4/5

[Medium impact] The change affects amd64 feature reporting for guests using deterministic RNG.

The PR appears safe to merge, with a non-blocking request to retain the CPUID test checks.

Fix All in Claude CodeFindings

  1. P2 CPUID checks are gone ▶
Fix with agent prompt
### Issue 1
none/tests/amd64/deterministic_rng.c:38
The test no longer checks the behavior this revert restores. Keep the CPUID queries and change the expected bits to `1`: `deterministic_rng.vgtest` already requires both features before running. Without those checks, hiding either feature again would leave this test passing.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

Restores host-based CPUID flags for RDRAND and RDSEED when deterministic RNG is enabled.

  • Deterministic RNG keeps host-supported RDRAND and RDSEED visible in CPUID.

Reviews (1) · Last reviewed commit: "Revert "feat(vex): hide RDRAND and RDSEE..." · Reviewed by Greptile

__cpuid_count(7, 0, eax, ebx, ecx, edx);
printf("cpuid rdseed: %u\n", (ebx >> 18) & 1);

void* (*draws[])(void*) = {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 CPUID checks are gone

The test no longer checks the behavior this revert restores. Keep the CPUID queries and change the expected bits to 1: deterministic_rng.vgtest already requires both features before running. Without those checks, hiding either feature again would leave this test passing.

Prompt To Fix With AI
This is a comment left during a code review.
Path: none/tests/amd64/deterministic_rng.c
Line: 38

Comment:
**CPUID checks are gone**

The test no longer checks the behavior this revert restores. Keep the CPUID queries and change the expected bits to `1`: `deterministic_rng.vgtest` already requires both features before running. Without those checks, hiding either feature again would leave this test passing.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Fix in Claude Code Fix in Codex

@codspeed

codspeed Bot commented Oct 29, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 84 untouched benchmarks
⏩ 60 skipped benchmarks1


Comparing cod-3957-determinstic-rng-2 (817bee1) with master (81d7365)

Open in CodSpeed

Footnotes

  1. 60 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants