Hi Pierrick,
On Mon, 15 Jun 2026 10:46:16 -0700, Pierrick Bouvier wrote:
We can't expose target dependent types, so having unsigned types for
args and ret was the correct thing to do. This way, a plugin can decide
to use any bit pattern, and upper part is discarded if needed.
For args, we explicitly clamp values depending on target:
https://gitlab.com/qemu-project/qemu/-/blob/master/plugins/core.c?ref_type=heads#L554
For ret, the type conversion is handled by writing ret to a temporary
variable, before sending it back:
https://gitlab.com/qemu-project/qemu/-/blob/master/linux-user/syscall.c?ref_type=heads#L14622
Indeed, there is an inconsistency between syscall_ret and syscall_filter
on that. However, they do different things:
- syscall_ret will read the value, so having a signed is mandatory.
`if ret < 0` needs to work.
- syscall_filter will only set it, so having a signed in not mandatory.
`*ret = -1` still does what is expected.
From that, I'm not sure any change is needed, but feel free to point a
use case I missed where this matter for syscall filter.
Regards,
Pierrick
You're right, and thanks for the detailed explanation. There is no
correctness issue here: the filter only sets sysret, and *sysret = -1
behaves the same whether the pointer is signed or unsigned, so nothing
is functionally broken.
My concern is purely about consistency. The syscall return value appears
in both callbacks: qemu_plugin_vcpu_syscall_filter_cb_t passes it as a
writable sysret and qemu_plugin_vcpu_syscall_ret_cb_t passes it as a
readable value, but in both it is the same kind of thing, a syscall
return value, whose semantics are inherently signed (the negative range
encodes errno). Having the write side use uint64_t while the read side
uses int64_t looks surprising to a plugin user, since it is the same
signed quantity in both places. Keeping the two consistent makes the
intent clearer, and as a bonus lets a filter write *sysret = -EINVAL
without an implicit signed to unsigned conversion.
As you say this is not mandatory, so I am happy to drop it if you would
rather leave it as is. It just seemed like a small readability
improvement given the two callbacks describe the same value.
Regards,
Ziyang Zhang
