Introduce pci_epf_get_bar_required_size() to retrieve the required BAR
size and memory size. Prepare for adding support to set an MMIO address to
a specific BAR.
Use two variables 'aligned_bar_size' and 'aligned_mem_size' to avoid
confuse.
No functional changes.
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
change in v3
- change return value to int.
- use two pointers return bar size aligned and memory start address aligned
- update comments about why need memory align size. Actually iATU require
start address match aligned requirement. Since kernel return align to
size's address.
- use two varible aligned_bar_size and aligned_mem_size to avoid confuse
use 'size'.
change in v2
- new patch
---
drivers/pci/endpoint/pci-epf-core.c | 84 +++++++++++++++++++++++--------------
1 file changed, 53 insertions(+), 31 deletions(-)
diff --git a/drivers/pci/endpoint/pci-epf-core.c b/drivers/pci/endpoint/pci-epf-core.c
index d54e18872aefc07c655c94c104a347328ff7a432..2cd0257831f9885a4381c087ed8f3326f5960966 100644
--- a/drivers/pci/endpoint/pci-epf-core.c
+++ b/drivers/pci/endpoint/pci-epf-core.c
@@ -208,6 +208,49 @@ void pci_epf_remove_vepf(struct pci_epf *epf_pf, struct pci_epf *epf_vf)
}
EXPORT_SYMBOL_GPL(pci_epf_remove_vepf);
+static int
+pci_epf_get_bar_required_size(struct pci_epf *epf, size_t size,
+ size_t *aligned_bar_size,
+ size_t *aligned_mem_size,
+ enum pci_barno bar,
+ const struct pci_epc_features *epc_features,
+ enum pci_epc_interface_type type)
+{
+ u64 bar_fixed_size = epc_features->bar[bar].fixed_size;
+ size_t align = epc_features->align;
+
+ if (size < 128)
+ size = 128;
+
+ /* According to PCIe base spec, min size for a resizable BAR is 1 MB. */
+ if (epc_features->bar[bar].type == BAR_RESIZABLE && size < SZ_1M)
+ size = SZ_1M;
+
+ if (epc_features->bar[bar].type == BAR_FIXED && bar_fixed_size) {
+ if (size > bar_fixed_size) {
+ dev_err(&epf->dev,
+ "requested BAR size is larger than fixed size\n");
+ return -ENOMEM;
+ }
+ size = bar_fixed_size;
+ } else {
+ /* BAR size must be power of two */
+ size = roundup_pow_of_two(size);
+ }
+
+ *aligned_bar_size = size;
+
+ /*
+ * The EPC's BAR start address must meet alignment requirements. In most
+ * cases, the alignment will match the BAR size. However, differences
+ * can occur—for example, when the fixed BAR size (e.g., 128 bytes) is
+ * smaller than the required alignment (e.g., 4 KB).
+ */
+ *aligned_mem_size = align ? ALIGN(size, align) : size;
+
+ return 0;
+}
+
/**
* pci_epf_free_space() - free the allocated PCI EPF register space
* @epf: the EPF device from whom to free the memory
@@ -264,40 +307,17 @@ 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)
{
- u64 bar_fixed_size = epc_features->bar[bar].fixed_size;
- size_t aligned_size, align = epc_features->align;
+ size_t aligned_mem_size, aligned_bar_size;
struct pci_epf_bar *epf_bar;
dma_addr_t phys_addr;
struct pci_epc *epc;
struct device *dev;
void *space;
- if (size < 128)
- size = 128;
-
- /* According to PCIe base spec, min size for a resizable BAR is 1 MB. */
- if (epc_features->bar[bar].type == BAR_RESIZABLE && size < SZ_1M)
- size = SZ_1M;
-
- if (epc_features->bar[bar].type == BAR_FIXED && bar_fixed_size) {
- if (size > bar_fixed_size) {
- dev_err(&epf->dev,
- "requested BAR size is larger than fixed size\n");
- return NULL;
- }
- size = bar_fixed_size;
- } else {
- /* BAR size must be power of two */
- size = roundup_pow_of_two(size);
- }
-
- /*
- * Allocate enough memory to accommodate the iATU alignment
- * 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;
+ if (pci_epf_get_bar_required_size(epf, size, &aligned_bar_size,
+ &aligned_mem_size, bar,
+ epc_features, type))
+ return NULL;
if (type == PRIMARY_INTERFACE) {
epc = epf->epc;
@@ -308,7 +328,9 @@ void *pci_epf_alloc_space(struct pci_epf *epf, size_t size, enum pci_barno bar,
}
dev = epc->dev.parent;
- space = dma_alloc_coherent(dev, aligned_size, &phys_addr, GFP_KERNEL);
+
+ space = dma_alloc_coherent(dev, aligned_mem_size,
+ &phys_addr, GFP_KERNEL);
if (!space) {
dev_err(dev, "failed to allocate mem space\n");
return NULL;
@@ -316,8 +338,8 @@ void *pci_epf_alloc_space(struct pci_epf *epf, size_t size, enum pci_barno bar,
epf_bar[bar].phys_addr = phys_addr;
epf_bar[bar].addr = space;
- epf_bar[bar].size = size;
- epf_bar[bar].aligned_size = aligned_size;
+ epf_bar[bar].size = aligned_bar_size;
+ epf_bar[bar].aligned_size = aligned_mem_size;
epf_bar[bar].barno = bar;
if (upper_32_bits(size) || epc_features->bar[bar].only_64bit)
epf_bar[bar].flags |= PCI_BASE_ADDRESS_MEM_TYPE_64;
--
2.34.1
On Thu, Sep 25, 2025 at 01:01:47PM -0400, Frank Li wrote: > Introduce pci_epf_get_bar_required_size() to retrieve the required BAR > size and memory size. Prepare for adding support to set an MMIO address to > a specific BAR. > > Use two variables 'aligned_bar_size' and 'aligned_mem_size' to avoid > confuse. s/confuse/confusion/ > > No functional changes. > > Signed-off-by: Frank Li <Frank.Li@nxp.com> > --- > change in v3 > - change return value to int. > - use two pointers return bar size aligned and memory start address aligned > - update comments about why need memory align size. Actually iATU require > start address match aligned requirement. Since kernel return align to > size's address. > - use two varible aligned_bar_size and aligned_mem_size to avoid confuse > use 'size'. > > change in v2 > - new patch > --- > drivers/pci/endpoint/pci-epf-core.c | 84 +++++++++++++++++++++++-------------- > 1 file changed, 53 insertions(+), 31 deletions(-) > > diff --git a/drivers/pci/endpoint/pci-epf-core.c b/drivers/pci/endpoint/pci-epf-core.c > index d54e18872aefc07c655c94c104a347328ff7a432..2cd0257831f9885a4381c087ed8f3326f5960966 100644 > --- a/drivers/pci/endpoint/pci-epf-core.c > +++ b/drivers/pci/endpoint/pci-epf-core.c > @@ -208,6 +208,49 @@ void pci_epf_remove_vepf(struct pci_epf *epf_pf, struct pci_epf *epf_vf) > } > EXPORT_SYMBOL_GPL(pci_epf_remove_vepf); > > +static int > +pci_epf_get_bar_required_size(struct pci_epf *epf, size_t size, > + size_t *aligned_bar_size, > + size_t *aligned_mem_size, > + enum pci_barno bar, > + const struct pci_epc_features *epc_features, > + enum pci_epc_interface_type type) > +{ > + u64 bar_fixed_size = epc_features->bar[bar].fixed_size; > + size_t align = epc_features->align; > + > + if (size < 128) > + size = 128; > + > + /* According to PCIe base spec, min size for a resizable BAR is 1 MB. */ > + if (epc_features->bar[bar].type == BAR_RESIZABLE && size < SZ_1M) > + size = SZ_1M; > + > + if (epc_features->bar[bar].type == BAR_FIXED && bar_fixed_size) { > + if (size > bar_fixed_size) { > + dev_err(&epf->dev, > + "requested BAR size is larger than fixed size\n"); > + return -ENOMEM; > + } > + size = bar_fixed_size; > + } else { > + /* BAR size must be power of two */ > + size = roundup_pow_of_two(size); > + } > + > + *aligned_bar_size = size; I think this name is wrong. The BAR size has not been aligned to anything. The BAR size has to be a power of two, but that is a requirement of the PCI specification, so that in an inherent property of a BAR. Perhaps just name it size or bar_size? Kind regards, Niklas
On Fri, Sep 26, 2025 at 02:31:42PM +0200, Niklas Cassel wrote: > On Thu, Sep 25, 2025 at 01:01:47PM -0400, Frank Li wrote: > > Introduce pci_epf_get_bar_required_size() to retrieve the required BAR > > size and memory size. Prepare for adding support to set an MMIO address to > > a specific BAR. > > > > Use two variables 'aligned_bar_size' and 'aligned_mem_size' to avoid > > confuse. > > s/confuse/confusion/ > > > > > > No functional changes. > > > > Signed-off-by: Frank Li <Frank.Li@nxp.com> > > --- > > change in v3 > > - change return value to int. > > - use two pointers return bar size aligned and memory start address aligned > > - update comments about why need memory align size. Actually iATU require > > start address match aligned requirement. Since kernel return align to > > size's address. > > - use two varible aligned_bar_size and aligned_mem_size to avoid confuse > > use 'size'. > > > > change in v2 > > - new patch > > --- > > drivers/pci/endpoint/pci-epf-core.c | 84 +++++++++++++++++++++++-------------- > > 1 file changed, 53 insertions(+), 31 deletions(-) > > > > diff --git a/drivers/pci/endpoint/pci-epf-core.c b/drivers/pci/endpoint/pci-epf-core.c > > index d54e18872aefc07c655c94c104a347328ff7a432..2cd0257831f9885a4381c087ed8f3326f5960966 100644 > > --- a/drivers/pci/endpoint/pci-epf-core.c > > +++ b/drivers/pci/endpoint/pci-epf-core.c > > @@ -208,6 +208,49 @@ void pci_epf_remove_vepf(struct pci_epf *epf_pf, struct pci_epf *epf_vf) > > } > > EXPORT_SYMBOL_GPL(pci_epf_remove_vepf); > > > > +static int > > +pci_epf_get_bar_required_size(struct pci_epf *epf, size_t size, > > + size_t *aligned_bar_size, > > + size_t *aligned_mem_size, > > + enum pci_barno bar, > > + const struct pci_epc_features *epc_features, > > + enum pci_epc_interface_type type) > > +{ > > + u64 bar_fixed_size = epc_features->bar[bar].fixed_size; > > + size_t align = epc_features->align; > > + > > + if (size < 128) > > + size = 128; > > + > > + /* According to PCIe base spec, min size for a resizable BAR is 1 MB. */ > > + if (epc_features->bar[bar].type == BAR_RESIZABLE && size < SZ_1M) > > + size = SZ_1M; > > + > > + if (epc_features->bar[bar].type == BAR_FIXED && bar_fixed_size) { > > + if (size > bar_fixed_size) { > > + dev_err(&epf->dev, > > + "requested BAR size is larger than fixed size\n"); > > + return -ENOMEM; > > + } > > + size = bar_fixed_size; > > + } else { > > + /* BAR size must be power of two */ > > + size = roundup_pow_of_two(size); > > + } > > + > > + *aligned_bar_size = size; > > I think this name is wrong. > The BAR size has not been aligned to anything. > The BAR size has to be a power of two, but that is a requirement of the PCI > specification, so that in an inherent property of a BAR. > > Perhaps just name it size or bar_size? there already have 'size' for input. It should match epc required's size. how about 'epc_bar_size'? Frank > > > Kind regards, > Niklas
On Fri, Sep 26, 2025 at 10:56:46AM -0400, Frank Li wrote: > On Fri, Sep 26, 2025 at 02:31:42PM +0200, Niklas Cassel wrote: > > On Thu, Sep 25, 2025 at 01:01:47PM -0400, Frank Li wrote: > > > Introduce pci_epf_get_bar_required_size() to retrieve the required BAR > > > size and memory size. Prepare for adding support to set an MMIO address to > > > a specific BAR. > > > > > > Use two variables 'aligned_bar_size' and 'aligned_mem_size' to avoid > > > confuse. > > > > s/confuse/confusion/ > > > > > > > > > > No functional changes. > > > > > > Signed-off-by: Frank Li <Frank.Li@nxp.com> > > > --- > > > change in v3 > > > - change return value to int. > > > - use two pointers return bar size aligned and memory start address aligned > > > - update comments about why need memory align size. Actually iATU require > > > start address match aligned requirement. Since kernel return align to > > > size's address. > > > - use two varible aligned_bar_size and aligned_mem_size to avoid confuse > > > use 'size'. > > > > > > change in v2 > > > - new patch > > > --- > > > drivers/pci/endpoint/pci-epf-core.c | 84 +++++++++++++++++++++++-------------- > > > 1 file changed, 53 insertions(+), 31 deletions(-) > > > > > > diff --git a/drivers/pci/endpoint/pci-epf-core.c b/drivers/pci/endpoint/pci-epf-core.c > > > index d54e18872aefc07c655c94c104a347328ff7a432..2cd0257831f9885a4381c087ed8f3326f5960966 100644 > > > --- a/drivers/pci/endpoint/pci-epf-core.c > > > +++ b/drivers/pci/endpoint/pci-epf-core.c > > > @@ -208,6 +208,49 @@ void pci_epf_remove_vepf(struct pci_epf *epf_pf, struct pci_epf *epf_vf) > > > } > > > EXPORT_SYMBOL_GPL(pci_epf_remove_vepf); > > > > > > +static int > > > +pci_epf_get_bar_required_size(struct pci_epf *epf, size_t size, > > > + size_t *aligned_bar_size, > > > + size_t *aligned_mem_size, > > > + enum pci_barno bar, > > > + const struct pci_epc_features *epc_features, > > > + enum pci_epc_interface_type type) > > > +{ > > > + u64 bar_fixed_size = epc_features->bar[bar].fixed_size; > > > + size_t align = epc_features->align; > > > + > > > + if (size < 128) > > > + size = 128; > > > + > > > + /* According to PCIe base spec, min size for a resizable BAR is 1 MB. */ > > > + if (epc_features->bar[bar].type == BAR_RESIZABLE && size < SZ_1M) > > > + size = SZ_1M; > > > + > > > + if (epc_features->bar[bar].type == BAR_FIXED && bar_fixed_size) { > > > + if (size > bar_fixed_size) { > > > + dev_err(&epf->dev, > > > + "requested BAR size is larger than fixed size\n"); > > > + return -ENOMEM; > > > + } > > > + size = bar_fixed_size; > > > + } else { > > > + /* BAR size must be power of two */ > > > + size = roundup_pow_of_two(size); > > > + } > > > + > > > + *aligned_bar_size = size; > > > > I think this name is wrong. > > The BAR size has not been aligned to anything. > > The BAR size has to be a power of two, but that is a requirement of the PCI > > specification, so that in an inherent property of a BAR. > > > > Perhaps just name it size or bar_size? > > there already have 'size' for input. It should match epc required's size. Why do you need both "size_t size" and "size_t *bar_size"? Isn't it enough with "size_t *bar_size" ? The user can supply a value, and the function could update that value. Kind regards, Niklas
On Fri, Sep 26, 2025 at 05:16:57PM +0200, Niklas Cassel wrote: > On Fri, Sep 26, 2025 at 10:56:46AM -0400, Frank Li wrote: > > On Fri, Sep 26, 2025 at 02:31:42PM +0200, Niklas Cassel wrote: > > > On Thu, Sep 25, 2025 at 01:01:47PM -0400, Frank Li wrote: > > > > Introduce pci_epf_get_bar_required_size() to retrieve the required BAR > > > > size and memory size. Prepare for adding support to set an MMIO address to > > > > a specific BAR. > > > > > > > > Use two variables 'aligned_bar_size' and 'aligned_mem_size' to avoid > > > > confuse. > > > > > > s/confuse/confusion/ > > > > > > > > > > > > > > No functional changes. > > > > > > > > Signed-off-by: Frank Li <Frank.Li@nxp.com> > > > > --- > > > > change in v3 > > > > - change return value to int. > > > > - use two pointers return bar size aligned and memory start address aligned > > > > - update comments about why need memory align size. Actually iATU require > > > > start address match aligned requirement. Since kernel return align to > > > > size's address. > > > > - use two varible aligned_bar_size and aligned_mem_size to avoid confuse > > > > use 'size'. > > > > > > > > change in v2 > > > > - new patch > > > > --- > > > > drivers/pci/endpoint/pci-epf-core.c | 84 +++++++++++++++++++++++-------------- > > > > 1 file changed, 53 insertions(+), 31 deletions(-) > > > > > > > > diff --git a/drivers/pci/endpoint/pci-epf-core.c b/drivers/pci/endpoint/pci-epf-core.c > > > > index d54e18872aefc07c655c94c104a347328ff7a432..2cd0257831f9885a4381c087ed8f3326f5960966 100644 > > > > --- a/drivers/pci/endpoint/pci-epf-core.c > > > > +++ b/drivers/pci/endpoint/pci-epf-core.c > > > > @@ -208,6 +208,49 @@ void pci_epf_remove_vepf(struct pci_epf *epf_pf, struct pci_epf *epf_vf) > > > > } > > > > EXPORT_SYMBOL_GPL(pci_epf_remove_vepf); > > > > > > > > +static int > > > > +pci_epf_get_bar_required_size(struct pci_epf *epf, size_t size, > > > > + size_t *aligned_bar_size, > > > > + size_t *aligned_mem_size, > > > > + enum pci_barno bar, > > > > + const struct pci_epc_features *epc_features, > > > > + enum pci_epc_interface_type type) > > > > +{ > > > > + u64 bar_fixed_size = epc_features->bar[bar].fixed_size; > > > > + size_t align = epc_features->align; > > > > + > > > > + if (size < 128) > > > > + size = 128; > > > > + > > > > + /* According to PCIe base spec, min size for a resizable BAR is 1 MB. */ > > > > + if (epc_features->bar[bar].type == BAR_RESIZABLE && size < SZ_1M) > > > > + size = SZ_1M; > > > > + > > > > + if (epc_features->bar[bar].type == BAR_FIXED && bar_fixed_size) { > > > > + if (size > bar_fixed_size) { > > > > + dev_err(&epf->dev, > > > > + "requested BAR size is larger than fixed size\n"); > > > > + return -ENOMEM; > > > > + } > > > > + size = bar_fixed_size; > > > > + } else { > > > > + /* BAR size must be power of two */ > > > > + size = roundup_pow_of_two(size); > > > > + } > > > > + > > > > + *aligned_bar_size = size; > > > > > > I think this name is wrong. > > > The BAR size has not been aligned to anything. > > > The BAR size has to be a power of two, but that is a requirement of the PCI > > > specification, so that in an inherent property of a BAR. > > > > > > Perhaps just name it size or bar_size? > > > > there already have 'size' for input. It should match epc required's size. > > Why do you need both "size_t size" and "size_t *bar_size"? > > Isn't it enough with "size_t *bar_size" ? > > The user can supply a value, and the function could update that value. If not 'aligned_mem_size' in list, it looks fine. But after add 'aligned_mem_size', I think use difference varible for two outputs will be clear and consistent and easy to understand. Frank > > > Kind regards, > Niklas
On 26 September 2025 19:10:16 CEST, Frank Li <Frank.li@nxp.com> wrote: >On Fri, Sep 26, 2025 at 05:16:57PM +0200, Niklas Cassel wrote: >> On Fri, Sep 26, 2025 at 10:56:46AM -0400, Frank Li wrote: >> > On Fri, Sep 26, 2025 at 02:31:42PM +0200, Niklas Cassel wrote: >> > > On Thu, Sep 25, 2025 at 01:01:47PM -0400, Frank Li wrote: >> > > > Introduce pci_epf_get_bar_required_size() to retrieve the required BAR >> > > > size and memory size. Prepare for adding support to set an MMIO address to >> > > > a specific BAR. >> > > > >> > > > Use two variables 'aligned_bar_size' and 'aligned_mem_size' to avoid >> > > > confuse. >> > > >> > > s/confuse/confusion/ >> > > >> > > >> > > > >> > > > No functional changes. >> > > > >> > > > Signed-off-by: Frank Li <Frank.Li@nxp.com> >> > > > --- >> > > > change in v3 >> > > > - change return value to int. >> > > > - use two pointers return bar size aligned and memory start address aligned >> > > > - update comments about why need memory align size. Actually iATU require >> > > > start address match aligned requirement. Since kernel return align to >> > > > size's address. >> > > > - use two varible aligned_bar_size and aligned_mem_size to avoid confuse >> > > > use 'size'. >> > > > >> > > > change in v2 >> > > > - new patch >> > > > --- >> > > > drivers/pci/endpoint/pci-epf-core.c | 84 +++++++++++++++++++++++-------------- >> > > > 1 file changed, 53 insertions(+), 31 deletions(-) >> > > > >> > > > diff --git a/drivers/pci/endpoint/pci-epf-core.c b/drivers/pci/endpoint/pci-epf-core.c >> > > > index d54e18872aefc07c655c94c104a347328ff7a432..2cd0257831f9885a4381c087ed8f3326f5960966 100644 >> > > > --- a/drivers/pci/endpoint/pci-epf-core.c >> > > > +++ b/drivers/pci/endpoint/pci-epf-core.c >> > > > @@ -208,6 +208,49 @@ void pci_epf_remove_vepf(struct pci_epf *epf_pf, struct pci_epf *epf_vf) >> > > > } >> > > > EXPORT_SYMBOL_GPL(pci_epf_remove_vepf); >> > > > >> > > > +static int >> > > > +pci_epf_get_bar_required_size(struct pci_epf *epf, size_t size, >> > > > + size_t *aligned_bar_size, >> > > > + size_t *aligned_mem_size, >> > > > + enum pci_barno bar, >> > > > + const struct pci_epc_features *epc_features, >> > > > + enum pci_epc_interface_type type) >> > > > +{ >> > > > + u64 bar_fixed_size = epc_features->bar[bar].fixed_size; >> > > > + size_t align = epc_features->align; >> > > > + >> > > > + if (size < 128) >> > > > + size = 128; >> > > > + >> > > > + /* According to PCIe base spec, min size for a resizable BAR is 1 MB. */ >> > > > + if (epc_features->bar[bar].type == BAR_RESIZABLE && size < SZ_1M) >> > > > + size = SZ_1M; >> > > > + >> > > > + if (epc_features->bar[bar].type == BAR_FIXED && bar_fixed_size) { >> > > > + if (size > bar_fixed_size) { >> > > > + dev_err(&epf->dev, >> > > > + "requested BAR size is larger than fixed size\n"); >> > > > + return -ENOMEM; >> > > > + } >> > > > + size = bar_fixed_size; >> > > > + } else { >> > > > + /* BAR size must be power of two */ >> > > > + size = roundup_pow_of_two(size); >> > > > + } >> > > > + >> > > > + *aligned_bar_size = size; >> > > >> > > I think this name is wrong. >> > > The BAR size has not been aligned to anything. >> > > The BAR size has to be a power of two, but that is a requirement of the PCI >> > > specification, so that in an inherent property of a BAR. >> > > >> > > Perhaps just name it size or bar_size? >> > >> > there already have 'size' for input. It should match epc required's size. >> >> Why do you need both "size_t size" and "size_t *bar_size"? >> >> Isn't it enough with "size_t *bar_size" ? >> >> The user can supply a value, and the function could update that value. > >If not 'aligned_mem_size' in list, it looks fine. But after add >'aligned_mem_size', I think use difference varible for two outputs will be >clear and consistent and easy to understand. What am trying to say is: Why not make "size_t *bar_size" both an input and an output? Kind regards, Niklas
© 2016 - 2025 Red Hat, Inc.