printf: Automatically prefix %p with 0x

According to the POSIX standard, %p is supposed to print a pointer "as
if by %#x", meaning the "0x" prefix should automatically be prepended.
All other implementations out there (glibc, Linux, even libpayload) do
this, so we should make coreboot match. This patch changes vtxprintf()
accordingly and removes any explicit instances of "0x%p" from existing
format strings.

How to handle zero padding is less clear: the official POSIX definition
above technically says there should be no automatic zero padding, but in
practice most other implementations seem to do it and I assume most
programmers would prefer it. The way chosen here is to always zero-pad
to 32 bits, even on a 64-bit system. The rationale for this is that even
on 64-bit systems, coreboot always avoids using any memory above 4GB for
itself, so in practice all pointers should fit in that range and padding
everything to 64 bits would just hurt readability. Padding it this way
also helps pointers that do exceed 4GB (e.g. prints from MMU config on
some arm64 systems) stand out better from the others.

Change-Id: I0171b52f7288abb40e3fc3c8b874aee14b9bdcd6
Signed-off-by: Julius Werner <jwerner@chromium.org>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/37626
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net>
Reviewed-by: Patrick Georgi <pgeorgi@google.com>
Reviewed-by: David Guckian

5 files changed, 11 insertions, 11 deletions

diff --git a/src/lib/coreboot_table.c b/src/lib/coreboot_table.c
index af9f6599c5..e42cb3bdd2 100644
--- a/src/lib/coreboot_table.c
+++ b/src/lib/coreboot_table.c
@@ -476,7 +476,7 @@ size_t write_coreboot_forwarding_table(uintptr_t entry, uintptr_t target)
 {
 	struct lb_header *head;
 
-	printk(BIOS_DEBUG, "Writing table forward entry at 0x%p\n",
+	printk(BIOS_DEBUG, "Writing table forward entry at %p\n",
 		(void *)entry);
 
 	head = lb_table_init(entry);
diff --git a/src/lib/imd.c b/src/lib/imd.c
index b5fc34a9a0..4fa8f7023b 100644
--- a/src/lib/imd.c
+++ b/src/lib/imd.c
@@ -687,7 +687,7 @@ static void imdr_print_entries(const struct imdr *imdr, const char *indent,
 			printk(BIOS_DEBUG, "%s", name);
 		printk(BIOS_DEBUG, "%2zu. ", i);
 		printk(BIOS_DEBUG, "%p ", imdr_entry_at(imdr, e));
-		printk(BIOS_DEBUG, "%08zx\n", imdr_entry_size(imdr, e));
+		printk(BIOS_DEBUG, "0x%08zx\n", imdr_entry_size(imdr, e));
 	}
 }
 
diff --git a/src/lib/rmodule.c b/src/lib/rmodule.c
index 56529d2fb2..96cee8aad3 100644
--- a/src/lib/rmodule.c
+++ b/src/lib/rmodule.c
@@ -278,7 +278,7 @@ int rmodule_stage_load(struct rmod_stage_load *rsl)
 
 	rmod_loc = &stage_region[rmodule_offset];
 
-	printk(BIOS_INFO, "Decompressing stage %s @ 0x%p (%d bytes)\n",
+	printk(BIOS_INFO, "Decompressing stage %s @ %p (%d bytes)\n",
 	       prog_name(rsl->prog), rmod_loc, stage.memlen);
 
 	if (!cbfs_load_and_decompress(fh, sizeof(stage), stage.len, rmod_loc,
diff --git a/src/lib/selfboot.c b/src/lib/selfboot.c
index a0bb711f8a..8cf7a6ff57 100644
--- a/src/lib/selfboot.c
+++ b/src/lib/selfboot.c
@@ -56,7 +56,7 @@ static int segment_targets_type(void *dest, unsigned long memsz,
 	if (payload_arch_usable_ram_quirk(d, memsz))
 		return 1;
 
-	printk(BIOS_ERR, "SELF segment doesn't target RAM: 0x%p, %lu bytes\n", dest, memsz);
+	printk(BIOS_ERR, "SELF segment doesn't target RAM: %p, %lu bytes\n", dest, memsz);
 	bootmem_dump_ranges();
 	return 0;
 }
@@ -69,7 +69,7 @@ static int load_one_segment(uint8_t *dest,
 			    int flags)
 {
 		unsigned char *middle, *end;
-		printk(BIOS_DEBUG, "Loading Segment: addr: 0x%p memsz: 0x%016zx filesz: 0x%016zx\n",
+		printk(BIOS_DEBUG, "Loading Segment: addr: %p memsz: 0x%016zx filesz: 0x%016zx\n",
 		       dest, memsz, len);
 
 		/* Compute the boundaries of the segment */
@@ -150,7 +150,7 @@ static int check_payload_segments(struct cbfs_payload_segment *cbfssegs,
 	enum bootmem_type dest_type = *(enum bootmem_type *)args;
 
 	for (seg = cbfssegs;; ++seg) {
-		printk(BIOS_DEBUG, "Checking segment from ROM address 0x%p\n", seg);
+		printk(BIOS_DEBUG, "Checking segment from ROM address %p\n", seg);
 		cbfs_decode_payload_segment(&segment, seg);
 		dest = (uint8_t *)(uintptr_t)segment.load_addr;
 		memsz = segment.mem_len;
@@ -171,7 +171,7 @@ static int load_payload_segments(struct cbfs_payload_segment *cbfssegs, uintptr_
 	int flags = 0;
 
 	for (first_segment = seg = cbfssegs;; ++seg) {
-		printk(BIOS_DEBUG, "Loading segment from ROM address 0x%p\n", seg);
+		printk(BIOS_DEBUG, "Loading segment from ROM address %p\n", seg);
 
 		cbfs_decode_payload_segment(&segment, seg);
 		dest = (uint8_t *)(uintptr_t)segment.load_addr;
@@ -187,7 +187,7 @@ static int load_payload_segments(struct cbfs_payload_segment *cbfssegs, uintptr_
 				?  "code" : "data", segment.compression);
 			src = ((uint8_t *)first_segment) + segment.offset;
 			printk(BIOS_DEBUG,
-				"  New segment dstaddr 0x%p memsize 0x%zx srcaddr 0x%p filesize 0x%zx\n",
+				"  New segment dstaddr %p memsize 0x%zx srcaddr %p filesize 0x%zx\n",
 			       dest, memsz, src, filesz);
 
 			/* Clean up the values */
@@ -198,7 +198,7 @@ static int load_payload_segments(struct cbfs_payload_segment *cbfssegs, uintptr_
 			break;
 
 		case PAYLOAD_SEGMENT_BSS:
-			printk(BIOS_DEBUG, "  BSS 0x%p (%d byte)\n", (void *)
+			printk(BIOS_DEBUG, "  BSS %p (%d byte)\n", (void *)
 				(intptr_t)segment.load_addr, segment.mem_len);
 			filesz = 0;
 			src = ((uint8_t *)first_segment) + segment.offset;
@@ -206,7 +206,7 @@ static int load_payload_segments(struct cbfs_payload_segment *cbfssegs, uintptr_
 			break;
 
 		case PAYLOAD_SEGMENT_ENTRY:
-			printk(BIOS_DEBUG, "  Entry Point 0x%p\n", (void *)
+			printk(BIOS_DEBUG, "  Entry Point %p\n", (void *)
 				(intptr_t)segment.load_addr);
 
 			*entry = segment.load_addr;
diff --git a/src/lib/trace.c b/src/lib/trace.c
index b11881760d..826fa3b671 100644
--- a/src/lib/trace.c
+++ b/src/lib/trace.c
@@ -26,7 +26,7 @@ void __cyg_profile_func_enter(void *func, void *callsite)
 		return;
 
 	DISABLE_TRACE
-	printk(BIOS_INFO, "~0x%p(0x%p)\n", func, callsite);
+	printk(BIOS_INFO, "~%p(%p)\n", func, callsite);
 	ENABLE_TRACE
 }