[RFC 00/10] Adds pcitest tool support for MSI-X

[RFC 00/10] Adds pcitest tool support for MSI-X

[Gustavo Pimentel]

This patch set depends the following series:
https://lkml.org/lkml/2018/4/10/421

This series aims to add pcitest tool support for MSI-X. 

Includes new callbacks methods and handlers to trigger the MSI-X
interruptions on the EP Designware IP driver.

Provides new methods on pci_epf_test driver that allows to set/get
EP maximum number of MSI-X entries (similar to set/get MSI methods).

Reworks on MSI set/get and triggering methods on EP Designware IP driver
to be more generic and flexible.

Adds a new input parameter (msix) and replicates the whole MSI mechanism
applied to the MSI-X feature on pcitest tool. Also updates the pcitest
script with the new test set applied to this new feature.

Gustavo Pimentel (10):
  PCI: dwc: Add MSI-X callbacks handler
  PCI: cadence: Update cdns_pcie_ep_raise_irq function signature
  PCI: endpoint: Add MSI-X interfaces
  PCI: dwc: MSI callbacks handler rework
  PCI: dwc: Add legacy interrupt callback handler
  misc: pci_endpoint_test: Add MSI-X support
  misc: pci_endpoint_test: Replace lower into upper case characters
  PCI: endpoint: functions/pci-epf-test: Add MSI-X support
  PCI: endpoint: functions/pci-epf-test: Replace lower into upper case
    characters
  tools: PCI: Add MSI-X support

 Documentation/misc-devices/pci-endpoint-test.txt |   3 +
 drivers/misc/pci_endpoint_test.c                 | 120 ++++++++++----
 drivers/pci/cadence/pcie-cadence-ep.c            |   2 +-
 drivers/pci/dwc/pci-dra7xx.c                     |   2 +-
 drivers/pci/dwc/pcie-artpec6.c                   |   2 +-
 drivers/pci/dwc/pcie-designware-ep.c             | 201 +++++++++++++++++++++--
 drivers/pci/dwc/pcie-designware-plat.c           |   9 +-
 drivers/pci/dwc/pcie-designware.h                |  40 +++--
 drivers/pci/endpoint/functions/pci-epf-test.c    | 113 +++++++++----
 drivers/pci/endpoint/pci-ep-cfs.c                |  24 +++
 drivers/pci/endpoint/pci-epc-core.c              |  60 ++++++-
 include/linux/pci-epc.h                          |  11 +-
 include/linux/pci-epf.h                          |   1 +
 include/uapi/linux/pcitest.h                     |   1 +
 tools/pci/pcitest.c                              |  18 +-
 tools/pci/pcitest.sh                             |  25 +++
 16 files changed, 528 insertions(+), 104 deletions(-)

-- 
2.7.4

[RFC 01/10] PCI: dwc: Add MSI-X callbacks handler

[Gustavo Pimentel]

Changes the pcie_raise_irq function signature, namely the interrupt_num
variable type from u8 to u16 to accommodate the MSI-X maximum interrupts
of 2048.

Implements a PCIe config space capability iterator function to search and
save the MSI and MSI-X pointers. With this method the code becomes more
generic and flexible.

Implements MSI-X set/get functions for sysfs interface in order to change
the EP entries number.

Implements EP MSI-X interface for triggering interruptions.

Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
---
 drivers/pci/dwc/pci-dra7xx.c           |   2 +-
 drivers/pci/dwc/pcie-artpec6.c         |   2 +-
 drivers/pci/dwc/pcie-designware-ep.c   | 145 ++++++++++++++++++++++++++++++++-
 drivers/pci/dwc/pcie-designware-plat.c |   6 +-
 drivers/pci/dwc/pcie-designware.h      |  23 +++++-
 5 files changed, 173 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/dwc/pci-dra7xx.c b/drivers/pci/dwc/pci-dra7xx.c
index ed8558d..5265725 100644
--- a/drivers/pci/dwc/pci-dra7xx.c
+++ b/drivers/pci/dwc/pci-dra7xx.c
@@ -369,7 +369,7 @@ static void dra7xx_pcie_raise_msi_irq(struct dra7xx_pcie *dra7xx,
 }
 
 static int dra7xx_pcie_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
-				 enum pci_epc_irq_type type, u8 interrupt_num)
+				 enum pci_epc_irq_type type, u16 interrupt_num)
 {
 	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
 	struct dra7xx_pcie *dra7xx = to_dra7xx_pcie(pci);
diff --git a/drivers/pci/dwc/pcie-artpec6.c b/drivers/pci/dwc/pcie-artpec6.c
index e66cede..96dc259 100644
--- a/drivers/pci/dwc/pcie-artpec6.c
+++ b/drivers/pci/dwc/pcie-artpec6.c
@@ -428,7 +428,7 @@ static void artpec6_pcie_ep_init(struct dw_pcie_ep *ep)
 }
 
 static int artpec6_pcie_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
-				  enum pci_epc_irq_type type, u8 interrupt_num)
+				  enum pci_epc_irq_type type, u16 interrupt_num)
 {
 	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
 
diff --git a/drivers/pci/dwc/pcie-designware-ep.c b/drivers/pci/dwc/pcie-designware-ep.c
index 15b22a6..874d4c2 100644
--- a/drivers/pci/dwc/pcie-designware-ep.c
+++ b/drivers/pci/dwc/pcie-designware-ep.c
@@ -40,6 +40,44 @@ void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar)
 	__dw_pcie_ep_reset_bar(pci, bar, 0);
 }
 
+void dw_pcie_ep_find_cap_addr(struct dw_pcie_ep *ep)
+{
+	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
+	u8 next_ptr, curr_ptr, cap_id;
+	u16 reg;
+
+	memset(&ep->cap_addr, 0, sizeof(ep->cap_addr));
+
+	reg = dw_pcie_readw_dbi(pci, PCI_STATUS);
+	if (!(reg & PCI_STATUS_CAP_LIST))
+		return;
+
+	reg = dw_pcie_readw_dbi(pci, PCI_CAPABILITY_LIST);
+	next_ptr = (reg & 0x00ff);
+	if (!next_ptr)
+		return;
+
+	reg = dw_pcie_readw_dbi(pci, next_ptr);
+	curr_ptr = next_ptr;
+	next_ptr = (reg & 0xff00) >> 8;
+	cap_id = (reg & 0x00ff);
+
+	while (next_ptr && (cap_id <= PCI_CAP_ID_MAX)) {
+		switch (cap_id) {
+		case PCI_CAP_ID_MSI:
+			ep->cap_addr.msi_addr = curr_ptr;
+			break;
+		case PCI_CAP_ID_MSIX:
+			ep->cap_addr.msix_addr = curr_ptr;
+			break;
+		}
+		reg = dw_pcie_readw_dbi(pci, next_ptr);
+		curr_ptr = next_ptr;
+		next_ptr = (reg & 0xff00) >> 8;
+		cap_id = (reg & 0x00ff);
+	}
+}
+
 static int dw_pcie_ep_write_header(struct pci_epc *epc, u8 func_no,
 				   struct pci_epf_header *hdr)
 {
@@ -241,8 +279,47 @@ static int dw_pcie_ep_set_msi(struct pci_epc *epc, u8 func_no, u8 encode_int)
 	return 0;
 }
 
+static int dw_pcie_ep_get_msix(struct pci_epc *epc, u8 func_no)
+{
+	struct dw_pcie_ep *ep = epc_get_drvdata(epc);
+	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
+	u32 val, reg;
+
+	if (ep->cap_addr.msix_addr == 0)
+		return 0;
+
+	reg = ep->cap_addr.msix_addr + PCI_MSIX_FLAGS;
+	val = dw_pcie_readw_dbi(pci, reg);
+	if (!(val & PCI_MSIX_FLAGS_ENABLE))
+		return -EINVAL;
+
+	val &= PCI_MSIX_FLAGS_QSIZE;
+
+	return val;
+}
+
+static int dw_pcie_ep_set_msix(struct pci_epc *epc, u8 func_no, u16 interrupts)
+{
+	struct dw_pcie_ep *ep = epc_get_drvdata(epc);
+	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
+	u32 val, reg;
+
+	if (ep->cap_addr.msix_addr == 0)
+		return 0;
+
+	reg = ep->cap_addr.msix_addr + PCI_MSIX_FLAGS;
+	val = dw_pcie_readw_dbi(pci, reg);
+	val &= ~PCI_MSIX_FLAGS_QSIZE;
+	val |= interrupts;
+	dw_pcie_dbi_ro_wr_en(pci);
+	dw_pcie_writew_dbi(pci, reg, val);
+	dw_pcie_dbi_ro_wr_dis(pci);
+
+	return 0;
+}
+
 static int dw_pcie_ep_raise_irq(struct pci_epc *epc, u8 func_no,
-				enum pci_epc_irq_type type, u8 interrupt_num)
+				enum pci_epc_irq_type type, u16 interrupt_num)
 {
 	struct dw_pcie_ep *ep = epc_get_drvdata(epc);
 
@@ -282,6 +359,8 @@ static const struct pci_epc_ops epc_ops = {
 	.unmap_addr		= dw_pcie_ep_unmap_addr,
 	.set_msi		= dw_pcie_ep_set_msi,
 	.get_msi		= dw_pcie_ep_get_msi,
+	.set_msix		= dw_pcie_ep_set_msix,
+	.get_msix		= dw_pcie_ep_get_msix,
 	.raise_irq		= dw_pcie_ep_raise_irq,
 	.start			= dw_pcie_ep_start,
 	.stop			= dw_pcie_ep_stop,
@@ -322,6 +401,60 @@ int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep *ep, u8 func_no,
 	return 0;
 }
 
+int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no,
+			     u16 interrupt_num)
+{
+	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
+	struct pci_epc *epc = ep->epc;
+	u16 tbl_offset, bir;
+	u32 bar_addr_upper, bar_addr_lower;
+	u32 msg_addr_upper, msg_addr_lower;
+	u32 reg, msg_data;
+	u64 tbl_addr, msg_addr, reg_u64;
+	void __iomem *msix_tbl;
+	int ret;
+
+	reg = ep->cap_addr.msix_addr + PCI_MSIX_TABLE;
+	tbl_offset = dw_pcie_readl_dbi(pci, reg);
+	bir = (tbl_offset & PCI_MSIX_TABLE_BIR);
+	tbl_offset &= PCI_MSIX_TABLE_OFFSET;
+	tbl_offset >>= 3;
+
+	reg = PCI_BASE_ADDRESS_0 + (4 * bir);
+	bar_addr_lower = dw_pcie_readl_dbi(pci, reg);
+	reg_u64 = (bar_addr_lower & PCI_BASE_ADDRESS_MEM_TYPE_MASK);
+	if (reg_u64 == PCI_BASE_ADDRESS_MEM_TYPE_64)
+		bar_addr_upper = dw_pcie_readl_dbi(pci, reg + 4);
+	else
+		bar_addr_upper = 0;
+
+	tbl_addr = ((u64) bar_addr_upper) << 32 | bar_addr_lower;
+	tbl_addr += (tbl_offset + ((interrupt_num - 1) * PCI_MSIX_ENTRY_SIZE));
+	tbl_addr &= PCI_BASE_ADDRESS_MEM_MASK;
+
+	msix_tbl = ioremap_nocache(ep->phys_base + tbl_addr, ep->addr_size);
+	if (!msix_tbl)
+		return -EINVAL;
+
+	msg_addr_lower = readl(msix_tbl + PCI_MSIX_ENTRY_LOWER_ADDR);
+	msg_addr_upper = readl(msix_tbl + PCI_MSIX_ENTRY_UPPER_ADDR);
+	msg_addr = ((u64) msg_addr_upper) << 32 | msg_addr_lower;
+	msg_data = readl(msix_tbl + PCI_MSIX_ENTRY_DATA);
+
+	iounmap(msix_tbl);
+
+	ret = dw_pcie_ep_map_addr(epc, func_no, ep->msix_mem_phys, msg_addr,
+				  epc->mem->page_size);
+	if (ret)
+		return ret;
+
+	writel(msg_data, ep->msix_mem);
+
+	dw_pcie_ep_unmap_addr(epc, func_no, ep->msix_mem_phys);
+
+	return 0;
+}
+
 void dw_pcie_ep_exit(struct dw_pcie_ep *ep)
 {
 	struct pci_epc *epc = ep->epc;
@@ -329,6 +462,9 @@ void dw_pcie_ep_exit(struct dw_pcie_ep *ep)
 	pci_epc_mem_free_addr(epc, ep->msi_mem_phys, ep->msi_mem,
 			      epc->mem->page_size);
 
+	pci_epc_mem_free_addr(epc, ep->msix_mem_phys, ep->msix_mem,
+			      epc->mem->page_size);
+
 	pci_epc_mem_exit(epc);
 }
 
@@ -411,6 +547,13 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
 		return -ENOMEM;
 	}
 
+	ep->msix_mem = pci_epc_mem_alloc_addr(epc, &ep->msix_mem_phys,
+					     epc->mem->page_size);
+	if (!ep->msix_mem) {
+		dev_err(dev, "Failed to reserve memory for MSI-\n");
+		return -ENOMEM;
+	}
+
 	ep->epc = epc;
 	epc_set_drvdata(epc, ep);
 	dw_pcie_setup(pci);
diff --git a/drivers/pci/dwc/pcie-designware-plat.c b/drivers/pci/dwc/pcie-designware-plat.c
index 2bad68d..c3a4707 100644
--- a/drivers/pci/dwc/pcie-designware-plat.c
+++ b/drivers/pci/dwc/pcie-designware-plat.c
@@ -74,11 +74,13 @@ static void dw_plat_pcie_ep_init(struct dw_pcie_ep *ep)
 
 	for (bar = BAR_0; bar <= BAR_5; bar++)
 		dw_pcie_ep_reset_bar(pci, bar);
+
+	dw_pcie_ep_find_cap_addr(ep);
 }
 
 static int dw_plat_pcie_ep_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
 				     enum pci_epc_irq_type type,
-				     u8 interrupt_num)
+				     u16 interrupt_num)
 {
 	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
 
@@ -88,6 +90,8 @@ static int dw_plat_pcie_ep_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
 		return -EINVAL;
 	case PCI_EPC_IRQ_MSI:
 		return dw_pcie_ep_raise_msi_irq(ep, func_no, interrupt_num);
+	case PCI_EPC_IRQ_MSIX:
+		return dw_pcie_ep_raise_msix_irq(ep, func_no, interrupt_num);
 	default:
 		dev_err(pci->dev, "UNKNOWN IRQ type\n");
 	}
diff --git a/drivers/pci/dwc/pcie-designware.h b/drivers/pci/dwc/pcie-designware.h
index bee4e25..456fd94 100644
--- a/drivers/pci/dwc/pcie-designware.h
+++ b/drivers/pci/dwc/pcie-designware.h
@@ -191,7 +191,12 @@ enum dw_pcie_as_type {
 struct dw_pcie_ep_ops {
 	void	(*ep_init)(struct dw_pcie_ep *ep);
 	int	(*raise_irq)(struct dw_pcie_ep *ep, u8 func_no,
-			     enum pci_epc_irq_type type, u8 interrupt_num);
+			     enum pci_epc_irq_type type, u16 interrupt_num);
+};
+
+struct dw_pcie_cap_addr {
+	u8	msi_addr;
+	u8	msix_addr;
 };
 
 struct dw_pcie_ep {
@@ -208,6 +213,9 @@ struct dw_pcie_ep {
 	u32			num_ob_windows;
 	void __iomem		*msi_mem;
 	phys_addr_t		msi_mem_phys;
+	void __iomem		*msix_mem;
+	phys_addr_t		msix_mem_phys;
+	struct dw_pcie_cap_addr	cap_addr;
 };
 
 struct dw_pcie_ops {
@@ -359,7 +367,10 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep);
 void dw_pcie_ep_exit(struct dw_pcie_ep *ep);
 int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep *ep, u8 func_no,
 			     u8 interrupt_num);
+int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no,
+			     u16 interrupt_num);
 void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar);
+void dw_pcie_ep_find_cap_addr(struct dw_pcie_ep *ep);
 #else
 static inline void dw_pcie_ep_linkup(struct dw_pcie_ep *ep)
 {
@@ -380,8 +391,18 @@ static inline int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep *ep, u8 func_no,
 	return 0;
 }
 
+static inline int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no,
+					   u16 interrupt_num)
+{
+	return 0;
+}
+
 static inline void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar)
 {
 }
+
+static inline void dw_pcie_ep_find_cap_addr(struct dw_pcie_ep *ep)
+{
+}
 #endif
 #endif /* _PCIE_DESIGNWARE_H */
-- 
2.7.4

Re: [RFC 01/10] PCI: dwc: Add MSI-X callbacks handler

[Kishon Vijay Abraham I]

Hi Gustavo,

