Re: [Qemu-devel] [RFC] [PATCHv8 09/30] aio / timers: Add QEMUTimerListGroup and helper functions

2013-08-12 Thread Alex Bligh



--On 12 August 2013 14:53:06 +0800 Wenchao Xia  
wrote:



1. Wrap TimerListGroup into a separate struct, leave all the
TimerListGroup functions in. This probably makes it easier if
(for instance) we decided to get rid of AioContexts entirely
and make them g_source subclasses (per Wenchao Xia).


   I have a quick view of this series, but not very carefully since
it is quite long.:) I have replied with Paolo's comments on my
RFC thread. Personally I like g_source sub class, which make code
clear, but let's discuss first if the direction is right in that mail.


Looks like Paolo is happy going with TimerListGroup for now. So
I wrapped the array in a struct for v10.

--
Alex Bligh



[Qemu-devel] [PATCHv2] add qemu-img convert -C option (skip target volume creation)

2013-08-12 Thread Alex Bligh
Add a -C option to skip volume creation on qemu-img convert.
This is useful for targets such as rbd / ceph, where the
target volume may already exist; we cannot always rely on
qemu-img convert to create the image, as dependent on the
output format, there may be parameters which are not possible
to specify through the qemu-img convert command line.

Code:

Author: Alexandre Derumier 
Signed-off-by: Alexandre Derumier 
Signed-off-by: Alex Bligh 

Documentaton:

Author: Alex Bligh 
Signed-off-by: Alex Bligh 
---
 qemu-img-cmds.hx |4 ++--
 qemu-img.c   |   39 ---
 qemu-img.texi|   15 ++-
 3 files changed, 40 insertions(+), 18 deletions(-)

diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
index 4ca7e95..74ced81 100644
--- a/qemu-img-cmds.hx
+++ b/qemu-img-cmds.hx
@@ -34,9 +34,9 @@ STEXI
 ETEXI
 
 DEF("convert", img_convert,
-"convert [-c] [-p] [-q] [-f fmt] [-t cache] [-O output_fmt] [-o options] 
[-s snapshot_name] [-S sparse_size] filename [filename2 [...]] output_filename")
+"convert [-c] [-p] [-q] [-C] [-f fmt] [-t cache] [-O output_fmt] [-o 
options] [-s snapshot_name] [-S sparse_size] filename [filename2 [...]] 
output_filename")
 STEXI
-@item convert [-c] [-p] [-q] [-f @var{fmt}] [-t @var{cache}] [-O 
@var{output_fmt}] [-o @var{options}] [-s @var{snapshot_name}] [-S 
@var{sparse_size}] @var{filename} [@var{filename2} [...]] @var{output_filename}
+@item convert [-c] [-p] [-q] [-C] [-f @var{fmt}] [-t @var{cache}] [-O 
@var{output_fmt}] [-o @var{options}] [-s @var{snapshot_name}] [-S 
@var{sparse_size}] @var{filename} [@var{filename2} [...]] @var{output_filename}
 ETEXI
 
 DEF("info", img_info,
diff --git a/qemu-img.c b/qemu-img.c
index b9a848d..c6e1cc0 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -103,6 +103,8 @@ static void help(void)
"  '-S' indicates the consecutive number of bytes that must contain 
only zeros\n"
"   for qemu-img to create a sparse image during conversion\n"
"  '--output' takes the format in which the output must be done 
(human or json)\n"
+   "  '-C' skips the target volume creation (useful if the volume is 
created)\n"
+   "   prior to running qemu-img)\n"
"\n"
"Parameters to check subcommand:\n"
"  '-r' tries to repair any inconsistencies that are found during 
the check.\n"
@@ -1116,7 +1118,8 @@ out3:
 
 static int img_convert(int argc, char **argv)
 {
-int c, ret = 0, n, n1, bs_n, bs_i, compress, cluster_size, cluster_sectors;
+int c, ret = 0, n, n1, bs_n, bs_i, compress, cluster_size,
+cluster_sectors, skipcreate;
 int progress = 0, flags;
 const char *fmt, *out_fmt, *cache, *out_baseimg, *out_filename;
 BlockDriver *drv, *proto_drv;
@@ -1139,8 +1142,9 @@ static int img_convert(int argc, char **argv)
 cache = "unsafe";
 out_baseimg = NULL;
 compress = 0;
+skipcreate = 0;
 for(;;) {
-c = getopt(argc, argv, "f:O:B:s:hce6o:pS:t:q");
+c = getopt(argc, argv, "f:O:B:s:hce6o:pS:t:qC");
 if (c == -1) {
 break;
 }
@@ -1161,6 +1165,9 @@ static int img_convert(int argc, char **argv)
 case 'c':
 compress = 1;
 break;
+case 'C':
+skipcreate = 1;
+break;
 case 'e':
 error_report("option -e is deprecated, please use \'-o "
   "encryption\' instead!");
@@ -1329,20 +1336,22 @@ static int img_convert(int argc, char **argv)
 }
 }
 
-/* Create the new image */
-ret = bdrv_create(drv, out_filename, param);
-if (ret < 0) {
-if (ret == -ENOTSUP) {
-error_report("Formatting not supported for file format '%s'",
- out_fmt);
-} else if (ret == -EFBIG) {
-error_report("The image size is too large for file format '%s'",
- out_fmt);
-} else {
-error_report("%s: error while converting %s: %s",
- out_filename, out_fmt, strerror(-ret));
+if (!skipcreate) {
+/* Create the new image */
+ret = bdrv_create(drv, out_filename, param);
+if (ret < 0) {
+if (ret == -ENOTSUP) {
+error_report("Formatting not supported for file format '%s'",
+ out_fmt);
+} else if (ret == -EFBIG) {
+error_report("The image size is too large for file format 
'%s'",
+ out_fmt);
+} else {
+error_report("%s: error while converting %s: %s",
+ out_filename, out_fmt, strerror(-ret));
+}
+goto out;
 }
-goto out;
 }
 
 flags = BDRV_O_RDWR;
diff --git a/qemu-img.texi b/qemu-img.texi
index 69f1bda..9e5ba36 100644
--- a/qemu-img.texi
+++ b/qemu-img.texi
@@ -96,6 +96,14 @@ Second image format
 Strict mode - fail

Re: [Qemu-devel] [PATCH] add qemu-img convert -C option (skip target volume creation)

2013-08-12 Thread Alex Bligh

On 12 Aug 2013, at 07:22, Fam Zheng wrote:
>> --- a/qemu-img.c
>> +++ b/qemu-img.c
>> @@ -103,6 +103,7 @@ static void help(void)
>>"  '-S' indicates the consecutive number of bytes that must 
>> contain only zeros\n"
>>"   for qemu-img to create a sparse image during conversion\n"
>>"  '--output' takes the format in which the output must be done 
>> (human or json)\n"
>> +   "  '-C' skips the target volume creation (useful for rbd)\n"
> 
> The statement "useful for rbd" is not very informative, maybe you could
> use "'-C' skips the target volume creation (assuming it already exists)".

Done with slightly different text - see v2 just posted

>> static int img_convert(int argc, char **argv)
>> {
>> -int c, ret = 0, n, n1, bs_n, bs_i, compress, cluster_size, 
>> cluster_sectors;
>> +int c, ret = 0, n, n1, bs_n, bs_i, compress, cluster_size, 
>> cluster_sectors, skipcreate;
> 
> This line is too long, please break it.

Done in v2 just posted.

-- 
Alex Bligh







Re: [Qemu-devel] [RFC] Convert AioContext to Gsource sub classes

2013-08-12 Thread Paolo Bonzini

> 1) rename AioContext to AioSource.
>   This is my major purpose, which declare it is not a "context" concept,
> and GMainContext is the entity represent the thread's activity.

Note that the nested event loops in QEMU are _very_ different from
glib nested event loops.  In QEMU, nested event loops only run block
layer events.  In glib, they run all events.  That's why you need
AioContext.

> 2) Break AioSource into FdSource and BhSource.
>   This make custom code less and simpler, one Gsource for one kind of
> job. It is not necessary but IMHO it will make things clear when add
> more things into main loop: add a new Gsource sub class, avoid to
> always have relationship with AioContext.

But this is only complicating things work since users rely on both file-
descriptor APIs and bottom half APIs.

> >>More reasons:
> >>When I thinking how to bind library code to a thread context, it may
> >> need to add Context's concept into API of block.c. If I use AioContext,
> >> there will need a wrapper API to run the event loop. But If I got
> >> glib's GmainContext, things become simple.

You already have it because AioContext is a GSource.  You do not need
to expose the AioContext, except as a GSource.

Paolo



Re: [Qemu-devel] [PATCH 1/2] vmdk: support vmfsSparse files

2013-08-12 Thread Paolo Bonzini


- Original Message -
> From: "Fam Zheng" 
> To: "Paolo Bonzini" 
> Cc: qemu-devel@nongnu.org, kw...@redhat.com
> Sent: Monday, August 12, 2013 3:31:44 AM
> Subject: Re: [PATCH 1/2] vmdk: support vmfsSparse files
> 
> On Sun, 08/11 18:13, Paolo Bonzini wrote:
> > VMware ESX hosts use a variant of the VMDK3 format, identified by the
> > vmfsSparse create type ad the VMFSSPARSE extent type.
> > 
> > It has 16 KB grain tables (L2) and a variable-size grain directory (L1).
> > In addition, the grain size is always 512, but that is not a problem
> > because it is included in the header.
> > 
> > The format of the extents is documented in the VMDK spec.  The format
> > of the descriptor file is not documented precisely, but it can be
> > found at http://kb.vmware.com/kb/10026353 (Recreating a missing virtual
> > machine disk (VMDK) descriptor file for delta disks).
> > 
> I don't have access to this link

It only works from Google.  Try Googling for "vmfssparse delta".

> , could you include some documents to
> this descriptor format in comment or commit message? IIRC, it's only the
> type be "VMFSSPARSE", right?

Yes.  And "VMFS" for the base (patch 2).

> What version of ESX has this format?

At least 4.0 and newer.

> This needs to be rebased, vmdk_add_extent() signature has been changed
> in:
> 
> commit 8aa1331c09a9b899f48d97f097bb49b7d458be1c
> Author: Fam Zheng 
> Date:   Tue Aug 6 15:44:51 2013 +0800
> 
> vmdk: check granularity field in opening
> 
> Granularity is used to calculate the cluster size and allocate r/w
> buffer. Check the value from image before using it, so we don't
> abort()
> for unbounded memory allocation.
> 
> Signed-off-by: Fam Zheng 
> Signed-off-by: Kevin Wolf 
> 
> Since the new function is a variant of vmdk_open_vmdk3(), would you
> consider doing a tiny refactor and reduce duplication? And l1dir_size
> and granularity need to be checked, as in vmdk_open_vmdk4().

I am not sure how to refactor it... in fact, since I'm on vacation I
wouldn't mind if somebody else fixes the patch.  I can test it either
tomorrow or next week.

Paolo



Re: [Qemu-devel] [SeaBIOS] [PATCH] acpi: hide 64-bit PCI hole for Windows XP

