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

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

[Aman Gupta]

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
@@ -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;
+	};
+
+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;
+	int final = 0;
+
+	for (int i = 0; i <= 5; i++) {
+		ret = ioctl(self->fd, PCITEST_BAR, i);
+
+		EXPECT_EQ(1, ret) {
+			TH_LOG("TEST FAILED FOR BAR %d\n", i);
+			final++;
+		}
+	}
+
+	ASSERT_EQ(0, final);
+}
+
+TEST_F(device, MSI_TEST)
+{
+	int ret = -EINVAL;
+	int final = 0;
+
+	ret = ioctl(self->fd, PCITEST_SET_IRQTYPE, 1);
+	ASSERT_EQ(1, ret);
+
+	for (int i = 1; i <= 32; i++) {
+		ret = ioctl(self->fd, PCITEST_MSI, i);
+		EXPECT_EQ(1, ret) {
+			TH_LOG("TEST FAILED FOR MSI%d\n", i);
+			final++;
+		}
+	}
+
+	ASSERT_EQ(0, final);
+}
+
+TEST_F(device, READ_TEST)
+{
+	int final = 0;
+	int ret = -EINVAL;
+	unsigned long SIZE[5] = {1, 1024, 1025, 1024000, 1024001};
+
+	ret = ioctl(self->fd, PCITEST_SET_IRQTYPE, 1);
+	ASSERT_EQ(1, ret);
+
+	struct xfer_param param;
+
+	param.flag = 0;
+	for (int i = 0; i < 5; i++) {
+		param.size = SIZE[i];
+		ret = ioctl(self->fd, PCITEST_READ, &param);
+		EXPECT_EQ(1, ret) {
+			TH_LOG("TEST FAILED FOR size =%ld.\n", SIZE[i]);
+			final++;
+		}
+	}
+
+	ASSERT_EQ(0, final);
+}
+
+TEST_F(device, WRITE_TEST)
+{
+	int final = 0;
+	int ret = -EINVAL;
+	unsigned long SIZE[5] = {1, 1024, 1025, 1024000, 1024001};
+
+	ret = ioctl(self->fd, PCITEST_SET_IRQTYPE, 1);
+	ASSERT_EQ(1, ret);
+
+	struct xfer_param param;
+
+	param.flag = 0;
+
+	for (int i = 0; i < 5; i++) {
+		param.size = SIZE[i];
+		ret = ioctl(self->fd, PCITEST_WRITE, &param);
+		EXPECT_EQ(1, ret) {
+			TH_LOG("TEST FAILED FOR size =%ld.\n", SIZE[i]);
+			final++;
+		}
+	}
+
+	ASSERT_EQ(0, final);
+}
+
+TEST_F(device, COPY_TEST)
+{
+	int final = 0;
+	int ret = -EINVAL;
+	unsigned long SIZE[5] = {1, 1024, 1025, 1024000, 1024001};
+
+	ret = ioctl(self->fd, PCITEST_SET_IRQTYPE, 1);
+	ASSERT_EQ(1, ret);
+
+	struct xfer_param param;
+
+	param.flag = 0;
+
+	for (int i = 0; i < 5; i++) {
+		param.size = SIZE[i];
+		ret = ioctl(self->fd, PCITEST_COPY, &param);
+		EXPECT_EQ(1, ret) {
+			TH_LOG("TEST FAILED FOR size =%ld.\n", SIZE[i]);
+			final++;
+		}
+	}
+
+	ASSERT_EQ(0, final);
+}
+TEST_HARNESS_MAIN
-- 
2.17.1

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

[Kishon Vijay Abraham I]

+Mani

On 07/10/22 11:09 am, 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
> @@ -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;
> +	};
> +
> +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;
> +	int final = 0;
> +
> +	for (int i = 0; i <= 5; i++) {
> +		ret = ioctl(self->fd, PCITEST_BAR, i);
> +
> +		EXPECT_EQ(1, ret) {
> +			TH_LOG("TEST FAILED FOR BAR %d\n", i);
> +			final++;
> +		}
> +	}
> +
> +	ASSERT_EQ(0, final);
> +}
> +
> +TEST_F(device, MSI_TEST)
> +{
> +	int ret = -EINVAL;
> +	int final = 0;
> +
> +	ret = ioctl(self->fd, PCITEST_SET_IRQTYPE, 1);
> +	ASSERT_EQ(1, ret);
> +
> +	for (int i = 1; i <= 32; i++) {
> +		ret = ioctl(self->fd, PCITEST_MSI, i);
> +		EXPECT_EQ(1, ret) {
> +			TH_LOG("TEST FAILED FOR MSI%d\n", i);
> +			final++;
> +		}
> +	}
> +
> +	ASSERT_EQ(0, final);
> +}
> +
> +TEST_F(device, READ_TEST)
> +{
> +	int final = 0;
> +	int ret = -EINVAL;
> +	unsigned long SIZE[5] = {1, 1024, 1025, 1024000, 1024001};
> +
> +	ret = ioctl(self->fd, PCITEST_SET_IRQTYPE, 1);
> +	ASSERT_EQ(1, ret);
> +
> +	struct xfer_param param;
> +
> +	param.flag = 0;
> +	for (int i = 0; i < 5; i++) {
> +		param.size = SIZE[i];
> +		ret = ioctl(self->fd, PCITEST_READ, &param);
> +		EXPECT_EQ(1, ret) {
> +			TH_LOG("TEST FAILED FOR size =%ld.\n", SIZE[i]);
> +			final++;
> +		}
> +	}
> +
> +	ASSERT_EQ(0, final);
> +}
> +
> +TEST_F(device, WRITE_TEST)
> +{
> +	int final = 0;
> +	int ret = -EINVAL;
> +	unsigned long SIZE[5] = {1, 1024, 1025, 1024000, 1024001};
> +
> +	ret = ioctl(self->fd, PCITEST_SET_IRQTYPE, 1);
> +	ASSERT_EQ(1, ret);
> +
> +	struct xfer_param param;
> +
> +	param.flag = 0;
> +
> +	for (int i = 0; i < 5; i++) {
> +		param.size = SIZE[i];
> +		ret = ioctl(self->fd, PCITEST_WRITE, &param);
> +		EXPECT_EQ(1, ret) {
> +			TH_LOG("TEST FAILED FOR size =%ld.\n", SIZE[i]);
> +			final++;
> +		}
> +	}
> +
> +	ASSERT_EQ(0, final);
> +}
> +
> +TEST_F(device, COPY_TEST)
> +{
> +	int final = 0;
> +	int ret = -EINVAL;
> +	unsigned long SIZE[5] = {1, 1024, 1025, 1024000, 1024001};
> +
> +	ret = ioctl(self->fd, PCITEST_SET_IRQTYPE, 1);
> +	ASSERT_EQ(1, ret);
> +
> +	struct xfer_param param;
> +
> +	param.flag = 0;
> +
> +	for (int i = 0; i < 5; i++) {
> +		param.size = SIZE[i];
> +		ret = ioctl(self->fd, PCITEST_COPY, &param);
> +		EXPECT_EQ(1, ret) {
> +			TH_LOG("TEST FAILED FOR size =%ld.\n", SIZE[i]);
> +			final++;
> +		}
> +	}
> +
> +	ASSERT_EQ(0, final);
> +}
> +TEST_HARNESS_MAIN
> 

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

[Aman Gupta]

> -----Original Message-----
> From: Kishon Vijay Abraham I [mailto:kishon@ti.com]
> Sent: Tuesday, October 11, 2022 4:29 PM
> To: Aman Gupta <aman1.gupta@samsung.com>; shradha.t@samsung.com;
> pankaj.dubey@samsung.com; lpieralisi@kernel.org; kw@linux.com;
> shuah@kernel.org
> Cc: linux-pci@vger.kernel.org; linux-kselftest@vger.kernel.org;
> Padmanabhan Rajanbabu <p.rajanbabu@samsung.com>; Manivannan
> Sadhasivam <manivannan.sadhasivam@linaro.org>
> Subject: Re: [PATCH] selftests: pci: pci-selftest: add support for PCI endpoint
> driver test
> 
> +Mani
Gentle reminder for review of this patch.

