qemu-kvm.git uq/master queue

GIT PULL New Gleb Natapov 3 participants 7 messages Nov 21, 2013 KVM

lore

Share & tools

lore

Gleb Natapov

author

2013-11-21 13:28 UTC #0 lore raw

The following changes since commit fc8ead74674b7129e8f31c2595c76658e5622197:

Merge remote-tracking branch 'qemu-kvm/uq/master' into staging (2013-10-18 10:03:24 -0700)

are available in the git repository at:

git://git.kernel.org/pub/scm/virt/kvm/qemu-kvm.git uq/master

for you to fetch changes up to ef4cbe14342c1f63b3c754e306218f004f4e26c4:

kvm: Fix uninitialized cpuid_data (2013-11-07 13:14:56 +0200)

----------------------------------------------------------------
Jan Kiszka (1):
pci-assign: Remove dead code for direct I/O region access from userspace

Paolo Bonzini (1):
KVM: x86: fix typo in KVM_GET_XCRS

Stefan Weil (1):
kvm: Fix uninitialized cpuid_data

 hw/i386/kvm/pci-assign.c | 56 +++++++++---------------------------------------
 target-i386/kvm.c        | 13 ++++-------
 2 files changed, 14 insertions(+), 55 deletions(-)

Gleb Natapov

author

2013-11-21 13:28 UTC #1 lore raw

PATCH 1/3 KVM: x86: fix typo in KVM_GET_XCRS

From: Paolo Bonzini <[email protected]>

Only the first item of the array was ever looked at. No practical effect, but still worth fixing.

Signed-off-by Paolo Bonzini <[email protected]>

Signed-off-by Gleb Natapov <[email protected]>

---
 target-i386/kvm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

target-i386/kvm.c +2 -2

