This small series updates the io_uring tests and adds them to the morello_transitional_extended list.
This series depends on the following Morello Linux series: "Support io_uring for Purecap apps"
Review branch: https://git.morello-project.org/tudcre01/morello-linux-ltp/-/commits/morello...
Tudor Cretu (2): syscalls/io_uring: Update uAPI structs runtest: Add io_uring tests to extended PCuABI syscall list
include/lapi/io_uring.h | 10 +++++----- runtest/morello_transitional_extended | 4 ++++ testcases/kernel/syscalls/io_uring/io_uring01.c | 4 ++-- testcases/kernel/syscalls/io_uring/io_uring02.c | 6 +++--- 4 files changed, 14 insertions(+), 10 deletions(-)
Update the signatures for the uAPI io_uring structs bringing in support for capabilities and re-visit relevant testcases to take into account the structs' new published versions.
Signed-off-by: Tudor Cretu tudor.cretu@arm.com --- include/lapi/io_uring.h | 10 +++++----- testcases/kernel/syscalls/io_uring/io_uring01.c | 4 ++-- testcases/kernel/syscalls/io_uring/io_uring02.c | 6 +++--- 3 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/include/lapi/io_uring.h b/include/lapi/io_uring.h index a63741a08..102e7945f 100644 --- a/include/lapi/io_uring.h +++ b/include/lapi/io_uring.h @@ -38,9 +38,9 @@ struct io_uring_sqe { int32_t fd; /* file descriptor to do IO on */ union { uint64_t off; /* offset into file */ - uint64_t addr2; + uintptr_t addr2; }; - uint64_t addr; /* pointer to buffer or iovecs */ + uintptr_t addr; /* pointer to buffer or iovecs */ uint32_t len; /* buffer size or number of iovecs */ union { __kernel_rwf_t rw_flags; @@ -55,7 +55,7 @@ struct io_uring_sqe { uint32_t statx_flags; uint32_t fadvise_advice; }; - uint64_t user_data; /* data to be passed back at completion time */ + uintptr_t user_data; /* data to be passed back at completion time */ union { struct { /* index into fixed buffers, if used */ @@ -143,7 +143,7 @@ enum { * IO completion data structure (Completion Queue Entry) */ struct io_uring_cqe { - uint64_t user_data; /* sqe->data submission passed back */ + uintptr_t user_data; /* sqe->data submission passed back */ int32_t res; /* result code for this event */ uint32_t flags; }; @@ -234,7 +234,7 @@ struct io_uring_params { struct io_uring_files_update { uint32_t offset; uint32_t resv; - uint64_t __attribute__((aligned(8))) fds; + uintptr_t __attribute__((aligned(8))) fds; };
#define IO_URING_OP_SUPPORTED (1U << 0) diff --git a/testcases/kernel/syscalls/io_uring/io_uring01.c b/testcases/kernel/syscalls/io_uring/io_uring01.c index 70151bb85..744c7eca2 100644 --- a/testcases/kernel/syscalls/io_uring/io_uring01.c +++ b/testcases/kernel/syscalls/io_uring/io_uring01.c @@ -203,10 +203,10 @@ static int submit_to_uring_sq(struct submitter *s, struct tcase *tc) sqe->flags = 0; sqe->fd = fd; sqe->opcode = tc->enter_flags; - sqe->addr = (unsigned long)iov->iov_base; + sqe->addr = (uintptr_t)iov->iov_base; sqe->len = BLOCK_SZ; sqe->off = 0; - sqe->user_data = (unsigned long long)iov; + sqe->user_data = (uintptr_t)iov; sring->array[index] = index; tail = next_tail;
diff --git a/testcases/kernel/syscalls/io_uring/io_uring02.c b/testcases/kernel/syscalls/io_uring/io_uring02.c index c5c770074..026c52b1b 100644 --- a/testcases/kernel/syscalls/io_uring/io_uring02.c +++ b/testcases/kernel/syscalls/io_uring/io_uring02.c @@ -103,7 +103,7 @@ static void drain_fallback(void) sqe_ptr->opcode = IORING_OP_SENDMSG; sqe_ptr->flags = IOSQE_IO_DRAIN; sqe_ptr->fd = sockpair[0]; - sqe_ptr->addr = (__u64)&spam_header; + sqe_ptr->addr = (uintptr_t)&spam_header; sqe_ptr->user_data = SPAM_MARK; uring.sqr_array[tail & *uring.sqr_mask] = i; } @@ -113,7 +113,7 @@ static void drain_fallback(void) sqe_ptr->opcode = IORING_OP_SENDMSG; sqe_ptr->flags = IOSQE_IO_DRAIN; sqe_ptr->fd = sendsock; - sqe_ptr->addr = (__u64)&beef_header; + sqe_ptr->addr = (uintptr_t)&beef_header; sqe_ptr->user_data = BEEF_MARK; uring.sqr_array[tail & *uring.sqr_mask] = i; count = ++i; @@ -218,7 +218,7 @@ static void run(void) sqe_ptr->opcode = IORING_OP_SENDMSG; sqe_ptr->flags = IOSQE_ASYNC; sqe_ptr->fd = sendsock; - sqe_ptr->addr = (__u64)&beef_header; + sqe_ptr->addr = (uintptr_t)&beef_header; sqe_ptr->user_data = BEEF_MARK; uring.sqr_array[tail & *uring.sqr_mask] = 0; tail++;
On 16/03/2023 15:23, Tudor Cretu wrote:
Update the signatures for the uAPI io_uring structs bringing in support for capabilities and re-visit relevant testcases to take into account the structs' new published versions.
Signed-off-by: Tudor Cretu tudor.cretu@arm.com
include/lapi/io_uring.h | 10 +++++----- testcases/kernel/syscalls/io_uring/io_uring01.c | 4 ++-- testcases/kernel/syscalls/io_uring/io_uring02.c | 6 +++--- 3 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/include/lapi/io_uring.h b/include/lapi/io_uring.h index a63741a08..102e7945f 100644 --- a/include/lapi/io_uring.h +++ b/include/lapi/io_uring.h @@ -38,9 +38,9 @@ struct io_uring_sqe { int32_t fd; /* file descriptor to do IO on */ union { uint64_t off; /* offset into file */
uint64_t addr2;
uintptr_t addr2;
This will break the struct layout in 32-bit. If we cannot use the uapi header directly, then we probably need to define something akin to __kernel_uintptr_t here.
};
- uint64_t addr; /* pointer to buffer or iovecs */
- uintptr_t addr; /* pointer to buffer or iovecs */ uint32_t len; /* buffer size or number of iovecs */ union { __kernel_rwf_t rw_flags;
@@ -55,7 +55,7 @@ struct io_uring_sqe { uint32_t statx_flags; uint32_t fadvise_advice; };
- uint64_t user_data; /* data to be passed back at completion time */
- uintptr_t user_data; /* data to be passed back at completion time */ union { struct { /* index into fixed buffers, if used */
AFAICT __pad2 also needs to be expanded. In fact I'm surprised that these changes work in PCuABI as they stand, because with __pad2 unchanged I would expect the struct to be 16 bytes smaller than the uapi definition. Maybe I'm missing something?
Kevin
@@ -143,7 +143,7 @@ enum {
- IO completion data structure (Completion Queue Entry)
*/ struct io_uring_cqe {
- uint64_t user_data; /* sqe->data submission passed back */
- uintptr_t user_data; /* sqe->data submission passed back */ int32_t res; /* result code for this event */ uint32_t flags;
}; @@ -234,7 +234,7 @@ struct io_uring_params { struct io_uring_files_update { uint32_t offset; uint32_t resv;
- uint64_t __attribute__((aligned(8))) fds;
- uintptr_t __attribute__((aligned(8))) fds;
}; #define IO_URING_OP_SUPPORTED (1U << 0) diff --git a/testcases/kernel/syscalls/io_uring/io_uring01.c b/testcases/kernel/syscalls/io_uring/io_uring01.c index 70151bb85..744c7eca2 100644 --- a/testcases/kernel/syscalls/io_uring/io_uring01.c +++ b/testcases/kernel/syscalls/io_uring/io_uring01.c @@ -203,10 +203,10 @@ static int submit_to_uring_sq(struct submitter *s, struct tcase *tc) sqe->flags = 0; sqe->fd = fd; sqe->opcode = tc->enter_flags;
- sqe->addr = (unsigned long)iov->iov_base;
- sqe->addr = (uintptr_t)iov->iov_base; sqe->len = BLOCK_SZ; sqe->off = 0;
- sqe->user_data = (unsigned long long)iov;
- sqe->user_data = (uintptr_t)iov; sring->array[index] = index; tail = next_tail;
diff --git a/testcases/kernel/syscalls/io_uring/io_uring02.c b/testcases/kernel/syscalls/io_uring/io_uring02.c index c5c770074..026c52b1b 100644 --- a/testcases/kernel/syscalls/io_uring/io_uring02.c +++ b/testcases/kernel/syscalls/io_uring/io_uring02.c @@ -103,7 +103,7 @@ static void drain_fallback(void) sqe_ptr->opcode = IORING_OP_SENDMSG; sqe_ptr->flags = IOSQE_IO_DRAIN; sqe_ptr->fd = sockpair[0];
sqe_ptr->addr = (__u64)&spam_header;
sqe_ptr->addr = (uintptr_t)&spam_header;@@ -113,7 +113,7 @@ static void drain_fallback(void) sqe_ptr->opcode = IORING_OP_SENDMSG; sqe_ptr->flags = IOSQE_IO_DRAIN; sqe_ptr->fd = sendsock;
- sqe_ptr->addr = (__u64)&beef_header;
- sqe_ptr->addr = (uintptr_t)&beef_header; sqe_ptr->user_data = BEEF_MARK; uring.sqr_array[tail & *uring.sqr_mask] = i; count = ++i;
@@ -218,7 +218,7 @@ static void run(void) sqe_ptr->opcode = IORING_OP_SENDMSG; sqe_ptr->flags = IOSQE_ASYNC; sqe_ptr->fd = sendsock;
- sqe_ptr->addr = (__u64)&beef_header;
- sqe_ptr->addr = (uintptr_t)&beef_header; sqe_ptr->user_data = BEEF_MARK; uring.sqr_array[tail & *uring.sqr_mask] = 0; tail++;
On 04-04-2023 10:18, Kevin Brodsky wrote:
On 16/03/2023 15:23, Tudor Cretu wrote:
Update the signatures for the uAPI io_uring structs bringing in support for capabilities and re-visit relevant testcases to take into account the structs' new published versions.
Signed-off-by: Tudor Cretu tudor.cretu@arm.com
include/lapi/io_uring.h | 10 +++++----- testcases/kernel/syscalls/io_uring/io_uring01.c | 4 ++-- testcases/kernel/syscalls/io_uring/io_uring02.c | 6 +++--- 3 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/include/lapi/io_uring.h b/include/lapi/io_uring.h index a63741a08..102e7945f 100644 --- a/include/lapi/io_uring.h +++ b/include/lapi/io_uring.h @@ -38,9 +38,9 @@ struct io_uring_sqe { int32_t fd; /* file descriptor to do IO on */ union { uint64_t off; /* offset into file */
uint64_t addr2;
uintptr_t addr2;This will break the struct layout in 32-bit. If we cannot use the uapi header directly, then we probably need to define something akin to __kernel_uintptr_t here.
Ah, good point!
};
- uint64_t addr; /* pointer to buffer or iovecs */
- uintptr_t addr; /* pointer to buffer or iovecs */ uint32_t len; /* buffer size or number of iovecs */ union { __kernel_rwf_t rw_flags;
@@ -55,7 +55,7 @@ struct io_uring_sqe { uint32_t statx_flags; uint32_t fadvise_advice; };
- uint64_t user_data; /* data to be passed back at completion time */
- uintptr_t user_data; /* data to be passed back at completion time */ union { struct { /* index into fixed buffers, if used */
AFAICT __pad2 also needs to be expanded. In fact I'm surprised that these changes work in PCuABI as they stand, because with __pad2 unchanged I would expect the struct to be 16 bytes smaller than the uapi definition. Maybe I'm missing something?
Good catch!
This struct is not actually used. The uAPI header is used if it exists, here (lines 21-25):
https://git.morello-project.org/tudcre01/morello-linux-ltp/-/blob/81111e0f44...
Kevin
@@ -143,7 +143,7 @@ enum {
- IO completion data structure (Completion Queue Entry)
*/ struct io_uring_cqe {
- uint64_t user_data; /* sqe->data submission passed back */
- uintptr_t user_data; /* sqe->data submission passed back */ int32_t res; /* result code for this event */ uint32_t flags; };
@@ -234,7 +234,7 @@ struct io_uring_params { struct io_uring_files_update { uint32_t offset; uint32_t resv;
- uint64_t __attribute__((aligned(8))) fds;
- uintptr_t __attribute__((aligned(8))) fds; };
#define IO_URING_OP_SUPPORTED (1U << 0) diff --git a/testcases/kernel/syscalls/io_uring/io_uring01.c b/testcases/kernel/syscalls/io_uring/io_uring01.c index 70151bb85..744c7eca2 100644 --- a/testcases/kernel/syscalls/io_uring/io_uring01.c +++ b/testcases/kernel/syscalls/io_uring/io_uring01.c @@ -203,10 +203,10 @@ static int submit_to_uring_sq(struct submitter *s, struct tcase *tc) sqe->flags = 0; sqe->fd = fd; sqe->opcode = tc->enter_flags;
- sqe->addr = (unsigned long)iov->iov_base;
- sqe->addr = (uintptr_t)iov->iov_base; sqe->len = BLOCK_SZ; sqe->off = 0;
- sqe->user_data = (unsigned long long)iov;
- sqe->user_data = (uintptr_t)iov; sring->array[index] = index; tail = next_tail;
diff --git a/testcases/kernel/syscalls/io_uring/io_uring02.c b/testcases/kernel/syscalls/io_uring/io_uring02.c index c5c770074..026c52b1b 100644 --- a/testcases/kernel/syscalls/io_uring/io_uring02.c +++ b/testcases/kernel/syscalls/io_uring/io_uring02.c @@ -103,7 +103,7 @@ static void drain_fallback(void) sqe_ptr->opcode = IORING_OP_SENDMSG; sqe_ptr->flags = IOSQE_IO_DRAIN; sqe_ptr->fd = sockpair[0];
sqe_ptr->addr = (__u64)&spam_header;
sqe_ptr->addr = (uintptr_t)&spam_header;@@ -113,7 +113,7 @@ static void drain_fallback(void) sqe_ptr->opcode = IORING_OP_SENDMSG; sqe_ptr->flags = IOSQE_IO_DRAIN; sqe_ptr->fd = sendsock;
- sqe_ptr->addr = (__u64)&beef_header;
- sqe_ptr->addr = (uintptr_t)&beef_header; sqe_ptr->user_data = BEEF_MARK; uring.sqr_array[tail & *uring.sqr_mask] = i; count = ++i;
@@ -218,7 +218,7 @@ static void run(void) sqe_ptr->opcode = IORING_OP_SENDMSG; sqe_ptr->flags = IOSQE_ASYNC; sqe_ptr->fd = sendsock;
- sqe_ptr->addr = (__u64)&beef_header;
- sqe_ptr->addr = (uintptr_t)&beef_header; sqe_ptr->user_data = BEEF_MARK; uring.sqr_array[tail & *uring.sqr_mask] = 0; tail++;
+ Beata
On 04/04/2023 12:22, Tudor Cretu wrote:
AFAICT __pad2 also needs to be expanded. In fact I'm surprised that these changes work in PCuABI as they stand, because with __pad2 unchanged I would expect the struct to be 16 bytes smaller than the uapi definition. Maybe I'm missing something?
Good catch!
This struct is not actually used. The uAPI header is used if it exists, here (lines 21-25):
https://git.morello-project.org/tudcre01/morello-linux-ltp/-/blob/81111e0f44...
Right I understand now! But then maybe we shouldn't bother updating these definitions? Do other tests actually build fine in purecap without uapi headers? If we modify these fallbacks we may want to test them too, but I'm not sure it's worth it.
I'm starting to wonder if it's not the same situation for struct clone_args, which we modified but might not be actually using?
Kevin
On Tue, Apr 04, 2023 at 01:46:37PM +0200, Kevin Brodsky wrote:
- Beata
On 04/04/2023 12:22, Tudor Cretu wrote:
AFAICT __pad2 also needs to be expanded. In fact I'm surprised that these changes work in PCuABI as they stand, because with __pad2 unchanged I would expect the struct to be 16 bytes smaller than the uapi definition. Maybe I'm missing something?
Good catch!
This struct is not actually used. The uAPI header is used if it exists, here (lines 21-25):
https://git.morello-project.org/tudcre01/morello-linux-ltp/-/blob/81111e0f44...
Right I understand now! But then maybe we shouldn't bother updating these definitions? Do other tests actually build fine in purecap without uapi headers? If we modify these fallbacks we may want to test them too, but I'm not sure it's worth it.
I'm starting to wonder if it's not the same situation for struct clone_args, which we modified but might not be actually using?
clone_args seems slightly different as its definition is not being pulled from kernel headers so this one needed to be updated as Musl does not provide one. I guess we could make it all depend on kernel headers but then we are ending up in a mess when respective libc(s) might be expecting smth else to maintain backward compatibility.
In the case of io_uring, the struct definition depends on IOSQE_FIXED_FILE (not sure why such a choice of def guard) so as long as that one is not defined in the uAPI header, the 'custom' definition of io_uring_sqe will be provided, so I assume that's for older kernels which might be potentially left-as-is, it is slightly behind kernel version either way. Though that would require placing somewhere a comment saying this one is not considered to be an issue for our use case. (?)
--- BR B.
Kevin
On 04-04-2023 14:28, Beata Michalska wrote:
On Tue, Apr 04, 2023 at 01:46:37PM +0200, Kevin Brodsky wrote:
- Beata
On 04/04/2023 12:22, Tudor Cretu wrote:
AFAICT __pad2 also needs to be expanded. In fact I'm surprised that these changes work in PCuABI as they stand, because with __pad2 unchanged I would expect the struct to be 16 bytes smaller than the uapi definition. Maybe I'm missing something?
Good catch!
This struct is not actually used. The uAPI header is used if it exists, here (lines 21-25):
https://git.morello-project.org/tudcre01/morello-linux-ltp/-/blob/81111e0f44...
Right I understand now! But then maybe we shouldn't bother updating these definitions? Do other tests actually build fine in purecap without uapi headers? If we modify these fallbacks we may want to test them too, but I'm not sure it's worth it.
I considered not updating them.
I'm starting to wonder if it's not the same situation for struct clone_args, which we modified but might not be actually using?
clone_args seems slightly different as its definition is not being pulled from kernel headers so this one needed to be updated as Musl does not provide one. I guess we could make it all depend on kernel headers but then we are ending up in a mess when respective libc(s) might be expecting smth else to maintain backward compatibility.
In the case of io_uring, the struct definition depends on IOSQE_FIXED_FILE (not sure why such a choice of def guard) so as long as that one is not defined in the uAPI header, the 'custom' definition of io_uring_sqe will be provided, so I assume that's for older kernels which might be potentially left-as-is, it is slightly behind kernel version either way.
The choice for the def guard is strange indeed. The struct io_uring_sqe was introduced in the io_uring uAPI header in a kernel version before IOSQE_FIXED_FILE was introduced. So there is a range of kernel versions where this LTP header would cause compilation issues of redefining the io_uring_sqe struct.
Though that would require placing somewhere a comment saying this one is not considered to be an issue for our use case. (?)
I'm not sure if I understand correctly. Is your suggestion to leave the 'custom' definition of io_uring_sqe unmodified and just add a comment?
Kind regards, Tudor
BR B.
Kevin
On 04/04/2023 16:09, Tudor Cretu wrote:
On 04-04-2023 14:28, Beata Michalska wrote:
On Tue, Apr 04, 2023 at 01:46:37PM +0200, Kevin Brodsky wrote:
- Beata
On 04/04/2023 12:22, Tudor Cretu wrote:
AFAICT __pad2 also needs to be expanded. In fact I'm surprised that these changes work in PCuABI as they stand, because with __pad2 unchanged I would expect the struct to be 16 bytes smaller than the uapi definition. Maybe I'm missing something?
Good catch!
This struct is not actually used. The uAPI header is used if it exists, here (lines 21-25):
https://git.morello-project.org/tudcre01/morello-linux-ltp/-/blob/81111e0f44...
Right I understand now! But then maybe we shouldn't bother updating these definitions? Do other tests actually build fine in purecap without uapi headers? If we modify these fallbacks we may want to test them too, but I'm not sure it's worth it.
I considered not updating them.
I'm starting to wonder if it's not the same situation for struct clone_args, which we modified but might not be actually using?
clone_args seems slightly different as its definition is not being pulled from kernel headers so this one needed to be updated as Musl does not provide one.
Right I hadn't realised that subtlety, it is a different case then indeed. I have no idea what defines (or not) HAVE_CLONE3 to be honest...
I guess we could make it all depend on kernel headers but then we are ending up in a mess when respective libc(s) might be expecting smth else to maintain backward compatibility.
I don't think we should diverge from the way LTP does things unless it really brings value, because the drawbacks are clear (more pain when merging from upstream mainly). In this case it doesn't sound worth diverging.
In the case of io_uring, the struct definition depends on IOSQE_FIXED_FILE (not sure why such a choice of def guard) so as long as that one is not defined in the uAPI header, the 'custom' definition of io_uring_sqe will be provided, so I assume that's for older kernels which might be potentially left-as-is, it is slightly behind kernel version either way.
The choice for the def guard is strange indeed. The struct io_uring_sqe was introduced in the io_uring uAPI header in a kernel version before IOSQE_FIXED_FILE was introduced. So there is a range of kernel versions where this LTP header would cause compilation issues of redefining the io_uring_sqe struct.
I don't think we really need to worry about this, we shouldn't try supporting building LTP in purecap against old kernel headers.
Though that would require placing somewhere a comment saying this one is not considered to be an issue for our use case. (?)
I'm not sure if I understand correctly. Is your suggestion to leave the 'custom' definition of io_uring_sqe unmodified and just add a comment?
That would make sense to me (for the whole header).
Kevin
On Tue, Apr 04, 2023 at 03:09:35PM +0100, Tudor Cretu wrote:
On 04-04-2023 14:28, Beata Michalska wrote:
On Tue, Apr 04, 2023 at 01:46:37PM +0200, Kevin Brodsky wrote:
- Beata
On 04/04/2023 12:22, Tudor Cretu wrote:
AFAICT __pad2 also needs to be expanded. In fact I'm surprised that these changes work in PCuABI as they stand, because with __pad2 unchanged I would expect the struct to be 16 bytes smaller than the uapi definition. Maybe I'm missing something?
Good catch!
This struct is not actually used. The uAPI header is used if it exists, here (lines 21-25):
https://git.morello-project.org/tudcre01/morello-linux-ltp/-/blob/81111e0f44...
Right I understand now! But then maybe we shouldn't bother updating these definitions? Do other tests actually build fine in purecap without uapi headers? If we modify these fallbacks we may want to test them too, but I'm not sure it's worth it.
I considered not updating them.
I'm starting to wonder if it's not the same situation for struct clone_args, which we modified but might not be actually using?
clone_args seems slightly different as its definition is not being pulled from kernel headers so this one needed to be updated as Musl does not provide one. I guess we could make it all depend on kernel headers but then we are ending up in a mess when respective libc(s) might be expecting smth else to maintain backward compatibility.
In the case of io_uring, the struct definition depends on IOSQE_FIXED_FILE (not sure why such a choice of def guard) so as long as that one is not defined in the uAPI header, the 'custom' definition of io_uring_sqe will be provided, so I assume that's for older kernels which might be potentially left-as-is, it is slightly behind kernel version either way.
The choice for the def guard is strange indeed. The struct io_uring_sqe was introduced in the io_uring uAPI header in a kernel version before IOSQE_FIXED_FILE was introduced. So there is a range of kernel versions where this LTP header would cause compilation issues of redefining the io_uring_sqe struct.
Though that would require placing somewhere a comment saying this one is not considered to be an issue for our use case. (?)
I'm not sure if I understand correctly. Is your suggestion to leave the 'custom' definition of io_uring_sqe unmodified and just add a comment?
Yes, what I am suggesting is leaving things as they are: this definition should not be pulled in for newer kernels, including Morello kernel, so it should be safe to leave it unmodified, as log as we leave some trace on why we have decided to do so.
--- BR B.
Kind regards, Tudor
BR B.
Kevin
On 04-04-2023 15:36, Beata Michalska wrote:
On Tue, Apr 04, 2023 at 03:09:35PM +0100, Tudor Cretu wrote:
On 04-04-2023 14:28, Beata Michalska wrote:
On Tue, Apr 04, 2023 at 01:46:37PM +0200, Kevin Brodsky wrote:
- Beata
On 04/04/2023 12:22, Tudor Cretu wrote:
AFAICT __pad2 also needs to be expanded. In fact I'm surprised that these changes work in PCuABI as they stand, because with __pad2 unchanged I would expect the struct to be 16 bytes smaller than the uapi definition. Maybe I'm missing something?
Good catch!
This struct is not actually used. The uAPI header is used if it exists, here (lines 21-25):
https://git.morello-project.org/tudcre01/morello-linux-ltp/-/blob/81111e0f44...
Right I understand now! But then maybe we shouldn't bother updating these definitions? Do other tests actually build fine in purecap without uapi headers? If we modify these fallbacks we may want to test them too, but I'm not sure it's worth it.
I considered not updating them.
I'm starting to wonder if it's not the same situation for struct clone_args, which we modified but might not be actually using?
clone_args seems slightly different as its definition is not being pulled from kernel headers so this one needed to be updated as Musl does not provide one. I guess we could make it all depend on kernel headers but then we are ending up in a mess when respective libc(s) might be expecting smth else to maintain backward compatibility.
In the case of io_uring, the struct definition depends on IOSQE_FIXED_FILE (not sure why such a choice of def guard) so as long as that one is not defined in the uAPI header, the 'custom' definition of io_uring_sqe will be provided, so I assume that's for older kernels which might be potentially left-as-is, it is slightly behind kernel version either way.
The choice for the def guard is strange indeed. The struct io_uring_sqe was introduced in the io_uring uAPI header in a kernel version before IOSQE_FIXED_FILE was introduced. So there is a range of kernel versions where this LTP header would cause compilation issues of redefining the io_uring_sqe struct.
Though that would require placing somewhere a comment saying this one is not considered to be an issue for our use case. (?)
I'm not sure if I understand correctly. Is your suggestion to leave the 'custom' definition of io_uring_sqe unmodified and just add a comment?
Yes, what I am suggesting is leaving things as they are: this definition should not be pulled in for newer kernels, including Morello kernel, so it should be safe to leave it unmodified, as log as we leave some trace on why we have decided to do so.
Many thanks, both!
I'll do that then.
Tudor
BR B.
Kind regards, Tudor
BR B.
Kevin
Now that the io_uring tests pass (for both purecap and compat), add them to the extended Morello transitional syscalls list.
Signed-off-by: Tudor Cretu tudor.cretu@arm.com --- runtest/morello_transitional_extended | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/runtest/morello_transitional_extended b/runtest/morello_transitional_extended index 7c980f230..45f64c760 100644 --- a/runtest/morello_transitional_extended +++ b/runtest/morello_transitional_extended @@ -41,6 +41,10 @@ io_submit01 io_submit01 io_submit02 io_submit02 io_submit03 io_submit03
+#KERN - io_uring*: depends on Morello Linux kernel release > morello-release-1.5.0 +io_uring01 io_uring01 +io_uring02 io_uring02 + #KERN - keyctl*: depends on Morello Linux kernel release >= morello-release-1.5.0 keyctl01 keyctl01 keyctl02 keyctl02
On 16/03/2023 15:23, Tudor Cretu wrote:
Now that the io_uring tests pass (for both purecap and compat), add them to the extended Morello transitional syscalls list.
Signed-off-by: Tudor Cretu tudor.cretu@arm.com
runtest/morello_transitional_extended | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/runtest/morello_transitional_extended b/runtest/morello_transitional_extended index 7c980f230..45f64c760 100644 --- a/runtest/morello_transitional_extended +++ b/runtest/morello_transitional_extended @@ -41,6 +41,10 @@ io_submit01 io_submit01 io_submit02 io_submit02 io_submit03 io_submit03 +#KERN - io_uring*: depends on Morello Linux kernel release > morello-release-1.5.0
The io_uring series is not in 1.6 either so the tag could be updated to morello-release-1.6.0.
Kevin
+io_uring01 io_uring01 +io_uring02 io_uring02
#KERN - keyctl*: depends on Morello Linux kernel release >= morello-release-1.5.0 keyctl01 keyctl01 keyctl02 keyctl02
On 16/03/2023 14:23, Tudor Cretu wrote:
This small series updates the io_uring tests and adds them to the morello_transitional_extended list.
This series depends on the following Morello Linux series: "Support io_uring for Purecap apps"
Review branch: https://git.morello-project.org/tudcre01/morello-linux-ltp/-/commits/morello...
Tudor Cretu (2): syscalls/io_uring: Update uAPI structs runtest: Add io_uring tests to extended PCuABI syscall list
include/lapi/io_uring.h | 10 +++++----- runtest/morello_transitional_extended | 4 ++++ testcases/kernel/syscalls/io_uring/io_uring01.c | 4 ++-- testcases/kernel/syscalls/io_uring/io_uring02.c | 6 +++--- 4 files changed, 14 insertions(+), 10 deletions(-)
Hi Tudor,
The series matches what I expected when I had a look myself. It looks good to me !
Thanks for the patches, Téo
Hi Tudor,
The series looks good though I'd rather wait for kernel changes to make their way through before applying those. Small note: support for skip list for syscalls will probably be there first so we would need to remove the io_uring01 from relevant skip file.
--- BR B. On Thu, Mar 16, 2023 at 02:23:39PM +0000, Tudor Cretu wrote:
This small series updates the io_uring tests and adds them to the morello_transitional_extended list.
This series depends on the following Morello Linux series: "Support io_uring for Purecap apps"
Review branch: https://git.morello-project.org/tudcre01/morello-linux-ltp/-/commits/morello...
Tudor Cretu (2): syscalls/io_uring: Update uAPI structs runtest: Add io_uring tests to extended PCuABI syscall list
include/lapi/io_uring.h | 10 +++++----- runtest/morello_transitional_extended | 4 ++++ testcases/kernel/syscalls/io_uring/io_uring01.c | 4 ++-- testcases/kernel/syscalls/io_uring/io_uring02.c | 6 +++--- 4 files changed, 14 insertions(+), 10 deletions(-)
-- 2.34.1
linux-morello-ltp mailing list -- linux-morello-ltp@op-lists.linaro.org To unsubscribe send an email to linux-morello-ltp-leave@op-lists.linaro.org
On 24-03-2023 10:35, Beata Michalska wrote:
Hi Tudor,
The series looks good though I'd rather wait for kernel changes to make their way through before applying those. Small note: support for skip list for syscalls will probably be there first so we would need to remove the io_uring01 from relevant skip file.
Ack, thanks Beata!
Tudor
BR B. On Thu, Mar 16, 2023 at 02:23:39PM +0000, Tudor Cretu wrote:
This small series updates the io_uring tests and adds them to the morello_transitional_extended list.
This series depends on the following Morello Linux series: "Support io_uring for Purecap apps"
Review branch: https://git.morello-project.org/tudcre01/morello-linux-ltp/-/commits/morello...
Tudor Cretu (2): syscalls/io_uring: Update uAPI structs runtest: Add io_uring tests to extended PCuABI syscall list
include/lapi/io_uring.h | 10 +++++----- runtest/morello_transitional_extended | 4 ++++ testcases/kernel/syscalls/io_uring/io_uring01.c | 4 ++-- testcases/kernel/syscalls/io_uring/io_uring02.c | 6 +++--- 4 files changed, 14 insertions(+), 10 deletions(-)
-- 2.34.1
linux-morello-ltp mailing list -- linux-morello-ltp@op-lists.linaro.org To unsubscribe send an email to linux-morello-ltp-leave@op-lists.linaro.org
linux-morello-ltp@op-lists.linaro.org
-
Beata Michalska -
Kevin Brodsky -
Teo Couprie Diaz -
Tudor Cretu