ipmi_ssif: avoid registering duplicate ssif interface

It is possible that SSIF interface entry is present in both DMI and ACPI
tables. In SMP systems, in such cases it is possible that ssif_probe could
be called simultaneously from i2c interface (from ACPI) and from DMI on
different CPUs at kernel boot. Both try to register same SSIF interface
simultaneously and result in race.

In such cases where ACPI and SMBIOS both IPMI entries are available, we
need to prefer ACPI over SMBIOS so that ACPI functions work properly if
they use IPMI.
So, if we get an ACPI interface and have already registered an SMBIOS
at the same address, we need to remove the SMBIOS one and add the ACPI.

Log:
[   38.774743] ipmi device interface
[   38.805006] ipmi_ssif: IPMI SSIF Interface driver
[   38.861979] ipmi_ssif i2c-IPI0001:06: ssif_probe CPU 99 ***
[   38.863655] ipmi_ssif 0-000e: ssif_probe CPU 14 ***
[   38.863658] ipmi_ssif: Trying SMBIOS-specified SSIF interface at i2c address 0xe, adapter xlp9xx-i2c, slave address 0x0
[   38.869500] ipmi_ssif: Trying ACPI-specified SSIF interface at i2c address 0xe, adapter xlp9xx-i2c, slave address 0x0
[   38.914530] ipmi_ssif: Unable to clear message flags: -22 7 c7
[   38.952429] ipmi_ssif: Unable to clear message flags: -22 7 00
[   38.994734] ipmi_ssif: Error getting global enables: -22 7 00
[   39.015877] ipmi_ssif 0-000e: IPMI message handler: Found new BMC (man_id: 0x00b3d1, prod_id: 0x0001, dev_id: 0x20)
[   39.377645] ipmi_ssif i2c-IPI0001:06: IPMI message handler: Found new BMC (man_id: 0x00b3d1, prod_id: 0x0001, dev_id: 0x20)
[   39.387863] ipmi_ssif 0-000e: IPMI message handler: BMC returned incorrect response, expected netfn 7 cmd 42, got netfn 7 cmd 1
...
[NOTE] : Added custom prints to explain the problem.

In the above log, ssif_probe is executed simultaneously on two different
CPUs.

This patch fixes this issue in following way:
 - Adds ACPI entry also to the 'ssif_infos' list.
 - Checks the list if SMBIOS is already registered, removes it and adds
   ACPI.
 - If ACPI is already registered, it ignores SMBIOS.
 - Adds mutex lock throughout the probe process to avoid race.

Signed-off-by: Kamlakant Patel <kamlakantp@marvell.com>
Message-Id: <1566389064-27356-1-git-send-email-kamlakantp@marvell.com>
Signed-off-by: Corey Minyard <cminyard@mvista.com>

1 files changed, 77 insertions, 1 deletions

