[coreboot-gerrit] Change in coreboot[master]: soc/intel/xeon_sp/cpx: use HOB_TYPE_GUID_EXTENSION to interpret platf...

[Jonathan Zhang (Code Review)]

Hello Anjaneya "Reddy" Chagam,

I'd like you to do a code review. Please visit

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

to review the following change.

Change subject: soc/intel/xeon_sp/cpx: use HOB_TYPE_GUID_EXTENSION to interpret platform HOBs
......................................................................

soc/intel/xeon_sp/cpx: use HOB_TYPE_GUID_EXTENSION to interpret platform HOBs

Platform HOBs (in particular IIO_UDS and MemoryMap HOBs) are of HOB type
HOB_TYPE_GUID_EXTENSION, therefore it does not have resource structure.

Remove the errorneous code related to resource structure.

Remove unnecessary function prototypes from heade files, and define them
as static.

Since we have the HOB pointer, there is not need to search HOB by GUID.
Remove unnecessary calling of fsp_find_extension_hob_by_guid().

Signed-off-by: Jonathan Zhang 
Signed-off-by: Reddy Chagam 
Change-Id: Ib99bce39e6eb2aeb95242dfba36774653bbe91fd
---
M src/soc/intel/xeon_sp/cpx/hob_display.c
M src/soc/intel/xeon_sp/cpx/include/soc/soc_util.h
M src/vendorcode/intel/fsp/fsp2_0/cooperlake_sp/hob_memmap.h
3 files changed, 25 insertions(+), 40 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/35/43335/1

diff --git a/src/soc/intel/xeon_sp/cpx/hob_display.c b/src/soc/intel/xeon_sp/cpx/hob_display.c
index d5910c3..0263472 100644
--- a/src/soc/intel/xeon_sp/cpx/hob_display.c
+++ b/src/soc/intel/xeon_sp/cpx/hob_display.c
@@ -33,37 +33,14 @@
    return NULL;
 }

-void soc_display_hob(const struct hob_header *hob)
+static void soc_display_memmap_hob(const struct SystemMemoryMapHob *hob_addr)
 {
-	const struct hob_resource *res;
+	struct SystemMemoryMapHob *hob = (struct SystemMemoryMapHob *)hob_addr;

-	res = fsp_hob_header_to_resource(hob);
-	assert(res != NULL);
-	printk(BIOS_DEBUG, "\tResource type: 0x%x, attribute: 0x%x, addr: 0x%08llx, len: 0x%08llx\n",
-		res->type, res->attribute_type, res->addr, res->length);
-	printk(BIOS_DEBUG, "\tOwner GUID: ");
-	fsp_print_guid(res->owner_guid);
-	printk(BIOS_DEBUG, " (%s)\n", fsp_get_guid_name(res->owner_guid));
+	printk(BIOS_DEBUG, "================== MEMORY MAP HOB DATA ==================\n");
+	printk(BIOS_DEBUG, "hob: %p, structure size: 0x%lx\n",
+		hob, sizeof(*hob));

-	if (fsp_guid_compare(res->owner_guid, fsp_hob_iio_uds_guid))
-		soc_display_iio_universal_data_hob();
-	else if (fsp_guid_compare(res->owner_guid, fsp_hob_memmap_guid))
-		soc_display_memmap_hob();
-}
-
-void soc_display_memmap_hob(void)
-{
-	size_t hob_size;
-	const struct SystemMemoryMapHob *hob;
-	const uint8_t mem_hob_guid[16] = FSP_SYSTEM_MEMORYMAP_HOB_GUID;
-
-	hob = (const struct SystemMemoryMapHob *)fsp_find_extension_hob_by_guid(
-		mem_hob_guid, &hob_size);
-	assert(hob != NULL);
-
-	printk(BIOS_DEBUG, "===================== MEMORY MAP HOB DATA =====================\n");
-	printk(BIOS_DEBUG, "hob: %p, size: 0x%lx, structure size: 0x%lx\n",
-		hob, hob_size, sizeof(*hob));
    printk(BIOS_DEBUG, "\tlowMemBase: 0x%x, lowMemSize: 0x%x, highMemBase: 0x%x, "
    	"highMemSize: 0x%x\n",
    	hob->lowMemBase, hob->lowMemSize, hob->highMemBase, hob->highMemSize);
@@ -82,17 +59,11 @@
    hexdump(hob, sizeof(*hob));
 }

-void soc_display_iio_universal_data_hob(void)
+static void soc_display_iio_universal_data_hob(const IIO_UDS *hob)
 {
-	size_t hob_size;
-	const IIO_UDS *hob;
-	const uint8_t fsp_hob_iio_universal_data_guid[16] = FSP_HOB_IIO_UNIVERSAL_DATA_GUID;
-
-	hob = (const IIO_UDS *)fsp_find_extension_hob_by_guid(
-		fsp_hob_iio_universal_data_guid, &hob_size);
-	assert(hob != NULL);
-
    printk(BIOS_DEBUG, "===================== IIO_UDS HOB DATA =====================\n");
+	printk(BIOS_DEBUG, "hob: %p, structure size: 0x%lx\n",
+		hob, sizeof(*hob));

printk(BIOS_DEBUG, "\t===================== SYSTEM STATUS =====================\n");
    printk(BIOS_DEBUG, "\tnumCpus: 0x%x\n", hob->SystemStatus.numCpus);
@@ -194,4 +165,21 @@
    			iio_resource.PcieInfo.PortInfo[p].Function);
    	}
    }
+
+	hexdump(hob, sizeof(*hob));
+}
+
+void soc_display_hob(const struct hob_header *hob)
+{
+	uint8_t *guid;
+
+	if (hob->type != HOB_TYPE_GUID_EXTENSION)
+		return;
+
+	guid = (uint8_t *) fsp_hob_header_to_resource(hob);
+
+	if (fsp_guid_compare(guid, fsp_hob_iio_uds_guid))
+		soc_display_iio_universal_data_hob((const IIO_UDS *)(guid + 16));
+	else if (fsp_guid_compare(guid, fsp_hob_memmap_guid))
+		soc_display_memmap_hob((const struct SystemMemoryMapHob *)(guid + 16));
 }
diff --git a/src/soc/intel/xeon_sp/cpx/include/soc/soc_util.h b/src/soc/intel/xeon_sp/cpx/include/soc/soc_util.h
index d9ef24d..412730b 100644
--- a/src/soc/intel/xeon_sp/cpx/include/soc/soc_util.h
+++ b/src/soc/intel/xeon_sp/cpx/include/soc/soc_util.h
@@ -42,6 +42,5 @@
 const struct SystemMemoryMapHob *get_system_memory_map(void);

void set_bios_init_completion(void);
-void soc_display_iio_universal_data_hob(void);

#endif /* _SOC_UTIL_H_ */
diff --git a/src/vendorcode/intel/fsp/fsp2_0/cooperlake_sp/hob_memmap.h b/src/vendorcode/intel/fsp/fsp2_0/cooperlake_sp/hob_memmap.h
index ee86a6d..df7787a 100644
--- a/src/vendorcode/intel/fsp/fsp2_0/cooperlake_sp/hob_memmap.h
+++ b/src/vendorcode/intel/fsp/fsp2_0/cooperlake_sp/hob_memmap.h
@@ -109,6 +109,4 @@

#pragma pack()

-void soc_display_memmap_hob(void);
-
 #endif