Linux-PCI Archive mirror
 help / color / mirror / Atom feed
From: Koichiro Den <den@valinux.co.jp>
To: Dave Jiang <dave.jiang@intel.com>,
	 Manivannan Sadhasivam <mani@kernel.org>
Cc: "Jon Mason" <jdmason@kudzu.us>, "Allen Hubbe" <allenbh@gmail.com>,
	"Krzysztof Wilczyński" <kwilczynski@kernel.org>,
	"Kishon Vijay Abraham I" <kishon@kernel.org>,
	"Bjorn Helgaas" <bhelgaas@google.com>,
	"Frank Li" <Frank.Li@nxp.com>,
	"Jerome Brunet" <jbrunet@baylibre.com>,
	"Lorenzo Pieralisi" <lpieralisi@kernel.org>,
	"Niklas Cassel" <cassel@kernel.org>,
	linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org,
	ntb@lists.linux.dev
Subject: Re: [PATCH v3 00/10] PCI: endpoint: pci-epf-vntb / NTB: epf: Enable per-doorbell bit handling
Date: Wed, 8 Apr 2026 00:21:21 +0900	[thread overview]
Message-ID: <64nprproiiwijfwzcizxu2aorieyy7rhek5qq5imgxomtdlrsk@47fdrlbff6o3> (raw)
In-Reply-To: <7jkibbdz3m3vuq2kq2xxnkt2rjdt5v7itgowc7qormtwgnf6tv@krl5brfjul5y>

On Tue, Mar 24, 2026 at 12:43:55AM +0900, Koichiro Den wrote:
> On Mon, Mar 23, 2026 at 12:15:34PM +0900, Koichiro Den wrote:
> > This series fixes doorbell bit/vector handling for the EPF-based NTB
> > pair (ntb_hw_epf <-> pci-epf-*ntb). Its primary goal is to enable safe
> > per-db-vector handling in the NTB core and clients (e.g. ntb_transport),
> > without changing the on-the-wire doorbell mapping.
> > 
> > 
> > Background / problem
> > ====================
> > 
> > ntb_hw_epf historically applies an extra offset when ringing peer
> > doorbells: the link event uses the first interrupt slot, and doorbells
> > start from the third slot (i.e. a second slot is effectively unused).
> > pci-epf-vntb carries the matching offset on the EP side as well.
> > 
> > As long as db_vector_count()/db_vector_mask() are not implemented, this
> > mismatch is mostly masked. Doorbell events are effectively treated as
> > "can hit any QP" and the off-by-one vector numbering does not surface
> > clearly.
> > 
> > However, once per-vector handling is enabled, the current state becomes
> > problematic:
> > 
> >   - db_valid_mask exposes bits that do not correspond to real doorbells
> >     (link/unused slots leak into the mask).
> >   - ntb_db_event() is fed with 1-based/shifted vectors, while NTB core
> >     expects a 0-based db_vector for doorbells.
> >   - On pci-epf-vntb, .peer_db_set() may be called in atomic context, but
> >     it directly calls pci_epc_raise_irq(), which can sleep.
> > 
> > 
> > Why NOT fix the root offset?
> > ============================
> > 
> > The natural "root" fix would be to remove the historical extra offset in
> > the peer_db_set() doorbell paths for ntb_hw_epf and pci-epf-vntb.
> > Unfortunately this would lead to interoperability issues when mixing old
> > and new kernel versions (old/new peers). A new side would ring a
> > different interrupt slot than what an old peer expects, leading to
> > missed or misrouted doorbells, once db_vector_count()/db_vector_mask()
> > are implemented.
> > 
> > Therefore this series intentionally keeps the legacy offset, and instead
> > fixes the surrounding pieces so the mapping is documented and handled
> > consistently in masks, vector numbering, and per-vector reporting.
> > 
> > 
> > What this series does
> > =====================
> > 
> > - pci-epf-vntb:
> > 
> >   - Document the legacy offset.
> >   - Defer MSI doorbell raises to process context to avoid sleeping in
> >     atomic context. This becomes relevant once multiple doorbells are
> >     raised concurrently at a high rate.
> >   - Report doorbell vectors as 0-based to ntb_db_event().
> >   - Fix db_valid_mask and implement db_vector_count()/db_vector_mask().
> > 
> > - ntb_hw_epf:
> > 
> >   - Document the legacy offset in ntb_epf_peer_db_set().
> >   - Fix db_valid_mask to cover only real doorbell bits.
> >   - Report 0-based db_vector to ntb_db_event() (accounting for the
> >     unused slot).
> >   - Keep db_val as a bitmask and fix db_read/db_clear semantics
> >     accordingly.
> >   - Implement db_vector_count()/db_vector_mask().
> > 
> > 
> > Compatibility
> > =============
> > 
> > By keeping the legacy offset intact, this series aims to remain
> > compatible across mixed kernel versions. The observable changes are
> > limited to correct mask/vector reporting and safer execution context
> > handling.
> > 
> > Patches 1-5 (PCI Endpoint) and 6-10 (NTB) are independent and can be
> > applied separately through the respective trees. They are sent together
> > in this v3 for convenience.
> > 
> > Once the remaining acks from NTB maintainers are collected, the plan is
> 
> Hi Dave,
> 
> When you have a chance, I'd appreciate another look at patches 6-10 (which are
> unchanged since v2). If you do not see any blockers, Acked-by would be
> greatly appreciated.

