Re: [PATCH v14 16/39] arm64/sme: Implement traps and syscall handling for SME

From: Zenghui Yu <yuzenghui@huawei.com>
To: Mark Brown <broonie@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>, Marc Zyngier <maz@kernel.org>,
	Shuah Khan <skhan@linuxfoundation.org>,
	Shuah Khan <shuah@kernel.org>,
	Basant Kumar Dwivedi <Basant.KumarDwivedi@arm.com>,
	Luis Machado <luis.machado@arm.com>,
	Szabolcs Nagy <szabolcs.nagy@arm.com>,
	<linux-arm-kernel@lists.infradead.org>,
	<linux-kselftest@vger.kernel.org>,
	Alan Hayward <alan.hayward@arm.com>,
	<kvmarm@lists.cs.columbia.edu>,
	Salil Akerkar <Salil.Akerkar@arm.com>,
	Luca Salabrino <luca.scalabrino@arm.com>
Subject: Re: [PATCH v14 16/39] arm64/sme: Implement traps and syscall handling for SME
Date: Wed, 7 Dec 2022 22:00:17 +0800	[thread overview]
Message-ID: <e57552ea-37fb-07da-8560-e26bae8205cd@huawei.com> (raw)
In-Reply-To: <20220419112247.711548-17-broonie@kernel.org>

On 2022/4/19 19:22, Mark Brown wrote:

> diff --git a/arch/arm64/kernel/syscall.c b/arch/arm64/kernel/syscall.c
> index c938603b3ba0..92c69e5ac269 100644
> --- a/arch/arm64/kernel/syscall.c
> +++ b/arch/arm64/kernel/syscall.c
> @@ -158,11 +158,36 @@ static void el0_svc_common(struct pt_regs *regs, int scno, int sc_nr,
>  	syscall_trace_exit(regs);
>  }
>  
> -static inline void sve_user_discard(void)
> +/*
> + * As per the ABI exit SME streaming mode and clear the SVE state not
> + * shared with FPSIMD on syscall entry.
> + */
> +static inline void fp_user_discard(void)
>  {
> +	/*
> +	 * If SME is active then exit streaming mode.  If ZA is active
> +	 * then flush the SVE registers but leave userspace access to
> +	 * both SVE and SME enabled, otherwise disable SME for the
> +	 * task and fall through to disabling SVE too.  This means

It looks a bit confusing to me that in the current implementation, if
ZA is *not* active, we don't actually go to disable SME for the task
(which IMHO can be disabled as user-space is not actively using it now).

> +	 * that after a syscall we never have any streaming mode
> +	 * register state to track, if this changes the KVM code will
> +	 * need updating.
> +	 */
> +	if (system_supports_sme() && test_thread_flag(TIF_SME)) {
> +		u64 svcr = read_sysreg_s(SYS_SVCR_EL0);
> +
> +		if (svcr & SYS_SVCR_EL0_SM_MASK)
> +			sme_smstop_sm();

As per the SME syscall ABI

| On syscall PSTATE.SM will be cleared and the SVE registers will be
| handled as per the standard SVE ABI.

and the SVE syscall ABI

| On syscall, V0..V31 are preserved (as without SVE).  Thus, bits
| [127:0] of Z0..Z31 are preserved.  All other bits of Z0..Z31, and all
| of P0..P15 and FFR become zero on return from a syscall.

Can we infer from the documentation that V0-V31 should be preserved on
return from a syscall? But with sme_smstop_sm(), all implemented bits of
Z0-Z31 are set to zero by hardware. Is this intentional?

Please fix me up if I've mis-read things here.

> +	}
> +
>  	if (!system_supports_sve())
>  		return;
>  
> +	/*
> +	 * If SME is not active then disable SVE, the registers will
> +	 * be cleared when userspace next attempts to access them and
> +	 * we do not need to track the SVE register state until then.
> +	 */
>  	clear_thread_flag(TIF_SVE);
>  
>  	/*

Thanks,
Zenghui

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