Skip to content

fix(ppc64): std/ld raise UC_ERR_EXCEPTION on UC_MODE_PPC64 (#1779)#2340

Open
iMoD1998 wants to merge 6 commits into
unicorn-engine:devfrom
iMoD1998:fix/ppc64-std-ld
Open

fix(ppc64): std/ld raise UC_ERR_EXCEPTION on UC_MODE_PPC64 (#1779)#2340
iMoD1998 wants to merge 6 commits into
unicorn-engine:devfrom
iMoD1998:fix/ppc64-std-ld

Conversation

@iMoD1998

@iMoD1998 iMoD1998 commented Jun 5, 2026

Copy link
Copy Markdown

Fixes #1779

std and ld instructions raise UC_ERR_EXCEPTION when emulating PPC64
code. Two independent bugs in qemu/target/ppc/translate_init.inc.c were
responsible.

Bug 1 — cpu_model index not offset for user-set PPC64 models

ppc_cpus[] is a flat array with PPC32 entries first, followed by PPC64
entries. UC_CPU_PPC64_* enum values are PPC64-relative (starting at 0),
so they must be offset by UC_CPU_PPC32_7457A_V1_2 + 1 to reach the
correct slot in the array.

The existing code applied this offset only for the default model
(INT_MAX). When a caller passed an explicit UC_CPU_PPC64_* value via
uc_ctl_set_cpu_model, the offset was never applied, so cpu_ppc_init
selected a PPC32 CPU definition. That definition lacks the PPC_64B flag
in insns_flags, so create_ppc_opcodes never registered 64-bit
instructions like std/ld, causing them to fall through to gen_invalid
and raise UC_ERR_EXCEPTION.

Fix: add an else branch that applies the offset for explicitly set
PPC64 models.

Bug 2 — MSR[HV] silently dropped at reset, breaking POWER9/10

ppc_cpu_reset calls hreg_store_msr(env, msr, 1) to apply the reset MSR
value. hreg_store_msr only propagates MSR[HV] when env->msr already
has that bit set:

if (!alter_hv || !(env->msr & MSR_HVB)) {
    value &= ~MSR_HVB;
    value |= env->msr & MSR_HVB;
}

cpu_common_reset zeroes env->msr before ppc_cpu_reset runs, so the
condition is always true and MSR[HV] is always cleared — even for CPUs
whose reset vector requires it.

On POWER9/10 (POWERPC_MMU_3_00) ppc_hash64_use_vrma() always returns
true, meaning the MMU always uses VRMA and never falls back to real-mode.
ppc_hash64_set_isi then raises POWERPC_EXCP_HISI (exception 70) when
vpm=1 && msr_hv=0, which is exactly what happens after reset. Earlier
CPUs (POWER5–8) use a real-mode path that doesn't require msr_hv=1 so
they were unaffected.

Fix: pre-seed env->msr with the masked reset value before calling
hreg_store_msr, so the MSR[HV] guard passes and the bit is preserved.

Testing

A regression test covering all eight PPC64 CPU models (970, 970fx, POWER5+,
POWER7, POWER8, POWER9, POWER10, and the default) is added in
tests/regress/ppc64_std_ld.c. Each case executes std r3, 0(r4) followed
by ld r5, 0(r4) and asserts that the round-trip value is correct.

iMoD1998 added 3 commits June 5, 2026 19:44
When a PPC64 CPU model is set via uc_ctl_set_cpu_model(), uc->cpu_model
holds a PPC64-relative index (0 = E5500, 10 = POWER5+ etc.), but
ppc_cpus[] is a flat array starting with all PPC32 entries. The range
check already adds UC_CPU_PPC32_7457A_V1_2+1 as the offset to validate
bounds, but that offset was never applied before the array lookup,
causing the wrong (PPC32) CPU family class to be loaded.

This meant all user-set PPC64 CPU models resolved to a PPC32 entry,
loading insns_flags without PPC_64B and leaving std/ld/other 64-bit
instructions unregistered, resulting in UC_ERR_EXCEPTION on any 64-bit
memory instruction.

Fixes: unicorn-engine#1779
…t reset

hreg_store_msr's HV-preservation check gates on whether env->msr already
has MSR_HV set.  Since env->msr is zeroed by cpu_common_reset() before
ppc_cpu_reset() calls hreg_store_msr(…, 1), the HV bit is silently
dropped for every CPU that has it in msr_mask (POWER5+, POWER7-10).

For POWER5–8 this was harmless because their real-mode path falls back to
RMO (RMLS-bounded) access with LPCR[VPM0]=0.  POWER9/10 (MMU_3_00)
always use VRMA and have no RMO fallback: a real-mode instruction fetch
with msr_hv=0 ends up in ppc_hash64_set_isi with vpm=true, producing a
Hypervisor Instruction Storage Exception (POWERPC_EXCP_HISI=70) before
the first instruction executes.

Pre-seeding env->msr with the masked reset value before the
hreg_store_msr call means the condition !(env->msr & MSR_HVB) is false,
so the HV bit flows through correctly.  All eight PPC64 models
(970/fx/mp, POWER5+, POWER7–10) now pass the std/ld test.
@PhilippTakacs

Copy link
Copy Markdown
Contributor

add an else branch that applies the offset for explicitly set PPC64 models

a better approach would be to handle this transparent in uc_ctl. so when setting the model add UC_CPU_PPC32_ENDING. this also need some code for reading the cpu_model. alternative split up the ppc_cpus array.

A regression test covering all eight PPC64 CPU models

Can you also put some tests in tests/unit.

@ZehMatt

ZehMatt commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

At least be honest and add whatever AI did this as the Co-Author.

@iMoD1998

iMoD1998 commented Jun 8, 2026

Copy link
Copy Markdown
Author

At least be honest and add whatever AI did this as the Co-Author.

There is nothing to be honest about. Claude found 3 line fix and i just submitted for other people.

Regression test is based on #1779.

@iMoD1998

iMoD1998 commented Jun 8, 2026

Copy link
Copy Markdown
Author

add an else branch that applies the offset for explicitly set PPC64 models

a better approach would be to handle this transparent in uc_ctl. so when setting the model add UC_CPU_PPC32_ENDING. this also need some code for reading the cpu_model. alternative split up the ppc_cpus array.

A regression test covering all eight PPC64 CPU models

Can you also put some tests in tests/unit.

I've now set the cpu model index in uc_ctl instead. Should work for both reading and writing.

Ive also changed the default cpu case to use UC_CPU_PPC32_ENDING aswell in cpu_ppc_init.

I have also added some random 64 bit tests which should catch any weird bugs. Just made a simple one for loading a 64 bit value into a register as that could possibly silently fail??

@wtdcode

wtdcode commented Jul 4, 2026

Copy link
Copy Markdown
Member

Bug 1 seems exactly #2308

@iMoD1998

iMoD1998 commented Jul 8, 2026

Copy link
Copy Markdown
Author

Anything else y'all need from me?

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.

4 participants