Thanks and Regards
Aman Gupta
> 
> On 07/10/22 11:09 am, 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
> > @@ -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;
> > +	};
> > +
> > +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;
> > +	int final = 0;
> > +
> > +	for (int i = 0; i <= 5; i++) {
> > +		ret = ioctl(self->fd, PCITEST_BAR, i);
> > +
> > +		EXPECT_EQ(1, ret) {
> > +			TH_LOG("TEST FAILED FOR BAR %d\n", i);
> > +			final++;
> > +		}
> > +	}
> > +
> > +	ASSERT_EQ(0, final);
> > +}
> > +
> > +TEST_F(device, MSI_TEST)
> > +{
> > +	int ret = -EINVAL;
> > +	int final = 0;
> > +
> > +	ret = ioctl(self->fd, PCITEST_SET_IRQTYPE, 1);
> > +	ASSERT_EQ(1, ret);
> > +
> > +	for (int i = 1; i <= 32; i++) {
> > +		ret = ioctl(self->fd, PCITEST_MSI, i);
> > +		EXPECT_EQ(1, ret) {
> > +			TH_LOG("TEST FAILED FOR MSI%d\n", i);
> > +			final++;
> > +		}
> > +	}
> > +
> > +	ASSERT_EQ(0, final);
> > +}
> > +
> > +TEST_F(device, READ_TEST)
> > +{
> > +	int final = 0;
> > +	int ret = -EINVAL;
> > +	unsigned long SIZE[5] = {1, 1024, 1025, 1024000, 1024001};
> > +
> > +	ret = ioctl(self->fd, PCITEST_SET_IRQTYPE, 1);
> > +	ASSERT_EQ(1, ret);
> > +
> > +	struct xfer_param param;
> > +
> > +	param.flag = 0;
> > +	for (int i = 0; i < 5; i++) {
> > +		param.size = SIZE[i];
> > +		ret = ioctl(self->fd, PCITEST_READ, &param);
> > +		EXPECT_EQ(1, ret) {
> > +			TH_LOG("TEST FAILED FOR size =%ld.\n", SIZE[i]);
> > +			final++;
> > +		}
> > +	}
> > +
> > +	ASSERT_EQ(0, final);
> > +}
> > +
> > +TEST_F(device, WRITE_TEST)
> > +{
> > +	int final = 0;
> > +	int ret = -EINVAL;
> > +	unsigned long SIZE[5] = {1, 1024, 1025, 1024000, 1024001};
> > +
> > +	ret = ioctl(self->fd, PCITEST_SET_IRQTYPE, 1);
> > +	ASSERT_EQ(1, ret);
> > +
> > +	struct xfer_param param;
> > +
> > +	param.flag = 0;
> > +
> > +	for (int i = 0; i < 5; i++) {
> > +		param.size = SIZE[i];
> > +		ret = ioctl(self->fd, PCITEST_WRITE, &param);
> > +		EXPECT_EQ(1, ret) {
> > +			TH_LOG("TEST FAILED FOR size =%ld.\n", SIZE[i]);
> > +			final++;
> > +		}
> > +	}
> > +
> > +	ASSERT_EQ(0, final);
> > +}
> > +
> > +TEST_F(device, COPY_TEST)
> > +{
> > +	int final = 0;
> > +	int ret = -EINVAL;
> > +	unsigned long SIZE[5] = {1, 1024, 1025, 1024000, 1024001};
> > +
> > +	ret = ioctl(self->fd, PCITEST_SET_IRQTYPE, 1);
> > +	ASSERT_EQ(1, ret);
> > +
> > +	struct xfer_param param;
> > +
> > +	param.flag = 0;
> > +
> > +	for (int i = 0; i < 5; i++) {
> > +		param.size = SIZE[i];
> > +		ret = ioctl(self->fd, PCITEST_COPY, &param);
> > +		EXPECT_EQ(1, ret) {
> > +			TH_LOG("TEST FAILED FOR size =%ld.\n", SIZE[i]);
> > +			final++;
> > +		}
> > +	}
> > +
> > +	ASSERT_EQ(0, final);
> > +}
> > +TEST_HARNESS_MAIN
> >

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

['Manivannan Sadhasivam']

On Fri, Oct 21, 2022 at 12:26:38PM +0530, Aman Gupta wrote:
> 
> 
> > -----Original Message-----
> > From: Kishon Vijay Abraham I [mailto:kishon@ti.com]
> > Sent: Tuesday, October 11, 2022 4:29 PM
> > To: Aman Gupta <aman1.gupta@samsung.com>; shradha.t@samsung.com;
> > pankaj.dubey@samsung.com; lpieralisi@kernel.org; kw@linux.com;
> > shuah@kernel.org
> > Cc: linux-pci@vger.kernel.org; linux-kselftest@vger.kernel.org;
> > Padmanabhan Rajanbabu <p.rajanbabu@samsung.com>; Manivannan
> > Sadhasivam <manivannan.sadhasivam@linaro.org>
> > Subject: Re: [PATCH] selftests: pci: pci-selftest: add support for PCI endpoint
> > driver test
> > 
> > +Mani
> Gentle reminder for review of this patch.
> 

Sorry for the delay. I'm on leave for the past few weeks. Will get to the patch
next week.

Thanks,
Mani

> Thanks and Regards
> Aman Gupta
> > 
> > On 07/10/22 11:09 am, 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
> > > @@ -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;
> > > +	};
> > > +
> > > +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;
> > > +	int final = 0;
> > > +
> > > +	for (int i = 0; i <= 5; i++) {
> > > +		ret = ioctl(self->fd, PCITEST_BAR, i);
> > > +
> > > +		EXPECT_EQ(1, ret) {
> > > +			TH_LOG("TEST FAILED FOR BAR %d\n", i);
> > > +			final++;
> > > +		}
> > > +	}
> > > +
> > > +	ASSERT_EQ(0, final);
> > > +}
> > > +
> > > +TEST_F(device, MSI_TEST)
> > > +{
> > > +	int ret = -EINVAL;
> > > +	int final = 0;
> > > +
> > > +	ret = ioctl(self->fd, PCITEST_SET_IRQTYPE, 1);
> > > +	ASSERT_EQ(1, ret);
> > > +
> > > +	for (int i = 1; i <= 32; i++) {
> > > +		ret = ioctl(self->fd, PCITEST_MSI, i);
> > > +		EXPECT_EQ(1, ret) {
> > > +			TH_LOG("TEST FAILED FOR MSI%d\n", i);
> > > +			final++;
> > > +		}
> > > +	}
> > > +
> > > +	ASSERT_EQ(0, final);
> > > +}
> > > +
> > > +TEST_F(device, READ_TEST)
> > > +{
> > > +	int final = 0;
> > > +	int ret = -EINVAL;
> > > +	unsigned long SIZE[5] = {1, 1024, 1025, 1024000, 1024001};
> > > +
> > > +	ret = ioctl(self->fd, PCITEST_SET_IRQTYPE, 1);
> > > +	ASSERT_EQ(1, ret);
> > > +
> > > +	struct xfer_param param;
> > > +
> > > +	param.flag = 0;
> > > +	for (int i = 0; i < 5; i++) {
> > > +		param.size = SIZE[i];
> > > +		ret = ioctl(self->fd, PCITEST_READ, &param);
> > > +		EXPECT_EQ(1, ret) {
> > > +			TH_LOG("TEST FAILED FOR size =%ld.\n", SIZE[i]);
> > > +			final++;
> > > +		}
> > > +	}
> > > +
> > > +	ASSERT_EQ(0, final);
> > > +}
> > > +
> > > +TEST_F(device, WRITE_TEST)
> > > +{
> > > +	int final = 0;
> > > +	int ret = -EINVAL;
> > > +	unsigned long SIZE[5] = {1, 1024, 1025, 1024000, 1024001};
> > > +
> > > +	ret = ioctl(self->fd, PCITEST_SET_IRQTYPE, 1);
> > > +	ASSERT_EQ(1, ret);
> > > +
> > > +	struct xfer_param param;
> > > +
> > > +	param.flag = 0;
> > > +
> > > +	for (int i = 0; i < 5; i++) {
> > > +		param.size = SIZE[i];
> > > +		ret = ioctl(self->fd, PCITEST_WRITE, &param);
> > > +		EXPECT_EQ(1, ret) {
> > > +			TH_LOG("TEST FAILED FOR size =%ld.\n", SIZE[i]);
> > > +			final++;
> > > +		}
> > > +	}
> > > +
> > > +	ASSERT_EQ(0, final);
> > > +}
> > > +
> > > +TEST_F(device, COPY_TEST)
> > > +{
> > > +	int final = 0;
> > > +	int ret = -EINVAL;
> > > +	unsigned long SIZE[5] = {1, 1024, 1025, 1024000, 1024001};
> > > +
> > > +	ret = ioctl(self->fd, PCITEST_SET_IRQTYPE, 1);
> > > +	ASSERT_EQ(1, ret);
> > > +
> > > +	struct xfer_param param;
> > > +
> > > +	param.flag = 0;
> > > +
> > > +	for (int i = 0; i < 5; i++) {
> > > +		param.size = SIZE[i];
> > > +		ret = ioctl(self->fd, PCITEST_COPY, &param);
> > > +		EXPECT_EQ(1, ret) {
> > > +			TH_LOG("TEST FAILED FOR size =%ld.\n", SIZE[i]);
> > > +			final++;
> > > +		}
> > > +	}
> > > +
> > > +	ASSERT_EQ(0, final);
> > > +}
> > > +TEST_HARNESS_MAIN
> > >
> 

