[U-Boot] [PATCH v2 00/10] bootstage: TPL and SPL improvements

[U-Boot] [PATCH v2 00/10] bootstage: TPL and SPL improvements

[Simon Glass]

At present bootstage cannot be fully used on x86 since it violates a few
U-Boot rules, mostly accessing pre-relocation memory after relocation.
This series corrects this and adds better support for using bootstage in
TPL.

It also includes a few improvements to tiny-printf.

Changes in v2:
- Add a new patch to support %p without DEBUG
- Adjust SPL logic to avoid failing if TPL does not provide bootstage data
- Add a new patch to support %p without DEBUG in tiny-printf

Simon Glass (10):
  tiny-printf: Reduce size by removing ctype
  tiny-printf: Add print_grouped_ull()
  tiny-printf: Reorder code to support %p
  bloblist: Reserve an aligned base
  bootstage: Store the next ID in the stash
  bootstage: Fix counting of entries in stash
  bootstage: Avoid conflicts between stash/unstash
  bootstage: Correct relocation algorithm
  bootstage: Mark the start/end of TPL and SPL separately
  bootstage: Allow SPL to obtain bootstage info from TPL

 common/board_f.c    |  2 ++
 common/board_r.c    |  1 -
 common/bootstage.c  | 53 ++++++++++++++++++++++++++++++---------------
 common/spl/spl.c    | 23 ++++++++++++++++----
 include/bootstage.h |  2 ++
 lib/tiny-printf.c   | 29 ++++++++++++++++++++-----
 6 files changed, 81 insertions(+), 29 deletions(-)

-- 
2.23.0.866.gb869b98d4c-goog

[U-Boot] [PATCH v2 01/10] tiny-printf: Reduce size by removing ctype

[Simon Glass]

The ctype array is brought into the image, adding 256 bytes, when it is
unlikely to be needed. The extra code for %p is only present when DEBUG
is defined, so let's drop ctype as well unless DEBUG is defined.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2: None

 lib/tiny-printf.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/lib/tiny-printf.c b/lib/tiny-printf.c
index ebef92fc9f6..632b4249142 100644
--- a/lib/tiny-printf.c
+++ b/lib/tiny-printf.c
@@ -289,8 +289,15 @@ static int _vprintf(struct printf_info *info, const char *fmt, va_list va)
 				break;
 			case 'p':
 				pointer(info, fmt, va_arg(va, void *));
+				/*
+				 * Skip this because it pulls in _ctype which is
+				 * 256 bytes, and we don't generally implement
+				 * pointer anyway
+				 */
+#ifdef DEBUG
 				while (isalnum(fmt[0]))
 					fmt++;
+#endif
 				break;
 			case '%':
 				out(info, '%');
-- 
2.23.0.866.gb869b98d4c-goog

[U-Boot] [PATCH v2 02/10] tiny-printf: Add print_grouped_ull()

[Simon Glass]

This function is used in the bootstage report which may be trigged in TPL
or TPL. Add a very basic implication of this function so that it builds.
There is no attempt to get the formatting right, since this would add too
much code size.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2: None

 lib/tiny-printf.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/lib/tiny-printf.c b/lib/tiny-printf.c
index 632b4249142..430c3255bc5 100644
--- a/lib/tiny-printf.c
+++ b/lib/tiny-printf.c
@@ -389,3 +389,9 @@ int snprintf(char *buf, size_t size, const char *fmt, ...)
 
 	return ret;
 }
+
+void print_grouped_ull(unsigned long long int_val, int digits)
+{
+	/* Don't try to ptint the upper 32-bits */
+	printf("%ld ", (ulong)int_val);
+}
-- 
2.23.0.866.gb869b98d4c-goog

[U-Boot] [PATCH v2 03/10] tiny-printf: Reorder code to support %p

[Simon Glass]

With a bit of code reordering we can support %p using the existing code
for ulong.

Move the %p code up and adjust the logic accordingly.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2:
- Add a new patch to support %p without DEBUG

 lib/tiny-printf.c | 30 +++++++++++++++++-------------
 1 file changed, 17 insertions(+), 13 deletions(-)

diff --git a/lib/tiny-printf.c b/lib/tiny-printf.c
index 430c3255bc5..c80c4f95ae0 100644
--- a/lib/tiny-printf.c
+++ b/lib/tiny-printf.c
@@ -157,7 +157,8 @@ static void ip4_addr_string(struct printf_info *info, u8 *addr)
  *       decimal).
  */
 
-static void pointer(struct printf_info *info, const char *fmt, void *ptr)
+static void __maybe_unused pointer(struct printf_info *info, const char *fmt,
+				   void *ptr)
 {
 #ifdef DEBUG
 	unsigned long num = (uintptr_t)ptr;
@@ -266,6 +267,21 @@ static int _vprintf(struct printf_info *info, const char *fmt, va_list va)
 						div_out(info, &num, div);
 				}
 				break;
