LinuxPPC-Dev Archive on lore.kernel.org
 help / color / Atom feed
* PPC_FPU, ALTIVEC: enable_kernel_fp, put_vr, get_vr
@ 2021-04-17 20:17 Randy Dunlap
  2021-04-18 16:24 ` Christophe Leroy
  0 siblings, 1 reply; 10+ messages in thread
From: Randy Dunlap @ 2021-04-17 20:17 UTC (permalink / raw)
  To: PowerPC; +Cc: LKML

Hi,

kernel test robot reports:

>> drivers/cpufreq/pmac32-cpufreq.c:262:2: error: implicit declaration of function 'enable_kernel_fp' [-Werror,-Wimplicit-function-declaration]
           enable_kernel_fp();
           ^

when
# CONFIG_PPC_FPU is not set
CONFIG_ALTIVEC=y

I see at least one other place that does not handle that
combination well, here:

../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);
      |   ^~~~~~


Should the code + Kconfigs/Makefiles handle that kind of
kernel config or should ALTIVEC always mean PPC_FPU as well?

I have patches to fix the build errors with the config as
reported but I don't know if that's the right thing to do...

thanks.
-- 
~Randy


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: PPC_FPU, ALTIVEC: enable_kernel_fp, put_vr, get_vr
  2021-04-17 20:17 PPC_FPU, ALTIVEC: enable_kernel_fp, put_vr, get_vr Randy Dunlap
@ 2021-04-18 16:24 ` Christophe Leroy
  2021-04-18 17:46   ` Segher Boessenkool
  0 siblings, 1 reply; 10+ messages in thread
From: Christophe Leroy @ 2021-04-18 16:24 UTC (permalink / raw)
  To: Randy Dunlap, PowerPC; +Cc: LKML



Le 17/04/2021 à 22:17, Randy Dunlap a écrit :
> Hi,
> 
> kernel test robot reports:
> 
>>> drivers/cpufreq/pmac32-cpufreq.c:262:2: error: implicit declaration of function 'enable_kernel_fp' [-Werror,-Wimplicit-function-declaration]
>             enable_kernel_fp();
>             ^
> 
> when
> # CONFIG_PPC_FPU is not set
> CONFIG_ALTIVEC=y
> 
> I see at least one other place that does not handle that
> combination well, here:
> 
> ../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);
>        |   ^~~~~~
> 
> 
> Should the code + Kconfigs/Makefiles handle that kind of
> kernel config or should ALTIVEC always mean PPC_FPU as well?

As far as I understand, Altivec is completely independant of FPU in Theory. So it should be possible 
to use Altivec without using FPU.

However, until recently, it was not possible to de-activate FPU support on book3s/32. I made it 
possible in order to reduce unneccessary processing on processors like the 832x that has no FPU.
As far as I can see in cputable.h/.c, 832x is the only book3s/32 without FPU, and it doesn't have 
ALTIVEC either.

So we can in the future ensure that Altivec can be used without FPU support, but for the time being 
I think it is OK to force selection of FPU when selecting ALTIVEC in order to avoid build failures.

> 
> I have patches to fix the build errors with the config as
> reported but I don't know if that's the right thing to do...
> 

Lets see them.

Christophe

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: PPC_FPU, ALTIVEC: enable_kernel_fp, put_vr, get_vr
  2021-04-18 16:24 ` Christophe Leroy
@ 2021-04-18 17:46   ` Segher Boessenkool
  2021-04-18 17:59     ` Randy Dunlap
  0 siblings, 1 reply; 10+ messages in thread
From: Segher Boessenkool @ 2021-04-18 17:46 UTC (permalink / raw)
  To: Christophe Leroy; +Cc: Randy Dunlap, PowerPC, LKML

On Sun, Apr 18, 2021 at 06:24:29PM +0200, Christophe Leroy wrote:
> Le 17/04/2021 à 22:17, Randy Dunlap a écrit :
> >Should the code + Kconfigs/Makefiles handle that kind of
> >kernel config or should ALTIVEC always mean PPC_FPU as well?
> 
> As far as I understand, Altivec is completely independant of FPU in Theory. 

And, as far as the hardware is concerned, in practice as well.

> So it should be possible to use Altivec without using FPU.

Yup.

