[v9,5/7] PCI: mediatek-gen3: Add MSI support

[Pali Rohár]

On Wednesday 24 March 2021 11:05:08 Jianjun Wang wrote:
> +static void mtk_pcie_msi_handler(struct mtk_pcie_port *port, int set_idx)
> +{
> +	struct mtk_msi_set *msi_set = &port->msi_sets[set_idx];
> +	unsigned long msi_enable, msi_status;
> +	unsigned int virq;
> +	irq_hw_number_t bit, hwirq;
> +
> +	msi_enable = readl_relaxed(msi_set->base + PCIE_MSI_SET_ENABLE_OFFSET);
> +
> +	do {
> +		msi_status = readl_relaxed(msi_set->base +
> +					   PCIE_MSI_SET_STATUS_OFFSET);
> +		msi_status &= msi_enable;
> +		if (!msi_status)
> +			break;
> +
> +		for_each_set_bit(bit, &msi_status, PCIE_MSI_IRQS_PER_SET) {
> +			hwirq = bit + set_idx * PCIE_MSI_IRQS_PER_SET;
> +			virq = irq_find_mapping(port->msi_bottom_domain, hwirq);
> +			generic_handle_irq(virq);
> +		}
> +	} while (true);

Hello!

Just a question, cannot this while-loop cause block of processing other
interrupts?

I have done tests with different HW (aardvark) but with same while(true)
loop logic. One XHCI PCIe controller was sending MSI interrupts too fast
and interrupt handler with this while(true) logic was in infinite loop.
During one IRQ it was calling infinite many times generic_handle_irq()
as HW was feeding new and new MSI hwirq into status register.

But this is different HW, so it can have different behavior and does not
have to cause above issue.

I have just spotted same code pattern for processing MSI interrupts...

> +}
> +
>  static void mtk_pcie_irq_handler(struct irq_desc *desc)
>  {
>  	struct mtk_pcie_port *port = irq_desc_get_handler_data(desc);
> @@ -405,6 +673,14 @@ static void mtk_pcie_irq_handler(struct irq_desc *desc)
>  		generic_handle_irq(virq);
>  	}
>  
> +	irq_bit = PCIE_MSI_SHIFT;
> +	for_each_set_bit_from(irq_bit, &status, PCIE_MSI_SET_NUM +
> +			      PCIE_MSI_SHIFT) {
> +		mtk_pcie_msi_handler(port, irq_bit - PCIE_MSI_SHIFT);
> +
> +		writel_relaxed(BIT(irq_bit), port->base + PCIE_INT_STATUS_REG);
> +	}
> +
>  	chained_irq_exit(irqchip, desc);
>  }
>  
> -- 
> 2.25.1
>