diff --git a/drivers/char/ipmi/ipmi_ssif.c b/drivers/char/ipmi/ipmi_ssif.c
index 6e070ef4391b..22c6a2e61236 100644
--- a/drivers/char/ipmi/ipmi_ssif.c
+++ b/drivers/char/ipmi/ipmi_ssif.c
@@ -1427,6 +1427,10 @@ static struct ssif_addr_info *ssif_info_find(unsigned short addr,
 restart:
 	list_for_each_entry(info, &ssif_infos, link) {
 		if (info->binfo.addr == addr) {
+			if (info->addr_src == SI_SMBIOS)
+				info->adapter_name = kstrdup(adapter_name,
+							     GFP_KERNEL);
+
 			if (info->adapter_name || adapter_name) {
 				if (!info->adapter_name != !adapter_name) {
 					/* One is NULL and one is not */
@@ -1602,6 +1606,60 @@ out_no_multi_part:
 #define GLOBAL_ENABLES_MASK (IPMI_BMC_EVT_MSG_BUFF | IPMI_BMC_RCV_MSG_INTR | \
 			     IPMI_BMC_EVT_MSG_INTR)
 
+static void ssif_remove_dup(struct i2c_client *client)
+{
+	struct ssif_info *ssif_info = i2c_get_clientdata(client);
+
+	ipmi_unregister_smi(ssif_info->intf);
+	kfree(ssif_info);
+}
+
+static int ssif_add_infos(struct i2c_client *client)
+{
+	struct ssif_addr_info *info;
+
+	info = kzalloc(sizeof(*info), GFP_KERNEL);
+	if (!info)
+		return -ENOMEM;
+	info->addr_src = SI_ACPI;
+	info->client = client;
+	info->adapter_name = kstrdup(client->adapter->name, GFP_KERNEL);
+	info->binfo.addr = client->addr;
+	list_add_tail(&info->link, &ssif_infos);
+	return 0;
+}
+
+/*
+ * Prefer ACPI over SMBIOS, if both are available.
+ * So if we get an ACPI interface and have already registered a SMBIOS
+ * interface at the same address, remove the SMBIOS and add the ACPI one.
+ */
+static int ssif_check_and_remove(struct i2c_client *client,
+			      struct ssif_info *ssif_info)
+{
+	struct ssif_addr_info *info;
+
+	list_for_each_entry(info, &ssif_infos, link) {
+		if (!info->client)
+			return 0;
+		if (!strcmp(info->adapter_name, client->adapter->name) &&
+		    info->binfo.addr == client->addr) {
+			if (info->addr_src == SI_ACPI)
+				return -EEXIST;
+
+			if (ssif_info->addr_source == SI_ACPI &&
+			    info->addr_src == SI_SMBIOS) {
+				dev_info(&client->dev,
+					 "Removing %s-specified SSIF interface in favor of ACPI\n",
+					 ipmi_addr_src_to_str(info->addr_src));
+				ssif_remove_dup(info->client);
+				return 0;
+			}
+		}
+	}
+	return 0;
+}
+
 static int ssif_probe(struct i2c_client *client, const struct i2c_device_id *id)
 {
 	unsigned char     msg[3];
@@ -1613,13 +1671,17 @@ static int ssif_probe(struct i2c_client *client, const struct i2c_device_id *id)
 	u8		  slave_addr = 0;
 	struct ssif_addr_info *addr_info = NULL;
 
+	mutex_lock(&ssif_infos_mutex);
 	resp = kmalloc(IPMI_MAX_MSG_LENGTH, GFP_KERNEL);
-	if (!resp)
+	if (!resp) {
+		mutex_unlock(&ssif_infos_mutex);
 		return -ENOMEM;
+	}
 
 	ssif_info = kzalloc(sizeof(*ssif_info), GFP_KERNEL);
 	if (!ssif_info) {
 		kfree(resp);
+		mutex_unlock(&ssif_infos_mutex);
 		return -ENOMEM;
 	}
 
@@ -1638,6 +1700,19 @@ static int ssif_probe(struct i2c_client *client, const struct i2c_device_id *id)
 		}
 	}
 
+	rv = ssif_check_and_remove(client, ssif_info);
+	/* If rv is 0 and addr source is not SI_ACPI, continue probing */
+	if (!rv && ssif_info->addr_source == SI_ACPI) {
+		rv = ssif_add_infos(client);
+		if (rv) {
+			dev_err(&client->dev, "Out of memory!, exiting ..\n");
+			goto out;
+		}
+	} else if (rv) {
+		dev_err(&client->dev, "Not probing, Interface already present\n");
+		goto out;
+	}
+
 	slave_addr = find_slave_address(client, slave_addr);
 
 	dev_info(&client->dev,
@@ -1850,6 +1925,7 @@ static int ssif_probe(struct i2c_client *client, const struct i2c_device_id *id)
 		kfree(ssif_info);
 	}
 	kfree(resp);
+	mutex_unlock(&ssif_infos_mutex);
 	return rv;
 
 out_remove_attr: