| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <jphfjfyr6ok76ljpf45ufxnzhicd5pcebu2t2jz35qpw5il6ai@ztf25y25fasf>
Date: Wed, 3 Dec 2025 23:12:20 +0900
From: Koichiro Den <den@...inux.co.jp>
To: Frank Li <Frank.li@....com>
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 10:58:08AM -0500, Frank Li wrote:
> 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().
My concern was that reusing struct pci_epf_bar, which represents an entire
BAR (starting at offset 0), for the new pci_epc_map_inbound(), might feel
semantically a bit unclean.
pci_epc_set_bars() idea sounds useful for some scenarios, thank you for the
suggestion. I also think it's not uncommon to want to add Address Match
Mode sub-range mappings incrementally, rather than configuring all ranges
in one shot through pci_epc_set_bars().
>
> If there are IOMMU in EP system, maybe use IOMMU to map to difference place
> is easier.
That certainly makes sense, while personally I would prefer a single, more
generic and intuitive interface that does not depend on the presence of an
IOMMU.
Thank you for the review,
Koichiro
>
> 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