Re: [PATCH 08/10] NTB: epf: Report 0-based doorbell vector via ntb_db_event()

On Wed, Feb 25, 2026 at 09:59:46AM -0700, Dave Jiang wrote:
> 
> 
> On 2/24/26 6:34 AM, Koichiro Den wrote:
> > ntb_db_event() expects the vector number to be relative to the first
> > doorbell vector starting at 0.
> > 
> > Vector 0 is reserved for link events in the EPF driver, so doorbells
> > start at vector 1. However, both supported peers (ntb_hw_epf with
> > pci-epf-ntb, and pci-epf-vntb) have historically skipped vector 1 and
> > started doorbells at vector 2.
> > 
> > Pass (irq_no - 2) to ntb_db_event() so doorbells are reported as 0..N-1.
> > If irq_no == 1 is ever observed, treat it as DB#0 and emit a warning, as
> > this would indicate an unexpected change in the slot layout.
> > 
> > Fixes: 812ce2f8d14e ("NTB: Add support for EPF PCI Non-Transparent Bridge")
> > Signed-off-by: Koichiro Den <[email protected]>
> > ---
> >  drivers/ntb/hw/epf/ntb_hw_epf.c | 11 ++++++++---
> >  1 file changed, 8 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/ntb/hw/epf/ntb_hw_epf.c b/drivers/ntb/hw/epf/ntb_hw_epf.c
> > index ee499eaed4f3..00956ab2fbf5 100644
> > --- a/drivers/ntb/hw/epf/ntb_hw_epf.c
> > +++ b/drivers/ntb/hw/epf/ntb_hw_epf.c
> > @@ -333,10 +333,15 @@ static irqreturn_t ntb_epf_vec_isr(int irq, void *dev)
> >  	irq_no = irq - pci_irq_vector(ndev->ntb.pdev, 0);
> >  	ndev->db_val = irq_no + 1;
> >  
> > -	if (irq_no == 0)
> > +	if (irq_no == 0) {
> >  		ntb_link_event(&ndev->ntb);
> > -	else
> > -		ntb_db_event(&ndev->ntb, irq_no);
> > +	} else if (irq_no == 1) {
> 
> Given that 0 and 1 have specific meanings, maybe create a enum with appropriate naming to make it more clear. Maybe something like this or however you want to name them:
> 
> enum EPF_IRQ_SLOT {
> 	EPF_IRQ_LINK = 0,
> 	EPF_IRQ_RESERVED_DB,
> 	EPF_IRQ_DB_START,
> };

Thanks for the suggestion, that sounds reasonable. I'd be happy to use the enum
names as proposed.

Frank, I'd like to introduce the same kind of enum in Patch 3 to eliminate
0/1/2 magic numbers for clarity and consistency. Please let me know if you have
a different view.

If you agree, I'll adjust vNTB part accordingly, and keep your Reviewed-by tags
if you're ok with that.

Thanks,
Koichiro

> > +		dev_warn_ratelimited(ndev->dev,
> > +				     "Unexpected irq_no 1 received. Treat it as DB#0.\n");
> > +		ntb_db_event(&ndev->ntb, 0);
> > +	} else {
> > +		ntb_db_event(&ndev->ntb, irq_no - 2);
> 
> And then here you can do
> ntb_db_event(&ndev->ntb, irq_no - EPF_IRQ_DB_START);
> 
> > +	}
> >  
> >  	return IRQ_HANDLED;
> >  }
> 
>