[PATCH] powerpc: Fix missing declaration of [en/dis]able_kernel_vsx()

Subsystems: linux for powerpc (32-bit and 64-bit), the rest

STALE1995d LANDED

Landed in mainline as bd73758803c2 on 2021-03-10.

8 messages, 3 authors, 2021-03-14 · open the first message on its own page

[PATCH] powerpc: Fix missing declaration of [en/dis]able_kernel_vsx()

From: Christophe Leroy <hidden>
Date: 2021-03-09 08:40:43

Add stub instances of enable_kernel_vsx() and disable_kernel_vsx()
when CONFIG_VSX is not set, to avoid following build failure.

  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.o
In file included from ./drivers/gpu/drm/amd/amdgpu/../display/dc/dm_services_types.h:29,
                 from ./drivers/gpu/drm/amd/amdgpu/../display/dc/dm_services.h:37,
                 from drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c:27:
drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c: In function 'dcn_bw_apply_registry_override':
./drivers/gpu/drm/amd/amdgpu/../display/dc/os_types.h:64:3: error: implicit declaration of function 'enable_kernel_vsx'; did you mean 'enable_kernel_fp'? [-Werror=implicit-function-declaration]
   64 |   enable_kernel_vsx(); \
      |   ^~~~~~~~~~~~~~~~~
drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c:640:2: note: in expansion of macro 'DC_FP_START'
  640 |  DC_FP_START();
      |  ^~~~~~~~~~~
./drivers/gpu/drm/amd/amdgpu/../display/dc/os_types.h:75:3: error: implicit declaration of function 'disable_kernel_vsx'; did you mean 'disable_kernel_fp'? [-Werror=implicit-function-declaration]
   75 |   disable_kernel_vsx(); \
      |   ^~~~~~~~~~~~~~~~~~
drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c:676:2: note: in expansion of macro 'DC_FP_END'
  676 |  DC_FP_END();
      |  ^~~~~~~~~
cc1: some warnings being treated as errors
make[5]: *** [drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.o] Error 1

Fixes: 16a9dea110a6 ("amdgpu: Enable initial DCN support on POWER")
Cc: stable@vger.kernel.org
Signed-off-by: Christophe Leroy <redacted>
---
 arch/powerpc/include/asm/switch_to.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/powerpc/include/asm/switch_to.h b/arch/powerpc/include/asm/switch_to.h
index fdab93428372..9d1fbd8be1c7 100644
--- a/arch/powerpc/include/asm/switch_to.h
+++ b/arch/powerpc/include/asm/switch_to.h

@@ -71,6 +71,16 @@ static inline void disable_kernel_vsx(void)
 {
 	msr_check_and_clear(MSR_FP|MSR_VEC|MSR_VSX);
 }
+#else
+static inline void enable_kernel_vsx(void)
+{
+	BUILD_BUG();
+}
+
+static inline void disable_kernel_vsx(void)
+{
+	BUILD_BUG();
+}
 #endif
 
 #ifdef CONFIG_SPE

-- 
2.25.0

Re: [PATCH] powerpc: Fix missing declaration of [en/dis]able_kernel_vsx()

From: Geert Uytterhoeven <geert@linux-m68k.org>
Date: 2021-03-09 08:46:11

Hi Christophe,

On Tue, Mar 9, 2021 at 9:39 AM Christophe Leroy
[off-list ref] wrote:

Add stub instances of enable_kernel_vsx() and disable_kernel_vsx()
when CONFIG_VSX is not set, to avoid following build failure.

  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.o
In file included from ./drivers/gpu/drm/amd/amdgpu/../display/dc/dm_services_types.h:29,
                 from ./drivers/gpu/drm/amd/amdgpu/../display/dc/dm_services.h:37,
                 from drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c:27:
drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c: In function 'dcn_bw_apply_registry_override':
./drivers/gpu/drm/amd/amdgpu/../display/dc/os_types.h:64:3: error: implicit declaration of function 'enable_kernel_vsx'; did you mean 'enable_kernel_fp'? [-Werror=implicit-function-declaration]
   64 |   enable_kernel_vsx(); \
      |   ^~~~~~~~~~~~~~~~~
drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c:640:2: note: in expansion of macro 'DC_FP_START'
  640 |  DC_FP_START();
      |  ^~~~~~~~~~~
./drivers/gpu/drm/amd/amdgpu/../display/dc/os_types.h:75:3: error: implicit declaration of function 'disable_kernel_vsx'; did you mean 'disable_kernel_fp'? [-Werror=implicit-function-declaration]
   75 |   disable_kernel_vsx(); \
      |   ^~~~~~~~~~~~~~~~~~
drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c:676:2: note: in expansion of macro 'DC_FP_END'
  676 |  DC_FP_END();
      |  ^~~~~~~~~
cc1: some warnings being treated as errors
make[5]: *** [drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.o] Error 1

Fixes: 16a9dea110a6 ("amdgpu: Enable initial DCN support on POWER")
Cc: stable@vger.kernel.org
Signed-off-by: Christophe Leroy <redacted>

Thanks for your patch!

quoted hunk

--- a/arch/powerpc/include/asm/switch_to.h
+++ b/arch/powerpc/include/asm/switch_to.h

@@ -71,6 +71,16 @@ static inline void disable_kernel_vsx(void)
 {
        msr_check_and_clear(MSR_FP|MSR_VEC|MSR_VSX);
 }
+#else
+static inline void enable_kernel_vsx(void)
+{
+       BUILD_BUG();
+}
+
+static inline void disable_kernel_vsx(void)
+{
+       BUILD_BUG();
+}
 #endif

I'm wondering how this is any better than the current situation: using
BUILD_BUG() will still cause a build failure?

What about adding "depends on !POWERPC || VSX" instead, to prevent
the issue from happening in the first place?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] powerpc: Fix missing declaration of [en/dis]able_kernel_vsx()

From: Christophe Leroy <hidden>
Date: 2021-03-09 08:53:03


Le 09/03/2021 à 09:45, Geert Uytterhoeven a écrit :

Hi Christophe,

On Tue, Mar 9, 2021 at 9:39 AM Christophe Leroy
[off-list ref] wrote:

quoted

Add stub instances of enable_kernel_vsx() and disable_kernel_vsx()
when CONFIG_VSX is not set, to avoid following build failure.

   CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.o
In file included from ./drivers/gpu/drm/amd/amdgpu/../display/dc/dm_services_types.h:29,
                  from ./drivers/gpu/drm/amd/amdgpu/../display/dc/dm_services.h:37,
                  from drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c:27:
drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c: In function 'dcn_bw_apply_registry_override':
./drivers/gpu/drm/amd/amdgpu/../display/dc/os_types.h:64:3: error: implicit declaration of function 'enable_kernel_vsx'; did you mean 'enable_kernel_fp'? [-Werror=implicit-function-declaration]
    64 |   enable_kernel_vsx(); \
       |   ^~~~~~~~~~~~~~~~~
drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c:640:2: note: in expansion of macro 'DC_FP_START'
   640 |  DC_FP_START();
       |  ^~~~~~~~~~~
./drivers/gpu/drm/amd/amdgpu/../display/dc/os_types.h:75:3: error: implicit declaration of function 'disable_kernel_vsx'; did you mean 'disable_kernel_fp'? [-Werror=implicit-function-declaration]
    75 |   disable_kernel_vsx(); \
       |   ^~~~~~~~~~~~~~~~~~
drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c:676:2: note: in expansion of macro 'DC_FP_END'
   676 |  DC_FP_END();
       |  ^~~~~~~~~
cc1: some warnings being treated as errors
make[5]: *** [drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.o] Error 1

Fixes: 16a9dea110a6 ("amdgpu: Enable initial DCN support on POWER")
Cc: stable@vger.kernel.org
Signed-off-by: Christophe Leroy <redacted>

Thanks for your patch!

quoted

--- a/arch/powerpc/include/asm/switch_to.h
+++ b/arch/powerpc/include/asm/switch_to.h

@@ -71,6 +71,16 @@ static inline void disable_kernel_vsx(void)
  {
         msr_check_and_clear(MSR_FP|MSR_VEC|MSR_VSX);
  }
