Skip to content

arm: Add missing hflag update after writing xpsr #2268

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wuetj wants to merge 2 commits into
base: dev
Choose a base branch
from
Open

arm: Add missing hflag update after writing xpsr #2268

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b36c0de
tests: Update exc_return test CPU
wuetj Nov 26, 2025
565aba0
target/arm: Rebuild hflags after modifying xpsr
wuetj Nov 26, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions qemu/target/arm/unicorn_arm.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@ static void v7m_msr_xpsr(CPUARMState *env, uint32_t mask, uint32_t reg,
}

xpsr_write(env, val, xpsrmask);
arm_rebuild_hflags(env);
}

static uc_err read_cp_reg(CPUARMState *env, uc_arm_cp_reg *cp)
Expand Down
4 changes: 2 additions & 2 deletions tests/unit/test_arm.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -286,8 +286,8 @@ static void test_arm_m_exc_return(void)
int r_sp = 0x8000;
uc_hook hook;

uc_common_setup(&uc, UC_ARCH_ARM, UC_MODE_THUMB | UC_MODE_MCLASS, code,
sizeof(code) - 1, UC_CPU_ARM_CORTEX_A15);
uc_common_setup(&uc, UC_ARCH_ARM, UC_MODE_THUMB, code,
sizeof(code) - 1, UC_CPU_ARM_CORTEX_M7);
Copy link
Contributor

@PhilippTakacs PhilippTakacs Nov 27, 2025

Choose a reason for hiding this comment

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

Can you create a second test instead changing the used CPU? Maybe add a CPU parameter to the function and write two tests simply calling test_arm_m_exc_return.

Copy link
Author

@wuetj wuetj Nov 27, 2025

Choose a reason for hiding this comment

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

I'm not sure why UC_CPU_ARM_CORTEX_A15 is being used here since it's not an M-class CPU. UC_MODE_MCLASS overwrites the cpu model with UC_CPU_ARM_CORTEX_M33, so this value is ignored anyways.
I could create two separate test cases for UC_CPU_ARM_CORTEX_M33 (with M security feature) and UC_CPU_ARM_CORTEX_M4 (no M security feature) if necessary.

Copy link
Contributor

@PhilippTakacs PhilippTakacs Nov 27, 2025

Choose a reason for hiding this comment

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

This are two different code path so we should have two tests. One for M33 and one for this bug.

Copy link
Author

@wuetj wuetj Nov 27, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

The check for EXC_RETURN is simply always true if the CPU has security extension, since in that case the EXC_RETURN code can also be triggered in thread mode.

unicorn/qemu/target/arm/translate.c

Lines 807 to 811 in c24c9eb

* To avoid having to do multiple comparisons in inline generated code,
* we make the check we do here loose, so it will match for EXC_RETURN
* in Thread mode. For system emulation do_v7m_exception_exit() checks
* for these spurious cases and returns without doing anything (giving
* the same behaviour as for a branch to a non-magic address).

If EXC_RETURN works for the M7, it will always work for the M33.

I can add tests for both the M7 and M33 though, if you think it's necessary.

OK(uc_mem_map(uc, r_sp - 0x1000, 0x1000, UC_PROT_ALL));
OK(uc_hook_add(uc, &hook, UC_HOOK_INTR,
test_arm_m_exc_return_hook_interrupt, NULL, 0, 0));
Expand Down