-- 
மணிவண்ணன் சதாசிவம்

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

[Manivannan Sadhasivam]

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

> @@ -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 '}'

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

> +	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-manivannan.sadhasivam@linaro.org/

I'd suggest to resubmit this selftest after that.

Thanks,
Mani

> +			TH_LOG("TEST FAILED FOR BAR %d\n", i);
> +			final++;
> +		}
> +	}
> +
> +	ASSERT_EQ(0, final);
> +}
> +
> +TEST_F(device, MSI_TEST)
> +{
> +	int ret = -EINVAL;
> +	int final = 0;
> +
> +	ret = ioctl(self->fd, PCITEST_SET_IRQTYPE, 1);
> +	ASSERT_EQ(1, ret);
> +
> +	for (int i = 1; i <= 32; i++) {
> +		ret = ioctl(self->fd, PCITEST_MSI, i);
> +		EXPECT_EQ(1, ret) {
> +			TH_LOG("TEST FAILED FOR MSI%d\n", i);
> +			final++;
> +		}
> +	}
> +
> +	ASSERT_EQ(0, final);
> +}
> +
> +TEST_F(device, READ_TEST)
> +{
> +	int final = 0;
> +	int ret = -EINVAL;
> +	unsigned long SIZE[5] = {1, 1024, 1025, 1024000, 1024001};
> +
> +	ret = ioctl(self->fd, PCITEST_SET_IRQTYPE, 1);
> +	ASSERT_EQ(1, ret);
> +
> +	struct xfer_param param;
> +
> +	param.flag = 0;
> +	for (int i = 0; i < 5; i++) {
> +		param.size = SIZE[i];
> +		ret = ioctl(self->fd, PCITEST_READ, &param);
> +		EXPECT_EQ(1, ret) {
> +			TH_LOG("TEST FAILED FOR size =%ld.\n", SIZE[i]);
> +			final++;
> +		}
> +	}
> +
> +	ASSERT_EQ(0, final);
> +}
> +
> +TEST_F(device, WRITE_TEST)
> +{
> +	int final = 0;
> +	int ret = -EINVAL;
> +	unsigned long SIZE[5] = {1, 1024, 1025, 1024000, 1024001};
> +
> +	ret = ioctl(self->fd, PCITEST_SET_IRQTYPE, 1);
> +	ASSERT_EQ(1, ret);
> +
> +	struct xfer_param param;
> +
> +	param.flag = 0;
> +
> +	for (int i = 0; i < 5; i++) {
> +		param.size = SIZE[i];
> +		ret = ioctl(self->fd, PCITEST_WRITE, &param);
> +		EXPECT_EQ(1, ret) {
> +			TH_LOG("TEST FAILED FOR size =%ld.\n", SIZE[i]);
> +			final++;
> +		}
> +	}
> +
> +	ASSERT_EQ(0, final);
> +}
> +
> +TEST_F(device, COPY_TEST)
> +{
> +	int final = 0;
> +	int ret = -EINVAL;
> +	unsigned long SIZE[5] = {1, 1024, 1025, 1024000, 1024001};
> +
> +	ret = ioctl(self->fd, PCITEST_SET_IRQTYPE, 1);
> +	ASSERT_EQ(1, ret);
> +
> +	struct xfer_param param;
> +
> +	param.flag = 0;
> +
> +	for (int i = 0; i < 5; i++) {
> +		param.size = SIZE[i];
> +		ret = ioctl(self->fd, PCITEST_COPY, &param);
> +		EXPECT_EQ(1, ret) {
> +			TH_LOG("TEST FAILED FOR size =%ld.\n", SIZE[i]);
> +			final++;
> +		}
> +	}
> +
> +	ASSERT_EQ(0, final);
> +}
> +TEST_HARNESS_MAIN
> -- 
> 2.17.1
> 

-- 
மணிவண்ணன் சதாசிவம்

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

[Manivannan Sadhasivam]

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
> 
> > @@ -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 '}'
> 
> > +
> > +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.
> 
> > +	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-manivannan.sadhasivam@linaro.org/
> 
> I'd suggest to resubmit this selftest after that.
> 

Looks like we might end up removing the tests under tools/pci and just use this
one. I will CC you on the v3 of PCI test cleanup series. Please rebase this
patch on top of that and post after incorporating the review comments.

Thanks,
Mani

> Thanks,
> Mani
> 
> > +			TH_LOG("TEST FAILED FOR BAR %d\n", i);
> > +			final++;
> > +		}
> > +	}
> > +
> > +	ASSERT_EQ(0, final);
> > +}
> > +
> > +TEST_F(device, MSI_TEST)
> > +{
> > +	int ret = -EINVAL;
> > +	int final = 0;
> > +
> > +	ret = ioctl(self->fd, PCITEST_SET_IRQTYPE, 1);
> > +	ASSERT_EQ(1, ret);
> > +
> > +	for (int i = 1; i <= 32; i++) {
> > +		ret = ioctl(self->fd, PCITEST_MSI, i);
> > +		EXPECT_EQ(1, ret) {
> > +			TH_LOG("TEST FAILED FOR MSI%d\n", i);
> > +			final++;
> > +		}
> > +	}
> > +
> > +	ASSERT_EQ(0, final);
> > +}
> > +
> > +TEST_F(device, READ_TEST)
> > +{
> > +	int final = 0;
> > +	int ret = -EINVAL;
> > +	unsigned long SIZE[5] = {1, 1024, 1025, 1024000, 1024001};
> > +
> > +	ret = ioctl(self->fd, PCITEST_SET_IRQTYPE, 1);
> > +	ASSERT_EQ(1, ret);
> > +
> > +	struct xfer_param param;
> > +
> > +	param.flag = 0;
> > +	for (int i = 0; i < 5; i++) {
> > +		param.size = SIZE[i];
> > +		ret = ioctl(self->fd, PCITEST_READ, &param);
> > +		EXPECT_EQ(1, ret) {
> > +			TH_LOG("TEST FAILED FOR size =%ld.\n", SIZE[i]);
> > +			final++;
> > +		}
> > +	}
> > +
> > +	ASSERT_EQ(0, final);
> > +}
> > +
> > +TEST_F(device, WRITE_TEST)
> > +{
> > +	int final = 0;
> > +	int ret = -EINVAL;
> > +	unsigned long SIZE[5] = {1, 1024, 1025, 1024000, 1024001};
> > +
> > +	ret = ioctl(self->fd, PCITEST_SET_IRQTYPE, 1);
> > +	ASSERT_EQ(1, ret);
> > +
> > +	struct xfer_param param;
> > +
> > +	param.flag = 0;
> > +
> > +	for (int i = 0; i < 5; i++) {
> > +		param.size = SIZE[i];
> > +		ret = ioctl(self->fd, PCITEST_WRITE, &param);
> > +		EXPECT_EQ(1, ret) {
> > +			TH_LOG("TEST FAILED FOR size =%ld.\n", SIZE[i]);
> > +			final++;
> > +		}
> > +	}
> > +
> > +	ASSERT_EQ(0, final);
> > +}
> > +
> > +TEST_F(device, COPY_TEST)
> > +{
> > +	int final = 0;
> > +	int ret = -EINVAL;
> > +	unsigned long SIZE[5] = {1, 1024, 1025, 1024000, 1024001};
> > +
> > +	ret = ioctl(self->fd, PCITEST_SET_IRQTYPE, 1);
> > +	ASSERT_EQ(1, ret);
> > +
> > +	struct xfer_param param;
> > +
> > +	param.flag = 0;
> > +
> > +	for (int i = 0; i < 5; i++) {
> > +		param.size = SIZE[i];
> > +		ret = ioctl(self->fd, PCITEST_COPY, &param);
> > +		EXPECT_EQ(1, ret) {
> > +			TH_LOG("TEST FAILED FOR size =%ld.\n", SIZE[i]);
> > +			final++;
> > +		}
> > +	}
> > +
> > +	ASSERT_EQ(0, final);
> > +}
> > +TEST_HARNESS_MAIN
> > -- 
> > 2.17.1
> > 
> 
> -- 
> மணிவண்ணன் சதாசிவம்

