[PowerPC] More relocation fixes

It turns out relocating the symbol table itself can cause issues, like fbt
crashing because it applies the offsets to the kernel twice.

This had been previously brought up in rS333447 when the stoffs hack was
added, but I had been unaware of this and reimplemented symtab relocation.

Instead of relocating the symbol table, keep track of the relocation base
in ddb, so the ddb symbols behave like the kernel linker-provided symbols.

This is intended to be NFC on platforms other than PowerPC, which do not
use fully relocatable kernels. (The relbase will always be 0)

 * Remove the rest of the stoffs hack.
 * Remove my half-baked displace_symbol_table() function.
 * Extend ddb initialization to cope with having a relocation offset on the
   kernel symbol table.
 * Fix my kernel-as-initrd hack to work with booke64 by using a temporary
   mapping to access the data.
 * Fix another instance of __powerpc__ that is actually RELOCATABLE_KERNEL.
 * Change the behavior or X_db_symbol_values to apply the relocation base
   when updating valp, to match link_elf_symbol_values() behavior.

Reviewed by:	jhibbits
Sponsored by:	Tag1 Consulting, Inc.
Differential Revision:	https://reviews.freebsd.org/D25223

sys/amd64/amd64/machdep.c

@@ -1508,7 +1508,7 @@ native_parse_preload_data(u_int64_t modulep)
 #ifdef DDB
 	ksym_start = MD_FETCH(kmdp, MODINFOMD_SSYM, uintptr_t);
 	ksym_end = MD_FETCH(kmdp, MODINFOMD_ESYM, uintptr_t);
-	db_fetch_ksymtab(ksym_start, ksym_end);
+	db_fetch_ksymtab(ksym_start, ksym_end, 0);
 #endif
 	efi_systbl_phys = MD_FETCH(kmdp, MODINFOMD_FW_HANDLE, vm_paddr_t);
 

sys/arm/arm/machdep_boot.c

@@ -302,7 +302,7 @@ freebsd_parse_boot_param(struct arm_boot_params *abp)
 #ifdef DDB
 	ksym_start = MD_FETCH(kmdp, MODINFOMD_SSYM, uintptr_t);
 	ksym_end = MD_FETCH(kmdp, MODINFOMD_ESYM, uintptr_t);
-	db_fetch_ksymtab(ksym_start, ksym_end);
+	db_fetch_ksymtab(ksym_start, ksym_end, 0);
 #endif
 	return lastaddr;
 }

sys/arm64/arm64/machdep_boot.c

@@ -208,7 +208,7 @@ freebsd_parse_boot_param(struct arm64_bootparams *abp)
 #ifdef DDB
 	ksym_start = MD_FETCH(kmdp, MODINFOMD_SSYM, uintptr_t);
 	ksym_end = MD_FETCH(kmdp, MODINFOMD_ESYM, uintptr_t);
-	db_fetch_ksymtab(ksym_start, ksym_end);
+	db_fetch_ksymtab(ksym_start, ksym_end, 0);
 #endif
 	return (lastaddr);
 }

sys/ddb/db_main.c

@@ -48,6 +48,14 @@ __FBSDID("$FreeBSD$");
 #include <ddb/db_command.h>
 #include <ddb/db_sym.h>
 
+struct db_private {
+	char*		strtab;
+	vm_offset_t	relbase;
+};
+typedef struct db_private *db_private_t;
+
+#define DB_PRIVATE(x) ((db_private_t)(x->private))
+
 SYSCTL_NODE(_debug, OID_AUTO, ddb, CTLFLAG_RW | CTLFLAG_MPSAFE, 0,
     "DDB settings");
 
