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

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

[Christophe Leroy]

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 <christophe.leroy@csgroup.eu>
---
 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()

[Geert Uytterhoeven]

Hi Christophe,

On Tue, Mar 9, 2021 at 9:39 AM Christophe Leroy
<christophe.leroy@csgroup.eu> 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 <christophe.leroy@csgroup.eu>

Thanks for your patch!

> --- 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()

[Christophe Leroy]

Le 09/03/2021 à 09:45, Geert Uytterhoeven a écrit :
> Hi Christophe,
> 
> On Tue, Mar 9, 2021 at 9:39 AM Christophe Leroy
> <christophe.leroy@csgroup.eu> 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 <christophe.leroy@csgroup.eu>
> 
> Thanks for your patch!
> 
>> --- 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()

[Geert Uytterhoeven]

Hi Christophe,

On Tue, Mar 9, 2021 at 9:52 AM Christophe Leroy
<christophe.leroy@csgroup.eu> wrote:
> Le 09/03/2021 à 09:45, Geert Uytterhoeven a écrit :
> > On Tue, Mar 9, 2021 at 9:39 AM Christophe Leroy
> > <christophe.leroy@csgroup.eu> 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 <christophe.leroy@csgroup.eu>
> >
> > Thanks for your patch!
> >
> >> --- 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()

[Christophe Leroy]

Le 09/03/2021 à 10:16, Geert Uytterhoeven a écrit :
> Hi Christophe,
> 
> On Tue, Mar 9, 2021 at 9:52 AM Christophe Leroy
> <christophe.leroy@csgroup.eu> wrote:
>> Le 09/03/2021 à 09:45, Geert Uytterhoeven a écrit :
>>> On Tue, Mar 9, 2021 at 9:39 AM Christophe Leroy
>>> <christophe.leroy@csgroup.eu> 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 <christophe.leroy@csgroup.eu>
>>>
>>> Thanks for your patch!
>>>
>>>> --- 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()

[Geert Uytterhoeven]

Hi Christophe,

On Tue, Mar 9, 2021 at 10:58 AM Christophe Leroy
<christophe.leroy@csgroup.eu> wrote:
> Le 09/03/2021 à 10:16, Geert Uytterhoeven a écrit :
> > On Tue, Mar 9, 2021 at 9:52 AM Christophe Leroy
> > <christophe.leroy@csgroup.eu> wrote:
> >> Le 09/03/2021 à 09:45, Geert Uytterhoeven a écrit :
> >>> On Tue, Mar 9, 2021 at 9:39 AM Christophe Leroy
> >>> <christophe.leroy@csgroup.eu> 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 <christophe.leroy@csgroup.eu>
> >>>
> >>> Thanks for your patch!
> >>>
> >>>> --- 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()

[Christophe Leroy]

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
> <christophe.leroy@csgroup.eu> wrote:
>> Le 09/03/2021 à 10:16, Geert Uytterhoeven a écrit :
>>> On Tue, Mar 9, 2021 at 9:52 AM Christophe Leroy
>>> <christophe.leroy@csgroup.eu> wrote:
>>>> Le 09/03/2021 à 09:45, Geert Uytterhoeven a écrit :
>>>>> On Tue, Mar 9, 2021 at 9:39 AM Christophe Leroy
>>>>> <christophe.leroy@csgroup.eu> 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 <christophe.leroy@csgroup.eu>
>>>>>
>>>>> Thanks for your patch!
>>>>>
>>>>>> --- 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()

[Michael Ellerman]

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