-- 
மணிவண்ணன் சதாசிவம்

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

[Shunsuke Mie]

Hi Aman,

It is a nice work.

On 2022/10/07 14:39, 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 +++++++++++++++++++++
This test is for a pci endpoint test driver. so I think it should be 
located on tools/testing/selftest/drivers/pci/endpoint. What do you think?
>   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
> @@ -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;
> +	};
> +
> +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;
> +	int final = 0;
> +
> +	for (int i = 0; i <= 5; i++) {
> +		ret = ioctl(self->fd, PCITEST_BAR, i);
> +
> +		EXPECT_EQ(1, ret) {
> +			TH_LOG("TEST FAILED FOR BAR %d\n", i);
> +			final++;
> +		}
> +	}
> +
> +	ASSERT_EQ(0, final);
> +}
> +
> +TEST_F(device, MSI_TEST)
> +{
> +	int ret = -EINVAL;
> +	int final = 0;
> +
> +	ret = ioctl(self->fd, PCITEST_SET_IRQTYPE, 1);
> +	ASSERT_EQ(1, ret);
> +
> +	for (int i = 1; i <= 32; i++) {
> +		ret = ioctl(self->fd, PCITEST_MSI, i);
> +		EXPECT_EQ(1, ret) {
> +			TH_LOG("TEST FAILED FOR MSI%d\n", i);
> +			final++;
> +		}
> +	}
> +
> +	ASSERT_EQ(0, final);
> +}
> +
> +TEST_F(device, READ_TEST)
> +{
> +	int final = 0;
> +	int ret = -EINVAL;
> +	unsigned long SIZE[5] = {1, 1024, 1025, 1024000, 1024001};
> +
> +	ret = ioctl(self->fd, PCITEST_SET_IRQTYPE, 1);
> +	ASSERT_EQ(1, ret);
> +
> +	struct xfer_param param;
> +
> +	param.flag = 0;
> +	for (int i = 0; i < 5; i++) {
> +		param.size = SIZE[i];
> +		ret = ioctl(self->fd, PCITEST_READ, &param);
> +		EXPECT_EQ(1, ret) {
> +			TH_LOG("TEST FAILED FOR size =%ld.\n", SIZE[i]);
> +			final++;
> +		}
> +	}
> +
> +	ASSERT_EQ(0, final);
> +}
> +
> +TEST_F(device, WRITE_TEST)
> +{
> +	int final = 0;
> +	int ret = -EINVAL;
> +	unsigned long SIZE[5] = {1, 1024, 1025, 1024000, 1024001};
> +
> +	ret = ioctl(self->fd, PCITEST_SET_IRQTYPE, 1);
> +	ASSERT_EQ(1, ret);
> +
> +	struct xfer_param param;
> +
> +	param.flag = 0;
> +
> +	for (int i = 0; i < 5; i++) {
> +		param.size = SIZE[i];
> +		ret = ioctl(self->fd, PCITEST_WRITE, &param);
> +		EXPECT_EQ(1, ret) {
> +			TH_LOG("TEST FAILED FOR size =%ld.\n", SIZE[i]);
> +			final++;
> +		}
> +	}
> +
> +	ASSERT_EQ(0, final);
> +}
> +
> +TEST_F(device, COPY_TEST)
> +{
> +	int final = 0;
> +	int ret = -EINVAL;
> +	unsigned long SIZE[5] = {1, 1024, 1025, 1024000, 1024001};
> +
> +	ret = ioctl(self->fd, PCITEST_SET_IRQTYPE, 1);
> +	ASSERT_EQ(1, ret);
> +
> +	struct xfer_param param;
> +
> +	param.flag = 0;
> +
> +	for (int i = 0; i < 5; i++) {
> +		param.size = SIZE[i];
> +		ret = ioctl(self->fd, PCITEST_COPY, &param);
> +		EXPECT_EQ(1, ret) {
> +			TH_LOG("TEST FAILED FOR size =%ld.\n", SIZE[i]);
> +			final++;
> +		}
> +	}
> +
> +	ASSERT_EQ(0, final);
> +}
> +TEST_HARNESS_MAIN


Best,

Shunsuke

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

[Shuah Khan]

On 10/6/22 23:39, 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>

Adding Bjorn Helgaas to the thread.

Adding pcit test under selftests is good. There is another pcitest
under tools/pci. I would like to see if the existing code in
tools/pci/pcitest.c can be leveraged.

As part of this test work, also look into removing tools/pci so
we don't have to maintain duplicate code in two places.

thanks,
-- Shuah

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

[Manivannan Sadhasivam]

On Thu, Dec 22, 2022 at 09:58:30AM -0700, Shuah Khan wrote:
> On 10/6/22 23:39, 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>
> 
> Adding Bjorn Helgaas to the thread.
> 
> Adding pcit test under selftests is good. There is another pcitest
> under tools/pci. I would like to see if the existing code in
> tools/pci/pcitest.c can be leveraged.
> 
> As part of this test work, also look into removing tools/pci so
> we don't have to maintain duplicate code in two places.
> 

It has been agreed in a thread with Greg [1] to {re}move the tests under
tools/pci and utilize the kselftest.

Thanks,
Mani

[1] https://lore.kernel.org/lkml/Y2FTWLw0tKuZ9Cdl@kroah.com/

> thanks,
> -- Shuah

-- 
மணிவண்ணன் சதாசிவம்

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

[Shuah Khan]

On 12/22/22 10:45, Manivannan Sadhasivam wrote:
> On Thu, Dec 22, 2022 at 09:58:30AM -0700, Shuah Khan wrote:
>> On 10/6/22 23:39, 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>
>>
>> Adding Bjorn Helgaas to the thread.
>>
>> Adding pcit test under selftests is good. There is another pcitest
>> under tools/pci. I would like to see if the existing code in
>> tools/pci/pcitest.c can be leveraged.
>>
>> As part of this test work, also look into removing tools/pci so
>> we don't have to maintain duplicate code in two places.
>>
> 
> It has been agreed in a thread with Greg [1] to {re}move the tests under
> tools/pci and utilize the kselftest.
> 

Inline with what I am suggesting. However, I don't see either move or
delete of tools/pci in the patch?

The first patch could start with git mv of the existing files and then
make changes to preserver the history.

> 
> [1] https://lore.kernel.org/lkml/Y2FTWLw0tKuZ9Cdl@kroah.com/
> 

thanks,
-- Shuah

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

[Aman Gupta/FDS SW /SSIR/Engineer/Samsung Electronics]

> -----Original Message-----
> From: Shunsuke Mie [mailto:mie@igel.co.jp]
> Sent: 21 December 2022 14:00
> To: 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
> Cc: 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
> 
> Hi Aman,
> 
> It is a nice work.
> 
> On 2022/10/07 14:39, 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
> > +++++++++++++++++++++
> This test is for a pci endpoint test driver. so I think it should be located on
> tools/testing/selftest/drivers/pci/endpoint. What do you think?

Hi Shunsuke,
Thanks for the review and nice thought about relocating the file. 
As per the review provided by Manivanan, I will be changing the name of the file to endpoint-test.c 
and hence I am thinking to move this file to tools/testing/selftest/driver/pci/endpoint_test.c   

Thanks,
Aman Gupta

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

[Manivannan Sadhasivam]