+#else
+static inline void enable_kernel_vsx(void)
+{
+       BUILD_BUG();
+}
+
+static inline void disable_kernel_vsx(void)
+{
+       BUILD_BUG();
+}
  #endif

I'm wondering how this is any better than the current situation: using
BUILD_BUG() will still cause a build failure?

No it won't cause a failure. In drivers/gpu/drm/amd/display/dc/os_types.h you have:

#define DC_FP_START() { \
	if (cpu_has_feature(CPU_FTR_VSX_COMP)) { \
		preempt_disable(); \
		enable_kernel_vsx(); \
	} else if (cpu_has_feature(CPU_FTR_ALTIVEC_COMP)) { \
		preempt_disable(); \
		enable_kernel_altivec(); \
	} else if (!cpu_has_feature(CPU_FTR_FPU_UNAVAILABLE)) { \
		preempt_disable(); \
		enable_kernel_fp(); \
	} \

When CONFIG_VSX is not selected, cpu_has_feature(CPU_FTR_VSX_COMP) constant folds to 'false' so the 
call to enable_kernel_vsx() is discarded and the build succeeds.

What about adding "depends on !POWERPC || VSX" instead, to prevent
the issue from happening in the first place?

CONFIG_VSX is not required as pointed by the DC_FP_START() macro above and the matching DC_FP_END() 
macro.

Gr{oetje,eeting}s,

                         Geert


Christophe

Re: [PATCH] powerpc: Fix missing declaration of [en/dis]able_kernel_vsx()

From: Geert Uytterhoeven <geert@linux-m68k.org>
Date: 2021-03-09 09:17:15

Hi Christophe,

On Tue, Mar 9, 2021 at 9:52 AM Christophe Leroy
[off-list ref] wrote:

Le 09/03/2021 à 09:45, Geert Uytterhoeven a écrit :

quoted

On Tue, Mar 9, 2021 at 9:39 AM Christophe Leroy
[off-list ref] wrote:

quoted

Add stub instances of enable_kernel_vsx() and disable_kernel_vsx()
when CONFIG_VSX is not set, to avoid following build failure.

   CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.o
In file included from ./drivers/gpu/drm/amd/amdgpu/../display/dc/dm_services_types.h:29,
                  from ./drivers/gpu/drm/amd/amdgpu/../display/dc/dm_services.h:37,
                  from drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c:27:
drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c: In function 'dcn_bw_apply_registry_override':
./drivers/gpu/drm/amd/amdgpu/../display/dc/os_types.h:64:3: error: implicit declaration of function 'enable_kernel_vsx'; did you mean 'enable_kernel_fp'? [-Werror=implicit-function-declaration]
    64 |   enable_kernel_vsx(); \
       |   ^~~~~~~~~~~~~~~~~
drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c:640:2: note: in expansion of macro 'DC_FP_START'
   640 |  DC_FP_START();
       |  ^~~~~~~~~~~
./drivers/gpu/drm/amd/amdgpu/../display/dc/os_types.h:75:3: error: implicit declaration of function 'disable_kernel_vsx'; did you mean 'disable_kernel_fp'? [-Werror=implicit-function-declaration]
    75 |   disable_kernel_vsx(); \
       |   ^~~~~~~~~~~~~~~~~~
drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c:676:2: note: in expansion of macro 'DC_FP_END'
   676 |  DC_FP_END();
       |  ^~~~~~~~~
cc1: some warnings being treated as errors
make[5]: *** [drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.o] Error 1

Fixes: 16a9dea110a6 ("amdgpu: Enable initial DCN support on POWER")
Cc: stable@vger.kernel.org
Signed-off-by: Christophe Leroy <redacted>

Thanks for your patch!

quoted

--- a/arch/powerpc/include/asm/switch_to.h
+++ b/arch/powerpc/include/asm/switch_to.h

@@ -71,6 +71,16 @@ static inline void disable_kernel_vsx(void)
  {
         msr_check_and_clear(MSR_FP|MSR_VEC|MSR_VSX);
  }
