Re: [Qemu-devel] [PATCH v8 06/12] block: Add bdrv_set_backing_hd()

[Stefan Hajnoczi]

On Fri, Dec 13, 2013 at 03:35:14PM +0800, Fam Zheng wrote:
> +void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd)
> +{
> +if (bs->backing_hd) {
> +bdrv_unref(bs->backing_hd);
> +}
> +
> +bs->backing_hd = backing_hd;
> +if (!backing_hd) {
> +return;
> +}

Maybe we should also do:
bs->backing_file[0] = '\0';
bs->backing_format[0] = '\0';

Re: [Qemu-devel] [PATCH v8 07/12] block: Add backing_blocker in BlockDriverState

[Stefan Hajnoczi]

On Fri, Dec 13, 2013 at 03:35:15PM +0800, Fam Zheng wrote:
> @@ -1476,6 +1486,9 @@ void bdrv_close(BlockDriverState *bs)
>  
>  if (bs->drv) {
>  if (bs->backing_hd) {
> +assert(error_is_set(&bs->backing_blocker));
> +bdrv_op_unblock_all(bs->backing_hd, bs->backing_blocker);
> +error_free(bs->backing_blocker);
>  bdrv_unref(bs->backing_hd);
>  bs->backing_hd = NULL;
>  }

This if statement duplicates bdrv_set_backing_hd() code.  I suggest:

if (bs->backing_hd) {
bdrv_set_backing_hd(bs, NULL);
}

bdrv_set_backing_hd() needs to be modified to call
error_free(bs->backing_blocker) when backing_hd is NULL.

Re: [Qemu-devel] [PATCH v8 08/12] block: Parse "backing" option to reference existing BDS

[Stefan Hajnoczi]

On Fri, Dec 13, 2013 at 03:35:16PM +0800, Fam Zheng wrote:
> diff --git a/block.c b/block.c
> index b3993d7..fba7148 100644
> --- a/block.c
> +++ b/block.c
> @@ -1191,11 +1191,25 @@ int bdrv_open(BlockDriverState *bs, const char 
> *filename, QDict *options,
>  /* If there is a backing file, use it */
>  if ((flags & BDRV_O_NO_BACKING) == 0) {
>  QDict *backing_options;
> -
> -qdict_extract_subqdict(options, &backing_options, "backing.");
> -ret = bdrv_open_backing_file(bs, backing_options, &local_err);
> -if (ret < 0) {
> -goto close_and_fail;
> +const char *backing_name;
> +BlockDriverState *backing_hd;
> +
> +backing_name = qdict_get_try_str(options, "backing");
> +qdict_del(options, "backing");

This causes a use-after-free since backing_name is a const char pointer
to the qdict element!

> +if (backing_name) {
> +backing_hd = bdrv_find(backing_name);
> +if (!backing_hd) {
> +error_set(&local_err, QERR_DEVICE_NOT_FOUND, backing_name);
> +ret = -ENOENT;
> +goto close_and_fail;
> +}
> +bdrv_set_backing_hd(bs, backing_hd);
> +} else {
> +qdict_extract_subqdict(options, &backing_options, "backing.");
> +ret = bdrv_open_backing_file(bs, backing_options, &local_err);
> +if (ret < 0) {
> +goto close_and_fail;
> +}
>  }

Seems like users can specify backing=foo backing.file=/tmp/a and we
ignore backing.file.  Is it useful to silently ignore the backing.
subdict?  The user may have given useless options by mistake.  An error
would help prevent weird options combinations.

>  }
>  
> @@ -1682,7 +1696,6 @@ void bdrv_swap(BlockDriverState *bs_new, 
> BlockDriverState *bs_old)
>  assert(QLIST_EMPTY(&bs_new->dirty_bitmaps));
>  assert(bs_new->job == NULL);
>  assert(bs_new->dev == NULL);
> -assert(bdrv_op_blocker_is_empty(bs_new));
>  assert(bs_new->io_limits_enabled == false);
>  assert(!throttle_have_timer(&bs_new->throttle_state));
>  
> @@ -1701,7 +1714,6 @@ void bdrv_swap(BlockDriverState *bs_new, 
> BlockDriverState *bs_old)
>  /* Check a few fields that should remain attached to the device */
>  assert(bs_new->dev == NULL);
>  assert(bs_new->job == NULL);
> -assert(bdrv_op_blocker_is_empty(bs_new));
>  assert(bs_new->io_limits_enabled == false);
>  assert(!throttle_have_timer(&bs_new->throttle_state));

Why are these hunks part of this patch?  I guess it makes sense *not* to
check for blockers in bdrv_swap().  Instead the high-level functions in
blockdev.c and elsewhere should check blockers.

Re: [Qemu-devel] [PATCH v2 1/1] qtest: Fix the bug about disabling vnc causes "make check" hang

[Paolo Bonzini]

Il 02/01/2014 15:45, Andreas Färber ha scritto:
>>> >>  v2: Consolidate VNC macro's #ifdef'ery to one central point 
>>> >> (tests/libqtest.c).
>> > 
>> > What happens if qtest instead uses "-display none"?
> It does use that, since the commit I pointed to in v1. :)

And why do you need at all "-vnc none" if it also uses "-display none"?

Paolo

Re: [Qemu-devel] [PATCH v8 09/12] block: Support dropping active in bdrv_drop_intermediate

[Stefan Hajnoczi]

On Fri, Dec 13, 2013 at 03:35:17PM +0800, Fam Zheng wrote:
> + * 1) This will convert the following chain:
> + * ... <- base <- ... <- top <- overlay <-... <- active
> + *
> + * to
>   *
> - * Requires that the overlay to 'top' is opened r/w, so that the backing file
> - * information in 'bs' can be properly updated.
> + * ... <- base <- overlay <- active
>   *
> - * E.g., this will convert the following chain:
> - * bottom <- base <- intermediate <- top <- active
> + * 2) It is allowed for bottom==base, in which case it converts:
> + *
> + * ... <- base <- ... <- top <- overlay <- ... <- active

If bottom==base then I think it should be:

base <- ... <- top <- overlay <- ... <- active

(Dropping the "... <- " prefix)

Re: [Qemu-devel] [PATCH v8 02/12] qapi: Add BlockOperationType enum

[Stefan Hajnoczi]

On Fri, Dec 13, 2013 at 03:35:10PM +0800, Fam Zheng wrote:
> This adds the enum of all the operations that can be taken on a block
> device.
> 
> Signed-off-by: Fam Zheng 
> ---
>  qapi-schema.json | 50 ++
>  1 file changed, 50 insertions(+)
> 
> diff --git a/qapi-schema.json b/qapi-schema.json
> index d6f8615..8e982a2 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -1440,6 +1440,56 @@
>'data': ['commit', 'stream', 'mirror', 'backup'] }
>  
>  ##
> +# @BlockOperationType
> +#
> +# Type of a block operation. (since 2.0)

Why is this exposed in qapi-schema.json?  The blockers concept is
internal to QEMU and not exposed via QMP.

Re: [Qemu-devel] [PATCH] block/iscsi: return -ENOMEM if an async call fails immediately

[Stefan Hajnoczi]

On Fri, Dec 20, 2013 at 10:02:47AM +0100, Peter Lieven wrote:
> if an async libiscsi call fails directly it can only be due
> to an out of memory condition. All other errors are returned
> through the callback.
> 
> Signed-off-by: Peter Lieven 
> ---
>  block/iscsi.c |   12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)

Thanks, applied to my block-next tree:
https://github.com/stefanha/qemu/commits/block-next

Stefan

Re: [Qemu-devel] [PATCHv2] block: add native support for NFS

[Peter Lieven]

On 20.12.2013 17:27, Stefan Hajnoczi wrote:

On Fri, Dec 20, 2013 at 4:57 PM, Peter Lieven  wrote:

On 20.12.2013 16:54, Stefan Hajnoczi wrote:

On Fri, Dec 20, 2013 at 4:49 PM, Peter Lieven  wrote:

On 20.12.2013 16:30, Stefan Hajnoczi wrote:

On Fri, Dec 20, 2013 at 3:43 PM, Peter Lieven  wrote:

On 20.12.2013 15:38, Stefan Hajnoczi wrote:

On Fri, Dec 20, 2013 at 3:07 PM, Peter Lieven  wrote:

On 20.12.2013 14:57, Stefan Hajnoczi wrote:

On Fri, Dec 20, 2013 at 1:53 PM, Peter Lieven  wrote:

On 20.12.2013 13:19, Stefan Hajnoczi wrote:

On Fri, Dec 20, 2013 at 10:48:41AM +0100, Peter Lieven wrote:

On 17.12.2013 17:47, Stefan Hajnoczi wrote:

On Tue, Dec 17, 2013 at 10:15:25AM +0100, Peter Lieven wrote:

+/* set to -ENOTSUP since bdrv_allocated_file_size is only
used
+ * in qemu-img open. So we can use the cached value for
allocate
+ * filesize obtained from fstat at open time */
+client->allocated_file_size = -ENOTSUP;

Can you implement this fully?  By stubbing it out like this we
won't
be
able to call get_allocated_file_size() at runtime in the future
without
updating the nfs block driver code.  It's just an fstat call,
shouldn't
be too hard to implement properly :).

It seems I have to leave it as is currently.
bdrv_get_allocated_file_size
is not in a coroutine context. I get coroutine yields to no one.

Create a coroutine and pump the event loop until it has reached
completion:

co = qemu_coroutine_create(my_coroutine_fn, ...);
qemu_coroutine_enter(co, foo);
while (!complete) {
  qemu_aio_wait();
}

See block.c for similar examples.

Wouldn't it make sense to make this modification to
bdrv_get_allocated_file_size in
block.c rather than in client/nfs.c and in the future potentially
other
drivers?

If yes, I would ask you to take v3 of the NFS protocol patch and I
promise
to send
a follow up early next year to make this modification to block.c
and
change
block/nfs.c
and other implementations to be a coroutine_fn.

.bdrv_get_allocated_file_size() implementations in other block
drivers
are synchronous.  Making the block driver interface use coroutines
would be wrong unless all the block drivers were updated to use
coroutines too.

I can do that. I think its not too complicated because all those
implementations do not rely on callbacks. It should be possible
to just rename the existing implemenations to lets say
.bdrv_co_get_allocated_file_size and call them inside a coroutine.

No, that would be wrong because coroutine functions should not block.
The point of coroutines is that if they cannot proceed they must yield
so the event loop regains control.  If you simply rename the function
to _co_ then they will block the event loop and not be true coroutine
functions.


Can you just call nfs_fstat() (the sync libnfs interface)?

I can only do that if its guaranteed that no other requests are in
flight
otherwise it will mess up.

How will it mess up?

The sync calls into libnfs are just wrappers around the async calls.
The problem is that this wrapper will handle all the callbacks for the
in-flight requests and they will never return.

So back to my original suggestion to use a qemu_aio_wait() loop in
block/nfs.c?

sorry, I cannot follow you. but maybe this here is a short solution.
question
is, what will happen when there are pending requests which invoke
callbacks.
will we end up returning from qemu_aio_wait? in the qemu-img info case
this here works:

static int64_t nfs_get_allocated_file_size(BlockDriverState *bs)
{

  NFSClient *client = bs->opaque;
  NFSRPC task = {0};
  struct stat st;

  task.st = &st;
  if (nfs_fstat_async(client->context, client->fh, nfs_co_generic_cb,
  &task) != 0) {
  return -ENOMEM;
  }

  while (!task.complete) {
  nfs_set_events(client);
  qemu_aio_wait();
  }

  return (task.status < 0 ? task.status : st.st_blocks *
st.st_blksize);
}

Yes, that's exactly what I had in mind.

Yes, other callbacks will get called and requests will complete in
this event loop.  That can be a problem in some scenarios but should
be okay with bdrv_get_allocated_file_size().

ok I will send v4 and then prepare for the holidays ;-)

Happy holidays!  I already sent out the block pull request earlier
today but your patch will be included in the first January pull
request.

If you pick this up please take v5.

Peter

Re: [Qemu-devel] [PATCHv5] block: add native support for NFS

[Stefan Hajnoczi]

Looks good.  In order to merge this new block driver qemu-iotests
support for nfs is required.  That way the block driver can be exercised
and checked for regressions (I guess you performed manual testing
during development).

Please see tests/qemu-iotests/common for examples of
NBD/SSH/Sheepdog/etc support.

The qemu-iotests test suite with raw, qcow2, and vmdk formats should
work on top of NFS.  Assuming you have an NFS server already running
on localhost, something like the following should succeed:

  cd tests/qemu-iotests
  ln -s ../../x86_64-softmmu/qemu-system-x86_64 qemu
  ln -s ../../qemu-img .
  ln -s ../../qemu-io .
  ./check -nfs # raw format by default
  ./check -nfs -qcow2
  ./check -nfs -vmdk

Maybe -nfs should take the base NFS URI as an argument to allow more
flexible test configurations.  It's up to you.

More info on qemu-iotests: http://qemu-project.org/Documentation/QemuIoTests

Re: [Qemu-devel] [PATCHv5] block: add native support for NFS

[Peter Lieven]

On 03.01.2014 11:37, Stefan Hajnoczi wrote:

Looks good.  In order to merge this new block driver qemu-iotests
support for nfs is required.  That way the block driver can be exercised
and checked for regressions (I guess you performed manual testing
during development).

Please see tests/qemu-iotests/common for examples of
NBD/SSH/Sheepdog/etc support.

The qemu-iotests test suite with raw, qcow2, and vmdk formats should
work on top of NFS.  Assuming you have an NFS server already running
on localhost, something like the following should succeed:

   cd tests/qemu-iotests
   ln -s ../../x86_64-softmmu/qemu-system-x86_64 qemu
   ln -s ../../qemu-img .
   ln -s ../../qemu-io .
   ./check -nfs # raw format by default
   ./check -nfs -qcow2
   ./check -nfs -vmdk

Maybe -nfs should take the base NFS URI as an argument to allow more
flexible test configurations.  It's up to you.

More info on qemu-iotests: http://qemu-project.org/Documentation/QemuIoTests

That would have been good to know earlier ;-)

Will look into this.

Peter

Re: [Qemu-devel] [PATCH regression] virtio: add back call to virtio_bus_device_unplugged

[Stefan Hajnoczi]

On Fri, Dec 20, 2013 at 07:48:51PM +0100, Paolo Bonzini wrote:
> This got lost in a rebase.
> 
> Reported-by: Stefan Hajnoczi 
> Signed-off-by: Paolo Bonzini 
> ---
>  hw/virtio/virtio.c | 2 ++
>  1 file changed, 2 insertions(+)

Makes qemu-iotests happy again.

Thanks, applied to my block-next tree:
https://github.com/stefanha/qemu/commits/block-next

Stefan

Re: [Qemu-devel] [PATCH target-arm v1 1/1] arm/xilinx_zynq: Always instantiate the GEMs

[Peter Maydell]

On 3 January 2014 03:21, Peter Crosthwaite  wrote:
> Don't conditionalise GEM instantiation on networking attachments. The
> device should always be present even if not attached to a network.
>
> This allows for probing of the device by expectant guests (such as
> OS's).  This is needed because sysbus (or AXI in Xilinx's real hw case)
> is not self identifying so the guest has no dynamic way of detecting
> device absence.

Agreed that this is the right thing.

Some day I might try to look into how to update our handling
of embedded NICs to work with non-legacy command lines...