On Tuesday 10 April 2018 10:44 PM, Gustavo Pimentel wrote:
> Changes the pcie_raise_irq function signature, namely the interrupt_num
> variable type from u8 to u16 to accommodate the MSI-X maximum interrupts
> of 2048.
> 
> Implements a PCIe config space capability iterator function to search and
> save the MSI and MSI-X pointers. With this method the code becomes more
> generic and flexible.
> 
> Implements MSI-X set/get functions for sysfs interface in order to change
> the EP entries number.
> 
> Implements EP MSI-X interface for triggering interruptions.
> 
> Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
> ---
>  drivers/pci/dwc/pci-dra7xx.c           |   2 +-
>  drivers/pci/dwc/pcie-artpec6.c         |   2 +-
>  drivers/pci/dwc/pcie-designware-ep.c   | 145 ++++++++++++++++++++++++++++++++-
>  drivers/pci/dwc/pcie-designware-plat.c |   6 +-
>  drivers/pci/dwc/pcie-designware.h      |  23 +++++-
>  5 files changed, 173 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/pci/dwc/pci-dra7xx.c b/drivers/pci/dwc/pci-dra7xx.c
> index ed8558d..5265725 100644
> --- a/drivers/pci/dwc/pci-dra7xx.c
> +++ b/drivers/pci/dwc/pci-dra7xx.c
> @@ -369,7 +369,7 @@ static void dra7xx_pcie_raise_msi_irq(struct dra7xx_pcie *dra7xx,
>  }
>  
>  static int dra7xx_pcie_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
> -				 enum pci_epc_irq_type type, u8 interrupt_num)
> +				 enum pci_epc_irq_type type, u16 interrupt_num)
>  {
>  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>  	struct dra7xx_pcie *dra7xx = to_dra7xx_pcie(pci);
> diff --git a/drivers/pci/dwc/pcie-artpec6.c b/drivers/pci/dwc/pcie-artpec6.c
> index e66cede..96dc259 100644
> --- a/drivers/pci/dwc/pcie-artpec6.c
> +++ b/drivers/pci/dwc/pcie-artpec6.c
> @@ -428,7 +428,7 @@ static void artpec6_pcie_ep_init(struct dw_pcie_ep *ep)
>  }
>  
>  static int artpec6_pcie_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
> -				  enum pci_epc_irq_type type, u8 interrupt_num)
> +				  enum pci_epc_irq_type type, u16 interrupt_num)
>  {
>  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>  
> diff --git a/drivers/pci/dwc/pcie-designware-ep.c b/drivers/pci/dwc/pcie-designware-ep.c
> index 15b22a6..874d4c2 100644
> --- a/drivers/pci/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/dwc/pcie-designware-ep.c
> @@ -40,6 +40,44 @@ void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar)
>  	__dw_pcie_ep_reset_bar(pci, bar, 0);
>  }
>  
> +void dw_pcie_ep_find_cap_addr(struct dw_pcie_ep *ep)
> +{

This should be implemented in a generic way similar to pci_find_capability().
It'll be useful when we try to implement other capabilities as well.

Thanks
Kishon

Re: [RFC 01/10] PCI: dwc: Add MSI-X callbacks handler

[Gustavo Pimentel]

Hi Kishon,

On 16/04/2018 10:29, Kishon Vijay Abraham I wrote:
> Hi Gustavo,
> 
> On Tuesday 10 April 2018 10:44 PM, Gustavo Pimentel wrote:
>> Changes the pcie_raise_irq function signature, namely the interrupt_num
>> variable type from u8 to u16 to accommodate the MSI-X maximum interrupts
>> of 2048.
>>
>> Implements a PCIe config space capability iterator function to search and
>> save the MSI and MSI-X pointers. With this method the code becomes more
>> generic and flexible.
>>
>> Implements MSI-X set/get functions for sysfs interface in order to change
>> the EP entries number.
>>
>> Implements EP MSI-X interface for triggering interruptions.
>>
>> Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
>> ---
>>  drivers/pci/dwc/pci-dra7xx.c           |   2 +-
>>  drivers/pci/dwc/pcie-artpec6.c         |   2 +-
>>  drivers/pci/dwc/pcie-designware-ep.c   | 145 ++++++++++++++++++++++++++++++++-
>>  drivers/pci/dwc/pcie-designware-plat.c |   6 +-
>>  drivers/pci/dwc/pcie-designware.h      |  23 +++++-
>>  5 files changed, 173 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/pci/dwc/pci-dra7xx.c b/drivers/pci/dwc/pci-dra7xx.c
>> index ed8558d..5265725 100644
>> --- a/drivers/pci/dwc/pci-dra7xx.c
>> +++ b/drivers/pci/dwc/pci-dra7xx.c
>> @@ -369,7 +369,7 @@ static void dra7xx_pcie_raise_msi_irq(struct dra7xx_pcie *dra7xx,
>>  }
>>  
>>  static int dra7xx_pcie_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
>> -				 enum pci_epc_irq_type type, u8 interrupt_num)
>> +				 enum pci_epc_irq_type type, u16 interrupt_num)
>>  {
>>  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>>  	struct dra7xx_pcie *dra7xx = to_dra7xx_pcie(pci);
>> diff --git a/drivers/pci/dwc/pcie-artpec6.c b/drivers/pci/dwc/pcie-artpec6.c
>> index e66cede..96dc259 100644
>> --- a/drivers/pci/dwc/pcie-artpec6.c
>> +++ b/drivers/pci/dwc/pcie-artpec6.c
>> @@ -428,7 +428,7 @@ static void artpec6_pcie_ep_init(struct dw_pcie_ep *ep)
>>  }
>>  
>>  static int artpec6_pcie_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
>> -				  enum pci_epc_irq_type type, u8 interrupt_num)
>> +				  enum pci_epc_irq_type type, u16 interrupt_num)
>>  {
>>  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>>  
>> diff --git a/drivers/pci/dwc/pcie-designware-ep.c b/drivers/pci/dwc/pcie-designware-ep.c
>> index 15b22a6..874d4c2 100644
>> --- a/drivers/pci/dwc/pcie-designware-ep.c
>> +++ b/drivers/pci/dwc/pcie-designware-ep.c
>> @@ -40,6 +40,44 @@ void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar)
>>  	__dw_pcie_ep_reset_bar(pci, bar, 0);
>>  }
>>  
>> +void dw_pcie_ep_find_cap_addr(struct dw_pcie_ep *ep)
>> +{
> 
> This should be implemented in a generic way similar to pci_find_capability().
> It'll be useful when we try to implement other capabilities as well.

Hum, what you suggest? Something implemented on the pci-epf-core?

> 
> Thanks
> Kishon
> 

Regards,
Gustavo

Re: [RFC 01/10] PCI: dwc: Add MSI-X callbacks handler

[Kishon Vijay Abraham I]

Hi,

On Monday 23 April 2018 03:06 PM, Gustavo Pimentel wrote:
> Hi Kishon,
> 
> On 16/04/2018 10:29, Kishon Vijay Abraham I wrote:
>> Hi Gustavo,
>>
>> On Tuesday 10 April 2018 10:44 PM, Gustavo Pimentel wrote:
>>> Changes the pcie_raise_irq function signature, namely the interrupt_num
>>> variable type from u8 to u16 to accommodate the MSI-X maximum interrupts
>>> of 2048.
>>>
>>> Implements a PCIe config space capability iterator function to search and
>>> save the MSI and MSI-X pointers. With this method the code becomes more
>>> generic and flexible.
>>>
>>> Implements MSI-X set/get functions for sysfs interface in order to change
>>> the EP entries number.
>>>
>>> Implements EP MSI-X interface for triggering interruptions.
>>>
>>> Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
>>> ---
>>>  drivers/pci/dwc/pci-dra7xx.c           |   2 +-
>>>  drivers/pci/dwc/pcie-artpec6.c         |   2 +-
>>>  drivers/pci/dwc/pcie-designware-ep.c   | 145 ++++++++++++++++++++++++++++++++-
>>>  drivers/pci/dwc/pcie-designware-plat.c |   6 +-
>>>  drivers/pci/dwc/pcie-designware.h      |  23 +++++-
>>>  5 files changed, 173 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/pci/dwc/pci-dra7xx.c b/drivers/pci/dwc/pci-dra7xx.c
>>> index ed8558d..5265725 100644
>>> --- a/drivers/pci/dwc/pci-dra7xx.c
>>> +++ b/drivers/pci/dwc/pci-dra7xx.c
>>> @@ -369,7 +369,7 @@ static void dra7xx_pcie_raise_msi_irq(struct dra7xx_pcie *dra7xx,
>>>  }
>>>  
>>>  static int dra7xx_pcie_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
>>> -				 enum pci_epc_irq_type type, u8 interrupt_num)
>>> +				 enum pci_epc_irq_type type, u16 interrupt_num)
>>>  {
>>>  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>>>  	struct dra7xx_pcie *dra7xx = to_dra7xx_pcie(pci);
>>> diff --git a/drivers/pci/dwc/pcie-artpec6.c b/drivers/pci/dwc/pcie-artpec6.c
>>> index e66cede..96dc259 100644
>>> --- a/drivers/pci/dwc/pcie-artpec6.c
>>> +++ b/drivers/pci/dwc/pcie-artpec6.c
>>> @@ -428,7 +428,7 @@ static void artpec6_pcie_ep_init(struct dw_pcie_ep *ep)
>>>  }
>>>  
>>>  static int artpec6_pcie_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
>>> -				  enum pci_epc_irq_type type, u8 interrupt_num)
>>> +				  enum pci_epc_irq_type type, u16 interrupt_num)
>>>  {
>>>  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>>>  
>>> diff --git a/drivers/pci/dwc/pcie-designware-ep.c b/drivers/pci/dwc/pcie-designware-ep.c
>>> index 15b22a6..874d4c2 100644
>>> --- a/drivers/pci/dwc/pcie-designware-ep.c
>>> +++ b/drivers/pci/dwc/pcie-designware-ep.c
>>> @@ -40,6 +40,44 @@ void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar)
>>>  	__dw_pcie_ep_reset_bar(pci, bar, 0);
>>>  }
>>>  
>>> +void dw_pcie_ep_find_cap_addr(struct dw_pcie_ep *ep)
>>> +{
>>
>> This should be implemented in a generic way similar to pci_find_capability().
>> It'll be useful when we try to implement other capabilities as well.
> 
> Hum, what you suggest? Something implemented on the pci-epf-core?

yeah, Initially thought it could be implemented as a helper function in
pci-epc-core so that both designware and cadence can use it.

But do we really have to find the address like this? since all designware IP's
will have a particular capability at a fixed address offset, why not follow use
existing mechanism in dw_pcie_ep_get_msi?

Or is it possible for a particular capability to have address offsets for
different vendors? How is it for cadence?

Thanks
Kishon

Re: [RFC 01/10] PCI: dwc: Add MSI-X callbacks handler

[Gustavo Pimentel]

Hi Kishon,

On 24/04/2018 08:07, Kishon Vijay Abraham I wrote:
> Hi,
> 
> On Monday 23 April 2018 03:06 PM, Gustavo Pimentel wrote:
>> Hi Kishon,
>>
>> On 16/04/2018 10:29, Kishon Vijay Abraham I wrote:
>>> Hi Gustavo,
>>>
>>> On Tuesday 10 April 2018 10:44 PM, Gustavo Pimentel wrote:
>>>> Changes the pcie_raise_irq function signature, namely the interrupt_num
>>>> variable type from u8 to u16 to accommodate the MSI-X maximum interrupts
>>>> of 2048.
>>>>
>>>> Implements a PCIe config space capability iterator function to search and
>>>> save the MSI and MSI-X pointers. With this method the code becomes more
>>>> generic and flexible.
>>>>
>>>> Implements MSI-X set/get functions for sysfs interface in order to change
>>>> the EP entries number.
>>>>
>>>> Implements EP MSI-X interface for triggering interruptions.
>>>>
>>>> Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
>>>> ---
>>>>  drivers/pci/dwc/pci-dra7xx.c           |   2 +-
>>>>  drivers/pci/dwc/pcie-artpec6.c         |   2 +-
>>>>  drivers/pci/dwc/pcie-designware-ep.c   | 145 ++++++++++++++++++++++++++++++++-
>>>>  drivers/pci/dwc/pcie-designware-plat.c |   6 +-
>>>>  drivers/pci/dwc/pcie-designware.h      |  23 +++++-
>>>>  5 files changed, 173 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/drivers/pci/dwc/pci-dra7xx.c b/drivers/pci/dwc/pci-dra7xx.c
>>>> index ed8558d..5265725 100644
>>>> --- a/drivers/pci/dwc/pci-dra7xx.c
>>>> +++ b/drivers/pci/dwc/pci-dra7xx.c
>>>> @@ -369,7 +369,7 @@ static void dra7xx_pcie_raise_msi_irq(struct dra7xx_pcie *dra7xx,
>>>>  }
>>>>  
>>>>  static int dra7xx_pcie_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
>>>> -				 enum pci_epc_irq_type type, u8 interrupt_num)
>>>> +				 enum pci_epc_irq_type type, u16 interrupt_num)
>>>>  {
>>>>  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>>>>  	struct dra7xx_pcie *dra7xx = to_dra7xx_pcie(pci);
>>>> diff --git a/drivers/pci/dwc/pcie-artpec6.c b/drivers/pci/dwc/pcie-artpec6.c
>>>> index e66cede..96dc259 100644
>>>> --- a/drivers/pci/dwc/pcie-artpec6.c
>>>> +++ b/drivers/pci/dwc/pcie-artpec6.c
>>>> @@ -428,7 +428,7 @@ static void artpec6_pcie_ep_init(struct dw_pcie_ep *ep)
>>>>  }
>>>>  
>>>>  static int artpec6_pcie_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
>>>> -				  enum pci_epc_irq_type type, u8 interrupt_num)
>>>> +				  enum pci_epc_irq_type type, u16 interrupt_num)
>>>>  {
>>>>  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>>>>  
>>>> diff --git a/drivers/pci/dwc/pcie-designware-ep.c b/drivers/pci/dwc/pcie-designware-ep.c
>>>> index 15b22a6..874d4c2 100644
>>>> --- a/drivers/pci/dwc/pcie-designware-ep.c
>>>> +++ b/drivers/pci/dwc/pcie-designware-ep.c
>>>> @@ -40,6 +40,44 @@ void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar)
>>>>  	__dw_pcie_ep_reset_bar(pci, bar, 0);
>>>>  }
>>>>  
>>>> +void dw_pcie_ep_find_cap_addr(struct dw_pcie_ep *ep)
>>>> +{
>>>
>>> This should be implemented in a generic way similar to pci_find_capability().
>>> It'll be useful when we try to implement other capabilities as well.
>>
>> Hum, what you suggest? Something implemented on the pci-epf-core?
> 
> yeah, Initially thought it could be implemented as a helper function in
> pci-epc-core so that both designware and cadence can use it.

That would be nice, however I couldn't find out how to access the config space,
through the pci_epf or pci_epc structs.

So, I reworked the functions like this:

(on pcie-designware-ep.c)

u8 __dw_pcie_ep_find_next_cap(struct dw_pcie *pci, u8 cap_ptr,
                              u8 cap)
{
        u8 cap_id, next_cap_ptr;
        u16 reg;

        reg = dw_pcie_readw_dbi(pci, cap_ptr);
        next_cap_ptr = (reg & 0xff00) >> 8;
        cap_id = (reg & 0x00ff);

        if (!next_cap_ptr || cap_id > PCI_CAP_ID_MAX)
                return 0;

        if (cap_id == cap)
                return cap_ptr;

        return __dw_pcie_ep_find_next_cap(pci, next_cap_ptr, cap);
}

u8 dw_pcie_ep_find_capability(struct dw_pcie *pci, u8 cap)
{
        u8 next_cap_ptr;
        u16 reg;

        reg = dw_pcie_readw_dbi(pci, PCI_CAPABILITY_LIST);
        next_cap_ptr = (reg & 0x00ff);

        if (!next_cap_ptr)
                return 0;

        return __dw_pcie_ep_find_next_cap(pci, next_cap_ptr, cap);
}

int dw_pcie_ep_init(struct dw_pcie_ep *ep)
{
[...]
        ep->msi_cap = dw_pcie_ep_find_capability(pci, PCI_CAP_ID_MSI);
        ep->msix_cap = dw_pcie_ep_find_capability(pci, PCI_CAP_ID_MSIX);
[...]
}

> 
> But do we really have to find the address like this? since all designware IP's
> will have a particular capability at a fixed address offset, why not follow use
> existing mechanism in dw_pcie_ep_get_msi?

The capabilities are not fixed to a specific address offset by default they
assume those values, but they can be easily change at design stage.

> 
> Or is it possible for a particular capability to have address offsets for
> different vendors? How is it for cadence?

Yes, it's possible to have different address offset for different vendors.

> 
> Thanks
> Kishon
> 

Thanks,
Gustavo

Re: [RFC 01/10] PCI: dwc: Add MSI-X callbacks handler

[Kishon Vijay Abraham I]

Hi,

On Tuesday 24 April 2018 03:06 PM, Gustavo Pimentel wrote:
> Hi Kishon,
> 
> On 24/04/2018 08:07, Kishon Vijay Abraham I wrote:
>> Hi,
>>
>> On Monday 23 April 2018 03:06 PM, Gustavo Pimentel wrote:
>>> Hi Kishon,
>>>
>>> On 16/04/2018 10:29, Kishon Vijay Abraham I wrote:
>>>> Hi Gustavo,
>>>>
>>>> On Tuesday 10 April 2018 10:44 PM, Gustavo Pimentel wrote:
>>>>> Changes the pcie_raise_irq function signature, namely the interrupt_num
>>>>> variable type from u8 to u16 to accommodate the MSI-X maximum interrupts
>>>>> of 2048.
>>>>>
>>>>> Implements a PCIe config space capability iterator function to search and
>>>>> save the MSI and MSI-X pointers. With this method the code becomes more
>>>>> generic and flexible.
>>>>>
>>>>> Implements MSI-X set/get functions for sysfs interface in order to change
>>>>> the EP entries number.
>>>>>
>>>>> Implements EP MSI-X interface for triggering interruptions.
>>>>>
>>>>> Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
>>>>> ---
>>>>>  drivers/pci/dwc/pci-dra7xx.c           |   2 +-
>>>>>  drivers/pci/dwc/pcie-artpec6.c         |   2 +-
>>>>>  drivers/pci/dwc/pcie-designware-ep.c   | 145 ++++++++++++++++++++++++++++++++-
>>>>>  drivers/pci/dwc/pcie-designware-plat.c |   6 +-
>>>>>  drivers/pci/dwc/pcie-designware.h      |  23 +++++-
>>>>>  5 files changed, 173 insertions(+), 5 deletions(-)
>>>>>
>>>>> diff --git a/drivers/pci/dwc/pci-dra7xx.c b/drivers/pci/dwc/pci-dra7xx.c
>>>>> index ed8558d..5265725 100644
>>>>> --- a/drivers/pci/dwc/pci-dra7xx.c
>>>>> +++ b/drivers/pci/dwc/pci-dra7xx.c
>>>>> @@ -369,7 +369,7 @@ static void dra7xx_pcie_raise_msi_irq(struct dra7xx_pcie *dra7xx,
>>>>>  }
>>>>>  
>>>>>  static int dra7xx_pcie_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
>>>>> -				 enum pci_epc_irq_type type, u8 interrupt_num)
>>>>> +				 enum pci_epc_irq_type type, u16 interrupt_num)
>>>>>  {
>>>>>  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>>>>>  	struct dra7xx_pcie *dra7xx = to_dra7xx_pcie(pci);
>>>>> diff --git a/drivers/pci/dwc/pcie-artpec6.c b/drivers/pci/dwc/pcie-artpec6.c
>>>>> index e66cede..96dc259 100644
>>>>> --- a/drivers/pci/dwc/pcie-artpec6.c
>>>>> +++ b/drivers/pci/dwc/pcie-artpec6.c
>>>>> @@ -428,7 +428,7 @@ static void artpec6_pcie_ep_init(struct dw_pcie_ep *ep)
>>>>>  }
>>>>>  
>>>>>  static int artpec6_pcie_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
>>>>> -				  enum pci_epc_irq_type type, u8 interrupt_num)
>>>>> +				  enum pci_epc_irq_type type, u16 interrupt_num)
>>>>>  {
>>>>>  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>>>>>  
>>>>> diff --git a/drivers/pci/dwc/pcie-designware-ep.c b/drivers/pci/dwc/pcie-designware-ep.c
>>>>> index 15b22a6..874d4c2 100644
>>>>> --- a/drivers/pci/dwc/pcie-designware-ep.c
>>>>> +++ b/drivers/pci/dwc/pcie-designware-ep.c
>>>>> @@ -40,6 +40,44 @@ void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar)
>>>>>  	__dw_pcie_ep_reset_bar(pci, bar, 0);
>>>>>  }
>>>>>  
>>>>> +void dw_pcie_ep_find_cap_addr(struct dw_pcie_ep *ep)
>>>>> +{
>>>>
>>>> This should be implemented in a generic way similar to pci_find_capability().
>>>> It'll be useful when we try to implement other capabilities as well.
>>>
>>> Hum, what you suggest? Something implemented on the pci-epf-core?
>>
>> yeah, Initially thought it could be implemented as a helper function in
>> pci-epc-core so that both designware and cadence can use it.
> 
> That would be nice, however I couldn't find out how to access the config space,
> through the pci_epf or pci_epc structs.

It's just a helper function so it can directly take the base address of the
configuration space as argument (in our case, it should be dbi_base).

Thanks
Kishon

> 
> So, I reworked the functions like this:
> 
> (on pcie-designware-ep.c)
> 
> u8 __dw_pcie_ep_find_next_cap(struct dw_pcie *pci, u8 cap_ptr,
>                               u8 cap)
> {
>         u8 cap_id, next_cap_ptr;
>         u16 reg;
> 
>         reg = dw_pcie_readw_dbi(pci, cap_ptr);
>         next_cap_ptr = (reg & 0xff00) >> 8;
>         cap_id = (reg & 0x00ff);
> 
>         if (!next_cap_ptr || cap_id > PCI_CAP_ID_MAX)
>                 return 0;
> 
>         if (cap_id == cap)
>                 return cap_ptr;
> 
>         return __dw_pcie_ep_find_next_cap(pci, next_cap_ptr, cap);
> }
> 
> u8 dw_pcie_ep_find_capability(struct dw_pcie *pci, u8 cap)
> {
>         u8 next_cap_ptr;
>         u16 reg;
> 
>         reg = dw_pcie_readw_dbi(pci, PCI_CAPABILITY_LIST);
>         next_cap_ptr = (reg & 0x00ff);
> 
>         if (!next_cap_ptr)
>                 return 0;
> 
>         return __dw_pcie_ep_find_next_cap(pci, next_cap_ptr, cap);
> }
> 
> int dw_pcie_ep_init(struct dw_pcie_ep *ep)
> {
> [...]
>         ep->msi_cap = dw_pcie_ep_find_capability(pci, PCI_CAP_ID_MSI);
>         ep->msix_cap = dw_pcie_ep_find_capability(pci, PCI_CAP_ID_MSIX);
> [...]
> }
> 
>>
>> But do we really have to find the address like this? since all designware IP's
>> will have a particular capability at a fixed address offset, why not follow use
>> existing mechanism in dw_pcie_ep_get_msi?
> 
> The capabilities are not fixed to a specific address offset by default they
> assume those values, but they can be easily change at design stage.
> 
>>
>> Or is it possible for a particular capability to have address offsets for
>> different vendors? How is it for cadence?
> 
> Yes, it's possible to have different address offset for different vendors.
> 
>>
>> Thanks
>> Kishon
>>
> 
> Thanks,
> Gustavo
> 

Re: [RFC 01/10] PCI: dwc: Add MSI-X callbacks handler

[Gustavo Pimentel]

Hi Kishon,

On 24/04/2018 12:24, Kishon Vijay Abraham I wrote:
> Hi,
> 
> On Tuesday 24 April 2018 03:06 PM, Gustavo Pimentel wrote:
>> Hi Kishon,
>>
>> On 24/04/2018 08:07, Kishon Vijay Abraham I wrote:
>>> Hi,
>>>
>>> On Monday 23 April 2018 03:06 PM, Gustavo Pimentel wrote:
>>>> Hi Kishon,
>>>>
>>>> On 16/04/2018 10:29, Kishon Vijay Abraham I wrote:
>>>>> Hi Gustavo,
>>>>>
>>>>> On Tuesday 10 April 2018 10:44 PM, Gustavo Pimentel wrote:
>>>>>> Changes the pcie_raise_irq function signature, namely the interrupt_num
>>>>>> variable type from u8 to u16 to accommodate the MSI-X maximum interrupts
>>>>>> of 2048.
>>>>>>
>>>>>> Implements a PCIe config space capability iterator function to search and
>>>>>> save the MSI and MSI-X pointers. With this method the code becomes more
>>>>>> generic and flexible.
>>>>>>
>>>>>> Implements MSI-X set/get functions for sysfs interface in order to change
>>>>>> the EP entries number.
>>>>>>
>>>>>> Implements EP MSI-X interface for triggering interruptions.
>>>>>>
>>>>>> Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
>>>>>> ---
>>>>>>  drivers/pci/dwc/pci-dra7xx.c           |   2 +-
>>>>>>  drivers/pci/dwc/pcie-artpec6.c         |   2 +-
>>>>>>  drivers/pci/dwc/pcie-designware-ep.c   | 145 ++++++++++++++++++++++++++++++++-
>>>>>>  drivers/pci/dwc/pcie-designware-plat.c |   6 +-
>>>>>>  drivers/pci/dwc/pcie-designware.h      |  23 +++++-
>>>>>>  5 files changed, 173 insertions(+), 5 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/pci/dwc/pci-dra7xx.c b/drivers/pci/dwc/pci-dra7xx.c
>>>>>> index ed8558d..5265725 100644
>>>>>> --- a/drivers/pci/dwc/pci-dra7xx.c
>>>>>> +++ b/drivers/pci/dwc/pci-dra7xx.c
>>>>>> @@ -369,7 +369,7 @@ static void dra7xx_pcie_raise_msi_irq(struct dra7xx_pcie *dra7xx,
>>>>>>  }
>>>>>>  
>>>>>>  static int dra7xx_pcie_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
>>>>>> -				 enum pci_epc_irq_type type, u8 interrupt_num)
>>>>>> +				 enum pci_epc_irq_type type, u16 interrupt_num)
>>>>>>  {
>>>>>>  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>>>>>>  	struct dra7xx_pcie *dra7xx = to_dra7xx_pcie(pci);
>>>>>> diff --git a/drivers/pci/dwc/pcie-artpec6.c b/drivers/pci/dwc/pcie-artpec6.c
>>>>>> index e66cede..96dc259 100644
>>>>>> --- a/drivers/pci/dwc/pcie-artpec6.c
>>>>>> +++ b/drivers/pci/dwc/pcie-artpec6.c
>>>>>> @@ -428,7 +428,7 @@ static void artpec6_pcie_ep_init(struct dw_pcie_ep *ep)
>>>>>>  }
>>>>>>  
>>>>>>  static int artpec6_pcie_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
>>>>>> -				  enum pci_epc_irq_type type, u8 interrupt_num)
>>>>>> +				  enum pci_epc_irq_type type, u16 interrupt_num)
>>>>>>  {
>>>>>>  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>>>>>>  
>>>>>> diff --git a/drivers/pci/dwc/pcie-designware-ep.c b/drivers/pci/dwc/pcie-designware-ep.c
>>>>>> index 15b22a6..874d4c2 100644
>>>>>> --- a/drivers/pci/dwc/pcie-designware-ep.c
>>>>>> +++ b/drivers/pci/dwc/pcie-designware-ep.c
>>>>>> @@ -40,6 +40,44 @@ void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar)
>>>>>>  	__dw_pcie_ep_reset_bar(pci, bar, 0);
>>>>>>  }
>>>>>>  
>>>>>> +void dw_pcie_ep_find_cap_addr(struct dw_pcie_ep *ep)
>>>>>> +{
>>>>>
>>>>> This should be implemented in a generic way similar to pci_find_capability().
>>>>> It'll be useful when we try to implement other capabilities as well.
>>>>
>>>> Hum, what you suggest? Something implemented on the pci-epf-core?
>>>
>>> yeah, Initially thought it could be implemented as a helper function in
>>> pci-epc-core so that both designware and cadence can use it.
>>
>> That would be nice, however I couldn't find out how to access the config space,
>> through the pci_epf or pci_epc structs.
> 
> It's just a helper function so it can directly take the base address of the
> configuration space as argument (in our case, it should be dbi_base).

I don't think it will bring much benefit to this particular scope at this time
being. In any case this could be improved later.

Regards,
Gustavo

> 
> Thanks
> Kishon
> 
>>
>> So, I reworked the functions like this:
>>
>> (on pcie-designware-ep.c)
>>
>> u8 __dw_pcie_ep_find_next_cap(struct dw_pcie *pci, u8 cap_ptr,
>>                               u8 cap)
>> {
>>         u8 cap_id, next_cap_ptr;
>>         u16 reg;
>>
>>         reg = dw_pcie_readw_dbi(pci, cap_ptr);
>>         next_cap_ptr = (reg & 0xff00) >> 8;
>>         cap_id = (reg & 0x00ff);
>>
>>         if (!next_cap_ptr || cap_id > PCI_CAP_ID_MAX)
>>                 return 0;
>>
>>         if (cap_id == cap)
>>                 return cap_ptr;
>>
>>         return __dw_pcie_ep_find_next_cap(pci, next_cap_ptr, cap);
>> }
>>
>> u8 dw_pcie_ep_find_capability(struct dw_pcie *pci, u8 cap)
>> {
>>         u8 next_cap_ptr;
>>         u16 reg;
>>
>>         reg = dw_pcie_readw_dbi(pci, PCI_CAPABILITY_LIST);
>>         next_cap_ptr = (reg & 0x00ff);
>>
>>         if (!next_cap_ptr)
>>                 return 0;
>>
>>         return __dw_pcie_ep_find_next_cap(pci, next_cap_ptr, cap);
>> }
>>
>> int dw_pcie_ep_init(struct dw_pcie_ep *ep)
>> {
>> [...]
>>         ep->msi_cap = dw_pcie_ep_find_capability(pci, PCI_CAP_ID_MSI);
>>         ep->msix_cap = dw_pcie_ep_find_capability(pci, PCI_CAP_ID_MSIX);
>> [...]
>> }
>>
>>>
>>> But do we really have to find the address like this? since all designware IP's
>>> will have a particular capability at a fixed address offset, why not follow use
>>> existing mechanism in dw_pcie_ep_get_msi?
>>
>> The capabilities are not fixed to a specific address offset by default they
>> assume those values, but they can be easily change at design stage.
>>
>>>
>>> Or is it possible for a particular capability to have address offsets for
>>> different vendors? How is it for cadence?
>>
>> Yes, it's possible to have different address offset for different vendors.
>>
>>>
>>> Thanks
>>> Kishon
>>>
>>
>> Thanks,
>> Gustavo
>>

RE: [RFC 01/10] PCI: dwc: Add MSI-X callbacks handler

[Alan Douglas]

Hi Kishon,

On 24 April 2018 10:36 Gustavo Pimentel wrote:
> Hi Kishon,
>=20
> On 24/04/2018 08:07, Kishon Vijay Abraham I wrote:
> > Hi,
> >
> > On Monday 23 April 2018 03:06 PM, Gustavo Pimentel wrote:
> >> Hi Kishon,
> >>
> >> On 16/04/2018 10:29, Kishon Vijay Abraham I wrote:
> >>> Hi Gustavo,
> >>>
> >>> On Tuesday 10 April 2018 10:44 PM, Gustavo Pimentel wrote:
> >>>> Changes the pcie_raise_irq function signature, namely the
> >>>> interrupt_num variable type from u8 to u16 to accommodate the MSI-X
> >>>> maximum interrupts of 2048.
> >>>>
> >>>> Implements a PCIe config space capability iterator function to
> >>>> search and save the MSI and MSI-X pointers. With this method the
> >>>> code becomes more generic and flexible.
> >>>>
> >>>> Implements MSI-X set/get functions for sysfs interface in order to
> >>>> change the EP entries number.
> >>>>
> >>>> Implements EP MSI-X interface for triggering interruptions.
> >>>>
> >>>> Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
> >>>> ---
> >>>>  drivers/pci/dwc/pci-dra7xx.c           |   2 +-
> >>>>  drivers/pci/dwc/pcie-artpec6.c         |   2 +-
> >>>>  drivers/pci/dwc/pcie-designware-ep.c   | 145
> ++++++++++++++++++++++++++++++++-
> >>>>  drivers/pci/dwc/pcie-designware-plat.c |   6 +-
> >>>>  drivers/pci/dwc/pcie-designware.h      |  23 +++++-
> >>>>  5 files changed, 173 insertions(+), 5 deletions(-)
> >>>>
> >>>> diff --git a/drivers/pci/dwc/pci-dra7xx.c
> >>>> b/drivers/pci/dwc/pci-dra7xx.c index ed8558d..5265725 100644
> >>>> --- a/drivers/pci/dwc/pci-dra7xx.c
> >>>> +++ b/drivers/pci/dwc/pci-dra7xx.c
> >>>> @@ -369,7 +369,7 @@ static void dra7xx_pcie_raise_msi_irq(struct
> >>>> dra7xx_pcie *dra7xx,  }
> >>>>
> >>>>  static int dra7xx_pcie_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
> >>>> -				 enum pci_epc_irq_type type, u8
> interrupt_num)
> >>>> +				 enum pci_epc_irq_type type, u16
> interrupt_num)
> >>>>  {
> >>>>  	struct dw_pcie *pci =3D to_dw_pcie_from_ep(ep);
> >>>>  	struct dra7xx_pcie *dra7xx =3D to_dra7xx_pcie(pci); diff --git
> >>>> a/drivers/pci/dwc/pcie-artpec6.c b/drivers/pci/dwc/pcie-artpec6.c
> >>>> index e66cede..96dc259 100644
> >>>> --- a/drivers/pci/dwc/pcie-artpec6.c
> >>>> +++ b/drivers/pci/dwc/pcie-artpec6.c
> >>>> @@ -428,7 +428,7 @@ static void artpec6_pcie_ep_init(struct
> >>>> dw_pcie_ep *ep)  }
> >>>>
> >>>>  static int artpec6_pcie_raise_irq(struct dw_pcie_ep *ep, u8 func_no=
,
> >>>> -				  enum pci_epc_irq_type type, u8
> interrupt_num)
> >>>> +				  enum pci_epc_irq_type type, u16
> interrupt_num)
> >>>>  {
> >>>>  	struct dw_pcie *pci =3D to_dw_pcie_from_ep(ep);
> >>>>
> >>>> diff --git a/drivers/pci/dwc/pcie-designware-ep.c
> >>>> b/drivers/pci/dwc/pcie-designware-ep.c
> >>>> index 15b22a6..874d4c2 100644
> >>>> --- a/drivers/pci/dwc/pcie-designware-ep.c
> >>>> +++ b/drivers/pci/dwc/pcie-designware-ep.c
> >>>> @@ -40,6 +40,44 @@ void dw_pcie_ep_reset_bar(struct dw_pcie
> *pci, enum pci_barno bar)
> >>>>  	__dw_pcie_ep_reset_bar(pci, bar, 0);  }
> >>>>
> >>>> +void dw_pcie_ep_find_cap_addr(struct dw_pcie_ep *ep) {
> >>>
> >>> This should be implemented in a generic way similar to
> pci_find_capability().
> >>> It'll be useful when we try to implement other capabilities as well.
> >>
> >> Hum, what you suggest? Something implemented on the pci-epf-core?
> >
> > yeah, Initially thought it could be implemented as a helper function
> > in pci-epc-core so that both designware and cadence can use it.
>=20
> That would be nice, however I couldn't find out how to access the config
> space, through the pci_epf or pci_epc structs.
>=20
> So, I reworked the functions like this:
>=20
> (on pcie-designware-ep.c)
>=20
> u8 __dw_pcie_ep_find_next_cap(struct dw_pcie *pci, u8 cap_ptr,
>                               u8 cap)
> {
>         u8 cap_id, next_cap_ptr;
>         u16 reg;
>=20
>         reg =3D dw_pcie_readw_dbi(pci, cap_ptr);
>         next_cap_ptr =3D (reg & 0xff00) >> 8;
>         cap_id =3D (reg & 0x00ff);
>=20
>         if (!next_cap_ptr || cap_id > PCI_CAP_ID_MAX)
>                 return 0;
>=20
>         if (cap_id =3D=3D cap)
>                 return cap_ptr;
>=20
>         return __dw_pcie_ep_find_next_cap(pci, next_cap_ptr, cap); }
>=20
> u8 dw_pcie_ep_find_capability(struct dw_pcie *pci, u8 cap) {
>         u8 next_cap_ptr;
>         u16 reg;
>=20
>         reg =3D dw_pcie_readw_dbi(pci, PCI_CAPABILITY_LIST);
>         next_cap_ptr =3D (reg & 0x00ff);
>=20
>         if (!next_cap_ptr)
>                 return 0;
>=20
>         return __dw_pcie_ep_find_next_cap(pci, next_cap_ptr, cap); }
>=20
> int dw_pcie_ep_init(struct dw_pcie_ep *ep) { [...]
>         ep->msi_cap =3D dw_pcie_ep_find_capability(pci, PCI_CAP_ID_MSI);
>         ep->msix_cap =3D dw_pcie_ep_find_capability(pci, PCI_CAP_ID_MSIX)=
; [...]
> }
>=20
> >
> > But do we really have to find the address like this? since all
> > designware IP's will have a particular capability at a fixed address
> > offset, why not follow use existing mechanism in dw_pcie_ep_get_msi?
>=20
> The capabilities are not fixed to a specific address offset by default th=
ey
> assume those values, but they can be easily change at design stage.
>=20
> >
> > Or is it possible for a particular capability to have address offsets
> > for different vendors? How is it for cadence?
>=20
> Yes, it's possible to have different address offset for different vendors=
.
>=20
For cadence the offsets are fixed for each specific hw (if a capability is=
=20
disabled it will just be removed from the linked list) but it would be=20
better to allow for the possibility for them to vary between different hw.

> >
> > Thanks
> > Kishon
> >
>=20
> Thanks,
> Gustavo
Regards,
Alan

RE: [RFC 01/10] PCI: dwc: Add MSI-X callbacks handler

[Alan Douglas]

Hi,

On 10 April 2018 18:15 Gustavo Pimentel wrote:
> Changes the pcie_raise_irq function signature, namely the interrupt_num
> variable type from u8 to u16 to accommodate the MSI-X maximum interrupts
> of 2048.
>=20
> Implements a PCIe config space capability iterator function to search and=
 save
> the MSI and MSI-X pointers. With this method the code becomes more
> generic and flexible.
>=20
> Implements MSI-X set/get functions for sysfs interface in order to change=
 the
> EP entries number.
>=20
> Implements EP MSI-X interface for triggering interruptions.
>=20
> Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
> ---
>  drivers/pci/dwc/pci-dra7xx.c           |   2 +-
>  drivers/pci/dwc/pcie-artpec6.c         |   2 +-
>  drivers/pci/dwc/pcie-designware-ep.c   | 145
> ++++++++++++++++++++++++++++++++-
>  drivers/pci/dwc/pcie-designware-plat.c |   6 +-
>  drivers/pci/dwc/pcie-designware.h      |  23 +++++-
>  5 files changed, 173 insertions(+), 5 deletions(-)
>=20
> diff --git a/drivers/pci/dwc/pci-dra7xx.c b/drivers/pci/dwc/pci-dra7xx.c =
index
> ed8558d..5265725 100644
> --- a/drivers/pci/dwc/pci-dra7xx.c
> +++ b/drivers/pci/dwc/pci-dra7xx.c
> @@ -369,7 +369,7 @@ static void dra7xx_pcie_raise_msi_irq(struct
> dra7xx_pcie *dra7xx,  }
>=20
>  static int dra7xx_pcie_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
> -				 enum pci_epc_irq_type type, u8
> interrupt_num)
> +				 enum pci_epc_irq_type type, u16
> interrupt_num)
>  {
>  	struct dw_pcie *pci =3D to_dw_pcie_from_ep(ep);
>  	struct dra7xx_pcie *dra7xx =3D to_dra7xx_pcie(pci); diff --git
> a/drivers/pci/dwc/pcie-artpec6.c b/drivers/pci/dwc/pcie-artpec6.c index
> e66cede..96dc259 100644
> --- a/drivers/pci/dwc/pcie-artpec6.c
> +++ b/drivers/pci/dwc/pcie-artpec6.c
> @@ -428,7 +428,7 @@ static void artpec6_pcie_ep_init(struct dw_pcie_ep
> *ep)  }
>=20
>  static int artpec6_pcie_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
> -				  enum pci_epc_irq_type type, u8
> interrupt_num)
> +				  enum pci_epc_irq_type type, u16
> interrupt_num)
>  {
>  	struct dw_pcie *pci =3D to_dw_pcie_from_ep(ep);
>=20
> diff --git a/drivers/pci/dwc/pcie-designware-ep.c b/drivers/pci/dwc/pcie-
> designware-ep.c
> index 15b22a6..874d4c2 100644
> --- a/drivers/pci/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/dwc/pcie-designware-ep.c
> @@ -40,6 +40,44 @@ void dw_pcie_ep_reset_bar(struct dw_pcie *pci,
> enum pci_barno bar)
>  	__dw_pcie_ep_reset_bar(pci, bar, 0);
>  }
>=20
> +void dw_pcie_ep_find_cap_addr(struct dw_pcie_ep *ep) {
> +	struct dw_pcie *pci =3D to_dw_pcie_from_ep(ep);
> +	u8 next_ptr, curr_ptr, cap_id;
> +	u16 reg;
> +
> +	memset(&ep->cap_addr, 0, sizeof(ep->cap_addr));
> +
> +	reg =3D dw_pcie_readw_dbi(pci, PCI_STATUS);
> +	if (!(reg & PCI_STATUS_CAP_LIST))
> +		return;
> +
> +	reg =3D dw_pcie_readw_dbi(pci, PCI_CAPABILITY_LIST);
> +	next_ptr =3D (reg & 0x00ff);
> +	if (!next_ptr)
> +		return;
> +
> +	reg =3D dw_pcie_readw_dbi(pci, next_ptr);
> +	curr_ptr =3D next_ptr;
> +	next_ptr =3D (reg & 0xff00) >> 8;
> +	cap_id =3D (reg & 0x00ff);
> +
> +	while (next_ptr && (cap_id <=3D PCI_CAP_ID_MAX)) {
> +		switch (cap_id) {
> +		case PCI_CAP_ID_MSI:
> +			ep->cap_addr.msi_addr =3D curr_ptr;
> +			break;
> +		case PCI_CAP_ID_MSIX:
> +			ep->cap_addr.msix_addr =3D curr_ptr;
> +			break;
> +		}
> +		reg =3D dw_pcie_readw_dbi(pci, next_ptr);
> +		curr_ptr =3D next_ptr;
> +		next_ptr =3D (reg & 0xff00) >> 8;
> +		cap_id =3D (reg & 0x00ff);
> +	}
> +}
> +
>  static int dw_pcie_ep_write_header(struct pci_epc *epc, u8 func_no,
>  				   struct pci_epf_header *hdr)
>  {
> @@ -241,8 +279,47 @@ static int dw_pcie_ep_set_msi(struct pci_epc *epc,
> u8 func_no, u8 encode_int)
>  	return 0;
>  }
>=20
> +static int dw_pcie_ep_get_msix(struct pci_epc *epc, u8 func_no) {
> +	struct dw_pcie_ep *ep =3D epc_get_drvdata(epc);
> +	struct dw_pcie *pci =3D to_dw_pcie_from_ep(ep);
> +	u32 val, reg;
> +
> +	if (ep->cap_addr.msix_addr =3D=3D 0)
> +		return 0;
> +
> +	reg =3D ep->cap_addr.msix_addr + PCI_MSIX_FLAGS;
> +	val =3D dw_pcie_readw_dbi(pci, reg);
> +	if (!(val & PCI_MSIX_FLAGS_ENABLE))
> +		return -EINVAL;
> +
> +	val &=3D PCI_MSIX_FLAGS_QSIZE;
> +
> +	return val;
> +}
> +
> +static int dw_pcie_ep_set_msix(struct pci_epc *epc, u8 func_no, u16
> +interrupts) {
> +	struct dw_pcie_ep *ep =3D epc_get_drvdata(epc);
> +	struct dw_pcie *pci =3D to_dw_pcie_from_ep(ep);
> +	u32 val, reg;
> +
> +	if (ep->cap_addr.msix_addr =3D=3D 0)
> +		return 0;
> +
> +	reg =3D ep->cap_addr.msix_addr + PCI_MSIX_FLAGS;
> +	val =3D dw_pcie_readw_dbi(pci, reg);
> +	val &=3D ~PCI_MSIX_FLAGS_QSIZE;
> +	val |=3D interrupts;
> +	dw_pcie_dbi_ro_wr_en(pci);
> +	dw_pcie_writew_dbi(pci, reg, val);
> +	dw_pcie_dbi_ro_wr_dis(pci);
> +
> +	return 0;
> +}
> +
>  static int dw_pcie_ep_raise_irq(struct pci_epc *epc, u8 func_no,
> -				enum pci_epc_irq_type type, u8
> interrupt_num)
> +				enum pci_epc_irq_type type, u16
> interrupt_num)
>  {
>  	struct dw_pcie_ep *ep =3D epc_get_drvdata(epc);
>=20
> @@ -282,6 +359,8 @@ static const struct pci_epc_ops epc_ops =3D {
>  	.unmap_addr		=3D dw_pcie_ep_unmap_addr,
>  	.set_msi		=3D dw_pcie_ep_set_msi,
>  	.get_msi		=3D dw_pcie_ep_get_msi,
> +	.set_msix		=3D dw_pcie_ep_set_msix,
> +	.get_msix		=3D dw_pcie_ep_get_msix,
>  	.raise_irq		=3D dw_pcie_ep_raise_irq,
>  	.start			=3D dw_pcie_ep_start,
>  	.stop			=3D dw_pcie_ep_stop,
> @@ -322,6 +401,60 @@ int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep
> *ep, u8 func_no,
>  	return 0;
>  }
>=20
> +int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no,
> +			     u16 interrupt_num)
> +{
> +	struct dw_pcie *pci =3D to_dw_pcie_from_ep(ep);
> +	struct pci_epc *epc =3D ep->epc;
> +	u16 tbl_offset, bir;
> +	u32 bar_addr_upper, bar_addr_lower;
> +	u32 msg_addr_upper, msg_addr_lower;
> +	u32 reg, msg_data;
> +	u64 tbl_addr, msg_addr, reg_u64;
> +	void __iomem *msix_tbl;
> +	int ret;
> +
> +	reg =3D ep->cap_addr.msix_addr + PCI_MSIX_TABLE;
> +	tbl_offset =3D dw_pcie_readl_dbi(pci, reg);
> +	bir =3D (tbl_offset & PCI_MSIX_TABLE_BIR);
> +	tbl_offset &=3D PCI_MSIX_TABLE_OFFSET;
> +	tbl_offset >>=3D 3;
> +
> +	reg =3D PCI_BASE_ADDRESS_0 + (4 * bir);
> +	bar_addr_lower =3D dw_pcie_readl_dbi(pci, reg);
> +	reg_u64 =3D (bar_addr_lower & PCI_BASE_ADDRESS_MEM_TYPE_MASK);
> +	if (reg_u64 =3D=3D PCI_BASE_ADDRESS_MEM_TYPE_64)
> +		bar_addr_upper =3D dw_pcie_readl_dbi(pci, reg + 4);
> +	else
> +		bar_addr_upper =3D 0;
> +
> +	tbl_addr =3D ((u64) bar_addr_upper) << 32 | bar_addr_lower;
> +	tbl_addr +=3D (tbl_offset + ((interrupt_num - 1) * PCI_MSIX_ENTRY_SIZE)=
);
> +	tbl_addr &=3D PCI_BASE_ADDRESS_MEM_MASK;
> +
> +	msix_tbl =3D ioremap_nocache(ep->phys_base + tbl_addr, ep->addr_size);
> +	if (!msix_tbl)
> +		return -EINVAL;
> +
I think you need to check the mask bit in  vector control for the requested=
 IRQ.
You could set the pending bit if masked, but would then need some output=20
signal to inform when the mask bit is cleared (or poll it) so the message c=
an be sent
later.

Also, do you need to check PCI_MSIX_FLAGS_ENABLE here as well, or is it che=
cked earlier?

Regards,
Alan

Re: [RFC 01/10] PCI: dwc: Add MSI-X callbacks handler

[Gustavo Pimentel]

On 24/04/2018 10:15, Alan Douglas wrote:
> Hi,
> 
> On 10 April 2018 18:15 Gustavo Pimentel wrote:
>> Changes the pcie_raise_irq function signature, namely the interrupt_num
>> variable type from u8 to u16 to accommodate the MSI-X maximum interrupts
>> of 2048.
>>
>> Implements a PCIe config space capability iterator function to search and save
>> the MSI and MSI-X pointers. With this method the code becomes more
>> generic and flexible.
>>
>> Implements MSI-X set/get functions for sysfs interface in order to change the
>> EP entries number.
>>
>> Implements EP MSI-X interface for triggering interruptions.
>>
>> Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
>> ---
>>  drivers/pci/dwc/pci-dra7xx.c           |   2 +-
>>  drivers/pci/dwc/pcie-artpec6.c         |   2 +-
>>  drivers/pci/dwc/pcie-designware-ep.c   | 145
>> ++++++++++++++++++++++++++++++++-
>>  drivers/pci/dwc/pcie-designware-plat.c |   6 +-
>>  drivers/pci/dwc/pcie-designware.h      |  23 +++++-
>>  5 files changed, 173 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/pci/dwc/pci-dra7xx.c b/drivers/pci/dwc/pci-dra7xx.c index
>> ed8558d..5265725 100644
>> --- a/drivers/pci/dwc/pci-dra7xx.c
>> +++ b/drivers/pci/dwc/pci-dra7xx.c
>> @@ -369,7 +369,7 @@ static void dra7xx_pcie_raise_msi_irq(struct
>> dra7xx_pcie *dra7xx,  }
>>
>>  static int dra7xx_pcie_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
>> -				 enum pci_epc_irq_type type, u8
>> interrupt_num)
>> +				 enum pci_epc_irq_type type, u16
>> interrupt_num)
>>  {
>>  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>>  	struct dra7xx_pcie *dra7xx = to_dra7xx_pcie(pci); diff --git
>> a/drivers/pci/dwc/pcie-artpec6.c b/drivers/pci/dwc/pcie-artpec6.c index
>> e66cede..96dc259 100644
>> --- a/drivers/pci/dwc/pcie-artpec6.c
>> +++ b/drivers/pci/dwc/pcie-artpec6.c
>> @@ -428,7 +428,7 @@ static void artpec6_pcie_ep_init(struct dw_pcie_ep
>> *ep)  }
>>
>>  static int artpec6_pcie_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
>> -				  enum pci_epc_irq_type type, u8
>> interrupt_num)
>> +				  enum pci_epc_irq_type type, u16
>> interrupt_num)
>>  {
>>  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>>
>> diff --git a/drivers/pci/dwc/pcie-designware-ep.c b/drivers/pci/dwc/pcie-
>> designware-ep.c
>> index 15b22a6..874d4c2 100644
>> --- a/drivers/pci/dwc/pcie-designware-ep.c
>> +++ b/drivers/pci/dwc/pcie-designware-ep.c
>> @@ -40,6 +40,44 @@ void dw_pcie_ep_reset_bar(struct dw_pcie *pci,
>> enum pci_barno bar)
>>  	__dw_pcie_ep_reset_bar(pci, bar, 0);
>>  }
>>
>> +void dw_pcie_ep_find_cap_addr(struct dw_pcie_ep *ep) {
>> +	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>> +	u8 next_ptr, curr_ptr, cap_id;
>> +	u16 reg;
>> +
>> +	memset(&ep->cap_addr, 0, sizeof(ep->cap_addr));
>> +
>> +	reg = dw_pcie_readw_dbi(pci, PCI_STATUS);
>> +	if (!(reg & PCI_STATUS_CAP_LIST))
>> +		return;
>> +
>> +	reg = dw_pcie_readw_dbi(pci, PCI_CAPABILITY_LIST);
>> +	next_ptr = (reg & 0x00ff);
>> +	if (!next_ptr)
>> +		return;
>> +
>> +	reg = dw_pcie_readw_dbi(pci, next_ptr);
>> +	curr_ptr = next_ptr;
>> +	next_ptr = (reg & 0xff00) >> 8;
>> +	cap_id = (reg & 0x00ff);
>> +
>> +	while (next_ptr && (cap_id <= PCI_CAP_ID_MAX)) {
>> +		switch (cap_id) {
>> +		case PCI_CAP_ID_MSI:
>> +			ep->cap_addr.msi_addr = curr_ptr;
>> +			break;
>> +		case PCI_CAP_ID_MSIX:
>> +			ep->cap_addr.msix_addr = curr_ptr;
>> +			break;
>> +		}
>> +		reg = dw_pcie_readw_dbi(pci, next_ptr);
>> +		curr_ptr = next_ptr;
>> +		next_ptr = (reg & 0xff00) >> 8;
>> +		cap_id = (reg & 0x00ff);
>> +	}
>> +}
>> +
>>  static int dw_pcie_ep_write_header(struct pci_epc *epc, u8 func_no,
>>  				   struct pci_epf_header *hdr)
>>  {
>> @@ -241,8 +279,47 @@ static int dw_pcie_ep_set_msi(struct pci_epc *epc,
>> u8 func_no, u8 encode_int)
>>  	return 0;
>>  }
>>
>> +static int dw_pcie_ep_get_msix(struct pci_epc *epc, u8 func_no) {
>> +	struct dw_pcie_ep *ep = epc_get_drvdata(epc);
>> +	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>> +	u32 val, reg;
>> +
>> +	if (ep->cap_addr.msix_addr == 0)
>> +		return 0;
>> +
>> +	reg = ep->cap_addr.msix_addr + PCI_MSIX_FLAGS;
>> +	val = dw_pcie_readw_dbi(pci, reg);
>> +	if (!(val & PCI_MSIX_FLAGS_ENABLE))
>> +		return -EINVAL;
>> +
>> +	val &= PCI_MSIX_FLAGS_QSIZE;
>> +
>> +	return val;
>> +}
>> +
>> +static int dw_pcie_ep_set_msix(struct pci_epc *epc, u8 func_no, u16
>> +interrupts) {
>> +	struct dw_pcie_ep *ep = epc_get_drvdata(epc);
>> +	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>> +	u32 val, reg;
>> +
>> +	if (ep->cap_addr.msix_addr == 0)
>> +		return 0;
>> +
>> +	reg = ep->cap_addr.msix_addr + PCI_MSIX_FLAGS;
>> +	val = dw_pcie_readw_dbi(pci, reg);
>> +	val &= ~PCI_MSIX_FLAGS_QSIZE;
>> +	val |= interrupts;
>> +	dw_pcie_dbi_ro_wr_en(pci);
>> +	dw_pcie_writew_dbi(pci, reg, val);
>> +	dw_pcie_dbi_ro_wr_dis(pci);
>> +
>> +	return 0;
>> +}
>> +
>>  static int dw_pcie_ep_raise_irq(struct pci_epc *epc, u8 func_no,
>> -				enum pci_epc_irq_type type, u8
>> interrupt_num)
>> +				enum pci_epc_irq_type type, u16
>> interrupt_num)
>>  {
>>  	struct dw_pcie_ep *ep = epc_get_drvdata(epc);
>>
>> @@ -282,6 +359,8 @@ static const struct pci_epc_ops epc_ops = {
>>  	.unmap_addr		= dw_pcie_ep_unmap_addr,
>>  	.set_msi		= dw_pcie_ep_set_msi,
>>  	.get_msi		= dw_pcie_ep_get_msi,
>> +	.set_msix		= dw_pcie_ep_set_msix,
>> +	.get_msix		= dw_pcie_ep_get_msix,
>>  	.raise_irq		= dw_pcie_ep_raise_irq,
>>  	.start			= dw_pcie_ep_start,
>>  	.stop			= dw_pcie_ep_stop,
>> @@ -322,6 +401,60 @@ int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep
>> *ep, u8 func_no,
>>  	return 0;
>>  }
>>
>> +int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no,
>> +			     u16 interrupt_num)
>> +{
>> +	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>> +	struct pci_epc *epc = ep->epc;
>> +	u16 tbl_offset, bir;
>> +	u32 bar_addr_upper, bar_addr_lower;
>> +	u32 msg_addr_upper, msg_addr_lower;
>> +	u32 reg, msg_data;
>> +	u64 tbl_addr, msg_addr, reg_u64;
>> +	void __iomem *msix_tbl;
>> +	int ret;
>> +
>> +	reg = ep->cap_addr.msix_addr + PCI_MSIX_TABLE;
>> +	tbl_offset = dw_pcie_readl_dbi(pci, reg);
>> +	bir = (tbl_offset & PCI_MSIX_TABLE_BIR);
>> +	tbl_offset &= PCI_MSIX_TABLE_OFFSET;
>> +	tbl_offset >>= 3;
>> +
>> +	reg = PCI_BASE_ADDRESS_0 + (4 * bir);
>> +	bar_addr_lower = dw_pcie_readl_dbi(pci, reg);
>> +	reg_u64 = (bar_addr_lower & PCI_BASE_ADDRESS_MEM_TYPE_MASK);
>> +	if (reg_u64 == PCI_BASE_ADDRESS_MEM_TYPE_64)
>> +		bar_addr_upper = dw_pcie_readl_dbi(pci, reg + 4);
>> +	else
>> +		bar_addr_upper = 0;
>> +
>> +	tbl_addr = ((u64) bar_addr_upper) << 32 | bar_addr_lower;
>> +	tbl_addr += (tbl_offset + ((interrupt_num - 1) * PCI_MSIX_ENTRY_SIZE));
>> +	tbl_addr &= PCI_BASE_ADDRESS_MEM_MASK;
>> +
>> +	msix_tbl = ioremap_nocache(ep->phys_base + tbl_addr, ep->addr_size);
>> +	if (!msix_tbl)
>> +		return -EINVAL;
>> +
> I think you need to check the mask bit in  vector control for the requested IRQ.
> You could set the pending bit if masked, but would then need some output 
> signal to inform when the mask bit is cleared (or poll it) so the message can be sent
> later.

I have to analyze this careful and with detail to provide you a valid and good
feedback as soon as possible.

> 
> Also, do you need to check PCI_MSIX_FLAGS_ENABLE here as well, or is it checked earlier?

I applied the same logic for MSI-X already implemented before for the MSI.

Before calling the pci_epc_raise_irq(), the number of interrupts is checked by
calling the pci_epc_get_msi() first, which leads to one of
dw_pcie_ep_get_msi()/cdns_pcie_ep_get_msi().

>From what I could see the PCI_MSI_FLAGS_ENABLE/PCI_MSIX_FLAGS_ENABLE flags are
check on the dw_pcie_ep_get_msi()/dw_pcie_ep_get_msix() funcitons, the same
logic is applied on cdns_pcie_ep_get_msi().

> 
> Regards,
> Alan
> 

Re: [RFC 01/10] PCI: dwc: Add MSI-X callbacks handler

[Gustavo Pimentel]

Hi Alan,

Sorry for the delay on the response, I only have time to proper analyze this now.

On 24/04/2018 10:15, Alan Douglas wrote:
> Hi,
> 
> On 10 April 2018 18:15 Gustavo Pimentel wrote:
>> Changes the pcie_raise_irq function signature, namely the interrupt_num
>> variable type from u8 to u16 to accommodate the MSI-X maximum interrupts
>> of 2048.
>>
>> Implements a PCIe config space capability iterator function to search and save
>> the MSI and MSI-X pointers. With this method the code becomes more
>> generic and flexible.
>>
>> Implements MSI-X set/get functions for sysfs interface in order to change the
>> EP entries number.
>>
>> Implements EP MSI-X interface for triggering interruptions.
>>
>> Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
>> ---
>>  drivers/pci/dwc/pci-dra7xx.c           |   2 +-
>>  drivers/pci/dwc/pcie-artpec6.c         |   2 +-
>>  drivers/pci/dwc/pcie-designware-ep.c   | 145
>> ++++++++++++++++++++++++++++++++-
>>  drivers/pci/dwc/pcie-designware-plat.c |   6 +-
>>  drivers/pci/dwc/pcie-designware.h      |  23 +++++-
>>  5 files changed, 173 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/pci/dwc/pci-dra7xx.c b/drivers/pci/dwc/pci-dra7xx.c index
>> ed8558d..5265725 100644
>> --- a/drivers/pci/dwc/pci-dra7xx.c
>> +++ b/drivers/pci/dwc/pci-dra7xx.c
>> @@ -369,7 +369,7 @@ static void dra7xx_pcie_raise_msi_irq(struct
>> dra7xx_pcie *dra7xx,  }
>>
>>  static int dra7xx_pcie_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
>> -				 enum pci_epc_irq_type type, u8
>> interrupt_num)
>> +				 enum pci_epc_irq_type type, u16
>> interrupt_num)
>>  {
>>  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>>  	struct dra7xx_pcie *dra7xx = to_dra7xx_pcie(pci); diff --git
>> a/drivers/pci/dwc/pcie-artpec6.c b/drivers/pci/dwc/pcie-artpec6.c index
>> e66cede..96dc259 100644
>> --- a/drivers/pci/dwc/pcie-artpec6.c
>> +++ b/drivers/pci/dwc/pcie-artpec6.c
>> @@ -428,7 +428,7 @@ static void artpec6_pcie_ep_init(struct dw_pcie_ep
>> *ep)  }
>>
>>  static int artpec6_pcie_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
>> -				  enum pci_epc_irq_type type, u8
>> interrupt_num)
>> +				  enum pci_epc_irq_type type, u16
>> interrupt_num)
>>  {
>>  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>>
>> diff --git a/drivers/pci/dwc/pcie-designware-ep.c b/drivers/pci/dwc/pcie-
>> designware-ep.c
>> index 15b22a6..874d4c2 100644
>> --- a/drivers/pci/dwc/pcie-designware-ep.c
>> +++ b/drivers/pci/dwc/pcie-designware-ep.c
>> @@ -40,6 +40,44 @@ void dw_pcie_ep_reset_bar(struct dw_pcie *pci,
>> enum pci_barno bar)
>>  	__dw_pcie_ep_reset_bar(pci, bar, 0);
>>  }
>>
>> +void dw_pcie_ep_find_cap_addr(struct dw_pcie_ep *ep) {
>> +	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>> +	u8 next_ptr, curr_ptr, cap_id;
>> +	u16 reg;
>> +
>> +	memset(&ep->cap_addr, 0, sizeof(ep->cap_addr));
>> +
>> +	reg = dw_pcie_readw_dbi(pci, PCI_STATUS);
>> +	if (!(reg & PCI_STATUS_CAP_LIST))
>> +		return;
>> +
>> +	reg = dw_pcie_readw_dbi(pci, PCI_CAPABILITY_LIST);
>> +	next_ptr = (reg & 0x00ff);
>> +	if (!next_ptr)
>> +		return;
>> +
>> +	reg = dw_pcie_readw_dbi(pci, next_ptr);
>> +	curr_ptr = next_ptr;
>> +	next_ptr = (reg & 0xff00) >> 8;
>> +	cap_id = (reg & 0x00ff);
>> +
>> +	while (next_ptr && (cap_id <= PCI_CAP_ID_MAX)) {
>> +		switch (cap_id) {
>> +		case PCI_CAP_ID_MSI:
>> +			ep->cap_addr.msi_addr = curr_ptr;
>> +			break;
>> +		case PCI_CAP_ID_MSIX:
>> +			ep->cap_addr.msix_addr = curr_ptr;
>> +			break;
>> +		}
>> +		reg = dw_pcie_readw_dbi(pci, next_ptr);
>> +		curr_ptr = next_ptr;
>> +		next_ptr = (reg & 0xff00) >> 8;
>> +		cap_id = (reg & 0x00ff);
>> +	}
>> +}
>> +
>>  static int dw_pcie_ep_write_header(struct pci_epc *epc, u8 func_no,
>>  				   struct pci_epf_header *hdr)
>>  {
>> @@ -241,8 +279,47 @@ static int dw_pcie_ep_set_msi(struct pci_epc *epc,
>> u8 func_no, u8 encode_int)
>>  	return 0;
>>  }
>>
>> +static int dw_pcie_ep_get_msix(struct pci_epc *epc, u8 func_no) {
>> +	struct dw_pcie_ep *ep = epc_get_drvdata(epc);
>> +	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>> +	u32 val, reg;
>> +
>> +	if (ep->cap_addr.msix_addr == 0)
>> +		return 0;
>> +
>> +	reg = ep->cap_addr.msix_addr + PCI_MSIX_FLAGS;
>> +	val = dw_pcie_readw_dbi(pci, reg);
>> +	if (!(val & PCI_MSIX_FLAGS_ENABLE))
>> +		return -EINVAL;
>> +
>> +	val &= PCI_MSIX_FLAGS_QSIZE;
>> +
>> +	return val;
>> +}
>> +
>> +static int dw_pcie_ep_set_msix(struct pci_epc *epc, u8 func_no, u16
>> +interrupts) {
>> +	struct dw_pcie_ep *ep = epc_get_drvdata(epc);
>> +	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>> +	u32 val, reg;
>> +
>> +	if (ep->cap_addr.msix_addr == 0)
>> +		return 0;
>> +
>> +	reg = ep->cap_addr.msix_addr + PCI_MSIX_FLAGS;
>> +	val = dw_pcie_readw_dbi(pci, reg);
>> +	val &= ~PCI_MSIX_FLAGS_QSIZE;
>> +	val |= interrupts;
>> +	dw_pcie_dbi_ro_wr_en(pci);
>> +	dw_pcie_writew_dbi(pci, reg, val);
>> +	dw_pcie_dbi_ro_wr_dis(pci);
>> +
>> +	return 0;
>> +}
>> +
>>  static int dw_pcie_ep_raise_irq(struct pci_epc *epc, u8 func_no,
>> -				enum pci_epc_irq_type type, u8
>> interrupt_num)
>> +				enum pci_epc_irq_type type, u16
>> interrupt_num)
>>  {
>>  	struct dw_pcie_ep *ep = epc_get_drvdata(epc);
>>
>> @@ -282,6 +359,8 @@ static const struct pci_epc_ops epc_ops = {
>>  	.unmap_addr		= dw_pcie_ep_unmap_addr,
>>  	.set_msi		= dw_pcie_ep_set_msi,
>>  	.get_msi		= dw_pcie_ep_get_msi,
>> +	.set_msix		= dw_pcie_ep_set_msix,
>> +	.get_msix		= dw_pcie_ep_get_msix,
>>  	.raise_irq		= dw_pcie_ep_raise_irq,
>>  	.start			= dw_pcie_ep_start,
>>  	.stop			= dw_pcie_ep_stop,
>> @@ -322,6 +401,60 @@ int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep
>> *ep, u8 func_no,
>>  	return 0;
>>  }
>>
>> +int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no,
>> +			     u16 interrupt_num)
>> +{
>> +	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>> +	struct pci_epc *epc = ep->epc;
>> +	u16 tbl_offset, bir;
>> +	u32 bar_addr_upper, bar_addr_lower;
>> +	u32 msg_addr_upper, msg_addr_lower;
>> +	u32 reg, msg_data;
>> +	u64 tbl_addr, msg_addr, reg_u64;
>> +	void __iomem *msix_tbl;
>> +	int ret;
>> +
>> +	reg = ep->cap_addr.msix_addr + PCI_MSIX_TABLE;
>> +	tbl_offset = dw_pcie_readl_dbi(pci, reg);
>> +	bir = (tbl_offset & PCI_MSIX_TABLE_BIR);
>> +	tbl_offset &= PCI_MSIX_TABLE_OFFSET;
>> +	tbl_offset >>= 3;
>> +
>> +	reg = PCI_BASE_ADDRESS_0 + (4 * bir);
>> +	bar_addr_lower = dw_pcie_readl_dbi(pci, reg);
>> +	reg_u64 = (bar_addr_lower & PCI_BASE_ADDRESS_MEM_TYPE_MASK);
>> +	if (reg_u64 == PCI_BASE_ADDRESS_MEM_TYPE_64)
>> +		bar_addr_upper = dw_pcie_readl_dbi(pci, reg + 4);
>> +	else
>> +		bar_addr_upper = 0;
>> +
>> +	tbl_addr = ((u64) bar_addr_upper) << 32 | bar_addr_lower;
>> +	tbl_addr += (tbl_offset + ((interrupt_num - 1) * PCI_MSIX_ENTRY_SIZE));
>> +	tbl_addr &= PCI_BASE_ADDRESS_MEM_MASK;
>> +
>> +	msix_tbl = ioremap_nocache(ep->phys_base + tbl_addr, ep->addr_size);
>> +	if (!msix_tbl)
>> +		return -EINVAL;
>> +
> I think you need to check the mask bit in  vector control for the requested IRQ.

Yes, you are right. I'll fix it.

Regards,
Gustavo

> You could set the pending bit if masked, but would then need some output 
> signal to inform when the mask bit is cleared (or poll it) so the message can be sent
> later.
> 
> Also, do you need to check PCI_MSIX_FLAGS_ENABLE here as well, or is it checked earlier?
> 
> Regards,
> Alan
> 

[RFC 02/10] PCI: cadence: Update cdns_pcie_ep_raise_irq function signature

[Gustavo Pimentel]

Changes the cdns_pcie_ep_raise_irq function signature, namely the
interrupt_num variable type from u8 to u16 to accommodate the MSI-X maximum
interrupts of 2048.

Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
---
 drivers/pci/cadence/pcie-cadence-ep.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/cadence/pcie-cadence-ep.c b/drivers/pci/cadence/pcie-cadence-ep.c
index 3d8283e..6d6322c 100644
--- a/drivers/pci/cadence/pcie-cadence-ep.c
+++ b/drivers/pci/cadence/pcie-cadence-ep.c
@@ -363,7 +363,7 @@ static int cdns_pcie_ep_send_msi_irq(struct cdns_pcie_ep *ep, u8 fn,
 }
 
 static int cdns_pcie_ep_raise_irq(struct pci_epc *epc, u8 fn,
-				  enum pci_epc_irq_type type, u8 interrupt_num)
+				  enum pci_epc_irq_type type, u16 interrupt_num)
 {
 	struct cdns_pcie_ep *ep = epc_get_drvdata(epc);
 
-- 
2.7.4

RE: [RFC 02/10] PCI: cadence: Update cdns_pcie_ep_raise_irq function signature

[Alan Douglas]

On Tue, Apr 10 at 18.15, Gustavo Pimentel wrote:
> From: Gustavo Pimentel [mailto:gustavo.pimentel@synopsys.com]
> Changes the cdns_pcie_ep_raise_irq function signature, namely the
> interrupt_num variable type from u8 to u16 to accommodate the MSI-X
> maximum interrupts of 2048.
>=20
> Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
> ---
>  drivers/pci/cadence/pcie-cadence-ep.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>=20
> diff --git a/drivers/pci/cadence/pcie-cadence-ep.c
> b/drivers/pci/cadence/pcie-cadence-ep.c
> index 3d8283e..6d6322c 100644
> --- a/drivers/pci/cadence/pcie-cadence-ep.c
> +++ b/drivers/pci/cadence/pcie-cadence-ep.c
> @@ -363,7 +363,7 @@ static int cdns_pcie_ep_send_msi_irq(struct
> cdns_pcie_ep *ep, u8 fn,  }
>=20
>  static int cdns_pcie_ep_raise_irq(struct pci_epc *epc, u8 fn,
> -				  enum pci_epc_irq_type type, u8
> interrupt_num)
> +				  enum pci_epc_irq_type type, u16
> interrupt_num)
>  {
>  	struct cdns_pcie_ep *ep =3D epc_get_drvdata(epc);
>=20
> --
> 2.7.4
>=20
Acked-by: Alan Douglas <adouglas@cadence.com>

[RFC 03/10] PCI: endpoint: Add MSI-X interfaces

[Gustavo Pimentel]

Implements the generic method for calling the get/set callbacks.

Adds the PCI_EPC_IRQ_MSIX type.

Adds the MSI-X callbacks signatures to the ops structure.

Adds sysfs interface for altering the number of MSI-X entries.

Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
---
 drivers/pci/endpoint/pci-ep-cfs.c   | 24 ++++++++++++++++
 drivers/pci/endpoint/pci-epc-core.c | 57 +++++++++++++++++++++++++++++++++++++
 include/linux/pci-epc.h             | 11 ++++++-
 include/linux/pci-epf.h             |  1 +
 4 files changed, 92 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/endpoint/pci-ep-cfs.c b/drivers/pci/endpoint/pci-ep-cfs.c
index 018ea34..d1288a0 100644
--- a/drivers/pci/endpoint/pci-ep-cfs.c
+++ b/drivers/pci/endpoint/pci-ep-cfs.c
@@ -286,6 +286,28 @@ static ssize_t pci_epf_msi_interrupts_show(struct config_item *item,
 		       to_pci_epf_group(item)->epf->msi_interrupts);
 }
 
