Change in ...coreboot[master]: soc/amd/stoneyridge: Generate MCFG table

[Raul Rangel (Code Review)]

Raul Rangel has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32445

Change subject: soc/amd/stoneyridge: Generate MCFG table
......................................................................

soc/amd/stoneyridge: Generate MCFG table

BUG=b:948241
TEST=Booted and decompiled the table
[000h 0000   4]                    Signature : "MCFG"
[004h 0004   4]                 Table Length : 0000003C
[008h 0008   1]                     Revision : 01
[009h 0009   1]                     Checksum : 15
[00Ah 0010   6]                       Oem ID : "COREv4"
[010h 0016   8]                 Oem Table ID : "COREBOOT"
[018h 0024   4]                 Oem Revision : 00000000
[01Ch 0028   4]              Asl Compiler ID : "CORE"
[020h 0032   4]        Asl Compiler Revision : 00000000

[024h 0036   8]                     Reserved : 0000000000000000

[02Ch 0044   8]                 Base Address : 00000000F8000000
[034h 0052   2]         Segment Group Number : 0000
[036h 0054   1]             Start Bus Number : 00
[037h 0055   1]               End Bus Number : 40
[038h 0056   4]                     Reserved : 00000000

Change-Id: I46dc1959971af4685a7ffd285429175d6882ae86
Signed-off-by: Raul E Rangel 
---
M src/soc/amd/stoneyridge/lpc.c
1 file changed, 7 insertions(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/45/32445/1

diff --git a/src/soc/amd/stoneyridge/lpc.c b/src/soc/amd/stoneyridge/lpc.c
index efc948d..8151b04 100644
--- a/src/soc/amd/stoneyridge/lpc.c
+++ b/src/soc/amd/stoneyridge/lpc.c
@@ -329,7 +329,13 @@

unsigned long acpi_fill_mcfg(unsigned long current)
 {
-	/* Just a dummy */
+
+	current += acpi_create_mcfg_mmconfig((acpi_mcfg_mmconfig_t *)current,
+					     CONFIG_MMCONF_BASE_ADDRESS,
+					     0,
+					     0,
+					     CONFIG_MMCONF_BUS_NUMBER);
+
    return current;
 }

[Edward Hill (Code Review)]

Edward Hill has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32445 )

Change subject: soc/amd/stoneyridge: Generate MCFG table
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/#/c/32445/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/#/c/32445/1//COMMIT_MSG@9 
PS1, Line 9: b:948241
crbug, not b

[Raul Rangel (Code Review)]

Hello Marshall Dawson, Martin Roth,

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

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

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

Change subject: soc/amd/stoneyridge: Generate MCFG table
......................................................................

soc/amd/stoneyridge: Generate MCFG table

BUG=crbug:948241
TEST=Booted and decompiled the table
[000h 0000   4]                    Signature : "MCFG"
[004h 0004   4]                 Table Length : 0000003C
[008h 0008   1]                     Revision : 01
[009h 0009   1]                     Checksum : 15
[00Ah 0010   6]                       Oem ID : "COREv4"
[010h 0016   8]                 Oem Table ID : "COREBOOT"
[018h 0024   4]                 Oem Revision : 00000000
[01Ch 0028   4]              Asl Compiler ID : "CORE"
[020h 0032   4]        Asl Compiler Revision : 00000000

[024h 0036   8]                     Reserved : 0000000000000000

[02Ch 0044   8]                 Base Address : 00000000F8000000
[034h 0052   2]         Segment Group Number : 0000
[036h 0054   1]             Start Bus Number : 00
[037h 0055   1]               End Bus Number : 40
[038h 0056   4]                     Reserved : 00000000

Change-Id: I46dc1959971af4685a7ffd285429175d6882ae86
Signed-off-by: Raul E Rangel 
---
M src/soc/amd/stoneyridge/lpc.c
1 file changed, 7 insertions(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/45/32445/2

[Raul Rangel (Code Review)]

Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32445 )

Change subject: soc/amd/stoneyridge: Generate MCFG table
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/#/c/32445/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/#/c/32445/1//COMMIT_MSG@9 
PS1, Line 9: b:948241
> crbug, not b

oops

[HAOUAS Elyes (Code Review)]

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32445 )

Change subject: soc/amd/stoneyridge: Generate MCFG table
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/#/c/32445/2//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/#/c/32445/2//COMMIT_MSG@19 
PS2, Line 19: Asl Compiler Revision : 00000000
Please, why "Asl Compiler Revision" is = to 00000000 ?

shouldn't be = asl_revision; ?

[Raul Rangel (Code Review)]

Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32445 )

