Existing MSI-X support in Endpoint core has limitations:
1) MSIX table (which is mapped to a BAR) is not allocated by
anyone. Ideally this should be allocated by endpoint
function driver.
2) Endpoint controller can choose any random BARs for MSIX
table (irrespective of whether the endpoint function driver
has allocated memory for it or not)
In order to avoid these limitations, pci_epc_set_msix() is
modified to include BAR Indicator register (BIR) configuration
and MSIX table offset as arguments. This series also fixed MSIX
support in dwc driver and add MSI-X support in Cadence PCIe driver.
The previous version of Cadence EP MSI-X support is @ [1].
This series is created on top of [2]
[1] -> https://patchwork.ozlabs.org/patch/971160/
[2] -> http://lore.kernel.org/r/[email protected]
Alan Douglas (1):
PCI: cadence: Add MSI-X support to Endpoint driver
Kishon Vijay Abraham I (3):
PCI: endpoint: Fix ->set_msix() to take BIR and offset as arguments
PCI: dwc: Fix dw_pcie_ep_raise_msix_irq() to get correct MSIX table
address
PCI: keystone: Add AM654 PCIe Endpoint to raise MSIX interrupt
.../pci/controller/cadence/pcie-cadence-ep.c | 112 +++++++++++++++++-
drivers/pci/controller/cadence/pcie-cadence.h | 10 ++
drivers/pci/controller/dwc/pci-keystone.c | 5 +-
.../pci/controller/dwc/pcie-designware-ep.c | 61 +++++-----
drivers/pci/controller/dwc/pcie-designware.h | 1 +
drivers/pci/endpoint/functions/pci-epf-test.c | 31 ++++-
drivers/pci/endpoint/pci-epc-core.c | 7 +-
drivers/pci/endpoint/pci-epf-core.c | 2 +
include/linux/pci-epc.h | 6 +-
include/linux/pci-epf.h | 15 +++
10 files changed, 207 insertions(+), 43 deletions(-)
--
2.17.1
commit beb4641a787df79a14 ("PCI: dwc: Add MSI-X callbacks handler"),
in order to raise MSIX interrupt, obtained MSIX table address from
Base Address Register (BAR). However BAR only holds PCI address
programmed by the host whereas the MSIX table should be in the local
memory.
Store the MSIX table address (virtual address) as part of ->set_bar()
callback and use that to get the message address and message data
here.
Fixes: commit beb4641a787df79a14 ("PCI: dwc: Add MSI-X callbacks
handler")
Signed-off-by: Kishon Vijay Abraham I <[email protected]>
---
.../pci/controller/dwc/pcie-designware-ep.c | 46 +++++++------------
drivers/pci/controller/dwc/pcie-designware.h | 1 +
drivers/pci/endpoint/pci-epf-core.c | 2 +
include/linux/pci-epf.h | 15 ++++++
4 files changed, 35 insertions(+), 29 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
index 4cd3193c9c7c..b61e47365456 100644
--- a/drivers/pci/controller/dwc/pcie-designware-ep.c
+++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
@@ -125,6 +125,7 @@ static void dw_pcie_ep_clear_bar(struct pci_epc *epc, u8 func_no,
dw_pcie_disable_atu(pci, atu_index, DW_PCIE_REGION_INBOUND);
clear_bit(atu_index, ep->ib_window_map);
+ ep->epf_bar[bar] = NULL;
}
static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8 func_no,
@@ -158,6 +159,7 @@ static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8 func_no,
dw_pcie_writel_dbi(pci, reg + 4, 0);
}
+ ep->epf_bar[bar] = epf_bar;
dw_pcie_dbi_ro_wr_dis(pci);
return 0;
@@ -420,55 +422,41 @@ 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_epf_msix_tbl *msix_tbl;
struct pci_epc *epc = ep->epc;
- u16 tbl_offset, bir;
- u32 bar_addr_upper, bar_addr_lower;
- u32 msg_addr_upper, msg_addr_lower;
+ struct pci_epf_bar *epf_bar;
u32 reg, msg_data, vec_ctrl;
- u64 tbl_addr, msg_addr, reg_u64;
- void __iomem *msix_tbl;
+ unsigned int aligned_offset;
+ u32 tbl_offset;
+ u64 msg_addr;
int ret;
+ u8 bir;
reg = ep->msix_cap + PCI_MSIX_TABLE;
tbl_offset = dw_pcie_readl_dbi(pci, reg);
bir = (tbl_offset & PCI_MSIX_TABLE_BIR);
tbl_offset &= PCI_MSIX_TABLE_OFFSET;
- reg = PCI_BASE_ADDRESS_0 + (4 * bir);
- bar_addr_upper = 0;
- 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);
+ epf_bar = ep->epf_bar[bir];
+ msix_tbl = epf_bar->addr;
+ msix_tbl = (struct pci_epf_msix_tbl *)((char *)msix_tbl + tbl_offset);
- 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,
- PCI_MSIX_ENTRY_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);
- vec_ctrl = readl(msix_tbl + PCI_MSIX_ENTRY_VECTOR_CTRL);
-
- iounmap(msix_tbl);
+ msg_addr = msix_tbl[(interrupt_num - 1)].msg_addr;
+ msg_data = msix_tbl[(interrupt_num - 1)].msg_data;
+ vec_ctrl = msix_tbl[(interrupt_num - 1)].vector_ctrl;
if (vec_ctrl & PCI_MSIX_ENTRY_CTRL_MASKBIT) {
dev_dbg(pci->dev, "MSI-X entry ctrl set\n");
return -EPERM;
}
- ret = dw_pcie_ep_map_addr(epc, func_no, ep->msi_mem_phys, msg_addr,
+ aligned_offset = msg_addr & (epc->mem->page_size - 1);
+ ret = dw_pcie_ep_map_addr(epc, func_no, ep->msi_mem_phys, msg_addr,
epc->mem->page_size);
if (ret)
return ret;
- writel(msg_data, ep->msi_mem);
+ writel(msg_data, ep->msi_mem + aligned_offset);
dw_pcie_ep_unmap_addr(epc, func_no, ep->msi_mem_phys);
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index 5accdd6bc388..6c7fcd867581 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -224,6 +224,7 @@ struct dw_pcie_ep {
phys_addr_t msi_mem_phys;
u8 msi_cap; /* MSI capability offset */
u8 msix_cap; /* MSI-X capability offset */
+ struct pci_epf_bar *epf_bar[PCI_STD_NUM_BARS];
};
struct dw_pcie_ops {
diff --git a/drivers/pci/endpoint/pci-epf-core.c b/drivers/pci/endpoint/pci-epf-core.c
index fb1306de8f40..93ebe916949e 100644
--- a/drivers/pci/endpoint/pci-epf-core.c
+++ b/drivers/pci/endpoint/pci-epf-core.c
@@ -99,6 +99,7 @@ void pci_epf_free_space(struct pci_epf *epf, void *addr, enum pci_barno bar)
epf->bar[bar].phys_addr);
epf->bar[bar].phys_addr = 0;
+ epf->bar[bar].addr = NULL;
epf->bar[bar].size = 0;
epf->bar[bar].barno = 0;
epf->bar[bar].flags = 0;
@@ -135,6 +136,7 @@ void *pci_epf_alloc_space(struct pci_epf *epf, size_t size, enum pci_barno bar,
}
epf->bar[bar].phys_addr = phys_addr;
+ epf->bar[bar].addr = space;
epf->bar[bar].size = size;
epf->bar[bar].barno = bar;
epf->bar[bar].flags |= upper_32_bits(size) ?
diff --git a/include/linux/pci-epf.h b/include/linux/pci-epf.h
index 2d6f07556682..bc5ce7afd79a 100644
--- a/include/linux/pci-epf.h
+++ b/include/linux/pci-epf.h
@@ -92,10 +92,12 @@ struct pci_epf_driver {
/**
* struct pci_epf_bar - represents the BAR of EPF device
* @phys_addr: physical address that should be mapped to the BAR
+ * @addr: virtual address corresponding to the @phys_addr
* @size: the size of the address space present in BAR
*/
struct pci_epf_bar {
dma_addr_t phys_addr;
+ void *addr;
size_t size;
enum pci_barno barno;
int flags;
@@ -127,6 +129,19 @@ struct pci_epf {
struct list_head list;
};
+/**
+ * struct pci_epf_msix_tbl - represents the MSIX table entry structure
+ * @msg_addr: Writes to this address will trigger MSIX interrupt in host
+ * @msg_data: Data that should be written to @msg_addr to trigger MSIX interrupt
+ * @vector_ctrl: Identifies if the function is prohibited from sending a message
+ * using this MSIX table entry
+ */
+struct pci_epf_msix_tbl {
+ u64 msg_addr;
+ u32 msg_data;
+ u32 vector_ctrl;
+};
+
#define to_pci_epf(epf_dev) container_of((epf_dev), struct pci_epf, dev)
#define pci_epf_register_driver(driver) \
--
2.17.1
On Wed, Dec 11, 2019 at 06:16:04PM +0530, Kishon Vijay Abraham I wrote:
> Existing MSI-X support in Endpoint core has limitations:
> 1) MSIX table (which is mapped to a BAR) is not allocated by
> anyone. Ideally this should be allocated by endpoint
> function driver.
> 2) Endpoint controller can choose any random BARs for MSIX
> table (irrespective of whether the endpoint function driver
> has allocated memory for it or not)
>
> In order to avoid these limitations, pci_epc_set_msix() is
> modified to include BAR Indicator register (BIR) configuration
> and MSIX table offset as arguments. This series also fixed MSIX
> support in dwc driver and add MSI-X support in Cadence PCIe driver.
>
> The previous version of Cadence EP MSI-X support is @ [1].
> This series is created on top of [2]
>
> [1] -> https://patchwork.ozlabs.org/patch/971160/
> [2] -> http://lore.kernel.org/r/[email protected]
>
> Alan Douglas (1):
> PCI: cadence: Add MSI-X support to Endpoint driver
>
> Kishon Vijay Abraham I (3):
> PCI: endpoint: Fix ->set_msix() to take BIR and offset as arguments
> PCI: dwc: Fix dw_pcie_ep_raise_msix_irq() to get correct MSIX table
> address
> PCI: keystone: Add AM654 PCIe Endpoint to raise MSIX interrupt
Trivial nits:
- There's a mix of "MSI-X" and "MSIX" in the subjects, commit logs,
and comments. I prefer "MSI-X" to match usage in the spec.
- "Fixes:" tags need not include "commit". It doesn't *hurt*
anything, but it takes up space that could be used for the
subject.
- Commit references typically use a 12-char SHA1. Again, doesn't
hurt anything.
Hi,
On 12/12/19 4:16 AM, Bjorn Helgaas wrote:
> On Wed, Dec 11, 2019 at 06:16:04PM +0530, Kishon Vijay Abraham I wrote:
>> Existing MSI-X support in Endpoint core has limitations:
>> 1) MSIX table (which is mapped to a BAR) is not allocated by
>> anyone. Ideally this should be allocated by endpoint
>> function driver.
>> 2) Endpoint controller can choose any random BARs for MSIX
>> table (irrespective of whether the endpoint function driver
>> has allocated memory for it or not)
>>
>> In order to avoid these limitations, pci_epc_set_msix() is
>> modified to include BAR Indicator register (BIR) configuration
>> and MSIX table offset as arguments. This series also fixed MSIX
>> support in dwc driver and add MSI-X support in Cadence PCIe driver.
>>
>> The previous version of Cadence EP MSI-X support is @ [1].
>> This series is created on top of [2]
>>
>> [1] -> https://patchwork.ozlabs.org/patch/971160/
>> [2] -> http://lore.kernel.org/r/[email protected]
>>
>> Alan Douglas (1):
>> PCI: cadence: Add MSI-X support to Endpoint driver
>>
>> Kishon Vijay Abraham I (3):
>> PCI: endpoint: Fix ->set_msix() to take BIR and offset as arguments
>> PCI: dwc: Fix dw_pcie_ep_raise_msix_irq() to get correct MSIX table
>> address
>> PCI: keystone: Add AM654 PCIe Endpoint to raise MSIX interrupt
>
> Trivial nits:
>
> - There's a mix of "MSI-X" and "MSIX" in the subjects, commit logs,
> and comments. I prefer "MSI-X" to match usage in the spec.
>
> - "Fixes:" tags need not include "commit". It doesn't *hurt*
> anything, but it takes up space that could be used for the
> subject.
>
> - Commit references typically use a 12-char SHA1. Again, doesn't
> hurt anything.
I'll fix all this in my next revision.
Xiaowei, Gustavo,
The issues we discussed in [1] should be fixed with this series. Can
you help test this in your platforms?
[1] -> https://lkml.org/lkml/2019/11/6/678
Thanks
Kishon
>
> -----Original Message-----
> From: Kishon Vijay Abraham I <[email protected]>
> Sent: 2020年1月9日 18:19
> To: Bjorn Helgaas <[email protected]>; Gustavo Pimentel
> <[email protected]>; Xiaowei Bao <[email protected]>
> Cc: Lorenzo Pieralisi <[email protected]>; Andrew Murray
> <[email protected]>; Murali Karicheri <[email protected]>; Jingoo
> Han <[email protected]>; [email protected];
> [email protected]; [email protected]
> Subject: Re: [PATCH 0/4] Redesign MSI-X support in PCIe Endpoint Core
>
> Hi,
>
> On 12/12/19 4:16 AM, Bjorn Helgaas wrote:
> > On Wed, Dec 11, 2019 at 06:16:04PM +0530, Kishon Vijay Abraham I wrote:
> >> Existing MSI-X support in Endpoint core has limitations:
> >> 1) MSIX table (which is mapped to a BAR) is not allocated by
> >> anyone. Ideally this should be allocated by endpoint
> >> function driver.
> >> 2) Endpoint controller can choose any random BARs for MSIX
> >> table (irrespective of whether the endpoint function driver
> >> has allocated memory for it or not)
> >>
> >> In order to avoid these limitations, pci_epc_set_msix() is modified
> >> to include BAR Indicator register (BIR) configuration and MSIX table
> >> offset as arguments. This series also fixed MSIX support in dwc
> >> driver and add MSI-X support in Cadence PCIe driver.
> >>
> >> The previous version of Cadence EP MSI-X support is @ [1].
> >> This series is created on top of [2]
> >>
> >> [1] ->
> >> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpat
> >>
> chwork.ozlabs.org%2Fpatch%2F971160%2F&data=02%7C01%7Cxiaowei
> .bao%
> >>
> 40nxp.com%7Ca1c78017032c4f4882b708d794ed1e9c%7C686ea1d3bc2b4c6
> fa92cd9
> >>
> 9c5c301635%7C0%7C0%7C637141618459605704&sdata=o1gFrMe%2B
> DERcNIVjK
> >> 5ZRJnDmO1QfAKQostQci05%2BrJA%3D&reserved=0
> >> [2] ->
> >> https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Flore
> >> .kernel.org%2Fr%2F20191209092147.22901-1-kishon%40ti.com&dat
> a=02%
> >>
> 7C01%7Cxiaowei.bao%40nxp.com%7Ca1c78017032c4f4882b708d794ed1e9
> c%7C686
> >>
> ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637141618459605704&a
> mp;sdata=
> >>
> fdZEFSCBc89vBEZlCUpuoIjZqrsqWPdNaNt3nMFdO0I%3D&reserved=0
> >>
> >> Alan Douglas (1):
> >> PCI: cadence: Add MSI-X support to Endpoint driver
> >>
> >> Kishon Vijay Abraham I (3):
> >> PCI: endpoint: Fix ->set_msix() to take BIR and offset as arguments
> >> PCI: dwc: Fix dw_pcie_ep_raise_msix_irq() to get correct MSIX table
> >> address
> >> PCI: keystone: Add AM654 PCIe Endpoint to raise MSIX interrupt
> >
> > Trivial nits:
> >
> > - There's a mix of "MSI-X" and "MSIX" in the subjects, commit logs,
> > and comments. I prefer "MSI-X" to match usage in the spec.
> >
> > - "Fixes:" tags need not include "commit". It doesn't *hurt*
> > anything, but it takes up space that could be used for the
> > subject.
> >
> > - Commit references typically use a 12-char SHA1. Again, doesn't
> > hurt anything.
>
> I'll fix all this in my next revision.
>
> Xiaowei, Gustavo,
>
> The issues we discussed in [1] should be fixed with this series. Can you help
> test this in your platforms?
OK, I will test it when I am free, and give the feedback to you.
Thanks
Xiaowei
>
> [1] ->
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flkml.or
> g%2Flkml%2F2019%2F11%2F6%2F678&data=02%7C01%7Cxiaowei.bao
> %40nxp.com%7Ca1c78017032c4f4882b708d794ed1e9c%7C686ea1d3bc2b4c
> 6fa92cd99c5c301635%7C0%7C0%7C637141618459605704&sdata=nmJ
> GiBLcEUnBFTaaoJUOhL290cmA7F%2F2uBnTpvw9ugo%3D&reserved=0
>
> Thanks
> Kishon
> >
Hi Kishon,
On Thu, Jan 9, 2020 at 10:19:17, Kishon Vijay Abraham I <[email protected]>
wrote:
> Hi,
>
> On 12/12/19 4:16 AM, Bjorn Helgaas wrote:
> > On Wed, Dec 11, 2019 at 06:16:04PM +0530, Kishon Vijay Abraham I wrote:
> >> Existing MSI-X support in Endpoint core has limitations:
> >> 1) MSIX table (which is mapped to a BAR) is not allocated by
> >> anyone. Ideally this should be allocated by endpoint
> >> function driver.
> >> 2) Endpoint controller can choose any random BARs for MSIX
> >> table (irrespective of whether the endpoint function driver
> >> has allocated memory for it or not)
> >>
> >> In order to avoid these limitations, pci_epc_set_msix() is
> >> modified to include BAR Indicator register (BIR) configuration
> >> and MSIX table offset as arguments. This series also fixed MSIX
> >> support in dwc driver and add MSI-X support in Cadence PCIe driver.
> >>
> >> The previous version of Cadence EP MSI-X support is @ [1].
> >> This series is created on top of [2]
> >>
> >> [1] -> https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.ozlabs.org_patch_971160_&d=DwICaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=bkWxpLoW-f-E3EdiDCCa0_h0PicsViasSlvIpzZvPxs&m=mDuurD6XufzL6j14X2LHC1ulMbU5dbmCtVUYVtCxNFM&s=IEKU31dHkOuXDfERPV1_QZ0U_BsjgCFgLwoE2ipAhFU&e=
> >> [2] -> https://urldefense.proofpoint.com/v2/url?u=http-3A__lore.kernel.org_r_20191209092147.22901-2D1-2Dkishon-40ti.com&d=DwICaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=bkWxpLoW-f-E3EdiDCCa0_h0PicsViasSlvIpzZvPxs&m=mDuurD6XufzL6j14X2LHC1ulMbU5dbmCtVUYVtCxNFM&s=9-DXCyz6iyuFk67BCnXeBt8HtJ-OOczk6ug_0ZZBgVE&e=
> >>
> >> Alan Douglas (1):
> >> PCI: cadence: Add MSI-X support to Endpoint driver
> >>
> >> Kishon Vijay Abraham I (3):
> >> PCI: endpoint: Fix ->set_msix() to take BIR and offset as arguments
> >> PCI: dwc: Fix dw_pcie_ep_raise_msix_irq() to get correct MSIX table
> >> address
> >> PCI: keystone: Add AM654 PCIe Endpoint to raise MSIX interrupt
> >
> > Trivial nits:
> >
> > - There's a mix of "MSI-X" and "MSIX" in the subjects, commit logs,
> > and comments. I prefer "MSI-X" to match usage in the spec.
> >
> > - "Fixes:" tags need not include "commit". It doesn't *hurt*
> > anything, but it takes up space that could be used for the
> > subject.
> >
> > - Commit references typically use a 12-char SHA1. Again, doesn't
> > hurt anything.
>
> I'll fix all this in my next revision.
>
> Xiaowei, Gustavo,
>
> The issues we discussed in [1] should be fixed with this series. Can
> you help test this in your platforms?
I didn't forget this, but unfortunately, I still don't have the HW
prototype required to be able to test this (there are some resources and
roadmap constraints that are blocking this).
To avoid blocking you and Xiaomi, I 'd suggest (assuming this MSI-X API
rework is working for layerscape and keystone drivers) to continue with
this patch series and take it to the mainline. If I get some problem with
my setup (as soon as I get the required conditions to test) I'll deal
with it then.
Regards
Gustavo
>
> [1] -> https://urldefense.proofpoint.com/v2/url?u=https-3A__lkml.org_lkml_2019_11_6_678&d=DwICaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=bkWxpLoW-f-E3EdiDCCa0_h0PicsViasSlvIpzZvPxs&m=mDuurD6XufzL6j14X2LHC1ulMbU5dbmCtVUYVtCxNFM&s=CbZ63jR-UW-NMY3U39htnXhperbQxlQX6dMQ9zpvBXg&e=
>
> Thanks
> Kishon
> >