> However, until recently, it was not possible to de-activate FPU support on 
> book3s/32. I made it possible in order to reduce unneccessary processing on 
> processors like the 832x that has no FPU.

The processor has to implement FP to be compliant to any version of
PowerPC, as far as I know?  So that is all done by emulation, including
all the registers?  Wow painful.

> As far as I can see in cputable.h/.c, 832x is the only book3s/32 without 
> FPU, and it doesn't have ALTIVEC either.

602 doesn't have double-precision hardware, also no 64-bit FP registers.
But that CPU was never any widely used :-)

> So we can in the future ensure that Altivec can be used without FPU 
> support, but for the time being I think it is OK to force selection of FPU 
> when selecting ALTIVEC in order to avoid build failures.

It is useful to allow MSR[VEC,FP]=1,0 but yeah there are no CPUs that
have VMX (aka AltiVec) but that do not have FP.  I don't see how making
that artificial dependency buys anything, but maybe it does?

> >I have patches to fix the build errors with the config as
> >reported but I don't know if that's the right thing to do...

Neither do we, we cannot see those patches :-)


Segher

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: PPC_FPU, ALTIVEC: enable_kernel_fp, put_vr, get_vr
  2021-04-18 17:46   ` Segher Boessenkool
@ 2021-04-18 17:59     ` Randy Dunlap
  2021-04-19 13:16       ` Michael Ellerman
  0 siblings, 1 reply; 10+ messages in thread
From: Randy Dunlap @ 2021-04-18 17:59 UTC (permalink / raw)
  To: Segher Boessenkool, Christophe Leroy; +Cc: PowerPC, LKML

On 4/18/21 10:46 AM, Segher Boessenkool wrote:
> On Sun, Apr 18, 2021 at 06:24:29PM +0200, Christophe Leroy wrote:
>> Le 17/04/2021 à 22:17, Randy Dunlap a écrit :
>>> Should the code + Kconfigs/Makefiles handle that kind of
>>> kernel config or should ALTIVEC always mean PPC_FPU as well?
>>
>> As far as I understand, Altivec is completely independant of FPU in Theory. 
> 
> And, as far as the hardware is concerned, in practice as well.
> 
>> So it should be possible to use Altivec without using FPU.
> 
> Yup.
> 
>> However, until recently, it was not possible to de-activate FPU support on 
>> book3s/32. I made it possible in order to reduce unneccessary processing on 
>> processors like the 832x that has no FPU.
> 
> The processor has to implement FP to be compliant to any version of
> PowerPC, as far as I know?  So that is all done by emulation, including
> all the registers?  Wow painful.
> 
>> As far as I can see in cputable.h/.c, 832x is the only book3s/32 without 
>> FPU, and it doesn't have ALTIVEC either.
> 
> 602 doesn't have double-precision hardware, also no 64-bit FP registers.
> But that CPU was never any widely used :-)
> 
>> So we can in the future ensure that Altivec can be used without FPU 
>> support, but for the time being I think it is OK to force selection of FPU 
>> when selecting ALTIVEC in order to avoid build failures.
> 
> It is useful to allow MSR[VEC,FP]=1,0 but yeah there are no CPUs that
> have VMX (aka AltiVec) but that do not have FP.  I don't see how making
> that artificial dependency buys anything, but maybe it does?
> 
>>> I have patches to fix the build errors with the config as
>>> reported but I don't know if that's the right thing to do...
> 
> Neither do we, we cannot see those patches :-)

Sure.  I'll post them later today.
They keep FPU and ALTIVEC as independent (build) features.

-- 
~Randy


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: PPC_FPU, ALTIVEC: enable_kernel_fp, put_vr, get_vr
  2021-04-18 17:59     ` Randy Dunlap
