Change in coreboot[master]: soc/intel/xeon_sp: Fix skx NVME drive boot issue

[Marc Jones (Code Review)]

Marc Jones has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/47579 )

Change subject: soc/intel/xeon_sp: Fix skx NVME drive boot issue
......................................................................

soc/intel/xeon_sp: Fix skx NVME drive boot issue

Skx FSP doesn't support x2apic setup, but cpx does. The skx DMAR table needs
the X2API opt out flag set. This fixes an issue with the NVME driver loading
on a kexec'd kernel. The change is easy to see in the coreboot output:
[DMA Remapping table] Flags: 0x3
or in the DMAR ACPI table.

Change-Id: Iec977c893b70e30875d9a92f24af009c1e90389e
Signed-off-by: Marc Jones 
---
M src/soc/intel/xeon_sp/nb_acpi.c
1 file changed, 6 insertions(+), 2 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/79/47579/1

diff --git a/src/soc/intel/xeon_sp/nb_acpi.c b/src/soc/intel/xeon_sp/nb_acpi.c
index 55c3d82..1b28f2d 100644
--- a/src/soc/intel/xeon_sp/nb_acpi.c
+++ b/src/soc/intel/xeon_sp/nb_acpi.c
@@ -443,9 +443,13 @@
    if (config->vtd_support) {
    	current = ALIGN(current, 8);
    	dmar = (acpi_dmar_t *)current;
+		enum dmar_flags flags = DMAR_INTR_REMAP;
+#if (CONFIG(SOC_INTEL_SKYLAKE_SP))
+		flags |= DMAR_X2APIC_OPT_OUT;
+#endif
    	printk(BIOS_DEBUG, "ACPI:    * DMAR\n");
-		printk(BIOS_DEBUG, "[DMA Remapping table] Flags: 0x%x\n", DMAR_INTR_REMAP);
-		acpi_create_dmar(dmar, DMAR_INTR_REMAP, acpi_fill_dmar);
+		printk(BIOS_DEBUG, "[DMA Remapping table] Flags: 0x%x\n", flags);
+		acpi_create_dmar(dmar, flags, acpi_fill_dmar);
    	current += dmar->header.length;
    	current = acpi_align_current(current);
    	acpi_add_table(rsdp, dmar);

[Javier Galindo (Code Review)]

Javier Galindo has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47579 )

Change subject: soc/intel/xeon_sp: Fix skx NVME drive boot issue
......................................................................

Patch Set 1: Code-Review+1

[Jonathan Zhang (Code Review)]

Jonathan Zhang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47579 )

Change subject: soc/intel/xeon_sp: Fix skx NVME drive boot issue
......................................................................

Patch Set 1: Code-Review+2

Thanks for fixing this, Javier/Marc!

Change in coreboot[master]: soc/intel/xeon_sp: Fix skx SATA drive boot issue

[Marc Jones (Code Review)]

Hello Jonathan Zhang, Jay Talbott, Stefan Reinauer, Angel Pons, Arthur Heymans, Javier Galindo, Patrick Rudolph,

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#2).

Change subject: soc/intel/xeon_sp: Fix skx SATA drive boot issue
......................................................................

soc/intel/xeon_sp: Fix skx SATA drive boot issue

Skx FSP doesn't support x2apic setup, but cpx does. The skx DMAR
table needs the X2API opt out flag set. This fixes the hang loading
a kexec'd kernel. The change is easy to see in the coreboot output:
[DMA Remapping table] Flags: 0x3
or in the DMAR ACPI table.

Change-Id: Iec977c893b70e30875d9a92f24af009c1e90389e
Signed-off-by: Marc Jones 
---
M src/soc/intel/xeon_sp/nb_acpi.c
1 file changed, 6 insertions(+), 2 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/79/47579/2

Change in coreboot[master]: soc/intel/xeon_sp: Fix skx SATA drive boot issue

[Jay Talbott (Code Review)]

Jay Talbott has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47579 )

Change subject: soc/intel/xeon_sp: Fix skx SATA drive boot issue
......................................................................

Patch Set 2: Code-Review+1

Change in coreboot[master]: soc/intel/xeon_sp: Fix skx SATA drive boot issue

[Paul Menzel (Code Review)]

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47579 )

Change subject: soc/intel/xeon_sp: Fix skx SATA drive boot issue
......................................................................

Patch Set 2: Code-Review+1

(1 comment)

https://review.coreboot.org/c/coreboot/+/47579/2//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/47579/2//COMMIT_MSG@10 
PS2, Line 10: X2API
X2APIC

Change in coreboot[master]: soc/intel/xeon_sp: Fix skx SATA drive boot issue

[Arthur Heymans (Code Review)]

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47579 )

Change subject: soc/intel/xeon_sp: Fix skx SATA drive boot issue
......................................................................

Patch Set 2: Code-Review+1

(1 comment)

https://review.coreboot.org/c/coreboot/+/47579/2/src/soc/intel/xeon_sp/nb_ac... 
File src/soc/intel/xeon_sp/nb_acpi.c:

https://review.coreboot.org/c/coreboot/+/47579/2/src/soc/intel/xeon_sp/nb_ac... 
PS2, Line 447: #if
Can you do this with C code and add a comment?

Change in coreboot[master]: soc/intel/xeon_sp: Fix skx SATA drive boot issue

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47579 )

Change subject: soc/intel/xeon_sp: Fix skx SATA drive boot issue
......................................................................

Patch Set 2: Code-Review+1

(5 comments)

https://review.coreboot.org/c/coreboot/+/47579/2//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/47579/2//COMMIT_MSG@9 
PS2, Line 9: cpx
CPX

https://review.coreboot.org/c/coreboot/+/47579/2//COMMIT_MSG@9 
PS2, Line 9: Skx
I'd use uppercase: SKX

https://review.coreboot.org/c/coreboot/+/47579/2//COMMIT_MSG@9 
PS2, Line 9: x2apic
I'd use uppercase: X2APIC

https://review.coreboot.org/c/coreboot/+/47579/2//COMMIT_MSG@9 
PS2, Line 9: skx
SKX

https://review.coreboot.org/c/coreboot/+/47579/2/src/soc/intel/xeon_sp/nb_ac... 
File src/soc/intel/xeon_sp/nb_acpi.c:

https://review.coreboot.org/c/coreboot/+/47579/2/src/soc/intel/xeon_sp/nb_ac... 
PS2, Line 447: #if
> Can you do this with C code and add a comment?

Definitely:

/* SKX FSP doesn't support X2APIC, but CPX FSP does */
 if (CONFIG(SOC_INTEL_SKYLAKE_SP))
    flags |= DMAR_X2APIC_OPT_OUT;

Change in coreboot[master]: soc/intel/xeon_sp: Fix SKX SATA drive boot issue

[Marc Jones (Code Review)]

Hello build bot (Jenkins), Jonathan Zhang, Jay Talbott, Johnny Lin, Paul Menzel, Stefan Reinauer, Angel Pons, Arthur Heymans, Javier Galindo, Patrick Rudolph,

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#3).

Change subject: soc/intel/xeon_sp: Fix SKX SATA drive boot issue
......................................................................

soc/intel/xeon_sp: Fix SKX SATA drive boot issue

SkX FSP doesn't support X2APIC setup, but CPX does. The CPX DMAR
table needs the X2API opt out flag set. This fixes the hang loading
a kexec'd kernel. The change is easy to see in the coreboot output:
[DMA Remapping table] Flags: 0x3
or in the DMAR ACPI table.

Change-Id: Iec977c893b70e30875d9a92f24af009c1e90389e
Signed-off-by: Marc Jones 
---
M src/soc/intel/xeon_sp/nb_acpi.c
1 file changed, 8 insertions(+), 2 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/79/47579/3

Change in coreboot[master]: soc/intel/xeon_sp: Fix SKX SATA drive boot issue

[Marc Jones (Code Review)]

Marc Jones has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47579 )

Change subject: soc/intel/xeon_sp: Fix SKX SATA drive boot issue
......................................................................

Patch Set 3:

(6 comments)

https://review.coreboot.org/c/coreboot/+/47579/2//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/47579/2//COMMIT_MSG@9 
PS2, Line 9: cpx
> CPX

Done

https://review.coreboot.org/c/coreboot/+/47579/2//COMMIT_MSG@9 
PS2, Line 9: x2apic
> I'd use uppercase: X2APIC

Done

https://review.coreboot.org/c/coreboot/+/47579/2//COMMIT_MSG@9 
PS2, Line 9: skx
> SKX

Done

https://review.coreboot.org/c/coreboot/+/47579/2//COMMIT_MSG@9 
PS2, Line 9: Skx
> I'd use uppercase: SKX

Done

https://review.coreboot.org/c/coreboot/+/47579/2//COMMIT_MSG@10 
PS2, Line 10: X2API
> X2APIC

Done

https://review.coreboot.org/c/coreboot/+/47579/2/src/soc/intel/xeon_sp/nb_ac... 
File src/soc/intel/xeon_sp/nb_acpi.c:

https://review.coreboot.org/c/coreboot/+/47579/2/src/soc/intel/xeon_sp/nb_ac... 
PS2, Line 447: #if
> Definitely: […]

Done

Change in coreboot[master]: soc/intel/xeon_sp: Fix SKX SATA drive boot issue

[Stefan Reinauer (Code Review)]

Stefan Reinauer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47579 )

Change subject: soc/intel/xeon_sp: Fix SKX SATA drive boot issue
......................................................................

Patch Set 3: Code-Review+2

Change in coreboot[master]: soc/intel/xeon_sp: Fix SKX SATA drive boot issue

[Jay Talbott (Code Review)]

Jay Talbott has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47579 )

Change subject: soc/intel/xeon_sp: Fix SKX SATA drive boot issue
......................................................................

