NAK: [PATCH] x86/pci/xen: populate MSI sysfs entries
Tim Gardner
tim.gardner at canonical.com
Mon Jun 5 13:43:20 UTC 2023
On 6/5/23 7:29 AM, Maximilian Heyne wrote:
> On Mon, Jun 05, 2023 at 07:20:42AM -0600, Tim Gardner wrote:
>> On 5/30/23 8:26 AM, Maximilian Heyne wrote:
>>> commit 335b4223466dd75f9f3ea4918187afbadd22e5c8 upstream.
>>>
>>> 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 at amazon.de>
>>> Reviewed-by: Juergen Gross <jgross at suse.com>
>>> Link: https://lore.kernel.org/r/20230503131656.15928-1-mheyne@amazon.de
>>> Signed-off-by: Juergen Gross <jgross at suse.com>
>>> ---
>>> arch/x86/pci/xen.c | 8 +++++---
>>> include/linux/msi.h | 9 ++++++++-
>>> kernel/irq/msi.c | 4 ++--
>>> 3 files changed, 15 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c
>>> index b94f727251b6..5a4ecf0c2ac4 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;
>>> }
>>> @@ -393,6 +393,8 @@ static void xen_teardown_msi_irqs(struct pci_dev *dev)
>>> for (i = 0; i < msidesc->nvec_used; i++)
>>> xen_destroy_irq(msidesc->irq + i);
>>> }
>>> +
>>> + 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 fc918a658d48..e5dfb9cf3aa1 100644
>>> --- a/include/linux/msi.h
>>> +++ b/include/linux/msi.h
>>> @@ -278,6 +278,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);
>>> @@ -285,7 +292,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 a9ee535293eb..77e513e2e5da 100644
>>> --- a/kernel/irq/msi.c
>>> +++ b/kernel/irq/msi.c
>>> @@ -420,7 +420,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
>>> @@ -452,7 +452,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; }
>>
>
> Hi Tim,
>
>> This patch needs a public Launchpad bug.
>
> I've created a launchpad bug in the meantime:
> https://bugs.launchpad.net/ubuntu/+source/linux-signed-aws-5.19/+bug/2022354
>
>>
>> Given that the "Fixes" commit was merged in 5.17, then I assume this patch is
>> targeted at Ubuntu kernels 5.19 and 6.2 ? That info needs to be in the subject
>> line, e.g.,
>>
>> [PATCH 1/1][Kinetic/Lunar] x86/pci/xen: populate MSI sysfs entries
>>
>
> sorry I didn't find the information how to properly post on this mailing list.
> Yes, this was supposed to go into kernel 5.19. If you also have 6.2 this would
> make sense in this series, too.
>
> Should I repost this with the proper subject line or is someone from canonical
> taking care?
>
Please repost with a v2 subject as mentioned above. Most devs will stop
looking if there is a NAK in the thread. Also use the short form of the
LP bug, e.g.,
BugLink: https://bugs.launchpad.net/bugs/2022354
Thanks.
rtg
--
-----------
Tim Gardner
Canonical, Inc
More information about the kernel-team
mailing list