Re: PPC_FPU, ALTIVEC: enable_kernel_fp, put_vr, get_vr

[Michael Ellerman <mpe@ellerman.id.au>]

Christophe Leroy <christophe.leroy@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@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@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@infradead.org>
>> Reported-by: kernel test robot <lkp@intel.com>
>> Cc: Michael Ellerman <mpe@ellerman.id.au>
>> Cc: linuxppc-dev@lists.ozlabs.org
>> Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
>> Cc: Segher Boessenkool <segher@kernel.crashing.org>
>> Cc: lkp@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