Change in coreboot[master]: drivers/intel/fsp1_1/hob.c: Don't use broken FSP macros

[Arthur Heymans (Code Review)]

Attention is currently required from: Lee Leahy.
Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/63205 )

Change subject: drivers/intel/fsp1_1/hob.c: Don't use broken FSP macros
......................................................................

drivers/intel/fsp1_1/hob.c: Don't use broken FSP macros

This rewrites the code to not use FSP macros to loop over HOB
structures and also avoid unnecessary nested loop which just make the
code hard to read. This gives more insight in what is going on and
allows to use newer UDK headers where the macros have been rewritten
in invalid C code, most likely to avoid the 'const' qualifier. In
coreboot working around the 'const' qualifier also seems needed for
now so use union types for this.

Change-Id: Ic5fbde9666be461232c2d6af181e72c6fbcf7425
Signed-off-by: Arthur Heymans 
---
M src/drivers/intel/fsp1_1/hob.c
1 file changed, 49 insertions(+), 52 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/05/63205/1

diff --git a/src/drivers/intel/fsp1_1/hob.c b/src/drivers/intel/fsp1_1/hob.c
index 976df5d..2d210bb 100644
--- a/src/drivers/intel/fsp1_1/hob.c
+++ b/src/drivers/intel/fsp1_1/hob.c
@@ -23,40 +23,56 @@
    return hob_list;
 }

-/* Returns the next instance of a HOB type from the starting HOB. */
-static void *get_next_hob(uint16_t type, const void *hob_start)
+struct hob_header {
+	uint16_t type;
+	uint16_t length;
+	uint8_t guid[16];
+};
+
+/* TODO: It looks like we violate the const qualifier a lot so use an union to work
+   it for now. */
+union efi_hob {
+	uintptr_t addr;
+	const struct hob_header *header;
+};
+
+/* Returns the next instance of a HOB */
+static void *next_hob(const void *parent)
 {
-	EFI_PEI_HOB_POINTERS hob;
+	union efi_hob hob = { .header = parent } ;
+	return (void *)(hob.addr + hob.header->length);
+}

-	if (!hob_start)
-		return NULL;
+static bool hob_match_type(const void *hob, const uint16_t type)
+{
+	const struct hob_header *header = hob;
+	return header->type == type;
+}

-	hob.Raw = (UINT8 *)hob_start;
+static bool is_last_hob(const void *hob)
+{
+	return hob_match_type(hob, EFI_HOB_TYPE_END_OF_HOB_LIST);
+}

-	/* Parse the HOB list until end of list or matching type is found. */
-	while (!END_OF_HOB_LIST(hob.Raw)) {
-		if (hob.Header->HobType == type)
-			return hob.Raw;
-		if (GET_HOB_LENGTH(hob.Raw) < sizeof(*hob.Header))
+static void *get_type_guid_hob(const uint16_t type, const EFI_GUID *guid, const void *hob_start)
+{
+        union efi_hob hob = { .header = hob_start };
+
+	while (hob.header && !is_last_hob(hob.header)) {
+		if (hob_match_type(hob.header, type))
+			continue;
+
+		if (compare_guid(guid, (EFI_GUID *)hob.header->guid))
    		break;
-		hob.Raw = GET_NEXT_HOB(hob.Raw);
+		hob.header = next_hob(hob.header);
    }
-	return NULL;
+	return (void *)hob.addr;
 }

/* Returns the next instance of the matched GUID HOB from the starting HOB. */
 void *get_guid_hob(const EFI_GUID *guid, const void *hob_start)
 {
-	EFI_PEI_HOB_POINTERS hob;
-
-	hob.Raw = (uint8_t *)hob_start;
-	while ((hob.Raw = get_next_hob(EFI_HOB_TYPE_GUID_EXTENSION, hob.Raw))
-					!= NULL) {
-		if (compare_guid(guid, &hob.Guid->Name))
-			break;
-		hob.Raw = GET_NEXT_HOB(hob.Raw);
-	}
-	return hob.Raw;
+	return get_type_guid_hob(EFI_HOB_TYPE_GUID_EXTENSION, guid, hob_start);
 }

/*
@@ -64,16 +80,7 @@
  */
 void *get_resource_hob(const EFI_GUID *guid, const void *hob_start)
 {
-	EFI_PEI_HOB_POINTERS hob;
-
-	hob.Raw = (UINT8 *)hob_start;
-	while ((hob.Raw = get_next_hob(EFI_HOB_TYPE_RESOURCE_DESCRIPTOR,
-					    hob.Raw)) != NULL) {
-		if (compare_guid(guid, &hob.ResourceDescriptor->Owner))
-			break;
-		hob.Raw = GET_NEXT_HOB(hob.Raw);
-	}
-	return hob.Raw;
+	return get_type_guid_hob(EFI_HOB_TYPE_RESOURCE_DESCRIPTOR, guid, hob_start);
 }

static void print_hob_mem_attributes(void *hob_ptr)
@@ -234,13 +241,11 @@
  */
 void print_hob_type_structure(u16 hob_type, void *hob_list_ptr)
 {
-	u32 *current_hob;
-	u32 *next_hob = 0;
-	u8  last_hob = 0;
+	void *hob = NULL;;
    u32 current_type;
    const char *current_type_str;

-	current_hob = hob_list_ptr;
+	hob = hob_list_ptr;

/*
     * Print out HOBs of our desired type until
@@ -250,35 +255,27 @@
    printk(BIOS_DEBUG, "%p: hob_list_ptr\n", hob_list_ptr);
    do {
    	EFI_HOB_GENERIC_HEADER *current_header_ptr =
-			(EFI_HOB_GENERIC_HEADER *)current_hob;
+			(EFI_HOB_GENERIC_HEADER *)hob;

/* Get the type of this HOB */
    	current_type = current_header_ptr->HobType;
-		current_type_str = get_hob_type_string(current_hob);
+		current_type_str = get_hob_type_string(hob);

if (current_type == hob_type || hob_type == 0x0000) {
    		printk(BIOS_DEBUG, "HOB %p is an %s (type 0x%0x)\n",
-					current_hob, current_type_str,
+					hob, current_type_str,
    				current_type);
    		switch (current_type) {
    		case EFI_HOB_TYPE_MEMORY_ALLOCATION:
-				print_hob_mem_attributes(current_hob);
+				print_hob_mem_attributes(hob);
    			break;
    		case EFI_HOB_TYPE_RESOURCE_DESCRIPTOR:
-				print_hob_resource_attributes(current_hob);
+				print_hob_resource_attributes(hob);
    			break;
    		}
    	}

-		/* Check for end of HOB list */
-		last_hob = END_OF_HOB_LIST(current_hob);
-		if (!last_hob) {
-			/* Get next HOB pointer */
-			next_hob = GET_NEXT_HOB(current_hob);
-
-			/* Start on next HOB */
-			current_hob = next_hob;
-		}
-	} while (!last_hob);
+		hob = next_hob(hob);
+	} while (!is_last_hob(hob));
    printk(BIOS_DEBUG, "=== End of FSP HOB Data Structure ===\n\n");
 }