Add a system call return test that tries all syscall types - #583
Conversation
Verify we can get all return types in userspace correctly
brghena
left a comment
There was a problem hiding this comment.
One question for you right now
| // Convert a `syscall_return_t` failure with one u32 to a `returncode_t`. | ||
| // | ||
| // This expects a failure with one u32 value (i.e. `TOCK_SYSCALL_FAILURE_U32`). | ||
| // Fills `val` with the failure u32 on failure. Do not use with other expected | ||
| // SyscallReturn variants. | ||
| returncode_t tock_command_return_failure_u32_to_returncode(syscall_return_t, uint32_t*); | ||
|
|
||
| // Convert a `syscall_return_t` failure with two u32 values to a `returncode_t`. | ||
| // | ||
| // This expects a failure with two u32 values (i.e. `TOCK_SYSCALL_FAILURE_U32_U32`). | ||
| // Fills `val1` and `val2` with the failure u32s on failure. Do not use with | ||
| // other expected SyscallReturn variants. | ||
| returncode_t tock_command_return_failure_u32_u32_to_returncode(syscall_return_t, uint32_t*, uint32_t*); | ||
|
|
||
| // Convert a `syscall_return_t` failure with a u64 value to a `returncode_t`. | ||
| // | ||
| // This expects a failure with one u64 value (i.e. `TOCK_SYSCALL_FAILURE_U64`). | ||
| // Fills `val` with the failure u64 on failure. Do not use with other expected | ||
| // SyscallReturn variants. | ||
| returncode_t tock_command_return_failure_u64_to_returncode(syscall_return_t, uint64_t*); |
There was a problem hiding this comment.
I'm trying to decide if these failure variants are generally useful or only applicable to your test application.
Let's say I was building a capsule which either returns success with U32 and U64 or failure with U64. Is this how you'd propose it be implemented? Any problems with this?
syscall_return_t cval = command(...);
returncode_t ret = tock_command_return_u32_u64_to_returncode(cval, u32_ptr, u64_ptr);
if (ret != RETURNCODE_SUCCESS) {
return tock_command_return_failure_u64_to_returncode(cval, u64_ptr);
}For IPC, I made a tock_command_return_u32_u64_or_u64_to_returncode() function, but I'll admit that it did feel quite unwieldy. I could replace it with the above snippet.
There was a problem hiding this comment.
Oh, interesting, you know I hadn't actually given this any thought. It seems wrong that we wrote the original converters assuming a default failure-no-value alternative without including that in the function name.
I kind of think what you did for IPC is what this all should look like. It's a little unwieldy, I guess, but it is what we want to express from syscalls: they have a single return success/failure variant.
But, I'm not sure how important it is to resolve this right now.
There was a problem hiding this comment.
Well, the reason I asked was because I was trying to figure out what the purpose of the Failure variants are at all. I think they're useful for testing, for what I described above, and for nothing else.
Matches tock/tock#5083
To accomplish this, this PR also: