date:20120310

On 11.03.2012 05:49, Michael Tokarev wrote:
> This is a little cleanup/consolidation for some iovec-related
> low-level routines in qemu.
> 
> The plan is to make library functions more understandable,
> consistent and useful.
> 
> The patch changes prototypes of several iov and qiov functions
> to match each other, changes types of arguments for some
> functions, _swaps_ some function arguments with each other,
> and makes use of common code in r/w path.
> 
> The result of all these changes.
> 
> 1. Most qiov-related (qemu_iovec_*) functions now accepts
>  'offset' parameter to specify from which (byte) position to
>  start operation.  This is added for _memset (removing
>  _memset_skip), _from_buffer (allowing to copy a bounce-
>  buffer to a middle of qiov).  Typical:
> 
>   void qemu_iovec_memset(QEMUIOVector *qiov, size_t offset, int c, size_t 
> bytes);
> 
> 2. All functions that accepts this `offset' argument does
>  it in a similar manner, following the
>iov,fromwhere,bytes
>  pattern.  This is consistent with (updated) qemu_sendv()
>  and qemu_recvv() and friends, where `offset' and `bytes'
>  arguments were _renamed_, with the following prototypes:
> 
>int qemu_sendv(sockfd, iov, size_t offset, size_t bytes)
> 
>  instead of
> 
>int qemu_sendv(sockfd, iov, int len, int iov_offset)
> 
>  See how offset & bytes are used in the same way as for qemu_iovec_*
>  A few callers of these are verified and converted.
> 
> 3. Used size_t instead of various variations for byte counts.
>  Including qemu_iovec_copy which used uint64_t(!) type.
> 
> 4. Function arguments are renamed to better match with their
>  actual meaning.  Compare new and original prototype of
>  qemu_sendv() above: old prototype with `len' does not
>  tell if `len' refers to number of iov elements (as
>  regular writev() call) or to number of data bytes.
>  Ditto for several usages of `count' for some qemu_iovec_*,
>  which is also replaced to `bytes'.
> 
> The resulting function usage is much more consistent, the
> functions themselves are nice and understandable, which
> means they're easier to use and less error-prone.
> 
> This patchset also consolidates a few low-level send&recv
> functions into one, since both versions were exactly
> the same (and were finally calling common function anyway).
> This is done by exporting a common send_recv function with
> one extra bool argument, and making current send&recv to
> be just #defines.
> 
> And while at it all, also made some implementations shorter,
> cleaner and much easier to read/understand, and add some
> code comments.
> 
> The read&write consolidation has great potential for the
> block layer, as has been demonstrated before.
> Unification and generalization of qemu_iovec_* functions
> will let to optimize/simplify some more code in block/*,
> especially qemu_iovec_memset() and _from_buffer() (this
> optimization/simplification isn't done in this series)

And ofcourse I forgot to mention: the difference from
initial patch:

- picked 2 more qemu_iovec_* functions (minor), and
- changed order of arguments of sendv() and recvv().

The second point is relatively major, since it changes quite
some innocent places, and has a potential of breaking things
due to someone not noticing the args has been reordered.

But this is something I was wanted to do initially, because
this is how the args should has been initially (IMHO anyway),
but wasn't brave enough to actually change it.  But it is
better to do it now while we don't have lots of callers,
instead of not changing them at all because it's "too late"
and everyone got used to it.

Thanks,

/mjt

Re: [Qemu-devel] [PATCH 13/13] pseries: Implement IOMMU and DMA for PAPR PCI devices

2012-03-10 Thread Benjamin Herrenschmidt

On Fri, 2012-03-09 at 21:58 +1100, David Gibson wrote:
> > What about virtio?
> 
> virtio doesn't use virtualized PCI DMA, it uses direct hypervisor
> access to guest memory, by guest physical address.  It *shouldn't*,
> but it does - that's the way it's specced and that's the way the guest
> kernel expects it to be.  It could be fixed with a new feature bit,
> but that's a project for another day.

More precisely, the patch don't break virtio as virtio just bypasses
this all.

Also having virtio go through the iommu might not be such a great idea,
it should definitely remain optional. The ability of virtio to go
straight to guest memory has some significant performance advantages.

Cheers,
Ben.

[Qemu-devel] [PATCHv2 3/7] consolidate qemu_iovec_copy() and qemu_iovec_concat() and make them consistent

qemu_iovec_concat() is currently a wrapper for qemu_iovec_copy(),
use the former (with extra "0" arg) in a few places where it is used.

Change skip argument of qemu_iovec_copy() from uint64_t to size_t,
since size of qiov itself is size_t, so there's no way to skip larger
sizes.  Rename it to soffset, to make it clear that the offset
is applied to src.

Also change the only usage of uint64_t in hw/9pfs/virtio-9p.c, in
v9fs_init_qiov_from_pdu() - all callers of it actually uses size_t
too, not uint64_t.

Semantic change in the meaning of `count' (now renamed to `sbytes')
argument.  Initial comment said that src is copied to dst until
_total_ size is less than specified, so it might be interpreted
as maximum size of the _dst_ vector.  Actual meaning of if was
that total amount of skipped and copied bytes should not exceed
`count'.  Make it just the amount of bytes to _copy_, without
counting skipped bytes.  This makes it consistent with other
iovec functions, and also matches actual _usage_ of this function.

Order of argumens is already good:
 qemu_iovec_memset(QEMUIOVector *qiov, size_t offset, int c, size_t bytes)
vs:
 qemu_iovec_concat(QEMUIOVector *dst, QEMUIOVector *src, size_t soffset, size_t 
sbytes)
(note soffset is after _src_ not dst, since it applies to src;
for memset it applies to qiov).

Note that in many places where this function is used, the previous
call is qemu_iovec_reset(), which means many callers actually want
copy (replacing dst content), not concat.  So we may want to add a
paramere to allow resetting dst in one go.
---
 block.c |4 +-
 block/qcow2.c   |6 ++--
 block/qed.c |6 ++--
 cutils.c|   56 +++---
 hw/9pfs/virtio-9p.c |8 +++---
 qemu-common.h   |5 +--
 6 files changed, 35 insertions(+), 50 deletions(-)

diff --git a/block.c b/block.c
index cbbedfa..3baa182 100644
--- a/block.c
+++ b/block.c
@@ -2959,13 +2959,13 @@ static int multiwrite_merge(BlockDriverState *bs, 
BlockRequest *reqs,
 // Add the first request to the merged one. If the requests are
 // overlapping, drop the last sectors of the first request.
 size = (reqs[i].sector - reqs[outidx].sector) << 9;
-qemu_iovec_concat(qiov, reqs[outidx].qiov, size);
+qemu_iovec_concat(qiov, reqs[outidx].qiov, 0, size);
 
 // We should need to add any zeros between the two requests
 assert (reqs[i].sector <= oldreq_last);
 
 // Add the second request
-qemu_iovec_concat(qiov, reqs[i].qiov, reqs[i].qiov->size);
+qemu_iovec_concat(qiov, reqs[i].qiov, 0, reqs[i].qiov->size);
 
 reqs[outidx].nb_sectors = qiov->size >> 9;
 reqs[outidx].qiov = qiov;
diff --git a/block/qcow2.c b/block/qcow2.c
index 668e2e2..1723275 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -445,7 +445,7 @@ static coroutine_fn int qcow2_co_readv(BlockDriverState 
*bs, int64_t sector_num,
 index_in_cluster = sector_num & (s->cluster_sectors - 1);
 
 qemu_iovec_reset(&hd_qiov);
-qemu_iovec_copy(&hd_qiov, qiov, bytes_done,
+qemu_iovec_concat(&hd_qiov, qiov, bytes_done,
 cur_nr_sectors * 512);
 
 if (!cluster_offset) {
@@ -514,7 +514,7 @@ static coroutine_fn int qcow2_co_readv(BlockDriverState 
*bs, int64_t sector_num,
 qcow2_encrypt_sectors(s, sector_num,  cluster_data,
 cluster_data, cur_nr_sectors, 0, &s->aes_decrypt_key);
 qemu_iovec_reset(&hd_qiov);
-qemu_iovec_copy(&hd_qiov, qiov, bytes_done,
+qemu_iovec_concat(&hd_qiov, qiov, bytes_done,
 cur_nr_sectors * 512);
 qemu_iovec_from_buffer(&hd_qiov, 0, cluster_data,
 512 * cur_nr_sectors);
@@ -596,7 +596,7 @@ static coroutine_fn int qcow2_co_writev(BlockDriverState 
*bs,
 assert((cluster_offset & 511) == 0);
 
 qemu_iovec_reset(&hd_qiov);
-qemu_iovec_copy(&hd_qiov, qiov, bytes_done,
+qemu_iovec_concat(&hd_qiov, qiov, bytes_done,
 cur_nr_sectors * 512);
 
 if (s->crypt_method) {
diff --git a/block/qed.c b/block/qed.c
index 6f9325b..a4ef12c 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -1133,7 +1133,7 @@ static void qed_aio_write_alloc(QEDAIOCB *acb, size_t len)
 
 acb->cur_nclusters = qed_bytes_to_clusters(s,
 qed_offset_into_cluster(s, acb->cur_pos) + len);
-qemu_iovec_copy(&acb->cur_qiov, acb->qiov, acb->qiov_offset, len);
+qemu_iovec_concat(&acb->cur_qiov, acb->qiov, acb->qiov_offset, len);
 
 if (acb->flags & QED_AIOCB_ZERO) {
 /* Skip ahead if the clusters are already zero */
@@ -1179,7 +1179,7 @@ static void qed_aio_write_inplace(QEDAIOCB *acb, uint64_t 
offset, size_t len)
 
 /* Calculate the I/O vector */
 acb->cur_cluster = offset;