> -for (n = 0; n < nb_nics; n++) {
> -nd = &nd_table[n];
> +for (n = 0; n < 2; n++) {
> +NICInfo *nd = n < nb_nics ? &nd_table[n] : NULL;
>  if (n == 0) {
>  gem_init(nd, 0xE000B000, pic[54-IRQ_OFFSET]);
>  } else if (n == 1) {

This is now a rather odd loop which goes round exactly twice
and uses an if() statement to make the body different each time.

Instead you can just say:
 gem_init(nd_table[0], 0xe000b000, pic[54 - IRQ_OFFSET]);
 gem_init(nd_table[1], 0xe000c000, pic[54 - IRQ_OFFSET]);

and have gem_init() condition the calls to qdev_set_nic_properties
and qdev_check_nic_model on "if (nd->used)".

thanks
-- PMM

Re: [Qemu-devel] [PATCHv5] block: add native support for NFS

[Peter Lieven]

On 03.01.2014 11:37, Stefan Hajnoczi wrote:

Looks good.  In order to merge this new block driver qemu-iotests
support for nfs is required.  That way the block driver can be exercised
and checked for regressions (I guess you performed manual testing
during development).

Please see tests/qemu-iotests/common for examples of
NBD/SSH/Sheepdog/etc support.

The qemu-iotests test suite with raw, qcow2, and vmdk formats should
work on top of NFS.  Assuming you have an NFS server already running
on localhost, something like the following should succeed:

   cd tests/qemu-iotests
   ln -s ../../x86_64-softmmu/qemu-system-x86_64 qemu
   ln -s ../../qemu-img .
   ln -s ../../qemu-io .
   ./check -nfs # raw format by default
   ./check -nfs -qcow2
   ./check -nfs -vmdk

Maybe -nfs should take the base NFS URI as an argument to allow more
flexible test configurations.  It's up to you.

More info on qemu-iotests: http://qemu-project.org/Documentation/QemuIoTests

i need to exclude test 063.

rm -f nfs://

obviously does not work. I wonder how sheepdog and rdb passed this test..

Peter

[Qemu-devel] [PATCH V8 3/8] util: add error_append()

[Wenchao Xia]

Signed-off-by: Wenchao Xia 
---
 include/qapi/error.h |6 ++
 util/error.c |   21 +
 2 files changed, 27 insertions(+), 0 deletions(-)

diff --git a/include/qapi/error.h b/include/qapi/error.h
index 7d4c696..bcfb724 100644
--- a/include/qapi/error.h
+++ b/include/qapi/error.h
@@ -30,6 +30,12 @@ typedef struct Error Error;
 void error_set(Error **err, ErrorClass err_class, const char *fmt, ...) 
GCC_FMT_ATTR(3, 4);
 
 /**
+ * Append message to err if err != NULL && *err != NULL. "\n" will be inserted
+ * after old message.
+ */
+void error_append(Error **err, const char *fmt, ...) GCC_FMT_ATTR(2, 3);
+
+/**
  * Set an indirect pointer to an error given a ErrorClass value and a
  * printf-style human message, followed by a strerror() string if
  * @os_error is not zero.
diff --git a/util/error.c b/util/error.c
index 3ee362a..64bbb2d 100644
--- a/util/error.c
+++ b/util/error.c
@@ -46,6 +46,27 @@ void error_set(Error **errp, ErrorClass err_class, const 
char *fmt, ...)
 errno = saved_errno;
 }
 
+void error_append(Error **err, const char *fmt, ...)
+{
+va_list ap;
+gchar *msg, *msg_old;
+
+if (!err || !*err) {
+return;
+}
+
+va_start(ap, fmt);
+msg = g_strdup_vprintf(fmt, ap);
+va_end(ap);
+
+msg_old = (*err)->msg;
+(*err)->msg = g_strdup_printf("%s\n%s", msg_old, msg);
+
+g_free(msg);
+g_free(msg_old);
+
+}
+
 void error_set_errno(Error **errp, int os_errno, ErrorClass err_class,
  const char *fmt, ...)
 {
-- 
1.7.1

[Qemu-devel] [PATCH V8 0/8] qcow2: rollback the modification on fail in snapshot creation

[Wenchao Xia]

V2:
  1: all fail case will goto fail section.
  2: add the goto code.

v3:
  Address Stefan's comments:
  2: don't goto fail after allocation failure.
  3: use sn->l1size correctly in qcow2_free_cluster().
  4-7: add test case to verify the error paths.
  Other:
  1: new patch fix a existing bug, which will be exposed in error path test.

v4:
  General change:
  rebased on upstream since error path for qcow2_write_snapshots() already
exist in upstream. removed old patch 1 since it is fixed by Max in upstream.
  5: moved the snapshot_l1_update event just before write operation, instead of
before overlap check, since it is more straight.
  6: remove a duplicated error path test about flush after snapshot list
update, add a filter which replace number to X, since now in error in report
detailed message including error cluster number.
  Address Stefan's comments:
  1, 2, 4: add *errp to store detailed error message, instead of error_report()
and compile time determined debug printf message.
  3: do not free cluster when fail in header update for safety reason.
  Address Eric's comments:
  1, 2, 4: add *errp to store detailed error message, instead of error_report()
and compile time determined debug printf message.
  5: squashed patches that add and use debug events.
  6: added comments about test only on Linux.

v5:
  General change:
  6: rebased on upstream, use case number 070, adjust 070.out due to error
message change in this version.

  Address Max's comments:
  1 use error_setg_errno() when possible, remove "ret =" in functions when
possible since the function does not need to return int value, fix 32bit/
64bit issue in printf for "sizeof" and "offse", typo fix.
  2 use error_setg_errno() when possible, fix 32bit/64bit issue in printf
for "sizeof" and "offse", typo fix.
  3 typo fix in comments.
  5 typo fix in commit message.

  Address Eric's comments:
  2 fix 32bit/64bit issue in printf for "sizeof" and "offse".

v6:
  Address Jeff's comments:
  6: add quote for image name in test case.

v7:
  Rebased on Stefan's block tree, since I need to test after Fam's
cache mode series.
  6: change case number to 075 to avoid conflict, add a comments in
case that it covers only default cache mode, qemu-img snapshot would
not be affected by case's cache setting.

v8:
  Address Stefan's comments:
  1/8: typo fix.
  2/8: remove the type case for sizeof and offsetof.
  3/8, 4/8: new patches help appending error message and detect error.
  5/8: old patch that skip cluster free when header update fail, is removed.
Instead, this patch improved qcow2_write_snapshots()'s rollback procedure by
restore header.
  6/8: new variable *err_rollback is introduced to detect sub function's
rollback error. With new function introduced by patch 3/8, message pending
is simplified so old variable Error *err is removed.
  Note: patch 5/8 and 6/8 does a full mirrored rollback operation, and
follows the rule that skip following steps when one step fail in rollback
procedure.
  8/8: changed the qcow2 header update fail case correspondly.

Wenchao Xia (8):
  1 snapshot: add parameter *errp in snapshot create
  2 qcow2: add error message in qcow2_write_snapshots()
  3 util: add error_append()
  4 qcow2: return int for qcow2_free_clusters()
  5 qcow2: full rollback on fail in qcow2_write_snapshots()
  6 qcow2: rollback on fail in qcow2_snapshot_create()
  7 blkdebug: add debug events for snapshot
  8 qemu-iotests: add test for qcow2 snapshot

 block/blkdebug.c |4 +
 block/qcow2-refcount.c   |8 +-
 block/qcow2-snapshot.c   |  164 -
 block/qcow2.h|   10 +-
 block/rbd.c  |   19 ++--
 block/sheepdog.c |   28 +++--
 block/snapshot.c |   19 +++-
 blockdev.c   |   10 +-
 include/block/block.h|4 +
 include/block/block_int.h|5 +-
 include/block/snapshot.h |5 +-
 include/qapi/error.h |6 +
 qemu-img.c   |   10 +-
 savevm.c |   12 ++-
 tests/qemu-iotests/075   |  216 ++
 tests/qemu-iotests/075.out   |   32 ++
 tests/qemu-iotests/common.filter |7 ++
 tests/qemu-iotests/group |1 +
 util/error.c |   21 
 19 files changed, 505 insertions(+), 76 deletions(-)
 create mode 100755 tests/qemu-iotests/075
 create mode 100644 tests/qemu-iotests/075.out

[Qemu-devel] [PATCH V8 8/8] qemu-iotests: add test for qcow2 snapshot

[Wenchao Xia]

This test will focus on the low level procedure of qcow2 snapshot
operations, now it covers only the create operation. Overlap error
paths are not checked since no good way to trigger those errors.

Signed-off-by: Wenchao Xia 
---
 tests/qemu-iotests/075   |  216 ++
 tests/qemu-iotests/075.out   |   32 ++
 tests/qemu-iotests/common.filter |7 ++
 tests/qemu-iotests/group |1 +
 4 files changed, 256 insertions(+), 0 deletions(-)
 create mode 100755 tests/qemu-iotests/075
 create mode 100644 tests/qemu-iotests/075.out

diff --git a/tests/qemu-iotests/075 b/tests/qemu-iotests/075
new file mode 100755
index 000..131eadf
--- /dev/null
+++ b/tests/qemu-iotests/075
@@ -0,0 +1,216 @@
+#!/bin/bash
+#
+# qcow2 internal snapshot test
+#
+# Copyright (C) 2013 IBM, Inc.
+#
+# 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 .
+#
+owner=xiaw...@linux.vnet.ibm.com
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1   # failure is the default!
+
+BLKDEBUG_CONF="$TEST_DIR/blkdebug.conf"
+
+_cleanup()
+{
+_cleanup_test_img
+rm "$BLKDEBUG_CONF"
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+. ./common.pattern
+
+# only test qcow2
+_supported_fmt qcow2
+_supported_proto generic
+# bind the errno correctly and filter the output of image check and qemu-img,
+# if you want to run it on other OS
+_supported_os Linux
+
+
+IMGOPTS="compat=1.1"
+
+CLUSTER_SIZE=65536
+
+SIZE=1G
+
+BLKDBG_TEST_IMG="blkdebug:$BLKDEBUG_CONF:$TEST_IMG"
+
+errno=5
+
+once=on
+
+imm=off
+
+
+# Start test, note that the injected errors are related to qcow2's snapshot
+# logic closely, see qcow2-snapshot.c for more details.
+
+# path 1: fail in L1 table allocation for snapshot
+echo
+echo "Path 1: fail in allocation of L1 table"
+
+_make_test_img $SIZE
+
+cat > "$BLKDEBUG_CONF" <&1
+
+
+# path 2: fail in update new L1 table
+echo
+echo "Path 2: fail in update new L1 table for snapshot"
+
+_make_test_img $SIZE
+
+cat > "$BLKDEBUG_CONF" <&1 | _filter_number
+$QEMU_IMG snapshot -l "$TEST_IMG"
+_check_test_img 2>&1
+
+# path 3: fail in update refcount block before write snapshot list
+echo
+echo "Path 3: fail in update refcount block before write snapshot list"
+
+_make_test_img $SIZE
+
+cat > "$BLKDEBUG_CONF" <&1 | _filter_number
+$QEMU_IMG snapshot -l "$TEST_IMG"
+_check_test_img 2>&1
+
+# path 4: fail in snapshot list allocation or its flush it is possible
+# qcow2_alloc_clusters() not fail immediately since cache hit, but in any
+# case, no error should be found in image check.
+echo
+echo "Path 4: fail in snapshot list allocation or its flush"
+
+_make_test_img $SIZE
+
+cat > "$BLKDEBUG_CONF" <&1 | grep "Failed" | 
grep "allocation" | grep "list"`
+if ! test -z "$err"
+then
+echo "Error happens as expected"
+fi
+$QEMU_IMG snapshot -l "$TEST_IMG"
+_check_test_img 2>&1
+
+
+# path 5: fail in snapshot list update
+echo
+echo "Path 5: fail in snapshot list update"
+
+_make_test_img $SIZE
+
+cat > "$BLKDEBUG_CONF" <&1 | _filter_number
+$QEMU_IMG snapshot -l "$TEST_IMG"
+_check_test_img 2>&1
+
+# path 6: fail in flush after snapshot list update, no good way to trigger it,
+# since the cache is empty and makes flush do nothing in that call, so leave
+# this path not tested
+
+# path 7: fail in update qcow2 header, normally header file will be recovered,
+# see qcow2-snapshot.c.
+echo
+echo "Path 7: fail in update qcow2 header"
+
+_make_test_img $SIZE
+
+cat > "$BLKDEBUG_CONF" <&1 | _filter_number
+$QEMU_IMG snapshot -l "$TEST_IMG"
+_check_test_img 2>&1 | _filter_number
+
+# path 8: fail in overlap check before update L1 table for snapshot
+# path 9: fail in overlap check before update snapshot list
+# Since those clusters are allocated at runtime, there is no good way to
+# make them overlap in this script, so skip those two paths now.
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/075.out b/tests/qemu-iotests/075.out
new file mode 100644
index 000..9a40e4a
--- /dev/null
+++ b/tests/qemu-iotests/075.out
@@ -0,0 +1,32 @@
+QA output created by 075
+
+Path 1: fail in allocation of L1 table
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
+qemu-img: Could not create snapshot 'snap1': Failed in allocation of snapshot

[Qemu-devel] [PATCH V8 2/8] qcow2: add error message in qcow2_write_snapshots()

[Wenchao Xia]

The function still returns int since qcow2_snapshot_delete() will
return the number.

Signed-off-by: Wenchao Xia 
---
 block/qcow2-snapshot.c |   43 +--
 1 files changed, 37 insertions(+), 6 deletions(-)

diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
index 8cac42d..5619fc3 100644
--- a/block/qcow2-snapshot.c
+++ b/block/qcow2-snapshot.c
@@ -152,7 +152,7 @@ fail:
 }
 
 /* add at the end of the file a new list of snapshots */
-static int qcow2_write_snapshots(BlockDriverState *bs)
+static int qcow2_write_snapshots(BlockDriverState *bs, Error **errp)
 {
 BDRVQcowState *s = bs->opaque;
 QCowSnapshot *sn;
@@ -183,10 +183,15 @@ static int qcow2_write_snapshots(BlockDriverState *bs)
 offset = snapshots_offset;
 if (offset < 0) {
 ret = offset;
+error_setg_errno(errp, -ret,
+ "Failed in allocation of cluster for snapshot list");
 goto fail;
 }
 ret = bdrv_flush(bs);
 if (ret < 0) {
+error_setg_errno(errp, -ret,
+ "Failed in flush after snapshot list cluster "
+ "allocation");
 goto fail;
 }
 
@@ -194,6 +199,10 @@ static int qcow2_write_snapshots(BlockDriverState *bs)
  * must indeed be completely free */
 ret = qcow2_pre_write_overlap_check(bs, 0, offset, snapshots_size);
 if (ret < 0) {
+error_setg_errno(errp, -ret,
+ "Failed in overlap check for snapshot list cluster "
+ "at %" PRIi64 " with size %d",
+ offset, snapshots_size);
 goto fail;
 }
 
@@ -227,24 +236,40 @@ static int qcow2_write_snapshots(BlockDriverState *bs)
 
 ret = bdrv_pwrite(bs->file, offset, &h, sizeof(h));
 if (ret < 0) {
+error_setg_errno(errp, -ret,
+ "Failed in write of snapshot header at %"
+ PRIi64 " with size %zu",
+ offset, sizeof(h));
 goto fail;
 }
 offset += sizeof(h);
 
 ret = bdrv_pwrite(bs->file, offset, &extra, sizeof(extra));
 if (ret < 0) {
+error_setg_errno(errp, -ret,
+ "Failed in write of extra snapshot data at %"
+ PRIi64 " with size %zu",
+ offset, sizeof(extra));
 goto fail;
 }
 offset += sizeof(extra);
 
 ret = bdrv_pwrite(bs->file, offset, sn->id_str, id_str_size);
 if (ret < 0) {
+error_setg_errno(errp, -ret,
+ "Failed in write of snapshot id string at %"
+ PRIi64 " with size %d",
+ offset, id_str_size);
 goto fail;
 }
 offset += id_str_size;
 
 ret = bdrv_pwrite(bs->file, offset, sn->name, name_size);
 if (ret < 0) {
+error_setg_errno(errp, -ret,
+ "Failed in write of snapshot name string at %"
+ PRIi64 " with size %d",
+ offset, name_size);
 goto fail;
 }
 offset += name_size;
@@ -256,6 +281,8 @@ static int qcow2_write_snapshots(BlockDriverState *bs)
  */
 ret = bdrv_flush(bs);
 if (ret < 0) {
+error_setg_errno(errp, -ret,
+ "Failed in flush after snapshot list update");
 goto fail;
 }
 
@@ -268,6 +295,10 @@ static int qcow2_write_snapshots(BlockDriverState *bs)
 ret = bdrv_pwrite_sync(bs->file, offsetof(QCowHeader, nb_snapshots),
&header_data, sizeof(header_data));
 if (ret < 0) {
+error_setg_errno(errp, -ret,
+ "Failed in update of image header at %zu with size 
%zu",
+ offsetof(QCowHeader, nb_snapshots),
+ sizeof(header_data));
 goto fail;
 }
 
@@ -283,6 +314,9 @@ fail:
 qcow2_free_clusters(bs, snapshots_offset, snapshots_size,
 QCOW2_DISCARD_ALWAYS);
 }
+if (errp) {
+g_assert(error_is_set(errp));
+}
 return ret;
 }
 
@@ -447,10 +481,8 @@ void qcow2_snapshot_create(BlockDriverState *bs,
 s->snapshots = new_snapshot_list;
 s->snapshots[s->nb_snapshots++] = *sn;
 
-ret = qcow2_write_snapshots(bs);
+ret = qcow2_write_snapshots(bs, errp);
 if (ret < 0) {
-/* Following line will be replaced with more detailed error later */
-error_setg(errp, "Failed in write of snapshot");
 g_free(s->snapshots);
 s->snapshots = old_snapshot_list;
 s->nb_snapshots--;
@@ -624,9 +656,8 @@ int qcow2_snapshot_delete(BlockDriverState *bs,
 s->snapshots + snapshot_index + 1,
 (s->nb_snapshots - snapshot_index - 1) * sizeof(sn));
 s->nb_snapshots--;
-ret = qcow2_write_

[Qemu-devel] [PATCH V8 5/8] qcow2: full rollback on fail in qcow2_write_snapshots()

[Wenchao Xia]

Header restore step is added, and old code section "fail" is
renamed to "dealloc_sn_table" to tip better, now "fail" section
does not rollback anything on disk. When one step in rollback
fails, following steps will be skipped, to make sure no dangling
pointer is left.

A new parameter "*errp_rollback" is added to tell caller the result
of rollback procedure.

Signed-off-by: Wenchao Xia 
---
 block/qcow2-snapshot.c |   63 +++
 1 files changed, 47 insertions(+), 16 deletions(-)

diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
index 5619fc3..5cac714 100644
--- a/block/qcow2-snapshot.c
+++ b/block/qcow2-snapshot.c
@@ -152,7 +152,9 @@ fail:
 }
 
 /* add at the end of the file a new list of snapshots */
-static int qcow2_write_snapshots(BlockDriverState *bs, Error **errp)
+static int qcow2_write_snapshots(BlockDriverState *bs,
+ Error **errp,
+ Error **errp_rollback)
 {
 BDRVQcowState *s = bs->opaque;
 QCowSnapshot *sn;
@@ -162,9 +164,9 @@ static int qcow2_write_snapshots(BlockDriverState *bs, 
Error **errp)
 struct {
 uint32_t nb_snapshots;
 uint64_t snapshots_offset;
-} QEMU_PACKED header_data;
+} QEMU_PACKED header_data, header_data_old;
 int64_t offset, snapshots_offset;
-int ret;
+int ret, ret0;
 
 /* compute the size of the snapshots */
 offset = 0;
@@ -192,7 +194,7 @@ static int qcow2_write_snapshots(BlockDriverState *bs, 
Error **errp)
 error_setg_errno(errp, -ret,
  "Failed in flush after snapshot list cluster "
  "allocation");
-goto fail;
+goto dealloc_sn_table;
 }
 
 /* The snapshot list position has not yet been updated, so these clusters
@@ -203,7 +205,7 @@ static int qcow2_write_snapshots(BlockDriverState *bs, 
Error **errp)
  "Failed in overlap check for snapshot list cluster "
  "at %" PRIi64 " with size %d",
  offset, snapshots_size);
-goto fail;
+goto dealloc_sn_table;
 }
 
 
@@ -240,7 +242,7 @@ static int qcow2_write_snapshots(BlockDriverState *bs, 
Error **errp)
  "Failed in write of snapshot header at %"
  PRIi64 " with size %zu",
  offset, sizeof(h));
-goto fail;
+goto dealloc_sn_table;
 }
 offset += sizeof(h);
 
@@ -250,7 +252,7 @@ static int qcow2_write_snapshots(BlockDriverState *bs, 
Error **errp)
  "Failed in write of extra snapshot data at %"
  PRIi64 " with size %zu",
  offset, sizeof(extra));
-goto fail;
+goto dealloc_sn_table;
 }
 offset += sizeof(extra);
 
@@ -260,7 +262,7 @@ static int qcow2_write_snapshots(BlockDriverState *bs, 
Error **errp)
  "Failed in write of snapshot id string at %"
  PRIi64 " with size %d",
  offset, id_str_size);
-goto fail;
+goto dealloc_sn_table;
 }
 offset += id_str_size;
 
@@ -270,7 +272,7 @@ static int qcow2_write_snapshots(BlockDriverState *bs, 
Error **errp)
  "Failed in write of snapshot name string at %"
  PRIi64 " with size %d",
  offset, name_size);
-goto fail;
+goto dealloc_sn_table;
 }
 offset += name_size;
 }
@@ -283,7 +285,18 @@ static int qcow2_write_snapshots(BlockDriverState *bs, 
Error **errp)
 if (ret < 0) {
 error_setg_errno(errp, -ret,
  "Failed in flush after snapshot list update");
-goto fail;
+goto dealloc_sn_table;
+}
+
+/* Start the qcow2 header update */
+ret = bdrv_pread(bs->file, offsetof(QCowHeader, nb_snapshots),
+ &header_data_old, sizeof(header_data_old));
+if (ret < 0) {
+error_setg_errno(errp, -ret,
+ "Failed in read of image header at %zu with size %zu",
+ offsetof(QCowHeader, nb_snapshots),
+ sizeof(header_data_old));
+goto dealloc_sn_table;
 }
 
 QEMU_BUILD_BUG_ON(offsetof(QCowHeader, snapshots_offset) !=
@@ -299,7 +312,7 @@ static int qcow2_write_snapshots(BlockDriverState *bs, 
Error **errp)
  "Failed in update of image header at %zu with size 
%zu",
  offsetof(QCowHeader, nb_snapshots),
  sizeof(header_data));
-goto fail;
+goto restore_header;
 }
 
 /* free the old snapshot table */
@@ -309,11 +322,29 @@ static int qcow2_write_snapshots(BlockDriverState *bs, 
Error **errp)
 s->snapshots_size = snapsho

[Qemu-devel] [PATCH V8 7/8] blkdebug: add debug events for snapshot

[Wenchao Xia]

Some code in qcow2-snapshot.c directly accesses bs->file, so in those
places errors can't be injected by other events. Since the code in
qcow2-snapshot.c is similar to the other qcow2 internal code (in regards
to e.g. the L1 table), add some debug events.

Signed-off-by: Wenchao Xia 
Reviewed-by: Max Reitz 
---
 block/blkdebug.c   |4 
 block/qcow2-snapshot.c |3 +++
 include/block/block.h  |4 
 3 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/block/blkdebug.c b/block/blkdebug.c
index 957be2c..9c801fb 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -186,6 +186,10 @@ static const char *event_names[BLKDBG_EVENT_MAX] = {
 
 [BLKDBG_FLUSH_TO_OS]= "flush_to_os",
 [BLKDBG_FLUSH_TO_DISK]  = "flush_to_disk",
+
+[BLKDBG_SNAPSHOT_L1_UPDATE] = "snapshot_l1_update",
+[BLKDBG_SNAPSHOT_LIST_UPDATE]   = "snapshot_list_update",
+[BLKDBG_SNAPSHOT_HEADER_UPDATE] = "snapshot_header_update",
 };
 
 static int get_event_by_name(const char *name, BlkDebugEvent *event)
diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
index 29ba534..8a143e7 100644
--- a/block/qcow2-snapshot.c
+++ b/block/qcow2-snapshot.c
@@ -209,6 +209,7 @@ static int qcow2_write_snapshots(BlockDriverState *bs,
 }
 
 
+BLKDBG_EVENT(bs->file, BLKDBG_SNAPSHOT_LIST_UPDATE);
 /* Write all snapshots to the new list */
 for(i = 0; i < s->nb_snapshots; i++) {
 sn = s->snapshots + i;
@@ -305,6 +306,7 @@ static int qcow2_write_snapshots(BlockDriverState *bs,
 header_data.nb_snapshots= cpu_to_be32(s->nb_snapshots);
 header_data.snapshots_offset= cpu_to_be64(snapshots_offset);
 
+BLKDBG_EVENT(bs->file, BLKDBG_SNAPSHOT_HEADER_UPDATE);
 ret = bdrv_pwrite_sync(bs->file, offsetof(QCowHeader, nb_snapshots),
&header_data, sizeof(header_data));
 if (ret < 0) {
@@ -475,6 +477,7 @@ void qcow2_snapshot_create(BlockDriverState *bs,
 goto dealloc_l1_table;
 }
 
+BLKDBG_EVENT(bs->file, BLKDBG_SNAPSHOT_L1_UPDATE);
 ret = bdrv_pwrite(bs->file, sn->l1_table_offset, l1_table,
   s->l1_size * sizeof(uint64_t));
 if (ret < 0) {
diff --git a/include/block/block.h b/include/block/block.h
index 36efaea..8901683 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -515,6 +515,10 @@ typedef enum {
 BLKDBG_FLUSH_TO_OS,
 BLKDBG_FLUSH_TO_DISK,
 
+BLKDBG_SNAPSHOT_L1_UPDATE,
+BLKDBG_SNAPSHOT_LIST_UPDATE,
+BLKDBG_SNAPSHOT_HEADER_UPDATE,
+
 BLKDBG_EVENT_MAX,
 } BlkDebugEvent;
 
-- 
1.7.1

[Qemu-devel] [PATCH V8 6/8] qcow2: rollback on fail in qcow2_snapshot_create()

[Wenchao Xia]

A new variable *err_rollback is added to detect sub function's
rollback failure. If one step in rollback procedure fails, following
steps will be skipped, and the error message will be appended
to errp.

Signed-off-by: Wenchao Xia 
---
 block/qcow2-snapshot.c |   37 -
 1 files changed, 32 insertions(+), 5 deletions(-)

diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
index 5cac714..29ba534 100644
--- a/block/qcow2-snapshot.c
+++ b/block/qcow2-snapshot.c
@@ -423,6 +423,7 @@ void qcow2_snapshot_create(BlockDriverState *bs,
 int i, ret;
 uint64_t *l1_table = NULL;
 int64_t l1_table_offset;
+Error *err_rollback = NULL;
 
 memset(sn, 0, sizeof(*sn));
 
@@ -471,7 +472,7 @@ void qcow2_snapshot_create(BlockDriverState *bs,
  PRIu64 " with size %" PRIu64,
  sn->l1_table_offset,
  (uint64_t)(s->l1_size * sizeof(uint64_t)));
-goto fail;
+goto dealloc_l1_table;
 }
 
 ret = bdrv_pwrite(bs->file, sn->l1_table_offset, l1_table,
@@ -482,7 +483,7 @@ void qcow2_snapshot_create(BlockDriverState *bs,
  PRIu64 " with size %" PRIu64,
  sn->l1_table_offset,
  (uint64_t)(s->l1_size * sizeof(uint64_t)));
-goto fail;
+goto dealloc_l1_table;
 }
 
 g_free(l1_table);
@@ -499,7 +500,7 @@ void qcow2_snapshot_create(BlockDriverState *bs,
  "Failed in update of refcount for snapshot at %"
  PRIu64 " with size %d",
  s->l1_table_offset, s->l1_size);
-goto fail;
+goto dealloc_l1_table;
 }
 
 /* Append the new snapshot to the snapshot list */
@@ -512,12 +513,18 @@ void qcow2_snapshot_create(BlockDriverState *bs,
 s->snapshots = new_snapshot_list;
 s->snapshots[s->nb_snapshots++] = *sn;
 
-ret = qcow2_write_snapshots(bs, errp, NULL);
+ret = qcow2_write_snapshots(bs, errp, &err_rollback);
 if (ret < 0) {
 g_free(s->snapshots);
 s->snapshots = old_snapshot_list;
 s->nb_snapshots--;
-goto fail;
+if (error_is_set(&err_rollback)) {
+error_append(errp, "%s", error_get_pretty(err_rollback));
+error_free(err_rollback);
+goto fail;
+} else {
+goto restore_refcount;
+}
 }
 
 g_free(old_snapshot_list);
@@ -537,6 +544,26 @@ void qcow2_snapshot_create(BlockDriverState *bs,
 #endif
 return;
 
+restore_refcount:
+if (qcow2_update_snapshot_refcount(bs, s->l1_table_offset, s->l1_size, -1)
+< 0) {
+/* Nothing can be done now, need image check later, skip following
+   rollback action. */
+error_append(errp,
+"In rollback failed to restore refcount in snapshot");
+goto fail;
+}
+
+dealloc_l1_table:
+ret = qcow2_free_clusters(bs, sn->l1_table_offset,
+  sn->l1_size * sizeof(uint64_t),
+  QCOW2_DISCARD_ALWAYS);
+if (ret < 0) {
+error_append(errp,
+ "In rollback failed to free L1 table: %s\n",
+ strerror(-ret));
+}
+
 fail:
 g_free(sn->id_str);
 g_free(sn->name);
-- 
1.7.1

[Qemu-devel] [PATCH V8 1/8] snapshot: add parameter *errp in snapshot create

[Wenchao Xia]

The return value is only used for error report before this patch,
so change the function protype to return void.

Signed-off-by: Wenchao Xia 
---
 block/qcow2-snapshot.c|   30 +-
 block/qcow2.h |4 +++-
 block/rbd.c   |   19 ++-
 block/sheepdog.c  |   28 ++--
 block/snapshot.c  |   19 +--
 blockdev.c|   10 --
 include/block/block_int.h |5 +++--
 include/block/snapshot.h  |5 +++--
 qemu-img.c|   10 ++
 savevm.c  |   12 
 10 files changed, 93 insertions(+), 49 deletions(-)

diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
index ad8bf3d..8cac42d 100644
--- a/block/qcow2-snapshot.c
+++ b/block/qcow2-snapshot.c
@@ -347,7 +347,9 @@ static int find_snapshot_by_id_or_name(BlockDriverState *bs,
 }
 
 /* if no id is provided, a new one is constructed */
-int qcow2_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info)
+void qcow2_snapshot_create(BlockDriverState *bs,
+   QEMUSnapshotInfo *sn_info,
+   Error **errp)
 {
 BDRVQcowState *s = bs->opaque;
 QCowSnapshot *new_snapshot_list = NULL;
@@ -366,7 +368,8 @@ int qcow2_snapshot_create(BlockDriverState *bs, 
QEMUSnapshotInfo *sn_info)
 
 /* Check that the ID is unique */
 if (find_snapshot_by_id_and_name(bs, sn_info->id_str, NULL) >= 0) {
-return -EEXIST;
+error_setg(errp, "Snapshot with id %s already exists", 
sn_info->id_str);
+return;
 }
 
 /* Populate sn with passed data */
@@ -382,7 +385,8 @@ int qcow2_snapshot_create(BlockDriverState *bs, 
QEMUSnapshotInfo *sn_info)
 /* Allocate the L1 table of the snapshot and copy the current one there. */
 l1_table_offset = qcow2_alloc_clusters(bs, s->l1_size * sizeof(uint64_t));
 if (l1_table_offset < 0) {
-ret = l1_table_offset;
+error_setg_errno(errp, -l1_table_offset,
+ "Failed in allocation of snapshot L1 table");
 goto fail;
 }
 
@@ -397,12 +401,22 @@ int qcow2_snapshot_create(BlockDriverState *bs, 
QEMUSnapshotInfo *sn_info)
 ret = qcow2_pre_write_overlap_check(bs, 0, sn->l1_table_offset,
 s->l1_size * sizeof(uint64_t));
 if (ret < 0) {
+error_setg_errno(errp, -ret,
+ "Failed in overlap check for snapshot L1 table at %"
+ PRIu64 " with size %" PRIu64,
+ sn->l1_table_offset,
+ (uint64_t)(s->l1_size * sizeof(uint64_t)));
 goto fail;
 }
 
 ret = bdrv_pwrite(bs->file, sn->l1_table_offset, l1_table,
   s->l1_size * sizeof(uint64_t));
 if (ret < 0) {
+error_setg_errno(errp, -ret,
+ "Failed in update of snapshot L1 table at %"
+ PRIu64 " with size %" PRIu64,
+ sn->l1_table_offset,
+ (uint64_t)(s->l1_size * sizeof(uint64_t)));
 goto fail;
 }
 
@@ -416,6 +430,10 @@ int qcow2_snapshot_create(BlockDriverState *bs, 
QEMUSnapshotInfo *sn_info)
  */
 ret = qcow2_update_snapshot_refcount(bs, s->l1_table_offset, s->l1_size, 
1);
 if (ret < 0) {
+error_setg_errno(errp, -ret,
+ "Failed in update of refcount for snapshot at %"
+ PRIu64 " with size %d",
+ s->l1_table_offset, s->l1_size);
 goto fail;
 }
 
@@ -431,6 +449,8 @@ int qcow2_snapshot_create(BlockDriverState *bs, 
QEMUSnapshotInfo *sn_info)
 
 ret = qcow2_write_snapshots(bs);
 if (ret < 0) {
+/* Following line will be replaced with more detailed error later */
+error_setg(errp, "Failed in write of snapshot");
 g_free(s->snapshots);
 s->snapshots = old_snapshot_list;
 s->nb_snapshots--;
@@ -452,14 +472,14 @@ int qcow2_snapshot_create(BlockDriverState *bs, 
QEMUSnapshotInfo *sn_info)
   qcow2_check_refcounts(bs, &result, 0);
 }
 #endif
-return 0;
+return;
 
 fail:
 g_free(sn->id_str);
 g_free(sn->name);
 g_free(l1_table);
 
-return ret;
+return;
 }
 
 /* copy the snapshot 'snapshot_name' into the current disk image */
diff --git a/block/qcow2.h b/block/qcow2.h
index 303eb26..c56a5b6 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -481,7 +481,9 @@ int qcow2_zero_clusters(BlockDriverState *bs, uint64_t 
offset, int nb_sectors);
 int qcow2_expand_zero_clusters(BlockDriverState *bs);
 
 /* qcow2-snapshot.c functions */
-int qcow2_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info);
+void qcow2_snapshot_create(BlockDriverState *bs,
+   QEMUSnapshotInfo *sn_info,
+   Error **errp);
 int qcow2_snapshot_goto(BlockDriverState *bs, const char *snapshot_i

Re: [Qemu-devel] [PATCHv5] block: add native support for NFS

[Peter Lieven]

On 03.01.2014 11:37, Stefan Hajnoczi wrote:

Looks good.  In order to merge this new block driver qemu-iotests
support for nfs is required.  That way the block driver can be exercised
and checked for regressions (I guess you performed manual testing
during development).

Please see tests/qemu-iotests/common for examples of
NBD/SSH/Sheepdog/etc support.

The qemu-iotests test suite with raw, qcow2, and vmdk formats should
work on top of NFS.  Assuming you have an NFS server already running
on localhost, something like the following should succeed:

   cd tests/qemu-iotests
   ln -s ../../x86_64-softmmu/qemu-system-x86_64 qemu
   ln -s ../../qemu-img .
   ln -s ../../qemu-io .
   ./check -nfs # raw format by default
   ./check -nfs -qcow2
   ./check -nfs -vmdk

Maybe -nfs should take the base NFS URI as an argument to allow more
flexible test configurations.  It's up to you.

More info on qemu-iotests: http://qemu-project.org/Documentation/QemuIoTests

it seems that several tests are broken since they use commands like
rm -f or mv and have protocol generic. shall I fix this?

Peter

[Qemu-devel] [PATCH V8 4/8] qcow2: return int for qcow2_free_clusters()

[Wenchao Xia]

The return value can help caller check whether error happens,
and it need not to have *errp since the return value already tip
what happend.

Signed-off-by: Wenchao Xia 
---
 block/qcow2-refcount.c |8 +---
 block/qcow2.h  |6 +++---
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index c974abe..4eb413a 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -761,9 +761,9 @@ int64_t qcow2_alloc_bytes(BlockDriverState *bs, int size)
 return offset;
 }
 
-void qcow2_free_clusters(BlockDriverState *bs,
-  int64_t offset, int64_t size,
-  enum qcow2_discard_type type)
+int qcow2_free_clusters(BlockDriverState *bs,
+int64_t offset, int64_t size,
+enum qcow2_discard_type type)
 {
 int ret;
 
@@ -773,6 +773,8 @@ void qcow2_free_clusters(BlockDriverState *bs,
 fprintf(stderr, "qcow2_free_clusters failed: %s\n", strerror(-ret));
 /* TODO Remember the clusters to free them later and avoid leaking */
 }
+
+return ret;
 }
 
 /*
diff --git a/block/qcow2.h b/block/qcow2.h
index c56a5b6..161549a 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -435,9 +435,9 @@ int64_t qcow2_alloc_clusters(BlockDriverState *bs, int64_t 
size);
 int qcow2_alloc_clusters_at(BlockDriverState *bs, uint64_t offset,
 int nb_clusters);
 int64_t qcow2_alloc_bytes(BlockDriverState *bs, int size);
-void qcow2_free_clusters(BlockDriverState *bs,
-  int64_t offset, int64_t size,
-  enum qcow2_discard_type type);
+int qcow2_free_clusters(BlockDriverState *bs,
+int64_t offset, int64_t size,
+enum qcow2_discard_type type);
 void qcow2_free_any_clusters(BlockDriverState *bs, uint64_t l2_entry,
  int nb_clusters, enum qcow2_discard_type type);
 
-- 
1.7.1

[Qemu-devel] [PATCH] sheepdog: fix clone operation by 'qemu-img create -b'

[Liu Yuan]

We should pass base_inode->vdi_id to base_vdi_id of SheepdogVdiReq so that sheep
can create a clone instead a fresh volume.

This fixes following command:

qemu-create -b sheepdog:base sheepdog:clone

so users can boot sheepdog:clone as a normal volume.

Cc: qemu-devel@nongnu.org
Cc: Kevin Wolf 
Cc: Stefan Hajnoczi 
Signed-off-by: Liu Yuan 
---
 block/sheepdog.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/block/sheepdog.c b/block/sheepdog.c
index ba451a9..843a4a5 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -161,7 +161,7 @@ typedef struct SheepdogVdiReq {
 uint32_t id;
 uint32_t data_length;
 uint64_t vdi_size;
-uint32_t vdi_id;
+uint32_t base_vdi_id;
 uint8_t copies;
 uint8_t copy_policy;
 uint8_t reserved[2];
@@ -1493,7 +1493,7 @@ static int do_sd_create(BDRVSheepdogState *s, uint32_t 
*vdi_id, int snapshot)
 
 memset(&hdr, 0, sizeof(hdr));
 hdr.opcode = SD_OP_NEW_VDI;
-hdr.vdi_id = s->inode.vdi_id;
+hdr.base_vdi_id = s->inode.vdi_id;
 
 wlen = SD_MAX_VDI_LEN;
 
@@ -1684,7 +1684,7 @@ static int sd_create(const char *filename, 
QEMUOptionParameter *options,
 
 if (backing_file) {
 BlockDriverState *bs;
-BDRVSheepdogState *s;
+BDRVSheepdogState *base;
 BlockDriver *drv;
 
 /* Currently, only Sheepdog backing image is supported. */
@@ -1702,15 +1702,15 @@ static int sd_create(const char *filename, 
QEMUOptionParameter *options,
 goto out;
 }
 
-s = bs->opaque;
+base = bs->opaque;
 
-if (!is_snapshot(&s->inode)) {
+if (!is_snapshot(&base->inode)) {
 error_report("cannot clone from a non snapshot vdi");
 bdrv_unref(bs);
 ret = -EINVAL;
 goto out;
 }
-
+s->inode.vdi_id = base->inode.vdi_id;
 bdrv_unref(bs);
 }
 
@@ -1743,7 +1743,7 @@ static void sd_close(BlockDriverState *bs)
 memset(&hdr, 0, sizeof(hdr));
 
 hdr.opcode = SD_OP_RELEASE_VDI;
-hdr.vdi_id = s->inode.vdi_id;
+hdr.base_vdi_id = s->inode.vdi_id;
 wlen = strlen(s->name) + 1;
 hdr.data_length = wlen;
 hdr.flags = SD_FLAG_CMD_WRITE;
@@ -1846,7 +1846,7 @@ static bool sd_delete(BDRVSheepdogState *s)
 unsigned int wlen = SD_MAX_VDI_LEN, rlen = 0;
 SheepdogVdiReq hdr = {
 .opcode = SD_OP_DEL_VDI,
-.vdi_id = s->inode.vdi_id,
+.base_vdi_id = s->inode.vdi_id,
 .data_length = wlen,
 .flags = SD_FLAG_CMD_WRITE,
 };
-- 
1.8.1.2

Re: [Qemu-devel] [Xen-devel] [PATCH v2 0/2] build QEMU with Xen support on ARM

[Stefano Stabellini]

On Wed, 18 Dec 2013, Konrad Rzeszutek Wilk wrote:
> On Wed, Dec 18, 2013 at 07:15:43PM +, Stefano Stabellini wrote:
> > Hi all,
> > the xenpv machine provides Xen paravirtualized backends for console,
> > disk and framebuffer. xenfb in particular is the only open source
> > framebuffer backend available.
> > On ARM we don't need QEMU to emulate any hardware but xenpv would still
> > be useful at least to provide xenfb.
> > This patch series allows QEMU to build and run (xenpv) with Xen support
> > on ARM.
> > 
> 
> This should work out of the box (with changes to the toolstack) to work
> under x86 right? I would have to do some 'vga=none' to disable the VGA
> framebuffer?

Yes, it should already be working on x86 without changes to the toolstack.
You don't even need to pass 'vga=none': that is for HVM guests, while
for PVH guests we simple want the xenpv QEMU, the one exclusively providing
userspace pv backends.



> > Changes in v2:
> > - use SCNu64 instead of PRIu64 with sscanf;
> > - assert mfn == (xen_pfn_t)mfn;
> > - use HOST_LONG_BITS to check for QEMU's address space size.
> > 
> > 
> > Stefano Stabellini (2):
> >   xen_backend: introduce xenstore_read_uint64 and 
> > xenstore_read_fe_uint64
> >   xen: build on ARM
> > 
> >  hw/display/xenfb.c   |   18 ++
> >  hw/xen/xen_backend.c |   18 ++
> >  include/hw/xen/xen_backend.h |2 ++
> >  xen-all.c|2 +-
> >  xen-mapcache.c   |4 ++--
> >  5 files changed, 33 insertions(+), 11 deletions(-)
> > 
> > ___
> > Xen-devel mailing list
> > xen-de...@lists.xen.org
> > http://lists.xen.org/xen-devel
> 

[Qemu-devel] [PATCH] qemu-iotests: adjust tests to work with the NFS protocol

[Peter Lieven]

this adds support for testing the NFS protocol.

Several tests had to be changed from protocol generic to protocol file
because they where not designed to work with something else than file.

This is due mainly due to 3 reasons:
 - the tests use rm, cp or mv shell commands which only work on file
 - the tests use qcow2.py
 - the images construct new filenames (e.g. backing file names) and
   the logic is broken for anything else than file

At least the last option can be addressed, but it is somewhat tricky.

Signed-off-by: Peter Lieven 
---
 tests/qemu-iotests/013   |2 +-
 tests/qemu-iotests/014   |2 +-
 tests/qemu-iotests/016   |2 +-
 tests/qemu-iotests/017   |2 +-
 tests/qemu-iotests/018   |2 +-
 tests/qemu-iotests/019   |2 +-
 tests/qemu-iotests/020   |2 +-
 tests/qemu-iotests/023   |2 +-
 tests/qemu-iotests/024   |2 +-
 tests/qemu-iotests/025   |2 +-
 tests/qemu-iotests/028   |2 +-
 tests/qemu-iotests/031   |2 +-
 tests/qemu-iotests/034   |2 +-
 tests/qemu-iotests/036   |2 +-
 tests/qemu-iotests/037   |2 +-
 tests/qemu-iotests/038   |2 +-
 tests/qemu-iotests/039   |2 +-
 tests/qemu-iotests/043   |2 +-
 tests/qemu-iotests/046   |2 +-
 tests/qemu-iotests/054   |2 +-
 tests/qemu-iotests/059   |2 +-
 tests/qemu-iotests/060   |2 +-
 tests/qemu-iotests/061   |2 +-
 tests/qemu-iotests/063   |2 +-
 tests/qemu-iotests/069   |2 +-
 tests/qemu-iotests/073   |2 +-
 tests/qemu-iotests/common|4 
 tests/qemu-iotests/common.rc |3 +++
 28 files changed, 33 insertions(+), 26 deletions(-)

diff --git a/tests/qemu-iotests/013 b/tests/qemu-iotests/013
index 389f4b8..ea3cab9 100755
--- a/tests/qemu-iotests/013
+++ b/tests/qemu-iotests/013
@@ -41,7 +41,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 
 # much of this could be generic for any format supporting compression.
 _supported_fmt qcow qcow2
-_supported_proto generic
+_supported_proto file
 _supported_os Linux
 
 TEST_OFFSETS="0 4294967296"
diff --git a/tests/qemu-iotests/014 b/tests/qemu-iotests/014
index 0edeb4b..b23c2db 100755
--- a/tests/qemu-iotests/014
+++ b/tests/qemu-iotests/014
@@ -43,7 +43,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 
 # much of this could be generic for any format supporting snapshots
 _supported_fmt qcow2
-_supported_proto generic
+_supported_proto file
 _supported_os Linux
 
 TEST_OFFSETS="0 4294967296"
diff --git a/tests/qemu-iotests/016 b/tests/qemu-iotests/016
index b87a32b..7ea9e94 100755
--- a/tests/qemu-iotests/016
+++ b/tests/qemu-iotests/016
@@ -39,7 +39,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 . ./common.filter
 
 _supported_fmt raw
-_supported_proto file sheepdog
+_supported_proto file sheepdog nfs
 _supported_os Linux
 
 
diff --git a/tests/qemu-iotests/017 b/tests/qemu-iotests/017
index aba3faf..ea8edc6 100755
--- a/tests/qemu-iotests/017
+++ b/tests/qemu-iotests/017
@@ -41,7 +41,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 
 # Any format supporting backing files
 _supported_fmt qcow qcow2 vmdk qed
-_supported_proto generic
+_supported_proto file
 _supported_os Linux
 
 TEST_OFFSETS="0 4294967296"
diff --git a/tests/qemu-iotests/018 b/tests/qemu-iotests/018
index 15fcfe5..aa9d3cb 100755
--- a/tests/qemu-iotests/018
+++ b/tests/qemu-iotests/018
@@ -41,7 +41,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 
 # Any format supporting backing files
 _supported_fmt qcow qcow2 vmdk qed
-_supported_proto generic
+_supported_proto file
 _supported_os Linux
 
 TEST_OFFSETS="0 4294967296"
diff --git a/tests/qemu-iotests/019 b/tests/qemu-iotests/019
index 5bb18d0..d75d125 100755
--- a/tests/qemu-iotests/019
+++ b/tests/qemu-iotests/019
@@ -45,7 +45,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 
 # Any format supporting backing files
 _supported_fmt qcow qcow2 vmdk qed
-_supported_proto generic
+_supported_proto file
 _supported_os Linux
 
 TEST_OFFSETS="0 4294967296"
diff --git a/tests/qemu-iotests/020 b/tests/qemu-iotests/020
index b3c86d8..a42f32f 100755
--- a/tests/qemu-iotests/020
+++ b/tests/qemu-iotests/020
@@ -43,7 +43,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 
 # Any format supporting backing files
 _supported_fmt qcow qcow2 vmdk qed
-_supported_proto generic
+_supported_proto file
 _supported_os Linux
 
 TEST_OFFSETS="0 4294967296"
diff --git a/tests/qemu-iotests/023 b/tests/qemu-iotests/023
index 090ed23..9ad06b9 100755
--- a/tests/qemu-iotests/023
+++ b/tests/qemu-iotests/023
@@ -41,7 +41,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 
 # much of this could be generic for any format supporting compression.
 _supported_fmt qcow qcow2
-_supported_proto generic
+_supported_proto file
 _supported_os Linux
 
 TEST_OFFSETS="0 4294967296"
diff --git a/tests/qemu-iotests/024 b/tests/qemu-iotests/024
index be974f0..9bf99e1 100755
--- a/tests/qemu-iotests/024
+++ b/tests/qemu-iotests/024
@@ -43,7 

Re: [Qemu-devel] [Qemu-ppc] [PATCH] spapr_vscsi: Fix REPORT_LUNS handling

[Paolo Bonzini]

Il 02/01/2014 19:23, Nathan Whitehorn ha scritto:
>>> Let me try to grasp what you're doing here. You're trying to
>>> figure out how many devices there are attached to the bus. For
>>> every device you reserve a buffer block. Lun0 is mandatory, all
>>> others are optional.
>>> 
>>> First off, I think the code would be easier to grasp if you'd
>>> count "number of entries" rather than "number of bytes". That way
>>> we don't have to mentally deal with the 8 byte block
>>> granularity.
>>> 
>>> Then IIUC you're jumping through a lot of hoops to count lun0 if
>>> it's there, but keep it reserved when it's not there. Why don't
>>> you just always reserve entry 0 for lun0? In the loop where
>>> you're actually filling in data you just skip lun0. Or is lun0 a
>>> terminator and always has to come last?

This is simply because you should not report lun 0 twice; even if it is
not defined, LUN 0 is there as a dummy device that only answers a
handful of commands (including INQUIRY and REPORT LUNS).  There are many
ways to write it, but unless you use GArray or something like that, it
will look very much like Nathan and hw/scsi/scsi-bus.c's code.

Paolo

Re: [Qemu-devel] [PATCH v3 0/5] Monitor commands for object-add/del

[Igor Mammedov]

On Fri, 20 Dec 2013 23:21:05 +0100
Paolo Bonzini  wrote:

> These allow hotplugging (and hot-unplugging without leaking an object)
> virtio-rng devices.  They can also be used for memory hotplug.
> 
> v1->v2: fix mistyped underscores in qapi-schema.json
> 
> v2->v3: preserve bisectability by inverting patches 4 and 5, keep lines
> under 80 characters since it can be done easily, drop error_is_set
Reviewed-By & Tested-By: Igor Mammedov 

> 
> Paolo Bonzini (5):
>   rng: initialize file descriptor to -1
>   qom: fix leak for objects created with -object
>   qom: catch errors in object_property_add_child
>   monitor: add object-del (QMP) and object_del (HMP) command
>   monitor: add object-add (QMP) and object_add (HMP) command
> 
>  backends/rng-random.c |  4 +--
>  hmp-commands.hx   | 28 +
>  hmp.c | 67 +
>  hmp.h |  2 ++
>  include/monitor/monitor.h |  3 ++
>  include/qapi/visitor.h|  3 +-
>  include/qemu/typedefs.h   |  2 ++
>  qapi-schema.json  | 34 +
>  qmp-commands.hx   | 51 +++
>  qmp.c | 76 
> +++
>  qom/object.c  |  9 --
>  vl.c  |  3 +-
>  12 files changed, 275 insertions(+), 7 deletions(-)
> 
> -- 
> 1.8.4.2
> 
> 


-- 
Regards,
  Igor

Re: [Qemu-devel] [PATCH v3 0/5] Monitor commands for object-add/del

[Igor Mammedov]

On Fri, 20 Dec 2013 23:21:05 +0100
Paolo Bonzini  wrote:

> These allow hotplugging (and hot-unplugging without leaking an object)
> virtio-rng devices.  They can also be used for memory hotplug.
> 
> v1->v2: fix mistyped underscores in qapi-schema.json
> 
> v2->v3: preserve bisectability by inverting patches 4 and 5, keep lines
> under 80 characters since it can be done easily, drop error_is_set
> 
Looks like there is typo in CC list, so Luiz was never CCed,
adding him to CC so series won't be lost.

-- 
Regards,
  Igor

Re: [Qemu-devel] [PATCH v3 0/5] Monitor commands for object-add/del

[Luiz Capitulino]

On Fri, 3 Jan 2014 15:24:57 +0100
Igor Mammedov  wrote:

> On Fri, 20 Dec 2013 23:21:05 +0100
> Paolo Bonzini  wrote:
> 
> > These allow hotplugging (and hot-unplugging without leaking an object)
> > virtio-rng devices.  They can also be used for memory hotplug.
> > 
> > v1->v2: fix mistyped underscores in qapi-schema.json
> > 
> > v2->v3: preserve bisectability by inverting patches 4 and 5, keep lines
> > under 80 characters since it can be done easily, drop error_is_set
> > 
> Looks like there is typo in CC list, so Luiz was never CCed,
> adding him to CC so series won't be lost.

Paolo did ping in private, but thanks for reminding me anyway.

[Qemu-devel] qemu-seccomp.c:228: error: '__NR_getcpu' undeclared here (not in a function)

[lejeczek]

dear developers

I was hoping someone could suggest what is missing or 
incompatible on my setup that causes a compilation to fail


I'm trying to rpmbuild-compile qemu-1.2.2-1.fc18.src.rpm on 
rhel 6.5
I thought I have got all dependencies from rpm perspective, 
naturally there is something wrong

I think here is where it fails

gcc 
-I/home/pe243/rpmbuild/BUILD/coruscant.ccnr.biotechnology/.el6/qemu-kvm-1.2.0/slirp 
-I. -I/home/pe243/rpmbuild/BUILD/coruscant.ccnr.biotec
hnology/.el6/qemu-kvm-1.2.0 
-I/home/pe243/rpmbuild/BUILD/coruscant.ccnr.biotechnology/.el6/qemu-kvm-1.2.0/fpu 
-I/home/pe243/rpmbuild/BUILD/cor
uscant.ccnr.biotechnology/.el6/qemu-kvm-1.2.0/libcacard 
-fPIE -DPIE -m64 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 
-D_LARGEFILE_SOURCE -Wstrict-pro
totypes -Wredundant-decls -Wall -Wundef -Wwrite-strings 
-Wmissing-prototypes -fno-strict-aliasing -O2 -g -pipe -Wall 
-Wp,-D_FORTIFY_SOURCE=2 -
fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 
-mtune=generic -fPIE -DPIE  -fstack-protector-all 
-Wendif-labels -Wmissing-includ
e-dirs -Wempty-body -Wnested-externs -Wformat-security 
-Wformat-y2k -Winit-self -Wignored-qualifiers 
-Wold-style-declaration -Wold-style-defin
ition -Wtype-limits -I/usr/include/libpng12 
-I/usr/include/spice-server -I/usr/include/cacard 
-I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/
include -I/usr/include/pixman-1 -I/usr/include/nss3 
-I/usr/include/nspr4 -I/usr/include/spice-1 
-I/usr/include/nss3 -I/usr/include/nspr4   -
pthread -I/usr/include/glib-2.0 
-I/usr/lib64/glib-2.0/include -pthread 
-I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -I/home/pe
243/rpmbuild/BUILD/coruscant.ccnr.biotechnology/.el6/qemu-kvm-1.2.0/include 
-MMD -MP -MT qemu-seccomp.o -MF ./qemu-seccomp.d -O2 
-D_FORTIFY_SO

URCE=2 -g  -c -o qemu-seccomp.o qemu-seccomp.c
qemu-seccomp.c:228: error: '__NR_getcpu' undeclared here 
(not in a function)


what can be wrong?
perhaps interestingly package qemu-1.2.0-22.fc19.src.rpm 
rpmbuild-comiles fine


best regards

Re: [Qemu-devel] [PATCH V8 3/8] util: add error_append()

[Peter Crosthwaite]

On Fri, Jan 3, 2014 at 1:08 PM, Wenchao Xia  wrote:
> Signed-off-by: Wenchao Xia 
> ---
>  include/qapi/error.h |6 ++
>  util/error.c |   21 +
>  2 files changed, 27 insertions(+), 0 deletions(-)
>
> diff --git a/include/qapi/error.h b/include/qapi/error.h
> index 7d4c696..bcfb724 100644
> --- a/include/qapi/error.h
> +++ b/include/qapi/error.h
> @@ -30,6 +30,12 @@ typedef struct Error Error;
>  void error_set(Error **err, ErrorClass err_class, const char *fmt, ...) 
> GCC_FMT_ATTR(3, 4);
>
>  /**
> + * Append message to err if err != NULL && *err != NULL. "\n" will be 
> inserted
> + * after old message.
> + */
> +void error_append(Error **err, const char *fmt, ...) GCC_FMT_ATTR(2, 3);
> +
> +/**
>   * Set an indirect pointer to an error given a ErrorClass value and a
>   * printf-style human message, followed by a strerror() string if
>   * @os_error is not zero.
> diff --git a/util/error.c b/util/error.c
> index 3ee362a..64bbb2d 100644
> --- a/util/error.c
> +++ b/util/error.c
> @@ -46,6 +46,27 @@ void error_set(Error **errp, ErrorClass err_class, const 
> char *fmt, ...)
>  errno = saved_errno;
>  }
>
> +void error_append(Error **err, const char *fmt, ...)

errp

> +{
> +va_list ap;
> +gchar *msg, *msg_old;
> +
> +if (!err || !*err) {

Should appending to an unset error really be a nop? You should just
set the error as normal in this case (error_set()?).

This avoids callers having to:

if (error_is_set(&err)) {
   error_append(&err);
} else {
   error_set(&err);
}

so they can just append if they want appending.

Generally speaking, appending to nothing should give you something.

Regards,
Peter

> +return;
> +}
> +
> +va_start(ap, fmt);
> +msg = g_strdup_vprintf(fmt, ap);
> +va_end(ap);
> +
> +msg_old = (*err)->msg;
> +(*err)->msg = g_strdup_printf("%s\n%s", msg_old, msg);
> +
> +g_free(msg);
> +g_free(msg_old);
> +
> +}
> +
>  void error_set_errno(Error **errp, int os_errno, ErrorClass err_class,
>   const char *fmt, ...)
>  {
> --
> 1.7.1
>
>

Re: [Qemu-devel] [PATCH target-arm v1 1/1] arm/xilinx_zynq: Always instantiate the GEMs

[Peter Crosthwaite]

On Fri, Jan 3, 2014 at 8:51 PM, Peter Maydell  wrote:
> On 3 January 2014 03:21, Peter Crosthwaite  
> wrote:
>> Don't conditionalise GEM instantiation on networking attachments. The
>> device should always be present even if not attached to a network.
>>
>> This allows for probing of the device by expectant guests (such as
>> OS's).  This is needed because sysbus (or AXI in Xilinx's real hw case)
>> is not self identifying so the guest has no dynamic way of detecting
>> device absence.
>
> Agreed that this is the right thing.
>
> Some day I might try to look into how to update our handling
> of embedded NICs to work with non-legacy command lines...
>
>
>> -for (n = 0; n < nb_nics; n++) {
>> -nd = &nd_table[n];
>> +for (n = 0; n < 2; n++) {
>> +NICInfo *nd = n < nb_nics ? &nd_table[n] : NULL;
>>  if (n == 0) {
>>  gem_init(nd, 0xE000B000, pic[54-IRQ_OFFSET]);
>>  } else if (n == 1) {
>
> This is now a rather odd loop which goes round exactly twice
> and uses an if() statement to make the body different each time.

Yeh, I think I was going for minimum diff when I did this one. Will fix.

>
> Instead you can just say:
>  gem_init(nd_table[0], 0xe000b000, pic[54 - IRQ_OFFSET]);
>  gem_init(nd_table[1], 0xe000c000, pic[54 - IRQ_OFFSET]);
>
> and have gem_init() condition the calls to qdev_set_nic_properties

One subtle question before respinning - should the
qdev_set_nic_properties actually be conditional? Setting it to an
unused NIC should be harmless, so perhaps it should be outside the
if(nd->used). Other boards/MAC combos that dont do the
qdev_check_nic_model can then just dispose of the if (nd->used) check
altogether.

Regards,
Peter

> and qdev_check_nic_model on "if (nd->used)".
>
> thanks
> -- PMM
>

Re: [Qemu-devel] [PATCH target-arm v1 1/1] arm/xilinx_zynq: Always instantiate the GEMs

[Peter Maydell]

On 3 January 2014 16:15, Peter Crosthwaite  wrote:
> One subtle question before respinning - should the
> qdev_set_nic_properties actually be conditional? Setting it to an
> unused NIC should be harmless, so perhaps it should be outside the
> if(nd->used). Other boards/MAC combos that dont do the
> qdev_check_nic_model can then just dispose of the if (nd->used) check
> altogether.

Well qdev_set_nic_properties as it stands today will unconditionally
assume you've passed it a valid NICInfo, so we have to a void calling
it if nd->used isn't set. If you're asking whether that nd->used check
should be moved into qdev_set_nic_properties, I'm not sure : I don't
have a good enough grasp of how the net code works and how it ought
to work for embedded always-instantiated NICs to be able to say.

thanks
-- PMM

Re: [Qemu-devel] [PATCH target-arm v1 1/1] arm/xilinx_zynq: Always instantiate the GEMs

[Peter Crosthwaite]

On Sat, Jan 4, 2014 at 2:22 AM, Peter Maydell  wrote:
> On 3 January 2014 16:15, Peter Crosthwaite  
> wrote:
>> One subtle question before respinning - should the
>> qdev_set_nic_properties actually be conditional? Setting it to an
>> unused NIC should be harmless, so perhaps it should be outside the
>> if(nd->used). Other boards/MAC combos that dont do the
>> qdev_check_nic_model can then just dispose of the if (nd->used) check
>> altogether.
>
> Well qdev_set_nic_properties as it stands today will unconditionally
> assume you've passed it a valid NICInfo, so we have to a void calling
> it if nd->used isn't set.

So I guess I'm thinking that the unusued nics should be a "valid"
NICInfo - just one that doesn't connect to anything. Currently,
qdev_set_nic_properties() sets three props - "mac", "netdev" and
"vectors". "mac" looks dubious to me, I'm not sure why the net layer
is telling devices what their MAC addresses are - it should be the
other way round. Setting of "netdev" is already protected against NULL
setting so that one is handled. "vectors" looks like it has a sanity
check guard on its setting as well via the DEV_NVECTORS_UNSPECIFIED
check. Although that value may rely on some net.c code paths for
non-NULL NICs.

Will experiment with it before net spin. Will just leave it in inside
the "if" if it goes pear shaped on me.

> If you're asking whether that nd->used check
> should be moved into qdev_set_nic_properties, I'm not sure : I don't

And that's probably still preferable to having all boards add null
guards on setting a qdev property.

Regards,
Peter

> have a good enough grasp of how the net code works and how it ought
> to work for embedded always-instantiated NICs to be able to say.
>
> thanks
> -- PMM
>

Re: [Qemu-devel] [PATCH target-arm v1 1/1] arm/xilinx_zynq: Always instantiate the GEMs

[Peter Maydell]

On 3 January 2014 16:56, Peter Crosthwaite  wrote:
> On Sat, Jan 4, 2014 at 2:22 AM, Peter Maydell  
> wrote:
>> On 3 January 2014 16:15, Peter Crosthwaite  
>> wrote:
>>> One subtle question before respinning - should the
>>> qdev_set_nic_properties actually be conditional? Setting it to an
>>> unused NIC should be harmless, so perhaps it should be outside the
>>> if(nd->used). Other boards/MAC combos that dont do the
>>> qdev_check_nic_model can then just dispose of the if (nd->used) check
>>> altogether.
>>
>> Well qdev_set_nic_properties as it stands today will unconditionally
>> assume you've passed it a valid NICInfo, so we have to a void calling
>> it if nd->used isn't set.
>
> So I guess I'm thinking that the unusued nics should be a "valid"
> NICInfo - just one that doesn't connect to anything. Currently,
> qdev_set_nic_properties() sets three props - "mac", "netdev" and
> "vectors". "mac" looks dubious to me, I'm not sure why the net layer
> is telling devices what their MAC addresses are - it should be the
> other way round.

The device doesn't (in the old -net model) know what its MAC address
is, because that's something you specify in the -net command line option.
So this is the passing on of that info to the device.

> Setting of "netdev" is already protected against NULL
> setting so that one is handled. "vectors" looks like it has a sanity
> check guard on its setting as well via the DEV_NVECTORS_UNSPECIFIED
> check. Although that value may rely on some net.c code paths for
> non-NULL NICs.

I think it would probably be better to look into properly cleaning up
the legacy -net nic support and moving embedded devices over to
work with some variant of the new command line model, rather
than tweaking the legacy stuff. nd_table eventually should ideally
just go away I think.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 1/2] hw/net: add support for Allwinner EMAC Fast Ethernet controller

[Beniamino Galvani]

On Fri, Jan 03, 2014 at 11:26:01AM +1000, Peter Crosthwaite wrote:
> >> > +static const VMStateDescription vmstate_tx_fifo = {
> >> > +.name = "allwinner_emac_tx_fifo",
> >> > +.version_id = 1,
> >> > +.minimum_version_id = 1,
> >> > +.fields = (VMStateField[]) {
> >> > +VMSTATE_UINT32_ARRAY(data, AwEmacTxFifo, FIFO_SIZE >> 2),
> >> > +VMSTATE_INT32(length, AwEmacTxFifo),
> >> > +VMSTATE_INT32(offset, AwEmacTxFifo),
> >> > +VMSTATE_END_OF_LIST()
> >> > +}
> >> > +};
> >>
> >> This might warrant a dup of fifo8 as fifo32 if you want to clean this
> >> up. I think I have a patch handy that might help, will send tmrw.
> >> Check util/fifo8.c for fifo8 example.
> >
> > Yes, probably AwEmacTxFifo can be replaced with Fifo32.
> >

I need to obtain a pointer to the fifo backing buffer after the frame
has been copied completely in order to pass it to qemu_send_packet().
The generic fifo implementation doesn't allow that (unless I access
directly the structure member, but I suppose that the intent is to
treat the fifo object as opaque).

Maybe a function fifo_get_data() could be added to the generic
implementation? Otherwise I can go on using my custom implementation.

Beniamino

[Qemu-devel] [V5 PATCH 04/14] target-ppc: VSX Stage 4: Refactor stxsdx

[Tom Musta]

This patch refactors the stxsdx instruction.  Reusable code is
extracted into a macro which will be used in subsequent patches
in this series.

Signed-off-by: Tom Musta 
Reviewed-by: Richard Henderson 
---
 target-ppc/translate.c |   27 +++
 1 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 958ea94..9f3dda7 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -7112,20 +7112,23 @@ static void gen_lxvw4x(DisasContext *ctx)
 tcg_temp_free_i64(tmp);
 }
 
-static void gen_stxsdx(DisasContext *ctx)
-{
-TCGv EA;
-if (unlikely(!ctx->vsx_enabled)) {
-gen_exception(ctx, POWERPC_EXCP_VSXU);
-return;
-}
-gen_set_access_type(ctx, ACCESS_INT);
-EA = tcg_temp_new();
-gen_addr_reg_index(ctx, EA);
-gen_qemu_st64(ctx, cpu_vsrh(xS(ctx->opcode)), EA);
-tcg_temp_free(EA);
+#define VSX_STORE_SCALAR(name, operation) \
+static void gen_##name(DisasContext *ctx) \
+{ \
+TCGv EA;  \
+if (unlikely(!ctx->vsx_enabled)) {\
+gen_exception(ctx, POWERPC_EXCP_VSXU);\
+return;   \
+} \
+gen_set_access_type(ctx, ACCESS_INT); \
+EA = tcg_temp_new();  \
+gen_addr_reg_index(ctx, EA);  \
+gen_qemu_##operation(ctx, cpu_vsrh(xS(ctx->opcode)), EA); \
+tcg_temp_free(EA);\
 }
 
+VSX_STORE_SCALAR(stxsdx, st64)
+
 static void gen_stxvd2x(DisasContext *ctx)
 {
 TCGv EA;
-- 
1.7.1

[Qemu-devel] [V5 PATCH 03/14] target-ppc: VSX Stage 4: Add lxsiwax, lxsiwzx and lxsspx

[Tom Musta]

This patch adds the scalar load instructions introduced in ISA
V2.07:

  - Load VSX Scalar as Integer Word Algebraic Indexd (lxsiwax)
  - Load VSX Scalar as Integer Word and Zero Indexed (lxsiwzx)
  - Load VSX Scalar Single-Precision Indexed (lxsspx)

Signed-off-by: Tom Musta 
Reviewed-by: Richard Henderson 
---
V5: Updated to fix tcg-debug compilation failures.

 target-ppc/translate.c |   14 ++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index ca26dcf..958ea94 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -2585,6 +2585,14 @@ static inline void gen_qemu_ld32s(DisasContext *ctx, 
TCGv arg1, TCGv arg2)
 tcg_gen_qemu_ld32s(arg1, arg2, ctx->mem_idx);
 }
 
+static void gen_qemu_ld32s_i64(DisasContext *ctx, TCGv_i64 val, TCGv addr)
+{
+TCGv tmp = tcg_temp_new();
+gen_qemu_ld32s(ctx, tmp, addr);
+tcg_gen_ext_tl_i64(val, tmp);
+tcg_temp_free(tmp);
+}
+
 static inline void gen_qemu_ld64(DisasContext *ctx, TCGv_i64 arg1, TCGv arg2)
 {
 tcg_gen_qemu_ld64(arg1, arg2, ctx->mem_idx);
@@ -7039,6 +7047,9 @@ static void gen_##name(DisasContext *ctx) 
\
 }
 
 VSX_LOAD_SCALAR(lxsdx, ld64)
+VSX_LOAD_SCALAR(lxsiwax, ld32s_i64)
+VSX_LOAD_SCALAR(lxsiwzx, ld32u_i64)
+VSX_LOAD_SCALAR(lxsspx, ld32fs)
 
 static void gen_lxvd2x(DisasContext *ctx)
 {
@@ -10044,6 +10055,9 @@ GEN_VAFORM_PAIRED(vsel, vperm, 21),
 GEN_VAFORM_PAIRED(vmaddfp, vnmsubfp, 23),
 
 GEN_HANDLER_E(lxsdx, 0x1F, 0x0C, 0x12, 0, PPC_NONE, PPC2_VSX),
+GEN_HANDLER_E(lxsiwax, 0x1F, 0x0C, 0x02, 0, PPC_NONE, PPC2_VSX207),
+GEN_HANDLER_E(lxsiwzx, 0x1F, 0x0C, 0x00, 0, PPC_NONE, PPC2_VSX207),
+GEN_HANDLER_E(lxsspx, 0x1F, 0x0C, 0x10, 0, PPC_NONE, PPC2_VSX207),
 GEN_HANDLER_E(lxvd2x, 0x1F, 0x0C, 0x1A, 0, PPC_NONE, PPC2_VSX),
 GEN_HANDLER_E(lxvdsx, 0x1F, 0x0C, 0x0A, 0, PPC_NONE, PPC2_VSX),
 GEN_HANDLER_E(lxvw4x, 0x1F, 0x0C, 0x18, 0, PPC_NONE, PPC2_VSX),
-- 
1.7.1

[Qemu-devel] [V5 PATCH 05/14] target-ppc: VSX Stage 4: Add stxsiwx and stxsspx

[Tom Musta]

This patch adds two store scalar instructions:

  - Store VSX Scalar as Integer Word Indexed (stxsiwx)
  - Store VSX Scalar Single-Precision Indexed (stxsspx)

Signed-off-by: Tom Musta 
Reviewed-by: Richard Henderson 
---
V5: Updated to address tcg-debug compliation errors.

 target-ppc/translate.c |4 
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 9f3dda7..28794d1 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -7128,6 +7128,8 @@ static void gen_##name(DisasContext *ctx) 
\
 }
 
 VSX_STORE_SCALAR(stxsdx, st64)
+VSX_STORE_SCALAR(stxsiwx, st32_i64)
+VSX_STORE_SCALAR(stxsspx, st32fs)
 
 static void gen_stxvd2x(DisasContext *ctx)
 {
@@ -10066,6 +10068,8 @@ GEN_HANDLER_E(lxvdsx, 0x1F, 0x0C, 0x0A, 0, PPC_NONE, 
PPC2_VSX),
 GEN_HANDLER_E(lxvw4x, 0x1F, 0x0C, 0x18, 0, PPC_NONE, PPC2_VSX),
 
 GEN_HANDLER_E(stxsdx, 0x1F, 0xC, 0x16, 0, PPC_NONE, PPC2_VSX),
+GEN_HANDLER_E(stxsiwx, 0x1F, 0xC, 0x04, 0, PPC_NONE, PPC2_VSX207),
+GEN_HANDLER_E(stxsspx, 0x1F, 0xC, 0x14, 0, PPC_NONE, PPC2_VSX207),
 GEN_HANDLER_E(stxvd2x, 0x1F, 0xC, 0x1E, 0, PPC_NONE, PPC2_VSX),
 GEN_HANDLER_E(stxvw4x, 0x1F, 0xC, 0x1C, 0, PPC_NONE, PPC2_VSX),
 
-- 
1.7.1

[Qemu-devel] [V5 PATCH 00/14] [V5 PATCH 00/14] target-ppc: VSX Stage 4

[Tom Musta]

This is the fourth and final series of patches that add emulation support
to QEMU for the PowerPC Vector Scalar Extension (VSX).

This series adds the instructions that were newly introduced with Power ISA
V2.07.  This includes 3 scalar load instructions, 2 scalar store instructions,
7 standard single precision scalar arithmetic instructions, 8 scalar single
precision fused multiply/add instructions, two integer-to-single-precision
conversion instructions and 3 vector logical instructions.

The single-precision scalar arithmetic instructions all interpret the most
significant 64 bits of a VSR as a single precision floating point number
stored in double precision format (similar to the standard PowerPC floating
point single precision instructions).  Thus a common theme in the supporting
code is rounding of an intermediate double-precision number to single 
precision.

V2: (a) Changed the rounding to single precision to reuse the existing
helper_frsp() routine.  (b) Re-implemented the fused multiply/add instructions
to use float32_muladd instead of float64_muladd, which avoids subtle rounding
errors.

V3: Re-implemented fused multiply/add per clarification from Richard Henderson.

V4: Changed fused multiply/add to use helper_frsp (inadvertently re-injected
when I used an earlier patch).  

V5: Fixed tcg compilation problems.

Tom Musta (14):
  target-ppc: VSX Stage 4: Add VSX 2.07 Flag
  target-ppc: VSX Stage 4: Refactor lxsdx
  target-ppc: VSX Stage 4: Add lxsiwax, lxsiwzx and lxsspx
  target-ppc: VSX Stage 4: Refactor stxsdx
  target-ppc: VSX Stage 4: Add stxsiwx and stxsspx
  target-ppc: VSX Stage 4: Add xsaddsp and xssubsp
  target-ppc: VSX Stage 4: Add xsmulsp
  target-ppc: VSX Stage 4: Add xsdivsp
  target-ppc: VSX Stage 4: Add xsresp
  target-ppc: VSX Stage 4: Add xssqrtsp
  target-ppc: VSX Stage 4: add xsrsqrtesp
  target-ppc: VSX Stage 4: Add Scalar SP Fused Multiply-Adds
  target-ppc: VSX Stage 4: Add xscvsxdsp and xscvuxdsp
  target-ppc: VSX Stage 4: Add xxleqv, xxlnand and xxlorc

 target-ppc/cpu.h|4 +-
 target-ppc/fpu_helper.c |  195 ---
 target-ppc/helper.h |   18 
 target-ppc/translate.c  |  116 --
 target-ppc/translate_init.c |2 +-
 5 files changed, 241 insertions(+), 94 deletions(-)

[Qemu-devel] [V5 PATCH 01/14] target-ppc: VSX Stage 4: Add VSX 2.07 Flag

[Tom Musta]

This patch adds a flag to identify those VSX instructions that are
new to Power ISA V2.07.  The flag is added to the Power 8 processor
initialization so that the P8 models understand how to decode and
emulate instructions in this category.

Signed-off-by: Tom Musta 
Reviewed-by: Richard Henderson 
---
 target-ppc/cpu.h|4 +++-
 target-ppc/translate_init.c |2 +-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index bb84767..0abc848 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -1875,9 +1875,11 @@ enum {
 PPC2_DBRX  = 0x0010ULL,
 /* Book I 2.05 PowerPC specification */
 PPC2_ISA205= 0x0020ULL,
+/* VSX additions in ISA 2.07 */
+PPC2_VSX207= 0x0040ULL,
 
 #define PPC_TCG_INSNS2 (PPC2_BOOKE206 | PPC2_VSX | PPC2_PRCNTL | PPC2_DBRX | \
-  PPC2_ISA205)
+PPC2_ISA205 | PPC2_VSX207)
 };
 
 /*/
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index c030a20..dd57df3 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -7312,7 +7312,7 @@ POWERPC_FAMILY(POWER8)(ObjectClass *oc, void *data)
PPC_64B | PPC_ALTIVEC |
PPC_SEGMENT_64B | PPC_SLBI |
PPC_POPCNTB | PPC_POPCNTWD;
-pcc->insns_flags2 = PPC2_VSX | PPC2_DFP | PPC2_DBRX;
+pcc->insns_flags2 = PPC2_VSX | PPC2_VSX207 | PPC2_DFP | PPC2_DBRX;
 pcc->msr_mask = 0x8284FF36ULL;
 pcc->mmu_model = POWERPC_MMU_2_06;
 #if defined(CONFIG_SOFTMMU)
-- 
1.7.1

[Qemu-devel] [V5 PATCH 02/14] target-ppc: VSX Stage 4: Refactor lxsdx

[Tom Musta]

This patch refactors the lxsdx generator. Resuable code is isolated
into a macro.  The macro will be used in subsequent patches in this
series to implement other scalar load instructions.

Signed-off-by: Tom Musta 
Reviewed-by: Richard Henderson 
---
 target-ppc/translate.c |   31 +--
 1 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 79be8ed..ca26dcf 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -7022,20 +7022,23 @@ static inline TCGv_i64 cpu_vsrl(int n)
 }
 }
 
-static void gen_lxsdx(DisasContext *ctx)
-{
-TCGv EA;
-if (unlikely(!ctx->vsx_enabled)) {
-gen_exception(ctx, POWERPC_EXCP_VSXU);
-return;
-}
-gen_set_access_type(ctx, ACCESS_INT);
-EA = tcg_temp_new();
-gen_addr_reg_index(ctx, EA);
-gen_qemu_ld64(ctx, cpu_vsrh(xT(ctx->opcode)), EA);
-/* NOTE: cpu_vsrl is undefined */
-tcg_temp_free(EA);
-}
+#define VSX_LOAD_SCALAR(name, operation)  \
+static void gen_##name(DisasContext *ctx) \
+{ \
+TCGv EA;  \
+if (unlikely(!ctx->vsx_enabled)) {\
+gen_exception(ctx, POWERPC_EXCP_VSXU);\
+return;   \
+} \
+gen_set_access_type(ctx, ACCESS_INT); \
+EA = tcg_temp_new();  \
+gen_addr_reg_index(ctx, EA);  \
+gen_qemu_##operation(ctx, cpu_vsrh(xT(ctx->opcode)), EA); \
+/* NOTE: cpu_vsrl is undefined */ \
+tcg_temp_free(EA);\
+}
+
+VSX_LOAD_SCALAR(lxsdx, ld64)
 
 static void gen_lxvd2x(DisasContext *ctx)
 {
-- 
1.7.1

[Qemu-devel] [V5 PATCH 10/14] target-ppc: VSX Stage 4: Add xssqrtsp

[Tom Musta]

This patch adds the VSX Scalar Square Root Single Precision (xssqrtsp)
instruction.

The existing VSX_SQRT() macro is modified to support rounding of the
intermediate double-precision result to single-precision.

Signed-off-by: Tom Musta 
Reviewed-by: Richard Henderson 
---
V2: Updated conversion to single precision range.

 target-ppc/fpu_helper.c |   13 +
 target-ppc/helper.h |1 +
 target-ppc/translate.c  |2 ++
 3 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/target-ppc/fpu_helper.c b/target-ppc/fpu_helper.c
index ac52c23..fec9d1b 100644
--- a/target-ppc/fpu_helper.c
+++ b/target-ppc/fpu_helper.c
@@ -1969,7 +1969,7 @@ VSX_RE(xvresp, 4, float32, f32, 0, 0)
  *   fld   - vsr_t field (f32 or f64)
  *   sfprf - set FPRF
  */
-#define VSX_SQRT(op, nels, tp, fld, sfprf)   \
+#define VSX_SQRT(op, nels, tp, fld, sfprf, r2sp) \
 void helper_##op(CPUPPCState *env, uint32_t opcode)  \
 {\
 ppc_vsr_t xt, xb;\
@@ -1993,6 +1993,10 @@ void helper_##op(CPUPPCState *env, uint32_t opcode)  
\
 }\
 }\
  \