+			case 'p':
+#ifdef DEBUG
+				pointer(info, fmt, va_arg(va, void *));
+				/*
+				 * Skip this because it pulls in _ctype which is
+				 * 256 bytes, and we don't generally implement
+				 * pointer anyway
+				 */
+				while (isalnum(fmt[0]))
+					fmt++;
+				break;
+#else
+				islong = true;
+				/* no break */
+#endif
 			case 'x':
 				if (islong) {
 					num = va_arg(va, unsigned long);
@@ -287,18 +303,6 @@ static int _vprintf(struct printf_info *info, const char *fmt, va_list va)
 			case 's':
 				p = va_arg(va, char*);
 				break;
-			case 'p':
-				pointer(info, fmt, va_arg(va, void *));
-				/*
-				 * Skip this because it pulls in _ctype which is
-				 * 256 bytes, and we don't generally implement
-				 * pointer anyway
-				 */
-#ifdef DEBUG
-				while (isalnum(fmt[0]))
-					fmt++;
-#endif
-				break;
 			case '%':
 				out(info, '%');
 			default:
-- 
2.23.0.866.gb869b98d4c-goog

[U-Boot] [PATCH v2 04/10] bloblist: Reserve an aligned base

[Simon Glass]

Make sure that the bloblist starts on an aligned boundary. This protects
against one of the early allocating causing the alignment to be lost.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2: None

 common/board_f.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/common/board_f.c b/common/board_f.c
index 591f18f391e..4852a3b0d84 100644
--- a/common/board_f.c
+++ b/common/board_f.c
@@ -588,6 +588,7 @@ static int reserve_stacks(void)
 static int reserve_bloblist(void)
 {
 #ifdef CONFIG_BLOBLIST
+	gd->start_addr_sp &= ~0xf;
 	gd->start_addr_sp -= CONFIG_BLOBLIST_SIZE;
 	gd->new_bloblist = map_sysmem(gd->start_addr_sp, CONFIG_BLOBLIST_SIZE);
 #endif
-- 
2.23.0.866.gb869b98d4c-goog

[U-Boot] [PATCH v2 05/10] bootstage: Store the next ID in the stash

[Simon Glass]

When stashing bootstage info, store the next ID so that it can be used
when the stash is restored. This avoids the ID starting at zero and
potentially overwriting existing entries.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2: None

 common/bootstage.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/common/bootstage.c b/common/bootstage.c
index 56ef91ad859..257dc5d402f 100644
--- a/common/bootstage.c
+++ b/common/bootstage.c
@@ -41,10 +41,11 @@ enum {
 };
 
 struct bootstage_hdr {
-	uint32_t version;	/* BOOTSTAGE_VERSION */
-	uint32_t count;		/* Number of records */
-	uint32_t size;		/* Total data size (non-zero if valid) */
-	uint32_t magic;		/* Unused */
+	u32 version;		/* BOOTSTAGE_VERSION */
+	u32 count;		/* Number of records */
+	u32 size;		/* Total data size (non-zero if valid) */
+	u32 magic;		/* Magic number */
+	u32 next_id;		/* Next ID to use for bootstage */
 };
 
 int bootstage_relocate(void)
@@ -392,6 +393,7 @@ int bootstage_stash(void *base, int size)
 	hdr->count = count;
 	hdr->size = 0;
 	hdr->magic = BOOTSTAGE_MAGIC;
+	hdr->next_id = data->next_id;
 	ptr += sizeof(*hdr);
 
 	/* Write the records, silently stopping when we run out of space */
@@ -485,6 +487,7 @@ int bootstage_unstash(const void *base, int size)
 
 	/* Mark the records as read */
 	data->rec_count += hdr->count;
+	data->next_id = hdr->next_id;
 	debug("Unstashed %d records\n", hdr->count);
 
 	return 0;
-- 
2.23.0.866.gb869b98d4c-goog

[U-Boot] [PATCH v2 06/10] bootstage: Fix counting of entries in stash

[Simon Glass]

The current code searches for empty records but these not existing with
bootstage now. This used to be needed when bootstage records were stored
in a spare array.

Drop the unnecessary code and fix a code-style nit at the same time.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2: None

 common/bootstage.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/common/bootstage.c b/common/bootstage.c
index 257dc5d402f..fe36bac0474 100644
--- a/common/bootstage.c
+++ b/common/bootstage.c
@@ -373,7 +373,6 @@ int bootstage_stash(void *base, int size)
 	const struct bootstage_record *rec;
 	char buf[20];
 	char *ptr = base, *end = ptr + size;
-	uint32_t count;
 	int i;
 
 	if (hdr + 1 > (struct bootstage_hdr *)end) {
@@ -384,22 +383,15 @@ int bootstage_stash(void *base, int size)
 	/* Write an arbitrary version number */
 	hdr->version = BOOTSTAGE_VERSION;
 
-	/* Count the number of records, and write that value first */
-	for (rec = data->record, i = count = 0; i < data->rec_count;
-	     i++, rec++) {
-		if (rec->id != 0)
-			count++;
-	}
-	hdr->count = count;
+	hdr->count = data->rec_count;
 	hdr->size = 0;
 	hdr->magic = BOOTSTAGE_MAGIC;
 	hdr->next_id = data->next_id;
 	ptr += sizeof(*hdr);
 
 	/* Write the records, silently stopping when we run out of space */
-	for (rec = data->record, i = 0; i < data->rec_count; i++, rec++) {
+	for (rec = data->record, i = 0; i < data->rec_count; i++, rec++)
 		append_data(&ptr, end, rec, sizeof(*rec));
-	}
 
 	/* Write the name strings */
 	for (rec = data->record, i = 0; i < data->rec_count; i++, rec++) {
-- 
2.23.0.866.gb869b98d4c-goog

[U-Boot] [PATCH v2 07/10] bootstage: Avoid conflicts between stash/unstash

[Simon Glass]

At present there is a single shared address for bootstage data in both
TPL and SPL. If SPL unstashs TPL bootstage info and then stashes it again
to pass it to U-Boot, the new stash overwrites the strings of the old
stash.

Fix this by duplicating the strings into the malloc() region. This should
be a small code. Fix the header-file order at the same time.

This problem doesn't happen at the next stage (SPL->U-Boot) since U-Boot
relocates the boostage data.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2: None

 common/bootstage.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/common/bootstage.c b/common/bootstage.c
index fe36bac0474..4557ed4508c 100644
--- a/common/bootstage.c
+++ b/common/bootstage.c
@@ -10,9 +10,10 @@
  */
 
 #include <common.h>
-#include <linux/libfdt.h>
 #include <malloc.h>
+#include <spl.h>
 #include <linux/compiler.h>
+#include <linux/libfdt.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -472,6 +473,8 @@ int bootstage_unstash(const void *base, int size)
 	for (rec = data->record + data->next_id, i = 0; i < hdr->count;
 	     i++, rec++) {
 		rec->name = ptr;
+		if (spl_phase() == PHASE_SPL)
+			rec->name = strdup(ptr);
 
 		/* Assume no data corruption here */
 		ptr += strlen(ptr) + 1;
-- 
2.23.0.866.gb869b98d4c-goog

[U-Boot] [PATCH v2 08/10] bootstage: Correct relocation algorithm

[Simon Glass]

At present bootstage relocation assumes that it is possible to point back
to memory available before relocation, so it does not relocate the
strings. However this is not the case on some platforms, such as x86 which
uses the cache as RAM and loses access to this when the cache is enabled.

Move the relocation step to before U-Boot relocates, expand the allocated
region to include space for the strings and relocate the strings at the
same time as the bootstage records.

This ensures that bootstage data can remain accessible from TPL through
SPL to U-Boot before/after relocation.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2: None

 common/board_f.c   |  1 +
 common/board_r.c   |  1 -
 common/bootstage.c | 25 ++++++++++++++++++++++---
 3 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/common/board_f.c b/common/board_f.c
index 4852a3b0d84..e3591cbaebd 100644
--- a/common/board_f.c
+++ b/common/board_f.c
@@ -696,6 +696,7 @@ static int reloc_bootstage(void)
 		      gd->bootstage, gd->new_bootstage, size);
 		memcpy(gd->new_bootstage, gd->bootstage, size);
 		gd->bootstage = gd->new_bootstage;
+		bootstage_relocate();
 	}
 #endif
 
diff --git a/common/board_r.c b/common/board_r.c
index d6fb5047a26..c1ecb06b743 100644
--- a/common/board_r.c
+++ b/common/board_r.c
@@ -670,7 +670,6 @@ static init_fnc_t init_sequence_r[] = {
 #ifdef CONFIG_SYS_NONCACHED_MEMORY
 	initr_noncached,
 #endif
-	bootstage_relocate,
 #ifdef CONFIG_OF_LIVE
 	initr_of_live,
 #endif
diff --git a/common/bootstage.c b/common/bootstage.c
index 4557ed4508c..e8b7bbf81a6 100644
--- a/common/bootstage.c
+++ b/common/bootstage.c
@@ -53,14 +53,23 @@ int bootstage_relocate(void)
 {
 	struct bootstage_data *data = gd->bootstage;
 	int i;
+	char *ptr;
+
+	/* Figure out where to relocate the strings to */
+	ptr = (char *)(data + 1);
 
 	/*
 	 * Duplicate all strings.  They may point to an old location in the
 	 * program .text section that can eventually get trashed.
 	 */
 	debug("Relocating %d records\n", data->rec_count);
-	for (i = 0; i < data->rec_count; i++)
-		data->record[i].name = strdup(data->record[i].name);
+	for (i = 0; i < data->rec_count; i++) {
+		const char *from = data->record[i].name;
+
+		strcpy(ptr, from);
+		data->record[i].name = ptr;
+		ptr += strlen(ptr) + 1;
+	}
 
 	return 0;
 }
@@ -490,7 +499,17 @@ int bootstage_unstash(const void *base, int size)
 
 int bootstage_get_size(void)
 {
-	return sizeof(struct bootstage_data);
+	struct bootstage_data *data = gd->bootstage;
+	struct bootstage_record *rec;
+	int size;
+	int i;
+
+	size = sizeof(struct bootstage_data);
+	for (rec = data->record, i = 0; i < data->rec_count;
+	     i++, rec++)
+		size += strlen(rec->name) + 1;
+
+	return size;
 }
 
 int bootstage_init(bool first)
-- 
2.23.0.866.gb869b98d4c-goog

[U-Boot] [PATCH v2 09/10] bootstage: Mark the start/end of TPL and SPL separately

[Simon Glass]

At present bootstage in TPL and SPL use the same ID so it is not possible
to see the timing of each. Separate out the IDs and use the correct one
depending on which phase we are at.

Example output:

Timer summary in microseconds (14 records):
       Mark    Elapsed  Stage
          0          0  reset
    224,787    224,787  TPL
    282,248     57,461  end TPL
    341,067     58,819  SPL
    925,436    584,369  end SPL
    931,710      6,274  board_init_f
  1,035,482    103,772  board_init_r
  1,387,852    352,370  main_loop
  1,387,911         59  id=175

Accumulated time:
                   196  dm_r
                 8,300  dm_spl
                14,139  dm_f
               229,121  fsp-m
               262,992  fsp-s

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2: None

 common/spl/spl.c    | 9 ++++++---
 include/bootstage.h | 2 ++
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/common/spl/spl.c b/common/spl/spl.c
index a9d3e847af5..eabb1fbc138 100644
--- a/common/spl/spl.c
+++ b/common/spl/spl.c
@@ -402,7 +402,8 @@ static int spl_common_init(bool setup_malloc)
 		      ret);
 		return ret;
 	}
-	bootstage_mark_name(BOOTSTAGE_ID_START_SPL, "spl");
+	bootstage_mark_name(spl_phase() == PHASE_TPL ? BOOTSTAGE_ID_START_TPL :
+			    BOOTSTAGE_ID_START_SPL, SPL_TPL_NAME);
 #if CONFIG_IS_ENABLED(LOG)
 	ret = log_init();
 	if (ret) {
@@ -418,7 +419,8 @@ static int spl_common_init(bool setup_malloc)
 		}
 	}
 	if (CONFIG_IS_ENABLED(DM)) {
-		bootstage_start(BOOTSTATE_ID_ACCUM_DM_SPL, "dm_spl");
+		bootstage_start(BOOTSTATE_ID_ACCUM_DM_SPL,
+				spl_phase() == PHASE_TPL ? "dm tpl" : "dm_spl");
 		/* With CONFIG_SPL_OF_PLATDATA, bring in all devices */
 		ret = dm_init_and_scan(!CONFIG_IS_ENABLED(OF_PLATDATA));
 		bootstage_accum(BOOTSTATE_ID_ACCUM_DM_SPL);
@@ -704,8 +706,9 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
 	debug("SPL malloc() used 0x%lx bytes (%ld KB)\n", gd->malloc_ptr,
 	      gd->malloc_ptr / 1024);
 #endif
+	bootstage_mark_name(spl_phase() == PHASE_TPL ? BOOTSTAGE_ID_END_TPL :
+			    BOOTSTAGE_ID_END_SPL, "end " SPL_TPL_NAME);
 #ifdef CONFIG_BOOTSTAGE_STASH
-	bootstage_mark_name(BOOTSTAGE_ID_END_SPL, "end_spl");
 	ret = bootstage_stash((void *)CONFIG_BOOTSTAGE_STASH_ADDR,
 			      CONFIG_BOOTSTAGE_STASH_SIZE);
 	if (ret)
diff --git a/include/bootstage.h b/include/bootstage.h
index 5e7e242b834..d105ae01813 100644
--- a/include/bootstage.h
+++ b/include/bootstage.h
@@ -170,6 +170,8 @@ enum bootstage_id {
 	 * rough boot timing information.
 	 */
 	BOOTSTAGE_ID_AWAKE,
+	BOOTSTAGE_ID_START_TPL,
+	BOOTSTAGE_ID_END_TPL,
 	BOOTSTAGE_ID_START_SPL,
 	BOOTSTAGE_ID_END_SPL,
 	BOOTSTAGE_ID_START_UBOOT_F,
-- 
2.23.0.866.gb869b98d4c-goog

[U-Boot] [PATCH v2 10/10] bootstage: Allow SPL to obtain bootstage info from TPL

[Simon Glass]

It is possible to enable bootstage in TPL. TPL can stash the info for SPL.
But at present this information is then lost because SPL does not read
from the stash.

Add support for SPL not being the first phase to enable bootstage.
Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2:
- Adjust SPL logic to avoid failing if TPL does not provide bootstage data
- Add a new patch to support %p without DEBUG in tiny-printf

 common/spl/spl.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/common/spl/spl.c b/common/spl/spl.c
index eabb1fbc138..f1ad8dc9dab 100644
--- a/common/spl/spl.c
+++ b/common/spl/spl.c
@@ -18,6 +18,7 @@
 #include <version.h>
 #include <image.h>
 #include <malloc.h>
+#include <mapmem.h>
 #include <dm/root.h>
 #include <linux/compiler.h>
 #include <fdt_support.h>
@@ -396,12 +397,23 @@ static int spl_common_init(bool setup_malloc)
 		gd->malloc_ptr = 0;
 	}
 #endif
-	ret = bootstage_init(true);
+	ret = bootstage_init(u_boot_first_phase());
 	if (ret) {
 		debug("%s: Failed to set up bootstage: ret=%d\n", __func__,
 		      ret);
 		return ret;
 	}
+#ifdef CONFIG_BOOTSTAGE_STASH
+	if (!u_boot_first_phase()) {
+		const void *stash = map_sysmem(CONFIG_BOOTSTAGE_STASH_ADDR,
+					       CONFIG_BOOTSTAGE_STASH_SIZE);
+
+		ret = bootstage_unstash(stash, CONFIG_BOOTSTAGE_STASH_SIZE);
+		if (ret)
+			debug("%s: Failed to unstash bootstage: ret=%d\n",
+			      __func__, ret);
+	}
+#endif /* CONFIG_BOOTSTAGE_STASH */
 	bootstage_mark_name(spl_phase() == PHASE_TPL ? BOOTSTAGE_ID_START_TPL :
 			    BOOTSTAGE_ID_START_SPL, SPL_TPL_NAME);
 #if CONFIG_IS_ENABLED(LOG)
-- 
2.23.0.866.gb869b98d4c-goog

[U-Boot] [PATCH v2 02/10] tiny-printf: Add print_grouped_ull()

[Stefan Roese]

On 22.10.19 01:26, Simon Glass wrote:
> This function is used in the bootstage report which may be trigged in TPL
> or TPL. Add a very basic implication of this function so that it builds.
> There is no attempt to get the formatting right, since this would add too
> much code size.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
> Changes in v2: None
> 
>   lib/tiny-printf.c | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/lib/tiny-printf.c b/lib/tiny-printf.c
> index 632b4249142..430c3255bc5 100644
> --- a/lib/tiny-printf.c
> +++ b/lib/tiny-printf.c
> @@ -389,3 +389,9 @@ int snprintf(char *buf, size_t size, const char *fmt, ...)
>   
>   	return ret;
>   }
> +
> +void print_grouped_ull(unsigned long long int_val, int digits)
> +{
> +	/* Don't try to ptint the upper 32-bits */

s/ptint/print ?

> +	printf("%ld ", (ulong)int_val);
> +}
> 

Other than that:

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

[U-Boot] [PATCH v2 10/10] bootstage: Allow SPL to obtain bootstage info from TPL

[sjg at google.com]

It is possible to enable bootstage in TPL. TPL can stash the info for SPL.
But at present this information is then lost because SPL does not read
from the stash.

Add support for SPL not being the first phase to enable bootstage.
Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2:
- Adjust SPL logic to avoid failing if TPL does not provide bootstage data
- Add a new patch to support %p without DEBUG in tiny-printf

 common/spl/spl.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

Applied to u-boot-dm, thanks!

[U-Boot] [PATCH v2 09/10] bootstage: Mark the start/end of TPL and SPL separately

[sjg at google.com]

At present bootstage in TPL and SPL use the same ID so it is not possible
to see the timing of each. Separate out the IDs and use the correct one
depending on which phase we are at.

Example output:

Timer summary in microseconds (14 records):
       Mark    Elapsed  Stage
          0          0  reset
    224,787    224,787  TPL
    282,248     57,461  end TPL
    341,067     58,819  SPL
    925,436    584,369  end SPL
    931,710      6,274  board_init_f
  1,035,482    103,772  board_init_r
  1,387,852    352,370  main_loop
  1,387,911         59  id=175

Accumulated time:
                   196  dm_r
                 8,300  dm_spl
                14,139  dm_f
               229,121  fsp-m
               262,992  fsp-s

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2: None

 common/spl/spl.c    | 9 ++++++---
 include/bootstage.h | 2 ++
 2 files changed, 8 insertions(+), 3 deletions(-)

Applied to u-boot-dm, thanks!

[U-Boot] [PATCH v2 07/10] bootstage: Avoid conflicts between stash/unstash

[sjg at google.com]

At present there is a single shared address for bootstage data in both
TPL and SPL. If SPL unstashs TPL bootstage info and then stashes it again
to pass it to U-Boot, the new stash overwrites the strings of the old
stash.

Fix this by duplicating the strings into the malloc() region. This should
be a small code. Fix the header-file order at the same time.

This problem doesn't happen at the next stage (SPL->U-Boot) since U-Boot
relocates the boostage data.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2: None

 common/bootstage.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Applied to u-boot-dm, thanks!

[U-Boot] [PATCH v2 08/10] bootstage: Correct relocation algorithm

[sjg at google.com]

At present bootstage relocation assumes that it is possible to point back
to memory available before relocation, so it does not relocate the
strings. However this is not the case on some platforms, such as x86 which
uses the cache as RAM and loses access to this when the cache is enabled.

Move the relocation step to before U-Boot relocates, expand the allocated
region to include space for the strings and relocate the strings at the
same time as the bootstage records.

This ensures that bootstage data can remain accessible from TPL through
SPL to U-Boot before/after relocation.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2: None

 common/board_f.c   |  1 +
 common/board_r.c   |  1 -
 common/bootstage.c | 25 ++++++++++++++++++++++---
 3 files changed, 23 insertions(+), 4 deletions(-)

Applied to u-boot-dm, thanks!

[U-Boot] [PATCH v2 06/10] bootstage: Fix counting of entries in stash

[sjg at google.com]

The current code searches for empty records but these not existing with
bootstage now. This used to be needed when bootstage records were stored
in a spare array.

Drop the unnecessary code and fix a code-style nit at the same time.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2: None

 common/bootstage.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

Applied to u-boot-dm, thanks!

[U-Boot] [PATCH v2 05/10] bootstage: Store the next ID in the stash

[sjg at google.com]

When stashing bootstage info, store the next ID so that it can be used
when the stash is restored. This avoids the ID starting at zero and
potentially overwriting existing entries.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2: None

 common/bootstage.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

Applied to u-boot-dm, thanks!

[U-Boot] [PATCH v2 03/10] tiny-printf: Reorder code to support %p

[sjg at google.com]

With a bit of code reordering we can support %p using the existing code
for ulong.

Move the %p code up and adjust the logic accordingly.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2:
- Add a new patch to support %p without DEBUG

 lib/tiny-printf.c | 30 +++++++++++++++++-------------
 1 file changed, 17 insertions(+), 13 deletions(-)

Applied to u-boot-dm, thanks!

[U-Boot] [PATCH v2 04/10] bloblist: Reserve an aligned base

[sjg at google.com]

Make sure that the bloblist starts on an aligned boundary. This protects
against one of the early allocating causing the alignment to be lost.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2: None

 common/board_f.c | 1 +
 1 file changed, 1 insertion(+)

Applied to u-boot-dm, thanks!

[U-Boot] [PATCH v2 02/10] tiny-printf: Add print_grouped_ull()

[sjg at google.com]

On 22.10.19 01:26, Simon Glass wrote:
> This function is used in the bootstage report which may be trigged in TPL
> or TPL. Add a very basic implication of this function so that it builds.
> There is no attempt to get the formatting right, since this would add too
> much code size.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
> Changes in v2: None
>
>   lib/tiny-printf.c | 6 ++++++
>   1 file changed, 6 insertions(+)
>
Applied to u-boot-dm, thanks!

[U-Boot] [PATCH v2 01/10] tiny-printf: Reduce size by removing ctype

[sjg at google.com]

The ctype array is brought into the image, adding 256 bytes, when it is
unlikely to be needed. The extra code for %p is only present when DEBUG
is defined, so let's drop ctype as well unless DEBUG is defined.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2: None

 lib/tiny-printf.c | 7 +++++++
 1 file changed, 7 insertions(+)

Applied to u-boot-dm, thanks!

[U-Boot] [PATCH v2 08/10] bootstage: Correct relocation algorithm

[Heinrich Schuchardt]

On 10/22/19 1:26 AM, Simon Glass wrote:
> At present bootstage relocation assumes that it is possible to point back
> to memory available before relocation, so it does not relocate the
> strings. However this is not the case on some platforms, such as x86 which
> uses the cache as RAM and loses access to this when the cache is enabled.
>
> Move the relocation step to before U-Boot relocates, expand the allocated
> region to include space for the strings and relocate the strings at the
> same time as the bootstage records.
>
> This ensures that bootstage data can remain accessible from TPL through
> SPL to U-Boot before/after relocation.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>

Hello Simon,

this merged patch seems to be incorrect. I compiled sandbox_defconfig
with clang and ran it with valgrind.

We allocate memory in bootstage_init() for gd->bootstage. But from
bootstage_get_size() we return a size that is larger than what we have
allocated and use that larger memory area in reloc_bootstage(). See
output below.

sudo docker pull trini/u-boot-gitlab-ci-runner:bionic-20200112-17Jan2020
sudo docker run -it 2e501a804876 /bin/bash
cd ~
git clone https://gitlab.denx.de/u-boot/u-boot.git
cd u-boot
sudo apt-get update
sudo apt-get install clang valgrind
make CC=clang HOSTCC=clang sandbox_defconfig
make CC=clang HOSTCC=clang -j8
valgrind ./u-boot -d arch/sandbox/dts/test.dtb

==89423== Memcheck, a memory error detector
==89423== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==89423== Using Valgrind-3.13.0 and LibVEX; rerun with -h for copyright info
==89423== Command: ./u-boot -d arch/sandbox/dts/test.dtb
==89423==


U-Boot 2020.01-00812-gc98c2b07e5 (Jan 25 2020 - 07:36:12 +0000)

Model: sandbox
DRAM:  128 MiB
==89423== Invalid read of size 8
==89423==    at 0x4D4E90: memcpy (string.c:543)
==89423==    by 0x424F88: reloc_bootstage (board_f.c:702)
==89423==    by 0x424B22: initcall_run_list (initcall.h:40)
==89423==    by 0x424B22: board_init_f (board_f.c:1019)
==89423==    by 0x402755: main (start.c:386)
==89423==  Address 0xa66f4e8 is 0 bytes after a block of size 968 alloc'd
==89423==    at 0x4C2FB0F: malloc (in
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==89423==    by 0x42E740: bootstage_init (bootstage.c:522)
==89423==    by 0x424B84: initf_bootstage (board_f.c:806)
==89423==    by 0x424B22: initcall_run_list (initcall.h:40)
==89423==    by 0x424B22: board_init_f (board_f.c:1019)
==89423==    by 0x402755: main (start.c:386)
==89423==
==89423== Invalid read of size 8
==89423==    at 0x4D4EA4: memcpy (string.c:542)
==89423==    by 0x424F88: reloc_bootstage (board_f.c:702)
==89423==    by 0x424B22: initcall_run_list (initcall.h:40)
==89423==    by 0x424B22: board_init_f (board_f.c:1019)
==89423==    by 0x402755: main (start.c:386)
==89423==  Address 0xa66f4f0 is 8 bytes after a block of size 968 alloc'd
==89423==    at 0x4C2FB0F: malloc (in
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==89423==    by 0x42E740: bootstage_init (bootstage.c:522)
==89423==    by 0x424B84: initf_bootstage (board_f.c:806)
==89423==    by 0x424B22: initcall_run_list (initcall.h:40)
==89423==    by 0x424B22: board_init_f (board_f.c:1019)
==89423==    by 0x402755: main (start.c:386)
==89423==
WDT:   Started with servicing (60s timeout)
MMC:   mmc2: 2 (SD), mmc1: 1 (SD), mmc0: 0 (SD)
In:    serial
Out:   vidconsole
Err:   vidconsole
Model: sandbox
SCSI:
Net:   eth0: eth at 10002000, eth5: eth at 10003000, eth3: sbe5, eth1:
eth at 10004000
Hit any key to stop autoboot:  0
=>

Best regards

Heinrich

> ---
>
> Changes in v2: None
>
>   common/board_f.c   |  1 +
>   common/board_r.c   |  1 -
>   common/bootstage.c | 25 ++++++++++++++++++++++---
>   3 files changed, 23 insertions(+), 4 deletions(-)
>
> diff --git a/common/board_f.c b/common/board_f.c
> index 4852a3b0d84..e3591cbaebd 100644
> --- a/common/board_f.c
> +++ b/common/board_f.c
> @@ -696,6 +696,7 @@ static int reloc_bootstage(void)
>   		      gd->bootstage, gd->new_bootstage, size);
>   		memcpy(gd->new_bootstage, gd->bootstage, size);
>   		gd->bootstage = gd->new_bootstage;
> +		bootstage_relocate();
>   	}
>   #endif
>
> diff --git a/common/board_r.c b/common/board_r.c
> index d6fb5047a26..c1ecb06b743 100644
> --- a/common/board_r.c
> +++ b/common/board_r.c
> @@ -670,7 +670,6 @@ static init_fnc_t init_sequence_r[] = {
>   #ifdef CONFIG_SYS_NONCACHED_MEMORY
>   	initr_noncached,
>   #endif
> -	bootstage_relocate,
>   #ifdef CONFIG_OF_LIVE
>   	initr_of_live,
>   #endif
> diff --git a/common/bootstage.c b/common/bootstage.c
> index 4557ed4508c..e8b7bbf81a6 100644
> --- a/common/bootstage.c
> +++ b/common/bootstage.c
> @@ -53,14 +53,23 @@ int bootstage_relocate(void)
>   {
>   	struct bootstage_data *data = gd->bootstage;
>   	int i;
> +	char *ptr;
> +
> +	/* Figure out where to relocate the strings to */
> +	ptr = (char *)(data + 1);
>
>   	/*
>   	 * Duplicate all strings.  They may point to an old location in the
>   	 * program .text section that can eventually get trashed.
>   	 */
>   	debug("Relocating %d records\n", data->rec_count);
> -	for (i = 0; i < data->rec_count; i++)
> -		data->record[i].name = strdup(data->record[i].name);
> +	for (i = 0; i < data->rec_count; i++) {
> +		const char *from = data->record[i].name;
> +
> +		strcpy(ptr, from);
> +		data->record[i].name = ptr;
> +		ptr += strlen(ptr) + 1;
> +	}
>
>   	return 0;
>   }
> @@ -490,7 +499,17 @@ int bootstage_unstash(const void *base, int size)
>
>   int bootstage_get_size(void)
>   {
> -	return sizeof(struct bootstage_data);
> +	struct bootstage_data *data = gd->bootstage;
> +	struct bootstage_record *rec;
> +	int size;
> +	int i;
> +
> +	size = sizeof(struct bootstage_data);
> +	for (rec = data->record, i = 0; i < data->rec_count;
> +	     i++, rec++)
> +		size += strlen(rec->name) + 1;
> +
> +	return size;
>   }
>
>   int bootstage_init(bool first)
>

[U-Boot] [PATCH v2 03/10] tiny-printf: Reorder code to support %p

[Faiz Abbas]

Hi Simon,

On 22/10/19 4:56 am, Simon Glass wrote:
> With a bit of code reordering we can support %p using the existing code
> for ulong.
> 
> Move the %p code up and adjust the logic accordingly.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 

This patch seems to have broken Ethernet boot in am335x-evm for me. It
seems to be caused by SPL not being able to set ethaddr variable but its
not obvious to me why this would cause it to happen. Here's a log:

Trying to boot from USB eth
## Error: flags type check failure for "ethaddr" <= "40309f20M" (type: m)
## Error inserting "ethaddr" variable, errno=1

Warning: eth_cpsw using MAC address from ROM
eth0: eth_cpsw## Error: flags type check failure for "eth1addr" <=
"81f01098M" (type: m)
## Error inserting "eth1addr" variable, errno=1

Warning: usb_ether using MAC address from ROM
, eth1: usb_ether
eth_cpsw Waiting for PHY auto negotiation to complete...... done
link up on port 0, speed 1000, full duplex
BOOTP broadcast 1
BOOTP broadcast 2
BOOTP broadcast 3
BOOTP broadcast 4
BOOTP broadcast 5
BOOTP broadcast 6
BOOTP broadcast 7
BOOTP broadcast 8
BOOTP broadcast 9
BOOTP broadcast 10
BOOTP broadcast 11
BOOTP broadcast 12
BOOTP broadcast 13
BOOTP broadcast 14
BOOTP broadcast 15
BOOTP broadcast 16
BOOTP broadcast 17

Retry time exceeded; starting again
Problem booting with BOOTP
SPL: failed to boot from all boot devices
### ERROR ### Please RESET the board ###

Reverting this patch on the latest U-boot master fixes the issue for me.

I'll look into this more deeply tomorrow. Let me know if you see
something obviously wrong with the patch.

Thanks,
Faiz

[U-Boot] [PATCH v2 03/10] tiny-printf: Reorder code to support %p

[Simon Glass]

Hi Faiz,

On Thu, 30 Jan 2020 at 08:22, Faiz Abbas <faiz_abbas@ti.com> wrote:
>
> Hi Simon,
>
> On 22/10/19 4:56 am, Simon Glass wrote:
> > With a bit of code reordering we can support %p using the existing code
> > for ulong.
> >
> > Move the %p code up and adjust the logic accordingly.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> >
>
> This patch seems to have broken Ethernet boot in am335x-evm for me. It
> seems to be caused by SPL not being able to set ethaddr variable but its
> not obvious to me why this would cause it to happen. Here's a log:
>
> Trying to boot from USB eth
> ## Error: flags type check failure for "ethaddr" <= "40309f20M" (type: m)
> ## Error inserting "ethaddr" variable, errno=1
>
> Warning: eth_cpsw using MAC address from ROM
> eth0: eth_cpsw## Error: flags type check failure for "eth1addr" <=
> "81f01098M" (type: m)
> ## Error inserting "eth1addr" variable, errno=1
>
> Warning: usb_ether using MAC address from ROM
> , eth1: usb_ether
> eth_cpsw Waiting for PHY auto negotiation to complete...... done
> link up on port 0, speed 1000, full duplex
> BOOTP broadcast 1
> BOOTP broadcast 2
> BOOTP broadcast 3
> BOOTP broadcast 4
> BOOTP broadcast 5
> BOOTP broadcast 6
> BOOTP broadcast 7
> BOOTP broadcast 8
> BOOTP broadcast 9
> BOOTP broadcast 10
> BOOTP broadcast 11
> BOOTP broadcast 12
> BOOTP broadcast 13
> BOOTP broadcast 14
> BOOTP broadcast 15
> BOOTP broadcast 16
> BOOTP broadcast 17
>
> Retry time exceeded; starting again
> Problem booting with BOOTP
> SPL: failed to boot from all boot devices
> ### ERROR ### Please RESET the board ###
>
> Reverting this patch on the latest U-boot master fixes the issue for me.
>
> I'll look into this more deeply tomorrow. Let me know if you see
> something obviously wrong with the patch.

Well one thing is that eth_env_set_enetaddr() called from the board's
board.c has this:

sprintf(buf, "%pM", enetaddr);

which is not supported with tiny-printf.

Before my patch it would probably produce an empty string, but now it
will produce garbage. Perhaps need an SPL check there.

Regards,
Simon

[U-Boot] [PATCH v2 03/10] tiny-printf: Reorder code to support %p

[Vignesh Raghavendra]

Hi Simon,

On 31/01/20 7:57 am, Simon Glass wrote:
> Hi Faiz,
> 
> On Thu, 30 Jan 2020 at 08:22, Faiz Abbas <faiz_abbas@ti.com> wrote:
>>
>> Hi Simon,
>>
>> On 22/10/19 4:56 am, Simon Glass wrote:
>>> With a bit of code reordering we can support %p using the existing code
>>> for ulong.
>>>
>>> Move the %p code up and adjust the logic accordingly.
>>>

[...]
>>
>> Retry time exceeded; starting again
>> Problem booting with BOOTP
>> SPL: failed to boot from all boot devices
>> ### ERROR ### Please RESET the board ###
>>
>> Reverting this patch on the latest U-boot master fixes the issue for me.
>>
>> I'll look into this more deeply tomorrow. Let me know if you see
>> something obviously wrong with the patch.
> 
> Well one thing is that eth_env_set_enetaddr() called from the board's
> board.c has this:
> 
> sprintf(buf, "%pM", enetaddr);
> 
> which is not supported with tiny-printf.

That is not true. %pM is supported when SPL_NET_SUPPORT is enabled. See:

https://gitlab.denx.de/u-boot/u-boot/blob/master/lib/tiny-printf.c#L183

I added this specifically to support Ethernet Boot usecases on TI platforms

But above commit seems to move pointer() function that formats the
output under #ifdef DEBUG which definitely breaks %pM

> 
> Before my patch it would probably produce an empty string, but now it
> will produce garbage. Perhaps need an SPL check there.
> 
> Regards,
> Simon
> 

-- 
Regards
Vignesh

[U-Boot] [PATCH v2 03/10] tiny-printf: Reorder code to support %p

[Simon Glass]

Hi Vignesh,

On Thu, 30 Jan 2020 at 22:12, Vignesh Raghavendra <vigneshr@ti.com> wrote:
>
> Hi Simon,
>
> On 31/01/20 7:57 am, Simon Glass wrote:
> > Hi Faiz,
> >
> > On Thu, 30 Jan 2020 at 08:22, Faiz Abbas <faiz_abbas@ti.com> wrote:
> >>
> >> Hi Simon,
> >>
> >> On 22/10/19 4:56 am, Simon Glass wrote:
> >>> With a bit of code reordering we can support %p using the existing code
> >>> for ulong.
> >>>
> >>> Move the %p code up and adjust the logic accordingly.
> >>>
>
> [...]
> >>
> >> Retry time exceeded; starting again
> >> Problem booting with BOOTP
> >> SPL: failed to boot from all boot devices
> >> ### ERROR ### Please RESET the board ###
> >>
> >> Reverting this patch on the latest U-boot master fixes the issue for me.
> >>
> >> I'll look into this more deeply tomorrow. Let me know if you see
> >> something obviously wrong with the patch.
> >
> > Well one thing is that eth_env_set_enetaddr() called from the board's
> > board.c has this:
> >
> > sprintf(buf, "%pM", enetaddr);
> >
> > which is not supported with tiny-printf.
>
> That is not true. %pM is supported when SPL_NET_SUPPORT is enabled. See:
>
> https://gitlab.denx.de/u-boot/u-boot/blob/master/lib/tiny-printf.c#L183
>
> I added this specifically to support Ethernet Boot usecases on TI platforms
>
> But above commit seems to move pointer() function that formats the
> output under #ifdef DEBUG which definitely breaks %pM

OK I see. I think it is too confusing to use #ifdef DEBUG in this code.

One fix would be to change pointer() to return true if it actually
does something. I'll take a look.

This code needs tests also. Vignesh, do you feel like writing something?


>
> >
> > Before my patch it would probably produce an empty string, but now it
> > will produce garbage. Perhaps need an SPL check there.

Regards,
Simon

[U-Boot] [PATCH v2 03/10] tiny-printf: Reorder code to support %p

[Vignesh Raghavendra]

Faiz,

On 31/01/20 11:44 pm, Simon Glass wrote:
> Hi Vignesh,
> 
> On Thu, 30 Jan 2020 at 22:12, Vignesh Raghavendra <vigneshr@ti.com> wrote:
>>
>> Hi Simon,
>>
>> On 31/01/20 7:57 am, Simon Glass wrote:
>>> Hi Faiz,
>>>
>>> On Thu, 30 Jan 2020 at 08:22, Faiz Abbas <faiz_abbas@ti.com> wrote:
>>>>
>>>> Hi Simon,
>>>>
>>>> On 22/10/19 4:56 am, Simon Glass wrote:
>>>>> With a bit of code reordering we can support %p using the existing code
>>>>> for ulong.
>>>>>
>>>>> Move the %p code up and adjust the logic accordingly.
>>>>>
>>
>> [...]
>>>>
>>>> Retry time exceeded; starting again
>>>> Problem booting with BOOTP
>>>> SPL: failed to boot from all boot devices
>>>> ### ERROR ### Please RESET the board ###
>>>>
>>>> Reverting this patch on the latest U-boot master fixes the issue for me.
>>>>
>>>> I'll look into this more deeply tomorrow. Let me know if you see
>>>> something obviously wrong with the patch.
>>>
>>> Well one thing is that eth_env_set_enetaddr() called from the board's
>>> board.c has this:
>>>
>>> sprintf(buf, "%pM", enetaddr);
>>>
>>> which is not supported with tiny-printf.
>>
>> That is not true. %pM is supported when SPL_NET_SUPPORT is enabled. See:
>>
>> https://gitlab.denx.de/u-boot/u-boot/blob/master/lib/tiny-printf.c#L183
>>
>> I added this specifically to support Ethernet Boot usecases on TI platforms
>>
>> But above commit seems to move pointer() function that formats the
>> output under #ifdef DEBUG which definitely breaks %pM
> 
> OK I see. I think it is too confusing to use #ifdef DEBUG in this code.
> 
> One fix would be to change pointer() to return true if it actually
> does something. I'll take a look.
> 
> This code needs tests also. Vignesh, do you feel like writing something?
> 

Is there a testcase for full printf()? I am not sure where to look for.
Is test/print_ut.c the right place to add new test?


-- 
Regards
Vignesh

[U-Boot] [PATCH v2 03/10] tiny-printf: Reorder code to support %p

[Simon Glass]

Hi Vignesh,

On Tue, 4 Feb 2020 at 00:58, Vignesh Raghavendra <vigneshr@ti.com> wrote:
>
> Faiz,
>
> On 31/01/20 11:44 pm, Simon Glass wrote:
> > Hi Vignesh,
> >
> > On Thu, 30 Jan 2020 at 22:12, Vignesh Raghavendra <vigneshr@ti.com> wrote:
> >>
> >> Hi Simon,
> >>
> >> On 31/01/20 7:57 am, Simon Glass wrote:
> >>> Hi Faiz,
> >>>
> >>> On Thu, 30 Jan 2020 at 08:22, Faiz Abbas <faiz_abbas@ti.com> wrote:
> >>>>
> >>>> Hi Simon,
> >>>>
> >>>> On 22/10/19 4:56 am, Simon Glass wrote:
> >>>>> With a bit of code reordering we can support %p using the existing code
> >>>>> for ulong.
> >>>>>
> >>>>> Move the %p code up and adjust the logic accordingly.
> >>>>>
> >>
> >> [...]
> >>>>
> >>>> Retry time exceeded; starting again
> >>>> Problem booting with BOOTP
> >>>> SPL: failed to boot from all boot devices
> >>>> ### ERROR ### Please RESET the board ###
> >>>>
> >>>> Reverting this patch on the latest U-boot master fixes the issue for me.
> >>>>
> >>>> I'll look into this more deeply tomorrow. Let me know if you see
> >>>> something obviously wrong with the patch.
> >>>
> >>> Well one thing is that eth_env_set_enetaddr() called from the board's
> >>> board.c has this:
> >>>
> >>> sprintf(buf, "%pM", enetaddr);
> >>>
> >>> which is not supported with tiny-printf.
> >>
> >> That is not true. %pM is supported when SPL_NET_SUPPORT is enabled. See:
> >>
> >> https://gitlab.denx.de/u-boot/u-boot/blob/master/lib/tiny-printf.c#L183
> >>
> >> I added this specifically to support Ethernet Boot usecases on TI platforms
> >>
> >> But above commit seems to move pointer() function that formats the
> >> output under #ifdef DEBUG which definitely breaks %pM
> >
> > OK I see. I think it is too confusing to use #ifdef DEBUG in this code.
> >
> > One fix would be to change pointer() to return true if it actually
> > does something. I'll take a look.
> >
> > This code needs tests also. Vignesh, do you feel like writing something?
> >
>
> Is there a testcase for full printf()? I am not sure where to look for.
> Is test/print_ut.c the right place to add new test?

Yes.

See also this commit in u-boot-dm/testing

    test: Add a way to check each line of console output

Regards,
Simon

[U-Boot] [PATCH v2 08/10] bootstage: Correct relocation algorithm

[Simon Glass]

Hi Heinrich,

On Sat, 25 Jan 2020 at 01:59, Heinrich Schuchardt <xypron.debian@gmx.de> wrote:
>
> On 10/22/19 1:26 AM, Simon Glass wrote:
> > At present bootstage relocation assumes that it is possible to point back
> > to memory available before relocation, so it does not relocate the
> > strings. However this is not the case on some platforms, such as x86 which
> > uses the cache as RAM and loses access to this when the cache is enabled.
> >
> > Move the relocation step to before U-Boot relocates, expand the allocated
> > region to include space for the strings and relocate the strings at the
> > same time as the bootstage records.
> >
> > This ensures that bootstage data can remain accessible from TPL through
> > SPL to U-Boot before/after relocation.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
>
> Hello Simon,
>
> this merged patch seems to be incorrect. I compiled sandbox_defconfig
> with clang and ran it with valgrind.
>
> We allocate memory in bootstage_init() for gd->bootstage. But from
> bootstage_get_size() we return a size that is larger than what we have
> allocated and use that larger memory area in reloc_bootstage(). See
> output below.

Yes that's right. This is a bit tricky.

The original malloc() does not include space for strings, since the
caller passes them in and we just use pointers.

When we relocate we copy the structure but then also write out the
strings after it.

The only obvious solution is to store the total size of the bootstage
record in the bootstage_data record, probably adding a version number
as well.

Regards,
Simon