+#else
+static inline void enable_kernel_vsx(void)
+{
+       BUILD_BUG();
+}
+
+static inline void disable_kernel_vsx(void)
+{
+       BUILD_BUG();
+}
  #endif

I'm wondering how this is any better than the current situation: using
BUILD_BUG() will still cause a build failure?

No it won't cause a failure. In drivers/gpu/drm/amd/display/dc/os_types.h you have:

#define DC_FP_START() { \
        if (cpu_has_feature(CPU_FTR_VSX_COMP)) { \
                preempt_disable(); \
                enable_kernel_vsx(); \
        } else if (cpu_has_feature(CPU_FTR_ALTIVEC_COMP)) { \
                preempt_disable(); \
                enable_kernel_altivec(); \
        } else if (!cpu_has_feature(CPU_FTR_FPU_UNAVAILABLE)) { \
                preempt_disable(); \
                enable_kernel_fp(); \
        } \

When CONFIG_VSX is not selected, cpu_has_feature(CPU_FTR_VSX_COMP) constant folds to 'false' so the
call to enable_kernel_vsx() is discarded and the build succeeds.

IC. So you might as well have an empty (dummy) function instead?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] powerpc: Fix missing declaration of [en/dis]able_kernel_vsx()

From: Christophe Leroy <hidden>
Date: 2021-03-09 09:59:08


Le 09/03/2021 à 10:16, Geert Uytterhoeven a écrit :

Hi Christophe,

On Tue, Mar 9, 2021 at 9:52 AM Christophe Leroy
[off-list ref] wrote:

quoted

Le 09/03/2021 à 09:45, Geert Uytterhoeven a écrit :

quoted

On Tue, Mar 9, 2021 at 9:39 AM Christophe Leroy
[off-list ref] wrote:

quoted

Add stub instances of enable_kernel_vsx() and disable_kernel_vsx()
when CONFIG_VSX is not set, to avoid following build failure.

    CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.o
In file included from ./drivers/gpu/drm/amd/amdgpu/../display/dc/dm_services_types.h:29,
                   from ./drivers/gpu/drm/amd/amdgpu/../display/dc/dm_services.h:37,
                   from drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c:27:
drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c: In function 'dcn_bw_apply_registry_override':
./drivers/gpu/drm/amd/amdgpu/../display/dc/os_types.h:64:3: error: implicit declaration of function 'enable_kernel_vsx'; did you mean 'enable_kernel_fp'? [-Werror=implicit-function-declaration]
     64 |   enable_kernel_vsx(); \
        |   ^~~~~~~~~~~~~~~~~
drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c:640:2: note: in expansion of macro 'DC_FP_START'
    640 |  DC_FP_START();
        |  ^~~~~~~~~~~
./drivers/gpu/drm/amd/amdgpu/../display/dc/os_types.h:75:3: error: implicit declaration of function 'disable_kernel_vsx'; did you mean 'disable_kernel_fp'? [-Werror=implicit-function-declaration]
     75 |   disable_kernel_vsx(); \
        |   ^~~~~~~~~~~~~~~~~~
drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c:676:2: note: in expansion of macro 'DC_FP_END'
    676 |  DC_FP_END();
        |  ^~~~~~~~~
cc1: some warnings being treated as errors
make[5]: *** [drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.o] Error 1

Fixes: 16a9dea110a6 ("amdgpu: Enable initial DCN support on POWER")
Cc: stable@vger.kernel.org
Signed-off-by: Christophe Leroy <redacted>

Thanks for your patch!

quoted

--- a/arch/powerpc/include/asm/switch_to.h
+++ b/arch/powerpc/include/asm/switch_to.h

@@ -71,6 +71,16 @@ static inline void disable_kernel_vsx(void)
   {
          msr_check_and_clear(MSR_FP|MSR_VEC|MSR_VSX);
   }
+#else
+static inline void enable_kernel_vsx(void)
+{
+       BUILD_BUG();
+}
+
+static inline void disable_kernel_vsx(void)
+{
+       BUILD_BUG();
+}
   #endif

I'm wondering how this is any better than the current situation: using
BUILD_BUG() will still cause a build failure?

