diff mbox series

[v2,2/4] MIPS: Introduce config options for LLSC availability

Message ID 20240612-mips-llsc-v2-2-a42bd5562bdb@flygoat.com (mailing list archive)
State New
Headers show
Series MIPS: Enable ARCH_SUPPORTS_ATOMIC_RMW | expand

Commit Message

Jiaxun Yang June 12, 2024, 9:53 a.m. UTC
Introduce CPU_HAS_LLSC and CPU_MAY_HAVE_LLSC to determine availability
of LLSC and Kconfig level.

They are both true for almost all supported CPUs besides:

R3000: Doesn't have LLSC, so both false.
R5000 series: LLSC is unusable for 64bit kernel, so both false.
R10000: Some platforms decided to opt-out LLSC due to errata, so only
	select CPU_MAY_HAVE_LLSC.
WAR_4KC_LLSC: LLSC is buggy on certain reversion, which can be detected
	at cpu-probe or platform override, so only select CPU_MAY_HAVE_LLSC.

Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
---
v2: Make cpu_has_llsc logic clear
---
 arch/mips/Kconfig                    | 20 ++++++++++++++++++++
 arch/mips/include/asm/cpu-features.h |  9 ++++++++-
 2 files changed, 28 insertions(+), 1 deletion(-)

Comments

Thomas Bogendoerfer June 20, 2024, 4:06 p.m. UTC | #1
On Wed, Jun 12, 2024 at 10:53:30AM +0100, Jiaxun Yang wrote:
> Introduce CPU_HAS_LLSC and CPU_MAY_HAVE_LLSC to determine availability
> of LLSC and Kconfig level.
> 
> They are both true for almost all supported CPUs besides:
> 
> R3000: Doesn't have LLSC, so both false.
> R5000 series: LLSC is unusable for 64bit kernel, so both false.
> R10000: Some platforms decided to opt-out LLSC due to errata, so only
> 	select CPU_MAY_HAVE_LLSC.
> WAR_4KC_LLSC: LLSC is buggy on certain reversion, which can be detected
> 	at cpu-probe or platform override, so only select CPU_MAY_HAVE_LLSC.
> 
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> ---
> v2: Make cpu_has_llsc logic clear
> ---
>  arch/mips/Kconfig                    | 20 ++++++++++++++++++++
>  arch/mips/include/asm/cpu-features.h |  9 ++++++++-
>  2 files changed, 28 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
> index 8ac467c1f9c8..50260a7e9b54 100644
> --- a/arch/mips/Kconfig
> +++ b/arch/mips/Kconfig
> @@ -1548,6 +1548,7 @@ config CPU_R3000
>  config CPU_R4300
>  	bool "R4300"
>  	depends on SYS_HAS_CPU_R4300
> +	select CPU_HAS_LLSC
>  	select CPU_SUPPORTS_32BIT_KERNEL
>  	select CPU_SUPPORTS_64BIT_KERNEL
>  	help
> @@ -1556,6 +1557,7 @@ config CPU_R4300
>  config CPU_R4X00
>  	bool "R4x00"
>  	depends on SYS_HAS_CPU_R4X00
> +	select CPU_HAS_LLSC
>  	select CPU_SUPPORTS_32BIT_KERNEL
>  	select CPU_SUPPORTS_64BIT_KERNEL
>  	select CPU_SUPPORTS_HUGEPAGES
> @@ -1566,6 +1568,7 @@ config CPU_R4X00
>  config CPU_TX49XX
>  	bool "R49XX"
>  	depends on SYS_HAS_CPU_TX49XX
> +	select CPU_HAS_LLSC
>  	select CPU_HAS_PREFETCH
>  	select CPU_SUPPORTS_32BIT_KERNEL
>  	select CPU_SUPPORTS_64BIT_KERNEL
> @@ -1574,6 +1577,7 @@ config CPU_TX49XX
>  config CPU_R5000
>  	bool "R5000"
>  	depends on SYS_HAS_CPU_R5000
> +	select CPU_HAS_LLSC if !64BIT
>  	select CPU_SUPPORTS_32BIT_KERNEL
>  	select CPU_SUPPORTS_64BIT_KERNEL
>  	select CPU_SUPPORTS_HUGEPAGES
> @@ -1583,6 +1587,7 @@ config CPU_R5000
>  config CPU_R5500
>  	bool "R5500"
>  	depends on SYS_HAS_CPU_R5500
> +	select CPU_HAS_LLSC

do you have an user manual stating that the R5k bug is fixed on this CPUs ?

