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

[Patrick Georgi (Code Review)]

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

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 they do not have resource structure.

Remove the erroneous code related to resource structure.

Remove unnecessary function prototypes from header files, and define them
as static in hob_display.c.

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
Reviewed-on: https://review.coreboot.org/c/coreboot/+/43335
Reviewed-by: Christian Walter 
Reviewed-by: Angel Pons 
Tested-by: build bot (Jenkins) 
---
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(-)

Approvals:
  build bot (Jenkins): Verified
  Angel Pons: Looks good to me, approved
  Christian Walter: Looks good to me, approved

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