@@ -64,7 +72,8 @@ KDB_BACKEND(ddb, db_init, db_trace_self_wrapper, db_trace_thread_wrapper,
  * the symtab and strtab in memory. This is used when loaded from
  * boot loaders different than the native one (like Xen).
  */
-vm_offset_t ksymtab, kstrtab, ksymtab_size;
+vm_offset_t ksymtab, kstrtab, ksymtab_size, ksymtab_relbase;
+static struct db_private ksymtab_private;
 
 bool
 X_db_line_at_pc(db_symtab_t *symtab, c_db_sym_t sym, char **file, int *line,
@@ -86,7 +95,8 @@ X_db_lookup(db_symtab_t *symtab, const char *symbol)
 		sym = (Elf_Sym *)symtab->start;
 		while ((char *)sym < symtab->end) {
 			if (sym->st_name != 0 &&
-			    !strcmp(symtab->private + sym->st_name, symbol))
+			    !strcmp(DB_PRIVATE(symtab)->strtab +
+			    sym->st_name, symbol))
 				return ((c_db_sym_t)sym);
 			sym++;
 		}
@@ -101,7 +111,7 @@ X_db_search_symbol(db_symtab_t *symtab, db_addr_t off, db_strategy_t strat,
 	c_linker_sym_t lsym;
 	Elf_Sym *sym, *match;
 	unsigned long diff;
-	db_addr_t stoffs;
+	db_addr_t stoffs = off;
 
 	if (symtab->private == NULL) {
 		if (!linker_ddb_search_symbol((caddr_t)off, &lsym, &diff)) {
@@ -110,10 +120,11 @@ X_db_search_symbol(db_symtab_t *symtab, db_addr_t off, db_strategy_t strat,
 		}
 		return (NULL);
 	}
+	else
+		stoffs -= DB_PRIVATE(symtab)->relbase;
 
 	diff = ~0UL;
 	match = NULL;
-	stoffs = DB_STOFFS(off);
 	for (sym = (Elf_Sym*)symtab->start; (char*)sym < symtab->end; sym++) {
 		if (sym->st_name == 0 || sym->st_shndx == SHN_UNDEF)
 			continue;
@@ -171,15 +182,17 @@ X_db_symbol_values(db_symtab_t *symtab, c_db_sym_t sym, const char **namep,
 			*valp = (db_expr_t)lval.value;
 	} else {
 		if (namep != NULL)
-			*namep = (const char *)symtab->private +
+			*namep = (const char *)DB_PRIVATE(symtab)->strtab +
 			    ((const Elf_Sym *)sym)->st_name;
 		if (valp != NULL)
-			*valp = (db_expr_t)((const Elf_Sym *)sym)->st_value;
+			*valp = (db_expr_t)((const Elf_Sym *)sym)->st_value +
+			    DB_PRIVATE(symtab)->relbase;
 	}
 }
 
 int
-db_fetch_ksymtab(vm_offset_t ksym_start, vm_offset_t ksym_end)
+db_fetch_ksymtab(vm_offset_t ksym_start, vm_offset_t ksym_end,
+    vm_offset_t relbase)
 {
 	Elf_Size strsz;
 
@@ -190,9 +203,11 @@ db_fetch_ksymtab(vm_offset_t ksym_start, vm_offset_t ksym_end)
 		kstrtab = ksymtab + ksymtab_size;
 		strsz = *(Elf_Size*)kstrtab;
 		kstrtab += sizeof(Elf_Size);
+		ksymtab_relbase = relbase;
 		if (kstrtab + strsz > ksym_end) {
 			/* Sizes doesn't match, unset everything. */
-			ksymtab = ksymtab_size = kstrtab = 0;
+			ksymtab = ksymtab_size = kstrtab = ksymtab_relbase
+			    = 0;
 		}
 	}
 
@@ -209,8 +224,10 @@ db_init(void)
 	db_command_init();
 
 	if (ksymtab != 0 && kstrtab != 0 && ksymtab_size != 0) {
+		ksymtab_private.strtab = (char *)kstrtab;
+		ksymtab_private.relbase = ksymtab_relbase;
 		db_add_symbol_table((char *)ksymtab,
-		    (char *)(ksymtab + ksymtab_size), "elf", (char *)kstrtab);
+		    (char *)(ksymtab + ksymtab_size), "elf", (char *)&ksymtab_private);
 	}
 	db_add_symbol_table(NULL, NULL, "kld", NULL);
 	return (1);	/* We're the default debugger. */

sys/ddb/ddb.h

@@ -72,10 +72,6 @@ SYSCTL_DECL(_debug_ddb);
 #define	DB_MAXSCRIPTRECURSION	3
 #endif
 
-#ifndef DB_STOFFS
-#define DB_STOFFS(offs)		(offs)
-#endif
-
 #ifndef DB_CALL
 #define	DB_CALL	db_fncall_generic
 #else
@@ -87,7 +83,7 @@ int	DB_CALL(db_expr_t, db_expr_t *, int, db_expr_t[]);
  * Most users should use db_fetch_symtab in order to set them from the
  * boot loader provided values.
  */
-extern vm_offset_t ksymtab, kstrtab, ksymtab_size;
+extern vm_offset_t ksymtab, kstrtab, ksymtab_size, ksymtab_relbase;
 
 /*
  * There are three "command tables":
@@ -232,7 +228,8 @@ bool		db_value_of_name_vnet(const char *name, db_expr_t *valuep);
 int		db_write_bytes(vm_offset_t addr, size_t size, char *data);
 void		db_command_register(struct command_table *, struct command *);
 void		db_command_unregister(struct command_table *, struct command *);
-int		db_fetch_ksymtab(vm_offset_t ksym_start, vm_offset_t ksym_end);
+int		db_fetch_ksymtab(vm_offset_t ksym_start, vm_offset_t ksym_end,
+		    vm_offset_t relbase);
 
 db_cmdfcn_t	db_breakpoint_cmd;
 db_cmdfcn_t	db_capture_cmd;

sys/dev/ksyms/ksyms.c

@@ -202,7 +202,7 @@ ksyms_add(linker_file_t lf, void *arg)
 	strsz = LINKER_STRTAB_GET(lf, &strtab);
 	symsz = numsyms * sizeof(Elf_Sym);
 
-#ifdef __powerpc__
+#ifdef RELOCATABLE_KERNEL
 	fixup = true;
 #else
 	fixup = lf->id > 1;

sys/i386/i386/machdep.c

@@ -2180,7 +2180,7 @@ static void
 i386_kdb_init(void)
 {
 #ifdef DDB
-	db_fetch_ksymtab(bootinfo.bi_symtab, bootinfo.bi_esymtab);
+	db_fetch_ksymtab(bootinfo.bi_symtab, bootinfo.bi_esymtab, 0);
 #endif
 	kdb_init();
 #ifdef KDB

sys/kern/link_elf.c

@@ -389,6 +389,21 @@ link_elf_link_common_finish(linker_file_t lf)
 }
 
 #ifdef RELOCATABLE_KERNEL
+/*
+ * __startkernel and __endkernel are symbols set up as relocation canaries.
+ *
+ * They are defined in locore to reference linker script symbols at the
+ * beginning and end of the LOAD area. This has the desired side effect of
+ * giving us variables that have relative relocations pointing at them, so
+ * relocation of the kernel object will cause the variables to be updated
+ * automatically by the runtime linker when we initialize.
+ *
+ * There are two main reasons to relocate the kernel:
+ * 1) If the loader needed to load the kernel at an alternate load address.
+ * 2) If the kernel is switching address spaces on machines like POWER9
+ *    under Radix where the high bits of the effective address are used to
+ *    differentiate between hypervisor, host, guest, and problem state.
+ */
 extern vm_offset_t __startkernel, __endkernel;
 #endif
 
@@ -427,6 +442,7 @@ link_elf_init(void* arg)
 	ef = (elf_file_t) linker_kernel_file;
 	ef->preloaded = 1;
 #ifdef RELOCATABLE_KERNEL
+	/* Compute relative displacement */
 	ef->address = (caddr_t) (__startkernel - KERNBASE);
 #else
 	ef->address = 0;

sys/mips/mips/machdep.c

@@ -447,7 +447,7 @@ mips_postboot_fixup(void)
 		kernel_kseg0_end += symtabsize;
 		/* end of .strtab */
 		ksym_end = kernel_kseg0_end;
-		db_fetch_ksymtab(ksym_start, ksym_end);
+		db_fetch_ksymtab(ksym_start, ksym_end, 0);
 	}
 #endif
 }

