| Message ID | bbee5395f02f728be88797bb450b999725ffbbc3.1530891871.git.gustavo.pimentel@synopsys.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
Hi, On Friday 06 July 2018 09:21 PM, Gustavo Pimentel wrote: > Add MSI-X support and update driver documentation accordingly. > > Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com> > --- > Change v2->v3: > - New patch file created base on the previous patch > "misc: pci_endpoint_test: Add MSI-X support" patch file following > Kishon's suggestion. > Change v3->v4: > - Rebased to Lorenzo's master branch v4.18-rc1. > Change v4->v5: > - Nothing changed, just to follow the patch set version. > Change v5->v6: > - Moved PCITEST_MSIX ioctl entry from patch #10 to here. > - Documented ioctl parameter type associated to > drivers/misc/pci_endpoint_test.c driver. > Change v6->v7: > - Updated documentation. > - Added flag that enables or not the MSI-X on the EP features. > Change v7->v8: > - Re-sending the patch series. > > Documentation/PCI/endpoint/pci-test-function.txt | 2 +- > Documentation/ioctl/ioctl-number.txt | 1 + > Documentation/misc-devices/pci-endpoint-test.txt | 3 +++ > drivers/misc/pci_endpoint_test.c | 29 +++++++++++++++++------- > drivers/pci/controller/dwc/pcie-designware-ep.c | 1 + > drivers/pci/endpoint/functions/pci-epf-test.c | 29 ++++++++++++++++++++++-- > include/linux/pci-epc.h | 1 + > include/uapi/linux/pcitest.h | 1 + > 8 files changed, 56 insertions(+), 11 deletions(-) > > diff --git a/Documentation/PCI/endpoint/pci-test-function.txt b/Documentation/PCI/endpoint/pci-test-function.txt > index 7ee2361..b1cbff3 100644 > --- a/Documentation/PCI/endpoint/pci-test-function.txt > +++ b/Documentation/PCI/endpoint/pci-test-function.txt > @@ -34,7 +34,7 @@ that the endpoint device must perform. > Bitfield Description: > Bit 0 : raise legacy IRQ > Bit 1 : raise MSI IRQ > - Bit 2 : raise MSI-X IRQ (reserved for future implementation) > + Bit 2 : raise MSI-X IRQ > Bit 3 : read command (read data from RC buffer) > Bit 4 : write command (write data to RC buffer) > Bit 5 : copy command (copy data from one RC buffer to another > diff --git a/Documentation/ioctl/ioctl-number.txt b/Documentation/ioctl/ioctl-number.txt > index 480c860..65259d4 100644 > --- a/Documentation/ioctl/ioctl-number.txt > +++ b/Documentation/ioctl/ioctl-number.txt > @@ -166,6 +166,7 @@ Code Seq#(hex) Include File Comments > 'P' all linux/soundcard.h conflict! > 'P' 60-6F sound/sscape_ioctl.h conflict! > 'P' 00-0F drivers/usb/class/usblp.c conflict! > +'P' 01-07 drivers/misc/pci_endpoint_test.c conflict! > 'Q' all linux/soundcard.h > 'R' 00-1F linux/random.h conflict! > 'R' 01 linux/rfkill.h conflict! > 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 349794c..f4fef10 100644 > --- a/drivers/misc/pci_endpoint_test.c > +++ b/drivers/misc/pci_endpoint_test.c > @@ -39,13 +39,14 @@ > > #define IRQ_TYPE_LEGACY 0 > #define IRQ_TYPE_MSI 1 > +#define IRQ_TYPE_MSIX 2 > > #define PCI_ENDPOINT_TEST_MAGIC 0x0 > > #define PCI_ENDPOINT_TEST_COMMAND 0x4 > #define COMMAND_RAISE_LEGACY_IRQ BIT(0) > #define COMMAND_RAISE_MSI_IRQ BIT(1) > -/* BIT(2) is reserved for raising MSI-X IRQ command */ > +#define COMMAND_RAISE_MSIX_IRQ BIT(2) > #define COMMAND_READ BIT(3) > #define COMMAND_WRITE BIT(4) > #define COMMAND_COPY BIT(5) > @@ -84,7 +85,7 @@ MODULE_PARM_DESC(no_msi, "Disable MSI interrupt in pci_endpoint_test"); > > static int irq_type = IRQ_TYPE_MSI; > module_param(irq_type, int, 0444); > -MODULE_PARM_DESC(irq_type, "IRQ mode selection in pci_endpoint_test (0 - Legacy, 1 - MSI)"); > +MODULE_PARM_DESC(irq_type, "IRQ mode selection in pci_endpoint_test (0 - Legacy, 1 - MSI, 2 - MSI-X)"); > > enum pci_barno { > BAR_0, > @@ -202,16 +203,18 @@ 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) > + u16 msi_num, bool msix) > { > u32 val; > struct pci_dev *pdev = test->pdev; > > pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_IRQ_TYPE, > - IRQ_TYPE_MSI); > + msix == false ? IRQ_TYPE_MSI : > + IRQ_TYPE_MSIX); > pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_IRQ_NUMBER, msi_num); > pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_COMMAND, > - COMMAND_RAISE_MSI_IRQ); > + msix == false ? COMMAND_RAISE_MSI_IRQ : > + COMMAND_RAISE_MSIX_IRQ); > val = wait_for_completion_timeout(&test->irq_raised, > msecs_to_jiffies(1000)); > if (!val) > @@ -456,7 +459,8 @@ static long pci_endpoint_test_ioctl(struct file *file, unsigned int cmd, > ret = pci_endpoint_test_legacy_irq(test); > break; > case PCITEST_MSI: > - ret = pci_endpoint_test_msi_irq(test, arg); > + case PCITEST_MSIX: > + ret = pci_endpoint_test_msi_irq(test, arg, cmd == PCITEST_MSIX); > break; > case PCITEST_WRITE: > ret = pci_endpoint_test_write(test, arg); > @@ -542,6 +546,12 @@ static int pci_endpoint_test_probe(struct pci_dev *pdev, > 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"); > } > @@ -558,8 +568,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", > - pci_irq_vector(pdev, i), i + 1); > + dev_err(dev, "Failed to request IRQ %d for MSI%s %d\n", > + pci_irq_vector(pdev, i), > + irq_type == IRQ_TYPE_MSIX ? "-X" : "", i + 1); > } > > for (bar = BAR_0; bar <= BAR_5; bar++) { > @@ -625,6 +636,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: > @@ -656,6 +668,7 @@ static void pci_endpoint_test_remove(struct pci_dev *pdev) > for (i = 0; i < test->num_irqs; i++) > devm_free_irq(&pdev->dev, pci_irq_vector(pdev, i), test); > pci_disable_msi(pdev); > + pci_disable_msix(pdev); > pci_release_regions(pdev); > pci_disable_device(pdev); > } > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c > index 70d0688..36d83d1 100644 > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c > @@ -585,6 +585,7 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep) > ep->msix_cap = dw_pcie_ep_find_capability(pci, PCI_CAP_ID_MSIX); > > epc->features = EPC_FEATURE_NO_LINKUP_NOTIFIER; > + epc->features |= EPC_FEATURE_MSIX_AVAILABLE; This indicates all vendors of designware has MSIX enabled which is not true. We'll need more logic than that. Thanks Kishon
Hi, On 09/07/2018 05:48, Kishon Vijay Abraham I wrote: > Hi, > > On Friday 06 July 2018 09:21 PM, Gustavo Pimentel wrote: >> Add MSI-X support and update driver documentation accordingly. >> >> Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com> >> --- >> Change v2->v3: >> - New patch file created base on the previous patch >> "misc: pci_endpoint_test: Add MSI-X support" patch file following >> Kishon's suggestion. >> Change v3->v4: >> - Rebased to Lorenzo's master branch v4.18-rc1. >> Change v4->v5: >> - Nothing changed, just to follow the patch set version. >> Change v5->v6: >> - Moved PCITEST_MSIX ioctl entry from patch #10 to here. >> - Documented ioctl parameter type associated to >> drivers/misc/pci_endpoint_test.c driver. >> Change v6->v7: >> - Updated documentation. >> - Added flag that enables or not the MSI-X on the EP features. >> Change v7->v8: >> - Re-sending the patch series. >> >> Documentation/PCI/endpoint/pci-test-function.txt | 2 +- >> Documentation/ioctl/ioctl-number.txt | 1 + >> Documentation/misc-devices/pci-endpoint-test.txt | 3 +++ >> drivers/misc/pci_endpoint_test.c | 29 +++++++++++++++++------- >> drivers/pci/controller/dwc/pcie-designware-ep.c | 1 + >> drivers/pci/endpoint/functions/pci-epf-test.c | 29 ++++++++++++++++++++++-- >> include/linux/pci-epc.h | 1 + >> include/uapi/linux/pcitest.h | 1 + >> 8 files changed, 56 insertions(+), 11 deletions(-) >> >> diff --git a/Documentation/PCI/endpoint/pci-test-function.txt b/Documentation/PCI/endpoint/pci-test-function.txt >> index 7ee2361..b1cbff3 100644 >> --- a/Documentation/PCI/endpoint/pci-test-function.txt >> +++ b/Documentation/PCI/endpoint/pci-test-function.txt >> @@ -34,7 +34,7 @@ that the endpoint device must perform. >> Bitfield Description: >> Bit 0 : raise legacy IRQ >> Bit 1 : raise MSI IRQ >> - Bit 2 : raise MSI-X IRQ (reserved for future implementation) >> + Bit 2 : raise MSI-X IRQ >> Bit 3 : read command (read data from RC buffer) >> Bit 4 : write command (write data to RC buffer) >> Bit 5 : copy command (copy data from one RC buffer to another >> diff --git a/Documentation/ioctl/ioctl-number.txt b/Documentation/ioctl/ioctl-number.txt >> index 480c860..65259d4 100644 >> --- a/Documentation/ioctl/ioctl-number.txt >> +++ b/Documentation/ioctl/ioctl-number.txt >> @@ -166,6 +166,7 @@ Code Seq#(hex) Include File Comments >> 'P' all linux/soundcard.h conflict! >> 'P' 60-6F sound/sscape_ioctl.h conflict! >> 'P' 00-0F drivers/usb/class/usblp.c conflict! >> +'P' 01-07 drivers/misc/pci_endpoint_test.c conflict! >> 'Q' all linux/soundcard.h >> 'R' 00-1F linux/random.h conflict! >> 'R' 01 linux/rfkill.h conflict! >> 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 349794c..f4fef10 100644 >> --- a/drivers/misc/pci_endpoint_test.c >> +++ b/drivers/misc/pci_endpoint_test.c >> @@ -39,13 +39,14 @@ >> >> #define IRQ_TYPE_LEGACY 0 >> #define IRQ_TYPE_MSI 1 >> +#define IRQ_TYPE_MSIX 2 >> >> #define PCI_ENDPOINT_TEST_MAGIC 0x0 >> >> #define PCI_ENDPOINT_TEST_COMMAND 0x4 >> #define COMMAND_RAISE_LEGACY_IRQ BIT(0) >> #define COMMAND_RAISE_MSI_IRQ BIT(1) >> -/* BIT(2) is reserved for raising MSI-X IRQ command */ >> +#define COMMAND_RAISE_MSIX_IRQ BIT(2) >> #define COMMAND_READ BIT(3) >> #define COMMAND_WRITE BIT(4) >> #define COMMAND_COPY BIT(5) >> @@ -84,7 +85,7 @@ MODULE_PARM_DESC(no_msi, "Disable MSI interrupt in pci_endpoint_test"); >> >> static int irq_type = IRQ_TYPE_MSI; >> module_param(irq_type, int, 0444); >> -MODULE_PARM_DESC(irq_type, "IRQ mode selection in pci_endpoint_test (0 - Legacy, 1 - MSI)"); >> +MODULE_PARM_DESC(irq_type, "IRQ mode selection in pci_endpoint_test (0 - Legacy, 1 - MSI, 2 - MSI-X)"); >> >> enum pci_barno { >> BAR_0, >> @@ -202,16 +203,18 @@ 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) >> + u16 msi_num, bool msix) >> { >> u32 val; >> struct pci_dev *pdev = test->pdev; >> >> pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_IRQ_TYPE, >> - IRQ_TYPE_MSI); >> + msix == false ? IRQ_TYPE_MSI : >> + IRQ_TYPE_MSIX); >> pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_IRQ_NUMBER, msi_num); >> pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_COMMAND, >> - COMMAND_RAISE_MSI_IRQ); >> + msix == false ? COMMAND_RAISE_MSI_IRQ : >> + COMMAND_RAISE_MSIX_IRQ); >> val = wait_for_completion_timeout(&test->irq_raised, >> msecs_to_jiffies(1000)); >> if (!val) >> @@ -456,7 +459,8 @@ static long pci_endpoint_test_ioctl(struct file *file, unsigned int cmd, >> ret = pci_endpoint_test_legacy_irq(test); >> break; >> case PCITEST_MSI: >> - ret = pci_endpoint_test_msi_irq(test, arg); >> + case PCITEST_MSIX: >> + ret = pci_endpoint_test_msi_irq(test, arg, cmd == PCITEST_MSIX); >> break; >> case PCITEST_WRITE: >> ret = pci_endpoint_test_write(test, arg); >> @@ -542,6 +546,12 @@ static int pci_endpoint_test_probe(struct pci_dev *pdev, >> 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"); >> } >> @@ -558,8 +568,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", >> - pci_irq_vector(pdev, i), i + 1); >> + dev_err(dev, "Failed to request IRQ %d for MSI%s %d\n", >> + pci_irq_vector(pdev, i), >> + irq_type == IRQ_TYPE_MSIX ? "-X" : "", i + 1); >> } >> >> for (bar = BAR_0; bar <= BAR_5; bar++) { >> @@ -625,6 +636,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: >> @@ -656,6 +668,7 @@ static void pci_endpoint_test_remove(struct pci_dev *pdev) >> for (i = 0; i < test->num_irqs; i++) >> devm_free_irq(&pdev->dev, pci_irq_vector(pdev, i), test); >> pci_disable_msi(pdev); >> + pci_disable_msix(pdev); >> pci_release_regions(pdev); >> pci_disable_device(pdev); >> } >> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c >> index 70d0688..36d83d1 100644 >> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c >> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c >> @@ -585,6 +585,7 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep) >> ep->msix_cap = dw_pcie_ep_find_capability(pci, PCI_CAP_ID_MSIX); >> >> epc->features = EPC_FEATURE_NO_LINKUP_NOTIFIER; >> + epc->features |= EPC_FEATURE_MSIX_AVAILABLE; > > This indicates all vendors of designware has MSIX enabled which is not true. > We'll need more logic than that. You mentioned and excellent point. Any particularity related to this features should be implemented each specific driver (in this case on pcie-designware-plat.c file). And by looking at this code now that you mentioned, I think the line code "epc->features = EPC_FEATURE_NO_LINKUP_NOTIFIER;" was added by mistake, if I remember well by default the linkup notification is expected, right? If I'm right, I may create an extra patch removing this 2 lines, do you agree? epc->features = EPC_FEATURE_NO_LINKUP_NOTIFIER; EPC_FEATURE_SET_BAR(epc->features, BAR_0); Meanwhile can you please ack the other patch files (#3 and #4)? They have remained unchanged for a long time. > > Thanks > Kishon >
Hi, On Monday 09 July 2018 03:38 PM, Gustavo Pimentel wrote: > Hi, > > On 09/07/2018 05:48, Kishon Vijay Abraham I wrote: >> Hi, >> >> On Friday 06 July 2018 09:21 PM, Gustavo Pimentel wrote: >>> Add MSI-X support and update driver documentation accordingly. >>> >>> Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com> >>> --- >>> Change v2->v3: >>> - New patch file created base on the previous patch >>> "misc: pci_endpoint_test: Add MSI-X support" patch file following >>> Kishon's suggestion. >>> Change v3->v4: >>> - Rebased to Lorenzo's master branch v4.18-rc1. >>> Change v4->v5: >>> - Nothing changed, just to follow the patch set version. >>> Change v5->v6: >>> - Moved PCITEST_MSIX ioctl entry from patch #10 to here. >>> - Documented ioctl parameter type associated to >>> drivers/misc/pci_endpoint_test.c driver. >>> Change v6->v7: >>> - Updated documentation. >>> - Added flag that enables or not the MSI-X on the EP features. >>> Change v7->v8: >>> - Re-sending the patch series. >>> >>> Documentation/PCI/endpoint/pci-test-function.txt | 2 +- >>> Documentation/ioctl/ioctl-number.txt | 1 + >>> Documentation/misc-devices/pci-endpoint-test.txt | 3 +++ >>> drivers/misc/pci_endpoint_test.c | 29 +++++++++++++++++------- >>> drivers/pci/controller/dwc/pcie-designware-ep.c | 1 + >>> drivers/pci/endpoint/functions/pci-epf-test.c | 29 ++++++++++++++++++++++-- >>> include/linux/pci-epc.h | 1 + >>> include/uapi/linux/pcitest.h | 1 + >>> 8 files changed, 56 insertions(+), 11 deletions(-) >>> >>> diff --git a/Documentation/PCI/endpoint/pci-test-function.txt b/Documentation/PCI/endpoint/pci-test-function.txt >>> index 7ee2361..b1cbff3 100644 >>> --- a/Documentation/PCI/endpoint/pci-test-function.txt >>> +++ b/Documentation/PCI/endpoint/pci-test-function.txt >>> @@ -34,7 +34,7 @@ that the endpoint device must perform. >>> Bitfield Description: >>> Bit 0 : raise legacy IRQ >>> Bit 1 : raise MSI IRQ >>> - Bit 2 : raise MSI-X IRQ (reserved for future implementation) >>> + Bit 2 : raise MSI-X IRQ >>> Bit 3 : read command (read data from RC buffer) >>> Bit 4 : write command (write data to RC buffer) >>> Bit 5 : copy command (copy data from one RC buffer to another >>> diff --git a/Documentation/ioctl/ioctl-number.txt b/Documentation/ioctl/ioctl-number.txt >>> index 480c860..65259d4 100644 >>> --- a/Documentation/ioctl/ioctl-number.txt >>> +++ b/Documentation/ioctl/ioctl-number.txt >>> @@ -166,6 +166,7 @@ Code Seq#(hex) Include File Comments >>> 'P' all linux/soundcard.h conflict! >>> 'P' 60-6F sound/sscape_ioctl.h conflict! >>> 'P' 00-0F drivers/usb/class/usblp.c conflict! >>> +'P' 01-07 drivers/misc/pci_endpoint_test.c conflict! >>> 'Q' all linux/soundcard.h >>> 'R' 00-1F linux/random.h conflict! >>> 'R' 01 linux/rfkill.h conflict! >>> 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 349794c..f4fef10 100644 >>> --- a/drivers/misc/pci_endpoint_test.c >>> +++ b/drivers/misc/pci_endpoint_test.c >>> @@ -39,13 +39,14 @@ >>> >>> #define IRQ_TYPE_LEGACY 0 >>> #define IRQ_TYPE_MSI 1 >>> +#define IRQ_TYPE_MSIX 2 >>> >>> #define PCI_ENDPOINT_TEST_MAGIC 0x0 >>> >>> #define PCI_ENDPOINT_TEST_COMMAND 0x4 >>> #define COMMAND_RAISE_LEGACY_IRQ BIT(0) >>> #define COMMAND_RAISE_MSI_IRQ BIT(1) >>> -/* BIT(2) is reserved for raising MSI-X IRQ command */ >>> +#define COMMAND_RAISE_MSIX_IRQ BIT(2) >>> #define COMMAND_READ BIT(3) >>> #define COMMAND_WRITE BIT(4) >>> #define COMMAND_COPY BIT(5) >>> @@ -84,7 +85,7 @@ MODULE_PARM_DESC(no_msi, "Disable MSI interrupt in pci_endpoint_test"); >>> >>> static int irq_type = IRQ_TYPE_MSI; >>> module_param(irq_type, int, 0444); >>> -MODULE_PARM_DESC(irq_type, "IRQ mode selection in pci_endpoint_test (0 - Legacy, 1 - MSI)"); >>> +MODULE_PARM_DESC(irq_type, "IRQ mode selection in pci_endpoint_test (0 - Legacy, 1 - MSI, 2 - MSI-X)"); >>> >>> enum pci_barno { >>> BAR_0, >>> @@ -202,16 +203,18 @@ 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) >>> + u16 msi_num, bool msix) >>> { >>> u32 val; >>> struct pci_dev *pdev = test->pdev; >>> >>> pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_IRQ_TYPE, >>> - IRQ_TYPE_MSI); >>> + msix == false ? IRQ_TYPE_MSI : >>> + IRQ_TYPE_MSIX); >>> pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_IRQ_NUMBER, msi_num); >>> pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_COMMAND, >>> - COMMAND_RAISE_MSI_IRQ); >>> + msix == false ? COMMAND_RAISE_MSI_IRQ : >>> + COMMAND_RAISE_MSIX_IRQ); >>> val = wait_for_completion_timeout(&test->irq_raised, >>> msecs_to_jiffies(1000)); >>> if (!val) >>> @@ -456,7 +459,8 @@ static long pci_endpoint_test_ioctl(struct file *file, unsigned int cmd, >>> ret = pci_endpoint_test_legacy_irq(test); >>> break; >>> case PCITEST_MSI: >>> - ret = pci_endpoint_test_msi_irq(test, arg); >>> + case PCITEST_MSIX: >>> + ret = pci_endpoint_test_msi_irq(test, arg, cmd == PCITEST_MSIX); >>> break; >>> case PCITEST_WRITE: >>> ret = pci_endpoint_test_write(test, arg); >>> @@ -542,6 +546,12 @@ static int pci_endpoint_test_probe(struct pci_dev *pdev, >>> 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"); >>> } >>> @@ -558,8 +568,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", >>> - pci_irq_vector(pdev, i), i + 1); >>> + dev_err(dev, "Failed to request IRQ %d for MSI%s %d\n", >>> + pci_irq_vector(pdev, i), >>> + irq_type == IRQ_TYPE_MSIX ? "-X" : "", i + 1); >>> } >>> >>> for (bar = BAR_0; bar <= BAR_5; bar++) { >>> @@ -625,6 +636,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: >>> @@ -656,6 +668,7 @@ static void pci_endpoint_test_remove(struct pci_dev *pdev) >>> for (i = 0; i < test->num_irqs; i++) >>> devm_free_irq(&pdev->dev, pci_irq_vector(pdev, i), test); >>> pci_disable_msi(pdev); >>> + pci_disable_msix(pdev); >>> pci_release_regions(pdev); >>> pci_disable_device(pdev); >>> } >>> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c >>> index 70d0688..36d83d1 100644 >>> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c >>> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c >>> @@ -585,6 +585,7 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep) >>> ep->msix_cap = dw_pcie_ep_find_capability(pci, PCI_CAP_ID_MSIX); >>> >>> epc->features = EPC_FEATURE_NO_LINKUP_NOTIFIER; >>> + epc->features |= EPC_FEATURE_MSIX_AVAILABLE; >> >> This indicates all vendors of designware has MSIX enabled which is not true. >> We'll need more logic than that. > > You mentioned and excellent point. Any particularity related to this features > should be implemented each specific driver (in this case on > pcie-designware-plat.c file). > > And by looking at this code now that you mentioned, I think the line code > "epc->features = EPC_FEATURE_NO_LINKUP_NOTIFIER;" was added by mistake, if I > remember well by default the linkup notification is expected, right? right, since dra7x was the first platform to have EP support added and it has linkup notification mechanism. > > If I'm right, I may create an extra patch removing this 2 lines, do you agree? Agree. I think we should use ->ep_init() present in dw_pcie_ep_ops for populating features. ->ep_init() right now is called in dw_pcie_ep_init() which is not needed. Stuffs that are right now done in ->ep_init callbacks can be done even before invoking dw_pcie_ep_init(). We might have to add a new API pci_epc_init() to be invoked from function driver, which should invoke ->ep_init() callback. The features of a particular platform should be populated here. Thanks Kishon
Hi, On 09/07/2018 11:26, Kishon Vijay Abraham I wrote: > Hi, > > On Monday 09 July 2018 03:38 PM, Gustavo Pimentel wrote: >> Hi, >> >> On 09/07/2018 05:48, Kishon Vijay Abraham I wrote: >>> Hi, >>> >>> On Friday 06 July 2018 09:21 PM, Gustavo Pimentel wrote: >>>> Add MSI-X support and update driver documentation accordingly. >>>> >>>> Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com> >>>> --- >>>> Change v2->v3: >>>> - New patch file created base on the previous patch >>>> "misc: pci_endpoint_test: Add MSI-X support" patch file following >>>> Kishon's suggestion. >>>> Change v3->v4: >>>> - Rebased to Lorenzo's master branch v4.18-rc1. >>>> Change v4->v5: >>>> - Nothing changed, just to follow the patch set version. >>>> Change v5->v6: >>>> - Moved PCITEST_MSIX ioctl entry from patch #10 to here. >>>> - Documented ioctl parameter type associated to >>>> drivers/misc/pci_endpoint_test.c driver. >>>> Change v6->v7: >>>> - Updated documentation. >>>> - Added flag that enables or not the MSI-X on the EP features. >>>> Change v7->v8: >>>> - Re-sending the patch series. >>>> >>>> Documentation/PCI/endpoint/pci-test-function.txt | 2 +- >>>> Documentation/ioctl/ioctl-number.txt | 1 + >>>> Documentation/misc-devices/pci-endpoint-test.txt | 3 +++ >>>> drivers/misc/pci_endpoint_test.c | 29 +++++++++++++++++------- >>>> drivers/pci/controller/dwc/pcie-designware-ep.c | 1 + >>>> drivers/pci/endpoint/functions/pci-epf-test.c | 29 ++++++++++++++++++++++-- >>>> include/linux/pci-epc.h | 1 + >>>> include/uapi/linux/pcitest.h | 1 + >>>> 8 files changed, 56 insertions(+), 11 deletions(-) >>>> >>>> diff --git a/Documentation/PCI/endpoint/pci-test-function.txt b/Documentation/PCI/endpoint/pci-test-function.txt >>>> index 7ee2361..b1cbff3 100644 >>>> --- a/Documentation/PCI/endpoint/pci-test-function.txt >>>> +++ b/Documentation/PCI/endpoint/pci-test-function.txt >>>> @@ -34,7 +34,7 @@ that the endpoint device must perform. >>>> Bitfield Description: >>>> Bit 0 : raise legacy IRQ >>>> Bit 1 : raise MSI IRQ >>>> - Bit 2 : raise MSI-X IRQ (reserved for future implementation) >>>> + Bit 2 : raise MSI-X IRQ >>>> Bit 3 : read command (read data from RC buffer) >>>> Bit 4 : write command (write data to RC buffer) >>>> Bit 5 : copy command (copy data from one RC buffer to another >>>> diff --git a/Documentation/ioctl/ioctl-number.txt b/Documentation/ioctl/ioctl-number.txt >>>> index 480c860..65259d4 100644 >>>> --- a/Documentation/ioctl/ioctl-number.txt >>>> +++ b/Documentation/ioctl/ioctl-number.txt >>>> @@ -166,6 +166,7 @@ Code Seq#(hex) Include File Comments >>>> 'P' all linux/soundcard.h conflict! >>>> 'P' 60-6F sound/sscape_ioctl.h conflict! >>>> 'P' 00-0F drivers/usb/class/usblp.c conflict! >>>> +'P' 01-07 drivers/misc/pci_endpoint_test.c conflict! >>>> 'Q' all linux/soundcard.h >>>> 'R' 00-1F linux/random.h conflict! >>>> 'R' 01 linux/rfkill.h conflict! >>>> 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 349794c..f4fef10 100644 >>>> --- a/drivers/misc/pci_endpoint_test.c >>>> +++ b/drivers/misc/pci_endpoint_test.c >>>> @@ -39,13 +39,14 @@ >>>> >>>> #define IRQ_TYPE_LEGACY 0 >>>> #define IRQ_TYPE_MSI 1 >>>> +#define IRQ_TYPE_MSIX 2 >>>> >>>> #define PCI_ENDPOINT_TEST_MAGIC 0x0 >>>> >>>> #define PCI_ENDPOINT_TEST_COMMAND 0x4 >>>> #define COMMAND_RAISE_LEGACY_IRQ BIT(0) >>>> #define COMMAND_RAISE_MSI_IRQ BIT(1) >>>> -/* BIT(2) is reserved for raising MSI-X IRQ command */ >>>> +#define COMMAND_RAISE_MSIX_IRQ BIT(2) >>>> #define COMMAND_READ BIT(3) >>>> #define COMMAND_WRITE BIT(4) >>>> #define COMMAND_COPY BIT(5) >>>> @@ -84,7 +85,7 @@ MODULE_PARM_DESC(no_msi, "Disable MSI interrupt in pci_endpoint_test"); >>>> >>>> static int irq_type = IRQ_TYPE_MSI; >>>> module_param(irq_type, int, 0444); >>>> -MODULE_PARM_DESC(irq_type, "IRQ mode selection in pci_endpoint_test (0 - Legacy, 1 - MSI)"); >>>> +MODULE_PARM_DESC(irq_type, "IRQ mode selection in pci_endpoint_test (0 - Legacy, 1 - MSI, 2 - MSI-X)"); >>>> >>>> enum pci_barno { >>>> BAR_0, >>>> @@ -202,16 +203,18 @@ 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) >>>> + u16 msi_num, bool msix) >>>> { >>>> u32 val; >>>> struct pci_dev *pdev = test->pdev; >>>> >>>> pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_IRQ_TYPE, >>>> - IRQ_TYPE_MSI); >>>> + msix == false ? IRQ_TYPE_MSI : >>>> + IRQ_TYPE_MSIX); >>>> pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_IRQ_NUMBER, msi_num); >>>> pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_COMMAND, >>>> - COMMAND_RAISE_MSI_IRQ); >>>> + msix == false ? COMMAND_RAISE_MSI_IRQ : >>>> + COMMAND_RAISE_MSIX_IRQ); >>>> val = wait_for_completion_timeout(&test->irq_raised, >>>> msecs_to_jiffies(1000)); >>>> if (!val) >>>> @@ -456,7 +459,8 @@ static long pci_endpoint_test_ioctl(struct file *file, unsigned int cmd, >>>> ret = pci_endpoint_test_legacy_irq(test); >>>> break; >>>> case PCITEST_MSI: >>>> - ret = pci_endpoint_test_msi_irq(test, arg); >>>> + case PCITEST_MSIX: >>>> + ret = pci_endpoint_test_msi_irq(test, arg, cmd == PCITEST_MSIX); >>>> break; >>>> case PCITEST_WRITE: >>>> ret = pci_endpoint_test_write(test, arg); >>>> @@ -542,6 +546,12 @@ static int pci_endpoint_test_probe(struct pci_dev *pdev, >>>> 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"); >>>> } >>>> @@ -558,8 +568,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", >>>> - pci_irq_vector(pdev, i), i + 1); >>>> + dev_err(dev, "Failed to request IRQ %d for MSI%s %d\n", >>>> + pci_irq_vector(pdev, i), >>>> + irq_type == IRQ_TYPE_MSIX ? "-X" : "", i + 1); >>>> } >>>> >>>> for (bar = BAR_0; bar <= BAR_5; bar++) { >>>> @@ -625,6 +636,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: >>>> @@ -656,6 +668,7 @@ static void pci_endpoint_test_remove(struct pci_dev *pdev) >>>> for (i = 0; i < test->num_irqs; i++) >>>> devm_free_irq(&pdev->dev, pci_irq_vector(pdev, i), test); >>>> pci_disable_msi(pdev); >>>> + pci_disable_msix(pdev); >>>> pci_release_regions(pdev); >>>> pci_disable_device(pdev); >>>> } >>>> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c >>>> index 70d0688..36d83d1 100644 >>>> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c >>>> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c >>>> @@ -585,6 +585,7 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep) >>>> ep->msix_cap = dw_pcie_ep_find_capability(pci, PCI_CAP_ID_MSIX); >>>> >>>> epc->features = EPC_FEATURE_NO_LINKUP_NOTIFIER; >>>> + epc->features |= EPC_FEATURE_MSIX_AVAILABLE; >>> >>> This indicates all vendors of designware has MSIX enabled which is not true. >>> We'll need more logic than that. >> >> You mentioned and excellent point. Any particularity related to this features >> should be implemented each specific driver (in this case on >> pcie-designware-plat.c file). >> >> And by looking at this code now that you mentioned, I think the line code >> "epc->features = EPC_FEATURE_NO_LINKUP_NOTIFIER;" was added by mistake, if I >> remember well by default the linkup notification is expected, right? > > right, since dra7x was the first platform to have EP support added and it has > linkup notification mechanism. >> >> If I'm right, I may create an extra patch removing this 2 lines, do you agree? I will send a patch as soon as possible to fix this nasty bug. Unfortunately it will be more than just deleting 2 lines.... > > Agree. I think we should use ->ep_init() present in dw_pcie_ep_ops for > populating features. ->ep_init() right now is called in dw_pcie_ep_init() which > is not needed. Stuffs that are right now done in ->ep_init callbacks can be > done even before invoking dw_pcie_ep_init(). I don't think so, that we can invoke before, dw_pcie_ep_init() is responsible for creating the epc object and set the object address to the epc pointer present on the ep struct. This pointer is used later to access and set the feature field. > > We might have to add a new API pci_epc_init() to be invoked from function > driver, which should invoke ->ep_init() callback. The features of a particular > platform should be populated here. My solution for now is to repair this damage as soon as possible, for that I'll move the the feature setting from the pcie-designware-ep.c to the pcie-designware-plat.c and change some code order to work. I propose we implement this change after this patch has entered, otherwise we'll be adding yet another degree of complexity to this series, already quite complex. Can you test the the patch series v9 on your side Kishon? Regards, Gustavo > > Thanks > Kishon >
diff --git a/Documentation/PCI/endpoint/pci-test-function.txt b/Documentation/PCI/endpoint/pci-test-function.txt index 7ee2361..b1cbff3 100644 --- a/Documentation/PCI/endpoint/pci-test-function.txt +++ b/Documentation/PCI/endpoint/pci-test-function.txt @@ -34,7 +34,7 @@ that the endpoint device must perform. Bitfield Description: Bit 0 : raise legacy IRQ Bit 1 : raise MSI IRQ - Bit 2 : raise MSI-X IRQ (reserved for future implementation) + Bit 2 : raise MSI-X IRQ Bit 3 : read command (read data from RC buffer) Bit 4 : write command (write data to RC buffer) Bit 5 : copy command (copy data from one RC buffer to another diff --git a/Documentation/ioctl/ioctl-number.txt b/Documentation/ioctl/ioctl-number.txt index 480c860..65259d4 100644 --- a/Documentation/ioctl/ioctl-number.txt +++ b/Documentation/ioctl/ioctl-number.txt @@ -166,6 +166,7 @@ Code Seq#(hex) Include File Comments 'P' all linux/soundcard.h conflict! 'P' 60-6F sound/sscape_ioctl.h conflict! 'P' 00-0F drivers/usb/class/usblp.c conflict! +'P' 01-07 drivers/misc/pci_endpoint_test.c conflict! 'Q' all linux/soundcard.h 'R' 00-1F linux/random.h conflict! 'R' 01 linux/rfkill.h conflict! 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 349794c..f4fef10 100644 --- a/drivers/misc/pci_endpoint_test.c +++ b/drivers/misc/pci_endpoint_test.c @@ -39,13 +39,14 @@ #define IRQ_TYPE_LEGACY 0 #define IRQ_TYPE_MSI 1 +#define IRQ_TYPE_MSIX 2 #define PCI_ENDPOINT_TEST_MAGIC 0x0 #define PCI_ENDPOINT_TEST_COMMAND 0x4 #define COMMAND_RAISE_LEGACY_IRQ BIT(0) #define COMMAND_RAISE_MSI_IRQ BIT(1) -/* BIT(2) is reserved for raising MSI-X IRQ command */ +#define COMMAND_RAISE_MSIX_IRQ BIT(2) #define COMMAND_READ BIT(3) #define COMMAND_WRITE BIT(4) #define COMMAND_COPY BIT(5) @@ -84,7 +85,7 @@ MODULE_PARM_DESC(no_msi, "Disable MSI interrupt in pci_endpoint_test"); static int irq_type = IRQ_TYPE_MSI; module_param(irq_type, int, 0444); -MODULE_PARM_DESC(irq_type, "IRQ mode selection in pci_endpoint_test (0 - Legacy, 1 - MSI)"); +MODULE_PARM_DESC(irq_type, "IRQ mode selection in pci_endpoint_test (0 - Legacy, 1 - MSI, 2 - MSI-X)"); enum pci_barno { BAR_0, @@ -202,16 +203,18 @@ 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) + u16 msi_num, bool msix) { u32 val; struct pci_dev *pdev = test->pdev; pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_IRQ_TYPE, - IRQ_TYPE_MSI); + msix == false ? IRQ_TYPE_MSI : + IRQ_TYPE_MSIX); pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_IRQ_NUMBER, msi_num); pci_endpoint_test_writel(test, PCI_ENDPOINT_TEST_COMMAND, - COMMAND_RAISE_MSI_IRQ); + msix == false ? COMMAND_RAISE_MSI_IRQ : + COMMAND_RAISE_MSIX_IRQ); val = wait_for_completion_timeout(&test->irq_raised, msecs_to_jiffies(1000)); if (!val) @@ -456,7 +459,8 @@ static long pci_endpoint_test_ioctl(struct file *file, unsigned int cmd, ret = pci_endpoint_test_legacy_irq(test); break; case PCITEST_MSI: - ret = pci_endpoint_test_msi_irq(test, arg); + case PCITEST_MSIX: + ret = pci_endpoint_test_msi_irq(test, arg, cmd == PCITEST_MSIX); break; case PCITEST_WRITE: ret = pci_endpoint_test_write(test, arg); @@ -542,6 +546,12 @@ static int pci_endpoint_test_probe(struct pci_dev *pdev, 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"); } @@ -558,8 +568,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", - pci_irq_vector(pdev, i), i + 1); + dev_err(dev, "Failed to request IRQ %d for MSI%s %d\n", + pci_irq_vector(pdev, i), + irq_type == IRQ_TYPE_MSIX ? "-X" : "", i + 1); } for (bar = BAR_0; bar <= BAR_5; bar++) { @@ -625,6 +636,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: @@ -656,6 +668,7 @@ static void pci_endpoint_test_remove(struct pci_dev *pdev) for (i = 0; i < test->num_irqs; i++) devm_free_irq(&pdev->dev, pci_irq_vector(pdev, i), test); pci_disable_msi(pdev); + pci_disable_msix(pdev); pci_release_regions(pdev); pci_disable_device(pdev); } diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c index 70d0688..36d83d1 100644 --- a/drivers/pci/controller/dwc/pcie-designware-ep.c +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c @@ -585,6 +585,7 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep) ep->msix_cap = dw_pcie_ep_find_capability(pci, PCI_CAP_ID_MSIX); epc->features = EPC_FEATURE_NO_LINKUP_NOTIFIER; + epc->features |= EPC_FEATURE_MSIX_AVAILABLE; EPC_FEATURE_SET_BAR(epc->features, BAR_0); ep->epc = epc; diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c index eb9cd00..123f58c 100644 --- a/drivers/pci/endpoint/functions/pci-epf-test.c +++ b/drivers/pci/endpoint/functions/pci-epf-test.c @@ -20,10 +20,11 @@ #define IRQ_TYPE_LEGACY 0 #define IRQ_TYPE_MSI 1 +#define IRQ_TYPE_MSIX 2 #define COMMAND_RAISE_LEGACY_IRQ BIT(0) #define COMMAND_RAISE_MSI_IRQ BIT(1) -/* BIT(2) is reserved for raising MSI-X IRQ command */ +#define COMMAND_RAISE_MSIX_IRQ BIT(2) #define COMMAND_READ BIT(3) #define COMMAND_WRITE BIT(4) #define COMMAND_COPY BIT(5) @@ -47,6 +48,7 @@ struct pci_epf_test { struct pci_epf *epf; enum pci_barno test_reg_bar; bool linkup_notifier; + bool msix_available; struct delayed_work cmd_handler; }; @@ -266,6 +268,9 @@ static void pci_epf_test_raise_irq(struct pci_epf_test *epf_test, u8 irq_type, 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; @@ -292,7 +297,7 @@ static void pci_epf_test_cmd_handler(struct work_struct *work) reg->command = 0; reg->status = 0; - if (reg->irq_type > IRQ_TYPE_MSI) { + if (reg->irq_type > IRQ_TYPE_MSIX) { dev_err(dev, "Failed to detect IRQ type\n"); goto reset_handler; } @@ -346,6 +351,16 @@ static void pci_epf_test_cmd_handler(struct work_struct *work) goto reset_handler; } + if (command & COMMAND_RAISE_MSIX_IRQ) { + count = pci_epc_get_msix(epc, epf->func_no); + if (reg->irq_number > count || count <= 0) + goto reset_handler; + reg->status = STATUS_IRQ_RAISED; + pci_epc_raise_irq(epc, epf->func_no, PCI_EPC_IRQ_MSIX, + reg->irq_number); + goto reset_handler; + } + reset_handler: queue_delayed_work(kpcitest_workqueue, &epf_test->cmd_handler, msecs_to_jiffies(1)); @@ -459,6 +474,8 @@ static int pci_epf_test_bind(struct pci_epf *epf) else epf_test->linkup_notifier = true; + epf_test->msix_available = epc->features & EPC_FEATURE_MSIX_AVAILABLE; + epf_test->test_reg_bar = EPC_FEATURE_GET_BAR(epc->features); ret = pci_epc_write_header(epc, epf->func_no, header); @@ -481,6 +498,14 @@ static int pci_epf_test_bind(struct pci_epf *epf) return ret; } + if (epf_test->msix_available) { + 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); diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h index bb2395b..37dab81 100644 --- a/include/linux/pci-epc.h +++ b/include/linux/pci-epc.h @@ -102,6 +102,7 @@ struct pci_epc { #define EPC_FEATURE_NO_LINKUP_NOTIFIER BIT(0) #define EPC_FEATURE_BAR_MASK (BIT(1) | BIT(2) | BIT(3)) +#define EPC_FEATURE_MSIX_AVAILABLE BIT(4) #define EPC_FEATURE_SET_BAR(features, bar) \ (features |= (EPC_FEATURE_BAR_MASK & (bar << 1))) #define EPC_FEATURE_GET_BAR(features) \ 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 */
Add MSI-X support and update driver documentation accordingly. Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com> --- Change v2->v3: - New patch file created base on the previous patch "misc: pci_endpoint_test: Add MSI-X support" patch file following Kishon's suggestion. Change v3->v4: - Rebased to Lorenzo's master branch v4.18-rc1. Change v4->v5: - Nothing changed, just to follow the patch set version. Change v5->v6: - Moved PCITEST_MSIX ioctl entry from patch #10 to here. - Documented ioctl parameter type associated to drivers/misc/pci_endpoint_test.c driver. Change v6->v7: - Updated documentation. - Added flag that enables or not the MSI-X on the EP features. Change v7->v8: - Re-sending the patch series. Documentation/PCI/endpoint/pci-test-function.txt | 2 +- Documentation/ioctl/ioctl-number.txt | 1 + Documentation/misc-devices/pci-endpoint-test.txt | 3 +++ drivers/misc/pci_endpoint_test.c | 29 +++++++++++++++++------- drivers/pci/controller/dwc/pcie-designware-ep.c | 1 + drivers/pci/endpoint/functions/pci-epf-test.c | 29 ++++++++++++++++++++++-- include/linux/pci-epc.h | 1 + include/uapi/linux/pcitest.h | 1 + 8 files changed, 56 insertions(+), 11 deletions(-)