> -----Original Message----- > From: 'Manivannan Sadhasivam' > [mailto:manivannan.sadhasivam@xxxxxxxxxx] > Sent: 21 December 2022 12:54 > To: Aman Gupta/FDS SW /SSIR/Engineer/Samsung Electronics > <aman1.gupta@xxxxxxxxxxx> > Cc: 'Manivannan Sadhasivam' <mani@xxxxxxxxxx>; shradha.t@xxxxxxxxxxx; > pankaj.dubey@xxxxxxxxxxx; kishon@xxxxxx; lpieralisi@xxxxxxxxxx; > kw@xxxxxxxxx; shuah@xxxxxxxxxx; linux-pci@xxxxxxxxxxxxxxx; linux- > kselftest@xxxxxxxxxxxxxxx; 'Padmanabhan Rajanbabu' > <p.rajanbabu@xxxxxxxxxxx> > 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@xxxxxxxxxx] > > > Sent: 01 November 2022 22:50 > > > To: Manivannan Sadhasivam <mani@xxxxxxxxxx> > > > Cc: Aman Gupta <aman1.gupta@xxxxxxxxxxx>; > shradha.t@xxxxxxxxxxx; > > > pankaj.dubey@xxxxxxxxxxx; kishon@xxxxxx; lpieralisi@xxxxxxxxxx; > > > kw@xxxxxxxxx; shuah@xxxxxxxxxx; linux-pci@xxxxxxxxxxxxxxx; linux- > > > kselftest@xxxxxxxxxxxxxxx; Padmanabhan Rajanbabu > > > <p.rajanbabu@xxxxxxxxxxx> > > > 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@xxxxxxxxxxx> > > > > > Signed-off-by: Padmanabhan Rajanbabu > <p.rajanbabu@xxxxxxxxxxx> > > > > > --- > > > > > 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@xxxxxxxxxxx> */ > > > > > + > > > > > +#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@xxxxxxxxxx/ > > > > > > > > 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