Change subject: soc/amd/stoneyridge: Generate MCFG table
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/#/c/32445/2//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/#/c/32445/2//COMMIT_MSG@19 
PS2, Line 19: Asl Compiler Revision : 00000000
> Please, why "Asl Compiler Revision" is = to 00000000 ? […]

The revision isn't set: https://github.com/coreboot/coreboot/blob/master/src/arch/x86/acpi.c#L254
Most things only set it for the fadt table.

[Marshall Dawson (Code Review)]

Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32445 )

Change subject: soc/amd/stoneyridge: Generate MCFG table
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/#/c/32445/2//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/#/c/32445/2//COMMIT_MSG@19 
PS2, Line 19: Asl Compiler Revision : 00000000
> The revision isn't set: https://github.com/coreboot/coreboot/blob/master/src/arch/x86/acpi.c#L254 […]

I'm confused.  It's line 256 of the same file you cited that ought to be setting the revision in the header.  And when I run with your change then decompile, I see the revision correctly reported as 20190108.

[HAOUAS Elyes (Code Review)]

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32445 )

Change subject: soc/amd/stoneyridge: Generate MCFG table
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/#/c/32445/2//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/#/c/32445/2//COMMIT_MSG@19 
PS2, Line 19: Asl Compiler Revision : 00000000
> I'm confused. […]

indeed, it should report the revision
( https://review.coreboot.org/c/31607 )

[Raul Rangel (Code Review)]

Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32445 )

Change subject: soc/amd/stoneyridge: Generate MCFG table
......................................................................

Patch Set 2:
> Patch Set 2:
>
> (1 comment)

Ah, that patch was not in the firmware branch I was using.

[HAOUAS Elyes (Code Review)]

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32445 )

Change subject: soc/amd/stoneyridge: Generate MCFG table
......................................................................

Patch Set 2: Code-Review+1

[Marshall Dawson (Code Review)]

Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32445 )

Change subject: soc/amd/stoneyridge: Generate MCFG table
......................................................................

Patch Set 2: Code-Review+2

[Martin Roth (Code Review)]

Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32445 )

Change subject: soc/amd/stoneyridge: Generate MCFG table
......................................................................

Patch Set 2: Code-Review+2

[Martin Roth (Code Review)]

Martin Roth has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/32445 )

Change subject: soc/amd/stoneyridge: Generate MCFG table
......................................................................

soc/amd/stoneyridge: Generate MCFG table

BUG=crbug:948241
TEST=Booted and decompiled the table
[000h 0000   4]                    Signature : "MCFG"
[004h 0004   4]                 Table Length : 0000003C
[008h 0008   1]                     Revision : 01
[009h 0009   1]                     Checksum : 15
[00Ah 0010   6]                       Oem ID : "COREv4"
[010h 0016   8]                 Oem Table ID : "COREBOOT"
[018h 0024   4]                 Oem Revision : 00000000
[01Ch 0028   4]              Asl Compiler ID : "CORE"
[020h 0032   4]        Asl Compiler Revision : 00000000

[024h 0036   8]                     Reserved : 0000000000000000

[02Ch 0044   8]                 Base Address : 00000000F8000000
[034h 0052   2]         Segment Group Number : 0000
[036h 0054   1]             Start Bus Number : 00
[037h 0055   1]               End Bus Number : 40
[038h 0056   4]                     Reserved : 00000000

Change-Id: I46dc1959971af4685a7ffd285429175d6882ae86
Signed-off-by: Raul E Rangel 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/32445
Tested-by: build bot (Jenkins) 
Reviewed-by: HAOUAS Elyes 
Reviewed-by: Marshall Dawson 
Reviewed-by: Martin Roth 
---
M src/soc/amd/stoneyridge/lpc.c
1 file changed, 7 insertions(+), 1 deletion(-)

Approvals:
  build bot (Jenkins): Verified
  Martin Roth: Looks good to me, approved
  HAOUAS Elyes: Looks good to me, but someone else must approve
  Marshall Dawson: Looks good to me, approved

diff --git a/src/soc/amd/stoneyridge/lpc.c b/src/soc/amd/stoneyridge/lpc.c
index e3475ee..87da9f1 100644
--- a/src/soc/amd/stoneyridge/lpc.c
+++ b/src/soc/amd/stoneyridge/lpc.c
@@ -328,7 +328,13 @@

unsigned long acpi_fill_mcfg(unsigned long current)
 {
-	/* Just a dummy */
+
+	current += acpi_create_mcfg_mmconfig((acpi_mcfg_mmconfig_t *)current,
+					     CONFIG_MMCONF_BASE_ADDRESS,
+					     0,
+					     0,
+					     CONFIG_MMCONF_BUS_NUMBER);
+
    return current;
 }