Re: [PATCH v3 03/24] PCI/MSI: Provide MSI_FLAG_PCI_MSI_MASK_PARENT

[Bjorn Helgaas <helgaas@kernel.org>]

On Fri, Jun 14, 2024 at 12:23:42PM +0200, Shivamurthy Shastri wrote:
> Most ARM(64) PCI/MSI domains mask and unmask in the parent domain after or
> before the PCI mask/unmask operation takes place. So there are more than a
> dozen of the same wrapper implementation all over the place.

Is this an opportunity to clean up all these wrappers?  If you could
mention an example or two here, maybe somebody would be motivated to
come back and simplify the existing wrappers to take advantage of this
new flag?

> Don't make the same mistake with the new per device PCI/MSI domains and
> provide a new MSI feature flag, which lets the domain implementation
> enable this sequence in the PCI/MSI code.
> 
> Signed-off-by: Shivamurthy Shastri <shivamurthy.shastri@linutronix.de>

I assume you'll merge this series via some other tree, so:

Acked-by: Bjorn Helgaas <bhelgaas@google.com>

> ---
> v3: new patch to replace the global static key - Marc Zyngier
> ---
>  drivers/pci/msi/irqdomain.c | 21 +++++++++++++++++++++
>  include/linux/msi.h         |  2 ++
>  2 files changed, 23 insertions(+)
> 
> diff --git a/drivers/pci/msi/irqdomain.c b/drivers/pci/msi/irqdomain.c
> index 03d2dd25790d..112c2ff3035c 100644
> --- a/drivers/pci/msi/irqdomain.c
> +++ b/drivers/pci/msi/irqdomain.c
> @@ -148,17 +148,35 @@ static void pci_device_domain_set_desc(msi_alloc_info_t *arg, struct msi_desc *d
>  	arg->hwirq = desc->msi_index;
>  }
>  
> +static __always_inline void cond_mask_parent(struct irq_data *data)
> +{
> +	struct msi_domain_info *info = data->domain->host_data;
> +
> +	if (unlikely(info->flags & MSI_FLAG_PCI_MSI_MASK_PARENT))
> +		irq_chip_mask_parent(data);
> +}
> +
> +static __always_inline void cond_unmask_parent(struct irq_data *data)
> +{
> +	struct msi_domain_info *info = data->domain->host_data;
> +
> +	if (unlikely(info->flags & MSI_FLAG_PCI_MSI_MASK_PARENT))
> +		irq_chip_unmask_parent(data);
> +}
> +
>  static void pci_irq_mask_msi(struct irq_data *data)
>  {
>  	struct msi_desc *desc = irq_data_get_msi_desc(data);
>  
>  	pci_msi_mask(desc, BIT(data->irq - desc->irq));
> +	cond_mask_parent(data);
>  }
>  
>  static void pci_irq_unmask_msi(struct irq_data *data)
>  {
>  	struct msi_desc *desc = irq_data_get_msi_desc(data);
>  
> +	cond_unmask_parent(data);
>  	pci_msi_unmask(desc, BIT(data->irq - desc->irq));
>  }
>  
> @@ -170,6 +188,7 @@ static void pci_irq_unmask_msi(struct irq_data *data)
>  
>  #define MSI_COMMON_FLAGS	(MSI_FLAG_FREE_MSI_DESCS |	\
>  				 MSI_FLAG_ACTIVATE_EARLY |	\
> +				 MSI_FLAG_PCI_MSI_MASK_PARENT |	\
>  				 MSI_FLAG_DEV_SYSFS |		\
>  				 MSI_REACTIVATE)
>  
> @@ -195,10 +214,12 @@ static const struct msi_domain_template pci_msi_template = {
>  static void pci_irq_mask_msix(struct irq_data *data)
>  {
>  	pci_msix_mask(irq_data_get_msi_desc(data));
> +	cond_mask_parent(data);
>  }
>  
>  static void pci_irq_unmask_msix(struct irq_data *data)
>  {
> +	cond_unmask_parent(data);
>  	pci_msix_unmask(irq_data_get_msi_desc(data));
>  }
>  
> diff --git a/include/linux/msi.h b/include/linux/msi.h
> index dc27cf3903d5..04f33e7f6f8b 100644
> --- a/include/linux/msi.h
> +++ b/include/linux/msi.h
> @@ -556,6 +556,8 @@ enum {
>  	MSI_FLAG_USE_DEV_FWNODE		= (1 << 7),
>  	/* Set parent->dev into domain->pm_dev on device domain creation */
>  	MSI_FLAG_PARENT_PM_DEV		= (1 << 8),
> +	/* Support for parent mask/unmask */
> +	MSI_FLAG_PCI_MSI_MASK_PARENT	= (1 << 9),
>  
>  	/* Mask for the generic functionality */
>  	MSI_GENERIC_FLAGS_MASK		= GENMASK(15, 0),
> -- 
> 2.34.1
>