Re: [PATCH 2/2] x86/cpu: Handle GUEST_MAXPHYADDR < HOST_MAXPHYADDR for hosts that don't support it

2020-07-09 Thread Mohammed Gamal
On Thu, 2020-07-09 at 11:44 +0200, Gerd Hoffmann wrote:
>   Hi,
> 
> > > (CCing libvir-list, and people who were included in the OVMF
> > > thread[1])
> > > 
> > > [1] 
> > > https://lore.kernel.org/qemu-devel/99779e9c-f05f-501b-b4be-ff719f140...@canonical.com/
> > > Also, it's important that we work with libvirt and management
> > > software to ensure they have appropriate APIs to choose what to
> > > do when a cluster has hosts with different MAXPHYADDR.
> > 
> > There's been so many complex discussions that it is hard to have
> > any
> > understanding of what we should be doing going forward. There's
> > enough
> > problems wrt phys bits, that I think we would benefit from a doc
> > that
> > outlines the big picture expectation for how to handle this in the
> > virt stack.
> 
> Well, the fundamental issue is not that hard actually.  We have three
> cases:
> 
> (1) GUEST_MAXPHYADDR == HOST_MAXPHYADDR
> 
> Everything is fine ;)
> 
> (2) GUEST_MAXPHYADDR < HOST_MAXPHYADDR
> 
> Mostly fine.  Some edge cases, like different page fault errors
> for
> addresses above GUEST_MAXPHYADDR and below
> HOST_MAXPHYADDR.  Which I
> think Mohammed fixed in the kernel recently.
> 
> (3) GUEST_MAXPHYADDR > HOST_MAXPHYADDR
> 
> Broken.  If the guest uses addresses above HOST_MAXPHYADDR
> everything
> goes south.
> 
> The (2) case isn't much of a problem.  We only need to figure
> whenever
> we want qemu allow this unconditionally (current state) or only in
> case
> the kernel fixes are present (state with this patch applied if I read
> it
> correctly).
> 
> The (3) case is the reason why guest firmware never ever uses
> GUEST_MAXPHYADDR and goes with very conservative heuristics instead,
> which in turn leads to the consequences discussed at length in the
> OVMF thread linked above.
> 
> Ideally we would simply outlaw (3), but it's hard for backward
> compatibility reasons.  Second best solution is a flag somewhere
> (msr, cpuid, ...) telling the guest firmware "you can use
> GUEST_MAXPHYADDR, we guarantee it is <= HOST_MAXPHYADDR".

Problem is GUEST_MAXPHYADDR > HOST_MAXPHYADDR is actually a supported
configuration on some setups. Namely when memory encryption is enabled
on AMD CPUs[1].

> 
> > As mentioned in the thread quoted above, using host_phys_bits is a
> > obvious thing to do when the user requested "-cpu host".
> > 
> > The harder issue is how to handle other CPU models. I had suggested
> > we should try associating a phys bits value with them, which would
> > probably involve creating Client/Server variants for all our CPU
> > models which don't currently have it. I still think that's worth
> > exploring as a strategy and with versioned CPU models we should
> > be ok wrt back compatibility with that approach.
> 
> Yep, better defaults for GUEST_MAXPHYADDR would be good too, but that
> is a separate (although related) discussion.
> 
> take care,
>   Gerd
> 
[1] - https://lkml.org/lkml/2020/6/19/2371




[PATCH 1/2] kvm: Add support for KVM_CAP_HAS_SMALLER_MAXPHYADDR

2020-06-19 Thread Mohammed Gamal
This adds the KVM_CAP_HAS_SMALLER_MAXPHYADDR capability
and a helper function to check for this capability.
This will allow QEMU to decide to what to do if the host
CPU can't handle GUEST_MAXPHYADDR < HOST_MAXPHYADDR properly.

Signed-off-by: Mohammed Gamal 
---
 linux-headers/linux/kvm.h | 1 +
 target/i386/kvm.c | 5 +
 target/i386/kvm_i386.h| 1 +
 3 files changed, 7 insertions(+)

diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
index 9804495a46..9eb61a303f 100644
--- a/linux-headers/linux/kvm.h
+++ b/linux-headers/linux/kvm.h
@@ -1017,6 +1017,7 @@ struct kvm_ppc_resize_hpt {
 #define KVM_CAP_S390_VCPU_RESETS 179
 #define KVM_CAP_S390_PROTECTED 180
 #define KVM_CAP_PPC_SECURE_GUEST 181
+#define KVM_CAP_SMALLER_MAXPHYADDR 184
 
 #ifdef KVM_CAP_IRQ_ROUTING
 
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index b3c13cb898..01100dbf20 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -136,6 +136,11 @@ bool kvm_has_smm(void)
 return kvm_check_extension(kvm_state, KVM_CAP_X86_SMM);
 }
 