+static ssize_t pci_epf_msix_interrupts_store(struct config_item *item,
+					     const char *page, size_t len)
+{
+	u16 val;
+	int ret;
+
+	ret = kstrtou16(page, 0, &val);
+	if (ret)
+		return ret;
+
+	to_pci_epf_group(item)->epf->msix_interrupts = val;
+
+	return len;
+}
+
+static ssize_t pci_epf_msix_interrupts_show(struct config_item *item,
+					    char *page)
+{
+	return sprintf(page, "%d\n",
+		       to_pci_epf_group(item)->epf->msix_interrupts);
+}
+
 PCI_EPF_HEADER_R(vendorid)
 PCI_EPF_HEADER_W_u16(vendorid)
 
@@ -327,6 +349,7 @@ CONFIGFS_ATTR(pci_epf_, subsys_vendor_id);
 CONFIGFS_ATTR(pci_epf_, subsys_id);
 CONFIGFS_ATTR(pci_epf_, interrupt_pin);
 CONFIGFS_ATTR(pci_epf_, msi_interrupts);
+CONFIGFS_ATTR(pci_epf_, msix_interrupts);
 
 static struct configfs_attribute *pci_epf_attrs[] = {
 	&pci_epf_attr_vendorid,
@@ -340,6 +363,7 @@ static struct configfs_attribute *pci_epf_attrs[] = {
 	&pci_epf_attr_subsys_id,
 	&pci_epf_attr_interrupt_pin,
 	&pci_epf_attr_msi_interrupts,
+	&pci_epf_attr_msix_interrupts,
 	NULL,
 };
 
diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
index b0ee427..294a383 100644
--- a/drivers/pci/endpoint/pci-epc-core.c
+++ b/drivers/pci/endpoint/pci-epc-core.c
@@ -218,6 +218,63 @@ int pci_epc_set_msi(struct pci_epc *epc, u8 func_no, u8 interrupts)
 EXPORT_SYMBOL_GPL(pci_epc_set_msi);
 
 /**
+ * pci_epc_get_msix() - get the number of MSI-X interrupt numbers allocated
+ * @epc: the EPC device to which MSI-X interrupts was requested
+ * @func_no: the endpoint function number in the EPC device
+ *
+ * Invoke to get the number of MSI-X interrupts allocated by the RC
+ */
+int pci_epc_get_msix(struct pci_epc *epc, u8 func_no)
+{
+	int interrupt;
+	unsigned long flags;
+
+	if (IS_ERR_OR_NULL(epc) || func_no >= epc->max_functions)
+		return 0;
+
+	if (!epc->ops->get_msix)
+		return 0;
+
+	spin_lock_irqsave(&epc->lock, flags);
+	interrupt = epc->ops->get_msix(epc, func_no);
+	spin_unlock_irqrestore(&epc->lock, flags);
+
+	if (interrupt < 0)
+		return 0;
+
+	return interrupt++;
+}
+EXPORT_SYMBOL_GPL(pci_epc_get_msix);
+
+/**
+ * pci_epc_set_msix() - set the number of MSI-X interrupt numbers required
+ * @epc: the EPC device on which MSI-X has to be configured
+ * @func_no: the endpoint function number in the EPC device
+ * @interrupts: number of MSI-X interrupts required by the EPF
+ *
+ * Invoke to set the required number of MSI-X interrupts.
+ */
+int pci_epc_set_msix(struct pci_epc *epc, u8 func_no, u16 interrupts)
+{
+	int ret;
+	unsigned long flags;
+
+	if (IS_ERR_OR_NULL(epc) || func_no >= epc->max_functions ||
+	    interrupts < 1 || interrupts > 2048)
+		return -EINVAL;
+
+	if (!epc->ops->set_msix)
+		return 0;
+
+	spin_lock_irqsave(&epc->lock, flags);
+	ret = epc->ops->set_msix(epc, func_no, interrupts - 1);
+	spin_unlock_irqrestore(&epc->lock, flags);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(pci_epc_set_msix);
+
+/**
  * pci_epc_unmap_addr() - unmap CPU address from PCI address
  * @epc: the EPC device on which address is allocated
  * @func_no: the endpoint function number in the EPC device
diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
index af657ca..32e8961 100644
--- a/include/linux/pci-epc.h
+++ b/include/linux/pci-epc.h
@@ -17,6 +17,7 @@ enum pci_epc_irq_type {
 	PCI_EPC_IRQ_UNKNOWN,
 	PCI_EPC_IRQ_LEGACY,
 	PCI_EPC_IRQ_MSI,
+	PCI_EPC_IRQ_MSIX,
 };
 
 /**
@@ -30,6 +31,10 @@ enum pci_epc_irq_type {
  *	     capability register
  * @get_msi: ops to get the number of MSI interrupts allocated by the RC from
  *	     the MSI capability register
+ * @set_msix: ops to set the requested number of MSI-X interrupts in the
+ *	     MSI-X capability register
+ * @get_msix: ops to get the number of MSI-X interrupts allocated by the RC
+ *	     from the MSI-X capability register
  * @raise_irq: ops to raise a legacy or MSI interrupt
  * @start: ops to start the PCI link
  * @stop: ops to stop the PCI link
@@ -48,8 +53,10 @@ struct pci_epc_ops {
 			      phys_addr_t addr);
 	int	(*set_msi)(struct pci_epc *epc, u8 func_no, u8 interrupts);
 	int	(*get_msi)(struct pci_epc *epc, u8 func_no);
+	int	(*set_msix)(struct pci_epc *epc, u8 func_no, u16 interrupts);
+	int	(*get_msix)(struct pci_epc *epc, u8 func_no);
 	int	(*raise_irq)(struct pci_epc *epc, u8 func_no,
-			     enum pci_epc_irq_type type, u8 interrupt_num);
+			     enum pci_epc_irq_type type, u16 interrupt_num);
 	int	(*start)(struct pci_epc *epc);
 	void	(*stop)(struct pci_epc *epc);
 	struct module *owner;
@@ -136,6 +143,8 @@ void pci_epc_unmap_addr(struct pci_epc *epc, u8 func_no,
 			phys_addr_t phys_addr);
 int pci_epc_set_msi(struct pci_epc *epc, u8 func_no, u8 interrupts);
 int pci_epc_get_msi(struct pci_epc *epc, u8 func_no);
+int pci_epc_set_msix(struct pci_epc *epc, u8 func_no, u16 interrupts);
+int pci_epc_get_msix(struct pci_epc *epc, u8 func_no);
 int pci_epc_raise_irq(struct pci_epc *epc, u8 func_no,
 		      enum pci_epc_irq_type type, u8 interrupt_num);
 int pci_epc_start(struct pci_epc *epc);
diff --git a/include/linux/pci-epf.h b/include/linux/pci-epf.h
index f7d6f48..9bb1f31 100644
--- a/include/linux/pci-epf.h
+++ b/include/linux/pci-epf.h
@@ -119,6 +119,7 @@ struct pci_epf {
 	struct pci_epf_header	*header;
 	struct pci_epf_bar	bar[6];
 	u8			msi_interrupts;
+	u16			msix_interrupts;
 	u8			func_no;
 
 	struct pci_epc		*epc;
-- 
2.7.4

Re: [RFC 03/10] PCI: endpoint: Add MSI-X interfaces

[Kishon Vijay Abraham I]

Hi,

On Tuesday 10 April 2018 10:44 PM, Gustavo Pimentel wrote:
> Implements the generic method for calling the get/set callbacks.
> 
> Adds the PCI_EPC_IRQ_MSIX type.
> 
> Adds the MSI-X callbacks signatures to the ops structure.
> 
> Adds sysfs interface for altering the number of MSI-X entries.
> 
> Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
> ---
>  drivers/pci/endpoint/pci-ep-cfs.c   | 24 ++++++++++++++++
>  drivers/pci/endpoint/pci-epc-core.c | 57 +++++++++++++++++++++++++++++++++++++
>  include/linux/pci-epc.h             | 11 ++++++-
>  include/linux/pci-epf.h             |  1 +
>  4 files changed, 92 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/endpoint/pci-ep-cfs.c b/drivers/pci/endpoint/pci-ep-cfs.c
> index 018ea34..d1288a0 100644
> --- a/drivers/pci/endpoint/pci-ep-cfs.c
> +++ b/drivers/pci/endpoint/pci-ep-cfs.c
> @@ -286,6 +286,28 @@ static ssize_t pci_epf_msi_interrupts_show(struct config_item *item,
>  		       to_pci_epf_group(item)->epf->msi_interrupts);
>  }
>  
> +static ssize_t pci_epf_msix_interrupts_store(struct config_item *item,
> +					     const char *page, size_t len)
> +{
> +	u16 val;
> +	int ret;
> +
> +	ret = kstrtou16(page, 0, &val);
> +	if (ret)
> +		return ret;
> +
> +	to_pci_epf_group(item)->epf->msix_interrupts = val;
> +
> +	return len;
> +}
> +
> +static ssize_t pci_epf_msix_interrupts_show(struct config_item *item,
> +					    char *page)
> +{
> +	return sprintf(page, "%d\n",
> +		       to_pci_epf_group(item)->epf->msix_interrupts);
> +}
> +
>  PCI_EPF_HEADER_R(vendorid)
>  PCI_EPF_HEADER_W_u16(vendorid)
>  
> @@ -327,6 +349,7 @@ CONFIGFS_ATTR(pci_epf_, subsys_vendor_id);
>  CONFIGFS_ATTR(pci_epf_, subsys_id);
>  CONFIGFS_ATTR(pci_epf_, interrupt_pin);
>  CONFIGFS_ATTR(pci_epf_, msi_interrupts);
> +CONFIGFS_ATTR(pci_epf_, msix_interrupts);
>  
>  static struct configfs_attribute *pci_epf_attrs[] = {
>  	&pci_epf_attr_vendorid,
> @@ -340,6 +363,7 @@ static struct configfs_attribute *pci_epf_attrs[] = {
>  	&pci_epf_attr_subsys_id,
>  	&pci_epf_attr_interrupt_pin,
>  	&pci_epf_attr_msi_interrupts,
> +	&pci_epf_attr_msix_interrupts,
>  	NULL,
>  };
>  
> diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
> index b0ee427..294a383 100644
> --- a/drivers/pci/endpoint/pci-epc-core.c
> +++ b/drivers/pci/endpoint/pci-epc-core.c
> @@ -218,6 +218,63 @@ int pci_epc_set_msi(struct pci_epc *epc, u8 func_no, u8 interrupts)
>  EXPORT_SYMBOL_GPL(pci_epc_set_msi);
>  
>  /**
> + * pci_epc_get_msix() - get the number of MSI-X interrupt numbers allocated
> + * @epc: the EPC device to which MSI-X interrupts was requested
> + * @func_no: the endpoint function number in the EPC device
> + *
> + * Invoke to get the number of MSI-X interrupts allocated by the RC
> + */
> +int pci_epc_get_msix(struct pci_epc *epc, u8 func_no)
> +{
> +	int interrupt;
> +	unsigned long flags;
> +
> +	if (IS_ERR_OR_NULL(epc) || func_no >= epc->max_functions)
> +		return 0;
> +
> +	if (!epc->ops->get_msix)
> +		return 0;
> +
> +	spin_lock_irqsave(&epc->lock, flags);
> +	interrupt = epc->ops->get_msix(epc, func_no);
> +	spin_unlock_irqrestore(&epc->lock, flags);
> +
> +	if (interrupt < 0)
> +		return 0;
> +
> +	return interrupt++;

return interrupt + 1?
> +}
> +EXPORT_SYMBOL_GPL(pci_epc_get_msix);
> +
> +/**
> + * pci_epc_set_msix() - set the number of MSI-X interrupt numbers required
> + * @epc: the EPC device on which MSI-X has to be configured
> + * @func_no: the endpoint function number in the EPC device
> + * @interrupts: number of MSI-X interrupts required by the EPF
> + *
> + * Invoke to set the required number of MSI-X interrupts.
> + */
> +int pci_epc_set_msix(struct pci_epc *epc, u8 func_no, u16 interrupts)
> +{
> +	int ret;
> +	unsigned long flags;
> +
> +	if (IS_ERR_OR_NULL(epc) || func_no >= epc->max_functions ||
> +	    interrupts < 1 || interrupts > 2048)
> +		return -EINVAL;
> +
> +	if (!epc->ops->set_msix)
> +		return 0;
> +
> +	spin_lock_irqsave(&epc->lock, flags);
> +	ret = epc->ops->set_msix(epc, func_no, interrupts - 1);
> +	spin_unlock_irqrestore(&epc->lock, flags);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(pci_epc_set_msix);
> +
> +/**
>   * pci_epc_unmap_addr() - unmap CPU address from PCI address
>   * @epc: the EPC device on which address is allocated
>   * @func_no: the endpoint function number in the EPC device
> diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
> index af657ca..32e8961 100644
> --- a/include/linux/pci-epc.h
> +++ b/include/linux/pci-epc.h
> @@ -17,6 +17,7 @@ enum pci_epc_irq_type {
>  	PCI_EPC_IRQ_UNKNOWN,
>  	PCI_EPC_IRQ_LEGACY,
>  	PCI_EPC_IRQ_MSI,
> +	PCI_EPC_IRQ_MSIX,
>  };
>  
>  /**
> @@ -30,6 +31,10 @@ enum pci_epc_irq_type {
>   *	     capability register
>   * @get_msi: ops to get the number of MSI interrupts allocated by the RC from
>   *	     the MSI capability register
> + * @set_msix: ops to set the requested number of MSI-X interrupts in the
> + *	     MSI-X capability register
> + * @get_msix: ops to get the number of MSI-X interrupts allocated by the RC
> + *	     from the MSI-X capability register
>   * @raise_irq: ops to raise a legacy or MSI interrupt
>   * @start: ops to start the PCI link
>   * @stop: ops to stop the PCI link
> @@ -48,8 +53,10 @@ struct pci_epc_ops {
>  			      phys_addr_t addr);
>  	int	(*set_msi)(struct pci_epc *epc, u8 func_no, u8 interrupts);
>  	int	(*get_msi)(struct pci_epc *epc, u8 func_no);
> +	int	(*set_msix)(struct pci_epc *epc, u8 func_no, u16 interrupts);
> +	int	(*get_msix)(struct pci_epc *epc, u8 func_no);

The 1st patch in the series is already using get_msix, set_msix. This patch
should precede the 1st patch.

Thanks
Kishon

Re: [RFC 03/10] PCI: endpoint: Add MSI-X interfaces

[Gustavo Pimentel]

Hi Kishon,

On 17/04/2018 11:24, Kishon Vijay Abraham I wrote:
> Hi,
> 
> On Tuesday 10 April 2018 10:44 PM, Gustavo Pimentel wrote:
>> Implements the generic method for calling the get/set callbacks.
>>
>> Adds the PCI_EPC_IRQ_MSIX type.
>>
>> Adds the MSI-X callbacks signatures to the ops structure.
>>
>> Adds sysfs interface for altering the number of MSI-X entries.
>>
>> Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
>> ---
>>  drivers/pci/endpoint/pci-ep-cfs.c   | 24 ++++++++++++++++
>>  drivers/pci/endpoint/pci-epc-core.c | 57 +++++++++++++++++++++++++++++++++++++
>>  include/linux/pci-epc.h             | 11 ++++++-
>>  include/linux/pci-epf.h             |  1 +
>>  4 files changed, 92 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/pci/endpoint/pci-ep-cfs.c b/drivers/pci/endpoint/pci-ep-cfs.c
>> index 018ea34..d1288a0 100644
>> --- a/drivers/pci/endpoint/pci-ep-cfs.c
>> +++ b/drivers/pci/endpoint/pci-ep-cfs.c
>> @@ -286,6 +286,28 @@ static ssize_t pci_epf_msi_interrupts_show(struct config_item *item,
>>  		       to_pci_epf_group(item)->epf->msi_interrupts);
>>  }
>>  
>> +static ssize_t pci_epf_msix_interrupts_store(struct config_item *item,
>> +					     const char *page, size_t len)
>> +{
>> +	u16 val;
>> +	int ret;
>> +
>> +	ret = kstrtou16(page, 0, &val);
>> +	if (ret)
>> +		return ret;
>> +
>> +	to_pci_epf_group(item)->epf->msix_interrupts = val;
>> +
>> +	return len;
>> +}
>> +
>> +static ssize_t pci_epf_msix_interrupts_show(struct config_item *item,
>> +					    char *page)
>> +{
>> +	return sprintf(page, "%d\n",
>> +		       to_pci_epf_group(item)->epf->msix_interrupts);
>> +}
>> +
>>  PCI_EPF_HEADER_R(vendorid)
>>  PCI_EPF_HEADER_W_u16(vendorid)
>>  
>> @@ -327,6 +349,7 @@ CONFIGFS_ATTR(pci_epf_, subsys_vendor_id);
>>  CONFIGFS_ATTR(pci_epf_, subsys_id);
>>  CONFIGFS_ATTR(pci_epf_, interrupt_pin);
>>  CONFIGFS_ATTR(pci_epf_, msi_interrupts);
>> +CONFIGFS_ATTR(pci_epf_, msix_interrupts);
>>  
>>  static struct configfs_attribute *pci_epf_attrs[] = {
>>  	&pci_epf_attr_vendorid,
>> @@ -340,6 +363,7 @@ static struct configfs_attribute *pci_epf_attrs[] = {
>>  	&pci_epf_attr_subsys_id,
>>  	&pci_epf_attr_interrupt_pin,
>>  	&pci_epf_attr_msi_interrupts,
>> +	&pci_epf_attr_msix_interrupts,
>>  	NULL,
>>  };
>>  
>> diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
>> index b0ee427..294a383 100644
>> --- a/drivers/pci/endpoint/pci-epc-core.c
>> +++ b/drivers/pci/endpoint/pci-epc-core.c
>> @@ -218,6 +218,63 @@ int pci_epc_set_msi(struct pci_epc *epc, u8 func_no, u8 interrupts)
>>  EXPORT_SYMBOL_GPL(pci_epc_set_msi);
>>  
>>  /**
>> + * pci_epc_get_msix() - get the number of MSI-X interrupt numbers allocated
>> + * @epc: the EPC device to which MSI-X interrupts was requested
>> + * @func_no: the endpoint function number in the EPC device
>> + *
>> + * Invoke to get the number of MSI-X interrupts allocated by the RC
>> + */
>> +int pci_epc_get_msix(struct pci_epc *epc, u8 func_no)
>> +{
>> +	int interrupt;
>> +	unsigned long flags;
>> +
>> +	if (IS_ERR_OR_NULL(epc) || func_no >= epc->max_functions)
>> +		return 0;
>> +
>> +	if (!epc->ops->get_msix)
>> +		return 0;
>> +
>> +	spin_lock_irqsave(&epc->lock, flags);
>> +	interrupt = epc->ops->get_msix(epc, func_no);
>> +	spin_unlock_irqrestore(&epc->lock, flags);
>> +
>> +	if (interrupt < 0)
>> +		return 0;
>> +
>> +	return interrupt++;
> 
> return interrupt + 1?

I'll change it.

>> +}
>> +EXPORT_SYMBOL_GPL(pci_epc_get_msix);
>> +
>> +/**
>> + * pci_epc_set_msix() - set the number of MSI-X interrupt numbers required
>> + * @epc: the EPC device on which MSI-X has to be configured
>> + * @func_no: the endpoint function number in the EPC device
>> + * @interrupts: number of MSI-X interrupts required by the EPF
>> + *
>> + * Invoke to set the required number of MSI-X interrupts.
>> + */
>> +int pci_epc_set_msix(struct pci_epc *epc, u8 func_no, u16 interrupts)
>> +{
>> +	int ret;
>> +	unsigned long flags;
>> +
>> +	if (IS_ERR_OR_NULL(epc) || func_no >= epc->max_functions ||
>> +	    interrupts < 1 || interrupts > 2048)
>> +		return -EINVAL;
>> +
>> +	if (!epc->ops->set_msix)
>> +		return 0;
>> +
>> +	spin_lock_irqsave(&epc->lock, flags);
>> +	ret = epc->ops->set_msix(epc, func_no, interrupts - 1);
>> +	spin_unlock_irqrestore(&epc->lock, flags);
>> +
>> +	return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(pci_epc_set_msix);
>> +
>> +/**
>>   * pci_epc_unmap_addr() - unmap CPU address from PCI address
>>   * @epc: the EPC device on which address is allocated
>>   * @func_no: the endpoint function number in the EPC device
>> diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
>> index af657ca..32e8961 100644
>> --- a/include/linux/pci-epc.h
>> +++ b/include/linux/pci-epc.h
>> @@ -17,6 +17,7 @@ enum pci_epc_irq_type {
>>  	PCI_EPC_IRQ_UNKNOWN,
>>  	PCI_EPC_IRQ_LEGACY,
>>  	PCI_EPC_IRQ_MSI,
>> +	PCI_EPC_IRQ_MSIX,
>>  };
>>  
>>  /**
>> @@ -30,6 +31,10 @@ enum pci_epc_irq_type {
>>   *	     capability register
>>   * @get_msi: ops to get the number of MSI interrupts allocated by the RC from
>>   *	     the MSI capability register
>> + * @set_msix: ops to set the requested number of MSI-X interrupts in the
>> + *	     MSI-X capability register
>> + * @get_msix: ops to get the number of MSI-X interrupts allocated by the RC
>> + *	     from the MSI-X capability register
>>   * @raise_irq: ops to raise a legacy or MSI interrupt
>>   * @start: ops to start the PCI link
>>   * @stop: ops to stop the PCI link
>> @@ -48,8 +53,10 @@ struct pci_epc_ops {
>>  			      phys_addr_t addr);
>>  	int	(*set_msi)(struct pci_epc *epc, u8 func_no, u8 interrupts);
>>  	int	(*get_msi)(struct pci_epc *epc, u8 func_no);
>> +	int	(*set_msix)(struct pci_epc *epc, u8 func_no, u16 interrupts);
>> +	int	(*get_msix)(struct pci_epc *epc, u8 func_no);
> 
> The 1st patch in the series is already using get_msix, set_msix. This patch
> should precede the 1st patch.

Yes, you're right.

> 
> Thanks
> Kishon
> 

Regards,
Gustavo

[RFC 04/10] PCI: dwc: MSI callbacks handler rework

[Gustavo Pimentel]

Adds in pci_epc_set_msi function a maximum number of 32 interrupts
validation.

Removes duplicate defines located on pcie-designware.h file. Uses now
the defines available on /include/uapi/linux/pci-regs.h file.

Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
---
 drivers/pci/dwc/pcie-designware-ep.c | 46 +++++++++++++++++++++++-------------
 drivers/pci/dwc/pcie-designware.h    | 11 ---------
 drivers/pci/endpoint/pci-epc-core.c  |  3 ++-
 3 files changed, 32 insertions(+), 28 deletions(-)

diff --git a/drivers/pci/dwc/pcie-designware-ep.c b/drivers/pci/dwc/pcie-designware-ep.c
index 874d4c2..e352786 100644
--- a/drivers/pci/dwc/pcie-designware-ep.c
+++ b/drivers/pci/dwc/pcie-designware-ep.c
@@ -251,29 +251,38 @@ static int dw_pcie_ep_map_addr(struct pci_epc *epc, u8 func_no,
 
 static int dw_pcie_ep_get_msi(struct pci_epc *epc, u8 func_no)
 {
-	int val;
 	struct dw_pcie_ep *ep = epc_get_drvdata(epc);
 	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
+	u32 val, reg;
+
+	if (ep->cap_addr.msi_addr == 0)
+		return 0;
 
-	val = dw_pcie_readw_dbi(pci, MSI_MESSAGE_CONTROL);
-	if (!(val & MSI_CAP_MSI_EN_MASK))
+	reg = ep->cap_addr.msi_addr + PCI_MSI_FLAGS;
+	val = dw_pcie_readw_dbi(pci, reg);
+	if (!(val & PCI_MSI_FLAGS_ENABLE))
 		return -EINVAL;
 
-	val = (val & MSI_CAP_MME_MASK) >> MSI_CAP_MME_SHIFT;
+	val = (val & PCI_MSI_FLAGS_QSIZE) >> 4;
+
 	return val;
 }
 
-static int dw_pcie_ep_set_msi(struct pci_epc *epc, u8 func_no, u8 encode_int)
+static int dw_pcie_ep_set_msi(struct pci_epc *epc, u8 func_no, u8 interrupts)
 {
-	int val;
 	struct dw_pcie_ep *ep = epc_get_drvdata(epc);
 	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
+	u32 val, reg;
+
+	if (ep->cap_addr.msi_addr == 0)
+		return 0;
 
-	val = dw_pcie_readw_dbi(pci, MSI_MESSAGE_CONTROL);
-	val &= ~MSI_CAP_MMC_MASK;
-	val |= (encode_int << MSI_CAP_MMC_SHIFT) & MSI_CAP_MMC_MASK;
+	reg = ep->cap_addr.msi_addr + PCI_MSI_FLAGS;
+	val = dw_pcie_readw_dbi(pci, reg);
+	val &= ~PCI_MSI_FLAGS_QMASK;
+	val |= (interrupts << 1) & PCI_MSI_FLAGS_QMASK;
 	dw_pcie_dbi_ro_wr_en(pci);
-	dw_pcie_writew_dbi(pci, MSI_MESSAGE_CONTROL, val);
+	dw_pcie_writew_dbi(pci, reg, val);
 	dw_pcie_dbi_ro_wr_dis(pci);
 
 	return 0;
@@ -372,21 +381,26 @@ int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep *ep, u8 func_no,
 	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
 	struct pci_epc *epc = ep->epc;
 	u16 msg_ctrl, msg_data;
-	u32 msg_addr_lower, msg_addr_upper;
+	u32 msg_addr_lower, msg_addr_upper, reg;
 	u64 msg_addr;
 	bool has_upper;
 	int ret;
 
 	/* Raise MSI per the PCI Local Bus Specification Revision 3.0, 6.8.1. */
-	msg_ctrl = dw_pcie_readw_dbi(pci, MSI_MESSAGE_CONTROL);
+	reg = ep->cap_addr.msi_addr + PCI_MSI_FLAGS;
+	msg_ctrl = dw_pcie_readw_dbi(pci, reg);
 	has_upper = !!(msg_ctrl & PCI_MSI_FLAGS_64BIT);
-	msg_addr_lower = dw_pcie_readl_dbi(pci, MSI_MESSAGE_ADDR_L32);
+	reg = ep->cap_addr.msi_addr + PCI_MSI_ADDRESS_LO;
+	msg_addr_lower = dw_pcie_readl_dbi(pci, reg);
 	if (has_upper) {
-		msg_addr_upper = dw_pcie_readl_dbi(pci, MSI_MESSAGE_ADDR_U32);
-		msg_data = dw_pcie_readw_dbi(pci, MSI_MESSAGE_DATA_64);
+		reg = ep->cap_addr.msi_addr + PCI_MSI_ADDRESS_HI;
+		msg_addr_upper = dw_pcie_readl_dbi(pci, reg);
+		reg = ep->cap_addr.msi_addr + PCI_MSI_DATA_64;
+		msg_data = dw_pcie_readw_dbi(pci, reg);
 	} else {
 		msg_addr_upper = 0;
-		msg_data = dw_pcie_readw_dbi(pci, MSI_MESSAGE_DATA_32);
+		reg = ep->cap_addr.msi_addr + PCI_MSI_DATA_32;
+		msg_data = dw_pcie_readw_dbi(pci, reg);
 	}
 	msg_addr = ((u64) msg_addr_upper) << 32 | msg_addr_lower;
 	ret = dw_pcie_ep_map_addr(epc, func_no, ep->msi_mem_phys, msg_addr,
diff --git a/drivers/pci/dwc/pcie-designware.h b/drivers/pci/dwc/pcie-designware.h
index 456fd94..2acf18b0 100644
--- a/drivers/pci/dwc/pcie-designware.h
+++ b/drivers/pci/dwc/pcie-designware.h
@@ -96,17 +96,6 @@
 #define PCIE_GET_ATU_INB_UNR_REG_OFFSET(region)				\
 			((0x3 << 20) | ((region) << 9) | (0x1 << 8))
 
-#define MSI_MESSAGE_CONTROL		0x52
-#define MSI_CAP_MMC_SHIFT		1
-#define MSI_CAP_MMC_MASK		(7 << MSI_CAP_MMC_SHIFT)
-#define MSI_CAP_MME_SHIFT		4
-#define MSI_CAP_MSI_EN_MASK		0x1
-#define MSI_CAP_MME_MASK		(7 << MSI_CAP_MME_SHIFT)
-#define MSI_MESSAGE_ADDR_L32		0x54
-#define MSI_MESSAGE_ADDR_U32		0x58
-#define MSI_MESSAGE_DATA_32		0x58
-#define MSI_MESSAGE_DATA_64		0x5C
-
 #define MAX_MSI_IRQS			256
 #define MAX_MSI_IRQS_PER_CTRL		32
 #define MAX_MSI_CTRLS			(MAX_MSI_IRQS / MAX_MSI_IRQS_PER_CTRL)
diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
index 294a383..dbd17e4 100644
--- a/drivers/pci/endpoint/pci-epc-core.c
+++ b/drivers/pci/endpoint/pci-epc-core.c
@@ -201,7 +201,8 @@ int pci_epc_set_msi(struct pci_epc *epc, u8 func_no, u8 interrupts)
 	u8 encode_int;
 	unsigned long flags;
 
-	if (IS_ERR_OR_NULL(epc) || func_no >= epc->max_functions)
+	if (IS_ERR_OR_NULL(epc) || func_no >= epc->max_functions ||
+	    interrupts > 32)
 		return -EINVAL;
 
 	if (!epc->ops->set_msi)
-- 
2.7.4

[RFC 05/10] PCI: dwc: Add legacy interrupt callback handler

[Gustavo Pimentel]

Adds a legacy interrupt callback handler. Currently Designware IP doesn't
allow triggering the legacy interrupt.

Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
---
 drivers/pci/dwc/pcie-designware-ep.c   | 10 ++++++++++
 drivers/pci/dwc/pcie-designware-plat.c |  3 +--
 drivers/pci/dwc/pcie-designware.h      |  6 ++++++
 3 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/dwc/pcie-designware-ep.c b/drivers/pci/dwc/pcie-designware-ep.c
index e352786..fb55259 100644
--- a/drivers/pci/dwc/pcie-designware-ep.c
+++ b/drivers/pci/dwc/pcie-designware-ep.c
@@ -375,6 +375,16 @@ static const struct pci_epc_ops epc_ops = {
 	.stop			= dw_pcie_ep_stop,
 };
 
+int dw_pcie_ep_raise_legacy_irq(struct dw_pcie_ep *ep, u8 func_no)
+{
+	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
+	struct device *dev = pci->dev;
+
+	dev_err(dev, "EP cannot trigger legacy IRQs\n");
+
+	return -EINVAL;
+}
+
 int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep *ep, u8 func_no,
 			     u8 interrupt_num)
 {
diff --git a/drivers/pci/dwc/pcie-designware-plat.c b/drivers/pci/dwc/pcie-designware-plat.c
index c3a4707..3874b02 100644
--- a/drivers/pci/dwc/pcie-designware-plat.c
+++ b/drivers/pci/dwc/pcie-designware-plat.c
@@ -86,8 +86,7 @@ static int dw_plat_pcie_ep_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
 
 	switch (type) {
 	case PCI_EPC_IRQ_LEGACY:
-		dev_err(pci->dev, "EP cannot trigger legacy IRQs\n");
-		return -EINVAL;
+		return dw_pcie_ep_raise_legacy_irq(ep, func_no);
 	case PCI_EPC_IRQ_MSI:
 		return dw_pcie_ep_raise_msi_irq(ep, func_no, interrupt_num);
 	case PCI_EPC_IRQ_MSIX:
diff --git a/drivers/pci/dwc/pcie-designware.h b/drivers/pci/dwc/pcie-designware.h
index 2acf18b0..808b280 100644
--- a/drivers/pci/dwc/pcie-designware.h
+++ b/drivers/pci/dwc/pcie-designware.h
@@ -354,6 +354,7 @@ static inline int dw_pcie_allocate_domains(struct pcie_port *pp)
 void dw_pcie_ep_linkup(struct dw_pcie_ep *ep);
 int dw_pcie_ep_init(struct dw_pcie_ep *ep);
 void dw_pcie_ep_exit(struct dw_pcie_ep *ep);
+int dw_pcie_ep_raise_legacy_irq(struct dw_pcie_ep *ep, u8 func_no);
 int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep *ep, u8 func_no,
 			     u8 interrupt_num);
 int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no,
@@ -374,6 +375,11 @@ static inline void dw_pcie_ep_exit(struct dw_pcie_ep *ep)
 {
 }
 
+static inline int dw_pcie_ep_raise_legacy_irq(struct dw_pcie_ep *ep, u8 func_no)
+{
+	return 0;
+}
+
 static inline int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep *ep, u8 func_no,
 					   u8 interrupt_num)
 {
-- 
2.7.4

[RFC 06/10] misc: pci_endpoint_test: Add MSI-X support

[Gustavo Pimentel]

Adds the MSI-X support and updates driver documentation accordingly.

Changes the driver parameter in order to allow the interruption type
selection.

Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
---
 Documentation/misc-devices/pci-endpoint-test.txt |   3 +
 drivers/misc/pci_endpoint_test.c                 | 102 +++++++++++++++++------
 2 files changed, 79 insertions(+), 26 deletions(-)

diff --git a/Documentation/misc-devices/pci-endpoint-test.txt b/Documentation/misc-devices/pci-endpoint-test.txt
index 4ebc359..fdfa0f6 100644
--- a/Documentation/misc-devices/pci-endpoint-test.txt
+++ b/Documentation/misc-devices/pci-endpoint-test.txt
@@ -10,6 +10,7 @@ The PCI driver for the test device performs the following tests
 	*) verifying addresses programmed in BAR
 	*) raise legacy IRQ
 	*) raise MSI IRQ
+	*) raise MSI-X IRQ
 	*) read data
 	*) write data
 	*) copy data