On Thu, Dec 22, 2022 at 10:49:48AM -0700, Shuah Khan wrote:
> On 12/22/22 10:45, Manivannan Sadhasivam wrote:
> > On Thu, Dec 22, 2022 at 09:58:30AM -0700, Shuah Khan wrote:
> > > On 10/6/22 23:39, 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>
> > > 
> > > Adding Bjorn Helgaas to the thread.
> > > 
> > > Adding pcit test under selftests is good. There is another pcitest
> > > under tools/pci. I would like to see if the existing code in
> > > tools/pci/pcitest.c can be leveraged.
> > > 
> > > As part of this test work, also look into removing tools/pci so
> > > we don't have to maintain duplicate code in two places.
> > > 
> > 
> > It has been agreed in a thread with Greg [1] to {re}move the tests under
> > tools/pci and utilize the kselftest.
> > 
> 
> Inline with what I am suggesting. However, I don't see either move or
> delete of tools/pci in the patch?
> 
> The first patch could start with git mv of the existing files and then
> make changes to preserver the history.
> 

Right. This patch was posted independently of the series [1] that I submitted to
fix the return values of IOCTL calls used in drivers/misc/pci_endpoint_test.c
driver.

Then in that series, it was decided to move the existing test to kselftest. So,
I suggested Aman Gupta [2] to integrate my latest patches, add the kselftest
patch on top, then remove the existing test under tools/pci.

The kselftest patch can also move the driver first and then make the change as
you suggested. Either way it is fine by me.

Thanks,
Mani

[1] https://lore.kernel.org/lkml/20220824123010.51763-1-manivannan.sadhasivam@linaro.org/
[2] https://lore.kernel.org/linux-pci/20221221072423.GC2922@thinkpad/ 

> > 
> > [1] https://lore.kernel.org/lkml/Y2FTWLw0tKuZ9Cdl@kroah.com/
> > 
> 
> thanks,
> -- Shuah
> 

-- 
மணிவண்ணன் சதாசிவம்

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

[Shuah Khan]

On 12/23/22 08:02, Manivannan Sadhasivam wrote:
> On Thu, Dec 22, 2022 at 10:49:48AM -0700, Shuah Khan wrote:
>> On 12/22/22 10:45, Manivannan Sadhasivam wrote:
>>> On Thu, Dec 22, 2022 at 09:58:30AM -0700, Shuah Khan wrote:
>>>> On 10/6/22 23:39, 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>
>>>>
>>>> Adding Bjorn Helgaas to the thread.
>>>>
>>>> Adding pcit test under selftests is good. There is another pcitest
>>>> under tools/pci. I would like to see if the existing code in
>>>> tools/pci/pcitest.c can be leveraged.
>>>>
>>>> As part of this test work, also look into removing tools/pci so
>>>> we don't have to maintain duplicate code in two places.
>>>>
>>>
>>> It has been agreed in a thread with Greg [1] to {re}move the tests under
>>> tools/pci and utilize the kselftest.
>>>
>>
>> Inline with what I am suggesting. However, I don't see either move or
>> delete of tools/pci in the patch?
>>
>> The first patch could start with git mv of the existing files and then
>> make changes to preserver the history.
>>
> 
> Right. This patch was posted independently of the series [1] that I submitted to
> fix the return values of IOCTL calls used in drivers/misc/pci_endpoint_test.c
> driver.
> 
> Then in that series, it was decided to move the existing test to kselftest. So,
> I suggested Aman Gupta [2] to integrate my latest patches, add the kselftest
> patch on top, then remove the existing test under tools/pci.
> 
> The kselftest patch can also move the driver first and then make the change as
> you suggested. Either way it is fine by me.
> 

As I mentioned in my previous email, I prefer to see the move as the first
patch and then changes on top. This preserves the history and cleaner.

thanks,
-- Shuah

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

[Aman Gupta/FDS SW /SSIR/Engineer/Samsung Electronics]

> -----Original Message-----
> From: Shuah Khan [mailto:skhan@linuxfoundation.org]
> Sent: 23 December 2022 22:01
> To: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.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; Bjorn Helgaas <helgaas@kernel.org>;
> linux-pci@vger.kernel.org; linux-kselftest@vger.kernel.org; Padmanabhan
> Rajanbabu <p.rajanbabu@samsung.com>; Shuah Khan
> <skhan@linuxfoundation.org>
> Subject: Re: [PATCH] selftests: pci: pci-selftest: add support for PCI endpoint
> driver test
> 
> On 12/23/22 08:02, Manivannan Sadhasivam wrote:
> > On Thu, Dec 22, 2022 at 10:49:48AM -0700, Shuah Khan wrote:
> >> On 12/22/22 10:45, Manivannan Sadhasivam wrote:
> >>> On Thu, Dec 22, 2022 at 09:58:30AM -0700, Shuah Khan wrote:
> >>>> On 10/6/22 23:39, 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>
> >>>>
> >>>> Adding Bjorn Helgaas to the thread.
> >>>>
> >>>> Adding pcit test under selftests is good. There is another pcitest
> >>>> under tools/pci. I would like to see if the existing code in
> >>>> tools/pci/pcitest.c can be leveraged.
> >>>>
> >>>> As part of this test work, also look into removing tools/pci so we
> >>>> don't have to maintain duplicate code in two places.
> >>>>
> >>>
> >>> It has been agreed in a thread with Greg [1] to {re}move the tests
> >>> under tools/pci and utilize the kselftest.
> >>>
> >>
> >> Inline with what I am suggesting. However, I don't see either move or
> >> delete of tools/pci in the patch?
> >>
> >> The first patch could start with git mv of the existing files and
> >> then make changes to preserver the history.
> >>
> >
> > Right. This patch was posted independently of the series [1] that I
> > submitted to fix the return values of IOCTL calls used in
> > drivers/misc/pci_endpoint_test.c driver.
> >
> > Then in that series, it was decided to move the existing test to
> > kselftest. So, I suggested Aman Gupta [2] to integrate my latest
> > patches, add the kselftest patch on top, then remove the existing test
> under tools/pci.
> >
> > The kselftest patch can also move the driver first and then make the
> > change as you suggested. Either way it is fine by me.
> >
> 
> As I mentioned in my previous email, I prefer to see the move as the first
> patch and then changes on top. This preserves the history and cleaner.
> 
> thanks,
> -- Shuah
> 

Hi Shuah,

Thanks for review and suggestion. I understand that we would like to reuse and preserve the history of tools/pci/pcietest.c. So we have two approaches:

1: Using git mv command move existing code from tools/pci/ to tools/testing/selftest/drivers/pci/ and then update the file to convert to kselftest framework. I thought about this but after movement, when we move it to kselftest format it is going to be huge churn and we will be having modification in almost all lines.

2: Develop kselftest based driver in tools/testing/selftest/drivers/pci/ and eventually delete existing file from tools/pci/ folder providing justification in commit message.

From my viewpoint, going with the second approach makes more sense because if almost complete file is getting modified, and it will make the review process complex and anyways there is not much code reusability.  
Please let me know if you have any other thought process or if I am missing anything to understand your approach.

Thanks,
Aman Gupta
> 

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

[Aman Gupta/FDS SW /SSIR/Engineer/Samsung Electronics]

Respected madam/sir
Gentle Reminder