+if (r2sp) {  \
+xt.fld[i] = helper_frsp(env, xt.fld[i]); \
+}\
+ \
 if (sfprf) { \
 helper_compute_fprf(env, xt.fld[i], sfprf);  \
 }\
@@ -2002,9 +2006,10 @@ void helper_##op(CPUPPCState *env, uint32_t opcode)  
\
 helper_float_check_status(env);  \
 }
 
-VSX_SQRT(xssqrtdp, 1, float64, f64, 1)
-VSX_SQRT(xvsqrtdp, 2, float64, f64, 0)
-VSX_SQRT(xvsqrtsp, 4, float32, f32, 0)
+VSX_SQRT(xssqrtdp, 1, float64, f64, 1, 0)
+VSX_SQRT(xssqrtsp, 1, float64, f64, 1, 1)
+VSX_SQRT(xvsqrtdp, 2, float64, f64, 0, 0)
+VSX_SQRT(xvsqrtsp, 4, float32, f32, 0, 0)
 
 /* VSX_RSQRTE - VSX floating point reciprocal square root estimate
  *   op- instruction mnemonic
diff --git a/target-ppc/helper.h b/target-ppc/helper.h
index b1cf3c0..0192043 100644
--- a/target-ppc/helper.h
+++ b/target-ppc/helper.h
@@ -291,6 +291,7 @@ DEF_HELPER_2(xssubsp, void, env, i32)
 DEF_HELPER_2(xsmulsp, void, env, i32)
 DEF_HELPER_2(xsdivsp, void, env, i32)
 DEF_HELPER_2(xsresp, void, env, i32)
+DEF_HELPER_2(xssqrtsp, void, env, i32)
 
 DEF_HELPER_2(xvadddp, void, env, i32)
 DEF_HELPER_2(xvsubdp, void, env, i32)
diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 3108a29..f4c1f42 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -7363,6 +7363,7 @@ GEN_VSX_HELPER_2(xssubsp, 0x00, 0x01, 0, PPC2_VSX207)
 GEN_VSX_HELPER_2(xsmulsp, 0x00, 0x02, 0, PPC2_VSX207)
 GEN_VSX_HELPER_2(xsdivsp, 0x00, 0x03, 0, PPC2_VSX207)
 GEN_VSX_HELPER_2(xsresp, 0x14, 0x01, 0, PPC2_VSX207)
+GEN_VSX_HELPER_2(xssqrtsp, 0x16, 0x00, 0, PPC2_VSX207)
 
 GEN_VSX_HELPER_2(xvadddp, 0x00, 0x0C, 0, PPC2_VSX)
 GEN_VSX_HELPER_2(xvsubdp, 0x00, 0x0D, 0, PPC2_VSX)
@@ -10175,6 +10176,7 @@ GEN_XX3FORM(xssubsp, 0x00, 0x01, PPC2_VSX207),
 GEN_XX3FORM(xsmulsp, 0x00, 0x02, PPC2_VSX207),
 GEN_XX3FORM(xsdivsp, 0x00, 0x03, PPC2_VSX207),
 GEN_XX2FORM(xsresp,  0x14, 0x01, PPC2_VSX207),
+GEN_XX2FORM(xssqrtsp,  0x16, 0x00, PPC2_VSX207),
 
 GEN_XX3FORM(xvadddp, 0x00, 0x0C, PPC2_VSX),
 GEN_XX3FORM(xvsubdp, 0x00, 0x0D, PPC2_VSX),
-- 
1.7.1

[Qemu-devel] [V5 PATCH 06/14] target-ppc: VSX Stage 4: Add xsaddsp and xssubsp

[Tom Musta]

This patch adds the VSX Scalar Add Single-Precision (xsaddsp) and
VSX Scalar Subtract Single-Precision (xssubsp) instructions.

The existing VSX_ADD_SUB macro is modified to support the rounding
of the (intermediate) result to single-precision.

Signed-off-by: Tom Musta 
Reviewed-by: Richard Henderson 
---
V2: updated conversion of result to single precision.

 target-ppc/fpu_helper.c |   20 +---
 target-ppc/helper.h |3 +++
 target-ppc/translate.c  |6 ++
 3 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/target-ppc/fpu_helper.c b/target-ppc/fpu_helper.c
index 3165ef0..f047640 100644
--- a/target-ppc/fpu_helper.c
+++ b/target-ppc/fpu_helper.c
@@ -1768,7 +1768,7 @@ static void putVSR(int n, ppc_vsr_t *vsr, CPUPPCState 
*env)
  *   fld   - vsr_t field (f32 or f64)
  *   sfprf - set FPRF
  */
