symbols: Change implementation details of DECLARE_OPTIONAL_REGION()

It seems that GCC's LTO doesn't like the way we implement
DECLARE_OPTIONAL_REGION(). This patch changes it so that rather than
having a normal DECLARE_REGION() in <symbols.h> and then an extra
DECLARE_OPTIONAL_REGION() in the C file using it, you just say
DECLARE_OPTIONAL_REGION() directly in <symbols.h> (in place and instead
of the usual DECLARE_REGION()). This basically looks the same way in the
resulting object file but somehow LTO seems to like it better.

Signed-off-by: Julius Werner <jwerner@chromium.org>
Change-Id: I6096207b311d70c8e9956cd9406bec45be04a4a2
Reviewed-on: https://review.coreboot.org/c/coreboot/+/44791
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Jacob Garber <jgarber1@ualberta.ca>
Reviewed-by: Hung-Te Lin <hungte@chromium.org>
Reviewed-by: HAOUAS Elyes <ehaouas@noos.fr>

10 files changed, 17 insertions, 31 deletions

diff --git a/src/arch/arm/armv7/mmu.c b/src/arch/arm/armv7/mmu.c
index 51b4860768..66ce53392d 100644
--- a/src/arch/arm/armv7/mmu.c
+++ b/src/arch/arm/armv7/mmu.c
@@ -87,9 +87,6 @@ typedef uint32_t pte_t;
 
 static pte_t *const ttb_buff = (void *)_ttb;
 
