[PATCH 0/4] Fix filename reporting in guest asserts

[PATCH 0/4] Fix filename reporting in guest asserts

[Colton Lewis]

Fix filename reporting in guest asserts by way of abstracting out
magic numbers and introducing new reporting macros to report
consistently with less duplication.

Colton Lewis (4):
  KVM: selftests: enumerate GUEST_ASSERT arguments
  KVM: selftests: Increase UCALL_MAX_ARGS to 7
  KVM: selftests: Write REPORT_GUEST_ASSERT macros to pair with
    GUEST_ASSERT
  KVM: selftests: Fix filename reporting in guest asserts

 .../selftests/kvm/aarch64/arch_timer.c        | 12 ++--
 .../selftests/kvm/aarch64/debug-exceptions.c  |  4 +-
 .../testing/selftests/kvm/aarch64/vgic_irq.c  |  4 +-
 .../selftests/kvm/include/ucall_common.h      | 66 +++++++++++++++++--
 .../testing/selftests/kvm/memslot_perf_test.c |  4 +-
 tools/testing/selftests/kvm/steal_time.c      |  3 +-
 .../kvm/system_counter_offset_test.c          |  3 +-
 tools/testing/selftests/kvm/x86_64/amx_test.c |  3 +-
 .../testing/selftests/kvm/x86_64/cpuid_test.c |  3 +-
 .../kvm/x86_64/cr4_cpuid_sync_test.c          |  2 +-
 .../kvm/x86_64/emulator_error_test.c          |  3 +-
 .../testing/selftests/kvm/x86_64/evmcs_test.c |  3 +-
 .../selftests/kvm/x86_64/hyperv_clock.c       |  3 +-
 .../selftests/kvm/x86_64/hyperv_features.c    |  6 +-
 .../selftests/kvm/x86_64/hyperv_svm_test.c    |  3 +-
 .../selftests/kvm/x86_64/kvm_clock_test.c     |  3 +-
 .../selftests/kvm/x86_64/kvm_pv_test.c        |  3 +-
 .../selftests/kvm/x86_64/set_boot_cpu_id.c    |  4 +-
 .../testing/selftests/kvm/x86_64/state_test.c |  3 +-
 .../selftests/kvm/x86_64/svm_int_ctl_test.c   |  2 +-
 .../selftests/kvm/x86_64/svm_vmcall_test.c    |  2 +-
 .../selftests/kvm/x86_64/tsc_msrs_test.c      |  4 +-
 .../selftests/kvm/x86_64/userspace_io_test.c  |  4 +-
 .../kvm/x86_64/userspace_msr_exit_test.c      |  5 +-
 .../kvm/x86_64/vmx_apic_access_test.c         |  3 +-
 .../kvm/x86_64/vmx_close_while_nested_test.c  |  2 +-
 .../selftests/kvm/x86_64/vmx_dirty_log_test.c |  3 +-
 .../x86_64/vmx_invalid_nested_guest_state.c   |  2 +-
 .../kvm/x86_64/vmx_nested_tsc_scaling_test.c  |  2 +-
 .../kvm/x86_64/vmx_preemption_timer_test.c    |  3 +-
 .../kvm/x86_64/vmx_tsc_adjust_test.c          |  2 +-
 .../selftests/kvm/x86_64/xen_shinfo_test.c    |  2 +-
 .../selftests/kvm/x86_64/xen_vmcall_test.c    |  2 +-
 33 files changed, 100 insertions(+), 73 deletions(-)

-- 
2.36.1.476.g0c4daa206d-goog

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH 0/4] Fix filename reporting in guest asserts

[Colton Lewis]

Fix filename reporting in guest asserts by way of abstracting out
magic numbers and introducing new reporting macros to report
consistently with less duplication.

Colton Lewis (4):
  KVM: selftests: enumerate GUEST_ASSERT arguments
  KVM: selftests: Increase UCALL_MAX_ARGS to 7
  KVM: selftests: Write REPORT_GUEST_ASSERT macros to pair with
    GUEST_ASSERT
  KVM: selftests: Fix filename reporting in guest asserts

 .../selftests/kvm/aarch64/arch_timer.c        | 12 ++--
 .../selftests/kvm/aarch64/debug-exceptions.c  |  4 +-
 .../testing/selftests/kvm/aarch64/vgic_irq.c  |  4 +-
 .../selftests/kvm/include/ucall_common.h      | 66 +++++++++++++++++--
 .../testing/selftests/kvm/memslot_perf_test.c |  4 +-
 tools/testing/selftests/kvm/steal_time.c      |  3 +-
 .../kvm/system_counter_offset_test.c          |  3 +-
 tools/testing/selftests/kvm/x86_64/amx_test.c |  3 +-
 .../testing/selftests/kvm/x86_64/cpuid_test.c |  3 +-
 .../kvm/x86_64/cr4_cpuid_sync_test.c          |  2 +-
 .../kvm/x86_64/emulator_error_test.c          |  3 +-
 .../testing/selftests/kvm/x86_64/evmcs_test.c |  3 +-
 .../selftests/kvm/x86_64/hyperv_clock.c       |  3 +-
 .../selftests/kvm/x86_64/hyperv_features.c    |  6 +-
 .../selftests/kvm/x86_64/hyperv_svm_test.c    |  3 +-
 .../selftests/kvm/x86_64/kvm_clock_test.c     |  3 +-
 .../selftests/kvm/x86_64/kvm_pv_test.c        |  3 +-
 .../selftests/kvm/x86_64/set_boot_cpu_id.c    |  4 +-
 .../testing/selftests/kvm/x86_64/state_test.c |  3 +-
 .../selftests/kvm/x86_64/svm_int_ctl_test.c   |  2 +-
 .../selftests/kvm/x86_64/svm_vmcall_test.c    |  2 +-
 .../selftests/kvm/x86_64/tsc_msrs_test.c      |  4 +-
 .../selftests/kvm/x86_64/userspace_io_test.c  |  4 +-
 .../kvm/x86_64/userspace_msr_exit_test.c      |  5 +-
 .../kvm/x86_64/vmx_apic_access_test.c         |  3 +-
 .../kvm/x86_64/vmx_close_while_nested_test.c  |  2 +-
 .../selftests/kvm/x86_64/vmx_dirty_log_test.c |  3 +-
 .../x86_64/vmx_invalid_nested_guest_state.c   |  2 +-
 .../kvm/x86_64/vmx_nested_tsc_scaling_test.c  |  2 +-
 .../kvm/x86_64/vmx_preemption_timer_test.c    |  3 +-
 .../kvm/x86_64/vmx_tsc_adjust_test.c          |  2 +-
 .../selftests/kvm/x86_64/xen_shinfo_test.c    |  2 +-
 .../selftests/kvm/x86_64/xen_vmcall_test.c    |  2 +-
 33 files changed, 100 insertions(+), 73 deletions(-)

-- 
2.36.1.476.g0c4daa206d-goog

[PATCH 1/4] KVM: selftests: enumerate GUEST_ASSERT arguments

[Colton Lewis]

Enumerate GUEST_ASSERT arguments to avoid magic indices to ucall.args.

Signed-off-by: Colton Lewis <coltonlewis@google.com>
---
 tools/testing/selftests/kvm/include/ucall_common.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/tools/testing/selftests/kvm/include/ucall_common.h b/tools/testing/selftests/kvm/include/ucall_common.h
index 98562f685151..dbe872870b83 100644
--- a/tools/testing/selftests/kvm/include/ucall_common.h
+++ b/tools/testing/selftests/kvm/include/ucall_common.h
@@ -32,6 +32,14 @@ uint64_t get_ucall(struct kvm_vcpu *vcpu, struct ucall *uc);
 				ucall(UCALL_SYNC, 6, "hello", stage, arg1, arg2, arg3, arg4)
 #define GUEST_SYNC(stage)	ucall(UCALL_SYNC, 2, "hello", stage)
 #define GUEST_DONE()		ucall(UCALL_DONE, 0)
+
+enum guest_assert_builtin_args {
+	GUEST_ERROR_STRING,
+	GUEST_FILE,
+	GUEST_LINE,
+	GUEST_ASSERT_BUILTIN_NARGS
+};
+
 #define __GUEST_ASSERT(_condition, _condstr, _nargs, _args...) do {    \
 	if (!(_condition))                                              \
 		ucall(UCALL_ABORT, 2 + _nargs,                          \
-- 
2.36.1.476.g0c4daa206d-goog

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH 1/4] KVM: selftests: enumerate GUEST_ASSERT arguments

[Colton Lewis]

Enumerate GUEST_ASSERT arguments to avoid magic indices to ucall.args.

Signed-off-by: Colton Lewis <coltonlewis@google.com>
---
 tools/testing/selftests/kvm/include/ucall_common.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/tools/testing/selftests/kvm/include/ucall_common.h b/tools/testing/selftests/kvm/include/ucall_common.h
index 98562f685151..dbe872870b83 100644
--- a/tools/testing/selftests/kvm/include/ucall_common.h
+++ b/tools/testing/selftests/kvm/include/ucall_common.h
@@ -32,6 +32,14 @@ uint64_t get_ucall(struct kvm_vcpu *vcpu, struct ucall *uc);
 				ucall(UCALL_SYNC, 6, "hello", stage, arg1, arg2, arg3, arg4)
 #define GUEST_SYNC(stage)	ucall(UCALL_SYNC, 2, "hello", stage)
 #define GUEST_DONE()		ucall(UCALL_DONE, 0)
+
+enum guest_assert_builtin_args {
+	GUEST_ERROR_STRING,
+	GUEST_FILE,
+	GUEST_LINE,
+	GUEST_ASSERT_BUILTIN_NARGS
+};
+
 #define __GUEST_ASSERT(_condition, _condstr, _nargs, _args...) do {    \
 	if (!(_condition))                                              \
 		ucall(UCALL_ABORT, 2 + _nargs,                          \
-- 
2.36.1.476.g0c4daa206d-goog

Re: [PATCH 1/4] KVM: selftests: enumerate GUEST_ASSERT arguments

[Andrew Jones]

On Wed, Jun 15, 2022 at 07:31:13PM +0000, Colton Lewis wrote:
> Enumerate GUEST_ASSERT arguments to avoid magic indices to ucall.args.
> 
> Signed-off-by: Colton Lewis <coltonlewis@google.com>
> ---
>  tools/testing/selftests/kvm/include/ucall_common.h | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/tools/testing/selftests/kvm/include/ucall_common.h b/tools/testing/selftests/kvm/include/ucall_common.h
> index 98562f685151..dbe872870b83 100644
> --- a/tools/testing/selftests/kvm/include/ucall_common.h
> +++ b/tools/testing/selftests/kvm/include/ucall_common.h
> @@ -32,6 +32,14 @@ uint64_t get_ucall(struct kvm_vcpu *vcpu, struct ucall *uc);
>  				ucall(UCALL_SYNC, 6, "hello", stage, arg1, arg2, arg3, arg4)
>  #define GUEST_SYNC(stage)	ucall(UCALL_SYNC, 2, "hello", stage)
>  #define GUEST_DONE()		ucall(UCALL_DONE, 0)
> +
> +enum guest_assert_builtin_args {
> +	GUEST_ERROR_STRING,
> +	GUEST_FILE,
> +	GUEST_LINE,
> +	GUEST_ASSERT_BUILTIN_NARGS
> +};
> +
>  #define __GUEST_ASSERT(_condition, _condstr, _nargs, _args...) do {    \
>  	if (!(_condition))                                              \
>  		ucall(UCALL_ABORT, 2 + _nargs,                          \
> -- 
> 2.36.1.476.g0c4daa206d-goog
>

Reviewed-by: Andrew Jones <drjones@redhat.com>

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH 1/4] KVM: selftests: enumerate GUEST_ASSERT arguments

[Andrew Jones]

On Wed, Jun 15, 2022 at 07:31:13PM +0000, Colton Lewis wrote:
> Enumerate GUEST_ASSERT arguments to avoid magic indices to ucall.args.
> 
> Signed-off-by: Colton Lewis <coltonlewis@google.com>
> ---
>  tools/testing/selftests/kvm/include/ucall_common.h | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/tools/testing/selftests/kvm/include/ucall_common.h b/tools/testing/selftests/kvm/include/ucall_common.h
> index 98562f685151..dbe872870b83 100644
> --- a/tools/testing/selftests/kvm/include/ucall_common.h
> +++ b/tools/testing/selftests/kvm/include/ucall_common.h
> @@ -32,6 +32,14 @@ uint64_t get_ucall(struct kvm_vcpu *vcpu, struct ucall *uc);
>  				ucall(UCALL_SYNC, 6, "hello", stage, arg1, arg2, arg3, arg4)
>  #define GUEST_SYNC(stage)	ucall(UCALL_SYNC, 2, "hello", stage)
>  #define GUEST_DONE()		ucall(UCALL_DONE, 0)
> +
> +enum guest_assert_builtin_args {
> +	GUEST_ERROR_STRING,
> +	GUEST_FILE,
> +	GUEST_LINE,
> +	GUEST_ASSERT_BUILTIN_NARGS
> +};
> +
>  #define __GUEST_ASSERT(_condition, _condstr, _nargs, _args...) do {    \
>  	if (!(_condition))                                              \
>  		ucall(UCALL_ABORT, 2 + _nargs,                          \
> -- 
> 2.36.1.476.g0c4daa206d-goog
>

Reviewed-by: Andrew Jones <drjones@redhat.com>

[PATCH 2/4] KVM: selftests: Increase UCALL_MAX_ARGS to 7

[Colton Lewis]

Increase UCALL_MAX_ARGS to 7 to allow GUEST_ASSERT_4 to pass 3 builtin
ucall arguments specified in guest_assert_builtin_args plus 4
user-specified arguments.

Signed-off-by: Colton Lewis <coltonlewis@google.com>
---
 tools/testing/selftests/kvm/include/ucall_common.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/kvm/include/ucall_common.h b/tools/testing/selftests/kvm/include/ucall_common.h
index dbe872870b83..568c562f14cd 100644
--- a/tools/testing/selftests/kvm/include/ucall_common.h
+++ b/tools/testing/selftests/kvm/include/ucall_common.h
@@ -16,7 +16,7 @@ enum {
 	UCALL_UNHANDLED,
 };
 
-#define UCALL_MAX_ARGS 6
+#define UCALL_MAX_ARGS 7
 
 struct ucall {
 	uint64_t cmd;
-- 
2.36.1.476.g0c4daa206d-goog

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH 2/4] KVM: selftests: Increase UCALL_MAX_ARGS to 7

[Colton Lewis]

Increase UCALL_MAX_ARGS to 7 to allow GUEST_ASSERT_4 to pass 3 builtin
ucall arguments specified in guest_assert_builtin_args plus 4
user-specified arguments.

Signed-off-by: Colton Lewis <coltonlewis@google.com>
---
 tools/testing/selftests/kvm/include/ucall_common.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/kvm/include/ucall_common.h b/tools/testing/selftests/kvm/include/ucall_common.h
index dbe872870b83..568c562f14cd 100644
--- a/tools/testing/selftests/kvm/include/ucall_common.h
+++ b/tools/testing/selftests/kvm/include/ucall_common.h
@@ -16,7 +16,7 @@ enum {
 	UCALL_UNHANDLED,
 };
 
-#define UCALL_MAX_ARGS 6
+#define UCALL_MAX_ARGS 7
 
 struct ucall {
 	uint64_t cmd;
-- 
2.36.1.476.g0c4daa206d-goog

Re: [PATCH 2/4] KVM: selftests: Increase UCALL_MAX_ARGS to 7

[Andrew Jones]

On Wed, Jun 15, 2022 at 07:31:14PM +0000, Colton Lewis wrote:
> Increase UCALL_MAX_ARGS to 7 to allow GUEST_ASSERT_4 to pass 3 builtin
> ucall arguments specified in guest_assert_builtin_args plus 4
> user-specified arguments.
> 
> Signed-off-by: Colton Lewis <coltonlewis@google.com>
> ---
>  tools/testing/selftests/kvm/include/ucall_common.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/kvm/include/ucall_common.h b/tools/testing/selftests/kvm/include/ucall_common.h
> index dbe872870b83..568c562f14cd 100644
> --- a/tools/testing/selftests/kvm/include/ucall_common.h
> +++ b/tools/testing/selftests/kvm/include/ucall_common.h
> @@ -16,7 +16,7 @@ enum {
>  	UCALL_UNHANDLED,
>  };
>  
> -#define UCALL_MAX_ARGS 6
> +#define UCALL_MAX_ARGS 7
>  
>  struct ucall {
>  	uint64_t cmd;
> -- 
> 2.36.1.476.g0c4daa206d-goog
>

We probably want to ensure all architectures are good with this. afaict,
riscv only expects 6 args and uses UCALL_MAX_ARGS to cap the ucall inputs,
for example.

Thanks,
drew

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH 2/4] KVM: selftests: Increase UCALL_MAX_ARGS to 7

[Andrew Jones]

On Wed, Jun 15, 2022 at 07:31:14PM +0000, Colton Lewis wrote:
> Increase UCALL_MAX_ARGS to 7 to allow GUEST_ASSERT_4 to pass 3 builtin
> ucall arguments specified in guest_assert_builtin_args plus 4
> user-specified arguments.
> 
> Signed-off-by: Colton Lewis <coltonlewis@google.com>
> ---
>  tools/testing/selftests/kvm/include/ucall_common.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/kvm/include/ucall_common.h b/tools/testing/selftests/kvm/include/ucall_common.h
> index dbe872870b83..568c562f14cd 100644
> --- a/tools/testing/selftests/kvm/include/ucall_common.h
> +++ b/tools/testing/selftests/kvm/include/ucall_common.h
> @@ -16,7 +16,7 @@ enum {
>  	UCALL_UNHANDLED,
>  };
>  
> -#define UCALL_MAX_ARGS 6
> +#define UCALL_MAX_ARGS 7
>  
>  struct ucall {
>  	uint64_t cmd;
> -- 
> 2.36.1.476.g0c4daa206d-goog
>

We probably want to ensure all architectures are good with this. afaict,
riscv only expects 6 args and uses UCALL_MAX_ARGS to cap the ucall inputs,
for example.

Thanks,
drew

Re: [PATCH 2/4] KVM: selftests: Increase UCALL_MAX_ARGS to 7

[Colton Lewis]

On Thu, Jun 16, 2022 at 02:10:06PM +0200, Andrew Jones wrote:
> We probably want to ensure all architectures are good with this. afaict,
> riscv only expects 6 args and uses UCALL_MAX_ARGS to cap the ucall inputs,
> for example.

All architectures use UCALL_MAX_ARGS for that. Are you saying there
might be limitations beyond the value of the macro? If so, who should
verify whether this is ok?
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH 2/4] KVM: selftests: Increase UCALL_MAX_ARGS to 7

[Colton Lewis]

On Thu, Jun 16, 2022 at 02:10:06PM +0200, Andrew Jones wrote:
> We probably want to ensure all architectures are good with this. afaict,
> riscv only expects 6 args and uses UCALL_MAX_ARGS to cap the ucall inputs,
> for example.

All architectures use UCALL_MAX_ARGS for that. Are you saying there
might be limitations beyond the value of the macro? If so, who should
verify whether this is ok?

Re: [PATCH 2/4] KVM: selftests: Increase UCALL_MAX_ARGS to 7

[Sean Christopherson]

On Fri, Jun 17, 2022, Colton Lewis wrote:
> On Thu, Jun 16, 2022 at 02:10:06PM +0200, Andrew Jones wrote:
> > We probably want to ensure all architectures are good with this. afaict,
> > riscv only expects 6 args and uses UCALL_MAX_ARGS to cap the ucall inputs,
> > for example.
> 
> All architectures use UCALL_MAX_ARGS for that. Are you saying there
> might be limitations beyond the value of the macro? If so, who should
> verify whether this is ok?

I thought there were architectural limitations too, but I believe I was thinking
of vcpu_args_set(), where the number of params is limited by the function call
ABI, e.g. the number of registers.

Unless there's something really, really subtle going on, all architectures pass
the actual ucall struct purely through memory.  Actually, that code is ripe for
deduplication, and amazingly it doesn't conflict with Colton's series.  Patches
incoming...
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH 2/4] KVM: selftests: Increase UCALL_MAX_ARGS to 7

[Sean Christopherson]

On Fri, Jun 17, 2022, Colton Lewis wrote:
> On Thu, Jun 16, 2022 at 02:10:06PM +0200, Andrew Jones wrote:
> > We probably want to ensure all architectures are good with this. afaict,
> > riscv only expects 6 args and uses UCALL_MAX_ARGS to cap the ucall inputs,
> > for example.
> 
> All architectures use UCALL_MAX_ARGS for that. Are you saying there
> might be limitations beyond the value of the macro? If so, who should
> verify whether this is ok?

I thought there were architectural limitations too, but I believe I was thinking
of vcpu_args_set(), where the number of params is limited by the function call
ABI, e.g. the number of registers.

Unless there's something really, really subtle going on, all architectures pass
the actual ucall struct purely through memory.  Actually, that code is ripe for
deduplication, and amazingly it doesn't conflict with Colton's series.  Patches
incoming...

Re: [PATCH 2/4] KVM: selftests: Increase UCALL_MAX_ARGS to 7

[Andrew Jones]

On Sat, Jun 18, 2022 at 12:09:11AM +0000, Sean Christopherson wrote:
> On Fri, Jun 17, 2022, Colton Lewis wrote:
> > On Thu, Jun 16, 2022 at 02:10:06PM +0200, Andrew Jones wrote:
> > > We probably want to ensure all architectures are good with this. afaict,
> > > riscv only expects 6 args and uses UCALL_MAX_ARGS to cap the ucall inputs,
> > > for example.
> > 
> > All architectures use UCALL_MAX_ARGS for that. Are you saying there
> > might be limitations beyond the value of the macro? If so, who should
> > verify whether this is ok?
> 
> I thought there were architectural limitations too, but I believe I was thinking
> of vcpu_args_set(), where the number of params is limited by the function call
> ABI, e.g. the number of registers.
> 
> Unless there's something really, really subtle going on, all architectures pass
> the actual ucall struct purely through memory.  Actually, that code is ripe for
> deduplication, and amazingly it doesn't conflict with Colton's series.  Patches
> incoming...
>

RISC-V uses sbi_ecall() for their implementation of ucall(). CC'ing Anup
for confirmation, but if I understand the SBI spec correctly, then inputs
are limited to registers a0-a5.

Thanks,
drew

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH 2/4] KVM: selftests: Increase UCALL_MAX_ARGS to 7

[Andrew Jones]

On Sat, Jun 18, 2022 at 12:09:11AM +0000, Sean Christopherson wrote:
> On Fri, Jun 17, 2022, Colton Lewis wrote:
> > On Thu, Jun 16, 2022 at 02:10:06PM +0200, Andrew Jones wrote:
> > > We probably want to ensure all architectures are good with this. afaict,
> > > riscv only expects 6 args and uses UCALL_MAX_ARGS to cap the ucall inputs,
> > > for example.
> > 
> > All architectures use UCALL_MAX_ARGS for that. Are you saying there
> > might be limitations beyond the value of the macro? If so, who should
> > verify whether this is ok?
> 
> I thought there were architectural limitations too, but I believe I was thinking
> of vcpu_args_set(), where the number of params is limited by the function call
> ABI, e.g. the number of registers.
> 
> Unless there's something really, really subtle going on, all architectures pass
> the actual ucall struct purely through memory.  Actually, that code is ripe for
> deduplication, and amazingly it doesn't conflict with Colton's series.  Patches
> incoming...
>

RISC-V uses sbi_ecall() for their implementation of ucall(). CC'ing Anup
for confirmation, but if I understand the SBI spec correctly, then inputs
are limited to registers a0-a5.

Thanks,
drew

Re: [PATCH 2/4] KVM: selftests: Increase UCALL_MAX_ARGS to 7

[Anup Patel]

On Mon, Jun 20, 2022 at 12:51 PM Andrew Jones <drjones@redhat.com> wrote:
>
> On Sat, Jun 18, 2022 at 12:09:11AM +0000, Sean Christopherson wrote:
> > On Fri, Jun 17, 2022, Colton Lewis wrote:
> > > On Thu, Jun 16, 2022 at 02:10:06PM +0200, Andrew Jones wrote:
> > > > We probably want to ensure all architectures are good with this. afaict,
> > > > riscv only expects 6 args and uses UCALL_MAX_ARGS to cap the ucall inputs,
> > > > for example.
> > >
> > > All architectures use UCALL_MAX_ARGS for that. Are you saying there
> > > might be limitations beyond the value of the macro? If so, who should
> > > verify whether this is ok?
> >
> > I thought there were architectural limitations too, but I believe I was thinking
> > of vcpu_args_set(), where the number of params is limited by the function call
> > ABI, e.g. the number of registers.
> >
> > Unless there's something really, really subtle going on, all architectures pass
> > the actual ucall struct purely through memory.  Actually, that code is ripe for
> > deduplication, and amazingly it doesn't conflict with Colton's series.  Patches
> > incoming...
> >
>
> RISC-V uses sbi_ecall() for their implementation of ucall(). CC'ing Anup
> for confirmation, but if I understand the SBI spec correctly, then inputs
> are limited to registers a0-a5.

Yes, we only have 6 parameters in ucall() since it is based on SBI spec.

Actually, a6 and a7 are used to identify the type of SBI call (i.e. extension
and function number) whereas a0-a5 are function parameters.

Regards,
Anup

>
> Thanks,
> drew
>
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH 2/4] KVM: selftests: Increase UCALL_MAX_ARGS to 7

[Anup Patel]

On Mon, Jun 20, 2022 at 12:51 PM Andrew Jones <drjones@redhat.com> wrote:
>
> On Sat, Jun 18, 2022 at 12:09:11AM +0000, Sean Christopherson wrote:
> > On Fri, Jun 17, 2022, Colton Lewis wrote:
> > > On Thu, Jun 16, 2022 at 02:10:06PM +0200, Andrew Jones wrote:
> > > > We probably want to ensure all architectures are good with this. afaict,
> > > > riscv only expects 6 args and uses UCALL_MAX_ARGS to cap the ucall inputs,
> > > > for example.
> > >
> > > All architectures use UCALL_MAX_ARGS for that. Are you saying there
> > > might be limitations beyond the value of the macro? If so, who should
> > > verify whether this is ok?
> >
> > I thought there were architectural limitations too, but I believe I was thinking
> > of vcpu_args_set(), where the number of params is limited by the function call
> > ABI, e.g. the number of registers.
> >
> > Unless there's something really, really subtle going on, all architectures pass
> > the actual ucall struct purely through memory.  Actually, that code is ripe for
> > deduplication, and amazingly it doesn't conflict with Colton's series.  Patches
> > incoming...
> >
>
> RISC-V uses sbi_ecall() for their implementation of ucall(). CC'ing Anup
> for confirmation, but if I understand the SBI spec correctly, then inputs
> are limited to registers a0-a5.

Yes, we only have 6 parameters in ucall() since it is based on SBI spec.

Actually, a6 and a7 are used to identify the type of SBI call (i.e. extension
and function number) whereas a0-a5 are function parameters.

Regards,
Anup

>
> Thanks,
> drew
>

Re: [PATCH 2/4] KVM: selftests: Increase UCALL_MAX_ARGS to 7

[Andrew Jones]

On Mon, Jun 20, 2022 at 09:21:11AM +0200, Andrew Jones wrote:
> On Sat, Jun 18, 2022 at 12:09:11AM +0000, Sean Christopherson wrote:
> > On Fri, Jun 17, 2022, Colton Lewis wrote:
> > > On Thu, Jun 16, 2022 at 02:10:06PM +0200, Andrew Jones wrote:
> > > > We probably want to ensure all architectures are good with this. afaict,
> > > > riscv only expects 6 args and uses UCALL_MAX_ARGS to cap the ucall inputs,
> > > > for example.
> > > 
> > > All architectures use UCALL_MAX_ARGS for that. Are you saying there
> > > might be limitations beyond the value of the macro? If so, who should
> > > verify whether this is ok?
> > 
> > I thought there were architectural limitations too, but I believe I was thinking
> > of vcpu_args_set(), where the number of params is limited by the function call
> > ABI, e.g. the number of registers.
> > 
> > Unless there's something really, really subtle going on, all architectures pass
> > the actual ucall struct purely through memory.  Actually, that code is ripe for
> > deduplication, and amazingly it doesn't conflict with Colton's series.  Patches
> > incoming...
> >
> 
> RISC-V uses sbi_ecall() for their implementation of ucall(). CC'ing Anup
> for confirmation, but if I understand the SBI spec correctly, then inputs
> are limited to registers a0-a5.

Ah, never mind... I see SBI is limited to 6 registers, but of course it
only needs one register to pass the uc address... We can make
UCALL_MAX_ARGS whatever we want.

Thanks,
drew

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH 2/4] KVM: selftests: Increase UCALL_MAX_ARGS to 7

[Andrew Jones]

On Mon, Jun 20, 2022 at 09:21:11AM +0200, Andrew Jones wrote:
> On Sat, Jun 18, 2022 at 12:09:11AM +0000, Sean Christopherson wrote:
> > On Fri, Jun 17, 2022, Colton Lewis wrote:
> > > On Thu, Jun 16, 2022 at 02:10:06PM +0200, Andrew Jones wrote:
> > > > We probably want to ensure all architectures are good with this. afaict,
> > > > riscv only expects 6 args and uses UCALL_MAX_ARGS to cap the ucall inputs,
> > > > for example.
> > > 
> > > All architectures use UCALL_MAX_ARGS for that. Are you saying there
> > > might be limitations beyond the value of the macro? If so, who should
> > > verify whether this is ok?
> > 
> > I thought there were architectural limitations too, but I believe I was thinking
> > of vcpu_args_set(), where the number of params is limited by the function call
> > ABI, e.g. the number of registers.
> > 
> > Unless there's something really, really subtle going on, all architectures pass
> > the actual ucall struct purely through memory.  Actually, that code is ripe for
> > deduplication, and amazingly it doesn't conflict with Colton's series.  Patches
> > incoming...
> >
> 
> RISC-V uses sbi_ecall() for their implementation of ucall(). CC'ing Anup
> for confirmation, but if I understand the SBI spec correctly, then inputs
> are limited to registers a0-a5.

Ah, never mind... I see SBI is limited to 6 registers, but of course it
only needs one register to pass the uc address... We can make
UCALL_MAX_ARGS whatever we want.

Thanks,
drew

Re: [PATCH 2/4] KVM: selftests: Increase UCALL_MAX_ARGS to 7

[Andrew Jones]

On Wed, Jun 15, 2022 at 07:31:14PM +0000, Colton Lewis wrote:
> Increase UCALL_MAX_ARGS to 7 to allow GUEST_ASSERT_4 to pass 3 builtin
> ucall arguments specified in guest_assert_builtin_args plus 4
> user-specified arguments.
> 
> Signed-off-by: Colton Lewis <coltonlewis@google.com>
> ---
>  tools/testing/selftests/kvm/include/ucall_common.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/kvm/include/ucall_common.h b/tools/testing/selftests/kvm/include/ucall_common.h
> index dbe872870b83..568c562f14cd 100644
> --- a/tools/testing/selftests/kvm/include/ucall_common.h
> +++ b/tools/testing/selftests/kvm/include/ucall_common.h
> @@ -16,7 +16,7 @@ enum {
>  	UCALL_UNHANDLED,
>  };
>  
> -#define UCALL_MAX_ARGS 6
> +#define UCALL_MAX_ARGS 7
>  
>  struct ucall {
>  	uint64_t cmd;
> -- 
> 2.36.1.476.g0c4daa206d-goog
>

Reviewed-by: Andrew Jones <drjones@redhat.com>

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH 2/4] KVM: selftests: Increase UCALL_MAX_ARGS to 7

[Andrew Jones]

On Wed, Jun 15, 2022 at 07:31:14PM +0000, Colton Lewis wrote:
> Increase UCALL_MAX_ARGS to 7 to allow GUEST_ASSERT_4 to pass 3 builtin
> ucall arguments specified in guest_assert_builtin_args plus 4
> user-specified arguments.
> 
> Signed-off-by: Colton Lewis <coltonlewis@google.com>
> ---
>  tools/testing/selftests/kvm/include/ucall_common.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/kvm/include/ucall_common.h b/tools/testing/selftests/kvm/include/ucall_common.h
> index dbe872870b83..568c562f14cd 100644
> --- a/tools/testing/selftests/kvm/include/ucall_common.h
> +++ b/tools/testing/selftests/kvm/include/ucall_common.h
> @@ -16,7 +16,7 @@ enum {
>  	UCALL_UNHANDLED,
>  };
>  
> -#define UCALL_MAX_ARGS 6
> +#define UCALL_MAX_ARGS 7
>  
>  struct ucall {
>  	uint64_t cmd;
> -- 
> 2.36.1.476.g0c4daa206d-goog
>

Reviewed-by: Andrew Jones <drjones@redhat.com>

[PATCH 3/4] KVM: selftests: Write REPORT_GUEST_ASSERT macros to pair with GUEST_ASSERT

[Colton Lewis]

Write REPORT_GUEST_ASSERT macros to pair with GUEST_ASSERT to abstract
and make consistent all guest assertion reporting. Every report
includes an explanatory string, a filename, and a line number.

Signed-off-by: Colton Lewis <coltonlewis@google.com>
---
 .../selftests/kvm/include/ucall_common.h      | 42 +++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/tools/testing/selftests/kvm/include/ucall_common.h b/tools/testing/selftests/kvm/include/ucall_common.h
index 568c562f14cd..e8af3b4fef6d 100644
--- a/tools/testing/selftests/kvm/include/ucall_common.h
+++ b/tools/testing/selftests/kvm/include/ucall_common.h
@@ -6,6 +6,7 @@
  */
 #ifndef SELFTEST_KVM_UCALL_COMMON_H
 #define SELFTEST_KVM_UCALL_COMMON_H
+#include "test_util.h"
 
 /* Common ucalls */
 enum {
@@ -64,4 +65,45 @@ enum guest_assert_builtin_args {
 
 #define GUEST_ASSERT_EQ(a, b) __GUEST_ASSERT((a) == (b), #a " == " #b, 2, a, b)
 
+#define __REPORT_GUEST_ASSERT(_ucall, fmt, _args...)			\
+	TEST_FAIL("%s at %s:%ld\n" fmt,					\
+		  (const char *)(_ucall).args[GUEST_ERROR_STRING],	\
+		  (const char *)(_ucall).args[GUEST_FILE],		\
+		  (_ucall).args[GUEST_LINE],				\
+		  ##_args)
+
+#define GUEST_ASSERT_ARG(ucall, i) ((ucall).args[GUEST_ASSERT_BUILTIN_NARGS + i])
+
+#define REPORT_GUEST_ASSERT(ucall)		\
+	__REPORT_GUEST_ASSERT((ucall), "")
+
+#define REPORT_GUEST_ASSERT_1(ucall, fmt)			\
+	__REPORT_GUEST_ASSERT((ucall),				\
+			      fmt,				\
+			      GUEST_ASSERT_ARG((ucall), 0))
+
+#define REPORT_GUEST_ASSERT_2(ucall, fmt)			\
+	__REPORT_GUEST_ASSERT((ucall),				\
+			      fmt,				\
+			      GUEST_ASSERT_ARG((ucall), 0),	\
+			      GUEST_ASSERT_ARG((ucall), 1))
+
+#define REPORT_GUEST_ASSERT_3(ucall, fmt)			\
+	__REPORT_GUEST_ASSERT((ucall),				\
+			      fmt,				\
+			      GUEST_ASSERT_ARG((ucall), 0),	\
+			      GUEST_ASSERT_ARG((ucall), 1),	\
+			      GUEST_ASSERT_ARG((ucall), 2))
+
+#define REPORT_GUEST_ASSERT_4(ucall, fmt)			\
+	__REPORT_GUEST_ASSERT((ucall),				\
+			      fmt,				\
+			      GUEST_ASSERT_ARG((ucall), 0),	\
+			      GUEST_ASSERT_ARG((ucall), 1),	\
+			      GUEST_ASSERT_ARG((ucall), 2),	\
+			      GUEST_ASSERT_ARG((ucall), 3))
+
+#define REPORT_GUEST_ASSERT_N(ucall, fmt, args...)	\
+	__REPORT_GUEST_ASSERT((ucall), fmt, ##args)
+
 #endif /* SELFTEST_KVM_UCALL_COMMON_H */
-- 
2.36.1.476.g0c4daa206d-goog

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH 3/4] KVM: selftests: Write REPORT_GUEST_ASSERT macros to pair with GUEST_ASSERT

[Colton Lewis]

Write REPORT_GUEST_ASSERT macros to pair with GUEST_ASSERT to abstract
and make consistent all guest assertion reporting. Every report
includes an explanatory string, a filename, and a line number.

Signed-off-by: Colton Lewis <coltonlewis@google.com>
---
 .../selftests/kvm/include/ucall_common.h      | 42 +++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/tools/testing/selftests/kvm/include/ucall_common.h b/tools/testing/selftests/kvm/include/ucall_common.h
index 568c562f14cd..e8af3b4fef6d 100644
--- a/tools/testing/selftests/kvm/include/ucall_common.h
+++ b/tools/testing/selftests/kvm/include/ucall_common.h
@@ -6,6 +6,7 @@
  */
 #ifndef SELFTEST_KVM_UCALL_COMMON_H
 #define SELFTEST_KVM_UCALL_COMMON_H
+#include "test_util.h"
 
 /* Common ucalls */
 enum {
@@ -64,4 +65,45 @@ enum guest_assert_builtin_args {
 
 #define GUEST_ASSERT_EQ(a, b) __GUEST_ASSERT((a) == (b), #a " == " #b, 2, a, b)
 
+#define __REPORT_GUEST_ASSERT(_ucall, fmt, _args...)			\
+	TEST_FAIL("%s at %s:%ld\n" fmt,					\
+		  (const char *)(_ucall).args[GUEST_ERROR_STRING],	\
+		  (const char *)(_ucall).args[GUEST_FILE],		\
+		  (_ucall).args[GUEST_LINE],				\
+		  ##_args)
+
+#define GUEST_ASSERT_ARG(ucall, i) ((ucall).args[GUEST_ASSERT_BUILTIN_NARGS + i])
+
+#define REPORT_GUEST_ASSERT(ucall)		\
+	__REPORT_GUEST_ASSERT((ucall), "")
+
+#define REPORT_GUEST_ASSERT_1(ucall, fmt)			\
+	__REPORT_GUEST_ASSERT((ucall),				\
+			      fmt,				\
+			      GUEST_ASSERT_ARG((ucall), 0))
+
+#define REPORT_GUEST_ASSERT_2(ucall, fmt)			\
+	__REPORT_GUEST_ASSERT((ucall),				\
+			      fmt,				\
+			      GUEST_ASSERT_ARG((ucall), 0),	\
+			      GUEST_ASSERT_ARG((ucall), 1))
+
+#define REPORT_GUEST_ASSERT_3(ucall, fmt)			\
+	__REPORT_GUEST_ASSERT((ucall),				\
+			      fmt,				\
+			      GUEST_ASSERT_ARG((ucall), 0),	\
+			      GUEST_ASSERT_ARG((ucall), 1),	\
+			      GUEST_ASSERT_ARG((ucall), 2))
+
+#define REPORT_GUEST_ASSERT_4(ucall, fmt)			\
+	__REPORT_GUEST_ASSERT((ucall),				\
+			      fmt,				\
+			      GUEST_ASSERT_ARG((ucall), 0),	\
+			      GUEST_ASSERT_ARG((ucall), 1),	\
+			      GUEST_ASSERT_ARG((ucall), 2),	\
+			      GUEST_ASSERT_ARG((ucall), 3))
+
+#define REPORT_GUEST_ASSERT_N(ucall, fmt, args...)	\
+	__REPORT_GUEST_ASSERT((ucall), fmt, ##args)
+
 #endif /* SELFTEST_KVM_UCALL_COMMON_H */
-- 
2.36.1.476.g0c4daa206d-goog

Re: [PATCH 3/4] KVM: selftests: Write REPORT_GUEST_ASSERT macros to pair with GUEST_ASSERT

[Andrew Jones]

On Wed, Jun 15, 2022 at 07:31:15PM +0000, Colton Lewis wrote:
> Write REPORT_GUEST_ASSERT macros to pair with GUEST_ASSERT to abstract
> and make consistent all guest assertion reporting. Every report
> includes an explanatory string, a filename, and a line number.
> 
> Signed-off-by: Colton Lewis <coltonlewis@google.com>
> ---
>  .../selftests/kvm/include/ucall_common.h      | 42 +++++++++++++++++++
>  1 file changed, 42 insertions(+)
> 
> diff --git a/tools/testing/selftests/kvm/include/ucall_common.h b/tools/testing/selftests/kvm/include/ucall_common.h
> index 568c562f14cd..e8af3b4fef6d 100644
> --- a/tools/testing/selftests/kvm/include/ucall_common.h
> +++ b/tools/testing/selftests/kvm/include/ucall_common.h
> @@ -6,6 +6,7 @@
>   */
>  #ifndef SELFTEST_KVM_UCALL_COMMON_H
>  #define SELFTEST_KVM_UCALL_COMMON_H
> +#include "test_util.h"
>  
>  /* Common ucalls */
>  enum {
> @@ -64,4 +65,45 @@ enum guest_assert_builtin_args {
>  
>  #define GUEST_ASSERT_EQ(a, b) __GUEST_ASSERT((a) == (b), #a " == " #b, 2, a, b)
>  
> +#define __REPORT_GUEST_ASSERT(_ucall, fmt, _args...)			\
> +	TEST_FAIL("%s at %s:%ld\n" fmt,					\
> +		  (const char *)(_ucall).args[GUEST_ERROR_STRING],	\
> +		  (const char *)(_ucall).args[GUEST_FILE],		\
> +		  (_ucall).args[GUEST_LINE],				\
> +		  ##_args)
> +
> +#define GUEST_ASSERT_ARG(ucall, i) ((ucall).args[GUEST_ASSERT_BUILTIN_NARGS + i])
> +
> +#define REPORT_GUEST_ASSERT(ucall)		\
> +	__REPORT_GUEST_ASSERT((ucall), "")
> +
> +#define REPORT_GUEST_ASSERT_1(ucall, fmt)			\
> +	__REPORT_GUEST_ASSERT((ucall),				\
> +			      fmt,				\
> +			      GUEST_ASSERT_ARG((ucall), 0))
> +
> +#define REPORT_GUEST_ASSERT_2(ucall, fmt)			\
> +	__REPORT_GUEST_ASSERT((ucall),				\
> +			      fmt,				\
> +			      GUEST_ASSERT_ARG((ucall), 0),	\
> +			      GUEST_ASSERT_ARG((ucall), 1))
> +
> +#define REPORT_GUEST_ASSERT_3(ucall, fmt)			\
> +	__REPORT_GUEST_ASSERT((ucall),				\
> +			      fmt,				\
> +			      GUEST_ASSERT_ARG((ucall), 0),	\
> +			      GUEST_ASSERT_ARG((ucall), 1),	\
> +			      GUEST_ASSERT_ARG((ucall), 2))
> +
> +#define REPORT_GUEST_ASSERT_4(ucall, fmt)			\
> +	__REPORT_GUEST_ASSERT((ucall),				\
> +			      fmt,				\
> +			      GUEST_ASSERT_ARG((ucall), 0),	\
> +			      GUEST_ASSERT_ARG((ucall), 1),	\
> +			      GUEST_ASSERT_ARG((ucall), 2),	\
> +			      GUEST_ASSERT_ARG((ucall), 3))
> +
> +#define REPORT_GUEST_ASSERT_N(ucall, fmt, args...)	\
> +	__REPORT_GUEST_ASSERT((ucall), fmt, ##args)
> +
>  #endif /* SELFTEST_KVM_UCALL_COMMON_H */
> -- 
> 2.36.1.476.g0c4daa206d-goog
>

nit: All the ()'s around ucall when it's between ( and , are unnecessary.

Otherwise,

Reviewed-by: Andrew Jones <drjones@redhat.com>

Thanks,
drew

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH 3/4] KVM: selftests: Write REPORT_GUEST_ASSERT macros to pair with GUEST_ASSERT

[Andrew Jones]

On Wed, Jun 15, 2022 at 07:31:15PM +0000, Colton Lewis wrote:
> Write REPORT_GUEST_ASSERT macros to pair with GUEST_ASSERT to abstract
> and make consistent all guest assertion reporting. Every report
> includes an explanatory string, a filename, and a line number.
> 
> Signed-off-by: Colton Lewis <coltonlewis@google.com>
> ---
>  .../selftests/kvm/include/ucall_common.h      | 42 +++++++++++++++++++
>  1 file changed, 42 insertions(+)
> 
> diff --git a/tools/testing/selftests/kvm/include/ucall_common.h b/tools/testing/selftests/kvm/include/ucall_common.h
> index 568c562f14cd..e8af3b4fef6d 100644
> --- a/tools/testing/selftests/kvm/include/ucall_common.h
> +++ b/tools/testing/selftests/kvm/include/ucall_common.h
> @@ -6,6 +6,7 @@
>   */
>  #ifndef SELFTEST_KVM_UCALL_COMMON_H
>  #define SELFTEST_KVM_UCALL_COMMON_H
> +#include "test_util.h"
>  
>  /* Common ucalls */
>  enum {
> @@ -64,4 +65,45 @@ enum guest_assert_builtin_args {
>  
>  #define GUEST_ASSERT_EQ(a, b) __GUEST_ASSERT((a) == (b), #a " == " #b, 2, a, b)
>  
> +#define __REPORT_GUEST_ASSERT(_ucall, fmt, _args...)			\
> +	TEST_FAIL("%s at %s:%ld\n" fmt,					\
> +		  (const char *)(_ucall).args[GUEST_ERROR_STRING],	\
> +		  (const char *)(_ucall).args[GUEST_FILE],		\
> +		  (_ucall).args[GUEST_LINE],				\
> +		  ##_args)
> +
> +#define GUEST_ASSERT_ARG(ucall, i) ((ucall).args[GUEST_ASSERT_BUILTIN_NARGS + i])
> +
> +#define REPORT_GUEST_ASSERT(ucall)		\
> +	__REPORT_GUEST_ASSERT((ucall), "")
> +
> +#define REPORT_GUEST_ASSERT_1(ucall, fmt)			\
> +	__REPORT_GUEST_ASSERT((ucall),				\
> +			      fmt,				\
> +			      GUEST_ASSERT_ARG((ucall), 0))
> +
> +#define REPORT_GUEST_ASSERT_2(ucall, fmt)			\
> +	__REPORT_GUEST_ASSERT((ucall),				\
> +			      fmt,				\
> +			      GUEST_ASSERT_ARG((ucall), 0),	\
> +			      GUEST_ASSERT_ARG((ucall), 1))
> +
> +#define REPORT_GUEST_ASSERT_3(ucall, fmt)			\
> +	__REPORT_GUEST_ASSERT((ucall),				\
> +			      fmt,				\
> +			      GUEST_ASSERT_ARG((ucall), 0),	\
> +			      GUEST_ASSERT_ARG((ucall), 1),	\
> +			      GUEST_ASSERT_ARG((ucall), 2))
> +
> +#define REPORT_GUEST_ASSERT_4(ucall, fmt)			\
> +	__REPORT_GUEST_ASSERT((ucall),				\
> +			      fmt,				\
> +			      GUEST_ASSERT_ARG((ucall), 0),	\
> +			      GUEST_ASSERT_ARG((ucall), 1),	\
> +			      GUEST_ASSERT_ARG((ucall), 2),	\
> +			      GUEST_ASSERT_ARG((ucall), 3))
> +
> +#define REPORT_GUEST_ASSERT_N(ucall, fmt, args...)	\
> +	__REPORT_GUEST_ASSERT((ucall), fmt, ##args)
> +
>  #endif /* SELFTEST_KVM_UCALL_COMMON_H */
> -- 
> 2.36.1.476.g0c4daa206d-goog
>

nit: All the ()'s around ucall when it's between ( and , are unnecessary.

Otherwise,

Reviewed-by: Andrew Jones <drjones@redhat.com>

Thanks,
drew

[PATCH 4/4] KVM: selftests: Fix filename reporting in guest asserts

[Colton Lewis]

Fix filename reporting in guest asserts by ensuring the GUEST_ASSERT
macro records __FILE__ and substituting REPORT_GUEST_ASSERT for many
repetitive calls to TEST_FAIL.

Previously filename was reported by using __FILE__ directly in the
selftest, wrongly assuming it would always be the same as where the
assertion failed.y

Signed-off-by: Colton Lewis <coltonlewis@google.com>
Reported-by: Ricardo Koller <ricarkol@google.com>
Fixes: 4e18bccc2e5544f0be28fc1c4e6be47a469d6c60
---
 tools/testing/selftests/kvm/aarch64/arch_timer.c   | 12 ++++++++----
 .../selftests/kvm/aarch64/debug-exceptions.c       |  4 +---
 tools/testing/selftests/kvm/aarch64/vgic_irq.c     |  4 +---
 tools/testing/selftests/kvm/include/ucall_common.h | 14 ++++++++------
 tools/testing/selftests/kvm/memslot_perf_test.c    |  4 +---
 tools/testing/selftests/kvm/steal_time.c           |  3 +--
 .../selftests/kvm/system_counter_offset_test.c     |  3 +--
 tools/testing/selftests/kvm/x86_64/amx_test.c      |  3 +--
 tools/testing/selftests/kvm/x86_64/cpuid_test.c    |  3 +--
 .../selftests/kvm/x86_64/cr4_cpuid_sync_test.c     |  2 +-
 .../selftests/kvm/x86_64/emulator_error_test.c     |  3 +--
 tools/testing/selftests/kvm/x86_64/evmcs_test.c    |  3 +--
 tools/testing/selftests/kvm/x86_64/hyperv_clock.c  |  3 +--
 .../testing/selftests/kvm/x86_64/hyperv_features.c |  6 ++----
 .../testing/selftests/kvm/x86_64/hyperv_svm_test.c |  3 +--
 .../testing/selftests/kvm/x86_64/kvm_clock_test.c  |  3 +--
 tools/testing/selftests/kvm/x86_64/kvm_pv_test.c   |  3 +--
 .../testing/selftests/kvm/x86_64/set_boot_cpu_id.c |  4 +---
 tools/testing/selftests/kvm/x86_64/state_test.c    |  3 +--
 .../selftests/kvm/x86_64/svm_int_ctl_test.c        |  2 +-
 .../testing/selftests/kvm/x86_64/svm_vmcall_test.c |  2 +-
 tools/testing/selftests/kvm/x86_64/tsc_msrs_test.c |  4 +---
 .../selftests/kvm/x86_64/userspace_io_test.c       |  4 +---
 .../selftests/kvm/x86_64/userspace_msr_exit_test.c |  5 ++---
 .../selftests/kvm/x86_64/vmx_apic_access_test.c    |  3 +--
 .../kvm/x86_64/vmx_close_while_nested_test.c       |  2 +-
 .../selftests/kvm/x86_64/vmx_dirty_log_test.c      |  3 +--
 .../kvm/x86_64/vmx_invalid_nested_guest_state.c    |  2 +-
 .../kvm/x86_64/vmx_nested_tsc_scaling_test.c       |  2 +-
 .../kvm/x86_64/vmx_preemption_timer_test.c         |  3 +--
 .../selftests/kvm/x86_64/vmx_tsc_adjust_test.c     |  2 +-
 .../testing/selftests/kvm/x86_64/xen_shinfo_test.c |  2 +-
 .../testing/selftests/kvm/x86_64/xen_vmcall_test.c |  2 +-
 33 files changed, 49 insertions(+), 72 deletions(-)

diff --git a/tools/testing/selftests/kvm/aarch64/arch_timer.c b/tools/testing/selftests/kvm/aarch64/arch_timer.c
index f68019be67c0..9d7bda70c20a 100644
--- a/tools/testing/selftests/kvm/aarch64/arch_timer.c
+++ b/tools/testing/selftests/kvm/aarch64/arch_timer.c
@@ -231,10 +231,14 @@ static void *test_vcpu_run(void *arg)
 		break;
 	case UCALL_ABORT:
 		sync_global_from_guest(vm, *shared_data);
-		TEST_FAIL("%s at %s:%ld\n\tvalues: %lu, %lu; %lu, vcpu: %u; stage: %u; iter: %u",
-			(const char *)uc.args[0], __FILE__, uc.args[1],
-			uc.args[2], uc.args[3], uc.args[4], vcpu_idx,
-			shared_data->guest_stage, shared_data->nr_iter);
+		REPORT_GUEST_ASSERT_N(uc,
+				      "values: %lu, %lu; %lu, vcpu %u; stage; %u; iter: %u",
+				      GUEST_ASSERT_ARG(uc, 0),
+				      GUEST_ASSERT_ARG(uc, 1),
+				      GUEST_ASSERT_ARG(uc, 2),
+				      vcpu_idx,
+				      shared_data->guest_stage,
+				      shared_data->nr_iter);
 		break;
 	default:
 		TEST_FAIL("Unexpected guest exit\n");
diff --git a/tools/testing/selftests/kvm/aarch64/debug-exceptions.c b/tools/testing/selftests/kvm/aarch64/debug-exceptions.c
index b8072b40ccc8..2ee35cf9801e 100644
--- a/tools/testing/selftests/kvm/aarch64/debug-exceptions.c
+++ b/tools/testing/selftests/kvm/aarch64/debug-exceptions.c
@@ -283,9 +283,7 @@ int main(int argc, char *argv[])
 				stage, (ulong)uc.args[1]);
 			break;
 		case UCALL_ABORT:
-			TEST_FAIL("%s at %s:%ld\n\tvalues: %#lx, %#lx",
-				(const char *)uc.args[0],
-				__FILE__, uc.args[1], uc.args[2], uc.args[3]);
+			REPORT_GUEST_ASSERT_2(uc, "values: %#lx, %#lx");
 			break;
 		case UCALL_DONE:
 			goto done;
diff --git a/tools/testing/selftests/kvm/aarch64/vgic_irq.c b/tools/testing/selftests/kvm/aarch64/vgic_irq.c
index 046ba4fde648..17417220a083 100644
--- a/tools/testing/selftests/kvm/aarch64/vgic_irq.c
+++ b/tools/testing/selftests/kvm/aarch64/vgic_irq.c
@@ -782,9 +782,7 @@ static void test_vgic(uint32_t nr_irqs, bool level_sensitive, bool eoi_split)
 			run_guest_cmd(vcpu, gic_fd, &inject_args, &args);
 			break;
 		case UCALL_ABORT:
-			TEST_FAIL("%s at %s:%ld\n\tvalues: %#lx, %#lx",
-					(const char *)uc.args[0],
-					__FILE__, uc.args[1], uc.args[2], uc.args[3]);
+			REPORT_GUEST_ASSERT_2(uc, "values: %#lx, %#lx");
 			break;
 		case UCALL_DONE:
 			goto done;
diff --git a/tools/testing/selftests/kvm/include/ucall_common.h b/tools/testing/selftests/kvm/include/ucall_common.h
index e8af3b4fef6d..e1cc72077eab 100644
--- a/tools/testing/selftests/kvm/include/ucall_common.h
+++ b/tools/testing/selftests/kvm/include/ucall_common.h
@@ -41,12 +41,14 @@ enum guest_assert_builtin_args {
 	GUEST_ASSERT_BUILTIN_NARGS
 };
 
-#define __GUEST_ASSERT(_condition, _condstr, _nargs, _args...) do {    \
-	if (!(_condition))                                              \
-		ucall(UCALL_ABORT, 2 + _nargs,                          \
-			"Failed guest assert: "                         \
-			_condstr, __LINE__, _args);                     \
-} while (0)
+#define __GUEST_ASSERT(_condition, _condstr, _nargs, _args...) do {	\
+		if (!(_condition))					\
+			ucall(UCALL_ABORT, GUEST_ASSERT_BUILTIN_NARGS + _nargs,	\
+			      "Failed guest assert: " _condstr,		\
+			      __FILE__,					\
+			      __LINE__,					\
+			      ##_args);					\
+	} while (0)
 
 #define GUEST_ASSERT(_condition) \
 	__GUEST_ASSERT(_condition, #_condition, 0, 0)
diff --git a/tools/testing/selftests/kvm/memslot_perf_test.c b/tools/testing/selftests/kvm/memslot_perf_test.c
index 5f98489e4f4d..44995446d942 100644
--- a/tools/testing/selftests/kvm/memslot_perf_test.c
+++ b/tools/testing/selftests/kvm/memslot_perf_test.c
@@ -162,9 +162,7 @@ static void *vcpu_worker(void *__data)
 				goto done;
 			break;
 		case UCALL_ABORT:
-			TEST_FAIL("%s at %s:%ld, val = %lu",
-					(const char *)uc.args[0],
-					__FILE__, uc.args[1], uc.args[2]);
+			REPORT_GUEST_ASSERT_1(uc, "val = %lu");
 			break;
 		case UCALL_DONE:
 			goto done;
diff --git a/tools/testing/selftests/kvm/steal_time.c b/tools/testing/selftests/kvm/steal_time.c
index d122f1e05cdd..9866a71463d7 100644
--- a/tools/testing/selftests/kvm/steal_time.c
+++ b/tools/testing/selftests/kvm/steal_time.c
@@ -234,8 +234,7 @@ static void run_vcpu(struct kvm_vcpu *vcpu)
 	case UCALL_DONE:
 		break;
 	case UCALL_ABORT:
-		TEST_ASSERT(false, "%s at %s:%ld", (const char *)uc.args[0],
-			    __FILE__, uc.args[1]);
+		REPORT_GUEST_ASSERT(uc);
 	default:
 		TEST_ASSERT(false, "Unexpected exit: %s",
 			    exit_reason_str(vcpu->run->exit_reason));
diff --git a/tools/testing/selftests/kvm/system_counter_offset_test.c b/tools/testing/selftests/kvm/system_counter_offset_test.c
index 862a8e93e070..1c274933912b 100644
--- a/tools/testing/selftests/kvm/system_counter_offset_test.c
+++ b/tools/testing/selftests/kvm/system_counter_offset_test.c
@@ -83,8 +83,7 @@ static void handle_sync(struct ucall *uc, uint64_t start, uint64_t end)
 
 static void handle_abort(struct ucall *uc)
 {
-	TEST_FAIL("%s at %s:%ld", (const char *)uc->args[0],
-		  __FILE__, uc->args[1]);
+	REPORT_GUEST_ASSERT(*uc);
 }
 
 static void enter_guest(struct kvm_vcpu *vcpu)
diff --git a/tools/testing/selftests/kvm/x86_64/amx_test.c b/tools/testing/selftests/kvm/x86_64/amx_test.c
index dab4ca16a2df..b71763b11b78 100644
--- a/tools/testing/selftests/kvm/x86_64/amx_test.c
+++ b/tools/testing/selftests/kvm/x86_64/amx_test.c
@@ -373,8 +373,7 @@ int main(int argc, char *argv[])
 
 		switch (get_ucall(vcpu, &uc)) {
 		case UCALL_ABORT:
-			TEST_FAIL("%s at %s:%ld", (const char *)uc.args[0],
-				  __FILE__, uc.args[1]);
+			REPORT_GUEST_ASSERT(uc);
 			/* NOT REACHED */
 		case UCALL_SYNC:
 			switch (uc.args[1]) {
diff --git a/tools/testing/selftests/kvm/x86_64/cpuid_test.c b/tools/testing/selftests/kvm/x86_64/cpuid_test.c
index 4aa784932597..3767a0cc694b 100644
--- a/tools/testing/selftests/kvm/x86_64/cpuid_test.c
+++ b/tools/testing/selftests/kvm/x86_64/cpuid_test.c
@@ -132,8 +132,7 @@ static void run_vcpu(struct kvm_vcpu *vcpu, int stage)
 	case UCALL_DONE:
 		return;
 	case UCALL_ABORT:
-		TEST_ASSERT(false, "%s at %s:%ld\n\tvalues: %#lx, %#lx", (const char *)uc.args[0],
-			    __FILE__, uc.args[1], uc.args[2], uc.args[3]);
+		REPORT_GUEST_ASSERT_2(uc, "values: %#lx, %#lx");
 	default:
 		TEST_ASSERT(false, "Unexpected exit: %s",
 			    exit_reason_str(vcpu->run->exit_reason));
diff --git a/tools/testing/selftests/kvm/x86_64/cr4_cpuid_sync_test.c b/tools/testing/selftests/kvm/x86_64/cr4_cpuid_sync_test.c
index a80940ac420f..56d8ab92eed4 100644
--- a/tools/testing/selftests/kvm/x86_64/cr4_cpuid_sync_test.c
+++ b/tools/testing/selftests/kvm/x86_64/cr4_cpuid_sync_test.c
@@ -94,7 +94,7 @@ int main(int argc, char *argv[])
 			vcpu_sregs_set(vcpu, &sregs);
 			break;
 		case UCALL_ABORT:
-			TEST_FAIL("Guest CR4 bit (OSXSAVE) unsynchronized with CPUID bit.");
+			REPORT_GUEST_ASSERT(uc);
 			break;
 		case UCALL_DONE:
 			goto done;
diff --git a/tools/testing/selftests/kvm/x86_64/emulator_error_test.c b/tools/testing/selftests/kvm/x86_64/emulator_error_test.c
index bfff2d271c48..3aa3d17f230f 100644
--- a/tools/testing/selftests/kvm/x86_64/emulator_error_test.c
+++ b/tools/testing/selftests/kvm/x86_64/emulator_error_test.c
@@ -92,8 +92,7 @@ static void process_exit_on_emulation_error(struct kvm_vcpu *vcpu)
 
 static void do_guest_assert(struct ucall *uc)
 {
-	TEST_FAIL("%s at %s:%ld", (const char *)uc->args[0], __FILE__,
-		  uc->args[1]);
+	REPORT_GUEST_ASSERT(*uc);
 }
 
 static void check_for_guest_assert(struct kvm_vcpu *vcpu)
diff --git a/tools/testing/selftests/kvm/x86_64/evmcs_test.c b/tools/testing/selftests/kvm/x86_64/evmcs_test.c
index 8dda527cc080..aacad86d90e1 100644
--- a/tools/testing/selftests/kvm/x86_64/evmcs_test.c
+++ b/tools/testing/selftests/kvm/x86_64/evmcs_test.c
@@ -236,8 +236,7 @@ int main(int argc, char *argv[])
 
 		switch (get_ucall(vcpu, &uc)) {
 		case UCALL_ABORT:
-			TEST_FAIL("%s at %s:%ld", (const char *)uc.args[0],
-		      		  __FILE__, uc.args[1]);
+			REPORT_GUEST_ASSERT(uc);
 			/* NOT REACHED */
 		case UCALL_SYNC:
 			break;
diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_clock.c b/tools/testing/selftests/kvm/x86_64/hyperv_clock.c
index f7a9e29ff0c7..d576bc8ce823 100644
--- a/tools/testing/selftests/kvm/x86_64/hyperv_clock.c
+++ b/tools/testing/selftests/kvm/x86_64/hyperv_clock.c
@@ -234,8 +234,7 @@ int main(void)
 
 		switch (get_ucall(vcpu, &uc)) {
 		case UCALL_ABORT:
-			TEST_FAIL("%s at %s:%ld", (const char *)uc.args[0],
-				  __FILE__, uc.args[1]);
+			REPORT_GUEST_ASSERT(uc);
 			/* NOT REACHED */
 		case UCALL_SYNC:
 			break;
diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_features.c b/tools/testing/selftests/kvm/x86_64/hyperv_features.c
index d5f37495ade8..0eca72333d66 100644
--- a/tools/testing/selftests/kvm/x86_64/hyperv_features.c
+++ b/tools/testing/selftests/kvm/x86_64/hyperv_features.c
@@ -483,8 +483,7 @@ static void guest_test_msrs_access(void)
 				    uc.args[1]);
 			break;
 		case UCALL_ABORT:
-			TEST_FAIL("%s at %s:%ld", (const char *)uc.args[0],
-				  __FILE__, uc.args[1]);
+			REPORT_GUEST_ASSERT(uc);
 			return;
 		case UCALL_DONE:
 			return;
@@ -653,8 +652,7 @@ static void guest_test_hcalls_access(void)
 				    uc.args[1]);
 			break;
 		case UCALL_ABORT:
-			TEST_FAIL("%s at %s:%ld", (const char *)uc.args[0],
-				  __FILE__, uc.args[1]);
+			REPORT_GUEST_ASSERT(uc);
 			return;
 		case UCALL_DONE:
 			return;
diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_svm_test.c b/tools/testing/selftests/kvm/x86_64/hyperv_svm_test.c
index c5cd9835dbd6..b7dc243ab8d5 100644
--- a/tools/testing/selftests/kvm/x86_64/hyperv_svm_test.c
+++ b/tools/testing/selftests/kvm/x86_64/hyperv_svm_test.c
@@ -145,8 +145,7 @@ int main(int argc, char *argv[])
 
 		switch (get_ucall(vcpu, &uc)) {
 		case UCALL_ABORT:
-			TEST_FAIL("%s at %s:%ld", (const char *)uc.args[0],
-				  __FILE__, uc.args[1]);
+			REPORT_GUEST_ASSERT(uc);
 			/* NOT REACHED */
 		case UCALL_SYNC:
 			break;
diff --git a/tools/testing/selftests/kvm/x86_64/kvm_clock_test.c b/tools/testing/selftests/kvm/x86_64/kvm_clock_test.c
index 138455575a11..813ce282cf56 100644
--- a/tools/testing/selftests/kvm/x86_64/kvm_clock_test.c
+++ b/tools/testing/selftests/kvm/x86_64/kvm_clock_test.c
@@ -71,8 +71,7 @@ static void handle_sync(struct ucall *uc, struct kvm_clock_data *start,
 
 static void handle_abort(struct ucall *uc)
 {
-	TEST_FAIL("%s at %s:%ld", (const char *)uc->args[0],
-		  __FILE__, uc->args[1]);
+	REPORT_GUEST_ASSERT(*uc);
 }
 
 static void setup_clock(struct kvm_vm *vm, struct test_case *test_case)
diff --git a/tools/testing/selftests/kvm/x86_64/kvm_pv_test.c b/tools/testing/selftests/kvm/x86_64/kvm_pv_test.c
index 5901ccec7079..fc9bd71e08b0 100644
--- a/tools/testing/selftests/kvm/x86_64/kvm_pv_test.c
+++ b/tools/testing/selftests/kvm/x86_64/kvm_pv_test.c
@@ -167,8 +167,7 @@ static void pr_hcall(struct ucall *uc)
 
 static void handle_abort(struct ucall *uc)
 {
-	TEST_FAIL("%s at %s:%ld", (const char *)uc->args[0],
-		  __FILE__, uc->args[1]);
+	REPORT_GUEST_ASSERT(*uc);
 }
 
 static void enter_guest(struct kvm_vcpu *vcpu)
diff --git a/tools/testing/selftests/kvm/x86_64/set_boot_cpu_id.c b/tools/testing/selftests/kvm/x86_64/set_boot_cpu_id.c
index 7ef713fdd0a5..b25d7556b638 100644
--- a/tools/testing/selftests/kvm/x86_64/set_boot_cpu_id.c
+++ b/tools/testing/selftests/kvm/x86_64/set_boot_cpu_id.c
@@ -65,9 +65,7 @@ static void run_vcpu(struct kvm_vcpu *vcpu)
 					stage);
 			break;
 		case UCALL_ABORT:
-			TEST_ASSERT(false, "%s at %s:%ld\n\tvalues: %#lx, %#lx",
-						(const char *)uc.args[0], __FILE__,
-						uc.args[1], uc.args[2], uc.args[3]);
+			REPORT_GUEST_ASSERT_2(uc, "values: %#lx, %#lx");
 		default:
 			TEST_ASSERT(false, "Unexpected exit: %s",
 				    exit_reason_str(vcpu->run->exit_reason));
diff --git a/tools/testing/selftests/kvm/x86_64/state_test.c b/tools/testing/selftests/kvm/x86_64/state_test.c
index 0bcd78cf7c79..11d036e25fd2 100644
--- a/tools/testing/selftests/kvm/x86_64/state_test.c
+++ b/tools/testing/selftests/kvm/x86_64/state_test.c
@@ -190,8 +190,7 @@ int main(int argc, char *argv[])
 
 		switch (get_ucall(vcpu, &uc)) {
 		case UCALL_ABORT:
-			TEST_FAIL("%s at %s:%ld", (const char *)uc.args[0],
-			       	  __FILE__, uc.args[1]);
+			REPORT_GUEST_ASSERT(uc);
 			/* NOT REACHED */
 		case UCALL_SYNC:
 			break;
diff --git a/tools/testing/selftests/kvm/x86_64/svm_int_ctl_test.c b/tools/testing/selftests/kvm/x86_64/svm_int_ctl_test.c
index 9c68a47b69e1..d978d1697f5a 100644
--- a/tools/testing/selftests/kvm/x86_64/svm_int_ctl_test.c
+++ b/tools/testing/selftests/kvm/x86_64/svm_int_ctl_test.c
@@ -113,7 +113,7 @@ int main(int argc, char *argv[])
 
 	switch (get_ucall(vcpu, &uc)) {
 	case UCALL_ABORT:
-		TEST_FAIL("%s", (const char *)uc.args[0]);
+		REPORT_GUEST_ASSERT(uc);
 		break;
 		/* NOT REACHED */
 	case UCALL_DONE:
diff --git a/tools/testing/selftests/kvm/x86_64/svm_vmcall_test.c b/tools/testing/selftests/kvm/x86_64/svm_vmcall_test.c
index e6d7191866a5..d53b1f7abb56 100644
--- a/tools/testing/selftests/kvm/x86_64/svm_vmcall_test.c
+++ b/tools/testing/selftests/kvm/x86_64/svm_vmcall_test.c
@@ -58,7 +58,7 @@ int main(int argc, char *argv[])
 
 		switch (get_ucall(vcpu, &uc)) {
 		case UCALL_ABORT:
-			TEST_FAIL("%s", (const char *)uc.args[0]);
+			REPORT_GUEST_ASSERT(uc);
 			/* NOT REACHED */
 		case UCALL_SYNC:
 			break;
diff --git a/tools/testing/selftests/kvm/x86_64/tsc_msrs_test.c b/tools/testing/selftests/kvm/x86_64/tsc_msrs_test.c
index 3165d3f7e065..22d366c697f7 100644
--- a/tools/testing/selftests/kvm/x86_64/tsc_msrs_test.c
+++ b/tools/testing/selftests/kvm/x86_64/tsc_msrs_test.c
@@ -79,9 +79,7 @@ static void run_vcpu(struct kvm_vcpu *vcpu, int stage)
 	case UCALL_DONE:
 		return;
 	case UCALL_ABORT:
-		TEST_ASSERT(false, "%s at %s:%ld\n" \
-			    "\tvalues: %#lx, %#lx", (const char *)uc.args[0],
-			    __FILE__, uc.args[1], uc.args[2], uc.args[3]);
+		REPORT_GUEST_ASSERT_2(uc, "values: %#lx, %#lx");
 	default:
 		TEST_ASSERT(false, "Unexpected exit: %s",
 			    exit_reason_str(vcpu->run->exit_reason));
diff --git a/tools/testing/selftests/kvm/x86_64/userspace_io_test.c b/tools/testing/selftests/kvm/x86_64/userspace_io_test.c
index 7538d57a41d5..7316521428f8 100644
--- a/tools/testing/selftests/kvm/x86_64/userspace_io_test.c
+++ b/tools/testing/selftests/kvm/x86_64/userspace_io_test.c
@@ -98,9 +98,7 @@ int main(int argc, char *argv[])
 	case UCALL_DONE:
 		break;
 	case UCALL_ABORT:
-		TEST_FAIL("%s at %s:%ld : argN+1 = 0x%lx, argN+2 = 0x%lx",
-			  (const char *)uc.args[0], __FILE__, uc.args[1],
-			  uc.args[2], uc.args[3]);
+		REPORT_GUEST_ASSERT_2(uc, "argN+1 = 0x%lx, argN+2 = 0x%lx");
 	default:
 		TEST_FAIL("Unknown ucall %lu", uc.cmd);
 	}
diff --git a/tools/testing/selftests/kvm/x86_64/userspace_msr_exit_test.c b/tools/testing/selftests/kvm/x86_64/userspace_msr_exit_test.c
index f84dc37426f5..a4f06370a245 100644
--- a/tools/testing/selftests/kvm/x86_64/userspace_msr_exit_test.c
+++ b/tools/testing/selftests/kvm/x86_64/userspace_msr_exit_test.c
@@ -400,8 +400,7 @@ static void check_for_guest_assert(struct kvm_vcpu *vcpu)
 
 	if (vcpu->run->exit_reason == KVM_EXIT_IO &&
 	    get_ucall(vcpu, &uc) == UCALL_ABORT) {
-		TEST_FAIL("%s at %s:%ld",
-			  (const char *)uc.args[0], __FILE__, uc.args[1]);
+		REPORT_GUEST_ASSERT(uc);
 	}
 }
 
@@ -610,7 +609,7 @@ static int handle_ucall(struct kvm_vcpu *vcpu)
 
 	switch (get_ucall(vcpu, &uc)) {
 	case UCALL_ABORT:
-		TEST_FAIL("Guest assertion not met");
+		REPORT_GUEST_ASSERT(uc);
 		break;
 	case UCALL_SYNC:
 		vm_ioctl(vcpu->vm, KVM_X86_SET_MSR_FILTER, &no_filter_deny);
diff --git a/tools/testing/selftests/kvm/x86_64/vmx_apic_access_test.c b/tools/testing/selftests/kvm/x86_64/vmx_apic_access_test.c
index ccb05ef7234e..d3582cea1258 100644
--- a/tools/testing/selftests/kvm/x86_64/vmx_apic_access_test.c
+++ b/tools/testing/selftests/kvm/x86_64/vmx_apic_access_test.c
@@ -114,8 +114,7 @@ int main(int argc, char *argv[])
 
 		switch (get_ucall(vcpu, &uc)) {
 		case UCALL_ABORT:
-			TEST_FAIL("%s at %s:%ld", (const char *)uc.args[0],
-				  __FILE__, uc.args[1]);
+			REPORT_GUEST_ASSERT(uc);
 			/* NOT REACHED */
 		case UCALL_SYNC:
 			apic_access_addr = uc.args[1];
diff --git a/tools/testing/selftests/kvm/x86_64/vmx_close_while_nested_test.c b/tools/testing/selftests/kvm/x86_64/vmx_close_while_nested_test.c
index 40c77bb706a1..e69e8963ed08 100644
--- a/tools/testing/selftests/kvm/x86_64/vmx_close_while_nested_test.c
+++ b/tools/testing/selftests/kvm/x86_64/vmx_close_while_nested_test.c
@@ -74,7 +74,7 @@ int main(int argc, char *argv[])
 
 		switch (get_ucall(vcpu, &uc)) {
 		case UCALL_ABORT:
-			TEST_FAIL("%s", (const char *)uc.args[0]);
+			REPORT_GUEST_ASSERT(uc);
 			/* NOT REACHED */
 		default:
 			TEST_FAIL("Unknown ucall %lu", uc.cmd);
diff --git a/tools/testing/selftests/kvm/x86_64/vmx_dirty_log_test.c b/tools/testing/selftests/kvm/x86_64/vmx_dirty_log_test.c
index 215ffa0589d4..f378960299c0 100644
--- a/tools/testing/selftests/kvm/x86_64/vmx_dirty_log_test.c
+++ b/tools/testing/selftests/kvm/x86_64/vmx_dirty_log_test.c
@@ -123,8 +123,7 @@ int main(int argc, char *argv[])
 
 		switch (get_ucall(vcpu, &uc)) {
 		case UCALL_ABORT:
-			TEST_FAIL("%s at %s:%ld", (const char *)uc.args[0],
-			       	  __FILE__, uc.args[1]);
+			REPORT_GUEST_ASSERT(uc);
 			/* NOT REACHED */
 		case UCALL_SYNC:
 			/*
diff --git a/tools/testing/selftests/kvm/x86_64/vmx_invalid_nested_guest_state.c b/tools/testing/selftests/kvm/x86_64/vmx_invalid_nested_guest_state.c
index 683f4f0a1616..8c854738f2cc 100644
--- a/tools/testing/selftests/kvm/x86_64/vmx_invalid_nested_guest_state.c
+++ b/tools/testing/selftests/kvm/x86_64/vmx_invalid_nested_guest_state.c
@@ -98,7 +98,7 @@ int main(int argc, char *argv[])
 	case UCALL_DONE:
 		break;
 	case UCALL_ABORT:
-		TEST_FAIL("%s", (const char *)uc.args[0]);
+		REPORT_GUEST_ASSERT(uc);
 	default:
 		TEST_FAIL("Unexpected ucall: %lu", uc.cmd);
 	}
diff --git a/tools/testing/selftests/kvm/x86_64/vmx_nested_tsc_scaling_test.c b/tools/testing/selftests/kvm/x86_64/vmx_nested_tsc_scaling_test.c
index ff4644038c55..6bfef77b87b7 100644
--- a/tools/testing/selftests/kvm/x86_64/vmx_nested_tsc_scaling_test.c
+++ b/tools/testing/selftests/kvm/x86_64/vmx_nested_tsc_scaling_test.c
@@ -194,7 +194,7 @@ int main(int argc, char *argv[])
 
 		switch (get_ucall(vcpu, &uc)) {
 		case UCALL_ABORT:
-			TEST_FAIL("%s", (const char *) uc.args[0]);
+			REPORT_GUEST_ASSERT(uc);
 		case UCALL_SYNC:
 			switch (uc.args[0]) {
 			case USLEEP:
diff --git a/tools/testing/selftests/kvm/x86_64/vmx_preemption_timer_test.c b/tools/testing/selftests/kvm/x86_64/vmx_preemption_timer_test.c
index 99e57b0cc2c9..0a8e989d4200 100644
--- a/tools/testing/selftests/kvm/x86_64/vmx_preemption_timer_test.c
+++ b/tools/testing/selftests/kvm/x86_64/vmx_preemption_timer_test.c
@@ -189,8 +189,7 @@ int main(int argc, char *argv[])
 
 		switch (get_ucall(vcpu, &uc)) {
 		case UCALL_ABORT:
-			TEST_FAIL("%s at %s:%ld", (const char *)uc.args[0],
-				  __FILE__, uc.args[1]);
+			REPORT_GUEST_ASSERT(uc);
 			/* NOT REACHED */
 		case UCALL_SYNC:
 			break;
diff --git a/tools/testing/selftests/kvm/x86_64/vmx_tsc_adjust_test.c b/tools/testing/selftests/kvm/x86_64/vmx_tsc_adjust_test.c
index e32bfb102699..2e75eef926ca 100644
--- a/tools/testing/selftests/kvm/x86_64/vmx_tsc_adjust_test.c
+++ b/tools/testing/selftests/kvm/x86_64/vmx_tsc_adjust_test.c
@@ -147,7 +147,7 @@ int main(int argc, char *argv[])
 
 		switch (get_ucall(vcpu, &uc)) {
 		case UCALL_ABORT:
-			TEST_FAIL("%s", (const char *)uc.args[0]);
+			REPORT_GUEST_ASSERT(uc);
 			/* NOT REACHED */
 		case UCALL_SYNC:
 			report(uc.args[1]);
diff --git a/tools/testing/selftests/kvm/x86_64/xen_shinfo_test.c b/tools/testing/selftests/kvm/x86_64/xen_shinfo_test.c
index a4a78637c35a..8a5cb800f50e 100644
--- a/tools/testing/selftests/kvm/x86_64/xen_shinfo_test.c
+++ b/tools/testing/selftests/kvm/x86_64/xen_shinfo_test.c
@@ -542,7 +542,7 @@ int main(int argc, char *argv[])
 
 		switch (get_ucall(vcpu, &uc)) {
 		case UCALL_ABORT:
-			TEST_FAIL("%s", (const char *)uc.args[0]);
+			REPORT_GUEST_ASSERT(uc);
 			/* NOT REACHED */
 		case UCALL_SYNC: {
 			struct kvm_xen_vcpu_attr rst;
diff --git a/tools/testing/selftests/kvm/x86_64/xen_vmcall_test.c b/tools/testing/selftests/kvm/x86_64/xen_vmcall_test.c
index 8b76cade9bcd..88914d48c65e 100644
--- a/tools/testing/selftests/kvm/x86_64/xen_vmcall_test.c
+++ b/tools/testing/selftests/kvm/x86_64/xen_vmcall_test.c
@@ -129,7 +129,7 @@ int main(int argc, char *argv[])
 
 		switch (get_ucall(vcpu, &uc)) {
 		case UCALL_ABORT:
-			TEST_FAIL("%s", (const char *)uc.args[0]);
+			REPORT_GUEST_ASSERT(uc);
 			/* NOT REACHED */
 		case UCALL_SYNC:
 			break;
-- 
2.36.1.476.g0c4daa206d-goog

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH 4/4] KVM: selftests: Fix filename reporting in guest asserts

[Colton Lewis]

Fix filename reporting in guest asserts by ensuring the GUEST_ASSERT
macro records __FILE__ and substituting REPORT_GUEST_ASSERT for many
repetitive calls to TEST_FAIL.

Previously filename was reported by using __FILE__ directly in the
selftest, wrongly assuming it would always be the same as where the
assertion failed.y

Signed-off-by: Colton Lewis <coltonlewis@google.com>
Reported-by: Ricardo Koller <ricarkol@google.com>
Fixes: 4e18bccc2e5544f0be28fc1c4e6be47a469d6c60
---
 tools/testing/selftests/kvm/aarch64/arch_timer.c   | 12 ++++++++----
 .../selftests/kvm/aarch64/debug-exceptions.c       |  4 +---
 tools/testing/selftests/kvm/aarch64/vgic_irq.c     |  4 +---
 tools/testing/selftests/kvm/include/ucall_common.h | 14 ++++++++------
 tools/testing/selftests/kvm/memslot_perf_test.c    |  4 +---
 tools/testing/selftests/kvm/steal_time.c           |  3 +--
 .../selftests/kvm/system_counter_offset_test.c     |  3 +--
 tools/testing/selftests/kvm/x86_64/amx_test.c      |  3 +--
 tools/testing/selftests/kvm/x86_64/cpuid_test.c    |  3 +--
 .../selftests/kvm/x86_64/cr4_cpuid_sync_test.c     |  2 +-
 .../selftests/kvm/x86_64/emulator_error_test.c     |  3 +--
 tools/testing/selftests/kvm/x86_64/evmcs_test.c    |  3 +--
 tools/testing/selftests/kvm/x86_64/hyperv_clock.c  |  3 +--
 .../testing/selftests/kvm/x86_64/hyperv_features.c |  6 ++----
 .../testing/selftests/kvm/x86_64/hyperv_svm_test.c |  3 +--
 .../testing/selftests/kvm/x86_64/kvm_clock_test.c  |  3 +--
 tools/testing/selftests/kvm/x86_64/kvm_pv_test.c   |  3 +--
 .../testing/selftests/kvm/x86_64/set_boot_cpu_id.c |  4 +---
 tools/testing/selftests/kvm/x86_64/state_test.c    |  3 +--
 .../selftests/kvm/x86_64/svm_int_ctl_test.c        |  2 +-
 .../testing/selftests/kvm/x86_64/svm_vmcall_test.c |  2 +-
 tools/testing/selftests/kvm/x86_64/tsc_msrs_test.c |  4 +---
 .../selftests/kvm/x86_64/userspace_io_test.c       |  4 +---
 .../selftests/kvm/x86_64/userspace_msr_exit_test.c |  5 ++---
 .../selftests/kvm/x86_64/vmx_apic_access_test.c    |  3 +--
 .../kvm/x86_64/vmx_close_while_nested_test.c       |  2 +-
 .../selftests/kvm/x86_64/vmx_dirty_log_test.c      |  3 +--
 .../kvm/x86_64/vmx_invalid_nested_guest_state.c    |  2 +-
 .../kvm/x86_64/vmx_nested_tsc_scaling_test.c       |  2 +-
 .../kvm/x86_64/vmx_preemption_timer_test.c         |  3 +--
 .../selftests/kvm/x86_64/vmx_tsc_adjust_test.c     |  2 +-
 .../testing/selftests/kvm/x86_64/xen_shinfo_test.c |  2 +-
 .../testing/selftests/kvm/x86_64/xen_vmcall_test.c |  2 +-
 33 files changed, 49 insertions(+), 72 deletions(-)

diff --git a/tools/testing/selftests/kvm/aarch64/arch_timer.c b/tools/testing/selftests/kvm/aarch64/arch_timer.c
index f68019be67c0..9d7bda70c20a 100644
--- a/tools/testing/selftests/kvm/aarch64/arch_timer.c
+++ b/tools/testing/selftests/kvm/aarch64/arch_timer.c
@@ -231,10 +231,14 @@ static void *test_vcpu_run(void *arg)
 		break;
 	case UCALL_ABORT:
 		sync_global_from_guest(vm, *shared_data);
-		TEST_FAIL("%s at %s:%ld\n\tvalues: %lu, %lu; %lu, vcpu: %u; stage: %u; iter: %u",
-			(const char *)uc.args[0], __FILE__, uc.args[1],
-			uc.args[2], uc.args[3], uc.args[4], vcpu_idx,
-			shared_data->guest_stage, shared_data->nr_iter);
+		REPORT_GUEST_ASSERT_N(uc,
+				      "values: %lu, %lu; %lu, vcpu %u; stage; %u; iter: %u",
+				      GUEST_ASSERT_ARG(uc, 0),
+				      GUEST_ASSERT_ARG(uc, 1),
+				      GUEST_ASSERT_ARG(uc, 2),
+				      vcpu_idx,
+				      shared_data->guest_stage,
+				      shared_data->nr_iter);
 		break;
 	default:
 		TEST_FAIL("Unexpected guest exit\n");
diff --git a/tools/testing/selftests/kvm/aarch64/debug-exceptions.c b/tools/testing/selftests/kvm/aarch64/debug-exceptions.c
index b8072b40ccc8..2ee35cf9801e 100644
--- a/tools/testing/selftests/kvm/aarch64/debug-exceptions.c
+++ b/tools/testing/selftests/kvm/aarch64/debug-exceptions.c
@@ -283,9 +283,7 @@ int main(int argc, char *argv[])
 				stage, (ulong)uc.args[1]);
 			break;
 		case UCALL_ABORT:
-			TEST_FAIL("%s at %s:%ld\n\tvalues: %#lx, %#lx",
-				(const char *)uc.args[0],
-				__FILE__, uc.args[1], uc.args[2], uc.args[3]);
+			REPORT_GUEST_ASSERT_2(uc, "values: %#lx, %#lx");
 			break;
 		case UCALL_DONE:
 			goto done;
diff --git a/tools/testing/selftests/kvm/aarch64/vgic_irq.c b/tools/testing/selftests/kvm/aarch64/vgic_irq.c
index 046ba4fde648..17417220a083 100644
--- a/tools/testing/selftests/kvm/aarch64/vgic_irq.c
+++ b/tools/testing/selftests/kvm/aarch64/vgic_irq.c
@@ -782,9 +782,7 @@ static void test_vgic(uint32_t nr_irqs, bool level_sensitive, bool eoi_split)
 			run_guest_cmd(vcpu, gic_fd, &inject_args, &args);
 			break;
 		case UCALL_ABORT:
-			TEST_FAIL("%s at %s:%ld\n\tvalues: %#lx, %#lx",
-					(const char *)uc.args[0],
-					__FILE__, uc.args[1], uc.args[2], uc.args[3]);
+			REPORT_GUEST_ASSERT_2(uc, "values: %#lx, %#lx");
 			break;
 		case UCALL_DONE:
 			goto done;
diff --git a/tools/testing/selftests/kvm/include/ucall_common.h b/tools/testing/selftests/kvm/include/ucall_common.h
index e8af3b4fef6d..e1cc72077eab 100644
--- a/tools/testing/selftests/kvm/include/ucall_common.h
+++ b/tools/testing/selftests/kvm/include/ucall_common.h
@@ -41,12 +41,14 @@ enum guest_assert_builtin_args {
 	GUEST_ASSERT_BUILTIN_NARGS
 };
 
-#define __GUEST_ASSERT(_condition, _condstr, _nargs, _args...) do {    \
-	if (!(_condition))                                              \
-		ucall(UCALL_ABORT, 2 + _nargs,                          \
-			"Failed guest assert: "                         \
-			_condstr, __LINE__, _args);                     \
-} while (0)
+#define __GUEST_ASSERT(_condition, _condstr, _nargs, _args...) do {	\
+		if (!(_condition))					\
+			ucall(UCALL_ABORT, GUEST_ASSERT_BUILTIN_NARGS + _nargs,	\
+			      "Failed guest assert: " _condstr,		\
+			      __FILE__,					\
+			      __LINE__,					\
+			      ##_args);					\
+	} while (0)
 
 #define GUEST_ASSERT(_condition) \
 	__GUEST_ASSERT(_condition, #_condition, 0, 0)
diff --git a/tools/testing/selftests/kvm/memslot_perf_test.c b/tools/testing/selftests/kvm/memslot_perf_test.c
index 5f98489e4f4d..44995446d942 100644
--- a/tools/testing/selftests/kvm/memslot_perf_test.c
+++ b/tools/testing/selftests/kvm/memslot_perf_test.c
@@ -162,9 +162,7 @@ static void *vcpu_worker(void *__data)
 				goto done;
 			break;
 		case UCALL_ABORT:
-			TEST_FAIL("%s at %s:%ld, val = %lu",
-					(const char *)uc.args[0],
-					__FILE__, uc.args[1], uc.args[2]);
+			REPORT_GUEST_ASSERT_1(uc, "val = %lu");
 			break;
 		case UCALL_DONE:
 			goto done;
diff --git a/tools/testing/selftests/kvm/steal_time.c b/tools/testing/selftests/kvm/steal_time.c
index d122f1e05cdd..9866a71463d7 100644
--- a/tools/testing/selftests/kvm/steal_time.c
+++ b/tools/testing/selftests/kvm/steal_time.c
@@ -234,8 +234,7 @@ static void run_vcpu(struct kvm_vcpu *vcpu)
 	case UCALL_DONE:
 		break;
 	case UCALL_ABORT:
-		TEST_ASSERT(false, "%s at %s:%ld", (const char *)uc.args[0],
-			    __FILE__, uc.args[1]);
+		REPORT_GUEST_ASSERT(uc);
 	default:
 		TEST_ASSERT(false, "Unexpected exit: %s",
 			    exit_reason_str(vcpu->run->exit_reason));
diff --git a/tools/testing/selftests/kvm/system_counter_offset_test.c b/tools/testing/selftests/kvm/system_counter_offset_test.c
index 862a8e93e070..1c274933912b 100644
--- a/tools/testing/selftests/kvm/system_counter_offset_test.c
+++ b/tools/testing/selftests/kvm/system_counter_offset_test.c
@@ -83,8 +83,7 @@ static void handle_sync(struct ucall *uc, uint64_t start, uint64_t end)
 
 static void handle_abort(struct ucall *uc)
 {
-	TEST_FAIL("%s at %s:%ld", (const char *)uc->args[0],
-		  __FILE__, uc->args[1]);
+	REPORT_GUEST_ASSERT(*uc);
 }
 
 static void enter_guest(struct kvm_vcpu *vcpu)
diff --git a/tools/testing/selftests/kvm/x86_64/amx_test.c b/tools/testing/selftests/kvm/x86_64/amx_test.c
index dab4ca16a2df..b71763b11b78 100644
--- a/tools/testing/selftests/kvm/x86_64/amx_test.c
+++ b/tools/testing/selftests/kvm/x86_64/amx_test.c
@@ -373,8 +373,7 @@ int main(int argc, char *argv[])
 
 		switch (get_ucall(vcpu, &uc)) {
 		case UCALL_ABORT:
-			TEST_FAIL("%s at %s:%ld", (const char *)uc.args[0],
-				  __FILE__, uc.args[1]);
+			REPORT_GUEST_ASSERT(uc);
 			/* NOT REACHED */
 		case UCALL_SYNC:
 			switch (uc.args[1]) {
diff --git a/tools/testing/selftests/kvm/x86_64/cpuid_test.c b/tools/testing/selftests/kvm/x86_64/cpuid_test.c
index 4aa784932597..3767a0cc694b 100644
--- a/tools/testing/selftests/kvm/x86_64/cpuid_test.c
+++ b/tools/testing/selftests/kvm/x86_64/cpuid_test.c
@@ -132,8 +132,7 @@ static void run_vcpu(struct kvm_vcpu *vcpu, int stage)
 	case UCALL_DONE:
 		return;
 	case UCALL_ABORT:
-		TEST_ASSERT(false, "%s at %s:%ld\n\tvalues: %#lx, %#lx", (const char *)uc.args[0],
-			    __FILE__, uc.args[1], uc.args[2], uc.args[3]);
+		REPORT_GUEST_ASSERT_2(uc, "values: %#lx, %#lx");
 	default:
 		TEST_ASSERT(false, "Unexpected exit: %s",
 			    exit_reason_str(vcpu->run->exit_reason));
diff --git a/tools/testing/selftests/kvm/x86_64/cr4_cpuid_sync_test.c b/tools/testing/selftests/kvm/x86_64/cr4_cpuid_sync_test.c
index a80940ac420f..56d8ab92eed4 100644
--- a/tools/testing/selftests/kvm/x86_64/cr4_cpuid_sync_test.c
+++ b/tools/testing/selftests/kvm/x86_64/cr4_cpuid_sync_test.c
@@ -94,7 +94,7 @@ int main(int argc, char *argv[])
 			vcpu_sregs_set(vcpu, &sregs);
 			break;
 		case UCALL_ABORT:
-			TEST_FAIL("Guest CR4 bit (OSXSAVE) unsynchronized with CPUID bit.");
+			REPORT_GUEST_ASSERT(uc);
 			break;
 		case UCALL_DONE:
 			goto done;
diff --git a/tools/testing/selftests/kvm/x86_64/emulator_error_test.c b/tools/testing/selftests/kvm/x86_64/emulator_error_test.c
index bfff2d271c48..3aa3d17f230f 100644
--- a/tools/testing/selftests/kvm/x86_64/emulator_error_test.c
+++ b/tools/testing/selftests/kvm/x86_64/emulator_error_test.c
@@ -92,8 +92,7 @@ static void process_exit_on_emulation_error(struct kvm_vcpu *vcpu)
 
 static void do_guest_assert(struct ucall *uc)
 {
-	TEST_FAIL("%s at %s:%ld", (const char *)uc->args[0], __FILE__,
-		  uc->args[1]);
+	REPORT_GUEST_ASSERT(*uc);
 }
 
 static void check_for_guest_assert(struct kvm_vcpu *vcpu)
diff --git a/tools/testing/selftests/kvm/x86_64/evmcs_test.c b/tools/testing/selftests/kvm/x86_64/evmcs_test.c
index 8dda527cc080..aacad86d90e1 100644
--- a/tools/testing/selftests/kvm/x86_64/evmcs_test.c
+++ b/tools/testing/selftests/kvm/x86_64/evmcs_test.c
@@ -236,8 +236,7 @@ int main(int argc, char *argv[])
 
 		switch (get_ucall(vcpu, &uc)) {
 		case UCALL_ABORT:
-			TEST_FAIL("%s at %s:%ld", (const char *)uc.args[0],
-		      		  __FILE__, uc.args[1]);
+			REPORT_GUEST_ASSERT(uc);
 			/* NOT REACHED */
 		case UCALL_SYNC:
 			break;
diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_clock.c b/tools/testing/selftests/kvm/x86_64/hyperv_clock.c
index f7a9e29ff0c7..d576bc8ce823 100644
--- a/tools/testing/selftests/kvm/x86_64/hyperv_clock.c
+++ b/tools/testing/selftests/kvm/x86_64/hyperv_clock.c
@@ -234,8 +234,7 @@ int main(void)
 
 		switch (get_ucall(vcpu, &uc)) {
 		case UCALL_ABORT:
-			TEST_FAIL("%s at %s:%ld", (const char *)uc.args[0],
-				  __FILE__, uc.args[1]);
+			REPORT_GUEST_ASSERT(uc);
 			/* NOT REACHED */
 		case UCALL_SYNC:
 			break;
diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_features.c b/tools/testing/selftests/kvm/x86_64/hyperv_features.c
index d5f37495ade8..0eca72333d66 100644
--- a/tools/testing/selftests/kvm/x86_64/hyperv_features.c
+++ b/tools/testing/selftests/kvm/x86_64/hyperv_features.c
@@ -483,8 +483,7 @@ static void guest_test_msrs_access(void)
 				    uc.args[1]);
 			break;
 		case UCALL_ABORT:
-			TEST_FAIL("%s at %s:%ld", (const char *)uc.args[0],
-				  __FILE__, uc.args[1]);
+			REPORT_GUEST_ASSERT(uc);
 			return;
 		case UCALL_DONE:
 			return;
@@ -653,8 +652,7 @@ static void guest_test_hcalls_access(void)
 				    uc.args[1]);
 			break;
 		case UCALL_ABORT:
-			TEST_FAIL("%s at %s:%ld", (const char *)uc.args[0],
-				  __FILE__, uc.args[1]);
+			REPORT_GUEST_ASSERT(uc);
 			return;
 		case UCALL_DONE:
 			return;
diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_svm_test.c b/tools/testing/selftests/kvm/x86_64/hyperv_svm_test.c
index c5cd9835dbd6..b7dc243ab8d5 100644
--- a/tools/testing/selftests/kvm/x86_64/hyperv_svm_test.c
+++ b/tools/testing/selftests/kvm/x86_64/hyperv_svm_test.c
@@ -145,8 +145,7 @@ int main(int argc, char *argv[])
 
 		switch (get_ucall(vcpu, &uc)) {
 		case UCALL_ABORT:
-			TEST_FAIL("%s at %s:%ld", (const char *)uc.args[0],
-				  __FILE__, uc.args[1]);
+			REPORT_GUEST_ASSERT(uc);
 			/* NOT REACHED */
 		case UCALL_SYNC:
 			break;
diff --git a/tools/testing/selftests/kvm/x86_64/kvm_clock_test.c b/tools/testing/selftests/kvm/x86_64/kvm_clock_test.c
index 138455575a11..813ce282cf56 100644
--- a/tools/testing/selftests/kvm/x86_64/kvm_clock_test.c
+++ b/tools/testing/selftests/kvm/x86_64/kvm_clock_test.c
@@ -71,8 +71,7 @@ static void handle_sync(struct ucall *uc, struct kvm_clock_data *start,
 
 static void handle_abort(struct ucall *uc)
 {
-	TEST_FAIL("%s at %s:%ld", (const char *)uc->args[0],
-		  __FILE__, uc->args[1]);
+	REPORT_GUEST_ASSERT(*uc);
 }
 
 static void setup_clock(struct kvm_vm *vm, struct test_case *test_case)
diff --git a/tools/testing/selftests/kvm/x86_64/kvm_pv_test.c b/tools/testing/selftests/kvm/x86_64/kvm_pv_test.c
index 5901ccec7079..fc9bd71e08b0 100644
--- a/tools/testing/selftests/kvm/x86_64/kvm_pv_test.c
+++ b/tools/testing/selftests/kvm/x86_64/kvm_pv_test.c
@@ -167,8 +167,7 @@ static void pr_hcall(struct ucall *uc)
 
 static void handle_abort(struct ucall *uc)
 {
-	TEST_FAIL("%s at %s:%ld", (const char *)uc->args[0],
-		  __FILE__, uc->args[1]);
+	REPORT_GUEST_ASSERT(*uc);
 }
 
 static void enter_guest(struct kvm_vcpu *vcpu)
diff --git a/tools/testing/selftests/kvm/x86_64/set_boot_cpu_id.c b/tools/testing/selftests/kvm/x86_64/set_boot_cpu_id.c
index 7ef713fdd0a5..b25d7556b638 100644
--- a/tools/testing/selftests/kvm/x86_64/set_boot_cpu_id.c
+++ b/tools/testing/selftests/kvm/x86_64/set_boot_cpu_id.c
@@ -65,9 +65,7 @@ static void run_vcpu(struct kvm_vcpu *vcpu)
 					stage);
 			break;
 		case UCALL_ABORT:
-			TEST_ASSERT(false, "%s at %s:%ld\n\tvalues: %#lx, %#lx",
-						(const char *)uc.args[0], __FILE__,
-						uc.args[1], uc.args[2], uc.args[3]);
+			REPORT_GUEST_ASSERT_2(uc, "values: %#lx, %#lx");
 		default:
 			TEST_ASSERT(false, "Unexpected exit: %s",
 				    exit_reason_str(vcpu->run->exit_reason));
diff --git a/tools/testing/selftests/kvm/x86_64/state_test.c b/tools/testing/selftests/kvm/x86_64/state_test.c
index 0bcd78cf7c79..11d036e25fd2 100644
--- a/tools/testing/selftests/kvm/x86_64/state_test.c
+++ b/tools/testing/selftests/kvm/x86_64/state_test.c
@@ -190,8 +190,7 @@ int main(int argc, char *argv[])
 
 		switch (get_ucall(vcpu, &uc)) {
 		case UCALL_ABORT:
-			TEST_FAIL("%s at %s:%ld", (const char *)uc.args[0],
-			       	  __FILE__, uc.args[1]);
+			REPORT_GUEST_ASSERT(uc);
 			/* NOT REACHED */
 		case UCALL_SYNC:
 			break;
diff --git a/tools/testing/selftests/kvm/x86_64/svm_int_ctl_test.c b/tools/testing/selftests/kvm/x86_64/svm_int_ctl_test.c
index 9c68a47b69e1..d978d1697f5a 100644
--- a/tools/testing/selftests/kvm/x86_64/svm_int_ctl_test.c
+++ b/tools/testing/selftests/kvm/x86_64/svm_int_ctl_test.c
@@ -113,7 +113,7 @@ int main(int argc, char *argv[])
 
 	switch (get_ucall(vcpu, &uc)) {
 	case UCALL_ABORT:
-		TEST_FAIL("%s", (const char *)uc.args[0]);
+		REPORT_GUEST_ASSERT(uc);
 		break;
 		/* NOT REACHED */
 	case UCALL_DONE:
diff --git a/tools/testing/selftests/kvm/x86_64/svm_vmcall_test.c b/tools/testing/selftests/kvm/x86_64/svm_vmcall_test.c
index e6d7191866a5..d53b1f7abb56 100644
--- a/tools/testing/selftests/kvm/x86_64/svm_vmcall_test.c
+++ b/tools/testing/selftests/kvm/x86_64/svm_vmcall_test.c
@@ -58,7 +58,7 @@ int main(int argc, char *argv[])
 
 		switch (get_ucall(vcpu, &uc)) {
 		case UCALL_ABORT:
-			TEST_FAIL("%s", (const char *)uc.args[0]);
+			REPORT_GUEST_ASSERT(uc);
 			/* NOT REACHED */
 		case UCALL_SYNC:
 			break;
diff --git a/tools/testing/selftests/kvm/x86_64/tsc_msrs_test.c b/tools/testing/selftests/kvm/x86_64/tsc_msrs_test.c
index 3165d3f7e065..22d366c697f7 100644
--- a/tools/testing/selftests/kvm/x86_64/tsc_msrs_test.c
+++ b/tools/testing/selftests/kvm/x86_64/tsc_msrs_test.c
@@ -79,9 +79,7 @@ static void run_vcpu(struct kvm_vcpu *vcpu, int stage)
 	case UCALL_DONE:
 		return;
 	case UCALL_ABORT:
-		TEST_ASSERT(false, "%s at %s:%ld\n" \
-			    "\tvalues: %#lx, %#lx", (const char *)uc.args[0],
-			    __FILE__, uc.args[1], uc.args[2], uc.args[3]);
+		REPORT_GUEST_ASSERT_2(uc, "values: %#lx, %#lx");
 	default:
 		TEST_ASSERT(false, "Unexpected exit: %s",
 			    exit_reason_str(vcpu->run->exit_reason));
diff --git a/tools/testing/selftests/kvm/x86_64/userspace_io_test.c b/tools/testing/selftests/kvm/x86_64/userspace_io_test.c
index 7538d57a41d5..7316521428f8 100644
--- a/tools/testing/selftests/kvm/x86_64/userspace_io_test.c
+++ b/tools/testing/selftests/kvm/x86_64/userspace_io_test.c
@@ -98,9 +98,7 @@ int main(int argc, char *argv[])
 	case UCALL_DONE:
 		break;
 	case UCALL_ABORT:
-		TEST_FAIL("%s at %s:%ld : argN+1 = 0x%lx, argN+2 = 0x%lx",
-			  (const char *)uc.args[0], __FILE__, uc.args[1],
-			  uc.args[2], uc.args[3]);
+		REPORT_GUEST_ASSERT_2(uc, "argN+1 = 0x%lx, argN+2 = 0x%lx");
 	default:
 		TEST_FAIL("Unknown ucall %lu", uc.cmd);
 	}
diff --git a/tools/testing/selftests/kvm/x86_64/userspace_msr_exit_test.c b/tools/testing/selftests/kvm/x86_64/userspace_msr_exit_test.c
index f84dc37426f5..a4f06370a245 100644
--- a/tools/testing/selftests/kvm/x86_64/userspace_msr_exit_test.c
+++ b/tools/testing/selftests/kvm/x86_64/userspace_msr_exit_test.c
@@ -400,8 +400,7 @@ static void check_for_guest_assert(struct kvm_vcpu *vcpu)
 
 	if (vcpu->run->exit_reason == KVM_EXIT_IO &&
 	    get_ucall(vcpu, &uc) == UCALL_ABORT) {
-		TEST_FAIL("%s at %s:%ld",
-			  (const char *)uc.args[0], __FILE__, uc.args[1]);
+		REPORT_GUEST_ASSERT(uc);
 	}
 }
 
@@ -610,7 +609,7 @@ static int handle_ucall(struct kvm_vcpu *vcpu)
 
 	switch (get_ucall(vcpu, &uc)) {
 	case UCALL_ABORT:
-		TEST_FAIL("Guest assertion not met");
+		REPORT_GUEST_ASSERT(uc);
 		break;
 	case UCALL_SYNC:
 		vm_ioctl(vcpu->vm, KVM_X86_SET_MSR_FILTER, &no_filter_deny);
diff --git a/tools/testing/selftests/kvm/x86_64/vmx_apic_access_test.c b/tools/testing/selftests/kvm/x86_64/vmx_apic_access_test.c
index ccb05ef7234e..d3582cea1258 100644
--- a/tools/testing/selftests/kvm/x86_64/vmx_apic_access_test.c
+++ b/tools/testing/selftests/kvm/x86_64/vmx_apic_access_test.c
@@ -114,8 +114,7 @@ int main(int argc, char *argv[])
 
 		switch (get_ucall(vcpu, &uc)) {
 		case UCALL_ABORT:
-			TEST_FAIL("%s at %s:%ld", (const char *)uc.args[0],
-				  __FILE__, uc.args[1]);
+			REPORT_GUEST_ASSERT(uc);
 			/* NOT REACHED */
 		case UCALL_SYNC:
 			apic_access_addr = uc.args[1];
diff --git a/tools/testing/selftests/kvm/x86_64/vmx_close_while_nested_test.c b/tools/testing/selftests/kvm/x86_64/vmx_close_while_nested_test.c
index 40c77bb706a1..e69e8963ed08 100644
--- a/tools/testing/selftests/kvm/x86_64/vmx_close_while_nested_test.c
+++ b/tools/testing/selftests/kvm/x86_64/vmx_close_while_nested_test.c
@@ -74,7 +74,7 @@ int main(int argc, char *argv[])
 
 		switch (get_ucall(vcpu, &uc)) {
 		case UCALL_ABORT:
-			TEST_FAIL("%s", (const char *)uc.args[0]);
+			REPORT_GUEST_ASSERT(uc);
 			/* NOT REACHED */
 		default:
 			TEST_FAIL("Unknown ucall %lu", uc.cmd);
diff --git a/tools/testing/selftests/kvm/x86_64/vmx_dirty_log_test.c b/tools/testing/selftests/kvm/x86_64/vmx_dirty_log_test.c
index 215ffa0589d4..f378960299c0 100644
--- a/tools/testing/selftests/kvm/x86_64/vmx_dirty_log_test.c
+++ b/tools/testing/selftests/kvm/x86_64/vmx_dirty_log_test.c
@@ -123,8 +123,7 @@ int main(int argc, char *argv[])
 
 		switch (get_ucall(vcpu, &uc)) {
 		case UCALL_ABORT:
-			TEST_FAIL("%s at %s:%ld", (const char *)uc.args[0],
-			       	  __FILE__, uc.args[1]);
+			REPORT_GUEST_ASSERT(uc);
 			/* NOT REACHED */
 		case UCALL_SYNC:
 			/*
diff --git a/tools/testing/selftests/kvm/x86_64/vmx_invalid_nested_guest_state.c b/tools/testing/selftests/kvm/x86_64/vmx_invalid_nested_guest_state.c
index 683f4f0a1616..8c854738f2cc 100644
--- a/tools/testing/selftests/kvm/x86_64/vmx_invalid_nested_guest_state.c
+++ b/tools/testing/selftests/kvm/x86_64/vmx_invalid_nested_guest_state.c
@@ -98,7 +98,7 @@ int main(int argc, char *argv[])
 	case UCALL_DONE:
 		break;
 	case UCALL_ABORT:
-		TEST_FAIL("%s", (const char *)uc.args[0]);
+		REPORT_GUEST_ASSERT(uc);
 	default:
 		TEST_FAIL("Unexpected ucall: %lu", uc.cmd);
 	}
diff --git a/tools/testing/selftests/kvm/x86_64/vmx_nested_tsc_scaling_test.c b/tools/testing/selftests/kvm/x86_64/vmx_nested_tsc_scaling_test.c
index ff4644038c55..6bfef77b87b7 100644
--- a/tools/testing/selftests/kvm/x86_64/vmx_nested_tsc_scaling_test.c
+++ b/tools/testing/selftests/kvm/x86_64/vmx_nested_tsc_scaling_test.c
@@ -194,7 +194,7 @@ int main(int argc, char *argv[])
 
 		switch (get_ucall(vcpu, &uc)) {
 		case UCALL_ABORT:
-			TEST_FAIL("%s", (const char *) uc.args[0]);
+			REPORT_GUEST_ASSERT(uc);
 		case UCALL_SYNC:
 			switch (uc.args[0]) {
 			case USLEEP:
diff --git a/tools/testing/selftests/kvm/x86_64/vmx_preemption_timer_test.c b/tools/testing/selftests/kvm/x86_64/vmx_preemption_timer_test.c
index 99e57b0cc2c9..0a8e989d4200 100644
--- a/tools/testing/selftests/kvm/x86_64/vmx_preemption_timer_test.c
+++ b/tools/testing/selftests/kvm/x86_64/vmx_preemption_timer_test.c
@@ -189,8 +189,7 @@ int main(int argc, char *argv[])
 
 		switch (get_ucall(vcpu, &uc)) {
 		case UCALL_ABORT:
-			TEST_FAIL("%s at %s:%ld", (const char *)uc.args[0],
-				  __FILE__, uc.args[1]);
+			REPORT_GUEST_ASSERT(uc);
 			/* NOT REACHED */
 		case UCALL_SYNC:
 			break;
diff --git a/tools/testing/selftests/kvm/x86_64/vmx_tsc_adjust_test.c b/tools/testing/selftests/kvm/x86_64/vmx_tsc_adjust_test.c
index e32bfb102699..2e75eef926ca 100644
--- a/tools/testing/selftests/kvm/x86_64/vmx_tsc_adjust_test.c
+++ b/tools/testing/selftests/kvm/x86_64/vmx_tsc_adjust_test.c
@@ -147,7 +147,7 @@ int main(int argc, char *argv[])
 
 		switch (get_ucall(vcpu, &uc)) {
 		case UCALL_ABORT:
-			TEST_FAIL("%s", (const char *)uc.args[0]);
+			REPORT_GUEST_ASSERT(uc);
 			/* NOT REACHED */
 		case UCALL_SYNC:
 			report(uc.args[1]);
diff --git a/tools/testing/selftests/kvm/x86_64/xen_shinfo_test.c b/tools/testing/selftests/kvm/x86_64/xen_shinfo_test.c
index a4a78637c35a..8a5cb800f50e 100644
--- a/tools/testing/selftests/kvm/x86_64/xen_shinfo_test.c
+++ b/tools/testing/selftests/kvm/x86_64/xen_shinfo_test.c
@@ -542,7 +542,7 @@ int main(int argc, char *argv[])
 
 		switch (get_ucall(vcpu, &uc)) {
 		case UCALL_ABORT:
-			TEST_FAIL("%s", (const char *)uc.args[0]);
+			REPORT_GUEST_ASSERT(uc);
 			/* NOT REACHED */
 		case UCALL_SYNC: {
 			struct kvm_xen_vcpu_attr rst;
diff --git a/tools/testing/selftests/kvm/x86_64/xen_vmcall_test.c b/tools/testing/selftests/kvm/x86_64/xen_vmcall_test.c
index 8b76cade9bcd..88914d48c65e 100644
--- a/tools/testing/selftests/kvm/x86_64/xen_vmcall_test.c
+++ b/tools/testing/selftests/kvm/x86_64/xen_vmcall_test.c
@@ -129,7 +129,7 @@ int main(int argc, char *argv[])
 
 		switch (get_ucall(vcpu, &uc)) {
 		case UCALL_ABORT:
-			TEST_FAIL("%s", (const char *)uc.args[0]);
+			REPORT_GUEST_ASSERT(uc);
 			/* NOT REACHED */
 		case UCALL_SYNC:
 			break;
-- 
2.36.1.476.g0c4daa206d-goog

Re: [PATCH 4/4] KVM: selftests: Fix filename reporting in guest asserts

[Andrew Jones]

On Wed, Jun 15, 2022 at 07:31:16PM +0000, Colton Lewis wrote:
> Fix filename reporting in guest asserts by ensuring the GUEST_ASSERT
> macro records __FILE__ and substituting REPORT_GUEST_ASSERT for many
> repetitive calls to TEST_FAIL.
> 
> Previously filename was reported by using __FILE__ directly in the
> selftest, wrongly assuming it would always be the same as where the
> assertion failed.y
> 
> Signed-off-by: Colton Lewis <coltonlewis@google.com>
> Reported-by: Ricardo Koller <ricarkol@google.com>
> Fixes: 4e18bccc2e5544f0be28fc1c4e6be47a469d6c60
> ---
>  tools/testing/selftests/kvm/aarch64/arch_timer.c   | 12 ++++++++----
>  .../selftests/kvm/aarch64/debug-exceptions.c       |  4 +---
>  tools/testing/selftests/kvm/aarch64/vgic_irq.c     |  4 +---
>  tools/testing/selftests/kvm/include/ucall_common.h | 14 ++++++++------
>  tools/testing/selftests/kvm/memslot_perf_test.c    |  4 +---
>  tools/testing/selftests/kvm/steal_time.c           |  3 +--
>  .../selftests/kvm/system_counter_offset_test.c     |  3 +--
>  tools/testing/selftests/kvm/x86_64/amx_test.c      |  3 +--
>  tools/testing/selftests/kvm/x86_64/cpuid_test.c    |  3 +--
>  .../selftests/kvm/x86_64/cr4_cpuid_sync_test.c     |  2 +-
>  .../selftests/kvm/x86_64/emulator_error_test.c     |  3 +--
>  tools/testing/selftests/kvm/x86_64/evmcs_test.c    |  3 +--
>  tools/testing/selftests/kvm/x86_64/hyperv_clock.c  |  3 +--
>  .../testing/selftests/kvm/x86_64/hyperv_features.c |  6 ++----
>  .../testing/selftests/kvm/x86_64/hyperv_svm_test.c |  3 +--
>  .../testing/selftests/kvm/x86_64/kvm_clock_test.c  |  3 +--
>  tools/testing/selftests/kvm/x86_64/kvm_pv_test.c   |  3 +--
>  .../testing/selftests/kvm/x86_64/set_boot_cpu_id.c |  4 +---
>  tools/testing/selftests/kvm/x86_64/state_test.c    |  3 +--
>  .../selftests/kvm/x86_64/svm_int_ctl_test.c        |  2 +-
>  .../testing/selftests/kvm/x86_64/svm_vmcall_test.c |  2 +-
>  tools/testing/selftests/kvm/x86_64/tsc_msrs_test.c |  4 +---
>  .../selftests/kvm/x86_64/userspace_io_test.c       |  4 +---
>  .../selftests/kvm/x86_64/userspace_msr_exit_test.c |  5 ++---
>  .../selftests/kvm/x86_64/vmx_apic_access_test.c    |  3 +--
>  .../kvm/x86_64/vmx_close_while_nested_test.c       |  2 +-
>  .../selftests/kvm/x86_64/vmx_dirty_log_test.c      |  3 +--
>  .../kvm/x86_64/vmx_invalid_nested_guest_state.c    |  2 +-
>  .../kvm/x86_64/vmx_nested_tsc_scaling_test.c       |  2 +-
>  .../kvm/x86_64/vmx_preemption_timer_test.c         |  3 +--
>  .../selftests/kvm/x86_64/vmx_tsc_adjust_test.c     |  2 +-
>  .../testing/selftests/kvm/x86_64/xen_shinfo_test.c |  2 +-
>  .../testing/selftests/kvm/x86_64/xen_vmcall_test.c |  2 +-
>  33 files changed, 49 insertions(+), 72 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/aarch64/arch_timer.c b/tools/testing/selftests/kvm/aarch64/arch_timer.c
> index f68019be67c0..9d7bda70c20a 100644
> --- a/tools/testing/selftests/kvm/aarch64/arch_timer.c
> +++ b/tools/testing/selftests/kvm/aarch64/arch_timer.c
> @@ -231,10 +231,14 @@ static void *test_vcpu_run(void *arg)
>  		break;
>  	case UCALL_ABORT:
>  		sync_global_from_guest(vm, *shared_data);
> -		TEST_FAIL("%s at %s:%ld\n\tvalues: %lu, %lu; %lu, vcpu: %u; stage: %u; iter: %u",
> -			(const char *)uc.args[0], __FILE__, uc.args[1],
> -			uc.args[2], uc.args[3], uc.args[4], vcpu_idx,
> -			shared_data->guest_stage, shared_data->nr_iter);
> +		REPORT_GUEST_ASSERT_N(uc,
> +				      "values: %lu, %lu; %lu, vcpu %u; stage; %u; iter: %u",
> +				      GUEST_ASSERT_ARG(uc, 0),
> +				      GUEST_ASSERT_ARG(uc, 1),
> +				      GUEST_ASSERT_ARG(uc, 2),
> +				      vcpu_idx,
> +				      shared_data->guest_stage,
> +				      shared_data->nr_iter);
>  		break;
>  	default:
>  		TEST_FAIL("Unexpected guest exit\n");
> diff --git a/tools/testing/selftests/kvm/aarch64/debug-exceptions.c b/tools/testing/selftests/kvm/aarch64/debug-exceptions.c
> index b8072b40ccc8..2ee35cf9801e 100644
> --- a/tools/testing/selftests/kvm/aarch64/debug-exceptions.c
> +++ b/tools/testing/selftests/kvm/aarch64/debug-exceptions.c
> @@ -283,9 +283,7 @@ int main(int argc, char *argv[])
>  				stage, (ulong)uc.args[1]);
>  			break;
>  		case UCALL_ABORT:
> -			TEST_FAIL("%s at %s:%ld\n\tvalues: %#lx, %#lx",
> -				(const char *)uc.args[0],
> -				__FILE__, uc.args[1], uc.args[2], uc.args[3]);
> +			REPORT_GUEST_ASSERT_2(uc, "values: %#lx, %#lx");
>  			break;
>  		case UCALL_DONE:
>  			goto done;
> diff --git a/tools/testing/selftests/kvm/aarch64/vgic_irq.c b/tools/testing/selftests/kvm/aarch64/vgic_irq.c
> index 046ba4fde648..17417220a083 100644
> --- a/tools/testing/selftests/kvm/aarch64/vgic_irq.c
> +++ b/tools/testing/selftests/kvm/aarch64/vgic_irq.c
> @@ -782,9 +782,7 @@ static void test_vgic(uint32_t nr_irqs, bool level_sensitive, bool eoi_split)
>  			run_guest_cmd(vcpu, gic_fd, &inject_args, &args);
>  			break;
>  		case UCALL_ABORT:
> -			TEST_FAIL("%s at %s:%ld\n\tvalues: %#lx, %#lx",
> -					(const char *)uc.args[0],
> -					__FILE__, uc.args[1], uc.args[2], uc.args[3]);
> +			REPORT_GUEST_ASSERT_2(uc, "values: %#lx, %#lx");
>  			break;
>  		case UCALL_DONE:
>  			goto done;
> diff --git a/tools/testing/selftests/kvm/include/ucall_common.h b/tools/testing/selftests/kvm/include/ucall_common.h
> index e8af3b4fef6d..e1cc72077eab 100644
> --- a/tools/testing/selftests/kvm/include/ucall_common.h
> +++ b/tools/testing/selftests/kvm/include/ucall_common.h
> @@ -41,12 +41,14 @@ enum guest_assert_builtin_args {
>  	GUEST_ASSERT_BUILTIN_NARGS
>  };
>  
> -#define __GUEST_ASSERT(_condition, _condstr, _nargs, _args...) do {    \
> -	if (!(_condition))                                              \
> -		ucall(UCALL_ABORT, 2 + _nargs,                          \
> -			"Failed guest assert: "                         \
> -			_condstr, __LINE__, _args);                     \
> -} while (0)
> +#define __GUEST_ASSERT(_condition, _condstr, _nargs, _args...) do {	\
> +		if (!(_condition))					\
> +			ucall(UCALL_ABORT, GUEST_ASSERT_BUILTIN_NARGS + _nargs,	\
> +			      "Failed guest assert: " _condstr,		\
> +			      __FILE__,					\
> +			      __LINE__,					\
> +			      ##_args);					\

We don't need another level of indentation nor the ## operator on _args.

> +	} while (0)
>  
>  #define GUEST_ASSERT(_condition) \
>  	__GUEST_ASSERT(_condition, #_condition, 0, 0)
> diff --git a/tools/testing/selftests/kvm/memslot_perf_test.c b/tools/testing/selftests/kvm/memslot_perf_test.c
> index 5f98489e4f4d..44995446d942 100644
> --- a/tools/testing/selftests/kvm/memslot_perf_test.c
> +++ b/tools/testing/selftests/kvm/memslot_perf_test.c
> @@ -162,9 +162,7 @@ static void *vcpu_worker(void *__data)
>  				goto done;
>  			break;
>  		case UCALL_ABORT:
> -			TEST_FAIL("%s at %s:%ld, val = %lu",
> -					(const char *)uc.args[0],
> -					__FILE__, uc.args[1], uc.args[2]);
> +			REPORT_GUEST_ASSERT_1(uc, "val = %lu");
>  			break;
>  		case UCALL_DONE:
>  			goto done;
> diff --git a/tools/testing/selftests/kvm/steal_time.c b/tools/testing/selftests/kvm/steal_time.c
> index d122f1e05cdd..9866a71463d7 100644
> --- a/tools/testing/selftests/kvm/steal_time.c
> +++ b/tools/testing/selftests/kvm/steal_time.c
> @@ -234,8 +234,7 @@ static void run_vcpu(struct kvm_vcpu *vcpu)
>  	case UCALL_DONE:
>  		break;
>  	case UCALL_ABORT:
> -		TEST_ASSERT(false, "%s at %s:%ld", (const char *)uc.args[0],
> -			    __FILE__, uc.args[1]);
> +		REPORT_GUEST_ASSERT(uc);
>  	default:
>  		TEST_ASSERT(false, "Unexpected exit: %s",
>  			    exit_reason_str(vcpu->run->exit_reason));
> diff --git a/tools/testing/selftests/kvm/system_counter_offset_test.c b/tools/testing/selftests/kvm/system_counter_offset_test.c
> index 862a8e93e070..1c274933912b 100644
> --- a/tools/testing/selftests/kvm/system_counter_offset_test.c
> +++ b/tools/testing/selftests/kvm/system_counter_offset_test.c
> @@ -83,8 +83,7 @@ static void handle_sync(struct ucall *uc, uint64_t start, uint64_t end)
>  
>  static void handle_abort(struct ucall *uc)
>  {
> -	TEST_FAIL("%s at %s:%ld", (const char *)uc->args[0],
> -		  __FILE__, uc->args[1]);
> +	REPORT_GUEST_ASSERT(*uc);
>  }
>  
>  static void enter_guest(struct kvm_vcpu *vcpu)
> diff --git a/tools/testing/selftests/kvm/x86_64/amx_test.c b/tools/testing/selftests/kvm/x86_64/amx_test.c
> index dab4ca16a2df..b71763b11b78 100644
> --- a/tools/testing/selftests/kvm/x86_64/amx_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/amx_test.c
> @@ -373,8 +373,7 @@ int main(int argc, char *argv[])
>  
>  		switch (get_ucall(vcpu, &uc)) {
>  		case UCALL_ABORT:
> -			TEST_FAIL("%s at %s:%ld", (const char *)uc.args[0],
> -				  __FILE__, uc.args[1]);
> +			REPORT_GUEST_ASSERT(uc);
>  			/* NOT REACHED */
>  		case UCALL_SYNC:
>  			switch (uc.args[1]) {
> diff --git a/tools/testing/selftests/kvm/x86_64/cpuid_test.c b/tools/testing/selftests/kvm/x86_64/cpuid_test.c
> index 4aa784932597..3767a0cc694b 100644
> --- a/tools/testing/selftests/kvm/x86_64/cpuid_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/cpuid_test.c
> @@ -132,8 +132,7 @@ static void run_vcpu(struct kvm_vcpu *vcpu, int stage)
>  	case UCALL_DONE:
>  		return;
>  	case UCALL_ABORT:
> -		TEST_ASSERT(false, "%s at %s:%ld\n\tvalues: %#lx, %#lx", (const char *)uc.args[0],
> -			    __FILE__, uc.args[1], uc.args[2], uc.args[3]);
> +		REPORT_GUEST_ASSERT_2(uc, "values: %#lx, %#lx");
>  	default:
>  		TEST_ASSERT(false, "Unexpected exit: %s",
>  			    exit_reason_str(vcpu->run->exit_reason));
> diff --git a/tools/testing/selftests/kvm/x86_64/cr4_cpuid_sync_test.c b/tools/testing/selftests/kvm/x86_64/cr4_cpuid_sync_test.c
> index a80940ac420f..56d8ab92eed4 100644
> --- a/tools/testing/selftests/kvm/x86_64/cr4_cpuid_sync_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/cr4_cpuid_sync_test.c
> @@ -94,7 +94,7 @@ int main(int argc, char *argv[])
>  			vcpu_sregs_set(vcpu, &sregs);
>  			break;
>  		case UCALL_ABORT:
> -			TEST_FAIL("Guest CR4 bit (OSXSAVE) unsynchronized with CPUID bit.");
> +			REPORT_GUEST_ASSERT(uc);

We lose the "Guest CR4..." message here.

>  			break;
>  		case UCALL_DONE:
>  			goto done;
> diff --git a/tools/testing/selftests/kvm/x86_64/emulator_error_test.c b/tools/testing/selftests/kvm/x86_64/emulator_error_test.c
> index bfff2d271c48..3aa3d17f230f 100644
> --- a/tools/testing/selftests/kvm/x86_64/emulator_error_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/emulator_error_test.c
> @@ -92,8 +92,7 @@ static void process_exit_on_emulation_error(struct kvm_vcpu *vcpu)
>  
>  static void do_guest_assert(struct ucall *uc)
>  {
> -	TEST_FAIL("%s at %s:%ld", (const char *)uc->args[0], __FILE__,
> -		  uc->args[1]);
> +	REPORT_GUEST_ASSERT(*uc);
>  }
>  
>  static void check_for_guest_assert(struct kvm_vcpu *vcpu)
> diff --git a/tools/testing/selftests/kvm/x86_64/evmcs_test.c b/tools/testing/selftests/kvm/x86_64/evmcs_test.c
> index 8dda527cc080..aacad86d90e1 100644
> --- a/tools/testing/selftests/kvm/x86_64/evmcs_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/evmcs_test.c
> @@ -236,8 +236,7 @@ int main(int argc, char *argv[])
>  
>  		switch (get_ucall(vcpu, &uc)) {
>  		case UCALL_ABORT:
> -			TEST_FAIL("%s at %s:%ld", (const char *)uc.args[0],
> -		      		  __FILE__, uc.args[1]);
> +			REPORT_GUEST_ASSERT(uc);
>  			/* NOT REACHED */
>  		case UCALL_SYNC:
>  			break;
> diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_clock.c b/tools/testing/selftests/kvm/x86_64/hyperv_clock.c
> index f7a9e29ff0c7..d576bc8ce823 100644
> --- a/tools/testing/selftests/kvm/x86_64/hyperv_clock.c
> +++ b/tools/testing/selftests/kvm/x86_64/hyperv_clock.c
> @@ -234,8 +234,7 @@ int main(void)
>  
>  		switch (get_ucall(vcpu, &uc)) {
>  		case UCALL_ABORT:
> -			TEST_FAIL("%s at %s:%ld", (const char *)uc.args[0],
> -				  __FILE__, uc.args[1]);
> +			REPORT_GUEST_ASSERT(uc);
>  			/* NOT REACHED */
>  		case UCALL_SYNC:
>  			break;
> diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_features.c b/tools/testing/selftests/kvm/x86_64/hyperv_features.c
> index d5f37495ade8..0eca72333d66 100644
> --- a/tools/testing/selftests/kvm/x86_64/hyperv_features.c
> +++ b/tools/testing/selftests/kvm/x86_64/hyperv_features.c
> @@ -483,8 +483,7 @@ static void guest_test_msrs_access(void)
>  				    uc.args[1]);
>  			break;
>  		case UCALL_ABORT:
> -			TEST_FAIL("%s at %s:%ld", (const char *)uc.args[0],
> -				  __FILE__, uc.args[1]);
> +			REPORT_GUEST_ASSERT(uc);
>  			return;
>  		case UCALL_DONE:
>  			return;
> @@ -653,8 +652,7 @@ static void guest_test_hcalls_access(void)
>  				    uc.args[1]);
>  			break;
>  		case UCALL_ABORT:
> -			TEST_FAIL("%s at %s:%ld", (const char *)uc.args[0],
> -				  __FILE__, uc.args[1]);
> +			REPORT_GUEST_ASSERT(uc);
>  			return;
>  		case UCALL_DONE:
>  			return;
> diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_svm_test.c b/tools/testing/selftests/kvm/x86_64/hyperv_svm_test.c
> index c5cd9835dbd6..b7dc243ab8d5 100644
> --- a/tools/testing/selftests/kvm/x86_64/hyperv_svm_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/hyperv_svm_test.c
> @@ -145,8 +145,7 @@ int main(int argc, char *argv[])
>  
>  		switch (get_ucall(vcpu, &uc)) {
>  		case UCALL_ABORT:
> -			TEST_FAIL("%s at %s:%ld", (const char *)uc.args[0],
> -				  __FILE__, uc.args[1]);
> +			REPORT_GUEST_ASSERT(uc);
>  			/* NOT REACHED */
>  		case UCALL_SYNC:
>  			break;
> diff --git a/tools/testing/selftests/kvm/x86_64/kvm_clock_test.c b/tools/testing/selftests/kvm/x86_64/kvm_clock_test.c
> index 138455575a11..813ce282cf56 100644
> --- a/tools/testing/selftests/kvm/x86_64/kvm_clock_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/kvm_clock_test.c
> @@ -71,8 +71,7 @@ static void handle_sync(struct ucall *uc, struct kvm_clock_data *start,
>  
>  static void handle_abort(struct ucall *uc)
>  {
> -	TEST_FAIL("%s at %s:%ld", (const char *)uc->args[0],
> -		  __FILE__, uc->args[1]);
> +	REPORT_GUEST_ASSERT(*uc);
>  }
>  
>  static void setup_clock(struct kvm_vm *vm, struct test_case *test_case)
> diff --git a/tools/testing/selftests/kvm/x86_64/kvm_pv_test.c b/tools/testing/selftests/kvm/x86_64/kvm_pv_test.c
> index 5901ccec7079..fc9bd71e08b0 100644
> --- a/tools/testing/selftests/kvm/x86_64/kvm_pv_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/kvm_pv_test.c
> @@ -167,8 +167,7 @@ static void pr_hcall(struct ucall *uc)
>  
>  static void handle_abort(struct ucall *uc)
>  {
> -	TEST_FAIL("%s at %s:%ld", (const char *)uc->args[0],
> -		  __FILE__, uc->args[1]);
> +	REPORT_GUEST_ASSERT(*uc);
>  }
>  
>  static void enter_guest(struct kvm_vcpu *vcpu)
> diff --git a/tools/testing/selftests/kvm/x86_64/set_boot_cpu_id.c b/tools/testing/selftests/kvm/x86_64/set_boot_cpu_id.c
> index 7ef713fdd0a5..b25d7556b638 100644
> --- a/tools/testing/selftests/kvm/x86_64/set_boot_cpu_id.c
> +++ b/tools/testing/selftests/kvm/x86_64/set_boot_cpu_id.c
> @@ -65,9 +65,7 @@ static void run_vcpu(struct kvm_vcpu *vcpu)
>  					stage);
>  			break;
>  		case UCALL_ABORT:
> -			TEST_ASSERT(false, "%s at %s:%ld\n\tvalues: %#lx, %#lx",
> -						(const char *)uc.args[0], __FILE__,
> -						uc.args[1], uc.args[2], uc.args[3]);
> +			REPORT_GUEST_ASSERT_2(uc, "values: %#lx, %#lx");
>  		default:
>  			TEST_ASSERT(false, "Unexpected exit: %s",
>  				    exit_reason_str(vcpu->run->exit_reason));
> diff --git a/tools/testing/selftests/kvm/x86_64/state_test.c b/tools/testing/selftests/kvm/x86_64/state_test.c
> index 0bcd78cf7c79..11d036e25fd2 100644
> --- a/tools/testing/selftests/kvm/x86_64/state_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/state_test.c
> @@ -190,8 +190,7 @@ int main(int argc, char *argv[])
>  
>  		switch (get_ucall(vcpu, &uc)) {
>  		case UCALL_ABORT:
> -			TEST_FAIL("%s at %s:%ld", (const char *)uc.args[0],
> -			       	  __FILE__, uc.args[1]);
> +			REPORT_GUEST_ASSERT(uc);
>  			/* NOT REACHED */
>  		case UCALL_SYNC:
>  			break;
> diff --git a/tools/testing/selftests/kvm/x86_64/svm_int_ctl_test.c b/tools/testing/selftests/kvm/x86_64/svm_int_ctl_test.c
> index 9c68a47b69e1..d978d1697f5a 100644
> --- a/tools/testing/selftests/kvm/x86_64/svm_int_ctl_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/svm_int_ctl_test.c
> @@ -113,7 +113,7 @@ int main(int argc, char *argv[])
>  
>  	switch (get_ucall(vcpu, &uc)) {
>  	case UCALL_ABORT:
> -		TEST_FAIL("%s", (const char *)uc.args[0]);
> +		REPORT_GUEST_ASSERT(uc);
>  		break;
>  		/* NOT REACHED */
>  	case UCALL_DONE:
> diff --git a/tools/testing/selftests/kvm/x86_64/svm_vmcall_test.c b/tools/testing/selftests/kvm/x86_64/svm_vmcall_test.c
> index e6d7191866a5..d53b1f7abb56 100644
> --- a/tools/testing/selftests/kvm/x86_64/svm_vmcall_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/svm_vmcall_test.c
> @@ -58,7 +58,7 @@ int main(int argc, char *argv[])
>  
>  		switch (get_ucall(vcpu, &uc)) {
>  		case UCALL_ABORT:
> -			TEST_FAIL("%s", (const char *)uc.args[0]);
> +			REPORT_GUEST_ASSERT(uc);
>  			/* NOT REACHED */
>  		case UCALL_SYNC:
>  			break;
> diff --git a/tools/testing/selftests/kvm/x86_64/tsc_msrs_test.c b/tools/testing/selftests/kvm/x86_64/tsc_msrs_test.c
> index 3165d3f7e065..22d366c697f7 100644
> --- a/tools/testing/selftests/kvm/x86_64/tsc_msrs_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/tsc_msrs_test.c
> @@ -79,9 +79,7 @@ static void run_vcpu(struct kvm_vcpu *vcpu, int stage)
>  	case UCALL_DONE:
>  		return;
>  	case UCALL_ABORT:
> -		TEST_ASSERT(false, "%s at %s:%ld\n" \
> -			    "\tvalues: %#lx, %#lx", (const char *)uc.args[0],
> -			    __FILE__, uc.args[1], uc.args[2], uc.args[3]);
> +		REPORT_GUEST_ASSERT_2(uc, "values: %#lx, %#lx");
>  	default:
>  		TEST_ASSERT(false, "Unexpected exit: %s",
>  			    exit_reason_str(vcpu->run->exit_reason));
> diff --git a/tools/testing/selftests/kvm/x86_64/userspace_io_test.c b/tools/testing/selftests/kvm/x86_64/userspace_io_test.c
> index 7538d57a41d5..7316521428f8 100644
> --- a/tools/testing/selftests/kvm/x86_64/userspace_io_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/userspace_io_test.c
> @@ -98,9 +98,7 @@ int main(int argc, char *argv[])
>  	case UCALL_DONE:
>  		break;
>  	case UCALL_ABORT:
> -		TEST_FAIL("%s at %s:%ld : argN+1 = 0x%lx, argN+2 = 0x%lx",
> -			  (const char *)uc.args[0], __FILE__, uc.args[1],
> -			  uc.args[2], uc.args[3]);
> +		REPORT_GUEST_ASSERT_2(uc, "argN+1 = 0x%lx, argN+2 = 0x%lx");
>  	default:
>  		TEST_FAIL("Unknown ucall %lu", uc.cmd);
>  	}
> diff --git a/tools/testing/selftests/kvm/x86_64/userspace_msr_exit_test.c b/tools/testing/selftests/kvm/x86_64/userspace_msr_exit_test.c
> index f84dc37426f5..a4f06370a245 100644
> --- a/tools/testing/selftests/kvm/x86_64/userspace_msr_exit_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/userspace_msr_exit_test.c
> @@ -400,8 +400,7 @@ static void check_for_guest_assert(struct kvm_vcpu *vcpu)
>  
>  	if (vcpu->run->exit_reason == KVM_EXIT_IO &&
>  	    get_ucall(vcpu, &uc) == UCALL_ABORT) {
> -		TEST_FAIL("%s at %s:%ld",
> -			  (const char *)uc.args[0], __FILE__, uc.args[1]);
> +		REPORT_GUEST_ASSERT(uc);
>  	}
>  }
>  
> @@ -610,7 +609,7 @@ static int handle_ucall(struct kvm_vcpu *vcpu)
>  
>  	switch (get_ucall(vcpu, &uc)) {
>  	case UCALL_ABORT:
> -		TEST_FAIL("Guest assertion not met");
> +		REPORT_GUEST_ASSERT(uc);
>  		break;
>  	case UCALL_SYNC:
>  		vm_ioctl(vcpu->vm, KVM_X86_SET_MSR_FILTER, &no_filter_deny);
> diff --git a/tools/testing/selftests/kvm/x86_64/vmx_apic_access_test.c b/tools/testing/selftests/kvm/x86_64/vmx_apic_access_test.c
> index ccb05ef7234e..d3582cea1258 100644
> --- a/tools/testing/selftests/kvm/x86_64/vmx_apic_access_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/vmx_apic_access_test.c
> @@ -114,8 +114,7 @@ int main(int argc, char *argv[])
>  
>  		switch (get_ucall(vcpu, &uc)) {
>  		case UCALL_ABORT:
> -			TEST_FAIL("%s at %s:%ld", (const char *)uc.args[0],
> -				  __FILE__, uc.args[1]);
> +			REPORT_GUEST_ASSERT(uc);
>  			/* NOT REACHED */
>  		case UCALL_SYNC:
>  			apic_access_addr = uc.args[1];
> diff --git a/tools/testing/selftests/kvm/x86_64/vmx_close_while_nested_test.c b/tools/testing/selftests/kvm/x86_64/vmx_close_while_nested_test.c
> index 40c77bb706a1..e69e8963ed08 100644
> --- a/tools/testing/selftests/kvm/x86_64/vmx_close_while_nested_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/vmx_close_while_nested_test.c
> @@ -74,7 +74,7 @@ int main(int argc, char *argv[])
>  
>  		switch (get_ucall(vcpu, &uc)) {
>  		case UCALL_ABORT:
> -			TEST_FAIL("%s", (const char *)uc.args[0]);
> +			REPORT_GUEST_ASSERT(uc);
>  			/* NOT REACHED */
>  		default:
>  			TEST_FAIL("Unknown ucall %lu", uc.cmd);
> diff --git a/tools/testing/selftests/kvm/x86_64/vmx_dirty_log_test.c b/tools/testing/selftests/kvm/x86_64/vmx_dirty_log_test.c
> index 215ffa0589d4..f378960299c0 100644
> --- a/tools/testing/selftests/kvm/x86_64/vmx_dirty_log_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/vmx_dirty_log_test.c
> @@ -123,8 +123,7 @@ int main(int argc, char *argv[])
>  
>  		switch (get_ucall(vcpu, &uc)) {
>  		case UCALL_ABORT:
> -			TEST_FAIL("%s at %s:%ld", (const char *)uc.args[0],
> -			       	  __FILE__, uc.args[1]);
> +			REPORT_GUEST_ASSERT(uc);
>  			/* NOT REACHED */
>  		case UCALL_SYNC:
>  			/*
> diff --git a/tools/testing/selftests/kvm/x86_64/vmx_invalid_nested_guest_state.c b/tools/testing/selftests/kvm/x86_64/vmx_invalid_nested_guest_state.c
> index 683f4f0a1616..8c854738f2cc 100644
> --- a/tools/testing/selftests/kvm/x86_64/vmx_invalid_nested_guest_state.c
> +++ b/tools/testing/selftests/kvm/x86_64/vmx_invalid_nested_guest_state.c
> @@ -98,7 +98,7 @@ int main(int argc, char *argv[])
>  	case UCALL_DONE:
>  		break;
>  	case UCALL_ABORT:
> -		TEST_FAIL("%s", (const char *)uc.args[0]);
> +		REPORT_GUEST_ASSERT(uc);
>  	default:
>  		TEST_FAIL("Unexpected ucall: %lu", uc.cmd);
>  	}
> diff --git a/tools/testing/selftests/kvm/x86_64/vmx_nested_tsc_scaling_test.c b/tools/testing/selftests/kvm/x86_64/vmx_nested_tsc_scaling_test.c
> index ff4644038c55..6bfef77b87b7 100644
> --- a/tools/testing/selftests/kvm/x86_64/vmx_nested_tsc_scaling_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/vmx_nested_tsc_scaling_test.c
> @@ -194,7 +194,7 @@ int main(int argc, char *argv[])
>  
>  		switch (get_ucall(vcpu, &uc)) {
>  		case UCALL_ABORT:
> -			TEST_FAIL("%s", (const char *) uc.args[0]);
> +			REPORT_GUEST_ASSERT(uc);
>  		case UCALL_SYNC:
>  			switch (uc.args[0]) {
>  			case USLEEP:
> diff --git a/tools/testing/selftests/kvm/x86_64/vmx_preemption_timer_test.c b/tools/testing/selftests/kvm/x86_64/vmx_preemption_timer_test.c
> index 99e57b0cc2c9..0a8e989d4200 100644
> --- a/tools/testing/selftests/kvm/x86_64/vmx_preemption_timer_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/vmx_preemption_timer_test.c
> @@ -189,8 +189,7 @@ int main(int argc, char *argv[])
>  
>  		switch (get_ucall(vcpu, &uc)) {
>  		case UCALL_ABORT:
> -			TEST_FAIL("%s at %s:%ld", (const char *)uc.args[0],
> -				  __FILE__, uc.args[1]);
> +			REPORT_GUEST_ASSERT(uc);
>  			/* NOT REACHED */
>  		case UCALL_SYNC:
>  			break;
> diff --git a/tools/testing/selftests/kvm/x86_64/vmx_tsc_adjust_test.c b/tools/testing/selftests/kvm/x86_64/vmx_tsc_adjust_test.c
> index e32bfb102699..2e75eef926ca 100644
> --- a/tools/testing/selftests/kvm/x86_64/vmx_tsc_adjust_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/vmx_tsc_adjust_test.c
> @@ -147,7 +147,7 @@ int main(int argc, char *argv[])
>  
>  		switch (get_ucall(vcpu, &uc)) {
>  		case UCALL_ABORT:
> -			TEST_FAIL("%s", (const char *)uc.args[0]);
> +			REPORT_GUEST_ASSERT(uc);
>  			/* NOT REACHED */
>  		case UCALL_SYNC:
>  			report(uc.args[1]);
> diff --git a/tools/testing/selftests/kvm/x86_64/xen_shinfo_test.c b/tools/testing/selftests/kvm/x86_64/xen_shinfo_test.c
> index a4a78637c35a..8a5cb800f50e 100644
> --- a/tools/testing/selftests/kvm/x86_64/xen_shinfo_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/xen_shinfo_test.c
> @@ -542,7 +542,7 @@ int main(int argc, char *argv[])
>  
>  		switch (get_ucall(vcpu, &uc)) {
>  		case UCALL_ABORT:
> -			TEST_FAIL("%s", (const char *)uc.args[0]);
> +			REPORT_GUEST_ASSERT(uc);
>  			/* NOT REACHED */
>  		case UCALL_SYNC: {
>  			struct kvm_xen_vcpu_attr rst;
> diff --git a/tools/testing/selftests/kvm/x86_64/xen_vmcall_test.c b/tools/testing/selftests/kvm/x86_64/xen_vmcall_test.c
> index 8b76cade9bcd..88914d48c65e 100644
> --- a/tools/testing/selftests/kvm/x86_64/xen_vmcall_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/xen_vmcall_test.c
> @@ -129,7 +129,7 @@ int main(int argc, char *argv[])
>  
>  		switch (get_ucall(vcpu, &uc)) {
>  		case UCALL_ABORT:
> -			TEST_FAIL("%s", (const char *)uc.args[0]);
> +			REPORT_GUEST_ASSERT(uc);
>  			/* NOT REACHED */
>  		case UCALL_SYNC:
>  			break;
> -- 
> 2.36.1.476.g0c4daa206d-goog
>

Thanks,
drew 

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH 4/4] KVM: selftests: Fix filename reporting in guest asserts

[Andrew Jones]

On Wed, Jun 15, 2022 at 07:31:16PM +0000, Colton Lewis wrote:
> Fix filename reporting in guest asserts by ensuring the GUEST_ASSERT
> macro records __FILE__ and substituting REPORT_GUEST_ASSERT for many
> repetitive calls to TEST_FAIL.
> 
> Previously filename was reported by using __FILE__ directly in the
> selftest, wrongly assuming it would always be the same as where the
> assertion failed.y
> 
> Signed-off-by: Colton Lewis <coltonlewis@google.com>
> Reported-by: Ricardo Koller <ricarkol@google.com>
> Fixes: 4e18bccc2e5544f0be28fc1c4e6be47a469d6c60
> ---
>  tools/testing/selftests/kvm/aarch64/arch_timer.c   | 12 ++++++++----
>  .../selftests/kvm/aarch64/debug-exceptions.c       |  4 +---
>  tools/testing/selftests/kvm/aarch64/vgic_irq.c     |  4 +---
>  tools/testing/selftests/kvm/include/ucall_common.h | 14 ++++++++------
>  tools/testing/selftests/kvm/memslot_perf_test.c    |  4 +---
>  tools/testing/selftests/kvm/steal_time.c           |  3 +--
>  .../selftests/kvm/system_counter_offset_test.c     |  3 +--
>  tools/testing/selftests/kvm/x86_64/amx_test.c      |  3 +--
>  tools/testing/selftests/kvm/x86_64/cpuid_test.c    |  3 +--
>  .../selftests/kvm/x86_64/cr4_cpuid_sync_test.c     |  2 +-
>  .../selftests/kvm/x86_64/emulator_error_test.c     |  3 +--
>  tools/testing/selftests/kvm/x86_64/evmcs_test.c    |  3 +--
>  tools/testing/selftests/kvm/x86_64/hyperv_clock.c  |  3 +--
>  .../testing/selftests/kvm/x86_64/hyperv_features.c |  6 ++----
>  .../testing/selftests/kvm/x86_64/hyperv_svm_test.c |  3 +--
>  .../testing/selftests/kvm/x86_64/kvm_clock_test.c  |  3 +--
>  tools/testing/selftests/kvm/x86_64/kvm_pv_test.c   |  3 +--
>  .../testing/selftests/kvm/x86_64/set_boot_cpu_id.c |  4 +---
>  tools/testing/selftests/kvm/x86_64/state_test.c    |  3 +--
>  .../selftests/kvm/x86_64/svm_int_ctl_test.c        |  2 +-
>  .../testing/selftests/kvm/x86_64/svm_vmcall_test.c |  2 +-
>  tools/testing/selftests/kvm/x86_64/tsc_msrs_test.c |  4 +---
>  .../selftests/kvm/x86_64/userspace_io_test.c       |  4 +---
>  .../selftests/kvm/x86_64/userspace_msr_exit_test.c |  5 ++---
>  .../selftests/kvm/x86_64/vmx_apic_access_test.c    |  3 +--
>  .../kvm/x86_64/vmx_close_while_nested_test.c       |  2 +-
>  .../selftests/kvm/x86_64/vmx_dirty_log_test.c      |  3 +--
>  .../kvm/x86_64/vmx_invalid_nested_guest_state.c    |  2 +-
>  .../kvm/x86_64/vmx_nested_tsc_scaling_test.c       |  2 +-
>  .../kvm/x86_64/vmx_preemption_timer_test.c         |  3 +--
>  .../selftests/kvm/x86_64/vmx_tsc_adjust_test.c     |  2 +-
>  .../testing/selftests/kvm/x86_64/xen_shinfo_test.c |  2 +-
>  .../testing/selftests/kvm/x86_64/xen_vmcall_test.c |  2 +-
>  33 files changed, 49 insertions(+), 72 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/aarch64/arch_timer.c b/tools/testing/selftests/kvm/aarch64/arch_timer.c
> index f68019be67c0..9d7bda70c20a 100644
> --- a/tools/testing/selftests/kvm/aarch64/arch_timer.c
> +++ b/tools/testing/selftests/kvm/aarch64/arch_timer.c
> @@ -231,10 +231,14 @@ static void *test_vcpu_run(void *arg)
>  		break;
>  	case UCALL_ABORT:
>  		sync_global_from_guest(vm, *shared_data);
> -		TEST_FAIL("%s at %s:%ld\n\tvalues: %lu, %lu; %lu, vcpu: %u; stage: %u; iter: %u",
> -			(const char *)uc.args[0], __FILE__, uc.args[1],
> -			uc.args[2], uc.args[3], uc.args[4], vcpu_idx,
> -			shared_data->guest_stage, shared_data->nr_iter);
> +		REPORT_GUEST_ASSERT_N(uc,
> +				      "values: %lu, %lu; %lu, vcpu %u; stage; %u; iter: %u",
> +				      GUEST_ASSERT_ARG(uc, 0),
> +				      GUEST_ASSERT_ARG(uc, 1),
> +				      GUEST_ASSERT_ARG(uc, 2),
> +				      vcpu_idx,
> +				      shared_data->guest_stage,
> +				      shared_data->nr_iter);
>  		break;
>  	default:
>  		TEST_FAIL("Unexpected guest exit\n");
> diff --git a/tools/testing/selftests/kvm/aarch64/debug-exceptions.c b/tools/testing/selftests/kvm/aarch64/debug-exceptions.c
> index b8072b40ccc8..2ee35cf9801e 100644
> --- a/tools/testing/selftests/kvm/aarch64/debug-exceptions.c
> +++ b/tools/testing/selftests/kvm/aarch64/debug-exceptions.c
> @@ -283,9 +283,7 @@ int main(int argc, char *argv[])
>  				stage, (ulong)uc.args[1]);
>  			break;
>  		case UCALL_ABORT:
> -			TEST_FAIL("%s at %s:%ld\n\tvalues: %#lx, %#lx",
> -				(const char *)uc.args[0],
> -				__FILE__, uc.args[1], uc.args[2], uc.args[3]);
> +			REPORT_GUEST_ASSERT_2(uc, "values: %#lx, %#lx");
>  			break;
>  		case UCALL_DONE:
>  			goto done;
> diff --git a/tools/testing/selftests/kvm/aarch64/vgic_irq.c b/tools/testing/selftests/kvm/aarch64/vgic_irq.c
> index 046ba4fde648..17417220a083 100644
> --- a/tools/testing/selftests/kvm/aarch64/vgic_irq.c
> +++ b/tools/testing/selftests/kvm/aarch64/vgic_irq.c
> @@ -782,9 +782,7 @@ static void test_vgic(uint32_t nr_irqs, bool level_sensitive, bool eoi_split)
>  			run_guest_cmd(vcpu, gic_fd, &inject_args, &args);
>  			break;
>  		case UCALL_ABORT:
> -			TEST_FAIL("%s at %s:%ld\n\tvalues: %#lx, %#lx",
> -					(const char *)uc.args[0],
> -					__FILE__, uc.args[1], uc.args[2], uc.args[3]);
> +			REPORT_GUEST_ASSERT_2(uc, "values: %#lx, %#lx");
>  			break;
>  		case UCALL_DONE:
>  			goto done;
> diff --git a/tools/testing/selftests/kvm/include/ucall_common.h b/tools/testing/selftests/kvm/include/ucall_common.h
> index e8af3b4fef6d..e1cc72077eab 100644
> --- a/tools/testing/selftests/kvm/include/ucall_common.h
> +++ b/tools/testing/selftests/kvm/include/ucall_common.h
> @@ -41,12 +41,14 @@ enum guest_assert_builtin_args {
>  	GUEST_ASSERT_BUILTIN_NARGS
>  };
>  
> -#define __GUEST_ASSERT(_condition, _condstr, _nargs, _args...) do {    \
> -	if (!(_condition))                                              \
> -		ucall(UCALL_ABORT, 2 + _nargs,                          \
> -			"Failed guest assert: "                         \
> -			_condstr, __LINE__, _args);                     \
> -} while (0)
> +#define __GUEST_ASSERT(_condition, _condstr, _nargs, _args...) do {	\
> +		if (!(_condition))					\
> +			ucall(UCALL_ABORT, GUEST_ASSERT_BUILTIN_NARGS + _nargs,	\
> +			      "Failed guest assert: " _condstr,		\
> +			      __FILE__,					\
> +			      __LINE__,					\
> +			      ##_args);					\

We don't need another level of indentation nor the ## operator on _args.

> +	} while (0)
>  
>  #define GUEST_ASSERT(_condition) \
>  	__GUEST_ASSERT(_condition, #_condition, 0, 0)
> diff --git a/tools/testing/selftests/kvm/memslot_perf_test.c b/tools/testing/selftests/kvm/memslot_perf_test.c
> index 5f98489e4f4d..44995446d942 100644
> --- a/tools/testing/selftests/kvm/memslot_perf_test.c
> +++ b/tools/testing/selftests/kvm/memslot_perf_test.c
> @@ -162,9 +162,7 @@ static void *vcpu_worker(void *__data)
>  				goto done;
>  			break;
>  		case UCALL_ABORT:
> -			TEST_FAIL("%s at %s:%ld, val = %lu",
> -					(const char *)uc.args[0],
> -					__FILE__, uc.args[1], uc.args[2]);
> +			REPORT_GUEST_ASSERT_1(uc, "val = %lu");
>  			break;
>  		case UCALL_DONE:
>  			goto done;
> diff --git a/tools/testing/selftests/kvm/steal_time.c b/tools/testing/selftests/kvm/steal_time.c
> index d122f1e05cdd..9866a71463d7 100644
> --- a/tools/testing/selftests/kvm/steal_time.c
> +++ b/tools/testing/selftests/kvm/steal_time.c
> @@ -234,8 +234,7 @@ static void run_vcpu(struct kvm_vcpu *vcpu)
>  	case UCALL_DONE:
>  		break;
>  	case UCALL_ABORT:
> -		TEST_ASSERT(false, "%s at %s:%ld", (const char *)uc.args[0],
> -			    __FILE__, uc.args[1]);
> +		REPORT_GUEST_ASSERT(uc);
>  	default:
>  		TEST_ASSERT(false, "Unexpected exit: %s",
>  			    exit_reason_str(vcpu->run->exit_reason));
> diff --git a/tools/testing/selftests/kvm/system_counter_offset_test.c b/tools/testing/selftests/kvm/system_counter_offset_test.c
> index 862a8e93e070..1c274933912b 100644
> --- a/tools/testing/selftests/kvm/system_counter_offset_test.c
> +++ b/tools/testing/selftests/kvm/system_counter_offset_test.c
> @@ -83,8 +83,7 @@ static void handle_sync(struct ucall *uc, uint64_t start, uint64_t end)
>  
>  static void handle_abort(struct ucall *uc)
>  {
> -	TEST_FAIL("%s at %s:%ld", (const char *)uc->args[0],
> -		  __FILE__, uc->args[1]);
> +	REPORT_GUEST_ASSERT(*uc);
>  }
>  
>  static void enter_guest(struct kvm_vcpu *vcpu)
> diff --git a/tools/testing/selftests/kvm/x86_64/amx_test.c b/tools/testing/selftests/kvm/x86_64/amx_test.c
> index dab4ca16a2df..b71763b11b78 100644
> --- a/tools/testing/selftests/kvm/x86_64/amx_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/amx_test.c
> @@ -373,8 +373,7 @@ int main(int argc, char *argv[])
>  
>  		switch (get_ucall(vcpu, &uc)) {
>  		case UCALL_ABORT:
> -			TEST_FAIL("%s at %s:%ld", (const char *)uc.args[0],
> -				  __FILE__, uc.args[1]);
> +			REPORT_GUEST_ASSERT(uc);
>  			/* NOT REACHED */
>  		case UCALL_SYNC:
>  			switch (uc.args[1]) {
> diff --git a/tools/testing/selftests/kvm/x86_64/cpuid_test.c b/tools/testing/selftests/kvm/x86_64/cpuid_test.c
> index 4aa784932597..3767a0cc694b 100644
> --- a/tools/testing/selftests/kvm/x86_64/cpuid_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/cpuid_test.c
> @@ -132,8 +132,7 @@ static void run_vcpu(struct kvm_vcpu *vcpu, int stage)
>  	case UCALL_DONE:
>  		return;
>  	case UCALL_ABORT:
> -		TEST_ASSERT(false, "%s at %s:%ld\n\tvalues: %#lx, %#lx", (const char *)uc.args[0],
> -			    __FILE__, uc.args[1], uc.args[2], uc.args[3]);
> +		REPORT_GUEST_ASSERT_2(uc, "values: %#lx, %#lx");
>  	default:
>  		TEST_ASSERT(false, "Unexpected exit: %s",
>  			    exit_reason_str(vcpu->run->exit_reason));
> diff --git a/tools/testing/selftests/kvm/x86_64/cr4_cpuid_sync_test.c b/tools/testing/selftests/kvm/x86_64/cr4_cpuid_sync_test.c
> index a80940ac420f..56d8ab92eed4 100644
> --- a/tools/testing/selftests/kvm/x86_64/cr4_cpuid_sync_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/cr4_cpuid_sync_test.c
> @@ -94,7 +94,7 @@ int main(int argc, char *argv[])
>  			vcpu_sregs_set(vcpu, &sregs);
>  			break;
>  		case UCALL_ABORT:
> -			TEST_FAIL("Guest CR4 bit (OSXSAVE) unsynchronized with CPUID bit.");
> +			REPORT_GUEST_ASSERT(uc);

We lose the "Guest CR4..." message here.

>  			break;
>  		case UCALL_DONE:
>  			goto done;
> diff --git a/tools/testing/selftests/kvm/x86_64/emulator_error_test.c b/tools/testing/selftests/kvm/x86_64/emulator_error_test.c
> index bfff2d271c48..3aa3d17f230f 100644
> --- a/tools/testing/selftests/kvm/x86_64/emulator_error_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/emulator_error_test.c
> @@ -92,8 +92,7 @@ static void process_exit_on_emulation_error(struct kvm_vcpu *vcpu)
>  
>  static void do_guest_assert(struct ucall *uc)
>  {
> -	TEST_FAIL("%s at %s:%ld", (const char *)uc->args[0], __FILE__,
> -		  uc->args[1]);
> +	REPORT_GUEST_ASSERT(*uc);
>  }
>  
>  static void check_for_guest_assert(struct kvm_vcpu *vcpu)
> diff --git a/tools/testing/selftests/kvm/x86_64/evmcs_test.c b/tools/testing/selftests/kvm/x86_64/evmcs_test.c
> index 8dda527cc080..aacad86d90e1 100644
> --- a/tools/testing/selftests/kvm/x86_64/evmcs_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/evmcs_test.c
> @@ -236,8 +236,7 @@ int main(int argc, char *argv[])
>  
>  		switch (get_ucall(vcpu, &uc)) {
>  		case UCALL_ABORT:
> -			TEST_FAIL("%s at %s:%ld", (const char *)uc.args[0],
> -		      		  __FILE__, uc.args[1]);
> +			REPORT_GUEST_ASSERT(uc);
>  			/* NOT REACHED */
>  		case UCALL_SYNC:
>  			break;
> diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_clock.c b/tools/testing/selftests/kvm/x86_64/hyperv_clock.c
> index f7a9e29ff0c7..d576bc8ce823 100644
> --- a/tools/testing/selftests/kvm/x86_64/hyperv_clock.c
> +++ b/tools/testing/selftests/kvm/x86_64/hyperv_clock.c
> @@ -234,8 +234,7 @@ int main(void)
>  
>  		switch (get_ucall(vcpu, &uc)) {
>  		case UCALL_ABORT:
> -			TEST_FAIL("%s at %s:%ld", (const char *)uc.args[0],
> -				  __FILE__, uc.args[1]);
> +			REPORT_GUEST_ASSERT(uc);
>  			/* NOT REACHED */
>  		case UCALL_SYNC:
>  			break;
> diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_features.c b/tools/testing/selftests/kvm/x86_64/hyperv_features.c
> index d5f37495ade8..0eca72333d66 100644
> --- a/tools/testing/selftests/kvm/x86_64/hyperv_features.c
> +++ b/tools/testing/selftests/kvm/x86_64/hyperv_features.c
> @@ -483,8 +483,7 @@ static void guest_test_msrs_access(void)
>  				    uc.args[1]);
>  			break;
>  		case UCALL_ABORT:
> -			TEST_FAIL("%s at %s:%ld", (const char *)uc.args[0],
> -				  __FILE__, uc.args[1]);
> +			REPORT_GUEST_ASSERT(uc);
>  			return;
>  		case UCALL_DONE:
>  			return;
> @@ -653,8 +652,7 @@ static void guest_test_hcalls_access(void)
>  				    uc.args[1]);
>  			break;
>  		case UCALL_ABORT:
> -			TEST_FAIL("%s at %s:%ld", (const char *)uc.args[0],
> -				  __FILE__, uc.args[1]);
> +			REPORT_GUEST_ASSERT(uc);
>  			return;
>  		case UCALL_DONE:
>  			return;
> diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_svm_test.c b/tools/testing/selftests/kvm/x86_64/hyperv_svm_test.c
> index c5cd9835dbd6..b7dc243ab8d5 100644
> --- a/tools/testing/selftests/kvm/x86_64/hyperv_svm_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/hyperv_svm_test.c
> @@ -145,8 +145,7 @@ int main(int argc, char *argv[])
>  
>  		switch (get_ucall(vcpu, &uc)) {
>  		case UCALL_ABORT:
> -			TEST_FAIL("%s at %s:%ld", (const char *)uc.args[0],
> -				  __FILE__, uc.args[1]);
> +			REPORT_GUEST_ASSERT(uc);
>  			/* NOT REACHED */
>  		case UCALL_SYNC:
>  			break;
> diff --git a/tools/testing/selftests/kvm/x86_64/kvm_clock_test.c b/tools/testing/selftests/kvm/x86_64/kvm_clock_test.c
> index 138455575a11..813ce282cf56 100644
> --- a/tools/testing/selftests/kvm/x86_64/kvm_clock_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/kvm_clock_test.c
> @@ -71,8 +71,7 @@ static void handle_sync(struct ucall *uc, struct kvm_clock_data *start,
>  
>  static void handle_abort(struct ucall *uc)
>  {
> -	TEST_FAIL("%s at %s:%ld", (const char *)uc->args[0],
> -		  __FILE__, uc->args[1]);
> +	REPORT_GUEST_ASSERT(*uc);
>  }
>  
>  static void setup_clock(struct kvm_vm *vm, struct test_case *test_case)
> diff --git a/tools/testing/selftests/kvm/x86_64/kvm_pv_test.c b/tools/testing/selftests/kvm/x86_64/kvm_pv_test.c
> index 5901ccec7079..fc9bd71e08b0 100644
> --- a/tools/testing/selftests/kvm/x86_64/kvm_pv_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/kvm_pv_test.c
> @@ -167,8 +167,7 @@ static void pr_hcall(struct ucall *uc)
>  
>  static void handle_abort(struct ucall *uc)
>  {
> -	TEST_FAIL("%s at %s:%ld", (const char *)uc->args[0],
> -		  __FILE__, uc->args[1]);
> +	REPORT_GUEST_ASSERT(*uc);
>  }
>  
>  static void enter_guest(struct kvm_vcpu *vcpu)
> diff --git a/tools/testing/selftests/kvm/x86_64/set_boot_cpu_id.c b/tools/testing/selftests/kvm/x86_64/set_boot_cpu_id.c
> index 7ef713fdd0a5..b25d7556b638 100644
> --- a/tools/testing/selftests/kvm/x86_64/set_boot_cpu_id.c
> +++ b/tools/testing/selftests/kvm/x86_64/set_boot_cpu_id.c
> @@ -65,9 +65,7 @@ static void run_vcpu(struct kvm_vcpu *vcpu)
>  					stage);
>  			break;
>  		case UCALL_ABORT:
> -			TEST_ASSERT(false, "%s at %s:%ld\n\tvalues: %#lx, %#lx",
> -						(const char *)uc.args[0], __FILE__,
> -						uc.args[1], uc.args[2], uc.args[3]);
> +			REPORT_GUEST_ASSERT_2(uc, "values: %#lx, %#lx");
>  		default:
>  			TEST_ASSERT(false, "Unexpected exit: %s",
>  				    exit_reason_str(vcpu->run->exit_reason));
> diff --git a/tools/testing/selftests/kvm/x86_64/state_test.c b/tools/testing/selftests/kvm/x86_64/state_test.c
> index 0bcd78cf7c79..11d036e25fd2 100644
> --- a/tools/testing/selftests/kvm/x86_64/state_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/state_test.c
> @@ -190,8 +190,7 @@ int main(int argc, char *argv[])
>  
>  		switch (get_ucall(vcpu, &uc)) {
>  		case UCALL_ABORT:
> -			TEST_FAIL("%s at %s:%ld", (const char *)uc.args[0],
> -			       	  __FILE__, uc.args[1]);
> +			REPORT_GUEST_ASSERT(uc);
>  			/* NOT REACHED */
>  		case UCALL_SYNC:
>  			break;
> diff --git a/tools/testing/selftests/kvm/x86_64/svm_int_ctl_test.c b/tools/testing/selftests/kvm/x86_64/svm_int_ctl_test.c
> index 9c68a47b69e1..d978d1697f5a 100644
> --- a/tools/testing/selftests/kvm/x86_64/svm_int_ctl_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/svm_int_ctl_test.c
> @@ -113,7 +113,7 @@ int main(int argc, char *argv[])
>  
>  	switch (get_ucall(vcpu, &uc)) {
>  	case UCALL_ABORT:
> -		TEST_FAIL("%s", (const char *)uc.args[0]);
> +		REPORT_GUEST_ASSERT(uc);
>  		break;
>  		/* NOT REACHED */
>  	case UCALL_DONE:
> diff --git a/tools/testing/selftests/kvm/x86_64/svm_vmcall_test.c b/tools/testing/selftests/kvm/x86_64/svm_vmcall_test.c
> index e6d7191866a5..d53b1f7abb56 100644
> --- a/tools/testing/selftests/kvm/x86_64/svm_vmcall_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/svm_vmcall_test.c
> @@ -58,7 +58,7 @@ int main(int argc, char *argv[])
>  
>  		switch (get_ucall(vcpu, &uc)) {
>  		case UCALL_ABORT:
> -			TEST_FAIL("%s", (const char *)uc.args[0]);
> +			REPORT_GUEST_ASSERT(uc);
>  			/* NOT REACHED */
>  		case UCALL_SYNC:
>  			break;
> diff --git a/tools/testing/selftests/kvm/x86_64/tsc_msrs_test.c b/tools/testing/selftests/kvm/x86_64/tsc_msrs_test.c
> index 3165d3f7e065..22d366c697f7 100644
> --- a/tools/testing/selftests/kvm/x86_64/tsc_msrs_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/tsc_msrs_test.c
> @@ -79,9 +79,7 @@ static void run_vcpu(struct kvm_vcpu *vcpu, int stage)
>  	case UCALL_DONE:
>  		return;
>  	case UCALL_ABORT:
> -		TEST_ASSERT(false, "%s at %s:%ld\n" \
> -			    "\tvalues: %#lx, %#lx", (const char *)uc.args[0],
> -			    __FILE__, uc.args[1], uc.args[2], uc.args[3]);
> +		REPORT_GUEST_ASSERT_2(uc, "values: %#lx, %#lx");
>  	default:
>  		TEST_ASSERT(false, "Unexpected exit: %s",
>  			    exit_reason_str(vcpu->run->exit_reason));
> diff --git a/tools/testing/selftests/kvm/x86_64/userspace_io_test.c b/tools/testing/selftests/kvm/x86_64/userspace_io_test.c
> index 7538d57a41d5..7316521428f8 100644
> --- a/tools/testing/selftests/kvm/x86_64/userspace_io_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/userspace_io_test.c
> @@ -98,9 +98,7 @@ int main(int argc, char *argv[])
>  	case UCALL_DONE:
>  		break;
>  	case UCALL_ABORT:
> -		TEST_FAIL("%s at %s:%ld : argN+1 = 0x%lx, argN+2 = 0x%lx",
> -			  (const char *)uc.args[0], __FILE__, uc.args[1],
> -			  uc.args[2], uc.args[3]);
> +		REPORT_GUEST_ASSERT_2(uc, "argN+1 = 0x%lx, argN+2 = 0x%lx");
>  	default:
>  		TEST_FAIL("Unknown ucall %lu", uc.cmd);
>  	}
> diff --git a/tools/testing/selftests/kvm/x86_64/userspace_msr_exit_test.c b/tools/testing/selftests/kvm/x86_64/userspace_msr_exit_test.c
> index f84dc37426f5..a4f06370a245 100644
> --- a/tools/testing/selftests/kvm/x86_64/userspace_msr_exit_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/userspace_msr_exit_test.c
> @@ -400,8 +400,7 @@ static void check_for_guest_assert(struct kvm_vcpu *vcpu)
>  
>  	if (vcpu->run->exit_reason == KVM_EXIT_IO &&
>  	    get_ucall(vcpu, &uc) == UCALL_ABORT) {
> -		TEST_FAIL("%s at %s:%ld",
> -			  (const char *)uc.args[0], __FILE__, uc.args[1]);
> +		REPORT_GUEST_ASSERT(uc);
>  	}
>  }
>  
> @@ -610,7 +609,7 @@ static int handle_ucall(struct kvm_vcpu *vcpu)
>  
>  	switch (get_ucall(vcpu, &uc)) {
>  	case UCALL_ABORT:
> -		TEST_FAIL("Guest assertion not met");
> +		REPORT_GUEST_ASSERT(uc);
>  		break;
>  	case UCALL_SYNC:
>  		vm_ioctl(vcpu->vm, KVM_X86_SET_MSR_FILTER, &no_filter_deny);
> diff --git a/tools/testing/selftests/kvm/x86_64/vmx_apic_access_test.c b/tools/testing/selftests/kvm/x86_64/vmx_apic_access_test.c
> index ccb05ef7234e..d3582cea1258 100644
> --- a/tools/testing/selftests/kvm/x86_64/vmx_apic_access_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/vmx_apic_access_test.c
> @@ -114,8 +114,7 @@ int main(int argc, char *argv[])
>  
>  		switch (get_ucall(vcpu, &uc)) {
>  		case UCALL_ABORT:
> -			TEST_FAIL("%s at %s:%ld", (const char *)uc.args[0],
> -				  __FILE__, uc.args[1]);
> +			REPORT_GUEST_ASSERT(uc);
>  			/* NOT REACHED */
>  		case UCALL_SYNC:
>  			apic_access_addr = uc.args[1];
> diff --git a/tools/testing/selftests/kvm/x86_64/vmx_close_while_nested_test.c b/tools/testing/selftests/kvm/x86_64/vmx_close_while_nested_test.c
> index 40c77bb706a1..e69e8963ed08 100644
> --- a/tools/testing/selftests/kvm/x86_64/vmx_close_while_nested_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/vmx_close_while_nested_test.c
> @@ -74,7 +74,7 @@ int main(int argc, char *argv[])
>  
>  		switch (get_ucall(vcpu, &uc)) {
>  		case UCALL_ABORT:
> -			TEST_FAIL("%s", (const char *)uc.args[0]);
> +			REPORT_GUEST_ASSERT(uc);
>  			/* NOT REACHED */
>  		default:
>  			TEST_FAIL("Unknown ucall %lu", uc.cmd);
> diff --git a/tools/testing/selftests/kvm/x86_64/vmx_dirty_log_test.c b/tools/testing/selftests/kvm/x86_64/vmx_dirty_log_test.c
> index 215ffa0589d4..f378960299c0 100644
> --- a/tools/testing/selftests/kvm/x86_64/vmx_dirty_log_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/vmx_dirty_log_test.c
> @@ -123,8 +123,7 @@ int main(int argc, char *argv[])
>  
>  		switch (get_ucall(vcpu, &uc)) {
>  		case UCALL_ABORT:
> -			TEST_FAIL("%s at %s:%ld", (const char *)uc.args[0],
> -			       	  __FILE__, uc.args[1]);
> +			REPORT_GUEST_ASSERT(uc);
>  			/* NOT REACHED */
>  		case UCALL_SYNC:
>  			/*
> diff --git a/tools/testing/selftests/kvm/x86_64/vmx_invalid_nested_guest_state.c b/tools/testing/selftests/kvm/x86_64/vmx_invalid_nested_guest_state.c
> index 683f4f0a1616..8c854738f2cc 100644
> --- a/tools/testing/selftests/kvm/x86_64/vmx_invalid_nested_guest_state.c
> +++ b/tools/testing/selftests/kvm/x86_64/vmx_invalid_nested_guest_state.c
> @@ -98,7 +98,7 @@ int main(int argc, char *argv[])
>  	case UCALL_DONE:
>  		break;
>  	case UCALL_ABORT:
> -		TEST_FAIL("%s", (const char *)uc.args[0]);
> +		REPORT_GUEST_ASSERT(uc);
>  	default:
>  		TEST_FAIL("Unexpected ucall: %lu", uc.cmd);
>  	}
> diff --git a/tools/testing/selftests/kvm/x86_64/vmx_nested_tsc_scaling_test.c b/tools/testing/selftests/kvm/x86_64/vmx_nested_tsc_scaling_test.c
> index ff4644038c55..6bfef77b87b7 100644
> --- a/tools/testing/selftests/kvm/x86_64/vmx_nested_tsc_scaling_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/vmx_nested_tsc_scaling_test.c
> @@ -194,7 +194,7 @@ int main(int argc, char *argv[])
>  
>  		switch (get_ucall(vcpu, &uc)) {
>  		case UCALL_ABORT:
> -			TEST_FAIL("%s", (const char *) uc.args[0]);
> +			REPORT_GUEST_ASSERT(uc);
>  		case UCALL_SYNC:
>  			switch (uc.args[0]) {
>  			case USLEEP:
> diff --git a/tools/testing/selftests/kvm/x86_64/vmx_preemption_timer_test.c b/tools/testing/selftests/kvm/x86_64/vmx_preemption_timer_test.c
> index 99e57b0cc2c9..0a8e989d4200 100644
> --- a/tools/testing/selftests/kvm/x86_64/vmx_preemption_timer_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/vmx_preemption_timer_test.c
> @@ -189,8 +189,7 @@ int main(int argc, char *argv[])
>  
>  		switch (get_ucall(vcpu, &uc)) {
>  		case UCALL_ABORT:
> -			TEST_FAIL("%s at %s:%ld", (const char *)uc.args[0],
> -				  __FILE__, uc.args[1]);
> +			REPORT_GUEST_ASSERT(uc);
>  			/* NOT REACHED */
>  		case UCALL_SYNC:
>  			break;
> diff --git a/tools/testing/selftests/kvm/x86_64/vmx_tsc_adjust_test.c b/tools/testing/selftests/kvm/x86_64/vmx_tsc_adjust_test.c
> index e32bfb102699..2e75eef926ca 100644
> --- a/tools/testing/selftests/kvm/x86_64/vmx_tsc_adjust_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/vmx_tsc_adjust_test.c
> @@ -147,7 +147,7 @@ int main(int argc, char *argv[])
>  
>  		switch (get_ucall(vcpu, &uc)) {
>  		case UCALL_ABORT:
> -			TEST_FAIL("%s", (const char *)uc.args[0]);
> +			REPORT_GUEST_ASSERT(uc);
>  			/* NOT REACHED */
>  		case UCALL_SYNC:
>  			report(uc.args[1]);
> diff --git a/tools/testing/selftests/kvm/x86_64/xen_shinfo_test.c b/tools/testing/selftests/kvm/x86_64/xen_shinfo_test.c
> index a4a78637c35a..8a5cb800f50e 100644
> --- a/tools/testing/selftests/kvm/x86_64/xen_shinfo_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/xen_shinfo_test.c
> @@ -542,7 +542,7 @@ int main(int argc, char *argv[])
>  
>  		switch (get_ucall(vcpu, &uc)) {
>  		case UCALL_ABORT:
> -			TEST_FAIL("%s", (const char *)uc.args[0]);
> +			REPORT_GUEST_ASSERT(uc);
>  			/* NOT REACHED */
>  		case UCALL_SYNC: {
>  			struct kvm_xen_vcpu_attr rst;
> diff --git a/tools/testing/selftests/kvm/x86_64/xen_vmcall_test.c b/tools/testing/selftests/kvm/x86_64/xen_vmcall_test.c
> index 8b76cade9bcd..88914d48c65e 100644
> --- a/tools/testing/selftests/kvm/x86_64/xen_vmcall_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/xen_vmcall_test.c
> @@ -129,7 +129,7 @@ int main(int argc, char *argv[])
>  
>  		switch (get_ucall(vcpu, &uc)) {
>  		case UCALL_ABORT:
> -			TEST_FAIL("%s", (const char *)uc.args[0]);
> +			REPORT_GUEST_ASSERT(uc);
>  			/* NOT REACHED */
>  		case UCALL_SYNC:
>  			break;
> -- 
> 2.36.1.476.g0c4daa206d-goog
>

Thanks,
drew 

Re: [PATCH 4/4] KVM: selftests: Fix filename reporting in guest asserts

[Paolo Bonzini]

On 6/16/22 14:45, Andrew Jones wrote:
>> +#define __GUEST_ASSERT(_condition, _condstr, _nargs, _args...) do {	\
>> +		if (!(_condition))					\
>> +			ucall(UCALL_ABORT, GUEST_ASSERT_BUILTIN_NARGS + _nargs,	\
>> +			      "Failed guest assert: " _condstr,		\
>> +			      __FILE__,					\
>> +			      __LINE__,					\
>> +			      ##_args);					\
> We don't need another level of indentation nor the ## operator on _args.
> 

The ## is needed to drop the comma if there are no _args.

Paolo

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH 4/4] KVM: selftests: Fix filename reporting in guest asserts

[Paolo Bonzini]

On 6/16/22 14:45, Andrew Jones wrote:
>> +#define __GUEST_ASSERT(_condition, _condstr, _nargs, _args...) do {	\
>> +		if (!(_condition))					\
>> +			ucall(UCALL_ABORT, GUEST_ASSERT_BUILTIN_NARGS + _nargs,	\
>> +			      "Failed guest assert: " _condstr,		\
>> +			      __FILE__,					\
>> +			      __LINE__,					\
>> +			      ##_args);					\
> We don't need another level of indentation nor the ## operator on _args.
> 

The ## is needed to drop the comma if there are no _args.

Paolo

Re: [PATCH 4/4] KVM: selftests: Fix filename reporting in guest asserts

[Colton Lewis]

On Mon, Jun 20, 2022 at 02:04:43PM +0200, Paolo Bonzini wrote:
> On 6/16/22 14:45, Andrew Jones wrote:
> > > +#define __GUEST_ASSERT(_condition, _condstr, _nargs, _args...) do {	\
> > > +		if (!(_condition))					\
> > > +			ucall(UCALL_ABORT, GUEST_ASSERT_BUILTIN_NARGS + _nargs,	\
> > > +			      "Failed guest assert: " _condstr,		\
> > > +			      __FILE__,					\
> > > +			      __LINE__,					\
> > > +			      ##_args);					\
> > We don't need another level of indentation nor the ## operator on _args.
> > 
> 
> The ## is needed to drop the comma if there are no _args.

I haven't heard anything more about part 4 of this patch in a while,
so I'm checking in that I didn't miss something requiring action on my
part.
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH 4/4] KVM: selftests: Fix filename reporting in guest asserts

[Colton Lewis]

On Mon, Jun 20, 2022 at 02:04:43PM +0200, Paolo Bonzini wrote:
> On 6/16/22 14:45, Andrew Jones wrote:
> > > +#define __GUEST_ASSERT(_condition, _condstr, _nargs, _args...) do {	\
> > > +		if (!(_condition))					\
> > > +			ucall(UCALL_ABORT, GUEST_ASSERT_BUILTIN_NARGS + _nargs,	\
> > > +			      "Failed guest assert: " _condstr,		\
> > > +			      __FILE__,					\
> > > +			      __LINE__,					\
> > > +			      ##_args);					\
> > We don't need another level of indentation nor the ## operator on _args.
> > 
> 
> The ## is needed to drop the comma if there are no _args.

I haven't heard anything more about part 4 of this patch in a while,
so I'm checking in that I didn't miss something requiring action on my
part.

Re: [PATCH 4/4] KVM: selftests: Fix filename reporting in guest asserts

[Andrew Jones]

On Wed, Jul 06, 2022 at 03:29:30PM +0000, Colton Lewis wrote:
> On Mon, Jun 20, 2022 at 02:04:43PM +0200, Paolo Bonzini wrote:
> > On 6/16/22 14:45, Andrew Jones wrote:
> > > > +#define __GUEST_ASSERT(_condition, _condstr, _nargs, _args...) do {	\
> > > > +		if (!(_condition))					\
> > > > +			ucall(UCALL_ABORT, GUEST_ASSERT_BUILTIN_NARGS + _nargs,	\
> > > > +			      "Failed guest assert: " _condstr,		\
> > > > +			      __FILE__,					\
> > > > +			      __LINE__,					\
> > > > +			      ##_args);					\
> > > We don't need another level of indentation nor the ## operator on _args.
> > > 
> > 
> > The ## is needed to drop the comma if there are no _args.
> 
> I haven't heard anything more about part 4 of this patch in a while,
> so I'm checking in that I didn't miss something requiring action on my
> part.

Paolo set me straight on the ## usage, so besides the indentation nit and
the other comment I had about losing an assert string, then it looks fine
to me.

Thanks,
drew
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH 4/4] KVM: selftests: Fix filename reporting in guest asserts

[Andrew Jones]

On Wed, Jul 06, 2022 at 03:29:30PM +0000, Colton Lewis wrote:
> On Mon, Jun 20, 2022 at 02:04:43PM +0200, Paolo Bonzini wrote:
> > On 6/16/22 14:45, Andrew Jones wrote:
> > > > +#define __GUEST_ASSERT(_condition, _condstr, _nargs, _args...) do {	\
> > > > +		if (!(_condition))					\
> > > > +			ucall(UCALL_ABORT, GUEST_ASSERT_BUILTIN_NARGS + _nargs,	\
> > > > +			      "Failed guest assert: " _condstr,		\
> > > > +			      __FILE__,					\
> > > > +			      __LINE__,					\
> > > > +			      ##_args);					\
> > > We don't need another level of indentation nor the ## operator on _args.
> > > 
> > 
> > The ## is needed to drop the comma if there are no _args.
> 
> I haven't heard anything more about part 4 of this patch in a while,
> so I'm checking in that I didn't miss something requiring action on my
> part.

Paolo set me straight on the ## usage, so besides the indentation nit and
the other comment I had about losing an assert string, then it looks fine
to me.

Thanks,
drew

Re: [PATCH 0/4] Fix filename reporting in guest asserts

[Sean Christopherson]

On Wed, Jun 15, 2022, Colton Lewis wrote:
> Fix filename reporting in guest asserts by way of abstracting out
> magic numbers and introducing new reporting macros to report
> consistently with less duplication.

FYI, this is queued up for 5.20.
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH 0/4] Fix filename reporting in guest asserts

[Sean Christopherson]

On Wed, Jun 15, 2022, Colton Lewis wrote:
> Fix filename reporting in guest asserts by way of abstracting out
> magic numbers and introducing new reporting macros to report
> consistently with less duplication.

FYI, this is queued up for 5.20.