Change in coreboot[master]: soc/intel/broadwell: Add RMRRs after all DRHDs

[Angel Pons (Code Review)]

Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/44111 )

Change subject: soc/intel/broadwell: Add RMRRs after all DRHDs
......................................................................

soc/intel/broadwell: Add RMRRs after all DRHDs

The VT-d architecture specification (Doc. D51397-011, Rev. 3.1) says:

BIOS implementations must report these remapping structure types in
 numerical order. i.e., All remapping structures of type 0 (DRHD)
 enumerated before remapping structures of type 1 (RMRR), and so forth.

So, update the corresponding code to adhere to the specification.

Change-Id: Ib5ef5e006e590d72bec52e057e9b72150e0e636f
Signed-off-by: Angel Pons 
---
M src/soc/intel/broadwell/acpi.c
1 file changed, 18 insertions(+), 11 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/11/44111/1

diff --git a/src/soc/intel/broadwell/acpi.c b/src/soc/intel/broadwell/acpi.c
index c76c8d2..760842b 100644
--- a/src/soc/intel/broadwell/acpi.c
+++ b/src/soc/intel/broadwell/acpi.c
@@ -435,22 +435,19 @@
    const bool vtvc0en = MCHBAR32(VTVC0BAR) & 0x1;

/* iGFX has to be enabled; GFXVTBAR set, enabled, in 32-bit space */
-	if (igfx_dev && igfx_dev->enabled && gfxvtbar
-			&& gfxvten && !MCHBAR32(GFXVTBAR + 4)) {
-		unsigned long tmp = current;
+	const bool emit_igd =
+			igfx_dev && igfx_dev->enabled &&
+			gfxvtbar && gfxvten &&
+			!MCHBAR32(GFXVTBAR + 4);
+
+	/* First, add DRHD entries */
+	if (emit_igd) {
+		const unsigned long tmp = current;

current += acpi_create_dmar_drhd(current, 0, 0, gfxvtbar);
    	current += acpi_create_dmar_ds_pci(current, 0, 2, 0);

acpi_dmar_drhd_fixup(tmp, current);
-
-		/* Add RMRR entry */
-		tmp = current;
-
-		current += acpi_create_dmar_rmrr(current, 0,
-				sa_get_gsm_base(), sa_get_tolud_base() - 1);
-		current += acpi_create_dmar_ds_pci(current, 0, 2, 0);
-		acpi_dmar_rmrr_fixup(tmp, current);
    }

/* VTVC0BAR has to be set, enabled, and in 32-bit space */
@@ -468,6 +465,16 @@
    	acpi_dmar_drhd_fixup(tmp, current);
    }

+	/* Then, add RMRR entries after all DRHD entries */
+	if (emit_igd) {
+		const unsigned long tmp = current;
+
+		current += acpi_create_dmar_rmrr(current, 0,
+				sa_get_gsm_base(), sa_get_tolud_base() - 1);
+		current += acpi_create_dmar_ds_pci(current, 0, 2, 0);
+		acpi_dmar_rmrr_fixup(tmp, current);
+	}
+
    return current;
 }

[Patrick Rudolph (Code Review)]

Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44111 )

Change subject: soc/intel/broadwell: Add RMRRs after all DRHDs
......................................................................

Patch Set 1: Code-Review+2

[Angel Pons (Code Review)]

Angel Pons has submitted this change. ( https://review.coreboot.org/c/coreboot/+/44111 )

Change subject: soc/intel/broadwell: Add RMRRs after all DRHDs
......................................................................

soc/intel/broadwell: Add RMRRs after all DRHDs

The VT-d architecture specification (Doc. D51397-011, Rev. 3.1) says:

BIOS implementations must report these remapping structure types in
 numerical order. i.e., All remapping structures of type 0 (DRHD)
 enumerated before remapping structures of type 1 (RMRR), and so forth.

So, update the corresponding code to adhere to the specification.

Change-Id: Ib5ef5e006e590d72bec52e057e9b72150e0e636f
Signed-off-by: Angel Pons 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/44111
Tested-by: build bot (Jenkins) 
Reviewed-by: Patrick Rudolph 
---
M src/soc/intel/broadwell/acpi.c
1 file changed, 18 insertions(+), 11 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Patrick Rudolph: Looks good to me, approved

diff --git a/src/soc/intel/broadwell/acpi.c b/src/soc/intel/broadwell/acpi.c
index c76c8d2..760842b 100644
--- a/src/soc/intel/broadwell/acpi.c
+++ b/src/soc/intel/broadwell/acpi.c
@@ -435,22 +435,19 @@
    const bool vtvc0en = MCHBAR32(VTVC0BAR) & 0x1;

/* iGFX has to be enabled; GFXVTBAR set, enabled, in 32-bit space */
-	if (igfx_dev && igfx_dev->enabled && gfxvtbar
-			&& gfxvten && !MCHBAR32(GFXVTBAR + 4)) {
-		unsigned long tmp = current;
+	const bool emit_igd =
+			igfx_dev && igfx_dev->enabled &&
+			gfxvtbar && gfxvten &&
+			!MCHBAR32(GFXVTBAR + 4);
+
+	/* First, add DRHD entries */
+	if (emit_igd) {
+		const unsigned long tmp = current;

current += acpi_create_dmar_drhd(current, 0, 0, gfxvtbar);
    	current += acpi_create_dmar_ds_pci(current, 0, 2, 0);

acpi_dmar_drhd_fixup(tmp, current);
-
-		/* Add RMRR entry */
-		tmp = current;
-
-		current += acpi_create_dmar_rmrr(current, 0,
-				sa_get_gsm_base(), sa_get_tolud_base() - 1);
-		current += acpi_create_dmar_ds_pci(current, 0, 2, 0);
-		acpi_dmar_rmrr_fixup(tmp, current);
    }

/* VTVC0BAR has to be set, enabled, and in 32-bit space */
@@ -468,6 +465,16 @@
    	acpi_dmar_drhd_fixup(tmp, current);
    }

+	/* Then, add RMRR entries after all DRHD entries */
+	if (emit_igd) {
+		const unsigned long tmp = current;
+
+		current += acpi_create_dmar_rmrr(current, 0,
+				sa_get_gsm_base(), sa_get_tolud_base() - 1);
+		current += acpi_create_dmar_ds_pci(current, 0, 2, 0);
+		acpi_dmar_rmrr_fixup(tmp, current);
+	}
+
    return current;
 }