* [PATCH 0/4] arm64: Rework handling of erratum 1188873
@ 2019-04-08 16:02 Marc Zyngier
2019-04-08 16:02 ` [PATCH 1/4] arm64: Restrict ARM64_ERRATUM_1188873 mitigation to AArch32 Marc Zyngier
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: Marc Zyngier @ 2019-04-08 16:02 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Mark Rutland, Catalin Marinas, Daniel Lezcano, Will Deacon
When the handling of erratum 1188873 was merged, it was decided that
we'd treat both AArch32 and AArch64 equally by trapping CNTVCT. It
turns out that users of such HW feel that the overhead placed on
AArch64 userspace is unacceptable, so a less invasive solution had to
be investigated.
Instead of configuring the trapping once and for all, we switch it on
and off on exiting to EL0, depending on whether we have a 32bit task
or not. Hopefully the overhead is minimal for non-affected cores.
We also take this opportunity to apply the same workaround to
Neoverse-N1, which suffers from the same erratum in its early
revisions.
Marc Zyngier (4):
arm64: Restrict ARM64_ERRATUM_1188873 mitigation to AArch32
arm64: Make ARM64_ERRATUM_1188873 depend on COMPAT
arm64: Add part number for Neoverse N1
arm64: Apply ARM64_ERRATUM_1188873 to Neoverse-N1
arch/arm64/Kconfig | 12 +++++++-----
arch/arm64/include/asm/cputype.h | 2 ++
arch/arm64/kernel/cpu_errata.c | 13 +++++++++++--
arch/arm64/kernel/entry.S | 23 +++++++++++++++++++++++
drivers/clocksource/arm_arch_timer.c | 15 ---------------
5 files changed, 43 insertions(+), 22 deletions(-)
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/4] arm64: Restrict ARM64_ERRATUM_1188873 mitigation to AArch32
2019-04-08 16:02 [PATCH 0/4] arm64: Rework handling of erratum 1188873 Marc Zyngier
@ 2019-04-08 16:02 ` Marc Zyngier
2019-04-08 18:34 ` Robin Murphy
2019-04-12 13:17 ` Will Deacon
2019-04-08 16:02 ` [PATCH 2/4] arm64: Make ARM64_ERRATUM_1188873 depend on COMPAT Marc Zyngier
` (2 subsequent siblings)
3 siblings, 2 replies; 11+ messages in thread
From: Marc Zyngier @ 2019-04-08 16:02 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Mark Rutland, Catalin Marinas, Daniel Lezcano, Will Deacon
We currently deal with ARM64_ERRATUM_1188873 by always trapping EL0
accesses for both instruction sets. Although nothing wrong comes out
of that, people trying to squeeze the last drop of performance from
buggy HW find this over the top. Oh well.
Let's change the mitigation by flipping the counter enable bit
on return to userspace. Non-broken HW gets an extra branch on
the fast path, which is hopefully not the end of the world.
The arch timer workaround it also removed.
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
arch/arm64/kernel/entry.S | 23 +++++++++++++++++++++++
drivers/clocksource/arm_arch_timer.c | 15 ---------------
2 files changed, 23 insertions(+), 15 deletions(-)
diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index c50a7a75f2e0..e0aed21ab402 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -336,6 +336,29 @@ alternative_if ARM64_WORKAROUND_845719
alternative_else_nop_endif
#endif
3:
+#ifdef CONFIG_ARM64_ERRATUM_1188873
+alternative_if_not ARM64_WORKAROUND_1188873
+ b 1f
+alternative_else_nop_endif
+
+ ubfx x0, x22, #4, #1 // Extract PSR_MODE32
+ eor x0, x0, #1 // Negate it
+alternative_if_not ARM64_HAS_VIRT_HOST_EXTN
+ mrs x1, cntkctl_el1
+alternative_else
+ mrs x1, cnthctl_el2
+alternative_endif
+ ubfx x2, x1, #1, #1 // Extract EL0VCTEN
+ cmp x2, x0
+ b.eq 1f // Matches, nothing to do
+ bfi x1, x0, #1, #1 // Move EL0VCTEN in place
+alternative_if_not ARM64_HAS_VIRT_HOST_EXTN
+ msr cntkctl_el1, x1
+alternative_else
+ msr cnthctl_el2, x1
+alternative_endif
+1:
+#endif
apply_ssbd 0, x0, x1
.endif
diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 5fcccc467868..27acc9eb0f7c 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -339,13 +339,6 @@ static u64 notrace arm64_858921_read_cntvct_el0(void)
}
#endif
-#ifdef CONFIG_ARM64_ERRATUM_1188873
-static u64 notrace arm64_1188873_read_cntvct_el0(void)
-{
- return read_sysreg(cntvct_el0);
-}
-#endif
-
#ifdef CONFIG_SUN50I_ERRATUM_UNKNOWN1
/*
* The low bits of the counter registers are indeterminate while bit 10 or
@@ -476,14 +469,6 @@ static const struct arch_timer_erratum_workaround ool_workarounds[] = {
.read_cntvct_el0 = arm64_858921_read_cntvct_el0,
},
#endif
-#ifdef CONFIG_ARM64_ERRATUM_1188873
- {
- .match_type = ate_match_local_cap_id,
- .id = (void *)ARM64_WORKAROUND_1188873,
- .desc = "ARM erratum 1188873",
- .read_cntvct_el0 = arm64_1188873_read_cntvct_el0,
- },
-#endif
#ifdef CONFIG_SUN50I_ERRATUM_UNKNOWN1
{
.match_type = ate_match_dt,
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/4] arm64: Make ARM64_ERRATUM_1188873 depend on COMPAT
2019-04-08 16:02 [PATCH 0/4] arm64: Rework handling of erratum 1188873 Marc Zyngier
2019-04-08 16:02 ` [PATCH 1/4] arm64: Restrict ARM64_ERRATUM_1188873 mitigation to AArch32 Marc Zyngier
@ 2019-04-08 16:02 ` Marc Zyngier
2019-04-08 16:02 ` [PATCH 3/4] arm64: Add part number for Neoverse N1 Marc Zyngier
2019-04-08 16:02 ` [PATCH 4/4] arm64: Apply ARM64_ERRATUM_1188873 to Neoverse-N1 Marc Zyngier
3 siblings, 0 replies; 11+ messages in thread
From: Marc Zyngier @ 2019-04-08 16:02 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Mark Rutland, Catalin Marinas, Daniel Lezcano, Will Deacon
Since ARM64_ERRATUM_1188873 only affects AArch32 EL0, it makes some
sense that it should depend on COMPAT.
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
arch/arm64/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 7e34b9eba5de..560f2a860637 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -477,6 +477,7 @@ config ARM64_ERRATUM_1024718
config ARM64_ERRATUM_1188873
bool "Cortex-A76: MRC read following MRRC read of specific Generic Timer in AArch32 might give incorrect result"
default y
+ depends on COMPAT
select ARM_ARCH_TIMER_OOL_WORKAROUND
help
This option adds work arounds for ARM Cortex-A76 erratum 1188873
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 3/4] arm64: Add part number for Neoverse N1
2019-04-08 16:02 [PATCH 0/4] arm64: Rework handling of erratum 1188873 Marc Zyngier
2019-04-08 16:02 ` [PATCH 1/4] arm64: Restrict ARM64_ERRATUM_1188873 mitigation to AArch32 Marc Zyngier
2019-04-08 16:02 ` [PATCH 2/4] arm64: Make ARM64_ERRATUM_1188873 depend on COMPAT Marc Zyngier
@ 2019-04-08 16:02 ` Marc Zyngier
2019-04-08 16:02 ` [PATCH 4/4] arm64: Apply ARM64_ERRATUM_1188873 to Neoverse-N1 Marc Zyngier
3 siblings, 0 replies; 11+ messages in thread
From: Marc Zyngier @ 2019-04-08 16:02 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Mark Rutland, Catalin Marinas, Daniel Lezcano, Will Deacon
New CPU, new part number. You know the drill.
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
arch/arm64/include/asm/cputype.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm64/include/asm/cputype.h b/arch/arm64/include/asm/cputype.h
index 5f1437099b99..2602bae334fb 100644
--- a/arch/arm64/include/asm/cputype.h
+++ b/arch/arm64/include/asm/cputype.h
@@ -89,6 +89,7 @@
#define ARM_CPU_PART_CORTEX_A35 0xD04
#define ARM_CPU_PART_CORTEX_A55 0xD05
#define ARM_CPU_PART_CORTEX_A76 0xD0B
+#define ARM_CPU_PART_NEOVERSE_N1 0xD0C
#define APM_CPU_PART_POTENZA 0x000
@@ -118,6 +119,7 @@
#define MIDR_CORTEX_A35 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A35)
#define MIDR_CORTEX_A55 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A55)
#define MIDR_CORTEX_A76 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A76)
+#define MIDR_NEOVERSE_N1 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_NEOVERSE_N1)
#define MIDR_THUNDERX MIDR_CPU_MODEL(ARM_CPU_IMP_CAVIUM, CAVIUM_CPU_PART_THUNDERX)
#define MIDR_THUNDERX_81XX MIDR_CPU_MODEL(ARM_CPU_IMP_CAVIUM, CAVIUM_CPU_PART_THUNDERX_81XX)
#define MIDR_THUNDERX_83XX MIDR_CPU_MODEL(ARM_CPU_IMP_CAVIUM, CAVIUM_CPU_PART_THUNDERX_83XX)
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 4/4] arm64: Apply ARM64_ERRATUM_1188873 to Neoverse-N1
2019-04-08 16:02 [PATCH 0/4] arm64: Rework handling of erratum 1188873 Marc Zyngier
` (2 preceding siblings ...)
2019-04-08 16:02 ` [PATCH 3/4] arm64: Add part number for Neoverse N1 Marc Zyngier
@ 2019-04-08 16:02 ` Marc Zyngier
3 siblings, 0 replies; 11+ messages in thread
From: Marc Zyngier @ 2019-04-08 16:02 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Mark Rutland, Catalin Marinas, Daniel Lezcano, Will Deacon
Neoverse-N1 is also affected by ARM64_ERRATUM_1188873, so let's
add it to the list of affected CPUs.
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
arch/arm64/Kconfig | 11 ++++++-----
arch/arm64/kernel/cpu_errata.c | 13 +++++++++++--
2 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 560f2a860637..fcda4e21fa8f 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -475,16 +475,17 @@ config ARM64_ERRATUM_1024718
If unsure, say Y.
config ARM64_ERRATUM_1188873
- bool "Cortex-A76: MRC read following MRRC read of specific Generic Timer in AArch32 might give incorrect result"
+ bool "Cortex-A76/Neoverse-N1: MRC read following MRRC read of specific Generic Timer in AArch32 might give incorrect result"
default y
depends on COMPAT
select ARM_ARCH_TIMER_OOL_WORKAROUND
help
- This option adds work arounds for ARM Cortex-A76 erratum 1188873
+ This option adds work arounds for ARM Cortex-A76/Neoverse-N1
+ erratum 1188873
- Affected Cortex-A76 cores (r0p0, r1p0, r2p0) could cause
- register corruption when accessing the timer registers from
- AArch32 userspace.
+ Affected Cortex-A76/Neoverse-N1 cores (r0p0, r1p0, r2p0) could
+ cause register corruption when accessing the timer registers
+ from AArch32 userspace.
If unsure, say Y.
diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
index 9950bb0cbd52..06f1c8aae1dc 100644
--- a/arch/arm64/kernel/cpu_errata.c
+++ b/arch/arm64/kernel/cpu_errata.c
@@ -603,6 +603,16 @@ static const struct midr_range workaround_clean_cache[] = {
};
#endif
+#ifdef CONFIG_ARM64_ERRATUM_1188873
+static const struct midr_range erratum_1188873_list[] = {
+ /* Cortex-A76 r0p0 to r2p0 */
+ MIDR_RANGE(MIDR_CORTEX_A76, 0, 0, 2, 0),
+ /* Neoverse-N1 r0p0 to r2p0 */
+ MIDR_RANGE(MIDR_NEOVERSE_N1, 0, 0, 2, 0),
+ {},
+};
+#endif
+
const struct arm64_cpu_capabilities arm64_errata[] = {
#ifdef CONFIG_ARM64_WORKAROUND_CLEAN_CACHE
{
@@ -725,10 +735,9 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
#endif
#ifdef CONFIG_ARM64_ERRATUM_1188873
{
- /* Cortex-A76 r0p0 to r2p0 */
.desc = "ARM erratum 1188873",
.capability = ARM64_WORKAROUND_1188873,
- ERRATA_MIDR_RANGE(MIDR_CORTEX_A76, 0, 0, 2, 0),
+ ERRATA_MIDR_RANGE_LIST(erratum_1188873_list),
},
#endif
#ifdef CONFIG_ARM64_ERRATUM_1165522
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/4] arm64: Restrict ARM64_ERRATUM_1188873 mitigation to AArch32
2019-04-08 16:02 ` [PATCH 1/4] arm64: Restrict ARM64_ERRATUM_1188873 mitigation to AArch32 Marc Zyngier
@ 2019-04-08 18:34 ` Robin Murphy
2019-04-09 9:18 ` Marc Zyngier
2019-04-12 13:17 ` Will Deacon
1 sibling, 1 reply; 11+ messages in thread
From: Robin Murphy @ 2019-04-08 18:34 UTC (permalink / raw)
To: Marc Zyngier, linux-arm-kernel
Cc: Mark Rutland, Catalin Marinas, Daniel Lezcano, Will Deacon
Hi Marc,
On 08/04/2019 17:02, Marc Zyngier wrote:
> We currently deal with ARM64_ERRATUM_1188873 by always trapping EL0
> accesses for both instruction sets. Although nothing wrong comes out
> of that, people trying to squeeze the last drop of performance from
> buggy HW find this over the top. Oh well.
>
> Let's change the mitigation by flipping the counter enable bit
> on return to userspace. Non-broken HW gets an extra branch on
> the fast path, which is hopefully not the end of the world.
> The arch timer workaround it also removed.
>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
> arch/arm64/kernel/entry.S | 23 +++++++++++++++++++++++
> drivers/clocksource/arm_arch_timer.c | 15 ---------------
> 2 files changed, 23 insertions(+), 15 deletions(-)
>
> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
> index c50a7a75f2e0..e0aed21ab402 100644
> --- a/arch/arm64/kernel/entry.S
> +++ b/arch/arm64/kernel/entry.S
> @@ -336,6 +336,29 @@ alternative_if ARM64_WORKAROUND_845719
> alternative_else_nop_endif
> #endif
> 3:
> +#ifdef CONFIG_ARM64_ERRATUM_1188873
> +alternative_if_not ARM64_WORKAROUND_1188873
> + b 1f
> +alternative_else_nop_endif
> +
> + ubfx x0, x22, #4, #1 // Extract PSR_MODE32
> + eor x0, x0, #1 // Negate it
> +alternative_if_not ARM64_HAS_VIRT_HOST_EXTN
> + mrs x1, cntkctl_el1
> +alternative_else
> + mrs x1, cnthctl_el2
> +alternative_endif
> + ubfx x2, x1, #1, #1 // Extract EL0VCTEN
> + cmp x2, x0
> + b.eq 1f // Matches, nothing to do
> + bfi x1, x0, #1, #1 // Move EL0VCTEN in place
> +alternative_if_not ARM64_HAS_VIRT_HOST_EXTN
> + msr cntkctl_el1, x1
> +alternative_else
> + msr cnthctl_el2, x1
> +alternative_endif
Unless I've gone horribly wrong in my reasoning somewhere, I reckon this
could be a fair bit shorter...
alternative_if_not ARM64_HAS_VIRT_HOST_EXTN
mrs x1, cntkctl_el1
alternative_else
mrs x1, cnthctl_el2
alternative_endif
eon x0, x1, x22, lsr #3
eor x1, x1, #1
tbz x0, #1, 1f
alternative_if_not ARM64_HAS_VIRT_HOST_EXTN
msr cntkctl_el1, x1
alternative_else
msr cnthctl_el2, x1
alternative_endif
...although whether that's a good idea at all probably depends mostly on
how many comments you think it needs :)
Robin.
> +1:
> +#endif
> apply_ssbd 0, x0, x1
> .endif
>
> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
> index 5fcccc467868..27acc9eb0f7c 100644
> --- a/drivers/clocksource/arm_arch_timer.c
> +++ b/drivers/clocksource/arm_arch_timer.c
> @@ -339,13 +339,6 @@ static u64 notrace arm64_858921_read_cntvct_el0(void)
> }
> #endif
>
> -#ifdef CONFIG_ARM64_ERRATUM_1188873
> -static u64 notrace arm64_1188873_read_cntvct_el0(void)
> -{
> - return read_sysreg(cntvct_el0);
> -}
> -#endif
> -
> #ifdef CONFIG_SUN50I_ERRATUM_UNKNOWN1
> /*
> * The low bits of the counter registers are indeterminate while bit 10 or
> @@ -476,14 +469,6 @@ static const struct arch_timer_erratum_workaround ool_workarounds[] = {
> .read_cntvct_el0 = arm64_858921_read_cntvct_el0,
> },
> #endif
> -#ifdef CONFIG_ARM64_ERRATUM_1188873
> - {
> - .match_type = ate_match_local_cap_id,
> - .id = (void *)ARM64_WORKAROUND_1188873,
> - .desc = "ARM erratum 1188873",
> - .read_cntvct_el0 = arm64_1188873_read_cntvct_el0,
> - },
> -#endif
> #ifdef CONFIG_SUN50I_ERRATUM_UNKNOWN1
> {
> .match_type = ate_match_dt,
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/4] arm64: Restrict ARM64_ERRATUM_1188873 mitigation to AArch32
2019-04-08 18:34 ` Robin Murphy
@ 2019-04-09 9:18 ` Marc Zyngier
2019-04-09 9:49 ` Robin Murphy
0 siblings, 1 reply; 11+ messages in thread
From: Marc Zyngier @ 2019-04-09 9:18 UTC (permalink / raw)
To: Robin Murphy, linux-arm-kernel
Cc: Mark Rutland, Catalin Marinas, Daniel Lezcano, Will Deacon
On 08/04/2019 19:34, Robin Murphy wrote:
> Hi Marc,
>
> On 08/04/2019 17:02, Marc Zyngier wrote:
>> We currently deal with ARM64_ERRATUM_1188873 by always trapping EL0
>> accesses for both instruction sets. Although nothing wrong comes out
>> of that, people trying to squeeze the last drop of performance from
>> buggy HW find this over the top. Oh well.
>>
>> Let's change the mitigation by flipping the counter enable bit
>> on return to userspace. Non-broken HW gets an extra branch on
>> the fast path, which is hopefully not the end of the world.
>> The arch timer workaround it also removed.
>>
>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>> ---
>> arch/arm64/kernel/entry.S | 23 +++++++++++++++++++++++
>> drivers/clocksource/arm_arch_timer.c | 15 ---------------
>> 2 files changed, 23 insertions(+), 15 deletions(-)
>>
>> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
>> index c50a7a75f2e0..e0aed21ab402 100644
>> --- a/arch/arm64/kernel/entry.S
>> +++ b/arch/arm64/kernel/entry.S
>> @@ -336,6 +336,29 @@ alternative_if ARM64_WORKAROUND_845719
>> alternative_else_nop_endif
>> #endif
>> 3:
>> +#ifdef CONFIG_ARM64_ERRATUM_1188873
>> +alternative_if_not ARM64_WORKAROUND_1188873
>> + b 1f
>> +alternative_else_nop_endif
>> +
>> + ubfx x0, x22, #4, #1 // Extract PSR_MODE32
>> + eor x0, x0, #1 // Negate it
>> +alternative_if_not ARM64_HAS_VIRT_HOST_EXTN
>> + mrs x1, cntkctl_el1
>> +alternative_else
>> + mrs x1, cnthctl_el2
>> +alternative_endif
>> + ubfx x2, x1, #1, #1 // Extract EL0VCTEN
>> + cmp x2, x0
>> + b.eq 1f // Matches, nothing to do
>> + bfi x1, x0, #1, #1 // Move EL0VCTEN in place
>> +alternative_if_not ARM64_HAS_VIRT_HOST_EXTN
>> + msr cntkctl_el1, x1
>> +alternative_else
>> + msr cnthctl_el2, x1
>> +alternative_endif
>
> Unless I've gone horribly wrong in my reasoning somewhere, I reckon this
> could be a fair bit shorter...
>
>
> alternative_if_not ARM64_HAS_VIRT_HOST_EXTN
> mrs x1, cntkctl_el1
> alternative_else
> mrs x1, cnthctl_el2
> alternative_endif
> eon x0, x1, x22, lsr #3
> eor x1, x1, #1
> tbz x0, #1, 1f
> alternative_if_not ARM64_HAS_VIRT_HOST_EXTN
> msr cntkctl_el1, x1
> alternative_else
> msr cnthctl_el2, x1
> alternative_endif
>
>
> ...although whether that's a good idea at all probably depends mostly on
> how many comments you think it needs :)
Neat! Totally impossible to decipher, and thus perfect! ;-) The way I
understand it that pstate.m32 and cntkctl_el1.el0vcten must always have
opposite values. If they don't, we flip el0vcten.
We could move the eor after the tbz, I think. And we can also get rid of
the VHE alternative, as cntkctl_el1 is a valid accessor for cnthctl_el2
in that case.
Shal we settle for this?
+#ifdef CONFIG_ARM64_ERRATUM_1188873
+alternative_if_not ARM64_WORKAROUND_1188873
+ b 1f
+alternative_else_nop_endif
+
+ // if (!x22.mode32 != cntkctl_el1.el0vcten)
+ // cntkctl_el1.el0vcten = !cntkctl_el1.el0vcten
+
+ mrs x1, cntkctl_el1
+
+ eon x0, x1, x22, lsr #3
+ tbz x0, #1, 1f
+
+ eor x1, x1, #1
+ msr cntkctl_el1, x1
+1:
+#endif
M.
--
Jazz is not dead. It just smells funny...
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/4] arm64: Restrict ARM64_ERRATUM_1188873 mitigation to AArch32
2019-04-09 9:18 ` Marc Zyngier
@ 2019-04-09 9:49 ` Robin Murphy
0 siblings, 0 replies; 11+ messages in thread
From: Robin Murphy @ 2019-04-09 9:49 UTC (permalink / raw)
To: Marc Zyngier, linux-arm-kernel
Cc: Mark Rutland, Catalin Marinas, Daniel Lezcano, Will Deacon
On 09/04/2019 10:18, Marc Zyngier wrote:
> On 08/04/2019 19:34, Robin Murphy wrote:
>> Hi Marc,
>>
>> On 08/04/2019 17:02, Marc Zyngier wrote:
>>> We currently deal with ARM64_ERRATUM_1188873 by always trapping EL0
>>> accesses for both instruction sets. Although nothing wrong comes out
>>> of that, people trying to squeeze the last drop of performance from
>>> buggy HW find this over the top. Oh well.
>>>
>>> Let's change the mitigation by flipping the counter enable bit
>>> on return to userspace. Non-broken HW gets an extra branch on
>>> the fast path, which is hopefully not the end of the world.
>>> The arch timer workaround it also removed.
>>>
>>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>>> ---
>>> arch/arm64/kernel/entry.S | 23 +++++++++++++++++++++++
>>> drivers/clocksource/arm_arch_timer.c | 15 ---------------
>>> 2 files changed, 23 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
>>> index c50a7a75f2e0..e0aed21ab402 100644
>>> --- a/arch/arm64/kernel/entry.S
>>> +++ b/arch/arm64/kernel/entry.S
>>> @@ -336,6 +336,29 @@ alternative_if ARM64_WORKAROUND_845719
>>> alternative_else_nop_endif
>>> #endif
>>> 3:
>>> +#ifdef CONFIG_ARM64_ERRATUM_1188873
>>> +alternative_if_not ARM64_WORKAROUND_1188873
>>> + b 1f
>>> +alternative_else_nop_endif
>>> +
>>> + ubfx x0, x22, #4, #1 // Extract PSR_MODE32
>>> + eor x0, x0, #1 // Negate it
>>> +alternative_if_not ARM64_HAS_VIRT_HOST_EXTN
>>> + mrs x1, cntkctl_el1
>>> +alternative_else
>>> + mrs x1, cnthctl_el2
>>> +alternative_endif
>>> + ubfx x2, x1, #1, #1 // Extract EL0VCTEN
>>> + cmp x2, x0
>>> + b.eq 1f // Matches, nothing to do
>>> + bfi x1, x0, #1, #1 // Move EL0VCTEN in place
>>> +alternative_if_not ARM64_HAS_VIRT_HOST_EXTN
>>> + msr cntkctl_el1, x1
>>> +alternative_else
>>> + msr cnthctl_el2, x1
>>> +alternative_endif
>>
>> Unless I've gone horribly wrong in my reasoning somewhere, I reckon this
>> could be a fair bit shorter...
>>
>>
>> alternative_if_not ARM64_HAS_VIRT_HOST_EXTN
>> mrs x1, cntkctl_el1
>> alternative_else
>> mrs x1, cnthctl_el2
>> alternative_endif
>> eon x0, x1, x22, lsr #3
>> eor x1, x1, #1
>> tbz x0, #1, 1f
>> alternative_if_not ARM64_HAS_VIRT_HOST_EXTN
>> msr cntkctl_el1, x1
>> alternative_else
>> msr cnthctl_el2, x1
>> alternative_endif
>>
>>
>> ...although whether that's a good idea at all probably depends mostly on
>> how many comments you think it needs :)
>
> Neat! Totally impossible to decipher, and thus perfect! ;-) The way I
> understand it that pstate.m32 and cntkctl_el1.el0vcten must always have
> opposite values. If they don't, we flip el0vcten.
Yup, that's the trick!
> We could move the eor after the tbz, I think. And we can also get rid of
> the VHE alternative, as cntkctl_el1 is a valid accessor for cnthctl_el2
> in that case.
Indeed I initially wrote the logic TBZ-first, but then decided to flip
them in the spirit of "trying to squeeze the last drop of performance"
:) Reason being that on most of our microarchitectures a
shifted-register operand tends to incur +1 cycle of result latency, so
in theory we can slip in the EOR for free while waiting on the x0
dependency. I may have got a bit carried away there, since on reflection
this is of course only going to be run by the affected CPUs which should
be big and OoO enough to make that entirely irrelevant, so dialling it
back a bit for the sake of a little clarity seems a sensible thing to do.
> Shal we settle for this?
>
> +#ifdef CONFIG_ARM64_ERRATUM_1188873
> +alternative_if_not ARM64_WORKAROUND_1188873
> + b 1f
> +alternative_else_nop_endif
> +
> + // if (!x22.mode32 != cntkctl_el1.el0vcten)
> + // cntkctl_el1.el0vcten = !cntkctl_el1.el0vcten
> +
> + mrs x1, cntkctl_el1
> +
> + eon x0, x1, x22, lsr #3
> + tbz x0, #1, 1f
> +
> + eor x1, x1, #1
> + msr cntkctl_el1, x1
> +1:
> +#endif
Nice, that looks about as tidy as it'll ever get!
Cheers,
Robin.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/4] arm64: Restrict ARM64_ERRATUM_1188873 mitigation to AArch32
2019-04-08 16:02 ` [PATCH 1/4] arm64: Restrict ARM64_ERRATUM_1188873 mitigation to AArch32 Marc Zyngier
2019-04-08 18:34 ` Robin Murphy
@ 2019-04-12 13:17 ` Will Deacon
2019-04-15 8:15 ` Marc Zyngier
1 sibling, 1 reply; 11+ messages in thread
From: Will Deacon @ 2019-04-12 13:17 UTC (permalink / raw)
To: Marc Zyngier
Cc: Mark Rutland, Catalin Marinas, Daniel Lezcano, linux-arm-kernel
Hi Marc,
On Mon, Apr 08, 2019 at 05:02:13PM +0100, Marc Zyngier wrote:
> We currently deal with ARM64_ERRATUM_1188873 by always trapping EL0
> accesses for both instruction sets. Although nothing wrong comes out
> of that, people trying to squeeze the last drop of performance from
> buggy HW find this over the top. Oh well.
>
> Let's change the mitigation by flipping the counter enable bit
> on return to userspace. Non-broken HW gets an extra branch on
> the fast path, which is hopefully not the end of the world.
> The arch timer workaround it also removed.
>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
> ---
> arch/arm64/kernel/entry.S | 23 +++++++++++++++++++++++
> drivers/clocksource/arm_arch_timer.c | 15 ---------------
> 2 files changed, 23 insertions(+), 15 deletions(-)
>
> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
> index c50a7a75f2e0..e0aed21ab402 100644
> --- a/arch/arm64/kernel/entry.S
> +++ b/arch/arm64/kernel/entry.S
> @@ -336,6 +336,29 @@ alternative_if ARM64_WORKAROUND_845719
> alternative_else_nop_endif
> #endif
> 3:
> +#ifdef CONFIG_ARM64_ERRATUM_1188873
> +alternative_if_not ARM64_WORKAROUND_1188873
> + b 1f
> +alternative_else_nop_endif
> +
> + ubfx x0, x22, #4, #1 // Extract PSR_MODE32
> + eor x0, x0, #1 // Negate it
> +alternative_if_not ARM64_HAS_VIRT_HOST_EXTN
> + mrs x1, cntkctl_el1
> +alternative_else
> + mrs x1, cnthctl_el2
> +alternative_endif
> + ubfx x2, x1, #1, #1 // Extract EL0VCTEN
> + cmp x2, x0
Aren't the flags live at this point (they indicate native vs compat)?
Maybe you can use that instead of re-extracting PSR_MODE32.
> + b.eq 1f // Matches, nothing to do
> + bfi x1, x0, #1, #1 // Move EL0VCTEN in place
ARCH_TIMER_USR_VCT_ACCESS_EN
> +alternative_if_not ARM64_HAS_VIRT_HOST_EXTN
> + msr cntkctl_el1, x1
> +alternative_else
> + msr cnthctl_el2, x1
> +alternative_endif
> +1:
Sorry to be a pain, but could you use label '4:' here and update the others
in this macro, please?
> +#endif
> apply_ssbd 0, x0, x1
> .endif
>
> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
> index 5fcccc467868..27acc9eb0f7c 100644
> --- a/drivers/clocksource/arm_arch_timer.c
> +++ b/drivers/clocksource/arm_arch_timer.c
I probably need an Ack from Thomas or Daniel on these parts, so I can take
the series via arm64.
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/4] arm64: Restrict ARM64_ERRATUM_1188873 mitigation to AArch32
2019-04-12 13:17 ` Will Deacon
@ 2019-04-15 8:15 ` Marc Zyngier
2019-04-15 8:26 ` Daniel Lezcano
0 siblings, 1 reply; 11+ messages in thread
From: Marc Zyngier @ 2019-04-15 8:15 UTC (permalink / raw)
To: Will Deacon
Cc: Mark Rutland, Catalin Marinas, Daniel Lezcano, linux-arm-kernel
On 12/04/2019 14:17, Will Deacon wrote:
> Hi Marc,
>
> On Mon, Apr 08, 2019 at 05:02:13PM +0100, Marc Zyngier wrote:
>> We currently deal with ARM64_ERRATUM_1188873 by always trapping EL0
>> accesses for both instruction sets. Although nothing wrong comes out
>> of that, people trying to squeeze the last drop of performance from
>> buggy HW find this over the top. Oh well.
>>
>> Let's change the mitigation by flipping the counter enable bit
>> on return to userspace. Non-broken HW gets an extra branch on
>> the fast path, which is hopefully not the end of the world.
>> The arch timer workaround it also removed.
>>
>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>> ---
>> arch/arm64/kernel/entry.S | 23 +++++++++++++++++++++++
>> drivers/clocksource/arm_arch_timer.c | 15 ---------------
>> 2 files changed, 23 insertions(+), 15 deletions(-)
>>
>> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
>> index c50a7a75f2e0..e0aed21ab402 100644
>> --- a/arch/arm64/kernel/entry.S
>> +++ b/arch/arm64/kernel/entry.S
>> @@ -336,6 +336,29 @@ alternative_if ARM64_WORKAROUND_845719
>> alternative_else_nop_endif
>> #endif
>> 3:
>> +#ifdef CONFIG_ARM64_ERRATUM_1188873
>> +alternative_if_not ARM64_WORKAROUND_1188873
>> + b 1f
>> +alternative_else_nop_endif
>> +
>> + ubfx x0, x22, #4, #1 // Extract PSR_MODE32
>> + eor x0, x0, #1 // Negate it
>> +alternative_if_not ARM64_HAS_VIRT_HOST_EXTN
>> + mrs x1, cntkctl_el1
>> +alternative_else
>> + mrs x1, cnthctl_el2
>> +alternative_endif
>> + ubfx x2, x1, #1, #1 // Extract EL0VCTEN
>> + cmp x2, x0
>
> Aren't the flags live at this point (they indicate native vs compat)?
> Maybe you can use that instead of re-extracting PSR_MODE32.
Robin and I have been reworking this, see other emails in the same
thread, and the code looks pretty different now (and not breaking things
anymore).
>
>> + b.eq 1f // Matches, nothing to do
>> + bfi x1, x0, #1, #1 // Move EL0VCTEN in place
>
> ARCH_TIMER_USR_VCT_ACCESS_EN
>
>> +alternative_if_not ARM64_HAS_VIRT_HOST_EXTN
>> + msr cntkctl_el1, x1
>> +alternative_else
>> + msr cnthctl_el2, x1
>> +alternative_endif
>> +1:
>
> Sorry to be a pain, but could you use label '4:' here and update the others
> in this macro, please?
Sure, no problem.
>
>> +#endif
>> apply_ssbd 0, x0, x1
>> .endif
>>
>> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
>> index 5fcccc467868..27acc9eb0f7c 100644
>> --- a/drivers/clocksource/arm_arch_timer.c
>> +++ b/drivers/clocksource/arm_arch_timer.c
>
> I probably need an Ack from Thomas or Daniel on these parts, so I can take
> the series via arm64.
Daniel is on Cc, and hopefully v2 will be OK.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/4] arm64: Restrict ARM64_ERRATUM_1188873 mitigation to AArch32
2019-04-15 8:15 ` Marc Zyngier
@ 2019-04-15 8:26 ` Daniel Lezcano
0 siblings, 0 replies; 11+ messages in thread
From: Daniel Lezcano @ 2019-04-15 8:26 UTC (permalink / raw)
To: Marc Zyngier, Will Deacon; +Cc: Mark Rutland, Catalin Marinas, linux-arm-kernel
On 15/04/2019 10:15, Marc Zyngier wrote:
> On 12/04/2019 14:17, Will Deacon wrote:
>> Hi Marc,
>>
>> On Mon, Apr 08, 2019 at 05:02:13PM +0100, Marc Zyngier wrote:
>>> We currently deal with ARM64_ERRATUM_1188873 by always trapping EL0
>>> accesses for both instruction sets. Although nothing wrong comes out
>>> of that, people trying to squeeze the last drop of performance from
>>> buggy HW find this over the top. Oh well.
>>>
>>> Let's change the mitigation by flipping the counter enable bit
>>> on return to userspace. Non-broken HW gets an extra branch on
>>> the fast path, which is hopefully not the end of the world.
>>> The arch timer workaround it also removed.
>>>
>>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
Hi Marc,
for the arm_arch_timer.c part:
Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
>>> ---
>>> arch/arm64/kernel/entry.S | 23 +++++++++++++++++++++++
>>> drivers/clocksource/arm_arch_timer.c | 15 ---------------
>>> 2 files changed, 23 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
>>> index c50a7a75f2e0..e0aed21ab402 100644
>>> --- a/arch/arm64/kernel/entry.S
>>> +++ b/arch/arm64/kernel/entry.S
>>> @@ -336,6 +336,29 @@ alternative_if ARM64_WORKAROUND_845719
>>> alternative_else_nop_endif
>>> #endif
>>> 3:
>>> +#ifdef CONFIG_ARM64_ERRATUM_1188873
>>> +alternative_if_not ARM64_WORKAROUND_1188873
>>> + b 1f
>>> +alternative_else_nop_endif
>>> +
>>> + ubfx x0, x22, #4, #1 // Extract PSR_MODE32
>>> + eor x0, x0, #1 // Negate it
>>> +alternative_if_not ARM64_HAS_VIRT_HOST_EXTN
>>> + mrs x1, cntkctl_el1
>>> +alternative_else
>>> + mrs x1, cnthctl_el2
>>> +alternative_endif
>>> + ubfx x2, x1, #1, #1 // Extract EL0VCTEN
>>> + cmp x2, x0
>>
>> Aren't the flags live at this point (they indicate native vs compat)?
>> Maybe you can use that instead of re-extracting PSR_MODE32.
>
> Robin and I have been reworking this, see other emails in the same
> thread, and the code looks pretty different now (and not breaking things
> anymore).
>
>>
>>> + b.eq 1f // Matches, nothing to do
>>> + bfi x1, x0, #1, #1 // Move EL0VCTEN in place
>>
>> ARCH_TIMER_USR_VCT_ACCESS_EN
>>
>>> +alternative_if_not ARM64_HAS_VIRT_HOST_EXTN
>>> + msr cntkctl_el1, x1
>>> +alternative_else
>>> + msr cnthctl_el2, x1
>>> +alternative_endif
>>> +1:
>>
>> Sorry to be a pain, but could you use label '4:' here and update the others
>> in this macro, please?
>
> Sure, no problem.
>
>>
>>> +#endif
>>> apply_ssbd 0, x0, x1
>>> .endif
>>>
>>> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
>>> index 5fcccc467868..27acc9eb0f7c 100644
>>> --- a/drivers/clocksource/arm_arch_timer.c
>>> +++ b/drivers/clocksource/arm_arch_timer.c
>>
>> I probably need an Ack from Thomas or Daniel on these parts, so I can take
>> the series via arm64.
>
> Daniel is on Cc, and hopefully v2 will be OK.
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2019-04-15 8:26 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-08 16:02 [PATCH 0/4] arm64: Rework handling of erratum 1188873 Marc Zyngier
2019-04-08 16:02 ` [PATCH 1/4] arm64: Restrict ARM64_ERRATUM_1188873 mitigation to AArch32 Marc Zyngier
2019-04-08 18:34 ` Robin Murphy
2019-04-09 9:18 ` Marc Zyngier
2019-04-09 9:49 ` Robin Murphy
2019-04-12 13:17 ` Will Deacon
2019-04-15 8:15 ` Marc Zyngier
2019-04-15 8:26 ` Daniel Lezcano
2019-04-08 16:02 ` [PATCH 2/4] arm64: Make ARM64_ERRATUM_1188873 depend on COMPAT Marc Zyngier
2019-04-08 16:02 ` [PATCH 3/4] arm64: Add part number for Neoverse N1 Marc Zyngier
2019-04-08 16:02 ` [PATCH 4/4] arm64: Apply ARM64_ERRATUM_1188873 to Neoverse-N1 Marc Zyngier
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).