Re: [patch V2 12/23] PCI/MSI: Make arch_restore_msi_irqs() less horrible.

From: Bjorn Helgaas
Date: Tue Dec 07 2021 - 15:57:03 EST

Next message: Bjorn Helgaas: "Re: [patch V2 13/23] PCI/MSI: Cleanup include zoo"
Previous message: Alexander Lobakin: "[PATCH v3 net-next 4/9] ice: don't reserve excessive XDP_PACKET_HEADROOM on XSK Rx to skb"
In reply to: Thomas Gleixner: "[patch V2 12/23] PCI/MSI: Make arch_restore_msi_irqs() less horrible."
Next in thread: Thomas Gleixner: "[patch V2 17/23] PCI/MSI: Split out !IRQDOMAIN code"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Mon, Dec 06, 2021 at 11:27:42PM +0100, Thomas Gleixner wrote:
> Make arch_restore_msi_irqs() return a boolean which indicates whether the
> core code should restore the MSI message or not. Get rid of the indirection
> in x86.
>
> Signed-off-by: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> Tested-by: Juergen Gross <jgross@xxxxxxxx>
> Reviewed-by: Jason Gunthorpe <jgg@xxxxxxxxxx>
> Cc: x86@xxxxxxxxxx
> Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx
> Cc: Christian Borntraeger <borntraeger@xxxxxxxxxx>
> Cc: Heiko Carstens <hca@xxxxxxxxxxxxx>

Acked-by: Bjorn Helgaas <bhelgaas@xxxxxxxxxx> # PCI

