Change in coreboot[master]: libpayload: refactor fetching cbmem pointers

[Patrick Georgi (Code Review)]

Patrick Georgi has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/35756 )

Change subject: libpayload: refactor fetching cbmem pointers
......................................................................

libpayload: refactor fetching cbmem pointers

There's a recurring pattern of reading cbtable entries that point into
cbmem entries. Move that pattern into its own function.

Coccinelle patch used for this:
  @@
  identifier T, T2;
  expression TARGET;
  @@
  -struct cb_cbmem_tab *const T2 = (struct cb_cbmem_tab *)T;
  -TARGET = phys_to_virt(T2->cbmem_tab);
  +TARGET = get_cbmem_ptr(T);

Change-Id: I7bd4a7ad8baeeaebf0fa7d4b4de6dbc719bc781f
Signed-off-by: Patrick Georgi 
---
M payloads/libpayload/arch/x86/coreboot.c
M payloads/libpayload/include/coreboot_tables.h
M payloads/libpayload/libc/coreboot.c
3 files changed, 15 insertions(+), 14 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/56/35756/1

diff --git a/payloads/libpayload/arch/x86/coreboot.c b/payloads/libpayload/arch/x86/coreboot.c
index 3a9eb1a..07adee8 100644
--- a/payloads/libpayload/arch/x86/coreboot.c
+++ b/payloads/libpayload/arch/x86/coreboot.c
@@ -50,8 +50,7 @@

static void cb_parse_mrc_cache(void *ptr, struct sysinfo_t *info)
 {
-	struct cb_cbmem_tab *const cbmem = (struct cb_cbmem_tab *)ptr;
-	info->mrc_cache = phys_to_virt(cbmem->cbmem_tab);
+	info->mrc_cache = get_cbmem_ptr(ptr);
 }

int cb_parse_arch_specific(struct cb_record *rec, struct sysinfo_t *info)
diff --git a/payloads/libpayload/include/coreboot_tables.h b/payloads/libpayload/include/coreboot_tables.h
index c24d6ac..188f426 100644
--- a/payloads/libpayload/include/coreboot_tables.h
+++ b/payloads/libpayload/include/coreboot_tables.h
@@ -395,4 +395,6 @@
    (void *)(((u8 *) (_rec)) + sizeof(*(_rec)) \
    	+ (sizeof((_rec)->map[0]) * (_idx)))

+/* Helper functions */
+void *get_cbmem_ptr(unsigned char *ptr);
 #endif
diff --git a/payloads/libpayload/libc/coreboot.c b/payloads/libpayload/libc/coreboot.c
index e14b144..2ff2090 100644
--- a/payloads/libpayload/libc/coreboot.c
+++ b/payloads/libpayload/libc/coreboot.c
@@ -42,6 +42,12 @@
 /* === Parsing code === */
 /* This is the generic parsing code. */

