Change in coreboot[master]: vendorcode/eltan/security: Add missing vb2ex_printf()

[Frans Hendriks (Code Review)]

Frans Hendriks has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/34666 )

Change subject: vendorcode/eltan/security: Add missing vb2ex_printf()
......................................................................

vendorcode/eltan/security: Add missing vb2ex_printf()

Build error on missing vb2ex_printf().
Add the file cb_vb2.c which contains the missing vb2ex_printf().

BUG=N/A
TEST=Boot Linux 4.20 and verify logging on Facebook FBG-1701

Change-Id: I3f649f3faf1e812d592e4981bc75698e2cad1cc8
Signed-off-by: Frans Hendriks 
---
M src/vendorcode/eltan/security/lib/Makefile.inc
A src/vendorcode/eltan/security/lib/cb_vb2.c
2 files changed, 37 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/66/34666/1

diff --git a/src/vendorcode/eltan/security/lib/Makefile.inc b/src/vendorcode/eltan/security/lib/Makefile.inc
index 5ef1bca..6fdab11 100644
--- a/src/vendorcode/eltan/security/lib/Makefile.inc
+++ b/src/vendorcode/eltan/security/lib/Makefile.inc
@@ -41,18 +41,22 @@
 ifneq ($(filter y,$(CONFIG_VENDORCODE_ELTAN_VBOOT) $(CONFIG_VENDORCODE_ELTAN_MBOOT)),)

bootblock-$(CONFIG_C_ENVIRONMENT_BOOTBLOCK) += cb_sha.c
+bootblock-y += cb_vb2.c
 $(eval $(call vendor-security-lib,bootblock))
 bootblock-srcs  += $(obj)/external/ven_sec_lib-bootblock/vboot_fw21.a

postcar-y += cb_sha.c
+postcar-y += cb_vb2.c
 $(eval $(call vendor-security-lib,postcar))
 postcar-srcs += $(obj)/external/ven_sec_lib-postcar/vboot_fw21.a

ramstage-y += cb_sha.c
+ramstage-y += cb_vb2.c
 $(eval $(call vendor-security-lib,ramstage))
 ramstage-srcs += $(obj)/external/ven_sec_lib-ramstage/vboot_fw21.a

romstage-y += cb_sha.c
+romstage-y += cb_vb2.c
 $(eval $(call vendor-security-lib,romstage))
 romstage-srcs += $(obj)/external/ven_sec_lib-romstage/vboot_fw21.a

diff --git a/src/vendorcode/eltan/security/lib/cb_vb2.c b/src/vendorcode/eltan/security/lib/cb_vb2.c
new file mode 100644
index 0000000..16c972e
--- /dev/null
+++ b/src/vendorcode/eltan/security/lib/cb_vb2.c
@@ -0,0 +1,33 @@
+/*
+ * This file is part of the coreboot project.
+ *
+ * Copyright 2014 Google Inc.
+ * Copyright 2019 Eltan B.V.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <console/console.h>
+#include <vb2_api.h>
+
+/* exports */
+void vb2ex_printf(const char *func, const char *fmt, ...)
+{
+	va_list args;
+
+	if (func)
+		printk(BIOS_INFO, "VB2:%s() ", func);
+
+	va_start(args, fmt);
+	vprintk(BIOS_INFO, fmt, args);
+	va_end(args);
+
+	return;
+}

Change in coreboot[master]: vendorcode/eltan/security/lib: Add missing vb2ex_printf()

[Frans Hendriks (Code Review)]

Hello Patrick Georgi, Martin Roth,

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

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

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

Change subject: vendorcode/eltan/security/lib: Add missing vb2ex_printf()
......................................................................

vendorcode/eltan/security/lib: Add missing vb2ex_printf()

Build error on missing vb2ex_printf().
Add the file cb_vb2.c which contains the missing vb2ex_printf().

BUG=N/A
TEST=Boot Linux 4.20 and verify logging on Facebook FBG-1701

Change-Id: I3f649f3faf1e812d592e4981bc75698e2cad1cc8
Signed-off-by: Frans Hendriks 
---
M src/vendorcode/eltan/security/lib/Makefile.inc
A src/vendorcode/eltan/security/lib/cb_vb2.c
2 files changed, 37 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/66/34666/2

Change in coreboot[master]: vendorcode/eltan/security/lib: Add missing vb2ex_printf()

[Lance Zhao (Code Review)]

Lance Zhao has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34666 )

Change subject: vendorcode/eltan/security/lib: Add missing vb2ex_printf()
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/34666/2/src/vendorcode/eltan/securi... 
File src/vendorcode/eltan/security/lib/cb_vb2.c:

https://review.coreboot.org/c/coreboot/+/34666/2/src/vendorcode/eltan/securi... 
PS2, Line 21: vb2ex_printf
Will that be redundant to src/security/vboot/vboot_logic.c?

Change in coreboot[master]: vendorcode/eltan/security/lib: Add missing vb2ex_printf()

[Christian Walter (Code Review)]

Christian Walter has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34666 )

Change subject: vendorcode/eltan/security/lib: Add missing vb2ex_printf()
......................................................................

Patch Set 2: Code-Review-1

Why do you need that code? And why do you just copy it over? And why do you have to add this into all stages? I can"t see this print get used anywhere..

Change in coreboot[master]: vendorcode/eltan/security/lib: Add missing vb2ex_printf()

[Frans Hendriks (Code Review)]

Frans Hendriks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34666 )

Change subject: vendorcode/eltan/security/lib: Add missing vb2ex_printf()
......................................................................

Patch Set 2:
> Patch Set 2: Code-Review-1
>
> Why do you need that code? And why do you just copy it over? And why do you have to add this into all stages? I can"t see this print get used anywhere.

Function vb2_rsa_verify_digest (file 3rdparty/vboot/firmware/2lib/include/2rsa.c) calls VB2_DEBUG. This VB2_DEBUG is defined as vb2ex_printf. 
Did a retry and noticed that this function is required for bootblock stage only.

Change in coreboot[master]: vendorcode/eltan/security/lib: Add missing vb2ex_printf()

[Frans Hendriks (Code Review)]

Frans Hendriks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34666 )

Change subject: vendorcode/eltan/security/lib: Add missing vb2ex_printf()
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/34666/2/src/vendorcode/eltan/securi... 
File src/vendorcode/eltan/security/lib/cb_vb2.c:

https://review.coreboot.org/c/coreboot/+/34666/2/src/vendorcode/eltan/securi... 
PS2, Line 21: vb2ex_printf
> Will that be redundant to src/security/vboot/vboot_logic. […]

Yes. My assumption was that including the vboot_logic.c would cause build errors, due too missing other includes.
Did some tests and noticed that using this vboot_logic.c (for bootblock only) works fine.

Change in coreboot[master]: vendorcode/eltan/security/lib: Add missing vb2ex_printf()

[Christian Walter (Code Review)]

Christian Walter has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34666 )

Change subject: vendorcode/eltan/security/lib: Add missing vb2ex_printf()
......................................................................

Patch Set 2: -Code-Review
> Patch Set 2:
> > Patch Set 2: Code-Review-1
> >
> > Why do you need that code? And why do you just copy it over? And why do you have to add this into all stages? I can"t see this print get used anywhere.
>
> Function vb2_rsa_verify_digest (file 3rdparty/vboot/firmware/2lib/include/2rsa.c) calls VB2_DEBUG. This VB2_DEBUG is defined as vb2ex_printf. 
> Did a retry and noticed that this function is required for bootblock stage only.

Okay - then you might want to remove it form the other stages. Can't you use the function which is already in coreboot for this? I guess because you are doing your own implementation of VBOOT this might be a bit tricky.

Change in coreboot[master]: vendorcode/eltan/security/lib: Add missing vb2ex_printf()

[Frans Hendriks (Code Review)]

Hello Patrick Rudolph, Felix Held, Christian Walter, Paul Menzel, Lance Zhao, Philipp Deppenwiese, build bot (Jenkins), Patrick Georgi, Martin Roth,

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

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

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

Change subject: vendorcode/eltan/security/lib: Add missing vb2ex_printf()
......................................................................

vendorcode/eltan/security/lib: Add missing vb2ex_printf()

Build error on missing vb2ex_printf() in bootblock stage
Add the file vboot_logic.c which contains the missing vb2ex_printf().

BUG=N/A
TEST=Boot Linux 4.20 and verify logging on Facebook FBG-1701

Change-Id: I3f649f3faf1e812d592e4981bc75698e2cad1cc8
Signed-off-by: Frans Hendriks 
---
M src/vendorcode/eltan/security/lib/Makefile.inc
1 file changed, 1 insertion(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/66/34666/3

Change in coreboot[master]: vendorcode/eltan/security/lib: Add missing vb2ex_printf()

[Lance Zhao (Code Review)]

Lance Zhao has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34666 )

Change subject: vendorcode/eltan/security/lib: Add missing vb2ex_printf()
......................................................................

Patch Set 3: Code-Review+2

Change in coreboot[master]: vendorcode/eltan/security/lib: Add missing vb2ex_printf()

[Martin Roth (Code Review)]

Martin Roth has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/34666 )

Change subject: vendorcode/eltan/security/lib: Add missing vb2ex_printf()
......................................................................

vendorcode/eltan/security/lib: Add missing vb2ex_printf()

Build error on missing vb2ex_printf() in bootblock stage
Add the file vboot_logic.c which contains the missing vb2ex_printf().

BUG=N/A
TEST=Boot Linux 4.20 and verify logging on Facebook FBG-1701

Change-Id: I3f649f3faf1e812d592e4981bc75698e2cad1cc8
Signed-off-by: Frans Hendriks 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/34666
Tested-by: build bot (Jenkins) 
Reviewed-by: Lance Zhao 
---
M src/vendorcode/eltan/security/lib/Makefile.inc
1 file changed, 1 insertion(+), 0 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Lance Zhao: Looks good to me, approved

diff --git a/src/vendorcode/eltan/security/lib/Makefile.inc b/src/vendorcode/eltan/security/lib/Makefile.inc
index 5ef1bca..1f74759 100644
--- a/src/vendorcode/eltan/security/lib/Makefile.inc
+++ b/src/vendorcode/eltan/security/lib/Makefile.inc
@@ -41,6 +41,7 @@
 ifneq ($(filter y,$(CONFIG_VENDORCODE_ELTAN_VBOOT) $(CONFIG_VENDORCODE_ELTAN_MBOOT)),)

bootblock-$(CONFIG_C_ENVIRONMENT_BOOTBLOCK) += cb_sha.c
+bootblock-y += ../../../../security/vboot/vboot_logic.c
 $(eval $(call vendor-security-lib,bootblock))
 bootblock-srcs  += $(obj)/external/ven_sec_lib-bootblock/vboot_fw21.a