Re: [Qemu-devel] [PATCH RFC 1/8] virtio: add subsections to the migration stream

2014-05-14 Thread Amit Shah
On (Thu) 15 May 2014 [08:49:48], Greg Kurz wrote:
> On Thu, 15 May 2014 11:34:25 +0530
> Amit Shah  wrote:
> 
> > On (Wed) 14 May 2014 [17:41:38], Greg Kurz wrote:
> > > There is a need to add some more fields to VirtIODevice that should be
> > > migrated (broken status, endianness). The problem is that we do not
> > > want to break compatibility while adding a new feature... This issue has
> > > been addressed in the generic VMState code with the use of optional
> > > subsections. As a *temporary* alternative to port the whole virtio
> > > migration code to VMState, this patch mimics a similar subsectionning
> > > ability for virtio.
> > > 
> > > Since each virtio device is streamed in its own section, the idea is to
> > > stream subsections between the end of the device section and the start
> > > of the next sections. This allows an older QEMU to complain and exit
> > > when fed with subsections:
> > > 
> > > Unknown savevm section type 5
> > > Error -22 while loading VM state
> > 
> > Please make this configurable -- either via configure or device
> > properties.  That avoids having to break existing configurations that
> > work without this patch.
> > 
> 
> Hmmm... you mean we support migration from a newer QEMU to an older one ?

Well, not really "support", since many things don't work anyway, but
let's make that easier on downstreams that may want to.

> > > All users of virtio_load()/virtio_save() need to be patched because the
> > > subsections are streamed AFTER the device itself.
> > 
> > Since all have the same fixup, I'm wondering if a new section can be
> > added to the virtio-bus itself, which gets propagated to all devices
> > upon load in the dest.
> > 
> 
> That would be nice if possible. I will have a closer look.

Thanks!

Amit



Re: [Qemu-devel] [PATCH v3] serial-pci: Set prog interface field of pci config to 16550 compatible

2014-05-14 Thread Gerd Hoffmann
> > >  static Property serial_pci_properties[] = {
> > >  DEFINE_PROP_CHR("chardev",  PCISerialState, state.chr),
> > > +DEFINE_PROP_UINT8("compat",  PCISerialState, compat, 0),
> > >  DEFINE_PROP_END_OF_LIST(),
> > >  };
> > 
> > mst, do you take that through the pci tree?
> > 
> > cheers,
> >   Gerd
> > 
> 
> Yes but I'd like the property renamed. Agree?

Yes, something more descriptive is reasonable.

cheers,
  Gerd





Re: [Qemu-devel] [PATCH] iotests: Use configured python

2014-05-14 Thread Markus Armbruster
Max Reitz  writes:

> On 14.05.2014 14:33, Markus Armbruster wrote:
>> Max Reitz  writes:
>>
>>> Currently, QEMU's iotests rely on /usr/bin/env to start the correct
>>> Python (that is, at least Python 2.4, but not 3). On systems where
>>> Python 3 is the default, the user has no clean way of making the iotests
>>> use the correct binary.
>>>
>>> This commit makes the iotests use the Python selected by configure.
>>>
>>> Signed-off-by: Max Reitz 
>> I'm afraid this broke qemu-iotests in a separate build tree:
>>
>>  ./check: line 38: ./common.env: No such file or directory
>
> As I already replied to Peter, I see two (or maybe three) ways to fix this:
>
> The first is, we use the correct path to common.env. This would
> however result in modification of the source tree although this is
> probably not what the user intends with an out-of-tree build. On the
> other hand, this would just work.

Writing to the source tree works only when the write is exactly the same
for every build tree.  Example: autoconf writing configure.

Modifying files under git-control is right out.

> The second is, we do not create common.env for out-of-tree builds and
> add a default common.env to the repository ("PYTHON = python" should
> probably suffice). This would not introduce any regressions, however,
> the iotests would remain problematic on systems with Python 3 being
> the default when using out-of-tree builds.

A difference between an in-tree and an out-of-tree build is a bug.

If plain "python" is good enough for out-of-tree builds, it should be
good enough for in-tree builds.

>As I guess that out-of-tree
> builds are actually recommended,

Correct.

>  this would mean that the better
> solution might be to revert my patch and instead change every "python"
> occurrence in the iotests' Shebangs to "python2", as kind of a third
> way to go. However, for this I'm not sure whether all systems which
> are supposed to be supported by qemu actually have a "python2"
> executable/symlink. I guess so, but then again...

I don't know.  Try and find out :)

> So, either we fix it but try to write to the source tree without the
> user intending to modify it; or we fix it for in-tree builds only; or
> we drop the magic and just use "python2" in the iotests' Shebangs.

The problem is including generated bits, namely results of configure,
into source files.

The Autoconf way is to substitute placeholders in FOO.in producing FOO.

When you want to limit .in contents as much as possible, you factor out
the stuff that needs substitutions into some SUB.in, which you then
include into FOO.  Requires a suitable include mechanism.  In shell,
builtin source.

But then you need to find SUB from FOO.  I've seen two ways used:

1. Assume SUB is in the current directory.  Link FOO into the build tree
to make it so.

2. Require FOO to be run in a way that lets it find its source
directory.  If whatever runs FOO puts the full path into argv[0], you
can use that.  Else, require a suitable argument or environment
variable.



Re: [Qemu-devel] usb: usb tablet freeze when save/restore guest os

2014-05-14 Thread Gerd Hoffmann
  Hi,

> Well then, may I post a formal patch for this issue, Gerd? Thanks.

I'd like to know what the root cause for the lost interrupt is.

Not implementing PIRQ enable could be it, especially as the guest os
seems to use it (otherwise your patch would have no effect).

The check for the PIRQ enable bit should be in uhci_update_irq though,
and you should check the single bit only, not the whole legacy support
register.

cheers,
  Gerd






Re: [Qemu-devel] [PATCH RFC 1/8] virtio: add subsections to the migration stream

2014-05-14 Thread Greg Kurz
On Thu, 15 May 2014 11:34:25 +0530
Amit Shah  wrote:

> On (Wed) 14 May 2014 [17:41:38], Greg Kurz wrote:
> > There is a need to add some more fields to VirtIODevice that should be
> > migrated (broken status, endianness). The problem is that we do not
> > want to break compatibility while adding a new feature... This issue has
> > been addressed in the generic VMState code with the use of optional
> > subsections. As a *temporary* alternative to port the whole virtio
> > migration code to VMState, this patch mimics a similar subsectionning
> > ability for virtio.
> > 
> > Since each virtio device is streamed in its own section, the idea is to
> > stream subsections between the end of the device section and the start
> > of the next sections. This allows an older QEMU to complain and exit
> > when fed with subsections:
> > 
> > Unknown savevm section type 5
> > Error -22 while loading VM state
> 
> Please make this configurable -- either via configure or device
> properties.  That avoids having to break existing configurations that
> work without this patch.
> 

Hmmm... you mean we support migration from a newer QEMU to an older one ?

> > All users of virtio_load()/virtio_save() need to be patched because the
> > subsections are streamed AFTER the device itself.
> 
> Since all have the same fixup, I'm wondering if a new section can be
> added to the virtio-bus itself, which gets propagated to all devices
> upon load in the dest.
> 

That would be nice if possible. I will have a closer look.

>   Amit
> 

Thanks.

-- 
Gregory Kurz kurzg...@fr.ibm.com
 gk...@linux.vnet.ibm.com
Software Engineer @ IBM/Meiosys  http://www.ibm.com
Tel +33 (0)562 165 496

"Anarchy is about taking complete responsibility for yourself."
Alan Moore.




Re: [Qemu-devel] [PATCH] rdma: Fix block during rdma migration

2014-05-14 Thread Gonglei (Arei)
> -Original Message-
> From: Michael R. Hines [mailto:mrhi...@linux.vnet.ibm.com]
> Sent: Thursday, May 15, 2014 8:44 AM
> To: Gonglei (Arei); qemu-devel@nongnu.org
> Cc: Huangweidong (C); quint...@redhat.com; dgilb...@redhat.com;
> owass...@redhat.com; mrhi...@us.ibm.com; pbonz...@redhat.com;
> Moyuxiang
> Subject: Re: [Qemu-devel] [PATCH] rdma: Fix block during rdma migration
> 
> On 05/09/2014 12:25 PM, Gonglei (Arei) wrote:
> > Hi,
> >
> >> -Original Message-
> >> From: Michael R. Hines [mailto:mrhi...@linux.vnet.ibm.com]
> >> Sent: Tuesday, April 01, 2014 8:42 AM
> >> To: Gonglei (Arei); qemu-devel@nongnu.org
> >> Cc: Huangweidong (C); quint...@redhat.com; dgilb...@redhat.com;
> >> owass...@redhat.com; mrhi...@us.ibm.com; Moyuxiang;
> >> pbonz...@redhat.com
> >> Subject: Re: [Qemu-devel] [PATCH] rdma: Fix block during rdma migration
> >>
> >> On 03/29/2014 03:39 PM, arei.gong...@huawei.com wrote:
> >>> From: Mo Yuxiang 
> >>>
> >>> If the networking break or there's something wrong with rdma
> >>> device(ib0 with no IP) during rdma migration, the main_loop of
> >>> qemu will be blocked in rdma_destroy_id. I add rdma_ack_cm_event
> >>> to fix this bug.
> >>>
> >>> Signed-off-by: Mo Yuxiang 
> >>> Signed-off-by: Gonglei 
> >>> ---
> >>>migration-rdma.c | 1 +
> >>>1 file changed, 1 insertion(+)
> >>>
> >>> diff --git a/migration-rdma.c b/migration-rdma.c
> >>> index eeb4302..f60749b 100644
> >>> --- a/migration-rdma.c
> >>> +++ b/migration-rdma.c
> >>> @@ -949,6 +949,7 @@ route:
> >>>ERROR(errp, "result not equal to event_addr_resolved %s",
> >>>rdma_event_str(cm_event->event));
> >>>perror("rdma_resolve_addr");
> >>> +rdma_ack_cm_event(cm_event);
> >>>ret = -EINVAL;
> >>>goto err_resolve_get_addr;
> >>>}
> >> Reviewed-by: Michael R. Hines 
> >>
> >> Good catch. =) That's an obvious bug. It looks like I need
> >> to do a much better job of "kill -9" inside the regression
> >> testing scripts - probably i should try killing the migration
> >> prematurely at different periods just to be sure there are
> >> no more places where the connection state is not getting
> >> cleaned up..
> >>
> >> - Michael
> >>
> > Michael, do you have a plan to pull this patch to master? Thanks.
> >
> > Best regards,
> > -Gonglei
> >
> 
> Sorry for the late reply, but I'm not the maintainer for migration,
> that's Juan
> (I can only signoff on patches like everyone else =).
> 
> I also have outstanding RDMA patches myself that have not yet been pulled.
> 
> Would you mind pinging Juan for both of us?
> 
Thanks.
The patch is Cc'ing Juan, maybe he is very busy. 
I have post v2 even, but I have not gotten any reply. 
I have no idea how to do next.


Best regards,
-Gonglei




Re: [Qemu-devel] [PATCH RFC 1/8] virtio: add subsections to the migration stream

2014-05-14 Thread Amit Shah
On (Thu) 15 May 2014 [09:23:51], Michael S. Tsirkin wrote:
> On Thu, May 15, 2014 at 11:34:25AM +0530, Amit Shah wrote:
> > On (Wed) 14 May 2014 [17:41:38], Greg Kurz wrote:
> > > There is a need to add some more fields to VirtIODevice that should be
> > > migrated (broken status, endianness). The problem is that we do not
> > > want to break compatibility while adding a new feature... This issue has
> > > been addressed in the generic VMState code with the use of optional
> > > subsections. As a *temporary* alternative to port the whole virtio
> > > migration code to VMState, this patch mimics a similar subsectionning
> > > ability for virtio.

BTW Greg, do you plan on working on vmstate for virtio?

> > > Since each virtio device is streamed in its own section, the idea is to
> > > stream subsections between the end of the device section and the start
> > > of the next sections. This allows an older QEMU to complain and exit
> > > when fed with subsections:
> > > 
> > > Unknown savevm section type 5
> > > Error -22 while loading VM state
> > 
> > Please make this configurable -- either via configure or device
> > properties.  That avoids having to break existing configurations that
> > work without this patch.
> > 
> > > All users of virtio_load()/virtio_save() need to be patched because the
> > > subsections are streamed AFTER the device itself.
> > 
> > Since all have the same fixup, I'm wondering if a new section can be
> > added to the virtio-bus itself, which gets propagated to all devices
> > upon load in the dest.
> 
> This calls for a way for devices to inherit properties from the bus,
> which doesn't exist ATM.
> Fine but let's not hold up this patchset because of this.

No, only suggestion is to add a migration section in the bus, and then
it's easier to do this in the post-migrate functions for each device
-- so only one new section gets introduced instead of all devices
being modified to send a new subsection.

Amit



Re: [Qemu-devel] [PATCH v3] serial-pci: Set prog interface field of pci config to 16550 compatible

2014-05-14 Thread Michael S. Tsirkin
On Thu, May 15, 2014 at 08:42:17AM +0200, Gerd Hoffmann wrote:
> On Do, 2014-02-27 at 02:05 +0100, BALATON Zoltan wrote:
>^^
> Looks like your clock is _way_ off.
> 
> > +if (!pci->compat) {
> > +pci->dev.config[PCI_CLASS_PROG] = 0x02; /* 16550 compatible */
> > +}
> 
> >  static Property serial_pci_properties[] = {
> >  DEFINE_PROP_CHR("chardev",  PCISerialState, state.chr),
> > +DEFINE_PROP_UINT8("compat",  PCISerialState, compat, 0),
> >  DEFINE_PROP_END_OF_LIST(),
> >  };
> 
> > +{\
> > +.driver   = "pci-serial",\
> > +.property = "compat",\
> > +.value= stringify(1),\
> > +},\
> 
> Reviewed-by: Gerd Hoffmann 
> 
> mst, do you take that through the pci tree?
> 
> cheers,
>   Gerd
> 

Yes but I'd like the property renamed. Agree?




Re: [Qemu-devel] [PATCH v3] serial-pci: Set prog interface field of pci config to 16550 compatible

2014-05-14 Thread Gerd Hoffmann
On Do, 2014-02-27 at 02:05 +0100, BALATON Zoltan wrote:
   ^^
Looks like your clock is _way_ off.

> +if (!pci->compat) {
> +pci->dev.config[PCI_CLASS_PROG] = 0x02; /* 16550 compatible */
> +}

>  static Property serial_pci_properties[] = {
>  DEFINE_PROP_CHR("chardev",  PCISerialState, state.chr),
> +DEFINE_PROP_UINT8("compat",  PCISerialState, compat, 0),
>  DEFINE_PROP_END_OF_LIST(),
>  };

> +{\
> +.driver   = "pci-serial",\
> +.property = "compat",\
> +.value= stringify(1),\
> +},\

Reviewed-by: Gerd Hoffmann 

mst, do you take that through the pci tree?

cheers,
  Gerd





Re: [Qemu-devel] [PATCH v3] serial-pci: Set prog interface field of pci config to 16550 compatible

2014-05-14 Thread Michael S. Tsirkin
On Thu, Feb 27, 2014 at 02:05:05AM +0100, BALATON Zoltan wrote:
> Signed-off-by: BALATON Zoltan 
> ---


This looks sane, a minor comment below (hopefully last).
Thanks!

>  v2: resubmission after pc-2.1 is added with the multiport case
>  v3: added compatibility check to avoid changing earlier than pc-2.1
> 
>  hw/char/serial-pci.c | 11 +++
>  include/hw/i386/pc.h | 15 +++
>  2 files changed, 26 insertions(+)
> 
> diff --git a/hw/char/serial-pci.c b/hw/char/serial-pci.c
> index 991c99f..ae57098 100644
> --- a/hw/char/serial-pci.c
> +++ b/hw/char/serial-pci.c
> @@ -34,6 +34,7 @@
>  typedef struct PCISerialState {
>  PCIDevice dev;
>  SerialState state;
> +uint8_t compat;
>  } PCISerialState;
>  
>  typedef struct PCIMultiSerialState {
> @@ -44,6 +45,7 @@ typedef struct PCIMultiSerialState {
>  SerialState  state[PCI_SERIAL_MAX_PORTS];
>  uint32_t level[PCI_SERIAL_MAX_PORTS];
>  qemu_irq *irqs;
> +uint8_t  compat;
>  } PCIMultiSerialState;
>  
>  static int serial_pci_init(PCIDevice *dev)

This name isn't very informative.
I think this should be a prog_if property
defaulting to 0x2 and over-written for old machine types,
Devices don't care why their prog_if is changed,
it's the machine type that cares about compatibility.

See
commit aa93200b88fb1071eaf21bf766711762ed4630e2
Author: Gabriel L. Somlo 
Date:   Mon May 5 10:52:51 2014 -0400

apic: use emulated lapic version 0x14 on pc machines >= 2.1
as an example.


Alternatively, create a bit property legacy_prog_if
and change field name to compat_flags, have property
set flags here.

See
commit 2af234e61d59f39ae16ba882271e7c4fef2c41c1
Author: Michael S. Tsirkin 
Date:   Thu Feb 14 19:11:27 2013 +0200

e1000: unbreak the guest network migration to 1.3
for an example.

I think the 1st option is better but second one would be
more or less ok.

> @@ -60,6 +62,9 @@ static int serial_pci_init(PCIDevice *dev)
>  return -1;
>  }
>  
> +if (!pci->compat) {
> +pci->dev.config[PCI_CLASS_PROG] = 0x02; /* 16550 compatible */
> +}
>  pci->dev.config[PCI_INTERRUPT_PIN] = 0x01;
>  s->irq = pci_allocate_irq(&pci->dev);
>  
> @@ -101,6 +106,9 @@ static int multi_serial_pci_init(PCIDevice *dev)
>  assert(pci->ports > 0);
>  assert(pci->ports <= PCI_SERIAL_MAX_PORTS);
>  
> +if (!pci->compat) {
> +pci->dev.config[PCI_CLASS_PROG] = 0x02; /* 16550 compatible */
> +}
>  pci->dev.config[PCI_INTERRUPT_PIN] = 0x01;
>  memory_region_init(&pci->iobar, OBJECT(pci), "multiserial", 8 * 
> pci->ports);
>  pci_register_bar(&pci->dev, 0, PCI_BASE_ADDRESS_SPACE_IO, &pci->iobar);
> @@ -177,12 +185,14 @@ static const VMStateDescription 
> vmstate_pci_multi_serial = {
>  
>  static Property serial_pci_properties[] = {
>  DEFINE_PROP_CHR("chardev",  PCISerialState, state.chr),
> +DEFINE_PROP_UINT8("compat",  PCISerialState, compat, 0),
>  DEFINE_PROP_END_OF_LIST(),
>  };
>  
>  static Property multi_2x_serial_pci_properties[] = {
>  DEFINE_PROP_CHR("chardev1",  PCIMultiSerialState, state[0].chr),
>  DEFINE_PROP_CHR("chardev2",  PCIMultiSerialState, state[1].chr),
> +DEFINE_PROP_UINT8("compat",  PCIMultiSerialState, compat, 0),
>  DEFINE_PROP_END_OF_LIST(),
>  };
>  
> @@ -191,6 +201,7 @@ static Property multi_4x_serial_pci_properties[] = {
>  DEFINE_PROP_CHR("chardev2",  PCIMultiSerialState, state[1].chr),
>  DEFINE_PROP_CHR("chardev3",  PCIMultiSerialState, state[2].chr),
>  DEFINE_PROP_CHR("chardev4",  PCIMultiSerialState, state[3].chr),
> +DEFINE_PROP_UINT8("compat",  PCIMultiSerialState, compat, 0),
>  DEFINE_PROP_END_OF_LIST(),
>  };
>  
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 32a7687..8fb8046 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -271,6 +271,21 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t 
> *);
>  .driver   = "apic",\
>  .property = "version",\
>  .value= stringify(0x11),\
> +},\
> +{\
> +.driver   = "pci-serial",\
> +.property = "compat",\
> +.value= stringify(1),\
> +},\
> +{\
> +.driver   = "pci-serial-2x",\
> +.property = "compat",\
> +.value= stringify(1),\
> +},\
> +{\
> +.driver   = "pci-serial-4x",\
> +.property = "compat",\
> +.value= stringify(1),\
>  }
>  
>  #define PC_COMPAT_1_7 \
> -- 
> 1.8.1.5



Re: [Qemu-devel] [PATCH] SMI handler should set the CPL to zero and save and restore it on rsm.

2014-05-14 Thread Paolo Bonzini

Il 15/05/2014 03:32, Kevin O'Connor ha scritto:

On Wed, May 14, 2014 at 08:20:59PM -0400, Kevin O'Connor wrote:

On Wed, May 14, 2014 at 10:05:47AM +0200, Paolo Bonzini wrote:

CPL isn't even altered when CS is reloaded, because you cannot jump out
of ring-0 except with an inter-privilege IRET, and that reloads SS too.

An IRET or task switch is also the only way to set EFLAGS.VM, and it will
hardcode SS.DPL=3, again matching CPL=3.

Finally, to get out of real mode you need to have CPL=0, and whatever got
you at CPL has also loaded SS with a ring-0 stack.  This means that SS.DPL=0
right after clearing CR0.PE.

Using SS.DPL as the CPL really sounds like the right approach.  I
tried it on my KVM testcase, and it works well.  For QEMU, even the
special case of SYSRET will be handled fine because QEMU does set
SS.DPL = 3:

cpu_x86_load_seg_cache(env, R_SS, selector + 8,
   0, 0x,
   DESC_G_MASK | DESC_B_MASK | DESC_P_MASK |
   DESC_S_MASK | (3 << DESC_DPL_SHIFT) |
   DESC_W_MASK | DESC_A_MASK);

SS.DPL=CPL=3, SS.RPL=selector & 3 is a mix of Intel behavior (which is
SS.DPL=SS.RPL=CPL=3) and AMD behavior (because they set CPL=3 but
SS.DPL=SS.RPL=selector & 3).  We may want to match Intel behavior,
but that's a different change.

Can you check if this patch works for you, and if so reply with
Tested-by/Reviewed-by?


Your patch causes Freedos to crash when emm386 is loaded, so I think
it broke VM86 mode.  Below are some logs I took from qemu at the point
of the crash.


FYI, with the patch below my quick test cases all look okay.

--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -974,7 +974,6 @@ static inline void cpu_x86_load_seg_cache(CPUX86State *env,
 /* update the hidden flags */
 {
 if (seg_reg == R_CS) {
-int cpl = selector & 3;
 #ifdef TARGET_X86_64
 if ((env->hflags & HF_LMA_MASK) && (flags & DESC_L_MASK)) {
 /* long mode */
@@ -984,16 +983,17 @@ static inline void cpu_x86_load_seg_cache(CPUX86State 
*env,
 #endif
 {
 /* legacy / compatibility case */
-if (!(env->cr[0] & CR0_PE_MASK)) {
-cpl = 0;
-} else if (env->eflags & VM_MASK) {
-cpl = 3;
-}
 new_hflags = (env->segs[R_CS].flags & DESC_B_MASK)
 >> (DESC_B_SHIFT - HF_CS32_SHIFT);
 env->hflags = (env->hflags & ~(HF_CS32_MASK | HF_CS64_MASK)) |
 new_hflags;
 }
+}
+if (seg_reg == R_SS) {
+int cpl = (flags >> DESC_DPL_SHIFT) & 3;
+if (env->eflags & VM_MASK) {
+cpl = 3;
+}
 #if HF_CPL_MASK != 3
 #error HF_CPL_MASK is hardcoded
 #endif



Looks like a bug entering VM86 mode.  I'll take a look, thanks for 
confirming what works and what doesn't!


Paolo



[Qemu-devel] pidfile option in config file

2014-05-14 Thread William Dauchy
Hello,

I'm using the `-pidfile` option in my qemu command line. Since I'm
also using the `-readconfig` option I thought it was a good thing to
include the pidfile option in my config file. Unfortunately I did not
found any possibility to add such option in my config file.
Is it something I could expect?

Regards,
-- 
William



Re: [Qemu-devel] [PATCH RFC 1/8] virtio: add subsections to the migration stream

2014-05-14 Thread Michael S. Tsirkin
On Thu, May 15, 2014 at 11:34:25AM +0530, Amit Shah wrote:
> On (Wed) 14 May 2014 [17:41:38], Greg Kurz wrote:
> > There is a need to add some more fields to VirtIODevice that should be
> > migrated (broken status, endianness). The problem is that we do not
> > want to break compatibility while adding a new feature... This issue has
> > been addressed in the generic VMState code with the use of optional
> > subsections. As a *temporary* alternative to port the whole virtio
> > migration code to VMState, this patch mimics a similar subsectionning
> > ability for virtio.
> > 
> > Since each virtio device is streamed in its own section, the idea is to
> > stream subsections between the end of the device section and the start
> > of the next sections. This allows an older QEMU to complain and exit
> > when fed with subsections:
> > 
> > Unknown savevm section type 5
> > Error -22 while loading VM state
> 
> Please make this configurable -- either via configure or device
> properties.  That avoids having to break existing configurations that
> work without this patch.
> 
> > All users of virtio_load()/virtio_save() need to be patched because the
> > subsections are streamed AFTER the device itself.
> 
> Since all have the same fixup, I'm wondering if a new section can be
> added to the virtio-bus itself, which gets propagated to all devices
> upon load in the dest.
> 
>   Amit

This calls for a way for devices to inherit properties from the bus,
which doesn't exist ATM.
Fine but let's not hold up this patchset because of this.

-- 
MST



Re: [Qemu-devel] [PATCH RFC 1/8] virtio: add subsections to the migration stream

2014-05-14 Thread Amit Shah
On (Wed) 14 May 2014 [17:41:38], Greg Kurz wrote:
> There is a need to add some more fields to VirtIODevice that should be
> migrated (broken status, endianness). The problem is that we do not
> want to break compatibility while adding a new feature... This issue has
> been addressed in the generic VMState code with the use of optional
> subsections. As a *temporary* alternative to port the whole virtio
> migration code to VMState, this patch mimics a similar subsectionning
> ability for virtio.
> 
> Since each virtio device is streamed in its own section, the idea is to
> stream subsections between the end of the device section and the start
> of the next sections. This allows an older QEMU to complain and exit
> when fed with subsections:
> 
> Unknown savevm section type 5
> Error -22 while loading VM state

Please make this configurable -- either via configure or device
properties.  That avoids having to break existing configurations that
work without this patch.

> All users of virtio_load()/virtio_save() need to be patched because the
> subsections are streamed AFTER the device itself.

Since all have the same fixup, I'm wondering if a new section can be
added to the virtio-bus itself, which gets propagated to all devices
upon load in the dest.

Amit



[Qemu-devel] Qemu stucking

2014-05-14 Thread sonia verma
Hi

I'm getting below error when trying to boot the KVM with ethernet
bridging,kvm support and universel TUN enabled by the following command..

/usr/bin/qemu-system-ppc64 -m 512 -nographic -hda
/var/volatile/debian_lenny_
powerpc_standard.qcow2


cannot manage 'OHCI USB controller' PCI device type 'usb':
>>  106b 3f (c 3 10)

>> =
>> OpenBIOS 1.0 [Aug 28 2012 05:40]
>> Configuration device id QEMU version 1 machine id 3
>> CPUs: 1
>> Memory: 512M
>> UUID: ----
>> CPU type PowerPC,970FX
Welcome to OpenBIOS v1.0 built on Aug 28 2012 05:40
Second-stage QUIK loader
Welcome to quik. mate is good.
Debian GNU/Linux PowerPCchosen/bootargs =
boot: `
Enter the kernel image name as [device:][partno]/path, where partno is a
number from 0 to 16.  Instead of /path you can type [mm-nn] to specify a
range of disk blocks (512B)
boot: Linux
initrd imagename = /initrd.img, mem_size: 4406840
initrd_start:
Starting at 51, , 1024
OF stdout device is: /pci@f000/mac-io@e/escc@13000/ch-b@13000
command line: console=ttyS0,9600 console=tty0
memory layout at init:
  alloc_bottom : 00c01000
  alloc_top: 2000
  alloc_top_hi : 2000
  rmo_top  : 2000
  ram_top  : 2000
Looking for displays
found display   : /pci@f000/QEMU,VGA@c, opening ... done
copying OF device tree ...
Building dt strings...
Building dt structure...
Device tree strings 0x00c02000 -> 0x00c024f0
Device tree struct  0x00c03000 -> 0x00c05000
Calling quiesce ...
returning from prom_init

The prompt is stuck at init and is not able to proceed even after wating
for 15-20 minutes.

Please help regarding this.

Thanks
Sonia


Re: [Qemu-devel] [PATCHv4] block: optimize zero writes with bdrv_write_zeroes

2014-05-14 Thread Peter Lieven
Am 14.05.2014 13:41, schrieb Kevin Wolf:
> Am 08.05.2014 um 18:22 hat Peter Lieven geschrieben:
>> this patch tries to optimize zero write requests
>> by automatically using bdrv_write_zeroes if it is
>> supported by the format.
>>
>> This significantly speeds up file system initialization and
>> should speed zero write test used to test backend storage
>> performance.
>>
>> I ran the following 2 tests on my internal SSD with a
>> 50G QCOW2 container and on an attached iSCSI storage.
>>
>> a) mkfs.ext4 -E lazy_itable_init=0,lazy_journal_init=0 /dev/vdX
>>
>> QCOW2 [off] [on] [unmap]
>> -
>> runtime:   14secs1.1secs  1.1secs
>> filesize:  937M  18M  18M
>>
>> iSCSI [off] [on] [unmap]
>> 
>> runtime:   9.3s  0.9s 0.9s
>>
>> b) dd if=/dev/zero of=/dev/vdX bs=1M oflag=direct
>>
>> QCOW2 [off] [on] [unmap]
>> -
>> runtime:   246secs   18secs   18secs
>> filesize:  51G   192K 192K
>> throughput:203M/s2.3G/s   2.3G/s
>>
>> iSCSI*[off] [on] [unmap]
>> 
>> runtime:   8mins 45secs   33secs
>> throughput:106M/s1.2G/s   1.6G/s
>> allocated: 100%  100% 0%
>>
>> * The storage was connected via an 1Gbit interface.
>>   It seems to internally handle writing zeroes
>>   via WRITESAME16 very fast.
>>
>> Signed-off-by: Peter Lieven 
>> ---
>> v3->v4: - use QAPI generated enum and lookup table [Kevin]
>> - added more details about the options in the comments
>>   of the qapi-schema [Eric]
>> - changed the type of detect_zeroes from str to
>>   BlockdevDetectZeroesOptions. I left the name
>>   as is because it is consistent with e.g.
>>   BlockdevDiscardOptions or BlockdevAioOptions [Eric]
>> - changed the parse function in blockdev_init to
>>   be generic usable for other enum parameters
>> 
>> v2->v3: - moved parameter parsing to blockdev_init
>> - added per device detect_zeroes status to 
>>   hmp (info block -v) and qmp (query-block) [Eric]
>> - added support to enable detect-zeroes also
>>   for hot added devices [Eric]
>> - added missing entry to qemu_common_drive_opts
>> - fixed description of qemu_iovec_is_zero [Fam]
>>
>> v1->v2: - added tests to commit message (Markus)
>> RFCv2->v1: - fixed paramter parsing strncmp -> strcmp (Eric)
>>- fixed typo (choosen->chosen) (Eric)
>>- added second example to commit msg
>>
>> RFCv1->RFCv2: - add detect-zeroes=off|on|unmap knob to drive cmdline 
>> parameter
>>   - call zero detection only for format (bs->file != NULL)
>>
>>  block.c   |   11 ++
>>  block/qapi.c  |1 +
>>  blockdev.c|   34 +
>>  hmp.c |5 +
>>  include/block/block_int.h |1 +
>>  include/qemu-common.h |1 +
>>  qapi-schema.json  |   52 
>> -
>>  qemu-options.hx   |6 ++
>>  qmp-commands.hx   |3 +++
>>  util/iov.c|   21 ++
>>  10 files changed, 120 insertions(+), 15 deletions(-)
>>
>> diff --git a/block.c b/block.c
>> index b749d31..aea4c77 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -3244,6 +3244,17 @@ static int coroutine_fn 
>> bdrv_aligned_pwritev(BlockDriverState *bs,
>>  
>>  ret = notifier_with_return_list_notify(&bs->before_write_notifiers, 
>> req);
>>  
>> +if (!ret && bs->detect_zeroes != BLOCKDEV_DETECT_ZEROES_OPTIONS_OFF &&
>> +!(flags & BDRV_REQ_ZERO_WRITE) && bs->file &&
>> +drv->bdrv_co_write_zeroes && qemu_iovec_is_zero(qiov)) {
> Pretty long condition. :-)
>
> Looks like most is obviously needed, but I wonder what the bs->file part
> is good for? That looks rather arbitrary.

What I wanted to achieve is that this condition is only true if we handle
the format (e.g. raw, qcow2, vmdk etc.). If e.g. qcow2 then sends a
zero write this should always end on the disk and should not be optimizable.


>
>> +flags |= BDRV_REQ_ZERO_WRITE;
>> +/* if the device was not opened with discard=on the below flag
>> + * is immediately cleared again in bdrv_co_do_write_zeroes */
> Is it? I only see it being cleared in bdrv_co_write_zeroes(), but that's
> not a function that seems to be called from here.

You are right. Question, do we want to support detect_zeroes = unmap
if discard = ignore? If yes, I have to update the docs. Otherwise
I have to check for BDRV_O_DISCARD before setting BDRV_REQ_MAY_UNMAP.

>
>> +if (bs->detect_zeroes == BLOCKDEV_DETECT_ZEROES_OPTIONS_UNMAP) {
>> +flags |= BDRV_REQ_MAY_UNMAP;
>> +}
>> +}
>> +
>>  if (ret < 0) {
>>  /* Do nothing, write notifier decided to fail this request */
>>  } else if (flags & BDRV_REQ_ZERO_WRITE) {
>> diff --git a/block/qap

Re: [Qemu-devel] [PATCH v3] vmdk: Optimize cluster allocation

2014-05-14 Thread Fam Zheng
On Wed, 05/14 16:23, Kevin Wolf wrote:
> Am 08.05.2014 um 07:57 hat Fam Zheng geschrieben:
> > This drops the unnecessary bdrv_truncate() from, and also improves,
> > cluster allocation code path.
> > [...]
> > 
> > Tested that this passes qemu-iotests for all VMDK subformats.
> > 
> > Signed-off-by: Fam Zheng 
> 
> Unfortunately, this is seriously broken and only compatible with itself,
> but appears not to work any more with real VMDKs.
> 
> > ---
> > V3: A new implementation following Kevin's suggestion.
> > 
> > Signed-off-by: Fam Zheng 
> > ---
> >  block/vmdk.c | 184 
> > +++
> >  1 file changed, 121 insertions(+), 63 deletions(-)
> > 
> > diff --git a/block/vmdk.c b/block/vmdk.c
> > index 06a1f9f..8c34d5e 100644
> > --- a/block/vmdk.c
> > +++ b/block/vmdk.c
> > @@ -106,6 +106,7 @@ typedef struct VmdkExtent {
> >  uint32_t l2_cache_counts[L2_CACHE_SIZE];
> >  
> >  int64_t cluster_sectors;
> > +int64_t next_cluster_offset;
> >  char *type;
> >  } VmdkExtent;
> >  
> > @@ -397,6 +398,7 @@ static int vmdk_add_extent(BlockDriverState *bs,
> >  {
> >  VmdkExtent *extent;
> >  BDRVVmdkState *s = bs->opaque;
> > +int64_t ret;
> >  
> >  if (cluster_sectors > 0x20) {
> >  /* 0x20 * 512Bytes = 1GB for one cluster is unrealistic */
> > @@ -428,6 +430,13 @@ static int vmdk_add_extent(BlockDriverState *bs,
> >  extent->l2_size = l2_size;
> >  extent->cluster_sectors = flat ? sectors : cluster_sectors;
> >  
> > +ret = bdrv_getlength(extent->file);
> > +
> 
> Why this empty line?

Removing.

> 
> > +if (ret < 0) {
> > +return ret;
> > +}
> > +extent->next_cluster_offset = ROUND_UP(ret, BDRV_SECTOR_SIZE);
> > +
> >  if (s->num_extents > 1) {
> >  extent->end_sector = (*(extent - 1)).end_sector + extent->sectors;
> >  } else {
> > @@ -954,42 +963,77 @@ static int vmdk_refresh_limits(BlockDriverState *bs)
> >  return 0;
> >  }
> >  
> > +/**
> > + * get_whole_cluster
> > + *
> > + * Copy backing file's cluster that covers @sector_num, otherwise write 
> > zero,
> > + * to the cluster at @cluster_sector_num.
> > + *
> > + * If @skip_start < @skip_end, the relative range [@skip_start, @skip_end) 
> > is
> > + * not copied or written, and leave it for call to write user data in the
> > + * request.
> > + */
> >  static int get_whole_cluster(BlockDriverState *bs,
> > -VmdkExtent *extent,
> > -uint64_t cluster_offset,
> > -uint64_t offset,
> > -bool allocate)
> > + VmdkExtent *extent,
> > + uint64_t cluster_sector_num,
> > + uint64_t sector_num,
> > + uint64_t skip_start, uint64_t skip_end)
> >  {
> >  int ret = VMDK_OK;
> > -uint8_t *whole_grain = NULL;
> > +int64_t cluster_bytes;
> > +uint8_t *whole_grain;
> > +
> > +/* For COW, align request sector_num to cluster start */
> > +sector_num -= sector_num % extent->cluster_sectors;
> 
> QEMU_ALIGN_DOWN?

Yes.

> 
> > +cluster_bytes = extent->cluster_sectors << BDRV_SECTOR_BITS;
> > +whole_grain = qemu_blockalign(bs, cluster_bytes);
> > +memset(whole_grain, 0, cluster_bytes);
> 
> This is completely unnecessary for cases with backing files, and
> unnecessary for the skipped part in any case.

Yes.

> 
> > +assert(skip_end <= sector_num + extent->cluster_sectors);
> >  /* we will be here if it's first write on non-exist grain(cluster).
> >   * try to read from parent image, if exist */
> > -if (bs->backing_hd) {
> > -whole_grain =
> > -qemu_blockalign(bs, extent->cluster_sectors << 
> > BDRV_SECTOR_BITS);
> > -if (!vmdk_is_cid_valid(bs)) {
> > -ret = VMDK_ERROR;
> > -goto exit;
> > -}
> > +if (bs->backing_hd && !vmdk_is_cid_valid(bs)) {
> > +ret = VMDK_ERROR;
> > +goto exit;
> > +}
> >  
> > -/* floor offset to cluster */
> > -offset -= offset % (extent->cluster_sectors * 512);
> > -ret = bdrv_read(bs->backing_hd, offset >> 9, whole_grain,
> > -extent->cluster_sectors);
> > +/* Read backing data before skip range */
> > +if (skip_start > 0) {
> > +if (bs->backing_hd) {
> > +ret = bdrv_read(bs->backing_hd, sector_num,
> > +whole_grain, skip_start);
> > +if (ret < 0) {
> > +ret = VMDK_ERROR;
> > +goto exit;
> > +}
> > +}
> > +ret = bdrv_write(extent->file, cluster_sector_num, whole_grain,
> > + skip_start);
> >  if (ret < 0) {
> >  ret = VMDK_ERROR;
> >  goto exit;
> >  }
> > -
> > -/* Write grain only into the active image */
> > -ret = bdrv_write(extent->file, cluster_of

[Qemu-devel] [PATCH 3/5] block: make 'top' argument to block-commit optional

2014-05-14 Thread Jeff Cody
Now that active layer block-commit is supported, the 'top' argument
no longer needs to be mandatory.

Change it optional, with the default being the active layer in the
device chain.

Signed-off-by: Jeff Cody 
---
 blockdev.c |  3 ++-
 qapi-schema.json   |  7 ---
 qmp-commands.hx|  5 +++--
 tests/qemu-iotests/040 | 28 ++--
 4 files changed, 27 insertions(+), 16 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 500707e..02c6525 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1868,7 +1868,8 @@ void qmp_block_stream(const char *device, bool has_base,
 }
 
 void qmp_block_commit(const char *device,
-  bool has_base, const char *base, const char *top,
+  bool has_base, const char *base,
+  bool has_top, const char *top,
   bool has_speed, int64_t speed,
   Error **errp)
 {
diff --git a/qapi-schema.json b/qapi-schema.json
index 36cb964..06a9b5d 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2099,8 +2099,9 @@
 # @base:   #optional The file name of the backing image to write data into.
 #If not specified, this is the deepest backing image
 #
-# @top:  The file name of the backing image within the image chain,
-#which contains the topmost data to be committed down.
+# @top:#optional The file name of the backing image within the image chain,
+#which contains the topmost data to be committed down. If
+#not specified, this is the active layer.
 #
 #If top == base, that is an error.
 #If top == active, the job will not be completed by itself,
@@ -2128,7 +2129,7 @@
 #
 ##
 { 'command': 'block-commit',
-  'data': { 'device': 'str', '*base': 'str', 'top': 'str',
+  'data': { 'device': 'str', '*base': 'str', '*top': 'str',
 '*speed': 'int' } }
 
 ##
diff --git a/qmp-commands.hx b/qmp-commands.hx
index cae890e..1aa3c65 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -985,7 +985,7 @@ EQMP
 
 {
 .name   = "block-commit",
-.args_type  = "device:B,base:s?,top:s,speed:o?",
+.args_type  = "device:B,base:s?,top:s?,speed:o?",
 .mhandler.cmd_new = qmp_marshal_input_block_commit,
 },
 
@@ -1003,7 +1003,8 @@ Arguments:
   If not specified, this is the deepest backing image
   (json-string, optional)
 - "top":  The file name of the backing image within the image chain,
-  which contains the topmost data to be committed down.
+  which contains the topmost data to be committed down. If
+  not specified, this is the active layer. (json-string, optional)
 
   If top == base, that is an error.
   If top == active, the job will not be completed by itself,
diff --git a/tests/qemu-iotests/040 b/tests/qemu-iotests/040
index 734b6a6..803b0c7 100755
--- a/tests/qemu-iotests/040
+++ b/tests/qemu-iotests/040
@@ -35,11 +35,7 @@ test_img = os.path.join(iotests.test_dir, 'test.img')
 class ImageCommitTestCase(iotests.QMPTestCase):
 '''Abstract base class for image commit test cases'''
 
-def run_commit_test(self, top, base):
-self.assert_no_active_block_jobs()
-result = self.vm.qmp('block-commit', device='drive0', top=top, 
base=base)
-self.assert_qmp(result, 'return', {})
-
+def wait_for_complete(self):
 completed = False
 while not completed:
 for event in self.vm.get_qmp_events(wait=True):
@@ -58,6 +54,18 @@ class ImageCommitTestCase(iotests.QMPTestCase):
 self.assert_no_active_block_jobs()
 self.vm.shutdown()
 
+def run_commit_test(self, top, base):
+self.assert_no_active_block_jobs()
+result = self.vm.qmp('block-commit', device='drive0', top=top, 
base=base)
+self.assert_qmp(result, 'return', {})
+self.wait_for_complete()
+
+def run_default_commit_test(self):
+self.assert_no_active_block_jobs()
+result = self.vm.qmp('block-commit', device='drive0')
+self.assert_qmp(result, 'return', {})
+self.wait_for_complete()
+
 class TestSingleDrive(ImageCommitTestCase):
 image_len = 1 * 1024 * 1024
 test_len = 1 * 1024 * 256
@@ -109,17 +117,17 @@ class TestSingleDrive(ImageCommitTestCase):
 self.assertEqual(-1, qemu_io('-c', 'read -P 0xab 0 524288', 
backing_img).find("verification failed"))
 self.assertEqual(-1, qemu_io('-c', 'read -P 0xef 524288 524288', 
backing_img).find("verification failed"))
 
+def test_top_is_default_active(self):
+self.run_default_commit_test()
+self.assertEqual(-1, qemu_io('-c', 'read -P 0xab 0 524288', 
backing_img).find("verification failed"))
+self.assertEqual(-1, qemu_io('-c', 'read -P 0xef 524288 524288', 
backing_img).find("verification failed"))
+
 def test_top_and_base_reversed(self):
 self.assert_no

[Qemu-devel] [PATCH 4/5] block: Accept node-name arguments for block-commit

2014-05-14 Thread Jeff Cody
This modifies the block operation block-commit so that it will
accept node-name arguments for either 'top' or 'base' BDS.

The filename and node-name are mutually exclusive to each other;
i.e.:
"top" and "top-node-name" are mutually exclusive (enforced)
"base" and "base-node-name" are mutually exclusive (enforced)

The preferred and recommended method now of using block-commit
is to specify the BDS to operate on via their node-name arguments.

This also adds an explicit check that base_bs is in the chain of
top_bs, because if they are referenced by their unique node-name
arguments, the discovery process does not intrinsically verify
they are in the same chain.

Signed-off-by: Jeff Cody 
---
 blockdev.c   | 35 +--
 qapi-schema.json | 35 ++-
 qmp-commands.hx  | 29 ++---
 3 files changed, 81 insertions(+), 18 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 02c6525..d8cb396 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1869,7 +1869,9 @@ void qmp_block_stream(const char *device, bool has_base,
 
 void qmp_block_commit(const char *device,
   bool has_base, const char *base,
+  bool has_base_node_name, const char *base_node_name,
   bool has_top, const char *top,
+  bool has_top_node_name, const char *top_node_name,
   bool has_speed, int64_t speed,
   Error **errp)
 {
@@ -1888,6 +1890,15 @@ void qmp_block_commit(const char *device,
 /* drain all i/o before commits */
 bdrv_drain_all();
 
+if (has_base && has_base_node_name) {
+error_setg(errp, "'base' and 'base-node-name' are mutually exclusive");
+return;
+}
+if (has_top && has_top_node_name) {
+error_setg(errp, "'top' and 'top-node-name' are mutually exclusive");
+return;
+}
+
 bs = bdrv_find(device);
 if (!bs) {
 error_set(errp, QERR_DEVICE_NOT_FOUND, device);
@@ -1897,7 +1908,14 @@ void qmp_block_commit(const char *device,
 /* default top_bs is the active layer */
 top_bs = bs;
 
-if (top) {
+/* Find the 'top' image file for the commit */
+if (has_top_node_name) {
+top_bs = bdrv_lookup_bs(NULL, top_node_name, &local_err);
+if (local_err) {
+error_propagate(errp, local_err);
+return;
+}
+} else if (has_top && top) {
 if (strcmp(bs->filename, top) != 0) {
 top_bs = bdrv_find_backing_image(bs, top);
 }
@@ -1908,7 +1926,14 @@ void qmp_block_commit(const char *device,
 return;
 }
 
-if (has_base && base) {
+/* Find the 'base' image file for the commit */
+if (has_base_node_name) {
+base_bs = bdrv_lookup_bs(NULL, base_node_name, &local_err);
+if (local_err) {
+error_propagate(errp, local_err);
+return;
+}
+} else if (has_base && base) {
 base_bs = bdrv_find_backing_image(top_bs, base);
 } else {
 base_bs = bdrv_find_base(top_bs);
@@ -1919,6 +1944,12 @@ void qmp_block_commit(const char *device,
 return;
 }
 
+/* Verify that 'base' is in the same chain as 'top' */
+if (!bdrv_is_in_chain(top_bs, base_bs)) {
+error_setg(errp, "'base' and 'top' are not in the same chain");
+return;
+}
+
 if (top_bs == bs) {
 commit_active_start(bs, base_bs, speed, on_error, block_job_cb,
 bs, &local_err);
diff --git a/qapi-schema.json b/qapi-schema.json
index 06a9b5d..eddb2b8 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2096,12 +2096,27 @@
 #
 # @device:  the name of the device
 #
-# @base:   #optional The file name of the backing image to write data into.
-#If not specified, this is the deepest backing image
+# For 'base', either @base or @base-node-name may be set but not both. If
+# neither is specified, this is the deepest backing image
 #
-# @top:#optional The file name of the backing image within the image chain,
-#which contains the topmost data to be committed down. If
-#not specified, this is the active layer.
+# @base:  #optional The file name of the backing image to write data
+#   into.
+#
+# @base-node-name #optional The name of the block driver state node of the
+#   backing image to write data into.
+#   (Since 2.1)
+#
+# For 'top', either @top or @top-node-name must be set but not both.
+#
+# @top:   #optional The file name of the backing image within the image
+#   chain, which contains the topmost data to be
+#   committed down. If not specified, this is the
+#   active layer.
+#
+# @top-node-name: #optional The block driver state node name of the backing
+#  

[Qemu-devel] [PATCH 2/5] block: add helper function to determine if a BDS is in a chain

2014-05-14 Thread Jeff Cody
This is a small helper function, to determine if 'base' is in the
chain of BlockDriverState 'top'.  It returns true if it is in the chain,
and false otherwise.

If either argument is NULL, it will also return false.

Signed-off-by: Jeff Cody 
---
 block.c   | 9 +
 include/block/block.h | 1 +
 2 files changed, 10 insertions(+)

diff --git a/block.c b/block.c
index 81945d3..c4f77c2 100644
--- a/block.c
+++ b/block.c
@@ -3734,6 +3734,15 @@ BlockDriverState *bdrv_lookup_bs(const char *device,
 return NULL;
 }
 
+bool bdrv_is_in_chain(BlockDriverState *top, BlockDriverState *base)
+{
+while (top && top != base) {
+top = top->backing_hd;
+}
+
+return top != NULL;
+}
+
 BlockDriverState *bdrv_next(BlockDriverState *bs)
 {
 if (!bs) {
diff --git a/include/block/block.h b/include/block/block.h
index 1b119aa..283a6f3 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -381,6 +381,7 @@ BlockDeviceInfoList *bdrv_named_nodes_list(void);
 BlockDriverState *bdrv_lookup_bs(const char *device,
  const char *node_name,
  Error **errp);
+bool bdrv_is_in_chain(BlockDriverState *top, BlockDriverState *base);
 BlockDriverState *bdrv_next(BlockDriverState *bs);
 void bdrv_iterate(void (*it)(void *opaque, BlockDriverState *bs),
   void *opaque);
-- 
1.8.3.1




[Qemu-devel] [PATCH 5/5] block: extend block-commit to accept a string for the backing file

2014-05-14 Thread Jeff Cody
On some image chains, QEMU may not always be able to resolve the
filenames properly, when updating the backing file of an image
after a block commit.

For instance, certain relative pathnames may fail, or drives may
have been specified originally by file descriptor (e.g. /dev/fd/???),
or a relative protocol pathname may have been used.

In these instances, QEMU may lack the information to be able to make
the correct choice, but the user or management layer most likely does
have that knowledge.

With this extension to the block-commit api, the user is able to change
the backing file of the overlay image as part of the block-commit
operation.

This allows the change to be 'safe', in the sense that if the attempt
to write the overlay image metadata fails, then the block-commit
operation returns failure, without disrupting the guest.

If the commit top is the active layer, then specifying the backing
file string will be treated as an error (there is no overlay image
to modify in that case).

If a backing file string is not specified in the command, the backing
file string to use is determined in the same manner as it was
previously.

Signed-off-by: Jeff Cody 
---
 block.c   |  8 ++--
 block/commit.c|  9 ++---
 blockdev.c|  8 +++-
 include/block/block.h |  3 ++-
 include/block/block_int.h |  3 ++-
 qapi-schema.json  | 18 --
 qmp-commands.hx   | 14 +-
 7 files changed, 52 insertions(+), 11 deletions(-)

diff --git a/block.c b/block.c
index c4f77c2..7e63a1f 100644
--- a/block.c
+++ b/block.c
@@ -2547,12 +2547,15 @@ typedef struct BlkIntermediateStates {
  *
  * base <- active
  *
+ * If backing_file_str is non-NULL, it will be used when modifying top's
+ * overlay image metadata.
+ *
  * Error conditions:
  *  if active == top, that is considered an error
  *
  */
 int bdrv_drop_intermediate(BlockDriverState *active, BlockDriverState *top,
-   BlockDriverState *base)
+   BlockDriverState *base, const char 
*backing_file_str)
 {
 BlockDriverState *intermediate;
 BlockDriverState *base_bs = NULL;
@@ -2604,7 +2607,8 @@ int bdrv_drop_intermediate(BlockDriverState *active, 
BlockDriverState *top,
 }
 
 /* success - we can delete the intermediate states, and link top->base */
-ret = bdrv_change_backing_file(new_top_bs, base_bs->filename,
+backing_file_str = backing_file_str ? backing_file_str : base_bs->filename;
+ret = bdrv_change_backing_file(new_top_bs, backing_file_str,
base_bs->drv ? base_bs->drv->format_name : 
"");
 if (ret) {
 goto exit;
diff --git a/block/commit.c b/block/commit.c
index 5c09f44..91517d3 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -37,6 +37,7 @@ typedef struct CommitBlockJob {
 BlockdevOnError on_error;
 int base_flags;
 int orig_overlay_flags;
+char *backing_file_str;
 } CommitBlockJob;
 
 static int coroutine_fn commit_populate(BlockDriverState *bs,
@@ -141,7 +142,7 @@ wait:
 
 if (!block_job_is_cancelled(&s->common) && sector_num == end) {
 /* success */
-ret = bdrv_drop_intermediate(active, top, base);
+ret = bdrv_drop_intermediate(active, top, base, s->backing_file_str);
 }
 
 exit_free_buf:
@@ -158,7 +159,7 @@ exit_restore_reopen:
 if (overlay_bs && s->orig_overlay_flags != bdrv_get_flags(overlay_bs)) {
 bdrv_reopen(overlay_bs, s->orig_overlay_flags, NULL);
 }
-
+g_free(s->backing_file_str);
 block_job_completed(&s->common, ret);
 }
 
@@ -182,7 +183,7 @@ static const BlockJobDriver commit_job_driver = {
 void commit_start(BlockDriverState *bs, BlockDriverState *base,
   BlockDriverState *top, int64_t speed,
   BlockdevOnError on_error, BlockDriverCompletionFunc *cb,
-  void *opaque, Error **errp)
+  void *opaque, const char *backing_file_str, Error **errp)
 {
 CommitBlockJob *s;
 BlockReopenQueue *reopen_queue = NULL;
@@ -244,6 +245,8 @@ void commit_start(BlockDriverState *bs, BlockDriverState 
*base,
 s->base_flags  = orig_base_flags;
 s->orig_overlay_flags  = orig_overlay_flags;
 
+s->backing_file_str = g_strdup(backing_file_str);
+
 s->on_error = on_error;
 s->common.co = qemu_coroutine_create(commit_run);
 
diff --git a/blockdev.c b/blockdev.c
index d8cb396..c0c7867 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1872,6 +1872,7 @@ void qmp_block_commit(const char *device,
   bool has_base_node_name, const char *base_node_name,
   bool has_top, const char *top,
   bool has_top_node_name, const char *top_node_name,
+  bool has_backing_file, const char *backing_file,
   bool has_speed, int64_t speed,
   Error **errp)
 {
@@ -1951,11 +1952,16 @@ void qmp_block_commit(const c

[Qemu-devel] [PATCH 1/5] block: Auto-generate node_names for each BDS entry

2014-05-14 Thread Jeff Cody
Currently, node_name is only filled in when done so explicitly by the
user.  If no node_name is specified, then the node name field is not
populated.

If node_names are automatically generated when not specified, that means
that all block job operations can be done by reference to the unique
node_name field.  This eliminates ambiguity in filename pathing
(relative filenames, or file descriptors, symlinks, mounts, etc..) that
qemu currently needs to deal with.

If a node name is specified, then it will not be automatically
generated for that BDS entry.

If it is automatically generated, it will be prefaced with "__qemu##",
followed by 8 characters of a unique number, followed by 8 random
ASCII characters in the range of 'A-Z'.  Some sample generated node-name
strings:
__qemu##IAIYNXXR
__qemu##0002METXTRBQ
__qemu##0001FMBORDWG

The prefix is to aid in identifying it as a qemu-generated name, the
numeric portion is to guarantee uniqueness in a given qemu session, and
the random characters are to further avoid any accidental collisions
with user-specified node-names.

Signed-off-by: Jeff Cody 
---
 block.c | 16 +++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/block.c b/block.c
index c90c71a..81945d3 100644
--- a/block.c
+++ b/block.c
@@ -838,12 +838,26 @@ static int bdrv_open_flags(BlockDriverState *bs, int 
flags)
 return open_flags;
 }
 
+#define GEN_NODE_NAME_PREFIX"__qemu##"
+#define GEN_NODE_NAME_MAX_LEN   (sizeof(GEN_NODE_NAME_PREFIX) + 8 + 8)
 static void bdrv_assign_node_name(BlockDriverState *bs,
   const char *node_name,
   Error **errp)
 {
+char gen_node_name[GEN_NODE_NAME_MAX_LEN];
+static uint32_t counter; /* simple counter to guarantee uniqueness */
+
+/* if node_name is NULL, auto-generate a node name */
 if (!node_name) {
-return;
+int len;
+snprintf(gen_node_name, GEN_NODE_NAME_MAX_LEN,
+ "%s%08x", GEN_NODE_NAME_PREFIX, counter++);
+len = strlen(gen_node_name);
+while (len < GEN_NODE_NAME_MAX_LEN - 1) {
+gen_node_name[len++] = g_random_int_range('A', 'Z');
+}
+gen_node_name[GEN_NODE_NAME_MAX_LEN - 1] = '\0';
+node_name = gen_node_name;
 }
 
 /* empty string node name is invalid */
-- 
1.8.3.1




[Qemu-devel] [PATCH 0/5] block: Modify block-commit to use node-names

2014-05-14 Thread Jeff Cody
Using node-names instead of filenames for block job operations
over QMP is a superior method of identifying the block driver
images to operate on, as it removes all pathname ambiguity.

This series is the conversion of block-commit to allow use of
node-names.  Also, it allows the user to specify the string for
the backing_file name to use in the overlay image.

So that node-names can be used as desired for all block job
operations, this series also auto-generates node-names for every
BDS.  User-specified node-names will override any autogenerated
node-names.

Subsequent patches will convert the remaining block operations
(stream, backup, mirror)

These patches can also be seen at: 
https://github.com/codyprime/qemu-kvm-jtc.git, tag block-commit-node-v1a

Jeff Cody (5):
  block: Auto-generate node_names for each BDS entry
  block: add helper function to determine if a BDS is in a chain
  block: make 'top' argument to block-commit optional
  block: Accept node-name arguments for block-commit
  block: extend block-commit to accept a string for the backing file

 block.c   | 33 ---
 block/commit.c|  9 ++---
 blockdev.c| 46 +++
 include/block/block.h |  4 +++-
 include/block/block_int.h |  3 ++-
 qapi-schema.json  | 50 ++-
 qmp-commands.hx   | 40 +++--
 tests/qemu-iotests/040| 28 --
 8 files changed, 176 insertions(+), 37 deletions(-)

-- 
1.8.3.1




[Qemu-devel] [PULL 2/6] Split ram_save_block

2014-05-14 Thread Juan Quintela
From: "Dr. David Alan Gilbert" 

ram_save_block is getting a bit too complicated, and does two separate
things:
   1) Finds a page to send
   2) Sends the page (dealing with compression etc)

Split into 'ram_save_page' to send the page and deal with compression (2)
Rename remaining function to 'ram_find_and_save_block'

Signed-off-by: Dr. David Alan Gilbert 
Signed-off-by: Juan Quintela 
---
 arch_init.c | 141 ++--
 1 file changed, 79 insertions(+), 62 deletions(-)

diff --git a/arch_init.c b/arch_init.c
index 4e8f2c8..685ba0e 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -560,20 +560,93 @@ static void migration_bitmap_sync(void)
 }

 /*
- * ram_save_block: Writes a page of memory to the stream f
+ * ram_save_page: Send the given page to the stream
+ *
+ * Returns: Number of bytes written.
+ */
+static int ram_save_page(QEMUFile *f, RAMBlock* block, ram_addr_t offset,
+ bool last_stage)
+{
+int bytes_sent;
+int cont;
+ram_addr_t current_addr;
+MemoryRegion *mr = block->mr;
+uint8_t *p;
+int ret;
+bool send_async = true;
+
+cont = (block == last_sent_block) ? RAM_SAVE_FLAG_CONTINUE : 0;
+
+p = memory_region_get_ram_ptr(mr) + offset;
+
+/* In doubt sent page as normal */
+bytes_sent = -1;
+ret = ram_control_save_page(f, block->offset,
+   offset, TARGET_PAGE_SIZE, &bytes_sent);
+
+XBZRLE_cache_lock();
+
+current_addr = block->offset + offset;
+if (ret != RAM_SAVE_CONTROL_NOT_SUPP) {
+if (ret != RAM_SAVE_CONTROL_DELAYED) {
+if (bytes_sent > 0) {
+acct_info.norm_pages++;
+} else if (bytes_sent == 0) {
+acct_info.dup_pages++;
+}
+}
+} else if (is_zero_range(p, TARGET_PAGE_SIZE)) {
+acct_info.dup_pages++;
+bytes_sent = save_block_hdr(f, block, offset, cont,
+RAM_SAVE_FLAG_COMPRESS);
+qemu_put_byte(f, 0);
+bytes_sent++;
+/* Must let xbzrle know, otherwise a previous (now 0'd) cached
+ * page would be stale
+ */
+xbzrle_cache_zero_page(current_addr);
+} else if (!ram_bulk_stage && migrate_use_xbzrle()) {
+bytes_sent = save_xbzrle_page(f, &p, current_addr, block,
+  offset, cont, last_stage);
+if (!last_stage) {
+/* Can't send this cached data async, since the cache page
+ * might get updated before it gets to the wire
+ */
+send_async = false;
+}
+}
+
+/* XBZRLE overflow or normal page */
+if (bytes_sent == -1) {
+bytes_sent = save_block_hdr(f, block, offset, cont, 
RAM_SAVE_FLAG_PAGE);
+if (send_async) {
+qemu_put_buffer_async(f, p, TARGET_PAGE_SIZE);
+} else {
+qemu_put_buffer(f, p, TARGET_PAGE_SIZE);
+}
+bytes_sent += TARGET_PAGE_SIZE;
+acct_info.norm_pages++;
+}
+
+XBZRLE_cache_unlock();
+
+return bytes_sent;
+}
+
+/*
+ * ram_find_and_save_block: Finds a page to send and sends it to f
  *
  * Returns:  The number of bytes written.
  *   0 means no dirty pages
  */

-static int ram_save_block(QEMUFile *f, bool last_stage)
+static int ram_find_and_save_block(QEMUFile *f, bool last_stage)
 {
 RAMBlock *block = last_seen_block;
 ram_addr_t offset = last_offset;
 bool complete_round = false;
 int bytes_sent = 0;
 MemoryRegion *mr;
-ram_addr_t current_addr;

 if (!block)
 block = QTAILQ_FIRST(&ram_list.blocks);
@@ -594,64 +667,8 @@ static int ram_save_block(QEMUFile *f, bool last_stage)
 ram_bulk_stage = false;
 }
 } else {
-int ret;
-uint8_t *p;
-bool send_async = true;
-int cont = (block == last_sent_block) ?
-RAM_SAVE_FLAG_CONTINUE : 0;
-
-p = memory_region_get_ram_ptr(mr) + offset;
-
-/* In doubt sent page as normal */
-bytes_sent = -1;
-ret = ram_control_save_page(f, block->offset,
-   offset, TARGET_PAGE_SIZE, &bytes_sent);
-
-XBZRLE_cache_lock();
-
-current_addr = block->offset + offset;
-if (ret != RAM_SAVE_CONTROL_NOT_SUPP) {
-if (ret != RAM_SAVE_CONTROL_DELAYED) {
-if (bytes_sent > 0) {
-acct_info.norm_pages++;
-} else if (bytes_sent == 0) {
-acct_info.dup_pages++;
-}
-}
-} else if (is_zero_range(p, TARGET_PAGE_SIZE)) {
-acct_info.dup_pages++;
-bytes_sent = save_block_hdr(f, block, offset, cont,
-RAM_SAVE_FLAG_COMPRESS);
-qemu_put_byte(f, 0);
-bytes_s

[Qemu-devel] [PULL 0/6] migration queue

2014-05-14 Thread Juan Quintela
Hi peter

This pull request includes:
- split ram_save_block into two functions (David)
- simplify code for load_xbzrle (Chen)
- fix usb tests (fix on top of previous fixes)) (mst)
- calculate at time of printing throughput (peter)
- cleanup rest & usb devices version_minimum_id_old (qunitela)

Please, apply.

The following changes since commit f30d56e7d63fe2f536511bffa13306bec2e01c37:

  Merge remote-tracking branch 'remotes/rth/fix-i386' into staging (2014-05-13 
18:36:19 +0100)

are available in the git repository at:


  git://github.com/juanquintela/qemu.git tags/migration/20140515

for you to fetch changes up to 719ffe1f5f72b1c7ace4afe9ba2815bcb53a829e:

  usb: fix up post load checks (2014-05-14 15:24:52 +0200)


migration/next for 20140515


Chen Gang (1):
  arch_init: Simplify code for load_xbzrle()

Dr. David Alan Gilbert (1):
  Split ram_save_block

Juan Quintela (2):
  savevm: Remove all the unneeded version_minimum_id_old (usb)
  savevm: Remove all the unneeded version_minimum_id_old (rest)

Michael S. Tsirkin (1):
  usb: fix up post load checks

Peter Lieven (1):
  migration: show average throughput when migration finishes

 arch_init.c  | 155 +++
 audio/audio.c|   3 +-
 cpus.c   |   3 +-
 docs/migration.txt   |   6 +-
 exec.c   |   3 +-
 hw/audio/milkymist-ac97.c|   3 +-
 hw/block/m25p80.c|   1 -
 hw/char/ipoctal232.c |   9 +--
 hw/char/lm32_juart.c |   3 +-
 hw/char/lm32_uart.c  |   3 +-
 hw/char/milkymist-uart.c |   3 +-
 hw/char/sclpconsole-lm.c |   3 +-
 hw/char/sclpconsole.c|   3 +-
 hw/core/ptimer.c |   3 +-
 hw/display/cg3.c |   2 +-
 hw/display/g364fb.c  |   1 -
 hw/display/jazz_led.c|   1 -
 hw/display/milkymist-tmu2.c  |   3 +-
 hw/display/milkymist-vgafb.c |   3 +-
 hw/display/tcx.c |   3 +-
 hw/dma/sparc32_dma.c |   3 +-
 hw/dma/sun4m_iommu.c |   3 +-
 hw/i2c/core.c|   6 +-
 hw/i2c/smbus_ich9.c  |   1 -
 hw/ide/core.c|  16 ++---
 hw/ide/macio.c   |   3 +-
 hw/ide/microdrive.c  |   3 +-
 hw/ide/mmio.c|   3 +-
 hw/input/adb.c   |   6 +-
 hw/input/milkymist-softusb.c |   3 +-
 hw/intc/lm32_pic.c   |   3 +-
 hw/intc/slavio_intctl.c  |   6 +-
 hw/ipack/ipack.c |   3 +-
 hw/ipack/tpci200.c   |   3 +-
 hw/misc/eccmemctl.c  |   3 +-
 hw/misc/lm32_sys.c   |   3 +-
 hw/misc/macio/cuda.c |   6 +-
 hw/misc/macio/mac_dbdma.c|   6 +-
 hw/misc/milkymist-hpdmc.c|   3 +-
 hw/misc/milkymist-pfpu.c |   3 +-
 hw/misc/slavio_misc.c|   3 +-
 hw/net/lance.c   |   3 +-
 hw/net/milkymist-minimac2.c  |   6 +-
 hw/net/mipsnet.c |   3 +-
 hw/nvram/ds1225y.c   |   1 -
 hw/nvram/mac_nvram.c |   3 +-
 hw/pci-host/bonito.c |   3 +-
 hw/s390x/event-facility.c|   3 +-
 hw/s390x/sclpquiesce.c   |   3 +-
 hw/scsi/esp-pci.c|   1 -
 hw/scsi/esp.c|   4 +-
 hw/sd/milkymist-memcard.c|   3 +-
 hw/sd/sdhci.c|   2 +-
 hw/timer/lm32_timer.c|   3 +-
 hw/timer/milkymist-sysctl.c  |   3 +-
 hw/timer/slavio_timer.c  |   6 +-
 hw/usb/bus.c |   6 +-
 hw/usb/dev-hid.c |   4 +-
 hw/usb/dev-hub.c |   4 +-
 hw/usb/dev-storage.c |   2 +-
 hw/usb/hcd-ehci-pci.c|   2 +-
 hw/usb/hcd-ehci-sysbus.c |   2 +-
 hw/usb/hcd-ehci.c|   2 +-
 hw/usb/hcd-uhci.c|   6 +-
 migration.c  |   5 ++
 target-alpha/machine.c   |   2 -
 target-lm32/machine.c|   6 +-
 target-moxie/machine.c   |   3 +-
 target-openrisc/machine.c|   2 -
 tests/test-vmstate.c |   9 +--
 70 files changed, 171 insertions(+), 234 deletions(-)



Re: [Qemu-devel] [PULL 5/6] migration: show average throughput when migration finishes

2014-05-14 Thread Eric Blake
On 05/14/2014 07:40 PM, Juan Quintela wrote:
> From: Peter Lieven 
> 
> currently the value of the throughput field contains whatever
> was the last calculated throughput shortly before the migration
> finished.
> 
> This patch updates the post migration contents of the field to
> the average throughput.
> 
> Signed-off-by: Peter Lieven 
> 
> Reviewed-by: Paolo Bonzini 
> Reviewed-by: Juan Quintela 
> Signed-by: Juan Quintela 
> Signed-off-by: Juan Quintela 

Signed-by is an unusual tag.  Worth fixing before the pull finalizes?

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH] iotests: Use configured python

2014-05-14 Thread Fam Zheng
On Thu, 05/15 01:41, Max Reitz wrote:
> On 14.05.2014 14:33, Markus Armbruster wrote:
> >Max Reitz  writes:
> >
> >>Currently, QEMU's iotests rely on /usr/bin/env to start the correct
> >>Python (that is, at least Python 2.4, but not 3). On systems where
> >>Python 3 is the default, the user has no clean way of making the iotests
> >>use the correct binary.
> >>
> >>This commit makes the iotests use the Python selected by configure.
> >>
> >>Signed-off-by: Max Reitz 
> >I'm afraid this broke qemu-iotests in a separate build tree:
> >
> > ./check: line 38: ./common.env: No such file or directory
> 
> As I already replied to Peter, I see two (or maybe three) ways to fix this:
> 
> The first is, we use the correct path to common.env. This would however
> result in modification of the source tree although this is probably not what
> the user intends with an out-of-tree build. On the other hand, this would
> just work.
> 
> The second is, we do not create common.env for out-of-tree builds and add a
> default common.env to the repository ("PYTHON = python" should probably
> suffice). This would not introduce any regressions, however, the iotests
> would remain problematic on systems with Python 3 being the default when
> using out-of-tree builds. As I guess that out-of-tree builds are actually
> recommended, this would mean that the better solution might be to revert my
> patch and instead change every "python" occurrence in the iotests' Shebangs
> to "python2", as kind of a third way to go. However, for this I'm not sure
> whether all systems which are supposed to be supported by qemu actually have
> a "python2" executable/symlink. I guess so, but then again...
> 
> So, either we fix it but try to write to the source tree without the user
> intending to modify it; or we fix it for in-tree builds only; or we drop the
> magic and just use "python2" in the iotests' Shebangs.

Why can't we just tell ./check the path to common.env with an env var, like how
we tell ./check the path to qemu-img with QEMU_IMG_PROG?

Fam



Re: [Qemu-devel] usb: usb tablet freeze when save/restore guest os

2014-05-14 Thread Gonglei (Arei)
Hi,

> -Original Message-
> From: Paolo Bonzini [mailto:pbonz...@redhat.com]
> Sent: Wednesday, May 14, 2014 6:26 PM
> To: Gonglei (Arei); Gerd Hoffmann
> Cc: qemu-devel@nongnu.org; Huangweidong (C); Michael S. Tsirkin
> Subject: Re: usb: usb tablet freeze when save/restore guest os
> 
> Il 14/05/2014 03:47, Gonglei (Arei) ha scritto:
> > > There was a kernel bug recently causing lost interrupts on migration,
> > > maybe this is the same (rh bug 1036478)?  Paolo?  Is the fix upstream?
> > > Which kernel has it?
> 
> It is in 3.15, but it only affects edge-triggered interrupts, so not UHCI.
> 
> Paolo
> 
Okay. 

Well then, may I post a formal patch for this issue, Gerd? Thanks.


Best regards,
-Gonglei


[Qemu-devel] [PULL 5/6] migration: show average throughput when migration finishes

2014-05-14 Thread Juan Quintela
From: Peter Lieven 

currently the value of the throughput field contains whatever
was the last calculated throughput shortly before the migration
finished.

This patch updates the post migration contents of the field to
the average throughput.

Signed-off-by: Peter Lieven 

Reviewed-by: Paolo Bonzini 
Reviewed-by: Juan Quintela 
Signed-by: Juan Quintela 
Signed-off-by: Juan Quintela 
---
 migration.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/migration.c b/migration.c
index 52cda27..3fc03d6 100644
--- a/migration.c
+++ b/migration.c
@@ -662,8 +662,13 @@ static void *migration_thread(void *opaque)
 qemu_mutex_lock_iothread();
 if (s->state == MIG_STATE_COMPLETED) {
 int64_t end_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
+uint64_t transferred_bytes = qemu_ftell(s->file);
 s->total_time = end_time - s->total_time;
 s->downtime = end_time - start_time;
+if (s->total_time) {
+s->mbps = (((double) transferred_bytes * 8.0) /
+   ((double) s->total_time)) / 1000;
+}
 runstate_set(RUN_STATE_POSTMIGRATE);
 } else {
 if (old_vm_running) {
-- 
1.9.0




[Qemu-devel] [PULL 4/6] savevm: Remove all the unneeded version_minimum_id_old (rest)

2014-05-14 Thread Juan Quintela
After previous Peter patch, they are redundant.  This way we don't
assign them except when needed.  Once there, there were lots of case
where the ".fields" indentation was wrong:

 .fields = (VMStateField []) {
and
 .fields =  (VMStateField []) {

Change all the combinations to:

 .fields = (VMStateField[]){

The biggest problem (appart from aesthetics) was that checkpatch complained
when we copy&pasted the code from one place to another.

Signed-off-by: Juan Quintela 
Reviewed-by: Peter Maydell 
---
 audio/audio.c|  3 +--
 cpus.c   |  3 +--
 docs/migration.txt   |  6 +++---
 exec.c   |  3 +--
 hw/audio/milkymist-ac97.c|  3 +--
 hw/block/m25p80.c|  1 -
 hw/char/ipoctal232.c |  9 +++--
 hw/char/lm32_juart.c |  3 +--
 hw/char/lm32_uart.c  |  3 +--
 hw/char/milkymist-uart.c |  3 +--
 hw/char/sclpconsole-lm.c |  3 +--
 hw/char/sclpconsole.c|  3 +--
 hw/core/ptimer.c |  3 +--
 hw/display/cg3.c |  2 +-
 hw/display/g364fb.c  |  1 -
 hw/display/jazz_led.c|  1 -
 hw/display/milkymist-tmu2.c  |  3 +--
 hw/display/milkymist-vgafb.c |  3 +--
 hw/display/tcx.c |  3 +--
 hw/dma/sparc32_dma.c |  3 +--
 hw/dma/sun4m_iommu.c |  3 +--
 hw/i2c/core.c|  6 ++
 hw/i2c/smbus_ich9.c  |  1 -
 hw/ide/core.c| 16 +---
 hw/ide/macio.c   |  3 +--
 hw/ide/microdrive.c  |  3 +--
 hw/ide/mmio.c|  3 +--
 hw/input/adb.c   |  6 ++
 hw/input/milkymist-softusb.c |  3 +--
 hw/intc/lm32_pic.c   |  3 +--
 hw/intc/slavio_intctl.c  |  6 ++
 hw/ipack/ipack.c |  3 +--
 hw/ipack/tpci200.c   |  3 +--
 hw/misc/eccmemctl.c  |  3 +--
 hw/misc/lm32_sys.c   |  3 +--
 hw/misc/macio/cuda.c |  6 ++
 hw/misc/macio/mac_dbdma.c|  6 ++
 hw/misc/milkymist-hpdmc.c|  3 +--
 hw/misc/milkymist-pfpu.c |  3 +--
 hw/misc/slavio_misc.c|  3 +--
 hw/net/lance.c   |  3 +--
 hw/net/milkymist-minimac2.c  |  6 ++
 hw/net/mipsnet.c |  3 +--
 hw/nvram/ds1225y.c   |  1 -
 hw/nvram/mac_nvram.c |  3 +--
 hw/pci-host/bonito.c |  3 +--
 hw/s390x/event-facility.c|  3 +--
 hw/s390x/sclpquiesce.c   |  3 +--
 hw/scsi/esp-pci.c|  1 -
 hw/scsi/esp.c|  4 +---
 hw/sd/milkymist-memcard.c|  3 +--
 hw/sd/sdhci.c|  2 +-
 hw/timer/lm32_timer.c|  3 +--
 hw/timer/milkymist-sysctl.c  |  3 +--
 hw/timer/slavio_timer.c  |  6 ++
 target-alpha/machine.c   |  2 --
 target-lm32/machine.c|  6 ++
 target-moxie/machine.c   |  3 +--
 target-openrisc/machine.c|  2 --
 tests/test-vmstate.c |  9 +++--
 60 files changed, 70 insertions(+), 147 deletions(-)

diff --git a/audio/audio.c b/audio/audio.c
index fc77511..9d018e9 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -1812,8 +1812,7 @@ static const VMStateDescription vmstate_audio = {
 .name = "audio",
 .version_id = 1,
 .minimum_version_id = 1,
-.minimum_version_id_old = 1,
-.fields  = (VMStateField []) {
+.fields = (VMStateField[]) {
 VMSTATE_END_OF_LIST()
 }
 };
diff --git a/cpus.c b/cpus.c
index 7bbe153..dd7ac13 100644
--- a/cpus.c
+++ b/cpus.c
@@ -430,8 +430,7 @@ static const VMStateDescription vmstate_timers = {
 .name = "timer",
 .version_id = 2,
 .minimum_version_id = 1,
-.minimum_version_id_old = 1,
-.fields  = (VMStateField[]) {
+.fields = (VMStateField[]) {
 VMSTATE_INT64(cpu_ticks_offset, TimersState),
 VMSTATE_INT64(dummy, TimersState),
 VMSTATE_INT64_V(cpu_clock_offset, TimersState, 2),
diff --git a/docs/migration.txt b/docs/migration.txt
index fe1f2bb..0492a45 100644
--- a/docs/migration.txt
+++ b/docs/migration.txt
@@ -139,7 +139,7 @@ static const VMStateDescription vmstate_kbd = {
 .name = "pckbd",
 .version_id = 3,
 .minimum_version_id = 3,
-.fields  = (VMStateField []) {
+.fields = (VMStateField[]) {
 VMSTATE_UINT8(write_cmd, KBDState),
 VMSTATE_UINT8(status, KBDState),
 VMSTATE_UINT8(mode, KBDState),
@@ -257,7 +257,7 @@ const VMStateDescription vmstate_ide_drive_pio_state = {
 .minimum_version_id = 1,
 .pre_save = ide_drive_pio_pre_save,
 .post_load = ide_drive_pio_post_load,
-.fields  = (VMStateField []) {
+.fields = (VMStateField[]) {
 VMSTATE_INT32(req_nb_sectors, IDEState),
 VMSTATE_VARRAY_INT32(io_buffer, IDEState, io_buffer_total_len, 1,
  vmstate_info_uint8, uint8_t),
@@ -275,7 +275,7 @@ const VMStateDescription vmstate_ide_drive = {
 .version_id = 3,
 .minimum_version_id = 0,
 .post_load = ide_drive_post_load,
-.fields  = (VMStateField []) {
+.fields = (

[Qemu-devel] [PULL 6/6] usb: fix up post load checks

2014-05-14 Thread Juan Quintela
From: "Michael S. Tsirkin" 

Correct post load checks:
1. dev->setup_len == sizeof(dev->data_buf)
seems fine, no need to fail migration
2. When state is DATA, passing index > len
   will cause memcpy with negative length,
   resulting in heap overflow

First of the issues was reported by dgilbert.

Reported-by: "Dr. David Alan Gilbert" 
Signed-off-by: Michael S. Tsirkin 
Signed-off-by: Juan Quintela 
---
 hw/usb/bus.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/usb/bus.c b/hw/usb/bus.c
index 699aa10..927a47b 100644
--- a/hw/usb/bus.c
+++ b/hw/usb/bus.c
@@ -51,8 +51,8 @@ static int usb_device_post_load(void *opaque, int version_id)
 }
 if (dev->setup_index < 0 ||
 dev->setup_len < 0 ||
-dev->setup_index >= sizeof(dev->data_buf) ||
-dev->setup_len >= sizeof(dev->data_buf)) {
+dev->setup_index > dev->setup_len ||
+dev->setup_len > sizeof(dev->data_buf)) {
 return -EINVAL;
 }
 return 0;
-- 
1.9.0




[Qemu-devel] [PULL 3/6] savevm: Remove all the unneeded version_minimum_id_old (usb)

2014-05-14 Thread Juan Quintela
After previous Peter patch, they are redundant.  This way we don't
assign them except when needed.  Once there, there were lots of case
where the ".fields" indentation was wrong:

 .fields = (VMStateField []) {
and
 .fields =  (VMStateField []) {

Change all the combinations to:

 .fields = (VMStateField[]){

The biggest problem (appart from aesthetics) was that checkpatch complained
when we copy&pasted the code from one place to another.

Signed-off-by: Juan Quintela 
Acked-by: Gerd Hoffmann 
---
 hw/usb/bus.c | 2 +-
 hw/usb/dev-hid.c | 4 ++--
 hw/usb/dev-hub.c | 4 ++--
 hw/usb/dev-storage.c | 2 +-
 hw/usb/hcd-ehci-pci.c| 2 +-
 hw/usb/hcd-ehci-sysbus.c | 2 +-
 hw/usb/hcd-ehci.c| 2 +-
 hw/usb/hcd-uhci.c| 6 ++
 8 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/hw/usb/bus.c b/hw/usb/bus.c
index e48b19f..699aa10 100644
--- a/hw/usb/bus.c
+++ b/hw/usb/bus.c
@@ -63,7 +63,7 @@ const VMStateDescription vmstate_usb_device = {
 .version_id = 1,
 .minimum_version_id = 1,
 .post_load = usb_device_post_load,
-.fields = (VMStateField []) {
+.fields = (VMStateField[]) {
 VMSTATE_UINT8(addr, USBDevice),
 VMSTATE_INT32(state, USBDevice),
 VMSTATE_INT32(remote_wakeup, USBDevice),
diff --git a/hw/usb/dev-hid.c b/hw/usb/dev-hid.c
index f36e617..d097d93 100644
--- a/hw/usb/dev-hid.c
+++ b/hw/usb/dev-hid.c
@@ -622,7 +622,7 @@ static const VMStateDescription vmstate_usb_ptr = {
 .version_id = 1,
 .minimum_version_id = 1,
 .post_load = usb_ptr_post_load,
-.fields = (VMStateField []) {
+.fields = (VMStateField[]) {
 VMSTATE_USB_DEVICE(dev, USBHIDState),
 VMSTATE_HID_POINTER_DEVICE(hid, USBHIDState),
 VMSTATE_END_OF_LIST()
@@ -633,7 +633,7 @@ static const VMStateDescription vmstate_usb_kbd = {
 .name = "usb-kbd",
 .version_id = 1,
 .minimum_version_id = 1,
-.fields = (VMStateField []) {
+.fields = (VMStateField[]) {
 VMSTATE_USB_DEVICE(dev, USBHIDState),
 VMSTATE_HID_KEYBOARD_DEVICE(hid, USBHIDState),
 VMSTATE_END_OF_LIST()
diff --git a/hw/usb/dev-hub.c b/hw/usb/dev-hub.c
index bc03531..7492174 100644
--- a/hw/usb/dev-hub.c
+++ b/hw/usb/dev-hub.c
@@ -540,7 +540,7 @@ static const VMStateDescription vmstate_usb_hub_port = {
 .name = "usb-hub-port",
 .version_id = 1,
 .minimum_version_id = 1,
-.fields = (VMStateField []) {
+.fields = (VMStateField[]) {
 VMSTATE_UINT16(wPortStatus, USBHubPort),
 VMSTATE_UINT16(wPortChange, USBHubPort),
 VMSTATE_END_OF_LIST()
@@ -551,7 +551,7 @@ static const VMStateDescription vmstate_usb_hub = {
 .name = "usb-hub",
 .version_id = 1,
 .minimum_version_id = 1,
-.fields = (VMStateField []) {
+.fields = (VMStateField[]) {
 VMSTATE_USB_DEVICE(dev, USBHubState),
 VMSTATE_STRUCT_ARRAY(ports, USBHubState, NUM_PORTS, 0,
  vmstate_usb_hub_port, USBHubPort),
diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
index 2852669..e919100 100644
--- a/hw/usb/dev-storage.c
+++ b/hw/usb/dev-storage.c
@@ -716,7 +716,7 @@ static const VMStateDescription vmstate_usb_msd = {
 .name = "usb-storage",
 .version_id = 1,
 .minimum_version_id = 1,
-.fields = (VMStateField []) {
+.fields = (VMStateField[]) {
 VMSTATE_USB_DEVICE(dev, MSDState),
 VMSTATE_UINT32(mode, MSDState),
 VMSTATE_UINT32(scsi_len, MSDState),
diff --git a/hw/usb/hcd-ehci-pci.c b/hw/usb/hcd-ehci-pci.c
index 484a9bd..505741a 100644
--- a/hw/usb/hcd-ehci-pci.c
+++ b/hw/usb/hcd-ehci-pci.c
@@ -108,7 +108,7 @@ static const VMStateDescription vmstate_ehci_pci = {
 .name= "ehci",
 .version_id  = 2,
 .minimum_version_id  = 1,
-.fields  = (VMStateField[]) {
+.fields = (VMStateField[]) {
 VMSTATE_PCI_DEVICE(pcidev, EHCIPCIState),
 VMSTATE_STRUCT(ehci, EHCIPCIState, 2, vmstate_ehci, EHCIState),
 VMSTATE_END_OF_LIST()
diff --git a/hw/usb/hcd-ehci-sysbus.c b/hw/usb/hcd-ehci-sysbus.c
index fe6eea5..19ed2c2 100644
--- a/hw/usb/hcd-ehci-sysbus.c
+++ b/hw/usb/hcd-ehci-sysbus.c
@@ -21,7 +21,7 @@ static const VMStateDescription vmstate_ehci_sysbus = {
 .name= "ehci-sysbus",
 .version_id  = 2,
 .minimum_version_id  = 1,
-.fields  = (VMStateField[]) {
+.fields = (VMStateField[]) {
 VMSTATE_STRUCT(ehci, EHCISysBusState, 2, vmstate_ehci, EHCIState),
 VMSTATE_END_OF_LIST()
 }
diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index 355bbd6..a3ae9f2 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -2492,7 +2492,7 @@ const VMStateDescription vmstate_ehci = {
 .minimum_version_id  = 1,
 .pre_save= usb_ehci_pre_save,
 .post_load   = usb_ehci_post_load,
-.fields  = (VMStateField[]) {
+.fields = (VMStateField[]) {
 /* mmio registers */
 VMSTATE_UINT32(usbcmd, EHCISt

[Qemu-devel] [PULL 1/6] arch_init: Simplify code for load_xbzrle()

2014-05-14 Thread Juan Quintela
From: Chen Gang 

For xbzrle_decode_buffer(), when decoding contents will exceed writing
buffer, it will return -1, so need not check the return value whether
large than writing buffer.

And when failure occurs within load_xbzrle(), it always return -1
without any resources which need release.

So can remove the related checking statements, and also can remove 'rc'
and 'ret' local variables,

Signed-off-by: Chen Gang 
Signed-off-by: Juan Quintela 
---
 arch_init.c | 14 --
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/arch_init.c b/arch_init.c
index 995f56d..4e8f2c8 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -946,7 +946,6 @@ static uint64_t ram_save_pending(QEMUFile *f, void *opaque, 
uint64_t max_size)

 static int load_xbzrle(QEMUFile *f, ram_addr_t addr, void *host)
 {
-int ret, rc = 0;
 unsigned int xh_len;
 int xh_flags;

@@ -971,18 +970,13 @@ static int load_xbzrle(QEMUFile *f, ram_addr_t addr, void 
*host)
 qemu_get_buffer(f, xbzrle_decoded_buf, xh_len);

 /* decode RLE */
-ret = xbzrle_decode_buffer(xbzrle_decoded_buf, xh_len, host,
-   TARGET_PAGE_SIZE);
-if (ret == -1) {
+if (xbzrle_decode_buffer(xbzrle_decoded_buf, xh_len, host,
+ TARGET_PAGE_SIZE) == -1) {
 fprintf(stderr, "Failed to load XBZRLE page - decode error!\n");
-rc = -1;
-} else  if (ret > TARGET_PAGE_SIZE) {
-fprintf(stderr, "Failed to load XBZRLE page - size %d exceeds %d!\n",
-ret, TARGET_PAGE_SIZE);
-abort();
+return -1;
 }

-return rc;
+return 0;
 }

 static inline void *host_from_stream_offset(QEMUFile *f,
-- 
1.9.0




Re: [Qemu-devel] [PATCH v3 21/25] vmdk: implement .bdrv_detach/attach_aio_context()

2014-05-14 Thread Fam Zheng
On Thu, 05/08 16:34, Stefan Hajnoczi wrote:
> Implement .bdrv_detach/attach_aio_context() interfaces to propagate
> detach/attach to BDRVVmdkState->extents[].file.  The block layer takes
> care of ->file and ->backing_hd but doesn't know about our extents
> BlockDriverStates, which is also part of the graph.
> 
> Reviewed-by: Fam Zheng 
> Signed-off-by: Stefan Hajnoczi 
> ---

Noticed this doesn't apply any more due to recent VMDK fixes, because the
context is different. Trivial to fix, though.

>  block/vmdk.c | 23 +++
>  1 file changed, 23 insertions(+)
> 
> diff --git a/block/vmdk.c b/block/vmdk.c
> index 06a1f9f..1ca944a 100644
> --- a/block/vmdk.c
> +++ b/block/vmdk.c
> @@ -2063,6 +2063,27 @@ static ImageInfoSpecific 
> *vmdk_get_specific_info(BlockDriverState *bs)
>  return spec_info;
>  }
>  
> +static void vmdk_detach_aio_context(BlockDriverState *bs)
> +{
> +BDRVVmdkState *s = bs->opaque;
> +int i;
> +
> +for (i = 0; i < s->num_extents; i++) {
> +bdrv_detach_aio_context(s->extents[i].file);
> +}
> +}
> +
> +static void vmdk_attach_aio_context(BlockDriverState *bs,
> +AioContext *new_context)
> +{
> +BDRVVmdkState *s = bs->opaque;
> +int i;
> +
> +for (i = 0; i < s->num_extents; i++) {
> +bdrv_attach_aio_context(s->extents[i].file, new_context);
> +}
> +}
> +
>  static QEMUOptionParameter vmdk_create_options[] = {
>  {
>  .name = BLOCK_OPT_SIZE,
> @@ -2118,6 +2139,8 @@ static BlockDriver bdrv_vmdk = {
>  .bdrv_has_zero_init   = vmdk_has_zero_init,
>  .bdrv_get_specific_info   = vmdk_get_specific_info,
>  .bdrv_refresh_limits  = vmdk_refresh_limits,
> +.bdrv_detach_aio_context  = vmdk_detach_aio_context,
> +.bdrv_attach_aio_context  = vmdk_attach_aio_context,
>  
>  .create_options   = vmdk_create_options,
>  };
> -- 
> 1.9.0
> 
> 



Re: [Qemu-devel] [PATCH] SMI handler should set the CPL to zero and save and restore it on rsm.

2014-05-14 Thread Kevin O'Connor
On Wed, May 14, 2014 at 08:20:59PM -0400, Kevin O'Connor wrote:
> On Wed, May 14, 2014 at 10:05:47AM +0200, Paolo Bonzini wrote:
> > CPL isn't even altered when CS is reloaded, because you cannot jump out
> > of ring-0 except with an inter-privilege IRET, and that reloads SS too.
> > 
> > An IRET or task switch is also the only way to set EFLAGS.VM, and it will
> > hardcode SS.DPL=3, again matching CPL=3.
> > 
> > Finally, to get out of real mode you need to have CPL=0, and whatever got
> > you at CPL has also loaded SS with a ring-0 stack.  This means that SS.DPL=0
> > right after clearing CR0.PE.
> > 
> > Using SS.DPL as the CPL really sounds like the right approach.  I 
> > tried it on my KVM testcase, and it works well.  For QEMU, even the
> > special case of SYSRET will be handled fine because QEMU does set 
> > SS.DPL = 3:
> > 
> > cpu_x86_load_seg_cache(env, R_SS, selector + 8,
> >0, 0x,
> >DESC_G_MASK | DESC_B_MASK | DESC_P_MASK |
> >DESC_S_MASK | (3 << DESC_DPL_SHIFT) |
> >DESC_W_MASK | DESC_A_MASK);
> > 
> > SS.DPL=CPL=3, SS.RPL=selector & 3 is a mix of Intel behavior (which is 
> > SS.DPL=SS.RPL=CPL=3) and AMD behavior (because they set CPL=3 but 
> > SS.DPL=SS.RPL=selector & 3).  We may want to match Intel behavior,
> > but that's a different change.
> > 
> > Can you check if this patch works for you, and if so reply with
> > Tested-by/Reviewed-by?
> 
> Your patch causes Freedos to crash when emm386 is loaded, so I think
> it broke VM86 mode.  Below are some logs I took from qemu at the point
> of the crash.

FYI, with the patch below my quick test cases all look okay.

-Kevin


--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -974,7 +974,6 @@ static inline void cpu_x86_load_seg_cache(CPUX86State *env,
 /* update the hidden flags */
 {
 if (seg_reg == R_CS) {
-int cpl = selector & 3;
 #ifdef TARGET_X86_64
 if ((env->hflags & HF_LMA_MASK) && (flags & DESC_L_MASK)) {
 /* long mode */
@@ -984,16 +983,17 @@ static inline void cpu_x86_load_seg_cache(CPUX86State 
*env,
 #endif
 {
 /* legacy / compatibility case */
-if (!(env->cr[0] & CR0_PE_MASK)) {
-cpl = 0;
-} else if (env->eflags & VM_MASK) {
-cpl = 3;
-}
 new_hflags = (env->segs[R_CS].flags & DESC_B_MASK)
 >> (DESC_B_SHIFT - HF_CS32_SHIFT);
 env->hflags = (env->hflags & ~(HF_CS32_MASK | HF_CS64_MASK)) |
 new_hflags;
 }
+}
+if (seg_reg == R_SS) {
+int cpl = (flags >> DESC_DPL_SHIFT) & 3;
+if (env->eflags & VM_MASK) {
+cpl = 3;
+}
 #if HF_CPL_MASK != 3
 #error HF_CPL_MASK is hardcoded
 #endif



[Qemu-devel] [PATCH] MAINTAINERS: update Calxeda Highbank maintainer and status

2014-05-14 Thread Rob Herring
From: Rob Herring 

Signed-off-by: Rob Herring 
---
Mark, I guessing you don't want to stay as maintainer?

 MAINTAINERS | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 97c9fa1..5ad7dcc 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -243,8 +243,8 @@ S: Maintained
 F: hw/*/exynos*
 
 Calxeda Highbank
-M: Mark Langsdorf 
-S: Supported
+M: Rob Herring 
+S: Maintained
 F: hw/arm/highbank.c
 F: hw/net/xgmac.c
 
-- 
1.9.1




Re: [Qemu-devel] [PATCH] rdma: Fix block during rdma migration

2014-05-14 Thread Michael R. Hines

On 05/09/2014 12:25 PM, Gonglei (Arei) wrote:

Hi,


-Original Message-
From: Michael R. Hines [mailto:mrhi...@linux.vnet.ibm.com]
Sent: Tuesday, April 01, 2014 8:42 AM
To: Gonglei (Arei); qemu-devel@nongnu.org
Cc: Huangweidong (C); quint...@redhat.com; dgilb...@redhat.com;
owass...@redhat.com; mrhi...@us.ibm.com; Moyuxiang;
pbonz...@redhat.com
Subject: Re: [Qemu-devel] [PATCH] rdma: Fix block during rdma migration

On 03/29/2014 03:39 PM, arei.gong...@huawei.com wrote:

From: Mo Yuxiang 

If the networking break or there's something wrong with rdma
device(ib0 with no IP) during rdma migration, the main_loop of
qemu will be blocked in rdma_destroy_id. I add rdma_ack_cm_event
to fix this bug.

Signed-off-by: Mo Yuxiang 
Signed-off-by: Gonglei 
---
   migration-rdma.c | 1 +
   1 file changed, 1 insertion(+)

diff --git a/migration-rdma.c b/migration-rdma.c
index eeb4302..f60749b 100644
--- a/migration-rdma.c
+++ b/migration-rdma.c
@@ -949,6 +949,7 @@ route:
   ERROR(errp, "result not equal to event_addr_resolved %s",
   rdma_event_str(cm_event->event));
   perror("rdma_resolve_addr");
+rdma_ack_cm_event(cm_event);
   ret = -EINVAL;
   goto err_resolve_get_addr;
   }

Reviewed-by: Michael R. Hines 

Good catch. =) That's an obvious bug. It looks like I need
to do a much better job of "kill -9" inside the regression
testing scripts - probably i should try killing the migration
prematurely at different periods just to be sure there are
no more places where the connection state is not getting
cleaned up..

- Michael


Michael, do you have a plan to pull this patch to master? Thanks.

Best regards,
-Gonglei



Sorry for the late reply, but I'm not the maintainer for migration, 
that's Juan

(I can only signoff on patches like everyone else =).

I also have outstanding RDMA patches myself that have not yet been pulled.

Would you mind pinging Juan for both of us?

- Michael

- Michael




Re: [Qemu-devel] [PATCH v4 2/4] util/fifo: Generalise for common integer widths

2014-05-14 Thread Peter Crosthwaite
On Tue, May 6, 2014 at 11:41 AM, Peter Crosthwaite
 wrote:
> On Tue, Apr 29, 2014 at 2:57 AM, Peter Maydell  
> wrote:
>> On 15 April 2014 04:18, Peter Crosthwaite  
>> wrote:
>>> Add support for 16, 32 and 64 bit width FIFOs. The push and pop
>>> functions are replicated to accept all four different integer types.
>>> The element width of the FIFO is set at creation time.
>>>
>>> The backing storage for all element types is still uint8_t regardless of
>>> element width so some save-load logic is needed to handle endianness
>>> issue WRT VMSD.
>>
>>> +/* Always store buffer data in little (arbitrarily chosen) endian format to
>>> + * allow for migration to/from BE <-> LE hosts.
>>> + */
>>> +
>>> +static inline void fifo_save_load_swap(Fifo *fifo)
>>> +{
>>> +#ifdef HOST_WORDS_BIGENDIAN
>>> +int i;
>>> +uint16_t *d16 = (uint16_t *)fifo->data;
>>> +uint32_t *d32 = (uint32_t *)fifo->data;
>>> +uint64_t *d64 = (uint64_t *)fifo->data;
>>> +
>>> +for (i = 0; i < fifo->capacity; ++i) {
>>> +switch (fifo->width) {
>>> +case 1:
>>> +return;
>>> +case 2:
>>> +d16[i] = bswap16(d16[i]);
>>> +break;
>>> +case 4:
>>> +d32[i] = bswap32(d32[i]);
>>> +break;
>>> +case 8:
>>> +d64[i] = bswap64(d64[i]);
>>> +break;
>>> +default:
>>> +abort();
>>> +}
>>> +}
>>> +#endif
>>> +}
>>> +
>>> +static void fifo_pre_save(void *opaque)
>>> +{
>>> +fifo_save_load_swap((Fifo *)opaque);
>>> +}
>>> +
>>> +static int fifo_post_load(void *opaque, int version_id)
>>> +{
>>> +fifo_save_load_swap((Fifo *)opaque);
>>> +return 0;
>>> +}
>>> +
>>>  const VMStateDescription vmstate_fifo = {
>>>  .name = "Fifo8",
>>>  .version_id = 1,
>>>  .minimum_version_id = 1,
>>>  .minimum_version_id_old = 1,
>>> +.pre_save = fifo_pre_save,
>>> +.post_load = fifo_post_load,
>>>  .fields  = (VMStateField[]) {
>>> -VMSTATE_VBUFFER_UINT32(data, Fifo, 1, NULL, 0, capacity),
>>> +VMSTATE_VBUFFER_UINT32(data, Fifo, 1, NULL, 0, buffer_size),
>>>  VMSTATE_UINT32(head, Fifo),
>>>  VMSTATE_UINT32(num, Fifo),
>>>  VMSTATE_END_OF_LIST()
>>
>> This doesn't look right -- when the VM continues after
>> a save on a bigendian system all its FIFO data will
>> have been byteswapped and it'll fall over.
>>
>
> Yep. My bad.
>
>> I think you need to implement this by adding get/put
>> functions which byteswap as they put the data onto
>> or take it off the wire. Check the use and definition
>> of vmstate_info_bitmap for an example of handling a
>> data structure where the on-the-wire and in-memory
>> formats differ.
>>
>
> So I am starting to think there a better way. Ultimately I want this
> API to work for random structs too, not just integer types (E.G. PL330
> would be a nice client). So I'm thinking this problem is outsourced to
> the client who is responsible from bringing a VMStateInfo to the table
> (input to fifo_create).

Ok so this is much trickier than I thought (+Juan). Opaquifying the
VMState info (and I use the term loosely there) is rather tricky as
sometimes the info is described via a VMStateInfo (usually for the
simpler types such sm_state_info_uint8), but for structs you use a
VMStateDescription. You therefore cant wrap these oqauely as you dont
know which of VMStateInfo and VMStateDescription you want to use until
you have a specific case.

I guess you could overcome this by merging VMStateInfo and
VMStateDescription. I think this may be a good idea. AFAICT, the
difference between the two, is that VMStateInfo (VMSI) is code driven
and forms the leaves of the VMSD tree, while VMStateDescription is
data driven and describes the tree branches (and trunk). What's
irregular here is that the "branch" (VMSD) and "leaf" (VMSI) types are
different and the branches need to be aware of that. E.G. VMSD needs
to know whether its children (i.e. the .fields) are VMSD's or VMSI's,
leading to significant API replication. E.G. VMSTATE_VARRAY_UINT32 and
VMSTATE_STRUCT_VARRAY_UINT32 are the same thing, except they describe
element types with VMSI and VMSD resp. Only diff is "my children are
leaves" or "my children are branches" which generally something you
want to abstract away from a recursion mechanism.

So this whole system could be made simpler, if what little
functionality VMSI implements (just some code driven hooks) was rolled
into VMSD and everyone used VMSD. Then all these duped APIs could be
consilidated.

And I could solve my opaque VMSD problem :)

Thoughts?

Regards,
Peter

 We then have some some wrappers for the simple
> integer types that trivally take the global vmstate_info_uintxx
> structs as appropriate:
>
>  void fifo_create8(Fifo *fifo, uint32_t capacity)
>  {
>  fifo_create(fifo, capacity, sizeof(uint8_t), vmstate_info_uint8);
>  }
>
> Most users will just just user createxx for an int fifo. But if

Re: [Qemu-devel] [PATCH v2] qcow2: Patch for shrinking qcow2 disk image

2014-05-14 Thread Max Reitz

On 09.05.2014 17:20, Jun Li wrote:

As the realization of raw shrinking, so when do qcow2 shrinking, do not


*when doing


check l1 entries. When resize to size1(size1 < "disk size"), the custemer


*customer


knows this will destory the data. So no need to check the l1 entries which
is used or not.


I'd somehow like to disagree, but you're correct. raw-posix truncates 
the file regardless of whether there is data or not as well. Maybe we 
should later add support for reporting potential data loss and asking 
the user what to do (I'm thinking of some "force" or "accept_loss" 
boolean for bdrv_truncate()).



This is v2 of the original patch. thx.


This should not be part of the commit message, but rather follow the 
"---" under your Signed-off-by.



Signed-off-by: Jun Li 
---
  block/qcow2-cluster.c  | 17 -
  block/qcow2-snapshot.c |  2 +-
  block/qcow2.c  | 10 ++
  block/qcow2.h  |  4 ++--
  4 files changed, 17 insertions(+), 16 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 76d2bcf..8fbbf7f 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -29,8 +29,8 @@
  #include "block/qcow2.h"
  #include "trace.h"
  
-int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size,

-bool exact_size)
+int qcow2_truncate_l1_table(BlockDriverState *bs, uint64_t min_size,
+bool exact_size)
  {
  BDRVQcowState *s = bs->opaque;
  int new_l1_size2, ret, i;
@@ -39,8 +39,9 @@ int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t 
min_size,
  int64_t new_l1_table_offset, new_l1_size;
  uint8_t data[12];
  
-if (min_size <= s->l1_size)

+if (min_size == s->l1_size) {
  return 0;
+}
  
  /* Do a sanity check on min_size before trying to calculate new_l1_size

   * (this prevents overflows during the while loop for the calculation of
@@ -73,7 +74,13 @@ int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t 
min_size,
  
  new_l1_size2 = sizeof(uint64_t) * new_l1_size;

  new_l1_table = g_malloc0(align_offset(new_l1_size2, 512));
-memcpy(new_l1_table, s->l1_table, s->l1_size * sizeof(uint64_t));
+
+/* shrinking or growing l1 table */
+if (min_size < s->l1_size) {
+memcpy(new_l1_table, s->l1_table, new_l1_size2);
+} else {
+memcpy(new_l1_table, s->l1_table, s->l1_size * sizeof(uint64_t));
+}
  
  /* write new table (align to cluster) */

  BLKDBG_EVENT(bs->file, BLKDBG_L1_GROW_ALLOC_TABLE);


So, as far as I understand the new logic of qcow2_truncate_l1_table(), 
it will always grow the table unless exact_size == true, in which case 
it may be shrunk as well. This should probably be reflected in the if 
condition ("if (exact_size ? min_size == s->l1_size : min_size <= 
s->l1_size)" or something like that). But on the other hand, I don't 
like a function which suggests being usable for both shrinking and 
growing, which then can be used for shrinking only in a special case 
(exact_size == true). You should at least add a comment which states 
that this function is generally intended for growing the L1 table with 
min_size being the new minimal size, but may also be used for shrinking 
if exact_size is true.


Apart from that, if you're shrinking the L1 table, you should in my 
opinion free all clusters referenced from the truncated area. It is true 
that it's the user's responsibility to make sure no data is lost, but if 
you just shrink the L1 table without doing anything about the lost data 
references, clusters will be leaked. This can easily be fixed by 
qemu-img check, but there are two problems with that: First, data should 
be leaked only if it cannot be avoided. It can easily be repaired, but 
unless there are errors during some operation, that should not be 
necessary. Second, qemu-img check actually does not work for all image 
sizes that qemu itself supports. This is even more reason to avoid 
leaks: Normally, it can easily be repaired, but sometimes, it cannot.



@@ -558,7 +565,7 @@ static int get_cluster_table(BlockDriverState *bs, uint64_t 
offset,
  
  l1_index = offset >> (s->l2_bits + s->cluster_bits);

  if (l1_index >= s->l1_size) {
-ret = qcow2_grow_l1_table(bs, l1_index + 1, false);
+ret = qcow2_truncate_l1_table(bs, l1_index + 1, false);
  if (ret < 0) {
  return ret;
  }
diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
index 0aa9def..6ba460e 100644
--- a/block/qcow2-snapshot.c
+++ b/block/qcow2-snapshot.c
@@ -483,7 +483,7 @@ int qcow2_snapshot_goto(BlockDriverState *bs, const char 
*snapshot_id)
   * L1 table of the snapshot. If the snapshot L1 table is smaller, the
   * current one must be padded with zeros.
   */
-ret = qcow2_grow_l1_table(bs, sn->l1_size, true);
+ret = qcow2_truncate_l1_table(bs, sn->l1_size, true);
  if (ret < 0) {
  goto fail;
  }
diff --git a/block/

Re: [Qemu-devel] [PATCH] SMI handler should set the CPL to zero and save and restore it on rsm.

2014-05-14 Thread Kevin O'Connor
On Wed, May 14, 2014 at 10:05:47AM +0200, Paolo Bonzini wrote:
> CPL isn't even altered when CS is reloaded, because you cannot jump out
> of ring-0 except with an inter-privilege IRET, and that reloads SS too.
> 
> An IRET or task switch is also the only way to set EFLAGS.VM, and it will
> hardcode SS.DPL=3, again matching CPL=3.
> 
> Finally, to get out of real mode you need to have CPL=0, and whatever got
> you at CPL has also loaded SS with a ring-0 stack.  This means that SS.DPL=0
> right after clearing CR0.PE.
> 
> Using SS.DPL as the CPL really sounds like the right approach.  I 
> tried it on my KVM testcase, and it works well.  For QEMU, even the
> special case of SYSRET will be handled fine because QEMU does set 
> SS.DPL = 3:
> 
> cpu_x86_load_seg_cache(env, R_SS, selector + 8,
>0, 0x,
>DESC_G_MASK | DESC_B_MASK | DESC_P_MASK |
>DESC_S_MASK | (3 << DESC_DPL_SHIFT) |
>DESC_W_MASK | DESC_A_MASK);
> 
> SS.DPL=CPL=3, SS.RPL=selector & 3 is a mix of Intel behavior (which is 
> SS.DPL=SS.RPL=CPL=3) and AMD behavior (because they set CPL=3 but 
> SS.DPL=SS.RPL=selector & 3).  We may want to match Intel behavior,
> but that's a different change.
> 
> Can you check if this patch works for you, and if so reply with
> Tested-by/Reviewed-by?

Your patch causes Freedos to crash when emm386 is loaded, so I think
it broke VM86 mode.  Below are some logs I took from qemu at the point
of the crash.

-Kevin


 Freedos with unmodified QEMU 2.0.0 (and SeaBIOS master)


IN: 
0x0012276e:  mov%cr0,%eax
0x00122771:  or $0x8000,%eax
0x00122777:  mov%eax,%cr0

EAX=00125000 EBX= ECX= EDX=
ESI= EDI= EBP= ESP=01dc
EIP=06fe EFL=0002 [---] CPL=0 II=0 A20=1 SMM=0 HLT=0
ES =0014 3a60 0b8c 9300 DPL=0 DS16 [-WA]
CS =000c 00122070 2713 9a00 DPL=0 CS16 [-R-]
SS =0038 3820 0200 9200 DPL=0 DS16 [-W-]
DS =0014 3a60 0b8c 9300 DPL=0 DS16 [-WA]
FS =0014 3a60 0b8c 9300 DPL=0 DS16 [-WA]
GS =0014 3a60 0b8c 9300 DPL=0 DS16 [-WA]
LDT=0008 3d64 0020 8200 DPL=0 LDT
TR =0010 0011 2069 8900 DPL=0 TSS32-avl
GDT= 3ce4 007f
IDT= 00124784 07ff
CR0=0011 CR2= CR3=00125000 CR4=
DR0= DR1= DR2= 
DR3= 
DR6=4ff0 DR7=0400
CCS= CCD=00126360 CCO=EFLAGS  
EFER=

IN: 
0x0012277a:  iretl  

EAX=8011 EBX= ECX= EDX=
ESI= EDI= EBP= ESP=01dc
EIP=070a EFL=0086 [--S--P-] CPL=0 II=0 A20=1 SMM=0 HLT=0
ES =0014 3a60 0b8c 9300 DPL=0 DS16 [-WA]
CS =000c 00122070 2713 9a00 DPL=0 CS16 [-R-]
SS =0038 3820 0200 9200 DPL=0 DS16 [-W-]
DS =0014 3a60 0b8c 9300 DPL=0 DS16 [-WA]
FS =0014 3a60 0b8c 9300 DPL=0 DS16 [-WA]
GS =0014 3a60 0b8c 9300 DPL=0 DS16 [-WA]
LDT=0008 3d64 0020 8200 DPL=0 LDT
TR =0010 0011 2069 8900 DPL=0 TSS32-avl
GDT= 3ce4 007f
IDT= 00124784 07ff
CR0=8011 CR2= CR3=00125000 CR4=
DR0= DR1= DR2= 
DR3= 
DR6=4ff0 DR7=0400
CCS= CCD=8011 CCO=LOGICL  
EFER=
Servicing hardware INT=0x08
 0: v=08 e= i=0 cpl=3 IP=0930:01c9 pc=94c9 
SP=0920:0100 env->regs[R_EAX]=8011
EAX=8011 EBX= ECX= EDX=
ESI= EDI= EBP= ESP=0100
EIP=01c9 EFL=00023202 [---] CPL=3 II=0 A20=1 SMM=0 HLT=0
ES =   
CS =0930 9300  
SS =0920 9200  
DS =03a6 3a60  
FS =   
GS =   
LDT=0008 3d64 0020 8200 DPL=0 LDT
TR =0010 0011 2069 8900 DPL=0 TSS32-avl
GDT= 3ce4 007f
IDT= 00124784 07ff
CR0=8011 CR2= CR3=00125000 CR4=
DR0= DR1= DR2= 
DR3= 
DR6=4ff0 DR7=0400
CCS= CCD=8011 CCO=EFLAGS  
EFER=

IN: 
0x0012279c:  call   0x122070


 Freedos with patched QEMU (and SeaBIOS master)


IN: 
0x0012276e:  mov%cr0,%eax
0x00122771:  or $0x8000,%eax
0x00122777:  mov%eax,%cr0

EAX=00125000 EBX= ECX= EDX=
ESI= EDI= EBP= ESP=01dc
EIP=06fe EFL=0002 [---] CPL=0 II=0 A20=1 SMM=0 HLT=0
ES =0014 3a60 0b8c 9300 DPL=0 D

Re: [Qemu-devel] [PATCH 5/7] target-arm: add emulation of PSCI calls for system emulation

2014-05-14 Thread Rob Herring
On Wed, May 14, 2014 at 1:12 PM, Peter Maydell  wrote:
> On 5 May 2014 17:00, Rob Herring  wrote:
>> From: Rob Herring 
>>
>> Add support for handling PSCI calls in system emulation. Both version
>> 0.1 and 0.2 of the PSCI spec are supported. Platforms can enable support
>> by setting "psci-method" QOM property on the cpus to SMC or HVC
>> emulation and having PSCI binding in their dtb.

[...]

>> +/* Initialize the cpu we are turning on */
>> +cpu_reset(cs);
>> +arm_cpu_set_pc(cs, entry);
>> +cpu->powered_off = false;
>> +cs->interrupt_request |= CPU_INTERRUPT_EXITTB;
>> +
>> +/* Set the context_id in r0/x0 */
>> +if (is_a64(env)) {
>> +cpu->env.xregs[0] = context_id;
>> +} else {
>> +cpu->env.regs[0] = context_id;
>> +}
>
> If the calling CPU is in AArch32 then we're going
> to restart the target CPU in AArch64 but with R0
> set rather than X0. That doesn't seem right...

Probably just one on many issues to support A32 EL1... Since the core
is being reset and register state is undefined, I can just do:

cpu->env.xregs[0] = cpu->env.regs[0] = context_id;

EL2/3 support will then further complicate things (or just remove all
this code).

>> +
>> +ret = 0;
>> +break;
>> +case QEMU_PSCI_FN_CPU_OFF:
>> +case PSCI_0_2_FN_CPU_OFF:
>> +cpu->powered_off = true;
>> +cs->exit_request = 1;
>
> I need to check up on whether setting exit_request here
> is right, but I don't have time just this instant. More
> later :-)

IIRC, things did not work without it.

Rob



Re: [Qemu-devel] [PATCH v2] qemu-img fails to delete last snapshot

2014-05-14 Thread Max Reitz

On 12.05.2014 22:27, Mike Day wrote:

When deleting the last snapshot, copying the resulting snapshot table
currently fails, causing the delete operation to also fail. Fix the
failure by skipping the copy and just writing the snapshot header and
freeing the extra clusters.

There are two specific problems in the current code. First is a lack of
parenthesis in the calculation of the memmove size parameter:

s->nb_snapshots - snapshot_index - 1

When s->nb_snapshots is 0, snapshot_index is 1.


Before this patch is applied, s->nb_snapshots is only increased after 
the memmove(). Therefore, it can never be 0 – snapshot_index on the 
other hand needs to be 0, as find_snapshot_by_id_and_name() forces it to 
be less than s->nb_snapshots (to elaborate on Kevin's review).



0 - 1 - 1 = 0xfffe

it should be:

0 - (1 - 1) = 0x00

The second problem is shifting the snapshot table to the left. After
removing the last snapshot there are no existing snapshots to be
shifted. All that needs to be done is to write the header and
unallocate the blocks.

Signed-off-by: Mike Day 
Reviewed-by: Eric Blake 
---
v2: improved the git log entry
 added Eric Blake as a reviewer


I do agree that this code is rather ugly and I had problems with it on 
more than one occasion (which should be speaking for itself, considering 
that I have not worked that long on qemu). On the other hand it is 
always a nice test case whether one broke zero-size allocations, though 
(while I'm not sure whether these should be allowed in the first place, 
though…).


Considering that this code indeed does perform a zero-size allocation 
reproducably, I'm rather surprised that we actually do not have a test 
case yet for snapshot deletion, though (as far as I can see).


So, all in all, I am kind of in favor of making the deletion of the last 
snapshot a special case as this would probably greatly improve 
readability; but on the other hand, it actually is a good test as it is 
right now.


Max



Re: [Qemu-devel] [PATCH] iotests: Use configured python

2014-05-14 Thread Max Reitz

On 14.05.2014 14:33, Markus Armbruster wrote:

Max Reitz  writes:


Currently, QEMU's iotests rely on /usr/bin/env to start the correct
Python (that is, at least Python 2.4, but not 3). On systems where
Python 3 is the default, the user has no clean way of making the iotests
use the correct binary.

This commit makes the iotests use the Python selected by configure.

Signed-off-by: Max Reitz 

I'm afraid this broke qemu-iotests in a separate build tree:

 ./check: line 38: ./common.env: No such file or directory


As I already replied to Peter, I see two (or maybe three) ways to fix this:

The first is, we use the correct path to common.env. This would however 
result in modification of the source tree although this is probably not 
what the user intends with an out-of-tree build. On the other hand, this 
would just work.


The second is, we do not create common.env for out-of-tree builds and 
add a default common.env to the repository ("PYTHON = python" should 
probably suffice). This would not introduce any regressions, however, 
the iotests would remain problematic on systems with Python 3 being the 
default when using out-of-tree builds. As I guess that out-of-tree 
builds are actually recommended, this would mean that the better 
solution might be to revert my patch and instead change every "python" 
occurrence in the iotests' Shebangs to "python2", as kind of a third way 
to go. However, for this I'm not sure whether all systems which are 
supposed to be supported by qemu actually have a "python2" 
executable/symlink. I guess so, but then again...


So, either we fix it but try to write to the source tree without the 
user intending to modify it; or we fix it for in-tree builds only; or we 
drop the magic and just use "python2" in the iotests' Shebangs.


Max



[Qemu-devel] [PATCH 2/4] curl: Remove broken parsing of options from url

2014-05-14 Thread Matthew Booth
The block layer now supports a generic json syntax for passing option parameters
explicitly, making parsing of options from the url redundant.

Signed-off-by: Matthew Booth 
---
 block/curl.c | 52 ++--
 1 file changed, 10 insertions(+), 42 deletions(-)

diff --git a/block/curl.c b/block/curl.c
index f3c797a..1b9f2f2 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -61,12 +61,15 @@ static CURLMcode __curl_multi_socket_action(CURLM 
*multi_handle,
 #define CURL_NUM_STATES 8
 #define CURL_NUM_ACB8
 #define SECTOR_SIZE 512
-#define READ_AHEAD_SIZE (256 * 1024)
+#define READ_AHEAD_DEFAULT (256 * 1024)
 
 #define FIND_RET_NONE   0
 #define FIND_RET_OK 1
 #define FIND_RET_WAIT   2
 
+#define CURL_BLOCK_OPT_URL   "url"
+#define CURL_BLOCK_OPT_READAHEAD "readahead"
+
 struct BDRVCURLState;
 
 typedef struct CURLAIOCB {
@@ -411,43 +414,7 @@ static void curl_clean_state(CURLState *s)
 static void curl_parse_filename(const char *filename, QDict *options,
 Error **errp)
 {
-
-#define RA_OPTSTR ":readahead="
-char *file;
-char *ra;
-const char *ra_val;
-int parse_state = 0;
-
-file = g_strdup(filename);
-
-/* Parse a trailing ":readahead=#:" param, if present. */
-ra = file + strlen(file) - 1;
-while (ra >= file) {
-if (parse_state == 0) {
-if (*ra == ':') {
-parse_state++;
-} else {
-break;
-}
-} else if (parse_state == 1) {
-if (*ra > '9' || *ra < '0') {
-char *opt_start = ra - strlen(RA_OPTSTR) + 1;
-if (opt_start > file &&
-strncmp(opt_start, RA_OPTSTR, strlen(RA_OPTSTR)) == 0) {
-ra_val = ra + 1;
-ra -= strlen(RA_OPTSTR) - 1;
-*ra = '\0';
-qdict_put(options, "readahead", qstring_from_str(ra_val));
-}
-break;
-}
-}
-ra--;
-}
-
-qdict_put(options, "url", qstring_from_str(file));
-
-g_free(file);
+qdict_put(options, CURL_BLOCK_OPT_URL, qstring_from_str(filename));
 }
 
 static QemuOptsList runtime_opts = {
@@ -455,12 +422,12 @@ static QemuOptsList runtime_opts = {
 .head = QTAILQ_HEAD_INITIALIZER(runtime_opts.head),
 .desc = {
 {
-.name = "url",
+.name = CURL_BLOCK_OPT_URL,
 .type = QEMU_OPT_STRING,
 .help = "URL to open",
 },
 {
-.name = "readahead",
+.name = CURL_BLOCK_OPT_READAHEAD,
 .type = QEMU_OPT_SIZE,
 .help = "Readahead size",
 },
@@ -492,14 +459,15 @@ static int curl_open(BlockDriverState *bs, QDict 
*options, int flags,
 goto out_noclean;
 }
 
-s->readahead_size = qemu_opt_get_size(opts, "readahead", READ_AHEAD_SIZE);
+s->readahead_size = qemu_opt_get_size(opts, CURL_BLOCK_OPT_READAHEAD,
+  READ_AHEAD_DEFAULT);
 if ((s->readahead_size & 0x1ff) != 0) {
 error_setg(errp, "HTTP_READAHEAD_SIZE %zd is not a multiple of 512",
s->readahead_size);
 goto out_noclean;
 }
 
-file = qemu_opt_get(opts, "url");
+file = qemu_opt_get(opts, CURL_BLOCK_OPT_URL);
 if (file == NULL) {
 error_setg(errp, "curl block driver requires an 'url' option");
 goto out_noclean;
-- 
1.9.0




[Qemu-devel] [PATCH 3/4] curl: Add sslverify option

2014-05-14 Thread Matthew Booth
This allows qemu to use images over https with a self-signed certificate. It
defaults to verifying the certificate.

Signed-off-by: Matthew Booth 
---
 block/curl.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/block/curl.c b/block/curl.c
index 1b9f2f2..43d6646 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -23,6 +23,7 @@
  */
 #include "qemu-common.h"
 #include "block/block_int.h"
+#include "qapi/qmp/qbool.h"
 #include 
 
 // #define DEBUG
@@ -69,6 +70,7 @@ static CURLMcode __curl_multi_socket_action(CURLM 
*multi_handle,
 
 #define CURL_BLOCK_OPT_URL   "url"
 #define CURL_BLOCK_OPT_READAHEAD "readahead"
+#define CURL_BLOCK_OPT_SSLVERIFY "sslverify"
 
 struct BDRVCURLState;
 
@@ -106,6 +108,7 @@ typedef struct BDRVCURLState {
 CURLState states[CURL_NUM_STATES];
 char *url;
 size_t readahead_size;
+bool sslverify;
 bool accept_range;
 } BDRVCURLState;
 
@@ -372,6 +375,7 @@ static CURLState *curl_init_state(BDRVCURLState *s)
 return NULL;
 }
 curl_easy_setopt(state->curl, CURLOPT_URL, s->url);
+curl_easy_setopt(state->curl, CURLOPT_SSL_VERIFYPEER, s->sslverify);
 curl_easy_setopt(state->curl, CURLOPT_TIMEOUT, 5);
 curl_easy_setopt(state->curl, CURLOPT_WRITEFUNCTION,
  (void *)curl_read_cb);
@@ -431,6 +435,11 @@ static QemuOptsList runtime_opts = {
 .type = QEMU_OPT_SIZE,
 .help = "Readahead size",
 },
+{
+.name = CURL_BLOCK_OPT_SSLVERIFY,
+.type = QEMU_OPT_BOOL,
+.help = "Verify SSL certificate"
+},
 { /* end of list */ }
 },
 };
@@ -467,6 +476,8 @@ static int curl_open(BlockDriverState *bs, QDict *options, 
int flags,
 goto out_noclean;
 }
 
+s->sslverify = qemu_opt_get_bool(opts, CURL_BLOCK_OPT_SSLVERIFY, true);
+
 file = qemu_opt_get(opts, CURL_BLOCK_OPT_URL);
 if (file == NULL) {
 error_setg(errp, "curl block driver requires an 'url' option");
-- 
1.9.0




[Qemu-devel] [PATCH 4/4] curl: Add usage documentation

2014-05-14 Thread Matthew Booth
Signed-off-by: Matthew Booth 
---
 qemu-options.hx | 68 +
 1 file changed, 68 insertions(+)

diff --git a/qemu-options.hx b/qemu-options.hx
index 781af14..7587bce 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2191,6 +2191,74 @@ qemu-system-x86_64 --drive 
file=gluster://192.0.2.1/testvol/a.img
 @end example
 
 See also @url{http://www.gluster.org}.
+
+@item HTTP/HTTPS/FTP/FTPS/TFTP
+QEMU supports read-only access to files accessed over http(s), ftp(s) and tftp.
+
+Syntax using a single filename:
+@example
+://[[:]@@]/
+@end example
+
+where:
+@table @option
+@item protocol
+'http', 'https', 'ftp', 'ftps', or 'tftp'.
+
+@item username
+Optional username for authentication to the remote server.
+
+@item password
+Optional password for authentication to the remote server.
+
+@item host
+Address of the remote server.
+
+@item path
+Path on the remote server, including any query string.
+@end table
+
+The following options are also supported:
+@table @option
+@item url
+The full URL when passing options to the driver explicitly.
+
+@item readahead
+The amount of data to read ahead with each range request to the remote server.
+This value may optionally have the suffix 'T', 'G', 'M', 'K', 'k' or 'b'. If it
+does not have a suffix, it will be assumed to be in bytes. The value must be a
+multiple of 512 bytes. It defaults to 256k.
+
+@item sslverify
+Whether to verify the remote server's certificate when connecting over SSL. It
+can have the value 'on' or 'off'. It defaults to 'on'.
+@end table
+
+Note that when passing options to qemu explicitly, @option{driver} is the value
+of .
+
+Example: boot from a remote Fedora 20 live ISO image
+@example
+qemu-system-x86_64 --drive 
media=cdrom,file=http://dl.fedoraproject.org/pub/fedora/linux/releases/20/Live/x86_64/Fedora-Live-Desktop-x86_64-20-1.iso,readonly
+
+qemu-system-x86_64 --drive 
media=cdrom,file.driver=http,file.url=http://dl.fedoraproject.org/pub/fedora/linux/releases/20/Live/x86_64/Fedora-Live-Desktop-x86_64-20-1.iso,readonly
+@end example
+
+Example: boot from a remote Fedora 20 cloud image using a local overlay for
+writes, copy-on-read, and a readahead of 64k
+@example
+qemu-img create -f qcow2 -o backing_file='json:@{"file.driver":"http",, 
"file.url":"https://dl.fedoraproject.org/pub/fedora/linux/releases/20/Images/x86_64/Fedora-x86_64-20-20131211.1-sda.qcow2";,,
 "file.readahead":"64k"@}' /tmp/Fedora-x86_64-20-20131211.1-sda.qcow2
+
+qemu-system-x86_64 -drive 
file=/tmp/Fedora-x86_64-20-20131211.1-sda.qcow2,copy-on-read=on
+@end example
+
+Example: boot from an image stored on a VMware vSphere server with a 
self-signed
+certificate using a local overlay for writes and a readahead of 64k
+@example
+qemu-img create -f qcow2 -o backing_file='json:@{"file.driver":"https",, 
"file.url":"https://user:password@@vsphere.example.com/folder/test/test-flat.vmdk?dcPath=Datacenter&dsName=datastore1";,,
 "file.sslverify":"off",, "file.readahead":"64k"@}' /tmp/test.qcow2
+
+qemu-system-x86_64 -drive file=/tmp/test.qcow2
+@end example
 ETEXI
 
 STEXI
-- 
1.9.0




[Qemu-devel] [PATCH 1/4] curl: Fix build when curl_multi_socket_action isn't available

2014-05-14 Thread Matthew Booth
Signed-off-by: Matthew Booth 
---
 block/curl.c | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/block/curl.c b/block/curl.c
index d2f1084..f3c797a 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -37,6 +37,21 @@
 #if LIBCURL_VERSION_NUM >= 0x071000
 /* The multi interface timer callback was introduced in 7.16.0 */
 #define NEED_CURL_TIMER_CALLBACK
+#define HAVE_SOCKET_ACTION
+#endif
+
+#ifndef HAVE_SOCKET_ACTION
+/* If curl_multi_socket_action isn't available, define it statically here in
+ * terms of curl_multi_socket. Note that ev_bitmask will be ignored, which is
+ * less efficient but still safe. */
+static CURLMcode __curl_multi_socket_action(CURLM *multi_handle,
+curl_socket_t sockfd,
+int ev_bitmask,
+int *running_handles)
+{
+return curl_multi_socket(multi_handle, sockfd, running_handles);
+}
+#define curl_multi_socket_action __curl_multi_socket_action
 #endif
 
 #define PROTOCOLS (CURLPROTO_HTTP | CURLPROTO_HTTPS | \
-- 
1.9.0




Re: [Qemu-devel] [Qemu-ppc] macio ide question/bug report

2014-05-14 Thread BALATON Zoltan

On Wed, 14 May 2014, Mark Cave-Ayland wrote:

On 14/05/14 12:10, BALATON Zoltan wrote:

The logs I've posted are with DEBUG_IDE_ATAPI, DEBUG_DBDMA and
DEBUG_MACIO already enabled...


Well sure, but not the ones in your last email - I had to go back several 
mails back into the thread to pull them out. Bear in mind the high volume of 
these lists means that a lot of people who could help won't have the time to 
do this.


All the logs I posted in this thread were with these debug options 
enabled. Maybe the beginning was missing as I only included the logs from 
the failing dma reply because before that I was tracing to see that the 
TOC was read and about to be returned so I did not include those logs 
again. (They were in the previous mail though.) I'm including them again 
below this time.


Which part is it that's still confusing you? Putting breakpoints on 
pmac_ide_transfer() and pmac_ide_atapi_transfer_cb() will show you the 
iterations on each DMA request (be sure to compare against a "known good" 
example to understand how it should work first). If you can give more detail 
as to which bits are confusing, I will try my best to explain them.


Looking at the backtrace:

#0  ide_atapi_cmd_error (s=0x563cb238, sense_key=5, asc=33)
at hw/ide/atapi.c:141
#1  0x556cecf5 in ide_atapi_io_error (s=0x563cb238, ret=-5)
at hw/ide/atapi.c:160
#2  0x556d9d01 in pmac_ide_atapi_transfer_cb (opaque=0x563ccc68,
ret=-5) at hw/ide/macio.c:64
#3  0x556780d2 in dma_complete (dbs=0x563ab840, ret=-5)
at dma-helpers.c:121
#4  0x556781db in dma_bdrv_cb (opaque=0x563ab840, ret=-5)
at dma-helpers.c:149
#5  0x55614dd1 in bdrv_co_em_bh (opaque=0x563b5000) at block.c:4602
#6  0x555f8170 in aio_bh_poll (ctx=0x5637fc00) at async.c:81
#7  0x555f7dc9 in aio_poll (ctx=0x5637fc00, blocking=false)
at aio-posix.c:188
#8  0x555f8587 in aio_ctx_dispatch (source=0x5637fc00, callback=
0x0, user_data=0x0) at async.c:205
#9  0x778ca6d5 in g_main_context_dispatch ()
   from /lib64/libglib-2.0.so.0
#10 0x557a0f42 in glib_pollfds_poll () at main-loop.c:190
#11 0x557a1042 in os_host_main_loop_wait (timeout=0) at main-loop.c:235
#12 0x557a1115 in main_loop_wait (nonblocking=1) at main-loop.c:484
#13 0x55844190 in main_loop () at vl.c:2075
#14 0x5584bc23 in main (argc=30, argv=0x7fffdc88, envp=
0x7fffdd80) at vl.c:4556

shows that pmac_ide_atapi_transfer_cb is called with ret=-5 which is why 
it fails, so putting a breakpoint there is too late. What I don't 
understand is where this -5 value comes from. I don't have a known good 
example because Darwin reads the TOC differently (probably before enabling 
DMA, I did not trace it more than the logs I've included earlier though) 
and MorphOS always fails.


Here are the logs of all requests MorphOS does up to the failing ReadTOC 
again:


ATAPI limit=0x8000 packet: 00 00 00 00 00 00 00 00 00 00 00 00
ATAPI limit=0x8000 packet: 12 00 00 00 24 00 00 00 00 00 00 00
reply: tx_size=36 elem_tx_size=0 index=0
byte_count_limit=32768
status=0x58
reply: tx_size=0 elem_tx_size=0 index=36
status=0x50
ATAPI limit=0x8000 packet: 1b 00 00 00 01 00 00 00 00 00 00 00
ATAPI limit=0x8000 packet: 00 00 00 00 00 00 00 00 00 00 00 00
ATAPI limit=0x8000 packet: 25 00 00 00 00 00 00 00 00 00 00 00
reply: tx_size=8 elem_tx_size=0 index=0
byte_count_limit=32768
status=0x58
reply: tx_size=0 elem_tx_size=0 index=8
status=0x50
ATAPI limit=0x8000 packet: 5a 00 05 00 00 00 00 00 30 00 00 00
atapi_cmd_error: sense=0x5 asc=0x24
ATAPI limit=0x8000 packet: 5a 00 04 00 00 00 00 00 28 00 00 00
atapi_cmd_error: sense=0x5 asc=0x24
ATAPI limit=0x8000 packet: 5a 00 03 00 00 00 00 00 28 00 00 00
atapi_cmd_error: sense=0x5 asc=0x24
ATAPI limit=0x8000 packet: 5a 00 3f 00 00 00 00 01 02 00 00 00
atapi_cmd_error: sense=0x5 asc=0x24
ATAPI limit=0x8000 packet: 51 00 00 00 00 00 00 00 22 00 00 00
reply: tx_size=34 elem_tx_size=0 index=0
byte_count_limit=32768
status=0x58
reply: tx_size=0 elem_tx_size=0 index=34
status=0x50
DBDMA: writel 0x0d0c <= 0x00e4e960
DBDMA: channel 0x1a reg 0x3
DBDMA: dbdma_cmdptr_load 0x00e4e960
ATAPI limit=0x8000 packet: 43 00 00 00 00 00 00 03 24 00 00 00

DBDMA: DBDMA_run_bh
DBDMA: writel 0x0d00 <= 0x80008000
DBDMA: channel 0x1a reg 0x0
DBDMA: status 0x8400
DBDMA: readl 0x0d00 => 0x80008000
DBDMA: channel 0x1a reg 0x0
DBDMA: DBDMA_run_bh
DBDMA: channel_run
dbdma_cmd 0x7f8f93662ee0
req_count 0x0324
command 0x3000
phy_addr 0x00e4f30c
cmd_dep 0x
res_count 0x
xfer_status 0x
DBDMA: start_input
DBDMA: addr 0xe4f30c key 0x0
pmac_ide_transfer(ATAPI) lba=, buffer_index=0, len=324
io_buffer_size = 0
io->len = 0x324
sector_num=-1 size=20, cmd_cmd=0
atapi_cmd_error: sense=0x5 asc=0x21
done DMA
DBDMA: dbdma_end
DBDMA: conditional_wait
DBDMA: dbdma_cmdptr_sav

Re: [Qemu-devel] [PATCH v2] block/raw-posix: Try both FIEMAP and SEEK_HOLE

2014-05-14 Thread Max Reitz

On 11.05.2014 19:26, Christoph Hellwig wrote:

On Tue, May 06, 2014 at 09:00:54PM +0200, Max Reitz wrote:

The current version of raw-posix always uses ioctl(FS_IOC_FIEMAP) if
FIEMAP is available; lseek with SEEK_HOLE/SEEK_DATA are not even
compiled in in this case. However, there may be implementations which
support the latter but not the former (e.g., NFSv4.2) as well as vice
versa.

To cover both cases, always try SEEK_HOLE/SEEK_DATA (as this will
probably be covered by POSIX soon) and if that does not work, fall back
to FIEMAP; and if that does not work either, treat everything as
allocated.

Btw, while I think that SEEK_HOLE/SEEK_DATA generally is the better API
for qemu, the NFS 4.2 SEEK operation will be sufficient for a proper FIEMAP
implementation, and we'll implement it for the Linux NFS client.


Hm, great, in that case this patch will probably never be put to the 
test. *g*


Max



Re: [Qemu-devel] [PATCH 6/7] arm/virt: enable PSCI emulation support for system emulation

2014-05-14 Thread Rob Herring
On Wed, May 14, 2014 at 4:25 PM, Peter Maydell  wrote:
> On 14 May 2014 20:15, Rob Herring  wrote:
>> On Wed, May 14, 2014 at 12:51 PM, Peter Maydell
>>  wrote:
>>> My suggestion to Pranav was that we abstract away the "which PSCI
>>> version?" decision into a field in ARMCPU, in which case we can
>>> just have TCG always set it to 0.2. So some of this logic
>>> will get a little simpler on rebase.
>>
>> You can't. You have to support both because you don't know what the
>> kernel supports. An old kernel will only support arm,psci.
>
> An old host kernel, or an old guest kernel? The former is fine,
> because the KVM CPU init code will just ask for the KVM
> capability and fill in the ARMCPU field appropriately.
> For the latter, how are you supposed to determine what the
> guest kernel can support?

Guest kernels and this was exactly my point that you can't determine
it. The virt dtb is for the guest kernel and must be either 0.1 PSCI
only or both 0.1 and 0.2. I think I misread what you meant. Reading
the other thread, as long as you just mean changing the if statement
like this, then we are in agreement:

if (psci version is 0.1) {
qemu_fdt_setprop_string(fdt, "/psci", "compatible", "arm,psci");
} else {
const char compat[] = "arm,psci-0.2\0arm,psci";
qemu_fdt_setprop(fdt, "/psci", "compatible", compat, sizeof(compat));
}

Rob



Re: [Qemu-devel] [PATCH] configure: Ensure tests/qemu-iotests exists before writing common.env

2014-05-14 Thread Max Reitz

On 14.05.2014 16:26, Peter Maydell wrote:

Before we write common.env to the tests/qemu-iotests directory, ensure
that it exists. This fixes out-of-tree builds from clean.

Signed-off-by: Peter Maydell 
---
If somebody would like to review this I'll apply it to master as
a buildfix...

  configure | 1 +
  1 file changed, 1 insertion(+)

diff --git a/configure b/configure
index 6adfa72..c4e43ed 100755
--- a/configure
+++ b/configure
@@ -4770,6 +4770,7 @@ fi
  
  iotests_common_env="tests/qemu-iotests/common.env"
  
+mkdir -p "$(dirname "$iotests_common_env")"

  echo "# Automatically generated by configure - do not modify" > 
$iotests_common_env
  echo >> $iotests_common_env
  echo "PYTHON='$python'" >> $iotests_common_env


If we do this, we'd have a commen.env which we cannot use, as all the 
tests are still in the original source tree.


I'd rather use "$source_path/tests/qemu_iotests/common.env" instead, or, 
if that is unacceptable as it modifies the original source tree (which 
is probably not desired when doing an out-of-tree build), write 
common.env only if this is an in-tree build.


Max



Re: [Qemu-devel] [PATCH v2 06/23] target-arm: add arm_is_secure() function

2014-05-14 Thread Greg Bellows
Thanks for the input Peter.

>From briefly looking, it actually looks like the ARMv7 spec more strictly
states that the default is secure.  ARMv8 on the other hand appears to
leave it open as implementation defined,

Based on out past discussions, I assumed non-secure as the default and that
appears to be the direction of Fabian's code as well.  Clearly if we want
to follow the spec, we'd both need to change this default.

Greg



On 14 May 2014 16:29, Peter Maydell  wrote:

> On 14 May 2014 21:22, Greg Bellows  wrote:
> > I suppose it depends on how true we want to be to the specification and
> > whether our default is NS=0 or NS=1 when the security extension is
> present
> > or not.  The code currently assumes non-secure as the default state.
>
> The v8 ARM ARM at least allows the CPU to behave as if only
> NS was present if there is no implementation of the Security
> extensions. I haven't checked the v7 wording.
>
> (In general I think QEMU's implementation of this should follow
> the v8 ARM ARM and treat v7 CPUs as a sort of special degenerate
> case.)
>
> > Is there a convention in qemu?  How closely do we attempt to stay to the
> > pseudo code provided in the spec?
>
> The pseudocode in the ARM ARM is part of the spec. We should
> strive to follow the spec. This doesn't necessarily mean matching
> pseudocode functions exactly -- the requirement is to be
> behaviourally the same, and sometimes the pseudocode is
> written to be clear rather than efficient or to deal with situations
> we don't necessarily care about.
>
> thanks
> -- PMM
>


Re: [Qemu-devel] Curl updates

2014-05-14 Thread Eric Blake
On 05/14/2014 03:20 PM, Matthew Booth wrote:

> 
>> Right, but we want that anyway. I applied Max's patches for the
>> json: pseudo-protocol today, so we should have everything we need.
> 
> I can't see this in block/master. Am I looking in the wrong place?

git://repo.or.cz/qemu/kevin.git block

or view in your browser here:
http://repo.or.cz/w/qemu/kevin.git/shortlog/refs/heads/block

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature


[Qemu-devel] [Bug 1303926] Re: qemu-system-x86_64 crashed with SIGABRT

2014-05-14 Thread Serge Hallyn
Hi,

the cause of this particular bug was introduced during 2014, so could
not have been present in precise.  We definately will want to figure out
the cause of your bug, so please open a new bug report using 'ubuntu-bug
qemu-kvm' immediately after a crash has happened.

Thanks!

** Also affects: qemu (Ubuntu Precise)
   Importance: Undecided
   Status: New

** No longer affects: qemu (Ubuntu Precise)

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

Title:
  qemu-system-x86_64 crashed with SIGABRT

Status in QEMU:
  New
Status in “qemu” package in Ubuntu:
  Fix Released

Bug description:
  I've been getting this periodically since my upgrade to qemu 2.0 in
  trusty this morning.

  ProblemType: Crash
  DistroRelease: Ubuntu 14.04
  Package: qemu-system-x86 2.0.0~rc1+dfsg-0ubuntu1
  ProcVersionSignature: Ubuntu 3.13.0-23.45-generic 3.13.8
  Uname: Linux 3.13.0-23-generic x86_64
  ApportVersion: 2.14.1-0ubuntu1
  Architecture: amd64
  Date: Mon Apr  7 13:31:53 2014
  ExecutablePath: /usr/bin/qemu-system-x86_64
  InstallationDate: Installed on 2013-11-26 (131 days ago)
  InstallationMedia: Ubuntu 13.10 "Saucy Salamander" - Release amd64 
(20131016.1)
  ProcEnviron: PATH=(custom, no user)
  Registers: No symbol table is loaded.  Use the "file" command.
  Signal: 6
  SourcePackage: qemu
  StacktraceTop:
   
  Title: qemu-system-x86_64 crashed with SIGABRT
  UpgradeStatus: Upgraded to trusty on 2014-01-17 (79 days ago)
  UserGroups:

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



Re: [Qemu-devel] [PATCH v2 06/23] target-arm: add arm_is_secure() function

2014-05-14 Thread Peter Maydell
On 14 May 2014 21:22, Greg Bellows  wrote:
> I suppose it depends on how true we want to be to the specification and
> whether our default is NS=0 or NS=1 when the security extension is present
> or not.  The code currently assumes non-secure as the default state.

The v8 ARM ARM at least allows the CPU to behave as if only
NS was present if there is no implementation of the Security
extensions. I haven't checked the v7 wording.

(In general I think QEMU's implementation of this should follow
the v8 ARM ARM and treat v7 CPUs as a sort of special degenerate
case.)

> Is there a convention in qemu?  How closely do we attempt to stay to the
> pseudo code provided in the spec?

The pseudocode in the ARM ARM is part of the spec. We should
strive to follow the spec. This doesn't necessarily mean matching
pseudocode functions exactly -- the requirement is to be
behaviourally the same, and sometimes the pseudocode is
written to be clear rather than efficient or to deal with situations
we don't necessarily care about.

thanks
-- PMM



Re: [Qemu-devel] [PATCH 6/7] arm/virt: enable PSCI emulation support for system emulation

2014-05-14 Thread Peter Maydell
On 14 May 2014 20:15, Rob Herring  wrote:
> On Wed, May 14, 2014 at 12:51 PM, Peter Maydell
>  wrote:
>> My suggestion to Pranav was that we abstract away the "which PSCI
>> version?" decision into a field in ARMCPU, in which case we can
>> just have TCG always set it to 0.2. So some of this logic
>> will get a little simpler on rebase.
>
> You can't. You have to support both because you don't know what the
> kernel supports. An old kernel will only support arm,psci.

An old host kernel, or an old guest kernel? The former is fine,
because the KVM CPU init code will just ask for the KVM
capability and fill in the ARMCPU field appropriately.
For the latter, how are you supposed to determine what the
guest kernel can support?

thanks
-- PMM



[Qemu-devel] [Bug 1318830] Re: High CPU usage on windows virtual machine

2014-05-14 Thread Serge Hallyn
** Package changed: libvirt (Ubuntu) => qemu (Ubuntu)

** Also affects: qemu
   Importance: Undecided
   Status: New

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

Title:
  High CPU usage on windows virtual machine

Status in QEMU:
  New
Status in “qemu” package in Ubuntu:
  New

Bug description:
  I got Ubuntu 14.04, with Qemu 2.0 and moved my windows VM to this new box, 
and made sure that what this article indicates was achieved
  https://www.kraxel.org/blog/2014/03/qemu-and-usb-tablet-cpu-consumtion/
  I can attest that it works following the instructions, erasing the registry, 
etc.
  Unfortunately, with 4 cpus as below, I still see 60% CPU outside as shown by 
"Top" versus 0% CPU inside. My Kernel is 3.15.0-031500rc4-generic
  If some developer wants to log in and take a look, I am happy to help. The 
box is not in production and I take full responsibility. Until this is solved, 
KVM is not going to compete with Hyper-V or Vmware.  Basically KVM is not 
suitable for the Enterprise as of yet.

  qemu-system-x86_64 -enable-kvm -name Production -S -machine pc-
  i440fx-2.0,accel=kvm,usb=off -cpu
  
kvm64,+rdtscp,+pdpe1gb,+x2apic,+dca,+pcid,+pdcm,+xtpr,+tm2,+est,+smx,+vmx,+ds_cpl,+monitor,+dtes64,+pclmuldq,+pbe,+tm,+ht,+ss,+acpi,+ds,+vme,hv_relaxed,hv_vapic,hv_spinlocks=0xfff
  -m 4024 -realtime mlock=off -smp 4,sockets=1,cores=4,threads=1 -uuid
  e8701c5c-b542-0199-fd2a-1047df24770e -no-user-config -nodefaults
  -chardev
  
socket,id=charmonitor,path=/var/lib/libvirt/qemu/Production.monitor,server,nowait
  -mon chardev=charmonitor,id=monitor,mode=control -rtc base=localtime
  -no-shutdown -boot strict=on -device piix3-usb-
  uhci,id=usb,bus=pci.0,addr=0x1.0x2 -drive
  file=/var/lib/libvirt/images/Production.img,if=none,id=drive-virtio-
  disk0,format=raw -device virtio-blk-
  pci,scsi=off,bus=pci.0,addr=0x6,drive=drive-virtio-disk0,id=virtio-
  disk0,bootindex=1 -netdev tap,fd=30,id=hostnet0,vhost=on,vhostfd=31
  -device virtio-net-
  pci,netdev=hostnet0,id=net0,mac=00:16:3a:d2:cd:ea,bus=pci.0,addr=0x3
  -netdev tap,fd=35,id=hostnet1,vhost=on,vhostfd=36 -device virtio-net-
  pci,netdev=hostnet1,id=net1,mac=52:54:00:70:fe:54,bus=pci.0,addr=0x4
  -chardev pty,id=charserial0 -device isa-
  serial,chardev=charserial0,id=serial0 -device usb-tablet,id=input0
  -vnc 127.0.0.1:0 -device VGA,id=video0,bus=pci.0,addr=0x2 -device
  intel-hda,id=sound0,bus=pci.0,addr=0x5 -device hda-
  duplex,id=sound0-codec0,bus=sound0.0,cad=0 -device virtio-balloon-
  pci,id=balloon0,bus=pci.0,addr=0x7

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



Re: [Qemu-devel] Curl updates

2014-05-14 Thread Matthew Booth
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 14/05/14 12:43, Kevin Wolf wrote:
> Am 14.05.2014 um 18:08 hat Matthew Booth geschrieben:
>> On 14/05/14 03:48, Kevin Wolf wrote:
>>> Am 13.05.2014 um 21:47 hat Eric Blake geschrieben:
 On 05/08/2014 02:42 AM, Matthew Booth wrote:
> [PATCH 1/4] curl: Fix parsing of readahead option from 
> filename [PATCH 2/4] curl: Add sslverify option [PATCH
> 3/4] curl: Add usage documentation
> 
> The first 3 patches are reposted with updates following 
> discussion of the option syntax. With this patch I've
> decided to break entirely with the previous syntax. Given
> that option parsing was previously both broken and
> undocumented, this is hopefully a forgivable sin.
> 
> The new syntax is:
> 
> http://user:passw...@example.com/path?query[opt1=val:opt2=val]
>
>
> 
I've bounded the option block in square brackets as these have
> no semantic meaning in any of the supported URI formats.
 
 Offhand, I'm not liking this.  Why not use a completely
 valid URI, with '.../path?query&opt1=val&opt2=val'?
 Inventing your own [opt1=val:opt2=val] on top of URI is
 asking for confusion.
 
 Are you trying to support a way to pass a query string to
 the curl URI, in addition to local options?  How often do
 curl URIs need a query?
>>> 
>>> My guess would be that you need this more often than local 
>>> options.
>>> 
>>> Anyway, let's not add new options encoded in the URL, but
>>> point users to separate options. We may decide that we need the
>>> support the old crude way of encoding local options for
>>> compatibility, but preferably I would make filename just a
>>> plain URL.
>> 
>> Agree, but only when we support giving options to a backing
>> file.
> 
> Right, but we want that anyway. I applied Max's patches for the
> json: pseudo-protocol today, so we should have everything we need.

I can't see this in block/master. Am I looking in the wrong place?

Matt

> 
> Kevin
> 


- -- 
Matthew Booth
Red Hat Engineering, Virtualisation Team

Phone: +442070094448 (UK)
GPG ID:  D33C3490
GPG FPR: 3733 612D 2D05 5458 8A8A 1600 3441 EA19 D33C 3490
-BEGIN PGP SIGNATURE-
Version: GnuPG v1
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iEYEARECAAYFAlNz3jQACgkQNEHqGdM8NJBBNACeKipCLadTv1+AANVJzLrMcJmU
q0IAnREbaaT3LVXaIYr09eej04mzvokG
=bEUQ
-END PGP SIGNATURE-



Re: [Qemu-devel] [PATCH v2 19/23] target-arm: maintain common bits of banked CP registers

2014-05-14 Thread Greg Bellows
On 13 May 2014 11:16, Fabian Aggeler  wrote:

> Some of SCTRL bits are common for secure and non-secure state.
> Translation table base masks are updated on NS-bit switch as
> well as on TTBCR writes.
>
> Signed-off-by: Sergey Fedorov 
> Signed-off-by: Fabian Aggeler 
> ---
>  target-arm/cpu.h| 10 ++
>  target-arm/helper.c | 39 ++-
>  2 files changed, 48 insertions(+), 1 deletion(-)
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index c20c44a..7893004 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -425,6 +425,14 @@ int arm_cpu_handle_mmu_fault(CPUState *cpu, vaddr
> address, int rw,
>  #define SCTLR_AFE (1U << 29)
>  #define SCTLR_TE  (1U << 30)
>
> +/* Bitmask for banked bits (Security Extensions) */
> +#define SCTLR_BANKED(SCTLR_TE | SCTLR_AFE | SCTLR_TRE | SCTLR_EE | \
> +SCTLR_VE | SCTLR_HA | SCTLR_UWXN | SCTLR_WXN | SCTLR_V | \
> +SCTLR_I | SCTLR_Z | SCTLR_SW | SCTLR_CP15BEN | SCTLR_C | \
> +SCTLR_A | SCTLR_M)
> +/* Treat bits that are not explicitly banked as common */
> +#define SCTLR_COMMON (~SCTLR_BANKED)
> +
>  #define CPSR_M (0x1fU)
>  #define CPSR_T (1U << 5)
>  #define CPSR_F (1U << 6)
> @@ -662,6 +670,8 @@ static inline int arm_feature(CPUARMState *env, int
> feature)
>  return (env->features & (1ULL << feature)) != 0;
>  }
>
> +#define SCR_NS (1U << 0)
> +
>  /* Return true if the processor is in secure state */
>  static inline bool arm_is_secure(CPUARMState *env)
>  {
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index c76a86b..618fd31 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -2165,12 +2165,49 @@ static void sctlr_write(CPUARMState *env, const
> ARMCPRegInfo *ri,
>  }
>
>  raw_write(env, ri, value);
> +
> +if (!arm_el_is_aa64(env, 3)) {
> +/* Aarch32 has some bits that are common to secure (mapped to
> + * SCTLR_EL3) and non-secure instances of the SCTLR. */
> +
> +env->cp15.c1_sys_el1 &= SCTLR_BANKED |
> +(arm_current_sctlr(env) & SCTLR_COMMON);
> +env->cp15.c1_sys_el3 &= SCTLR_BANKED |
> +(arm_current_sctlr(env) & SCTLR_COMMON);
> +}
> +
>  /* ??? Lots of these bits are not implemented.  */
>  /* This may enable/disable the MMU, so do a TLB flush.  */
>  tlb_flush(CPU(cpu), 1);
>  }
>
>  #ifndef CONFIG_USER_ONLY
> +static void scr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t
> value)
> +{
> +if (!arm_el_is_aa64(env, 3) &&
> +(value & SCR_NS) != (env->cp15.c1_scr & SCR_NS)) {
> +/* If EL3 is using Aarch32 switching NS-bit may make the CPU use a
> + * different instance (secure or non-secure) when accessing CP
> + * registers.
> + * Common bits of otherwise banked registers need to by
> synchronized
> + * at this point.
> + */
> +env->cp15.c1_sys_el1 &= SCTLR_BANKED |
> +(arm_current_sctlr(env) & SCTLR_COMMON);
> +env->cp15.c1_sys_el3 &= SCTLR_BANKED |
> +(arm_current_sctlr(env) & SCTLR_COMMON);
>

I must be missing something, if the common bits are sync'd across the banks
in sctlr_write(), why do we need to do it here?


> +}
> +
> +env->cp15.c1_scr = value;
> +
> +/* After possible switch, calculate c2_mask and c2_base_mask again
> for the
> + * instance which is now active (secure or non-secure).
> + */
> +int maskshift = extract32(A32_BANKED_REG_GET(env, c2_control), 0, 3);
> +env->cp15.c2_mask = ~(((uint32_t)0xu) >> maskshift);
> +env->cp15.c2_base_mask = ~((uint32_t)0x3fffu >> maskshift);
>

As mentioned elsewhere, does it make sense to bank the c2_mask fields so
they stay in sync with c2_control?  Presumably we would not need to do
these updates in this case.


> +
> +}
>
>  static void nsacr_write(CPUARMState *env, const ARMCPRegInfo *ri,
>uint64_t value)
> @@ -2183,7 +2220,7 @@ static const ARMCPRegInfo tz_cp_reginfo[] = {
>  #ifndef CONFIG_USER_ONLY
>  { .name = "SCR", .cp = 15, .crn = 1, .crm = 1, .opc1 = 0, .opc2 = 0,
>.access = PL3_RW, .fieldoffset = offsetof(CPUARMState, cp15.c1_scr),
> -  .resetvalue = 0, },
> +  .resetvalue = 0, .writefn = scr_write },
>  { .name = "SCR_EL3", .state = ARM_CP_STATE_AA64,
>.opc0 = 3, .crn = 6, .crm = 1, .opc1 = 0, .opc2 = 0,
>.access = PL3_RW, .fieldoffset = offsetof(CPUARMState, cp15.c1_scr),
> --
> 1.8.3.2
>
>
>


Re: [Qemu-devel] [PATCH 3/7] target-arm: add hvc and smc exception emulation handling infrastructure

2014-05-14 Thread Rob Herring
On Wed, May 14, 2014 at 12:44 PM, Peter Maydell
 wrote:
> On 5 May 2014 17:00, Rob Herring  wrote:
>> From: Rob Herring 
>>
>> Add the infrastructure to handle and emulate hvc and smc exceptions.
>> This will enable emulation of things such as PSCI calls. This commit
>> does not change the behavior and will exit with unknown exception.

[...]

>> +case EXCP_HVC:
>> +if (arm_cpu_do_hvc(cs)) {
>> +return;
>> +}
>> +cpu_abort(cs, "Unhandled exception 0x%x\n", cs->exception_index);
>> +return;
>> +case EXCP_SMC:
>> +if (arm_cpu_do_smc(cs)) {
>> +return;
>> +}
>> +/* Fall-though */
>
> We can't cpu_abort() just because the guest hands us an HVC
> or SMC that we don't happen to handle in QEMU. We should
> just qemu_log_mask(LOG_GUEST_ERROR, ...) and then do the
> architecturally mandated behaviour for SMC or HVC instructions
> on a CPU which doesn't implement EL2/EL3, ie treat as
> UnallocatedEncoding(). (Don't forget to fix up exception.syndrome
> in this case, since it should be syn_uncategorized(), not
> the SMC/HVC value.)

So I think this and shifting setting ESR/FAR after the switch should
be sufficient:

case EXCP_SMC:
if (arm_cpu_do_smc(cs)) {
return;
}
env->exception.syndrome = syn_uncategorized();
if (is_a64(env)) {
env->pc -= 4;
} else {
env->regs[15] -= env->thumb ? 2 : 4;
}
break;

I'm not completely sure the PC adjustment is correct.

>> diff --git a/target-arm/helper.c b/target-arm/helper.c
>> index 3be917c..b5b4a17 100644
>> --- a/target-arm/helper.c
>> +++ b/target-arm/helper.c
>> @@ -3253,6 +3253,28 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs)
>>  env->thumb = addr & 1;
>>  }
>>
>> +bool arm_cpu_do_hvc(CPUState *cs)
>> +{
>> +bool ret;
>> +
>> +ret = arm_handle_psci(cs);
>> +if (ret) {
>> +return ret;
>> +}
>> +return false;
>> +}
>> +
>> +bool arm_cpu_do_smc(CPUState *cs)
>> +{
>> +bool ret;
>> +
>> +ret = arm_handle_psci(cs);
>> +if (ret) {
>> +return ret;
>> +}
>> +return false;
>> +}
>
> This hunk means the series doesn't compile after this
> patch is applied. I think in this patch these two functions
> should both just "return false;" indicating that no HVC
> or SMC calls have any special handling by QEMU. Then the
> patch which adds psci.c can also add the calls.

Ah yes, I had it like that and forgot to go back and split it up in
the process of rebasing.


>> +static inline uint32_t syn_aa64_hvc(uint32_t imm16)
>> +{
>> +return (EC_AA64_HVC << ARM_EL_EC_SHIFT) | ARM_EL_IL | (imm16 & 0x);
>> +}
>> +
>> +static inline uint32_t syn_aa32_hvc(uint32_t imm16)
>> +{
>> +return (EC_AA32_HVC << ARM_EL_EC_SHIFT) | ARM_EL_IL | (imm16 & 0x);
>> +}
>> +
>> +static inline uint32_t syn_aa64_smc(uint32_t imm16)
>> +{
>> +return (EC_AA64_SMC << ARM_EL_EC_SHIFT) | ARM_EL_IL | (imm16 & 0x);
>> +}
>> +
>
> You want a syn_aa32_smc() as well [note that it doesn't
> take an immediate].

I also noticed I need to set ARM_EL_IL based on thumb or not on the
aa32 variants.


>> +if (!(insn & (1 << 20))) {
>> +/* Hypervisor call (v7) */
>> +uint32_t imm16 = extract32(insn, 16, 4);
>> +imm16 |= extract32(insn, 0, 12) << 4;
>
> This is the wrong way round, I think: imm16 is imm4:imm12, not
> imm12:imm4.

You're right. ARM and Thumb are reversed.

Rob



Re: [Qemu-devel] Curl updates

2014-05-14 Thread Matthew Booth
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 14/05/14 13:02, Eric Blake wrote:
> On 05/14/2014 10:06 AM, Matthew Booth wrote:
> 
 The new syntax is:
 
 http://user:passw...@example.com/path?query[opt1=val:opt2=val]

>
 
>> 
>> A URI can, by definition, contain a query string, and we cannot
>> assume that it won't. In fact, the use case I'm specifically
>> interested in always includes a query string. If we try to
>> overload the query string, we're adding heuristic fuzziness. My
>> syntax makes the option string distinct from the URI, so no
>> heuristics are required. It's also very clear to read IMHO.
> 
> But your proposed syntax is no longer a URI.  I'd much rather see:
> 
> 'json:{"driver":"curl","filename":"http://user:passw...@example.com/path?query","opt1":"val","opt2":"val"}'
>
>  which then shares the same syntax as all other drivers for
> creating a flat string that encodes multiple pieces of information,
> rather than having to overload the filename to be a non-URI
> encoding locally useful information.
> 

Agree: if it's possible to pass explicit parameters to a backing file
then we should ditch the hokey parsing. I'll check out the new syntax,
rip out the parser and update the docs.

Matt
- -- 
Matthew Booth
Red Hat Engineering, Virtualisation Team

Phone: +442070094448 (UK)
GPG ID:  D33C3490
GPG FPR: 3733 612D 2D05 5458 8A8A 1600 3441 EA19 D33C 3490
-BEGIN PGP SIGNATURE-
Version: GnuPG v1
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iEYEARECAAYFAlNz1f4ACgkQNEHqGdM8NJBQ8ACfeekpMvSJS0kh1sx+/gtT6lS6
nwgAn2yxW2ympFXfvTybxQhBfdL907Cc
=HDQu
-END PGP SIGNATURE-



Re: [Qemu-devel] [PATCH RESEND v4 00/18] target-i386: CPU feature flag queue

2014-05-14 Thread Andreas Färber
Am 14.05.2014 21:29, schrieb Eduardo Habkost:
> (Resending due to complete lack of feedback on v4 submission from 15 days 
> ago.)

Marcelo had reminded me, and I had started review of the original v4,
but not through yet. It looks as if the resend changed nothing, so I'll
continue on that one.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



Re: [Qemu-devel] [PATCH 2/4] qapi: output visitor crashes qemu if it encounters a NULL value

2014-05-14 Thread Michael Roth
Quoting Luiz Capitulino (2014-05-14 13:25:16)
> On Wed, 14 May 2014 20:29:37 +0300
> Marcel Apfelbaum  wrote:
> 
> > On Wed, 2014-05-14 at 19:00 +0200, Andreas Färber wrote:
> > > Am 13.05.2014 21:08, schrieb Eric Blake:
> > > > On 05/13/2014 11:36 AM, Andreas Färber wrote:
> > > >> Am 07.05.2014 16:42, schrieb Marcel Apfelbaum:
> > > >>> A NULL value is not added to visitor's stack, but there is no
> > > >>> check for that when the visitor tries to return that value,
> > > >>> leading to Qemu crash.
> > > >>> 
> > > >>> Reviewed-by: Eric Blake  Signed-off-by:
> > > >>> Marcel Apfelbaum 
> > > >> 
> > > >> Where does the Rb come from on this v1? Is it in any tree
> > > >> already?
> > > >> 
> > > > 
> > > > The (weak) R-b was here: 
> > > > https://lists.gnu.org/archive/html/qemu-devel/2014-02/msg02861.html
> > > 
> > > Thanks.
> > > > 
> > > So Luiz was okay with it too, but his last message seems to be
> > > indicating this needs to be fixed somewhere else, too:
> > > 
> > > https://lists.gnu.org/archive/html/qemu-devel/2014-02/msg05228.html
> > > https://lists.gnu.org/archive/html/qemu-devel/2014-03/msg00217.html
> > > 
> > > Can/should that be addressed as a follow-up? Or is there a test case
> > > that breaks?
> > Simple and "popular" test case: the user does not use the -kernel-cmdline 
> > parameter.
> > The patch is needed because otherwise the main function will fail
> > if no value is passed by the user to string parameters. 
> > 
> > Regarding Luiz's concern, it can be a follow-up as I am not aware of
> > any problem with that.
> 
> My concern was that I wasn't sure if this is the right fix for the issue
> or if it's papering over the real bug. I quickly checked the code and it
> seemed to make sense, but I didn't have time to study it deeper.

Not sure the fix is bad or not, but the cause might be a little more subtle
than NULL string values as mentioned in the other thread. QmpOutputVisitor
encodes NULL strings as "" via qmp_output_type_str(), so the problem doesn't
seem to lie there: it shouldn't generate NULL values on the stack.

I think the real issue is that object_property_get_str() actually calls an
accessor via property_get_str to get the string, then explicitly *skips*
the call to visit_type_str() if it is NULL (as it would be in the case of,
say, kernel_cmdline option being NULL). So I wonder if maybe the real issue
we're fixing is a corner case where you call qmp_output_get_qobject() on
an "empty" QmpOutputVisitor.

Surprised that's not covered by tests, but didn't see any coverage doing
a cursory glance. Actually, might as well just add one..

diff --git a/tests/test-qmp-output-visitor.c b/tests/test-qmp-output-visitor.c
index e073d83..f190eaa 100644
--- a/tests/test-qmp-output-visitor.c
+++ b/tests/test-qmp-output-visitor.c
@@ -434,6 +434,17 @@ static void test_visitor_out_union(TestOutputVisitorData 
*data,
 QDECREF(qdict);
 }
 
+static void test_visitor_out_empty(TestOutputVisitorData *data,
+   const void *unused)
+{
+QObject *arg;
+QDict *qdict;
+
+arg = qmp_output_get_qobject(data->qov);
+qdict = qobject_to_qdict(arg);
+QDECREF(qdict);
+}
+
 static void init_native_list(UserDefNativeListUnion *cvalue)
 {
 int i;
@@ -782,6 +793,8 @@ int main(int argc, char **argv)
 &out_visitor_data, 
test_visitor_out_list_qapi_free);
 output_visitor_test_add("/visitor/output/union",
 &out_visitor_data, test_visitor_out_union);
+output_visitor_test_add("/visitor/output/empty",
+&out_visitor_data, test_visitor_out_empty);
 output_visitor_test_add("/visitor/output/native_list/int",
 &out_visitor_data, 
test_visitor_out_native_list_int);
 output_visitor_test_add("/visitor/output/native_list/int8",

mdroth@loki:~/w/qemu-build$ tests/test-qmp-output-visitor 
/visitor/output/int: OK
/visitor/output/bool: OK
/visitor/output/number: OK
/visitor/output/string: OK
/visitor/output/no-string: OK
/visitor/output/enum: OK
/visitor/output/enum-errors: OK
/visitor/output/struct: OK
/visitor/output/struct-nested: OK
/visitor/output/struct-errors: OK
/visitor/output/list: OK
/visitor/output/list-qapi-free: OK
/visitor/output/union: OK
/visitor/output/empty: Segmentation fault (core dumped)

So I guess the question is whether we should support converting an empty
QmpOutputVisitor to a QObject. I would say yes, and that a NULL value is
probably the most reasonable value.

I would ask that commit/code is a little more explicit about what corner case
is being handled though, and that something like the above unit test be
included with the series.

> 
> We could ask Michael Roth or Anthony, but I wouldn't hold this series
> because of that. Here's my ACK if you need it:
> 
> Acked-by: Luiz Capitulino 



Re: [Qemu-devel] [PATCH v3.2 11/31] numa: introduce memory_region_allocate_system_memory

2014-05-14 Thread Eduardo Habkost
On Wed, May 14, 2014 at 05:43:15PM +0800, Hu Tao wrote:
> From: Paolo Bonzini 
> 
> Signed-off-by: Paolo Bonzini 
> Signed-off-by: Hu Tao 
> ---
>  hw/i386/pc.c| 4 +---
>  include/hw/boards.h | 6 +-
>  include/sysemu/sysemu.h | 1 +
>  numa.c  | 9 +
>  4 files changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 3673da8..3778d41 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1210,9 +1210,7 @@ FWCfgState *pc_memory_init(QEMUMachineInitArgs *args,
>   * with older qemus that used qemu_ram_alloc().
>   */
>  ram = g_malloc(sizeof(*ram));
> -memory_region_init_ram(ram, NULL, "pc.ram",
> -   below_4g_mem_size + above_4g_mem_size);
> -vmstate_register_ram_global(ram);
> +memory_region_allocate_system_memory(ram, NULL, "pc.ram", 
> args->ram_size);

I had to check if below_4g_mem_size+above_4g_mem_size can be always
safely replaced by args->ram_size. Personally, wouldn't change the
ram_size expression in the same patch that adds the new function, but:

Reviewed-by: Eduardo Habkost 


Maybe we could at least add:
  assert(below_4g_mem_size + above_4g_mem_size == args->ram_size);
to the code, later?


>  *ram_memory = ram;
>  ram_below_4g = g_malloc(sizeof(*ram_below_4g));
>  memory_region_init_alias(ram_below_4g, NULL, "ram-below-4g", ram,
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 4345bd0..3f1c17d 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -50,9 +50,13 @@ struct QEMUMachine {
>  const char *hw_version;
>  };
>  
> -#define TYPE_MACHINE_SUFFIX "-machine"
> +void memory_region_allocate_system_memory(MemoryRegion *mr, Object *owner,
> +  const char *name,
> +  uint64_t ram_size);
> +
>  int qemu_register_machine(QEMUMachine *m);
>  
> +#define TYPE_MACHINE_SUFFIX "-machine"
>  #define TYPE_MACHINE "machine"
>  #undef MACHINE  /* BSD defines it and QEMU does not use it */
>  #define MACHINE(obj) \
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index 423d49e..caf88dd 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -10,6 +10,7 @@
>  #include "qemu/notify.h"
>  #include "qemu/main-loop.h"
>  #include "qemu/bitmap.h"
> +#include "qom/object.h"
>  
>  /* vl.c */
>  
> diff --git a/numa.c b/numa.c
> index 439df87..bcd7b04 100644
> --- a/numa.c
> +++ b/numa.c
> @@ -33,6 +33,7 @@
>  #include "qapi/opts-visitor.h"
>  #include "qapi/dealloc-visitor.h"
>  #include "qapi/qmp/qerror.h"
> +#include "hw/boards.h"
>  
>  QemuOptsList qemu_numa_opts = {
>  .name = "numa",
> @@ -194,3 +195,11 @@ void set_numa_modes(void)
>  }
>  }
>  }
> +
> +void memory_region_allocate_system_memory(MemoryRegion *mr, Object *owner,
> +  const char *name,
> +  uint64_t ram_size)
> +{
> +memory_region_init_ram(mr, owner, name, ram_size);
> +vmstate_register_ram_global(mr);
> +}
> -- 
> 1.8.5.2.229.g4448466
> 
> 

-- 
Eduardo



[Qemu-devel] [PATCH RESEND v4 09/18] target-i386: Define TCG_*_FEATURES earlier on cpu.c

2014-05-14 Thread Eduardo Habkost
Those macros will be used in the feature_word_info array data, so need
to be defined earlier.

Signed-off-by: Eduardo Habkost 
---
Changes v1 -> v2:
 * Rebase to latest qom-cpu (commit 90c5d39c)
Changes v2 -> v3:
 * Trivial rebase after QEMU 2.0 (onto commit 2d03b49)
---
 target-i386/cpu.c | 117 +++---
 1 file changed, 59 insertions(+), 58 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 0e9f3ea..ccd05ad 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -262,6 +262,65 @@ static const char *cpuid_7_0_ebx_feature_name[] = {
 NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
 };
 
+#define I486_FEATURES (CPUID_FP87 | CPUID_VME | CPUID_PSE)
+#define PENTIUM_FEATURES (I486_FEATURES | CPUID_DE | CPUID_TSC | \
+  CPUID_MSR | CPUID_MCE | CPUID_CX8 | CPUID_MMX | CPUID_APIC)
+#define PENTIUM2_FEATURES (PENTIUM_FEATURES | CPUID_PAE | CPUID_SEP | \
+  CPUID_MTRR | CPUID_PGE | CPUID_MCA | CPUID_CMOV | CPUID_PAT | \
+  CPUID_PSE36 | CPUID_FXSR)
+#define PENTIUM3_FEATURES (PENTIUM2_FEATURES | CPUID_SSE)
+#define PPRO_FEATURES (CPUID_FP87 | CPUID_DE | CPUID_PSE | CPUID_TSC | \
+  CPUID_MSR | CPUID_MCE | CPUID_CX8 | CPUID_PGE | CPUID_CMOV | \
+  CPUID_PAT | CPUID_FXSR | CPUID_MMX | CPUID_SSE | CPUID_SSE2 | \
+  CPUID_PAE | CPUID_SEP | CPUID_APIC)
+
+#define TCG_FEATURES (CPUID_FP87 | CPUID_PSE | CPUID_TSC | CPUID_MSR | \
+  CPUID_PAE | CPUID_MCE | CPUID_CX8 | CPUID_APIC | CPUID_SEP | \
+  CPUID_MTRR | CPUID_PGE | CPUID_MCA | CPUID_CMOV | CPUID_PAT | \
+  CPUID_PSE36 | CPUID_CLFLUSH | CPUID_ACPI | CPUID_MMX | \
+  CPUID_FXSR | CPUID_SSE | CPUID_SSE2 | CPUID_SS)
+  /* partly implemented:
+  CPUID_MTRR, CPUID_MCA, CPUID_CLFLUSH (needed for Win64)
+  CPUID_PSE36 (needed for Solaris) */
+  /* missing:
+  CPUID_VME, CPUID_DTS, CPUID_SS, CPUID_HT, CPUID_TM, CPUID_PBE */
+#define TCG_EXT_FEATURES (CPUID_EXT_SSE3 | CPUID_EXT_PCLMULQDQ | \
+  CPUID_EXT_MONITOR | CPUID_EXT_SSSE3 | CPUID_EXT_CX16 | \
+  CPUID_EXT_SSE41 | CPUID_EXT_SSE42 | CPUID_EXT_POPCNT | \
+  CPUID_EXT_MOVBE | CPUID_EXT_AES | CPUID_EXT_HYPERVISOR)
+  /* missing:
+  CPUID_EXT_DTES64, CPUID_EXT_DSCPL, CPUID_EXT_VMX, CPUID_EXT_SMX,
+  CPUID_EXT_EST, CPUID_EXT_TM2, CPUID_EXT_CID, CPUID_EXT_FMA,
+  CPUID_EXT_XTPR, CPUID_EXT_PDCM, CPUID_EXT_PCID, CPUID_EXT_DCA,
+  CPUID_EXT_X2APIC, CPUID_EXT_TSC_DEADLINE_TIMER, CPUID_EXT_XSAVE,
+  CPUID_EXT_OSXSAVE, CPUID_EXT_AVX, CPUID_EXT_F16C,
+  CPUID_EXT_RDRAND */
+
+#ifdef TARGET_X86_64
+#define TCG_EXT2_X86_64_FEATURES (CPUID_EXT2_SYSCALL | CPUID_EXT2_LM)
+#else
+#define TCG_EXT2_X86_64_FEATURES 0
+#endif
+
+#define TCG_EXT2_FEATURES ((TCG_FEATURES & CPUID_EXT2_AMD_ALIASES) | \
+  CPUID_EXT2_NX | CPUID_EXT2_MMXEXT | CPUID_EXT2_RDTSCP | \
+  CPUID_EXT2_3DNOW | CPUID_EXT2_3DNOWEXT | \
+  TCG_EXT2_X86_64_FEATURES)
+  /* missing:
+  CPUID_EXT2_PDPE1GB */
+#define TCG_EXT3_FEATURES (CPUID_EXT3_LAHF_LM | CPUID_EXT3_SVM | \
+  CPUID_EXT3_CR8LEG | CPUID_EXT3_ABM | CPUID_EXT3_SSE4A)
+#define TCG_EXT4_FEATURES 0
+#define TCG_SVM_FEATURES 0
+#define TCG_KVM_FEATURES 0
+#define TCG_7_0_EBX_FEATURES (CPUID_7_0_EBX_SMEP | CPUID_7_0_EBX_SMAP | \
+  CPUID_7_0_EBX_BMI1 | CPUID_7_0_EBX_BMI2 | CPUID_7_0_EBX_ADX)
+  /* missing:
+  CPUID_7_0_EBX_FSGSBASE, CPUID_7_0_EBX_HLE, CPUID_7_0_EBX_AVX2,
+  CPUID_7_0_EBX_ERMS, CPUID_7_0_EBX_INVPCID, CPUID_7_0_EBX_RTM,
+  CPUID_7_0_EBX_RDSEED */
+
+
 typedef struct FeatureWordInfo {
 const char **feat_names;
 uint32_t cpuid_eax;   /* Input EAX for CPUID */
@@ -539,64 +598,6 @@ struct X86CPUDefinition {
 bool cache_info_passthrough;
 };
 
-#define I486_FEATURES (CPUID_FP87 | CPUID_VME | CPUID_PSE)
-#define PENTIUM_FEATURES (I486_FEATURES | CPUID_DE | CPUID_TSC | \
-  CPUID_MSR | CPUID_MCE | CPUID_CX8 | CPUID_MMX | CPUID_APIC)
-#define PENTIUM2_FEATURES (PENTIUM_FEATURES | CPUID_PAE | CPUID_SEP | \
-  CPUID_MTRR | CPUID_PGE | CPUID_MCA | CPUID_CMOV | CPUID_PAT | \
-  CPUID_PSE36 | CPUID_FXSR)
-#define PENTIUM3_FEATURES (PENTIUM2_FEATURES | CPUID_SSE)
-#define PPRO_FEATURES (CPUID_FP87 | CPUID_DE | CPUID_PSE | CPUID_TSC | \
-  CPUID_MSR | CPUID_MCE | CPUID_CX8 | CPUID_PGE | CPUID_CMOV | \
-  CPUID_PAT | CPUID_FXSR | CPUID_MMX | CPUID_SSE | CPUID_SSE2 | \
-  CPUID_PAE | CPUID_SEP | CPUID_APIC)
-
-#define TCG_FEATURES (CPUID_FP87 | CPUID_PSE | CPUID_TSC | CPUID_MSR | \
-  CPUID_PAE | CPUID_MCE | CPUID_CX8 | CPUID_APIC | CPUID_SEP | \
-  CPUID_MTRR | CPUID_PGE | CPUID_MCA | CPUID_CMOV | CPUID_PAT | \
-  CPUID_PSE36 | CPUID_CLFLUSH | CPUID_ACPI | CPUID_MMX | \
-  CPUID_FXSR | CPUID_SSE | CPUID_SSE2 | CPUID_SS)
-  /* partly implemented:
-  CPUID_MTRR, CP

Re: [Qemu-devel] [PATCH 2/4] qapi: output visitor crashes qemu if it encounters a NULL value

2014-05-14 Thread Andreas Färber
Am 14.05.2014 19:29, schrieb Marcel Apfelbaum:
> On Wed, 2014-05-14 at 19:00 +0200, Andreas Färber wrote:
>> Am 13.05.2014 21:08, schrieb Eric Blake:
>>> On 05/13/2014 11:36 AM, Andreas Färber wrote:
 Am 07.05.2014 16:42, schrieb Marcel Apfelbaum:
> A NULL value is not added to visitor's stack, but there is no
> check for that when the visitor tries to return that value,
> leading to Qemu crash.
>
> Reviewed-by: Eric Blake  Signed-off-by:
> Marcel Apfelbaum 

 Where does the Rb come from on this v1? Is it in any tree
 already?

>>>
>>> The (weak) R-b was here: 
>>> https://lists.gnu.org/archive/html/qemu-devel/2014-02/msg02861.html
>>
>> Thanks.
>>>
>> So Luiz was okay with it too, but his last message seems to be
>> indicating this needs to be fixed somewhere else, too:
>>
>> https://lists.gnu.org/archive/html/qemu-devel/2014-02/msg05228.html
>> https://lists.gnu.org/archive/html/qemu-devel/2014-03/msg00217.html
>>
>> Can/should that be addressed as a follow-up? Or is there a test case
>> that breaks?
> Simple and "popular" test case: the user does not use the -kernel-cmdline 
> parameter.

You had already mentioned a NULL -kernel. I was asking what test case
Luiz' qmp_output_last() would be about. :)

Cheers,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



Re: [Qemu-devel] [PATCH v2 06/23] target-arm: add arm_is_secure() function

2014-05-14 Thread Greg Bellows
I suppose it depends on how true we want to be to the specification and
whether our default is NS=0 or NS=1 when the security extension is present
or not.  The code currently assumes non-secure as the default state.

Is there a convention in qemu?  How closely do we attempt to stay to the
pseudo code provided in the spec?



On 14 May 2014 13:35, Fedorov Sergey  wrote:

>
> 14.05.2014 18:42, Greg Bellows пишет:
> > On 14 May 2014 00:53, Sergey Fedorov  wrote:
> >
> >> On 13.05.2014 20:15, Fabian Aggeler wrote:
> >>> arm_is_secure() function allows to determine CPU security state
> >>> if the CPU implements Security Extensions.
> >>>
> >>> Signed-off-by: Sergey Fedorov 
> >>> Signed-off-by: Fabian Aggeler 
> >>> ---
> >>>  target-arm/cpu.h | 15 +++
> >>>  1 file changed, 15 insertions(+)
> >>>
> >>> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> >>> index a56d3d6..6ea0432 100644
> >>> --- a/target-arm/cpu.h
> >>> +++ b/target-arm/cpu.h
> >>> @@ -640,6 +640,21 @@ static inline int arm_feature(CPUARMState *env,
> int
> >> feature)
> >>>  return (env->features & (1ULL << feature)) != 0;
> >>>  }
> >>>
> >>> +/* Return true if the processor is in secure state */
> >>> +static inline bool arm_is_secure(CPUARMState *env)
> >>> +{
> >>> +#if !defined(CONFIG_USER_ONLY)
> >>> +if (arm_feature(env, ARM_FEATURE_SECURITY_EXTENSIONS)) {
> >> I think feature test can be safely avoided here. Without this feature
> >> that should be no way to switch to monitor mode and to access SCR
> register.
> >>
> > I agree with the feature check here.  For correctness, we should only be
> > examining c1_scr if the security extension is enabled.   This is
> consistent
> > with only registering the SCR register if the feature is enabled.
>
> So this check will be done every time arm_is_secure() is called, e.g. on
> each MMU table walk.
>
> Moreover I've noticed that this function deviates from ARM ARM v7-AR
> description in section B1.5.1 which states: "The IsSecure() function
> returns TRUE if the processor is in Secure state, or if the
> implementation does not include
> the Security Extensions, and FALSE otherwise." Then there is a pseudo
> code for that function.
>
> >
> >>> +return ((env->uncached_cpsr & CPSR_M) == ARM_CPU_MODE_MON) ||
> >>> +!(env->cp15.c1_scr & 1);
> >>> +} else {
> >>> +return false;
> >>> +}
> >>> +#else
> >>> +return false;
> >> That is a good question how to treat user emulation: secure or
> >> non-secure. Perhaps assuming user emulation in secure state may simplify
> >> code in the following patches.
> >
> >>> +#endif
> >>> +}
> >>> +
> >>>  /* Return true if the specified exception level is running in AArch64
> >> state. */
> >>>  static inline bool arm_el_is_aa64(CPUARMState *env, int el)
> >>>  {
> >> Thanks,
> >> Sergey.
> >>
> >>
>
>


Re: [Qemu-devel] [PATCH v3.2 10/31] pc: pass QEMUMachineInitArgs to pc_memory_init

2014-05-14 Thread Eduardo Habkost
On Wed, May 14, 2014 at 05:43:14PM +0800, Hu Tao wrote:
> From: Paolo Bonzini 
> 
> Reviewed-by: Michael S. Tsirkin 
> Signed-off-by: Paolo Bonzini 
> Signed-off-by: Hu Tao 

Reviewed-by: Eduardo Habkost 

-- 
Eduardo



Re: [Qemu-devel] [PATCH v3.2 09/31] qmp: improve error reporting for -object and object-add

2014-05-14 Thread Eduardo Habkost
On Wed, May 14, 2014 at 05:43:13PM +0800, Hu Tao wrote:
> From: Paolo Bonzini 
> 
> Use QERR_INVALID_PARAMETER_VALUE for consistency.
> 
> Signed-off-by: Paolo Bonzini 
> Signed-off-by: Hu Tao 

Reviewed-by: Eduardo Habkost 

-- 
Eduardo



Re: [Qemu-devel] [Qemu-ppc] macio ide question/bug report

2014-05-14 Thread Mark Cave-Ayland

On 14/05/14 12:10, BALATON Zoltan wrote:


I've tried doing this and it seems that the cmd_read_toc_pma_atip
function returns all right (via the case 0 path) with a 20 byte result
array and calls ide_atapi_cmd_reply which takes the DMA path as
s->atapi_dma is set to 1 and the error comes from somewhere during
trying to DMA the result back to the client. I could not follow it so
I've only got a backtrace from where ide_atapi_cmd_error is called:


So this basically confirms that the DMA errors out because s->lba ==
-1 in the macio callback. FWIW you should probably ensure that
DEBUG_IDE_ATAPI is enabled when posting logs in future as this helps
show the interaction between the two systems.


The logs I've posted are with DEBUG_IDE_ATAPI, DEBUG_DBDMA and
DEBUG_MACIO already enabled...


Well sure, but not the ones in your last email - I had to go back 
several mails back into the thread to pull them out. Bear in mind the 
high volume of these lists means that a lot of people who could help 
won't have the time to do this.



Do you have any idea how to debug this further or does this help to tell
where is the problem? (I think the question is where does the -5 return
value come from?)


Well from this the cause is fairly easy to spot: ide_atapi_cmd_reply()
sets s->lba == -1 when called from cmd_read_toc_pma_atip(). And since
as you correctly point out this is a DMA request, it invokes the
start_dma function in macio's dbdma_ops (ide_dbdma_start), which kicks
the DBDMA engine into life.

I think the issue here is related to the fact that reading a TOC
doesn't actually involve reading physical blocks from disk (as the TOC
is placed directly in the buffer) and so the dma_bdrv_* functions
shouldn't actually be invoked in the first place. And because of the
DBDMA setup, ide_atapi_cmd_read_dma_cb() can't be used which is why
pmac_ide_transfer_cb() needs to do a lot of similar work itself. Maybe
you can find some clues there as to what the logic should be?


I'm afraid I still can't understand it. Is there a way to trace the path
after DBDMA engine is kicked? Where should I break to get some insight
on what is happening? (Maybe it's a dbdma bug not a macio one.)


Which part is it that's still confusing you? Putting breakpoints on 
pmac_ide_transfer() and pmac_ide_atapi_transfer_cb() will show you the 
iterations on each DMA request (be sure to compare against a "known 
good" example to understand how it should work first). If you can give 
more detail as to which bits are confusing, I will try my best to 
explain them.



ATB,

Mark.




Re: [Qemu-devel] [PATCH v3.2 06/31] man: improve -numa doc

2014-05-14 Thread Eduardo Habkost
On Wed, May 14, 2014 at 05:43:10PM +0800, Hu Tao wrote:
> From: Luiz Capitulino 
> 
> The -numa option documentation in qemu's manpage lacks the command-line
> options and some information regarding how it relates to options -m and
> -smp. This commit fills in the missing text.
> 
> Signed-off-by: Luiz Capitulino 
> Signed-off-by: Paolo Bonzini 
> Signed-off-by: Hu Tao 

Reviewed-by: Eduardo Habkost 

-- 
Eduardo



[Qemu-devel] [PATCH v3] serial-pci: Set prog interface field of pci config to 16550 compatible

2014-05-14 Thread BALATON Zoltan
Signed-off-by: BALATON Zoltan 
---

 v2: resubmission after pc-2.1 is added with the multiport case
 v3: added compatibility check to avoid changing earlier than pc-2.1

 hw/char/serial-pci.c | 11 +++
 include/hw/i386/pc.h | 15 +++
 2 files changed, 26 insertions(+)

diff --git a/hw/char/serial-pci.c b/hw/char/serial-pci.c
index 991c99f..ae57098 100644
--- a/hw/char/serial-pci.c
+++ b/hw/char/serial-pci.c
@@ -34,6 +34,7 @@
 typedef struct PCISerialState {
 PCIDevice dev;
 SerialState state;
+uint8_t compat;
 } PCISerialState;
 
 typedef struct PCIMultiSerialState {
@@ -44,6 +45,7 @@ typedef struct PCIMultiSerialState {
 SerialState  state[PCI_SERIAL_MAX_PORTS];
 uint32_t level[PCI_SERIAL_MAX_PORTS];
 qemu_irq *irqs;
+uint8_t  compat;
 } PCIMultiSerialState;
 
 static int serial_pci_init(PCIDevice *dev)
@@ -60,6 +62,9 @@ static int serial_pci_init(PCIDevice *dev)
 return -1;
 }
 
+if (!pci->compat) {
+pci->dev.config[PCI_CLASS_PROG] = 0x02; /* 16550 compatible */
+}
 pci->dev.config[PCI_INTERRUPT_PIN] = 0x01;
 s->irq = pci_allocate_irq(&pci->dev);
 
@@ -101,6 +106,9 @@ static int multi_serial_pci_init(PCIDevice *dev)
 assert(pci->ports > 0);
 assert(pci->ports <= PCI_SERIAL_MAX_PORTS);
 
+if (!pci->compat) {
+pci->dev.config[PCI_CLASS_PROG] = 0x02; /* 16550 compatible */
+}
 pci->dev.config[PCI_INTERRUPT_PIN] = 0x01;
 memory_region_init(&pci->iobar, OBJECT(pci), "multiserial", 8 * 
pci->ports);
 pci_register_bar(&pci->dev, 0, PCI_BASE_ADDRESS_SPACE_IO, &pci->iobar);
@@ -177,12 +185,14 @@ static const VMStateDescription vmstate_pci_multi_serial 
= {
 
 static Property serial_pci_properties[] = {
 DEFINE_PROP_CHR("chardev",  PCISerialState, state.chr),
+DEFINE_PROP_UINT8("compat",  PCISerialState, compat, 0),
 DEFINE_PROP_END_OF_LIST(),
 };
 
 static Property multi_2x_serial_pci_properties[] = {
 DEFINE_PROP_CHR("chardev1",  PCIMultiSerialState, state[0].chr),
 DEFINE_PROP_CHR("chardev2",  PCIMultiSerialState, state[1].chr),
+DEFINE_PROP_UINT8("compat",  PCIMultiSerialState, compat, 0),
 DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -191,6 +201,7 @@ static Property multi_4x_serial_pci_properties[] = {
 DEFINE_PROP_CHR("chardev2",  PCIMultiSerialState, state[1].chr),
 DEFINE_PROP_CHR("chardev3",  PCIMultiSerialState, state[2].chr),
 DEFINE_PROP_CHR("chardev4",  PCIMultiSerialState, state[3].chr),
+DEFINE_PROP_UINT8("compat",  PCIMultiSerialState, compat, 0),
 DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 32a7687..8fb8046 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -271,6 +271,21 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
 .driver   = "apic",\
 .property = "version",\
 .value= stringify(0x11),\
+},\
+{\
+.driver   = "pci-serial",\
+.property = "compat",\
+.value= stringify(1),\
+},\
+{\
+.driver   = "pci-serial-2x",\
+.property = "compat",\
+.value= stringify(1),\
+},\
+{\
+.driver   = "pci-serial-4x",\
+.property = "compat",\
+.value= stringify(1),\
 }
 
 #define PC_COMPAT_1_7 \
-- 
1.8.1.5




Re: [Qemu-devel] [PATCH 2/4] qapi: output visitor crashes qemu if it encounters a NULL value

2014-05-14 Thread Markus Armbruster
Luiz Capitulino  writes:

> On Wed, 14 May 2014 20:29:37 +0300
> Marcel Apfelbaum  wrote:
>
>> On Wed, 2014-05-14 at 19:00 +0200, Andreas Färber wrote:
>> > Am 13.05.2014 21:08, schrieb Eric Blake:
>> > > On 05/13/2014 11:36 AM, Andreas Färber wrote:
>> > >> Am 07.05.2014 16:42, schrieb Marcel Apfelbaum:
>> > >>> A NULL value is not added to visitor's stack, but there is no
>> > >>> check for that when the visitor tries to return that value,
>> > >>> leading to Qemu crash.
>> > >>> 
>> > >>> Reviewed-by: Eric Blake  Signed-off-by:
>> > >>> Marcel Apfelbaum 
>> > >> 
>> > >> Where does the Rb come from on this v1? Is it in any tree
>> > >> already?
>> > >> 
>> > > 
>> > > The (weak) R-b was here: 
>> > > https://lists.gnu.org/archive/html/qemu-devel/2014-02/msg02861.html
>> > 
>> > Thanks.
>> > > 
>> > So Luiz was okay with it too, but his last message seems to be
>> > indicating this needs to be fixed somewhere else, too:
>> > 
>> > https://lists.gnu.org/archive/html/qemu-devel/2014-02/msg05228.html
>> > https://lists.gnu.org/archive/html/qemu-devel/2014-03/msg00217.html
>> > 
>> > Can/should that be addressed as a follow-up? Or is there a test case
>> > that breaks?
>> Simple and "popular" test case: the user does not use the
>> -kernel-cmdline parameter.
>> The patch is needed because otherwise the main function will fail
>> if no value is passed by the user to string parameters. 
>> 
>> Regarding Luiz's concern, it can be a follow-up as I am not aware of
>> any problem with that.
>
> My concern was that I wasn't sure if this is the right fix for the issue
> or if it's papering over the real bug. I quickly checked the code and it
> seemed to make sense, but I didn't have time to study it deeper.

I can have a look tomorrow.

> We could ask Michael Roth or Anthony, but I wouldn't hold this series
> because of that. Here's my ACK if you need it:
>
> Acked-by: Luiz Capitulino 



Re: [Qemu-devel] [PATCH 11/18] block/vvfat: Propagate errors through init_directories()

2014-05-14 Thread Markus Armbruster
Eric Blake  writes:

> On 05/13/2014 10:02 AM, Markus Armbruster wrote:
>> Completes the conversion of the open method to Error started in commit
>> 015a103.
>> 
>> Signed-off-by: Markus Armbruster 
>> ---
>>  block/vvfat.c | 16 +---
>>  1 file changed, 9 insertions(+), 7 deletions(-)
>> 
>> @@ -892,8 +893,8 @@ static int init_directories(BDRVVVFATState* s,
>>  if (mapping->mode & MODE_DIRECTORY) {
>>  mapping->begin = cluster;
>>  if(read_directory(s, i)) {
>> -fprintf(stderr, "Could not read directory %s\n",
>> -mapping->path);
>> +error_setg(errp, "Could not read directory %s",
>> +   mapping->path);
>
> I see you fixed some TABs in the process; is it worth widening the fix
> to the rest of the 'if' statement?  I don't care either way, as long as
> checkpatch.pl didn't call you out (the new code is correct, even though
> the existing code was not).

checkpatch is happy.

The fewer lines in vvfat.c git blames on me, the happier I am ;)

> Reviewed-by: Eric Blake 

Thanks!



[Qemu-devel] qemu error

2014-05-14 Thread sonia verma
Hi

I'm getting below error when trying to boot the KVM with ethernet
bridging,kvm support and universel TUN enabled by the following command..

/usr/bin/qemu-system-ppc64 -m 512 -nographic -hda
/var/volatile/debian_lenny_powerpc_standard.qcow2


cannot manage 'OHCI USB controller' PCI device type 'usb':
>>  106b 3f (c 3 10)

>> =
>> OpenBIOS 1.0 [Aug 28 2012 05:40]
>> Configuration device id QEMU version 1 machine id 3
>> CPUs: 1
>> Memory: 512M
>> UUID: ----
>> CPU type PowerPC,970FX
Welcome to OpenBIOS v1.0 built on Aug 28 2012 05:40
Second-stage QUIK loader
Welcome to quik. mate is good.
Debian GNU/Linux PowerPCchosen/bootargs =
boot: `
Enter the kernel image name as [device:][partno]/path, where partno is a
number from 0 to 16.  Instead of /path you can type [mm-nn] to specify a
range of disk blocks (512B)
boot: Linux
initrd imagename = /initrd.img, mem_size: 4406840
initrd_start:
Starting at 51, , 1024
OF stdout device is: /pci@f000/mac-io@e/escc@13000/ch-b@13000
command line: console=ttyS0,9600 console=tty0
memory layout at init:
  alloc_bottom : 00c01000
  alloc_top: 2000
  alloc_top_hi : 2000
  rmo_top  : 2000
  ram_top  : 2000
Looking for displays
found display   : /pci@f000/QEMU,VGA@c, opening ... done
copying OF device tree ...
Building dt strings...
Building dt structure...
Device tree strings 0x00c02000 -> 0x00c024f0
Device tree struct  0x00c03000 -> 0x00c05000
Calling quiesce ...
returning from prom_init

The prompt is stuck at init and is not able to proceed even after wating
for 15-20 minutes.

Please help regarding this.


Thanks
Sonia


[Qemu-devel] [PATCH RESEND v4 14/18] target-i386: Add "migratable" property to "host" CPU model

2014-05-14 Thread Eduardo Habkost
This flag will allow the user to choose between two modes:
 * All flags that can be enabled on the host, even if unmigratable
   (migratable=no);
 * All flags that can be enabled on the host, known to QEMU,
   and migratable (migratable=yes).

The default is still migratable=false, to keep current behavior, but
this will be changed to migratable=true by another patch.

My plan was to support the "migratable" flag on all CPU classes, but
have the default to "false" on all CPU models except "host". However,
DeviceClass has no mechanism to allow a child class to have a different
property default from the parent class yet, so by now only the "host"
CPU model will support the "migratable" flag.

Signed-off-by: Eduardo Habkost 
---
 target-i386/cpu-qom.h |  5 +
 target-i386/cpu.c | 52 +--
 2 files changed, 51 insertions(+), 6 deletions(-)

diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h
index e9b3d57..016f90d 100644
--- a/target-i386/cpu-qom.h
+++ b/target-i386/cpu-qom.h
@@ -87,6 +87,11 @@ typedef struct X86CPU {
 bool hyperv_time;
 bool check_cpuid;
 bool enforce_cpuid;
+/* If set, only migratable flags will be accepted when "enforce" mode is
+ * used, and only migratable flags will be included in the "host"
+ * CPU model.
+ */
+bool migratable;
 
 /* if true the CPUID code directly forward host cache leaves to the guest 
*/
 bool cache_info_passthrough;
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 77d6d3c..105006b 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -338,6 +338,7 @@ typedef struct FeatureWordInfo {
 uint32_t cpuid_ecx;   /* Input ECX value for CPUID */
 int cpuid_reg;/* output register (R_* constant) */
 uint32_t tcg_features; /* Feature flags supported by TCG */
+uint32_t unmigratable_flags; /* Feature flags known to be unmigratable */
 } FeatureWordInfo;
 
 static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
@@ -461,6 +462,30 @@ void x86_cpu_compat_disable_kvm_features(FeatureWord w, 
uint32_t features)
 kvm_default_features[w] &= ~features;
 }
 
+/* Returns the set of feature flags that are supported and migratable by
+ * QEMU, for a given FeatureWord
+ */
+static uint32_t x86_cpu_get_migratable_flags(FeatureWord w)
+{
+uint32_t r = 0;
+int i;
+
+FeatureWordInfo *wi = &feature_word_info[w];
+for (i = 0; i < 32; i++) {
+uint32_t f = 1U << i;
+/* If the feature name is unknown, it is not supported by QEMU yet */
+if (!wi->feat_names[i]) {
+continue;
+}
+/* Skip features known to QEMU, but explicitly marked as unmigratable 
*/
+if (wi->unmigratable_flags & f) {
+continue;
+}
+r |= f;
+}
+return r;
+}
+
 void host_cpuid(uint32_t function, uint32_t count,
 uint32_t *eax, uint32_t *ebx, uint32_t *ecx, uint32_t *edx)
 {
@@ -1206,6 +1231,11 @@ static int cpu_x86_fill_model_id(char *str)
 
 static X86CPUDefinition host_cpudef;
 
+static Property x86_host_cpu_properties[] = {
+DEFINE_PROP_BOOL("migratable", X86CPU, migratable, false),
+DEFINE_PROP_END_OF_LIST()
+};
+
 /* class_init for the "host" CPU model
  *
  * This function may be called before KVM is initialized.
@@ -1213,6 +1243,7 @@ static X86CPUDefinition host_cpudef;
 static void host_x86_cpu_class_init(ObjectClass *oc, void *data)
 {
 X86CPUClass *xcc = X86_CPU_CLASS(oc);
+DeviceClass *dc = DEVICE_CLASS(oc);
 uint32_t eax = 0, ebx = 0, ecx = 0, edx = 0;
 
 host_cpuid(0x0, 0, &eax, &ebx, &ecx, &edx);
@@ -1228,12 +1259,14 @@ static void host_x86_cpu_class_init(ObjectClass *oc, 
void *data)
 xcc->cpu_def = &host_cpudef;
 host_cpudef.cache_info_passthrough = true;
 
+dc->props = x86_host_cpu_properties;
 /* level, xlevel, xlevel2, and the feature words are initialized on
  * instance_init, because they require KVM to be initialized.
  */
 }
 
-static uint32_t x86_cpu_get_supported_feature_word(FeatureWord w);
+static uint32_t x86_cpu_get_supported_feature_word(FeatureWord w,
+   bool migratable_only);
 
 static void host_x86_cpu_initfn(Object *obj)
 {
@@ -1257,7 +1290,7 @@ static void host_x86_cpu_initfn(Object *obj)
 
 for (w = 0; w < FEATURE_WORDS; w++) {
 env->features[w] =
-x86_cpu_get_supported_feature_word(w);
+x86_cpu_get_supported_feature_word(w, cpu->migratable);
 }
 object_property_set_bool(OBJECT(cpu), true, "pmu", &error_abort);
 }
@@ -1847,16 +1880,22 @@ CpuDefinitionInfoList *arch_query_cpu_definitions(Error 
**errp)
 return cpu_list;
 }
 
-static uint32_t x86_cpu_get_supported_feature_word(FeatureWord w)
+static uint32_t x86_cpu_get_supported_feature_word(FeatureWord w,
+   bool migratable_only)
 {
 FeatureWordInfo *wi = &feature_word_info[w];
+uint32

[Qemu-devel] [PATCH RESEND v4 12/18] target-i386: Support check/enforce flags in TCG mode, too

2014-05-14 Thread Eduardo Habkost
If enforce/check is specified in TCG mode, QEMU will ensure all CPU
features are supported by TCG, so no CPU feature is silently disabled.

Reviewed-by: Richard Henderson 
Signed-off-by: Eduardo Habkost 
---
Changes v1 -> v2:
 * Trivial rebase to latest qom-cpu (commit 90c5d39c)
   (Reviewed-by line kept)
Changes v2 -> v3:
 * Trivial rebase after QEMU 2.0 (onto commit 2d03b49)
   (Reviewed-by line kept)
---
 target-i386/cpu.c | 34 --
 1 file changed, 16 insertions(+), 18 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 8ebf8c5..d3c1663 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1265,8 +1265,9 @@ static int report_unavailable_features(FeatureWord w, 
uint32_t mask)
 if (1 << i & mask) {
 const char *reg = get_register_name_32(f->cpuid_reg);
 assert(reg);
-fprintf(stderr, "warning: host doesn't support requested feature: "
+fprintf(stderr, "warning: %s doesn't support requested feature: "
 "CPUID.%02XH:%s%s%s [bit %d]\n",
+kvm_enabled() ? "host" : "TCG",
 f->cpuid_eax, reg,
 f->feat_names[i] ? "." : "",
 f->feat_names[i] ? f->feat_names[i] : "", i);
@@ -1834,17 +1835,18 @@ CpuDefinitionInfoList *arch_query_cpu_definitions(Error 
**errp)
 static uint32_t x86_cpu_get_supported_feature_word(FeatureWord w)
 {
 FeatureWordInfo *wi = &feature_word_info[w];
-assert(kvm_enabled());
-return kvm_arch_get_supported_cpuid(kvm_state, wi->cpuid_eax,
-   wi->cpuid_ecx,
-   wi->cpuid_reg);
+if (kvm_enabled()) {
+return kvm_arch_get_supported_cpuid(kvm_state, wi->cpuid_eax,
+   wi->cpuid_ecx,
+   wi->cpuid_reg);
+} else {
+return wi->tcg_features;
+}
 }
 
 /* Filters CPU feature words based on host availability of each feature
  *
  * Returns 0 if all flags are supported by the host, non-zero otherwise.
- *
- * This function may be called only if KVM is enabled.
  */
 static int x86_cpu_filter_features(X86CPU *cpu)
 {
@@ -2598,17 +2600,13 @@ static void x86_cpu_realizefn(DeviceState *dev, Error 
**errp)
& CPUID_EXT2_AMD_ALIASES);
 }
 
-if (!kvm_enabled()) {
-FeatureWord w;
-for (w = 0; w < FEATURE_WORDS; w++) {
-env->features[w] &= feature_word_info[w].tcg_features;
-}
-} else {
-if (x86_cpu_filter_features(cpu) && cpu->enforce_cpuid) {
-error_setg(&local_err,
-   "Host's CPU doesn't support requested features");
-goto out;
-}
+
+if (x86_cpu_filter_features(cpu) && cpu->enforce_cpuid) {
+error_setg(&local_err,
+   kvm_enabled() ?
+   "Host doesn't support requested features" :
+   "TCG doesn't support requested features");
+goto out;
 }
 
 #ifndef CONFIG_USER_ONLY
-- 
1.9.0




[Qemu-devel] [PATCH RESEND v4 18/18] target-i386: support "invariant tsc" flag

2014-05-14 Thread Eduardo Habkost
From: Marcelo Tosatti 

Expose "Invariant TSC" flag, if KVM is enabled. From Intel documentation:

17.13.1 Invariant TSC The time stamp counter in newer processors may
support an enhancement, referred to as invariant TSC. Processor’s
support for invariant TSC is indicated by CPUID.8007H:EDX[8].
The invariant TSC will run at a constant rate in all ACPI P-, C-.
and T-states. This is the architectural behavior moving forward. On
processors with invariant TSC support, the OS may use the TSC for wall
clock timer services (instead of ACPI or HPET timers). TSC reads are
much more efficient and do not incur the overhead associated with a ring
transition or access to a platform resource.

Signed-off-by: Marcelo Tosatti 
[ehabkost: redo feature filtering to use .tcg_features]
[ehabkost: add CPUID_APM_INVTSC macro, add it to .unmigratable_flags]
Signed-off-by: Eduardo Habkost 
---
 target-i386/cpu.c | 25 +
 target-i386/cpu.h |  4 
 2 files changed, 29 insertions(+)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index d43209e..f8e7eb1 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -262,6 +262,17 @@ static const char *cpuid_7_0_ebx_feature_name[] = {
 NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
 };
 
+static const char *cpuid_apm_edx_feature_name[] = {
+NULL, NULL, NULL, NULL,
+NULL, NULL, NULL, NULL,
+"invtsc", NULL, NULL, NULL,
+NULL, NULL, NULL, NULL,
+NULL, NULL, NULL, NULL,
+NULL, NULL, NULL, NULL,
+NULL, NULL, NULL, NULL,
+NULL, NULL, NULL, NULL,
+};
+
 #define I486_FEATURES (CPUID_FP87 | CPUID_VME | CPUID_PSE)
 #define PENTIUM_FEATURES (I486_FEATURES | CPUID_DE | CPUID_TSC | \
   CPUID_MSR | CPUID_MCE | CPUID_CX8 | CPUID_MMX | CPUID_APIC)
@@ -329,6 +340,7 @@ static const char *cpuid_7_0_ebx_feature_name[] = {
   CPUID_7_0_EBX_FSGSBASE, CPUID_7_0_EBX_HLE, CPUID_7_0_EBX_AVX2,
   CPUID_7_0_EBX_ERMS, CPUID_7_0_EBX_INVPCID, CPUID_7_0_EBX_RTM,
   CPUID_7_0_EBX_RDSEED */
+#define TCG_APM_FEATURES 0
 
 
 typedef struct FeatureWordInfo {
@@ -384,6 +396,13 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
 .cpuid_reg = R_EBX,
 .tcg_features = TCG_7_0_EBX_FEATURES,
 },
+[FEAT_8000_0007_EDX] = {
+.feat_names = cpuid_apm_edx_feature_name,
+.cpuid_eax = 0x8007,
+.cpuid_reg = R_EDX,
+.tcg_features = TCG_APM_FEATURES,
+.unmigratable_flags = CPUID_APM_INVTSC,
+},
 };
 
 typedef struct X86RegisterInfo32 {
@@ -2400,6 +2419,12 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, 
uint32_t count,
(AMD_ENC_ASSOC(L3_ASSOCIATIVITY) << 12) | \
(L3_LINES_PER_TAG << 8) | (L3_LINE_SIZE);
 break;
+case 0x8007:
+*eax = 0;
+*ebx = 0;
+*ecx = 0;
+*edx = env->features[FEAT_8000_0007_EDX];
+break;
 case 0x8008:
 /* virtual & phys address size in low 2 bytes. */
 /* XXX: This value must match the one used in the MMU code. */
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 2a22a7d..1bb98e6 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -398,6 +398,7 @@ typedef enum FeatureWord {
 FEAT_7_0_EBX,   /* CPUID[EAX=7,ECX=0].EBX */
 FEAT_8000_0001_EDX, /* CPUID[8000_0001].EDX */
 FEAT_8000_0001_ECX, /* CPUID[8000_0001].ECX */
+FEAT_8000_0007_EDX, /* CPUID[8000_0007].EDX */
 FEAT_C000_0001_EDX, /* CPUID[C000_0001].EDX */
 FEAT_KVM,   /* CPUID[4000_0001].EAX (KVM_CPUID_FEATURES) */
 FEAT_SVM,   /* CPUID[8000_000A].EDX */
@@ -557,6 +558,9 @@ typedef uint32_t FeatureWordArray[FEATURE_WORDS];
 #define CPUID_7_0_EBX_ADX  (1U << 19)
 #define CPUID_7_0_EBX_SMAP (1U << 20)
 
+/* CPUID[0x8007].EDX flags: */
+#define CPUID_APM_INVTSC   (1U << 8)
+
 #define CPUID_VENDOR_SZ  12
 
 #define CPUID_VENDOR_INTEL_1 0x756e6547 /* "Genu" */
-- 
1.9.0




[Qemu-devel] [PATCH RESEND v4 15/18] target-i386: Set migratable=yes by default

2014-05-14 Thread Eduardo Habkost
Having only migratable flags reported by default on the "host" CPU model
is safer for the following reasons:

 * Existing users may expect "-cpu host" to be migration-safe, if they
   take care of always using compatible host CPUs, host kernels, and
   QEMU versions.
 * Users who don't care aboug migration and want to enable all features
   supported by the host kernel can simply change their setup to use
   migratable=no.

Signed-off-by: Eduardo Habkost 
---
 target-i386/cpu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 105006b..d43209e 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1232,7 +1232,7 @@ static int cpu_x86_fill_model_id(char *str)
 static X86CPUDefinition host_cpudef;
 
 static Property x86_host_cpu_properties[] = {
-DEFINE_PROP_BOOL("migratable", X86CPU, migratable, false),
+DEFINE_PROP_BOOL("migratable", X86CPU, migratable, true),
 DEFINE_PROP_END_OF_LIST()
 };
 
-- 
1.9.0




[Qemu-devel] [PATCH RESEND v4 13/18] target-i386: Support "-cpu host" in TCG mode

2014-05-14 Thread Eduardo Habkost
As "-cpu host" simply means "enable every bit that can be enabled on
this host", we can emulate similar behavior even if KVM is not enabled.
We just need to set all feature bits supported by TCG, accordingly.

Signed-off-by: Eduardo Habkost 
---
Changes v2:
 * Coding style fix (break long lines)
---
 target-i386/cpu.c | 35 +--
 1 file changed, 25 insertions(+), 10 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index d3c1663..77d6d3c 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -274,6 +274,16 @@ static const char *cpuid_7_0_ebx_feature_name[] = {
   CPUID_PAT | CPUID_FXSR | CPUID_MMX | CPUID_SSE | CPUID_SSE2 | \
   CPUID_PAE | CPUID_SEP | CPUID_APIC)
 
+/* Maximum CPUID level values for TCG: */
+
+/* CPUID level 7 is needed for TCG_7_0_EBX_FEATURES */
+#define TCG_MAX_LEVEL7
+/* 0x800A is needed for CPUID_EXT3_SVM */
+#define TCG_MAX_XLEVEL   0x800A
+/* TCG_EXT4_FEATURES is 0 */
+#define TCG_MAX_XLEVEL2  0
+
+
 #define TCG_FEATURES (CPUID_FP87 | CPUID_PSE | CPUID_TSC | CPUID_MSR | \
   CPUID_PAE | CPUID_MCE | CPUID_CX8 | CPUID_APIC | CPUID_SEP | \
   CPUID_MTRR | CPUID_PGE | CPUID_MCA | CPUID_CMOV | CPUID_PAT | \
@@ -1205,8 +1215,6 @@ static void host_x86_cpu_class_init(ObjectClass *oc, void 
*data)
 X86CPUClass *xcc = X86_CPU_CLASS(oc);
 uint32_t eax = 0, ebx = 0, ecx = 0, edx = 0;
 
-xcc->kvm_required = true;
-
 host_cpuid(0x0, 0, &eax, &ebx, &ecx, &edx);
 x86_cpu_vendor_words2str(host_cpudef.vendor, ebx, edx, ecx);
 
@@ -1225,6 +1233,8 @@ static void host_x86_cpu_class_init(ObjectClass *oc, void 
*data)
  */
 }
 
+static uint32_t x86_cpu_get_supported_feature_word(FeatureWord w);
+
 static void host_x86_cpu_initfn(Object *obj)
 {
 X86CPU *cpu = X86_CPU(obj);
@@ -1232,17 +1242,22 @@ static void host_x86_cpu_initfn(Object *obj)
 KVMState *s = kvm_state;
 FeatureWord w;
 
-assert(kvm_enabled());
-
-env->cpuid_level = kvm_arch_get_supported_cpuid(s, 0x0, 0, R_EAX);
-env->cpuid_xlevel = kvm_arch_get_supported_cpuid(s, 0x8000, 0, R_EAX);
-env->cpuid_xlevel2 = kvm_arch_get_supported_cpuid(s, 0xC000, 0, R_EAX);
+if (kvm_enabled()) {
+env->cpuid_level =
+kvm_arch_get_supported_cpuid(s, 0x0, 0, R_EAX);
+env->cpuid_xlevel =
+kvm_arch_get_supported_cpuid(s, 0x8000, 0, R_EAX);
+env->cpuid_xlevel2 =
+kvm_arch_get_supported_cpuid(s, 0xC000, 0, R_EAX);
+} else {
+env->cpuid_level = TCG_MAX_LEVEL;
+env->cpuid_xlevel = TCG_MAX_XLEVEL;
+env->cpuid_xlevel2 = TCG_MAX_XLEVEL2;
+}
 
 for (w = 0; w < FEATURE_WORDS; w++) {
-FeatureWordInfo *wi = &feature_word_info[w];
 env->features[w] =
-kvm_arch_get_supported_cpuid(s, wi->cpuid_eax, wi->cpuid_ecx,
- wi->cpuid_reg);
+x86_cpu_get_supported_feature_word(w);
 }
 object_property_set_bool(OBJECT(cpu), true, "pmu", &error_abort);
 }
-- 
1.9.0




[Qemu-devel] [PATCH RESEND v4 11/18] target-i386: Loop-based feature word filtering in TCG mode

2014-05-14 Thread Eduardo Habkost
Instead of manually filtering each feature word, add a tcg_features
field to FeatureWordInfo, and use that field to filter all feature words
in TCG mode.

Reviewed-by: Richard Henderson 
Signed-off-by: Eduardo Habkost 
---
Changes v1 -> v2:
 * Trivial rebase to latest qom-cpu (commit 90c5d39c)
   (Reviewed-by line kept)
Changes v2 -> v3:
 * Trivial rebase after QEMU 2.0 (onto commit 2d03b49)
   (Reviewed-by line kept)
---
 target-i386/cpu.c | 21 +
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 816adc2..8ebf8c5 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -327,42 +327,51 @@ typedef struct FeatureWordInfo {
 bool cpuid_needs_ecx; /* CPUID instruction uses ECX as input */
 uint32_t cpuid_ecx;   /* Input ECX value for CPUID */
 int cpuid_reg;/* output register (R_* constant) */
+uint32_t tcg_features; /* Feature flags supported by TCG */
 } FeatureWordInfo;
 
 static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
 [FEAT_1_EDX] = {
 .feat_names = feature_name,
 .cpuid_eax = 1, .cpuid_reg = R_EDX,
+.tcg_features = TCG_FEATURES,
 },
 [FEAT_1_ECX] = {
 .feat_names = ext_feature_name,
 .cpuid_eax = 1, .cpuid_reg = R_ECX,
+.tcg_features = TCG_EXT_FEATURES,
 },
 [FEAT_8000_0001_EDX] = {
 .feat_names = ext2_feature_name,
 .cpuid_eax = 0x8001, .cpuid_reg = R_EDX,
+.tcg_features = TCG_EXT2_FEATURES,
 },
 [FEAT_8000_0001_ECX] = {
 .feat_names = ext3_feature_name,
 .cpuid_eax = 0x8001, .cpuid_reg = R_ECX,
+.tcg_features = TCG_EXT3_FEATURES,
 },
 [FEAT_C000_0001_EDX] = {
 .feat_names = ext4_feature_name,
 .cpuid_eax = 0xC001, .cpuid_reg = R_EDX,
+.tcg_features = TCG_EXT4_FEATURES,
 },
 [FEAT_KVM] = {
 .feat_names = kvm_feature_name,
 .cpuid_eax = KVM_CPUID_FEATURES, .cpuid_reg = R_EAX,
+.tcg_features = TCG_KVM_FEATURES,
 },
 [FEAT_SVM] = {
 .feat_names = svm_feature_name,
 .cpuid_eax = 0x800A, .cpuid_reg = R_EDX,
+.tcg_features = TCG_SVM_FEATURES,
 },
 [FEAT_7_0_EBX] = {
 .feat_names = cpuid_7_0_ebx_feature_name,
 .cpuid_eax = 7,
 .cpuid_needs_ecx = true, .cpuid_ecx = 0,
 .cpuid_reg = R_EBX,
+.tcg_features = TCG_7_0_EBX_FEATURES,
 },
 };
 
@@ -2590,14 +2599,10 @@ static void x86_cpu_realizefn(DeviceState *dev, Error 
**errp)
 }
 
 if (!kvm_enabled()) {
-env->features[FEAT_1_EDX] &= TCG_FEATURES;
-env->features[FEAT_1_ECX] &= TCG_EXT_FEATURES;
-env->features[FEAT_7_0_EBX] &= TCG_7_0_EBX_FEATURES;
-env->features[FEAT_8000_0001_EDX] &= TCG_EXT2_FEATURES;
-env->features[FEAT_8000_0001_ECX] &= TCG_EXT3_FEATURES;
-env->features[FEAT_SVM] &= TCG_SVM_FEATURES;
-env->features[FEAT_KVM] &= TCG_KVM_FEATURES;
-env->features[FEAT_C000_0001_EDX] &= TCG_EXT4_FEATURES;
+FeatureWord w;
+for (w = 0; w < FEATURE_WORDS; w++) {
+env->features[w] &= feature_word_info[w].tcg_features;
+}
 } else {
 if (x86_cpu_filter_features(cpu) && cpu->enforce_cpuid) {
 error_setg(&local_err,
-- 
1.9.0




[Qemu-devel] [PATCH RESEND v4 10/18] target-i386: Loop-based copying and setting/unsetting of feature words

2014-05-14 Thread Eduardo Habkost
Now that we have the feature word arrays, we don't need to manually copy
each array item, we can simply iterate through each feature word.

Signed-off-by: Eduardo Habkost 
---
 target-i386/cpu.c | 35 ++-
 1 file changed, 10 insertions(+), 25 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index ccd05ad..816adc2 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1652,6 +1652,7 @@ static void x86_cpu_parse_featurestr(CPUState *cs, char 
*features,
 {
 X86CPU *cpu = X86_CPU(cs);
 char *featurestr; /* Single 'key=value" string being parsed */
+FeatureWord w;
 /* Features to be added */
 FeatureWordArray plus_features = { 0 };
 /* Features to be removed */
@@ -1731,22 +1732,11 @@ static void x86_cpu_parse_featurestr(CPUState *cs, char 
*features,
 }
 featurestr = strtok(NULL, ",");
 }
-env->features[FEAT_1_EDX] |= plus_features[FEAT_1_EDX];
-env->features[FEAT_1_ECX] |= plus_features[FEAT_1_ECX];
-env->features[FEAT_8000_0001_EDX] |= plus_features[FEAT_8000_0001_EDX];
-env->features[FEAT_8000_0001_ECX] |= plus_features[FEAT_8000_0001_ECX];
-env->features[FEAT_C000_0001_EDX] |= plus_features[FEAT_C000_0001_EDX];
-env->features[FEAT_KVM] |= plus_features[FEAT_KVM];
-env->features[FEAT_SVM] |= plus_features[FEAT_SVM];
-env->features[FEAT_7_0_EBX] |= plus_features[FEAT_7_0_EBX];
-env->features[FEAT_1_EDX] &= ~minus_features[FEAT_1_EDX];
-env->features[FEAT_1_ECX] &= ~minus_features[FEAT_1_ECX];
-env->features[FEAT_8000_0001_EDX] &= ~minus_features[FEAT_8000_0001_EDX];
-env->features[FEAT_8000_0001_ECX] &= ~minus_features[FEAT_8000_0001_ECX];
-env->features[FEAT_C000_0001_EDX] &= ~minus_features[FEAT_C000_0001_EDX];
-env->features[FEAT_KVM] &= ~minus_features[FEAT_KVM];
-env->features[FEAT_SVM] &= ~minus_features[FEAT_SVM];
-env->features[FEAT_7_0_EBX] &= ~minus_features[FEAT_7_0_EBX];
+
+for (w = 0; w < FEATURE_WORDS; w++) {
+env->features[w] |= plus_features[w];
+env->features[w] &= ~minus_features[w];
+}
 
 out:
 return;
@@ -1876,24 +1866,19 @@ static void x86_cpu_load_def(X86CPU *cpu, 
X86CPUDefinition *def, Error **errp)
 CPUX86State *env = &cpu->env;
 const char *vendor;
 char host_vendor[CPUID_VENDOR_SZ + 1];
+FeatureWord w;
 
 object_property_set_int(OBJECT(cpu), def->level, "level", errp);
 object_property_set_int(OBJECT(cpu), def->family, "family", errp);
 object_property_set_int(OBJECT(cpu), def->model, "model", errp);
 object_property_set_int(OBJECT(cpu), def->stepping, "stepping", errp);
-env->features[FEAT_1_EDX] = def->features[FEAT_1_EDX];
-env->features[FEAT_1_ECX] = def->features[FEAT_1_ECX];
-env->features[FEAT_8000_0001_EDX] = def->features[FEAT_8000_0001_EDX];
-env->features[FEAT_8000_0001_ECX] = def->features[FEAT_8000_0001_ECX];
 object_property_set_int(OBJECT(cpu), def->xlevel, "xlevel", errp);
-env->features[FEAT_KVM] = def->features[FEAT_KVM];
-env->features[FEAT_SVM] = def->features[FEAT_SVM];
-env->features[FEAT_C000_0001_EDX] = def->features[FEAT_C000_0001_EDX];
-env->features[FEAT_7_0_EBX] = def->features[FEAT_7_0_EBX];
 env->cpuid_xlevel2 = def->xlevel2;
 cpu->cache_info_passthrough = def->cache_info_passthrough;
-
 object_property_set_str(OBJECT(cpu), def->model_id, "model-id", errp);
+for (w = 0; w < FEATURE_WORDS; w++) {
+env->features[w] = def->features[w];
+}
 
 /* Special cases not set in the X86CPUDefinition structs: */
 if (kvm_enabled()) {
-- 
1.9.0




[Qemu-devel] [PATCH RESEND v4 05/18] target-i386: Isolate KVM-specific code on CPU feature filtering logic

2014-05-14 Thread Eduardo Habkost
This will allow us to re-use the feature filtering logic (and the
check/enforce flag logic) for TCG.

Reviewed-by: Richard Henderson 
Signed-off-by: Eduardo Habkost 
---
Changes v1 -> v2:
 * Trivial rebase to latest qom-cpu (commit 90c5d39c)
   (Reviewed-by line kept)
Changes v2 -> v3:
 * Trivial rebase after QEMU 2.0 (onto commit 2d03b49)
   (Reviewed-by line kept)
---
 target-i386/cpu.c | 21 -
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index b097c0d..4dd522a 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1821,26 +1821,29 @@ CpuDefinitionInfoList *arch_query_cpu_definitions(Error 
**errp)
 return cpu_list;
 }
 
+static uint32_t x86_cpu_get_supported_feature_word(FeatureWord w)
+{
+FeatureWordInfo *wi = &feature_word_info[w];
+assert(kvm_enabled());
+return kvm_arch_get_supported_cpuid(kvm_state, wi->cpuid_eax,
+   wi->cpuid_ecx,
+   wi->cpuid_reg);
+}
+
 /* Filters CPU feature words based on host availability of each feature
  *
  * Returns 0 if all flags are supported by the host, non-zero otherwise.
  *
  * This function may be called only if KVM is enabled.
  */
-static int filter_features_for_kvm(X86CPU *cpu)
+static int x86_cpu_filter_features(X86CPU *cpu)
 {
 CPUX86State *env = &cpu->env;
-KVMState *s = kvm_state;
 FeatureWord w;
 int rv = 0;
 
-assert(kvm_enabled());
-
 for (w = 0; w < FEATURE_WORDS; w++) {
-FeatureWordInfo *wi = &feature_word_info[w];
-uint32_t host_feat = kvm_arch_get_supported_cpuid(s, wi->cpuid_eax,
- wi->cpuid_ecx,
- wi->cpuid_reg);
+uint32_t host_feat = x86_cpu_get_supported_feature_word(w);
 uint32_t requested_features = env->features[w];
 env->features[w] &= host_feat;
 cpu->filtered_features[w] = requested_features & ~env->features[w];
@@ -2601,7 +2604,7 @@ static void x86_cpu_realizefn(DeviceState *dev, Error 
**errp)
 env->features[FEAT_8000_0001_ECX] &= TCG_EXT3_FEATURES;
 env->features[FEAT_SVM] &= TCG_SVM_FEATURES;
 } else {
-if (filter_features_for_kvm(cpu) && cpu->enforce_cpuid) {
+if (x86_cpu_filter_features(cpu) && cpu->enforce_cpuid) {
 error_setg(&local_err,
"Host's CPU doesn't support requested features");
 goto out;
-- 
1.9.0




[Qemu-devel] [PATCH RESEND v4 17/18] target-i386: block migration and savevm if invariant tsc is exposed

2014-05-14 Thread Eduardo Habkost
From: Marcelo Tosatti 

Invariant TSC documentation mentions that "invariant TSC will run at a
constant rate in all ACPI P-, C-. and T-states".

This is not the case if migration to a host with different TSC frequency
is allowed, or if savevm is performed. So block migration/savevm.

Cc: Juan Quintela 
Signed-off-by: Marcelo Tosatti 
Reviewed-by: Eduardo Habkost 
Signed-off-by: Eduardo Habkost 
---
 target-i386/cpu-qom.h |  2 +-
 target-i386/kvm.c | 13 +
 target-i386/machine.c |  2 +-
 3 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h
index 016f90d..473d803 100644
--- a/target-i386/cpu-qom.h
+++ b/target-i386/cpu-qom.h
@@ -121,7 +121,7 @@ static inline X86CPU *x86_env_get_cpu(CPUX86State *env)
 #define ENV_OFFSET offsetof(X86CPU, env)
 
 #ifndef CONFIG_USER_ONLY
-extern const struct VMStateDescription vmstate_x86_cpu;
+extern struct VMStateDescription vmstate_x86_cpu;
 #endif
 
 /**
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 4389959..99cc7e3 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -33,6 +33,8 @@
 #include "exec/ioport.h"
 #include 
 #include "hw/pci/pci.h"
+#include "migration/migration.h"
+#include "qapi/qmp/qerror.h"
 
 //#define DEBUG_KVM
 
@@ -447,6 +449,8 @@ static bool hyperv_enabled(X86CPU *cpu)
 cpu->hyperv_relaxed_timing);
 }
 
+Error *invtsc_mig_blocker;
+
 #define KVM_MAX_CPUID_ENTRIES  100
 
 int kvm_arch_init_vcpu(CPUState *cs)
@@ -702,6 +706,15 @@ int kvm_arch_init_vcpu(CPUState *cs)
   !!(c->ecx & CPUID_EXT_SMX);
 }
 
+c = cpuid_find_entry(&cpuid_data.cpuid, 0x8007, 0);
+if (c && (c->edx & 1<<8) && invtsc_mig_blocker == NULL) {
+/* for migration */
+error_set(&invtsc_mig_blocker, QERR_MIGRATION_NOT_SUPPORTED, "cpu");
+migrate_add_blocker(invtsc_mig_blocker);
+/* for savevm */
+vmstate_x86_cpu.unmigratable = 1;
+}
+
 cpuid_data.cpuid.padding = 0;
 r = kvm_vcpu_ioctl(cs, KVM_SET_CPUID2, &cpuid_data);
 if (r) {
diff --git a/target-i386/machine.c b/target-i386/machine.c
index 168cab6..4d4c023 100644
--- a/target-i386/machine.c
+++ b/target-i386/machine.c
@@ -613,7 +613,7 @@ static const VMStateDescription vmstate_msr_hyperv_time = {
 }
 };
 
-const VMStateDescription vmstate_x86_cpu = {
+VMStateDescription vmstate_x86_cpu = {
 .name = "cpu",
 .version_id = 12,
 .minimum_version_id = 3,
-- 
1.9.0




[Qemu-devel] [PATCH RESEND v4 08/18] target-i386: Filter KVM and 0xC0000001 features on TCG

2014-05-14 Thread Eduardo Habkost
TCG doesn't support any of the feature flags on FEAT_KVM and
FEAT_C000_0001_EDX feature words, so clear all bits on those feature
words.

Reviewed-by: Richard Henderson 
Signed-off-by: Eduardo Habkost 
---
Changes v1 -> v2:
 * Trivial rebase to latest qom-cpu (commit 90c5d39c)
   (Reviewed-by line kept)
Changes v2 -> v3:
 * Trivial rebase after QEMU 2.0 (onto commit 2d03b49)
   (Reviewed-by line kept)
---
 target-i386/cpu.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 8d0ae12..0e9f3ea 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -587,7 +587,9 @@ struct X86CPUDefinition {
   CPUID_EXT2_PDPE1GB */
 #define TCG_EXT3_FEATURES (CPUID_EXT3_LAHF_LM | CPUID_EXT3_SVM | \
   CPUID_EXT3_CR8LEG | CPUID_EXT3_ABM | CPUID_EXT3_SSE4A)
+#define TCG_EXT4_FEATURES 0
 #define TCG_SVM_FEATURES 0
+#define TCG_KVM_FEATURES 0
 #define TCG_7_0_EBX_FEATURES (CPUID_7_0_EBX_SMEP | CPUID_7_0_EBX_SMAP | \
   CPUID_7_0_EBX_BMI1 | CPUID_7_0_EBX_BMI2 | CPUID_7_0_EBX_ADX)
   /* missing:
@@ -2608,6 +2610,8 @@ static void x86_cpu_realizefn(DeviceState *dev, Error 
**errp)
 env->features[FEAT_8000_0001_EDX] &= TCG_EXT2_FEATURES;
 env->features[FEAT_8000_0001_ECX] &= TCG_EXT3_FEATURES;
 env->features[FEAT_SVM] &= TCG_SVM_FEATURES;
+env->features[FEAT_KVM] &= TCG_KVM_FEATURES;
+env->features[FEAT_C000_0001_EDX] &= TCG_EXT4_FEATURES;
 } else {
 if (x86_cpu_filter_features(cpu) && cpu->enforce_cpuid) {
 error_setg(&local_err,
-- 
1.9.0




[Qemu-devel] [PATCH RESEND v4 00/18] target-i386: CPU feature flag queue

2014-05-14 Thread Eduardo Habkost
(Resending due to complete lack of feedback on v4 submission from 15 days ago.)

This started as a TCG vs KVM feature flag code cleanup, but now it is a queue
which includes other feature-flag-related patches that depend on each other.

Changes v3 -> v4:
 * New patch: target-i386: kvm: Don't enable MONITOR by default on any CPU model
 * New patch: target-i386: Add "migratable" property to "host" CPU model
 * New patch: target-i386: Set migratable=yes by default
 * New patch: savevm: check vmsd for migratability status
 * New patch: target-i386: Loop-based copying and setting/unsetting of feature 
words
 * Patch changed to use the new .migratable_flags field:
   * target-i386: support "invariant tsc" flag

Changes v2 -> v3:
 * Rebase after QEMU v2.0.0 (onto commit 2d03b49)
 * Added new patch: target-i386: support "invariant tsc" flag
 * Added new patch: target-i386: Support "-cpu host" in TCG mode

Changes v1 -> v2:
 * Rebase to latest qom-cpu (commit 90c5d39c)

Cc: Igor Mammedov 
Cc: Andreas Färber 
Cc: Paolo Bonzini 
Cc: Aurelien Jarno 
Cc: Richard Henderson 
Cc: Marcelo Tosatti 

Eduardo Habkost (15):
  target-i386: kvm: Don't enable MONITOR by default on any CPU model
  target-i386: Simplify reporting of unavailable features
  target-i386: Merge feature filtering/checking functions
  target-i386: Pass FeatureWord argument to
report_unavailable_features()
  target-i386: Isolate KVM-specific code on CPU feature filtering logic
  target-i386: Make TCG feature filtering more readable
  target-i386: Filter FEAT_7_0_EBX TCG features too
  target-i386: Filter KVM and 0xC001 features on TCG
  target-i386: Define TCG_*_FEATURES earlier on cpu.c
  target-i386: Loop-based copying and setting/unsetting of feature words
  target-i386: Loop-based feature word filtering in TCG mode
  target-i386: Support check/enforce flags in TCG mode, too
  target-i386: Support "-cpu host" in TCG mode
  target-i386: Add "migratable" property to "host" CPU model
  target-i386: Set migratable=yes by default

Marcelo Tosatti (3):
  savevm: check vmsd for migratability status
  target-i386: block migration and savevm if invariant tsc is exposed
  target-i386: support "invariant tsc" flag

 savevm.c  |   5 +-
 target-i386/cpu-qom.h |   7 +-
 target-i386/cpu.c | 358 ++
 target-i386/cpu.h |   4 +
 target-i386/kvm.c |  13 ++
 target-i386/machine.c |   2 +-
 6 files changed, 240 insertions(+), 149 deletions(-)

-- 
1.9.0




[Qemu-devel] [PATCH RESEND v4 16/18] savevm: check vmsd for migratability status

2014-05-14 Thread Eduardo Habkost
From: Marcelo Tosatti 

Check vmsd for unmigratable field, allowing migratibility status
to be modified after vmstate_register.

Cc: Juan Quintela 
Signed-off-by: Marcelo Tosatti 
Reviewed-by: Eduardo Habkost 
Signed-off-by: Eduardo Habkost 
---
 savevm.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/savevm.c b/savevm.c
index da8aa24..c578e42 100644
--- a/savevm.c
+++ b/savevm.c
@@ -232,7 +232,6 @@ typedef struct SaveStateEntry {
 const VMStateDescription *vmsd;
 void *opaque;
 CompatEntry *compat;
-int no_migrate;
 int is_ram;
 } SaveStateEntry;
 
@@ -292,7 +291,6 @@ int register_savevm_live(DeviceState *dev,
 se->ops = ops;
 se->opaque = opaque;
 se->vmsd = NULL;
-se->no_migrate = 0;
 /* if this is a live_savem then set is_ram */
 if (ops->save_live_setup != NULL) {
 se->is_ram = 1;
@@ -383,7 +381,6 @@ int vmstate_register_with_alias_id(DeviceState *dev, int 
instance_id,
 se->opaque = opaque;
 se->vmsd = vmsd;
 se->alias_id = alias_id;
-se->no_migrate = vmsd->unmigratable;
 
 if (dev) {
 char *id = qdev_get_dev_path(dev);
@@ -452,7 +449,7 @@ bool qemu_savevm_state_blocked(Error **errp)
 SaveStateEntry *se;
 
 QTAILQ_FOREACH(se, &savevm_handlers, entry) {
-if (se->no_migrate) {
+if (se->vmsd && se->vmsd->unmigratable) {
 error_setg(errp, "State blocked by non-migratable device '%s'",
se->idstr);
 return true;
-- 
1.9.0




[Qemu-devel] [PATCH RESEND v4 02/18] target-i386: Simplify reporting of unavailable features

2014-05-14 Thread Eduardo Habkost
Instead of checking and calling unavailable_host_feature() once for each
bit, simply call the function (now renamed to
report_unavailable_features()) once for each feature word.

Signed-off-by: Eduardo Habkost 
---
Changes v1 -> v2:
 * Rebase to latest qom-cpu (commit 90c5d39c)
Changes v2 -> v3:
 * Trivial rebase after QEMU 2.0 (onto commit 2d03b49)
---
 target-i386/cpu.c | 16 +++-
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 694348e..3c4f327 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1236,11 +1236,11 @@ static const TypeInfo host_x86_cpu_type_info = {
 
 #endif
 
-static int unavailable_host_feature(FeatureWordInfo *f, uint32_t mask)
+static int report_unavailable_features(FeatureWordInfo *f, uint32_t mask)
 {
 int i;
 
-for (i = 0; i < 32; ++i)
+for (i = 0; i < 32; ++i) {
 if (1 << i & mask) {
 const char *reg = get_register_name_32(f->cpuid_reg);
 assert(reg);
@@ -1249,8 +1249,8 @@ static int unavailable_host_feature(FeatureWordInfo *f, 
uint32_t mask)
 f->cpuid_eax, reg,
 f->feat_names[i] ? "." : "",
 f->feat_names[i] ? f->feat_names[i] : "", i);
-break;
 }
+}
 return 0;
 }
 
@@ -1274,12 +1274,10 @@ static int kvm_check_features_against_host(KVMState *s, 
X86CPU *cpu)
 uint32_t host_feat = kvm_arch_get_supported_cpuid(s, wi->cpuid_eax,
  wi->cpuid_ecx,
  wi->cpuid_reg);
-uint32_t mask;
-for (mask = 1; mask; mask <<= 1) {
-if (guest_feat & mask && !(host_feat & mask)) {
-unavailable_host_feature(wi, mask);
-rv = 1;
-}
+uint32_t unavailable_features = guest_feat & ~host_feat;
+if (unavailable_features) {
+report_unavailable_features(wi, unavailable_features);
+rv = 1;
 }
 }
 return rv;
-- 
1.9.0




[Qemu-devel] [PATCH RESEND v4 07/18] target-i386: Filter FEAT_7_0_EBX TCG features too

2014-05-14 Thread Eduardo Habkost
The TCG_7_0_EBX_FEATURES macro was defined but never used (it even had a
typo that was never noticed). Make the existing TCG feature filtering
code use it.

Reviewed-by: Richard Henderson 
Signed-off-by: Eduardo Habkost 
---
Changes v1 -> v2:
 * Trivial rebase to latest qom-cpu (commit 90c5d39c)
   (Reviewed-by line kept)
Changes v2 -> v3:
 * Trivial rebase after QEMU 2.0 (onto commit 2d03b49)
   (Reviewed-by line kept)
---
 target-i386/cpu.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 1a1e390..8d0ae12 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -588,7 +588,7 @@ struct X86CPUDefinition {
 #define TCG_EXT3_FEATURES (CPUID_EXT3_LAHF_LM | CPUID_EXT3_SVM | \
   CPUID_EXT3_CR8LEG | CPUID_EXT3_ABM | CPUID_EXT3_SSE4A)
 #define TCG_SVM_FEATURES 0
-#define TCG_7_0_EBX_FEATURES (CPUID_7_0_EBX_SMEP | CPUID_7_0_EBX_SMAP \
+#define TCG_7_0_EBX_FEATURES (CPUID_7_0_EBX_SMEP | CPUID_7_0_EBX_SMAP | \
   CPUID_7_0_EBX_BMI1 | CPUID_7_0_EBX_BMI2 | CPUID_7_0_EBX_ADX)
   /* missing:
   CPUID_7_0_EBX_FSGSBASE, CPUID_7_0_EBX_HLE, CPUID_7_0_EBX_AVX2,
@@ -2604,6 +2604,7 @@ static void x86_cpu_realizefn(DeviceState *dev, Error 
**errp)
 if (!kvm_enabled()) {
 env->features[FEAT_1_EDX] &= TCG_FEATURES;
 env->features[FEAT_1_ECX] &= TCG_EXT_FEATURES;
+env->features[FEAT_7_0_EBX] &= TCG_7_0_EBX_FEATURES;
 env->features[FEAT_8000_0001_EDX] &= TCG_EXT2_FEATURES;
 env->features[FEAT_8000_0001_ECX] &= TCG_EXT3_FEATURES;
 env->features[FEAT_SVM] &= TCG_SVM_FEATURES;
-- 
1.9.0




[Qemu-devel] [PATCH RESEND v4 01/18] target-i386: kvm: Don't enable MONITOR by default on any CPU model

2014-05-14 Thread Eduardo Habkost
KVM never supported the MONITOR flag so it doesn't make sense to have it
enabled by default when KVM is enabled.

The rationale here is similar to the cases where it makes sense to have
a feature enabled by default on all CPU models when on KVM mode (e.g.
x2apic). In this case we are having a feature disabled by default for
the same reasons.

In this case we don't need machine-type compat code because it is
currently impossible to run a KVM VM with the MONITOR flag set.

Reviewed-by: Paolo Bonzini 
Signed-off-by: Eduardo Habkost 
---
 target-i386/cpu.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 8f193a9..694348e 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -372,6 +372,12 @@ static uint32_t kvm_default_features[FEATURE_WORDS] = {
 [FEAT_1_ECX] = CPUID_EXT_X2APIC,
 };
 
+/* Features that are not added by default to any CPU model when KVM is enabled.
+ */
+static uint32_t kvm_default_unset_features[FEATURE_WORDS] = {
+[FEAT_1_ECX] = CPUID_EXT_MONITOR,
+};
+
 void x86_cpu_compat_disable_kvm_features(FeatureWord w, uint32_t features)
 {
 kvm_default_features[w] &= ~features;
@@ -1893,6 +1899,7 @@ static void x86_cpu_load_def(X86CPU *cpu, 
X86CPUDefinition *def, Error **errp)
 FeatureWord w;
 for (w = 0; w < FEATURE_WORDS; w++) {
 env->features[w] |= kvm_default_features[w];
+env->features[w] &= ~kvm_default_unset_features[w];
 }
 }
 
-- 
1.9.0




[Qemu-devel] [PATCH RESEND v4 04/18] target-i386: Pass FeatureWord argument to report_unavailable_features()

2014-05-14 Thread Eduardo Habkost
This will help us simplify the code that calls
report_unavailable_features() later.

Signed-off-by: Eduardo Habkost 
---
Changes v1 -> v2:
 * Rebase to latest qom-cpu (commit 90c5d39c)
Changes v2 -> v3:
 * Trivial rebase after QEMU 2.0 (onto commit 2d03b49)
---
 target-i386/cpu.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 370da81..b097c0d 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1236,8 +1236,9 @@ static const TypeInfo host_x86_cpu_type_info = {
 
 #endif
 
-static int report_unavailable_features(FeatureWordInfo *f, uint32_t mask)
+static int report_unavailable_features(FeatureWord w, uint32_t mask)
 {
+FeatureWordInfo *f = &feature_word_info[w];
 int i;
 
 for (i = 0; i < 32; ++i) {
@@ -1845,7 +1846,7 @@ static int filter_features_for_kvm(X86CPU *cpu)
 cpu->filtered_features[w] = requested_features & ~env->features[w];
 if (cpu->filtered_features[w]) {
 if (cpu->check_cpuid || cpu->enforce_cpuid) {
-report_unavailable_features(wi, cpu->filtered_features[w]);
+report_unavailable_features(w, cpu->filtered_features[w]);
 }
 rv = 1;
 }
-- 
1.9.0




[Qemu-devel] [PATCH RESEND v4 06/18] target-i386: Make TCG feature filtering more readable

2014-05-14 Thread Eduardo Habkost
Instead of an #ifdef in the middle of the code, just set
TCG_EXT2_FEATURES to a different value depending on TARGET_X86_64.

Reviewed-by: Richard Henderson 
Signed-off-by: Eduardo Habkost 
---
Changes v1 -> v2:
 * Trivial rebase to latest qom-cpu (commit 90c5d39c)
   (Reviewed-by line kept)
Changes v2 -> v3:
 * Trivial rebase after QEMU 2.0 (onto commit 2d03b49)
   (Reviewed-by line kept)
---
 target-i386/cpu.c | 16 ++--
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 4dd522a..1a1e390 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -572,9 +572,17 @@ struct X86CPUDefinition {
   CPUID_EXT_X2APIC, CPUID_EXT_TSC_DEADLINE_TIMER, CPUID_EXT_XSAVE,
   CPUID_EXT_OSXSAVE, CPUID_EXT_AVX, CPUID_EXT_F16C,
   CPUID_EXT_RDRAND */
+
+#ifdef TARGET_X86_64
+#define TCG_EXT2_X86_64_FEATURES (CPUID_EXT2_SYSCALL | CPUID_EXT2_LM)
+#else
+#define TCG_EXT2_X86_64_FEATURES 0
+#endif
+
 #define TCG_EXT2_FEATURES ((TCG_FEATURES & CPUID_EXT2_AMD_ALIASES) | \
   CPUID_EXT2_NX | CPUID_EXT2_MMXEXT | CPUID_EXT2_RDTSCP | \
-  CPUID_EXT2_3DNOW | CPUID_EXT2_3DNOWEXT)
+  CPUID_EXT2_3DNOW | CPUID_EXT2_3DNOWEXT | \
+  TCG_EXT2_X86_64_FEATURES)
   /* missing:
   CPUID_EXT2_PDPE1GB */
 #define TCG_EXT3_FEATURES (CPUID_EXT3_LAHF_LM | CPUID_EXT3_SVM | \
@@ -2596,11 +2604,7 @@ static void x86_cpu_realizefn(DeviceState *dev, Error 
**errp)
 if (!kvm_enabled()) {
 env->features[FEAT_1_EDX] &= TCG_FEATURES;
 env->features[FEAT_1_ECX] &= TCG_EXT_FEATURES;
-env->features[FEAT_8000_0001_EDX] &= (TCG_EXT2_FEATURES
-#ifdef TARGET_X86_64
-| CPUID_EXT2_SYSCALL | CPUID_EXT2_LM
-#endif
-);
+env->features[FEAT_8000_0001_EDX] &= TCG_EXT2_FEATURES;
 env->features[FEAT_8000_0001_ECX] &= TCG_EXT3_FEATURES;
 env->features[FEAT_SVM] &= TCG_SVM_FEATURES;
 } else {
-- 
1.9.0




[Qemu-devel] [PATCH RESEND v4 03/18] target-i386: Merge feature filtering/checking functions

2014-05-14 Thread Eduardo Habkost
Merge filter_features_for_kvm() and kvm_check_features_against_host().

Both functions made exactly the same calculations, the only difference
was that filter_features_for_kvm() changed the bits on cpu->features[],
and kvm_check_features_against_host() did error reporting.

Reviewed-by: Richard Henderson 
Signed-off-by: Eduardo Habkost 
---
Changes v1 -> v2:
 * Trivial rebase to latest qom-cpu (commit 90c5d39c)
   (Reviewed-by line kept)
Changes v2 -> v3:
 * Trivial rebase after QEMU 2.0 (onto commit 2d03b49)
   (Reviewed-by line kept)
---
 target-i386/cpu.c | 53 +++--
 1 file changed, 19 insertions(+), 34 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 3c4f327..370da81 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1254,35 +1254,6 @@ static int report_unavailable_features(FeatureWordInfo 
*f, uint32_t mask)
 return 0;
 }
 
-/* Check if all requested cpu flags are making their way to the guest
- *
- * Returns 0 if all flags are supported by the host, non-zero otherwise.
- *
- * This function may be called only if KVM is enabled.
- */
-static int kvm_check_features_against_host(KVMState *s, X86CPU *cpu)
-{
-CPUX86State *env = &cpu->env;
-int rv = 0;
-FeatureWord w;
-
-assert(kvm_enabled());
-
-for (w = 0; w < FEATURE_WORDS; w++) {
-FeatureWordInfo *wi = &feature_word_info[w];
-uint32_t guest_feat = env->features[w];
-uint32_t host_feat = kvm_arch_get_supported_cpuid(s, wi->cpuid_eax,
- wi->cpuid_ecx,
- wi->cpuid_reg);
-uint32_t unavailable_features = guest_feat & ~host_feat;
-if (unavailable_features) {
-report_unavailable_features(wi, unavailable_features);
-rv = 1;
-}
-}
-return rv;
-}
-
 static void x86_cpuid_version_get_family(Object *obj, Visitor *v, void *opaque,
  const char *name, Error **errp)
 {
@@ -1849,11 +1820,20 @@ CpuDefinitionInfoList *arch_query_cpu_definitions(Error 
**errp)
 return cpu_list;
 }
 
-static void filter_features_for_kvm(X86CPU *cpu)
+/* Filters CPU feature words based on host availability of each feature
+ *
+ * Returns 0 if all flags are supported by the host, non-zero otherwise.
+ *
+ * This function may be called only if KVM is enabled.
+ */
+static int filter_features_for_kvm(X86CPU *cpu)
 {
 CPUX86State *env = &cpu->env;
 KVMState *s = kvm_state;
 FeatureWord w;
+int rv = 0;
+
+assert(kvm_enabled());
 
 for (w = 0; w < FEATURE_WORDS; w++) {
 FeatureWordInfo *wi = &feature_word_info[w];
@@ -1863,7 +1843,15 @@ static void filter_features_for_kvm(X86CPU *cpu)
 uint32_t requested_features = env->features[w];
 env->features[w] &= host_feat;
 cpu->filtered_features[w] = requested_features & ~env->features[w];
+if (cpu->filtered_features[w]) {
+if (cpu->check_cpuid || cpu->enforce_cpuid) {
+report_unavailable_features(wi, cpu->filtered_features[w]);
+}
+rv = 1;
+}
 }
+
+return rv;
 }
 
 /* Load data from X86CPUDefinition
@@ -2612,14 +2600,11 @@ static void x86_cpu_realizefn(DeviceState *dev, Error 
**errp)
 env->features[FEAT_8000_0001_ECX] &= TCG_EXT3_FEATURES;
 env->features[FEAT_SVM] &= TCG_SVM_FEATURES;
 } else {
-KVMState *s = kvm_state;
-if ((cpu->check_cpuid || cpu->enforce_cpuid)
-&& kvm_check_features_against_host(s, cpu) && cpu->enforce_cpuid) {
+if (filter_features_for_kvm(cpu) && cpu->enforce_cpuid) {
 error_setg(&local_err,
"Host's CPU doesn't support requested features");
 goto out;
 }
-filter_features_for_kvm(cpu);
 }
 
 #ifndef CONFIG_USER_ONLY
-- 
1.9.0




[Qemu-devel] [PATCH] hw/arm/boot: get AArch64 kernel Image load offset from Image file

2014-05-14 Thread Rob Herring
From: Rob Herring 

The AArch64 kermel Image format defines the load offset in its header.
Retrieve the offset from the file instead of hardcoding it to 0x8.

Use of the hardcoded value will break when text_offset randomization is
added to the kernel.

Signed-off-by: Rob Herring 
---
 hw/arm/boot.c | 32 ++--
 1 file changed, 30 insertions(+), 2 deletions(-)

diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index 3d1f4a2..4adce9e 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -24,7 +24,14 @@
  */
 #define KERNEL_ARGS_ADDR 0x100
 #define KERNEL_LOAD_ADDR 0x0001
-#define KERNEL64_LOAD_ADDR 0x0008
+
+typedef struct {
+uint32_t code[2];
+uint64_t text_offset;
+uint64_t res[5];
+uint32_t magic;
+uint32_t res5;
+} aarch64_image_header_t;
 
 typedef enum {
 FIXUP_NONE = 0,   /* do nothing */
@@ -441,6 +448,26 @@ static void do_cpu_reset(void *opaque)
 }
 }
 
+static hwaddr aarch64_get_image_offset(const char *filename)
+{
+int fd, size;
+aarch64_image_header_t hdr;
+
+fd = open(filename, O_RDONLY | O_BINARY);
+if (fd < 0) {
+return 0;
+}
+
+size = read(fd, &hdr, sizeof(hdr));
+close(fd);
+
+if (size < sizeof(aarch64_image_header_t) ||
+le32_to_cpu(hdr.magic) != 0x644d5241) {
+return 0;
+}
+return le32_to_cpu(hdr.text_offset);
+}
+
 void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info)
 {
 CPUState *cs = CPU(cpu);
@@ -463,7 +490,7 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info 
*info)
 
 if (arm_feature(&cpu->env, ARM_FEATURE_AARCH64)) {
 primary_loader = bootloader_aarch64;
-kernel_load_offset = KERNEL64_LOAD_ADDR;
+kernel_load_offset = aarch64_get_image_offset(info->kernel_filename);
 elf_machine = EM_AARCH64;
 } else {
 primary_loader = bootloader;
@@ -505,6 +532,7 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info 
*info)
 /* Assume that raw images are linux kernels, and ELF images are not.  */
 kernel_size = load_elf(info->kernel_filename, NULL, NULL, &elf_entry,
NULL, NULL, big_endian, elf_machine, 1);
+
 entry = elf_entry;
 if (kernel_size < 0) {
 kernel_size = load_uimage(info->kernel_filename, &entry, NULL,
-- 
1.9.1




[Qemu-devel] [PATCH v2 RESEND] vl.c: Unify MAX_CPUMASK_BITS and machine->max_cpus checks

2014-05-14 Thread Eduardo Habkost
If a given machine have max_cpus set, not just smp_cpus needs to be
limited, but the total number of CPUs (considering CPU hotplug) for the
machine.

We also had yet another max_cpus limit check at smp_parse(), ensuring
that max_cpus < MAX_CPUMASK_BITS.

This patch unifies the machine->max_cpus and MAX_CPUMASK_BITS checks,
and also moves the (smp_cpus <= max_cpus) outside smp_parse(), to keep
all checks in the same place.

With those changes, the new code ensures that:

  1 <= smp_cpus <= max_cpus <= machine->max_cpus <= MAX_CPUMASK_BITS

Signed-off-by: Eduardo Habkost 
Cc: Peter Maydell 
Cc: Andreas Färber 
Cc: Igor Mammedov 
Cc: Marcelo Tosatti 
---
Changes v1 -> v2:
 * v1 was: [PATCH] vl.c: Check max_cpus limit instead of smp_cpus
 * Unify machine->max_cpus and MAX_CPUMASK_BITS check
 * Move all checks outside smp_parse()
 * Add assert() lines ensuring the results are consistent
 * s/machine/machine_class/, after rebase to latest qemu.git
   (commit 6b342cc)
---
 vl.c | 26 ++
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/vl.c b/vl.c
index 709d8cd..749abbd 100644
--- a/vl.c
+++ b/vl.c
@@ -1425,15 +1425,6 @@ static void smp_parse(QemuOpts *opts)
 max_cpus = smp_cpus;
 }
 
-if (max_cpus > MAX_CPUMASK_BITS) {
-fprintf(stderr, "Unsupported number of maxcpus\n");
-exit(1);
-}
-if (max_cpus < smp_cpus) {
-fprintf(stderr, "maxcpus must be equal to or greater than smp\n");
-exit(1);
-}
-
 }
 
 static void configure_realtime(QemuOpts *opts)
@@ -4054,13 +4045,24 @@ int main(int argc, char **argv, char **envp)
 smp_parse(qemu_opts_find(qemu_find_opts("smp-opts"), NULL));
 
 machine_class->max_cpus = machine_class->max_cpus ?: 1; /* Default to UP */
-if (smp_cpus > machine_class->max_cpus) {
-fprintf(stderr, "Number of SMP cpus requested (%d), exceeds max cpus "
-"supported by machine `%s' (%d)\n", smp_cpus,
+machine_class->max_cpus = MIN(machine_class->max_cpus, MAX_CPUMASK_BITS);
+
+if (max_cpus < smp_cpus) {
+fprintf(stderr, "maxcpus must be equal to or greater than smp\n");
+exit(1);
+}
+if (max_cpus > machine_class->max_cpus) {
+fprintf(stderr, "Total number of CPUs (%d), exceeds maximum "
+"supported by machine `%s' (%d)\n", max_cpus,
 machine_class->name, machine_class->max_cpus);
 exit(1);
 }
 
+assert(1 <= smp_cpus);
+assert(smp_cpus <= max_cpus);
+assert(max_cpus <= machine_class->max_cpus);
+assert(machine_class->max_cpus <= MAX_CPUMASK_BITS);
+
 /*
  * Get the default machine options from the machine if it is not already
  * specified either by the configuration file or by the command line.
-- 
1.9.0




Re: [Qemu-devel] [PATCH 6/7] arm/virt: enable PSCI emulation support for system emulation

2014-05-14 Thread Rob Herring
On Wed, May 14, 2014 at 12:51 PM, Peter Maydell
 wrote:
> On 5 May 2014 17:00, Rob Herring  wrote:
>> From: Rob Herring 
>>
>> Now that we have PSCI emulation, enable it for the virt platform.
>> This simplifies the virt machine a bit now that PSCI and SMP no longer
>> need to be KVM only features.

[...]

>> +qemu_fdt_add_subnode(fdt, "/psci");
>> +qemu_fdt_setprop_string(fdt, "/psci", "method", "hvc");
>> +if (kvm_enabled() && !kvm_check_extension(kvm_state, 
>> KVM_CAP_ARM_PSCI_0_2)) {
>> +qemu_fdt_setprop_string(fdt, "/psci", "compatible", "arm,psci");
>> +} else {
>> +const char compat[] = "arm,psci-0.2\0arm,psci";
>> +qemu_fdt_setprop(fdt, "/psci", "compatible", compat, 
>> sizeof(compat));
>>  }
>
> My suggestion to Pranav was that we abstract away the "which PSCI
> version?" decision into a field in ARMCPU, in which case we can
> just have TCG always set it to 0.2. So some of this logic
> will get a little simpler on rebase.

You can't. You have to support both because you don't know what the
kernel supports. An old kernel will only support arm,psci.

Rob



Re: [Qemu-devel] uvesafb doesn't work with seabios

2014-05-14 Thread Bernhard Walle
Am 13.05.14 21:48, schrieb Bernhard Walle:
> However, I took the step to update the x86emu code from X.org. That
> seems to work! At least with my test VM that is based on Arch Linux.
> I'll try the original Gentoo-based VM tomorrow.

That worked, too. :)

I sent a pull request via https://github.com/bwalle/v86d to
https://github.com/mjanusz/v86d.

Regards,
Bernhard




Re: [Qemu-devel] [PATCH 12/18] block/sheepdog: Propagate errors through connect_to_sdog()

2014-05-14 Thread Eric Blake
On 05/13/2014 10:02 AM, Markus Armbruster wrote:
> Cc: MORITA Kazutaka 
> Signed-off-by: Markus Armbruster 
> ---
>  block/sheepdog.c | 77 
> 
>  1 file changed, 55 insertions(+), 22 deletions(-)
> 

Reviewed-by: Eric Blake 


-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH v2 10/23] target-arm: implement CPACR register logic

2014-05-14 Thread Fedorov Sergey

14.05.2014 10:06, Sergey Fedorov пишет:
> On 13.05.2014 20:15, Fabian Aggeler wrote:
>> From: Sergey Fedorov 
>>
>> CPACR register allows to control access rights to coprocessor 0-13
>> interfaces. Bits corresponding to unimplemented coprocessors should be
>> RAZ/WI. QEMU implements only VFP coprocessor on ARMv6+ targets. So only
>> cp10 & cp11 bits are writable.
>>
>> Signed-off-by: Sergey Fedorov 
>> Signed-off-by: Fabian Aggeler 
>> ---
>>  target-arm/helper.c|  6 ++
>>  target-arm/translate.c | 26 +++---
>>  2 files changed, 29 insertions(+), 3 deletions(-)
>>
>> diff --git a/target-arm/helper.c b/target-arm/helper.c
>> index cf1f88c..4e82259 100644
>> --- a/target-arm/helper.c
>> +++ b/target-arm/helper.c
>> @@ -477,6 +477,12 @@ static const ARMCPRegInfo not_v7_cp_reginfo[] = {
>>  static void cpacr_write(CPUARMState *env, const ARMCPRegInfo *ri,
>>  uint64_t value)
>>  {
>> +uint32_t mask = 0;
>> +
>> +if (arm_feature(env, ARM_FEATURE_VFP)) {
>> +mask |= 0x00f0; /* VFP coprocessor: cp10 & cp11 */
>> +}
>> +value &= mask;
>>  if (env->cp15.c1_coproc != value) {
>>  env->cp15.c1_coproc = value;
>>  /* ??? Is this safe when called from within a TB?  */
>> diff --git a/target-arm/translate.c b/target-arm/translate.c
>> index 87d0918..c815fb3 100644
>> --- a/target-arm/translate.c
>> +++ b/target-arm/translate.c
>> @@ -6866,9 +6866,29 @@ static int disas_coproc_insn(CPUARMState * env, 
>> DisasContext *s, uint32_t insn)
>>  const ARMCPRegInfo *ri;
>>  
>>  cpnum = (insn >> 8) & 0xf;
>> -if (arm_feature(env, ARM_FEATURE_XSCALE)
>> -&& ((env->cp15.c15_cpar ^ 0x3fff) & (1 << cpnum)))
>> -return 1;
>> +if (cpnum < 14) {
>> +if (arm_feature(env, ARM_FEATURE_XSCALE)) {
>> +if (~env->cp15.c15_cpar & (1 << cpnum)) {
>> +return 1;
>> +}
>> +} else {
>> +/* Bits [20:21] of CPACR control access to cp10
>> + * Bits [23:22] of CPACR control access to cp11 */
>> +switch ((env->cp15.c1_coproc >> (cpnum * 2)) & 3) {
>> +case 0: /* access denied */
>> +return 1;
>> +case 1: /* privileged mode access only */
>> +if (IS_USER(s)) {
>> +return 1;
>> +}
>> +break;
>> +case 2: /* reserved */
>> +return 1;
>> +case 3: /* privileged and user mode access */
>> +break;
>> +}
>> +}
>> +}
>>  
>>  /* First check for coprocessor space used for actual instructions */
>>  switch (cpnum) {
> Please, look at disas_vfp_insn() and disas_neon_*_insn() functions.
> Looks like them should be updated. In that case do not forget to adjust
> arm_cpu_reset() so user emulation would be able to execute VFP/NEON
> instructions.

See ARM ARM v7-AR B1.11.1

>
> Thanks,
> Sergey.




Re: [Qemu-devel] [PATCH v2 06/23] target-arm: add arm_is_secure() function

2014-05-14 Thread Fedorov Sergey

14.05.2014 18:42, Greg Bellows пишет:
> On 14 May 2014 00:53, Sergey Fedorov  wrote:
>
>> On 13.05.2014 20:15, Fabian Aggeler wrote:
>>> arm_is_secure() function allows to determine CPU security state
>>> if the CPU implements Security Extensions.
>>>
>>> Signed-off-by: Sergey Fedorov 
>>> Signed-off-by: Fabian Aggeler 
>>> ---
>>>  target-arm/cpu.h | 15 +++
>>>  1 file changed, 15 insertions(+)
>>>
>>> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
>>> index a56d3d6..6ea0432 100644
>>> --- a/target-arm/cpu.h
>>> +++ b/target-arm/cpu.h
>>> @@ -640,6 +640,21 @@ static inline int arm_feature(CPUARMState *env, int
>> feature)
>>>  return (env->features & (1ULL << feature)) != 0;
>>>  }
>>>
>>> +/* Return true if the processor is in secure state */
>>> +static inline bool arm_is_secure(CPUARMState *env)
>>> +{
>>> +#if !defined(CONFIG_USER_ONLY)
>>> +if (arm_feature(env, ARM_FEATURE_SECURITY_EXTENSIONS)) {
>> I think feature test can be safely avoided here. Without this feature
>> that should be no way to switch to monitor mode and to access SCR register.
>>
> I agree with the feature check here.  For correctness, we should only be
> examining c1_scr if the security extension is enabled.   This is consistent
> with only registering the SCR register if the feature is enabled.

So this check will be done every time arm_is_secure() is called, e.g. on
each MMU table walk.

Moreover I've noticed that this function deviates from ARM ARM v7-AR
description in section B1.5.1 which states: "The IsSecure() function
returns TRUE if the processor is in Secure state, or if the
implementation does not include
the Security Extensions, and FALSE otherwise." Then there is a pseudo
code for that function.

>
>>> +return ((env->uncached_cpsr & CPSR_M) == ARM_CPU_MODE_MON) ||
>>> +!(env->cp15.c1_scr & 1);
>>> +} else {
>>> +return false;
>>> +}
>>> +#else
>>> +return false;
>> That is a good question how to treat user emulation: secure or
>> non-secure. Perhaps assuming user emulation in secure state may simplify
>> code in the following patches.
>
>>> +#endif
>>> +}
>>> +
>>>  /* Return true if the specified exception level is running in AArch64
>> state. */
>>>  static inline bool arm_el_is_aa64(CPUARMState *env, int el)
>>>  {
>> Thanks,
>> Sergey.
>>
>>




  1   2   3   4   >