2013-08-12 Thread Michael S. Tsirkin
On Mon, Aug 12, 2013 at 08:37:00AM +0200, Gerd Hoffmann wrote:
>   Hi,
> 
> > If we make it a rule that PCI is`setup before ACPI tables
> > are read, then QEMU can do the patching itself when
> > it detects BIOS reading the tables.
> 
> Approach makes sense to me.  The ordering constrain shouldn't be a big
> burden, hardware detection+bringup (including pci setup) is the first
> thing done by the firmware, loading/generating acpi tables is one of the
> last things.  And it avoids the need to communicate the addresses (or
> patch locations) between qemu+firmware.
> 
> What do you want to use this for?  pmbase and xbar are simple, they are
> just a single register read.  pci io windows needs a root bus scan, but
> should be doable too.

Right. We'll need to migrate the offsets for patching since
they are tied to specific AML and this can change.

> > Gerd, Laszlo,others,  does this rule work for alternative firmwares?
> 
> It surely works for coreboot, and I would be very surprised if this
> causes trouble for ovmf.
> 
> cheers,
>   Gerd



[Qemu-devel] [PATCH 0/8] virtio for endian curious guests Take #2

2013-08-12 Thread Rusty Russell
The driver parts (patches 3-8) are unchanged, so didn't repost.

1) Rebased onto more recent git tree.
2) New stub is virtio_get_byteswap()
3) PPC64 uses endianness of first CPU interrupt vectors, not CPU endiannes.

Hope this one is closer...
Rusty.

Rusty Russell (8):
  virtio_get_byteswap: function for endian-ambivalent targets using
virtio.
  target-ppc: ppc64 target's virtio can be either endian.
  virtio: allow byte swapping for vring and config access
  hw/net/virtio-net: use virtio wrappers to access headers.
  hw/net/virtio-balloon: use virtio wrappers to access page frame
numbers.
  hw/block/virtio-blk: use virtio wrappers to access headers.
  hw/scsi/virtio-scsi: use virtio wrappers to access headers.
  hw/char/virtio-serial-bus: use virtio wrappers to access headers.

 hw/block/virtio-blk.c |  35 +-
 hw/char/virtio-serial-bus.c   |  34 +-
 hw/net/virtio-net.c   |  15 +++--
 hw/scsi/virtio-scsi.c |  33 +-
 hw/virtio/virtio-balloon.c|   3 +-
 hw/virtio/virtio.c|  29 +
 include/hw/virtio/virtio-access.h | 130 ++
 include/hw/virtio/virtio.h|   2 +
 stubs/Makefile.objs   |   1 +
 stubs/virtio_get_byteswap.c   |   6 ++
 target-ppc/misc_helper.c  |   9 +++
 11 files changed, 226 insertions(+), 71 deletions(-)
 create mode 100644 include/hw/virtio/virtio-access.h
 create mode 100644 stubs/virtio_get_byteswap.c

-- 
1.8.1.2




Re: [Qemu-devel] [PATCH 1/7] virtio: allow byte swapping for vring and config access

2013-08-12 Thread Rusty Russell
Peter Maydell  writes:
> On 9 August 2013 08:35, Rusty Russell  wrote:
>> That's a lot of replumbing and indirect function calls for a fairly
>> obscure case.  We certainly don't have a nice CPUState lying around in
>> virtio at the moment, for example.
>
> Actually if you're in an IO routine you do: it will be in the
> global variable 'current_cpu'. Most IO functions don't need this,
> but it's what we use for things like "this device behaviour varies
> depending on which CPU accesses it".

Hmm, that was NULL for me when called from virtio.  I have stuck with
first_cpu in the ppc64 case.

Patches coming,
Rusty.




[Qemu-devel] [PATCH 3/8] virtio: allow byte swapping for vring and config access

2013-08-12 Thread Rusty Russell
This is based on a simpler patch by Anthony Liguouri, which only handled
the vring accesses.  We also need some drivers to access these helpers,
eg. for data which contains headers.

Signed-off-by: Rusty Russell 
---
 hw/virtio/virtio.c | 29 +++--
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 09f62c6..178647b 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -18,6 +18,7 @@
 #include "hw/virtio/virtio.h"
 #include "qemu/atomic.h"
 #include "hw/virtio/virtio-bus.h"
+#include "hw/virtio/virtio-access.h"
 
 /*
  * The alignment to use between consumer and producer parts of vring.
@@ -104,49 +105,49 @@ static inline uint64_t vring_desc_addr(hwaddr desc_pa, 
int i)
 {
 hwaddr pa;
 pa = desc_pa + sizeof(VRingDesc) * i + offsetof(VRingDesc, addr);
-return ldq_phys(pa);
+return virtio_ldq_phys(pa);
 }
 
 static inline uint32_t vring_desc_len(hwaddr desc_pa, int i)
 {
 hwaddr pa;
 pa = desc_pa + sizeof(VRingDesc) * i + offsetof(VRingDesc, len);
-return ldl_phys(pa);
+return virtio_ldl_phys(pa);
 }
 
 static inline uint16_t vring_desc_flags(hwaddr desc_pa, int i)
 {
 hwaddr pa;
 pa = desc_pa + sizeof(VRingDesc) * i + offsetof(VRingDesc, flags);
-return lduw_phys(pa);
+return virtio_lduw_phys(pa);
 }
 
 static inline uint16_t vring_desc_next(hwaddr desc_pa, int i)
 {
 hwaddr pa;
 pa = desc_pa + sizeof(VRingDesc) * i + offsetof(VRingDesc, next);
-return lduw_phys(pa);
+return virtio_lduw_phys(pa);
 }
 
 static inline uint16_t vring_avail_flags(VirtQueue *vq)
 {
 hwaddr pa;
 pa = vq->vring.avail + offsetof(VRingAvail, flags);
-return lduw_phys(pa);
+return virtio_lduw_phys(pa);
 }
 
 static inline uint16_t vring_avail_idx(VirtQueue *vq)
 {
 hwaddr pa;
 pa = vq->vring.avail + offsetof(VRingAvail, idx);
-return lduw_phys(pa);
+return virtio_lduw_phys(pa);
 }
 
 static inline uint16_t vring_avail_ring(VirtQueue *vq, int i)
 {
 hwaddr pa;
 pa = vq->vring.avail + offsetof(VRingAvail, ring[i]);
-return lduw_phys(pa);
+return virtio_lduw_phys(pa);
 }
 
 static inline uint16_t vring_used_event(VirtQueue *vq)
@@ -158,42 +159,42 @@ static inline void vring_used_ring_id(VirtQueue *vq, int 
i, uint32_t val)
 {
 hwaddr pa;
 pa = vq->vring.used + offsetof(VRingUsed, ring[i].id);
-stl_phys(pa, val);
+virtio_stl_phys(pa, val);
 }
 
 static inline void vring_used_ring_len(VirtQueue *vq, int i, uint32_t val)
 {
 hwaddr pa;
 pa = vq->vring.used + offsetof(VRingUsed, ring[i].len);
-stl_phys(pa, val);
+virtio_stl_phys(pa, val);
 }
 
 static uint16_t vring_used_idx(VirtQueue *vq)
 {
 hwaddr pa;
 pa = vq->vring.used + offsetof(VRingUsed, idx);
-return lduw_phys(pa);
+return virtio_lduw_phys(pa);
 }
 
 static inline void vring_used_idx_set(VirtQueue *vq, uint16_t val)
 {
 hwaddr pa;
 pa = vq->vring.used + offsetof(VRingUsed, idx);
-stw_phys(pa, val);
+virtio_stw_phys(pa, val);
 }
 
 static inline void vring_used_flags_set_bit(VirtQueue *vq, int mask)
 {
 hwaddr pa;
 pa = vq->vring.used + offsetof(VRingUsed, flags);
-stw_phys(pa, lduw_phys(pa) | mask);
+virtio_stw_phys(pa, virtio_lduw_phys(pa) | mask);
 }
 
 static inline void vring_used_flags_unset_bit(VirtQueue *vq, int mask)
 {
 hwaddr pa;
 pa = vq->vring.used + offsetof(VRingUsed, flags);
-stw_phys(pa, lduw_phys(pa) & ~mask);
+virtio_stw_phys(pa, virtio_lduw_phys(pa) & ~mask);
 }
 
 static inline void vring_avail_event(VirtQueue *vq, uint16_t val)
@@ -203,7 +204,7 @@ static inline void vring_avail_event(VirtQueue *vq, 
uint16_t val)
 return;
 }
 pa = vq->vring.used + offsetof(VRingUsed, ring[vq->vring.num]);
-stw_phys(pa, val);
+virtio_stw_phys(pa, val);
 }
 
 void virtio_queue_set_notification(VirtQueue *vq, int enable)
-- 
1.8.1.2




[Qemu-devel] [PATCH 1/8] virtio_get_byteswap: function for endian-ambivalent targets using virtio.

2013-08-12 Thread Rusty Russell
virtio data structures are defined as "target endian", which assumes
that's a fixed value.  In fact, that actually means it's
platform-specific.

Hopefully the OASIS virtio 1.0 spec will fix this.  Meanwhile, create
a hook for little endian ppc.

Signed-off-by: Rusty Russell 
---
 include/hw/virtio/virtio-access.h | 130 ++
 include/hw/virtio/virtio.h|   2 +
 stubs/Makefile.objs   |   1 +
 stubs/virtio_get_byteswap.c   |   6 ++
 4 files changed, 139 insertions(+)
 create mode 100644 include/hw/virtio/virtio-access.h
 create mode 100644 stubs/virtio_get_byteswap.c

diff --git a/include/hw/virtio/virtio-access.h 
b/include/hw/virtio/virtio-access.h
new file mode 100644
index 000..01d7dd6
--- /dev/null
+++ b/include/hw/virtio/virtio-access.h
@@ -0,0 +1,130 @@
+/*
+ * Virtio Accessor Support: In case your target can change endian.
+ *
+ * Copyright IBM, Corp. 2013
+ *
+ * Authors:
+ *  Rusty Russell   
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ */
+#ifndef _QEMU_VIRTIO_ACCESS_H
+#define _QEMU_VIRTIO_ACCESS_H
+#include "hw/virtio/virtio.h"
+
+static inline uint16_t virtio_lduw_phys(hwaddr pa)
+{
+if (virtio_get_byteswap()) {
+return bswap16(lduw_phys(pa));
+}
+return lduw_phys(pa);
+
+}
+
+static inline uint32_t virtio_ldl_phys(hwaddr pa)
+{
+if (virtio_get_byteswap()) {
+return bswap32(ldl_phys(pa));
+}
+return ldl_phys(pa);
+}
+
+static inline uint64_t virtio_ldq_phys(hwaddr pa)
+{
+if (virtio_get_byteswap()) {
+return bswap64(ldq_phys(pa));
+}
+return ldq_phys(pa);
+}
+
+static inline void virtio_stw_phys(hwaddr pa, uint16_t value)
+{
+if (virtio_get_byteswap()) {
+stw_phys(pa, bswap16(value));
+} else {
+stw_phys(pa, value);
+}
+}
+
+static inline void virtio_stl_phys(hwaddr pa, uint32_t value)
+{
+if (virtio_get_byteswap()) {
+stl_phys(pa, bswap32(value));
+} else {
+stl_phys(pa, value);
+}
+}
+
+static inline void virtio_stw_p(void *ptr, uint16_t v)
+{
+if (virtio_get_byteswap()) {
+stw_p(ptr, bswap16(v));
+} else {
+stw_p(ptr, v);
+}
+}
+
+static inline void virtio_stl_p(void *ptr, uint32_t v)
+{
+if (virtio_get_byteswap()) {
+stl_p(ptr, bswap32(v));
+} else {
+stl_p(ptr, v);
+}
+}
+
+static inline void virtio_stq_p(void *ptr, uint64_t v)
+{
+if (virtio_get_byteswap()) {
+stq_p(ptr, bswap64(v));
+} else {
+stq_p(ptr, v);
+}
+}
+
+static inline int virtio_lduw_p(const void *ptr)
+{
+if (virtio_get_byteswap()) {
+return bswap16(lduw_p(ptr));
+} else {
+return lduw_p(ptr);
+}
+}
+
+static inline int virtio_ldl_p(const void *ptr)
+{
+if (virtio_get_byteswap()) {
+return bswap32(ldl_p(ptr));
+} else {
+return ldl_p(ptr);
+}
+}
+
+static inline uint64_t virtio_ldq_p(const void *ptr)
+{
+if (virtio_get_byteswap()) {
+return bswap64(ldq_p(ptr));
+} else {
+return ldq_p(ptr);
+}
+}
+
+static inline uint32_t virtio_tswap32(uint32_t s)
+{
+if (virtio_get_byteswap()) {
+return bswap32(tswap32(s));
+} else {
+return tswap32(s);
+}
+}
+
+static inline void virtio_tswap32s(uint32_t *s)
+{
+tswap32s(s);
+if (virtio_get_byteswap()) {
+*s = bswap32(*s);
+}
+}
+#endif /* _QEMU_VIRTIO_ACCESS_H */
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index d7e9e0f..18ca725 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -248,4 +248,6 @@ void virtio_queue_set_host_notifier_fd_handler(VirtQueue 
*vq, bool assign,
bool set_handler);
 void virtio_queue_notify_vq(VirtQueue *vq);
 void virtio_irq(VirtQueue *vq);
+
+extern bool virtio_get_byteswap(void);
 #endif
diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index f306cba..05f7bab 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -26,3 +26,4 @@ stub-obj-y += vm-stop.o
 stub-obj-y += vmstate.o
 stub-obj-$(CONFIG_WIN32) += fd-register.o
 stub-obj-y += cpus.o
+stub-obj-y += virtio_get_byteswap.o
diff --git a/stubs/virtio_get_byteswap.c b/stubs/virtio_get_byteswap.c
new file mode 100644
index 000..7cf764d
--- /dev/null
+++ b/stubs/virtio_get_byteswap.c
@@ -0,0 +1,6 @@
+#include "hw/virtio/virtio.h"
+
+bool virtio_get_byteswap(void)
+{
+return false;
+}
-- 
1.8.1.2




[Qemu-devel] [PATCH 2/8] target-ppc: ppc64 target's virtio can be either endian.

2013-08-12 Thread Rusty Russell
We base it on the OS endian, as reflected by the endianness of the
interrupt vectors.

Suggested-by: Benjamin Herrenschmidt 
Signed-off-by: Rusty Russell 
---
 target-ppc/misc_helper.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/target-ppc/misc_helper.c b/target-ppc/misc_helper.c
index 616aab6..6c97c81 100644
--- a/target-ppc/misc_helper.c
+++ b/target-ppc/misc_helper.c
@@ -20,6 +20,7 @@
 #include "helper.h"
 
 #include "helper_regs.h"
+#include "hw/virtio/virtio.h"
 
 /*/
 /* SPR accesses */
@@ -116,3 +117,11 @@ void ppc_store_msr(CPUPPCState *env, target_ulong value)
 {
 hreg_store_msr(env, value, 0);
 }
+
+bool virtio_get_byteswap(void)
+{
+PowerPCCPU *cp = POWERPC_CPU(first_cpu);
+CPUPPCState *env = &cp->env;
+
+return env->spr[SPR_LPCR] & LPCR_ILE;
+}
-- 
1.8.1.2




[Qemu-devel] seabios 1.7.3.1 released

2013-08-12 Thread Gerd Hoffmann
  Hi,

The seabios 1.7.3.1 bugfix release is out of the door.
Here is the shortlog:

Kevin O'Connor (2):
  Fix USB EHCI detection that was broken in hlist conversion of
PCIDevices.
  Fix bug in CBFS file walking with compressed files.

Michael S. Tsirkin (1):
  acpi: sync FADT flags from PIIX4 to Q35

cheers,
  Gerd



[Qemu-devel] [PATCH for-1.6 0/1] seabios: update to 1.7.3.1 bugfix release

2013-08-12 Thread Gerd Hoffmann
  Hi,

seabios 1.7.3.1 has been released with a few bugfixes,
here comes the update for qemu.

please pull,
  Gerd

The following changes since commit 2e985fe000e73097e325e18b943e8babfa96c35c:

  mips: revert commit b332d24a8e1290954029814d09156b06ede358e2 (2013-08-08 
23:06:15 +0200)

are available in the git repository at:

  git://git.kraxel.org/qemu seabios-1.7.3.1

for you to fetch changes up to bc55080d60d65964edd361263d3dbbe2b34ca70c:

  seabios: update to 1.7.3.1 bugfix release (2013-08-12 10:29:52 +0200)


Gerd Hoffmann (1):
  seabios: update to 1.7.3.1 bugfix release

 pc-bios/bios.bin |  Bin 131072 -> 131072 bytes
 roms/seabios |2 +-
 2 files changed, 1 insertion(+), 1 deletion(-)



[Qemu-devel] [PATCH 1/1] seabios: update to 1.7.3.1 bugfix release

2013-08-12 Thread Gerd Hoffmann
shortlog from 1.7.3:

Kevin O'Connor (2):
  Fix USB EHCI detection that was broken in hlist conversion of PCIDevices.
  Fix bug in CBFS file walking with compressed files.

Michael S. Tsirkin (1):
  acpi: sync FADT flags from PIIX4 to Q35

Signed-off-by: Gerd Hoffmann 
---
 pc-bios/bios.bin |  Bin 131072 -> 131072 bytes
 roms/seabios |2 +-
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/pc-bios/bios.bin b/pc-bios/bios.bin
index 
487814eb68a2d25c3c90e088bcc60a3da2f8..8c9a487e20f248e4afc8ce3170eef37df4ff2cea
 100644
GIT binary patch
[ ... snipped ... ]

diff --git a/roms/seabios b/roms/seabios
index d4f7d90..7d9cbe6 16
--- a/roms/seabios
+++ b/roms/seabios
@@ -1 +1 @@
-Subproject commit d4f7d90f47462b4e8836899adc5060fbde5253e9
+Subproject commit 7d9cbe613694924921ed1a6f8947d711c5832eee
-- 
1.7.9.7




Re: [Qemu-devel] [PATCH 1/2] vmdk: support vmfsSparse files

2013-08-12 Thread Andreas Färber
Am 11.08.2013 18:13, schrieb Paolo Bonzini:
> VMware ESX hosts use a variant of the VMDK3 format, identified by the
> vmfsSparse create type ad the VMFSSPARSE extent type.

"and"?

> It has 16 KB grain tables (L2) and a variable-size grain directory (L1).
> In addition, the grain size is always 512, but that is not a problem
> because it is included in the header.
> 
> The format of the extents is documented in the VMDK spec.  The format
> of the descriptor file is not documented precisely, but it can be
> found at http://kb.vmware.com/kb/10026353 (Recreating a missing virtual
> machine disk (VMDK) descriptor file for delta disks).
> 
> With these patches, vmfsSparse files only work if opened through the
> descriptor file.  Data files without descriptor files, as far as I
> could understand, are not supported by ESX.
> 
> Signed-off-by: Paolo Bonzini 
> ---
>  block/vmdk.c | 51 ++-
>  1 file changed, 46 insertions(+), 5 deletions(-)
> 
> diff --git a/block/vmdk.c b/block/vmdk.c
> index b16d509..eaf484a 100644
> --- a/block/vmdk.c
> +++ b/block/vmdk.c
> @@ -505,6 +505,34 @@ static int vmdk_open_vmdk3(BlockDriverState *bs,
>  return ret;
>  }
>  
> +static int vmdk_open_vmfs_sparse(BlockDriverState *bs,
> + BlockDriverState *file,
> + int flags)
> +{
> +int ret;
> +uint32_t magic;
> +VMDK3Header header;
> +VmdkExtent *extent;
> +
> +ret = bdrv_pread(file, sizeof(magic), &header, sizeof(header));
> +if (ret < 0) {
> +return ret;
> +}
> +extent = vmdk_add_extent(bs, file, false,
> +  le64_to_cpu(header.disk_sectors),
> +  le64_to_cpu(header.l1dir_offset) << 9,
> +  0,
> +  le64_to_cpu(header.l1dir_size) * 4,
> +  4096,
> +  le64_to_cpu(header.granularity)); /* always 512 */

Indentation is 3 off (guessing the variable was called ext before ;)).

Andreas

> +ret = vmdk_init_tables(bs, extent);
> +if (ret) {
> +/* free extent allocated by vmdk_add_extent */
> +vmdk_free_last_extent(bs);
> +}
> +return ret;
> +}
> +
>  static int vmdk_open_desc_file(BlockDriverState *bs, int flags,
> uint64_t desc_offset);
>  
> @@ -663,7 +691,7 @@ static int vmdk_parse_description(const char *desc, const 
> char *opt_name,
>  /* Open an extent file and append to bs array */
>  static int vmdk_open_sparse(BlockDriverState *bs,
>  BlockDriverState *file,
> -int flags)
> +int flags, bool vmfs_sparse)
>  {
>  uint32_t magic;
>  
> @@ -674,7 +702,11 @@ static int vmdk_open_sparse(BlockDriverState *bs,
>  magic = be32_to_cpu(magic);
>  switch (magic) {
>  case VMDK3_MAGIC:
> -return vmdk_open_vmdk3(bs, file, flags);
> +if (vmfs_sparse) {
> +return vmdk_open_vmfs_sparse(bs, file, flags);
> +} else {
> +return vmdk_open_vmdk3(bs, file, flags);
> +}
>  break;
>  case VMDK4_MAGIC:
>  return vmdk_open_vmdk4(bs, file, flags);
> @@ -718,7 +750,8 @@ static int vmdk_parse_extents(const char *desc, 
> BlockDriverState *bs,
>  }
>  
>  if (sectors <= 0 ||
> -(strcmp(type, "FLAT") && strcmp(type, "SPARSE")) ||
> +(strcmp(type, "FLAT") && strcmp(type, "SPARSE") &&
> + strcmp(type, "VMFSSPARSE")) ||
>  (strcmp(access, "RW"))) {
>  goto next_line;
>  }
> @@ -743,7 +776,14 @@ static int vmdk_parse_extents(const char *desc, 
> BlockDriverState *bs,
>  extent->flat_start_offset = flat_offset << 9;
>  } else if (!strcmp(type, "SPARSE")) {
>  /* SPARSE extent */
> -ret = vmdk_open_sparse(bs, extent_file, bs->open_flags);
> +ret = vmdk_open_sparse(bs, extent_file, bs->open_flags, false);
> +if (ret) {
> +bdrv_delete(extent_file);
> +return ret;
> +}
> +} else if (!strcmp(type, "VMFSSPARSE")) {
> +/* VMFSSPARSE extent */
> +ret = vmdk_open_sparse(bs, extent_file, bs->open_flags, true);
>  if (ret) {
>  bdrv_delete(extent_file);
>  return ret;
> @@ -789,6 +829,7 @@ static int vmdk_open_desc_file(BlockDriverState *bs, int 
> flags,
>  goto exit;
>  }
>  if (strcmp(ct, "monolithicFlat") &&
> +strcmp(ct, "vmfsSparse") &&
>  strcmp(ct, "twoGbMaxExtentSparse") &&
>  strcmp(ct, "twoGbMaxExtentFlat")) {
>  fprintf(stderr,
> @@ -808,7 +849,7 @@ static int vmdk_open(BlockDriverState *bs, QDict 
> *options, int flags)
>  int ret;
>  BDRVVmdkState *s = bs->opaque;
>  
> -if

[Qemu-devel] [PATCH for-1.6 1/2] memory: export target page size

2013-08-12 Thread Michael S. Tsirkin
Add symbol to make it possible to use target page size
in target-independent code.

Signed-off-by: Michael S. Tsirkin 
---
 exec.c| 2 ++
 include/exec/memory.h | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/exec.c b/exec.c
index 3ca9381..965076e 100644
--- a/exec.c
+++ b/exec.c
@@ -2659,6 +2659,8 @@ bool virtio_is_big_endian(void)
 
 #endif
 
+uint64_t qemu_target_page_size = TARGET_PAGE_SIZE;
+
 #ifndef CONFIG_USER_ONLY
 bool cpu_physical_memory_is_io(hwaddr phys_addr)
 {
diff --git a/include/exec/memory.h b/include/exec/memory.h
index ebe0d24..3ed747c 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -1056,6 +1056,8 @@ void address_space_unmap(AddressSpace *as, void *buffer, 
hwaddr len,
  int is_write, hwaddr access_len);
 
 
+extern uint64_t qemu_target_page_size;
+
 #endif
 
 #endif
-- 
MST




[Qemu-devel] [PATCH for-1.6 0/2] future proof rom loading for cross versiom migration

2013-08-12 Thread Michael S. Tsirkin
ROM files that are put in FW CFG are copied to guest ram, by BIOS, but
they are not backed by RAM so they don't get migrated.

Each time we'll change at least two bytes in such a ROM this will break
cross-version migration: since we can migrate after BIOS has read the first
byte but before it has read the second one, getting an inconsistent state.

This patchset makes QEMU future-proof against such changes.

Naturally, this only helps for -M 1.6 and up, older machine types
will still have the cross-version migration bug.

I think this should be applied for 1.6, this way we won't
have this problem from 1.7 and on.

Michael S. Tsirkin (2):
  memory: export target page size
  loader: put FW CFG ROM files into RAM

 exec.c|  2 ++
 hw/core/loader.c  | 54 ---
 hw/i386/pc_piix.c |  2 ++
 hw/i386/pc_q35.c  |  2 ++
 include/exec/memory.h |  2 ++
 include/hw/loader.h   |  1 +
 6 files changed, 60 insertions(+), 3 deletions(-)

-- 
MST




Re: [Qemu-devel] [PATCH] gdb: Fix gdb error

2013-08-12 Thread Andreas Färber
Hi Aneesh,

Am 11.08.2013 20:14, schrieb Aneesh Kumar K.V:
> From: "Aneesh Kumar K.V" 
> 
> Don't update the global register count if not requested.
> Without this patch a remote gdb session gives
> 
> (gdb) target remote localhost:1234
> Remote debugging using localhost:1234
> Remote 'g' packet reply is too long:
> 2884c0ccba50c0c ...
> 
> ...
> (gdb)
> 
> This is a regression introduce by a0e372f0c49ac01faeaeb73a6e8f50e8ac615f34
> 
> Signed-off-by: Aneesh Kumar K.V 

Thanks for tracking this down. I'm willing to include a variation in
today's pull to fix 1.6.0-rc3. However, did you find an explanation
*why* it needs to be like this? I understand it is a revert to using the
static variable, updated to using the CPUClass field rather than the
previous preprocessor constant.

> ---
>  gdbstub.c | 12 ++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/gdbstub.c b/gdbstub.c
> index 1af25a6..4b58a1e 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -598,6 +598,12 @@ void gdb_register_coprocessor(CPUState *cpu,
>  {
>  GDBRegisterState *s;
>  GDBRegisterState **p;
> +static int last_reg;
> +CPUClass *cc = CPU_GET_CLASS(cpu);
> +
> +if (!last_reg) {
> +last_reg = cc->gdb_num_core_regs;
> +}
>  
>  p = &cpu->gdb_regs;
>  while (*p) {
> @@ -608,19 +614,21 @@ void gdb_register_coprocessor(CPUState *cpu,
>  }
>  
>  s = g_new0(GDBRegisterState, 1);
> -s->base_reg = cpu->gdb_num_regs;
> +s->base_reg = last_reg;
>  s->num_regs = num_regs;
>  s->get_reg = get_reg;
>  s->set_reg = set_reg;
>  s->xml = xml;
>  
>  /* Add to end of list.  */
> -cpu->gdb_num_regs += num_regs;
> +last_reg += num_regs;
>  *p = s;
>  if (g_pos) {
>  if (g_pos != s->base_reg) {
>  fprintf(stderr, "Error: Bad gdb register numbering for '%s'\n"
>  "Expected %d got %d\n", xml, g_pos, s->base_reg);

> +} else {
> +cpu->gdb_num_regs = last_reg;

This bit looks wrong to me - it is updating the per-CPU count with the
global value. Could you retest without this please?

Regards,
Andreas

>  }
>  }
>  }
> 


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



[Qemu-devel] [PATCH for-1.6 2/2] loader: put FW CFG ROM files into RAM

2013-08-12 Thread Michael S. Tsirkin
ROM files that are put in FW CFG are copied to guest ram, by BIOS, but
they are not backed by RAM so they don't get migrated.

Each time we change two bytes in such a ROM this breaks cross-version
migration: since we can migrate after BIOS has read the first byte but
before it has read the second one, getting an inconsistent state.

Future-proof this by creating, for each such ROM,
an MR serving as the backing store.
This MR is never mapped into guest memory, but it's registered
as RAM so it's migrated with the guest.

Naturally, this only helps for -M 1.6 and up, older machine types
will still have the cross-version migration bug.
Luckily the race window for the problem to trigger is very small,
which is also likely why we didn't notice the cross-version
migration bug in testing yet.

Cc: Gerd Hoffmann 
Cc: Laszlo Ersek 
Signed-off-by: Michael S. Tsirkin 
---
 hw/core/loader.c| 54 ++---
 hw/i386/pc_piix.c   |  2 ++
 hw/i386/pc_q35.c|  2 ++
 include/hw/loader.h |  1 +
 4 files changed, 56 insertions(+), 3 deletions(-)

diff --git a/hw/core/loader.c b/hw/core/loader.c
index 6875b7e..6e99750 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -54,6 +54,8 @@
 
 #include 
 
+bool rom_file_in_ram = true;
+
 static int roms_loaded;
 
 /* return the size or -1 if error */
@@ -576,6 +578,7 @@ struct Rom {
 size_t datasize;
 
 uint8_t *data;
+MemoryRegion *mr;
 int isrom;
 char *fw_dir;
 char *fw_file;
@@ -605,6 +608,26 @@ static void rom_insert(Rom *rom)
 QTAILQ_INSERT_TAIL(&roms, rom, next);
 }
 
+static void *rom_set_mr(Rom *rom, Object *owner, const char *name)
+{
+/*
+ * Migration code expects that all RAM blocks are full target pages.
+ * Round MR size up to make this work.
+ */
+unsigned size = ROUND_UP(rom->datasize, qemu_target_page_size);
+void *data = g_malloc0(size);
+
+memcpy(data, rom->data, rom->datasize);
+
+rom->mr = g_malloc(sizeof(*rom->mr));
+data = data;
+memory_region_init_ram_ptr(rom->mr, owner, name, size, data);
+memory_region_set_readonly(rom->mr, true);
+vmstate_register_ram_global(rom->mr);
+
+return data;
+}
+
 int rom_add_file(const char *file, const char *fw_dir,
  hwaddr addr, int32_t bootindex)
 {
@@ -646,6 +669,7 @@ int rom_add_file(const char *file, const char *fw_dir,
 if (rom->fw_file && fw_cfg) {
 const char *basename;
 char fw_file_name[56];
+void *data;
 
 basename = strrchr(rom->fw_file, '/');
 if (basename) {
@@ -655,8 +679,15 @@ int rom_add_file(const char *file, const char *fw_dir,
 }
 snprintf(fw_file_name, sizeof(fw_file_name), "%s/%s", rom->fw_dir,
  basename);
-fw_cfg_add_file(fw_cfg, fw_file_name, rom->data, rom->romsize);
 snprintf(devpath, sizeof(devpath), "/rom@%s", fw_file_name);
+
+if (rom_file_in_ram) {
+data = rom_set_mr(rom, OBJECT(fw_cfg), devpath);
+} else {
+data = rom->data;
+}
+
+fw_cfg_add_file(fw_cfg, fw_file_name, data, rom->romsize);
 } else {
 snprintf(devpath, sizeof(devpath), "/rom@" TARGET_FMT_plx, addr);
 }
@@ -731,7 +762,12 @@ static void rom_reset(void *unused)
 if (rom->data == NULL) {
 continue;
 }
-cpu_physical_memory_write_rom(rom->addr, rom->data, rom->datasize);
+if (rom->mr) {
+void *host = memory_region_get_ram_ptr(rom->mr);
+memcpy(host, rom->data, rom->datasize);
+} else {
+cpu_physical_memory_write_rom(rom->addr, rom->data, rom->datasize);
+}
 if (rom->isrom) {
 /* rom needs to be written only once */
 g_free(rom->data);
@@ -781,6 +817,9 @@ static Rom *find_rom(hwaddr addr)
 if (rom->fw_file) {
 continue;
 }
+if (rom->mr) {
+continue;
+}
 if (rom->addr > addr) {
 continue;
 }
@@ -808,6 +847,9 @@ int rom_copy(uint8_t *dest, hwaddr addr, size_t size)
 if (rom->fw_file) {
 continue;
 }
+if (rom->mr) {
+continue;
+}
 if (rom->addr + rom->romsize < addr) {
 continue;
 }
@@ -866,7 +908,13 @@ void do_info_roms(Monitor *mon, const QDict *qdict)
 Rom *rom;
 
 QTAILQ_FOREACH(rom, &roms, next) {
-if (!rom->fw_file) {
+if (rom->mr) {
+monitor_printf(mon, "%s"
+   " size=0x%06zx name=\"%s\"\n",
+   rom->mr->name,
+   rom->romsize,
+   rom->name);
+} else if (!rom->fw_file) {
 monitor_printf(mon, "addr=" TARGET_FMT_plx
" size=0x%06zx mem=%s name=\"%s\"\n",
rom->addr, rom->romsize,
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 9

Re: [Qemu-devel] [PATCH for-1.6 v2 1/2] i82801b11: Fix i82801b11 PCI host bridge config space

2013-08-12 Thread Gerd Hoffmann
On 08/05/13 16:36, Andreas Färber wrote:
> From: Gerd Hoffmann 
> 
> pci_bridge_write_config() was not being used.
> 
> Cc: qemu-sta...@nongnu.org
> Signed-off-by: Gerd Hoffmann 
> Signed-off-by: Andreas Färber 

Ping?  I see it's not yet in master, can we *please* get this in?
It is save to take just this one-liner and defer 2/2 to 1.7.

cheers,
  Gerd




Re: [Qemu-devel] [PATCH for-1.6 v2 1/2] i82801b11: Fix i82801b11 PCI host bridge config space

2013-08-12 Thread Andreas Färber
Am 12.08.2013 10:51, schrieb Gerd Hoffmann:
> On 08/05/13 16:36, Andreas Färber wrote:
>> From: Gerd Hoffmann 
>>
>> pci_bridge_write_config() was not being used.
>>
>> Cc: qemu-sta...@nongnu.org
>> Signed-off-by: Gerd Hoffmann 
>> Signed-off-by: Andreas Färber 
> 
> Ping?  I see it's not yet in master, can we *please* get this in?
> It is save to take just this one-liner and defer 2/2 to 1.7.

Michael, please decide as PCI maintainer and send a pull. Thanks!

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 v3] pci: Introduce helper to retrieve a PCI device's DMA address space

2013-08-12 Thread Michael S. Tsirkin
On Mon, Aug 12, 2013 at 12:36:57AM +1000, Alexey Kardashevskiy wrote:
> On 08/11/2013 11:58 PM, Michael S. Tsirkin wrote:
> > On Sat, Aug 10, 2013 at 01:09:08AM +1000, Alexey Kardashevskiy wrote:
> >> A PCI device's DMA address space (possibly an IOMMU) is returned by a
> >> method on the PCIBus.  At the moment that only has one caller, so the
> >> method is simply open coded.  We'll need another caller for VFIO, so
> >> this patch introduces a helper/wrapper function.
> >>
> >> If IOMMU is not set, the pci_device_iommu_address_space() function
> >> returns the parent's IOMMU skipping the "bus master" address space as
> >> otherwise proper emulation would require more effort for no benefit.
> >>
> >> Signed-off-by: David Gibson 
> >> [aik: added inheritance from parent if iommu is not set for the current 
> >> bus]
> >> Signed-off-by: Alexey Kardashevskiy 
> >>
> >> ---
> >> Changes:
> >> v3:
> >> * added comment about ignoring bus master address space
> >>
> >> v2:
> >> * added inheritance, needed for a pci-bridge on spapr-ppc64
> >> * pci_iommu_as renamed to pci_device_iommu_address_space
> >> ---
> >>  hw/pci/pci.c | 24 ++--
> >>  include/hw/pci/pci.h |  1 +
> >>  2 files changed, 19 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> >> index 4c004f5..dbfa395 100644
> >> --- a/hw/pci/pci.c
> >> +++ b/hw/pci/pci.c
> >> @@ -812,12 +812,7 @@ static PCIDevice *do_pci_register_device(PCIDevice 
> >> *pci_dev, PCIBus *bus,
> >>  }
> >>  
> >>  pci_dev->bus = bus;
> >> -if (bus->iommu_fn) {
> >> -dma_as = bus->iommu_fn(bus, bus->iommu_opaque, devfn);
> >> -} else {
> >> -/* FIXME: inherit memory region from bus creator */
> >> -dma_as = &address_space_memory;
> >> -}
> >> +dma_as = pci_device_iommu_address_space(pci_dev);
> >>  
> >>  memory_region_init_alias(&pci_dev->bus_master_enable_region,
> >>   OBJECT(pci_dev), "bus master",
> >> @@ -2239,6 +2234,23 @@ static void pci_device_class_init(ObjectClass 
> >> *klass, void *data)
> >>  k->props = pci_props;
> >>  }
> >>  
> >> +AddressSpace *pci_device_iommu_address_space(PCIDevice *dev)
> >> +{
> >> +PCIBus *bus = PCI_BUS(dev->bus);
> >> +
> >> +if (bus->iommu_fn) {
> >> +return bus->iommu_fn(bus, bus->iommu_opaque, dev->devfn);
> >> +}
> >> +
> >> +if (bus->parent_dev) {
> >> +/** We are ignoring the bus master DMA bit of the bridge
> >> + *  as it would complicate things such as VFIO for no good reason 
> >> */
> > 
> > /*
> >  * Always
> >  * like
> >  * this
> >  */
> > 
> > /** Never
> >  * like this */
> 
> 
> Hm. I thought I saw a lot of those but it was the kernel :)
> btw may comments start with "/**" (with no text in that line but still) -
> what is the difference to "/*"?

/** are normally for automated generation of docbook from code.
I don't think we do that for QEMU, but in any case, this
would be only any good for properly formatted comments
at top of a function.

> 
> > The comment should be improved I think.
> > I would put it like this:
> > /*
> >  * Note: this does not check bus master enable bit on device or
> >  * any of the pci to pci bridges above it, it's up to the caller to
> >  * check that before initiating the transaction.
> >  *
> >  * TODO: design a mechanism for callers to do this without
> >  * doing bus scans on data path.
> >  */
> 
> What exactly do you call here "bus scans"?

Probably better as 'PCI hierarchy walk'.

> 
> > Would you like me to queue this on the pci tree? If yes I can
> > tweak the comment myself, no need to repost.
> 
> Yes, please. Your tree is fine. Thanks!
> 

OK, I'll apply this.
Thanks!

> >> +return pci_device_iommu_address_space(bus->parent_dev);
> >> +}
> >> +
> >> +return &address_space_memory;
> >> +}
> >> +
> >>  void pci_setup_iommu(PCIBus *bus, PCIIOMMUFunc fn, void *opaque)
> >>  {
> >>  bus->iommu_fn = fn;
> >> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> >> index ccec2ba..2374aa9 100644
> >> --- a/include/hw/pci/pci.h
> >> +++ b/include/hw/pci/pci.h
> >> @@ -405,6 +405,7 @@ void pci_device_deassert_intx(PCIDevice *dev);
> >>  
> >>  typedef AddressSpace *(*PCIIOMMUFunc)(PCIBus *, void *, int);
> >>  
> >> +AddressSpace *pci_device_iommu_address_space(PCIDevice *dev);
> >>  void pci_setup_iommu(PCIBus *bus, PCIIOMMUFunc fn, void *opaque);
> >>  
> >>  static inline void
> 
> 
> 
> -- 
> Alexey



Re: [Qemu-devel] [PATCHv2] add qemu-img convert -C option (skip target volume creation)

2013-08-12 Thread Fam Zheng
On Mon, 08/12 08:09, Alex Bligh wrote:
> Add a -C option to skip volume creation on qemu-img convert.
> This is useful for targets such as rbd / ceph, where the
> target volume may already exist; we cannot always rely on
> qemu-img convert to create the image, as dependent on the
> output format, there may be parameters which are not possible
> to specify through the qemu-img convert command line.
> 
> Code:
> 
> Author: Alexandre Derumier 
> Signed-off-by: Alexandre Derumier 
> Signed-off-by: Alex Bligh 
> 
> Documentaton:
> 
> Author: Alex Bligh 
> Signed-off-by: Alex Bligh 
> ---
>  qemu-img-cmds.hx |4 ++--
>  qemu-img.c   |   39 ---
>  qemu-img.texi|   15 ++-
>  3 files changed, 40 insertions(+), 18 deletions(-)
> 
> diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
> index 4ca7e95..74ced81 100644
> --- a/qemu-img-cmds.hx
> +++ b/qemu-img-cmds.hx
> @@ -34,9 +34,9 @@ STEXI
>  ETEXI
>  
>  DEF("convert", img_convert,
> -"convert [-c] [-p] [-q] [-f fmt] [-t cache] [-O output_fmt] [-o options] 
> [-s snapshot_name] [-S sparse_size] filename [filename2 [...]] 
> output_filename")
> +"convert [-c] [-p] [-q] [-C] [-f fmt] [-t cache] [-O output_fmt] [-o 
> options] [-s snapshot_name] [-S sparse_size] filename [filename2 [...]] 
> output_filename")
>  STEXI
> -@item convert [-c] [-p] [-q] [-f @var{fmt}] [-t @var{cache}] [-O 
> @var{output_fmt}] [-o @var{options}] [-s @var{snapshot_name}] [-S 
> @var{sparse_size}] @var{filename} [@var{filename2} [...]] 
> @var{output_filename}
> +@item convert [-c] [-p] [-q] [-C] [-f @var{fmt}] [-t @var{cache}] [-O 
> @var{output_fmt}] [-o @var{options}] [-s @var{snapshot_name}] [-S 
> @var{sparse_size}] @var{filename} [@var{filename2} [...]] 
> @var{output_filename}
>  ETEXI
>  
>  DEF("info", img_info,
> diff --git a/qemu-img.c b/qemu-img.c
> index b9a848d..c6e1cc0 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -103,6 +103,8 @@ static void help(void)
> "  '-S' indicates the consecutive number of bytes that must 
> contain only zeros\n"
> "   for qemu-img to create a sparse image during conversion\n"
> "  '--output' takes the format in which the output must be done 
> (human or json)\n"
> +   "  '-C' skips the target volume creation (useful if the volume is 
> created)\n"
> +   "   prior to running qemu-img)\n"

Parenthesis not balanced. Otherwise,

Reviewed-by: Fam Zheng 

> "\n"
> "Parameters to check subcommand:\n"
> "  '-r' tries to repair any inconsistencies that are found during 
> the check.\n"
> @@ -1116,7 +1118,8 @@ out3:
>  
>  static int img_convert(int argc, char **argv)
>  {
> -int c, ret = 0, n, n1, bs_n, bs_i, compress, cluster_size, 
> cluster_sectors;
> +int c, ret = 0, n, n1, bs_n, bs_i, compress, cluster_size,
> +cluster_sectors, skipcreate;
>  int progress = 0, flags;
>  const char *fmt, *out_fmt, *cache, *out_baseimg, *out_filename;
>  BlockDriver *drv, *proto_drv;
> @@ -1139,8 +1142,9 @@ static int img_convert(int argc, char **argv)
>  cache = "unsafe";
>  out_baseimg = NULL;
>  compress = 0;
> +skipcreate = 0;
>  for(;;) {
> -c = getopt(argc, argv, "f:O:B:s:hce6o:pS:t:q");
> +c = getopt(argc, argv, "f:O:B:s:hce6o:pS:t:qC");
>  if (c == -1) {
>  break;
>  }
> @@ -1161,6 +1165,9 @@ static int img_convert(int argc, char **argv)
>  case 'c':
>  compress = 1;
>  break;
> +case 'C':
> +skipcreate = 1;
> +break;
>  case 'e':
>  error_report("option -e is deprecated, please use \'-o "
>"encryption\' instead!");
> @@ -1329,20 +1336,22 @@ static int img_convert(int argc, char **argv)
>  }
>  }
>  
> -/* Create the new image */
> -ret = bdrv_create(drv, out_filename, param);
> -if (ret < 0) {
> -if (ret == -ENOTSUP) {
> -error_report("Formatting not supported for file format '%s'",
> - out_fmt);
> -} else if (ret == -EFBIG) {
> -error_report("The image size is too large for file format '%s'",
> - out_fmt);
> -} else {
> -error_report("%s: error while converting %s: %s",
> - out_filename, out_fmt, strerror(-ret));
> +if (!skipcreate) {
> +/* Create the new image */
> +ret = bdrv_create(drv, out_filename, param);
> +if (ret < 0) {
> +if (ret == -ENOTSUP) {
> +error_report("Formatting not supported for file format '%s'",
> + out_fmt);
> +} else if (ret == -EFBIG) {
> +error_report("The image size is too large for file format 
> '%s'",
> + out_fmt);
> +} else {
> +error_report("%s: error while con

[Qemu-devel] [PATCH for-1.6] virtio: clear signalled_used_valid when switching from dataplane

2013-08-12 Thread Stefan Hajnoczi
When the dataplane thread stops, its vring.c implementation synchronizes
vring state back to virtio.c so we can continue emulating the virtio
device.

This patch ensures that virtio.c's signalled_used_valid flag is reset so
that we do not suppress guest notifications due to stale signalled_used
values.

Suggested-by: Kevin Wolf 
Signed-off-by: Stefan Hajnoczi 
---
 hw/virtio/dataplane/vring.c | 1 +
 hw/virtio/virtio.c  | 5 +
 include/hw/virtio/virtio.h  | 1 +
 3 files changed, 7 insertions(+)

diff --git a/hw/virtio/dataplane/vring.c b/hw/virtio/dataplane/vring.c
index 82cc151..351a343 100644
--- a/hw/virtio/dataplane/vring.c
+++ b/hw/virtio/dataplane/vring.c
@@ -52,6 +52,7 @@ bool vring_setup(Vring *vring, VirtIODevice *vdev, int n)
 void vring_teardown(Vring *vring, VirtIODevice *vdev, int n)
 {
 virtio_queue_set_last_avail_idx(vdev, n, vring->last_avail_idx);
+virtio_queue_invalidate_signalled_used(vdev, n);
 
 hostmem_finalize(&vring->hostmem);
 }
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 09f62c6..706bdf4 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -1059,6 +1059,11 @@ void virtio_queue_set_last_avail_idx(VirtIODevice *vdev, 
int n, uint16_t idx)
 vdev->vq[n].last_avail_idx = idx;
 }
 
+void virtio_queue_invalidate_signalled_used(VirtIODevice *vdev, int n)
+{
+vdev->vq[n].signalled_used_valid = false;
+}
+
 VirtQueue *virtio_get_queue(VirtIODevice *vdev, int n)
 {
 return vdev->vq + n;
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index d7e9e0f..a90522d 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -237,6 +237,7 @@ hwaddr virtio_queue_get_used_size(VirtIODevice *vdev, int 
n);
 hwaddr virtio_queue_get_ring_size(VirtIODevice *vdev, int n);
 uint16_t virtio_queue_get_last_avail_idx(VirtIODevice *vdev, int n);
 void virtio_queue_set_last_avail_idx(VirtIODevice *vdev, int n, uint16_t idx);
+void virtio_queue_invalidate_signalled_used(VirtIODevice *vdev, int n);
 VirtQueue *virtio_get_queue(VirtIODevice *vdev, int n);
 uint16_t virtio_get_queue_index(VirtQueue *vq);
 int virtio_queue_get_id(VirtQueue *vq);
-- 
1.8.3.1




Re: [Qemu-devel] [PATCH 1/2] vmdk: support vmfsSparse files

2013-08-12 Thread Fam Zheng
On Mon, 08/12 03:40, Paolo Bonzini wrote:
> 
> 
> - Original Message -
> > From: "Fam Zheng" 
> > To: "Paolo Bonzini" 
> > Cc: qemu-devel@nongnu.org, kw...@redhat.com
> > Sent: Monday, August 12, 2013 3:31:44 AM
> > Subject: Re: [PATCH 1/2] vmdk: support vmfsSparse files
> > 
> > On Sun, 08/11 18:13, Paolo Bonzini wrote:
> > > VMware ESX hosts use a variant of the VMDK3 format, identified by the
> > > vmfsSparse create type ad the VMFSSPARSE extent type.
> > > 
> > > It has 16 KB grain tables (L2) and a variable-size grain directory (L1).
> > > In addition, the grain size is always 512, but that is not a problem
> > > because it is included in the header.
> > > 
> > > The format of the extents is documented in the VMDK spec.  The format
> > > of the descriptor file is not documented precisely, but it can be
> > > found at http://kb.vmware.com/kb/10026353 (Recreating a missing virtual
> > > machine disk (VMDK) descriptor file for delta disks).
> > > 
> > I don't have access to this link
> 
> It only works from Google.  Try Googling for "vmfssparse delta".
> 
> > , could you include some documents to
> > this descriptor format in comment or commit message? IIRC, it's only the
> > type be "VMFSSPARSE", right?
> 
> Yes.  And "VMFS" for the base (patch 2).
> 
> > What version of ESX has this format?
> 
> At least 4.0 and newer.
> 
> > This needs to be rebased, vmdk_add_extent() signature has been changed
> > in:
> > 
> > commit 8aa1331c09a9b899f48d97f097bb49b7d458be1c
> > Author: Fam Zheng 
> > Date:   Tue Aug 6 15:44:51 2013 +0800
> > 
> > vmdk: check granularity field in opening
> > 
> > Granularity is used to calculate the cluster size and allocate r/w
> > buffer. Check the value from image before using it, so we don't
> > abort()
> > for unbounded memory allocation.
> > 
> > Signed-off-by: Fam Zheng 
> > Signed-off-by: Kevin Wolf 
> > 
> > Since the new function is a variant of vmdk_open_vmdk3(), would you
> > consider doing a tiny refactor and reduce duplication? And l1dir_size
> > and granularity need to be checked, as in vmdk_open_vmdk4().
> 
> I am not sure how to refactor it... in fact, since I'm on vacation I
> wouldn't mind if somebody else fixes the patch.  I can test it either
> tomorrow or next week.
> 
OK, I'll take it for your happy vocation. :)

-- 
Fam



Re: [Qemu-devel] [PATCH for-1.6] virtio: clear signalled_used_valid when switching from dataplane

2013-08-12 Thread Michael S. Tsirkin
On Mon, Aug 12, 2013 at 11:08:09AM +0200, Stefan Hajnoczi wrote:
> When the dataplane thread stops, its vring.c implementation synchronizes
> vring state back to virtio.c so we can continue emulating the virtio
> device.
> 
> This patch ensures that virtio.c's signalled_used_valid flag is reset so
> that we do not suppress guest notifications due to stale signalled_used
> values.
> 
> Suggested-by: Kevin Wolf 
> Signed-off-by: Stefan Hajnoczi 

Good point

Reviewed-by: Michael S. Tsirkin 

and we also need this for vhost.c right?

> ---
>  hw/virtio/dataplane/vring.c | 1 +
>  hw/virtio/virtio.c  | 5 +
>  include/hw/virtio/virtio.h  | 1 +
>  3 files changed, 7 insertions(+)
> 
> diff --git a/hw/virtio/dataplane/vring.c b/hw/virtio/dataplane/vring.c
> index 82cc151..351a343 100644
> --- a/hw/virtio/dataplane/vring.c
> +++ b/hw/virtio/dataplane/vring.c
> @@ -52,6 +52,7 @@ bool vring_setup(Vring *vring, VirtIODevice *vdev, int n)
>  void vring_teardown(Vring *vring, VirtIODevice *vdev, int n)
>  {
>  virtio_queue_set_last_avail_idx(vdev, n, vring->last_avail_idx);
> +virtio_queue_invalidate_signalled_used(vdev, n);
>  
>  hostmem_finalize(&vring->hostmem);
>  }
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 09f62c6..706bdf4 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -1059,6 +1059,11 @@ void virtio_queue_set_last_avail_idx(VirtIODevice 
> *vdev, int n, uint16_t idx)
>  vdev->vq[n].last_avail_idx = idx;
>  }
>  
> +void virtio_queue_invalidate_signalled_used(VirtIODevice *vdev, int n)
> +{
> +vdev->vq[n].signalled_used_valid = false;
> +}
> +
>  VirtQueue *virtio_get_queue(VirtIODevice *vdev, int n)
>  {
>  return vdev->vq + n;
> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> index d7e9e0f..a90522d 100644
> --- a/include/hw/virtio/virtio.h
> +++ b/include/hw/virtio/virtio.h
> @@ -237,6 +237,7 @@ hwaddr virtio_queue_get_used_size(VirtIODevice *vdev, int 
> n);
>  hwaddr virtio_queue_get_ring_size(VirtIODevice *vdev, int n);
>  uint16_t virtio_queue_get_last_avail_idx(VirtIODevice *vdev, int n);
>  void virtio_queue_set_last_avail_idx(VirtIODevice *vdev, int n, uint16_t 
> idx);
> +void virtio_queue_invalidate_signalled_used(VirtIODevice *vdev, int n);
>  VirtQueue *virtio_get_queue(VirtIODevice *vdev, int n);
>  uint16_t virtio_get_queue_index(VirtQueue *vq);
>  int virtio_queue_get_id(VirtQueue *vq);
> -- 
> 1.8.3.1



Re: [Qemu-devel] [PATCH for-1.6 1/2] memory: export target page size

2013-08-12 Thread Peter Maydell
On 12 August 2013 09:49, Michael S. Tsirkin  wrote:
> Add symbol to make it possible to use target page size
> in target-independent code.

Given that TARGET_PAGE_SIZE is "smallest page size the
TCG implementation currently supports for this core"
(ie it is a TCG internal implementation detail as much
as a property of the target CPU and it may well not
match the actual page size being used by the guest)
I would be very suspicious of any target-independent
code that uses it.

-- PMM



Re: [Qemu-devel] [PATCH] gdb: Fix gdb error

2013-08-12 Thread Aneesh Kumar K.V
Andreas Färber  writes:

> Hi Aneesh,
>
> Am 11.08.2013 20:14, schrieb Aneesh Kumar K.V:
>> From: "Aneesh Kumar K.V" 
>> 
>> Don't update the global register count if not requested.
>> Without this patch a remote gdb session gives
>> 
>> (gdb) target remote localhost:1234
>> Remote debugging using localhost:1234
>> Remote 'g' packet reply is too long:
>> 2884c0ccba50c0c ...
>> 
>> ...
>> (gdb)
>> 
>> This is a regression introduce by a0e372f0c49ac01faeaeb73a6e8f50e8ac615f34
>> 
>> Signed-off-by: Aneesh Kumar K.V 
>
> Thanks for tracking this down. I'm willing to include a variation in
> today's pull to fix 1.6.0-rc3. However, did you find an explanation
> *why* it needs to be like this?

IIUC our reply packet for 'g' contain more data becaue we ended up with
larger cpu->gdb_num_regs. This only happens for archs that do a
gdb_register_coprocessor with gpos == 0. The older code didn't update
num_g_regs in that case. Not sure why we do like that

> I understand it is a revert to using the
> static variable, updated to using the CPUClass field rather than the
> previous preprocessor constant.
>

I don't really like the patch. But I also don't know enough to fix this
without using the static variable.  If you want me to try another
version please send it across. I can easily reproduce this on PowerPC.

>> ---
>>  gdbstub.c | 12 ++--
>>  1 file changed, 10 insertions(+), 2 deletions(-)
>> 
>> diff --git a/gdbstub.c b/gdbstub.c
>> index 1af25a6..4b58a1e 100644
>> --- a/gdbstub.c
>> +++ b/gdbstub.c
>> @@ -598,6 +598,12 @@ void gdb_register_coprocessor(CPUState *cpu,
>>  {
>>  GDBRegisterState *s;
>>  GDBRegisterState **p;
>> +static int last_reg;
>> +CPUClass *cc = CPU_GET_CLASS(cpu);
>> +
>> +if (!last_reg) {
>> +last_reg = cc->gdb_num_core_regs;
>> +}
>>  
>>  p = &cpu->gdb_regs;
>>  while (*p) {
>> @@ -608,19 +614,21 @@ void gdb_register_coprocessor(CPUState *cpu,
>>  }
>>  
>>  s = g_new0(GDBRegisterState, 1);
>> -s->base_reg = cpu->gdb_num_regs;
>> +s->base_reg = last_reg;
>>  s->num_regs = num_regs;
>>  s->get_reg = get_reg;
>>  s->set_reg = set_reg;
>>  s->xml = xml;
>>  
>>  /* Add to end of list.  */
>> -cpu->gdb_num_regs += num_regs;
>> +last_reg += num_regs;
>>  *p = s;
>>  if (g_pos) {
>>  if (g_pos != s->base_reg) {
>>  fprintf(stderr, "Error: Bad gdb register numbering for '%s'\n"
>>  "Expected %d got %d\n", xml, g_pos, s->base_reg);
>
>> +} else {
>> +cpu->gdb_num_regs = last_reg;
>
> This bit looks wrong to me - it is updating the per-CPU count with the
> global value. Could you retest without this please?
>

We loop with cpu->gdb_num_regs as below in gdb_handle_packet.


-for (addr = 0; addr < num_g_regs && len > 0; addr++) {
+for (addr = 0; addr < s->g_cpu->gdb_num_regs && len > 0; addr++) {

We updated num_g_regs if g_pos is not set before
a0e372f0c49ac01faeaeb73a6e8f50e8ac615f34

@@ -2036,25 +2003,22 @@ void gdb_register_coprocessor(CPUState *cpu,
 }
 
 s = g_new0(GDBRegisterState, 1);
-s->base_reg = last_reg;
+s->base_reg = cpu->gdb_num_regs;
 s->num_regs = num_regs;
 s->get_reg = get_reg;
 s->set_reg = set_reg;
 s->xml = xml;
 
 /* Add to end of list.  */
-last_reg += num_regs;
+cpu->gdb_num_regs += num_regs;
 *p = s;
 if (g_pos) {
 if (g_pos != s->base_reg) {
 fprintf(stderr, "Error: Bad gdb register numbering for '%s'\n"
 "Expected %d got %d\n", xml, g_pos, s->base_reg);
-} else {
-num_g_regs = last_reg;
 }
 }
 }




Re: [Qemu-devel] [PATCH for-1.6 1/2] memory: export target page size

2013-08-12 Thread Michael S. Tsirkin
On Mon, Aug 12, 2013 at 10:17:46AM +0100, Peter Maydell wrote:
> On 12 August 2013 09:49, Michael S. Tsirkin  wrote:
> > Add symbol to make it possible to use target page size
> > in target-independent code.
> 
> Given that TARGET_PAGE_SIZE is "smallest page size the
> TCG implementation currently supports for this core"
> (ie it is a TCG internal implementation detail as much
> as a property of the target CPU and it may well not
> match the actual page size being used by the guest)
> I would be very suspicious of any target-independent
> code that uses it.
> 
> -- PMM

Look at the next patch and the comment justifying its use please.
Point is migration works in units of TARGET_PAGE_SIZE
and expects all RAM blocks to be a multiple of that.

What if we rename this to qemu_migration_page_size ?
Will this address your comment?

-- 
MST



[Qemu-devel] [PATCH for-1.6] vhost: clear signalled_used_valid on vhost stop

2013-08-12 Thread Michael S. Tsirkin
When vhost device stops, its implementation synchronizes kernel state
back to virtio.c so we can continue emulating the device
in userspace.

This patch ensures that virtio.c's signalled_used_valid flag is reset so
that userspace does not suppress guest notifications due to stale
signalled_used values.

Cc: qemu-sta...@nongnu.org
Signed-off-by: Michael S. Tsirkin 
---
 hw/virtio/vhost.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 8f6ab13..9e336ad 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -762,6 +762,7 @@ static void vhost_virtqueue_stop(struct vhost_dev *dev,
 fflush(stderr);
 }
 virtio_queue_set_last_avail_idx(vdev, idx, state.num);
+virtio_queue_invalidate_signalled_used(vdev, idx);
 assert (r >= 0);
 cpu_physical_memory_unmap(vq->ring, virtio_queue_get_ring_size(vdev, idx),
   0, virtio_queue_get_ring_size(vdev, idx));
-- 
MST



Re: [Qemu-devel] [PATCH 1/8] virtio_get_byteswap: function for endian-ambivalent targets using virtio.

2013-08-12 Thread Benjamin Herrenschmidt
On Mon, 2013-08-12 at 17:29 +0930, Rusty Russell wrote:
> virtio data structures are defined as "target endian", which assumes
> that's a fixed value.  In fact, that actually means it's
> platform-specific.
> 
> Hopefully the OASIS virtio 1.0 spec will fix this.  Meanwhile, create
> a hook for little endian ppc.

Ok, sorry if I missed a previous debate on that one but why do you do a
call-out to architecture specific stuff (that is not even inline) on
every access ?

If we consider that virtio byte order is global, can't you make it a
global that is *set* by the architecture rather than *polled* by
virtio ?

We have explicit knowledge of when our endianness change (we get the
hcall or a write to some SPR), we can call virtio *then* to adjust the
endianness rather than having a call-out to the platform on every
access.

Ben.





Re: [Qemu-devel] [PATCH for-1.6 2/2] loader: put FW CFG ROM files into RAM

2013-08-12 Thread Peter Maydell
On 12 August 2013 09:49, Michael S. Tsirkin  wrote:
> +static void *rom_set_mr(Rom *rom, Object *owner, const char *name)
> +{
> +/*
> + * Migration code expects that all RAM blocks are full target pages.
> + * Round MR size up to make this work.
> + */
> +unsigned size = ROUND_UP(rom->datasize, qemu_target_page_size);
> +void *data = g_malloc0(size);

If we don't really care where the data lives (ie we are just
allocating a block for it here) it would be better to get the
memory subsystem to do the allocation, because then the information
about the constraints on the size of the region would be confined
to the memory system.

> +data = data;

Huh?

-- PMM



Re: [Qemu-devel] [PATCHv2] add qemu-img convert -C option (skip target volume creation)

2013-08-12 Thread Alex Bligh

On 12 Aug 2013, at 10:03, Fam Zheng wrote:

>> +   "   prior to running qemu-img)\n"
> 
> Parenthesis not balanced. Otherwise,
> 
> Reviewed-by: Fam Zheng 

Fixed in PATCHv3, just sent.

-- 
Alex Bligh







[Qemu-devel] [PATCHv3] add qemu-img convert -C option (skip target volume creation)

2013-08-12 Thread Alex Bligh
Add a -C option to skip volume creation on qemu-img convert.
This is useful for targets such as rbd / ceph, where the
target volume may already exist; we cannot always rely on
qemu-img convert to create the image, as dependent on the
output format, there may be parameters which are not possible
to specify through the qemu-img convert command line.

Code:

Author: Alexandre Derumier 
Signed-off-by: Alexandre Derumier 
Signed-off-by: Alex Bligh 

Documentaton:

Author: Alex Bligh 
Signed-off-by: Alex Bligh 
---
 qemu-img-cmds.hx |4 ++--
 qemu-img.c   |   39 ---
 qemu-img.texi|   15 ++-
 3 files changed, 40 insertions(+), 18 deletions(-)

diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
index 4ca7e95..74ced81 100644
--- a/qemu-img-cmds.hx
+++ b/qemu-img-cmds.hx
@@ -34,9 +34,9 @@ STEXI
 ETEXI
 
 DEF("convert", img_convert,
-"convert [-c] [-p] [-q] [-f fmt] [-t cache] [-O output_fmt] [-o options] 
[-s snapshot_name] [-S sparse_size] filename [filename2 [...]] output_filename")
+"convert [-c] [-p] [-q] [-C] [-f fmt] [-t cache] [-O output_fmt] [-o 
options] [-s snapshot_name] [-S sparse_size] filename [filename2 [...]] 
output_filename")
 STEXI
-@item convert [-c] [-p] [-q] [-f @var{fmt}] [-t @var{cache}] [-O 
@var{output_fmt}] [-o @var{options}] [-s @var{snapshot_name}] [-S 
@var{sparse_size}] @var{filename} [@var{filename2} [...]] @var{output_filename}
+@item convert [-c] [-p] [-q] [-C] [-f @var{fmt}] [-t @var{cache}] [-O 
@var{output_fmt}] [-o @var{options}] [-s @var{snapshot_name}] [-S 
@var{sparse_size}] @var{filename} [@var{filename2} [...]] @var{output_filename}
 ETEXI
 
 DEF("info", img_info,
diff --git a/qemu-img.c b/qemu-img.c
index b9a848d..146909b 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -103,6 +103,8 @@ static void help(void)
"  '-S' indicates the consecutive number of bytes that must contain 
only zeros\n"
"   for qemu-img to create a sparse image during conversion\n"
"  '--output' takes the format in which the output must be done 
(human or json)\n"
+   "  '-C' skips the target volume creation (useful if the volume is 
created\n"
+   "   prior to running qemu-img)\n"
"\n"
"Parameters to check subcommand:\n"
"  '-r' tries to repair any inconsistencies that are found during 
the check.\n"
@@ -1116,7 +1118,8 @@ out3:
 
 static int img_convert(int argc, char **argv)
 {
-int c, ret = 0, n, n1, bs_n, bs_i, compress, cluster_size, cluster_sectors;
+int c, ret = 0, n, n1, bs_n, bs_i, compress, cluster_size,
+cluster_sectors, skipcreate;
 int progress = 0, flags;
 const char *fmt, *out_fmt, *cache, *out_baseimg, *out_filename;
 BlockDriver *drv, *proto_drv;
@@ -1139,8 +1142,9 @@ static int img_convert(int argc, char **argv)
 cache = "unsafe";
 out_baseimg = NULL;
 compress = 0;
+skipcreate = 0;
 for(;;) {
-c = getopt(argc, argv, "f:O:B:s:hce6o:pS:t:q");
+c = getopt(argc, argv, "f:O:B:s:hce6o:pS:t:qC");
 if (c == -1) {
 break;
 }
@@ -1161,6 +1165,9 @@ static int img_convert(int argc, char **argv)
 case 'c':
 compress = 1;
 break;
+case 'C':
+skipcreate = 1;
+break;
 case 'e':
 error_report("option -e is deprecated, please use \'-o "
   "encryption\' instead!");
@@ -1329,20 +1336,22 @@ static int img_convert(int argc, char **argv)
 }
 }
 
-/* Create the new image */
-ret = bdrv_create(drv, out_filename, param);
-if (ret < 0) {
-if (ret == -ENOTSUP) {
-error_report("Formatting not supported for file format '%s'",
- out_fmt);
-} else if (ret == -EFBIG) {
-error_report("The image size is too large for file format '%s'",
- out_fmt);
-} else {
-error_report("%s: error while converting %s: %s",
- out_filename, out_fmt, strerror(-ret));
+if (!skipcreate) {
+/* Create the new image */
+ret = bdrv_create(drv, out_filename, param);
+if (ret < 0) {
+if (ret == -ENOTSUP) {
+error_report("Formatting not supported for file format '%s'",
+ out_fmt);
+} else if (ret == -EFBIG) {
+error_report("The image size is too large for file format 
'%s'",
+ out_fmt);
+} else {
+error_report("%s: error while converting %s: %s",
+ out_filename, out_fmt, strerror(-ret));
+}
+goto out;
 }
-goto out;
 }
 
 flags = BDRV_O_RDWR;
diff --git a/qemu-img.texi b/qemu-img.texi
index 69f1bda..9e5ba36 100644
--- a/qemu-img.texi
+++ b/qemu-img.texi
@@ -96,6 +96,14 @@ Second image format
 Strict mode - fail 

Re: [Qemu-devel] [PATCH 1/8] virtio_get_byteswap: function for endian-ambivalent targets using virtio.

2013-08-12 Thread Benjamin Herrenschmidt
On Mon, 2013-08-12 at 10:39 +0100, Peter Maydell wrote:
> On 12 August 2013 10:28, Benjamin Herrenschmidt
>  wrote:
> > We have explicit knowledge of when our endianness change (we get the
> > hcall or a write to some SPR), we can call virtio *then* to adjust the
> > endianness rather than having a call-out to the platform on every
> > access.
> 
> ARM doesn't -- I wouldn't expect changing the endianness of
> exceptions via writing to the SCTLR to trap to the hypervisor
> (and in any case it certainly won't result in a return to
> userspace).

But don't you need to reconfigure the bridge (as per our previous
discussion) ? In that case you do need to call out to qemu ...

Cheers,
Ben.






Re: [Qemu-devel] [PATCH 1/8] virtio_get_byteswap: function for endian-ambivalent targets using virtio.

2013-08-12 Thread Peter Maydell
On 12 August 2013 10:28, Benjamin Herrenschmidt
 wrote:
> We have explicit knowledge of when our endianness change (we get the
> hcall or a write to some SPR), we can call virtio *then* to adjust the
> endianness rather than having a call-out to the platform on every
> access.

ARM doesn't -- I wouldn't expect changing the endianness of
exceptions via writing to the SCTLR to trap to the hypervisor
(and in any case it certainly won't result in a return to
userspace).

-- PMM



Re: [Qemu-devel] [PATCHv3] add qemu-img convert -C option (skip target volume creation)

2013-08-12 Thread Alex Bligh



--On 12 August 2013 10:42:05 +0100 Alex Bligh  wrote:


Add a -C option to skip volume creation on qemu-img convert.
This is useful for targets such as rbd / ceph, where the
target volume may already exist; we cannot always rely on
qemu-img convert to create the image, as dependent on the
output format, there may be parameters which are not possible
to specify through the qemu-img convert command line.

Code:

Author: Alexandre Derumier 
Signed-off-by: Alexandre Derumier 
Signed-off-by: Alex Bligh 

Documentaton:


Would you consider taking this for 1.6? It's really hard
to do rbd work without it, and it's pretty harmless.

--
Alex Bligh



Re: [Qemu-devel] [PATCH 1/8] virtio_get_byteswap: function for endian-ambivalent targets using virtio.

2013-08-12 Thread Peter Maydell
On 12 August 2013 10:43, Benjamin Herrenschmidt
 wrote:
> On Mon, 2013-08-12 at 10:39 +0100, Peter Maydell wrote:
>> ARM doesn't -- I wouldn't expect changing the endianness of
>> exceptions via writing to the SCTLR to trap to the hypervisor
>> (and in any case it certainly won't result in a return to
>> userspace).
>
> But don't you need to reconfigure the bridge (as per our previous
> discussion) ? In that case you do need to call out to qemu ...

Bridge? You've lost me, I'm afraid.

-- PMM



Re: [Qemu-devel] [PATCH 1/8] virtio_get_byteswap: function for endian-ambivalent targets using virtio.

2013-08-12 Thread Benjamin Herrenschmidt
On Mon, 2013-08-12 at 10:45 +0100, Peter Maydell wrote:
> On 12 August 2013 10:43, Benjamin Herrenschmidt
>  wrote:
> > On Mon, 2013-08-12 at 10:39 +0100, Peter Maydell wrote:
> >> ARM doesn't -- I wouldn't expect changing the endianness of
> >> exceptions via writing to the SCTLR to trap to the hypervisor
> >> (and in any case it certainly won't result in a return to
> >> userspace).
> >
> > But don't you need to reconfigure the bridge (as per our previous
> > discussion) ? In that case you do need to call out to qemu ...
> 
> Bridge? You've lost me, I'm afraid.

I must be confused ... you mentioned in a previous discussion around
endianness that on some ARM cores at least, when changing the OS
endianness, you had to configure a different lane swapping in the bridge
to the the IO devices (AXI ?)

But I might be confusing with something else.

Ben.





Re: [Qemu-devel] [PATCH 1/8] virtio_get_byteswap: function for endian-ambivalent targets using virtio.

2013-08-12 Thread Peter Maydell
On 12 August 2013 10:50, Benjamin Herrenschmidt
 wrote:
> I must be confused ... you mentioned in a previous discussion around
> endianness that on some ARM cores at least, when changing the OS
> endianness, you had to configure a different lane swapping in the bridge
> to the the IO devices (AXI ?)

No, that's just the implementation -- the bit in the control
register is effectively controlling whether there is byte lane
swapping in the part of the CPU which is the data path between
it and its bus to the outside world.

-- PMM



Re: [Qemu-devel] [PATCH for-1.6] virtio: clear signalled_used_valid when switching from dataplane

2013-08-12 Thread Kevin Wolf
Am 12.08.2013 um 11:08 hat Stefan Hajnoczi geschrieben:
> When the dataplane thread stops, its vring.c implementation synchronizes
> vring state back to virtio.c so we can continue emulating the virtio
> device.
> 
> This patch ensures that virtio.c's signalled_used_valid flag is reset so
> that we do not suppress guest notifications due to stale signalled_used
> values.
> 
> Suggested-by: Kevin Wolf 
> Signed-off-by: Stefan Hajnoczi 

Reviewed-by: Kevin Wolf 



Re: [Qemu-devel] [PATCH 1/8] virtio_get_byteswap: function for endian-ambivalent targets using virtio.

2013-08-12 Thread Benjamin Herrenschmidt
On Mon, 2013-08-12 at 10:52 +0100, Peter Maydell wrote:
> On 12 August 2013 10:50, Benjamin Herrenschmidt
>  wrote:
> > I must be confused ... you mentioned in a previous discussion around
> > endianness that on some ARM cores at least, when changing the OS
> > endianness, you had to configure a different lane swapping in the bridge
> > to the the IO devices (AXI ?)
> 
> No, that's just the implementation -- the bit in the control
> register is effectively controlling whether there is byte lane
> swapping in the part of the CPU which is the data path between
> it and its bus to the outside world.

I find it amazing that an OS can touch that without hitting the
hypervisor :-) Anyway, ok, we do need to poll from virtio then, but we
probably need to cache as well, no ?

When do you sample it in qemu ?

Cheers,
Ben.





Re: [Qemu-devel] [PATCHv3] add qemu-img convert -C option (skip target volume creation)

2013-08-12 Thread Kevin Wolf
Am 12.08.2013 um 11:44 hat Alex Bligh geschrieben:
> 
> 
> --On 12 August 2013 10:42:05 +0100 Alex Bligh  wrote:
> 
> >Add a -C option to skip volume creation on qemu-img convert.
> >This is useful for targets such as rbd / ceph, where the
> >target volume may already exist; we cannot always rely on
> >qemu-img convert to create the image, as dependent on the
> >output format, there may be parameters which are not possible
> >to specify through the qemu-img convert command line.
> >
> >Code:
> >
> >Author: Alexandre Derumier 
> >Signed-off-by: Alexandre Derumier 
> >Signed-off-by: Alex Bligh 
> >
> >Documentaton:
> 
> Would you consider taking this for 1.6? It's really hard
> to do rbd work without it, and it's pretty harmless.

Stefan's decision, but considering that the very last release candidate
is planned for today, I doubt that he's willing to take it. At least I
wouldn't. Feature freeze was long ago, and even for real bug fixes I
would be careful at this point and take nothing that isn't critical.

And even if he did take it, I wouldn't be surprised if Anthony simply
refused to pull.

Kevin



Re: [Qemu-devel] Are there plans to achieve ram live Snapshot feature?

2013-08-12 Thread Stefan Hajnoczi
On Fri, Aug 09, 2013 at 10:20:49AM +, Chijianchun wrote:
> Now in KVM, when RAM snapshot, vcpus needs stopped, it is Unfriendly 
> restrictions to users.
> 
> Are there plans to achieve ram live Snapshot feature?
> 
> in my mind, Snapshots can not occupy additional too much memory, So when the 
> memory needs to be changed, the old memory page is needed to flush to the 
> file first.  But flushing to file is too slower than memory,  and when 
> flushing, the vcpu or VM is need to be paused until finished flushing,  so 
> pause...resume...pause...resume., more and more slower.
> 
> Is this idea feasible? Are there any other thoughts?

A few people have looked at live vmsave or guest RAM snapshots.

The idea that was discussed on qemu-devel@nongnu.org uses fork(2) to
capture the state of guest RAM and then send it back to the parent
process.  The guest is only paused for a brief instant during fork(2)
and can continue to run afterwards.

The child process is a simple loop that sends the contents of guest RAM
back to the parent process over a pipe or writes the memory pages to the
save file on disk.  It performs no logic besides writing out guest RAM.

Stefan



Re: [Qemu-devel] [PATCH for-1.6 2/2] loader: put FW CFG ROM files into RAM

2013-08-12 Thread Michael S. Tsirkin
On Mon, Aug 12, 2013 at 10:32:40AM +0100, Peter Maydell wrote:
> On 12 August 2013 09:49, Michael S. Tsirkin  wrote:
> > +static void *rom_set_mr(Rom *rom, Object *owner, const char *name)
> > +{
> > +/*
> > + * Migration code expects that all RAM blocks are full target pages.
> > + * Round MR size up to make this work.
> > + */
> > +unsigned size = ROUND_UP(rom->datasize, qemu_target_page_size);
> > +void *data = g_malloc0(size);
> 
> If we don't really care where the data lives (ie we are just
> allocating a block for it here) it would be better to get the
> memory subsystem to do the allocation, because then the information
> about the constraints on the size of the region would be confined
> to the memory system.

OK but that looks like a bigger patch. I'm looking
for a small fix that we can make for 1.6.
APIs are easy to tweak migration on-wire format is
set in stone on release.

OK

> > +data = data;
> 
> Huh?
> 
> -- PMM

I'll drop this in v2 but let's figure out what's acceptable.

-- 
MST



Re: [Qemu-devel] [PATCHv3] add qemu-img convert -C option (skip target volume creation)

2013-08-12 Thread Alex Bligh

On 12 Aug 2013, at 10:58, Kevin Wolf wrote:

> Stefan's decision, but considering that the very last release candidate
> is planned for today,

Ouch - I didn't realise we were that close. Point taken. I should
have gotten around to tidying up months ago.

FWIW save for the whitespace changes, I've been running this for 
a good while. The patch was I think originally posted by
Alexandre to ceph-users, and I believe there are several others
using it.

-- 
Alex Bligh







Re: [Qemu-devel] Are there plans to achieve ram live Snapshot feature?

2013-08-12 Thread Alex Bligh



--On 12 August 2013 11:59:03 +0200 Stefan Hajnoczi  
wrote:



The idea that was discussed on qemu-devel@nongnu.org uses fork(2) to
capture the state of guest RAM and then send it back to the parent
process.  The guest is only paused for a brief instant during fork(2)
and can continue to run afterwards.


How would you capture the state of emulated hardware which might not
be in the guest RAM?

--
Alex Bligh



Re: [Qemu-devel] [PATCH 1/8] virtio_get_byteswap: function for endian-ambivalent targets using virtio.

2013-08-12 Thread Peter Maydell
On 12 August 2013 10:56, Benjamin Herrenschmidt
 wrote:
> On Mon, 2013-08-12 at 10:52 +0100, Peter Maydell wrote:
>> On 12 August 2013 10:50, Benjamin Herrenschmidt
>>  wrote:
>> > I must be confused ... you mentioned in a previous discussion around
>> > endianness that on some ARM cores at least, when changing the OS
>> > endianness, you had to configure a different lane swapping in the bridge
>> > to the the IO devices (AXI ?)
>>
>> No, that's just the implementation -- the bit in the control
>> register is effectively controlling whether there is byte lane
>> swapping in the part of the CPU which is the data path between
>> it and its bus to the outside world.
>
> I find it amazing that an OS can touch that without hitting the
> hypervisor :-)

It's no different to having a userspace process able to have
a different setting from the OS, really. (There is an equivalent
bit in another register that controls what endianness we
use if we trap to hyp mode.)

> Anyway, ok, we do need to poll from virtio then, but we
> probably need to cache as well, no ?
>
> When do you sample it in qemu ?

It's a bit theoretical at the moment since QEMU's ARM code
kind of assumes little endian. I would expect that at the
point when virtio was in an MMIO callback the CPUState
struct would have been updated via the usual sync process in
kvm_arch_get_registers().

-- PMM



[Qemu-devel] [PULL 1/2] hw/virtio/virtio: Don't allow guests to add/remove queues

2013-08-12 Thread Peter Maydell
A queue size of 0 is used to indicate a nonexistent queue, so
don't allow the guest to flip a queue between zero-size and
non-zero-size. Don't permit setting of negative queue sizes
either.

Signed-off-by: Peter Maydell 
Message-id: 1374853288-9912-2-git-send-email-peter.mayd...@linaro.org
Reviewed-by: Michael S. Tsirkin 
---
 hw/virtio/virtio.c |   12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 09f62c6..60653f7 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -673,10 +673,16 @@ hwaddr virtio_queue_get_addr(VirtIODevice *vdev, int n)
 
 void virtio_queue_set_num(VirtIODevice *vdev, int n, int num)
 {
-if (num <= VIRTQUEUE_MAX_SIZE) {
-vdev->vq[n].vring.num = num;
-virtqueue_init(&vdev->vq[n]);
+/* Don't allow guest to flip queue between existent and
+ * nonexistent states, or to set it to an invalid size.
+ */
+if (!!num != !!vdev->vq[n].vring.num ||
+num > VIRTQUEUE_MAX_SIZE ||
+num < 0) {
+return;
 }
+vdev->vq[n].vring.num = num;
+virtqueue_init(&vdev->vq[n]);
 }
 
 int virtio_queue_get_num(VirtIODevice *vdev, int n)
-- 
1.7.9.5




[Qemu-devel] [PULL for-1.6 0/2] arm-devs queue

2013-08-12 Thread Peter Maydell
This pullrequest is just two bugfixes for virtio-mmio, which
have now been code-reviewed; MST recommended they go into 1.6,
so here they are.

thanks
-- PMM

The following changes since commit 2e985fe000e73097e325e18b943e8babfa96c35c:

  mips: revert commit b332d24a8e1290954029814d09156b06ede358e2 (2013-08-08 
23:06:15 +0200)

are available in the git repository at:

  git://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-arm-devs-20130812

for you to fetch changes up to f7b803b377f74f7e109559e8e64f04c4c1fcd86b:

  hw/virtio/virtio-mmio: Make QueueNumMax read 0 for unavailable queues 
(2013-08-12 11:57:56 +0100)


arm-devs queue


Peter Maydell (2):
  hw/virtio/virtio: Don't allow guests to add/remove queues
  hw/virtio/virtio-mmio: Make QueueNumMax read 0 for unavailable queues

 hw/virtio/virtio-mmio.c |3 +++
 hw/virtio/virtio.c  |   12 +---
 2 files changed, 12 insertions(+), 3 deletions(-)



[Qemu-devel] [PULL 2/2] hw/virtio/virtio-mmio: Make QueueNumMax read 0 for unavailable queues

2013-08-12 Thread Peter Maydell
The virtio-mmio spec says that QueueNumMax must read zero for queues
which are unavailable; implement this, rather than always returning
VIRTQUEUE_MAX_SIZE.

Signed-off-by: Peter Maydell 
Message-id: 1374853288-9912-3-git-send-email-peter.mayd...@linaro.org
Reviewed-by: Michael S. Tsirkin 
---
 hw/virtio/virtio-mmio.c |3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c
index 88cf994..4bd2953 100644
--- a/hw/virtio/virtio-mmio.c
+++ b/hw/virtio/virtio-mmio.c
@@ -151,6 +151,9 @@ static uint64_t virtio_mmio_read(void *opaque, hwaddr 
offset, unsigned size)
 }
 return proxy->host_features;
 case VIRTIO_MMIO_QUEUENUMMAX:
+if (!virtio_queue_get_num(vdev, vdev->queue_sel)) {
+return 0;
+}
 return VIRTQUEUE_MAX_SIZE;
 case VIRTIO_MMIO_QUEUEPFN:
 return virtio_queue_get_addr(vdev, vdev->queue_sel)
-- 
1.7.9.5




Re: [Qemu-devel] [PATCHv3] add qemu-img convert -C option (skip target volume creation)

2013-08-12 Thread Andreas Färber
Am 12.08.2013 11:42, schrieb Alex Bligh:
> Add a -C option to skip volume creation on qemu-img convert.
> This is useful for targets such as rbd / ceph, where the
> target volume may already exist; we cannot always rely on
> qemu-img convert to create the image, as dependent on the
> output format, there may be parameters which are not possible
> to specify through the qemu-img convert command line.
> 
> Code:
> 
> Author: Alexandre Derumier 
> Signed-off-by: Alexandre Derumier 
> Signed-off-by: Alex Bligh 
> 
> Documentaton:
> 
> Author: Alex Bligh 
> Signed-off-by: Alex Bligh 
> ---

This is a rather odd notation...

The way git-commit works, any committer's Signed-off-by will be placed
directly under the documentation chunk, but it's supposed to cover the
whole commit/patch, not just some aspect of it.

Since as you write, the code is his, the patch should have his From, his
Signed-off-by and finally your Signed-off-by. You can detail your
changes in [AB: ...] or similar between the Signed-off-bys. (To change
the authorship, use git commit --amend --author="Name ".)

Regards,
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] [PATCHv3] add qemu-img convert -C option (skip target volume creation)

2013-08-12 Thread Alex Bligh



--On 12 August 2013 13:11:54 +0200 Andreas Färber  wrote:


The way git-commit works, any committer's Signed-off-by will be placed
directly under the documentation chunk, but it's supposed to cover the
whole commit/patch, not just some aspect of it.

Since as you write, the code is his, the patch should have his From, his
Signed-off-by and finally your Signed-off-by. You can detail your
changes in [AB: ...] or similar between the Signed-off-bys. (To change
the authorship, use git commit --amend --author="Name ".)


By that logic I should split the code and the documentation. I'll do that.

--
Alex Bligh



Re: [Qemu-devel] [PATCH 1/2] vmdk: support vmfsSparse files

2013-08-12 Thread Stefan Hajnoczi
On Sun, Aug 11, 2013 at 06:13:57PM +0200, Paolo Bonzini wrote:
> @@ -505,6 +505,34 @@ static int vmdk_open_vmdk3(BlockDriverState *bs,
>  return ret;
>  }
>  
> +static int vmdk_open_vmfs_sparse(BlockDriverState *bs,
> + BlockDriverState *file,
> + int flags)
> +{
> +int ret;
> +uint32_t magic;
> +VMDK3Header header;
> +VmdkExtent *extent;
> +
> +ret = bdrv_pread(file, sizeof(magic), &header, sizeof(header));
> +if (ret < 0) {
> +return ret;
> +}

magic is unused.  Did you forget to check it?



Re: [Qemu-devel] Are there plans to achieve ram live Snapshot feature?

2013-08-12 Thread Stefan Hajnoczi
On Mon, Aug 12, 2013 at 12:26 PM, Alex Bligh  wrote:
> --On 12 August 2013 11:59:03 +0200 Stefan Hajnoczi 
> wrote:
>
>> The idea that was discussed on qemu-devel@nongnu.org uses fork(2) to
>> capture the state of guest RAM and then send it back to the parent
>> process.  The guest is only paused for a brief instant during fork(2)
>> and can continue to run afterwards.
>
>
> How would you capture the state of emulated hardware which might not
> be in the guest RAM?

Exactly the same way vmsave works today.  It calls the device's save
functions which serialize state to file.

The difference between today's vmsave and the fork(2) approach is that
QEMU does not need to wait for guest RAM to be written to file before
resuming the guest.

Stefan



[Qemu-devel] [PATCHv4] add qemu-img convert -C option (skip target volume creation)

2013-08-12 Thread Alex Bligh
From: Alexandre Derumier 

Add a -C option to skip volume creation on qemu-img convert.
This is useful for targets such as rbd / ceph, where the
target volume may already exist; we cannot always rely on
qemu-img convert to create the image, as dependent on the
output format, there may be parameters which are not possible
to specify through the qemu-img convert command line.

Signed-off-by: Alexandre Derumier 
Signed-off-by: Alex Bligh 
---
 qemu-img-cmds.hx |4 ++--
 qemu-img.c   |   39 ---
 qemu-img.texi|   15 ++-
 3 files changed, 40 insertions(+), 18 deletions(-)

diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
index 4ca7e95..74ced81 100644
--- a/qemu-img-cmds.hx
+++ b/qemu-img-cmds.hx
@@ -34,9 +34,9 @@ STEXI
 ETEXI
 
 DEF("convert", img_convert,
-"convert [-c] [-p] [-q] [-f fmt] [-t cache] [-O output_fmt] [-o options] 
[-s snapshot_name] [-S sparse_size] filename [filename2 [...]] output_filename")
+"convert [-c] [-p] [-q] [-C] [-f fmt] [-t cache] [-O output_fmt] [-o 
options] [-s snapshot_name] [-S sparse_size] filename [filename2 [...]] 
output_filename")
 STEXI
-@item convert [-c] [-p] [-q] [-f @var{fmt}] [-t @var{cache}] [-O 
@var{output_fmt}] [-o @var{options}] [-s @var{snapshot_name}] [-S 
@var{sparse_size}] @var{filename} [@var{filename2} [...]] @var{output_filename}
+@item convert [-c] [-p] [-q] [-C] [-f @var{fmt}] [-t @var{cache}] [-O 
@var{output_fmt}] [-o @var{options}] [-s @var{snapshot_name}] [-S 
@var{sparse_size}] @var{filename} [@var{filename2} [...]] @var{output_filename}
 ETEXI
 
 DEF("info", img_info,
diff --git a/qemu-img.c b/qemu-img.c
index b9a848d..146909b 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -103,6 +103,8 @@ static void help(void)
"  '-S' indicates the consecutive number of bytes that must contain 
only zeros\n"
"   for qemu-img to create a sparse image during conversion\n"
"  '--output' takes the format in which the output must be done 
(human or json)\n"
+   "  '-C' skips the target volume creation (useful if the volume is 
created\n"
+   "   prior to running qemu-img)\n"
"\n"
"Parameters to check subcommand:\n"
"  '-r' tries to repair any inconsistencies that are found during 
the check.\n"
@@ -1116,7 +1118,8 @@ out3:
 
 static int img_convert(int argc, char **argv)
 {
-int c, ret = 0, n, n1, bs_n, bs_i, compress, cluster_size, cluster_sectors;
+int c, ret = 0, n, n1, bs_n, bs_i, compress, cluster_size,
+cluster_sectors, skipcreate;
 int progress = 0, flags;
 const char *fmt, *out_fmt, *cache, *out_baseimg, *out_filename;
 BlockDriver *drv, *proto_drv;
@@ -1139,8 +1142,9 @@ static int img_convert(int argc, char **argv)
 cache = "unsafe";
 out_baseimg = NULL;
 compress = 0;
+skipcreate = 0;
 for(;;) {
-c = getopt(argc, argv, "f:O:B:s:hce6o:pS:t:q");
+c = getopt(argc, argv, "f:O:B:s:hce6o:pS:t:qC");
 if (c == -1) {
 break;
 }
@@ -1161,6 +1165,9 @@ static int img_convert(int argc, char **argv)
 case 'c':
 compress = 1;
 break;
+case 'C':
+skipcreate = 1;
+break;
 case 'e':
 error_report("option -e is deprecated, please use \'-o "
   "encryption\' instead!");
@@ -1329,20 +1336,22 @@ static int img_convert(int argc, char **argv)
 }
 }
 
-/* Create the new image */
-ret = bdrv_create(drv, out_filename, param);
-if (ret < 0) {
-if (ret == -ENOTSUP) {
-error_report("Formatting not supported for file format '%s'",
- out_fmt);
-} else if (ret == -EFBIG) {
-error_report("The image size is too large for file format '%s'",
- out_fmt);
-} else {
-error_report("%s: error while converting %s: %s",
- out_filename, out_fmt, strerror(-ret));
+if (!skipcreate) {
+/* Create the new image */
+ret = bdrv_create(drv, out_filename, param);
+if (ret < 0) {
+if (ret == -ENOTSUP) {
+error_report("Formatting not supported for file format '%s'",
+ out_fmt);
+} else if (ret == -EFBIG) {
+error_report("The image size is too large for file format 
'%s'",
+ out_fmt);
+} else {
+error_report("%s: error while converting %s: %s",
+ out_filename, out_fmt, strerror(-ret));
+}
+goto out;
 }
-goto out;
 }
 
 flags = BDRV_O_RDWR;
diff --git a/qemu-img.texi b/qemu-img.texi
index 69f1bda..9e5ba36 100644
--- a/qemu-img.texi
+++ b/qemu-img.texi
@@ -96,6 +96,14 @@ Second image format
 Strict mode - fail on on different image size or sector allocation
 @end table
 
+Paramete

Re: [Qemu-devel] [PATCHv3] add qemu-img convert -C option (skip target volume creation)

2013-08-12 Thread Alex Bligh

On 12 Aug 2013, at 12:11, Andreas Färber wrote:

> Since as you write, the code is his, the patch should have his From, his
> Signed-off-by and finally your Signed-off-by. You can detail your
> changes in [AB: ...] or similar between the Signed-off-bys. (To change
> the authorship, use git commit --amend --author="Name ".)

Per IRC conversation with you and kwolf, done the way you suggested
in patchv4 just posted.

-- 
Alex Bligh







Re: [Qemu-devel] [PATCH 1/2] vmdk: support vmfsSparse files

2013-08-12 Thread Stefan Hajnoczi
On Mon, Aug 12, 2013 at 1:28 PM, Stefan Hajnoczi  wrote:
> On Sun, Aug 11, 2013 at 06:13:57PM +0200, Paolo Bonzini wrote:
>> @@ -505,6 +505,34 @@ static int vmdk_open_vmdk3(BlockDriverState *bs,
>>  return ret;
>>  }
>>
>> +static int vmdk_open_vmfs_sparse(BlockDriverState *bs,
>> + BlockDriverState *file,
>> + int flags)
>> +{
>> +int ret;
>> +uint32_t magic;
>> +VMDK3Header header;
>> +VmdkExtent *extent;
>> +
>> +ret = bdrv_pread(file, sizeof(magic), &header, sizeof(header));
>> +if (ret < 0) {
>> +return ret;
>> +}
>
> magic is unused.  Did you forget to check it?

Andreas Färber pointed out that we're only using the sizeof(magic),
not the actual variable.

Feel free to ignore my comment, at least the code documents that the
uint32_t offset belongs to the magic number before the header.

Stefan



[Qemu-devel] virtio-net with vhost RX stall

2013-08-12 Thread Benoît Canet

Hello,

I am chasing a randomly occuring RX stall with virtio-net and vhost-net.

So far we have a test setup with a kgdb to debug the guest and a regular gdb
to debug QEMU.
We are still trying to reproduce the bug in a test environment.

Once the bug reproduced would you have hints on what to look for in virtio-net
driver state ?
Any idea to debug this would be wellcome.

Best regards

Benoît



[Qemu-devel] [PULL for-1.6 3/4] virtio: clear signalled_used_valid when switching from dataplane

2013-08-12 Thread Michael S. Tsirkin
From: Stefan Hajnoczi 

When the dataplane thread stops, its vring.c implementation synchronizes
vring state back to virtio.c so we can continue emulating the virtio
device.

This patch ensures that virtio.c's signalled_used_valid flag is reset so
that we do not suppress guest notifications due to stale signalled_used
values.

Suggested-by: Kevin Wolf 
Signed-off-by: Stefan Hajnoczi 
Signed-off-by: Michael S. Tsirkin 
---
 hw/virtio/dataplane/vring.c | 1 +
 hw/virtio/virtio.c  | 5 +
 include/hw/virtio/virtio.h  | 1 +
 3 files changed, 7 insertions(+)

diff --git a/hw/virtio/dataplane/vring.c b/hw/virtio/dataplane/vring.c
index 82cc151..351a343 100644
--- a/hw/virtio/dataplane/vring.c
+++ b/hw/virtio/dataplane/vring.c
@@ -52,6 +52,7 @@ bool vring_setup(Vring *vring, VirtIODevice *vdev, int n)
 void vring_teardown(Vring *vring, VirtIODevice *vdev, int n)
 {
 virtio_queue_set_last_avail_idx(vdev, n, vring->last_avail_idx);
+virtio_queue_invalidate_signalled_used(vdev, n);
 
 hostmem_finalize(&vring->hostmem);
 }
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 09f62c6..706bdf4 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -1059,6 +1059,11 @@ void virtio_queue_set_last_avail_idx(VirtIODevice *vdev, 
int n, uint16_t idx)
 vdev->vq[n].last_avail_idx = idx;
 }
 
+void virtio_queue_invalidate_signalled_used(VirtIODevice *vdev, int n)
+{
+vdev->vq[n].signalled_used_valid = false;
+}
+
 VirtQueue *virtio_get_queue(VirtIODevice *vdev, int n)
 {
 return vdev->vq + n;
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index d7e9e0f..a90522d 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -237,6 +237,7 @@ hwaddr virtio_queue_get_used_size(VirtIODevice *vdev, int 
n);
 hwaddr virtio_queue_get_ring_size(VirtIODevice *vdev, int n);
 uint16_t virtio_queue_get_last_avail_idx(VirtIODevice *vdev, int n);
 void virtio_queue_set_last_avail_idx(VirtIODevice *vdev, int n, uint16_t idx);
+void virtio_queue_invalidate_signalled_used(VirtIODevice *vdev, int n);
 VirtQueue *virtio_get_queue(VirtIODevice *vdev, int n);
 uint16_t virtio_get_queue_index(VirtQueue *vq);
 int virtio_queue_get_id(VirtQueue *vq);
-- 
MST




[Qemu-devel] [PULL for-1.6 0/4] pci,virtio fixes for 1.6

2013-08-12 Thread Michael S. Tsirkin
From: Michael S. Tsirkin 

The following changes since commit 6fdf98f281f85ae6e2883bed2f691bcfe33b1f9f:

  fw_cfg: the I/O port variant expects little-endian (2013-08-07 12:48:15 -0500)

are available in the git repository at:

  git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_anthony

for you to fetch changes up to 3561ba14188b3c1e54246ed6db97896bbc082d2f:

  vhost: clear signalled_used_valid on vhost stop (2013-08-12 12:25:17 +0300)


pci,virtio fixes for 1.6

This includes some last-minute bugfixes for 1.6.
All very small patches that also look very safe to me.

Signed-off-by: Michael S. Tsirkin 


Gerd Hoffmann (1):
  i82801b11: Fix i82801b11 PCI host bridge config space

Michael S. Tsirkin (2):
  pc: disable pci-info for 1.6
  vhost: clear signalled_used_valid on vhost stop

Stefan Hajnoczi (1):
  virtio: clear signalled_used_valid when switching from dataplane

 hw/i386/pc_piix.c   | 9 +++--
 hw/i386/pc_q35.c| 9 +++--
 hw/pci-bridge/i82801b11.c   | 1 +
 hw/virtio/dataplane/vring.c | 1 +
 hw/virtio/vhost.c   | 1 +
 hw/virtio/virtio.c  | 5 +
 include/hw/virtio/virtio.h  | 1 +
 7 files changed, 23 insertions(+), 4 deletions(-)




[Qemu-devel] [PULL for-1.6 4/4] vhost: clear signalled_used_valid on vhost stop

2013-08-12 Thread Michael S. Tsirkin
When vhost device stops, its implementation synchronizes kernel state
back to virtio.c so we can continue emulating the device
in userspace.

This patch ensures that virtio.c's signalled_used_valid flag is reset so
that userspace does not suppress guest notifications due to stale
signalled_used values.

Cc: qemu-sta...@nongnu.org
Signed-off-by: Michael S. Tsirkin 
---
 hw/virtio/vhost.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 8f6ab13..9e336ad 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -762,6 +762,7 @@ static void vhost_virtqueue_stop(struct vhost_dev *dev,
 fflush(stderr);
 }
 virtio_queue_set_last_avail_idx(vdev, idx, state.num);
+virtio_queue_invalidate_signalled_used(vdev, idx);
 assert (r >= 0);
 cpu_physical_memory_unmap(vq->ring, virtio_queue_get_ring_size(vdev, idx),
   0, virtio_queue_get_ring_size(vdev, idx));
-- 
MST




Re: [Qemu-devel] virtio-net with vhost RX stall

2013-08-12 Thread Michael S. Tsirkin
On Mon, Aug 12, 2013 at 01:57:40PM +0200, Benoît Canet wrote:
> 
> Hello,
> 
> I am chasing a randomly occuring RX stall with virtio-net and vhost-net.

Any chance the bug is fixed by 'virtio_net: fix race in RX VQ processing'?

> 
> So far we have a test setup with a kgdb to debug the guest and a regular gdb
> to debug QEMU.
> We are still trying to reproduce the bug in a test environment.
> 
> Once the bug reproduced would you have hints on what to look for in virtio-net
> driver state ?
> Any idea to debug this would be wellcome.
> 
> Best regards
> 
> Benoît

Dump the ring state and index values where they poll, on host and guest.

-- 
MST



[Qemu-devel] [PULL for-1.6 1/4] pc: disable pci-info for 1.6

2013-08-12 Thread Michael S. Tsirkin
The BIOS that we ship in 1.6 does not use pci info
from host (yet). Several issues turned up
(e.g. around winXP boot crashes). So it's safest to disable that
interface for 1.6 machine types for now, leave it on for 1.7
as we have enough time to fix issues if any.

Reviewed-by: Richard Henderson 
Reviewed-by: Andreas Färber 
Signed-off-by: Michael S. Tsirkin 
---
 hw/i386/pc_piix.c | 9 +++--
 hw/i386/pc_q35.c  | 9 +++--
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index ab25458..95c45b8 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -249,12 +249,17 @@ static void pc_init_pci(QEMUMachineInitArgs *args)
  initrd_filename, cpu_model, 1, 1);
 }
 
-static void pc_init_pci_1_5(QEMUMachineInitArgs *args)
+static void pc_init_pci_1_6(QEMUMachineInitArgs *args)
 {
 has_pci_info = false;
 pc_init_pci(args);
 }
 
+static void pc_init_pci_1_5(QEMUMachineInitArgs *args)
+{
+pc_init_pci_1_6(args);
+}
+
 static void pc_init_pci_1_4(QEMUMachineInitArgs *args)
 {
 has_pvpanic = false;
@@ -340,7 +345,7 @@ static QEMUMachine pc_i440fx_machine_v1_6 = {
 .name = "pc-i440fx-1.6",
 .alias = "pc",
 .desc = "Standard PC (i440FX + PIIX, 1996)",
-.init = pc_init_pci,
+.init = pc_init_pci_1_6,
 .hot_add_cpu = pc_hot_add_cpu,
 .max_cpus = 255,
 .is_default = 1,
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 2f35d12..6bfc2ca 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -217,12 +217,17 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
 }
 }
 
-static void pc_q35_init_1_5(QEMUMachineInitArgs *args)
+static void pc_q35_init_1_6(QEMUMachineInitArgs *args)
 {
 has_pci_info = false;
 pc_q35_init(args);
 }
 
+static void pc_q35_init_1_5(QEMUMachineInitArgs *args)
+{
+pc_q35_init_1_6(args);
+}
+
 static void pc_q35_init_1_4(QEMUMachineInitArgs *args)
 {
 has_pvpanic = false;
@@ -234,7 +239,7 @@ static QEMUMachine pc_q35_machine_v1_6 = {
 .name = "pc-q35-1.6",
 .alias = "q35",
 .desc = "Standard PC (Q35 + ICH9, 2009)",
-.init = pc_q35_init,
+.init = pc_q35_init_1_6,
 .hot_add_cpu = pc_hot_add_cpu,
 .max_cpus = 255,
 DEFAULT_MACHINE_OPTIONS,
-- 
MST




[Qemu-devel] [PULL for-1.6 2/4] i82801b11: Fix i82801b11 PCI host bridge config space

2013-08-12 Thread Michael S. Tsirkin
From: Gerd Hoffmann 

pci_bridge_write_config() was not being used.

Cc: qemu-sta...@nongnu.org
Signed-off-by: Gerd Hoffmann 
Signed-off-by: Andreas Färber 
Signed-off-by: Michael S. Tsirkin 
---
 hw/pci-bridge/i82801b11.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/pci-bridge/i82801b11.c b/hw/pci-bridge/i82801b11.c
index 8a5e426..14cd7fd 100644
--- a/hw/pci-bridge/i82801b11.c
+++ b/hw/pci-bridge/i82801b11.c
@@ -90,6 +90,7 @@ static void i82801b11_bridge_class_init(ObjectClass *klass, 
void *data)
 k->device_id = PCI_DEVICE_ID_INTEL_82801BA_11;
 k->revision = ICH9_D2P_A2_REVISION;
 k->init = i82801b11_bridge_initfn;
+k->config_write = pci_bridge_write_config;
 set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
 }
 
-- 
MST




[Qemu-devel] [PATCH 0/2] qemu-timer: make qemu_timer_mod_ns() and qemu_timer_del() thread-safe

2013-08-12 Thread Stefan Hajnoczi
These patches are based on Alex Bligh's v10 AioContext timers series.

The purpose of these patches is to eventually allow device models to set and
cancel timers without holding the global mutex.  When the device model runs in
a vcpu thread and the iothread processes timers, the
QEMUTimerList->active_timers must be protected from concurrent access.

Patch 1 is a clean-up.

Patch 2 is the entire change needed to protect ->active_timers.

Stefan Hajnoczi (2):
  qemu-timer: drop outdated signal safety comments
  qemu-timer: make qemu_timer_mod_ns() and qemu_timer_del() thread-safe

 include/qemu/timer.h | 17 +
 qemu-timer.c | 70 ++--
 2 files changed, 69 insertions(+), 18 deletions(-)

-- 
1.8.3.1




[Qemu-devel] [PATCH 2/2] qemu-timer: make qemu_timer_mod_ns() and qemu_timer_del() thread-safe

2013-08-12 Thread Stefan Hajnoczi
Introduce QEMUTimerList->active_timers_lock to protect the linked list
of active timers.  This allows qemu_timer_mod_ns() to be called from any
thread.

Note that vm_clock is not thread-safe and its use of
qemu_clock_has_timers() works fine today but is also not thread-safe.

The purpose of this patch is to eventually let device models set or
cancel timers from a vcpu thread without holding the global mutex.

Signed-off-by: Stefan Hajnoczi 
---
 include/qemu/timer.h | 17 ++
 qemu-timer.c | 66 +---
 2 files changed, 69 insertions(+), 14 deletions(-)

diff --git a/include/qemu/timer.h b/include/qemu/timer.h
index 829c005..3543b0e 100644
--- a/include/qemu/timer.h
+++ b/include/qemu/timer.h
@@ -114,6 +114,10 @@ static inline int64_t qemu_clock_get_us(QEMUClockType type)
  * Determines whether a clock's default timer list
  * has timers attached
  *
+ * Note that this function should not be used when other threads also access
+ * the timer list.  The return value may be outdated by the time it is acted
+ * upon.
+ *
  * Returns: true if the clock's default timer list
  * has timers attached
  */
@@ -270,6 +274,10 @@ void timerlist_free(QEMUTimerList *timer_list);
  *
  * Determine whether a timer list has active timers
  *
+ * Note that this function should not be used when other threads also access
+ * the timer list.  The return value may be outdated by the time it is acted
+ * upon.
+ *
  * Returns: true if the timer list has timers.
  */
 bool timerlist_has_timers(QEMUTimerList *timer_list);
@@ -511,6 +519,9 @@ void timer_free(QEMUTimer *ts);
  * @ts: the timer
  *
  * Delete a timer from the active list.
+ *
+ * This function is thread-safe but the timer and its timer list must not be
+ * freed while this function is running.
  */
 void timer_del(QEMUTimer *ts);
 
@@ -520,6 +531,9 @@ void timer_del(QEMUTimer *ts);
  * @expire_time: the expiry time in nanoseconds
  *
  * Modify a timer to expire at @expire_time
+ *
+ * This function is thread-safe but the timer and its timer list must not be
+ * freed while this function is running.
  */
 void timer_mod_ns(QEMUTimer *ts, int64_t expire_time);
 
@@ -530,6 +544,9 @@ void timer_mod_ns(QEMUTimer *ts, int64_t expire_time);
  *
  * Modify a timer to expiry at @expire_time, taking into
  * account the scale associated with the timer.
+ *
+ * This function is thread-safe but the timer and its timer list must not be
+ * freed while this function is running.
  */
 void timer_mod(QEMUTimer *ts, int64_t expire_timer);
 
diff --git a/qemu-timer.c b/qemu-timer.c
index 818d235..3a5b46d 100644
--- a/qemu-timer.c
+++ b/qemu-timer.c
@@ -66,6 +66,7 @@ QEMUClock qemu_clocks[QEMU_CLOCK_MAX];
 
 struct QEMUTimerList {
 QEMUClock *clock;
+QemuMutex active_timers_lock;
 QEMUTimer *active_timers;
 QLIST_ENTRY(QEMUTimerList) list;
 QEMUTimerListNotifyCB *notify_cb;
@@ -101,6 +102,7 @@ QEMUTimerList *timerlist_new(QEMUClockType type,
 timer_list->clock = clock;
 timer_list->notify_cb = cb;
 timer_list->notify_opaque = opaque;
+qemu_mutex_init(&timer_list->active_timers_lock);
 QLIST_INSERT_HEAD(&clock->timerlists, timer_list, list);
 return timer_list;
 }
@@ -111,6 +113,7 @@ void timerlist_free(QEMUTimerList *timer_list)
 if (timer_list->clock) {
 QLIST_REMOVE(timer_list, list);
 }
+qemu_mutex_destroy(&timer_list->active_timers_lock);
 g_free(timer_list);
 }
 
@@ -163,9 +166,17 @@ bool qemu_clock_has_timers(QEMUClockType type)
 
 bool timerlist_expired(QEMUTimerList *timer_list)
 {
-return (timer_list->active_timers &&
-timer_list->active_timers->expire_time <
-qemu_clock_get_ns(timer_list->clock->type));
+int64_t expire_time;
+
+qemu_mutex_lock(&timer_list->active_timers_lock);
+if (!timer_list->active_timers) {
+qemu_mutex_unlock(&timer_list->active_timers_lock);
+return false;
+}
+expire_time = timer_list->active_timers->expire_time;
+qemu_mutex_unlock(&timer_list->active_timers_lock);
+
+return expire_time < qemu_clock_get_ns(timer_list->clock->type);
 }
 
 bool qemu_clock_expired(QEMUClockType type)
@@ -182,13 +193,25 @@ bool qemu_clock_expired(QEMUClockType type)
 int64_t timerlist_deadline_ns(QEMUTimerList *timer_list)
 {
 int64_t delta;
+int64_t expire_time;
 
-if (!timer_list->clock->enabled || !timer_list->active_timers) {
+if (!timer_list->clock->enabled) {
 return -1;
 }
 
-delta = timer_list->active_timers->expire_time -
-qemu_clock_get_ns(timer_list->clock->type);
+/* The active timers list may be modified before the caller uses our return
+ * value but ->notify_cb() is called when the deadline changes.  Therefore
+ * the caller should notice the change and there is no race condition.
+ */
+qemu_mutex_lock(&timer_list->active_timers_lock);
+if (!timer_list->active_timers) {
+qemu_mutex_unlock(&

[Qemu-devel] [PATCH 1/2] qemu-timer: drop outdated signal safety comments

2013-08-12 Thread Stefan Hajnoczi
host_alarm_handler() is invoked from the signal processing thread
(currently the iothread).  Previously we did processing in a real signal
handler with signalfd and therefore needed signal-safe timer code.

Today host_alarm_handler() just marks the alarm timer as expired/pending
and notifies the main loop using qemu_notify_event().

Therefore these outdated comments about signal safety can be dropped.

Signed-off-by: Stefan Hajnoczi 
---
 qemu-timer.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/qemu-timer.c b/qemu-timer.c
index 5b9a722..818d235 100644
--- a/qemu-timer.c
+++ b/qemu-timer.c
@@ -301,8 +301,6 @@ void timer_del(QEMUTimer *ts)
 {
 QEMUTimer **pt, *t;
 
-/* NOTE: this code must be signal safe because
-   timer_expired() can be called from a signal. */
 pt = &ts->timer_list->active_timers;
 for(;;) {
 t = *pt;
@@ -325,8 +323,6 @@ void timer_mod_ns(QEMUTimer *ts, int64_t expire_time)
 timer_del(ts);
 
 /* add the timer in the sorted list */
-/* NOTE: this code must be signal safe because
-   timer_expired() can be called from a signal. */
 pt = &ts->timer_list->active_timers;
 for(;;) {
 t = *pt;
-- 
1.8.3.1




Re: [Qemu-devel] [PATCH 1/8] virtio_get_byteswap: function for endian-ambivalent targets using virtio.

2013-08-12 Thread Anthony Liguori
Benjamin Herrenschmidt  writes:

> On Mon, 2013-08-12 at 17:29 +0930, Rusty Russell wrote:
>> virtio data structures are defined as "target endian", which assumes
>> that's a fixed value.  In fact, that actually means it's
>> platform-specific.
>> 
>> Hopefully the OASIS virtio 1.0 spec will fix this.  Meanwhile, create
>> a hook for little endian ppc.
>
> Ok, sorry if I missed a previous debate on that one but why do you do a
> call-out to architecture specific stuff (that is not even inline) on
> every access ?

Let's focus on getting something merged.  Then we can muck around with
it down the road.

Having target-ppc call into virtio is a layering violation.  This
approach keeps the dependencies cleaner.

Regards,

Anthony Liguori

>
> If we consider that virtio byte order is global, can't you make it a
> global that is *set* by the architecture rather than *polled* by
> virtio ?
>
> We have explicit knowledge of when our endianness change (we get the
> hcall or a write to some SPR), we can call virtio *then* to adjust the
> endianness rather than having a call-out to the platform on every
> access.
>
> Ben.



Re: [Qemu-devel] [SeaBIOS] [PATCH] acpi: hide 64-bit PCI hole for Windows XP

2013-08-12 Thread Laszlo Ersek
On 08/09/13 17:49, Michael S. Tsirkin wrote:
> On Fri, Aug 09, 2013 at 12:13:06AM -0400, Kevin O'Connor wrote:
>> On Thu, Aug 08, 2013 at 04:56:55PM +0200, Gerd Hoffmann wrote:
>>> On 08/08/13 16:13, Michael S. Tsirkin wrote:
 On Thu, Aug 08, 2013 at 12:21:32PM +0200, Gerd Hoffmann wrote:
> On 08/08/13 11:52, Michael S. Tsirkin wrote:
>> On Thu, Aug 08, 2013 at 10:57:44AM +0200, Gerd Hoffmann wrote:
>>> On 08/08/13 10:37, Michael S. Tsirkin wrote:
 On Thu, Aug 08, 2013 at 09:57:39AM +0200, Gerd Hoffmann wrote:
> Also coreboot and seabios use different values for pmbase.  coreboot 
> on
> q35 maps the pmbase below 0x1000.  Which surely makes sense.  When we
> don't place chipset stuff at 0xb000 we can assign the 0xb000->0xbfff
> window to a pci bridge instead.

 Re-reading this - if this has value, can't we generalize it
 and make all firmware behave the same, getting values from QEMU?
>>>
>>> pmbase is a compile-time constant (aka #define) in both seabios and
>>> coreboot, and making this runtime-configurable is non-trivial.  See
>>> src/smm.c in seabios for one reason why.
>>
>> I don't think SeaBIOS should modify tables provided by QEMU.  That
>> leads to confusion on the source of the data and mixed
>> responsibilities which results in greater complexity in both QEMU and
>> SeaBIOS.
>>
>> SeaBIOS doesn't have any info that QEMU doesn't have.  So, I think
>> it's safe for QEMU to be the sole authority for the table content.
>>
>> Converting src/smm.c to use a runtime value isn't hard - just change
>> the assembler from: "mov $" __stringify(PORT_ACPI_PM_BASE) " + 0x04,
>> %dx\n" to: "mov 4(my_acpi_base), %dx\n" and make sure to define the
>> global variable my_acpi_base as VARFSEG.
>>
> Yes, the address ranges used for pci devices (aka 32bit + 64bit pci
> window) need to be there.  Well, placing in SSDT, then referencing from
> DSDT works too, and this is what seabios does today to dynamically
> adjust stuff.  Fixing up the SSDT using the linker is probably easier as
> we generate it anyway.
>
 Yes but as I said, this makes things messy, since AML encoding for
 numbers isn't fixed width.
>>>
>>> In seabios we have fixed 32bit / 64bit width today, from acpi.c:
>>>
>>> // store pci io windows
>>> *(u32*)&ssdt_ptr[acpi_pci32_start[0]] = cpu_to_le32(pcimem_start);
>>> *(u32*)&ssdt_ptr[acpi_pci32_end[0]] = cpu_to_le32(pcimem_end - 1);
>>> if (pcimem64_start) {
>>> ssdt_ptr[acpi_pci64_valid[0]] = 1;
>>> *(u64*)&ssdt_ptr[acpi_pci64_start[0]] = cpu_to_le64(pcimem64_start);
>>> *(u64*)&ssdt_ptr[acpi_pci64_end[0]] = cpu_to_le64(pcimem64_end - 1);
>>> *(u64*)&ssdt_ptr[acpi_pci64_length[0]] = cpu_to_le64(
>>> pcimem64_end - pcimem64_start);
>>> } else {
>>> ssdt_ptr[acpi_pci64_valid[0]] = 0;
>>> }
>>>
>>> Storing fixup instructions for these fields in the linker script
>>> shouldn't be hard I think.
>>
>> I don't think SeaBIOS should continue to do the above once the tables
>> are moved to QEMU.  QEMU has all the info SeaBIOS has, so it can
>> generate the tables correctly on its own.
>>
>> In all practical situations, the PCI window should be at least an
>> order of magnatude greater than the sum of the PCI bars in the system.
>> If the bars are actually bigger than the window, then things are going
>> to fail - the best the firmware can do is try to fail gracefully.  I
>> don't think it's worth the complexity to design mixed ownership and
>> advanced interfaces just so we can fail slightly better.
>>
>> If this is a real worry, QEMU can sum all the PCI bars and warn the
>> user if they don't fit.
>>
>> -Kevin
> 
> If we make it a rule that PCI is`setup before ACPI tables
> are read, then QEMU can do the patching itself when
> it detects BIOS reading the tables.
> 
> Gerd, Laszlo,others,  does this rule work for alternative firmwares?