Thomas.
Jiaxun Yang June 20, 2024, 4:30 p.m. UTC | #2
在2024年6月20日六月 下午5:06,Thomas Bogendoerfer写道:
[...]
> do you have an user manual stating that the R5k bug is fixed on this CPUs ?

I actually investigated R5500 a little bit.

There is no explicit document mentioned this bug is fixed on R5500, but it's not
mentioned on "VR Series 64-/32-Bit Microprocessor Programming Guide" [1] either,
while some other hardware limitations are mentioned in that guide.

Plus EMMA platform, which was removed form the kernel, doesn't have
cpu-feature-overrides.h. This means that the platform was running with LLSC
enabled.

So I think this CPU is not affected.

[1]: https://repo.oss.cipunited.com/archives/docs/NEC/U10710EJ5V0AN00.pdf
>
> Thomas.
>
> -- 
> Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
> good idea.                                                [ RFC1925, 2.3 ]
Thomas Bogendoerfer June 20, 2024, 5:41 p.m. UTC | #3
On Thu, Jun 20, 2024 at 05:30:48PM +0100, Jiaxun Yang wrote:
> 
> 
> 在2024年6月20日六月 下午5:06,Thomas Bogendoerfer写道:
> [...]
> > do you have an user manual stating that the R5k bug is fixed on this CPUs ?
> 
> I actually investigated R5500 a little bit.
> 
> There is no explicit document mentioned this bug is fixed on R5500, but it's not
> mentioned on "VR Series 64-/32-Bit Microprocessor Programming Guide" [1] either,
> while some other hardware limitations are mentioned in that guide.
> 
> Plus EMMA platform, which was removed form the kernel, doesn't have
> cpu-feature-overrides.h. This means that the platform was running with LLSC
> enabled.
> 
> So I think this CPU is not affected.

found a preliminary datasheet and it supports your thinking;-)

Thomas.
Maciej W. Rozycki June 21, 2024, midnight UTC | #4
On Wed, 12 Jun 2024, Jiaxun Yang wrote:

> Introduce CPU_HAS_LLSC and CPU_MAY_HAVE_LLSC to determine availability
> of LLSC and Kconfig level.

 Taking the subsequent patches in this series into account this seems to 
create a parallel universe in which the availability of LL/SC for certain 
features is handled at the Kconfig level while in the other universe it's 
handled via <asm/mach-*/cpu-feature-overrides.h>.

 I think this ought not to be done in two places independently and the 
pieces in <asm/mach-*/cpu-feature-overrides.h> need to be removed, likely 
in the same change even, *however* not without double-checking whether 
there is not a case among them where a platform actually has LL/SC support 
disabled despite the CPU used there having architectural support for the 
feature.  Otherwise we may end up with a case where a platform has LL/SC 
support disabled via its <asm/mach-*/cpu-feature-overrides.h> setting and 
yet we enable ARCH_SUPPORTS_ATOMIC_RMW or ARCH_HAVE_NMI_SAFE_CMPXCHG for 
it via Kconfig.

 The note from <asm/mach-ip32/cpu-feature-overrides.h> seems a candidate 
to move to arch/mips/Kconfig at the relevant place on this occasion too.
There may be more such notes and they ought not to be lost.

  Maciej
Jiaxun Yang June 21, 2024, 10:45 a.m. UTC | #5
在2024年6月21日六月 上午1:00,Maciej W. Rozycki写道:
> On Wed, 12 Jun 2024, Jiaxun Yang wrote:
>
>> Introduce CPU_HAS_LLSC and CPU_MAY_HAVE_LLSC to determine availability
>> of LLSC and Kconfig level.
>
>  Taking the subsequent patches in this series into account this seems to 
> create a parallel universe in which the availability of LL/SC for certain 
> features is handled at the Kconfig level while in the other universe it's 
> handled via <asm/mach-*/cpu-feature-overrides.h>.
>
>  I think this ought not to be done in two places independently and the 
> pieces in <asm/mach-*/cpu-feature-overrides.h> need to be removed, likely 
> in the same change even, *however* not without double-checking whether 
> there is not a case among them where a platform actually has LL/SC support 
> disabled despite the CPU used there having architectural support for the 
> feature.  Otherwise we may end up with a case where a platform has LL/SC 
> support disabled via its <asm/mach-*/cpu-feature-overrides.h> setting and 
> yet we enable ARCH_SUPPORTS_ATOMIC_RMW or ARCH_HAVE_NMI_SAFE_CMPXCHG for 
> it via Kconfig.