@ 2021-04-19 13:16       ` Michael Ellerman
  2021-04-19 17:59         ` Randy Dunlap
  2021-04-19 21:39         ` Randy Dunlap
  0 siblings, 2 replies; 10+ messages in thread
From: Michael Ellerman @ 2021-04-19 13:16 UTC (permalink / raw)
  To: Randy Dunlap, Segher Boessenkool, Christophe Leroy; +Cc: PowerPC, LKML

Randy Dunlap <rdunlap@infradead.org> writes:
> On 4/18/21 10:46 AM, Segher Boessenkool wrote:
>> On Sun, Apr 18, 2021 at 06:24:29PM +0200, Christophe Leroy wrote:
>>> Le 17/04/2021 à 22:17, Randy Dunlap a écrit :
>>>> Should the code + Kconfigs/Makefiles handle that kind of
>>>> kernel config or should ALTIVEC always mean PPC_FPU as well?
>>>
>>> As far as I understand, Altivec is completely independant of FPU in Theory. 
>> 
>> And, as far as the hardware is concerned, in practice as well.
>> 
>>> So it should be possible to use Altivec without using FPU.
>> 
>> Yup.
>> 
>>> However, until recently, it was not possible to de-activate FPU support on 
>>> book3s/32. I made it possible in order to reduce unneccessary processing on 
>>> processors like the 832x that has no FPU.
>> 
>> The processor has to implement FP to be compliant to any version of
>> PowerPC, as far as I know?  So that is all done by emulation, including
>> all the registers?  Wow painful.
>> 
>>> As far as I can see in cputable.h/.c, 832x is the only book3s/32 without 
>>> FPU, and it doesn't have ALTIVEC either.
>> 
>> 602 doesn't have double-precision hardware, also no 64-bit FP registers.
>> But that CPU was never any widely used :-)
>> 
>>> So we can in the future ensure that Altivec can be used without FPU 
>>> support, but for the time being I think it is OK to force selection of FPU 
>>> when selecting ALTIVEC in order to avoid build failures.
>> 
>> It is useful to allow MSR[VEC,FP]=1,0 but yeah there are no CPUs that
>> have VMX (aka AltiVec) but that do not have FP.  I don't see how making
>> that artificial dependency buys anything, but maybe it does?
>> 
>>>> I have patches to fix the build errors with the config as
>>>> reported but I don't know if that's the right thing to do...
>> 
>> Neither do we, we cannot see those patches :-)
>
> 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.

cheers

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: PPC_FPU, ALTIVEC: enable_kernel_fp, put_vr, get_vr
  2021-04-19 13:16       ` Michael Ellerman
@ 2021-04-19 17:59         ` Randy Dunlap
  2021-04-19 21:39         ` Randy Dunlap
  1 sibling, 0 replies; 10+ messages in thread
From: Randy Dunlap @ 2021-04-19 17:59 UTC (permalink / raw)
  To: Michael Ellerman, Segher Boessenkool, Christophe Leroy; +Cc: PowerPC, LKML

On 4/19/21 6:16 AM, Michael Ellerman wrote:
> Randy Dunlap <rdunlap@infradead.org> writes:
>> On 4/18/21 10:46 AM, Segher Boessenkool wrote:
>>> On Sun, Apr 18, 2021 at 06:24:29PM +0200, Christophe Leroy wrote:
>>>> Le 17/04/2021 à 22:17, Randy Dunlap a écrit :
>>>>> Should the code + Kconfigs/Makefiles handle that kind of
>>>>> kernel config or should ALTIVEC always mean PPC_FPU as well?
>>>>
>>>> As far as I understand, Altivec is completely independant of FPU in Theory. 
>>>
>>> And, as far as the hardware is concerned, in practice as well.
>>>
>>>> So it should be possible to use Altivec without using FPU.
>>>
>>> Yup.
>>>
>>>> However, until recently, it was not possible to de-activate FPU support on 
>>>> book3s/32. I made it possible in order to reduce unneccessary processing on 
>>>> processors like the 832x that has no FPU.
>>>
>>> The processor has to implement FP to be compliant to any version of
>>> PowerPC, as far as I know?  So that is all done by emulation, including
>>> all the registers?  Wow painful.
>>>
>>>> As far as I can see in cputable.h/.c, 832x is the only book3s/32 without 
>>>> FPU, and it doesn't have ALTIVEC either.
>>>
>>> 602 doesn't have double-precision hardware, also no 64-bit FP registers.
>>> But that CPU was never any widely used :-)
>>>
>>>> So we can in the future ensure that Altivec can be used without FPU 
>>>> support, but for the time being I think it is OK to force selection of FPU 
>>>> when selecting ALTIVEC in order to avoid build failures.
>>>
>>> It is useful to allow MSR[VEC,FP]=1,0 but yeah there are no CPUs that
>>> have VMX (aka AltiVec) but that do not have FP.  I don't see how making
>>> that artificial dependency buys anything, but maybe it does?
>>>
>>>>> I have patches to fix the build errors with the config as
>>>>> reported but I don't know if that's the right thing to do...
>>>
>>> Neither do we, we cannot see those patches :-)
>>
>> 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