@@ -25,6 +26,8 @@ ioctl
  PCITEST_LEGACY_IRQ: Tests legacy IRQ
  PCITEST_MSI: Tests message signalled interrupts. The MSI number
 	      to be tested should be passed as argument.
+ PCITEST_MSIX: Tests message signalled interrupts. The MSI-X number
+	      to be tested should be passed as argument.
  PCITEST_WRITE: Perform write tests. The size of the buffer should be passed
 		as argument.
  PCITEST_READ: Perform read tests. The size of the buffer should be passed
diff --git a/drivers/misc/pci_endpoint_test.c b/drivers/misc/pci_endpoint_test.c
index 37db0fc..a7d9354 100644
--- a/drivers/misc/pci_endpoint_test.c
+++ b/drivers/misc/pci_endpoint_test.c
@@ -42,11 +42,16 @@
 #define PCI_ENDPOINT_TEST_COMMAND	0x4
 #define COMMAND_RAISE_LEGACY_IRQ	BIT(0)
 #define COMMAND_RAISE_MSI_IRQ		BIT(1)
-#define MSI_NUMBER_SHIFT		2
-/* 6 bits for MSI number */
-#define COMMAND_READ                    BIT(8)
-#define COMMAND_WRITE                   BIT(9)
-#define COMMAND_COPY                    BIT(10)
+#define COMMAND_RAISE_MSIX_IRQ		BIT(2)
+#define IRQ_TYPE_SHIFT			3
+#define IRQ_TYPE_LEGACY			0
+#define IRQ_TYPE_MSI			1
+#define IRQ_TYPE_MSIX			2
+#define MSI_NUMBER_SHIFT		5
+/* 12 bits for MSI number */
+#define COMMAND_READ                    BIT(17)
+#define COMMAND_WRITE                   BIT(18)
+#define COMMAND_COPY                    BIT(19)
 
 #define PCI_ENDPOINT_TEST_STATUS	0x8
 #define STATUS_READ_SUCCESS             BIT(0)