Hi Dave (cc: Mani),

Just a gentle ping on this.

I'd appreciate your thoughts on patches 6-10 when you have a chance. If they
look fine to you, an Acked-by from the NTB side would be greatly appreciated.

Best regards,
Koichiro

> 
> 
> P.S. Regarding Sashiko's feedback [1], my understanding is that there are no
> blockers, but there are a few points that would be better addressed separately
> as orthogonal follow-ups:
> 
> - configfs knobs mutability and bounds checking, including (but not limited to)
>   db_count.
>   In my opinion, allowing updates after .bind() looks questionable, and
>   returning -EBUSY once bound seems more appropriate. I'm leaning toward
>   handling this as a separate hardening series.
> 
> - ntb_hw_epf IRQ unwind concern.
>   This is what I was trying to address in [2], which I hope will land soon.
> 
> - Other lifecycle concerns.
>   These are largely tied to the current vNTB implementation and were part of
>   [3], for which I still plan to post a follow-up series that adds .remove()
>   implementation to vntb_pci_driver.
> 
> [1] https://sashiko.dev/#/patchset/20260323031544.2598111-1-den%40valinux.co.jp
> [2] https://lore.kernel.org/ntb/20260304083028.1391068-1-den@valinux.co.jp/
> [3] https://lore.kernel.org/all/20260226084142.2226875-1-den@valinux.co.jp/
> 
> 
> Best regards,
> Koichiro
> 
> > to take the whole series through the PCI EP tree. See:
> > https://lore.kernel.org/linux-pci/rnzsnp5de4qf5w7smebkmqekpuaqckltx73rj6ha3q2nrby5yp@7hsgvdzvjkp6/
> > 
> > ---
> > Changelog
> > =========
> > 
> > Changes since v2:
> >   - No functional changes.
> >   - Rebased onto current pci/endpoint
> >     e022f0c72c7f ("selftests: pci_endpoint: Skip reserved BARs").
> >     * Patch 2 needed a trivial context-only adjustment while rebasing, due
> >       to commit d799984233a5 ("PCI: endpoint: pci-epf-vntb: Stop
> >       cmd_handler work in epf_ntb_epc_cleanup").
> >   - Picked up additional Reviewed-by tags from Frank.
> >   - Fixed the incorrect v2 series title.
> > 
> > Changes since v1:
> >   - Addressed feedback from Dave (add a source code comment, introduce
> >     enum to eliminate magic numbers)
> >   - Updated source code comment in Patch 2.
> >   - No functional changes, so retained Reviewed-by tags by Frank and Dave.
> >     Thank you both for the review.
> > 
> > v2: https://lore.kernel.org/linux-pci/20260227084955.3184017-1-den@valinux.co.jp/
> > v1: https://lore.kernel.org/linux-pci/20260224133459.1741537-1-den@valinux.co.jp/
> > 
> > 
> > Best regards,
> > Koichiro
> > 
> > 
> > Koichiro Den (10):
> >   PCI: endpoint: pci-epf-vntb: Document legacy MSI doorbell offset
> >   PCI: endpoint: pci-epf-vntb: Defer pci_epc_raise_irq() out of atomic
> >     context
> >   PCI: endpoint: pci-epf-vntb: Report 0-based doorbell vector via
> >     ntb_db_event()
> >   PCI: endpoint: pci-epf-vntb: Exclude reserved slots from db_valid_mask
> >   PCI: endpoint: pci-epf-vntb: Implement db_vector_count/mask for
> >     doorbells
> >   NTB: epf: Document legacy doorbell slot offset in
> >     ntb_epf_peer_db_set()
> >   NTB: epf: Make db_valid_mask cover only real doorbell bits
> >   NTB: epf: Report 0-based doorbell vector via ntb_db_event()
> >   NTB: epf: Fix doorbell bitmask handling in db_read/db_clear
> >   NTB: epf: Implement db_vector_count/mask for doorbells
> > 
> >  drivers/ntb/hw/epf/ntb_hw_epf.c               |  89 ++++++++++-
> >  drivers/pci/endpoint/functions/pci-epf-vntb.c | 147 +++++++++++++++---
> >  2 files changed, 210 insertions(+), 26 deletions(-)
> > 
> > -- 
> > 2.51.0
> > 
> > 

      parent reply	other threads:[~2026-04-07 15:21 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-23  3:15 [PATCH v3 00/10] PCI: endpoint: pci-epf-vntb / NTB: epf: Enable per-doorbell bit handling Koichiro Den
2026-03-23  3:15 ` [PATCH v3 01/10] PCI: endpoint: pci-epf-vntb: Document legacy MSI doorbell offset Koichiro Den
2026-03-23  3:15 ` [PATCH v3 02/10] PCI: endpoint: pci-epf-vntb: Defer pci_epc_raise_irq() out of atomic context Koichiro Den
2026-03-23 18:27   ` Frank Li
2026-05-01 16:26   ` Manivannan Sadhasivam
2026-05-11  7:54     ` Koichiro Den
2026-03-23  3:15 ` [PATCH v3 03/10] PCI: endpoint: pci-epf-vntb: Report 0-based doorbell vector via ntb_db_event() Koichiro Den
2026-03-23  3:15 ` [PATCH v3 04/10] PCI: endpoint: pci-epf-vntb: Exclude reserved slots from db_valid_mask Koichiro Den
2026-03-23  3:15 ` [PATCH v3 05/10] PCI: endpoint: pci-epf-vntb: Implement db_vector_count/mask for doorbells Koichiro Den
2026-03-23  3:15 ` [PATCH v3 06/10] NTB: epf: Document legacy doorbell slot offset in ntb_epf_peer_db_set() Koichiro Den
2026-03-23  3:15 ` [PATCH v3 07/10] NTB: epf: Make db_valid_mask cover only real doorbell bits Koichiro Den
2026-03-23  3:15 ` [PATCH v3 08/10] NTB: epf: Report 0-based doorbell vector via ntb_db_event() Koichiro Den
2026-03-23  3:15 ` [PATCH v3 09/10] NTB: epf: Fix doorbell bitmask handling in db_read/db_clear Koichiro Den
2026-05-01 16:29   ` Manivannan Sadhasivam
2026-05-11  9:15     ` Koichiro Den
2026-03-23  3:15 ` [PATCH v3 10/10] NTB: epf: Implement db_vector_count/mask for doorbells Koichiro Den
2026-03-23 15:43 ` [PATCH v3 00/10] PCI: endpoint: pci-epf-vntb / NTB: epf: Enable per-doorbell bit handling Koichiro Den
2026-03-25  6:23   ` Niklas Cassel
2026-03-25  8:44     ` Koichiro Den
2026-04-07 15:21   ` Koichiro Den [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=64nprproiiwijfwzcizxu2aorieyy7rhek5qq5imgxomtdlrsk@47fdrlbff6o3 \
    --to=den@valinux.co.jp \
    --cc=Frank.Li@nxp.com \
    --cc=allenbh@gmail.com \
    --cc=bhelgaas@google.com \
    --cc=cassel@kernel.org \
    --cc=dave.jiang@intel.com \
    --cc=jbrunet@baylibre.com \
    --cc=jdmason@kudzu.us \
    --cc=kishon@kernel.org \
    --cc=kwilczynski@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=lpieralisi@kernel.org \
    --cc=mani@kernel.org \
    --cc=ntb@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).