Agreed.

> 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.
> 
> cheers

Makes sense and sounds good to me.

thanks.
-- 
~Randy


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: PPC_FPU, ALTIVEC: enable_kernel_fp, put_vr, get_vr
  2021-04-19 13:16       ` Michael Ellerman
  2021-04-19 17:59         ` Randy Dunlap
@ 2021-04-19 21:39         ` Randy Dunlap
  2021-04-20  4:55           ` Christophe Leroy
  1 sibling, 1 reply; 10+ messages in thread
From: Randy Dunlap @ 2021-04-19 21:39 UTC (permalink / raw)
  To: Michael Ellerman, Segher Boessenkool, Christophe Leroy; +Cc: PowerPC, LKML

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
 	depends on PPC_BOOK3S_32 || PPC_BOOK3S_64 || (PPC_E500MC && PPC64)
 	help
 	  This option enables kernel support for the Altivec extensions to the

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: PPC_FPU, ALTIVEC: enable_kernel_fp, put_vr, get_vr
  2021-04-19 21:39         ` Randy Dunlap
@ 2021-04-20  4:55           ` Christophe Leroy
  2021-04-20 13:15             ` Michael Ellerman
  0 siblings, 1 reply; 10+ messages in thread
From: Christophe Leroy @ 2021-04-20  4:55 UTC (permalink / raw)
  To: Randy Dunlap, Michael Ellerman, Segher Boessenkool; +Cc: PowerPC, LKML



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 ?

>   	depends on PPC_BOOK3S_32 || PPC_BOOK3S_64 || (PPC_E500MC && PPC64)
>   	help
>   	  This option enables kernel support for the Altivec extensions to the
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: PPC_FPU, ALTIVEC: enable_kernel_fp, put_vr, get_vr
  2021-04-20  4:55           ` Christophe Leroy
@ 2021-04-20 13:15             ` Michael Ellerman
  2021-04-20 18:25               ` Randy Dunlap
  0 siblings, 1 reply; 10+ messages in thread
From: Michael Ellerman @ 2021-04-20 13:15 UTC (permalink / raw)
  To: Christophe Leroy, Randy Dunlap, Segher Boessenkool; +Cc: PowerPC, LKML

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: PPC_FPU, ALTIVEC: enable_kernel_fp, put_vr, get_vr
  2021-04-20 13:15             ` Michael Ellerman
@ 2021-04-20 18:25               ` Randy Dunlap
  0 siblings, 0 replies; 10+ messages in thread
From: Randy Dunlap @ 2021-04-20 18:25 UTC (permalink / raw)
  To: Michael Ellerman, Christophe Leroy, Segher Boessenkool; +Cc: PowerPC, LKML

On 4/20/21 6:15 AM, Michael Ellerman wrote:
> 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.

OK, I'll run that thru some tests today.

thanks.
-- 
~Randy


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, back to index

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-17 20:17 PPC_FPU, ALTIVEC: enable_kernel_fp, put_vr, get_vr Randy Dunlap
2021-04-18 16:24 ` Christophe Leroy
2021-04-18 17:46   ` Segher Boessenkool
2021-04-18 17:59     ` Randy Dunlap
2021-04-19 13:16       ` Michael Ellerman
2021-04-19 17:59         ` Randy Dunlap
2021-04-19 21:39         ` Randy Dunlap
2021-04-20  4:55           ` Christophe Leroy
2021-04-20 13:15             ` Michael Ellerman
2021-04-20 18:25               ` Randy Dunlap

LinuxPPC-Dev Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linuxppc-dev/0 linuxppc-dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linuxppc-dev linuxppc-dev/ https://lore.kernel.org/linuxppc-dev \
		linuxppc-dev@lists.ozlabs.org linuxppc-dev@ozlabs.org
	public-inbox-index linuxppc-dev

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.ozlabs.lists.linuxppc-dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git