On 7/20/23 13:10, Kevin Brodsky wrote:
On 17/07/2023 07:11, Chaitanya S Prakash wrote:
Wrapper functions for system calls that are frequently invoked are added in order to improve ease of usability. Making use of the newly added
Nit: either "ease of use", or "usability" :)
I'll change it.
support, mmap.c can be re-written in a manner that avoids directly interacting with syscalls. Required datatypes and constants have also been defined. Additionally, dependency required to utilise signal support has also been added.
Signed-off-by: Chaitanya S Prakash chaitanyas.prakash@arm.com
.../testing/selftests/arm64/morello/Makefile | 1 + .../selftests/arm64/morello/freestanding.h | 62 ++++++++++++++++++- tools/testing/selftests/arm64/morello/mmap.c | 11 ++-- 3 files changed, 67 insertions(+), 7 deletions(-)
diff --git a/tools/testing/selftests/arm64/morello/Makefile b/tools/testing/selftests/arm64/morello/Makefile index ecfa6fc5e989..d2e100a5aadc 100644 --- a/tools/testing/selftests/arm64/morello/Makefile +++ b/tools/testing/selftests/arm64/morello/Makefile @@ -43,3 +43,4 @@ $(OUTPUT)/%: $(OUTPUT)/%.o $(OUTPUT)/freestanding_start.o $(OUTPUT)/freestanding $(OUTPUT)/signal: $(OUTPUT)/signal_common.o $(OUTPUT)/clone: $(OUTPUT)/signal_common.o +$(OUTPUT)/mmap: $(OUTPUT)/signal_common.o
If I understand correctly, this is required due to patch 5. I agree with Tudor, I would rather move this diff to that patch, as this new dependency is otherwise rather mysterious.
Yes, it's required due to patch 5. I'll move it to that patch.
diff --git a/tools/testing/selftests/arm64/morello/freestanding.h b/tools/testing/selftests/arm64/morello/freestanding.h index 2beb52eafae1..3badf163569c 100644 --- a/tools/testing/selftests/arm64/morello/freestanding.h +++ b/tools/testing/selftests/arm64/morello/freestanding.h @@ -21,6 +21,9 @@ typedef __kernel_timer_t timer_t; typedef __kernel_clockid_t clockid_t; typedef __kernel_uid_t uid_t; typedef __kernel_mode_t mode_t; +typedef __kernel_off_t off_t; +typedef __kernel_key_t key_t;
I don't see key_t getting used in any patch.
I'll remove it.
- #ifndef __clang__ typedef __uintcap_t uintcap_t; #endif
@@ -29,6 +32,7 @@ typedef __uintcap_t uintcap_t; #define INT_MAX __INT_MAX__ #endif +#define PAGE_SIZE 4096
We should avoid hardcoding the page size to 4K. We only fully support what is in the transitional defconfig at the moment, i.e. 4K pages, but in principle other page sizes should just work too. The initial mmap tests do hardcode sizes, but this is safe as they are set to multiples of 64K (the maximum page size).
We could either set PAGE_SIZE as 64K, which is safe but may be limiting to test certain edge cases for the mmap syscalls. Or we could use the real page size - it does not seem to be necessary right now, but maybe later.
The canonical way to obtain the page size is sysconf(_SC_PAGESIZE). That said it may not be worth implementing sysconf() just for that. The way sysconf() itself obtains the page size is by reading the AT_PAGESZ auxiliary value. We could call some function in _start that finds this value and sets a global variable to it. The function could be written either in C or assembly, in that case both are about as simple.
I'll look into this and change it accordingly.
Kevin