diff mbox

Re-activate task to add MSI-X to pcie-designware

Message ID 513e28db-924d-569c-b3ae-cbb949229cc9@ti.com (mailing list archive)
State New, archived
Delegated to: Bjorn Helgaas
Headers show

Commit Message

Kishon Vijay Abraham I Dec. 26, 2017, 12:57 p.m. UTC
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.
> 
> So I tried disabling the pcieportbus driver. These are my observations once I
> disable pcieportbus driver
> 
> root@k2g-evm:~# cat /proc/interrupts
>            CPU0
>  17:          0     GICv2  29 Level     arch_timer
>  18:       2222     GICv2  30 Level     arch_timer
>  21:          0     GICv2  36 Edge      arm-pmu
>  22:        821     GICv2 196 Edge      ttyS0
>  23:          5     GICv2 120 Edge      2530000.i2c
>  24:          0     GICv2  33 Edge      soc:keystone_irq@26202a0
>  25:        909     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:       2442     GICv2 240 Edge      2728000.edma_ccint
>  46:          0     GICv2 252 Edge      2728000.edma_ccerrint
>  47:       7297     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 524288 Edge      pci-endpoint-test
> 281:          0   PCI-MSI 524289 Edge      pci-endpoint-test
> 282:          0   PCI-MSI 524290 Edge      pci-endpoint-test
> 283:          0   PCI-MSI 524291 Edge      pci-endpoint-test
> 284:          0   PCI-MSI 524292 Edge      pci-endpoint-test
> 285:          0   PCI-MSI 524293 Edge      pci-endpoint-test
> 286:          0   PCI-MSI 524294 Edge      pci-endpoint-test
> 287:          0   PCI-MSI 524295 Edge      pci-endpoint-test
> 288:          0   PCI-MSI 524296 Edge      pci-endpoint-test
> 289:          0   PCI-MSI 524297 Edge      pci-endpoint-test
> 290:          0   PCI-MSI 524298 Edge      pci-endpoint-test
> 291:          0   PCI-MSI 524299 Edge      pci-endpoint-test
> 292:          0   PCI-MSI 524300 Edge      pci-endpoint-test
> 293:          0   PCI-MSI 524301 Edge      pci-endpoint-test
> 294:          0   PCI-MSI 524302 Edge      pci-endpoint-test
> 295:          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
> [   57.791535] keystone-pcie 21801000.pcie: ks_pcie_msi_irq_handler, irq 271
> [   57.798643] keystone-pcie 21801000.pcie: irq: bit 0, vector 0, virq 280
> MSI1:           OKAY
> root@k2g-evm:~# pcitest -m 2
> [   59.791509] keystone-pcie 21801000.pcie: ks_pcie_msi_irq_handler, irq 272
> [   59.798614] keystone-pcie 21801000.pcie: irq: bit 0, vector 1, virq 281
> MSI2:           OKAY
> root@k2g-evm:~# pcitest -m 3
> [   61.271509] keystone-pcie 21801000.pcie: ks_pcie_msi_irq_handler, irq 273
> [   61.278616] keystone-pcie 21801000.pcie: irq: bit 0, vector 2, virq 282
> MSI3:           OKAY
> root@k2g-evm:~# pcitest -m 4
> [   62.791514] keystone-pcie 21801000.pcie: ks_pcie_msi_irq_handler, irq 274
> [   62.798619] keystone-pcie 21801000.pcie: irq: bit 0, vector 3, virq 283
> MSI4:           OKAY
> root@k2g-evm:~# pcitest -m 5
> [   64.311513] keystone-pcie 21801000.pcie: ks_pcie_msi_irq_handler, irq 275
> [   64.318620] keystone-pcie 21801000.pcie: irq: bit 0, vector 4, virq 284
> MSI5:           OKAY
> root@k2g-evm:~# pcitest -m 6
> [   66.351518] keystone-pcie 21801000.pcie: ks_pcie_msi_irq_handler, irq 276
> [   66.358624] keystone-pcie 21801000.pcie: irq: bit 0, vector 5, virq 285
> MSI6:           OKAY
> root@k2g-evm:~# pcitest -m 7
> [   67.811516] keystone-pcie 21801000.pcie: ks_pcie_msi_irq_handler, irq 277
> [   67.818623] keystone-pcie 21801000.pcie: irq: bit 0, vector 6, virq 286
> MSI7:           OKAY
> root@k2g-evm:~# pcitest -m 8
> [   69.291518] keystone-pcie 21801000.pcie: ks_pcie_msi_irq_handler, irq 278
> [   69.298625] keystone-pcie 21801000.pcie: irq: bit 0, vector 7, virq 287
> MSI8:           OKAY
> root@k2g-evm:~# pcitest -m 9
> MSI9:           NOT OKAY
> root@k2g-evm:~# pcitest -m 10
> MSI10:          NOT OKAY
> 
> So once I disable pcieportbus, I could get the first 8 interrupts correctly.
> But after that there are no interrupts at all.

This issues is due to the previous interrupt not acknowledged and below fixes
the issue.


Thanks
Kishon

Comments

Gustavo Pimentel Dec. 27, 2017, 2:25 p.m. UTC | #1
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!

>>
>> So I tried disabling the pcieportbus driver. These are my observations once I
>> disable pcieportbus driver
>>
>> root@k2g-evm:~# cat /proc/interrupts
>>            CPU0
>>  17:          0     GICv2  29 Level     arch_timer
>>  18:       2222     GICv2  30 Level     arch_timer
>>  21:          0     GICv2  36 Edge      arm-pmu
>>  22:        821     GICv2 196 Edge      ttyS0
>>  23:          5     GICv2 120 Edge      2530000.i2c
>>  24:          0     GICv2  33 Edge      soc:keystone_irq@26202a0
>>  25:        909     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:       2442     GICv2 240 Edge      2728000.edma_ccint
>>  46:          0     GICv2 252 Edge      2728000.edma_ccerrint
>>  47:       7297     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 524288 Edge      pci-endpoint-test
>> 281:          0   PCI-MSI 524289 Edge      pci-endpoint-test
>> 282:          0   PCI-MSI 524290 Edge      pci-endpoint-test
>> 283:          0   PCI-MSI 524291 Edge      pci-endpoint-test
>> 284:          0   PCI-MSI 524292 Edge      pci-endpoint-test
>> 285:          0   PCI-MSI 524293 Edge      pci-endpoint-test
>> 286:          0   PCI-MSI 524294 Edge      pci-endpoint-test
>> 287:          0   PCI-MSI 524295 Edge      pci-endpoint-test
>> 288:          0   PCI-MSI 524296 Edge      pci-endpoint-test
>> 289:          0   PCI-MSI 524297 Edge      pci-endpoint-test
>> 290:          0   PCI-MSI 524298 Edge      pci-endpoint-test
>> 291:          0   PCI-MSI 524299 Edge      pci-endpoint-test
>> 292:          0   PCI-MSI 524300 Edge      pci-endpoint-test
>> 293:          0   PCI-MSI 524301 Edge      pci-endpoint-test
>> 294:          0   PCI-MSI 524302 Edge      pci-endpoint-test
>> 295:          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
>> [   57.791535] keystone-pcie 21801000.pcie: ks_pcie_msi_irq_handler, irq 271
>> [   57.798643] keystone-pcie 21801000.pcie: irq: bit 0, vector 0, virq 280
>> MSI1:           OKAY
>> root@k2g-evm:~# pcitest -m 2
>> [   59.791509] keystone-pcie 21801000.pcie: ks_pcie_msi_irq_handler, irq 272
>> [   59.798614] keystone-pcie 21801000.pcie: irq: bit 0, vector 1, virq 281
>> MSI2:           OKAY
>> root@k2g-evm:~# pcitest -m 3
>> [   61.271509] keystone-pcie 21801000.pcie: ks_pcie_msi_irq_handler, irq 273
>> [   61.278616] keystone-pcie 21801000.pcie: irq: bit 0, vector 2, virq 282
>> MSI3:           OKAY
>> root@k2g-evm:~# pcitest -m 4
>> [   62.791514] keystone-pcie 21801000.pcie: ks_pcie_msi_irq_handler, irq 274
>> [   62.798619] keystone-pcie 21801000.pcie: irq: bit 0, vector 3, virq 283
>> MSI4:           OKAY
>> root@k2g-evm:~# pcitest -m 5
>> [   64.311513] keystone-pcie 21801000.pcie: ks_pcie_msi_irq_handler, irq 275
>> [   64.318620] keystone-pcie 21801000.pcie: irq: bit 0, vector 4, virq 284
>> MSI5:           OKAY
>> root@k2g-evm:~# pcitest -m 6
>> [   66.351518] keystone-pcie 21801000.pcie: ks_pcie_msi_irq_handler, irq 276
>> [   66.358624] keystone-pcie 21801000.pcie: irq: bit 0, vector 5, virq 285
>> MSI6:           OKAY
>> root@k2g-evm:~# pcitest -m 7
>> [   67.811516] keystone-pcie 21801000.pcie: ks_pcie_msi_irq_handler, irq 277
>> [   67.818623] keystone-pcie 21801000.pcie: irq: bit 0, vector 6, virq 286
>> MSI7:           OKAY
>> root@k2g-evm:~# pcitest -m 8
>> [   69.291518] keystone-pcie 21801000.pcie: ks_pcie_msi_irq_handler, irq 278
>> [   69.298625] keystone-pcie 21801000.pcie: irq: bit 0, vector 7, virq 287
>> MSI8:           OKAY
>> root@k2g-evm:~# pcitest -m 9
>> MSI9:           NOT OKAY
>> root@k2g-evm:~# pcitest -m 10
>> MSI10:          NOT OKAY
>>
>> So once I disable pcieportbus, I could get the first 8 interrupts correctly.
>> But after that there are no interrupts at all.
> 
> This issues is due to the previous interrupt not acknowledged and below fixes
> the issue.

Besides this issue that you fixed with this patch, is everything working well in
your setup?

> 
> diff --git a/drivers/pci/dwc/pcie-designware-host.c
> b/drivers/pci/dwc/pcie-designware-host.c
> index 2816bd98f66c..b25e788e9828 100644
> --- a/drivers/pci/dwc/pcie-designware-host.c
> +++ b/drivers/pci/dwc/pcie-designware-host.c
> @@ -58,8 +58,20 @@ static void dw_msi_unmask_irq(struct irq_data *d)
>         irq_chip_unmask_parent(d);
>  }
> 
> +static void dw_pci_ack_irq(struct irq_data *d)
> +{
> +       struct msi_desc *msi = irq_data_get_msi_desc(d);
> +       struct pcie_port *pp;
> +
> +       pp = (struct pcie_port *) msi_desc_to_pci_sysdata(msi);
> +
> +       if (pp->ops->msi_irq_ack)
> +               pp->ops->msi_irq_ack(d->irq, pp);
> +}
> +
>  static struct irq_chip dw_pcie_msi_irq_chip = {
>         .name = "PCI-MSI",
> +       .irq_ack = dw_pci_ack_irq,
>         .irq_mask = dw_msi_mask_irq,
>         .irq_unmask = dw_msi_unmask_irq,
>  };
> @@ -193,20 +205,8 @@ static void dw_pci_bottom_unmask(struct irq_data *data)
>         spin_unlock_irqrestore(&pp->lock, flags);
>  }
> 
> -static void dw_pci_bottom_ack(struct irq_data *d)
> -{
> -       struct msi_desc *msi = irq_data_get_msi_desc(d);
> -       struct pcie_port *pp;
> -
> -       pp = (struct pcie_port *) msi_desc_to_pci_sysdata(msi);
> -
> -       if (pp->ops->msi_irq_ack)
> -               pp->ops->msi_irq_ack(d->irq, pp);
> -}
> -
>  static struct irq_chip dw_pci_msi_bottom_irq_chip = {
>         .name = "DWPCI-MSI",
> -       .irq_ack                = dw_pci_bottom_ack,
>         .irq_compose_msi_msg = dw_pci_setup_msi_msg,
>         .irq_set_affinity = dw_pci_msi_set_affinity,
>         .irq_mask = dw_pci_bottom_mask,
> @@ -240,7 +240,7 @@ static int dw_pcie_irq_domain_alloc(struct irq_domain *domain,
>         for (i = 0; i < nr_irqs; i++)
>                 irq_domain_set_info(domain, virq + i, bit + i,
>                                     &dw_pci_msi_bottom_irq_chip,
> -                                   domain->host_data, handle_simple_irq,
> +                                   domain->host_data, handle_level_irq,
>                                     NULL, NULL);
> 
>         return 0;
> 
> Thanks
> Kishon
>
Kishon Vijay Abraham I Dec. 28, 2017, 2:58 p.m. UTC | #2
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.

Thanks
Kishon
Gustavo Pimentel Dec. 29, 2017, 9:48 a.m. UTC | #3
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.

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).

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.

Regards,
Gustavo

> 
> Thanks
> Kishon
>
Kishon Vijay Abraham I Dec. 29, 2017, 10:05 a.m. UTC | #4
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.
> 
> 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.

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..

Thanks
Kishon
Gustavo Pimentel Jan. 2, 2018, 7:21 p.m. UTC | #5
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.

Ok, I will try to replicate your test environment to fix this error, meanwhile I
will send a patch v4 to everyone in order to keep this alive and receive more
comments from everyone.

>>>
>>> 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.
>>
>> 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 have to check with HW team maybe. I think I don't have access to that info
through lspci.

> 
> 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));

I didn't understand this. Sorry. Can you explain me your idea?

>>
>> 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..

I have enable this settings in the kernel:
PCI_EPF_TEST
PCI_ENDPOINT_TEST
PCI_ENDPOINT_CONFIGFS

and also compile the pcitest.c file and generate a uImage for my ARC setup.

# ls /sys/class/pci_epc/
#

# ls /sys/kernel/config/
#

# ls /sys/bus/pci-epf/drivers
pci_epf_test


Probably it's missing a very basic thing, but i don't see what. Do you have any
idea?

> 
> Thanks
> Kishon
>
Kishon Vijay Abraham I Jan. 4, 2018, 11:12 a.m. UTC | #6
Hi,

On Wednesday 03 January 2018 12:51 AM, Gustavo Pimentel wrote:
> 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.
> 
> Ok, I will try to replicate your test environment to fix this error, meanwhile I
> will send a patch v4 to everyone in order to keep this alive and receive more
> comments from everyone.
> 
>>>>
>>>> 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.
>>>
>>> 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 have to check with HW team maybe. I think I don't have access to that info
> through lspci.
> 
>>
>> 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));
> 
> I didn't understand this. Sorry. Can you explain me your idea?

Never mind.
What I'm suggesting is msi_data (programmed in PCI_MSI_DATA) cannot be linear
across devices.

For example say you have 2 devices connected to the host, one requesting '2'
MSI interrupts and the other requesting '8' (MMC has a value of 3) msi interrupts.

The msi_data programmed in these devices right now (i.e after applying your
patches) are 0x0 and 0x2 which is not correct.

[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].

This means a device requesting 8 MSI interrupts can modify the lower 3 bits to
raise an interrupt. So if we program 0x2 in msi_data (starting msi vector
data), the device might not write a 0x2 to raise an interrupt. So msi_data can
be either 0x0 and if 0x0 is not available (since it's been allocated to some
other device) then msi_data should be 0x8.

Before your patch series, this issue was not present.
> 
>>>
>>> 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..
> 
> I have enable this settings in the kernel:
> PCI_EPF_TEST
> PCI_ENDPOINT_TEST
> PCI_ENDPOINT_CONFIGFS
> 
> and also compile the pcitest.c file and generate a uImage for my ARC setup.
> 
> # ls /sys/class/pci_epc/
> #

you need to add endpoint controller support for your platform. You can either
refer pci-dra7xx.c or Niklas series adding EP mode support for ARTPEC-6 [1]

[1] -> https://lkml.org/lkml/2017/11/3/321

Thanks
Kishon
Gustavo Pimentel Jan. 9, 2018, 10:32 a.m. UTC | #7
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
>
Kishon Vijay Abraham I Jan. 17, 2018, 1:05 p.m. UTC | #8
On Tuesday 09 January 2018 04:02 PM, Gustavo Pimentel wrote:
> 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?

correct.
> 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

What is the msg data?
> 
> 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 issue happens when 2 devices requesting for MSI.
> 
>>>
>>> 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. :)

yeah.. that would be great!

Thanks
Kishon
diff mbox

Patch

diff --git a/drivers/pci/dwc/pcie-designware-host.c
b/drivers/pci/dwc/pcie-designware-host.c
index 2816bd98f66c..b25e788e9828 100644
--- a/drivers/pci/dwc/pcie-designware-host.c
+++ b/drivers/pci/dwc/pcie-designware-host.c
@@ -58,8 +58,20 @@  static void dw_msi_unmask_irq(struct irq_data *d)
        irq_chip_unmask_parent(d);
 }

+static void dw_pci_ack_irq(struct irq_data *d)
+{
+       struct msi_desc *msi = irq_data_get_msi_desc(d);
+       struct pcie_port *pp;
+
+       pp = (struct pcie_port *) msi_desc_to_pci_sysdata(msi);
+
+       if (pp->ops->msi_irq_ack)
+               pp->ops->msi_irq_ack(d->irq, pp);
+}
+
 static struct irq_chip dw_pcie_msi_irq_chip = {
        .name = "PCI-MSI",
+       .irq_ack = dw_pci_ack_irq,
        .irq_mask = dw_msi_mask_irq,
        .irq_unmask = dw_msi_unmask_irq,
 };
@@ -193,20 +205,8 @@  static void dw_pci_bottom_unmask(struct irq_data *data)
        spin_unlock_irqrestore(&pp->lock, flags);
 }

-static void dw_pci_bottom_ack(struct irq_data *d)
-{
-       struct msi_desc *msi = irq_data_get_msi_desc(d);
-       struct pcie_port *pp;
-
-       pp = (struct pcie_port *) msi_desc_to_pci_sysdata(msi);
-
-       if (pp->ops->msi_irq_ack)
-               pp->ops->msi_irq_ack(d->irq, pp);
-}
-
 static struct irq_chip dw_pci_msi_bottom_irq_chip = {
        .name = "DWPCI-MSI",
-       .irq_ack                = dw_pci_bottom_ack,
        .irq_compose_msi_msg = dw_pci_setup_msi_msg,
        .irq_set_affinity = dw_pci_msi_set_affinity,
        .irq_mask = dw_pci_bottom_mask,
@@ -240,7 +240,7 @@  static int dw_pcie_irq_domain_alloc(struct irq_domain *domain,
        for (i = 0; i < nr_irqs; i++)
                irq_domain_set_info(domain, virq + i, bit + i,
                                    &dw_pci_msi_bottom_irq_chip,
-                                   domain->host_data, handle_simple_irq,
+                                   domain->host_data, handle_level_irq,
                                    NULL, NULL);

        return 0;