OVMF currently determines the boundaries of the PCI window (32-bit) that
it is going to advertise in the _CRS method by scanning the "map of the
memory resources in the global coherency domain of the processor".

It finds the top of the system memory (under 4GB) and the smallest
interval that covers all of the memory mapped IO ranges. The subset of
the latter interval that is above the top of the system memory is then
advertised.

See PopulateFwData() in "OvmfPkg/AcpiPlatformDxe/Qemu.c", and
GetMemorySpaceMap() in "VOLUME 2: Platform Initialization Specification
/ Driver Execution Environment Core Interface".

... With this I'm trying to say that for now OVMF keys the 32-bit window
it advertises off regions that are "currently being decoded by a
component as memory-mapped I/O that can be used to access I/O devices in
the platform".

This seems to imply that PCI setup precedes ACPI table installation.

Laszlo




Re: [Qemu-devel] [PATCH 1/2] qemu-timer: drop outdated signal safety comments

2013-08-12 Thread Alex Bligh



--On 12 August 2013 14:49:28 +0200 Stefan Hajnoczi  
wrote:



host_alarm_handler() is invoked from the signal processing thread
(currently the iothread).  Previously we did processing in a real signal
handler with signalfd and therefore needed signal-safe timer code.

Today host_alarm_handler() just marks the alarm timer as expired/pending
and notifies the main loop using qemu_notify_event().

