Re: [PATCH v5 00/18] IOMMUFD Dirty Tracking

[Joao Martins <joao.m.martins@oracle.com>]

On 23/10/2023 19:10, Nicolin Chen wrote:
> On Mon, Oct 23, 2023 at 10:15:24AM +0100, Joao Martins wrote:
> 
>> 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)
> 
> You'd need to run "make headers" before building the test.
> 

Eventually I noticed (some time ago), I just didn't undo the scripting

>> ... 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
> 
> Selftest is a user space program, so only uapi headers are
> allowed 

But OTOH selftests are in-tree and meant to exercise kernel code, so tieing them
up could make sense.

> unless you could find similar helpers in a library.
> 
Not in a library, but radix-tree, kvm selftests and nfit (NVDIMM) seem to use
similar non-uapi header (just talking about bitmap/bitops) more could be used.
But it still doesn't make it OK nonetheless and it's not even worth starting a
precedent in these selftests for no good reason.

>> 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.
> 
> Yea, thanks
> Nic