> -----Original Message-----
> From: Aman Gupta/FDS SW /SSIR/Engineer/Samsung Electronics
> [mailto:aman1.gupta@samsung.com]
> Sent: 27 December 2022 10:45
> To: 'Shuah Khan' <skhan@linuxfoundation.org>; 'Manivannan Sadhasivam'
> <manivannan.sadhasivam@linaro.org>
> Cc: 'shradha.t@samsung.com' <shradha.t@samsung.com>;
> 'pankaj.dubey@samsung.com' <pankaj.dubey@samsung.com>;
> 'kishon@ti.com' <kishon@ti.com>; 'lpieralisi@kernel.org'
> <lpieralisi@kernel.org>; 'kw@linux.com' <kw@linux.com>;
> 'shuah@kernel.org' <shuah@kernel.org>; 'Bjorn Helgaas'
> <helgaas@kernel.org>; 'linux-pci@vger.kernel.org' <linux-
> pci@vger.kernel.org>; 'linux-kselftest@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
> 
> 
> 
> > -----Original Message-----
> > From: Shuah Khan [mailto:skhan@linuxfoundation.org]
> > Sent: 23 December 2022 22:01
> > To: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.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; Bjorn Helgaas <helgaas@kernel.org>;
> > linux-pci@vger.kernel.org; linux-kselftest@vger.kernel.org;
> > Padmanabhan Rajanbabu <p.rajanbabu@samsung.com>; Shuah Khan
> > <skhan@linuxfoundation.org>
> > Subject: Re: [PATCH] selftests: pci: pci-selftest: add support for PCI
> > endpoint driver test
> >
> > On 12/23/22 08:02, Manivannan Sadhasivam wrote:
> > > On Thu, Dec 22, 2022 at 10:49:48AM -0700, Shuah Khan wrote:
> > >> On 12/22/22 10:45, Manivannan Sadhasivam wrote:
> > >>> On Thu, Dec 22, 2022 at 09:58:30AM -0700, Shuah Khan wrote:
> > >>>> On 10/6/22 23:39, 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>
> > >>>>
> > >>>> Adding Bjorn Helgaas to the thread.
> > >>>>
> > >>>> Adding pcit test under selftests is good. There is another
> > >>>> pcitest under tools/pci. I would like to see if the existing code
> > >>>> in tools/pci/pcitest.c can be leveraged.
> > >>>>
> > >>>> As part of this test work, also look into removing tools/pci so
> > >>>> we don't have to maintain duplicate code in two places.
> > >>>>
> > >>>
> > >>> It has been agreed in a thread with Greg [1] to {re}move the tests
> > >>> under tools/pci and utilize the kselftest.
> > >>>
> > >>
> > >> Inline with what I am suggesting. However, I don't see either move
> > >> or delete of tools/pci in the patch?
> > >>
> > >> The first patch could start with git mv of the existing files and
> > >> then make changes to preserver the history.
> > >>
> > >
> > > Right. This patch was posted independently of the series [1] that I
> > > submitted to fix the return values of IOCTL calls used in
> > > drivers/misc/pci_endpoint_test.c driver.
> > >
> > > Then in that series, it was decided to move the existing test to
> > > kselftest. So, I suggested Aman Gupta [2] to integrate my latest
> > > patches, add the kselftest patch on top, then remove the existing
> > > test
> > under tools/pci.
> > >
> > > The kselftest patch can also move the driver first and then make the
> > > change as you suggested. Either way it is fine by me.
> > >
> >
> > As I mentioned in my previous email, I prefer to see the move as the
> > first patch and then changes on top. This preserves the history and cleaner.
> >
> > thanks,
> > -- Shuah
> >
> 
> Hi Shuah,
> 
> Thanks for review and suggestion. I understand that we would like to reuse
> and preserve the history of tools/pci/pcietest.c. So we have two approaches:
> 
> 1: Using git mv command move existing code from tools/pci/ to
> tools/testing/selftest/drivers/pci/ and then update the file to convert to
> kselftest framework. I thought about this but after movement, when we
> move it to kselftest format it is going to be huge churn and we will be having
> modification in almost all lines.
> 
> 2: Develop kselftest based driver in tools/testing/selftest/drivers/pci/ and
> eventually delete existing file from tools/pci/ folder providing justification in
> commit message.
> 
> From my viewpoint, going with the second approach makes more sense
> because if almost complete file is getting modified, and it will make the
> review process complex and anyways there is not much code reusability.
> Please let me know if you have any other thought process or if I am missing
> anything to understand your approach.
> 
> Thanks,
> Aman Gupta
> >

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

[Bjorn Helgaas]

On Tue, Dec 27, 2022 at 10:45:26AM +0530, Aman Gupta/FDS SW /SSIR/Engineer/Samsung Electronics wrote:
> ...
> Thanks for review and suggestion. I understand that we would like to
> reuse and preserve the history of tools/pci/pcietest.c. So we have
> two approaches:
> 
> 1: Using git mv command move existing code from tools/pci/ to
> tools/testing/selftest/drivers/pci/ and then update the file to
> convert to kselftest framework. I thought about this but after
> movement, when we move it to kselftest format it is going to be huge
> churn and we will be having modification in almost all lines.
> 
> 2: Develop kselftest based driver in
> tools/testing/selftest/drivers/pci/ and eventually delete existing
> file from tools/pci/ folder providing justification in commit
> message.
> 
> From my viewpoint, going with the second approach makes more sense
> because if almost complete file is getting modified, and it will
> make the review process complex and anyways there is not much code
> reusability.
>
> Please let me know if you have any other thought
> process or if I am missing anything to understand your approach.

I vote for the first approach, with "git mv" and subsequent conversion
(in separate patches, of course).  If git knows about the move,
"git log --follow" will be useful even though the conversion will be a
big patch.  Adding a new test with the connection to the old one only
in the commit log makes more work for people who dig through the
history in the future.

Bjorn

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

[Shuah Khan]

On 1/17/23 12:59, Bjorn Helgaas wrote:
> On Tue, Dec 27, 2022 at 10:45:26AM +0530, Aman Gupta/FDS SW /SSIR/Engineer/Samsung Electronics wrote:
>> ...
>> Thanks for review and suggestion. I understand that we would like to
>> reuse and preserve the history of tools/pci/pcietest.c. So we have
>> two approaches:
>>
>> 1: Using git mv command move existing code from tools/pci/ to
>> tools/testing/selftest/drivers/pci/ and then update the file to
>> convert to kselftest framework. I thought about this but after
>> movement, when we move it to kselftest format it is going to be huge
>> churn and we will be having modification in almost all lines.
>>
>> 2: Develop kselftest based driver in
>> tools/testing/selftest/drivers/pci/ and eventually delete existing
>> file from tools/pci/ folder providing justification in commit
>> message.
>>
>>  From my viewpoint, going with the second approach makes more sense
>> because if almost complete file is getting modified, and it will
>> make the review process complex and anyways there is not much code
>> reusability.
>>
>> Please let me know if you have any other thought
>> process or if I am missing anything to understand your approach.
> 
> I vote for the first approach, with "git mv" and subsequent conversion
> (in separate patches, of course).  If git knows about the move,
> "git log --follow" will be useful even though the conversion will be a
> big patch.  Adding a new test with the connection to the old one only
> in the commit log makes more work for people who dig through the
> history in the future.
> 

Thanks Bjorn for explaining this in more detail that I did.

Please send revised patches following the first approach.

thanks,
-- Shuah

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

['Manivannan Sadhasivam']

On Tue, Dec 27, 2022 at 10:45:26AM +0530, Aman Gupta/FDS SW /SSIR/Engineer/Samsung Electronics wrote:
> 
> 
> > -----Original Message-----
> > From: Shuah Khan [mailto:skhan@linuxfoundation.org]
> > Sent: 23 December 2022 22:01
> > To: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.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; Bjorn Helgaas <helgaas@kernel.org>;
> > linux-pci@vger.kernel.org; linux-kselftest@vger.kernel.org; Padmanabhan
> > Rajanbabu <p.rajanbabu@samsung.com>; Shuah Khan
> > <skhan@linuxfoundation.org>
> > Subject: Re: [PATCH] selftests: pci: pci-selftest: add support for PCI endpoint
> > driver test
> > 
> > On 12/23/22 08:02, Manivannan Sadhasivam wrote:
> > > On Thu, Dec 22, 2022 at 10:49:48AM -0700, Shuah Khan wrote:
> > >> On 12/22/22 10:45, Manivannan Sadhasivam wrote:
> > >>> On Thu, Dec 22, 2022 at 09:58:30AM -0700, Shuah Khan wrote:
> > >>>> On 10/6/22 23:39, 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>
> > >>>>
> > >>>> Adding Bjorn Helgaas to the thread.
> > >>>>
> > >>>> Adding pcit test under selftests is good. There is another pcitest
> > >>>> under tools/pci. I would like to see if the existing code in
> > >>>> tools/pci/pcitest.c can be leveraged.
> > >>>>
> > >>>> As part of this test work, also look into removing tools/pci so we
> > >>>> don't have to maintain duplicate code in two places.
> > >>>>
> > >>>
> > >>> It has been agreed in a thread with Greg [1] to {re}move the tests
> > >>> under tools/pci and utilize the kselftest.
> > >>>
> > >>
> > >> Inline with what I am suggesting. However, I don't see either move or
> > >> delete of tools/pci in the patch?
> > >>
> > >> The first patch could start with git mv of the existing files and
> > >> then make changes to preserver the history.
> > >>
> > >
> > > Right. This patch was posted independently of the series [1] that I
> > > submitted to fix the return values of IOCTL calls used in
> > > drivers/misc/pci_endpoint_test.c driver.
> > >
> > > Then in that series, it was decided to move the existing test to
> > > kselftest. So, I suggested Aman Gupta [2] to integrate my latest
> > > patches, add the kselftest patch on top, then remove the existing test
> > under tools/pci.
> > >
> > > The kselftest patch can also move the driver first and then make the
> > > change as you suggested. Either way it is fine by me.
> > >
> > 
> > As I mentioned in my previous email, I prefer to see the move as the first
> > patch and then changes on top. This preserves the history and cleaner.
> > 
> > thanks,
> > -- Shuah
> > 
> 
> Hi Shuah,
> 
> Thanks for review and suggestion. I understand that we would like to reuse and preserve the history of tools/pci/pcietest.c. So we have two approaches:
> 
> 1: Using git mv command move existing code from tools/pci/ to tools/testing/selftest/drivers/pci/ and then update the file to convert to kselftest framework. I thought about this but after movement, when we move it to kselftest format it is going to be huge churn and we will be having modification in almost all lines.
> 
> 2: Develop kselftest based driver in tools/testing/selftest/drivers/pci/ and eventually delete existing file from tools/pci/ folder providing justification in commit message.
> 
> From my viewpoint, going with the second approach makes more sense because if almost complete file is getting modified, and it will make the review process complex and anyways there is not much code reusability.  
> Please let me know if you have any other thought process or if I am missing anything to understand your approach.
> 