+void *get_cbmem_ptr(unsigned char *ptr)
+{
+	struct cb_cbmem_tab *const cbmem = (struct cb_cbmem_tab *)ptr;
+	return phys_to_virt(cbmem->cbmem_tab);
+}
+
 static void cb_parse_memory(void *ptr, struct sysinfo_t *info)
 {
    struct cb_memory *mem = ptr;
@@ -128,20 +134,17 @@

static void cb_parse_tstamp(unsigned char *ptr, struct sysinfo_t *info)
 {
-	struct cb_cbmem_tab *const cbmem = (struct cb_cbmem_tab *)ptr;
-	info->tstamp_table = phys_to_virt(cbmem->cbmem_tab);
+	info->tstamp_table = get_cbmem_ptr(ptr);
 }

static void cb_parse_cbmem_cons(unsigned char *ptr, struct sysinfo_t *info)
 {
-	struct cb_cbmem_tab *const cbmem = (struct cb_cbmem_tab *)ptr;
-	info->cbmem_cons = phys_to_virt(cbmem->cbmem_tab);
+	info->cbmem_cons = get_cbmem_ptr(ptr);
 }

static void cb_parse_acpi_gnvs(unsigned char *ptr, struct sysinfo_t *info)
 {
-	struct cb_cbmem_tab *const cbmem = (struct cb_cbmem_tab *)ptr;
-	info->acpi_gnvs = phys_to_virt(cbmem->cbmem_tab);
+	info->acpi_gnvs = get_cbmem_ptr(ptr);
 }

static void cb_parse_board_id(unsigned char *ptr, struct sysinfo_t *info)
@@ -193,8 +196,7 @@

static void cb_parse_wifi_calibration(void *ptr, struct sysinfo_t *info)
 {
-	struct cb_cbmem_tab *const cbmem = (struct cb_cbmem_tab *)ptr;
-	info->wifi_calibration = phys_to_virt(cbmem->cbmem_tab);
+	info->wifi_calibration = get_cbmem_ptr(ptr);
 }

static void cb_parse_ramoops(void *ptr, struct sysinfo_t *info)
@@ -235,14 +237,12 @@

static void cb_parse_vpd(void *ptr, struct sysinfo_t *info)
 {
-	struct cb_cbmem_tab *const cbmem = (struct cb_cbmem_tab *)ptr;
-	info->chromeos_vpd = phys_to_virt(cbmem->cbmem_tab);
+	info->chromeos_vpd = get_cbmem_ptr(ptr);
 }

static void cb_parse_fmap_cache(void *ptr, struct sysinfo_t *info)
 {
-	struct cb_cbmem_tab *const cbmem = (struct cb_cbmem_tab *)ptr;
-	info->fmap_cache = phys_to_virt(cbmem->cbmem_tab);
+	info->fmap_cache = get_cbmem_ptr(ptr);
 }

#if CONFIG(LP_TIMER_RDTSC)

[Paul Menzel (Code Review)]

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35756 )

Change subject: libpayload: refactor fetching cbmem pointers
......................................................................

Patch Set 1: Code-Review+1

[Furquan Shaikh (Code Review)]

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35756 )

Change subject: libpayload: refactor fetching cbmem pointers
......................................................................

Patch Set 1: Code-Review+2

[Stefan Reinauer (Code Review)]

Stefan Reinauer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35756 )

Change subject: libpayload: refactor fetching cbmem pointers
......................................................................

Patch Set 1: Code-Review+2

[Patrick Georgi (Code Review)]

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

Change subject: libpayload: refactor fetching cbmem pointers
......................................................................

libpayload: refactor fetching cbmem pointers

There's a recurring pattern of reading cbtable entries that point into
cbmem entries. Move that pattern into its own function.

Coccinelle patch used for this:
  @@
  identifier T, T2;
  expression TARGET;
  @@
  -struct cb_cbmem_tab *const T2 = (struct cb_cbmem_tab *)T;
  -TARGET = phys_to_virt(T2->cbmem_tab);
  +TARGET = get_cbmem_ptr(T);

Change-Id: I7bd4a7ad8baeeaebf0fa7d4b4de6dbc719bc781f
Signed-off-by: Patrick Georgi 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/35756
Tested-by: build bot (Jenkins) 
Reviewed-by: Paul Menzel 
Reviewed-by: Furquan Shaikh 
Reviewed-by: Stefan Reinauer 
---
M payloads/libpayload/arch/x86/coreboot.c
M payloads/libpayload/include/coreboot_tables.h
M payloads/libpayload/libc/coreboot.c
3 files changed, 15 insertions(+), 14 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Stefan Reinauer: Looks good to me, approved
  Paul Menzel: Looks good to me, but someone else must approve
  Furquan Shaikh: Looks good to me, approved

diff --git a/payloads/libpayload/arch/x86/coreboot.c b/payloads/libpayload/arch/x86/coreboot.c
index 3a9eb1a..07adee8 100644
--- a/payloads/libpayload/arch/x86/coreboot.c
+++ b/payloads/libpayload/arch/x86/coreboot.c
@@ -50,8 +50,7 @@

static void cb_parse_mrc_cache(void *ptr, struct sysinfo_t *info)
 {
-	struct cb_cbmem_tab *const cbmem = (struct cb_cbmem_tab *)ptr;
-	info->mrc_cache = phys_to_virt(cbmem->cbmem_tab);
+	info->mrc_cache = get_cbmem_ptr(ptr);
 }

