All of lore.kernel.org
 help / color / mirror / Atom feed
* arch/arm64/kernel/fpsimd.c:1944:55: sparse: sparse: dereference of noderef expression
@ 2022-05-27 22:49 kernel test robot
  2022-05-31 16:57   ` Catalin Marinas
  0 siblings, 1 reply; 9+ messages in thread
From: kernel test robot @ 2022-05-27 22:49 UTC (permalink / raw)
  To: Mark Brown; +Cc: kbuild-all, linux-kernel, Catalin Marinas

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
head:   8291eaafed36f575f23951f3ce18407f480e9ecf
commit: e0838f6373e5cb72516fc4c26bba309097e2a80a arm64/sme: Save and restore streaming mode over EFI runtime calls
date:   5 weeks ago
config: arm64-randconfig-s032-20220527 (https://download.01.org/0day-ci/archive/20220528/202205280635.XKlMkfKD-lkp@intel.com/config)
compiler: aarch64-linux-gcc (GCC) 11.3.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # apt-get install sparse
        # sparse version: v0.6.4-14-g5a0004b5-dirty
        # https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=e0838f6373e5cb72516fc4c26bba309097e2a80a
        git remote add linus https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
        git fetch --no-tags linus master
        git checkout e0838f6373e5cb72516fc4c26bba309097e2a80a
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=arm64 SHELL=/bin/bash arch/arm64/kernel/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)
>> arch/arm64/kernel/fpsimd.c:1944:55: sparse: sparse: dereference of noderef expression

vim +1944 arch/arm64/kernel/fpsimd.c

  1916	
  1917	/*
  1918	 * __efi_fpsimd_end(): clean up FPSIMD after an EFI runtime services call
  1919	 */
  1920	void __efi_fpsimd_end(void)
  1921	{
  1922		if (!system_supports_fpsimd())
  1923			return;
  1924	
  1925		if (!__this_cpu_xchg(efi_fpsimd_state_used, false)) {
  1926			kernel_neon_end();
  1927		} else {
  1928			if (system_supports_sve() &&
  1929			    likely(__this_cpu_read(efi_sve_state_used))) {
  1930				char const *sve_state = this_cpu_ptr(efi_sve_state);
  1931				bool ffr = true;
  1932	
  1933				/*
  1934				 * Restore streaming mode; EFI calls are
  1935				 * normal function calls so should not return in
  1936				 * streaming mode.
  1937				 */
  1938				if (system_supports_sme()) {
  1939					if (__this_cpu_read(efi_sm_state)) {
  1940						sysreg_clear_set_s(SYS_SVCR_EL0,
  1941								   0,
  1942								   SYS_SVCR_EL0_SM_MASK);
  1943						if (!system_supports_fa64())
> 1944							ffr = efi_sm_state;
  1945					}
  1946				}
  1947	
  1948				sve_load_state(sve_state + sve_ffr_offset(sve_max_vl()),
  1949					       &this_cpu_ptr(&efi_fpsimd_state)->fpsr,
  1950					       ffr);
  1951	
  1952				__this_cpu_write(efi_sve_state_used, false);
  1953			} else {
  1954				fpsimd_load_state(this_cpu_ptr(&efi_fpsimd_state));
  1955			}
  1956		}
  1957	}
  1958	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: arch/arm64/kernel/fpsimd.c:1944:55: sparse: sparse: dereference of noderef expression
  2022-05-27 22:49 arch/arm64/kernel/fpsimd.c:1944:55: sparse: sparse: dereference of noderef expression kernel test robot
@ 2022-05-31 16:57   ` Catalin Marinas
  0 siblings, 0 replies; 9+ messages in thread
From: Catalin Marinas @ 2022-05-31 16:57 UTC (permalink / raw)
  To: kernel test robot; +Cc: Mark Brown, kbuild-all, linux-kernel