As Bjorn and Shuah said, I presume you are working on option 1.

Thanks,
Mani

> Thanks,
> Aman Gupta
> > 
> 
> 

-- 
மணிவண்ணன் சதாசிவம்

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

[Aman Gupta/FDS SW /SSIR/Engineer/Samsung Electronics]

> -----Original Message-----
> From: 'Manivannan Sadhasivam'
> [mailto:manivannan.sadhasivam@linaro.org]
> Sent: 14 February 2023 11:47
> To: Aman Gupta/FDS SW /SSIR/Engineer/Samsung Electronics
> <aman1.gupta@samsung.com>
> Cc: 'Shuah Khan' <skhan@linuxfoundation.org>; shradha.t@samsung.com;
> pankaj.dubey@samsung.com; kishon@ti.com; lpieralisi@kernel.org;
> kw@linux.com; shuah@kernel.org; 'Bjorn Helgaas' <helgaas@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, Dec 27, 2022 at 10:45:26AM +0530, Aman Gupta/FDS SW
> /SSIR/Engineer/Samsung Electronics wrote:
> >
> >
> > > -----Original Message-----
> > > From: Shuah Khan [mailto:skhan@linuxfoundation.org]
> > > Sent: 23 December 2022 22:01
> > > To: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.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; Bjorn Helgaas <helgaas@kernel.org>;
> > > linux-pci@vger.kernel.org; linux-kselftest@vger.kernel.org;
> > > Padmanabhan Rajanbabu <p.rajanbabu@samsung.com>; Shuah Khan
> > > <skhan@linuxfoundation.org>
> > > Subject: Re: [PATCH] selftests: pci: pci-selftest: add support for
> > > PCI endpoint driver test
> > >
> > > On 12/23/22 08:02, Manivannan Sadhasivam wrote:
> > > > On Thu, Dec 22, 2022 at 10:49:48AM -0700, Shuah Khan wrote:
> > > >> On 12/22/22 10:45, Manivannan Sadhasivam wrote:
> > > >>> On Thu, Dec 22, 2022 at 09:58:30AM -0700, Shuah Khan wrote:
> > > >>>> On 10/6/22 23:39, 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>
> > > >>>>
> > > >>>> Adding Bjorn Helgaas to the thread.
> > > >>>>
> > > >>>> Adding pcit test under selftests is good. There is another
> > > >>>> pcitest under tools/pci. I would like to see if the existing
> > > >>>> code in tools/pci/pcitest.c can be leveraged.
> > > >>>>
> > > >>>> As part of this test work, also look into removing tools/pci so
> > > >>>> we don't have to maintain duplicate code in two places.
> > > >>>>
> > > >>>
> > > >>> It has been agreed in a thread with Greg [1] to {re}move the
> > > >>> tests under tools/pci and utilize the kselftest.
> > > >>>
> > > >>
> > > >> Inline with what I am suggesting. However, I don't see either
> > > >> move or delete of tools/pci in the patch?
> > > >>
> > > >> The first patch could start with git mv of the existing files and
> > > >> then make changes to preserver the history.
> > > >>
> > > >
> > > > Right. This patch was posted independently of the series [1] that
> > > > I submitted to fix the return values of IOCTL calls used in
> > > > drivers/misc/pci_endpoint_test.c driver.
> > > >
> > > > Then in that series, it was decided to move the existing test to
> > > > kselftest. So, I suggested Aman Gupta [2] to integrate my latest
> > > > patches, add the kselftest patch on top, then remove the existing
> > > > test
> > > under tools/pci.
> > > >
> > > > The kselftest patch can also move the driver first and then make
> > > > the change as you suggested. Either way it is fine by me.
> > > >
> > >
> > > As I mentioned in my previous email, I prefer to see the move as the
> > > first patch and then changes on top. This preserves the history and
> cleaner.
> > >
> > > thanks,
> > > -- Shuah
> > >
> >
> > Hi Shuah,
> >
> > Thanks for review and suggestion. I understand that we would like to reuse
> and preserve the history of tools/pci/pcietest.c. So we have two approaches:
> >
> > 1: Using git mv command move existing code from tools/pci/ to
> tools/testing/selftest/drivers/pci/ and then update the file to convert to
> kselftest framework. I thought about this but after movement, when we
> move it to kselftest format it is going to be huge churn and we will be having
> modification in almost all lines.
> >
> > 2: Develop kselftest based driver in tools/testing/selftest/drivers/pci/ and
> eventually delete existing file from tools/pci/ folder providing justification in
> commit message.
> >
> > From my viewpoint, going with the second approach makes more sense
> because if almost complete file is getting modified, and it will make the
> review process complex and anyways there is not much code reusability.
> > Please let me know if you have any other thought process or if I am missing
> anything to understand your approach.
> >
> 
> As Bjorn and Shuah said, I presume you are working on option 1.
> 
> Thanks,
> Mani
> 
Hi Mani,
Yes I am working on it, soon I will post the patches.
Thanks ,
Aman Gupta
> > Thanks,
> > Aman Gupta
> > >
> >
> >
> 
> --
> மணிவண்ணன் சதாசிவம்

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

[Aman Gupta/FDS SW /SSIR/Engineer/Samsung Electronics]

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

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

['Manivannan Sadhasivam']

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-manivannan.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://git.linaro.org/people/manivannan.sadhasivam/linux.git/log/?h=pci-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.

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

