Change in coreboot[master]: lib/coreboot_table: Add CBMEM_ID_VBOOT_WORKBUF pointer to coreboot table

[Yu-Ping Wu (Code Review)]

Yu-Ping Wu has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37234 )

Change subject: lib/coreboot_table: Add CBMEM_ID_VBOOT_WORKBUF pointer to coreboot table
......................................................................

lib/coreboot_table: Add CBMEM_ID_VBOOT_WORKBUF pointer to coreboot table

Since struct vb2_shared_data already contains workbuf_size and
vboot_workbuf_size is never used in depthcharge, remove it from struct
sysinfo_t. In addition, remove lb_vboot_workbuf() and add
CBMEM_ID_VBOOT_WORKBUF pointer to coreboot table with
add_cbmem_pointers(). Parsing of coreboot table in libpayload is
modified accordingly.

BRANCH=none
BUG=chromium:1021452
TEST=emerge-nami coreboot libpayload depthcharge; Akali booted correctly

Change-Id: I890df3ff93fa44ed6d3f9ad05f9c6e49780a8ecb
Signed-off-by: Yu-Ping Wu 
---
M payloads/libpayload/include/sysinfo.h
M payloads/libpayload/libc/coreboot.c
M src/lib/coreboot_table.c
3 files changed, 4 insertions(+), 27 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/34/37234/1

diff --git a/payloads/libpayload/include/sysinfo.h b/payloads/libpayload/include/sysinfo.h
index c05be7c..4b929f1 100644
--- a/payloads/libpayload/include/sysinfo.h
+++ b/payloads/libpayload/include/sysinfo.h
@@ -96,7 +96,6 @@
    struct cb_mainboard *mainboard;

void *vboot_workbuf;
-	uint32_t vboot_workbuf_size;

#if CONFIG(LP_ARCH_X86)
    int x86_rom_var_mtrr_index;
diff --git a/payloads/libpayload/libc/coreboot.c b/payloads/libpayload/libc/coreboot.c
index 2ff2090..f6e9379 100644
--- a/payloads/libpayload/libc/coreboot.c
+++ b/payloads/libpayload/libc/coreboot.c
@@ -86,10 +86,7 @@

static void cb_parse_vboot_workbuf(unsigned char *ptr, struct sysinfo_t *info)
 {
-	struct lb_range *vbwb = (struct lb_range *)ptr;
-
-	info->vboot_workbuf = (void *)(uintptr_t)vbwb->range_start;
-	info->vboot_workbuf_size = vbwb->range_size;
+	info->vboot_workbuf = get_cbmem_ptr(ptr);
 }

static void cb_parse_vbnv(unsigned char *ptr, struct sysinfo_t *info)
diff --git a/src/lib/coreboot_table.c b/src/lib/coreboot_table.c
index 8b18dfb..2083b2a 100644
--- a/src/lib/coreboot_table.c
+++ b/src/lib/coreboot_table.c
@@ -218,23 +218,6 @@
 }
 #endif /* CONFIG_CHROMEOS */

-static void lb_vboot_workbuf(struct lb_header *header)
-{
-	struct lb_range *vbwb;
-	void *wb = vboot_get_workbuf();
-
-	vbwb = (struct lb_range *)lb_new_record(header);
-	vbwb->tag = LB_TAG_VBOOT_WORKBUF;
-	vbwb->size = sizeof(*vbwb);
-	vbwb->range_start = (uintptr_t)wb;
-	/*
-	 * TODO(chromium:1021452): Since cbmem size of vboot workbuf is now
-	 * always a known value, we hardcode the value of range_size here.
-	 * Ultimately we'll want to move this to add_cbmem_pointers() below.
-	 */
-	vbwb->range_size = VB2_KERNEL_WORKBUF_RECOMMENDED_SIZE;
-}
-
 __weak uint32_t board_id(void) { return UNDEFINED_STRAPPING_ID; }
 __weak uint32_t ram_code(void) { return UNDEFINED_STRAPPING_ID; }
 __weak uint32_t sku_id(void) { return UNDEFINED_STRAPPING_ID; }
@@ -349,6 +332,9 @@
    	{CBMEM_ID_WIFI_CALIBRATION, LB_TAG_WIFI_CALIBRATION},
    	{CBMEM_ID_TCPA_LOG, LB_TAG_TCPA_LOG},
    	{CBMEM_ID_FMAP, LB_TAG_FMAP},
+#if CONFIG(VBOOT)
+		{CBMEM_ID_VBOOT_WORKBUF, LB_TAG_VBOOT_WORKBUF},
+#endif
    };
    int i;

@@ -558,11 +544,6 @@
    lb_vbnv(head);
 #endif

-	if (CONFIG(VBOOT)) {
-		/* pass along the vboot workbuf address. */
-		lb_vboot_workbuf(head);
-	}
-
    /* Add strapping IDs if available */
    lb_board_id(head);
    lb_ram_code(head);