@@ -73,9 +78,9 @@ static DEFINE_IDA(pci_endpoint_test_ida);
 #define to_endpoint_test(priv) container_of((priv), struct pci_endpoint_test, \
 					    miscdev)
 
-static bool no_msi;
-module_param(no_msi, bool, 0444);
-MODULE_PARM_DESC(no_msi, "Disable MSI interrupt in pci_endpoint_test");
+static int irq_type = IRQ_TYPE_MSIX;
+module_param(irq_type, int, 0444);
+MODULE_PARM_DESC(irq_type, "IRQ mode selection in pci_endpoint_test (0 - Legacy, 1 - MSI, 2 - MSI-X)");
 
 enum pci_barno {
 	BAR_0,
@@ -103,7 +108,7 @@ struct pci_endpoint_test {
 struct pci_endpoint_test_data {
 	enum pci_barno test_reg_bar;
 	size_t alignment;
-	bool no_msi;
+	int irq_type;
 };
 
 static inline u32 pci_endpoint_test_readl(struct pci_endpoint_test *test,
@@ -177,10 +182,10 @@ static bool pci_endpoint_test_bar(struct pci_endpoint_test *test,
 
 static bool pci_endpoint_test_legacy_irq(struct pci_endpoint_test *test)
 {
-	u32 val;
+	u32 val = COMMAND_RAISE_LEGACY_IRQ;
 
-	pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_COMMAND,
-				 COMMAND_RAISE_LEGACY_IRQ);
+	val |= (IRQ_TYPE_LEGACY << IRQ_TYPE_SHIFT);
+	pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_COMMAND, val);
 	val = wait_for_completion_timeout(&test->irq_raised,
 					  msecs_to_jiffies(1000));
 	if (!val)
@@ -192,12 +197,12 @@ static bool pci_endpoint_test_legacy_irq(struct pci_endpoint_test *test)
 static bool pci_endpoint_test_msi_irq(struct pci_endpoint_test *test,
 				      u8 msi_num)
 {
-	u32 val;
+	u32 val = COMMAND_RAISE_MSI_IRQ;
 	struct pci_dev *pdev = test->pdev;
 
-	pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_COMMAND,
-				 msi_num << MSI_NUMBER_SHIFT |
-				 COMMAND_RAISE_MSI_IRQ);
+	val |= (msi_num << MSI_NUMBER_SHIFT);
+	val |= (IRQ_TYPE_MSI << IRQ_TYPE_SHIFT);
+	pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_COMMAND, val);
 	val = wait_for_completion_timeout(&test->irq_raised,
 					  msecs_to_jiffies(1000));
 	if (!val)
@@ -209,6 +214,26 @@ static bool pci_endpoint_test_msi_irq(struct pci_endpoint_test *test,
 	return false;
 }
 
+static bool pci_endpoint_test_msix_irq(struct pci_endpoint_test *test,
+				       u16 msix_num)
+{
+	u32 val = COMMAND_RAISE_MSIX_IRQ;
+	struct pci_dev *pdev = test->pdev;
+
+	val |= (msix_num << MSI_NUMBER_SHIFT);
+	val |= (IRQ_TYPE_MSIX << IRQ_TYPE_SHIFT);
+	pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_COMMAND, val);
+	val = wait_for_completion_timeout(&test->irq_raised,
+					  msecs_to_jiffies(1000));
+	if (!val)
+		return false;
+
+	if (test->last_irq - pdev->irq == msix_num - 1)
+		return true;
+
+	return false;
+}
+
 static bool pci_endpoint_test_copy(struct pci_endpoint_test *test, size_t size)
 {
 	bool ret = false;
@@ -226,6 +251,7 @@ static bool pci_endpoint_test_copy(struct pci_endpoint_test *test, size_t size)
 	size_t alignment = test->alignment;
 	u32 src_crc32;
 	u32 dst_crc32;
+	u32 val = COMMAND_COPY;
 
 	if (size > SIZE_MAX - alignment)
 		goto err;
@@ -281,8 +307,9 @@ static bool pci_endpoint_test_copy(struct pci_endpoint_test *test, size_t size)
 	pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_SIZE,
 				 size);
 
-	pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_COMMAND,
-				 1 << MSI_NUMBER_SHIFT | COMMAND_COPY);
+	val |= (1 << MSI_NUMBER_SHIFT);
+	val |= (irq_type << IRQ_TYPE_SHIFT);
+	pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_COMMAND, val);
 
 	wait_for_completion(&test->irq_raised);
 
@@ -314,6 +341,7 @@ static bool pci_endpoint_test_write(struct pci_endpoint_test *test, size_t size)
 	size_t offset;
 	size_t alignment = test->alignment;
 	u32 crc32;
+	u32 val = COMMAND_READ;
 
 	if (size > SIZE_MAX - alignment)
 		goto err;
@@ -348,8 +376,9 @@ static bool pci_endpoint_test_write(struct pci_endpoint_test *test, size_t size)
 
 	pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_SIZE, size);
 
-	pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_COMMAND,
-				 1 << MSI_NUMBER_SHIFT | COMMAND_READ);
+	val |= (1 << MSI_NUMBER_SHIFT);
+	val |= (irq_type << IRQ_TYPE_SHIFT);
+	pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_COMMAND, val);
 
 	wait_for_completion(&test->irq_raised);
 
@@ -375,6 +404,7 @@ static bool pci_endpoint_test_read(struct pci_endpoint_test *test, size_t size)
 	size_t offset;
 	size_t alignment = test->alignment;
 	u32 crc32;
+	u32 val = COMMAND_WRITE;
 
 	if (size > SIZE_MAX - alignment)
 		goto err;
@@ -403,8 +433,9 @@ static bool pci_endpoint_test_read(struct pci_endpoint_test *test, size_t size)
 
 	pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_SIZE, size);
 
-	pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_COMMAND,
-				 1 << MSI_NUMBER_SHIFT | COMMAND_WRITE);
+	val |= (1 << MSI_NUMBER_SHIFT);
+	val |= (irq_type << IRQ_TYPE_SHIFT);
+	pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_COMMAND, val);
 
 	wait_for_completion(&test->irq_raised);
 
@@ -438,6 +469,9 @@ static long pci_endpoint_test_ioctl(struct file *file, unsigned int cmd,
 	case PCITEST_MSI:
 		ret = pci_endpoint_test_msi_irq(test, arg);
 		break;
+	case PCITEST_MSIX:
+		ret = pci_endpoint_test_msix_irq(test, arg);
+		break;
 	case PCITEST_WRITE:
 		ret = pci_endpoint_test_write(test, arg);
 		break;
@@ -490,7 +524,7 @@ static int pci_endpoint_test_probe(struct pci_dev *pdev,
 	if (data) {
 		test_reg_bar = data->test_reg_bar;
 		test->alignment = data->alignment;
-		no_msi = data->no_msi;
+		irq_type = data->irq_type;
 	}
 
 	init_completion(&test->irq_raised);
@@ -510,11 +544,24 @@ static int pci_endpoint_test_probe(struct pci_dev *pdev,
 
 	pci_set_master(pdev);
 
-	if (!no_msi) {
+	switch (irq_type) {
+	case IRQ_TYPE_LEGACY:
+		break;
+	case IRQ_TYPE_MSI:
 		irq = pci_alloc_irq_vectors(pdev, 1, 32, PCI_IRQ_MSI);
 		if (irq < 0)
 			dev_err(dev, "failed to get MSI interrupts\n");
 		test->num_irqs = irq;
+		break;
+	case IRQ_TYPE_MSIX:
+		irq = pci_alloc_irq_vectors(pdev, 1, 2048, PCI_IRQ_MSIX);
+		if (irq < 0)
+			dev_err(dev, "Failed to get MSI-X interrupts\n");
+		test->num_irqs = irq;
+		break;
+	default:
+		dev_err(dev, "Invalid IRQ type selected\n");
+		goto err_disable_msi;
 	}
 
 	err = devm_request_irq(dev, pdev->irq, pci_endpoint_test_irqhandler,
@@ -529,8 +576,9 @@ static int pci_endpoint_test_probe(struct pci_dev *pdev,
 				       pci_endpoint_test_irqhandler,
 				       IRQF_SHARED, DRV_MODULE_NAME, test);
 		if (err)
-			dev_err(dev, "failed to request IRQ %d for MSI %d\n",
-				pdev->irq + i, i + 1);
+			dev_err(dev, "Failed to request IRQ %d for MSI%s %d\n",
+				pdev->irq + i,
+				irq_type == IRQ_TYPE_MSIX ? "-X" : "", i + 1);
 	}
 
 	for (bar = BAR_0; bar <= BAR_5; bar++) {
@@ -596,6 +644,7 @@ static int pci_endpoint_test_probe(struct pci_dev *pdev,
 
 err_disable_msi:
 	pci_disable_msi(pdev);
+	pci_disable_msix(pdev);
 	pci_release_regions(pdev);
 
 err_disable_pdev:
@@ -627,6 +676,7 @@ static void pci_endpoint_test_remove(struct pci_dev *pdev)
 	for (i = 0; i < test->num_irqs; i++)
 		devm_free_irq(&pdev->dev, pdev->irq + i, test);
 	pci_disable_msi(pdev);
+	pci_disable_msix(pdev);
 	pci_release_regions(pdev);
 	pci_disable_device(pdev);
 }
-- 
2.7.4

Re: [RFC 06/10] misc: pci_endpoint_test: Add MSI-X support

[Kishon Vijay Abraham I]

Hi,

On Tuesday 10 April 2018 10:44 PM, Gustavo Pimentel wrote:
> Adds the MSI-X support and updates driver documentation accordingly.
> 
> Changes the driver parameter in order to allow the interruption type
> selection.
> 
> Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
> ---
>  Documentation/misc-devices/pci-endpoint-test.txt |   3 +
>  drivers/misc/pci_endpoint_test.c                 | 102 +++++++++++++++++------
>  2 files changed, 79 insertions(+), 26 deletions(-)
> 
> diff --git a/Documentation/misc-devices/pci-endpoint-test.txt b/Documentation/misc-devices/pci-endpoint-test.txt
> index 4ebc359..fdfa0f6 100644
> --- a/Documentation/misc-devices/pci-endpoint-test.txt
> +++ b/Documentation/misc-devices/pci-endpoint-test.txt
> @@ -10,6 +10,7 @@ The PCI driver for the test device performs the following tests
>  	*) verifying addresses programmed in BAR
>  	*) raise legacy IRQ
>  	*) raise MSI IRQ
> +	*) raise MSI-X IRQ
>  	*) read data
>  	*) write data
>  	*) copy data
> @@ -25,6 +26,8 @@ ioctl
>   PCITEST_LEGACY_IRQ: Tests legacy IRQ
>   PCITEST_MSI: Tests message signalled interrupts. The MSI number
>  	      to be tested should be passed as argument.
> + PCITEST_MSIX: Tests message signalled interrupts. The MSI-X number
> +	      to be tested should be passed as argument.
>   PCITEST_WRITE: Perform write tests. The size of the buffer should be passed
>  		as argument.
>   PCITEST_READ: Perform read tests. The size of the buffer should be passed
> diff --git a/drivers/misc/pci_endpoint_test.c b/drivers/misc/pci_endpoint_test.c
> index 37db0fc..a7d9354 100644
> --- a/drivers/misc/pci_endpoint_test.c
> +++ b/drivers/misc/pci_endpoint_test.c
> @@ -42,11 +42,16 @@
>  #define PCI_ENDPOINT_TEST_COMMAND	0x4
>  #define COMMAND_RAISE_LEGACY_IRQ	BIT(0)
>  #define COMMAND_RAISE_MSI_IRQ		BIT(1)
> -#define MSI_NUMBER_SHIFT		2
> -/* 6 bits for MSI number */
> -#define COMMAND_READ                    BIT(8)
> -#define COMMAND_WRITE                   BIT(9)
> -#define COMMAND_COPY                    BIT(10)
> +#define COMMAND_RAISE_MSIX_IRQ		BIT(2)
> +#define IRQ_TYPE_SHIFT			3
> +#define IRQ_TYPE_LEGACY			0
> +#define IRQ_TYPE_MSI			1
> +#define IRQ_TYPE_MSIX			2
> +#define MSI_NUMBER_SHIFT		5

Now that you are anyways fixing this, add a new register entry for MSI numbers.
Let's not keep COMMAND and MSI's together.
> +/* 12 bits for MSI number */
> +#define COMMAND_READ                    BIT(17)
> +#define COMMAND_WRITE                   BIT(18)
> +#define COMMAND_COPY                    BIT(19)

This change should be done along with the pci-epf-test in a single patch.
>  
>  #define PCI_ENDPOINT_TEST_STATUS	0x8
>  #define STATUS_READ_SUCCESS             BIT(0)
> @@ -73,9 +78,9 @@ static DEFINE_IDA(pci_endpoint_test_ida);
>  #define to_endpoint_test(priv) container_of((priv), struct pci_endpoint_test, \
>  					    miscdev)
>  
> -static bool no_msi;
> -module_param(no_msi, bool, 0444);
> -MODULE_PARM_DESC(no_msi, "Disable MSI interrupt in pci_endpoint_test");

Let's not remove this just to make sure existing users doesn't get affected.
> +static int irq_type = IRQ_TYPE_MSIX;
> +module_param(irq_type, int, 0444);
> +MODULE_PARM_DESC(irq_type, "IRQ mode selection in pci_endpoint_test (0 - Legacy, 1 - MSI, 2 - MSI-X)");
>  
>  enum pci_barno {
>  	BAR_0,
> @@ -103,7 +108,7 @@ struct pci_endpoint_test {
>  struct pci_endpoint_test_data {
>  	enum pci_barno test_reg_bar;
>  	size_t alignment;
> -	bool no_msi;
> +	int irq_type;
>  };
>  
>  static inline u32 pci_endpoint_test_readl(struct pci_endpoint_test *test,
> @@ -177,10 +182,10 @@ static bool pci_endpoint_test_bar(struct pci_endpoint_test *test,
>  
>  static bool pci_endpoint_test_legacy_irq(struct pci_endpoint_test *test)
>  {
> -	u32 val;
> +	u32 val = COMMAND_RAISE_LEGACY_IRQ;
>  
> -	pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_COMMAND,
> -				 COMMAND_RAISE_LEGACY_IRQ);
> +	val |= (IRQ_TYPE_LEGACY << IRQ_TYPE_SHIFT);
> +	pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_COMMAND, val);
>  	val = wait_for_completion_timeout(&test->irq_raised,
>  					  msecs_to_jiffies(1000));
>  	if (!val)
> @@ -192,12 +197,12 @@ static bool pci_endpoint_test_legacy_irq(struct pci_endpoint_test *test)
>  static bool pci_endpoint_test_msi_irq(struct pci_endpoint_test *test,
>  				      u8 msi_num)
>  {
> -	u32 val;
> +	u32 val = COMMAND_RAISE_MSI_IRQ;
>  	struct pci_dev *pdev = test->pdev;
>  
> -	pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_COMMAND,
> -				 msi_num << MSI_NUMBER_SHIFT |
> -				 COMMAND_RAISE_MSI_IRQ);
> +	val |= (msi_num << MSI_NUMBER_SHIFT);
> +	val |= (IRQ_TYPE_MSI << IRQ_TYPE_SHIFT);
> +	pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_COMMAND, val);
>  	val = wait_for_completion_timeout(&test->irq_raised,
>  					  msecs_to_jiffies(1000));
>  	if (!val)
> @@ -209,6 +214,26 @@ static bool pci_endpoint_test_msi_irq(struct pci_endpoint_test *test,
>  	return false;
>  }
>  
> +static bool pci_endpoint_test_msix_irq(struct pci_endpoint_test *test,
> +				       u16 msix_num)
> +{
> +	u32 val = COMMAND_RAISE_MSIX_IRQ;
> +	struct pci_dev *pdev = test->pdev;
> +
> +	val |= (msix_num << MSI_NUMBER_SHIFT);
> +	val |= (IRQ_TYPE_MSIX << IRQ_TYPE_SHIFT);
> +	pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_COMMAND, val);
> +	val = wait_for_completion_timeout(&test->irq_raised,
> +					  msecs_to_jiffies(1000));
> +	if (!val)
> +		return false;
> +
> +	if (test->last_irq - pdev->irq == msix_num - 1)
> +		return true;
> +
> +	return false;

I think you can have a single function for msix_irq and msi_irq.

Thanks
Kishon

Re: [RFC 06/10] misc: pci_endpoint_test: Add MSI-X support

[Gustavo Pimentel]

Hi Kishon,

On 17/04/2018 11:33, Kishon Vijay Abraham I wrote:
> Hi,
> 
> On Tuesday 10 April 2018 10:44 PM, Gustavo Pimentel wrote:
>> Adds the MSI-X support and updates driver documentation accordingly.
>>
>> Changes the driver parameter in order to allow the interruption type
>> selection.
>>
>> Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
>> ---
>>  Documentation/misc-devices/pci-endpoint-test.txt |   3 +
>>  drivers/misc/pci_endpoint_test.c                 | 102 +++++++++++++++++------
>>  2 files changed, 79 insertions(+), 26 deletions(-)
>>
>> diff --git a/Documentation/misc-devices/pci-endpoint-test.txt b/Documentation/misc-devices/pci-endpoint-test.txt
>> index 4ebc359..fdfa0f6 100644
>> --- a/Documentation/misc-devices/pci-endpoint-test.txt
>> +++ b/Documentation/misc-devices/pci-endpoint-test.txt
>> @@ -10,6 +10,7 @@ The PCI driver for the test device performs the following tests
>>  	*) verifying addresses programmed in BAR
>>  	*) raise legacy IRQ
>>  	*) raise MSI IRQ
>> +	*) raise MSI-X IRQ
>>  	*) read data
>>  	*) write data
>>  	*) copy data
>> @@ -25,6 +26,8 @@ ioctl
>>   PCITEST_LEGACY_IRQ: Tests legacy IRQ
>>   PCITEST_MSI: Tests message signalled interrupts. The MSI number
>>  	      to be tested should be passed as argument.
>> + PCITEST_MSIX: Tests message signalled interrupts. The MSI-X number
>> +	      to be tested should be passed as argument.
>>   PCITEST_WRITE: Perform write tests. The size of the buffer should be passed
>>  		as argument.
>>   PCITEST_READ: Perform read tests. The size of the buffer should be passed
>> diff --git a/drivers/misc/pci_endpoint_test.c b/drivers/misc/pci_endpoint_test.c
>> index 37db0fc..a7d9354 100644
>> --- a/drivers/misc/pci_endpoint_test.c
>> +++ b/drivers/misc/pci_endpoint_test.c
>> @@ -42,11 +42,16 @@
>>  #define PCI_ENDPOINT_TEST_COMMAND	0x4
>>  #define COMMAND_RAISE_LEGACY_IRQ	BIT(0)
>>  #define COMMAND_RAISE_MSI_IRQ		BIT(1)
>> -#define MSI_NUMBER_SHIFT		2
>> -/* 6 bits for MSI number */
>> -#define COMMAND_READ                    BIT(8)
>> -#define COMMAND_WRITE                   BIT(9)
>> -#define COMMAND_COPY                    BIT(10)
>> +#define COMMAND_RAISE_MSIX_IRQ		BIT(2)
>> +#define IRQ_TYPE_SHIFT			3
>> +#define IRQ_TYPE_LEGACY			0
>> +#define IRQ_TYPE_MSI			1
>> +#define IRQ_TYPE_MSIX			2
>> +#define MSI_NUMBER_SHIFT		5
> 
> Now that you are anyways fixing this, add a new register entry for MSI numbers.
> Let's not keep COMMAND and MSI's together.

What you suggest?

>> +/* 12 bits for MSI number */
>> +#define COMMAND_READ                    BIT(17)
>> +#define COMMAND_WRITE                   BIT(18)
>> +#define COMMAND_COPY                    BIT(19)
> 
> This change should be done along with the pci-epf-test in a single patch.

To be clear, you're saying is this patch should be just be squashed into the
patch number 8 [1], because there is a lot of dependencies namely the defines,
that is used on the alter functions.

[1] -> https://patchwork.ozlabs.org/patch/896841/

>>  
>>  #define PCI_ENDPOINT_TEST_STATUS	0x8
>>  #define STATUS_READ_SUCCESS             BIT(0)
>> @@ -73,9 +78,9 @@ static DEFINE_IDA(pci_endpoint_test_ida);
>>  #define to_endpoint_test(priv) container_of((priv), struct pci_endpoint_test, \
>>  					    miscdev)
>>  
>> -static bool no_msi;
>> -module_param(no_msi, bool, 0444);
>> -MODULE_PARM_DESC(no_msi, "Disable MSI interrupt in pci_endpoint_test");
> 
> Let's not remove this just to make sure existing users doesn't get affected.

Hum, by making an internal conversion? Like this
no_msi = false <=> irq_type = 1
no_msi = true <=> irq_type = 0

>> +static int irq_type = IRQ_TYPE_MSIX;
>> +module_param(irq_type, int, 0444);
>> +MODULE_PARM_DESC(irq_type, "IRQ mode selection in pci_endpoint_test (0 - Legacy, 1 - MSI, 2 - MSI-X)");
>>  
>>  enum pci_barno {
>>  	BAR_0,
>> @@ -103,7 +108,7 @@ struct pci_endpoint_test {
>>  struct pci_endpoint_test_data {
>>  	enum pci_barno test_reg_bar;
>>  	size_t alignment;
>> -	bool no_msi;
>> +	int irq_type;
>>  };
>>  
>>  static inline u32 pci_endpoint_test_readl(struct pci_endpoint_test *test,
>> @@ -177,10 +182,10 @@ static bool pci_endpoint_test_bar(struct pci_endpoint_test *test,
>>  
>>  static bool pci_endpoint_test_legacy_irq(struct pci_endpoint_test *test)
>>  {
>> -	u32 val;
>> +	u32 val = COMMAND_RAISE_LEGACY_IRQ;
>>  
>> -	pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_COMMAND,
>> -				 COMMAND_RAISE_LEGACY_IRQ);
>> +	val |= (IRQ_TYPE_LEGACY << IRQ_TYPE_SHIFT);
>> +	pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_COMMAND, val);
>>  	val = wait_for_completion_timeout(&test->irq_raised,
>>  					  msecs_to_jiffies(1000));
>>  	if (!val)
>> @@ -192,12 +197,12 @@ static bool pci_endpoint_test_legacy_irq(struct pci_endpoint_test *test)
>>  static bool pci_endpoint_test_msi_irq(struct pci_endpoint_test *test,
>>  				      u8 msi_num)
>>  {
>> -	u32 val;
>> +	u32 val = COMMAND_RAISE_MSI_IRQ;
>>  	struct pci_dev *pdev = test->pdev;
>>  
>> -	pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_COMMAND,
>> -				 msi_num << MSI_NUMBER_SHIFT |
>> -				 COMMAND_RAISE_MSI_IRQ);
>> +	val |= (msi_num << MSI_NUMBER_SHIFT);
>> +	val |= (IRQ_TYPE_MSI << IRQ_TYPE_SHIFT);
>> +	pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_COMMAND, val);
>>  	val = wait_for_completion_timeout(&test->irq_raised,
>>  					  msecs_to_jiffies(1000));
>>  	if (!val)
>> @@ -209,6 +214,26 @@ static bool pci_endpoint_test_msi_irq(struct pci_endpoint_test *test,
>>  	return false;
>>  }
>>  
>> +static bool pci_endpoint_test_msix_irq(struct pci_endpoint_test *test,
>> +				       u16 msix_num)
>> +{
>> +	u32 val = COMMAND_RAISE_MSIX_IRQ;
>> +	struct pci_dev *pdev = test->pdev;
>> +
>> +	val |= (msix_num << MSI_NUMBER_SHIFT);
>> +	val |= (IRQ_TYPE_MSIX << IRQ_TYPE_SHIFT);
>> +	pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_COMMAND, val);
>> +	val = wait_for_completion_timeout(&test->irq_raised,
>> +					  msecs_to_jiffies(1000));
>> +	if (!val)
>> +		return false;
>> +
>> +	if (test->last_irq - pdev->irq == msix_num - 1)
>> +		return true;
>> +
>> +	return false;
> 
> I think you can have a single function for msix_irq and msi_irq.

Ok.

> 
> Thanks
> Kishon
> 

Re: [RFC 06/10] misc: pci_endpoint_test: Add MSI-X support

[Kishon Vijay Abraham I]

Hi,

On Tuesday 17 April 2018 11:08 PM, Gustavo Pimentel wrote:
> Hi Kishon,
> 
> On 17/04/2018 11:33, Kishon Vijay Abraham I wrote:
>> Hi,
>>
>> On Tuesday 10 April 2018 10:44 PM, Gustavo Pimentel wrote:
>>> Adds the MSI-X support and updates driver documentation accordingly.
>>>
>>> Changes the driver parameter in order to allow the interruption type
>>> selection.
>>>
>>> Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
>>> ---
>>>  Documentation/misc-devices/pci-endpoint-test.txt |   3 +
>>>  drivers/misc/pci_endpoint_test.c                 | 102 +++++++++++++++++------
>>>  2 files changed, 79 insertions(+), 26 deletions(-)
>>>
>>> diff --git a/Documentation/misc-devices/pci-endpoint-test.txt b/Documentation/misc-devices/pci-endpoint-test.txt
>>> index 4ebc359..fdfa0f6 100644
>>> --- a/Documentation/misc-devices/pci-endpoint-test.txt
>>> +++ b/Documentation/misc-devices/pci-endpoint-test.txt
>>> @@ -10,6 +10,7 @@ The PCI driver for the test device performs the following tests
>>>  	*) verifying addresses programmed in BAR
>>>  	*) raise legacy IRQ
>>>  	*) raise MSI IRQ
>>> +	*) raise MSI-X IRQ
>>>  	*) read data
>>>  	*) write data
>>>  	*) copy data
>>> @@ -25,6 +26,8 @@ ioctl
>>>   PCITEST_LEGACY_IRQ: Tests legacy IRQ
>>>   PCITEST_MSI: Tests message signalled interrupts. The MSI number
>>>  	      to be tested should be passed as argument.
>>> + PCITEST_MSIX: Tests message signalled interrupts. The MSI-X number
>>> +	      to be tested should be passed as argument.
>>>   PCITEST_WRITE: Perform write tests. The size of the buffer should be passed
>>>  		as argument.
>>>   PCITEST_READ: Perform read tests. The size of the buffer should be passed
>>> diff --git a/drivers/misc/pci_endpoint_test.c b/drivers/misc/pci_endpoint_test.c
>>> index 37db0fc..a7d9354 100644
>>> --- a/drivers/misc/pci_endpoint_test.c
>>> +++ b/drivers/misc/pci_endpoint_test.c
>>> @@ -42,11 +42,16 @@
>>>  #define PCI_ENDPOINT_TEST_COMMAND	0x4
>>>  #define COMMAND_RAISE_LEGACY_IRQ	BIT(0)
>>>  #define COMMAND_RAISE_MSI_IRQ		BIT(1)
>>> -#define MSI_NUMBER_SHIFT		2
>>> -/* 6 bits for MSI number */
>>> -#define COMMAND_READ                    BIT(8)
>>> -#define COMMAND_WRITE                   BIT(9)
>>> -#define COMMAND_COPY                    BIT(10)
>>> +#define COMMAND_RAISE_MSIX_IRQ		BIT(2)
>>> +#define IRQ_TYPE_SHIFT			3
>>> +#define IRQ_TYPE_LEGACY			0
>>> +#define IRQ_TYPE_MSI			1
>>> +#define IRQ_TYPE_MSIX			2
>>> +#define MSI_NUMBER_SHIFT		5
>>
>> Now that you are anyways fixing this, add a new register entry for MSI numbers.
>> Let's not keep COMMAND and MSI's together.
> 
> What you suggest?

