* [PATCH v2 1/3] PCI: endpoint: Add helper function pci_epf_get_bar_required_size()
2025-09-15 22:22 [PATCH v2 0/3] pci: endpoint: vntb: add MSI doorbell support Frank Li
@ 2025-09-15 22:22 ` Frank Li
2025-09-24 17:17 ` Manivannan Sadhasivam
2025-09-15 22:22 ` [PATCH v2 2/3] PCI: endpoint: Add API pci_epf_set_inbound_space() Frank Li
` (2 subsequent siblings)
3 siblings, 1 reply; 13+ messages in thread
From: Frank Li @ 2025-09-15 22:22 UTC (permalink / raw)
To: Manivannan Sadhasivam, Krzysztof Wilczyński,
Kishon Vijay Abraham I, Bjorn Helgaas, Jon Mason, Dave Jiang,
Allen Hubbe
Cc: linux-pci, linux-kernel, ntb, imx, Frank Li
Introduce pci_epf_get_bar_required_size() to retrieve the required BAR
size. Prepare for adding support to set an MMIO address to a specific BAR.
No functional changes.
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
change in v2
- new patch
---
drivers/pci/endpoint/pci-epf-core.c | 52 ++++++++++++++++++++++++-------------
1 file changed, 34 insertions(+), 18 deletions(-)
diff --git a/drivers/pci/endpoint/pci-epf-core.c b/drivers/pci/endpoint/pci-epf-core.c
index d54e18872aefc07c655c94c104a347328ff7a432..4281067d4a62da6fbf6c2fb807b0f1b5afd1f45b 100644
--- a/drivers/pci/endpoint/pci-epf-core.c
+++ b/drivers/pci/endpoint/pci-epf-core.c
@@ -248,6 +248,36 @@ void pci_epf_free_space(struct pci_epf *epf, void *addr, enum pci_barno bar,
}
EXPORT_SYMBOL_GPL(pci_epf_free_space);
+static size_t
+pci_epf_get_bar_required_size(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;
+
+ 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 0;
+ }
+ size = bar_fixed_size;
+ } else {
+ /* BAR size must be power of two */
+ size = roundup_pow_of_two(size);
+ }
+
+ return size;
+}
+
/**
* pci_epf_alloc_space() - allocate memory for the PCI EPF register space
* @epf: the EPF device to whom allocate the memory
@@ -264,7 +294,6 @@ 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;
struct pci_epf_bar *epf_bar;
dma_addr_t phys_addr;
@@ -272,24 +301,11 @@ void *pci_epf_alloc_space(struct pci_epf *epf, size_t size, enum pci_barno bar,
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;
+ size = pci_epf_get_bar_required_size(epf, size, bar,
+ epc_features, type);
- 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);
- }
+ if (size == 0)
+ return NULL;
/*
* Allocate enough memory to accommodate the iATU alignment
--
2.34.1
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v2 1/3] PCI: endpoint: Add helper function pci_epf_get_bar_required_size()
2025-09-15 22:22 ` [PATCH v2 1/3] PCI: endpoint: Add helper function pci_epf_get_bar_required_size() Frank Li
@ 2025-09-24 17:17 ` Manivannan Sadhasivam
0 siblings, 0 replies; 13+ messages in thread
From: Manivannan Sadhasivam @ 2025-09-24 17:17 UTC (permalink / raw)
To: Frank Li
Cc: Krzysztof Wilczyński, Kishon Vijay Abraham I, Bjorn Helgaas,
Jon Mason, Dave Jiang, Allen Hubbe, linux-pci, linux-kernel, ntb,
imx
On Mon, Sep 15, 2025 at 06:22:44PM -0400, Frank Li wrote:
> Introduce pci_epf_get_bar_required_size() to retrieve the required BAR
> size. Prepare for adding support to set an MMIO address to a specific BAR.
>
> No functional changes.
>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
> change in v2
> - new patch
> ---
> drivers/pci/endpoint/pci-epf-core.c | 52 ++++++++++++++++++++++++-------------
> 1 file changed, 34 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/pci/endpoint/pci-epf-core.c b/drivers/pci/endpoint/pci-epf-core.c
> index d54e18872aefc07c655c94c104a347328ff7a432..4281067d4a62da6fbf6c2fb807b0f1b5afd1f45b 100644
> --- a/drivers/pci/endpoint/pci-epf-core.c
> +++ b/drivers/pci/endpoint/pci-epf-core.c
> @@ -248,6 +248,36 @@ void pci_epf_free_space(struct pci_epf *epf, void *addr, enum pci_barno bar,
> }
> EXPORT_SYMBOL_GPL(pci_epf_free_space);
>
> +static size_t
s/size_t/int to return errno.
> +pci_epf_get_bar_required_size(struct pci_epf *epf, size_t size,
Rename this helper to pci_epf_align_bar_size() and pass 'size' pointer.
> + 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;
> +
> + 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 0;
Return value of 0 should not be considered as an error. Return the errno here,
which is -ENOMEM.
> + }
> + size = bar_fixed_size;
> + } else {
> + /* BAR size must be power of two */
> + size = roundup_pow_of_two(size);
> + }
> +
> + return size;
return 0;
> +}
> +
> /**
> * pci_epf_alloc_space() - allocate memory for the PCI EPF register space
> * @epf: the EPF device to whom allocate the memory
> @@ -264,7 +294,6 @@ 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;
> struct pci_epf_bar *epf_bar;
> dma_addr_t phys_addr;
> @@ -272,24 +301,11 @@ void *pci_epf_alloc_space(struct pci_epf *epf, size_t size, enum pci_barno bar,
> 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;
> + size = pci_epf_get_bar_required_size(epf, size, bar,
> + epc_features, type);
>
> - 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);
> - }
> + if (size == 0)
> + return NULL;
int ret;
ret = pci_epf_align_bar_size(..., &size, ...);
if (ret < 0)
return NULL;
- Mani
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 2/3] PCI: endpoint: Add API pci_epf_set_inbound_space()
2025-09-15 22:22 [PATCH v2 0/3] pci: endpoint: vntb: add MSI doorbell support Frank Li
2025-09-15 22:22 ` [PATCH v2 1/3] PCI: endpoint: Add helper function pci_epf_get_bar_required_size() Frank Li
@ 2025-09-15 22:22 ` Frank Li
2025-09-24 18:09 ` Manivannan Sadhasivam
2025-09-15 22:22 ` [PATCH v2 3/3] PCI: endpoint: pci-epf-vntb: Add MSI doorbell support Frank Li
2025-09-24 16:23 ` [PATCH v2 0/3] pci: endpoint: vntb: add " Frank Li
3 siblings, 1 reply; 13+ messages in thread
From: Frank Li @ 2025-09-15 22:22 UTC (permalink / raw)
To: Manivannan Sadhasivam, Krzysztof Wilczyński,
Kishon Vijay Abraham I, Bjorn Helgaas, Jon Mason, Dave Jiang,
Allen Hubbe
Cc: linux-pci, linux-kernel, ntb, imx, Frank Li
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.
Signed-off-by: Frank Li <Frank.Li@nxp.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
+ * @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
+ * @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.
+ */
+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;
+
+ /*
+ * 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
+ * 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 (type == PRIMARY_INTERFACE) {
+ epc = epf->epc;
+ epf_bar = epf->bar;
+ } else {
+ epc = epf->sec_epc;
+ epf_bar = epf->sec_epc_bar;
+ }
+
+ epf_bar[bar].phys_addr = ALIGN_DOWN(inbound_addr, aligned_size);
+ epf_bar[bar].addr = NULL;
+ epf_bar[bar].size = size;
+ epf_bar[bar].aligned_size = aligned_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;
+ else
+ epf_bar[bar].flags |= PCI_BASE_ADDRESS_MEM_TYPE_32;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(pci_epf_set_inbound_space);
+
static void pci_epf_remove_cfs(struct pci_epf_driver *driver)
{
struct config_group *group, *tmp;
diff --git a/include/linux/pci-epf.h b/include/linux/pci-epf.h
index 2e85504ba2baf93827224884ca19ae2bd0e6906b..11c08209c27c3f523cca18a1ae8f1352b8b0c24a 100644
--- a/include/linux/pci-epf.h
+++ b/include/linux/pci-epf.h
@@ -242,6 +242,12 @@ void *pci_epf_alloc_space(struct pci_epf *epf, size_t size, enum pci_barno bar,
void pci_epf_free_space(struct pci_epf *epf, void *addr, enum pci_barno bar,
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,
+ dma_addr_t inbound_addr);
+
int pci_epf_align_inbound_addr(struct pci_epf *epf, enum pci_barno bar,
u64 addr, dma_addr_t *base, size_t *off);
int pci_epf_bind(struct pci_epf *epf);
--
2.34.1
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v2 2/3] PCI: endpoint: Add API pci_epf_set_inbound_space()
2025-09-15 22:22 ` [PATCH v2 2/3] PCI: endpoint: Add API pci_epf_set_inbound_space() Frank Li
@ 2025-09-24 18:09 ` Manivannan Sadhasivam
2025-09-24 18:35 ` Frank Li
0 siblings, 1 reply; 13+ messages in thread
From: Manivannan Sadhasivam @ 2025-09-24 18:09 UTC (permalink / raw)
To: Frank Li
Cc: Krzysztof Wilczyński, Kishon Vijay Abraham I, Bjorn Helgaas,
Jon Mason, Dave Jiang, Allen Hubbe, linux-pci, linux-kernel, ntb,
imx
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?
> Signed-off-by: Frank Li <Frank.Li@nxp.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
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v2 2/3] PCI: endpoint: Add API pci_epf_set_inbound_space()
2025-09-24 18:09 ` Manivannan Sadhasivam
@ 2025-09-24 18:35 ` Frank Li
0 siblings, 0 replies; 13+ messages in thread
From: Frank Li @ 2025-09-24 18:35 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: Krzysztof Wilczyński, Kishon Vijay Abraham I, Bjorn Helgaas,
Jon Mason, Dave Jiang, Allen Hubbe, linux-pci, linux-kernel, ntb,
imx
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@nxp.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
>
> --
> மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 3/3] PCI: endpoint: pci-epf-vntb: Add MSI doorbell support
2025-09-15 22:22 [PATCH v2 0/3] pci: endpoint: vntb: add MSI doorbell support Frank Li
2025-09-15 22:22 ` [PATCH v2 1/3] PCI: endpoint: Add helper function pci_epf_get_bar_required_size() Frank Li
2025-09-15 22:22 ` [PATCH v2 2/3] PCI: endpoint: Add API pci_epf_set_inbound_space() Frank Li
@ 2025-09-15 22:22 ` Frank Li
2025-09-24 18:22 ` Manivannan Sadhasivam
2025-09-24 16:23 ` [PATCH v2 0/3] pci: endpoint: vntb: add " Frank Li
3 siblings, 1 reply; 13+ messages in thread
From: Frank Li @ 2025-09-15 22:22 UTC (permalink / raw)
To: Manivannan Sadhasivam, Krzysztof Wilczyński,
Kishon Vijay Abraham I, Bjorn Helgaas, Jon Mason, Dave Jiang,
Allen Hubbe
Cc: linux-pci, linux-kernel, ntb, imx, Frank Li
Add MSI doorbell support to reduce latency between PCI host and EP.
Before this change:
ping 169.254.172.137
64 bytes from 169.254.172.137: icmp_seq=1 ttl=64 time=0.575 ms
64 bytes from 169.254.172.137: icmp_seq=2 ttl=64 time=1.80 ms
64 bytes from 169.254.172.137: icmp_seq=3 ttl=64 time=8.19 ms
64 bytes from 169.254.172.137: icmp_seq=4 ttl=64 time=2.00 ms
After this change:
ping 169.254.144.71
64 bytes from 169.254.144.71: icmp_seq=1 ttl=64 time=0.215 ms
64 bytes from 169.254.144.71: icmp_seq=2 ttl=64 time=0.456 ms
64 bytes from 169.254.144.71: icmp_seq=3 ttl=64 time=0.448 ms
Change u64 db to atomic_64 because difference doorbell may happen at the
same time.
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
change in v2
- update api pci_epf_set_inbound_space
- atomic_64 should be enough, which just record doorbell events, which is
similar with W1C irq status register.
---
drivers/pci/endpoint/functions/pci-epf-vntb.c | 153 +++++++++++++++++++++++---
1 file changed, 136 insertions(+), 17 deletions(-)
diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c b/drivers/pci/endpoint/functions/pci-epf-vntb.c
index 83e9ab10f9c4fc2b485d5463faa2172500f12999..06c13f26db1c6e55d23769e98e2cfd80da693a20 100644
--- a/drivers/pci/endpoint/functions/pci-epf-vntb.c
+++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c
@@ -36,11 +36,13 @@
* PCIe Root Port PCI EP
*/
+#include <linux/atomic.h>
#include <linux/delay.h>
#include <linux/io.h>
#include <linux/module.h>
#include <linux/slab.h>
+#include <linux/pci-ep-msi.h>
#include <linux/pci-epc.h>
#include <linux/pci-epf.h>
#include <linux/ntb.h>
@@ -126,12 +128,13 @@ struct epf_ntb {
u32 db_count;
u32 spad_count;
u64 mws_size[MAX_MW];
- u64 db;
+ atomic64_t db;
u32 vbus_number;
u16 vntb_pid;
u16 vntb_vid;
bool linkup;
+ bool msi_doorbell;
u32 spad_size;
enum pci_barno epf_ntb_bar[VNTB_BAR_NUM];
@@ -258,9 +261,9 @@ static void epf_ntb_cmd_handler(struct work_struct *work)
ntb = container_of(work, struct epf_ntb, cmd_handler.work);
- for (i = 1; i < ntb->db_count; i++) {
+ for (i = 1; i < ntb->db_count && !ntb->msi_doorbell; i++) {
if (ntb->epf_db[i]) {
- ntb->db |= 1 << (i - 1);
+ atomic64_or(1 << (i - 1), &ntb->db);
ntb_db_event(&ntb->ntb, i);
ntb->epf_db[i] = 0;
}
@@ -319,7 +322,24 @@ static void epf_ntb_cmd_handler(struct work_struct *work)
reset_handler:
queue_delayed_work(kpcintb_workqueue, &ntb->cmd_handler,
- msecs_to_jiffies(5));
+ ntb->msi_doorbell ? msecs_to_jiffies(500) : msecs_to_jiffies(5));
+}
+
+static irqreturn_t epf_ntb_doorbell_handler(int irq, void *data)
+{
+ struct epf_ntb *ntb = data;
+ int i = 0;
+
+ for (i = 1; i < ntb->db_count; i++)
+ if (irq == ntb->epf->db_msg[i].virq) {
+ atomic64_or(1 << (i - 1), &ntb->db);
+ ntb_db_event(&ntb->ntb, i);
+ }
+
+ if (irq == ntb->epf->db_msg[0].virq)
+ queue_delayed_work(kpcintb_workqueue, &ntb->cmd_handler, 0);
+
+ return IRQ_HANDLED;
}
/**
@@ -500,6 +520,90 @@ static int epf_ntb_configure_interrupt(struct epf_ntb *ntb)
return 0;
}
+static int epf_ntb_db_bar_init_msi_doorbell(struct epf_ntb *ntb,
+ struct pci_epf_bar *db_bar,
+ const struct pci_epc_features *epc_features,
+ enum pci_barno barno)
+{
+ struct pci_epf *epf = ntb->epf;
+ dma_addr_t low, high;
+ struct msi_msg *msg;
+ size_t sz;
+ int ret;
+ int i;
+
+ ret = pci_epf_alloc_doorbell(epf, ntb->db_count);
+ if (ret)
+ return ret;
+
+ for (i = 0; i < ntb->db_count; i++) {
+ ret = request_irq(epf->db_msg[i].virq, epf_ntb_doorbell_handler,
+ 0, "vntb_db", ntb);
+
+ if (ret) {
+ dev_err(&epf->dev,
+ "Failed to request doorbell IRQ: %d\n",
+ epf->db_msg[i].virq);
+ goto err_request_irq;
+ }
+ }
+
+ msg = &epf->db_msg[0].msg;
+
+ high = 0;
+ low = (u64)msg->address_hi << 32 | msg->address_lo;
+
+ for (i = 0; i < ntb->db_count; i++) {
+ struct msi_msg *msg = &epf->db_msg[i].msg;
+ dma_addr_t addr = (u64)msg->address_hi << 32 | msg->address_lo;
+
+ low = min(low, addr);
+ high = max(high, addr);
+ }
+
+ sz = high - low + sizeof(u32);
+
+ ret = pci_epf_set_inbound_space(epf, sz, barno,
+ epc_features, 0, low);
+
+ ret = pci_epc_set_bar(ntb->epf->epc, ntb->epf->func_no, ntb->epf->vfunc_no, db_bar);
+ if (ret) {
+ dev_err(&epf->dev, "Doorbell BAR set failed\n");
+ goto err_request_irq;
+ }
+
+ for (i = 0; i < ntb->db_count; i++) {
+ struct msi_msg *msg = &epf->db_msg[i].msg;
+ dma_addr_t addr;
+ size_t offset;
+
+ ret = pci_epf_align_inbound_addr(epf, db_bar->barno,
+ ((u64)msg->address_hi << 32) | msg->address_lo,
+ &addr, &offset);
+
+ if (ret) {
+ ntb->msi_doorbell = false;
+ goto err_request_irq;
+ }
+
+ ntb->reg->db_data[i] = msg->data;
+ ntb->reg->db_offset[i] = offset;
+ }
+
+ ntb->reg->db_entry_size = 0;
+
+ ntb->msi_doorbell = true;
+
+ return 0;
+
+err_request_irq:
+ for (i--; i >= 0; i--)
+ free_irq(epf->db_msg[i].virq, ntb);
+
+ pci_epf_free_doorbell(ntb->epf);
+ return ret;
+}
+
/**
* epf_ntb_db_bar_init() - Configure Doorbell window BARs
* @ntb: NTB device that facilitates communication between HOST and VHOST
@@ -520,22 +624,27 @@ static int epf_ntb_db_bar_init(struct epf_ntb *ntb)
ntb->epf->func_no,
ntb->epf->vfunc_no);
barno = ntb->epf_ntb_bar[BAR_DB];
-
- mw_addr = pci_epf_alloc_space(ntb->epf, size, barno, epc_features, 0);
- if (!mw_addr) {
- dev_err(dev, "Failed to allocate OB address\n");
- return -ENOMEM;
- }
-
- ntb->epf_db = mw_addr;
-
epf_bar = &ntb->epf->bar[barno];
- ret = pci_epc_set_bar(ntb->epf->epc, ntb->epf->func_no, ntb->epf->vfunc_no, epf_bar);
+ ret = epf_ntb_db_bar_init_msi_doorbell(ntb, epf_bar, epc_features, barno);
if (ret) {
- dev_err(dev, "Doorbell BAR set failed\n");
+ /* fall back to polling mode */
+ mw_addr = pci_epf_alloc_space(ntb->epf, size, barno, epc_features, 0);
+ if (!mw_addr) {
+ dev_err(dev, "Failed to allocate OB address\n");
+ return -ENOMEM;
+ }
+
+ ntb->epf_db = mw_addr;
+
+ ret = pci_epc_set_bar(ntb->epf->epc, ntb->epf->func_no,
+ ntb->epf->vfunc_no, epf_bar);
+ if (ret) {
+ dev_err(dev, "Doorbell BAR set failed\n");
goto err_alloc_peer_mem;
+ }
}
+
return ret;
err_alloc_peer_mem:
@@ -554,6 +663,16 @@ static void epf_ntb_db_bar_clear(struct epf_ntb *ntb)
{
enum pci_barno barno;
+ if (ntb->msi_doorbell) {
+ int i;
+
+ for (i = 0; i < ntb->db_count; i++)
+ free_irq(ntb->epf->db_msg[i].virq, ntb);
+ }
+
+ if (ntb->epf->db_msg)
+ pci_epf_free_doorbell(ntb->epf);
+
barno = ntb->epf_ntb_bar[BAR_DB];
pci_epf_free_space(ntb->epf, ntb->epf_db, barno, 0);
pci_epc_clear_bar(ntb->epf->epc,
@@ -1268,7 +1387,7 @@ static u64 vntb_epf_db_read(struct ntb_dev *ndev)
{
struct epf_ntb *ntb = ntb_ndev(ndev);
- return ntb->db;
+ return atomic64_read(&ntb->db);
}
static int vntb_epf_mw_get_align(struct ntb_dev *ndev, int pidx, int idx,
@@ -1308,7 +1427,7 @@ static int vntb_epf_db_clear(struct ntb_dev *ndev, u64 db_bits)
{
struct epf_ntb *ntb = ntb_ndev(ndev);
- ntb->db &= ~db_bits;
+ atomic64_and(~db_bits, &ntb->db);
return 0;
}
--
2.34.1
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v2 3/3] PCI: endpoint: pci-epf-vntb: Add MSI doorbell support
2025-09-15 22:22 ` [PATCH v2 3/3] PCI: endpoint: pci-epf-vntb: Add MSI doorbell support Frank Li
@ 2025-09-24 18:22 ` Manivannan Sadhasivam
2025-09-24 20:51 ` Frank Li
0 siblings, 1 reply; 13+ messages in thread
From: Manivannan Sadhasivam @ 2025-09-24 18:22 UTC (permalink / raw)
To: Frank Li
Cc: Krzysztof Wilczyński, Kishon Vijay Abraham I, Bjorn Helgaas,
Jon Mason, Dave Jiang, Allen Hubbe, linux-pci, linux-kernel, ntb,
imx
On Mon, Sep 15, 2025 at 06:22:46PM -0400, Frank Li wrote:
> Add MSI doorbell support to reduce latency between PCI host and EP.
>
> Before this change:
> ping 169.254.172.137
> 64 bytes from 169.254.172.137: icmp_seq=1 ttl=64 time=0.575 ms
> 64 bytes from 169.254.172.137: icmp_seq=2 ttl=64 time=1.80 ms
> 64 bytes from 169.254.172.137: icmp_seq=3 ttl=64 time=8.19 ms
> 64 bytes from 169.254.172.137: icmp_seq=4 ttl=64 time=2.00 ms
>
> After this change:
> ping 169.254.144.71
> 64 bytes from 169.254.144.71: icmp_seq=1 ttl=64 time=0.215 ms
> 64 bytes from 169.254.144.71: icmp_seq=2 ttl=64 time=0.456 ms
> 64 bytes from 169.254.144.71: icmp_seq=3 ttl=64 time=0.448 ms
>
> Change u64 db to atomic_64 because difference doorbell may happen at the
> same time.
>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
> change in v2
> - update api pci_epf_set_inbound_space
> - atomic_64 should be enough, which just record doorbell events, which is
> similar with W1C irq status register.
> ---
> drivers/pci/endpoint/functions/pci-epf-vntb.c | 153 +++++++++++++++++++++++---
> 1 file changed, 136 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> index 83e9ab10f9c4fc2b485d5463faa2172500f12999..06c13f26db1c6e55d23769e98e2cfd80da693a20 100644
> --- a/drivers/pci/endpoint/functions/pci-epf-vntb.c
> +++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> @@ -36,11 +36,13 @@
> * PCIe Root Port PCI EP
> */
>
> +#include <linux/atomic.h>
> #include <linux/delay.h>
> #include <linux/io.h>
> #include <linux/module.h>
> #include <linux/slab.h>
>
> +#include <linux/pci-ep-msi.h>
> #include <linux/pci-epc.h>
> #include <linux/pci-epf.h>
> #include <linux/ntb.h>
> @@ -126,12 +128,13 @@ struct epf_ntb {
> u32 db_count;
> u32 spad_count;
> u64 mws_size[MAX_MW];
> - u64 db;
> + atomic64_t db;
> u32 vbus_number;
> u16 vntb_pid;
> u16 vntb_vid;
>
> bool linkup;
> + bool msi_doorbell;
> u32 spad_size;
>
> enum pci_barno epf_ntb_bar[VNTB_BAR_NUM];
> @@ -258,9 +261,9 @@ static void epf_ntb_cmd_handler(struct work_struct *work)
>
> ntb = container_of(work, struct epf_ntb, cmd_handler.work);
>
> - for (i = 1; i < ntb->db_count; i++) {
> + for (i = 1; i < ntb->db_count && !ntb->msi_doorbell; i++) {
> if (ntb->epf_db[i]) {
> - ntb->db |= 1 << (i - 1);
> + atomic64_or(1 << (i - 1), &ntb->db);
> ntb_db_event(&ntb->ntb, i);
> ntb->epf_db[i] = 0;
> }
> @@ -319,7 +322,24 @@ static void epf_ntb_cmd_handler(struct work_struct *work)
>
> reset_handler:
> queue_delayed_work(kpcintb_workqueue, &ntb->cmd_handler,
> - msecs_to_jiffies(5));
> + ntb->msi_doorbell ? msecs_to_jiffies(500) : msecs_to_jiffies(5));
> +}
> +
> +static irqreturn_t epf_ntb_doorbell_handler(int irq, void *data)
> +{
> + struct epf_ntb *ntb = data;
> + int i = 0;
> +
> + for (i = 1; i < ntb->db_count; i++)
> + if (irq == ntb->epf->db_msg[i].virq) {
> + atomic64_or(1 << (i - 1), &ntb->db);
> + ntb_db_event(&ntb->ntb, i);
> + }
> +
> + if (irq == ntb->epf->db_msg[0].virq)
So doorbell 0 is supposed to trigger the command handler? Is it documented
somewhere?
> + queue_delayed_work(kpcintb_workqueue, &ntb->cmd_handler, 0);
> +
> + return IRQ_HANDLED;
> }
>
> /**
> @@ -500,6 +520,90 @@ static int epf_ntb_configure_interrupt(struct epf_ntb *ntb)
> return 0;
> }
>
> +static int epf_ntb_db_bar_init_msi_doorbell(struct epf_ntb *ntb,
> + struct pci_epf_bar *db_bar,
> + const struct pci_epc_features *epc_features,
> + enum pci_barno barno)
> +{
> + struct pci_epf *epf = ntb->epf;
> + dma_addr_t low, high;
> + struct msi_msg *msg;
> + size_t sz;
> + int ret;
> + int i;
> +
> + ret = pci_epf_alloc_doorbell(epf, ntb->db_count);
> + if (ret)
> + return ret;
> +
> + for (i = 0; i < ntb->db_count; i++) {
> + ret = request_irq(epf->db_msg[i].virq, epf_ntb_doorbell_handler,
> + 0, "vntb_db", ntb);
> +
> + if (ret) {
> + dev_err(&epf->dev,
> + "Failed to request doorbell IRQ: %d\n",
> + epf->db_msg[i].virq);
> + goto err_request_irq;
> + }
> + }
> +
> + msg = &epf->db_msg[0].msg;
> +
> + high = 0;
> + low = (u64)msg->address_hi << 32 | msg->address_lo;
> +
> + for (i = 0; i < ntb->db_count; i++) {
> + struct msi_msg *msg = &epf->db_msg[i].msg;
> + dma_addr_t addr = (u64)msg->address_hi << 32 | msg->address_lo;
> +
> + low = min(low, addr);
> + high = max(high, addr);
> + }
> +
> + sz = high - low + sizeof(u32);
> +
> + ret = pci_epf_set_inbound_space(epf, sz, barno,
> + epc_features, 0, low);
Should this new API be used in pci-epf-test also?
- Mani
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v2 3/3] PCI: endpoint: pci-epf-vntb: Add MSI doorbell support
2025-09-24 18:22 ` Manivannan Sadhasivam
@ 2025-09-24 20:51 ` Frank Li
2025-09-25 9:22 ` Manivannan Sadhasivam
0 siblings, 1 reply; 13+ messages in thread
From: Frank Li @ 2025-09-24 20:51 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: Krzysztof Wilczyński, Kishon Vijay Abraham I, Bjorn Helgaas,
Jon Mason, Dave Jiang, Allen Hubbe, linux-pci, linux-kernel, ntb,
imx
On Wed, Sep 24, 2025 at 11:52:29PM +0530, Manivannan Sadhasivam wrote:
> On Mon, Sep 15, 2025 at 06:22:46PM -0400, Frank Li wrote:
> > Add MSI doorbell support to reduce latency between PCI host and EP.
> >
> > Before this change:
> > ping 169.254.172.137
> > 64 bytes from 169.254.172.137: icmp_seq=1 ttl=64 time=0.575 ms
> > 64 bytes from 169.254.172.137: icmp_seq=2 ttl=64 time=1.80 ms
> > 64 bytes from 169.254.172.137: icmp_seq=3 ttl=64 time=8.19 ms
> > 64 bytes from 169.254.172.137: icmp_seq=4 ttl=64 time=2.00 ms
> >
> > After this change:
> > ping 169.254.144.71
> > 64 bytes from 169.254.144.71: icmp_seq=1 ttl=64 time=0.215 ms
> > 64 bytes from 169.254.144.71: icmp_seq=2 ttl=64 time=0.456 ms
> > 64 bytes from 169.254.144.71: icmp_seq=3 ttl=64 time=0.448 ms
> >
> > Change u64 db to atomic_64 because difference doorbell may happen at the
> > same time.
> >
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > ---
> > change in v2
> > - update api pci_epf_set_inbound_space
> > - atomic_64 should be enough, which just record doorbell events, which is
> > similar with W1C irq status register.
> > ---
> > drivers/pci/endpoint/functions/pci-epf-vntb.c | 153 +++++++++++++++++++++++---
> > 1 file changed, 136 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> > index 83e9ab10f9c4fc2b485d5463faa2172500f12999..06c13f26db1c6e55d23769e98e2cfd80da693a20 100644
> > --- a/drivers/pci/endpoint/functions/pci-epf-vntb.c
> > +++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> > @@ -36,11 +36,13 @@
> > * PCIe Root Port PCI EP
> > */
> >
> > +#include <linux/atomic.h>
> > #include <linux/delay.h>
> > #include <linux/io.h>
> > #include <linux/module.h>
> > #include <linux/slab.h>
> >
> > +#include <linux/pci-ep-msi.h>
> > #include <linux/pci-epc.h>
> > #include <linux/pci-epf.h>
> > #include <linux/ntb.h>
> > @@ -126,12 +128,13 @@ struct epf_ntb {
> > u32 db_count;
> > u32 spad_count;
> > u64 mws_size[MAX_MW];
> > - u64 db;
> > + atomic64_t db;
> > u32 vbus_number;
> > u16 vntb_pid;
> > u16 vntb_vid;
> >
> > bool linkup;
> > + bool msi_doorbell;
> > u32 spad_size;
> >
> > enum pci_barno epf_ntb_bar[VNTB_BAR_NUM];
> > @@ -258,9 +261,9 @@ static void epf_ntb_cmd_handler(struct work_struct *work)
> >
> > ntb = container_of(work, struct epf_ntb, cmd_handler.work);
> >
> > - for (i = 1; i < ntb->db_count; i++) {
> > + for (i = 1; i < ntb->db_count && !ntb->msi_doorbell; i++) {
> > if (ntb->epf_db[i]) {
> > - ntb->db |= 1 << (i - 1);
> > + atomic64_or(1 << (i - 1), &ntb->db);
> > ntb_db_event(&ntb->ntb, i);
> > ntb->epf_db[i] = 0;
> > }
> > @@ -319,7 +322,24 @@ static void epf_ntb_cmd_handler(struct work_struct *work)
> >
> > reset_handler:
> > queue_delayed_work(kpcintb_workqueue, &ntb->cmd_handler,
> > - msecs_to_jiffies(5));
> > + ntb->msi_doorbell ? msecs_to_jiffies(500) : msecs_to_jiffies(5));
> > +}
> > +
> > +static irqreturn_t epf_ntb_doorbell_handler(int irq, void *data)
> > +{
> > + struct epf_ntb *ntb = data;
> > + int i = 0;
> > +
> > + for (i = 1; i < ntb->db_count; i++)
> > + if (irq == ntb->epf->db_msg[i].virq) {
> > + atomic64_or(1 << (i - 1), &ntb->db);
> > + ntb_db_event(&ntb->ntb, i);
> > + }
> > +
> > + if (irq == ntb->epf->db_msg[0].virq)
>
> So doorbell 0 is supposed to trigger the command handler? Is it documented
> somewhere?
I missed my very old patch in drivers/ntb/hw/epf/ntb_hw_epf.c to use
doorbell 0 as trigger command handler, this path should never triggered.
db 0 is not used in ntb_hw_epf.c
>
> > + queue_delayed_work(kpcintb_workqueue, &ntb->cmd_handler, 0);
> > +
> > + return IRQ_HANDLED;
> > }
> >
> > /**
> > @@ -500,6 +520,90 @@ static int epf_ntb_configure_interrupt(struct epf_ntb *ntb)
> > return 0;
> > }
> >
> > +static int epf_ntb_db_bar_init_msi_doorbell(struct epf_ntb *ntb,
> > + struct pci_epf_bar *db_bar,
> > + const struct pci_epc_features *epc_features,
> > + enum pci_barno barno)
> > +{
> > + struct pci_epf *epf = ntb->epf;
> > + dma_addr_t low, high;
> > + struct msi_msg *msg;
> > + size_t sz;
> > + int ret;
> > + int i;
> > +
> > + ret = pci_epf_alloc_doorbell(epf, ntb->db_count);
> > + if (ret)
> > + return ret;
> > +
> > + for (i = 0; i < ntb->db_count; i++) {
> > + ret = request_irq(epf->db_msg[i].virq, epf_ntb_doorbell_handler,
> > + 0, "vntb_db", ntb);
> > +
> > + if (ret) {
> > + dev_err(&epf->dev,
> > + "Failed to request doorbell IRQ: %d\n",
> > + epf->db_msg[i].virq);
> > + goto err_request_irq;
> > + }
> > + }
> > +
> > + msg = &epf->db_msg[0].msg;
> > +
> > + high = 0;
> > + low = (u64)msg->address_hi << 32 | msg->address_lo;
> > +
> > + for (i = 0; i < ntb->db_count; i++) {
> > + struct msi_msg *msg = &epf->db_msg[i].msg;
> > + dma_addr_t addr = (u64)msg->address_hi << 32 | msg->address_lo;
> > +
> > + low = min(low, addr);
> > + high = max(high, addr);
> > + }
> > +
> > + sz = high - low + sizeof(u32);
> > +
> > + ret = pci_epf_set_inbound_space(epf, sz, barno,
> > + epc_features, 0, low);
>
> Should this new API be used in pci-epf-test also?
Needn't, because pcie-epf-test default set system memory as bar's space.
switch to MMIO when enable doorbell and switch back to system memory.
size alignment already consider at bar initilization, and we can't change
bar's size after bind now.
Ideally, msg MMIO space can append into BAR0 space to avoid polling status
change. But not easy to implement yet.
Frank
>
> - Mani
>
> --
> மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v2 3/3] PCI: endpoint: pci-epf-vntb: Add MSI doorbell support
2025-09-24 20:51 ` Frank Li
@ 2025-09-25 9:22 ` Manivannan Sadhasivam
2025-09-25 15:25 ` Frank Li
0 siblings, 1 reply; 13+ messages in thread
From: Manivannan Sadhasivam @ 2025-09-25 9:22 UTC (permalink / raw)
To: Frank Li
Cc: Krzysztof Wilczyński, Kishon Vijay Abraham I, Bjorn Helgaas,
Jon Mason, Dave Jiang, Allen Hubbe, linux-pci, linux-kernel, ntb,
imx
On Wed, Sep 24, 2025 at 04:51:16PM -0400, Frank Li wrote:
> On Wed, Sep 24, 2025 at 11:52:29PM +0530, Manivannan Sadhasivam wrote:
> > On Mon, Sep 15, 2025 at 06:22:46PM -0400, Frank Li wrote:
> > > Add MSI doorbell support to reduce latency between PCI host and EP.
> > >
> > > Before this change:
> > > ping 169.254.172.137
> > > 64 bytes from 169.254.172.137: icmp_seq=1 ttl=64 time=0.575 ms
> > > 64 bytes from 169.254.172.137: icmp_seq=2 ttl=64 time=1.80 ms
> > > 64 bytes from 169.254.172.137: icmp_seq=3 ttl=64 time=8.19 ms
> > > 64 bytes from 169.254.172.137: icmp_seq=4 ttl=64 time=2.00 ms
> > >
> > > After this change:
> > > ping 169.254.144.71
> > > 64 bytes from 169.254.144.71: icmp_seq=1 ttl=64 time=0.215 ms
> > > 64 bytes from 169.254.144.71: icmp_seq=2 ttl=64 time=0.456 ms
> > > 64 bytes from 169.254.144.71: icmp_seq=3 ttl=64 time=0.448 ms
> > >
> > > Change u64 db to atomic_64 because difference doorbell may happen at the
> > > same time.
> > >
> > > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > > ---
> > > change in v2
> > > - update api pci_epf_set_inbound_space
> > > - atomic_64 should be enough, which just record doorbell events, which is
> > > similar with W1C irq status register.
> > > ---
> > > drivers/pci/endpoint/functions/pci-epf-vntb.c | 153 +++++++++++++++++++++++---
> > > 1 file changed, 136 insertions(+), 17 deletions(-)
> > >
> > > diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> > > index 83e9ab10f9c4fc2b485d5463faa2172500f12999..06c13f26db1c6e55d23769e98e2cfd80da693a20 100644
> > > --- a/drivers/pci/endpoint/functions/pci-epf-vntb.c
> > > +++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> > > @@ -36,11 +36,13 @@
> > > * PCIe Root Port PCI EP
> > > */
> > >
> > > +#include <linux/atomic.h>
> > > #include <linux/delay.h>
> > > #include <linux/io.h>
> > > #include <linux/module.h>
> > > #include <linux/slab.h>
> > >
> > > +#include <linux/pci-ep-msi.h>
> > > #include <linux/pci-epc.h>
> > > #include <linux/pci-epf.h>
> > > #include <linux/ntb.h>
> > > @@ -126,12 +128,13 @@ struct epf_ntb {
> > > u32 db_count;
> > > u32 spad_count;
> > > u64 mws_size[MAX_MW];
> > > - u64 db;
> > > + atomic64_t db;
> > > u32 vbus_number;
> > > u16 vntb_pid;
> > > u16 vntb_vid;
> > >
> > > bool linkup;
> > > + bool msi_doorbell;
> > > u32 spad_size;
> > >
> > > enum pci_barno epf_ntb_bar[VNTB_BAR_NUM];
> > > @@ -258,9 +261,9 @@ static void epf_ntb_cmd_handler(struct work_struct *work)
> > >
> > > ntb = container_of(work, struct epf_ntb, cmd_handler.work);
> > >
> > > - for (i = 1; i < ntb->db_count; i++) {
> > > + for (i = 1; i < ntb->db_count && !ntb->msi_doorbell; i++) {
> > > if (ntb->epf_db[i]) {
> > > - ntb->db |= 1 << (i - 1);
> > > + atomic64_or(1 << (i - 1), &ntb->db);
> > > ntb_db_event(&ntb->ntb, i);
> > > ntb->epf_db[i] = 0;
> > > }
> > > @@ -319,7 +322,24 @@ static void epf_ntb_cmd_handler(struct work_struct *work)
> > >
> > > reset_handler:
> > > queue_delayed_work(kpcintb_workqueue, &ntb->cmd_handler,
> > > - msecs_to_jiffies(5));
> > > + ntb->msi_doorbell ? msecs_to_jiffies(500) : msecs_to_jiffies(5));
> > > +}
> > > +
> > > +static irqreturn_t epf_ntb_doorbell_handler(int irq, void *data)
> > > +{
> > > + struct epf_ntb *ntb = data;
> > > + int i = 0;
> > > +
> > > + for (i = 1; i < ntb->db_count; i++)
> > > + if (irq == ntb->epf->db_msg[i].virq) {
> > > + atomic64_or(1 << (i - 1), &ntb->db);
> > > + ntb_db_event(&ntb->ntb, i);
> > > + }
> > > +
> > > + if (irq == ntb->epf->db_msg[0].virq)
> >
> > So doorbell 0 is supposed to trigger the command handler? Is it documented
> > somewhere?
>
> I missed my very old patch in drivers/ntb/hw/epf/ntb_hw_epf.c to use
> doorbell 0 as trigger command handler, this path should never triggered.
>
> db 0 is not used in ntb_hw_epf.c
>
> >
> > > + queue_delayed_work(kpcintb_workqueue, &ntb->cmd_handler, 0);
> > > +
> > > + return IRQ_HANDLED;
> > > }
> > >
> > > /**
> > > @@ -500,6 +520,90 @@ static int epf_ntb_configure_interrupt(struct epf_ntb *ntb)
> > > return 0;
> > > }
> > >
> > > +static int epf_ntb_db_bar_init_msi_doorbell(struct epf_ntb *ntb,
> > > + struct pci_epf_bar *db_bar,
> > > + const struct pci_epc_features *epc_features,
> > > + enum pci_barno barno)
> > > +{
> > > + struct pci_epf *epf = ntb->epf;
> > > + dma_addr_t low, high;
> > > + struct msi_msg *msg;
> > > + size_t sz;
> > > + int ret;
> > > + int i;
> > > +
> > > + ret = pci_epf_alloc_doorbell(epf, ntb->db_count);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + for (i = 0; i < ntb->db_count; i++) {
> > > + ret = request_irq(epf->db_msg[i].virq, epf_ntb_doorbell_handler,
> > > + 0, "vntb_db", ntb);
> > > +
> > > + if (ret) {
> > > + dev_err(&epf->dev,
> > > + "Failed to request doorbell IRQ: %d\n",
> > > + epf->db_msg[i].virq);
> > > + goto err_request_irq;
> > > + }
> > > + }
> > > +
> > > + msg = &epf->db_msg[0].msg;
> > > +
> > > + high = 0;
> > > + low = (u64)msg->address_hi << 32 | msg->address_lo;
> > > +
> > > + for (i = 0; i < ntb->db_count; i++) {
> > > + struct msi_msg *msg = &epf->db_msg[i].msg;
> > > + dma_addr_t addr = (u64)msg->address_hi << 32 | msg->address_lo;
> > > +
> > > + low = min(low, addr);
> > > + high = max(high, addr);
> > > + }
> > > +
> > > + sz = high - low + sizeof(u32);
> > > +
> > > + ret = pci_epf_set_inbound_space(epf, sz, barno,
> > > + epc_features, 0, low);
> >
> > Should this new API be used in pci-epf-test also?
>
> Needn't, because pcie-epf-test default set system memory as bar's space.
> switch to MMIO when enable doorbell and switch back to system memory.
>
> size alignment already consider at bar initilization, and we can't change
> bar's size after bind now.
>
The memory and size are allocated during pci_epf_test_bind(), I agree. But
that's for memory allocated through pci_epf_alloc_space(). So if the MSI region
has size restrictions (as taken care by this API), it should be handled in the
pci-epf-test driver as well.
Otherwise, we will end up with different implementations between EPF drivers.
- Mani
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v2 3/3] PCI: endpoint: pci-epf-vntb: Add MSI doorbell support
2025-09-25 9:22 ` Manivannan Sadhasivam
@ 2025-09-25 15:25 ` Frank Li
2025-09-25 16:09 ` Manivannan Sadhasivam
0 siblings, 1 reply; 13+ messages in thread
From: Frank Li @ 2025-09-25 15:25 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: Krzysztof Wilczyński, Kishon Vijay Abraham I, Bjorn Helgaas,
Jon Mason, Dave Jiang, Allen Hubbe, linux-pci, linux-kernel, ntb,
imx
On Thu, Sep 25, 2025 at 02:52:38PM +0530, Manivannan Sadhasivam wrote:
> On Wed, Sep 24, 2025 at 04:51:16PM -0400, Frank Li wrote:
> > On Wed, Sep 24, 2025 at 11:52:29PM +0530, Manivannan Sadhasivam wrote:
> > > On Mon, Sep 15, 2025 at 06:22:46PM -0400, Frank Li wrote:
> > > > Add MSI doorbell support to reduce latency between PCI host and EP.
> > > >
> > > > Before this change:
> > > > ping 169.254.172.137
> > > > 64 bytes from 169.254.172.137: icmp_seq=1 ttl=64 time=0.575 ms
> > > > 64 bytes from 169.254.172.137: icmp_seq=2 ttl=64 time=1.80 ms
> > > > 64 bytes from 169.254.172.137: icmp_seq=3 ttl=64 time=8.19 ms
> > > > 64 bytes from 169.254.172.137: icmp_seq=4 ttl=64 time=2.00 ms
> > > >
> > > > After this change:
> > > > ping 169.254.144.71
> > > > 64 bytes from 169.254.144.71: icmp_seq=1 ttl=64 time=0.215 ms
> > > > 64 bytes from 169.254.144.71: icmp_seq=2 ttl=64 time=0.456 ms
> > > > 64 bytes from 169.254.144.71: icmp_seq=3 ttl=64 time=0.448 ms
> > > >
> > > > Change u64 db to atomic_64 because difference doorbell may happen at the
> > > > same time.
> > > >
> > > > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > > > ---
> > > > change in v2
> > > > - update api pci_epf_set_inbound_space
> > > > - atomic_64 should be enough, which just record doorbell events, which is
> > > > similar with W1C irq status register.
> > > > ---
> > > > drivers/pci/endpoint/functions/pci-epf-vntb.c | 153 +++++++++++++++++++++++---
> > > > 1 file changed, 136 insertions(+), 17 deletions(-)
> > > >
> > > > diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> > > > index 83e9ab10f9c4fc2b485d5463faa2172500f12999..06c13f26db1c6e55d23769e98e2cfd80da693a20 100644
> > > > --- a/drivers/pci/endpoint/functions/pci-epf-vntb.c
> > > > +++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> > > > @@ -36,11 +36,13 @@
> > > > * PCIe Root Port PCI EP
> > > > */
> > > >
> > > > +#include <linux/atomic.h>
> > > > #include <linux/delay.h>
> > > > #include <linux/io.h>
> > > > #include <linux/module.h>
> > > > #include <linux/slab.h>
> > > >
> > > > +#include <linux/pci-ep-msi.h>
> > > > #include <linux/pci-epc.h>
> > > > #include <linux/pci-epf.h>
> > > > #include <linux/ntb.h>
> > > > @@ -126,12 +128,13 @@ struct epf_ntb {
> > > > u32 db_count;
> > > > u32 spad_count;
> > > > u64 mws_size[MAX_MW];
> > > > - u64 db;
> > > > + atomic64_t db;
> > > > u32 vbus_number;
> > > > u16 vntb_pid;
> > > > u16 vntb_vid;
> > > >
> > > > bool linkup;
> > > > + bool msi_doorbell;
> > > > u32 spad_size;
> > > >
> > > > enum pci_barno epf_ntb_bar[VNTB_BAR_NUM];
> > > > @@ -258,9 +261,9 @@ static void epf_ntb_cmd_handler(struct work_struct *work)
> > > >
> > > > ntb = container_of(work, struct epf_ntb, cmd_handler.work);
> > > >
> > > > - for (i = 1; i < ntb->db_count; i++) {
> > > > + for (i = 1; i < ntb->db_count && !ntb->msi_doorbell; i++) {
> > > > if (ntb->epf_db[i]) {
> > > > - ntb->db |= 1 << (i - 1);
> > > > + atomic64_or(1 << (i - 1), &ntb->db);
> > > > ntb_db_event(&ntb->ntb, i);
> > > > ntb->epf_db[i] = 0;
> > > > }
> > > > @@ -319,7 +322,24 @@ static void epf_ntb_cmd_handler(struct work_struct *work)
> > > >
> > > > reset_handler:
> > > > queue_delayed_work(kpcintb_workqueue, &ntb->cmd_handler,
> > > > - msecs_to_jiffies(5));
> > > > + ntb->msi_doorbell ? msecs_to_jiffies(500) : msecs_to_jiffies(5));
> > > > +}
> > > > +
> > > > +static irqreturn_t epf_ntb_doorbell_handler(int irq, void *data)
> > > > +{
> > > > + struct epf_ntb *ntb = data;
> > > > + int i = 0;
> > > > +
> > > > + for (i = 1; i < ntb->db_count; i++)
> > > > + if (irq == ntb->epf->db_msg[i].virq) {
> > > > + atomic64_or(1 << (i - 1), &ntb->db);
> > > > + ntb_db_event(&ntb->ntb, i);
> > > > + }
> > > > +
> > > > + if (irq == ntb->epf->db_msg[0].virq)
> > >
> > > So doorbell 0 is supposed to trigger the command handler? Is it documented
> > > somewhere?
> >
> > I missed my very old patch in drivers/ntb/hw/epf/ntb_hw_epf.c to use
> > doorbell 0 as trigger command handler, this path should never triggered.
> >
> > db 0 is not used in ntb_hw_epf.c
> >
> > >
> > > > + queue_delayed_work(kpcintb_workqueue, &ntb->cmd_handler, 0);
> > > > +
> > > > + return IRQ_HANDLED;
> > > > }
> > > >
> > > > /**
> > > > @@ -500,6 +520,90 @@ static int epf_ntb_configure_interrupt(struct epf_ntb *ntb)
> > > > return 0;
> > > > }
> > > >
> > > > +static int epf_ntb_db_bar_init_msi_doorbell(struct epf_ntb *ntb,
> > > > + struct pci_epf_bar *db_bar,
> > > > + const struct pci_epc_features *epc_features,
> > > > + enum pci_barno barno)
> > > > +{
> > > > + struct pci_epf *epf = ntb->epf;
> > > > + dma_addr_t low, high;
> > > > + struct msi_msg *msg;
> > > > + size_t sz;
> > > > + int ret;
> > > > + int i;
> > > > +
> > > > + ret = pci_epf_alloc_doorbell(epf, ntb->db_count);
> > > > + if (ret)
> > > > + return ret;
> > > > +
> > > > + for (i = 0; i < ntb->db_count; i++) {
> > > > + ret = request_irq(epf->db_msg[i].virq, epf_ntb_doorbell_handler,
> > > > + 0, "vntb_db", ntb);
> > > > +
> > > > + if (ret) {
> > > > + dev_err(&epf->dev,
> > > > + "Failed to request doorbell IRQ: %d\n",
> > > > + epf->db_msg[i].virq);
> > > > + goto err_request_irq;
> > > > + }
> > > > + }
> > > > +
> > > > + msg = &epf->db_msg[0].msg;
> > > > +
> > > > + high = 0;
> > > > + low = (u64)msg->address_hi << 32 | msg->address_lo;
> > > > +
> > > > + for (i = 0; i < ntb->db_count; i++) {
> > > > + struct msi_msg *msg = &epf->db_msg[i].msg;
> > > > + dma_addr_t addr = (u64)msg->address_hi << 32 | msg->address_lo;
> > > > +
> > > > + low = min(low, addr);
> > > > + high = max(high, addr);
> > > > + }
> > > > +
> > > > + sz = high - low + sizeof(u32);
> > > > +
> > > > + ret = pci_epf_set_inbound_space(epf, sz, barno,
> > > > + epc_features, 0, low);
> > >
> > > Should this new API be used in pci-epf-test also?
> >
> > Needn't, because pcie-epf-test default set system memory as bar's space.
> > switch to MMIO when enable doorbell and switch back to system memory.
> >
> > size alignment already consider at bar initilization, and we can't change
> > bar's size after bind now.
> >
>
> The memory and size are allocated during pci_epf_test_bind(), I agree. But
> that's for memory allocated through pci_epf_alloc_space(). So if the MSI region
> has size restrictions (as taken care by this API), it should be handled in the
> pci-epf-test driver as well.
pci_epf_alloc_space() is not only allocate memory, but also set bar's size.
But we can't change bar's size, for example allocated 64K at
pci_epf_alloc_space()
but MSI map only require 4K. Current framework can't change bar's size.
still have to use 64K for doorbell even MSI just require 4k.
>
> Otherwise, we will end up with different implementations between EPF drivers.
pci_epf_test is special use case for doorbell. No one switch a bar to MSI
as doorbell dynamtically. Generally, allocated a dedicated bar for
doorbell.
pci_epf_set_inbound_space() and pci_epf_alloc_space() should be exclusived.
Frank
>
> - Mani
>
> --
> மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v2 3/3] PCI: endpoint: pci-epf-vntb: Add MSI doorbell support
2025-09-25 15:25 ` Frank Li
@ 2025-09-25 16:09 ` Manivannan Sadhasivam
0 siblings, 0 replies; 13+ messages in thread
From: Manivannan Sadhasivam @ 2025-09-25 16:09 UTC (permalink / raw)
To: Frank Li
Cc: Krzysztof Wilczyński, Kishon Vijay Abraham I, Bjorn Helgaas,
Jon Mason, Dave Jiang, Allen Hubbe, linux-pci, linux-kernel, ntb,
imx
On Thu, Sep 25, 2025 at 11:25:41AM -0400, Frank Li wrote:
[...]
> > > > > +static int epf_ntb_db_bar_init_msi_doorbell(struct epf_ntb *ntb,
> > > > > + struct pci_epf_bar *db_bar,
> > > > > + const struct pci_epc_features *epc_features,
> > > > > + enum pci_barno barno)
> > > > > +{
> > > > > + struct pci_epf *epf = ntb->epf;
> > > > > + dma_addr_t low, high;
> > > > > + struct msi_msg *msg;
> > > > > + size_t sz;
> > > > > + int ret;
> > > > > + int i;
> > > > > +
> > > > > + ret = pci_epf_alloc_doorbell(epf, ntb->db_count);
> > > > > + if (ret)
> > > > > + return ret;
> > > > > +
> > > > > + for (i = 0; i < ntb->db_count; i++) {
> > > > > + ret = request_irq(epf->db_msg[i].virq, epf_ntb_doorbell_handler,
> > > > > + 0, "vntb_db", ntb);
> > > > > +
> > > > > + if (ret) {
> > > > > + dev_err(&epf->dev,
> > > > > + "Failed to request doorbell IRQ: %d\n",
> > > > > + epf->db_msg[i].virq);
> > > > > + goto err_request_irq;
> > > > > + }
> > > > > + }
> > > > > +
> > > > > + msg = &epf->db_msg[0].msg;
> > > > > +
> > > > > + high = 0;
> > > > > + low = (u64)msg->address_hi << 32 | msg->address_lo;
> > > > > +
> > > > > + for (i = 0; i < ntb->db_count; i++) {
> > > > > + struct msi_msg *msg = &epf->db_msg[i].msg;
> > > > > + dma_addr_t addr = (u64)msg->address_hi << 32 | msg->address_lo;
> > > > > +
> > > > > + low = min(low, addr);
> > > > > + high = max(high, addr);
> > > > > + }
> > > > > +
> > > > > + sz = high - low + sizeof(u32);
> > > > > +
> > > > > + ret = pci_epf_set_inbound_space(epf, sz, barno,
> > > > > + epc_features, 0, low);
> > > >
> > > > Should this new API be used in pci-epf-test also?
> > >
> > > Needn't, because pcie-epf-test default set system memory as bar's space.
> > > switch to MMIO when enable doorbell and switch back to system memory.
> > >
> > > size alignment already consider at bar initilization, and we can't change
> > > bar's size after bind now.
> > >
> >
> > The memory and size are allocated during pci_epf_test_bind(), I agree. But
> > that's for memory allocated through pci_epf_alloc_space(). So if the MSI region
> > has size restrictions (as taken care by this API), it should be handled in the
> > pci-epf-test driver as well.
>
> pci_epf_alloc_space() is not only allocate memory, but also set bar's size.
>
> But we can't change bar's size, for example allocated 64K at
> pci_epf_alloc_space()
>
> but MSI map only require 4K. Current framework can't change bar's size.
> still have to use 64K for doorbell even MSI just require 4k.
>
> >
> > Otherwise, we will end up with different implementations between EPF drivers.
>
> pci_epf_test is special use case for doorbell. No one switch a bar to MSI
> as doorbell dynamtically. Generally, allocated a dedicated bar for
> doorbell.
>
Ok! I think we would ideally need to expose doorbell registers in any used BAR,
not just in a dedicated BAR. But I believe it is difficult to have multiple
inbound mappings for the same BAR with ATUs.
- Mani
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 0/3] pci: endpoint: vntb: add MSI doorbell support
2025-09-15 22:22 [PATCH v2 0/3] pci: endpoint: vntb: add MSI doorbell support Frank Li
` (2 preceding siblings ...)
2025-09-15 22:22 ` [PATCH v2 3/3] PCI: endpoint: pci-epf-vntb: Add MSI doorbell support Frank Li
@ 2025-09-24 16:23 ` Frank Li
3 siblings, 0 replies; 13+ messages in thread
From: Frank Li @ 2025-09-24 16:23 UTC (permalink / raw)
To: Manivannan Sadhasivam, Krzysztof Wilczyński,
Kishon Vijay Abraham I, Bjorn Helgaas, Jon Mason, Dave Jiang,
Allen Hubbe
Cc: linux-pci, linux-kernel, ntb, imx
On Mon, Sep 15, 2025 at 06:22:43PM -0400, Frank Li wrote:
> Since commit 1c3b002c6bf68 PCI: endpoint: Add RC-to-EP doorbell support
> using platform MSI controller, PCI EP can get notification from Host.
>
> VNTB use this feature to reduce ping latency.
>
> The first patch impove epf core API to allow set any MMIO address to specfic
> bar.
>
> The second patch add MSI doorbell support.
Manivannan Sadhasivam:
Do you have chance to check these patch again?
Frank
>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
> Changes in v2:
> - add help funciton to get bar's inbounce size
> - fix miss x8 when caculate bits
> - Link to v1: https://lore.kernel.org/r/20250815-vntb_msi_doorbell-v1-0-32df6c4bf96c@nxp.com
>
> ---
> Frank Li (3):
> PCI: endpoint: Add helper function pci_epf_get_bar_required_size()
> PCI: endpoint: Add API pci_epf_set_inbound_space()
> PCI: endpoint: pci-epf-vntb: Add MSI doorbell support
>
> drivers/pci/endpoint/functions/pci-epf-vntb.c | 153 +++++++++++++++++++++++---
> drivers/pci/endpoint/pci-epf-core.c | 138 ++++++++++++++++++++---
> include/linux/pci-epf.h | 6 +
> 3 files changed, 262 insertions(+), 35 deletions(-)
> ---
> base-commit: c2a282d1fccc53a989da61a5da4f03c9d67ee99a
> change-id: 20250812-vntb_msi_doorbell-bf0fbac6d6d7
>
> Best regards,
> --
> Frank Li <Frank.Li@nxp.com>
>
^ permalink raw reply [flat|nested] 13+ messages in thread