[Qemu-devel] [PATCH 0/3] [PULL] qemu-kvm.git uq/master queue

[Qemu-devel] [PATCH 0/3] [PULL] qemu-kvm.git uq/master queue

[Gleb Natapov]

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(-)

[Qemu-devel] [PATCH 1/3] KVM: x86: fix typo in KVM_GET_XCRS

[Gleb Natapov]

From: Paolo Bonzini <pbonzini@redhat.com>

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

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Gleb Natapov <gleb@redhat.com>
---
 target-i386/kvm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 749aa09..27071e3 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -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;
         }
     }
-- 
1.8.4.rc3

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

[Gleb Natapov]

From: Jan Kiszka <jan.kiszka@siemens.com>

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 <jan.kiszka@siemens.com>
Acked-by: Alex Williamson <alex.williamson@redhat.com>
Signed-off-by: Gleb Natapov <gleb@redhat.com>
---
 hw/i386/kvm/pci-assign.c | 56 +++++++++---------------------------------------
 1 file changed, 10 insertions(+), 46 deletions(-)

diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c
index 011764f..4e65110 100644
--- a/hw/i386/kvm/pci-assign.c
+++ b/hw/i386/kvm/pci-assign.c
@@ -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;
 }
-- 
1.8.4.rc3

[Qemu-devel] [PATCH 3/3] kvm: Fix uninitialized cpuid_data

[Gleb Natapov]

From: Stefan Weil <sw@weilnetz.de>

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 <sw@weilnetz.de>
Signed-off-by: Gleb Natapov <gleb@redhat.com>
---
 target-i386/kvm.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 27071e3..1188482 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -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;
-- 
1.8.4.rc3

Re: [Qemu-devel] [PATCH 0/3] [PULL for 1.7?] qemu-kvm.git uq/master queue

[Stefan Weil]

Am 21.11.2013 14:28, schrieb Gleb Natapov:
> 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(-)
>

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

Stefan

Re: [Qemu-devel] [PATCH 0/3] [PULL for 1.7?] qemu-kvm.git uq/master queue

[Paolo Bonzini]

Il 21/11/2013 18:38, Stefan Weil ha scritto:
>> > 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(-)
>> >
> I think these patches should be included in QEMU 1.7, too.

Yes.

Paolo

Re: [Qemu-devel] [PATCH 0/3] [PULL for 1.7?] qemu-kvm.git uq/master queue

[Gleb Natapov]

On Thu, Nov 21, 2013 at 07:11:46PM +0100, Paolo Bonzini wrote:
> Il 21/11/2013 18:38, Stefan Weil ha scritto:
> >> > 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(-)
> >> >
> > I think these patches should be included in QEMU 1.7, too.
> 
> Yes.
> 
Yeah, forget to add 1.7 to the subject.

--
			Gleb.