-qemu_iovec_copy(&acb->cur_qiov, acb->qiov, acb->qio

[Qemu-devel] [PATCHv2 6/7] cleanup qemu_co_sendv(), qemu_co_recvv() and friends

The same as for non-coroutine versions in previous patches:
rename arguments to be more obvious, change type of arguments
from int to size_t where appropriate, and use common code for
send and receive paths (with one extra argument) since these
are exactly the same.  Use common qemu_sendv_recvv() directly.
Also constify buf arg of qemu_co_send().

qemu_co_sendv(), qemu_co_recvv(), and qemu_co_recv() are now
trivial #define's merely adding one extra arg.  qemu_co_send()
is an inline function due to `buf' arg de-constification.

qemu_co_sendv() and qemu_co_recvv() callers are converted to
different argument order.

Signed-off-by: Michael Tokarev 
---
 block/nbd.c |4 +-
 block/sheepdog.c|6 ++--
 qemu-common.h   |   38 +++---
 qemu-coroutine-io.c |   83 +-
 4 files changed, 46 insertions(+), 85 deletions(-)

diff --git a/block/nbd.c b/block/nbd.c
index 161b299..9919acc 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -194,7 +194,7 @@ static int nbd_co_send_request(BDRVNBDState *s, struct 
nbd_request *request,
 nbd_have_request, NULL, s);
 rc = nbd_send_request(s->sock, request);
 if (rc != -1 && iov) {
-ret = qemu_co_sendv(s->sock, iov, request->len, offset);
+ret = qemu_co_sendv(s->sock, iov, offset, request->len);
 if (ret != request->len) {
 errno = -EIO;
 rc = -1;
@@ -221,7 +221,7 @@ static void nbd_co_receive_reply(BDRVNBDState *s, struct 
nbd_request *request,
 reply->error = EIO;
 } else {
 if (iov && reply->error == 0) {
-ret = qemu_co_recvv(s->sock, iov, request->len, offset);
+ret = qemu_co_recvv(s->sock, iov, offset, request->len);
 if (ret != request->len) {
 reply->error = EIO;
 }
diff --git a/block/sheepdog.c b/block/sheepdog.c
index 00276f6f..fed436b 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -657,8 +657,8 @@ static void coroutine_fn aio_read_response(void *opaque)
 }
 break;
 case AIOCB_READ_UDATA:
-ret = qemu_co_recvv(fd, acb->qiov->iov, rsp.data_length,
-aio_req->iov_offset);
+ret = qemu_co_recvv(fd, acb->qiov->iov, aio_req->iov_offset,
+rsp.data_length);
 if (ret < 0) {
 error_report("failed to get the data, %s", strerror(errno));
 goto out;
@@ -924,7 +924,7 @@ static int coroutine_fn add_aio_request(BDRVSheepdogState 
*s, AIOReq *aio_req,
 }
 
 if (wlen) {
-ret = qemu_co_sendv(s->fd, iov, wlen, aio_req->iov_offset);
+ret = qemu_co_sendv(s->fd, iov, aio_req->iov_offset, wlen);
 if (ret < 0) {
 qemu_co_mutex_unlock(&s->lock);
 error_report("failed to send a data, %s", strerror(errno));
diff --git a/qemu-common.h b/qemu-common.h
index b8190e9..110ec06 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -306,31 +306,31 @@ struct qemu_work_item {
 void qemu_init_vcpu(void *env);
 #endif
 
-/**
- * Sends an iovec (or optionally a part of it) down a socket, yielding
- * when the socket is full.
- */
-int qemu_co_sendv(int sockfd, struct iovec *iov,
-  int len, int iov_offset);
-
-/**
- * Receives data into an iovec (or optionally into a part of it) from
- * a socket, yielding when there is no data in the socket.
- */
-int qemu_co_recvv(int sockfd, struct iovec *iov,
-  int len, int iov_offset);
-
 
 /**
- * Sends a buffer down a socket, yielding when the socket is full.
+ * Sends a (part of) iovec down a socket, yielding when the socket is full, or
+ * Receives data into a (part of) iovec from a socket,
+ * yielding when there is no data in the socket.
+ * The same interface as qemu_sendv_recvv(), with added yielding.
+ * XXX should mark these as coroutine_fn
  */
-int qemu_co_send(int sockfd, void *buf, int len);
+int qemu_co_sendv_recvv(int sockfd, struct iovec *iov,
+   size_t offset, size_t bytes, bool do_send);
+#define qemu_co_recvv(sockfd, iov, offset, bytes) \
+  qemu_co_sendv_recvv(sockfd, iov, offset, bytes, false)
+#define qemu_co_sendv(sockfd, iov, offset, bytes) \
+  qemu_co_sendv_recvv(sockfd, iov, offset, bytes, true)
 
 /**
- * Receives data into a buffer from a socket, yielding when there
- * is no data in the socket.
+ * The same as above, but with just a single buffer
  */
-int qemu_co_recv(int sockfd, void *buf, int len);
+int qemu_co_send_recv(int sockfd, void *buf, size_t bytes, bool do_send);
+#define qemu_co_recv(sockfd, buf, bytes) \
+  qemu_co_send_recv(sockfd, buf, bytes, false)
+static inline int qemu_co_send(int sockfd, const void *buf, size_t bytes)
+{
+return qemu_co_send_recv(sockfd, (void*)buf, bytes, true);
+}
 
 
 typedef struct QEMUIOVector {
diff --git a/qemu-coroutine-io.c b/qemu-coroutine-io.c
index df8ff21..75a4117 100644
--- a/qemu-coroutine-io.c
+++ b/qemu-coroutin

[Qemu-devel] [PATCHv2 4/7] change prototypes of qemu_sendv() and qemu_recvv()

Rename arguments and use size_t for sizes instead of int

-qemu_sendv(int sockfd, struct iovec *iov, int len, int iov_offset)
+qemu_sendv(int sockfd, struct iovec *iov, size_t offset, size_t bytes)

The main motivation was to make it clear that length and offset are
in _bytes_, not in iov elements: it was very confusing before, because
all standard functions which deals with iovecs expects number of iovs,
not bytes, even the fact that struct iovec has iov_len and iov_ prefix
does not help.  With "bytes" and "offset", especially since they're
now size_t, it is much more explicit.

All callers of qemu_sendv() and qemu_recvv() and related, like
qemu_co_sendv() and qemu_co_recvv(), were checked to verify that
it is safe to use unsigned datatype instead of int.

While at it, clean up misleading comment near do_sendv_recvv().

Note that the order of arguments is changed to: offset and bytes
(len and iov_offset) are swapped with each other.  This is to make
them consistent with very similar functions from qemu_iovec family,
where offset always follows qiov, to mean the place in it to start
from.

Signed-off-by: Michael Tokarev 
---
 cutils.c|   34 --
 qemu-common.h   |4 ++--
 qemu-coroutine-io.c |4 ++--
 3 files changed, 16 insertions(+), 26 deletions(-)

diff --git a/cutils.c b/cutils.c
index 04e51f7..b3b9cfb 100644
--- a/cutils.c
+++ b/cutils.c
@@ -417,38 +417,28 @@ int qemu_parse_fd(const char *param)
  *
  * This function send/recv data from/to the iovec buffer directly.
  * The first `offset' bytes in the iovec buffer are skipped and next
- * `len' bytes are used.
- *
- * For example,
- *
- *   do_sendv_recvv(sockfd, iov, len, offset, 1);
- *
- * is equal to
- *
- *   char *buf = malloc(size);
- *   iov_to_buf(iov, iovcnt, buf, offset, size);
- *   send(sockfd, buf, size, 0);
- *   free(buf);
+ * `bytes' bytes are used.
  */
-static int do_sendv_recvv(int sockfd, struct iovec *iov, int len, int offset,
+static int do_sendv_recvv(int sockfd, struct iovec *iov, size_t offset, size_t 
bytes,
   int do_sendv)
 {
-int ret, diff, iovlen;
+int ret, iovlen;
+size_t diff;
 struct iovec *last_iov;
 
 /* last_iov is inclusive, so count from one.  */
 iovlen = 1;
 last_iov = iov;
-len += offset;
+bytes += offset;
 
-while (last_iov->iov_len < len) {
-len -= last_iov->iov_len;
+while (last_iov->iov_len < bytes) {
+bytes -= last_iov->iov_len;
 
 last_iov++;
 iovlen++;
 }
 
-diff = last_iov->iov_len - len;
+diff = last_iov->iov_len - bytes;
 last_iov->iov_len -= diff;
 
 while (iov->iov_len <= offset) {
@@ -510,13 +500,13 @@ static int do_sendv_recvv(int sockfd, struct iovec *iov, 
int len, int offset,
 return ret;
 }
 
-int qemu_recvv(int sockfd, struct iovec *iov, int len, int iov_offset)
+int qemu_recvv(int sockfd, struct iovec *iov, size_t offset, size_t bytes)
 {
-return do_sendv_recvv(sockfd, iov, len, iov_offset, 0);
+return do_sendv_recvv(sockfd, iov, offset, bytes, 0);
 }
 
-int qemu_sendv(int sockfd, struct iovec *iov, int len, int iov_offset)
+int qemu_sendv(int sockfd, struct iovec *iov, size_t offset, size_t bytes)
 {
-return do_sendv_recvv(sockfd, iov, len, iov_offset, 1);
+return do_sendv_recvv(sockfd, iov, offset, bytes, 1);
 }
 
diff --git a/qemu-common.h b/qemu-common.h
index e8d1429..bb75dd3 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -199,8 +199,8 @@ int qemu_pipe(int pipefd[2]);
 #define qemu_recv(sockfd, buf, len, flags) recv(sockfd, buf, len, flags)
 #endif
 
-int qemu_recvv(int sockfd, struct iovec *iov, int len, int iov_offset);
-int qemu_sendv(int sockfd, struct iovec *iov, int len, int iov_offset);
+int qemu_recvv(int sockfd, struct iovec *iov, size_t offset, size_t bytes);
+int qemu_sendv(int sockfd, struct iovec *iov, size_t offset, size_t bytes);
 
 /* Error handling.  */
 
diff --git a/qemu-coroutine-io.c b/qemu-coroutine-io.c
index 40fd514..df8ff21 100644
--- a/qemu-coroutine-io.c
+++ b/qemu-coroutine-io.c
@@ -32,7 +32,7 @@ int coroutine_fn qemu_co_recvv(int sockfd, struct iovec *iov,
 int total = 0;
 int ret;
 while (len) {
-ret = qemu_recvv(sockfd, iov, len, iov_offset + total);
+ret = qemu_recvv(sockfd, iov, iov_offset + total, len);
 if (ret < 0) {
 if (errno == EAGAIN) {
 qemu_coroutine_yield();
@@ -58,7 +58,7 @@ int coroutine_fn qemu_co_sendv(int sockfd, struct iovec *iov,
 int total = 0;
 int ret;
 while (len) {
-ret = qemu_sendv(sockfd, iov, len, iov_offset + total);
+ret = qemu_sendv(sockfd, iov, iov_offset + total, len);
 if (ret < 0) {
 if (errno == EAGAIN) {
 qemu_coroutine_yield();
-- 
1.7.9.1

[Qemu-devel] [PATCHv2 0/7] cleanup/consolidate some iovec functions

This is a little cleanup/consolidation for some iovec-related
low-level routines in qemu.

The plan is to make library functions more understandable,
consistent and useful.

The patch changes prototypes of several iov and qiov functions
to match each other, changes types of arguments for some
functions, _swaps_ some function arguments with each other,
and makes use of common code in r/w path.

The result of all these changes.

1. Most qiov-related (qemu_iovec_*) functions now accepts
 'offset' parameter to specify from which (byte) position to
 start operation.  This is added for _memset (removing
 _memset_skip), _from_buffer (allowing to copy a bounce-
 buffer to a middle of qiov).  Typical:

  void qemu_iovec_memset(QEMUIOVector *qiov, size_t offset, int c, size_t 
bytes);

2. All functions that accepts this `offset' argument does
 it in a similar manner, following the
   iov,fromwhere,bytes
 pattern.  This is consistent with (updated) qemu_sendv()
 and qemu_recvv() and friends, where `offset' and `bytes'
 arguments were _renamed_, with the following prototypes:

   int qemu_sendv(sockfd, iov, size_t offset, size_t bytes)

 instead of

   int qemu_sendv(sockfd, iov, int len, int iov_offset)

 See how offset & bytes are used in the same way as for qemu_iovec_*
 A few callers of these are verified and converted.

3. Used size_t instead of various variations for byte counts.
 Including qemu_iovec_copy which used uint64_t(!) type.

4. Function arguments are renamed to better match with their
 actual meaning.  Compare new and original prototype of
 qemu_sendv() above: old prototype with `len' does not
 tell if `len' refers to number of iov elements (as
 regular writev() call) or to number of data bytes.
 Ditto for several usages of `count' for some qemu_iovec_*,
 which is also replaced to `bytes'.

The resulting function usage is much more consistent, the
functions themselves are nice and understandable, which
means they're easier to use and less error-prone.

This patchset also consolidates a few low-level send&recv
functions into one, since both versions were exactly
the same (and were finally calling common function anyway).
This is done by exporting a common send_recv function with
one extra bool argument, and making current send&recv to
be just #defines.

And while at it all, also made some implementations shorter,
cleaner and much easier to read/understand, and add some
code comments.

The read&write consolidation has great potential for the
block layer, as has been demonstrated before.
Unification and generalization of qemu_iovec_* functions
will let to optimize/simplify some more code in block/*,
especially qemu_iovec_memset() and _from_buffer() (this
optimization/simplification isn't done in this series)

Michael Tokarev (7):
  Consolidate qemu_iovec_memset{,_skip}() into single, simplified function
  allow qemu_iovec_from_buffer() to specify offset from which to start copying
  consolidate qemu_iovec_copy() and qemu_iovec_concat() and make them consistent
  change prototypes of qemu_sendv() and qemu_recvv()
  Export qemu_sendv_recvv() and use it in qemu_sendv() and qemu_recvv()
  cleanup qemu_co_sendv(), qemu_co_recvv() and friends
  rewrite and comment qemu_sendv_recvv()

 block.c |8 +-
 block/curl.c|4 +-
 block/nbd.c |4 +-
 block/qcow.c|2 +-
 block/qcow2.c   |   14 ++--
 block/qed.c |   10 +-
 block/sheepdog.c|6 +-
 block/vdi.c |2 +-
 cutils.c|  275 +--
 hw/9pfs/virtio-9p.c |8 +-
 linux-aio.c |4 +-
 posix-aio-compat.c  |2 +-
 qemu-common.h   |   64 +++-
 qemu-coroutine-io.c |   83 ---
 14 files changed, 205 insertions(+), 281 deletions(-)

-- 
1.7.9.1

[Qemu-devel] [PATCHv2 7/7] rewrite and comment qemu_sendv_recvv()

Make it much more understandable, and add comments to it.
This is in order to prepare similar function for writev_readv().

The new implementation has been extensively tested by
splitting a large buffer into many small randomly-sized
chunks, sending it over socket to another, slow process
and verifying the receiving data is the same.

Signed-off-by: Michael Tokarev 
---
 cutils.c  |  119 +---
 qemu-common.h |2 +-
 2 files changed, 63 insertions(+), 58 deletions(-)

diff --git a/cutils.c b/cutils.c
index b6f8eb8..3d1b2ea 100644
--- a/cutils.c
+++ b/cutils.c
@@ -419,83 +419,88 @@ int qemu_parse_fd(const char *param)
  * The first `offset' bytes in the iovec buffer are skipped and next
  * `bytes' bytes are used.
  */
-int qemu_sendv_recvv(int sockfd, struct iovec *iov, size_t offset, size_t 
bytes,
-bool do_sendv)
+int qemu_sendv_recvv(int sockfd, struct iovec *iov,
+size_t offset, size_t bytes, bool do_send)
 {
-int ret, iovlen;
-size_t diff;
-struct iovec *last_iov;
-
-/* last_iov is inclusive, so count from one.  */
-iovlen = 1;
-last_iov = iov;
-bytes += offset;
-
-while (last_iov->iov_len < bytes) {
-bytes -= last_iov->iov_len;
-
-last_iov++;
-iovlen++;
+int ret;
+struct iovec *end;
+
+/* Find the start position, skipping `offset' bytes:
+ * first, skip all full-sized vector elements, */
+while(offset >= iov->iov_len) {
+   offset -= iov->iov_len;
+   ++iov;
 }
-
-diff = last_iov->iov_len - bytes;
-last_iov->iov_len -= diff;
-
-while (iov->iov_len <= offset) {
-offset -= iov->iov_len;
-
-iov++;
-iovlen--;
+if (offset) {
+   /* second, skip `offset' bytes from the (now) first element,
+* undo it on exit */
+   iov->iov_base += offset;
+   iov->iov_len -= offset;
+}
+/* Find the end position skipping `bytes' bytes: */
+/* first, skip all full-sized elements */
+end = iov;
+while(end->iov_len <= bytes) {
+   bytes -= end->iov_len;
+   ++end;
+}
+if (bytes) {
+   /* second, fixup the last element, and remember
+* the length we've cut from the end of it in `bytes' */
+   size_t tail = end->iov_len - bytes;
+   end->iov_len = bytes;
+   ++end;
+   bytes = tail;
 }
-
-iov->iov_base = (char *) iov->iov_base + offset;
-iov->iov_len -= offset;
 
 {
 #if defined CONFIG_IOVEC && defined CONFIG_POSIX
 struct msghdr msg;
 memset(&msg, 0, sizeof(msg));
 msg.msg_iov = iov;
-msg.msg_iovlen = iovlen;
-
+msg.msg_iovlen = end - iov;;
 do {
-if (do_sendv) {
-ret = sendmsg(sockfd, &msg, 0);
-} else {
-ret = recvmsg(sockfd, &msg, 0);
-}
-} while (ret == -1 && errno == EINTR);
+   ret = do_send
+   ? sendmsg(sockfd, &msg, 0)
+: recvmsg(sockfd, &msg, 0);
+} while (ret < 0 && errno == EINTR);
 #else
-struct iovec *p = iov;
+   /* else send piece-by-piece */
+   /*XXX Note: windows has WSASend() and WSARecv() */
+   struct iovec *p = iov;
 ret = 0;
-while (iovlen > 0) {
-int rc;
-if (do_sendv) {
-rc = send(sockfd, p->iov_base, p->iov_len, 0);
-} else {
-rc = qemu_recv(sockfd, p->iov_base, p->iov_len, 0);
-}
-if (rc == -1) {
-if (errno == EINTR) {
-continue;
-}
+   while(p < end) {
+   int r = do_send
+   ? send(sockfd, p->iov_base, p->iov_len, 0)
+   : qemu_recv(sockfd, p->iov_base, p->iov_len, 0);
+   if (r > 0) {
+   ret += r;
+   ++i;
+   } else if (!r) {
+   break;
+   } else if (errno == EINTR) {
+   continue;
+   } else {
+   /* else it is some "other" error,
+* only return if there was no data processed. */
 if (ret == 0) {
 ret = -1;
 }
 break;
-}
-if (rc == 0) {
-break;
-}
-ret += rc;
-iovlen--, p++;
+   }
 }
 #endif
 }
 
 /* Undo the changes above */
-iov->iov_base = (char *) iov->iov_base - offset;
-iov->iov_len += offset;
-last_iov->iov_len += diff;
+if (offset) {
+   iov->iov_base -= offset;
+   iov->iov_len += offset;
+}
+if (bytes) {
+   --end;
+   end->iov_len += bytes;
+}
+
 return ret;
 }
diff --git a/qemu-common.h b/qemu-common.h
index 110ec06..2097fa4 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -206,7 +206,7 @@ int qemu_pipe(int pipefd[2]);
  * the beginning of iovec but at byte position

[Qemu-devel] [PATCHv2 2/7] allow qemu_iovec_from_buffer() to specify offset from which to start copying

Similar to
 qemu_iovec_memset(QEMUIOVector *qiov, size_t offset, int c, size_t bytes);
the new prototype is:
 qemu_iovec_from_buffer(QEMUIOVector *qiov, size_t offset, const void *buf, 
size_t bytes);

The processing starts at offset bytes within qiov.

This way, we may copy a bounce buffer directly to a middle of qiov.

While at it, add some asserts to ensure that buffer content fits
within qiov: offset + bytes <= size.
---
 block.c   |4 ++--
 block/curl.c  |4 ++--
 block/qcow.c  |2 +-
 block/qcow2.c |4 ++--
 block/vdi.c   |2 +-
 cutils.c  |   27 +--
 qemu-common.h |3 ++-
 7 files changed, 27 insertions(+), 19 deletions(-)

diff --git a/block.c b/block.c
index 52ffe14..cbbedfa 100644
--- a/block.c
+++ b/block.c
@@ -1692,7 +1692,7 @@ static int coroutine_fn 
bdrv_co_do_copy_on_readv(BlockDriverState *bs,
 }
 
 skip_bytes = (sector_num - cluster_sector_num) * BDRV_SECTOR_SIZE;
-qemu_iovec_from_buffer(qiov, bounce_buffer + skip_bytes,
+qemu_iovec_from_buffer(qiov, 0, bounce_buffer + skip_bytes,
nb_sectors * BDRV_SECTOR_SIZE);
 
 err:
@@ -3240,7 +3240,7 @@ static void bdrv_aio_bh_cb(void *opaque)
 BlockDriverAIOCBSync *acb = opaque;
 
 if (!acb->is_write)
-qemu_iovec_from_buffer(acb->qiov, acb->bounce, acb->qiov->size);
+qemu_iovec_from_buffer(acb->qiov, 0, acb->bounce, acb->qiov->size);
 qemu_vfree(acb->bounce);
 acb->common.cb(acb->common.opaque, acb->ret);
 qemu_bh_delete(acb->bh);
diff --git a/block/curl.c b/block/curl.c
index e9102e3..e772912 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -142,7 +142,7 @@ static size_t curl_read_cb(void *ptr, size_t size, size_t 
nmemb, void *opaque)
 continue;
 
 if ((s->buf_off >= acb->end)) {
-qemu_iovec_from_buffer(acb->qiov, s->orig_buf + acb->start,
+qemu_iovec_from_buffer(acb->qiov, 0, s->orig_buf + acb->start,
acb->end - acb->start);
 acb->common.cb(acb->common.opaque, 0);
 qemu_aio_release(acb);
@@ -178,7 +178,7 @@ static int curl_find_buf(BDRVCURLState *s, size_t start, 
size_t len,
 {
 char *buf = state->orig_buf + (start - state->buf_start);
 
-qemu_iovec_from_buffer(acb->qiov, buf, len);
+qemu_iovec_from_buffer(acb->qiov, 0, buf, len);
 acb->common.cb(acb->common.opaque, 0);
 
 return FIND_RET_OK;
diff --git a/block/qcow.c b/block/qcow.c
index b1cfe1f..f95b93f 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -540,7 +540,7 @@ done:
 qemu_co_mutex_unlock(&s->lock);
 
 if (qiov->niov > 1) {
-qemu_iovec_from_buffer(qiov, orig_buf, qiov->size);
+qemu_iovec_from_buffer(qiov, 0, orig_buf, qiov->size);
 qemu_vfree(orig_buf);
 }
 
diff --git a/block/qcow2.c b/block/qcow2.c
index 6d11bc0..668e2e2 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -475,7 +475,7 @@ static coroutine_fn int qcow2_co_readv(BlockDriverState 
*bs, int64_t sector_num,
 goto fail;
 }
 
-qemu_iovec_from_buffer(&hd_qiov,
+qemu_iovec_from_buffer(&hd_qiov, 0,
 s->cluster_cache + index_in_cluster * 512,
 512 * cur_nr_sectors);
 } else {
@@ -516,7 +516,7 @@ static coroutine_fn int qcow2_co_readv(BlockDriverState 
*bs, int64_t sector_num,
 qemu_iovec_reset(&hd_qiov);
 qemu_iovec_copy(&hd_qiov, qiov, bytes_done,
 cur_nr_sectors * 512);
-qemu_iovec_from_buffer(&hd_qiov, cluster_data,
+qemu_iovec_from_buffer(&hd_qiov, 0, cluster_data,
 512 * cur_nr_sectors);
 }
 }
diff --git a/block/vdi.c b/block/vdi.c
index 6a0011f..db0650e 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -635,7 +635,7 @@ static void vdi_aio_read_cb(void *opaque, int ret)
 return;
 done:
 if (acb->qiov->niov > 1) {
-qemu_iovec_from_buffer(acb->qiov, acb->orig_buf, acb->qiov->size);
+qemu_iovec_from_buffer(acb->qiov, 0, acb->orig_buf, acb->qiov->size);
 qemu_vfree(acb->orig_buf);
 }
 acb->common.cb(acb->common.opaque, ret);
diff --git a/cutils.c b/cutils.c
index 9451c86..5353f89 100644
--- a/cutils.c
+++ b/cutils.c
@@ -244,19 +244,26 @@ void qemu_iovec_to_buffer(QEMUIOVector *qiov, void *buf)
 }
 }
 
-void qemu_iovec_from_buffer(QEMUIOVector *qiov, const void *buf, size_t count)
+void qemu_iovec_from_buffer(QEMUIOVector *qiov, size_t offset,
+   const void *buf, size_t bytes)
 {
-const uint8_t *p = (const uint8_t *)buf;
-size_t copy;
+const struct iovec *iov = qiov->iov;
 int i;
+assert(qiov->size >= offset);
+assert(qiov->size - offset >= bytes);
 
-for (i = 0; i < qiov->niov && count; ++i) {
-copy = count;
-if (copy > qiov->iov[i].iov_len)
-copy = qiov->iov[i].iov_len

[Qemu-devel] [PATCHv2 1/7] Consolidate qemu_iovec_memset{, _skip}() into single, simplified function

This patch combines two functions into one, simplifies the
implementation and adds some assert()s into place.

The new prototype of qemu_iovec_memset():
  void qemu_iovec_memset(qiov, size_t offset, int c, size_t bytes)
It is different from former qemu_iovec_memset_skip(), and
I want to make other functions to be consistent with it too:
first how much to skip, second what, and 3rd how many of it.

Signed-off-by: Michael Tokarev 
---
 block/qcow2.c  |4 ++--
 block/qed.c|4 ++--
 cutils.c   |   50 ++
 linux-aio.c|4 ++--
 posix-aio-compat.c |2 +-
 qemu-common.h  |4 +---
 6 files changed, 22 insertions(+), 46 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index eb5ea48..6d11bc0 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -406,7 +406,7 @@ int qcow2_backing_read1(BlockDriverState *bs, QEMUIOVector 
*qiov,
 else
 n1 = bs->total_sectors - sector_num;
 
-qemu_iovec_memset_skip(qiov, 0, 512 * (nb_sectors - n1), 512 * n1);
+qemu_iovec_memset(qiov, 512 * n1, 0, 512 * (nb_sectors - n1));
 
 return n1;
 }
@@ -466,7 +466,7 @@ static coroutine_fn int qcow2_co_readv(BlockDriverState 
*bs, int64_t sector_num,
 }
 } else {
 /* Note: in this case, no need to wait */
-qemu_iovec_memset(&hd_qiov, 0, 512 * cur_nr_sectors);
+   qemu_iovec_memset(&hd_qiov, 0, 0, 512 * cur_nr_sectors);
 }
 } else if (cluster_offset & QCOW_OFLAG_COMPRESSED) {
 /* add AIO support for compressed blocks ? */
diff --git a/block/qed.c b/block/qed.c
index a041d31..6f9325b 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -738,7 +738,7 @@ static void qed_read_backing_file(BDRVQEDState *s, uint64_t 
pos,
 /* Zero all sectors if reading beyond the end of the backing file */
 if (pos >= backing_length ||
 pos + qiov->size > backing_length) {
-qemu_iovec_memset(qiov, 0, qiov->size);
+qemu_iovec_memset(qiov, 0, 0, qiov->size);
 }
 
 /* Complete now if there are no backing file sectors to read */
@@ -1253,7 +1253,7 @@ static void qed_aio_read_data(void *opaque, int ret,
 
 /* Handle zero cluster and backing file reads */
 if (ret == QED_CLUSTER_ZERO) {
-qemu_iovec_memset(&acb->cur_qiov, 0, acb->cur_qiov.size);
+qemu_iovec_memset(&acb->cur_qiov, 0, 0, acb->cur_qiov.size);
 qed_aio_next_io(acb, 0);
 return;
 } else if (ret != QED_CLUSTER_FOUND) {
diff --git a/cutils.c b/cutils.c
index af308cd..9451c86 100644
--- a/cutils.c
+++ b/cutils.c
@@ -260,46 +260,24 @@ void qemu_iovec_from_buffer(QEMUIOVector *qiov, const 
void *buf, size_t count)
 }
 }
 
-void qemu_iovec_memset(QEMUIOVector *qiov, int c, size_t count)
+void qemu_iovec_memset(QEMUIOVector *qiov, size_t offset, int c, size_t bytes)
 {
-size_t n;
+struct iovec *iov = qiov->iov;
 int i;
+assert(qiov->size >= offset);
+assert(qiov->size - offset >= bytes);
 
-for (i = 0; i < qiov->niov && count; ++i) {
-n = MIN(count, qiov->iov[i].iov_len);
-memset(qiov->iov[i].iov_base, c, n);
-count -= n;
+/* first skip initial full-sized elements */
+for(i = 0; offset >= iov[i].iov_len; ++i) {
+   offset -= iov[i].iov_len;
 }
-}
-
-void qemu_iovec_memset_skip(QEMUIOVector *qiov, int c, size_t count,
-size_t skip)
-{
-int i;
-size_t done;
-void *iov_base;
-uint64_t iov_len;
-
-done = 0;
-for (i = 0; (i < qiov->niov) && (done != count); i++) {
-if (skip >= qiov->iov[i].iov_len) {
-/* Skip the whole iov */
-skip -= qiov->iov[i].iov_len;
-continue;
-} else {
-/* Skip only part (or nothing) of the iov */
-iov_base = (uint8_t*) qiov->iov[i].iov_base + skip;
-iov_len = qiov->iov[i].iov_len - skip;
-skip = 0;
-}
-
-if (done + iov_len > count) {
-memset(iov_base, c, count - done);
-break;
-} else {
-memset(iov_base, c, iov_len);
-}
-done += iov_len;
+/* skip/memset partial element and memset the rest */
+while(bytes) {
+   size_t n = MIN(bytes, iov[i].iov_len - offset);
+   memset((char*)iov[i].iov_base + offset, c, n);
+   bytes -= n;
+   ++i;
+   offset = 0;
 }
 }
 
diff --git a/linux-aio.c b/linux-aio.c
index d2fc2e7..5a46c13 100644
--- a/linux-aio.c
+++ b/linux-aio.c
@@ -64,8 +64,8 @@ static void qemu_laio_process_completion(struct 
qemu_laio_state *s,
 } else if (ret >= 0) {
 /* Short reads mean EOF, pad with zeros. */
 if (laiocb->is_read) {
-qemu_iovec_memset_skip(laiocb->qiov, 0,
-laiocb->qiov->size - ret, ret);
+qemu_iovec_memset(laiocb->qiov, ret, 0,
+laiocb->qiov->size - ret);

[Qemu-devel] [PATCHv2 5/7] Export qemu_sendv_recvv() and use it in qemu_sendv() and qemu_recvv()

Rename do_sendv_recvv() to qemu_sendv_recvv(),
change its last arg (do_send) from int to bool,
export it in qemu-common.h, and made the two
callers of it (qemu_sendv() and qemu_recvv())
to be trivial #defines just adding 5th arg.

qemu_sendv_recvv() will be used later.

Signed-off-by: Michael Tokarev 
---
 cutils.c  |   15 ++-
 qemu-common.h |   14 --
 2 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/cutils.c b/cutils.c
index b3b9cfb..b6f8eb8 100644
--- a/cutils.c
+++ b/cutils.c
@@ -419,8 +419,8 @@ int qemu_parse_fd(const char *param)
  * The first `offset' bytes in the iovec buffer are skipped and next
  * `bytes' bytes are used.
  */
-static int do_sendv_recvv(int sockfd, struct iovec *iov, size_t offset, size_t 
bytes,
-  int do_sendv)
+int qemu_sendv_recvv(int sockfd, struct iovec *iov, size_t offset, size_t 
bytes,
+bool do_sendv)
 {
 int ret, iovlen;
 size_t diff;
@@ -499,14 +499,3 @@ static int do_sendv_recvv(int sockfd, struct iovec *iov, 
size_t offset, size_t b
 last_iov->iov_len += diff;
 return ret;
 }
-
-int qemu_recvv(int sockfd, struct iovec *iov, size_t offset, size_t bytes)
-{
-return do_sendv_recvv(sockfd, iov, offset, bytes, 0);
-}
-
-int qemu_sendv(int sockfd, struct iovec *iov, size_t offset, size_t bytes)
-{
-return do_sendv_recvv(sockfd, iov, offset, bytes, 1);
-}
-
diff --git a/qemu-common.h b/qemu-common.h
index bb75dd3..b8190e9 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -199,8 +199,18 @@ int qemu_pipe(int pipefd[2]);
 #define qemu_recv(sockfd, buf, len, flags) recv(sockfd, buf, len, flags)
 #endif
 
-int qemu_recvv(int sockfd, struct iovec *iov, size_t offset, size_t bytes);
-int qemu_sendv(int sockfd, struct iovec *iov, size_t offset, size_t bytes);
+/**
+ * Send or receive data from/to an (optionally partial) iovec.
+ * Instead of processing whole iovector, this routine can process
+ * not more than a specified number of bytes, and start not at
+ * the beginning of iovec but at byte position `offset'.
+ */
+int qemu_sendv_recvv(int sockfd, struct iovec *iov,
+size_t offset, size_t bytes, bool do_sendv);
+#define qemu_recvv(sockfd, iov, offset, bytes) \
+  qemu_sendv_recvv(sockfd, iov, offset, bytes, false)
+#define qemu_sendv(sockfd, iov, offset, bytes) \
+  qemu_sendv_recvv(sockfd, iov, offset, bytes, true)
 
 /* Error handling.  */
 
-- 
1.7.9.1

Re: [Qemu-devel] [PATCH 1/2] pseries: Add support for level interrupts to XICS

2012-03-10 Thread Alexander Graf


On 08.03.2012, at 02:12, David Gibson wrote:

> The pseries "xics" interrupt controller, like most interrupt
> controllers can support both message (i.e. edge sensitive) interrupts
> and level sensitive interrupts, but it needs to know which are which.
> 
> When I implemented the xics emulation for qemu, the only devices we
> supported were the PAPR virtual IO devices.  These devices only use
> message interrupts, so they were the only ones I implemented in xics.
> 
> Since then, however, we have added support for PCI devices, which use
> level sensitive interrupts.  It turns out the message interrupt logic
> still actually works most of the time for these, but there are
> circumstances where we can lost interrupts due to the incorrect
> interrupt logic.
> 
> This patch, therefore, implements the correct xics level-sensitive
> interrupt logic.  The type of the interrupt is set when a device
> allocates a new xics interrupt.
> 
> Signed-off-by: David Gibson 

Thanks, applied to ppc-next.


Alex

Re: [Qemu-devel] [PATCH 2/2] pseries: Configure PCI bridge using properties

2012-03-10 Thread Alexander Graf


On 08.03.2012, at 02:12, David Gibson wrote:

> Currently, the function spapr_create_phb() uses its parameters to
> initialize the correct memory windows for the new PCI Host Bridge
> (PHB).  This is not the way things are supposed to be done with qdevs,
> and means you can't create extra PHBs easily using -device.
> 
> Since pSeries machines can and do have many PHBs with various
> configurations, this is a real limitation, not just a theoretical.
> This patch, therefore, alters the PHB initialization code to use qdev
> properties to set these parameters of the new bridge, moving most of
> the code from spapr_create_phb() to spapr_phb_init().
> 
> While we're at it, we change the naming of each PCI bus and its
> associated memory regions to be less arbitrary and make it easier to
> relate the guest and qemu views of memory to each other.
> 
> Signed-off-by: Alexey Kardashevskiy 
> Signed-off-by: David Gibson 

Andreas, could you please (n)ack this version?


Alex

[Qemu-devel] [Bug 950692] Re: High CPU usage in Host (revisited)

2012-03-10 Thread PetaMem

*Newsflash*

We do have a "well-behaving" KVM Host with 3.2.9 kernel on machine C

After again numerous attempts to find the culprit, I decided to copy the
kernel 2.6.35 and modules from machine A to machine C, where it
exhibited also the desired "well-behaving".

I then simply copied its config to a 3.2.9 kernel and did "make
oldconfig", kept all defaults offered and restarted the machine with
that newly created 3.2.9 and it seems it got soem right genes from
2.6.35 config.

I will now poke the config and see if something breaks. Currently the
only significant difference to our unsuccessfull 3.2.9 kernel is the
fact, that the bad kernel was configured with kvm and kvm_intel not as
module but compiled in. Should that be the culprit... oh man...

I will test that and report.

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

Title:
  High CPU usage in Host (revisited)

Status in QEMU:
  New

Bug description:
  Hi,

  last time QEMU(KVM) was working for us flawlessly was 2.6.35 kernel.

  Actually it still works flawlessly on that one single machine, that
  still has this kernel. Qemu version is meanwhile 1.0-r3, so the
  problem seems to be dependent on kernel version and not qemu version.

  We have several other machines, where the "high CPU usage in host"
  problem is present in various degrees of annoyingness.

  Both host and guest are Gentoo linux, at least that's what we test
  with. Several tested systems with other linux distributions and
  FreeBSD show similar - if not worse - behaviour. I will talk about 3
  hosts, machine A, machine B and machine C

  A:

  2.6.35-gentoo-r9 #2 SMP Sat Nov 6 22:32:28 CET 2010 x86_64 Intel(R) Xeon(R) 
CPU L5410 @ 2.33GHz
  32GB, runs about 15 KVM guests (all Gentoo, some 32bit, some 64bit, all SMP)
  no problems whatsoever, host CPU usage corresponds to Guest CPU usage + 1-2%, 
that's how we like it
  qemu 1.0-r3

  B:

  3.0.6-gentoo #1 SMP Sun Oct 16 18:57:31 CEST 2011 x86_64 Intel(R) Xeon(R) CPU 
L5630 @ 2.13GHz
  144GB, runs 1(!) KVM guest (Debian 6.x)
  /usr/bin/qemu-system-x86_64 --enable-kvm -daemonize -cpu host -k de -net tap 
-tdf -hda /data/vm/disk.raw -m 768 -smp 1 -vnc :5 -net 
nic,model=e1000,macaddr=...
  100% host CPU load always, therefore it got only "smp 1", if we gave it smp 
2, it would have 200%, smp 4 400% and so on.
  qemu 1.0-r3

  C:

  3.1.6-gentoo #5 SMP Tue Mar 6 20:34:44 CET 2012 x86_64 Intel(R) Xeon(R) CPU 
5148 @ 2.33GHz
  16GB, runs 1-4 KVM guests (mostly Gentoo machines from A, plus some SuSE, 
RedHat etc.)
  X00% CPU usage, where x corresponds to the smp X parameter, at startup as 
well as if someone "touches" the VM, like logging in, doing a "ls". If the 
machine is ABSOLUTELY IDLE, the process also exhibits 1-2% CPU load in host, 
but as soon as you do a simple ls, usage goes to - say - 400%, where it remains 
for some seconds, then slowly falls 280%, 120%, 60%, ... back to 1-2%
  qemu 1.0-r3

  
  B is no go, C tries to well-behave but ultimatively fails, A is golden.

  There seems to be REAL high CPU usage and not only an error in
  displaying it. Other processes get less CPU power and exhibit
  definitely a slower runtime. On B, definitely one CPU core is hogged
  all the time

  
  Some years ago we experienced something similar with ~2.6.26 and after a long 
and woeful period, we found out that compiling the host kernel as a tickless 
system caused the problem. Enabling high resolution timers made the problem go 
away and that is the situation on machine A until today. Since then no one 
dared to touch this production server. Unfortunately, this recipe didn't help 
with the other machines.

  I have scanned the net for similar problems and there are people
  complaining about high CPU usage. Unfortunately very often the devs or
  maintainers cannot reproduce it and the issue is dropped. Well - we
  cannot reproduce a "good behaviour"(tm) on any but one machine with
  any recent (read: post-2.6.35) linux kernel.

  Summary what we tried so far:

  * different linux kernels @ host, and @ guest

  -> no difference, especially there are guests @ A, that run newer
  kernels and there are Guests at B and C that run older kernels than is
  the host kernel

  * smp and non-smp, 32bit and 64bit guests

  -> 32/64bit in the guest makes no difference whatsoever. The smp just
  limits how much of the host CPU the guest hogs on non-well behaving
  systems (smp X -> X * 100%)

  * various linux guest OS, as well as FreeBSD

  -> no difference whatsoever

  * various options parameters in the host kernel (other schedulers,
  HRT, tickless,...)

  -> no difference whatsoever

  * various versions of qemu/kvm since 0.13

  -> no difference whatsoever

  * various qemu/kvm options, virtio and non-virtio configurations (most
  of the VMs @ A run blk-virtio but emulate an e1000)

  -> no difference whatsoever

  
  You could say, we've reache

Re: [Qemu-devel] [PATCH] ds1338: Add missing break statement

Am 25.02.2012 14:50, schrieb Stefan Weil:
> Without the break statement, case 5 sets month and year from the same
> data. This does not look correct.
> 
> The missing break was reported by splint.
> 
> Signed-off-by: Stefan Weil 

Assigning data to both month and year certainly makes no sense,

Reviewed-by: Andreas Färber 

Andreas

> ---
>  hw/ds1338.c |1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/hw/ds1338.c b/hw/ds1338.c
> index 6397f0a..d590d9c 100644
> --- a/hw/ds1338.c
> +++ b/hw/ds1338.c
> @@ -100,6 +100,7 @@ static int ds1338_send(I2CSlave *i2c, uint8_t data)
>  break;
>  case 5:
>  s->now.tm_mon = from_bcd(data & 0x1f) - 1;
> +break;
>  case 6:
>  s->now.tm_year = from_bcd(data) + 100;
>  break;

-- 
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] slirp: Fix compiler warning for w64

Casting a pointer to an integer value must use uintptr_t or intptr_t
(not long) for portable code. MinGW-w64 requires this because
sizeof(long) != sizeof(void *) for w64 hosts, so casting to long
raises a compiler warning.

I use uintptr_t instead of intptr_t because changing the sign does not
matter here and casting pointers to unsigned values seems more
reasonable (the unsigned value is a non negative offset.

Cc: Jan Kiszka 
Signed-off-by: Stefan Weil 
---
 slirp/cksum.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/slirp/cksum.c b/slirp/cksum.c
index e43867d..6328660 100644
--- a/slirp/cksum.c
+++ b/slirp/cksum.c
@@ -75,7 +75,7 @@ int cksum(struct mbuf *m, int len)
/*
 * Force to even boundary.
 */
-   if ((1 & (long) w) && (mlen > 0)) {
+   if ((1 & (uintptr_t)w) && (mlen > 0)) {
REDUCE;
sum <<= 8;
s_util.c[0] = *(uint8_t *)w;
-- 
1.7.9

Re: [Qemu-devel] [PATCH] ds1338: Add missing break statement


Am 25.02.2012 14:50, schrieb Stefan Weil:

Without the break statement, case 5 sets month and year from the same
data. This does not look correct.

The missing break was reported by splint.

Signed-off-by: Stefan Weil 
---
hw/ds1338.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/hw/ds1338.c b/hw/ds1338.c
index 6397f0a..d590d9c 100644
--- a/hw/ds1338.c
+++ b/hw/ds1338.c
@@ -100,6 +100,7 @@ static int ds1338_send(I2CSlave *i2c, uint8_t data)
break;
case 5:
s->now.tm_mon = from_bcd(data & 0x1f) - 1;
+ break;
case 6:
s->now.tm_year = from_bcd(data) + 100;
break;


Ping. This is a simple bug fix. It was reviewed by Peter Maydell (thanks!).
Could someone please commit it to QEMU master (maybe to stable-1.0, too)?

Regards,
Stefan Weil

Re: [Qemu-devel] [PATCH 2/3] Redesign of pciinit.c

2012-03-10 Thread Kevin O'Connor

On Fri, Mar 09, 2012 at 07:51:06PM +1300, Alexey Korolev wrote:
> In this patch we introduce the pri_region_entry structure and
> significantly simplify the pci_bus structure. 
> Instead of arrays now we are using linked lists to account resources and
> assign memory bases. 
> 
> Since we modify the main structure of pciinit.c this is the minimum
> chunk of changes which could keep code workable. I mean further patch
> splitting is not possible as there are too many references to pci_bus
> members.

Thanks Alexey.  This is better than the previous patch, but I still
don't feel comfortable with the amount of change in a single patch.
I'm having a hard time reviewing the change.  I'd much prefer to see
one patch (or more) with just the data structure changes, and then one
patch (or more) with the change from the array based allocation system
to the list traversal allocation system.  (That is, it should be
possible to have all the linked lists in place while still using the
array system to do the allocations, and then in a separate patch
delete the arrays and convert over to list traversal.)

-Kevin

Re: [Qemu-devel] [PATCH] Add missing const attributes for MemoryRegionOps


Am 05.02.2012 21:19, schrieb Stefan Weil:

Most MemoryRegionOps already had the const attribute.
This patch adds it to the remaining ones.

Signed-off-by: Stefan Weil
---
  hw/cuda.c  |2 +-
  hw/ide/ahci.c  |4 ++--
  hw/ide/cmd646.c|6 +++---
  hw/ide/macio.c |2 +-
  hw/ide/piix.c  |2 +-
  hw/ide/via.c   |2 +-
  hw/mipsnet.c   |2 +-
  hw/opencores_eth.c |4 ++--
  hw/spapr_pci.c |2 +-
  9 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/hw/cuda.c b/hw/cuda.c
index 4077436..233ab66 100644
--- a/hw/cuda.c
+++ b/hw/cuda.c
@@ -634,7 +634,7 @@ static uint32_t cuda_readl (void *opaque, 
target_phys_addr_t addr)
  return 0;
  }

-static MemoryRegionOps cuda_ops = {
+static const MemoryRegionOps cuda_ops = {
  .old_mmio = {
  .write = {
  cuda_writeb,
diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 630d572..cc54590 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -365,7 +365,7 @@ static void ahci_mem_write(void *opaque, target_phys_addr_t 
addr,

  }

-static MemoryRegionOps ahci_mem_ops = {
+static const MemoryRegionOps ahci_mem_ops = {
  .read = ahci_mem_read,
  .write = ahci_mem_write,
  .endianness = DEVICE_LITTLE_ENDIAN,
@@ -401,7 +401,7 @@ static void ahci_idp_write(void *opaque, target_phys_addr_t 
addr,
  }
  }

-static MemoryRegionOps ahci_idp_ops = {
+static const MemoryRegionOps ahci_idp_ops = {
  .read = ahci_idp_read,
  .write = ahci_idp_write,
  .endianness = DEVICE_LITTLE_ENDIAN,
diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c
index d78ed69..0aac94a 100644
--- a/hw/ide/cmd646.c
+++ b/hw/ide/cmd646.c
@@ -65,7 +65,7 @@ static void cmd646_cmd_write(void *opaque, target_phys_addr_t 
addr,
  ide_cmd_write(cmd646bar->bus, addr + 2, data);
  }

-static MemoryRegionOps cmd646_cmd_ops = {
+static const MemoryRegionOps cmd646_cmd_ops = {
  .read = cmd646_cmd_read,
  .write = cmd646_cmd_write,
  .endianness = DEVICE_LITTLE_ENDIAN,
@@ -104,7 +104,7 @@ static void cmd646_data_write(void *opaque, 
target_phys_addr_t addr,
  }
  }

-static MemoryRegionOps cmd646_data_ops = {
+static const MemoryRegionOps cmd646_data_ops = {
  .read = cmd646_data_read,
  .write = cmd646_data_write,
  .endianness = DEVICE_LITTLE_ENDIAN,
@@ -193,7 +193,7 @@ static void bmdma_write(void *opaque, target_phys_addr_t 
addr,
  }
  }

-static MemoryRegionOps cmd646_bmdma_ops = {
+static const MemoryRegionOps cmd646_bmdma_ops = {
  .read = bmdma_read,
  .write = bmdma_write,
  };
diff --git a/hw/ide/macio.c b/hw/ide/macio.c
index a827d81..2c4027d 100644
--- a/hw/ide/macio.c
+++ b/hw/ide/macio.c
@@ -291,7 +291,7 @@ static uint32_t pmac_ide_readl (void 
*opaque,target_phys_addr_t addr)
  return retval;
  }

-static MemoryRegionOps pmac_ide_ops = {
+static const MemoryRegionOps pmac_ide_ops = {
  .old_mmio = {
  .write = {
  pmac_ide_writeb,
diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index a472bff..c524f55 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -79,7 +79,7 @@ static void bmdma_write(void *opaque, target_phys_addr_t addr,
  }
  }

-static MemoryRegionOps piix_bmdma_ops = {
+static const MemoryRegionOps piix_bmdma_ops = {
  .read = bmdma_read,
  .write = bmdma_write,
  };
diff --git a/hw/ide/via.c b/hw/ide/via.c
index 2771f0c..ad6f302 100644
--- a/hw/ide/via.c
+++ b/hw/ide/via.c
@@ -82,7 +82,7 @@ static void bmdma_write(void *opaque, target_phys_addr_t addr,
  }
  }

-static MemoryRegionOps via_bmdma_ops = {
+static const MemoryRegionOps via_bmdma_ops = {
  .read = bmdma_read,
  .write = bmdma_write,
  };
diff --git a/hw/mipsnet.c b/hw/mipsnet.c
index a0e6c9f..1b49a79 100644
--- a/hw/mipsnet.c
+++ b/hw/mipsnet.c
@@ -224,7 +224,7 @@ static NetClientInfo net_mipsnet_info = {
  .cleanup = mipsnet_cleanup,
  };

-static MemoryRegionOps mipsnet_ioport_ops = {
+static const MemoryRegionOps mipsnet_ioport_ops = {
  .read = mipsnet_ioport_read,
  .write = mipsnet_ioport_write,
  .impl.min_access_size = 1,
diff --git a/hw/opencores_eth.c b/hw/opencores_eth.c
index 09f2757..6f3f5fc 100644
--- a/hw/opencores_eth.c
+++ b/hw/opencores_eth.c
@@ -692,12 +692,12 @@ static void open_eth_desc_write(void *opaque,
  }


-static MemoryRegionOps open_eth_reg_ops = {
+static const MemoryRegionOps open_eth_reg_ops = {
  .read = open_eth_reg_read,
  .write = open_eth_reg_write,
  };

-static MemoryRegionOps open_eth_desc_ops = {
+static const MemoryRegionOps open_eth_desc_ops = {
  .read = open_eth_desc_read,
  .write = open_eth_desc_write,
  };
diff --git a/hw/spapr_pci.c b/hw/spapr_pci.c
index ed2e4b3..3c08d57 100644
--- a/hw/spapr_pci.c
+++ b/hw/spapr_pci.c
@@ -281,7 +281,7 @@ static void spapr_io_write(void *opaque, target_phys_addr_t 
addr,
  assert(0);
  }

-static MemoryRegionOps spapr_io_ops = {
+static const MemoryRegionOps spapr_io_ops = {
  .endianness = DEVICE_LITTLE_ENDIAN,

[Qemu-devel] [PATCH] tcg: Improve tcg_out_label and fix its usage for w64

tcg_out_label is always called with a third argument of pointer type
which was casted to tcg_target_long.

These casts can be avoided by changing the prototype of tcg_out_label.

There was also a cast to long. For most hosts with
sizeof(long) == sizeof(tcg_target_long) == sizeof(void *) this did not
matter, but for w64 it was wrong. This is fixed now.

Cc: Blue Swirl 
Cc: Richard Henderson 
Signed-off-by: Stefan Weil 
---
 tcg/hppa/tcg-target.c  |8 
 tcg/i386/tcg-target.c  |8 
 tcg/sparc/tcg-target.c |6 +++---
 tcg/tcg.c  |6 +++---
 4 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/tcg/hppa/tcg-target.c b/tcg/hppa/tcg-target.c
index 59d4d12..71f4a8a 100644
--- a/tcg/hppa/tcg-target.c
+++ b/tcg/hppa/tcg-target.c
@@ -1052,7 +1052,7 @@ static void tcg_out_qemu_ld(TCGContext *s, const TCGArg 
*args, int opc)
 
 /* TLB Miss.  */
 /* label1: */
-tcg_out_label(s, lab1, (tcg_target_long)s->code_ptr);
+tcg_out_label(s, lab1, s->code_ptr);
 
 argreg = TCG_REG_R26;
 tcg_out_mov(s, TCG_TYPE_I32, argreg--, addrlo_reg);
@@ -1089,7 +1089,7 @@ static void tcg_out_qemu_ld(TCGContext *s, const TCGArg 
*args, int opc)
 }
 
 /* label2: */
-tcg_out_label(s, lab2, (tcg_target_long)s->code_ptr);
+tcg_out_label(s, lab2, s->code_ptr);
 #else
 tcg_out_qemu_ld_direct(s, datalo_reg, datahi_reg, addrlo_reg,
(GUEST_BASE ? TCG_GUEST_BASE_REG : TCG_REG_R0), 
opc);
@@ -1171,7 +1171,7 @@ static void tcg_out_qemu_st(TCGContext *s, const TCGArg 
*args, int opc)
 
 /* TLB Miss.  */
 /* label1: */
-tcg_out_label(s, lab1, (tcg_target_long)s->code_ptr);
+tcg_out_label(s, lab1, s->code_ptr);
 
 argreg = TCG_REG_R26;
 tcg_out_mov(s, TCG_TYPE_I32, argreg--, addrlo_reg);
@@ -1215,7 +1215,7 @@ static void tcg_out_qemu_st(TCGContext *s, const TCGArg 
*args, int opc)
 tcg_out_call(s, qemu_st_helpers[opc]);
 
 /* label2: */
-tcg_out_label(s, lab2, (tcg_target_long)s->code_ptr);
+tcg_out_label(s, lab2, s->code_ptr);
 #else
 /* There are no indexed stores, so if GUEST_BASE is set we must do the add
explicitly.  Careful to avoid R20, which is used for the bswaps to 
follow.  */
diff --git a/tcg/i386/tcg-target.c b/tcg/i386/tcg-target.c
index dc81572..1dbe240 100644
--- a/tcg/i386/tcg-target.c
+++ b/tcg/i386/tcg-target.c
@@ -875,7 +875,7 @@ static void tcg_out_brcond2(TCGContext *s, const TCGArg 
*args,
 default:
 tcg_abort();
 }
-tcg_out_label(s, label_next, (tcg_target_long)s->code_ptr);
+tcg_out_label(s, label_next, s->code_ptr);
 }
 #endif
 
@@ -917,10 +917,10 @@ static void tcg_out_setcond2(TCGContext *s, const TCGArg 
*args,
 
 tcg_out_movi(s, TCG_TYPE_I32, args[0], 0);
 tcg_out_jxx(s, JCC_JMP, label_over, 1);
-tcg_out_label(s, label_true, (tcg_target_long)s->code_ptr);
+tcg_out_label(s, label_true, s->code_ptr);
 
 tcg_out_movi(s, TCG_TYPE_I32, args[0], 1);
-tcg_out_label(s, label_over, (tcg_target_long)s->code_ptr);
+tcg_out_label(s, label_over, s->code_ptr);
 } else {
 /* When the destination does not overlap one of the arguments,
clear the destination first, jump if cond false, and emit an
@@ -934,7 +934,7 @@ static void tcg_out_setcond2(TCGContext *s, const TCGArg 
*args,
 tcg_out_brcond2(s, new_args, const_args+1, 1);
 
 tgen_arithi(s, ARITH_ADD, args[0], 1, 0);
-tcg_out_label(s, label_over, (tcg_target_long)s->code_ptr);
+tcg_out_label(s, label_over, s->code_ptr);
 }
 }
 #endif
diff --git a/tcg/sparc/tcg-target.c b/tcg/sparc/tcg-target.c
index 5cd5a3b..4461fb4 100644
--- a/tcg/sparc/tcg-target.c
+++ b/tcg/sparc/tcg-target.c
@@ -582,7 +582,7 @@ static void tcg_out_brcond2_i32(TCGContext *s, TCGCond cond,
 }
 tcg_out_nop(s);
 
-tcg_out_label(s, label_next, (tcg_target_long)s->code_ptr);
+tcg_out_label(s, label_next, s->code_ptr);
 }
 #endif
 
@@ -628,7 +628,7 @@ static void tcg_out_setcond_i32(TCGContext *s, TCGCond 
cond, TCGArg ret,
 tcg_out_branch_i32(s, INSN_COND(tcg_cond_to_bcond[cond], 1), t);
 tcg_out_movi_imm13(s, ret, 1);
 tcg_out_movi_imm13(s, ret, 0);
-tcg_out_label(s, t, (tcg_target_long)s->code_ptr);
+tcg_out_label(s, t, s->code_ptr);
 #endif
 return;
 }
@@ -683,7 +683,7 @@ static void tcg_out_setcond2_i32(TCGContext *s, TCGCond 
cond, TCGArg ret,
 
 tcg_out_setcond_i32(s, tcg_unsigned_cond(cond), ret, al, bl, blconst);
 
-tcg_out_label(s, lab, (tcg_target_long)s->code_ptr);
+tcg_out_label(s, lab, s->code_ptr);
 break;
 }
 }
diff --git a/tcg/tcg.c b/tcg/tcg.c
index cd2db3c..531db55 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -128,11 +128,11 @@ static void tcg_out_reloc(TCGContext *s, uint8_t 
*code_ptr, int type,
 }
 }
 
-static void tcg_out_label(TCGContext *s, int label_index, 
-  tcg_ta

Re: [Qemu-devel] [PATCH] configure: Test for libiberty.a (mingw32)


Am 10.03.2012 17:28, schrieb Andreas Färber:

Am 10.03.2012 11:14, schrieb Stefan Weil:

MinGW-w64 and some versions of MinGW32 don't provide libiberty.a,
so add this library only if it was found.


What do we need it for at all then?

Andreas


Signed-off-by: Stefan Weil 
---
configure | 8 +++-
1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/configure b/configure
index ca25250..bb16498 100755
--- a/configure
+++ b/configure
@@ -511,7 +511,13 @@ if test "$mingw32" = "yes" ; then
QEMU_CFLAGS="-DWIN32_LEAN_AND_MEAN -DWINVER=0x501 $QEMU_CFLAGS"
# enable C99/POSIX format strings (needs mingw32-runtime 3.15 or later)
QEMU_CFLAGS="-D__USE_MINGW_ANSI_STDIO=1 $QEMU_CFLAGS"
- LIBS="-lwinmm -lws2_32 -liberty -liphlpapi $LIBS"
+ LIBS="-lwinmm -lws2_32 -liphlpapi $LIBS"
+cat > $TMPC << EOF
+int main(void) { return 0; }
+EOF
+ if compile_prog "" "-liberty" ; then
+ LIBS="-liberty $LIBS"
+ fi
prefix="c:/Program Files/Qemu"
mandir="\${prefix}"
datadir="\${prefix}"


See commit 08f3896a072c6d05e36ec3fa4fd303ea550ee38f:

Author: Stefan Weil 
Date:   Fri Feb 4 22:38:47 2011 +0100

w32: Use additional library libiberty.a

libiberty.a is part of MinGW and provides useful functions
like ffs (MinGW) and getopt (MinGW-w64).

It is needed for w64 compilations and allows simpler code for w32.

Cc: Anthony Liguori 
Signed-off-by: Stefan Weil 
Signed-off-by: Blue Swirl 



It looks like newer versions of MinGW-w64 don't have libiberty.a
while older ones (Debian squeeze) provide it. For those versions
which have libiberty.a, it is needed because it includes ffs, strncmp
and several other functions used by QEMU code.

Stefan

[Qemu-devel] [PATCH RFC v4 16/20] target-arm: Store CCSIDRs in ARMCPUClass

Signed-off-by: Andreas Färber 
Cc: Peter Maydell 
---
 target-arm/cpu-qom.h |1 +
 target-arm/cpu.c |   17 +
 target-arm/helper.c  |   14 --
 3 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
index 101cdb1..cf79ed6 100644
--- a/target-arm/cpu-qom.h
+++ b/target-arm/cpu-qom.h
@@ -49,6 +49,7 @@ typedef struct ARMCPUClass {
 struct {
 uint32_t c0_cpuid;
 uint32_t c0_cachetype;
+uint32_t c0_ccsid[16];
 uint32_t c0_clid;
 uint32_t c0_c1[8];
 uint32_t c0_c2[8];
diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index 1a06c52..9f571a1 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -47,6 +47,7 @@ static void arm_cpu_reset(CPUState *c)
 
 /* TODO Move these into arm_cpu_initfn() once no longer zeroed above. */
 env->cp15.c0_cachetype = klass->cp15.c0_cachetype;
+memcpy(env->cp15.c0_ccsid, klass->cp15.c0_ccsid, 16 * sizeof(uint32_t));
 env->cp15.c0_clid = klass->cp15.c0_clid;
 memcpy(env->cp15.c0_c1, klass->cp15.c0_c1, 8 * sizeof(uint32_t));
 memcpy(env->cp15.c0_c2, klass->cp15.c0_c2, 8 * sizeof(uint32_t));
@@ -178,6 +179,7 @@ typedef struct ARMCPUInfo {
 const char *name;
 uint32_t id;
 uint32_t cp15_c0_cachetype;
+uint32_t cp15_c0_ccsid[16];
 uint32_t cp15_c0_clid;
 uint32_t cp15_c0_c1[8];
 uint32_t cp15_c0_c2[8];
@@ -366,6 +368,11 @@ static const ARMCPUInfo arm_cpus[] = {
 .name = "cortex-a8",
 .id = 0x410fc080,
 .cp15_c0_cachetype = 0x82048004,
+.cp15_c0_ccsid = {
+0xe007e01a, /* 16k L1 dcache. */
+0x2007e01a, /* 16k L1 icache. */
+0xf000, /* No L2 icache. */
+},
 .cp15_c0_clid = (1 << 27) | (2 << 24) | 3,
 .cp15_c0_c1 = {
 0x1031, 0x11, 0x400, 0,
@@ -387,6 +394,10 @@ static const ARMCPUInfo arm_cpus[] = {
 .name = "cortex-a9",
 .id = 0x410fc090,
 .cp15_c0_cachetype = 0x80038003,
+.cp15_c0_ccsid = {
+0xe00fe015, /* 16k L1 dcache. */
+0x200fe015, /* 16k L1 icache. */
+},
 .cp15_c0_clid = (1 << 27) | (1 << 24) | 3,
 .cp15_c0_c1 = {
 0x1031, 0x11, 0x000, 0,
@@ -414,6 +425,11 @@ static const ARMCPUInfo arm_cpus[] = {
 .name = "cortex-a15",
 .id = 0x412fc0f1,
 .cp15_c0_cachetype = 0x8444c004,
+.cp15_c0_ccsid = {
+0x701fe00a, /* 32K L1 dcache */
+0x201fe00a, /* 32K L1 icache */
+0x711fe07a, /* 4096K L2 unified cache */
+},
 .cp15_c0_clid = 0x0a200023,
 .cp15_c0_c1 = {
 0x1131, 0x00011011, 0x02010555, 0x,
@@ -547,6 +563,7 @@ static void arm_cpu_class_init(ObjectClass *klass, void 
*data)
 
 k->cp15.c0_cpuid = info->id;
 k->cp15.c0_cachetype = info->cp15_c0_cachetype;
+memcpy(k->cp15.c0_ccsid, info->cp15_c0_ccsid, 16 * sizeof(uint32_t));
 k->cp15.c0_clid = info->cp15_c0_clid;
 memcpy(k->cp15.c0_c1, info->cp15_c0_c1, 8 * sizeof(uint32_t));
 memcpy(k->cp15.c0_c2, info->cp15_c0_c2, 8 * sizeof(uint32_t));
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 1e3576c..ba8a4ae 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -8,20 +8,6 @@
 static void cpu_reset_model_id(CPUARMState *env, uint32_t id)
 {
 switch (id) {
-case ARM_CPUID_CORTEXA8:
-env->cp15.c0_ccsid[0] = 0xe007e01a; /* 16k L1 dcache. */
-env->cp15.c0_ccsid[1] = 0x2007e01a; /* 16k L1 icache. */
-env->cp15.c0_ccsid[2] = 0xf000; /* No L2 icache. */
-break;
-case ARM_CPUID_CORTEXA9:
-env->cp15.c0_ccsid[0] = 0xe00fe015; /* 16k L1 dcache. */
-env->cp15.c0_ccsid[1] = 0x200fe015; /* 16k L1 icache. */
-break;
-case ARM_CPUID_CORTEXA15:
-env->cp15.c0_ccsid[0] = 0x701fe00a; /* 32K L1 dcache */
-env->cp15.c0_ccsid[1] = 0x201fe00a; /* 32K L1 icache */
-env->cp15.c0_ccsid[2] = 0x711fe07a; /* 4096K L2 unified cache */
-break;
 case ARM_CPUID_TI915T:
 case ARM_CPUID_TI925T:
 env->cp15.c15_i_max = 0x000;
-- 
1.7.7

[Qemu-devel] [PATCH RFC v4 00/20] QOM'ify ARM CPU

Hello,

Since the ARM target lately has been the target with the highest rate of 
changes,
let's convert it first, as a proof of concept. Especially its reset handler can
be greatly simplified through QOM.

Patch 1 is a RESEND of object_class_get_list() v2. Needed for virtually every 
target.

Patch 2 moves the CPUID lookup to ARMCPUClass.
Patch 3 integrates legacy CPUARMState into ARMCPU.

Patches 5-17 step by step move CPUID-dependent initializations to CPU-specific 
code.
Patch 6 is a proof of concept for CPUState -> CPU lookup.

(NEW:)
Patch 18 adds CPU properties to manipulate parts of the CPUID (MIDR).
Patch 19 uses them to drop CPU classes that differ only by variant and revision.
Patch 20 adds a "/cpu" property for -M integratorcp (a single-CPU board for 
which
we have a test image on qemu.org) to allow inspection of CPU properties over 
QMP.

Available at:
http://repo.or.cz/w/qemu/afaerber.git/shortlog/refs/heads/qom-cpu-arm.v4

Regards,
Andreas

Cc: Anthony Liguori 
Cc: Peter Maydell 
Cc: Paul Brook 
Cc: Andrzej Zaborowski 

v3 -> v4:
* Rebased on top of type_init() v2, object_class_get_list() v2, qom-cpu v4.

* Rename cpu-core.h to cpu-qom.h. While the term "ARM core" is quite common,
  it is less so for other architectures like s390x so use a neutral term.
* Use container_of() for CPUState -> CPU macros (suggested by Anthony).
* Rework arm_env_get_object() -> arm_env_get_cpu(), avoids some casts
  (suggested by Anthony). Also rename ENV_GET_OBJECT() -> ENV_GET_CPU().
* Sort -cpu ? list.
* Use object_class_get_list() and sort ourselves rather than using
  object_class_foreach_ordered() with callbacks (suggested by Anthony).
* Drop ARMCPUClass jtag_id since it turned out unneeded in QEMU (Peter+Andrzej).

* Drop experimental "halted" property since that should be in common code.
* Introduce "cpuid-variant" and "cpuid-revision" properties.
* Use CPU properties to drop unneeded pxa270-* classes.
* Move "/cpu" child property to integratorcp machine.

v2 -> v3:
* Rebased against qom-upstream.14 branch (and that against master).

* Rename target-arm/cpu-core.c to cpu.c now that we no longer need VPATH.
* Leave cpu-core.h as is to separate from legacy cpu.h.
* Fix -cpu alias "pxa270": handled in cpu_arm_init().
* Use proper GPL headers.

* Start removing CPUID uses in cpu_reset_model_id() and cpu.h.
* Fully convert cpu_reset_model_id() to ARMCPUInfo or per-model code.

* Experiment with adding properties ("halted").
* For testing, add a "/cpu" child property (HACK).

v1 -> v2:

* Cherry-pick Anthony's object_class_foreach() patch.

* Fix ARMCPUClass type name (arm-cpu-core -> arm-cpu).
* Add documentation.
* Rename ARMCPUDef to ARMCPUInfo.
* Use a C99-style table for initializing the classes through class_data
  instead of individual class_init functions (suggested by Anthony).
* Prepare reset callback.

* Make ENV_GET_OBJECT() use an inline function for readability.
* Invoke the CPU's reset method from cpu_reset().

* Do feature initialization via table where sensible.
* Add feature flags to ARMCPU as well (suggested by PMM for future tweaking,
  also simplifies load/save a bit) and initialize them from ARMCPUClass.
* Make feature inference work for ARMCPU as well by not passing the ARMCPUClass.
  Use function-local macros to avoid the ugliness of deferencing the features 
pointer.

Andreas Färber (20):
  qom: Introduce object_class_get_list()
  target-arm: Introduce QOM ARMCPUClass
  target-arm: Embed CPUARMState in QOM ARMCPU
  target-arm: Prepare model-specific class_init function
  target-arm: Overwrite reset handler for ti925t
  target-arm: Move CPU feature flags out of CPUState
  target-arm: No longer abort on unhandled CPUIDs on reset
  target-arm: Store cp15 c0_c1 and c0_c2 in ARMCPUClass
  target-arm: Store CTR in ARMCPUClass
  target-arm: Store SCTLR in ARMCPUClass
  target-arm: Drop JTAG_ID documentation
  target-arm: Move the PXA270's iwMMXt reset to pxa270_reset()
  target-arm: Store VFP FPSID register in ARMCPUClass
  target-arm: Store VFP MVFR0 and MVFR1 in ARMCPUClass
  target-arm: Store CLIDR in ARMCPUClass
  target-arm: Store CCSIDRs in ARMCPUClass
  target-arm: Kill off cpu_reset_model_id()
  target-arm: Add cpuid-{variant,revision} properties to CPU
  target-arm: Simplify pxa270 CPU classes
  hw/integratorcp: Add child property for CPU

 Makefile.target   |1 +
 hw/integratorcp.c |3 +
 include/qemu/object.h |   11 +
 qom/object.c  |   17 ++
 target-arm/cpu-qom.h  |   98 
 target-arm/cpu.c  |  643 +
 target-arm/cpu.h  |   27 +--
 target-arm/helper.c   |  471 ++--
 target-arm/machine.c  |6 +-
 9 files changed, 851 insertions(+), 426 deletions(-)
 create mode 100644 target-arm/cpu-qom.h
 create mode 100644 target-arm/cpu.c

-- 
1.7.7

[Qemu-devel] [PATCH RFC v4 10/20] target-arm: Store SCTLR in ARMCPUClass

Signed-off-by: Andreas Färber 
Cc: Peter Maydell 
---
 target-arm/cpu-qom.h |1 +
 target-arm/cpu.c |   19 +++
 target-arm/cpu.h |3 ---
 target-arm/helper.c  |   17 -
 4 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
index 722d164..3832986 100644
--- a/target-arm/cpu-qom.h
+++ b/target-arm/cpu-qom.h
@@ -51,6 +51,7 @@ typedef struct ARMCPUClass {
 uint32_t c0_cachetype;
 uint32_t c0_c1[8];
 uint32_t c0_c2[8];
+uint32_t c1_sys;
 } cp15;
 
 uint32_t features;
diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index c7cf46d..2f3190a 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -50,6 +50,9 @@ static void arm_cpu_reset(CPUState *c)
 memcpy(env->cp15.c0_c1, klass->cp15.c0_c1, 8 * sizeof(uint32_t));
 memcpy(env->cp15.c0_c2, klass->cp15.c0_c2, 8 * sizeof(uint32_t));
 
+/* Genuine reset */
+env->cp15.c1_sys = klass->cp15.c1_sys;
+
 #if defined(CONFIG_USER_ONLY)
 env->uncached_cpsr = ARM_CPU_MODE_USR;
 /* For user mode we must enable access to coprocessors */
@@ -173,6 +176,7 @@ typedef struct ARMCPUInfo {
 uint32_t cp15_c0_cachetype;
 uint32_t cp15_c0_c1[8];
 uint32_t cp15_c0_c2[8];
+uint32_t cp15_c1_sys;
 uint32_t features;
 void (*class_init)(ARMCPUClass *klass, const struct ARMCPUInfo *info);
 } ARMCPUInfo;
@@ -193,6 +197,7 @@ static void arm1136_r0_class_init(ARMCPUClass *k, const 
ARMCPUInfo *info)
 k->cp15.c0_cachetype = r1_class->cp15.c0_cachetype;
 memcpy(k->cp15.c0_c1, r1_class->cp15.c0_c1, 8 * sizeof(uint32_t));
 memcpy(k->cp15.c0_c2, r1_class->cp15.c0_c2, 8 * sizeof(uint32_t));
+k->cp15.c1_sys = r1_class->cp15.c1_sys;
 }
 
 static void ti925t_reset(CPUState *c)
@@ -214,12 +219,15 @@ static void ti925t_class_init(ARMCPUClass *klass, const 
ARMCPUInfo *info)
 
 static void sa11xx_class_init(ARMCPUClass *k, const ARMCPUInfo *info)
 {
+k->cp15.c1_sys = 0x0070;
+
 set_class_feature(k, ARM_FEATURE_STRONGARM);
 }
 
 static void pxa25x_class_init(ARMCPUClass *k, const ARMCPUInfo *info)
 {
 k->cp15.c0_cachetype = 0xd172172;
+k->cp15.c1_sys = 0x0078;
 
 set_class_feature(k, ARM_FEATURE_V5);
 set_class_feature(k, ARM_FEATURE_XSCALE);
@@ -228,6 +236,7 @@ static void pxa25x_class_init(ARMCPUClass *k, const 
ARMCPUInfo *info)
 static void pxa270_class_init(ARMCPUClass *k, const ARMCPUInfo *info)
 {
 k->cp15.c0_cachetype = 0xd172172;
+k->cp15.c1_sys = 0x0078;
 
 set_class_feature(k, ARM_FEATURE_V5);
 set_class_feature(k, ARM_FEATURE_XSCALE);
@@ -239,6 +248,7 @@ static const ARMCPUInfo arm_cpus[] = {
 .name = "arm926",
 .id = 0x41069265,
 .cp15_c0_cachetype = 0x1dd20d2,
+.cp15_c1_sys = 0x00090078,
 .features = ARM_FEATURE(V5) |
 ARM_FEATURE(VFP),
 },
@@ -246,6 +256,7 @@ static const ARMCPUInfo arm_cpus[] = {
 .name = "arm946",
 .id = 0x41059461,
 .cp15_c0_cachetype = 0x0f004006,
+.cp15_c1_sys = 0x0078,
 .features = ARM_FEATURE(V5) |
 ARM_FEATURE(MPU),
 },
@@ -253,6 +264,7 @@ static const ARMCPUInfo arm_cpus[] = {
 .name = "arm1026",
 .id = 0x4106a262,
 .cp15_c0_cachetype = 0x1dd20d2,
+.cp15_c1_sys = 0x00090078,
 .features = ARM_FEATURE(V5) |
 ARM_FEATURE(VFP) |
 ARM_FEATURE(AUXCR),
@@ -278,6 +290,7 @@ static const ARMCPUInfo arm_cpus[] = {
 0x00140011, 0x12002111, 0x1123, 0x01102131,
 0x141, 0, 0, 0
 },
+.cp15_c1_sys = 0x00050078,
 .features = ARM_FEATURE(V6) |
 ARM_FEATURE(VFP),
 },
@@ -293,6 +306,7 @@ static const ARMCPUInfo arm_cpus[] = {
 0x0140011, 0x12002111, 0x11231121, 0x01102131,
 0x01141, 0, 0, 0
 },
+.cp15_c1_sys = 0x00050078,
 .features = ARM_FEATURE(V6K) |
 ARM_FEATURE(VFP) |
 ARM_FEATURE(VAPA),
@@ -331,6 +345,7 @@ static const ARMCPUInfo arm_cpus[] = {
 0x0010, 0x12112111, 0x21232031, 0x2131,
 0x0042, 0, 0, 0
 },
+.cp15_c1_sys = 0x00c50078,
 .features = ARM_FEATURE(V7) |
 ARM_FEATURE(VFP3) |
 ARM_FEATURE(NEON) |
@@ -348,6 +363,7 @@ static const ARMCPUInfo arm_cpus[] = {
 0x0010, 0x13112111, 0x21232041, 0x2131,
 0x0042, 0, 0, 0
 },
+.cp15_c1_sys = 0x00c50078,
 .features = ARM_FEATURE(V7) |
 ARM_FEATURE(VFP3) |
 ARM_FEATURE(VFP_FP16) |
@@ -371,6 +387,7 @@ static const ARMCPUInfo arm_cpus[] = {
 0x02101110, 0x13112111, 0x21232041, 0x2131,
 0x10011142, 0, 0, 0
 },
+.cp15_c1_sys = 0x00c50078,
 .features = ARM_FEATURE(V7) |

[Qemu-devel] [PATCH RFC v4 04/20] target-arm: Prepare model-specific class_init function

This allows to share initialization between CPU models.

Signed-off-by: Andreas Färber 
---
 target-arm/cpu.c |5 +
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index 00b7ecc..1e34cba 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -102,6 +102,7 @@ static void arm_cpu_reset(CPUState *c)
 typedef struct ARMCPUInfo {
 const char *name;
 uint32_t id;
+void (*class_init)(ARMCPUClass *klass, const struct ARMCPUInfo *info);
 } ARMCPUInfo;
 
 static const ARMCPUInfo arm_cpus[] = {
@@ -237,6 +238,10 @@ static void arm_cpu_class_init(ObjectClass *klass, void 
*data)
 cpu_class->reset = arm_cpu_reset;
 
 k->cp15.c0_cpuid = info->id;
+
+if (info->class_init != NULL) {
+(*info->class_init)(k, info);
+}
 }
 
 static void cpu_register(const ARMCPUInfo *info)
-- 
1.7.7

[Qemu-devel] [PATCH RFC v4 08/20] target-arm: Store cp15 c0_c1 and c0_c2 in ARMCPUClass

For now set them in the reset function.

Signed-off-by: Andreas Färber 
Cc: Peter Maydell 
---
 target-arm/cpu-qom.h |2 +
 target-arm/cpu.c |   63 ++
 target-arm/helper.c  |   51 
 3 files changed, 65 insertions(+), 51 deletions(-)

diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
index 6c97337..0148d18 100644
--- a/target-arm/cpu-qom.h
+++ b/target-arm/cpu-qom.h
@@ -48,6 +48,8 @@ typedef struct ARMCPUClass {
 
 struct {
 uint32_t c0_cpuid;
+uint32_t c0_c1[8];
+uint32_t c0_c2[8];
 } cp15;
 
 uint32_t features;
diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index f4c05d8..74be400 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -45,6 +45,10 @@ static void arm_cpu_reset(CPUState *c)
 env->cp15.c0_cpuid = id;
 env->cp15.c15_config_base_address = tmp;
 
+/* TODO Move these into arm_cpu_initfn() once no longer zeroed above. */
+memcpy(env->cp15.c0_c1, klass->cp15.c0_c1, 8 * sizeof(uint32_t));
+memcpy(env->cp15.c0_c2, klass->cp15.c0_c2, 8 * sizeof(uint32_t));
+
 #if defined(CONFIG_USER_ONLY)
 env->uncached_cpsr = ARM_CPU_MODE_USR;
 /* For user mode we must enable access to coprocessors */
@@ -165,6 +169,8 @@ static inline void unset_class_feature(ARMCPUClass *klass, 
int feature)
 typedef struct ARMCPUInfo {
 const char *name;
 uint32_t id;
+uint32_t cp15_c0_c1[8];
+uint32_t cp15_c0_c2[8];
 uint32_t features;
 void (*class_init)(ARMCPUClass *klass, const struct ARMCPUInfo *info);
 } ARMCPUInfo;
@@ -177,6 +183,13 @@ static void arm1136_r0_class_init(ARMCPUClass *k, const 
ARMCPUInfo *info)
 
 k->features = r1_class->features;
 unset_class_feature(k, ARM_FEATURE_V6K);
+
+/* These ID register values are correct for 1136 but may be wrong
+ * for 1136_r2 (in particular r0p2 does not actually implement most
+ * of the ID registers).
+ */
+memcpy(k->cp15.c0_c1, r1_class->cp15.c0_c1, 8 * sizeof(uint32_t));
+memcpy(k->cp15.c0_c2, r1_class->cp15.c0_c2, 8 * sizeof(uint32_t));
 }
 
 static void ti925t_reset(CPUState *c)
@@ -246,12 +259,28 @@ static const ARMCPUInfo arm_cpus[] = {
 {
 .name = "arm1136",
 .id = 0x4117b363,
+.cp15_c0_c1 = {
+0x111, 0x1, 0x2, 0x3,
+0x01130003, 0x10030302, 0x01222110, 0
+},
+.cp15_c0_c2 = {
+0x00140011, 0x12002111, 0x1123, 0x01102131,
+0x141, 0, 0, 0
+},
 .features = ARM_FEATURE(V6) |
 ARM_FEATURE(VFP),
 },
 {
 .name = "arm1176",
 .id = 0x410fb767,
+.cp15_c0_c1 = {
+0x111, 0x11, 0x33, 0,
+0x01130003, 0x10030302, 0x01222100, 0
+},
+.cp15_c0_c2 = {
+0x0140011, 0x12002111, 0x11231121, 0x01102131,
+0x01141, 0, 0, 0
+},
 .features = ARM_FEATURE(V6K) |
 ARM_FEATURE(VFP) |
 ARM_FEATURE(VAPA),
@@ -259,6 +288,14 @@ static const ARMCPUInfo arm_cpus[] = {
 {
 .name = "arm11mpcore",
 .id = 0x410fb022,
+.cp15_c0_c1 = {
+0x111, 0x1, 0, 0x2,
+0x01100103, 0x10020302, 0x01222000, 0
+},
+.cp15_c0_c2 = {
+0x00100011, 0x12002111, 0x11221011, 0x01102131,
+0x141, 0, 0, 0
+},
 .features = ARM_FEATURE(V6K) |
 ARM_FEATURE(VFP) |
 ARM_FEATURE(VAPA),
@@ -272,6 +309,14 @@ static const ARMCPUInfo arm_cpus[] = {
 {
 .name = "cortex-a8",
 .id = 0x410fc080,
+.cp15_c0_c1 = {
+0x1031, 0x11, 0x400, 0,
+0x3113, 0x2000, 0x01202000, 0x11
+},
+.cp15_c0_c2 = {
+0x0010, 0x12112111, 0x21232031, 0x2131,
+0x0042, 0, 0, 0
+},
 .features = ARM_FEATURE(V7) |
 ARM_FEATURE(VFP3) |
 ARM_FEATURE(NEON) |
@@ -280,6 +325,14 @@ static const ARMCPUInfo arm_cpus[] = {
 {
 .name = "cortex-a9",
 .id = 0x410fc090,
+.cp15_c0_c1 = {
+0x1031, 0x11, 0x000, 0,
+0x00100103, 0x2000, 0x0123, 0x2111
+},
+.cp15_c0_c2 = {
+0x0010, 0x13112111, 0x21232041, 0x2131,
+0x0042, 0, 0, 0
+},
 .features = ARM_FEATURE(V7) |
 ARM_FEATURE(VFP3) |
 ARM_FEATURE(VFP_FP16) |
@@ -294,6 +347,14 @@ static const ARMCPUInfo arm_cpus[] = {
 {
 .name = "cortex-a15",
 .id = 0x412fc0f1,
+.cp15_c0_c1 = {
+0x1131, 0x00011011, 0x02010555, 0x,
+0x10201105, 0x2000, 0x0124, 0x02102211
+},
+.cp15_c0_c2 = {
+0x02101110, 0x13112111, 0x21232041, 0x2131,
+0x10011142, 0, 0, 0
+},
 .featu

Re: [Qemu-devel] [libvirt] Modern CPU models cannot be used with libvirt

Am 10.03.2012 19:24, schrieb Anthony Liguori:
> Humans probably do one of two things: 1) no cpu option or 2) -cpu host.
> 
> So then why are you introducing -cpu Westmere?
[...]
> P.S. I spent 30 minutes the other day helping a user who was attempting
> to figure out whether his processor was a Conroe, Penryn, etc.  Making
> this determination is fairly difficult and it makes me wonder whether
> having CPU code names is even the best interface for oVirt..

That's why Alex suggested -cpu best, which goes through all those
definitions whatever their name and chooses the closest one IIUC.

http://patchwork.ozlabs.org/patch/134955/

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 RFC v4 09/20] target-arm: Store CTR in ARMCPUClass

Signed-off-by: Andreas Färber 
Cc: Peter Maydell 
---
 target-arm/cpu-qom.h |1 +
 target-arm/cpu.c |   18 ++
 target-arm/helper.c  |   12 
 3 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
index 0148d18..722d164 100644
--- a/target-arm/cpu-qom.h
+++ b/target-arm/cpu-qom.h
@@ -48,6 +48,7 @@ typedef struct ARMCPUClass {
 
 struct {
 uint32_t c0_cpuid;
+uint32_t c0_cachetype;
 uint32_t c0_c1[8];
 uint32_t c0_c2[8];
 } cp15;
diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index 74be400..c7cf46d 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -46,6 +46,7 @@ static void arm_cpu_reset(CPUState *c)
 env->cp15.c15_config_base_address = tmp;
 
 /* TODO Move these into arm_cpu_initfn() once no longer zeroed above. */
+env->cp15.c0_cachetype = klass->cp15.c0_cachetype;
 memcpy(env->cp15.c0_c1, klass->cp15.c0_c1, 8 * sizeof(uint32_t));
 memcpy(env->cp15.c0_c2, klass->cp15.c0_c2, 8 * sizeof(uint32_t));
 
@@ -169,6 +170,7 @@ static inline void unset_class_feature(ARMCPUClass *klass, 
int feature)
 typedef struct ARMCPUInfo {
 const char *name;
 uint32_t id;
+uint32_t cp15_c0_cachetype;
 uint32_t cp15_c0_c1[8];
 uint32_t cp15_c0_c2[8];
 uint32_t features;
@@ -188,6 +190,7 @@ static void arm1136_r0_class_init(ARMCPUClass *k, const 
ARMCPUInfo *info)
  * for 1136_r2 (in particular r0p2 does not actually implement most
  * of the ID registers).
  */
+k->cp15.c0_cachetype = r1_class->cp15.c0_cachetype;
 memcpy(k->cp15.c0_c1, r1_class->cp15.c0_c1, 8 * sizeof(uint32_t));
 memcpy(k->cp15.c0_c2, r1_class->cp15.c0_c2, 8 * sizeof(uint32_t));
 }
@@ -216,12 +219,16 @@ static void sa11xx_class_init(ARMCPUClass *k, const 
ARMCPUInfo *info)
 
 static void pxa25x_class_init(ARMCPUClass *k, const ARMCPUInfo *info)
 {
+k->cp15.c0_cachetype = 0xd172172;
+
 set_class_feature(k, ARM_FEATURE_V5);
 set_class_feature(k, ARM_FEATURE_XSCALE);
 }
 
 static void pxa270_class_init(ARMCPUClass *k, const ARMCPUInfo *info)
 {
+k->cp15.c0_cachetype = 0xd172172;
+
 set_class_feature(k, ARM_FEATURE_V5);
 set_class_feature(k, ARM_FEATURE_XSCALE);
 set_class_feature(k, ARM_FEATURE_IWMMXT);
@@ -231,18 +238,21 @@ static const ARMCPUInfo arm_cpus[] = {
 {
 .name = "arm926",
 .id = 0x41069265,
+.cp15_c0_cachetype = 0x1dd20d2,
 .features = ARM_FEATURE(V5) |
 ARM_FEATURE(VFP),
 },
 {
 .name = "arm946",
 .id = 0x41059461,
+.cp15_c0_cachetype = 0x0f004006,
 .features = ARM_FEATURE(V5) |
 ARM_FEATURE(MPU),
 },
 {
 .name = "arm1026",
 .id = 0x4106a262,
+.cp15_c0_cachetype = 0x1dd20d2,
 .features = ARM_FEATURE(V5) |
 ARM_FEATURE(VFP) |
 ARM_FEATURE(AUXCR),
@@ -259,6 +269,7 @@ static const ARMCPUInfo arm_cpus[] = {
 {
 .name = "arm1136",
 .id = 0x4117b363,
+.cp15_c0_cachetype = 0x1dd20d2,
 .cp15_c0_c1 = {
 0x111, 0x1, 0x2, 0x3,
 0x01130003, 0x10030302, 0x01222110, 0
@@ -273,6 +284,7 @@ static const ARMCPUInfo arm_cpus[] = {
 {
 .name = "arm1176",
 .id = 0x410fb767,
+.cp15_c0_cachetype = 0x1dd20d2,
 .cp15_c0_c1 = {
 0x111, 0x11, 0x33, 0,
 0x01130003, 0x10030302, 0x01222100, 0
@@ -288,6 +300,7 @@ static const ARMCPUInfo arm_cpus[] = {
 {
 .name = "arm11mpcore",
 .id = 0x410fb022,
+.cp15_c0_cachetype = 0x1dd20d2,
 .cp15_c0_c1 = {
 0x111, 0x1, 0, 0x2,
 0x01100103, 0x10020302, 0x01222000, 0
@@ -309,6 +322,7 @@ static const ARMCPUInfo arm_cpus[] = {
 {
 .name = "cortex-a8",
 .id = 0x410fc080,
+.cp15_c0_cachetype = 0x82048004,
 .cp15_c0_c1 = {
 0x1031, 0x11, 0x400, 0,
 0x3113, 0x2000, 0x01202000, 0x11
@@ -325,6 +339,7 @@ static const ARMCPUInfo arm_cpus[] = {
 {
 .name = "cortex-a9",
 .id = 0x410fc090,
+.cp15_c0_cachetype = 0x80038003,
 .cp15_c0_c1 = {
 0x1031, 0x11, 0x000, 0,
 0x00100103, 0x2000, 0x0123, 0x2111
@@ -347,6 +362,7 @@ static const ARMCPUInfo arm_cpus[] = {
 {
 .name = "cortex-a15",
 .id = 0x412fc0f1,
+.cp15_c0_cachetype = 0x8444c004,
 .cp15_c0_c1 = {
 0x1131, 0x00011011, 0x02010555, 0x,
 0x10201105, 0x2000, 0x0124, 0x02102211
@@ -367,6 +383,7 @@ static const ARMCPUInfo arm_cpus[] = {
 {
 .name = "ti925t",
 .id = 0x54029252,
+.cp15_c0_cachetype = 0x5109149,
 .features = ARM_FEATURE(V4T) |
 ARM_FEATURE(OMAPCP),
 .class_init = ti925t_class_init,
@@ -473,6 +490,7 @@ static void arm_c

[Qemu-devel] [PATCH RFC v4 13/20] target-arm: Store VFP FPSID register in ARMCPUClass

Signed-off-by: Andreas Färber 
Cc: Peter Maydell 
---
 target-arm/cpu-qom.h |3 +++
 target-arm/cpu.c |   11 +++
 target-arm/cpu.h |1 -
 target-arm/helper.c  |   12 
 4 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
index 3832986..6cb7891 100644
--- a/target-arm/cpu-qom.h
+++ b/target-arm/cpu-qom.h
@@ -53,6 +53,9 @@ typedef struct ARMCPUClass {
 uint32_t c0_c2[8];
 uint32_t c1_sys;
 } cp15;
+struct {
+uint32_t fpsid;
+} vfp;
 
 uint32_t features;
 } ARMCPUClass;
diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index 1614be4..ad4a90a 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -52,6 +52,7 @@ static void arm_cpu_reset(CPUState *c)
 
 /* Genuine reset */
 env->cp15.c1_sys = klass->cp15.c1_sys;
+env->vfp.xregs[ARM_VFP_FPSID] = klass->vfp.fpsid;
 
 #if defined(CONFIG_USER_ONLY)
 env->uncached_cpsr = ARM_CPU_MODE_USR;
@@ -177,6 +178,7 @@ typedef struct ARMCPUInfo {
 uint32_t cp15_c0_c1[8];
 uint32_t cp15_c0_c2[8];
 uint32_t cp15_c1_sys;
+uint32_t vfp_fpsid;
 uint32_t features;
 void (*class_init)(ARMCPUClass *klass, const struct ARMCPUInfo *info);
 } ARMCPUInfo;
@@ -263,6 +265,7 @@ static const ARMCPUInfo arm_cpus[] = {
 .id = 0x41069265,
 .cp15_c0_cachetype = 0x1dd20d2,
 .cp15_c1_sys = 0x00090078,
+.vfp_fpsid = 0x41011090,
 .features = ARM_FEATURE(V5) |
 ARM_FEATURE(VFP),
 },
@@ -279,6 +282,7 @@ static const ARMCPUInfo arm_cpus[] = {
 .id = 0x4106a262,
 .cp15_c0_cachetype = 0x1dd20d2,
 .cp15_c1_sys = 0x00090078,
+.vfp_fpsid = 0x410110a0,
 .features = ARM_FEATURE(V5) |
 ARM_FEATURE(VFP) |
 ARM_FEATURE(AUXCR),
@@ -305,6 +309,7 @@ static const ARMCPUInfo arm_cpus[] = {
 0x141, 0, 0, 0
 },
 .cp15_c1_sys = 0x00050078,
+.vfp_fpsid = 0x410120b4,
 .features = ARM_FEATURE(V6) |
 ARM_FEATURE(VFP),
 },
@@ -321,6 +326,7 @@ static const ARMCPUInfo arm_cpus[] = {
 0x01141, 0, 0, 0
 },
 .cp15_c1_sys = 0x00050078,
+.vfp_fpsid = 0x410120b5,
 .features = ARM_FEATURE(V6K) |
 ARM_FEATURE(VFP) |
 ARM_FEATURE(VAPA),
@@ -337,6 +343,7 @@ static const ARMCPUInfo arm_cpus[] = {
 0x00100011, 0x12002111, 0x11221011, 0x01102131,
 0x141, 0, 0, 0
 },
+.vfp_fpsid = 0x410120b4,
 .features = ARM_FEATURE(V6K) |
 ARM_FEATURE(VFP) |
 ARM_FEATURE(VAPA),
@@ -360,6 +367,7 @@ static const ARMCPUInfo arm_cpus[] = {
 0x0042, 0, 0, 0
 },
 .cp15_c1_sys = 0x00c50078,
+.vfp_fpsid = 0x410330c0,
 .features = ARM_FEATURE(V7) |
 ARM_FEATURE(VFP3) |
 ARM_FEATURE(NEON) |
@@ -378,6 +386,7 @@ static const ARMCPUInfo arm_cpus[] = {
 0x0042, 0, 0, 0
 },
 .cp15_c1_sys = 0x00c50078,
+.vfp_fpsid = 0x41033090,
 .features = ARM_FEATURE(V7) |
 ARM_FEATURE(VFP3) |
 ARM_FEATURE(VFP_FP16) |
@@ -402,6 +411,7 @@ static const ARMCPUInfo arm_cpus[] = {
 0x10011142, 0, 0, 0
 },
 .cp15_c1_sys = 0x00c50078,
+.vfp_fpsid = 0x410430f0,
 .features = ARM_FEATURE(V7) |
 ARM_FEATURE(VFP4) |
 ARM_FEATURE(VFP_FP16) |
@@ -526,6 +536,7 @@ static void arm_cpu_class_init(ObjectClass *klass, void 
*data)
 memcpy(k->cp15.c0_c1, info->cp15_c0_c1, 8 * sizeof(uint32_t));
 memcpy(k->cp15.c0_c2, info->cp15_c0_c2, 8 * sizeof(uint32_t));
 k->cp15.c1_sys = info->cp15_c1_sys;
+k->vfp.fpsid = info->vfp_fpsid;
 k->features = info->features;
 
 if (info->class_init != NULL) {
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index d135221..e0d711f 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -403,7 +403,6 @@ void cpu_arm_set_cp_io(CPUARMState *env, int cpnum,
 #define ARM_CPUID(env) (env->cp15.c0_cpuid)
 
 #define ARM_CPUID_ARM1026 0x4106a262
-#define ARM_CPUID_ARM926  0x41069265
 #define ARM_CPUID_TI915T  0x54029152
 #define ARM_CPUID_TI925T  0x54029252
 #define ARM_CPUID_ARM1136 0x4117b363
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 421ce98..6ea656c 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -8,12 +8,6 @@
 static void cpu_reset_model_id(CPUARMState *env, uint32_t id)
 {
 switch (id) {
-case ARM_CPUID_ARM926:
-env->vfp.xregs[ARM_VFP_FPSID] = 0x41011090;
-break;
-case ARM_CPUID_ARM1026:
-env->vfp.xregs[ARM_VFP_FPSID] = 0x410110a0;
-break;
 case ARM_CPUID_ARM1136:
 /* This is the 1136 r1, which is a v6K core */
 /* Fall through */
@@ -26,22 +20

[Qemu-devel] [PATCH RFC v4 19/20] target-arm: Simplify pxa270 CPU classes

Instead of having classes per CPUID value, keep just one pxa270 class
and use pxa270-a1 etc. as aliases that override default CPU properties.

Signed-off-by: Andreas Färber 
Cc: Paul Brook 
Cc: Peter Maydell 
---
 target-arm/cpu.c|   27 +--
 target-arm/helper.c |   29 -
 2 files changed, 25 insertions(+), 31 deletions(-)

diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index ad33742..da5bb6e 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -539,36 +539,11 @@ static const ARMCPUInfo arm_cpus[] = {
 .class_init = pxa25x_class_init,
 },
 {
-.name = "pxa270-a0",
+.name = "pxa270",
 .id = 0x69054110,
 .class_init = pxa270_class_init,
 },
 {
-.name = "pxa270-a1",
-.id = 0x69054111,
-.class_init = pxa270_class_init,
-},
-{
-.name = "pxa270-b0",
-.id = 0x69054112,
-.class_init = pxa270_class_init,
-},
-{
-.name = "pxa270-b1",
-.id = 0x69054113,
-.class_init = pxa270_class_init,
-},
-{
-.name = "pxa270-c0",
-.id = 0x69054114,
-.class_init = pxa270_class_init,
-},
-{
-.name = "pxa270-c5",
-.id = 0x69054117,
-.class_init = pxa270_class_init,
-},
-{
 .name = "any",
 .id = 0x,
 .features = ARM_FEATURE(V7) |
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 2d6331d..f66f500 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -64,22 +64,41 @@ static int vfp_gdb_set_reg(CPUARMState *env, uint8_t *buf, 
int reg)
 
 CPUARMState *cpu_arm_init(const char *cpu_model)
 {
+const char *cpu_typename = cpu_model;
 ObjectClass *klass;
 ARMCPU *cpu;
 CPUARMState *env;
 static int inited = 0;
 
-/* One legacy alias to check */
-if (strcmp(cpu_model, "pxa270") == 0) {
-cpu_model = "pxa270-a0";
+/* Map legacy aliases to base class */
+if (strcmp(cpu_model, "pxa270-a0") == 0 ||
+strcmp(cpu_model, "pxa270-a1") == 0 ||
+strcmp(cpu_model, "pxa270-b0") == 0 ||
+strcmp(cpu_model, "pxa270-b1") == 0 ||
+strcmp(cpu_model, "pxa270-c0") == 0 ||
+strcmp(cpu_model, "pxa270-c5") == 0) {
+cpu_typename = "pxa270";
 }
 
-klass = object_class_by_name(cpu_model);
+klass = object_class_by_name(cpu_typename);
 if (klass == NULL) {
 return NULL;
 }
-cpu = ARM_CPU(object_new(cpu_model));
+cpu = ARM_CPU(object_new(cpu_typename));
 env = &cpu->env;
+env->cpu_model_str = cpu_model;
+/* Set properties on base object */
+if (strcmp(cpu_model, "pxa270-a1") == 0) {
+object_property_set_int(OBJECT(cpu), 0x1, "cpuid-revision", NULL);
+} else if (strcmp(cpu_model, "pxa270-b0") == 0) {
+object_property_set_int(OBJECT(cpu), 0x2, "cpuid-revision", NULL);
+} else if (strcmp(cpu_model, "pxa270-b1") == 0) {
+object_property_set_int(OBJECT(cpu), 0x3, "cpuid-revision", NULL);
+} else if (strcmp(cpu_model, "pxa270-c0") == 0) {
+object_property_set_int(OBJECT(cpu), 0x4, "cpuid-revision", NULL);
+} else if (strcmp(cpu_model, "pxa270-c5") == 0) {
+object_property_set_int(OBJECT(cpu), 0x7, "cpuid-revision", NULL);
+}
 
 if (tcg_enabled() && !inited) {
 inited = 1;
-- 
1.7.7

[Qemu-devel] [PATCH RFC v4 18/20] target-arm: Add cpuid-{variant, revision} properties to CPU

Allow to inspect and manipulate MIDR variant and revision fields.

Signed-off-by: Andreas Färber 
Cc: Peter Maydell 
---
 target-arm/cpu.c |   48 
 1 files changed, 48 insertions(+), 0 deletions(-)

diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index 8917a20..ad33742 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -20,6 +20,7 @@
 
 #include "cpu-qom.h"
 #include "qemu-common.h"
+#include "qapi/qapi-visit-core.h"
 #if !defined(CONFIG_USER_ONLY)
 #include "hw/loader.h"
 #endif
@@ -173,6 +174,46 @@ static inline void unset_class_feature(ARMCPUClass *klass, 
int feature)
 klass->features &= ~(1u << feature);
 }
 
+static void arm_cpuid_variant_get(Object *obj, Visitor *v, void *opaque,
+  const char *name, Error **errp)
+{
+ARMCPU *cpu = ARM_CPU(obj);
+int64_t value = (cpu->env.cp15.c0_cpuid >> 20) & 0xf;
+
+visit_type_int(v, &value, name, errp);
+}
+
+static void arm_cpuid_variant_set(Object *obj, Visitor *v, void *opaque,
+  const char *name, Error **errp)
+{
+ARMCPU *cpu = ARM_CPU(obj);
+int64_t value;
+
+visit_type_int(v, &value, name, errp);
+cpu->env.cp15.c0_cpuid &= ~(0xf << 20);
+cpu->env.cp15.c0_cpuid |= (value << 20) & 0xf;
+}
+
+static void arm_cpuid_revision_get(Object *obj, Visitor *v, void *opaque,
+   const char *name, Error **errp)
+{
+ARMCPU *cpu = ARM_CPU(obj);
+int64_t value = cpu->env.cp15.c0_cpuid & 0xf;
+
+visit_type_int(v, &value, name, errp);
+}
+
+static void arm_cpuid_revision_set(Object *obj, Visitor *v, void *opaque,
+   const char *name, Error **errp)
+{
+ARMCPU *cpu = ARM_CPU(obj);
+int64_t value;
+
+visit_type_int(v, &value, name, errp);
+cpu->env.cp15.c0_cpuid &= ~0xf;
+cpu->env.cp15.c0_cpuid |= value & 0xf;
+}
+
 /* CPU models */
 
 typedef struct ARMCPUInfo {
@@ -554,6 +595,13 @@ static void arm_cpu_initfn(Object *obj)
 cpu->env.cp15.c0_cpuid = cpu_class->cp15.c0_cpuid;
 
 cpu_reset(CPU(cpu));
+
+object_property_add(obj, "cpuid-variant", "uint4",
+arm_cpuid_variant_get,
+arm_cpuid_variant_set, NULL, NULL, NULL);
+object_property_add(obj, "cpuid-revision", "uint4",
+arm_cpuid_revision_get,
+arm_cpuid_revision_set, NULL, NULL, NULL);
 }
 
 static void arm_cpu_class_init(ObjectClass *klass, void *data)
-- 
1.7.7

Re: [Qemu-devel] [libvirt] Modern CPU models cannot be used with libvirt

2012-03-10 Thread Anthony Liguori

On 03/10/2012 09:58 AM, Eduardo Habkost wrote:

On Sat, Mar 10, 2012 at 12:42:46PM +, Daniel P. Berrange wrote:

I could have sworn we had this discussion a year ago or so, and had decided
that the default CPU models would be in something like
/usr/share/qemu/cpu-x86_64.conf
and loaded regardless of the -nodefconfig setting. /etc/qemu/target-x86_64.conf
would be solely for end user configuration changes, not for QEMU builtin
defaults.

But looking at the code in QEMU, it doesn't seem we ever implemented this ?

Arrrgggh. It seems this was implemented as a patch in RHEL-6 qemu RPMs but,
contrary to our normal RHEL development practice, it was not based on
a cherry-pick of an upstream patch :-(

For sake of reference, I'm attaching the two patches from the RHEL6 source
RPM that do what I'm describing

NB, I'm not neccessarily advocating these patches for upstream. I still
maintain that libvirt should write out a config file containing the
exact CPU model description it desires and specify that with -readconfig.
The end result would be identical from QEMU's POV and it would avoid
playing games with QEMU's config loading code.

I agree that libvirt should just write the config somewhere. The problem
here is to define: 1) what information should be mandatory on that
config data; 2) who should be responsible to test and maintain sane
defaults (and where should they be maintained).

The current cpudef definitions are simply too low-level to require it to
be written from scratch. Lots of testing have to be done to make sure we
have working combinations of CPUID bits defined, so they can be used as
defaults or templates. Not facilitating reuse of those tested
defauls/templates by libvirt is duplication of efforts.

Really, if we expect libvirt to define all the CPU bits from scratch on
a config file, we could as well just expect libvirt to open /dev/kvm
itself and call the all CPUID setup ioctl()s itself. That's how
low-level some of the cpudef bits are.

Let's step back here.

Why are you writing these patches? It's probably not because you have a desire
to say -cpu Westmere when you run QEMU on your laptop. I'd wager to say that no
human has ever done that or that if they had, they did so by accident because
they read documentation and thought they had to.

Humans probably do one of two things: 1) no cpu option or 2) -cpu host.

So then why are you introducing -cpu Westmere? Because ovirt-engine has a
concept of datacenters and the entire datacenter has to use a compatible CPU
model to allow migration compatibility. Today, the interface that ovirt-engine
exposes is based on CPU codenames. Presumably ovirt-engine wants to add a
Westmere CPU group and as such have levied a requirement down the stack to QEMU.

But there's no intrinsic reason why it uses CPU model names. VMware doesn't do
this. It has a concept of compatibility groups[1].

oVirt could just as well define compatibility groups like GroupA, GroupB,
GroupC, etc. and then the -cpu option we would be discussing would be -cpu GroupA.

This is why it's a configuration option and not builtin to QEMU. It's a user
interface as as such, should be defined at a higher level.

Perhaps it really should be VDSM that is providing the model info to libvirt?
Then they can add whatever groups then want whenever they want as long as we
have the appropriate feature bits.

P.S. I spent 30 minutes the other day helping a user who was attempting to
figure out whether his processor was a Conroe, Penryn, etc. Making this
determination is fairly difficult and it makes me wonder whether having CPU code
names is even the best interface for oVirt..

[1]
http://kb.vmware.com/selfservice/microsites/search.do?language=en_US&cmd=displayKC&externalId=1991

Regards,

Anthony Liguori

(Also, there are additional low-level bits that really have to be
maintained somewhere, just to have sane defaults. Currently many CPUID
leafs are exposed to the guest without letting the user control them,
and worse: without keeping stability of guest-visible bits when
upgrading Qemu or the host kernel. And that's what machine-types are
for: to have sane defaults to be used as base.)

Let me give you a practical example: I had a bug report about improper
CPU topology information[1]. After investigating it, I have found out
that the "level" cpudef field is too low; CPU core topology information
is provided on CPUID leaf 4, and most of the Intel CPU models on Qemu
have level=2 today (I don't know why). So, Qemu is responsible for
exposing CPU topology information set using '-smp' to the guest OS, but
libvirt would have to be responsible for choosing a proper "level" value
that makes that information visible to the guest. We can _allow_ libvirt
to fiddle with these low-level bits, of course, but requiring every
management layer to build this low-level information from scratch is
just a recipe to waste developer time.

(And I really hope that there's no plan to requi

[Qemu-devel] [PATCH RFC v4 11/20] target-arm: Drop JTAG_ID documentation

None of the machines in QEMU offer a JTAG debug interface, so this info
was unused. Further, the PXA250 ID contradicts the February 2002
Developer's Manual, which has it as 0xn9264013 with n the MIDR Revision.

Signed-off-by: Andreas Färber 
Cc: Peter Maydell 
Cc: Andrzej Zaborowski 
---
 target-arm/cpu.h|5 -
 target-arm/helper.c |8 
 2 files changed, 0 insertions(+), 13 deletions(-)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 66e62e2..d7e73d8 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -406,11 +406,6 @@ void cpu_arm_set_cp_io(CPUARMState *env, int cpnum,
 #define ARM_CPUID_ARM926  0x41069265
 #define ARM_CPUID_TI915T  0x54029152
 #define ARM_CPUID_TI925T  0x54029252
-#define ARM_CPUID_PXA250  0x69052100
-#define ARM_CPUID_PXA255  0x69052d00
-#define ARM_CPUID_PXA260  0x69052903
-#define ARM_CPUID_PXA261  0x69052d05
-#define ARM_CPUID_PXA262  0x69052d06
 #define ARM_CPUID_PXA270_A0   0x69054110
 #define ARM_CPUID_PXA270_A1   0x69054111
 #define ARM_CPUID_PXA270_B0   0x69054112
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 548d8cf..46e9dc5 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -71,20 +71,12 @@ static void cpu_reset_model_id(CPUARMState *env, uint32_t 
id)
 env->cp15.c15_i_max = 0x000;
 env->cp15.c15_i_min = 0xff0;
 break;
-case ARM_CPUID_PXA250:
-case ARM_CPUID_PXA255:
-case ARM_CPUID_PXA260:
-case ARM_CPUID_PXA261:
-case ARM_CPUID_PXA262:
-/* JTAG_ID is ((id << 28) | 0x09265013) */
-break;
 case ARM_CPUID_PXA270_A0:
 case ARM_CPUID_PXA270_A1:
 case ARM_CPUID_PXA270_B0:
 case ARM_CPUID_PXA270_B1:
 case ARM_CPUID_PXA270_C0:
 case ARM_CPUID_PXA270_C5:
-/* JTAG_ID is ((id << 28) | 0x09265013) */
 env->iwmmxt.cregs[ARM_IWMMXT_wCID] = 0x69051000 | 'Q';
 break;
 default:
-- 
1.7.7

[Qemu-devel] [PATCH RFC v4 02/20] target-arm: Introduce QOM ARMCPUClass

Create a CPU subclass, and register classes matching all CPU models
except for "pxa270", which is an alias for "pxa270-a0".

Let arm_cpu_list() enumerate CPU subclasses in alphabetical order,
except for special value "any".

Replace cpu_arm_find_by_name()'s string -> CPUID lookup by storing the
CPUID in the class.
NB: CPUIDs were first introduced by Paul Brook in r1765 (2006).

Signed-off-by: Andreas Färber 
Cc: Anthony Liguori 
Cc: Paul Brook 
Cc: Peter Maydell 
---
 Makefile.target  |1 +
 target-arm/cpu-qom.h |   64 +
 target-arm/cpu.c |  193 ++
 target-arm/cpu.h |1 -
 target-arm/helper.c  |  108 +---
 5 files changed, 308 insertions(+), 59 deletions(-)
 create mode 100644 target-arm/cpu-qom.h
 create mode 100644 target-arm/cpu.c

diff --git a/Makefile.target b/Makefile.target
index cb1532a..c2c4bca 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -87,6 +87,7 @@ endif
 libobj-$(TARGET_SPARC64) += vis_helper.o
 libobj-$(CONFIG_NEED_MMU) += mmu.o
 libobj-$(TARGET_ARM) += neon_helper.o iwmmxt_helper.o
+libobj-$(TARGET_ARM) += cpu.o
 ifeq ($(TARGET_BASE_ARCH), sparc)
 libobj-y += fop_helper.o cc_helper.o win_helper.o mmu_helper.o ldst_helper.o
 libobj-y += cpu_init.o
diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
new file mode 100644
index 000..b2917ea
--- /dev/null
+++ b/target-arm/cpu-qom.h
@@ -0,0 +1,64 @@
+/*
+ * QEMU ARM CPU
+ *
+ * Copyright (c) 2012 SUSE LINUX Products GmbH
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see
+ * 
+ */
+#ifndef QEMU_ARM_CPU_QOM_H
+#define QEMU_ARM_CPU_QOM_H
+
+#include "qemu/cpu.h"
+
+#define TYPE_ARM_CPU "arm-cpu"
+
+#define ARM_CPU_CLASS(klass) \
+OBJECT_CLASS_CHECK(ARMCPUClass, (klass), TYPE_ARM_CPU)
+#define ARM_CPU(obj) \
+OBJECT_CHECK(ARMCPU, (obj), TYPE_ARM_CPU)
+#define ARM_CPU_GET_CLASS(obj) \
+OBJECT_GET_CLASS(ARMCPUClass, (obj), TYPE_ARM_CPU)
+
+/**
+ * ARMCPUClass:
+ * @parent_reset: The parent class' reset handler.
+ *
+ * An ARM CPU model.
+ */
+typedef struct ARMCPUClass {
+/*< private >*/
+CPUClass parent_class;
+/*< public >*/
+
+void (*parent_reset)(CPUState *cpu);
+
+struct {
+uint32_t c0_cpuid;
+} cp15;
+} ARMCPUClass;
+
+/**
+ * ARMCPU:
+ *
+ * An ARM CPU core.
+ */
+typedef struct ARMCPU {
+/*< private >*/
+CPUState parent_obj;
+/*< public >*/
+} ARMCPU;
+
+
+#endif
diff --git a/target-arm/cpu.c b/target-arm/cpu.c
new file mode 100644
index 000..dabc094
--- /dev/null
+++ b/target-arm/cpu.c
@@ -0,0 +1,193 @@
+/*
+ * QEMU ARM CPU
+ *
+ * Copyright (c) 2012 SUSE LINUX Products GmbH
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see
+ * 
+ */
+
+#include "cpu-qom.h"
+#include "qemu-common.h"
+
+static void arm_cpu_reset(CPUState *c)
+{
+ARMCPUClass *klass = ARM_CPU_GET_CLASS(c);
+
+klass->parent_reset(c);
+}
+
+/* CPU models */
+
+typedef struct ARMCPUInfo {
+const char *name;
+uint32_t id;
+} ARMCPUInfo;
+
+static const ARMCPUInfo arm_cpus[] = {
+{
+.name = "arm926",
+.id = 0x41069265,
+},
+{
+.name = "arm946",
+.id = 0x41059461,
+},
+{
+.name = "arm1026",
+.id = 0x4106a262,
+},
+/* What QEMU calls "arm1136-r2" is actually the 1136 r0p2, i.e. an
+ * older core than plain "arm1136". In particular this does not
+ * have the v6K features.
+ */
+{
+.name = "arm1136-r2",
+.id = 0x4107b362,
+},
+{
+.name = "arm1136",
+.id = 0x4117b363,
+},
+{
+.name = "arm1176",
+.id = 0x410fb767,
+},
+{
+.name = "arm11mpcore",
+.id = 0x410fb022,
+},
+{
+.name = "cor

[Qemu-devel] [PATCH RFC v4 05/20] target-arm: Overwrite reset handler for ti925t

The OMAPCP feature allows to switch between TI915T and TI925T via
cp15 c15_ticonfig register. Move reset into ti925t-specific callback.

Signed-off-by: Andreas Färber 
Cc: Peter Maydell 
---
 target-arm/cpu.c|   18 ++
 target-arm/helper.c |1 -
 2 files changed, 18 insertions(+), 1 deletions(-)

diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index 1e34cba..097701f 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -105,6 +105,23 @@ typedef struct ARMCPUInfo {
 void (*class_init)(ARMCPUClass *klass, const struct ARMCPUInfo *info);
 } ARMCPUInfo;
 
+static void ti925t_reset(CPUState *c)
+{
+ARMCPU *cpu = ARM_CPU(c);
+CPUARMState *env = &cpu->env;
+
+arm_cpu_reset(c);
+
+env->cp15.c0_cpuid = ARM_CPUID_TI925T; /* Depends on wiring. */
+}
+
+static void ti925t_class_init(ARMCPUClass *klass, const ARMCPUInfo *info)
+{
+CPUClass *cpu_class = CPU_CLASS(klass);
+
+cpu_class->reset = ti925t_reset;
+}
+
 static const ARMCPUInfo arm_cpus[] = {
 {
 .name = "arm926",
@@ -157,6 +174,7 @@ static const ARMCPUInfo arm_cpus[] = {
 {
 .name = "ti925t",
 .id = 0x54029252,
+.class_init = ti925t_class_init,
 },
 {
 .name = "sa1100",
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 39f3c40..5ebe308 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -196,7 +196,6 @@ static void cpu_reset_model_id(CPUARMState *env, uint32_t 
id)
 case ARM_CPUID_TI925T:
 set_feature(env, ARM_FEATURE_V4T);
 set_feature(env, ARM_FEATURE_OMAPCP);
-env->cp15.c0_cpuid = ARM_CPUID_TI925T; /* Depends on wiring.  */
 env->cp15.c0_cachetype = 0x5109149;
 env->cp15.c1_sys = 0x0070;
 env->cp15.c15_i_max = 0x000;
-- 
1.7.7

Re: [Qemu-devel] [RFC PATCH 06/17] block: use bdrv_{co, aio}_discard for write_zeroes operations

2012-03-10 Thread Richard Laager

I'm believe your patch set provides these behaviors now:
  * QEMU block drivers report discard_granularity. 
  * discard_granularity = 0 means no discard 
  * The guest is told there's no discard support.
  * discard_granularity < 0 is undefined.
discard_granularity > 0 is reported to the guest as
discard support.
  * QEMU block drivers report discard_zeros_data.
This is passed to the guest when discard_granularity >
0.

I propose adding the following behaviors in any event:
  * If a QEMU block device reports a discard_granularity > 0, it
must be equal to 2^n (n >= 0), or QEMU's block core will change
it to 0. (Non-power-of-two granularities are not likely to exist
in the real world, and this assumption greatly simplifies
ensuring correctness.)
  * For SCSI, report an unmap_granularity to the guest as follows:
  max(logical_block_size, discard_granularity) / logical_block_size


Regarding emulating discard_zeros_data...

I agree that when discard_zeros_data is set, we will need to write
zeroes in some cases. As you noted, IDE has a fixed granularity of one
sector. And the SCSI granularity is a hint only; guests are not
guaranteed to align to that value either. [0]

As a design concept, instead of guaranteeing that 512B zero'ing discards
are supported, I think the QEMU block layer should instead guarantee
aligned discards to QEMU block devices, emulating any misaligned
discards (or portions thereof) by writing zeroes if (and only if)
discard_zeros_data is set. When the QEMU block layer gets a discard:
  * Of the specified discard range, see if it includes an aligned
multiple of discard granularity. If so, save that as the
starting point of a subrange. Then find the last aligned
multiple, if any, and pass that subrange (if start != end) down
to the block driver's discard function.
  * If the discard really fails (i.e. returns failure and sets errno
to something other than "not supported" or equivalent), return
failure to the guest. For "not supported", fall through to the
code below with the full range.
  * At this point, we have zero, one, or two subranges to handle.
  * If and only if discard_zeros_data is set, write zeros to the
remaining subranges, if any. (This would use a lower-level
write_zeroes call which does not attempt to use discard.) If
this fails, return failure to the guest.
  * Return success.

This leaves one remaining issue: In raw-posix.c, for files (i.e. not
devices), I assume you're going to advertise discard_granularity=1 and
discard_zeros_data=1 when compiled with support for
fallocate(FALLOC_FL_PUNCH_HOLE). Note, I'm assuming fallocate() actually
guarantees that it zeros the data when punching holes. I haven't
verified this.

If the guest does a big discard (think mkfs) and fallocate() returns
EOPNOTSUPP, you'll have to zero essentially the whole virtual disk,
which, as you noted, will also allocate it (unless you explicitly check
for holes). This is bad. It can be avoided by not advertising
discard_zeros_data, but as you noted, that's unfortunate.

If we could probe for FALLOC_FL_PUNCH_HOLE support, then we could avoid
advertising discard support based on FALLOC_FL_PUNCH_HOLE when it is not
going to work. This would side step these problems. You said it wasn't
possible to probe for FALLOC_FL_PUNCH_HOLE. Have you considered probing
by extending the file by one byte and then punching that:
char buf = 0;
fstat(s->fd, &st);
pwrite(s->fd, &buf, 1, st.st_size + 1);
has_discard = !fallocate(s->fd, FALLOC_FL_PUNCH_HOLE | 
FALLOC_FL_KEEP_SIZE,
 st.st_size + 1, 1);
ftruncate(s->fd, st.st_size);


[0] See the last paragraph starting on page 8:
http://mkp.net/pubs/linux-advanced-storage.pdf

-- 
Richard



signature.asc
Description: This is a digitally signed message part

[Qemu-devel] [PATCH RFC v4 12/20] target-arm: Move the PXA270's iwMMXt reset to pxa270_reset()

No other emulated CPU uses this at this time.

Signed-off-by: Andreas Färber 
Cc: Peter Maydell 
Cc: Andrzej Zaborowski 
---
 target-arm/cpu.c|   14 ++
 target-arm/cpu.h|6 --
 target-arm/helper.c |8 
 3 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index 2f3190a..1614be4 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -233,8 +233,22 @@ static void pxa25x_class_init(ARMCPUClass *k, const 
ARMCPUInfo *info)
 set_class_feature(k, ARM_FEATURE_XSCALE);
 }
 
+static void pxa270_reset(CPUState *c)
+{
+ARMCPU *cpu = ARM_CPU(c);
+CPUARMState *env = &cpu->env;
+
+arm_cpu_reset(c);
+
+env->iwmmxt.cregs[ARM_IWMMXT_wCID] = 0x69051000 | 'Q';
+}
+
 static void pxa270_class_init(ARMCPUClass *k, const ARMCPUInfo *info)
 {
+CPUClass *cpu_class = CPU_CLASS(k);
+
+cpu_class->reset = pxa270_reset;
+
 k->cp15.c0_cachetype = 0xd172172;
 k->cp15.c1_sys = 0x0078;
 
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index d7e73d8..d135221 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -406,12 +406,6 @@ void cpu_arm_set_cp_io(CPUARMState *env, int cpnum,
 #define ARM_CPUID_ARM926  0x41069265
 #define ARM_CPUID_TI915T  0x54029152
 #define ARM_CPUID_TI925T  0x54029252
-#define ARM_CPUID_PXA270_A0   0x69054110
-#define ARM_CPUID_PXA270_A1   0x69054111
-#define ARM_CPUID_PXA270_B0   0x69054112
-#define ARM_CPUID_PXA270_B1   0x69054113
-#define ARM_CPUID_PXA270_C0   0x69054114
-#define ARM_CPUID_PXA270_C5   0x69054117
 #define ARM_CPUID_ARM1136 0x4117b363
 #define ARM_CPUID_ARM1136_R2  0x4107b362
 #define ARM_CPUID_ARM1176 0x410fb767
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 46e9dc5..421ce98 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -71,14 +71,6 @@ static void cpu_reset_model_id(CPUARMState *env, uint32_t id)
 env->cp15.c15_i_max = 0x000;
 env->cp15.c15_i_min = 0xff0;
 break;
-case ARM_CPUID_PXA270_A0:
-case ARM_CPUID_PXA270_A1:
-case ARM_CPUID_PXA270_B0:
-case ARM_CPUID_PXA270_B1:
-case ARM_CPUID_PXA270_C0:
-case ARM_CPUID_PXA270_C5:
-env->iwmmxt.cregs[ARM_IWMMXT_wCID] = 0x69051000 | 'Q';
-break;
 default:
 break;
 }
-- 
1.7.7

[Qemu-devel] [PATCH RFC v4 07/20] target-arm: No longer abort on unhandled CPUIDs on reset

Having moved the reset code for some CPUs to class_init, we can drop
some of the cases in the switch.

Signed-off-by: Andreas Färber 
Cc: Peter Maydell 
---
 target-arm/cpu.h|2 --
 target-arm/helper.c |5 -
 2 files changed, 0 insertions(+), 7 deletions(-)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index d14fb01..48d42b7 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -427,8 +427,6 @@ void cpu_arm_set_cp_io(CPUARMState *env, int cpnum,
 #define ARM_CPUID_CORTEXA80x410fc080
 #define ARM_CPUID_CORTEXA90x410fc090
 #define ARM_CPUID_CORTEXA15   0x412fc0f1
-#define ARM_CPUID_CORTEXM30x410fc231
-#define ARM_CPUID_ANY 0x
 
 #if defined(CONFIG_USER_ONLY)
 #define TARGET_PAGE_BITS 12
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 0dd6065..59a9812 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -136,10 +136,6 @@ static void cpu_reset_model_id(CPUARMState *env, uint32_t 
id)
 env->cp15.c0_ccsid[2] = 0x711fe07a; /* 4096K L2 unified cache */
 env->cp15.c1_sys = 0x00c50078;
 break;
-case ARM_CPUID_CORTEXM3:
-break;
-case ARM_CPUID_ANY: /* For userspace emulation.  */
-break;
 case ARM_CPUID_TI915T:
 case ARM_CPUID_TI925T:
 env->cp15.c0_cachetype = 0x5109149;
@@ -172,7 +168,6 @@ static void cpu_reset_model_id(CPUARMState *env, uint32_t 
id)
 env->cp15.c1_sys = 0x0070;
 break;
 default:
-cpu_abort(env, "Bad CPU ID: %x\n", id);
 break;
 }
 }
-- 
1.7.7

[Qemu-devel] [PATCH RFC v4 20/20] hw/integratorcp: Add child property for CPU

Only one CPU is created on this board, so we can use "/cpu".

Signed-off-by: Andreas Färber 
---
 hw/integratorcp.c |3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/hw/integratorcp.c b/hw/integratorcp.c
index 9bdb9e6..ac3fd7e 100644
--- a/hw/integratorcp.c
+++ b/hw/integratorcp.c
@@ -443,6 +443,7 @@ static void integratorcp_init(ram_addr_t ram_size,
  const char *kernel_filename, const char *kernel_cmdline,
  const char *initrd_filename, const char *cpu_model)
 {
+ARMCPU *cpu;
 CPUARMState *env;
 MemoryRegion *address_space_mem = get_system_memory();
 MemoryRegion *ram = g_new(MemoryRegion, 1);
@@ -459,6 +460,8 @@ static void integratorcp_init(ram_addr_t ram_size,
 fprintf(stderr, "Unable to find CPU definition\n");
 exit(1);
 }
+cpu = arm_env_get_cpu(env);
+object_property_add_child(object_get_root(), "cpu", OBJECT(cpu), NULL);
 memory_region_init_ram(ram, "integrator.ram", ram_size);
 vmstate_register_ram_global(ram);
 /* ??? On a real system the first 1Mb is mapped as SSRAM or boot flash.  */
-- 
1.7.7

[Qemu-devel] [PATCH RFC v4 17/20] target-arm: Kill off cpu_reset_model_id()

Move last remaining TI925T bits to ti925t_reset().

This reduces cpu_state_reset() to triviality and lets us reset the CPU
directly from arm_cpu_initfn().

Signed-off-by: Andreas Färber 
Cc: Peter Maydell 
---
 target-arm/cpu.c|4 
 target-arm/helper.c |   20 
 2 files changed, 4 insertions(+), 20 deletions(-)

diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index 9f571a1..8917a20 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -217,6 +217,8 @@ static void ti925t_reset(CPUState *c)
 arm_cpu_reset(c);
 
 env->cp15.c0_cpuid = ARM_CPUID_TI925T; /* Depends on wiring. */
+env->cp15.c15_i_max = 0x000;
+env->cp15.c15_i_min = 0xff0;
 }
 
 static void ti925t_class_init(ARMCPUClass *klass, const ARMCPUInfo *info)
@@ -550,6 +552,8 @@ static void arm_cpu_initfn(Object *obj)
 
 cpu->env.cpu_model_str = object_get_typename(obj);
 cpu->env.cp15.c0_cpuid = cpu_class->cp15.c0_cpuid;
+
+cpu_reset(CPU(cpu));
 }
 
 static void arm_cpu_class_init(ObjectClass *klass, void *data)
diff --git a/target-arm/helper.c b/target-arm/helper.c
index ba8a4ae..2d6331d 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -5,28 +5,9 @@
 #include "sysemu.h"
 #include "cpu-qom.h"
 
-static void cpu_reset_model_id(CPUARMState *env, uint32_t id)
-{
-switch (id) {
-case ARM_CPUID_TI915T:
-case ARM_CPUID_TI925T:
-env->cp15.c15_i_max = 0x000;
-env->cp15.c15_i_min = 0xff0;
-break;
-default:
-break;
-}
-}
-
 void cpu_state_reset(CPUARMState *env)
 {
-uint32_t id;
-
 cpu_reset(ENV_GET_CPU(env));
-
-id = env->cp15.c0_cpuid;
-if (id)
-cpu_reset_model_id(env, id);
 }
 
 static int vfp_gdb_get_reg(CPUARMState *env, uint8_t *buf, int reg)
@@ -105,7 +86,6 @@ CPUARMState *cpu_arm_init(const char *cpu_model)
 arm_translate_init();
 }
 
-cpu_state_reset(env);
 if (arm_feature(env, ARM_FEATURE_NEON)) {
 gdb_register_coprocessor(env, vfp_gdb_get_reg, vfp_gdb_set_reg,
  51, "arm-neon.xml", 0);
-- 
1.7.7

[Qemu-devel] [PATCH RFC v4 15/20] target-arm: Store CLIDR in ARMCPUClass

Signed-off-by: Andreas Färber 
Cc: Peter Maydell 
---
 target-arm/cpu-qom.h |1 +
 target-arm/cpu.c |6 ++
 target-arm/helper.c  |3 ---
 3 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
index cd711fa..101cdb1 100644
--- a/target-arm/cpu-qom.h
+++ b/target-arm/cpu-qom.h
@@ -49,6 +49,7 @@ typedef struct ARMCPUClass {
 struct {
 uint32_t c0_cpuid;
 uint32_t c0_cachetype;
+uint32_t c0_clid;
 uint32_t c0_c1[8];
 uint32_t c0_c2[8];
 uint32_t c1_sys;
diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index 850aff1..1a06c52 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -47,6 +47,7 @@ static void arm_cpu_reset(CPUState *c)
 
 /* TODO Move these into arm_cpu_initfn() once no longer zeroed above. */
 env->cp15.c0_cachetype = klass->cp15.c0_cachetype;
+env->cp15.c0_clid = klass->cp15.c0_clid;
 memcpy(env->cp15.c0_c1, klass->cp15.c0_c1, 8 * sizeof(uint32_t));
 memcpy(env->cp15.c0_c2, klass->cp15.c0_c2, 8 * sizeof(uint32_t));
 env->vfp.xregs[ARM_VFP_MVFR0] = klass->vfp.mvfr[0];
@@ -177,6 +178,7 @@ typedef struct ARMCPUInfo {
 const char *name;
 uint32_t id;
 uint32_t cp15_c0_cachetype;
+uint32_t cp15_c0_clid;
 uint32_t cp15_c0_c1[8];
 uint32_t cp15_c0_c2[8];
 uint32_t cp15_c1_sys;
@@ -364,6 +366,7 @@ static const ARMCPUInfo arm_cpus[] = {
 .name = "cortex-a8",
 .id = 0x410fc080,
 .cp15_c0_cachetype = 0x82048004,
+.cp15_c0_clid = (1 << 27) | (2 << 24) | 3,
 .cp15_c0_c1 = {
 0x1031, 0x11, 0x400, 0,
 0x3113, 0x2000, 0x01202000, 0x11
@@ -384,6 +387,7 @@ static const ARMCPUInfo arm_cpus[] = {
 .name = "cortex-a9",
 .id = 0x410fc090,
 .cp15_c0_cachetype = 0x80038003,
+.cp15_c0_clid = (1 << 27) | (1 << 24) | 3,
 .cp15_c0_c1 = {
 0x1031, 0x11, 0x000, 0,
 0x00100103, 0x2000, 0x0123, 0x2111
@@ -410,6 +414,7 @@ static const ARMCPUInfo arm_cpus[] = {
 .name = "cortex-a15",
 .id = 0x412fc0f1,
 .cp15_c0_cachetype = 0x8444c004,
+.cp15_c0_clid = 0x0a200023,
 .cp15_c0_c1 = {
 0x1131, 0x00011011, 0x02010555, 0x,
 0x10201105, 0x2000, 0x0124, 0x02102211
@@ -542,6 +547,7 @@ static void arm_cpu_class_init(ObjectClass *klass, void 
*data)
 
 k->cp15.c0_cpuid = info->id;
 k->cp15.c0_cachetype = info->cp15_c0_cachetype;
+k->cp15.c0_clid = info->cp15_c0_clid;
 memcpy(k->cp15.c0_c1, info->cp15_c0_c1, 8 * sizeof(uint32_t));
 memcpy(k->cp15.c0_c2, info->cp15_c0_c2, 8 * sizeof(uint32_t));
 k->cp15.c1_sys = info->cp15_c1_sys;
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 1f5043c..1e3576c 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -9,18 +9,15 @@ static void cpu_reset_model_id(CPUARMState *env, uint32_t id)
 {
 switch (id) {
 case ARM_CPUID_CORTEXA8:
-env->cp15.c0_clid = (1 << 27) | (2 << 24) | 3;
 env->cp15.c0_ccsid[0] = 0xe007e01a; /* 16k L1 dcache. */
 env->cp15.c0_ccsid[1] = 0x2007e01a; /* 16k L1 icache. */
 env->cp15.c0_ccsid[2] = 0xf000; /* No L2 icache. */
 break;
 case ARM_CPUID_CORTEXA9:
-env->cp15.c0_clid = (1 << 27) | (1 << 24) | 3;
 env->cp15.c0_ccsid[0] = 0xe00fe015; /* 16k L1 dcache. */
 env->cp15.c0_ccsid[1] = 0x200fe015; /* 16k L1 icache. */
 break;
 case ARM_CPUID_CORTEXA15:
-env->cp15.c0_clid = 0x0a200023;
 env->cp15.c0_ccsid[0] = 0x701fe00a; /* 32K L1 dcache */
 env->cp15.c0_ccsid[1] = 0x201fe00a; /* 32K L1 icache */
 env->cp15.c0_ccsid[2] = 0x711fe07a; /* 4096K L2 unified cache */
-- 
1.7.7

[Qemu-devel] [PATCH RFC v4 03/20] target-arm: Embed CPUARMState in QOM ARMCPU

We g_malloc0()'ed CPUARMState ourself, and exec.c's cpu_copy() runs
through cpu_init() as well, so we are at liberty to supply the CPUState
any way we see fit. Having CPUARMState as field in the QOM CPU allows
both to access env from an ARMCPU object and to access the QOM Object
and its ObjectClass from an env pointer, in ARM code for now.

The goal is to convert all CPUs to QOM and to use CPU objects in central
places, especially once we have property support for Object.
This will then allow to have TCG AREG0 point to target-specific fields
where small immediate offsets are desired (as pointed out by rth) while
allowing for common fields at known offsets from the base class.

Having the CPUID in ARMCPUClass, we can set it from the instance_init
function. Same for cpu_model_str, which is now the QOM class name.

Make cpu_state_reset() call cpu_reset() and move most of its code to
arm_cpu_reset().

Signed-off-by: Andreas Färber 
Cc: Anthony Liguori 
Cc: Peter Maydell 
Cc: Richard Henderson 
---
 target-arm/cpu-qom.h |   12 +++
 target-arm/cpu.c |   82 ++
 target-arm/helper.c  |   75 --
 3 files changed, 100 insertions(+), 69 deletions(-)

diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
index b2917ea..9e33b6a 100644
--- a/target-arm/cpu-qom.h
+++ b/target-arm/cpu-qom.h
@@ -21,6 +21,7 @@
 #define QEMU_ARM_CPU_QOM_H
 
 #include "qemu/cpu.h"
+#include "cpu.h"
 
 #define TYPE_ARM_CPU "arm-cpu"
 
@@ -51,6 +52,7 @@ typedef struct ARMCPUClass {
 
 /**
  * ARMCPU:
+ * @env: Legacy CPU state.
  *
  * An ARM CPU core.
  */
@@ -58,7 +60,17 @@ typedef struct ARMCPU {
 /*< private >*/
 CPUState parent_obj;
 /*< public >*/
+
+/* TODO Inline this and split off common state */
+CPUARMState env;
 } ARMCPU;
 
+static inline ARMCPU *arm_env_get_cpu(CPUARMState *env)
+{
+return ARM_CPU(container_of(env, ARMCPU, env));
+}
+
+#define ENV_GET_CPU(e) CPU(arm_env_get_cpu(e))
+
 
 #endif
diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index dabc094..00b7ecc 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -20,12 +20,81 @@
 
 #include "cpu-qom.h"
 #include "qemu-common.h"
+#if !defined(CONFIG_USER_ONLY)
+#include "hw/loader.h"
+#endif
 
 static void arm_cpu_reset(CPUState *c)
 {
 ARMCPUClass *klass = ARM_CPU_GET_CLASS(c);
+ARMCPU *cpu = ARM_CPU(c);
+CPUARMState *env = &cpu->env;
+uint32_t id;
+uint32_t tmp;
+
+if (qemu_loglevel_mask(CPU_LOG_RESET)) {
+qemu_log("CPU Reset (CPU %d)\n", env->cpu_index);
+log_cpu_state(env, 0);
+}
 
 klass->parent_reset(c);
+
+id = env->cp15.c0_cpuid;
+tmp = env->cp15.c15_config_base_address;
+memset(env, 0, offsetof(CPUARMState, breakpoints));
+env->cp15.c0_cpuid = id;
+env->cp15.c15_config_base_address = tmp;
+
+#if defined(CONFIG_USER_ONLY)
+env->uncached_cpsr = ARM_CPU_MODE_USR;
+/* For user mode we must enable access to coprocessors */
+env->vfp.xregs[ARM_VFP_FPEXC] = 1 << 30;
+if (arm_feature(env, ARM_FEATURE_IWMMXT)) {
+env->cp15.c15_cpar = 3;
+} else if (arm_feature(env, ARM_FEATURE_XSCALE)) {
+env->cp15.c15_cpar = 1;
+}
+#else
+/* SVC mode with interrupts disabled.  */
+env->uncached_cpsr = ARM_CPU_MODE_SVC | CPSR_A | CPSR_F | CPSR_I;
+/* On ARMv7-M the CPSR_I is the value of the PRIMASK register, and is
+   clear at reset.  Initial SP and PC are loaded from ROM.  */
+if (IS_M(env)) {
+uint32_t pc;
+uint8_t *rom;
+env->uncached_cpsr &= ~CPSR_I;
+rom = rom_ptr(0);
+if (rom) {
+/* We should really use ldl_phys here, in case the guest
+   modified flash and reset itself.  However images
+   loaded via -kernel have not been copied yet, so load the
+   values directly from there.  */
+env->regs[13] = ldl_p(rom);
+pc = ldl_p(rom + 4);
+env->thumb = pc & 1;
+env->regs[15] = pc & ~1;
+}
+}
+env->vfp.xregs[ARM_VFP_FPEXC] = 0;
+env->cp15.c2_base_mask = 0xc000u;
+/* v7 performance monitor control register: same implementor
+ * field as main ID register, and we implement no event counters.
+ */
+env->cp15.c9_pmcr = (id & 0xff00);
+#endif
+set_flush_to_zero(1, &env->vfp.standard_fp_status);
+set_flush_inputs_to_zero(1, &env->vfp.standard_fp_status);
+set_default_nan_mode(1, &env->vfp.standard_fp_status);
+set_float_detect_tininess(float_tininess_before_rounding,
+  &env->vfp.fp_status);
+set_float_detect_tininess(float_tininess_before_rounding,
+  &env->vfp.standard_fp_status);
+tlb_flush(env, 1);
+/* Reset is a state change for some CPUState fields which we
+ * bake assumptions about into translated code, so we need to
+ * tb_flush().
+ */
+tb_flush(env);
 }
 
 /* CPU m

[Qemu-devel] [PATCH RFC v4 14/20] target-arm: Store VFP MVFR0 and MVFR1 in ARMCPUClass

Signed-off-by: Andreas Färber 
Cc: Peter Maydell 
---
 target-arm/cpu-qom.h |1 +
 target-arm/cpu.c |   11 +++
 target-arm/helper.c  |   29 -
 3 files changed, 12 insertions(+), 29 deletions(-)

diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
index 6cb7891..cd711fa 100644
--- a/target-arm/cpu-qom.h
+++ b/target-arm/cpu-qom.h
@@ -55,6 +55,7 @@ typedef struct ARMCPUClass {
 } cp15;
 struct {
 uint32_t fpsid;
+uint32_t mvfr[2];
 } vfp;
 
 uint32_t features;
diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index ad4a90a..850aff1 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -49,6 +49,8 @@ static void arm_cpu_reset(CPUState *c)
 env->cp15.c0_cachetype = klass->cp15.c0_cachetype;
 memcpy(env->cp15.c0_c1, klass->cp15.c0_c1, 8 * sizeof(uint32_t));
 memcpy(env->cp15.c0_c2, klass->cp15.c0_c2, 8 * sizeof(uint32_t));
+env->vfp.xregs[ARM_VFP_MVFR0] = klass->vfp.mvfr[0];
+env->vfp.xregs[ARM_VFP_MVFR1] = klass->vfp.mvfr[1];
 
 /* Genuine reset */
 env->cp15.c1_sys = klass->cp15.c1_sys;
@@ -179,6 +181,7 @@ typedef struct ARMCPUInfo {
 uint32_t cp15_c0_c2[8];
 uint32_t cp15_c1_sys;
 uint32_t vfp_fpsid;
+uint32_t vfp_mvfr[2];
 uint32_t features;
 void (*class_init)(ARMCPUClass *klass, const struct ARMCPUInfo *info);
 } ARMCPUInfo;
@@ -310,6 +313,7 @@ static const ARMCPUInfo arm_cpus[] = {
 },
 .cp15_c1_sys = 0x00050078,
 .vfp_fpsid = 0x410120b4,
+.vfp_mvfr = { 0x, 0x },
 .features = ARM_FEATURE(V6) |
 ARM_FEATURE(VFP),
 },
@@ -327,6 +331,7 @@ static const ARMCPUInfo arm_cpus[] = {
 },
 .cp15_c1_sys = 0x00050078,
 .vfp_fpsid = 0x410120b5,
+.vfp_mvfr = { 0x, 0x },
 .features = ARM_FEATURE(V6K) |
 ARM_FEATURE(VFP) |
 ARM_FEATURE(VAPA),
@@ -344,6 +349,7 @@ static const ARMCPUInfo arm_cpus[] = {
 0x141, 0, 0, 0
 },
 .vfp_fpsid = 0x410120b4,
+.vfp_mvfr = { 0x, 0x },
 .features = ARM_FEATURE(V6K) |
 ARM_FEATURE(VFP) |
 ARM_FEATURE(VAPA),
@@ -368,6 +374,7 @@ static const ARMCPUInfo arm_cpus[] = {
 },
 .cp15_c1_sys = 0x00c50078,
 .vfp_fpsid = 0x410330c0,
+.vfp_mvfr = { 0x0222, 0x00011100 },
 .features = ARM_FEATURE(V7) |
 ARM_FEATURE(VFP3) |
 ARM_FEATURE(NEON) |
@@ -387,6 +394,7 @@ static const ARMCPUInfo arm_cpus[] = {
 },
 .cp15_c1_sys = 0x00c50078,
 .vfp_fpsid = 0x41033090,
+.vfp_mvfr = { 0x0222, 0x0111 },
 .features = ARM_FEATURE(V7) |
 ARM_FEATURE(VFP3) |
 ARM_FEATURE(VFP_FP16) |
@@ -412,6 +420,7 @@ static const ARMCPUInfo arm_cpus[] = {
 },
 .cp15_c1_sys = 0x00c50078,
 .vfp_fpsid = 0x410430f0,
+.vfp_mvfr = { 0x10110222, 0x },
 .features = ARM_FEATURE(V7) |
 ARM_FEATURE(VFP4) |
 ARM_FEATURE(VFP_FP16) |
@@ -537,6 +546,8 @@ static void arm_cpu_class_init(ObjectClass *klass, void 
*data)
 memcpy(k->cp15.c0_c2, info->cp15_c0_c2, 8 * sizeof(uint32_t));
 k->cp15.c1_sys = info->cp15_c1_sys;
 k->vfp.fpsid = info->vfp_fpsid;
+k->vfp.mvfr[0] = info->vfp_mvfr[0];
+k->vfp.mvfr[1] = info->vfp_mvfr[1];
 k->features = info->features;
 
 if (info->class_init != NULL) {
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 6ea656c..1f5043c 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -8,47 +8,18 @@
 static void cpu_reset_model_id(CPUARMState *env, uint32_t id)
 {
 switch (id) {
-case ARM_CPUID_ARM1136:
-/* This is the 1136 r1, which is a v6K core */
-/* Fall through */
-case ARM_CPUID_ARM1136_R2:
-/* What qemu calls "arm1136_r2" is actually the 1136 r0p2, ie an
- * older core than plain "arm1136". In particular this does not
- * have the v6K features.
- */
-/* These ID register values are correct for 1136 but may be wrong
- * for 1136_r2 (in particular r0p2 does not actually implement most
- * of the ID registers).
- */
-env->vfp.xregs[ARM_VFP_MVFR0] = 0x;
-env->vfp.xregs[ARM_VFP_MVFR1] = 0x;
-break;
-case ARM_CPUID_ARM1176:
-env->vfp.xregs[ARM_VFP_MVFR0] = 0x;
-env->vfp.xregs[ARM_VFP_MVFR1] = 0x;
-break;
-case ARM_CPUID_ARM11MPCORE:
-env->vfp.xregs[ARM_VFP_MVFR0] = 0x;
-env->vfp.xregs[ARM_VFP_MVFR1] = 0x;
-break;
 case ARM_CPUID_CORTEXA8:
-env->vfp.xregs[ARM_VFP_MVFR0] = 0x0222;
-env->vfp.xregs[ARM_VFP_MVFR1] = 0x00011100;
 env->cp15.c0_clid = (1 << 27) | (2 << 24) | 3;

Re: [Qemu-devel] Add support for new image type

2012-03-10 Thread Nate Bushman

If storagecraft disappears the customers still have the existing tools, which 
can, for instance, convert their backup images to vhd or VMDK, so I think this 
is probably not an issue. 

Sent from a phone

On Mar 9, 2012, at 3:28 AM, "Paolo Bonzini"  wrote:

> Il 09/03/2012 10:23, Kevin Wolf ha scritto:
>>> You make really good points on opening up the image file format.
>>> I see real value there in that it would foster development of
>>> other solutions around that format.  The problem in this case is
>>> that StorageCraft's backup image format uses proprietary
>>> compression and encryption, and so, regrettably, we can't
>>> (currently) open up that code.
>> 
>> This is an interesting statement. Not so much for compression, for
>> which I can't know how much better your solution is than common
>> algorithms. But I always thought that closed encryption algorithms
>> have to be considered bad, because using them would mean relying on
>> security by obscurity. Which probably everyone agrees is a bad idea.
> 
> And even for compression, having an open decompressor is usually a good
> idea.  Some people have no problems with proprietary solutions but do
> not like lock in (and wonder what to do with their backups if
> StorageCraft disappears).
> 
> Paolo

[Qemu-devel] [PATCH RFC v4 06/20] target-arm: Move CPU feature flags out of CPUState

The internal CPU feature flags were only ever set in
cpu_reset_model_id(). Therefore move their initialization into
ARMCPUClass. We might want to tweak them in the future though (e.g.,
-cpu cortex-r4,+fpu), so keep a copy in ARMCPU. This in turn means we
need to infer features for both ARMCPUClass and ARMCPU, so move feature
inference to arm_infer_features() and use macros to simplify it.

Since cpu.h defines ARMCPUState, which has been incorporated into
ARMCPU, and tries to use arm_feature() in cpu_get_tb_cpu_state(),
move arm_feature() to cpu-core.h and add a forward declaration.

Signed-off-by: Andreas Färber 
Cc: Peter Maydell 
---
 target-arm/cpu-qom.h |   12 
 target-arm/cpu.c |  159 ++
 target-arm/cpu.h |9 +--
 target-arm/helper.c  |   95 --
 target-arm/machine.c |6 +-
 5 files changed, 177 insertions(+), 104 deletions(-)

diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
index 9e33b6a..6c97337 100644
--- a/target-arm/cpu-qom.h
+++ b/target-arm/cpu-qom.h
@@ -35,6 +35,7 @@
 /**
  * ARMCPUClass:
  * @parent_reset: The parent class' reset handler.
+ * @features: Internal CPU feature flags.
  *
  * An ARM CPU model.
  */
@@ -48,10 +49,13 @@ typedef struct ARMCPUClass {
 struct {
 uint32_t c0_cpuid;
 } cp15;
+
+uint32_t features;
 } ARMCPUClass;
 
 /**
  * ARMCPU:
+ * @features: Internal CPU feature flags.
  * @env: Legacy CPU state.
  *
  * An ARM CPU core.
@@ -61,6 +65,8 @@ typedef struct ARMCPU {
 CPUState parent_obj;
 /*< public >*/
 
+uint32_t features;
+
 /* TODO Inline this and split off common state */
 CPUARMState env;
 } ARMCPU;
@@ -72,5 +78,11 @@ static inline ARMCPU *arm_env_get_cpu(CPUARMState *env)
 
 #define ENV_GET_CPU(e) CPU(arm_env_get_cpu(e))
 
+static inline int arm_feature(CPUARMState *env, int feature)
+{
+ARMCPU *cpu = arm_env_get_cpu(env);
+return (cpu->features & (1u << feature)) != 0;
+}
+
 
 #endif
diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index 097701f..f4c05d8 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -97,14 +97,88 @@ static void arm_cpu_reset(CPUState *c)
 tb_flush(env);
 }
 
+/* CPU feature flags */
+
+#define ARM_FEATURE(x) (1u << ARM_FEATURE_ ## x)
+
+#define has_feature(x) ((*features & ARM_FEATURE(x)) != 0)
+#define set_feature(x) (*features |= ARM_FEATURE(x))
+
+/**
+ * arm_infer_features:
+ * @features: Pointer to the feature flags of #ARMCPUClass or #ARMCPU.
+ *
+ * Some features automatically imply others.
+ */
+static void arm_infer_features(uint32_t *features)
+{
+if (has_feature(V7)) {
+set_feature(VAPA);
+set_feature(THUMB2);
+if (!has_feature(M)) {
+set_feature(V6K);
+} else {
+set_feature(V6);
+}
+}
+if (has_feature(V6K)) {
+set_feature(V6);
+}
+if (has_feature(V6)) {
+set_feature(V5);
+if (!has_feature(M)) {
+set_feature(AUXCR);
+}
+}
+if (has_feature(V5)) {
+set_feature(V4T);
+}
+if (has_feature(M)) {
+set_feature(THUMB_DIV);
+}
+if (has_feature(ARM_DIV)) {
+set_feature(THUMB_DIV);
+}
+if (has_feature(VFP4)) {
+set_feature(VFP3);
+}
+if (has_feature(VFP3)) {
+set_feature(VFP);
+}
+}
+
+#undef has_feature
+#undef set_feature
+
+static inline void set_class_feature(ARMCPUClass *klass, int feature)
+{
+klass->features |= 1u << feature;
+}
+
+static inline void unset_class_feature(ARMCPUClass *klass, int feature)
+{
+klass->features &= ~(1u << feature);
+}
+
 /* CPU models */
 
 typedef struct ARMCPUInfo {
 const char *name;
 uint32_t id;
+uint32_t features;
 void (*class_init)(ARMCPUClass *klass, const struct ARMCPUInfo *info);
 } ARMCPUInfo;
 
+static void arm1136_r0_class_init(ARMCPUClass *k, const ARMCPUInfo *info)
+{
+ARMCPUClass *r1_class;
+
+r1_class = ARM_CPU_CLASS(object_class_by_name("arm1136"));
+
+k->features = r1_class->features;
+unset_class_feature(k, ARM_FEATURE_V6K);
+}
+
 static void ti925t_reset(CPUState *c)
 {
 ARMCPU *cpu = ARM_CPU(c);
@@ -122,18 +196,43 @@ static void ti925t_class_init(ARMCPUClass *klass, const 
ARMCPUInfo *info)
 cpu_class->reset = ti925t_reset;
 }
 
+static void sa11xx_class_init(ARMCPUClass *k, const ARMCPUInfo *info)
+{
+set_class_feature(k, ARM_FEATURE_STRONGARM);
+}
+
+static void pxa25x_class_init(ARMCPUClass *k, const ARMCPUInfo *info)
+{
+set_class_feature(k, ARM_FEATURE_V5);
+set_class_feature(k, ARM_FEATURE_XSCALE);
+}
+
+static void pxa270_class_init(ARMCPUClass *k, const ARMCPUInfo *info)
+{
+set_class_feature(k, ARM_FEATURE_V5);
+set_class_feature(k, ARM_FEATURE_XSCALE);
+set_class_feature(k, ARM_FEATURE_IWMMXT);
+}
+
 static const ARMCPUInfo arm_cpus[] = {
 {
 .name = "arm926",
 .id = 0x41069265,
+.features = ARM_FEATURE(V5) |
+

[Qemu-devel] [PATCH v2 RESEND 01/20] qom: Introduce object_class_get_list()

This function allows to obtain a singly-linked list of classes, which
can be sorted by the caller.

Signed-off-by: Andreas Färber 
Cc: Anthony Liguori 
---
 include/qemu/object.h |   11 +++
 qom/object.c  |   17 +
 2 files changed, 28 insertions(+), 0 deletions(-)

diff --git a/include/qemu/object.h b/include/qemu/object.h
index ec2d294..e8fc126 100644
--- a/include/qemu/object.h
+++ b/include/qemu/object.h
@@ -560,6 +560,17 @@ ObjectClass *object_class_by_name(const char *typename);
 void object_class_foreach(void (*fn)(ObjectClass *klass, void *opaque),
   const char *implements_type, bool include_abstract,
   void *opaque);
+
+/**
+ * object_class_get_list:
+ * @implements_type: The type to filter for, including its derivatives.
+ * @include_abstract: Whether to include abstract classes.
+ *
+ * Returns: A singly-linked list of the classes in reverse hashtable order.
+ */
+GSList *object_class_get_list(const char *implements_type,
+  bool include_abstract);
+
 /**
  * object_ref:
  * @obj: the object
diff --git a/qom/object.c b/qom/object.c
index aa037d2..eef0b22 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -572,6 +572,23 @@ void object_class_foreach(void (*fn)(ObjectClass *klass, 
void *opaque),
 g_hash_table_foreach(type_table_get(), object_class_foreach_tramp, &data);
 }
 
+static void object_class_get_list_tramp(ObjectClass *klass, void *opaque)
+{
+GSList **list = opaque;
+
+*list = g_slist_prepend(*list, klass);
+}
+
+GSList *object_class_get_list(const char *implements_type,
+  bool include_abstract)
+{
+GSList *list = NULL;
+
+object_class_foreach(object_class_get_list_tramp,
+ implements_type, include_abstract, &list);
+return list;
+}
+
 void object_ref(Object *obj)
 {
 obj->ref++;
-- 
1.7.7

Re: [Qemu-devel] Add support for new image type

2012-03-10 Thread Nate Bushman

The encryption is speed-optimized AES256. The compression is proprietary, as is 
common for backup image files, however in hindsight I agree with you. I too 
would prefer for it to all be open. It's possible we maybe able to make such a 
shift. Thanks for your thoughts and suggestions. 

Sent from a phone

On Mar 9, 2012, at 2:19 AM, "Kevin Wolf"  wrote:

> Am 08.03.2012 18:16, schrieb Nate Bushman:
>> You make really good points on opening up the image file format.  I see real 
>> value there in that it would foster development of other solutions around 
>> that format.  The problem in this case is that StorageCraft's backup image 
>> format uses proprietary compression and encryption, and so, regrettably, we 
>> can't (currently) open up that code. 
> 
> This is an interesting statement. Not so much for compression, for which
> I can't know how much better your solution is than common algorithms.
> But I always thought that closed encryption algorithms have to be
> considered bad, because using them would mean relying on security by
> obscurity. Which probably everyone agrees is a bad idea.
> 
>> This is an interesting lesson in retrospect.  We could possibly publish a 
>> spec of the format when encryption and compression aren't used, but in 
>> practice this doesn't happen (compression speeds up the backups because the 
>> CPU time it takes to compress is far less than the time saved by writing the 
>> smaller amount of compressed data to the disk), so because compression saves 
>> both time and space nearly all of our customers use the compression options 
>> at a minimum, and many also use the encryption.
> 
> I see. Though it probably depends on your use case for qemu with these
> images. If you create an image specifically for use with qemu, it could
> make sense to use uncompressed images (or possibly even introduce
> another zlib-compressed mode or something). On the other hand, if qemu
> was to be used to work with already existing images, it wouldn't help.
> 
> Kevin
> 
>> -Original Message-
>> From: Kevin Wolf [mailto:kw...@redhat.com] 
>> Sent: Thursday, March 08, 2012 8:18 AM
>> To: Kai Meyer
>> Cc: qemu-devel@nongnu.org; Nate Bushman
>> Subject: Re: [Qemu-devel] Add support for new image type
>> 
>> (Sorry, missed the rest of this thread as I was expecting to be kept in the 
>> CC list)
>> 
>> Am 01.03.2012 20:45, schrieb Kai Meyer:
>>> On 03/01/2012 08:03 AM, Kevin Wolf wrote:
 Am 29.02.2012 22:52, schrieb Kai Meyer:
> Is it possible to extend qemu to support a new image type? I have an 
> image type that is ready for consumption and I'm looking for the 
> integration point between qemu and the new image format.
 Which image format do you want to get integrated?
 
 Have a look at block/qcow2.c to get an idea of what a qemu block 
 driver looks like. At the bottom of the file there is a struct that 
 contains function pointers to all exported functions, so this is 
 usually a good place to start exploring a driver.
 
 Kevin
>>> 
>>> Great, this is exactly what we're after. I work for StorageCraft, and 
>>> we would like to figure out some way to allow qemu to directly consume 
>>> our image-based backups. It would provide us with user-space mounting 
>>> (via
>>> libguestfs) as well as booting VMs directly from Backup images. We 
>>> already have a proprietary image access library that provides 
>>> block-wise access to our image files. I have been able to scratch 
>>> together a proof of concept already, which I am really pleased with.
>>> 
>>> I see only two roadblocks for which I don't have immediate answers for.
>>> 
>>> 1) Licensing
>>> Is it possible to license our contributions in such a way that we do 
>>> not need to open the source code of our image access library?
>> 
>> As other people already said, there's no way that we would accept this.
>> 
>> However, what do you really gain from keeping your file format secret? I 
>> can't imagine that the information required for a (possibly read-only) 
>> properly licensed block driver contains anything of what makes the core of 
>> your products. I can understand that you want to keep sophisticated source 
>> code closed, but I really can't for file formats.
>> 
>> So if you tried to get permission to publish a specification of your image 
>> format and wrote a minimal qemu block driver from scratch, I believe this 
>> would be best for everyone.
>> 
>>> 2) External dependency on our image access library.
>>> We do not want to force qemu to require our image access library to be 
>>> present to build. Would it be better to do a conditional build 
>>> (./configure --with-spf) or a run-time check for our image access library?
>> 
>> A conditional build would be required and I'd consider a run-time check 
>> optional.
>> 
>> Kevin
>

[Qemu-devel] [PATCH 3/5] Export qemu_sendv_recvv() and use it in (inlined) qemu_sendv() and qemu_recvv()

Rename do_sendv_recvv() to qemu_sendv_recvv(),
change its last arg (do_send) from int to bool,
export it in qemu-common.h, and made the two
callers of it (qemu_sendv() and qemu_recvv())
to be trivial #defines just adding 5th arg.

qemu_sendv_recvv() will be used later.

Signed-off-by: Michael Tokarev 
---
 cutils.c  |   15 ++-
 qemu-common.h |   14 --
 2 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/cutils.c b/cutils.c
index 89305ad..6d9175f 100644
--- a/cutils.c
+++ b/cutils.c
@@ -426,8 +426,8 @@ int qemu_parse_fd(const char *param)
  * The first `offset' bytes in the iovec buffer are skipped and next
  * `bytes' bytes are used.
  */
-static int do_sendv_recvv(int sockfd, struct iovec *iov, size_t bytes, size_t 
offset,
-  int do_sendv)
+int qemu_sendv_recvv(int sockfd, struct iovec *iov, size_t bytes, size_t 
offset,
+bool do_sendv)
 {
 int ret, iovlen;
 size_t diff;
@@ -506,14 +506,3 @@ static int do_sendv_recvv(int sockfd, struct iovec *iov, 
size_t bytes, size_t of
 last_iov->iov_len += diff;
 return ret;
 }
-
-int qemu_recvv(int sockfd, struct iovec *iov, size_t bytes, size_t offset)
-{
-return do_sendv_recvv(sockfd, iov, bytes, offset, 0);
-}
-
-int qemu_sendv(int sockfd, struct iovec *iov, size_t bytes, size_t offset)
-{
-return do_sendv_recvv(sockfd, iov, bytes, offset, 1);
-}
-
diff --git a/qemu-common.h b/qemu-common.h
index c0536b3..474cbcc 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -199,8 +199,18 @@ int qemu_pipe(int pipefd[2]);
 #define qemu_recv(sockfd, buf, len, flags) recv(sockfd, buf, len, flags)
 #endif
 
-int qemu_recvv(int sockfd, struct iovec *iov, size_t bytes, size_t offset);
-int qemu_sendv(int sockfd, struct iovec *iov, size_t bytes, size_t offset);
+/**
+ * Send or receive data from/to an (optionally partial) iovec.
+ * Instead of processing whole iovector, this routine can process
+ * not more than a specified number of bytes, and start not at
+ * the beginning of iovec but at byte position `offset'.
+ */
+int qemu_sendv_recvv(int sockfd, struct iovec *iov,
+size_t bytes, size_t offset, bool do_sendv);
+#define qemu_recvv(sockfd, iov, bytes, offset) \
+  qemu_sendv_recvv(sockfd, iov, bytes, offset, false)
+#define qemu_sendv(sockfd, iov, bytes, offset) \
+  qemu_sendv_recvv(sockfd, iov, bytes, offset, true)
 
 /* Error handling.  */
 
-- 
1.7.9.1

[Qemu-devel] [PATCH 5/5] rewrite and comment qemu_sendv_recvv()

Make it much more understandable, and add comments to it.
This is in order to prepare similar function for writev_readv().

The new implementation has been extensively tested by
splitting a large buffer into many small randomly-sized
chunks, sending it over socket to another, slow process
and verifying the receiving data is the same.

Signed-off-by: Michael Tokarev 
---
 cutils.c  |  119 +---
 qemu-common.h |2 +-
 2 files changed, 63 insertions(+), 58 deletions(-)

diff --git a/cutils.c b/cutils.c
index 6d9175f..e77bbda 100644
--- a/cutils.c
+++ b/cutils.c
@@ -426,83 +426,88 @@ int qemu_parse_fd(const char *param)
  * The first `offset' bytes in the iovec buffer are skipped and next
  * `bytes' bytes are used.
  */
-int qemu_sendv_recvv(int sockfd, struct iovec *iov, size_t bytes, size_t 
offset,
-bool do_sendv)
+int qemu_sendv_recvv(int sockfd, struct iovec *iov,
+size_t bytes, size_t offset, bool do_send)
 {
-int ret, iovlen;
-size_t diff;
-struct iovec *last_iov;
-
-/* last_iov is inclusive, so count from one.  */
-iovlen = 1;
-last_iov = iov;
-bytes += offset;
-
-while (last_iov->iov_len < bytes) {
-bytes -= last_iov->iov_len;
-
-last_iov++;
-iovlen++;
+int ret;
+struct iovec *end;
+
+/* Find the start position, skipping `offset' bytes:
+ * first, skip all full-sized vector elements, */
+while(offset >= iov->iov_len) {
+   offset -= iov->iov_len;
+   ++iov;
 }
-
-diff = last_iov->iov_len - bytes;
-last_iov->iov_len -= diff;
-
-while (iov->iov_len <= offset) {
-offset -= iov->iov_len;
-
-iov++;
-iovlen--;
+if (offset) {
+   /* second, skip `offset' bytes from the (now) first element,
+* undo it on exit */
+   iov->iov_base += offset;
+   iov->iov_len -= offset;
+}
+/* Find the end position skipping `bytes' bytes: */
+/* first, skip all full-sized elements */
+end = iov;
+while(end->iov_len <= bytes) {
+   bytes -= end->iov_len;
+   ++end;
+}
+if (bytes) {
+   /* second, fixup the last element, and remember
+* the length we've cut from the end of it in `bytes' */
+   size_t tail = end->iov_len - bytes;
+   end->iov_len = bytes;
+   ++end;
+   bytes = tail;
 }
-
-iov->iov_base = (char *) iov->iov_base + offset;
-iov->iov_len -= offset;
 
 {
 #if defined CONFIG_IOVEC && defined CONFIG_POSIX
 struct msghdr msg;
 memset(&msg, 0, sizeof(msg));
 msg.msg_iov = iov;
-msg.msg_iovlen = iovlen;
-
+msg.msg_iovlen = end - iov;;
 do {
-if (do_sendv) {
-ret = sendmsg(sockfd, &msg, 0);
-} else {
-ret = recvmsg(sockfd, &msg, 0);
-}
-} while (ret == -1 && errno == EINTR);
+   ret = do_send
+   ? sendmsg(sockfd, &msg, 0)
+: recvmsg(sockfd, &msg, 0);
+} while (ret < 0 && errno == EINTR);
 #else
-struct iovec *p = iov;
+   /* else send piece-by-piece */
+   /*XXX Note: windows has WSASend() and WSARecv() */
+   struct iovec *p = iov;
 ret = 0;
-while (iovlen > 0) {
-int rc;
-if (do_sendv) {
-rc = send(sockfd, p->iov_base, p->iov_len, 0);
-} else {
-rc = qemu_recv(sockfd, p->iov_base, p->iov_len, 0);
-}
-if (rc == -1) {
-if (errno == EINTR) {
-continue;
-}
+   while(p < end) {
+   int r = do_send
+   ? send(sockfd, p->iov_base, p->iov_len, 0)
+   : qemu_recv(sockfd, p->iov_base, p->iov_len, 0);
+   if (r > 0) {
+   ret += r;
+   ++i;
+   } else if (!r) {
+   break;
+   } else if (errno == EINTR) {
+   continue;
+   } else {
+   /* else it is some "other" error,
+* only return if there was no data processed. */
 if (ret == 0) {
 ret = -1;
 }
 break;
-}
-if (rc == 0) {
-break;
-}
-ret += rc;
-iovlen--, p++;
+   }
 }
 #endif
 }
 
 /* Undo the changes above */
-iov->iov_base = (char *) iov->iov_base - offset;
-iov->iov_len += offset;
-last_iov->iov_len += diff;
+if (offset) {
+   iov->iov_base -= offset;
+   iov->iov_len += offset;
+}
+if (bytes) {
+   --end;
+   end->iov_len += bytes;
+}
+
 return ret;
 }
diff --git a/qemu-common.h b/qemu-common.h
index 2bf630d..17d5321 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -206,7 +206,7 @@ int qemu_pipe(int pipefd[2]);
  * the beginning of iovec but at byte position

[Qemu-devel] [PATCH 4/5] cleanup qemu_co_sendv(), qemu_co_recvv() and friends

The same as for non-coroutine versions in previous patches:
rename arguments to be more obvious, change type of arguments
from int to size_t where appropriate, and use common code for
send and receive paths (with one extra argument) since these
are exactly the same.  Use common qemu_sendv_recvv() directly.
Also constify buf arg of qemu_co_send().

qemu_co_sendv(), qemu_co_recvv(), and qemu_co_recv() are now
trivial #define's merely adding one extra arg.  qemu_co_send()
is an inline function due to `buf' arg de-constification.

Signed-off-by: Michael Tokarev 
---
 qemu-common.h   |   37 +++---
 qemu-coroutine-io.c |   83 +-
 2 files changed, 40 insertions(+), 80 deletions(-)

diff --git a/qemu-common.h b/qemu-common.h
index 474cbcc..2bf630d 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -306,31 +306,30 @@ struct qemu_work_item {
 void qemu_init_vcpu(void *env);
 #endif
 
-/**
- * Sends an iovec (or optionally a part of it) down a socket, yielding
- * when the socket is full.
- */
-int qemu_co_sendv(int sockfd, struct iovec *iov,
-  int len, int iov_offset);
-
-/**
- * Receives data into an iovec (or optionally into a part of it) from
- * a socket, yielding when there is no data in the socket.
- */
-int qemu_co_recvv(int sockfd, struct iovec *iov,
-  int len, int iov_offset);
-
 
 /**
- * Sends a buffer down a socket, yielding when the socket is full.
+ * Sends a (part of) iovec down a socket, yielding when the socket is full, or
+ * Receives data into a (part of) iovec from a socket,
+ * yielding when there is no data in the socket.
+ * XXX should mark these as coroutine_fn
  */
-int qemu_co_send(int sockfd, void *buf, int len);
+int qemu_co_sendv_recvv(int sockfd, struct iovec *iov,
+   size_t bytes, size_t offset, bool do_send);
+#define qemu_co_recvv(sockfd, iov, bytes, offset) \
+  qemu_co_sendv_recvv(sockfd, iov, bytes, offset, false)
+#define qemu_co_sendv(sockfd, iov, bytes, offset) \
+  qemu_co_sendv_recvv(sockfd, iov, bytes, offset, true)
 
 /**
- * Receives data into a buffer from a socket, yielding when there
- * is no data in the socket.
+ * The same as above, but with just a single buffer
  */
-int qemu_co_recv(int sockfd, void *buf, int len);
+int qemu_co_send_recv(int sockfd, void *buf, size_t bytes, bool do_send);
+#define qemu_co_recv(sockfd, buf, bytes) \
+  qemu_co_send_recv(sockfd, buf, bytes, false)
+static inline int qemu_co_send(int sockfd, const void *buf, size_t bytes)
+{
+return qemu_co_send_recv(sockfd, (void*)buf, bytes, true);
+}
 
 
 typedef struct QEMUIOVector {
diff --git a/qemu-coroutine-io.c b/qemu-coroutine-io.c
index 40fd514..5631c84 100644
--- a/qemu-coroutine-io.c
+++ b/qemu-coroutine-io.c
@@ -26,71 +26,32 @@
 #include "qemu_socket.h"
 #include "qemu-coroutine.h"
 
-int coroutine_fn qemu_co_recvv(int sockfd, struct iovec *iov,
-   int len, int iov_offset)
+int coroutine_fn
+qemu_co_sendv_recvv(int sockfd, struct iovec *iov,
+   size_t bytes, size_t offset, bool do_send)
 {
-int total = 0;
+size_t done = 0;
 int ret;
-while (len) {
-ret = qemu_recvv(sockfd, iov, len, iov_offset + total);
-if (ret < 0) {
-if (errno == EAGAIN) {
-qemu_coroutine_yield();
-continue;
-}
-if (total == 0) {
-total = -1;
-}
-break;
-}
-if (ret == 0) {
-break;
-}
-total += ret, len -= ret;
+while (done < bytes) {
+ret = qemu_sendv_recvv(sockfd, iov, bytes - done, offset + done, 
do_send);
+   if (ret > 0) {
+   done += ret;
+   } else if (!ret) {
+   break;
+   } else if (errno == EAGAIN) {
+   qemu_coroutine_yield();
+   } else if (done == 0) {
+   return -1;
+   } else {
+   break;
+   }
 }
-
-return total;
-}
-
-int coroutine_fn qemu_co_sendv(int sockfd, struct iovec *iov,
-   int len, int iov_offset)
-{
-int total = 0;
-int ret;
-while (len) {
-ret = qemu_sendv(sockfd, iov, len, iov_offset + total);
-if (ret < 0) {
-if (errno == EAGAIN) {
-qemu_coroutine_yield();
-continue;
-}
-if (total == 0) {
-total = -1;
-}
-break;
-}
-total += ret, len -= ret;
-}
-
-return total;
+return done;
 }
 
-int coroutine_fn qemu_co_recv(int sockfd, void *buf, int len)
+int coroutine_fn
+qemu_co_send_recv(int sockfd, void *buf, size_t bytes, bool do_send)
 {
-struct iovec iov;
-
-iov.iov_base = buf;
-iov.iov_len = len;
-
-return qemu_co_recvv(sockfd, &iov, len, 0);
-}
-
-int coroutine_fn qemu_co_send(int sockfd, void *buf, int len)
-{
-struct iovec iov;
-
-iov.iov_ba

[Qemu-devel] [PATCH 2/5] change prototypes of qemu_sendv() and qemu_recvv()

Rename arguments and use size_t for sizes instead of int

-qemu_sendv(int sockfd, struct iovec *iov, int len, int iov_offset)
+qemu_sendv(int sockfd, struct iovec *iov, size_t bytes, size_t offset)

The main motivation was to make it clear that length and offset are
in _bytes_, not in iov elements: it was very confusing before, because
all standard functions which deals with iovecs expects number of iovs,
not bytes, even the fact that struct iovec has iov_len and iov_ prefix
does not help.  With "bytes" and "offset", especially since they're
now size_t, it is much more explicit.

All callers of qemu_sendv() and qemu_recvv() and related, like
qemu_co_sendv() and qemu_co_recvv(), were checked to verify that
it is safe to use unsigned datatype instead of int.

While at it, clean up misleading comment near do_sendv_recvv().

Signed-off-by: Michael Tokarev 
---
 cutils.c  |   34 --
 qemu-common.h |4 ++--
 2 files changed, 14 insertions(+), 24 deletions(-)

diff --git a/cutils.c b/cutils.c
index 27687c8..89305ad 100644
--- a/cutils.c
+++ b/cutils.c
@@ -424,38 +424,28 @@ int qemu_parse_fd(const char *param)
  *
  * This function send/recv data from/to the iovec buffer directly.
  * The first `offset' bytes in the iovec buffer are skipped and next
- * `len' bytes are used.
- *
- * For example,
- *
- *   do_sendv_recvv(sockfd, iov, len, offset, 1);
- *
- * is equal to
- *
- *   char *buf = malloc(size);
- *   iov_to_buf(iov, iovcnt, buf, offset, size);
- *   send(sockfd, buf, size, 0);
- *   free(buf);
+ * `bytes' bytes are used.
  */
-static int do_sendv_recvv(int sockfd, struct iovec *iov, int len, int offset,
+static int do_sendv_recvv(int sockfd, struct iovec *iov, size_t bytes, size_t 
offset,
   int do_sendv)
 {
-int ret, diff, iovlen;
+int ret, iovlen;
+size_t diff;
 struct iovec *last_iov;
 
 /* last_iov is inclusive, so count from one.  */
 iovlen = 1;
 last_iov = iov;
-len += offset;
+bytes += offset;
 
-while (last_iov->iov_len < len) {
-len -= last_iov->iov_len;
+while (last_iov->iov_len < bytes) {
+bytes -= last_iov->iov_len;
 
 last_iov++;
 iovlen++;
 }
 
-diff = last_iov->iov_len - len;
+diff = last_iov->iov_len - bytes;
 last_iov->iov_len -= diff;
 
 while (iov->iov_len <= offset) {
@@ -517,13 +507,13 @@ static int do_sendv_recvv(int sockfd, struct iovec *iov, 
int len, int offset,
 return ret;
 }
 
-int qemu_recvv(int sockfd, struct iovec *iov, int len, int iov_offset)
+int qemu_recvv(int sockfd, struct iovec *iov, size_t bytes, size_t offset)
 {
-return do_sendv_recvv(sockfd, iov, len, iov_offset, 0);
+return do_sendv_recvv(sockfd, iov, bytes, offset, 0);
 }
 
-int qemu_sendv(int sockfd, struct iovec *iov, int len, int iov_offset)
+int qemu_sendv(int sockfd, struct iovec *iov, size_t bytes, size_t offset)
 {
-return do_sendv_recvv(sockfd, iov, len, iov_offset, 1);
+return do_sendv_recvv(sockfd, iov, bytes, offset, 1);
 }
 
diff --git a/qemu-common.h b/qemu-common.h
index b7e426e..c0536b3 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -199,8 +199,8 @@ int qemu_pipe(int pipefd[2]);
 #define qemu_recv(sockfd, buf, len, flags) recv(sockfd, buf, len, flags)
 #endif
 
-int qemu_recvv(int sockfd, struct iovec *iov, int len, int iov_offset);
-int qemu_sendv(int sockfd, struct iovec *iov, int len, int iov_offset);
+int qemu_recvv(int sockfd, struct iovec *iov, size_t bytes, size_t offset);
+int qemu_sendv(int sockfd, struct iovec *iov, size_t bytes, size_t offset);
 
 /* Error handling.  */
 
-- 
1.7.9.1

[Qemu-devel] [PATCH 1/5] Consolidate qemu_iovec_memset{, _skip}() into single, simplified function

This patch combines two functions into one, simplifies the
implementation and adds some assert()s into place.

Signed-off-by: Michael Tokarev 
---
 block/qcow2.c  |4 +-
 block/qed.c|4 +-
 cutils.c   |   54 +++
 linux-aio.c|2 +-
 posix-aio-compat.c |2 +-
 qemu-common.h  |4 +--
 6 files changed, 23 insertions(+), 47 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index eb5ea48..b39bdcc 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -406,7 +406,7 @@ int qcow2_backing_read1(BlockDriverState *bs, QEMUIOVector 
*qiov,
 else
 n1 = bs->total_sectors - sector_num;
 
-qemu_iovec_memset_skip(qiov, 0, 512 * (nb_sectors - n1), 512 * n1);
+qemu_iovec_memset(qiov, 0, 512 * (nb_sectors - n1), 512 * n1);
 
 return n1;
 }
@@ -466,7 +466,7 @@ static coroutine_fn int qcow2_co_readv(BlockDriverState 
*bs, int64_t sector_num,
 }
 } else {
 /* Note: in this case, no need to wait */
-qemu_iovec_memset(&hd_qiov, 0, 512 * cur_nr_sectors);
+qemu_iovec_memset(&hd_qiov, 0, 512 * cur_nr_sectors, 0);
 }
 } else if (cluster_offset & QCOW_OFLAG_COMPRESSED) {
 /* add AIO support for compressed blocks ? */
diff --git a/block/qed.c b/block/qed.c
index a041d31..427cf13 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -738,7 +738,7 @@ static void qed_read_backing_file(BDRVQEDState *s, uint64_t 
pos,
 /* Zero all sectors if reading beyond the end of the backing file */
 if (pos >= backing_length ||
 pos + qiov->size > backing_length) {
-qemu_iovec_memset(qiov, 0, qiov->size);
+qemu_iovec_memset(qiov, 0, qiov->size, 0);
 }
 
 /* Complete now if there are no backing file sectors to read */
@@ -1253,7 +1253,7 @@ static void qed_aio_read_data(void *opaque, int ret,
 
 /* Handle zero cluster and backing file reads */
 if (ret == QED_CLUSTER_ZERO) {
-qemu_iovec_memset(&acb->cur_qiov, 0, acb->cur_qiov.size);
+qemu_iovec_memset(&acb->cur_qiov, 0, acb->cur_qiov.size, 0);
 qed_aio_next_io(acb, 0);
 return;
 } else if (ret != QED_CLUSTER_FOUND) {
diff --git a/cutils.c b/cutils.c
index af308cd..27687c8 100644
--- a/cutils.c
+++ b/cutils.c
@@ -260,46 +260,24 @@ void qemu_iovec_from_buffer(QEMUIOVector *qiov, const 
void *buf, size_t count)
 }
 }
 
-void qemu_iovec_memset(QEMUIOVector *qiov, int c, size_t count)
+void qemu_iovec_memset(QEMUIOVector *qiov, int c, size_t bytes, size_t offset)
 {
-size_t n;
-int i;
-
-for (i = 0; i < qiov->niov && count; ++i) {
-n = MIN(count, qiov->iov[i].iov_len);
-memset(qiov->iov[i].iov_base, c, n);
-count -= n;
+struct iovec *iov = qiov->iov;
+unsigned i;
+assert(qiov->size >= offset);
+assert(qiov->size - offset >= bytes);
+
+/* first skip initial full-sized elements */
+for(i = 0; offset >= iov[i].iov_len; ++i) {
+   offset -= iov[i].iov_len;
 }
-}
-
-void qemu_iovec_memset_skip(QEMUIOVector *qiov, int c, size_t count,
-size_t skip)
-{
-int i;
-size_t done;
-void *iov_base;
-uint64_t iov_len;
-
-done = 0;
-for (i = 0; (i < qiov->niov) && (done != count); i++) {
-if (skip >= qiov->iov[i].iov_len) {
-/* Skip the whole iov */
-skip -= qiov->iov[i].iov_len;
-continue;
-} else {
-/* Skip only part (or nothing) of the iov */
-iov_base = (uint8_t*) qiov->iov[i].iov_base + skip;
-iov_len = qiov->iov[i].iov_len - skip;
-skip = 0;
-}
-
-if (done + iov_len > count) {
-memset(iov_base, c, count - done);
-break;
-} else {
-memset(iov_base, c, iov_len);
-}
-done += iov_len;
+/* skip/memset partial element and memset the rest */
+while(bytes) {
+   size_t n = MIN(bytes, iov[i].iov_len - offset);
+   memset(iov[i].iov_base + offset, c, n);
+   bytes -= n;
+   ++i;
+   offset = 0;
 }
 }
 
diff --git a/linux-aio.c b/linux-aio.c
index d2fc2e7..303f410 100644
--- a/linux-aio.c
+++ b/linux-aio.c
@@ -64,7 +64,7 @@ static void qemu_laio_process_completion(struct 
qemu_laio_state *s,
 } else if (ret >= 0) {
 /* Short reads mean EOF, pad with zeros. */
 if (laiocb->is_read) {
-qemu_iovec_memset_skip(laiocb->qiov, 0,
+qemu_iovec_memset(laiocb->qiov, 0,
 laiocb->qiov->size - ret, ret);
 } else {
 ret = -EINVAL;
diff --git a/posix-aio-compat.c b/posix-aio-compat.c
index d311d13..84aafed 100644
--- a/posix-aio-compat.c
+++ b/posix-aio-compat.c
@@ -355,7 +355,7 @@ static void *aio_thread(void *unused)
 
 qemu_iovec_init_external(&qiov, aiocb->aio_iov,

[Qemu-devel] [PATCH 0/5] cleanup/consolidate some iovec functions

This is a little cleanup/consolidation for some iovec-related
low-level routines in qemu.

First patch consolidates qemu_iovec_memset_skip() and
qemu_iovec_memset(), which is just a little cleanup.

The rest renames arguments of iovec I/O functions to be
more understandable/obvious (see comments in patch 2),
changes the types to be size_t instead of int, and
consolidates send and recv paths into one (with one
extra argument, do_send), since the code is exactly
the same.  This also lets us to use common read/write
code paths in upper layer.

Michael Tokarev (5):
  Consolidate qemu_iovec_memset{,_skip}() into single, simplified function
  change prototypes of qemu_sendv() and qemu_recvv()
  Export qemu_sendv_recvv() and use it in (inlined) qemu_sendv() and 
qemu_recvv()
  cleanup qemu_co_sendv(), qemu_co_recvv() and friends
  rewrite and comment qemu_sendv_recvv()

 block/qcow2.c   |4 +-
 block/qed.c |4 +-
 cutils.c|  196 +--
 linux-aio.c |2 +-
 posix-aio-compat.c  |2 +-
 qemu-common.h   |   55 --
 qemu-coroutine-io.c |   83 ++
 7 files changed, 138 insertions(+), 208 deletions(-)

-- 
1.7.9.1

Re: [Qemu-devel] [PATCH] configure: Test for libiberty.a (mingw32)

Am 10.03.2012 11:14, schrieb Stefan Weil:
> MinGW-w64 and some versions of MinGW32 don't provide libiberty.a,
> so add this library only if it was found.

What do we need it for at all then?

Andreas

> Signed-off-by: Stefan Weil 
> ---
>  configure |8 +++-
>  1 files changed, 7 insertions(+), 1 deletions(-)
> 
> diff --git a/configure b/configure
> index ca25250..bb16498 100755
> --- a/configure
> +++ b/configure
> @@ -511,7 +511,13 @@ if test "$mingw32" = "yes" ; then
>QEMU_CFLAGS="-DWIN32_LEAN_AND_MEAN -DWINVER=0x501 $QEMU_CFLAGS"
># enable C99/POSIX format strings (needs mingw32-runtime 3.15 or later)
>QEMU_CFLAGS="-D__USE_MINGW_ANSI_STDIO=1 $QEMU_CFLAGS"
> -  LIBS="-lwinmm -lws2_32 -liberty -liphlpapi $LIBS"
> +  LIBS="-lwinmm -lws2_32 -liphlpapi $LIBS"
> +cat > $TMPC << EOF
> +int main(void) { return 0; }
> +EOF
> +  if compile_prog "" "-liberty" ; then
> +LIBS="-liberty $LIBS"
> +  fi
>prefix="c:/Program Files/Qemu"
>mandir="\${prefix}"
>datadir="\${prefix}"

-- 
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] w64: Don't redefine lseek, ftruncate


Am 10.03.2012 13:11, schrieb Andreas Färber:

Am 10.03.2012 11:14, schrieb Stefan Weil:
   

MinGW-w64 already defines lseek and ftruncate (and uses the 64 bit
variants). The conditional compilation avoids redefinitions
(which would be wrong) and compiler warnings.

Signed-off-by: Stefan Weil
---
  qemu-common.h |8 ++--
  1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/qemu-common.h b/qemu-common.h
index dbfce6f..b0fdf5c 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -93,9 +93,13 @@ typedef int (*fprintf_function)(FILE *f, const char *fmt, 
...)

  #ifdef _WIN32
  #define fsync _commit
-#define lseek _lseeki64
+#if !defined(lseek)
+# define lseek _lseeki64
+#endif
  int qemu_ftruncate64(int, int64_t);
-#define ftruncate qemu_ftruncate64
+#if !defined(ftruncate)
+# define ftruncate qemu_ftruncate64
+#endif
 

lseek looks okay to me, but did you check that ftruncate and
qemu_ftruncate64 behave the same?

Andreas
   


qemu_ftruncate64 was added in 1996. It is only used for MinGW32,
and maybe not even needed there, because it is simply a 64 bit
implementation of ftruncate. Eliminating qemu_ftruncate64
might be handled in a separate patch.

The MinGW32 and MinGW-w64 functions ftruncate64 should be sufficient,
and MinGW-w64 uses #define ftruncate ftruncate64.

Regards,
Stefan

Re: [Qemu-devel] QEMU for begineers

2012-03-10 Thread Jaspal Dhillon

Hi Avinash ,

These links contain enough information for you to get started :

http://wiki.qemu.org/Manual
https://help.ubuntu.com/community/Installation/QemuEmulator


--Jaspal


On Sat, Mar 10, 2012 at 1:24 PM, Avinash Jain  wrote:

> Hello, myself Avinash Jain. I want to contribute to QEMU but i don't know
> anything about it, I am new to this. Can any buddy please provide me an
> initial step. I work on Ubuntu 10.10. Please provide me some demo tutorial
> to start with Qemu on ubutnu.
>
> --
> thanx & regards,
> avinash
> (+91) 9502681447
>
>

Re: [Qemu-devel] [libvirt] Modern CPU models cannot be used with libvirt

2012-03-10 Thread Eduardo Habkost

On Sat, Mar 10, 2012 at 12:42:46PM +, Daniel P. Berrange wrote:
> > 
> > I could have sworn we had this discussion a year ago or so, and had decided
> > that the default CPU models would be in something like 
> > /usr/share/qemu/cpu-x86_64.conf
> > and loaded regardless of the -nodefconfig setting. 
> > /etc/qemu/target-x86_64.conf
> > would be solely for end user configuration changes, not for QEMU builtin
> > defaults.
> > 
> > But looking at the code in QEMU, it doesn't seem we ever implemented this ?
> 
> Arrrgggh. It seems this was implemented as a patch in RHEL-6 qemu RPMs but,
> contrary to our normal RHEL development practice, it was not based on
> a cherry-pick of an upstream patch :-(
> 
> For sake of reference, I'm attaching the two patches from the RHEL6 source
> RPM that do what I'm describing
> 
> NB, I'm not neccessarily advocating these patches for upstream. I still
> maintain that libvirt should write out a config file containing the
> exact CPU model description it desires and specify that with -readconfig.
> The end result would be identical from QEMU's POV and it would avoid
> playing games with QEMU's config loading code.

I agree that libvirt should just write the config somewhere. The problem
here is to define: 1) what information should be mandatory on that
config data; 2) who should be responsible to test and maintain sane
defaults (and where should they be maintained).

The current cpudef definitions are simply too low-level to require it to
be written from scratch. Lots of testing have to be done to make sure we
have working combinations of CPUID bits defined, so they can be used as
defaults or templates. Not facilitating reuse of those tested
defauls/templates by libvirt is duplication of efforts.

Really, if we expect libvirt to define all the CPU bits from scratch on
a config file, we could as well just expect libvirt to open /dev/kvm
itself and call the all CPUID setup ioctl()s itself. That's how
low-level some of the cpudef bits are.

(Also, there are additional low-level bits that really have to be
maintained somewhere, just to have sane defaults. Currently many CPUID
leafs are exposed to the guest without letting the user control them,
and worse: without keeping stability of guest-visible bits when
upgrading Qemu or the host kernel. And that's what machine-types are
for: to have sane defaults to be used as base.)

Let me give you a practical example: I had a bug report about improper
CPU topology information[1]. After investigating it, I have found out
that the "level" cpudef field is too low; CPU core topology information
is provided on CPUID leaf 4, and most of the Intel CPU models on Qemu
have level=2 today (I don't know why). So, Qemu is responsible for
exposing CPU topology information set using '-smp' to the guest OS, but
libvirt would have to be responsible for choosing a proper "level" value
that makes that information visible to the guest. We can _allow_ libvirt
to fiddle with these low-level bits, of course, but requiring every
management layer to build this low-level information from scratch is
just a recipe to waste developer time.

(And I really hope that there's no plan to require all those low-level
bits to appear as-is on the libvirt XML definitions. Because that would
require users to read the Intel 64 and IA-32 Architectures Software
Developer's Manual, or the AMD64 Architecture Programmer's Manual and
BIOS and Kernel Developer's Guides, just to understand why something is
not working on his Virtual Machine.)

[1] https://bugzilla.redhat.com/show_bug.cgi?id=689665

-- 
Eduardo

Re: [Qemu-devel] [PATCH] use an unsigned long for the max_sz parameter in load_image_targphys

2012-03-10 Thread Peter Maydell

On 10 March 2012 14:08, Andreas Färber  wrote:
> Am 10.03.2012 14:51, schrieb Peter Maydell:
>> "Length of a block of memory on the guest" is what I meant.
>> What you need is "an integer type large enough to hold the
>> difference between two guest pointer values". The size of
>> that type should depend only on the guest config, not on the
>> host, so 'unsigned long', 'size_t', 'off_t' etc are all wrong.
>
> Your view is very ARM-centric.

I don't understand this remark. Nothing about the above explanation
is ARM-centric -- it's just pointing out that the guest and the
host are two separate things and maximum widths of size and
pointer types aren't necessarily the same. Assuming they were the
same would be an x86-64-ism :-)

> In the PowerPC domain for instance, we
> have a number of usages where we hardcode a size of, e.g., 1 MB. And
> Alex should know that. I don't want to use target_phys_addr_t for that
> and forcing an end address calculation, as suggested by Alex, would be
> possible but is not as convenient as the current API.
>
> Doing a size check as Mark has demonstrated in ARM code (one place!)
> seems much simpler to me than hurting all targets just because ARM wants
> to pass a possibly stupid value unchecked to the common API.

Where the ARM specific code has particular restrictions then
I'm happy to have the ARM specific code either relax those, or
do explicit checks so we can fail cleanly or whatever.

Putting a "restrict to INT_MAX" in the highbank code is definitely
wrong (not least because passing values to load_image_targphys isn't
the only thing we use that field in arm_boot_info for!)
The ARM boot code needs updating because it shouldn't be
using 'int' for arm_boot_info.ram_size, and using target_phys_addr_t
the same as we do for initrd_size is the obvious thing. I have no
particular objection to having some new target_phys_size_t or whatever,
and I could be persuaded that we should follow the memory API in
using uint64_t for sizes, but it needs to be a type that either follows
guest phys addr restrictions or a fixed-width type which we have decreed
is always large enough, not a type which varies based on host properties.

(arm_boot also needs to fail if the size is > 4GB since at the moment
we do assume it to be 32 bits wide, but that's a different problem.)

> Compare David's off_t patch from March 8th: We'll never get an image
> size larger than off_t's max. Using target_phys_addr_t, uint64_t or
> __int128_t for the max are all moot (academic) because we'll never get
> to their max if it's larger than off_t. Therefore I've been saying, the
> host's limit is the upper realistic limit for image_load_targphys().

You mean this patch?
http://patchwork.ozlabs.org/patch/140064/
get_image_size() is asking a question about the host (ie "what is
the size of this host file?"). It therefore makes perfect sense
for its return type to be one whose size depends on properties of
the host, like off_t. load_image_targphys()'s max_sz parameter is
passing it information which is about the guest (usually "how big is
this block of RAM we're going to try to load this thing into?").
It therefore makes perfect sense for its type to be one whose size
depends on properties of the guest, or alternatively one whose
type is guaranteed to always be at least as large as the worst
case guest we support (so you could use uint64_t, as the memory
region API does for region sizes).

-- PMM

Re: [Qemu-devel] [PATCH] use an unsigned long for the max_sz parameter in load_image_targphys

Am 09.03.2012 20:04, schrieb Alexander Graf:
> 
> On 09.03.2012, at 19:47, Andreas Färber wrote:
> 
>> Am 09.03.2012 18:11, schrieb Peter Maydell:
>>> On 9 March 2012 14:28, Andreas Färber  wrote:
 No, please. We're describing sizes, not addresses. target_phys_addr_t
 thus is semantically wrong here. The RAM size is unsigned long IIRC (it
 is limited by the host's available memory). If you subtract something
 from a size it remains a size. I had therefore suggested size_t before.
 I expect sizeof(size_t) >= sizeof(unsigned long).
>>>
>>> We're discussing target sizes. size_t might be smaller than
>>> target_phys_addr_t, so it's also semantically wrong. We don't
>>> have a target_size_t, though, and I think "use an address
>>> related type for an offset" is less bad than "use a host
>>> sized type for a guest sized value".
>>
>> That is a moot point. There is no such thing as a "target size". The
>> size is defined by the guest OS, not by the architecture. And it doesn't
>> matter if the guest OS's size is defined larger than the host's because
>> we process those files on the host and they must fit into host memory.
>>
>> So unsigned long would be perfectly fine if ignoring oddball win64.
>>
>>> Compare the way we use target_phys_addr_t for the offset arguments
>>> to MemoryRegion read/write functions.
>>
>> Nobody here has been arguing against using target_phys_addr_t for guest
>> memory *offsets*.
>>
>> Actually, the size (1, 2, 4) is an unsigned int there. :) Fine with me.
>>
>> And due to very similar signed overflow issues with int64_t, 128-bit
>> integer support was introduced as a workaround. ;)
>>
>>
>> Anyway, POSIX:2008 says this about sys/types.h:
>>
>> off_t
>>  Used for file sizes.
>> size_t
>>  Used for sizes of objects.
>> ssize_t
>>  Used for a count of bytes or an error indication.
>>
>> So off_t seems right for get_image_size() although a bit
>> counter-intuitive. However later is said, "off_t shall be signed integer
>> types". So on a 32-bit host that does not necessarily help for the case
>> at hands. (Since get_image_size() gets its value as an off_t though, it
>> doesn't matter there and improves the 64-bit host case.)
>>
>> By comparison, "size_t shall be an unsigned integer type" and "The
>> implementation shall support one or more programming environments in
>> which the widths of [...] size_t, ssize_t, [...] are no greater than the
>> width of type long".
>>
>> So I still think that size_t is our best bet for potentially large sizes
>> here. Comparison with off_t would cause warnings though...
> 
> You're on the wrong track here really :). Size, max_size and friends are all 
> offsets into the guest's address space, so that's the type they should have. 
> We don't care about host POSIX whatever semantics here - it's a question of 
> how big can guest address space grow.

No, we're talking about different use cases here. You *assume* the size
depends on guest RAM. I don't.

And no, max_size is not an offset. start + max_size is. The difference
is that this expression may overflow to zero in cases I care about.

For both PReP and Macs the size depends solely on the physical address
space and we know the max size ahead of time.

For your use case you can easily provide a separate function as a
wrapper, taking target_phys_addr_t start, end if you like (you're right,
that's possible) and calculating the off_t/int/... max_sz there and pass
it on to the existing API. Anything bigger we just cannot load as a blob.

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] use an unsigned long for the max_sz parameter in load_image_targphys

Am 10.03.2012 14:51, schrieb Peter Maydell:
> On 9 March 2012 18:47, Andreas Färber  wrote:
>> Am 09.03.2012 18:11, schrieb Peter Maydell:
>>> On 9 March 2012 14:28, Andreas Färber  wrote:
 No, please. We're describing sizes, not addresses. target_phys_addr_t
 thus is semantically wrong here. The RAM size is unsigned long IIRC (it
 is limited by the host's available memory). If you subtract something
 from a size it remains a size. I had therefore suggested size_t before.
 I expect sizeof(size_t) >= sizeof(unsigned long).
>>>
>>> We're discussing target sizes. size_t might be smaller than
>>> target_phys_addr_t, so it's also semantically wrong. We don't
>>> have a target_size_t, though, and I think "use an address
>>> related type for an offset" is less bad than "use a host
>>> sized type for a guest sized value".
>>
>> That is a moot point. There is no such thing as a "target size".
> 
> "Length of a block of memory on the guest" is what I meant.
> What you need is "an integer type large enough to hold the
> difference between two guest pointer values". The size of
> that type should depend only on the guest config, not on the
> host, so 'unsigned long', 'size_t', 'off_t' etc are all wrong.

Your view is very ARM-centric. In the PowerPC domain for instance, we
have a number of usages where we hardcode a size of, e.g., 1 MB. And
Alex should know that. I don't want to use target_phys_addr_t for that
and forcing an end address calculation, as suggested by Alex, would be
possible but is not as convenient as the current API.

Doing a size check as Mark has demonstrated in ARM code (one place!)
seems much simpler to me than hurting all targets just because ARM wants
to pass a possibly stupid value unchecked to the common API.

Compare David's off_t patch from March 8th: We'll never get an image
size larger than off_t's max. Using target_phys_addr_t, uint64_t or
__int128_t for the max are all moot (academic) because we'll never get
to their max if it's larger than off_t. Therefore I've been saying, the
host's limit is the upper realistic limit for image_load_targphys().

ELF may be a different case to consider since it can be sparse.

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] use an unsigned long for the max_sz parameter in load_image_targphys

2012-03-10 Thread Peter Maydell

On 9 March 2012 18:47, Andreas Färber  wrote:
> Am 09.03.2012 18:11, schrieb Peter Maydell:
>> On 9 March 2012 14:28, Andreas Färber  wrote:
>>> No, please. We're describing sizes, not addresses. target_phys_addr_t
>>> thus is semantically wrong here. The RAM size is unsigned long IIRC (it
>>> is limited by the host's available memory). If you subtract something
>>> from a size it remains a size. I had therefore suggested size_t before.
>>> I expect sizeof(size_t) >= sizeof(unsigned long).
>>
>> We're discussing target sizes. size_t might be smaller than
>> target_phys_addr_t, so it's also semantically wrong. We don't
>> have a target_size_t, though, and I think "use an address
>> related type for an offset" is less bad than "use a host
>> sized type for a guest sized value".
>
> That is a moot point. There is no such thing as a "target size".

"Length of a block of memory on the guest" is what I meant.
What you need is "an integer type large enough to hold the
difference between two guest pointer values". The size of
that type should depend only on the guest config, not on the
host, so 'unsigned long', 'size_t', 'off_t' etc are all wrong.

-- PMM

Re: [Qemu-devel] [PATCH 1/2] qdev: accept empty string properties

Am 28.02.2012 09:54, schrieb Paolo Bonzini:
> These were stored as NULL due to wrong cut-and-paste from set_pointer.
> 
> Reported-by: Gerhard Wiesinger 
> Signed-off-by: Paolo Bonzini 

Reviewed-by: Andreas Färber 

/-F

> ---
>  hw/qdev-properties.c |4 
>  1 files changed, 0 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
> index 0423af1..bff9152 100644
> --- a/hw/qdev-properties.c
> +++ b/hw/qdev-properties.c
> @@ -421,10 +421,6 @@ static void set_string(Object *obj, Visitor *v, void 
> *opaque,
>  error_propagate(errp, local_err);
>  return;
>  }
> -if (!*str) {
> -g_free(str);
> -str = NULL;
> -}
>  if (*ptr) {
>  g_free(*ptr);
>  }

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

Re: [Qemu-devel] [PATCH 2/2] qom: fix device hot-unplug

Am 28.02.2012 09:54, schrieb Paolo Bonzini:
> Property removal modifies the list, so it is not safe to continue
> iteration.  We know anyway that each object can have only one
> parent (see object_property_add_child), so exit after finding
> the requested object.
> 
> Reported-by: Michael S. Tsirkin 
> Signed-off-by: Paolo Bonzini 
> ---
>  qom/object.c |7 ++-
>  1 files changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/qom/object.c b/qom/object.c
> index aa037d2..39cbcb9 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -304,12 +304,9 @@ static void object_property_del_child(Object *obj, 
> Object *child, Error **errp)
>  ObjectProperty *prop;
>  
>  QTAILQ_FOREACH(prop, &obj->properties, node) {
> -if (!strstart(prop->type, "child<", NULL)) {
> -continue;
> -}
> -
> -if (prop->opaque == child) {
> +if (strstart(prop->type, "child<", NULL) && prop->opaque == child) {

Didn't someone post a patch introducing an object_property_is_child() or
so? Would be handy here.

Otherwise looks okay.

Andreas

>  object_property_del(obj, prop->name, errp);
> +break;
>  }
>  }
>  }

-- 
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] pseries: Don't try to munmap() a malloc()ed TCE table

Am 08.02.2012 06:53, schrieb David Gibson:
> For the pseries machine, TCE (IOMMU) tables can either be directly
> malloc()ed in qemu or, when running on a KVM which supports it, mmap()ed
> from a KVM ioctl.  The latter option is used when available, because it
> allows the (frequent bottlenext) H_PUT_TCE hypercall to be KVM accelerated.
> However, even when KVM is persent, TCE acceleration is not always possible.
> Only KVM HV supports this ioctl(), not KVM PR, or the kernel could run out
> of contiguous memory to allocate the new table.  In this case we need to
> fall back on the malloc()ed table.
> 
> When a device is removed, and we need to remove the TCE table, we need to
> either munmap() or free() the table as appropriate for how it was
> allocated.  The code is supposed to do that, but we buggily fail to
> initialize the tcet->fd variable in the malloc() case, which is used as a
> flag to determine which is the right choice.
> 
> This patch fixes the bug, and cleans up error messages relating to this
> path while we're at it.
> 
> Signed-off-by: Benjamin Herrenschmidt 
> Signed-off-by: David Gibson 

Looks okay except that your fprintf()s should use PRIx32 for uint32_t
liobn for safety (no difference on Linux).

Andreas

> ---
>  target-ppc/kvm.c |   12 ++--
>  1 files changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
> index 50cfa02..90c6941 100644
> --- a/target-ppc/kvm.c
> +++ b/target-ppc/kvm.c
> @@ -843,12 +843,18 @@ void *kvmppc_create_spapr_tce(uint32_t liobn, uint32_t 
> window_size, int *pfd)
>  int fd;
>  void *table;
>  
> +/* Must set fd to -1 so we don't try to munmap when called for
> + * destroying the table, which the upper layers -will- do
> + */
> +*pfd = -1;
>  if (!cap_spapr_tce) {
>  return NULL;
>  }
>  
>  fd = kvm_vm_ioctl(kvm_state, KVM_CREATE_SPAPR_TCE, &args);
>  if (fd < 0) {
> +fprintf(stderr, "KVM: Failed to create TCE table for liobn 0x%x\n",
> +liobn);
>  return NULL;
>  }
>  
> @@ -857,6 +863,8 @@ void *kvmppc_create_spapr_tce(uint32_t liobn, uint32_t 
> window_size, int *pfd)
>  
>  table = mmap(NULL, len, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0);
>  if (table == MAP_FAILED) {
> +fprintf(stderr, "KVM: Failed to map TCE table for liobn 0x%x\n",
> +liobn);
>  close(fd);
>  return NULL;
>  }
> @@ -876,8 +884,8 @@ int kvmppc_remove_spapr_tce(void *table, int fd, uint32_t 
> window_size)
>  len = (window_size / SPAPR_VIO_TCE_PAGE_SIZE)*sizeof(VIOsPAPR_RTCE);
>  if ((munmap(table, len) < 0) ||
>  (close(fd) < 0)) {
> -fprintf(stderr, "KVM: Unexpected error removing KVM SPAPR TCE "
> -"table: %s", strerror(errno));
> +fprintf(stderr, "KVM: Unexpected error removing TCE table: %s",
> +strerror(errno));
>  /* Leak the table */
>  }
>  

-- 
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] [libvirt] Modern CPU models cannot be used with libvirt

2012-03-10 Thread Daniel P. Berrange

On Fri, Mar 09, 2012 at 09:04:03PM +, Daniel P. Berrange wrote:
> On Fri, Mar 09, 2012 at 05:56:52PM -0300, Eduardo Habkost wrote:
> > Resurrecting an old thread:
> > 
> > I didn't see any clear conclusion in this thread (this is why I am
> > resurrecting it), except that many were arguing that libvirt should
> > simply copy and/or generate the CPU model definitions from Qemu. I
> > really don't think it's reasonable to expect that.
> > 
> > On Thu, Dec 15, 2011 at 03:54:15PM +0100, Jiri Denemark wrote:
> > > Hi,
> > > 
> > > Recently I realized that all modern CPU models defined in
> > > /etc/qemu/target-x86_64.conf are useless when qemu is used through 
> > > libvirt.
> > > That's because we start qemu with -nodefconfig which results in qemu 
> > > ignoring
> > > that file with CPU model definitions. We have a very good reason for using
> > > -nodefconfig because we need to control the ABI presented to a guest OS 
> > > and we
> > > don't want any configuration file that can contain lots of things 
> > > including
> > > device definitions to be read by qemu. However, we would really like the 
> > > new
> > > CPU models to be understood by qemu even if used through libvirt. What 
> > > would
> > > be the best way to solve this?
> > > 
> > > I suspect this could have been already discussed in the past but 
> > > obviously a
> > > workable solution was either not found or just not implemented.
> > 
> > So, our problem today is basically:
> > 
> > A) libvirt uses -nodefconfig;
> > B) -nodefconfig makes Qemu not load the config file containing the CPU
> >model definitions; and
> > C) libvirt expects the full CPU model list from Qemu to be available.
> 
> I could have sworn we had this discussion a year ago or so, and had decided
> that the default CPU models would be in something like 
> /usr/share/qemu/cpu-x86_64.conf
> and loaded regardless of the -nodefconfig setting. 
> /etc/qemu/target-x86_64.conf
> would be solely for end user configuration changes, not for QEMU builtin
> defaults.
> 
> But looking at the code in QEMU, it doesn't seem we ever implemented this ?

Arrrgggh. It seems this was implemented as a patch in RHEL-6 qemu RPMs but,
contrary to our normal RHEL development practice, it was not based on
a cherry-pick of an upstream patch :-(

For sake of reference, I'm attaching the two patches from the RHEL6 source
RPM that do what I'm describing

NB, I'm not neccessarily advocating these patches for upstream. I still
maintain that libvirt should write out a config file containing the
exact CPU model description it desires and specify that with -readconfig.
The end result would be identical from QEMU's POV and it would avoid
playing games with QEMU's config loading code.

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|
>From ab8e4c035638685f1e579ce472dd8b4c957f8097 Mon Sep 17 00:00:00 2001
From: john cooper 
Date: Thu, 8 Jul 2010 04:17:13 -0300
Subject: [PATCH 1/3] Move CPU definitions to /usr/share/... BZ #610805

RH-Author: john cooper 
Message-id: <4c355149.2050...@redhat.com>
Patchwork-id: 10557
O-Subject: [RHEL6 PATCH] Move CPU definitions to /usr/share/...  BZ #610805
Bugzilla: 610805
RH-Acked-by: Markus Armbruster 
RH-Acked-by: Jes Sorensen 
RH-Acked-by: Juan Quintela 

Description:

libvirt requested the cpu model configuration file be
relocated to /usr/share to more clearly signify it
was not to be modified by users.  In addition the
model file is now loaded even in the presence of the
"-nodefconfig" command line flag.

Note this is intended as an interim work-around for
rhel6.0.  While the new location of the config file
should remain the same, the mechanism to direct qemu
to it will likely differ going forward.

Minor Caveat:

Although the model definitions have moved from
/etc/qemu/target-x86_64.conf to /usr/share/qemu-kvm/cpu-model/cpu-x86_64.conf
an open of the former default config file is still attempted
by qemu.  In a new installation of the rpm this is a
non-issue as well as when "-nodefconfig" is specified.
However if the former config file exists, and is allowed
to be read, and and contains model definitions conflicting
with those in cpu-x86_64.conf, they will override those
from the new config file.  This realistically is only an
issue for development testing and may be detected by
"-readconfig ?" which will indicate when the old file had
been found or "-cpu ?" which will display replicated model
definitions.

Verification:

# /usr/libexec/qemu-kvm -nodefconfig -readconfig ? -cpu ?
parsed config file /usr/share/qemu-kvm/cpu-model/cpu-x86_64.conf
x86   Opteron_G3
x86   Opteron_G2
x86   Opteron_G1
x86  Nehalem
x86   Penryn
x86   Conroe
x86   [n270]
x86

[Qemu-devel] [PATCH] maintainers: Add docs/tracing.txt to Tracing

The topic of whether and by whom docs/tracing.txt is maintained was
brought up. It currently does not have an official maintainer.

Add it to the tracing section so that Stefan gets cc'ed on patches.

Signed-off-by: Andreas Färber 
Cc: Stefan Hajnoczi 
Cc: Peter Maydell 
---
 MAINTAINERS |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index d249947..f83d07c2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -548,6 +548,7 @@ Tracing
 M: Stefan Hajnoczi 
 S: Maintained
 F: trace/
+F: docs/tracing.txt
 T: git://github.com/stefanha/qemu.git tracing

 Checkpatch
-- 
1.7.7

Re: [Qemu-devel] [PATCH] w64: Don't redefine lseek, ftruncate

Am 10.03.2012 11:14, schrieb Stefan Weil:
> MinGW-w64 already defines lseek and ftruncate (and uses the 64 bit
> variants). The conditional compilation avoids redefinitions
> (which would be wrong) and compiler warnings.
> 
> Signed-off-by: Stefan Weil 
> ---
>  qemu-common.h |8 ++--
>  1 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/qemu-common.h b/qemu-common.h
> index dbfce6f..b0fdf5c 100644
> --- a/qemu-common.h
> +++ b/qemu-common.h
> @@ -93,9 +93,13 @@ typedef int (*fprintf_function)(FILE *f, const char *fmt, 
> ...)
>  
>  #ifdef _WIN32
>  #define fsync _commit
> -#define lseek _lseeki64
> +#if !defined(lseek)
> +# define lseek _lseeki64
> +#endif
>  int qemu_ftruncate64(int, int64_t);
> -#define ftruncate qemu_ftruncate64
> +#if !defined(ftruncate)
> +# define ftruncate qemu_ftruncate64
> +#endif

lseek looks okay to me, but did you check that ftruncate and
qemu_ftruncate64 behave the same?

Andreas

>  
>  static inline char *realpath(const char *path, char *resolved_path)
>  {

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

Re: [Qemu-devel] [PATCH 2/2] Drop obsolete nographic timer

2012-03-10 Thread Jan Kiszka

On 2012-03-10 07:19, Marek Vasut wrote:
> Dear Jan Kiszka,
> 
>> We flush coalesced MMIO in the device models now, and VNC - for which
>> this was once introduced - is also fine without it as it has its own
>> refresh timer.
>>
>> Signed-off-by: Jan Kiszka 
>> ---
>>  vl.c |   13 -
>>  1 files changed, 0 insertions(+), 13 deletions(-)
> 
> This patch seems to break QEMU/PXA emulation for me on the ZipitZ2 platform. 
> The 
> serial console became very slow, but only when I type something in. The 
> output 
> from the device to the console is ok. After reverting this particular one, 
> the 
> console behaves normally.

Removing that timer removal likely just revealed some formerly hidden
bug. Can you share the command line used for starting and some test image?

Jan



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v9 4/4] xilinx_zynq: machine model initial version

2012-03-10 Thread Peter Crosthwaite

nvm,

youve got it covered. Thanks Anthony.

On Sat, Mar 10, 2012 at 8:15 PM, Peter Crosthwaite
 wrote:
> On it
>
> On Fri, Mar 9, 2012 at 2:42 AM, Anthony Liguori  wrote:
>> On 03/04/2012 10:39 PM, Peter A. G. Crosthwaite wrote:
>>>
>>> Xilinx zynq-7000 machine model. Also includes device model for the
>>> zynq-specific
>>> system level control register (SLCR) module.
>>>
>>> Signed-off-by: Peter A. G. Crosthwaite
>>> Acked-by: Edgar E. Iglesias
>>
>>
>> This breaks the build for me:
>>
>> cc1: warnings being treated as errors
>> /home/anthony/git/qemu/hw/zynq_slcr.c: In function ‘zynq_slcr_read’:
>> /home/anthony/git/qemu/hw/zynq_slcr.c:314:25: error: array subscript is
>> above array bounds
>> /home/anthony/git/qemu/hw/zynq_slcr.c: In function ‘zynq_slcr_write’:
>> /home/anthony/git/qemu/hw/zynq_slcr.c:457:22: error: array subscript is
>> above array bounds
>> make[1]: *** [zynq_slcr.o] Error 1
>>
>>
>>
>>> +        case 0xA60 ... 0xA8C:
>>> +            s->afi[0][(offset - 0x700) / 4] = val;
>>> +            break;
>>
>>
>> And GCC is definitely right here.  This will pretty clearly overflow the
>> array if offset is any of the allowable values per the switch statement.
>>
>> Regards,
>>
>> Anthony Liguori

Re: [Qemu-devel] [PATCH v9 4/4] xilinx_zynq: machine model initial version

2012-03-10 Thread Peter Crosthwaite

On it

On Fri, Mar 9, 2012 at 2:42 AM, Anthony Liguori  wrote:
> On 03/04/2012 10:39 PM, Peter A. G. Crosthwaite wrote:
>>
>> Xilinx zynq-7000 machine model. Also includes device model for the
>> zynq-specific
>> system level control register (SLCR) module.
>>
>> Signed-off-by: Peter A. G. Crosthwaite
>> Acked-by: Edgar E. Iglesias
>
>
> This breaks the build for me:
>
> cc1: warnings being treated as errors
> /home/anthony/git/qemu/hw/zynq_slcr.c: In function ‘zynq_slcr_read’:
> /home/anthony/git/qemu/hw/zynq_slcr.c:314:25: error: array subscript is
> above array bounds
> /home/anthony/git/qemu/hw/zynq_slcr.c: In function ‘zynq_slcr_write’:
> /home/anthony/git/qemu/hw/zynq_slcr.c:457:22: error: array subscript is
> above array bounds
> make[1]: *** [zynq_slcr.o] Error 1
>
>
>
>> +        case 0xA60 ... 0xA8C:
>> +            s->afi[0][(offset - 0x700) / 4] = val;
>> +            break;
>
>
> And GCC is definitely right here.  This will pretty clearly overflow the
> array if offset is any of the allowable values per the switch statement.
>
> Regards,
>
> Anthony Liguori

[Qemu-devel] [PATCH] configure: Test for libiberty.a (mingw32)

MinGW-w64 and some versions of MinGW32 don't provide libiberty.a,
so add this library only if it was found.

Signed-off-by: Stefan Weil 
---
 configure |8 +++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/configure b/configure
index ca25250..bb16498 100755
--- a/configure
+++ b/configure
@@ -511,7 +511,13 @@ if test "$mingw32" = "yes" ; then
   QEMU_CFLAGS="-DWIN32_LEAN_AND_MEAN -DWINVER=0x501 $QEMU_CFLAGS"
   # enable C99/POSIX format strings (needs mingw32-runtime 3.15 or later)
   QEMU_CFLAGS="-D__USE_MINGW_ANSI_STDIO=1 $QEMU_CFLAGS"
-  LIBS="-lwinmm -lws2_32 -liberty -liphlpapi $LIBS"
+  LIBS="-lwinmm -lws2_32 -liphlpapi $LIBS"
+cat > $TMPC << EOF
+int main(void) { return 0; }
+EOF
+  if compile_prog "" "-liberty" ; then
+LIBS="-liberty $LIBS"
+  fi
   prefix="c:/Program Files/Qemu"
   mandir="\${prefix}"
   datadir="\${prefix}"
-- 
1.7.9

[Qemu-devel] [PATCH] w64: Don't redefine lseek, ftruncate