NAK: [PATCH v2 1/1][Kinetic/Lunar] x86/pci/xen: populate MSI sysfs entries

[Tim Gardner]

On 6/5/23 8:27 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; }

You're still missing the Buglink in the patch. Also use 'git cherry-pick 
-s -x 335b4223466dd75f9f3ea4918187afbadd22e5c8' so that provenance is 
preserved.

rtg
-- 
-----------
Tim Gardner
Canonical, Inc