-#define VSX_ADD_SUB(name, op, nels, tp, fld, sfprf)  \
+#define VSX_ADD_SUB(name, op, nels, tp, fld, sfprf, r2sp)\
 void helper_##name(CPUPPCState *env, uint32_t opcode)\
 {\
 ppc_vsr_t xt, xa, xb;\
@@ -1794,6 +1794,10 @@ void helper_##name(CPUPPCState *env, uint32_t opcode)
\
 }\
 }\
  \
+if (r2sp) {  \
+xt.fld[i] = helper_frsp(env, xt.fld[i]); \
+}\
+ \
 if (sfprf) { \
 helper_compute_fprf(env, xt.fld[i], sfprf);  \
 }\
@@ -1802,12 +1806,14 @@ void helper_##name(CPUPPCState *env, uint32_t opcode)   
 \
 helper_float_check_status(env);  \
 }
 
-VSX_ADD_SUB(xsadddp, add, 1, float64, f64, 1)
-VSX_ADD_SUB(xvadddp, add, 2, float64, f64, 0)
-VSX_ADD_SUB(xvaddsp, add, 4, float32, f32, 0)
-VSX_ADD_SUB(xssubdp, sub, 1, float64, f64, 1)
-VSX_ADD_SUB(xvsubdp, sub, 2, float64, f64, 0)
-VSX_ADD_SUB(xvsubsp, sub, 4, float32, f32, 0)
+VSX_ADD_SUB(xsadddp, add, 1, float64, f64, 1, 0)
+VSX_ADD_SUB(xsaddsp, add, 1, float64, f64, 1, 1)
+VSX_ADD_SUB(xvadddp, add, 2, float64, f64, 0, 0)
+VSX_ADD_SUB(xvaddsp, add, 4, float32, f32, 0, 0)
+VSX_ADD_SUB(xssubdp, sub, 1, float64, f64, 1, 0)
+VSX_ADD_SUB(xssubsp, sub, 1, float64, f64, 1, 1)
+VSX_ADD_SUB(xvsubdp, sub, 2, float64, f64, 0, 0)
+VSX_ADD_SUB(xvsubsp, sub, 4, float32, f32, 0, 0)
 
 /* VSX_MUL - VSX floating point multiply
  *   op- instruction mnemonic
diff --git a/target-ppc/helper.h b/target-ppc/helper.h
index 0276b02..696b9d3 100644
--- a/target-ppc/helper.h
+++ b/target-ppc/helper.h
@@ -286,6 +286,9 @@ DEF_HELPER_2(xsrdpim, void, env, i32)
 DEF_HELPER_2(xsrdpip, void, env, i32)
 DEF_HELPER_2(xsrdpiz, void, env, i32)
 
+DEF_HELPER_2(xsaddsp, void, env, i32)
+DEF_HELPER_2(xssubsp, void, env, i32)
+
 DEF_HELPER_2(xvadddp, void, env, i32)
 DEF_HELPER_2(xvsubdp, void, env, i32)
 DEF_HELPER_2(xvmuldp, void, env, i32)
diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 28794d1..c50d800 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -7358,6 +7358,9 @@ GEN_VSX_HELPER_2(xsrdpim, 0x12, 0x07, 0, PPC2_VSX)
 GEN_VSX_HELPER_2(xsrdpip, 0x12, 0x06, 0, PPC2_VSX)
 GEN_VSX_HELPER_2(xsrdpiz, 0x12, 0x05, 0, PPC2_VSX)
 
+GEN_VSX_HELPER_2(xsaddsp, 0x00, 0x00, 0, PPC2_VSX207)
+GEN_VSX_HELPER_2(xssubsp, 0x00, 0x01, 0, PPC2_VSX207)
+
 GEN_VSX_HELPER_2(xvadddp, 0x00, 0x0C, 0, PPC2_VSX)
 GEN_VSX_HELPER_2(xvsubdp, 0x00, 0x0D, 0, PPC2_VSX)
 GEN_VSX_HELPER_2(xvmuldp, 0x00, 0x0E, 0, PPC2_VSX)
@@ -10164,6 +10167,9 @@ GEN_XX2FORM(xsrdpim, 0x12, 0x07, PPC2_VSX),
 GEN_XX2FORM(xsrdpip, 0x12, 0x06, PPC2_VSX),
 GEN_XX2FORM(xsrdpiz, 0x12, 0x05, PPC2_VSX),
 
+GEN_XX3FORM(xsaddsp, 0x00, 0x00, PPC2_VSX207),
+GEN_XX3FORM(xssubsp, 0x00, 0x01, PPC2_VSX207),
+
 GEN_XX3FORM(xvadddp, 0x00, 0x0C, PPC2_VSX),
 GEN_XX3FORM(xvsubdp, 0x00, 0x0D, PPC2_VSX),
 GEN_XX3FORM(xvmuldp, 0x00, 0x0E, PPC2_VSX),
-- 
1.7.1

[Qemu-devel] [V5 PATCH 11/14] target-ppc: VSX Stage 4: add xsrsqrtesp

[Tom Musta]

This patch adds the VSX Scalar Reciprocal Square Root Estimate
Single Precision (xsrsqrtesp) instruction.

The existing VSX_RSQRTE() macro is modified to support rounding
of the intermediate double-precision result to single precision.

Signed-off-by: Tom Musta 
Reviewed-by: Richard Henderson 
---
V2: Updated conversion to single precision range.

 target-ppc/fpu_helper.c |   13 +
 target-ppc/helper.h |1 +
 target-ppc/translate.c  |2 ++
 3 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/target-ppc/fpu_helper.c b/target-ppc/fpu_helper.c
index fec9d1b..33da462 100644
--- a/target-ppc/fpu_helper.c
+++ b/target-ppc/fpu_helper.c
@@ -2018,7 +2018,7 @@ VSX_SQRT(xvsqrtsp, 4, float32, f32, 0, 0)
  *   fld   - vsr_t field (f32 or f64)
  *   sfprf - set FPRF
  */
