[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <udt2t2c43nw5de5q5vu2etknbnhrr3m5mqjk72nuyiurk46kbp@e47e5kvk2m22>
Date: Sat, 30 Aug 2025 20:16:33 +0530
From: Manivannan Sadhasivam <mani@...nel.org>
To: Frank Li <Frank.Li@....com>
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
Subject: Re: [PATCH 1/2] PCI: endpoint: Enhance pci_epf_alloc_space() and
rename to pci_epf_set_inbound_space()
On Fri, Aug 15, 2025 at 06:20:53PM GMT, Frank Li wrote:
> Enhance pci_epf_alloc_space() to handle setting any physical address as
> inbound memory space, such as an MSI message base address. The function
> already accounts for different alignment requirements for different BARs,
> so reuse this logic and rename the function to pci_epf_set_inbound_space().
>
I don't think combining both APIs is a good idea. One allocates space for
inbound memory/populates epf_bar and another reuses existing memory/populates
epf_bar. Combining both, logically makes little sense and another makes the code
messy.
If you want to reuse the alignment checks and epf_bar setting from
pci_epf_alloc_space(), then create a separate helper(s) out of it and call from
both APIs.
> Make pci_epf_alloc_space() inline and call pci_epf_set_inbound_space() with
> from_space set to true to maintain compatibility.
>
> Signed-off-by: Frank Li <Frank.Li@....com>
> ---
> ---
> drivers/pci/endpoint/pci-epf-core.c | 69 ++++++++++++++++++++++++++++++-------
> include/linux/pci-epc.h | 5 ---
> include/linux/pci-epf.h | 35 ++++++++++++++++---
> 3 files changed, 87 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/pci/endpoint/pci-epf-core.c b/drivers/pci/endpoint/pci-epf-core.c
> index d54e18872aefc07c655c94c104a347328ff7a432..5b802b1ea3e28a32e38f4ab6a649cb97a2f29b95 100644
> --- a/drivers/pci/endpoint/pci-epf-core.c
> +++ b/drivers/pci/endpoint/pci-epf-core.c
> @@ -249,20 +249,26 @@ void pci_epf_free_space(struct pci_epf *epf, void *addr, enum pci_barno bar,
> EXPORT_SYMBOL_GPL(pci_epf_free_space);
>
> /**
> - * pci_epf_alloc_space() - allocate memory for the PCI EPF register space
> + * pci_epf_set_inbound_space() - set memory for the PCI EPF inbound address space
> * @epf: the EPF device to whom allocate the memory
> * @size: the size of the memory that has to be allocated
> * @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
> + * @from_memory: allocate from system memory
> + * @inbound_addr: Any physical address space such as MSI message address that
> + * work as inbound address space. from_memory need be false.
> *
> * Invoke to allocate memory for the PCI EPF register space.
> * 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.
> */
> -void *pci_epf_alloc_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)
> +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,
> + bool from_memory,
> + dma_addr_t inbound_addr)
> {
> u64 bar_fixed_size = epc_features->bar[bar].fixed_size;
> size_t aligned_size, align = epc_features->align;
> @@ -270,7 +276,32 @@ void *pci_epf_alloc_space(struct pci_epf *epf, size_t size, enum pci_barno bar,
> dma_addr_t phys_addr;
> struct pci_epc *epc;
> struct device *dev;
> - void *space;
> + void *space = NULL;
> + dma_addr_t up;
> +
> + up = inbound_addr + size - 1;
> +
> + /*
> + * 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.
On what basis this size calculation is used?
- Mani
--
மணிவண்ணன் சதாசிவம்
Powered by blists - more mailing lists