Re: [Qemu-devel] [PATCH v2 7/9] i.MX: Add i.MX6 SOC implementation.

[Peter Crosthwaite]

On Sat, Feb 20, 2016 at 10:03 AM, Jean-Christophe DUBOIS
 wrote:
> Le 20/02/2016 16:30, Peter Crosthwaite a écrit :
>>
>> On Sat, Feb 20, 2016 at 2:55 AM, Jean-Christophe DUBOIS
>>  wrote:
>>>
>>> Just to compare I tried to run Linux on QEMU emulating highbank.
>>>
>>> For now I am unable to start in SMP mode. Only one core is activated.
>>>
>> This is probably due to the fact that the PSCI command encodings for
>> Highbank expected by the kernel are mismatched to the ones in QEMU. I
>> had some patches to change them, but once I got the PSCI right, the
>> boot hung (might be just extreme slowdown like you describe) and I
>> never got the time to fully debug it. SO there is a good chance
>> Highbank has the same bug.
>
>
> Thanks Peter,
>
> So we are not sure we have a reference Cortex A9 platform working with
> has_el3 set to true.
>

Not for SMP linux.

Regards,
Peter

> This is annoying.
>
> JC
>
>>
>> Regards,
>> Peter
>>
>

Re: [Qemu-devel] [PATCH v5] vhost-user interrupt management fixes

[Michael S. Tsirkin]

On Fri, Feb 19, 2016 at 10:02:16AM +0100, Didier Pallard wrote:
> Hi victor,
> 
> I'm sorry, i didn't get time to work on this patch.
> Thanks for your work.
> 
> didier

Could you please confirm that the patch works for you?