-#define VSX_RSQRTE(op, nels, tp, fld, sfprf) \
+#define VSX_RSQRTE(op, nels, tp, fld, sfprf, r2sp)   \
 void helper_##op(CPUPPCState *env, uint32_t opcode)  \
 {\
 ppc_vsr_t xt, xb;\
@@ -2043,6 +2043,10 @@ void helper_##op(CPUPPCState *env, uint32_t opcode)  
\
 }\
 }\
  \
+if (r2sp) {  \
+xt.fld[i] = helper_frsp(env, xt.fld[i]); \
+}\
+ \
 if (sfprf) { \
 helper_compute_fprf(env, xt.fld[i], sfprf);  \
 }\
@@ -2052,9 +2056,10 @@ void helper_##op(CPUPPCState *env, uint32_t opcode)  
\
 helper_float_check_status(env);  \
 }
 
-VSX_RSQRTE(xsrsqrtedp, 1, float64, f64, 1)
-VSX_RSQRTE(xvrsqrtedp, 2, float64, f64, 0)
-VSX_RSQRTE(xvrsqrtesp, 4, float32, f32, 0)
+VSX_RSQRTE(xsrsqrtedp, 1, float64, f64, 1, 0)
+VSX_RSQRTE(xsrsqrtesp, 1, float64, f64, 1, 1)
+VSX_RSQRTE(xvrsqrtedp, 2, float64, f64, 0, 0)
+VSX_RSQRTE(xvrsqrtesp, 4, float32, f32, 0, 0)
 
 static inline int ppc_float32_get_unbiased_exp(float32 f)
 {
diff --git a/target-ppc/helper.h b/target-ppc/helper.h
index 0192043..84c6ee1 100644
--- a/target-ppc/helper.h
+++ b/target-ppc/helper.h
@@ -292,6 +292,7 @@ DEF_HELPER_2(xsmulsp, void, env, i32)
 DEF_HELPER_2(xsdivsp, void, env, i32)
 DEF_HELPER_2(xsresp, void, env, i32)
 DEF_HELPER_2(xssqrtsp, void, env, i32)
+DEF_HELPER_2(xsrsqrtesp, void, env, i32)
 
 DEF_HELPER_2(xvadddp, void, env, i32)
 DEF_HELPER_2(xvsubdp, void, env, i32)
diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index f4c1f42..950c02e 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -7364,6 +7364,7 @@ GEN_VSX_HELPER_2(xsmulsp, 0x00, 0x02, 0, PPC2_VSX207)
 GEN_VSX_HELPER_2(xsdivsp, 0x00, 0x03, 0, PPC2_VSX207)
 GEN_VSX_HELPER_2(xsresp, 0x14, 0x01, 0, PPC2_VSX207)
 GEN_VSX_HELPER_2(xssqrtsp, 0x16, 0x00, 0, PPC2_VSX207)
+GEN_VSX_HELPER_2(xsrsqrtesp, 0x14, 0x00, 0, PPC2_VSX207)
 
 GEN_VSX_HELPER_2(xvadddp, 0x00, 0x0C, 0, PPC2_VSX)
 GEN_VSX_HELPER_2(xvsubdp, 0x00, 0x0D, 0, PPC2_VSX)
@@ -10177,6 +10178,7 @@ GEN_XX3FORM(xsmulsp, 0x00, 0x02, PPC2_VSX207),
 GEN_XX3FORM(xsdivsp, 0x00, 0x03, PPC2_VSX207),
 GEN_XX2FORM(xsresp,  0x14, 0x01, PPC2_VSX207),
 GEN_XX2FORM(xssqrtsp,  0x16, 0x00, PPC2_VSX207),
+GEN_XX2FORM(xsrsqrtesp,  0x14, 0x00, PPC2_VSX207),
 
 GEN_XX3FORM(xvadddp, 0x00, 0x0C, PPC2_VSX),
 GEN_XX3FORM(xvsubdp, 0x00, 0x0D, PPC2_VSX),
-- 
1.7.1

[Qemu-devel] [V5 PATCH 09/14] target-ppc: VSX Stage 4: Add xsresp

[Tom Musta]

This patch adds the VSX Scalar Reciprocal Estimate Single Precision
(xsresp) instruction.

The existing VSX_RE macro is modified to support rounding of the
intermediate double precision result to single precision.

Signed-off-by: Tom Musta 
Reviewed-by: Richard Henderson 
---
V2: Updated conversion to single precision range.

 target-ppc/fpu_helper.c |   14 ++
 target-ppc/helper.h |1 +
 target-ppc/translate.c  |2 ++
 3 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/target-ppc/fpu_helper.c b/target-ppc/fpu_helper.c
index 49cf09a..ac52c23 100644
--- a/target-ppc/fpu_helper.c
+++ b/target-ppc/fpu_helper.c
@@ -1928,7 +1928,7 @@ VSX_DIV(xvdivsp, 4, float32, f32, 0, 0)
  *   fld   - vsr_t field (f32 or f64)
  *   sfprf - set FPRF
  */
-#define VSX_RE(op, nels, tp, fld, sfprf)  \
+#define VSX_RE(op, nels, tp, fld, sfprf, r2sp)\
 void helper_##op(CPUPPCState *env, uint32_t opcode)   \
 { \
 ppc_vsr_t xt, xb; \
@@ -1943,6 +1943,11 @@ void helper_##op(CPUPPCState *env, uint32_t opcode)  
 \
 fload_invalid_op_excp(env, POWERPC_EXCP_FP_VXSNAN, sfprf);\
 } \
 xt.fld[i] = tp##_div(tp##_one, xb.fld[i], &env->fp_status);   \
+  \
+if (r2sp) {   \
+xt.fld[i] = helper_frsp(env, xt.fld[i]);  \
+} \
+  \
 if (sfprf) {  \
 helper_compute_fprf(env, xt.fld[0], sfprf);   \
 } \
@@ -1952,9 +1957,10 @@ void helper_##op(CPUPPCState *env, uint32_t opcode)  
 \
 helper_float_check_status(env);   \
 }
 
