mbox series

[v3,0/4] Migrate PCI Endpoint Subsystem tests to Kselftest

Message ID 20241211080105.11104-1-manivannan.sadhasivam@linaro.org (mailing list archive)
Headers show
Series Migrate PCI Endpoint Subsystem tests to Kselftest | expand

Message

Manivannan Sadhasivam Dec. 11, 2024, 8:01 a.m. UTC
Hi,

This series carries forward the effort to add Kselftest for PCI Endpoint
Subsystem started by Aman Gupta [1] a while ago. I reworked the initial version
based on another patch that fixes the return values of IOCTLs in
pci_endpoint_test driver and did many cleanups. Since the resulting work
modified the initial version substantially, I took over the authorship.

This series also incorporates the review comment by Shuah Khan [2] to move the
existing tests from 'tools/pci' to 'tools/testing/kselftest/pci_endpoint' before
migrating to Kselftest framework. I made sure that the tests are executable in
each commit and updated documentation accordingly.

NOTE: Patch 1 is strictly not related to this series, but necessary to execute
Kselftests with Qualcomm Endpoint devices. So this can be merged separately.

- Mani

[1] https://lore.kernel.org/linux-pci/20221007053934.5188-1-aman1.gupta@samsung.com
[2] https://lore.kernel.org/linux-pci/b2a5db97-dc59-33ab-71cd-f591e0b1b34d@linuxfoundation.org

Changes in v3:

* Collected tags.
* Added a note about failing testcase 10 and command to skip it in
  documentation.
* Removed Aman Gupta and Padmanabhan Rajanbabu from CC as their addresses are
  bouncing.

Changes in v2:

* Added a patch that fixes return values of IOCTL in pci_endpoint_test driver
* Moved the existing tests to new location before migrating
* Added a fix for BARs on Qcom devices
* Updated documentation and also added fixture variants for memcpy & DMA modes

Manivannan Sadhasivam (4):
  PCI: qcom-ep: Mark BAR0/BAR2 as 64bit BARs and BAR1/BAR3 as RESERVED
  misc: pci_endpoint_test: Fix the return value of IOCTL
  selftests: Move PCI Endpoint tests from tools/pci to Kselftests
  selftests: pci_endpoint: Migrate to Kselftest framework

 Documentation/PCI/endpoint/pci-test-howto.rst | 152 ++++-------
 MAINTAINERS                                   |   2 +-
 drivers/misc/pci_endpoint_test.c              | 236 ++++++++---------
 drivers/pci/controller/dwc/pcie-qcom-ep.c     |   4 +
 tools/pci/Build                               |   1 -
 tools/pci/Makefile                            |  58 ----
 tools/pci/pcitest.c                           | 250 ------------------
 tools/pci/pcitest.sh                          |  72 -----
 tools/testing/selftests/Makefile              |   1 +
 .../testing/selftests/pci_endpoint/.gitignore |   2 +
 tools/testing/selftests/pci_endpoint/Makefile |   7 +
 tools/testing/selftests/pci_endpoint/config   |   4 +
 .../pci_endpoint/pci_endpoint_test.c          | 186 +++++++++++++
 13 files changed, 373 insertions(+), 602 deletions(-)
 delete mode 100644 tools/pci/Build
 delete mode 100644 tools/pci/Makefile
 delete mode 100644 tools/pci/pcitest.c
 delete mode 100644 tools/pci/pcitest.sh
 create mode 100644 tools/testing/selftests/pci_endpoint/.gitignore
 create mode 100644 tools/testing/selftests/pci_endpoint/Makefile
 create mode 100644 tools/testing/selftests/pci_endpoint/config
 create mode 100644 tools/testing/selftests/pci_endpoint/pci_endpoint_test.c

Comments

Niklas Cassel Dec. 12, 2024, 9:25 a.m. UTC | #1
Hello Mani,

On Wed, Dec 11, 2024 at 01:31:01PM +0530, Manivannan Sadhasivam wrote:
> Hi,
> 
> This series carries forward the effort to add Kselftest for PCI Endpoint
> Subsystem started by Aman Gupta [1] a while ago. I reworked the initial version
> based on another patch that fixes the return values of IOCTLs in
> pci_endpoint_test driver and did many cleanups. Since the resulting work
> modified the initial version substantially, I took over the authorship.
> 
> This series also incorporates the review comment by Shuah Khan [2] to move the
> existing tests from 'tools/pci' to 'tools/testing/kselftest/pci_endpoint' before
> migrating to Kselftest framework. I made sure that the tests are executable in
> each commit and updated documentation accordingly.
> 
> NOTE: Patch 1 is strictly not related to this series, but necessary to execute
> Kselftests with Qualcomm Endpoint devices. So this can be merged separately.

Having to write a big NOTE is usually a hint that you should just have done
things differently :)

If you need to respin this series, I strongly suggest that you send the
Qcom fix separately. It is totally independent, and should be merged ASAP.

As you know, this series conflicts with:
https://lore.kernel.org/linux-pci/20241116032045.2574168-2-cassel@kernel.org/

I don't see any reason why the above patch has not been merged yet,
but it would be really nice if the above could be picked up first,
so this series could also add a kselftest testcase for the above.


Kind regards,
Niklas
Manivannan Sadhasivam Dec. 16, 2024, 6:03 a.m. UTC | #2
On Thu, Dec 12, 2024 at 10:25:53AM +0100, Niklas Cassel wrote:
> Hello Mani,
> 
> On Wed, Dec 11, 2024 at 01:31:01PM +0530, Manivannan Sadhasivam wrote:
> > Hi,
> > 
> > This series carries forward the effort to add Kselftest for PCI Endpoint
> > Subsystem started by Aman Gupta [1] a while ago. I reworked the initial version
> > based on another patch that fixes the return values of IOCTLs in
> > pci_endpoint_test driver and did many cleanups. Since the resulting work
> > modified the initial version substantially, I took over the authorship.
> > 
> > This series also incorporates the review comment by Shuah Khan [2] to move the
> > existing tests from 'tools/pci' to 'tools/testing/kselftest/pci_endpoint' before
> > migrating to Kselftest framework. I made sure that the tests are executable in
> > each commit and updated documentation accordingly.
> > 
> > NOTE: Patch 1 is strictly not related to this series, but necessary to execute
> > Kselftests with Qualcomm Endpoint devices. So this can be merged separately.
> 
> Having to write a big NOTE is usually a hint that you should just have done
> things differently :)
> 
> If you need to respin this series, I strongly suggest that you send the
> Qcom fix separately. It is totally independent, and should be merged ASAP.
> 

Even though it is an independent fix, it is needed to get Kselftests (also the
legacy ones) passing without failures. That's why I kept it as patch 1.
Otherwise, someone may test it and report failures.

> As you know, this series conflicts with:
> https://lore.kernel.org/linux-pci/20241116032045.2574168-2-cassel@kernel.org/
> 
> I don't see any reason why the above patch has not been merged yet,
> but it would be really nice if the above could be picked up first,
> so this series could also add a kselftest testcase for the above.
> 

I was hoping that Greg would pick misc driver changes, but looking at the git
log of this driver I got to know that the changes were picked by PCI folks only.

@kw: Could you please pick the patch from Niklas?

- Mani
Niklas Cassel Dec. 18, 2024, 9:19 a.m. UTC | #3
On Mon, Dec 16, 2024 at 11:33:37AM +0530, Manivannan Sadhasivam wrote:
> On Thu, Dec 12, 2024 at 10:25:53AM +0100, Niklas Cassel wrote:
> > 
> > If you need to respin this series, I strongly suggest that you send the
> > Qcom fix separately. It is totally independent, and should be merged ASAP.
> > 
> 
> Even though it is an independent fix, it is needed to get Kselftests (also the
> legacy ones) passing without failures. That's why I kept it as patch 1.
> Otherwise, someone may test it and report failures.

If qcom needs this fix, then surely pcitest.sh is already failing for the
BAR test for BAR 1 and BAR 3, for all qcom SoCs, and must have been doing
so since the introduction of the introduction of the qcom-pcie-ep driver.

That should be fixed of course, but I do not see why converting the tests
to kselftests is related in any way.

It seems cleaner if this series just converts the tests cases to
kselftests, and nothing else. EPC drivers that was passing before should
still pass after this conversion.

I understand that the qcom fix is important though, which is even bigger
reason that it should be sent separately, so that it can go in ASAP, and
not be blocked on this series landing. Because, AFAICT, you need that qcom
fix to make the pcitest.sh test cases (even without converting to
kselftests).


Kind regards,
Niklas
Krzysztof Wilczyński Dec. 19, 2024, 12:01 a.m. UTC | #4
Hello,

> This series carries forward the effort to add Kselftest for PCI Endpoint
> Subsystem started by Aman Gupta [1] a while ago. I reworked the initial version
> based on another patch that fixes the return values of IOCTLs in
> pci_endpoint_test driver and did many cleanups. Since the resulting work
> modified the initial version substantially, I took over the authorship.
> 
> This series also incorporates the review comment by Shuah Khan [2] to move the
> existing tests from 'tools/pci' to 'tools/testing/kselftest/pci_endpoint' before
> migrating to Kselftest framework. I made sure that the tests are executable in
> each commit and updated documentation accordingly.
> 
> NOTE: Patch 1 is strictly not related to this series, but necessary to execute
> Kselftests with Qualcomm Endpoint devices. So this can be merged separately.

Applied to selftests, thank you!

[01/04] PCI: qcom-ep: Mark BAR0/BAR2 as 64bit BARs and BAR1/BAR3 as RESERVED
        https://git.kernel.org/pci/pci/c/71ae1c3a342c

[02/04] misc: pci_endpoint_test: Fix the return value of IOCTL
        https://git.kernel.org/pci/pci/c/7908208a2f6a

