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

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