Change in ...coreboot[master]: qemu-q35: die if started on wrong machine

[Patrick Rudolph (Code Review)]

Patrick Rudolph has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/31425

Change subject: qemu-q35: die if started on wrong machine
......................................................................

qemu-q35: die if started on wrong machine

The QEMU machine "PC" doesn't support MCFG.
Die in bootblock if the user selected the wrong qemu machine and
print a message to use the correct machine type.

Without this patch ramstage dies with non-helpful message:
"get_pbus: dev is NULL!"

Change-Id: I9d1b24176de971c5f827091bc5bc1bac8426f3f6
Signed-off-by: Patrick Rudolph 
---
M src/mainboard/emulation/qemu-q35/bootblock.c
1 file changed, 6 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/25/31425/1

diff --git a/src/mainboard/emulation/qemu-q35/bootblock.c b/src/mainboard/emulation/qemu-q35/bootblock.c
index 18a083d..96d3457 100644
--- a/src/mainboard/emulation/qemu-q35/bootblock.c
+++ b/src/mainboard/emulation/qemu-q35/bootblock.c
@@ -14,6 +14,7 @@
 #include <arch/io.h>
 #include <bootblock_common.h>
 #include <southbridge/intel/i82801ix/i82801ix.h>
+#include <console/console.h>

/* Just define these here, there is no gm35.h file to include. */
 #define D0F0_PCIEXBAR_LO 0x60
@@ -39,6 +40,11 @@
    pci_io_write_config32(PCI_DEV(0,0,0), D0F0_PCIEXBAR_HI, reg);
    reg = CONFIG_MMCONF_BASE_ADDRESS | 1; /* 256MiB - 0-255 buses. */
    pci_io_write_config32(PCI_DEV(0,0,0), D0F0_PCIEXBAR_LO, reg);
+
+	/* MCFG is now active. If it's not qemu was started for machine PC */
+	if (pci_read_config32(PCI_DEV(0,0,0), D0F0_PCIEXBAR_LO) !=
+	    (CONFIG_MMCONF_BASE_ADDRESS | 1))
+		die("You must run qemu for machine Q35");
 }

static void enable_spi_prefetch(void)

[build bot (Jenkins) (Code Review)]

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31425 )

Change subject: qemu-q35: die if started on wrong machine
......................................................................

Patch Set 1:

(2 comments)

https://review.coreboot.org/#/c/31425/1/src/mainboard/emulation/qemu-q35/boo... 
File src/mainboard/emulation/qemu-q35/bootblock.c:

