Hi Kumar,
On Mon, Jun 20, 2011 at 07:56:10AM +0300, Baruch Siach wrote:
CONFIG_E500MC breaks e500/e500v2 systems. It defines L1_CACHE_SHIFT to 6, thus
breaking clear_pages(), probably others too.
Ping?
Ack/Nack?
baruch
quoted hunk
Cc: Kumar Gala <redacted>
Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
Is this the right approach?
arch/powerpc/platforms/85xx/Kconfig | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
From: Tabi Timur-B04825 <hidden> Date: 2011-07-28 19:57:03
On Sun, Jun 19, 2011 at 11:56 PM, Baruch Siach [off-list ref] wrote:
CONFIG_E500MC breaks e500/e500v2 systems. It defines L1_CACHE_SHIFT to 6,=
thus
breaking clear_pages(), probably others too.
Cc: Kumar Gala <redacted>
Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
Is this the right approach?
It doesn't work for me.
I need something that if an e500v2 platform (e.g. the P1022DS) is
selected, then I won't be able to select any e500mc platforms (e.g.
P4080DS). And if I don't select any e500v2 platforms, then I will be
able to select an e500mc platform. This patch doesn't seem to do
that.
It might be necessary to split the entire menu into two parts, one for
e500v2 parts and one for e500mc parts.
--=20
Timur Tabi
Linux kernel developer at Freescale=
From: Timur Tabi <hidden> Date: 2011-07-28 20:02:33
wrote:
On Sun, Jun 19, 2011 at 11:56 PM, Baruch Siach [off-list ref] wrote:
quoted
CONFIG_E500MC breaks e500/e500v2 systems. It defines L1_CACHE_SHIFT to 6, thus
breaking clear_pages(), probably others too.
Cc: Kumar Gala <redacted>
Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
Is this the right approach?
It doesn't work for me.
I also get this error if I try to build corenet32_smp_defconfig:
arch/powerpc/platforms/Kconfig.cputype:136:error: recursive dependency detected!
arch/powerpc/platforms/Kconfig.cputype:136: symbol PPC_E500MC is selected by
P2040_RDB
arch/powerpc/platforms/85xx/Kconfig:176: symbol P2040_RDB depends on PPC_E500MC
--
Timur Tabi
Linux kernel developer at Freescale
From: Scott Wood <hidden> Date: 2011-07-28 20:20:44
On Thu, 28 Jul 2011 19:56:53 +0000
Tabi Timur-B04825 [off-list ref] wrote:
On Sun, Jun 19, 2011 at 11:56 PM, Baruch Siach [off-list ref] wrote:
quoted
CONFIG_E500MC breaks e500/e500v2 systems. It defines L1_CACHE_SHIFT to 6, thus
breaking clear_pages(), probably others too.
Cc: Kumar Gala <redacted>
Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
Is this the right approach?
It doesn't work for me.
I need something that if an e500v2 platform (e.g. the P1022DS) is
selected, then I won't be able to select any e500mc platforms (e.g.
P4080DS). And if I don't select any e500v2 platforms, then I will be
able to select an e500mc platform. This patch doesn't seem to do
that.
It might be necessary to split the entire menu into two parts, one for
e500v2 parts and one for e500mc parts.
How about making the "Processor Type" entry be either E500 or E500MC, both
of which select PPC_85xx?
-Scott
Hi Tabi,
On Thu, Jul 28, 2011 at 07:56:53PM +0000, Tabi Timur-B04825 wrote:
On Sun, Jun 19, 2011 at 11:56 PM, Baruch Siach [off-list ref] wrote:
quoted
CONFIG_E500MC breaks e500/e500v2 systems. It defines L1_CACHE_SHIFT to 6, thus
breaking clear_pages(), probably others too.
Cc: Kumar Gala <redacted>
Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
Is this the right approach?
It doesn't work for me.
I need something that if an e500v2 platform (e.g. the P1022DS) is
selected, then I won't be able to select any e500mc platforms (e.g.
P4080DS). And if I don't select any e500v2 platforms, then I will be
able to select an e500mc platform. This patch doesn't seem to do
that.
The source of the trouble seems to be the user selectable CONFIG_PPC_E500MC
with the misleading "e500mc Support" description. I'll try to post something
better next week.
It might be necessary to split the entire menu into two parts, one for
e500v2 parts and one for e500mc parts.
baurch
--
~. .~ Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
- baruch@tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -
Hi Scott,
On Thu, Jul 28, 2011 at 03:20:33PM -0500, Scott Wood wrote:
On Thu, 28 Jul 2011 19:56:53 +0000
Tabi Timur-B04825 [off-list ref] wrote:
quoted
On Sun, Jun 19, 2011 at 11:56 PM, Baruch Siach [off-list ref] wrote:
quoted
CONFIG_E500MC breaks e500/e500v2 systems. It defines L1_CACHE_SHIFT to 6, thus
breaking clear_pages(), probably others too.
Cc: Kumar Gala <redacted>
Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
Is this the right approach?
It doesn't work for me.
I need something that if an e500v2 platform (e.g. the P1022DS) is
selected, then I won't be able to select any e500mc platforms (e.g.
P4080DS). And if I don't select any e500v2 platforms, then I will be
able to select an e500mc platform. This patch doesn't seem to do
that.
It might be necessary to split the entire menu into two parts, one for
e500v2 parts and one for e500mc parts.
How about making the "Processor Type" entry be either E500 or E500MC, both
of which select PPC_85xx?
Thanks for the tip. A patch along these lines follows.
baruch
--
~. .~ Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
- baruch@tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -
Hi Timur,
On Thu, Jul 28, 2011 at 03:02:21PM -0500, Timur Tabi wrote:
wrote:
quoted
On Sun, Jun 19, 2011 at 11:56 PM, Baruch Siach [off-list ref] wrote:
quoted
CONFIG_E500MC breaks e500/e500v2 systems. It defines L1_CACHE_SHIFT to 6, thus
breaking clear_pages(), probably others too.
Cc: Kumar Gala <redacted>
Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
Is this the right approach?
It doesn't work for me.
I also get this error if I try to build corenet32_smp_defconfig:
arch/powerpc/platforms/Kconfig.cputype:136:error: recursive dependency detected!
arch/powerpc/platforms/Kconfig.cputype:136: symbol PPC_E500MC is selected by
P2040_RDB
arch/powerpc/platforms/85xx/Kconfig:176: symbol P2040_RDB depends on PPC_E500MC
Thanks for reporting. Where can I get this corenet32_smp_defconfig for
testing?
baruch
--
~. .~ Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
- baruch@tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -
CONFIG_E500MC breaks e500/e500v2 systems. It defines L1_CACHE_SHIFT to 6, thus
breaking clear_pages(), probably others too.
This patch adds a new "Processor Type" entry for e500mc, and makes e500 systems
depend on PPC_E500.
Cc: Kumar Gala <redacted>
Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
Sending again with the correct list address. Sorry for the noise.
arch/powerpc/platforms/85xx/Kconfig | 12 +++++++++---
arch/powerpc/platforms/Kconfig.cputype | 27 +++++++++++++++------------
2 files changed, 24 insertions(+), 15 deletions(-)
CONFIG_E500MC breaks e500/e500v2 systems. It defines L1_CACHE_SHIFT to 6, thus
breaking clear_pages(), probably others too.
This patch adds a new "Processor Type" entry for e500mc, and makes e500 systems
depend on PPC_E500.
Cc: Kumar Gala <redacted>
Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
Changes from v1:
* Rebase on 3.1-rc1
* Remove the list of processor families from the PPC_E500 and
PPC_E500MC options description. The P20xx can be either e500v2 or
e500mc.
arch/powerpc/platforms/85xx/Kconfig | 13 +++++++++----
arch/powerpc/platforms/Kconfig.cputype | 27 +++++++++++++++------------
2 files changed, 24 insertions(+), 16 deletions(-)
From: Scott Wood <hidden> Date: 2011-08-08 19:43:02
On 08/08/2011 04:07 AM, Baruch Siach wrote:
CONFIG_E500MC breaks e500/e500v2 systems. It defines L1_CACHE_SHIFT to 6, thus
breaking clear_pages(), probably others too.
This patch adds a new "Processor Type" entry for e500mc, and makes e500 systems
depend on PPC_E500.
I can see confusion coming from having CONFIG_E500 and CONFIG_PPC_E500...
Maybe CONFIG_PPC_E500V1_2? Or CONFIG_PPC_E500V2, with a note in the
kconfig help that it supports e500v1 as well.
-Scott
Hi Scott,
On Mon, Aug 08, 2011 at 02:42:52PM -0500, Scott Wood wrote:
On 08/08/2011 04:07 AM, Baruch Siach wrote:
quoted
CONFIG_E500MC breaks e500/e500v2 systems. It defines L1_CACHE_SHIFT to 6, thus
breaking clear_pages(), probably others too.
This patch adds a new "Processor Type" entry for e500mc, and makes e500 systems
depend on PPC_E500.
I can see confusion coming from having CONFIG_E500 and CONFIG_PPC_E500...
I agree.
Maybe CONFIG_PPC_E500V1_2? Or CONFIG_PPC_E500V2, with a note in the
kconfig help that it supports e500v1 as well.
I'll go for CONFIG_PPC_E500_V1_V2. A revised patch is on the way.
baruch
--
~. .~ Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
- baruch@tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -
CONFIG_E500MC breaks e500/e500v2 systems. It defines L1_CACHE_SHIFT to 6, thus
breaking clear_pages(), probably others too.
This patch adds a new "Processor Type" entry for e500mc, and makes e500 systems
depend on PPC_E500_V1_V2.
Cc: Kumar Gala <redacted>
Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
Changes from v2:
* s/CONFIG_PPC_E500/CONFIG_PPC_E500_V1_V2/ to avoid confusion as
noted by Scott Wood
Changes from v1:
* Rebase on 3.1-rc1
* Remove the list of processor families from the PPC_E500 and
PPC_E500MC options description. The P20xx can be either e500v2 or
e500mc.
arch/powerpc/platforms/85xx/Kconfig | 13 +++++++++----
arch/powerpc/platforms/Kconfig.cputype | 27 +++++++++++++++------------
2 files changed, 24 insertions(+), 16 deletions(-)
From: Paul Gortmaker <hidden> Date: 2011-08-10 15:39:23
On Wed, Aug 10, 2011 at 1:21 AM, Baruch Siach [off-list ref] wrote:
CONFIG_E500MC breaks e500/e500v2 systems. It defines L1_CACHE_SHIFT to 6,=
thus
breaking clear_pages(), probably others too.
This patch adds a new "Processor Type" entry for e500mc, and makes e500 s=
ystems
depend on PPC_E500_V1_V2.
Isn't the original invalid configuration still possible, i.e. I can
choose E500_V1_V2
and also E500MC at the same time, unless you add something like a
"depends !E500MC" to your new V1_V2 option?
Alternatively, you could treat it like using i386 kernel on a modern
core by taking
the LCD for the L1_CACHE_SHIFT of the configured in platforms. I have boot=
ed
a kernel built for an mpc8548 core on a P4080 CPU, so that does work (with =
only
minimal dts fiddling). And it keeps the ability to boot one kernel on seve=
ral
platforms open (one of the reasons for the ppc --> powerpc shuffle a couple
of years ago...)
Paul.
Cc: Kumar Gala <redacted>
Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
Changes from v2:
=A0 =A0 =A0 =A0* s/CONFIG_PPC_E500/CONFIG_PPC_E500_V1_V2/ to avoid confus=
ion as
=A0 =A0 =A0 =A0 =A0noted by Scott Wood
Changes from v1:
=A0 =A0 =A0 =A0* Rebase on 3.1-rc1
=A0 =A0 =A0 =A0* Remove the list of processor families from the PPC_E500 =
and
=A0 =A0 =A0 =A0 =A0PPC_E500MC options description. The P20xx can be eithe=
From: Scott Wood <hidden> Date: 2011-08-10 16:02:01
On 08/10/2011 10:39 AM, Paul Gortmaker wrote:
On Wed, Aug 10, 2011 at 1:21 AM, Baruch Siach [off-list ref] wrote:
quoted
CONFIG_E500MC breaks e500/e500v2 systems. It defines L1_CACHE_SHIFT to 6, thus
breaking clear_pages(), probably others too.
This patch adds a new "Processor Type" entry for e500mc, and makes e500 systems
depend on PPC_E500_V1_V2.
Isn't the original invalid configuration still possible, i.e. I can
choose E500_V1_V2
and also E500MC at the same time, unless you add something like a
"depends !E500MC" to your new V1_V2 option?
They're members of a "choice", not standalone bools -- so they're
mutually exclusive.
Alternatively, you could treat it like using i386 kernel on a modern
core by taking
the LCD for the L1_CACHE_SHIFT of the configured in platforms.
For alignment you want to err on the high side, but for invalidation you
want to err on the low side. For dcbz you can't err at all.
And there are other issues than cache size with combining e500v2 and e500mc.
Could it be done with sufficient hoop-jumping? Probably. Is it worth
it? No. These chips don't even have compatible userspace, unless you
use soft-float.
I have booted
a kernel built for an mpc8548 core on a P4080 CPU, so that does work (with only
minimal dts fiddling).
The opposite direction does not work, and simply booting doesn't mean
there wouldn't be issues in running that kernel on a p4080 (floating
point? bad cache size information being given to userspace? emulation
of non-cacheable dcbz? performance?).
What dts fiddling?
And it keeps the ability to boot one kernel on several
platforms open (one of the reasons for the ppc --> powerpc shuffle a couple
of years ago...)
It's much better than the arch/ppc way of a separate kernel build for
every board. Beyond a certain point there are diminishing returns on
the effort.
-Scott
From: Paul Gortmaker <hidden> Date: 2011-08-10 17:38:18
On 11-08-10 12:01 PM, Scott Wood wrote:
On 08/10/2011 10:39 AM, Paul Gortmaker wrote:
quoted
On Wed, Aug 10, 2011 at 1:21 AM, Baruch Siach [off-list ref] wrote:
quoted
CONFIG_E500MC breaks e500/e500v2 systems. It defines L1_CACHE_SHIFT to 6, thus
breaking clear_pages(), probably others too.
This patch adds a new "Processor Type" entry for e500mc, and makes e500 systems
depend on PPC_E500_V1_V2.
Isn't the original invalid configuration still possible, i.e. I can
choose E500_V1_V2
and also E500MC at the same time, unless you add something like a
"depends !E500MC" to your new V1_V2 option?
They're members of a "choice", not standalone bools -- so they're
mutually exclusive.
OK, I missed that.
quoted
Alternatively, you could treat it like using i386 kernel on a modern
core by taking
the LCD for the L1_CACHE_SHIFT of the configured in platforms.
For alignment you want to err on the high side, but for invalidation you
want to err on the low side. For dcbz you can't err at all.
And there are other issues than cache size with combining e500v2 and e500mc.
Could it be done with sufficient hoop-jumping? Probably. Is it worth
it? No. These chips don't even have compatible userspace, unless you
use soft-float.
Yeah, if there are lots of other issues and the value return is low,
then I can't argue with that. And yes I did use soft float in the
thing I was meddling with.
quoted
I have booted
a kernel built for an mpc8548 core on a P4080 CPU, so that does work (with only
minimal dts fiddling).
The opposite direction does not work, and simply booting doesn't mean
there wouldn't be issues in running that kernel on a p4080 (floating
point? bad cache size information being given to userspace? emulation
of non-cacheable dcbz? performance?).
What dts fiddling?
Just making sure that the 8548 dts had the right address to
find the uart on the actual p4080 platform.
quoted
And it keeps the ability to boot one kernel on several
platforms open (one of the reasons for the ppc --> powerpc shuffle a couple
of years ago...)
It's much better than the arch/ppc way of a separate kernel build for
every board. Beyond a certain point there are diminishing returns on
the effort.
Given the extra info you list above, I agree. I just thought it worth
a mention since I had happened to boot the 8548 kernel on a p4080 as
part of something else I was experimenting with, and it didn't totally
catch fire (which somewhat surprised me).
P.
Hi Kumar,
On Wed, Aug 10, 2011 at 08:21:18AM +0300, Baruch Siach wrote:
CONFIG_E500MC breaks e500/e500v2 systems. It defines L1_CACHE_SHIFT to 6, thus
breaking clear_pages(), probably others too.
This patch adds a new "Processor Type" entry for e500mc, and makes e500 systems
depend on PPC_E500_V1_V2.
Ping.
Any chance of merging this for 3.2?
baruch
quoted hunk
Cc: Kumar Gala <redacted>
Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
Changes from v2:
* s/CONFIG_PPC_E500/CONFIG_PPC_E500_V1_V2/ to avoid confusion as
noted by Scott Wood
Changes from v1:
* Rebase on 3.1-rc1
* Remove the list of processor families from the PPC_E500 and
PPC_E500MC options description. The P20xx can be either e500v2 or
e500mc.
arch/powerpc/platforms/85xx/Kconfig | 13 +++++++++----
arch/powerpc/platforms/Kconfig.cputype | 27 +++++++++++++++------------
2 files changed, 24 insertions(+), 16 deletions(-)
Hello,
I saw Baruch Siach's patch:
powerpc: 85xx: separate e500 from e500mc
Unfortunately, that patch breaks the dependencies for the P5020DS
platform and does not fix the underlying code which does not
understand what the ambiguous "CONFIG_E500" means.
In order to fix the issue at the fundamental level, I created the
following 17-patch series loosely based on Baruch's patch.
=== High-Level Summary ===
The e500v1/v2 and e500mc/e5500 CPU families are not compatible with
each other, yet they share the same "CONFIG_E500" Kconfig option.
The following patch series splits the 32-bit CPU support into two
separate options: "CONFIG_FSL_E500_V1_V2" and "CONFIG_FSL_E500MC".
Additionally, the 64-bit e5500 support is separated to its own config
option ("CONFIG_FSL_E5500") which is automatically combined with
either 32-bit e500MC or 64-bit Book-3E when the P5020DS board support
is enabled.
I based the patches on v3.2-rc1, please let me know if I should
update the patches against a different tree.
The first 4 patches stand on their own merits; they are generic code
cleanups necessary to support the later patches.
I'd like to know what you all think.
Cheers,
Kyle Moffett
--
Curious about my work on the Debian powerpcspe port?
I'm keeping a blog here: http://pureperl.blogspot.com/
(Sorry for the repost, I accidentally omitted Baruch's email).
Hello,
I saw Baruch Siach's patch:
powerpc: 85xx: separate e500 from e500mc
Unfortunately, that patch breaks the dependencies for the P5020DS
platform and does not fix the underlying code which does not
understand what the ambiguous "CONFIG_E500" means.
In order to fix the issue at the fundamental level, I created the
following 17-patch series loosely based on Baruch's patch.
=== High-Level Summary ===
The e500v1/v2 and e500mc/e5500 CPU families are not compatible with
each other, yet they share the same "CONFIG_E500" Kconfig option.
The following patch series splits the 32-bit CPU support into two
separate options: "CONFIG_FSL_E500_V1_V2" and "CONFIG_FSL_E500MC".
Additionally, the 64-bit e5500 support is separated to its own config
option ("CONFIG_FSL_E5500") which is automatically combined with
either 32-bit e500MC or 64-bit Book-3E when the P5020DS board support
is enabled.
I based the patches on v3.2-rc1, please let me know if I should
update the patches against a different tree.
The first 4 patches stand on their own merits; they are generic code
cleanups necessary to support the later patches.
I'd like to know what you all think.
Cheers,
Kyle Moffett
--
Curious about my work on the Debian powerpcspe port?
I'm keeping a blog here: http://pureperl.blogspot.com/
The code inside the conditional is only used by 85xx CoreNet fabric
platforms, so add a new config define and use it where necessary.
Signed-off-by: Kyle Moffett <redacted>
---
arch/powerpc/platforms/85xx/Kconfig | 5 +++++
arch/powerpc/platforms/85xx/Makefile | 11 ++++++-----
arch/powerpc/platforms/Kconfig.cputype | 3 +++
arch/powerpc/sysdev/mpic.c | 6 ++----
4 files changed, 16 insertions(+), 9 deletions(-)
@@ -177,6 +177,9 @@ config FSL_BOOKEselectSYS_SUPPORTS_HUGETLBFSifPHYS_64BITdefaulty+configFSL_CORENET+bool+# this is for common code between PPC32 & PPC64 FSL BOOKEconfigPPC_FSL_BOOK3Ebool
The "CONFIG_PHYS_64BIT" option violates the Kconfig best-practices in
various colorful ways. It has explicit dependencies, but it is also
"select"ed by various CPUs and platforms. It is not set on 64-bit
systems, but it is used by a number of pieces of powerpc code to enable
or disable 64-bit physical address support.
To resolve these issues, the option has now been split into 3:
CONFIG_PHYS_64BIT_SUPPORTED:
This hidden option should be selected by any CPU type which supports
64-bit physical addresses. This will enable the PHYS_64BIT option
to be selected. It is (obviously) always set on PPC64.
CONFIG_PHYS_64BIT_DT_REQUIRED:
This hidden option should be selected by any board or platform which
has >32-bit physical devices present in hardware. If this is set
then the CONFIG_PHYS_64BIT option will be forcibly enabled and
hidden from the user. It is (obviously) always set on PPC64.
CONFIG_PHYS_64BIT:
This option is user-controllable, where allowed by CPU and platform
settings, and should never be pointed at with a "select" statement.
Due to the values of the above two options, this is never visible on
PPC64.
Signed-off-by: Kyle Moffett <redacted>
---
arch/powerpc/Kconfig | 4 ++--
arch/powerpc/platforms/82xx/Kconfig | 2 +-
arch/powerpc/platforms/83xx/Kconfig | 2 +-
arch/powerpc/platforms/85xx/Kconfig | 12 ++++++------
arch/powerpc/platforms/86xx/Kconfig | 1 +
arch/powerpc/platforms/Kconfig.cputype | 25 ++++++++++++++++++-------
6 files changed, 29 insertions(+), 17 deletions(-)
@@ -80,7 +80,7 @@ config P1010_RDBconfigP1022_DSbool"Freescale P1022 DS"selectDEFAULT_UIMAGE-selectPHYS_64BIT# The DTS has 36-bit addresses+selectPHYS_64BIT_DT_REQUIRED# The DTS has 36-bit addressesselectSWIOTLBhelpThisoptionenablessupportfortheFreescaleP1022DSreferenceboard.
@@ -187,14 +189,20 @@ config PPC_FSL_BOOK3EselectPPC_SMP_MUXED_IPIdefaultyifFSL_BOOKE-configPTE_64BIT-bool-depends on44x||E500||PPC_86xx-defaultyifPHYS_64BIT+## Only some 32-bit CPUs support 64-bit physical addresses+configPHYS_64BIT_SUPPORTED+def_boolPPC64++## Some 32-bit system device trees have >32-bit physical addresses, and so+## the kernel won't boot unless those are supported.+configPHYS_64BIT_DT_REQUIRED+def_boolPPC64+## DO NOT "select" this user-visible symbol, select the two above instead!configPHYS_64BIT-bool'Large physical address support'ifE500||PPC_86xx-depends on(44x||E500||PPC_86xx)&&!PPC_83xx&&!PPC_82xx+def_boolPHYS_64BIT_DT_REQUIRED+prompt'Large physical address support'if!PHYS_64BIT_DT_REQUIRED+depends onPHYS_64BIT_SUPPORTED---help---Thisoptionenableskernelsupportforlargerthan32-bitphysicaladdresses.Thisfeaturemaynotbeavailableonallcores.
The "CONFIG_E500" conditional should be unnecessary. The fsl_rio
hardware is only present on MPC85xx (e500/e500mc) anyways, although it
should build everywhere for better compile-test coverage.
The conditionally-defined fsl_rio_mcheck_exception() function lost its
two e500-specific constants in commit 82a9a4809f:
powerpc/e500: fix breakage with fsl_rio_mcheck_exception
Specifically, the references to SPRN_MCSR and MCSR_BUS_RBERR were moved
to an e500-specific file.
This patch just removes the unnecessary #ifdef, as the entire file is
effectively dead code on non-e500 platforms anyways.
Signed-off-by: Kyle Moffett <redacted>
---
arch/powerpc/sysdev/fsl_rio.c | 2 --
1 files changed, 0 insertions(+), 2 deletions(-)
Certain processor types are co-supportable, and their machine-check
handlers will be referenced if the entries in cputable.c are actually
generated, so allow more than one machine-check handler to be built in.
This fixes a bug where configuring FreeScale E5500 support (P5020DS)
into the kernel would break machine-check handling on PPC64 A2 systems
by using a bogus machine_check_generic() handler that does nothing.
Signed-off-by: Kyle Moffett <redacted>
---
arch/powerpc/kernel/traps.c | 18 ++++++++----------
1 files changed, 8 insertions(+), 10 deletions(-)
As the 3 "e500" variants are generally incompatible with each other,
(32bit-spe, 32bit-classic-fpu, 64bit-classic-fpu), there is no such
thing as a "default e500" processor.
Signed-off-by: Kyle Moffett <redacted>
---
arch/powerpc/kernel/cputable.c | 16 ----------------
1 files changed, 0 insertions(+), 16 deletions(-)
Kernels built for e500 and e500mc are fundamentally incompatible with
each other due to different cacheline sizes, support for SPE
instructions, etc.
Unfortuantely, there is a lot of support code with a morass of config
options which mostly mean the same things (E500/E500MC/PPC_85xx/etc).
As preparation for fixing up all the support code, split the config
options into two separate parts.
Signed-off-by: Kyle Moffett <redacted>
---
arch/powerpc/platforms/Kconfig.cputype | 34 +++++++++++++++++++++++--------
1 files changed, 25 insertions(+), 9 deletions(-)
@@ -24,10 +26,21 @@ config PPC_BOOK3S_32bool"512x/52xx/6xx/7xx/74xx/82xx/83xx/86xx"selectPPC_FPU-configPPC_85xx-bool"Freescale 85xx"+configFSL_E200+bool"Freescale e200 (also known as MPC5xx/MPC55xx)"+selectE200++configFSL_E500_V1_V2+bool"Freescale e500v1/e500v2 (also known as MPC85xx)"selectPHYS_64BIT_SUPPORTEDselectE500+selectPPC_85xx++configFSL_E500MC+bool"Freescale e500mc (also e5500 in 32-bit mode)"+selectPHYS_64BIT_SUPPORTED+selectE500+selectPPC_85xxconfigPPC_8xxbool"Freescale 8xx"
@@ -28,7 +28,6 @@ config PPC_BOOK3S_32configFSL_E200bool"Freescale e200 (also known as MPC5xx/MPC55xx)"-selectE200configFSL_E500_V1_V2bool"Freescale e500v1/e500v2 (also known as MPC85xx)"
As FreeScale e500 systems have different cacheline sizes from e500mc, it
is basically impossible for the kernel to support both in a single
system image at present.
Given that one is SPE-float and the other is classic-float, they are not
generally userspace-compatible either.
This patch updates the conditional to depend on whether the system is
actually targetting an "e500" or "e500mc" core and entirely removes the
unused sync-to-lwsync-replacement on e500v1/e500v2 systems.
Signed-off-by: Kyle Moffett <redacted>
---
arch/powerpc/include/asm/synch.h | 16 ++++------------
1 files changed, 4 insertions(+), 12 deletions(-)
This moves the "select PPC_FPU" from PPC_E500MC to the new FSL_E500MC
processor selection, where it belongs.
The only actual difference that this patch introduces is the P5020DS
board (FreeScale e5500 CPU) will not set FSL_E500MC when building in
64-bit mode.
This is OK as all of the existing code that the 64-bit P5020DS needs is
still conditional on CONFIG_E500 or CONFIG_PPC_FPU, which are both set
properly on the P5020DS (PPC_FPU is always enabled on 64-bit).
Signed-off-by: Kyle Moffett <redacted>
---
arch/powerpc/include/asm/cache.h | 2 +-
arch/powerpc/platforms/85xx/Kconfig | 10 +++++-----
arch/powerpc/platforms/Kconfig.cputype | 8 ++------
3 files changed, 8 insertions(+), 12 deletions(-)
The e500mc CPUs have a classic FPU and are not compatible with e500v1 or
e500v2. This patch fixes the 32-bit CPU feature tables so that e500mc
entries are not present on e500v1/e500v2 systems and vice versa.
Signed-off-by: Kyle Moffett <redacted>
---
arch/powerpc/include/asm/cputable.h | 16 ++++++++++------
arch/powerpc/kernel/cputable.c | 8 +++++---
2 files changed, 15 insertions(+), 9 deletions(-)
The only systems which need the sparse PowerPC register map are the
e500v1/e500v2.
NOTE: The kgdb code does not otherwise use CONFIG_E500, so this support
is probably still buggy on e500v1/e500v2, but I don't know kgdb enough
to fix it.
Signed-off-by: Kyle Moffett <redacted>
---
arch/powerpc/include/asm/kgdb.h | 17 ++++++++++-------
1 files changed, 10 insertions(+), 7 deletions(-)
The CONFIG_E500 config option is ambiguous and used incorrectly in many
places to refer to some combination of e500v1/v2, e500mc, and e5500.
Fix up each reference to use the correct combinations of the following
config options:
CONFIG_FSL_E500_V1_V2
CONFIG_FSL_E500MC
CONFIG_FSL_E5500
Signed-off-by: Kyle Moffett <redacted>
---
arch/powerpc/Kconfig | 2 +-
arch/powerpc/include/asm/reg_booke.h | 17 ++++++++++-------
arch/powerpc/kernel/cputable.c | 8 ++++----
arch/powerpc/kernel/entry_32.S | 8 +++++---
arch/powerpc/kvm/Kconfig | 3 ++-
arch/powerpc/platforms/Kconfig.cputype | 22 ++++++++++------------
6 files changed, 32 insertions(+), 28 deletions(-)
@@ -69,10 +69,10 @@ extern void __setup_cpu_power7(unsigned long offset, struct cpu_spec* spec);externvoid__restore_cpu_power7(void);externvoid__restore_cpu_a2(void);#endif /* CONFIG_PPC64 */-#if defined(CONFIG_E500)+#ifdef CONFIG_FSL_E5500externvoid__setup_cpu_e5500(unsignedlongoffset,structcpu_spec*spec);externvoid__restore_cpu_e5500(void);-#endif /* CONFIG_E500 */+#endif /* CONFIG_FSL_E5500 *//* This table only contains "desktop" CPUs, it need to be filled with embedded*onesaswell...
@@ -236,7 +237,8 @@ reenable_mmu: /* re-enable mmu so we can */RFI/*jumptohandler,enableMMU*/#endif /* CONFIG_TRACE_IRQFLAGS */-#if defined (CONFIG_6xx) || defined(CONFIG_E500)+#if defined (CONFIG_6xx) || defined(CONFIG_FSL_E500_V1_V2) \+||defined(CONFIG_FSL_E500MC)4:rlwinmr12,r12,0,~_TLF_NAPPINGstwr12,TI_LOCAL_FLAGS(r9)bpower_save_ppc32_restore
This ambiguous config option is now only present as a Kconfig stub.
Remove it and fix up the users.
Signed-off-by: Kyle Moffett <redacted>
---
arch/powerpc/platforms/Kconfig.cputype | 10 ++++------
1 files changed, 4 insertions(+), 6 deletions(-)
Only build the setup functions when the corresponding entries are
included in the CPU table.
Signed-off-by: Kyle Moffett <redacted>
---
arch/powerpc/kernel/cpu_setup_fsl_booke.S | 14 +++++++++++++-
1 files changed, 13 insertions(+), 1 deletions(-)
As part of splitting CONFIG_E500 into separate options for e500v1/v2 and
e500mc/e5500, some code only needs to be built when e5500 support is
required.
This adds a new internal-use config option for both 32-bit and 64-bit
builds that enables only the e5500 support code.
Signed-off-by: Kyle Moffett <redacted>
---
arch/powerpc/platforms/85xx/Kconfig | 2 +-
arch/powerpc/platforms/Kconfig.cputype | 4 ++++
2 files changed, 5 insertions(+), 1 deletions(-)
The "-me500" assembler option is intended only for use on e500v1 or
e500v2 systems. On modern binutils it will refuse to assemble standard
floating point opcodes, and it will translate "lwsync" into "sync".
This fixes the e500mc/e5500 kernel build on recent binutils.
For e500mc/e5500 with standard PowerPC floating point, the assembler
options "-me500mc" or "-me500mc64" should be used, although they are not
strictly necessary depending on which opcodes are in use.
Signed-off-by: Kyle Moffett <redacted>
---
arch/powerpc/Makefile | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
From: Kumar Gala <hidden> Date: 2011-11-10 13:34:19
On Nov 9, 2011, at 6:06 PM, Kyle Moffett wrote:
The code inside the conditional is only used by 85xx CoreNet fabric
platforms, so add a new config define and use it where necessary.
1. why make the change
2. if you're using FSL_CORENET as a SoC feature CONFIG option than other =
bits of this patch are wrong
It should NOT be used in platforms/85xx/Makefile those should be left =
alone and FSL_CORENET should be selected by having E500MC or similar =
set.
select SYS_SUPPORTS_HUGETLBFS if PHYS_64BIT
default y
=20
+config FSL_CORENET
+ bool
+
# this is for common code between PPC32 & PPC64 FSL BOOKE
config PPC_FSL_BOOK3E
bool
From: Kumar Gala <hidden> Date: 2011-11-10 13:36:57
On Nov 9, 2011, at 6:07 PM, Kyle Moffett wrote:
The "CONFIG_PHYS_64BIT" option violates the Kconfig best-practices in
various colorful ways. It has explicit dependencies, but it is also
"select"ed by various CPUs and platforms. It is not set on 64-bit
systems, but it is used by a number of pieces of powerpc code to =
enable
or disable 64-bit physical address support.
=20
To resolve these issues, the option has now been split into 3:
=20
CONFIG_PHYS_64BIT_SUPPORTED:
This hidden option should be selected by any CPU type which =
supports
64-bit physical addresses. This will enable the PHYS_64BIT option
to be selected. It is (obviously) always set on PPC64.
=20
CONFIG_PHYS_64BIT_DT_REQUIRED:
This hidden option should be selected by any board or platform =
which
has >32-bit physical devices present in hardware. If this is set
then the CONFIG_PHYS_64BIT option will be forcibly enabled and
hidden from the user. It is (obviously) always set on PPC64.
=20
CONFIG_PHYS_64BIT:
This option is user-controllable, where allowed by CPU and platform
settings, and should never be pointed at with a "select" statement.
Due to the values of the above two options, this is never visible =
config P1022_DS
bool "Freescale P1022 DS"
select DEFAULT_UIMAGE
- select PHYS_64BIT # The DTS has 36-bit addresses
+ select PHYS_64BIT_DT_REQUIRED # The DTS has 36-bit addresses
select SWIOTLB
help
This option enables support for the Freescale P1022DS =
select PPC_SMP_MUXED_IPI
default y if FSL_BOOKE
=20
-config PTE_64BIT
- bool
- depends on 44x || E500 || PPC_86xx
- default y if PHYS_64BIT
+## Only some 32-bit CPUs support 64-bit physical addresses
+config PHYS_64BIT_SUPPORTED
+ def_bool PPC64
+
+## Some 32-bit system device trees have >32-bit physical addresses, =
and so
+## the kernel won't boot unless those are supported.
+config PHYS_64BIT_DT_REQUIRED
+ def_bool PPC64
=20
+## DO NOT "select" this user-visible symbol, select the two above =
From: Kumar Gala <hidden> Date: 2011-11-10 13:38:12
On Nov 9, 2011, at 6:07 PM, Kyle Moffett wrote:
quoted hunk
Certain processor types are co-supportable, and their machine-check
handlers will be referenced if the entries in cputable.c are actually
generated, so allow more than one machine-check handler to be built in.
This fixes a bug where configuring FreeScale E5500 support (P5020DS)
into the kernel would break machine-check handling on PPC64 A2 systems
by using a bogus machine_check_generic() handler that does nothing.
Signed-off-by: Kyle Moffett <redacted>
---
arch/powerpc/kernel/traps.c | 18 ++++++++----------
1 files changed, 8 insertions(+), 10 deletions(-)
From: Kumar Gala <hidden> Date: 2011-11-10 13:40:48
On Nov 9, 2011, at 6:07 PM, Kyle Moffett wrote:
As FreeScale e500 systems have different cacheline sizes from e500mc, =
it
is basically impossible for the kernel to support both in a single
system image at present.
=20
Given that one is SPE-float and the other is classic-float, they are =
not
generally userspace-compatible either.
=20
This patch updates the conditional to depend on whether the system is
actually targetting an "e500" or "e500mc" core and entirely removes =
From: Kumar Gala <hidden> Date: 2011-11-10 13:46:45
On Nov 9, 2011, at 6:07 PM, Kyle Moffett wrote:
As part of splitting CONFIG_E500 into separate options for e500v1/v2 =
and
quoted hunk
e500mc/e5500, some code only needs to be built when e5500 support is
required.
=20
This adds a new internal-use config option for both 32-bit and 64-bit
builds that enables only the e5500 support code.
=20
Signed-off-by: Kyle Moffett <redacted>
---
arch/powerpc/platforms/85xx/Kconfig | 2 +-
arch/powerpc/platforms/Kconfig.cputype | 4 ++++
2 files changed, 5 insertions(+), 1 deletions(-)
=20
Nak, I don't want to add a unique build option for 64-bit E5500 vs =
E500MC. We have E6500 coming and I don't want to introduce =
CONFIG_FSL_E6500 at that point.
- k
From: Kumar Gala <hidden> Date: 2011-11-10 13:59:24
On Nov 9, 2011, at 6:03 PM, Kyle Moffett wrote:
Hello,
=20
I saw Baruch Siach's patch:
powerpc: 85xx: separate e500 from e500mc
=20
Unfortunately, that patch breaks the dependencies for the P5020DS
platform and does not fix the underlying code which does not
understand what the ambiguous "CONFIG_E500" means.
=20
In order to fix the issue at the fundamental level, I created the
following 17-patch series loosely based on Baruch's patch.
=20
=3D=3D=3D High-Level Summary =3D=3D=3D
=20
The e500v1/v2 and e500mc/e5500 CPU families are not compatible with
each other, yet they share the same "CONFIG_E500" Kconfig option.
=20
The following patch series splits the 32-bit CPU support into two
separate options: "CONFIG_FSL_E500_V1_V2" and "CONFIG_FSL_E500MC".
Additionally, the 64-bit e5500 support is separated to its own config
option ("CONFIG_FSL_E5500") which is automatically combined with
either 32-bit e500MC or 64-bit Book-3E when the P5020DS board support
is enabled.
So its clear from the community that there is confusion here and we need =
to clean this up. I guess my attempt to support an kernel that ran on =
both E500v2 and E500mc isn't worth it. However I don't want to =
completely remove the ability to do this.
Towards the cleanup I'd ask for a proposal on what exactly the CONFIG_ =
options we'd end up with would be and their meaning.
So today we have:
CONFIG_E500
CONFIG_PPC_E500MC
What do we want to move to? I want to keep the builds such that we have =
only 2 classes: e500V1/V2 and e500mc/e5500/e6500/.../eX500. I see no =
reason to hyper-optimize e500mc vs e5500 vs e6500.
- k=
From: Timur Tabi <hidden> Date: 2011-11-10 14:04:24
Kyle Moffett wrote:
CONFIG_PHYS_64BIT_SUPPORTED:
This hidden option should be selected by any CPU type which supports
64-bit physical addresses. This will enable the PHYS_64BIT option
to be selected. It is (obviously) always set on PPC64.
CONFIG_PHYS_64BIT_DT_REQUIRED:
This hidden option should be selected by any board or platform which
has >32-bit physical devices present in hardware. If this is set
then the CONFIG_PHYS_64BIT option will be forcibly enabled and
hidden from the user. It is (obviously) always set on PPC64.
CONFIG_PHYS_64BIT:
This option is user-controllable, where allowed by CPU and platform
settings, and should never be pointed at with a "select" statement.
Due to the values of the above two options, this is never visible on
PPC64.
I'm with Kumar on this. I don't see the point of making it so complicated. No kernel should ever have to care with a DT is 64-bit or 32-bit. If you build a kernel with 64-bit phys support, then it will work with any DT.
U-Boot already warns you if the DT and the actual physical addresses of the devices don't match.
There are only two reasons to create a 32-bit kernel:
1) A small performance improvement on systems with 2GB or less.
2) Some SOC devices only support 32-bit physical addresses, so the easiest way to ensure a compatible address is to prohibit memory above 4GB.
Item #2 is not something that we really worry about. It's only a real problem on true 64-bit kernels, and in those cases, we cannot build a kernel with 32-bit phys support, so the driver needs to handle it internally.
--
Timur Tabi
Linux kernel developer at Freescale
From: Kumar Gala <hidden> Date: 2011-11-10 16:31:03
On Nov 10, 2011, at 10:17 AM, Moffett, Kyle D wrote:
On Nov 10, 2011, at 08:59, Kumar Gala wrote:
quoted
On Nov 9, 2011, at 6:03 PM, Kyle Moffett wrote:
quoted
I saw Baruch Siach's patch:
powerpc: 85xx: separate e500 from e500mc
=20
Unfortunately, that patch breaks the dependencies for the P5020DS
platform and does not fix the underlying code which does not
understand what the ambiguous "CONFIG_E500" means.
=20
In order to fix the issue at the fundamental level, I created the
following 17-patch series loosely based on Baruch's patch.
=20
=3D=3D=3D High-Level Summary =3D=3D=3D
=20
The e500v1/v2 and e500mc/e5500 CPU families are not compatible with
each other, yet they share the same "CONFIG_E500" Kconfig option.
=20
The following patch series splits the 32-bit CPU support into two
separate options: "CONFIG_FSL_E500_V1_V2" and "CONFIG_FSL_E500MC".
Additionally, the 64-bit e5500 support is separated to its own =
config
quoted
quoted
option ("CONFIG_FSL_E5500") which is automatically combined with
either 32-bit e500MC or 64-bit Book-3E when the P5020DS board =
support
quoted
quoted
is enabled.
=20
So its clear from the community that there is confusion here and we
need to clean this up. I guess my attempt to support an kernel that
ran on both E500v2 and E500mc isn't worth it. However I don't want =
to
quoted
completely remove the ability to do this.
=20
Well, a kernel built with CONFIG_PPC_E500MC today appears to be
fundamentally broken on E500v1/E500v2:
=20
#if defined(CONFIG_8xx) || defined(CONFIG_403GCX)
#define L1_CACHE_SHIFT 4
#define MAX_COPY_PREFETCH 1
#elif defined(CONFIG_PPC_E500MC)
#define L1_CACHE_SHIFT 6
#define MAX_COPY_PREFETCH 4
#elif defined(CONFIG_PPC32)
#define MAX_COPY_PREFETCH 4
#if defined(CONFIG_PPC_47x)
#define L1_CACHE_SHIFT 7
#else
#define L1_CACHE_SHIFT 5
#endif
#else /* CONFIG_PPC64 */
#define L1_CACHE_SHIFT 7
#endif
=20
E500MC will set L1_CACHE_SHIFT to 6, while regular E500 appears to
want it set to 5. I don't know if that's a mistake or exactly what
code that affects, but it looks very wrong.
This is correct for E500 & E500MC. However we have a HW mode that =
allows us to handle running in 'e500' mode on e500mc.
Furthermore, it looks like there are a couple issues here I missed
before. PPC64 systems all appear to have an L1_CACHE_SHIFT of 7,
except when you turn on the P5020DS board option which magically
changes it to "6" and breaks lord-knows-what. I think my patch
series actually "breaks" that and makes e5500 use 7 as well.
a value of '6' on E5500 / P5020DS is correct and doesn't break anything. =
Setting it to 7 is wrong and thus the code is correct today.
Are you sure that a kernel built to support E5500 can also run on
other 64-bit PowerPC/POWER systems?
No it will not. There is not expectation of that as E5500 is an =
embedded / Book-E class part and uses that ISA version. Book-S (server) =
64-bit machines are not OS compatible and we are not trying to make them =
as such (but we do re-use a lot of code).
quoted
Towards the cleanup I'd ask for a proposal on what exactly the
CONFIG_ options we'd end up with would be and their meaning.
So today we have:
=20
CONFIG_E500
CONFIG_PPC_E500MC
=20
It's actually a bit more complicated than that. There are 3 ways
that the user can configure an e500 kernel today. I'm omitting
the "FSL_SOC_BOOKE" menu that wraps around all of the 85xx/e5500
boards today, because that is set for all of these platforms:
=20
* PPC32 + PPC_85xx + E500 [+ boards]
* PPC64 + BOOK3E_64 + P5020_DS (which adds E500 and PPC_E500MC)
=20
Note that whether or not "PPC_E500MC" is set on PPC32 depends
only on which boards the user picked. So if I am trying to
build an e500v2 kernel and I accidentally also turn on support
for one of the e500mc boards, my kernel mysteriously breaks.
sure, I understand I'm fine with us 'fixing' things such that we treat =
E500V1/V2 differently from E500MC/E5500 in user Kconfig choices
quoted
What do we want to move to? I want to keep the builds such that we
have only 2 classes: e500V1/V2 and e500mc/e5500/e6500/.../eX500.
I see no reason to hyper-optimize e500mc vs e5500 vs e6500.
=20
So after my changes, there are the following user-configurable
option sets:
* PPC32 + FSL_E500_V1_V2 [+ e500v1/v2 boards]
* PPC32 + FSL_E500MC [+ e500mc boards]
* PPC64 + BOOK3E_64 + P5020_DS (which adds FSL_E5500)
=20
Since most of the "e500mc"-specific code was in 32-bit-only ASM
or inside of #ifdef PPC32, the new FSL_E500MC option is only
set on 32-bit builds, even if it is running in compat mode on
64-bit e5500 hardware)
=20
Internally the P5020_DS option turns on the hidden FSL_E5500
option for both 32-bit and 64-bit; that config option enables
platform drivers and similar stuff.
=20
Depending on how compatible the AMP processors are, you could
rename the option to be "FSL_E5X00" or add a hidden option for
"FSL_E6500" that is also selected by appropriate boards.
=20
Please let me know if you think!
I'd like to avoid adding FSL_E5500, FSL_E6500, etc CONFIG options =
getting added. I'd like to keep things as:
32-bit:
e500v1/v2
e500mc/e5500/e6500/...
64-bit:
e5500/e6500/...
We need to come up with some CONFIG option that covers =
e500mc/e5500/e6500/...
- k=
From: Scott Wood <hidden> Date: 2011-11-10 16:31:10
On Thu, Nov 10, 2011 at 07:40:04AM -0600, Kumar Gala wrote:
On Nov 9, 2011, at 6:07 PM, Kyle Moffett wrote:
quoted
As FreeScale e500 systems have different cacheline sizes from e500mc, it
is basically impossible for the kernel to support both in a single
system image at present.
Given that one is SPE-float and the other is classic-float, they are not
generally userspace-compatible either.
This patch updates the conditional to depend on whether the system is
actually targetting an "e500" or "e500mc" core and entirely removes the
unused sync-to-lwsync-replacement on e500v1/e500v2 systems.
Signed-off-by: Kyle Moffett <redacted>
---
arch/powerpc/include/asm/synch.h | 16 ++++------------
1 files changed, 4 insertions(+), 12 deletions(-)
Nak, we can run an e500mc in a mode that is compatible with e500v1/v2. I see no reason to change the support we have there.
What "mode" do you mean? DCBZ32? We don't support using that currently,
and I'd imagine the performance implication would be such that you'd
never want to do it unless it's the only way to make some piece of legacy
software work.
I see no reason to change the support we have there.
No reason to remove complexity that is not needed, and is not planned to
be needed?
-Scott
From: Moffett, Kyle D <hidden> Date: 2011-11-10 16:33:35
On Nov 10, 2011, at 09:04, Timur Tabi wrote:
Kyle Moffett wrote:
quoted
CONFIG_PHYS_64BIT_SUPPORTED:
This hidden option should be selected by any CPU type which supports
64-bit physical addresses. This will enable the PHYS_64BIT option
to be selected. It is (obviously) always set on PPC64.
=20
CONFIG_PHYS_64BIT_DT_REQUIRED:
This hidden option should be selected by any board or platform which
has >32-bit physical devices present in hardware. If this is set
then the CONFIG_PHYS_64BIT option will be forcibly enabled and
hidden from the user. It is (obviously) always set on PPC64.
=20
CONFIG_PHYS_64BIT:
This option is user-controllable, where allowed by CPU and platform
settings, and should never be pointed at with a "select" statement.
Due to the values of the above two options, this is never visible on
PPC64.
=20
I'm with Kumar on this. I don't see the point of making it so complicate=
d.
Did you look at the existing code? It's already that complicated:
config ARCH_PHYS_ADDR_T_64BIT
def_bool PPC64 || PHYS_64BIT
config ARCH_DMA_ADDR_T_64BIT
def_bool ARCH_PHYS_ADDR_T_64BIT
config {P1022_DS,P2041_RDB,P3041_DS,P3060_QDS,P4080_DS,P5020_DS}
select PHYS_64BIT
config 44x
select PHYS_64BIT
config PTE_64BIT
bool
depends on 44x || E500 || PPC_86xx
default y if PHYS_64BIT
config PHYS_64BIT
bool 'Large physical address support' if E500 || PPC_86xx
depends on (44x || E500 || PPC_86xx) && !PPC_83xx && !PPC_82xx
Even worse, PHYS_64BIT is not set on 64-bit processors, but there is
a lot of driver code that seems to assume PHYS_64BIT indicates the
size of "phys_addr_t".
No kernel should ever have to care with a DT is 64-bit or 32-bit. If
you build a kernel with 64-bit phys support, then it will work with
any DT.
=20
U-Boot already warns you if the DT and the actual physical addresses
of the devices don't match.
The big issue is that the Kconfig docs are very clear that "select"
should not be used on user-visible options (AKA: PHYS_64BIT), and yet
half the PPC_85xx boards have this tidbit:
select PHYS_64BIT # The DTS has 36-bit addresses
I'm totally OK with removing that from all those boards, but to preserve
the existing behavior (also used by the entire 44x platform) I added the
new config symbol PHYS_64BIT_DT_REQUIRED, which is used to control
whether or not the "PHYS_64BIT" option is even visible to the user.
I originally called it "PHYS_64BIT_REQUIRED", but since all of the
board comments talked about 36-bit DTS addresses, I added the _DT_.
There are only two reasons to create a 32-bit kernel:
=20
1) A small performance improvement on systems with 2GB or less.
2) Some SOC devices only support 32-bit physical addresses, so the
easiest way to ensure a compatible address is to prohibit memory above
4GB.
If this is true, then why does PHYS_64BIT have that big ugly list of
dependencies right now? I don't know about the other platforms well
enough to tell what would break by enabling PHYS_64BIT, but I assume
that something in the past caused that dependency list.
Cheers,
Kyle Moffett
--
Curious about my work on the Debian powerpcspe port?
I'm keeping a blog here: http://pureperl.blogspot.com/
From: Kumar Gala <hidden> Date: 2011-11-10 16:42:55
On Nov 10, 2011, at 10:31 AM, Scott Wood wrote:
On Thu, Nov 10, 2011 at 07:40:04AM -0600, Kumar Gala wrote:
quoted
=20
On Nov 9, 2011, at 6:07 PM, Kyle Moffett wrote:
=20
quoted
As FreeScale e500 systems have different cacheline sizes from =
e500mc, it
quoted
quoted
is basically impossible for the kernel to support both in a single
system image at present.
=20
Given that one is SPE-float and the other is classic-float, they are =
not
quoted
quoted
generally userspace-compatible either.
=20
This patch updates the conditional to depend on whether the system =
is
quoted
quoted
actually targetting an "e500" or "e500mc" core and entirely removes =
From: Scott Wood <hidden> Date: 2011-11-10 16:47:47
On Wed, Nov 09, 2011 at 07:07:14PM -0500, Kyle Moffett wrote:
quoted hunk
Only build the setup functions when the corresponding entries are
included in the CPU table.
Signed-off-by: Kyle Moffett <redacted>
---
arch/powerpc/kernel/cpu_setup_fsl_booke.S | 14 +++++++++++++-
1 files changed, 13 insertions(+), 1 deletions(-)
From: Scott Wood <hidden> Date: 2011-11-10 16:49:32
On Wed, Nov 09, 2011 at 07:07:11PM -0500, Kyle Moffett wrote:
As part of splitting CONFIG_E500 into separate options for e500v1/v2 and
e500mc/e5500, some code only needs to be built when e5500 support is
required.
How much code is this? e5500 is basically just a 64-bit version of
e500mc. When running a 32-bit kernel the extra code should be minimal.
-Scott
I posted a patch to remove the P1022_DS version of this. I'm in favor of removing it from the other boards and letting the defconfig define it, but Kumar nak'd that idea.
config PTE_64BIT
bool
depends on 44x || E500 || PPC_86xx
default y if PHYS_64BIT
I think this one is about how our TLB registers look, and it's not related to anything else.
Even worse, PHYS_64BIT is not set on 64-bit processors, but there is
a lot of driver code that seems to assume PHYS_64BIT indicates the
size of "phys_addr_t".
There's a Kconfig option somewhere that takes care of this.
The big issue is that the Kconfig docs are very clear that "select"
should not be used on user-visible options (AKA: PHYS_64BIT), and yet
half the PPC_85xx boards have this tidbit:
select PHYS_64BIT # The DTS has 36-bit addresses
I'm not sure that Kconfig rule is enforceable. We use 'select' all the time to make sure that our boards enable the options that need to be enabled. I'd rather see this:
config X
select A
config Y
select A
config Z
select A
than this:
config A
default y if X | Y | Z
The former is more maintainable and does not allow the option of disabling "A" if someone wants "X".
I'm totally OK with removing that from all those boards, but to preserve
the existing behavior (also used by the entire 44x platform) I added the
new config symbol PHYS_64BIT_DT_REQUIRED, which is used to control
whether or not the "PHYS_64BIT" option is even visible to the user.
I originally called it "PHYS_64BIT_REQUIRED", but since all of the
board comments talked about 36-bit DTS addresses, I added the _DT_.
They're really 64-bit DTS addresses. The 36 bit is just a hardware quirk of our SOCs. From the kernel's perspective, there's no difference between 36-bit and 64-bit physical address.
quoted
There are only two reasons to create a 32-bit kernel:
1) A small performance improvement on systems with 2GB or less.
2) Some SOC devices only support 32-bit physical addresses, so the
easiest way to ensure a compatible address is to prohibit memory above
4GB.
If this is true, then why does PHYS_64BIT have that big ugly list of
dependencies right now?
Because Kumar doesn't want to allow 32-bit kernels on our high-end parts. Like I said, I'm not a fan of that decision.
I don't know about the other platforms well
enough to tell what would break by enabling PHYS_64BIT, but I assume
that something in the past caused that dependency list.
It's just an arbitrary decision. We don't ship 32-bit DTs for the P4080-class parts, and we don't have a U-Boot configuration option for building a 32-bit compatible U-Boot.
--
Timur Tabi
Linux kernel developer at Freescale
From: Scott Wood <hidden> Date: 2011-11-10 16:55:04
On Thu, Nov 10, 2011 at 10:30:41AM -0600, Kumar Gala wrote:
On Nov 10, 2011, at 10:17 AM, Moffett, Kyle D wrote:
quoted
Furthermore, it looks like there are a couple issues here I missed
before. PPC64 systems all appear to have an L1_CACHE_SHIFT of 7,
except when you turn on the P5020DS board option which magically
changes it to "6" and breaks lord-knows-what. I think my patch
series actually "breaks" that and makes e5500 use 7 as well.
a value of '6' on E5500 / P5020DS is correct and doesn't break anything. Setting it to 7 is wrong and thus the code is correct today.
quoted
Are you sure that a kernel built to support E5500 can also run on
other 64-bit PowerPC/POWER systems?
No it will not. There is not expectation of that as E5500 is an
embedded / Book-E class part and uses that ISA version. Book-S
(server) 64-bit machines are not OS compatible and we are not trying to
make them as such (but we do re-use a lot of code).
What about other 64-bit book3e chips? What cache block size does A2 have?
-Scott
From: Scott Wood <hidden> Date: 2011-11-10 17:03:44
On Thu, Nov 10, 2011 at 10:42:25AM -0600, Kumar Gala wrote:
On Nov 10, 2011, at 10:31 AM, Scott Wood wrote:
quoted
On Thu, Nov 10, 2011 at 07:40:04AM -0600, Kumar Gala wrote:
quoted
Nak, we can run an e500mc in a mode that is compatible with e500v1/v2. I see no reason to change the support we have there.
What "mode" do you mean? DCBZ32? We don't support using that currently,
and I'd imagine the performance implication would be such that you'd
never want to do it unless it's the only way to make some piece of legacy
software work.
Correct, DCBZ32, we've had customers that go down this path.
For running legacy software, or for multiplatform Linux kernels?
And if you're willing to toss performance away for this goal, why do you
need lwsync? :-)
DCBZ32 is not a "mode that is compatible with v1/v2", BTW. It only
affects cache block size (for dcbz/dcba only), not SPE versus FP, not
changes in power management, not changes in machine check handling, etc.
Using DCBZ32 for the kernel would also complicate switching the kernel to
dcbzl, to support enabling DCBZ32 for certain userspace apps (a more
likely use case) without making it systemwide.
-Scott
From: Moffett, Kyle D <hidden> Date: 2011-11-10 17:18:14
On Nov 10, 2011, at 08:59, Kumar Gala wrote:
On Nov 9, 2011, at 6:03 PM, Kyle Moffett wrote:
quoted
I saw Baruch Siach's patch:
powerpc: 85xx: separate e500 from e500mc
=20
Unfortunately, that patch breaks the dependencies for the P5020DS
platform and does not fix the underlying code which does not
understand what the ambiguous "CONFIG_E500" means.
=20
In order to fix the issue at the fundamental level, I created the
following 17-patch series loosely based on Baruch's patch.
=20
=3D=3D=3D High-Level Summary =3D=3D=3D
=20
The e500v1/v2 and e500mc/e5500 CPU families are not compatible with
each other, yet they share the same "CONFIG_E500" Kconfig option.
=20
The following patch series splits the 32-bit CPU support into two
separate options: "CONFIG_FSL_E500_V1_V2" and "CONFIG_FSL_E500MC".
Additionally, the 64-bit e5500 support is separated to its own config
option ("CONFIG_FSL_E5500") which is automatically combined with
either 32-bit e500MC or 64-bit Book-3E when the P5020DS board support
is enabled.
=20
So its clear from the community that there is confusion here and we
need to clean this up. I guess my attempt to support an kernel that
ran on both E500v2 and E500mc isn't worth it. However I don't want to
completely remove the ability to do this.
Well, a kernel built with CONFIG_PPC_E500MC today appears to be
fundamentally broken on E500v1/E500v2:
#if defined(CONFIG_8xx) || defined(CONFIG_403GCX)
#define L1_CACHE_SHIFT 4
#define MAX_COPY_PREFETCH 1
#elif defined(CONFIG_PPC_E500MC)
#define L1_CACHE_SHIFT 6
#define MAX_COPY_PREFETCH 4
#elif defined(CONFIG_PPC32)
#define MAX_COPY_PREFETCH 4
#if defined(CONFIG_PPC_47x)
#define L1_CACHE_SHIFT 7
#else
#define L1_CACHE_SHIFT 5
#endif
#else /* CONFIG_PPC64 */
#define L1_CACHE_SHIFT 7
#endif
E500MC will set L1_CACHE_SHIFT to 6, while regular E500 appears to
want it set to 5. I don't know if that's a mistake or exactly what
code that affects, but it looks very wrong.
Furthermore, it looks like there are a couple issues here I missed
before. PPC64 systems all appear to have an L1_CACHE_SHIFT of 7,
except when you turn on the P5020DS board option which magically
changes it to "6" and breaks lord-knows-what. I think my patch
series actually "breaks" that and makes e5500 use 7 as well.
Are you sure that a kernel built to support E5500 can also run on
other 64-bit PowerPC/POWER systems?
Towards the cleanup I'd ask for a proposal on what exactly the
CONFIG_ options we'd end up with would be and their meaning.
So today we have:
=20
CONFIG_E500
CONFIG_PPC_E500MC
It's actually a bit more complicated than that. There are 3 ways
that the user can configure an e500 kernel today. I'm omitting
the "FSL_SOC_BOOKE" menu that wraps around all of the 85xx/e5500
boards today, because that is set for all of these platforms:
* PPC32 + PPC_85xx + E500 [+ boards]
* PPC64 + BOOK3E_64 + P5020_DS (which adds E500 and PPC_E500MC)
Note that whether or not "PPC_E500MC" is set on PPC32 depends
only on which boards the user picked. So if I am trying to
build an e500v2 kernel and I accidentally also turn on support
for one of the e500mc boards, my kernel mysteriously breaks.
What do we want to move to? I want to keep the builds such that we
have only 2 classes: e500V1/V2 and e500mc/e5500/e6500/.../eX500.
I see no reason to hyper-optimize e500mc vs e5500 vs e6500.
So after my changes, there are the following user-configurable
option sets:
* PPC32 + FSL_E500_V1_V2 [+ e500v1/v2 boards]
* PPC32 + FSL_E500MC [+ e500mc boards]
* PPC64 + BOOK3E_64 + P5020_DS (which adds FSL_E5500)
Since most of the "e500mc"-specific code was in 32-bit-only ASM
or inside of #ifdef PPC32, the new FSL_E500MC option is only
set on 32-bit builds, even if it is running in compat mode on
64-bit e5500 hardware)
Internally the P5020_DS option turns on the hidden FSL_E5500
option for both 32-bit and 64-bit; that config option enables
platform drivers and similar stuff.
Depending on how compatible the AMP processors are, you could
rename the option to be "FSL_E5X00" or add a hidden option for
"FSL_E6500" that is also selected by appropriate boards.
Please let me know if you think!
Cheers,
Kyle Moffett
--
Curious about my work on the Debian powerpcspe port?
I'm keeping a blog here: http://pureperl.blogspot.com/
From: Scott Wood <hidden> Date: 2011-11-10 17:38:07
BOn Wed, Nov 09, 2011 at 07:07:12PM -0500, Kyle Moffett wrote:
The only systems which need the sparse PowerPC register map are the
e500v1/e500v2.
Is this due to SPE?
+/*
+ * On FreeScale e500v1 or e500v2 processors we need to skip some register
+ * sections, so just add up a list of what we need to store.
+ *
+ * On all other 32-bit PowerPC we can just pick the last needed register.
+ */
The "s" in "Freescale" is not capitalized, BTW.
-Scott
From: Kumar Gala <hidden> Date: 2011-11-10 18:53:27
On Nov 10, 2011, at 10:47 AM, Scott Wood wrote:
On Wed, Nov 09, 2011 at 07:07:14PM -0500, Kyle Moffett wrote:
quoted
Only build the setup functions when the corresponding entries are
included in the CPU table.
=20
Signed-off-by: Kyle Moffett <redacted>
---
arch/powerpc/kernel/cpu_setup_fsl_booke.S | 14 +++++++++++++-
1 files changed, 13 insertions(+), 1 deletions(-)
=20
From: Moffett, Kyle D <hidden> Date: 2011-11-10 20:28:45
On Nov 10, 2011, at 12:03, Scott Wood wrote:
On Thu, Nov 10, 2011 at 10:42:25AM -0600, Kumar Gala wrote:
quoted
=20
On Nov 10, 2011, at 10:31 AM, Scott Wood wrote:
=20
quoted
On Thu, Nov 10, 2011 at 07:40:04AM -0600, Kumar Gala wrote:
quoted
Nak, we can run an e500mc in a mode that is compatible with e500v1/v2.
I see no reason to change the support we have there.
=20
What "mode" do you mean? DCBZ32? We don't support using that currentl=
y,
quoted
quoted
and I'd imagine the performance implication would be such that you'd
never want to do it unless it's the only way to make some piece of lega=
cy
quoted
quoted
software work.
=20
Correct, DCBZ32, we've had customers that go down this path.
=20
For running legacy software, or for multiplatform Linux kernels?
=20
And if you're willing to toss performance away for this goal, why do you
need lwsync? :-)
=20
DCBZ32 is not a "mode that is compatible with v1/v2", BTW. It only
affects cache block size (for dcbz/dcba only), not SPE versus FP, not
changes in power management, not changes in machine check handling, etc.
=20
Using DCBZ32 for the kernel would also complicate switching the kernel to
dcbzl, to support enabling DCBZ32 for certain userspace apps (a more
likely use case) without making it systemwide.
So, as far as I can tell the kernel doesn't even try to touch DCBZ32.
Even if it did, if you are building a new kernel that includes this patch,
surely you can actually build a proper e500mc kernel instead of trying to
build a new kernel to run on hardware it wasn't designed to run on, right?
I think the bigger issue is the fact that building a PPC_BOOK3E_64 kernel
with both e5500 and PowerPC A2 support turned on will not actually run on
both. Before my v1-patch-series, machine-check handling is messed up for
PowerPC A2, and afterwards cacheline sizes are messed up for e5500.
Does this mean that PPC_BOOK3E_64 needs to be split into two separate
Book 3-III families the same way that 32-bit has been split? Is there
another way around it?
Cheers,
Kyle Moffett
From: Kumar Gala <hidden> Date: 2011-11-10 20:35:01
On Nov 10, 2011, at 2:27 PM, Moffett, Kyle D wrote:
On Nov 10, 2011, at 12:03, Scott Wood wrote:
quoted
On Thu, Nov 10, 2011 at 10:42:25AM -0600, Kumar Gala wrote:
quoted
=20
On Nov 10, 2011, at 10:31 AM, Scott Wood wrote:
=20
quoted
On Thu, Nov 10, 2011 at 07:40:04AM -0600, Kumar Gala wrote:
quoted
Nak, we can run an e500mc in a mode that is compatible with =
e500v1/v2.
quoted
quoted
quoted
quoted
I see no reason to change the support we have there.
=20
What "mode" do you mean? DCBZ32? We don't support using that =
currently,
quoted
quoted
quoted
and I'd imagine the performance implication would be such that =
you'd
quoted
quoted
quoted
never want to do it unless it's the only way to make some piece of =
legacy
quoted
quoted
quoted
software work.
=20
Correct, DCBZ32, we've had customers that go down this path.
=20
For running legacy software, or for multiplatform Linux kernels?
=20
And if you're willing to toss performance away for this goal, why do =
you
quoted
need lwsync? :-)
=20
DCBZ32 is not a "mode that is compatible with v1/v2", BTW. It only
affects cache block size (for dcbz/dcba only), not SPE versus FP, not
changes in power management, not changes in machine check handling, =
etc.
quoted
=20
Using DCBZ32 for the kernel would also complicate switching the =
kernel to
quoted
dcbzl, to support enabling DCBZ32 for certain userspace apps (a more
likely use case) without making it systemwide.
=20
So, as far as I can tell the kernel doesn't even try to touch DCBZ32.
Correct, it was my thinking I'd get there an add this one day, that day =
never came.
Even if it did, if you are building a new kernel that includes this =
patch,
surely you can actually build a proper e500mc kernel instead of trying =
to
build a new kernel to run on hardware it wasn't designed to run on, =
right?
=20
I think the bigger issue is the fact that building a PPC_BOOK3E_64 =
kernel
with both e5500 and PowerPC A2 support turned on will not actually run =
on
both. Before my v1-patch-series, machine-check handling is messed up =
for
PowerPC A2, and afterwards cacheline sizes are messed up for e5500.
That might be, but who is asking or wanting to run a BOOK3E_64 kernel on =
both. I'm guessing there are a number of issues with this.
Does this mean that PPC_BOOK3E_64 needs to be split into two separate
Book 3-III families the same way that 32-bit has been split? Is there
another way around it?
No idea, we have to ask Ben how much he cares. I don't see any FSL =
customers pushing us to run the same kernel on A2 and P5020 (or future =
FSL devices).
- k=
From: Moffett, Kyle D <hidden> Date: 2011-11-11 00:38:56
On Nov 10, 2011, at 11:54, Scott Wood wrote:
On Thu, Nov 10, 2011 at 10:30:41AM -0600, Kumar Gala wrote:
quoted
On Nov 10, 2011, at 10:17 AM, Moffett, Kyle D wrote:
quoted
Furthermore, it looks like there are a couple issues here I missed
before. PPC64 systems all appear to have an L1_CACHE_SHIFT of 7,
except when you turn on the P5020DS board option which magically
changes it to "6" and breaks lord-knows-what. I think my patch
series actually "breaks" that and makes e5500 use 7 as well.
=20
a value of '6' on E5500 / P5020DS is correct and doesn't break anything.
Setting it to 7 is wrong and thus the code is correct today.
=20
quoted
Are you sure that a kernel built to support E5500 can also run on
other 64-bit PowerPC/POWER systems?
=20
No it will not. There is not expectation of that as E5500 is an
embedded / Book-E class part and uses that ISA version. Book-S
(server) 64-bit machines are not OS compatible and we are not trying to
make them as such (but we do re-use a lot of code).
=20
What about other 64-bit book3e chips? What cache block size does A2 have=
?
Ok, so I've been poking around this code a bunch and as far as I can
tell, the cacheline stuff has basically always been subtly wrong in
twelve different ways and it's only largely coincidence that it works
today.
So PowerPC64 systems have their own "ppc64_caches" structure set up
before start_kernel() is called by parsing the OpenFirmware "cpu" nodes.
That structure is then checked in every piece of 64-bit kernel code
(except xmon) that uses the "dcbXX" and "icbXX" opcodes.
There is an entirely separate mechanism built into the "cputable" that
is used on all PowerPC systems to compute cacheline sizes to pass in via
ELF headers for userspace to use in memset()/memcpy(), etc.
Furthermore, the VDSO gets cacheline sizes stored into it, but on 64-bit
they come from the ppc64_caches structure and on 32-bit they come from
dcache_bsize/icache_bsize copied from the cputable.
Then there's the value in arch/powerpc/include/asm/cache.h which is used
throughout the kernel to figure out how far apart to space CPU-specific
datastructures (EG: __cacheline_aligned_on_smp).
Despite the fact that all PPC64 have an "L1_CACHE_SIZE" value of 128,
the PowerPC A2 and e5500 have {d,i}cache_bsize values of 64 in cputable
and presumably also get correct values from OpenFirmware, so the bogus
constant in asm/cache.h does nothing more than waste a bit of memory
for unnecessary padding.
Unfortunately, lots of PPC32 assembly pretends that the value found in
asm/cache.h is a hard truth and uses it for "dcbz", etc, which is why
there are all of those ugly #ifdefs in asm/cache.h
Based on all of that, my proposal is going to be a patch which does the
following:
(1) Conditionally set L1_CACHE_SHIFT to the maximum value used by any
platform being compiled in for alignment purposes.
(2) Make the ppc64_caches struct apply to ppc32 as well, and
preinitialize it with a minimum value used by any platform being
compiled in (for "dcbXX"/"icbXX" purposes). This is safe because
the pagesize is always a multiple of the cache block size and the
kernel only uses dcbXX/icbXX on whole pages. The only impact is a
temporary small performance hit from flushing or zeroing the same
block 8 times if too small.
(3) Try to initialize the ppc_caches struct on ppc32 from the
OpenFirmware device-tree. If that fails, then use the values we
find in the cputable. After this is initialized any performance
hit in copy_page()/zero_page() will obviously disappear.
(4) Fix all of the PPC32 assembly code that is misusing L1_CACHE_SHIFT
to use the ppc_caches struct instead.
Does that sound like a reasonable approach?
Cheers,
Kyle Moffett
--
Curious about my work on the Debian powerpcspe port?
I'm keeping a blog here: http://pureperl.blogspot.com/
From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: 2011-11-11 04:40:40
On Thu, 2011-11-10 at 18:38 -0600, Moffett, Kyle D wrote:
Ok, so I've been poking around this code a bunch and as far as I can
tell, the cacheline stuff has basically always been subtly wrong in
twelve different ways and it's only largely coincidence that it works
today.
Yay ! Somebody to clean that shit up ! :-)
That's the biggest missing step to being able to have 440 and 476 in a
single binary :-)
So PowerPC64 systems have their own "ppc64_caches" structure set up
before start_kernel() is called by parsing the OpenFirmware "cpu" nodes.
That structure is then checked in every piece of 64-bit kernel code
(except xmon) that uses the "dcbXX" and "icbXX" opcodes.
Yup. (And we should really fix xmon btw...)
There is an entirely separate mechanism built into the "cputable" that
is used on all PowerPC systems to compute cacheline sizes to pass in via
ELF headers for userspace to use in memset()/memcpy(), etc.
Yeah well, it actually uses global variables which are set from cputable
on ppc32 and from the ppc64_caches structure on ppc64. Yeah it's not
pretty.
Furthermore, the VDSO gets cacheline sizes stored into it, but on 64-bit
they come from the ppc64_caches structure and on 32-bit they come from
dcache_bsize/icache_bsize copied from the cputable.
Yup.
Then there's the value in arch/powerpc/include/asm/cache.h which is used
throughout the kernel to figure out how far apart to space CPU-specific
datastructures (EG: __cacheline_aligned_on_smp).
Not much we can do about that one since it has to be compile time. Maybe
something like calculating the biggest cache line size supported by all
built-in processor types ?
Despite the fact that all PPC64 have an "L1_CACHE_SIZE" value of 128,
the PowerPC A2 and e5500 have {d,i}cache_bsize values of 64 in cputable
and presumably also get correct values from OpenFirmware, so the bogus
constant in asm/cache.h does nothing more than waste a bit of memory
for unnecessary padding.
More or less yes, though we haven't totally given up on the idea of
eventually, one day, produce binaries capable of running both 64-bit S
and E :-)
Unfortunately, lots of PPC32 assembly pretends that the value found in
asm/cache.h is a hard truth and uses it for "dcbz", etc, which is why
there are all of those ugly #ifdefs in asm/cache.h
Yes, well... -some- assembly, mostly the copy routines. It's been the
main reason why this hasn't been fixed yet.
Based on all of that, my proposal is going to be a patch which does the
following:
(1) Conditionally set L1_CACHE_SHIFT to the maximum value used by any
platform being compiled in for alignment purposes.
Yay !
(2) Make the ppc64_caches struct apply to ppc32 as well, and
preinitialize it with a minimum value used by any platform being
compiled in (for "dcbXX"/"icbXX" purposes). This is safe because
the pagesize is always a multiple of the cache block size and the
kernel only uses dcbXX/icbXX on whole pages. The only impact is a
temporary small performance hit from flushing or zeroing the same
block 8 times if too small.
Are you sure about dcbz ? Getting that wrong can be deadly ... I'd
rather get rid of some fancy optims and use a soft value in some cases.
That or we can compile multiple variants for the common case of some of
the copy routines and use patching (alternate sections) to branch to the
right one at runtime, at least for the common cases (32 and 128 for
example for 440 and 476).
(3) Try to initialize the ppc_caches struct on ppc32 from the
OpenFirmware device-tree. If that fails, then use the values we
find in the cputable. After this is initialized any performance
hit in copy_page()/zero_page() will obviously disappear.
(4) Fix all of the PPC32 assembly code that is misusing L1_CACHE_SHIFT
to use the ppc_caches struct instead.
Yes. This could be done while keeping the hand-optimized stuff by
compiling several variants of it.
Does that sound like a reasonable approach?
It absolutely does ! Thanks for looking at that, it's been on my todo
list for ages and I've been always finding good reasons to do something
else instead :-)
Cheers,
Ben.
Cheers,
Kyle Moffett
--
Curious about my work on the Debian powerpcspe port?
I'm keeping a blog here: http://pureperl.blogspot.com/
From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: 2011-11-11 04:43:46
On Thu, 2011-11-10 at 14:27 -0600, Moffett, Kyle D wrote:
Does this mean that PPC_BOOK3E_64 needs to be split into two separate
Book 3-III families the same way that 32-bit has been split? Is there
another way around it?
No, I don't want more split, on the contrary.
Cheers,
Ben.
From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: 2011-11-11 04:46:08
On Thu, 2011-11-10 at 14:34 -0600, Kumar Gala wrote:
No idea, we have to ask Ben how much he cares. I don't see any FSL
customers pushing us to run the same kernel on A2 and P5020 (or future
FSL devices).
I do care. For example, imagine somebody wanting to support an
enterprise distro on both BG/Q and some FSL based HW ...
Besides, this has generally forced us to do things more cleanly and I
don't want to go back into #ifdef land. These cores are both arch 2.06 E
compilant, there is no good reason to prevent or forbid having them
build into a single binary image.
Look at the mess ARM got into and the pain they are having getting out
of with that stuff ... No way I'm going backward with split configs. If
anything, I'd like to reconcile things even more.
Cheers,
Ben.
From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: 2011-11-11 04:51:51
On Thu, 2011-11-10 at 10:50 -0600, Timur Tabi wrote:
I'm not sure that Kconfig rule is enforceable. We use 'select' all the time to make sure that our boards enable the options that need to be enabled. I'd rather see this:
config X
select A
config Y
select A
config Z
select A
than this:
config A
default y if X | Y | Z
The former is more maintainable and does not allow the option of disabling "A" if someone wants "X".
This is fine as long as config A is not a user selectable option.
We do the wrong thing in a few places (i2c comes to mind) and we should
most certainly consider fixing it. Some of the solution is to move some
select's to defconfigs, some is to do things the way Kyle proposed.
For PHYS_64BIT, I'm not 100% certain at this point what the best
approach it. It should clearly be fixed and hidden for real 64-bit. As
for the rest, should it be user selectable ? I tend to think it should
be fixed & selected by the board config and keep it that way, avoiding
the user question entirely... but you may have good reasons to want to
keep it around.
Cheers,
Ben.
From: Tabi Timur-B04825 <hidden> Date: 2011-11-11 13:12:38
Benjamin Herrenschmidt wrote:
For PHYS_64BIT, I'm not 100% certain at this point what the best
approach it. It should clearly be fixed and hidden for real 64-bit. As
for the rest, should it be user selectable ?
I have been unable to convince our BSP teams that the minor performance=20
improvement of a 32-bit kernel vs 36-bit is not worth the hassle. We ship=
=20
a lot of lower-end e500 systems that will never have more than 2GB of memor=
y.
The P1022DS is an example. We ship 32-bit and 36-bit versions of U-Boot,=20
the kernel, and the device tree. I recently added code to U-boot to=20
detect if you have a mismatched U-Boot and device tree, because it is so=20
easy to mess that up. I would like nothing more than to abandon 32-bit=20
for all e500, but I don't see Freescale allowing that any time soon.
--=20
Timur Tabi
Linux kernel developer at Freescale=
This badly needs breaking up, and a better changelog... oh well...
The big changes:
* The "ppc64_caches" structure is now "powerpc_caches" and is used on
both PPC32 and PPC64. I hated staring at the pages and pages of
assembly code, so nearly all of the functions are now C with tiny
snippets of inline ASM in the loops.
* Lots of ugly assembly functions in arch/powerpc/kernel/misc_*.S were
rewritten as cleaner inline ASM in arch/powerpc/mm/cache.c
* I'm not sure that the physical address functions from those files
actually came out cleaner, but they are now more correct.
* I'm not 100% sure I like the new FOR_EACH_CACHE_LINE() macro, but it
sure does make a lot of the other code much cleaner.
* I have a bit of a temptation to try to merge the 32/64-bit variants
of copy_page() into a single C function. A quick test seems to show
that I can get nearly identical output to the 64-bit ASM with very
little work.
---
arch/powerpc/include/asm/cache.h | 155 ++++++++++++---
arch/powerpc/include/asm/cacheflush.h | 3 -
arch/powerpc/include/asm/page.h | 6 +
arch/powerpc/include/asm/page_32.h | 4 +-
arch/powerpc/include/asm/page_64.h | 17 --
arch/powerpc/kernel/align.c | 7 +-
arch/powerpc/kernel/asm-offsets.c | 13 +-
arch/powerpc/kernel/head_32.S | 9 +-
arch/powerpc/kernel/head_64.S | 2 +-
arch/powerpc/kernel/misc_32.S | 193 ------------------
arch/powerpc/kernel/misc_64.S | 182 -----------------
arch/powerpc/kernel/ppc_ksyms.c | 3 -
arch/powerpc/kernel/setup-common.c | 103 ++++++++++
arch/powerpc/kernel/setup.h | 1 +
arch/powerpc/kernel/setup_32.c | 11 +-
arch/powerpc/kernel/setup_64.c | 118 +----------
arch/powerpc/kernel/vdso.c | 27 +--
arch/powerpc/lib/copypage_64.S | 10 +-
arch/powerpc/mm/Makefile | 2 +-
arch/powerpc/mm/cache.c | 279 ++++++++++++++++++++++++++
arch/powerpc/mm/dma-noncoherent.c | 2 +-
arch/powerpc/platforms/52xx/lite5200_sleep.S | 9 +-
arch/powerpc/platforms/powermac/pci.c | 2 +-
arch/powerpc/xmon/xmon.c | 53 +++---
drivers/macintosh/smu.c | 8 +-
25 files changed, 599 insertions(+), 620 deletions(-)
create mode 100644 arch/powerpc/mm/cache.c
@@ -321,199 +321,6 @@ END_FTR_SECTION_IFSET(CPU_FTR_UNIFIED_ID_CACHE)blr/*-*Writeanymodifieddatacacheblocksouttomemory-*andinvalidatethecorrespondinginstructioncacheblocks.-*Thisisano-oponthe601.-*-*flush_icache_range(unsignedlongstart,unsignedlongstop)-*/-_KPROBE(__flush_icache_range)-BEGIN_FTR_SECTION-blr/*for601,donothing*/-END_FTR_SECTION_IFSET(CPU_FTR_COHERENT_ICACHE)-lir5,L1_CACHE_BYTES-1-andcr3,r3,r5-subfr4,r3,r4-addr4,r4,r5-srwi.r4,r4,L1_CACHE_SHIFT-beqlr-mtctrr4-mrr6,r3-1:dcbst0,r3-addir3,r3,L1_CACHE_BYTES-bdnz1b-sync/*waitfordcbst's to get to ram */-#ifndef CONFIG_44x-mtctrr4-2:icbi0,r6-addir6,r6,L1_CACHE_BYTES-bdnz2b-#else-/*Flashinvalidateon44xbecausewearepassedkmappedaddressesand-thisdoesn't work for userspace pages due to the virtually tagged-icache.Sigh.*/-iccci0,r0-#endif-sync/*additionalsyncneededong4*/-isync-blr-/*-*Writeanymodifieddatacacheblocksouttomemory.-*Doesnotinvalidatethecorrespondingcachelines (especiallyfor-*anycorrespondinginstructioncache).-*-*clean_dcache_range(unsignedlongstart,unsignedlongstop)-*/-_GLOBAL(clean_dcache_range)-lir5,L1_CACHE_BYTES-1-andcr3,r3,r5-subfr4,r3,r4-addr4,r4,r5-srwi.r4,r4,L1_CACHE_SHIFT-beqlr-mtctrr4--1:dcbst0,r3-addir3,r3,L1_CACHE_BYTES-bdnz1b-sync/*waitfordcbst's to get to ram */-blr--/*-*Writeanymodifieddatacacheblocksouttomemoryandinvalidatethem.-*Doesnotinvalidatethecorrespondinginstructioncacheblocks.-*-*flush_dcache_range(unsignedlongstart,unsignedlongstop)-*/-_GLOBAL(flush_dcache_range)-lir5,L1_CACHE_BYTES-1-andcr3,r3,r5-subfr4,r3,r4-addr4,r4,r5-srwi.r4,r4,L1_CACHE_SHIFT-beqlr-mtctrr4--1:dcbf0,r3-addir3,r3,L1_CACHE_BYTES-bdnz1b-sync/*waitfordcbst's to get to ram */-blr--/*-*Likeabove,butinvalidatetheD-cache.Thisisusedbythe8xx-*toinvalidatethecachesothePPCcoredoesn't get stale data-*fromtheCPM (nocachesnoopinghere:-).-*-*invalidate_dcache_range(unsignedlongstart,unsignedlongstop)-*/-_GLOBAL(invalidate_dcache_range)-lir5,L1_CACHE_BYTES-1-andcr3,r3,r5-subfr4,r3,r4-addr4,r4,r5-srwi.r4,r4,L1_CACHE_SHIFT-beqlr-mtctrr4--1:dcbi0,r3-addir3,r3,L1_CACHE_BYTES-bdnz1b-sync/*waitfordcbi's to get to ram */-blr--/*-*FlushaparticularpagefromthedatacachetoRAM.-*Note:thisisnecessarybecausetheinstructioncachedoes*not*-*snoopfromthedatacache.-*Thisisano-oponthe601whichhasaunifiedcache.-*-*void__flush_dcache_icache(void*page)-*/-_GLOBAL(__flush_dcache_icache)-BEGIN_FTR_SECTION-blr-END_FTR_SECTION_IFSET(CPU_FTR_COHERENT_ICACHE)-rlwinmr3,r3,0,0,31-PAGE_SHIFT/*Getpagebaseaddress*/-lir4,PAGE_SIZE/L1_CACHE_BYTES/*Numberoflinesinapage*/-mtctrr4-mrr6,r3-0:dcbst0,r3/*Writelinetoram*/-addir3,r3,L1_CACHE_BYTES-bdnz0b-sync-#ifdef CONFIG_44x-/*Wedon't flush the icache on 44x. Those have a virtual icache-*andwedon't have access to the virtual address here (it's-*notthepagevaddrbutwhereit's mapped in user space). The-*flushingoftheicacheontheseishandledelsewhere,when-*achangeintheaddressspaceoccurs,beforereturningto-*userspace-*/-BEGIN_MMU_FTR_SECTION-blr-END_MMU_FTR_SECTION_IFSET(MMU_FTR_TYPE_44x)-#endif /* CONFIG_44x */-mtctrr4-1:icbi0,r6-addir6,r6,L1_CACHE_BYTES-bdnz1b-sync-isync-blr--#ifndef CONFIG_BOOKE-/*-*FlushaparticularpagefromthedatacachetoRAM,identified-*byitsphysicaladdress.WeturnofftheMMUsowecanjustuse-*thephysicaladdress (thismaybeahighmempagewithoutakernel-*mapping).-*-*void__flush_dcache_icache_phys(unsignedlongphysaddr)-*/-_GLOBAL(__flush_dcache_icache_phys)-BEGIN_FTR_SECTION-blr/*for601,donothing*/-END_FTR_SECTION_IFSET(CPU_FTR_COHERENT_ICACHE)-mfmsrr10-rlwinmr0,r10,0,28,26/*clearDR*/-mtmsrr0-isync-rlwinmr3,r3,0,0,31-PAGE_SHIFT/*Getpagebaseaddress*/-lir4,PAGE_SIZE/L1_CACHE_BYTES/*Numberoflinesinapage*/-mtctrr4-mrr6,r3-0:dcbst0,r3/*Writelinetoram*/-addir3,r3,L1_CACHE_BYTES-bdnz0b-sync-mtctrr4-1:icbi0,r6-addir6,r6,L1_CACHE_BYTES-bdnz1b-sync-mtmsrr10/*restoreDR*/-isync-blr-#endif /* CONFIG_BOOKE */--/*-*Clearpagesusingthedcbzinstruction,whichdoesn't cause any-*memorytraffic (excepttowriteoutanycachelineswhichget-*displaced).Thisonlyworksoncacheablememory.-*-*voidclear_pages(void*page,intorder);-*/-_GLOBAL(clear_pages)-lir0,PAGE_SIZE/L1_CACHE_BYTES-slwr0,r0,r4-mtctrr0-1:dcbz0,r3-addir3,r3,L1_CACHE_BYTES-bdnz1b-blr--/**Copyawholepage.Weusethedcbzinstructiononthedestination*toreducememorytraffic (iteliminatestheunnecessaryreadsof*thedestinationintocache).Thisrequiresthatthedestination
@@ -83,6 +83,54 @@ unsigned long klimit = (unsigned long) _end;charcmd_line[COMMAND_LINE_SIZE];/*+*Initializethesevaluestominimumsafedefaultsincasetheyneedtobe+*usedearlyduringthebootprocess.Whilethismaynotseemsafe,itis+*actuallysafeinpractice,becauseallofthekernelloopsthatusethis+*dataoperateonwholepages.+*+*ThePowerPCBookIII-Especdocumentsthatthepagesizeisaneven+*multipleofthecacheblocksizeandthecacheblocksarealways+*page-aligned.+*+*So,forexample,whenclearingawholepagethereareonlytwothingsthat+*canbedonewrongwith"dcbz":+*+*(1)Call"dcbz"withanaddressoutsidethepageyouwanttozero.+*+*(2)Call"dcbz"toofewtimestoactuallyhitallofthecachelines,+*IE:Useatoo-largecachelinestride.+*+*Soaslongasweensurethatthisnumberissmallenoughforthecurrent+*CPUeverythingwilloperatecorrectly,albeitwithaslightperformance+*hit,untilwegetachancetoparsethedevice-treefortherightvalue.+*+*NOTE:Userspaceexpectsanexactvalue,sononeoftheaboveappliesafter+*thedevicetreehasbeenunflattenedandactualvaluescomputed.+*+*Seearch/powerpc/asm/caches.hformoreinformation.+*/+structpowerpc_cachespowerpc_caches={+/* Data cache sizes */+.dcache_total_bytes=0,/* Unknown */+.dcache_block_bytes=L1_CACHE_BYTES_MIN,+.dcache_block_shift=L1_CACHE_SHIFT_MIN,+.dcache_blocks_per_page=(PAGE_SIZE>>L1_CACHE_SHIFT_MIN),++/* Instruction cache sizes */+.icache_total_bytes=0,+.icache_block_bytes=L1_CACHE_BYTES_MIN,+.icache_block_shift=L1_CACHE_SHIFT_MIN,+.icache_blocks_per_page=(PAGE_SIZE>>L1_CACHE_SHIFT_MIN),++/* Unified cache (assume cache is split by default) */+.ucache_total_bytes=0,+.ucache_block_bytes=0,+.ucache_block_shift=0,+.ucache_blocks_per_page=0,+};+EXPORT_SYMBOL_GPL(powerpc_caches);++/**Thisstillseemstobeneeded...--paulus*/structscreen_infoscreen_info={
@@ -349,6 +397,61 @@ const struct seq_operations cpuinfo_op = {.show=show_cpuinfo,};+/* Helper functions to compute various values from a cache block size */+staticvoid__initset_dcache_block_data(u32bytes)+{+u32shift=__ilog2(bytes);+powerpc_caches.dcache_block_bytes=bytes;+powerpc_caches.dcache_block_shift=shift;+powerpc_caches.dcache_blocks_per_page=(PAGE_SIZE>>shift);+}+staticvoid__initset_icache_block_data(u32bytes)+{+u32shift=__ilog2(bytes);+powerpc_caches.icache_block_bytes=bytes;+powerpc_caches.icache_block_shift=shift;+powerpc_caches.icache_blocks_per_page=(PAGE_SIZE>>shift);+}++/*+*Preinitializethepowerpc_cachesstructurefromthecputable.Wewill+*laterscanthedevice-treeforthisinformation,whichmaybemore+*accurate.+*/+void__initinitialize_early_cache_info(void)+{+set_dcache_block_data(cur_cpu_spec->dcache_bsize);+set_icache_block_data(cur_cpu_spec->icache_bsize);+}++/*+*Initializethepowerpc_cachesstructurefromthedevice-treeforuseby+*copy_page(),cacheflushroutines,andAT_DCACHEBSIZEelfheaders.+*+*Intheunlikelyeventthatthedevice-treedoesn'thavethisinformation,+*thedefaultsloadedbyinitialize_early_cache_info()fromthecputable+*willbeused.+*/+void__initinitialize_cache_info(void)+{+/* Assume that the cache properties are the same across all nodes */+structdevice_node*np=of_find_node_by_type(NULL,"cpu");+u32value=0;++/* First check data/instruction cache block sizes */+if(!of_property_read_u32(np,"d-cache-block-size",&value)||+!of_property_read_u32(np,"d-cache-line-size",&value))+set_dcache_block_data(value);++if(!of_property_read_u32(np,"i-cache-block-size",&value)||+!of_property_read_u32(np,"i-cache-line-size",&value))+set_icache_block_data(value);++/* Also read total cache sizes (no defaults here) */+of_property_read_u32(np,"d-cache-size",&powerpc_caches.dcache_total_bytes);+of_property_read_u32(np,"i-cache-size",&powerpc_caches.icache_total_bytes);+}+void__initcheck_for_initrd(void){#ifdef CONFIG_BLK_DEV_INITRD
@@ -77,25 +77,6 @@ int boot_cpuid = 0;int__initdataspinning_secondaries;u64ppc64_pft_size;-/* Pick defaults since we might want to patch instructions-*beforewe'vereadthisfromthedevicetree.-*/-structppc64_cachesppc64_caches={-.dline_size=0x40,-.log_dline_size=6,-.iline_size=0x40,-.log_iline_size=6-};-EXPORT_SYMBOL_GPL(ppc64_caches);--/*-*Theseareusedinbinfmt_elf.ctoputauxentriesonthestack-*foreachelfexecutablebeingstarted.-*/-intdcache_bsize;-inticache_bsize;-intucache_bsize;-#ifdef CONFIG_SMPstaticchar*smt_enabled_cmdline;
@@ -365,10 +270,7 @@ void __init setup_system(void)*/unflatten_device_tree();-/*-*Filltheppc64_caches&systemcfgstructureswithinformations-*retrievedfromthedevice-tree.-*/+/* Fill the powerpc_caches structure with device-tree data */initialize_cache_info();#ifdef CONFIG_PPC_RTAS
@@ -726,6 +726,7 @@ static int __init vdso_init(void)vdso_data->version.major=SYSTEMCFG_MAJOR;vdso_data->version.minor=SYSTEMCFG_MINOR;vdso_data->processor=mfspr(SPRN_PVR);+/**FaketheoldplatformnumberforpSeriesandiSeriesandadd*inLPARbitifnecessary
@@ -734,29 +735,25 @@ static int __init vdso_init(void)if(firmware_has_feature(FW_FEATURE_LPAR))vdso_data->platform|=1;vdso_data->physicalMemorySize=memblock_phys_mem_size();-vdso_data->dcache_size=ppc64_caches.dsize;-vdso_data->dcache_line_size=ppc64_caches.dline_size;-vdso_data->icache_size=ppc64_caches.isize;-vdso_data->icache_line_size=ppc64_caches.iline_size;-/* XXXOJN: Blocks should be added to ppc64_caches and used instead */-vdso_data->dcache_block_size=ppc64_caches.dline_size;-vdso_data->icache_block_size=ppc64_caches.iline_size;-vdso_data->dcache_log_block_size=ppc64_caches.log_dline_size;-vdso_data->icache_log_block_size=ppc64_caches.log_iline_size;+/* There are more cache parameters saved for 64-bit than 32-bit */+vdso_data->dcache_size=powerpc_caches.dcache_total_size;+vdso_data->icache_size=powerpc_caches.icache_total_size;+vdso_data->dcache_line_size=powerpc_caches.dcache_block_bytes;+vdso_data->icache_line_size=powerpc_caches.icache_block_bytes;/**Calculatethesizeofthe64bitsvDSO*/vdso64_pages=(&vdso64_end-&vdso64_start)>>PAGE_SHIFT;DBG("vdso64_kbase: %p, 0x%x pages\n",vdso64_kbase,vdso64_pages);-#else-vdso_data->dcache_block_size=L1_CACHE_BYTES;-vdso_data->dcache_log_block_size=L1_CACHE_SHIFT;-vdso_data->icache_block_size=L1_CACHE_BYTES;-vdso_data->icache_log_block_size=L1_CACHE_SHIFT;-#endif /* CONFIG_PPC64 */+#endif+/* Save the cache-block sizes for the VDSO */+vdso_data->dcache_block_size=powerpc_caches.dcache_block_bytes;+vdso_data->icache_block_size=powerpc_caches.icache_block_bytes;+vdso_data->dcache_log_block_size=powerpc_caches.dcache_block_shift;+vdso_data->icache_log_block_size=powerpc_caches.icache_block_shift;/**Calculatethesizeofthe32bitsvDSO
@@ -0,0 +1,279 @@+#include<linux/kprobes.h>+#include<linux/export.h>+#include<linux/types.h>++#include<asm/cputable.h>+#include<asm/system.h>+#include<asm/cache.h>+#include<asm/page.h>+#include<asm/mmu.h>++/*+*Writeanymodifieddatacacheblocksouttomemory.+*Doesnotinvalidatethecorrespondingcachelines(especiallyfor+*anycorrespondinginstructioncache).+*/+voidclean_dcache_range(unsignedlongstart,unsignedlongstop)+{+unsignedlongaddr;+FOR_EACH_CACHELINE(addr,start,stop,dcache)+dcbst(addr);+mb();+}++/*+*Writeanymodifieddatacacheblocksouttomemoryandinvalidatethem.+*Doesnotinvalidatethecorrespondinginstructioncacheblocks.+*/+voidflush_dcache_range(unsignedlongstart,unsignedlongstop)+{+unsignedlongaddr;+FOR_EACH_CACHELINE(addr,start,stop,dcache)+dcbf(addr);+mb();+}+EXPORT_SYMBOL(flush_dcache_range);++/*+*Likeabove,butinvalidatetheD-cache.Thisisusedbythe8xx+*toinvalidatethecachesothePPCcoredoesn'tgetstaledata+*fromtheCPM(nocachesnoopinghere:-).+*+*invalidate_dcache_range(unsignedlongstart,unsignedlongstop)+*/+voidinvalidate_dcache_range(unsignedlongstart,unsignedlongstop)+{+unsignedlongaddr;+FOR_EACH_CACHELINE(addr,start,stop,dcache)+dcbi(addr);+mb();+}++/*+*Unfortunately,wecannotflushindividualchunksoftheicacheon44xas+*wearepassedkmappedaddressesandwehaveavirtually-taggedicache.+*+*Theonlyworkaroundistoinvalidatethewholeicache.+*+*NOTE:TheCPUdoesnotusetheoperandsforthisinstruction,so+*theyarepassedasdummies.+*/+__kprobesvoid__flush_icache_range(unsignedlongstart,unsignedlongstop)+{+unsignedlongaddr;++if(cpu_has_feature(CPU_FTR_COHERENT_ICACHE))+return;++/* First ensure that data has been written to memory */+FOR_EACH_CACHELINE(addr,start,stop,dcache)+dcbst(addr);+mb();++#ifdef CONFIG_44x+if(mmu_has_feature(MMU_FTR_TYPE_44x)){+asmvolatile("iccci 0, r0":::"memory");+return;+}+#endif++/* Now discard the corresponding icache */+FOR_EACH_CACHELINE(addr,start,stop,icache)+icbi(addr);+mb();+isync();+}+EXPORT_SYMBOL(__flush_icache_range);++/*+*FlushaparticularpagefromthedatacachetoRAM.+*Note:thisisnecessarybecausetheinstructioncachedoes*not*+*snoopfromthedatacache.+*Thisisano-oponthe601whichhasaunifiedcache.+*+*void__flush_dcache_icache(void*page)+*/+void__flush_dcache_icache(void*page)+{+unsignedlongbase=((unsignedlong)page)&~(PAGE_SIZE-1);+unsignedlongaddr;++if(cpu_has_feature(CPU_FTR_COHERENT_ICACHE))+return;++/* First ensure that data has been written to memory */+FOR_EACH_CACHELINE(addr,base,base+PAGE_SIZE,dcache)+dcbst(addr);++#ifdef CONFIG_44x+/*+*Wedon'tflushtheicacheon44x.Thosehaveavirtualicacheand+*wedon'thaveaccesstothevirtualaddresshere(it'snotthe+*pagevaddrbutwhereit'smappedinuserspace).Theflushingof+*theicacheontheseishandledelsewhere,whenachangeinthe+*addressspaceoccurs,beforereturningtouserspace.+*/+if(mmu_has_feature(MMU_FTR_TYPE_44x))+return;+#endif++FOR_EACH_CACHELINE(addr,base,base+PAGE_SIZE,icache)+icbi(addr);++mb();+isync();+}++/*+*Clearpagesusingthedcbzinstruction,whichdoesn'tcauseany+*memorytraffic(excepttowriteoutanycachelineswhichget+*displaced).Thisonlyworksoncacheablememory.+*+*/+voidclear_pages(void*page,intorder)+{+unsignedlongaddr,base=(unsignedlong)page;+FOR_EACH_CACHELINE(addr,base,base+(PAGE_SIZE<<order),dcache)+dcbz(addr);+}+EXPORT_SYMBOL(clear_pages);++#if defined(CONFIG_PPC32) && !defined(CONFIG_BOOKE)+/*+*FlushaparticularpagefromthedatacachetoRAM,identified+*byitsphysicaladdress.WeturnofftheMMUsowecanjustuse+*thephysicaladdress(thismaybeahighmempagewithoutakernel+*mapping).+*/+void__flush_dcache_icache_phys(unsignedlongphys_page)+{+u32d_size=powerpc_caches.dcache_block_bytes;+u32i_size=powerpc_caches.icache_block_bytes;+u32d_per_page=powerpc_caches.dcache_blocks_per_page;+u32i_per_page=powerpc_caches.icache_blocks_per_page;++/* Temporary registers for the ASM to use */+unsignedlongold_msr,tmp_msr,d_phys_page,i_phys_page;++if(cpu_has_feature(CPU_FTR_COHERENT_ICACHE))+return;++/* Page base address (used in 2 different loops) */+d_phys_page=i_phys_page=phys_page&~(PAGE_SIZE-1);++/*+*Thispartneedstobe100%ASMbecausewedisabletheMMU,andwe+*can'taccidentallyletsomeCcodegopokingatmemorywhilethe+*MMUisn'tenabled.+*+*NOTE:Thislooksblatantlyunsafewithrespecttointerrupts.+*Hopefullyallthecallersprovidesufficientprotection?+*/+asmvolatile(+/* First disable the MMU */+"mfmsr %[old_msr]\n\t"+"rlwinm %[tmp_msr], %[old_msr], 0, 28, 26\n\t"+"mtmsr %[tmp_msr]\n\t"+"isync\n\t"++/* Clean the data cache */+"mtctr %[d_per_page]\n"+"0: dcbst 0, %[d_phys_page]\n\t"+"add %[d_phys_page], %[d_phys_page], %[d_size]\n\t"+"bdnz 0b\n\t"+"sync\n\t"++/* Invalidate the instruction cache */+"mtctr %[i_per_page]\n"+"0: icbi 0, %[i_phys_page]\n\t"+"add %[i_phys_page], %[i_phys_page], %[i_size]\n\t"+"bdnz 0b\n\t"++/* Finally, re-enable the MMU */+"sync\n\t"+"mtmsr %[old_msr]\n\t"+"isync\n\t"++/* Temporary variables and inputs */+:[old_msr]"=&r"(old_msr),+[tmp_msr]"=&r"(tmp_msr),+[d_phys_page]"=b"(d_phys_page),+[i_phys_page]"=b"(i_phys_page)++/* Inputs */+:[d_size]"b"(d_size),+[i_size]"b"(i_size),+[d_per_page]"b"(d_per_page),+[i_per_page]"b"(i_per_page),+"[d_phys_page]"(d_phys_page),+"[i_phys_page]"(i_phys_page)++/* Clobbers */+:"memory","c"+);+}+#endif /* CONFIG_PPC32 && !CONFIG_BOOKE */++#ifdef CONFIG_PPC64+/*+*Datacacheflushthatworksonnon-mappedphysicaladdresses.+*Useonlyfornon-LPARsetups!Italsoassumesrealmode+*iscacheable.UsedforflushingouttheDARTbeforeusing+*itasuncacheablememory+*/+voidflush_dcache_phys_range(unsignedlongstart,unsignedlongstop)+{+/* System data cache block size */+unsignedlongbytes=powerpc_caches.dcache_block_bytes;+unsignedlongshift=powerpc_caches.dcache_block_shift;++/* Temporary registers for the ASM to use */+unsignedlongold_msr,tmp_msr;++/* Compute a start address and number of cachelines */+unsignedlongphys_addr=start&~(bytes-1);+unsignedlongnr_lines=((stop-phys_addr)+(bytes-1))>>shift;++/*+*Thispartneedstobe100%ASMbecausewedisabletheMMU,andwe+*can'taccidentallyletsomeCcodegopokingatmemorywhilethe+*MMUisn'tenabled.+*+*NOTE:Thislooksblatantlyunsafewithrespecttointerrupts.+*Hopefullyallthecallersprovidesufficientprotection?+*/+asmvolatile(+/* First disable the MMU */+"mfmsr %[old_msr]\n\t"+"rlwinm %[tmp_msr], %[old_msr], 0, 28, 26\n\t"+"mtmsr %[tmp_msr]\n\t"+"isync\n\t"++/* Clean the data cache */+"mtctr %[nr_lines]\n"+"0: dcbst 0, %[phys_addr]\n\t"+"add %[phys_addr], %[phys_addr], %[bytes]\n\t"+"bdnz 0b\n\t"+"sync\n\t"+"isync\n\t"++/* Finally, re-enable the MMU */+"mtmsr %[old_msr]\n\t"+"sync\n\t"+"isync\n\t"++/* Temporary variables and inputs */+:[old_msr]"=&r"(old_msr),+[tmp_msr]"=&r"(tmp_msr),+[phys_addr]"=b"(phys_addr)++/* Inputs */+:[bytes]"b"(bytes),+[nr_lines]"b"(nr_lines),+"[phys_addr]"(phys_addr)++/* Clobbers */+:"memory","c"+);+}+#endif /* CONFIG_PPC64 */
@@ -1524,23 +1514,30 @@ static void cacheflush(void)termch=0;nflush=1;scanhex(&nflush);-nflush=(nflush+L1_CACHE_BYTES-1)/L1_CACHE_BYTES;-if(setjmp(bus_error_jmp)==0){-catch_memory_errors=1;-sync();-if(cmd!='i'){-for(;nflush>0;--nflush,adrs+=L1_CACHE_BYTES)-cflush((void*)adrs);-}else{-for(;nflush>0;--nflush,adrs+=L1_CACHE_BYTES)-cinval((void*)adrs);-}-sync();-/* wait a little while to see if we get a machine check */-__delay(200);+if(setjmp(bus_error_jmp)!=0){+catch_memory_errors=0;+return;}-catch_memory_errors=0;+catch_memory_errors=1;+sync();++/* First flush/invalidate data caches */+if(cmd!='i'){+FOR_EACH_CACHELINE(i,adrs,adrs+nflush,dcache)+dcbf(i);+}else{+FOR_EACH_CACHELINE(i,adrs,adrs+nflush,dcache)+dcbi(i);+}++/* Now invalidate instruction caches */+FOR_EACH_CACHELINE(i,adrs,adrs+nflush,icache)+icbi(i);++sync();+/* wait a little while to see if we get a machine check */+__delay(200);}staticunsignedlong
@@ -136,7 +136,9 @@ static void smu_start_cmd(void)/* Flush command and data to RAM */faddr=(unsignedlong)smu->cmd_buf;fend=faddr+smu->cmd_buf->length+2;-flush_inval_dcache_range(faddr,fend);+flush_dcache_range(faddr,fend);+mb();+isync();/* We also disable NAP mode for the duration of the command
Ok, so I have a work-in-progress patch for cleaning up the CPU cache
handling, and I'd like some comments on the approach.
It's not really split up, and it's kind of a huge patch because it
tries to tackle a lot of things at once. Unfortunately, I'm having a
hard time finding good clean places to break things apart.
Furthermore, I know 100% that it is not complete on PPC32 yet, and it
almost certainly does not build on PPC64 yet either.
These are the only files in arch/powerpc/ which have known-incorrect
references to L1_CACHE_* variables:
arch/powerpc/lib/copy_32.S
arch/powerpc/kernel/misc_32.S
Unfortunately, I've been staring at PPC asm for long enough that I
have a migraine headache and I'm going to have to stop here for now.
If somebody else wants to tackle fixing up the 32-bit copy_page() and
__copy_tofrom_user() routines it would be highly appreciated.
Cheers,
Kyle Moffett
--
Curious about my work on the Debian powerpcspe port?
I'm keeping a blog here: http://pureperl.blogspot.com/
These functions are only used from one place each. If the cacheable_*
versions really are more efficient, then those changes should be
migrated into the common code instead.
NOTE: The old routines are just flat buggy on kernels that support
hardware with different cacheline sizes.
Signed-off-by: Kyle Moffett <redacted>
---
arch/powerpc/include/asm/system.h | 2 -
arch/powerpc/kernel/ppc_ksyms.c | 2 -
arch/powerpc/lib/copy_32.S | 127 ----------------------------------
arch/powerpc/mm/ppc_mmu_32.c | 2 +-
drivers/net/ethernet/ibm/emac/core.c | 12 +---
5 files changed, 3 insertions(+), 142 deletions(-)
@@ -77,13 +77,6 @@ MODULE_AUTHOR("Eugene Surovegin <eugene.surovegin@zultys.com> or <ebs@ebshome.net>");MODULE_LICENSE("GPL");-/*-*PPC64doesn't(yet)haveacacheable_memcpy-*/-#ifdef CONFIG_PPC64-#define cacheable_memcpy(d,s,n) memcpy((d),(s),(n))-#endif-/* minimum number of free TX descriptors required to wake up TX process */#define EMAC_TX_WAKEUP_THRESH (NUM_TX_BUFF / 4)
@@ -1637,7 +1630,7 @@ static inline int emac_rx_sg_append(struct emac_instance *dev, int slot)dev_kfree_skb(dev->rx_sg_skb);dev->rx_sg_skb=NULL;}else{-cacheable_memcpy(skb_tail_pointer(dev->rx_sg_skb),+memcpy(skb_tail_pointer(dev->rx_sg_skb),dev->rx_skb[slot]->data,len);skb_put(dev->rx_sg_skb,len);emac_recycle_rx_skb(dev,slot,len);
@@ -1694,8 +1687,7 @@ static int emac_poll_rx(void *param, int budget)gotooom;skb_reserve(copy_skb,EMAC_RX_SKB_HEADROOM+2);-cacheable_memcpy(copy_skb->data-2,skb->data-2,-len+2);+memcpy(copy_skb->data-2,skb->data-2,len+2);emac_recycle_rx_skb(dev,slot,len);skb=copy_skb;}elseif(unlikely(emac_alloc_rx_skb(dev,slot,GFP_ATOMIC)))
From: Moffett, Kyle D <hidden> Date: 2011-11-15 02:53:54
On Nov 10, 2011, at 23:40, Benjamin Herrenschmidt wrote:
On Thu, 2011-11-10 at 18:38 -0600, Moffett, Kyle D wrote:
quoted
(2) Make the ppc64_caches struct apply to ppc32 as well, and
preinitialize it with a minimum value used by any platform being
compiled in (for "dcbXX"/"icbXX" purposes). This is safe because
the pagesize is always a multiple of the cache block size and the
kernel only uses dcbXX/icbXX on whole pages. The only impact is a
temporary small performance hit from flushing or zeroing the same
block 8 times if too small.
=20
Are you sure about dcbz ? Getting that wrong can be deadly ... I'd
rather get rid of some fancy optims and use a soft value in some cases.
That or we can compile multiple variants for the common case of some of
the copy routines and use patching (alternate sections) to branch to the
right one at runtime, at least for the common cases (32 and 128 for
example for 440 and 476).
Well, all of the kernel loops that use dcbz are operating on whole pages,
and the PPC Book-E spec documents that the pagesize is an even multiple
of the cacheline size and the cachelines are always page-aligned.
So when you are clearing a whole page, there are only 2 things you can do
wrong with "dcbz":
(1) Call "dcbz" with an address outside of the page you want to zero.
(2) Omit calls "dcbz" to dcbz for some physical cachelines in the page.
Now, that's a totally different story from the userspace memset() calls
that caused the problem originally, because they were frequently given
memory much smaller than a page to clear, and if you didn't know exactly
how many bytes a "dcbz" was going to clear you couldn't use it at all.
But the kernel doesn't do that anywhere, it just uses it for page clears.
Cheers,
Kyle Moffett
--
Curious about my work on the Debian powerpcspe port?
I'm keeping a blog here: http://pureperl.blogspot.com/
On Mon, Nov 14, 2011 at 21:41, Tabi Timur-B04825 [off-list ref] wrote:
Moffett, Kyle D wrote:
quoted
the PPC Book-E spec documents that the pagesize is an even multiple
of the cacheline size and the cachelines are always page-aligned.
cachelines are page aligned?
Whoops, good catch. That should have been:
"the PPC Book-E spec documents that the pagesize is an even multiple
of the cacheline size and that the pages are always cacheline-aligned."
Thanks!
Cheers,
Kyle Moffett
--
Curious about my work on the Debian powerpcspe port?
I'm keeping a blog here: http://pureperl.blogspot.com/
From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: 2011-11-15 22:29:28
On Mon, 2011-11-14 at 21:32 -0500, Kyle Moffett wrote:
Unfortunately, I've been staring at PPC asm for long enough that I
have a migraine headache and I'm going to have to stop here for now.
If somebody else wants to tackle fixing up the 32-bit copy_page() and
__copy_tofrom_user() routines it would be highly appreciated.
Yeah that's the one everybody's avoiding :-)
What about my idea of instead compiling it multiple times with a
different size and fixing up the branch to call the right one ?
Cheers,
Ben.
From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: 2011-11-15 22:32:12
On Mon, 2011-11-14 at 21:32 -0500, Kyle Moffett wrote:
These functions are only used from one place each. If the cacheable_*
versions really are more efficient, then those changes should be
migrated into the common code instead.
NOTE: The old routines are just flat buggy on kernels that support
hardware with different cacheline sizes.
Signed-off-by: Kyle Moffett <redacted>
---
Right, considering where those are used, I think we can safely remove
them. Thanks.
Ben.
@@ -77,13 +77,6 @@ MODULE_AUTHOR("Eugene Surovegin <eugene.surovegin@zultys.com> or <ebs@ebshome.net>");MODULE_LICENSE("GPL");-/*-*PPC64doesn't(yet)haveacacheable_memcpy-*/-#ifdef CONFIG_PPC64-#define cacheable_memcpy(d,s,n) memcpy((d),(s),(n))-#endif-/* minimum number of free TX descriptors required to wake up TX process */#define EMAC_TX_WAKEUP_THRESH (NUM_TX_BUFF / 4)
@@ -1637,7 +1630,7 @@ static inline int emac_rx_sg_append(struct emac_instance *dev, int slot)dev_kfree_skb(dev->rx_sg_skb);dev->rx_sg_skb=NULL;}else{-cacheable_memcpy(skb_tail_pointer(dev->rx_sg_skb),+memcpy(skb_tail_pointer(dev->rx_sg_skb),dev->rx_skb[slot]->data,len);skb_put(dev->rx_sg_skb,len);emac_recycle_rx_skb(dev,slot,len);
@@ -1694,8 +1687,7 @@ static int emac_poll_rx(void *param, int budget)gotooom;skb_reserve(copy_skb,EMAC_RX_SKB_HEADROOM+2);-cacheable_memcpy(copy_skb->data-2,skb->data-2,-len+2);+memcpy(copy_skb->data-2,skb->data-2,len+2);emac_recycle_rx_skb(dev,slot,len);skb=copy_skb;}elseif(unlikely(emac_alloc_rx_skb(dev,slot,GFP_ATOMIC)))
From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: 2011-11-15 22:41:44
On Mon, 2011-11-14 at 20:36 -0600, Moffett, Kyle D wrote:
So when you are clearing a whole page, there are only 2 things you can do
wrong with "dcbz":
(1) Call "dcbz" with an address outside of the page you want to zero.
(2) Omit calls "dcbz" to dcbz for some physical cachelines in the page.
Now, that's a totally different story from the userspace memset() calls
that caused the problem originally, because they were frequently given
memory much smaller than a page to clear, and if you didn't know exactly
how many bytes a "dcbz" was going to clear you couldn't use it at all.
Right. That's why we pass the cache line sizes to userspace via the elf
AUX table so they don't do stupid things like that :-)
But the kernel doesn't do that anywhere, it just uses it for page clears.
Right, so we could easily precalc the count & increment and use a "soft"
loop.
Cheers,
Ben.
From: Moffett, Kyle D <hidden> Date: 2011-11-15 22:45:45
On Nov 15, 2011, at 17:29, Benjamin Herrenschmidt wrote:
On Mon, 2011-11-14 at 21:32 -0500, Kyle Moffett wrote:
quoted
Unfortunately, I've been staring at PPC asm for long enough that I
have a migraine headache and I'm going to have to stop here for now.
If somebody else wants to tackle fixing up the 32-bit copy_page() and
__copy_tofrom_user() routines it would be highly appreciated.=20
=20
Yeah that's the one everybody's avoiding :-)
=20
What about my idea of instead compiling it multiple times with a
different size and fixing up the branch to call the right one ?
I guess that's doable, although I have to admit that idea almost gives
me more of a headache than trying to fix up the 32-bit ASM.
One thing that bothers me in particular is that both 32/64 versions of
__copy_tofrom_user() are dramatically overcomplicated for what they
ought to be doing.
It would seem that if we get a page fault during an unaligned copy, we
ought to just give up and fall back to a simple byte-by-byte copy loop
from wherever we left off. That would eliminate 90% of the ugly
special cases without actually hurting performance, right?
For a page-fault during a cacheline-aligned copy, we should be able to
handle the exception and retry from the last cacheline without much
logic, again with good performance.
With that said, I'm curious about the origin of the PPC32 ASM. In
particular, it looks like it was generated by GCC at some point in the
distant past, and I'm wondering if there's a good way to rewrite that
file in C and trick GCC into generating the relevant exception tables
for it?
Cheers,
Kyle Moffett
--
Curious about my work on the Debian powerpcspe port?
I'm keeping a blog here: http://pureperl.blogspot.com/
From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: 2011-11-15 23:46:58
On Tue, 2011-11-15 at 16:45 -0600, Moffett, Kyle D wrote:
I guess that's doable, although I have to admit that idea almost gives
me more of a headache than trying to fix up the 32-bit ASM.
One thing that bothers me in particular is that both 32/64 versions of
__copy_tofrom_user() are dramatically overcomplicated for what they
ought to be doing.
It would seem that if we get a page fault during an unaligned copy, we
ought to just give up and fall back to a simple byte-by-byte copy loop
from wherever we left off. That would eliminate 90% of the ugly
special cases without actually hurting performance, right?
For a page-fault during a cacheline-aligned copy, we should be able to
handle the exception and retry from the last cacheline without much
logic, again with good performance.
With that said, I'm curious about the origin of the PPC32 ASM. In
particular, it looks like it was generated by GCC at some point in the
distant past, and I'm wondering if there's a good way to rewrite that
file in C and trick GCC into generating the relevant exception tables
for it?
There is some serious history in there :-)
I would check with Anton, he's been doing some performance work on those
lately (the 64-bit ones).
It's probably worth throwing a proof-of-concept simpler variant for
32-bit at least on the table and have people compare the perfs
(typically network perfs). I can test on a range of ppc32 here (6xx,
7xxx, 4xx).
Cheers,
Ben.