On 29/12/2017 10:05, Kishon Vijay Abraham I wrote: > Hi Gustavo, > > On Friday 29 December 2017 03:18 PM, Gustavo Pimentel wrote: >> Hi Kishon, >> >> On 28/12/2017 14:58, Kishon Vijay Abraham I wrote: >>> Hi Gustavo, >>> >>> On Wednesday 27 December 2017 07:55 PM, Gustavo Pimentel wrote: >>>> On 26/12/2017 12:57, Kishon Vijay Abraham I wrote: >>>>> Hi, >>>>> >>>>> On Saturday 23 December 2017 03:46 PM, Kishon Vijay Abraham I wrote: >>>>>> Hi Gustavo, >>>>>> >>>>>> On Thursday 21 December 2017 07:38 PM, Gustavo Pimentel wrote: >>>>>>> On 20/12/2017 13:03, Kishon Vijay Abraham I wrote:This is our most recent >>>>>>> version of the patches that are running on our equipment, please check with yours >>>>>>>> Hi, >>>>>>>> >>>>>>>> On Monday 18 December 2017 09:31 PM, Gustavo Pimentel wrote: >>>>>>>>> Hi Kishon, >>>>>>>>> >>>>>>>>> I would like to collaborate with you on this subject, I have on my side João's >>>>>>>>> patches updated to the Bjorn's latest kernel version. >>>>>>>> >>>>>>>> cool, do you have a branch so that I can check what breaks in keystone and debug? >>>>>>>> >>>>>>> Unfortunately not, however I'll mailed you the patches immediately. >>>>>>> Once again, thanks. >>>>>> >>>>>> Thanks. >>>>>> >>>>>> I tried raising MSI with pci_endpoint_test EP device (16 MSI interrupts). Here >>>>>> are some of my observations after applying your patch series. >>>>>> >>>>>> root@k2g-evm:~# cat /proc/interrupts >>>>>> CPU0 >>>>>> 17: 0 GICv2 29 Level arch_timer >>>>>> 18: 1816 GICv2 30 Level arch_timer >>>>>> 21: 0 GICv2 36 Edge arm-pmu >>>>>> 22: 792 GICv2 196 Edge ttyS0 >>>>>> 23: 5 GICv2 120 Edge 2530000.i2c >>>>>> 24: 0 GICv2 33 Edge soc:keystone_irq@26202a0 >>>>>> 25: 901 GICv2 356 Level 2a00000.msgmgr rx_005_002 >>>>>> 41: 0 GICv2 232 Edge 2700000.edma_ccint >>>>>> 43: 0 GICv2 249 Edge 2700000.edma_ccerrint >>>>>> 44: 2497 GICv2 240 Edge 2728000.edma_ccint >>>>>> 46: 0 GICv2 252 Edge 2728000.edma_ccerrint >>>>>> 47: 7551 GICv2 128 Edge mmc0 >>>>>> 48: 0 GICv2 160 Edge 2680000.keystone-dwc3, xhci-hcd:usb1 >>>>>> 49: 0 GICv2 176 Edge 2580000.keystone-dwc3, xhci-hcd:usb3 >>>>>> 50: 0 GICv2 96 Edge 21805400.spi >>>>>> 52: 0 GICv2 100 Edge 21805c00.spi >>>>>> 53: 0 GICv2 102 Edge 21806000.spi >>>>>> 54: 0 GICv2 92 Edge pcie-error-irq >>>>>> 211: 0 GPIO 12 Edge -davinci_gpio 23000000.mmc cd >>>>>> 280: 0 PCI-MSI 0 Edge PCIe PME, aerdrv >>>>>> 281: 0 PCI-MSI 524288 Edge pci-endpoint-test >>>>>> 282: 0 PCI-MSI 524289 Edge pci-endpoint-test >>>>>> 283: 0 PCI-MSI 524290 Edge pci-endpoint-test >>>>>> 284: 0 PCI-MSI 524291 Edge pci-endpoint-test >>>>>> 285: 0 PCI-MSI 524292 Edge pci-endpoint-test >>>>>> 286: 0 PCI-MSI 524293 Edge pci-endpoint-test >>>>>> 287: 0 PCI-MSI 524294 Edge pci-endpoint-test >>>>>> 288: 0 PCI-MSI 524295 Edge pci-endpoint-test >>>>>> 289: 0 PCI-MSI 524296 Edge pci-endpoint-test >>>>>> 290: 0 PCI-MSI 524297 Edge pci-endpoint-test >>>>>> 291: 0 PCI-MSI 524298 Edge pci-endpoint-test >>>>>> 292: 0 PCI-MSI 524299 Edge pci-endpoint-test >>>>>> 293: 0 PCI-MSI 524300 Edge pci-endpoint-test >>>>>> 294: 0 PCI-MSI 524301 Edge pci-endpoint-test >>>>>> 295: 0 PCI-MSI 524302 Edge pci-endpoint-test >>>>>> 296: 0 PCI-MSI 524303 Edge pci-endpoint-test >>>>>> IPI0: 0 CPU wakeup interrupts >>>>>> IPI1: 0 Timer broadcast interrupts >>>>>> IPI2: 0 Rescheduling interrupts >>>>>> IPI3: 0 Function call interrupts >>>>>> IPI4: 0 CPU stop interrupts >>>>>> IPI5: 0 IRQ work interrupts >>>>>> IPI6: 0 completion interrupts >>>>>> Err: 0 >>>>>> >>>>>> root@k2g-evm:~# pcitest -m 1 >>>>>> [ 45.966437] keystone-pcie 21801000.pcie: ks_pcie_msi_irq_handler, irq 271 >>>>>> [ 45.973544] keystone-pcie 21801000.pcie: irq: bit 0, vector 0, virq 280 >>>>>> MSI1: NOT OKAY >>>>>> >>>>>> Here there is an off by one error. "pcitest -m 1" should ideally have raised an >>>>>> interrupt in 281 but from the above debug print it looks like it's raising an >>>>>> interrupt in 280. >>>>> >>>>> The above error was due to an pci_endpoint bug. I'll send a patch to fix this >>>>> separately. >>>> >>>> Ok great! >>> >>> This too looks like a bug with the new API. If the EP requests 16 interrupts, >>> the MSI vector (programmed in PCI_MSI_DATA) should be 0 and if 0 is not >>> available it should be 16 >>> [From the spec: The Multiple Message Enable field (bits 6-4 of the Message >>> Control register) defines the number of low >>> order message data bits the function is permitted to modify to generate its >>> system software allocated vectors]. For 16 interrupts, Multiple Message Enable >>> will have a value of 4 which means the EP can modify the 4 low order bits to >>> raise the 16 interrupts. >>> >>> With this series added, PCI_MSI_DATA is programmed to 0x1 which gets masked >>> while the EP tries to raise an interrupt and hence we are observing off by one >>> error. So the problem that you saying could be on setting the correct value on the PCI_MSI_DATA, right? While I'm trying to support pcitest to be able in future to run the same tests that you on my system, I have added some debug on msi.c file that could provide me the same info that you have refer before in a short time. With the same USB 3.1 PCIe board as a EP, I have generated 2 linux images (with and without the patches series). Without the patches DEBUG:drivers/pci/msi.c:pci_msi_vec_count:881 msgctl = 0x86 DEBUG:drivers/pci/msi.c:msi_setup_entry:554 control = 0x86 DEBUG:drivers/pci/msi.c:msi_setup_entry:555 nvec = 0x1 DEBUG:drivers/pci/msi.c:msi_setup_entry:563 entry->msi_attrib.multi_cap = 0x3 DEBUG:drivers/pci/msi.c:msi_setup_entry:564 entry->msi_attrib.multiple = 0x0 DEBUG:drivers/pci/msi.c:__pci_write_msi_msg:314 msgctl = 0x86 DEBUG:drivers/pci/msi.c:__pci_write_msi_msg:317 entry->msi_attrib.multiple = 0x0, msgctl = 0x86 With the patches (I have commented the MSI_FLAG_PCI_MSIX flag) DEBUG:drivers/pci/msi.c:pci_msi_vec_count:882 msgctl = 0x86 DEBUG:drivers/pci/msi.c:msi_setup_entry:555 control = 0x86 DEBUG:drivers/pci/msi.c:msi_setup_entry:556 nvec = 0x1 DEBUG:drivers/pci/msi.c:msi_setup_entry:564 entry->msi_attrib.multi_cap = 0x3 DEBUG:drivers/pci/msi.c:msi_setup_entry:565 entry->msi_attrib.multiple = 0x0 DEBUG:drivers/pci/msi.c:__pci_write_msi_msg:315 msgctl = 0x86 DEBUG:drivers/pci/msi.c:__pci_write_msi_msg:318 entry->msi_attrib.multiple = 0x0, msgctl = 0x86 The defined values before the patch are the same with the patch series. Maybe this only happens with a bigger number of interrupts required. I will continue to work on pcitest to perform the same test that you have done. >> >> The EP (USB 3.1 PCIe board) that I usually use only have 2 interrupts. I didn't >> perform the tests like you did. I typically plug the USB flash drive and observe >> the number of interrupts and perform some basic file operations on the USB flash >> drive (copy/move/delete/rename). > > Do you have PCIEPORTBUS enabled? That takes 1 interrupt (assuming your host has > the required capabilities). You should be able to reproduce the issue with just > PCIEPORTBUS enabled then. (With your series added msg_data on USB device will > be 0x1 whereas it should be 0x2. I checked now, I have PCIEPORTBUS enabled. > > Before the patch series order_base_2 below helped in taking care of that. > pos0 = bitmap_find_free_region(pp->msi_irq_in_use, MAX_MSI_IRQS, > order_base_2(no_irqs)); >> >> Can you provide me information (source code, how to compile, how to use, that >> kind of info) about the pcitest tool that you mentioned? I would like to use it >> if possible, please. > > Documentation/PCI/endpoint/ should have all the info.. Ok. After this patch series, maybe we could improve the tool and add the MSI-X support also. :) > > Thanks > Kishon >