-VSX_RE(xsredp, 1, float64, f64, 1)
-VSX_RE(xvredp, 2, float64, f64, 0)
-VSX_RE(xvresp, 4, float32, f32, 0)
+VSX_RE(xsredp, 1, float64, f64, 1, 0)
+VSX_RE(xsresp, 1, float64, f64, 1, 1)
+VSX_RE(xvredp, 2, float64, f64, 0, 0)
+VSX_RE(xvresp, 4, float32, f32, 0, 0)
 
 /* VSX_SQRT - VSX floating point square root
  *   op- instruction mnemonic
diff --git a/target-ppc/helper.h b/target-ppc/helper.h
index 308f97c..b1cf3c0 100644
--- a/target-ppc/helper.h
+++ b/target-ppc/helper.h
@@ -290,6 +290,7 @@ DEF_HELPER_2(xsaddsp, void, env, i32)
 DEF_HELPER_2(xssubsp, void, env, i32)
 DEF_HELPER_2(xsmulsp, void, env, i32)
 DEF_HELPER_2(xsdivsp, void, env, i32)
+DEF_HELPER_2(xsresp, void, env, i32)
 
 DEF_HELPER_2(xvadddp, void, env, i32)
 DEF_HELPER_2(xvsubdp, void, env, i32)
diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index eddb9d2..3108a29 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -7362,6 +7362,7 @@ GEN_VSX_HELPER_2(xsaddsp, 0x00, 0x00, 0, PPC2_VSX207)
 GEN_VSX_HELPER_2(xssubsp, 0x00, 0x01, 0, PPC2_VSX207)
 GEN_VSX_HELPER_2(xsmulsp, 0x00, 0x02, 0, PPC2_VSX207)
 GEN_VSX_HELPER_2(xsdivsp, 0x00, 0x03, 0, PPC2_VSX207)
+GEN_VSX_HELPER_2(xsresp, 0x14, 0x01, 0, PPC2_VSX207)
 
 GEN_VSX_HELPER_2(xvadddp, 0x00, 0x0C, 0, PPC2_VSX)
 GEN_VSX_HELPER_2(xvsubdp, 0x00, 0x0D, 0, PPC2_VSX)
@@ -10173,6 +10174,7 @@ GEN_XX3FORM(xsaddsp, 0x00, 0x00, PPC2_VSX207),
 GEN_XX3FORM(xssubsp, 0x00, 0x01, PPC2_VSX207),
 GEN_XX3FORM(xsmulsp, 0x00, 0x02, PPC2_VSX207),
 GEN_XX3FORM(xsdivsp, 0x00, 0x03, PPC2_VSX207),
+GEN_XX2FORM(xsresp,  0x14, 0x01, PPC2_VSX207),
 
 GEN_XX3FORM(xvadddp, 0x00, 0x0C, PPC2_VSX),
 GEN_XX3FORM(xvsubdp, 0x00, 0x0D, PPC2_VSX),
-- 
1.7.1

[Qemu-devel] [V5 PATCH 07/14] target-ppc: VSX Stage 4: Add xsmulsp

[Tom Musta]

This patch adds the VSX Scalar Multiply Single-Precision (xsmulsp)
instruction.

The existing VSX_MUL macro is modified to support rounding of the
intermediate result to single precision.

Signed-off-by: Tom Musta 
Reviewed-by: Richard Henderson 
---
V2: Updated conversion to single precision.

 target-ppc/fpu_helper.c |   13 +
 target-ppc/helper.h |1 +
 target-ppc/translate.c  |2 ++
 3 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/target-ppc/fpu_helper.c b/target-ppc/fpu_helper.c
index f047640..dc9849f 100644
--- a/target-ppc/fpu_helper.c
+++ b/target-ppc/fpu_helper.c
@@ -1822,7 +1822,7 @@ VSX_ADD_SUB(xvsubsp, sub, 4, float32, f32, 0, 0)
  *   fld   - vsr_t field (f32 or f64)
  *   sfprf - set FPRF
  */
-#define VSX_MUL(op, nels, tp, fld, sfprf)\
+#define VSX_MUL(op, nels, tp, fld, sfprf, r2sp)  \
 void helper_##op(CPUPPCState *env, uint32_t opcode)  \
 {\
 ppc_vsr_t xt, xa, xb;\
@@ -1849,6 +1849,10 @@ void helper_##op(CPUPPCState *env, uint32_t opcode)  
\
 }\
 }\
  \
+if (r2sp) {  \
+xt.fld[i] = helper_frsp(env, xt.fld[i]); \
+}\
+ \
 if (sfprf) { \
 helper_compute_fprf(env, xt.fld[i], sfprf);  \
 }\
@@ -1858,9 +1862,10 @@ void helper_##op(CPUPPCState *env, uint32_t opcode)  
\
 helper_float_check_status(env);  \
 }
 
-VSX_MUL(xsmuldp, 1, float64, f64, 1)
-VSX_MUL(xvmuldp, 2, float64, f64, 0)
-VSX_MUL(xvmulsp, 4, float32, f32, 0)
+VSX_MUL(xsmuldp, 1, float64, f64, 1, 0)
+VSX_MUL(xsmulsp, 1, float64, f64, 1, 1)
+VSX_MUL(xvmuldp, 2, float64, f64, 0, 0)
+VSX_MUL(xvmulsp, 4, float32, f32, 0, 0)
 
 /* VSX_DIV - VSX floating point divide
  *   op- instruction mnemonic
diff --git a/target-ppc/helper.h b/target-ppc/helper.h
index 696b9d3..0ccdc96 100644
--- a/target-ppc/helper.h
+++ b/target-ppc/helper.h
@@ -288,6 +288,7 @@ DEF_HELPER_2(xsrdpiz, void, env, i32)
 
 DEF_HELPER_2(xsaddsp, void, env, i32)
 DEF_HELPER_2(xssubsp, void, env, i32)
+DEF_HELPER_2(xsmulsp, void, env, i32)
 
 DEF_HELPER_2(xvadddp, void, env, i32)
 DEF_HELPER_2(xvsubdp, void, env, i32)
diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index c50d800..3a6a94b 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -7360,6 +7360,7 @@ GEN_VSX_HELPER_2(xsrdpiz, 0x12, 0x05, 0, PPC2_VSX)
 
 GEN_VSX_HELPER_2(xsaddsp, 0x00, 0x00, 0, PPC2_VSX207)
 GEN_VSX_HELPER_2(xssubsp, 0x00, 0x01, 0, PPC2_VSX207)
+GEN_VSX_HELPER_2(xsmulsp, 0x00, 0x02, 0, PPC2_VSX207)
 
 GEN_VSX_HELPER_2(xvadddp, 0x00, 0x0C, 0, PPC2_VSX)
 GEN_VSX_HELPER_2(xvsubdp, 0x00, 0x0D, 0, PPC2_VSX)
@@ -10169,6 +10170,7 @@ GEN_XX2FORM(xsrdpiz, 0x12, 0x05, PPC2_VSX),
 
 GEN_XX3FORM(xsaddsp, 0x00, 0x00, PPC2_VSX207),
 GEN_XX3FORM(xssubsp, 0x00, 0x01, PPC2_VSX207),
+GEN_XX3FORM(xsmulsp, 0x00, 0x02, PPC2_VSX207),
 
 GEN_XX3FORM(xvadddp, 0x00, 0x0C, PPC2_VSX),
 GEN_XX3FORM(xvsubdp, 0x00, 0x0D, PPC2_VSX),
-- 
1.7.1

[Qemu-devel] [V5 PATCH 12/14] target-ppc: VSX Stage 4: Add Scalar SP Fused Multiply-Adds

[Tom Musta]

This patch adds the Single Precision VSX Scalar Fused Multiply-Add
instructions: xsmaddasp, xsmaddmsp, xssubasp, xssubmsp, xsnmaddasp,
xsnmaddmsp, xsnmsubasp, xsnmsubmsp.

The existing VSX_MADD() macro is modified to support rounding of the
intermediate double precision result to single precision.

Signed-off-by: Tom Musta 
Reviewed-by: Richard Henderson 
---
V2: Re-implemented per feedback from Richard Henderson.  In order to
avoid double rounding and incorrect results, the operands must be
converted to true single precision values and use the single precision
fused multiply/add routine.

V3: Re-implemented per feedback from Richard Henderson (I did not
fully understand his comment when I implemented V2).

V4: Changed to use helper_frsp (inadvertently re-injected when I used
an earlier patch).  Thanks to Richard Henderson for catching this.

 target-ppc/fpu_helper.c |   82 ++
 target-ppc/helper.h |8 
 target-ppc/translate.c  |   16 +
 3 files changed, 77 insertions(+), 29 deletions(-)

diff --git a/target-ppc/fpu_helper.c b/target-ppc/fpu_helper.c
index 33da462..7e5003a 100644
--- a/target-ppc/fpu_helper.c
+++ b/target-ppc/fpu_helper.c
@@ -2192,7 +2192,7 @@ VSX_TSQRT(xvtsqrtsp, 4, float32, f32, -126, 23)
  *   afrm  - A form (1=A, 0=M)
  *   sfprf - set FPRF
  */