[03/04] selftests: Move PCI Endpoint tests from tools/pci to Kselftests
        https://git.kernel.org/pci/pci/c/5c892b60e4c6

[04/04] selftests: pci_endpoint: Migrate to Kselftest framework
        https://git.kernel.org/pci/pci/c/62f966e676b5

	Krzysztof
Niklas Cassel Dec. 19, 2024, 2:31 p.m. UTC | #5
On Thu, Dec 19, 2024 at 09:01:12AM +0900, Krzysztof Wilczyński wrote:
> Hello,
> 
> > This series carries forward the effort to add Kselftest for PCI Endpoint
> > Subsystem started by Aman Gupta [1] a while ago. I reworked the initial version
> > based on another patch that fixes the return values of IOCTLs in
> > pci_endpoint_test driver and did many cleanups. Since the resulting work
> > modified the initial version substantially, I took over the authorship.
> > 
> > This series also incorporates the review comment by Shuah Khan [2] to move the
> > existing tests from 'tools/pci' to 'tools/testing/kselftest/pci_endpoint' before
> > migrating to Kselftest framework. I made sure that the tests are executable in
> > each commit and updated documentation accordingly.
> > 
> > NOTE: Patch 1 is strictly not related to this series, but necessary to execute
> > Kselftests with Qualcomm Endpoint devices. So this can be merged separately.
> 
> Applied to selftests, thank you!
> 
> [01/04] PCI: qcom-ep: Mark BAR0/BAR2 as 64bit BARs and BAR1/BAR3 as RESERVED
>         https://git.kernel.org/pci/pci/c/71ae1c3a342c
> 
> [02/04] misc: pci_endpoint_test: Fix the return value of IOCTL
>         https://git.kernel.org/pci/pci/c/7908208a2f6a
> 
> [03/04] selftests: Move PCI Endpoint tests from tools/pci to Kselftests
>         https://git.kernel.org/pci/pci/c/5c892b60e4c6
> 
> [04/04] selftests: pci_endpoint: Migrate to Kselftest framework
>         https://git.kernel.org/pci/pci/c/62f966e676b5
> 
> 	Krzysztof

I'm a bit surprised that this series was picked up,
since as you could see earlier in this same thread:
https://lore.kernel.org/linux-pci/20241219000112.GE1444967@rocinante/T/#m7bb0e624a4bf88f5cc13dc3804972c4fa9a79bcd

Mani suggested that my patch (which conflicts with this),
should be picked up first.

Is there a reason for the sudden chance of plans?

Please advice on how to proceed.


Kind regards,
Niklas
Krzysztof Wilczyński Dec. 19, 2024, 3:55 p.m. UTC | #6
Hello,

[...]
> > Applied to selftests, thank you!
> > 
> > [01/04] PCI: qcom-ep: Mark BAR0/BAR2 as 64bit BARs and BAR1/BAR3 as RESERVED
> >         https://git.kernel.org/pci/pci/c/71ae1c3a342c
> > 
> > [02/04] misc: pci_endpoint_test: Fix the return value of IOCTL
> >         https://git.kernel.org/pci/pci/c/7908208a2f6a
> > 
> > [03/04] selftests: Move PCI Endpoint tests from tools/pci to Kselftests
> >         https://git.kernel.org/pci/pci/c/5c892b60e4c6
> > 
> > [04/04] selftests: pci_endpoint: Migrate to Kselftest framework
> >         https://git.kernel.org/pci/pci/c/62f966e676b5
> > 
> > 	Krzysztof
> 
> I'm a bit surprised that this series was picked up,
> since as you could see earlier in this same thread:
> https://lore.kernel.org/linux-pci/20241219000112.GE1444967@rocinante/T/#m7bb0e624a4bf88f5cc13dc3804972c4fa9a79bcd
> 
> Mani suggested that my patch (which conflicts with this),
> should be picked up first.
> 
> Is there a reason for the sudden chance of plans?

No, no change to the plan here.

There were some mixed signals between the mailing list, IRC and
the Patchwork queue.  But I will proceed as planned there.

> Please advice on how to proceed.

I will pick your patch and drop Mani's series.  Mani told me on IRC that he
plans to work on it a bit more.

	Krzysztof
Niklas Cassel Dec. 19, 2024, 8:17 p.m. UTC | #7
On Fri, Dec 20, 2024 at 12:55:04AM +0900, Krzysztof Wilczyński wrote:

[...]

> > Mani suggested that my patch (which conflicts with this),
> > should be picked up first.
> > 
> > Is there a reason for the sudden chance of plans?
> 
> No, no change to the plan here.
> 
> There were some mixed signals between the mailing list, IRC and
> the Patchwork queue.  But I will proceed as planned there.
> 
> > Please advice on how to proceed.
> 
> I will pick your patch and drop Mani's series.  Mani told me on IRC that he
> plans to work on it a bit more.

Ok, thanks!

I really hope that the kselftest conversion will have time to land this
cycle as well.

Feel free to CC me on the next revision and I will make sure to send a
Tested-by and/or Reviewed-by.


Kind regards,
Niklas