#define PCI_ENDPOINT_TEST_COMMAND	0x4
#define COMMAND_RAISE_LEGACY_IRQ	BIT(0)
#define COMMAND_RAISE_MSI_IRQ		BIT(1)
#define COMMAND_RAISE_MSIX_IRQ		BIT(2)
#define COMMAND_READ                    BIT(3)
#define COMMAND_WRITE                   BIT(4)
#define COMMAND_COPY                    BIT(5)

#define PCI_ENDPOINT_TEST_STATUS	0x8
#define STATUS_READ_SUCCESS             BIT(0)
#define STATUS_READ_FAIL                BIT(1)
#define STATUS_WRITE_SUCCESS            BIT(2)
#define STATUS_WRITE_FAIL               BIT(3)
#define STATUS_COPY_SUCCESS             BIT(4)
#define STATUS_COPY_FAIL                BIT(5)
#define STATUS_IRQ_RAISED               BIT(6)
#define STATUS_SRC_ADDR_INVALID         BIT(7)
#define STATUS_DST_ADDR_INVALID         BIT(8)

#define PCI_ENDPOINT_TEST_LOWER_SRC_ADDR	0xc
#define PCI_ENDPOINT_TEST_UPPER_SRC_ADDR	0x10

#define PCI_ENDPOINT_TEST_LOWER_DST_ADDR	0x14
#define PCI_ENDPOINT_TEST_UPPER_DST_ADDR	0x18

#define PCI_ENDPOINT_TEST_SIZE		0x1c
#define PCI_ENDPOINT_TEST_CHECKSUM	0x20

#define PCI_ENDPOINT_TEST_MSI_NUMBER	0x24

We should try not to modify either the existing register offsets or the bit
fields within these registers in the future as EP and RC will be running on
different systems and it is possible one of them might not have the updated
kernel.
> 
>>> +/* 12 bits for MSI number */
>>> +#define COMMAND_READ                    BIT(17)
>>> +#define COMMAND_WRITE                   BIT(18)
>>> +#define COMMAND_COPY                    BIT(19)
>>
>> This change should be done along with the pci-epf-test in a single patch.
> 
> To be clear, you're saying is this patch should be just be squashed into the
> patch number 8 [1], because there is a lot of dependencies namely the defines,
> that is used on the alter functions.
> 
> [1] -> https://patchwork.ozlabs.org/patch/896841/

yeah. We have to make sure git bisect doesn't break functionality.
> 
>>>  
>>>  #define PCI_ENDPOINT_TEST_STATUS	0x8
>>>  #define STATUS_READ_SUCCESS             BIT(0)
>>> @@ -73,9 +78,9 @@ static DEFINE_IDA(pci_endpoint_test_ida);
>>>  #define to_endpoint_test(priv) container_of((priv), struct pci_endpoint_test, \
>>>  					    miscdev)
>>>  
>>> -static bool no_msi;
>>> -module_param(no_msi, bool, 0444);
>>> -MODULE_PARM_DESC(no_msi, "Disable MSI interrupt in pci_endpoint_test");
>>
>> Let's not remove this just to make sure existing users doesn't get affected.
> 
> Hum, by making an internal conversion? Like this
> no_msi = false <=> irq_type = 1
> no_msi = true <=> irq_type = 0

yeah..

Thanks
Kishon

Re: [RFC 06/10] misc: pci_endpoint_test: Add MSI-X support

[Gustavo Pimentel]

Hi Kishon,

On 24/04/2018 08:19, Kishon Vijay Abraham I wrote:
> Hi,
> 
> On Tuesday 17 April 2018 11:08 PM, Gustavo Pimentel wrote:
>> Hi Kishon,
>>
>> On 17/04/2018 11:33, Kishon Vijay Abraham I wrote:
>>> Hi,
>>>
>>> On Tuesday 10 April 2018 10:44 PM, Gustavo Pimentel wrote:
>>>> Adds the MSI-X support and updates driver documentation accordingly.
>>>>
>>>> Changes the driver parameter in order to allow the interruption type
>>>> selection.
>>>>
>>>> Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
>>>> ---
>>>>  Documentation/misc-devices/pci-endpoint-test.txt |   3 +
>>>>  drivers/misc/pci_endpoint_test.c                 | 102 +++++++++++++++++------
>>>>  2 files changed, 79 insertions(+), 26 deletions(-)
>>>>
>>>> diff --git a/Documentation/misc-devices/pci-endpoint-test.txt b/Documentation/misc-devices/pci-endpoint-test.txt
>>>> index 4ebc359..fdfa0f6 100644
>>>> --- a/Documentation/misc-devices/pci-endpoint-test.txt
>>>> +++ b/Documentation/misc-devices/pci-endpoint-test.txt
>>>> @@ -10,6 +10,7 @@ The PCI driver for the test device performs the following tests
>>>>  	*) verifying addresses programmed in BAR
>>>>  	*) raise legacy IRQ
>>>>  	*) raise MSI IRQ
>>>> +	*) raise MSI-X IRQ
>>>>  	*) read data
>>>>  	*) write data
>>>>  	*) copy data
>>>> @@ -25,6 +26,8 @@ ioctl
>>>>   PCITEST_LEGACY_IRQ: Tests legacy IRQ
>>>>   PCITEST_MSI: Tests message signalled interrupts. The MSI number
>>>>  	      to be tested should be passed as argument.
>>>> + PCITEST_MSIX: Tests message signalled interrupts. The MSI-X number
>>>> +	      to be tested should be passed as argument.
>>>>   PCITEST_WRITE: Perform write tests. The size of the buffer should be passed
>>>>  		as argument.
>>>>   PCITEST_READ: Perform read tests. The size of the buffer should be passed
>>>> diff --git a/drivers/misc/pci_endpoint_test.c b/drivers/misc/pci_endpoint_test.c
>>>> index 37db0fc..a7d9354 100644
>>>> --- a/drivers/misc/pci_endpoint_test.c
>>>> +++ b/drivers/misc/pci_endpoint_test.c
>>>> @@ -42,11 +42,16 @@
>>>>  #define PCI_ENDPOINT_TEST_COMMAND	0x4
>>>>  #define COMMAND_RAISE_LEGACY_IRQ	BIT(0)
>>>>  #define COMMAND_RAISE_MSI_IRQ		BIT(1)
>>>> -#define MSI_NUMBER_SHIFT		2
>>>> -/* 6 bits for MSI number */
>>>> -#define COMMAND_READ                    BIT(8)
>>>> -#define COMMAND_WRITE                   BIT(9)
>>>> -#define COMMAND_COPY                    BIT(10)
>>>> +#define COMMAND_RAISE_MSIX_IRQ		BIT(2)
>>>> +#define IRQ_TYPE_SHIFT			3
>>>> +#define IRQ_TYPE_LEGACY			0
>>>> +#define IRQ_TYPE_MSI			1
>>>> +#define IRQ_TYPE_MSIX			2
>>>> +#define MSI_NUMBER_SHIFT		5
>>>
>>> Now that you are anyways fixing this, add a new register entry for MSI numbers.
>>> Let's not keep COMMAND and MSI's together.
>>
>> What you suggest?
> 
> #define PCI_ENDPOINT_TEST_COMMAND	0x4
> #define COMMAND_RAISE_LEGACY_IRQ	BIT(0)
> #define COMMAND_RAISE_MSI_IRQ		BIT(1)
> #define COMMAND_RAISE_MSIX_IRQ		BIT(2)
> #define COMMAND_READ                    BIT(3)
> #define COMMAND_WRITE                   BIT(4)
> #define COMMAND_COPY                    BIT(5)
> 
> #define PCI_ENDPOINT_TEST_STATUS	0x8
> #define STATUS_READ_SUCCESS             BIT(0)
> #define STATUS_READ_FAIL                BIT(1)
> #define STATUS_WRITE_SUCCESS            BIT(2)
> #define STATUS_WRITE_FAIL               BIT(3)
> #define STATUS_COPY_SUCCESS             BIT(4)
> #define STATUS_COPY_FAIL                BIT(5)
> #define STATUS_IRQ_RAISED               BIT(6)
> #define STATUS_SRC_ADDR_INVALID         BIT(7)
> #define STATUS_DST_ADDR_INVALID         BIT(8)
> 
> #define PCI_ENDPOINT_TEST_LOWER_SRC_ADDR	0xc
> #define PCI_ENDPOINT_TEST_UPPER_SRC_ADDR	0x10
> 
> #define PCI_ENDPOINT_TEST_LOWER_DST_ADDR	0x14
> #define PCI_ENDPOINT_TEST_UPPER_DST_ADDR	0x18
> 
> #define PCI_ENDPOINT_TEST_SIZE		0x1c
> #define PCI_ENDPOINT_TEST_CHECKSUM	0x20
> 
> #define PCI_ENDPOINT_TEST_MSI_NUMBER	0x24

Ok. I will do it.

> 
> We should try not to modify either the existing register offsets or the bit
> fields within these registers in the future as EP and RC will be running on
> different systems and it is possible one of them might not have the updated
> kernel.

I totally agree.

>>
>>>> +/* 12 bits for MSI number */
>>>> +#define COMMAND_READ                    BIT(17)
>>>> +#define COMMAND_WRITE                   BIT(18)
>>>> +#define COMMAND_COPY                    BIT(19)
>>>
>>> This change should be done along with the pci-epf-test in a single patch.
>>
>> To be clear, you're saying is this patch should be just be squashed into the
>> patch number 8 [1], because there is a lot of dependencies namely the defines,
>> that is used on the alter functions.
>>
>> [1] -> https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.ozlabs.org_patch_896841_&d=DwIC-g&c=DPL6_X_6JkXFx7AXWqB0tg&r=bkWxpLoW-f-E3EdiDCCa0_h0PicsViasSlvIpzZvPxs&m=8urVwHCybXa1XMxsEbwHZAzzaEI_HJGXqmWgXpXb9TY&s=MRVr2YPY2Bk_WNFOxBfU4FGrFReTKdPhfzNDLiVxDbs&e=
> 
> yeah. We have to make sure git bisect doesn't break functionality.

Ok, it'll be squashed.

>>
>>>>  
>>>>  #define PCI_ENDPOINT_TEST_STATUS	0x8
>>>>  #define STATUS_READ_SUCCESS             BIT(0)
>>>> @@ -73,9 +78,9 @@ static DEFINE_IDA(pci_endpoint_test_ida);
>>>>  #define to_endpoint_test(priv) container_of((priv), struct pci_endpoint_test, \
>>>>  					    miscdev)
>>>>  
>>>> -static bool no_msi;
>>>> -module_param(no_msi, bool, 0444);
>>>> -MODULE_PARM_DESC(no_msi, "Disable MSI interrupt in pci_endpoint_test");
>>>
>>> Let's not remove this just to make sure existing users doesn't get affected.
>>
>> Hum, by making an internal conversion? Like this
>> no_msi = false <=> irq_type = 1
>> no_msi = true <=> irq_type = 0
> 
Disregard previous comment, it doesn't make sense. I don't know where my head was.

It will be like this on probe:

if (no_msi)
	irq_type = IRQ_TYPE_LEGACY;

However since we are breaking the compatibility on terms of MSI/MSI-X
bits/registers (discussion on the top), it makes sense to keep the compatibility
on this parameter?

> yeah..
> 
> Thanks
> Kishon
> 

Regards,
Gustavo

Re: [RFC 06/10] misc: pci_endpoint_test: Add MSI-X support

[Kishon Vijay Abraham I]

Hi,

On Tuesday 24 April 2018 04:27 PM, Gustavo Pimentel wrote:
> Hi Kishon,
> 
> On 24/04/2018 08:19, Kishon Vijay Abraham I wrote:
>> Hi,
>>
>> On Tuesday 17 April 2018 11:08 PM, Gustavo Pimentel wrote:
>>> Hi Kishon,
>>>
>>> On 17/04/2018 11:33, Kishon Vijay Abraham I wrote:
>>>> Hi,
>>>>
>>>> On Tuesday 10 April 2018 10:44 PM, Gustavo Pimentel wrote:
>>>>> Adds the MSI-X support and updates driver documentation accordingly.
>>>>>
>>>>> Changes the driver parameter in order to allow the interruption type
>>>>> selection.
>>>>>
>>>>> Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
>>>>> ---
>>>>>  Documentation/misc-devices/pci-endpoint-test.txt |   3 +
>>>>>  drivers/misc/pci_endpoint_test.c                 | 102 +++++++++++++++++------
>>>>>  2 files changed, 79 insertions(+), 26 deletions(-)
>>>>>
>>>>> diff --git a/Documentation/misc-devices/pci-endpoint-test.txt b/Documentation/misc-devices/pci-endpoint-test.txt
>>>>> index 4ebc359..fdfa0f6 100644
>>>>> --- a/Documentation/misc-devices/pci-endpoint-test.txt
>>>>> +++ b/Documentation/misc-devices/pci-endpoint-test.txt
>>>>> @@ -10,6 +10,7 @@ The PCI driver for the test device performs the following tests
>>>>>  	*) verifying addresses programmed in BAR
>>>>>  	*) raise legacy IRQ
>>>>>  	*) raise MSI IRQ
>>>>> +	*) raise MSI-X IRQ
>>>>>  	*) read data
>>>>>  	*) write data
>>>>>  	*) copy data
>>>>> @@ -25,6 +26,8 @@ ioctl
>>>>>   PCITEST_LEGACY_IRQ: Tests legacy IRQ
>>>>>   PCITEST_MSI: Tests message signalled interrupts. The MSI number
>>>>>  	      to be tested should be passed as argument.
>>>>> + PCITEST_MSIX: Tests message signalled interrupts. The MSI-X number
>>>>> +	      to be tested should be passed as argument.
>>>>>   PCITEST_WRITE: Perform write tests. The size of the buffer should be passed
>>>>>  		as argument.
>>>>>   PCITEST_READ: Perform read tests. The size of the buffer should be passed
>>>>> diff --git a/drivers/misc/pci_endpoint_test.c b/drivers/misc/pci_endpoint_test.c
>>>>> index 37db0fc..a7d9354 100644
>>>>> --- a/drivers/misc/pci_endpoint_test.c
>>>>> +++ b/drivers/misc/pci_endpoint_test.c
>>>>> @@ -42,11 +42,16 @@
>>>>>  #define PCI_ENDPOINT_TEST_COMMAND	0x4
>>>>>  #define COMMAND_RAISE_LEGACY_IRQ	BIT(0)
>>>>>  #define COMMAND_RAISE_MSI_IRQ		BIT(1)
>>>>> -#define MSI_NUMBER_SHIFT		2
>>>>> -/* 6 bits for MSI number */
>>>>> -#define COMMAND_READ                    BIT(8)
>>>>> -#define COMMAND_WRITE                   BIT(9)
>>>>> -#define COMMAND_COPY                    BIT(10)
>>>>> +#define COMMAND_RAISE_MSIX_IRQ		BIT(2)
>>>>> +#define IRQ_TYPE_SHIFT			3
>>>>> +#define IRQ_TYPE_LEGACY			0
>>>>> +#define IRQ_TYPE_MSI			1
>>>>> +#define IRQ_TYPE_MSIX			2
>>>>> +#define MSI_NUMBER_SHIFT		5
>>>>
>>>> Now that you are anyways fixing this, add a new register entry for MSI numbers.
>>>> Let's not keep COMMAND and MSI's together.
>>>
>>> What you suggest?
>>
>> #define PCI_ENDPOINT_TEST_COMMAND	0x4
>> #define COMMAND_RAISE_LEGACY_IRQ	BIT(0)
>> #define COMMAND_RAISE_MSI_IRQ		BIT(1)
>> #define COMMAND_RAISE_MSIX_IRQ		BIT(2)
>> #define COMMAND_READ                    BIT(3)
>> #define COMMAND_WRITE                   BIT(4)
>> #define COMMAND_COPY                    BIT(5)
>>
>> #define PCI_ENDPOINT_TEST_STATUS	0x8
>> #define STATUS_READ_SUCCESS             BIT(0)
>> #define STATUS_READ_FAIL                BIT(1)
>> #define STATUS_WRITE_SUCCESS            BIT(2)
>> #define STATUS_WRITE_FAIL               BIT(3)
>> #define STATUS_COPY_SUCCESS             BIT(4)
>> #define STATUS_COPY_FAIL                BIT(5)
>> #define STATUS_IRQ_RAISED               BIT(6)
>> #define STATUS_SRC_ADDR_INVALID         BIT(7)
>> #define STATUS_DST_ADDR_INVALID         BIT(8)
>>
>> #define PCI_ENDPOINT_TEST_LOWER_SRC_ADDR	0xc
>> #define PCI_ENDPOINT_TEST_UPPER_SRC_ADDR	0x10
>>
>> #define PCI_ENDPOINT_TEST_LOWER_DST_ADDR	0x14
>> #define PCI_ENDPOINT_TEST_UPPER_DST_ADDR	0x18
>>
>> #define PCI_ENDPOINT_TEST_SIZE		0x1c
>> #define PCI_ENDPOINT_TEST_CHECKSUM	0x20
>>
>> #define PCI_ENDPOINT_TEST_MSI_NUMBER	0x24
> 
> Ok. I will do it.
> 
>>
>> We should try not to modify either the existing register offsets or the bit
>> fields within these registers in the future as EP and RC will be running on
>> different systems and it is possible one of them might not have the updated
>> kernel.
> 
> I totally agree.
> 
>>>
>>>>> +/* 12 bits for MSI number */
>>>>> +#define COMMAND_READ                    BIT(17)
>>>>> +#define COMMAND_WRITE                   BIT(18)
>>>>> +#define COMMAND_COPY                    BIT(19)
>>>>
>>>> This change should be done along with the pci-epf-test in a single patch.
>>>
>>> To be clear, you're saying is this patch should be just be squashed into the
>>> patch number 8 [1], because there is a lot of dependencies namely the defines,
>>> that is used on the alter functions.
>>>
>>> [1] -> https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.ozlabs.org_patch_896841_&d=DwIC-g&c=DPL6_X_6JkXFx7AXWqB0tg&r=bkWxpLoW-f-E3EdiDCCa0_h0PicsViasSlvIpzZvPxs&m=8urVwHCybXa1XMxsEbwHZAzzaEI_HJGXqmWgXpXb9TY&s=MRVr2YPY2Bk_WNFOxBfU4FGrFReTKdPhfzNDLiVxDbs&e=
>>
>> yeah. We have to make sure git bisect doesn't break functionality.
> 
> Ok, it'll be squashed.
> 
>>>
>>>>>  
>>>>>  #define PCI_ENDPOINT_TEST_STATUS	0x8
>>>>>  #define STATUS_READ_SUCCESS             BIT(0)
>>>>> @@ -73,9 +78,9 @@ static DEFINE_IDA(pci_endpoint_test_ida);
>>>>>  #define to_endpoint_test(priv) container_of((priv), struct pci_endpoint_test, \
>>>>>  					    miscdev)
>>>>>  
>>>>> -static bool no_msi;
>>>>> -module_param(no_msi, bool, 0444);
>>>>> -MODULE_PARM_DESC(no_msi, "Disable MSI interrupt in pci_endpoint_test");
>>>>
>>>> Let's not remove this just to make sure existing users doesn't get affected.
>>>
>>> Hum, by making an internal conversion? Like this
>>> no_msi = false <=> irq_type = 1
>>> no_msi = true <=> irq_type = 0
>>
> Disregard previous comment, it doesn't make sense. I don't know where my head was.
> 
> It will be like this on probe:
> 
> if (no_msi)
> 	irq_type = IRQ_TYPE_LEGACY;
> 
> However since we are breaking the compatibility on terms of MSI/MSI-X
> bits/registers (discussion on the top), it makes sense to keep the compatibility
> on this parameter?

This is userspace compatibility, so lets not break it.
Btw can we have a sysfs entry per device for defining irq_type. Having a sysfs
entry might be helpful instead of insmod/rmmod with different irq_type values?
It will also help if a system has enumerated multiple PCI_ENDPOINT_TEST EP devices.

Thanks
Kishon

Re: [RFC 06/10] misc: pci_endpoint_test: Add MSI-X support

[Gustavo Pimentel]

Hi Kishon,

On 24/04/2018 12:43, Kishon Vijay Abraham I wrote:
> Hi,
> 
> On Tuesday 24 April 2018 04:27 PM, Gustavo Pimentel wrote:
>> Hi Kishon,
>>
>> On 24/04/2018 08:19, Kishon Vijay Abraham I wrote:
>>> Hi,
>>>
>>> On Tuesday 17 April 2018 11:08 PM, Gustavo Pimentel wrote:
>>>> Hi Kishon,
>>>>
>>>> On 17/04/2018 11:33, Kishon Vijay Abraham I wrote:
>>>>> Hi,
>>>>>
>>>>> On Tuesday 10 April 2018 10:44 PM, Gustavo Pimentel wrote:
>>>>>> Adds the MSI-X support and updates driver documentation accordingly.
>>>>>>
>>>>>> Changes the driver parameter in order to allow the interruption type
>>>>>> selection.
>>>>>>
>>>>>> Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
>>>>>> ---
>>>>>>  Documentation/misc-devices/pci-endpoint-test.txt |   3 +
>>>>>>  drivers/misc/pci_endpoint_test.c                 | 102 +++++++++++++++++------
>>>>>>  2 files changed, 79 insertions(+), 26 deletions(-)
>>>>>>
>>>>>> diff --git a/Documentation/misc-devices/pci-endpoint-test.txt b/Documentation/misc-devices/pci-endpoint-test.txt
>>>>>> index 4ebc359..fdfa0f6 100644
>>>>>> --- a/Documentation/misc-devices/pci-endpoint-test.txt
>>>>>> +++ b/Documentation/misc-devices/pci-endpoint-test.txt
>>>>>> @@ -10,6 +10,7 @@ The PCI driver for the test device performs the following tests
>>>>>>  	*) verifying addresses programmed in BAR
>>>>>>  	*) raise legacy IRQ
>>>>>>  	*) raise MSI IRQ
>>>>>> +	*) raise MSI-X IRQ
>>>>>>  	*) read data
>>>>>>  	*) write data
>>>>>>  	*) copy data
>>>>>> @@ -25,6 +26,8 @@ ioctl
>>>>>>   PCITEST_LEGACY_IRQ: Tests legacy IRQ
>>>>>>   PCITEST_MSI: Tests message signalled interrupts. The MSI number
>>>>>>  	      to be tested should be passed as argument.
>>>>>> + PCITEST_MSIX: Tests message signalled interrupts. The MSI-X number
>>>>>> +	      to be tested should be passed as argument.
>>>>>>   PCITEST_WRITE: Perform write tests. The size of the buffer should be passed
>>>>>>  		as argument.
>>>>>>   PCITEST_READ: Perform read tests. The size of the buffer should be passed
>>>>>> diff --git a/drivers/misc/pci_endpoint_test.c b/drivers/misc/pci_endpoint_test.c
>>>>>> index 37db0fc..a7d9354 100644
>>>>>> --- a/drivers/misc/pci_endpoint_test.c
>>>>>> +++ b/drivers/misc/pci_endpoint_test.c
>>>>>> @@ -42,11 +42,16 @@
>>>>>>  #define PCI_ENDPOINT_TEST_COMMAND	0x4
>>>>>>  #define COMMAND_RAISE_LEGACY_IRQ	BIT(0)
>>>>>>  #define COMMAND_RAISE_MSI_IRQ		BIT(1)
>>>>>> -#define MSI_NUMBER_SHIFT		2
>>>>>> -/* 6 bits for MSI number */
>>>>>> -#define COMMAND_READ                    BIT(8)
>>>>>> -#define COMMAND_WRITE                   BIT(9)
>>>>>> -#define COMMAND_COPY                    BIT(10)
>>>>>> +#define COMMAND_RAISE_MSIX_IRQ		BIT(2)
>>>>>> +#define IRQ_TYPE_SHIFT			3
>>>>>> +#define IRQ_TYPE_LEGACY			0
>>>>>> +#define IRQ_TYPE_MSI			1
>>>>>> +#define IRQ_TYPE_MSIX			2
>>>>>> +#define MSI_NUMBER_SHIFT		5
>>>>>
>>>>> Now that you are anyways fixing this, add a new register entry for MSI numbers.
>>>>> Let's not keep COMMAND and MSI's together.
>>>>
>>>> What you suggest?
>>>
>>> #define PCI_ENDPOINT_TEST_COMMAND	0x4
>>> #define COMMAND_RAISE_LEGACY_IRQ	BIT(0)
>>> #define COMMAND_RAISE_MSI_IRQ		BIT(1)
>>> #define COMMAND_RAISE_MSIX_IRQ		BIT(2)
>>> #define COMMAND_READ                    BIT(3)
>>> #define COMMAND_WRITE                   BIT(4)
>>> #define COMMAND_COPY                    BIT(5)
>>>
>>> #define PCI_ENDPOINT_TEST_STATUS	0x8
>>> #define STATUS_READ_SUCCESS             BIT(0)
>>> #define STATUS_READ_FAIL                BIT(1)
>>> #define STATUS_WRITE_SUCCESS            BIT(2)
>>> #define STATUS_WRITE_FAIL               BIT(3)
>>> #define STATUS_COPY_SUCCESS             BIT(4)
>>> #define STATUS_COPY_FAIL                BIT(5)
>>> #define STATUS_IRQ_RAISED               BIT(6)
>>> #define STATUS_SRC_ADDR_INVALID         BIT(7)
>>> #define STATUS_DST_ADDR_INVALID         BIT(8)
>>>
>>> #define PCI_ENDPOINT_TEST_LOWER_SRC_ADDR	0xc
>>> #define PCI_ENDPOINT_TEST_UPPER_SRC_ADDR	0x10
>>>
>>> #define PCI_ENDPOINT_TEST_LOWER_DST_ADDR	0x14
>>> #define PCI_ENDPOINT_TEST_UPPER_DST_ADDR	0x18
>>>
>>> #define PCI_ENDPOINT_TEST_SIZE		0x1c
>>> #define PCI_ENDPOINT_TEST_CHECKSUM	0x20
>>>
>>> #define PCI_ENDPOINT_TEST_MSI_NUMBER	0x24
>>
>> Ok. I will do it.
>>
>>>
>>> We should try not to modify either the existing register offsets or the bit
>>> fields within these registers in the future as EP and RC will be running on
>>> different systems and it is possible one of them might not have the updated
>>> kernel.
>>
>> I totally agree.
>>
>>>>
>>>>>> +/* 12 bits for MSI number */
>>>>>> +#define COMMAND_READ                    BIT(17)
>>>>>> +#define COMMAND_WRITE                   BIT(18)
>>>>>> +#define COMMAND_COPY                    BIT(19)
>>>>>
>>>>> This change should be done along with the pci-epf-test in a single patch.
>>>>
>>>> To be clear, you're saying is this patch should be just be squashed into the
>>>> patch number 8 [1], because there is a lot of dependencies namely the defines,
>>>> that is used on the alter functions.
>>>>
>>>> [1] -> https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.ozlabs.org_patch_896841_&d=DwIC-g&c=DPL6_X_6JkXFx7AXWqB0tg&r=bkWxpLoW-f-E3EdiDCCa0_h0PicsViasSlvIpzZvPxs&m=8urVwHCybXa1XMxsEbwHZAzzaEI_HJGXqmWgXpXb9TY&s=MRVr2YPY2Bk_WNFOxBfU4FGrFReTKdPhfzNDLiVxDbs&e=
>>>
>>> yeah. We have to make sure git bisect doesn't break functionality.
>>
>> Ok, it'll be squashed.
>>
>>>>
>>>>>>  
>>>>>>  #define PCI_ENDPOINT_TEST_STATUS	0x8
>>>>>>  #define STATUS_READ_SUCCESS             BIT(0)
>>>>>> @@ -73,9 +78,9 @@ static DEFINE_IDA(pci_endpoint_test_ida);
>>>>>>  #define to_endpoint_test(priv) container_of((priv), struct pci_endpoint_test, \
>>>>>>  					    miscdev)
>>>>>>  
>>>>>> -static bool no_msi;
>>>>>> -module_param(no_msi, bool, 0444);
>>>>>> -MODULE_PARM_DESC(no_msi, "Disable MSI interrupt in pci_endpoint_test");
>>>>>
>>>>> Let's not remove this just to make sure existing users doesn't get affected.
>>>>
>>>> Hum, by making an internal conversion? Like this
>>>> no_msi = false <=> irq_type = 1
>>>> no_msi = true <=> irq_type = 0
>>>
>> Disregard previous comment, it doesn't make sense. I don't know where my head was.
>>
>> It will be like this on probe:
>>
>> if (no_msi)
>> 	irq_type = IRQ_TYPE_LEGACY;
>>
>> However since we are breaking the compatibility on terms of MSI/MSI-X
>> bits/registers (discussion on the top), it makes sense to keep the compatibility
>> on this parameter?
> 
> This is userspace compatibility, so lets not break it.
> Btw can we have a sysfs entry per device for defining irq_type. Having a sysfs
> entry might be helpful instead of insmod/rmmod with different irq_type values?

