On Fri, Apr 10, 2020 at 04:17:01PM -0700, Sean Christopherson wrote:
> Add variants of GUEST_ASSERT to pass values back to the host, e.g. to
> help debug/understand a failure when the the cause of the assert isn't
> necessarily binary.
>
> It'd probably be possible to auto-calculate the number of arguments and
> just have a single GUEST_ASSERT, but there are a limited number of
> variants and silently eating arguments could lead to subtle code bugs.
>
> Signed-off-by: Sean Christopherson <sean.j.christopherson@xxxxxxxxx>
> ---
> .../testing/selftests/kvm/include/kvm_util.h | 25 +++++++++++++++----
> 1 file changed, 20 insertions(+), 5 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
> index d4c3e4d9cd92..e38d91bd8ec1 100644
> --- a/tools/testing/selftests/kvm/include/kvm_util.h
> +++ b/tools/testing/selftests/kvm/include/kvm_util.h
> @@ -313,11 +313,26 @@ uint64_t get_ucall(struct kvm_vm *vm, uint32_t vcpu_id, struct ucall *uc);
>
> #define GUEST_SYNC(stage) ucall(UCALL_SYNC, 2, "hello", stage)
> #define GUEST_DONE() ucall(UCALL_DONE, 0)
> -#define GUEST_ASSERT(_condition) do { \
> - if (!(_condition)) \
> - ucall(UCALL_ABORT, 2, \
> - "Failed guest assert: " \
> - #_condition, __LINE__); \
> +#define __GUEST_ASSERT(_condition, _nargs, _args...) do { \
> + if (!(_condition)) \
> + ucall(UCALL_ABORT, 2 + _nargs, \
Need () around _nargs
> + "Failed guest assert: " \
> + #_condition, __LINE__, _args); \
> } while (0)
We can free up another arg and add __FILE__. Something like the following
(untested):
#include "linux/stringify.h"
#define __GUEST_ASSERT(_condition, _nargs, _args...) do { \
if (!(_condition)) \
ucall(UCALL_ABORT, (_nargs) + 1, \
"Failed guest assert: " \
#_condition " at " __FILE__ ":" __stringify(__LINE__), _args); \
} while (0)
>
> +#define GUEST_ASSERT(_condition) \
> + __GUEST_ASSERT((_condition), 0, 0)
> +
> +#define GUEST_ASSERT_1(_condition, arg1) \
> + __GUEST_ASSERT((_condition), 1, (arg1))
> +
> +#define GUEST_ASSERT_2(_condition, arg1, arg2) \
> + __GUEST_ASSERT((_condition), 2, (arg1), (arg2))
> +
> +#define GUEST_ASSERT_3(_condition, arg1, arg2, arg3) \
> + __GUEST_ASSERT((_condition), 3, (arg1), (arg2), (arg3))
> +
> +#define GUEST_ASSERT_4(_condition, arg1, arg2, arg3, arg4) \
> + __GUEST_ASSERT((_condition), 4, (arg1), (arg2), (arg3), (arg4))
> +
nit: don't need the () around any of the macro params above
> #endif /* SELFTEST_KVM_UTIL_H */
> --
> 2.26.0
>
We could instead add test specific ucalls. To do so, we should first add
UCALL_TEST_SPECIFIC = 32 to the ucall enum, and then tests could extend it
enum {
MY_UCALL_1 = UCALL_TEST_SPECIFIC,
MY_UCALL_2,
};
With appropriately named test specific ucalls it may allow for clearer
code. At least GUEST_ASSERT_<N> wouldn't take on new meanings for each
test.
Thanks,
drew
