PPC_FPU, ALTIVEC: enable_kernel_fp, put_vr, get_vr

[Michael Ellerman]

Christophe Leroy <christophe.leroy at csgroup.eu> writes:
> Le 19/04/2021 à 23:39, Randy Dunlap a écrit :
>> On 4/19/21 6:16 AM, Michael Ellerman wrote:
>>> Randy Dunlap <rdunlap at infradead.org> writes:
>> 
>>>> Sure.  I'll post them later today.
>>>> They keep FPU and ALTIVEC as independent (build) features.
>>>
>>> Those patches look OK.
>>>
>>> But I don't think it makes sense to support that configuration, FPU=n
>>> ALTVEC=y. No one is ever going to make a CPU like that. We have enough
>>> testing surface due to configuration options, without adding artificial
>>> combinations that no one is ever going to use.
>>>
>>> IMHO :)
>>>
>>> So I'd rather we just make ALTIVEC depend on FPU.
>> 
>> That's rather simple. See below.
>> I'm doing a bunch of randconfig builds with it now.
>> 
>> ---
>> From: Randy Dunlap <rdunlap at infradead.org>
>> Subject: [PATCH] powerpc: make ALTIVEC depend PPC_FPU
>> 
>> On a kernel config with ALTIVEC=y and PPC_FPU not set/enabled,
>> there are build errors:
>> 
>> drivers/cpufreq/pmac32-cpufreq.c:262:2: error: implicit declaration of function 'enable_kernel_fp' [-Werror,-Wimplicit-function-declaration]
>>             enable_kernel_fp();
>> ../arch/powerpc/lib/sstep.c: In function 'do_vec_load':
>> ../arch/powerpc/lib/sstep.c:637:3: error: implicit declaration of function 'put_vr' [-Werror=implicit-function-declaration]
>>    637 |   put_vr(rn, &u.v);
>>        |   ^~~~~~
>> ../arch/powerpc/lib/sstep.c: In function 'do_vec_store':
>> ../arch/powerpc/lib/sstep.c:660:3: error: implicit declaration of function 'get_vr'; did you mean 'get_oc'? [-Werror=implicit-function-declaration]
>>    660 |   get_vr(rn, &u.v);
>>        |   ^~~~~~
>> 
>> In theory ALTIVEC is independent of PPC_FPU but in practice nobody
>> is going to build such a machine, so make ALTIVEC require PPC_FPU
>> by depending on PPC_FPU.
>> 
>> Signed-off-by: Randy Dunlap <rdunlap at infradead.org>
>> Reported-by: kernel test robot <lkp at intel.com>
>> Cc: Michael Ellerman <mpe at ellerman.id.au>
>> Cc: linuxppc-dev at lists.ozlabs.org
>> Cc: Christophe Leroy <christophe.leroy at csgroup.eu>
>> Cc: Segher Boessenkool <segher at kernel.crashing.org>
>> Cc: lkp at intel.com
>> ---
>>   arch/powerpc/platforms/86xx/Kconfig    |    1 +
>>   arch/powerpc/platforms/Kconfig.cputype |    2 ++
>>   2 files changed, 3 insertions(+)
>> 
>> --- linux-next-20210416.orig/arch/powerpc/platforms/86xx/Kconfig
>> +++ linux-next-20210416/arch/powerpc/platforms/86xx/Kconfig
>> @@ -4,6 +4,7 @@ menuconfig PPC_86xx
>>   	bool "86xx-based boards"
>>   	depends on PPC_BOOK3S_32
>>   	select FSL_SOC
>> +	select PPC_FPU
>>   	select ALTIVEC
>>   	help
>>   	  The Freescale E600 SoCs have 74xx cores.
>> --- linux-next-20210416.orig/arch/powerpc/platforms/Kconfig.cputype
>> +++ linux-next-20210416/arch/powerpc/platforms/Kconfig.cputype
>> @@ -186,6 +186,7 @@ config E300C3_CPU
>>   config G4_CPU
>>   	bool "G4 (74xx)"
>>   	depends on PPC_BOOK3S_32
>> +	select PPC_FPU
>>   	select ALTIVEC
>>   
>>   endchoice
>> @@ -309,6 +310,7 @@ config PHYS_64BIT
>>   
>>   config ALTIVEC
>>   	bool "AltiVec Support"
>> +	depends on PPC_FPU
>
> Shouldn't we do it the other way round ? In extenso make ALTIVEC select PPC_FPU and avoid the two 
> selects that are above ?

Yes, ALTIVEC should select PPC_FPU.

The latter is (generally) not user selectable, so there's no issue with
selecting it, whereas the reverse is not true.

For 64-bit Book3S I think we could just always enable ALTIVEC these
days. It's only Power5 that doesn't have it, and essentially no one is
running mainline on those AFAIK. But that can be done separately.

cheers