https://review.coreboot.org/#/c/31425/1/src/mainboard/emulation/qemu-q35/boo... 
PS1, Line 45: 	if (pci_read_config32(PCI_DEV(0,0,0), D0F0_PCIEXBAR_LO) !=
space required after that ',' (ctx:VxV)

https://review.coreboot.org/#/c/31425/1/src/mainboard/emulation/qemu-q35/boo... 
PS1, Line 45: 	if (pci_read_config32(PCI_DEV(0,0,0), D0F0_PCIEXBAR_LO) !=
space required after that ',' (ctx:VxV)

[Patrick Rudolph (Code Review)]

Patrick Rudolph has uploaded a new patch set (#2). ( https://review.coreboot.org/c/coreboot/+/31425 )

Change subject: qemu-q35: die if started on wrong machine
......................................................................

qemu-q35: die if started on wrong machine

The QEMU machine "PC" doesn't support MCFG.
Die in bootblock if the user selected the wrong qemu machine and
print a message to use the correct machine type.

Without this patch ramstage dies with non-helpful message:
"get_pbus: dev is NULL!"

Change-Id: I9d1b24176de971c5f827091bc5bc1bac8426f3f6
Signed-off-by: Patrick Rudolph 
---
M src/mainboard/emulation/qemu-q35/bootblock.c
1 file changed, 6 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/25/31425/2

[Kyösti Mälkki (Code Review)]

Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31425 )

Change subject: qemu-q35: die if started on wrong machine
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/#/c/31425/2/src/mainboard/emulation/qemu-q35/boo... 
File src/mainboard/emulation/qemu-q35/bootblock.c:

https://review.coreboot.org/#/c/31425/2/src/mainboard/emulation/qemu-q35/boo... 
PS2, Line 47: 		die("You must run qemu for machine Q35");
Detect this after console_init() in romstage so it has small chance of ending up on the console.

Change in coreboot[master]: qemu-q35: die if started on wrong machine

[Patrick Rudolph (Code Review)]

Patrick Rudolph has uploaded a new patch set (#3) to the change originally created by Patrick Rudolph. ( https://review.coreboot.org/c/coreboot/+/31425 )

Change subject: qemu-q35: die if started on wrong machine
......................................................................

qemu-q35: die if started on wrong machine

The QEMU machine "PC" doesn't support MCFG.
Die after console init if the user selected the wrong qemu machine
and print a message to use the correct machine type.

Without this patch ramstage dies with non-helpful message:
"get_pbus: dev is NULL!"

Change-Id: I9d1b24176de971c5f827091bc5bc1bac8426f3f6
Signed-off-by: Patrick Rudolph 
---
M src/mainboard/emulation/qemu-q35/bootblock.c
M src/mainboard/emulation/qemu-q35/romstage.c
2 files changed, 21 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/25/31425/3

Change in coreboot[master]: qemu-q35: die if started on wrong machine

[Patrick Rudolph (Code Review)]

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

Change subject: qemu-q35: die if started on wrong machine
......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/31425/2/src/mainboard/emulation/qem... 
File src/mainboard/emulation/qemu-q35/bootblock.c:

https://review.coreboot.org/c/coreboot/+/31425/2/src/mainboard/emulation/qem... 
PS2, Line 47: 		die("You must run qemu for machine Q35");
> Detect this after console_init() in romstage so it has small chance of ending up on the console.

Done
Added the same code in romstage if !CONSOLE_BOOTBLOCK.

Change in coreboot[master]: qemu-q35: die if started on wrong machine

[Angel Pons (Code Review)]

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

Change subject: qemu-q35: die if started on wrong machine
......................................................................

Patch Set 3: Code-Review+1

Change in coreboot[master]: qemu-q35: die if started on wrong machine

[Kyösti Mälkki (Code Review)]

Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31425 )

Change subject: qemu-q35: die if started on wrong machine
......................................................................

Patch Set 3: Code-Review+2

(1 comment)

https://review.coreboot.org/c/coreboot/+/31425/3/src/mainboard/emulation/qem... 
File src/mainboard/emulation/qemu-q35/bootblock.c:

https://review.coreboot.org/c/coreboot/+/31425/3/src/mainboard/emulation/qem... 
PS3, Line 46: 	    (pci_read_config32(PCI_DEV(0, 0, 0), D0F0_PCIEXBAR_LO) !=
Maybe pci_io_read() instead. Without MCFG that pci_read() is invalid access.

Change in coreboot[master]: qemu-q35: die if started on wrong machine

[Patrick Rudolph (Code Review)]

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

Change subject: qemu-q35: die if started on wrong machine
......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/31425/3/src/mainboard/emulation/qem... 
File src/mainboard/emulation/qemu-q35/bootblock.c:

https://review.coreboot.org/c/coreboot/+/31425/3/src/mainboard/emulation/qem... 
PS3, Line 46: 	    (pci_read_config32(PCI_DEV(0, 0, 0), D0F0_PCIEXBAR_LO) !=
> Maybe pci_io_read() instead. Without MCFG that pci_read() is invalid access.

The reason this works is it doesn't use pci_io_read. It will try to access the PCI device using MCFG, but of course it doesn't work and thus the check fails.

Change in coreboot[master]: qemu-q35: die if started on wrong machine

[Kyösti Mälkki (Code Review)]

Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31425 )

Change subject: qemu-q35: die if started on wrong machine
......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/31425/3/src/mainboard/emulation/qem... 
File src/mainboard/emulation/qemu-q35/bootblock.c:

https://review.coreboot.org/c/coreboot/+/31425/3/src/mainboard/emulation/qem... 
PS3, Line 46: 	    (pci_read_config32(PCI_DEV(0, 0, 0), D0F0_PCIEXBAR_LO) !=
> The reason this works is it doesn't use pci_io_read. […]

Ah. I thought QEMU would not implement that register PCIEXBAR at all if it will not do MCFG.

Change in coreboot[master]: qemu-q35: die if started on wrong machine

[Paul Menzel (Code Review)]

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

Change subject: qemu-q35: die if started on wrong machine
......................................................................

Patch Set 3: Code-Review+1

Change in coreboot[master]: qemu-q35: die if started on wrong machine

[Martin Roth (Code Review)]

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

Change subject: qemu-q35: die if started on wrong machine
......................................................................

qemu-q35: die if started on wrong machine

The QEMU machine "PC" doesn't support MCFG.
Die after console init if the user selected the wrong qemu machine
and print a message to use the correct machine type.

Without this patch ramstage dies with non-helpful message:
"get_pbus: dev is NULL!"

Change-Id: I9d1b24176de971c5f827091bc5bc1bac8426f3f6
Signed-off-by: Patrick Rudolph 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/31425
Tested-by: build bot (Jenkins) 
Reviewed-by: Angel Pons 
Reviewed-by: Kyösti Mälkki 
Reviewed-by: Paul Menzel 
---
M src/mainboard/emulation/qemu-q35/bootblock.c
M src/mainboard/emulation/qemu-q35/romstage.c
2 files changed, 21 insertions(+), 0 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Kyösti Mälkki: Looks good to me, approved
  Paul Menzel: Looks good to me, but someone else must approve
  Angel Pons: Looks good to me, but someone else must approve

diff --git a/src/mainboard/emulation/qemu-q35/bootblock.c b/src/mainboard/emulation/qemu-q35/bootblock.c
index 13550f7..d5ca7f9 100644
--- a/src/mainboard/emulation/qemu-q35/bootblock.c
+++ b/src/mainboard/emulation/qemu-q35/bootblock.c
@@ -14,6 +14,7 @@
 #include <device/pci_ops.h>
 #include <bootblock_common.h>
 #include <southbridge/intel/i82801ix/i82801ix.h>
+#include <console/console.h>

/* Just define these here, there is no gm35.h file to include. */
 #define D0F0_PCIEXBAR_LO 0x60
@@ -39,6 +40,12 @@
    pci_io_write_config32(PCI_DEV(0,0,0), D0F0_PCIEXBAR_HI, reg);
    reg = CONFIG_MMCONF_BASE_ADDRESS | 1; /* 256MiB - 0-255 buses. */
    pci_io_write_config32(PCI_DEV(0,0,0), D0F0_PCIEXBAR_LO, reg);
+
+	/* MCFG is now active. If it's not qemu was started for machine PC */
+	if (CONFIG(BOOTBLOCK_CONSOLE) &&
+	    (pci_read_config32(PCI_DEV(0, 0, 0), D0F0_PCIEXBAR_LO) !=
+	     (CONFIG_MMCONF_BASE_ADDRESS | 1)))
+		die("You must run qemu for machine Q35 (-M q35)");
 }

static void enable_spi_prefetch(void)
diff --git a/src/mainboard/emulation/qemu-q35/romstage.c b/src/mainboard/emulation/qemu-q35/romstage.c
index e409ad1..dbaead9 100644
--- a/src/mainboard/emulation/qemu-q35/romstage.c
+++ b/src/mainboard/emulation/qemu-q35/romstage.c
@@ -22,6 +22,18 @@
 #include <timestamp.h>
 #include <southbridge/intel/i82801ix/i82801ix.h>
 #include <program_loading.h>
+#include <device/pci_ops.h>
+
+#define D0F0_PCIEXBAR_LO 0x60
+
+static void mainboard_machine_check(void)
+{
+	/* Check that MCFG is active. If it's not qemu was started for machine PC */
+	if (!CONFIG(BOOTBLOCK_CONSOLE) &&
+	    (pci_read_config32(PCI_DEV(0, 0, 0), D0F0_PCIEXBAR_LO) !=
+	     (CONFIG_MMCONF_BASE_ADDRESS | 1)))
+		die("You must run qemu for machine Q35 (-M q35)");
+}

asmlinkage void car_stage_entry(void)
 {
@@ -29,6 +41,8 @@
    i82801ix_early_init();
    console_init();

+	mainboard_machine_check();
+
    cbmem_recovery(0);

timestamp_add_now(TS_START_ROMSTAGE);