Therefore these outdated comments about signal safety can be dropped.

Signed-off-by: Stefan Hajnoczi 


Reviewed-by: Alex Bligh 


---
 qemu-timer.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/qemu-timer.c b/qemu-timer.c
index 5b9a722..818d235 100644
--- a/qemu-timer.c
+++ b/qemu-timer.c
@@ -301,8 +301,6 @@ void timer_del(QEMUTimer *ts)
 {
 QEMUTimer **pt, *t;

-/* NOTE: this code must be signal safe because
-   timer_expired() can be called from a signal. */
 pt = &ts->timer_list->active_timers;
 for(;;) {
 t = *pt;
@@ -325,8 +323,6 @@ void timer_mod_ns(QEMUTimer *ts, int64_t expire_time)
 timer_del(ts);

 /* add the timer in the sorted list */
-/* NOTE: this code must be signal safe because
-   timer_expired() can be called from a signal. */
 pt = &ts->timer_list->active_timers;
 for(;;) {
 t = *pt;
--
1.8.3.1






--
Alex Bligh



Re: [Qemu-devel] [PATCH 2/2] qemu-timer: make qemu_timer_mod_ns() and qemu_timer_del() thread-safe

2013-08-12 Thread Alex Bligh



--On 12 August 2013 14:49:29 +0200 Stefan Hajnoczi  
wrote:



Introduce QEMUTimerList->active_timers_lock to protect the linked list
of active timers.  This allows qemu_timer_mod_ns() to be called from any
thread.

Note that vm_clock is not thread-safe and its use of
qemu_clock_has_timers() works fine today but is also not thread-safe.

The purpose of this patch is to eventually let device models set or
cancel timers from a vcpu thread without holding the global mutex.

Signed-off-by: Stefan Hajnoczi 


Reviewed-by: Alex Bligh 


---
 include/qemu/timer.h | 17 ++
 qemu-timer.c | 66
+---  2 files changed, 69
insertions(+), 14 deletions(-)

diff --git a/include/qemu/timer.h b/include/qemu/timer.h
index 829c005..3543b0e 100644
--- a/include/qemu/timer.h
+++ b/include/qemu/timer.h
@@ -114,6 +114,10 @@ static inline int64_t
qemu_clock_get_us(QEMUClockType type)   * Determines whether a clock's
default timer list
  * has timers attached
  *
+ * Note that this function should not be used when other threads also
access + * the timer list.  The return value may be outdated by the time
it is acted + * upon.
+ *
  * Returns: true if the clock's default timer list
  * has timers attached
  */
@@ -270,6 +274,10 @@ void timerlist_free(QEMUTimerList *timer_list);
  *
  * Determine whether a timer list has active timers
  *
+ * Note that this function should not be used when other threads also
access + * the timer list.  The return value may be outdated by the time
it is acted + * upon.
+ *
  * Returns: true if the timer list has timers.
  */
 bool timerlist_has_timers(QEMUTimerList *timer_list);
@@ -511,6 +519,9 @@ void timer_free(QEMUTimer *ts);
  * @ts: the timer
  *
  * Delete a timer from the active list.
+ *
+ * This function is thread-safe but the timer and its timer list must
not be + * freed while this function is running.
  */
 void timer_del(QEMUTimer *ts);

@@ -520,6 +531,9 @@ void timer_del(QEMUTimer *ts);
  * @expire_time: the expiry time in nanoseconds
  *
  * Modify a timer to expire at @expire_time
+ *
+ * This function is thread-safe but the timer and its timer list must
not be + * freed while this function is running.
  */
 void timer_mod_ns(QEMUTimer *ts, int64_t expire_time);

@@ -530,6 +544,9 @@ void timer_mod_ns(QEMUTimer *ts, int64_t expire_time);
  *
  * Modify a timer to expiry at @expire_time, taking into
  * account the scale associated with the timer.
+ *
+ * This function is thread-safe but the timer and its timer list must
not be + * freed while this function is running.
  */
 void timer_mod(QEMUTimer *ts, int64_t expire_timer);

diff --git a/qemu-timer.c b/qemu-timer.c
index 818d235..3a5b46d 100644
--- a/qemu-timer.c
+++ b/qemu-timer.c
@@ -66,6 +66,7 @@ QEMUClock qemu_clocks[QEMU_CLOCK_MAX];

 struct QEMUTimerList {
 QEMUClock *clock;
+QemuMutex active_timers_lock;
 QEMUTimer *active_timers;
 QLIST_ENTRY(QEMUTimerList) list;
 QEMUTimerListNotifyCB *notify_cb;
@@ -101,6 +102,7 @@ QEMUTimerList *timerlist_new(QEMUClockType type,
 timer_list->clock = clock;
 timer_list->notify_cb = cb;
 timer_list->notify_opaque = opaque;
+qemu_mutex_init(&timer_list->active_timers_lock);
 QLIST_INSERT_HEAD(&clock->timerlists, timer_list, list);
 return timer_list;
 }
@@ -111,6 +113,7 @@ void timerlist_free(QEMUTimerList *timer_list)
 if (timer_list->clock) {
 QLIST_REMOVE(timer_list, list);
 }
+qemu_mutex_destroy(&timer_list->active_timers_lock);
 g_free(timer_list);
 }

@@ -163,9 +166,17 @@ bool qemu_clock_has_timers(QEMUClockType type)

 bool timerlist_expired(QEMUTimerList *timer_list)
 {
-return (timer_list->active_timers &&
-timer_list->active_timers->expire_time <
-qemu_clock_get_ns(timer_list->clock->type));
+int64_t expire_time;
+
+qemu_mutex_lock(&timer_list->active_timers_lock);
+if (!timer_list->active_timers) {
+qemu_mutex_unlock(&timer_list->active_timers_lock);
+return false;
+}
+expire_time = timer_list->active_timers->expire_time;
+qemu_mutex_unlock(&timer_list->active_timers_lock);
+
+return expire_time < qemu_clock_get_ns(timer_list->clock->type);
 }

 bool qemu_clock_expired(QEMUClockType type)
@@ -182,13 +193,25 @@ bool qemu_clock_expired(QEMUClockType type)
 int64_t timerlist_deadline_ns(QEMUTimerList *timer_list)
 {
 int64_t delta;
+int64_t expire_time;

-if (!timer_list->clock->enabled || !timer_list->active_timers) {
+if (!timer_list->clock->enabled) {
 return -1;
 }

-delta = timer_list->active_timers->expire_time -
-qemu_clock_get_ns(timer_list->clock->type);
+/* The active timers list may be modified before the caller uses our
return + * value but ->notify_cb() is called when the deadline
changes.  Therefore + * the caller should notice the change and there
is no race condition. + */
+qemu_mutex_lock(&timer_list->ac

Re: [Qemu-devel] [PATCH v2] virtio-serial: Do not notify virtqueue if no element was pushed back.

2013-08-12 Thread Laszlo Ersek
On 08/11/13 15:25, Gal Hammer wrote:
> The redundant notification caused the Windows' driver to duplicate the
> pending write request's buffer. The driver was fixed, but I think this
> change is still required.
> 
> Signed-off-by: Gal Hammer 
> ---
>  hw/char/virtio-serial-bus.c | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
> index da417c7..0d38b4b 100644
> --- a/hw/char/virtio-serial-bus.c
> +++ b/hw/char/virtio-serial-bus.c
> @@ -105,6 +105,7 @@ static void do_flush_queued_data(VirtIOSerialPort *port, 
> VirtQueue *vq,
>   VirtIODevice *vdev)
>  {
>  VirtIOSerialPortClass *vsc;
> +bool elem_pushed = false;
>  
>  assert(port);
>  assert(virtio_queue_ready(vq));
> @@ -145,9 +146,12 @@ static void do_flush_queued_data(VirtIOSerialPort *port, 
> VirtQueue *vq,
>  break;
>  }
>  virtqueue_push(vq, &port->elem, 0);
> +elem_pushed = true;
>  port->elem.out_num = 0;
>  }
> -virtio_notify(vdev, vq);
> +if (elem_pushed) {
> +virtio_notify(vdev, vq);
> +}
>  }
>  
>  static void flush_queued_data(VirtIOSerialPort *port)
> 

Differs from v1 only in a more precise subject / more details in the
commit msg.

Reviewed-by: Laszlo Ersek 



Re: [Qemu-devel] [PULL for-1.6] TCG mips --enable-debug fix

2013-08-12 Thread Anthony Liguori
Richard Henderson  writes:

> Please pull for -rc2, thanks.
>
>
> r~
>
>
> The following changes since commit 6fdf98f281f85ae6e2883bed2f691bcfe33b1f9f:
>
>   fw_cfg: the I/O port variant expects little-endian (2013-08-07 12:48:15 
> -0500)
>
> are available in the git repository at:
>
>   git://github.com/rth7680/qemu.git for-1.6

I don't see a published branch of this name in the tree.

Regards,

Anthony Liguori

>
> for you to fetch changes up to 31e846e8f3885f82df7cc96f0a14a6665b42388e:
>
>   tcg/mips: fix invalid op definition errors (2013-08-08 06:11:19 -1000)
>
> 
> James Hogan (1):
>   tcg/mips: fix invalid op definition errors
>
>  tcg/mips/tcg-target.c | 10 ++
>  1 file changed, 10 insertions(+)




Re: [Qemu-devel] [PATCH for-1.6] virtio: clear signalled_used_valid when switching from dataplane

2013-08-12 Thread Stefan Hajnoczi
On Mon, Aug 12, 2013 at 11:18 AM, Michael S. Tsirkin  wrote:
> On Mon, Aug 12, 2013 at 11:08:09AM +0200, Stefan Hajnoczi wrote:
>> When the dataplane thread stops, its vring.c implementation synchronizes
>> vring state back to virtio.c so we can continue emulating the virtio
>> device.
>>
>> This patch ensures that virtio.c's signalled_used_valid flag is reset so
>> that we do not suppress guest notifications due to stale signalled_used
>> values.
>>
>> Suggested-by: Kevin Wolf 
>> Signed-off-by: Stefan Hajnoczi 
>
> Good point
>
> Reviewed-by: Michael S. Tsirkin 
>
> and we also need this for vhost.c right?

I think vhost is not affected by this because virtio.c clears the
signalled_used_valid flag when the device is reset.  Since vhost_net
does not transition back and forth between userspace virtio and vhost
(without a device reset), there is no need to reset the flag.

Stefan



Re: [Qemu-devel] [PULL for-1.6] TCG mips --enable-debug fix

2013-08-12 Thread Andreas Färber
Am 12.08.2013 15:31, schrieb Anthony Liguori:
> Richard Henderson  writes:
> 
>> Please pull for -rc2, thanks.
>>
>>
>> r~
>>
>>
>> The following changes since commit 6fdf98f281f85ae6e2883bed2f691bcfe33b1f9f:
>>
>>   fw_cfg: the I/O port variant expects little-endian (2013-08-07 12:48:15 
>> -0500)
>>
>> are available in the git repository at:
>>
>>   git://github.com/rth7680/qemu.git for-1.6
> 
> I don't see a published branch of this name in the tree.

Aurélien seems to have already applied the original patch FWIW.

Andreas

> 
> Regards,
> 
> Anthony Liguori
> 
>>
>> for you to fetch changes up to 31e846e8f3885f82df7cc96f0a14a6665b42388e:
>>
>>   tcg/mips: fix invalid op definition errors (2013-08-08 06:11:19 -1000)
>>
>> 
>> James Hogan (1):
>>   tcg/mips: fix invalid op definition errors
>>
>>  tcg/mips/tcg-target.c | 10 ++
>>  1 file changed, 10 insertions(+)
> 
> 


-- 
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 for-1.6 0/2] future proof rom loading for cross versiom migration

2013-08-12 Thread Anthony Liguori
"Michael S. Tsirkin"  writes:

> ROM files that are put in FW CFG are copied to guest ram, by BIOS, but
> they are not backed by RAM so they don't get migrated.
>
> Each time we'll change at least two bytes in such a ROM this will break
> cross-version migration: since we can migrate after BIOS has read the first
> byte but before it has read the second one, getting an inconsistent state.
>
> This patchset makes QEMU future-proof against such changes.
>
> Naturally, this only helps for -M 1.6 and up, older machine types
> will still have the cross-version migration bug.
>
> I think this should be applied for 1.6, this way we won't
> have this problem from 1.7 and on.

This is not for 1.6.  It's far too late to make a change like this.

Regards,

Anthony Liguori

>
> Michael S. Tsirkin (2):
>   memory: export target page size
>   loader: put FW CFG ROM files into RAM
>
>  exec.c|  2 ++
>  hw/core/loader.c  | 54 
> ---
>  hw/i386/pc_piix.c |  2 ++
>  hw/i386/pc_q35.c  |  2 ++
>  include/exec/memory.h |  2 ++
>  include/hw/loader.h   |  1 +
>  6 files changed, 60 insertions(+), 3 deletions(-)
>
> -- 
> MST




[Qemu-devel] [PATCH v2] pc: drop external DSDT loading

2013-08-12 Thread Anthony Liguori
This breaks migration and is unneeded with modern SeaBIOS.

Signed-off-by: Anthony Liguori 
---
v1 -> v2
 - Still load external DSDT for q35
---
 hw/i386/pc_piix.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 95c45b8..311574a 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -103,7 +103,6 @@ static void pc_init1(MemoryRegion *system_memory,
   OBJECT(icc_bridge), NULL);
 
 pc_cpus_init(cpu_model, icc_bridge);
-pc_acpi_init("acpi-dsdt.aml");
 
 if (kvm_enabled() && kvmclock_enabled) {
 kvmclock_create();
-- 
1.8.0




Re: [Qemu-devel] [PATCH v2] pc: drop external DSDT loading

2013-08-12 Thread Andreas Färber
Am 12.08.2013 16:01, schrieb Anthony Liguori:
> This breaks migration and is unneeded with modern SeaBIOS.
> 
> Signed-off-by: Anthony Liguori 
> ---
> v1 -> v2
>  - Still load external DSDT for q35

Care to extend $subject with "for i440fx" then before applying? :)

Regards,
Andreas

> ---
>  hw/i386/pc_piix.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 95c45b8..311574a 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -103,7 +103,6 @@ static void pc_init1(MemoryRegion *system_memory,
>OBJECT(icc_bridge), NULL);
>  
>  pc_cpus_init(cpu_model, icc_bridge);
> -pc_acpi_init("acpi-dsdt.aml");
>  
>  if (kvm_enabled() && kvmclock_enabled) {
>  kvmclock_create();
> 


-- 
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 for-1.6] virtio: clear signalled_used_valid when switching from dataplane

2013-08-12 Thread Michael S. Tsirkin
On Mon, Aug 12, 2013 at 03:39:23PM +0200, Stefan Hajnoczi wrote:
> On Mon, Aug 12, 2013 at 11:18 AM, Michael S. Tsirkin  wrote:
> > On Mon, Aug 12, 2013 at 11:08:09AM +0200, Stefan Hajnoczi wrote:
> >> When the dataplane thread stops, its vring.c implementation synchronizes
> >> vring state back to virtio.c so we can continue emulating the virtio
> >> device.
> >>
> >> This patch ensures that virtio.c's signalled_used_valid flag is reset so
> >> that we do not suppress guest notifications due to stale signalled_used
> >> values.
> >>
> >> Suggested-by: Kevin Wolf 
> >> Signed-off-by: Stefan Hajnoczi 
> >
> > Good point
> >
> > Reviewed-by: Michael S. Tsirkin 
> >
> > and we also need this for vhost.c right?
> 
> I think vhost is not affected by this because virtio.c clears the
> signalled_used_valid flag when the device is reset.  Since vhost_net
> does not transition back and forth between userspace virtio and vhost
> (without a device reset), there is no need to reset the flag.
> 
> Stefan

Yes it does transition on vm stop/start.



Re: [Qemu-devel] [PATCH v2] Fix query-migrate documentation in qmp-commands.hx

2013-08-12 Thread Luiz Capitulino
On Thu,  8 Aug 2013 20:05:48 +0300
Orit Wasserman  wrote:

> "ram" is present also when migration completes.
> expected-downtime, total-time and downtime are no longer part of "ram" data.
> 
> Signed-off-by: Orit Wasserman 

Applied to the qmp branch, thanks.

> ---
>  qmp-commands.hx | 20 ++--
>  1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 2e59b0d..a22a841 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -2626,8 +2626,8 @@ The main json-object contains the following:
>  - "expected-downtime": only present while migration is active
>  total amount in ms for downtime that was calculated on
>  the last bitmap round (json-int)
> -- "ram": only present if "status" is "active", it is a json-object with the
> -  following RAM information:
> +- "ram": only present if "status" is "active" or "complete", it is a
> + json-object with the following RAM information:
>   - "transferred": amount transferred in bytes (json-int)
>   - "remaining": amount remaining to transfer in bytes (json-int)
>   - "total": total amount of memory in bytes (json-int)
> @@ -2669,12 +2669,12 @@ Examples:
>  -> { "execute": "query-migrate" }
>  <- { "return": {
>  "status": "completed",
> +"total-time":12345,
> +"downtime":12345,
>  "ram":{
>"transferred":123,
>"remaining":123,
>"total":246,
> -  "total-time":12345,
> -  "downtime":12345,
>"duplicate":123,
>"normal":123,
>"normal-bytes":123456
> @@ -2693,12 +2693,12 @@ Examples:
>  <- {
>"return":{
>   "status":"active",
> + "total-time":12345,
> + "expected-downtime":12345,
>   "ram":{
>  "transferred":123,
>  "remaining":123,
>  "total":246,
> -"total-time":12345,
> -"expected-downtime":12345,
>  "duplicate":123,
>  "normal":123,
>  "normal-bytes":123456
> @@ -2712,12 +2712,12 @@ Examples:
>  <- {
>"return":{
>   "status":"active",
> + "total-time":12345,
> + "expected-downtime":12345,
>   "ram":{
>  "total":1057024,
>  "remaining":1053304,
>  "transferred":3720,
> -"total-time":12345,
> -"expected-downtime":12345,
>  "duplicate":123,
>  "normal":123,
>  "normal-bytes":123456
> @@ -2736,13 +2736,13 @@ Examples:
>  <- {
>"return":{
>   "status":"active",
> + "total-time":12345,
> + "expected-downtime":12345,
>   "capabilities" : [ { "capability": "xbzrle", "state" : true } ],
>   "ram":{
>  "total":1057024,
>  "remaining":1053304,
>  "transferred":3720,
> -"total-time":12345,
> -"expected-downtime":12345,
>  "duplicate":10,
>  "normal":,
>  "normal-bytes":3412992




Re: [Qemu-devel] [PATCH] pc: compat: remove PCLMULQDQ from Westmere on pc-*-1.4 and older

2013-08-12 Thread Andreas Färber
Am 09.08.2013 19:24, schrieb Eduardo Habkost:
> On Fri, Aug 09, 2013 at 04:47:53PM +0200, Andreas Färber wrote:
>> Am 09.08.2013 16:11, schrieb Eduardo Habkost:
>>> commit 41cb383f42d0cb51d8e3e25e3ecebc954dd4196f made a guest-visible
>>> change by adding the PCLMULQDQ bit to Westmere without adding
>>> compatibility code to keep the ABI older machine-types. This patch fixes
>>> it by adding the missing compat code.
>>>
>>> Signed-off-by: Eduardo Habkost 
>>> ---
>>> Bug detected by the virt-test CPUID-dump comparison test case, available at:
>>>   https://github.com/autotest/virt-test/pull/714
>>> ---
>>>  hw/i386/pc_piix.c | 1 +
>>>  hw/i386/pc_q35.c  | 1 +
>>>  2 files changed, 2 insertions(+)
>>>
>>> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
>>> index ab25458..2817092 100644
>>> --- a/hw/i386/pc_piix.c
>>> +++ b/hw/i386/pc_piix.c
>>> @@ -259,6 +259,7 @@ static void pc_init_pci_1_4(QEMUMachineInitArgs *args)
>>>  {
>>>  has_pvpanic = false;
>>>  x86_cpu_compat_set_features("n270", FEAT_1_ECX, 0, CPUID_EXT_MOVBE);
>>> +x86_cpu_compat_set_features("Westmere", FEAT_1_ECX, 0, 
>>> CPUID_EXT_PCLMULQDQ);
>>>  pc_init_pci_1_5(args);
>>>  }
>>>  
>>> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
>>> index 2f35d12..25c6b33 100644
>>> --- a/hw/i386/pc_q35.c
>>> +++ b/hw/i386/pc_q35.c
>>> @@ -227,6 +227,7 @@ static void pc_q35_init_1_4(QEMUMachineInitArgs *args)
>>>  {
>>>  has_pvpanic = false;
>>>  x86_cpu_compat_set_features("n270", FEAT_1_ECX, 0, CPUID_EXT_MOVBE);
>>> +x86_cpu_compat_set_features("Westmere", FEAT_1_ECX, 0, 
>>> CPUID_EXT_PCLMULQDQ);
>>>  pc_q35_init_1_5(args);
>>>  }
>>>  
>>
>> Looks good to me, still need to test, and I should probably add
>>
>> Cc: qemu-sta...@nongnu.org
>>
>> for 1.5.3 then.
> 
> The patch doesn't apply cleanly on stable-1.5, but I will send a version
> for qemu-stable as well.

It's not required to apply cleanly to a specific stable- branch. :)
We need a commit id to reference in the backported version.

Thanks, applied to qom-cpu, pull coming later today.
https://github.com/afaerber/qemu-cpu/commits/qom-cpu

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 for 1.6 1/2] memory: Provide separate handling of unassigned io ports accesses

2013-08-12 Thread Anthony Liguori
Jan Kiszka  writes:

> Accesses to unassigned io ports shall return -1 on read and be ignored
> on write. Ensure these properties via dedicated ops, decoupling us from
> the memory core's handling of unassigned accesses.
>
> Signed-off-by: Jan Kiszka 

Breaks the build (linux-user):

  LINK  xtensa-softmmu/qemu-system-xtensa
  CCalpha-linux-user/exec.o
In file included from /home/aliguori/git/qemu/include/hw/hw.h:11:0,
 from /home/aliguori/git/qemu/exec.c:30:
/home/aliguori/git/qemu/include/exec/ioport.h:48:1: error: unknown type name 
‘MemoryRegionOps’
make[1]: *** [exec.o] Error 1
make: *** [subdir-alpha-linux-user] Error 2

Regards,

Anthony Liguori

> ---
>  exec.c|3 ++-
>  include/exec/ioport.h |2 ++
>  ioport.c  |   16 
>  3 files changed, 20 insertions(+), 1 deletions(-)
>
> diff --git a/exec.c b/exec.c
> index 3ca9381..9ed598f 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -1820,7 +1820,8 @@ static void memory_map_init(void)
>  address_space_init(&address_space_memory, system_memory, "memory");
>  
>  system_io = g_malloc(sizeof(*system_io));
> -memory_region_init(system_io, NULL, "io", 65536);
> +memory_region_init_io(system_io, NULL, &unassigned_io_ops, NULL, "io",
> +  65536);
>  address_space_init(&address_space_io, system_io, "I/O");
>  
>  memory_listener_register(&core_memory_listener, &address_space_memory);
> diff --git a/include/exec/ioport.h b/include/exec/ioport.h
> index bdd4e96..84f7f85 100644
> --- a/include/exec/ioport.h
> +++ b/include/exec/ioport.h
> @@ -45,6 +45,8 @@ typedef struct MemoryRegionPortio {
>  
>  #define PORTIO_END_OF_LIST() { }
>  
> +extern const MemoryRegionOps unassigned_io_ops;
> +
>  void cpu_outb(pio_addr_t addr, uint8_t val);
>  void cpu_outw(pio_addr_t addr, uint16_t val);
>  void cpu_outl(pio_addr_t addr, uint32_t val);
> diff --git a/ioport.c b/ioport.c
> index 79b7f1a..707cce8 100644
> --- a/ioport.c
> +++ b/ioport.c
> @@ -44,6 +44,22 @@ typedef struct MemoryRegionPortioList {
>  MemoryRegionPortio ports[];
>  } MemoryRegionPortioList;
>  
> +static uint64_t unassigned_io_read(void *opaque, hwaddr addr, unsigned size)
> +{
> +return -1ULL;
> +}
> +
> +static void unassigned_io_write(void *opaque, hwaddr addr, uint64_t val,
> +unsigned size)
> +{
> +}
> +
> +const MemoryRegionOps unassigned_io_ops = {
> +.read = unassigned_io_read,
> +.write = unassigned_io_write,
> +.endianness = DEVICE_NATIVE_ENDIAN,
> +};
> +
>  void cpu_outb(pio_addr_t addr, uint8_t val)
>  {
>  LOG_IOPORT("outb: %04"FMT_pioaddr" %02"PRIx8"\n", addr, val);
> -- 
> 1.7.3.4




Re: [Qemu-devel] [PATCH v2] pc: drop external DSDT loading

2013-08-12 Thread Michael S. Tsirkin
On Mon, Aug 12, 2013 at 09:01:44AM -0500, Anthony Liguori wrote:
> This breaks migration and is unneeded with modern SeaBIOS.
> 
> Signed-off-by: Anthony Liguori 

Hmm don't we want to keep it around for machine types
1.4.0 and 1.5.0?

By the way, copy stable as well?
Loading it unconditonally is a cross
version migration bug that we probably want to fix
on stable branch - disabling for 1.3.0 and older.

> ---
> v1 -> v2
>  - Still load external DSDT for q35
> ---
>  hw/i386/pc_piix.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 95c45b8..311574a 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -103,7 +103,6 @@ static void pc_init1(MemoryRegion *system_memory,
>OBJECT(icc_bridge), NULL);
>  
>  pc_cpus_init(cpu_model, icc_bridge);
> -pc_acpi_init("acpi-dsdt.aml");
>  
>  if (kvm_enabled() && kvmclock_enabled) {
>  kvmclock_create();
> -- 
> 1.8.0



Re: [Qemu-devel] [PATCH v2] pc: drop external DSDT loading

2013-08-12 Thread Anthony Liguori
"Michael S. Tsirkin"  writes:

> On Mon, Aug 12, 2013 at 09:01:44AM -0500, Anthony Liguori wrote:
>> This breaks migration and is unneeded with modern SeaBIOS.
>> 
>> Signed-off-by: Anthony Liguori 
>
> Hmm don't we want to keep it around for machine types
> 1.4.0 and 1.5.0?

Hrm, why?

Regards,

Anthony Liguori

>
> By the way, copy stable as well?
> Loading it unconditonally is a cross
> version migration bug that we probably want to fix
> on stable branch - disabling for 1.3.0 and older.
>
>> ---
>> v1 -> v2
>>  - Still load external DSDT for q35
>> ---
>>  hw/i386/pc_piix.c | 1 -
>>  1 file changed, 1 deletion(-)
>> 
>> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
>> index 95c45b8..311574a 100644
>> --- a/hw/i386/pc_piix.c
>> +++ b/hw/i386/pc_piix.c
>> @@ -103,7 +103,6 @@ static void pc_init1(MemoryRegion *system_memory,
>>OBJECT(icc_bridge), NULL);
>>  
>>  pc_cpus_init(cpu_model, icc_bridge);
>> -pc_acpi_init("acpi-dsdt.aml");
>>  
>>  if (kvm_enabled() && kvmclock_enabled) {
>>  kvmclock_create();
>> -- 
>> 1.8.0




Re: [Qemu-devel] SCSI bus failures with qemu-arm in kernel 3.8+

2013-08-12 Thread Guenter Roeck

On 08/11/2013 03:04 PM, Russell King - ARM Linux wrote:

On Sun, Aug 11, 2013 at 08:54:43AM -0700, Guenter Roeck wrote:

Hi,

trying to boot arm versatile images with qemu results in the following error
if I try to boot with a disk image.

  sym0: <895a> rev 0x0 at pci :00:0d.0 irq 92
  sym0: SCSI BUS has been reset.
  scsi0 : sym-2.2.3
  [...]
  scsi 0:0:0:0: ABORT operation started
  scsi 0:0:0:0: ABORT operation timed-out.
  [...]

Yocto's 3.8 kernel images work, upstream kernels 3.8 and later fail
(I did not check if/how earlier kernels are affected).

Tracking this down shows that the problem is known and has been fixed with
commit 351d1339 (arm: add dummy swizzle for versatile with qemu) in the
Yocto 3.8 kernel at git://git.yoctoproject.org/linux-yocto-3.8.

Would it be possible to submit this patch for inclusion into affected upstream 
kernels ?


It could be that it's qemu's PCI routing is wrong - it's not the first
time that qemu has got something wrong.

Unfortunately, the PCI routing is totally undocumented, and as I understand
it, there's very few backplanes out there now that finding out their real
routing is virtually impossible.  I'm loathed to change it unless someone
can point to a definitive source of information on this.



Maybe Paul can comment, as he wrote the patch.

If it helps, I tried with qemu 1.4.0 from Ubuntu 13.4, qemu 1.4.0 from Poky 
1.4.0-1,
and qemu 1.5.2 from the qemu repository.

Copying qemu-devel to increase the audience.

Guenter




[Qemu-devel] [PATCH v2 for 1.6 1/2] memory: Provide separate handling of unassigned io ports accesses

2013-08-12 Thread Jan Kiszka
Accesses to unassigned io ports shall return -1 on read and be ignored
on write. Ensure these properties via dedicated ops, decoupling us from
the memory core's handling of unassigned accesses.

Signed-off-by: Jan Kiszka 
---

Changes in v2:
 - avoid breakage in ioport.h for user build

 exec.c|3 ++-
 include/exec/ioport.h |4 
 ioport.c  |   16 
 3 files changed, 22 insertions(+), 1 deletions(-)

diff --git a/exec.c b/exec.c
index 3ca9381..9ed598f 100644
--- a/exec.c
+++ b/exec.c
@@ -1820,7 +1820,8 @@ static void memory_map_init(void)
 address_space_init(&address_space_memory, system_memory, "memory");
 
 system_io = g_malloc(sizeof(*system_io));
-memory_region_init(system_io, NULL, "io", 65536);
+memory_region_init_io(system_io, NULL, &unassigned_io_ops, NULL, "io",
+  65536);
 address_space_init(&address_space_io, system_io, "I/O");
 
 memory_listener_register(&core_memory_listener, &address_space_memory);
diff --git a/include/exec/ioport.h b/include/exec/ioport.h
index bdd4e96..b3848be 100644
--- a/include/exec/ioport.h
+++ b/include/exec/ioport.h
@@ -45,6 +45,10 @@ typedef struct MemoryRegionPortio {
 
 #define PORTIO_END_OF_LIST() { }
 
+#ifndef CONFIG_USER_ONLY
+extern const MemoryRegionOps unassigned_io_ops;
+#endif
+
 void cpu_outb(pio_addr_t addr, uint8_t val);
 void cpu_outw(pio_addr_t addr, uint16_t val);
 void cpu_outl(pio_addr_t addr, uint32_t val);
diff --git a/ioport.c b/ioport.c
index 79b7f1a..707cce8 100644
--- a/ioport.c
+++ b/ioport.c
@@ -44,6 +44,22 @@ typedef struct MemoryRegionPortioList {
 MemoryRegionPortio ports[];
 } MemoryRegionPortioList;
 
+static uint64_t unassigned_io_read(void *opaque, hwaddr addr, unsigned size)
+{
+return -1ULL;
+}
+
+static void unassigned_io_write(void *opaque, hwaddr addr, uint64_t val,
+unsigned size)
+{
+}
+
+const MemoryRegionOps unassigned_io_ops = {
+.read = unassigned_io_read,
+.write = unassigned_io_write,
+.endianness = DEVICE_NATIVE_ENDIAN,
+};
+
 void cpu_outb(pio_addr_t addr, uint8_t val)
 {
 LOG_IOPORT("outb: %04"FMT_pioaddr" %02"PRIx8"\n", addr, val);
-- 
1.7.3.4



[Qemu-devel] [PULL for-1.6 0/1] QOM CPUState patch queue 2013-08-12

2013-08-12 Thread Andreas Färber
Hello Anthony,

This is my current QOM CPU patch queue for 1.6. Please pull.

Thanks,
Andreas

Cc: Anthony Liguori 

The following changes since commit 8f3067bd86485f8cd03abc940ddb2b8467ef3627:

  rdma: remaining documentation fixes (2013-08-12 09:31:16 -0500)

are available in the git repository at:

  git://github.com/afaerber/qemu-cpu.git tags/qom-cpu-for-anthony

for you to fetch changes up to 56383703c060777fd01aaf8d63d5f46d660e9fb9:

  pc: Remove PCLMULQDQ from Westmere on pc-*-1.4 and older (2013-08-12 17:33:28 
+0200)


QOM CPUState refactorings

* Fix X86CPU Westmere CPUID for pc-*-1.4 and older


Eduardo Habkost (1):
  pc: Remove PCLMULQDQ from Westmere on pc-*-1.4 and older

 hw/i386/pc_piix.c | 1 +
 hw/i386/pc_q35.c  | 1 +
 2 files changed, 2 insertions(+)



[Qemu-devel] [PULL 1/1] pc: Remove PCLMULQDQ from Westmere on pc-*-1.4 and older

2013-08-12 Thread Andreas Färber
From: Eduardo Habkost 

Commit 41cb383f42d0cb51d8e3e25e3ecebc954dd4196f made a guest-visible
change by adding the PCLMULQDQ bit to Westmere without adding
compatibility code to keep the ABI for older machine-types.
Fix it by adding the missing compat code.

Signed-off-by: Eduardo Habkost 
Signed-off-by: Andreas Färber 
---
 hw/i386/pc_piix.c | 1 +
 hw/i386/pc_q35.c  | 1 +
 2 files changed, 2 insertions(+)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index a19e172..1329f97 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -265,6 +265,7 @@ static void pc_init_pci_1_4(QEMUMachineInitArgs *args)
 {
 has_pvpanic = false;
 x86_cpu_compat_set_features("n270", FEAT_1_ECX, 0, CPUID_EXT_MOVBE);
+x86_cpu_compat_set_features("Westmere", FEAT_1_ECX, 0, 
CPUID_EXT_PCLMULQDQ);
 pc_init_pci_1_5(args);
 }
 
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index be6013f..1d84ead 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -233,6 +233,7 @@ static void pc_q35_init_1_4(QEMUMachineInitArgs *args)
 {
 has_pvpanic = false;
 x86_cpu_compat_set_features("n270", FEAT_1_ECX, 0, CPUID_EXT_MOVBE);
+x86_cpu_compat_set_features("Westmere", FEAT_1_ECX, 0, 
CPUID_EXT_PCLMULQDQ);
 pc_q35_init_1_5(args);
 }
 
-- 
1.8.1.4




Re: [Qemu-devel] [PATCH v2 for 1.6 1/2] memory: Provide separate handling of unassigned io ports accesses

2013-08-12 Thread Anthony Liguori
Jan Kiszka  writes:

> Accesses to unassigned io ports shall return -1 on read and be ignored
> on write. Ensure these properties via dedicated ops, decoupling us from
> the memory core's handling of unassigned accesses.
>
> Signed-off-by: Jan Kiszka 

Please top-post.

Regards,

Anthony Liguori

> ---
>
> Changes in v2:
>  - avoid breakage in ioport.h for user build
>
>  exec.c|3 ++-
>  include/exec/ioport.h |4 
>  ioport.c  |   16 
>  3 files changed, 22 insertions(+), 1 deletions(-)
>
> diff --git a/exec.c b/exec.c
> index 3ca9381..9ed598f 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -1820,7 +1820,8 @@ static void memory_map_init(void)
>  address_space_init(&address_space_memory, system_memory, "memory");
>  
>  system_io = g_malloc(sizeof(*system_io));
> -memory_region_init(system_io, NULL, "io", 65536);
> +memory_region_init_io(system_io, NULL, &unassigned_io_ops, NULL, "io",
> +  65536);
>  address_space_init(&address_space_io, system_io, "I/O");
>  
>  memory_listener_register(&core_memory_listener, &address_space_memory);
> diff --git a/include/exec/ioport.h b/include/exec/ioport.h
> index bdd4e96..b3848be 100644
> --- a/include/exec/ioport.h
> +++ b/include/exec/ioport.h
> @@ -45,6 +45,10 @@ typedef struct MemoryRegionPortio {
>  
>  #define PORTIO_END_OF_LIST() { }
>  
> +#ifndef CONFIG_USER_ONLY
> +extern const MemoryRegionOps unassigned_io_ops;
> +#endif
> +
>  void cpu_outb(pio_addr_t addr, uint8_t val);
>  void cpu_outw(pio_addr_t addr, uint16_t val);
>  void cpu_outl(pio_addr_t addr, uint32_t val);
> diff --git a/ioport.c b/ioport.c
> index 79b7f1a..707cce8 100644
> --- a/ioport.c
> +++ b/ioport.c
> @@ -44,6 +44,22 @@ typedef struct MemoryRegionPortioList {
>  MemoryRegionPortio ports[];
>  } MemoryRegionPortioList;
>  
> +static uint64_t unassigned_io_read(void *opaque, hwaddr addr, unsigned size)
> +{
> +return -1ULL;
> +}
> +
> +static void unassigned_io_write(void *opaque, hwaddr addr, uint64_t val,
> +unsigned size)
> +{
> +}
> +
> +const MemoryRegionOps unassigned_io_ops = {
> +.read = unassigned_io_read,
> +.write = unassigned_io_write,
> +.endianness = DEVICE_NATIVE_ENDIAN,
> +};
> +
>  void cpu_outb(pio_addr_t addr, uint8_t val)
>  {
>  LOG_IOPORT("outb: %04"FMT_pioaddr" %02"PRIx8"\n", addr, val);
> -- 
> 1.7.3.4




Re: [Qemu-devel] virtio-net with vhost RX stall

2013-08-12 Thread Benoît Canet
> Any chance the bug is fixed by 'virtio_net: fix race in RX VQ processing'?

We will test if we have more luck with a 3.10 and a backport of this patch.

> Dump the ring state and index values where they poll, on host and guest.

ok

Thanks

Benoît



Re: [Qemu-devel] [PATCH v2 for 1.6 1/2] memory: Provide separate handling of unassigned io ports accesses

2013-08-12 Thread Andreas Färber
Am 12.08.2013 17:29, schrieb Jan Kiszka:
> Accesses to unassigned io ports shall return -1 on read and be ignored
> on write. Ensure these properties via dedicated ops, decoupling us from
> the memory core's handling of unassigned accesses.
> 
> Signed-off-by: Jan Kiszka 
> ---
> 
> Changes in v2:
>  - avoid breakage in ioport.h for user build

Looks OK, but if you want Anthony to apply this for rc3 then please
repost as a full, top-level series.

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 for-1.6 V2 0/2] pvpanic: Separate pvpanic from machine type

2013-08-12 Thread Eric Blake
On 08/11/2013 09:10 AM, Marcel Apfelbaum wrote:
> Creating the pvpanic device as part of the machine type has the
> potential to trigger guest OS, guest firmware and driver bugs.
> The potential of such was originally viewed as minimal.
> However, since releasing 1.5 with pvpanic as part
> of the builtin machine type, several issues were observed
> in the field:
>  - Some Windows versions triggered 'New Hardware Wizard' and
>an unidentified device appeared in Device Manager.
>  - Issue reported off list: on Linux >= 3.10
>the pvpanic driver breaks the reset on crash option:
>VM stops instead of being reset.
> 
> pvpanic device also changes monitor command behaviour in some cases,
> such silent incompatible changes aren't expected by management tools:
>  - Monitor command requires 'cont' before 'system_reset'
>in order to restart the VM after kernel panic/BSOD 
> 
> Note that libvirt is the main user and libvirt people indicated their
> preference to creating device with -device pvpanic rather than a
> built-in one that can't be removed.
> 
> These issues were raised at last KVM call. The agreement reached
> there was that we were a bit too rash to make the device
> a builtin, and that for 1.6 we should drop the pvpanic device from the
> default machine type, instead teach management tools to add it by
> default using -device pvpanic.
> It's not clear whether changing 1.5 behaviour at this point
> is a sane thing, so this patchset doesn't touch 1.5 machine type.

Thanks for doing this; it makes sense to get this in for 1.6.  From the
libvirt point of view:

Series: 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 v5] vl.c: Output error on invalid machine type provided

2013-08-12 Thread Eric Blake
On 07/31/2013 01:04 AM, Michal Novotny wrote:
> Output error message using qemu's error_report() function when user
> provides the invalid machine type on the command line. This also saves
> time to find what issue is when you downgrade from one version of qemu
> to another that doesn't support required machine type yet (the version
> user downgraded to have to have this patch applied too, of course).
> 
> Signed-off-by: Michal Novotny 
> ---
>  vl.c | 5 +
>  1 file changed, 5 insertions(+)

Are you trying to get this in 1.6?

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 v5] vl.c: Output error on invalid machine type provided

2013-08-12 Thread Michal Novotny

On 08/12/2013 05:55 PM, Eric Blake wrote:
> On 07/31/2013 01:04 AM, Michal Novotny wrote:
>> Output error message using qemu's error_report() function when user
>> provides the invalid machine type on the command line. This also saves
>> time to find what issue is when you downgrade from one version of qemu
>> to another that doesn't support required machine type yet (the version
>> user downgraded to have to have this patch applied too, of course).
>>
>> Signed-off-by: Michal Novotny 
>> ---
>>  vl.c | 5 +
>>  1 file changed, 5 insertions(+)
> Are you trying to get this in 1.6?
>
> Reviewed-by: Eric Blake 
>

It's ok for 1.6. It applies cleanly. So if it's fine with you please
push it to the repository.

Thanks,
Michal

-- 
Michal Novotny , RHCE, Red Hat
Virtualization | libvirt-php bindings | php-virt-control.org




Re: [Qemu-devel] [PATCH for-1.6 V2 1/2] hw/misc: don't create pvpanic device by default

2013-08-12 Thread Andreas Färber
Am 11.08.2013 17:10, schrieb Marcel Apfelbaum:
> This patch is based on Hu Tao's:
> http://lists.nongnu.org/archive/html/qemu-devel/2013-08/msg00124.html
> 
> No need to hard-code pvpanic as part of the machine.
> It can be added with "-device pvpanic" from command line (The next patch).
> Anyway, for backport compatibility it is still part of 1.5
> machine.
> 
> Signed-off-by: Marcel Apfelbaum 
> ---
> Changes from v1:
>  - Keep pvpanic device enabled by default for 1.5
>for backport compatibility
> 
>  hw/i386/pc_piix.c | 9 -
>  hw/i386/pc_q35.c  | 7 ---
>  2 files changed, 8 insertions(+), 8 deletions(-)

Reviewed-by: Andreas Färber 

Anthony, note that my pending CPU pull adds a line to the two _1_4
functions, so that they trivially conflict.

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] [RFC] [PATCHv10 08/31] aio / timers: Split QEMUClock into QEMUClock and QEMUTimerList

2013-08-12 Thread Jan Kiszka
On 2013-08-11 18:43, Alex Bligh wrote:
> Split QEMUClock into QEMUClock and QEMUTimerList so that we can
> have more than one QEMUTimerList associated with the same clock.
> 
> Introduce a main_loop_timerlist concept and make existing
> qemu_clock_* calls that actually should operate on a QEMUTimerList
> call the relevant QEMUTimerList implementations, using the clock's
> default timerlist. This vastly reduces the invasiveness of this
> change and means the API stays constant for existing users.
> 
> Introduce a list of QEMUTimerLists associated with each clock
> so that reenabling the clock can cause all the notifiers
> to be called. Note the code to do the notifications is added
> in a later patch.
> 
> Switch QEMUClockType to an enum. Remove global variables vm_clock,
> host_clock and rt_clock and add compatibility defines. Do not
> fix qemu_next_alarm_deadline as it's going to be deleted.
> 
> Add qemu_clock_use_for_deadline to indicate whether a particular
> clock should be used for deadline calculations. When use_icount
> is true, vm_clock should not be used for deadline calculations
> as it does not contain a nanosecond count. Instead, icount
> timeouts come from the execution thread doing aio_notify or
> qemu_notify as appropriate. This function is used in the next
> patch.
> 
> Signed-off-by: Alex Bligh 
> ---
>  include/qemu/timer.h |  347 
> ++
>  qemu-timer.c |  207 ++
>  2 files changed, 475 insertions(+), 79 deletions(-)
> 
> diff --git a/include/qemu/timer.h b/include/qemu/timer.h
> index fcb6a42..a217a81 100644
> --- a/include/qemu/timer.h
> +++ b/include/qemu/timer.h
> @@ -11,34 +11,84 @@
>  #define SCALE_US 1000
>  #define SCALE_NS 1
>  
> -#define QEMU_CLOCK_REALTIME 0
> -#define QEMU_CLOCK_VIRTUAL  1
> -#define QEMU_CLOCK_HOST 2
> +/**
> + * QEMUClockType:
> + *
> + * The following clock types are available:
> + *
> + * @QEMU_CLOCK_REALTIME: Real time clock
> + *
> + * The real time clock should be used only for stuff which does not
> + * change the virtual machine state, as it is run even if the virtual
> + * machine is stopped. The real time clock has a frequency of 1000
> + * Hz.
> + *
> + * Formerly rt_clock
> + *
> + * @QEMU_CLOCK_VIRTUAL: virtual clock
> + *
> + * The virtual clock is only run during the emulation. It is stopped
> + * when the virtual machine is stopped. Virtual timers use a high
> + * precision clock, usually cpu cycles (use ticks_per_sec).
> + *
> + * Formerly vm_clock
> + *
> + * @QEMU_CLOCK_HOST: host clock
> + *
> + * The host clock should be use for device models that emulate accurate
> + * real time sources. It will continue to run when the virtual machine
> + * is suspended, and it will reflect system time changes the host may
> + * undergo (e.g. due to NTP). The host clock has the same precision as
> + * the virtual clock.
> + *
> + * Formerly host_clock
> + */
> +
> +typedef enum {
> +QEMU_CLOCK_REALTIME = 0,
> +QEMU_CLOCK_VIRTUAL = 1,
> +QEMU_CLOCK_HOST = 2,
> +QEMU_CLOCK_MAX
> +} QEMUClockType;
>  
>  typedef struct QEMUClock QEMUClock;
> +typedef struct QEMUTimerList QEMUTimerList;
>  typedef void QEMUTimerCB(void *opaque);
>  
> -/* The real time clock should be used only for stuff which does not
> -   change the virtual machine state, as it is run even if the virtual
> -   machine is stopped. The real time clock has a frequency of 1000
> -   Hz. */
> -extern QEMUClock *rt_clock;
> +typedef struct QEMUTimer {
> +int64_t expire_time;/* in nanoseconds */
> +QEMUTimerList *timer_list;
> +QEMUTimerCB *cb;
> +void *opaque;
> +QEMUTimer *next;
> +int scale;
> +} QEMUTimer;

The typedef part is a duplication of what we already have in
qemu/typedefs.h and breaks the build for me. Just declare the struct here.

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux



Re: [Qemu-devel] [PATCH v5] vl.c: Output error on invalid machine type provided

2013-08-12 Thread Eric Blake
On 08/12/2013 09:58 AM, Michal Novotny wrote:
> 
> On 08/12/2013 05:55 PM, Eric Blake wrote:
>> On 07/31/2013 01:04 AM, Michal Novotny wrote:
>>> Output error message using qemu's error_report() function when user
>>> provides the invalid machine type on the command line. This also saves
>>> time to find what issue is when you downgrade from one version of qemu
>>> to another that doesn't support required machine type yet (the version
>>> user downgraded to have to have this patch applied too, of course).
>>>
>>> Signed-off-by: Michal Novotny 
>>> ---
>>>  vl.c | 5 +
>>>  1 file changed, 5 insertions(+)
>> Are you trying to get this in 1.6?
>>
>> Reviewed-by: Eric Blake 
>>
> 
> It's ok for 1.6. It applies cleanly. So if it's fine with you please
> push it to the repository.

I'm not the maintainer, so I'm not the one that can push it.  At this
phase in the game, it helps if you resend a patch with 'for 1.6' inside
the [] as part of the subject line, as well as cc the maintainer listed
by ./scripts/get_maintainer.pl, to make sure the actual maintainer sees
it and can make a more-informed decision on whether it qualifies as a
bug fix.

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



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] SCSI bus failures with qemu-arm in kernel 3.8+

2013-08-12 Thread Peter Maydell
On 12 August 2013 01:40, Guenter Roeck  wrote:
> On 08/11/2013 03:04 PM, Russell King - ARM Linux wrote:
>> It could be that it's qemu's PCI routing is wrong - it's not the first
>> time that qemu has got something wrong.

QEMU 1.5 has had its Versatile PCI routing code rewritten to
correspond with the hardware (cross-tested versus Arnd Bergmann's
patchset
http://marc.info/?l=linux-arm-kernel&m=128707282403376&w=2
which was run on real versatilePB backplane hardware and
could handle a PCI SATA card). I believe it to be correct,
and I spent a fairly long time wading through the various bits
of documentation and testing those kernel patches on h/w.

(It also includes a back-compatibility fudge so that old
kernels which assumed the old broken QEMU behaviour will
continue to work; it does not attempt to fix up the problems
with kernels from commit 1bc39ac5d to present which don't
work properly on either QEMU or real hardware. A fixed
kernel can force-disable the fudge.)

http://git.qemu.org/?p=qemu.git;a=blob;f=hw/pci-host/versatile.c
has the current PCI code, which includes comments about what
the routing is (in particular tables at lines 321 and 340).

>> Unfortunately, the PCI routing is totally undocumented, and as I
>> understand
>> it, there's very few backplanes out there now that finding out their real
>> routing is virtually impossible.  I'm loathed to change it unless someone
>> can point to a definitive source of information on this.

The Versatile boards have TRMs on infocenter.arm.com; the
backplane's wiring is documented in the circuit diagrams
which were distributed on the CD which came with the board
(ie they are non-confidential information, just difficult
to lay your hands on). It's also possible to just test
the hardware, which is what Arnd and I did to confirm that
his patchset was OK.

If somebody would like to fix the kernel I am happy to
locate the PCI backplane and test everything (again).
I would suggest that producing some patches which work
with QEMU 1.5 or later would be a good start; then we
can test on h/w as confirmation before they are applied.

-- PMM



Re: [Qemu-devel] [RFC] [PATCHv10 08/31] aio / timers: Split QEMUClock into QEMUClock and QEMUTimerList

2013-08-12 Thread Alex Bligh

On 12 Aug 2013, at 17:14, Jan Kiszka wrote:

> The typedef part is a duplication of what we already have in
> qemu/typedefs.h and breaks the build for me. Just declare the struct here.

You mean one can't do

typedef struct foo foo;
...
typedef struct foo {
  ...
} foo;

?

I don't even get a warning for that. Learn a new thing every
day. OK will fix. I guess that means timer.h needs to explicitly
include typedefs.h in case whatever is including timer.h does
not first include typedefs.h.

-- 
Alex Bligh







  1   2   3   >