Can you explain it? An sysfs entry where, on RC side? How this will work with
the irq allocation/deallocation in runtime?

> It will also help if a system has enumerated multiple PCI_ENDPOINT_TEST EP devices.

Can you elaborate more this idea?

> 
> Thanks
> Kishon
> 

Regards,
Gustavo

RE: [RFC 06/10] misc: pci_endpoint_test: Add MSI-X support

[Alan Douglas]

Hi Gustavo,

On 10 April 2018 18:15 Gustavo Pimentel wrote:
>=20
> Adds the MSI-X support and updates driver documentation accordingly.
>=20
> Changes the driver parameter in order to allow the interruption type
> selection.
>=20
> Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
> ---
>  Documentation/misc-devices/pci-endpoint-test.txt |   3 +
>  drivers/misc/pci_endpoint_test.c                 | 102 +++++++++++++++++=
------
>  2 files changed, 79 insertions(+), 26 deletions(-)
>=20
> diff --git a/Documentation/misc-devices/pci-endpoint-test.txt
> b/Documentation/misc-devices/pci-endpoint-test.txt
> index 4ebc359..fdfa0f6 100644
> --- a/Documentation/misc-devices/pci-endpoint-test.txt
> +++ b/Documentation/misc-devices/pci-endpoint-test.txt
> @@ -10,6 +10,7 @@ The PCI driver for the test device performs the
> following tests
>  	*) verifying addresses programmed in BAR
>  	*) raise legacy IRQ
>  	*) raise MSI IRQ
> +	*) raise MSI-X IRQ
>  	*) read data
>  	*) write data
>  	*) copy data
> @@ -25,6 +26,8 @@ ioctl
>   PCITEST_LEGACY_IRQ: Tests legacy IRQ
>   PCITEST_MSI: Tests message signalled interrupts. The MSI number
>  	      to be tested should be passed as argument.
> + PCITEST_MSIX: Tests message signalled interrupts. The MSI-X number
> +	      to be tested should be passed as argument.
>   PCITEST_WRITE: Perform write tests. The size of the buffer should be pa=
ssed
>  		as argument.
>   PCITEST_READ: Perform read tests. The size of the buffer should be pass=
ed
> diff --git a/drivers/misc/pci_endpoint_test.c
> b/drivers/misc/pci_endpoint_test.c
> index 37db0fc..a7d9354 100644
> --- a/drivers/misc/pci_endpoint_test.c
> +++ b/drivers/misc/pci_endpoint_test.c
> @@ -42,11 +42,16 @@
>  #define PCI_ENDPOINT_TEST_COMMAND	0x4
>  #define COMMAND_RAISE_LEGACY_IRQ	BIT(0)
>  #define COMMAND_RAISE_MSI_IRQ		BIT(1)
> -#define MSI_NUMBER_SHIFT		2
> -/* 6 bits for MSI number */
> -#define COMMAND_READ                    BIT(8)
> -#define COMMAND_WRITE                   BIT(9)
> -#define COMMAND_COPY                    BIT(10)
> +#define COMMAND_RAISE_MSIX_IRQ		BIT(2)
> +#define IRQ_TYPE_SHIFT			3
> +#define IRQ_TYPE_LEGACY			0
> +#define IRQ_TYPE_MSI			1
> +#define IRQ_TYPE_MSIX			2
> +#define MSI_NUMBER_SHIFT		5
> +/* 12 bits for MSI number */
> +#define COMMAND_READ                    BIT(17)
> +#define COMMAND_WRITE                   BIT(18)
> +#define COMMAND_COPY                    BIT(19)
>=20
>  #define PCI_ENDPOINT_TEST_STATUS	0x8
>  #define STATUS_READ_SUCCESS             BIT(0)
> @@ -73,9 +78,9 @@ static DEFINE_IDA(pci_endpoint_test_ida);
>  #define to_endpoint_test(priv) container_of((priv), struct pci_endpoint_=
test,
> \
>  					    miscdev)
>=20
> -static bool no_msi;
> -module_param(no_msi, bool, 0444);
> -MODULE_PARM_DESC(no_msi, "Disable MSI interrupt in
> pci_endpoint_test");
> +static int irq_type =3D IRQ_TYPE_MSIX;
> +module_param(irq_type, int, 0444);
> +MODULE_PARM_DESC(irq_type, "IRQ mode selection in pci_endpoint_test
> (0
> +- Legacy, 1 - MSI, 2 - MSI-X)");
>=20
>  enum pci_barno {
>  	BAR_0,
> @@ -103,7 +108,7 @@ struct pci_endpoint_test {  struct
> pci_endpoint_test_data {
>  	enum pci_barno test_reg_bar;
>  	size_t alignment;
> -	bool no_msi;
> +	int irq_type;
>  };
>=20
>  static inline u32 pci_endpoint_test_readl(struct pci_endpoint_test *test=
,
> @@ -177,10 +182,10 @@ static bool pci_endpoint_test_bar(struct
> pci_endpoint_test *test,
>=20
>  static bool pci_endpoint_test_legacy_irq(struct pci_endpoint_test *test)=
  {
> -	u32 val;
> +	u32 val =3D COMMAND_RAISE_LEGACY_IRQ;
>=20
> -	pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_COMMAND,
> -				 COMMAND_RAISE_LEGACY_IRQ);
> +	val |=3D (IRQ_TYPE_LEGACY << IRQ_TYPE_SHIFT);
> +	pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_COMMAND, val);
>  	val =3D wait_for_completion_timeout(&test->irq_raised,
>  					  msecs_to_jiffies(1000));
>  	if (!val)
> @@ -192,12 +197,12 @@ static bool pci_endpoint_test_legacy_irq(struct
> pci_endpoint_test *test)  static bool pci_endpoint_test_msi_irq(struct
> pci_endpoint_test *test,
>  				      u8 msi_num)
>  {
> -	u32 val;
> +	u32 val =3D COMMAND_RAISE_MSI_IRQ;
>  	struct pci_dev *pdev =3D test->pdev;
>=20
> -	pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_COMMAND,
> -				 msi_num << MSI_NUMBER_SHIFT |
> -				 COMMAND_RAISE_MSI_IRQ);
> +	val |=3D (msi_num << MSI_NUMBER_SHIFT);
> +	val |=3D (IRQ_TYPE_MSI << IRQ_TYPE_SHIFT);
> +	pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_COMMAND, val);
>  	val =3D wait_for_completion_timeout(&test->irq_raised,
>  					  msecs_to_jiffies(1000));
>  	if (!val)
> @@ -209,6 +214,26 @@ static bool pci_endpoint_test_msi_irq(struct
> pci_endpoint_test *test,
>  	return false;
>  }
>=20
> +static bool pci_endpoint_test_msix_irq(struct pci_endpoint_test *test,
> +				       u16 msix_num)
> +{
> +	u32 val =3D COMMAND_RAISE_MSIX_IRQ;
> +	struct pci_dev *pdev =3D test->pdev;
> +
> +	val |=3D (msix_num << MSI_NUMBER_SHIFT);
> +	val |=3D (IRQ_TYPE_MSIX << IRQ_TYPE_SHIFT);
> +	pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_COMMAND, val);
> +	val =3D wait_for_completion_timeout(&test->irq_raised,
> +					  msecs_to_jiffies(1000));
> +	if (!val)
> +		return false;
> +
> +	if (test->last_irq - pdev->irq =3D=3D msix_num - 1)
> +		return true;
I think we should use pci_irq_vector() to convert from device vector to Lin=
ux IRQ.
It can be done in pci_endpoint_test_irqhandler()
(With MSI, pdev->irq will be updated during MSI init, but it is not for MSI=
-X.)

pci_irq_vector()  should also be used for devm_request_irq() and devm_free_=
irq()
so some changes required in probe and remove.

Regards,
Alan

Re: [RFC 06/10] misc: pci_endpoint_test: Add MSI-X support

[Gustavo Pimentel]

Hi Alan,

On 24/04/2018 07:59, Alan Douglas wrote:
> Hi Gustavo,
> 
> On 10 April 2018 18:15 Gustavo Pimentel wrote:
>>
>> Adds the MSI-X support and updates driver documentation accordingly.
>>
>> Changes the driver parameter in order to allow the interruption type
>> selection.
>>
>> Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
>> ---
>>  Documentation/misc-devices/pci-endpoint-test.txt |   3 +
>>  drivers/misc/pci_endpoint_test.c                 | 102 +++++++++++++++++------
>>  2 files changed, 79 insertions(+), 26 deletions(-)
>>
>> diff --git a/Documentation/misc-devices/pci-endpoint-test.txt
>> b/Documentation/misc-devices/pci-endpoint-test.txt
>> index 4ebc359..fdfa0f6 100644
>> --- a/Documentation/misc-devices/pci-endpoint-test.txt
>> +++ b/Documentation/misc-devices/pci-endpoint-test.txt
>> @@ -10,6 +10,7 @@ The PCI driver for the test device performs the
>> following tests
>>  	*) verifying addresses programmed in BAR
>>  	*) raise legacy IRQ
>>  	*) raise MSI IRQ
>> +	*) raise MSI-X IRQ
>>  	*) read data
>>  	*) write data
>>  	*) copy data
>> @@ -25,6 +26,8 @@ ioctl
>>   PCITEST_LEGACY_IRQ: Tests legacy IRQ
>>   PCITEST_MSI: Tests message signalled interrupts. The MSI number
>>  	      to be tested should be passed as argument.
>> + PCITEST_MSIX: Tests message signalled interrupts. The MSI-X number
>> +	      to be tested should be passed as argument.
>>   PCITEST_WRITE: Perform write tests. The size of the buffer should be passed
>>  		as argument.
>>   PCITEST_READ: Perform read tests. The size of the buffer should be passed
>> diff --git a/drivers/misc/pci_endpoint_test.c
>> b/drivers/misc/pci_endpoint_test.c
>> index 37db0fc..a7d9354 100644
>> --- a/drivers/misc/pci_endpoint_test.c
>> +++ b/drivers/misc/pci_endpoint_test.c
>> @@ -42,11 +42,16 @@
>>  #define PCI_ENDPOINT_TEST_COMMAND	0x4
>>  #define COMMAND_RAISE_LEGACY_IRQ	BIT(0)
>>  #define COMMAND_RAISE_MSI_IRQ		BIT(1)
>> -#define MSI_NUMBER_SHIFT		2
>> -/* 6 bits for MSI number */
>> -#define COMMAND_READ                    BIT(8)
>> -#define COMMAND_WRITE                   BIT(9)
>> -#define COMMAND_COPY                    BIT(10)
>> +#define COMMAND_RAISE_MSIX_IRQ		BIT(2)
>> +#define IRQ_TYPE_SHIFT			3
>> +#define IRQ_TYPE_LEGACY			0
>> +#define IRQ_TYPE_MSI			1
>> +#define IRQ_TYPE_MSIX			2
>> +#define MSI_NUMBER_SHIFT		5
>> +/* 12 bits for MSI number */
>> +#define COMMAND_READ                    BIT(17)
>> +#define COMMAND_WRITE                   BIT(18)
>> +#define COMMAND_COPY                    BIT(19)
>>
>>  #define PCI_ENDPOINT_TEST_STATUS	0x8
>>  #define STATUS_READ_SUCCESS             BIT(0)
>> @@ -73,9 +78,9 @@ static DEFINE_IDA(pci_endpoint_test_ida);
>>  #define to_endpoint_test(priv) container_of((priv), struct pci_endpoint_test,
>> \
>>  					    miscdev)
>>
>> -static bool no_msi;
>> -module_param(no_msi, bool, 0444);
>> -MODULE_PARM_DESC(no_msi, "Disable MSI interrupt in
>> pci_endpoint_test");
>> +static int irq_type = IRQ_TYPE_MSIX;
>> +module_param(irq_type, int, 0444);
>> +MODULE_PARM_DESC(irq_type, "IRQ mode selection in pci_endpoint_test
>> (0
>> +- Legacy, 1 - MSI, 2 - MSI-X)");
>>
>>  enum pci_barno {
>>  	BAR_0,
>> @@ -103,7 +108,7 @@ struct pci_endpoint_test {  struct
>> pci_endpoint_test_data {
>>  	enum pci_barno test_reg_bar;
>>  	size_t alignment;
>> -	bool no_msi;
>> +	int irq_type;
>>  };
>>
>>  static inline u32 pci_endpoint_test_readl(struct pci_endpoint_test *test,
>> @@ -177,10 +182,10 @@ static bool pci_endpoint_test_bar(struct
>> pci_endpoint_test *test,
>>
>>  static bool pci_endpoint_test_legacy_irq(struct pci_endpoint_test *test)  {
>> -	u32 val;
>> +	u32 val = COMMAND_RAISE_LEGACY_IRQ;
>>
>> -	pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_COMMAND,
>> -				 COMMAND_RAISE_LEGACY_IRQ);
>> +	val |= (IRQ_TYPE_LEGACY << IRQ_TYPE_SHIFT);
>> +	pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_COMMAND, val);
>>  	val = wait_for_completion_timeout(&test->irq_raised,
>>  					  msecs_to_jiffies(1000));
>>  	if (!val)
>> @@ -192,12 +197,12 @@ static bool pci_endpoint_test_legacy_irq(struct
>> pci_endpoint_test *test)  static bool pci_endpoint_test_msi_irq(struct
>> pci_endpoint_test *test,
>>  				      u8 msi_num)
>>  {
>> -	u32 val;
>> +	u32 val = COMMAND_RAISE_MSI_IRQ;
>>  	struct pci_dev *pdev = test->pdev;
>>
>> -	pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_COMMAND,
>> -				 msi_num << MSI_NUMBER_SHIFT |
>> -				 COMMAND_RAISE_MSI_IRQ);
>> +	val |= (msi_num << MSI_NUMBER_SHIFT);
>> +	val |= (IRQ_TYPE_MSI << IRQ_TYPE_SHIFT);
>> +	pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_COMMAND, val);
>>  	val = wait_for_completion_timeout(&test->irq_raised,
>>  					  msecs_to_jiffies(1000));
>>  	if (!val)
>> @@ -209,6 +214,26 @@ static bool pci_endpoint_test_msi_irq(struct
>> pci_endpoint_test *test,
>>  	return false;
>>  }
>>
>> +static bool pci_endpoint_test_msix_irq(struct pci_endpoint_test *test,
>> +				       u16 msix_num)
>> +{
>> +	u32 val = COMMAND_RAISE_MSIX_IRQ;
>> +	struct pci_dev *pdev = test->pdev;
>> +
>> +	val |= (msix_num << MSI_NUMBER_SHIFT);
>> +	val |= (IRQ_TYPE_MSIX << IRQ_TYPE_SHIFT);
>> +	pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_COMMAND, val);
>> +	val = wait_for_completion_timeout(&test->irq_raised,
>> +					  msecs_to_jiffies(1000));
>> +	if (!val)
>> +		return false;
>> +
>> +	if (test->last_irq - pdev->irq == msix_num - 1)
>> +		return true;
> I think we should use pci_irq_vector() to convert from device vector to Linux IRQ.
> It can be done in pci_endpoint_test_irqhandler()
> (With MSI, pdev->irq will be updated during MSI init, but it is not for MSI-X.)
> 
> pci_irq_vector()  should also be used for devm_request_irq() and devm_free_irq()
> so some changes required in probe and remove.

We could try it, sounds good.
Let's see if Kishon also agrees.

> 
> Regards,
> Alan
> 

Regards,
Gustavo

[RFC 07/10] misc: pci_endpoint_test: Replace lower into upper case characters

[Gustavo Pimentel]

Replaces lower into upper case characters in comments and debug printks.

Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
---
 drivers/misc/pci_endpoint_test.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/misc/pci_endpoint_test.c b/drivers/misc/pci_endpoint_test.c
index a7d9354..7212a7d 100644
--- a/drivers/misc/pci_endpoint_test.c
+++ b/drivers/misc/pci_endpoint_test.c
@@ -259,7 +259,7 @@ static bool pci_endpoint_test_copy(struct pci_endpoint_test *test, size_t size)
 	orig_src_addr = dma_alloc_coherent(dev, size + alignment,
 					   &orig_src_phys_addr, GFP_KERNEL);
 	if (!orig_src_addr) {
-		dev_err(dev, "failed to allocate source buffer\n");
+		dev_err(dev, "Failed to allocate source buffer\n");
 		ret = false;
 		goto err;
 	}
@@ -285,7 +285,7 @@ static bool pci_endpoint_test_copy(struct pci_endpoint_test *test, size_t size)
 	orig_dst_addr = dma_alloc_coherent(dev, size + alignment,
 					   &orig_dst_phys_addr, GFP_KERNEL);
 	if (!orig_dst_addr) {
-		dev_err(dev, "failed to allocate destination address\n");
+		dev_err(dev, "Failed to allocate destination address\n");
 		ret = false;
 		goto err_orig_src_addr;
 	}
@@ -349,7 +349,7 @@ static bool pci_endpoint_test_write(struct pci_endpoint_test *test, size_t size)
 	orig_addr = dma_alloc_coherent(dev, size + alignment, &orig_phys_addr,
 				       GFP_KERNEL);
 	if (!orig_addr) {
-		dev_err(dev, "failed to allocate address\n");
+		dev_err(dev, "Failed to allocate address\n");
 		ret = false;
 		goto err;
 	}
@@ -412,7 +412,7 @@ static bool pci_endpoint_test_read(struct pci_endpoint_test *test, size_t size)
 	orig_addr = dma_alloc_coherent(dev, size + alignment, &orig_phys_addr,
 				       GFP_KERNEL);
 	if (!orig_addr) {
-		dev_err(dev, "failed to allocate destination address\n");
+		dev_err(dev, "Failed to allocate destination address\n");
 		ret = false;
 		goto err;
 	}
@@ -550,7 +550,7 @@ static int pci_endpoint_test_probe(struct pci_dev *pdev,
 	case IRQ_TYPE_MSI:
 		irq = pci_alloc_irq_vectors(pdev, 1, 32, PCI_IRQ_MSI);
 		if (irq < 0)
-			dev_err(dev, "failed to get MSI interrupts\n");
+			dev_err(dev, "Failed to get MSI interrupts\n");
 		test->num_irqs = irq;
 		break;
 	case IRQ_TYPE_MSIX:
@@ -567,7 +567,7 @@ static int pci_endpoint_test_probe(struct pci_dev *pdev,
 	err = devm_request_irq(dev, pdev->irq, pci_endpoint_test_irqhandler,
 			       IRQF_SHARED, DRV_MODULE_NAME, test);
 	if (err) {
-		dev_err(dev, "failed to request IRQ %d\n", pdev->irq);
+		dev_err(dev, "Failed to request IRQ %d\n", pdev->irq);
 		goto err_disable_msi;
 	}
 
@@ -585,7 +585,7 @@ static int pci_endpoint_test_probe(struct pci_dev *pdev,
 		if (pci_resource_flags(pdev, bar) & IORESOURCE_MEM) {
 			base = pci_ioremap_bar(pdev, bar);
 			if (!base) {
-				dev_err(dev, "failed to read BAR%d\n", bar);
+				dev_err(dev, "Failed to read BAR%d\n", bar);
 				WARN_ON(bar == test_reg_bar);
 			}
 			test->bar[bar] = base;
@@ -605,7 +605,7 @@ static int pci_endpoint_test_probe(struct pci_dev *pdev,
 	id = ida_simple_get(&pci_endpoint_test_ida, 0, 0, GFP_KERNEL);
 	if (id < 0) {
 		err = id;
-		dev_err(dev, "unable to get id\n");
+		dev_err(dev, "Unable to get id\n");
 		goto err_iounmap;
 	}
 
@@ -621,7 +621,7 @@ static int pci_endpoint_test_probe(struct pci_dev *pdev,
 
 	err = misc_register(misc_device);
 	if (err) {
-		dev_err(dev, "failed to register device\n");
+		dev_err(dev, "Failed to register device\n");
 		goto err_kfree_name;
 	}
 
-- 
2.7.4

[RFC 08/10] PCI: endpoint: functions/pci-epf-test: Add MSI-X support

[Gustavo Pimentel]

Adds driver's MSI-X support.

Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
---
 drivers/pci/endpoint/functions/pci-epf-test.c | 87 +++++++++++++++++++++------
 1 file changed, 69 insertions(+), 18 deletions(-)

diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
index 63dca44..5997c6e 100644
--- a/drivers/pci/endpoint/functions/pci-epf-test.c
+++ b/drivers/pci/endpoint/functions/pci-epf-test.c
@@ -20,11 +20,18 @@
 
 #define COMMAND_RAISE_LEGACY_IRQ	BIT(0)
 #define COMMAND_RAISE_MSI_IRQ		BIT(1)
-#define MSI_NUMBER_SHIFT		2
+#define COMMAND_RAISE_MSIX_IRQ		BIT(2)
+#define IRQ_TYPE_SHIFT			3
+#define MSI_NUMBER_SHIFT		5
+#define IRQ_TYPE_MASK			(0x3 << IRQ_TYPE_SHIFT)
+#define IRQ_TYPE_LEGACY			0
+#define IRQ_TYPE_MSI			1
+#define IRQ_TYPE_MSIX			2
 #define MSI_NUMBER_MASK			(0x3f << MSI_NUMBER_SHIFT)
-#define COMMAND_READ			BIT(8)
-#define COMMAND_WRITE			BIT(9)
-#define COMMAND_COPY			BIT(10)
+#define MSIX_NUMBER_MASK		(0xfff << MSI_NUMBER_SHIFT)
+#define COMMAND_READ			BIT(17)
+#define COMMAND_WRITE			BIT(18)
+#define COMMAND_COPY			BIT(19)
 
 #define STATUS_READ_SUCCESS		BIT(0)
 #define STATUS_READ_FAIL		BIT(1)
@@ -244,31 +251,44 @@ static int pci_epf_test_write(struct pci_epf_test *epf_test)
 	return ret;
 }
 
-static void pci_epf_test_raise_irq(struct pci_epf_test *epf_test, u8 irq)
+static void pci_epf_test_raise_irq(struct pci_epf_test *epf_test, u8 irq_type,
+				   u16 irq)
 {
-	u8 msi_count;
 	struct pci_epf *epf = epf_test->epf;
+	struct device *dev = &epf->dev;
 	struct pci_epc *epc = epf->epc;
 	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_IRQ_RAISED;
-	msi_count = pci_epc_get_msi(epc, epf->func_no);
-	if (irq > msi_count || msi_count <= 0)
-		pci_epc_raise_irq(epc, epf->func_no, PCI_EPC_IRQ_LEGACY, 0);
-	else
+
+	switch (irq_type) {
+	case IRQ_TYPE_LEGACY:
+		pci_epc_raise_irq(epc, epf->func_no, PCI_EPC_IRQ_LEGACY, irq);
+		break;
+	case IRQ_TYPE_MSI:
 		pci_epc_raise_irq(epc, epf->func_no, PCI_EPC_IRQ_MSI, irq);
+		break;
+	case IRQ_TYPE_MSIX:
+		pci_epc_raise_irq(epc, epf->func_no, PCI_EPC_IRQ_MSIX, irq);
+		break;
+	default:
+		dev_err(dev, "Failed to raise IRQ, unknown type\n");
+		break;
+	}
 }
 
 static void pci_epf_test_cmd_handler(struct work_struct *work)
 {
 	int ret;
-	u8 irq;
-	u8 msi_count;
+	u16 irq;
+	u8 irq_type;
+	u16 msi_count;
 	u32 command;
 	struct pci_epf_test *epf_test = container_of(work, struct pci_epf_test,
 						     cmd_handler.work);
 	struct pci_epf *epf = epf_test->epf;
+	struct device *dev = &epf->dev;
 	struct pci_epc *epc = epf->epc;
 	enum pci_barno test_reg_bar = epf_test->test_reg_bar;
 	struct pci_epf_test_reg *reg = epf_test->reg[test_reg_bar];
@@ -280,11 +300,25 @@ static void pci_epf_test_cmd_handler(struct work_struct *work)
 	reg->command = 0;
 	reg->status = 0;
 
-	irq = (command & MSI_NUMBER_MASK) >> MSI_NUMBER_SHIFT;
+	irq_type = (command & IRQ_TYPE_MASK) >> IRQ_TYPE_SHIFT;
+	switch (irq_type) {
+	case IRQ_TYPE_LEGACY:
+		irq = 0;
+		break;
+	case IRQ_TYPE_MSI:
+		irq = (command & MSI_NUMBER_MASK) >> MSI_NUMBER_SHIFT;
+		break;
+	case IRQ_TYPE_MSIX:
+		irq = (command & MSIX_NUMBER_MASK) >> MSI_NUMBER_SHIFT;
+		break;
+	default:
+		dev_err(dev, "Failed to detect IRQ type\n");
+		goto reset_handler;
+	}
 
 	if (command & COMMAND_RAISE_LEGACY_IRQ) {
 		reg->status = STATUS_IRQ_RAISED;
-		pci_epc_raise_irq(epc, epf->func_no, PCI_EPC_IRQ_LEGACY, 0);
+		pci_epc_raise_irq(epc, epf->func_no, PCI_EPC_IRQ_LEGACY, irq);
 		goto reset_handler;
 	}
 
@@ -294,7 +328,7 @@ static void pci_epf_test_cmd_handler(struct work_struct *work)
 			reg->status |= STATUS_WRITE_FAIL;
 		else
 			reg->status |= STATUS_WRITE_SUCCESS;
-		pci_epf_test_raise_irq(epf_test, irq);
+		pci_epf_test_raise_irq(epf_test, irq_type, irq);
 		goto reset_handler;
 	}
 
@@ -304,7 +338,7 @@ static void pci_epf_test_cmd_handler(struct work_struct *work)
 			reg->status |= STATUS_READ_SUCCESS;
 		else
 			reg->status |= STATUS_READ_FAIL;
-		pci_epf_test_raise_irq(epf_test, irq);
+		pci_epf_test_raise_irq(epf_test, irq_type, irq);
 		goto reset_handler;
 	}
 
@@ -314,7 +348,7 @@ static void pci_epf_test_cmd_handler(struct work_struct *work)
 			reg->status |= STATUS_COPY_SUCCESS;
 		else
 			reg->status |= STATUS_COPY_FAIL;
-		pci_epf_test_raise_irq(epf_test, irq);
+		pci_epf_test_raise_irq(epf_test, irq_type, irq);
 		goto reset_handler;
 	}
 
@@ -327,6 +361,15 @@ static void pci_epf_test_cmd_handler(struct work_struct *work)
 		goto reset_handler;
 	}
 
+	if (command & COMMAND_RAISE_MSIX_IRQ) {
+		msi_count = pci_epc_get_msix(epc, epf->func_no);
+		if (irq > msi_count || msi_count <= 0)
+			goto reset_handler;
+		reg->status = STATUS_IRQ_RAISED;
+		pci_epc_raise_irq(epc, epf->func_no, PCI_EPC_IRQ_MSIX, irq);
+		goto reset_handler;
+	}
+
 reset_handler:
 	queue_delayed_work(kpcitest_workqueue, &epf_test->cmd_handler,
 			   msecs_to_jiffies(1));
@@ -450,8 +493,16 @@ static int pci_epf_test_bind(struct pci_epf *epf)
 		return ret;
 
 	ret = pci_epc_set_msi(epc, epf->func_no, epf->msi_interrupts);
-	if (ret)
+	if (ret) {
+		dev_err(dev, "MSI configuration failed\n");
 		return ret;
+	}
+
+	ret = pci_epc_set_msix(epc, epf->func_no, epf->msix_interrupts);
+	if (ret) {
+		dev_err(dev, "MSI-X configuration failed\n");
+		return ret;
+	}
 
 	if (!epf_test->linkup_notifier)
 		queue_work(kpcitest_workqueue, &epf_test->cmd_handler.work);
-- 
2.7.4

[RFC 09/10] PCI: endpoint: functions/pci-epf-test: Replace lower into upper case characters

[Gustavo Pimentel]

Replaces lower into upper case characters in comments and debug printks.

Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
---
 drivers/pci/endpoint/functions/pci-epf-test.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
index 5997c6e..e3d4af0 100644
--- a/drivers/pci/endpoint/functions/pci-epf-test.c
+++ b/drivers/pci/endpoint/functions/pci-epf-test.c
@@ -94,7 +94,7 @@ static int pci_epf_test_copy(struct pci_epf_test *epf_test)
 
 	src_addr = pci_epc_mem_alloc_addr(epc, &src_phys_addr, reg->size);
 	if (!src_addr) {
-		dev_err(dev, "failed to allocate source address\n");
+		dev_err(dev, "Failed to allocate source address\n");
 		reg->status = STATUS_SRC_ADDR_INVALID;
 		ret = -ENOMEM;
 		goto err;
@@ -103,14 +103,14 @@ static int pci_epf_test_copy(struct pci_epf_test *epf_test)
 	ret = pci_epc_map_addr(epc, epf->func_no, src_phys_addr, reg->src_addr,
 			       reg->size);
 	if (ret) {
-		dev_err(dev, "failed to map source address\n");
+		dev_err(dev, "Failed to map source address\n");
 		reg->status = STATUS_SRC_ADDR_INVALID;
 		goto err_src_addr;
 	}
 
 	dst_addr = pci_epc_mem_alloc_addr(epc, &dst_phys_addr, reg->size);
 	if (!dst_addr) {
-		dev_err(dev, "failed to allocate destination address\n");
+		dev_err(dev, "Failed to allocate destination address\n");
 		reg->status = STATUS_DST_ADDR_INVALID;
 		ret = -ENOMEM;
 		goto err_src_map_addr;
@@ -119,7 +119,7 @@ static int pci_epf_test_copy(struct pci_epf_test *epf_test)
 	ret = pci_epc_map_addr(epc, epf->func_no, dst_phys_addr, reg->dst_addr,
 			       reg->size);
 	if (ret) {
-		dev_err(dev, "failed to map destination address\n");
+		dev_err(dev, "Failed to map destination address\n");
 		reg->status = STATUS_DST_ADDR_INVALID;
 		goto err_dst_addr;
 	}
@@ -156,7 +156,7 @@ static int pci_epf_test_read(struct pci_epf_test *epf_test)
 
 	src_addr = pci_epc_mem_alloc_addr(epc, &phys_addr, reg->size);
 	if (!src_addr) {
-		dev_err(dev, "failed to allocate address\n");
+		dev_err(dev, "Failed to allocate address\n");
 		reg->status = STATUS_SRC_ADDR_INVALID;
 		ret = -ENOMEM;
 		goto err;
@@ -165,7 +165,7 @@ static int pci_epf_test_read(struct pci_epf_test *epf_test)
 	ret = pci_epc_map_addr(epc, epf->func_no, phys_addr, reg->src_addr,
 			       reg->size);
 	if (ret) {
-		dev_err(dev, "failed to map address\n");
+		dev_err(dev, "Failed to map address\n");
 		reg->status = STATUS_SRC_ADDR_INVALID;
 		goto err_addr;
 	}
@@ -208,7 +208,7 @@ static int pci_epf_test_write(struct pci_epf_test *epf_test)
 
 	dst_addr = pci_epc_mem_alloc_addr(epc, &phys_addr, reg->size);
 	if (!dst_addr) {
-		dev_err(dev, "failed to allocate address\n");
+		dev_err(dev, "Failed to allocate address\n");
 		reg->status = STATUS_DST_ADDR_INVALID;
 		ret = -ENOMEM;
 		goto err;
@@ -217,7 +217,7 @@ static int pci_epf_test_write(struct pci_epf_test *epf_test)
 	ret = pci_epc_map_addr(epc, epf->func_no, phys_addr, reg->dst_addr,
 			       reg->size);
 	if (ret) {
-		dev_err(dev, "failed to map address\n");
+		dev_err(dev, "Failed to map address\n");
 		reg->status = STATUS_DST_ADDR_INVALID;
 		goto err_addr;
 	}
@@ -422,7 +422,7 @@ static int pci_epf_test_set_bar(struct pci_epf *epf)
 		ret = pci_epc_set_bar(epc, epf->func_no, epf_bar);
 		if (ret) {
 			pci_epf_free_space(epf, epf_test->reg[bar], bar);
-			dev_err(dev, "failed to set BAR%d\n", bar);
+			dev_err(dev, "Failed to set BAR%d\n", bar);
 			if (bar == test_reg_bar)
 				return ret;
 		}
@@ -449,7 +449,7 @@ static int pci_epf_test_alloc_space(struct pci_epf *epf)
 	base = pci_epf_alloc_space(epf, sizeof(struct pci_epf_test_reg),
 				   test_reg_bar);
 	if (!base) {
-		dev_err(dev, "failed to allocated register space\n");
+		dev_err(dev, "Failed to allocated register space\n");
 		return -ENOMEM;
 	}
 	epf_test->reg[test_reg_bar] = base;
@@ -459,7 +459,7 @@ static int pci_epf_test_alloc_space(struct pci_epf *epf)
 			continue;
 		base = pci_epf_alloc_space(epf, bar_size[bar], bar);
 		if (!base)
-			dev_err(dev, "failed to allocate space for BAR%d\n",
+			dev_err(dev, "Failed to allocate space for BAR%d\n",
 				bar);
 		epf_test->reg[bar] = base;
 	}
@@ -480,7 +480,7 @@ static int pci_epf_test_bind(struct pci_epf *epf)
 
 	ret = pci_epc_write_header(epc, epf->func_no, header);
 	if (ret) {
-		dev_err(dev, "configuration header write failed\n");
+		dev_err(dev, "Configuration header write failed\n");
 		return ret;
 	}
 
@@ -578,7 +578,7 @@ static int __init pci_epf_test_init(void)
 					     WQ_MEM_RECLAIM | WQ_HIGHPRI, 0);
 	ret = pci_epf_register_driver(&test_driver);
 	if (ret) {
-		pr_err("failed to register pci epf test driver --> %d\n", ret);
+		pr_err("Failed to register pci epf test driver --> %d\n", ret);
 		return ret;
 	}
 
-- 
2.7.4

[RFC 10/10] tools: PCI: Add MSI-X support

[Gustavo Pimentel]

Adds MSI-X support to the pcitest tool and modified the pcitest.sh script
to accomodate this new type of interruption test.

Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
---
 include/uapi/linux/pcitest.h |  1 +
 tools/pci/pcitest.c          | 18 +++++++++++++++++-
 tools/pci/pcitest.sh         | 25 +++++++++++++++++++++++++
 3 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/pcitest.h b/include/uapi/linux/pcitest.h
index 953cf03..d746fb1 100644
--- a/include/uapi/linux/pcitest.h
+++ b/include/uapi/linux/pcitest.h
@@ -16,5 +16,6 @@
 #define PCITEST_WRITE		_IOW('P', 0x4, unsigned long)
 #define PCITEST_READ		_IOW('P', 0x5, unsigned long)
 #define PCITEST_COPY		_IOW('P', 0x6, unsigned long)
+#define PCITEST_MSIX		_IOW('P', 0x7, int)
 
 #endif /* __UAPI_LINUX_PCITEST_H */
diff --git a/tools/pci/pcitest.c b/tools/pci/pcitest.c
index 9074b47..9d145a3 100644
--- a/tools/pci/pcitest.c
+++ b/tools/pci/pcitest.c
@@ -37,6 +37,7 @@ struct pci_test {
 	char		barnum;
 	bool		legacyirq;
 	unsigned int	msinum;
+	unsigned int	msixnum;
 	bool		read;
 	bool		write;
 	bool		copy;
@@ -83,6 +84,15 @@ static int run_test(struct pci_test *test)
 			fprintf(stdout, "%s\n", result[ret]);
 	}
 
+	if (test->msixnum > 0 && test->msixnum <= 2048) {
+		ret = ioctl(fd, PCITEST_MSIX, test->msixnum);
+		fprintf(stdout, "MSI-X%d:\t\t", test->msixnum);
+		if (ret < 0)
+			fprintf(stdout, "TEST FAILED\n");
+		else
+			fprintf(stdout, "%s\n", result[ret]);
+	}
+
 	if (test->write) {
 		ret = ioctl(fd, PCITEST_WRITE, test->size);
 		fprintf(stdout, "WRITE (%7ld bytes):\t\t", test->size);
@@ -133,7 +143,7 @@ int main(int argc, char **argv)
 	/* set default endpoint device */
 	test->device = "/dev/pci-endpoint-test.0";
 
-	while ((c = getopt(argc, argv, "D:b:m:lrwcs:")) != EOF)
+	while ((c = getopt(argc, argv, "D:b:m:x:lrwcs:")) != EOF)
 	switch (c) {
 	case 'D':
 		test->device = optarg;
@@ -151,6 +161,11 @@ int main(int argc, char **argv)
 		if (test->msinum < 1 || test->msinum > 32)
 			goto usage;
 		continue;
+	case 'x':
+		test->msixnum = atoi(optarg);
+		if (test->msixnum < 1 || test->msixnum > 2048)
+			goto usage;
+		continue;
 	case 'r':
 		test->read = true;
 		continue;
@@ -173,6 +188,7 @@ int main(int argc, char **argv)
 			"\t-D <dev>		PCI endpoint test device {default: /dev/pci-endpoint-test.0}\n"
 			"\t-b <bar num>		BAR test (bar number between 0..5)\n"
 			"\t-m <msi num>		MSI test (msi number between 1..32)\n"
+			"\t-x <msix num>	MSI-X test (msix number between 1..2048)\n"
 			"\t-l			Legacy IRQ test\n"
 			"\t-r			Read buffer test\n"
 			"\t-w			Write buffer test\n"
diff --git a/tools/pci/pcitest.sh b/tools/pci/pcitest.sh
index 77e8c85..86709a2 100644
--- a/tools/pci/pcitest.sh
+++ b/tools/pci/pcitest.sh
@@ -4,6 +4,8 @@
 echo "BAR tests"
 echo
 
+modprobe pci_endpoint_test
+sleep 2
 bar=0
 
 while [ $bar -lt 6 ]
@@ -16,7 +18,14 @@ echo
 echo "Interrupt tests"
 echo
 
+rmmod pci_endpoint_test
+sleep 2
+modprobe pci_endpoint_test irq_type=0
 pcitest -l
+
+rmmod pci_endpoint_test
+sleep 2
+modprobe pci_endpoint_test irq_type=1
 msi=1
 
 while [ $msi -lt 33 ]
@@ -26,9 +35,25 @@ do
 done
 echo
 
+rmmod pci_endpoint_test
+sleep 2
+modprobe pci_endpoint_test irq_type=2
+msix=1
+
+while [ $msix -lt 2049 ]
+do
+        pcitest -x $msix
+        msix=`expr $msix + 1`
+done
+echo
+
 echo "Read Tests"
 echo
 
+rmmod pci_endpoint_test
+sleep 2
+modprobe pci_endpoint_test irq_type=1
+
 pcitest -r -s 1
 pcitest -r -s 1024
 pcitest -r -s 1025
-- 
2.7.4

RE: [RFC 10/10] tools: PCI: Add MSI-X support

[Alan Douglas]

Hi Gustavo,

On 10 April 2018 18:15, Gustavo Pimentel wrote:
> Adds MSI-X support to the pcitest tool and modified the pcitest.sh script=
 to
> accomodate this new type of interruption test.
>=20
> Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
> ---
>  include/uapi/linux/pcitest.h |  1 +
>  tools/pci/pcitest.c          | 18 +++++++++++++++++-
>  tools/pci/pcitest.sh         | 25 +++++++++++++++++++++++++
>  3 files changed, 43 insertions(+), 1 deletion(-)
I found some possible problems when testing with the Cadence EP driver.  Th=
e problem
is that pcitest uses the BARs for tests, but we also use one for the MSI-X =
tables

In Cadence core the MSI-X table is in BAR0 by default, but this is configur=
ed to a size
of 0x80 in the test driver, since it is used as the test_reg_bar.  So, I ch=
anged the=20
configuration to use BAR4 instead, which is configured to a size of 131072=
=20
in pci-efp-test.c, and this gives me enough space.

However, if I run the BAR tests in pcitest before running the MSI-X tests, =
the
MSI-X tests fail, since the BAR content is overwritten.  It's not a problem=
 with the=20
scenario in pcitest.sh, but it would be if the module wasn't re-loaded.

So, wondering if we need to come up with some mechanism to specify that a s=
pecific
BAR will be used for MSI-X, and that its size and content shouldn't be modi=
fied by
pcitest?

Regards,
Alan

Re: [RFC 10/10] tools: PCI: Add MSI-X support

[Gustavo Pimentel]

Hi Alan,

On 24/04/2018 10:57, Alan Douglas wrote:
> Hi Gustavo,
> 
> On 10 April 2018 18:15, Gustavo Pimentel wrote:
>> Adds MSI-X support to the pcitest tool and modified the pcitest.sh script to
>> accomodate this new type of interruption test.
>>
>> Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
>> ---
>>  include/uapi/linux/pcitest.h |  1 +
>>  tools/pci/pcitest.c          | 18 +++++++++++++++++-
>>  tools/pci/pcitest.sh         | 25 +++++++++++++++++++++++++
>>  3 files changed, 43 insertions(+), 1 deletion(-)
> I found some possible problems when testing with the Cadence EP driver.  The problem
> is that pcitest uses the BARs for tests, but we also use one for the MSI-X tables
> 
> In Cadence core the MSI-X table is in BAR0 by default, but this is configured to a size
> of 0x80 in the test driver, since it is used as the test_reg_bar.  So, I changed the 
> configuration to use BAR4 instead, which is configured to a size of 131072 
> in pci-efp-test.c, and this gives me enough space.
> 
> However, if I run the BAR tests in pcitest before running the MSI-X tests, the
> MSI-X tests fail, since the BAR content is overwritten.  It's not a problem with the 
> scenario in pcitest.sh, but it would be if the module wasn't re-loaded.
> 
> So, wondering if we need to come up with some mechanism to specify that a specific
> BAR will be used for MSI-X, and that its size and content shouldn't be modified by
> pcitest?

I see your point. I have bypassed the problem by doing the module load/unload
(to avoid having to fight on multiple fronts).

I like your suggestion. Maybe we could have a bool variable for each BARs that
could be set to false if a resource have intent to use it.

However this change must be accepted by Kishon.

> 
> Regards,
> Alan
> 
Regards,
Gustavo

RE: [RFC 00/10] Adds pcitest tool support for MSI-X

[Alan Douglas]

Hi Gustavo,

On 10 April 2018 18:15 Gustavo Pimentel wrote:
> This patch set depends the following series:
> https://lkml.org/lkml/2018/4/10/421> This series aims to add pcitest tool=
 support for MSI-X.
>=20
> Includes new callbacks methods and handlers to trigger the MSI-X
> interruptions on the EP Designware IP driver.
>=20
> Provides new methods on pci_epf_test driver that allows to set/get EP
> maximum number of MSI-X entries (similar to set/get MSI methods).
>=20
> Reworks on MSI set/get and triggering methods on EP Designware IP driver =
to
> be more generic and flexible.
>=20
> Adds a new input parameter (msix) and replicates the whole MSI mechanism
> applied to the MSI-X feature on pcitest tool. Also updates the pcitest sc=
ript
> with the new test set applied to this new feature.
>=20
> Gustavo Pimentel (10):
>   PCI: dwc: Add MSI-X callbacks handler
>   PCI: cadence: Update cdns_pcie_ep_raise_irq function signature
>   PCI: endpoint: Add MSI-X interfaces
>   PCI: dwc: MSI callbacks handler rework
>   PCI: dwc: Add legacy interrupt callback handler
>   misc: pci_endpoint_test: Add MSI-X support
>   misc: pci_endpoint_test: Replace lower into upper case characters
>   PCI: endpoint: functions/pci-epf-test: Add MSI-X support
>   PCI: endpoint: functions/pci-epf-test: Replace lower into upper case
>     characters
>   tools: PCI: Add MSI-X support
>=20
>  Documentation/misc-devices/pci-endpoint-test.txt |   3 +
>  drivers/misc/pci_endpoint_test.c                 | 120 ++++++++++----
>  drivers/pci/cadence/pcie-cadence-ep.c            |   2 +-
>  drivers/pci/dwc/pci-dra7xx.c                     |   2 +-
>  drivers/pci/dwc/pcie-artpec6.c                   |   2 +-
>  drivers/pci/dwc/pcie-designware-ep.c             | 201
> +++++++++++++++++++++--
>  drivers/pci/dwc/pcie-designware-plat.c           |   9 +-
>  drivers/pci/dwc/pcie-designware.h                |  40 +++--
>  drivers/pci/endpoint/functions/pci-epf-test.c    | 113 +++++++++----
>  drivers/pci/endpoint/pci-ep-cfs.c                |  24 +++
>  drivers/pci/endpoint/pci-epc-core.c              |  60 ++++++-
>  include/linux/pci-epc.h                          |  11 +-
>  include/linux/pci-epf.h                          |   1 +
>  include/uapi/linux/pcitest.h                     |   1 +
>  tools/pci/pcitest.c                              |  18 +-
>  tools/pci/pcitest.sh                             |  25 +++
>  16 files changed, 528 insertions(+), 104 deletions(-)
>=20
> --
> 2.7.4
>=20
Nice set of patches.  I have tested this with the Cadence EP driver after a=
dding MSI-X support, and found a few changes required.
I will send you comments.

Thanks,
Alan

Re: [RFC 00/10] Adds pcitest tool support for MSI-X

[Gustavo Pimentel]

Hi Alan,

On 24/04/2018 07:48, Alan Douglas wrote:
> Hi Gustavo,
> 
> On 10 April 2018 18:15 Gustavo Pimentel wrote:
>> This patch set depends the following series:
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__lkml.org_lkml_2018_4_10_421&d=DwIFAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=bkWxpLoW-f-E3EdiDCCa0_h0PicsViasSlvIpzZvPxs&m=R3h7a4AkN9--FFv6jtHaknKIzx6NieDdkYxyCe4obB8&s=J2A11L2_WwzMdaBheYSTwfjPDA7sx3_DyoGPAYsJ9A4&e=> This series aims to add pcitest tool support for MSI-X.
>>
>> Includes new callbacks methods and handlers to trigger the MSI-X
>> interruptions on the EP Designware IP driver.
>>
>> Provides new methods on pci_epf_test driver that allows to set/get EP
>> maximum number of MSI-X entries (similar to set/get MSI methods).
>>
>> Reworks on MSI set/get and triggering methods on EP Designware IP driver to
>> be more generic and flexible.
>>
>> Adds a new input parameter (msix) and replicates the whole MSI mechanism
>> applied to the MSI-X feature on pcitest tool. Also updates the pcitest script
>> with the new test set applied to this new feature.
>>
>> Gustavo Pimentel (10):
>>   PCI: dwc: Add MSI-X callbacks handler
>>   PCI: cadence: Update cdns_pcie_ep_raise_irq function signature
>>   PCI: endpoint: Add MSI-X interfaces
>>   PCI: dwc: MSI callbacks handler rework
>>   PCI: dwc: Add legacy interrupt callback handler
>>   misc: pci_endpoint_test: Add MSI-X support
>>   misc: pci_endpoint_test: Replace lower into upper case characters
>>   PCI: endpoint: functions/pci-epf-test: Add MSI-X support
>>   PCI: endpoint: functions/pci-epf-test: Replace lower into upper case
>>     characters
>>   tools: PCI: Add MSI-X support
>>
>>  Documentation/misc-devices/pci-endpoint-test.txt |   3 +
>>  drivers/misc/pci_endpoint_test.c                 | 120 ++++++++++----
>>  drivers/pci/cadence/pcie-cadence-ep.c            |   2 +-
>>  drivers/pci/dwc/pci-dra7xx.c                     |   2 +-
>>  drivers/pci/dwc/pcie-artpec6.c                   |   2 +-
>>  drivers/pci/dwc/pcie-designware-ep.c             | 201
>> +++++++++++++++++++++--
>>  drivers/pci/dwc/pcie-designware-plat.c           |   9 +-
>>  drivers/pci/dwc/pcie-designware.h                |  40 +++--
>>  drivers/pci/endpoint/functions/pci-epf-test.c    | 113 +++++++++----
>>  drivers/pci/endpoint/pci-ep-cfs.c                |  24 +++
>>  drivers/pci/endpoint/pci-epc-core.c              |  60 ++++++-
>>  include/linux/pci-epc.h                          |  11 +-
>>  include/linux/pci-epf.h                          |   1 +
>>  include/uapi/linux/pcitest.h                     |   1 +
>>  tools/pci/pcitest.c                              |  18 +-
>>  tools/pci/pcitest.sh                             |  25 +++
>>  16 files changed, 528 insertions(+), 104 deletions(-)
>>
>> --
>> 2.7.4
>>
> Nice set of patches.  I have tested this with the Cadence EP driver after adding MSI-X support, and found a few changes required.
> I will send you comments.

Ok, great news!

Maybe after this patch series submission we could start a new thread about new
features that could be tested/verified using pcitest. I think this could be
helpful for everybody.

> 
> Thanks,
> Alan
> 

Thanks,
Gustavo

RE: [RFC 00/10] Adds pcitest tool support for MSI-X

[Alan Douglas]

T24gMjQgQXByaWwgMjAxOCAwOTo1MCwgR3VzdGF2byBQaW1lbnRlbCB3cm90ZToNCj4gSGkgQWxh
biwNCj4gDQo+IE9uIDI0LzA0LzIwMTggMDc6NDgsIEFsYW4gRG91Z2xhcyB3cm90ZToNCj4gPiBI
aSBHdXN0YXZvLA0KPiA+DQo+ID4gT24gMTAgQXByaWwgMjAxOCAxODoxNSBHdXN0YXZvIFBpbWVu
dGVsIHdyb3RlOg0KPiA+PiAgaHR0cHM6Ly9sa21sLm9yZy9sa21sLzIwMTgvNC8xMC80MjENCj4g
Pj4gIFRoaXMgc2VyaWVzIGFpbXMgdG8gYWRkIHBjaXRlc3QgdG9vbCBzdXBwb3J0IGZvciBNU0kt
WC4NCj4gPj4gSW5jbHVkZXMgbmV3IGNhbGxiYWNrcyBtZXRob2RzIGFuZCBoYW5kbGVycyB0byB0
cmlnZ2VyIHRoZSBNU0ktWA0KPiA+PiBpbnRlcnJ1cHRpb25zIG9uIHRoZSBFUCBEZXNpZ253YXJl
IElQIGRyaXZlci4NCj4gPj4NCj4gPj4gUHJvdmlkZXMgbmV3IG1ldGhvZHMgb24gcGNpX2VwZl90
ZXN0IGRyaXZlciB0aGF0IGFsbG93cyB0byBzZXQvZ2V0IEVQDQo+ID4+IG1heGltdW0gbnVtYmVy
IG9mIE1TSS1YIGVudHJpZXMgKHNpbWlsYXIgdG8gc2V0L2dldCBNU0kgbWV0aG9kcykuDQo+ID4+
DQo+ID4+IFJld29ya3Mgb24gTVNJIHNldC9nZXQgYW5kIHRyaWdnZXJpbmcgbWV0aG9kcyBvbiBF
UCBEZXNpZ253YXJlIElQDQo+ID4+IGRyaXZlciB0byBiZSBtb3JlIGdlbmVyaWMgYW5kIGZsZXhp
YmxlLg0KPiA+Pg0KPiA+PiBBZGRzIGEgbmV3IGlucHV0IHBhcmFtZXRlciAobXNpeCkgYW5kIHJl
cGxpY2F0ZXMgdGhlIHdob2xlIE1TSQ0KPiA+PiBtZWNoYW5pc20gYXBwbGllZCB0byB0aGUgTVNJ
LVggZmVhdHVyZSBvbiBwY2l0ZXN0IHRvb2wuIEFsc28gdXBkYXRlcw0KPiA+PiB0aGUgcGNpdGVz
dCBzY3JpcHQgd2l0aCB0aGUgbmV3IHRlc3Qgc2V0IGFwcGxpZWQgdG8gdGhpcyBuZXcgZmVhdHVy
ZS4NCj4gPj4NCj4gPj4gR3VzdGF2byBQaW1lbnRlbCAoMTApOg0KPiA+PiAgIFBDSTogZHdjOiBB
ZGQgTVNJLVggY2FsbGJhY2tzIGhhbmRsZXINCj4gPj4gICBQQ0k6IGNhZGVuY2U6IFVwZGF0ZSBj
ZG5zX3BjaWVfZXBfcmFpc2VfaXJxIGZ1bmN0aW9uIHNpZ25hdHVyZQ0KPiA+PiAgIFBDSTogZW5k
cG9pbnQ6IEFkZCBNU0ktWCBpbnRlcmZhY2VzDQo+ID4+ICAgUENJOiBkd2M6IE1TSSBjYWxsYmFj
a3MgaGFuZGxlciByZXdvcmsNCj4gPj4gICBQQ0k6IGR3YzogQWRkIGxlZ2FjeSBpbnRlcnJ1cHQg
Y2FsbGJhY2sgaGFuZGxlcg0KPiA+PiAgIG1pc2M6IHBjaV9lbmRwb2ludF90ZXN0OiBBZGQgTVNJ
LVggc3VwcG9ydA0KPiA+PiAgIG1pc2M6IHBjaV9lbmRwb2ludF90ZXN0OiBSZXBsYWNlIGxvd2Vy
IGludG8gdXBwZXIgY2FzZSBjaGFyYWN0ZXJzDQo+ID4+ICAgUENJOiBlbmRwb2ludDogZnVuY3Rp
b25zL3BjaS1lcGYtdGVzdDogQWRkIE1TSS1YIHN1cHBvcnQNCj4gPj4gICBQQ0k6IGVuZHBvaW50
OiBmdW5jdGlvbnMvcGNpLWVwZi10ZXN0OiBSZXBsYWNlIGxvd2VyIGludG8gdXBwZXIgY2FzZQ0K
PiA+PiAgICAgY2hhcmFjdGVycw0KPiA+PiAgIHRvb2xzOiBQQ0k6IEFkZCBNU0ktWCBzdXBwb3J0
DQo+ID4+DQo+ID4+ICBEb2N1bWVudGF0aW9uL21pc2MtZGV2aWNlcy9wY2ktZW5kcG9pbnQtdGVz
dC50eHQgfCAgIDMgKw0KPiA+PiAgZHJpdmVycy9taXNjL3BjaV9lbmRwb2ludF90ZXN0LmMgICAg
ICAgICAgICAgICAgIHwgMTIwICsrKysrKysrKystLS0tDQo+ID4+ICBkcml2ZXJzL3BjaS9jYWRl
bmNlL3BjaWUtY2FkZW5jZS1lcC5jICAgICAgICAgICAgfCAgIDIgKy0NCj4gPj4gIGRyaXZlcnMv
cGNpL2R3Yy9wY2ktZHJhN3h4LmMgICAgICAgICAgICAgICAgICAgICB8ICAgMiArLQ0KPiA+PiAg
ZHJpdmVycy9wY2kvZHdjL3BjaWUtYXJ0cGVjNi5jICAgICAgICAgICAgICAgICAgIHwgICAyICst
DQo+ID4+ICBkcml2ZXJzL3BjaS9kd2MvcGNpZS1kZXNpZ253YXJlLWVwLmMgICAgICAgICAgICAg
fCAyMDENCj4gPj4gKysrKysrKysrKysrKysrKysrKysrLS0NCj4gPj4gIGRyaXZlcnMvcGNpL2R3
Yy9wY2llLWRlc2lnbndhcmUtcGxhdC5jICAgICAgICAgICB8ICAgOSArLQ0KPiA+PiAgZHJpdmVy
cy9wY2kvZHdjL3BjaWUtZGVzaWdud2FyZS5oICAgICAgICAgICAgICAgIHwgIDQwICsrKy0tDQo+
ID4+ICBkcml2ZXJzL3BjaS9lbmRwb2ludC9mdW5jdGlvbnMvcGNpLWVwZi10ZXN0LmMgICAgfCAx
MTMgKysrKysrKysrLS0tLQ0KPiA+PiAgZHJpdmVycy9wY2kvZW5kcG9pbnQvcGNpLWVwLWNmcy5j
ICAgICAgICAgICAgICAgIHwgIDI0ICsrKw0KPiA+PiAgZHJpdmVycy9wY2kvZW5kcG9pbnQvcGNp
LWVwYy1jb3JlLmMgICAgICAgICAgICAgIHwgIDYwICsrKysrKy0NCj4gPj4gIGluY2x1ZGUvbGlu
dXgvcGNpLWVwYy5oICAgICAgICAgICAgICAgICAgICAgICAgICB8ICAxMSArLQ0KPiA+PiAgaW5j
bHVkZS9saW51eC9wY2ktZXBmLmggICAgICAgICAgICAgICAgICAgICAgICAgIHwgICAxICsNCj4g
Pj4gIGluY2x1ZGUvdWFwaS9saW51eC9wY2l0ZXN0LmggICAgICAgICAgICAgICAgICAgICB8ICAg
MSArDQo+ID4+ICB0b29scy9wY2kvcGNpdGVzdC5jICAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgfCAgMTggKy0NCj4gPj4gIHRvb2xzL3BjaS9wY2l0ZXN0LnNoICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICB8ICAyNSArKysNCj4gPj4gIDE2IGZpbGVzIGNoYW5nZWQsIDUyOCBpbnNlcnRp
b25zKCspLCAxMDQgZGVsZXRpb25zKC0pDQo+ID4+DQo+ID4+IC0tDQo+ID4+IDIuNy40DQo+ID4+
DQo+ID4gTmljZSBzZXQgb2YgcGF0Y2hlcy4gIEkgaGF2ZSB0ZXN0ZWQgdGhpcyB3aXRoIHRoZSBD
YWRlbmNlIEVQIGRyaXZlciBhZnRlcg0KPiBhZGRpbmcgTVNJLVggc3VwcG9ydCwgYW5kIGZvdW5k
IGEgZmV3IGNoYW5nZXMgcmVxdWlyZWQuDQo+ID4gSSB3aWxsIHNlbmQgeW91IGNvbW1lbnRzLg0K
PiANCj4gT2ssIGdyZWF0IG5ld3MhDQo+IA0KPiBNYXliZSBhZnRlciB0aGlzIHBhdGNoIHNlcmll
cyBzdWJtaXNzaW9uIHdlIGNvdWxkIHN0YXJ0IGEgbmV3IHRocmVhZCBhYm91dA0KPiBuZXcgZmVh
dHVyZXMgdGhhdCBjb3VsZCBiZSB0ZXN0ZWQvdmVyaWZpZWQgdXNpbmcgcGNpdGVzdC4gSSB0aGlu
ayB0aGlzIGNvdWxkIGJlDQo+IGhlbHBmdWwgZm9yIGV2ZXJ5Ym9keS4NCj4gDQpHcmVhdCwgSSB0
aGluayB0aGF0IHdvdWxkIGRlZmluaXRlbHkgYmUgb2YgYmVuZWZpdC4NCj4gPg0KPiA+IFRoYW5r
cywNCj4gPiBBbGFuDQo+ID4NCj4gDQo+IFRoYW5rcywNCj4gR3VzdGF2bw0KVGhhbmtzLA0KQWxh
bg0KDQoNCg==