index 749aa09..27071e3 100644
@@ -1314,8 +1314,8 @@ static int kvm_get_xcrs(X86CPU *cpu)
 
     for (i = 0; i < xcrs.nr_xcrs; i++) {
         /* Only support xcr0 now */
-        if (xcrs.xcrs[0].xcr == 0) {
-            env->xcr0 = xcrs.xcrs[0].value;
+        if (xcrs.xcrs[i].xcr == 0) {
+            env->xcr0 = xcrs.xcrs[i].value;
             break;
         }
     }

Signature · 1 line

1.8.4.rc3

Gleb Natapov

author

2013-11-21 13:28 UTC #2 lore raw

PATCH 2/3 pci-assign: Remove dead code for direct I/O region access from userspace

In reply to Gleb Natapov

From: Jan Kiszka <[email protected]>

This feature was already deprecated back then in qemu-kvm, ie. before pci-assign went upstream. assigned_dev_ioport_rw will never be invoked with resource_fd < 0.

Signed-off-by Jan Kiszka <[email protected]>

Acked-by Alex Williamson <[email protected]>

Signed-off-by Gleb Natapov <[email protected]>

---
 hw/i386/kvm/pci-assign.c | 56 +++++++++---------------------------------------
 1 file changed, 10 insertions(+), 46 deletions(-)

hw/i386/kvm/pci-assign.c +10 ~~-46~~

index 011764f..4e65110 100644
@@ -154,55 +154,19 @@ static uint64_t assigned_dev_ioport_rw(AssignedDevRegion *dev_region,
     uint64_t val = 0;
     int fd = dev_region->region->resource_fd;
 
-    if (fd >= 0) {
-        if (data) {
-            DEBUG("pwrite data=%" PRIx64 ", size=%d, e_phys=" TARGET_FMT_plx
-                  ", addr="TARGET_FMT_plx"\n", *data, size, addr, addr);
-            if (pwrite(fd, data, size, addr) != size) {
-                error_report("%s - pwrite failed %s",
-                             __func__, strerror(errno));
-            }
-        } else {
-            if (pread(fd, &val, size, addr) != size) {
-                error_report("%s - pread failed %s",
-                             __func__, strerror(errno));
-                val = (1UL << (size * 8)) - 1;
-            }
-            DEBUG("pread val=%" PRIx64 ", size=%d, e_phys=" TARGET_FMT_plx
-                  ", addr=" TARGET_FMT_plx "\n", val, size, addr, addr);
+    if (data) {
+        DEBUG("pwrite data=%" PRIx64 ", size=%d, e_phys=" TARGET_FMT_plx
+              ", addr="TARGET_FMT_plx"\n", *data, size, addr, addr);
+        if (pwrite(fd, data, size, addr) != size) {
+            error_report("%s - pwrite failed %s", __func__, strerror(errno));
         }
     } else {
-        uint32_t port = addr + dev_region->u.r_baseport;
-
-        if (data) {
-            DEBUG("out data=%" PRIx64 ", size=%d, e_phys=" TARGET_FMT_plx
-                  ", host=%x\n", *data, size, addr, port);
-            switch (size) {
-            case 1:
-                outb(*data, port);
-                break;
-            case 2:
-                outw(*data, port);
-                break;
-            case 4:
-                outl(*data, port);
-                break;
-            }
-        } else {
-            switch (size) {
-            case 1:
-                val = inb(port);
-                break;
-            case 2:
-                val = inw(port);
-                break;
-            case 4:
-                val = inl(port);
-                break;
-            }
-            DEBUG("in data=%" PRIx64 ", size=%d, e_phys=" TARGET_FMT_plx
-                  ", host=%x\n", val, size, addr, port);
+        if (pread(fd, &val, size, addr) != size) {
+            error_report("%s - pread failed %s", __func__, strerror(errno));
+            val = (1UL << (size * 8)) - 1;
         }
+        DEBUG("pread val=%" PRIx64 ", size=%d, e_phys=" TARGET_FMT_plx
+              ", addr=" TARGET_FMT_plx "\n", val, size, addr, addr);
     }
     return val;
 }

Signature · 1 line

1.8.4.rc3

Gleb Natapov

author

2013-11-21 13:28 UTC #3 lore raw

PATCH 3/3 kvm: Fix uninitialized cpuid_data

In reply to Gleb Natapov

From: Stefan Weil <[email protected]>

This error was reported by valgrind when running qemu-system-x86_64 with kvm:

==975== Conditional jump or move depends on uninitialised value(s)

==975==    at 0x521C38: cpuid_find_entry (kvm.c:176)
==975==    by 0x5235BA: kvm_arch_init_vcpu (kvm.c:686)
==975==    by 0x4D5175: kvm_init_vcpu (kvm-all.c:267)
==975==    by 0x45035B: qemu_kvm_cpu_thread_fn (cpus.c:858)
==975==    by 0xD361E0D: start_thread (pthread_create.c:311)
==975==    by 0xD65E9EC: clone (clone.S:113)

==975== Uninitialised value was created by a stack allocation
==975== at 0x5226E4: kvm_arch_init_vcpu (kvm.c:446)

Instead of adding more memset calls for parts of cpuid_data, the existing calls were removed and cpuid_data is now initialized completely in one call.

Signed-off-by Stefan Weil <[email protected]>

Signed-off-by Gleb Natapov <[email protected]>

---
 target-i386/kvm.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

target-i386/kvm.c +2 -7

index 27071e3..1188482 100644
@@ -456,11 +456,12 @@ int kvm_arch_init_vcpu(CPUState *cs)
     uint32_t signature[3];
     int r;
 
+    memset(&cpuid_data, 0, sizeof(cpuid_data));
+
     cpuid_i = 0;
 
     /* Paravirtualization CPUIDs */
     c = &cpuid_data.entries[cpuid_i++];
-    memset(c, 0, sizeof(*c));
     c->function = KVM_CPUID_SIGNATURE;
     if (!hyperv_enabled(cpu)) {
         memcpy(signature, "KVMKVMKVM\0\0\0", 12);

@@ -474,7 +475,6 @@ int kvm_arch_init_vcpu(CPUState *cs)
     c->edx = signature[2];
 
     c = &cpuid_data.entries[cpuid_i++];
-    memset(c, 0, sizeof(*c));
     c->function = KVM_CPUID_FEATURES;
     c->eax = env->features[FEAT_KVM];

@@ -483,13 +483,11 @@ int kvm_arch_init_vcpu(CPUState *cs)
         c->eax = signature[0];
 
         c = &cpuid_data.entries[cpuid_i++];
-        memset(c, 0, sizeof(*c));
         c->function = HYPERV_CPUID_VERSION;
         c->eax = 0x00001bbc;
         c->ebx = 0x00060001;
 
         c = &cpuid_data.entries[cpuid_i++];
-        memset(c, 0, sizeof(*c));
         c->function = HYPERV_CPUID_FEATURES;
         if (cpu->hyperv_relaxed_timing) {
             c->eax |= HV_X64_MSR_HYPERCALL_AVAILABLE;

@@ -500,7 +498,6 @@ int kvm_arch_init_vcpu(CPUState *cs)
         }
 
         c = &cpuid_data.entries[cpuid_i++];
-        memset(c, 0, sizeof(*c));
         c->function = HYPERV_CPUID_ENLIGHTMENT_INFO;
         if (cpu->hyperv_relaxed_timing) {
             c->eax |= HV_X64_RELAXED_TIMING_RECOMMENDED;

@@ -511,13 +508,11 @@ int kvm_arch_init_vcpu(CPUState *cs)
         c->ebx = cpu->hyperv_spinlock_attempts;
 
         c = &cpuid_data.entries[cpuid_i++];
-        memset(c, 0, sizeof(*c));
         c->function = HYPERV_CPUID_IMPLEMENT_LIMITS;
         c->eax = 0x40;
         c->ebx = 0x40;
 
         c = &cpuid_data.entries[cpuid_i++];
-        memset(c, 0, sizeof(*c));
         c->function = KVM_CPUID_SIGNATURE_NEXT;
         memcpy(signature, "KVMKVMKVM\0\0\0", 12);
         c->eax = 0;

Signature · 1 line

1.8.4.rc3

Stefan Weil

2013-11-21 17:38 UTC #4 lore raw

In reply to Gleb Natapov

Am 21.11.2013 14:28, schrieb Gleb Natapov:

Quoted 20 lines

The following changes since commit fc8ead74674b7129e8f31c2595c76658e5622197:
Merge remote-tracking branch 'qemu-kvm/uq/master' into staging (2013-10-18 10:03:24 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/virt/kvm/qemu-kvm.git uq/master
for you to fetch changes up to ef4cbe14342c1f63b3c754e306218f004f4e26c4:
kvm: Fix uninitialized cpuid_data (2013-11-07 13:14:56 +0200)
----------------------------------------------------------------
Jan Kiszka (1):
pci-assign: Remove dead code for direct I/O region access from userspace
Paolo Bonzini (1):
KVM: x86: fix typo in KVM_GET_XCRS

Quoted 7 lines

Stefan Weil (1):
kvm: Fix uninitialized cpuid_data

 hw/i386/kvm/pci-assign.c | 56 +++++++++---------------------------------------
 target-i386/kvm.c        | 13 ++++-------
 2 files changed, 14 insertions(+), 55 deletions(-)

I think these patches should be included in QEMU 1.7, too.

Stefan

Paolo Bonzini

maintainer

2013-11-21 18:11 UTC #5 lore raw

Il 21/11/2013 18:38, Stefan Weil ha scritto:

Quoted 6 lines

Quoted 6 lines
Quoted 6 lines
Jan Kiszka (1):
pci-assign: Remove dead code for direct I/O region access from userspace
Paolo Bonzini (1):
KVM: x86: fix typo in KVM_GET_XCRS

Quoted 8 lines

Quoted 7 lines
Quoted 7 lines
Stefan Weil (1):
kvm: Fix uninitialized cpuid_data
 hw/i386/kvm/pci-assign.c | 56 +++++++++---------------------------------------
 target-i386/kvm.c        | 13 ++++-------
 2 files changed, 14 insertions(+), 55 deletions(-)
I think these patches should be included in QEMU 1.7, too.

Yes.

Paolo

Gleb Natapov

author

2013-11-21 18:31 UTC #6 lore raw

Quoted 10 lines · Paolo Bonzini

Il 21/11/2013 18:38, Stefan Weil ha scritto:
Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
Jan Kiszka (1):
pci-assign: Remove dead code for direct I/O region access from userspace
Paolo Bonzini (1):
KVM: x86: fix typo in KVM_GET_XCRS
Stefan Weil (1):
kvm: Fix uninitialized cpuid_data

Quoted 8 lines

Quoted 5 lines
Quoted 4 lines
Quoted 4 lines
 hw/i386/kvm/pci-assign.c | 56 +++++++++---------------------------------------
 target-i386/kvm.c        | 13 ++++-------
 2 files changed, 14 insertions(+), 55 deletions(-)
I think these patches should be included in QEMU 1.7, too.
Yes.

Yeah, forget to add 1.7 to the subject.

--
Gleb.