[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aS8MkMlNOaLANauE@lizhi-Precision-Tower-5810>
Date: Tue, 2 Dec 2025 10:58:08 -0500
From: Frank Li <Frank.li@....com>
To: Koichiro Den <den@...inux.co.jp>
Cc: ntb@...ts.linux.dev, linux-pci@...r.kernel.org,
dmaengine@...r.kernel.org, linux-kernel@...r.kernel.org,
mani@...nel.org, kwilczynski@...nel.org, kishon@...nel.org,
bhelgaas@...gle.com, corbet@....net, vkoul@...nel.org,
jdmason@...zu.us, dave.jiang@...el.com, allenbh@...il.com,
Basavaraj.Natikar@....com, Shyam-sundar.S-k@....com,
kurt.schwemmer@...rosemi.com, logang@...tatee.com,
jingoohan1@...il.com, lpieralisi@...nel.org, robh@...nel.org,
jbrunet@...libre.com, fancer.lancer@...il.com, arnd@...db.de,
pstanner@...hat.com, elfring@...rs.sourceforge.net
Subject: Re: [RFC PATCH v2 04/27] PCI: endpoint: Add inbound mapping ops to
EPC core
On Tue, Dec 02, 2025 at 03:25:31PM +0900, Koichiro Den wrote:
> On Mon, Dec 01, 2025 at 02:19:55PM -0500, Frank Li wrote:
> > On Sun, Nov 30, 2025 at 01:03:42AM +0900, Koichiro Den wrote:
> > > Add new EPC ops map_inbound() and unmap_inbound() for mapping a subrange
> > > of a BAR into CPU space. These will be implemented by controller drivers
> > > such as DesignWare.
> > >
> > > Signed-off-by: Koichiro Den <den@...inux.co.jp>
> > > ---
> > > drivers/pci/endpoint/pci-epc-core.c | 44 +++++++++++++++++++++++++++++
> > > include/linux/pci-epc.h | 11 ++++++++
> > > 2 files changed, 55 insertions(+)
> > >
> > > diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
> > > index ca7f19cc973a..825109e54ba9 100644
> > > --- a/drivers/pci/endpoint/pci-epc-core.c
> > > +++ b/drivers/pci/endpoint/pci-epc-core.c
> > > @@ -444,6 +444,50 @@ int pci_epc_map_addr(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> > > }
> > > EXPORT_SYMBOL_GPL(pci_epc_map_addr);
> > >
> > > +/**
> > > + * pci_epc_map_inbound() - map a BAR subrange to the local CPU address
> > > + * @epc: the EPC device on which BAR has to be configured
> > > + * @func_no: the physical endpoint function number in the EPC device
> > > + * @vfunc_no: the virtual endpoint function number in the physical function
> > > + * @epf_bar: the struct epf_bar that contains the BAR information
> > > + * @offset: byte offset from the BAR base selected by the host
> > > + *
> > > + * Invoke to configure the BAR of the endpoint device and map a subrange
> > > + * selected by @offset to a CPU address.
> > > + *
> > > + * Returns 0 on success, -EOPNOTSUPP if unsupported, or a negative errno.
> > > + */
> > > +int pci_epc_map_inbound(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> > > + struct pci_epf_bar *epf_bar, u64 offset)
> >
> > Supposed need size information? if BAR's size is 4G,
> >
> > you may just want to map from 0x4000 to 0x5000, not whole offset to end's
> > space.
>
> That sounds reasonable, the interface should accept a size parameter so that it
> is flexible enough to configure arbitrary sub-ranges inside a BAR, instead of
> implicitly using "offset to end of BAR".
>
> For the ntb_transport use_remote_edma=1 testing on R‑Car S4 I only needed at
> most two sub-ranges inside one BAR, so a size parameter was not strictly
> necessary in that setup, but I agree that the current interface looks
> half-baked and not very generic. I'll extend it to take size as well.
>
> >
> > commit message said map into CPU space, where CPU address?
>
> The interface currently requires a pointer to a struct pci_epf_bar instance and
> uses its phys_addr field as the CPU physical base address.
> I'm not fully convinced that using struct pci_epf_bar this way is the cleanest
> approach, so I'm open to better suggestions if you have any.
struct pci_epf_bar already have phys_addr and size information.
pci_epc_set_bars(..., struct pci_epf_bar *epf_bar, size_t num_of_bar)
to set many memory regions to one bar space. when num_of_bar is 1, fallback
to exitting pci_epc_set_bar().
If there are IOMMU in EP system, maybe use IOMMU to map to difference place
is easier.
Frank
>
> Koichiro
>
> >
> > Frank
> > > +{
> > > + if (!epc || !epc->ops || !epc->ops->map_inbound)
> > > + return -EOPNOTSUPP;
> > > +
> > > + return epc->ops->map_inbound(epc, func_no, vfunc_no, epf_bar, offset);
> > > +}
> > > +EXPORT_SYMBOL_GPL(pci_epc_map_inbound);
> > > +
> > > +/**
> > > + * pci_epc_unmap_inbound() - unmap a previously mapped BAR subrange
> > > + * @epc: the EPC device on which the inbound mapping was programmed
> > > + * @func_no: the physical endpoint function number in the EPC device
> > > + * @vfunc_no: the virtual endpoint function number in the physical function
> > > + * @epf_bar: the struct epf_bar used when the mapping was created
> > > + * @offset: byte offset from the BAR base that was mapped
> > > + *
> > > + * Invoke to remove a BAR subrange mapping created by pci_epc_map_inbound().
> > > + * If the controller has no support, this call is a no-op.
> > > + */
> > > +void pci_epc_unmap_inbound(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> > > + struct pci_epf_bar *epf_bar, u64 offset)
> > > +{
> > > + if (!epc || !epc->ops || !epc->ops->unmap_inbound)
> > > + return;
> > > +
> > > + epc->ops->unmap_inbound(epc, func_no, vfunc_no, epf_bar, offset);
> > > +}
> > > +EXPORT_SYMBOL_GPL(pci_epc_unmap_inbound);
> > > +
> > > /**
> > > * pci_epc_mem_map() - allocate and map a PCI address to a CPU address
> > > * @epc: the EPC device on which the CPU address is to be allocated and mapped
> > > diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
> > > index 4286bfdbfdfa..a5fb91cc2982 100644
> > > --- a/include/linux/pci-epc.h
> > > +++ b/include/linux/pci-epc.h
> > > @@ -71,6 +71,8 @@ struct pci_epc_map {
> > > * region
> > > * @map_addr: ops to map CPU address to PCI address
> > > * @unmap_addr: ops to unmap CPU address and PCI address
> > > + * @map_inbound: ops to map a subrange inside a BAR to CPU address.
> > > + * @unmap_inbound: ops to unmap a subrange inside a BAR and CPU address.
> > > * @set_msi: ops to set the requested number of MSI interrupts in the MSI
> > > * capability register
> > > * @get_msi: ops to get the number of MSI interrupts allocated by the RC from
> > > @@ -99,6 +101,10 @@ struct pci_epc_ops {
> > > phys_addr_t addr, u64 pci_addr, size_t size);
> > > void (*unmap_addr)(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> > > phys_addr_t addr);
> > > + int (*map_inbound)(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> > > + struct pci_epf_bar *epf_bar, u64 offset);
> > > + void (*unmap_inbound)(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> > > + struct pci_epf_bar *epf_bar, u64 offset);
> > > int (*set_msi)(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> > > u8 nr_irqs);
> > > int (*get_msi)(struct pci_epc *epc, u8 func_no, u8 vfunc_no);
> > > @@ -286,6 +292,11 @@ int pci_epc_map_addr(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> > > u64 pci_addr, size_t size);
> > > void pci_epc_unmap_addr(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> > > phys_addr_t phys_addr);
> > > +
> > > +int pci_epc_map_inbound(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> > > + struct pci_epf_bar *epf_bar, u64 offset);
> > > +void pci_epc_unmap_inbound(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> > > + struct pci_epf_bar *epf_bar, u64 offset);
> > > int pci_epc_set_msi(struct pci_epc *epc, u8 func_no, u8 vfunc_no, u8 nr_irqs);
> > > int pci_epc_get_msi(struct pci_epc *epc, u8 func_no, u8 vfunc_no);
> > > int pci_epc_set_msix(struct pci_epc *epc, u8 func_no, u8 vfunc_no, u16 nr_irqs,
> > > --
> > > 2.48.1
> > >
Powered by blists - more mailing lists