Re: [Qemu-devel] [PATCH v3 00/16] Virtio devices split from virtio-pci

[Gonglei (Arei)]

> -Original Message-
> From: Michael S. Tsirkin [mailto:m...@redhat.com]
> Sent: Friday, December 14, 2018 8:53 PM
> To: Gonglei (Arei) 
> Cc: Juan Quintela ; qemu-devel@nongnu.org; Thomas
> Huth ; Gerd Hoffmann 
> Subject: Re: [PATCH v3 00/16] Virtio devices split from virtio-pci
> 
> On Fri, Dec 14, 2018 at 07:07:44AM +, Gonglei (Arei) wrote:
> >
> > > -Original Message-
> > > From: Juan Quintela [mailto:quint...@redhat.com]
> > > Sent: Friday, December 14, 2018 5:01 AM
> > > To: qemu-devel@nongnu.org
> > > Cc: Michael S. Tsirkin ; Thomas Huth
> ;
> > > Gerd Hoffmann ; Gonglei (Arei)
> > > ; Juan Quintela 
> > > Subject: [PATCH v3 00/16] Virtio devices split from virtio-pci
> > >
> > > Hi
> > >
> > > v3:
> > > - rebase to master
> > > - only compile them if CONFIG_PCI is set (thomas)
> > >
> > > Please review.
> > >
> > > Later, Juan.
> > >
> > > V2:
> > >
> > > - Rebase on top of master
> > >
> > > Please review.
> > >
> > > Later, Juan.
> > >
> > > [v1]
> > > From previous verision (in the middle of make check tests):
> > > - split also the bits of virtio-pci.h (mst suggestion)
> > > - add gpu, crypt and gpg bits
> > > - more cleanups
> > > - fix all the copyrights (the ones not changed have been there
> > >   foverever)
> > > - be consistent with naming, vhost-* or virtio-*
> > >
> > > Please review, Juan.
> > >
> > > Juan Quintela (16):
> > >   virtio: split vhost vsock bits from virtio-pci
> > >   virtio: split virtio input host bits from virtio-pci
> > >   virtio: split virtio input bits from virtio-pci
> > >   virtio: split virtio rng bits from virtio-pci
> > >   virtio: split virtio balloon bits from virtio-pci
> > >   virtio: split virtio 9p bits from virtio-pci
> > >   virtio: split vhost user blk bits from virtio-pci
> > >   virtio: split vhost user scsi bits from virtio-pci
> > >   virtio: split vhost scsi bits from virtio-pci
> > >   virtio: split virtio scsi bits from virtio-pci
> > >   virtio: split virtio blk bits rom virtio-pci
> > >   virtio: split virtio net bits rom virtio-pci
> > >   virtio: split virtio serial bits rom virtio-pci
> > >   virtio: split virtio gpu bits rom virtio-pci.h
> > >   virtio: split virtio crypto bits rom virtio-pci.h
> > >   virtio: virtio 9p really requires CONFIG_VIRTFS to work
> > >
> > >  default-configs/virtio.mak|   3 +-
> > >  hw/display/virtio-gpu-pci.c   |  14 +
> > >  hw/display/virtio-vga.c   |   1 +
> > >  hw/virtio/Makefile.objs   |  15 +
> > >  hw/virtio/vhost-scsi-pci.c|  95 
> > >  hw/virtio/vhost-user-blk-pci.c| 101 
> > >  hw/virtio/vhost-user-scsi-pci.c   | 101 
> > >  hw/virtio/vhost-vsock-pci.c   |  82 
> > >  hw/virtio/virtio-9p-pci.c |  86 
> > >  hw/virtio/virtio-balloon-pci.c|  94 
> > >  hw/virtio/virtio-blk-pci.c|  97 
> > >  hw/virtio/virtio-crypto-pci.c |  14 +
> > >  hw/virtio/virtio-input-host-pci.c |  45 ++
> > >  hw/virtio/virtio-input-pci.c  | 154 ++
> > >  hw/virtio/virtio-net-pci.c|  96 
> > >  hw/virtio/virtio-pci.c| 783 --
> > >  hw/virtio/virtio-pci.h| 234 -
> > >  hw/virtio/virtio-rng-pci.c|  86 
> > >  hw/virtio/virtio-scsi-pci.c   | 106 
> > >  hw/virtio/virtio-serial-pci.c | 112 +
> > >  tests/Makefile.include|  20 +-
> > >  21 files changed, 1311 insertions(+), 1028 deletions(-)
> > >  create mode 100644 hw/virtio/vhost-scsi-pci.c
> > >  create mode 100644 hw/virtio/vhost-user-blk-pci.c
> > >  create mode 100644 hw/virtio/vhost-user-scsi-pci.c
> > >  create mode 100644 hw/virtio/vhost-vsock-pci.c
> > >  create mode 100644 hw/virtio/virtio-9p-pci.c
> > >  create mode 100644 hw/virtio/virtio-balloon-pci.c
> > >  create mode 100644 hw/virtio/virtio-blk-pci.c
> > >  create mode 100644 hw/virtio/virtio-input-host-pci.c
> > >  create mode 100644 hw/virtio/virtio-input-pci.c
> > >  create mode 100644 hw/virtio/virtio-net-pci.c
> > >  create mode 100644 hw/virtio/virtio-rng-pci.c
> > >  create mode 100644 hw/virtio/virtio-scsi-pci.c
> > >  create mode 100644 hw/virtio/virtio-serial-pci.c
> > >
> > > --
> > > 2.19.2
> >
> > For series:
> > Reviewed-by: Gonglei 
> >
> >
> > Thanks,
> > -Gonglei
> 
> Thanks!
> Can you pls align Reviewed-by: tag at the 1st column in the future?
> Makes it easier to apply the tag.

OK, I will, thanks :)

Thanks,
-Gonglei

Re: [Qemu-devel] [PATCH v5 07/73] cpu: make per-CPU locks an alias of the BQL in TCG rr mode

[Emilio G. Cota]

On Thu, Dec 13, 2018 at 00:03:47 -0500, Emilio G. Cota wrote:
(snip)
> diff --git a/qom/cpu.c b/qom/cpu.c
> index aa15ea4af5..2ea5b1da08 100644
> --- a/qom/cpu.c
> +++ b/qom/cpu.c
> @@ -371,7 +371,6 @@ static void cpu_common_initfn(Object *obj)
>  cpu->nr_cores = 1;
>  cpu->nr_threads = 1;
>  
> -qemu_mutex_init(>lock);
>  qemu_cond_init(>cond);
>  QSIMPLEQ_INIT(>work_list);
>  QTAILQ_INIT(>breakpoints);

*ouch* this breaks user-mode, since we end up with cpu->lock == NULL.
I'm surprised that make check-qtest didn't pick this up--guess it's
all system-mode tests.

I've fixed this commit on github's v5 branch with the appended.

Thanks,