No it won't cause a failure. In drivers/gpu/drm/amd/display/dc/os_types.h you have:

#define DC_FP_START() { \
         if (cpu_has_feature(CPU_FTR_VSX_COMP)) { \
                 preempt_disable(); \
                 enable_kernel_vsx(); \
         } else if (cpu_has_feature(CPU_FTR_ALTIVEC_COMP)) { \
                 preempt_disable(); \
                 enable_kernel_altivec(); \
         } else if (!cpu_has_feature(CPU_FTR_FPU_UNAVAILABLE)) { \
                 preempt_disable(); \
                 enable_kernel_fp(); \
         } \

When CONFIG_VSX is not selected, cpu_has_feature(CPU_FTR_VSX_COMP) constant folds to 'false' so the
call to enable_kernel_vsx() is discarded and the build succeeds.

IC. So you might as well have an empty (dummy) function instead?

But with an empty function, you take the risk that one day, someone calls it without checking that 
CONFIG_VSX is selected. Here if someone does that, build will fail.

Another solution is to declare a non static prototype of it, like __put_user_bad() for instance. In 
that case, the link will fail.

I prefer the BUILD_BUG() approach as I find it cleaner and more explicit, and also because it breaks 
the build at compile time, you don't have to wait link time to catch the error.

Christophe

Re: [PATCH] powerpc: Fix missing declaration of [en/dis]able_kernel_vsx()

From: Geert Uytterhoeven <geert@linux-m68k.org>
Date: 2021-03-09 10:56:26

Hi Christophe,

On Tue, Mar 9, 2021 at 10:58 AM Christophe Leroy
[off-list ref] wrote:

Le 09/03/2021 à 10:16, Geert Uytterhoeven a écrit :

quoted

On Tue, Mar 9, 2021 at 9:52 AM Christophe Leroy
[off-list ref] wrote:

quoted

Le 09/03/2021 à 09:45, Geert Uytterhoeven a écrit :

quoted

On Tue, Mar 9, 2021 at 9:39 AM Christophe Leroy
[off-list ref] wrote:

quoted

Add stub instances of enable_kernel_vsx() and disable_kernel_vsx()
when CONFIG_VSX is not set, to avoid following build failure.

    CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.o
In file included from ./drivers/gpu/drm/amd/amdgpu/../display/dc/dm_services_types.h:29,
                   from ./drivers/gpu/drm/amd/amdgpu/../display/dc/dm_services.h:37,
                   from drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c:27:
drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c: In function 'dcn_bw_apply_registry_override':
./drivers/gpu/drm/amd/amdgpu/../display/dc/os_types.h:64:3: error: implicit declaration of function 'enable_kernel_vsx'; did you mean 'enable_kernel_fp'? [-Werror=implicit-function-declaration]
     64 |   enable_kernel_vsx(); \
        |   ^~~~~~~~~~~~~~~~~
drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c:640:2: note: in expansion of macro 'DC_FP_START'
    640 |  DC_FP_START();
        |  ^~~~~~~~~~~
./drivers/gpu/drm/amd/amdgpu/../display/dc/os_types.h:75:3: error: implicit declaration of function 'disable_kernel_vsx'; did you mean 'disable_kernel_fp'? [-Werror=implicit-function-declaration]
     75 |   disable_kernel_vsx(); \
        |   ^~~~~~~~~~~~~~~~~~
drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c:676:2: note: in expansion of macro 'DC_FP_END'
    676 |  DC_FP_END();
        |  ^~~~~~~~~
cc1: some warnings being treated as errors
make[5]: *** [drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.o] Error 1

Fixes: 16a9dea110a6 ("amdgpu: Enable initial DCN support on POWER")
Cc: stable@vger.kernel.org
Signed-off-by: Christophe Leroy <redacted>

Thanks for your patch!

quoted

--- a/arch/powerpc/include/asm/switch_to.h
+++ b/arch/powerpc/include/asm/switch_to.h

@@ -71,6 +71,16 @@ static inline void disable_kernel_vsx(void)
   {
          msr_check_and_clear(MSR_FP|MSR_VEC|MSR_VSX);
   }