On Sat, May 28, 2022 at 06:49:01AM +0800, kernel test robot wrote:
> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> head:   8291eaafed36f575f23951f3ce18407f480e9ecf
> commit: e0838f6373e5cb72516fc4c26bba309097e2a80a arm64/sme: Save and restore streaming mode over EFI runtime calls
> date:   5 weeks ago
> config: arm64-randconfig-s032-20220527 (https://download.01.org/0day-ci/archive/20220528/202205280635.XKlMkfKD-lkp@intel.com/config)
> compiler: aarch64-linux-gcc (GCC) 11.3.0
> reproduce:
>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # apt-get install sparse
>         # sparse version: v0.6.4-14-g5a0004b5-dirty
>         # https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=e0838f6373e5cb72516fc4c26bba309097e2a80a
>         git remote add linus https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
>         git fetch --no-tags linus master
>         git checkout e0838f6373e5cb72516fc4c26bba309097e2a80a
>         # save the config file
>         mkdir build_dir && cp config build_dir/.config
>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=arm64 SHELL=/bin/bash arch/arm64/kernel/
> 
> If you fix the issue, kindly add following tag where applicable
> Reported-by: kernel test robot <lkp@intel.com>
> 
> 
> sparse warnings: (new ones prefixed by >>)
> >> arch/arm64/kernel/fpsimd.c:1944:55: sparse: sparse: dereference of noderef expression
> 
> vim +1944 arch/arm64/kernel/fpsimd.c
> 
>   1916	
>   1917	/*
>   1918	 * __efi_fpsimd_end(): clean up FPSIMD after an EFI runtime services call
>   1919	 */
>   1920	void __efi_fpsimd_end(void)
>   1921	{
>   1922		if (!system_supports_fpsimd())
>   1923			return;
>   1924	
>   1925		if (!__this_cpu_xchg(efi_fpsimd_state_used, false)) {
>   1926			kernel_neon_end();
>   1927		} else {
>   1928			if (system_supports_sve() &&
>   1929			    likely(__this_cpu_read(efi_sve_state_used))) {
>   1930				char const *sve_state = this_cpu_ptr(efi_sve_state);
>   1931				bool ffr = true;
>   1932	
>   1933				/*
>   1934				 * Restore streaming mode; EFI calls are
>   1935				 * normal function calls so should not return in
>   1936				 * streaming mode.
>   1937				 */
>   1938				if (system_supports_sme()) {
>   1939					if (__this_cpu_read(efi_sm_state)) {
>   1940						sysreg_clear_set_s(SYS_SVCR_EL0,
>   1941								   0,
>   1942								   SYS_SVCR_EL0_SM_MASK);
>   1943						if (!system_supports_fa64())
> > 1944							ffr = efi_sm_state;

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;
 				}
 			}
 
-- 
Catalin

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: arch/arm64/kernel/fpsimd.c:1944:55: sparse: sparse: dereference of noderef expression
@ 2022-05-31 16:57   ` Catalin Marinas
  0 siblings, 0 replies; 9+ messages in thread
From: Catalin Marinas @ 2022-05-31 16:57 UTC (permalink / raw)
  To: kbuild-all

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

On Sat, May 28, 2022 at 06:49:01AM +0800, kernel test robot wrote:
> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> head:   8291eaafed36f575f23951f3ce18407f480e9ecf
> commit: e0838f6373e5cb72516fc4c26bba309097e2a80a arm64/sme: Save and restore streaming mode over EFI runtime calls
> date:   5 weeks ago
> config: arm64-randconfig-s032-20220527 (https://download.01.org/0day-ci/archive/20220528/202205280635.XKlMkfKD-lkp(a)intel.com/config)
> compiler: aarch64-linux-gcc (GCC) 11.3.0
> reproduce:
>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # apt-get install sparse
>         # sparse version: v0.6.4-14-g5a0004b5-dirty
>         # https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=e0838f6373e5cb72516fc4c26bba309097e2a80a
>         git remote add linus https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
>         git fetch --no-tags linus master
>         git checkout e0838f6373e5cb72516fc4c26bba309097e2a80a
>         # save the config file
>         mkdir build_dir && cp config build_dir/.config
>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=arm64 SHELL=/bin/bash arch/arm64/kernel/
> 
> If you fix the issue, kindly add following tag where applicable
> Reported-by: kernel test robot <lkp@intel.com>
> 
> 
> sparse warnings: (new ones prefixed by >>)
> >> arch/arm64/kernel/fpsimd.c:1944:55: sparse: sparse: dereference of noderef expression
> 
> vim +1944 arch/arm64/kernel/fpsimd.c
> 
>   1916	
>   1917	/*
>   1918	 * __efi_fpsimd_end(): clean up FPSIMD after an EFI runtime services call
>   1919	 */
>   1920	void __efi_fpsimd_end(void)
>   1921	{
>   1922		if (!system_supports_fpsimd())
>   1923			return;
>   1924	
>   1925		if (!__this_cpu_xchg(efi_fpsimd_state_used, false)) {
>   1926			kernel_neon_end();
>   1927		} else {
>   1928			if (system_supports_sve() &&
>   1929			    likely(__this_cpu_read(efi_sve_state_used))) {
>   1930				char const *sve_state = this_cpu_ptr(efi_sve_state);
>   1931				bool ffr = true;
>   1932	
>   1933				/*
>   1934				 * Restore streaming mode; EFI calls are
>   1935				 * normal function calls so should not return in
>   1936				 * streaming mode.
>   1937				 */
>   1938				if (system_supports_sme()) {
>   1939					if (__this_cpu_read(efi_sm_state)) {
>   1940						sysreg_clear_set_s(SYS_SVCR_EL0,
>   1941								   0,
>   1942								   SYS_SVCR_EL0_SM_MASK);
>   1943						if (!system_supports_fa64())
> > 1944							ffr = efi_sm_state;

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;
 				}
 			}
 
-- 
Catalin

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: arch/arm64/kernel/fpsimd.c:1944:55: sparse: sparse: dereference of noderef expression
  2022-05-31 16:57   ` Catalin Marinas
@ 2022-06-01  7:50     ` Mark Brown
  -1 siblings, 0 replies; 9+ messages in thread
From: Mark Brown @ 2022-06-01  7:50 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: kernel test robot, kbuild-all, linux-kernel

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

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.

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: arch/arm64/kernel/fpsimd.c:1944:55: sparse: sparse: dereference of noderef expression
@ 2022-06-01  7:50     ` Mark Brown
  0 siblings, 0 replies; 9+ messages in thread
From: Mark Brown @ 2022-06-01  7:50 UTC (permalink / raw)
  To: kbuild-all

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

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.

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: arch/arm64/kernel/fpsimd.c:1944:55: sparse: sparse: dereference of noderef expression
  2022-06-01  7:50     ` Mark Brown
@ 2022-06-01 16:54       ` Catalin Marinas
  -1 siblings, 0 replies; 9+ messages in thread
From: Catalin Marinas @ 2022-06-01 16:54 UTC (permalink / raw)
  To: Mark Brown; +Cc: kernel test robot, kbuild-all, linux-kernel

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

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: arch/arm64/kernel/fpsimd.c:1944:55: sparse: sparse: dereference of noderef expression
@ 2022-06-01 16:54       ` Catalin Marinas
  0 siblings, 0 replies; 9+ messages in thread
From: Catalin Marinas @ 2022-06-01 16:54 UTC (permalink / raw)
  To: kbuild-all

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

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

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: arch/arm64/kernel/fpsimd.c:1944:55: sparse: sparse: dereference of noderef expression
  2022-06-01 16:54       ` Catalin Marinas
@ 2022-06-02  8:22         ` Mark Brown
  -1 siblings, 0 replies; 9+ messages in thread
From: Mark Brown @ 2022-06-02  8:22 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: kernel test robot, kbuild-all, linux-kernel

[-- 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 --]

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: arch/arm64/kernel/fpsimd.c:1944:55: sparse: sparse: dereference of noderef expression
@ 2022-06-02  8:22         ` Mark Brown
  0 siblings, 0 replies; 9+ messages in thread
From: Mark Brown @ 2022-06-02  8:22 UTC (permalink / raw)
  To: kbuild-all

[-- 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 --]

^ permalink raw reply related	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2022-06-02  8:23 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-27 22:49 arch/arm64/kernel/fpsimd.c:1944:55: sparse: sparse: dereference of noderef expression kernel test robot
2022-05-31 16:57 ` Catalin Marinas
2022-05-31 16:57   ` Catalin Marinas
2022-06-01  7:50   ` Mark Brown
2022-06-01  7:50     ` Mark Brown
2022-06-01 16:54     ` Catalin Marinas
2022-06-01 16:54       ` Catalin Marinas
2022-06-02  8:22       ` Mark Brown
2022-06-02  8:22         ` Mark Brown

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.