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

From: Catalin Marinas
Date: Wed Jun 01 2022 - 12:54:53 EST


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:
> > This looks wrong indeed. IIUC, something like below but waiting for Mark
> > to confirm:
> >
> > diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
> > index 819979398127..ac521b3403d6 100644
> > --- a/arch/arm64/kernel/fpsimd.c
> > +++ b/arch/arm64/kernel/fpsimd.c
> > @@ -1965,7 +1965,7 @@ void __efi_fpsimd_end(void)
> > 0,
> > SVCR_SM_MASK);
> > if (!system_supports_fa64())
> > - ffr = efi_sm_state;
> > + ffr = false;
>
> 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.

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:

diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
index 819979398127..8bc9080adab2 100644
--- a/arch/arm64/kernel/fpsimd.c
+++ b/arch/arm64/kernel/fpsimd.c
@@ -1960,13 +1960,12 @@ void __efi_fpsimd_end(void)
* streaming mode.
*/
if (system_supports_sme()) {
- if (__this_cpu_read(efi_sm_state)) {
+ if (__this_cpu_read(efi_sm_state))
sysreg_clear_set_s(SYS_SVCR,
0,
SVCR_SM_MASK);
- if (!system_supports_fa64())
- ffr = efi_sm_state;
- }
+ else
+ ffr = false;
}

sve_load_state(sve_state + sve_ffr_offset(sve_max_vl()),

--
Catalin