[Joel Kitching (Code Review)]

Joel Kitching has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37234 )

Change subject: lib/coreboot_table: Add CBMEM_ID_VBOOT_WORKBUF pointer to coreboot table
......................................................................

Patch Set 1: Code-Review+2

[Julius Werner (Code Review)]

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

Change subject: lib/coreboot_table: Add CBMEM_ID_VBOOT_WORKBUF pointer to coreboot table
......................................................................

Patch Set 2:

(2 comments)

https://review.coreboot.org/c/coreboot/+/37234/2/src/lib/coreboot_table.c 
File src/lib/coreboot_table.c:

https://review.coreboot.org/c/coreboot/+/37234/2/src/lib/coreboot_table.c@a2... 
PS2, Line 224: vboot_get_workbuf
You should now make vboot_get_workbuf local (static) to its file because it is no longer needed anywhere else (and there shouldn't be any need to access the workbuffer directly in the future).

https://review.coreboot.org/c/coreboot/+/37234/2/src/lib/coreboot_table.c@33... 
PS2, Line 335: #if CONFIG(VBOOT)
I don't think you need to guard this, other Kconfig-dependent stuff (e.g. TIMESTAMP) isn't guarded either. It will just omit the pointer if it doesn't find it.

[Yu-Ping Wu (Code Review)]

Hello Aaron Durbin, Julius Werner, build bot (Jenkins), Joel Kitching,

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

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

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

Change subject: lib/coreboot_table: Add CBMEM_ID_VBOOT_WORKBUF pointer to coreboot table
......................................................................

lib/coreboot_table: Add CBMEM_ID_VBOOT_WORKBUF pointer to coreboot table

Since struct vb2_shared_data already contains workbuf_size and
vboot_workbuf_size is never used in depthcharge, remove it from struct
sysinfo_t. In addition, remove lb_vboot_workbuf() and add
CBMEM_ID_VBOOT_WORKBUF pointer to coreboot table with
add_cbmem_pointers(). Parsing of coreboot table in libpayload is
modified accordingly.

BRANCH=none
BUG=chromium:1021452
TEST=emerge-nami coreboot libpayload depthcharge; Akali booted correctly

Change-Id: I890df3ff93fa44ed6d3f9ad05f9c6e49780a8ecb
Signed-off-by: Yu-Ping Wu 
---
M payloads/libpayload/include/sysinfo.h
M payloads/libpayload/libc/coreboot.c
M src/lib/coreboot_table.c
M src/security/vboot/common.c
M src/security/vboot/misc.h
5 files changed, 3 insertions(+), 29 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/34/37234/3

[Yu-Ping Wu (Code Review)]

Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37234 )

Change subject: lib/coreboot_table: Add CBMEM_ID_VBOOT_WORKBUF pointer to coreboot table
......................................................................

Patch Set 3:

(2 comments)

https://review.coreboot.org/c/coreboot/+/37234/2/src/lib/coreboot_table.c 
File src/lib/coreboot_table.c:

https://review.coreboot.org/c/coreboot/+/37234/2/src/lib/coreboot_table.c@a2... 
PS2, Line 224: vboot_get_workbuf
> You should now make vboot_get_workbuf local (static) to its file because it is no longer needed anyw […]

Done

https://review.coreboot.org/c/coreboot/+/37234/2/src/lib/coreboot_table.c@33... 
PS2, Line 335: #if CONFIG(VBOOT)
> I don't think you need to guard this, other Kconfig-dependent stuff (e.g. […]

Done

[Joel Kitching (Code Review)]

Joel Kitching has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37234 )

Change subject: lib/coreboot_table: Add CBMEM_ID_VBOOT_WORKBUF pointer to coreboot table
......................................................................

Patch Set 3: Code-Review+2

[Julius Werner (Code Review)]

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

Change subject: lib/coreboot_table: Add CBMEM_ID_VBOOT_WORKBUF pointer to coreboot table
......................................................................

Patch Set 3: Code-Review+2

[Patrick Georgi (Code Review)]

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/37234 )

Change subject: lib/coreboot_table: Add CBMEM_ID_VBOOT_WORKBUF pointer to coreboot table
......................................................................

lib/coreboot_table: Add CBMEM_ID_VBOOT_WORKBUF pointer to coreboot table

Since struct vb2_shared_data already contains workbuf_size and
vboot_workbuf_size is never used in depthcharge, remove it from struct
sysinfo_t. In addition, remove lb_vboot_workbuf() and add
CBMEM_ID_VBOOT_WORKBUF pointer to coreboot table with
add_cbmem_pointers(). Parsing of coreboot table in libpayload is
modified accordingly.

BRANCH=none
BUG=chromium:1021452
TEST=emerge-nami coreboot libpayload depthcharge; Akali booted correctly

