[coreboot-gerrit] Change in ...coreboot[master]: sb/intel: Check for NULL-return of pcidev_on_root()

[Nico Huber (Code Review)]

Nico Huber has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/30879 )

Change subject: sb/intel: Check for NULL-return of pcidev_on_root()
......................................................................

sb/intel: Check for NULL-return of pcidev_on_root()

In these cases we have to expect a NULL pointer because the IGD
device 0:2.0 may be disabled.

The behaviour still differs from using dev_find_slot(), which may
return a disabled device. Though, if you'd try to read its config
space you'd only read garbage (0xff) and in cases where we filled
ACPI data with devicetree information, the information shouldn't
be interpreted by the OS because of the disabled device.

Change-Id: I1bab8fa3a82daca71d03453315cdd69d8951fc24
Signed-off-by: Nico Huber 
Reviewed-on: https://review.coreboot.org/c/30879
Reviewed-by: Kyösti Mälkki 
Reviewed-by: Angel Pons 
Tested-by: build bot (Jenkins) 
---
M src/southbridge/intel/bd82x6x/lpc.c
M src/southbridge/intel/i82801gx/lpc.c
M src/southbridge/intel/i82801ix/lpc.c
M src/southbridge/intel/i82801jx/lpc.c
M src/southbridge/intel/ibexpeak/lpc.c
M src/southbridge/intel/lynxpoint/lpc.c
6 files changed, 27 insertions(+), 13 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Kyösti Mälkki: Looks good to me, approved
  Angel Pons: Looks good to me, approved

diff --git a/src/southbridge/intel/bd82x6x/lpc.c b/src/southbridge/intel/bd82x6x/lpc.c
index e13c666..4de4f16 100644
--- a/src/southbridge/intel/bd82x6x/lpc.c
+++ b/src/southbridge/intel/bd82x6x/lpc.c
@@ -719,8 +719,10 @@
    	gnvs->mpen = 1; /* Enable Multi Processing */
    	gnvs->pcnt = dev_count_cpu();

-		gnvs->ndid = gfx->ndid;
-		memcpy(gnvs->did, gfx->did, sizeof(gnvs->did));
+		if (gfx) {
+			gnvs->ndid = gfx->ndid;
+			memcpy(gnvs->did, gfx->did, sizeof(gnvs->did));
+		}

#if IS_ENABLED(CONFIG_CHROMEOS)
    	chromeos_init_chromeos_acpi(&(gnvs->chromeos));
diff --git a/src/southbridge/intel/i82801gx/lpc.c b/src/southbridge/intel/i82801gx/lpc.c
index e8cfc74..b20b2aa 100644
--- a/src/southbridge/intel/i82801gx/lpc.c
+++ b/src/southbridge/intel/i82801gx/lpc.c
@@ -705,8 +705,10 @@

acpi_create_gnvs(gnvs);

-		gnvs->ndid = gfx->ndid;
-		memcpy(gnvs->did, gfx->did, sizeof(gnvs->did));
+		if (gfx) {
+			gnvs->ndid = gfx->ndid;
+			memcpy(gnvs->did, gfx->did, sizeof(gnvs->did));
+		}

/* And tell SMI about it */
    	smm_setup_structures(gnvs, NULL, NULL);
diff --git a/src/southbridge/intel/i82801ix/lpc.c b/src/southbridge/intel/i82801ix/lpc.c
index dd37a0b..e18f616 100644
--- a/src/southbridge/intel/i82801ix/lpc.c
+++ b/src/southbridge/intel/i82801ix/lpc.c
@@ -545,8 +545,10 @@
    	memset(gnvs, 0, sizeof(*gnvs));
    	acpi_create_gnvs(gnvs);

-		gnvs->ndid = gfx->ndid;
-		memcpy(gnvs->did, gfx->did, sizeof(gnvs->did));
+		if (gfx) {
+			gnvs->ndid = gfx->ndid;
+			memcpy(gnvs->did, gfx->did, sizeof(gnvs->did));
+		}

/* And tell SMI about it */
    	smm_setup_structures(gnvs, NULL, NULL);
diff --git a/src/southbridge/intel/i82801jx/lpc.c b/src/southbridge/intel/i82801jx/lpc.c
index 0f82f90..3f7095c 100644
--- a/src/southbridge/intel/i82801jx/lpc.c
+++ b/src/southbridge/intel/i82801jx/lpc.c
@@ -707,8 +707,10 @@
    	memset(gnvs, 0, sizeof(*gnvs));
    	acpi_create_gnvs(gnvs);

-		gnvs->ndid = gfx->ndid;
-		memcpy(gnvs->did, gfx->did, sizeof(gnvs->did));
+		if (gfx) {
+			gnvs->ndid = gfx->ndid;
+			memcpy(gnvs->did, gfx->did, sizeof(gnvs->did));
+		}

/* And tell SMI about it */
    	smm_setup_structures(gnvs, NULL, NULL);
diff --git a/src/southbridge/intel/ibexpeak/lpc.c b/src/southbridge/intel/ibexpeak/lpc.c
index 3358633..a96b7ff 100644
--- a/src/southbridge/intel/ibexpeak/lpc.c
+++ b/src/southbridge/intel/ibexpeak/lpc.c
@@ -632,8 +632,11 @@
    	gnvs->apic = 1;
    	gnvs->mpen = 1;		/* Enable Multi Processing */
    	gnvs->pcnt = dev_count_cpu();
-		gnvs->ndid = gfx->ndid;
-		memcpy(gnvs->did, gfx->did, sizeof(gnvs->did));
+
+		if (gfx) {
+			gnvs->ndid = gfx->ndid;
+			memcpy(gnvs->did, gfx->did, sizeof(gnvs->did));
+		}

/* And tell SMI about it */
    	smm_setup_structures(gnvs, NULL, NULL);
diff --git a/src/southbridge/intel/lynxpoint/lpc.c b/src/southbridge/intel/lynxpoint/lpc.c
index 474c7df..59074e0 100644
--- a/src/southbridge/intel/lynxpoint/lpc.c
+++ b/src/southbridge/intel/lynxpoint/lpc.c
@@ -456,7 +456,8 @@
    RCBA32_AND_OR(0x2614, 0x8bffffff, 0x0a206500);

/* Check for LPT-LP B2 stepping and 0:31.0@0xFA > 4 */
-	if (pci_read_config8(pcidev_on_root(2, 0), 0x8) >= 0x0b)
+	struct device *const gma = pcidev_on_root(2, 0);
+	if (gma && pci_read_config8(gma, 0x8) >= 0x0b)
    	RCBA32_OR(0x2614, (1 << 26));

RCBA32_OR(0x900, 0x0000031f);
@@ -760,8 +761,10 @@
    	/* Update the mem console pointer. */
    	gnvs->cbmc = (u32)cbmem_find(CBMEM_ID_CONSOLE);

-		gnvs->ndid = gfx->ndid;
-		memcpy(gnvs->did, gfx->did, sizeof(gnvs->did));
+		if (gfx) {
+			gnvs->ndid = gfx->ndid;
+			memcpy(gnvs->did, gfx->did, sizeof(gnvs->did));
+		}

/* And tell SMI about it */
    	smm_setup_structures(gnvs, NULL, NULL);