Skip to content

Conversation

fsammoura1980
Copy link
Contributor

@fsammoura1980 fsammoura1980 commented Oct 10, 2025

The PMP stack guard test was previously limited to the non-multithreaded (no-mt) configuration.

This commit updates the test implementation to conditionally handle stack boundary lookup:

  1. When CONFIG_MULTITHREADING is enabled, it uses thread-specific stack information (e.g., z_interrupt_stacks[_current_cpu->id] for ISR stack and k_current_get()->stack_info for the main thread stack).

  2. New test configurations (arch.riscv.pmp.mt.*) are added to explicitly run the tests with CONFIG_MULTITHREADING=y.

This ensures PMP stack guards are correctly verified in a multi-threaded environment.

Copy link
Member

@fkokosinski fkokosinski left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

It looks like the test you're adding here is basically a copied version of the test in tests/arch/riscv/pmp/no-multithreading.

So my question is - are we covering here anything meaningful that the testcase mentioned above doesn't cover? We're only using the main() thread here anyways, which is available with CONFIG_MULTITHREADING=n as well.

Could we maybe rework this test so that it actually spawns separate threads that later try to trigger stack protection mechanisms?

@fsammoura1980
Copy link
Contributor Author

fsammoura1980 commented Oct 13, 2025

Thanks for the PR!

It looks like the test you're adding here is basically a copied version of the test in tests/arch/riscv/pmp/no-multithreading.

So my question is - are we covering here anything meaningful that the testcase mentioned above doesn't cover? We're only using the main() thread here anyways, which is available with CONFIG_MULTITHREADING=n as well.

Could we maybe rework this test so that it actually spawns separate threads that later try to trigger stack protection mechanisms?

For CONFIG_MULTITHREADING=y, the definitions for IRQ and the main stacks are different. Also MPRV has to be 1 and MPP has to be 00 for the pmp entries to be effective (main stack is unlocked). These are the 2 major differences between the 2 tests. Do you have a better suggestion of how to do the testing? The multithreading case isnot covered in the existing tests.

#ifdef CONFIG_PMP_STACK_GUARD
static void check_isr_stack_guard(void)
{
	char *isr_stack = (char *)z_interrupt_stacks[_current_cpu->id];
.
.
.
}

static void check_main_stack_guard(void)
{
	struct k_thread *current_thread_ptr = k_current_get();
	uintptr_t stack_bottom = current_thread_ptr->stack_info.start - K_KERNEL_STACK_RESERVED;

	char *main_stack = (char *)stack_bottom;
.
.
.
}

It also seems that these tests cause crashing so that nothing can be added to them i.e. I cannot add another test case in addition to ZTEST(riscv_pmp_no_mt, test_pmp)

@fsammoura1980
Copy link
Contributor Author

This test has be updated and consolidated with the existing tests.

@fsammoura1980
Copy link
Contributor Author

Thanks for the PR!

It looks like the test you're adding here is basically a copied version of the test in tests/arch/riscv/pmp/no-multithreading.

So my question is - are we covering here anything meaningful that the testcase mentioned above doesn't cover? We're only using the main() thread here anyways, which is available with CONFIG_MULTITHREADING=n as well.

Could we maybe rework this test so that it actually spawns separate threads that later try to trigger stack protection mechanisms?

The tests has been updated and combined with the existing tests.

The PMP stack guard test was previously limited to the non-multithreaded
(no-mt) configuration.

This commit updates the test implementation to conditionally handle
stack boundary lookup:

1. When CONFIG_MULTITHREADING is enabled, it uses thread-specific stack
   information (e.g., z_interrupt_stacks[_current_cpu->id] for ISR stack
   and k_current_get()->stack_info for the main thread stack).

2. New test configurations (arch.riscv.pmp.mt.*) are added to explicitly
   run the tests with CONFIG_MULTITHREADING=y.

This ensures PMP stack guards are correctly verified in a multi-threaded
environment.

Signed-off-by: Firas Sammoura <[email protected]>
Copy link

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

Labels

area: RISCV RISCV Architecture (32-bit & 64-bit) area: Tests Issues related to a particular existing or missing test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants