Re: arch/arm64/kernel/fpsimd.c:1944:55: sparse: sparse: dereference of noderef expression

[Mark Brown <broonie@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 1784 bytes --]

On Wed, Jun 01, 2022 at 05:54:40PM +0100, Catalin Marinas wrote:
> On Wed, Jun 01, 2022 at 09:50:39AM +0200, Mark Brown wrote:
> > On Tue, May 31, 2022 at 05:57:42PM +0100, Catalin Marinas wrote:

> > Yes, that looks right - feel free to add my Reviewed-by or I'll
> > do that when I see the patch sent properly.

> I'm no longer convinced that's the right fix. In __efi_fpsimd_begin(),
> ffr is true if either !system_supports_fa64() or SVCR.SM is set. The FFR
> state would be saved in this case.

That's the wrong way round - we should only save FFR if we have
FA64 or SVCR.SM is *not* set.  Checking again efi_fpsimd_begin()
is wrong.

> In __efi_fpsimd_end(), shouldn't we just restore the FFR state if
> efi_sm_state, irrespective of system_supports_fa64() (as it was checked
> in the beginning). Something like:

No, we should restore it only if we saved it.  The below should
resolve things but it's completely untested:

diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
index 819979398127..3c66a061ff6f 100644
--- a/arch/arm64/kernel/fpsimd.c
+++ b/arch/arm64/kernel/fpsimd.c
@@ -1916,10 +1916,11 @@ void __efi_fpsimd_begin(void)
 			if (system_supports_sme()) {
 				svcr = read_sysreg_s(SYS_SVCR);
 
-				if (!system_supports_fa64())
-					ffr = svcr & SVCR_SM_MASK;
+				__this_cpu_write(efi_sm_state,
+						 svcr & SVCR_SM_MASK);
 
-				__this_cpu_write(efi_sm_state, ffr);
+				if (!system_supports_fa64())
+					ffr = !(svcr & SVCR_SM_MASK);
 			}
 
 			sve_save_state(sve_state + sve_ffr_offset(sve_max_vl()),
@@ -1965,7 +1966,7 @@ void __efi_fpsimd_end(void)
 							   0,
 							   SVCR_SM_MASK);
 					if (!system_supports_fa64())
-						ffr = efi_sm_state;
+						ffr = false;
 				}
 			}
 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]