Change in coreboot[master]: console: change header when vboot starts before bootblock

[Kangheui Won (Code Review)]

Kangheui Won has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/46060 )

Change subject: console: change header when vboot starts before bootblock
......................................................................

console: change header when vboot starts before bootblock

VBOOT_STARTS_VEFORE_BOOTBLOCK indicates that verstage starts before
bootblock. However "cbmem -1" will first try to match "bootblock
starting" to find out the beginning of console for current boot.

In case "cbmem -1" cannot find "bootblock starting", it will fall back
to find "verstage starting". Replace "bootblock starting" to
"bootblock-after-verstage starting" in header to let cbmem utility
find for "verstage starting" for beginning of console.

BUG=b:159220781
TEST=flash and boot, check `cbmem -1`
BRANCH=zork

Signed-off-by: Kangheui Won 
Change-Id: Ica38f6bfeb05605caadac208e790fd072b352732
---
M src/console/init.c
1 file changed, 13 insertions(+), 3 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/60/46060/1

diff --git a/src/console/init.c b/src/console/init.c
index 1dba9ad..adf6239 100644
--- a/src/console/init.c
+++ b/src/console/init.c
@@ -74,7 +74,17 @@

console_inited = 1;

-	printk(BIOS_NOTICE, "\n\ncoreboot-%s%s %s " ENV_STRING " starting (log level: %i)...\n",
-	       coreboot_version, coreboot_extra_version, coreboot_build,
-	       get_log_level());
+	printk(BIOS_NOTICE, "\n\ncoreboot-%s%s %s " ENV_STRING,
+	       coreboot_version, coreboot_extra_version, coreboot_build);
+
+	/*
+	 * if verstage runs before bootblock and we're in bootblock,
+	 * append something to ENV_STRING not to be matched against
+	 * "bootblock starting". This will let cbmem utility to
+	 * look for "verstage starting" for start of the log.
+	 */
+	if (CONFIG(VBOOT_STARTS_BEFORE_BOOTBLOCK) && ENV_BOOTBLOCK)
+		printk(BIOS_NOTICE, "-after-verstage");
+
+	printk(BIOS_NOTICE, " starting (log level: %i)...\n", get_log_level());
 }

[Julius Werner (Code Review)]

Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46060 )

Change subject: console: change header when vboot starts before bootblock
......................................................................

Patch Set 1:

Isn't it easier to just change ENV_STRING in <rules.h> for this case? It is only used to print banners like this.

I would suggest renaming the verstage string in this case (i.e. verstage-before-bootblock or something) because that's really the odd one out, and then adding that to the banner regex list in CBMEM with higher priority than bootblock.

Change in coreboot[master]: rules.h: change verstage name if it starts before bootblock

[Kangheui Won (Code Review)]

Hello build bot (Jenkins), Martin Roth, Furquan Shaikh, Eric Peers, Rob Barnes, Aaron Durbin,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/46060

to look at the new patch set (#2).

Change subject: rules.h: change verstage name if it starts before bootblock
......................................................................

rules.h: change verstage name if it starts before bootblock

VBOOT_STARTS_VEFORE_BOOTBLOCK indicates that verstage starts before
bootblock. However "cbmem -1" will first try to match "bootblock
starting" to find out the beginning of console for current boot.

Change ENV_STRING for verstage to "verstage-before-bootblock" in the
case and add regex in cbmem utility to grab it.

BUG=b:159220781
TEST=flash and boot, check `cbmem -1`
BRANCH=zork

Signed-off-by: Kangheui Won 
Change-Id: Ica38f6bfeb05605caadac208e790fd072b352732
---
M src/include/rules.h
M util/cbmem/cbmem.c
2 files changed, 6 insertions(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/60/46060/2

Change in coreboot[master]: rules.h: change verstage name if it starts before bootblock

[build bot (Jenkins) (Code Review)]

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46060 )

Change subject: rules.h: change verstage name if it starts before bootblock
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/46060/2/util/cbmem/cbmem.c 
File util/cbmem/cbmem.c:

https://review.coreboot.org/c/coreboot/+/46060/2/util/cbmem/cbmem.c@785 
PS2, Line 785: 		const char *regex[] = { BANNER_REGEX("verstage-before-bootblock"),
char * array declaration might be better as static const

Change in coreboot[master]: rules.h: change verstage name if it starts before bootblock

[Kangheui Won (Code Review)]

Kangheui Won has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46060 )

Change subject: rules.h: change verstage name if it starts before bootblock
......................................................................

Patch Set 2:
> Patch Set 1:
>
> Isn't it easier to just change ENV_STRING in <rules.h> for this case? It is only used to print banners like this.
>
> I would suggest renaming the verstage string in this case (i.e. verstage-before-bootblock or something) because that's really the odd one out, and then adding that to the banner regex list in CBMEM with higher priority than bootblock.

I agree, thought I had to avoid changes in host utility but I was wrong.

Change in coreboot[master]: rules.h: change verstage name if it starts before bootblock

[Martin Roth (Code Review)]

Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46060 )

