Re: [kvm-unit-tests PATCH v1 5/7] arm64: selftest: update test for running at EL2

Alexandru Elisei
2025-02-27T16:58:49Z

Hi Joey,

On Thu, Feb 20, 2025 at 02:13:52PM +0000, Joey Gouly wrote:
> Remove some hard-coded assumptions that this test is running at EL1.
>
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>

We discussed this privately, and this patch was split from from the bigger
patch that added support to all tests to run at EL2 [1]. Joey kept my
Signed-off-by, but accidently dropped the authorship.

[1] https://lore.kernel.org/all/1577972806-16184-3-git-send-email-alexandru.elisei@arm.com/

> Signed-off-by: Joey Gouly <joey.gouly@arm.com>
> ---
> arm/selftest.c | 18 +++++++++++++-----
> 1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/arm/selftest.c b/arm/selftest.c
> index 1553ed8e..eccdc3d4 100644
> --- a/arm/selftest.c
> +++ b/arm/selftest.c
> @@ -232,6 +232,7 @@ static void user_psci_system_off(struct pt_regs *regs)
> __user_psci_system_off();
> }
> #elif defined(__aarch64__)
> +static unsigned long expected_level;
>
> /*
> * Capture the current register state and execute an instruction
> @@ -276,8 +277,7 @@ static bool check_regs(struct pt_regs *regs)
> {
> unsigned i;
>
> - /* exception handlers should always run in EL1 */
> - if (current_level() != CurrentEL_EL1)
> + if (current_level() != expected_level)
> return false;
>
> for (i = 0; i < ARRAY_SIZE(regs->regs); ++i) {
> @@ -301,7 +301,11 @@ static enum vector check_vector_prep(void)
> return EL0_SYNC_64;
>
> asm volatile("mrs %0, daif" : "=r" (daif) ::);
> - expected_regs.pstate = daif | PSR_MODE_EL1h;
> + expected_regs.pstate = daif;
> + if (current_level() == CurrentEL_EL1)
> + expected_regs.pstate |= PSR_MODE_EL1h;
> + else
> + expected_regs.pstate |= PSR_MODE_EL2h;
> return EL1H_SYNC;
> }
>
> @@ -317,8 +321,8 @@ static bool check_und(void)
>
> install_exception_handler(v, ESR_EL1_EC_UNKNOWN, unknown_handler);
>
> - /* try to read an el2 sysreg from el0/1 */
> - test_exception("", "mrs x0, sctlr_el2", "", "x0");
> + /* try to read an el3 sysreg from el0/1/2 */
> + test_exception("", "mrs x0, sctlr_el3", "", "x0");
>
> install_exception_handler(v, ESR_EL1_EC_UNKNOWN, NULL);
>
> @@ -429,6 +433,10 @@ int main(int argc, char **argv)
> if (argc < 2)
> report_abort("no test specified");
>
> +#if defined(__aarch64__)
> + expected_level = current_level();
> +#endif
> +
> report_prefix_push(argv[1]);
>
> if (strcmp(argv[1], "setup") == 0) {

Looks good to me:

Reviewed-by: Alexandru Elisei <alexandru.elisei@arm.com>

Thanks,
Alex