Patch Set 3:

(2 comments)

https://review.coreboot.org/c/coreboot/+/47579/2//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/47579/2//COMMIT_MSG@10 
PS2, Line 10: X2API
> Done

Not Done.

https://review.coreboot.org/c/coreboot/+/47579/3//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/47579/3//COMMIT_MSG@9 
PS3, Line 9: SkX
SKX

Change in coreboot[master]: soc/intel/xeon_sp: Fix SKX SATA drive boot issue

[Marc Jones (Code Review)]

Hello build bot (Jenkins), Jonathan Zhang, Jay Talbott, Johnny Lin, Stefan Reinauer, Paul Menzel, Angel Pons, Arthur Heymans, Javier Galindo, Patrick Rudolph,

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#4).

Change subject: soc/intel/xeon_sp: Fix SKX SATA drive boot issue
......................................................................

soc/intel/xeon_sp: Fix SKX SATA drive boot issue

SKX FSP doesn't support X2APIC setup, but CPX does. The CPX DMAR
table needs the X2APIC opt out flag set. This fixes the hang loading
a kexec'd kernel. The change is easy to see in the coreboot output:
[DMA Remapping table] Flags: 0x3
or in the DMAR ACPI table.

Change-Id: Iec977c893b70e30875d9a92f24af009c1e90389e
Signed-off-by: Marc Jones 
---
M src/soc/intel/xeon_sp/nb_acpi.c
1 file changed, 8 insertions(+), 2 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/79/47579/4

Change in coreboot[master]: soc/intel/xeon_sp: Fix SKX SATA drive boot issue

[Marc Jones (Code Review)]

Marc Jones has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47579 )

Change subject: soc/intel/xeon_sp: Fix SKX SATA drive boot issue
......................................................................

Patch Set 4:

(2 comments)

https://review.coreboot.org/c/coreboot/+/47579/2//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/47579/2//COMMIT_MSG@10 
PS2, Line 10: X2API
> Not Done.

Done

https://review.coreboot.org/c/coreboot/+/47579/3//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/47579/3//COMMIT_MSG@9 
PS3, Line 9: SkX
> SKX

Done

Change in coreboot[master]: soc/intel/xeon_sp: Fix SKX SATA drive boot issue

[Marc Jones (Code Review)]

Marc Jones has submitted this change. ( https://review.coreboot.org/c/coreboot/+/47579 )

Change subject: soc/intel/xeon_sp: Fix SKX SATA drive boot issue
......................................................................

soc/intel/xeon_sp: Fix SKX SATA drive boot issue

SKX FSP doesn't support X2APIC setup, but CPX does. The CPX DMAR
table needs the X2APIC opt out flag set. This fixes the hang loading
a kexec'd kernel. The change is easy to see in the coreboot output:
[DMA Remapping table] Flags: 0x3
or in the DMAR ACPI table.

Change-Id: Iec977c893b70e30875d9a92f24af009c1e90389e
Signed-off-by: Marc Jones 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/47579
Tested-by: build bot (Jenkins) 
Reviewed-by: Stefan Reinauer 
---
M src/soc/intel/xeon_sp/nb_acpi.c
1 file changed, 8 insertions(+), 2 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Stefan Reinauer: Looks good to me, approved

diff --git a/src/soc/intel/xeon_sp/nb_acpi.c b/src/soc/intel/xeon_sp/nb_acpi.c
index 4a44980..6d6eb00 100644
--- a/src/soc/intel/xeon_sp/nb_acpi.c
+++ b/src/soc/intel/xeon_sp/nb_acpi.c
@@ -434,9 +434,15 @@
    if (config->vtd_support) {
    	current = ALIGN(current, 8);
    	dmar = (acpi_dmar_t *)current;
+		enum dmar_flags flags = DMAR_INTR_REMAP;
+
+		/* SKX FSP doesn't support X2APIC, but CPX FSP does */
+		if (CONFIG(SOC_INTEL_SKYLAKE_SP))
+			flags |= DMAR_X2APIC_OPT_OUT;
+
    	printk(BIOS_DEBUG, "ACPI:    * DMAR\n");
-		printk(BIOS_DEBUG, "[DMA Remapping table] Flags: 0x%x\n", DMAR_INTR_REMAP);
-		acpi_create_dmar(dmar, DMAR_INTR_REMAP, acpi_fill_dmar);
+		printk(BIOS_DEBUG, "[DMA Remapping table] Flags: 0x%x\n", flags);
+		acpi_create_dmar(dmar, flags, acpi_fill_dmar);
    	current += dmar->header.length;
    	current = acpi_align_current(current);
    	acpi_add_table(rsdp, dmar);

Change in coreboot[master]: soc/intel/xeon_sp: Fix SKX SATA drive boot issue

[Jay Talbott (Code Review)]

Jay Talbott has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47579 )

Change subject: soc/intel/xeon_sp: Fix SKX SATA drive boot issue
......................................................................

Patch Set 5: Code-Review+1