[PATCH 0/4] KVM: selftests: Various cleanups and fixes

[PATCH 0/4] KVM: selftests: Various cleanups and fixes

[Andrew Jones]

Andrew Jones (4):
  fixup! selftests: KVM: SVM: Add vmcall test
  KVM: selftests: Share common API documentation
  KVM: selftests: Enable printf format warnings for TEST_ASSERT
  KVM: selftests: Use consistent message for test skipping

 tools/testing/selftests/kvm/.gitignore        |   5 +-
 .../selftests/kvm/demand_paging_test.c        |   6 +-
 tools/testing/selftests/kvm/dirty_log_test.c  |   3 +-
 .../testing/selftests/kvm/include/kvm_util.h  | 100 ++++++++-
 .../testing/selftests/kvm/include/test_util.h |   5 +-
 .../selftests/kvm/lib/aarch64/processor.c     |  17 --
 tools/testing/selftests/kvm/lib/assert.c      |   6 +-
 tools/testing/selftests/kvm/lib/kvm_util.c    |  10 +-
 .../selftests/kvm/lib/kvm_util_internal.h     |  48 +++++
 .../selftests/kvm/lib/s390x/processor.c       |  74 -------
 tools/testing/selftests/kvm/lib/test_util.c   |  12 ++
 .../selftests/kvm/lib/x86_64/processor.c      | 196 ++++--------------
 tools/testing/selftests/kvm/lib/x86_64/svm.c  |   2 +-
 tools/testing/selftests/kvm/lib/x86_64/vmx.c  |   2 +-
 tools/testing/selftests/kvm/s390x/memop.c     |   2 +-
 .../selftests/kvm/s390x/sync_regs_test.c      |   2 +-
 .../kvm/x86_64/cr4_cpuid_sync_test.c          |   2 +-
 .../testing/selftests/kvm/x86_64/evmcs_test.c |   6 +-
 .../selftests/kvm/x86_64/hyperv_cpuid.c       |   8 +-
 .../selftests/kvm/x86_64/mmio_warning_test.c  |   4 +-
 .../selftests/kvm/x86_64/platform_info_test.c |   3 +-
 .../kvm/x86_64/set_memory_region_test.c       |   3 +-
 .../testing/selftests/kvm/x86_64/state_test.c |   4 +-
 .../selftests/kvm/x86_64/svm_vmcall_test.c    |   3 +-
 .../selftests/kvm/x86_64/sync_regs_test.c     |   4 +-
 .../selftests/kvm/x86_64/vmx_dirty_log_test.c |   2 +-
 .../kvm/x86_64/vmx_set_nested_state_test.c    |   4 +-
 .../selftests/kvm/x86_64/xss_msr_test.c       |   2 +-
 28 files changed, 243 insertions(+), 292 deletions(-)

-- 
2.21.1

[PATCH 1/4] fixup! selftests: KVM: SVM: Add vmcall test

[Andrew Jones]

[Add svm_vmcall_test to gitignore list, and realphabetize it.]
Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 tools/testing/selftests/kvm/.gitignore | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/kvm/.gitignore b/tools/testing/selftests/kvm/.gitignore
index 0abf0a8f00d5..8bc104d39e78 100644
--- a/tools/testing/selftests/kvm/.gitignore
+++ b/tools/testing/selftests/kvm/.gitignore
@@ -1,5 +1,5 @@
-/s390x/sync_regs_test
 /s390x/memop
+/s390x/sync_regs_test
 /x86_64/cr4_cpuid_sync_test
 /x86_64/evmcs_test
 /x86_64/hyperv_cpuid
@@ -9,6 +9,7 @@
 /x86_64/set_sregs_test
 /x86_64/smm_test
 /x86_64/state_test
+/x86_64/svm_vmcall_test
 /x86_64/sync_regs_test
 /x86_64/vmx_close_while_nested_test
 /x86_64/vmx_dirty_log_test
@@ -16,6 +17,6 @@
 /x86_64/vmx_tsc_adjust_test
 /x86_64/xss_msr_test
 /clear_dirty_log_test
+/demand_paging_test
 /dirty_log_test
 /kvm_create_max_vcpus
-/demand_paging_test
-- 
2.21.1

[PATCH 2/4] KVM: selftests: Share common API documentation

[Andrew Jones]

Move function documentation comment blocks to the header files in
order to avoid duplicating them for each architecture. While at
it clean up and fix up the comment blocks.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 .../testing/selftests/kvm/include/kvm_util.h  | 100 ++++++++-
 .../selftests/kvm/lib/aarch64/processor.c     |  17 --
 .../selftests/kvm/lib/kvm_util_internal.h     |  48 +++++
 .../selftests/kvm/lib/s390x/processor.c       |  74 -------
 .../selftests/kvm/lib/x86_64/processor.c      | 196 ++++--------------
 5 files changed, 187 insertions(+), 248 deletions(-)

diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
index 707b44805149..3420ca0c2e86 100644
--- a/tools/testing/selftests/kvm/include/kvm_util.h
+++ b/tools/testing/selftests/kvm/include/kvm_util.h
@@ -16,7 +16,8 @@
 #include "sparsebit.h"
 
 
-/* Callers of kvm_util only have an incomplete/opaque description of the
+/*
+ * Callers of kvm_util only have an incomplete/opaque description of the
  * structure kvm_util is using to maintain the state of a VM.
  */
 struct kvm_vm;
@@ -78,6 +79,23 @@ void kvm_vm_elf_load(struct kvm_vm *vm, const char *filename,
 		     uint32_t data_memslot, uint32_t pgd_memslot);
 
 void vm_dump(FILE *stream, struct kvm_vm *vm, uint8_t indent);
+
+/*
+ * VM VCPU Dump
+ *
+ * Input Args:
+ *   stream - Output FILE stream
+ *   vm     - Virtual Machine
+ *   vcpuid - VCPU ID
+ *   indent - Left margin indent amount
+ *
+ * Output Args: None
+ *
+ * Return: None
+ *
+ * Dumps the current state of the VCPU specified by @vcpuid, within the VM
+ * given by @vm, to the FILE stream given by @stream.
+ */
 void vcpu_dump(FILE *stream, struct kvm_vm *vm, uint32_t vcpuid,
 	       uint8_t indent);
 
@@ -103,6 +121,22 @@ void virt_map(struct kvm_vm *vm, uint64_t vaddr, uint64_t paddr,
 void *addr_gpa2hva(struct kvm_vm *vm, vm_paddr_t gpa);
 void *addr_gva2hva(struct kvm_vm *vm, vm_vaddr_t gva);
 vm_paddr_t addr_hva2gpa(struct kvm_vm *vm, void *hva);
+
+/*
+ * Address Guest Virtual to Guest Physical
+ *
+ * Input Args:
+ *   vm - Virtual Machine
+ *   gva - VM virtual address
+ *
+ * Output Args: None
+ *
+ * Return:
+ *   Equivalent VM physical address
+ *
+ * Returns the VM physical address of the translated VM virtual
+ * address given by @gva.
+ */
 vm_paddr_t addr_gva2gpa(struct kvm_vm *vm, vm_vaddr_t gva);
 
 struct kvm_run *vcpu_state(struct kvm_vm *vm, uint32_t vcpuid);
@@ -113,7 +147,27 @@ void vcpu_set_mp_state(struct kvm_vm *vm, uint32_t vcpuid,
 		       struct kvm_mp_state *mp_state);
 void vcpu_regs_get(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_regs *regs);
 void vcpu_regs_set(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_regs *regs);
+
+/*
+ * VM VCPU Args Set
+ *
+ * Input Args:
+ *   vm - Virtual Machine
+ *   vcpuid - VCPU ID
+ *   num - number of arguments
+ *   ... - arguments, each of type uint64_t
+ *
+ * Output Args: None
+ *
+ * Return: None
+ *
+ * Sets the first @num function input registers of the VCPU with @vcpuid,
+ * per the C calling convention of the architecture, to the values given
+ * as variable args. Each of the variable args is expected to be of type
+ * uint64_t. The maximum @num can be is specific to the architecture.
+ */
 void vcpu_args_set(struct kvm_vm *vm, uint32_t vcpuid, unsigned int num, ...);
+
 void vcpu_sregs_get(struct kvm_vm *vm, uint32_t vcpuid,
 		    struct kvm_sregs *sregs);
 void vcpu_sregs_set(struct kvm_vm *vm, uint32_t vcpuid,
@@ -142,15 +196,57 @@ int vcpu_nested_state_set(struct kvm_vm *vm, uint32_t vcpuid,
 const char *exit_reason_str(unsigned int exit_reason);
 
 void virt_pgd_alloc(struct kvm_vm *vm, uint32_t pgd_memslot);
+
+/*
+ * VM Virtual Page Map
+ *
+ * Input Args:
+ *   vm - Virtual Machine
+ *   vaddr - VM Virtual Address
+ *   paddr - VM Physical Address
+ *   memslot - Memory region slot for new virtual translation tables
+ *
+ * Output Args: None
+ *
+ * Return: None
+ *
+ * Within @vm, creates a virtual translation for the page starting
+ * at @vaddr to the page starting at @paddr.
+ */
 void virt_pg_map(struct kvm_vm *vm, uint64_t vaddr, uint64_t paddr,
-		 uint32_t pgd_memslot);
+		 uint32_t memslot);
+
 vm_paddr_t vm_phy_page_alloc(struct kvm_vm *vm, vm_paddr_t paddr_min,
 			     uint32_t memslot);
 vm_paddr_t vm_phy_pages_alloc(struct kvm_vm *vm, size_t num,
 			      vm_paddr_t paddr_min, uint32_t memslot);
 
+/*
+ * Create a VM with reasonable defaults
+ *
+ * Input Args:
+ *   vcpuid - The id of the single VCPU to add to the VM.
+ *   extra_mem_pages - The size of extra memories to add (this will
+ *                     decide how much extra space we will need to
+ *                     setup the page tables using memslot 0)
+ *   guest_code - The vCPU's entry point
+ *
+ * Output Args: None
+ *
+ * Return:
+ *   Pointer to opaque structure that describes the created VM.
+ */
 struct kvm_vm *vm_create_default(uint32_t vcpuid, uint64_t extra_mem_size,
 				 void *guest_code);
+
+/*
+ * Adds a vCPU with reasonable defaults (e.g. a stack)
+ *
+ * Input Args:
+ *   vm - Virtual Machine
+ *   vcpuid - The id of the VCPU to add to the VM.
+ *   guest_code - The vCPU's entry point
+ */
 void vm_vcpu_add_default(struct kvm_vm *vm, uint32_t vcpuid, void *guest_code);
 
 bool vm_is_unrestricted_guest(struct kvm_vm *vm);
diff --git a/tools/testing/selftests/kvm/lib/aarch64/processor.c b/tools/testing/selftests/kvm/lib/aarch64/processor.c
index ba2ff3241781..f84270f0e32c 100644
--- a/tools/testing/selftests/kvm/lib/aarch64/processor.c
+++ b/tools/testing/selftests/kvm/lib/aarch64/processor.c
@@ -334,23 +334,6 @@ void vm_vcpu_add_default(struct kvm_vm *vm, uint32_t vcpuid, void *guest_code)
 	aarch64_vcpu_add_default(vm, vcpuid, NULL, guest_code);
 }
 
-
-/* VM VCPU Args Set
- *
- * Input Args:
- *   vm - Virtual Machine
- *   vcpuid - VCPU ID
- *   num - number of arguments
- *   ... - arguments, each of type uint64_t
- *
- * Output Args: None
- *
- * Return: None
- *
- * Sets the first num function input arguments to the values
- * given as variable args.  Each of the variable args is expected to
- * be of type uint64_t. The registers set by this function are r0-r7.
- */
 void vcpu_args_set(struct kvm_vm *vm, uint32_t vcpuid, unsigned int num, ...)
 {
 	va_list ap;
diff --git a/tools/testing/selftests/kvm/lib/kvm_util_internal.h b/tools/testing/selftests/kvm/lib/kvm_util_internal.h
index 2fce6750b8b3..ca56a0133127 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util_internal.h
+++ b/tools/testing/selftests/kvm/lib/kvm_util_internal.h
@@ -53,8 +53,56 @@ struct kvm_vm {
 };
 
 struct vcpu *vcpu_find(struct kvm_vm *vm, uint32_t vcpuid);
+
+/*
+ * Virtual Translation Tables Dump
+ *
+ * Input Args:
+ *   stream - Output FILE stream
+ *   vm     - Virtual Machine
+ *   indent - Left margin indent amount
+ *
+ * Output Args: None
+ *
+ * Return: None
+ *
+ * Dumps to the FILE stream given by @stream, the contents of all the
+ * virtual translation tables for the VM given by @vm.
+ */
 void virt_dump(FILE *stream, struct kvm_vm *vm, uint8_t indent);
+
+/*
+ * Register Dump
+ *
+ * Input Args:
+ *   stream - Output FILE stream
+ *   regs   - Registers
+ *   indent - Left margin indent amount
+ *
+ * Output Args: None
+ *
+ * Return: None
+ *
+ * Dumps the state of the registers given by @regs, to the FILE stream
+ * given by @stream.
+ */
 void regs_dump(FILE *stream, struct kvm_regs *regs, uint8_t indent);
+
+/*
+ * System Register Dump
+ *
+ * Input Args:
+ *   stream - Output FILE stream
+ *   sregs  - System registers
+ *   indent - Left margin indent amount
+ *
+ * Output Args: None
+ *
+ * Return: None
+ *
+ * Dumps the state of the system registers given by @sregs, to the FILE stream
+ * given by @stream.
+ */
 void sregs_dump(FILE *stream, struct kvm_sregs *sregs, uint8_t indent);
 
 struct userspace_mem_region *
diff --git a/tools/testing/selftests/kvm/lib/s390x/processor.c b/tools/testing/selftests/kvm/lib/s390x/processor.c
index a0b84235c848..8d94961bd046 100644
--- a/tools/testing/selftests/kvm/lib/s390x/processor.c
+++ b/tools/testing/selftests/kvm/lib/s390x/processor.c
@@ -51,22 +51,6 @@ static uint64_t virt_alloc_region(struct kvm_vm *vm, int ri, uint32_t memslot)
 		| ((ri < 4 ? (PAGES_PER_REGION - 1) : 0) & REGION_ENTRY_LENGTH);
 }
 
-/*
- * VM Virtual Page Map
- *
- * Input Args:
- *   vm - Virtual Machine
- *   gva - VM Virtual Address
- *   gpa - VM Physical Address
- *   memslot - Memory region slot for new virtual translation tables
- *
- * Output Args: None
- *
- * Return: None
- *
- * Within the VM given by vm, creates a virtual translation for the page
- * starting at vaddr to the page starting at paddr.
- */
 void virt_pg_map(struct kvm_vm *vm, uint64_t gva, uint64_t gpa,
 		 uint32_t memslot)
 {
@@ -107,26 +91,6 @@ void virt_pg_map(struct kvm_vm *vm, uint64_t gva, uint64_t gpa,
 	entry[idx] = gpa;
 }
 
-/*
- * Address Guest Virtual to Guest Physical
- *
- * Input Args:
- *   vm - Virtual Machine
- *   gpa - VM virtual address
- *
- * Output Args: None
- *
- * Return:
- *   Equivalent VM physical address
- *
- * Translates the VM virtual address given by gva to a VM physical
- * address and then locates the memory region containing the VM
- * physical address, within the VM given by vm.  When found, the host
- * virtual address providing the memory to the vm physical address is
- * returned.
- * A TEST_ASSERT failure occurs if no region containing translated
- * VM virtual address exists.
- */
 vm_paddr_t addr_gva2gpa(struct kvm_vm *vm, vm_vaddr_t gva)
 {
 	int ri, idx;
@@ -196,21 +160,6 @@ void virt_dump(FILE *stream, struct kvm_vm *vm, uint8_t indent)
 	virt_dump_region(stream, vm, indent, vm->pgd);
 }
 
-/*
- * Create a VM with reasonable defaults
- *
- * Input Args:
- *   vcpuid - The id of the single VCPU to add to the VM.
- *   extra_mem_pages - The size of extra memories to add (this will
- *                     decide how much extra space we will need to
- *                     setup the page tables using mem slot 0)
- *   guest_code - The vCPU's entry point
- *
- * Output Args: None
- *
- * Return:
- *   Pointer to opaque structure that describes the created VM.
- */
 struct kvm_vm *vm_create_default(uint32_t vcpuid, uint64_t extra_mem_pages,
 				 void *guest_code)
 {
@@ -231,13 +180,6 @@ struct kvm_vm *vm_create_default(uint32_t vcpuid, uint64_t extra_mem_pages,
 	return vm;
 }
 
-/*
- * Adds a vCPU with reasonable defaults (i.e. a stack and initial PSW)
- *
- * Input Args:
- *   vcpuid - The id of the VCPU to add to the VM.
- *   guest_code - The vCPU's entry point
- */
 void vm_vcpu_add_default(struct kvm_vm *vm, uint32_t vcpuid, void *guest_code)
 {
 	size_t stack_size =  DEFAULT_STACK_PGS * getpagesize();
@@ -269,22 +211,6 @@ void vm_vcpu_add_default(struct kvm_vm *vm, uint32_t vcpuid, void *guest_code)
 	run->psw_addr = (uintptr_t)guest_code;
 }
 
-/* VM VCPU Args Set
- *
- * Input Args:
- *   vm - Virtual Machine
- *   vcpuid - VCPU ID
- *   num - number of arguments
- *   ... - arguments, each of type uint64_t
- *
- * Output Args: None
- *
- * Return: None
- *
- * Sets the first num function input arguments to the values
- * given as variable args.  Each of the variable args is expected to
- * be of type uint64_t. The registers set by this function are r2-r6.
- */
 void vcpu_args_set(struct kvm_vm *vm, uint32_t vcpuid, unsigned int num, ...)
 {
 	va_list ap;
diff --git a/tools/testing/selftests/kvm/lib/x86_64/processor.c b/tools/testing/selftests/kvm/lib/x86_64/processor.c
index 683d3bdb8f6a..7ce067c8a05d 100644
--- a/tools/testing/selftests/kvm/lib/x86_64/processor.c
+++ b/tools/testing/selftests/kvm/lib/x86_64/processor.c
@@ -77,20 +77,6 @@ struct pageTableEntry {
 	uint64_t execute_disable:1;
 };
 
-/* Register Dump
- *
- * Input Args:
- *   indent - Left margin indent amount
- *   regs - register
- *
- * Output Args:
- *   stream - Output FILE stream
- *
- * Return: None
- *
- * Dumps the state of the registers given by regs, to the FILE stream
- * given by steam.
- */
 void regs_dump(FILE *stream, struct kvm_regs *regs,
 	       uint8_t indent)
 {
@@ -115,19 +101,20 @@ void regs_dump(FILE *stream, struct kvm_regs *regs,
 		regs->rip, regs->rflags);
 }
 
-/* Segment Dump
+/*
+ * Segment Dump
  *
  * Input Args:
- *   indent - Left margin indent amount
+ *   stream  - Output FILE stream
  *   segment - KVM segment
+ *   indent  - Left margin indent amount
  *
- * Output Args:
- *   stream - Output FILE stream
+ * Output Args: None
  *
  * Return: None
  *
- * Dumps the state of the KVM segment given by segment, to the FILE stream
- * given by steam.
+ * Dumps the state of the KVM segment given by @segment, to the FILE stream
+ * given by @stream.
  */
 static void segment_dump(FILE *stream, struct kvm_segment *segment,
 			 uint8_t indent)
@@ -146,19 +133,20 @@ static void segment_dump(FILE *stream, struct kvm_segment *segment,
 		segment->unusable, segment->padding);
 }
 
-/* dtable Dump
+/*
+ * dtable Dump
  *
  * Input Args:
- *   indent - Left margin indent amount
+ *   stream - Output FILE stream
  *   dtable - KVM dtable
+ *   indent - Left margin indent amount
  *
- * Output Args:
- *   stream - Output FILE stream
+ * Output Args: None
  *
  * Return: None
  *
- * Dumps the state of the KVM dtable given by dtable, to the FILE stream
- * given by steam.
+ * Dumps the state of the KVM dtable given by @dtable, to the FILE stream
+ * given by @stream.
  */
 static void dtable_dump(FILE *stream, struct kvm_dtable *dtable,
 			uint8_t indent)
@@ -169,20 +157,6 @@ static void dtable_dump(FILE *stream, struct kvm_dtable *dtable,
 		dtable->padding[0], dtable->padding[1], dtable->padding[2]);
 }
 
-/* System Register Dump
- *
- * Input Args:
- *   indent - Left margin indent amount
- *   sregs - System registers
- *
- * Output Args:
- *   stream - Output FILE stream
- *
- * Return: None
- *
- * Dumps the state of the system registers given by sregs, to the FILE stream
- * given by steam.
- */
 void sregs_dump(FILE *stream, struct kvm_sregs *sregs,
 		uint8_t indent)
 {
@@ -240,21 +214,6 @@ void virt_pgd_alloc(struct kvm_vm *vm, uint32_t pgd_memslot)
 	}
 }
 
-/* VM Virtual Page Map
- *
- * Input Args:
- *   vm - Virtual Machine
- *   vaddr - VM Virtual Address
- *   paddr - VM Physical Address
- *   pgd_memslot - Memory region slot for new virtual translation tables
- *
- * Output Args: None
- *
- * Return: None
- *
- * Within the VM given by vm, creates a virtual translation for the page
- * starting at vaddr to the page starting at paddr.
- */
 void virt_pg_map(struct kvm_vm *vm, uint64_t vaddr, uint64_t paddr,
 	uint32_t pgd_memslot)
 {
@@ -326,20 +285,6 @@ void virt_pg_map(struct kvm_vm *vm, uint64_t vaddr, uint64_t paddr,
 	pte[index[0]].present = 1;
 }
 
-/* Virtual Translation Tables Dump
- *
- * Input Args:
- *   vm - Virtual Machine
- *   indent - Left margin indent amount
- *
- * Output Args:
- *   stream - Output FILE stream
- *
- * Return: None
- *
- * Dumps to the FILE stream given by stream, the contents of all the
- * virtual translation tables for the VM given by vm.
- */
 void virt_dump(FILE *stream, struct kvm_vm *vm, uint8_t indent)
 {
 	struct pageMapL4Entry *pml4e, *pml4e_start;
@@ -421,7 +366,8 @@ void virt_dump(FILE *stream, struct kvm_vm *vm, uint8_t indent)
 	}
 }
 
-/* Set Unusable Segment
+/*
+ * Set Unusable Segment
  *
  * Input Args: None
  *
@@ -430,7 +376,7 @@ void virt_dump(FILE *stream, struct kvm_vm *vm, uint8_t indent)
  *
  * Return: None
  *
- * Sets the segment register pointed to by segp to an unusable state.
+ * Sets the segment register pointed to by @segp to an unusable state.
  */
 static void kvm_seg_set_unusable(struct kvm_segment *segp)
 {
@@ -460,7 +406,8 @@ static void kvm_seg_fill_gdt_64bit(struct kvm_vm *vm, struct kvm_segment *segp)
 }
 
 
-/* Set Long Mode Flat Kernel Code Segment
+/*
+ * Set Long Mode Flat Kernel Code Segment
  *
  * Input Args:
  *   vm - VM whose GDT is being filled, or NULL to only write segp
@@ -471,8 +418,8 @@ static void kvm_seg_fill_gdt_64bit(struct kvm_vm *vm, struct kvm_segment *segp)
  *
  * Return: None
  *
- * Sets up the KVM segment pointed to by segp, to be a code segment
- * with the selector value given by selector.
+ * Sets up the KVM segment pointed to by @segp, to be a code segment
+ * with the selector value given by @selector.
  */
 static void kvm_seg_set_kernel_code_64bit(struct kvm_vm *vm, uint16_t selector,
 	struct kvm_segment *segp)
@@ -491,7 +438,8 @@ static void kvm_seg_set_kernel_code_64bit(struct kvm_vm *vm, uint16_t selector,
 		kvm_seg_fill_gdt_64bit(vm, segp);
 }
 
-/* Set Long Mode Flat Kernel Data Segment
+/*
+ * Set Long Mode Flat Kernel Data Segment
  *
  * Input Args:
  *   vm - VM whose GDT is being filled, or NULL to only write segp
@@ -502,8 +450,8 @@ static void kvm_seg_set_kernel_code_64bit(struct kvm_vm *vm, uint16_t selector,
  *
  * Return: None
  *
- * Sets up the KVM segment pointed to by segp, to be a data segment
- * with the selector value given by selector.
+ * Sets up the KVM segment pointed to by @segp, to be a data segment
+ * with the selector value given by @selector.
  */
 static void kvm_seg_set_kernel_data_64bit(struct kvm_vm *vm, uint16_t selector,
 	struct kvm_segment *segp)
@@ -521,24 +469,6 @@ static void kvm_seg_set_kernel_data_64bit(struct kvm_vm *vm, uint16_t selector,
 		kvm_seg_fill_gdt_64bit(vm, segp);
 }
 
-/* Address Guest Virtual to Guest Physical
- *
- * Input Args:
- *   vm - Virtual Machine
- *   gpa - VM virtual address
- *
- * Output Args: None
- *
- * Return:
- *   Equivalent VM physical address
- *
- * Translates the VM virtual address given by gva to a VM physical
- * address and then locates the memory region containing the VM
- * physical address, within the VM given by vm.  When found, the host
- * virtual address providing the memory to the vm physical address is returned.
- * A TEST_ASSERT failure occurs if no region containing translated
- * VM virtual address exists.
- */
 vm_paddr_t addr_gva2gpa(struct kvm_vm *vm, vm_vaddr_t gva)
 {
 	uint16_t index[4];
@@ -640,12 +570,7 @@ static void vcpu_setup(struct kvm_vm *vm, int vcpuid, int pgd_memslot, int gdt_m
 	sregs.cr3 = vm->pgd;
 	vcpu_sregs_set(vm, vcpuid, &sregs);
 }
-/* Adds a vCPU with reasonable defaults (i.e., a stack)
- *
- * Input Args:
- *   vcpuid - The id of the VCPU to add to the VM.
- *   guest_code - The vCPU's entry point
- */
+
 void vm_vcpu_add_default(struct kvm_vm *vm, uint32_t vcpuid, void *guest_code)
 {
 	struct kvm_mp_state mp_state;
@@ -670,7 +595,8 @@ void vm_vcpu_add_default(struct kvm_vm *vm, uint32_t vcpuid, void *guest_code)
 	vcpu_set_mp_state(vm, vcpuid, &mp_state);
 }
 
-/* Allocate an instance of struct kvm_cpuid2
+/*
+ * Allocate an instance of struct kvm_cpuid2
  *
  * Input Args: None
  *
@@ -703,7 +629,8 @@ static struct kvm_cpuid2 *allocate_kvm_cpuid2(void)
 	return cpuid;
 }
 
-/* KVM Supported CPUID Get
+/*
+ * KVM Supported CPUID Get
  *
  * Input Args: None
  *
@@ -735,11 +662,12 @@ struct kvm_cpuid2 *kvm_get_supported_cpuid(void)
 	return cpuid;
 }
 
-/* Locate a cpuid entry.
+/*
+ * Locate a cpuid entry.
  *
  * Input Args:
- *   cpuid: The cpuid.
  *   function: The function of the cpuid entry to find.
+ *   index: The index of the cpuid entry.
  *
  * Output Args: None
  *
@@ -766,7 +694,8 @@ kvm_get_supported_cpuid_index(uint32_t function, uint32_t index)
 	return entry;
 }
 
-/* VM VCPU CPUID Set
+/*
+ * VM VCPU CPUID Set
  *
  * Input Args:
  *   vm - Virtual Machine
@@ -793,20 +722,6 @@ void vcpu_set_cpuid(struct kvm_vm *vm,
 
 }
 
-/* Create a VM with reasonable defaults
- *
- * Input Args:
- *   vcpuid - The id of the single VCPU to add to the VM.
- *   extra_mem_pages - The size of extra memories to add (this will
- *                     decide how much extra space we will need to
- *                     setup the page tables using mem slot 0)
- *   guest_code - The vCPU's entry point
- *
- * Output Args: None
- *
- * Return:
- *   Pointer to opaque structure that describes the created VM.
- */
 struct kvm_vm *vm_create_default(uint32_t vcpuid, uint64_t extra_mem_pages,
 				 void *guest_code)
 {
@@ -837,7 +752,8 @@ struct kvm_vm *vm_create_default(uint32_t vcpuid, uint64_t extra_mem_pages,
 	return vm;
 }
 
-/* VCPU Get MSR
+/*
+ * VCPU Get MSR
  *
  * Input Args:
  *   vm - Virtual Machine
@@ -869,7 +785,8 @@ uint64_t vcpu_get_msr(struct kvm_vm *vm, uint32_t vcpuid, uint64_t msr_index)
 	return buffer.entry.data;
 }
 
-/* _VCPU Set MSR
+/*
+ * _VCPU Set MSR
  *
  * Input Args:
  *   vm - Virtual Machine
@@ -902,7 +819,8 @@ int _vcpu_set_msr(struct kvm_vm *vm, uint32_t vcpuid, uint64_t msr_index,
 	return r;
 }
 
-/* VCPU Set MSR
+/*
+ * VCPU Set MSR
  *
  * Input Args:
  *   vm - Virtual Machine
@@ -926,22 +844,6 @@ void vcpu_set_msr(struct kvm_vm *vm, uint32_t vcpuid, uint64_t msr_index,
 		"  rc: %i errno: %i", r, errno);
 }
 
-/* VM VCPU Args Set
- *
- * Input Args:
- *   vm - Virtual Machine
- *   vcpuid - VCPU ID
- *   num - number of arguments
- *   ... - arguments, each of type uint64_t
- *
- * Output Args: None
- *
- * Return: None
- *
- * Sets the first num function input arguments to the values
- * given as variable args.  Each of the variable args is expected to
- * be of type uint64_t.
- */
 void vcpu_args_set(struct kvm_vm *vm, uint32_t vcpuid, unsigned int num, ...)
 {
 	va_list ap;
@@ -976,22 +878,6 @@ void vcpu_args_set(struct kvm_vm *vm, uint32_t vcpuid, unsigned int num, ...)
 	va_end(ap);
 }
 
-/*
- * VM VCPU Dump
- *
- * Input Args:
- *   vm - Virtual Machine
- *   vcpuid - VCPU ID
- *   indent - Left margin indent amount
- *
- * Output Args:
- *   stream - Output FILE stream
- *
- * Return: None
- *
- * Dumps the current state of the VCPU specified by vcpuid, within the VM
- * given by vm, to the FILE stream given by stream.
- */
 void vcpu_dump(FILE *stream, struct kvm_vm *vm, uint32_t vcpuid, uint8_t indent)
 {
 	struct kvm_regs regs;
-- 
2.21.1

[PATCH 3/4] KVM: selftests: Enable printf format warnings for TEST_ASSERT

[Andrew Jones]

Use the format attribute to enable printf format warnings, and
then fix them all.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 tools/testing/selftests/kvm/demand_paging_test.c          | 2 +-
 tools/testing/selftests/kvm/include/test_util.h           | 3 ++-
 tools/testing/selftests/kvm/lib/kvm_util.c                | 8 ++++----
 tools/testing/selftests/kvm/x86_64/evmcs_test.c           | 4 ++--
 tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c         | 2 +-
 .../testing/selftests/kvm/x86_64/set_memory_region_test.c | 3 +--
 tools/testing/selftests/kvm/x86_64/state_test.c           | 4 ++--
 tools/testing/selftests/kvm/x86_64/svm_vmcall_test.c      | 3 +--
 tools/testing/selftests/kvm/x86_64/vmx_dirty_log_test.c   | 2 +-
 .../selftests/kvm/x86_64/vmx_set_nested_state_test.c      | 2 +-
 10 files changed, 16 insertions(+), 17 deletions(-)

diff --git a/tools/testing/selftests/kvm/demand_paging_test.c b/tools/testing/selftests/kvm/demand_paging_test.c
index c1e326d3ed7f..47654071544c 100644
--- a/tools/testing/selftests/kvm/demand_paging_test.c
+++ b/tools/testing/selftests/kvm/demand_paging_test.c
@@ -388,7 +388,7 @@ static void run_test(enum vm_guest_mode mode, bool use_uffd,
 	 */
 	TEST_ASSERT(guest_num_pages < vm_get_max_gfn(vm),
 		    "Requested more guest memory than address space allows.\n"
-		    "    guest pages: %lx max gfn: %lx vcpus: %d wss: %lx]\n",
+		    "    guest pages: %lx max gfn: %x vcpus: %d wss: %lx]\n",
 		    guest_num_pages, vm_get_max_gfn(vm), vcpus,
 		    vcpu_memory_bytes);
 
diff --git a/tools/testing/selftests/kvm/include/test_util.h b/tools/testing/selftests/kvm/include/test_util.h
index c921ea719ae0..07823740227b 100644
--- a/tools/testing/selftests/kvm/include/test_util.h
+++ b/tools/testing/selftests/kvm/include/test_util.h
@@ -37,7 +37,8 @@ ssize_t test_read(int fd, void *buf, size_t count);
 int test_seq_read(const char *path, char **bufp, size_t *sizep);
 
 void test_assert(bool exp, const char *exp_str,
-		 const char *file, unsigned int line, const char *fmt, ...);
+		 const char *file, unsigned int line, const char *fmt, ...)
+		__attribute__((format(printf, 5, 6)));
 
 #define TEST_ASSERT(e, fmt, ...) \
 	test_assert((e), #e, __FILE__, __LINE__, fmt, ##__VA_ARGS__)
diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
index 69a28a9211b4..b29c5d338555 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -265,7 +265,7 @@ void kvm_vm_restart(struct kvm_vm *vmp, int perm)
 		TEST_ASSERT(ret == 0, "KVM_SET_USER_MEMORY_REGION IOCTL failed,\n"
 			    "  rc: %i errno: %i\n"
 			    "  slot: %u flags: 0x%x\n"
-			    "  guest_phys_addr: 0x%lx size: 0x%lx",
+			    "  guest_phys_addr: 0x%llx size: 0x%llx",
 			    ret, errno, region->region.slot,
 			    region->region.flags,
 			    region->region.guest_phys_addr,
@@ -280,7 +280,7 @@ void kvm_vm_get_dirty_log(struct kvm_vm *vm, int slot, void *log)
 
 	ret = ioctl(vm->fd, KVM_GET_DIRTY_LOG, &args);
 	TEST_ASSERT(ret == 0, "%s: KVM_GET_DIRTY_LOG failed: %s",
-		    strerror(-ret));
+		    __func__, strerror(-ret));
 }
 
 void kvm_vm_clear_dirty_log(struct kvm_vm *vm, int slot, void *log,
@@ -293,7 +293,7 @@ void kvm_vm_clear_dirty_log(struct kvm_vm *vm, int slot, void *log,
 
 	ret = ioctl(vm->fd, KVM_CLEAR_DIRTY_LOG, &args);
 	TEST_ASSERT(ret == 0, "%s: KVM_CLEAR_DIRTY_LOG failed: %s",
-		    strerror(-ret));
+		    __func__, strerror(-ret));
 }
 
 /*
@@ -785,7 +785,7 @@ void vm_mem_region_move(struct kvm_vm *vm, uint32_t slot, uint64_t new_gpa)
 	ret = ioctl(vm->fd, KVM_SET_USER_MEMORY_REGION, &region->region);
 
 	TEST_ASSERT(!ret, "KVM_SET_USER_MEMORY_REGION failed\n"
-		    "ret: %i errno: %i slot: %u flags: 0x%x",
+		    "ret: %i errno: %i slot: %u flags: 0x%lx",
 		    ret, errno, slot, new_gpa);
 }
 
diff --git a/tools/testing/selftests/kvm/x86_64/evmcs_test.c b/tools/testing/selftests/kvm/x86_64/evmcs_test.c
index 185226c39c03..464a55217085 100644
--- a/tools/testing/selftests/kvm/x86_64/evmcs_test.c
+++ b/tools/testing/selftests/kvm/x86_64/evmcs_test.c
@@ -109,7 +109,7 @@ int main(int argc, char *argv[])
 
 		switch (get_ucall(vm, VCPU_ID, &uc)) {
 		case UCALL_ABORT:
-			TEST_ASSERT(false, "%s at %s:%d", (const char *)uc.args[0],
+			TEST_ASSERT(false, "%s at %s:%ld", (const char *)uc.args[0],
 				    __FILE__, uc.args[1]);
 			/* NOT REACHED */
 		case UCALL_SYNC:
@@ -122,7 +122,7 @@ int main(int argc, char *argv[])
 
 		/* UCALL_SYNC is handled here.  */
 		TEST_ASSERT(!strcmp((const char *)uc.args[0], "hello") &&
-			    uc.args[1] == stage, "Unexpected register values vmexit #%lx, got %lx",
+			    uc.args[1] == stage, "Stage %d: Unexpected register values vmexit, got %lx",
 			    stage, (ulong)uc.args[1]);
 
 		state = vcpu_save_state(vm, VCPU_ID);
diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c b/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c
index 443a2b54645b..3edf3b517f9f 100644
--- a/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c
+++ b/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c
@@ -66,7 +66,7 @@ static void test_hv_cpuid(struct kvm_cpuid2 *hv_cpuid_entries,
 
 		TEST_ASSERT((entry->function >= 0x40000000) &&
 			    (entry->function <= 0x4000000A),
-			    "function %lx is our of supported range",
+			    "function %x is our of supported range",
 			    entry->function);
 
 		TEST_ASSERT(entry->index == 0,
diff --git a/tools/testing/selftests/kvm/x86_64/set_memory_region_test.c b/tools/testing/selftests/kvm/x86_64/set_memory_region_test.c
index 125aeab59ab6..f2efaa576794 100644
--- a/tools/testing/selftests/kvm/x86_64/set_memory_region_test.c
+++ b/tools/testing/selftests/kvm/x86_64/set_memory_region_test.c
@@ -61,8 +61,7 @@ static void *vcpu_worker(void *data)
 		    "Unexpected exit reason = %d", run->exit_reason);
 
 	cmd = get_ucall(vm, VCPU_ID, &uc);
-	TEST_ASSERT(cmd == UCALL_DONE, "Unexpected val in guest = %llu",
-		    uc.args[0]);
+	TEST_ASSERT(cmd == UCALL_DONE, "Unexpected val in guest = %lu", uc.args[0]);
 	return NULL;
 }
 
diff --git a/tools/testing/selftests/kvm/x86_64/state_test.c b/tools/testing/selftests/kvm/x86_64/state_test.c
index 164774206170..a4dc1ee59659 100644
--- a/tools/testing/selftests/kvm/x86_64/state_test.c
+++ b/tools/testing/selftests/kvm/x86_64/state_test.c
@@ -152,7 +152,7 @@ int main(int argc, char *argv[])
 
 		switch (get_ucall(vm, VCPU_ID, &uc)) {
 		case UCALL_ABORT:
-			TEST_ASSERT(false, "%s at %s:%d", (const char *)uc.args[0],
+			TEST_ASSERT(false, "%s at %s:%ld", (const char *)uc.args[0],
 				    __FILE__, uc.args[1]);
 			/* NOT REACHED */
 		case UCALL_SYNC:
@@ -165,7 +165,7 @@ int main(int argc, char *argv[])
 
 		/* UCALL_SYNC is handled here.  */
 		TEST_ASSERT(!strcmp((const char *)uc.args[0], "hello") &&
-			    uc.args[1] == stage, "Unexpected register values vmexit #%lx, got %lx",
+			    uc.args[1] == stage, "Stage %d: Unexpected register values vmexit, got %lx",
 			    stage, (ulong)uc.args[1]);
 
 		state = vcpu_save_state(vm, VCPU_ID);
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 e280f68f6365..8cd841ff6305 100644
--- a/tools/testing/selftests/kvm/x86_64/svm_vmcall_test.c
+++ b/tools/testing/selftests/kvm/x86_64/svm_vmcall_test.c
@@ -69,8 +69,7 @@ int main(int argc, char *argv[])
 		case UCALL_DONE:
 			goto done;
 		default:
-			TEST_ASSERT(false,
-				    "Unknown ucall 0x%x.", uc.cmd);
+			TEST_ASSERT(false, "Unknown ucall 0x%lx.", uc.cmd);
 		}
 	}
 done:
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 fe0734d9ef75..d9ca948d0b72 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
@@ -126,7 +126,7 @@ int main(int argc, char *argv[])
 
 		switch (get_ucall(vm, VCPU_ID, &uc)) {
 		case UCALL_ABORT:
-			TEST_ASSERT(false, "%s at %s:%d", (const char *)uc.args[0],
+			TEST_ASSERT(false, "%s at %s:%ld", (const char *)uc.args[0],
 				    __FILE__, uc.args[1]);
 			/* NOT REACHED */
 		case UCALL_SYNC:
diff --git a/tools/testing/selftests/kvm/x86_64/vmx_set_nested_state_test.c b/tools/testing/selftests/kvm/x86_64/vmx_set_nested_state_test.c
index 9ef7fab39d48..7962f2fe575d 100644
--- a/tools/testing/selftests/kvm/x86_64/vmx_set_nested_state_test.c
+++ b/tools/testing/selftests/kvm/x86_64/vmx_set_nested_state_test.c
@@ -212,7 +212,7 @@ void test_vmx_nested_state(struct kvm_vm *vm)
 	test_nested_state(vm, state);
 	vcpu_nested_state_get(vm, VCPU_ID, state);
 	TEST_ASSERT(state->size >= sizeof(*state) && state->size <= state_sz,
-		    "Size must be between %d and %d.  The size returned was %d.",
+		    "Size must be between %ld and %d.  The size returned was %d.",
 		    sizeof(*state), state_sz, state->size);
 	TEST_ASSERT(state->hdr.vmx.vmxon_pa == -1ull, "vmxon_pa must be -1ull.");
 	TEST_ASSERT(state->hdr.vmx.vmcs12_pa == -1ull, "vmcs_pa must be -1ull.");
-- 
2.21.1

Re: [PATCH 3/4] KVM: selftests: Enable printf format warnings for TEST_ASSERT

[Christian Borntraeger]

On 10.03.20 10:15, Andrew Jones wrote:
> Use the format attribute to enable printf format warnings, and
> then fix them all.

(for x86).

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

Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>

> ---
>  tools/testing/selftests/kvm/demand_paging_test.c          | 2 +-
>  tools/testing/selftests/kvm/include/test_util.h           | 3 ++-
>  tools/testing/selftests/kvm/lib/kvm_util.c                | 8 ++++----
>  tools/testing/selftests/kvm/x86_64/evmcs_test.c           | 4 ++--
>  tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c         | 2 +-
>  .../testing/selftests/kvm/x86_64/set_memory_region_test.c | 3 +--
>  tools/testing/selftests/kvm/x86_64/state_test.c           | 4 ++--
>  tools/testing/selftests/kvm/x86_64/svm_vmcall_test.c      | 3 +--
>  tools/testing/selftests/kvm/x86_64/vmx_dirty_log_test.c   | 2 +-
>  .../selftests/kvm/x86_64/vmx_set_nested_state_test.c      | 2 +-
>  10 files changed, 16 insertions(+), 17 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/demand_paging_test.c b/tools/testing/selftests/kvm/demand_paging_test.c
> index c1e326d3ed7f..47654071544c 100644
> --- a/tools/testing/selftests/kvm/demand_paging_test.c
> +++ b/tools/testing/selftests/kvm/demand_paging_test.c
> @@ -388,7 +388,7 @@ static void run_test(enum vm_guest_mode mode, bool use_uffd,
>  	 */
>  	TEST_ASSERT(guest_num_pages < vm_get_max_gfn(vm),
>  		    "Requested more guest memory than address space allows.\n"
> -		    "    guest pages: %lx max gfn: %lx vcpus: %d wss: %lx]\n",
> +		    "    guest pages: %lx max gfn: %x vcpus: %d wss: %lx]\n",
>  		    guest_num_pages, vm_get_max_gfn(vm), vcpus,
>  		    vcpu_memory_bytes);
>  
> diff --git a/tools/testing/selftests/kvm/include/test_util.h b/tools/testing/selftests/kvm/include/test_util.h
> index c921ea719ae0..07823740227b 100644
> --- a/tools/testing/selftests/kvm/include/test_util.h
> +++ b/tools/testing/selftests/kvm/include/test_util.h
> @@ -37,7 +37,8 @@ ssize_t test_read(int fd, void *buf, size_t count);
>  int test_seq_read(const char *path, char **bufp, size_t *sizep);
>  
>  void test_assert(bool exp, const char *exp_str,
> -		 const char *file, unsigned int line, const char *fmt, ...);
> +		 const char *file, unsigned int line, const char *fmt, ...)
> +		__attribute__((format(printf, 5, 6)));
>  
>  #define TEST_ASSERT(e, fmt, ...) \
>  	test_assert((e), #e, __FILE__, __LINE__, fmt, ##__VA_ARGS__)
> diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
> index 69a28a9211b4..b29c5d338555 100644
> --- a/tools/testing/selftests/kvm/lib/kvm_util.c
> +++ b/tools/testing/selftests/kvm/lib/kvm_util.c
> @@ -265,7 +265,7 @@ void kvm_vm_restart(struct kvm_vm *vmp, int perm)
>  		TEST_ASSERT(ret == 0, "KVM_SET_USER_MEMORY_REGION IOCTL failed,\n"
>  			    "  rc: %i errno: %i\n"
>  			    "  slot: %u flags: 0x%x\n"
> -			    "  guest_phys_addr: 0x%lx size: 0x%lx",
> +			    "  guest_phys_addr: 0x%llx size: 0x%llx",
>  			    ret, errno, region->region.slot,
>  			    region->region.flags,
>  			    region->region.guest_phys_addr,
> @@ -280,7 +280,7 @@ void kvm_vm_get_dirty_log(struct kvm_vm *vm, int slot, void *log)
>  
>  	ret = ioctl(vm->fd, KVM_GET_DIRTY_LOG, &args);
>  	TEST_ASSERT(ret == 0, "%s: KVM_GET_DIRTY_LOG failed: %s",
> -		    strerror(-ret));
> +		    __func__, strerror(-ret));
>  }
>  
>  void kvm_vm_clear_dirty_log(struct kvm_vm *vm, int slot, void *log,
> @@ -293,7 +293,7 @@ void kvm_vm_clear_dirty_log(struct kvm_vm *vm, int slot, void *log,
>  
>  	ret = ioctl(vm->fd, KVM_CLEAR_DIRTY_LOG, &args);
>  	TEST_ASSERT(ret == 0, "%s: KVM_CLEAR_DIRTY_LOG failed: %s",
> -		    strerror(-ret));
> +		    __func__, strerror(-ret));
>  }
>  
>  /*
> @@ -785,7 +785,7 @@ void vm_mem_region_move(struct kvm_vm *vm, uint32_t slot, uint64_t new_gpa)
>  	ret = ioctl(vm->fd, KVM_SET_USER_MEMORY_REGION, &region->region);
>  
>  	TEST_ASSERT(!ret, "KVM_SET_USER_MEMORY_REGION failed\n"
> -		    "ret: %i errno: %i slot: %u flags: 0x%x",
> +		    "ret: %i errno: %i slot: %u flags: 0x%lx",
>  		    ret, errno, slot, new_gpa);
>  }
>  
> diff --git a/tools/testing/selftests/kvm/x86_64/evmcs_test.c b/tools/testing/selftests/kvm/x86_64/evmcs_test.c
> index 185226c39c03..464a55217085 100644
> --- a/tools/testing/selftests/kvm/x86_64/evmcs_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/evmcs_test.c
> @@ -109,7 +109,7 @@ int main(int argc, char *argv[])
>  
>  		switch (get_ucall(vm, VCPU_ID, &uc)) {
>  		case UCALL_ABORT:
> -			TEST_ASSERT(false, "%s at %s:%d", (const char *)uc.args[0],
> +			TEST_ASSERT(false, "%s at %s:%ld", (const char *)uc.args[0],
>  				    __FILE__, uc.args[1]);
>  			/* NOT REACHED */
>  		case UCALL_SYNC:
> @@ -122,7 +122,7 @@ int main(int argc, char *argv[])
>  
>  		/* UCALL_SYNC is handled here.  */
>  		TEST_ASSERT(!strcmp((const char *)uc.args[0], "hello") &&
> -			    uc.args[1] == stage, "Unexpected register values vmexit #%lx, got %lx",
> +			    uc.args[1] == stage, "Stage %d: Unexpected register values vmexit, got %lx",
>  			    stage, (ulong)uc.args[1]);
>  
>  		state = vcpu_save_state(vm, VCPU_ID);
> diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c b/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c
> index 443a2b54645b..3edf3b517f9f 100644
> --- a/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c
> +++ b/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c
> @@ -66,7 +66,7 @@ static void test_hv_cpuid(struct kvm_cpuid2 *hv_cpuid_entries,
>  
>  		TEST_ASSERT((entry->function >= 0x40000000) &&
>  			    (entry->function <= 0x4000000A),
> -			    "function %lx is our of supported range",
> +			    "function %x is our of supported range",
>  			    entry->function);
>  
>  		TEST_ASSERT(entry->index == 0,
> diff --git a/tools/testing/selftests/kvm/x86_64/set_memory_region_test.c b/tools/testing/selftests/kvm/x86_64/set_memory_region_test.c
> index 125aeab59ab6..f2efaa576794 100644
> --- a/tools/testing/selftests/kvm/x86_64/set_memory_region_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/set_memory_region_test.c
> @@ -61,8 +61,7 @@ static void *vcpu_worker(void *data)
>  		    "Unexpected exit reason = %d", run->exit_reason);
>  
>  	cmd = get_ucall(vm, VCPU_ID, &uc);
> -	TEST_ASSERT(cmd == UCALL_DONE, "Unexpected val in guest = %llu",
> -		    uc.args[0]);
> +	TEST_ASSERT(cmd == UCALL_DONE, "Unexpected val in guest = %lu", uc.args[0]);
>  	return NULL;
>  }
>  
> diff --git a/tools/testing/selftests/kvm/x86_64/state_test.c b/tools/testing/selftests/kvm/x86_64/state_test.c
> index 164774206170..a4dc1ee59659 100644
> --- a/tools/testing/selftests/kvm/x86_64/state_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/state_test.c
> @@ -152,7 +152,7 @@ int main(int argc, char *argv[])
>  
>  		switch (get_ucall(vm, VCPU_ID, &uc)) {
>  		case UCALL_ABORT:
> -			TEST_ASSERT(false, "%s at %s:%d", (const char *)uc.args[0],
> +			TEST_ASSERT(false, "%s at %s:%ld", (const char *)uc.args[0],
>  				    __FILE__, uc.args[1]);
>  			/* NOT REACHED */
>  		case UCALL_SYNC:
> @@ -165,7 +165,7 @@ int main(int argc, char *argv[])
>  
>  		/* UCALL_SYNC is handled here.  */
>  		TEST_ASSERT(!strcmp((const char *)uc.args[0], "hello") &&
> -			    uc.args[1] == stage, "Unexpected register values vmexit #%lx, got %lx",
> +			    uc.args[1] == stage, "Stage %d: Unexpected register values vmexit, got %lx",
>  			    stage, (ulong)uc.args[1]);
>  
>  		state = vcpu_save_state(vm, VCPU_ID);
> 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 e280f68f6365..8cd841ff6305 100644
> --- a/tools/testing/selftests/kvm/x86_64/svm_vmcall_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/svm_vmcall_test.c
> @@ -69,8 +69,7 @@ int main(int argc, char *argv[])
>  		case UCALL_DONE:
>  			goto done;
>  		default:
> -			TEST_ASSERT(false,
> -				    "Unknown ucall 0x%x.", uc.cmd);
> +			TEST_ASSERT(false, "Unknown ucall 0x%lx.", uc.cmd);
>  		}
>  	}
>  done:
> 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 fe0734d9ef75..d9ca948d0b72 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
> @@ -126,7 +126,7 @@ int main(int argc, char *argv[])
>  
>  		switch (get_ucall(vm, VCPU_ID, &uc)) {
>  		case UCALL_ABORT:
> -			TEST_ASSERT(false, "%s at %s:%d", (const char *)uc.args[0],
> +			TEST_ASSERT(false, "%s at %s:%ld", (const char *)uc.args[0],
>  				    __FILE__, uc.args[1]);
>  			/* NOT REACHED */
>  		case UCALL_SYNC:
> diff --git a/tools/testing/selftests/kvm/x86_64/vmx_set_nested_state_test.c b/tools/testing/selftests/kvm/x86_64/vmx_set_nested_state_test.c
> index 9ef7fab39d48..7962f2fe575d 100644
> --- a/tools/testing/selftests/kvm/x86_64/vmx_set_nested_state_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/vmx_set_nested_state_test.c
> @@ -212,7 +212,7 @@ void test_vmx_nested_state(struct kvm_vm *vm)
>  	test_nested_state(vm, state);
>  	vcpu_nested_state_get(vm, VCPU_ID, state);
>  	TEST_ASSERT(state->size >= sizeof(*state) && state->size <= state_sz,
> -		    "Size must be between %d and %d.  The size returned was %d.",
> +		    "Size must be between %ld and %d.  The size returned was %d.",
>  		    sizeof(*state), state_sz, state->size);
>  	TEST_ASSERT(state->hdr.vmx.vmxon_pa == -1ull, "vmxon_pa must be -1ull.");
>  	TEST_ASSERT(state->hdr.vmx.vmcs12_pa == -1ull, "vmcs_pa must be -1ull.");
> 

[PATCH 4/4] KVM: selftests: Use consistent message for test skipping

[Andrew Jones]

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 tools/testing/selftests/kvm/demand_paging_test.c     |  4 ++--
 tools/testing/selftests/kvm/dirty_log_test.c         |  3 +--
 tools/testing/selftests/kvm/include/test_util.h      |  2 ++
 tools/testing/selftests/kvm/lib/assert.c             |  6 ++++--
 tools/testing/selftests/kvm/lib/kvm_util.c           |  2 +-
 tools/testing/selftests/kvm/lib/test_util.c          | 12 ++++++++++++
 tools/testing/selftests/kvm/lib/x86_64/svm.c         |  2 +-
 tools/testing/selftests/kvm/lib/x86_64/vmx.c         |  2 +-
 tools/testing/selftests/kvm/s390x/memop.c            |  2 +-
 tools/testing/selftests/kvm/s390x/sync_regs_test.c   |  2 +-
 .../selftests/kvm/x86_64/cr4_cpuid_sync_test.c       |  2 +-
 tools/testing/selftests/kvm/x86_64/evmcs_test.c      |  2 +-
 tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c    |  6 ++----
 .../testing/selftests/kvm/x86_64/mmio_warning_test.c |  4 ++--
 .../selftests/kvm/x86_64/platform_info_test.c        |  3 +--
 tools/testing/selftests/kvm/x86_64/sync_regs_test.c  |  4 ++--
 .../selftests/kvm/x86_64/vmx_set_nested_state_test.c |  2 +-
 tools/testing/selftests/kvm/x86_64/xss_msr_test.c    |  2 +-
 18 files changed, 37 insertions(+), 25 deletions(-)

diff --git a/tools/testing/selftests/kvm/demand_paging_test.c b/tools/testing/selftests/kvm/demand_paging_test.c
index 47654071544c..9077b5e81d10 100644
--- a/tools/testing/selftests/kvm/demand_paging_test.c
+++ b/tools/testing/selftests/kvm/demand_paging_test.c
@@ -664,8 +664,8 @@ int main(int argc, char *argv[])
 
 int main(void)
 {
-        printf("skip: Skipping userfaultfd test (missing __NR_userfaultfd)\n");
-        return KSFT_SKIP;
+	print_skip("__NR_userfaultfd must be present for userfaultfd test");
+	return KSFT_SKIP;
 }
 
 #endif /* __NR_userfaultfd */
diff --git a/tools/testing/selftests/kvm/dirty_log_test.c b/tools/testing/selftests/kvm/dirty_log_test.c
index 518a94a7a8b5..a64384bb7268 100644
--- a/tools/testing/selftests/kvm/dirty_log_test.c
+++ b/tools/testing/selftests/kvm/dirty_log_test.c
@@ -440,8 +440,7 @@ int main(int argc, char *argv[])
 	dirty_log_manual_caps =
 		kvm_check_cap(KVM_CAP_MANUAL_DIRTY_LOG_PROTECT2);
 	if (!dirty_log_manual_caps) {
-		fprintf(stderr, "KVM_CLEAR_DIRTY_LOG not available, "
-				"skipping tests\n");
+		print_skip("KVM_CLEAR_DIRTY_LOG not available");
 		exit(KSFT_SKIP);
 	}
 	dirty_log_manual_caps &= (KVM_DIRTY_LOG_MANUAL_PROTECT_ENABLE |
diff --git a/tools/testing/selftests/kvm/include/test_util.h b/tools/testing/selftests/kvm/include/test_util.h
index 07823740227b..1e1487a30402 100644
--- a/tools/testing/selftests/kvm/include/test_util.h
+++ b/tools/testing/selftests/kvm/include/test_util.h
@@ -32,6 +32,8 @@ static inline int _no_printf(const char *format, ...) { return 0; }
 #define pr_info(...) _no_printf(__VA_ARGS__)
 #endif
 
+void print_skip(const char *fmt, ...) __attribute__((format(printf, 1, 2)));
+
 ssize_t test_write(int fd, const void *buf, size_t count);
 ssize_t test_read(int fd, void *buf, size_t count);
 int test_seq_read(const char *path, char **bufp, size_t *sizep);
diff --git a/tools/testing/selftests/kvm/lib/assert.c b/tools/testing/selftests/kvm/lib/assert.c
index d1cf9f6e0e6b..5ebbd0d6b472 100644
--- a/tools/testing/selftests/kvm/lib/assert.c
+++ b/tools/testing/selftests/kvm/lib/assert.c
@@ -82,8 +82,10 @@ test_assert(bool exp, const char *exp_str,
 		}
 		va_end(ap);
 
-		if (errno == EACCES)
-			ksft_exit_skip("Access denied - Exiting.\n");
+		if (errno == EACCES) {
+			print_skip("Access denied - Exiting");
+			exit(KSFT_SKIP);
+		}
 		exit(254);
 	}
 
diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
index b29c5d338555..aa7697212267 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -92,7 +92,7 @@ static void vm_open(struct kvm_vm *vm, int perm)
 		exit(KSFT_SKIP);
 
 	if (!kvm_check_cap(KVM_CAP_IMMEDIATE_EXIT)) {
-		fprintf(stderr, "immediate_exit not available, skipping test\n");
+		print_skip("immediate_exit not available");
 		exit(KSFT_SKIP);
 	}
 
diff --git a/tools/testing/selftests/kvm/lib/test_util.c b/tools/testing/selftests/kvm/lib/test_util.c
index 1c0d45afdf36..26fb3d73dc74 100644
--- a/tools/testing/selftests/kvm/lib/test_util.c
+++ b/tools/testing/selftests/kvm/lib/test_util.c
@@ -7,6 +7,7 @@
 #include <stdlib.h>
 #include <ctype.h>
 #include <limits.h>
+#include <assert.h>
 #include "test_util.h"
 
 /*
@@ -69,3 +70,14 @@ struct timespec timespec_diff(struct timespec start, struct timespec end)
 
 	return temp;
 }
+
+void print_skip(const char *fmt, ...)
+{
+	va_list ap;
+
+	assert(fmt);
+	va_start(ap, fmt);
+	vprintf(fmt, ap);
+	va_end(ap);
+	puts(", skipping test");
+}
diff --git a/tools/testing/selftests/kvm/lib/x86_64/svm.c b/tools/testing/selftests/kvm/lib/x86_64/svm.c
index 6e05a8fc3fe0..c42401068373 100644
--- a/tools/testing/selftests/kvm/lib/x86_64/svm.c
+++ b/tools/testing/selftests/kvm/lib/x86_64/svm.c
@@ -154,7 +154,7 @@ void nested_svm_check_supported(void)
 		kvm_get_supported_cpuid_entry(0x80000001);
 
 	if (!(entry->ecx & CPUID_SVM)) {
-		fprintf(stderr, "nested SVM not enabled, skipping test\n");
+		print_skip("nested SVM not enabled");
 		exit(KSFT_SKIP);
 	}
 }
diff --git a/tools/testing/selftests/kvm/lib/x86_64/vmx.c b/tools/testing/selftests/kvm/lib/x86_64/vmx.c
index 7aaa99ca4dbc..ff0a657a42d3 100644
--- a/tools/testing/selftests/kvm/lib/x86_64/vmx.c
+++ b/tools/testing/selftests/kvm/lib/x86_64/vmx.c
@@ -381,7 +381,7 @@ void nested_vmx_check_supported(void)
 	struct kvm_cpuid_entry2 *entry = kvm_get_supported_cpuid_entry(1);
 
 	if (!(entry->ecx & CPUID_VMX)) {
-		fprintf(stderr, "nested VMX not enabled, skipping test\n");
+		print_skip("nested VMX not enabled");
 		exit(KSFT_SKIP);
 	}
 }
diff --git a/tools/testing/selftests/kvm/s390x/memop.c b/tools/testing/selftests/kvm/s390x/memop.c
index 9edaa9a134ce..9f49ead380ab 100644
--- a/tools/testing/selftests/kvm/s390x/memop.c
+++ b/tools/testing/selftests/kvm/s390x/memop.c
@@ -40,7 +40,7 @@ int main(int argc, char *argv[])
 
 	maxsize = kvm_check_cap(KVM_CAP_S390_MEM_OP);
 	if (!maxsize) {
-		fprintf(stderr, "CAP_S390_MEM_OP not supported -> skip test\n");
+		print_skip("CAP_S390_MEM_OP not supported");
 		exit(KSFT_SKIP);
 	}
 	if (maxsize > sizeof(mem1))
diff --git a/tools/testing/selftests/kvm/s390x/sync_regs_test.c b/tools/testing/selftests/kvm/s390x/sync_regs_test.c
index b705637ca14b..14d85fb2a1d9 100644
--- a/tools/testing/selftests/kvm/s390x/sync_regs_test.c
+++ b/tools/testing/selftests/kvm/s390x/sync_regs_test.c
@@ -79,7 +79,7 @@ int main(int argc, char *argv[])
 
 	cap = kvm_check_cap(KVM_CAP_SYNC_REGS);
 	if (!cap) {
-		fprintf(stderr, "CAP_SYNC_REGS not supported, skipping test\n");
+		print_skip("CAP_SYNC_REGS not supported");
 		exit(KSFT_SKIP);
 	}
 
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 003d1422705a..a646843137c7 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
@@ -72,7 +72,7 @@ int main(int argc, char *argv[])
 
 	entry = kvm_get_supported_cpuid_entry(1);
 	if (!(entry->ecx & X86_FEATURE_XSAVE)) {
-		printf("XSAVE feature not supported, skipping test\n");
+		print_skip("XSAVE feature not supported");
 		return 0;
 	}
 
diff --git a/tools/testing/selftests/kvm/x86_64/evmcs_test.c b/tools/testing/selftests/kvm/x86_64/evmcs_test.c
index 464a55217085..15241dc2ded0 100644
--- a/tools/testing/selftests/kvm/x86_64/evmcs_test.c
+++ b/tools/testing/selftests/kvm/x86_64/evmcs_test.c
@@ -87,7 +87,7 @@ int main(int argc, char *argv[])
 
 	if (!kvm_check_cap(KVM_CAP_NESTED_STATE) ||
 	    !kvm_check_cap(KVM_CAP_HYPERV_ENLIGHTENED_VMCS)) {
-		printf("capabilities not available, skipping test\n");
+		print_skip("capabilities not available");
 		exit(KSFT_SKIP);
 	}
 
diff --git a/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c b/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c
index 3edf3b517f9f..83323f3d7ca0 100644
--- a/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c
+++ b/tools/testing/selftests/kvm/x86_64/hyperv_cpuid.c
@@ -141,8 +141,7 @@ int main(int argc, char *argv[])
 
 	rv = kvm_check_cap(KVM_CAP_HYPERV_CPUID);
 	if (!rv) {
-		fprintf(stderr,
-			"KVM_CAP_HYPERV_CPUID not supported, skip test\n");
+		print_skip("KVM_CAP_HYPERV_CPUID not supported");
 		exit(KSFT_SKIP);
 	}
 
@@ -160,8 +159,7 @@ int main(int argc, char *argv[])
 	free(hv_cpuid_entries);
 
 	if (!kvm_check_cap(KVM_CAP_HYPERV_ENLIGHTENED_VMCS)) {
-		fprintf(stderr,
-			"Enlightened VMCS is unsupported, skip related test\n");
+		print_skip("Enlightened VMCS is unsupported");
 		goto vm_free;
 	}
 
diff --git a/tools/testing/selftests/kvm/x86_64/mmio_warning_test.c b/tools/testing/selftests/kvm/x86_64/mmio_warning_test.c
index 5350c2d6f736..e6480fd5c4bd 100644
--- a/tools/testing/selftests/kvm/x86_64/mmio_warning_test.c
+++ b/tools/testing/selftests/kvm/x86_64/mmio_warning_test.c
@@ -93,12 +93,12 @@ int main(void)
 	int warnings_before, warnings_after;
 
 	if (!is_intel_cpu()) {
-		printf("Must be run on an Intel CPU, skipping test\n");
+		print_skip("Must be run on an Intel CPU");
 		exit(KSFT_SKIP);
 	}
 
 	if (vm_is_unrestricted_guest(NULL)) {
-		printf("Unrestricted guest must be disabled, skipping test\n");
+		print_skip("Unrestricted guest must be disabled");
 		exit(KSFT_SKIP);
 	}
 
diff --git a/tools/testing/selftests/kvm/x86_64/platform_info_test.c b/tools/testing/selftests/kvm/x86_64/platform_info_test.c
index 54a960ff63aa..1e89688cbbbf 100644
--- a/tools/testing/selftests/kvm/x86_64/platform_info_test.c
+++ b/tools/testing/selftests/kvm/x86_64/platform_info_test.c
@@ -88,8 +88,7 @@ int main(int argc, char *argv[])
 
 	rv = kvm_check_cap(KVM_CAP_MSR_PLATFORM_INFO);
 	if (!rv) {
-		fprintf(stderr,
-			"KVM_CAP_MSR_PLATFORM_INFO not supported, skip test\n");
+		print_skip("KVM_CAP_MSR_PLATFORM_INFO not supported");
 		exit(KSFT_SKIP);
 	}
 
diff --git a/tools/testing/selftests/kvm/x86_64/sync_regs_test.c b/tools/testing/selftests/kvm/x86_64/sync_regs_test.c
index 5c8224256294..d672f0a473f8 100644
--- a/tools/testing/selftests/kvm/x86_64/sync_regs_test.c
+++ b/tools/testing/selftests/kvm/x86_64/sync_regs_test.c
@@ -91,11 +91,11 @@ int main(int argc, char *argv[])
 
 	cap = kvm_check_cap(KVM_CAP_SYNC_REGS);
 	if ((cap & TEST_SYNC_FIELDS) != TEST_SYNC_FIELDS) {
-		fprintf(stderr, "KVM_CAP_SYNC_REGS not supported, skipping test\n");
+		print_skip("KVM_CAP_SYNC_REGS not supported");
 		exit(KSFT_SKIP);
 	}
 	if ((cap & INVALID_SYNC_FIELD) != 0) {
-		fprintf(stderr, "The \"invalid\" field is not invalid, skipping test\n");
+		print_skip("The \"invalid\" field is not invalid");
 		exit(KSFT_SKIP);
 	}
 
diff --git a/tools/testing/selftests/kvm/x86_64/vmx_set_nested_state_test.c b/tools/testing/selftests/kvm/x86_64/vmx_set_nested_state_test.c
index 7962f2fe575d..54cdefdfb49d 100644
--- a/tools/testing/selftests/kvm/x86_64/vmx_set_nested_state_test.c
+++ b/tools/testing/selftests/kvm/x86_64/vmx_set_nested_state_test.c
@@ -228,7 +228,7 @@ int main(int argc, char *argv[])
 	have_evmcs = kvm_check_cap(KVM_CAP_HYPERV_ENLIGHTENED_VMCS);
 
 	if (!kvm_check_cap(KVM_CAP_NESTED_STATE)) {
-		printf("KVM_CAP_NESTED_STATE not available, skipping test\n");
+		print_skip("KVM_CAP_NESTED_STATE not available");
 		exit(KSFT_SKIP);
 	}
 
diff --git a/tools/testing/selftests/kvm/x86_64/xss_msr_test.c b/tools/testing/selftests/kvm/x86_64/xss_msr_test.c
index fc8328d8d5b0..3529376747c2 100644
--- a/tools/testing/selftests/kvm/x86_64/xss_msr_test.c
+++ b/tools/testing/selftests/kvm/x86_64/xss_msr_test.c
@@ -51,7 +51,7 @@ int main(int argc, char *argv[])
 		xss_supported = entry && !!(entry->eax & X86_FEATURE_XSAVES);
 	}
 	if (!xss_supported) {
-		printf("IA32_XSS is not supported by the vCPU, skipping test\n");
+		print_skip("IA32_XSS is not supported by the vCPU");
 		exit(KSFT_SKIP);
 	}
 
-- 
2.21.1

Re: [PATCH 0/4] KVM: selftests: Various cleanups and fixes

[Christian Borntraeger]

On 10.03.20 10:15, Andrew Jones wrote:
> 
> Andrew Jones (4):
>   fixup! selftests: KVM: SVM: Add vmcall test
>   KVM: selftests: Share common API documentation
>   KVM: selftests: Enable printf format warnings for TEST_ASSERT
>   KVM: selftests: Use consistent message for test skipping

This looks like a nice cleanup but this does not seem to apply
cleanly on kvm/master or linus/master. Which tree is this based on?

> 
>  tools/testing/selftests/kvm/.gitignore        |   5 +-
>  .../selftests/kvm/demand_paging_test.c        |   6 +-
>  tools/testing/selftests/kvm/dirty_log_test.c  |   3 +-
>  .../testing/selftests/kvm/include/kvm_util.h  | 100 ++++++++-
>  .../testing/selftests/kvm/include/test_util.h |   5 +-
>  .../selftests/kvm/lib/aarch64/processor.c     |  17 --
>  tools/testing/selftests/kvm/lib/assert.c      |   6 +-
>  tools/testing/selftests/kvm/lib/kvm_util.c    |  10 +-
>  .../selftests/kvm/lib/kvm_util_internal.h     |  48 +++++
>  .../selftests/kvm/lib/s390x/processor.c       |  74 -------
>  tools/testing/selftests/kvm/lib/test_util.c   |  12 ++
>  .../selftests/kvm/lib/x86_64/processor.c      | 196 ++++--------------
>  tools/testing/selftests/kvm/lib/x86_64/svm.c  |   2 +-
>  tools/testing/selftests/kvm/lib/x86_64/vmx.c  |   2 +-
>  tools/testing/selftests/kvm/s390x/memop.c     |   2 +-
>  .../selftests/kvm/s390x/sync_regs_test.c      |   2 +-
>  .../kvm/x86_64/cr4_cpuid_sync_test.c          |   2 +-
>  .../testing/selftests/kvm/x86_64/evmcs_test.c |   6 +-
>  .../selftests/kvm/x86_64/hyperv_cpuid.c       |   8 +-
>  .../selftests/kvm/x86_64/mmio_warning_test.c  |   4 +-
>  .../selftests/kvm/x86_64/platform_info_test.c |   3 +-
>  .../kvm/x86_64/set_memory_region_test.c       |   3 +-
>  .../testing/selftests/kvm/x86_64/state_test.c |   4 +-
>  .../selftests/kvm/x86_64/svm_vmcall_test.c    |   3 +-
>  .../selftests/kvm/x86_64/sync_regs_test.c     |   4 +-
>  .../selftests/kvm/x86_64/vmx_dirty_log_test.c |   2 +-
>  .../kvm/x86_64/vmx_set_nested_state_test.c    |   4 +-
>  .../selftests/kvm/x86_64/xss_msr_test.c       |   2 +-
>  28 files changed, 243 insertions(+), 292 deletions(-)
> 

Re: [PATCH 0/4] KVM: selftests: Various cleanups and fixes

[Andrew Jones]

On Tue, Mar 10, 2020 at 10:45:43AM +0100, Christian Borntraeger wrote:
> On 10.03.20 10:15, Andrew Jones wrote:
> > 
> > Andrew Jones (4):
> >   fixup! selftests: KVM: SVM: Add vmcall test
> >   KVM: selftests: Share common API documentation
> >   KVM: selftests: Enable printf format warnings for TEST_ASSERT
> >   KVM: selftests: Use consistent message for test skipping
> 
> This looks like a nice cleanup but this does not seem to apply
> cleanly on kvm/master or linus/master. Which tree is this based on?

This is based on kvm/queue. Sorry, I should have mentioned that in
the cover letter.

Thanks,
drew

> 
> > 
> >  tools/testing/selftests/kvm/.gitignore        |   5 +-
> >  .../selftests/kvm/demand_paging_test.c        |   6 +-
> >  tools/testing/selftests/kvm/dirty_log_test.c  |   3 +-
> >  .../testing/selftests/kvm/include/kvm_util.h  | 100 ++++++++-
> >  .../testing/selftests/kvm/include/test_util.h |   5 +-
> >  .../selftests/kvm/lib/aarch64/processor.c     |  17 --
> >  tools/testing/selftests/kvm/lib/assert.c      |   6 +-
> >  tools/testing/selftests/kvm/lib/kvm_util.c    |  10 +-
> >  .../selftests/kvm/lib/kvm_util_internal.h     |  48 +++++
> >  .../selftests/kvm/lib/s390x/processor.c       |  74 -------
> >  tools/testing/selftests/kvm/lib/test_util.c   |  12 ++
> >  .../selftests/kvm/lib/x86_64/processor.c      | 196 ++++--------------
> >  tools/testing/selftests/kvm/lib/x86_64/svm.c  |   2 +-
> >  tools/testing/selftests/kvm/lib/x86_64/vmx.c  |   2 +-
> >  tools/testing/selftests/kvm/s390x/memop.c     |   2 +-
> >  .../selftests/kvm/s390x/sync_regs_test.c      |   2 +-
> >  .../kvm/x86_64/cr4_cpuid_sync_test.c          |   2 +-
> >  .../testing/selftests/kvm/x86_64/evmcs_test.c |   6 +-
> >  .../selftests/kvm/x86_64/hyperv_cpuid.c       |   8 +-
> >  .../selftests/kvm/x86_64/mmio_warning_test.c  |   4 +-
> >  .../selftests/kvm/x86_64/platform_info_test.c |   3 +-
> >  .../kvm/x86_64/set_memory_region_test.c       |   3 +-
> >  .../testing/selftests/kvm/x86_64/state_test.c |   4 +-
> >  .../selftests/kvm/x86_64/svm_vmcall_test.c    |   3 +-
> >  .../selftests/kvm/x86_64/sync_regs_test.c     |   4 +-
> >  .../selftests/kvm/x86_64/vmx_dirty_log_test.c |   2 +-
> >  .../kvm/x86_64/vmx_set_nested_state_test.c    |   4 +-
> >  .../selftests/kvm/x86_64/xss_msr_test.c       |   2 +-
> >  28 files changed, 243 insertions(+), 292 deletions(-)
> > 
> 

Re: [PATCH 0/4] KVM: selftests: Various cleanups and fixes

[Christian Borntraeger]

I get the following with your patches.


In file included from s390x/sync_regs_test.c:21:
s390x/sync_regs_test.c: In function ‘compare_sregs’:
s390x/sync_regs_test.c:41:7: warning: format ‘%llx’ expects argument of type ‘long long unsigned int’, but argument 6 has type ‘__u32’ {aka ‘unsigned int’} [-Wformat=]
   41 |       "Register " #reg \
      |       ^~~~~~~~~~~
   42 |       " values did not match: 0x%llx, 0x%llx\n", \
   43 |       left->reg, right->reg)
      |       ~~~~~~~~~~~~~~~~~~~~~~
   44 | 
      |        
   45 | static void compare_regs(struct kvm_regs *left, struct kvm_sync_regs *right)
      | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   46 | {
      | ~      
   47 |  int i;
      |  ~~~~~~
   48 | 
      |        
   49 |  for (i = 0; i < 16; i++)
      |  ~~~~~~~~~~~~~~~~~~~~~~~~
   50 |   REG_COMPARE(gprs[i]);
      |   ~~~~~~~~~~~~~~~~~~~~~
   51 | }
      | ~      
   52 | 
      |        
   53 | static void compare_sregs(struct kvm_sregs *left, struct kvm_sync_regs *right)
      | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   54 | {
      | ~      
   55 |  int i;
      |  ~~~~~~
   56 | 
      |        
   57 |  for (i = 0; i < 16; i++)
      |  ~~~~~~~~~~~~~~~~~~~~~~~~
   58 |   REG_COMPARE(acrs[i]);
      |   ~~~~~~~~~~~~~~~~~~~
      |                   |
      |                   __u32 {aka unsigned int}
include/test_util.h:46:43: note: in definition of macro ‘TEST_ASSERT’
   46 |  test_assert((e), #e, __FILE__, __LINE__, fmt, ##__VA_ARGS__)
      |                                           ^~~
s390x/sync_regs_test.c:58:3: note: in expansion of macro ‘REG_COMPARE’
   58 |   REG_COMPARE(acrs[i]);
      |   ^~~~~~~~~~~
s390x/sync_regs_test.c:41:7: warning: format ‘%llx’ expects argument of type ‘long long unsigned int’, but argument 7 has type ‘__u32’ {aka ‘unsigned int’} [-Wformat=]
   41 |       "Register " #reg \
      |       ^~~~~~~~~~~
   42 |       " values did not match: 0x%llx, 0x%llx\n", \
   43 |       left->reg, right->reg)
      |                  ~~~~~~~~~~~
   44 | 
      |        
   45 | static void compare_regs(struct kvm_regs *left, struct kvm_sync_regs *right)
      | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   46 | {
      | ~      
   47 |  int i;
      |  ~~~~~~
   48 | 
      |        
   49 |  for (i = 0; i < 16; i++)
      |  ~~~~~~~~~~~~~~~~~~~~~~~~
   50 |   REG_COMPARE(gprs[i]);
      |   ~~~~~~~~~~~~~~~~~~~~~
   51 | }
      | ~      
   52 | 
      |        
   53 | static void compare_sregs(struct kvm_sregs *left, struct kvm_sync_regs *right)
      | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   54 | {
      | ~      
   55 |  int i;
      |  ~~~~~~
   56 | 
      |        
   57 |  for (i = 0; i < 16; i++)
      |  ~~~~~~~~~~~~~~~~~~~~~~~~
   58 |   REG_COMPARE(acrs[i]);
      |   ~~~~~~~~~~~~~~~~~~~
      |                   |
      |                   __u32 {aka unsigned int}
include/test_util.h:46:43: note: in definition of macro ‘TEST_ASSERT’
   46 |  test_assert((e), #e, __FILE__, __LINE__, fmt, ##__VA_ARGS__)
      |                                           ^~~
s390x/sync_regs_test.c:58:3: note: in expansion of macro ‘REG_COMPARE’
   58 |   REG_COMPARE(acrs[i]);
      |   ^~~~~~~~~~~
s390x/sync_regs_test.c: In function ‘main’:
s390x/sync_regs_test.c:158:7: warning: format ‘%llx’ expects argument of type ‘long long unsigned int’, but argument 6 has type ‘__u32’ {aka ‘unsigned int’} [-Wformat=]
  158 |       "acr0 sync regs value incorrect 0x%llx.",
      |       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  159 |       run->s.regs.acrs[0]);
      |       ~~~~~~~~~~~~~~~~~~~
      |                       |
      |                       __u32 {aka unsigned int}
include/test_util.h:46:43: note: in definition of macro ‘TEST_ASSERT’
   46 |  test_assert((e), #e, __FILE__, __LINE__, fmt, ##__VA_ARGS__)
      |                                           ^~~
s390x/sync_regs_test.c:158:44: note: format string is defined here
  158 |       "acr0 sync regs value incorrect 0x%llx.",
      |                                         ~~~^
      |                                            |
      |                                            long long unsigned int
      |      



On 10.03.20 12:58, Andrew Jones wrote:
> On Tue, Mar 10, 2020 at 10:45:43AM +0100, Christian Borntraeger wrote:
>> On 10.03.20 10:15, Andrew Jones wrote:
>>>
>>> Andrew Jones (4):
>>>   fixup! selftests: KVM: SVM: Add vmcall test
>>>   KVM: selftests: Share common API documentation
>>>   KVM: selftests: Enable printf format warnings for TEST_ASSERT
>>>   KVM: selftests: Use consistent message for test skipping
>>
>> This looks like a nice cleanup but this does not seem to apply
>> cleanly on kvm/master or linus/master. Which tree is this based on?
> 
> This is based on kvm/queue. Sorry, I should have mentioned that in
> the cover letter.
> 
> Thanks,
> drew
> 
>>
>>>
>>>  tools/testing/selftests/kvm/.gitignore        |   5 +-
>>>  .../selftests/kvm/demand_paging_test.c        |   6 +-
>>>  tools/testing/selftests/kvm/dirty_log_test.c  |   3 +-
>>>  .../testing/selftests/kvm/include/kvm_util.h  | 100 ++++++++-
>>>  .../testing/selftests/kvm/include/test_util.h |   5 +-
>>>  .../selftests/kvm/lib/aarch64/processor.c     |  17 --
>>>  tools/testing/selftests/kvm/lib/assert.c      |   6 +-
>>>  tools/testing/selftests/kvm/lib/kvm_util.c    |  10 +-
>>>  .../selftests/kvm/lib/kvm_util_internal.h     |  48 +++++
>>>  .../selftests/kvm/lib/s390x/processor.c       |  74 -------
>>>  tools/testing/selftests/kvm/lib/test_util.c   |  12 ++
>>>  .../selftests/kvm/lib/x86_64/processor.c      | 196 ++++--------------
>>>  tools/testing/selftests/kvm/lib/x86_64/svm.c  |   2 +-
>>>  tools/testing/selftests/kvm/lib/x86_64/vmx.c  |   2 +-
>>>  tools/testing/selftests/kvm/s390x/memop.c     |   2 +-
>>>  .../selftests/kvm/s390x/sync_regs_test.c      |   2 +-
>>>  .../kvm/x86_64/cr4_cpuid_sync_test.c          |   2 +-
>>>  .../testing/selftests/kvm/x86_64/evmcs_test.c |   6 +-
>>>  .../selftests/kvm/x86_64/hyperv_cpuid.c       |   8 +-
>>>  .../selftests/kvm/x86_64/mmio_warning_test.c  |   4 +-
>>>  .../selftests/kvm/x86_64/platform_info_test.c |   3 +-
>>>  .../kvm/x86_64/set_memory_region_test.c       |   3 +-
>>>  .../testing/selftests/kvm/x86_64/state_test.c |   4 +-
>>>  .../selftests/kvm/x86_64/svm_vmcall_test.c    |   3 +-
>>>  .../selftests/kvm/x86_64/sync_regs_test.c     |   4 +-
>>>  .../selftests/kvm/x86_64/vmx_dirty_log_test.c |   2 +-
>>>  .../kvm/x86_64/vmx_set_nested_state_test.c    |   4 +-
>>>  .../selftests/kvm/x86_64/xss_msr_test.c       |   2 +-
>>>  28 files changed, 243 insertions(+), 292 deletions(-)
>>>
>>
> 

Re: [PATCH 0/4] KVM: selftests: Various cleanups and fixes

[Christian Borntraeger]

On 10.03.20 13:29, Christian Borntraeger wrote:
> I get the following with your patches.

And those errors have been there before. 
I will provide fixups for sync_regs_test.c and reset.c. So you patch set really has value.


> 
> 
> In file included from s390x/sync_regs_test.c:21:
> s390x/sync_regs_test.c: In function ‘compare_sregs’:
> s390x/sync_regs_test.c:41:7: warning: format ‘%llx’ expects argument of type ‘long long unsigned int’, but argument 6 has type ‘__u32’ {aka ‘unsigned int’} [-Wformat=]
>    41 |       "Register " #reg \
>       |       ^~~~~~~~~~~
>    42 |       " values did not match: 0x%llx, 0x%llx\n", \
>    43 |       left->reg, right->reg)
>       |       ~~~~~~~~~~~~~~~~~~~~~~
>    44 | 
>       |        
>    45 | static void compare_regs(struct kvm_regs *left, struct kvm_sync_regs *right)
>       | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>    46 | {
>       | ~      
>    47 |  int i;
>       |  ~~~~~~
>    48 | 
>       |        
>    49 |  for (i = 0; i < 16; i++)
>       |  ~~~~~~~~~~~~~~~~~~~~~~~~
>    50 |   REG_COMPARE(gprs[i]);
>       |   ~~~~~~~~~~~~~~~~~~~~~
>    51 | }
>       | ~      
>    52 | 
>       |        
>    53 | static void compare_sregs(struct kvm_sregs *left, struct kvm_sync_regs *right)
>       | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>    54 | {
>       | ~      
>    55 |  int i;
>       |  ~~~~~~
>    56 | 
>       |        
>    57 |  for (i = 0; i < 16; i++)
>       |  ~~~~~~~~~~~~~~~~~~~~~~~~
>    58 |   REG_COMPARE(acrs[i]);
>       |   ~~~~~~~~~~~~~~~~~~~
>       |                   |
>       |                   __u32 {aka unsigned int}
> include/test_util.h:46:43: note: in definition of macro ‘TEST_ASSERT’
>    46 |  test_assert((e), #e, __FILE__, __LINE__, fmt, ##__VA_ARGS__)
>       |                                           ^~~
> s390x/sync_regs_test.c:58:3: note: in expansion of macro ‘REG_COMPARE’
>    58 |   REG_COMPARE(acrs[i]);
>       |   ^~~~~~~~~~~
> s390x/sync_regs_test.c:41:7: warning: format ‘%llx’ expects argument of type ‘long long unsigned int’, but argument 7 has type ‘__u32’ {aka ‘unsigned int’} [-Wformat=]
>    41 |       "Register " #reg \
>       |       ^~~~~~~~~~~
>    42 |       " values did not match: 0x%llx, 0x%llx\n", \
>    43 |       left->reg, right->reg)
>       |                  ~~~~~~~~~~~
>    44 | 
>       |        
>    45 | static void compare_regs(struct kvm_regs *left, struct kvm_sync_regs *right)
>       | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>    46 | {
>       | ~      
>    47 |  int i;
>       |  ~~~~~~
>    48 | 
>       |        
>    49 |  for (i = 0; i < 16; i++)
>       |  ~~~~~~~~~~~~~~~~~~~~~~~~
>    50 |   REG_COMPARE(gprs[i]);
>       |   ~~~~~~~~~~~~~~~~~~~~~
>    51 | }
>       | ~      
>    52 | 
>       |        
>    53 | static void compare_sregs(struct kvm_sregs *left, struct kvm_sync_regs *right)
>       | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>    54 | {
>       | ~      
>    55 |  int i;
>       |  ~~~~~~
>    56 | 
>       |        
>    57 |  for (i = 0; i < 16; i++)
>       |  ~~~~~~~~~~~~~~~~~~~~~~~~
>    58 |   REG_COMPARE(acrs[i]);
>       |   ~~~~~~~~~~~~~~~~~~~
>       |                   |
>       |                   __u32 {aka unsigned int}
> include/test_util.h:46:43: note: in definition of macro ‘TEST_ASSERT’
>    46 |  test_assert((e), #e, __FILE__, __LINE__, fmt, ##__VA_ARGS__)
>       |                                           ^~~
> s390x/sync_regs_test.c:58:3: note: in expansion of macro ‘REG_COMPARE’
>    58 |   REG_COMPARE(acrs[i]);
>       |   ^~~~~~~~~~~
> s390x/sync_regs_test.c: In function ‘main’:
> s390x/sync_regs_test.c:158:7: warning: format ‘%llx’ expects argument of type ‘long long unsigned int’, but argument 6 has type ‘__u32’ {aka ‘unsigned int’} [-Wformat=]
>   158 |       "acr0 sync regs value incorrect 0x%llx.",
>       |       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   159 |       run->s.regs.acrs[0]);
>       |       ~~~~~~~~~~~~~~~~~~~
>       |                       |
>       |                       __u32 {aka unsigned int}
> include/test_util.h:46:43: note: in definition of macro ‘TEST_ASSERT’
>    46 |  test_assert((e), #e, __FILE__, __LINE__, fmt, ##__VA_ARGS__)
>       |                                           ^~~
> s390x/sync_regs_test.c:158:44: note: format string is defined here
>   158 |       "acr0 sync regs value incorrect 0x%llx.",
>       |                                         ~~~^
>       |                                            |
>       |                                            long long unsigned int
>       |      
> 
> 
> 
> On 10.03.20 12:58, Andrew Jones wrote:
>> On Tue, Mar 10, 2020 at 10:45:43AM +0100, Christian Borntraeger wrote:
>>> On 10.03.20 10:15, Andrew Jones wrote:
>>>>
>>>> Andrew Jones (4):
>>>>   fixup! selftests: KVM: SVM: Add vmcall test
>>>>   KVM: selftests: Share common API documentation
>>>>   KVM: selftests: Enable printf format warnings for TEST_ASSERT
>>>>   KVM: selftests: Use consistent message for test skipping
>>>
>>> This looks like a nice cleanup but this does not seem to apply
>>> cleanly on kvm/master or linus/master. Which tree is this based on?
>>
>> This is based on kvm/queue. Sorry, I should have mentioned that in
>> the cover letter.
>>
>> Thanks,
>> drew
>>
>>>
>>>>
>>>>  tools/testing/selftests/kvm/.gitignore        |   5 +-
>>>>  .../selftests/kvm/demand_paging_test.c        |   6 +-
>>>>  tools/testing/selftests/kvm/dirty_log_test.c  |   3 +-
>>>>  .../testing/selftests/kvm/include/kvm_util.h  | 100 ++++++++-
>>>>  .../testing/selftests/kvm/include/test_util.h |   5 +-
>>>>  .../selftests/kvm/lib/aarch64/processor.c     |  17 --
>>>>  tools/testing/selftests/kvm/lib/assert.c      |   6 +-
>>>>  tools/testing/selftests/kvm/lib/kvm_util.c    |  10 +-
>>>>  .../selftests/kvm/lib/kvm_util_internal.h     |  48 +++++
>>>>  .../selftests/kvm/lib/s390x/processor.c       |  74 -------
>>>>  tools/testing/selftests/kvm/lib/test_util.c   |  12 ++
>>>>  .../selftests/kvm/lib/x86_64/processor.c      | 196 ++++--------------
>>>>  tools/testing/selftests/kvm/lib/x86_64/svm.c  |   2 +-
>>>>  tools/testing/selftests/kvm/lib/x86_64/vmx.c  |   2 +-
>>>>  tools/testing/selftests/kvm/s390x/memop.c     |   2 +-
>>>>  .../selftests/kvm/s390x/sync_regs_test.c      |   2 +-
>>>>  .../kvm/x86_64/cr4_cpuid_sync_test.c          |   2 +-
>>>>  .../testing/selftests/kvm/x86_64/evmcs_test.c |   6 +-
>>>>  .../selftests/kvm/x86_64/hyperv_cpuid.c       |   8 +-
>>>>  .../selftests/kvm/x86_64/mmio_warning_test.c  |   4 +-
>>>>  .../selftests/kvm/x86_64/platform_info_test.c |   3 +-
>>>>  .../kvm/x86_64/set_memory_region_test.c       |   3 +-
>>>>  .../testing/selftests/kvm/x86_64/state_test.c |   4 +-
>>>>  .../selftests/kvm/x86_64/svm_vmcall_test.c    |   3 +-
>>>>  .../selftests/kvm/x86_64/sync_regs_test.c     |   4 +-
>>>>  .../selftests/kvm/x86_64/vmx_dirty_log_test.c |   2 +-
>>>>  .../kvm/x86_64/vmx_set_nested_state_test.c    |   4 +-
>>>>  .../selftests/kvm/x86_64/xss_msr_test.c       |   2 +-
>>>>  28 files changed, 243 insertions(+), 292 deletions(-)
>>>>
>>>
>>
> 

Re: [PATCH 0/4] KVM: selftests: Various cleanups and fixes

[Andrew Jones]

On Tue, Mar 10, 2020 at 01:34:54PM +0100, Christian Borntraeger wrote:
> On 10.03.20 13:29, Christian Borntraeger wrote:
> > I get the following with your patches.
> 
> And those errors have been there before. 
> I will provide fixups for sync_regs_test.c and reset.c. So you patch set really has value.

Yeah, another thing I intended to say in the cover-letter, but forgot,
was that I hadn't tested on s390x, and that there was a chance
TEST_ASSERT warnings would be found.

Thanks for fixing them!

drew

> 
> 
> > 
> > 
> > In file included from s390x/sync_regs_test.c:21:
> > s390x/sync_regs_test.c: In function ‘compare_sregs’:
> > s390x/sync_regs_test.c:41:7: warning: format ‘%llx’ expects argument of type ‘long long unsigned int’, but argument 6 has type ‘__u32’ {aka ‘unsigned int’} [-Wformat=]
> >    41 |       "Register " #reg \
> >       |       ^~~~~~~~~~~
> >    42 |       " values did not match: 0x%llx, 0x%llx\n", \
> >    43 |       left->reg, right->reg)
> >       |       ~~~~~~~~~~~~~~~~~~~~~~
> >    44 | 
> >       |        
> >    45 | static void compare_regs(struct kvm_regs *left, struct kvm_sync_regs *right)
> >       | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >    46 | {
> >       | ~      
> >    47 |  int i;
> >       |  ~~~~~~
> >    48 | 
> >       |        
> >    49 |  for (i = 0; i < 16; i++)
> >       |  ~~~~~~~~~~~~~~~~~~~~~~~~
> >    50 |   REG_COMPARE(gprs[i]);
> >       |   ~~~~~~~~~~~~~~~~~~~~~
> >    51 | }
> >       | ~      
> >    52 | 
> >       |        
> >    53 | static void compare_sregs(struct kvm_sregs *left, struct kvm_sync_regs *right)
> >       | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >    54 | {
> >       | ~      
> >    55 |  int i;
> >       |  ~~~~~~
> >    56 | 
> >       |        
> >    57 |  for (i = 0; i < 16; i++)
> >       |  ~~~~~~~~~~~~~~~~~~~~~~~~
> >    58 |   REG_COMPARE(acrs[i]);
> >       |   ~~~~~~~~~~~~~~~~~~~
> >       |                   |
> >       |                   __u32 {aka unsigned int}
> > include/test_util.h:46:43: note: in definition of macro ‘TEST_ASSERT’
> >    46 |  test_assert((e), #e, __FILE__, __LINE__, fmt, ##__VA_ARGS__)
> >       |                                           ^~~
> > s390x/sync_regs_test.c:58:3: note: in expansion of macro ‘REG_COMPARE’
> >    58 |   REG_COMPARE(acrs[i]);
> >       |   ^~~~~~~~~~~
> > s390x/sync_regs_test.c:41:7: warning: format ‘%llx’ expects argument of type ‘long long unsigned int’, but argument 7 has type ‘__u32’ {aka ‘unsigned int’} [-Wformat=]
> >    41 |       "Register " #reg \
> >       |       ^~~~~~~~~~~
> >    42 |       " values did not match: 0x%llx, 0x%llx\n", \
> >    43 |       left->reg, right->reg)
> >       |                  ~~~~~~~~~~~
> >    44 | 
> >       |        
> >    45 | static void compare_regs(struct kvm_regs *left, struct kvm_sync_regs *right)
> >       | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >    46 | {
> >       | ~      
> >    47 |  int i;
> >       |  ~~~~~~
> >    48 | 
> >       |        
> >    49 |  for (i = 0; i < 16; i++)
> >       |  ~~~~~~~~~~~~~~~~~~~~~~~~
> >    50 |   REG_COMPARE(gprs[i]);
> >       |   ~~~~~~~~~~~~~~~~~~~~~
> >    51 | }
> >       | ~      
> >    52 | 
> >       |        
> >    53 | static void compare_sregs(struct kvm_sregs *left, struct kvm_sync_regs *right)
> >       | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >    54 | {
> >       | ~      
> >    55 |  int i;
> >       |  ~~~~~~
> >    56 | 
> >       |        
> >    57 |  for (i = 0; i < 16; i++)
> >       |  ~~~~~~~~~~~~~~~~~~~~~~~~
> >    58 |   REG_COMPARE(acrs[i]);
> >       |   ~~~~~~~~~~~~~~~~~~~
> >       |                   |
> >       |                   __u32 {aka unsigned int}
> > include/test_util.h:46:43: note: in definition of macro ‘TEST_ASSERT’
> >    46 |  test_assert((e), #e, __FILE__, __LINE__, fmt, ##__VA_ARGS__)
> >       |                                           ^~~
> > s390x/sync_regs_test.c:58:3: note: in expansion of macro ‘REG_COMPARE’
> >    58 |   REG_COMPARE(acrs[i]);
> >       |   ^~~~~~~~~~~
> > s390x/sync_regs_test.c: In function ‘main’:
> > s390x/sync_regs_test.c:158:7: warning: format ‘%llx’ expects argument of type ‘long long unsigned int’, but argument 6 has type ‘__u32’ {aka ‘unsigned int’} [-Wformat=]
> >   158 |       "acr0 sync regs value incorrect 0x%llx.",
> >       |       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >   159 |       run->s.regs.acrs[0]);
> >       |       ~~~~~~~~~~~~~~~~~~~
> >       |                       |
> >       |                       __u32 {aka unsigned int}
> > include/test_util.h:46:43: note: in definition of macro ‘TEST_ASSERT’
> >    46 |  test_assert((e), #e, __FILE__, __LINE__, fmt, ##__VA_ARGS__)
> >       |                                           ^~~
> > s390x/sync_regs_test.c:158:44: note: format string is defined here
> >   158 |       "acr0 sync regs value incorrect 0x%llx.",
> >       |                                         ~~~^
> >       |                                            |
> >       |                                            long long unsigned int
> >       |      
> > 
> > 
> > 
> > On 10.03.20 12:58, Andrew Jones wrote:
> >> On Tue, Mar 10, 2020 at 10:45:43AM +0100, Christian Borntraeger wrote:
> >>> On 10.03.20 10:15, Andrew Jones wrote:
> >>>>
> >>>> Andrew Jones (4):
> >>>>   fixup! selftests: KVM: SVM: Add vmcall test
> >>>>   KVM: selftests: Share common API documentation
> >>>>   KVM: selftests: Enable printf format warnings for TEST_ASSERT
> >>>>   KVM: selftests: Use consistent message for test skipping
> >>>
> >>> This looks like a nice cleanup but this does not seem to apply
> >>> cleanly on kvm/master or linus/master. Which tree is this based on?
> >>
> >> This is based on kvm/queue. Sorry, I should have mentioned that in
> >> the cover letter.
> >>
> >> Thanks,
> >> drew
> >>
> >>>
> >>>>
> >>>>  tools/testing/selftests/kvm/.gitignore        |   5 +-
> >>>>  .../selftests/kvm/demand_paging_test.c        |   6 +-
> >>>>  tools/testing/selftests/kvm/dirty_log_test.c  |   3 +-
> >>>>  .../testing/selftests/kvm/include/kvm_util.h  | 100 ++++++++-
> >>>>  .../testing/selftests/kvm/include/test_util.h |   5 +-
> >>>>  .../selftests/kvm/lib/aarch64/processor.c     |  17 --
> >>>>  tools/testing/selftests/kvm/lib/assert.c      |   6 +-
> >>>>  tools/testing/selftests/kvm/lib/kvm_util.c    |  10 +-
> >>>>  .../selftests/kvm/lib/kvm_util_internal.h     |  48 +++++
> >>>>  .../selftests/kvm/lib/s390x/processor.c       |  74 -------
> >>>>  tools/testing/selftests/kvm/lib/test_util.c   |  12 ++
> >>>>  .../selftests/kvm/lib/x86_64/processor.c      | 196 ++++--------------
> >>>>  tools/testing/selftests/kvm/lib/x86_64/svm.c  |   2 +-
> >>>>  tools/testing/selftests/kvm/lib/x86_64/vmx.c  |   2 +-
> >>>>  tools/testing/selftests/kvm/s390x/memop.c     |   2 +-
> >>>>  .../selftests/kvm/s390x/sync_regs_test.c      |   2 +-
> >>>>  .../kvm/x86_64/cr4_cpuid_sync_test.c          |   2 +-
> >>>>  .../testing/selftests/kvm/x86_64/evmcs_test.c |   6 +-
> >>>>  .../selftests/kvm/x86_64/hyperv_cpuid.c       |   8 +-
> >>>>  .../selftests/kvm/x86_64/mmio_warning_test.c  |   4 +-
> >>>>  .../selftests/kvm/x86_64/platform_info_test.c |   3 +-
> >>>>  .../kvm/x86_64/set_memory_region_test.c       |   3 +-
> >>>>  .../testing/selftests/kvm/x86_64/state_test.c |   4 +-
> >>>>  .../selftests/kvm/x86_64/svm_vmcall_test.c    |   3 +-
> >>>>  .../selftests/kvm/x86_64/sync_regs_test.c     |   4 +-
> >>>>  .../selftests/kvm/x86_64/vmx_dirty_log_test.c |   2 +-
> >>>>  .../kvm/x86_64/vmx_set_nested_state_test.c    |   4 +-
> >>>>  .../selftests/kvm/x86_64/xss_msr_test.c       |   2 +-
> >>>>  28 files changed, 243 insertions(+), 292 deletions(-)
> >>>>
> >>>
> >>
> > 
> 

Re: [PATCH 0/4] KVM: selftests: Various cleanups and fixes

[Peter Xu]

On Tue, Mar 10, 2020 at 10:15:52AM +0100, Andrew Jones wrote:
> 
> Andrew Jones (4):
>   fixup! selftests: KVM: SVM: Add vmcall test
>   KVM: selftests: Share common API documentation
>   KVM: selftests: Enable printf format warnings for TEST_ASSERT
>   KVM: selftests: Use consistent message for test skipping

Reviewed-by: Peter Xu <peterx@redhat.com>

I'll rebase my further tests against this too.  Thanks!

-- 
Peter Xu

Re: [PATCH 0/4] KVM: selftests: Various cleanups and fixes

[Paolo Bonzini]

On 11/03/20 20:29, Peter Xu wrote:
> On Tue, Mar 10, 2020 at 10:15:52AM +0100, Andrew Jones wrote:
>>
>> Andrew Jones (4):
>>   fixup! selftests: KVM: SVM: Add vmcall test
>>   KVM: selftests: Share common API documentation
>>   KVM: selftests: Enable printf format warnings for TEST_ASSERT
>>   KVM: selftests: Use consistent message for test skipping
> 
> Reviewed-by: Peter Xu <peterx@redhat.com>
> 
> I'll rebase my further tests against this too.  Thanks!
> 

Queued, thanks.

Paolo