* [PATCH v2 0/3] arm64/boot: Forbid the use of BSS symbols in startup code
@ 2025-05-08 11:43 Ard Biesheuvel
2025-05-08 11:43 ` [PATCH v2 1/3] arm64/boot: Move init_pgdir[] and init_idmap_pgdir[] into __pi_ namespace Ard Biesheuvel
` (4 more replies)
0 siblings, 5 replies; 9+ messages in thread
From: Ard Biesheuvel @ 2025-05-08 11:43 UTC (permalink / raw)
To: linux-arm-kernel
Cc: linux-kernel, will, catalin.marinas, mark.rutland, Ard Biesheuvel,
Yeoreum Yun
From: Ard Biesheuvel <ardb@kernel.org>
Move any variables accessed or assigned by the startup code out of BSS,
and into .data, so that we can forbid the use of BSS variables
altogether, by ASSERT()'ing in the linker script that each symbol made
available to the startup code lives before __bss_start in the linker
map.
Changes since v1:
- fix build error due to missing declaration in #1
- work around Clang complaining about the ASSERT() expression in the
linker script
Cc: Yeoreum Yun <yeoreum.yun@arm.com>
Ard Biesheuvel (3):
arm64/boot: Move init_pgdir[] and init_idmap_pgdir[] into __pi_
namespace
arm64/boot: Move global CPU override variables out of BSS
arm64/boot: Disallow BSS exports to startup code
arch/arm64/include/asm/pgtable.h | 2 -
arch/arm64/kernel/cpufeature.c | 22 +++----
arch/arm64/kernel/head.S | 6 +-
arch/arm64/kernel/image-vars.h | 66 ++++++++++----------
arch/arm64/kernel/pi/pi.h | 1 +
arch/arm64/kernel/vmlinux.lds.S | 10 +--
6 files changed, 54 insertions(+), 53 deletions(-)
base-commit: 363cd2b81cfdf706bbfc9ec78db000c9b1ecc552
--
2.49.0.987.g0cc8ee98dc-goog
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 1/3] arm64/boot: Move init_pgdir[] and init_idmap_pgdir[] into __pi_ namespace
2025-05-08 11:43 [PATCH v2 0/3] arm64/boot: Forbid the use of BSS symbols in startup code Ard Biesheuvel
@ 2025-05-08 11:43 ` Ard Biesheuvel
2025-05-08 11:43 ` [PATCH v2 2/3] arm64/boot: Move global CPU override variables out of BSS Ard Biesheuvel
` (3 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Ard Biesheuvel @ 2025-05-08 11:43 UTC (permalink / raw)
To: linux-arm-kernel
Cc: linux-kernel, will, catalin.marinas, mark.rutland, Ard Biesheuvel,
Yeoreum Yun
From: Ard Biesheuvel <ardb@kernel.org>
init_pgdir[] is only referenced from the startup code, but lives after
BSS in the linker map. Before tightening the rules about accessing BSS
from startup code, move init_pgdir[] into the __pi_ namespace, so it
does not need to be exported explicitly.
For symmetry, do the same with init_idmap_pgdir[], although it lives
before BSS.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
arch/arm64/include/asm/pgtable.h | 2 --
arch/arm64/kernel/head.S | 6 +++---
arch/arm64/kernel/image-vars.h | 4 ----
arch/arm64/kernel/pi/pi.h | 1 +
arch/arm64/kernel/vmlinux.lds.S | 8 ++++----
5 files changed, 8 insertions(+), 13 deletions(-)
diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
index d3b538be1500..6a040f0bbfe1 100644
--- a/arch/arm64/include/asm/pgtable.h
+++ b/arch/arm64/include/asm/pgtable.h
@@ -754,8 +754,6 @@ static inline bool pud_table(pud_t pud) { return true; }
PUD_TYPE_TABLE)
#endif
-extern pgd_t init_pg_dir[];
-extern pgd_t init_pg_end[];
extern pgd_t swapper_pg_dir[];
extern pgd_t idmap_pg_dir[];
extern pgd_t tramp_pg_dir[];
diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
index 2ce73525de2c..ca04b338cb0d 100644
--- a/arch/arm64/kernel/head.S
+++ b/arch/arm64/kernel/head.S
@@ -89,7 +89,7 @@ SYM_CODE_START(primary_entry)
adrp x1, early_init_stack
mov sp, x1
mov x29, xzr
- adrp x0, init_idmap_pg_dir
+ adrp x0, __pi_init_idmap_pg_dir
mov x1, xzr
bl __pi_create_init_idmap
@@ -101,7 +101,7 @@ SYM_CODE_START(primary_entry)
cbnz x19, 0f
dmb sy
mov x1, x0 // end of used region
- adrp x0, init_idmap_pg_dir
+ adrp x0, __pi_init_idmap_pg_dir
adr_l x2, dcache_inval_poc
blr x2
b 1f
@@ -507,7 +507,7 @@ SYM_FUNC_END(__no_granule_support)
SYM_FUNC_START_LOCAL(__primary_switch)
adrp x1, reserved_pg_dir
- adrp x2, init_idmap_pg_dir
+ adrp x2, __pi_init_idmap_pg_dir
bl __enable_mmu
adrp x1, early_init_stack
diff --git a/arch/arm64/kernel/image-vars.h b/arch/arm64/kernel/image-vars.h
index 5e3c4b58f279..c3b4c0479d5c 100644
--- a/arch/arm64/kernel/image-vars.h
+++ b/arch/arm64/kernel/image-vars.h
@@ -54,10 +54,6 @@ PROVIDE(__pi_is_midr_in_range_list = is_midr_in_range_list);
PROVIDE(__pi__ctype = _ctype);
PROVIDE(__pi_memstart_offset_seed = memstart_offset_seed);
-PROVIDE(__pi_init_idmap_pg_dir = init_idmap_pg_dir);
-PROVIDE(__pi_init_idmap_pg_end = init_idmap_pg_end);
-PROVIDE(__pi_init_pg_dir = init_pg_dir);
-PROVIDE(__pi_init_pg_end = init_pg_end);
PROVIDE(__pi_swapper_pg_dir = swapper_pg_dir);
PROVIDE(__pi__text = _text);
diff --git a/arch/arm64/kernel/pi/pi.h b/arch/arm64/kernel/pi/pi.h
index c91e5e965cd3..1f4731a4e17e 100644
--- a/arch/arm64/kernel/pi/pi.h
+++ b/arch/arm64/kernel/pi/pi.h
@@ -22,6 +22,7 @@ static inline void *prel64_to_pointer(const prel64_t *offset)
extern bool dynamic_scs_is_enabled;
extern pgd_t init_idmap_pg_dir[], init_idmap_pg_end[];
+extern pgd_t init_pg_dir[], init_pg_end[];
void init_feature_override(u64 boot_status, const void *fdt, int chosen);
u64 kaslr_early_init(void *fdt, int chosen);
diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S
index e73326bd3ff7..466544c47dca 100644
--- a/arch/arm64/kernel/vmlinux.lds.S
+++ b/arch/arm64/kernel/vmlinux.lds.S
@@ -249,9 +249,9 @@ SECTIONS
__inittext_end = .;
__initdata_begin = .;
- init_idmap_pg_dir = .;
+ __pi_init_idmap_pg_dir = .;
. += INIT_IDMAP_DIR_SIZE;
- init_idmap_pg_end = .;
+ __pi_init_idmap_pg_end = .;
.init.data : {
INIT_DATA
@@ -321,9 +321,9 @@ SECTIONS
BSS_SECTION(SBSS_ALIGN, 0, 0)
. = ALIGN(PAGE_SIZE);
- init_pg_dir = .;
+ __pi_init_pg_dir = .;
. += INIT_DIR_SIZE;
- init_pg_end = .;
+ __pi_init_pg_end = .;
/* end of zero-init region */
. += SZ_4K; /* stack for the early C runtime */
--
2.49.0.987.g0cc8ee98dc-goog
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 2/3] arm64/boot: Move global CPU override variables out of BSS
2025-05-08 11:43 [PATCH v2 0/3] arm64/boot: Forbid the use of BSS symbols in startup code Ard Biesheuvel
2025-05-08 11:43 ` [PATCH v2 1/3] arm64/boot: Move init_pgdir[] and init_idmap_pgdir[] into __pi_ namespace Ard Biesheuvel
@ 2025-05-08 11:43 ` Ard Biesheuvel
2025-05-08 11:43 ` [PATCH v2 3/3] arm64/boot: Disallow BSS exports to startup code Ard Biesheuvel
` (2 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Ard Biesheuvel @ 2025-05-08 11:43 UTC (permalink / raw)
To: linux-arm-kernel
Cc: linux-kernel, will, catalin.marinas, mark.rutland, Ard Biesheuvel,
Yeoreum Yun
From: Ard Biesheuvel <ardb@kernel.org>
Accessing BSS will no longer be permitted from the startup code in
arch/arm64/kernel/pi, as some of it executes before BSS is cleared.
Clearing BSS earlier would involve managing cache coherency explicitly
in software, which is a hassle we prefer to avoid.
So move some variables that are assigned by the startup code out of BSS
and into .data.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
arch/arm64/kernel/cpufeature.c | 22 ++++++++++----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 4c46d80aa64b..2cac5e5ca94d 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -765,17 +765,17 @@ static const struct arm64_ftr_bits ftr_raz[] = {
#define ARM64_FTR_REG(id, table) \
__ARM64_FTR_REG_OVERRIDE(#id, id, table, &no_override)
-struct arm64_ftr_override id_aa64mmfr0_override;
-struct arm64_ftr_override id_aa64mmfr1_override;
-struct arm64_ftr_override id_aa64mmfr2_override;
-struct arm64_ftr_override id_aa64pfr0_override;
-struct arm64_ftr_override id_aa64pfr1_override;
-struct arm64_ftr_override id_aa64zfr0_override;
-struct arm64_ftr_override id_aa64smfr0_override;
-struct arm64_ftr_override id_aa64isar1_override;
-struct arm64_ftr_override id_aa64isar2_override;
-
-struct arm64_ftr_override arm64_sw_feature_override;
+struct arm64_ftr_override __read_mostly id_aa64mmfr0_override;
+struct arm64_ftr_override __read_mostly id_aa64mmfr1_override;
+struct arm64_ftr_override __read_mostly id_aa64mmfr2_override;
+struct arm64_ftr_override __read_mostly id_aa64pfr0_override;
+struct arm64_ftr_override __read_mostly id_aa64pfr1_override;
+struct arm64_ftr_override __read_mostly id_aa64zfr0_override;
+struct arm64_ftr_override __read_mostly id_aa64smfr0_override;
+struct arm64_ftr_override __read_mostly id_aa64isar1_override;
+struct arm64_ftr_override __read_mostly id_aa64isar2_override;
+
+struct arm64_ftr_override __read_mostly arm64_sw_feature_override;
static const struct __ftr_reg_entry {
u32 sys_id;
--
2.49.0.987.g0cc8ee98dc-goog
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 3/3] arm64/boot: Disallow BSS exports to startup code
2025-05-08 11:43 [PATCH v2 0/3] arm64/boot: Forbid the use of BSS symbols in startup code Ard Biesheuvel
2025-05-08 11:43 ` [PATCH v2 1/3] arm64/boot: Move init_pgdir[] and init_idmap_pgdir[] into __pi_ namespace Ard Biesheuvel
2025-05-08 11:43 ` [PATCH v2 2/3] arm64/boot: Move global CPU override variables out of BSS Ard Biesheuvel
@ 2025-05-08 11:43 ` Ard Biesheuvel
2025-05-08 13:34 ` Yeoreum Yun
2025-05-09 17:10 ` [PATCH v2 0/3] arm64/boot: Forbid the use of BSS symbols in " Yeoreum Yun
2025-05-16 15:37 ` Will Deacon
4 siblings, 1 reply; 9+ messages in thread
From: Ard Biesheuvel @ 2025-05-08 11:43 UTC (permalink / raw)
To: linux-arm-kernel
Cc: linux-kernel, will, catalin.marinas, mark.rutland, Ard Biesheuvel,
Yeoreum Yun
From: Ard Biesheuvel <ardb@kernel.org>
BSS might be uninitialized when entering the startup code, so forbid the
use by the startup code of any variables that live after __bss_start in
the linker map.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
arch/arm64/kernel/image-vars.h | 62 +++++++++++---------
arch/arm64/kernel/vmlinux.lds.S | 2 +
2 files changed, 35 insertions(+), 29 deletions(-)
diff --git a/arch/arm64/kernel/image-vars.h b/arch/arm64/kernel/image-vars.h
index c3b4c0479d5c..a928e0c0b45a 100644
--- a/arch/arm64/kernel/image-vars.h
+++ b/arch/arm64/kernel/image-vars.h
@@ -10,6 +10,12 @@
#error This file should only be included in vmlinux.lds.S
#endif
+#define PI_EXPORT_SYM(sym) \
+ __PI_EXPORT_SYM(sym, __pi_ ## sym, Cannot export BSS symbol sym to startup code)
+#define __PI_EXPORT_SYM(sym, pisym, msg)\
+ PROVIDE(pisym = sym); \
+ ASSERT((sym - KIMAGE_VADDR) < (__bss_start - KIMAGE_VADDR), #msg)
+
PROVIDE(__efistub_primary_entry = primary_entry);
/*
@@ -36,37 +42,35 @@ PROVIDE(__pi___memcpy = __pi_memcpy);
PROVIDE(__pi___memmove = __pi_memmove);
PROVIDE(__pi___memset = __pi_memset);
-PROVIDE(__pi_id_aa64isar1_override = id_aa64isar1_override);
-PROVIDE(__pi_id_aa64isar2_override = id_aa64isar2_override);
-PROVIDE(__pi_id_aa64mmfr0_override = id_aa64mmfr0_override);
-PROVIDE(__pi_id_aa64mmfr1_override = id_aa64mmfr1_override);
-PROVIDE(__pi_id_aa64mmfr2_override = id_aa64mmfr2_override);
-PROVIDE(__pi_id_aa64pfr0_override = id_aa64pfr0_override);
-PROVIDE(__pi_id_aa64pfr1_override = id_aa64pfr1_override);
-PROVIDE(__pi_id_aa64smfr0_override = id_aa64smfr0_override);
-PROVIDE(__pi_id_aa64zfr0_override = id_aa64zfr0_override);
-PROVIDE(__pi_arm64_sw_feature_override = arm64_sw_feature_override);
-PROVIDE(__pi_arm64_use_ng_mappings = arm64_use_ng_mappings);
+PI_EXPORT_SYM(id_aa64isar1_override);
+PI_EXPORT_SYM(id_aa64isar2_override);
+PI_EXPORT_SYM(id_aa64mmfr0_override);
+PI_EXPORT_SYM(id_aa64mmfr1_override);
+PI_EXPORT_SYM(id_aa64mmfr2_override);
+PI_EXPORT_SYM(id_aa64pfr0_override);
+PI_EXPORT_SYM(id_aa64pfr1_override);
+PI_EXPORT_SYM(id_aa64smfr0_override);
+PI_EXPORT_SYM(id_aa64zfr0_override);
+PI_EXPORT_SYM(arm64_sw_feature_override);
+PI_EXPORT_SYM(arm64_use_ng_mappings);
#ifdef CONFIG_CAVIUM_ERRATUM_27456
-PROVIDE(__pi_cavium_erratum_27456_cpus = cavium_erratum_27456_cpus);
-PROVIDE(__pi_is_midr_in_range_list = is_midr_in_range_list);
+PI_EXPORT_SYM(cavium_erratum_27456_cpus);
+PI_EXPORT_SYM(is_midr_in_range_list);
#endif
-PROVIDE(__pi__ctype = _ctype);
-PROVIDE(__pi_memstart_offset_seed = memstart_offset_seed);
-
-PROVIDE(__pi_swapper_pg_dir = swapper_pg_dir);
-
-PROVIDE(__pi__text = _text);
-PROVIDE(__pi__stext = _stext);
-PROVIDE(__pi__etext = _etext);
-PROVIDE(__pi___start_rodata = __start_rodata);
-PROVIDE(__pi___inittext_begin = __inittext_begin);
-PROVIDE(__pi___inittext_end = __inittext_end);
-PROVIDE(__pi___initdata_begin = __initdata_begin);
-PROVIDE(__pi___initdata_end = __initdata_end);
-PROVIDE(__pi__data = _data);
-PROVIDE(__pi___bss_start = __bss_start);
-PROVIDE(__pi__end = _end);
+PI_EXPORT_SYM(_ctype);
+PI_EXPORT_SYM(memstart_offset_seed);
+
+PI_EXPORT_SYM(swapper_pg_dir);
+
+PI_EXPORT_SYM(_text);
+PI_EXPORT_SYM(_stext);
+PI_EXPORT_SYM(_etext);
+PI_EXPORT_SYM(__start_rodata);
+PI_EXPORT_SYM(__inittext_begin);
+PI_EXPORT_SYM(__inittext_end);
+PI_EXPORT_SYM(__initdata_begin);
+PI_EXPORT_SYM(__initdata_end);
+PI_EXPORT_SYM(_data);
#ifdef CONFIG_KVM
diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S
index 466544c47dca..e4a525a865c1 100644
--- a/arch/arm64/kernel/vmlinux.lds.S
+++ b/arch/arm64/kernel/vmlinux.lds.S
@@ -319,6 +319,7 @@ SECTIONS
/* start of zero-init region */
BSS_SECTION(SBSS_ALIGN, 0, 0)
+ __pi___bss_start = __bss_start;
. = ALIGN(PAGE_SIZE);
__pi_init_pg_dir = .;
@@ -332,6 +333,7 @@ SECTIONS
. = ALIGN(SEGMENT_ALIGN);
__pecoff_data_size = ABSOLUTE(. - __initdata_begin);
_end = .;
+ __pi__end = .;
STABS_DEBUG
DWARF_DEBUG
--
2.49.0.987.g0cc8ee98dc-goog
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 3/3] arm64/boot: Disallow BSS exports to startup code
2025-05-08 11:43 ` [PATCH v2 3/3] arm64/boot: Disallow BSS exports to startup code Ard Biesheuvel
@ 2025-05-08 13:34 ` Yeoreum Yun
2025-05-08 13:41 ` Yeoreum Yun
2025-05-09 6:43 ` Ard Biesheuvel
0 siblings, 2 replies; 9+ messages in thread
From: Yeoreum Yun @ 2025-05-08 13:34 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: linux-arm-kernel, linux-kernel, will, catalin.marinas,
mark.rutland, Ard Biesheuvel
Hi Ard,
> From: Ard Biesheuvel <ardb@kernel.org>
>
> BSS might be uninitialized when entering the startup code, so forbid the
> use by the startup code of any variables that live after __bss_start in
> the linker map.
>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
> arch/arm64/kernel/image-vars.h | 62 +++++++++++---------
> arch/arm64/kernel/vmlinux.lds.S | 2 +
> 2 files changed, 35 insertions(+), 29 deletions(-)
>
> diff --git a/arch/arm64/kernel/image-vars.h b/arch/arm64/kernel/image-vars.h
> index c3b4c0479d5c..a928e0c0b45a 100644
> --- a/arch/arm64/kernel/image-vars.h
> +++ b/arch/arm64/kernel/image-vars.h
> @@ -10,6 +10,12 @@
> #error This file should only be included in vmlinux.lds.S
> #endif
>
> +#define PI_EXPORT_SYM(sym) \
> + __PI_EXPORT_SYM(sym, __pi_ ## sym, Cannot export BSS symbol sym to startup code)
> +#define __PI_EXPORT_SYM(sym, pisym, msg)\
> + PROVIDE(pisym = sym); \
> + ASSERT((sym - KIMAGE_VADDR) < (__bss_start - KIMAGE_VADDR), #msg)
> +
> PROVIDE(__efistub_primary_entry = primary_entry);
>
> /*
> @@ -36,37 +42,35 @@ PROVIDE(__pi___memcpy = __pi_memcpy);
> PROVIDE(__pi___memmove = __pi_memmove);
> PROVIDE(__pi___memset = __pi_memset);
>
> -PROVIDE(__pi_id_aa64isar1_override = id_aa64isar1_override);
> -PROVIDE(__pi_id_aa64isar2_override = id_aa64isar2_override);
> -PROVIDE(__pi_id_aa64mmfr0_override = id_aa64mmfr0_override);
> -PROVIDE(__pi_id_aa64mmfr1_override = id_aa64mmfr1_override);
> -PROVIDE(__pi_id_aa64mmfr2_override = id_aa64mmfr2_override);
> -PROVIDE(__pi_id_aa64pfr0_override = id_aa64pfr0_override);
> -PROVIDE(__pi_id_aa64pfr1_override = id_aa64pfr1_override);
> -PROVIDE(__pi_id_aa64smfr0_override = id_aa64smfr0_override);
> -PROVIDE(__pi_id_aa64zfr0_override = id_aa64zfr0_override);
> -PROVIDE(__pi_arm64_sw_feature_override = arm64_sw_feature_override);
> -PROVIDE(__pi_arm64_use_ng_mappings = arm64_use_ng_mappings);
> +PI_EXPORT_SYM(id_aa64isar1_override);
> +PI_EXPORT_SYM(id_aa64isar2_override);
> +PI_EXPORT_SYM(id_aa64mmfr0_override);
> +PI_EXPORT_SYM(id_aa64mmfr1_override);
> +PI_EXPORT_SYM(id_aa64mmfr2_override);
> +PI_EXPORT_SYM(id_aa64pfr0_override);
> +PI_EXPORT_SYM(id_aa64pfr1_override);
> +PI_EXPORT_SYM(id_aa64smfr0_override);
> +PI_EXPORT_SYM(id_aa64zfr0_override);
> +PI_EXPORT_SYM(arm64_sw_feature_override);
> +PI_EXPORT_SYM(arm64_use_ng_mappings);
> #ifdef CONFIG_CAVIUM_ERRATUM_27456
> -PROVIDE(__pi_cavium_erratum_27456_cpus = cavium_erratum_27456_cpus);
> -PROVIDE(__pi_is_midr_in_range_list = is_midr_in_range_list);
> +PI_EXPORT_SYM(cavium_erratum_27456_cpus);
> +PI_EXPORT_SYM(is_midr_in_range_list);
small nit:
Would you rebase this patchset after
commit 117c3b21d3c7 ("arm64: Rework checks for broken Cavium HW in the PI code")?
Otherwise, I experience boot failure because of SCS related code:
ffff80008009fbc0 <is_midr_in_range_list>:
ffff80008009fbc0: d503245f bti c
ffff80008009fbc4: d503201f nop
ffff80008009fbc8: d503201f nop
ffff80008009fbcc: f800865e str x30, [x18], #0x8 ---- (1)
ffff80008009fbd0: d503233f paciasp
...
At pi phase, platform register initialized properly...
So it makes panic on (1).
Thanks!
> #endif
> -PROVIDE(__pi__ctype = _ctype);
> -PROVIDE(__pi_memstart_offset_seed = memstart_offset_seed);
> -
> -PROVIDE(__pi_swapper_pg_dir = swapper_pg_dir);
> -
> -PROVIDE(__pi__text = _text);
> -PROVIDE(__pi__stext = _stext);
> -PROVIDE(__pi__etext = _etext);
> -PROVIDE(__pi___start_rodata = __start_rodata);
> -PROVIDE(__pi___inittext_begin = __inittext_begin);
> -PROVIDE(__pi___inittext_end = __inittext_end);
> -PROVIDE(__pi___initdata_begin = __initdata_begin);
> -PROVIDE(__pi___initdata_end = __initdata_end);
> -PROVIDE(__pi__data = _data);
> -PROVIDE(__pi___bss_start = __bss_start);
> -PROVIDE(__pi__end = _end);
> +PI_EXPORT_SYM(_ctype);
> +PI_EXPORT_SYM(memstart_offset_seed);
> +
> +PI_EXPORT_SYM(swapper_pg_dir);
> +
> +PI_EXPORT_SYM(_text);
> +PI_EXPORT_SYM(_stext);
> +PI_EXPORT_SYM(_etext);
> +PI_EXPORT_SYM(__start_rodata);
> +PI_EXPORT_SYM(__inittext_begin);
> +PI_EXPORT_SYM(__inittext_end);
> +PI_EXPORT_SYM(__initdata_begin);
> +PI_EXPORT_SYM(__initdata_end);
> +PI_EXPORT_SYM(_data);
>
> #ifdef CONFIG_KVM
>
> diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S
> index 466544c47dca..e4a525a865c1 100644
> --- a/arch/arm64/kernel/vmlinux.lds.S
> +++ b/arch/arm64/kernel/vmlinux.lds.S
> @@ -319,6 +319,7 @@ SECTIONS
>
> /* start of zero-init region */
> BSS_SECTION(SBSS_ALIGN, 0, 0)
> + __pi___bss_start = __bss_start;
>
> . = ALIGN(PAGE_SIZE);
> __pi_init_pg_dir = .;
> @@ -332,6 +333,7 @@ SECTIONS
> . = ALIGN(SEGMENT_ALIGN);
> __pecoff_data_size = ABSOLUTE(. - __initdata_begin);
> _end = .;
> + __pi__end = .;
>
> STABS_DEBUG
> DWARF_DEBUG
> --
> 2.49.0.987.g0cc8ee98dc-goog
>
--
Sincerely,
Yeoreum Yun
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 3/3] arm64/boot: Disallow BSS exports to startup code
2025-05-08 13:34 ` Yeoreum Yun
@ 2025-05-08 13:41 ` Yeoreum Yun
2025-05-09 6:43 ` Ard Biesheuvel
1 sibling, 0 replies; 9+ messages in thread
From: Yeoreum Yun @ 2025-05-08 13:41 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: linux-arm-kernel, linux-kernel, will, catalin.marinas,
mark.rutland, Ard Biesheuvel
On Thu, May 08, 2025 at 02:34:12PM +0100, Yeoreum Yun wrote:
> Hi Ard,
>
> > From: Ard Biesheuvel <ardb@kernel.org>
> >
> > BSS might be uninitialized when entering the startup code, so forbid the
> > use by the startup code of any variables that live after __bss_start in
> > the linker map.
> >
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > ---
> > arch/arm64/kernel/image-vars.h | 62 +++++++++++---------
> > arch/arm64/kernel/vmlinux.lds.S | 2 +
> > 2 files changed, 35 insertions(+), 29 deletions(-)
> >
> > diff --git a/arch/arm64/kernel/image-vars.h b/arch/arm64/kernel/image-vars.h
> > index c3b4c0479d5c..a928e0c0b45a 100644
> > --- a/arch/arm64/kernel/image-vars.h
> > +++ b/arch/arm64/kernel/image-vars.h
> > @@ -10,6 +10,12 @@
> > #error This file should only be included in vmlinux.lds.S
> > #endif
> >
> > +#define PI_EXPORT_SYM(sym) \
> > + __PI_EXPORT_SYM(sym, __pi_ ## sym, Cannot export BSS symbol sym to startup code)
> > +#define __PI_EXPORT_SYM(sym, pisym, msg)\
> > + PROVIDE(pisym = sym); \
> > + ASSERT((sym - KIMAGE_VADDR) < (__bss_start - KIMAGE_VADDR), #msg)
> > +
> > PROVIDE(__efistub_primary_entry = primary_entry);
> >
> > /*
> > @@ -36,37 +42,35 @@ PROVIDE(__pi___memcpy = __pi_memcpy);
> > PROVIDE(__pi___memmove = __pi_memmove);
> > PROVIDE(__pi___memset = __pi_memset);
> >
> > -PROVIDE(__pi_id_aa64isar1_override = id_aa64isar1_override);
> > -PROVIDE(__pi_id_aa64isar2_override = id_aa64isar2_override);
> > -PROVIDE(__pi_id_aa64mmfr0_override = id_aa64mmfr0_override);
> > -PROVIDE(__pi_id_aa64mmfr1_override = id_aa64mmfr1_override);
> > -PROVIDE(__pi_id_aa64mmfr2_override = id_aa64mmfr2_override);
> > -PROVIDE(__pi_id_aa64pfr0_override = id_aa64pfr0_override);
> > -PROVIDE(__pi_id_aa64pfr1_override = id_aa64pfr1_override);
> > -PROVIDE(__pi_id_aa64smfr0_override = id_aa64smfr0_override);
> > -PROVIDE(__pi_id_aa64zfr0_override = id_aa64zfr0_override);
> > -PROVIDE(__pi_arm64_sw_feature_override = arm64_sw_feature_override);
> > -PROVIDE(__pi_arm64_use_ng_mappings = arm64_use_ng_mappings);
> > +PI_EXPORT_SYM(id_aa64isar1_override);
> > +PI_EXPORT_SYM(id_aa64isar2_override);
> > +PI_EXPORT_SYM(id_aa64mmfr0_override);
> > +PI_EXPORT_SYM(id_aa64mmfr1_override);
> > +PI_EXPORT_SYM(id_aa64mmfr2_override);
> > +PI_EXPORT_SYM(id_aa64pfr0_override);
> > +PI_EXPORT_SYM(id_aa64pfr1_override);
> > +PI_EXPORT_SYM(id_aa64smfr0_override);
> > +PI_EXPORT_SYM(id_aa64zfr0_override);
> > +PI_EXPORT_SYM(arm64_sw_feature_override);
> > +PI_EXPORT_SYM(arm64_use_ng_mappings);
> > #ifdef CONFIG_CAVIUM_ERRATUM_27456
> > -PROVIDE(__pi_cavium_erratum_27456_cpus = cavium_erratum_27456_cpus);
> > -PROVIDE(__pi_is_midr_in_range_list = is_midr_in_range_list);
> > +PI_EXPORT_SYM(cavium_erratum_27456_cpus);
> > +PI_EXPORT_SYM(is_midr_in_range_list);
>
> small nit:
> Would you rebase this patchset after
> commit 117c3b21d3c7 ("arm64: Rework checks for broken Cavium HW in the PI code")?
> Otherwise, I experience boot failure because of SCS related code:
>
> ffff80008009fbc0 <is_midr_in_range_list>:
> ffff80008009fbc0: d503245f bti c
> ffff80008009fbc4: d503201f nop
> ffff80008009fbc8: d503201f nop
> ffff80008009fbcc: f800865e str x30, [x18], #0x8 ---- (1)
> ffff80008009fbd0: d503233f paciasp
> ...
>
> At pi phase, platform register initialized properly...
> So it makes panic on (1).
Doesn't initialize properly...
Sorry for typo.
>
> Thanks!
>
>
> > #endif
> > -PROVIDE(__pi__ctype = _ctype);
> > -PROVIDE(__pi_memstart_offset_seed = memstart_offset_seed);
> > -
> > -PROVIDE(__pi_swapper_pg_dir = swapper_pg_dir);
> > -
> > -PROVIDE(__pi__text = _text);
> > -PROVIDE(__pi__stext = _stext);
> > -PROVIDE(__pi__etext = _etext);
> > -PROVIDE(__pi___start_rodata = __start_rodata);
> > -PROVIDE(__pi___inittext_begin = __inittext_begin);
> > -PROVIDE(__pi___inittext_end = __inittext_end);
> > -PROVIDE(__pi___initdata_begin = __initdata_begin);
> > -PROVIDE(__pi___initdata_end = __initdata_end);
> > -PROVIDE(__pi__data = _data);
> > -PROVIDE(__pi___bss_start = __bss_start);
> > -PROVIDE(__pi__end = _end);
> > +PI_EXPORT_SYM(_ctype);
> > +PI_EXPORT_SYM(memstart_offset_seed);
> > +
> > +PI_EXPORT_SYM(swapper_pg_dir);
> > +
> > +PI_EXPORT_SYM(_text);
> > +PI_EXPORT_SYM(_stext);
> > +PI_EXPORT_SYM(_etext);
> > +PI_EXPORT_SYM(__start_rodata);
> > +PI_EXPORT_SYM(__inittext_begin);
> > +PI_EXPORT_SYM(__inittext_end);
> > +PI_EXPORT_SYM(__initdata_begin);
> > +PI_EXPORT_SYM(__initdata_end);
> > +PI_EXPORT_SYM(_data);
> >
> > #ifdef CONFIG_KVM
> >
> > diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S
> > index 466544c47dca..e4a525a865c1 100644
> > --- a/arch/arm64/kernel/vmlinux.lds.S
> > +++ b/arch/arm64/kernel/vmlinux.lds.S
> > @@ -319,6 +319,7 @@ SECTIONS
> >
> > /* start of zero-init region */
> > BSS_SECTION(SBSS_ALIGN, 0, 0)
> > + __pi___bss_start = __bss_start;
> >
> > . = ALIGN(PAGE_SIZE);
> > __pi_init_pg_dir = .;
> > @@ -332,6 +333,7 @@ SECTIONS
> > . = ALIGN(SEGMENT_ALIGN);
> > __pecoff_data_size = ABSOLUTE(. - __initdata_begin);
> > _end = .;
> > + __pi__end = .;
> >
> > STABS_DEBUG
> > DWARF_DEBUG
> > --
> > 2.49.0.987.g0cc8ee98dc-goog
> >
>
> --
> Sincerely,
> Yeoreum Yun
>
--
Sincerely,
Yeoreum Yun
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 3/3] arm64/boot: Disallow BSS exports to startup code
2025-05-08 13:34 ` Yeoreum Yun
2025-05-08 13:41 ` Yeoreum Yun
@ 2025-05-09 6:43 ` Ard Biesheuvel
1 sibling, 0 replies; 9+ messages in thread
From: Ard Biesheuvel @ 2025-05-09 6:43 UTC (permalink / raw)
To: Yeoreum Yun
Cc: Ard Biesheuvel, linux-arm-kernel, linux-kernel, will,
catalin.marinas, mark.rutland
On Thu, 8 May 2025 at 15:34, Yeoreum Yun <yeoreum.yun@arm.com> wrote:
>
> Hi Ard,
>
> > From: Ard Biesheuvel <ardb@kernel.org>
> >
> > BSS might be uninitialized when entering the startup code, so forbid the
> > use by the startup code of any variables that live after __bss_start in
> > the linker map.
> >
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > ---
> > arch/arm64/kernel/image-vars.h | 62 +++++++++++---------
> > arch/arm64/kernel/vmlinux.lds.S | 2 +
> > 2 files changed, 35 insertions(+), 29 deletions(-)
> >
> > diff --git a/arch/arm64/kernel/image-vars.h b/arch/arm64/kernel/image-vars.h
> > index c3b4c0479d5c..a928e0c0b45a 100644
> > --- a/arch/arm64/kernel/image-vars.h
> > +++ b/arch/arm64/kernel/image-vars.h
> > @@ -10,6 +10,12 @@
> > #error This file should only be included in vmlinux.lds.S
> > #endif
> >
> > +#define PI_EXPORT_SYM(sym) \
> > + __PI_EXPORT_SYM(sym, __pi_ ## sym, Cannot export BSS symbol sym to startup code)
> > +#define __PI_EXPORT_SYM(sym, pisym, msg)\
> > + PROVIDE(pisym = sym); \
> > + ASSERT((sym - KIMAGE_VADDR) < (__bss_start - KIMAGE_VADDR), #msg)
> > +
> > PROVIDE(__efistub_primary_entry = primary_entry);
> >
> > /*
> > @@ -36,37 +42,35 @@ PROVIDE(__pi___memcpy = __pi_memcpy);
> > PROVIDE(__pi___memmove = __pi_memmove);
> > PROVIDE(__pi___memset = __pi_memset);
> >
> > -PROVIDE(__pi_id_aa64isar1_override = id_aa64isar1_override);
> > -PROVIDE(__pi_id_aa64isar2_override = id_aa64isar2_override);
> > -PROVIDE(__pi_id_aa64mmfr0_override = id_aa64mmfr0_override);
> > -PROVIDE(__pi_id_aa64mmfr1_override = id_aa64mmfr1_override);
> > -PROVIDE(__pi_id_aa64mmfr2_override = id_aa64mmfr2_override);
> > -PROVIDE(__pi_id_aa64pfr0_override = id_aa64pfr0_override);
> > -PROVIDE(__pi_id_aa64pfr1_override = id_aa64pfr1_override);
> > -PROVIDE(__pi_id_aa64smfr0_override = id_aa64smfr0_override);
> > -PROVIDE(__pi_id_aa64zfr0_override = id_aa64zfr0_override);
> > -PROVIDE(__pi_arm64_sw_feature_override = arm64_sw_feature_override);
> > -PROVIDE(__pi_arm64_use_ng_mappings = arm64_use_ng_mappings);
> > +PI_EXPORT_SYM(id_aa64isar1_override);
> > +PI_EXPORT_SYM(id_aa64isar2_override);
> > +PI_EXPORT_SYM(id_aa64mmfr0_override);
> > +PI_EXPORT_SYM(id_aa64mmfr1_override);
> > +PI_EXPORT_SYM(id_aa64mmfr2_override);
> > +PI_EXPORT_SYM(id_aa64pfr0_override);
> > +PI_EXPORT_SYM(id_aa64pfr1_override);
> > +PI_EXPORT_SYM(id_aa64smfr0_override);
> > +PI_EXPORT_SYM(id_aa64zfr0_override);
> > +PI_EXPORT_SYM(arm64_sw_feature_override);
> > +PI_EXPORT_SYM(arm64_use_ng_mappings);
> > #ifdef CONFIG_CAVIUM_ERRATUM_27456
> > -PROVIDE(__pi_cavium_erratum_27456_cpus = cavium_erratum_27456_cpus);
> > -PROVIDE(__pi_is_midr_in_range_list = is_midr_in_range_list);
> > +PI_EXPORT_SYM(cavium_erratum_27456_cpus);
> > +PI_EXPORT_SYM(is_midr_in_range_list);
>
> small nit:
> Would you rebase this patchset after
> commit 117c3b21d3c7 ("arm64: Rework checks for broken Cavium HW in the PI code")?
> Otherwise, I experience boot failure because of SCS related code:
>
> ffff80008009fbc0 <is_midr_in_range_list>:
> ffff80008009fbc0: d503245f bti c
> ffff80008009fbc4: d503201f nop
> ffff80008009fbc8: d503201f nop
> ffff80008009fbcc: f800865e str x30, [x18], #0x8 ---- (1)
> ffff80008009fbd0: d503233f paciasp
> ...
>
> At pi phase, platform register initialized properly...
> So it makes panic on (1).
>
That commit is not in the arm64 tree, so it will be up to Marc and
Catalin to resolve the conflict.
Your commit
363cd2b81cfd arm64: cpufeature: Move arm64_use_ng_mappings to the
.data section to prevent wrong idmap generation
is not in the kvmarm tree, so the build will not even complete if you
rebase it on that.
For testing, please merge the kvmarm tree and resolve the conflict by hand.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 0/3] arm64/boot: Forbid the use of BSS symbols in startup code
2025-05-08 11:43 [PATCH v2 0/3] arm64/boot: Forbid the use of BSS symbols in startup code Ard Biesheuvel
` (2 preceding siblings ...)
2025-05-08 11:43 ` [PATCH v2 3/3] arm64/boot: Disallow BSS exports to startup code Ard Biesheuvel
@ 2025-05-09 17:10 ` Yeoreum Yun
2025-05-16 15:37 ` Will Deacon
4 siblings, 0 replies; 9+ messages in thread
From: Yeoreum Yun @ 2025-05-09 17:10 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: linux-arm-kernel, linux-kernel, will, catalin.marinas,
mark.rutland, Ard Biesheuvel
Hi Ard,
> From: Ard Biesheuvel <ardb@kernel.org>
>
> Move any variables accessed or assigned by the startup code out of BSS,
> and into .data, so that we can forbid the use of BSS variables
> altogether, by ASSERT()'ing in the linker script that each symbol made
> available to the startup code lives before __bss_start in the linker
> map.
>
> Changes since v1:
> - fix build error due to missing declaration in #1
> - work around Clang complaining about the ASSERT() expression in the
> linker script
>
> Cc: Yeoreum Yun <yeoreum.yun@arm.com>
Sorry for late.
with commit 117c3b21d3c7 ("arm64: Rework checks for broken Cavium HW in the PI code"):
Tested-by: Yeoreum Yun <yeoreum.yun@arm.com>
Reviewed-by: Yeoreum Yun <yeoreum.yun@arm.com>
Thanks!
>
> Ard Biesheuvel (3):
> arm64/boot: Move init_pgdir[] and init_idmap_pgdir[] into __pi_
> namespace
> arm64/boot: Move global CPU override variables out of BSS
> arm64/boot: Disallow BSS exports to startup code
>
> arch/arm64/include/asm/pgtable.h | 2 -
> arch/arm64/kernel/cpufeature.c | 22 +++----
> arch/arm64/kernel/head.S | 6 +-
> arch/arm64/kernel/image-vars.h | 66 ++++++++++----------
> arch/arm64/kernel/pi/pi.h | 1 +
> arch/arm64/kernel/vmlinux.lds.S | 10 +--
> 6 files changed, 54 insertions(+), 53 deletions(-)
>
>
> base-commit: 363cd2b81cfdf706bbfc9ec78db000c9b1ecc552
> --
> 2.49.0.987.g0cc8ee98dc-goog
>
--
Sincerely,
Yeoreum Yun
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 0/3] arm64/boot: Forbid the use of BSS symbols in startup code
2025-05-08 11:43 [PATCH v2 0/3] arm64/boot: Forbid the use of BSS symbols in startup code Ard Biesheuvel
` (3 preceding siblings ...)
2025-05-09 17:10 ` [PATCH v2 0/3] arm64/boot: Forbid the use of BSS symbols in " Yeoreum Yun
@ 2025-05-16 15:37 ` Will Deacon
4 siblings, 0 replies; 9+ messages in thread
From: Will Deacon @ 2025-05-16 15:37 UTC (permalink / raw)
To: linux-arm-kernel, Ard Biesheuvel
Cc: catalin.marinas, kernel-team, Will Deacon, linux-kernel,
mark.rutland, Ard Biesheuvel, Yeoreum Yun
On Thu, 08 May 2025 13:43:29 +0200, Ard Biesheuvel wrote:
> Move any variables accessed or assigned by the startup code out of BSS,
> and into .data, so that we can forbid the use of BSS variables
> altogether, by ASSERT()'ing in the linker script that each symbol made
> available to the startup code lives before __bss_start in the linker
> map.
>
> Changes since v1:
> - fix build error due to missing declaration in #1
> - work around Clang complaining about the ASSERT() expression in the
> linker script
>
> [...]
Applied to arm64 (for-next/mm), thanks!
[1/3] arm64/boot: Move init_pgdir[] and init_idmap_pgdir[] into __pi_ namespace
https://git.kernel.org/arm64/c/93d0d6f8a654
[2/3] arm64/boot: Move global CPU override variables out of BSS
https://git.kernel.org/arm64/c/4afff6cc9a55
[3/3] arm64/boot: Disallow BSS exports to startup code
https://git.kernel.org/arm64/c/90530521079e
Cheers,
--
Will
https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2025-05-16 16:02 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-08 11:43 [PATCH v2 0/3] arm64/boot: Forbid the use of BSS symbols in startup code Ard Biesheuvel
2025-05-08 11:43 ` [PATCH v2 1/3] arm64/boot: Move init_pgdir[] and init_idmap_pgdir[] into __pi_ namespace Ard Biesheuvel
2025-05-08 11:43 ` [PATCH v2 2/3] arm64/boot: Move global CPU override variables out of BSS Ard Biesheuvel
2025-05-08 11:43 ` [PATCH v2 3/3] arm64/boot: Disallow BSS exports to startup code Ard Biesheuvel
2025-05-08 13:34 ` Yeoreum Yun
2025-05-08 13:41 ` Yeoreum Yun
2025-05-09 6:43 ` Ard Biesheuvel
2025-05-09 17:10 ` [PATCH v2 0/3] arm64/boot: Forbid the use of BSS symbols in " Yeoreum Yun
2025-05-16 15:37 ` Will Deacon
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).