-#define VSX_MADD(op, nels, tp, fld, maddflgs, afrm, sfprf)\
+#define VSX_MADD(op, nels, tp, fld, maddflgs, afrm, sfprf, r2sp)  \
 void helper_##op(CPUPPCState *env, uint32_t opcode)   \
 { \
 ppc_vsr_t xt_in, xa, xb, xt_out;  \
@@ -2218,8 +2218,18 @@ void helper_##op(CPUPPCState *env, uint32_t opcode)  
 \
 for (i = 0; i < nels; i++) {  \
 float_status tstat = env->fp_status;  \
 set_float_exception_flags(0, &tstat); \
-xt_out.fld[i] = tp##_muladd(xa.fld[i], b->fld[i], c->fld[i],  \
- maddflgs, &tstat);   \
+if (r2sp && (tstat.float_rounding_mode == float_round_nearest_even)) {\
+/* Avoid double rounding errors by rounding the intermediate */   \
+/* result to odd.*/   \
+set_float_rounding_mode(float_round_to_zero, &tstat); \
+xt_out.fld[i] = tp##_muladd(xa.fld[i], b->fld[i], c->fld[i],  \
+   maddflgs, &tstat); \
+xt_out.fld[i] |= (get_float_exception_flags(&tstat) & \
+  float_flag_inexact) != 0;   \
+} else {  \
+xt_out.fld[i] = tp##_muladd(xa.fld[i], b->fld[i], c->fld[i],  \
+maddflgs, &tstat);\
+} \
 env->fp_status.float_exception_flags |= tstat.float_exception_flags;  \
   \
 if (unlikely(tstat.float_exception_flags & float_flag_invalid)) { \
@@ -2242,6 +2252,11 @@ void helper_##op(CPUPPCState *env, uint32_t opcode)  
 \
 fload_invalid_op_excp(env, POWERPC_EXCP_FP_VXISI, sfprf); \
 } \
 } \
+  \
+if (r2sp) {   \
+xt_out.fld[i] = helper_frsp(env, xt_out.fld[i]);  \
+} \
+  \
 if (sfprf) {  \
 helper_compute_fprf(env, xt_out.fld[i], sfprf);   \
 } \
@@ -2255,32 +2270,41 @@ void helper_##op(CPUPPCState *env, uint32_t opcode) 
  \
 #define NMADD_FLGS float_muladd_negate_result
 #define NMSUB_FLGS (float_muladd_negate_c | float_muladd_negate_result)
 
-VSX_MADD(xsmaddadp, 1, float64, f64, MADD_FLGS, 1, 1)
-VSX_MADD(xsmaddmdp, 1, float64, f64, MADD_FLGS, 0, 1)
-VSX_MADD(xsmsubadp, 1, float64, f64, MSUB_FLGS, 1, 1)
-VSX_MADD(xsmsubmdp, 1, float64, f64, MSUB_FLGS, 0, 1)
-VSX_MADD(xsnmaddadp, 1, float64, f64, NMADD_FLGS, 1, 1)
-VSX_MADD(xsnmaddmdp, 

[Qemu-devel] [V5 PATCH 08/14] target-ppc: VSX Stage 4: Add xsdivsp

[Tom Musta]

This patch adds the VSX Scalar Divide Single Precision (xsdivsp)
instruction.

The existing VSX_DIV macro is modified to support rounding of the
intermediate double precision result to single precision.

Signed-off-by: Tom Musta 
Reviewed-by: Richard Henderson 
---
V2: Updated conversion to single precision.

 target-ppc/fpu_helper.c |   13 +
 target-ppc/helper.h |1 +
 target-ppc/translate.c  |2 ++
 3 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/target-ppc/fpu_helper.c b/target-ppc/fpu_helper.c
index dc9849f..49cf09a 100644
--- a/target-ppc/fpu_helper.c
+++ b/target-ppc/fpu_helper.c
@@ -1874,7 +1874,7 @@ VSX_MUL(xvmulsp, 4, float32, f32, 0, 0)
  *   fld   - vsr_t field (f32 or f64)
  *   sfprf - set FPRF
  */
-#define VSX_DIV(op, nels, tp, fld, sfprf) \
+#define VSX_DIV(op, nels, tp, fld, sfprf, r2sp)   \
 void helper_##op(CPUPPCState *env, uint32_t opcode)   \
 { \
 ppc_vsr_t xt, xa, xb; \
@@ -1903,6 +1903,10 @@ void helper_##op(CPUPPCState *env, uint32_t opcode)  
 \
 } \
 } \
   \
+if (r2sp) {   \
+xt.fld[i] = helper_frsp(env, xt.fld[i]);  \
+} \
+  \
 if (sfprf) {  \
 helper_compute_fprf(env, xt.fld[i], sfprf);   \
 } \
@@ -1912,9 +1916,10 @@ void helper_##op(CPUPPCState *env, uint32_t opcode)  
 \
 helper_float_check_status(env);   \
 }
 
-VSX_DIV(xsdivdp, 1, float64, f64, 1)
-VSX_DIV(xvdivdp, 2, float64, f64, 0)
-VSX_DIV(xvdivsp, 4, float32, f32, 0)
+VSX_DIV(xsdivdp, 1, float64, f64, 1, 0)
+VSX_DIV(xsdivsp, 1, float64, f64, 1, 1)
+VSX_DIV(xvdivdp, 2, float64, f64, 0, 0)
+VSX_DIV(xvdivsp, 4, float32, f32, 0, 0)
 
 /* VSX_RE  - VSX floating point reciprocal estimate
  *   op- instruction mnemonic
diff --git a/target-ppc/helper.h b/target-ppc/helper.h
index 0ccdc96..308f97c 100644
--- a/target-ppc/helper.h
+++ b/target-ppc/helper.h
@@ -289,6 +289,7 @@ DEF_HELPER_2(xsrdpiz, void, env, i32)
 DEF_HELPER_2(xsaddsp, void, env, i32)
 DEF_HELPER_2(xssubsp, void, env, i32)
 DEF_HELPER_2(xsmulsp, void, env, i32)
+DEF_HELPER_2(xsdivsp, void, env, i32)
 
 DEF_HELPER_2(xvadddp, void, env, i32)
 DEF_HELPER_2(xvsubdp, void, env, i32)
diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 3a6a94b..eddb9d2 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -7361,6 +7361,7 @@ GEN_VSX_HELPER_2(xsrdpiz, 0x12, 0x05, 0, PPC2_VSX)
 GEN_VSX_HELPER_2(xsaddsp, 0x00, 0x00, 0, PPC2_VSX207)
 GEN_VSX_HELPER_2(xssubsp, 0x00, 0x01, 0, PPC2_VSX207)
 GEN_VSX_HELPER_2(xsmulsp, 0x00, 0x02, 0, PPC2_VSX207)
+GEN_VSX_HELPER_2(xsdivsp, 0x00, 0x03, 0, PPC2_VSX207)
 
 GEN_VSX_HELPER_2(xvadddp, 0x00, 0x0C, 0, PPC2_VSX)
 GEN_VSX_HELPER_2(xvsubdp, 0x00, 0x0D, 0, PPC2_VSX)
@@ -10171,6 +10172,7 @@ GEN_XX2FORM(xsrdpiz, 0x12, 0x05, PPC2_VSX),
 GEN_XX3FORM(xsaddsp, 0x00, 0x00, PPC2_VSX207),
 GEN_XX3FORM(xssubsp, 0x00, 0x01, PPC2_VSX207),
 GEN_XX3FORM(xsmulsp, 0x00, 0x02, PPC2_VSX207),
+GEN_XX3FORM(xsdivsp, 0x00, 0x03, PPC2_VSX207),
 
 GEN_XX3FORM(xvadddp, 0x00, 0x0C, PPC2_VSX),
 GEN_XX3FORM(xvsubdp, 0x00, 0x0D, PPC2_VSX),
-- 
1.7.1

[Qemu-devel] [V5 PATCH 13/14] target-ppc: VSX Stage 4: Add xscvsxdsp and xscvuxdsp

[Tom Musta]

This patch adds the VSX Scalar Convert Unsigned Integer Doubleword
to Floating Point Format and Round to Single Precision (xscvuxdsp)
and VSX Scalar Convert Signed Integer Douglbeword to Floating Point
Format and Round to Single Precision (xscvsxdsp) instructions.

The existing integer to floating point conversion macro (VSX_CVT_INT_TO_FP)
is modified to support the rounding of the intermediate floating point
result to single precision.

Signed-off-by: Tom Musta 
Reviewed-by: Richard Henderson 
---
V2: updated conversion to single precision range.

 target-ppc/fpu_helper.c |   27 ---
 target-ppc/helper.h |2 ++
 target-ppc/translate.c  |4 
 3 files changed, 22 insertions(+), 11 deletions(-)

diff --git a/target-ppc/fpu_helper.c b/target-ppc/fpu_helper.c
index 7e5003a..1dfb3c0 100644
--- a/target-ppc/fpu_helper.c
+++ b/target-ppc/fpu_helper.c
@@ -2558,7 +2558,7 @@ VSX_CVT_FP_TO_INT(xvcvspuxws, 4, float32, uint32, f32[j], 
u32[i], i, 0)
  *   jdef  - definition of the j index (i or 2*i)
  *   sfprf - set FPRF
  */