int cb_parse_arch_specific(struct cb_record *rec, struct sysinfo_t *info)
diff --git a/payloads/libpayload/include/coreboot_tables.h b/payloads/libpayload/include/coreboot_tables.h
index c24d6ac..188f426 100644
--- a/payloads/libpayload/include/coreboot_tables.h
+++ b/payloads/libpayload/include/coreboot_tables.h
@@ -395,4 +395,6 @@
    (void *)(((u8 *) (_rec)) + sizeof(*(_rec)) \
    	+ (sizeof((_rec)->map[0]) * (_idx)))

+/* Helper functions */
+void *get_cbmem_ptr(unsigned char *ptr);
 #endif
diff --git a/payloads/libpayload/libc/coreboot.c b/payloads/libpayload/libc/coreboot.c
index e14b144..2ff2090 100644
--- a/payloads/libpayload/libc/coreboot.c
+++ b/payloads/libpayload/libc/coreboot.c
@@ -42,6 +42,12 @@
 /* === Parsing code === */
 /* This is the generic parsing code. */

+void *get_cbmem_ptr(unsigned char *ptr)
+{
+	struct cb_cbmem_tab *const cbmem = (struct cb_cbmem_tab *)ptr;
+	return phys_to_virt(cbmem->cbmem_tab);
+}
+
 static void cb_parse_memory(void *ptr, struct sysinfo_t *info)
 {
    struct cb_memory *mem = ptr;
@@ -128,20 +134,17 @@

static void cb_parse_tstamp(unsigned char *ptr, struct sysinfo_t *info)
 {
-	struct cb_cbmem_tab *const cbmem = (struct cb_cbmem_tab *)ptr;
-	info->tstamp_table = phys_to_virt(cbmem->cbmem_tab);
+	info->tstamp_table = get_cbmem_ptr(ptr);
 }

static void cb_parse_cbmem_cons(unsigned char *ptr, struct sysinfo_t *info)
 {
-	struct cb_cbmem_tab *const cbmem = (struct cb_cbmem_tab *)ptr;
-	info->cbmem_cons = phys_to_virt(cbmem->cbmem_tab);
+	info->cbmem_cons = get_cbmem_ptr(ptr);
 }

static void cb_parse_acpi_gnvs(unsigned char *ptr, struct sysinfo_t *info)
 {
-	struct cb_cbmem_tab *const cbmem = (struct cb_cbmem_tab *)ptr;
-	info->acpi_gnvs = phys_to_virt(cbmem->cbmem_tab);
+	info->acpi_gnvs = get_cbmem_ptr(ptr);
 }

static void cb_parse_board_id(unsigned char *ptr, struct sysinfo_t *info)
@@ -193,8 +196,7 @@

static void cb_parse_wifi_calibration(void *ptr, struct sysinfo_t *info)
 {
-	struct cb_cbmem_tab *const cbmem = (struct cb_cbmem_tab *)ptr;
-	info->wifi_calibration = phys_to_virt(cbmem->cbmem_tab);
+	info->wifi_calibration = get_cbmem_ptr(ptr);
 }

static void cb_parse_ramoops(void *ptr, struct sysinfo_t *info)
@@ -235,14 +237,12 @@

static void cb_parse_vpd(void *ptr, struct sysinfo_t *info)
 {
-	struct cb_cbmem_tab *const cbmem = (struct cb_cbmem_tab *)ptr;
-	info->chromeos_vpd = phys_to_virt(cbmem->cbmem_tab);
+	info->chromeos_vpd = get_cbmem_ptr(ptr);
 }

static void cb_parse_fmap_cache(void *ptr, struct sysinfo_t *info)
 {
-	struct cb_cbmem_tab *const cbmem = (struct cb_cbmem_tab *)ptr;
-	info->fmap_cache = phys_to_virt(cbmem->cbmem_tab);
+	info->fmap_cache = get_cbmem_ptr(ptr);
 }

#if CONFIG(LP_TIMER_RDTSC)