[coreboot-gerrit] Change in coreboot[master]: drivers/ipmi: Handle the condition when (dev->chip_info == NULL)

[Bill XIE (Code Review)]

Bill XIE has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/47387 )

Change subject: drivers/ipmi: Handle the condition when (dev->chip_info == NULL)
......................................................................

drivers/ipmi: Handle the condition when (dev->chip_info == NULL)

Some former commits (e.g. Ieb41771c75aae902191bba5d220796e6c343f8e0)
blindly assume that dev->chip_info is capable to be dereferenced,
making at least compilers complain about potential null pointer
dereference. They might cause crash if truly (dev->chip_info == NULL).

Their code should be adjusted to be runnable even if dev->chip_info is
NULL.

Signed-off-by: Bill XIE 
Change-Id: I1d694b12f6c42961c104fe839d4ee46c0f111197
---
M src/drivers/ipmi/ipmi_kcs_ops.c
1 file changed, 14 insertions(+), 9 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/87/47387/1

diff --git a/src/drivers/ipmi/ipmi_kcs_ops.c b/src/drivers/ipmi/ipmi_kcs_ops.c
index 362f17a..3af44ef 100644
--- a/src/drivers/ipmi/ipmi_kcs_ops.c
+++ b/src/drivers/ipmi/ipmi_kcs_ops.c
@@ -77,8 +77,9 @@
    struct ipmi_devid_rsp rsp;
    uint32_t man_id = 0, prod_id = 0;
    struct drivers_ipmi_config *conf = NULL;
-	struct ipmi_selftest_rsp selftestrsp;
+	struct ipmi_selftest_rsp selftestrsp = {{ 0, 0, 0 }, 0, 0};
    uint8_t retry_count;
+	uint16_t bmc_boot_timeout = 0;

if (!dev->enabled)
    	return;
@@ -90,8 +91,9 @@

/* Get IPMI version for ACPI and SMBIOS */
    if (conf && conf->wait_for_bmc && conf->bmc_boot_timeout) {
+		bmc_boot_timeout = conf->bmc_boot_timeout;
    	struct stopwatch sw;
-		stopwatch_init_msecs_expire(&sw, conf->bmc_boot_timeout * 1000);
+		stopwatch_init_msecs_expire(&sw, bmc_boot_timeout * 1000);
    	printk(BIOS_INFO, "IPMI: Waiting for BMC...\n");

while (!stopwatch_expired(&sw)) {
@@ -108,7 +110,7 @@
    }

printk(BIOS_INFO, "Get BMC self test result...");
-	for (retry_count = 0; retry_count < conf->bmc_boot_timeout; retry_count++) {
+	for (retry_count = 0; retry_count < bmc_boot_timeout; retry_count++) {
    	if (!ipmi_get_bmc_self_test_result(dev, &selftestrsp))
    		break;

@@ -175,6 +177,7 @@
    	       struct acpi_rsdp *rsdp)
 {
    struct drivers_ipmi_config *conf = NULL;
+	uint32_t uid = 0;
    struct acpi_spmi *spmi;
    s8 gpe_interrupt = -1;
    u32 apic_interrupt = 0;
@@ -214,11 +217,12 @@
    		apic_interrupt = conf->apic_interrupt;
    }

-	/* Use command to get UID from ipmi_ssdt */
+	if (uid_cnt > 0)
+		uid = uid_cnt - 1;
    acpi_create_ipmi(dev, spmi, (ipmi_revision_major << 8) |
    		 (ipmi_revision_minor << 4), &addr,
    		 IPMI_INTERFACE_KCS, gpe_interrupt, apic_interrupt,
-			 conf->uid);
+			 uid);

acpi_add_table(rsdp, spmi);

@@ -241,15 +245,12 @@
    if (dev->chip_info)
    	conf = dev->chip_info;

-	/* Use command to pass UID to ipmi_write_acpi_tables */
-	conf->uid = uid_cnt++;
-
    /* write SPMI device */
    acpigen_write_scope(scope);
    acpigen_write_device("SPMI");
    acpigen_write_name_string("_HID", "IPI0001");
    acpigen_write_name_unicode("_STR", "IPMI_KCS");
-	acpigen_write_name_byte("_UID", conf->uid);
+	acpigen_write_name_byte("_UID", uid_cnt);
    acpigen_write_STA(0xf);
    acpigen_write_name("_CRS");
    acpigen_write_resourcetemplate_header();
@@ -261,8 +262,12 @@
    	// FIXME: is that correct?
    	if (conf->have_apic)
    		acpigen_write_irq(1 << conf->apic_interrupt);
+
+		conf->uid = uid_cnt;
    }

+	uid_cnt++;
+
    acpigen_write_resourcetemplate_footer();

acpigen_write_method("_IFT", 0);