> 
> On 02/18/2016 03:12 PM, Victor Kaplansky wrote:
> >Since guest_mask_notifier can not be used in vhost-user mode due
> >to buffering implied by unix control socket, force
> >use_mask_notifier on virtio devices of vhost-user interfaces, and
> >send correct callfd to the guest at vhost start.
> >
> >Using guest_notifier_mask function in vhost-user case may
> >break interrupt mask paradigm, because mask/unmask is not
> >really done when returning from guest_notifier_mask call, instead
> >message is posted in a unix socket, and processed later.
> >
> >Add an option boolean flag 'use_mask_notifier' to disable the use
> >of guest_notifier_mask in virtio pci.
> >
> >Signed-off-by: Didier Pallard 
> >Signed-off-by: Victor Kaplansky 
> >---
> >v5 changes:
> >   - rebased to mst tree.
> >   - removed a traling space.
> >
> >v4 changes:
> >  In respond to Michael S. Tsirkin comments:
> >- changed the name of new field to use_guest_notifier_mask
> >- clarified comments
> >- vhost_virtqueue_mask() called for unmask
> >- cosmetic changes
> >- moved the initialization of the new field from virtio_reset
> >  to virtio_init.
> >
> >v3 changes:
> >  In respond to Michael S. Tsirkin comments:
> >- vhost_net.c: removed dependency on virtio-pci.h
> >- vhost.c: simplified the check for vhost-user backend,
> >  replaced by checking use_mask_notifier; added comment
> >  explaining why vring for vhost-user initialized in
> >  unmasked state;
> >- cosmetic fixes.
> >
> >v2 changes:
> >  - a new boolean field is added to all virtio devices instead
> >of defining a property in some virtio-pci devices.
> >
> >  include/hw/virtio/virtio.h |  1 +
> >  hw/net/vhost_net.c | 15 +--
> >  hw/virtio/vhost.c  |  9 +
> >  hw/virtio/virtio-pci.c | 14 --
> >  hw/virtio/virtio.c |  1 +
> >  5 files changed, 32 insertions(+), 8 deletions(-)
> >
> >diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> >index 108cdb0f..c38a2fef 100644
> >--- a/include/hw/virtio/virtio.h
> >+++ b/include/hw/virtio/virtio.h
> >@@ -90,6 +90,7 @@ struct VirtIODevice
> >  VMChangeStateEntry *vmstate;
> >  char *bus_name;
> >  uint8_t device_endian;
> >+bool use_guest_notifier_mask;
> >  QLIST_HEAD(, VirtQueue) *vector_queues;
> >  };
> >diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> >index b2428324..6e1032fc 100644
> >--- a/hw/net/vhost_net.c
> >+++ b/hw/net/vhost_net.c
> >@@ -284,8 +284,19 @@ int vhost_net_start(VirtIODevice *dev, NetClientState 
> >*ncs,
> >  }
> >  for (i = 0; i < total_queues; i++) {
> >-vhost_net_set_vq_index(get_vhost_net(ncs[i].peer), i * 2);
> >-}
> >+struct vhost_net *net;
> >+
> >+net = get_vhost_net(ncs[i].peer);
> >+vhost_net_set_vq_index(net, i * 2);
> >+
> >+/* Suppress the masking guest notifiers on vhost user
> >+ * because vhost user doesn't interrupt masking/unmasking
> >+ * properly.
> >+ */
> >+if (net->nc->info->type == NET_CLIENT_OPTIONS_KIND_VHOST_USER) {
> >+dev->use_guest_notifier_mask = false;
> >+}
> >+ }
> >  r = k->set_guest_notifiers(qbus->parent, total_queues * 2, true);
> >  if (r < 0) {
> >diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> >index 9f8ac38c..72d0c9e9 100644
> >--- a/hw/virtio/vhost.c
> >+++ b/hw/virtio/vhost.c
> >@@ -875,6 +875,14 @@ static int vhost_virtqueue_start(struct vhost_dev *dev,
> >  /* Clear and discard previous events if any. */
> >  event_notifier_test_and_clear(>masked_notifier);
> >+/* Init vring in unmasked state, unless guest_notifier_mask
> >+ * will do it later.
> >+ */
> >+if (!vdev->use_guest_notifier_mask) {
> >+/* TODO: check and handle errors. */
> >+vhost_virtqueue_mask(dev, vdev, idx, false);
> >+}
> >+
> >  return 0;
> >  fail_kick:
> >@@ -1167,6 +1175,7 @@ void vhost_virtqueue_mask(struct vhost_dev *hdev, 
> >VirtIODevice *vdev, int n,
> >  struct vhost_vring_file file;
> >  if (mask) {
> >+assert(vdev->use_guest_notifier_mask);
> >  file.fd = event_notifier_get_fd(>vqs[index].masked_notifier);
> >  } else {
> >  file.fd = 
> > event_notifier_get_fd(virtio_queue_get_guest_notifier(vvq));
> >diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> >index 5494ff4a..440776c0 100644
> >--- a/hw/virtio/virtio-pci.c
> >+++ b/hw/virtio/virtio-pci.c
> >@@ -806,7 +806,7 @@ static int kvm_virtio_pci_vector_use(VirtIOPCIProxy 
> >*proxy, int nvqs)
> >  /* If guest supports masking, set up irqfd now.
> >   * Otherwise, delay until unmasked in 

Re: [Qemu-devel] [PATCH v2 7/9] i.MX: Add i.MX6 SOC implementation.

[Jean-Christophe DUBOIS]

Le 20/02/2016 16:30, Peter Crosthwaite a écrit :

On Sat, Feb 20, 2016 at 2:55 AM, Jean-Christophe DUBOIS
 wrote:

Just to compare I tried to run Linux on QEMU emulating highbank.

For now I am unable to start in SMP mode. Only one core is activated.


This is probably due to the fact that the PSCI command encodings for
Highbank expected by the kernel are mismatched to the ones in QEMU. I
had some patches to change them, but once I got the PSCI right, the
boot hung (might be just extreme slowdown like you describe) and I
never got the time to fully debug it. SO there is a good chance
Highbank has the same bug.


Thanks Peter,

So we are not sure we have a reference Cortex A9 platform working with 
has_el3 set to true.


This is annoying.

JC



Regards,
Peter

[Qemu-devel] [PATCH 3/4] block/null-{co, aio}: Implement get_block_status()

[Max Reitz]

Signed-off-by: Max Reitz 
---
 block/null.c | 17 +
 1 file changed, 17 insertions(+)

diff --git a/block/null.c b/block/null.c
index ad883d9..0a8ff7b 100644
--- a/block/null.c
+++ b/block/null.c
@@ -186,6 +186,19 @@ static int null_reopen_prepare(BDRVReopenState 
*reopen_state,
 return 0;
 }
 
+static int64_t coroutine_fn null_co_get_block_status(BlockDriverState *bs,
+ int64_t sector_num,
+ int nb_sectors, int *pnum,
+ BlockDriverState **file)
+{
+off_t start = sector_num * BDRV_SECTOR_SIZE;
+
+*pnum = nb_sectors;
+*file = bs;
+
+return BDRV_BLOCK_ZERO | BDRV_BLOCK_OFFSET_VALID | start;
+}
+
 static BlockDriver bdrv_null_co = {
 .format_name= "null-co",
 .protocol_name  = "null-co",
@@ -199,6 +212,8 @@ static BlockDriver bdrv_null_co = {
 .bdrv_co_writev = null_co_writev,
 .bdrv_co_flush_to_disk  = null_co_flush,
 .bdrv_reopen_prepare= null_reopen_prepare,
+
+.bdrv_co_get_block_status   = null_co_get_block_status,
 };
 
 static BlockDriver bdrv_null_aio = {
@@ -214,6 +229,8 @@ static BlockDriver bdrv_null_aio = {
 .bdrv_aio_writev= null_aio_writev,
 .bdrv_aio_flush = null_aio_flush,
 .bdrv_reopen_prepare= null_reopen_prepare,
+
+.bdrv_co_get_block_status   = null_co_get_block_status,
 };
 
 static void bdrv_null_init(void)
-- 
2.7.1

[Qemu-devel] [PATCH 0/4] qemu-img: Fix preallocation with -S 0 for convert

[Max Reitz]

Using -S 0 is supposed to allocate everything in the output image; or at
least it is supposed to always explicitly write zeros even if the area
in question is known to only contain zeros. That doesn't always work
right now, so this series fixes it (patch 1, to be specific).

I only noticed after I had written the test added by patch 4 that we
already had an -S 0 test case which is included in the iotest 122.
However, the test added here works for all image formats and is maybe
more of a direct test (instead of querying the format whether it thinks
it allocated all of the data we directly ask du whether everything has
been allocated) so maybe it reflects better what users expect -S 0 to
do. Maybe.

Patches 2 and 3 are required for the test. I could have written the test
without doing a convert with null-co as the source, but that would have
been boring, so I did not.

If you want to argue that in light of the existence of test 122 the new
test added here is unnecessary and we therefore do not need patches 2, 3
and 4, please go ahead. I won't put up too much of a fight.


Max Reitz (4):
  qemu-img: Fix preallocation with -S 0 for convert
  block/null-{co,aio}: Return zeros when read
  block/null-{co,aio}: Implement get_block_status()
  iotests: Test qemu-img convert -S 0 behavior

 block/null.c   |  19 
 qemu-img.c |  37 ++--
 tests/qemu-iotests/122.out |   6 +--
 tests/qemu-iotests/146 | 105 +
 tests/qemu-iotests/146.out |  14 ++
 tests/qemu-iotests/group   |   1 +
 6 files changed, 165 insertions(+), 17 deletions(-)
 create mode 100755 tests/qemu-iotests/146
 create mode 100644 tests/qemu-iotests/146.out

-- 
2.7.1

[Qemu-devel] [PATCH 2/4] block/null-{co, aio}: Return zeros when read

[Max Reitz]

Currently, we do not define exactly what is returned when read, but
having a reliable source of zeros is always nice.

Signed-off-by: Max Reitz 
---
 block/null.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/block/null.c b/block/null.c
index d90165d..ad883d9 100644
--- a/block/null.c
+++ b/block/null.c
@@ -90,6 +90,7 @@ static coroutine_fn int null_co_readv(BlockDriverState *bs,
   int64_t sector_num, int nb_sectors,
   QEMUIOVector *qiov)
 {
+qemu_iovec_memset(qiov, 0, 0, nb_sectors * BDRV_SECTOR_SIZE);
 return null_co_common(bs);
 }
 
@@ -159,6 +160,7 @@ static BlockAIOCB *null_aio_readv(BlockDriverState *bs,
   BlockCompletionFunc *cb,
   void *opaque)
 {
+qemu_iovec_memset(qiov, 0, 0, nb_sectors * BDRV_SECTOR_SIZE);
 return null_aio_common(bs, cb, opaque);
 }
 
-- 
2.7.1

[Qemu-devel] [PATCH 4/4] iotests: Test qemu-img convert -S 0 behavior

[Max Reitz]

Passing -S 0 to qemu-img convert should result in all source data being
copied to the output, even if that source data is known to be 0. The
output image should therefore have exactly the same size on disk as an
image which we explicitly filled with data.

Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/146 | 105 +
 tests/qemu-iotests/146.out |  14 ++
 tests/qemu-iotests/group   |   1 +
 3 files changed, 120 insertions(+)
 create mode 100755 tests/qemu-iotests/146
 create mode 100644 tests/qemu-iotests/146.out

diff --git a/tests/qemu-iotests/146 b/tests/qemu-iotests/146
new file mode 100755
index 000..611602e
--- /dev/null
+++ b/tests/qemu-iotests/146
@@ -0,0 +1,105 @@
+#!/bin/bash
+#
+# Test that qemu-img convert -S 0 fully allocates the target image
+#
+# Copyright (C) 2016 Red Hat, 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 .
+#
+
+# creator
+owner=mre...@redhat.com
+
+seq="$(basename $0)"
+echo "QA output created by $seq"
+
+here="$PWD"
+tmp=/tmp/$$
+status=1   # failure is the default!
+
+_cleanup()
+{
+_cleanup_test_img
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+_supported_fmt generic
+_supported_proto file
+_supported_os Linux
+
+
+on_disk_size()
+{
+du "$@" | sed -e 's/\t\+.*//'
+}
+
+
+img_size=1048576
+
+
+echo
+echo '=== Comparing empty image against sparse conversion ==='
+echo
+
+_make_test_img $img_size
+
+empty_size=$(on_disk_size "$TEST_IMG")
+
+
+$QEMU_IMG_PROG convert -O "$IMGFMT" -S 512 \
+"json:{ 'driver': 'null-co', 'size': $img_size }" \
+"$TEST_IMG"
+
+sparse_convert_size=$(on_disk_size "$TEST_IMG")
+
+
+if [ "$empty_size" -eq "$sparse_convert_size" ]; then
+echo 'Equal image size'
+else
+echo 'Different image size'
+fi
+
+
+echo
+echo '=== Comparing full image against non-sparse conversion ==='
+echo
+
+_make_test_img $img_size
+$QEMU_IO -c "write 0 $img_size" "$TEST_IMG" | _filter_qemu_io
+
+full_size=$(on_disk_size "$TEST_IMG")
+
+
+$QEMU_IMG convert -O "$IMGFMT" -S 0 \
+"json:{ 'driver': 'null-co', 'size': $img_size }" \
+"$TEST_IMG"
+
+non_sparse_convert_size=$(on_disk_size "$TEST_IMG")
+
+
+if [ "$full_size" -eq "$non_sparse_convert_size" ]; then
+echo 'Equal image size'
+else
+echo 'Different image size'
+fi
+
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/146.out b/tests/qemu-iotests/146.out
new file mode 100644
index 000..5464fba
--- /dev/null
+++ b/tests/qemu-iotests/146.out
@@ -0,0 +1,14 @@
+QA output created by 146
+
+=== Comparing empty image against sparse conversion ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576
+Equal image size
+
+=== Comparing full image against non-sparse conversion ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576
+wrote 1048576/1048576 bytes at offset 0
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+Equal image size
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 65df029..524cd30 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -147,3 +147,4 @@
 142 auto
 143 auto quick
 144 rw auto quick
+146 rw auto quick
-- 
2.7.1

[Qemu-devel] [PATCH 1/4] qemu-img: Fix preallocation with -S 0 for convert

[Max Reitz]

When passing -S 0 to qemu-img convert, the target image is supposed to
be fully allocated. Right now, this is not the case if the source image
contains areas which bdrv_get_block_status() reports as being zero.

This patch introduces a new ImgConvertBlockStatus named BLK_ZERO_DATA
which is set by convert_iteration_sectors() if an area is detected to be
zero and min_sparse is 0. Simply using BLK_DATA is not optimal, because
knowing an area to be zero allows us to memset() the buffer to be
written directly rather than having to use blk_read().

Other than that, BLK_ZERO_DATA is handled exactly like BLK_DATA.

This patch changes the reference output for iotest 122; contrary to what
it assumed, -S 0 really should allocate everything in the output, not
just areas that are filled with zeros (as opposed to being zeroed).

Signed-off-by: Max Reitz 
---
 qemu-img.c | 37 -
 tests/qemu-iotests/122.out |  6 ++
 2 files changed, 26 insertions(+), 17 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index fb9ab1f..809ea3b 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1232,6 +1232,10 @@ enum ImgConvertBlockStatus {
 BLK_DATA,
 BLK_ZERO,
 BLK_BACKING_FILE,
+
+/* Will return zeros when read but must be written as data (i.e. is set for
+ * zeroed areas with min_sparse == 0) */
+BLK_ZERO_DATA,
 };
 
 typedef struct ImgConvertState {
@@ -1282,7 +1286,13 @@ static int convert_iteration_sectors(ImgConvertState *s, 
int64_t sector_num)
 }
 
 if (ret & BDRV_BLOCK_ZERO) {
-s->status = BLK_ZERO;
+if (s->min_sparse) {
+s->status = BLK_ZERO;
+} else {
+/* If min_sparse is 0, this area must be written to the target
+ * image as data */
+s->status = BLK_ZERO_DATA;
+}
 } else if (ret & BDRV_BLOCK_DATA) {
 s->status = BLK_DATA;
 } else if (!s->target_has_backing) {
@@ -1300,7 +1310,7 @@ static int convert_iteration_sectors(ImgConvertState *s, 
int64_t sector_num)
 }
 
 n = MIN(n, s->sector_next_status - sector_num);
-if (s->status == BLK_DATA) {
+if (s->status == BLK_DATA || s->status == BLK_ZERO_DATA) {
 n = MIN(n, s->buf_sectors);
 }
 
@@ -1325,10 +1335,6 @@ static int convert_read(ImgConvertState *s, int64_t 
sector_num, int nb_sectors,
 int n;
 int ret;
 
-if (s->status == BLK_ZERO || s->status == BLK_BACKING_FILE) {
-return 0;
-}
-
 assert(nb_sectors <= s->buf_sectors);
 while (nb_sectors > 0) {
 BlockBackend *blk;
@@ -1372,6 +1378,7 @@ static int convert_write(ImgConvertState *s, int64_t 
sector_num, int nb_sectors,
 break;
 
 case BLK_DATA:
+case BLK_ZERO_DATA:
 /* We must always write compressed clusters as a whole, so don't
  * try to find zeroed parts in the buffer. We can only save the
  * write if the buffer is completely zeroed and we're allowed to
@@ -1466,7 +1473,7 @@ static int convert_do_copy(ImgConvertState *s)
 ret = n;
 goto fail;
 }
-if (s->status == BLK_DATA) {
+if (s->status == BLK_DATA || s->status == BLK_ZERO_DATA) {
 s->allocated_sectors += n;
 }
 sector_num += n;
@@ -1486,17 +1493,21 @@ static int convert_do_copy(ImgConvertState *s)
 ret = n;
 goto fail;
 }
-if (s->status == BLK_DATA) {
+if (s->status == BLK_DATA || s->status == BLK_ZERO_DATA) {
 allocated_done += n;
 qemu_progress_print(100.0 * allocated_done / s->allocated_sectors,
 0);
 }
 
-ret = convert_read(s, sector_num, n, buf);
-if (ret < 0) {
-error_report("error while reading sector %" PRId64
- ": %s", sector_num, strerror(-ret));
-goto fail;
+if (s->status == BLK_DATA) {
+ret = convert_read(s, sector_num, n, buf);
+if (ret < 0) {
+error_report("error while reading sector %" PRId64
+ ": %s", sector_num, strerror(-ret));
+goto fail;
+}
+} else if (s->status == BLK_ZERO_DATA) {
+memset(buf, 0, n * BDRV_SECTOR_SIZE);
 }
 
 ret = convert_write(s, sector_num, n, buf);
diff --git a/tests/qemu-iotests/122.out b/tests/qemu-iotests/122.out
index 0068e96..98814de 100644
--- a/tests/qemu-iotests/122.out
+++ b/tests/qemu-iotests/122.out
@@ -112,16 +112,14 @@ read 3145728/3145728 bytes at offset 0
 3 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 read 63963136/63963136 bytes at offset 3145728
 61 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-[{ "start": 0, "length": 6291456, "depth": 0, "zero": false, "data": true, 
"offset": 327680},
-{ "start": 6291456, "length": 60817408, "depth": 

Re: [Qemu-devel] [PATCH v2 7/9] i.MX: Add i.MX6 SOC implementation.

[Peter Crosthwaite]

On Sat, Feb 20, 2016 at 2:55 AM, Jean-Christophe DUBOIS
 wrote:
> Just to compare I tried to run Linux on QEMU emulating highbank.
>
> For now I am unable to start in SMP mode. Only one core is activated.
>

This is probably due to the fact that the PSCI command encodings for
Highbank expected by the kernel are mismatched to the ones in QEMU. I
had some patches to change them, but once I got the PSCI right, the
boot hung (might be just extreme slowdown like you describe) and I
never got the time to fully debug it. SO there is a good chance
Highbank has the same bug.

Regards,
Peter

Re: [Qemu-devel] [PATCH v2 0/2] qemu-iotests: fix 140 on s390x

[Max Reitz]

On 18.02.2016 21:37, Sascha Silbe wrote:
> Yet another IDE-using test crept in (commit 16dee418). Fix it by
> disabling the implicit drive.
> 
> v1->v2:
> - don't use any drive at all instead of replacing IDE with virtio-scsi
> - split off virtio alias patch into separate series (no longer needed
>   in this one)
> - add patch to improve description of qemu-iotest 140
> 
> 
> Sascha Silbe (2):
>   qemu-iotests: 140: don't use IDE drive
>   qemu-iotests: 140: make description slightly more verbose
> 
>  tests/qemu-iotests/140 | 8 ++--
>  tests/qemu-iotests/140.out | 1 -
>  2 files changed, 6 insertions(+), 3 deletions(-)

Thanks, applied to my block tree:

https://github.com/XanClic/qemu/commits/block

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v2 2/2] qemu-iotests: 140: make description slightly more verbose

[Max Reitz]

On 18.02.2016 21:37, Sascha Silbe wrote:
> Describe in a little more detail what the test is supposed to achieve.
> 
> Signed-off-by: Sascha Silbe 
> ---
> Max, does this reflect your intentions well enough? I took the liberty
> of re-using some of your review comments to extend the description.

Yes, it's fine, thanks.

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v2 1/1] qemu-iotests: 067: ignore QMP events

[Max Reitz]

On 19.02.2016 14:01, Sascha Silbe wrote:
> The relative ordering of "device_del" return value and the
> "DEVICE_DELETED" QMP event depends on the architecture being
> tested. On x86 unplugging virtio disks is asynchronous
> (=qdev_unplug()= → =hotplug_handler_unplug_request()=) while on s390x
> it is synchronous (=qdev_unplug()= → =hotplug_handler_unplug()=). This
> leads to the actual output on s390x consistently differing from the
> reference output (that was probably produced on x86).
> 
> The easiest way to address this is to filter out QMP events in
> 067. The DEVICE_DELETED event is already getting explicitly tested by
> the Python-based test case 139, so the test coverage should be
> unaffected. Make use of the recently introduced _filter_qmp_events()
> to remove QMP events from the test case output and adjust the
> reference output accordingly.
> 
> The tr / sed / tr trick used for filtering was suggested by Max Reitz
> .
> 
> Signed-off-by: Sascha Silbe 
> ---
> v1->v2:
>   - squashed the two patches
>   - using tr + sed incantation so we can keep the pretty-printing, but
> moved the filter into 067 as it's not general enough for
> common.filter
> ---
>  tests/qemu-iotests/067 |  11 +++-
>  tests/qemu-iotests/067.out | 144 
> -
>  2 files changed, 10 insertions(+), 145 deletions(-)

Thanks, applied to my block tree:

https://github.com/XanClic/qemu/commits/block

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v2 1/1] quorum: Change vote rules for 64 bits hash

[Max Reitz]

On 19.02.2016 12:24, Alberto Garcia wrote:
> On Fri 19 Feb 2016 09:26:53 AM CET, Wen Congyang  wrote:
> 
 If quorum has two children(A, B). A do flush sucessfully, but B
 flush failed.  We MUST choice A as winner rather than just pick
 anyone of them. Otherwise the filesystem of guest will become
 read-only with following errors:

 end_request: I/O error, dev vda, sector 11159960
 Aborting journal on device vda3-8
 EXT4-fs error (device vda3): ext4_journal_start_sb:327: Detected abort 
 journal
 EXT4-fs (vda3): Remounting filesystem read-only
>>>
>>> Hi Xie,
>>>
>>> Let's see if I'm getting this right:
>>>
>>> - When Quorum flushes to disk, there's a vote among the return values of
>>>   the flush operations of its members, and the one that wins is the one
>>>   that Quorum returns.
>>>
>>> - If there's a tie then Quorum choses the first result from the list of
>>>   winners.
>>>
>>> - With your patch you want to give priority to the vote with result == 0
>>>   if there's any, so Quorum would return 0 (and succeed).
>>>
>>> This seems to me like an ad-hoc fix for a particular use case. What
>>> if you have 3 members and two of them fail with the same error code?
>>> Would you still return 0 or the error code from the other two?
>>
>> For example:
>> children.0 returns 0
>> children.1 returns -EIO
>> children.2 returns -EPIPE
>>
>> In this case, quorum returns -EPIPE now(without this patch).
>>
>> For example:
>> children.0 returns -EPIPE
>> children.1 returns -EIO
>> children.2 returns 0
>> In this case, quorum returns 0 now.
> 
> My question is: what's the rationale for returning 0 in case a) but not
> in case b)?
> 
>   a)
> children.0 returns -EPIPE
> children.1 returns -EIO
> children.2 returns 0
> 
>   b)
> children.0 returns -EIO
> children.1 returns -EIO
> children.2 returns 0
> 
> In both cases you have one successful flush and two errors. You want to
> return always 0 in case a) and always -EIO in case b). But the only
> difference is that in case b) the errors happen to be the same, so why
> does that matter?
> 
> That said, I'm not very convinced of the current logics of the Quorum
> flush code either, so it's not even a problem with your patch... it
> seems to me that the code should follow the same logics as in the
> read/write case: if the number of correct flushes >= threshold then
> return 0, else select the most common error code.

I'm not convinced of the logic either, which is why I waited for you to
respond to this patch. :-)

Intuitively, I'd expect Quorum to return an error if flushing failed for
any of the children, because, well, flushing failed. I somehow feel like
flushing is different from a read or write operation and therefore
ignoring the threshold would be fine here. However, maybe my intuition
is just off.

Anyway, regardless of that, if we do take the threshold into account, we
should not use the exact error value for voting but just whether an
error occurred or not. If all but one children fail to flush (all for
different reasons), I find it totally wrong to return success. We should
then just return -EIO or something.

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v3 09/14] block: Move some bdrv_*_all() functions to BB

[Max Reitz]

On 17.02.2016 16:51, Kevin Wolf wrote:
> Am 16.02.2016 um 19:08 hat Max Reitz geschrieben:
>> Move bdrv_commit_all() and bdrv_flush_all() to the BlockBackend level.
>>
>> Signed-off-by: Max Reitz 
>> ---
>>  block.c   | 20 
>>  block/block-backend.c | 44 
>> +++
>>  block/io.c| 20 
>>  include/block/block.h |  2 --
>>  stubs/Makefile.objs   |  2 +-
>>  stubs/{bdrv-commit-all.c => blk-commit-all.c} |  4 +--
>>  6 files changed, 41 insertions(+), 51 deletions(-)
>>  rename stubs/{bdrv-commit-all.c => blk-commit-all.c} (53%)
>>

[...]

>> diff --git a/block/block-backend.c b/block/block-backend.c
>> index a10fe44..a918c35 100644
>> --- a/block/block-backend.c
>> +++ b/block/block-backend.c

[...]

>> @@ -1361,5 +1356,42 @@ BlockBackendRootState 
>> *blk_get_root_state(BlockBackend *blk)
>>  
>>  int blk_commit_all(void)
>>  {
>> -return bdrv_commit_all();
>> +BlockBackend *blk = NULL;
>> +
>> +while ((blk = blk_all_next(blk)) != NULL) {
>> +AioContext *aio_context = blk_get_aio_context(blk);
>> +
>> +aio_context_acquire(aio_context);
>> +if (blk_is_available(blk) && blk->bs->drv && blk->bs->backing) {
> 
> blk->bs->drv is already implied by blk_is_available(), so it's
> unnecessary, even though it doesn't actively hurt.
> 
> Also, using blk_is_available() instead of blk_is_inserted() as in
> blk_flush_all() means that a drive with an open tray, but inserted
> medium isn't committed. I assume you did this on purpose because you're
> using two different functions in this patch, but isn't this a change in
> behaviour? If so, and we really want it, it should at least be mentioned
> in the commit message.

Drives with open trays are strange.

I found it reasonable that every medium in the system should be flushed
on blk_flush_all(), because flushing data should only bring the on-disk
state to a state it is supposed to be in anyway.

On the other hand, the commit operation actively changes data.
Therefore, I decided to treat it like a guest write operation. However,
considering that blk_commit_all() is basically only available through
HMP and is thus more of a legacy operation, I don't think its behavior
should be changed here. I'm therefore going to change this to
blk_is_inserted(), thanks.

Max



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [Bug 1547526] Re: Java program does not execute on SPARC Solaris 8

[leoheck]

Hi Mark, it is very good to hear that it isn't only with me.

Let me know if you need a help to test and/or collect any information to
help you to discover/fix this problem.

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

Title:
  Java program does not execute on SPARC Solaris 8

Status in QEMU:
  New

Bug description:
  Hello,

  I am trying to run a java program that never execute. The program uses
  jre1.1.5 which came with the java program. I don't know what to do to
  run this application. There are some random messages in command line
  that can be related to my problem (or not). They are:

#1. Webstart launcher crashing.
Also found here: 
http://www.openfirmware.info/pipermail/openbios/2011-May/006472.html

#2. Assertion failed: MUTEX_HELD(_mutex), file rpc/svc_run.c, line 
766
Which was already reported here: 
https://bugs.launchpad.net/qemu/+bug/1450881

#3. Some problems with libthread in Solaris. 
I have tried a workaround setting LM_LIBRARY_PATH to use another 
version of libthread that Solaris 8 has.

  I don't know if this is a qemu problem or Solaris problem.
  My java application can be executed in command line or in GUI but I've tried 
both with no luck. I also have tryed other versions of JRE from 1.1.8 to 1.5 
but no luck either.

  I appreciate **any information** that can help me to execute the java 
program!!
  Thank you.

  I am using qemu-system-sparc (v2.5.50) with Solaris 8 
(solaris-8-hw4-2.04-sparc).
  The host is an Ubuntu 15.10 and I am using the openbios-sparc from Ubuntus 
ppa as shown bellow:

  openbios-sparc | 1.1+svn1334-1 |
  http://archive.ubuntu.com/ubuntu/ wily/universe amd64 Packages

  The command line used to launch qemu is:

qemu-system-sparc \
-M SS-5 \
-m 256 \
-boot c \
-cdrom $(DATA_ISO) \
-drive file=root-disk.img,index=0,media=disk,format=raw \
-serial stdio \
-monitor tcp::,server,nowait \
-localtime \
-net user \
-net nic \
$(ui)

  DATA_ISO is the way I found to send my data to the guest.

  The root-disk.img is:

Disk root-disk.img: 36 GiB, 38654705664 bytes, 75497472 sectors
Geometry: 27 heads, 107 sectors/track, 24620 cylinders
Units: sectors of 1 * 512 = 512 bytes
Sector size (logical/physical): 512 bytes / 512 bytes
I/O size (minimum/optimal): 512 bytes / 512 bytes
Disklabel type: sun

Device   Start  End  Sectors   Size Id Type   Flags
root-disk.img1   0  2744549  2744550   1.3G  2 SunOS root  
root-disk.img2 2744550  3047894   303345 148.1M  3 SunOS swapu 
root-disk.img3   0 71127179 71127180  33.9G  5 Whole disk  
root-disk.img8 3047895 71127179 68079285  32.5G  8 SunOS home  

image: root-disk.img
file format: raw
virtual size: 36G (38654705664 bytes)
disk size: 1.2G

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

Re: [Qemu-devel] [PATCH v3 07/14] blockdev: Add list of monitor-owned BlockBackends

[Max Reitz]

On 20.02.2016 14:34, Max Reitz wrote:
> On 17.02.2016 17:20, Kevin Wolf wrote:
>> Am 17.02.2016 um 16:41 hat Max Reitz geschrieben:
>>> On 17.02.2016 11:53, Kevin Wolf wrote:
 Am 16.02.2016 um 19:08 hat Max Reitz geschrieben:
> The monitor does hold references to some BlockBackends so it should have

 s/does hold/holds/?
>>>
>>> It was intentional, so I'd keep it unless you drop the question mark.
>>
>> For me it seems to imply something like "contrary to your expectations",
>> but maybe that's just my non-native English needing a fix.
>>
>> I don't find it surprising anyway that the monitor holds BB references.
> 
> The contrast I tried to point out is that while we do have these
> references in theory, and they are reflected by a refcount, too, we do
> not actually have these references because the monitor does not yet have
> a list of the BBs it owns.
> 
> So it's not an "emphasize the verb because it may be contrary to your
> expectations", but an "emphasize it because it is contrary to what the
> current code does" (which is not actually referencing these BBs).
> 
> Like: It is supposed to have references. It says it does. But it
> actually doesn't. It does "hold" them, however, because they are
> accounted for in the BBs' refcounts.
> 
> a list of those BBs; blk_backends is a different list, as it contains
> references to all BBs (after a follow-up patch, that is), and that
> should not be changed because we do need such a list.
>
> monitor_remove_blk() is idempotent so that we can call it in
> blockdev_auto_del() without having to care whether it had been called in
> do_drive_del() before. monitor_add_blk() is idempotent for symmetry
> reasons (monitor_remove_blk() is, so it would be strange for
> monitor_add_blk() not to be).
>
> Signed-off-by: Max Reitz 

 I think hmp_drive_add() needs a monitor_remove_blk() in its error path.
>>>
>>> You're right, thanks.
>>>
>>> In addition, if we really do say that a BB having a name equals being
>>> referenced by the monitor, then maybe we don't need explicit calls to
>>> monitor_add_blk() because any BB that is created with a non-NULL name
>>> should be automatically added to the list of monitor BBs.
>>
>> While probably workable, I'd rather avoid this kind of magic where the
>> presence of a name parameter decides whether a reference is taken or
>> not. It makes the interface of the function a lot less obvious.
> 
> Still you want the name to be the monitor's reference to the BB. Thus,
> if monitor_add_blk() should not be called implicitly by blk_new(), then
> I'd instead move the @name parameter from blk_new() to monitor_add_blk().

...aaand I noticed that this is what you said for patch 8 anyway. Good.

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v3 07/14] blockdev: Add list of monitor-owned BlockBackends

[Max Reitz]

On 17.02.2016 17:20, Kevin Wolf wrote:
> Am 17.02.2016 um 16:41 hat Max Reitz geschrieben:
>> On 17.02.2016 11:53, Kevin Wolf wrote:
>>> Am 16.02.2016 um 19:08 hat Max Reitz geschrieben:
 The monitor does hold references to some BlockBackends so it should have
>>>
>>> s/does hold/holds/?
>>
>> It was intentional, so I'd keep it unless you drop the question mark.
> 
> For me it seems to imply something like "contrary to your expectations",
> but maybe that's just my non-native English needing a fix.
> 
> I don't find it surprising anyway that the monitor holds BB references.

The contrast I tried to point out is that while we do have these
references in theory, and they are reflected by a refcount, too, we do
not actually have these references because the monitor does not yet have
a list of the BBs it owns.

So it's not an "emphasize the verb because it may be contrary to your
expectations", but an "emphasize it because it is contrary to what the
current code does" (which is not actually referencing these BBs).

Like: It is supposed to have references. It says it does. But it
actually doesn't. It does "hold" them, however, because they are
accounted for in the BBs' refcounts.

 a list of those BBs; blk_backends is a different list, as it contains
 references to all BBs (after a follow-up patch, that is), and that
 should not be changed because we do need such a list.

 monitor_remove_blk() is idempotent so that we can call it in
 blockdev_auto_del() without having to care whether it had been called in
 do_drive_del() before. monitor_add_blk() is idempotent for symmetry
 reasons (monitor_remove_blk() is, so it would be strange for
 monitor_add_blk() not to be).

 Signed-off-by: Max Reitz 
>>>
>>> I think hmp_drive_add() needs a monitor_remove_blk() in its error path.
>>
>> You're right, thanks.
>>
>> In addition, if we really do say that a BB having a name equals being
>> referenced by the monitor, then maybe we don't need explicit calls to
>> monitor_add_blk() because any BB that is created with a non-NULL name
>> should be automatically added to the list of monitor BBs.
> 
> While probably workable, I'd rather avoid this kind of magic where the
> presence of a name parameter decides whether a reference is taken or
> not. It makes the interface of the function a lot less obvious.

Still you want the name to be the monitor's reference to the BB. Thus,
if monitor_add_blk() should not be called implicitly by blk_new(), then
I'd instead move the @name parameter from blk_new() to monitor_add_blk().

Max

>> But that would mean that qemu-img's, qemu-nbd's and qemu-io's BBs would
>> have to be monitor-owned, too, and they'd all have to call
>> monitor_remove_blk() all over the place... Unless we'd allow NULL BB
>> names now and make them use it (I don't really see a reason why not;
>> them calling their BBs "hda" seems weird anyway), or implicitly call
>> monitor_remove_blk() in blk_delete(). Or maybe both, because the latter
>> seems convenient anyway.
> 
> I'm not sure. It would be correct even when we start to create BBs
> automatically, because monitor_remove_blk() doesn't do anything when
> there is no monitor reference and the monitor reference is the last
> thing that can be returned (hopefully). But I like reference counting to
> be as explicit as possible.
> 
> Kevin
> 




signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v9 4/5] acpi: arm: add fw_cfg device node to dsdt

[Shannon Zhao]

On 2016/2/20 2:20, Gabriel L. Somlo wrote:

Add a fw_cfg device node to the ACPI DSDT. This is mostly
informational, as the authoritative fw_cfg MMIO region(s)
are listed in the Device Tree. However, since we are building
ACPI tables, we might as well be thorough while at it...

Signed-off-by: Gabriel Somlo 
Reviewed-by: Laszlo Ersek 
Tested-by: Laszlo Ersek 
Reviewed-by: Marc Marí 

Reviewed-by: Shannon Zhao 


---
  hw/arm/virt-acpi-build.c | 15 +++
  1 file changed, 15 insertions(+)

diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 8cf9a21..7d7998b 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -81,6 +81,20 @@ static void acpi_dsdt_add_uart(Aml *scope, const MemMapEntry 
*uart_memmap,
  aml_append(scope, dev);
  }

+static void acpi_dsdt_add_fw_cfg(Aml *scope, const MemMapEntry *fw_cfg_memmap)
+{
+Aml *dev = aml_device("FWCF");
+aml_append(dev, aml_name_decl("_HID", aml_string("QEMU0002")));
+/* device present, functioning, decoding, not shown in UI */
+aml_append(dev, aml_name_decl("_STA", aml_int(0xB)));
+
+Aml *crs = aml_resource_template();
+aml_append(crs, aml_memory32_fixed(fw_cfg_memmap->base,
+   fw_cfg_memmap->size, AML_READ_WRITE));
+aml_append(dev, aml_name_decl("_CRS", crs));
+aml_append(scope, dev);
+}
+
  static void acpi_dsdt_add_flash(Aml *scope, const MemMapEntry *flash_memmap)
  {
  Aml *dev, *crs;
@@ -548,6 +562,7 @@ build_dsdt(GArray *table_data, GArray *linker, 
VirtGuestInfo *guest_info)
  acpi_dsdt_add_uart(scope, [VIRT_UART],
 (irqmap[VIRT_UART] + ARM_SPI_BASE));
  acpi_dsdt_add_flash(scope, [VIRT_FLASH]);
+acpi_dsdt_add_fw_cfg(scope, [VIRT_FW_CFG]);
  acpi_dsdt_add_virtio(scope, [VIRT_MMIO],
  (irqmap[VIRT_MMIO] + ARM_SPI_BASE), 
NUM_VIRTIO_TRANSPORTS);
  acpi_dsdt_add_pci(scope, memmap, (irqmap[VIRT_PCIE] + ARM_SPI_BASE),



--
Shannon

[Qemu-devel] [Bug 1547526] Re: Java program does not execute on SPARC Solaris 8

[Mark Cave-Ayland]

I've been looking at this as I have time, and IMO it's a qemu bug.
Fortunately I have a easy local reproducer here, and from what I can see
in this one particular case it seems the QEMU doesn't honour the annul
bit in the branch (or apparently corrupts it according to the guest?!).
So yes, it's a known issue and I am working hard to try and isolate the
bug and come up with a fix.

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

Title:
  Java program does not execute on SPARC Solaris 8

Status in QEMU:
  New

Bug description:
  Hello,

  I am trying to run a java program that never execute. The program uses
  jre1.1.5 which came with the java program. I don't know what to do to
  run this application. There are some random messages in command line
  that can be related to my problem (or not). They are:

#1. Webstart launcher crashing.
Also found here: 
http://www.openfirmware.info/pipermail/openbios/2011-May/006472.html

#2. Assertion failed: MUTEX_HELD(_mutex), file rpc/svc_run.c, line 
766
Which was already reported here: 
https://bugs.launchpad.net/qemu/+bug/1450881

#3. Some problems with libthread in Solaris. 
I have tried a workaround setting LM_LIBRARY_PATH to use another 
version of libthread that Solaris 8 has.

  I don't know if this is a qemu problem or Solaris problem.
  My java application can be executed in command line or in GUI but I've tried 
both with no luck. I also have tryed other versions of JRE from 1.1.8 to 1.5 
but no luck either.

  I appreciate **any information** that can help me to execute the java 
program!!
  Thank you.

  I am using qemu-system-sparc (v2.5.50) with Solaris 8 
(solaris-8-hw4-2.04-sparc).
  The host is an Ubuntu 15.10 and I am using the openbios-sparc from Ubuntus 
ppa as shown bellow:

  openbios-sparc | 1.1+svn1334-1 |
  http://archive.ubuntu.com/ubuntu/ wily/universe amd64 Packages

  The command line used to launch qemu is:

qemu-system-sparc \
-M SS-5 \
-m 256 \
-boot c \
-cdrom $(DATA_ISO) \
-drive file=root-disk.img,index=0,media=disk,format=raw \
-serial stdio \
-monitor tcp::,server,nowait \
-localtime \
-net user \
-net nic \
$(ui)

  DATA_ISO is the way I found to send my data to the guest.

  The root-disk.img is:

Disk root-disk.img: 36 GiB, 38654705664 bytes, 75497472 sectors
Geometry: 27 heads, 107 sectors/track, 24620 cylinders
Units: sectors of 1 * 512 = 512 bytes
Sector size (logical/physical): 512 bytes / 512 bytes
I/O size (minimum/optimal): 512 bytes / 512 bytes
Disklabel type: sun

Device   Start  End  Sectors   Size Id Type   Flags
root-disk.img1   0  2744549  2744550   1.3G  2 SunOS root  
root-disk.img2 2744550  3047894   303345 148.1M  3 SunOS swapu 
root-disk.img3   0 71127179 71127180  33.9G  5 Whole disk  
root-disk.img8 3047895 71127179 68079285  32.5G  8 SunOS home  

image: root-disk.img
file format: raw
virtual size: 36G (38654705664 bytes)
disk size: 1.2G

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

Re: [Qemu-devel] [PATCH] hw/arm/virt: Assume EL3 boot rom will handle PSCI if one is provided

[Peter Maydell]

On 19 February 2016 at 21:58, Laszlo Ersek  wrote:
> On 02/19/16 22:41, Ard Biesheuvel wrote:
>> On 19 February 2016 at 22:03, Laszlo Ersek  wrote:
>>> Ard, any opinion on this?
>>>
>>
>> I agree with Peter. Note that this is strictly about emulation, under
>> KVM we always run at EL1 or below and PSCI calls are handled by the
>> host kernel, not QEMU
>
> Great, that's all I wanted to hear -- out of scope for me. :)
>
> Actually, I have now read the patch even, and I have the following comments:
>
> - As long as "using_psci" is true, the behavior doesn't change. Great.
>
> - The only place where using_psci *changes* to false is reachable only
> with (vms->secure && firmware_loaded). That's what wasn't immediately
> obvious from the patch -- when vms->secure is true (-machine secure=on),
> it's out of scope for me. :)
>
> - However, I think I might have noticed a bug -- or rather missed
> something trivial --, namely, where is "using_psci" *initially* set to
> true? The "machines" static array is not touched.

Derp. I changed at the last minute from having using_psci be a
local bool in the machvirt_init() function to putting it in the
vbi struct, and forgot that when I deleted the "bool using_psci = true;"
I needed to add something to init vbi->using_psci to true.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 2/2] target-arm: Implement MDCR_EL3.TPM and MDCR_EL2.TPM traps

[Peter Maydell]

On 19 February 2016 at 19:38, Alistair Francis  wrote:
> On Fri, Feb 19, 2016 at 6:39 AM, Peter Maydell  
> wrote:
>> +/* Check for traps to performance monitor registers, which are controlled
>> + * by MDCR_EL2.TPM for EL2 and MDCR_EL3.TPM for EL3.
>> + */
>> +static CPAccessResult access_tpm(CPUARMState *env, const ARMCPRegInfo *ri,
>> + bool isread)
>> +{
>> +int el = arm_current_el(env);
>> +
>> +if (el < 2 && (env->cp15.mdcr_el2 & MDCR_TPM)
>> +&& !arm_is_secure_below_el3(env)) {
>> +return CP_ACCESS_TRAP_EL2;
>> +}
>> +if (el < 3 && (env->cp15.mdcr_el3 & MDCR_TPM)) {
>> +return CP_ACCESS_TRAP_EL3;
>> +}
>
> Hey Peter,
>
> Why not use else if?

I generally tend not to use else-if ladders if the thing
in the conditional returns unconditionally, just as a
personal style preference. "if () { X } else if () { Y } Z"
implies a possible control flow path of "take the if
branch so run X, then skip Y, and continue after to run Z",
and if X returns unconditionally that can't happen.
It also matches up with the usual approach of
   if (something) {
   early return;
   }
   main body of function;

which you wouldn't want to write as
   if (something) {
  early return;
   } else {
  main body;
   }

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 7/9] i.MX: Add i.MX6 SOC implementation.

[Jean-Christophe DUBOIS]

Just to compare I tried to run Linux on QEMU emulating highbank.

For now I am unable to start in SMP mode. Only one core is activated.

And there is linux backtrace in L2C-310 init.

My command line:

# qemu-system-arm -smp 4 -M highbank -m 1024M -display none -no-reboot 
-kernel zImage -initrd rootfs.cpio.gz -dtb highbank.dtb -serial stdio


The kernel output:

   [0.00] Booting Linux on physical CPU 0x0
   [0.00] Linux version 4.5.0-rc1-00028-g03c21cb-dirty
   (jcd@jcd-U31SG) (gcc version 4.6.3 (GCC) ) #20 SMP Sat Feb 20
   11:27:10 CET 2016
   [0.00] CPU: ARMv7 Processor [410fc090] revision 0 (ARMv7),
   cr=10c5387d
   [0.00] CPU: PIPT / VIPT nonaliasing data cache, VIPT
   nonaliasing instruction cache
   [0.00] Machine model: Calxeda Highbank
   [0.00] cma: Reserved 64 MiB at 0x3c00
   [0.00] Memory policy: Data cache writealloc
   [0.00] DT missing boot CPU MPIDR[23:0], fall back to default
   cpu_logical_map
   [0.00] psci: probing for conduit method from DT.
   [0.00] psci: Using PSCI v0.1 Function IDs from DT
   [0.00] PERCPU: Embedded 12 pages/cpu @ef7e3000 s18880 r8192
   d22080 u49152
   [0.00] Built 1 zonelists in Zone order, mobility grouping
   on.  Total pages: 260608
   [0.00] Kernel command line: console=ttyAMA0
   [0.00] PID hash table entries: 4096 (order: 2, 16384 bytes)
   [0.00] Dentry cache hash table entries: 131072 (order: 7,
   524288 bytes)
   [0.00] Inode-cache hash table entries: 65536 (order: 6,
   262144 bytes)
   [0.00] Memory: 958304K/1048576K available (8557K kernel
   code, 1112K rwdata, 4036K rodata, 1104K init, 345K bss, 24736K
   reserved, 65536K cma-reserved, 196608K highmem)
   [0.00] Virtual kernel memory layout:
   [0.00] vector  : 0x - 0x1000   (   4 kB)
   [0.00] fixmap  : 0xffc0 - 0xfff0   (3072 kB)
   [0.00] vmalloc : 0xf080 - 0xff80   ( 240 MB)
   [0.00] lowmem  : 0xc000 - 0xf000   ( 768 MB)
   [0.00] pkmap   : 0xbfe0 - 0xc000   (   2 MB)
   [0.00] modules : 0xbf00 - 0xbfe0   (  14 MB)
   [0.00]   .text : 0xc0208000 - 0xc0e554cc   (12598 kB)
   [0.00]   .init : 0xc0e56000 - 0xc0f6a000   (1104 kB)
   [0.00]   .data : 0xc0f6a000 - 0xc10803c0   (1113 kB)
   [0.00].bss : 0xc1083000 - 0xc10d9598   ( 346 kB)
   [0.00] SLUB: HWalign=64, Order=0-3, MinObjects=0, CPUs=1,
   Nodes=1
   [0.00] Hierarchical RCU implementation.
   [0.00] Build-time adjustment of leaf fanout to 32.
   [0.00] RCU restricting CPUs from NR_CPUS=16 to nr_cpu_ids=1.
   [0.00] RCU: Adjusting geometry for rcu_fanout_leaf=32,
   nr_cpu_ids=1
   [0.00] NR_IRQS:16 nr_irqs:16 16
   [0.00] L2C-310 erratum 769419 enabled
   [0.00] L2C-310 enabling early BRESP for Cortex-A9
   [0.00] [ cut here ]
   [0.00] WARNING: CPU: 0 PID: 0 at
   arch/arm/mach-highbank/highbank.c:60
   highbank_l2c310_write_sec+0x34/0x58()
   [0.00] Highbank L2C310: ignoring write to reg 0x104
   [0.00] Modules linked in:
   [0.00] CPU: 0 PID: 0 Comm: swapper/0 Not tainted
   4.5.0-rc1-00028-g03c21cb-dirty #20
   [0.00] Hardware name: Highbank
   [0.00] [] (unwind_backtrace) from []
   (show_stack+0x10/0x14)
   [0.00] [] (show_stack) from []
   (dump_stack+0x70/0x8c)
   [0.00] [] (dump_stack) from []
   (warn_slowpath_common+0x74/0xac)
   [0.00] [] (warn_slowpath_common) from []
   (warn_slowpath_fmt+0x30/0x40)
   [0.00] [] (warn_slowpath_fmt) from []
   (highbank_l2c310_write_sec+0x34/0x58)
   [0.00] [] (highbank_l2c310_write_sec) from
   [] (l2c_configure+0x34/0x48)
   [0.00] [] (l2c_configure) from []
   (l2c310_configure+0xc/0x16c)
   [0.00] [] (l2c310_configure) from []
   (l2c_enable+0xe8/0xf8)
   [0.00] [] (l2c_enable) from []
   (l2c310_enable+0x140/0x21c)
   [0.00] [] (l2c310_enable) from []
   (__l2c_init.part.6+0x19c/0x234)
   [0.00] [] (__l2c_init.part.6) from []
   (l2x0_of_init+0x208/0x254)
   [0.00] [] (l2x0_of_init) from []
   (init_IRQ+0x5c/0x80)
   [0.00] [] (init_IRQ) from []
   (start_kernel+0x244/0x3dc)
   [0.00] [] (start_kernel) from [<0020807c>] (0x20807c)
   [0.00] ---[ end trace cb88537fdc8fa200 ]---
   [0.00] L2C-310 dynamic clock gating disabled, standby mode
   disabled
   [0.00] L2C-310 cache controller enabled, 8 ways, 128 kB
   [0.00] L2C-310: CACHE_ID 0x41c8, AUX_CTRL 0x0202
   [0.00] clocksource: arm,sp804: mask: 0x max_cycles:
   0x, max_idle_ns: 12741736309 ns
   [0.000184] sched_clock: 32 bits at 150MHz, resolution 6ns, wraps
   every 14316557820ns
   [

Re: [Qemu-devel] [PATCH 1/1] arm: virt: change GPIO trigger interrupt to pulse

[Shannon Zhao]

Hi Wei,

On 2016/2/10 6:59, Wei Huang wrote:
> 
> On 02/04/2016 12:51 AM, Shannon Zhao wrote:
>>
>>
>> On 2016/2/4 14:10, Wei Huang wrote:
>>>
>>> On 02/03/2016 07:44 PM, Shannon Zhao wrote:
> 
> 
> 
>>> I reversed the order of edge pulling. The state is 1 according to printk
>>> inside gpio_keys driver. However the reboot still failed with two
>>> reboots (1 very early, 1 later).
>>>
>> Because to make the input work, it should call input_event twice I think.
>>
>> input_event(input, type, button->code, 1) means the button pressed
>> input_event(input, type, button->code, 0) means the button released
>>
>> But We only see guest entering gpio_keys_gpio_report_event once.
>>
>> My original purpose is like below:
>>
>> call qemu_set_irq(qdev_get_gpio_in(pl061_dev, 3), 1) to make guest
>> execute input_event(input, type, button->code, 1)
>> call qemu_set_irq(qdev_get_gpio_in(pl061_dev, 3), 0) to make guest
>> execute input_event(input, type, button->code, 0).
>>
>> But even though it calls qemu_set_irq twice, it only calls pl061_update
>> once in qemu.
> 
> Hi Shannon,
> 
> Assuming that we are talking about the special case you found (i.e. send
> reboot very early and then send another one after guest VM fully
> booted). Dug into the code further, here are my findings:
> 
> === Why ACPI case failed? ===
> QEMU pl061.c checks the current request against the new request (see
> s->old_in_data ^ s->data in pl061.c file). If no change, nothing will
> happen.
> 
> So two consecutive reboots will cause the following state change;
> apparently the second request won't trigger VM reboot because
> pl01_update() decides _not_ to inject IRQ into guest VM.
> 
>   (PL061State fields)   data   old_in_data   istate
> VM boot 0  0 0
> 1st ACPI reboot request 8  0 0
> After VM PL061 driver ACK   8  8 0
> 2nd ACPI reboot request 8 no-change, no IRQ <==
> 
> To solve this problem, we have to invert PL061State->data at least once
> to trigger IRQ inside VM. Two solutions:
> 
> S1: "Pulse"
> static void virt_powerdown_req(Notifier *n, void *opaque)
> {
> qemu_set_irq(qdev_get_gpio_in(pl061_dev, 3), 0);
> qemu_set_irq(qdev_get_gpio_in(pl061_dev, 3), 1);
> }
> 
> S2: "Invert"
> static int old_gpio_level = 0;
> static void virt_powerdown_req(Notifier *n, void *opaque)
> {
> /* use gpio Pin 3 for power button event */
> qemu_set_irq(qdev_get_gpio_in(pl061_dev, 3), !old_gpio_level);
> old_gpio_level = !old_gpio_level;
> }
> 
The S2 still doesn't work. After sending the early first reboot, whne
guest boots well, it doesn't react to the second reboot while it reacts
to the third one.

> Both S1 and S2 works for ACPI. But S1 has problem with DT, which is
> explained below.
> 
> === Why DT fails with S1 ===
> DT approach requires both PL061 and gpio-keys to work together. Looking
> into guest kernel gpio-keys/input code, you will find that it only
> reacts to both LEVEL-HI and input changes. Here is the code snip from
> drivers/input/input.c file:
> 
> /* auto-repeat bypasses state updates */
> if (value == 2) {
> disposition = INPUT_PASS_TO_HANDLERS;
> break;
> }
> 
> if (!!test_bit(code, dev->key) != !!value) {
> __change_bit(code, dev->key);
> disposition = INPUT_PASS_TO_HANDLERS;
> }
> 
> Unless we adds gpio-keys DT property to "autorepeat", the
> "!!test_bit(code, dev->key) != !!value" is always FALSE with S1. Thus
> systemd won't receive any input event; and no power-switch will be
> triggered.
> 
> In comparison S2 will work because value is changed very time.
> 
> === Summary ===
> 1. Cleaning PL061 state is required. That means "[PATCH V2 1/2] ARM:
> PL061: Clear PL061 device state after reset" is necessary.
> 2. S2 is better. To support S2, we needs to change GPIO IRQ polarity to
> AML_ACTIVE_BOTH in ACPI description.
> 
Thanks. But we don't have the AML_ACTIVE_BOTH since there is only one
bit for "Interrupt Polarity" in ACPI table.
See ACPI SPEC 6.0 "Table 6-216 Extended Interrupt Descriptor Definition"
Bit [2] Interrupt Polarity, _LL
0 Active-High: This interrupt is sampled when the signal is high,
or true.
1 Active-Low: This interrupt is sampled when the signal is low, or
false.

> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -326,7 +326,7 @@ static void acpi_dsdt_add_gpio(Aml *scope, const 
> MemMapEntry *gpio_memmap,
>  Aml *aei = aml_resource_template();
>  /* Pin 3 for power button */
>  const uint32_t pin_list[1] = {3};
> -aml_append(aei, aml_gpio_int(AML_CONSUMER, AML_EDGE, AML_ACTIVE_HIGH,
> +aml_append(aei, aml_gpio_int(AML_CONSUMER, AML_EDGE, 3,
What's the meaning of 3 here?

>   AML_EXCLUSIVE, AML_PULL_UP, 0, pin_list, 1,
>   "GPO0", NULL, 0));
>  aml_append(dev, aml_name_decl("_AEI", aei));

Thanks,
-- 
Shannon

[Qemu-devel] kernel 4.4.2: kvm_irq_delivery_to_api / rwsem_down_read_failed

[Stefan Priebe]

Hi,

while testing Kernel 4.4.2 and starting 20 Qemu 2.4.1 virtual machines. 
I got those traces and a load of 500 on those system. I was only abler 
to recover by sysrq-trigger.


All traces:

INFO: task pvedaemon worke:7470 blocked for more than 120 seconds.
Not tainted 4.4.2+1-ph #1
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
pvedaemon worke D 88239c367ca0 0 7470 7468 0x0008
88239c367ca0 8840a6232500 8823ed83a500 88239c367c90
88239c368000 8845f5f070e8 8845f5f07100 
7ffc73b48e58 88239c367cc0 b66a4d89 88239c367cf0
Call Trace:
[] schedule+0x39/0x80
[] rwsem_down_read_failed+0xc7/0x120
[] call_rwsem_down_read_failed+0x14/0x30
[] ? down_read+0x17/0x20
[] __access_remote_vm+0x3e/0x1c0
[] ? call_rwsem_down_read_failed+0x14/0x30
[] access_remote_vm+0x1f/0x30
[] proc_pid_cmdline_read+0x16e/0x4f0
[] ? acct_account_cputime+0x1c/0x20
[] __vfs_read+0x18/0x40
[] vfs_read+0x8e/0x140
[] SyS_read+0x4f/0xa0
[] entry_SYSCALL_64_fastpath+0x12/0x71
INFO: task pvestatd:7633 blocked for more than 120 seconds.
Not tainted 4.4.2+1-ph #1
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
pvestatd D 88239f16fd40 0 7633 1 0x0008
88239f16fd40 8824e76a8000 8823e5fc2500 8823e5fc2500
88239f17 8845f5f070e8 8845f5f07100 8845f5f07080
0341bf10 88239f16fd60 b66a4d89 024000d00058
Call Trace:
[] schedule+0x39/0x80
[] rwsem_down_read_failed+0xc7/0x120
[] call_rwsem_down_read_failed+0x14/0x30
[] ? down_read+0x17/0x20
[] proc_pid_cmdline_read+0xac/0x4f0
[] ? acct_account_cputime+0x1c/0x20
[] ? account_user_time+0x73/0x80
[] ? vtime_account_user+0x4e/0x70
[] __vfs_read+0x18/0x40
[] vfs_read+0x8e/0x140
[] SyS_read+0x4f/0xa0
[] entry_SYSCALL_64_fastpath+0x12/0x71
INFO: task kvm:11766 blocked for more than 120 seconds.
Not tainted 4.4.2+1-ph #1
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
kvm D 88452a2d3dd0 0 11766 1 0x0008
88452a2d3dd0 880166c74a00 8845b7354a00 b617fc8e
88452a2d4000 8845f5f070e8 8845f5f07100 88452a2d3f58
8845b7354a00 88452a2d3df0 b66a4d89 7fa807abbf80
Call Trace:
[] ? __handle_mm_fault+0xd1e/0x1260
[] schedule+0x39/0x80
[] rwsem_down_read_failed+0xc7/0x120
[] call_rwsem_down_read_failed+0x14/0x30
[] ? down_read+0x17/0x20
[] __do_page_fault+0x2b7/0x380
[] ? account_user_time+0x73/0x80
[] ? vtime_account_user+0x4e/0x70
[] do_page_fault+0x37/0x90
[] page_fault+0x28/0x30
INFO: task kvm:11824 blocked for more than 120 seconds.
Not tainted 4.4.2+1-ph #1
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
kvm D 8840a867faa0 0 11824 1 0x0008
8840a867faa0 8845866a4a00 8840a6232500 0001
8840a868 8845f5f070e8 8845f5f07100 8840a867fc0e
 8840a867fac0 b66a4d89 c0606a06
Call Trace:
[] schedule+0x39/0x80
[] ? kvm_irq_delivery_to_apic+0x56/0x220 [kvm]
[] rwsem_down_read_failed+0xc7/0x120
[] call_rwsem_down_read_failed+0x14/0x30
[] ? down_read+0x17/0x20
[] kvm_host_page_size+0x60/0xa0 [kvm]
[] mapping_level+0x5c/0x130 [kvm]
[] tdp_page_fault+0x9b/0x260 [kvm]
[] ? kernel_pio+0x2d/0x40 [kvm]
[] kvm_mmu_page_fault+0x31/0x120 [kvm]
[] handle_ept_violation+0xa4/0x170 [kvm_intel]
[] vmx_handle_exit+0x257/0x490 [kvm_intel]
[] ? __vtime_account_system+0x31/0x40
[] vcpu_enter_guest+0x6af/0xff0 [kvm]
[] ? kvm_apic_local_deliver+0x5d/0x60 [kvm]
[] kvm_arch_vcpu_ioctl_run+0xc4/0x3c0 [kvm]
[] kvm_vcpu_ioctl+0x324/0x5d0 [kvm]
[] ? acct_account_cputime+0x1c/0x20
[] ? account_user_time+0x73/0x80
[] do_vfs_ioctl+0x83/0x4e0
[] ? enter_from_user_mode+0x1f/0x50
[] ? syscall_trace_enter_phase1+0xc1/0x110
[] SyS_ioctl+0x4c/0x80
[] entry_SYSCALL_64_fastpath+0x12/0x71
INFO: task kvm:11825 blocked for more than 120 seconds.
Not tainted 4.4.2+1-ph #1
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
kvm D 88458d6a3aa0 0 11825 1 0x00080002
88458d6a3aa0 880167302500 8840a6234a00 0001
88458d6a4000 8845f5f070e8 8845f5f07100 88458d6a3c0e
 88458d6a3ac0 b66a4d89 c0606a06
Call Trace:
[] schedule+0x39/0x80
[] ? kvm_irq_delivery_to_apic+0x56/0x220 [kvm]
[] rwsem_down_read_failed+0xc7/0x120
[] call_rwsem_down_read_failed+0x14/0x30
[] ? down_read+0x17/0x20
[] kvm_host_page_size+0x60/0xa0 [kvm]
[] mapping_level+0x5c/0x130 [kvm]
[] tdp_page_fault+0x9b/0x260 [kvm]
[] kvm_mmu_page_fault+0x31/0x120 [kvm]
[] handle_ept_violation+0xa4/0x170 [kvm_intel]
[] vmx_handle_exit+0x257/0x490 [kvm_intel]
[] ? __vtime_account_system+0x31/0x40
[] vcpu_enter_guest+0x6af/0xff0 [kvm]
[] ? kvm_apic_local_deliver+0x5d/0x60 [kvm]
[] kvm_arch_vcpu_ioctl_run+0xc4/0x3c0 [kvm]
[] kvm_vcpu_ioctl+0x324/0x5d0 [kvm]
[] ? acct_account_cputime+0x1c/0x20
[] ? account_user_time+0x73/0x80
[] do_vfs_ioctl+0x83/0x4e0
[] ? 

Re: [Qemu-devel] [PATCH 0/3] memory: an optimization

[Gonglei (Arei)]

Hi Paolo,


> -Original Message-
> From: Paolo Bonzini [mailto:paolo.bonz...@gmail.com] On Behalf Of Paolo
> Bonzini
> Sent: Saturday, February 20, 2016 5:48 PM
> To: Gonglei (Arei); qemu-devel@nongnu.org
> Cc: Huangpeng (Peter)
> Subject: Re: [PATCH 0/3] memory: an optimization
> 
> 
> 
> On 20/02/2016 03:35, Gonglei wrote:
> > Perf top tells me qemu_get_ram_ptr consume too much cpu cycles.
> >> 22.56%  qemu-kvm [.] address_space_translate
> >>  13.29%  qemu-kvm [.] qemu_get_ram_ptr
> >>   4.71%  qemu-kvm [.] phys_page_find
> >>   4.43%  qemu-kvm [.]
> address_space_translate_internal
> >>   3.47%  libpthread-2.19.so   [.] __pthread_mutex_unlock_usercnt
> >>   3.08%  qemu-kvm [.] qemu_ram_addr_from_host
> >>   2.62%  qemu-kvm [.] address_space_map
> >>   2.61%  libc-2.19.so [.] _int_malloc
> >>   2.58%  libc-2.19.so [.] _int_free
> >>   2.38%  libc-2.19.so [.] malloc
> >>   2.06%  libpthread-2.19.so   [.] pthread_mutex_lock
> >>   1.68%  libc-2.19.so [.] malloc_consolidate
> >>   1.35%  libc-2.19.so [.] __memcpy_sse2_unaligned
> >>   1.23%  qemu-kvm [.] lduw_le_phys
> >>   1.18%  qemu-kvm [.] find_next_zero_bit
> >>   1.02%  qemu-kvm [.] object_unref
> >
> > And Paolo suggested that we can get rid of qemu_get_ram_ptr
> > by storing the RAMBlock pointer into the memory region,
> > instead of the ram_addr_t value. And after appling this change,
> > I got much better performance indeed.
> 
> What's the gain like?
> 
After rebased on the master branch right now, I found that the qemu_get_ram_ptr 
is
not one of main consumers. But I also get some bonus from this patch set.

Before this optimization:
  1.26%  qemu-kvm  [.] qemu_get_ram_ptr
  0.89%  qemu-kvm  [.] qemu_get_ram_block

Applied the patch set:
 0.87%  qemu-kvm [.] qemu_get_ram_ptr

Now the main consumers are (too much different with qemu-2.3):
 6.38%  libpthread-2.19.so   [.] __pthread_mutex_unlock_usercnt
  6.02%  qemu-kvm [.] vring_desc_read.isra.26
  5.27%  qemu-kvm [.] address_space_map
  4.45%  qemu-kvm [.] qemu_ram_block_from_host
  4.13%  libpthread-2.19.so   [.] pthread_mutex_lock
  3.95%  libc-2.19.so [.] _int_free
  3.46%  qemu-kvm [.] address_space_translate_internal
  3.40%  qemu-kvm [.] address_space_translate
  3.39%  qemu-kvm [.] phys_page_find
  3.37%  libc-2.19.so [.] _int_malloc
  3.21%  qemu-kvm [.] stw_le_phys
  2.70%  libc-2.19.so [.] malloc
  2.18%  qemu-kvm [.] lduw_le_phys
  2.15%  libc-2.19.so [.] __memcpy_sse2_unaligned
  1.58%  qemu-kvm [.] address_space_write
  1.48%  libc-2.19.so [.] memset
  1.22%  qemu-kvm [.] virtqueue_map_desc
  1.22%  libc-2.19.so [.] __libc_calloc
  1.21%  qemu-kvm [.] virtio_notify

And the speed based on the master branch and my patch series:
 Testing AES-128-CBC cipher: 
Encrypting in chunks of 256 bytes: done. 506.27 MiB in 5.01 secs: 
100.97 MiB/sec (2073684 packets)
Encrypting in chunks of 256 bytes: done. 505.89 MiB in 5.02 secs: 
100.85 MiB/sec (2072106 packets)
Encrypting in chunks of 256 bytes: done. 505.94 MiB in 5.02 secs: 
100.86 MiB/sec (2072343 packets)
Encrypting in chunks of 256 bytes: done. 505.96 MiB in 5.02 secs: 
100.87 MiB/sec (2072412 packets)
Encrypting in chunks of 256 bytes: done. 505.92 MiB in 5.02 secs: 
100.86 MiB/sec (2072241 packets)
Encrypting in chunks of 256 bytes: done. 506.36 MiB in 5.02 secs: 
100.95 MiB/sec (2074057 packets)
Encrypting in chunks of 256 bytes: done. 506.35 MiB in 5.01 secs: 
101.02 MiB/sec (2073998 packets)
Encrypting in chunks of 256 bytes: done. 505.41 MiB in 5.01 secs: 
100.92 MiB/sec (2070157 packets)

> I've not reviewed the patch in depth, but what I can say is that I like
> it a lot.  It only does the bare minimum needed to provide the
> optimization, but this also makes it very simple to understand.  More
> cleanups and further optimizations are possible (including removing
> mr->ram_addr completely), but your patches really does one thing and
> does it well.  Good job!
> 
Thanks!

Regards,
-Gonglei

Re: [Qemu-devel] [PATCH 00/13] IOMMU: Enable interrupt remapping for Intel IOMMU

[Jan Kiszka]

On 2016-02-19 10:29, Peter Xu wrote:
>> Would be helpful for more convenient import and experiments. E.g. I
>> could quickly through my test setup at them and tell you if they work
>> for it.
> 
> I have put codes onto github for better reference:
> 
> https://github.com/xzpeter/qemu/tree/vt-d-intr

Just a quick feedback: Interrupts of the root cell (the Linux that hands
over the control to the hypervisor) get stuck when starting Jailhouse.

What basically happens there is that Linux boots up with DMAR off but IR
on, then Jailhouse takes over, turning IR off for a moment, reconfigures
things so that it gains control, and then reenables IR (and also turns
on DMAR). After that, interrupts no longer arrive at that guest, which
is now L2.

Jan




signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 0/3] memory: an optimization

[Paolo Bonzini]

On 20/02/2016 03:35, Gonglei wrote:
> Perf top tells me qemu_get_ram_ptr consume too much cpu cycles.
>> 22.56%  qemu-kvm [.] address_space_translate
>>  13.29%  qemu-kvm [.] qemu_get_ram_ptr
>>   4.71%  qemu-kvm [.] phys_page_find
>>   4.43%  qemu-kvm [.] address_space_translate_internal
>>   3.47%  libpthread-2.19.so   [.] __pthread_mutex_unlock_usercnt
>>   3.08%  qemu-kvm [.] qemu_ram_addr_from_host
>>   2.62%  qemu-kvm [.] address_space_map
>>   2.61%  libc-2.19.so [.] _int_malloc
>>   2.58%  libc-2.19.so [.] _int_free
>>   2.38%  libc-2.19.so [.] malloc
>>   2.06%  libpthread-2.19.so   [.] pthread_mutex_lock
>>   1.68%  libc-2.19.so [.] malloc_consolidate
>>   1.35%  libc-2.19.so [.] __memcpy_sse2_unaligned
>>   1.23%  qemu-kvm [.] lduw_le_phys
>>   1.18%  qemu-kvm [.] find_next_zero_bit
>>   1.02%  qemu-kvm [.] object_unref
> 
> And Paolo suggested that we can get rid of qemu_get_ram_ptr
> by storing the RAMBlock pointer into the memory region,
> instead of the ram_addr_t value. And after appling this change,
> I got much better performance indeed.

What's the gain like?

I've not reviewed the patch in depth, but what I can say is that I like
it a lot.  It only does the bare minimum needed to provide the
optimization, but this also makes it very simple to understand.  More
cleanups and further optimizations are possible (including removing
mr->ram_addr completely), but your patches really does one thing and
does it well.  Good job!

Paolo

Re: [Qemu-devel] kvm: "warning: host doesn't support requested feature: CPUID.01H:ECX.x2apic [bit 21]"

[Paolo Bonzini]

- Original Message -
> From: "Jan Kiszka" 
> To: "Eduardo Habkost" , "Paolo Bonzini" 
> 
> Cc: "qemu-devel" , "kvm" 
> Sent: Saturday, February 20, 2016 9:09:32 AM
> Subject: kvm: "warning: host doesn't support requested feature: 
> CPUID.01H:ECX.x2apic [bit 21]"
> 
> Hi all,
> 
> I suppose 5120901a37 introduced this: qemu with kernel_irqchip=off now
> generates these warnings, one per VCPU, during QEMU startup. Is the plan
> to live with them until we finally have x2APIC emulation in userspace
> (ie. also MSR vmexiting to there), or should we otherwise avoid it?

I think it's a bug, x2apic should be auto-suppressed with kernel_irqchip=off.

Paolo

[Qemu-devel] kvm: "warning: host doesn't support requested feature: CPUID.01H:ECX.x2apic [bit 21]"

[Jan Kiszka]

Hi all,

I suppose 5120901a37 introduced this: qemu with kernel_irqchip=off now
generates these warnings, one per VCPU, during QEMU startup. Is the plan
to live with them until we finally have x2APIC emulation in userspace
(ie. also MSR vmexiting to there), or should we otherwise avoid it?

Thanks,
Jan