Change-Id: I890df3ff93fa44ed6d3f9ad05f9c6e49780a8ecb
Signed-off-by: Yu-Ping Wu 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/37234
Tested-by: build bot (Jenkins) 
Reviewed-by: Joel Kitching 
Reviewed-by: Julius Werner 
---
M payloads/libpayload/include/sysinfo.h
M payloads/libpayload/libc/coreboot.c
M src/lib/coreboot_table.c
M src/security/vboot/common.c
M src/security/vboot/misc.h
5 files changed, 3 insertions(+), 29 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Julius Werner: Looks good to me, approved
  Joel Kitching: Looks good to me, approved

diff --git a/payloads/libpayload/include/sysinfo.h b/payloads/libpayload/include/sysinfo.h
index c05be7c..4b929f1 100644
--- a/payloads/libpayload/include/sysinfo.h
+++ b/payloads/libpayload/include/sysinfo.h
@@ -96,7 +96,6 @@
    struct cb_mainboard *mainboard;

void *vboot_workbuf;
-	uint32_t vboot_workbuf_size;

#if CONFIG(LP_ARCH_X86)
    int x86_rom_var_mtrr_index;
diff --git a/payloads/libpayload/libc/coreboot.c b/payloads/libpayload/libc/coreboot.c
index 2ff2090..f6e9379 100644
--- a/payloads/libpayload/libc/coreboot.c
+++ b/payloads/libpayload/libc/coreboot.c
@@ -86,10 +86,7 @@

static void cb_parse_vboot_workbuf(unsigned char *ptr, struct sysinfo_t *info)
 {
-	struct lb_range *vbwb = (struct lb_range *)ptr;
-
-	info->vboot_workbuf = (void *)(uintptr_t)vbwb->range_start;
-	info->vboot_workbuf_size = vbwb->range_size;
+	info->vboot_workbuf = get_cbmem_ptr(ptr);
 }

static void cb_parse_vbnv(unsigned char *ptr, struct sysinfo_t *info)
diff --git a/src/lib/coreboot_table.c b/src/lib/coreboot_table.c
index 8b18dfb..af9f659 100644
--- a/src/lib/coreboot_table.c
+++ b/src/lib/coreboot_table.c
@@ -218,23 +218,6 @@
 }
 #endif /* CONFIG_CHROMEOS */

-static void lb_vboot_workbuf(struct lb_header *header)
-{
-	struct lb_range *vbwb;
-	void *wb = vboot_get_workbuf();
-
-	vbwb = (struct lb_range *)lb_new_record(header);
-	vbwb->tag = LB_TAG_VBOOT_WORKBUF;
-	vbwb->size = sizeof(*vbwb);
-	vbwb->range_start = (uintptr_t)wb;
-	/*
-	 * TODO(chromium:1021452): Since cbmem size of vboot workbuf is now
-	 * always a known value, we hardcode the value of range_size here.
-	 * Ultimately we'll want to move this to add_cbmem_pointers() below.
-	 */
-	vbwb->range_size = VB2_KERNEL_WORKBUF_RECOMMENDED_SIZE;
-}
-
 __weak uint32_t board_id(void) { return UNDEFINED_STRAPPING_ID; }
 __weak uint32_t ram_code(void) { return UNDEFINED_STRAPPING_ID; }
 __weak uint32_t sku_id(void) { return UNDEFINED_STRAPPING_ID; }
@@ -349,6 +332,7 @@
    	{CBMEM_ID_WIFI_CALIBRATION, LB_TAG_WIFI_CALIBRATION},
    	{CBMEM_ID_TCPA_LOG, LB_TAG_TCPA_LOG},
    	{CBMEM_ID_FMAP, LB_TAG_FMAP},
+		{CBMEM_ID_VBOOT_WORKBUF, LB_TAG_VBOOT_WORKBUF},
    };
    int i;

@@ -558,11 +542,6 @@
    lb_vbnv(head);
 #endif

-	if (CONFIG(VBOOT)) {
-		/* pass along the vboot workbuf address. */
-		lb_vboot_workbuf(head);
-	}
-
    /* Add strapping IDs if available */
    lb_board_id(head);
    lb_ram_code(head);
diff --git a/src/security/vboot/common.c b/src/security/vboot/common.c
index 517a1d4..c21fe15 100644
--- a/src/security/vboot/common.c
+++ b/src/security/vboot/common.c
@@ -27,7 +27,7 @@

static struct vb2_context *vboot_ctx;

-void *vboot_get_workbuf(void)
+static void *vboot_get_workbuf(void)
 {
    void *wb = NULL;

diff --git a/src/security/vboot/misc.h b/src/security/vboot/misc.h
index 9dd482e..d03e76e 100644
--- a/src/security/vboot/misc.h
+++ b/src/security/vboot/misc.h
@@ -25,7 +25,6 @@
 /*
  * Source: security/vboot/common.c
  */
-void *vboot_get_workbuf(void);
 struct vb2_context *vboot_get_context(void);

/*