Emilio
---
diff --git a/cpus.c b/cpus.c
index d8261903ac..9c6cd9b90f 100644
--- a/cpus.c
+++ b/cpus.c
@@ -2326,10 +2326,9 @@ void qemu_init_vcpu(CPUState *cpu)
  * cpu->lock is a standalone per-CPU lock.
  */
 if (qemu_is_tcg_rr()) {
+qemu_mutex_destroy(cpu->lock);
+g_free(cpu->lock);
 cpu->lock = _global_mutex;
-} else {
-cpu->lock = g_malloc(sizeof(*cpu->lock));
-qemu_mutex_init(cpu->lock);
 }
 
 if (kvm_enabled()) {
diff --git a/qom/cpu.c b/qom/cpu.c
index 386b1e29dd..c4cb626393 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -367,6 +367,8 @@ static void cpu_common_initfn(Object *obj)
 cpu->nr_cores = 1;
 cpu->nr_threads = 1;
 
+cpu->lock = g_new(QemuMutex, 1);
+qemu_mutex_init(cpu->lock);
 qemu_cond_init(>cond);
 QSIMPLEQ_INIT(>work_list);
 QTAILQ_INIT(>breakpoints);

[Qemu-devel] [PATCH 0/3] vhost-user-test fix

[Li Qiang]

Currently, the vhost-user-test is not correct.
When in qtest mode, the accel is qtest, not kvm.
So when the client side of vhost-user-test send
'VHOST_USER_SET_VRING_CALL' msg, the 'fd' will
no be added in 'fds' in 'vhost_set_vring_file'.
In 'chr_read' of the server side in the 
vhost-user-test, it calls 'qemu_chr_fe_get_msgfds'
to get the fd in 'VHOST_USER_SET_VRING_CALL'. Though
there is no fd returned, but as the 'fd' is not initialized
so 'fd' maybe valid, and 'qemu_set_nonblock' will be success.
Even worse, 'qemu_set_nonblock' doesn't check the return value
of fcntl.

So this cause the interesting bug here: there are three issues,
but they combined and will bypass the qtest.

This patchset tries to address these issue.

Li Qiang (3):
  tests: vhost-user-test: initialize 'fd' in chr_read
  vhost-user: add fds inf 'vhost_set_vring_file' in qtest
  util: check the return value of fcntl in qemu_set_{block, nonblock}

 hw/virtio/vhost-user.c  | 3 ++-
 tests/vhost-user-test.c | 2 +-
 util/oslib-posix.c  | 8 ++--
 3 files changed, 9 insertions(+), 4 deletions(-)

-- 
2.17.1

[Qemu-devel] [PATCH 1/3] tests: vhost-user-test: initialize 'fd' in chr_read

[Li Qiang]

Currentyly when processing VHOST_USER_SET_VRING_CALL
if 'qemu_chr_fe_get_msgfds' get no fd, the 'fd' will
be a stack uninitialized value.

Signed-off-by: Li Qiang 
---
 tests/vhost-user-test.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/vhost-user-test.c b/tests/vhost-user-test.c
index 45d58d8ea2..86039e61e0 100644
--- a/tests/vhost-user-test.c
+++ b/tests/vhost-user-test.c
@@ -309,7 +309,7 @@ static void chr_read(void *opaque, const uint8_t *buf, int 
size)
 CharBackend *chr = >chr;
 VhostUserMsg msg;
 uint8_t *p = (uint8_t *) 
-int fd;
+int fd = -1;
 
 if (s->test_fail) {
 qemu_chr_fe_disconnect(chr);
-- 
2.17.1

[Qemu-devel] [PATCH 3/3] util: check the return value of fcntl in qemu_set_{block, nonblock}

[Li Qiang]

Assert that the return value is not an error. This is like commit
7e6478e7d4f for qemu_set_cloexec.

Signed-off-by: Li Qiang 
---
 util/oslib-posix.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index c1bee2a581..4ce1ba9ca4 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -233,14 +233,18 @@ void qemu_set_block(int fd)
 {
 int f;
 f = fcntl(fd, F_GETFL);
-fcntl(fd, F_SETFL, f & ~O_NONBLOCK);
+assert(f != -1);
+f = fcntl(fd, F_SETFL, f & ~O_NONBLOCK);
+assert(f != -1);
 }
 
 void qemu_set_nonblock(int fd)
 {
 int f;
 f = fcntl(fd, F_GETFL);
-fcntl(fd, F_SETFL, f | O_NONBLOCK);
+assert(f != -1);
+f = fcntl(fd, F_SETFL, f | O_NONBLOCK);
+assert(f != -1);
 }
 
 int socket_set_fast_reuse(int fd)
-- 
2.17.1

[Qemu-devel] [PATCH 2/3] vhost-user: add fds inf 'vhost_set_vring_file' in qtest

[Li Qiang]

Currently, the vhost-user-test assumes the eventfd is available.
However it's not true because the accel is qtest. So the
'vhost_set_vring_file' will not add fds to the msg and the server
side of vhost-user-test will be broken. This patch avoid this.

Signed-off-by: Li Qiang 
---
 hw/virtio/vhost-user.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index e09bed0e4a..3b666f093c 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -23,6 +23,7 @@
 #include "migration/migration.h"
 #include "migration/postcopy-ram.h"
 #include "trace.h"
+#include "sysemu/qtest.h"
 
 #include 
 #include 
@@ -742,7 +743,7 @@ static int vhost_set_vring_file(struct vhost_dev *dev,
 .hdr.size = sizeof(msg.payload.u64),
 };
 
-if (ioeventfd_enabled() && file->fd > 0) {
+if ((qtest_enabled() || ioeventfd_enabled()) && file->fd > 0) {
 fds[fd_num++] = file->fd;
 } else {
 msg.payload.u64 |= VHOST_USER_VRING_NOFD_MASK;
-- 
2.17.1

[Qemu-devel] [PATCH v3 5/7 RESEND] iotests: add filter_generated_node_ids

[John Snow]

To mimic the common filter of the same name, but for the python tests.

Signed-off-by: John Snow 
Reviewed-by: Eric Blake 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
---
 tests/qemu-iotests/iotests.py | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index a34e66813a..9595429fea 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -239,6 +239,9 @@ def filter_testfiles(msg):
 prefix = os.path.join(test_dir, "%s-" % (os.getpid()))
 return msg.replace(prefix, 'TEST_DIR/PID-')
 
+def filter_generated_node_ids(msg):
+return re.sub("#block[0-9]+", "NODE_NAME", msg)
+
 def filter_img_info(output, filename):
 lines = []
 for line in output.split('\n'):
-- 
2.17.2

[Qemu-devel] [PATCH v3 7/7] iotests: add iotest 236 for testing bitmap merge

[John Snow]

New interface, new smoke test.
---
 tests/qemu-iotests/236 | 124 +++
 tests/qemu-iotests/236.out | 200 +
 tests/qemu-iotests/group   |   1 +
 3 files changed, 325 insertions(+)
 create mode 100755 tests/qemu-iotests/236
 create mode 100644 tests/qemu-iotests/236.out

diff --git a/tests/qemu-iotests/236 b/tests/qemu-iotests/236
new file mode 100755
index 00..edf16c4ab1
--- /dev/null
+++ b/tests/qemu-iotests/236
@@ -0,0 +1,124 @@
+#!/usr/bin/env python
+#
+# Test bitmap merges.
+#
+# Copyright (c) 2018 John Snow for 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 .
+#
+# owner=js...@redhat.com
+
+import json
+import iotests
+from iotests import log
+
+iotests.verify_image_format(supported_fmts=['qcow2'])
+
+patterns = [("0x5d", "0", "64k"),
+("0xd5", "1M","64k"),
+("0xdc", "32M",   "64k"),
+("0xcd", "0x3ff", "64k")]  # 64M - 64K
+
+overwrite = [("0xab", "0", "64k"), # Full overwrite
+ ("0xad", "0x00f8000", "64k"), # Partial-left (1M-32K)
+ ("0x1d", "0x2008000", "64k"), # Partial-right (32M+32K)
+ ("0xea", "0x3fe", "64k")] # Adjacent-left (64M - 128K)
+
+def query_bitmaps(vm):
+res = vm.qmp("query-block")
+return {device['device']: device['dirty-bitmaps'] for
+device in res['return']}
+
+with iotests.FilePath('img') as img_path, \
+ iotests.VM() as vm:
+
+log('--- Preparing image & VM ---\n')
+iotests.qemu_img_pipe('create', '-f', iotests.imgfmt, img_path, '64M')
+vm.add_drive(img_path)
+vm.launch()
+
+log('--- Adding preliminary bitmaps A & B ---\n')
+vm.qmp_log("block-dirty-bitmap-add", node="drive0", name="bitmapA")
+vm.qmp_log("block-dirty-bitmap-add", node="drive0", name="bitmapB")
+
+# Dirties 4 clusters. count=262144
+log('')
+log('--- Emulating writes ---\n')
+for p in patterns:
+cmd = "write -P%s %s %s" % p
+log(cmd)
+log(vm.hmp_qemu_io("drive0", cmd))
+
+log(json.dumps(query_bitmaps(vm), indent=2, separators=(',', ': ')))
+
+log('')
+log('--- Disabling B & Adding C ---\n')
+vm.qmp_log("transaction", indent=2, actions=[
+{ "type": "block-dirty-bitmap-disable",
+  "data": { "node": "drive0", "name": "bitmapB" }},
+{ "type": "block-dirty-bitmap-add",
+  "data": { "node": "drive0", "name": "bitmapC" }},
+# Purely extraneous, but test that it works:
+{ "type": "block-dirty-bitmap-disable",
+  "data": { "node": "drive0", "name": "bitmapC" }},
+{ "type": "block-dirty-bitmap-enable",
+  "data": { "node": "drive0", "name": "bitmapC" }},
+])
+
+log('')
+log('--- Emulating further writes ---\n')
+# Dirties 6 clusters, 3 of which are new in contrast to "A".
+# A = 64 * 1024 * (4 + 3) = 458752
+# C = 64 * 1024 * 6   = 393216
+for p in overwrite:
+cmd = "write -P%s %s %s" % p
+log(cmd)
+log(vm.hmp_qemu_io("drive0", cmd))
+
+log('')
+log('--- Disabling A & C ---\n')
+vm.qmp_log("transaction", indent=2, actions=[
+{ "type": "block-dirty-bitmap-disable",
+  "data": { "node": "drive0", "name": "bitmapA" }},
+{ "type": "block-dirty-bitmap-disable",
+  "data": { "node": "drive0", "name": "bitmapC" }}
+])
+
+# A: 7 clusters
+# B: 4 clusters
+# C: 6 clusters
+log(json.dumps(query_bitmaps(vm), indent=2, separators=(',', ': ')))
+
+log('')
+log('--- Creating D as a merge of B & C ---\n')
+# Good hygiene: create a disabled bitmap as a merge target.
+vm.qmp_log("transaction", indent=2, actions=[
+{ "type": "block-dirty-bitmap-add",
+  "data": { "node": "drive0", "name": "bitmapD", "disabled": True }},
+{ "type": "block-dirty-bitmap-merge",
+  "data": { "node": "drive0", "target": "bitmapD",
+"bitmaps": ["bitmapB", "bitmapC"] }}
+])
+
+# A and D should now both have 7 clusters apiece.
+# B and C remain unchanged with 4 and 6 respectively.
+log(json.dumps(query_bitmaps(vm), indent=2, separators=(',', ': ')))
+
+# A and D should be equivalent.
+vm.qmp_log('x-debug-block-dirty-bitmap-sha256',
+   node="drive0", name="bitmapA")
+

[Qemu-devel] [PATCH v3 6/7] iotests: allow pretty-print for qmp_log

[John Snow]

If iotests have lines exceeding >998 characters long, git doesn't
want to send it plaintext to the list. We can solve this by allowing
the iotests to use pretty printed QMP output that we can match against
instead.

As a bonus, it's much nicer for human eyes, too.

Note that this changes the sort order for "command" and "arguments",
so I restrict this reordering to occur only when the indent is specified.
---
 tests/qemu-iotests/iotests.py | 17 +
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 9595429fea..ab5823c011 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -447,12 +447,21 @@ class VM(qtest.QEMUQtestMachine):
 result.append(filter_qmp_event(ev))
 return result
 
-def qmp_log(self, cmd, filters=[filter_testfiles], **kwargs):
-logmsg = '{"execute": "%s", "arguments": %s}' % \
-(cmd, json.dumps(kwargs, sort_keys=True))
+def qmp_log(self, cmd, indent=None, filters=[filter_testfiles], **kwargs):
+# Python < 3.4 needs to know not to add whitespace when 
pretty-printing:
+separators = (',', ': ') if indent is not None else (',', ': ')
+if indent is not None:
+fullcmd = { "execute": cmd,
+"arguments": kwargs }
+logmsg = json.dumps(fullcmd, indent=indent, separators=separators,
+sort_keys=True)
+else:
+logmsg = '{"execute": "%s", "arguments": %s}' % \
+(cmd, json.dumps(kwargs, sort_keys=True))
 log(logmsg, filters)
 result = self.qmp(cmd, **kwargs)
-log(json.dumps(result, sort_keys=True), filters)
+log(json.dumps(result, indent=indent, separators=separators,
+   sort_keys=True), filters)
 return result
 
 def run_job(self, job, auto_finalize=True, auto_dismiss=False):
-- 
2.17.2

[Qemu-devel] [PATCH v3 4/7] iotests.py: don't abort if IMGKEYSECRET is undefined

[John Snow]

Instead of using os.environ[], use .get with a default of empty string
to match the setup in check to allow us to import the iotests module
(for debugging, say) without needing a crafted environment just to
import the module.

Signed-off-by: John Snow 
Reviewed-by: Eric Blake 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
---
 tests/qemu-iotests/iotests.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index d537538ba0..a34e66813a 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -63,7 +63,7 @@ socket_scm_helper = os.environ.get('SOCKET_SCM_HELPER', 
'socket_scm_helper')
 debug = False
 
 luks_default_secret_object = 'secret,id=keysec0,data=' + \
- os.environ['IMGKEYSECRET']
+ os.environ.get('IMGKEYSECRET', '')
 luks_default_key_secret_opt = 'key-secret=keysec0'
 
 
-- 
2.17.2

[Qemu-devel] [PATCH v3 3/7] block: remove 'x' prefix from experimental bitmap APIs

[John Snow]

The 'x' prefix was added because I was uncertain of the direction we'd
take for the libvirt API. With the general approach solidified, I feel
comfortable committing to this API for 4.0.

Signed-off-by: John Snow 
Reviewed-by: Eric Blake 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
---
 blockdev.c | 22 +++---
 qapi/block-core.json   | 34 +-
 qapi/transaction.json  | 12 ++--
 tests/qemu-iotests/223 |  4 ++--
 4 files changed, 36 insertions(+), 36 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 0f740fd964..da87aae5cf 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1966,7 +1966,7 @@ static void block_dirty_bitmap_add_prepare(BlkActionState 
*common,
action->has_granularity, action->granularity,
action->has_persistent, action->persistent,
action->has_autoload, action->autoload,
-   action->has_x_disabled, action->x_disabled,
+   action->has_disabled, action->disabled,
_err);
 
 if (!local_err) {
@@ -2051,7 +2051,7 @@ static void 
block_dirty_bitmap_enable_prepare(BlkActionState *common,
 return;
 }
 
-action = common->action->u.x_block_dirty_bitmap_enable.data;
+action = common->action->u.block_dirty_bitmap_enable.data;
 state->bitmap = block_dirty_bitmap_lookup(action->node,
   action->name,
   NULL,
@@ -2092,7 +2092,7 @@ static void 
block_dirty_bitmap_disable_prepare(BlkActionState *common,
 return;
 }
 
-action = common->action->u.x_block_dirty_bitmap_disable.data;
+action = common->action->u.block_dirty_bitmap_disable.data;
 state->bitmap = block_dirty_bitmap_lookup(action->node,
   action->name,
   NULL,
@@ -2137,7 +2137,7 @@ static void 
block_dirty_bitmap_merge_prepare(BlkActionState *common,
 return;
 }
 
-action = common->action->u.x_block_dirty_bitmap_merge.data;
+action = common->action->u.block_dirty_bitmap_merge.data;
 
 do_block_dirty_bitmap_merge(action->node, action->target,
 action->bitmaps, >backup,
@@ -2205,17 +2205,17 @@ static const BlkActionOps actions[] = {
 .commit = block_dirty_bitmap_free_backup,
 .abort = block_dirty_bitmap_restore,
 },
-[TRANSACTION_ACTION_KIND_X_BLOCK_DIRTY_BITMAP_ENABLE] = {
+[TRANSACTION_ACTION_KIND_BLOCK_DIRTY_BITMAP_ENABLE] = {
 .instance_size = sizeof(BlockDirtyBitmapState),
 .prepare = block_dirty_bitmap_enable_prepare,
 .abort = block_dirty_bitmap_enable_abort,
 },
-[TRANSACTION_ACTION_KIND_X_BLOCK_DIRTY_BITMAP_DISABLE] = {
+[TRANSACTION_ACTION_KIND_BLOCK_DIRTY_BITMAP_DISABLE] = {
 .instance_size = sizeof(BlockDirtyBitmapState),
 .prepare = block_dirty_bitmap_disable_prepare,
 .abort = block_dirty_bitmap_disable_abort,
 },
-[TRANSACTION_ACTION_KIND_X_BLOCK_DIRTY_BITMAP_MERGE] = {
+[TRANSACTION_ACTION_KIND_BLOCK_DIRTY_BITMAP_MERGE] = {
 .instance_size = sizeof(BlockDirtyBitmapState),
 .prepare = block_dirty_bitmap_merge_prepare,
 .commit = block_dirty_bitmap_free_backup,
@@ -2931,7 +2931,7 @@ void qmp_block_dirty_bitmap_clear(const char *node, const 
char *name,
 bdrv_clear_dirty_bitmap(bitmap, NULL);
 }
 
-void qmp_x_block_dirty_bitmap_enable(const char *node, const char *name,
+void qmp_block_dirty_bitmap_enable(const char *node, const char *name,
Error **errp)
 {
 BlockDriverState *bs;
@@ -2952,7 +2952,7 @@ void qmp_x_block_dirty_bitmap_enable(const char *node, 
const char *name,
 bdrv_enable_dirty_bitmap(bitmap);
 }
 
-void qmp_x_block_dirty_bitmap_disable(const char *node, const char *name,
+void qmp_block_dirty_bitmap_disable(const char *node, const char *name,
 Error **errp)
 {
 BlockDriverState *bs;
@@ -3014,8 +3014,8 @@ void do_block_dirty_bitmap_merge(const char *node, const 
char *target,
 bdrv_release_dirty_bitmap(bs, anon);
 }
 
-void qmp_x_block_dirty_bitmap_merge(const char *node, const char *target,
-strList *bitmaps, Error **errp)
+void qmp_block_dirty_bitmap_merge(const char *node, const char *target,
+  strList *bitmaps, Error **errp)
 {
 do_block_dirty_bitmap_merge(node, target, bitmaps, NULL, errp);
 }
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 320d74ef34..fde96fdb50 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1803,15 +1803,15 @@
 #Currently, all dirty tracking bitmaps are loaded from Qcow2 on
 #open.
 #
-# @x-disabled: the bitmap is created in the disabled 

[Qemu-devel] [PATCH v3 1/7] blockdev: abort transactions in reverse order

[John Snow]

Presently, we abort transactions in the same order they were processed in.
Bitmap commands, though, attempt to restore backup data structures on abort.
To that end, though, they need to be aborted in reverse chronological order.

Replace the QSIMPLEQ data structure with a QTAILQ one, so we can iterate
in reverse for the abort phase of the transaction.

Signed-off-by: John Snow 
Reviewed-by: Eric Blake 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
---
 blockdev.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 81f95d920b..1ba706df8b 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1341,7 +1341,7 @@ struct BlkActionState {
 const BlkActionOps *ops;
 JobTxn *block_job_txn;
 TransactionProperties *txn_props;
-QSIMPLEQ_ENTRY(BlkActionState) entry;
+QTAILQ_ENTRY(BlkActionState) entry;
 };
 
 /* internal snapshot private data */
@@ -2269,8 +2269,8 @@ void qmp_transaction(TransactionActionList *dev_list,
 BlkActionState *state, *next;
 Error *local_err = NULL;
 
-QSIMPLEQ_HEAD(snap_bdrv_states, BlkActionState) snap_bdrv_states;
-QSIMPLEQ_INIT(_bdrv_states);
+QTAILQ_HEAD(snap_bdrv_states, BlkActionState) snap_bdrv_states;
+QTAILQ_INIT(_bdrv_states);
 
 /* Does this transaction get canceled as a group on failure?
  * If not, we don't really need to make a JobTxn.
@@ -2301,7 +2301,7 @@ void qmp_transaction(TransactionActionList *dev_list,
 state->action = dev_info;
 state->block_job_txn = block_job_txn;
 state->txn_props = props;
-QSIMPLEQ_INSERT_TAIL(_bdrv_states, state, entry);
+QTAILQ_INSERT_TAIL(_bdrv_states, state, entry);
 
 state->ops->prepare(state, _err);
 if (local_err) {
@@ -2310,7 +2310,7 @@ void qmp_transaction(TransactionActionList *dev_list,
 }
 }
 
-QSIMPLEQ_FOREACH(state, _bdrv_states, entry) {
+QTAILQ_FOREACH(state, _bdrv_states, entry) {
 if (state->ops->commit) {
 state->ops->commit(state);
 }
@@ -2321,13 +2321,13 @@ void qmp_transaction(TransactionActionList *dev_list,
 
 delete_and_fail:
 /* failure, and it is all-or-none; roll back all operations */
-QSIMPLEQ_FOREACH(state, _bdrv_states, entry) {
+QTAILQ_FOREACH_REVERSE(state, _bdrv_states, snap_bdrv_states, entry) {
 if (state->ops->abort) {
 state->ops->abort(state);
 }
 }
 exit:
-QSIMPLEQ_FOREACH_SAFE(state, _bdrv_states, entry, next) {
+QTAILQ_FOREACH_SAFE(state, _bdrv_states, entry, next) {
 if (state->ops->clean) {
 state->ops->clean(state);
 }
-- 
2.17.2

[Qemu-devel] [PATCH v3 2/7] blockdev: n-ary bitmap merge

[John Snow]

Especially outside of transactions, it is helpful to provide
all-or-nothing semantics for bitmap merges. This facilitates
the coalescing of multiple bitmaps into a single target for
the "checkpoint" interpretation when assembling bitmaps that
represent arbitrary points in time from component bitmaps.

This is an incompatible change from the preliminary version
of the API.

Signed-off-by: John Snow 
Reviewed-by: Eric Blake 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
---
 blockdev.c   | 64 +---
 qapi/block-core.json | 22 +++
 2 files changed, 53 insertions(+), 33 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 1ba706df8b..0f740fd964 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2122,33 +2122,26 @@ static void 
block_dirty_bitmap_disable_abort(BlkActionState *common)
 }
 }
 
+void do_block_dirty_bitmap_merge(const char *node, const char *target,
+ strList *bitmaps, HBitmap **backup,
+ Error **errp);
+
 static void block_dirty_bitmap_merge_prepare(BlkActionState *common,
  Error **errp)
 {
 BlockDirtyBitmapMerge *action;
 BlockDirtyBitmapState *state = DO_UPCAST(BlockDirtyBitmapState,
  common, common);
-BdrvDirtyBitmap *merge_source;
 
 if (action_check_completion_mode(common, errp) < 0) {
 return;
 }
 
 action = common->action->u.x_block_dirty_bitmap_merge.data;
-state->bitmap = block_dirty_bitmap_lookup(action->node,
-  action->dst_name,
-  >bs,
-  errp);
-if (!state->bitmap) {
-return;
-}
 
-merge_source = bdrv_find_dirty_bitmap(state->bs, action->src_name);
-if (!merge_source) {
-return;
-}
-
-bdrv_merge_dirty_bitmap(state->bitmap, merge_source, >backup, errp);
+do_block_dirty_bitmap_merge(action->node, action->target,
+action->bitmaps, >backup,
+errp);
 }
 
 static void abort_prepare(BlkActionState *common, Error **errp)
@@ -2980,24 +2973,51 @@ void qmp_x_block_dirty_bitmap_disable(const char *node, 
const char *name,
 bdrv_disable_dirty_bitmap(bitmap);
 }
 
-void qmp_x_block_dirty_bitmap_merge(const char *node, const char *dst_name,
-const char *src_name, Error **errp)
+void do_block_dirty_bitmap_merge(const char *node, const char *target,
+ strList *bitmaps, HBitmap **backup,
+ Error **errp)
 {
 BlockDriverState *bs;
-BdrvDirtyBitmap *dst, *src;
+BdrvDirtyBitmap *dst, *src, *anon;
+strList *lst;
+Error *local_err = NULL;
 
-dst = block_dirty_bitmap_lookup(node, dst_name, , errp);
+dst = block_dirty_bitmap_lookup(node, target, , errp);
 if (!dst) {
 return;
 }
 
-src = bdrv_find_dirty_bitmap(bs, src_name);
-if (!src) {
-error_setg(errp, "Dirty bitmap '%s' not found", src_name);
+anon = bdrv_create_dirty_bitmap(bs, bdrv_dirty_bitmap_granularity(dst),
+NULL, errp);
+if (!anon) {
 return;
 }
 
-bdrv_merge_dirty_bitmap(dst, src, NULL, errp);
+for (lst = bitmaps; lst; lst = lst->next) {
+src = bdrv_find_dirty_bitmap(bs, lst->value);
+if (!src) {
+error_setg(errp, "Dirty bitmap '%s' not found", lst->value);
+goto out;
+}
+
+bdrv_merge_dirty_bitmap(anon, src, NULL, _err);
+if (local_err) {
+error_propagate(errp, local_err);
+goto out;
+}
+}
+
+/* Merge into dst; dst is unchanged on failure. */
+bdrv_merge_dirty_bitmap(dst, anon, backup, errp);
+
+ out:
+bdrv_release_dirty_bitmap(bs, anon);
+}
+
+void qmp_x_block_dirty_bitmap_merge(const char *node, const char *target,
+strList *bitmaps, Error **errp)
+{
+do_block_dirty_bitmap_merge(node, target, bitmaps, NULL, errp);
 }
 
 BlockDirtyBitmapSha256 *qmp_x_debug_block_dirty_bitmap_sha256(const char *node,
diff --git a/qapi/block-core.json b/qapi/block-core.json
index d4fe710836..320d74ef34 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1818,14 +1818,14 @@
 #
 # @node: name of device/node which the bitmap is tracking
 #
-# @dst_name: name of the destination dirty bitmap
+# @target: name of the destination dirty bitmap
 #
-# @src_name: name of the source dirty bitmap
+# @bitmaps: name(s) of the source dirty bitmap(s)
 #
 # Since: 3.0
 ##
 { 'struct': 'BlockDirtyBitmapMerge',
-  'data': { 'node': 'str', 'dst_name': 'str', 'src_name': 'str' } }
+  'data': { 'node': 'str', 'target': 'str', 'bitmaps': ['str'] } }
 
 ##
 # @block-dirty-bitmap-add:
@@ -1940,23 +1940,23 @@
 ##
 # 

[Qemu-devel] [PATCH v3 0/7] bitmaps: remove x- prefix from QMP api

[John Snow]

Touch up a few last things and remove the x- prefix.

V3:
 - Reworked qmp_log to pretty-print the outgoing command, too [Vladimir]
 - Modified test to log only bitmap information [Vladimir]
 - Test disable/enable transaction toggle [Eric]

Note that the filter I added is now unused, but I think we will want it
and it's small enough, so I'm going to check it in anyway. If you disagree,
I'll just drop the patch instead.

--js

John Snow (7):
  blockdev: abort transactions in reverse order
  blockdev: n-ary bitmap merge
  block: remove 'x' prefix from experimental bitmap APIs
  iotests.py: don't abort if IMGKEYSECRET is undefined
  iotests: add filter_generated_node_ids
  iotests: allow pretty-print for qmp_log
  iotests: add iotest 236 for testing bitmap merge

 blockdev.c|  96 +---
 qapi/block-core.json  |  56 +-
 qapi/transaction.json |  12 +-
 tests/qemu-iotests/223|   4 +-
 tests/qemu-iotests/236| 124 +
 tests/qemu-iotests/236.out| 200 ++
 tests/qemu-iotests/group  |   1 +
 tests/qemu-iotests/iotests.py |  22 +++-
 8 files changed, 436 insertions(+), 79 deletions(-)
 create mode 100755 tests/qemu-iotests/236
 create mode 100644 tests/qemu-iotests/236.out

-- 
2.17.2

Re: [Qemu-devel] [PATCH for-4.0 0/6] vhost-user-blk: Add support for backend reconnecting

[Michael S. Tsirkin]

On Fri, Dec 14, 2018 at 10:33:54AM +0800, Yongji Xie wrote:
> On Fri, 14 Dec 2018 at 10:20, Michael S. Tsirkin  wrote:
> >
> > On Fri, Dec 14, 2018 at 09:56:41AM +0800, Yongji Xie wrote:
> > > On Thu, 13 Dec 2018 at 22:45, Michael S. Tsirkin  wrote:
> > > >
> > > > On Thu, Dec 06, 2018 at 02:35:46PM +0800, elohi...@gmail.com wrote:
> > > > > From: Xie Yongji 
> > > > >
> > > > > This patchset is aimed at supporting qemu to reconnect
> > > > > vhost-user-blk backend after vhost-user-blk backend crash or
> > > > > restart.
> > > > >
> > > > > The patch 1 tries to implenment the sync connection for
> > > > > "reconnect socket".
> > > > >
> > > > > The patch 2 introduces a new message VHOST_USER_SET_VRING_INFLIGHT
> > > > > to support offering shared memory to backend to record
> > > > > its inflight I/O.
> > > > >
> > > > > The patch 3,4 are the corresponding libvhost-user patches of
> > > > > patch 2. Make libvhost-user support VHOST_USER_SET_VRING_INFLIGHT.
> > > > >
> > > > > The patch 5 supports vhost-user-blk to reconnect backend when
> > > > > connection closed.
> > > > >
> > > > > The patch 6 tells qemu that we support reconnecting now.
> > > > >
> > > > > To use it, we could start qemu with:
> > > > >
> > > > > qemu-system-x86_64 \
> > > > > -chardev 
> > > > > socket,id=char0,path=/path/vhost.socket,reconnect=1,wait \
> > > > > -device vhost-user-blk-pci,chardev=char0 \
> > > > >
> > > > > and start vhost-user-blk backend with:
> > > > >
> > > > > vhost-user-blk -b /path/file -s /path/vhost.socket
> > > > >
> > > > > Then we can restart vhost-user-blk at any time during VM running.
> > > > >
> > > > > Xie Yongji (6):
> > > > >   char-socket: Enable "wait" option for client mode
> > > > >   vhost-user: Add shared memory to record inflight I/O
> > > > >   libvhost-user: Introduce vu_queue_map_desc()
> > > > >   libvhost-user: Support recording inflight I/O in shared memory
> > > > >   vhost-user-blk: Add support for reconnecting backend
> > > > >   contrib/vhost-user-blk: enable inflight I/O recording
> > > >
> > > > What is missing in all this is documentation.
> > > > Specifically docs/interop/vhost-user.txt.
> > > >
> > > > At a high level the design is IMO a good one.
> > > >
> > > > However I would prefer reading the protocol first before
> > > > the code.
> > > >
> > > > So here's what I managed to figure out, and it matches
> > > > how I imagined it would work when I was still
> > > > thinking about out of order for net:
> > > >
> > > > - backend allocates memory to keep its stuff around
> > > > - sends it to qemu so it can maintain it
> > > > - gets it back on reconnect
> > > >
> > > > format and size etc are all up to the backend,
> > > > a good implementation would probably implement some
> > > > kind of versioning.
> > > >
> > > > Is this what this implements?
> > > >
> > >
> > > Definitely, yes. And the comments looks good to me. Qemu get size and
> > > version from backend, then allocate memory and send it back with
> > > version. Backend knows how to use the memory according to the version.
> > > If we do that, should we allocate the memory per device rather than
> > > per virtqueue?
> > >
> > > Thanks,
> > > Yongji
> >
> > It's up to you. Maybe both.
> >
> 
> OK. I think I may still keep it in virtqueue level in v2. Thank you.
> 
> Thanks,
> Yongji

I'd actually add options for both, and backend can set size 0 if it
wants to.

-- 
MST

Re: [Qemu-devel] [PATCH] hw/pci-bridge: check invalid slot number for root and downstream port

[Michael S. Tsirkin]

On Wed, Dec 05, 2018 at 05:41:26PM +0800, Huan Xiong wrote:
> Since root and downstream port have only one slot, device should be
> connected to them using slot 0. QEMU doesn't have a check for that
> and starts up when a non-zero slot is specified, though the device
> is not seen in guest OS.
> 
> The change fixes that by adding a check in PCI device "attr" property
> setter function. The check is performed only if a PCI device has been
> connected to a bus, otherwise it does nothing. The latter occurs because
> setter function is also called in object instantiation phase to set
> property default value.
> 
> Signed-off-by: Huan Xiong 

I thought that a non 0 slot is useful for ARI. No?


> ---
>  hw/core/qdev-properties.c  |  5 -
>  hw/pci-bridge/pcie_root_port.c |  2 +-
>  hw/pci-bridge/xio3130_downstream.c |  2 +-
>  hw/pci/pci.c   | 13 +
>  include/hw/pci/pci.h   |  1 +
>  5 files changed, 20 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
> index 35072de..6e79219 100644
> --- a/hw/core/qdev-properties.c
> +++ b/hw/core/qdev-properties.c
> @@ -654,6 +654,8 @@ static void set_pci_devfn(Object *obj, Visitor *v, const 
> char *name,
>void *opaque, Error **errp)
>  {
>  DeviceState *dev = DEVICE(obj);
> +BusState *bus = qdev_get_parent_bus(dev);
> +BusClass *bus_class = bus ? BUS_GET_CLASS(bus) : NULL;
>  Property *prop = opaque;
>  int32_t value, *ptr = qdev_get_prop_ptr(dev, prop);
>  unsigned int slot, fn, n;
> @@ -687,7 +689,8 @@ static void set_pci_devfn(Object *obj, Visitor *v, const 
> char *name,
>  goto invalid;
>  }
>  }
> -if (str[n] != '\0' || fn > 7 || slot > 31) {
> +if (str[n] != '\0' || fn > 7 || slot > 31 || (bus_class &&
> +bus_class->max_dev != 0 && slot >= bus_class->max_dev)) {
>  goto invalid;
>  }

This looks kind of complicated. Isn't there an easier way?

>  *ptr = slot << 3 | fn;
> diff --git a/hw/pci-bridge/pcie_root_port.c b/hw/pci-bridge/pcie_root_port.c
> index 45f9e8c..ee42411 100644
> --- a/hw/pci-bridge/pcie_root_port.c
> +++ b/hw/pci-bridge/pcie_root_port.c
> @@ -61,7 +61,7 @@ static void rp_realize(PCIDevice *d, Error **errp)
>  int rc;
>  
>  pci_config_set_interrupt_pin(d->config, 1);
> -pci_bridge_initfn(d, TYPE_PCIE_BUS);
> +pci_bridge_initfn(d, TYPE_PCIE_DOWNSTREAM_BUS);
>  pcie_port_init_reg(d);
>  
>  rc = pci_bridge_ssvid_init(d, rpc->ssvid_offset, dc->vendor_id,
> diff --git a/hw/pci-bridge/xio3130_downstream.c 
> b/hw/pci-bridge/xio3130_downstream.c
> index 467bbab..960a90c 100644
> --- a/hw/pci-bridge/xio3130_downstream.c
> +++ b/hw/pci-bridge/xio3130_downstream.c
> @@ -62,7 +62,7 @@ static void xio3130_downstream_realize(PCIDevice *d, Error 
> **errp)
>  PCIESlot *s = PCIE_SLOT(d);
>  int rc;
>  
> -pci_bridge_initfn(d, TYPE_PCIE_BUS);
> +pci_bridge_initfn(d, TYPE_PCIE_DOWNSTREAM_BUS);
>  pcie_port_init_reg(d);
>  
>  rc = msi_init(d, XIO3130_MSI_OFFSET, XIO3130_MSI_NR_VECTOR,
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 56b13b3..457736d 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -187,6 +187,18 @@ static const TypeInfo pcie_bus_info = {
>  .parent = TYPE_PCI_BUS,
>  };
>  
> +static void pcie_downstream_bus_class_init(ObjectClass *klass, void *data)
> +{
> +BusClass *k = BUS_CLASS(klass);
> +k->max_dev = 1;
> +}
> +
> +static const TypeInfo pcie_downstream_bus_info = {
> +.name = TYPE_PCIE_DOWNSTREAM_BUS,
> +.parent = TYPE_PCIE_BUS,
> +.class_init = pcie_downstream_bus_class_init,
> +};
> +
>  static PCIBus *pci_find_bus_nr(PCIBus *bus, int bus_num);
>  static void pci_update_mappings(PCIDevice *d);
>  static void pci_irq_handler(void *opaque, int irq_num, int level);
> @@ -2681,6 +2693,7 @@ static void pci_register_types(void)
>  {
>  type_register_static(_bus_info);
>  type_register_static(_bus_info);
> +type_register_static(_downstream_bus_info);
>  type_register_static(_pci_interface_info);
>  type_register_static(_interface_info);
>  type_register_static(_device_type_info);
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index e6514bb..2253757 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -393,6 +393,7 @@ typedef PCIINTxRoute (*pci_route_irq_fn)(void *opaque, 
> int pin);
>  #define PCI_BUS_CLASS(klass) OBJECT_CLASS_CHECK(PCIBusClass, (klass), 
> TYPE_PCI_BUS)
>  #define PCI_BUS_GET_CLASS(obj) OBJECT_GET_CLASS(PCIBusClass, (obj), 
> TYPE_PCI_BUS)
>  #define TYPE_PCIE_BUS "PCIE"
> +#define TYPE_PCIE_DOWNSTREAM_BUS "PCIE-downstream"
>  
>  bool pci_bus_is_express(PCIBus *bus);
>  bool pci_bus_is_root(PCIBus *bus);
> -- 
> 2.7.4

Re: [Qemu-devel] [PATCH v11 0/7] virtio-balloon: free page hint support

[Michael S. Tsirkin]

On Tue, Dec 11, 2018 at 04:24:46PM +0800, Wei Wang wrote:
> This is the deivce part implementation to add a new feature,
> VIRTIO_BALLOON_F_FREE_PAGE_HINT to the virtio-balloon device. The device
> receives the guest free page hints from the driver and clears the
> corresponding bits in the dirty bitmap, so that those free pages are
> not sent by the migration thread to the destination.

For virtio bits:

Reviewed-by: Michael S. Tsirkin 

I think this is primarily a migration feature so please merge
through the migration tree.



> *Tests
> 1 Test Environment
> Host: Intel(R) Xeon(R) CPU E5-2699 v4 @ 2.20GHz
> Migration setup: migrate_set_speed 100G, migrate_set_downtime 400ms
> 
> 2 Test Results (results are averaged over several repeated runs)
> 2.1 Guest setup: 8G RAM, 4 vCPU
> 2.1.1 Idle guest live migration time
> Optimization v.s. Legacy = 620ms vs 2970ms
> --> ~79% reduction
> 2.1.2 Guest live migration with Linux compilation workload
>   (i.e. make bzImage -j4) running
>   1) Live Migration Time:
>  Optimization v.s. Legacy = 2273ms v.s. 4502ms
>  --> ~50% reduction
>   2) Linux Compilation Time:
>  Optimization v.s. Legacy = 8min42s v.s. 8min43s
>  --> no obvious difference
> 
> 2.2 Guest setup: 128G RAM, 4 vCPU
> 2.2.1 Idle guest live migration time
> Optimization v.s. Legacy = 5294ms vs 41651ms
> --> ~87% reduction
> 2.2.2 Guest live migration with Linux compilation workload
>   1) Live Migration Time:
> Optimization v.s. Legacy = 8816ms v.s. 54201ms
> --> 84% reduction
>   2) Linux Compilation Time:
> Optimization v.s. Legacy = 8min30s v.s. 8min36s
> --> no obvious difference
> 
> ChangeLog:
> v10->v11:
> migration:
> - qemu_guest_free_page_hint:
> - "offset >= block->used_length", instead of
>   "offset > block->used_length";
> - RAMState: enable the "fpo_enabled" flag, when the free page
>   optimization feature is used, instead of disabling ram_bulk_stage.
>   Please see patch 6 commit log for details.
> 
> Previous changelog:
> http://lists.nongnu.org/archive/html/qemu-devel/2018-12/msg00055.html
> 
> Wei Wang (7):
>   bitmap: fix bitmap_count_one
>   bitmap: bitmap_count_one_with_offset
>   migration: use bitmap_mutex in migration_bitmap_clear_dirty
>   migration: API to clear bits of guest free pages from the dirty bitmap
>   migration/ram.c: add a notifier chain for precopy
>   migration/ram.c: add the free page optimization enable flag
>   virtio-balloon: VIRTIO_BALLOON_F_FREE_PAGE_HINT
> 
>  hw/virtio/virtio-balloon.c  | 263 
> 
>  include/hw/virtio/virtio-balloon.h  |  28 ++-
>  include/migration/misc.h|  22 ++
>  include/qemu/bitmap.h   |  17 ++
>  include/standard-headers/linux/virtio_balloon.h |   5 +
>  migration/ram.c | 121 ++-
>  migration/savevm.c  |  15 ++
>  vl.c|   1 +
>  8 files changed, 466 insertions(+), 6 deletions(-)
> 
> -- 
> 1.8.3.1

Re: [Qemu-devel] [PULL 01/15] contrib: add a basic gitdm config

[Alex Bennée]

Alex Bennée  writes:

> This is a QEMU specific version of a gitdm config for generating
> reports on the contributor base of the project. I've added enough
> group maps and domain aliases to ensure the current top ten is as
> reflective as it can be. As of this commit running:

> +# Also group together our prolific individual contributors
> +# and those working under academic auspices
> +GroupMap contrib/gitdm/group-map-individuals (None)
> +GroupMap contrib/gitdm/group-map-academics Academics (various)

Have sent v4 for these two files. I'll make a new PR on Monday.

> +
> +#
> +#
> +# Use FileTypeMap to map a file types to file names using regular
> +# regular expressions.
> +#
> +FileTypeMap contrib/gitdm/filetypes.txt


--
Alex Bennée

[Qemu-devel] [PATCH v4] contrib: add a basic gitdm config

[Alex Bennée]

This is a QEMU specific version of a gitdm config for generating
reports on the contributor base of the project. I've added enough
group maps and domain aliases to ensure the current top ten is as
reflective as it can be. As of this commit running:

  git log --numstat --since "Last Year" | gitdm -n -l 10

Reports:

  Top changeset contributors by employer
  Red Hat   3172 (44.3%)
  Linaro1153 (16.1%)
  (None) 549 (7.7%)
  IBM348 (4.9%)
  Academics (various)170 (2.4%)
  Virtuozzo  168 (2.3%)
  Wave Computing 118 (1.6%)
  Xilinx 102 (1.4%)
  Igalia  93 (1.3%)
  Cadence Design Systems  88 (1.2%)

  Top lines changed by employer
  Red Hat   144092 (28.1%)
  Cadence Design Systems126554 (24.6%)
  Linaro77480 (15.1%)
  Wave Computing33134 (6.5%)
  SiFive14392 (2.8%)
  IBM   12219 (2.4%)
  (None)11948 (2.3%)
  Academics (various)   10447 (2.0%)
  Virtuozzo 10445 (2.0%)
  CodeWeavers   9179 (1.8%)

Signed-off-by: Alex Bennée 
Reviewed-by: Daniel P. Berrangé 
Reviewed-by: Markus Armbruster 
Reviewed-by: Aleksandar Markovic 

---
v2
  - use aliases instead of .mailmap
  - add more companies to domainmap
  - add more groups to cover non-corporate email
  - add an individuals group-map for (None)
  - limit the stats to top ten
v3
  - updates to wavecomp group
  - grouped ispras & others under academics
  - tweaked invocation
  - added tags
  - updated stats while based of master
  - slimmed down filetypes, added QEMU specific patterns
v4
  - missing group maps
---
 contrib/gitdm/aliases   |  27 +
 contrib/gitdm/domain-map|  19 
 contrib/gitdm/filetypes.txt | 146 
 contrib/gitdm/group-map-academics   |  14 +++
 contrib/gitdm/group-map-cadence |   3 +
 contrib/gitdm/group-map-codeweavers |   1 +
 contrib/gitdm/group-map-ibm |   6 ++
 contrib/gitdm/group-map-individuals |  10 ++
 contrib/gitdm/group-map-redhat  |   7 ++
 contrib/gitdm/group-map-wavecomp|  18 
 gitdm.config|  50 ++
 11 files changed, 301 insertions(+)
 create mode 100644 contrib/gitdm/aliases
 create mode 100644 contrib/gitdm/domain-map
 create mode 100644 contrib/gitdm/filetypes.txt
 create mode 100644 contrib/gitdm/group-map-academics
 create mode 100644 contrib/gitdm/group-map-cadence
 create mode 100644 contrib/gitdm/group-map-codeweavers
 create mode 100644 contrib/gitdm/group-map-ibm
 create mode 100644 contrib/gitdm/group-map-individuals
 create mode 100644 contrib/gitdm/group-map-redhat
 create mode 100644 contrib/gitdm/group-map-wavecomp
 create mode 100644 gitdm.config

diff --git a/contrib/gitdm/aliases b/contrib/gitdm/aliases
new file mode 100644
index 00..07fd3391a5
--- /dev/null
+++ b/contrib/gitdm/aliases
@@ -0,0 +1,27 @@
+#
+# This is the email aliases file, mapping secondary addresses
+# onto a single, canonical address. Duplicates some info from .mailmap
+#
+
+# weird commits
+balrog@c046a42c-6fe2-441c-8c8c-71466251a162 balr...@gmail.com
+aliguori@c046a42c-6fe2-441c-8c8c-71466251a162 anth...@codemonkey.ws
+aurel32@c046a42c-6fe2-441c-8c8c-71466251a162 aurel...@aurel32.net
+blueswir1@c046a42c-6fe2-441c-8c8c-71466251a162 blauwir...@gmail.com
+edgar_igl@c046a42c-6fe2-441c-8c8c-71466251a162 edgar.igles...@gmail.com
+bellard@c046a42c-6fe2-441c-8c8c-71466251a162 fabr...@bellard.org
+j_mayer@c046a42c-6fe2-441c-8c8c-71466251a162 l_ind...@magic.fr
+pbrook@c046a42c-6fe2-441c-8c8c-71466251a162 p...@codesourcery.com
+ths@c046a42c-6fe2-441c-8c8c-71466251a162 t...@networkno.de
+malc@c046a42c-6fe2-441c-8c8c-71466251a162 av1...@comtv.ru
+
+# There is also a:
+#(no author) <(no author)@c046a42c-6fe2-441c-8c8c-71466251a162>
+# for the cvs2svn initialization commit e63c3dc74bf.
+
+# Next, translate a few commits where mailman rewrote the From: line due
+# to strict SPF, although we prefer to avoid adding more entries like that.
+"Ed Swierk via Qemu-devel" eswi...@skyportsystems.com
+"Ian McKellar via Qemu-devel" ianl...@google.com
+"Julia Suvorova via Qemu-devel" jus...@mail.ru
+"Justin Terry (VM) via Qemu-devel" jute...@microsoft.com
diff --git a/contrib/gitdm/domain-map b/contrib/gitdm/domain-map
new file mode 100644
index 00..8cbbcfe93d
--- /dev/null
+++ b/contrib/gitdm/domain-map
@@ -0,0 +1,19 @@
+#
+# QEMU gitdm domain-map
+#
+# This maps email domains to nice easy to read company names
+#
+
+amd.com AMD
+greensocs.com   GreenSocs
+ibm.com IBM
+igalia.com  Igalia
+linaro.org  Linaro
+oracle.com  Oracle
+redhat.com  Red Hat
+siemens.com Siemens
+sifive.com  SiFive
+suse.de SUSE
+virtuozzo.com   Virtuozzo
+wdc.com Western Digital
+xilinx.com  Xilinx
diff 

Re: [Qemu-devel] [PATCH v2 6/7] iotests: allow pretty-print for qmp_log

[John Snow]

On 12/13/18 8:09 AM, Vladimir Sementsov-Ogievskiy wrote:
> 13.12.2018 4:50, John Snow wrote:
>> If iotests have lines exceeding >998 characters long, git doesn't
>> want to send it plaintext to the list. We can solve this by allowing
>> the iotests to use pretty printed QMP output that we can match against
>> instead.
>>
>> As a bonus, it's much nicer for human eyes, too.
>>
>> Signed-off-by: John Snow 
>> ---
>>   tests/qemu-iotests/iotests.py | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
>> index 9595429fea..dbbef4bad3 100644
>> --- a/tests/qemu-iotests/iotests.py
>> +++ b/tests/qemu-iotests/iotests.py
>> @@ -447,12 +447,12 @@ class VM(qtest.QEMUQtestMachine):
>>   result.append(filter_qmp_event(ev))
>>   return result
>>   
>> -def qmp_log(self, cmd, filters=[filter_testfiles], **kwargs):
>> +def qmp_log(self, cmd, indent=None, filters=[filter_testfiles], 
>> **kwargs):
>>   logmsg = '{"execute": "%s", "arguments": %s}' % \
>>   (cmd, json.dumps(kwargs, sort_keys=True))
>>   log(logmsg, filters)
> 
> why not to prettify cmd json too? Just make fullobj = {"execute": cmd, 
> "arguments": kwargs}, and prettify it.
> (hmm, unfortunately, "execute" < "arguments", and they will be rearranged)
> 

It wasn't long enough to irritate me :)
but we're here, so I'll do that too.

> with or without (as the patch don't aim to prettify everything):
> Reviewed-by: Vladimir Sementsov-Ogievskiy 
> 
>>   result = self.qmp(cmd, **kwargs)
>> -log(json.dumps(result, sort_keys=True), filters)
>> +log(json.dumps(result, indent=indent, sort_keys=True), filters)
>>   return result
> 
> hmm, doing the same thing as Eric (check specs), I see the following note:
> 
>  > Note: Since the default item separator is ', ', the output might include 
> trailing whitespace when indent is specified.
> 
> And I remember a pain of trailing whitespaces in iotests on, may be, 
> backporting, or something like this some time ago.
> It's of course not about this patch, but I think it is a good idea to avoid 
> trailing whitespaces in test output, at least
> in common helpers. May be best place to fix is iotests.log() function
> 

Oh, good spot. I actually did run into this and wasn't aware of what
caused it! I will change the default separator.

>>   
>>   def run_job(self, job, auto_finalize=True, auto_dismiss=False):
>>
> 
> 

Re: [Qemu-devel] Loading snapshot with readonly qcow2 image

[Eric Blake]

On 12/14/18 10:03 AM, Michael Spradling wrote:


Can you combine -s (create a writable temp file) with -l to get what you
want?

/me tries:




I can confirm that 'qemu-nbd -s a' lets me write data that is discarded on
disconnect (lsof says a temp file in /var/tmp/vl.XX was created); and
that 'qemu-nbd -l snap a' lets me read the snapshot data. But mixing the two
fails, and it would be a nice bug to fix.


I briefly looked at the code and is seams to be using the same base
functions as qemu does.  So, if I get this working for the model it
might also start working for qemu-nbd.





Ideally, I want to not modify old images or create new images with
qemu-img, so I have been not modifing qemu-img, but qemu directly
itself.  My use case will have several snapshots in an image.(say
100).  I will then later resume each of these snapshots in a qemu
session in parallel.  This is why I have gone done the route of modifying
the temp snapshots file /var/tmp/vl.X L1 and l2 tables.  My
understanding is if these are updated and the cluster doesn't exists in
the temp file the code will then look for it in the backing file.  Still
researching this area.


Right now, the only thing that qemu reads from a backing file is a guest 
cluster. L1/L2 clusters have to be local to the file that they are 
describing (there is no way to make an L2 table fall back to the 
contents of a different cluster in the backing file).  It boils down to:


Reads:
Does the active layer have an L2 mapping for the current cluster being 
read?  Yes - read that cluster. No - ask the backing layer to provide 
the contents of that cluster (and if copy-on-read is enabled, also write 
those contents in a fresh allocation so that the current layer no longer 
has to defer to the backing).


Writes:
Does the active layer have an L2 mapping for the current cluster 
containing the data being written? Yes - modify that cluster in place. 
No - allocate an new cluster, and if the write was for less than a full 
cluster, also ask the backing layer to provide the contents of the rest 
of the cluster for a copy-on-write action. After the write, the current 
layer no longer has to defer to the backing.


Creating an arbitrary qcow2 file on top of any arbitrary read-only 
backing layer (including 'qemu-nbd -l snap image) should be doable, even 
if verbose (since the "backing file" of a qcow2 BDS node can be any 
other BDS).  Providing some shorter command lines, like making 'qemu-nbd 
-s -l snap image' work so that you don't have to provide your own manual 
overlay, is thus not a high priority.






I still don't have this working yet and I believe my area of problems is
qcow2_update_snapshot_refcount.  Can anyone explain what this does
exactly.  It seems the function does three different things based on the
value of addend, either -1, 0, 1, but its somewhat unclear.


Every cluster of qcow2 is reference-counted, to track which portions of the
file are (supposed to be) in use according to following the metadata trails.
When internal snapshots are used, this is implemented by incrementing the
refcount for each cluster that is reachable both from the snapshot and from
the current L1 table (update_snapshot_refcount +1), then when writing to the
cluster we break the reference count by writing the new data to a new
allocation and decrementing the reference count of the old cluster. When
trimming clusters, we decrement the refcount, and if it goes to 0 the
cluster can be reused for something else.


I think I understand this.  That would satifys addend being a -1 or 1.
I am still unclear why you would call the fuction with addend being 0.


An addend of 0 allows a couple of callers to temporarily have an 
inconsistent image for the sake of optimizing a bulk allocation/freeing, 
followed by informing the refcount table to match, with fewer changes to 
the cluster containing the refcounts than if the algorithm had to 
accurately use -1/+1 on a per-cluster basis.


--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH] qemu: avoid memory leak while remove disk

[Michael S. Tsirkin]

On Fri, Dec 07, 2018 at 09:53:06AM +0800, wangjian wrote:
> Memset vhost_dev to zero in the vhost_dev_cleanup function.
> This causes dev.vqs to be NULL, so that
> vqs does not free up space when calling the g_free function.
> This will result in a memory leak. But you can't release vqs
> directly in the vhost_dev_cleanup function, because vhost_net
> will also call this function, and vhost_net's vqs is assigned by array.
> In order to solve this problem, we first save the pointer of vqs,
> and release the space of vqs after vhost_dev_cleanup is called.
> 
> Signed-off-by: Jian Wang 

The patch does not seem to apply.
I suspect it was corrupted by your mailer, judging by
the fact that you also sent the patch in HTML format.

Can you please fix and repost?
Thanks!


> ---
>  hw/block/vhost-user-blk.c | 7 +--
>  hw/scsi/vhost-scsi.c  | 3 ++-
>  hw/scsi/vhost-user-scsi.c | 3 ++-
>  3 files changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
> index 1451940..c3af28f 100644
> --- a/hw/block/vhost-user-blk.c
> +++ b/hw/block/vhost-user-blk.c
> @@ -250,6 +250,7 @@ static void vhost_user_blk_device_realize(DeviceState 
> *dev, Error **errp)
>  VirtIODevice *vdev = VIRTIO_DEVICE(dev);
>  VHostUserBlk *s = VHOST_USER_BLK(vdev);
>  VhostUserState *user;
> +struct vhost_virtqueue *vqs = NULL;
>  int i, ret;
> 
>  if (!s->chardev.chr) {
> @@ -288,6 +289,7 @@ static void vhost_user_blk_device_realize(DeviceState 
> *dev, Error **errp)
>  s->dev.vqs = g_new(struct vhost_virtqueue, s->dev.nvqs);
>  s->dev.vq_index = 0;
>  s->dev.backend_features = 0;
> +vqs = s->dev.vqs;
> 
>  vhost_dev_set_config_notifier(>dev, _ops);
> 
> @@ -314,7 +316,7 @@ static void vhost_user_blk_device_realize(DeviceState 
> *dev, Error **errp)
>  vhost_err:
>  vhost_dev_cleanup(>dev);
>  virtio_err:
> -g_free(s->dev.vqs);
> +g_free(vqs);
>  virtio_cleanup(vdev);
> 
>  vhost_user_cleanup(user);
> @@ -326,10 +328,11 @@ static void vhost_user_blk_device_unrealize(DeviceState 
> *dev, Error **errp)
>  {
>  VirtIODevice *vdev = VIRTIO_DEVICE(dev);
>  VHostUserBlk *s = VHOST_USER_BLK(dev);
> +struct vhost_virtqueue *vqs = s->dev.vqs;
> 
>  vhost_user_blk_set_status(vdev, 0);
>  vhost_dev_cleanup(>dev);
> -g_free(s->dev.vqs);
> +g_free(vqs);
>  virtio_cleanup(vdev);
> 
>  if (s->vhost_user) {
> diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c
> index 7f21b4f..61e2e57 100644
> --- a/hw/scsi/vhost-scsi.c
> +++ b/hw/scsi/vhost-scsi.c
> @@ -215,6 +215,7 @@ static void vhost_scsi_unrealize(DeviceState *dev, Error 
> **errp)
>  {
>  VirtIODevice *vdev = VIRTIO_DEVICE(dev);
>  VHostSCSICommon *vsc = VHOST_SCSI_COMMON(dev);
> +struct vhost_virtqueue *vqs = vsc->dev.vqs;
> 
>  migrate_del_blocker(vsc->migration_blocker);
>  error_free(vsc->migration_blocker);
> @@ -223,7 +224,7 @@ static void vhost_scsi_unrealize(DeviceState *dev, Error 
> **errp)
>  vhost_scsi_set_status(vdev, 0);
> 
>  vhost_dev_cleanup(>dev);
> -g_free(vsc->dev.vqs);
> +g_free(vqs);
> 
>  virtio_scsi_common_unrealize(dev, errp);
>  }
> diff --git a/hw/scsi/vhost-user-scsi.c b/hw/scsi/vhost-user-scsi.c
> index 2e1ba4a..6728878 100644
> --- a/hw/scsi/vhost-user-scsi.c
> +++ b/hw/scsi/vhost-user-scsi.c
> @@ -121,12 +121,13 @@ static void vhost_user_scsi_unrealize(DeviceState *dev, 
> Error **errp)
>  VirtIODevice *vdev = VIRTIO_DEVICE(dev);
>  VHostUserSCSI *s = VHOST_USER_SCSI(dev);
>  VHostSCSICommon *vsc = VHOST_SCSI_COMMON(s);
> +struct vhost_virtqueue *vqs = vsc->dev.vqs;
> 
>  /* This will stop the vhost backend. */
>  vhost_user_scsi_set_status(vdev, 0);
> 
>  vhost_dev_cleanup(>dev);
> -g_free(vsc->dev.vqs);
> +g_free(vqs);
> 
>  virtio_scsi_common_unrealize(dev, errp);
> 
> --
> 1.8.3.1
> 

[Qemu-devel] [PATCH] target/arm: Convert ARM_TBFLAG_* to FIELDs

[Richard Henderson]

Use "register" TBFLAG_ANY to indicate shared state between
A32 and A64, and "registers" TBFLAG_A32 & TBFLAG_A64 for
fields that are specific to the given cpu state.

Move ARM_TBFLAG_BE to shared state, instead of its current
placement within "Bit usage when in AArch32 state".

Signed-off-by: Richard Henderson 
---
 target/arm/cpu.h   | 107 -
 target/arm/helper.c|  49 -
 target/arm/translate-a64.c |  24 +
 target/arm/translate.c |  40 +++---
 4 files changed, 79 insertions(+), 141 deletions(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 1e22a9a8bd..6211e21046 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -2954,106 +2954,41 @@ static inline bool 
arm_cpu_data_is_big_endian(CPUARMState *env)
  * We put flags which are shared between 32 and 64 bit mode at the top
  * of the word, and flags which apply to only one mode at the bottom.
  */
-#define ARM_TBFLAG_AARCH64_STATE_SHIFT 31
-#define ARM_TBFLAG_AARCH64_STATE_MASK  (1U << ARM_TBFLAG_AARCH64_STATE_SHIFT)
-#define ARM_TBFLAG_MMUIDX_SHIFT 28
-#define ARM_TBFLAG_MMUIDX_MASK (0x7 << ARM_TBFLAG_MMUIDX_SHIFT)
-#define ARM_TBFLAG_SS_ACTIVE_SHIFT 27
-#define ARM_TBFLAG_SS_ACTIVE_MASK (1 << ARM_TBFLAG_SS_ACTIVE_SHIFT)
-#define ARM_TBFLAG_PSTATE_SS_SHIFT 26
-#define ARM_TBFLAG_PSTATE_SS_MASK (1 << ARM_TBFLAG_PSTATE_SS_SHIFT)
+FIELD(TBFLAG_ANY, AARCH64_STATE, 31, 1)
+FIELD(TBFLAG_ANY, MMUIDX, 28, 3)
+FIELD(TBFLAG_ANY, SS_ACTIVE, 27, 1)
+FIELD(TBFLAG_ANY, PSTATE_SS, 26, 1)
 /* Target EL if we take a floating-point-disabled exception */
-#define ARM_TBFLAG_FPEXC_EL_SHIFT 24
-#define ARM_TBFLAG_FPEXC_EL_MASK (0x3 << ARM_TBFLAG_FPEXC_EL_SHIFT)
+FIELD(TBFLAG_ANY, FPEXC_EL, 24, 2)
+FIELD(TBFLAG_ANY, BE, 23, 1)
 
 /* Bit usage when in AArch32 state: */
-#define ARM_TBFLAG_THUMB_SHIFT  0
-#define ARM_TBFLAG_THUMB_MASK   (1 << ARM_TBFLAG_THUMB_SHIFT)
-#define ARM_TBFLAG_VECLEN_SHIFT 1
-#define ARM_TBFLAG_VECLEN_MASK  (0x7 << ARM_TBFLAG_VECLEN_SHIFT)
-#define ARM_TBFLAG_VECSTRIDE_SHIFT  4
-#define ARM_TBFLAG_VECSTRIDE_MASK   (0x3 << ARM_TBFLAG_VECSTRIDE_SHIFT)
-#define ARM_TBFLAG_VFPEN_SHIFT  7
-#define ARM_TBFLAG_VFPEN_MASK   (1 << ARM_TBFLAG_VFPEN_SHIFT)
-#define ARM_TBFLAG_CONDEXEC_SHIFT   8
-#define ARM_TBFLAG_CONDEXEC_MASK(0xff << ARM_TBFLAG_CONDEXEC_SHIFT)
-#define ARM_TBFLAG_SCTLR_B_SHIFT16
-#define ARM_TBFLAG_SCTLR_B_MASK (1 << ARM_TBFLAG_SCTLR_B_SHIFT)
+FIELD(TBFLAG_A32, THUMB, 0, 1)
+FIELD(TBFLAG_A32, VECLEN, 1, 3)
+FIELD(TBFLAG_A32, VECSTRIDE, 4, 2)
+FIELD(TBFLAG_A32, VFPEN, 7, 1)
+FIELD(TBFLAG_A32, CONDEXEC, 8, 8)
+FIELD(TBFLAG_A32, SCTLR_B, 16, 1)
 /* We store the bottom two bits of the CPAR as TB flags and handle
  * checks on the other bits at runtime
  */
-#define ARM_TBFLAG_XSCALE_CPAR_SHIFT 17
-#define ARM_TBFLAG_XSCALE_CPAR_MASK (3 << ARM_TBFLAG_XSCALE_CPAR_SHIFT)
+FIELD(TBFLAG_A32, XSCALE_CPAR, 17, 2)
 /* Indicates whether cp register reads and writes by guest code should access
  * the secure or nonsecure bank of banked registers; note that this is not
  * the same thing as the current security state of the processor!
  */
-#define ARM_TBFLAG_NS_SHIFT 19
-#define ARM_TBFLAG_NS_MASK  (1 << ARM_TBFLAG_NS_SHIFT)
-#define ARM_TBFLAG_BE_DATA_SHIFT20
-#define ARM_TBFLAG_BE_DATA_MASK (1 << ARM_TBFLAG_BE_DATA_SHIFT)
+FIELD(TBFLAG_A32, NS, 19, 1)
 /* For M profile only, Handler (ie not Thread) mode */
-#define ARM_TBFLAG_HANDLER_SHIFT21
-#define ARM_TBFLAG_HANDLER_MASK (1 << ARM_TBFLAG_HANDLER_SHIFT)
+FIELD(TBFLAG_A32, HANDLER, 21, 1)
 /* For M profile only, whether we should generate stack-limit checks */
-#define ARM_TBFLAG_STACKCHECK_SHIFT 22
-#define ARM_TBFLAG_STACKCHECK_MASK  (1 << ARM_TBFLAG_STACKCHECK_SHIFT)
+FIELD(TBFLAG_A32, STACKCHECK, 22, 1)
 
 /* Bit usage when in AArch64 state */
-#define ARM_TBFLAG_TBI0_SHIFT 0/* TBI0 for EL0/1 or TBI for EL2/3 */
-#define ARM_TBFLAG_TBI0_MASK (0x1ull << ARM_TBFLAG_TBI0_SHIFT)
-#define ARM_TBFLAG_TBI1_SHIFT 1/* TBI1 for EL0/1  */
-#define ARM_TBFLAG_TBI1_MASK (0x1ull << ARM_TBFLAG_TBI1_SHIFT)
-#define ARM_TBFLAG_SVEEXC_EL_SHIFT  2
-#define ARM_TBFLAG_SVEEXC_EL_MASK   (0x3 << ARM_TBFLAG_SVEEXC_EL_SHIFT)
-#define ARM_TBFLAG_ZCR_LEN_SHIFT4
-#define ARM_TBFLAG_ZCR_LEN_MASK (0xf << ARM_TBFLAG_ZCR_LEN_SHIFT)
-#define ARM_TBFLAG_PAUTH_ACTIVE_SHIFT  8
-#define ARM_TBFLAG_PAUTH_ACTIVE_MASK   (1ull << ARM_TBFLAG_PAUTH_ACTIVE_SHIFT)
-
-/* some convenience accessor macros */
-#define ARM_TBFLAG_AARCH64_STATE(F) \
-(((F) & ARM_TBFLAG_AARCH64_STATE_MASK) >> ARM_TBFLAG_AARCH64_STATE_SHIFT)
-#define ARM_TBFLAG_MMUIDX(F) \
-(((F) & ARM_TBFLAG_MMUIDX_MASK) >> ARM_TBFLAG_MMUIDX_SHIFT)
-#define ARM_TBFLAG_SS_ACTIVE(F) \
-(((F) & ARM_TBFLAG_SS_ACTIVE_MASK) >> ARM_TBFLAG_SS_ACTIVE_SHIFT)
-#define ARM_TBFLAG_PSTATE_SS(F) \
-(((F) & ARM_TBFLAG_PSTATE_SS_MASK) >> ARM_TBFLAG_PSTATE_SS_SHIFT)
-#define ARM_TBFLAG_FPEXC_EL(F) \
-   

Re: [Qemu-devel] [PATCH 2/2] avoid TABs in files that only contain a few

[Eric Blake]

On 12/13/18 4:37 PM, Paolo Bonzini wrote:

Most files that have TABs only contain a handful of them.  Change
them to spaces so that we don't confuse people.

disas, standard-headers, linux-headers and libdecnumber are imported
from other projects and probably should be exempted from the check.
Outside those, after this patch the following files still contain both
8-space and TAB sequences at the beginning of the line.  Many of them
have a majority of TABs, or were initially committed with all tabs.




Signed-off-by: Paolo Bonzini 
---



  nbd/client.c   |  2 +-


NBD part:
Acked-by: Eric Blake 

but I have patches that remove the line in question entirely as part of 
adding 'qemu-nbd --list'.


--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [RFC v2 05/38] plugin: add user-facing API

[Emilio G. Cota]

On Fri, Dec 14, 2018 at 18:47:42 +, Aaron Lindsay wrote:
> On Dec 14 12:50, Emilio G. Cota wrote:
> > On Fri, Dec 14, 2018 at 12:08:22 -0500, Emilio G. Cota wrote:
> > > On Fri, Dec 14, 2018 at 15:57:32 +, Aaron Lindsay wrote:
> > (snip)
> > > > I added a function to the user-facing plugin API in my own version of
> > > > Pavel's plugin patchset to clear all existing plugin instrumentation,
> > (snip)
> > > I think the following API call would do what you need:
> > > 
> > >   typedef int (*qemu_plugin_reset_cb_t)(qemu_plugin_id_t id);
> > >   void qemu_plugin_reset(qemu_plugin_id_t id, qemu_plugin_reset_cb_t cb);
> > > 
> > > (or maybe qemu_plugin_reinstall?)
> > 
> > An alternative is to track the TBs that a plugin has inserted
> > instrumentation into, and only flush those. This would require
> > us to do an additional hash table insert when adding a
> > direct callback, but it allow us to avoid exporting tb_flush indirectly
> > to plugins, which could be abused by malicious plugins to perform
> > a DoS attack.
> 
> I don't think I have a preference. Though now I'm curious - when/why do
> you expect users might run plugins that could be malicious in this way?

When this work started, others expressed concern about potentially
malicious plugins, although I think they were thinking of preventing
plugins from gaining access to data structures that could affect
the guest, e.g. CPUState. That's why I'm using a hash table for
the plugin context id, although that's probably overkill (do we
care about malicious plugins messing with the subscriptions of
other plugins? Not sure we should.)

Here I'm thinking more about giving plugin developers too much rope to
hang themselves with, e.g. calling repeatedly plugin_reset() out
of convenience, without realising the performance impact that
it entails. That's why I'd like to explore the alternative.

Emilio

[Qemu-devel] [PATCH v1 1/1] target/riscv/pmp.c: Fix pmp_decode_napot()

[Alistair Francis]

From: Anup Patel 

Currently, start and end address of a PMP region are not decoded
correctly by pmp_decode_napot().

Let's say we have a 128KB PMP region with base address as 0x8000.
Now, the PMPADDRx CSR value for this region will be 0x20003fff.

The current pmp_decode_napot() implementation will decode PMPADDRx
CSR as t1=14, base=0x1, and range=0x1 whereas it should
have decoded PMPADDRx CSR as t1=14, base=0x8000, and range=0x1fff.

This patch fixes the base value decoding in pmp_decode_napot() when
PMPADDRx CSR is not -1 (i.e. 0x).

Signed-off-by: Anup Patel 
Signed-off-by: Anup Patel 
Signed-off-by: Alistair Francis 
---
 target/riscv/pmp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
index 03abd8fe5e..15a5366616 100644
--- a/target/riscv/pmp.c
+++ b/target/riscv/pmp.c
@@ -138,7 +138,7 @@ static void pmp_decode_napot(target_ulong a, target_ulong 
*sa, target_ulong *ea)
 return;
 } else {
 target_ulong t1 = ctz64(~a);
-target_ulong base = (a & ~(((target_ulong)1 << t1) - 1)) << 3;
+target_ulong base = (a & ~(((target_ulong)1 << t1) - 1)) << 2;
 target_ulong range = ((target_ulong)1 << (t1 + 3)) - 1;
 *sa = base;
 *ea = base + range;
-- 
2.19.1

Re: [Qemu-devel] [PULL 0/3] 9p patches 2018-12-13

[Peter Maydell]

On Thu, 13 Dec 2018 at 09:03, Greg Kurz  wrote:
>
> The following changes since commit bb9bf94b3e8926553290bc9a7cb84315af422086:
>
>   Merge remote-tracking branch 
> 'remotes/ehabkost/tags/machine-next-pull-request' into staging (2018-12-11 
> 19:18:58 +)
>
> are available in the Git repository at:
>
>   https://github.com/gkurz/qemu.git tags/for-upstream
>
> for you to fetch changes up to 93aee84f575d46699f49af3c96194012527e0b22:
>
>   9p: remove support for the "handle" backend (2018-12-12 14:18:10 +0100)
>
> 
> Most notable change in this PR is the full removal of the "handle" fsdev
> backend.
>
> 
> Greg Kurz (3):
>   9p: use g_new(T, n) instead of g_malloc(sizeof(T) * n)
>   xen/9pfs: use g_new(T, n) instead of g_malloc(sizeof(T) * n)
>   9p: remove support for the "handle" backend

Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/4.0
for any user-visible changes.

-- PMM

Re: [Qemu-devel] [RFC v2 05/38] plugin: add user-facing API

[Aaron Lindsay]

On Dec 14 12:50, Emilio G. Cota wrote:
> On Fri, Dec 14, 2018 at 12:08:22 -0500, Emilio G. Cota wrote:
> > On Fri, Dec 14, 2018 at 15:57:32 +, Aaron Lindsay wrote:
> (snip)
> > > I added a function to the user-facing plugin API in my own version of
> > > Pavel's plugin patchset to clear all existing plugin instrumentation,
> (snip)
> > I think the following API call would do what you need:
> > 
> >   typedef int (*qemu_plugin_reset_cb_t)(qemu_plugin_id_t id);
> >   void qemu_plugin_reset(qemu_plugin_id_t id, qemu_plugin_reset_cb_t cb);
> > 
> > (or maybe qemu_plugin_reinstall?)
> 
> An alternative is to track the TBs that a plugin has inserted
> instrumentation into, and only flush those. This would require
> us to do an additional hash table insert when adding a
> direct callback, but it allow us to avoid exporting tb_flush indirectly
> to plugins, which could be abused by malicious plugins to perform
> a DoS attack.

I don't think I have a preference. Though now I'm curious - when/why do
you expect users might run plugins that could be malicious in this way?

-Aaron

Re: [Qemu-devel] [RFC v2 05/38] plugin: add user-facing API

[Emilio G. Cota]

On Fri, Dec 14, 2018 at 17:59:20 +, Aaron Lindsay wrote:
> On Dec 14 12:08, Emilio G. Cota wrote:
(snip)
> > The idea is that a plugin can "reset" itself, so that (1) all
> > its CBs are cleared and (2) the plugin can register new callbacks.
> > This would all happen in an atomic context (no vCPU running), so
> > that the plugin would miss no CPU events.
> 
> The implication being that there would not be the same possibility of
> other callbacks being called between when qemu_plugin_reset and the
> qemu_plugin_reset_cb_t callback are called as there is at plugin
> un-installation time?

The callback is needed for the same reason -- we can only guarantee
that there will be no callbacks once the current RCU read critical section
expires.

> > How does this sound?
> 
> I think what you describe is exactly what I'm interested in.

Nice. I'll work on this for v3.

Thanks,

Emilio

Re: [Qemu-devel] [PATCH v2] scripts: add script to convert multiline comments into 4-line format

[Wainer dos Santos Moschetta]

On 12/14/2018 04:15 PM, Paolo Bonzini wrote:

On 14/12/18 19:06, Wainer dos Santos Moschetta wrote:

IIUC above block handles the lines between lead and trail. So it would
fix (but it doesn't) this:

# cat foo
/*
  comment 1
  comment 2
  */

# scripts/fix-multiline-comments.sh foo
/*
  comment 1
  comment 2
  */

Not enough spaces :)  It would fix

  /*
 comment 1
 comment 2
   */

or what is actually found in QEMU:

/* comment 1
   comment 2 */


I tried hard but I failed to find problems. :)

Thus,

Reviewed-by: Wainer dos Santos Moschetta 




Thanks,

Paolo

Re: [Qemu-devel] [PATCH v2] scripts: add script to convert multiline comments into 4-line format

[Paolo Bonzini]

On 14/12/18 19:06, Wainer dos Santos Moschetta wrote:
> 
> IIUC above block handles the lines between lead and trail. So it would
> fix (but it doesn't) this:
> 
> # cat foo
> /*
>  comment 1
>  comment 2
>  */
> 
> # scripts/fix-multiline-comments.sh foo
> /*
>  comment 1
>  comment 2
>  */

Not enough spaces :)  It would fix

 /*
comment 1
comment 2
  */

or what is actually found in QEMU:

   /* comment 1
  comment 2 */

Thanks,

Paolo

Re: [Qemu-devel] [PATCH] .shippable.yml: disable the win cross tests

[Philippe Mathieu-Daudé]

Le ven. 14 déc. 2018 16:17, Alex Bennée  a écrit :

> The pkg.mxe.cc package repositories have been down for the last two
> weeks causing the builds to fail when shippable re-builds the
> containers.
>
> This is really just a sticking plaster until we can get our own docker
> hub images properly setup so we can avoid having dependencies on
> external repos.
>

I have few WiP branches on this topic but it is taking time...
Meanwhile:
Acked-by: Philippe Mathieu-Daudé 
Maybe Peter can directly apply this as a build fix.


> Signed-off-by: Alex Bennée 
> Cc: Philippe Mathieu-Daudé 
> ---
>  .shippable.yml | 9 +
>  1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/.shippable.yml b/.shippable.yml
> index f74a3de3ff..f2ffef21d1 100644
> --- a/.shippable.yml
> +++ b/.shippable.yml
> @@ -7,10 +7,11 @@ env:
>matrix:
>  - IMAGE=debian-amd64
>TARGET_LIST=x86_64-softmmu,x86_64-linux-user
> -- IMAGE=debian-win32-cross
> -  TARGET_LIST=arm-softmmu,i386-softmmu,lm32-softmmu
> -- IMAGE=debian-win64-cross
> -  TARGET_LIST=aarch64-softmmu,sparc64-softmmu,x86_64-softmmu
> +# currently disabled as the mxe.cc repos are down
> +# - IMAGE=debian-win32-cross
> +#   TARGET_LIST=arm-softmmu,i386-softmmu,lm32-softmmu
> +# - IMAGE=debian-win64-cross
> +#   TARGET_LIST=aarch64-softmmu,sparc64-softmmu,x86_64-softmmu
>  - IMAGE=debian-armel-cross
>TARGET_LIST=arm-softmmu,arm-linux-user,armeb-linux-user
>  - IMAGE=debian-armhf-cross
> --
> 2.17.1
>
>

Re: [Qemu-devel] [PATCH v2] scripts: add script to convert multiline comments into 4-line format

[Wainer dos Santos Moschetta]

On 12/14/2018 07:48 AM, Paolo Bonzini wrote:

Since we're adding checkpatch rules to enforce 4-line multiline comment
format, i.e. with lone /* and */, this script can be run on existing
code so that the comment style does not become inconsistent within a
file.

The alternative to awk-in-a-shell-script could be Perl, which also
supports -i directly, but a2p seems to have bitrotten and I didn't quite
feel like writing this twice...

Signed-off-by: Paolo Bonzini 
---
 v1->v2: fix handling of multiline doc comments without lone /**
---
  scripts/fix-multiline-comments.sh | 62 +++
  1 file changed, 62 insertions(+)
  create mode 100755 scripts/fix-multiline-comments.sh

diff --git a/scripts/fix-multiline-comments.sh 
b/scripts/fix-multiline-comments.sh
new file mode 100755
index 00..93f9b10669
--- /dev/null
+++ b/scripts/fix-multiline-comments.sh
@@ -0,0 +1,62 @@
+#! /bin/sh
+#
+# Fix multiline comments to match CODING_STYLE
+#
+# Copyright (C) 2018 Red Hat, Inc.
+#
+# Author: Paolo Bonzini
+#
+# Usage: scripts/fix-multiline-comments.sh [-i] FILE...
+#
+# -i edits the file in place (requires gawk 4.1.0).
+#
+# Set the AWK environment variable to choose the awk interpreter to use
+# (default 'awk')
+
+if test "$1" = -i; then
+  # gawk extension
+  inplace="-i inplace"
+  shift
+fi
+${AWK-awk} $inplace 'BEGIN { indent = -1 }
+{
+line = $0
+# apply a star to the indent on lines after the first
+if (indent != -1) {
+if (line == "") {
+line = sp " *"
+} else if (substr(line, 1, indent + 2) == sp "  ") {
+line = sp " *" substr(line, indent + 3)
+}
+}


IIUC above block handles the lines between lead and trail. So it would 
fix (but it doesn't) this:


# cat foo
/*
 comment 1
 comment 2
 */

# scripts/fix-multiline-comments.sh foo
/*
 comment 1
 comment 2
 */

- Wainer


+
+is_lead = (line ~ /^[ \t]*\/\*/)
+is_trail = (line ~ /\*\//)
+if (is_lead && !is_trail) {
+# grab the indent at the start of a comment, but not for
+# single-line comments
+match(line, /^[ \t]*\/\*/)
+indent = RLENGTH - 2
+sp = substr(line, 1, indent)
+}
+
+# the regular expression filters out lone /*, /**, or */
+if (indent != -1 && !(line ~ /^[ \t]*(\/\*+|\*\/)[ \t]*$/)) {
+if (is_lead) {
+# split the leading /* or /** on a separate line
+match(line, /^[ \t]*\/\*+/)
+lead = substr(line, 1, RLENGTH)
+match(line, /^[ \t]*\/\*+[ \t]*/)
+line = lead "\n" sp " *" substr(line, RLENGTH)
+}
+if (is_trail) {
+# split the trailing */ on a separate line
+match(line, /[ \t]*\*\//)
+line = substr(line, 1, RSTART - 1) "\n" sp " */"
+}
+}
+if (is_trail) {
+indent = -1
+}
+print line
+}' "$@"

[Qemu-devel] [Bug 1808565] [NEW] Reading /proc/self/task//maps is not remapped to the target

[Alan Jones]

Public bug reported:

Seeing this in qemu-user 3.1.0

The code in is_proc_myself which supports remapping of /proc/self/maps
and /proc//maps does not support remapping of
/proc/self/task//maps or /proc//task//maps. Extending
is_proc_myself to cover these cases causes the maps to be rewritten
correctly.

These are useful in multithreaded programs to avoid freezing the entire
program to capture the maps for a single tid.

** Affects: qemu
 Importance: Undecided
 Status: New

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

Title:
  Reading /proc/self/task//maps is not remapped to the target

Status in QEMU:
  New

Bug description:
  Seeing this in qemu-user 3.1.0

  The code in is_proc_myself which supports remapping of /proc/self/maps
  and /proc//maps does not support remapping of
  /proc/self/task//maps or /proc//task//maps. Extending
  is_proc_myself to cover these cases causes the maps to be rewritten
  correctly.

  These are useful in multithreaded programs to avoid freezing the
  entire program to capture the maps for a single tid.

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

Re: [Qemu-devel] [RFC v2 05/38] plugin: add user-facing API

[Aaron Lindsay]

On Dec 14 12:08, Emilio G. Cota wrote:
> On Fri, Dec 14, 2018 at 15:57:32 +, Aaron Lindsay wrote:
> > Emilio,
> > 
> > First, thanks for putting this together - I think everyone doing this
> > sort of thing will benefit if we're able to agree on one upstream plugin
> > interface.
> > 
> > One thing I'd like to see is support for unregistering callbacks once
> > they are registered. For instance, you can imagine that a plugin may
> > have 'modality', where it may care about tracing very detailed
> > information in one mode, but trace only limited information otherwise -
> > perhaps only enough to figure out when it needs to switch back to the
> > other mode.
> > 
> > I added a function to the user-facing plugin API in my own version of
> > Pavel's plugin patchset to clear all existing plugin instrumentation,
> > allowing the plugin to drop instrumentation if it was no longer
> > interested. Of course, you could always add logic to a plugin to ignore
> > unwanted callbacks, but it could be significantly more efficient to
> > remove them entirely. In Pavel's patchset, this was essentially just a
> > call to tb_flush(), though I think it would be a little more involved
> > for yours.
> 
> This is a good point.
> 
> "Regular" callbacks can be unregistered by just passing NULL as the
> callback (see plugin_unregister_cb__locked, which is called from
> do_plugin_register_cb).

Ah, okay, thanks - I missed this detail when looking through your
patches earlier.

> "Direct" callbacks, however (i.e. the ones
> that embed a callback directly in the translated code),
> would have to be unregistered by flushing the particular TB
> (or all TBs, which is probably better from an API viewpoint).
> 
> I think the following API call would do what you need:
> 
>   typedef int (*qemu_plugin_reset_cb_t)(qemu_plugin_id_t id);
>   void qemu_plugin_reset(qemu_plugin_id_t id, qemu_plugin_reset_cb_t cb);
> 
> (or maybe qemu_plugin_reinstall?)

I think I prefer your initial suggestion of qemu_plugin_reset - to me it
does a better job communicating that existing callbacks will be removed.
But, then again, _reinstall makes it more clear that you are responsible
for re-adding any callbacks you still want...

> The idea is that a plugin can "reset" itself, so that (1) all
> its CBs are cleared and (2) the plugin can register new callbacks.
> This would all happen in an atomic context (no vCPU running), so
> that the plugin would miss no CPU events.

The implication being that there would not be the same possibility of
other callbacks being called between when qemu_plugin_reset and the
qemu_plugin_reset_cb_t callback are called as there is at plugin
un-installation time?

> How does this sound?

I think what you describe is exactly what I'm interested in.

-Aaron

Re: [Qemu-devel] [RFC v2 05/38] plugin: add user-facing API

[Emilio G. Cota]

On Fri, Dec 14, 2018 at 12:08:22 -0500, Emilio G. Cota wrote:
> On Fri, Dec 14, 2018 at 15:57:32 +, Aaron Lindsay wrote:
(snip)
> > I added a function to the user-facing plugin API in my own version of
> > Pavel's plugin patchset to clear all existing plugin instrumentation,
(snip)
> I think the following API call would do what you need:
> 
>   typedef int (*qemu_plugin_reset_cb_t)(qemu_plugin_id_t id);
>   void qemu_plugin_reset(qemu_plugin_id_t id, qemu_plugin_reset_cb_t cb);
> 
> (or maybe qemu_plugin_reinstall?)

An alternative is to track the TBs that a plugin has inserted
instrumentation into, and only flush those. This would require
us to do an additional hash table insert when adding a
direct callback, but it allow us to avoid exporting tb_flush indirectly
to plugins, which could be abused by malicious plugins to perform
a DoS attack.

I'll look into this.

E.

Re: [Qemu-devel] [PULL 00/27] ppc-for-4.0 queue 20181213

[Cédric Le Goater]

On 12/14/18 5:03 PM, Peter Maydell wrote:
> On Thu, 13 Dec 2018 at 04:01, David Gibson  
> wrote:
>>
>> The following changes since commit 4b3aab204204ca742836219b97b538d90584f4f2:
>>
>>   Merge remote-tracking branch 
>> 'remotes/vivier2/tags/trivial-patches-pull-request' into staging (2018-12-11 
>> 22:26:44 +)
>>
>> are available in the Git repository at:
>>
>>   git://github.com/dgibson/qemu.git tags/ppc-for-4.0-20181213
>>
>> for you to fetch changes up to 67888a17b6683600ae3fa64ca275c737ba8a9a45:
>>
>>   spapr/xive: use the VCPU id as a NVT identifier (2018-12-13 09:44:04 +1100)
>>
>> 
>> ppc patch queue 2018-12-13
>>
>> Here's the first ppc and spapr pull request for 4.0.  Highlights are:
>>
>>  * The start of support for the POWER9 "XIVE" interrupt controller
>>(not complete enough to use yet, but we're getting there)
>>  * A number of g_new vs. g_malloc cleanups
>>  * Some IRQ wiring cleanups
>>  * A fix for how we advertise NUMA nodes to the guest for pseries
>>
>> ---
> 
> 
> Compile errors in the windows cross-build. 

are you compiling with the mingw64-* packages ? as documented in :

https://wiki.qemu.org/Hosts/W32

> These look like
> they're assumptions that "long" is 64 bits, which it is not on Windows.
> For instance the PPC_BIT macro should be using the ULL suffix, not UL
> ("UL" is almost always a bug: either the constant is 32-bit, in
> which case "U" is what you want, or it's 64-bit and you need "ULL").

ok. These definitions come from our skiboot firmware which I wanted 
to keep as it was. It seems I will need to adapt.

> Using __builtin_ffsl() directly in target/ppc/cpu.h also looks
> a bit dubious -- this should be rephrased to use ctz32() or ctz64()
> instead.

ok. I will work on a fix.

Thanks,

C.


> In file included from /home/petmay01/qemu-for-merges/hw/intc/xive.c:13:0:
> /home/petmay01/qemu-for-merges/hw/intc/xive.c: In function 
> 'xive_router_notify':
> /home/petmay01/qemu-for-merges/target/ppc/cpu.h:76:33: error: overflow
> in implicit constant conversion [-Werror=overflow]
>  #define PPC_BITMASK(bs, be) ((PPC_BIT(bs) - PPC_BIT(be)) | PPC_BIT(bs))
>  ^
> /home/petmay01/qemu-for-merges/target/ppc/cpu.h:84:50: note: in
> definition of macro 'MASK_TO_LSH'
>  # define MASK_TO_LSH(m)  (__builtin_ffsl(m) - 1)
>   ^
> /home/petmay01/qemu-for-merges/include/hw/ppc/xive_regs.h:129:34:
> note: in expansion of macro 'GETFIELD'
>  #define GETFIELD_BE64(m, v)  GETFIELD(m, be64_to_cpu(v))
>   ^
> /home/petmay01/qemu-for-merges/hw/intc/xive.c:1366:28: note: in
> expansion of macro 'GETFIELD_BE64'
> GETFIELD_BE64(EAS_END_BLOCK, eas.w),
> ^
> /home/petmay01/qemu-for-merges/include/hw/ppc/xive_regs.h:120:25:
> note: in expansion of macro 'PPC_BITMASK'
>  #define EAS_END_BLOCK   PPC_BITMASK(4, 7)/* Destination END block# */
>  ^
> /home/petmay01/qemu-for-merges/hw/intc/xive.c:1366:42: note: in
> expansion of macro 'EAS_END_BLOCK'
> GETFIELD_BE64(EAS_END_BLOCK, eas.w),
>   ^
> /home/petmay01/qemu-for-merges/target/ppc/cpu.h:89:46: error: right
> shift count is negative [-Werror=shift-count-negative]
>  #define GETFIELD(m, v)  (((v) & (m)) >> MASK_TO_LSH(m))
>   ^
> /home/petmay01/qemu-for-merges/include/hw/ppc/xive_regs.h:129:34:
> note: in expansion of macro 'GETFIELD'
>  #define GETFIELD_BE64(m, v)  GETFIELD(m, be64_to_cpu(v))
>   ^
> /home/petmay01/qemu-for-merges/hw/intc/xive.c:1366:28: note: in
> expansion of macro 'GETFIELD_BE64'
> GETFIELD_BE64(EAS_END_BLOCK, eas.w),
> ^
> /home/petmay01/qemu-for-merges/target/ppc/cpu.h:76:33: error: overflow
> in implicit constant conversion [-Werror=overflow]
>  #define PPC_BITMASK(bs, be) ((PPC_BIT(bs) - PPC_BIT(be)) | PPC_BIT(bs))
>  ^
> /home/petmay01/qemu-for-merges/target/ppc/cpu.h:84:50: note: in
> definition of macro 'MASK_TO_LSH'
>  # define MASK_TO_LSH(m)  (__builtin_ffsl(m) - 1)
>   ^
> /home/petmay01/qemu-for-merges/include/hw/ppc/xive_regs.h:129:34:
> note: in expansion of macro 'GETFIELD'
>  #define GETFIELD_BE64(m, v)  GETFIELD(m, be64_to_cpu(v))
>   ^
> /home/petmay01/qemu-for-merges/hw/intc/xive.c:1367:28: note: in
> expansion of macro 'GETFIELD_BE64'
> GETFIELD_BE64(EAS_END_INDEX, eas.w),
> ^
> /home/petmay01/qemu-for-merges/include/hw/ppc/xive_regs.h:121:25:
> note: in expansion of macro 

[Qemu-devel] [PATCH v5 3/3] tcg/i386: enable dynamic TLB sizing

[Emilio G. Cota]

As the following experiments show, this series is a net perf gain,
particularly for memory-heavy workloads. Experiments are run on an
Intel(R) Xeon(R) Gold 6142 CPU @ 2.60GHz.

1. System boot + shudown, debian aarch64:

- Before (v3.1.0):
 Performance counter stats for './die.sh v3.1.0' (10 runs):

   9019.797015  task-clock (msec) #0.993 CPUs utilized  
  ( +-  0.23% )
29,910,312,379  cycles#3.316 GHz
  ( +-  0.14% )
54,699,252,014  instructions  #1.83  insn per cycle 
  ( +-  0.08% )
10,061,951,686  branches  # 1115.541 M/sec  
  ( +-  0.08% )
   172,966,530  branch-misses #1.72% of all branches
  ( +-  0.07% )

   9.084039051 seconds time elapsed 
 ( +-  0.23% )

- After:
 Performance counter stats for './die.sh tlb-dyn-v5' (10 runs):

   8624.084842  task-clock (msec) #0.993 CPUs utilized  
  ( +-  0.23% )
28,556,123,404  cycles#3.311 GHz
  ( +-  0.13% )
51,755,089,512  instructions  #1.81  insn per cycle 
  ( +-  0.05% )
 9,526,513,946  branches  # 1104.641 M/sec  
  ( +-  0.05% )
   166,578,509  branch-misses #1.75% of all branches
  ( +-  0.19% )

   8.680540350 seconds time elapsed 
 ( +-  0.24% )

That is, a 4.4% perf increase.

2. System boot + shutdown, ubuntu 18.04 x86_64:

- Before (v3.1.0):
  56100.574751  task-clock (msec) #1.016 CPUs utilized  
  ( +-  4.81% )
   200,745,466,128  cycles#3.578 GHz
  ( +-  5.24% )
   431,949,100,608  instructions  #2.15  insn per cycle 
  ( +-  5.65% )
77,502,383,330  branches  # 1381.490 M/sec  
  ( +-  6.18% )
   844,681,191  branch-misses #1.09% of all branches
  ( +-  3.82% )

  55.221556378 seconds time elapsed 
 ( +-  5.01% )

- After:
  56603.419540  task-clock (msec) #1.019 CPUs utilized  
  ( +- 10.19% )
   202,217,930,479  cycles#3.573 GHz
  ( +- 10.69% )
   439,336,291,626  instructions  #2.17  insn per cycle 
  ( +- 14.14% )
80,538,357,447  branches  # 1422.853 M/sec  
  ( +- 16.09% )
   776,321,622  branch-misses #0.96% of all branches
  ( +-  3.77% )

  55.549661409 seconds time elapsed 
 ( +- 10.44% )

No improvement (within noise range). Note that for this workload,
increasing the time window too much can lead to perf degradation,
since it flushes the TLB *very* frequently.

3. x86_64 SPEC06int:

   x86_64-softmmu speedup vs. v3.1.0 for SPEC06int (test set)
Host: Intel(R) Xeon(R) Gold 6142 CPU @ 2.60GHz (Skylake)

5.5 ++
|   +-+  |
  5 |-+.+-+...tlb-dyn-v5...+-|
|   * *  |
4.5 |-+.*.*+-|
|   * *  |
  4 |-+.*.*+-|
|   * *  |
3.5 |-+.*.*+-|
|   * *  |
  3 |-+..+-+*...*.*+-|
|*  *   * *  |
2.5 |-+..*..*...*.*.+-+*...+-|
|*  *   * * *  * |
  2 |-+..*..*...*.*.*..*...+-|
|*  *   * * *  *  +-+|
1.5 |-+..*..*...*.*.*..*.*+-+.*+-+.+-|
|*  * *+-+  * *  +-+   *+-+  +-+   +-+  *  * *  * *  *   |
  1 |-+*+*++*+*++*++*+*++*+*+++-+*+*+-++*+---+++*++*+*++*+*++*+++|
|   *  * *  * *  *  * *  * *  *  * *  * *  *  * *  * *  *  * *  * *  *   |
0.5 ++
  400.perlb401.bzip403.g429445.g456.hm462.libq464.h471.omn47483.xalancbgeomean
  png: 

[Qemu-devel] [Bug 1808563] [NEW] Listing the contents of / lists QEMU_LD_PREFIX instead

[Alan Jones]

Public bug reported:

Seeing this in qemu-user version 3.1.0

Demo:
$ QEMU_LD_PREFIX=$(pwd)/usr/armv7a-cros-linux-gnueabi ../run/qemu-arm 
/tmp/coreutils --coreutils-prog=ls / 
etc  lib  usr
$ ls /
boot  etc   lib lib64   lost+found  mntroot  sbin  sys  usr
bin   dev   export  homelib32   netproc  run   tmp  var
$ ls usr/armv7a-cros-linux-gnueabi
etc  lib  usr

In strace, the openat for "/" is remapped to the directory specified in 
QEMU_LD_PREFIX:
[pid  5302] openat(AT_FDCWD, "/tmp/qemu/usr/armv7a-cros-linux-gnueabi", 
O_RDONLY|O_NONBLOCK|O_DIRECTORY|O_CLOEXEC) = 3

As an aside, if I change the code to do chdir("/"); opendir("."); it
works fine.

** Affects: qemu
 Importance: Undecided
 Status: New

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

Title:
  Listing the contents of / lists QEMU_LD_PREFIX instead

Status in QEMU:
  New

Bug description:
  Seeing this in qemu-user version 3.1.0

  Demo:
  $ QEMU_LD_PREFIX=$(pwd)/usr/armv7a-cros-linux-gnueabi ../run/qemu-arm 
/tmp/coreutils --coreutils-prog=ls / 
  etc  lib  usr
  $ ls /
  boot  etc   lib lib64   lost+found  mntroot  sbin  sys  usr
  bin   dev   export  homelib32   netproc  run   tmp  var
  $ ls usr/armv7a-cros-linux-gnueabi
  etc  lib  usr

  In strace, the openat for "/" is remapped to the directory specified in 
QEMU_LD_PREFIX:
  [pid  5302] openat(AT_FDCWD, "/tmp/qemu/usr/armv7a-cros-linux-gnueabi", 
O_RDONLY|O_NONBLOCK|O_DIRECTORY|O_CLOEXEC) = 3

  As an aside, if I change the code to do chdir("/"); opendir("."); it
  works fine.

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

[Qemu-devel] [PATCH v5 2/3] tcg: introduce dynamic TLB sizing

[Emilio G. Cota]

Disabled in all TCG backends for now.

Signed-off-by: Emilio G. Cota 
---
 include/exec/cpu-defs.h  |  48 +-
 include/exec/cpu_ldst.h  |  21 +
 tcg/aarch64/tcg-target.h |   1 +
 tcg/arm/tcg-target.h |   1 +
 tcg/i386/tcg-target.h|   1 +
 tcg/mips/tcg-target.h|   1 +
 tcg/ppc/tcg-target.h |   1 +
 tcg/s390/tcg-target.h|   1 +
 tcg/sparc/tcg-target.h   |   1 +
 tcg/tci/tcg-target.h |   1 +
 accel/tcg/cputlb.c   | 184 +--
 11 files changed, 254 insertions(+), 7 deletions(-)

diff --git a/include/exec/cpu-defs.h b/include/exec/cpu-defs.h
index 6a60f94a41..b14a00b027 100644
--- a/include/exec/cpu-defs.h
+++ b/include/exec/cpu-defs.h
@@ -67,6 +67,19 @@ typedef uint64_t target_ulong;
 #define CPU_TLB_ENTRY_BITS 5
 #endif
 
+#if TCG_TARGET_IMPLEMENTS_DYN_TLB
+#define CPU_TLB_DYN_MIN_BITS 6
+#define CPU_TLB_DYN_DEFAULT_BITS 8
+/*
+ * Assuming TARGET_PAGE_BITS==12, with 2**22 entries we can cover 2**(22+12) ==
+ * 2**34 == 16G of address space. This is roughly what one would expect a
+ * TLB to cover in a modern (as of 2018) x86_64 CPU. For instance, Intel
+ * Skylake's Level-2 STLB has 16 1G entries.
+ */
+#define CPU_TLB_DYN_MAX_BITS 22
+
+#else /* !TCG_TARGET_IMPLEMENTS_DYN_TLB */
+
 /* TCG_TARGET_TLB_DISPLACEMENT_BITS is used in CPU_TLB_BITS to ensure that
  * the TLB is not unnecessarily small, but still small enough for the
  * TLB lookup instruction sequence used by the TCG target.
@@ -98,6 +111,7 @@ typedef uint64_t target_ulong;
  NB_MMU_MODES <= 8 ? 3 : 4))
 
 #define CPU_TLB_SIZE (1 << CPU_TLB_BITS)
+#endif /* TCG_TARGET_IMPLEMENTS_DYN_TLB */
 
 typedef struct CPUTLBEntry {
 /* bit TARGET_LONG_BITS to TARGET_PAGE_BITS : virtual address
@@ -141,6 +155,18 @@ typedef struct CPUIOTLBEntry {
 MemTxAttrs attrs;
 } CPUIOTLBEntry;
 
+/**
+ * struct CPUTLBWindow
+ * @begin_ns: host time (in ns) at the beginning of the time window
+ * @max_entries: maximum number of entries observed in the window
+ *
+ * See also: tlb_mmu_resize_locked()
+ */
+typedef struct CPUTLBWindow {
+int64_t begin_ns;
+size_t max_entries;
+} CPUTLBWindow;
+
 typedef struct CPUTLBDesc {
 /*
  * Describe a region covering all of the large pages allocated
@@ -152,6 +178,10 @@ typedef struct CPUTLBDesc {
 target_ulong large_page_mask;
 /* The next index to use in the tlb victim table.  */
 size_t vindex;
+#if TCG_TARGET_IMPLEMENTS_DYN_TLB
+CPUTLBWindow window;
+size_t n_used_entries;
+#endif
 } CPUTLBDesc;
 
 /*
@@ -176,6 +206,20 @@ typedef struct CPUTLBCommon {
 size_t elide_flush_count;
 } CPUTLBCommon;
 
+#if TCG_TARGET_IMPLEMENTS_DYN_TLB
+# define CPU_TLB\
+/* tlb_mask[i] contains (n_entries - 1) << CPU_TLB_ENTRY_BITS */\
+uintptr_t tlb_mask[NB_MMU_MODES];   \
+CPUTLBEntry *tlb_table[NB_MMU_MODES];
+# define CPU_IOTLB  \
+CPUIOTLBEntry *iotlb[NB_MMU_MODES];
+#else
+# define CPU_TLB\
+CPUTLBEntry tlb_table[NB_MMU_MODES][CPU_TLB_SIZE];
+# define CPU_IOTLB  \
+CPUIOTLBEntry iotlb[NB_MMU_MODES][CPU_TLB_SIZE];
+#endif
+
 /*
  * The meaning of each of the MMU modes is defined in the target code.
  * Note that NB_MMU_MODES is not yet defined; we can only reference it
@@ -184,9 +228,9 @@ typedef struct CPUTLBCommon {
 #define CPU_COMMON_TLB \
 CPUTLBCommon tlb_c; \
 CPUTLBDesc tlb_d[NB_MMU_MODES]; \
-CPUTLBEntry tlb_table[NB_MMU_MODES][CPU_TLB_SIZE];  \
+CPU_TLB \
 CPUTLBEntry tlb_v_table[NB_MMU_MODES][CPU_VTLB_SIZE];   \
-CPUIOTLBEntry iotlb[NB_MMU_MODES][CPU_TLB_SIZE];\
+CPU_IOTLB   \
 CPUIOTLBEntry iotlb_v[NB_MMU_MODES][CPU_VTLB_SIZE];
 
 #else
diff --git a/include/exec/cpu_ldst.h b/include/exec/cpu_ldst.h
index 959068495a..83b2907d86 100644
--- a/include/exec/cpu_ldst.h
+++ b/include/exec/cpu_ldst.h
@@ -135,6 +135,21 @@ static inline target_ulong tlb_addr_write(const 
CPUTLBEntry *entry)
 #endif
 }
 
+#if TCG_TARGET_IMPLEMENTS_DYN_TLB
+/* Find the TLB index corresponding to the mmu_idx + address pair.  */
+static inline uintptr_t tlb_index(CPUArchState *env, uintptr_t mmu_idx,
+  target_ulong addr)
+{
+uintptr_t size_mask = env->tlb_mask[mmu_idx] >> CPU_TLB_ENTRY_BITS;
+
+return (addr >> TARGET_PAGE_BITS) & size_mask;
+}
+
+static inline size_t tlb_n_entries(CPUArchState *env, uintptr_t mmu_idx)
+{
+return (env->tlb_mask[mmu_idx] >> CPU_TLB_ENTRY_BITS) + 1;
+}
+#else
 /* Find the TLB index corresponding to the mmu_idx + address pair.  */
 static inline uintptr_t 

[Qemu-devel] [PATCH v5 1/3] cputlb: do not evict empty entries to the vtlb

[Emilio G. Cota]

Currently we evict an entry to the victim TLB when it doesn't match
the current address. But it could be that there's no match because
the current entry is empty (i.e. all -1's, for instance via tlb_flush).
Do not evict the entry to the vtlb in that case.

This change will help us keep track of the TLB's use rate, which
we'll use to implement a policy for dynamic TLB sizing.

Reviewed-by: Alex Bennée 
Reviewed-by: Richard Henderson 
Signed-off-by: Emilio G. Cota 
---
 include/exec/cpu-all.h | 9 +
 accel/tcg/cputlb.c | 2 +-
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index 117d2fbbca..e21140049b 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -362,6 +362,15 @@ static inline bool tlb_hit(target_ulong tlb_addr, 
target_ulong addr)
 return tlb_hit_page(tlb_addr, addr & TARGET_PAGE_MASK);
 }
 
+/**
+ * tlb_entry_is_empty - return true if the entry is not in use
+ * @te: pointer to CPUTLBEntry
+ */
+static inline bool tlb_entry_is_empty(const CPUTLBEntry *te)
+{
+return te->addr_read == -1 && te->addr_write == -1 && te->addr_code == -1;
+}
+
 void dump_exec_info(FILE *f, fprintf_function cpu_fprintf);
 void dump_opcount_info(FILE *f, fprintf_function cpu_fprintf);
 #endif /* !CONFIG_USER_ONLY */
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index af6bd8ccf9..5dc97212a9 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -591,7 +591,7 @@ void tlb_set_page_with_attrs(CPUState *cpu, target_ulong 
vaddr,
  * Only evict the old entry to the victim tlb if it's for a
  * different page; otherwise just overwrite the stale data.
  */
-if (!tlb_hit_page_anyprot(te, vaddr_page)) {
+if (!tlb_hit_page_anyprot(te, vaddr_page) && !tlb_entry_is_empty(te)) {
 unsigned vidx = env->tlb_d[mmu_idx].vindex++ % CPU_VTLB_SIZE;
 CPUTLBEntry *tv = >tlb_v_table[mmu_idx][vidx];
 
-- 
2.17.1

[Qemu-devel] [PATCH v5 0/3] Dynamic TLB sizing

[Emilio G. Cota]

v4: https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg02679.html

Changes since v4:

- Rebase on v3.1.0. Quite a few conflicts, but the resulting diffs
  are simpler than in v4.

- Re-run benchmarks on a different Skylake machine (this one is
  a server with a lower clock frequency).

- Add comment on the resizing policy and mechanism.

- Change the downsizing policy. Instead of not downsizing until
  a certain number of consecutive low use rates, keep track of
  the max use rate in a given time window (100ms), and
  downsize according to that max. This makes the policy resistant
  to guests that flush *very* quickly and often (I was not happy
  about just waiting until a certain number of flushes, since
  we cannot know what that number should be for all guests).
  The new policy has the shortcoming that since we check the
  time on the host, if the host is overcommitted we might downsize
  too aggressively. So the policy isn't perfect, but the gains
  are big enough that we might want to merge this as a start.
  Here's v4 vs. v5:
https://imgur.com/jkcFkdv
  v4 is a little better for mcf, but v5 is better for xalancbmk,
  and the average speedup is almost the same (1.48x vs. 1.51x,
  respectively).

You can fetch this series from:
  https://github.com/cota/qemu/tree/tlb-dyn-v5

Thanks,

Emilio
---
 accel/tcg/cputlb.c| 186 --
 include/exec/cpu-all.h|   9 +++
 include/exec/cpu-defs.h   |  48 +++-
 include/exec/cpu_ldst.h   |  21 ++
 tcg/aarch64/tcg-target.h  |   1 +
 tcg/arm/tcg-target.h  |   1 +
 tcg/i386/tcg-target.h |   1 +
 tcg/i386/tcg-target.inc.c |  28 +++
 tcg/mips/tcg-target.h |   1 +
 tcg/ppc/tcg-target.h  |   1 +
 tcg/s390/tcg-target.h |   1 +
 tcg/sparc/tcg-target.h|   1 +
 tcg/tci/tcg-target.h  |   1 +
 13 files changed, 278 insertions(+), 22 deletions(-)

[Qemu-devel] [PATCH] virtio: add ORDER_PLATFORM feature support

[Ilya Maximets]

This patch adds new property "order_platform" which is required to
allow VIRTIO_F_ORDER_PLATFORM feature negotiation. Disabled by
default because needed only for HW assited vhost backends.

Enabling of this feature will request guest drivers to use heavier
(platform dependent) techniques for memory ordering if negotiated.

Signed-off-by: Ilya Maximets 
---

Note: Patch to change the name of the feature from VIRTIO_F_IO_BARRIER
  to VIRTIO_F_ORDER_PLATFORM is not merged yet:
  https://www.mail-archive.com/virtio-dev@lists.oasis-open.org/msg04114.html

Patch for DPDK virtio driver available here:
  http://patches.dpdk.org/patch/48886/

 hw/net/vhost_net.c |  1 +
 include/hw/virtio/virtio.h | 13 -
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index e037db63a3..71877e46a5 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -78,6 +78,7 @@ static const int user_feature_bits[] = {
 VIRTIO_NET_F_MRG_RXBUF,
 VIRTIO_NET_F_MTU,
 VIRTIO_F_IOMMU_PLATFORM,
+VIRTIO_F_ORDER_PLATFORM,
 
 /* This bit implies RARP isn't sent by QEMU out of band */
 VIRTIO_NET_F_GUEST_ANNOUNCE,
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 9c1fa07d6d..7e206dadbc 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -29,6 +29,15 @@
 (0x1ULL << VIRTIO_F_NOTIFY_ON_EMPTY) | \
 (0x1ULL << VIRTIO_F_ANY_LAYOUT))
 
+#ifndef VIRTIO_F_ORDER_PLATFORM
+/*
+ * This feature indicates that memory accesses by the driver and the device
+ * are ordered in a way described by the platform.
+ * Not yet defined in Linux, i.e. not in standard-headers.
+ */
+#define VIRTIO_F_ORDER_PLATFORM 36
+#endif
+
 struct VirtQueue;
 
 static inline hwaddr vring_align(hwaddr addr,
@@ -264,7 +273,9 @@ typedef struct VirtIORNGConf VirtIORNGConf;
 DEFINE_PROP_BIT64("any_layout", _state, _field, \
   VIRTIO_F_ANY_LAYOUT, true), \
 DEFINE_PROP_BIT64("iommu_platform", _state, _field, \
-  VIRTIO_F_IOMMU_PLATFORM, false)
+  VIRTIO_F_IOMMU_PLATFORM, false),  \
+DEFINE_PROP_BIT64("order_platform", _state, _field, \
+  VIRTIO_F_ORDER_PLATFORM, false)
 
 hwaddr virtio_queue_get_desc_addr(VirtIODevice *vdev, int n);
 hwaddr virtio_queue_get_avail_addr(VirtIODevice *vdev, int n);
-- 
2.17.1

Re: [Qemu-devel] [PULL v2 00/46] Misc patches for 2018-12-13

[Peter Maydell]

On Thu, 13 Dec 2018 at 18:15, Paolo Bonzini  wrote:
>
> The following changes since commit bb9bf94b3e8926553290bc9a7cb84315af422086:
>
>   Merge remote-tracking branch 
> 'remotes/ehabkost/tags/machine-next-pull-request' into staging (2018-12-11 
> 19:18:58 +)
>
> are available in the Git repository at:
>
>   git://github.com/bonzini/qemu.git tags/for-upstream
>
> for you to fetch changes up to d115ceb731fc09134fb0816c956ab0eb88fbfa7b:
>
>   vhost-user-test: create a temporary directory per TestServer (2018-12-13 
> 10:57:30 +0100)
>
> 
> * HAX support for Linux hosts (Alejandro)
> * esp bugfixes (Guenter)
> * Windows build cleanup (Marc-André)
> * checkpatch logic improvements (Paolo)
> * coalesced range bugfix (Paolo)
> * switch testsuite to TAP (Paolo)
> * QTAILQ rewrite (Paolo)
> * enable vhost for TCG and clean up vhost-user-test (Paolo)
> * block/iscsi.c cancellation fixes (Stefan)
> * improve selection of the default accelerator (Thomas)
>
> 

Compile issues, I'm afraid (and a monitor.c conflict, but that
was trivial so I fixed it up):

Compile failures on OpenBSD, NetBSD, FreeBSD, OSX trying to use a linux header:

hw/net/vhost_net.c:26:10: fatal error: 'linux/vhost.h' file not found
#include 
 ^~~

and one on sparc Linux:
In file included from /home/pm215/qemu/hw/net/vhost_net.c:28:
/home/pm215/qemu/linux-headers/linux/kvm.h:14:10: fatal error:
asm/kvm.h: No such file or directory
 #include 
  ^~~

trying to use our linux-headers headers on a host architecture
which isn't a KVM-supporting one (those are the only ones we
sync headers for).

thanks
-- PMM

Re: [Qemu-devel] [PATCH 2/2] avoid TABs in files that only contain a few

[Aleksandar Markovic]

On Dec 13, 2018 11:40 PM, "Paolo Bonzini"  wrote:
>
> Most files that have TABs only contain a handful of them.  Change
> them to spaces so that we don't confuse people.
>
> disas, standard-headers, linux-headers and libdecnumber are imported
> from other projects and probably should be exempted from the check.

For MIPS parts, they are all ok and desireable:

Reviewed-by: Aleksandar Markovic 

> Outside those, after this patch the following files still contain both
> 8-space and TAB sequences at the beginning of the line.  Many of them
> have a majority of TABs, or were initially committed with all tabs.
>
> bsd-user/i386/target_syscall.h
> bsd-user/x86_64/target_syscall.h
> crypto/aes.c
> hw/audio/fmopl.c
> hw/audio/fmopl.h
> hw/block/tc58128.c
> hw/display/cirrus_vga.c
> hw/display/xenfb.c
> hw/dma/etraxfs_dma.c
> hw/intc/sh_intc.c
> hw/misc/mst_fpga.c
> hw/net/pcnet.c
> hw/sh4/sh7750.c
> hw/timer/m48t59.c
> hw/timer/sh_timer.c
> include/crypto/aes.h
> include/disas/bfd.h
> include/hw/sh4/sh.h
> libdecnumber/decNumber.c
> linux-headers/asm-generic/unistd.h
> linux-headers/linux/kvm.h
> linux-user/alpha/target_syscall.h
> linux-user/arm/nwfpe/double_cpdo.c
> linux-user/arm/nwfpe/fpa11_cpdt.c
> linux-user/arm/nwfpe/fpa11_cprt.c
> linux-user/arm/nwfpe/fpa11.h
> linux-user/flat.h
> linux-user/flatload.c
> linux-user/i386/target_syscall.h
> linux-user/ppc/target_syscall.h
> linux-user/sparc/target_syscall.h
> linux-user/syscall.c
> linux-user/syscall_defs.h
> linux-user/x86_64/target_syscall.h
> slirp/cksum.c
> slirp/if.c
> slirp/ip.h
> slirp/ip_icmp.c
> slirp/ip_icmp.h
> slirp/ip_input.c
> slirp/ip_output.c
> slirp/mbuf.c
> slirp/misc.c
> slirp/sbuf.c
> slirp/socket.c
> slirp/socket.h
> slirp/tcp_input.c
> slirp/tcpip.h
> slirp/tcp_output.c
> slirp/tcp_subr.c
> slirp/tcp_timer.c
> slirp/tftp.c
> slirp/udp.c
> slirp/udp.h
> target/cris/cpu.h
> target/cris/mmu.c
> target/cris/op_helper.c
> target/sh4/helper.c
> target/sh4/op_helper.c
> target/sh4/translate.c
> tcg/sparc/tcg-target.inc.c
> tests/tcg/cris/check_addo.c
> tests/tcg/cris/check_moveq.c
> tests/tcg/cris/check_swap.c
> tests/tcg/multiarch/test-mmap.c
> ui/vnc-enc-hextile-template.h
> ui/vnc-enc-zywrle.h
> util/envlist.c
> util/readline.c
>
> The following have only TABs:
>
> bsd-user/i386/target_signal.h
> bsd-user/sparc64/target_signal.h
> bsd-user/sparc64/target_syscall.h
> bsd-user/sparc/target_signal.h
> bsd-user/sparc/target_syscall.h
> bsd-user/x86_64/target_signal.h
> crypto/desrfb.c
> hw/audio/intel-hda-defs.h
> hw/core/uboot_image.h
> hw/sh4/sh7750_regnames.c
> hw/sh4/sh7750_regs.h
> include/hw/cris/etraxfs_dma.h
> linux-user/alpha/termbits.h
> linux-user/arm/nwfpe/fpopcode.h
> linux-user/arm/nwfpe/fpsr.h
> linux-user/arm/syscall_nr.h
> linux-user/arm/target_signal.h
> linux-user/cris/target_signal.h
> linux-user/i386/target_signal.h
> linux-user/linux_loop.h
> linux-user/m68k/target_signal.h
> linux-user/microblaze/target_signal.h
> linux-user/mips64/target_signal.h
> linux-user/mips/target_signal.h
> linux-user/mips/target_syscall.h
> linux-user/mips/termbits.h
> linux-user/ppc/target_signal.h
> linux-user/sh4/target_signal.h
> linux-user/sh4/termbits.h
> linux-user/sparc64/target_syscall.h
> linux-user/sparc/target_signal.h
> linux-user/x86_64/target_signal.h
> linux-user/x86_64/termbits.h
> pc-bios/optionrom/optionrom.h
> slirp/mbuf.h
> slirp/misc.h
> slirp/sbuf.h
> slirp/tcp.h
> slirp/tcp_timer.h
> slirp/tcp_var.h
> target/i386/svm.h
> target/sparc/asi.h
> target/xtensa/core-dc232b/xtensa-modules.inc.c
> target/xtensa/core-dc233c/xtensa-modules.inc.c
> target/xtensa/core-de212/core-isa.h
> target/xtensa/core-de212/xtensa-modules.inc.c
> target/xtensa/core-fsf/xtensa-modules.inc.c
> target/xtensa/core-sample_controller/core-isa.h
> target/xtensa/core-sample_controller/xtensa-modules.inc.c
> target/xtensa/core-test_kc705_be/core-isa.h
> target/xtensa/core-test_kc705_be/xtensa-modules.inc.c
> tests/tcg/cris/check_abs.c
> tests/tcg/cris/check_addc.c
> tests/tcg/cris/check_addcm.c
> tests/tcg/cris/check_addoq.c
> tests/tcg/cris/check_bound.c
> tests/tcg/cris/check_ftag.c
> tests/tcg/cris/check_int64.c
> tests/tcg/cris/check_lz.c
> tests/tcg/cris/check_openpf5.c
> tests/tcg/cris/check_sigalrm.c
> tests/tcg/cris/crisutils.h
> tests/tcg/cris/sys.c
> tests/tcg/i386/test-i386-ssse3.c
> ui/vgafont.h
>
> Signed-off-by: Paolo Bonzini 
> ---
>  block/bochs.c  | 22 ++---
>  block/file-posix.c  

Re: [Qemu-devel] [PATCH for-3.1] hw/xen/xen_pt_graphics: Don't trust the BIOS ROM contents so much

[Stefano Stabellini]

On Fri, 14 Dec 2018, Peter Maydell wrote:
> On Mon, 26 Nov 2018 at 15:03, Anthony PERARD  
> wrote:
> >
> > On Mon, Nov 19, 2018 at 04:26:58PM +, Peter Maydell wrote:
> > > Coverity (CID 796599) points out that xen_pt_setup_vga() trusts
> > > the rom->size field in the BIOS ROM from a PCI passthrough VGA
> > > device, and uses it as an index into the memory which contains
> > > the BIOS image. A corrupt BIOS ROM could therefore cause us to
> > > index off the end of the buffer.
> > >
> > > Check that the size is within bounds before we use it.
> > >
> > > We are also trusting the pcioffset field, and assuming that
> > > the whole rom_header is present; Coverity doesn't notice these,
> > > but check them too.
> > >
> > > Signed-off-by: Peter Maydell 
> > > ---
> > > Disclaimer: compile tested only, as I don't have a Xen setup,
> > > let alone one with pass-through PCI graphics.
> > >
> > > Note that https://xenbits.xen.org/xsa/advisory-124.html
> > > defines that bugs which are only exploitable by a malicious
> > > piece of hardware that is passed through to the guest are
> > > not security vulnerabilities as far as the Xen Project is
> > > concerned, and are treated like normal non-security-related bugs.
> > > So this is just a bugfix, not a security issue.
> > >
> > > Marked "for-3.1" because it would let us squash another Coverity
> > > issue, and it is a bug fix; on the other hand it's an obscure
> > > corner case and has been this way since forever.
> >
> > I haven't tested that patch either, but the changes looks fine, so:
> >
> > Acked-by: Anthony PERARD 
> 
> Ping! Would the Xen folks like to test this and/or send it in
> via a xen pullreq now that 4.0 has reopened for development?
> 
> Alternatively I can put it in via a pullreq I'm currently
> doing in its current "not tested but looks fine" state :-)

Hi Peter,

I know that Anthony is preparing a pretty large pull request for you.
You should see something coming your way soon.

Cheers,

Stefano

Re: [Qemu-devel] [RFC v2 05/38] plugin: add user-facing API

[Emilio G. Cota]

On Fri, Dec 14, 2018 at 15:57:32 +, Aaron Lindsay wrote:
> Emilio,
> 
> First, thanks for putting this together - I think everyone doing this
> sort of thing will benefit if we're able to agree on one upstream plugin
> interface.
> 
> One thing I'd like to see is support for unregistering callbacks once
> they are registered. For instance, you can imagine that a plugin may
> have 'modality', where it may care about tracing very detailed
> information in one mode, but trace only limited information otherwise -
> perhaps only enough to figure out when it needs to switch back to the
> other mode.
> 
> I added a function to the user-facing plugin API in my own version of
> Pavel's plugin patchset to clear all existing plugin instrumentation,
> allowing the plugin to drop instrumentation if it was no longer
> interested. Of course, you could always add logic to a plugin to ignore
> unwanted callbacks, but it could be significantly more efficient to
> remove them entirely. In Pavel's patchset, this was essentially just a
> call to tb_flush(), though I think it would be a little more involved
> for yours.

This is a good point.

"Regular" callbacks can be unregistered by just passing NULL as the
callback (see plugin_unregister_cb__locked, which is called from
do_plugin_register_cb). "Direct" callbacks, however (i.e. the ones
that embed a callback directly in the translated code),
would have to be unregistered by flushing the particular TB
(or all TBs, which is probably better from an API viewpoint).

I think the following API call would do what you need:

  typedef int (*qemu_plugin_reset_cb_t)(qemu_plugin_id_t id);
  void qemu_plugin_reset(qemu_plugin_id_t id, qemu_plugin_reset_cb_t cb);

(or maybe qemu_plugin_reinstall?)

The idea is that a plugin can "reset" itself, so that (1) all
its CBs are cleared and (2) the plugin can register new callbacks.
This would all happen in an atomic context (no vCPU running), so
that the plugin would miss no CPU events.

How does this sound?

Thanks,

Emilio

[Qemu-devel] [RFC PATCH 3/5] qdev-properties: add r/o 64bit bitfield property

[Roman Kagan]

Add a version 64bit bitfield property with no setter, useful for
introspecting the device state without being able to modify it.

Signed-off-by: Roman Kagan 
---
 include/hw/qdev-properties.h | 9 +
 hw/core/qdev-properties.c| 9 -
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
index 3ab9cd2eb6..24df135ff8 100644
--- a/include/hw/qdev-properties.h
+++ b/include/hw/qdev-properties.h
@@ -9,6 +9,7 @@
 
 extern const PropertyInfo qdev_prop_bit;
 extern const PropertyInfo qdev_prop_bit64;
+extern const PropertyInfo qdev_prop_bit64_ro;
 extern const PropertyInfo qdev_prop_bool;
 extern const PropertyInfo qdev_prop_uint8;
 extern const PropertyInfo qdev_prop_uint16;
@@ -96,6 +97,14 @@ extern const PropertyInfo qdev_prop_off_auto_pcibar;
 .defval.u  = (bool)_defval, \
 }
 
+#define DEFINE_PROP_BIT64_RO(_name, _state, _field, _bit) { \
+.name  = (_name),   \
+.info  = &(qdev_prop_bit64_ro), \
+.bitnr= (_bit), \
+.offset= offsetof(_state, _field)   \
++ type_check(uint64_t, typeof_field(_state, _field)),   \
+}
+
 #define DEFINE_PROP_BOOL(_name, _state, _field, _defval) {   \
 .name  = (_name),\
 .info  = &(qdev_prop_bool),  \
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index bd84c4ea4c..bb9bd48e5c 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -146,7 +146,8 @@ const PropertyInfo qdev_prop_bit = {
 
 static uint64_t qdev_get_prop_mask64(Property *prop)
 {
-assert(prop->info == _prop_bit64);
+assert(prop->info == _prop_bit64 ||
+   prop->info == _prop_bit64_ro);
 return 0x1ull << prop->bitnr;
 }
 
@@ -201,6 +202,12 @@ const PropertyInfo qdev_prop_bit64 = {
 .set_default_value = set_default_value_bool,
 };
 
+const PropertyInfo qdev_prop_bit64_ro = {
+.name  = "bool",
+.description = "on/off",
+.get   = prop_get_bit64,
+};
+
 /* --- bool --- */
 
 static void get_bool(Object *obj, Visitor *v, const char *name, void *opaque,
-- 
2.19.2

[Qemu-devel] [RFC PATCH 2/5] qmp: further consolidate listing of device and object properties

[Roman Kagan]

Take the approach of commit 35f63767dc77d85bebff6c6565aceaf74023776a
"qmp: Merge ObjectPropertyInfo and DevicePropertyInfo" one step further:
drop device property-specific code from qmp_device_list_properties and
consolidate the resulting common part with qmp_qom_list_properties.

Signed-off-by: Roman Kagan 
---
 qmp.c | 92 ++-
 1 file changed, 22 insertions(+), 70 deletions(-)

diff --git a/qmp.c b/qmp.c
index e7c0a2fd60..673dfa72ce 100644
--- a/qmp.c
+++ b/qmp.c
@@ -430,56 +430,22 @@ ObjectTypeInfoList *qmp_qom_list_types(bool 
has_implements,
 return ret;
 }
 
-/* Return a DevicePropertyInfo for a qdev property.
- *
- * If a qdev property with the given name does not exist, use the given default
- * type.  If the qdev property info should not be shown, return NULL.
- *
- * The caller must free the return value.
- */
-static ObjectPropertyInfo *make_device_property_info(ObjectClass *klass,
-  const char *name,
-  const char *default_type,
-  const char *description)
+static void push_property_info(ObjectPropertyInfoList **prop_list,
+   ObjectProperty *prop)
 {
 ObjectPropertyInfo *info;
-Property *prop;
-
-do {
-for (prop = DEVICE_CLASS(klass)->props; prop && prop->name; prop++) {
-if (strcmp(name, prop->name) != 0) {
-continue;
-}
-
-/*
- * TODO Properties without a parser are just for dirty hacks.
- * qdev_prop_ptr is the only such PropertyInfo.  It's marked
- * for removal.  This conditional should be removed along with
- * it.
- */
-if (!prop->info->set && !prop->info->create) {
-return NULL;   /* no way to set it, don't show */
-}
-
-info = g_malloc0(sizeof(*info));
-info->name = g_strdup(prop->name);
-info->type = default_type ? g_strdup(default_type)
-  : g_strdup(prop->info->name);
-info->has_description = !!prop->info->description;
-info->description = g_strdup(prop->info->description);
-return info;
-}
-klass = object_class_get_parent(klass);
-} while (klass != object_class_by_name(TYPE_DEVICE));
-
-/* Not a qdev property, use the default type */
-info = g_malloc0(sizeof(*info));
-info->name = g_strdup(name);
-info->type = g_strdup(default_type);
-info->has_description = !!description;
-info->description = g_strdup(description);
-
-return info;
+ObjectPropertyInfoList *entry;
+
+info = g_new0(ObjectPropertyInfo, 1);
+info->name = g_strdup(prop->name);
+info->type = g_strdup(prop->type);
+info->has_description = !!prop->description;
+info->description = g_strdup(prop->description);
+
+entry = g_new0(ObjectPropertyInfoList, 1);
+entry->value = info;
+entry->next = *prop_list;
+*prop_list = entry;
 }
 
 ObjectPropertyInfoList *qmp_device_list_properties(const char *typename,
@@ -514,9 +480,6 @@ ObjectPropertyInfoList *qmp_device_list_properties(const 
char *typename,
 
 object_property_iter_init(, obj);
 while ((prop = object_property_iter_next())) {
-ObjectPropertyInfo *info;
-ObjectPropertyInfoList *entry;
-
 /* Skip Object and DeviceState properties */
 if (strcmp(prop->name, "type") == 0 ||
 strcmp(prop->name, "realized") == 0 ||
@@ -533,16 +496,12 @@ ObjectPropertyInfoList *qmp_device_list_properties(const 
char *typename,
 continue;
 }
 
-info = make_device_property_info(klass, prop->name, prop->type,
- prop->description);
-if (!info) {
+/* Skip readonly properties. */
+if (!prop->set) {
 continue;
 }
 
-entry = g_malloc0(sizeof(*entry));
-entry->value = info;
-entry->next = prop_list;
-prop_list = entry;
+push_property_info(_list, prop);
 }
 
 object_unref(obj);
@@ -579,19 +538,12 @@ ObjectPropertyInfoList *qmp_qom_list_properties(const 
char *typename,
 object_property_iter_init(, obj);
 }
 while ((prop = object_property_iter_next())) {
-ObjectPropertyInfo *info;
-ObjectPropertyInfoList *entry;
+/* Skip readonly properties. */
+if (!prop->set) {
+continue;
+}
 
-info = g_malloc0(sizeof(*info));
-info->name = g_strdup(prop->name);
-info->type = g_strdup(prop->type);
-info->has_description = !!prop->description;
-info->description = g_strdup(prop->description);
-
-entry = g_malloc0(sizeof(*entry));
-entry->value = info;
-entry->next = prop_list;
-

[Qemu-devel] [RFC PATCH 4/5] virtio: drop DEFINE_VIRTIO_COMMON_FEATURES

[Roman Kagan]

This macro is only used in one place so seems to be unnecessary.

Signed-off-by: Roman Kagan 
---
 include/hw/virtio/virtio.h | 12 
 hw/virtio/virtio.c | 11 ++-
 2 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 9c1fa07d6d..cea356efed 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -254,18 +254,6 @@ typedef struct virtio_input_conf virtio_input_conf;
 typedef struct VirtIOSCSIConf VirtIOSCSIConf;
 typedef struct VirtIORNGConf VirtIORNGConf;
 
-#define DEFINE_VIRTIO_COMMON_FEATURES(_state, _field) \
-DEFINE_PROP_BIT64("indirect_desc", _state, _field,\
-  VIRTIO_RING_F_INDIRECT_DESC, true), \
-DEFINE_PROP_BIT64("event_idx", _state, _field,\
-  VIRTIO_RING_F_EVENT_IDX, true), \
-DEFINE_PROP_BIT64("notify_on_empty", _state, _field,  \
-  VIRTIO_F_NOTIFY_ON_EMPTY, true), \
-DEFINE_PROP_BIT64("any_layout", _state, _field, \
-  VIRTIO_F_ANY_LAYOUT, true), \
-DEFINE_PROP_BIT64("iommu_platform", _state, _field, \
-  VIRTIO_F_IOMMU_PLATFORM, false)
-
 hwaddr virtio_queue_get_desc_addr(VirtIODevice *vdev, int n);
 hwaddr virtio_queue_get_avail_addr(VirtIODevice *vdev, int n);
 hwaddr virtio_queue_get_used_addr(VirtIODevice *vdev, int n);
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 22bd1ac34e..99d396c516 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -2601,7 +2601,16 @@ static void virtio_device_instance_finalize(Object *obj)
 }
 
 static Property virtio_properties[] = {
-DEFINE_VIRTIO_COMMON_FEATURES(VirtIODevice, host_features),
+DEFINE_PROP_BIT64("indirect_desc", VirtIODevice, host_features,
+  VIRTIO_RING_F_INDIRECT_DESC, true),
+DEFINE_PROP_BIT64("event_idx", VirtIODevice, host_features,
+  VIRTIO_RING_F_EVENT_IDX, true),
+DEFINE_PROP_BIT64("notify_on_empty", VirtIODevice, host_features,
+  VIRTIO_F_NOTIFY_ON_EMPTY, true),
+DEFINE_PROP_BIT64("any_layout", VirtIODevice, host_features,
+  VIRTIO_F_ANY_LAYOUT, true),
+DEFINE_PROP_BIT64("iommu_platform", VirtIODevice, host_features,
+  VIRTIO_F_IOMMU_PLATFORM, false),
 DEFINE_PROP_END_OF_LIST(),
 };
 
-- 
2.19.2

[Qemu-devel] [RFC PATCH 1/5] qom: preserve get/set presence in aliased properties

[Roman Kagan]

Usually in order to tell if a property is read-only, write-only, or
read-write, one has to look at whether it has .get or .set methods.

However, property aliases are always defined with both, and it's not
until the call to the getter or setter when the support for the
corresponding operation can be found out.

To make it easier to determine if an operation is supported for an alias
property, only assign it getter and setter if the target property has
the corresponding method.

Signed-off-by: Roman Kagan 
---
 qom/object.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/qom/object.c b/qom/object.c
index 17921c0a71..b362ebab19 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -2382,8 +2382,8 @@ void object_property_add_alias(Object *obj, const char 
*name,
 prop->target_name = g_strdup(target_name);
 
 op = object_property_add(obj, name, prop_type,
- property_get_alias,
- property_set_alias,
+ target_prop->get ? property_get_alias : NULL,
+ target_prop->set ? property_set_alias : NULL,
  property_release_alias,
  prop, _err);
 if (local_err) {
-- 
2.19.2

[Qemu-devel] [RFC PATCH 0/5] expose negotiated virtio features in r/o properties

[Roman Kagan]

This series is an attempt to make virtio features acknowledged by the
guest visible as read-only QOM properties.  One potential usecase of
this is debugging; another is when the upper layer needs to do something
only when/if the guest has acknowledged the support for a feature (e.g.
hot-plug a VFIO device once the guest claims VIRTIO_NET_F_STANDBY
support).

Being an RFC, it's incomplete and fails checkpatch, but I'd be intersted
to know if the approach is sane and worthwhile before I invest more time
in it.

Roman Kagan (5):
  qom: preserve get/set presence in aliased properties
  qmp: further consolidate listing of device and object properties
  qdev-properties: add r/o 64bit bitfield property
  virtio: drop DEFINE_VIRTIO_COMMON_FEATURES
  virtio: expose negotiated features in r/o properties

 include/hw/qdev-properties.h|  9 
 include/hw/virtio/virtio-scsi.h |  2 +-
 include/hw/virtio/virtio.h  | 18 +++
 hw/char/virtio-serial-bus.c |  6 ++-
 hw/core/qdev-properties.c   |  9 +++-
 hw/net/virtio-net.c | 88 +--
 hw/scsi/virtio-scsi.c   |  4 +-
 hw/virtio/virtio.c  | 11 +++-
 qmp.c   | 92 -
 qom/object.c|  4 +-
 10 files changed, 113 insertions(+), 130 deletions(-)

-- 
2.19.2

[Qemu-devel] [RFC PATCH 5/5] virtio: expose negotiated features in r/o properties

[Roman Kagan]

Make virtio features acknowledged by the guest visible through QOM as
read-only properties.  One potential usecase of this is debugging;
another is when the upper layer needs to do something only when/if the
guest has acknowledged the support for a feature (e.g. hot-plug a VFIO
device once the guest claims VIRTIO_NET_F_STANDBY support).

Since most of the feature bits already have associated properties for
host_features, reuse those definitions by creating a new macro that
combines the original definition for the host_features bit property and
a definition of a read-only guest_features bit property with the same
name prefixed with "negotiated-".  For the features which have no
associated host_features bit property, only the latter is defined.

Note #1: the macro is somewhat fragile as it produces two values
separated by a comma, to be used for initializing consecutive elements
in an array
Note #2: due to note #1, it fails checkpatch.
Note #3: it is also somewhat fragile as it assumes its first argument to
be a string literal
Note #4: for RFC purposes I only converted some virtio devices.

Signed-off-by: Roman Kagan 
---
 include/hw/virtio/virtio-scsi.h |  2 +-
 include/hw/virtio/virtio.h  |  8 +++
 hw/char/virtio-serial-bus.c |  6 ++-
 hw/net/virtio-net.c | 88 ++---
 hw/scsi/virtio-scsi.c   |  4 +-
 hw/virtio/virtio.c  | 20 
 6 files changed, 73 insertions(+), 55 deletions(-)

diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h
index 4c0bcdb788..9b412bd2c3 100644
--- a/include/hw/virtio/virtio-scsi.h
+++ b/include/hw/virtio/virtio-scsi.h
@@ -86,7 +86,7 @@ typedef struct VirtIOSCSI {
 bool dataplane_starting;
 bool dataplane_stopping;
 bool dataplane_fenced;
-uint32_t host_features;
+uint64_t host_features;
 } VirtIOSCSI;
 
 typedef struct VirtIOSCSIReq {
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index cea356efed..a4690e6176 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -254,6 +254,14 @@ typedef struct virtio_input_conf virtio_input_conf;
 typedef struct VirtIOSCSIConf VirtIOSCSIConf;
 typedef struct VirtIORNGConf VirtIORNGConf;
 
+#define DEFINE_VIRTIO_FEATURE_BIT_NEGOTIATED(_name, _bit) \
+DEFINE_PROP_BIT64_RO("negotiated-" _name, VirtIODevice,   \
+ guest_features, _bit)
+
+#define DEFINE_VIRTIO_FEATURE_BIT(_name, _state, _field, _bit, _defval) \
+DEFINE_PROP_BIT64(_name, _state, _field, _bit, _defval),\
+DEFINE_VIRTIO_FEATURE_BIT_NEGOTIATED(_name, _bit)
+
 hwaddr virtio_queue_get_desc_addr(VirtIODevice *vdev, int n);
 hwaddr virtio_queue_get_avail_addr(VirtIODevice *vdev, int n);
 hwaddr virtio_queue_get_used_addr(VirtIODevice *vdev, int n);
diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
index 04e3ebe352..07bf729891 100644
--- a/hw/char/virtio-serial-bus.c
+++ b/hw/char/virtio-serial-bus.c
@@ -1154,8 +1154,10 @@ static const VMStateDescription vmstate_virtio_console = 
{
 static Property virtio_serial_properties[] = {
 DEFINE_PROP_UINT32("max_ports", VirtIOSerial, serial.max_virtserial_ports,
   31),
-DEFINE_PROP_BIT64("emergency-write", VirtIOSerial, host_features,
-  VIRTIO_CONSOLE_F_EMERG_WRITE, true),
+DEFINE_VIRTIO_FEATURE_BIT_NEGOTIATED("multiport",
+ VIRTIO_CONSOLE_F_MULTIPORT),
+DEFINE_VIRTIO_FEATURE_BIT("emergency-write", VirtIOSerial, host_features,
+  VIRTIO_CONSOLE_F_EMERG_WRITE, true),
 DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 385b1a03e9..d4df3394ee 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -2144,46 +2144,54 @@ static const VMStateDescription vmstate_virtio_net = {
 };
 
 static Property virtio_net_properties[] = {
-DEFINE_PROP_BIT64("csum", VirtIONet, host_features,
-VIRTIO_NET_F_CSUM, true),
-DEFINE_PROP_BIT64("guest_csum", VirtIONet, host_features,
-VIRTIO_NET_F_GUEST_CSUM, true),
-DEFINE_PROP_BIT64("gso", VirtIONet, host_features, VIRTIO_NET_F_GSO, true),
-DEFINE_PROP_BIT64("guest_tso4", VirtIONet, host_features,
-VIRTIO_NET_F_GUEST_TSO4, true),
-DEFINE_PROP_BIT64("guest_tso6", VirtIONet, host_features,
-VIRTIO_NET_F_GUEST_TSO6, true),
-DEFINE_PROP_BIT64("guest_ecn", VirtIONet, host_features,
-VIRTIO_NET_F_GUEST_ECN, true),
-DEFINE_PROP_BIT64("guest_ufo", VirtIONet, host_features,
-VIRTIO_NET_F_GUEST_UFO, true),
-DEFINE_PROP_BIT64("guest_announce", VirtIONet, host_features,
-VIRTIO_NET_F_GUEST_ANNOUNCE, true),
-DEFINE_PROP_BIT64("host_tso4", VirtIONet, host_features,
-VIRTIO_NET_F_HOST_TSO4, true),
-

[Qemu-devel] [PATCH v3] s390x/pci: add common function measurement block

[Pierre Morel]

From: Yi Min Zhao 

Common function measurement block is used to report zPCI internal
counters of successful pcilg/stg/stb and rpcit instructions to
a memory location provided by the program.

This patch introduces a new ZpciFmb structure and schedules a timer
callback to copy the zPCI measures to the FMB in the guest memory
at an interval time set to 4s by default.

An error while attemping to update the FMB, would generated an error
event to the guest.

The pcilg/stg/stb and rpcit interception handlers issue, increase
the related counter on success.
The guest shall pass a null FMBA (FMB address) in the FIB (Function
Information Block) when it issues a Modify PCI Function Control
instrcuction to switch off FMB and stop the corresponding timer.

Signed-off-by: Yi Min Zhao 
Signed-off-by: Pierre Morel 
---
 hw/s390x/s390-pci-bus.c  |   4 +-
 hw/s390x/s390-pci-bus.h  |  29 ++
 hw/s390x/s390-pci-inst.c | 141 ++-
 hw/s390x/s390-pci-inst.h |   1 +
 4 files changed, 171 insertions(+), 4 deletions(-)

diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
index 060ff06..f0d34dd 100644
--- a/hw/s390x/s390-pci-bus.c
+++ b/hw/s390x/s390-pci-bus.c
@@ -989,6 +989,7 @@ static void s390_pcihost_hot_unplug(HotplugHandler 
*hotplug_dev,
 bus = pci_get_bus(pci_dev);
 devfn = pci_dev->devfn;
 object_unparent(OBJECT(pci_dev));
+fmb_timer_free(pbdev);
 s390_pci_msix_free(pbdev);
 s390_pci_iommu_free(s, bus, devfn);
 pbdev->pdev = NULL;
@@ -1136,6 +1137,7 @@ static void s390_pci_device_realize(DeviceState *dev, 
Error **errp)
 }
 
 zpci->state = ZPCI_FS_RESERVED;
+zpci->fmb.format = ZPCI_FMB_FORMAT;
 }
 
 static void s390_pci_device_reset(DeviceState *dev)
@@ -1160,7 +1162,7 @@ static void s390_pci_device_reset(DeviceState *dev)
 pci_dereg_ioat(pbdev->iommu);
 }
 
-pbdev->fmb_addr = 0;
+fmb_timer_free(pbdev);
 }
 
 static void s390_pci_get_fid(Object *obj, Visitor *v, const char *name,
diff --git a/hw/s390x/s390-pci-bus.h b/hw/s390x/s390-pci-bus.h
index 1f7f9b5..69353ef 100644
--- a/hw/s390x/s390-pci-bus.h
+++ b/hw/s390x/s390-pci-bus.h
@@ -286,6 +286,33 @@ typedef struct S390PCIIOMMUTable {
 S390PCIIOMMU *iommu[PCI_SLOT_MAX];
 } S390PCIIOMMUTable;
 
+/* Function Measurement Block */
+#define DEFAULT_MUI 4000
+#define UPDATE_U_BIT 0x1ULL
+#define FMBK_MASK 0xfULL
+
+typedef struct ZpciFmbFmt0 {
+uint64_t dma_rbytes;
+uint64_t dma_wbytes;
+} ZpciFmbFmt0;
+
+#define ZPCI_FMB_CNT_LD0
+#define ZPCI_FMB_CNT_ST1
+#define ZPCI_FMB_CNT_STB   2
+#define ZPCI_FMB_CNT_RPCIT 3
+#define ZPCI_FMB_CNT_MAX   4
+
+#define ZPCI_FMB_FORMAT0
+
+typedef struct ZpciFmb {
+uint32_t format;
+uint32_t sample;
+uint64_t last_update;
+uint64_t counter[ZPCI_FMB_CNT_MAX];
+ZpciFmbFmt0 fmt0;
+} ZpciFmb;
+QEMU_BUILD_BUG_MSG(offsetof(ZpciFmb, fmt0) != 48, "padding in ZpciFmb");
+
 struct S390PCIBusDevice {
 DeviceState qdev;
 PCIDevice *pdev;
@@ -297,6 +324,8 @@ struct S390PCIBusDevice {
 uint32_t fid;
 bool fid_defined;
 uint64_t fmb_addr;
+ZpciFmb fmb;
+QEMUTimer *fmb_timer;
 uint8_t isc;
 uint16_t noi;
 uint16_t maxstbl;
diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c
index 7b61367..117a217 100644
--- a/hw/s390x/s390-pci-inst.c
+++ b/hw/s390x/s390-pci-inst.c
@@ -19,6 +19,7 @@
 #include "exec/memory-internal.h"
 #include "qemu/error-report.h"
 #include "sysemu/hw_accel.h"
+#include "hw/s390x/tod.h"
 
 #ifndef DEBUG_S390PCI_INST
 #define DEBUG_S390PCI_INST  0
@@ -293,7 +294,7 @@ int clp_service_call(S390CPU *cpu, uint8_t r2, uintptr_t ra)
 resgrp->fr = 1;
 stq_p(>dasm, 0);
 stq_p(>msia, ZPCI_MSI_ADDR);
-stw_p(>mui, 0);
+stw_p(>mui, DEFAULT_MUI);
 stw_p(>i, 128);
 stw_p(>maxstbl, 128);
 resgrp->version = 0;
@@ -456,6 +457,8 @@ int pcilg_service_call(S390CPU *cpu, uint8_t r1, uint8_t 
r2, uintptr_t ra)
 return 0;
 }
 
+pbdev->fmb.counter[ZPCI_FMB_CNT_LD]++;
+
 env->regs[r1] = data;
 setcc(cpu, ZPCI_PCI_LS_OK);
 return 0;
@@ -561,6 +564,8 @@ int pcistg_service_call(S390CPU *cpu, uint8_t r1, uint8_t 
r2, uintptr_t ra)
 return 0;
 }
 
+pbdev->fmb.counter[ZPCI_FMB_CNT_ST]++;
+
 setcc(cpu, ZPCI_PCI_LS_OK);
 return 0;
 }
@@ -681,6 +686,7 @@ err:
 s390_set_status_code(env, r1, ZPCI_PCI_ST_FUNC_IN_ERR);
 s390_pci_generate_error_event(error, pbdev->fh, pbdev->fid, start, 0);
 } else {
+pbdev->fmb.counter[ZPCI_FMB_CNT_RPCIT]++;
 setcc(cpu, ZPCI_PCI_LS_OK);
 }
 return 0;
@@ -783,6 +789,8 @@ int pcistb_service_call(S390CPU *cpu, uint8_t r1, uint8_t 
r3, uint64_t gaddr,
 }
 }
 
+pbdev->fmb.counter[ZPCI_FMB_CNT_STB]++;
+
 setcc(cpu, ZPCI_PCI_LS_OK);
 return 0;
 
@@ -889,6 +897,107 @@ void pci_dereg_ioat(S390PCIIOMMU *iommu)
 iommu->g_iota = 0;
 }
 
+void 

[Qemu-devel] [PATCH v3] s390x/pci: add common fmb

[Pierre Morel]

After the last review round I worked on endianness.
Doing this I found some errors in the code and in the interpretation
of the documentation.
  
The new patch changed the following from previous version:
 
In s390-pci-bus:
- Initialize the FMB Format.
 
In s390-pci-bus.h 
- re-organization of the internal counters, having a table for the
  internal counters.

In s390-pci-inst.c
- Internal counters update (LD/ST/STB/RPCIT) is done always.
  even if the FMB if fmb_addr is NULL.
  AFAIU this respect the documentation which only states that FMB
  update is stopped.
- in mpcifc_service_call(), moved the setting of fmb_addr after
  the timer has been stopped.
- fmb_update((), use address_space_stq_be() to handle endianness
  when storing the FMB.
- define the format with 32 bits instead of one char and reserved
  chars, this is easier to handle the FMB copy.
- No update of the DMA fields inside the FMB, as stipulated by the
  documentation when format32 is 0.

Patch is tested (for the good case) on Z(KVM) and X(TCG).

Yi Min Zhao (1):
  s390x/pci: add common function measurement block

 hw/s390x/s390-pci-bus.c  |   4 +-
 hw/s390x/s390-pci-bus.h  |  29 ++
 hw/s390x/s390-pci-inst.c | 141 ++-
 hw/s390x/s390-pci-inst.h |   1 +
 4 files changed, 171 insertions(+), 4 deletions(-)

-- 
2.7.4

Re: [Qemu-devel] [PULL 00/37] target-arm queue

[Peter Maydell]

On Thu, 13 Dec 2018 at 14:56, Peter Maydell  wrote:
>
> First target-arm pullreq of the 4.0 series; most of this
> is Mao's cleanups that finally let us drop sysbus::init;
> the most interesting user-visible feature is RTH's patches
> adding some v8.1 and v8.2 architecture features.
>
> thanks
> -- PMM
>
> The following changes since commit 6145a6d84b3bf0f25935b88543febe076c61b0f4:
>
>   Merge remote-tracking branch 'remotes/cohuck/tags/s390x-20181212' into 
> staging (2018-12-13 13:06:09 +)
>
> are available in the Git repository at:
>
>   https://git.linaro.org/people/pmaydell/qemu-arm.git 
> tags/pull-target-arm-20181213
>
> for you to fetch changes up to 2d7137c10fafefe40a0a049ff8a7bd78b66e661f:
>
>   target/arm: Implement the ARMv8.1-LOR extension (2018-12-13 14:41:24 +)
>
> 
> target-arm queue:
>  * Convert various devices from sysbus init to instance_init
>  * Remove the now unused sysbus init support entirely
>  * Allow AArch64 processors to boot from a kernel placed over 4GB
>  * hw: arm: musicpal: drop TYPE_WM8750 in object_property_set_link()
>  * versal: minor fixes to virtio-mmio instantation
>  * arm: Implement the ARMv8.1-HPD extension
>  * arm: Implement the ARMv8.2-AA32HPD extension
>  * arm: Implement the ARMv8.1-LOR extension (as the trivial
>"no limited ordering regions provided" minimum)
>
Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/4.0
for any user-visible changes.

-- PMM

Re: [Qemu-devel] [PATCH v1 1/2] target/arm: kvm64 make guest debug AA32 break point aware

[Ard Biesheuvel]

On Fri, 14 Dec 2018 at 17:26, Alex Bennée  wrote:
>
>
> Richard Henderson  writes:
>
> > On 12/13/18 8:55 AM, Alex Bennée wrote:
> >>
> >> Ard Biesheuvel  writes:
> >>
> >>> Hi Alex,
> >>>
> >>> Thanks again for looking into this.
> >>>
> >>> On Thu, 13 Dec 2018 at 12:55, Alex Bennée  wrote:
> >> 
> >>>
> 
>   int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct 
>  kvm_sw_breakpoint *bp)
>   {
>  +CPUARMState *env = _CPU(cs)->env;
>  +int el = arm_current_el(env);
>  +bool is_aa64 = arm_el_is_aa64(env, el);
>  +const uint32_t *bpi = is_aa64 ? _insn : _insn;
>  +
>   if (have_guest_debug) {
>   if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)>saved_insn, 
>  4, 0) ||
>  -cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)_insn, 4, 
>  1)) {
>  +cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)bpi, 4, 1)) {
> >>>
> >>> Should we be dealing with endianness here?
> >>>
> >> 
> >>
> >> I don't think so - everything eventually ends up (ld|st)n_p which deals
> >> with the endianness details.
> >
> > I think Ard is right.  You need to consider dynamic endianness with
> >
> > bswap_code(arm_sctlr_b(env))
>
> *sigh* I guess. It of course still a heuristic that can break because we
> don't know if the system will have switched mode by the time it gets to
> the breakpoint.
>

Actually, I was referring to the QEMU/host side. Instruction encodings
are always little endian in ARMv7 and v8 (which is all KVM cares about
in any case), but I guess it is [theoretically?] possible that we are
running a BE QEMU?

Re: [Qemu-devel] [PATCH v1 1/2] target/arm: kvm64 make guest debug AA32 break point aware

[Alex Bennée]

Richard Henderson  writes:

> On 12/13/18 8:55 AM, Alex Bennée wrote:
>>
>> Ard Biesheuvel  writes:
>>
>>> Hi Alex,
>>>
>>> Thanks again for looking into this.
>>>
>>> On Thu, 13 Dec 2018 at 12:55, Alex Bennée  wrote:
>> 
>>>

  int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint 
 *bp)
  {
 +CPUARMState *env = _CPU(cs)->env;
 +int el = arm_current_el(env);
 +bool is_aa64 = arm_el_is_aa64(env, el);
 +const uint32_t *bpi = is_aa64 ? _insn : _insn;
 +
  if (have_guest_debug) {
  if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)>saved_insn, 
 4, 0) ||
 -cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)_insn, 4, 1)) {
 +cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)bpi, 4, 1)) {
>>>
>>> Should we be dealing with endianness here?
>>>
>> 
>>
>> I don't think so - everything eventually ends up (ld|st)n_p which deals
>> with the endianness details.
>
> I think Ard is right.  You need to consider dynamic endianness with
>
> bswap_code(arm_sctlr_b(env))

*sigh* I guess. It of course still a heuristic that can break because we
don't know if the system will have switched mode by the time it gets to
the breakpoint.

--
Alex Bennée

Re: [Qemu-devel] [PATCH v10 8/9] qcow2: skip writing zero buffers to empty COW areas

[Vladimir Sementsov-Ogievskiy]

03.12.2018 13:14, Anton Nefedov wrote:
> If COW areas of the newly allocated clusters are zeroes on the backing image,
> efficient bdrv_write_zeroes(flags=BDRV_REQ_ALLOCATE) can be used on the whole
> cluster instead of writing explicit zero buffers later in perform_cow().
> 
> iotest 060:
> write to the discarded cluster does not trigger COW anymore.
> Use a backing image instead.
> 

[..]

> --- a/tests/qemu-iotests/060
> +++ b/tests/qemu-iotests/060
> @@ -150,27 +150,33 @@ $QEMU_IO -c "$OPEN_RO" -c "read -P 1 0 512" | 
> _filter_qemu_io
>   echo
>   echo "=== Testing overlap while COW is in flight ==="
>   echo
> +BACKING_IMG=$TEST_IMG.base
> +TEST_IMG=$BACKING_IMG _make_test_img 1G
> +
> +$QEMU_IO -c 'write 64k 64k' "$BACKING_IMG" | _filter_qemu_io
> +
>   # compat=0.10 is required in order to make the following discard actually
> -# unallocate the sector rather than make it a zero sector - we want COW, 
> after
> -# all.
> -IMGOPTS='compat=0.10' _make_test_img 1G
> +# unallocate the sector rather than make it a zero sector as we would like
> +# to reuse it for another guest offset
> +IMGOPTS='compat=0.10' _make_test_img -b "$BACKING_IMG" 1G
>   # Write two clusters, the second one enforces creation of an L2 table after
>   # the first data cluster.
>   $QEMU_IO -c 'write 0k 64k' -c 'write 512M 64k' "$TEST_IMG" | _filter_qemu_io
> -# Discard the first cluster. This cluster will soon enough be reallocated and
> -# used for COW.
> +# Discard the first cluster. This cluster will soon enough be reallocated
>   $QEMU_IO -c 'discard 0k 64k' "$TEST_IMG" | _filter_qemu_io
>   # Now, corrupt the image by marking the second L2 table cluster as free.
>   poke_file "$TEST_IMG" '131084' "\x00\x00" # 0x2000c
> -# Start a write operation requiring COW on the image stopping it right before
> -# doing the read; then, trigger the corruption prevention by writing 
> anything to
> -# any unallocated cluster, leading to an attempt to overwrite the second L2
> +# Start a write operation requiring COW on the image;
> +# this write will reuse the host offset released by a previous discard.
> +# Stop it right before doing the read.
> +# Then, trigger the corruption prevention by writing anything to
> +# another unallocated cluster, leading to an attempt to overwrite the second 
> L2
>   # table. Finally, resume the COW write and see it fail (but not crash).
>   echo "open -o file.driver=blkdebug $TEST_IMG
>   break cow_read 0
> -aio_write 0k 1k
> +aio_write 64k 1k
>   wait_break 0
> -write 64k 64k
> +write 128k 64k

don't understand why you need these changes.

works for me, without them, if write to backing at 0 offset, of course.

As I understand, discard create unallocated holes in top qcow2 for old qcow2 
version.

>   resume 0" | $QEMU_IO | _filter_qemu_io
>   
>   echo
> diff --git a/tests/qemu-iotests/060.out b/tests/qemu-iotests/060.out
> index d67c6234a4..ec8f15d2f0 100644
> --- a/tests/qemu-iotests/060.out
> +++ b/tests/qemu-iotests/060.out
> @@ -97,7 +97,10 @@ read 512/512 bytes at offset 0
>   
>   === Testing overlap while COW is in flight ===
>   
> -Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
> +Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=1073741824
> +wrote 65536/65536 bytes at offset 65536
> +64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
> backing_file=TEST_DIR/t.IMGFMT.base
>   wrote 65536/65536 bytes at offset 0
>   64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>   wrote 65536/65536 bytes at offset 536870912
> 


-- 
Best regards,
Vladimir

Re: [Qemu-devel] [PATCH v3] s390x/pci: add common function measurement block

[Pierre Morel]

On 14/12/2018 15:20, Cornelia Huck wrote:

On Fri, 14 Dec 2018 15:11:18 +0100
Pierre Morel  wrote:


From: Yi Min Zhao 

Common function measurement block is used to report zPCI internal
counters of successful pcilg/stg/stb and rpcit instructions to
a memory location provided by the program.

This patch introduces a new ZpciFmb structure and schedules a timer
callback to copy the zPCI measures to the FMB in the guest memory
at an interval time set to 4s by default.

An error while attemping to update the FMB, would generated an error
event to the guest.

The pcilg/stg/stb and rpcit interception handlers issue, increase
the related counter on success.
The guest shall pass a null FMBA (FMB address) in the FIB (Function
Information Block) when it issues a Modify PCI Function Control
instrcuction to switch off FMB and stop the corresponding timer.

Signed-off-by: Yi Min Zhao 
Signed-off-by: Pierre Morel 
---
  hw/s390x/s390-pci-bus.c  |   4 +-
  hw/s390x/s390-pci-bus.h  |  29 ++
  hw/s390x/s390-pci-inst.c | 138 +--
  hw/s390x/s390-pci-inst.h |   1 +
  4 files changed, 168 insertions(+), 4 deletions(-)


Are there any changes in there other than the endianness handling?



Hum, yes.
Forget this patch, I post a new one with a complete changelog.
I also noticed that I forgot an error report.

Regards,
Pierre


--
Pierre Morel
Linux/KVM/QEMU in Böblingen - Germany

Re: [Qemu-devel] [PATCH 0/1] checkpatch: checker for comment block

[Markus Armbruster]

Peter Maydell  writes:

> On Fri, 14 Dec 2018 at 12:31, Markus Armbruster  wrote:
>> Peter Maydell  writes:
>> > On Fri, 14 Dec 2018 at 06:29, Markus Armbruster  wrote:
>> > I have to admit I never really understood what tweak
>> > you wanted making to the commit message. I'm happy
>> > to make it clearer: do you want to suggest a rewording?
>>
>> The commit message claims "(The only changes needed are that Linux's
>> checkpatch.pl WARN() function takes an extra argument that ours does
>> not.)"  This isn't the case.  You also dropped the kernel's "Networking
>> with an initial /*" special case.
>
> The bit of the commit message you didn't quote says
> "by backporting the relevant
> parts of the Linux kernel's checkpatch.pl. (The only changes
> needed are that Linux's checkpatch.pl WARN() function takes
> an extra argument that ours does not.)"
>
> The networking special case is not a "relevant part", which
> is why it's not in the patch. The bracketed statement applies
> to the code in the patch, not any lumps of code in the
> kernel's checkpatch that are not in the patch.

I understand you logic now.

> Anyway, I've adjusted the commit message as you suggest.
>
> Since we seem to now have consensus on the checkpatch patch,
> I'm going to put it into the "misc" pull request I'm putting
> together.

Thanks!

Re: [Qemu-devel] Loading snapshot with readonly qcow2 image

[Michael Spradling]

On Dec 13 15:43, Eric Blake wrote:
> On 12/13/18 12:33 PM, Michael Spradling wrote:
> 
> > > > My question is has anyone looked into loading snapshots from a backing
> > > > file?  I have attempted to look through the code and this looks to be
> > > > difficult.  If I attempt to add support for this is there any general
> > > > advice to follow?  Any other ideas?
> > > 
> > > 'qemu-nbd -l' can serve snapshots from a qcow2 file; perhaps that can be
> > > used to cobble together something that works for your needs?
> > > 
> 
> > 
> > I looked at "qemu-nbd -l" and this seems to only export a readonly
> > interface.  Really, what I need is a writable temp file that can load a
> > snapshot snapshot.
> 
> Can you combine -s (create a writable temp file) with -l to get what you
> want?
> 
> /me tries:
> 
> $ qemu-img create -f qcow2 a 1M
> Formatting 'a', fmt=qcow2 size=1048576 cluster_size=65536 lazy_refcounts=off
> refcount_bits=16
> $ qemu-io -c 'w -P 1 0 512' a
> wrote 512/512 bytes at offset 0
> 512 bytes, 1 ops; 0.0487 sec (10.257 KiB/sec and 20.5137 ops/sec)
> $ qemu-img snapshot -c snap a
> $ qemu-io -c 'w -P 2 0 512' a
> wrote 512/512 bytes at offset 0
> 512 bytes, 1 ops; 0.0752 sec (6.645 KiB/sec and 13.2903 ops/sec)
> $ qemu-nbd -l snap -s a
> Failed to load snapshot: Can't find snapshot
> 
> I can confirm that 'qemu-nbd -s a' lets me write data that is discarded on
> disconnect (lsof says a temp file in /var/tmp/vl.XX was created); and
> that 'qemu-nbd -l snap a' lets me read the snapshot data. But mixing the two
> fails, and it would be a nice bug to fix.

I briefly looked at the code and is seams to be using the same base
functions as qemu does.  So, if I get this working for the model it
might also start working for qemu-nbd.

> 
> > 
> > Please excuse and correct me if I get some of the terminology of the
> > sections below wrong.
> > 
> > I went down the path of hacking up some of the qemu qcow2 file system
> > code to see if I can achieve the ability to restore a snapshot from a
> > backing file to the temporarily created "-snapshot" qcow2 image.  The
> > backing file has been marked readonly by the filesystem and the active
> > image file was created with the "-snapshot" option.  I spend some time
> > reading the qcow2 documentation and it seems I have to copy the l1 and
> > l2 table values(are these actual host clusters) from the backing file
> > snapshot to the active images l1 and l2 tables.  Is there anything else
> > that may need updated that I have not yet stumbled upon?
> 
> Mucking with the l1 and l2 tables implies that you are directly manipulating
> qcow2 contents.  It's much nicer if you can come up with a solution where
> qemu-img does all the qcow2 work for you, and you just worry about
> guest-visible data.  Or are you actually patching the code compiled into
> qemu-img?
> 
Ideally, I want to not modify old images or create new images with
qemu-img, so I have been not modifing qemu-img, but qemu directly
itself.  My use case will have several snapshots in an image.(say
100).  I will then later resume each of these snapshots in a qemu
session in parallel.  This is why I have gone done the route of modifying
the temp snapshots file /var/tmp/vl.X L1 and l2 tables.  My
understanding is if these are updated and the cluster doesn't exists in
the temp file the code will then look for it in the backing file.  Still
researching this area.

> > 
> > I still don't have this working yet and I believe my area of problems is
> > qcow2_update_snapshot_refcount.  Can anyone explain what this does
> > exactly.  It seems the function does three different things based on the
> > value of addend, either -1, 0, 1, but its somewhat unclear.
> 
> Every cluster of qcow2 is reference-counted, to track which portions of the
> file are (supposed to be) in use according to following the metadata trails.
> When internal snapshots are used, this is implemented by incrementing the
> refcount for each cluster that is reachable both from the snapshot and from
> the current L1 table (update_snapshot_refcount +1), then when writing to the
> cluster we break the reference count by writing the new data to a new
> allocation and decrementing the reference count of the old cluster. When
> trimming clusters, we decrement the refcount, and if it goes to 0 the
> cluster can be reused for something else.

I think I understand this.  That would satifys addend being a -1 or 1.
I am still unclear why you would call the fuction with addend being 0.
> 
> -- 
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.   +1-919-301-3266
> Virtualization:  qemu.org | libvirt.org

Thanks for your help so far.
Michael

Re: [Qemu-devel] [RFC v2 05/38] plugin: add user-facing API

[Aaron Lindsay]

On Dec 14 10:57, Aaron Lindsay wrote:
> One thing I'd like to see is support for unregistering callbacks once
> they are registered.

By the way, I'm willing to work on this if we agree it sounds reasonable
and fits in with the rest of your implementation.

-Aaron

Re: [Qemu-devel] [PULL 00/27] ppc-for-4.0 queue 20181213

[Peter Maydell]

On Thu, 13 Dec 2018 at 04:01, David Gibson  wrote:
>
> The following changes since commit 4b3aab204204ca742836219b97b538d90584f4f2:
>
>   Merge remote-tracking branch 
> 'remotes/vivier2/tags/trivial-patches-pull-request' into staging (2018-12-11 
> 22:26:44 +)
>
> are available in the Git repository at:
>
>   git://github.com/dgibson/qemu.git tags/ppc-for-4.0-20181213
>
> for you to fetch changes up to 67888a17b6683600ae3fa64ca275c737ba8a9a45:
>
>   spapr/xive: use the VCPU id as a NVT identifier (2018-12-13 09:44:04 +1100)
>
> 
> ppc patch queue 2018-12-13
>
> Here's the first ppc and spapr pull request for 4.0.  Highlights are:
>
>  * The start of support for the POWER9 "XIVE" interrupt controller
>(not complete enough to use yet, but we're getting there)
>  * A number of g_new vs. g_malloc cleanups
>  * Some IRQ wiring cleanups
>  * A fix for how we advertise NUMA nodes to the guest for pseries
>
> ---


Compile errors in the windows cross-build. These look like
they're assumptions that "long" is 64 bits, which it is not on Windows.
For instance the PPC_BIT macro should be using the ULL suffix, not UL
("UL" is almost always a bug: either the constant is 32-bit, in
which case "U" is what you want, or it's 64-bit and you need "ULL").

Using __builtin_ffsl() directly in target/ppc/cpu.h also looks
a bit dubious -- this should be rephrased to use ctz32() or ctz64()
instead.

In file included from /home/petmay01/qemu-for-merges/hw/intc/xive.c:13:0:
/home/petmay01/qemu-for-merges/hw/intc/xive.c: In function 'xive_router_notify':
/home/petmay01/qemu-for-merges/target/ppc/cpu.h:76:33: error: overflow
in implicit constant conversion [-Werror=overflow]
 #define PPC_BITMASK(bs, be) ((PPC_BIT(bs) - PPC_BIT(be)) | PPC_BIT(bs))
 ^
/home/petmay01/qemu-for-merges/target/ppc/cpu.h:84:50: note: in
definition of macro 'MASK_TO_LSH'
 # define MASK_TO_LSH(m)  (__builtin_ffsl(m) - 1)
  ^
/home/petmay01/qemu-for-merges/include/hw/ppc/xive_regs.h:129:34:
note: in expansion of macro 'GETFIELD'
 #define GETFIELD_BE64(m, v)  GETFIELD(m, be64_to_cpu(v))
  ^
/home/petmay01/qemu-for-merges/hw/intc/xive.c:1366:28: note: in
expansion of macro 'GETFIELD_BE64'
GETFIELD_BE64(EAS_END_BLOCK, eas.w),
^
/home/petmay01/qemu-for-merges/include/hw/ppc/xive_regs.h:120:25:
note: in expansion of macro 'PPC_BITMASK'
 #define EAS_END_BLOCK   PPC_BITMASK(4, 7)/* Destination END block# */
 ^
/home/petmay01/qemu-for-merges/hw/intc/xive.c:1366:42: note: in
expansion of macro 'EAS_END_BLOCK'
GETFIELD_BE64(EAS_END_BLOCK, eas.w),
  ^
/home/petmay01/qemu-for-merges/target/ppc/cpu.h:89:46: error: right
shift count is negative [-Werror=shift-count-negative]
 #define GETFIELD(m, v)  (((v) & (m)) >> MASK_TO_LSH(m))
  ^
/home/petmay01/qemu-for-merges/include/hw/ppc/xive_regs.h:129:34:
note: in expansion of macro 'GETFIELD'
 #define GETFIELD_BE64(m, v)  GETFIELD(m, be64_to_cpu(v))
  ^
/home/petmay01/qemu-for-merges/hw/intc/xive.c:1366:28: note: in
expansion of macro 'GETFIELD_BE64'
GETFIELD_BE64(EAS_END_BLOCK, eas.w),
^
/home/petmay01/qemu-for-merges/target/ppc/cpu.h:76:33: error: overflow
in implicit constant conversion [-Werror=overflow]
 #define PPC_BITMASK(bs, be) ((PPC_BIT(bs) - PPC_BIT(be)) | PPC_BIT(bs))
 ^
/home/petmay01/qemu-for-merges/target/ppc/cpu.h:84:50: note: in
definition of macro 'MASK_TO_LSH'
 # define MASK_TO_LSH(m)  (__builtin_ffsl(m) - 1)
  ^
/home/petmay01/qemu-for-merges/include/hw/ppc/xive_regs.h:129:34:
note: in expansion of macro 'GETFIELD'
 #define GETFIELD_BE64(m, v)  GETFIELD(m, be64_to_cpu(v))
  ^
/home/petmay01/qemu-for-merges/hw/intc/xive.c:1367:28: note: in
expansion of macro 'GETFIELD_BE64'
GETFIELD_BE64(EAS_END_INDEX, eas.w),
^
/home/petmay01/qemu-for-merges/include/hw/ppc/xive_regs.h:121:25:
note: in expansion of macro 'PPC_BITMASK'
 #define EAS_END_INDEX   PPC_BITMASK(8, 31)   /* Destination END index */
 ^
/home/petmay01/qemu-for-merges/hw/intc/xive.c:1367:42: note: in
expansion of macro 'EAS_END_INDEX'
GETFIELD_BE64(EAS_END_INDEX, eas.w),
  ^
/home/petmay01/qemu-for-merges/target/ppc/cpu.h:89:46: error: right
shift count is negative [-Werror=shift-count-negative]
 #define GETFIELD(m, v)  

Re: [Qemu-devel] [RFC v2 05/38] plugin: add user-facing API

[Aaron Lindsay]

Emilio,

First, thanks for putting this together - I think everyone doing this
sort of thing will benefit if we're able to agree on one upstream plugin
interface.

One thing I'd like to see is support for unregistering callbacks once
they are registered. For instance, you can imagine that a plugin may
have 'modality', where it may care about tracing very detailed
information in one mode, but trace only limited information otherwise -
perhaps only enough to figure out when it needs to switch back to the
other mode.

I added a function to the user-facing plugin API in my own version of
Pavel's plugin patchset to clear all existing plugin instrumentation,
allowing the plugin to drop instrumentation if it was no longer
interested. Of course, you could always add logic to a plugin to ignore
unwanted callbacks, but it could be significantly more efficient to
remove them entirely. In Pavel's patchset, this was essentially just a
call to tb_flush(), though I think it would be a little more involved
for yours.

-Aaron

[Qemu-devel] [PATCH v3] Add getsockopt for settable SOL_IPV6 options

[tom . deseyn]

From: Tom Deseyn 

Thank you for reviewing Laurant.
Sorry for missing history, I'm not used to sending patches via mail.
I got an email about code style. For now, I'm sticking to the style
that is used in the function.

v2: default to unimplemented
v3: match kernel behavior

Signed-off-by: Tom Deseyn 
---
 linux-user/syscall.c | 36 
 1 file changed, 36 insertions(+)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 280137da8c..f103437f26 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -2352,6 +2352,42 @@ static abi_long do_getsockopt(int sockfd, int level, int 
optname,
 break;
 }
 break;
+case SOL_IPV6:
+switch (optname) {
+case IPV6_MTU_DISCOVER:
+case IPV6_MTU:
+case IPV6_V6ONLY:
+case IPV6_RECVPKTINFO:
+case IPV6_UNICAST_HOPS:
+case IPV6_MULTICAST_HOPS:
+case IPV6_MULTICAST_LOOP:
+case IPV6_RECVERR:
+case IPV6_RECVHOPLIMIT:
+case IPV6_2292HOPLIMIT:
+case IPV6_CHECKSUM: {
+void* target_addr;
+if (get_user_u32(len, optlen))
+return -TARGET_EFAULT;
+if (len < 0)
+return -TARGET_EINVAL;
+lv = sizeof(val);
+ret = get_errno(getsockopt(sockfd, level, optname, , ));
+if (ret < 0)
+return ret;
+if (lv < len)
+len = lv;
+if (put_user_u32(len, optlen))
+return -TARGET_EFAULT;
+target_addr = lock_user(VERIFY_WRITE, optval_addr, len, 0);
+tswap32s((uint32_t*));
+memcpy(target_addr, , len);
+unlock_user(target_addr, optval_addr, len);
+break;
+}
+default:
+goto unimplemented;
+}
+break;
 default:
 unimplemented:
 gemu_log("getsockopt level=%d optname=%d not yet supported\n",
-- 
2.19.2

Re: [Qemu-devel] [PATCH v4 16/18] xen: automatically create XenBlockDevice-s

[Paul Durrant]

> -Original Message-
> From: Xen-devel [mailto:xen-devel-boun...@lists.xenproject.org] On Behalf
> Of Paul Durrant
> Sent: 14 December 2018 14:50
> To: 'Kevin Wolf' 
> Cc: xen-de...@lists.xenproject.org; Stefano Stabellini
> ; qemu-devel@nongnu.org; qemu-bl...@nongnu.org;
> Max Reitz 
> Subject: Re: [Xen-devel] [PATCH v4 16/18] xen: automatically create
> XenBlockDevice-s
> 
> > -Original Message-
> [snip]
> > > +
> > > +blockdev->auto_iothread = iothread;
> > > +
> > > +object_property_set_bool(OBJECT(dev), true, "realized",
> > _err);
> > > +if (local_err) {
> > > +error_propagate_prepend(errp, local_err,
> > > +"initialization of device %s failed:
> ",
> > > +type);
> > > +goto unref;
> > > +}
> > > +
> > > +blockdev_mark_auto_del(blk);
> >
> > You don't need this one any more then (if you look into the details,
> > it's one of the more confusing parts of the drive_*() magic, so it's
> > good to get rid of it). When you use the anonymous BlockBackend created
> > by the qdev drive property (because you passed it a node-name rather
> > than a BlockBackend name) means that the BlockBackend disappears
> > together with the drive.
> >
> > Note that explicitly created block nodes must also be unreferenced
> > explicitly (bdrv_open() should be paired with bdrv_unref() and
> > qmp_blockdev_add() with qmp_blockdev_del()). Maybe XenBackendInfo needs
> > a .destroy callback so we can do destruction symmetrically to device
> > creation?
> >
> 
> I have something implemented using qmp_blockdev_add() now and it seems to
> work, but when I call into qmp_blockdev_del() (passing in the node-name I
> used to the set the "drive" parameter) during unrealize then it tells me
> that the device is in use. Do I need a callback that runs after unrealize
> of the device?

I have coded it up now and apparently I do.

  Paul

> 
>   Paul
> 
> > > +return;
> > > +
> > > +unref:
> > > +if (dev) {
> > > +object_unparent(OBJECT(dev));
> > > +}
> > > +
> > > +if (iothread) {
> > > +iothread_destroy(iothread);
> > > +}
> > > +
> > > +if (blk) {
> > > +monitor_remove_blk(blk);
> > > +blk_unref(blk);
> > > +}
> > > +}
> >
> > Kevin
> 
> ___
> Xen-devel mailing list
> xen-de...@lists.xenproject.org
> https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Qemu-devel] [PATCH 2/2] avoid TABs in files that only contain a few

[Michael S. Tsirkin]

On Thu, Dec 13, 2018 at 11:37:37PM +0100, Paolo Bonzini wrote:
> Most files that have TABs only contain a handful of them.  Change
> them to spaces so that we don't confuse people.
> 
> disas, standard-headers, linux-headers and libdecnumber are imported
> from other projects and probably should be exempted from the check.
> Outside those, after this patch the following files still contain both
> 8-space and TAB sequences at the beginning of the line.  Many of them
> have a majority of TABs, or were initially committed with all tabs.
> 
> bsd-user/i386/target_syscall.h
> bsd-user/x86_64/target_syscall.h
> crypto/aes.c
> hw/audio/fmopl.c
> hw/audio/fmopl.h
> hw/block/tc58128.c
> hw/display/cirrus_vga.c
> hw/display/xenfb.c
> hw/dma/etraxfs_dma.c
> hw/intc/sh_intc.c
> hw/misc/mst_fpga.c
> hw/net/pcnet.c
> hw/sh4/sh7750.c
> hw/timer/m48t59.c
> hw/timer/sh_timer.c
> include/crypto/aes.h
> include/disas/bfd.h
> include/hw/sh4/sh.h
> libdecnumber/decNumber.c
> linux-headers/asm-generic/unistd.h
> linux-headers/linux/kvm.h
> linux-user/alpha/target_syscall.h
> linux-user/arm/nwfpe/double_cpdo.c
> linux-user/arm/nwfpe/fpa11_cpdt.c
> linux-user/arm/nwfpe/fpa11_cprt.c
> linux-user/arm/nwfpe/fpa11.h
> linux-user/flat.h
> linux-user/flatload.c
> linux-user/i386/target_syscall.h
> linux-user/ppc/target_syscall.h
> linux-user/sparc/target_syscall.h
> linux-user/syscall.c
> linux-user/syscall_defs.h
> linux-user/x86_64/target_syscall.h
> slirp/cksum.c
> slirp/if.c
> slirp/ip.h
> slirp/ip_icmp.c
> slirp/ip_icmp.h
> slirp/ip_input.c
> slirp/ip_output.c
> slirp/mbuf.c
> slirp/misc.c
> slirp/sbuf.c
> slirp/socket.c
> slirp/socket.h
> slirp/tcp_input.c
> slirp/tcpip.h
> slirp/tcp_output.c
> slirp/tcp_subr.c
> slirp/tcp_timer.c
> slirp/tftp.c
> slirp/udp.c
> slirp/udp.h
> target/cris/cpu.h
> target/cris/mmu.c
> target/cris/op_helper.c
> target/sh4/helper.c
> target/sh4/op_helper.c
> target/sh4/translate.c
> tcg/sparc/tcg-target.inc.c
> tests/tcg/cris/check_addo.c
> tests/tcg/cris/check_moveq.c
> tests/tcg/cris/check_swap.c
> tests/tcg/multiarch/test-mmap.c
> ui/vnc-enc-hextile-template.h
> ui/vnc-enc-zywrle.h
> util/envlist.c
> util/readline.c
> 
> The following have only TABs:
> 
> bsd-user/i386/target_signal.h
> bsd-user/sparc64/target_signal.h
> bsd-user/sparc64/target_syscall.h
> bsd-user/sparc/target_signal.h
> bsd-user/sparc/target_syscall.h
> bsd-user/x86_64/target_signal.h
> crypto/desrfb.c
> hw/audio/intel-hda-defs.h
> hw/core/uboot_image.h
> hw/sh4/sh7750_regnames.c
> hw/sh4/sh7750_regs.h
> include/hw/cris/etraxfs_dma.h
> linux-user/alpha/termbits.h
> linux-user/arm/nwfpe/fpopcode.h
> linux-user/arm/nwfpe/fpsr.h
> linux-user/arm/syscall_nr.h
> linux-user/arm/target_signal.h
> linux-user/cris/target_signal.h
> linux-user/i386/target_signal.h
> linux-user/linux_loop.h
> linux-user/m68k/target_signal.h
> linux-user/microblaze/target_signal.h
> linux-user/mips64/target_signal.h
> linux-user/mips/target_signal.h
> linux-user/mips/target_syscall.h
> linux-user/mips/termbits.h
> linux-user/ppc/target_signal.h
> linux-user/sh4/target_signal.h
> linux-user/sh4/termbits.h
> linux-user/sparc64/target_syscall.h
> linux-user/sparc/target_signal.h
> linux-user/x86_64/target_signal.h
> linux-user/x86_64/termbits.h
> pc-bios/optionrom/optionrom.h
> slirp/mbuf.h
> slirp/misc.h
> slirp/sbuf.h
> slirp/tcp.h
> slirp/tcp_timer.h
> slirp/tcp_var.h
> target/i386/svm.h
> target/sparc/asi.h
> target/xtensa/core-dc232b/xtensa-modules.inc.c
> target/xtensa/core-dc233c/xtensa-modules.inc.c
> target/xtensa/core-de212/core-isa.h
> target/xtensa/core-de212/xtensa-modules.inc.c
> target/xtensa/core-fsf/xtensa-modules.inc.c
> target/xtensa/core-sample_controller/core-isa.h
> target/xtensa/core-sample_controller/xtensa-modules.inc.c
> target/xtensa/core-test_kc705_be/core-isa.h
> target/xtensa/core-test_kc705_be/xtensa-modules.inc.c
> tests/tcg/cris/check_abs.c
> tests/tcg/cris/check_addc.c
> tests/tcg/cris/check_addcm.c
> tests/tcg/cris/check_addoq.c
> tests/tcg/cris/check_bound.c
> tests/tcg/cris/check_ftag.c
> tests/tcg/cris/check_int64.c
> tests/tcg/cris/check_lz.c
> tests/tcg/cris/check_openpf5.c
> tests/tcg/cris/check_sigalrm.c
> tests/tcg/cris/crisutils.h
> tests/tcg/cris/sys.c
> tests/tcg/i386/test-i386-ssse3.c
> ui/vgafont.h
> 
> Signed-off-by: Paolo Bonzini 

Seems sane:
Reviewed-by: Michael S. Tsirkin 

[Qemu-devel] [PATCH] .shippable.yml: disable the win cross tests

[Alex Bennée]

The pkg.mxe.cc package repositories have been down for the last two
weeks causing the builds to fail when shippable re-builds the
containers.

This is really just a sticking plaster until we can get our own docker
hub images properly setup so we can avoid having dependencies on
external repos.

Signed-off-by: Alex Bennée 
Cc: Philippe Mathieu-Daudé 
---
 .shippable.yml | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/.shippable.yml b/.shippable.yml
index f74a3de3ff..f2ffef21d1 100644
--- a/.shippable.yml
+++ b/.shippable.yml
@@ -7,10 +7,11 @@ env:
   matrix:
 - IMAGE=debian-amd64
   TARGET_LIST=x86_64-softmmu,x86_64-linux-user
-- IMAGE=debian-win32-cross
-  TARGET_LIST=arm-softmmu,i386-softmmu,lm32-softmmu
-- IMAGE=debian-win64-cross
-  TARGET_LIST=aarch64-softmmu,sparc64-softmmu,x86_64-softmmu
+# currently disabled as the mxe.cc repos are down
+# - IMAGE=debian-win32-cross
+#   TARGET_LIST=arm-softmmu,i386-softmmu,lm32-softmmu
+# - IMAGE=debian-win64-cross
+#   TARGET_LIST=aarch64-softmmu,sparc64-softmmu,x86_64-softmmu
 - IMAGE=debian-armel-cross
   TARGET_LIST=arm-softmmu,arm-linux-user,armeb-linux-user
 - IMAGE=debian-armhf-cross
-- 
2.17.1

Re: [Qemu-devel] [PATCH 2/2] avoid TABs in files that only contain a few

[Stefan Hajnoczi]

On Thu, Dec 13, 2018 at 11:37:37PM +0100, Paolo Bonzini wrote:
> Most files that have TABs only contain a handful of them.  Change
> them to spaces so that we don't confuse people.
> 
> disas, standard-headers, linux-headers and libdecnumber are imported
> from other projects and probably should be exempted from the check.
> Outside those, after this patch the following files still contain both
> 8-space and TAB sequences at the beginning of the line.  Many of them
> have a majority of TABs, or were initially committed with all tabs.
> 
> bsd-user/i386/target_syscall.h
> bsd-user/x86_64/target_syscall.h
> crypto/aes.c
> hw/audio/fmopl.c
> hw/audio/fmopl.h
> hw/block/tc58128.c
> hw/display/cirrus_vga.c
> hw/display/xenfb.c
> hw/dma/etraxfs_dma.c
> hw/intc/sh_intc.c
> hw/misc/mst_fpga.c
> hw/net/pcnet.c
> hw/sh4/sh7750.c
> hw/timer/m48t59.c
> hw/timer/sh_timer.c
> include/crypto/aes.h
> include/disas/bfd.h
> include/hw/sh4/sh.h
> libdecnumber/decNumber.c
> linux-headers/asm-generic/unistd.h
> linux-headers/linux/kvm.h
> linux-user/alpha/target_syscall.h
> linux-user/arm/nwfpe/double_cpdo.c
> linux-user/arm/nwfpe/fpa11_cpdt.c
> linux-user/arm/nwfpe/fpa11_cprt.c
> linux-user/arm/nwfpe/fpa11.h
> linux-user/flat.h
> linux-user/flatload.c
> linux-user/i386/target_syscall.h
> linux-user/ppc/target_syscall.h
> linux-user/sparc/target_syscall.h
> linux-user/syscall.c
> linux-user/syscall_defs.h
> linux-user/x86_64/target_syscall.h
> slirp/cksum.c
> slirp/if.c
> slirp/ip.h
> slirp/ip_icmp.c
> slirp/ip_icmp.h
> slirp/ip_input.c
> slirp/ip_output.c
> slirp/mbuf.c
> slirp/misc.c
> slirp/sbuf.c
> slirp/socket.c
> slirp/socket.h
> slirp/tcp_input.c
> slirp/tcpip.h
> slirp/tcp_output.c
> slirp/tcp_subr.c
> slirp/tcp_timer.c
> slirp/tftp.c
> slirp/udp.c
> slirp/udp.h
> target/cris/cpu.h
> target/cris/mmu.c
> target/cris/op_helper.c
> target/sh4/helper.c
> target/sh4/op_helper.c
> target/sh4/translate.c
> tcg/sparc/tcg-target.inc.c
> tests/tcg/cris/check_addo.c
> tests/tcg/cris/check_moveq.c
> tests/tcg/cris/check_swap.c
> tests/tcg/multiarch/test-mmap.c
> ui/vnc-enc-hextile-template.h
> ui/vnc-enc-zywrle.h
> util/envlist.c
> util/readline.c
> 
> The following have only TABs:
> 
> bsd-user/i386/target_signal.h
> bsd-user/sparc64/target_signal.h
> bsd-user/sparc64/target_syscall.h
> bsd-user/sparc/target_signal.h
> bsd-user/sparc/target_syscall.h
> bsd-user/x86_64/target_signal.h
> crypto/desrfb.c
> hw/audio/intel-hda-defs.h
> hw/core/uboot_image.h
> hw/sh4/sh7750_regnames.c
> hw/sh4/sh7750_regs.h
> include/hw/cris/etraxfs_dma.h
> linux-user/alpha/termbits.h
> linux-user/arm/nwfpe/fpopcode.h
> linux-user/arm/nwfpe/fpsr.h
> linux-user/arm/syscall_nr.h
> linux-user/arm/target_signal.h
> linux-user/cris/target_signal.h
> linux-user/i386/target_signal.h
> linux-user/linux_loop.h
> linux-user/m68k/target_signal.h
> linux-user/microblaze/target_signal.h
> linux-user/mips64/target_signal.h
> linux-user/mips/target_signal.h
> linux-user/mips/target_syscall.h
> linux-user/mips/termbits.h
> linux-user/ppc/target_signal.h
> linux-user/sh4/target_signal.h
> linux-user/sh4/termbits.h
> linux-user/sparc64/target_syscall.h
> linux-user/sparc/target_signal.h
> linux-user/x86_64/target_signal.h
> linux-user/x86_64/termbits.h
> pc-bios/optionrom/optionrom.h
> slirp/mbuf.h
> slirp/misc.h
> slirp/sbuf.h
> slirp/tcp.h
> slirp/tcp_timer.h
> slirp/tcp_var.h
> target/i386/svm.h
> target/sparc/asi.h
> target/xtensa/core-dc232b/xtensa-modules.inc.c
> target/xtensa/core-dc233c/xtensa-modules.inc.c
> target/xtensa/core-de212/core-isa.h
> target/xtensa/core-de212/xtensa-modules.inc.c
> target/xtensa/core-fsf/xtensa-modules.inc.c
> target/xtensa/core-sample_controller/core-isa.h
> target/xtensa/core-sample_controller/xtensa-modules.inc.c
> target/xtensa/core-test_kc705_be/core-isa.h
> target/xtensa/core-test_kc705_be/xtensa-modules.inc.c
> tests/tcg/cris/check_abs.c
> tests/tcg/cris/check_addc.c
> tests/tcg/cris/check_addcm.c
> tests/tcg/cris/check_addoq.c
> tests/tcg/cris/check_bound.c
> tests/tcg/cris/check_ftag.c
> tests/tcg/cris/check_int64.c
> tests/tcg/cris/check_lz.c
> tests/tcg/cris/check_openpf5.c
> tests/tcg/cris/check_sigalrm.c
> tests/tcg/cris/crisutils.h
> tests/tcg/cris/sys.c
> tests/tcg/i386/test-i386-ssse3.c
> ui/vgafont.h
> 
> Signed-off-by: Paolo Bonzini 
> ---
>  block/bochs.c  | 22 ++---
>  block/file-posix.c |  2 +-
>  block/file-win32.c 

Re: [Qemu-devel] [PATCH v3 08/16] virtio: split vhost user scsi bits from virtio-pci

[Felipe Franciosi]

> On Dec 14, 2018, at 9:14 AM, Thomas Huth  wrote:
> 
> On 2018-12-13 22:00, Juan Quintela wrote:
>> Reviewed-by: Laurent Vivier 
>> Signed-off-by: Juan Quintela 
>> ---
>> hw/virtio/Makefile.objs |   1 +
>> hw/virtio/vhost-user-scsi-pci.c | 101 
>> hw/virtio/virtio-pci.c  |  58 --
>> hw/virtio/virtio-pci.h  |  11 
>> 4 files changed, 102 insertions(+), 69 deletions(-)
>> create mode 100644 hw/virtio/vhost-user-scsi-pci.c
>> 
>> diff --git a/hw/virtio/Makefile.objs b/hw/virtio/Makefile.objs
>> index 35b7698446..f851a6f2b5 100644
>> --- a/hw/virtio/Makefile.objs
>> +++ b/hw/virtio/Makefile.objs
>> @@ -14,6 +14,7 @@ obj-$(CONFIG_VHOST_VSOCK) += vhost-vsock.o
>> ifeq ($(CONFIG_PCI),y)
>> obj-$(CONFIG_VHOST_VSOCK) += vhost-vsock-pci.o
>> obj-$(CONFIG_VHOST_USER_BLK) += vhost-user-blk-pci.o
>> +obj-$(CONFIG_VHOST_USER_SCSI) += vhost-user-scsi-pci.o
>> obj-$(CONFIG_VIRTIO_INPUT_HOST) += virtio-input-host-pci.o
>> obj-$(CONFIG_VIRTIO_INPUT) += virtio-input-pci.o
>> obj-$(CONFIG_VIRTIO_RNG) += virtio-rng-pci.o
>> diff --git a/hw/virtio/vhost-user-scsi-pci.c 
>> b/hw/virtio/vhost-user-scsi-pci.c
>> new file mode 100644
>> index 00..5baec9c356
>> --- /dev/null
>> +++ b/hw/virtio/vhost-user-scsi-pci.c
>> @@ -0,0 +1,101 @@
>> +/*
>> + * Vhost user scsi PCI Bindings
>> + *
>> + * Copyright (c) 2016 Nutanix Inc. All rights reserved.
>> + *
>> + * Author:
>> + *  Felipe Franciosi 
>> + *
>> + * This work is largely based on the "vhost-scsi" implementation by:
>> + *  Stefan Hajnoczi
>> + *  Nicholas Bellinger 
>> + *
>> + * This work is licensed under the terms of the GNU LGPL, version 2 or 
>> later.
>> + * See the COPYING.LIB file in the top-level directory.
> 
> Not sure whether this should be LGPL (since hw/scsi/vhost-user-scsi.c is
> LGPL, too) or GPLv2+ (since the original code in the file
> hw/virtio/virtio-pci.c is GPLv2+) ... Felipe, any preferences?

I have absolutely no preferences and am perfectly happy with however it is 
decided to take this forward.

Cheers,
Felipe

> 
> NB: "LGPL, version 2" is also inaccurate. According to our COPYING.LIB
> file which is referenced in the comment, we are using version 2.1 ...
> 
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +
>> +#include "standard-headers/linux/virtio_pci.h"
>> +#include "hw/virtio/vhost-user-scsi.h"
>> +#include "hw/virtio/virtio.h"
>> +#include "hw/virtio/virtio-scsi.h"
>> +#include "hw/pci/pci.h"
>> +#include "qapi/error.h"
>> +#include "qemu/error-report.h"
>> +#include "hw/pci/msi.h"
>> +#include "hw/pci/msix.h"
>> +#include "hw/loader.h"
>> +#include "sysemu/kvm.h"
>> +#include "virtio-pci.h"
>> +
>> +typedef struct VHostUserSCSIPCI VHostUserSCSIPCI;
>> +
>> +#define TYPE_VHOST_USER_SCSI_PCI "vhost-user-scsi-pci"
>> +#define VHOST_USER_SCSI_PCI(obj) \
>> +OBJECT_CHECK(VHostUserSCSIPCI, (obj), TYPE_VHOST_USER_SCSI_PCI)
>> +
>> +struct VHostUserSCSIPCI {
>> +VirtIOPCIProxy parent_obj;
>> +VHostUserSCSI vdev;
>> +};
>> +
>> +static Property vhost_user_scsi_pci_properties[] = {
>> +DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors,
>> +   DEV_NVECTORS_UNSPECIFIED),
>> +DEFINE_PROP_END_OF_LIST(),
>> +};
>> +
>> +static void vhost_user_scsi_pci_realize(VirtIOPCIProxy *vpci_dev, Error 
>> **errp)
>> +{
>> +VHostUserSCSIPCI *dev = VHOST_USER_SCSI_PCI(vpci_dev);
>> +DeviceState *vdev = DEVICE(>vdev);
>> +VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(vdev);
>> +
>> +if (vpci_dev->nvectors == DEV_NVECTORS_UNSPECIFIED) {
>> +vpci_dev->nvectors = vs->conf.num_queues + 3;
>> +}
>> +
>> +qdev_set_parent_bus(vdev, BUS(_dev->bus));
>> +object_property_set_bool(OBJECT(vdev), true, "realized", errp);
>> +}
>> +
>> +static void vhost_user_scsi_pci_class_init(ObjectClass *klass, void *data)
>> +{
>> +DeviceClass *dc = DEVICE_CLASS(klass);
>> +VirtioPCIClass *k = VIRTIO_PCI_CLASS(klass);
>> +PCIDeviceClass *pcidev_k = PCI_DEVICE_CLASS(klass);
>> +k->realize = vhost_user_scsi_pci_realize;
>> +set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
>> +dc->props = vhost_user_scsi_pci_properties;
>> +pcidev_k->vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET;
>> +pcidev_k->device_id = PCI_DEVICE_ID_VIRTIO_SCSI;
>> +pcidev_k->revision = 0x00;
>> +pcidev_k->class_id = PCI_CLASS_STORAGE_SCSI;
>> +}
>> +
>> +static void vhost_user_scsi_pci_instance_init(Object *obj)
>> +{
>> +VHostUserSCSIPCI *dev = VHOST_USER_SCSI_PCI(obj);
>> +
>> +virtio_instance_init_common(obj, >vdev, sizeof(dev->vdev),
>> +TYPE_VHOST_USER_SCSI);
>> +object_property_add_alias(obj, "bootindex", OBJECT(>vdev),
>> +  "bootindex", _abort);
>> +}
>> +
>> +static const TypeInfo vhost_user_scsi_pci_info = {
>> +.name  = TYPE_VHOST_USER_SCSI_PCI,
>> +.parent= TYPE_VIRTIO_PCI,
>> +.instance_size = 

Re: [Qemu-devel] [Bug 1806824] Re: SIE-200 (TrustZone) MPC: BLK_MAX returns an incorrect value

[Alex Bennée]

Peter Maydell  writes:

> On Fri, 14 Dec 2018 at 13:56, Alex Bennée  wrote:
>>
>>
>> Peter Maydell  writes:
>>
>> > Thanks for the bug report and the test program. The fix seems 
>> > straightforward -- just adjust what we return for the register value. I've 
>> > sent a patch:
>> > http://patchwork.ozlabs.org/patch/1013034/
>>
>> I know you had a bunch of M-profile test cases. Once we get tcg system
>> tests enabled would it be worth porting some of those into the QEMU src
>> tree?
>
> I don't have anything suitable -- unless we have
> support for "system test of this guest kernel", in which case
> we could add the arm trusted firmware boot/selftests.

That's the next step, enabling the building of a known good test case
from an external tree and depositing the images in the right place so we
can use them as tests.

Things like LTP, kvm-unit-tests and various kernels.

>
> thanks
> -- PMM


--
Alex Bennée

Re: [Qemu-devel] Hosted CI for FreeBSD - Cirrus CI

[Ed Maste]

On Fri, 7 Dec 2018 at 04:54, Daniel P. Berrangé  wrote:
>
> Looking at this more generally I see they support Linux containers,
> native Windows containers, macOS and FreeBSD. IOW, they offer more
> platforms than our current Travis setup does and aren't stuck on an
> amcient Ubuntu version.

Indeed - having a hosted CI service to build on FreeBSD is something
I've wanted for a long time; so much better if one service can support
most or all platforms of interest.

> One key thing I can't find out is what, if any, limitations they put
> on resources used by the free service for OSS projects.  Does anyone
> know if they limit the number of concurrent build jobs like Travis
> does ? Do they put a fixed time limit on execution of a single job ?

I'm not sure what concurrency limits they have. I have empirically
discovered a 1hr limit for a job to complete, but I believe it can be
increased via the config file.

Re: [Qemu-devel] [PATCH v4 16/18] xen: automatically create XenBlockDevice-s

[Paul Durrant]

> -Original Message-
[snip]
> > +
> > +blockdev->auto_iothread = iothread;
> > +
> > +object_property_set_bool(OBJECT(dev), true, "realized",
> _err);
> > +if (local_err) {
> > +error_propagate_prepend(errp, local_err,
> > +"initialization of device %s failed: ",
> > +type);
> > +goto unref;
> > +}
> > +
> > +blockdev_mark_auto_del(blk);
> 
> You don't need this one any more then (if you look into the details,
> it's one of the more confusing parts of the drive_*() magic, so it's
> good to get rid of it). When you use the anonymous BlockBackend created
> by the qdev drive property (because you passed it a node-name rather
> than a BlockBackend name) means that the BlockBackend disappears
> together with the drive.
> 
> Note that explicitly created block nodes must also be unreferenced
> explicitly (bdrv_open() should be paired with bdrv_unref() and
> qmp_blockdev_add() with qmp_blockdev_del()). Maybe XenBackendInfo needs
> a .destroy callback so we can do destruction symmetrically to device
> creation?
> 

I have something implemented using qmp_blockdev_add() now and it seems to work, 
but when I call into qmp_blockdev_del() (passing in the node-name I used to the 
set the "drive" parameter) during unrealize then it tells me that the device is 
in use. Do I need a callback that runs after unrealize of the device?

  Paul 

> > +return;
> > +
> > +unref:
> > +if (dev) {
> > +object_unparent(OBJECT(dev));
> > +}
> > +
> > +if (iothread) {
> > +iothread_destroy(iothread);
> > +}
> > +
> > +if (blk) {
> > +monitor_remove_blk(blk);
> > +blk_unref(blk);
> > +}
> > +}
> 
> Kevin

[Qemu-devel] [PULL 19/22] hw/sd/sdhci: Don't leak memory region in sdhci_sysbus_realize()

[Peter Maydell]

In sdhci_sysbus_realize() we override the initialization of
s->iomem that sdhci_common_realize() performs. However we
don't destroy the old memory region before reinitializing
it, which means that the memory allocated for mr->name in
memory_region_do_init() is leaked.

Since sdhci_initfn() already initializes s->io_ops to
_mmio_ops, always use that in sdhci_common_realize()
and remove the now-unnecessary reinitialization of the
MMIO region from sdhci_sysbus_realize().

Spotted by clang's leak sanitizer.

Signed-off-by: Peter Maydell 
Reviewed-by: Richard Henderson 
Reviewed-by: Philippe Mathieu-Daudé 
Message-id: 20181204132952.2601-4-peter.mayd...@linaro.org
---
 hw/sd/sdhci.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 81bbf032794..83f1574ffdc 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -1371,7 +1371,7 @@ static void sdhci_common_realize(SDHCIState *s, Error 
**errp)
 s->buf_maxsz = sdhci_get_fifolen(s);
 s->fifo_buffer = g_malloc0(s->buf_maxsz);
 
-memory_region_init_io(>iomem, OBJECT(s), _mmio_ops, s, "sdhci",
+memory_region_init_io(>iomem, OBJECT(s), s->io_ops, s, "sdhci",
   SDHC_REGISTERS_MAP_SIZE);
 }
 
@@ -1565,9 +1565,6 @@ static void sdhci_sysbus_realize(DeviceState *dev, Error 
** errp)
 
 sysbus_init_irq(sbd, >irq);
 
-memory_region_init_io(>iomem, OBJECT(s), s->io_ops, s, "sdhci",
-SDHC_REGISTERS_MAP_SIZE);
-
 sysbus_init_mmio(sbd, >iomem);
 }
 
-- 
2.19.2

[Qemu-devel] [PULL 15/22] hw/core/loader.c: Remove load_image()

[Peter Maydell]

The load_image() function is now no longer used anywhere, so
we can remove it completely. (Use load_image_size() or
g_file_get_contents() instead.)

Signed-off-by: Peter Maydell 
Reviewed-by: Richard Henderson 
Reviewed-by: Stefan Hajnoczi 
Reviewed-by: Michael S. Tsirkin 
Reviewed-by: Eric Blake 
Message-id: 20181130151712.2312-10-peter.mayd...@linaro.org
---
 include/hw/loader.h |  1 -
 hw/core/loader.c| 25 -
 2 files changed, 26 deletions(-)

diff --git a/include/hw/loader.h b/include/hw/loader.h
index 67a0af84ac3..3766559bc24 100644
--- a/include/hw/loader.h
+++ b/include/hw/loader.h
@@ -11,7 +11,6 @@
  * On error, errno is also set as appropriate.
  */
 int64_t get_image_size(const char *filename);
-int load_image(const char *filename, uint8_t *addr); /* deprecated */
 ssize_t load_image_size(const char *filename, void *addr, size_t size);
 
 /**load_image_targphys_as:
diff --git a/hw/core/loader.c b/hw/core/loader.c
index 66a616608af..fa41842280a 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -73,31 +73,6 @@ int64_t get_image_size(const char *filename)
 return size;
 }
 
-/* return the size or -1 if error */
-/* deprecated, because caller does not specify buffer size! */
-int load_image(const char *filename, uint8_t *addr)
-{
-int fd, size;
-fd = open(filename, O_RDONLY | O_BINARY);
-if (fd < 0)
-return -1;
-size = lseek(fd, 0, SEEK_END);
-if (size == -1) {
-fprintf(stderr, "file %-20s: get size error: %s\n",
-filename, strerror(errno));
-close(fd);
-return -1;
-}
-
-lseek(fd, 0, SEEK_SET);
-if (read(fd, addr, size) != size) {
-close(fd);
-return -1;
-}
-close(fd);
-return size;
-}
-
 /* return the size or -1 if error */
 ssize_t load_image_size(const char *filename, void *addr, size_t size)
 {
-- 
2.19.2

[Qemu-devel] [PULL 22/22] virt: Fix broken indentation

[Peter Maydell]

From: Eduardo Habkost 

I introduced indentation using tabs instead of spaces in another
commit.  Peter reported the problem, and I failed to fix that
before sending my pull request.

Reported-by: Peter Maydell 
Reviewed-by: Thomas Huth 
Reviewed-by: Philippe Mathieu-Daudé 
Message-id: 20181212003147.29604-1-ehabk...@redhat.com
Fixes: 951597607696 ("virt: Eliminate separate instance_init functions")
Signed-off-by: Eduardo Habkost 
Signed-off-by: Peter Maydell 
---
 hw/arm/virt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 17f1b49d11f..5b678237b7d 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1854,7 +1854,7 @@ static const TypeInfo virt_machine_info = {
 .instance_size = sizeof(VirtMachineState),
 .class_size= sizeof(VirtMachineClass),
 .class_init= virt_machine_class_init,
-   .instance_init = virt_instance_init,
+.instance_init = virt_instance_init,
 .interfaces = (InterfaceInfo[]) {
  { TYPE_HOTPLUG_HANDLER },
  { }
-- 
2.19.2

[Qemu-devel] [PULL 18/22] hw/arm/mps2-tz.c: Free mscname string in make_dma()

[Peter Maydell]

The clang leak sanitizer spots a (one-off, trivial) memory
leak in make_dma() due to a missing free.

Signed-off-by: Peter Maydell 
Reviewed-by: Richard Henderson 
Reviewed-by: Philippe Mathieu-Daudé 
Message-id: 20181204132952.2601-3-peter.mayd...@linaro.org
---
 hw/arm/mps2-tz.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/arm/mps2-tz.c b/hw/arm/mps2-tz.c
index 6dd02ae47e8..82b1d020a58 100644
--- a/hw/arm/mps2-tz.c
+++ b/hw/arm/mps2-tz.c
@@ -322,6 +322,7 @@ static MemoryRegion *make_dma(MPS2TZMachineState *mms, void 
*opaque,
 sysbus_connect_irq(s, 2, qdev_get_gpio_in_named(iotkitdev,
 "EXP_IRQ", 57 + i * 3));
 
+g_free(mscname);
 return sysbus_mmio_get_region(s, 0);
 }
 
-- 
2.19.2

[Qemu-devel] [PULL 16/22] include/hw/loader.h: Document load_image_size()

[Peter Maydell]

Add a documentation comment for load_image_size().

Signed-off-by: Peter Maydell 
Reviewed-by: Richard Henderson 
Reviewed-by: Stefan Hajnoczi 
Reviewed-by: Michael S. Tsirkin 
Reviewed-by: Eric Blake 
Message-id: 20181130151712.2312-11-peter.mayd...@linaro.org
---
 include/hw/loader.h | 16 
 1 file changed, 16 insertions(+)

diff --git a/include/hw/loader.h b/include/hw/loader.h
index 3766559bc24..0a0ad808ea3 100644
--- a/include/hw/loader.h
+++ b/include/hw/loader.h
@@ -11,6 +11,22 @@
  * On error, errno is also set as appropriate.
  */
 int64_t get_image_size(const char *filename);
+/**
+ * load_image_size: load an image file into specified buffer
+ * @filename: Path to the image file
+ * @addr: Buffer to load image into
+ * @size: Size of buffer in bytes
+ *
+ * Load an image file from disk into the specified buffer.
+ * If the image is larger than the specified buffer, only
+ * @size bytes are read (this is not considered an error).
+ *
+ * Prefer to use the GLib function g_file_get_contents() rather
+ * than a "get_image_size()/g_malloc()/load_image_size()" sequence.
+ *
+ * Returns the number of bytes read, or -1 on error. On error,
+ * errno is also set as appropriate.
+ */
 ssize_t load_image_size(const char *filename, void *addr, size_t size);
 
 /**load_image_targphys_as:
-- 
2.19.2

[Qemu-devel] [PULL 11/22] hw/i386/pc.c: Don't use load_image()

[Peter Maydell]

The load_image() function is deprecated, as it does not let the
caller specify how large the buffer to read the file into is.
Use the glib g_file_get_contents() function instead, which does
the whole "allocate memory for the file and read it in" operation.

Signed-off-by: Peter Maydell 
Reviewed-by: Richard Henderson 
Reviewed-by: Stefan Hajnoczi 
Reviewed-by: Michael S. Tsirkin 
Reviewed-by: Eric Blake 
Message-id: 20181130151712.2312-6-peter.mayd...@linaro.org
---
 hw/i386/pc.c | 22 --
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 4cd2fbca4d5..115bc2825ce 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -839,10 +839,9 @@ static void load_linux(PCMachineState *pcms,
 {
 uint16_t protocol;
 int setup_size, kernel_size, cmdline_size;
-int64_t initrd_size = 0;
 int dtb_size, setup_data_offset;
 uint32_t initrd_max;
-uint8_t header[8192], *setup, *kernel, *initrd_data;
+uint8_t header[8192], *setup, *kernel;
 hwaddr real_addr, prot_addr, cmdline_addr, initrd_addr = 0;
 FILE *f;
 char *vmode;
@@ -965,27 +964,30 @@ static void load_linux(PCMachineState *pcms,
 
 /* load initrd */
 if (initrd_filename) {
+gsize initrd_size;
+gchar *initrd_data;
+GError *gerr = NULL;
+
 if (protocol < 0x200) {
 fprintf(stderr, "qemu: linux kernel too old to load a ram disk\n");
 exit(1);
 }
 
-initrd_size = get_image_size(initrd_filename);
-if (initrd_size < 0) {
+if (!g_file_get_contents(initrd_filename, _data,
+ _size, )) {
 fprintf(stderr, "qemu: error reading initrd %s: %s\n",
-initrd_filename, strerror(errno));
+initrd_filename, gerr->message);
 exit(1);
-} else if (initrd_size >= initrd_max) {
+}
+if (initrd_size >= initrd_max) {
 fprintf(stderr, "qemu: initrd is too large, cannot support."
-"(max: %"PRIu32", need %"PRId64")\n", initrd_max, 
initrd_size);
+"(max: %"PRIu32", need %"PRId64")\n",
+initrd_max, (uint64_t)initrd_size);
 exit(1);
 }
 
 initrd_addr = (initrd_max-initrd_size) & ~4095;
 
-initrd_data = g_malloc(initrd_size);
-load_image(initrd_filename, initrd_data);
-
 fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_ADDR, initrd_addr);
 fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_SIZE, initrd_size);
 fw_cfg_add_bytes(fw_cfg, FW_CFG_INITRD_DATA, initrd_data, initrd_size);
-- 
2.19.2

[Qemu-devel] [PULL 12/22] hw/i386/multiboot.c: Don't use load_image()

[Peter Maydell]

The load_image() function is deprecated, as it does not let the
caller specify how large the buffer to read the file into is.
Instead use load_image_size().

While we are converting the code, add the missing error check.

Signed-off-by: Peter Maydell 
Reviewed-by: Richard Henderson 
Reviewed-by: Stefan Hajnoczi 
Reviewed-by: Michael S. Tsirkin 
Reviewed-by: Eric Blake 
Message-id: 20181130151712.2312-7-peter.mayd...@linaro.org
---
 hw/i386/multiboot.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/hw/i386/multiboot.c b/hw/i386/multiboot.c
index 1a4344f5fc3..62340687e8e 100644
--- a/hw/i386/multiboot.c
+++ b/hw/i386/multiboot.c
@@ -343,7 +343,11 @@ int load_multiboot(FWCfgState *fw_cfg,
 mbs.mb_buf_size = TARGET_PAGE_ALIGN(mb_mod_length + 
mbs.mb_buf_size);
 mbs.mb_buf = g_realloc(mbs.mb_buf, mbs.mb_buf_size);
 
-load_image(one_file, (unsigned char *)mbs.mb_buf + offs);
+if (load_image_size(one_file, (unsigned char *)mbs.mb_buf + offs,
+mbs.mb_buf_size - offs) < 0) {
+error_report("Error loading file '%s'", one_file);
+exit(1);
+}
 mb_add_mod(, mbs.mb_buf_phys + offs,
mbs.mb_buf_phys + offs + mb_mod_length, c);
 
-- 
2.19.2

[Qemu-devel] [PULL 17/22] target/arm: Free name string in ARMCPRegInfo hashtable entries

[Peter Maydell]

When we add a new entry to the ARMCPRegInfo hash table in
add_cpreg_to_hashtable(), we allocate memory for tehe
ARMCPRegInfo struct itself, and we also g_strdup() the
name string. So the hashtable's value destructor function
must free the name string as well as the struct.

Spotted by clang's leak sanitizer. The leak here is a
small one-off leak at startup, because we don't support
CPU hotplug, and so the only time when we destroy
hash table entries is for the case where ARM_CP_OVERRIDE
means we register a wildcard entry and then override it later.

Signed-off-by: Peter Maydell 
Reviewed-by: Richard Henderson 
Reviewed-by: Philippe Mathieu-Daudé 
Message-id: 20181204132952.2601-2-peter.mayd...@linaro.org
---
 target/arm/cpu.c | 16 +++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 60411f6bfe0..b84a6c0e678 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -642,6 +642,20 @@ uint64_t arm_cpu_mp_affinity(int idx, uint8_t clustersz)
 return (Aff1 << ARM_AFF1_SHIFT) | Aff0;
 }
 
+static void cpreg_hashtable_data_destroy(gpointer data)
+{
+/*
+ * Destroy function for cpu->cp_regs hashtable data entries.
+ * We must free the name string because it was g_strdup()ed in
+ * add_cpreg_to_hashtable(). It's OK to cast away the 'const'
+ * from r->name because we know we definitely allocated it.
+ */
+ARMCPRegInfo *r = data;
+
+g_free((void *)r->name);
+g_free(r);
+}
+
 static void arm_cpu_initfn(Object *obj)
 {
 CPUState *cs = CPU(obj);
@@ -649,7 +663,7 @@ static void arm_cpu_initfn(Object *obj)
 
 cs->env_ptr = >env;
 cpu->cp_regs = g_hash_table_new_full(g_int_hash, g_int_equal,
- g_free, g_free);
+ g_free, cpreg_hashtable_data_destroy);
 
 QLIST_INIT(>pre_el_change_hooks);
 QLIST_INIT(>el_change_hooks);
-- 
2.19.2

[Qemu-devel] [PULL 07/22] hw/ppc/mac_newworld, mac_oldworld: Don't use load_image()

[Peter Maydell]

The load_image() function is deprecated, as it does not let the
caller specify how large the buffer to read the file into is.
Use the glib g_file_get_contents() function instead, which does
the whole "allocate memory for the file and read it in" operation.

Signed-off-by: Peter Maydell 
Reviewed-by: Richard Henderson 
Reviewed-by: Stefan Hajnoczi 
Reviewed-by: Michael S. Tsirkin 
Reviewed-by: Eric Blake 
Acked-by: David Gibson 
Message-id: 20181130151712.2312-2-peter.mayd...@linaro.org
---
 hw/ppc/mac_newworld.c | 10 --
 hw/ppc/mac_oldworld.c | 10 --
 2 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index 14273a123e5..7e45afae7c5 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -127,8 +127,7 @@ static void ppc_core99_init(MachineState *machine)
 MACIOIDEState *macio_ide;
 BusState *adb_bus;
 MacIONVRAMState *nvr;
-int bios_size, ndrv_size;
-uint8_t *ndrv_file;
+int bios_size;
 int ppc_boot_device;
 DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
 void *fw_cfg;
@@ -510,11 +509,10 @@ static void ppc_core99_init(MachineState *machine)
 /* MacOS NDRV VGA driver */
 filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, NDRV_VGA_FILENAME);
 if (filename) {
-ndrv_size = get_image_size(filename);
-if (ndrv_size != -1) {
-ndrv_file = g_malloc(ndrv_size);
-ndrv_size = load_image(filename, ndrv_file);
+gchar *ndrv_file;
+gsize ndrv_size;
 
+if (g_file_get_contents(filename, _file, _size, NULL)) {
 fw_cfg_add_file(fw_cfg, "ndrv/qemu_vga.ndrv", ndrv_file, 
ndrv_size);
 }
 g_free(filename);
diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index 9891c325a9b..817f70e52cf 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -99,8 +99,7 @@ static void ppc_heathrow_init(MachineState *machine)
 SysBusDevice *s;
 DeviceState *dev, *pic_dev;
 BusState *adb_bus;
-int bios_size, ndrv_size;
-uint8_t *ndrv_file;
+int bios_size;
 uint16_t ppc_boot_device;
 DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
 void *fw_cfg;
@@ -361,11 +360,10 @@ static void ppc_heathrow_init(MachineState *machine)
 /* MacOS NDRV VGA driver */
 filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, NDRV_VGA_FILENAME);
 if (filename) {
-ndrv_size = get_image_size(filename);
-if (ndrv_size != -1) {
-ndrv_file = g_malloc(ndrv_size);
-ndrv_size = load_image(filename, ndrv_file);
+gchar *ndrv_file;
+gsize ndrv_size;
 
+if (g_file_get_contents(filename, _file, _size, NULL)) {
 fw_cfg_add_file(fw_cfg, "ndrv/qemu_vga.ndrv", ndrv_file, 
ndrv_size);
 }
 g_free(filename);
-- 
2.19.2

[Qemu-devel] [PATCH] microbit: make -kernel optional

[Stefan Hajnoczi]

ARMv7M machine types support -kernel for ELF and raw image files.
Microbit programs are typically in Intel HEX (.hex) format.  The generic
loader supports .hex files but it doesn't work as expected:

  $ qemu-system-arm -M microbit -device loader,file=microbit.hex
  Guest image must be specified (using -kernel)

This error comes from armv7m_load_kernel() but we don't have -kernel in
this case.

This patch makes -kernel optional since most of the time we'll want to
use -device loader instead.

Note that we need to register the reset handler that
armv7m_load_kernel() used to register for us.

Signed-off-by: Stefan Hajnoczi 
---
 hw/arm/microbit.c | 17 +++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/hw/arm/microbit.c b/hw/arm/microbit.c
index a734e7f650..638f638792 100644
--- a/hw/arm/microbit.c
+++ b/hw/arm/microbit.c
@@ -14,6 +14,7 @@
 #include "hw/arm/arm.h"
 #include "sysemu/sysemu.h"
 #include "exec/address-spaces.h"
+#include "qemu/error-report.h"
 
 #include "hw/arm/nrf51_soc.h"
 
@@ -28,6 +29,13 @@ typedef struct {
 #define MICROBIT_MACHINE(obj) \
 OBJECT_CHECK(MicrobitMachineState, obj, TYPE_MICROBIT_MACHINE)
 
+static void microbit_cpu_reset(void *opaque)
+{
+ARMCPU *cpu = opaque;
+
+cpu_reset(CPU(cpu));
+}
+
 static void microbit_init(MachineState *machine)
 {
 MicrobitMachineState *s = MICROBIT_MACHINE(machine);
@@ -41,8 +49,13 @@ static void microbit_init(MachineState *machine)
  _fatal);
 object_property_set_bool(soc, true, "realized", _fatal);
 
-armv7m_load_kernel(ARM_CPU(first_cpu), machine->kernel_filename,
-   NRF51_SOC(soc)->flash_size);
+if (machine->kernel_filename) {
+armv7m_load_kernel(ARM_CPU(first_cpu), machine->kernel_filename,
+   NRF51_SOC(soc)->flash_size);
+} else {
+/* armv7m_load_kernel() does this, we need to do it manually here */
+qemu_register_reset(microbit_cpu_reset, ARM_CPU(first_cpu));
+}
 }
 
 static void microbit_machine_class_init(ObjectClass *oc, void *data)
-- 
2.19.2

[Qemu-devel] [PULL 09/22] hw/smbios/smbios.c: Don't use load_image()

[Peter Maydell]

The load_image() function is deprecated, as it does not let the
caller specify how large the buffer to read the file into is.
Instead use load_image_size().

Signed-off-by: Peter Maydell 
Reviewed-by: Richard Henderson 
Reviewed-by: Stefan Hajnoczi 
Reviewed-by: Michael S. Tsirkin 
Reviewed-by: Eric Blake 
Message-id: 20181130151712.2312-4-peter.mayd...@linaro.org
---
 hw/smbios/smbios.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
index 920939454e7..04811279a08 100644
--- a/hw/smbios/smbios.c
+++ b/hw/smbios/smbios.c
@@ -982,7 +982,7 @@ void smbios_entry_add(QemuOpts *opts, Error **errp)
 header = (struct smbios_structure_header *)(smbios_tables +
 smbios_tables_len);
 
-if (load_image(val, (uint8_t *)header) != size) {
+if (load_image_size(val, (uint8_t *)header, size) != size) {
 error_setg(errp, "Failed to load SMBIOS file %s", val);
 return;
 }
-- 
2.19.2

[Qemu-devel] [PULL 14/22] device_tree.c: Don't use load_image()

[Peter Maydell]

The load_image() function is deprecated, as it does not let the
caller specify how large the buffer to read the file into is.
Instead use load_image_size().

Signed-off-by: Peter Maydell 
Reviewed-by: Richard Henderson 
Reviewed-by: Stefan Hajnoczi 
Reviewed-by: Michael S. Tsirkin 
Reviewed-by: Eric Blake 
Message-id: 20181130151712.2312-9-peter.mayd...@linaro.org
---
 device_tree.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/device_tree.c b/device_tree.c
index 6d9c9726f66..296278e12ae 100644
--- a/device_tree.c
+++ b/device_tree.c
@@ -91,7 +91,7 @@ void *load_device_tree(const char *filename_path, int *sizep)
 /* First allocate space in qemu for device tree */
 fdt = g_malloc0(dt_size);
 
-dt_file_load_size = load_image(filename_path, fdt);
+dt_file_load_size = load_image_size(filename_path, fdt, dt_size);
 if (dt_file_load_size < 0) {
 error_report("Unable to open device tree file '%s'",
  filename_path);
-- 
2.19.2

[Qemu-devel] [PULL 05/22] monitor: Use address_space_read() to read memory

[Peter Maydell]

Currently monitor.c reads physical memory using
cpu_physical_memory_read(). This effectively hard-codes
assuming that all CPUs have the same view of physical
memory. Switch to address_space_read() instead, which
lets us use the AddressSpace for the CPU we're
reading memory for (falling back to address_space_memory
if there is no CPU, as happens with the "none" board).
As a bonus, this allows us to detect failures to read memory.

Signed-off-by: Peter Maydell 
Reviewed-by: Dr. David Alan Gilbert 
Reviewed-by: Philippe Mathieu-Daudé 
Message-id: 20181122172653.3413-3-peter.mayd...@linaro.org
---
 monitor.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/monitor.c b/monitor.c
index 6e81b09294e..4c8d8c2a5a0 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1605,7 +1605,13 @@ static void memory_dump(Monitor *mon, int count, int 
format, int wsize,
 if (l > line_size)
 l = line_size;
 if (is_physical) {
-cpu_physical_memory_read(addr, buf, l);
+AddressSpace *as = cs ? cs->as : _space_memory;
+MemTxResult r = address_space_read(as, addr,
+   MEMTXATTRS_UNSPECIFIED, buf, l);
+if (r != MEMTX_OK) {
+monitor_printf(mon, " Cannot access memory\n");
+break;
+}
 } else {
 if (cpu_memory_rw_debug(cs, addr, buf, l, 0) < 0) {
 monitor_printf(mon, " Cannot access memory\n");
-- 
2.19.2

[Qemu-devel] [PULL 20/22] tests/test-arm-mptimer: Don't leak string memory

[Peter Maydell]

The test-arm-mptimer setup creates a lot of test names using
g_strdup_printf() and never frees them. This is entirely
harmless since it's one-shot test code, but it clutters
up the output from clang's LeakSanitizer. Refactor to
use a helper function so we can free the memory.

Signed-off-by: Peter Maydell 
Reviewed-by: Richard Henderson 
Reviewed-by: Philippe Mathieu-Daudé 
Message-id: 20181204132952.2601-5-peter.mayd...@linaro.org
---
 tests/test-arm-mptimer.c | 153 ++-
 1 file changed, 69 insertions(+), 84 deletions(-)

diff --git a/tests/test-arm-mptimer.c b/tests/test-arm-mptimer.c
index cb8f2df9141..156a39f50dd 100644
--- a/tests/test-arm-mptimer.c
+++ b/tests/test-arm-mptimer.c
@@ -991,10 +991,25 @@ static void 
test_timer_zero_load_nonscaled_periodic_to_prescaled_oneshot(void)
 g_assert_cmpuint(timer_get_and_clr_int_sts(), ==, 0);
 }
 
+/*
+ * Add a qtest test that comes in two versions: one with
+ * a timer scaler setting, and one with the timer nonscaled.
+ */
+static void add_scaler_test(const char *str, bool scale,
+void (*fn)(const void *))
+{
+char *name;
+int *scaler = scale ?  : 
+
+name = g_strdup_printf("%s=%d", str, *scaler);
+qtest_add_data_func(name, scaler, fn);
+g_free(name);
+}
+
 int main(int argc, char **argv)
 {
-int *scaler = 
 int ret;
+int scale;
 
 g_test_init(, , NULL);
 
@@ -1012,89 +1027,59 @@ int main(int argc, char **argv)
 qtest_add_func("mptimer/prescaler", test_timer_prescaler);
 qtest_add_func("mptimer/prescaler_on_the_fly", 
test_timer_prescaler_on_the_fly);
 
-tests_with_prescaler_arg:
-qtest_add_data_func(
-g_strdup_printf("mptimer/oneshot scaler=%d", *scaler),
-scaler, test_timer_oneshot);
-qtest_add_data_func(
-g_strdup_printf("mptimer/pause scaler=%d", *scaler),
-scaler, test_timer_pause);
-qtest_add_data_func(
-g_strdup_printf("mptimer/reload scaler=%d", *scaler),
-scaler, test_timer_reload);
-qtest_add_data_func(
-g_strdup_printf("mptimer/periodic scaler=%d", *scaler),
-scaler, test_timer_periodic);
-qtest_add_data_func(
-g_strdup_printf("mptimer/oneshot_to_periodic scaler=%d", *scaler),
-scaler, test_timer_oneshot_to_periodic);
-qtest_add_data_func(
-g_strdup_printf("mptimer/periodic_to_oneshot scaler=%d", *scaler),
-scaler, test_timer_periodic_to_oneshot);
-qtest_add_data_func(
-g_strdup_printf("mptimer/set_oneshot_counter_to_0 scaler=%d", *scaler),
-scaler, test_timer_set_oneshot_counter_to_0);
-qtest_add_data_func(
-g_strdup_printf("mptimer/set_periodic_counter_to_0 scaler=%d", 
*scaler),
-scaler, test_timer_set_periodic_counter_to_0);
-qtest_add_data_func(
-g_strdup_printf("mptimer/noload_oneshot scaler=%d", *scaler),
-scaler, test_timer_noload_oneshot);
-qtest_add_data_func(
-g_strdup_printf("mptimer/noload_periodic scaler=%d", *scaler),
-scaler, test_timer_noload_periodic);
-qtest_add_data_func(
-g_strdup_printf("mptimer/zero_load_oneshot scaler=%d", *scaler),
-scaler, test_timer_zero_load_oneshot);
-qtest_add_data_func(
-g_strdup_printf("mptimer/zero_load_periodic scaler=%d", *scaler),
-scaler, test_timer_zero_load_periodic);
-qtest_add_data_func(
-g_strdup_printf("mptimer/zero_load_oneshot_to_nonzero scaler=%d", 
*scaler),
-scaler, test_timer_zero_load_oneshot_to_nonzero);
-qtest_add_data_func(
-g_strdup_printf("mptimer/zero_load_periodic_to_nonzero scaler=%d", 
*scaler),
-scaler, test_timer_zero_load_periodic_to_nonzero);
-qtest_add_data_func(
-g_strdup_printf("mptimer/nonzero_load_oneshot_to_zero scaler=%d", 
*scaler),
-scaler, test_timer_nonzero_load_oneshot_to_zero);
-qtest_add_data_func(
-g_strdup_printf("mptimer/nonzero_load_periodic_to_zero scaler=%d", 
*scaler),
-scaler, test_timer_nonzero_load_periodic_to_zero);
-qtest_add_data_func(
-g_strdup_printf("mptimer/set_periodic_counter_on_the_fly scaler=%d", 
*scaler),
-scaler, test_timer_set_periodic_counter_on_the_fly);
-qtest_add_data_func(
-g_strdup_printf("mptimer/enable_and_set_counter scaler=%d", *scaler),
-scaler, test_timer_enable_and_set_counter);
-qtest_add_data_func(
-g_strdup_printf("mptimer/set_counter_and_enable scaler=%d", *scaler),
-scaler, test_timer_set_counter_and_enable);
-qtest_add_data_func(
-g_strdup_printf("mptimer/oneshot_with_counter_0_on_start scaler=%d", 
*scaler),
-   

[Qemu-devel] [PULL 08/22] hw/ppc/ppc405_boards: Don't use load_image()

[Peter Maydell]

The load_image() function is deprecated, as it does not let the
caller specify how large the buffer to read the file into is.
Instead use load_image_size().

Signed-off-by: Peter Maydell 
Reviewed-by: Richard Henderson 
Reviewed-by: Stefan Hajnoczi 
Reviewed-by: Michael S. Tsirkin 
Reviewed-by: Eric Blake 
Acked-by: David Gibson 
Message-id: 20181130151712.2312-3-peter.mayd...@linaro.org
---
 hw/ppc/ppc405_boards.c | 12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c
index 3be3fe4432b..1b0a0a8ba3a 100644
--- a/hw/ppc/ppc405_boards.c
+++ b/hw/ppc/ppc405_boards.c
@@ -219,9 +219,11 @@ static void ref405ep_init(MachineState *machine)
 bios_name = BIOS_FILENAME;
 filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
 if (filename) {
-bios_size = load_image(filename, memory_region_get_ram_ptr(bios));
+bios_size = load_image_size(filename,
+memory_region_get_ram_ptr(bios),
+BIOS_SIZE);
 g_free(filename);
-if (bios_size < 0 || bios_size > BIOS_SIZE) {
+if (bios_size < 0) {
 error_report("Could not load PowerPC BIOS '%s'", bios_name);
 exit(1);
 }
@@ -515,9 +517,11 @@ static void taihu_405ep_init(MachineState *machine)
_fatal);
 filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
 if (filename) {
-bios_size = load_image(filename, memory_region_get_ram_ptr(bios));
+bios_size = load_image_size(filename,
+memory_region_get_ram_ptr(bios),
+BIOS_SIZE);
 g_free(filename);
-if (bios_size < 0 || bios_size > BIOS_SIZE) {
+if (bios_size < 0) {
 error_report("Could not load PowerPC BIOS '%s'", bios_name);
 exit(1);
 }
-- 
2.19.2

[Qemu-devel] [PULL 13/22] hw/block/tc58128.c: Don't use load_image()

[Peter Maydell]

The load_image() function is deprecated, as it does not let the
caller specify how large the buffer to read the file into is.
Instead use load_image_size().

Signed-off-by: Peter Maydell 
Reviewed-by: Richard Henderson 
Reviewed-by: Stefan Hajnoczi 
Reviewed-by: Michael S. Tsirkin 
Reviewed-by: Eric Blake 
Message-id: 20181130151712.2312-8-peter.mayd...@linaro.org
---
 hw/block/tc58128.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/block/tc58128.c b/hw/block/tc58128.c
index 808ad76ba60..d0fae248dcc 100644
--- a/hw/block/tc58128.c
+++ b/hw/block/tc58128.c
@@ -38,7 +38,8 @@ static void init_dev(tc58128_dev * dev, const char *filename)
 memset(dev->flash_contents, 0xff, FLASH_SIZE);
 if (filename) {
/* Load flash image skipping the first block */
-   ret = load_image(filename, dev->flash_contents + 528 * 32);
+ret = load_image_size(filename, dev->flash_contents + 528 * 32,
+  FLASH_SIZE - 528 * 32);
if (ret < 0) {
 if (!qtest_enabled()) {
 error_report("Could not load flash image %s", filename);
-- 
2.19.2

[Qemu-devel] [PULL 04/22] disas.c: Use address_space_read() to read memory

[Peter Maydell]

Currently disas.c reads physical memory using
cpu_physical_memory_read(). This effectively hard-codes
assuming that all CPUs have the same view of physical
memory. Switch to address_space_read() instead, which
lets us use the AddressSpace for the CPU we're
disassembling for.

Signed-off-by: Peter Maydell 
Reviewed-by: Philippe Mathieu-Daudé 
Message-id: 20181122172653.3413-2-peter.mayd...@linaro.org
---
 disas.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/disas.c b/disas.c
index 5325b7e6be6..f9c517b3588 100644
--- a/disas.c
+++ b/disas.c
@@ -588,7 +588,10 @@ static int
 physical_read_memory(bfd_vma memaddr, bfd_byte *myaddr, int length,
  struct disassemble_info *info)
 {
-cpu_physical_memory_read(memaddr, myaddr, length);
+CPUDebug *s = container_of(info, CPUDebug, info);
+
+address_space_read(s->cpu->as, memaddr, MEMTXATTRS_UNSPECIFIED,
+   myaddr, length);
 return 0;
 }
 
-- 
2.19.2

[Qemu-devel] [PULL 03/22] Rename cpu_physical_memory_write_rom() to address_space_write_rom()

[Peter Maydell]

The API of cpu_physical_memory_write_rom() is odd, because it
takes an AddressSpace, unlike all the other cpu_physical_memory_*
access functions. Rename it to address_space_write_rom(), and
bring its API into line with address_space_write().

Signed-off-by: Peter Maydell 
Reviewed-by: Philippe Mathieu-Daudé 
Acked-by: Michael S. Tsirkin 
Message-id: 20181122133507.30950-3-peter.mayd...@linaro.org
---
 include/exec/cpu-common.h   |  2 --
 include/exec/memory.h   | 26 ++
 exec.c  | 14 --
 hw/core/loader.c|  4 ++--
 hw/intc/apic.c  |  7 ---
 hw/misc/tz-mpc.c|  2 +-
 hw/sparc/sun4m.c|  5 +++--
 docs/devel/loads-stores.rst | 35 ---
 8 files changed, 60 insertions(+), 35 deletions(-)

diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
index 18b40d6145c..2ad2d6d86bb 100644
--- a/include/exec/cpu-common.h
+++ b/include/exec/cpu-common.h
@@ -111,8 +111,6 @@ bool cpu_physical_memory_is_io(hwaddr phys_addr);
  */
 void qemu_flush_coalesced_mmio_buffer(void);
 
-void cpu_physical_memory_write_rom(AddressSpace *as, hwaddr addr,
-   const uint8_t *buf, int len);
 void cpu_flush_icache_range(hwaddr start, int len);
 
 extern struct MemoryRegion io_mem_rom;
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 8e61450de32..ffd23ed8d8d 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -1792,6 +1792,32 @@ MemTxResult address_space_write(AddressSpace *as, hwaddr 
addr,
 MemTxAttrs attrs,
 const uint8_t *buf, int len);
 
+/**
+ * address_space_write_rom: write to address space, including ROM.
+ *
+ * This function writes to the specified address space, but will
+ * write data to both ROM and RAM. This is used for non-guest
+ * writes like writes from the gdb debug stub or initial loading
+ * of ROM contents.
+ *
+ * Note that portions of the write which attempt to write data to
+ * a device will be silently ignored -- only real RAM and ROM will
+ * be written to.
+ *
+ * Return a MemTxResult indicating whether the operation succeeded
+ * or failed (eg unassigned memory, device rejected the transaction,
+ * IOMMU fault).
+ *
+ * @as: #AddressSpace to be accessed
+ * @addr: address within that address space
+ * @attrs: memory transaction attributes
+ * @buf: buffer with the data transferred
+ * @len: the number of bytes to write
+ */
+MemTxResult address_space_write_rom(AddressSpace *as, hwaddr addr,
+MemTxAttrs attrs,
+const uint8_t *buf, int len);
+
 /* address_space_ld*: load from an address space
  * address_space_st*: store to an address space
  *
diff --git a/exec.c b/exec.c
index 92679508ba3..6e875f0640a 100644
--- a/exec.c
+++ b/exec.c
@@ -3430,11 +3430,12 @@ static inline MemTxResult 
address_space_write_rom_internal(AddressSpace *as,
 }
 
 /* used for ROM loading : can write in RAM and ROM */
-void cpu_physical_memory_write_rom(AddressSpace *as, hwaddr addr,
-   const uint8_t *buf, int len)
+MemTxResult address_space_write_rom(AddressSpace *as, hwaddr addr,
+MemTxAttrs attrs,
+const uint8_t *buf, int len)
 {
-address_space_write_rom_internal(as, addr, MEMTXATTRS_UNSPECIFIED,
- buf, len, WRITE_DATA);
+return address_space_write_rom_internal(as, addr, attrs,
+buf, len, WRITE_DATA);
 }
 
 void cpu_flush_icache_range(hwaddr start, int len)
@@ -3879,8 +3880,9 @@ int cpu_memory_rw_debug(CPUState *cpu, target_ulong addr,
 l = len;
 phys_addr += (addr & ~TARGET_PAGE_MASK);
 if (is_write) {
-cpu_physical_memory_write_rom(cpu->cpu_ases[asidx].as,
-  phys_addr, buf, l);
+address_space_write_rom(cpu->cpu_ases[asidx].as, phys_addr,
+MEMTXATTRS_UNSPECIFIED,
+buf, l);
 } else {
 address_space_rw(cpu->cpu_ases[asidx].as, phys_addr,
  MEMTXATTRS_UNSPECIFIED,
diff --git a/hw/core/loader.c b/hw/core/loader.c
index aa0b3fc8679..66a616608af 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -1103,8 +1103,8 @@ static void rom_reset(void *unused)
 void *host = memory_region_get_ram_ptr(rom->mr);
 memcpy(host, rom->data, rom->datasize);
 } else {
-cpu_physical_memory_write_rom(rom->as, rom->addr, rom->data,
-  rom->datasize);
+address_space_write_rom(rom->as, rom->addr, MEMTXATTRS_UNSPECIFIED,
+rom->data, rom->datasize);
 }
 if (rom->isrom) {

[Qemu-devel] [PULL 10/22] hw/pci/pci.c: Don't use load_image()

[Peter Maydell]

The load_image() function is deprecated, as it does not let the
caller specify how large the buffer to read the file into is.
Instead use load_image_size().

While we are converting this code, add an error-check
for read failure.

Signed-off-by: Peter Maydell 
Reviewed-by: Richard Henderson 
Reviewed-by: Stefan Hajnoczi 
Reviewed-by: Michael S. Tsirkin 
Reviewed-by: Eric Blake 
Message-id: 20181130151712.2312-5-peter.mayd...@linaro.org
---
 hw/pci/pci.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 56b13b3320e..efb5ce196ff 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -2261,7 +2261,11 @@ static void pci_add_option_rom(PCIDevice *pdev, bool 
is_default_rom,
 pdev->has_rom = true;
 memory_region_init_rom(>rom, OBJECT(pdev), name, size, _fatal);
 ptr = memory_region_get_ram_ptr(>rom);
-load_image(path, ptr);
+if (load_image_size(path, ptr, size) < 0) {
+error_setg(errp, "failed to load romfile \"%s\"", pdev->romfile);
+g_free(path);
+return;
+}
 g_free(path);
 
 if (is_default_rom) {
-- 
2.19.2

[Qemu-devel] [PULL 21/22] target/arm: Create timers in realize, not init

[Peter Maydell]

The timer_new() function allocates memory; this means that
if we call it in the CPU's init method we would need
to provide an instance_finalize method to free it. Defer
the timer creation to the realize function instead.

This fixes a memory leak spotted by clang LeakSanitizer
when a CPU object is created for introspection.

Signed-off-by: Peter Maydell 
Reviewed-by: Richard Henderson 
Message-id: 20181204132952.2601-6-peter.mayd...@linaro.org
---
 target/arm/cpu.c | 17 +
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index b84a6c0e678..0e7138c9bfb 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -679,14 +679,6 @@ static void arm_cpu_initfn(Object *obj)
 qdev_init_gpio_in(DEVICE(cpu), arm_cpu_set_irq, 4);
 }
 
-cpu->gt_timer[GTIMER_PHYS] = timer_new(QEMU_CLOCK_VIRTUAL, GTIMER_SCALE,
-arm_gt_ptimer_cb, cpu);
-cpu->gt_timer[GTIMER_VIRT] = timer_new(QEMU_CLOCK_VIRTUAL, GTIMER_SCALE,
-arm_gt_vtimer_cb, cpu);
-cpu->gt_timer[GTIMER_HYP] = timer_new(QEMU_CLOCK_VIRTUAL, GTIMER_SCALE,
-arm_gt_htimer_cb, cpu);
-cpu->gt_timer[GTIMER_SEC] = timer_new(QEMU_CLOCK_VIRTUAL, GTIMER_SCALE,
-arm_gt_stimer_cb, cpu);
 qdev_init_gpio_out(DEVICE(cpu), cpu->gt_timer_outputs,
ARRAY_SIZE(cpu->gt_timer_outputs));
 
@@ -882,6 +874,15 @@ static void arm_cpu_realizefn(DeviceState *dev, Error 
**errp)
 return;
 }
 }
+
+cpu->gt_timer[GTIMER_PHYS] = timer_new(QEMU_CLOCK_VIRTUAL, GTIMER_SCALE,
+   arm_gt_ptimer_cb, cpu);
+cpu->gt_timer[GTIMER_VIRT] = timer_new(QEMU_CLOCK_VIRTUAL, GTIMER_SCALE,
+   arm_gt_vtimer_cb, cpu);
+cpu->gt_timer[GTIMER_HYP] = timer_new(QEMU_CLOCK_VIRTUAL, GTIMER_SCALE,
+  arm_gt_htimer_cb, cpu);
+cpu->gt_timer[GTIMER_SEC] = timer_new(QEMU_CLOCK_VIRTUAL, GTIMER_SCALE,
+  arm_gt_stimer_cb, cpu);
 #endif
 
 cpu_exec_realizefn(cs, _err);
-- 
2.19.2

[Qemu-devel] [PULL 01/22] scripts/checkpatch.pl: Enforce multiline comment syntax

[Peter Maydell]

We now require Linux-kernel-style multiline comments:
/*
 * line one
 * line two
 */

Enforce this in checkpatch.pl, by backporting the relevant
parts of the Linux kernel's checkpatch.pl. (The only changes
needed are that Linux's checkpatch.pl WARN() function takes
an extra argument that ours does not, and the kernel has a
special case for networking code we don't want.)"

The kernel's checkpatch does not enforce "leading /* on
a line of its own, so that part is unique to QEMU's checkpatch.

Sample warning output:

WARNING: Block comments use a leading /* on a separate line
#34: FILE: hw/intc/arm_gicv3_common.c:39:
+/* Older versions of QEMU had a bug in the handling of state save/restore

Signed-off-by: Peter Maydell 
Acked-by: Thomas Huth 
Reviewed-by: Markus Armbruster 
---
 scripts/checkpatch.pl | 48 +++
 1 file changed, 48 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index a892a6cc7c3..18e16b79dfc 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1569,6 +1569,54 @@ sub process {
 # check we are in a valid C source file if not then ignore this hunk
next if ($realfile !~ /\.(h|c|cpp)$/);
 
+# Block comment styles
+
+   # Block comments use /* on a line of its own
+   if ($rawline !~ m@^\+.*/\*.*\*/[ \t]*$@ &&  #inline /*...*/
+   $rawline =~ m@^\+.*/\*\*?[ \t]*.+[ \t]*$@) { # /* or /** 
non-blank
+   WARN("Block comments use a leading /* on a separate 
line\n" . $herecurr);
+   }
+
+# Block comments use * on subsequent lines
+   if ($prevline =~ /$;[ \t]*$/ && #ends in comment
+   $prevrawline =~ /^\+.*?\/\*/ && #starting /*
+   $prevrawline !~ /\*\/[ \t]*$/ &&#no trailing */
+   $rawline =~ /^\+/ &&#line is new
+   $rawline !~ /^\+[ \t]*\*/) {#no leading *
+   WARN("Block comments use * on subsequent lines\n" . 
$hereprev);
+   }
+
+# Block comments use */ on trailing lines
+   if ($rawline !~ m@^\+[ \t]*\*/[ \t]*$@ &&   #trailing */
+   $rawline !~ m@^\+.*/\*.*\*/[ \t]*$@ &&  #inline /*...*/
+   $rawline !~ m@^\+.*\*{2,}/[ \t]*$@ &&   #trailing **/
+   $rawline =~ m@^\+[ \t]*.+\*\/[ \t]*$@) {#non blank */
+   WARN("Block comments use a trailing */ on a separate 
line\n" . $herecurr);
+   }
+
+# Block comment * alignment
+   if ($prevline =~ /$;[ \t]*$/ && #ends in comment
+   $line =~ /^\+[ \t]*$;/ &&   #leading comment
+   $rawline =~ /^\+[ \t]*\*/ &&#leading *
+   (($prevrawline =~ /^\+.*?\/\*/ &&   #leading /*
+ $prevrawline !~ /\*\/[ \t]*$/) || #no trailing */
+$prevrawline =~ /^\+[ \t]*\*/)) {  #leading *
+   my $oldindent;
+   $prevrawline =~ m@^\+([ \t]*/?)\*@;
+   if (defined($1)) {
+   $oldindent = expand_tabs($1);
+   } else {
+   $prevrawline =~ m@^\+(.*/?)\*@;
+   $oldindent = expand_tabs($1);
+   }
+   $rawline =~ m@^\+([ \t]*)\*@;
+   my $newindent = $1;
+   $newindent = expand_tabs($newindent);
+   if (length($oldindent) ne length($newindent)) {
+   WARN("Block comments should align the * on each 
line\n" . $hereprev);
+   }
+   }
+
 # Check for potential 'bare' types
my ($stat, $cond, $line_nr_next, $remain_next, $off_next,
$realline_next);
-- 
2.19.2

[Qemu-devel] [PULL 06/22] elf_ops.h: Use address_space_write() to write memory

[Peter Maydell]

Currently the load_elf function in elf_ops.h uses
cpu_physical_memory_write() to write the ELF file to
memory if it is not handling it as a ROM blob. This
means we ignore the AddressSpace that the function
is passed to define where it should be loaded.
Use address_space_write() instead.

Signed-off-by: Peter Maydell 
Reviewed-by: Philippe Mathieu-Daudé 
Message-id: 20181122172653.3413-4-peter.mayd...@linaro.org
---
 include/hw/elf_ops.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/hw/elf_ops.h b/include/hw/elf_ops.h
index 81cecaf27e2..74679ff8da3 100644
--- a/include/hw/elf_ops.h
+++ b/include/hw/elf_ops.h
@@ -482,7 +482,9 @@ static int glue(load_elf, SZ)(const char *name, int fd,
 rom_add_elf_program(label, data, file_size, mem_size,
 addr, as);
 } else {
-cpu_physical_memory_write(addr, data, file_size);
+address_space_write(as ? as : _space_memory,
+addr, MEMTXATTRS_UNSPECIFIED,
+data, file_size);
 g_free(data);
 }
 }
-- 
2.19.2

[Qemu-devel] [PULL 02/22] exec.c: Rename cpu_physical_memory_write_rom_internal()

[Peter Maydell]

Rename cpu_physical_memory_write_rom_internal() to
address_space_write_rom_internal(), and make it take
MemTxAttrs and return a MemTxResult. This brings its
API into line with address_space_write().

This is an internal function to exec.c; fixing its API
will allow us to change the global function
cpu_physical_memory_write_rom().

Signed-off-by: Peter Maydell 
Reviewed-by: Philippe Mathieu-Daudé 
Acked-by: Michael S. Tsirkin 
Message-id: 20181122133507.30950-2-peter.mayd...@linaro.org
---
 exec.c | 20 +---
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/exec.c b/exec.c
index bb6170dbffe..92679508ba3 100644
--- a/exec.c
+++ b/exec.c
@@ -3388,8 +3388,12 @@ enum write_rom_type {
 FLUSH_CACHE,
 };
 
-static inline void cpu_physical_memory_write_rom_internal(AddressSpace *as,
-hwaddr addr, const uint8_t *buf, int len, enum write_rom_type type)
+static inline MemTxResult address_space_write_rom_internal(AddressSpace *as,
+   hwaddr addr,
+   MemTxAttrs attrs,
+   const uint8_t *buf,
+   int len,
+   enum write_rom_type 
type)
 {
 hwaddr l;
 uint8_t *ptr;
@@ -3399,8 +3403,7 @@ static inline void 
cpu_physical_memory_write_rom_internal(AddressSpace *as,
 rcu_read_lock();
 while (len > 0) {
 l = len;
-mr = address_space_translate(as, addr, , , true,
- MEMTXATTRS_UNSPECIFIED);
+mr = address_space_translate(as, addr, , , true, attrs);
 
 if (!(memory_region_is_ram(mr) ||
   memory_region_is_romd(mr))) {
@@ -3423,13 +3426,15 @@ static inline void 
cpu_physical_memory_write_rom_internal(AddressSpace *as,
 addr += l;
 }
 rcu_read_unlock();
+return MEMTX_OK;
 }
 
 /* used for ROM loading : can write in RAM and ROM */
 void cpu_physical_memory_write_rom(AddressSpace *as, hwaddr addr,
const uint8_t *buf, int len)
 {
-cpu_physical_memory_write_rom_internal(as, addr, buf, len, WRITE_DATA);
+address_space_write_rom_internal(as, addr, MEMTXATTRS_UNSPECIFIED,
+ buf, len, WRITE_DATA);
 }
 
 void cpu_flush_icache_range(hwaddr start, int len)
@@ -3444,8 +3449,9 @@ void cpu_flush_icache_range(hwaddr start, int len)
 return;
 }
 
-cpu_physical_memory_write_rom_internal(_space_memory,
-   start, NULL, len, FLUSH_CACHE);
+address_space_write_rom_internal(_space_memory,
+ start, MEMTXATTRS_UNSPECIFIED,
+ NULL, len, FLUSH_CACHE);
 }
 
 typedef struct {
-- 
2.19.2

[Qemu-devel] [PULL 00/22] misc queue

[Peter Maydell]

This is a colletion of miscellaneous patches (mostly mine),
which fix minor bugs or do some refactoring/cleanup.
No user-visible changes in here.

thanks
-- PMM

The following changes since commit 0f98c9945899c5dfacd5a410ff04178eda605a16:

  Merge remote-tracking branch 
'remotes/huth-gitlab/tags/pull-request-2018-12-12' into staging (2018-12-14 
10:19:47 +)

are available in the Git repository at:

  https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-misc-20181214

for you to fetch changes up to bbac02f1e8edfd0663543f6fdad1e7094d860b29:

  virt: Fix broken indentation (2018-12-14 13:30:55 +)


miscellaneous patches:
 * checkpatch.pl: Enforce multiline comment syntax
 * Rename cpu_physical_memory_write_rom() to address_space_write_rom()
 * disas, monitor, elf_ops: Use address_space_read() to read memory
 * Remove load_image() in favour of load_image_size()
 * Fix some minor memory leaks in arm boards/devices
 * virt: fix broken indentation


Eduardo Habkost (1):
  virt: Fix broken indentation

Peter Maydell (21):
  scripts/checkpatch.pl: Enforce multiline comment syntax
  exec.c: Rename cpu_physical_memory_write_rom_internal()
  Rename cpu_physical_memory_write_rom() to address_space_write_rom()
  disas.c: Use address_space_read() to read memory
  monitor: Use address_space_read() to read memory
  elf_ops.h: Use address_space_write() to write memory
  hw/ppc/mac_newworld, mac_oldworld: Don't use load_image()
  hw/ppc/ppc405_boards: Don't use load_image()
  hw/smbios/smbios.c: Don't use load_image()
  hw/pci/pci.c: Don't use load_image()
  hw/i386/pc.c: Don't use load_image()
  hw/i386/multiboot.c: Don't use load_image()
  hw/block/tc58128.c: Don't use load_image()
  device_tree.c: Don't use load_image()
  hw/core/loader.c: Remove load_image()
  include/hw/loader.h: Document load_image_size()
  target/arm: Free name string in ARMCPRegInfo hashtable entries
  hw/arm/mps2-tz.c: Free mscname string in make_dma()
  hw/sd/sdhci: Don't leak memory region in sdhci_sysbus_realize()
  tests/test-arm-mptimer: Don't leak string memory
  target/arm: Create timers in realize, not init

 include/exec/cpu-common.h   |   2 -
 include/exec/memory.h   |  26 
 include/hw/elf_ops.h|   4 +-
 include/hw/loader.h |  17 -
 device_tree.c   |   2 +-
 disas.c |   5 +-
 exec.c  |  30 +
 hw/arm/mps2-tz.c|   1 +
 hw/arm/virt.c   |   2 +-
 hw/block/tc58128.c  |   3 +-
 hw/core/loader.c|  29 +
 hw/i386/multiboot.c |   6 +-
 hw/i386/pc.c|  22 ---
 hw/intc/apic.c  |   7 +-
 hw/misc/tz-mpc.c|   2 +-
 hw/pci/pci.c|   6 +-
 hw/ppc/mac_newworld.c   |  10 ++-
 hw/ppc/mac_oldworld.c   |  10 ++-
 hw/ppc/ppc405_boards.c  |  12 ++--
 hw/sd/sdhci.c   |   5 +-
 hw/smbios/smbios.c  |   2 +-
 hw/sparc/sun4m.c|   5 +-
 monitor.c   |   8 ++-
 target/arm/cpu.c|  33 +++---
 tests/test-arm-mptimer.c| 153 
 docs/devel/loads-stores.rst |  35 +-
 scripts/checkpatch.pl   |  48 ++
 27 files changed, 287 insertions(+), 198 deletions(-)

Re: [Qemu-devel] [PATCH v2] qdev/core: Can not replug device on bus that allows one device

[Tony Krowiak]

On 12/14/18 7:09 AM, Halil Pasic wrote:

On Thu, 13 Dec 2018 11:26:42 -0500
Tony Krowiak  wrote:


If the maximum number of devices allowed on a bus is 1 and a device
which is plugged into the bus is subsequently unplugged, attempting to replug
the device fails with error "Bus 'xxx' does not support hotplugging".
The "error" is detected in the qbus_is_full(BusState *bus) function
(qdev_monitor.c) because bus->max_index >= bus_class->max_dev. The
root of the problem is that the bus->max_index is not decremented when a device
is unplugged from the bus. This patch fixes that problem.



As Pierre has pointed out, the commit message is stale and inaccurate.
Subject could be better as well. I mean the problem is not limited to
buses that allow only one device. Each bus that can get full looses
capacity with every plug/unplug op pair. With that fixed:

Reviewed-by: Halil Pasic 


I'll make the changes recommended by you and Pierre.




Signed-off-by: Tony Krowiak 
---
  hw/core/qdev.c | 3 +++
  include/hw/qdev-core.h | 1 +
  qdev-monitor.c | 2 +-
  3 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 6b3cc55b27c2..956923f33520 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -59,6 +59,8 @@ static void bus_remove_child(BusState *bus, DeviceState 
*child)
  snprintf(name, sizeof(name), "child[%d]", kid->index);
  QTAILQ_REMOVE(>children, kid, sibling);
  
+bus->num_children--;

+
  /* This gives back ownership of kid->child back to us.  */
  object_property_del(OBJECT(bus), name, NULL);
  object_unref(OBJECT(kid->child));
@@ -73,6 +75,7 @@ static void bus_add_child(BusState *bus, DeviceState *child)
  char name[32];
  BusChild *kid = g_malloc0(sizeof(*kid));
  
+bus->num_children++;

  kid->index = bus->max_index++;
  kid->child = child;
  object_ref(OBJECT(kid->child));
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index a24d0dd566e3..521f0a947ead 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -206,6 +206,7 @@ struct BusState {
  HotplugHandler *hotplug_handler;
  int max_index;
  bool realized;
+int num_children;
  QTAILQ_HEAD(ChildrenHead, BusChild) children;
  QLIST_ENTRY(BusState) sibling;
  };
diff --git a/qdev-monitor.c b/qdev-monitor.c
index 07147c63bf8b..45a8ba49644c 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -414,7 +414,7 @@ static DeviceState *qbus_find_dev(BusState *bus, char *elem)
  static inline bool qbus_is_full(BusState *bus)
  {
  BusClass *bus_class = BUS_GET_CLASS(bus);
-return bus_class->max_dev && bus->max_index >= bus_class->max_dev;
+return bus_class->max_dev && bus->num_children >= bus_class->max_dev;
  }
  
  /*

Re: [Qemu-devel] [PATCH v2] qdev/core: Can not replug device on bus that allows one device

[Tony Krowiak]

On 12/14/18 4:16 AM, Pierre Morel wrote:

On 13/12/2018 17:26, Tony Krowiak wrote:

If the maximum number of devices allowed on a bus is 1 and a device
which is plugged into the bus is subsequently unplugged, attempting to 
replug

the device fails with error "Bus 'xxx' does not support hotplugging".
The "error" is detected in the qbus_is_full(BusState *bus) function
(qdev_monitor.c) because bus->max_index >= bus_class->max_dev. The
root of the problem is that the bus->max_index is not decremented when 
a device

is unplugged from the bus. This patch fixes that problem.


I think that the root of the problem is that the max_index is not the 
right measure to determine the number of devices currently plugged.


With the comment amended in this direction:


Good point, I should have changed that when I refactored this fix.



Reviewed-by: Pierre Morel




Signed-off-by: Tony Krowiak 
---
  hw/core/qdev.c | 3 +++
  include/hw/qdev-core.h | 1 +
  qdev-monitor.c | 2 +-
  3 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 6b3cc55b27c2..956923f33520 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -59,6 +59,8 @@ static void bus_remove_child(BusState *bus, 
DeviceState *child)

  snprintf(name, sizeof(name), "child[%d]", kid->index);
  QTAILQ_REMOVE(>children, kid, sibling);

+    bus->num_children--;
+
  /* This gives back ownership of kid->child back to us.  */
  object_property_del(OBJECT(bus), name, NULL);
  object_unref(OBJECT(kid->child));
@@ -73,6 +75,7 @@ static void bus_add_child(BusState *bus, DeviceState 
*child)

  char name[32];
  BusChild *kid = g_malloc0(sizeof(*kid));

+    bus->num_children++;
  kid->index = bus->max_index++;
  kid->child = child;
  object_ref(OBJECT(kid->child));
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index a24d0dd566e3..521f0a947ead 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -206,6 +206,7 @@ struct BusState {
  HotplugHandler *hotplug_handler;
  int max_index;
  bool realized;
+    int num_children;
  QTAILQ_HEAD(ChildrenHead, BusChild) children;
  QLIST_ENTRY(BusState) sibling;
  };
diff --git a/qdev-monitor.c b/qdev-monitor.c
index 07147c63bf8b..45a8ba49644c 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -414,7 +414,7 @@ static DeviceState *qbus_find_dev(BusState *bus, 
char *elem)

  static inline bool qbus_is_full(BusState *bus)
  {
  BusClass *bus_class = BUS_GET_CLASS(bus);
-    return bus_class->max_dev && bus->max_index >= bus_class->max_dev;
+    return bus_class->max_dev && bus->num_children >= 
bus_class->max_dev;

  }

  /*