> > >
> > 
> > Looks like we might end up removing the tests under tools/pci and just use
> > this one. I will CC you on the v3 of PCI test cleanup series. Please rebase this
> > patch on top of that and post after incorporating the review comments.
> 
> Okay, I will rebase the patches after v3 clean up gets posted.
> > 
> > Thanks,
> > Mani
> > 
> > > Thanks,
> > > Mani
> > >
> > > > +			TH_LOG("TEST FAILED FOR BAR %d\n", i);
> > > > +			final++;
> > > > +		}
> > > > +	}
> > > > +
> > > > +	ASSERT_EQ(0, final);
> > > > +}
> > > > +
> > > > +TEST_F(device, MSI_TEST)
> > > > +{
> > > > +	int ret = -EINVAL;
> > > > +	int final = 0;
> > > > +
> > > > +	ret = ioctl(self->fd, PCITEST_SET_IRQTYPE, 1);
> > > > +	ASSERT_EQ(1, ret);
> > > > +
> > > > +	for (int i = 1; i <= 32; i++) {
> > > > +		ret = ioctl(self->fd, PCITEST_MSI, i);
> > > > +		EXPECT_EQ(1, ret) {
> > > > +			TH_LOG("TEST FAILED FOR MSI%d\n", i);
> > > > +			final++;
> > > > +		}
> > > > +	}
> > > > +
> > > > +	ASSERT_EQ(0, final);
> > > > +}
> > > > +
> > > > +TEST_F(device, READ_TEST)
> > > > +{
> > > > +	int final = 0;
> > > > +	int ret = -EINVAL;
> > > > +	unsigned long SIZE[5] = {1, 1024, 1025, 1024000, 1024001};
> > > > +
> > > > +	ret = ioctl(self->fd, PCITEST_SET_IRQTYPE, 1);
> > > > +	ASSERT_EQ(1, ret);
> > > > +
> > > > +	struct xfer_param param;
> > > > +
> > > > +	param.flag = 0;
> > > > +	for (int i = 0; i < 5; i++) {
> > > > +		param.size = SIZE[i];
> > > > +		ret = ioctl(self->fd, PCITEST_READ, &param);
> > > > +		EXPECT_EQ(1, ret) {
> > > > +			TH_LOG("TEST FAILED FOR size =%ld.\n", SIZE[i]);
> > > > +			final++;
> > > > +		}
> > > > +	}
> > > > +
> > > > +	ASSERT_EQ(0, final);
> > > > +}
> > > > +
> > > > +TEST_F(device, WRITE_TEST)
> > > > +{
> > > > +	int final = 0;
> > > > +	int ret = -EINVAL;
> > > > +	unsigned long SIZE[5] = {1, 1024, 1025, 1024000, 1024001};
> > > > +
> > > > +	ret = ioctl(self->fd, PCITEST_SET_IRQTYPE, 1);
> > > > +	ASSERT_EQ(1, ret);
> > > > +
> > > > +	struct xfer_param param;
> > > > +
> > > > +	param.flag = 0;
> > > > +
> > > > +	for (int i = 0; i < 5; i++) {
> > > > +		param.size = SIZE[i];
> > > > +		ret = ioctl(self->fd, PCITEST_WRITE, &param);
> > > > +		EXPECT_EQ(1, ret) {
> > > > +			TH_LOG("TEST FAILED FOR size =%ld.\n", SIZE[i]);
> > > > +			final++;
> > > > +		}
> > > > +	}
> > > > +
> > > > +	ASSERT_EQ(0, final);
> > > > +}
> > > > +
> > > > +TEST_F(device, COPY_TEST)
> > > > +{
> > > > +	int final = 0;
> > > > +	int ret = -EINVAL;
> > > > +	unsigned long SIZE[5] = {1, 1024, 1025, 1024000, 1024001};
> > > > +
> > > > +	ret = ioctl(self->fd, PCITEST_SET_IRQTYPE, 1);
> > > > +	ASSERT_EQ(1, ret);
> > > > +
> > > > +	struct xfer_param param;
> > > > +
> > > > +	param.flag = 0;
> > > > +
> > > > +	for (int i = 0; i < 5; i++) {
> > > > +		param.size = SIZE[i];
> > > > +		ret = ioctl(self->fd, PCITEST_COPY, &param);
> > > > +		EXPECT_EQ(1, ret) {
> > > > +			TH_LOG("TEST FAILED FOR size =%ld.\n", SIZE[i]);
> > > > +			final++;
> > > > +		}
> > > > +	}
> > > > +
> > > > +	ASSERT_EQ(0, final);
> > > > +}
> > > > +TEST_HARNESS_MAIN
> > > > --
> > > > 2.17.1
> > > >
> > >
> > > --
> > > மணிவண்ணன் சதாசிவம்
> > 
> > --
> > மணிவண்ணன் சதாசிவம்
> Thank you for reviewing the patch.
> 
> Regards
> Aman gupta
> 
> 

-- 
மணிவண்ணன் சதாசிவம்

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

[Aman Gupta/FDS SW /SSIR/Engineer/Samsung Electronics]

> -----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-manivannan.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.
> >
> 
> Looks like we might end up removing the tests under tools/pci and just use
> this one. I will CC you on the v3 of PCI test cleanup series. Please rebase this
> patch on top of that and post after incorporating the review comments.

Okay, I will rebase the patches after v3 clean up gets posted.
> 
> Thanks,
> Mani
> 
> > Thanks,
> > Mani
> >
> > > +			TH_LOG("TEST FAILED FOR BAR %d\n", i);
> > > +			final++;
> > > +		}
> > > +	}
> > > +
> > > +	ASSERT_EQ(0, final);
> > > +}
> > > +
> > > +TEST_F(device, MSI_TEST)
> > > +{
> > > +	int ret = -EINVAL;
> > > +	int final = 0;
> > > +
> > > +	ret = ioctl(self->fd, PCITEST_SET_IRQTYPE, 1);
> > > +	ASSERT_EQ(1, ret);
> > > +
> > > +	for (int i = 1; i <= 32; i++) {
> > > +		ret = ioctl(self->fd, PCITEST_MSI, i);
> > > +		EXPECT_EQ(1, ret) {
> > > +			TH_LOG("TEST FAILED FOR MSI%d\n", i);
> > > +			final++;
> > > +		}
> > > +	}
> > > +
> > > +	ASSERT_EQ(0, final);
> > > +}
> > > +
> > > +TEST_F(device, READ_TEST)
> > > +{
> > > +	int final = 0;
> > > +	int ret = -EINVAL;
> > > +	unsigned long SIZE[5] = {1, 1024, 1025, 1024000, 1024001};
> > > +
> > > +	ret = ioctl(self->fd, PCITEST_SET_IRQTYPE, 1);
> > > +	ASSERT_EQ(1, ret);
> > > +
> > > +	struct xfer_param param;
> > > +
> > > +	param.flag = 0;
> > > +	for (int i = 0; i < 5; i++) {
> > > +		param.size = SIZE[i];
> > > +		ret = ioctl(self->fd, PCITEST_READ, &param);
> > > +		EXPECT_EQ(1, ret) {
> > > +			TH_LOG("TEST FAILED FOR size =%ld.\n", SIZE[i]);
> > > +			final++;
> > > +		}
> > > +	}
> > > +
> > > +	ASSERT_EQ(0, final);
> > > +}
> > > +
> > > +TEST_F(device, WRITE_TEST)
> > > +{
> > > +	int final = 0;
> > > +	int ret = -EINVAL;
> > > +	unsigned long SIZE[5] = {1, 1024, 1025, 1024000, 1024001};
> > > +
> > > +	ret = ioctl(self->fd, PCITEST_SET_IRQTYPE, 1);
> > > +	ASSERT_EQ(1, ret);
> > > +
> > > +	struct xfer_param param;
> > > +
> > > +	param.flag = 0;
> > > +
> > > +	for (int i = 0; i < 5; i++) {
> > > +		param.size = SIZE[i];
> > > +		ret = ioctl(self->fd, PCITEST_WRITE, &param);
> > > +		EXPECT_EQ(1, ret) {
> > > +			TH_LOG("TEST FAILED FOR size =%ld.\n", SIZE[i]);
> > > +			final++;
> > > +		}
> > > +	}
> > > +
> > > +	ASSERT_EQ(0, final);
> > > +}
> > > +
> > > +TEST_F(device, COPY_TEST)
> > > +{
> > > +	int final = 0;
> > > +	int ret = -EINVAL;
> > > +	unsigned long SIZE[5] = {1, 1024, 1025, 1024000, 1024001};
> > > +
> > > +	ret = ioctl(self->fd, PCITEST_SET_IRQTYPE, 1);
> > > +	ASSERT_EQ(1, ret);
> > > +
> > > +	struct xfer_param param;
> > > +
> > > +	param.flag = 0;
> > > +
> > > +	for (int i = 0; i < 5; i++) {
> > > +		param.size = SIZE[i];
> > > +		ret = ioctl(self->fd, PCITEST_COPY, &param);
> > > +		EXPECT_EQ(1, ret) {
> > > +			TH_LOG("TEST FAILED FOR size =%ld.\n", SIZE[i]);
> > > +			final++;
> > > +		}
> > > +	}
> > > +
> > > +	ASSERT_EQ(0, final);
> > > +}
> > > +TEST_HARNESS_MAIN
> > > --
> > > 2.17.1
> > >
> >
> > --
> > மணிவண்ணன் சதாசிவம்
> 
> --
> மணிவண்ணன் சதாசிவம்
Thank you for reviewing the patch.

Regards
Aman gupta