[Qemu-devel] [PULL 0/9] valgrind/coverity/i386/s390x: memcheck false positives

[Qemu-devel] [PULL 0/9] valgrind/coverity/i386/s390x: memcheck false positives

[Christian Borntraeger]

Paolo, Peter,

here is an updated version of my valgrind tree. Please review and
consider for 2.3.

The following changes since commit af3ff19b48f0bbf3a8bd35c47460358e8c6ae5e5:

  Update version for v2.2.0-rc2 release (2014-11-18 18:00:58 +0000)

are available in the git repository at:

  git://github.com/borntraeger/qemu.git tags/memcheck

for you to fetch changes up to 113fb9793bf21a3219d305206c79024b0801d7ab:

  coverity/s390x: avoid false positive in kvm_irqchip_add_adapter_route (2014-11-20 22:10:58 +0100)

----------------------------------------------------------------
valgrind/coverity/i386/s390x: memcheck false positives

Let's avoid most memcheck false positives in KVM ioctls.

----------------------------------------------------------------
Christian Borntraeger (9):
      valgrind: avoid false positives in KVM_GET_DIRTY_LOG ioctl
      valgrind/i386: avoid false positives on KVM_SET_CLOCK ioctl
      valgrind/i386: avoid false positives on KVM_SET_PIT ioctl
      valgrind/i386: avoid false positives on KVM_SET_XCRS ioctl
      valgrind/i386: avoid false positives on KVM_SET_MSRS ioctl
      valgrind/i386: avoid false positives on KVM_GET_MSRS ioctl
      valgrind/i386: avoid false positives on KVM_SET_VCPU_EVENTS ioctl
      valgrind/s390x: avoid false positives on KVM_SET_FPU ioctl
      coverity/s390x: avoid false positive in kvm_irqchip_add_adapter_route

 hw/i386/kvm/clock.c |  3 +--
 hw/i386/kvm/i8254.c |  2 +-
 kvm-all.c           |  4 ++--
 target-i386/kvm.c   | 24 ++++++++++++++++++------
 target-s390x/kvm.c  |  2 +-
 5 files changed, 23 insertions(+), 12 deletions(-)

[Qemu-devel] [PULL 1/9] valgrind: avoid false positives in KVM_GET_DIRTY_LOG ioctl

[Christian Borntraeger]

