Hi Corey,
On 12/20/18 7:55 PM, Corey Minyard wrote:
External Email
> ----------------------------------------------------------------------
> 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
Thanks for your comments. I will send a patch addressing above comments.
Thanks,
Kamlakant patel
>> 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
