Re: [PATCH] KVM: selftests: Compile code with warnings enabled

2019-05-17 Thread Thomas Huth
On 17/05/2019 04.45, Peter Xu wrote:
> Hi, Thomas,
> 
> On Thu, May 16, 2019 at 03:02:57PM +0200, Thomas Huth wrote:
>> So far the KVM selftests are compiled without any compiler warnings
>> enabled. That's quite bad, since we miss a lot of possible bugs this
>> way. Let's enable at least "-Wall" and some other useful warning flags
>> now.
>>
>> Signed-off-by: Thomas Huth 
>> ---
>>  This patch fixes most of the warnings in the x86 code already - but
>>  for some warnings, I was not quite sure (e.g. about the need for the
>>  kvm_get_supported_cpuid_entry(1) in some tests), so I did not touch
> 
> If you mean the two calls in state_test and evmcs_test I would agree
> they should be dropped directly.

Yes, that were the once I had in mind. I'll drop them in v2.

> Just to mention that the patch may not apply cleanly to kvm/queue now
> probably because the dirty-log-test.c touchup recently, so may need a
> rebase.  Otherwise it looks nice at least to me to have these checks.

Ok, thanks for the hint, I'll rebase and send a v2.

 Thomas


Re: [PATCH] KVM: selftests: Compile code with warnings enabled

2019-05-16 Thread Peter Xu
Hi, Thomas,

On Thu, May 16, 2019 at 03:02:57PM +0200, Thomas Huth wrote:
> So far the KVM selftests are compiled without any compiler warnings
> enabled. That's quite bad, since we miss a lot of possible bugs this
> way. Let's enable at least "-Wall" and some other useful warning flags
> now.
> 
> Signed-off-by: Thomas Huth 
> ---
>  This patch fixes most of the warnings in the x86 code already - but
>  for some warnings, I was not quite sure (e.g. about the need for the
>  kvm_get_supported_cpuid_entry(1) in some tests), so I did not touch

If you mean the two calls in state_test and evmcs_test I would agree
they should be dropped directly.

Just to mention that the patch may not apply cleanly to kvm/queue now
probably because the dirty-log-test.c touchup recently, so may need a
rebase.  Otherwise it looks nice at least to me to have these checks.

Thanks,

-- 
Peter Xu


[PATCH] KVM: selftests: Compile code with warnings enabled

2019-05-16 Thread Thomas Huth
So far the KVM selftests are compiled without any compiler warnings
enabled. That's quite bad, since we miss a lot of possible bugs this
way. Let's enable at least "-Wall" and some other useful warning flags
now.

Signed-off-by: Thomas Huth 
---
 This patch fixes most of the warnings in the x86 code already - but
 for some warnings, I was not quite sure (e.g. about the need for the
 kvm_get_supported_cpuid_entry(1) in some tests), so I did not touch
 that code yet. I also did not check aarch64 yet. I'd be glad if
 someone who knows these parts of the code could have a look at the
 warnings there.

 tools/testing/selftests/kvm/Makefile| 4 +++-
 tools/testing/selftests/kvm/dirty_log_test.c| 6 +-
 tools/testing/selftests/kvm/lib/kvm_util.c  | 3 ---
 tools/testing/selftests/kvm/lib/x86_64/processor.c  | 4 +---
 tools/testing/selftests/kvm/x86_64/cr4_cpuid_sync_test.c| 1 +
 tools/testing/selftests/kvm/x86_64/platform_info_test.c | 1 -
 tools/testing/selftests/kvm/x86_64/smm_test.c   | 3 +--
 .../selftests/kvm/x86_64/vmx_close_while_nested_test.c  | 5 +
 tools/testing/selftests/kvm/x86_64/vmx_tsc_adjust_test.c| 5 ++---
 9 files changed, 14 insertions(+), 18 deletions(-)

diff --git a/tools/testing/selftests/kvm/Makefile 
b/tools/testing/selftests/kvm/Makefile
index f8588cca2bef..93f344bb96af 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -32,7 +32,9 @@ LIBKVM += $(LIBKVM_$(UNAME_M))
 INSTALL_HDR_PATH = $(top_srcdir)/usr
 LINUX_HDR_PATH = $(INSTALL_HDR_PATH)/include/
 LINUX_TOOL_INCLUDE = $(top_srcdir)/tools/include
-CFLAGS += -O2 -g -std=gnu99 -fno-stack-protector -fno-PIE 
-I$(LINUX_TOOL_INCLUDE) -I$(LINUX_HDR_PATH) -Iinclude -I$(page_size;
 
@@ -1302,7 +1300,6 @@ void vcpu_sregs_set(struct kvm_vm *vm, uint32_t vcpuid, 
struct kvm_sregs *sregs)
 int _vcpu_sregs_set(struct kvm_vm *vm, uint32_t vcpuid, struct kvm_sregs 
*sregs)
 {
struct vcpu *vcpu = vcpu_find(vm, vcpuid);
-   int ret;
 
TEST_ASSERT(vcpu != NULL, "vcpu not found, vcpuid: %u", vcpuid);
 
diff --git a/tools/testing/selftests/kvm/lib/x86_64/processor.c 
b/tools/testing/selftests/kvm/lib/x86_64/processor.c
index dc7fae9fa424..21f3040d90cb 100644
--- a/tools/testing/selftests/kvm/lib/x86_64/processor.c
+++ b/tools/testing/selftests/kvm/lib/x86_64/processor.c
@@ -229,8 +229,6 @@ void sregs_dump(FILE *stream, struct kvm_sregs *sregs,
 
 void virt_pgd_alloc(struct kvm_vm *vm, uint32_t pgd_memslot)
 {
-   int rc;
-
TEST_ASSERT(vm->mode == VM_MODE_P52V48_4K, "Attempt to use "
"unknown or unsupported guest mode, mode: 0x%x", vm->mode);
 
@@ -549,7 +547,6 @@ vm_paddr_t addr_gva2gpa(struct kvm_vm *vm, vm_vaddr_t gva)
struct pageDirectoryPointerEntry *pdpe;
struct pageDirectoryEntry *pde;
struct pageTableEntry *pte;
-   void *hva;
 
TEST_ASSERT(vm->mode == VM_MODE_P52V48_4K, "Attempt to use "
"unknown or unsupported guest mode, mode: 0x%x", vm->mode);
@@ -582,6 +579,7 @@ vm_paddr_t addr_gva2gpa(struct kvm_vm *vm, vm_vaddr_t gva)
 unmapped_gva:
TEST_ASSERT(false, "No mapping for vm virtual address, "
"gva: 0x%lx", gva);
+   exit(EXIT_FAILURE);
 }
 
 static void kvm_setup_gdt(struct kvm_vm *vm, struct kvm_dtable *dt, int 
gdt_memslot,
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 7c2c4d4055a8..63cc9c3f5ab6 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
@@ -87,6 +87,7 @@ int main(int argc, char *argv[])
while (1) {
rc = _vcpu_run(vm, VCPU_ID);
 
+   TEST_ASSERT(rc == 0, "vcpu_run failed: %d\n", rc);
TEST_ASSERT(run->exit_reason == KVM_EXIT_IO,
"Unexpected exit reason: %u (%s),\n",
run->exit_reason,
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 eb3e7a838cb4..40050e44ec0a 100644
--- a/tools/testing/selftests/kvm/x86_64/platform_info_test.c
+++ b/tools/testing/selftests/kvm/x86_64/platform_info_test.c
@@ -81,7 +81,6 @@ static void test_msr_platform_info_disabled(struct kvm_vm *vm)
 int main(int argc, char *argv[])
 {
struct kvm_vm *vm;
-   struct kvm_run *state;
int rv;
uint64_t msr_platform_info;
 
diff --git a/tools/testing/selftests/kvm/x86_64/smm_test.c 
b/tools/testing/selftests/kvm/x86_64/smm_test.c
index fb8086964d83..4daf520bada1 100644
--- a/tools/testing/selftests/kvm/x86_64/smm_test.c
+++ b/tools/testing/selftests/kvm/x86_64/smm_test.c
@@ -87,7 +87,6 @@ void guest_code(struct vmx_pages *vmx_pages)
 
 int main(int argc, char *argv[])
 {
-   struct vmx_pages *vmx_pages = NULL;