struct kvm_dirty_log contains padding fields that trigger false
positives in valgrind. Let's use a designated initializer to avoid
false positives from valgrind/memcheck.

Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 kvm-all.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kvm-all.c b/kvm-all.c
index 44a5e72..b951320 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -400,7 +400,7 @@ static int kvm_physical_sync_dirty_bitmap(MemoryRegionSection *section)
 {
     KVMState *s = kvm_state;
     unsigned long size, allocated_size = 0;
-    KVMDirtyLog d;
+    KVMDirtyLog d = {};
     KVMSlot *mem;
     int ret = 0;
     hwaddr start_addr = section->offset_within_address_space;
-- 
1.9.3

[Qemu-devel] [PULL 2/9] valgrind/i386: avoid false positives on KVM_SET_CLOCK ioctl

[Christian Borntraeger]

kvm_clock_data contains pad fields. Let's use a designated
initializer to avoid false positives from valgrind/memcheck.

Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 hw/i386/kvm/clock.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/hw/i386/kvm/clock.c b/hw/i386/kvm/clock.c
index 58be2bd..efdf165 100644
--- a/hw/i386/kvm/clock.c
+++ b/hw/i386/kvm/clock.c
@@ -88,7 +88,7 @@ static void kvmclock_vm_state_change(void *opaque, int running,
     int ret;
 
     if (running) {
-        struct kvm_clock_data data;
+        struct kvm_clock_data data = {};
         uint64_t time_at_migration = kvmclock_current_nsec(s);
 
         s->clock_valid = false;
@@ -99,7 +99,6 @@ static void kvmclock_vm_state_change(void *opaque, int running,
         }
 
         data.clock = s->clock;
-        data.flags = 0;
         ret = kvm_vm_ioctl(kvm_state, KVM_SET_CLOCK, &data);
         if (ret < 0) {
             fprintf(stderr, "KVM_SET_CLOCK failed: %s\n", strerror(ret));
-- 
1.9.3

[Qemu-devel] [PULL 3/9] valgrind/i386: avoid false positives on KVM_SET_PIT ioctl

[Christian Borntraeger]

struct kvm_pit_state2 contains pad fields. Let's use a designated
initializer to avoid false positives from valgrind/memcheck.

Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 hw/i386/kvm/i8254.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/i386/kvm/i8254.c b/hw/i386/kvm/i8254.c
index 472af81..90eea10 100644
--- a/hw/i386/kvm/i8254.c
+++ b/hw/i386/kvm/i8254.c
@@ -138,7 +138,7 @@ static void kvm_pit_get(PITCommonState *pit)
 static void kvm_pit_put(PITCommonState *pit)
 {
     KVMPITState *s = KVM_PIT(pit);
-    struct kvm_pit_state2 kpit;
+    struct kvm_pit_state2 kpit = {};
     struct kvm_pit_channel_state *kchan;
     struct PITChannelState *sc;
     int i, ret;
-- 
1.9.3

[Qemu-devel] [PULL 4/9] valgrind/i386: avoid false positives on KVM_SET_XCRS ioctl

[Christian Borntraeger]

struct kvm_xcrs contains padding bytes. Let's use a designated
initializer to avoid false positives from valgrind/memcheck.

Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 target-i386/kvm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index ccf36e8..f42b4bf 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -1085,7 +1085,7 @@ static int kvm_put_xsave(X86CPU *cpu)
 static int kvm_put_xcrs(X86CPU *cpu)
 {
     CPUX86State *env = &cpu->env;
-    struct kvm_xcrs xcrs;
+    struct kvm_xcrs xcrs = {};
 
     if (!kvm_has_xcrs()) {
         return 0;
-- 
1.9.3

[Qemu-devel] [PULL 5/9] valgrind/i386: avoid false positives on KVM_SET_MSRS ioctl

[Christian Borntraeger]

struct kvm_msrs contains padding bytes. Let's use a designated
initializer on the info part to avoid false positives from
valgrind/memcheck. Do the same for generic MSRS, the TSC and
feature control.

We also need to zero out the reserved fields in the entries.
We do this in kvm_msr_entry_set as suggested by Paolo. This
avoids a big memset that a designated initializer on the
full structure would do.

Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 target-i386/kvm.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index f42b4bf..8b4a9e9 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -1152,6 +1152,7 @@ static void kvm_msr_entry_set(struct kvm_msr_entry *entry,
                               uint32_t index, uint64_t value)
 {
     entry->index = index;
+    entry->reserved = 0;
     entry->data = value;
 }
 
@@ -1170,7 +1171,9 @@ static int kvm_put_tscdeadline_msr(X86CPU *cpu)
 
     kvm_msr_entry_set(&msrs[0], MSR_IA32_TSCDEADLINE, env->tsc_deadline);
 
-    msr_data.info.nmsrs = 1;
+    msr_data.info = (struct kvm_msrs) {
+        .nmsrs = 1,
+    };
 
     return kvm_vcpu_ioctl(CPU(cpu), KVM_SET_MSRS, &msr_data);
 }
@@ -1190,7 +1193,11 @@ static int kvm_put_msr_feature_control(X86CPU *cpu)
 
     kvm_msr_entry_set(&msr_data.entry, MSR_IA32_FEATURE_CONTROL,
                       cpu->env.msr_ia32_feature_control);
-    msr_data.info.nmsrs = 1;
+
+    msr_data.info = (struct kvm_msrs) {
+        .nmsrs = 1,
+    };
+
     return kvm_vcpu_ioctl(CPU(cpu), KVM_SET_MSRS, &msr_data);
 }
 
@@ -1339,7 +1346,9 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
         }
     }
 
-    msr_data.info.nmsrs = n;
+    msr_data.info = (struct kvm_msrs) {
+        .nmsrs = n,
+    };
 
     return kvm_vcpu_ioctl(CPU(cpu), KVM_SET_MSRS, &msr_data);
 
-- 
1.9.3

[Qemu-devel] [PULL 6/9] valgrind/i386: avoid false positives on KVM_GET_MSRS ioctl

[Christian Borntraeger]

struct kvm_msrs contains a pad field. Let's use a designated
initializer on the info part to avoid false positives from
valgrind/memcheck.

Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 target-i386/kvm.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 8b4a9e9..7919d3e 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -1655,7 +1655,10 @@ static int kvm_get_msrs(X86CPU *cpu)
         }
     }
 
-    msr_data.info.nmsrs = n;
+    msr_data.info = (struct kvm_msrs) {
+        .nmsrs = n,
+    };
+
     ret = kvm_vcpu_ioctl(CPU(cpu), KVM_GET_MSRS, &msr_data);
     if (ret < 0) {
         return ret;
-- 
1.9.3

[Qemu-devel] [PULL 7/9] valgrind/i386: avoid false positives on KVM_SET_VCPU_EVENTS ioctl

[Christian Borntraeger]

struct kvm_vcpu_events contains reserved fields. Let's use a
designated initializer to avoid false positives in valgrind.

Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 target-i386/kvm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 7919d3e..43963c1 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -1884,7 +1884,7 @@ static int kvm_put_apic(X86CPU *cpu)
 static int kvm_put_vcpu_events(X86CPU *cpu, int level)
 {
     CPUX86State *env = &cpu->env;
-    struct kvm_vcpu_events events;
+    struct kvm_vcpu_events events = {};
 
     if (!kvm_has_vcpu_events()) {
         return 0;
-- 
1.9.3

[Qemu-devel] [PULL 8/9] valgrind/s390x: avoid false positives on KVM_SET_FPU ioctl

[Christian Borntraeger]

struct kvm_fpu contains an alignment padding on s390x. Let's use a
designated initializer to avoid false positives from valgrind/memcheck.

Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 target-s390x/kvm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index d247471..0c1da6e 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -208,7 +208,7 @@ int kvm_arch_put_registers(CPUState *cs, int level)
     CPUS390XState *env = &cpu->env;
     struct kvm_sregs sregs;
     struct kvm_regs regs;
-    struct kvm_fpu fpu;
+    struct kvm_fpu fpu = {};
     int r;
     int i;
 
-- 
1.9.3

[Qemu-devel] [PULL 9/9] coverity/s390x: avoid false positive in kvm_irqchip_add_adapter_route

[Christian Borntraeger]

Paolo Bonzini reported that Coverity reports an uninitialized pad value.
Let's use a designated initializer for kvm_irq_routing_entry to avoid
this false positive. This is similar to kvm_irqchip_add_msi_route and
other users of kvm_irq_routing_entry.

Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 kvm-all.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kvm-all.c b/kvm-all.c
index b951320..cfd49f0 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -1258,7 +1258,7 @@ static int kvm_irqchip_assign_irqfd(KVMState *s, int fd, int rfd, int virq,
 
 int kvm_irqchip_add_adapter_route(KVMState *s, AdapterInfo *adapter)
 {
-    struct kvm_irq_routing_entry kroute;
+    struct kvm_irq_routing_entry kroute = {};
     int virq;
 
     if (!kvm_gsi_routing_enabled()) {
-- 
1.9.3