IMO it's necessary for platforms who know what are they doing such as ATH25,
which we took care in this series.

I'll add a build time assertion to ensure when CONFIG_CPU_HAS_LLSC is selected
kernel_uses_llsc is statically 1, so any incorrect overrides can be spotted
at build time.

It's better to clean up platform's overrides at some point, I'll leave
it to a future patch.

>
>  The note from <asm/mach-ip32/cpu-feature-overrides.h> seems a candidate 
> to move to arch/mips/Kconfig at the relevant place on this occasion too.
> There may be more such notes and they ought not to be lost.

Noted, I'll add a Kconfig symbol WAR_R5000_LLSC and rescue this comment.

Thanks
>
>   Maciej
Maciej W. Rozycki June 21, 2024, 1:57 p.m. UTC | #6
On Fri, 21 Jun 2024, Jiaxun Yang wrote:

> >  I think this ought not to be done in two places independently and the 
> > pieces in <asm/mach-*/cpu-feature-overrides.h> need to be removed, likely 
> > in the same change even, *however* not without double-checking whether 
> > there is not a case among them where a platform actually has LL/SC support 
> > disabled despite the CPU used there having architectural support for the 
> > feature.  Otherwise we may end up with a case where a platform has LL/SC 
> > support disabled via its <asm/mach-*/cpu-feature-overrides.h> setting and 
> > yet we enable ARCH_SUPPORTS_ATOMIC_RMW or ARCH_HAVE_NMI_SAFE_CMPXCHG for 
> > it via Kconfig.
> 
> IMO it's necessary for platforms who know what are they doing such as ATH25,
> which we took care in this series.
> 
> I'll add a build time assertion to ensure when CONFIG_CPU_HAS_LLSC is selected
> kernel_uses_llsc is statically 1, so any incorrect overrides can be spotted
> at build time.

 That might do in the interim as a sanity check, however ultimately the 
