| Message ID | 20191211124608.887-1-kishon@ti.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Redesign MSI-X support in PCIe Endpoint Core | expand |
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/20191209092147.22901-1-kishon@ti.com > > 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/20191209092147.22901-1-kishon@ti.com >> >> 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 <kishon@ti.com> > Sent: 2020年1月9日 18:19 > To: Bjorn Helgaas <helgaas@kernel.org>; Gustavo Pimentel > <gustavo.pimentel@synopsys.com>; Xiaowei Bao <xiaowei.bao@nxp.com> > Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>; Andrew Murray > <andrew.murray@arm.com>; Murali Karicheri <m-karicheri2@ti.com>; Jingoo > Han <jingoohan1@gmail.com>; linux-pci@vger.kernel.org; > linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org > 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 <kishon@ti.com> 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 > >
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/20191209092147.22901-1-kishon@ti.com 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(-)