Re: [PATCH V5 9/9] iommufd: map file selftest

[Nicolin Chen <nicolinc@nvidia.com>]

On Tue, Oct 22, 2024 at 02:00:38PM -0700, Steve Sistare wrote:

> Add test cases to exercise IOMMU_IOAS_MAP_FILE.
> 
> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>

LGTM overall.

Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>

With some nits:

> ---
>  tools/testing/selftests/iommu/iommufd.c          | 136 ++++++++++++++++++++---
>  tools/testing/selftests/iommu/iommufd_fail_nth.c |  39 +++++++
>  tools/testing/selftests/iommu/iommufd_utils.h    |  57 ++++++++++
>  3 files changed, 217 insertions(+), 15 deletions(-)
> 
> diff --git a/tools/testing/selftests/iommu/iommufd.c b/tools/testing/selftests/iommu/iommufd.c
> index 4927b9a..3d62951 100644
> --- a/tools/testing/selftests/iommu/iommufd.c
> +++ b/tools/testing/selftests/iommu/iommufd.c
> @@ -3,6 +3,7 @@
>  #include <stdlib.h>
>  #include <sys/mman.h>
>  #include <sys/eventfd.h>
> +#include <asm/unistd.h>

Can we add it to the top of the list for an alphabetical order?

>  TEST_F(iommufd_mock_domain, ro_unshare)
>  {
>         uint8_t *buf;
>         __u64 iova;
>         int fd;
> 
> +       if (variant->file)
> +               SKIP(return, "redundant test");

Why is it redundant? A line of comments maybe?

> +/* iopt_area_fill_domains() and iopt_area_fill_domain() */
> +TEST_FAIL_NTH(basic_fail_nth, map_file_domain)
> +{
[...]
> +       if (_test_ioctl_ioas_map_file(self->fd, ioas_id, mfd, 0, 262144, &iova,
> +                                IOMMU_IOAS_MAP_WRITEABLE |
> +                                        IOMMU_IOAS_MAP_READABLE))
[...]
> +#define test_ioctl_ioas_map_id_file(ioas_id, mfd, start, length, iova_p)     \
> +       ASSERT_EQ(0, _test_ioctl_ioas_map_file(self->fd, ioas_id, mfd,       \
> +                                         start, length, iova_p,             \
> +                                         IOMMU_IOAS_MAP_WRITEABLE |         \
> +                                                 IOMMU_IOAS_MAP_READABLE))
> +

The indentations are a bit odd here. Would you please do git-
clang-format to fix them?

Thanks
Nicolin