[v2] Add RC-to-EP doorbell with platform MSI controller

[PATCH v2 3/5] PCI: endpoint: pci-epf-test: add doorbell test

Posted by Frank Li 2 years, 3 months ago

Add three register: doorbell_bar, doorbell_addr, doorbell_data,
doorbell_done. Call pci_epf_alloc_doorbell() all a doorbell address space.

Root complex(RC) side driver can trigger pci-epc-test's doorbell callback
handler by write doorbell_data to mapped doorbell_bar's address space.

pci-epc-test will set doorbell_done in doorbell callback.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
 drivers/pci/endpoint/functions/pci-epf-test.c | 59 ++++++++++++++++++-
 1 file changed, 58 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
index 1f0d2b84296a3..566549919b87b 100644
--- a/drivers/pci/endpoint/functions/pci-epf-test.c
+++ b/drivers/pci/endpoint/functions/pci-epf-test.c
@@ -11,6 +11,7 @@
 #include <linux/dmaengine.h>
 #include <linux/io.h>
 #include <linux/module.h>
+#include <linux/msi.h>
 #include <linux/slab.h>
 #include <linux/pci_ids.h>
 #include <linux/random.h>
@@ -39,17 +40,21 @@
 #define STATUS_IRQ_RAISED		BIT(6)
 #define STATUS_SRC_ADDR_INVALID		BIT(7)
 #define STATUS_DST_ADDR_INVALID		BIT(8)
+#define STATUS_DOORBELL_SUCCESS		BIT(9)
 
 #define FLAG_USE_DMA			BIT(0)
 
 #define TIMER_RESOLUTION		1
 
