| Message ID | 20230503131656.15928-1-mheyne@amazon.de |
|---|---|
| State | New |
| Headers | show |
| Series | x86/pci/xen: populate MSI sysfs entries | expand |
On 03.05.23 15:16, Maximilian Heyne wrote: > Commit bf5e758f02fc ("genirq/msi: Simplify sysfs handling") reworked the > creation of sysfs entries for MSI IRQs. The creation used to be in > msi_domain_alloc_irqs_descs_locked after calling ops->domain_alloc_irqs. > Then it moved into __msi_domain_alloc_irqs which is an implementation of > domain_alloc_irqs. However, Xen comes with the only other implementation > of domain_alloc_irqs and hence doesn't run the sysfs population code > anymore. > > Commit 6c796996ee70 ("x86/pci/xen: Fixup fallout from the PCI/MSI > overhaul") set the flag MSI_FLAG_DEV_SYSFS for the xen msi_domain_info > but that doesn't actually have an effect because Xen uses it's own > domain_alloc_irqs implementation. > > Fix this by making use of the fallback functions for sysfs population. > > Fixes: bf5e758f02fc ("genirq/msi: Simplify sysfs handling") > Signed-off-by: Maximilian Heyne <mheyne@amazon.de> Reviewed-by: Juergen Gross <jgross@suse.com> Juergen
On Wed, May 03, 2023 at 01:16:53PM +0000, Maximilian Heyne wrote: > Commit bf5e758f02fc ("genirq/msi: Simplify sysfs handling") reworked the > creation of sysfs entries for MSI IRQs. The creation used to be in > msi_domain_alloc_irqs_descs_locked after calling ops->domain_alloc_irqs. > Then it moved into __msi_domain_alloc_irqs which is an implementation of > domain_alloc_irqs. However, Xen comes with the only other implementation > of domain_alloc_irqs and hence doesn't run the sysfs population code > anymore. > > Commit 6c796996ee70 ("x86/pci/xen: Fixup fallout from the PCI/MSI > overhaul") set the flag MSI_FLAG_DEV_SYSFS for the xen msi_domain_info > but that doesn't actually have an effect because Xen uses it's own > domain_alloc_irqs implementation. > > Fix this by making use of the fallback functions for sysfs population. > > Fixes: bf5e758f02fc ("genirq/msi: Simplify sysfs handling") > Signed-off-by: Maximilian Heyne <mheyne@amazon.de> Any other feedback on this one? This is definitely a bug but I understand that there might be different ways to fix it. Amazon Development Center Germany GmbH Krausenstr. 38 10117 Berlin Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B Sitz: Berlin Ust-ID: DE 289 237 879
On 24.05.23 17:43, Maximilian Heyne wrote: > On Wed, May 03, 2023 at 01:16:53PM +0000, Maximilian Heyne wrote: >> Commit bf5e758f02fc ("genirq/msi: Simplify sysfs handling") reworked the >> creation of sysfs entries for MSI IRQs. The creation used to be in >> msi_domain_alloc_irqs_descs_locked after calling ops->domain_alloc_irqs. >> Then it moved into __msi_domain_alloc_irqs which is an implementation of >> domain_alloc_irqs. However, Xen comes with the only other implementation >> of domain_alloc_irqs and hence doesn't run the sysfs population code >> anymore. >> >> Commit 6c796996ee70 ("x86/pci/xen: Fixup fallout from the PCI/MSI >> overhaul") set the flag MSI_FLAG_DEV_SYSFS for the xen msi_domain_info >> but that doesn't actually have an effect because Xen uses it's own >> domain_alloc_irqs implementation. >> >> Fix this by making use of the fallback functions for sysfs population. >> >> Fixes: bf5e758f02fc ("genirq/msi: Simplify sysfs handling") >> Signed-off-by: Maximilian Heyne <mheyne@amazon.de> > > > Any other feedback on this one? This is definitely a bug but I understand that > there might be different ways to fix it. I'd be happy to take the patch via the Xen tree, but I think x86 maintainers should at least ack that. Juergen
On 5/24/23 08:47, Juergen Gross wrote: >> Any other feedback on this one? This is definitely a bug but I >> understand that >> there might be different ways to fix it. > > I'd be happy to take the patch via the Xen tree, but I think x86 > maintainers should at least ack that. Ack. Works for me.
diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c index 8babce71915f..014c508e914d 100644 --- a/arch/x86/pci/xen.c +++ b/arch/x86/pci/xen.c @@ -198,7 +198,7 @@ static int xen_setup_msi_irqs(struct pci_dev *dev, int nvec, int type) i++; } kfree(v); - return 0; + return msi_device_populate_sysfs(&dev->dev); error: if (ret == -ENOSYS) @@ -254,7 +254,7 @@ static int xen_hvm_setup_msi_irqs(struct pci_dev *dev, int nvec, int type) dev_dbg(&dev->dev, "xen: msi --> pirq=%d --> irq=%d\n", pirq, irq); } - return 0; + return msi_device_populate_sysfs(&dev->dev); error: dev_err(&dev->dev, "Failed to create MSI%s! ret=%d!\n", @@ -346,7 +346,7 @@ static int xen_initdom_setup_msi_irqs(struct pci_dev *dev, int nvec, int type) if (ret < 0) goto out; } - ret = 0; + ret = msi_device_populate_sysfs(&dev->dev); out: return ret; } @@ -394,6 +394,8 @@ static void xen_teardown_msi_irqs(struct pci_dev *dev) xen_destroy_irq(msidesc->irq + i); msidesc->irq = 0; } + + msi_device_destroy_sysfs(&dev->dev); } static void xen_pv_teardown_msi_irqs(struct pci_dev *dev) diff --git a/include/linux/msi.h b/include/linux/msi.h index cdb14a1ef268..a50ea79522f8 100644 --- a/include/linux/msi.h +++ b/include/linux/msi.h @@ -383,6 +383,13 @@ int arch_setup_msi_irq(struct pci_dev *dev, struct msi_desc *desc); void arch_teardown_msi_irq(unsigned int irq); int arch_setup_msi_irqs(struct pci_dev *dev, int nvec, int type); void arch_teardown_msi_irqs(struct pci_dev *dev); +#endif /* CONFIG_PCI_MSI_ARCH_FALLBACKS */ + +/* + * Xen uses non-default msi_domain_ops and hence needs a way to populate sysfs + * entries of MSI IRQs. + */ +#if defined(CONFIG_PCI_XEN) || defined(CONFIG_PCI_MSI_ARCH_FALLBACKS) #ifdef CONFIG_SYSFS int msi_device_populate_sysfs(struct device *dev); void msi_device_destroy_sysfs(struct device *dev); @@ -390,7 +397,7 @@ void msi_device_destroy_sysfs(struct device *dev); static inline int msi_device_populate_sysfs(struct device *dev) { return 0; } static inline void msi_device_destroy_sysfs(struct device *dev) { } #endif /* !CONFIG_SYSFS */ -#endif /* CONFIG_PCI_MSI_ARCH_FALLBACKS */ +#endif /* CONFIG_PCI_XEN || CONFIG_PCI_MSI_ARCH_FALLBACKS */ /* * The restore hook is still available even for fully irq domain based diff --git a/kernel/irq/msi.c b/kernel/irq/msi.c index 7a97bcb086bf..b4c31a5c1147 100644 --- a/kernel/irq/msi.c +++ b/kernel/irq/msi.c @@ -542,7 +542,7 @@ static int msi_sysfs_populate_desc(struct device *dev, struct msi_desc *desc) return ret; } -#ifdef CONFIG_PCI_MSI_ARCH_FALLBACKS +#if defined(CONFIG_PCI_MSI_ARCH_FALLBACKS) || defined(CONFIG_PCI_XEN) /** * msi_device_populate_sysfs - Populate msi_irqs sysfs entries for a device * @dev: The device (PCI, platform etc) which will get sysfs entries @@ -574,7 +574,7 @@ void msi_device_destroy_sysfs(struct device *dev) msi_for_each_desc(desc, dev, MSI_DESC_ALL) msi_sysfs_remove_desc(dev, desc); } -#endif /* CONFIG_PCI_MSI_ARCH_FALLBACK */ +#endif /* CONFIG_PCI_MSI_ARCH_FALLBACK || CONFIG_PCI_XEN */ #else /* CONFIG_SYSFS */ static inline int msi_sysfs_create_group(struct device *dev) { return 0; } static inline int msi_sysfs_populate_desc(struct device *dev, struct msi_desc *desc) { return 0; }
Commit bf5e758f02fc ("genirq/msi: Simplify sysfs handling") reworked the creation of sysfs entries for MSI IRQs. The creation used to be in msi_domain_alloc_irqs_descs_locked after calling ops->domain_alloc_irqs. Then it moved into __msi_domain_alloc_irqs which is an implementation of domain_alloc_irqs. However, Xen comes with the only other implementation of domain_alloc_irqs and hence doesn't run the sysfs population code anymore. Commit 6c796996ee70 ("x86/pci/xen: Fixup fallout from the PCI/MSI overhaul") set the flag MSI_FLAG_DEV_SYSFS for the xen msi_domain_info but that doesn't actually have an effect because Xen uses it's own domain_alloc_irqs implementation. Fix this by making use of the fallback functions for sysfs population. Fixes: bf5e758f02fc ("genirq/msi: Simplify sysfs handling") Signed-off-by: Maximilian Heyne <mheyne@amazon.de> --- arch/x86/pci/xen.c | 8 +++++--- include/linux/msi.h | 9 ++++++++- kernel/irq/msi.c | 4 ++-- 3 files changed, 15 insertions(+), 6 deletions(-)