sys/powerpc/powerpc/machdep.c

@@ -251,7 +251,6 @@ void booke_cpu_init(void);
 
 #ifdef DDB
 static void	load_external_symtab(void);
-static void	displace_symbol_table(vm_offset_t, vm_offset_t, vm_offset_t);
 #endif
 
 uintptr_t
@@ -360,16 +359,8 @@ powerpc_init(vm_offset_t fdt, vm_offset_t toc, vm_offset_t ofentry, void *mdp,
 			ksym_end = MD_FETCH(kmdp, MODINFOMD_ESYM, uintptr_t);
 			ksym_sz = *(Elf_Size*)ksym_start;
 
-			/*
-			 * Loader already handled displacing to the load
-			 * address, but we still need to displace it to the
-			 * DMAP.
-			 */
-			displace_symbol_table(
-			    (vm_offset_t)(ksym_start + sizeof(Elf_Size)),
-			    ksym_sz, md_offset);
-
-			db_fetch_ksymtab(ksym_start, ksym_end);
+			db_fetch_ksymtab(ksym_start, ksym_end, md_offset);
+			/* Symbols provided by loader. */
 			symbols_provided = true;
 #endif
 		}
@@ -509,37 +500,13 @@ powerpc_init(vm_offset_t fdt, vm_offset_t toc, vm_offset_t ofentry, void *mdp,
 
 #ifdef DDB
 /*
- * XXX Figure out where to move this.
- */
-static void
-displace_symbol_table(vm_offset_t ksym_start,
-    vm_offset_t ksym_sz, vm_offset_t displacement) {
-	Elf_Sym *sym;
-
-	/*
-	 * Relocate the symbol table to our final load address.
-	 */
-	for (sym = (Elf_Sym *)ksym_start;
-	    (vm_paddr_t)sym < (ksym_start + ksym_sz);
-	    sym++) {
-		if (sym->st_name == 0 ||
-		    sym->st_shndx == SHN_UNDEF ||
-		    sym->st_value == 0)
-			continue;
-		if (ELF_ST_TYPE(sym->st_info) != STT_OBJECT &&
-		    ELF_ST_TYPE(sym->st_info) != STT_FUNC &&
-		    ELF_ST_TYPE(sym->st_info) != STT_NOTYPE)
-			continue;
-		/* Skip relocating any implausible symbols */
-		if (sym->st_value > KERNBASE)
-			sym->st_value += displacement;
-	}
-}
-
-/*
- * On powernv, we might not have symbols loaded via loader. However, if the
- * user passed the kernel in as the initrd as well, we can manually load it
- * via reinterpreting the initrd copy of the kernel.
+ * On powernv and some booke systems, we might not have symbols loaded via
+ * loader. However, if the user passed the kernel in as the initrd as well,
+ * we can manually load it via reinterpreting the initrd copy of the kernel.
+ *
+ * In the BOOKE case, we don't actually have a DMAP yet, so we have to use
+ * temporary maps to inspect the memory, but write DMAP addresses to the
+ * configuration variables.
  */
 static void
 load_external_symtab(void) {
@@ -547,7 +514,8 @@ load_external_symtab(void) {
 	vm_paddr_t start, end;
 	pcell_t cell[2];
 	ssize_t size;
-	u_char *kernelimg;
+	u_char *kernelimg;		/* Temporary map */
+	u_char *kernelimg_final;	/* Final location */
 
 	int i;
 
@@ -555,7 +523,8 @@ load_external_symtab(void) {
 	Elf_Phdr *phdr;
 	Elf_Shdr *shdr;
 
-        vm_offset_t ksym_start, ksym_sz, kstr_start, kstr_sz;
+	vm_offset_t ksym_start, ksym_sz, kstr_start, kstr_sz,
+	    ksym_start_final, kstr_start_final;
 
 	if (!hw_direct_map)
 		return;
@@ -587,27 +556,48 @@ load_external_symtab(void) {
 	if (!(end - start > 0))
 		return;
 
-	kernelimg = (u_char *) PHYS_TO_DMAP(start);
-
+	kernelimg_final = (u_char *) PHYS_TO_DMAP(start);
+#ifdef	AIM
+	kernelimg = kernelimg_final;
+#else	/* BOOKE */
+	kernelimg = (u_char *)pmap_early_io_map(start, PAGE_SIZE);
+#endif
 	ehdr = (Elf_Ehdr *)kernelimg;
 
-	if (!IS_ELF(*ehdr))
+	if (!IS_ELF(*ehdr)) {
+#ifdef	BOOKE
+		pmap_early_io_unmap(start, PAGE_SIZE);
+#endif
 		return;
+	}
+
+#ifdef	BOOKE
+	pmap_early_io_unmap(start, PAGE_SIZE);
+	kernelimg = (u_char *)pmap_early_io_map(start, (end - start));
+#endif
 
 	phdr = (Elf_Phdr *)(kernelimg + ehdr->e_phoff);
 	shdr = (Elf_Shdr *)(kernelimg + ehdr->e_shoff);
 
 	ksym_start = 0;
 	ksym_sz = 0;
+	ksym_start_final = 0;
 	kstr_start = 0;
 	kstr_sz = 0;
+	kstr_start_final = 0;
 	for (i = 0; i < ehdr->e_shnum; i++) {
 		if (shdr[i].sh_type == SHT_SYMTAB) {
 			ksym_start = (vm_offset_t)(kernelimg +
 			    shdr[i].sh_offset);
+			ksym_start_final = (vm_offset_t)
+			    (kernelimg_final + shdr[i].sh_offset);
 			ksym_sz = (vm_offset_t)(shdr[i].sh_size);
 			kstr_start = (vm_offset_t)(kernelimg +
 			    shdr[shdr[i].sh_link].sh_offset);
+			kstr_start_final = (vm_offset_t)
+			    (kernelimg_final +
+			    shdr[shdr[i].sh_link].sh_offset);
+
 			kstr_sz = (vm_offset_t)
 			    (shdr[shdr[i].sh_link].sh_size);
 		}
@@ -615,14 +605,23 @@ load_external_symtab(void) {
 
 	if (ksym_start != 0 && kstr_start != 0 && ksym_sz != 0 &&
 	    kstr_sz != 0 && ksym_start < kstr_start) {
-
-		displace_symbol_table(ksym_start, ksym_sz,
-		    (__startkernel - KERNBASE));
-		ksymtab = ksym_start;
+		/*
+		 * We can't use db_fetch_ksymtab() here, because we need to
+		 * feed in DMAP addresses that are not mapped yet on booke.
+		 *
+		 * Write the variables directly, where db_init() will pick
+		 * them up later, after the DMAP is up.
+		 */
+		ksymtab = ksym_start_final;
 		ksymtab_size = ksym_sz;
-		kstrtab = kstr_start;
+		kstrtab = kstr_start_final;
+		ksymtab_relbase = (__startkernel - KERNBASE);
 	}
 
+#ifdef	BOOKE
+	pmap_early_io_unmap(start, (end - start));
+#endif
+
 };
 #endif