Re: [PATCH v4 12/12] NTB: epf: Implement db_vector_count/mask for doorbells

[Bjorn Helgaas <helgaas@kernel.org>]

On Wed, May 13, 2026 at 11:49:23AM +0900, Koichiro Den wrote:
> Implement .db_vector_count and .db_vector_mask so ntb core/clients can
> map doorbell events to per-vector work.
> 
> Report vectors as 0..(db_count - 2) (skipping the unused slot) and return
> BIT_ULL(db_vector) for the corresponding doorbell bit. Use
> ntb_epf_db_vector_count() for bounds checks in ntb_epf_db_vector_mask(), so
> the same lower-bound guard is applied before building the bitmask.
> 
> Signed-off-by: Koichiro Den <den@valinux.co.jp>
> ---
> Changes since v3:
>   - Reuse ntb_epf_db_vector_count() from ntb_epf_db_vector_mask() for bounds.
>   - Return 0 when db_count is below NTB_EPF_MIN_DB_COUNT.
>   - Drop Reviewed-by tags due to the changes.
> 
>  drivers/ntb/hw/epf/ntb_hw_epf.c | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
> 
> diff --git a/drivers/ntb/hw/epf/ntb_hw_epf.c b/drivers/ntb/hw/epf/ntb_hw_epf.c
> index 10618e462229..af5755472842 100644
> --- a/drivers/ntb/hw/epf/ntb_hw_epf.c
> +++ b/drivers/ntb/hw/epf/ntb_hw_epf.c
> @@ -434,6 +434,36 @@ static u64 ntb_epf_db_valid_mask(struct ntb_dev *ntb)
>  	return ntb_ndev(ntb)->db_valid_mask;
>  }
>  
> +static int ntb_epf_db_vector_count(struct ntb_dev *ntb)
> +{
> +	struct ntb_epf_dev *ndev = ntb_ndev(ntb);
> +	unsigned int db_count = ndev->db_count;
> +
> +	/*
> +	 * db_count includes an extra skipped slot due to the legacy
> +	 * doorbell layout. Expose only the real doorbell vectors.
> +	 */
> +	if (db_count < NTB_EPF_MIN_DB_COUNT)
> +		return 0;
> +
> +	return db_count - 1;
> +}
> +
> +static u64 ntb_epf_db_vector_mask(struct ntb_dev *ntb, int db_vector)
> +{
> +	int nr_vec;
> +
> +	/*
> +	 * db_count includes one skipped slot in the legacy layout. Valid
> +	 * doorbell vectors are therefore [0 .. (db_count - 2)].
> +	 */
> +	nr_vec = ntb_epf_db_vector_count(ntb);
> +	if (db_vector < 0 || db_vector >= nr_vec)
> +		return 0;
> +
> +	return BIT_ULL(db_vector);

This all ends up looking slightly different from pci-epf-vntb.c, and
it's not obvious whether the differences are essential or superfluous.

pci-epf-vntb.c:

  enum EPF_IRQ_DB_START                           # value 2
  #define MIN_DB_COUNT  (EPF_IRQ_DB_START + 1)    # value 3

  vntb_epf_db_vector_count() comment says "db_count is total number of
  doorbell slots exposed to peer, including reserved ones".  But it
  returns "db_count - EPF_IRQ_DB_START", which suggests that the
  reserves slots are not exposed?

  vntb_epf_db_vector_count() comment lists two reserved slots

  vntb_epf_db_vector_count() returns "db_count - EPF_IRQ_DB_START"

  vntb_epf_db_vector_mask() comment mentions "two reserved slots"

ntb_hw_epf.c:

  enum EPF_IRQ_DB_START                # value 2
  #define NTB_EPF_MIN_DB_COUNT  3      # value 3, not based on EPF_IRQ_DB_START

  ntb_epf_db_vector_count() comment says "expose only real doorbell
  vectors".  I guess this *excludes* the reserved ones?

  ntb_epf_db_vector_count() comment mentions "an extra skipped slot"
  but apparently actually accounts for *two* slots
  (NTB_EPF_MIN_DB_COUNT==3)

  ntb_epf_db_vector_count() returns "db_count - 1"   # should it be -2?

  ntb_epf_db_vector_mask() comment mentions "one skipped slot"

I don't want to change anything during the v7.2 merge window, but if
there are any actual bugs here, they could be fixed after v7.2-rc1.
If there are no bugs but this could be clarified to reduce confusion,
we could do that for v7.3.