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: [email protected]
Signed-off-by: Christophe Leroy <[email protected]>
---
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
Hi Christophe,
On Tue, Mar 9, 2021 at 9:39 AM Christophe Leroy
<[email protected]> 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: [email protected]
> Signed-off-by: Christophe Leroy <[email protected]>
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 -- [email protected]
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
Le 09/03/2021 à 09:45, Geert Uytterhoeven a écrit :
> Hi Christophe,
>
> On Tue, Mar 9, 2021 at 9:39 AM Christophe Leroy
> <[email protected]> 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: [email protected]
>> Signed-off-by: Christophe Leroy <[email protected]>
>
> 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
Hi Christophe,
On Tue, Mar 9, 2021 at 9:52 AM Christophe Leroy
<[email protected]> wrote:
> Le 09/03/2021 à 09:45, Geert Uytterhoeven a écrit :
> > On Tue, Mar 9, 2021 at 9:39 AM Christophe Leroy
> > <[email protected]> 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: [email protected]
> >> Signed-off-by: Christophe Leroy <[email protected]>
> >
> > 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 -- [email protected]
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
Le 09/03/2021 à 10:16, Geert Uytterhoeven a écrit :
> Hi Christophe,
>
> On Tue, Mar 9, 2021 at 9:52 AM Christophe Leroy
> <[email protected]> wrote:
>> Le 09/03/2021 à 09:45, Geert Uytterhoeven a écrit :
>>> On Tue, Mar 9, 2021 at 9:39 AM Christophe Leroy
>>> <[email protected]> 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: [email protected]
>>>> Signed-off-by: Christophe Leroy <[email protected]>
>>>
>>> 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
Hi Christophe,
On Tue, Mar 9, 2021 at 10:58 AM Christophe Leroy
<[email protected]> wrote:
> Le 09/03/2021 à 10:16, Geert Uytterhoeven a écrit :
> > On Tue, Mar 9, 2021 at 9:52 AM Christophe Leroy
> > <[email protected]> wrote:
> >> Le 09/03/2021 à 09:45, Geert Uytterhoeven a écrit :
> >>> On Tue, Mar 9, 2021 at 9:39 AM Christophe Leroy
> >>> <[email protected]> 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: [email protected]
> >>>> Signed-off-by: Christophe Leroy <[email protected]>
> >>>
> >>> 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 -- [email protected]
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
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
> <[email protected]> wrote:
>> Le 09/03/2021 à 10:16, Geert Uytterhoeven a écrit :
>>> On Tue, Mar 9, 2021 at 9:52 AM Christophe Leroy
>>> <[email protected]> wrote:
>>>> Le 09/03/2021 à 09:45, Geert Uytterhoeven a écrit :
>>>>> On Tue, Mar 9, 2021 at 9:39 AM Christophe Leroy
>>>>> <[email protected]> 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: [email protected]
>>>>>> Signed-off-by: Christophe Leroy <[email protected]>
>>>>>
>>>>> 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
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