Re: [PATCH v4 07/21] arm64/sme: Enable host kernel to access ZT0

From: Marc Zyngier <maz@kernel.org>
To: Mark Brown <broonie@kernel.org>,
	 Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>, Oleg Nesterov <oleg@redhat.com>,
	James Morse <james.morse@arm.com>,
	Alexandru Elisei <alexandru.elisei@arm.com>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	Oliver Upton <oliver.upton@linux.dev>,
	Shuah Khan <shuah@kernel.org>,
	Alan Hayward <alan.hayward@arm.com>,
	Luis Machado <luis.machado@arm.com>,
	Szabolcs Nagy <szabolcs.nagy@arm.com>,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, kvmarm@lists.linux.dev,
	linux-kselftest@vger.kernel.org
Subject: Re: [PATCH v4 07/21] arm64/sme: Enable host kernel to access ZT0
Date: Mon, 06 Feb 2023 09:31:20 +0000	[thread overview]
Message-ID: <86y1pbywbb.wl-maz@kernel.org> (raw)
In-Reply-To: <20221208-arm64-sme2-v4-7-f2fa0aef982f@kernel.org>

On Mon, 16 Jan 2023 16:04:42 +0000,
Mark Brown <broonie@kernel.org> wrote:
> 
> The new register ZT0 introduced by SME2 comes with a new trap, disable it
> for the host kernel so that we can implement support for it.
> 
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
>  arch/arm64/kernel/hyp-stub.S       | 6 ++++++
>  arch/arm64/kernel/idreg-override.c | 1 +
>  2 files changed, 7 insertions(+)
> 
> diff --git a/arch/arm64/kernel/hyp-stub.S b/arch/arm64/kernel/hyp-stub.S
> index 2ee18c860f2a..d31d1acb170d 100644
> --- a/arch/arm64/kernel/hyp-stub.S
> +++ b/arch/arm64/kernel/hyp-stub.S
> @@ -132,6 +132,12 @@ SYM_CODE_START_LOCAL(__finalise_el2)
>  	orr	x0, x0, SMCR_ELx_FA64_MASK
>  .Lskip_sme_fa64:
>  
> +	// ZT0 available?
> +	__check_override id_aa64smfr0 ID_AA64SMFR0_EL1_SMEver_SHIFT 4 .Linit_sme_zt0 .Lskip_sme_zt0
> +.Linit_sme_zt0:
> +	orr	x0, x0, SMCR_ELx_EZT0_MASK
> +.Lskip_sme_zt0:
> +

I've been looking at this in order to solve a merge conflict in next,
and couldn't convince myself that the above actually works.

__check_override assumes that the ID_AA64SMFR0_EL1 value is in x1, and
I guess that the intent of the code is to reuse value read a few lines
above. But as the comment says at the beginning of the macro, x1 will
be clobbered, and the checks always fails.

I presume we're just lucky that sme2_kernel_enable() does the same
thing unconditionally, which probably means this was only ever tested
with a VHE kernel (it'd otherwise catch fire).

The easiest fix is just to reload the id register before checking it,
something like the patch below, compile-tested only.

	M.

From a6c4aaccd33e453ffc8d8ea23a4dd4d9a263cc89 Mon Sep 17 00:00:00 2001
From: Marc Zyngier <maz@kernel.org>
Date: Mon, 6 Feb 2023 09:24:40 +0000
Subject: [PATCH] arm64/sme: Fix __finalise_el2 SMEver check

When checking for ID_AA64SMFR0_EL1.SMEver, __check_override assumes
that the ID_AA64SMFR0_EL1 value is in x1, and the intent of the code
is to reuse value read a few lines above.

However, as the comment says at the beginning of the macro, x1 will
be clobbered, and the checks always fails.

The easiest fix is just to reload the id register before checking it.

Fixes: f122576f3533 ("arm64/sme: Enable host kernel to access ZT0")
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/kernel/hyp-stub.S | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/kernel/hyp-stub.S b/arch/arm64/kernel/hyp-stub.S
index d31d1acb170d..111ff33d93ee 100644
--- a/arch/arm64/kernel/hyp-stub.S
+++ b/arch/arm64/kernel/hyp-stub.S
@@ -133,6 +133,7 @@ SYM_CODE_START_LOCAL(__finalise_el2)
 .Lskip_sme_fa64:
 
 	// ZT0 available?
+	mrs_s	x1, SYS_ID_AA64SMFR0_EL1
 	__check_override id_aa64smfr0 ID_AA64SMFR0_EL1_SMEver_SHIFT 4 .Linit_sme_zt0 .Lskip_sme_zt0
 .Linit_sme_zt0:
 	orr	x0, x0, SMCR_ELx_EZT0_MASK
-- 
2.34.1

-- 
Without deviation from the norm, progress is not possible.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
