[coreboot-gerrit] Change in ...coreboot[master]: acpi: device: avoid empty property list in acpi_dp_write

[Patrick Georgi (Code Review)]

Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/31161 )

Change subject: acpi: device: avoid empty property list in acpi_dp_write
......................................................................

acpi: device: avoid empty property list in acpi_dp_write

If an acpi_dp table has children but no properties then acpi_dp_write()
will write out a properties UUID and package that contains no properties.
The existing function will avoid writing out a UUID and empty package
when no children exist, but it seems to assume that properties will
always be used.  With this change properties are handled in a manner
akin to children so that a UUID and package are only written if
properties exist.

BUG=none
BRANCH=none
TEST=Confirmed that prior to this change a UUID and empty package was
present for a device that had children but no properties.  Verified that
after this change the UUID and empty package are no longer present but
the child UUID and package are still present.

Change-Id: I6f5597713a1e91ca26b409f36b3ff9eb90a010af
Signed-off-by: Matt Delco 
Reviewed-on: https://review.coreboot.org/c/31161
Tested-by: build bot (Jenkins) 
Reviewed-by: Duncan Laurie 
Reviewed-by: Furquan Shaikh 
---
M src/arch/x86/acpi_device.c
1 file changed, 26 insertions(+), 16 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Duncan Laurie: Looks good to me, approved
  Furquan Shaikh: Looks good to me, approved

diff --git a/src/arch/x86/acpi_device.c b/src/arch/x86/acpi_device.c
index 48b7fee..c62d6f3 100644
--- a/src/arch/x86/acpi_device.c
+++ b/src/arch/x86/acpi_device.c
@@ -632,7 +632,7 @@
 void acpi_dp_write(struct acpi_dp *table)
 {
    struct acpi_dp *dp, *prop;
-	char *dp_count, *prop_count;
+	char *dp_count, *prop_count = NULL;
    int child_count = 0;

if (!table || table->type != ACPI_DP_TYPE_TABLE)
@@ -644,32 +644,42 @@
    /* Device Property list starts with the next entry */
    prop = table->next;

-	/* Package (DP), default to 2 elements (assuming no children) */
-	dp_count = acpigen_write_package(2);
-
-	/* ToUUID (ACPI_DP_UUID) */
-	acpigen_write_uuid(ACPI_DP_UUID);
-
-	/* Package (PROP), element count determined as it is populated */
-	prop_count = acpigen_write_package(0);
+	/* Package (DP), default to assuming no properties or children */
+	dp_count = acpigen_write_package(0);

/* Print base properties */
    for (dp = prop; dp; dp = dp->next) {
    	if (dp->type == ACPI_DP_TYPE_CHILD) {
    		child_count++;
    	} else {
+			/*
+			 * The UUID and package is only added when
+			 * we come across the first property.  This
+			 * is to avoid creating a zero-length package
+			 * in situations where there are only children.
+			 */
+			if (!prop_count) {
+				*dp_count += 2;
+				/* ToUUID (ACPI_DP_UUID) */
+				acpigen_write_uuid(ACPI_DP_UUID);
+				/*
+				 * Package (PROP), element count determined as
+				 * it is populated
+				 */
+				prop_count = acpigen_write_package(0);
+			}
    		(*prop_count)++;
    		acpi_dp_write_property(dp);
    	}
    }
-
-	/* Package (PROP) length */
-	acpigen_pop_len();
+	if (prop_count) {
+		/* Package (PROP) length, if a package was written */
+		acpigen_pop_len();
+	}

if (child_count) {
-		/* Update DP package count to 4 */
-		*dp_count = 4;
-
+		/* Update DP package count to 2 or 4 */
+		*dp_count += 2;
    	/* ToUUID (ACPI_DP_CHILD_UUID) */
    	acpigen_write_uuid(ACPI_DP_CHILD_UUID);

@@ -679,7 +689,7 @@
    	for (dp = prop; dp; dp = dp->next)
    		if (dp->type == ACPI_DP_TYPE_CHILD)
    			acpi_dp_write_property(dp);
-
+		/* Package (CHILD) length */
    	acpigen_pop_len();
    }