Change subject: rules.h: change verstage name if it starts before bootblock
......................................................................

Patch Set 2: Code-Review+2

Change in coreboot[master]: rules.h: change verstage name if it starts before bootblock

[Julius Werner (Code Review)]

Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46060 )

Change subject: rules.h: change verstage name if it starts before bootblock
......................................................................

Patch Set 2: Code-Review+2

Change in coreboot[master]: rules.h: change verstage name if it starts before bootblock

[Edward O'Callaghan (Code Review)]

Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46060 )

Change subject: rules.h: change verstage name if it starts before bootblock
......................................................................

Patch Set 2: Code-Review+2

Change in coreboot[master]: rules.h: change verstage name if it starts before bootblock

[Edward O'Callaghan (Code Review)]

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/coreboot/+/46060 )

Change subject: rules.h: change verstage name if it starts before bootblock
......................................................................

rules.h: change verstage name if it starts before bootblock

VBOOT_STARTS_VEFORE_BOOTBLOCK indicates that verstage starts before
bootblock. However "cbmem -1" will first try to match "bootblock
starting" to find out the beginning of console for current boot.

Change ENV_STRING for verstage to "verstage-before-bootblock" in the
case and add regex in cbmem utility to grab it.

BUG=b:159220781
TEST=flash and boot, check `cbmem -1`
BRANCH=zork

Signed-off-by: Kangheui Won 
Change-Id: Ica38f6bfeb05605caadac208e790fd072b352732
Reviewed-on: https://review.coreboot.org/c/coreboot/+/46060
Tested-by: build bot (Jenkins) 
Reviewed-by: Martin Roth 
Reviewed-by: Julius Werner 
Reviewed-by: Edward O'Callaghan 
---
M src/include/rules.h
M util/cbmem/cbmem.c
2 files changed, 6 insertions(+), 1 deletion(-)

Approvals:
  build bot (Jenkins): Verified
  Martin Roth: Looks good to me, approved
  Julius Werner: Looks good to me, approved
  Edward O'Callaghan: Looks good to me, approved

diff --git a/src/include/rules.h b/src/include/rules.h
index 39836b1..d30b2b8 100644
--- a/src/include/rules.h
+++ b/src/include/rules.h
@@ -78,7 +78,11 @@
 #define ENV_RMODULE 0
 #define ENV_POSTCAR 0
 #define ENV_LIBAGESA 0
+#if CONFIG(VBOOT_STARTS_BEFORE_BOOTBLOCK)
+#define ENV_STRING "verstage-before-bootblock"
+#else
 #define ENV_STRING "verstage"
+#endif

#elif defined(__RAMSTAGE__)
 #define ENV_DECOMPRESSOR 0
diff --git a/util/cbmem/cbmem.c b/util/cbmem/cbmem.c
index 34e79d3..3a94f3e 100644
--- a/util/cbmem/cbmem.c
+++ b/util/cbmem/cbmem.c
@@ -782,7 +782,8 @@
 #define BANNER_REGEX(stage) \
    	"\n\ncoreboot-[^\n]* " stage " starting.*\.\.\.\n"
 #define OVERFLOW_REGEX(stage) "\n\*\*\* Pre-CBMEM " stage " console overflow"
-		const char *regex[] = { BANNER_REGEX("bootblock"),
+		const char *regex[] = { BANNER_REGEX("verstage-before-bootblock"),
+					BANNER_REGEX("bootblock"),
    				BANNER_REGEX("verstage"),
    				OVERFLOW_REGEX("romstage"),
    				BANNER_REGEX("romstage"),