For upcoming runtime extensions of MSI-X interrupts it's required to
protect the MSI descriptor list. Add a mutex to struct msi_device_data and
provide lock/unlock functions.
Signed-off-by: Thomas Gleixner <[email protected]>
---
include/linux/msi.h | 5 +++++
kernel/irq/msi.c | 25 +++++++++++++++++++++++++
2 files changed, 30 insertions(+)
--- a/include/linux/msi.h
+++ b/include/linux/msi.h
@@ -3,6 +3,7 @@
#define LINUX_MSI_H
#include <linux/cpumask.h>
+#include <linux/mutex.h>
#include <linux/list.h>
#include <linux/bits.h>
#include <asm/msi.h>
@@ -146,12 +147,14 @@ struct msi_desc {
* @attrs: Pointer to the sysfs attribute group
* @platform_data: Platform-MSI specific data
* @list: List of MSI descriptors associated to the device
+ * @mutex: Mutex protecting the MSI list
*/
struct msi_device_data {
unsigned long properties;
const struct attribute_group **attrs;
struct platform_msi_priv_data *platform_data;
struct list_head list;
+ struct mutex mutex;
};
int msi_setup_device_data(struct device *dev);
@@ -173,6 +176,8 @@ static inline void msi_device_set_proper
#endif
unsigned int msi_get_virq(struct device *dev, unsigned int index);
+void msi_lock_descs(struct device *dev);
+void msi_unlock_descs(struct device *dev);
/* Helpers to hide struct msi_desc implementation details */
#define msi_desc_to_dev(desc) ((desc)->dev)
--- a/kernel/irq/msi.c
+++ b/kernel/irq/msi.c
@@ -127,12 +127,37 @@ int msi_setup_device_data(struct device
return -ENOMEM;
INIT_LIST_HEAD(&md->list);
+ mutex_init(&md->mutex);
dev->msi.data = md;
devres_add(dev, md);
return 0;
}
/**
+ * msi_lock_descs - Lock the MSI descriptor storage of a device
+ * @dev: Device to operate on
+ */
+void msi_lock_descs(struct device *dev)
+{
+ if (WARN_ON_ONCE(!dev->msi.data))
+ return;
+ mutex_lock(&dev->msi.data->mutex);
+}
+EXPORT_SYMBOL_GPL(msi_lock_descs);
+
+/**
+ * msi_unlock_descs - Unlock the MSI descriptor storage of a device
+ * @dev: Device to operate on
+ */
+void msi_unlock_descs(struct device *dev)
+{
+ if (WARN_ON_ONCE(!dev->msi.data))
+ return;
+ mutex_unlock(&dev->msi.data->mutex);
+}
+EXPORT_SYMBOL_GPL(msi_unlock_descs);
+
+/**
* msi_get_virq - Return Linux interrupt number of a MSI interrupt
* @dev: Device to operate on
* @index: MSI interrupt index to look for (0-based)
On Mon, Dec 06, 2021 at 11:51:05PM +0100, Thomas Gleixner wrote:
> +++ b/kernel/irq/msi.c
> @@ -127,12 +127,37 @@ int msi_setup_device_data(struct device
> return -ENOMEM;
>
> INIT_LIST_HEAD(&md->list);
> + mutex_init(&md->mutex);
> dev->msi.data = md;
> devres_add(dev, md);
> return 0;
> }
>
> /**
> + * msi_lock_descs - Lock the MSI descriptor storage of a device
> + * @dev: Device to operate on
> + */
> +void msi_lock_descs(struct device *dev)
> +{
> + if (WARN_ON_ONCE(!dev->msi.data))
> + return;
Is this useful? Other places that call msi_lock_descs will continue on and deref
null dev->msi anyhow - is the dump from the WARN_ON that much better
than the oops from the null deref here:
> + mutex_lock(&dev->msi.data->mutex);
?
Honestly, still a bit unclear on what the community consensus is for
using WARN_ON.
Jason
On Wed, Dec 08 2021 at 20:47, Jason Gunthorpe wrote:
> On Mon, Dec 06, 2021 at 11:51:05PM +0100, Thomas Gleixner wrote:
>> +++ b/kernel/irq/msi.c
>> @@ -127,12 +127,37 @@ int msi_setup_device_data(struct device
>> return -ENOMEM;
>>
>> INIT_LIST_HEAD(&md->list);
>> + mutex_init(&md->mutex);
>> dev->msi.data = md;
>> devres_add(dev, md);
>> return 0;
>> }
>>
>> /**
>> + * msi_lock_descs - Lock the MSI descriptor storage of a device
>> + * @dev: Device to operate on
>> + */
>> +void msi_lock_descs(struct device *dev)
>> +{
>> + if (WARN_ON_ONCE(!dev->msi.data))
>> + return;
>
> Is this useful? Other places that call msi_lock_descs will continue on and deref
> null dev->msi anyhow - is the dump from the WARN_ON that much better
> than the oops from the null deref here:
>
>> + mutex_lock(&dev->msi.data->mutex);
I put it there for paranoia reasons and forgot to revist it later. In
that case yes, it's of questionable value.
Thanks,
tglx
The following commit has been merged into the irq/msi branch of tip:
Commit-ID: b5f687f97d1e112493fe0447a1fb09fbd93c334b
Gitweb: https://git.kernel.org/tip/b5f687f97d1e112493fe0447a1fb09fbd93c334b
Author: Thomas Gleixner <[email protected]>
AuthorDate: Mon, 06 Dec 2021 23:51:05 +01:00
Committer: Thomas Gleixner <[email protected]>
CommitterDate: Thu, 16 Dec 2021 22:22:17 +01:00
genirq/msi: Add mutex for MSI list protection
For upcoming runtime extensions of MSI-X interrupts it's required to
protect the MSI descriptor list. Add a mutex to struct msi_device_data and
provide lock/unlock functions.
Signed-off-by: Thomas Gleixner <[email protected]>
Tested-by: Michael Kelley <[email protected]>
Tested-by: Nishanth Menon <[email protected]>
Reviewed-by: Jason Gunthorpe <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
include/linux/msi.h | 5 +++++
kernel/irq/msi.c | 21 +++++++++++++++++++++
2 files changed, 26 insertions(+)
diff --git a/include/linux/msi.h b/include/linux/msi.h
index 4223e47..2cf6c53 100644
--- a/include/linux/msi.h
+++ b/include/linux/msi.h
@@ -3,6 +3,7 @@
#define LINUX_MSI_H
#include <linux/cpumask.h>
+#include <linux/mutex.h>
#include <linux/list.h>
#include <asm/msi.h>
@@ -145,17 +146,21 @@ struct msi_desc {
* @attrs: Pointer to the sysfs attribute group
* @platform_data: Platform-MSI specific data
* @list: List of MSI descriptors associated to the device
+ * @mutex: Mutex protecting the MSI list
*/
struct msi_device_data {
unsigned long properties;
const struct attribute_group **attrs;
struct platform_msi_priv_data *platform_data;
struct list_head list;
+ struct mutex mutex;
};
int msi_setup_device_data(struct device *dev);
unsigned int msi_get_virq(struct device *dev, unsigned int index);
+void msi_lock_descs(struct device *dev);
+void msi_unlock_descs(struct device *dev);
/* Helpers to hide struct msi_desc implementation details */
#define msi_desc_to_dev(desc) ((desc)->dev)
diff --git a/kernel/irq/msi.c b/kernel/irq/msi.c
index c66787d..97ec245 100644
--- a/kernel/irq/msi.c
+++ b/kernel/irq/msi.c
@@ -103,12 +103,33 @@ int msi_setup_device_data(struct device *dev)
return -ENOMEM;
INIT_LIST_HEAD(&md->list);
+ mutex_init(&md->mutex);
dev->msi.data = md;
devres_add(dev, md);
return 0;
}
/**
+ * msi_lock_descs - Lock the MSI descriptor storage of a device
+ * @dev: Device to operate on
+ */
+void msi_lock_descs(struct device *dev)
+{
+ mutex_lock(&dev->msi.data->mutex);
+}
+EXPORT_SYMBOL_GPL(msi_lock_descs);
+
+/**
+ * msi_unlock_descs - Unlock the MSI descriptor storage of a device
+ * @dev: Device to operate on
+ */
+void msi_unlock_descs(struct device *dev)
+{
+ mutex_unlock(&dev->msi.data->mutex);
+}
+EXPORT_SYMBOL_GPL(msi_unlock_descs);
+
+/**
* msi_get_virq - Return Linux interrupt number of a MSI interrupt
* @dev: Device to operate on
* @index: MSI interrupt index to look for (0-based)