RE: [PATCH] selftests: pci: pci-selftest: add support for PCI endpoint driver test

["Aman Gupta/FDS SW /SSIR/Engineer/Samsung Electronics" <aman1.gupta@samsung.com>]

> -----Original Message-----
> From: 'Manivannan Sadhasivam'
> [mailto:manivannan.sadhasivam@linaro.org]
> Sent: 21 December 2022 12:54
> To: Aman Gupta/FDS SW /SSIR/Engineer/Samsung Electronics
> <aman1.gupta@samsung.com>
> Cc: 'Manivannan Sadhasivam' <mani@kernel.org>; shradha.t@samsung.com;
> pankaj.dubey@samsung.com; kishon@ti.com; lpieralisi@kernel.org;
> kw@linux.com; shuah@kernel.org; linux-pci@vger.kernel.org; linux-
> kselftest@vger.kernel.org; 'Padmanabhan Rajanbabu'
> <p.rajanbabu@samsung.com>
> Subject: Re: [PATCH] selftests: pci: pci-selftest: add support for PCI endpoint
> driver test
> 
> On Mon, Dec 19, 2022 at 10:00:40AM +0530, Aman Gupta/FDS SW
> /SSIR/Engineer/Samsung Electronics wrote:
> >
> >
> > > -----Original Message-----
> > > From: Manivannan Sadhasivam
> > > [mailto:manivannan.sadhasivam@linaro.org]
> > > Sent: 01 November 2022 22:50
> > > To: Manivannan Sadhasivam <mani@kernel.org>
> > > Cc: Aman Gupta <aman1.gupta@samsung.com>;
> shradha.t@samsung.com;
> > > pankaj.dubey@samsung.com; kishon@ti.com; lpieralisi@kernel.org;
> > > kw@linux.com; shuah@kernel.org; linux-pci@vger.kernel.org; linux-
> > > kselftest@vger.kernel.org; Padmanabhan Rajanbabu
> > > <p.rajanbabu@samsung.com>
> > > Subject: Re: [PATCH] selftests: pci: pci-selftest: add support for
> > > PCI endpoint driver test
> > >
> > > On Tue, Nov 01, 2022 at 07:32:16PM +0530, Manivannan Sadhasivam
> wrote:
> > > > On Fri, Oct 07, 2022 at 11:09:34AM +0530, Aman Gupta wrote:
> > > > > This patch enables the support to perform selftest on PCIe
> > > > > endpoint driver present in the system. The following tests are
> > > > > currently performed by the selftest utility
> > > > >
> > > > > 1. BAR Tests (BAR0 to BAR5)
> > > > > 2. MSI Interrupt Tests (MSI1 to MSI32) 3. Read Tests (For 1,
> > > > > 1024, 1025, 1024000, 1024001 Bytes) 4. Write Tests (For 1, 1024,
> > > > > 1025, 1024000, 1024001 Bytes) 5. Copy Tests (For 1, 1024, 1025,
> > > > > 1024000,
> > > > > 1024001 Bytes)
> > > > >
> > > > > Signed-off-by: Aman Gupta <aman1.gupta@samsung.com>
> > > > > Signed-off-by: Padmanabhan Rajanbabu
> <p.rajanbabu@samsung.com>
> > > > > ---
> > > > >  tools/testing/selftests/Makefile           |   1 +
> > > > >  tools/testing/selftests/pci/.gitignore     |   1 +
> > > > >  tools/testing/selftests/pci/Makefile       |   7 +
> > > > >  tools/testing/selftests/pci/pci-selftest.c | 167
> > > > > +++++++++++++++++++++
> > > > >  4 files changed, 176 insertions(+)  create mode 100644
> > > > > tools/testing/selftests/pci/.gitignore
> > > > >  create mode 100644 tools/testing/selftests/pci/Makefile
> > > > >  create mode 100644 tools/testing/selftests/pci/pci-selftest.c
> > > > >
> > > > > diff --git a/tools/testing/selftests/Makefile
> > > > > b/tools/testing/selftests/Makefile
> > > > > index c2064a35688b..81584169a80f 100644
> > > > > --- a/tools/testing/selftests/Makefile
> > > > > +++ b/tools/testing/selftests/Makefile
> > > > > @@ -49,6 +49,7 @@ TARGETS += net/forwarding  TARGETS +=
> > > > > net/mptcp TARGETS += netfilter  TARGETS += nsfs
> > > > > +TARGETS += pci
> > > > >  TARGETS += pidfd
> > > > >  TARGETS += pid_namespace
> > > > >  TARGETS += powerpc
> > > > > diff --git a/tools/testing/selftests/pci/.gitignore
> > > > > b/tools/testing/selftests/pci/.gitignore
> > > > > new file mode 100644
> > > > > index 000000000000..db01411b8200
> > > > > --- /dev/null
> > > > > +++ b/tools/testing/selftests/pci/.gitignore
> > > > > @@ -0,0 +1 @@
> > > > > +pci-selftest
> > > > > diff --git a/tools/testing/selftests/pci/Makefile
> > > > > b/tools/testing/selftests/pci/Makefile
> > > > > new file mode 100644
> > > > > index 000000000000..76b7725a45ae
> > > > > --- /dev/null
> > > > > +++ b/tools/testing/selftests/pci/Makefile
> > > > > @@ -0,0 +1,7 @@
> > > > > +# SPDX-License-Identifier: GPL-2.0 CFLAGS += -O2
> > > > > +-Wl,-no-as-needed -Wall LDFLAGS += -lrt -lpthread -lm
> > > > > +
> > > > > +TEST_GEN_PROGS = pci-selftest
> > > > > +
> > > > > +include ../lib.mk
> > > > > diff --git a/tools/testing/selftests/pci/pci-selftest.c
> > > > > b/tools/testing/selftests/pci/pci-selftest.c
> > > > > new file mode 100644
> > > > > index 000000000000..73e8f3eb1982
> > > > > --- /dev/null
> > > > > +++ b/tools/testing/selftests/pci/pci-selftest.c
> > > >
> > > > endpoint-test.c
> > Okay I will change the file name in the next patch.
> > > >
> > > > > @@ -0,0 +1,167 @@
> > > > > +// SPDX-License-Identifier: GPL-2.0
> > > > > +/*
> > > > > + * PCI Endpoint Driver Test Program
> > > > > + *
> > > > > + * Copyright (c) 2022 Samsung Electronics Co., Ltd.
> > > > > + *             https://www.samsung.com
> > > > > + * Author: Aman Gupta <aman1.gupta@samsung.com>  */
> > > > > +
> > > > > +#include <errno.h>
> > > > > +#include <fcntl.h>
> > > > > +#include <stdbool.h>
> > > > > +#include <stdio.h>
> > > > > +#include <stdlib.h>
> > > > > +#include <sys/ioctl.h>
> > > > > +#include <unistd.h>
> > > > > +
> > > > > +#include "../kselftest_harness.h"
> > > > > +
> > > > > +#define PCITEST_BAR		_IO('P', 0x1)
> > > > > +#define PCITEST_LEGACY_IRQ	_IO('P', 0x2)
> > > > > +#define PCITEST_MSI		_IOW('P', 0x3, int)
> > > > > +#define PCITEST_WRITE		_IOW('P', 0x4, unsigned long)
> > > > > +#define PCITEST_READ		_IOW('P', 0x5, unsigned long)
> > > > > +#define PCITEST_COPY		_IOW('P', 0x6, unsigned long)
> > > > > +#define PCITEST_MSIX		_IOW('P', 0x7, int)
> > > > > +#define PCITEST_SET_IRQTYPE	_IOW('P', 0x8, int)
> > > > > +#define PCITEST_GET_IRQTYPE	_IO('P', 0x9)
> > > > > +#define PCITEST_CLEAR_IRQ	_IO('P', 0x10)
> > > > > +
> > > > > +static char *test_device = "/dev/pci-endpoint-test.0";
> > > > > +
> > > > > +struct xfer_param {
> > > > > +	unsigned long size;
> > > > > +	unsigned char flag;
> > > > > +	};
> > > >
> > > > Align '}'
> > Okay.
> > > >
> > > > > +
> > > > > +FIXTURE(device)
> > > > > +{
> > > > > +	int fd;
> > > > > +};
> > > > > +
> > > > > +FIXTURE_SETUP(device)
> > > > > +{
> > > > > +
> > > > > +	self->fd = open(test_device, O_RDWR);
> > > > > +
> > > > > +	ASSERT_NE(-1, self->fd) {
> > > > > +		TH_LOG("Can't open PCI Endpoint Test device\n");
> > > > > +	}
> > > > > +}
> > > > > +
> > > > > +FIXTURE_TEARDOWN(device)
> > > > > +{
> > > > > +	close(self->fd);
> > > > > +}
> > > > > +
> > > > > +TEST_F(device, BAR_TEST)
> > > > > +{
> > > > > +	int ret = -EINVAL;
> > > >
> > > > Ininitialization not required here and also in other functions.
> > Understood , I will make the changes in the next patch.
> > > >
> > > > > +	int final = 0;
> > > > > +
> > > > > +	for (int i = 0; i <= 5; i++) {
> > > > > +		ret = ioctl(self->fd, PCITEST_BAR, i);
> > > > > +
> > > > > +		EXPECT_EQ(1, ret) {
> > > >
> > > > The return value of all these IOCTL's are going to change when [1]
> > > > get's
> > > merged.
> > > >
> > > > [1]
> > > > https://lore.kernel.org/linux-pci/20220824123010.51763-1-manivanna
> > > > n.sa
> > > > dhasivam@linaro.org/
> > > >
> > > > I'd suggest to resubmit this selftest after that.
> >
> >
> > Manivannan, the patch link you have provided cannot be directly
> > applies on the latest kernel and hence it requires some re work .I can
> > rework these patches along with the kselftest patch if not please
> > allow me to go ahead and post the kselftest patch till then.
> 
> I've pushed my patches here:
> https://protect2.fireeye.com/v1/url?k=f8a46815-992f7d3a-f8a5e35a-
> 74fe485cbfe7-1e3e3a1d7f2a8577&q=1&e=e8998bc0-4133-4737-9572-
> 17b47a4d953c&u=https%3A%2F%2Fgit.linaro.org%2Fpeople%2Fmanivannan.
> sadhasivam%2Flinux.git%2Flog%2F%3Fh%3Dpci-endpoint-test-fix
> 
> Please pick them and rebase your patch(es) on top and post as a series. You
> should also make the series a continuation of my v2, preserving the cover
> letter and keeping everyone in CC.
> 

Hi Manivanan,
Thanks for the reply and sharing the patches.
I will post the kselftest patch along with your three patches as version 3, preserving your cover letter.

> After the kselftest patch, the tools/pci directory should be removed and the
> endpoint documentation (Documentation/PCI/endpoint/pci-test-howto.rst)
> should be modified to reflect kselftest.
> 
> Thanks for your contribution!
> 
> - Mani
> 

Please allow me to reply to this comment in different thread, where you and Shuah are already having discussion about this.
Thanks for the review.
Aman Gupta