On 23/10/2023 10:15, Joao Martins wrote:
> On 23/10/2023 02:36, Nicolin Chen wrote:
>> On Sat, Oct 21, 2023 at 01:23:21PM -0300, Jason Gunthorpe wrote:
>>> On Fri, Oct 20, 2023 at 11:27:46PM +0100, Joao Martins wrote:
>>>> Changes since v4[8]:
>>>> * Rename HWPT_SET_DIRTY to HWPT_SET_DIRTY_TRACKING
>>>> * Rename IOMMU_CAP_DIRTY to IOMMU_CAP_DIRTY_TRACKING
>>>> * Rename HWPT_GET_DIRTY_IOVA to HWPT_GET_DIRTY_BITMAP
>>>> * Rename IOMMU_HWPT_ALLOC_ENFORCE_DIRTY to IOMMU_HWPT_ALLOC_DIRTY_TRACKING
>>>> including commit messages, code comments. Additionally change the
>>>> variable in drivers from enforce_dirty to dirty_tracking.
>>>> * Reflect all the mass renaming in commit messages/structs/docs.
>>>> * Fix the enums prefix to be IOMMU_HWPT like everyone else
>>>> * UAPI docs fixes/spelling and minor consistency issues/adjustments
>>>> * Change function exit style in __iommu_read_and_clear_dirty to return
>>>> right away instead of storing ret and returning at the end.
>>>> * Check 0 page_size and replace find-first-bit + left-shift with a
>>>> simple divide in iommufd_check_iova_range()
>>>> * Handle empty iommu domains when setting dirty tracking in intel-iommu;
>>>> Verified and amd-iommu was already the case.
>>>> * Remove unnecessary extra check for PGTT type
>>>> * Fix comment on function clearing the SLADE bit
>>>> * Fix wrong check that validates domain_alloc_user()
>>>> accepted flags in amd-iommu driver
>>>> * Skip IOTLB domain flush if no devices exist on the iommu domain,
>>>> while setting dirty tracking in amd-iommu driver.
>>>> * Collect Reviewed-by tags by Jason, Lu Baolu, Brett, Kevin, Alex
>>>
>>> I put this toward linux-next, let's see if we need a v6 next week with
>>> any remaining items.
>>
>> The selftest seems to be broken with this series:
>>
>> In file included from iommufd.c:10:0:
>> iommufd_utils.h:12:10: fatal error: linux/bitmap.h: No such file or directory
>> #include <linux/bitmap.h>
>> ^~~~~~~~~~~~~~~~
>> In file included from iommufd.c:10:0:
>> iommufd_utils.h:12:10: fatal error: linux/bitops.h: No such file or directory
>> #include <linux/bitops.h>
>> ^~~~~~~~~~~~~~~~
>> compilation terminated.
>>
>> Some of the tests are using kernel functions from these two headers
>> so I am not sure how to do any quick fix...
>
> Sorry for the oversight.
>
> It comes from the GET_DIRTY_BITMAP selftest ("iommufd/selftest: Test
> IOMMU_HWPT_GET_DIRTY_BITMAP") because I use test_bit/set_bit/BITS_PER_BYTE in
> bitmap validation to make sure all the bits are set/unset as expected. I think
> some time ago I had an issue on my environment that the selftests didn't build
> in-tree with the kernel unless it has the kernel headers installed in the
> system/path (between before/after commit 0d7a91678aaa ("selftests: iommu: Use
> installed kernel headers search path")) so I was mistakenly using:
>
> CFLAGS="-I../../../../tools/include/ -I../../../../include/uapi/
> -I../../../../include/"
>
> Just for the iommufd selftests, to replace what was prior to the commit plus
> `tools/include`:
>
> diff --git a/tools/testing/selftests/iommu/Makefile
> b/tools/testing/selftests/iommu/Makefile
> index 7cb74d26f141..32c5fdfd0eef 100644
> --- a/tools/testing/selftests/iommu/Makefile
> +++ b/tools/testing/selftests/iommu/Makefile
> @@ -1,7 +1,6 @@
> # SPDX-License-Identifier: GPL-2.0-only
> CFLAGS += -Wall -O2 -Wno-unused-function
> -CFLAGS += -I../../../../include/uapi/
> -CFLAGS += -I../../../../include/
> +CFLAGS += $(KHDR_INCLUDES)
>
> ... Which is what is masking your reported build problem for me.
> [The tests will build and run fine though once having the above]
>
> The usage of non UAPI kernel headers in selftests isn't unprecedented as I
> understand (if you grep for 'linux/bitmap.h') you will see a whole bunch. But
> maybe it isn't supposed to be used. Nonetheless the brokeness assumption was on
> my environment, and I have fixed up the environment now. Except for the above
> that you are reporting
>
> Perhaps the simpler change is to just import those two functions into the
> iommufd_util.h, since the selftest doesn't require any other non-UAPI headers. I
> have also had a couple more warnings/issues (in other patches), so I will need a
> v6 address to address everything.
Here's an example down that avoids the kernel header dependency; imported from
the arch-independent non-atomic bitops
(include/asm-generic/bitops/generic-non-atomic.h)
diff --git a/tools/testing/selftests/iommu/iommufd.c
b/tools/testing/selftests/iommu/iommufd.c
index 96837369a0aa..026ff9f5c1f3 100644
--- a/tools/testing/selftests/iommu/iommufd.c
+++ b/tools/testing/selftests/iommu/iommufd.c
@@ -12,7 +12,6 @@
static unsigned long HUGEPAGE_SIZE;
#define MOCK_PAGE_SIZE (PAGE_SIZE / 2)
-#define BITS_PER_BYTE 8
static unsigned long get_huge_page_size(void)
{
diff --git a/tools/testing/selftests/iommu/iommufd_utils.h
b/tools/testing/selftests/iommu/iommufd_utils.h
index 390563ff7935..6bbcab7fd6ab 100644
--- a/tools/testing/selftests/iommu/iommufd_utils.h
+++ b/tools/testing/selftests/iommu/iommufd_utils.h
@@ -9,8 +9,6 @@
#include <sys/ioctl.h>
#include <stdint.h>
#include <assert.h>
-#include <linux/bitmap.h>
-#include <linux/bitops.h>
#include "../kselftest_harness.h"
#include "../../../../drivers/iommu/iommufd/iommufd_test.h"
@@ -18,6 +16,24 @@
/* Hack to make assertions more readable */
#define _IOMMU_TEST_CMD(x) IOMMU_TEST_CMD
+/* Imported from include/asm-generic/bitops/generic-non-atomic.h */
+#define BITS_PER_BYTE 8
+#define BITS_PER_LONG __BITS_PER_LONG
+#define BIT_MASK(nr) (1UL << ((nr) % __BITS_PER_LONG))
+#define BIT_WORD(nr) ((nr) / __BITS_PER_LONG)
+
+static inline void set_bit(unsigned int nr, unsigned long *addr)
+{
+ unsigned long mask = BIT_MASK(nr);
+ unsigned long *p = ((unsigned long *)addr) + BIT_WORD(nr);
+
+ *p |= mask;
+}
+
+static inline bool test_bit(unsigned int nr, unsigned long *addr)
+{
+ return 1UL & (addr[BIT_WORD(nr)] >> (nr & (BITS_PER_LONG-1)));
+}
+
static void *buffer;
static unsigned long BUFFER_SIZE;
