Re: [PATCH v4 15/17] iommufd: Add a selftest

Jason Gunthorpe
2022-11-08T13:28:08Z

On Mon, Nov 07, 2022 at 09:48:32PM -0800, Nicolin Chen wrote:
> On Mon, Nov 07, 2022 at 08:49:08PM -0400, Jason Gunthorpe wrote:
>
> > diff --git a/tools/testing/selftests/iommu/iommufd.c b/tools/testing/selftests/iommu/iommufd.c
>
> > +TEST_F(iommufd, cmd_length)
> > +{
> > +#define TEST_LENGTH(_struct, _ioctl) \
> > + { \
> > + struct { \
> > + struct _struct cmd; \
> > + uint8_t extra; \
> > + } cmd = { .cmd = { .size = sizeof(struct _struct) - 1 }, \
> > + .extra = UINT8_MAX }; \
> > + int old_errno; \
> > + int rc; \
> > + \
> > + EXPECT_ERRNO(EOPNOTSUPP, ioctl(self->fd, _ioctl, &cmd)); \
>
> I guess it should be EINVAL corresponding to updated kernel code?
>
> > +TEST_F(iommufd, cmd_ex_fail)
> > +{
> > + struct {
> > + struct iommu_destroy cmd;
> > + __u64 future;
> > + } cmd = { .cmd = { .size = sizeof(cmd), .id = 0 } };
> > +
> > + /* object id is invalid and command is longer */
> > + EXPECT_ERRNO(ENOENT, ioctl(self->fd, IOMMU_DESTROY, &cmd));
> > + /* future area is non-zero */
> > + cmd.future = 1;
> > + EXPECT_ERRNO(E2BIG, ioctl(self->fd, IOMMU_DESTROY, &cmd));
> > + /* Original command "works" */
> > + cmd.cmd.size = sizeof(cmd.cmd);
> > + EXPECT_ERRNO(ENOENT, ioctl(self->fd, IOMMU_DESTROY, &cmd));
> > + /* Short command fails */
> > + cmd.cmd.size = sizeof(cmd.cmd) - 1;
> > + EXPECT_ERRNO(EOPNOTSUPP, ioctl(self->fd, IOMMU_DESTROY, &cmd));
>
> Ditto

Oops, yes, I fixed these

>
> > +TEST_HARNESS_MAIN
> > diff --git a/tools/testing/selftests/iommu/iommufd_fail_nth.c b/tools/testing/selftests/iommu/iommufd_fail_nth.c
>
> > +static void fail_nth_first(struct __test_metadata *_metadata,
> > + struct fail_nth_state *nth_state)
> > +{
> > + char buf[300];
> > +
> > + snprintf(buf, sizeof(buf), "/proc/self/task/%u/fail-nth", gettid());
>
> Not sure what's missing, I have a build error at gettid. Copying
> a solution from tools/perf/jvmti/jvmti_agent.c file, can fix with:

I think your glibc is probably old

> ------------------------------
> diff --git a/tools/testing/selftests/iommu/iommufd_fail_nth.c b/tools/testing/selftests/iommu/iommufd_fail_nth.c
> index 99eaa9f32e0b..7704b3a754d3 100644
> --- a/tools/testing/selftests/iommu/iommufd_fail_nth.c
> +++ b/tools/testing/selftests/iommu/iommufd_fail_nth.c
> @@ -19,6 +19,7 @@
>
> #define __EXPORTED_HEADERS__
> #include <linux/vfio.h>
> +#include <syscall.h> /* for gettid() */
>
> #include "iommufd_utils.h"
>
> @@ -84,6 +85,13 @@ struct fail_nth_state {
> unsigned int iteration;
> };
>
> +#ifndef HAVE_GETTID
> +static inline pid_t gettid(void)
> +{
> + return (pid_t)syscall(__NR_gettid);
> +}
> +#endif

Ah, there is a lot of complicated makefile stuff to make this work,
and it only works for perf/bpf not selftests

It looks like there is no reason for this to need gettid, we don't use
threads. So this can just be getpid and that is portable.

Thanks,
Jason