Skip to content

Commit

Permalink
ipmi_ssif: avoid registering duplicate ssif interface
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
kamlakantp authored and cminyard committed Aug 22, 2019
1 parent 2033f68 commit c4436c9
Showing 1 changed file with 77 additions and 1 deletion.
78 changes: 77 additions & 1 deletion drivers/char/ipmi/ipmi_ssif.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down Expand Up @@ -1602,6 +1606,60 @@ static void test_multipart_messages(struct i2c_client *client,
#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];
Expand All @@ -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;
}

Expand All @@ -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,
Expand Down Expand Up @@ -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:
Expand Down

0 comments on commit c4436c9

Please sign in to comment.