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

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