> ---
> arch/s390/pci/pci_irq.c | 4 +-
> arch/x86/include/asm/x86_init.h | 6 ---
> arch/x86/include/asm/xen/hypervisor.h | 8 +++++
> arch/x86/kernel/apic/msi.c | 6 +++
> arch/x86/kernel/x86_init.c | 12 -------
> arch/x86/pci/xen.c | 13 ++++----
> drivers/pci/msi.c | 54 +++++++++++-----------------------
> include/linux/msi.h | 7 +---
> 8 files changed, 45 insertions(+), 65 deletions(-)
>
> --- a/arch/s390/pci/pci_irq.c
> +++ b/arch/s390/pci/pci_irq.c
> @@ -387,13 +387,13 @@ void arch_teardown_msi_irqs(struct pci_d
> airq_iv_free(zpci_ibv[0], zdev->msi_first_bit, zdev->msi_nr_irqs);
> }
>
> -void arch_restore_msi_irqs(struct pci_dev *pdev)
> +bool arch_restore_msi_irqs(struct pci_dev *pdev)
> {
> struct zpci_dev *zdev = to_zpci(pdev);
>
> if (!zdev->irqs_registered)
> zpci_set_irq(zdev);
> - default_restore_msi_irqs(pdev);
> + return true;
> }
>
> static struct airq_struct zpci_airq = {
> --- a/arch/x86/include/asm/x86_init.h
> +++ b/arch/x86/include/asm/x86_init.h
> @@ -289,12 +289,6 @@ struct x86_platform_ops {
> struct x86_hyper_runtime hyper;
> };
>
> -struct pci_dev;
> -
> -struct x86_msi_ops {
> - void (*restore_msi_irqs)(struct pci_dev *dev);
> -};
> -
> struct x86_apic_ops {
> unsigned int (*io_apic_read) (unsigned int apic, unsigned int reg);
> void (*restore)(void);
> --- a/arch/x86/include/asm/xen/hypervisor.h
> +++ b/arch/x86/include/asm/xen/hypervisor.h
> @@ -57,6 +57,14 @@ static inline bool __init xen_x2apic_par
> }
> #endif
>
> +struct pci_dev;
> +
> +#ifdef CONFIG_XEN_DOM0
> +bool xen_initdom_restore_msi(struct pci_dev *dev);
> +#else
> +static inline bool xen_initdom_restore_msi(struct pci_dev *dev) { return true; }
> +#endif
> +
> #ifdef CONFIG_HOTPLUG_CPU
> void xen_arch_register_cpu(int num);
> void xen_arch_unregister_cpu(int num);
> --- a/arch/x86/kernel/apic/msi.c
> +++ b/arch/x86/kernel/apic/msi.c
> @@ -19,6 +19,7 @@
> #include <asm/hw_irq.h>
> #include <asm/apic.h>
> #include <asm/irq_remapping.h>
> +#include <asm/xen/hypervisor.h>
>
> struct irq_domain *x86_pci_msi_default_domain __ro_after_init;
>
> @@ -345,3 +346,8 @@ void dmar_free_hwirq(int irq)
> irq_domain_free_irqs(irq, 1);
> }
> #endif
> +
> +bool arch_restore_msi_irqs(struct pci_dev *dev)
> +{
> + return xen_initdom_restore_msi(dev);
> +}
> --- a/arch/x86/kernel/x86_init.c
> +++ b/arch/x86/kernel/x86_init.c
> @@ -145,18 +145,6 @@ struct x86_platform_ops x86_platform __r
>
> EXPORT_SYMBOL_GPL(x86_platform);
>
> -#if defined(CONFIG_PCI_MSI)
> -struct x86_msi_ops x86_msi __ro_after_init = {
> - .restore_msi_irqs = default_restore_msi_irqs,
> -};
> -
> -/* MSI arch specific hooks */
> -void arch_restore_msi_irqs(struct pci_dev *dev)
> -{
> - x86_msi.restore_msi_irqs(dev);
> -}
> -#endif
> -
> struct x86_apic_ops x86_apic_ops __ro_after_init = {
> .io_apic_read = native_io_apic_read,
> .restore = native_restore_boot_irq_mode,
> --- a/arch/x86/pci/xen.c
> +++ b/arch/x86/pci/xen.c
> @@ -351,10 +351,13 @@ static int xen_initdom_setup_msi_irqs(st
> return ret;
> }
>
> -static void xen_initdom_restore_msi_irqs(struct pci_dev *dev)
> +bool xen_initdom_restore_msi(struct pci_dev *dev)
> {
> int ret = 0;
>
> + if (!xen_initial_domain())
> + return true;
> +
> if (pci_seg_supported) {
> struct physdev_pci_device restore_ext;
>
> @@ -375,10 +378,10 @@ static void xen_initdom_restore_msi_irqs
> ret = HYPERVISOR_physdev_op(PHYSDEVOP_restore_msi, &restore);
> WARN(ret && ret != -ENOSYS, "restore_msi -> %d\n", ret);
> }
> + return false;
> }
> #else /* CONFIG_XEN_PV_DOM0 */
> #define xen_initdom_setup_msi_irqs NULL
> -#define xen_initdom_restore_msi_irqs NULL
> #endif /* !CONFIG_XEN_PV_DOM0 */
>
> static void xen_teardown_msi_irqs(struct pci_dev *dev)
> @@ -466,12 +469,10 @@ static __init struct irq_domain *xen_cre
> static __init void xen_setup_pci_msi(void)
> {
> if (xen_pv_domain()) {
> - if (xen_initial_domain()) {
> + if (xen_initial_domain())
> xen_msi_ops.setup_msi_irqs = xen_initdom_setup_msi_irqs;
> - x86_msi.restore_msi_irqs = xen_initdom_restore_msi_irqs;
> - } else {
> + else
> xen_msi_ops.setup_msi_irqs = xen_setup_msi_irqs;
> - }
> xen_msi_ops.teardown_msi_irqs = xen_pv_teardown_msi_irqs;
> pci_msi_ignore_mask = 1;
> } else if (xen_hvm_domain()) {
> --- a/drivers/pci/msi.c
> +++ b/drivers/pci/msi.c
> @@ -106,29 +106,6 @@ void __weak arch_teardown_msi_irqs(struc
> }
> #endif /* CONFIG_PCI_MSI_ARCH_FALLBACKS */
>
> -static void default_restore_msi_irq(struct pci_dev *dev, int irq)
> -{
> - struct msi_desc *entry;
> -
> - entry = NULL;
> - if (dev->msix_enabled) {
> - for_each_pci_msi_entry(entry, dev) {
> - if (irq == entry->irq)
> - break;
> - }
> - } else if (dev->msi_enabled) {
> - entry = irq_get_msi_desc(irq);
> - }
> -
> - if (entry)
> - __pci_write_msi_msg(entry, &entry->msg);
> -}
> -
> -void __weak arch_restore_msi_irqs(struct pci_dev *dev)
> -{
> - return default_restore_msi_irqs(dev);
> -}
> -
> /*
> * PCI 2.3 does not specify mask bits for each MSI interrupt. Attempting to
> * mask all MSI interrupts by clearing the MSI enable bit does not work
> @@ -242,14 +219,6 @@ void pci_msi_unmask_irq(struct irq_data
> }
> EXPORT_SYMBOL_GPL(pci_msi_unmask_irq);
>
> -void default_restore_msi_irqs(struct pci_dev *dev)
> -{
> - struct msi_desc *entry;
> -
> - for_each_pci_msi_entry(entry, dev)
> - default_restore_msi_irq(dev, entry->irq);
> -}
> -
> void __pci_read_msi_msg(struct msi_desc *entry, struct msi_msg *msg)
> {
> struct pci_dev *dev = msi_desc_to_pci_dev(entry);
> @@ -403,10 +372,19 @@ static void pci_msi_set_enable(struct pc
> pci_write_config_word(dev, dev->msi_cap + PCI_MSI_FLAGS, control);
> }
>
> +/*
> + * Architecture override returns true when the PCI MSI message should be
> + * written by the generic restore function.
> + */
> +bool __weak arch_restore_msi_irqs(struct pci_dev *dev)
> +{
> + return true;
> +}
> +
> static void __pci_restore_msi_state(struct pci_dev *dev)
> {
> - u16 control;
> struct msi_desc *entry;
> + u16 control;
>
> if (!dev->msi_enabled)
> return;
> @@ -415,7 +393,8 @@ static void __pci_restore_msi_state(stru
>
> pci_intx_for_msi(dev, 0);
> pci_msi_set_enable(dev, 0);
> - arch_restore_msi_irqs(dev);
> + if (arch_restore_msi_irqs(dev))
> + __pci_write_msi_msg(entry, &entry->msg);
>
> pci_read_config_word(dev, dev->msi_cap + PCI_MSI_FLAGS, &control);
> pci_msi_update_mask(entry, 0, 0);
> @@ -437,6 +416,7 @@ static void pci_msix_clear_and_set_ctrl(
> static void __pci_restore_msix_state(struct pci_dev *dev)
> {
> struct msi_desc *entry;
> + bool write_msg;
>
> if (!dev->msix_enabled)
> return;
> @@ -447,9 +427,13 @@ static void __pci_restore_msix_state(str
> pci_msix_clear_and_set_ctrl(dev, 0,
> PCI_MSIX_FLAGS_ENABLE | PCI_MSIX_FLAGS_MASKALL);
>
> - arch_restore_msi_irqs(dev);
> - for_each_pci_msi_entry(entry, dev)
> + write_msg = arch_restore_msi_irqs(dev);
> +
> + for_each_pci_msi_entry(entry, dev) {
> + if (write_msg)
> + __pci_write_msi_msg(entry, &entry->msg);
> pci_msix_write_vector_ctrl(entry, entry->pci.msix_ctrl);
> + }
>
> pci_msix_clear_and_set_ctrl(dev, PCI_MSIX_FLAGS_MASKALL, 0);
> }
> --- a/include/linux/msi.h
> +++ b/include/linux/msi.h
> @@ -273,11 +273,10 @@ static inline void arch_teardown_msi_irq
> #endif
>
> /*
> - * The restore hooks are still available as they are useful even
> - * for fully irq domain based setups. Courtesy to XEN/X86.
> + * The restore hook is still available even for fully irq domain based
> + * setups. Courtesy to XEN/X86.
> */
> -void arch_restore_msi_irqs(struct pci_dev *dev);
> -void default_restore_msi_irqs(struct pci_dev *dev);
> +bool arch_restore_msi_irqs(struct pci_dev *dev);
>
> #ifdef CONFIG_GENERIC_MSI_IRQ_DOMAIN
>
>

Next message: Bjorn Helgaas: "Re: [patch V2 13/23] PCI/MSI: Cleanup include zoo"
Previous message: Alexander Lobakin: "[PATCH v3 net-next 4/9] ice: don't reserve excessive XDP_PACKET_HEADROOM on XSK Rx to skb"
In reply to: Thomas Gleixner: "[patch V2 12/23] PCI/MSI: Make arch_restore_msi_irqs() less horrible."
Next in thread: Thomas Gleixner: "[patch V2 17/23] PCI/MSI: Split out !IRQDOMAIN code"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]