| Message ID | 20211126230524.045836616@linutronix.de |
|---|---|
| State | New |
| Headers | show |
| Series | genirq/msi, PCI/MSI: Spring cleaning - Part 2 | expand |
On Sat, Nov 27, 2021 at 02:20:09AM +0100, Thomas Gleixner wrote: > Create struct msi_device_data and add a pointer of that type to struct > dev_msi_info, which is part of struct device. Provide an allocator function > which can be invoked from the MSI interrupt allocation code pathes. > > Signed-off-by: Thomas Gleixner <tglx@linutronix.de> > --- > include/linux/device.h | 5 +++++ > include/linux/msi.h | 12 +++++++++++- > kernel/irq/msi.c | 33 +++++++++++++++++++++++++++++++++ > 3 files changed, 49 insertions(+), 1 deletion(-) Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
On Sat, Nov 27, 2021 at 02:20:09AM +0100, Thomas Gleixner wrote: > +/** > + * msi_setup_device_data - Setup MSI device data > + * @dev: Device for which MSI device data should be set up > + * > + * Return: 0 on success, appropriate error code otherwise > + * > + * This can be called more than once for @dev. If the MSI device data is > + * already allocated the call succeeds. The allocated memory is > + * automatically released when the device is destroyed. I would say 'by devres when the driver is removed' rather than device is destroyed - to me the latter implies it would happen at device_del or perhaps during release.. > +int msi_setup_device_data(struct device *dev) > +{ > + struct msi_device_data *md; > + > + if (dev->msi.data) > + return 0; > + > + md = devres_alloc(msi_device_data_release, sizeof(*md), GFP_KERNEL); > + if (!md) > + return -ENOMEM; > + > + raw_spin_lock_init(&md->lock); I also couldn't guess why this needed to be raw? Jason
On Sat, Nov 27 2021 at 20:14, Jason Gunthorpe wrote: > On Sat, Nov 27, 2021 at 02:20:09AM +0100, Thomas Gleixner wrote: > >> +/** >> + * msi_setup_device_data - Setup MSI device data >> + * @dev: Device for which MSI device data should be set up >> + * >> + * Return: 0 on success, appropriate error code otherwise >> + * >> + * This can be called more than once for @dev. If the MSI device data is >> + * already allocated the call succeeds. The allocated memory is >> + * automatically released when the device is destroyed. > > I would say 'by devres when the driver is removed' rather than device > is destroyed - to me the latter implies it would happen at device_del > or perhaps during release.. Ah. Indeed it's when the driver unbinds because that's what disables MSI. >> +int msi_setup_device_data(struct device *dev) >> +{ >> + struct msi_device_data *md; >> + >> + if (dev->msi.data) >> + return 0; >> + >> + md = devres_alloc(msi_device_data_release, sizeof(*md), GFP_KERNEL); >> + if (!md) >> + return -ENOMEM; >> + >> + raw_spin_lock_init(&md->lock); > > I also couldn't guess why this needed to be raw? That lock is to replace the raw spinlock we have in struct device right now, which is protecting low level register access and that's called from within irq_desc::lock held regions with interrupts disabled. I had to stick something into the data structure because allocating 0 bytes is invalid. But yes, I should have mentioned it in the changelog. Thanks, tglx
--- a/include/linux/device.h +++ b/include/linux/device.h @@ -45,6 +45,7 @@ struct iommu_ops; struct iommu_group; struct dev_pin_info; struct dev_iommu; +struct msi_device_data; /** * struct subsys_interface - interfaces to device functions @@ -374,11 +375,15 @@ struct dev_links_info { /** * struct dev_msi_info - Device data related to MSI * @domain: The MSI interrupt domain associated to the device + * @data: Pointer to MSI device data */ struct dev_msi_info { #ifdef CONFIG_GENERIC_MSI_IRQ_DOMAIN struct irq_domain *domain; #endif +#ifdef CONFIG_GENERIC_MSI_IRQ + struct msi_device_data *data; +#endif }; /** --- a/include/linux/msi.h +++ b/include/linux/msi.h @@ -2,6 +2,7 @@ #ifndef LINUX_MSI_H #define LINUX_MSI_H +#include <linux/spinlock.h> #include <linux/list.h> #include <asm/msi.h> @@ -170,6 +171,16 @@ struct msi_desc { }; }; +/** + * msi_device_data - MSI per device data + * @lock: Spinlock to protect register access + */ +struct msi_device_data { + raw_spinlock_t lock; +}; + +int msi_setup_device_data(struct device *dev); + /* Helpers to hide struct msi_desc implementation details */ #define msi_desc_to_dev(desc) ((desc)->dev) #define dev_to_msi_list(dev) (&(dev)->msi_list) @@ -185,7 +196,6 @@ struct msi_desc { for (__irq = (desc)->irq; \ __irq < ((desc)->irq + (desc)->nvec_used); \ __irq++) - #ifdef CONFIG_IRQ_MSI_IOMMU static inline const void *msi_desc_get_iommu_cookie(struct msi_desc *desc) { --- a/kernel/irq/msi.c +++ b/kernel/irq/msi.c @@ -73,6 +73,39 @@ void get_cached_msi_msg(unsigned int irq } EXPORT_SYMBOL_GPL(get_cached_msi_msg); +static void msi_device_data_release(struct device *dev, void *res) +{ + WARN_ON_ONCE(!list_empty(&dev->msi_list)); + dev->msi.data = NULL; +} + +/** + * msi_setup_device_data - Setup MSI device data + * @dev: Device for which MSI device data should be set up + * + * Return: 0 on success, appropriate error code otherwise + * + * This can be called more than once for @dev. If the MSI device data is + * already allocated the call succeeds. The allocated memory is + * automatically released when the device is destroyed. + */ +int msi_setup_device_data(struct device *dev) +{ + struct msi_device_data *md; + + if (dev->msi.data) + return 0; + + md = devres_alloc(msi_device_data_release, sizeof(*md), GFP_KERNEL); + if (!md) + return -ENOMEM; + + raw_spin_lock_init(&md->lock); + dev->msi.data = md; + devres_add(dev, md); + return 0; +} + #ifdef CONFIG_SYSFS static ssize_t msi_mode_show(struct device *dev, struct device_attribute *attr, char *buf)
Create struct msi_device_data and add a pointer of that type to struct dev_msi_info, which is part of struct device. Provide an allocator function which can be invoked from the MSI interrupt allocation code pathes. Signed-off-by: Thomas Gleixner <tglx@linutronix.de> --- include/linux/device.h | 5 +++++ include/linux/msi.h | 12 +++++++++++- kernel/irq/msi.c | 33 +++++++++++++++++++++++++++++++++ 3 files changed, 49 insertions(+), 1 deletion(-)