On 12/20/18 7:18 AM, Kamlakant Patel wrote:
We have SSIF interface present in both DMI and ACPI tables, sometimes
ssif_probe is called simultaneously from i2c interface (from ACPI)
and while loading ipmi_ssif driver(from DMI table) at kernel boot.
Both try to register the same SSIF interface simultaneously, where it
hits a race. In this case both send a command and get a currupted response.
[sp] corrupted.
This patch adds check to void registering the same device twice.
Yuck. I was hoping that systems with SSIF wouldn't put it in both places,
but I suppose that was a bad expectation.
What you want to do here is prefer ACPI over SMBIOS, so that ACPI functions
work properly if they use IPMI. So if you get an ACPI interface and have
already registered a SMBIOS interface at the same address, you need to
remove the SMBIOS one and add the ACPI one. I don't know of an easy
way to look ahead and see if an ACPI interface is coming in the future.
However, there is one piece of information in the SMBIOS tables that is not
in ACPI: the slave address. But you can pull the slave address from the
SMBIOS tables using ipmi_dmi_get_slave_addr() in ipmi_dmi.c.
You probably need a mutex around the whole probe process, too.
This is how the SSIF interface works. It's not great, but it's the best I
could come up with.
Thanks,
-corey
Signed-off-by: Kamlakant Patel <kamlaka...@marvell.com>
---
drivers/char/ipmi/ipmi_ssif.c | 39 +++++++++++++++++++++++++++++++++++
1 file changed, 39 insertions(+)
diff --git a/drivers/char/ipmi/ipmi_ssif.c b/drivers/char/ipmi/ipmi_ssif.c
index ca9528c4f183..97830d31c5d3 100644
--- a/drivers/char/ipmi/ipmi_ssif.c
+++ b/drivers/char/ipmi/ipmi_ssif.c
@@ -1146,6 +1146,7 @@ MODULE_PARM_DESC(trydmi, "Setting this to zero will
disable the default scan of
static DEFINE_MUTEX(ssif_infos_mutex);
static LIST_HEAD(ssif_infos);
+static LIST_HEAD(ssif_clients);
#define IPMI_SSIF_ATTR(name) \
static ssize_t ipmi_##name##_show(struct device *dev, \
@@ -1523,6 +1524,34 @@ 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 int ssif_is_registered(struct i2c_client *client)
+{
+ struct ssif_addr_info *info;
+ int rv = 0;
+
+ mutex_lock(&ssif_infos_mutex);
+ list_for_each_entry(info, &ssif_clients, link) {
+ if ((info->binfo.addr == client->addr) &&
+ (!strcmp(info->adapter_name, client->adapter->name))) {
+ rv = -EEXIST;
+ goto exit;
+ }
+ }
+
+ info = kzalloc(sizeof(*info), GFP_KERNEL);
+ if (!info){
+ rv = -ENOMEM;
+ goto exit;
+ }
+
+ info->adapter_name = client->adapter->name;
+ info->binfo.addr = client->addr;
+ list_add_tail(&info->link, &ssif_clients);
+exit:
+ mutex_unlock(&ssif_infos_mutex);
+ return rv;
+}
+
static int ssif_probe(struct i2c_client *client, const struct i2c_device_id
*id)
{
unsigned char msg[3];
@@ -1534,6 +1563,10 @@ 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;
+ /* Check if the client is already registered. */
+ if (ssif_is_registered(client))
+ return 0;
+
resp = kmalloc(IPMI_MAX_MSG_LENGTH, GFP_KERNEL);
if (!resp)
return -ENOMEM;
@@ -1844,6 +1877,12 @@ static void free_ssif_clients(void)
kfree(info->adapter_name);
kfree(info);
}
+
+ list_for_each_entry_safe(info, tmp, &ssif_clients, link) {
+ list_del(&info->link);
+ kfree(info);
+ }
+
mutex_unlock(&ssif_infos_mutex);
}
_______________________________________________
Openipmi-developer mailing list
Openipmi-developer@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openipmi-developer