+bool kvm_has_smaller_maxphyaddr(void)
+{
+return kvm_check_extension(kvm_state, KVM_CAP_SMALLER_MAXPHYADDR);
+}
+
 bool kvm_has_adjust_clock_stable(void)
 {
 int ret = kvm_check_extension(kvm_state, KVM_CAP_ADJUST_CLOCK);
diff --git a/target/i386/kvm_i386.h b/target/i386/kvm_i386.h
index 00bde7acaf..513f8eebbb 100644
--- a/target/i386/kvm_i386.h
+++ b/target/i386/kvm_i386.h
@@ -34,6 +34,7 @@
 
 bool kvm_allows_irq0_override(void);
 bool kvm_has_smm(void);
+bool kvm_has_smaller_maxphyaddr(void);
 bool kvm_has_adjust_clock_stable(void);
 bool kvm_has_exception_payload(void);
 void kvm_synchronize_all_tsc(void);
-- 
2.26.2




[PATCH 2/2] x86/cpu: Handle GUEST_MAXPHYADDR < HOST_MAXPHYADDR for hosts that don't support it

2020-06-19 Thread Mohammed Gamal
If the CPU doesn't support GUEST_MAXPHYADDR < HOST_MAXPHYADDR we
let QEMU choose to use the host MAXPHYADDR and print a warning to the
user.

Signed-off-by: Mohammed Gamal 
---
 target/i386/cpu.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index b1b311baa2..91c57117ce 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -6589,6 +6589,17 @@ static void x86_cpu_realizefn(DeviceState *dev, Error 
**errp)
 uint32_t host_phys_bits = x86_host_phys_bits();
 static bool warned;
 
+   /*
+* If host doesn't support setting physical bits on the guest,
+* report it and return
+*/
+   if (cpu->phys_bits < host_phys_bits &&
+   !kvm_has_smaller_maxphyaddr()) {
+   warn_report("Host doesn't support setting smaller phys-bits."
+   " Using host phys-bits\n");
+   cpu->phys_bits = host_phys_bits;
+   }
+
 /* Print a warning if the user set it to a value that's not the
  * host value.
  */
-- 
2.26.2




[PATCH 0/2] kvm: x86/cpu: Support guest MAXPHYADDR < host MAXPHYADDR

2020-06-19 Thread Mohammed Gamal
This series adds support for KVM_CAP_HAS_SMALLER_MAXPHYADDR to QEMU.
Some processors might not handle GUEST_MAXPHYADDR < HOST_MAXPHYADDR in
the expected manner. Hence, we added KVM_CAP_HAS_SMALLER_MAXPHYADDR to
KVM.
In this implementation KVM is queried for KVM_CAP_HAS_SMALLER_MAXPHYADDR
when setting vCPU physical bits, and if the CPU doesn't support 
KVM_CAP_HAS_SMALLER_MAXPHYADDR the ,phys-bits is ignore and host phyiscal
bits are used. A warning message is printed to the user.

Mohammed Gamal (2):
  kvm: Add support for KVM_CAP_HAS_SMALLER_MAXPHYADDR
  x86/cpu: Handle GUEST_MAXPHYADDR < HOST_MAXPHYADDR for hosts that
don't support it

 linux-headers/linux/kvm.h |  1 +
 target/i386/cpu.c | 11 +++
 target/i386/kvm.c |  5 +
 target/i386/kvm_i386.h|  1 +
 4 files changed, 18 insertions(+)

-- 
2.26.2




[Qemu-devel] [Bug 1673130] Re: qemu 2.7.0 receives SIGABRT in qemu_coroutine_enter()

2017-12-06 Thread Mohammed Gamal
Fixed by commit 528f449f590829b53ea01ed91817a695b540421d

** Changed in: qemu
   Status: New => Fix Released

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1673130

Title:
  qemu 2.7.0 receives SIGABRT in qemu_coroutine_enter()

Status in QEMU:
  Fix Released

Bug description:
  I've been experiencing frequent SIGABRTs (in addition to segfaults in
  #1671876) lately with qemu 2.7.0 running Ubuntu 16.04 guests. The
  crash usually happens in qemu_coroutine_enter(). I haven't seen this
  so far with any other guests or distros.

  Here is one stack trace I obtained
  --
  (gdb) bt
  #0  0x7fd7cc676067 in __GI_raise (sig=sig@entry=6) at 
../nptl/sysdeps/unix/sysv/linux/raise.c:56
  #1  0x7fd7cc677448 in __GI_abort () at abort.c:89
  #2  0x556aed247b6c in qemu_coroutine_enter (co=0x7fd574300df0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:113
  #3  0x556aed247e55 in qemu_co_queue_run_restart (co=0x7fd574300ce0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
  #4  0x556aed2479a9 in qemu_coroutine_enter (co=0x7fd574300ce0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
  #5  0x556aed247e74 in qemu_co_queue_run_restart (co=0x7fd589111670) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
  #6  0x556aed2479a9 in qemu_coroutine_enter (co=0x7fd589111670) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
  #7  0x556aed247e74 in qemu_co_queue_run_restart (co=0x7fd57430dba0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
  #8  0x556aed2479a9 in qemu_coroutine_enter (co=0x7fd57430dba0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
  #9  0x556aed247e74 in qemu_co_queue_run_restart (co=0x7fd589119130) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
  #10 0x556aed2479a9 in qemu_coroutine_enter (co=0x7fd589119130) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
  #11 0x556aed247e74 in qemu_co_queue_run_restart (co=0x7fd589117410) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
  #12 0x556aed2479a9 in qemu_coroutine_enter (co=0x7fd589117410) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
  #13 0x556aed247e74 in qemu_co_queue_run_restart (co=0x7fd577f00e00) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
  #14 0x556aed2479a9 in qemu_coroutine_enter (co=0x7fd577f00e00) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
  #15 0x556aed247fa0 in qemu_co_enter_next 
(queue=queue@entry=0x556aef34e5e0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:106
  #16 0x556aed1e6060 in timer_cb (blk=0x556aef34e590, is_write=) at /build/pb-qemu-pssKUp/pb-qemu-2.7.0/block/throttle-groups.c:400
  #17 0x556aed1a3615 in timerlist_run_timers (timer_list=0x556aef3bad40) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/qemu-timer.c:528
  #18 0x556aed1a3679 in timerlistgroup_run_timers 
(tlg=tlg@entry=0x556af0738758) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/qemu-timer.c:564
  #19 0x556aed1a3f47 in aio_dispatch (ctx=ctx@entry=0x556af0738610) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/aio-posix.c:357
  #20 0x556aed1a40e8 in aio_poll (ctx=0x556af0738610, blocking=) at /build/pb-qemu-pssKUp/pb-qemu-2.7.0/aio-posix.c:479
  #21 0x556aed005c79 in iothread_run (opaque=0x556af07383c0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/iothread.c:46
  #22 0x7fd7cc9f40a4 in start_thread (arg=0x7fd7aafff700) at 
pthread_create.c:403
  #23 0x7fd7cc72962d in clone () at 
../sysdeps/unix/sysv/linux/x86_64/clone.S:111
  --

  The code crashes here
  --
  void qemu_coroutine_enter(Coroutine *co)
  {
  Coroutine *self = qemu_coroutine_self();
  CoroutineAction ret;

  trace_qemu_coroutine_enter(self, co, co->entry_arg);

  if (co->caller) {
  fprintf(stderr, "Co-routine re-entered recursively\n");
  abort();  <--- Code aborts here   

  }

  [...]
  }
  --

  Debugging further we see:
  --
  (gdb) frame 2
  #2  0x556aed247b6c in qemu_coroutine_enter (co=0x7fd574300df0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:113
  113/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c: No such 
file or directory.
  (gdb) print *co
  $1 = {entry = 0x7fd793e95a58, entry_arg = 0x1, caller = 0x7fd793e95a38, 
pool_next = {sle_next = 0x10}, co_queue_wakeup = {sqh_fir

[Qemu-devel] [Bug 1671876] Re: qemu 2.7.0 segfaults in qemu_co_queue_run_restart()

2017-12-06 Thread Mohammed Gamal
Fixed by commit 528f449f590829b53ea01ed91817a695b540421d

** Changed in: qemu
   Status: New => Fix Released

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1671876

Title:
  qemu 2.7.0 segfaults in qemu_co_queue_run_restart()

Status in QEMU:
  Fix Released

Bug description:
  Hi,

  I've been experiencing frequent segfaults lately with qemu 2.7.0
  running Ubuntu 16.04 guests. The crash usually happens in
  qemu_co_queue_run_restart(). I haven't seen this so far with any other
  guests or distros.

  Here is one back trace I obtained from one of the crashing VMs.

  --
  (gdb) bt
  #0  qemu_co_queue_run_restart (co=0x7fba8ff05aa0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:59
  #1  0x55c1656f39a9 in qemu_coroutine_enter (co=0x7fba8ff05aa0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
  #2  0x55c1656f3e74 in qemu_co_queue_run_restart (co=0x7fba8dd20430) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
  #3  0x55c1656f39a9 in qemu_coroutine_enter (co=0x7fba8dd20430) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
  #4  0x55c1656f3e74 in qemu_co_queue_run_restart (co=0x7fba8dd14ea0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
  #5  0x55c1656f39a9 in qemu_coroutine_enter (co=0x7fba8dd14ea0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
  #6  0x55c1656f3e74 in qemu_co_queue_run_restart (co=0x7fba80c11dc0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
  #7  0x55c1656f39a9 in qemu_coroutine_enter (co=0x7fba80c11dc0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
  #8  0x55c1656f3e74 in qemu_co_queue_run_restart (co=0x7fba8dd0bd70) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
  #9  0x55c1656f39a9 in qemu_coroutine_enter (co=0x7fba8dd0bd70) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
  #10 0x55c1656f3fa0 in qemu_co_enter_next 
(queue=queue@entry=0x55c1669e75e0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:106
  #11 0x55c165692060 in timer_cb (blk=0x55c1669e7590, is_write=) at /build/pb-qemu-pssKUp/pb-qemu-2.7.0/block/throttle-groups.c:400
  #12 0x55c16564f615 in timerlist_run_timers (timer_list=0x55c166a53e80) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/qemu-timer.c:528
  #13 0x55c16564f679 in timerlistgroup_run_timers 
(tlg=tlg@entry=0x55c167c81cf8) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/qemu-timer.c:564
  #14 0x55c16564ff47 in aio_dispatch (ctx=ctx@entry=0x55c167c81bb0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/aio-posix.c:357
  #15 0x55c1656500e8 in aio_poll (ctx=0x55c167c81bb0, blocking=) at /build/pb-qemu-pssKUp/pb-qemu-2.7.0/aio-posix.c:479
  #16 0x55c1654b1c79 in iothread_run (opaque=0x55c167c81960) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/iothread.c:46
  #17 0x7fbc4b64f0a4 in allocate_stack (stack=, 
pdp=, attr=0x0) at allocatestack.c:416
  #18 __pthread_create_2_1 (newthread=, attr=,
  start_routine=, arg=) at pthread_create.c:539
  Backtrace stopped: Cannot access memory at address 0x8
  --

  The code that crashes is this
  --
  void qemu_co_queue_run_restart(Coroutine *co)
  {
  Coroutine *next;

  trace_qemu_co_queue_run_restart(co);
  while ((next = QSIMPLEQ_FIRST(&co->co_queue_wakeup))) {
  QSIMPLEQ_REMOVE_HEAD(&co->co_queue_wakeup, co_queue_next); <-Crash
  qemu_coroutine_enter(next);
  }
  }
  --

  Expanding the macro QSIMPLEQ_REMOVE_HEAD gives us
  --
  #define QSIMPLEQ_REMOVE_HEAD(head, field) do {  \
  if (((head)->sqh_first = (head)->sqh_first->field.sqe_next) == NULL)\
  (head)->sqh_last = &(head)->sqh_first;  \
  } while (/*CONSTCOND*/0)
  --

  which corrsponds to
  --
  if (((&co->co_queue_wakeup)->sqh_first = 
(&co->co_queue_wakeup)->sqh_first->co_queue_next.sqe_next) == NULL)\
  (&co->co_queue_wakeup)->sqh_last = &(&co->co_queue_wakeup)->sqh_first;
  --

  Debugging the list we see
  --
  (gdb) print *(&co->co_queue_wakeup->sqh_first)
  $6 = (struct Coroutine *) 0x1000
  (gdb) print *(&co->co_queue_wakeup->sqh_first->co_queue_next)
  Cannot access mem

[Qemu-devel] [PATCH RESEND 0/2] x86_iommu: Fix segfault when starting on non-PCI machines

2017-11-29 Thread Mohammed Gamal
[Resending for the second time]

Starting qemu with
qemu-system-x86_64 -S -M isapc -device {amd|intel}-iommu
leads to a segfault. The code assume PCI bus is present and
tries to access the bus structure without checking.

The patch series moves the error checks from vtd_realize()
and amdvi_realize() to the generic x86_iommu_realize() and
adds a check for PCI bus presence.


Mohammed Gamal (2):
  x86_iommu: Move machine check to x86_iommu_realize()
  x86_iommu: check if machine has PCI bus

 hw/i386/amd_iommu.c   | 13 ++---
 hw/i386/intel_iommu.c | 13 ++---
 hw/i386/x86-iommu.c   | 13 +
 3 files changed, 17 insertions(+), 22 deletions(-)

-- 
1.8.3.1




[Qemu-devel] [PATCH RESEND 2/2] x86_iommu: check if machine has PCI bus

2017-11-29 Thread Mohammed Gamal
Starting qemu with
qemu-system-x86_64 -S -M isapc -device {amd|intel}-iommu
leads to a segfault. The code assume PCI bus is present and
tries to access the bus structure without checking.

Since Intel VT-d and AMDVI should only work with PCI, add a
check for PCI bus and return error if not present.

Reviewed-by: Peter Xu 
Reviewed-by: Eduardo Habkost 
Signed-off-by: Mohammed Gamal 
---
 hw/i386/x86-iommu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/i386/x86-iommu.c b/hw/i386/x86-iommu.c
index 51de519..8a01a2d 100644
--- a/hw/i386/x86-iommu.c
+++ b/hw/i386/x86-iommu.c
@@ -88,7 +88,7 @@ static void x86_iommu_realize(DeviceState *dev, Error **errp)
 PC_MACHINE(object_dynamic_cast(OBJECT(ms), TYPE_PC_MACHINE));
 QLIST_INIT(&x86_iommu->iec_notifiers);
 
-if (!pcms) {
+if (!pcms || !pcms->bus) {
 error_setg(errp, "Machine-type '%s' not supported by IOMMU",
mc->name);
 return;
-- 
1.8.3.1




[Qemu-devel] [PATCH RESEND 1/2] x86_iommu: Move machine check to x86_iommu_realize()

2017-11-29 Thread Mohammed Gamal
Instead of having the same error checks in vtd_realize()
and amdvi_realize(), move that over to the generic
x86_iommu_realize().

Reviewed-by: Peter Xu 
Reviewed-by: Eduardo Habkost 
Signed-off-by: Mohammed Gamal 
---
 hw/i386/amd_iommu.c   | 13 ++---
 hw/i386/intel_iommu.c | 13 ++---
 hw/i386/x86-iommu.c   | 13 +
 3 files changed, 17 insertions(+), 22 deletions(-)

diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index ad8155c..1618282 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -1141,18 +1141,9 @@ static void amdvi_realize(DeviceState *dev, Error **err)
 AMDVIState *s = AMD_IOMMU_DEVICE(dev);
 X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(dev);
 MachineState *ms = MACHINE(qdev_get_machine());
-MachineClass *mc = MACHINE_GET_CLASS(ms);
-PCMachineState *pcms =
-PC_MACHINE(object_dynamic_cast(OBJECT(ms), TYPE_PC_MACHINE));
-PCIBus *bus;
-
-if (!pcms) {
-error_setg(err, "Machine-type '%s' not supported by amd-iommu",
-   mc->name);
-return;
-}
+PCMachineState *pcms = PC_MACHINE(ms);
+PCIBus *bus = pcms->bus;
 
-bus = pcms->bus;
 s->iotlb = g_hash_table_new_full(amdvi_uint64_hash,
  amdvi_uint64_equal, g_free, g_free);
 
diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 3a5bb0b..0138b3b 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -3027,20 +3027,11 @@ static bool vtd_decide_config(IntelIOMMUState *s, Error 
**errp)
 static void vtd_realize(DeviceState *dev, Error **errp)
 {
 MachineState *ms = MACHINE(qdev_get_machine());
-MachineClass *mc = MACHINE_GET_CLASS(ms);
-PCMachineState *pcms =
-PC_MACHINE(object_dynamic_cast(OBJECT(ms), TYPE_PC_MACHINE));
-PCIBus *bus;
+PCMachineState *pcms = PC_MACHINE(ms);
+PCIBus *bus = pcms->bus;
 IntelIOMMUState *s = INTEL_IOMMU_DEVICE(dev);
 X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(dev);
 
-if (!pcms) {
-error_setg(errp, "Machine-type '%s' not supported by intel-iommu",
-   mc->name);
-return;
-}
-
-bus = pcms->bus;
 x86_iommu->type = TYPE_INTEL;
 
 if (!vtd_decide_config(s, errp)) {
diff --git a/hw/i386/x86-iommu.c b/hw/i386/x86-iommu.c
index 293caf8..51de519 100644
--- a/hw/i386/x86-iommu.c
+++ b/hw/i386/x86-iommu.c
@@ -21,6 +21,8 @@
 #include "hw/sysbus.h"
 #include "hw/boards.h"
 #include "hw/i386/x86-iommu.h"
+#include "hw/i386/pc.h"
+#include "qapi/error.h"
 #include "qemu/error-report.h"
 #include "trace.h"
 
@@ -80,7 +82,18 @@ static void x86_iommu_realize(DeviceState *dev, Error **errp)
 {
 X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(dev);
 X86IOMMUClass *x86_class = X86_IOMMU_GET_CLASS(dev);
+MachineState *ms = MACHINE(qdev_get_machine());
+MachineClass *mc = MACHINE_GET_CLASS(ms);
+PCMachineState *pcms =
+PC_MACHINE(object_dynamic_cast(OBJECT(ms), TYPE_PC_MACHINE));
 QLIST_INIT(&x86_iommu->iec_notifiers);
+
+if (!pcms) {
+error_setg(errp, "Machine-type '%s' not supported by IOMMU",
+   mc->name);
+return;
+}
+
 if (x86_class->realize) {
 x86_class->realize(dev, errp);
 }
-- 
1.8.3.1




[Qemu-devel] [PATCH RESEND 2/2] x86_iommu: check if machine has PCI bus

2017-10-16 Thread Mohammed Gamal
Starting qemu with
qemu-system-x86_64 -S -M isapc -device {amd|intel}-iommu
leads to a segfault. The code assume PCI bus is present and
tries to access the bus structure without checking.

Since Intel VT-d and AMDVI should only work with PCI, add a
check for PCI bus and return error if not present.

Reviewed-by: Peter Xu 
Reviewed-by: Eduardo Habkost 
Signed-off-by: Mohammed Gamal 
---
 hw/i386/x86-iommu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/i386/x86-iommu.c b/hw/i386/x86-iommu.c
index 51de519..8a01a2d 100644
--- a/hw/i386/x86-iommu.c
+++ b/hw/i386/x86-iommu.c
@@ -88,7 +88,7 @@ static void x86_iommu_realize(DeviceState *dev, Error **errp)
 PC_MACHINE(object_dynamic_cast(OBJECT(ms), TYPE_PC_MACHINE));
 QLIST_INIT(&x86_iommu->iec_notifiers);
 
-if (!pcms) {
+if (!pcms || !pcms->bus) {
 error_setg(errp, "Machine-type '%s' not supported by IOMMU",
mc->name);
 return;
-- 
1.8.3.1




[Qemu-devel] [PATCH RESEND 1/2] x86_iommu: Move machine check to x86_iommu_realize()

2017-10-16 Thread Mohammed Gamal
Instead of having the same error checks in vtd_realize()
and amdvi_realize(), move that over to the generic
x86_iommu_realize().

Reviewed-by: Peter Xu 
Reviewed-by: Eduardo Habkost 
Signed-off-by: Mohammed Gamal 
---
 hw/i386/amd_iommu.c   | 13 ++---
 hw/i386/intel_iommu.c | 13 ++---
 hw/i386/x86-iommu.c   | 13 +
 3 files changed, 17 insertions(+), 22 deletions(-)

diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index 334938a..f2e1868 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -1141,18 +1141,9 @@ static void amdvi_realize(DeviceState *dev, Error **err)
 AMDVIState *s = AMD_IOMMU_DEVICE(dev);
 X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(dev);
 MachineState *ms = MACHINE(qdev_get_machine());
-MachineClass *mc = MACHINE_GET_CLASS(ms);
-PCMachineState *pcms =
-PC_MACHINE(object_dynamic_cast(OBJECT(ms), TYPE_PC_MACHINE));
-PCIBus *bus;
-
-if (!pcms) {
-error_setg(err, "Machine-type '%s' not supported by amd-iommu",
-   mc->name);
-return;
-}
+PCMachineState *pcms = PC_MACHINE(ms);
+PCIBus *bus = pcms->bus;
 
-bus = pcms->bus;
 s->iotlb = g_hash_table_new_full(amdvi_uint64_hash,
  amdvi_uint64_equal, g_free, g_free);
 
diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 3a5bb0b..0138b3b 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -3027,20 +3027,11 @@ static bool vtd_decide_config(IntelIOMMUState *s, Error 
**errp)
 static void vtd_realize(DeviceState *dev, Error **errp)
 {
 MachineState *ms = MACHINE(qdev_get_machine());
-MachineClass *mc = MACHINE_GET_CLASS(ms);
-PCMachineState *pcms =
-PC_MACHINE(object_dynamic_cast(OBJECT(ms), TYPE_PC_MACHINE));
-PCIBus *bus;
+PCMachineState *pcms = PC_MACHINE(ms);
+PCIBus *bus = pcms->bus;
 IntelIOMMUState *s = INTEL_IOMMU_DEVICE(dev);
 X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(dev);
 
-if (!pcms) {
-error_setg(errp, "Machine-type '%s' not supported by intel-iommu",
-   mc->name);
-return;
-}
-
-bus = pcms->bus;
 x86_iommu->type = TYPE_INTEL;
 
 if (!vtd_decide_config(s, errp)) {
diff --git a/hw/i386/x86-iommu.c b/hw/i386/x86-iommu.c
index 293caf8..51de519 100644
--- a/hw/i386/x86-iommu.c
+++ b/hw/i386/x86-iommu.c
@@ -21,6 +21,8 @@
 #include "hw/sysbus.h"
 #include "hw/boards.h"
 #include "hw/i386/x86-iommu.h"
+#include "hw/i386/pc.h"
+#include "qapi/error.h"
 #include "qemu/error-report.h"
 #include "trace.h"
 
@@ -80,7 +82,18 @@ static void x86_iommu_realize(DeviceState *dev, Error **errp)
 {
 X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(dev);
 X86IOMMUClass *x86_class = X86_IOMMU_GET_CLASS(dev);
+MachineState *ms = MACHINE(qdev_get_machine());
+MachineClass *mc = MACHINE_GET_CLASS(ms);
+PCMachineState *pcms =
+PC_MACHINE(object_dynamic_cast(OBJECT(ms), TYPE_PC_MACHINE));
 QLIST_INIT(&x86_iommu->iec_notifiers);
+
+if (!pcms) {
+error_setg(errp, "Machine-type '%s' not supported by IOMMU",
+   mc->name);
+return;
+}
+
 if (x86_class->realize) {
 x86_class->realize(dev, errp);
 }
-- 
1.8.3.1




[Qemu-devel] [PATCH RESEND 0/2] x86_iommu: Fix segfault when starting on non-PCI machines

2017-10-16 Thread Mohammed Gamal
Starting qemu with
qemu-system-x86_64 -S -M isapc -device {amd|intel}-iommu
leads to a segfault. The code assume PCI bus is present and
tries to access the bus structure without checking.

The patch series moves the error checks from vtd_realize()
and amdvi_realize() to the generic x86_iommu_realize() and
adds a check for PCI bus presence.


Mohammed Gamal (2):
  x86_iommu: Move machine check to x86_iommu_realize()
  x86_iommu: check if machine has PCI bus

 hw/i386/amd_iommu.c   | 13 ++---
 hw/i386/intel_iommu.c | 13 ++---
 hw/i386/x86-iommu.c   | 13 +
 3 files changed, 17 insertions(+), 22 deletions(-)

-- 
1.8.3.1




[Qemu-devel] [PATCH v5 1/2] x86_iommu: Move machine check to x86_iommu_realize()

2017-09-19 Thread Mohammed Gamal
Instead of having the same error checks in vtd_realize()
and amdvi_realize(), move that over to the generic
x86_iommu_realize().

Signed-off-by: Mohammed Gamal 
---
 hw/i386/amd_iommu.c   | 13 ++---
 hw/i386/intel_iommu.c | 13 ++---
 hw/i386/x86-iommu.c   | 13 +
 3 files changed, 17 insertions(+), 22 deletions(-)

diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index 334938a..f2e1868 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -1141,18 +1141,9 @@ static void amdvi_realize(DeviceState *dev, Error **err)
 AMDVIState *s = AMD_IOMMU_DEVICE(dev);
 X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(dev);
 MachineState *ms = MACHINE(qdev_get_machine());
-MachineClass *mc = MACHINE_GET_CLASS(ms);
-PCMachineState *pcms =
-PC_MACHINE(object_dynamic_cast(OBJECT(ms), TYPE_PC_MACHINE));
-PCIBus *bus;
-
-if (!pcms) {
-error_setg(err, "Machine-type '%s' not supported by amd-iommu",
-   mc->name);
-return;
-}
+PCMachineState *pcms = PC_MACHINE(ms);
+PCIBus *bus = pcms->bus;
 
-bus = pcms->bus;
 s->iotlb = g_hash_table_new_full(amdvi_uint64_hash,
  amdvi_uint64_equal, g_free, g_free);
 
diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 3a5bb0b..0138b3b 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -3027,20 +3027,11 @@ static bool vtd_decide_config(IntelIOMMUState *s, Error 
**errp)
 static void vtd_realize(DeviceState *dev, Error **errp)
 {
 MachineState *ms = MACHINE(qdev_get_machine());
-MachineClass *mc = MACHINE_GET_CLASS(ms);
-PCMachineState *pcms =
-PC_MACHINE(object_dynamic_cast(OBJECT(ms), TYPE_PC_MACHINE));
-PCIBus *bus;
+PCMachineState *pcms = PC_MACHINE(ms);
+PCIBus *bus = pcms->bus;
 IntelIOMMUState *s = INTEL_IOMMU_DEVICE(dev);
 X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(dev);
 
-if (!pcms) {
-error_setg(errp, "Machine-type '%s' not supported by intel-iommu",
-   mc->name);
-return;
-}
-
-bus = pcms->bus;
 x86_iommu->type = TYPE_INTEL;
 
 if (!vtd_decide_config(s, errp)) {
diff --git a/hw/i386/x86-iommu.c b/hw/i386/x86-iommu.c
index 293caf8..51de519 100644
--- a/hw/i386/x86-iommu.c
+++ b/hw/i386/x86-iommu.c
@@ -21,6 +21,8 @@
 #include "hw/sysbus.h"
 #include "hw/boards.h"
 #include "hw/i386/x86-iommu.h"
+#include "hw/i386/pc.h"
+#include "qapi/error.h"
 #include "qemu/error-report.h"
 #include "trace.h"
 
@@ -80,7 +82,18 @@ static void x86_iommu_realize(DeviceState *dev, Error **errp)
 {
 X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(dev);
 X86IOMMUClass *x86_class = X86_IOMMU_GET_CLASS(dev);
+MachineState *ms = MACHINE(qdev_get_machine());
+MachineClass *mc = MACHINE_GET_CLASS(ms);
+PCMachineState *pcms =
+PC_MACHINE(object_dynamic_cast(OBJECT(ms), TYPE_PC_MACHINE));
 QLIST_INIT(&x86_iommu->iec_notifiers);
+
+if (!pcms) {
+error_setg(errp, "Machine-type '%s' not supported by IOMMU",
+   mc->name);
+return;
+}
+
 if (x86_class->realize) {
 x86_class->realize(dev, errp);
 }
-- 
1.8.3.1




[Qemu-devel] [PATCH v5 2/2] x86_iommu: check if machine has PCI bus

2017-09-19 Thread Mohammed Gamal
Starting qemu with
qemu-system-x86_64 -S -M isapc -device {amd|intel}-iommu
leads to a segfault. The code assume PCI bus is present and
tries to access the bus structure without checking.

Since Intel VT-d and AMDVI should only work with PCI, add a
check for PCI bus and return error if not present.

Signed-off-by: Mohammed Gamal 
---
 hw/i386/x86-iommu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/i386/x86-iommu.c b/hw/i386/x86-iommu.c
index 51de519..8a01a2d 100644
--- a/hw/i386/x86-iommu.c
+++ b/hw/i386/x86-iommu.c
@@ -88,7 +88,7 @@ static void x86_iommu_realize(DeviceState *dev, Error **errp)
 PC_MACHINE(object_dynamic_cast(OBJECT(ms), TYPE_PC_MACHINE));
 QLIST_INIT(&x86_iommu->iec_notifiers);
 
-if (!pcms) {
+if (!pcms || !pcms->bus) {
 error_setg(errp, "Machine-type '%s' not supported by IOMMU",
mc->name);
 return;
-- 
1.8.3.1




[Qemu-devel] [PATCH v5 0/2] x86_iommu: Fix segfault when starting on non-PCI machines

2017-09-19 Thread Mohammed Gamal
Starting qemu with
qemu-system-x86_64 -S -M isapc -device {amd|intel}-iommu
leads to a segfault. The code assume PCI bus is present and
tries to access the bus structure without checking.

The patch series moves the error checks from vtd_realize()
and amdvi_realize() to the generic x86_iommu_realize() and
adds a check for PCI bus presence.

v4 --> v5:
   * Squash patch 2/3 from v4 into patch 1/3

Mohammed Gamal (2):
  x86_iommu: Move machine check to x86_iommu_realize()
  x86_iommu: check if machine has PCI bus

 hw/i386/amd_iommu.c   | 13 ++---
 hw/i386/intel_iommu.c | 13 ++---
 hw/i386/x86-iommu.c   | 13 +
 3 files changed, 17 insertions(+), 22 deletions(-)

-- 
1.8.3.1




[Qemu-devel] [PATCH v4 1/3] x86_iommu: Move machine check to x86_iommu_realize()

2017-09-18 Thread Mohammed Gamal
Instead of having the same error checks in vtd_realize()
and amdvi_realize(), move that over to the generic
x86_iommu_realize().

Signed-off-by: Mohammed Gamal 
---
 hw/i386/amd_iommu.c   | 10 +-
 hw/i386/intel_iommu.c | 10 +-
 hw/i386/x86-iommu.c   | 13 +
 3 files changed, 15 insertions(+), 18 deletions(-)

diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index 334938a..839f01f 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -1141,18 +1141,10 @@ static void amdvi_realize(DeviceState *dev, Error **err)
 AMDVIState *s = AMD_IOMMU_DEVICE(dev);
 X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(dev);
 MachineState *ms = MACHINE(qdev_get_machine());
-MachineClass *mc = MACHINE_GET_CLASS(ms);
 PCMachineState *pcms =
 PC_MACHINE(object_dynamic_cast(OBJECT(ms), TYPE_PC_MACHINE));
-PCIBus *bus;
+PCIBus *bus = pcms->bus;
 
-if (!pcms) {
-error_setg(err, "Machine-type '%s' not supported by amd-iommu",
-   mc->name);
-return;
-}
-
-bus = pcms->bus;
 s->iotlb = g_hash_table_new_full(amdvi_uint64_hash,
  amdvi_uint64_equal, g_free, g_free);
 
diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 3a5bb0b..aa01812 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -3027,20 +3027,12 @@ static bool vtd_decide_config(IntelIOMMUState *s, Error 
**errp)
 static void vtd_realize(DeviceState *dev, Error **errp)
 {
 MachineState *ms = MACHINE(qdev_get_machine());
-MachineClass *mc = MACHINE_GET_CLASS(ms);
 PCMachineState *pcms =
 PC_MACHINE(object_dynamic_cast(OBJECT(ms), TYPE_PC_MACHINE));
-PCIBus *bus;
+PCIBus *bus = pcms->bus;
 IntelIOMMUState *s = INTEL_IOMMU_DEVICE(dev);
 X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(dev);
 
-if (!pcms) {
-error_setg(errp, "Machine-type '%s' not supported by intel-iommu",
-   mc->name);
-return;
-}
-
-bus = pcms->bus;
 x86_iommu->type = TYPE_INTEL;
 
 if (!vtd_decide_config(s, errp)) {
diff --git a/hw/i386/x86-iommu.c b/hw/i386/x86-iommu.c
index 293caf8..51de519 100644
--- a/hw/i386/x86-iommu.c
+++ b/hw/i386/x86-iommu.c
@@ -21,6 +21,8 @@
 #include "hw/sysbus.h"
 #include "hw/boards.h"
 #include "hw/i386/x86-iommu.h"
+#include "hw/i386/pc.h"
+#include "qapi/error.h"
 #include "qemu/error-report.h"
 #include "trace.h"
 
@@ -80,7 +82,18 @@ static void x86_iommu_realize(DeviceState *dev, Error **errp)
 {
 X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(dev);
 X86IOMMUClass *x86_class = X86_IOMMU_GET_CLASS(dev);
+MachineState *ms = MACHINE(qdev_get_machine());
+MachineClass *mc = MACHINE_GET_CLASS(ms);
+PCMachineState *pcms =
+PC_MACHINE(object_dynamic_cast(OBJECT(ms), TYPE_PC_MACHINE));
 QLIST_INIT(&x86_iommu->iec_notifiers);
+
+if (!pcms) {
+error_setg(errp, "Machine-type '%s' not supported by IOMMU",
+   mc->name);
+return;
+}
+
 if (x86_class->realize) {
 x86_class->realize(dev, errp);
 }
-- 
1.8.3.1




[Qemu-devel] [PATCH v4 3/3] x86_iommu: check if machine has PCI bus

2017-09-18 Thread Mohammed Gamal
Starting qemu with
qemu-system-x86_64 -S -M isapc -device {amd|intel}-iommu
leads to a segfault. The code assume PCI bus is present and
tries to access the bus structure without checking.

Since Intel VT-d and AMDVI should only work with PCI, add a
check for PCI bus and return error if not present.

Signed-off-by: Mohammed Gamal 
---
 hw/i386/x86-iommu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/i386/x86-iommu.c b/hw/i386/x86-iommu.c
index 51de519..8a01a2d 100644
--- a/hw/i386/x86-iommu.c
+++ b/hw/i386/x86-iommu.c
@@ -88,7 +88,7 @@ static void x86_iommu_realize(DeviceState *dev, Error **errp)
 PC_MACHINE(object_dynamic_cast(OBJECT(ms), TYPE_PC_MACHINE));
 QLIST_INIT(&x86_iommu->iec_notifiers);
 
-if (!pcms) {
+if (!pcms || !pcms->bus) {
 error_setg(errp, "Machine-type '%s' not supported by IOMMU",
mc->name);
 return;
-- 
1.8.3.1




[Qemu-devel] [PATCH v4 2/3] intel_iommu, amd_iommu: Remove redundant object_dynamic_cast calls

2017-09-18 Thread Mohammed Gamal
Now that the calling x86_iommu_realize() calls object_dynamic_cast(),
doing the cast in amdvi_realize() and vtd_realize() is not needed.

Use PC_MACHINE macro directly.

Signed-off-by: Mohammed Gamal 
---
 hw/i386/amd_iommu.c   | 3 +--
 hw/i386/intel_iommu.c | 3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index 839f01f..f2e1868 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -1141,8 +1141,7 @@ static void amdvi_realize(DeviceState *dev, Error **err)
 AMDVIState *s = AMD_IOMMU_DEVICE(dev);
 X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(dev);
 MachineState *ms = MACHINE(qdev_get_machine());
-PCMachineState *pcms =
-PC_MACHINE(object_dynamic_cast(OBJECT(ms), TYPE_PC_MACHINE));
+PCMachineState *pcms = PC_MACHINE(ms);
 PCIBus *bus = pcms->bus;
 
 s->iotlb = g_hash_table_new_full(amdvi_uint64_hash,
diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index aa01812..0138b3b 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -3027,8 +3027,7 @@ static bool vtd_decide_config(IntelIOMMUState *s, Error 
**errp)
 static void vtd_realize(DeviceState *dev, Error **errp)
 {
 MachineState *ms = MACHINE(qdev_get_machine());
-PCMachineState *pcms =
-PC_MACHINE(object_dynamic_cast(OBJECT(ms), TYPE_PC_MACHINE));
+PCMachineState *pcms = PC_MACHINE(ms);
 PCIBus *bus = pcms->bus;
 IntelIOMMUState *s = INTEL_IOMMU_DEVICE(dev);
 X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(dev);
-- 
1.8.3.1




[Qemu-devel] [PATCH v4 0/3] x86_iommu: Fix segfault when starting on non-PCI machines

2017-09-18 Thread Mohammed Gamal
Starting qemu with
qemu-system-x86_64 -S -M isapc -device {amd|intel}-iommu
leads to a segfault. The code assume PCI bus is present and
tries to access the bus structure without checking.

The patch series moves the error checks from vtd_realize()
and amdvi_realize() to the generic x86_iommu_realize() and
adds a check for PCI bus presence.

v3 --> v4:
  * Restore correct object_dynamic_cast() in x86_iommu_realize()
  * Remove redundant casting in callee functions. Implemented in
a new patch

Mohammed Gamal (3):
  x86_iommu: Move machine check to x86_iommu_realize()
  intel_iommu, amd_iommu: Remove redundant object_dynamic_cast calls
  x86_iommu: check if machine has PCI bus

 hw/i386/amd_iommu.c   | 13 ++---
 hw/i386/intel_iommu.c | 13 ++---
 hw/i386/x86-iommu.c   | 13 +
 3 files changed, 17 insertions(+), 22 deletions(-)

-- 
1.8.3.1




Re: [Qemu-devel] [PATCH v3 1/2] x86_iommu: Move machine check to x86_iommu_realize()

2017-09-18 Thread Mohammed Gamal
On Mon, 2017-09-18 at 10:13 -0300, Eduardo Habkost wrote:
> On Mon, Sep 18, 2017 at 12:14:08PM +0200, Mohammed Gamal wrote:
> > Instead of having the same error checks in vtd_realize()
> > and amdvi_realize(), move that over to the generic
> > x86_iommu_realize().
> > 
> > Signed-off-by: Mohammed Gamal 
> > ---
> >  hw/i386/amd_iommu.c   | 10 +-
> >  hw/i386/intel_iommu.c | 10 +-
> >  hw/i386/x86-iommu.c   | 12 
> >  3 files changed, 14 insertions(+), 18 deletions(-)
> > 
> > diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
> > index 334938a..839f01f 100644
> > --- a/hw/i386/amd_iommu.c
> > +++ b/hw/i386/amd_iommu.c
> > @@ -1141,18 +1141,10 @@ static void amdvi_realize(DeviceState *dev, Error 
> > **err)
> >  AMDVIState *s = AMD_IOMMU_DEVICE(dev);
> >  X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(dev);
> >  MachineState *ms = MACHINE(qdev_get_machine());
> > -MachineClass *mc = MACHINE_GET_CLASS(ms);
> >  PCMachineState *pcms =
> >  PC_MACHINE(object_dynamic_cast(OBJECT(ms), TYPE_PC_MACHINE));
> 
> This is where you shouldn't use object_dynamic_cast() because the
> code now expects pcms to never be NULL...
> 
> > -PCIBus *bus;
> > +PCIBus *bus = pcms->bus;
> >  
> > -if (!pcms) {
> > -error_setg(err, "Machine-type '%s' not supported by amd-iommu",
> > -   mc->name);
> > -return;
> > -}
> > -
> > -bus = pcms->bus;
> >  s->iotlb = g_hash_table_new_full(amdvi_uint64_hash,
> >   amdvi_uint64_equal, g_free, g_free);
> >  
> > diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> > index 3a5bb0b..aa01812 100644
> > --- a/hw/i386/intel_iommu.c
> > +++ b/hw/i386/intel_iommu.c
> > @@ -3027,20 +3027,12 @@ static bool vtd_decide_config(IntelIOMMUState *s, 
> > Error **errp)
> >  static void vtd_realize(DeviceState *dev, Error **errp)
> >  {
> >  MachineState *ms = MACHINE(qdev_get_machine());
> > -MachineClass *mc = MACHINE_GET_CLASS(ms);
> >  PCMachineState *pcms =
> >  PC_MACHINE(object_dynamic_cast(OBJECT(ms), TYPE_PC_MACHINE));
> 
> Same as above.
> 
> > -PCIBus *bus;
> > +PCIBus *bus = pcms->bus;
> >  IntelIOMMUState *s = INTEL_IOMMU_DEVICE(dev);
> >  X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(dev);
> >  
> > -if (!pcms) {
> > -error_setg(errp, "Machine-type '%s' not supported by intel-iommu",
> > -   mc->name);
> > -return;
> > -}
> > -
> > -bus = pcms->bus;
> >  x86_iommu->type = TYPE_INTEL;
> >  
> >  if (!vtd_decide_config(s, errp)) {
> > diff --git a/hw/i386/x86-iommu.c b/hw/i386/x86-iommu.c
> > index 293caf8..d43b08a 100644
> > --- a/hw/i386/x86-iommu.c
> > +++ b/hw/i386/x86-iommu.c
> > @@ -21,6 +21,8 @@
> >  #include "hw/sysbus.h"
> >  #include "hw/boards.h"
> >  #include "hw/i386/x86-iommu.h"
> > +#include "hw/i386/pc.h"
> > +#include "qapi/error.h"
> >  #include "qemu/error-report.h"
> >  #include "trace.h"
> >  
> > @@ -80,7 +82,17 @@ static void x86_iommu_realize(DeviceState *dev, Error 
> > **errp)
> >  {
> >  X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(dev);
> >  X86IOMMUClass *x86_class = X86_IOMMU_GET_CLASS(dev);
> > +MachineState *ms = MACHINE(qdev_get_machine());
> > +MachineClass *mc = MACHINE_GET_CLASS(ms);
> > +PCMachineState *pcms = PC_MACHINE(ms);
> 
> And here is where you really need object_dynamic_cast(),
> otherwise the "if (!pcms)" check below will be useless and you'll
> get this:
> 
>   $ qemu-system-x86_64 -machine none -device intel-iommu
>   qemu/hw/i386/x86-iommu.c:87:x86_iommu_realize: Object 0x53d7d02060 is not 
> an instance of type generic-pc-machine
>   Aborted (core dumped)

This is what happens when you miss your daily Caffeine dose :)
I thought the object_dynamic_cast call was redundant and thus needs to
be removed when moving the code.

I see the correct sequence you mean now. Since the check is done in the
calling x86_iommu_realize(), the check won't be need in the callees.

I will send the fixed version shortly.
> 
> >  QLIST_INIT(&x86_iommu->iec_notifiers);
> > +
> > +if (!pcms) {
> > +error_setg(errp, "Machine-type '%s' not supported by IOMMU",
> > +   mc->name);
> > +return;
> > +}
> > +
> >  if (x86_class->realize) {
> >  x86_class->realize(dev, errp);
> >  }
> > -- 
> > 1.8.3.1
> > 
> 





[Qemu-devel] [PATCH v3 2/2] x86_iommu: check if machine has PCI bus

2017-09-18 Thread Mohammed Gamal
Starting qemu with
qemu-system-x86_64 -S -M isapc -device {amd|intel}-iommu
leads to a segfault. The code assume PCI bus is present and
tries to access the bus structure without checking.

Since Intel VT-d and AMDVI should only work with PCI, add a
check for PCI bus and return error if not present.

Signed-off-by: Mohammed Gamal 
---
 hw/i386/x86-iommu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/i386/x86-iommu.c b/hw/i386/x86-iommu.c
index d43b08a..00f70bb 100644
--- a/hw/i386/x86-iommu.c
+++ b/hw/i386/x86-iommu.c
@@ -87,7 +87,7 @@ static void x86_iommu_realize(DeviceState *dev, Error **errp)
 PCMachineState *pcms = PC_MACHINE(ms);
 QLIST_INIT(&x86_iommu->iec_notifiers);
 
-if (!pcms) {
+if (!pcms || !pcms->bus) {
 error_setg(errp, "Machine-type '%s' not supported by IOMMU",
mc->name);
 return;
-- 
1.8.3.1




[Qemu-devel] [PATCH v3 0/2] x86_iommu: Fix segfault when starting on non-PCI machines

2017-09-18 Thread Mohammed Gamal
Starting qemu with
qemu-system-x86_64 -S -M isapc -device {amd|intel}-iommu
leads to a segfault. The code assume PCI bus is present and
tries to access the bus structure without checking.

The patch series moves the error checks from vtd_realize()
and amdvi_realize() to the generic x86_iommu_realize() and
adds a check for PCI bus presence.

v2 --> v3:
* Use PC_MACHINE macro directly

Mohammed Gamal (2):
  x86_iommu: Move machine check to x86_iommu_realize()
  x86_iommu: check if machine has PCI bus

 hw/i386/amd_iommu.c   | 10 +-
 hw/i386/intel_iommu.c | 10 +-
 hw/i386/x86-iommu.c   | 12 
 3 files changed, 14 insertions(+), 18 deletions(-)

-- 
1.8.3.1




[Qemu-devel] [PATCH v3 1/2] x86_iommu: Move machine check to x86_iommu_realize()

2017-09-18 Thread Mohammed Gamal
Instead of having the same error checks in vtd_realize()
and amdvi_realize(), move that over to the generic
x86_iommu_realize().

Signed-off-by: Mohammed Gamal 
---
 hw/i386/amd_iommu.c   | 10 +-
 hw/i386/intel_iommu.c | 10 +-
 hw/i386/x86-iommu.c   | 12 
 3 files changed, 14 insertions(+), 18 deletions(-)

diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index 334938a..839f01f 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -1141,18 +1141,10 @@ static void amdvi_realize(DeviceState *dev, Error **err)
 AMDVIState *s = AMD_IOMMU_DEVICE(dev);
 X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(dev);
 MachineState *ms = MACHINE(qdev_get_machine());
-MachineClass *mc = MACHINE_GET_CLASS(ms);
 PCMachineState *pcms =
 PC_MACHINE(object_dynamic_cast(OBJECT(ms), TYPE_PC_MACHINE));
-PCIBus *bus;
+PCIBus *bus = pcms->bus;
 
-if (!pcms) {
-error_setg(err, "Machine-type '%s' not supported by amd-iommu",
-   mc->name);
-return;
-}
-
-bus = pcms->bus;
 s->iotlb = g_hash_table_new_full(amdvi_uint64_hash,
  amdvi_uint64_equal, g_free, g_free);
 
diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 3a5bb0b..aa01812 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -3027,20 +3027,12 @@ static bool vtd_decide_config(IntelIOMMUState *s, Error 
**errp)
 static void vtd_realize(DeviceState *dev, Error **errp)
 {
 MachineState *ms = MACHINE(qdev_get_machine());
-MachineClass *mc = MACHINE_GET_CLASS(ms);
 PCMachineState *pcms =
 PC_MACHINE(object_dynamic_cast(OBJECT(ms), TYPE_PC_MACHINE));
-PCIBus *bus;
+PCIBus *bus = pcms->bus;
 IntelIOMMUState *s = INTEL_IOMMU_DEVICE(dev);
 X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(dev);
 
-if (!pcms) {
-error_setg(errp, "Machine-type '%s' not supported by intel-iommu",
-   mc->name);
-return;
-}
-
-bus = pcms->bus;
 x86_iommu->type = TYPE_INTEL;
 
 if (!vtd_decide_config(s, errp)) {
diff --git a/hw/i386/x86-iommu.c b/hw/i386/x86-iommu.c
index 293caf8..d43b08a 100644
--- a/hw/i386/x86-iommu.c
+++ b/hw/i386/x86-iommu.c
@@ -21,6 +21,8 @@
 #include "hw/sysbus.h"
 #include "hw/boards.h"
 #include "hw/i386/x86-iommu.h"
+#include "hw/i386/pc.h"
+#include "qapi/error.h"
 #include "qemu/error-report.h"
 #include "trace.h"
 
@@ -80,7 +82,17 @@ static void x86_iommu_realize(DeviceState *dev, Error **errp)
 {
 X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(dev);
 X86IOMMUClass *x86_class = X86_IOMMU_GET_CLASS(dev);
+MachineState *ms = MACHINE(qdev_get_machine());
+MachineClass *mc = MACHINE_GET_CLASS(ms);
+PCMachineState *pcms = PC_MACHINE(ms);
 QLIST_INIT(&x86_iommu->iec_notifiers);
+
+if (!pcms) {
+error_setg(errp, "Machine-type '%s' not supported by IOMMU",
+   mc->name);
+return;
+}
+
 if (x86_class->realize) {
 x86_class->realize(dev, errp);
 }
-- 
1.8.3.1




[Qemu-devel] [PATCH v2 0/2] x86_iommu: Fix segfault when starting on non-PCI machines

2017-09-15 Thread Mohammed Gamal
Starting qemu with
qemu-system-x86_64 -S -M isapc -device {amd|intel}-iommu
leads to a segfault. The code assume PCI bus is present and
tries to access the bus structure without checking.

The patch series moves the error checks from vtd_realize()
and amdvi_realize() to the generic x86_iommu_realize() and
adds a check for PCI bus presence.

v1 --> v2:
* Change error message with capitalized acronym

Mohammed Gamal (2):
  x86_iommu: Move machine check to x86_iommu_realize()
  x86_iommu: check if machine has PCI bus

 hw/i386/amd_iommu.c   | 10 +-
 hw/i386/intel_iommu.c | 10 +-
 hw/i386/x86-iommu.c   | 13 +
 3 files changed, 15 insertions(+), 18 deletions(-)

-- 
1.8.3.1




[Qemu-devel] [PATCH v2 2/2] x86_iommu: check if machine has PCI bus

2017-09-15 Thread Mohammed Gamal
Starting qemu with
qemu-system-x86_64 -S -M isapc -device {amd|intel}-iommu
leads to a segfault. The code assume PCI bus is present and
tries to access the bus structure without checking.

Since Intel VT-d and AMDVI should only work with PCI, add a
check for PCI bus and return error if not present.

Signed-off-by: Mohammed Gamal 
---
 hw/i386/x86-iommu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/i386/x86-iommu.c b/hw/i386/x86-iommu.c
index 51de519..8a01a2d 100644
--- a/hw/i386/x86-iommu.c
+++ b/hw/i386/x86-iommu.c
@@ -88,7 +88,7 @@ static void x86_iommu_realize(DeviceState *dev, Error **errp)
 PC_MACHINE(object_dynamic_cast(OBJECT(ms), TYPE_PC_MACHINE));
 QLIST_INIT(&x86_iommu->iec_notifiers);
 
-if (!pcms) {
+if (!pcms || !pcms->bus) {
 error_setg(errp, "Machine-type '%s' not supported by IOMMU",
mc->name);
 return;
-- 
1.8.3.1




[Qemu-devel] [PATCH v2 1/2] x86_iommu: Move machine check to x86_iommu_realize()

2017-09-15 Thread Mohammed Gamal
Instead of having the same error checks in vtd_realize()
and amdvi_realize(), move that over to the generic
x86_iommu_realize().

Signed-off-by: Mohammed Gamal 
---
 hw/i386/amd_iommu.c   | 10 +-
 hw/i386/intel_iommu.c | 10 +-
 hw/i386/x86-iommu.c   | 13 +
 3 files changed, 15 insertions(+), 18 deletions(-)

diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index 334938a..839f01f 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -1141,18 +1141,10 @@ static void amdvi_realize(DeviceState *dev, Error **err)
 AMDVIState *s = AMD_IOMMU_DEVICE(dev);
 X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(dev);
 MachineState *ms = MACHINE(qdev_get_machine());
-MachineClass *mc = MACHINE_GET_CLASS(ms);
 PCMachineState *pcms =
 PC_MACHINE(object_dynamic_cast(OBJECT(ms), TYPE_PC_MACHINE));
-PCIBus *bus;
+PCIBus *bus = pcms->bus;
 
-if (!pcms) {
-error_setg(err, "Machine-type '%s' not supported by amd-iommu",
-   mc->name);
-return;
-}
-
-bus = pcms->bus;
 s->iotlb = g_hash_table_new_full(amdvi_uint64_hash,
  amdvi_uint64_equal, g_free, g_free);
 
diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 3a5bb0b..aa01812 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -3027,20 +3027,12 @@ static bool vtd_decide_config(IntelIOMMUState *s, Error 
**errp)
 static void vtd_realize(DeviceState *dev, Error **errp)
 {
 MachineState *ms = MACHINE(qdev_get_machine());
-MachineClass *mc = MACHINE_GET_CLASS(ms);
 PCMachineState *pcms =
 PC_MACHINE(object_dynamic_cast(OBJECT(ms), TYPE_PC_MACHINE));
-PCIBus *bus;
+PCIBus *bus = pcms->bus;
 IntelIOMMUState *s = INTEL_IOMMU_DEVICE(dev);
 X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(dev);
 
-if (!pcms) {
-error_setg(errp, "Machine-type '%s' not supported by intel-iommu",
-   mc->name);
-return;
-}
-
-bus = pcms->bus;
 x86_iommu->type = TYPE_INTEL;
 
 if (!vtd_decide_config(s, errp)) {
diff --git a/hw/i386/x86-iommu.c b/hw/i386/x86-iommu.c
index 293caf8..51de519 100644
--- a/hw/i386/x86-iommu.c
+++ b/hw/i386/x86-iommu.c
@@ -21,6 +21,8 @@
 #include "hw/sysbus.h"
 #include "hw/boards.h"
 #include "hw/i386/x86-iommu.h"
+#include "hw/i386/pc.h"
+#include "qapi/error.h"
 #include "qemu/error-report.h"
 #include "trace.h"
 
@@ -80,7 +82,18 @@ static void x86_iommu_realize(DeviceState *dev, Error **errp)
 {
 X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(dev);
 X86IOMMUClass *x86_class = X86_IOMMU_GET_CLASS(dev);
+MachineState *ms = MACHINE(qdev_get_machine());
+MachineClass *mc = MACHINE_GET_CLASS(ms);
+PCMachineState *pcms =
+PC_MACHINE(object_dynamic_cast(OBJECT(ms), TYPE_PC_MACHINE));
 QLIST_INIT(&x86_iommu->iec_notifiers);
+
+if (!pcms) {
+error_setg(errp, "Machine-type '%s' not supported by IOMMU",
+   mc->name);
+return;
+}
+
 if (x86_class->realize) {
 x86_class->realize(dev, errp);
 }
-- 
1.8.3.1




[Qemu-devel] [PATCH 2/2] x86_iommu: check if machine has PCI bus

2017-09-15 Thread Mohammed Gamal
Starting qemu with
qemu-system-x86_64 -S -M isapc -device {amd|intel}-iommu
leads to a segfault. The code assume PCI bus is present and
tries to access the bus structure without checking.

Since Intel VT-d and AMDVI should only work with PCI, add a
check for PCI bus and return error if not present.

Signed-off-by: Mohammed Gamal 
---
 hw/i386/x86-iommu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/i386/x86-iommu.c b/hw/i386/x86-iommu.c
index 4d17e1f..afd8cd9 100644
--- a/hw/i386/x86-iommu.c
+++ b/hw/i386/x86-iommu.c
@@ -88,7 +88,7 @@ static void x86_iommu_realize(DeviceState *dev, Error **errp)
 PC_MACHINE(object_dynamic_cast(OBJECT(ms), TYPE_PC_MACHINE));
 QLIST_INIT(&x86_iommu->iec_notifiers);
 
-if (!pcms) {
+if (!pcms || !pcms->bus) {
 error_setg(errp, "Machine-type '%s' not supported by iommu",
mc->name);
 return;
-- 
1.8.3.1




[Qemu-devel] [PATCH 0/2] x86_iommu: Fix segfault when starting on non-PCI machines

2017-09-15 Thread Mohammed Gamal
Starting qemu with
qemu-system-x86_64 -S -M isapc -device {amd|intel}-iommu
leads to a segfault. The code assume PCI bus is present and
tries to access the bus structure without checking.

The patch series moves the error checks from vtd_realize()
and amdvi_realize() to the generic x86_iommu_realize() and
adds a check for PCI bus presence.

Mohammed Gamal (2):
  x86_iommu: Move machine check to x86_iommu_realize()
  x86_iommu: check if machine has PCI bus

 hw/i386/amd_iommu.c   | 10 +-
 hw/i386/intel_iommu.c | 10 +-
 hw/i386/x86-iommu.c   | 13 +
 3 files changed, 15 insertions(+), 18 deletions(-)

-- 
1.8.3.1




[Qemu-devel] [PATCH 1/2] x86_iommu: Move machine check to x86_iommu_realize()

2017-09-15 Thread Mohammed Gamal
Instead of having the same error checks in vtd_realize()
and amdvi_realize(), move that over to the generic
x86_iommu_realize().

Signed-off-by: Mohammed Gamal 
---
 hw/i386/amd_iommu.c   | 10 +-
 hw/i386/intel_iommu.c | 10 +-
 hw/i386/x86-iommu.c   | 13 +
 3 files changed, 15 insertions(+), 18 deletions(-)

diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index 334938a..839f01f 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -1141,18 +1141,10 @@ static void amdvi_realize(DeviceState *dev, Error **err)
 AMDVIState *s = AMD_IOMMU_DEVICE(dev);
 X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(dev);
 MachineState *ms = MACHINE(qdev_get_machine());
-MachineClass *mc = MACHINE_GET_CLASS(ms);
 PCMachineState *pcms =
 PC_MACHINE(object_dynamic_cast(OBJECT(ms), TYPE_PC_MACHINE));
-PCIBus *bus;
+PCIBus *bus = pcms->bus;
 
-if (!pcms) {
-error_setg(err, "Machine-type '%s' not supported by amd-iommu",
-   mc->name);
-return;
-}
-
-bus = pcms->bus;
 s->iotlb = g_hash_table_new_full(amdvi_uint64_hash,
  amdvi_uint64_equal, g_free, g_free);
 
diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 3a5bb0b..aa01812 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -3027,20 +3027,12 @@ static bool vtd_decide_config(IntelIOMMUState *s, Error 
**errp)
 static void vtd_realize(DeviceState *dev, Error **errp)
 {
 MachineState *ms = MACHINE(qdev_get_machine());
-MachineClass *mc = MACHINE_GET_CLASS(ms);
 PCMachineState *pcms =
 PC_MACHINE(object_dynamic_cast(OBJECT(ms), TYPE_PC_MACHINE));
-PCIBus *bus;
+PCIBus *bus = pcms->bus;
 IntelIOMMUState *s = INTEL_IOMMU_DEVICE(dev);
 X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(dev);
 
-if (!pcms) {
-error_setg(errp, "Machine-type '%s' not supported by intel-iommu",
-   mc->name);
-return;
-}
-
-bus = pcms->bus;
 x86_iommu->type = TYPE_INTEL;
 
 if (!vtd_decide_config(s, errp)) {
diff --git a/hw/i386/x86-iommu.c b/hw/i386/x86-iommu.c
index 293caf8..4d17e1f 100644
--- a/hw/i386/x86-iommu.c
+++ b/hw/i386/x86-iommu.c
@@ -21,6 +21,8 @@
 #include "hw/sysbus.h"
 #include "hw/boards.h"
 #include "hw/i386/x86-iommu.h"
+#include "hw/i386/pc.h"
+#include "qapi/error.h"
 #include "qemu/error-report.h"
 #include "trace.h"
 
@@ -80,7 +82,18 @@ static void x86_iommu_realize(DeviceState *dev, Error **errp)
 {
 X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(dev);
 X86IOMMUClass *x86_class = X86_IOMMU_GET_CLASS(dev);
+MachineState *ms = MACHINE(qdev_get_machine());
+MachineClass *mc = MACHINE_GET_CLASS(ms);
+PCMachineState *pcms =
+PC_MACHINE(object_dynamic_cast(OBJECT(ms), TYPE_PC_MACHINE));
 QLIST_INIT(&x86_iommu->iec_notifiers);
+
+if (!pcms) {
+error_setg(errp, "Machine-type '%s' not supported by iommu",
+   mc->name);
+return;
+}
+
 if (x86_class->realize) {
 x86_class->realize(dev, errp);
 }
-- 
1.8.3.1




Re: [Qemu-devel] [PATCH] amd_iommu: Return error on machines with no PCI

2017-09-15 Thread Mohammed Gamal
On Fri, 2017-09-15 at 10:07 +0800, Peter Xu wrote:
> On Thu, Sep 14, 2017 at 05:31:38PM -0300, Eduardo Habkost wrote:
> > On Thu, Sep 14, 2017 at 10:24:23PM +0200, Thomas Huth wrote:
> > > On 14.09.2017 22:18, Mohammed Gamal wrote:
> > > > Starting the following command line causes a segfault
> > > > qemu-system-x86_64 -S -machine isapc,accel=kvm -device amd-iommu
> > > > 
> > > > This is due to the fact that the machine type 'isapc' doesn't have
> > > > a PCI bus, while amd_iommu doesn't check if the machine has PCI support
> > > > and subsequently does a null-pointer access. AMD IOMMU shouldn't even 
> > > > work
> > > > if the target machine doesn't have PCI.
> > > > 
> > > > Add a check for PCI on the given machine type and return an error if PCI
> > > > is not supported.
> > > > 
> > > > Signed-off-by: Mohammed Gamal 
> > > > ---
> > > >  hw/i386/amd_iommu.c | 7 +++
> > > >  1 file changed, 7 insertions(+)
> > > > 
> > > > diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
> > > > index 334938a..9a667b7 100644
> > > > --- a/hw/i386/amd_iommu.c
> > > > +++ b/hw/i386/amd_iommu.c
> > > > @@ -1153,6 +1153,13 @@ static void amdvi_realize(DeviceState *dev, 
> > > > Error **err)
> > > >  }
> > > >  
> > > >  bus = pcms->bus;
> > > > +
> > > > +if (!bus) {
> > > > +error_setg(err, "Machine-type '%s' does not support PCI",
> > > > +   mc->name);
> > > > +return;
> > > > +}
> > > > +
> > > >  s->iotlb = g_hash_table_new_full(amdvi_uint64_hash,
> > > >   amdvi_uint64_equal, g_free, 
> > > > g_free);
> > > >  
> > > > 
> > > 
> > > Patch looks fine to me, but I think it would also be sufficient to
> > > change the check at the beginning of the function to test "if (!pcms ||
> > > !pcms->bus)" instead of just "if (!pcms)" ... the error message
> > > "Machine-type 'xxx' not supported by amd-iommu" is also adequate if
> > > there is no PCI bus available on the system.
> > 
> > I agree this would be much simpler.
> 
> Even, shall we move the pcms && bus check into x86_iommu_realize()
> directly?  Then we will only need one single patch for Intel/AMD, and
> it's also a cleanup.  Thanks,

Although it's more straight forward to do the checks in amdvi_realize()
and vtd_realize() at the moment, I think moving the checks to
x86_iommu_realize() would be better on the longer term. I will be
sending out a new patch with this change shortly.

Regards,
Mohammed
 




[Qemu-devel] [PATCH] amd_iommu: Return error on machines with no PCI

2017-09-14 Thread Mohammed Gamal
Starting the following command line causes a segfault
qemu-system-x86_64 -S -machine isapc,accel=kvm -device amd-iommu

This is due to the fact that the machine type 'isapc' doesn't have
a PCI bus, while amd_iommu doesn't check if the machine has PCI support
and subsequently does a null-pointer access. AMD IOMMU shouldn't even work
if the target machine doesn't have PCI.

Add a check for PCI on the given machine type and return an error if PCI
is not supported.

Signed-off-by: Mohammed Gamal 
---
 hw/i386/amd_iommu.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index 334938a..9a667b7 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -1153,6 +1153,13 @@ static void amdvi_realize(DeviceState *dev, Error **err)
 }
 
 bus = pcms->bus;
+
+if (!bus) {
+error_setg(err, "Machine-type '%s' does not support PCI",
+   mc->name);
+return;
+}
+
 s->iotlb = g_hash_table_new_full(amdvi_uint64_hash,
  amdvi_uint64_equal, g_free, g_free);
 
-- 
1.8.3.1




[Qemu-devel] [PATCH] amd_iommu: Return error on machines with no PCI

2017-09-14 Thread Mohammed Gamal
Starting the following command line causes a segfault
qemu-system-x86_64 -S -machine isapc,accel=kvm -device amd-iommu

This is due to the fact that the machine type 'isapc' doesn't have
a PCI bus, while amd_iommu doesn't check if the machine has PCI support
and subsequently does a null-pointer access. AMD IOMMU shouldn't even work
if the target machine doesn't have PCI.

Add a check for PCI on the given machine type and return an error if PCI
is not supported.

Signed-off-by: Mohammed Gamal 
---
 hw/i386/amd_iommu.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index 334938a..9a667b7 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -1153,6 +1153,13 @@ static void amdvi_realize(DeviceState *dev, Error **err)
 }
 
 bus = pcms->bus;
+
+if (!bus) {
+error_setg(err, "Machine-type '%s' does not support PCI",
+   mc->name);
+return;
+}
+
 s->iotlb = g_hash_table_new_full(amdvi_uint64_hash,
  amdvi_uint64_equal, g_free, g_free);
 
-- 
1.8.3.1




[Qemu-devel] [PATCH] intel_iommu: Return error on machines with no PCI

2017-09-14 Thread Mohammed Gamal
Starting the following command line causes a segfault
qemu-system-x86_64 -S -machine isapc,accel=kvm -device intel-iommu

This is due to the fact that the machine type 'isapc' doesn't have
a PCI bus, while intel_iommu doesn't check if the machine has PCI support
and subsequently does a null-pointer access. Intel IOMMU shouldn't even work
if the target machine doesn't have PCI.

Add a check for PCI on the given machine type and return an error if PCI
is not supported.

Signed-off-by: Mohammed Gamal 
---
 hw/i386/intel_iommu.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 3a5bb0b..fab0b4b 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -3041,6 +3041,13 @@ static void vtd_realize(DeviceState *dev, Error **errp)
 }
 
 bus = pcms->bus;
+
+if (!bus) {
+error_setg(errp, "Machine-type '%s' does not support PCI",
+   mc->name);
+return;
+}
+
 x86_iommu->type = TYPE_INTEL;
 
 if (!vtd_decide_config(s, errp)) {
-- 
1.8.3.1




[Qemu-devel] [Bug 1673130] Re: qemu 2.7.0 receives SIGABRT in qemu_coroutine_enter()

2017-03-15 Thread Mohammed Gamal
Another stack trace:

--
(gdb) bt
#0  0x7f2f34690067 in __GI_raise (sig=sig@entry=6) at 
../nptl/sysdeps/unix/sysv/linux/raise.c:56
#1  0x7f2f34691448 in __GI_abort () at abort.c:89
#2  0x5629b8260b6c in qemu_coroutine_enter (co=0x7f2cd6a00940) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:113
#3  0x5629b8260e74 in qemu_co_queue_run_restart (co=0x7f2cd6a00880) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
#4  0x5629b82609a9 in qemu_coroutine_enter (co=0x7f2cd6a00880) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
#5  0x5629b8260e74 in qemu_co_queue_run_restart (co=0x7f2cee202b00) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
#6  0x5629b82609a9 in qemu_coroutine_enter (co=0x7f2cee202b00) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
#7  0x5629b8260e74 in qemu_co_queue_run_restart (co=0x7f2cee2141d0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
#8  0x5629b82609a9 in qemu_coroutine_enter (co=0x7f2cee2141d0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
#9  0x5629b8260e74 in qemu_co_queue_run_restart (co=0x7f2cf300b370) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
#10 0x5629b82609a9 in qemu_coroutine_enter (co=0x7f2cf300b370) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
#11 0x5629b8260e74 in qemu_co_queue_run_restart (co=0x7f2cf1a03560) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
#12 0x5629b82609a9 in qemu_coroutine_enter (co=0x7f2cf1a03560) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
#13 0x5629b8260e74 in qemu_co_queue_run_restart (co=0x7f2cf3e15ba0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
#14 0x5629b82609a9 in qemu_coroutine_enter (co=0x7f2cf3e15ba0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
#15 0x5629b8260e74 in qemu_co_queue_run_restart (co=0x7f2ce80087f0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
#16 0x5629b82609a9 in qemu_coroutine_enter (co=0x7f2ce80087f0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
#17 0x5629b8260e74 in qemu_co_queue_run_restart (co=0x7f2cee20d9c0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
#18 0x5629b82609a9 in qemu_coroutine_enter (co=0x7f2cee20d9c0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
#19 0x5629b8260e74 in qemu_co_queue_run_restart (co=0x7f2ceff04850) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
#20 0x5629b82609a9 in qemu_coroutine_enter (co=0x7f2ceff04850) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
#21 0x5629b8260e74 in qemu_co_queue_run_restart (co=0x7f2cf21061c0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
#22 0x5629b82609a9 in qemu_coroutine_enter (co=0x7f2cf21061c0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
#23 0x5629b8260e74 in qemu_co_queue_run_restart (co=0x7f2cf2105c00) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
#24 0x5629b82609a9 in qemu_coroutine_enter (co=0x7f2cf2105c00) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
#25 0x5629b8260e74 in qemu_co_queue_run_restart (co=0x7f2cf3e1d590) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
#26 0x5629b82609a9 in qemu_coroutine_enter (co=0x7f2cf3e1d590) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
#27 0x5629b8260e74 in qemu_co_queue_run_restart (co=0x7f2cf3e16a00) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
#28 0x5629b82609a9 in qemu_coroutine_enter (co=0x7f2cf3e16a00) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
#29 0x5629b8260e74 in qemu_co_queue_run_restart (co=0x7f2ce8004da0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
#30 0x5629b82609a9 in qemu_coroutine_enter (co=0x7f2ce8004da0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
#31 0x5629b8260e74 in qemu_co_queue_run_restart (co=0x7f2cf3e15dc0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
#32 0x5629b82609a9 in qemu_coroutine_enter (co=0x7f2cf3e15dc0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
#33 0x5629b8260e74 in qemu_co_queue_run_restart (co=0x7f2ccff00420) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
#34 0x5629b82609a9 in qemu_coroutine_enter (co=0x7f2ccff00420) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
#35 0x5629b8260e74 in qemu_co_queue_run_restart (co=0x7f2cf1e04900) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
#36 0x5629b82609a9 in qemu_coroutine_enter (co=0x7f2cf1e04

[Qemu-devel] [Bug 1673130] Re: qemu 2.7.0 receives SIGABRT in qemu_coroutine_enter()

2017-03-15 Thread Mohammed Gamal
Third stack trace:

--
#0  0x7f4d5ad6a067 in raise () from /lib/x86_64-linux-gnu/libc.so.6
#1  0x7f4d5ad6b448 in abort () from /lib/x86_64-linux-gnu/libc.so.6
#2  0x562a4c582b6c in qemu_coroutine_enter (co=0x7f4b1bf0a900) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:113
#3  0x562a4c582e55 in qemu_co_queue_run_restart (co=0x7f4b1bf0a830) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
#4  0x562a4c5829a9 in qemu_coroutine_enter (co=0x7f4b1bf0a830) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
#5  0x562a4c582e74 in qemu_co_queue_run_restart (co=0x7f4b1bf0f4c0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
#6  0x562a4c5829a9 in qemu_coroutine_enter (co=0x7f4b1bf0f4c0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
#7  0x562a4c582e74 in qemu_co_queue_run_restart (co=0x7f4b17e07c40) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
#8  0x562a4c5829a9 in qemu_coroutine_enter (co=0x7f4b17e07c40) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
#9  0x562a4c582e74 in qemu_co_queue_run_restart (co=0x7f4b17e11420) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
#10 0x562a4c5829a9 in qemu_coroutine_enter (co=0x7f4b17e11420) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
#11 0x562a4c582e74 in qemu_co_queue_run_restart (co=0x7f4b17e18c30) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
#12 0x562a4c5829a9 in qemu_coroutine_enter (co=0x7f4b17e18c30) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
#13 0x562a4c582e74 in qemu_co_queue_run_restart (co=0x7f4b1bf07ea0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
#14 0x562a4c5829a9 in qemu_coroutine_enter (co=0x7f4b1bf07ea0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
#15 0x562a4c582e74 in qemu_co_queue_run_restart (co=0x7f4b1000c0c0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
#16 0x562a4c5829a9 in qemu_coroutine_enter (co=0x7f4b1000c0c0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
#17 0x562a4c582e74 in qemu_co_queue_run_restart (co=0x7f4b17e11b10) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
#18 0x562a4c5829a9 in qemu_coroutine_enter (co=0x7f4b17e11b10) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
#19 0x562a4c582e74 in qemu_co_queue_run_restart (co=0x7f4b17e10500) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
#20 0x562a4c5829a9 in qemu_coroutine_enter (co=0x7f4b17e10500) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
#21 0x562a4c582e74 in qemu_co_queue_run_restart (co=0x7f4b1bf0a610) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
#22 0x562a4c5829a9 in qemu_coroutine_enter (co=0x7f4b1bf0a610) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
#23 0x562a4c582e74 in qemu_co_queue_run_restart (co=0x7f4b17e12820) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
#24 0x562a4c5829a9 in qemu_coroutine_enter (co=0x7f4b17e12820) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
#25 0x562a4c582e74 in qemu_co_queue_run_restart (co=0x7f4b10002b10) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
#26 0x562a4c5829a9 in qemu_coroutine_enter (co=0x7f4b10002b10) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
#27 0x562a4c582e74 in qemu_co_queue_run_restart (co=0x7f4b1000bfb0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
#28 0x562a4c5829a9 in qemu_coroutine_enter (co=0x7f4b1000bfb0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
#29 0x562a4c582e74 in qemu_co_queue_run_restart (co=0x7f4b17e103f0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
#30 0x562a4c5829a9 in qemu_coroutine_enter (co=0x7f4b17e103f0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
#31 0x562a4c582e74 in qemu_co_queue_run_restart (co=0x7f4b17e078b0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
#32 0x562a4c5829a9 in qemu_coroutine_enter (co=0x7f4b17e078b0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
#33 0x562a4c582e74 in qemu_co_queue_run_restart (co=0x7f4adfe02b00) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
#34 0x562a4c5829a9 in qemu_coroutine_enter (co=0x7f4adfe02b00) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
#35 0x562a4c582e74 in qemu_co_queue_run_restart (co=0x7f4b15701ae0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
#36 0x562a4c5829a9 in qemu_coroutine_enter (co=0x7f4b15701ae0) at 
/build/pb-qemu

[Qemu-devel] [Bug 1673130] [NEW] qemu 2.7.0 receives SIGABRT in qemu_coroutine_enter()

2017-03-15 Thread Mohammed Gamal
Public bug reported:

I've been experiencing frequent SIGABRTs (in addition to segfaults in
#1671876) lately with qemu 2.7.0 running Ubuntu 16.04 guests. The crash
usually happens in qemu_coroutine_enter(). I haven't seen this so far
with any other guests or distros.

Here is one stack trace I obtained
--
(gdb) bt
#0  0x7fd7cc676067 in __GI_raise (sig=sig@entry=6) at 
../nptl/sysdeps/unix/sysv/linux/raise.c:56
#1  0x7fd7cc677448 in __GI_abort () at abort.c:89
#2  0x556aed247b6c in qemu_coroutine_enter (co=0x7fd574300df0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:113
#3  0x556aed247e55 in qemu_co_queue_run_restart (co=0x7fd574300ce0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
#4  0x556aed2479a9 in qemu_coroutine_enter (co=0x7fd574300ce0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
#5  0x556aed247e74 in qemu_co_queue_run_restart (co=0x7fd589111670) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
#6  0x556aed2479a9 in qemu_coroutine_enter (co=0x7fd589111670) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
#7  0x556aed247e74 in qemu_co_queue_run_restart (co=0x7fd57430dba0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
#8  0x556aed2479a9 in qemu_coroutine_enter (co=0x7fd57430dba0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
#9  0x556aed247e74 in qemu_co_queue_run_restart (co=0x7fd589119130) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
#10 0x556aed2479a9 in qemu_coroutine_enter (co=0x7fd589119130) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
#11 0x556aed247e74 in qemu_co_queue_run_restart (co=0x7fd589117410) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
#12 0x556aed2479a9 in qemu_coroutine_enter (co=0x7fd589117410) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
#13 0x556aed247e74 in qemu_co_queue_run_restart (co=0x7fd577f00e00) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
#14 0x556aed2479a9 in qemu_coroutine_enter (co=0x7fd577f00e00) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
#15 0x556aed247fa0 in qemu_co_enter_next (queue=queue@entry=0x556aef34e5e0) 
at /build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:106
#16 0x556aed1e6060 in timer_cb (blk=0x556aef34e590, is_write=) at /build/pb-qemu-pssKUp/pb-qemu-2.7.0/block/throttle-groups.c:400
#17 0x556aed1a3615 in timerlist_run_timers (timer_list=0x556aef3bad40) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/qemu-timer.c:528
#18 0x556aed1a3679 in timerlistgroup_run_timers 
(tlg=tlg@entry=0x556af0738758) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/qemu-timer.c:564
#19 0x556aed1a3f47 in aio_dispatch (ctx=ctx@entry=0x556af0738610) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/aio-posix.c:357
#20 0x556aed1a40e8 in aio_poll (ctx=0x556af0738610, blocking=) at /build/pb-qemu-pssKUp/pb-qemu-2.7.0/aio-posix.c:479
#21 0x556aed005c79 in iothread_run (opaque=0x556af07383c0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/iothread.c:46
#22 0x7fd7cc9f40a4 in start_thread (arg=0x7fd7aafff700) at 
pthread_create.c:403
#23 0x7fd7cc72962d in clone () at 
../sysdeps/unix/sysv/linux/x86_64/clone.S:111
--

The code crashes here
--
void qemu_coroutine_enter(Coroutine *co)
{
Coroutine *self = qemu_coroutine_self();
CoroutineAction ret;

trace_qemu_coroutine_enter(self, co, co->entry_arg);

if (co->caller) {
fprintf(stderr, "Co-routine re-entered recursively\n");
abort();  <--- Code aborts here 
  
}

[...]
}
--

Debugging further we see:
--
(gdb) frame 2
#2  0x556aed247b6c in qemu_coroutine_enter (co=0x7fd574300df0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:113
113/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c: No such file 
or directory.
(gdb) print *co
$1 = {entry = 0x7fd793e95a58, entry_arg = 0x1, caller = 0x7fd793e95a38, 
pool_next = {sle_next = 0x10}, co_queue_wakeup = {sqh_first = 0x7fd6ebbd2000, 
sqh_last = 0x1000}, co_queue_next = {
sqe_next = 0x7fd6ebbd1000}}
(gdb) print *co->caller
$2 = {entry = 0x40040001, entry_arg = 0xc546a20, caller = 0x0, pool_next = 
{sle_next = 0x0}, co_queue_wakeup = {sqh_first = 0x0, sqh_last = 
0xea00061f7480}, co_queue_next = {sqe_next = 0x1000}}
(gdb) frame 4
#4  0x556aed2479a9 in qemu_coroutine_enter (co=0x7fd574300ce0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-corout

[Qemu-devel] [Bug 1671876] Re: qemu 2.7.0 segfaults in qemu_co_queue_run_restart()

2017-03-15 Thread Mohammed Gamal
Unfortunately it'd not be possible to use another version at the moment.
Is it possible that someone takes a look at the stack traces?

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1671876

Title:
  qemu 2.7.0 segfaults in qemu_co_queue_run_restart()

Status in QEMU:
  New

Bug description:
  Hi,

  I've been experiencing frequent segfaults lately with qemu 2.7.0
  running Ubuntu 16.04 guests. The crash usually happens in
  qemu_co_queue_run_restart(). I haven't seen this so far with any other
  guests or distros.

  Here is one back trace I obtained from one of the crashing VMs.

  --
  (gdb) bt
  #0  qemu_co_queue_run_restart (co=0x7fba8ff05aa0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:59
  #1  0x55c1656f39a9 in qemu_coroutine_enter (co=0x7fba8ff05aa0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
  #2  0x55c1656f3e74 in qemu_co_queue_run_restart (co=0x7fba8dd20430) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
  #3  0x55c1656f39a9 in qemu_coroutine_enter (co=0x7fba8dd20430) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
  #4  0x55c1656f3e74 in qemu_co_queue_run_restart (co=0x7fba8dd14ea0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
  #5  0x55c1656f39a9 in qemu_coroutine_enter (co=0x7fba8dd14ea0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
  #6  0x55c1656f3e74 in qemu_co_queue_run_restart (co=0x7fba80c11dc0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
  #7  0x55c1656f39a9 in qemu_coroutine_enter (co=0x7fba80c11dc0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
  #8  0x55c1656f3e74 in qemu_co_queue_run_restart (co=0x7fba8dd0bd70) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
  #9  0x55c1656f39a9 in qemu_coroutine_enter (co=0x7fba8dd0bd70) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
  #10 0x55c1656f3fa0 in qemu_co_enter_next 
(queue=queue@entry=0x55c1669e75e0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:106
  #11 0x55c165692060 in timer_cb (blk=0x55c1669e7590, is_write=) at /build/pb-qemu-pssKUp/pb-qemu-2.7.0/block/throttle-groups.c:400
  #12 0x55c16564f615 in timerlist_run_timers (timer_list=0x55c166a53e80) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/qemu-timer.c:528
  #13 0x55c16564f679 in timerlistgroup_run_timers 
(tlg=tlg@entry=0x55c167c81cf8) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/qemu-timer.c:564
  #14 0x55c16564ff47 in aio_dispatch (ctx=ctx@entry=0x55c167c81bb0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/aio-posix.c:357
  #15 0x55c1656500e8 in aio_poll (ctx=0x55c167c81bb0, blocking=) at /build/pb-qemu-pssKUp/pb-qemu-2.7.0/aio-posix.c:479
  #16 0x55c1654b1c79 in iothread_run (opaque=0x55c167c81960) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/iothread.c:46
  #17 0x7fbc4b64f0a4 in allocate_stack (stack=, 
pdp=, attr=0x0) at allocatestack.c:416
  #18 __pthread_create_2_1 (newthread=, attr=,
  start_routine=, arg=) at pthread_create.c:539
  Backtrace stopped: Cannot access memory at address 0x8
  --

  The code that crashes is this
  --
  void qemu_co_queue_run_restart(Coroutine *co)
  {
  Coroutine *next;

  trace_qemu_co_queue_run_restart(co);
  while ((next = QSIMPLEQ_FIRST(&co->co_queue_wakeup))) {
  QSIMPLEQ_REMOVE_HEAD(&co->co_queue_wakeup, co_queue_next); <-Crash
  qemu_coroutine_enter(next);
  }
  }
  --

  Expanding the macro QSIMPLEQ_REMOVE_HEAD gives us
  --
  #define QSIMPLEQ_REMOVE_HEAD(head, field) do {  \
  if (((head)->sqh_first = (head)->sqh_first->field.sqe_next) == NULL)\
  (head)->sqh_last = &(head)->sqh_first;  \
  } while (/*CONSTCOND*/0)
  --

  which corrsponds to
  --
  if (((&co->co_queue_wakeup)->sqh_first = 
(&co->co_queue_wakeup)->sqh_first->co_queue_next.sqe_next) == NULL)\
  (&co->co_queue_wakeup)->sqh_last = &(&co->co_queue_wakeup)->sqh_first;
  --

  Debugging the list we see
  --
  (gdb) print *(&co->co_queue_wakeup->sqh_first)
  $6 = (struct Coroutine *) 0x1000
  (gdb) print *(&co->co_queue_wakeup->sqh_first->co_queue_next)
  Cann

[Qemu-devel] [Bug 1671876] Re: qemu 2.7.0 segfaults in qemu_co_queue_run_restart()

2017-03-10 Thread Mohammed Gamal
** Description changed:

  Hi,
  
  I've been experiencing frequent segfaults lately with qemu 2.7.0 running
  Ubuntu 16.04 guests. The crash usually happens in
  qemu_co_queue_run_restart(). I haven't seen this so far with any other
  guests or distros.
  
  Here is one back trace I obtained from one of the crashing VMs.
  
  --
  (gdb) bt
  #0  qemu_co_queue_run_restart (co=0x7fba8ff05aa0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:59
  #1  0x55c1656f39a9 in qemu_coroutine_enter (co=0x7fba8ff05aa0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
  #2  0x55c1656f3e74 in qemu_co_queue_run_restart (co=0x7fba8dd20430) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
  #3  0x55c1656f39a9 in qemu_coroutine_enter (co=0x7fba8dd20430) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
  #4  0x55c1656f3e74 in qemu_co_queue_run_restart (co=0x7fba8dd14ea0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
  #5  0x55c1656f39a9 in qemu_coroutine_enter (co=0x7fba8dd14ea0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
  #6  0x55c1656f3e74 in qemu_co_queue_run_restart (co=0x7fba80c11dc0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
  #7  0x55c1656f39a9 in qemu_coroutine_enter (co=0x7fba80c11dc0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
  #8  0x55c1656f3e74 in qemu_co_queue_run_restart (co=0x7fba8dd0bd70) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
  #9  0x55c1656f39a9 in qemu_coroutine_enter (co=0x7fba8dd0bd70) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
  #10 0x55c1656f3fa0 in qemu_co_enter_next 
(queue=queue@entry=0x55c1669e75e0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:106
  #11 0x55c165692060 in timer_cb (blk=0x55c1669e7590, is_write=) at /build/pb-qemu-pssKUp/pb-qemu-2.7.0/block/throttle-groups.c:400
  #12 0x55c16564f615 in timerlist_run_timers (timer_list=0x55c166a53e80) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/qemu-timer.c:528
  #13 0x55c16564f679 in timerlistgroup_run_timers 
(tlg=tlg@entry=0x55c167c81cf8) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/qemu-timer.c:564
  #14 0x55c16564ff47 in aio_dispatch (ctx=ctx@entry=0x55c167c81bb0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/aio-posix.c:357
  #15 0x55c1656500e8 in aio_poll (ctx=0x55c167c81bb0, blocking=) at /build/pb-qemu-pssKUp/pb-qemu-2.7.0/aio-posix.c:479
  #16 0x55c1654b1c79 in iothread_run (opaque=0x55c167c81960) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/iothread.c:46
  #17 0x7fbc4b64f0a4 in allocate_stack (stack=, 
pdp=, attr=0x0) at allocatestack.c:416
  #18 __pthread_create_2_1 (newthread=, attr=,
  start_routine=, arg=) at pthread_create.c:539
  Backtrace stopped: Cannot access memory at address 0x8
  --
  
  The code that crashes is this
  --
  void qemu_co_queue_run_restart(Coroutine *co)
  {
  Coroutine *next;
  
  trace_qemu_co_queue_run_restart(co);
  while ((next = QSIMPLEQ_FIRST(&co->co_queue_wakeup))) {
- QSIMPLEQ_REMOVE_HEAD(&co->co_queue_wakeup, co_queue_next);   <--- 
Crash occurs here this time
+ QSIMPLEQ_REMOVE_HEAD(&co->co_queue_wakeup, co_queue_next); <-Crash
  qemu_coroutine_enter(next);
  }
  }
  --
  
  Expanding the macro QSIMPLEQ_REMOVE_HEAD gives us
  --
  #define QSIMPLEQ_REMOVE_HEAD(head, field) do {  \
  if (((head)->sqh_first = (head)->sqh_first->field.sqe_next) == NULL)\
  (head)->sqh_last = &(head)->sqh_first;  \
  } while (/*CONSTCOND*/0)
  --
  
  which corrsponds to
  --
  if (((&co->co_queue_wakeup)->sqh_first = 
(&co->co_queue_wakeup)->sqh_first->co_queue_next.sqe_next) == NULL)\
  (&co->co_queue_wakeup)->sqh_last = &(&co->co_queue_wakeup)->sqh_first;
  --
  
  Debugging the list we see
  --
  (gdb) print *(&co->co_queue_wakeup->sqh_first)
  $6 = (struct Coroutine *) 0x1000
  (gdb) print *(&co->co_queue_wakeup->sqh_first->co_queue_next)
  Cannot access memory at address 0x1030
  --
  
  So the data in co->co_queue_wakeup->sqh_first is corrupted and
  represents an invalid address. Any idea why is that?

-- 
Y

[Qemu-devel] [Bug 1671876] Re: qemu 2.7.0 segfaults in qemu_co_queue_run_restart()

2017-03-10 Thread Mohammed Gamal
The VMs were running with the following arguments
-
-m 1024,slots=255,maxmem=256G -M pc-i440fx-2.7 -enable-kvm -nodefconfig 
-nodefaults -rtc base=utc -netdev 
tap,ifname=n020133f0895e,id=hostnet6,vhost=on,vhostforce=on,vnet_hdr=off,script=no,downscript=no
 -device 
virtio-net-pci,netdev=hostnet6,id=net6,mac=02:01:33:f0:89:5e,bus=pci.0,addr=0x6 
-chardev pty,id=charserial0 -device isa-serial,chardev=charserial0,id=serial0 
-usb -device usb-tablet,id=input0 -vnc 0.0.0.0:94 -vga qxl -cpu Haswell,+vmx 
-smp 6,sockets=32,cores=1,maxcpus=64,threads=2 -drive 
file=/dev/md10,if=none,id=drive-virtio-disk5,format=raw,snapshot=off,aio=native,cache=none
 -device 
virtio-blk-pci,bus=pci.0,addr=0x5,drive=drive-virtio-disk5,num-queues=3,id=virtio-disk5,bootindex=1
 -S
-

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1671876

Title:
  qemu 2.7.0 segfaults in qemu_co_queue_run_restart()

Status in QEMU:
  New

Bug description:
  Hi,

  I've been experiencing frequent segfaults lately with qemu 2.7.0
  running Ubuntu 16.04 guests. The crash usually happens in
  qemu_co_queue_run_restart(). I haven't seen this so far with any other
  guests or distros.

  Here is one back trace I obtained from one of the crashing VMs.

  --
  (gdb) bt
  #0  qemu_co_queue_run_restart (co=0x7fba8ff05aa0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:59
  #1  0x55c1656f39a9 in qemu_coroutine_enter (co=0x7fba8ff05aa0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
  #2  0x55c1656f3e74 in qemu_co_queue_run_restart (co=0x7fba8dd20430) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
  #3  0x55c1656f39a9 in qemu_coroutine_enter (co=0x7fba8dd20430) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
  #4  0x55c1656f3e74 in qemu_co_queue_run_restart (co=0x7fba8dd14ea0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
  #5  0x55c1656f39a9 in qemu_coroutine_enter (co=0x7fba8dd14ea0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
  #6  0x55c1656f3e74 in qemu_co_queue_run_restart (co=0x7fba80c11dc0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
  #7  0x55c1656f39a9 in qemu_coroutine_enter (co=0x7fba80c11dc0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
  #8  0x55c1656f3e74 in qemu_co_queue_run_restart (co=0x7fba8dd0bd70) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
  #9  0x55c1656f39a9 in qemu_coroutine_enter (co=0x7fba8dd0bd70) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
  #10 0x55c1656f3fa0 in qemu_co_enter_next 
(queue=queue@entry=0x55c1669e75e0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:106
  #11 0x55c165692060 in timer_cb (blk=0x55c1669e7590, is_write=) at /build/pb-qemu-pssKUp/pb-qemu-2.7.0/block/throttle-groups.c:400
  #12 0x55c16564f615 in timerlist_run_timers (timer_list=0x55c166a53e80) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/qemu-timer.c:528
  #13 0x55c16564f679 in timerlistgroup_run_timers 
(tlg=tlg@entry=0x55c167c81cf8) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/qemu-timer.c:564
  #14 0x55c16564ff47 in aio_dispatch (ctx=ctx@entry=0x55c167c81bb0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/aio-posix.c:357
  #15 0x55c1656500e8 in aio_poll (ctx=0x55c167c81bb0, blocking=) at /build/pb-qemu-pssKUp/pb-qemu-2.7.0/aio-posix.c:479
  #16 0x55c1654b1c79 in iothread_run (opaque=0x55c167c81960) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/iothread.c:46
  #17 0x7fbc4b64f0a4 in allocate_stack (stack=, 
pdp=, attr=0x0) at allocatestack.c:416
  #18 __pthread_create_2_1 (newthread=, attr=,
  start_routine=, arg=) at pthread_create.c:539
  Backtrace stopped: Cannot access memory at address 0x8
  --

  The code that crashes is this
  --
  void qemu_co_queue_run_restart(Coroutine *co)
  {
  Coroutine *next;

  trace_qemu_co_queue_run_restart(co);
  while ((next = QSIMPLEQ_FIRST(&co->co_queue_wakeup))) {
  QSIMPLEQ_REMOVE_HEAD(&co->co_queue_wakeup, co_queue_next);   <--- 
Crash occurs here this time
  qemu_coroutine_enter(next);
  }
  }
  --

  Expanding the macro QSIMPLEQ_REMOVE_HEAD gives us
  --
  #define QSIMPLEQ_REMOVE_HEAD(head, field) do {  \
  if (((head)->sqh_first = (head)->sqh_first->field.sqe_next) == NULL)\
  (

[Qemu-devel] [Bug 1671876] Re: qemu 2.7.0 segfaults in qemu_co_queue_run_restart()

2017-03-10 Thread Mohammed Gamal
A third stack trace

It generates the following stack trace
-
(gdb) bt
#0  qemu_co_queue_run_restart (co=0x7f75ed30dbc0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:59
#1  0x5619274829a9 in qemu_coroutine_enter (co=0x7f75ed30dbc0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
#2  0x561927482e74 in qemu_co_queue_run_restart (co=0x7f75f1c0f200) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
#3  0x5619274829a9 in qemu_coroutine_enter (co=0x7f75f1c0f200) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
#4  0x561927482e74 in qemu_co_queue_run_restart (co=0x7f75ed304870) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
#5  0x5619274829a9 in qemu_coroutine_enter (co=0x7f75ed304870) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
#6  0x561927482e74 in qemu_co_queue_run_restart (co=0x7f75e800fcd0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
#7  0x5619274829a9 in qemu_coroutine_enter (co=0x7f75e800fcd0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
#8  0x561927482e74 in qemu_co_queue_run_restart (co=0x7f75e800fac0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
#9  0x5619274829a9 in qemu_coroutine_enter (co=0x7f75e800fac0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
#10 0x561927482e74 in qemu_co_queue_run_restart (co=0x7f75e800f8b0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
#11 0x5619274829a9 in qemu_coroutine_enter (co=0x7f75e800f8b0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
#12 0x561927482e74 in qemu_co_queue_run_restart (co=0x7f75fbf05570) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
#13 0x5619274829a9 in qemu_coroutine_enter (co=0x7f75fbf05570) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
#14 0x561927482e74 in qemu_co_queue_run_restart (co=0x7f75e8009b70) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
#15 0x5619274829a9 in qemu_coroutine_enter (co=0x7f75e8009b70) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
#16 0x561927482e74 in qemu_co_queue_run_restart (co=0x7f75e800b5d0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
#17 0x5619274829a9 in qemu_coroutine_enter (co=0x7f75e800b5d0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
#18 0x561927482e74 in qemu_co_queue_run_restart (co=0x7f75e8008910) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
#19 0x5619274829a9 in qemu_coroutine_enter (co=0x7f75e8008910) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
#20 0x561927482e74 in qemu_co_queue_run_restart (co=0x7f75e800f6a0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
#21 0x5619274829a9 in qemu_coroutine_enter (co=0x7f75e800f6a0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
#22 0x561927482e74 in qemu_co_queue_run_restart (co=0x7f75fbf05100) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
#23 0x5619274829a9 in qemu_coroutine_enter (co=0x7f75fbf05100) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
#24 0x561927482e74 in qemu_co_queue_run_restart (co=0x7f75fbf04ee0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
#25 0x5619274829a9 in qemu_coroutine_enter (co=0x7f75fbf04ee0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
#26 0x561927482e74 in qemu_co_queue_run_restart (co=0x7f75ed301c50) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
#27 0x5619274829a9 in qemu_coroutine_enter (co=0x7f75ed301c50) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
#28 0x561927482e74 in qemu_co_queue_run_restart (co=0x7f75ed315270) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
#29 0x5619274829a9 in qemu_coroutine_enter (co=0x7f75ed315270) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
#30 0x561927482e74 in qemu_co_queue_run_restart (co=0x7f75ed31cf10) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
#31 0x5619274829a9 in qemu_coroutine_enter (co=0x7f75ed31cf10) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
#32 0x561927482e74 in qemu_co_queue_run_restart (co=0x7f75e800a970) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
#33 0x5619274829a9 in qemu_coroutine_enter (co=0x7f75e800a970) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
#34 0x561927482e74 in qemu_co_queue_run_restart (co=0x7f75e8007df0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
#35 0x5619274829a9 in qemu_coroutine_enter (co=0x7f75e8007df0

[Qemu-devel] [Bug 1671876] [NEW] qemu 2.7.0 segfaults in qemu_co_queue_run_restart()

2017-03-10 Thread Mohammed Gamal
Public bug reported:

Hi,

I've been experiencing frequent segfaults lately with qemu 2.7.0 running
Ubuntu 16.04 guests. The crash usually happens in
qemu_co_queue_run_restart(). I haven't seen this so far with any other
guests or distros.

Here is one back trace I obtained from one of the crashing VMs.

--
(gdb) bt
#0  qemu_co_queue_run_restart (co=0x7fba8ff05aa0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:59
#1  0x55c1656f39a9 in qemu_coroutine_enter (co=0x7fba8ff05aa0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
#2  0x55c1656f3e74 in qemu_co_queue_run_restart (co=0x7fba8dd20430) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
#3  0x55c1656f39a9 in qemu_coroutine_enter (co=0x7fba8dd20430) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
#4  0x55c1656f3e74 in qemu_co_queue_run_restart (co=0x7fba8dd14ea0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
#5  0x55c1656f39a9 in qemu_coroutine_enter (co=0x7fba8dd14ea0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
#6  0x55c1656f3e74 in qemu_co_queue_run_restart (co=0x7fba80c11dc0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
#7  0x55c1656f39a9 in qemu_coroutine_enter (co=0x7fba80c11dc0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
#8  0x55c1656f3e74 in qemu_co_queue_run_restart (co=0x7fba8dd0bd70) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
#9  0x55c1656f39a9 in qemu_coroutine_enter (co=0x7fba8dd0bd70) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
#10 0x55c1656f3fa0 in qemu_co_enter_next (queue=queue@entry=0x55c1669e75e0) 
at /build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:106
#11 0x55c165692060 in timer_cb (blk=0x55c1669e7590, is_write=) at /build/pb-qemu-pssKUp/pb-qemu-2.7.0/block/throttle-groups.c:400
#12 0x55c16564f615 in timerlist_run_timers (timer_list=0x55c166a53e80) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/qemu-timer.c:528
#13 0x55c16564f679 in timerlistgroup_run_timers 
(tlg=tlg@entry=0x55c167c81cf8) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/qemu-timer.c:564
#14 0x55c16564ff47 in aio_dispatch (ctx=ctx@entry=0x55c167c81bb0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/aio-posix.c:357
#15 0x55c1656500e8 in aio_poll (ctx=0x55c167c81bb0, blocking=) at /build/pb-qemu-pssKUp/pb-qemu-2.7.0/aio-posix.c:479
#16 0x55c1654b1c79 in iothread_run (opaque=0x55c167c81960) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/iothread.c:46
#17 0x7fbc4b64f0a4 in allocate_stack (stack=, 
pdp=, attr=0x0) at allocatestack.c:416
#18 __pthread_create_2_1 (newthread=, attr=,
start_routine=, arg=) at pthread_create.c:539
Backtrace stopped: Cannot access memory at address 0x8
--

The code that crashes is this
--
void qemu_co_queue_run_restart(Coroutine *co)
{
Coroutine *next;

trace_qemu_co_queue_run_restart(co);
while ((next = QSIMPLEQ_FIRST(&co->co_queue_wakeup))) {
QSIMPLEQ_REMOVE_HEAD(&co->co_queue_wakeup, co_queue_next);   <--- 
Crash occurs here this time
qemu_coroutine_enter(next);
}
}
--

Expanding the macro QSIMPLEQ_REMOVE_HEAD gives us
--
#define QSIMPLEQ_REMOVE_HEAD(head, field) do {  \
if (((head)->sqh_first = (head)->sqh_first->field.sqe_next) == NULL)\
(head)->sqh_last = &(head)->sqh_first;  \
} while (/*CONSTCOND*/0)
--

which corrsponds to
--
if (((&co->co_queue_wakeup)->sqh_first = 
(&co->co_queue_wakeup)->sqh_first->co_queue_next.sqe_next) == NULL)\
(&co->co_queue_wakeup)->sqh_last = &(&co->co_queue_wakeup)->sqh_first;
--

Debugging the list we see
--
(gdb) print *(&co->co_queue_wakeup->sqh_first)
$6 = (struct Coroutine *) 0x1000
(gdb) print *(&co->co_queue_wakeup->sqh_first->co_queue_next)
Cannot access memory at address 0x1030
--

So the data in co->co_queue_wakeup->sqh_first is corrupted and
represents an invalid address. Any idea why is that?

** Affects: qemu
 Importance: Undecided
 Status: New


** Tags: coroutine qemu segfault ubuntu

** Description changed:

  I've been experiencing frequent segfaults lately with qemu 2.7.0 running
  Ubuntu 16.04 guests.

[Qemu-devel] [Bug 1671876] Re: qemu 2.7.0 segfaults in qemu_co_queue_run_restart()

2017-03-10 Thread Mohammed Gamal
Another stack trace

-
(gdb) bt
#0  qemu_co_queue_run_restart (co=0x7f668be15260) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:59
#1  0x564cb19f59a9 in qemu_coroutine_enter (co=0x7f668be15260) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
#2  0x564cb19f5fa0 in qemu_co_enter_next (queue=queue@entry=0x564cb35e55e0) 
at /build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:106
#3  0x564cb1994060 in timer_cb (blk=0x564cb35e5590, is_write=) at /build/pb-qemu-pssKUp/pb-qemu-2.7.0/block/throttle-groups.c:400
#4  0x564cb1951615 in timerlist_run_timers (timer_list=0x564cb3651e80) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/qemu-timer.c:528
#5  0x564cb1951679 in timerlistgroup_run_timers 
(tlg=tlg@entry=0x564cb487fcf8) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/qemu-timer.c:564
#6  0x564cb1951f47 in aio_dispatch (ctx=ctx@entry=0x564cb487fbb0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/aio-posix.c:357
#7  0x564cb19520e8 in aio_poll (ctx=0x564cb487fbb0, blocking=) at /build/pb-qemu-pssKUp/pb-qemu-2.7.0/aio-posix.c:479
#8  0x564cb17b3c79 in iothread_run (opaque=0x564cb487f960) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/iothread.c:46
#9  0x7f684b0b30a4 in allocate_stack (stack=, 
pdp=, attr=0x0) at allocatestack.c:416
#10 __pthread_create_2_1 (newthread=, attr=, 
start_routine=, arg=) at pthread_create.c:539
Backtrace stopped: Cannot access memory at address 0x8
---


Here is a bit of examination of the data
---
(gdb) print *(&co->co_queue_wakeup->sqh_first)
$1 = (struct Coroutine *) 0xc54b578
(gdb) print *(&co->co_queue_wakeup->sqh_first->co_queue_next)
Cannot access memory at address 0xc54b5a8
---

Again seems to be pointing at an invalid address. It's worth noting here
that it the number of restarted and re-run co-routines is much smaller.

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1671876

Title:
  qemu 2.7.0 segfaults in qemu_co_queue_run_restart()

Status in QEMU:
  New

Bug description:
  Hi,

  I've been experiencing frequent segfaults lately with qemu 2.7.0
  running Ubuntu 16.04 guests. The crash usually happens in
  qemu_co_queue_run_restart(). I haven't seen this so far with any other
  guests or distros.

  Here is one back trace I obtained from one of the crashing VMs.

  --
  (gdb) bt
  #0  qemu_co_queue_run_restart (co=0x7fba8ff05aa0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:59
  #1  0x55c1656f39a9 in qemu_coroutine_enter (co=0x7fba8ff05aa0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
  #2  0x55c1656f3e74 in qemu_co_queue_run_restart (co=0x7fba8dd20430) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
  #3  0x55c1656f39a9 in qemu_coroutine_enter (co=0x7fba8dd20430) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
  #4  0x55c1656f3e74 in qemu_co_queue_run_restart (co=0x7fba8dd14ea0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
  #5  0x55c1656f39a9 in qemu_coroutine_enter (co=0x7fba8dd14ea0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
  #6  0x55c1656f3e74 in qemu_co_queue_run_restart (co=0x7fba80c11dc0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
  #7  0x55c1656f39a9 in qemu_coroutine_enter (co=0x7fba80c11dc0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
  #8  0x55c1656f3e74 in qemu_co_queue_run_restart (co=0x7fba8dd0bd70) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:60
  #9  0x55c1656f39a9 in qemu_coroutine_enter (co=0x7fba8dd0bd70) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine.c:119
  #10 0x55c1656f3fa0 in qemu_co_enter_next 
(queue=queue@entry=0x55c1669e75e0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/util/qemu-coroutine-lock.c:106
  #11 0x55c165692060 in timer_cb (blk=0x55c1669e7590, is_write=) at /build/pb-qemu-pssKUp/pb-qemu-2.7.0/block/throttle-groups.c:400
  #12 0x55c16564f615 in timerlist_run_timers (timer_list=0x55c166a53e80) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/qemu-timer.c:528
  #13 0x55c16564f679 in timerlistgroup_run_timers 
(tlg=tlg@entry=0x55c167c81cf8) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/qemu-timer.c:564
  #14 0x55c16564ff47 in aio_dispatch (ctx=ctx@entry=0x55c167c81bb0) at 
/build/pb-qemu-pssKUp/pb-qemu-2.7.0/aio-posix.c:357
  #15 0x55c1656500e8 in aio_poll (ctx=0x55c167c81bb0, blocking=) at /build/pb-qemu-pssKUp/pb-qemu-2.7.0/ai

Re: [Qemu-devel] [PATCH] configure: don't optimize away avx2 test functions

2016-06-21 Thread Mohammed Gamal
Please drop this patch. I see it has already been fixed upstream

On Tue, Jun 21, 2016 at 11:10 AM, Mohammed Gamal
 wrote:
> The configure script contains an embedded test C function
> to test for avx2 support. It gets optimized away with
> gcc 4.7 on wheezy.
> Add __attribute__((optimize("O0"))) to it in order
> to prevent optimizing it away
>
> Signed-off-by: Mohammed Gamal 
> ---
>  configure | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/configure b/configure
> index 8ecf29d..ba31d3a 100755
> --- a/configure
> +++ b/configure
> @@ -1782,7 +1782,7 @@ fi
>
>  cat > $TMPC << EOF
>  static void bar(void) {}
> -static void *bar_ifunc(void) {return (void*) bar;}
> +static void __attribute__((optimize("O0"))) *bar_ifunc(void) {return (void*) 
> bar;}
>  static void foo(void) __attribute__((ifunc("bar_ifunc")));
>  int main(void) { foo(); return 0; }
>  EOF
> --
> 1.9.1
>



-- 
Mohammed Gamal
Software Engineer

ProfitBricks GmbH
Greifswalder Straße 207
D - 10405 Berlin

Tel: +49 30 577 008 20
Email:   mohammed.ga...@profitbricks.com
Web: http://www.profitbricks.de

Sitz der Gesellschaft: Berlin.
Registergericht: Amtsgericht Charlottenburg, HRB 125506B.
Geschäftsführer: Andreas Gauger, Achim Weiss.



[Qemu-devel] [PATCH] configure: don't optimize away avx2 test functions

2016-06-21 Thread Mohammed Gamal
The configure script contains an embedded test C function
to test for avx2 support. It gets optimized away with
gcc 4.7 on wheezy.
Add __attribute__((optimize("O0"))) to it in order
to prevent optimizing it away

Signed-off-by: Mohammed Gamal 
---
 configure | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configure b/configure
index 8ecf29d..ba31d3a 100755
--- a/configure
+++ b/configure
@@ -1782,7 +1782,7 @@ fi
 
 cat > $TMPC << EOF
 static void bar(void) {}
-static void *bar_ifunc(void) {return (void*) bar;}
+static void __attribute__((optimize("O0"))) *bar_ifunc(void) {return (void*) 
bar;}
 static void foo(void) __attribute__((ifunc("bar_ifunc")));
 int main(void) { foo(); return 0; }
 EOF
-- 
1.9.1




Re: [Qemu-devel] CPU topology and hyperthreading

2016-03-21 Thread Mohammed Gamal
Any ideas?

On Thu, Mar 17, 2016 at 4:19 PM, Mohammed Gamal  wrote:
> Hi All,
>
> I have a question regarding the way CPU topology is exposed to the guest.
>
> On a 4-core Amazon AWS VM I can see the CPU topology exposed to the
> guest in the following manner:
>
> # lstopo
> Machine (7480MB)
>   Socket L#0 + L3 L#0 (25MB)
> L2 L#0 (256KB) + L1d L#0 (32KB) + L1i L#0 (32KB) + Core L#0
>   PU L#0 (P#0)
>   PU L#1 (P#2)
> L2 L#1 (256KB) + L1d L#1 (32KB) + L1i L#1 (32KB) + Core L#1
>   PU L#2 (P#1)
>   PU L#3 (P#3)
> [...]
>
> Now trying to emulate this topology in qemu/kvm using the following
> command line options:
> -cpu Haswell,+ht -smp 4,sockets=1,cores=2,maxcpus=64,threads=2
>
> as well as
> -cpu kvm64,+ht -smp 4,sockets=1,cores=2,maxcpus=64,threads=2
>
>
> Shows me something like this:
>
> # lstopo
> Machine (1870MB)
>   Socket L#0
>  L2 L#0 (4096KB) + Core L#0
>L1d L#0 (32KB) + L1i L#0 (32KB) + PU L#0 (P#0)
>L1d L#1 (32KB) + L1i L#1 (32KB) + PU L#1 (P#1)
>  L2 L#1 (4096KB) + Core L#1
>L1d L#2 (32KB) + L1i L#2 (32KB) + PU L#2 (P#2)
>L1d L#3 (32KB) + L1i L#3 (32KB) + PU L#3 (P#3)
> [...]
>
> In other words, qemu exposes each hyperthread as if it has its own L1
> data and instruction caches. Should the be a correct behavior?
>
> In all cases, what gets exposed in the guests's /proc/cpuinfo would be
> the same, but I wonder why the topology is different?
>
> Regards,
> Mohammed



[Qemu-devel] CPU topology and hyperthreading

2016-03-19 Thread Mohammed Gamal
Hi All,

I have a question regarding the way CPU topology is exposed to the guest.

On a 4-core Amazon AWS VM I can see the CPU topology exposed to the
guest in the following manner:

# lstopo
Machine (7480MB)
  Socket L#0 + L3 L#0 (25MB)
L2 L#0 (256KB) + L1d L#0 (32KB) + L1i L#0 (32KB) + Core L#0
  PU L#0 (P#0)
  PU L#1 (P#2)
L2 L#1 (256KB) + L1d L#1 (32KB) + L1i L#1 (32KB) + Core L#1
  PU L#2 (P#1)
  PU L#3 (P#3)
[...]

Now trying to emulate this topology in qemu/kvm using the following
command line options:
-cpu Haswell,+ht -smp 4,sockets=1,cores=2,maxcpus=64,threads=2

as well as
-cpu kvm64,+ht -smp 4,sockets=1,cores=2,maxcpus=64,threads=2


Shows me something like this:

# lstopo
Machine (1870MB)
  Socket L#0
 L2 L#0 (4096KB) + Core L#0
   L1d L#0 (32KB) + L1i L#0 (32KB) + PU L#0 (P#0)
   L1d L#1 (32KB) + L1i L#1 (32KB) + PU L#1 (P#1)
 L2 L#1 (4096KB) + Core L#1
   L1d L#2 (32KB) + L1i L#2 (32KB) + PU L#2 (P#2)
   L1d L#3 (32KB) + L1i L#3 (32KB) + PU L#3 (P#3)
[...]

In other words, qemu exposes each hyperthread as if it has its own L1
data and instruction caches. Should the be a correct behavior?

In all cases, what gets exposed in the guests's /proc/cpuinfo would be
the same, but I wonder why the topology is different?

Regards,
Mohammed



Re: [Qemu-devel] Emulation failure on booting/rebooting VMs

2015-03-19 Thread Mohammed Gamal
On Wed, Mar 18, 2015 at 7:09 PM, Kevin O'Connor  wrote:

> On Wed, Mar 18, 2015 at 06:36:48PM +0100, Mohammed Gamal wrote:
> > Hi,
> > I've been sporadically getting my KVM virtual machines crashing with this
> > message while they're booting
> >
> > KVM internal error. Suberror: 1
> > emulation failure
> > EAX= EBX= ECX= EDX=00600f12
> > ESI= EDI= EBP= ESP=fffa
> > EIP=ff53 EFL=0006 [-P-] CPL=0 II=0 A20=1 SMM=0 HLT=0
> > ES =   9300
> > CS =f000 000f  9b00
> > SS =   9200
> > DS =   9300
> > FS =   9300
> > GS =   9300
> > LDT=   8200
> > TR =   8300
> > GDT=  
> > IDT=  
> > CR0=6010 CR2= CR3= CR4=
> > DR0= DR1= DR2=
> > DR3=
> > DR6=0ff0 DR7=0400
> > EFER=
> > Code=74 65 61 6d 2e 00 66 68 5f 55 00 00 e9 c3 f8 fb f4 cb 66 c3  66
> 68
> > ff e6 00 00 e9 8b b1 66 55 66 57 66 56 66 53 66 89 c1 66 89 d6 a8 07 75
> 23
> > 66 0f
> >
> > I am running qemu 1.2, seabios 1.7.3 and ipxe (1.0.0-f6840ba) and the
> host
> > CPU is AMD Opteron 6386 SE running kernel 3.4.71.
>
> To debug seabios issues, we need the full log - see:
>   http://www.seabios.org/Debugging
>
> But, your software versions are a bit old which makes diagnosing any
> issues hard.  I suggest updating to the latest software and seeing if
> the problem is still present.
>
> -Kevin
>

Unfortunately I can't change that setup. The problem is also rather
sporadic so I have no other means to reproduce it and debug it.
I tried looking into the seabios code but it looks like the instruction
pointer is pointing at the default interrupt vector table entry
'entry_iret_official'. I presume this means that was triggered with an
interrupt of some sort? The other register values don't seem to provide any
clues nevertheless.


[Qemu-devel] Emulation failure on booting/rebooting VMs

2015-03-18 Thread Mohammed Gamal
Hi,
I've been sporadically getting my KVM virtual machines crashing with this
message while they're booting

KVM internal error. Suberror: 1
emulation failure
EAX= EBX= ECX= EDX=00600f12
ESI= EDI= EBP= ESP=fffa
EIP=ff53 EFL=0006 [-P-] CPL=0 II=0 A20=1 SMM=0 HLT=0
ES =   9300
CS =f000 000f  9b00
SS =   9200
DS =   9300
FS =   9300
GS =   9300
LDT=   8200
TR =   8300
GDT=  
IDT=  
CR0=6010 CR2= CR3= CR4=
DR0= DR1= DR2=
DR3=
DR6=0ff0 DR7=0400
EFER=
Code=74 65 61 6d 2e 00 66 68 5f 55 00 00 e9 c3 f8 fb f4 cb 66 c3  66 68
ff e6 00 00 e9 8b b1 66 55 66 57 66 56 66 53 66 89 c1 66 89 d6 a8 07 75 23
66 0f

I am running qemu 1.2, seabios 1.7.3 and ipxe (1.0.0-f6840ba) and the host
CPU is AMD Opteron 6386 SE running kernel 3.4.71.

I digged a little into the kvm kernel module code and I can trace these
emulation failures only to either a call to pf_interception() - which is
highly unlikely since the machine doesn't appear to have setup paging yet -
and task_switch_interception(). I am suspecting that ipxe or seabios might
be executing some invalid code or something, since this failure only
appears sporadically and is not 100% reproducible. Any ideas why this might
be happening?

Regards,
Mohammed


[Qemu-devel] [Bug 1297218] Re: guest hangs after live migration due to tsc jump

2015-03-12 Thread Mohammed Gamal
Ping.

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1297218

Title:
  guest hangs after live migration due to tsc jump

Status in QEMU:
  New
Status in glusterfs package in Ubuntu:
  Invalid
Status in qemu package in Ubuntu:
  Triaged
Status in glusterfs source package in Trusty:
  New
Status in qemu source package in Trusty:
  Confirmed

Bug description:
  We have two identical Ubuntu servers running libvirt/kvm/qemu, sharing
  a Gluster filesystem. Guests can be live migrated between them.
  However, live migration often leads to the guest being stuck at 100%
  for a while. In that case, the dmesg output for such a guest will show
  (once it recovers): Clocksource tsc unstable (delta = 662463064082
  ns). In this particular example, a guest was migrated and only after
  11 minutes (662 seconds) did it become responsive again.

  It seems that newly booted guests doe not suffer from this problem,
  these can be migrated back and forth at will. After a day or so, the
  problem becomes apparent. It also seems that migrating from server A
  to server B causes much more problems than going from B back to A. If
  necessary, I can do more measurements to qualify these observations.

  The VM servers run Ubuntu 13.04 with these packages:
  Kernel: 3.8.0-35-generic x86_64
  Libvirt: 1.0.2
  Qemu: 1.4.0
  Gluster-fs: 3.4.2 (libvirt access the images via the filesystem, not using 
libgfapi yet as the Ubuntu libvirt is not linked against libgfapi).
  The interconnect between both machines (both for migration and gluster) is 
10GbE. 
  Both servers are synced to NTP and well within 1ms form one another.

  Guests are either Ubuntu 13.04 or 13.10.

  On the guests, the current_clocksource is kvm-clock.
  The XML definition of the guests only contains:   

  Now as far as I've read in the documentation of kvm-clock, it specifically 
supports live migrations, so I'm a bit surprised at these problems. There isn't 
all that much information to find on these issue, although I have found 
postings by others that seem to have run into the same issues, but without a 
solution.
  --- 
  ApportVersion: 2.14.1-0ubuntu3
  Architecture: amd64
  DistroRelease: Ubuntu 14.04
  Package: libvirt (not installed)
  ProcCmdline: BOOT_IMAGE=/boot/vmlinuz-3.13.0-24-generic 
root=UUID=1b0c3c6d-a9b8-4e84-b076-117ae267d178 ro console=ttyS1,115200n8 
BOOTIF=01-00-25-90-75-b5-c8
  ProcVersionSignature: Ubuntu 3.13.0-24.47-generic 3.13.9
  Tags:  trusty apparmor apparmor apparmor apparmor apparmor
  Uname: Linux 3.13.0-24-generic x86_64
  UpgradeStatus: No upgrade log present (probably fresh install)
  UserGroups:
   
  _MarkForUpload: True
  modified.conffile..etc.default.libvirt.bin: [modified]
  modified.conffile..etc.libvirt.libvirtd.conf: [modified]
  modified.conffile..etc.libvirt.qemu.conf: [modified]
  modified.conffile..etc.libvirt.qemu.networks.default.xml: [deleted]
  mtime.conffile..etc.default.libvirt.bin: 2014-05-12T19:07:40.020662
  mtime.conffile..etc.libvirt.libvirtd.conf: 2014-05-13T14:40:25.894837
  mtime.conffile..etc.libvirt.qemu.conf: 2014-05-12T18:58:27.885506

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1297218/+subscriptions



[Qemu-devel] [Bug 1297218] Re: guest hangs after live migration due to tsc jump

2015-01-28 Thread Mohammed Gamal
Hi Serge,
Yes, that's the case. Let me also make it clear that this is a backport on top 
of qemu 1.2 stable.

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1297218

Title:
  guest hangs after live migration due to tsc jump

Status in QEMU:
  New
Status in glusterfs package in Ubuntu:
  Invalid
Status in qemu package in Ubuntu:
  Triaged
Status in glusterfs source package in Trusty:
  New
Status in qemu source package in Trusty:
  Confirmed

Bug description:
  We have two identical Ubuntu servers running libvirt/kvm/qemu, sharing
  a Gluster filesystem. Guests can be live migrated between them.
  However, live migration often leads to the guest being stuck at 100%
  for a while. In that case, the dmesg output for such a guest will show
  (once it recovers): Clocksource tsc unstable (delta = 662463064082
  ns). In this particular example, a guest was migrated and only after
  11 minutes (662 seconds) did it become responsive again.

  It seems that newly booted guests doe not suffer from this problem,
  these can be migrated back and forth at will. After a day or so, the
  problem becomes apparent. It also seems that migrating from server A
  to server B causes much more problems than going from B back to A. If
  necessary, I can do more measurements to qualify these observations.

  The VM servers run Ubuntu 13.04 with these packages:
  Kernel: 3.8.0-35-generic x86_64
  Libvirt: 1.0.2
  Qemu: 1.4.0
  Gluster-fs: 3.4.2 (libvirt access the images via the filesystem, not using 
libgfapi yet as the Ubuntu libvirt is not linked against libgfapi).
  The interconnect between both machines (both for migration and gluster) is 
10GbE. 
  Both servers are synced to NTP and well within 1ms form one another.

  Guests are either Ubuntu 13.04 or 13.10.

  On the guests, the current_clocksource is kvm-clock.
  The XML definition of the guests only contains:   

  Now as far as I've read in the documentation of kvm-clock, it specifically 
supports live migrations, so I'm a bit surprised at these problems. There isn't 
all that much information to find on these issue, although I have found 
postings by others that seem to have run into the same issues, but without a 
solution.
  --- 
  ApportVersion: 2.14.1-0ubuntu3
  Architecture: amd64
  DistroRelease: Ubuntu 14.04
  Package: libvirt (not installed)
  ProcCmdline: BOOT_IMAGE=/boot/vmlinuz-3.13.0-24-generic 
root=UUID=1b0c3c6d-a9b8-4e84-b076-117ae267d178 ro console=ttyS1,115200n8 
BOOTIF=01-00-25-90-75-b5-c8
  ProcVersionSignature: Ubuntu 3.13.0-24.47-generic 3.13.9
  Tags:  trusty apparmor apparmor apparmor apparmor apparmor
  Uname: Linux 3.13.0-24-generic x86_64
  UpgradeStatus: No upgrade log present (probably fresh install)
  UserGroups:
   
  _MarkForUpload: True
  modified.conffile..etc.default.libvirt.bin: [modified]
  modified.conffile..etc.libvirt.libvirtd.conf: [modified]
  modified.conffile..etc.libvirt.qemu.conf: [modified]
  modified.conffile..etc.libvirt.qemu.networks.default.xml: [deleted]
  mtime.conffile..etc.default.libvirt.bin: 2014-05-12T19:07:40.020662
  mtime.conffile..etc.libvirt.libvirtd.conf: 2014-05-13T14:40:25.894837
  mtime.conffile..etc.libvirt.qemu.conf: 2014-05-12T18:58:27.885506

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1297218/+subscriptions



Re: [Qemu-devel] kvmclock, Migration, and NTP clock jitter

2015-01-21 Thread Mohammed Gamal
On Fri, Jan 16, 2015 at 11:21 AM, Mohammed Gamal <
mohammed.ga...@profitbricks.com> wrote:

> On Thu, Jan 15, 2015 at 06:27:54PM +0100, Paolo Bonzini wrote:
> >
> >
> > On 15/01/2015 17:39, Mohammed Gamal wrote:
> > > The increase in the jitter and offset values is well within the 500 ppm
> > > frequency tolerance limit, and therefore are easily corrected by
> > > subsequent NTP clock sync events, but some live migrations do cause
> much
> > > higher jitter and offset jumps, which can not be corrected by NTP and
> > > cause the time to go way off. Any idea why this is the case?
> >
> > It might be fixed in QEMU 2.2.
> >
> > See https://lists.gnu.org/archive/html/qemu-devel/2014-09/msg01239.html
> >
> > Paolo
>
> Hi Paolo,
>
> I did try to backport these patches to qemu 1.2. However, migrations
> resulted in *higher* jitter and offset values (i.e. in the order of 100+
> ppm).
> I am not sure if I've done the backporting correctly though. Here are my
> patches on top of the qemu 1.2 stable tree.
>

Anyone?


[Qemu-devel] [Bug 1297218] Re: guest hangs after live migration due to tsc jump

2015-01-19 Thread Mohammed Gamal
Hi,

I've seen some strange time behavior in some of our VMs usually
triggered by live migration. In some VMs we have seen some significant
time drift which NTP was not able to correct after doing a live
migration.

I've not been able so far to reproduce the same case, however, I did
notice that live migration does introduce some increase in clock jitter
values, and I am not sure if that might have anything to do with any
significant time drift.

Here is an example of a CentOS 6 guest running under qemu 1.2 before
doing a live migration:

[root@centos ~]# ntpq -pcrv
 remote   refid  st t when poll reach   delay   offset  jitter
==
+helium.constant 18.26.4.105  2 u   65   64  377   60.539   -0.011   0.554
-209.118.204.201 128.9.176.30 2 u   47   64  377   15.750   -1.835   0.388
*time3.chpc.utah 198.60.22.2402 u   46   64  377   30.5853.934   0.253
+dns2.untangle.c 216.218.254.202  2 u   21   64  377   22.1962.345   0.740
associd=0 status=0615 leap_none, sync_ntp, 1 event, clock_sync,
version="ntpd 4.2.6p5@1.2349-o Sat Dec 20 02:53:39 UTC 2014 (1)",
processor="x86_64", system="Linux/2.6.32-504.3.3.el6.x86_64", leap=00,
stratum=3, precision=-21, rootdelay=32.355, rootdisp=53.173,
refid=155.101.3.115,
reftime=d86264f3.444c75e7  Thu, Jan 15 2015 16:10:27.266,
clock=d86265ed.10a34c1c  Thu, Jan 15 2015 16:14:37.064, peer=3418, tc=6,
mintc=3, offset=0.000, frequency=2.863, sys_jitter=2.024,
clk_jitter=2.283, clk_wander=0.000

[root@centos ~]# ntpdc -c kerninfo
pll offset:   0 s
pll frequency:2.863 ppm
maximum error:0.19838 s
estimated error:  0.002282 s
status:   2001  pll nano
pll time constant:6
precision:1e-09 s
frequency tolerance:  500 ppm

Immediately after live migration, you can see that there is an increase in 
jitter values:
[root@centos ~]# ntpq -pcrv
 remote   refid  st t when poll reach   delay   offset  jitter
==
-helium.constant 18.26.4.105  2 u   59   64  377   60.556   -0.916  31.921
+209.118.204.201 128.9.176.30 2 u   38   64  377   15.717   28.879  12.220
+time3.chpc.utah 132.163.4.1032 u   45   64  353   30.6393.240  26.975
*dns2.untangle.c 216.218.254.202  2 u   17   64  377   22.248   33.039  11.791
associd=0 status=0615 leap_none, sync_ntp, 1 event, clock_sync,
version="ntpd 4.2.6p5@1.2349-o Sat Dec 20 02:53:39 UTC 2014 (1)",
processor="x86_64", system="Linux/2.6.32-504.3.3.el6.x86_64", leap=00,
stratum=3, precision=-21, rootdelay=25.086, rootdisp=83.736,
refid=74.123.29.4,
reftime=d8626838.47529689  Thu, Jan 15 2015 16:24:24.278,
clock=d8626849.4920018a  Thu, Jan 15 2015 16:24:41.285, peer=3419, tc=6,
mintc=3, offset=24.118, frequency=11.560, sys_jitter=15.145,
clk_jitter=8.056, clk_wander=2.757

[root@centos ~]# ntpdc -c kerninfo
pll offset:   0.0211957 s
pll frequency:11.560 ppm
maximum error:0.112523 s
estimated error:  0.008055 s
status:   2001  pll nano
pll time constant:6
precision:1e-09 s
frequency tolerance:  500 ppm


The increase in the jitter and offset values is well within the 500 ppm 
frequency tolerance limit, and therefore are easily corrected by subsequent NTP 
clock sync events, but some live migrations do cause much higher jitter and 
offset jumps, which can not be corrected by NTP and cause the time to go way 
off. Any idea why this is the case?

I've tried backporting the patches
(9a48bcd1b82494671c09b0eefdb882581499 and
317b0a6d8ba44e9bf8f9c3dbd776c4536843d82c) on top of upstream qemu 1.2,
but it actually caused even higher jitter in the order of 100+ ppm.

Any idea what I might be missing?

** Patch added: "backport.patch"
   
https://bugs.launchpad.net/qemu/+bug/1297218/+attachment/4301780/+files/backport.patch

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1297218

Title:
  guest hangs after live migration due to tsc jump

Status in QEMU:
  New
Status in glusterfs package in Ubuntu:
  Invalid
Status in qemu package in Ubuntu:
  Triaged
Status in glusterfs source package in Trusty:
  New
Status in qemu source package in Trusty:
  Confirmed

Bug description:
  We have two identical Ubuntu servers running libvirt/kvm/qemu, sharing
  a Gluster filesystem. Guests can be live migrated between them.
  However, live migration often leads to the guest being stuck at 100%
  for a while. In that case, the dmesg output for such a guest will show
  (once it recovers): Clocksource tsc unstable (delta = 662463064082
  ns). In this particular example, a guest was migrated and only after
  11 minutes (662 seconds) did it become responsive again.

  It seems that newly booted guests doe not suffer from this problem,
  these can be migrated back and forth at will. A

Re: [Qemu-devel] kvmclock, Migration, and NTP clock jitter

2015-01-16 Thread Mohammed Gamal
On Thu, Jan 15, 2015 at 06:27:54PM +0100, Paolo Bonzini wrote:
> 
> 
> On 15/01/2015 17:39, Mohammed Gamal wrote:
> > The increase in the jitter and offset values is well within the 500 ppm
> > frequency tolerance limit, and therefore are easily corrected by
> > subsequent NTP clock sync events, but some live migrations do cause much
> > higher jitter and offset jumps, which can not be corrected by NTP and
> > cause the time to go way off. Any idea why this is the case?
> 
> It might be fixed in QEMU 2.2.
> 
> See https://lists.gnu.org/archive/html/qemu-devel/2014-09/msg01239.html
> 
> Paolo

Hi Paolo,

I did try to backport these patches to qemu 1.2. However, migrations 
resulted in *higher* jitter and offset values (i.e. in the order of 100+ ppm).
I am not sure if I've done the backporting correctly though. Here are my
patches on top of the qemu 1.2 stable tree.
diff --git a/cpus.c b/cpus.c
index 29aced5..e079ee5 100644
--- a/cpus.c
+++ b/cpus.c
@@ -187,6 +187,15 @@ void cpu_disable_ticks(void)
 }
 }
 
+void cpu_clean_all_dirty(void)
+{
+CPUArchState *cpu;
+
+for (cpu = first_cpu; cpu; cpu = cpu->next_cpu) {
+cpu_clean_state(cpu);
+}
+}
+
 /* Correlation between real and virtual time is always going to be
fairly approximate, so ignore small variation.
When the guest is idle real and virtual time will be aligned in
diff --git a/cpus.h b/cpus.h
index 3fc1a4a..1ff166b 100644
--- a/cpus.h
+++ b/cpus.h
@@ -12,6 +12,7 @@ void unplug_vcpu(void *p);
 void cpu_synchronize_all_states(void);
 void cpu_synchronize_all_post_reset(void);
 void cpu_synchronize_all_post_init(void);
+void cpu_clean_all_dirty(void);
 
 void qtest_clock_warp(int64_t dest);
 
diff --git a/hw/kvm/clock.c b/hw/kvm/clock.c
index 824b978..b2bdda4 100644
--- a/hw/kvm/clock.c
+++ b/hw/kvm/clock.c
@@ -16,6 +16,8 @@
 #include "qemu-common.h"
 #include "sysemu.h"
 #include "kvm.h"
+#include "host-utils.h"
+#include "cpus.h"
 #include "hw/sysbus.h"
 #include "hw/kvm/clock.h"
 
@@ -28,6 +30,46 @@ typedef struct KVMClockState {
 bool clock_valid;
 } KVMClockState;
 
+struct pvclock_vcpu_time_info {
+uint32_t   version;
+uint32_t   pad0;
+uint64_t   tsc_timestamp;
+uint64_t   system_time;
+uint32_t   tsc_to_system_mul;
+int8_t tsc_shift;
+uint8_tflags;
+uint8_tpad[2];
+} __attribute__((__packed__)); /* 32 bytes */
+
+static uint64_t kvmclock_current_nsec(KVMClockState *s)
+{
+CPUArchState *env = first_cpu;
+uint64_t migration_tsc = env->tsc;
+struct pvclock_vcpu_time_info time;
+uint64_t delta;
+uint64_t nsec_lo;
+uint64_t nsec_hi;
+uint64_t nsec;
+
+if (!(env->system_time_msr & 1ULL)) {
+/* KVM clock not active */
+return 0;
+}
+cpu_physical_memory_read((env->system_time_msr & ~1ULL), &time, sizeof(time));
+
+assert(time.tsc_timestamp <= migration_tsc);
+delta = migration_tsc - time.tsc_timestamp;
+if (time.tsc_shift < 0) {
+delta >>= -time.tsc_shift;
+} else {
+delta <<= time.tsc_shift;
+}
+
+mulu64(&nsec_lo, &nsec_hi, delta, time.tsc_to_system_mul);
+nsec = (nsec_lo >> 32) | (nsec_hi << 32);
+return nsec + time.system_time;
+}
+
 static void kvmclock_pre_save(void *opaque)
 {
 KVMClockState *s = opaque;
@@ -37,6 +79,23 @@ static void kvmclock_pre_save(void *opaque)
 if (s->clock_valid) {
 return;
 }
+
+cpu_synchronize_all_states();
+/* In theory, the cpu_synchronize_all_states() call above wouldn't
+ * affect the rest of the code, as the VCPU state inside CPUArchState
+ * is supposed to always match the VCPU state on the kernel side.
+ *
+ * In practice, calling cpu_synchronize_state() too soon will load the
+ * kernel-side APIC state into X86CPU.apic_state too early, APIC state
+ * won't be reloaded later because CPUState.vcpu_dirty==true, and
+ * outdated APIC state may be migrated to another host.
+ *
+ * The real fix would be to make sure outdated APIC state is read
+ * from the kernel again when necessary. While this is not fixed, we
+ * need the cpu_clean_all_dirty() call below.
+ */
+cpu_clean_all_dirty();
+
 ret = kvm_vm_ioctl(kvm_state, KVM_GET_CLOCK, &data);
 if (ret < 0) {
 fprintf(stderr, "KVM_GET_CLOCK failed: %s\n", strerror(ret));
@@ -55,6 +114,12 @@ static int kvmclock_post_load(void *opaque, int version_id)
 {
 KVMClockState *s = opaque;
 struct kvm_clock_data data;
+uint64_t time_at_migration = kvmclock_current_nsec(s);
+
+/* We can't rely on the migrated clock value, just discard it */
+if (time_at_migration) {
+s->clock = time_at_migration;
+}
 
 data.clock = s->clock;
 data.flags = 0;

[Qemu-devel] kvmclock, Migration, and NTP clock jitter

2015-01-15 Thread Mohammed Gamal
Hi,

I've seen some strange time behavior in some of our VMs usually triggered
by live migration. In some VMs we have seen some significant time drift
which NTP was not able to correct after doing a live migration.

I've not been able so far to reproduce the same case, however, I did notice
that live migration does introduce some increase in clock jitter values,
and I am not sure if that might have anything to do with any significant
time drift.

Here is an example of a CentOS 6 guest running under qemu 1.2 before doing
a live migration:

[root@centos ~]# ntpq -pcrv
 remote   refid  st t when poll reach   delay   offset
 jitter
==
+helium.constant 18.26.4.105  2 u   65   64  377   60.539   -0.011
0.554
-209.118.204.201 128.9.176.30 2 u   47   64  377   15.750   -1.835
0.388
*time3.chpc.utah 198.60.22.2402 u   46   64  377   30.5853.934
0.253
+dns2.untangle.c 216.218.254.202  2 u   21   64  377   22.1962.345
0.740
associd=0 status=0615 leap_none, sync_ntp, 1 event, clock_sync,
version="ntpd 4.2.6p5@1.2349-o Sat Dec 20 02:53:39 UTC 2014 (1)",
processor="x86_64", system="Linux/2.6.32-504.3.3.el6.x86_64", leap=00,
stratum=3, precision=-21, rootdelay=32.355, rootdisp=53.173,
refid=155.101.3.115,
reftime=d86264f3.444c75e7  Thu, Jan 15 2015 16:10:27.266,
clock=d86265ed.10a34c1c  Thu, Jan 15 2015 16:14:37.064, peer=3418, tc=6,
mintc=3, offset=0.000, frequency=2.863, sys_jitter=2.024,
clk_jitter=2.283, clk_wander=0.000

[root@centos ~]# ntpdc -c kerninfo
pll offset:   0 s
pll frequency:2.863 ppm
maximum error:0.19838 s
estimated error:  0.002282 s
status:   2001  pll nano
pll time constant:6
precision:1e-09 s
frequency tolerance:  500 ppm

Immediately after live migration, you can see that there is an increase in
jitter values:
[root@centos ~]# ntpq -pcrv
 remote   refid  st t when poll reach   delay   offset
 jitter
==
-helium.constant 18.26.4.105  2 u   59   64  377   60.556   -0.916
 31.921
+209.118.204.201 128.9.176.30 2 u   38   64  377   15.717   28.879
 12.220
+time3.chpc.utah 132.163.4.1032 u   45   64  353   30.6393.240
 26.975
*dns2.untangle.c 216.218.254.202  2 u   17   64  377   22.248   33.039
 11.791
associd=0 status=0615 leap_none, sync_ntp, 1 event, clock_sync,
version="ntpd 4.2.6p5@1.2349-o Sat Dec 20 02:53:39 UTC 2014 (1)",
processor="x86_64", system="Linux/2.6.32-504.3.3.el6.x86_64", leap=00,
stratum=3, precision=-21, rootdelay=25.086, rootdisp=83.736,
refid=74.123.29.4,
reftime=d8626838.47529689  Thu, Jan 15 2015 16:24:24.278,
clock=d8626849.4920018a  Thu, Jan 15 2015 16:24:41.285, peer=3419, tc=6,
mintc=3, offset=24.118, frequency=11.560, sys_jitter=15.145,
clk_jitter=8.056, clk_wander=2.757

[root@centos ~]# ntpdc -c kerninfo
pll offset:   0.0211957 s
pll frequency:11.560 ppm
maximum error:0.112523 s
estimated error:  0.008055 s
status:   2001  pll nano
pll time constant:6
precision:1e-09 s
frequency tolerance:  500 ppm


The increase in the jitter and offset values is well within the 500 ppm
frequency tolerance limit, and therefore are easily corrected by subsequent
NTP clock sync events, but some live migrations do cause much higher jitter
and offset jumps, which can not be corrected by NTP and cause the time to
go way off. Any idea why this is the case?

Regards,
Mohammed


[Qemu-devel] QMP unix socket randomly returning -EAGAIN

2014-12-12 Thread Mohammed Gamal
Hi,

We are experiencing random errors with communication with VMs with QMP via
unix sockets. At some moment of time the QMP socket keeps on returning
-EAGAIN and never recovering from this state unless the qemu process is
stopped then started. This socket hickup happens specfically when
connecting to the socket or receiving a response from it.

I suspected that this might be caused by the guest hanging on some IO or
being unresponsive for any reason,  but the VMs in question were not
unresponsive at these times and could be accessed via VNC or SSH normally.
I've seen this on machines runnnig qemu 1.0,1 1.2.0 and 2.0.0

I am suspecting it might be a bug in the monitor code, but I couldn't reach
anything tangible. Any ideas what might be causing this?

Regards,
Mohammed


[Qemu-devel] Discrepancy representing slot IDs in query-memory-devices and query-acpi-ospm-status

2014-08-13 Thread Mohammed Gamal
Hi,

QMP command 'query-memory-devices' returns DIMM slots IDs as integers,
while 'query-acpi-ospm-status' returns them as strings. Why is this the
case? Aren't slot IDs numeric anyway?

I've looked up the ACPI specs but I can't find any specific mention of how
DIMM IDs should be represented, so what's the reason for this design choice?

BR,
Mohammed


Re: [Qemu-devel] [Bug 267542] Re: MINIX 3 won't boot in qemu 0.9.1

2010-05-20 Thread Mohammed Gamal
On Thu, May 20, 2010 at 12:44 PM, Andre Przywara  wrote:
> Is that still a problem? What was the exact error?
> I quickly tried the 3.1.2a on qemu 0.12.4 (with and without KVM) and I could 
> easily login.
This happens with MINIX 3.1.6, during boot it briefly goes into an
invalid state while switching to protected mode IIRC.

>
>
> ** Changed in: qemu
>       Status: New => Incomplete
>
> --
> MINIX 3 won't boot in qemu 0.9.1
> https://bugs.launchpad.net/bugs/267542
> You received this bug notification because you are a member of qemu-
> devel-ml, which is subscribed to QEMU.
>
> Status in QEMU: Incomplete
>
> Bug description:
> CD Image 3.1.2a was downloaded from http://www.minix3.org/download/
>
> It booted with previous version of qemu but hangs at startup with 0.9.1.
>
> Hardware acceleration is disabled.
>
> Please ask if there is other information I can give you.
>
>
>
>



Re: [Qemu-devel] qemu-kvm problem with DOS/4GW extender and EMM386.EXE

2010-05-11 Thread Mohammed Gamal
On Tue, May 11, 2010 at 11:56 PM, Andy Walls  wrote:
> Running an MS-DOS 6.22 image with qemu-kvm on a RedHat Linux OS, I
> noticed the guest OS becomes hung and my dmesg gets spammed with
>
>        set_cr0: #GP, set PG flag with a clear PE flag
>
> That message appears to be the linux kernel's kvm emulator griping about
> Paging Enable bit being enabled while the Protection Enable bit is set
> for real mode.  (The Intel manual says this should be a protection
> fault).
>
> The program that causes this has the DOS/4GW DOS extender runtime
> compiled into it.
>
> I found that when I don't load the EMM386.EXE memory manager, the
> problem doesn't occur.
>
> Here's a kvmtrace segment of when things are not working:
>
> 0 (+           0)  IO_WRITE      vcpu = 0x  pid = 0x1997 [ port = 
> 0x0070, size = 1 ]
> 28471049668815 (+        4000)  VMENTRY       vcpu = 0x  pid = 
> 0x1997
> 28471049671815 (+        3000)  VMEXIT        vcpu = 0x  pid = 
> 0x1997 [ exitcode = 0x004e, rip = 0x 2a18 ]
> 0 (+           0)  PAGE_FAULT    vcpu = 0x  pid = 0x1997 [ 
> errorcode = 0x, virt = 0x 0001ba28 ]
> 28471049675815 (+        4000)  VMENTRY       vcpu = 0x  pid = 
> 0x1997
> 28471049678815 (+        3000)  VMEXIT        vcpu = 0x  pid = 
> 0x1997 [ exitcode = 0x004e, rip = 0x 0334 ]
> 0 (+           0)  PAGE_FAULT    vcpu = 0x  pid = 0x1997 [ 
> errorcode = 0x, virt = 0x 00019344 ]
> 28471049681815 (+        3000)  VMENTRY       vcpu = 0x  pid = 
> 0x1997
> 28471049685815 (+        4000)  VMEXIT        vcpu = 0x  pid = 
> 0x1997 [ exitcode = 0x, rip = 0x 02a7 ]
> 0 (+           0)  CR_READ       vcpu = 0x  pid = 0x1997 [ CR# = 
> 0, value = 0x 8011 ]
> 28471049688815 (+        3000)  VMENTRY       vcpu = 0x  pid = 
> 0x1997
> 28471049691815 (+        3000)  VMEXIT        vcpu = 0x  pid = 
> 0x1997 [ exitcode = 0x0010, rip = 0x 02ae ]
> 0 (+           0)  LMSW          vcpu = 0x  pid = 0x1997 [ value 
> = 0x8011 ]
> 28471049696815 (+        5000)  VMENTRY       vcpu = 0x  pid = 
> 0x1997
> 28471049699815 (+        3000)  VMEXIT        vcpu = 0x  pid = 
> 0x1997 [ exitcode = 0x004e, rip = 0x 5593 ]
> 0 (+           0)  PAGE_FAULT    vcpu = 0x  pid = 0x1997 [ 
> errorcode = 0x, virt = 0x 000262e3 ]
> 28471049703815 (+        4000)  VMENTRY       vcpu = 0x  pid = 
> 0x1997
> 28471049706815 (+        3000)  VMEXIT        vcpu = 0x  pid = 
> 0x1997 [ exitcode = 0x004e, rip = 0x 44d6 ]
> 0 (+           0)  PAGE_FAULT    vcpu = 0x  pid = 0x1997 [ 
> errorcode = 0x, virt = 0x 00025226 ]
> 28471049709815 (+        3000)  VMENTRY       vcpu = 0x  pid = 
> 0x1997
> 28471049713815 (+        4000)  VMEXIT        vcpu = 0x  pid = 
> 0x1997 [ exitcode = 0x004e, rip = 0x 55c0 ]
> 0 (+           0)  PAGE_FAULT    vcpu = 0x  pid = 0x1997 [ 
> errorcode = 0x0002, virt = 0x 00024f79 ]
> 28471049717815 (+        4000)  VMENTRY       vcpu = 0x  pid = 
> 0x1997
> 28471049721815 (+        4000)  VMEXIT        vcpu = 0x  pid = 
> 0x1997 [ exitcode = 0x, rip = 0x 2a69 ]
> 0 (+           0)  CR_READ       vcpu = 0x  pid = 0x1997 [ CR# = 
> 0, value = 0x 8011 ]
> 28471049723815 (+        2000)  VMENTRY       vcpu = 0x  pid = 
> 0x1997
> 28471049726815 (+        3000)  VMEXIT        vcpu = 0x  pid = 
> 0x1997 [ exitcode = 0x0010, rip = 0x 2a73 ]
> 0 (+           0)  LMSW          vcpu = 0x  pid = 0x1997 [ value 
> = 0x8010 ]
> 28471049781815 (+       55000)  VMENTRY       vcpu = 0x  pid = 
> 0x1997
> 28471049784815 (+        3000)  VMEXIT        vcpu = 0x  pid = 
> 0x1997 [ exitcode = 0x004e, rip = 0x 1fb8 ]
> 0 (+           0)  PAGE_FAULT    vcpu = 0x  pid = 0x1997 [ 
> errorcode = 0x, virt = 0x 00022d08 ]
> 28471049788815 (+        4000)  VMENTRY       vcpu = 0x  pid = 
> 0x1997
> 28471049792815 (+        4000)  VMEXIT        vcpu = 0x  pid = 
> 0x1997 [ exitcode = 0x007b, rip = 0x 1fd6 ]
> 0 (+           0)  IO_WRITE      vcpu = 0x  pid = 0x1997 [ port = 
> 0x0020, size = 1 ]
> 28471049794815 (+        2000)  VMENTRY       vcpu = 0x  pid = 
> 0x1997
> 28471049797815 (+        3000)  VMEXIT        vcpu = 0x  pid = 
> 0x1997 [ exitcode = 0x007b, rip = 0x 1fd9 ]
> 0 (+           0)  IO_READ       vcpu = 0x  pid = 0x1997 [ port = 
> 0x0020, size = 1 ]
> 28471049800815 (+        3000)  VMENTRY     

Re: [Qemu-devel] [GSoC 2010] Pass-through filesystem support.

2010-04-20 Thread Mohammed Gamal
On Tue, Apr 20, 2010 at 8:36 PM, jvrao  wrote:

...  ...

>> This'd be something interesting to do. I wonder if that would fit in
>> the GSoC timeframe, or whether it'd be a little too short. So how long
>> you'd estimate something like that would take?
>
> I think it would take ~3PM for someone with decent VFS/NFS knowledge.
> They key is fh-to-dentry mapping. In the loose cache mode client caches
> this information .. but even in this mode we can't assume that it will be 
> cached
> forever. Need protocol amendments, client/server side changes to implement
> this in the no-cache mode which can be used even in the loose cache mode when
> we get a cache-miss.
>
> Thanks,
> JV

I think I'd be glad to go for virtio-9p in GSoC. The roadmap is a
little bit hazy for me at the moment but I think we can set the goals.
I'd appreciate some pointers as to where to get more info on what to
do and if there is any relevant documentation on that matter.

Regards,
Mohammed




Re: [Qemu-devel] [GSoC 2010] Pass-through filesystem support.

2010-04-19 Thread Mohammed Gamal
On Tue, Apr 20, 2010 at 12:54 AM, jvrao  wrote:
> Mohammed Gamal wrote:
>> On Tue, Apr 13, 2010 at 9:08 PM, jvrao  wrote:
>>> jvrao wrote:
>>>> Alexander Graf wrote:
>>>>> On 12.04.2010, at 13:58, Jamie Lokier wrote:
>>>>>
>>>>>> Mohammed Gamal wrote:
>>>>>>> On Mon, Apr 12, 2010 at 12:29 AM, Jamie Lokier  
>>>>>>> wrote:
>>>>>>>> Javier Guerra Giraldez wrote:
>>>>>>>>> On Sat, Apr 10, 2010 at 7:42 AM, Mohammed Gamal 
>>>>>>>>>  wrote:
>>>>>>>>>> On Sat, Apr 10, 2010 at 2:12 PM, Jamie Lokier  
>>>>>>>>>> wrote:
>>>>>>>>>>> To throw a spanner in, the most widely supported filesystem across
>>>>>>>>>>> operating systems is probably NFS, version 2 :-)
>>>>>>>>>> Remember that Windows usage on a VM is not some rare use case, and
>>>>>>>>>> it'd be a little bit of a pain from a user's perspective to have to
>>>>>>>>>> install a third party NFS client for every VM they use. Having
>>>>>>>>>> something supported on the VM out of the box is a better option IMO.
>>>>>>>>> i don't think virtio-CIFS has any more support out of the box (on any
>>>>>>>>> system) than virtio-9P.
>>>>>>>> It doesn't, but at least network-CIFS tends to work ok and is the
>>>>>>>> method of choice for Windows VMs - when you can setup Samba on the
>>>>>>>> host (which as previously noted you cannot always do non-disruptively
>>>>>>>> with current Sambas).
>>>>>>>>
>>>>>>>> -- Jamie
>>>>>>>>
>>>>>>> I think having support for both 9p and CIFS would be the best option.
>>>>>>> In that case the user will have the option to use either one,
>>>>>>> depending on how their guests support these filesystems. In that case
>>>>>>> I'd prefer to work on CIFS support while the 9p effort can still go
>>>>>>> on. I don't think both efforts are mutually exclusive.
>>>>>>>
>>>>>>> What do the rest of you guys think?
>>>>>> I only noted NFS because most old OSes do not support CIFS or 9P -
>>>>>> especially all the old unixes.
>>>>>>
>>>>>> I don't think old versions of MS-DOS and Windows (95, 98, ME, Nt4?)
>>>>>> even support current CIFS.  They need extra server settings to work
>>>>>> - such as setting passwords on the server to non-encrypted and other 
>>>>>> quirks.
>>>>>>
>>>>>> Meanwhile Windows Vista/2008/7 works better with SMB2, not CIFS, to
>>>>>> properly see symlinks and hard links.
>>>>>>
>>>>>> So there is no really nice out of the box file service which works
>>>>>> easily with all guest OSes.
>>>>>>
>>>>>> I'm guessing that out of all the filesystems, CIFS is the most widely
>>>>>> supported in recent OSes (released in the last 10 years).  But I'm not
>>>>>> really sure what the state of CIFS is for non-Windows, non-Linux,
>>>>>> non-BSD guests.
>>>>> So what? If you want to have direct host fs access, install guest 
>>>>> drivers. If you can't, set up networking and use CIFS or NFS or whatever.
>>>>>
>>>>>> I'm not sure why 9P is being pursued.  Does anything much support it,
>>>>>> or do all OSes except quite recent Linux need a custom driver for 9P?
>>>>>> Even Linux only got the first commit in the kernel 5 years ago, so
>>>>>> probably it was only about 3 years ago that it will have begun
>>>>>> appearing in stable distros, if at all.  Filesystem passthrough to
>>>>>> Linux guests installed in the last couple of years is a useful
>>>>>> feature, and I know that for many people that is their only use of
>>>>>> KVM, but compared with CIFS' broad support it seems like quite a
>>>>>> narrow goal.
>>>>> The goal is to have something simple and fast. We can fine-tune 9P to 
>>>>> align with the Linux VFS structures, making it really 

Re: [Qemu-devel] [GSoC 2010] Pass-through filesystem support.

2010-04-13 Thread Mohammed Gamal
On Tue, Apr 13, 2010 at 9:08 PM, jvrao  wrote:
> jvrao wrote:
>> Alexander Graf wrote:
>>> On 12.04.2010, at 13:58, Jamie Lokier wrote:
>>>
>>>> Mohammed Gamal wrote:
>>>>> On Mon, Apr 12, 2010 at 12:29 AM, Jamie Lokier  
>>>>> wrote:
>>>>>> Javier Guerra Giraldez wrote:
>>>>>>> On Sat, Apr 10, 2010 at 7:42 AM, Mohammed Gamal  
>>>>>>> wrote:
>>>>>>>> On Sat, Apr 10, 2010 at 2:12 PM, Jamie Lokier  
>>>>>>>> wrote:
>>>>>>>>> To throw a spanner in, the most widely supported filesystem across
>>>>>>>>> operating systems is probably NFS, version 2 :-)
>>>>>>>> Remember that Windows usage on a VM is not some rare use case, and
>>>>>>>> it'd be a little bit of a pain from a user's perspective to have to
>>>>>>>> install a third party NFS client for every VM they use. Having
>>>>>>>> something supported on the VM out of the box is a better option IMO.
>>>>>>> i don't think virtio-CIFS has any more support out of the box (on any
>>>>>>> system) than virtio-9P.
>>>>>> It doesn't, but at least network-CIFS tends to work ok and is the
>>>>>> method of choice for Windows VMs - when you can setup Samba on the
>>>>>> host (which as previously noted you cannot always do non-disruptively
>>>>>> with current Sambas).
>>>>>>
>>>>>> -- Jamie
>>>>>>
>>>>> I think having support for both 9p and CIFS would be the best option.
>>>>> In that case the user will have the option to use either one,
>>>>> depending on how their guests support these filesystems. In that case
>>>>> I'd prefer to work on CIFS support while the 9p effort can still go
>>>>> on. I don't think both efforts are mutually exclusive.
>>>>>
>>>>> What do the rest of you guys think?
>>>> I only noted NFS because most old OSes do not support CIFS or 9P -
>>>> especially all the old unixes.
>>>>
>>>> I don't think old versions of MS-DOS and Windows (95, 98, ME, Nt4?)
>>>> even support current CIFS.  They need extra server settings to work
>>>> - such as setting passwords on the server to non-encrypted and other 
>>>> quirks.
>>>>
>>>> Meanwhile Windows Vista/2008/7 works better with SMB2, not CIFS, to
>>>> properly see symlinks and hard links.
>>>>
>>>> So there is no really nice out of the box file service which works
>>>> easily with all guest OSes.
>>>>
>>>> I'm guessing that out of all the filesystems, CIFS is the most widely
>>>> supported in recent OSes (released in the last 10 years).  But I'm not
>>>> really sure what the state of CIFS is for non-Windows, non-Linux,
>>>> non-BSD guests.
>>> So what? If you want to have direct host fs access, install guest drivers. 
>>> If you can't, set up networking and use CIFS or NFS or whatever.
>>>
>>>> I'm not sure why 9P is being pursued.  Does anything much support it,
>>>> or do all OSes except quite recent Linux need a custom driver for 9P?
>>>> Even Linux only got the first commit in the kernel 5 years ago, so
>>>> probably it was only about 3 years ago that it will have begun
>>>> appearing in stable distros, if at all.  Filesystem passthrough to
>>>> Linux guests installed in the last couple of years is a useful
>>>> feature, and I know that for many people that is their only use of
>>>> KVM, but compared with CIFS' broad support it seems like quite a
>>>> narrow goal.
>>> The goal is to have something simple and fast. We can fine-tune 9P to align 
>>> with the Linux VFS structures, making it really little overhead (and little 
>>> headache too). For Windows guests, nothing prevents us to expose yet 
>>> another 9P flavor. That again would be aligned well with Windows's VFS and 
>>> be slim and fast there.
>>>
>>> The biggest problem I see with CIFS is that it's a huge beast. There are a 
>>> lot of corner cases where it just doesn't fit in. See my previous mail for 
>>> more details.
>>>
>> As Alex mentioned, 9P is chosen for its mere simplicity and easy 
>> adaptability.
>> NFS and CIFS does not give that flexibility. As we mentioned in the patch 
>> series, we are
>> already seeing better numbers with 9P. Looking ahead 9P can embed KVM/QEMU 
>> knowledge
>> to share physical resources like memory/cache between the host and the guest.
>>
>> I think looking into the windows side of 9P client would be great option too.
>> The current patch on the mailing list supports .U (unix) protocol and will 
>> be introducing
>> .L (Linux) variant as we move forward.
>
> Hi Mohammed,
> Please let us know once you decide on where your interest lies.
> Will be glad to have you on VirtFS (9P) though. :)
>
>
> - JV
>

It seems the community is more keen on getting 9P support merged than
getting CIFS supported, and they have made good points to support
their argument. I'm not sure whether work on this project could fit in
as a GSoC project and if there is much remaining work that could make
it fit in that direction. But I'd be glad to volunteer anyway :)

Regards,
Mohammed




Re: [Qemu-devel] [GSoC 2010] Pass-through filesystem support.

2010-04-12 Thread Mohammed Gamal
On Mon, Apr 12, 2010 at 12:29 AM, Jamie Lokier  wrote:
> Javier Guerra Giraldez wrote:
>> On Sat, Apr 10, 2010 at 7:42 AM, Mohammed Gamal  wrote:
>> > On Sat, Apr 10, 2010 at 2:12 PM, Jamie Lokier  wrote:
>> >> To throw a spanner in, the most widely supported filesystem across
>> >> operating systems is probably NFS, version 2 :-)
>> >
>> > Remember that Windows usage on a VM is not some rare use case, and
>> > it'd be a little bit of a pain from a user's perspective to have to
>> > install a third party NFS client for every VM they use. Having
>> > something supported on the VM out of the box is a better option IMO.
>>
>> i don't think virtio-CIFS has any more support out of the box (on any
>> system) than virtio-9P.
>
> It doesn't, but at least network-CIFS tends to work ok and is the
> method of choice for Windows VMs - when you can setup Samba on the
> host (which as previously noted you cannot always do non-disruptively
> with current Sambas).
>
> -- Jamie
>

I think having support for both 9p and CIFS would be the best option.
In that case the user will have the option to use either one,
depending on how their guests support these filesystems. In that case
I'd prefer to work on CIFS support while the 9p effort can still go
on. I don't think both efforts are mutually exclusive.

What do the rest of you guys think?

Regards,
Mohammed




Re: [Qemu-devel] [GSoC 2010] Pass-through filesystem support.

2010-04-10 Thread Mohammed Gamal
On Sat, Apr 10, 2010 at 2:12 PM, Jamie Lokier  wrote:
> Mohammed Gamal wrote:
>> Hi Javier,
>> Thanks for the link. However, I'm still concerned with
>> interoperability with other operating systems, including non-Windows
>> ones. I am not sure of how many operating systems actually support 9p,
>> but I'm almost certain that CIFS would be more widely-supported.
>> I am still a newbie as far as all this is concerned, so if anyone has
>> any arguments as to whether which approach should be taken, I'd be
>> enlightened to hear them.
>
> To throw a spanner in, the most widely supported filesystem across
> operating systems is probably NFS, version 2 :-)
>
> -- Jamie
>

Remember that Windows usage on a VM is not some rare use case, and
it'd be a little bit of a pain from a user's perspective to have to
install a third party NFS client for every VM they use. Having
something supported on the VM out of the box is a better option IMO.

Regards,
Mohammed




Re: [Qemu-devel] [GSoC 2010] Pass-through filesystem support.

2010-04-09 Thread Mohammed Gamal
On Sat, Apr 10, 2010 at 12:22 AM, Javier Guerra Giraldez
 wrote:
> On Fri, Apr 9, 2010 at 5:17 PM, Mohammed Gamal  wrote:
>> That's all good and well. The question now is which direction would
>> the community prefer to go. Would everyone be just happy with
>> virtio-9p passthrough? Would it support multiple OSs (Windows comes to
>> mind here)? Or would we eventually need to patch Samba for passthrough
>> filesystems?
>
> found this:
>
> http://code.google.com/p/ninefs/
>
> it's a BSD-licensed 9p client for windows i have no idea of how
> stable / complete / trustable it is; but might be some start
>
>
> --
> Javier
>

Hi Javier,
Thanks for the link. However, I'm still concerned with
interoperability with other operating systems, including non-Windows
ones. I am not sure of how many operating systems actually support 9p,
but I'm almost certain that CIFS would be more widely-supported.
I am still a newbie as far as all this is concerned, so if anyone has
any arguments as to whether which approach should be taken, I'd be
enlightened to hear them.

Regards,
Mohammed




Re: [Qemu-devel] [GSoC 2010] Pass-through filesystem support.

2010-04-09 Thread Mohammed Gamal
On Fri, Apr 9, 2010 at 11:22 PM, Jamie Lokier  wrote:
> Mohammed Gamal wrote:
>> 2- With respect to CIFS. I wonder how the shares are supposed to be
>> exposed to the guest. Should the Samba server be modified to be able
>> to use unix domain sockets instead of TCP ports and then QEMU
>> communicating on these sockets. With that approach, how should the
>> guest be able to see the exposed share? And what is the problem of
>> using Samba with TCP ports?
>
> One problem with TCP ports is it only works when the guest's network
> is up :) You can't boot from that.  It also makes things fragile or
> difficult if the guest work you are doing involves fiddling with the
> network settings.
>
> Doing it over virtio-serial would have many benefits.
>
> On the other hand, Samba+TCP+CIFS does have the advantage of working
> with virtually all guest OSes, including Linux / BSDs / Windows /
> MacOSX / Solaris etc.  9P only works with Linux as far as I know.
>
> I big problem with Samba at the moment is it's not possible to
> instantiate multiple instances of Samba any more, and not as a
> non-root user.  That's because it contains some hard-coded paths to
> directories of run-time state, at least on Debian/Ubuntu hosts where I
> have tried and failed to use qemu's smb option, and there is no config
> file option to disable that or even change all the paths.
>
> Patching Samba to make per-user instantiations possible again would go
> a long way to making it useful for filesystem passthrough.  Patching
> it so you can turn off all the fancy features and have it _just_ serve
> a filesystem with the most basic necessary authentication would be
> even better.
>
> -- Jamie
>

Hi Jamie,

Thanks for your input.

That's all good and well. The question now is which direction would
the community prefer to go. Would everyone be just happy with
virtio-9p passthrough? Would it support multiple OSs (Windows comes to
mind here)? Or would we eventually need to patch Samba for passthrough
filesystems?

Regards,
Mohammed




Re: [Qemu-devel] [GSoC 2010] Pass-through filesystem support.

2010-04-09 Thread Mohammed Gamal
On Fri, Apr 9, 2010 at 7:11 PM, jvrao  wrote:
> Luiz Capitulino wrote:
>> On Thu, 8 Apr 2010 18:01:01 +0200
>> Mohammed Gamal  wrote:
>>
>>> Hi,
>>> Now that Cam is almost done with his ivshmem patches, I was thinking
>>> of another idea for GSoC which is improving the pass-though
>>> filesystems.
>>> I've got some questions on that:
>>>
>>> 1- What does the community prefer to use and improve? CIFS, 9p, or
>>> both? And which is better taken up for GSoC.
>
> Please look at our recent set of patches.
> We are developing a 9P server for QEMU and client is already part of mainline 
> Linux.
> Our goal is to optimize it for virualization environment and will work as FS 
> pass-through
> mechanism between host and the guest.
>
> Here is the latest set of patches..
>
> http://www.mail-archive.com/qemu-devel@nongnu.org/msg29267.html
>
> Please let us know if you are interested ... we can coordinate.
>
> Thanks,
> JV
>

I'd be interested indeed.

>>>
>>> 2- With respect to CIFS. I wonder how the shares are supposed to be
>>> exposed to the guest. Should the Samba server be modified to be able
>>> to use unix domain sockets instead of TCP ports and then QEMU
>>> communicating on these sockets. With that approach, how should the
>>> guest be able to see the exposed share? And what is the problem of
>>> using Samba with TCP ports?
>>>
>>> 3- In addition, I see the idea mentions that some Windows code needs
>>> to be written to use network shares on a special interface. What's
>>> that interface? And what's the nature of that Windows code? (a driver
>>> a la "guest additions"?)
>>
>>  CC'ing Aneesh as he's working on that.
>>
>>
>
>
>




[Qemu-devel] Re: [GSoC 2010] Pass-through filesystem support.

2010-04-08 Thread Mohammed Gamal
On Thu, Apr 8, 2010 at 6:01 PM, Mohammed Gamal  wrote:
> Hi,
> Now that Cam is almost done with his ivshmem patches, I was thinking
> of another idea for GSoC which is improving the pass-though
> filesystems.
> I've got some questions on that:
>
> 1- What does the community prefer to use and improve? CIFS, 9p, or
> both? And which is better taken up for GSoC.
>
> 2- With respect to CIFS. I wonder how the shares are supposed to be
> exposed to the guest. Should the Samba server be modified to be able
> to use unix domain sockets instead of TCP ports and then QEMU
> communicating on these sockets. With that approach, how should the
> guest be able to see the exposed share? And what is the problem of
> using Samba with TCP ports?
>
> 3- In addition, I see the idea mentions that some Windows code needs
> to be written to use network shares on a special interface. What's
> that interface? And what's the nature of that Windows code? (a driver
> a la "guest additions"?)
>
> Regards,
> Mohammed
>

P.S.: A gentle reminder. The proposal submission deadline is tomorrow,
so I'd appreciate responses as soon as possible.

Regards,
Mohammed




[Qemu-devel] [GSoC 2010] Pass-through filesystem support.

2010-04-08 Thread Mohammed Gamal
Hi,
Now that Cam is almost done with his ivshmem patches, I was thinking
of another idea for GSoC which is improving the pass-though
filesystems.
I've got some questions on that:

1- What does the community prefer to use and improve? CIFS, 9p, or
both? And which is better taken up for GSoC.

2- With respect to CIFS. I wonder how the shares are supposed to be
exposed to the guest. Should the Samba server be modified to be able
to use unix domain sockets instead of TCP ports and then QEMU
communicating on these sockets. With that approach, how should the
guest be able to see the exposed share? And what is the problem of
using Samba with TCP ports?

3- In addition, I see the idea mentions that some Windows code needs
to be written to use network shares on a special interface. What's
that interface? And what's the nature of that Windows code? (a driver
a la "guest additions"?)

Regards,
Mohammed




[Qemu-devel] [GSoC 2010][RESEND] Shared memory transport between guest(s) and host

2010-04-07 Thread Mohammed Gamal
Hi,
I am interested in the "Shared memory transport between guest(s) and
host" project for GSoC 2010. The description of the project is pretty
straightforward, but I am a little bit lost on some parts:

1- Is there any documentation available on KVM shared memory
transport. This'd definitely help understand how inter-vm shared
memory should work.

2- Does the project only aim at providing a shared memory transport
between a single host and a number of guests, with the host acting as
a central node containing shared memory objects and communication
taking placde only between guests and host, or is there any kind of
guest-guest communications to be supported? If yes, how should it be
done?




[Qemu-devel] [GSoC 2010] Shared memory transport between guest(s) and host

2010-04-06 Thread Mohammed Gamal
Hi,
I am interested in the "Shared memory transport between guest(s) and
host" project for GSoC 2010. The description of the project is pretty
straightforward, but I am a little bit lost on some parts:

1- Is there any documentation available on KVM shared memory
transport. This'd definitely help understand how inter-vm shared
memory should work.

2- Does the project only aim at providing a shared memory transport
between a single host and a number of guests, with the host acting as
a central node containing shared memory objects and communication
taking placde only between guests and host, or is there any kind of
guest-guest communications to be supported? If yes, how should it be
done?

Regards,
Mohammed




[Qemu-devel] Re: Completing big real mode emulation

2010-03-20 Thread Mohammed Gamal
On Sat, Mar 20, 2010 at 3:18 PM, Avi Kivity  wrote:
> On 03/20/2010 10:55 AM, Alexander Graf wrote:
>>>
 I'd say that a GSoC project would rather focus on making a guest OS work
 than working on generic big real mode. Having Windows 98 support is way 
 more
 visible to the users. And hopefully more fun to implement too, as it's a
 visible goal :-).


>>>
>>> Big real mode allows you to boot various OSes, such as that old
>>> Ubuntu/SuSE boot loader which triggered the whole thing.
>>>
>>
>> I thought legacy Windows uses it too?
>>
>
> IIRC even current Windows (last I checked was XP, but it's probably true for
> newer) invokes big real mode inadvertently.  All it takes is not to clear fs
> and gs while switching to real mode.  It works because the real mode code
> never uses gs and fs (i.e. while we are technically in big real mode, the
> guest never relies on this), and because there are enough hacks in vmx.c to
> make it work (restoring fs and gs after the switch back).  IIRC there are
> other cases of invalid guest state that we hack into place during mode
> switches.
>
>> Either way - then we should make the goal of the project to support those
>> old boot loaders. IMHO it should contain visibility. Doing theoretical stuff
>> is just less fun for all parties. Or does that stuff work already?
>>
>
> Mostly those old guests aged beyond usefulness.  They are still broken, but
> nobody installs new images.  Old images installed via workarounds work.
>
> Goals for this task could include:
>
>  - get those older guests working
>  - get emulate_invalid_guest_state=1 to work on all supported guests
>  - switch to emulate_invalid_guest_state=1 as the default
>  - drop the code supporting emulate_invalid_guest_state=0 eventually

To this end I guess the next logical step is to compile a list of
guests that are currently not working/work with hacks only, and get
them working. Here are some suggestions:
- MINIX 3.1.6 (developers have been recently filing bug reports
because of boot failures)
- Win XP with emulation enabled
- FreeDOS with memory extenders

Any other guests you'd like to see on this list?




[Qemu-devel] Completing big real mode emulation

2010-03-19 Thread Mohammed Gamal
Hello all,
As some of you might know, I've worked on supporting big real mode
emulation on VMX back in GSoC 2008. Looking at the Qemu GSoC ideas
list for this year, I found it among the possible ideas for a GSoC
project. I'd be interested in driving this feature towards completion,
and I have a few questions about it.

- The kernel-space modifications needed to detect an invalid guest
state on VMX and drive emulation from that point was almost complete.
The part that was missing the most, is that the kvm x86 emulator
wasn't complete and didn't support the entire instruction set. I've
seen that the emulator has been the focus of some recent patches
(namely by Gleb Natapov). Is there anything else required to get big
real mode to work correctly on KVM?

- Do we have other problems supporting big real mode on non-VMX
instruction sets? And do we have problems supporting it on the
userspace side?

- Is there anything I am missing?

Regards,
Mohammed