[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <gb3mr7onokhufasxaeoxiqft22incwxxlf43m6jcrhrem3477j@63oi3ztvbqku>
Date: Tue, 6 Jan 2026 10:52:54 +0900
From: Koichiro Den <den@...inux.co.jp>
To: Niklas Cassel <cassel@...nel.org>
Cc: jingoohan1@...il.com, mani@...nel.org, lpieralisi@...nel.org,
kwilczynski@...nel.org, robh@...nel.org, bhelgaas@...gle.com, Frank.Li@....com,
linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 0/2] PCI: endpoint: BAR subrange mapping support
On Mon, Jan 05, 2026 at 05:55:30PM +0100, Niklas Cassel wrote:
> Hello Koichiro,
>
> On Mon, Jan 05, 2026 at 05:02:12PM +0900, Koichiro Den wrote:
> > This series proposes support for mapping subranges within a PCIe endpoint
> > BAR and enables controllers to program inbound address translation for
> > those subranges.
> >
> > The first patch introduces generic BAR subrange mapping support in the
> > PCI endpoint core. The second patch adds an implementation for the
> > DesignWare PCIe endpoint controller using Address Match Mode IB iATU.
> >
> > This series is a spin-off from a larger RFC series posted earlier:
> > https://lore.kernel.org/all/20251217151609.3162665-4-den@valinux.co.jp/
> >
> > Base:
> > git://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git
> > branch: controller/dwc
> > commit: 68ac85fb42cf ("PCI: dwc: Use cfg0_base as iMSI-RX target address
> > to support 32-bit MSI devices")
> >
> > Thank you for reviewing,
> >
> > Koichiro Den (2):
> > PCI: endpoint: Add BAR subrange mapping support
> > PCI: dwc: ep: Support BAR subrange inbound mapping via address-match
> > iATU
>
> I have nothing against this feature, but the big question here is:
> where is the user?
>
> Usually, we don't add a new feature without having a single user of said
> feature.
>
The first user will likely be Remote eDMA-backed NTB transport. An RFC
series,
https://lore.kernel.org/all/20251217151609.3162665-4-den@valinux.co.jp/
referenced in the cover letter relies on Address Match Mode support.
In that sense, this split series is prerequisite work, and if this gets
acked, I will post another patch series that utilizes this in the NTB code.
At least for Renesas R-Car S4, where 64-bit BAR0/BAR2 and 32-bit BAR4 are
available, exposing the eDMA regsister and LL regions to the host requires
at least two mappings (one for register and another for a contiguous LL
memory). Address Match Mode allows a flexible and extensible layout for the
required regions.
>
> One thing that I would like to see though:
> stricter verification of the pci_epf_bar_submap array.
>
> For DWC, we know that the minimum size that an iATU can map is stored in:
> (struct dw_pcie *pci) pci->region_align.
>
> Thus, each element in the pci_epf_bar_submap array has to have a size that
> is a multiple of pci->region_align.
>
> I don't see that you ever verify this anywhere.
I missed it, will add the check.
>
> You should also verify that the sum of all the sizes in the pci_epf_bar_submap
> array adds up to exactly pci_epf_bar->size.
I didn't think this was a requirement. I experimented with it just now, and
seems to me that no harm is caused even if the sum of the submap sizes is
less than the BAR size. Could you point me to any description of this
requirement in the databook if available?
>
> Also, we currently have code in dw_pcie_prog_inbound_atu() that verifies
> that the physical memory address is aligned to the size of the BAR, as that
> is a requirement in BAR match mode, see:
> 129f6af747b2 ("PCI: dwc: ep: Add 'address' alignment to 'size' check in dw_pcie_prog_ep_inbound_atu()")
>
> This is not a requirement in address match mode, so you probably don't
> want to do that check in address match mode.
> (You will want to keep the check that the physical memory address is
> aligned to pci->region_align though.)
With this series, the call graph would change as follows:
-> dw_pcie_ep_set_bar()
# For BAR Match Mode:
-> dw_pcie_ep_ib_atu_bar()
-> dw_pcie_prog_ep_inbound_atu()
# For Address Match Mode:
-> dw_pcie_ep_ib_atu_addr()
-> dw_pcie_prog_inbound_atu()
and the size check that was introduced in the commit 129f6af747b2 should
not run for Address Match Mode in any case.
Thank you for the review!
Koichiro
>
>
> Kind regards,
> Niklas
Powered by blists - more mailing lists