Re: [patch V2 22/23] genirq/msi: Handle PCI/MSI allocation fail in core code

[Bjorn Helgaas <helgaas@xxxxxxxxxx>]

On Mon, Dec 06, 2021 at 11:27:59PM +0100, Thomas Gleixner wrote:
> Get rid of yet another irqdomain callback and let the core code return the
> already available information of how many descriptors could be allocated.
> 
> Signed-off-by: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> Tested-by: Juergen Gross <jgross@xxxxxxxx>
> Reviewed-by: Jason Gunthorpe <jgg@xxxxxxxxxx>

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

> ---
>  drivers/pci/msi/irqdomain.c |   13 -------------
>  include/linux/msi.h         |    5 +----
>  kernel/irq/msi.c            |   29 +++++++++++++++++++++++++----
>  3 files changed, 26 insertions(+), 21 deletions(-)
> 
> --- a/drivers/pci/msi/irqdomain.c
> +++ b/drivers/pci/msi/irqdomain.c
> @@ -95,16 +95,6 @@ static int pci_msi_domain_check_cap(stru
>       return 0;
>  }
>  
> -static int pci_msi_domain_handle_error(struct irq_domain *domain,
> -                                    struct msi_desc *desc, int error)
> -{
> -     /* Special handling to support __pci_enable_msi_range() */
> -     if (pci_msi_desc_is_multi_msi(desc) && error == -ENOSPC)
> -             return 1;
> -
> -     return error;
> -}
> -
>  static void pci_msi_domain_set_desc(msi_alloc_info_t *arg,
>                                   struct msi_desc *desc)
>  {
> @@ -115,7 +105,6 @@ static void pci_msi_domain_set_desc(msi_
>  static struct msi_domain_ops pci_msi_domain_ops_default = {
>       .set_desc       = pci_msi_domain_set_desc,
>       .msi_check      = pci_msi_domain_check_cap,
> -     .handle_error   = pci_msi_domain_handle_error,
>  };
>  
>  static void pci_msi_domain_update_dom_ops(struct msi_domain_info *info)
> @@ -129,8 +118,6 @@ static void pci_msi_domain_update_dom_op
>                       ops->set_desc = pci_msi_domain_set_desc;
>               if (ops->msi_check == NULL)
>                       ops->msi_check = pci_msi_domain_check_cap;
> -             if (ops->handle_error == NULL)
> -                     ops->handle_error = pci_msi_domain_handle_error;
>       }
>  }
>  
> --- a/include/linux/msi.h
> +++ b/include/linux/msi.h
> @@ -286,7 +286,6 @@ struct msi_domain_info;
>   * @msi_check:               Callback for verification of the 
> domain/info/dev data
>   * @msi_prepare:     Prepare the allocation of the interrupts in the domain
>   * @set_desc:                Set the msi descriptor for an interrupt
> - * @handle_error:    Optional error handler if the allocation fails
>   * @domain_alloc_irqs:       Optional function to override the default 
> allocation
>   *                   function.
>   * @domain_free_irqs:        Optional function to override the default free
> @@ -295,7 +294,7 @@ struct msi_domain_info;
>   * @get_hwirq, @msi_init and @msi_free are callbacks used by the underlying
>   * irqdomain.
>   *
> - * @msi_check, @msi_prepare, @handle_error and @set_desc are callbacks used 
> by
> + * @msi_check, @msi_prepare and @set_desc are callbacks used by
>   * msi_domain_alloc/free_irqs().
>   *
>   * @domain_alloc_irqs, @domain_free_irqs can be used to override the
> @@ -332,8 +331,6 @@ struct msi_domain_ops {
>                                      msi_alloc_info_t *arg);
>       void            (*set_desc)(msi_alloc_info_t *arg,
>                                   struct msi_desc *desc);
> -     int             (*handle_error)(struct irq_domain *domain,
> -                                     struct msi_desc *desc, int error);
>       int             (*domain_alloc_irqs)(struct irq_domain *domain,
>                                            struct device *dev, int nvec);
>       void            (*domain_free_irqs)(struct irq_domain *domain,
> --- a/kernel/irq/msi.c
> +++ b/kernel/irq/msi.c
> @@ -538,6 +538,27 @@ static bool msi_check_reservation_mode(s
>       return desc->pci.msi_attrib.is_msix || desc->pci.msi_attrib.can_mask;
>  }
>  
> +static int msi_handle_pci_fail(struct irq_domain *domain, struct msi_desc 
> *desc,
> +                            int allocated)
> +{
> +     switch(domain->bus_token) {
> +     case DOMAIN_BUS_PCI_MSI:
> +     case DOMAIN_BUS_VMD_MSI:
> +             if (IS_ENABLED(CONFIG_PCI_MSI))
> +                     break;
> +             fallthrough;
> +     default:
> +             return -ENOSPC;
> +     }
> +
> +     /* Let a failed PCI multi MSI allocation retry */
> +     if (desc->nvec_used > 1)
> +             return 1;
> +
> +     /* If there was a successful allocation let the caller know */
> +     return allocated ? allocated : -ENOSPC;
> +}
> +
>  int __msi_domain_alloc_irqs(struct irq_domain *domain, struct device *dev,
>                           int nvec)
>  {
> @@ -546,6 +567,7 @@ int __msi_domain_alloc_irqs(struct irq_d
>       struct irq_data *irq_data;
>       struct msi_desc *desc;
>       msi_alloc_info_t arg = { };
> +     int allocated = 0;
>       int i, ret, virq;
>       bool can_reserve;
>  
> @@ -560,16 +582,15 @@ int __msi_domain_alloc_irqs(struct irq_d
>                                              dev_to_node(dev), &arg, false,
>                                              desc->affinity);
>               if (virq < 0) {
> -                     ret = -ENOSPC;
> -                     if (ops->handle_error)
> -                             ret = ops->handle_error(domain, desc, ret);
> -                     return ret;
> +                     ret = msi_handle_pci_fail(domain, desc, allocated);
> +                     goto cleanup;
>               }
>  
>               for (i = 0; i < desc->nvec_used; i++) {
>                       irq_set_msi_desc_off(virq, i, desc);
>                       irq_debugfs_copy_devname(virq + i, dev);
>               }
> +             allocated++;
>       }
>  
>       can_reserve = msi_check_reservation_mode(domain, info, dev);
>