sole reason these <asm/mach-*/cpu-feature-overrides.h> exist (and the 
`cpu_has_llsc' setting there) is so that a dynamic check at run time is 
avoided where the result is known from elsewhere beforehand anyway, and 
your change effectively supersedes the overrides, and therefore they need 
to be removed.

> It's better to clean up platform's overrides at some point, I'll leave
> it to a future patch.

 I think it will best be done now while we are at it and the change is in 
scope.

 It should be possible to automatically run over the available defconfigs 
and see if there is any change to `vmlinux' produced between the current 
state upstream and the state where this patch has been applied and the 
`cpu_has_llsc' setting removed from <asm/mach-*/cpu-feature-overrides.h> 
both at a time for each defconfig.  There should be none.

 It'll be tougher once your changes to add ARCH_SUPPORTS_ATOMIC_RMW or 
ARCH_HAVE_NMI_SAFE_CMPXCHG have been applied.

  Maciej
Jiaxun Yang June 21, 2024, 3:21 p.m. UTC | #7
在2024年6月21日六月 下午2:57,Maciej W. Rozycki写道:
> On Fri, 21 Jun 2024, Jiaxun Yang wrote:
>
>> >  I think this ought not to be done in two places independently and the 
>> > pieces in <asm/mach-*/cpu-feature-overrides.h> need to be removed, likely 
>> > in the same change even, *however* not without double-checking whether 
>> > there is not a case among them where a platform actually has LL/SC support 
>> > disabled despite the CPU used there having architectural support for the 
>> > feature.  Otherwise we may end up with a case where a platform has LL/SC 
>> > support disabled via its <asm/mach-*/cpu-feature-overrides.h> setting and 
>> > yet we enable ARCH_SUPPORTS_ATOMIC_RMW or ARCH_HAVE_NMI_SAFE_CMPXCHG for 
>> > it via Kconfig.
>> 
>> IMO it's necessary for platforms who know what are they doing such as ATH25,
>> which we took care in this series.
>> 
>> I'll add a build time assertion to ensure when CONFIG_CPU_HAS_LLSC is selected
>> kernel_uses_llsc is statically 1, so any incorrect overrides can be spotted
>> at build time.
>
>  That might do in the interim as a sanity check, however ultimately the 
> sole reason these <asm/mach-*/cpu-feature-overrides.h> exist (and the 
> `cpu_has_llsc' setting there) is so that a dynamic check at run time is 
> avoided where the result is known from elsewhere beforehand anyway, and 
> your change effectively supersedes the overrides, and therefore they need 
> to be removed.
>
No, overrides are still valid if platform did CPU_MAY_HAVE_LLSC, this is at
least valid for R10000 systems (IP28 decided to opt-out from llsc somehow),
ATH25 (platform made assumption on IP version shipped with CPU), cavium
octeon (platform decided to opt-out llsc for non-SMP build). I'm not confident
with handling them all in Kconfig so I think the best approach so far is to do
build time assertion.

Does anyone reckon the reason behind opt-out LLSC for IP28? As far as I understand
there is no restriction on using LLSC after workaround being applied. If it's purely
performance reason, I think I'll need to move kernel_uses_llsc logic to Kconfig as well.

Thanks
>
>   Maciej
Maciej W. Rozycki June 21, 2024, 5:40 p.m. UTC | #8
On Fri, 21 Jun 2024, Jiaxun Yang wrote:

> >  That might do in the interim as a sanity check, however ultimately the 
> > sole reason these <asm/mach-*/cpu-feature-overrides.h> exist (and the 
> > `cpu_has_llsc' setting there) is so that a dynamic check at run time is 
> > avoided where the result is known from elsewhere beforehand anyway, and 
> > your change effectively supersedes the overrides, and therefore they need 
> > to be removed.
> >
> No, overrides are still valid if platform did CPU_MAY_HAVE_LLSC, this is at
> least valid for R10000 systems (IP28 decided to opt-out from llsc somehow),
> ATH25 (platform made assumption on IP version shipped with CPU), cavium
> octeon (platform decided to opt-out llsc for non-SMP build). I'm not confident
> with handling them all in Kconfig so I think the best approach so far is to do
> build time assertion.

 No, CPU_MAY_HAVE_LLSC is the dynamic case, in which case you need to run 
verification at run time and access the result via the CPU feature vector.  

 If you insist that you need a static override in this case, then you've 
got your CPU_MAY_HAVE_LLSC setting wrong, it should be either CPU_HAS_LLSC 
or nil, according to what <asm/mach-*/cpu-feature-overrides.h> currently 
sets for the platform in question.  Whatever is set statically there at 
build time can be reproduced in Kconfig.

 Going through platforms should be easy if not a bit tedious, but that's 
the cost you sometimes need to pay for progress.

  Maciej
Thomas Bogendoerfer June 21, 2024, 8:31 p.m. UTC | #9
On Fri, Jun 21, 2024 at 04:21:49PM +0100, Jiaxun Yang wrote:
> Does anyone reckon the reason behind opt-out LLSC for IP28? As far as I understand
> there is no restriction on using LLSC after workaround being applied. If it's purely
> performance reason, I think I'll need to move kernel_uses_llsc logic to Kconfig as well.

commit 46dd40aa376c8158b6aa17510079caf5c3af6237
Author: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Date:   Wed Oct 7 12:17:04 2020 +0200

    MIPS: SGI-IP28: disable use of ll/sc in kernel
    
    SGI-IP28 systems only use broken R10k rev 2.5 CPUs, which could lock
    up, if ll/sc sequences are issued in certain order. Since those systems
    are all non-SMP, we can disable ll/sc usage in kernel.
    
    Signed-off-by: Thomas Bogendoerfer <tsbogend@alpha.franken.de>


Thomas.
diff mbox series

Patch

diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index 8ac467c1f9c8..50260a7e9b54 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -1548,6 +1548,7 @@  config CPU_R3000
 config CPU_R4300
 	bool "R4300"
 	depends on SYS_HAS_CPU_R4300
+	select CPU_HAS_LLSC
 	select CPU_SUPPORTS_32BIT_KERNEL
 	select CPU_SUPPORTS_64BIT_KERNEL
 	help
@@ -1556,6 +1557,7 @@  config CPU_R4300
 config CPU_R4X00
 	bool "R4x00"
 	depends on SYS_HAS_CPU_R4X00
+	select CPU_HAS_LLSC
 	select CPU_SUPPORTS_32BIT_KERNEL
 	select CPU_SUPPORTS_64BIT_KERNEL
 	select CPU_SUPPORTS_HUGEPAGES
@@ -1566,6 +1568,7 @@  config CPU_R4X00
 config CPU_TX49XX
 	bool "R49XX"
 	depends on SYS_HAS_CPU_TX49XX
+	select CPU_HAS_LLSC
 	select CPU_HAS_PREFETCH
 	select CPU_SUPPORTS_32BIT_KERNEL
 	select CPU_SUPPORTS_64BIT_KERNEL
@@ -1574,6 +1577,7 @@  config CPU_TX49XX
 config CPU_R5000
 	bool "R5000"
 	depends on SYS_HAS_CPU_R5000
+	select CPU_HAS_LLSC if !64BIT
 	select CPU_SUPPORTS_32BIT_KERNEL
 	select CPU_SUPPORTS_64BIT_KERNEL
 	select CPU_SUPPORTS_HUGEPAGES
@@ -1583,6 +1587,7 @@  config CPU_R5000
 config CPU_R5500
 	bool "R5500"
 	depends on SYS_HAS_CPU_R5500
+	select CPU_HAS_LLSC
 	select CPU_SUPPORTS_32BIT_KERNEL
 	select CPU_SUPPORTS_64BIT_KERNEL
 	select CPU_SUPPORTS_HUGEPAGES
@@ -1593,6 +1598,7 @@  config CPU_R5500
 config CPU_NEVADA
 	bool "RM52xx"
 	depends on SYS_HAS_CPU_NEVADA
+	select CPU_HAS_LLSC if !64BIT
 	select CPU_SUPPORTS_32BIT_KERNEL
 	select CPU_SUPPORTS_64BIT_KERNEL
 	select CPU_SUPPORTS_HUGEPAGES
@@ -1602,6 +1608,8 @@  config CPU_NEVADA
 config CPU_R10000
 	bool "R10000"
 	depends on SYS_HAS_CPU_R10000
+	select CPU_HAS_LLSC if !WAR_R10000_LLSC
+	select CPU_MAY_HAVE_LLSC
 	select CPU_HAS_PREFETCH
 	select CPU_SUPPORTS_32BIT_KERNEL
 	select CPU_SUPPORTS_64BIT_KERNEL
@@ -1613,6 +1621,7 @@  config CPU_R10000
 config CPU_RM7000
 	bool "RM7000"
 	depends on SYS_HAS_CPU_RM7000
+	select CPU_HAS_LLSC
 	select CPU_HAS_PREFETCH
 	select CPU_SUPPORTS_32BIT_KERNEL
 	select CPU_SUPPORTS_64BIT_KERNEL
@@ -1622,6 +1631,7 @@  config CPU_RM7000
 config CPU_SB1
 	bool "SB1"
 	depends on SYS_HAS_CPU_SB1
+	select CPU_HAS_LLSC
 	select CPU_SUPPORTS_32BIT_KERNEL
 	select CPU_SUPPORTS_64BIT_KERNEL
 	select CPU_SUPPORTS_HIGHMEM
@@ -1656,6 +1666,7 @@  config CPU_BMIPS
 	select CPU_BMIPS4350 if SYS_HAS_CPU_BMIPS4350
 	select CPU_BMIPS4380 if SYS_HAS_CPU_BMIPS4380
 	select CPU_BMIPS5000 if SYS_HAS_CPU_BMIPS5000
+	select CPU_HAS_LLSC
 	select CPU_SUPPORTS_32BIT_KERNEL
 	select DMA_NONCOHERENT
 	select IRQ_MIPS_CPU
@@ -2381,6 +2392,15 @@  config CPU_DIEI_BROKEN
 config CPU_HAS_RIXI
 	bool
 
+# For CPU that must have LLSC
+config CPU_HAS_LLSC
+	def_bool TARGET_ISA_REV > 0 && !WAR_4KC_LLSC
+	select CPU_MAY_HAVE_LLSC
+
+# For CPU that LLSC support is optional
+config CPU_MAY_HAVE_LLSC
+	def_bool TARGET_ISA_REV > 0
+
 config CPU_NO_LOAD_STORE_LR
 	bool
 	help
diff --git a/arch/mips/include/asm/cpu-features.h b/arch/mips/include/asm/cpu-features.h
index 404390bb87ea..40f5570de563 100644
--- a/arch/mips/include/asm/cpu-features.h
+++ b/arch/mips/include/asm/cpu-features.h
@@ -185,8 +185,15 @@ 
 #ifndef cpu_has_ejtag
 #define cpu_has_ejtag		__opt(MIPS_CPU_EJTAG)
 #endif
+
 #ifndef cpu_has_llsc
-#define cpu_has_llsc		__isa_ge_or_opt(1, MIPS_CPU_LLSC)
+# if defined(CONFIG_CPU_HAS_LLSC)
+#  define cpu_has_llsc		1
+# elif defined(CONFIG_CPU_MAY_HAVE_LLSC)
+#  define cpu_has_llsc		__opt(MIPS_CPU_LLSC)
+# else
+#  define cpu_has_llsc		0
+# endif
 #endif
 #ifndef kernel_uses_llsc
 #define kernel_uses_llsc	cpu_has_llsc