+#else
+static inline void enable_kernel_vsx(void)
+{
+       BUILD_BUG();
+}
+
+static inline void disable_kernel_vsx(void)
+{
+       BUILD_BUG();
+}
   #endif

I'm wondering how this is any better than the current situation: using
BUILD_BUG() will still cause a build failure?

No it won't cause a failure. In drivers/gpu/drm/amd/display/dc/os_types.h you have:

#define DC_FP_START() { \
         if (cpu_has_feature(CPU_FTR_VSX_COMP)) { \
                 preempt_disable(); \
                 enable_kernel_vsx(); \
         } else if (cpu_has_feature(CPU_FTR_ALTIVEC_COMP)) { \
                 preempt_disable(); \
                 enable_kernel_altivec(); \
         } else if (!cpu_has_feature(CPU_FTR_FPU_UNAVAILABLE)) { \
                 preempt_disable(); \
                 enable_kernel_fp(); \
         } \

When CONFIG_VSX is not selected, cpu_has_feature(CPU_FTR_VSX_COMP) constant folds to 'false' so the
call to enable_kernel_vsx() is discarded and the build succeeds.

IC. So you might as well have an empty (dummy) function instead?

But with an empty function, you take the risk that one day, someone calls it without checking that
CONFIG_VSX is selected. Here if someone does that, build will fail.

OK, convinced.

Another solution is to declare a non static prototype of it, like __put_user_bad() for instance. In
that case, the link will fail.

I prefer the BUILD_BUG() approach as I find it cleaner and more explicit, and also because it breaks
the build at compile time, you don't have to wait link time to catch the error.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] powerpc: Fix missing declaration of [en/dis]able_kernel_vsx()

From: Christophe Leroy <hidden>
Date: 2021-03-10 13:34:04

Hi Geert,

Le 09/03/2021 à 11:55, Geert Uytterhoeven a écrit :

Hi Christophe,

On Tue, Mar 9, 2021 at 10:58 AM Christophe Leroy
[off-list ref] wrote:

quoted

Le 09/03/2021 à 10:16, Geert Uytterhoeven a écrit :

quoted

On Tue, Mar 9, 2021 at 9:52 AM Christophe Leroy
[off-list ref] wrote:

quoted

Le 09/03/2021 à 09:45, Geert Uytterhoeven a écrit :

quoted

On Tue, Mar 9, 2021 at 9:39 AM Christophe Leroy
[off-list ref] wrote:

quoted

Add stub instances of enable_kernel_vsx() and disable_kernel_vsx()
when CONFIG_VSX is not set, to avoid following build failure.

     CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.o
In file included from ./drivers/gpu/drm/amd/amdgpu/../display/dc/dm_services_types.h:29,
                    from ./drivers/gpu/drm/amd/amdgpu/../display/dc/dm_services.h:37,
                    from drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c:27:
drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c: In function 'dcn_bw_apply_registry_override':
./drivers/gpu/drm/amd/amdgpu/../display/dc/os_types.h:64:3: error: implicit declaration of function 'enable_kernel_vsx'; did you mean 'enable_kernel_fp'? [-Werror=implicit-function-declaration]
      64 |   enable_kernel_vsx(); \
         |   ^~~~~~~~~~~~~~~~~
drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c:640:2: note: in expansion of macro 'DC_FP_START'
     640 |  DC_FP_START();
         |  ^~~~~~~~~~~
./drivers/gpu/drm/amd/amdgpu/../display/dc/os_types.h:75:3: error: implicit declaration of function 'disable_kernel_vsx'; did you mean 'disable_kernel_fp'? [-Werror=implicit-function-declaration]
      75 |   disable_kernel_vsx(); \
         |   ^~~~~~~~~~~~~~~~~~
drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c:676:2: note: in expansion of macro 'DC_FP_END'
     676 |  DC_FP_END();
         |  ^~~~~~~~~
cc1: some warnings being treated as errors
make[5]: *** [drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.o] Error 1

Fixes: 16a9dea110a6 ("amdgpu: Enable initial DCN support on POWER")
Cc: stable@vger.kernel.org
Signed-off-by: Christophe Leroy <redacted>

Thanks for your patch!

