Re: [PATCH v9 09/10] NTB: ntb_hw_epf: Discover vNTB-embedded DMA

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] NTB: ntb_hw_epf: DMA IRQ reservation conflicts with vNTB tail
  doorbell vector
--

commit f7a63c5c0603979fdb93290006c7e19909b1a6bf
Author: Koichiro Den <den@valinux.co.jp>

NTB: ntb_hw_epf: Discover vNTB-embedded DMA

This commit decodes the vNTB endpoint-local DMA extension in ntb_hw_epf
and registers advertised DW eDMA channels before registering the NTB
device.

> diff --git a/drivers/ntb/hw/epf/ntb_hw_epf.c b/drivers/ntb/hw/epf/ntb_hw_epf.c
> index ff92c2a61ce7d..e0016128b95c0 100644
> --- a/drivers/ntb/hw/epf/ntb_hw_epf.c
> +++ b/drivers/ntb/hw/epf/ntb_hw_epf.c

[ ... ]

> @@ -347,16 +581,20 @@ static int ntb_epf_init_isr(struct ntb_epf_dev *ndev, int msi_min, int msi_max)
>  {
>  	struct pci_dev *pdev = ndev->ntb.pdev;
>  	struct device *dev = ndev->dev;
> +	unsigned int dma_max = ndev->dma.nr_irqs;
> +	unsigned int dma_min;
>  	u32 argument = EPF_NTB_MSIX_ENABLE;
> +	unsigned int ntb_irqs;
>  	int irq;
>  	int ret;
>  	int i;
>  
> -	irq = pci_alloc_irq_vectors(pdev, msi_min, msi_max, PCI_IRQ_MSIX);
> +	irq = pci_alloc_irq_vectors(pdev, msi_min,
> +				    msi_max + dma_max, PCI_IRQ_MSIX);
>  	if (irq < 0) {
>  		dev_dbg(dev, "Failed to get MSIX interrupts\n");
> -		irq = pci_alloc_irq_vectors(pdev, msi_min, msi_max,
> -					    PCI_IRQ_MSI);
> +		irq = pci_alloc_irq_vectors(pdev, msi_min,
> +					    msi_max + dma_max, PCI_IRQ_MSI);
>  		if (irq < 0) {
>  			dev_err(dev, "Failed to get MSI interrupts\n");
>  			return irq;
> @@ -364,8 +602,17 @@ static int ntb_epf_init_isr(struct ntb_epf_dev *ndev, int msi_min, int msi_max)
>  		argument &= ~EPF_NTB_MSIX_ENABLE;
>  	}
>  
> -	ndev->db_count = irq - 1;
> -	for (i = 0; i < irq; i++) {
> +	dma_min = dma_max && irq > msi_min;
> +	/* Reserve one shared DMA IRQ when available, then give the rest to NTB. */
> +	ntb_irqs = min_t(unsigned int, irq - dma_min, msi_max);
> +	ndev->dma.nr_irqs = irq - ntb_irqs;
> +	if (dma_max && !ndev->dma.nr_irqs) {
> +		dev_warn(dev,
> +			 "No IRQ available for endpoint DMA, continuing without DMA\n");
> +		ndev->dma.type = 0;
> +	}
> +	ndev->db_count = ntb_irqs - 1;
> +	for (i = 0; i < ntb_irqs; i++) {
>  		ndev->irq_ctx[i].ndev = ndev;
>  		ndev->irq_ctx[i].irq_no = i;
>  		ret = request_irq(pci_irq_vector(pdev, i), ntb_epf_vec_isr,
> @@ -377,7 +624,7 @@ static int ntb_epf_init_isr(struct ntb_epf_dev *ndev, int msi_min, int msi_max)
>  	}
>  
>  	ret = ntb_epf_send_command(ndev, EPF_NTB_COMMAND_CONFIGURE_DOORBELL,
> -				   argument | irq);
> +				   argument | ntb_irqs);

[Severity: Medium]
Does reserving tail vectors for DMA here conflict with the endpoint's
last doorbell vector?

In MSI-X mode, pci-epf-vntb sets the MSI-X table size to ntb->db_count
(at most 32), so pci_alloc_irq_vectors() returns irq = ntb->db_count <=
msi_max.  When irq > msi_min, dma_min is 1, which sets ntb_irqs =
ntb->db_count - 1 and assigns 0-based vector ntb->db_count - 1 to
dw_edma via ntb_epf_dw_edma_irq_vector().  However, pci-epf-vntb ignores
the vector count in argument:

drivers/pci/endpoint/functions/pci-epf-vntb.c:epf_ntb_cmd_handler() {
	case EPF_NTB_COMMAND_CONFIGURE_DOORBELL:
		...
		ntb->peer_msix = argument & EPF_NTB_MSIX_ENABLE;
		ctrl->command_status = EPF_NTB_COMMAND_STATUS_OK;
		break;
}

Because vntb_epf_db_valid_mask() still exposes all ntb->db_count - 2
doorbells on the endpoint, ringing doorbell bit ntb->db_count - 3 raises
vector ntb->db_count - 1 on the host, routing the doorbell interrupt to
dw_edma instead of ntb_epf_vec_isr().  The mismatched
ntb_db_valid_mask() between host and endpoint also causes
ntb_transport_probe() to compute a different qp_count on each side when
mw_count >= ntb->db_count - 3, preventing the transport link from coming
up.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261110082459.1798763-1-den@valinux.co.jp?part=9