-#define VSX_CVT_INT_TO_FP(op, nels, stp, ttp, sfld, tfld, jdef, sfprf)  \
+#define VSX_CVT_INT_TO_FP(op, nels, stp, ttp, sfld, tfld, jdef, sfprf, r2sp) \
 void helper_##op(CPUPPCState *env, uint32_t opcode) \
 {   \
 ppc_vsr_t xt, xb;   \
@@ -2570,6 +2570,9 @@ void helper_##op(CPUPPCState *env, uint32_t opcode)   
  \
 for (i = 0; i < nels; i++) {\
 int j = jdef;   \
 xt.tfld = stp##_to_##ttp(xb.sfld, &env->fp_status); \
+if (r2sp) { \
+xt.tfld = helper_frsp(env, xt.tfld);\
+}   \
 if (sfprf) {\
 helper_compute_fprf(env, xt.tfld, sfprf);   \
 }   \
@@ -2579,20 +2582,22 @@ void helper_##op(CPUPPCState *env, uint32_t opcode) 
\
 helper_float_check_status(env); \
 }
 
-VSX_CVT_INT_TO_FP(xscvsxddp, 1, int64, float64, u64[j], f64[i], i, 1)
-VSX_CVT_INT_TO_FP(xscvuxddp, 1, uint64, float64, u64[j], f64[i], i, 1)
-VSX_CVT_INT_TO_FP(xvcvsxddp, 2, int64, float64, u64[j], f64[i], i, 0)
-VSX_CVT_INT_TO_FP(xvcvuxddp, 2, uint64, float64, u64[j], f64[i], i, 0)
+VSX_CVT_INT_TO_FP(xscvsxddp, 1, int64, float64, u64[j], f64[i], i, 1, 0)
+VSX_CVT_INT_TO_FP(xscvuxddp, 1, uint64, float64, u64[j], f64[i], i, 1, 0)
+VSX_CVT_INT_TO_FP(xscvsxdsp, 1, int64, float64, u64[j], f64[i], i, 1, 1)
+VSX_CVT_INT_TO_FP(xscvuxdsp, 1, uint64, float64, u64[j], f64[i], i, 1, 1)
+VSX_CVT_INT_TO_FP(xvcvsxddp, 2, int64, float64, u64[j], f64[i], i, 0, 0)
+VSX_CVT_INT_TO_FP(xvcvuxddp, 2, uint64, float64, u64[j], f64[i], i, 0, 0)
 VSX_CVT_INT_TO_FP(xvcvsxwdp, 2, int32, float64, u32[j], f64[i], \
-  2*i + JOFFSET, 0)
+  2*i + JOFFSET, 0, 0)
 VSX_CVT_INT_TO_FP(xvcvuxwdp, 2, uint64, float64, u32[j], f64[i], \
-  2*i + JOFFSET, 0)
+  2*i + JOFFSET, 0, 0)
 VSX_CVT_INT_TO_FP(xvcvsxdsp, 2, int64, float32, u64[i], f32[j], \
-  2*i + JOFFSET, 0)
+  2*i + JOFFSET, 0, 0)
 VSX_CVT_INT_TO_FP(xvcvuxdsp, 2, uint64, float32, u64[i], f32[j], \
-  2*i + JOFFSET, 0)
-VSX_CVT_INT_TO_FP(xvcvsxwsp, 4, int32, float32, u32[j], f32[i], i, 0)
-VSX_CVT_INT_TO_FP(xvcvuxwsp, 4, uint32, float32, u32[j], f32[i], i, 0)
+  2*i + JOFFSET, 0, 0)
+VSX_CVT_INT_TO_FP(xvcvsxwsp, 4, int32, float32, u32[j], f32[i], i, 0, 0)
+VSX_CVT_INT_TO_FP(xvcvuxwsp, 4, uint32, float32, u32[j], f32[i], i, 0, 0)
 
 /* For "use current rounding mode", define a value that will not be one of
  * the existing rounding model enums.
diff --git a/target-ppc/helper.h b/target-ppc/helper.h
index 655b670..6250eba 100644
--- a/target-ppc/helper.h
+++ b/target-ppc/helper.h
@@ -279,6 +279,8 @@ DEF_HELPER_2(xscvdpsxws, void, env, i32)
 DEF_HELPER_2(xscvdpuxds, void, env, i32)
 DEF_HELPER_2(xscvdpuxws, void, env, i32)
 DEF_HELPER_2(xscvsxddp, void, env, i32)
+DEF_HELPER_2(xscvuxdsp, void, env, i32)
+DEF_HELPER_2(xscvsxdsp, void, env, i32)
 DEF_HELPER_2(xscvuxddp, void, env, i32)
 DEF_HELPER_2(xsrdpi, void, env, i32)
 DEF_HELPER_2(xsrdpic, void, env, i32)
diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 5b68c7e..7659085 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -7373,6 +7373,8 @@ GEN_VSX_HELPER_2(xsnmaddasp, 0x04, 0x10, 0, PPC2_VSX207)
 GEN_VSX_HELPER_2(xsnmaddmsp, 0x04, 0x11, 0, PPC2_VSX207)
 GEN_VSX_HELPER_2(xsnmsubasp, 0x04, 0x12, 0, PPC2_VSX207)
 GEN_VSX_HELPE

[Qemu-devel] [V5 PATCH 14/14] target-ppc: VSX Stage 4: Add xxleqv, xxlnand and xxlorc

[Tom Musta]

This patchs adds the VSX Logical instructions that are new with
ISA V2.07:

  - VSX Logical Equivalence (xxleqv)
  - VSX Logical NAND (xxlnand)
  - VSX Logical ORC (xxlorc)

Signed-off-by: Tom Musta 
Reviewed-by: Richard Henderson 
---
V5: Changes to address tcg-debug compilation errors.

 target-ppc/translate.c |6 ++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 7659085..e2dd272 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -7468,6 +7468,9 @@ VSX_LOGICAL(xxlandc, tcg_gen_andc_i64)
 VSX_LOGICAL(xxlor, tcg_gen_or_i64)
 VSX_LOGICAL(xxlxor, tcg_gen_xor_i64)
 VSX_LOGICAL(xxlnor, tcg_gen_nor_i64)
+VSX_LOGICAL(xxleqv, tcg_gen_eqv_i64)
+VSX_LOGICAL(xxlnand, tcg_gen_nand_i64)
+VSX_LOGICAL(xxlorc, tcg_gen_orc_i64)
 
 #define VSX_XXMRG(name, high)   \
 static void glue(gen_, name)(DisasContext * ctx)\
@@ -10283,6 +10286,9 @@ VSX_LOGICAL(xxlandc, 0x8, 0x11, PPC2_VSX),
 VSX_LOGICAL(xxlor, 0x8, 0x12, PPC2_VSX),
 VSX_LOGICAL(xxlxor, 0x8, 0x13, PPC2_VSX),
 VSX_LOGICAL(xxlnor, 0x8, 0x14, PPC2_VSX),
+VSX_LOGICAL(xxleqv, 0x8, 0x17, PPC2_VSX207),
+VSX_LOGICAL(xxlnand, 0x8, 0x16, PPC2_VSX207),
+VSX_LOGICAL(xxlorc, 0x8, 0x15, PPC2_VSX207),
 GEN_XX3FORM(xxmrghw, 0x08, 0x02, PPC2_VSX),
 GEN_XX3FORM(xxmrglw, 0x08, 0x06, PPC2_VSX),
 GEN_XX2FORM(xxspltw, 0x08, 0x0A, PPC2_VSX),
-- 
1.7.1

Re: [Qemu-devel] [Qemu-ppc] [V3 PATCH 03/14] target-ppc: Add ISA2.06 divdeu[o] Instructions

[Tom Musta]

On 12/27/2013 6:30 PM, Scott Wood wrote:
> These instructions are phased-in on embedded, and unlike bpermd are not
> present on e5500/e6500 which are 64-bit ISA 2.06 implementations.
> Wasn't the conclusion in a previous thread to use separate flags for
> these instruction groups?
> 
> -Scott

Scott:

Yes ... but those comments were made *after* this revision was published
(http://lists.nongnu.org/archive/html/qemu-devel/2013-12/msg03681.html).

I have yet to produce V4 of this series which will include the new and
separate flags.

Re: [Qemu-devel] [Qemu-ppc] [V3 PATCH 03/14] target-ppc: Add ISA2.06 divdeu[o] Instructions

[Scott Wood]

On Fri, 2014-01-03 at 13:24 -0600, Tom Musta wrote:
> On 12/27/2013 6:30 PM, Scott Wood wrote:
> > These instructions are phased-in on embedded, and unlike bpermd are not
> > present on e5500/e6500 which are 64-bit ISA 2.06 implementations.
> > Wasn't the conclusion in a previous thread to use separate flags for
> > these instruction groups?
> > 
> > -Scott
> 
> Scott:
> 
> Yes ... but those comments were made *after* this revision was published
> (http://lists.nongnu.org/archive/html/qemu-devel/2013-12/msg03681.html).
> 
> I have yet to produce V4 of this series which will include the new and
> separate flags.

Oops... for some reason I thought the patchset was newer.  Sorry about
that.

-Scott

[Qemu-devel] [PATCH] seccomp: add mkdir() and fchmod() to the whitelist

[Paul Moore]

The PulseAudio library attempts to do a mkdir(2) and fchmod(2) on
"/run/user//pulse" which is currently blocked by the syscall
filter; this patch adds the two missing syscalls to the whitelist.
You can reproduce this problem with the following command:

 # qemu -monitor stdio -device intel-hda -device hda-duplex

If watched under strace the following syscalls are shown:

 mkdir("/run/user/0/pulse", 0700)
 fchmod(11, 0700) [NOTE: 11 is the fd for /run/user/0/pulse]

Reported-by: xu...@redhat.com
Signed-off-by: Paul Moore 
---
 qemu-seccomp.c |4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/qemu-seccomp.c b/qemu-seccomp.c
index cf07869..bb19306 100644
--- a/qemu-seccomp.c
+++ b/qemu-seccomp.c
@@ -220,7 +220,9 @@ static const struct QemuSeccompSyscall seccomp_whitelist[] 
= {
 { SCMP_SYS(io_cancel), 241 },
 { SCMP_SYS(io_setup), 241 },
 { SCMP_SYS(io_destroy), 241 },
-{ SCMP_SYS(arch_prctl), 240 }
+{ SCMP_SYS(arch_prctl), 240 },
+{ SCMP_SYS(mkdir), 240 },
+{ SCMP_SYS(fchmod), 240 }
 };
 
 int seccomp_start(void)

Re: [Qemu-devel] [PATCH] seccomp: add mkdir() and fchmod() to the whitelist

[Paolo Bonzini]

Il 03/01/2014 20:58, Paul Moore ha scritto:
> The PulseAudio library attempts to do a mkdir(2) and fchmod(2) on
> "/run/user//pulse" which is currently blocked by the syscall
> filter; this patch adds the two missing syscalls to the whitelist.
> You can reproduce this problem with the following command:
> 
>  # qemu -monitor stdio -device intel-hda -device hda-duplex
> 
> If watched under strace the following syscalls are shown:
> 
>  mkdir("/run/user/0/pulse", 0700)
>  fchmod(11, 0700) [NOTE: 11 is the fd for /run/user/0/pulse]

Can fchmod be exploited to violate the sandbox (e.g. to let data escape
from a VM that ought not to have any way to communicate with the outside
world)?

Paolo

Re: [Qemu-devel] [PATCH] seccomp: add mkdir() and fchmod() to the whitelist

[Paul Moore]

On Friday, January 03, 2014 09:24:57 PM Paolo Bonzini wrote:
> Il 03/01/2014 20:58, Paul Moore ha scritto:
> > The PulseAudio library attempts to do a mkdir(2) and fchmod(2) on
> > "/run/user//pulse" which is currently blocked by the syscall
> > filter; this patch adds the two missing syscalls to the whitelist.
> > 
> > You can reproduce this problem with the following command:
> >  # qemu -monitor stdio -device intel-hda -device hda-duplex
> > 
> > If watched under strace the following syscalls are shown:
> >  mkdir("/run/user/0/pulse", 0700)
> >  fchmod(11, 0700) [NOTE: 11 is the fd for /run/user/0/pulse]
> 
> Can fchmod be exploited to violate the sandbox (e.g. to let data escape
> from a VM that ought not to have any way to communicate with the outside
> world)?

Technically, there is the potential for any syscall to be exploited in such a 
way that a malicious guest could gain greater access than desired and do 
something evil with that access.  After all, that was the motivation behind 
seccomp: disable unused syscalls to reduce the chance of an attacker 
exploiting a syscall bug.

The important thing to remember here is that the seccomp code in QEMU is not 
enabling syscalls, it is disabling them.  In other words, a QEMU instance with 
the seccomp functionality enabled, e.g. '-sandbox on', only reduces the number 
of syscalls available to the QEMU process, it never increases or adds 
vulnerable syscalls to the QEMU process.

Granted, yes, there are syscalls in the current whitelist that I wish we could 
disable, but we are still trying to arrive a whitelist that is all 
encompassing (or close to it) with respect to QEMU functionality.  Once we 
have that list in hand (each fix like the one I posted gets us closer) we can 
start looking at selectively shrinking the whitelist*.

* We've talked about this on-list previously and there are several approaches 
here, some include conditionally adding/removing syscalls based on the QEMU 
functionality requested, e.g. command line, different sandbox "profiles", e.g. 
standalone vs libvirt, and staged seccomp filters, e.g. a whitelist followed 
by progressively tighter blacklists.

-- 
paul moore
security and virtualization @ redhat

Re: [Qemu-devel] [PATCH 1/2] hw/net: add support for Allwinner EMAC Fast Ethernet controller

[Peter Crosthwaite]

On Thu, Jan 2, 2014 at 7:18 PM, Beniamino Galvani  wrote:
> This patch adds support for the Fast Ethernet MAC found on Allwinner
> SoCs, together with a basic emulation of Realtek RTL8201CP PHY.
>
> Since there is no public documentation of the Allwinner controller, the
> implementation is based on Linux kernel driver.
>
> Signed-off-by: Beniamino Galvani 
> ---
>  default-configs/arm-softmmu.mak |1 +
>  hw/net/Makefile.objs|1 +
>  hw/net/allwinner_emac.c |  466 
> +++
>  include/hw/net/allwinner_emac.h |  204 +
>  4 files changed, 672 insertions(+)
>  create mode 100644 hw/net/allwinner_emac.c
>  create mode 100644 include/hw/net/allwinner_emac.h
>
> diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
> index ce1d620..f3513fa 100644
> --- a/default-configs/arm-softmmu.mak
> +++ b/default-configs/arm-softmmu.mak
> @@ -27,6 +27,7 @@ CONFIG_SSI_SD=y
>  CONFIG_SSI_M25P80=y
>  CONFIG_LAN9118=y
>  CONFIG_SMC91C111=y
> +CONFIG_ALLWINNER_EMAC=y
>  CONFIG_DS1338=y
>  CONFIG_PFLASH_CFI01=y
>  CONFIG_PFLASH_CFI02=y
> diff --git a/hw/net/Makefile.objs b/hw/net/Makefile.objs
> index 951cca3..75e80c2 100644
> --- a/hw/net/Makefile.objs
> +++ b/hw/net/Makefile.objs
> @@ -18,6 +18,7 @@ common-obj-$(CONFIG_OPENCORES_ETH) += opencores_eth.o
>  common-obj-$(CONFIG_XGMAC) += xgmac.o
>  common-obj-$(CONFIG_MIPSNET) += mipsnet.o
>  common-obj-$(CONFIG_XILINX_AXI) += xilinx_axienet.o
> +common-obj-$(CONFIG_ALLWINNER_EMAC) += allwinner_emac.o
>
>  common-obj-$(CONFIG_CADENCE) += cadence_gem.o
>  common-obj-$(CONFIG_STELLARIS_ENET) += stellaris_enet.o
> diff --git a/hw/net/allwinner_emac.c b/hw/net/allwinner_emac.c
> new file mode 100644
> index 000..9791be4
> --- /dev/null
> +++ b/hw/net/allwinner_emac.c
> @@ -0,0 +1,466 @@
> +/*
> + * Emulation of Allwinner EMAC Fast Ethernet controller and
> + * Realtek RTL8201CP PHY
> + *
> + * Copyright (C) 2013 Beniamino Galvani 
> + *
> + * 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.
> + *
> + */
> +#include "hw/sysbus.h"
> +#include "net/net.h"
> +#include "hw/net/allwinner_emac.h"
> +#include 
> +
> +#undef AW_EMAC_DEBUG
> +
> +#ifdef AW_EMAC_DEBUG
> +#define debug(...)  \
> +do {\
> +fprintf(stderr,  "allwinner_emac : %s: ", __func__);\
> +fprintf(stderr, ## __VA_ARGS__);\
> +} while (0)
> +#else
> +#define debug(...) do {} while (0)
> +#endif
> +
> +static void mii_set_link(AwEmacMii *mii, bool link_ok)
> +{
> +if (link_ok) {
> +mii->bmsr |= MII_BMSR_LINK_ST;
> +mii->anlpar |= MII_ANAR_TXFD | MII_ANAR_TX | MII_ANAR_10FD |
> +   MII_ANAR_10 | MII_ANAR_CSMACD;
> +} else {
> +mii->bmsr &= ~MII_BMSR_LINK_ST;
> +mii->anlpar = MII_ANAR_TX;
> +}
> +mii->link_ok = link_ok;
> +}
> +
> +static void mii_reset(AwEmacMii *mii)
> +{
> +mii->bmcr = MII_BMCR_FD | MII_BMCR_AUTOEN | MII_BMCR_SPEED;
> +mii->bmsr = MII_BMSR_100TX_FD | MII_BMSR_100TX_HD | MII_BMSR_10T_FD |
> +MII_BMSR_10T_HD | MII_BMSR_MFPS | MII_BMSR_AUTONEG;
> +mii->anar = MII_ANAR_TXFD | MII_ANAR_TX | MII_ANAR_10FD | MII_ANAR_10 |
> +MII_ANAR_CSMACD;
> +mii->anlpar = MII_ANAR_TX;
> +mii_set_link(mii, mii->link_ok);
> +}
> +
> +static uint16_t mii_read(AwEmacMii *mii, uint8_t phy_addr, uint8_t reg)
> +{
> +uint16_t ret = 0x;
> +
> +if (phy_addr == BOARD_PHY_ADDRESS) {
> +switch (reg) {
> +case MII_BMCR:
> +ret = mii->bmcr;
> +break;
> +case MII_BMSR:
> +ret = mii->bmsr;
> +break;
> +case MII_PHYID1:
> +ret = RTL8201CP_PHYID1;
> +break;
> +case MII_PHYID2:
> +ret = RTL8201CP_PHYID2;
> +break;
> +case MII_ANAR:
> +ret = mii->anar;
> +break;
> +case MII_ANLPAR:
> +ret = mii->anlpar;
> +break;
> +default:
> +debug("unknown mii register %x\n", reg);
> +ret = 0;
> +}
> +}
> +return ret;
> +}
> +
> +static void mii_write(AwEmacMii *mii, uint8_t phy_addr, uint8_t reg,
> +  uint16_t value)
> +{
> +if (phy_addr == BOARD_PHY_ADDRESS) {
> +switch (reg) {
> +case MII_BMCR:
> +if (value & MII_BMCR_RESE

[Qemu-devel] [PATCH target-arm v2 1/1] arm/xilinx_zynq: Always instantiate the GEMs

[Peter Crosthwaite]

Don't conditionalise GEM instantiation on networking attachments. The
device should always be present even if not attached to a network.

This allows for probing of the device by expectant guests (such as
OS's).  This is needed because sysbus (or AXI in Xilinx's real hw case)
is not self identifying so the guest has no dynamic way of detecting
device absence.

Also allows for testing of the GEM in loopback mode with -net none.

Signed-off-by: Peter Crosthwaite 
---
changed since v1 (PMM review):
use nd->used rather than nb_nics checks.
Ditch the two iteration loop completely.

 hw/arm/xilinx_zynq.c | 17 ++---
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
index 17251c7..98e0958 100644
--- a/hw/arm/xilinx_zynq.c
+++ b/hw/arm/xilinx_zynq.c
@@ -49,9 +49,11 @@ static void gem_init(NICInfo *nd, uint32_t base, qemu_irq 
irq)
 DeviceState *dev;
 SysBusDevice *s;
 
-qemu_check_nic_model(nd, "cadence_gem");
 dev = qdev_create(NULL, "cadence_gem");
-qdev_set_nic_properties(dev, nd);
+if (nd->used) {
+qemu_check_nic_model(nd, "cadence_gem");
+qdev_set_nic_properties(dev, nd);
+}
 qdev_init_nofail(dev);
 s = SYS_BUS_DEVICE(dev);
 sysbus_mmio_map(s, 0, base);
@@ -113,7 +115,6 @@ static void zynq_init(QEMUMachineInitArgs *args)
 DeviceState *dev;
 SysBusDevice *busdev;
 qemu_irq pic[64];
-NICInfo *nd;
 Error *err = NULL;
 int n;
 
@@ -190,14 +191,8 @@ static void zynq_init(QEMUMachineInitArgs *args)
 sysbus_create_varargs("cadence_ttc", 0xF8002000,
 pic[69-IRQ_OFFSET], pic[70-IRQ_OFFSET], pic[71-IRQ_OFFSET], NULL);
 
-for (n = 0; n < nb_nics; n++) {
-nd = &nd_table[n];
-if (n == 0) {
-gem_init(nd, 0xE000B000, pic[54-IRQ_OFFSET]);
-} else if (n == 1) {
-gem_init(nd, 0xE000C000, pic[77-IRQ_OFFSET]);
-}
-}
+gem_init(&nd_table[0], 0xE000B000, pic[54-IRQ_OFFSET]);
+gem_init(&nd_table[1], 0xE000C000, pic[77-IRQ_OFFSET]);
 
 dev = qdev_create(NULL, "generic-sdhci");
 qdev_init_nofail(dev);
-- 
1.8.5.2

Re: [Qemu-devel] [PATCH target-arm v1 1/1] arm/xilinx_zynq: Always instantiate the GEMs

[Peter Crosthwaite]

On Sat, Jan 4, 2014 at 3:24 AM, Peter Maydell  wrote:
> On 3 January 2014 16:56, Peter Crosthwaite  
> wrote:
>> On Sat, Jan 4, 2014 at 2:22 AM, Peter Maydell  
>> wrote:
>>> On 3 January 2014 16:15, Peter Crosthwaite  
>>> wrote:
 One subtle question before respinning - should the
 qdev_set_nic_properties actually be conditional? Setting it to an
 unused NIC should be harmless, so perhaps it should be outside the
 if(nd->used). Other boards/MAC combos that dont do the
 qdev_check_nic_model can then just dispose of the if (nd->used) check
 altogether.
>>>
>>> Well qdev_set_nic_properties as it stands today will unconditionally
>>> assume you've passed it a valid NICInfo, so we have to a void calling
>>> it if nd->used isn't set.
>>
>> So I guess I'm thinking that the unusued nics should be a "valid"
>> NICInfo - just one that doesn't connect to anything. Currently,
>> qdev_set_nic_properties() sets three props - "mac", "netdev" and
>> "vectors". "mac" looks dubious to me, I'm not sure why the net layer
>> is telling devices what their MAC addresses are - it should be the
>> other way round.
>
> The device doesn't (in the old -net model) know what its MAC address
> is, because that's something you specify in the -net command line option.
> So this is the passing on of that info to the device.
>
>> Setting of "netdev" is already protected against NULL
>> setting so that one is handled. "vectors" looks like it has a sanity
>> check guard on its setting as well via the DEV_NVECTORS_UNSPECIFIED
>> check. Although that value may rely on some net.c code paths for
>> non-NULL NICs.
>
> I think it would probably be better to look into properly cleaning up
> the legacy -net nic support and moving embedded devices over to
> work with some variant of the new command line model, rather
> than tweaking the legacy stuff. nd_table eventually should ideally
> just go away I think.
>

Ok, you've talked me out of it. Respun with the if.

Regards,
Peter

> thanks
> -- PMM
>

Re: [Qemu-devel] [PATCH v2 15/25] target-arm: A64: Implement minimal set of EL0-visible sysregs

[Peter Crosthwaite]

On Mon, Dec 23, 2013 at 8:49 AM, Peter Maydell  wrote:
> Implement an initial minimal set of EL0-visible system registers:
>  * NZCV
>  * FPCR
>  * FPSR
>  * CTR_EL0
>  * DCZID_EL0
>
> Signed-off-by: Peter Maydell 
> Reviewed-by: Richard Henderson 
> ---
>  target-arm/cpu.h   |  3 ++-
>  target-arm/helper.c| 61 
> ++
>  target-arm/translate-a64.c | 52 +++
>  3 files changed, 115 insertions(+), 1 deletion(-)
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 66490ce..1e0800e 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -660,7 +660,8 @@ static inline uint64_t cpreg_to_kvm_id(uint32_t cpregid)
>  #define ARM_CP_IO 64
>  #define ARM_CP_NOP (ARM_CP_SPECIAL | (1 << 8))
>  #define ARM_CP_WFI (ARM_CP_SPECIAL | (2 << 8))
> -#define ARM_LAST_SPECIAL ARM_CP_WFI
> +#define ARM_CP_NZCV (ARM_CP_SPECIAL | (3 << 8))
> +#define ARM_LAST_SPECIAL ARM_CP_NZCV
>  /* Used only as a terminator for ARMCPRegInfo lists */
>  #define ARM_CP_SENTINEL 0x
>  /* Mask of only the flag bits in a type field */
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 66214293..ca27c8e 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -1560,6 +1560,64 @@ static const ARMCPRegInfo lpae_cp_reginfo[] = {
>  REGINFO_SENTINEL
>  };
>
> +static int aa64_fpcr_read(CPUARMState *env, const ARMCPRegInfo *ri,
> +  uint64_t *value)
> +{
> +*value = vfp_get_fpcr(env);
> +return 0;
> +}
> +
> +static int aa64_fpcr_write(CPUARMState *env, const ARMCPRegInfo *ri,
> +   uint64_t value)
> +{
> +vfp_set_fpcr(env, value);
> +return 0;
> +}
> +
> +static int aa64_fpsr_read(CPUARMState *env, const ARMCPRegInfo *ri,
> +  uint64_t *value)
> +{
> +*value = vfp_get_fpsr(env);
> +return 0;
> +}
> +
> +static int aa64_fpsr_write(CPUARMState *env, const ARMCPRegInfo *ri,
> +   uint64_t value)
> +{
> +vfp_set_fpsr(env, value);
> +return 0;
> +}
> +
> +static const ARMCPRegInfo v8_cp_reginfo[] = {
> +/* Minimal set of EL0-visible registers. This will need to be expanded
> + * significantly for system emulation of AArch64 CPUs.
> + */
> +{ .name = "NZCV", .state = ARM_CP_STATE_AA64,
> +  .opc0 = 3, .opc1 = 3, .opc2 = 0, .crn = 4, .crm = 2,
> +  .access = PL0_RW, .type = ARM_CP_NZCV },
> +{. name = "FPCR", .state = ARM_CP_STATE_AA64,
> + .opc0 = 3, .opc1 = 3, .opc2 = 0, .crn = 4, .crm = 4,
> + .access = PL0_RW, .readfn = aa64_fpcr_read, .writefn = aa64_fpcr_write 
> },

Indentation and spacing looks inconsistent.

Regards,
Peter

> +{. name = "FPSR", .state = ARM_CP_STATE_AA64,
> + .opc0 = 3, .opc1 = 3, .opc2 = 1, .crn = 4, .crm = 4,
> + .access = PL0_RW, .readfn = aa64_fpsr_read, .writefn = aa64_fpsr_write 
> },
> +/* This claims a 32 byte cacheline size for icache and dcache, VIPT 
> icache.
> + * It will eventually need to have a CPU-specified reset value.
> + */
> +{ .name = "CTR_EL0", .state = ARM_CP_STATE_AA64,
> +  .opc0 = 3, .opc1 = 3, .opc2 = 1, .crn = 0, .crm = 0,
> +  .access = PL0_R, .type = ARM_CP_CONST,
> +  .resetvalue = 0x80030003 },
> +/* Prohibit use of DC ZVA. OPTME: implement DC ZVA and allow its use.
> + * For system mode the DZP bit here will need to be computed, not 
> constant.
> + */
> +{ .name = "DCZID_EL0", .state = ARM_CP_STATE_AA64,
> +  .opc0 = 3, .opc1 = 3, .opc2 = 7, .crn = 0, .crm = 0,
> +  .access = PL0_R, .type = ARM_CP_CONST,
> +  .resetvalue = 0x10 },
> +REGINFO_SENTINEL
> +};
> +
>  static int sctlr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t 
> value)
>  {
>  env->cp15.c1_sys = value;
> @@ -1662,6 +1720,9 @@ void register_cp_regs_for_features(ARMCPU *cpu)
>  } else {
>  define_arm_cp_regs(cpu, not_v7_cp_reginfo);
>  }
> +if (arm_feature(env, ARM_FEATURE_V8)) {
> +define_arm_cp_regs(cpu, v8_cp_reginfo);
> +}
>  if (arm_feature(env, ARM_FEATURE_MPU)) {
>  /* These are the MPU registers prior to PMSAv6. Any new
>   * PMSA core later than the ARM946 will require that we
> diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
> index 7a9ee82..c8ed799 100644
> --- a/target-arm/translate-a64.c
> +++ b/target-arm/translate-a64.c
> @@ -733,6 +733,50 @@ static void handle_msr_i(DisasContext *s, uint32_t insn,
>  unsupported_encoding(s, insn);
>  }
>
> +static void gen_get_nzcv(TCGv_i64 tcg_rt)
> +{
> +TCGv_i32 tmp = tcg_temp_new_i32();
> +TCGv_i32 nzcv = tcg_temp_new_i32();
> +
> +/* build bit 31, N */
> +tcg_gen_andi_i32(nzcv, cpu_NF, (1 << 31));
> +/* build bit 30, Z */
> +tcg_gen_setcondi_i32(TCG_COND_EQ, tmp, cpu_ZF, 0);
> +tcg_gen_deposit_i32(nzcv, nzcv, tmp, 30, 1);
> +/* build bit 29, C */
> +tcg_gen_deposit_i32(nzcv, nzcv, cpu_CF,