-/* Not all boards want to use subtables and declare them in memlayout.ld. */
-DECLARE_OPTIONAL_REGION(ttb_subtables);
-
 static struct {
 	pte_t value;
 	const char *name;
diff --git a/src/arch/arm/tables.c b/src/arch/arm/tables.c
index 3b47a5bf0c..0c68fc7c51 100644
--- a/src/arch/arm/tables.c
+++ b/src/arch/arm/tables.c
@@ -11,8 +11,6 @@ void arch_write_tables(uintptr_t coreboot_table)
 
 void bootmem_arch_add_ranges(void)
 {
-	DECLARE_OPTIONAL_REGION(ttb_subtables);
-
 	bootmem_add_range((uintptr_t)_ttb, REGION_SIZE(ttb), BM_MEM_RAMSTAGE);
 	bootmem_add_range((uintptr_t)_ttb_subtables, REGION_SIZE(ttb_subtables),
 			  BM_MEM_RAMSTAGE);
diff --git a/src/arch/arm64/tables.c b/src/arch/arm64/tables.c
index 321d348602..b97297c1b9 100644
--- a/src/arch/arm64/tables.c
+++ b/src/arch/arm64/tables.c
@@ -5,8 +5,6 @@
 #include <boot/coreboot_tables.h>
 #include <symbols.h>
 
-DECLARE_OPTIONAL_REGION(bl31);
-
 void arch_write_tables(uintptr_t coreboot_table)
 {
 }
diff --git a/src/arch/riscv/tables.c b/src/arch/riscv/tables.c
index 4935ef5ece..9fc75f455c 100644
--- a/src/arch/riscv/tables.c
+++ b/src/arch/riscv/tables.c
@@ -5,8 +5,6 @@
 #include <boot/coreboot_tables.h>
 #include <symbols.h>
 
-DECLARE_OPTIONAL_REGION(opensbi);
-
 void arch_write_tables(uintptr_t coreboot_table)
 {
 }
diff --git a/src/include/symbols.h b/src/include/symbols.h
index fe3f46ab80..371d84bf9b 100644
--- a/src/include/symbols.h
+++ b/src/include/symbols.h
@@ -13,8 +13,20 @@ extern u8 _dram[];
 	extern u8 _##name[];	\
 	extern u8 _e##name[];
 
+/*
+ * Regions can be declared optional if not all configurations provide them in
+ * memlayout and you want code to be able to check for their existence at
+ * runtime. Not every region that is architecture or platform-specific should
+ * use this -- only declare regions optional if the code *accessing* them runs
+ * both on configurations that have the region and those that don't.  That code
+ * should then check (REGION_SIZE(name) != 0) before accessing it.
+ */
+#define DECLARE_OPTIONAL_REGION(name)	\
+	__weak extern u8 _##name[];	\
+	__weak extern u8 _e##name[];
+
 DECLARE_REGION(sram)
-DECLARE_REGION(timestamp)
+DECLARE_OPTIONAL_REGION(timestamp)
 DECLARE_REGION(preram_cbmem_console)
 DECLARE_REGION(cbmem_init_hooks)
 DECLARE_REGION(stack)
@@ -53,24 +65,15 @@ DECLARE_REGION(ramstage)
 
 DECLARE_REGION(pagetables)
 DECLARE_REGION(ttb)
-DECLARE_REGION(ttb_subtables)
+DECLARE_OPTIONAL_REGION(ttb_subtables)
 DECLARE_REGION(dma_coherent)
 DECLARE_REGION(soc_registers)
 DECLARE_REGION(framebuffer)
 DECLARE_REGION(pdpt)
-DECLARE_REGION(opensbi)
-DECLARE_REGION(bl31)
+DECLARE_OPTIONAL_REGION(opensbi)
+DECLARE_OPTIONAL_REGION(bl31)
 DECLARE_REGION(transfer_buffer)
 
-/*
- * Put this into a .c file accessing a linker script region to mark that region
- * as "optional". If it is defined in memlayout.ld (or anywhere else), the
- * values from that definition will be used. If not, start, end and size will
- * all evaluate to 0. (We can't explicitly assign the symbols to 0 in the
- * assembly due to https://sourceware.org/bugzilla/show_bug.cgi?id=1038.)
- */
-#define DECLARE_OPTIONAL_REGION(name) asm (".weak _" #name ", _e" #name)
-
 /* Returns true when pre-RAM symbols are known to the linker.
  * (Does not necessarily mean that the memory is accessible.) */
 static inline int preram_symbols_available(void)
diff --git a/src/lib/bootblock.c b/src/lib/bootblock.c
index 1c433d008e..7ee246e459 100644
--- a/src/lib/bootblock.c
+++ b/src/lib/bootblock.c
@@ -10,8 +10,6 @@
 #include <symbols.h>
 #include <timestamp.h>
 
-DECLARE_OPTIONAL_REGION(timestamp);
-
 __weak void bootblock_mainboard_early_init(void) { /* no-op */ }
 __weak void bootblock_soc_early_init(void) { /* do nothing */ }
 __weak void bootblock_soc_init(void) { /* do nothing */ }
diff --git a/src/lib/timestamp.c b/src/lib/timestamp.c
index 5121eb859c..7347d07b16 100644
--- a/src/lib/timestamp.c
+++ b/src/lib/timestamp.c
@@ -11,8 +11,6 @@
 
 #define MAX_TIMESTAMPS 192
 
-DECLARE_OPTIONAL_REGION(timestamp);
-
 /* This points to the active timestamp_table and can change within a stage
    as CBMEM comes available. */
 static struct timestamp_table *glob_ts_table;
diff --git a/src/soc/cavium/common/bootblock.c b/src/soc/cavium/common/bootblock.c
index 80a41fa375..2fca0abbac 100644
--- a/src/soc/cavium/common/bootblock.c
+++ b/src/soc/cavium/common/bootblock.c
@@ -8,8 +8,6 @@
 #include <timestamp.h>
 #include <soc/bootblock.h>
 
-DECLARE_OPTIONAL_REGION(timestamp);
-
 __attribute__((weak)) void bootblock_mainboard_early_init(void) { /* no-op */ }
 __attribute__((weak)) void bootblock_soc_early_init(void) { /* do nothing */ }
 __attribute__((weak)) void bootblock_soc_init(void) { /* do nothing */ }
diff --git a/src/vendorcode/google/chromeos/symbols.h b/src/vendorcode/google/chromeos/symbols.h
index 1e966da338..45f9c3e10b 100644
--- a/src/vendorcode/google/chromeos/symbols.h
+++ b/src/vendorcode/google/chromeos/symbols.h
@@ -5,6 +5,6 @@
 
 #include <symbols.h>
 
-DECLARE_REGION(watchdog_tombstone)
+DECLARE_OPTIONAL_REGION(watchdog_tombstone)
 
 #endif /* __CHROMEOS_SYMBOLS_H */
diff --git a/src/vendorcode/google/chromeos/watchdog.c b/src/vendorcode/google/chromeos/watchdog.c
index 5b3f1844a7..d3273cb442 100644
--- a/src/vendorcode/google/chromeos/watchdog.c
+++ b/src/vendorcode/google/chromeos/watchdog.c
@@ -14,8 +14,6 @@
 
 #define WATCHDOG_TOMBSTONE_MAGIC	0x9d2f41a7
 
-DECLARE_OPTIONAL_REGION(watchdog_tombstone);
-
 static void elog_handle_watchdog_tombstone(void *unused)
 {
 	bool flag = false;