Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1556544 > unrolled thread
| Started by | Christopher Covington <cov@codeaurora.org> |
|---|---|
| First post | 2017-01-11 15:50 +0100 |
| Last post | 2017-01-11 19:40 +0100 |
| Articles | 16 — 6 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
[PATCH v3 2/5] arm64: Work around Falkor erratum 1003 Christopher Covington <cov@codeaurora.org> - 2017-01-11 15:50 +0100
Re: [PATCH v3 2/5] arm64: Work around Falkor erratum 1003 Catalin Marinas <catalin.marinas@arm.com> - 2017-01-11 19:10 +0100
Re: [PATCH v3 2/5] arm64: Work around Falkor erratum 1003 Marc Zyngier <marc.zyngier@arm.com> - 2017-01-11 19:30 +0100
Re: [PATCH v3 2/5] arm64: Work around Falkor erratum 1003 Mark Rutland <mark.rutland@arm.com> - 2017-01-11 19:50 +0100
Re: [PATCH v3 2/5] arm64: Work around Falkor erratum 1003 Catalin Marinas <catalin.marinas@arm.com> - 2017-01-12 16:50 +0100
Re: [PATCH v3 2/5] arm64: Work around Falkor erratum 1003 Mark Rutland <mark.rutland@arm.com> - 2017-01-12 17:20 +0100
Re: [PATCH v3 2/5] arm64: Work around Falkor erratum 1003 Catalin Marinas <catalin.marinas@arm.com> - 2017-01-12 17:00 +0100
Re: [PATCH v3 2/5] arm64: Work around Falkor erratum 1003 Will Deacon <will.deacon@arm.com> - 2017-01-12 17:10 +0100
Re: [PATCH v3 2/5] arm64: Work around Falkor erratum 1003 Timur Tabi <timur@codeaurora.org> - 2017-01-11 19:40 +0100
Re: [PATCH v3 2/5] arm64: Work around Falkor erratum 1003 Mark Rutland <mark.rutland@arm.com> - 2017-01-11 19:40 +0100
Re: [PATCH v3 2/5] arm64: Work around Falkor erratum 1003 Timur Tabi <timur@codeaurora.org> - 2017-01-11 19:50 +0100
Re: [PATCH v3 2/5] arm64: Work around Falkor erratum 1003 Mark Rutland <mark.rutland@arm.com> - 2017-01-11 19:50 +0100
Re: [PATCH v3 2/5] arm64: Work around Falkor erratum 1003 Christopher Covington <cov@codeaurora.org> - 2017-01-16 15:30 +0100
Re: [PATCH v3 2/5] arm64: Work around Falkor erratum 1003 Marc Zyngier <marc.zyngier@arm.com> - 2017-01-11 20:00 +0100
Re: [PATCH v3 2/5] arm64: Work around Falkor erratum 1003 Catalin Marinas <catalin.marinas@arm.com> - 2017-01-12 11:00 +0100
Re: [PATCH v3 2/5] arm64: Work around Falkor erratum 1003 Mark Rutland <mark.rutland@arm.com> - 2017-01-11 19:40 +0100
| From | Christopher Covington <cov@codeaurora.org> |
|---|---|
| Date | 2017-01-11 15:50 +0100 |
| Subject | [PATCH v3 2/5] arm64: Work around Falkor erratum 1003 |
| Message-ID | <sYsnU-86a-25@gated-at.bofh.it> |
From: Shanker Donthineni <shankerd@codeaurora.org>
On the Qualcomm Datacenter Technologies Falkor v1 CPU, memory accesses may
allocate TLB entries using an incorrect ASID when TTBRx_EL1 is being
updated. Changing the TTBRx_EL1[ASID] and TTBRx_EL1[BADDR] fields
separately using a reserved ASID will ensure that there are no TLB entries
with incorrect ASID after changing the the ASID.
Pseudo code:
write TTBRx_EL1[ASID] to a reserved value
ISB
write TTBRx_EL1[BADDR] to a desired value
ISB
write TTBRx_EL1[ASID] to a desired value
ISB
EL2 and EL3 code changing the EL1&0 ASID is not subject to this erratum
because hardware is prohibited from performing translations from an
out-of-context translation regime.
Signed-off-by: Shanker Donthineni <shankerd@codeaurora.org>
Signed-off-by: Christopher Covington <cov@codeaurora.org>
---
Documentation/arm64/silicon-errata.txt | 43 +++++++++++++++++-----------------
arch/arm64/Kconfig | 11 +++++++++
arch/arm64/include/asm/cpucaps.h | 3 ++-
arch/arm64/include/asm/mmu_context.h | 8 ++++++-
arch/arm64/kernel/cpu_errata.c | 7 ++++++
arch/arm64/mm/context.c | 10 ++++++++
arch/arm64/mm/proc.S | 13 ++++++++++
7 files changed, 72 insertions(+), 23 deletions(-)
diff --git a/Documentation/arm64/silicon-errata.txt b/Documentation/arm64/silicon-errata.txt
index 405da11..7151aed 100644
--- a/Documentation/arm64/silicon-errata.txt
+++ b/Documentation/arm64/silicon-errata.txt
@@ -42,24 +42,25 @@ file acts as a registry of software workarounds in the Linux Kernel and
will be updated when new workarounds are committed and backported to
stable kernels.
-| Implementor | Component | Erratum ID | Kconfig |
-+----------------+-----------------+-----------------+-------------------------+
-| ARM | Cortex-A53 | #826319 | ARM64_ERRATUM_826319 |
-| ARM | Cortex-A53 | #827319 | ARM64_ERRATUM_827319 |
-| ARM | Cortex-A53 | #824069 | ARM64_ERRATUM_824069 |
-| ARM | Cortex-A53 | #819472 | ARM64_ERRATUM_819472 |
-| ARM | Cortex-A53 | #845719 | ARM64_ERRATUM_845719 |
-| ARM | Cortex-A53 | #843419 | ARM64_ERRATUM_843419 |
-| ARM | Cortex-A57 | #832075 | ARM64_ERRATUM_832075 |
-| ARM | Cortex-A57 | #852523 | N/A |
-| ARM | Cortex-A57 | #834220 | ARM64_ERRATUM_834220 |
-| ARM | Cortex-A72 | #853709 | N/A |
-| ARM | MMU-500 | #841119,#826419 | N/A |
-| | | | |
-| Cavium | ThunderX ITS | #22375, #24313 | CAVIUM_ERRATUM_22375 |
-| Cavium | ThunderX ITS | #23144 | CAVIUM_ERRATUM_23144 |
-| Cavium | ThunderX GICv3 | #23154 | CAVIUM_ERRATUM_23154 |
-| Cavium | ThunderX Core | #27456 | CAVIUM_ERRATUM_27456 |
-| Cavium | ThunderX SMMUv2 | #27704 | N/A |
-| | | | |
-| Freescale/NXP | LS2080A/LS1043A | A-008585 | FSL_ERRATUM_A008585 |
+| Implementor | Component | Erratum ID | Kconfig |
++---------------+-----------------+-----------------+--------------------------+
+| ARM | Cortex-A53 | #826319 | ARM64_ERRATUM_826319 |
+| ARM | Cortex-A53 | #827319 | ARM64_ERRATUM_827319 |
+| ARM | Cortex-A53 | #824069 | ARM64_ERRATUM_824069 |
+| ARM | Cortex-A53 | #819472 | ARM64_ERRATUM_819472 |
+| ARM | Cortex-A53 | #845719 | ARM64_ERRATUM_845719 |
+| ARM | Cortex-A53 | #843419 | ARM64_ERRATUM_843419 |
+| ARM | Cortex-A57 | #832075 | ARM64_ERRATUM_832075 |
+| ARM | Cortex-A57 | #852523 | N/A |
+| ARM | Cortex-A57 | #834220 | ARM64_ERRATUM_834220 |
+| ARM | Cortex-A72 | #853709 | N/A |
+| ARM | MMU-500 | #841119,#826419 | N/A |
+| | | | |
+| Cavium | ThunderX ITS | #22375, #24313 | CAVIUM_ERRATUM_22375 |
+| Cavium | ThunderX ITS | #23144 | CAVIUM_ERRATUM_23144 |
+| Cavium | ThunderX GICv3 | #23154 | CAVIUM_ERRATUM_23154 |
+| Cavium | ThunderX Core | #27456 | CAVIUM_ERRATUM_27456 |
+| Cavium | ThunderX SMMUv2 | #27704 | N/A |
+| | | | |
+| Freescale/NXP | LS2080A/LS1043A | A-008585 | FSL_ERRATUM_A008585 |
+| Qualcomm | Falkor v1 | E1003 | QCOM_FALKOR_ERRATUM_1003 |
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 1117421..2a80ac9 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -479,6 +479,17 @@ config CAVIUM_ERRATUM_27456
If unsure, say Y.
+config QCOM_FALKOR_ERRATUM_1003
+ bool "Falkor E1003: Incorrect translation due to ASID change"
+ default y
+ help
+ An incorrect translation TLBI entry may be created while changing the
+ ASID and translation table address together for TTBR0_EL1. The
+ workaround for this issue is to use a reserved ASID in
+ cpu_do_switch_mm() before switching to the target ASID.
+
+ If unsure, say Y.
+
endmenu
diff --git a/arch/arm64/include/asm/cpucaps.h b/arch/arm64/include/asm/cpucaps.h
index 4174f09..5aaf7ee 100644
--- a/arch/arm64/include/asm/cpucaps.h
+++ b/arch/arm64/include/asm/cpucaps.h
@@ -35,7 +35,8 @@
#define ARM64_HYP_OFFSET_LOW 14
#define ARM64_MISMATCHED_CACHE_LINE_SIZE 15
#define ARM64_HAS_NO_FPSIMD 16
+#define ARM64_WORKAROUND_QCOM_FALKOR_E1003 17
-#define ARM64_NCAPS 17
+#define ARM64_NCAPS 18
#endif /* __ASM_CPUCAPS_H */
diff --git a/arch/arm64/include/asm/mmu_context.h b/arch/arm64/include/asm/mmu_context.h
index 0363fe8..9632b05 100644
--- a/arch/arm64/include/asm/mmu_context.h
+++ b/arch/arm64/include/asm/mmu_context.h
@@ -19,6 +19,10 @@
#ifndef __ASM_MMU_CONTEXT_H
#define __ASM_MMU_CONTEXT_H
+#define FALKOR_RESERVED_ASID 1
+
+#ifndef __ASSEMBLY__
+
#include <linux/compiler.h>
#include <linux/sched.h>
@@ -220,4 +224,6 @@ switch_mm(struct mm_struct *prev, struct mm_struct *next,
void verify_cpu_asid_bits(void);
-#endif
+#endif /* !__ASSEMBLY__ */
+
+#endif /* !__ASM_MMU_CONTEXT_H */
diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
index b75e917..787b542 100644
--- a/arch/arm64/kernel/cpu_errata.c
+++ b/arch/arm64/kernel/cpu_errata.c
@@ -130,6 +130,13 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
.def_scope = SCOPE_LOCAL_CPU,
.enable = cpu_enable_trap_ctr_access,
},
+#ifdef CONFIG_QCOM_FALKOR_ERRATUM_1003
+ {
+ .desc = "Qualcomm Falkor erratum 1003",
+ .capability = ARM64_WORKAROUND_QCOM_FALKOR_E1003,
+ MIDR_RANGE(MIDR_QCOM_FALKOR_V1, 0x00, 0x00),
+ },
+#endif
{
}
};
diff --git a/arch/arm64/mm/context.c b/arch/arm64/mm/context.c
index 4c63cb1..5a0a82a 100644
--- a/arch/arm64/mm/context.c
+++ b/arch/arm64/mm/context.c
@@ -87,6 +87,11 @@ static void flush_context(unsigned int cpu)
/* Update the list of reserved ASIDs and the ASID bitmap. */
bitmap_clear(asid_map, 0, NUM_USER_ASIDS);
+ /* Reserve ASID for Falkor erratum 1003 */
+ if (IS_ENABLED(CONFIG_QCOM_FALKOR_ERRATUM_1003) &&
+ cpus_have_cap(ARM64_WORKAROUND_QCOM_FALKOR_E1003))
+ __set_bit(FALKOR_RESERVED_ASID, asid_map);
+
/*
* Ensure the generation bump is observed before we xchg the
* active_asids.
@@ -244,6 +249,11 @@ static int asids_init(void)
panic("Failed to allocate bitmap for %lu ASIDs\n",
NUM_USER_ASIDS);
+ /* Reserve ASID for Falkor erratum 1003 */
+ if (IS_ENABLED(CONFIG_QCOM_FALKOR_ERRATUM_1003) &&
+ cpus_have_cap(ARM64_WORKAROUND_QCOM_FALKOR_E1003))
+ __set_bit(FALKOR_RESERVED_ASID, asid_map);
+
pr_info("ASID allocator initialised with %lu entries\n", NUM_USER_ASIDS);
return 0;
}
diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
index 32682be..9ee46df 100644
--- a/arch/arm64/mm/proc.S
+++ b/arch/arm64/mm/proc.S
@@ -23,6 +23,7 @@
#include <asm/assembler.h>
#include <asm/asm-offsets.h>
#include <asm/hwcap.h>
+#include <asm/mmu_context.h>
#include <asm/pgtable.h>
#include <asm/pgtable-hwdef.h>
#include <asm/cpufeature.h>
@@ -140,6 +141,18 @@ ENDPROC(cpu_do_resume)
ENTRY(cpu_do_switch_mm)
mmid x1, x1 // get mm->context.id
bfi x0, x1, #48, #16 // set the ASID
+#ifdef CONFIG_QCOM_FALKOR_ERRATUM_1003
+alternative_if ARM64_WORKAROUND_QCOM_FALKOR_E1003
+ mrs x2, ttbr0_el1
+ mov x3, #FALKOR_RESERVED_ASID
+ bfi x2, x3, #48, #16 // reserved ASID + old BADDR
+ msr ttbr0_el1, x2
+ isb
+ bfi x2, x0, #0, #48 // reserved ASID + new BADDR
+ msr ttbr0_el1, x2
+ isb
+alternative_else_nop_endif
+#endif
msr ttbr0_el1, x0 // set TTBR0
isb
post_ttbr0_update_workaround
--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora
Forum, a Linux Foundation Collaborative Project.
[toc] | [next] | [standalone]
| From | Catalin Marinas <catalin.marinas@arm.com> |
|---|---|
| Date | 2017-01-11 19:10 +0100 |
| Message-ID | <sYvvt-1Kq-51@gated-at.bofh.it> |
| In reply to | #1556544 |
Some minor comments below, nothing fundamental (as long as you say the
new sequence doesn't have the speculative TLB load problem I mentioned
on a previous version).
On Wed, Jan 11, 2017 at 09:41:15AM -0500, Christopher Covington wrote:
> diff --git a/Documentation/arm64/silicon-errata.txt b/Documentation/arm64/silicon-errata.txt
> index 405da11..7151aed 100644
> --- a/Documentation/arm64/silicon-errata.txt
> +++ b/Documentation/arm64/silicon-errata.txt
> @@ -42,24 +42,25 @@ file acts as a registry of software workarounds in the Linux Kernel and
> will be updated when new workarounds are committed and backported to
> stable kernels.
>
> -| Implementor | Component | Erratum ID | Kconfig |
> -+----------------+-----------------+-----------------+-------------------------+
> -| ARM | Cortex-A53 | #826319 | ARM64_ERRATUM_826319 |
> -| ARM | Cortex-A53 | #827319 | ARM64_ERRATUM_827319 |
> -| ARM | Cortex-A53 | #824069 | ARM64_ERRATUM_824069 |
> -| ARM | Cortex-A53 | #819472 | ARM64_ERRATUM_819472 |
> -| ARM | Cortex-A53 | #845719 | ARM64_ERRATUM_845719 |
> -| ARM | Cortex-A53 | #843419 | ARM64_ERRATUM_843419 |
> -| ARM | Cortex-A57 | #832075 | ARM64_ERRATUM_832075 |
> -| ARM | Cortex-A57 | #852523 | N/A |
> -| ARM | Cortex-A57 | #834220 | ARM64_ERRATUM_834220 |
> -| ARM | Cortex-A72 | #853709 | N/A |
> -| ARM | MMU-500 | #841119,#826419 | N/A |
> -| | | | |
> -| Cavium | ThunderX ITS | #22375, #24313 | CAVIUM_ERRATUM_22375 |
> -| Cavium | ThunderX ITS | #23144 | CAVIUM_ERRATUM_23144 |
> -| Cavium | ThunderX GICv3 | #23154 | CAVIUM_ERRATUM_23154 |
> -| Cavium | ThunderX Core | #27456 | CAVIUM_ERRATUM_27456 |
> -| Cavium | ThunderX SMMUv2 | #27704 | N/A |
> -| | | | |
> -| Freescale/NXP | LS2080A/LS1043A | A-008585 | FSL_ERRATUM_A008585 |
> +| Implementor | Component | Erratum ID | Kconfig |
> ++---------------+-----------------+-----------------+--------------------------+
> +| ARM | Cortex-A53 | #826319 | ARM64_ERRATUM_826319 |
> +| ARM | Cortex-A53 | #827319 | ARM64_ERRATUM_827319 |
> +| ARM | Cortex-A53 | #824069 | ARM64_ERRATUM_824069 |
> +| ARM | Cortex-A53 | #819472 | ARM64_ERRATUM_819472 |
> +| ARM | Cortex-A53 | #845719 | ARM64_ERRATUM_845719 |
> +| ARM | Cortex-A53 | #843419 | ARM64_ERRATUM_843419 |
> +| ARM | Cortex-A57 | #832075 | ARM64_ERRATUM_832075 |
> +| ARM | Cortex-A57 | #852523 | N/A |
> +| ARM | Cortex-A57 | #834220 | ARM64_ERRATUM_834220 |
> +| ARM | Cortex-A72 | #853709 | N/A |
> +| ARM | MMU-500 | #841119,#826419 | N/A |
> +| | | | |
> +| Cavium | ThunderX ITS | #22375, #24313 | CAVIUM_ERRATUM_22375 |
> +| Cavium | ThunderX ITS | #23144 | CAVIUM_ERRATUM_23144 |
> +| Cavium | ThunderX GICv3 | #23154 | CAVIUM_ERRATUM_23154 |
> +| Cavium | ThunderX Core | #27456 | CAVIUM_ERRATUM_27456 |
> +| Cavium | ThunderX SMMUv2 | #27704 | N/A |
> +| | | | |
> +| Freescale/NXP | LS2080A/LS1043A | A-008585 | FSL_ERRATUM_A008585 |
> +| Qualcomm | Falkor v1 | E1003 | QCOM_FALKOR_ERRATUM_1003 |
Please don't change the "Implementor" column width, there is no point
and it makes the patch harder to read (i.e. this hunk should only have
one line).
> diff --git a/arch/arm64/mm/context.c b/arch/arm64/mm/context.c
> index 4c63cb1..5a0a82a 100644
> --- a/arch/arm64/mm/context.c
> +++ b/arch/arm64/mm/context.c
> @@ -87,6 +87,11 @@ static void flush_context(unsigned int cpu)
> /* Update the list of reserved ASIDs and the ASID bitmap. */
> bitmap_clear(asid_map, 0, NUM_USER_ASIDS);
>
> + /* Reserve ASID for Falkor erratum 1003 */
> + if (IS_ENABLED(CONFIG_QCOM_FALKOR_ERRATUM_1003) &&
> + cpus_have_cap(ARM64_WORKAROUND_QCOM_FALKOR_E1003))
> + __set_bit(FALKOR_RESERVED_ASID, asid_map);
> +
> /*
> * Ensure the generation bump is observed before we xchg the
> * active_asids.
> @@ -244,6 +249,11 @@ static int asids_init(void)
> panic("Failed to allocate bitmap for %lu ASIDs\n",
> NUM_USER_ASIDS);
>
> + /* Reserve ASID for Falkor erratum 1003 */
> + if (IS_ENABLED(CONFIG_QCOM_FALKOR_ERRATUM_1003) &&
> + cpus_have_cap(ARM64_WORKAROUND_QCOM_FALKOR_E1003))
> + __set_bit(FALKOR_RESERVED_ASID, asid_map);
> +
> pr_info("ASID allocator initialised with %lu entries\n", NUM_USER_ASIDS);
> return 0;
> }
You could as well write a small static function in this file and call it
twice.
> diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
> index 32682be..9ee46df 100644
> --- a/arch/arm64/mm/proc.S
> +++ b/arch/arm64/mm/proc.S
> @@ -23,6 +23,7 @@
> #include <asm/assembler.h>
> #include <asm/asm-offsets.h>
> #include <asm/hwcap.h>
> +#include <asm/mmu_context.h>
> #include <asm/pgtable.h>
> #include <asm/pgtable-hwdef.h>
> #include <asm/cpufeature.h>
> @@ -140,6 +141,18 @@ ENDPROC(cpu_do_resume)
> ENTRY(cpu_do_switch_mm)
> mmid x1, x1 // get mm->context.id
> bfi x0, x1, #48, #16 // set the ASID
> +#ifdef CONFIG_QCOM_FALKOR_ERRATUM_1003
> +alternative_if ARM64_WORKAROUND_QCOM_FALKOR_E1003
> + mrs x2, ttbr0_el1
> + mov x3, #FALKOR_RESERVED_ASID
> + bfi x2, x3, #48, #16 // reserved ASID + old BADDR
> + msr ttbr0_el1, x2
> + isb
> + bfi x2, x0, #0, #48 // reserved ASID + new BADDR
> + msr ttbr0_el1, x2
> + isb
> +alternative_else_nop_endif
> +#endif
> msr ttbr0_el1, x0 // set TTBR0
> isb
> post_ttbr0_update_workaround
Please move the above hunk to a pre_ttbr0_update_workaround macro for
consistency with post_ttbr0_update_workaround.
--
Catalin
[toc] | [prev] | [next] | [standalone]
| From | Marc Zyngier <marc.zyngier@arm.com> |
|---|---|
| Date | 2017-01-11 19:30 +0100 |
| Message-ID | <sYvOO-1QW-21@gated-at.bofh.it> |
| In reply to | #1556798 |
On 11/01/17 18:06, Catalin Marinas wrote:
> Some minor comments below, nothing fundamental (as long as you say the
> new sequence doesn't have the speculative TLB load problem I mentioned
> on a previous version).
>
> On Wed, Jan 11, 2017 at 09:41:15AM -0500, Christopher Covington wrote:
>> diff --git a/Documentation/arm64/silicon-errata.txt b/Documentation/arm64/silicon-errata.txt
>> index 405da11..7151aed 100644
>> --- a/Documentation/arm64/silicon-errata.txt
>> +++ b/Documentation/arm64/silicon-errata.txt
>> @@ -42,24 +42,25 @@ file acts as a registry of software workarounds in the Linux Kernel and
>> will be updated when new workarounds are committed and backported to
>> stable kernels.
>>
>> -| Implementor | Component | Erratum ID | Kconfig |
>> -+----------------+-----------------+-----------------+-------------------------+
>> -| ARM | Cortex-A53 | #826319 | ARM64_ERRATUM_826319 |
>> -| ARM | Cortex-A53 | #827319 | ARM64_ERRATUM_827319 |
>> -| ARM | Cortex-A53 | #824069 | ARM64_ERRATUM_824069 |
>> -| ARM | Cortex-A53 | #819472 | ARM64_ERRATUM_819472 |
>> -| ARM | Cortex-A53 | #845719 | ARM64_ERRATUM_845719 |
>> -| ARM | Cortex-A53 | #843419 | ARM64_ERRATUM_843419 |
>> -| ARM | Cortex-A57 | #832075 | ARM64_ERRATUM_832075 |
>> -| ARM | Cortex-A57 | #852523 | N/A |
>> -| ARM | Cortex-A57 | #834220 | ARM64_ERRATUM_834220 |
>> -| ARM | Cortex-A72 | #853709 | N/A |
>> -| ARM | MMU-500 | #841119,#826419 | N/A |
>> -| | | | |
>> -| Cavium | ThunderX ITS | #22375, #24313 | CAVIUM_ERRATUM_22375 |
>> -| Cavium | ThunderX ITS | #23144 | CAVIUM_ERRATUM_23144 |
>> -| Cavium | ThunderX GICv3 | #23154 | CAVIUM_ERRATUM_23154 |
>> -| Cavium | ThunderX Core | #27456 | CAVIUM_ERRATUM_27456 |
>> -| Cavium | ThunderX SMMUv2 | #27704 | N/A |
>> -| | | | |
>> -| Freescale/NXP | LS2080A/LS1043A | A-008585 | FSL_ERRATUM_A008585 |
>> +| Implementor | Component | Erratum ID | Kconfig |
>> ++---------------+-----------------+-----------------+--------------------------+
>> +| ARM | Cortex-A53 | #826319 | ARM64_ERRATUM_826319 |
>> +| ARM | Cortex-A53 | #827319 | ARM64_ERRATUM_827319 |
>> +| ARM | Cortex-A53 | #824069 | ARM64_ERRATUM_824069 |
>> +| ARM | Cortex-A53 | #819472 | ARM64_ERRATUM_819472 |
>> +| ARM | Cortex-A53 | #845719 | ARM64_ERRATUM_845719 |
>> +| ARM | Cortex-A53 | #843419 | ARM64_ERRATUM_843419 |
>> +| ARM | Cortex-A57 | #832075 | ARM64_ERRATUM_832075 |
>> +| ARM | Cortex-A57 | #852523 | N/A |
>> +| ARM | Cortex-A57 | #834220 | ARM64_ERRATUM_834220 |
>> +| ARM | Cortex-A72 | #853709 | N/A |
>> +| ARM | MMU-500 | #841119,#826419 | N/A |
>> +| | | | |
>> +| Cavium | ThunderX ITS | #22375, #24313 | CAVIUM_ERRATUM_22375 |
>> +| Cavium | ThunderX ITS | #23144 | CAVIUM_ERRATUM_23144 |
>> +| Cavium | ThunderX GICv3 | #23154 | CAVIUM_ERRATUM_23154 |
>> +| Cavium | ThunderX Core | #27456 | CAVIUM_ERRATUM_27456 |
>> +| Cavium | ThunderX SMMUv2 | #27704 | N/A |
>> +| | | | |
>> +| Freescale/NXP | LS2080A/LS1043A | A-008585 | FSL_ERRATUM_A008585 |
>> +| Qualcomm | Falkor v1 | E1003 | QCOM_FALKOR_ERRATUM_1003 |
>
> Please don't change the "Implementor" column width, there is no point
> and it makes the patch harder to read (i.e. this hunk should only have
> one line).
>
>> diff --git a/arch/arm64/mm/context.c b/arch/arm64/mm/context.c
>> index 4c63cb1..5a0a82a 100644
>> --- a/arch/arm64/mm/context.c
>> +++ b/arch/arm64/mm/context.c
>> @@ -87,6 +87,11 @@ static void flush_context(unsigned int cpu)
>> /* Update the list of reserved ASIDs and the ASID bitmap. */
>> bitmap_clear(asid_map, 0, NUM_USER_ASIDS);
>>
>> + /* Reserve ASID for Falkor erratum 1003 */
>> + if (IS_ENABLED(CONFIG_QCOM_FALKOR_ERRATUM_1003) &&
>> + cpus_have_cap(ARM64_WORKAROUND_QCOM_FALKOR_E1003))
>> + __set_bit(FALKOR_RESERVED_ASID, asid_map);
>> +
>> /*
>> * Ensure the generation bump is observed before we xchg the
>> * active_asids.
>> @@ -244,6 +249,11 @@ static int asids_init(void)
>> panic("Failed to allocate bitmap for %lu ASIDs\n",
>> NUM_USER_ASIDS);
>>
>> + /* Reserve ASID for Falkor erratum 1003 */
>> + if (IS_ENABLED(CONFIG_QCOM_FALKOR_ERRATUM_1003) &&
>> + cpus_have_cap(ARM64_WORKAROUND_QCOM_FALKOR_E1003))
>> + __set_bit(FALKOR_RESERVED_ASID, asid_map);
>> +
>> pr_info("ASID allocator initialised with %lu entries\n", NUM_USER_ASIDS);
>> return 0;
>> }
>
> You could as well write a small static function in this file and call it
> twice.
>
>> diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
>> index 32682be..9ee46df 100644
>> --- a/arch/arm64/mm/proc.S
>> +++ b/arch/arm64/mm/proc.S
>> @@ -23,6 +23,7 @@
>> #include <asm/assembler.h>
>> #include <asm/asm-offsets.h>
>> #include <asm/hwcap.h>
>> +#include <asm/mmu_context.h>
>> #include <asm/pgtable.h>
>> #include <asm/pgtable-hwdef.h>
>> #include <asm/cpufeature.h>
>> @@ -140,6 +141,18 @@ ENDPROC(cpu_do_resume)
>> ENTRY(cpu_do_switch_mm)
>> mmid x1, x1 // get mm->context.id
>> bfi x0, x1, #48, #16 // set the ASID
>> +#ifdef CONFIG_QCOM_FALKOR_ERRATUM_1003
>> +alternative_if ARM64_WORKAROUND_QCOM_FALKOR_E1003
>> + mrs x2, ttbr0_el1
>> + mov x3, #FALKOR_RESERVED_ASID
>> + bfi x2, x3, #48, #16 // reserved ASID + old BADDR
>> + msr ttbr0_el1, x2
>> + isb
>> + bfi x2, x0, #0, #48 // reserved ASID + new BADDR
>> + msr ttbr0_el1, x2
>> + isb
>> +alternative_else_nop_endif
>> +#endif
>> msr ttbr0_el1, x0 // set TTBR0
>> isb
>> post_ttbr0_update_workaround
>
> Please move the above hunk to a pre_ttbr0_update_workaround macro for
> consistency with post_ttbr0_update_workaround.
In which case (and also for consistency), should we add that pre_ttbr0
macro to entry.S, just before __uaccess_ttbr0_enable? It may not be
needed in the SW pan case, but it is probably worth entertaining the
idea that there may be something to do there...
Thanks,
M.
--
Jazz is not dead. It just smells funny...
[toc] | [prev] | [next] | [standalone]
| From | Mark Rutland <mark.rutland@arm.com> |
|---|---|
| Date | 2017-01-11 19:50 +0100 |
| Message-ID | <sYw8a-1XP-21@gated-at.bofh.it> |
| In reply to | #1556815 |
On Wed, Jan 11, 2017 at 06:22:08PM +0000, Marc Zyngier wrote: > On 11/01/17 18:06, Catalin Marinas wrote: > > On Wed, Jan 11, 2017 at 09:41:15AM -0500, Christopher Covington wrote: > >> diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S > >> index 32682be..9ee46df 100644 > >> --- a/arch/arm64/mm/proc.S > >> +++ b/arch/arm64/mm/proc.S > >> @@ -23,6 +23,7 @@ > >> #include <asm/assembler.h> > >> #include <asm/asm-offsets.h> > >> #include <asm/hwcap.h> > >> +#include <asm/mmu_context.h> > >> #include <asm/pgtable.h> > >> #include <asm/pgtable-hwdef.h> > >> #include <asm/cpufeature.h> > >> @@ -140,6 +141,18 @@ ENDPROC(cpu_do_resume) > >> ENTRY(cpu_do_switch_mm) > >> mmid x1, x1 // get mm->context.id > >> bfi x0, x1, #48, #16 // set the ASID > >> +#ifdef CONFIG_QCOM_FALKOR_ERRATUM_1003 > >> +alternative_if ARM64_WORKAROUND_QCOM_FALKOR_E1003 > >> + mrs x2, ttbr0_el1 > >> + mov x3, #FALKOR_RESERVED_ASID > >> + bfi x2, x3, #48, #16 // reserved ASID + old BADDR > >> + msr ttbr0_el1, x2 > >> + isb > >> + bfi x2, x0, #0, #48 // reserved ASID + new BADDR > >> + msr ttbr0_el1, x2 > >> + isb > >> +alternative_else_nop_endif > >> +#endif > >> msr ttbr0_el1, x0 // set TTBR0 > >> isb > >> post_ttbr0_update_workaround > > > > Please move the above hunk to a pre_ttbr0_update_workaround macro for > > consistency with post_ttbr0_update_workaround. > > In which case (and also for consistency), should we add that pre_ttbr0 > macro to entry.S, just before __uaccess_ttbr0_enable? It may not be > needed in the SW pan case, but it is probably worth entertaining the > idea that there may be something to do there... Likewise, I beleive we may need to modify cpu_set_reserved_ttbr0(). Thanks, Mark.
[toc] | [prev] | [next] | [standalone]
| From | Catalin Marinas <catalin.marinas@arm.com> |
|---|---|
| Date | 2017-01-12 16:50 +0100 |
| Message-ID | <sYPNw-5HL-27@gated-at.bofh.it> |
| In reply to | #1556836 |
On Wed, Jan 11, 2017 at 06:40:52PM +0000, Mark Rutland wrote: > On Wed, Jan 11, 2017 at 06:22:08PM +0000, Marc Zyngier wrote: > > On 11/01/17 18:06, Catalin Marinas wrote: > > > On Wed, Jan 11, 2017 at 09:41:15AM -0500, Christopher Covington wrote: > > >> diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S > > >> index 32682be..9ee46df 100644 > > >> --- a/arch/arm64/mm/proc.S > > >> +++ b/arch/arm64/mm/proc.S > > >> @@ -23,6 +23,7 @@ > > >> #include <asm/assembler.h> > > >> #include <asm/asm-offsets.h> > > >> #include <asm/hwcap.h> > > >> +#include <asm/mmu_context.h> > > >> #include <asm/pgtable.h> > > >> #include <asm/pgtable-hwdef.h> > > >> #include <asm/cpufeature.h> > > >> @@ -140,6 +141,18 @@ ENDPROC(cpu_do_resume) > > >> ENTRY(cpu_do_switch_mm) > > >> mmid x1, x1 // get mm->context.id > > >> bfi x0, x1, #48, #16 // set the ASID > > >> +#ifdef CONFIG_QCOM_FALKOR_ERRATUM_1003 > > >> +alternative_if ARM64_WORKAROUND_QCOM_FALKOR_E1003 > > >> + mrs x2, ttbr0_el1 > > >> + mov x3, #FALKOR_RESERVED_ASID > > >> + bfi x2, x3, #48, #16 // reserved ASID + old BADDR > > >> + msr ttbr0_el1, x2 > > >> + isb > > >> + bfi x2, x0, #0, #48 // reserved ASID + new BADDR > > >> + msr ttbr0_el1, x2 > > >> + isb > > >> +alternative_else_nop_endif > > >> +#endif > > >> msr ttbr0_el1, x0 // set TTBR0 > > >> isb > > >> post_ttbr0_update_workaround > > > > > > Please move the above hunk to a pre_ttbr0_update_workaround macro for > > > consistency with post_ttbr0_update_workaround. > > > > In which case (and also for consistency), should we add that pre_ttbr0 > > macro to entry.S, just before __uaccess_ttbr0_enable? It may not be > > needed in the SW pan case, but it is probably worth entertaining the > > idea that there may be something to do there... > > Likewise, I beleive we may need to modify cpu_set_reserved_ttbr0(). This may be fine if my assumptions about this erratum are correct. In the cpu_set_reserved_ttbr0() case we set TTBR0_EL1 to a table without any entries, so no new entries could be tagged with the old ASID. -- Catalin
[toc] | [prev] | [next] | [standalone]
| From | Mark Rutland <mark.rutland@arm.com> |
|---|---|
| Date | 2017-01-12 17:20 +0100 |
| Message-ID | <sYQgy-66R-29@gated-at.bofh.it> |
| In reply to | #1557535 |
On Thu, Jan 12, 2017 at 03:45:48PM +0000, Catalin Marinas wrote: > On Wed, Jan 11, 2017 at 06:40:52PM +0000, Mark Rutland wrote: > > Likewise, I beleive we may need to modify cpu_set_reserved_ttbr0(). > > This may be fine if my assumptions about this erratum are correct. In > the cpu_set_reserved_ttbr0() case we set TTBR0_EL1 to a table without > any entries, so no new entries could be tagged with the old ASID. For some reason, I was under the impression that the issue was old table entries being allocated to the new ASID. Looking over the series again, it's not clear to me precisely which cases can occur. It would be good to see that clarified. Thanks, Mark.
[toc] | [prev] | [next] | [standalone]
| From | Catalin Marinas <catalin.marinas@arm.com> |
|---|---|
| Date | 2017-01-12 17:00 +0100 |
| Message-ID | <sYPXb-5KY-3@gated-at.bofh.it> |
| In reply to | #1556815 |
On Wed, Jan 11, 2017 at 06:22:08PM +0000, Marc Zyngier wrote: > On 11/01/17 18:06, Catalin Marinas wrote: > > On Wed, Jan 11, 2017 at 09:41:15AM -0500, Christopher Covington wrote: > >> diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S > >> index 32682be..9ee46df 100644 > >> --- a/arch/arm64/mm/proc.S > >> +++ b/arch/arm64/mm/proc.S > >> @@ -23,6 +23,7 @@ > >> #include <asm/assembler.h> > >> #include <asm/asm-offsets.h> > >> #include <asm/hwcap.h> > >> +#include <asm/mmu_context.h> > >> #include <asm/pgtable.h> > >> #include <asm/pgtable-hwdef.h> > >> #include <asm/cpufeature.h> > >> @@ -140,6 +141,18 @@ ENDPROC(cpu_do_resume) > >> ENTRY(cpu_do_switch_mm) > >> mmid x1, x1 // get mm->context.id > >> bfi x0, x1, #48, #16 // set the ASID > >> +#ifdef CONFIG_QCOM_FALKOR_ERRATUM_1003 > >> +alternative_if ARM64_WORKAROUND_QCOM_FALKOR_E1003 > >> + mrs x2, ttbr0_el1 > >> + mov x3, #FALKOR_RESERVED_ASID > >> + bfi x2, x3, #48, #16 // reserved ASID + old BADDR > >> + msr ttbr0_el1, x2 > >> + isb > >> + bfi x2, x0, #0, #48 // reserved ASID + new BADDR > >> + msr ttbr0_el1, x2 > >> + isb > >> +alternative_else_nop_endif > >> +#endif > >> msr ttbr0_el1, x0 // set TTBR0 > >> isb > >> post_ttbr0_update_workaround > > > > Please move the above hunk to a pre_ttbr0_update_workaround macro for > > consistency with post_ttbr0_update_workaround. > > In which case (and also for consistency), should we add that pre_ttbr0 > macro to entry.S, just before __uaccess_ttbr0_enable? It may not be > needed in the SW pan case, but it is probably worth entertaining the > idea that there may be something to do there... It may actually be needed in entry.S as well. With SW PAN, we move the context switching from cpu_do_switch_mm to the kernel_exit macro when returning to user. In this case we are switching from the reserved ASID 0 and reserved TTBR0_EL1 (pointing to a zeroed page) to the user's TTBR0_EL1 and ASID. If the ASID switch isn't taken into account, we may end up with new TLB entries being tagged with the reserved ASID. Apart from a potential loss of protection with TTBR0 PAN, is there anything else that could go wrong? Maybe a TLB conflict if we mix TLBs from multiple address spaces tagged with the same reserved ASID. If the above is an issue, we would need to patch __uaccess_ttbr0_enable() as well, though I'm more inclined to make this erratum not selectable when TTBR0 PAN is enabled. -- Catalin
[toc] | [prev] | [next] | [standalone]
| From | Will Deacon <will.deacon@arm.com> |
|---|---|
| Date | 2017-01-12 17:10 +0100 |
| Message-ID | <sYQ6R-63x-11@gated-at.bofh.it> |
| In reply to | #1557545 |
On Thu, Jan 12, 2017 at 03:55:58PM +0000, Catalin Marinas wrote: > On Wed, Jan 11, 2017 at 06:22:08PM +0000, Marc Zyngier wrote: > > On 11/01/17 18:06, Catalin Marinas wrote: > > > On Wed, Jan 11, 2017 at 09:41:15AM -0500, Christopher Covington wrote: > > >> diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S > > >> index 32682be..9ee46df 100644 > > >> --- a/arch/arm64/mm/proc.S > > >> +++ b/arch/arm64/mm/proc.S > > >> @@ -23,6 +23,7 @@ > > >> #include <asm/assembler.h> > > >> #include <asm/asm-offsets.h> > > >> #include <asm/hwcap.h> > > >> +#include <asm/mmu_context.h> > > >> #include <asm/pgtable.h> > > >> #include <asm/pgtable-hwdef.h> > > >> #include <asm/cpufeature.h> > > >> @@ -140,6 +141,18 @@ ENDPROC(cpu_do_resume) > > >> ENTRY(cpu_do_switch_mm) > > >> mmid x1, x1 // get mm->context.id > > >> bfi x0, x1, #48, #16 // set the ASID > > >> +#ifdef CONFIG_QCOM_FALKOR_ERRATUM_1003 > > >> +alternative_if ARM64_WORKAROUND_QCOM_FALKOR_E1003 > > >> + mrs x2, ttbr0_el1 > > >> + mov x3, #FALKOR_RESERVED_ASID > > >> + bfi x2, x3, #48, #16 // reserved ASID + old BADDR > > >> + msr ttbr0_el1, x2 > > >> + isb > > >> + bfi x2, x0, #0, #48 // reserved ASID + new BADDR > > >> + msr ttbr0_el1, x2 > > >> + isb > > >> +alternative_else_nop_endif > > >> +#endif > > >> msr ttbr0_el1, x0 // set TTBR0 > > >> isb > > >> post_ttbr0_update_workaround > > > > > > Please move the above hunk to a pre_ttbr0_update_workaround macro for > > > consistency with post_ttbr0_update_workaround. > > > > In which case (and also for consistency), should we add that pre_ttbr0 > > macro to entry.S, just before __uaccess_ttbr0_enable? It may not be > > needed in the SW pan case, but it is probably worth entertaining the > > idea that there may be something to do there... > > It may actually be needed in entry.S as well. With SW PAN, we move the > context switching from cpu_do_switch_mm to the kernel_exit macro when > returning to user. In this case we are switching from the reserved ASID > 0 and reserved TTBR0_EL1 (pointing to a zeroed page) to the user's > TTBR0_EL1 and ASID. If the ASID switch isn't taken into account, we may > end up with new TLB entries being tagged with the reserved ASID. Apart > from a potential loss of protection with TTBR0 PAN, is there anything > else that could go wrong? Maybe a TLB conflict if we mix TLBs from > multiple address spaces tagged with the same reserved ASID. > > If the above is an issue, we would need to patch > __uaccess_ttbr0_enable() as well, though I'm more inclined to make this > erratum not selectable when TTBR0 PAN is enabled. I don't think that's a reasonable approach. By all means change the default, but we need to support kernel images with both of these kconfig options enabled. Will
[toc] | [prev] | [next] | [standalone]
| From | Timur Tabi <timur@codeaurora.org> |
|---|---|
| Date | 2017-01-11 19:40 +0100 |
| Message-ID | <sYvYt-1Uj-1@gated-at.bofh.it> |
| In reply to | #1556798 |
On 01/11/2017 12:33 PM, Mark Rutland wrote: > It'll need to affect all lines since the kconfig column needs to expand > by at least one character to fit QCOM_FALKOR_ERRATUM_1003. Or we can make the macro shorter. -- Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
[toc] | [prev] | [next] | [standalone]
| From | Mark Rutland <mark.rutland@arm.com> |
|---|---|
| Date | 2017-01-11 19:40 +0100 |
| Message-ID | <sYvYu-1Uj-43@gated-at.bofh.it> |
| In reply to | #1556818 |
On Wed, Jan 11, 2017 at 12:35:55PM -0600, Timur Tabi wrote: > On 01/11/2017 12:33 PM, Mark Rutland wrote: > >It'll need to affect all lines since the kconfig column needs to expand > >by at least one character to fit QCOM_FALKOR_ERRATUM_1003. > > Or we can make the macro shorter. The name, as it is, is perfectly descriptive. Let's not sacrifice legibility over a non-issue. Thanks, Mark.
[toc] | [prev] | [next] | [standalone]
| From | Timur Tabi <timur@codeaurora.org> |
|---|---|
| Date | 2017-01-11 19:50 +0100 |
| Message-ID | <sYw8a-1XP-23@gated-at.bofh.it> |
| In reply to | #1556826 |
On 01/11/2017 12:37 PM, Mark Rutland wrote: > The name, as it is, is perfectly descriptive. > > Let's not sacrifice legibility over a non-issue. I don't want to kick a dead horse or anything, but changing it to QCOM_FLKR_ERRATUM_1003 would eliminate all the spacing problems without sacrificing anything. -- Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
[toc] | [prev] | [next] | [standalone]
| From | Mark Rutland <mark.rutland@arm.com> |
|---|---|
| Date | 2017-01-11 19:50 +0100 |
| Message-ID | <sYw8a-1XP-33@gated-at.bofh.it> |
| In reply to | #1556837 |
On Wed, Jan 11, 2017 at 12:40:42PM -0600, Timur Tabi wrote: > On 01/11/2017 12:37 PM, Mark Rutland wrote: > >The name, as it is, is perfectly descriptive. > > > >Let's not sacrifice legibility over a non-issue. > > I don't want to kick a dead horse or anything, but changing it to > QCOM_FLKR_ERRATUM_1003 would eliminate all the spacing problems > without sacrificing anything. The CPU is called "Falkor", not "FLKR", and we're not coming up with an ACPI table name... The ARM Ltd. erratum numbers are global to all parts, so we don't include the part name. Is the 1003 erratum number specific to Falkor? If it's global, you could use QCOM_ERRATUM_1003 instead. Otherwise, QCOM_FALKOR_ERRATUM_1003 is preferable. Thanks, Mark.
[toc] | [prev] | [next] | [standalone]
| From | Christopher Covington <cov@codeaurora.org> |
|---|---|
| Date | 2017-01-16 15:30 +0100 |
| Message-ID | <t0gsh-1yN-1@gated-at.bofh.it> |
| In reply to | #1556840 |
Hi Mark, On 01/11/2017 01:45 PM, Mark Rutland wrote: > On Wed, Jan 11, 2017 at 12:40:42PM -0600, Timur Tabi wrote: >> On 01/11/2017 12:37 PM, Mark Rutland wrote: >>> The name, as it is, is perfectly descriptive. >>> >>> Let's not sacrifice legibility over a non-issue. >> >> I don't want to kick a dead horse or anything, but changing it to >> QCOM_FLKR_ERRATUM_1003 would eliminate all the spacing problems >> without sacrificing anything. > > The CPU is called "Falkor", not "FLKR", and we're not coming up with an > ACPI table name... > > The ARM Ltd. erratum numbers are global to all parts, so we don't > include the part name. Is the 1003 erratum number specific to Falkor? > > If it's global, you could use QCOM_ERRATUM_1003 instead. E1003 is specific to Falkor, and hopefully just its first major revision. Qualcomm Technology's first/previous generation ARMv8 custom microarchitecture used errata numbers below 1000. I am not aware of global coordination in the numbering, unfortunately. > Otherwise, QCOM_FALKOR_ERRATUM_1003 is preferable. Thanks, Cov -- Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
[toc] | [prev] | [next] | [standalone]
| From | Marc Zyngier <marc.zyngier@arm.com> |
|---|---|
| Date | 2017-01-11 20:00 +0100 |
| Message-ID | <sYwhP-21a-23@gated-at.bofh.it> |
| In reply to | #1556837 |
[finally, some proper bikeshedding] On 11/01/17 18:40, Timur Tabi wrote: > On 01/11/2017 12:37 PM, Mark Rutland wrote: >> The name, as it is, is perfectly descriptive. >> >> Let's not sacrifice legibility over a non-issue. > > I don't want to kick a dead horse or anything, but changing it to > QCOM_FLKR_ERRATUM_1003 would eliminate all the spacing problems without > sacrificing anything. Other than not being able to grep for the core name in the source tree, how do you suggest we pronounce FLKR? Because so far, it rolls off the tongue in an interesting way... Thanks, M. -- Jazz is not dead. It just smells funny...
[toc] | [prev] | [next] | [standalone]
| From | Catalin Marinas <catalin.marinas@arm.com> |
|---|---|
| Date | 2017-01-12 11:00 +0100 |
| Message-ID | <sYKkN-2oa-21@gated-at.bofh.it> |
| In reply to | #1556826 |
On Wed, Jan 11, 2017 at 06:37:39PM +0000, Mark Rutland wrote: > On Wed, Jan 11, 2017 at 12:35:55PM -0600, Timur Tabi wrote: > > On 01/11/2017 12:33 PM, Mark Rutland wrote: > > >It'll need to affect all lines since the kconfig column needs to expand > > >by at least one character to fit QCOM_FALKOR_ERRATUM_1003. > > > > Or we can make the macro shorter. > > The name, as it is, is perfectly descriptive. > > Let's not sacrifice legibility over a non-issue. I agree, I didn't realise that the we expand the last column already. It's a non-issue indeed. -- Catalin
[toc] | [prev] | [next] | [standalone]
| From | Mark Rutland <mark.rutland@arm.com> |
|---|---|
| Date | 2017-01-11 19:40 +0100 |
| Message-ID | <sYvYt-1Uj-3@gated-at.bofh.it> |
| In reply to | #1556798 |
On Wed, Jan 11, 2017 at 06:06:27PM +0000, Catalin Marinas wrote: > On Wed, Jan 11, 2017 at 09:41:15AM -0500, Christopher Covington wrote: > > -| Implementor | Component | Erratum ID | Kconfig | > > +| Implementor | Component | Erratum ID | Kconfig | > > +| Qualcomm | Falkor v1 | E1003 | QCOM_FALKOR_ERRATUM_1003 | > > Please don't change the "Implementor" column width, there is no point > and it makes the patch harder to read (i.e. this hunk should only have > one line). It'll need to affect all lines since the kconfig column needs to expand by at least one character to fit QCOM_FALKOR_ERRATUM_1003. I beleive the intent here was to keep the table fitting into a width of 80 characters. IMO we should allow the table to expand past 80 chars (everyone reading this file should be able to resize tehir terminal), and only expand the kconfig column. Thanks, Mark.
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web