quoted

--- a/arch/powerpc/include/asm/switch_to.h
+++ b/arch/powerpc/include/asm/switch_to.h

@@ -71,6 +71,16 @@ static inline void disable_kernel_vsx(void)
    {
           msr_check_and_clear(MSR_FP|MSR_VEC|MSR_VSX);
    }
+#else
+static inline void enable_kernel_vsx(void)
+{
+       BUILD_BUG();
+}
+
+static inline void disable_kernel_vsx(void)
+{
+       BUILD_BUG();
+}
    #endif

I'm wondering how this is any better than the current situation: using
BUILD_BUG() will still cause a build failure?

No it won't cause a failure. In drivers/gpu/drm/amd/display/dc/os_types.h you have:

#define DC_FP_START() { \
          if (cpu_has_feature(CPU_FTR_VSX_COMP)) { \
                  preempt_disable(); \
                  enable_kernel_vsx(); \
          } else if (cpu_has_feature(CPU_FTR_ALTIVEC_COMP)) { \
                  preempt_disable(); \
                  enable_kernel_altivec(); \
          } else if (!cpu_has_feature(CPU_FTR_FPU_UNAVAILABLE)) { \
                  preempt_disable(); \
                  enable_kernel_fp(); \
          } \

When CONFIG_VSX is not selected, cpu_has_feature(CPU_FTR_VSX_COMP) constant folds to 'false' so the
call to enable_kernel_vsx() is discarded and the build succeeds.

IC. So you might as well have an empty (dummy) function instead?

But with an empty function, you take the risk that one day, someone calls it without checking that
CONFIG_VSX is selected. Here if someone does that, build will fail.

OK, convinced.

Note that following build test performed on kisskb, with gcc 4.9 the following change is required in 
addition: 
https://patchwork.ozlabs.org/project/linuxppc-dev/patch/b231dfa040ce4cc37f702f5c3a595fdeabfe0462.1615378209.git.christophe.leroy@csgroup.eu/

Christophe

Re: [PATCH] powerpc: Fix missing declaration of [en/dis]able_kernel_vsx()

From: Michael Ellerman <hidden>
Date: 2021-03-14 10:12:59

On Tue, 9 Mar 2021 08:39:39 +0000 (UTC), Christophe Leroy wrote:

Add stub instances of enable_kernel_vsx() and disable_kernel_vsx()
when CONFIG_VSX is not set, to avoid following build failure.

  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.o
In file included from ./drivers/gpu/drm/amd/amdgpu/../display/dc/dm_services_types.h:29,
                 from ./drivers/gpu/drm/amd/amdgpu/../display/dc/dm_services.h:37,
                 from drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c:27:
drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c: In function 'dcn_bw_apply_registry_override':
./drivers/gpu/drm/amd/amdgpu/../display/dc/os_types.h:64:3: error: implicit declaration of function 'enable_kernel_vsx'; did you mean 'enable_kernel_fp'? [-Werror=implicit-function-declaration]
   64 |   enable_kernel_vsx(); \
      |   ^~~~~~~~~~~~~~~~~
drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c:640:2: note: in expansion of macro 'DC_FP_START'
  640 |  DC_FP_START();
      |  ^~~~~~~~~~~
./drivers/gpu/drm/amd/amdgpu/../display/dc/os_types.h:75:3: error: implicit declaration of function 'disable_kernel_vsx'; did you mean 'disable_kernel_fp'? [-Werror=implicit-function-declaration]
   75 |   disable_kernel_vsx(); \
      |   ^~~~~~~~~~~~~~~~~~
drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.c:676:2: note: in expansion of macro 'DC_FP_END'
  676 |  DC_FP_END();
      |  ^~~~~~~~~
cc1: some warnings being treated as errors
make[5]: *** [drivers/gpu/drm/amd/amdgpu/../display/dc/calcs/dcn_calcs.o] Error 1

Applied to powerpc/fixes.

[1/1] powerpc: Fix missing declaration of [en/dis]able_kernel_vsx()
      https://git.kernel.org/powerpc/c/bd73758803c2eedc037c2268b65a19542a832594

cheers

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help