Re: [PATCH v3 1/6] selftests/mm: mseal, self_elf: fix missing __NR_mseal

[David Hildenbrand <david@redhat.com>]

On 18.06.24 22:14, John Hubbard wrote:
> On 6/17/24 11:56 PM, David Hildenbrand wrote:
>> On 18.06.24 04:24, John Hubbard wrote:
> ...
>>> diff --git a/tools/testing/selftests/mm/seal_elf.c b/tools/testing/selftests/mm/seal_elf.c
>>> index f2babec79bb6..27bf2f84231d 100644
>>> --- a/tools/testing/selftests/mm/seal_elf.c
>>> +++ b/tools/testing/selftests/mm/seal_elf.c
>>> @@ -2,7 +2,7 @@
>>>    #define _GNU_SOURCE
>>>    #include <sys/mman.h>
>>>    #include <stdint.h>
>>> -#include <unistd.h>
>>> +#include <asm-generic/unistd.h>
>>>    #include <string.h>
>>>    #include <sys/time.h>
>>>    #include <sys/resource.h>
>>
>> Still confused. Let's take a look at "microblaze".
>>
>> arch/microblaze/include/asm/unistd.h
>>    -> #include <uapi/asm/unistd.h>
>>
>> arch/microblaze/include/uapi/asm/unistd.h
>>    -> #include <asm/unistd_32.h>
>>     -> Generated during "make headers"
>>
>> usr/include/asm/unistd_32.h is generated via
>> arch/microblaze/kernel/syscalls/Makefile with the syshdr command.
>>
>> So we never end up including asm-generic/unistd.h directly on microblaze, but rather converts it (IIUC) to something else.
>>
> 
> Yes.
>    
>> That will work as expected here?
>>
> 
> No. :)
> 
> The problem, and the source of confusion here, is that for most user
> space programs, the header file inclusion behaves as you've mentioned
> above. However, those programs are installed on a single computer that
> has a single set of asm and kernel headers installed.
> 
> We are quite special here, because we are building a set of user space
> programs that:
> 
>       a) Mostly avoids using the installed (distro) system header files.
> 
>       b) Must build (and run) on all supported CPU architectures
> 
>       c) Must occasionally use symbols that have so new that they have not
>          yet been included in the distro's header files.
> 
> Doing (a) creates a new problem: how to get a set of cross-platform
> headers that works in all cases.
> 
> Fortunately, asm-generic headers solve that one. Which is why we need to
> use them here.
> 
> The reason this hasn't really come up yet, is that until now, the
> kselftests requirement (which I'm trying to remove) was that "make
> headers" must first be run. That allowed the selftests to get a snapshot
> of sufficiently new header files that looked just like (and conflict
> with) the installed system headers.
> 
> I can update the commit description with some of the above, if it helps.

I think it will. The main concern I had was that we could be ending up 
including headers with *wrong* data. As long as (a) it compiles where 
it's supposed to compile (b) it runs where it's supposed to run, we're 
good :)

-- 
Cheers,

David / dhildenb