Re: [PATCH v2 0/3] PCI: endpoint: Support hardware-owned MSI-X table and PBA

Niklas Cassel <[email protected]>
Newsgroups org.infradead.lists.linux-nvme,dev.linux.lists.ntb,org.infradead.lists.linux-arm-kernel,org.infradead.lists.linux-rockchip,org.kernel.vger.linux-kernel,org.kernel.vger.linux-pci
Message-ID <apXdm2I33edo8kpZ@ryzen>
Hello Koichiro,

On Tue, Sep 01, 2026 at 01:11:09AM +0900, Koichiro Den wrote:
> This is not driven by a performance target, so I do not have numbers. The
> immediate issue is correctness. ntb_hw_epf can select MSI-X, but pci-epf-vntb
> currently configures and raises MSI. I want that MSI-X path to work on RK3588
> with the controller doorbell. I will make it clear in the next cover letter
> that, with this series, a doorbell sent from ntb_tool on the pci-epf-vntb side
> of RK3588 reaches ntb_tool on the ntb_hw_epf host side when MSI-X is selected.
> I believe that is the simplest E2E test showing what this series fixes.

Okay, if the main issue is that pci-epf-vntb does not raise MSI-X, even when
ntb_hw_epf host side has requested MSI-X, then I think that the first patch
in the series should fix this, so that the fix can easily be backported.

(Right now you fix this in patch 3/3, and the fix in patch 3/3 depends on
both patches 1/3 and 2/3.)

I think that pci-epf-vntb actually respecting the configured IRQ type by the
host is a separate logical change from adding support for HW defined layout.


> I initially considered making the hardware-owned layout automatic for every EPF.
> pci-epf-ntb (not vNTB!) is the reason I did not. It currently reads
> host-programmed MSI-X entries through epf_bar[] backing and uses them to set up
> peer outbound mappings. A hardware-owned Table has no such backing, and the
> current EPC API has no generic way to read those entries. I suspect that
> supporting it there would require a broader design change. Also, I do not have
> hardware for the pci-epf-ntb bridge configuration.
> 
> pci-epf-test and nvmet-pci-epf should be easier to convert. However, converting
> only those would not remove the per-layout handling while pci-epf-ntb still uses
> an EPF-owned Table. I would prefer to handle those separately when they can be
> properly tested. This is what I meant by this part of the cover letter:
> 
>      [...]
> 
>      layouts. This avoids unnecessary changes and reduces regression risk.
>      They can use a hardware-owned layout later if/when needed.

Okay, if pci-epf-ntb is special, perhaps we should add a comment in
pci-epf-ntb which explains why it cannot support HW defined layout.

But for consistency, if we modify pci-epf-vntb, I think it would make sense
to also modify pci-epf-test and nvmet-pci-epf as well in the same series.

pci-epf-test should be easy to test, just run the pci endpoint selftest.

nvmet-pci-epf should be quite easy to test too, just run the normal fio based
testing that we usually run, and unless you see worse performance, I think we
are good. (If we see worse performance, that could be an indication that one
of the MSI-X for the different completion queues are not being correctly
triggered. By default, the host side nvme driver creates one completion queue
(and one submission queue) per host CPU.


What I suggest that you instead do something like:
Patch 1/7: pci-epf-vntb: Fix to actually use MSI-X when requested.
           This probably includes adding the code that is inside the
	   if (ret == -ENOENT) { }.
Patch 2/7: PCI: endpoint: Support hardware-owned MSI-X table and PBA
Patch 3/7: PCI: dw-rockchip: Support fixed MSI-X table and PBA on RK3588
Patch 4/7: pci-epf-vntb: Add code that calls pci_epc_get_hw_msix_layout()
           and uses that layout, if the function returned success.
Patch 5/7: pci-epf-test: Add code that calls pci_epc_get_hw_msix_layout()
           and uses that layout, if the function returned success.
Patch 6/7: nvmet-pci-epf: Add code that calls pci_epc_get_hw_msix_layout()
           and uses that layout, if the function returned success.
Patch 7/7: pci-epf-ntb: Add comment that explains why it cannot call
           pci_epc_get_hw_msix_layout() like all other EPF drivers that
	   support MSI-X.


This way, patch 1/7 can be backported, and will help all users vNTB users,
even the users that do not use RK3588 as an EPC.


> > I understand that you introduce dw_pcie_ep_msix_layout_is_hw_owned()
> > because you want an EPF driver optionally use the HW defined table.
> > But if all EPF drivers always use the HW defined table if available, I think
> > you can avoid introducing this helper, and let rockchip_pcie_raise_irq()
> > unconditionally call dw_pcie_ep_raise_msix_irq_doorbell() for case PCI_IRQ_MSIX.
> 
> Yes, under that assumption I agree. With the current mix, however, the Rockchip
> callback still has to choose the helper according to the selected layout. That
> is why the DWC code records which layout was selected.
> 
> I will update the cover letter to state the motivation and this boundary more
> directly. Does keeping the layout choice with each EPF for this reason sound
> reasonable?

Yes, since pci-epf-ntb apparently is special, I can see why you need
both ep_func->msix_hw_owned and dw_pcie_ep_msix_layout_is_hw_owned().


Kind regards,
Niklas
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.