+#define MAGIC_VERSION_MASK		GENMASK(7, 0)
+
 static struct workqueue_struct *kpcitest_workqueue;
 
 struct pci_epf_test {
 	void			*reg[PCI_STD_NUM_BARS];
 	struct pci_epf		*epf;
 	enum pci_barno		test_reg_bar;
+	enum pci_barno		doorbell_bar;
 	size_t			msix_table_offset;
 	struct delayed_work	cmd_handler;
 	struct dma_chan		*dma_chan_tx;
@@ -74,6 +79,9 @@ struct pci_epf_test_reg {
 	u32	irq_type;
 	u32	irq_number;
 	u32	flags;
+	u32	doorbell_bar;
+	u32	doorbell_addr;
+	u32	doorbell_data;
 } __packed;
 
 static struct pci_epf_header test_header = {
@@ -693,6 +701,8 @@ static void pci_epf_test_unbind(struct pci_epf *epf)
 	struct pci_epf_bar *epf_bar;
 	int bar;
 
+	pci_epf_free_doorbell(epf);
+
 	cancel_delayed_work(&epf_test->cmd_handler);
 	pci_epf_test_clean_dma_chan(epf_test);
 	for (bar = 0; bar < PCI_STD_NUM_BARS; bar++) {
@@ -808,9 +818,22 @@ static int pci_epf_test_link_up(struct pci_epf *epf)
 	return 0;
 }
 
+static int pci_epf_test_doorbell(struct pci_epf *epf, int index)
+{
+	struct pci_epf_test *epf_test = epf_get_drvdata(epf);
+	enum pci_barno test_reg_bar = epf_test->test_reg_bar;
+	struct pci_epf_test_reg *reg = epf_test->reg[test_reg_bar];
+
+	reg->status |= STATUS_DOORBELL_SUCCESS;
+	pci_epf_test_raise_irq(epf_test, reg);
+
+	return 0;
+}
+
 static const struct pci_epc_event_ops pci_epf_test_event_ops = {
 	.core_init = pci_epf_test_core_init,
 	.link_up = pci_epf_test_link_up,
+	.doorbell = pci_epf_test_doorbell,
 };
 
 static int pci_epf_test_alloc_space(struct pci_epf *epf)
@@ -859,7 +882,7 @@ static int pci_epf_test_alloc_space(struct pci_epf *epf)
 		epf_bar = &epf->bar[bar];
 		add = (epf_bar->flags & PCI_BASE_ADDRESS_MEM_TYPE_64) ? 2 : 1;
 
-		if (bar == test_reg_bar)
+		if (bar == test_reg_bar || bar == epf_test->doorbell_bar)
 			continue;
 
 		if (!!(epc_features->reserved_bar & (1 << bar)))
@@ -900,9 +923,14 @@ static int pci_epf_test_bind(struct pci_epf *epf)
 	struct pci_epf_test *epf_test = epf_get_drvdata(epf);
 	const struct pci_epc_features *epc_features;
 	enum pci_barno test_reg_bar = BAR_0;
+	enum pci_barno doorbell_bar = NO_BAR;
 	struct pci_epc *epc = epf->epc;
 	bool linkup_notifier = false;
 	bool core_init_notifier = false;
+	struct pci_epf_test_reg *reg;
+	struct msi_msg *msg;
+	u64 doorbell_addr;
+	u32 align;
 
 	if (WARN_ON_ONCE(!epc))
 		return -EINVAL;
@@ -923,10 +951,39 @@ static int pci_epf_test_bind(struct pci_epf *epf)
 	epf_test->test_reg_bar = test_reg_bar;
 	epf_test->epc_features = epc_features;
 
+	align = epc_features->align;
+	align = align ? align : 128;
+
+	ret = pci_epf_alloc_doorbell(epf, 1);
+	if (!ret) {
+		msg = epf->msg;
+		doorbell_bar = pci_epc_get_next_free_bar(epc_features, test_reg_bar + 1);
+
+		if (doorbell_bar > 0) {
+			epf_test->doorbell_bar = doorbell_bar;
+			doorbell_addr = msg->address_hi;
+			doorbell_addr <<= 32;
+			doorbell_addr |= msg->address_lo;
+			epf->bar[doorbell_bar].phys_addr = round_down(doorbell_addr, align);
+			epf->bar[doorbell_bar].barno = doorbell_bar;
+			epf->bar[doorbell_bar].size = align;
+		} else {
+			pci_epf_free_doorbell(epf);
+		}
+	}
+
 	ret = pci_epf_test_alloc_space(epf);
 	if (ret)
 		return ret;
 
+	reg = epf_test->reg[test_reg_bar];
+	reg->magic |= FIELD_PREP(MAGIC_VERSION_MASK, 0x1);
+	if (doorbell_bar > 0) {
+		reg->doorbell_addr = doorbell_addr & (align - 1);
+		reg->doorbell_data = msg->data;
+		reg->doorbell_bar = doorbell_bar;
+	}
+
 	if (!core_init_notifier) {
 		ret = pci_epf_test_core_init(epf);
 		if (ret)
-- 
2.34.1

Re: [PATCH v2 3/5] PCI: endpoint: pci-epf-test: add doorbell test

Posted by Manivannan Sadhasivam 2 years, 1 month ago

On Mon, Sep 11, 2023 at 06:09:18PM -0400, Frank Li wrote:

Subject could be,

PCI: endpoint: pci-epf-test: Add doorbell support

> Add three register: doorbell_bar, doorbell_addr, doorbell_data,
> doorbell_done. Call pci_epf_alloc_doorbell() all a doorbell address space.
> 
> Root complex(RC) side driver can trigger pci-epc-test's doorbell callback
> handler by write doorbell_data to mapped doorbell_bar's address space.
> 
> pci-epc-test will set doorbell_done in doorbell callback.
> 

How about,

Add doorbell support to the EPF test driver by introducing 3 new registers:

doorbell_bar
doorbell_addr
doorbell_data

The PCI RC driver can trigger the doorbell on the EP side by writing the
content of "doorbell_data" to the address specified by the "doorbell_addr"
register in the "doorbell_bar" BAR region.

> Signed-off-by: Frank Li <Frank.Li@nxp.com>

You should also update Documentation/PCI/endpoint/pci-test-* files in a separate
commit with doorbell support.

> ---
>  drivers/pci/endpoint/functions/pci-epf-test.c | 59 ++++++++++++++++++-
>  1 file changed, 58 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
> index 1f0d2b84296a3..566549919b87b 100644
> --- a/drivers/pci/endpoint/functions/pci-epf-test.c
> +++ b/drivers/pci/endpoint/functions/pci-epf-test.c
> @@ -11,6 +11,7 @@
>  #include <linux/dmaengine.h>
>  #include <linux/io.h>
>  #include <linux/module.h>
> +#include <linux/msi.h>
>  #include <linux/slab.h>
>  #include <linux/pci_ids.h>
>  #include <linux/random.h>
> @@ -39,17 +40,21 @@
>  #define STATUS_IRQ_RAISED		BIT(6)
>  #define STATUS_SRC_ADDR_INVALID		BIT(7)
>  #define STATUS_DST_ADDR_INVALID		BIT(8)
> +#define STATUS_DOORBELL_SUCCESS		BIT(9)
>  
>  #define FLAG_USE_DMA			BIT(0)
>  
>  #define TIMER_RESOLUTION		1
>  
> +#define MAGIC_VERSION_MASK		GENMASK(7, 0)
> +
>  static struct workqueue_struct *kpcitest_workqueue;
>  
>  struct pci_epf_test {
>  	void			*reg[PCI_STD_NUM_BARS];
>  	struct pci_epf		*epf;
>  	enum pci_barno		test_reg_bar;
> +	enum pci_barno		doorbell_bar;
>  	size_t			msix_table_offset;
>  	struct delayed_work	cmd_handler;
>  	struct dma_chan		*dma_chan_tx;
> @@ -74,6 +79,9 @@ struct pci_epf_test_reg {
>  	u32	irq_type;
>  	u32	irq_number;
>  	u32	flags;
> +	u32	doorbell_bar;
> +	u32	doorbell_addr;
> +	u32	doorbell_data;
>  } __packed;
>  
>  static struct pci_epf_header test_header = {
> @@ -693,6 +701,8 @@ static void pci_epf_test_unbind(struct pci_epf *epf)
>  	struct pci_epf_bar *epf_bar;
>  	int bar;
>  
> +	pci_epf_free_doorbell(epf);
> +
>  	cancel_delayed_work(&epf_test->cmd_handler);
>  	pci_epf_test_clean_dma_chan(epf_test);
>  	for (bar = 0; bar < PCI_STD_NUM_BARS; bar++) {
> @@ -808,9 +818,22 @@ static int pci_epf_test_link_up(struct pci_epf *epf)
>  	return 0;
>  }
>  
> +static int pci_epf_test_doorbell(struct pci_epf *epf, int index)
> +{
> +	struct pci_epf_test *epf_test = epf_get_drvdata(epf);
> +	enum pci_barno test_reg_bar = epf_test->test_reg_bar;
> +	struct pci_epf_test_reg *reg = epf_test->reg[test_reg_bar];
> +
> +	reg->status |= STATUS_DOORBELL_SUCCESS;
> +	pci_epf_test_raise_irq(epf_test, reg);
> +
> +	return 0;
> +}
> +
>  static const struct pci_epc_event_ops pci_epf_test_event_ops = {
>  	.core_init = pci_epf_test_core_init,
>  	.link_up = pci_epf_test_link_up,
> +	.doorbell = pci_epf_test_doorbell,

I would like to pass this callback directly to the pci_epf_alloc_doorbell() API.
 Would that be feasible?

>  };
>  
>  static int pci_epf_test_alloc_space(struct pci_epf *epf)
> @@ -859,7 +882,7 @@ static int pci_epf_test_alloc_space(struct pci_epf *epf)
>  		epf_bar = &epf->bar[bar];
>  		add = (epf_bar->flags & PCI_BASE_ADDRESS_MEM_TYPE_64) ? 2 : 1;
>  
> -		if (bar == test_reg_bar)
> +		if (bar == test_reg_bar || bar == epf_test->doorbell_bar)
>  			continue;
>  
>  		if (!!(epc_features->reserved_bar & (1 << bar)))
> @@ -900,9 +923,14 @@ static int pci_epf_test_bind(struct pci_epf *epf)
>  	struct pci_epf_test *epf_test = epf_get_drvdata(epf);
>  	const struct pci_epc_features *epc_features;
>  	enum pci_barno test_reg_bar = BAR_0;
> +	enum pci_barno doorbell_bar = NO_BAR;
>  	struct pci_epc *epc = epf->epc;
>  	bool linkup_notifier = false;
>  	bool core_init_notifier = false;
> +	struct pci_epf_test_reg *reg;
> +	struct msi_msg *msg;
> +	u64 doorbell_addr;
> +	u32 align;
>  
>  	if (WARN_ON_ONCE(!epc))
>  		return -EINVAL;
> @@ -923,10 +951,39 @@ static int pci_epf_test_bind(struct pci_epf *epf)
>  	epf_test->test_reg_bar = test_reg_bar;
>  	epf_test->epc_features = epc_features;
>  
> +	align = epc_features->align;
> +	align = align ? align : 128;
> +
> +	ret = pci_epf_alloc_doorbell(epf, 1);

This should be renamed as pci_epc_alloc_doorbell() as per comment on patch 1/3.
Also, the "msi_msg" pointer should be part of the EPC struct.

> +	if (!ret) {
> +		msg = epf->msg;
> +		doorbell_bar = pci_epc_get_next_free_bar(epc_features, test_reg_bar + 1);
> +
> +		if (doorbell_bar > 0) {
> +			epf_test->doorbell_bar = doorbell_bar;
> +			doorbell_addr = msg->address_hi;
> +			doorbell_addr <<= 32;
> +			doorbell_addr |= msg->address_lo;
> +			epf->bar[doorbell_bar].phys_addr = round_down(doorbell_addr, align);
> +			epf->bar[doorbell_bar].barno = doorbell_bar;
> +			epf->bar[doorbell_bar].size = align;
> +		} else {
> +			pci_epf_free_doorbell(epf);

This one too should be renamed. 

> +		}
> +	}
> +
>  	ret = pci_epf_test_alloc_space(epf);

This one too.

>  	if (ret)
>  		return ret;
>  
> +	reg = epf_test->reg[test_reg_bar];
> +	reg->magic |= FIELD_PREP(MAGIC_VERSION_MASK, 0x1);

Why are you writing this register? This register serves for the purpose of
testing BAR0.

- Mani

> +	if (doorbell_bar > 0) {
> +		reg->doorbell_addr = doorbell_addr & (align - 1);
> +		reg->doorbell_data = msg->data;
> +		reg->doorbell_bar = doorbell_bar;
> +	}
> +
>  	if (!core_init_notifier) {
>  		ret = pci_epf_test_core_init(epf);
>  		if (ret)
> -- 
> 2.34.1
> 

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v2 3/5] PCI: endpoint: pci-epf-test: add doorbell test

Posted by Kishon Vijay Abraham I 2 years, 2 months ago

Hi Frank,

On 9/12/2023 3:39 AM, Frank Li wrote:
> Add three register: doorbell_bar, doorbell_addr, doorbell_data,
> doorbell_done. Call pci_epf_alloc_doorbell() all a doorbell address space.
> 
> Root complex(RC) side driver can trigger pci-epc-test's doorbell callback
> handler by write doorbell_data to mapped doorbell_bar's address space.
> 
> pci-epc-test will set doorbell_done in doorbell callback.
> 
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
>   drivers/pci/endpoint/functions/pci-epf-test.c | 59 ++++++++++++++++++-
>   1 file changed, 58 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
> index 1f0d2b84296a3..566549919b87b 100644
> --- a/drivers/pci/endpoint/functions/pci-epf-test.c
> +++ b/drivers/pci/endpoint/functions/pci-epf-test.c
> @@ -11,6 +11,7 @@
>   #include <linux/dmaengine.h>
>   #include <linux/io.h>
>   #include <linux/module.h>
> +#include <linux/msi.h>
>   #include <linux/slab.h>
>   #include <linux/pci_ids.h>
>   #include <linux/random.h>
> @@ -39,17 +40,21 @@
>   #define STATUS_IRQ_RAISED		BIT(6)
>   #define STATUS_SRC_ADDR_INVALID		BIT(7)
>   #define STATUS_DST_ADDR_INVALID		BIT(8)
> +#define STATUS_DOORBELL_SUCCESS		BIT(9)
>   
>   #define FLAG_USE_DMA			BIT(0)
>   
>   #define TIMER_RESOLUTION		1
>   
> +#define MAGIC_VERSION_MASK		GENMASK(7, 0)
> +
>   static struct workqueue_struct *kpcitest_workqueue;
>   
>   struct pci_epf_test {
>   	void			*reg[PCI_STD_NUM_BARS];
>   	struct pci_epf		*epf;
>   	enum pci_barno		test_reg_bar;
> +	enum pci_barno		doorbell_bar;
>   	size_t			msix_table_offset;
>   	struct delayed_work	cmd_handler;
>   	struct dma_chan		*dma_chan_tx;
> @@ -74,6 +79,9 @@ struct pci_epf_test_reg {
>   	u32	irq_type;
>   	u32	irq_number;
>   	u32	flags;
> +	u32	doorbell_bar;

You could extend test_reg_bar for doorbell to avoid using additional BAR.


> +	u32	doorbell_addr;
> +	u32	doorbell_data;
>   } __packed;
>   

Thanks,
Kishon

Re: [PATCH v2 3/5] PCI: endpoint: pci-epf-test: add doorbell test

Posted by Frank Li 2 years, 2 months ago

On Fri, Sep 29, 2023 at 03:03:35PM +0530, Kishon Vijay Abraham I wrote:
> Hi Frank,
> 
> On 9/12/2023 3:39 AM, Frank Li wrote:
> > Add three register: doorbell_bar, doorbell_addr, doorbell_data,
> > doorbell_done. Call pci_epf_alloc_doorbell() all a doorbell address space.
> > 
> > Root complex(RC) side driver can trigger pci-epc-test's doorbell callback
> > handler by write doorbell_data to mapped doorbell_bar's address space.
> > 
> > pci-epc-test will set doorbell_done in doorbell callback.
> > 
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > ---
> >   drivers/pci/endpoint/functions/pci-epf-test.c | 59 ++++++++++++++++++-
> >   1 file changed, 58 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
> > index 1f0d2b84296a3..566549919b87b 100644
> > --- a/drivers/pci/endpoint/functions/pci-epf-test.c
> > +++ b/drivers/pci/endpoint/functions/pci-epf-test.c
> > @@ -11,6 +11,7 @@
> >   #include <linux/dmaengine.h>
> >   #include <linux/io.h>
> >   #include <linux/module.h>
> > +#include <linux/msi.h>
> >   #include <linux/slab.h>
> >   #include <linux/pci_ids.h>
> >   #include <linux/random.h>
> > @@ -39,17 +40,21 @@
> >   #define STATUS_IRQ_RAISED		BIT(6)
> >   #define STATUS_SRC_ADDR_INVALID		BIT(7)
> >   #define STATUS_DST_ADDR_INVALID		BIT(8)
> > +#define STATUS_DOORBELL_SUCCESS		BIT(9)
> >   #define FLAG_USE_DMA			BIT(0)
> >   #define TIMER_RESOLUTION		1
> > +#define MAGIC_VERSION_MASK		GENMASK(7, 0)
> > +
> >   static struct workqueue_struct *kpcitest_workqueue;
> >   struct pci_epf_test {
> >   	void			*reg[PCI_STD_NUM_BARS];
> >   	struct pci_epf		*epf;
> >   	enum pci_barno		test_reg_bar;
> > +	enum pci_barno		doorbell_bar;
> >   	size_t			msix_table_offset;
> >   	struct delayed_work	cmd_handler;
> >   	struct dma_chan		*dma_chan_tx;
> > @@ -74,6 +79,9 @@ struct pci_epf_test_reg {
> >   	u32	irq_type;
> >   	u32	irq_number;
> >   	u32	flags;
> > +	u32	doorbell_bar;
> 
> You could extend test_reg_bar for doorbell to avoid using additional BAR.

It is sperated physical address space. So far, epc still not support map
two difference physcial address space into a bar.

So I have to use a sperate pci bar for doorbell.

Frank

> 
> 
> > +	u32	doorbell_addr;
> > +	u32	doorbell_data;
> >   } __packed;
> 
> Thanks,
> Kishon

[PATCH v2 1/5] PCI: endpoint: Add RC-to-EP doorbell support using platform MSI controller
[PATCH v2 2/5] PCI: dwc: add doorbell support by use MSI controller
[PATCH v2 3/5] PCI: endpoint: pci-epf-test: add doorbell test
[PATCH v2 4/5] misc: pci_endpoint_test: Add doorbell test case
[PATCH v2 5/5] tools: PCI: Add 'B' option for test doorbell