linux-kernel - Re: [PATCH v2 2/3] PCI: endpoint: Add API pci_epf_set_inbound

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aNQ59vGIu82JtFhT@lizhi-Precision-Tower-5810>
Date: Wed, 24 Sep 2025 14:35:34 -0400
From: Frank Li <Frank.li@....com>
To: Manivannan Sadhasivam <mani@...nel.org>
Cc: Krzysztof Wilczyński <kwilczynski@...nel.org>,
	Kishon Vijay Abraham I <kishon@...nel.org>,
	Bjorn Helgaas <bhelgaas@...gle.com>, Jon Mason <jdmason@...zu.us>,
	Dave Jiang <dave.jiang@...el.com>, Allen Hubbe <allenbh@...il.com>,
	linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org,
	ntb@...ts.linux.dev, imx@...ts.linux.dev
Subject: Re: [PATCH v2 2/3] PCI: endpoint: Add API pci_epf_set_inbound_space()

On Wed, Sep 24, 2025 at 11:39:56PM +0530, Manivannan Sadhasivam wrote:
> On Mon, Sep 15, 2025 at 06:22:45PM -0400, Frank Li wrote:
> > Add pci_epf_set_inbound_space() to allow setting any physical address as
> > inbound memory space, such as an MSI message base address.
> >
> > Since PCI BAR size must be a power of two, the input MMIO range
> > [inbound_addr, inbound_addr + size) is mapped by finding n such that
> > [base, base + 2^n) covers the range.
>
> > The base address is also required
> > to be aligned to 2^n.
> >
>
> Where does this alignment restriction gets checked?

Needn't check here because set physical address to
ALIGN_DOWN(inbound_addr, aligned_size) in this function, which must be
aligned.

memory allocate return pointer also align to size. So check is not
necessary.

Frank

>
> > Signed-off-by: Frank Li <Frank.Li@....com>
> > ---
> > change in v2
> > - add new API pci_epf_set_inbound_space()
> > - fix bits 8 * size_of(dma_addr_t);
> > ---
> >  drivers/pci/endpoint/pci-epf-core.c | 86 +++++++++++++++++++++++++++++++++++++
> >  include/linux/pci-epf.h             |  6 +++
> >  2 files changed, 92 insertions(+)
> >
> > diff --git a/drivers/pci/endpoint/pci-epf-core.c b/drivers/pci/endpoint/pci-epf-core.c
> > index 4281067d4a62da6fbf6c2fb807b0f1b5afd1f45b..cd10e8619d03c7e2ffe48e437b0aecf0e8a499f4 100644
> > --- a/drivers/pci/endpoint/pci-epf-core.c
> > +++ b/drivers/pci/endpoint/pci-epf-core.c
> > @@ -344,6 +344,92 @@ void *pci_epf_alloc_space(struct pci_epf *epf, size_t size, enum pci_barno bar,
> >  }
> >  EXPORT_SYMBOL_GPL(pci_epf_alloc_space);
> >
> > +/**
> > + * pci_epf_set_inbound_space() - set MMIO for the PCI EPF register space
>
> This is just assigning the memory for @bar in epf_bar. So, how about,
> pci_epf_assign_bar_space()?
>
> 'Assign PCI EPF BAR space'
>
> > + * @epf: the EPF device to whom allocate the memory
> > + * @size: the size of the memory that has to be allocated
>
> s/allocated/assigned since this API is not allocating the space.
>
> > + * @bar: the BAR number corresponding to the allocated register space
> > + * @epc_features: the features provided by the EPC specific to this EPF
> > + * @type: Identifies if the allocation is for primary EPC or secondary EPC
> > + * @inbound_addr: Any physical address space such as MSI message address that
>
> s/inbound_addr/bar_addr
>
> "Address to be assigned for the @bar"
>
> > + *		work as inbound address space. from_memory need be false.
> > + *
> > + * Invoke to allocate memory for the PCI EPF register space.
>
> s/allocate/assign
>
> > + * Flag PCI_BASE_ADDRESS_MEM_TYPE_64 will automatically get set if the BAR
> > + * can only be a 64-bit BAR, or if the requested size is larger than 2 GB.
> > + */
> > +int pci_epf_set_inbound_space(struct pci_epf *epf, size_t size,
> > +			      enum pci_barno bar,
> > +			      const struct pci_epc_features *epc_features,
> > +			      enum pci_epc_interface_type type,
> > +			      dma_addr_t inbound_addr)
> > +{
> > +	size_t aligned_size, align = epc_features->align;
> > +	struct pci_epf_bar *epf_bar;
> > +	struct pci_epc *epc;
> > +	dma_addr_t up;
> > +	int pos;
> > +
> > +	if (!size)
> > +		return -EINVAL;
> > +
> > +	up = inbound_addr + size - 1;
>
> s/up/limit?
>
> > +
> > +	/*
> > +	 *  Bits:		15 14 13 12 11 10 9 8 7 6 5 4 3 2 1 0
> > +	 *  Inbound_addr:	U  U  U  U  U  U  0 X X X X X X X X X
> > +	 *  Up:			U  U  U  U  U  U  1 X X X X X X X X X
> > +	 *
> > +	 *  U means address bits have not change in Range [Inbound_Addr, Up]
> > +	 *  X means bit 0 or 1.
> > +	 *
> > +	 *  Inbound_addr^Up  0  0  0  0  0  0  1 X X X X X X X X X
> > +	 *  Find first bit 1 pos from MSB, 2 ^ pos windows will cover
> > +	 *  [Inbound_Addr, Up] range.
> > +	 */
> > +	for (pos = 8 * sizeof(dma_addr_t) - 1; pos > 0; pos--)
> > +		if ((up ^ inbound_addr) & BIT_ULL(pos))
> > +			break;
> > +
> > +	if (pos == 8 * sizeof(dma_addr_t) - 1)
> > +		return -EINVAL;
> > +
> > +	size = pci_epf_get_bar_required_size(epf, BIT_ULL(pos + 1), bar,
> > +					     epc_features, type);
> > +
> > +	if (size == 0)
> > +		return -EINVAL;
> > +
> > +	/*
> > +	 * Allocate enough memory to accommodate the iATU alignment
>
> s/iATU/EPC
>
> > +	 * requirement.  In most cases, this will be the same as .size but
> > +	 * it might be different if, for example, the fixed size of a BAR
> > +	 * is smaller than align.
> > +	 */
> > +	aligned_size = align ? ALIGN(size, align) : size;
>
> This should be handled inside the above API.
>
> - Mani
>
> --
> மணிவண்ணன் சதாசிவம்