Re: [PATCH 0/3] reference implementation of RSS

[Michael S. Tsirkin]

On Fri, Mar 06, 2020 at 11:50:30AM +0200, Yuri Benditovich wrote:
> 
> 
> On Fri, Mar 6, 2020 at 11:27 AM Jason Wang  wrote:
> 
> 
> On 2020/2/27 上午1:48, Yuri Benditovich wrote:
> > Support for VIRTIO_NET_F_RSS feature in QEMU for reference
> > purpose. Implements Toeplitz hash calculation for incoming
> > packets according to configuration provided by driver.
> >
> > This series requires previously submitted and accepted
> > patch to be applied:
> > https://lists.gnu.org/archive/html/qemu-devel/2020-01/msg06448.html
> >
> > Yuri Benditovich (3):
> >    virtio-net: introduce RSS RX steering feature
> >    virtio-net: implement RSS configuration command
> >    virtio-net: implement RX RSS processing
> >
> >   hw/net/trace-events                         |   3 +
> >   hw/net/virtio-net.c                         | 234
> +++-VIRTIO_NET_F_RSS
> >   include/hw/virtio/virtio-net.h              |  12 +
> >   include/standard-headers/linux/virtio_net.h |  37 +++-
> >   4 files changed, 273 insertions(+), 13 deletions(-)
> >
> 
> One question before the reviewing.
> 
> Do we need to deal with migration (which I think yes)?
> 
> 
> I think we don't (yet) as this is a reference implementation and the main goal
> is to provide the functional reference for hardware solution.


That's fine, but then we must block migration, and add appropriate code
comments. Just silently losing data isn't a good idea.

> I agree with the general direction that for complete support of RSS and hash
> delivery the best way is to do that in kernel using bpf.
> For that, IMO, the bpf should be a part of the kernel (it uses skb fields) and
> the tap should receive just RSS parameters for it.
> At this stage we definitely will need to add migration support and propagation
> of RSS parameters.
>  
> 
> 
> Thanks
> 
> 

[PATCH v5 01/11] usb/dev-storage: remove unused include

[Maxim Levitsky]

Signed-off-by: Maxim Levitsky 
Reviewed-by: Philippe Mathieu-Daudé 
---
 hw/usb/dev-storage.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
index 90da008df1..5629213d55 100644
--- a/hw/usb/dev-storage.c
+++ b/hw/usb/dev-storage.c
@@ -19,7 +19,6 @@
 #include "hw/scsi/scsi.h"
 #include "ui/console.h"
 #include "migration/vmstate.h"
-#include "monitor/monitor.h"
 #include "sysemu/sysemu.h"
 #include "sysemu/block-backend.h"
 #include "qapi/visitor.h"
-- 
2.17.2

[PATCH v5 00/11] HMP monitor handlers refactoring

[Maxim Levitsky]

This patch series is bunch of cleanups to the hmp monitor code.
It mostly moves the blockdev related hmp handlers to its own file,
and does some minor refactoring.

No functional changes expected.

Changes from V1:
   * move the handlers to block/monitor/block-hmp-cmds.c
   * tiny cleanup for the commit messages

Changes from V2:
   * Moved all the function prototypes to new header (blockdev-hmp-cmds.h)
   * Set the license of blockdev-hmp-cmds.c to GPLv2+
   * Moved hmp_snapshot_* functions to blockdev-hmp-cmds.c
   * Moved hmp_drive_add_node to blockdev-hmp-cmds.c
 (this change needed some new exports, thus in separate new patch)
   * Moved hmp_qemu_io and hmp_eject to blockdev-hmp-cmds.c
   * Added 'error:' prefix to vreport, and updated the iotests
 This is invasive change, but really feels like the right one
   * Added minor refactoring patch that drops an unused #include

Changes from V3:
   * Dropped the error prefix patches for now due to fact that it seems
 that libvirt doesn't need that after all. Oh well...
 I'll send them in a separate series.

   * Hopefully correctly merged the copyright info the new files
 Both files are GPLv2 now (due to code from hmp.h/hmp-cmds.c)

   * Addressed review feedback
   * Renamed the added header to block-hmp-cmds.h

   * Got rid of checkpatch.pl warnings in the moved code
 (cosmetic code changes only)

   * I kept the reviewed-by tags, since the changes I did are minor.
 I hope that this is right thing to do.

Changes from V4:
   * Rebase with recent changes
   * Fixed review feedback

Best regards,
Maxim Levitsky

Maxim Levitsky (11):
  usb/dev-storage: remove unused include
  monitor/hmp: inline add_init_drive
  monitor/hmp: rename device-hotplug.c to block/monitor/block-hmp-cmds.c
  monitor/hmp: move hmp_drive_del and hmp_commit to block-hmp-cmds.c
  monitor/hmp: move hmp_drive_mirror and hmp_drive_backup to
block-hmp-cmds.c Moved code was added after 2012-01-13, thus under
GPLv2+
  monitor/hmp: move hmp_block_job* to block-hmp-cmds.c
  monitor/hmp: move hmp_snapshot_* to block-hmp-cmds.c
  monitor/hmp: move hmp_nbd_server* to block-hmp-cmds.c
  monitor/hmp: move remaining hmp_block* functions to block-hmp-cmds.c
  monitor/hmp: move hmp_info_block* to block-hmp-cmds.c
  monitor/hmp: Move hmp_drive_add_node to block-hmp-cmds.c

 MAINTAINERS|1 +
 Makefile.objs  |2 +-
 block/Makefile.objs|1 +
 block/monitor/Makefile.objs|1 +
 block/monitor/block-hmp-cmds.c | 1015 
 blockdev.c |  137 +
 device-hotplug.c   |   91 ---
 hw/usb/dev-storage.c   |1 -
 include/block/block-hmp-cmds.h |   54 ++
 include/block/block_int.h  |5 +-
 include/monitor/hmp.h  |   24 -
 include/sysemu/blockdev.h  |4 -
 include/sysemu/sysemu.h|3 -
 monitor/hmp-cmds.c |  782 
 monitor/misc.c |1 +
 15 files changed, 1085 insertions(+), 1037 deletions(-)
 create mode 100644 block/monitor/Makefile.objs
 create mode 100644 block/monitor/block-hmp-cmds.c
 delete mode 100644 device-hotplug.c
 create mode 100644 include/block/block-hmp-cmds.h

-- 
2.17.2

[PATCH v5 02/11] monitor/hmp: inline add_init_drive

[Maxim Levitsky]

This function is only used by hmp_drive_add.
The code is just a bit shorter this way.

No functional changes

Signed-off-by: Maxim Levitsky 
Reviewed-by: Markus Armbruster 
---
 device-hotplug.c | 31 ---
 1 file changed, 12 insertions(+), 19 deletions(-)

diff --git a/device-hotplug.c b/device-hotplug.c
index f01d53774b..554e4d98db 100644
--- a/device-hotplug.c
+++ b/device-hotplug.c
@@ -34,42 +34,35 @@
 #include "monitor/monitor.h"
 #include "block/block_int.h"
 
-static DriveInfo *add_init_drive(const char *optstr)
+
+void hmp_drive_add(Monitor *mon, const QDict *qdict)
 {
 Error *err = NULL;
 DriveInfo *dinfo;
 QemuOpts *opts;
 MachineClass *mc;
+const char *optstr = qdict_get_str(qdict, "opts");
+bool node = qdict_get_try_bool(qdict, "node", false);
+
+if (node) {
+hmp_drive_add_node(mon, optstr);
+return;
+}
 
 opts = drive_def(optstr);
 if (!opts)
-return NULL;
+return;
 
 mc = MACHINE_GET_CLASS(current_machine);
 dinfo = drive_new(opts, mc->block_default_type, &err);
 if (err) {
 error_report_err(err);
 qemu_opts_del(opts);
-return NULL;
-}
-
-return dinfo;
-}
-
-void hmp_drive_add(Monitor *mon, const QDict *qdict)
-{
-DriveInfo *dinfo = NULL;
-const char *opts = qdict_get_str(qdict, "opts");
-bool node = qdict_get_try_bool(qdict, "node", false);
-
-if (node) {
-hmp_drive_add_node(mon, opts);
-return;
+goto err;
 }
 
-dinfo = add_init_drive(opts);
 if (!dinfo) {
-goto err;
+return;
 }
 
 switch (dinfo->type) {
-- 
2.17.2

[PATCH v5 03/11] monitor/hmp: rename device-hotplug.c to block/monitor/block-hmp-cmds.c

[Maxim Levitsky]

These days device-hotplug.c only contains the hmp_drive_add
In the next patch, rest of hmp_drive* functions will be moved
there.

Also add block-hmp-cmds.h to contain prototypes of these
functions

License for block-hmp-cmds.h since it contains the code
moved from sysemu.h which lacks license and thus according
to LICENSE is under GPLv2+


Signed-off-by: Maxim Levitsky 
Reviewed-by: Markus Armbruster 
---
 MAINTAINERS  |  1 +
 Makefile.objs|  2 +-
 block/Makefile.objs  |  1 +
 block/monitor/Makefile.objs  |  1 +
 .../monitor/block-hmp-cmds.c |  3 ++-
 include/block/block-hmp-cmds.h   | 16 
 include/sysemu/sysemu.h  |  3 ---
 monitor/misc.c   |  1 +
 8 files changed, 23 insertions(+), 5 deletions(-)
 create mode 100644 block/monitor/Makefile.objs
 rename device-hotplug.c => block/monitor/block-hmp-cmds.c (97%)
 create mode 100644 include/block/block-hmp-cmds.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 36d0c6887a..d881ba7d9c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1920,6 +1920,7 @@ Block QAPI, monitor, command line
 M: Markus Armbruster 
 S: Supported
 F: blockdev.c
+F: blockdev-hmp-cmds.c
 F: block/qapi.c
 F: qapi/block*.json
 F: qapi/transaction.json
diff --git a/Makefile.objs b/Makefile.objs
index e288663d89..40d3a1696c 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -48,7 +48,7 @@ common-obj-y += dump/
 common-obj-y += job-qmp.o
 common-obj-y += monitor/
 common-obj-y += net/
-common-obj-y += qdev-monitor.o device-hotplug.o
+common-obj-y += qdev-monitor.o
 common-obj-$(CONFIG_WIN32) += os-win32.o
 common-obj-$(CONFIG_POSIX) += os-posix.o
 
diff --git a/block/Makefile.objs b/block/Makefile.objs
index cb36ae2503..3635b6b4c1 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -45,6 +45,7 @@ block-obj-y += crypto.o
 block-obj-y += aio_task.o
 block-obj-y += backup-top.o
 block-obj-y += filter-compress.o
+common-obj-y += monitor/
 
 block-obj-y += stream.o
 
diff --git a/block/monitor/Makefile.objs b/block/monitor/Makefile.objs
new file mode 100644
index 00..0a74f9a8b5
--- /dev/null
+++ b/block/monitor/Makefile.objs
@@ -0,0 +1 @@
+common-obj-y += block-hmp-cmds.o
diff --git a/device-hotplug.c b/block/monitor/block-hmp-cmds.c
similarity index 97%
rename from device-hotplug.c
rename to block/monitor/block-hmp-cmds.c
index 554e4d98db..bcf35b4b44 100644
--- a/device-hotplug.c
+++ b/block/monitor/block-hmp-cmds.c
@@ -1,5 +1,5 @@
 /*
- * QEMU device hotplug helpers
+ * Blockdev HMP commands
  *
  * Copyright (c) 2004 Fabrice Bellard
  *
@@ -33,6 +33,7 @@
 #include "sysemu/sysemu.h"
 #include "monitor/monitor.h"
 #include "block/block_int.h"
+#include "block/block-hmp-cmds.h"
 
 
 void hmp_drive_add(Monitor *mon, const QDict *qdict)
diff --git a/include/block/block-hmp-cmds.h b/include/block/block-hmp-cmds.h
new file mode 100644
index 00..0db8a889a1
--- /dev/null
+++ b/include/block/block-hmp-cmds.h
@@ -0,0 +1,16 @@
+/*
+ * HMP commands related to the block layer
+ *
+ * Copyright (c) 2020 Red Hat, Inc.
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.
+ * or (at your option) any later version.
+ * See the COPYING file in the top-level directory.
+ */
+
+#ifndef BLOCK_HMP_COMMANDS_H
+#define BLOCK_HMP_COMMANDS_H
+
+void hmp_drive_add(Monitor *mon, const QDict *qdict);
+
+#endif
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 479d90bcea..ef81302e1a 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -63,9 +63,6 @@ extern int nb_option_roms;
 extern const char *prom_envs[MAX_PROM_ENVS];
 extern unsigned int nb_prom_envs;
 
-/* generic hotplug */
-void hmp_drive_add(Monitor *mon, const QDict *qdict);
-
 /* pcie aer error injection */
 void hmp_pcie_aer_inject_error(Monitor *mon, const QDict *qdict);
 
diff --git a/monitor/misc.c b/monitor/misc.c
index 1748ab3911..c3bc34c099 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -66,6 +66,7 @@
 #include "qemu/option.h"
 #include "qemu/thread.h"
 #include "block/qapi.h"
+#include "block/block-hmp-cmds.h"
 #include "qapi/qapi-commands-char.h"
 #include "qapi/qapi-commands-control.h"
 #include "qapi/qapi-commands-migration.h"
-- 
2.17.2

[PATCH v5 04/11] monitor/hmp: move hmp_drive_del and hmp_commit to block-hmp-cmds.c

[Maxim Levitsky]

Signed-off-by: Maxim Levitsky 
Reviewed-by: Dr. David Alan Gilbert 
---
 block/monitor/block-hmp-cmds.c | 108 -
 blockdev.c |  96 +
 include/block/block-hmp-cmds.h |   4 ++
 include/sysemu/blockdev.h  |   4 --
 4 files changed, 111 insertions(+), 101 deletions(-)

diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c
index bcf35b4b44..ad727a6b08 100644
--- a/block/monitor/block-hmp-cmds.c
+++ b/block/monitor/block-hmp-cmds.c
@@ -1,7 +1,15 @@
 /*
  * Blockdev HMP commands
  *
- * Copyright (c) 2004 Fabrice Bellard
+ * Copyright (c) 2003-2008 Fabrice Bellard
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * later.  See the COPYING file in the top-level directory.
+ *
+ * This file incorporates work covered by the following copyright and
+ * permission notice:
+ *
+ * Copyright (c) 2003-2008 Fabrice Bellard
  *
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the "Software"), to 
deal
@@ -26,6 +34,7 @@
 #include "hw/boards.h"
 #include "sysemu/block-backend.h"
 #include "sysemu/blockdev.h"
+#include "qapi/qapi-commands-block.h"
 #include "qapi/qmp/qdict.h"
 #include "qapi/error.h"
 #include "qemu/config-file.h"
@@ -35,7 +44,6 @@
 #include "block/block_int.h"
 #include "block/block-hmp-cmds.h"
 
-
 void hmp_drive_add(Monitor *mon, const QDict *qdict)
 {
 Error *err = NULL;
@@ -83,3 +91,99 @@ err:
 blk_unref(blk);
 }
 }
+
+void hmp_drive_del(Monitor *mon, const QDict *qdict)
+{
+const char *id = qdict_get_str(qdict, "id");
+BlockBackend *blk;
+BlockDriverState *bs;
+AioContext *aio_context;
+Error *local_err = NULL;
+
+bs = bdrv_find_node(id);
+if (bs) {
+qmp_blockdev_del(id, &local_err);
+if (local_err) {
+error_report_err(local_err);
+}
+return;
+}
+
+blk = blk_by_name(id);
+if (!blk) {
+error_report("Device '%s' not found", id);
+return;
+}
+
+if (!blk_legacy_dinfo(blk)) {
+error_report("Deleting device added with blockdev-add"
+ " is not supported");
+return;
+}
+
+aio_context = blk_get_aio_context(blk);
+aio_context_acquire(aio_context);
+
+bs = blk_bs(blk);
+if (bs) {
+if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_DRIVE_DEL, &local_err)) {
+error_report_err(local_err);
+aio_context_release(aio_context);
+return;
+}
+
+blk_remove_bs(blk);
+}
+
+/* Make the BlockBackend and the attached BlockDriverState anonymous */
+monitor_remove_blk(blk);
+
+/*
+ * If this BlockBackend has a device attached to it, its refcount will be
+ * decremented when the device is removed; otherwise we have to do so here.
+ */
+if (blk_get_attached_dev(blk)) {
+/* Further I/O must not pause the guest */
+blk_set_on_error(blk, BLOCKDEV_ON_ERROR_REPORT,
+ BLOCKDEV_ON_ERROR_REPORT);
+} else {
+blk_unref(blk);
+}
+
+aio_context_release(aio_context);
+}
+
+void hmp_commit(Monitor *mon, const QDict *qdict)
+{
+const char *device = qdict_get_str(qdict, "device");
+BlockBackend *blk;
+int ret;
+
+if (!strcmp(device, "all")) {
+ret = blk_commit_all();
+} else {
+BlockDriverState *bs;
+AioContext *aio_context;
+
+blk = blk_by_name(device);
+if (!blk) {
+error_report("Device '%s' not found", device);
+return;
+}
+if (!blk_is_available(blk)) {
+error_report("Device '%s' has no medium", device);
+return;
+}
+
+bs = blk_bs(blk);
+aio_context = bdrv_get_aio_context(bs);
+aio_context_acquire(aio_context);
+
+ret = bdrv_commit(bs);
+
+aio_context_release(aio_context);
+}
+if (ret < 0) {
+error_report("'commit' error for '%s': %s", device, strerror(-ret));
+}
+}
diff --git a/blockdev.c b/blockdev.c
index 3e44fa766b..b38c247cdc 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1039,41 +1039,6 @@ static BlockDriverState *qmp_get_root_bs(const char 
*name, Error **errp)
 return bs;
 }
 
-void hmp_commit(Monitor *mon, const QDict *qdict)
-{
-const char *device = qdict_get_str(qdict, "device");
-BlockBackend *blk;
-int ret;
-
-if (!strcmp(device, "all")) {
-ret = blk_commit_all();
-} else {
-BlockDriverState *bs;
-AioContext *aio_context;
-
-blk = blk_by_name(device);
-if (!blk) {
-error_report("Device '%s' not found", device);
-return;
-}
-if (!blk_is_available(blk)) {
-error_report("Device '%s' has no medium", device);
-return;
-}
-
-bs = blk_bs(blk);
-aio_context = bdrv_get_aio_

[PATCH v5 06/11] monitor/hmp: move hmp_block_job* to block-hmp-cmds.c

[Maxim Levitsky]

Signed-off-by: Maxim Levitsky 
Reviewed-by: Dr. David Alan Gilbert 
---
 block/monitor/block-hmp-cmds.c | 52 ++
 include/block/block-hmp-cmds.h |  6 
 include/monitor/hmp.h  |  5 
 monitor/hmp-cmds.c | 52 --
 4 files changed, 58 insertions(+), 57 deletions(-)

diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c
index d6dd5d97f7..8e8288c2f1 100644
--- a/block/monitor/block-hmp-cmds.c
+++ b/block/monitor/block-hmp-cmds.c
@@ -247,3 +247,55 @@ void hmp_drive_backup(Monitor *mon, const QDict *qdict)
 qmp_drive_backup(&backup, &err);
 hmp_handle_error(mon, err);
 }
+
+void hmp_block_job_set_speed(Monitor *mon, const QDict *qdict)
+{
+Error *error = NULL;
+const char *device = qdict_get_str(qdict, "device");
+int64_t value = qdict_get_int(qdict, "speed");
+
+qmp_block_job_set_speed(device, value, &error);
+
+hmp_handle_error(mon, error);
+}
+
+void hmp_block_job_cancel(Monitor *mon, const QDict *qdict)
+{
+Error *error = NULL;
+const char *device = qdict_get_str(qdict, "device");
+bool force = qdict_get_try_bool(qdict, "force", false);
+
+qmp_block_job_cancel(device, true, force, &error);
+
+hmp_handle_error(mon, error);
+}
+
+void hmp_block_job_pause(Monitor *mon, const QDict *qdict)
+{
+Error *error = NULL;
+const char *device = qdict_get_str(qdict, "device");
+
+qmp_block_job_pause(device, &error);
+
+hmp_handle_error(mon, error);
+}
+
+void hmp_block_job_resume(Monitor *mon, const QDict *qdict)
+{
+Error *error = NULL;
+const char *device = qdict_get_str(qdict, "device");
+
+qmp_block_job_resume(device, &error);
+
+hmp_handle_error(mon, error);
+}
+
+void hmp_block_job_complete(Monitor *mon, const QDict *qdict)
+{
+Error *error = NULL;
+const char *device = qdict_get_str(qdict, "device");
+
+qmp_block_job_complete(device, &error);
+
+hmp_handle_error(mon, error);
+}
diff --git a/include/block/block-hmp-cmds.h b/include/block/block-hmp-cmds.h
index a64b737b3a..fcdf1eec48 100644
--- a/include/block/block-hmp-cmds.h
+++ b/include/block/block-hmp-cmds.h
@@ -23,4 +23,10 @@ void hmp_drive_del(Monitor *mon, const QDict *qdict);
 void hmp_drive_mirror(Monitor *mon, const QDict *qdict);
 void hmp_drive_backup(Monitor *mon, const QDict *qdict);
 
+void hmp_block_job_set_speed(Monitor *mon, const QDict *qdict);
+void hmp_block_job_cancel(Monitor *mon, const QDict *qdict);
+void hmp_block_job_pause(Monitor *mon, const QDict *qdict);
+void hmp_block_job_resume(Monitor *mon, const QDict *qdict);
+void hmp_block_job_complete(Monitor *mon, const QDict *qdict);
+
 #endif
diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h
index c1b363ee57..592ce0ccfe 100644
--- a/include/monitor/hmp.h
+++ b/include/monitor/hmp.h
@@ -87,11 +87,6 @@ void hmp_eject(Monitor *mon, const QDict *qdict);
 void hmp_change(Monitor *mon, const QDict *qdict);
 void hmp_block_set_io_throttle(Monitor *mon, const QDict *qdict);
 void hmp_block_stream(Monitor *mon, const QDict *qdict);
-void hmp_block_job_set_speed(Monitor *mon, const QDict *qdict);
-void hmp_block_job_cancel(Monitor *mon, const QDict *qdict);
-void hmp_block_job_pause(Monitor *mon, const QDict *qdict);
-void hmp_block_job_resume(Monitor *mon, const QDict *qdict);
-void hmp_block_job_complete(Monitor *mon, const QDict *qdict);
 void hmp_migrate(Monitor *mon, const QDict *qdict);
 void hmp_device_add(Monitor *mon, const QDict *qdict);
 void hmp_device_del(Monitor *mon, const QDict *qdict);
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 06f0cb4bb9..ac90a9e0c6 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -1997,58 +1997,6 @@ void hmp_block_stream(Monitor *mon, const QDict *qdict)
 hmp_handle_error(mon, error);
 }
 
-void hmp_block_job_set_speed(Monitor *mon, const QDict *qdict)
-{
-Error *error = NULL;
-const char *device = qdict_get_str(qdict, "device");
-int64_t value = qdict_get_int(qdict, "speed");
-
-qmp_block_job_set_speed(device, value, &error);
-
-hmp_handle_error(mon, error);
-}
-
-void hmp_block_job_cancel(Monitor *mon, const QDict *qdict)
-{
-Error *error = NULL;
-const char *device = qdict_get_str(qdict, "device");
-bool force = qdict_get_try_bool(qdict, "force", false);
-
-qmp_block_job_cancel(device, true, force, &error);
-
-hmp_handle_error(mon, error);
-}
-
-void hmp_block_job_pause(Monitor *mon, const QDict *qdict)
-{
-Error *error = NULL;
-const char *device = qdict_get_str(qdict, "device");
-
-qmp_block_job_pause(device, &error);
-
-hmp_handle_error(mon, error);
-}
-
-void hmp_block_job_resume(Monitor *mon, const QDict *qdict)
-{
-Error *error = NULL;
-const char *device = qdict_get_str(qdict, "device");
-
-qmp_block_job_resume(device, &error);
-
-hmp_handle_error(mon, error);
-}
-
-void hmp_block_job_complete(Monitor *mon, const QDict *qdict)
-{
-Error *e

[PATCH v5 08/11] monitor/hmp: move hmp_nbd_server* to block-hmp-cmds.c

[Maxim Levitsky]

Signed-off-by: Maxim Levitsky 
Reviewed-by: Dr. David Alan Gilbert 
---
 block/monitor/block-hmp-cmds.c | 101 +
 include/block/block-hmp-cmds.h |   5 ++
 include/monitor/hmp.h  |   4 --
 monitor/hmp-cmds.c | 100 
 4 files changed, 106 insertions(+), 104 deletions(-)

diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c
index 0131be8ecf..188374abbc 100644
--- a/block/monitor/block-hmp-cmds.c
+++ b/block/monitor/block-hmp-cmds.c
@@ -45,9 +45,11 @@
 #include "qapi/qmp/qerror.h"
 #include "qemu/config-file.h"
 #include "qemu/option.h"
+#include "qemu/sockets.h"
 #include "sysemu/sysemu.h"
 #include "monitor/monitor.h"
 #include "monitor/hmp.h"
+#include "block/nbd.h"
 #include "block/block_int.h"
 #include "block/block-hmp-cmds.h"
 
@@ -353,3 +355,102 @@ void hmp_snapshot_delete_blkdev_internal(Monitor *mon, 
const QDict *qdict)
true, name, &err);
 hmp_handle_error(mon, err);
 }
+
+void hmp_nbd_server_start(Monitor *mon, const QDict *qdict)
+{
+const char *uri = qdict_get_str(qdict, "uri");
+bool writable = qdict_get_try_bool(qdict, "writable", false);
+bool all = qdict_get_try_bool(qdict, "all", false);
+Error *local_err = NULL;
+BlockInfoList *block_list, *info;
+SocketAddress *addr;
+BlockExportNbd export;
+
+if (writable && !all) {
+error_setg(&local_err, "-w only valid together with -a");
+goto exit;
+}
+
+/* First check if the address is valid and start the server.  */
+addr = socket_parse(uri, &local_err);
+if (local_err != NULL) {
+goto exit;
+}
+
+nbd_server_start(addr, NULL, NULL, &local_err);
+qapi_free_SocketAddress(addr);
+if (local_err != NULL) {
+goto exit;
+}
+
+if (!all) {
+return;
+}
+
+/* Then try adding all block devices.  If one fails, close all and
+ * exit.
+ */
+block_list = qmp_query_block(NULL);
+
+for (info = block_list; info; info = info->next) {
+if (!info->value->has_inserted) {
+continue;
+}
+
+export = (BlockExportNbd) {
+.device = info->value->device,
+.has_writable   = true,
+.writable   = writable,
+};
+
+qmp_nbd_server_add(&export, &local_err);
+
+if (local_err != NULL) {
+qmp_nbd_server_stop(NULL);
+break;
+}
+}
+
+qapi_free_BlockInfoList(block_list);
+
+exit:
+hmp_handle_error(mon, local_err);
+}
+
+void hmp_nbd_server_add(Monitor *mon, const QDict *qdict)
+{
+const char *device = qdict_get_str(qdict, "device");
+const char *name = qdict_get_try_str(qdict, "name");
+bool writable = qdict_get_try_bool(qdict, "writable", false);
+Error *local_err = NULL;
+
+BlockExportNbd export = {
+.device = (char *) device,
+.has_name   = !!name,
+.name   = (char *) name,
+.has_writable   = true,
+.writable   = writable,
+};
+
+qmp_nbd_server_add(&export, &local_err);
+hmp_handle_error(mon, local_err);
+}
+
+void hmp_nbd_server_remove(Monitor *mon, const QDict *qdict)
+{
+const char *name = qdict_get_str(qdict, "name");
+bool force = qdict_get_try_bool(qdict, "force", false);
+Error *err = NULL;
+
+/* Rely on NBD_SERVER_REMOVE_MODE_SAFE being the default */
+qmp_nbd_server_remove(name, force, NBD_SERVER_REMOVE_MODE_HARD, &err);
+hmp_handle_error(mon, err);
+}
+
+void hmp_nbd_server_stop(Monitor *mon, const QDict *qdict)
+{
+Error *err = NULL;
+
+qmp_nbd_server_stop(&err);
+hmp_handle_error(mon, err);
+}
diff --git a/include/block/block-hmp-cmds.h b/include/block/block-hmp-cmds.h
index cc81779c7c..50ff802598 100644
--- a/include/block/block-hmp-cmds.h
+++ b/include/block/block-hmp-cmds.h
@@ -33,4 +33,9 @@ void hmp_snapshot_blkdev(Monitor *mon, const QDict *qdict);
 void hmp_snapshot_blkdev_internal(Monitor *mon, const QDict *qdict);
 void hmp_snapshot_delete_blkdev_internal(Monitor *mon, const QDict *qdict);
 
+void hmp_nbd_server_start(Monitor *mon, const QDict *qdict);
+void hmp_nbd_server_add(Monitor *mon, const QDict *qdict);
+void hmp_nbd_server_remove(Monitor *mon, const QDict *qdict);
+void hmp_nbd_server_stop(Monitor *mon, const QDict *qdict);
+
 #endif
diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h
index 6d34e29bb6..736a969131 100644
--- a/include/monitor/hmp.h
+++ b/include/monitor/hmp.h
@@ -94,10 +94,6 @@ void hmp_getfd(Monitor *mon, const QDict *qdict);
 void hmp_closefd(Monitor *mon, const QDict *qdict);
 void hmp_sendkey(Monitor *mon, const QDict *qdict);
 void hmp_screendump(Monitor *mon, const QDict *qdict);
-void hmp_nbd_server_start(Monitor *mon, const QDict *qdict);
-void hmp_nbd_server_add(Monitor *mon, const QDict *qdict);
-void hmp_nbd_server_remove(Monitor *mon, const QDict *qdict);
-vo

[PATCH v5 07/11] monitor/hmp: move hmp_snapshot_* to block-hmp-cmds.c

[Maxim Levitsky]

hmp_snapshot_blkdev is from GPLv2 version of the hmp-cmds.c thus
have to change the licence to GPLv2

Signed-off-by: Maxim Levitsky 
Reviewed-by: Dr. David Alan Gilbert 
---
 block/monitor/block-hmp-cmds.c | 58 --
 include/block/block-hmp-cmds.h |  4 +++
 include/monitor/hmp.h  |  3 --
 monitor/hmp-cmds.c | 47 ---
 4 files changed, 60 insertions(+), 52 deletions(-)

diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c
index 8e8288c2f1..0131be8ecf 100644
--- a/block/monitor/block-hmp-cmds.c
+++ b/block/monitor/block-hmp-cmds.c
@@ -1,10 +1,15 @@
 /*
  * Blockdev HMP commands
  *
+ *  Authors:
+ *  Anthony Liguori   
+ *
  * Copyright (c) 2003-2008 Fabrice Bellard
  *
- * This work is licensed under the terms of the GNU GPL, version 2 or
- * later.  See the COPYING file in the top-level directory.
+ * This work is licensed under the terms of the GNU GPL, version 2.
+ * See the COPYING file in the top-level directory.
+ * Contributions after 2012-01-13 are licensed under the terms of the
+ * GNU GPL, version 2 or (at your option) any later version.
  *
  * This file incorporates work covered by the following copyright and
  * permission notice:
@@ -299,3 +304,52 @@ void hmp_block_job_complete(Monitor *mon, const QDict 
*qdict)
 
 hmp_handle_error(mon, error);
 }
+
+void hmp_snapshot_blkdev(Monitor *mon, const QDict *qdict)
+{
+const char *device = qdict_get_str(qdict, "device");
+const char *filename = qdict_get_try_str(qdict, "snapshot-file");
+const char *format = qdict_get_try_str(qdict, "format");
+bool reuse = qdict_get_try_bool(qdict, "reuse", false);
+enum NewImageMode mode;
+Error *err = NULL;
+
+if (!filename) {
+/*
+ * In the future, if 'snapshot-file' is not specified, the snapshot
+ * will be taken internally. Today it's actually required.
+ */
+error_setg(&err, QERR_MISSING_PARAMETER, "snapshot-file");
+hmp_handle_error(mon, err);
+return;
+}
+
+mode = reuse ? NEW_IMAGE_MODE_EXISTING : NEW_IMAGE_MODE_ABSOLUTE_PATHS;
+qmp_blockdev_snapshot_sync(true, device, false, NULL,
+   filename, false, NULL,
+   !!format, format,
+   true, mode, &err);
+hmp_handle_error(mon, err);
+}
+
+void hmp_snapshot_blkdev_internal(Monitor *mon, const QDict *qdict)
+{
+const char *device = qdict_get_str(qdict, "device");
+const char *name = qdict_get_str(qdict, "name");
+Error *err = NULL;
+
+qmp_blockdev_snapshot_internal_sync(device, name, &err);
+hmp_handle_error(mon, err);
+}
+
+void hmp_snapshot_delete_blkdev_internal(Monitor *mon, const QDict *qdict)
+{
+const char *device = qdict_get_str(qdict, "device");
+const char *name = qdict_get_str(qdict, "name");
+const char *id = qdict_get_try_str(qdict, "id");
+Error *err = NULL;
+
+qmp_blockdev_snapshot_delete_internal_sync(device, !!id, id,
+   true, name, &err);
+hmp_handle_error(mon, err);
+}
diff --git a/include/block/block-hmp-cmds.h b/include/block/block-hmp-cmds.h
index fcdf1eec48..cc81779c7c 100644
--- a/include/block/block-hmp-cmds.h
+++ b/include/block/block-hmp-cmds.h
@@ -29,4 +29,8 @@ void hmp_block_job_pause(Monitor *mon, const QDict *qdict);
 void hmp_block_job_resume(Monitor *mon, const QDict *qdict);
 void hmp_block_job_complete(Monitor *mon, const QDict *qdict);
 
+void hmp_snapshot_blkdev(Monitor *mon, const QDict *qdict);
+void hmp_snapshot_blkdev_internal(Monitor *mon, const QDict *qdict);
+void hmp_snapshot_delete_blkdev_internal(Monitor *mon, const QDict *qdict);
+
 #endif
diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h
index 592ce0ccfe..6d34e29bb6 100644
--- a/include/monitor/hmp.h
+++ b/include/monitor/hmp.h
@@ -61,9 +61,6 @@ void hmp_set_link(Monitor *mon, const QDict *qdict);
 void hmp_block_passwd(Monitor *mon, const QDict *qdict);
 void hmp_balloon(Monitor *mon, const QDict *qdict);
 void hmp_block_resize(Monitor *mon, const QDict *qdict);
-void hmp_snapshot_blkdev(Monitor *mon, const QDict *qdict);
-void hmp_snapshot_blkdev_internal(Monitor *mon, const QDict *qdict);
-void hmp_snapshot_delete_blkdev_internal(Monitor *mon, const QDict *qdict);
 void hmp_loadvm(Monitor *mon, const QDict *qdict);
 void hmp_savevm(Monitor *mon, const QDict *qdict);
 void hmp_delvm(Monitor *mon, const QDict *qdict);
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index ac90a9e0c6..74e6e5b7ef 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -1342,53 +1342,6 @@ void hmp_block_resize(Monitor *mon, const QDict *qdict)
 hmp_handle_error(mon, err);
 }
 
-void hmp_snapshot_blkdev(Monitor *mon, const QDict *qdict)
-{
-const char *device = qdict_get_str(qdict, "device");
-const char *filename = qdict_get_try_str(qdict, "snapshot-file");
-const char *format = qd

[PATCH v5 05/11] monitor/hmp: move hmp_drive_mirror and hmp_drive_backup to block-hmp-cmds.c Moved code was added after 2012-01-13, thus under GPLv2+

[Maxim Levitsky]

Signed-off-by: Maxim Levitsky 
Reviewed-by: Dr. David Alan Gilbert 
---
 block/monitor/block-hmp-cmds.c | 60 ++
 include/block/block-hmp-cmds.h | 12 +--
 include/monitor/hmp.h  |  2 --
 monitor/hmp-cmds.c | 58 
 4 files changed, 69 insertions(+), 63 deletions(-)

diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c
index ad727a6b08..d6dd5d97f7 100644
--- a/block/monitor/block-hmp-cmds.c
+++ b/block/monitor/block-hmp-cmds.c
@@ -37,10 +37,12 @@
 #include "qapi/qapi-commands-block.h"
 #include "qapi/qmp/qdict.h"
 #include "qapi/error.h"
+#include "qapi/qmp/qerror.h"
 #include "qemu/config-file.h"
 #include "qemu/option.h"
 #include "sysemu/sysemu.h"
 #include "monitor/monitor.h"
+#include "monitor/hmp.h"
 #include "block/block_int.h"
 #include "block/block-hmp-cmds.h"
 
@@ -187,3 +189,61 @@ void hmp_commit(Monitor *mon, const QDict *qdict)
 error_report("'commit' error for '%s': %s", device, strerror(-ret));
 }
 }
+
+void hmp_drive_mirror(Monitor *mon, const QDict *qdict)
+{
+const char *filename = qdict_get_str(qdict, "target");
+const char *format = qdict_get_try_str(qdict, "format");
+bool reuse = qdict_get_try_bool(qdict, "reuse", false);
+bool full = qdict_get_try_bool(qdict, "full", false);
+Error *err = NULL;
+DriveMirror mirror = {
+.device = (char *)qdict_get_str(qdict, "device"),
+.target = (char *)filename,
+.has_format = !!format,
+.format = (char *)format,
+.sync = full ? MIRROR_SYNC_MODE_FULL : MIRROR_SYNC_MODE_TOP,
+.has_mode = true,
+.mode = reuse ? NEW_IMAGE_MODE_EXISTING : 
NEW_IMAGE_MODE_ABSOLUTE_PATHS,
+.unmap = true,
+};
+
+if (!filename) {
+error_setg(&err, QERR_MISSING_PARAMETER, "target");
+hmp_handle_error(mon, err);
+return;
+}
+qmp_drive_mirror(&mirror, &err);
+hmp_handle_error(mon, err);
+}
+
+void hmp_drive_backup(Monitor *mon, const QDict *qdict)
+{
+const char *device = qdict_get_str(qdict, "device");
+const char *filename = qdict_get_str(qdict, "target");
+const char *format = qdict_get_try_str(qdict, "format");
+bool reuse = qdict_get_try_bool(qdict, "reuse", false);
+bool full = qdict_get_try_bool(qdict, "full", false);
+bool compress = qdict_get_try_bool(qdict, "compress", false);
+Error *err = NULL;
+DriveBackup backup = {
+.device = (char *)device,
+.target = (char *)filename,
+.has_format = !!format,
+.format = (char *)format,
+.sync = full ? MIRROR_SYNC_MODE_FULL : MIRROR_SYNC_MODE_TOP,
+.has_mode = true,
+.mode = reuse ? NEW_IMAGE_MODE_EXISTING : 
NEW_IMAGE_MODE_ABSOLUTE_PATHS,
+.has_compress = !!compress,
+.compress = compress,
+};
+
+if (!filename) {
+error_setg(&err, QERR_MISSING_PARAMETER, "target");
+hmp_handle_error(mon, err);
+return;
+}
+
+qmp_drive_backup(&backup, &err);
+hmp_handle_error(mon, err);
+}
diff --git a/include/block/block-hmp-cmds.h b/include/block/block-hmp-cmds.h
index 30b0f56415..a64b737b3a 100644
--- a/include/block/block-hmp-cmds.h
+++ b/include/block/block-hmp-cmds.h
@@ -3,10 +3,13 @@
  *
  * Copyright (c) 2003-2008 Fabrice Bellard
  * Copyright (c) 2020 Red Hat, Inc.
+ * Copyright IBM, Corp. 2011
  *
- * This work is licensed under the terms of the GNU GPL, version 2.
- * or (at your option) any later version.
- * See the COPYING file in the top-level directory.
+ * Authors:
+ *  Anthony Liguori   
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
  */
 
 #ifndef BLOCK_HMP_COMMANDS_H
@@ -17,4 +20,7 @@ void hmp_drive_add(Monitor *mon, const QDict *qdict);
 void hmp_commit(Monitor *mon, const QDict *qdict);
 void hmp_drive_del(Monitor *mon, const QDict *qdict);
 
+void hmp_drive_mirror(Monitor *mon, const QDict *qdict);
+void hmp_drive_backup(Monitor *mon, const QDict *qdict);
+
 #endif
diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h
index 3d329853b2..c1b363ee57 100644
--- a/include/monitor/hmp.h
+++ b/include/monitor/hmp.h
@@ -64,8 +64,6 @@ void hmp_block_resize(Monitor *mon, const QDict *qdict);
 void hmp_snapshot_blkdev(Monitor *mon, const QDict *qdict);
 void hmp_snapshot_blkdev_internal(Monitor *mon, const QDict *qdict);
 void hmp_snapshot_delete_blkdev_internal(Monitor *mon, const QDict *qdict);
-void hmp_drive_mirror(Monitor *mon, const QDict *qdict);
-void hmp_drive_backup(Monitor *mon, const QDict *qdict);
 void hmp_loadvm(Monitor *mon, const QDict *qdict);
 void hmp_savevm(Monitor *mon, const QDict *qdict);
 void hmp_delvm(Monitor *mon, const QDict *qdict);
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index fb4c2fd2a8..06f0cb4bb9 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -1342,64 +1342,6 @@ void hmp_block_resize(Monitor *mon,

[PATCH v5 10/11] monitor/hmp: move hmp_info_block* to block-hmp-cmds.c

[Maxim Levitsky]

Signed-off-by: Maxim Levitsky 
Reviewed-by: Dr. David Alan Gilbert 
---
 block/monitor/block-hmp-cmds.c | 389 +
 include/block/block-hmp-cmds.h |   4 +
 include/monitor/hmp.h  |   4 -
 monitor/hmp-cmds.c | 388 
 4 files changed, 393 insertions(+), 392 deletions(-)

diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c
index 5beb7df2f7..aebf1dce0d 100644
--- a/block/monitor/block-hmp-cmds.c
+++ b/block/monitor/block-hmp-cmds.c
@@ -46,10 +46,12 @@
 #include "qemu/config-file.h"
 #include "qemu/option.h"
 #include "qemu/sockets.h"
+#include "qemu/cutils.h"
 #include "sysemu/sysemu.h"
 #include "monitor/monitor.h"
 #include "monitor/hmp.h"
 #include "block/nbd.h"
+#include "block/qapi.h"
 #include "block/block_int.h"
 #include "block/block-hmp-cmds.h"
 #include "qemu-io.h"
@@ -594,3 +596,390 @@ fail:
 blk_unref(local_blk);
 hmp_handle_error(mon, err);
 }
+
+static void print_block_info(Monitor *mon, BlockInfo *info,
+ BlockDeviceInfo *inserted, bool verbose)
+{
+ImageInfo *image_info;
+
+assert(!info || !info->has_inserted || info->inserted == inserted);
+
+if (info && *info->device) {
+monitor_printf(mon, "%s", info->device);
+if (inserted && inserted->has_node_name) {
+monitor_printf(mon, " (%s)", inserted->node_name);
+}
+} else {
+assert(info || inserted);
+monitor_printf(mon, "%s",
+   inserted && inserted->has_node_name ? 
inserted->node_name
+   : info && info->has_qdev ? info->qdev
+   : "");
+}
+
+if (inserted) {
+monitor_printf(mon, ": %s (%s%s%s)\n",
+   inserted->file,
+   inserted->drv,
+   inserted->ro ? ", read-only" : "",
+   inserted->encrypted ? ", encrypted" : "");
+} else {
+monitor_printf(mon, ": [not inserted]\n");
+}
+
+if (info) {
+if (info->has_qdev) {
+monitor_printf(mon, "Attached to:  %s\n", info->qdev);
+}
+if (info->has_io_status && info->io_status != 
BLOCK_DEVICE_IO_STATUS_OK) {
+monitor_printf(mon, "I/O status:   %s\n",
+   BlockDeviceIoStatus_str(info->io_status));
+}
+
+if (info->removable) {
+monitor_printf(mon, "Removable device: %slocked, tray %s\n",
+   info->locked ? "" : "not ",
+   info->tray_open ? "open" : "closed");
+}
+}
+
+
+if (!inserted) {
+return;
+}
+
+monitor_printf(mon, "Cache mode:   %s%s%s\n",
+   inserted->cache->writeback ? "writeback" : "writethrough",
+   inserted->cache->direct ? ", direct" : "",
+   inserted->cache->no_flush ? ", ignore flushes" : "");
+
+if (inserted->has_backing_file) {
+monitor_printf(mon,
+   "Backing file: %s "
+   "(chain depth: %" PRId64 ")\n",
+   inserted->backing_file,
+   inserted->backing_file_depth);
+}
+
+if (inserted->detect_zeroes != BLOCKDEV_DETECT_ZEROES_OPTIONS_OFF) {
+monitor_printf(mon, "Detect zeroes:%s\n",
+BlockdevDetectZeroesOptions_str(inserted->detect_zeroes));
+}
+
+if (inserted->bps  || inserted->bps_rd  || inserted->bps_wr  ||
+inserted->iops || inserted->iops_rd || inserted->iops_wr)
+{
+monitor_printf(mon, "I/O throttling:   bps=%" PRId64
+" bps_rd=%" PRId64  " bps_wr=%" PRId64
+" bps_max=%" PRId64
+" bps_rd_max=%" PRId64
+" bps_wr_max=%" PRId64
+" iops=%" PRId64 " iops_rd=%" PRId64
+" iops_wr=%" PRId64
+" iops_max=%" PRId64
+" iops_rd_max=%" PRId64
+" iops_wr_max=%" PRId64
+" iops_size=%" PRId64
+" group=%s\n",
+inserted->bps,
+inserted->bps_rd,
+inserted->bps_wr,
+inserted->bps_max,
+inserted->bps_rd_max,
+inserted->bps_wr_max,
+inserted->iops,
+inserted->iops_rd,
+inserted->iops_wr,
+inserted->iops_max,
+inserted->iops_rd_max,
+inserted->iops_wr_max,
+inserted->iops_size,
+inserted->group);
+}
+
+if (verbose) {
+monitor_printf(mon, "\nImages:\n");
+image_info = inserted->image;
+w

[PATCH v5 09/11] monitor/hmp: move remaining hmp_block* functions to block-hmp-cmds.c

[Maxim Levitsky]

Signed-off-by: Maxim Levitsky 
Reviewed-by: Dr. David Alan Gilbert 
---
 block/monitor/block-hmp-cmds.c | 140 +
 include/block/block-hmp-cmds.h |   9 +++
 include/monitor/hmp.h  |   6 --
 monitor/hmp-cmds.c | 137 
 4 files changed, 149 insertions(+), 143 deletions(-)

diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c
index 188374abbc..5beb7df2f7 100644
--- a/block/monitor/block-hmp-cmds.c
+++ b/block/monitor/block-hmp-cmds.c
@@ -52,6 +52,7 @@
 #include "block/nbd.h"
 #include "block/block_int.h"
 #include "block/block-hmp-cmds.h"
+#include "qemu-io.h"
 
 void hmp_drive_add(Monitor *mon, const QDict *qdict)
 {
@@ -454,3 +455,142 @@ void hmp_nbd_server_stop(Monitor *mon, const QDict *qdict)
 qmp_nbd_server_stop(&err);
 hmp_handle_error(mon, err);
 }
+
+void hmp_block_resize(Monitor *mon, const QDict *qdict)
+{
+const char *device = qdict_get_str(qdict, "device");
+int64_t size = qdict_get_int(qdict, "size");
+Error *err = NULL;
+
+qmp_block_resize(true, device, false, NULL, size, &err);
+hmp_handle_error(mon, err);
+}
+
+void hmp_block_stream(Monitor *mon, const QDict *qdict)
+{
+Error *error = NULL;
+const char *device = qdict_get_str(qdict, "device");
+const char *base = qdict_get_try_str(qdict, "base");
+int64_t speed = qdict_get_try_int(qdict, "speed", 0);
+
+qmp_block_stream(true, device, device, base != NULL, base, false, NULL,
+ false, NULL, qdict_haskey(qdict, "speed"), speed, true,
+ BLOCKDEV_ON_ERROR_REPORT, false, false, false, false,
+ &error);
+
+hmp_handle_error(mon, error);
+}
+
+void hmp_block_passwd(Monitor *mon, const QDict *qdict)
+{
+const char *device = qdict_get_str(qdict, "device");
+const char *password = qdict_get_str(qdict, "password");
+Error *err = NULL;
+
+qmp_block_passwd(true, device, false, NULL, password, &err);
+hmp_handle_error(mon, err);
+}
+
+void hmp_block_set_io_throttle(Monitor *mon, const QDict *qdict)
+{
+Error *err = NULL;
+char *device = (char *) qdict_get_str(qdict, "device");
+BlockIOThrottle throttle = {
+.bps = qdict_get_int(qdict, "bps"),
+.bps_rd = qdict_get_int(qdict, "bps_rd"),
+.bps_wr = qdict_get_int(qdict, "bps_wr"),
+.iops = qdict_get_int(qdict, "iops"),
+.iops_rd = qdict_get_int(qdict, "iops_rd"),
+.iops_wr = qdict_get_int(qdict, "iops_wr"),
+};
+
+/*
+ * qmp_block_set_io_throttle has separate parameters for the
+ * (deprecated) block device name and the qdev ID but the HMP
+ * version has only one, so we must decide which one to pass.
+ */
+if (blk_by_name(device)) {
+throttle.has_device = true;
+throttle.device = device;
+} else {
+throttle.has_id = true;
+throttle.id = device;
+}
+
+qmp_block_set_io_throttle(&throttle, &err);
+hmp_handle_error(mon, err);
+}
+
+void hmp_eject(Monitor *mon, const QDict *qdict)
+{
+bool force = qdict_get_try_bool(qdict, "force", false);
+const char *device = qdict_get_str(qdict, "device");
+Error *err = NULL;
+
+qmp_eject(true, device, false, NULL, true, force, &err);
+hmp_handle_error(mon, err);
+}
+
+void hmp_qemu_io(Monitor *mon, const QDict *qdict)
+{
+BlockBackend *blk;
+BlockBackend *local_blk = NULL;
+bool qdev = qdict_get_try_bool(qdict, "qdev", false);
+const char *device = qdict_get_str(qdict, "device");
+const char *command = qdict_get_str(qdict, "command");
+Error *err = NULL;
+int ret;
+
+if (qdev) {
+blk = blk_by_qdev_id(device, &err);
+if (!blk) {
+goto fail;
+}
+} else {
+blk = blk_by_name(device);
+if (!blk) {
+BlockDriverState *bs = bdrv_lookup_bs(NULL, device, &err);
+if (bs) {
+blk = local_blk = blk_new(bdrv_get_aio_context(bs),
+  0, BLK_PERM_ALL);
+ret = blk_insert_bs(blk, bs, &err);
+if (ret < 0) {
+goto fail;
+}
+} else {
+goto fail;
+}
+}
+}
+
+/*
+ * Notably absent: Proper permission management. This is sad, but it seems
+ * almost impossible to achieve without changing the semantics and thereby
+ * limiting the use cases of the qemu-io HMP command.
+ *
+ * In an ideal world we would unconditionally create a new BlockBackend for
+ * qemuio_command(), but we have commands like 'reopen' and want them to
+ * take effect on the exact BlockBackend whose name the user passed instead
+ * of just on a temporary copy of it.
+ *
+ * Another problem is that deleting the temporary BlockBackend involves
+ * draining all requests on it first, but some qemu-iotests cases want to
+ * 

[PATCH v5 11/11] monitor/hmp: Move hmp_drive_add_node to block-hmp-cmds.c

[Maxim Levitsky]

Signed-off-by: Maxim Levitsky 
Reviewed-by: Dr. David Alan Gilbert 
---
 block/monitor/block-hmp-cmds.c | 30 
 blockdev.c | 43 +++---
 include/block/block_int.h  |  5 ++--
 3 files changed, 41 insertions(+), 37 deletions(-)

diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c
index aebf1dce0d..c3a6368dfc 100644
--- a/block/monitor/block-hmp-cmds.c
+++ b/block/monitor/block-hmp-cmds.c
@@ -56,6 +56,36 @@
 #include "block/block-hmp-cmds.h"
 #include "qemu-io.h"
 
+static void hmp_drive_add_node(Monitor *mon, const char *optstr)
+{
+QemuOpts *opts;
+QDict *qdict;
+Error *local_err = NULL;
+
+opts = qemu_opts_parse_noisily(&qemu_drive_opts, optstr, false);
+if (!opts) {
+return;
+}
+
+qdict = qemu_opts_to_qdict(opts, NULL);
+
+if (!qdict_get_try_str(qdict, "node-name")) {
+qobject_unref(qdict);
+error_report("'node-name' needs to be specified");
+goto out;
+}
+
+BlockDriverState *bs = bds_tree_init(qdict, &local_err);
+if (!bs) {
+error_report_err(local_err);
+goto out;
+}
+
+bdrv_set_monitor_owned(bs);
+out:
+qemu_opts_del(opts);
+}
+
 void hmp_drive_add(Monitor *mon, const QDict *qdict)
 {
 Error *err = NULL;
diff --git a/blockdev.c b/blockdev.c
index b38c247cdc..257cb37682 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -64,9 +64,14 @@
 #include "qemu/main-loop.h"
 #include "qemu/throttle-options.h"
 
-static QTAILQ_HEAD(, BlockDriverState) monitor_bdrv_states =
+QTAILQ_HEAD(, BlockDriverState) monitor_bdrv_states =
 QTAILQ_HEAD_INITIALIZER(monitor_bdrv_states);
 
+void bdrv_set_monitor_owned(BlockDriverState *bs)
+{
+QTAILQ_INSERT_TAIL(&monitor_bdrv_states, bs, monitor_list);
+}
+
 static const char *const if_name[IF_COUNT] = {
 [IF_NONE] = "none",
 [IF_IDE] = "ide",
@@ -640,7 +645,7 @@ err_no_opts:
 }
 
 /* Takes the ownership of bs_opts */
-static BlockDriverState *bds_tree_init(QDict *bs_opts, Error **errp)
+BlockDriverState *bds_tree_init(QDict *bs_opts, Error **errp)
 {
 int bdrv_flags = 0;
 
@@ -3719,38 +3724,6 @@ out:
 aio_context_release(aio_context);
 }
 
-
-void hmp_drive_add_node(Monitor *mon, const char *optstr)
-{
-QemuOpts *opts;
-QDict *qdict;
-Error *local_err = NULL;
-
-opts = qemu_opts_parse_noisily(&qemu_drive_opts, optstr, false);
-if (!opts) {
-return;
-}
-
-qdict = qemu_opts_to_qdict(opts, NULL);
-
-if (!qdict_get_try_str(qdict, "node-name")) {
-qobject_unref(qdict);
-error_report("'node-name' needs to be specified");
-goto out;
-}
-
-BlockDriverState *bs = bds_tree_init(qdict, &local_err);
-if (!bs) {
-error_report_err(local_err);
-goto out;
-}
-
-QTAILQ_INSERT_TAIL(&monitor_bdrv_states, bs, monitor_list);
-
-out:
-qemu_opts_del(opts);
-}
-
 void qmp_blockdev_add(BlockdevOptions *options, Error **errp)
 {
 BlockDriverState *bs;
@@ -3780,7 +3753,7 @@ void qmp_blockdev_add(BlockdevOptions *options, Error 
**errp)
 goto fail;
 }
 
-QTAILQ_INSERT_TAIL(&monitor_bdrv_states, bs, monitor_list);
+bdrv_set_monitor_owned(bs);
 
 fail:
 visit_free(v);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index f422c0bff0..3f70a98b2d 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -1216,8 +1216,6 @@ BlockJob *backup_job_create(const char *job_id, 
BlockDriverState *bs,
 BlockCompletionFunc *cb, void *opaque,
 JobTxn *txn, Error **errp);
 
-void hmp_drive_add_node(Monitor *mon, const char *optstr);
-
 BdrvChild *bdrv_root_attach_child(BlockDriverState *child_bs,
   const char *child_name,
   const BdrvChildRole *child_role,
@@ -1322,4 +1320,7 @@ int coroutine_fn bdrv_co_copy_range_to(BdrvChild *src, 
uint64_t src_offset,
 
 int refresh_total_sectors(BlockDriverState *bs, int64_t hint);
 
+void bdrv_set_monitor_owned(BlockDriverState *bs);
+BlockDriverState *bds_tree_init(QDict *bs_opts, Error **errp);
+
 #endif /* BLOCK_INT_H */
-- 
2.17.2

Re: [PATCH v5 05/11] monitor/hmp: move hmp_drive_mirror and hmp_drive_backup to block-hmp-cmds.c Moved code was added after 2012-01-13, thus under GPLv2+

[Maxim Levitsky]

I see that I have the same issue of long subject line here.
Its because I forgot the space after first line, when adding this.
If I need to resend another version of this patchset I'll fix this,
but otherwise maybe that can be fixed when applying this to one of maintainer's
trees.

Sorry for noise.

Best regards,
Maxim Levitsky

On Sun, 2020-03-08 at 11:24 +0200, Maxim Levitsky wrote:
> Signed-off-by: Maxim Levitsky 
> Reviewed-by: Dr. David Alan Gilbert 
> ---
>  block/monitor/block-hmp-cmds.c | 60 ++
>  include/block/block-hmp-cmds.h | 12 +--
>  include/monitor/hmp.h  |  2 --
>  monitor/hmp-cmds.c | 58 
>  4 files changed, 69 insertions(+), 63 deletions(-)
> 
> diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c
> index ad727a6b08..d6dd5d97f7 100644
> --- a/block/monitor/block-hmp-cmds.c
> +++ b/block/monitor/block-hmp-cmds.c
> @@ -37,10 +37,12 @@
>  #include "qapi/qapi-commands-block.h"
>  #include "qapi/qmp/qdict.h"
>  #include "qapi/error.h"
> +#include "qapi/qmp/qerror.h"
>  #include "qemu/config-file.h"
>  #include "qemu/option.h"
>  #include "sysemu/sysemu.h"
>  #include "monitor/monitor.h"
> +#include "monitor/hmp.h"
>  #include "block/block_int.h"
>  #include "block/block-hmp-cmds.h"
>  
> @@ -187,3 +189,61 @@ void hmp_commit(Monitor *mon, const QDict *qdict)
>  error_report("'commit' error for '%s': %s", device, strerror(-ret));
>  }
>  }
> +
> +void hmp_drive_mirror(Monitor *mon, const QDict *qdict)
> +{
> +const char *filename = qdict_get_str(qdict, "target");
> +const char *format = qdict_get_try_str(qdict, "format");
> +bool reuse = qdict_get_try_bool(qdict, "reuse", false);
> +bool full = qdict_get_try_bool(qdict, "full", false);
> +Error *err = NULL;
> +DriveMirror mirror = {
> +.device = (char *)qdict_get_str(qdict, "device"),
> +.target = (char *)filename,
> +.has_format = !!format,
> +.format = (char *)format,
> +.sync = full ? MIRROR_SYNC_MODE_FULL : MIRROR_SYNC_MODE_TOP,
> +.has_mode = true,
> +.mode = reuse ? NEW_IMAGE_MODE_EXISTING : 
> NEW_IMAGE_MODE_ABSOLUTE_PATHS,
> +.unmap = true,
> +};
> +
> +if (!filename) {
> +error_setg(&err, QERR_MISSING_PARAMETER, "target");
> +hmp_handle_error(mon, err);
> +return;
> +}
> +qmp_drive_mirror(&mirror, &err);
> +hmp_handle_error(mon, err);
> +}
> +
> +void hmp_drive_backup(Monitor *mon, const QDict *qdict)
> +{
> +const char *device = qdict_get_str(qdict, "device");
> +const char *filename = qdict_get_str(qdict, "target");
> +const char *format = qdict_get_try_str(qdict, "format");
> +bool reuse = qdict_get_try_bool(qdict, "reuse", false);
> +bool full = qdict_get_try_bool(qdict, "full", false);
> +bool compress = qdict_get_try_bool(qdict, "compress", false);
> +Error *err = NULL;
> +DriveBackup backup = {
> +.device = (char *)device,
> +.target = (char *)filename,
> +.has_format = !!format,
> +.format = (char *)format,
> +.sync = full ? MIRROR_SYNC_MODE_FULL : MIRROR_SYNC_MODE_TOP,
> +.has_mode = true,
> +.mode = reuse ? NEW_IMAGE_MODE_EXISTING : 
> NEW_IMAGE_MODE_ABSOLUTE_PATHS,
> +.has_compress = !!compress,
> +.compress = compress,
> +};
> +
> +if (!filename) {
> +error_setg(&err, QERR_MISSING_PARAMETER, "target");
> +hmp_handle_error(mon, err);
> +return;
> +}
> +
> +qmp_drive_backup(&backup, &err);
> +hmp_handle_error(mon, err);
> +}
> diff --git a/include/block/block-hmp-cmds.h b/include/block/block-hmp-cmds.h
> index 30b0f56415..a64b737b3a 100644
> --- a/include/block/block-hmp-cmds.h
> +++ b/include/block/block-hmp-cmds.h
> @@ -3,10 +3,13 @@
>   *
>   * Copyright (c) 2003-2008 Fabrice Bellard
>   * Copyright (c) 2020 Red Hat, Inc.
> + * Copyright IBM, Corp. 2011
>   *
> - * This work is licensed under the terms of the GNU GPL, version 2.
> - * or (at your option) any later version.
> - * See the COPYING file in the top-level directory.
> + * Authors:
> + *  Anthony Liguori   
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2.  See
> + * the COPYING file in the top-level directory.
>   */
>  
>  #ifndef BLOCK_HMP_COMMANDS_H
> @@ -17,4 +20,7 @@ void hmp_drive_add(Monitor *mon, const QDict *qdict);
>  void hmp_commit(Monitor *mon, const QDict *qdict);
>  void hmp_drive_del(Monitor *mon, const QDict *qdict);
>  
> +void hmp_drive_mirror(Monitor *mon, const QDict *qdict);
> +void hmp_drive_backup(Monitor *mon, const QDict *qdict);
> +
>  #endif
> diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h
> index 3d329853b2..c1b363ee57 100644
> --- a/include/monitor/hmp.h
> +++ b/include/monitor/hmp.h
> @@ -64,8 +64,6 @@ void hmp_block_resize(Monitor *mon, const QDict *qdict);
>  void hmp_snapshot_blkdev(Mo

Re: [PATCH] dp8393x: Mask EOL bit from descriptor addresses, take 2

[Laurent Vivier]

Le 04/03/2020 à 04:23, Finn Thain a écrit :
> A portion of a recent patch got lost due to a merge snafu. That patch is
> now commit 88f632fbb1 ("dp8393x: Mask EOL bit from descriptor addresses").
> This patch restores the portion that got lost.
> 
> Signed-off-by: Finn Thain 
> ---
>  hw/net/dp8393x.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
> index 8a3504d962..81fc13ee9f 100644
> --- a/hw/net/dp8393x.c
> +++ b/hw/net/dp8393x.c
> @@ -525,8 +525,8 @@ static void dp8393x_do_transmit_packets(dp8393xState *s)
>   * (4 + 3 * s->regs[SONIC_TFC]),
> MEMTXATTRS_UNSPECIFIED, s->data,
> size);
> -s->regs[SONIC_CTDA] = dp8393x_get(s, width, 0) & ~0x1;
> -if (dp8393x_get(s, width, 0) & SONIC_DESC_EOL) {
> +s->regs[SONIC_CTDA] = dp8393x_get(s, width, 0);
> +if (s->regs[SONIC_CTDA] & SONIC_DESC_EOL) {
>  /* EOL detected */
>  break;
>  }
> 

Jason,

as it's a trivial bug fixes (only a diff between the commit and the
patch), will you merge it via the network queue or do you want I take it
via trivial queue?

Thanks,
Laurent

Re: [PATCH 0/3] reference implementation of RSS

[Yuri Benditovich]

On Sun, Mar 8, 2020 at 10:06 AM Michael S. Tsirkin  wrote:
>
> On Fri, Mar 06, 2020 at 11:50:30AM +0200, Yuri Benditovich wrote:
> >
> >
> > On Fri, Mar 6, 2020 at 11:27 AM Jason Wang  wrote:
> >
> >
> > On 2020/2/27 上午1:48, Yuri Benditovich wrote:
> > > Support for VIRTIO_NET_F_RSS feature in QEMU for reference
> > > purpose. Implements Toeplitz hash calculation for incoming
> > > packets according to configuration provided by driver.
> > >
> > > This series requires previously submitted and accepted
> > > patch to be applied:
> > > https://lists.gnu.org/archive/html/qemu-devel/2020-01/msg06448.html
> > >
> > > Yuri Benditovich (3):
> > >virtio-net: introduce RSS RX steering feature
> > >virtio-net: implement RSS configuration command
> > >virtio-net: implement RX RSS processing
> > >
> > >   hw/net/trace-events |   3 +
> > >   hw/net/virtio-net.c | 234
> > +++-VIRTIO_NET_F_RSS
> > >   include/hw/virtio/virtio-net.h  |  12 +
> > >   include/standard-headers/linux/virtio_net.h |  37 +++-
> > >   4 files changed, 273 insertions(+), 13 deletions(-)
> > >
> >
> > One question before the reviewing.
> >
> > Do we need to deal with migration (which I think yes)?
> >
> >
> > I think we don't (yet) as this is a reference implementation and the main 
> > goal
> > is to provide the functional reference for hardware solution.
>
>
> That's fine, but then we must block migration, and add appropriate code
> comments. Just silently losing data isn't a good idea.

Note that there is no data lost, just the configuration of RSS is not migrating.
So, qemu just will not redirect the data to different queues after migration.
I would add the migration prevention in the next series with
implementation of hash delivery to prevent different packet sizes in
driver and qemu.

>
> > I agree with the general direction that for complete support of RSS and hash
> > delivery the best way is to do that in kernel using bpf.
> > For that, IMO, the bpf should be a part of the kernel (it uses skb fields) 
> > and
> > the tap should receive just RSS parameters for it.
> > At this stage we definitely will need to add migration support and 
> > propagation
> > of RSS parameters.
> >
> >
> >
> > Thanks
> >
> >
>

Re: [PATCH for-5.0] vl.c: fix migration failure for 3.1 and older machine types

[Lukáš Doktor]

Dne 04. 03. 20 v 18:27 Igor Mammedov napsal(a):
> Migration from QEMU(v4.0) fails when using 3.1 or older machine
> type. For example if one attempts to migrate
> QEMU-2.12 started as
>   qemu-system-ppc64 -nodefaults -M pseries-2.12 -m 4096 -mem-path /tmp/
> to current master, it will fail with
>   qemu-system-ppc64: Unknown ramblock "ppc_spapr.ram", cannot accept migration
>   qemu-system-ppc64: error while loading state for instance 0x0 of device 
> 'ram'
>   qemu-system-ppc64: load of migration failed: Invalid argument
> 
> Caused by 900c0ba373 commit which switches main RAM allocation to
> memory backends and the fact in 3.1 and older QEMU, backends used
> full[***] QOM path as memory region name instead of backend's name.
> That was changed after 3.1 to use prefix-less names by default
> (fa0cb34d22) for new machine types.
> *** effectively makes main RAM memory region names defined by
> MachineClass::default_ram_id being altered with '/objects/' prefix
> and therefore migration fails as old QEMU sends prefix-less
> name while new QEMU expects name with prefix when using 3.1 and
> older machine types.
> 
> Fix it by forcing implicit[1] memory backend to always use
> prefix-less names for its memory region by setting
>   'x-use-canonical-path-for-ramblock-id'
> property to false.
> 
> 1) i.e. memory backend created by compat glue which maps
> -m/-mem-path/-mem-prealloc/default RAM size into
> appropriate backend type/options to match old CLI format.
> 
> Fixes: 900c0ba373
> Signed-off-by: Igor Mammedov 
> Reported-by: Lukáš Doktor 
> ---
> CC: ldok...@redhat.com
> CC: marcandre.lur...@redhat.com
> CC: dgilb...@redhat.com
> ---
>  softmmu/vl.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/softmmu/vl.c b/softmmu/vl.c
> index 5549f4b619..1101b1cb41 100644
> --- a/softmmu/vl.c
> +++ b/softmmu/vl.c
> @@ -2800,6 +2800,9 @@ static void create_default_memdev(MachineState *ms, 
> const char *path)
>  object_property_set_int(obj, ms->ram_size, "size", &error_fatal);
>  object_property_add_child(object_get_objects_root(), mc->default_ram_id,
>obj, &error_fatal);
> +/* Ensure backend's memory region name is equal to mc->default_ram_id */
> +object_property_set_bool(obj, false, 
> "x-use-canonical-path-for-ramblock-id",
> + &error_fatal);
>  user_creatable_complete(USER_CREATABLE(obj), &error_fatal);
>  object_unref(obj);
>  object_property_set_str(OBJECT(ms), mc->default_ram_id, "memory-backend",
> 

Tested-by: Lukáš Doktor  

I can't say anything about the code, but functional-wise it addresses the issue 
and fixes post_copy hugepages migration in my mini-CI (variants 2.10, 2.12, 
3.0, 3.1, 4.0 -> master and vice-versa are failing with master and passing with 
this patch).

Regards,
Lukáš



signature.asc
Description: OpenPGP digital signature

Re: [PATCH v4 0/3] delay timer_new from init to realize to fix memleaks.

[Mark Cave-Ayland]

On 05/03/2020 06:54, Pan Nengyuan wrote:

> This series delay timer_new from init into realize to avoid memleaks when we 
> call 'device_list_properties'.
> And do timer_free only in s390x_cpu_finalize because it's hotplugable. 
> However, mos6522_realize is never called 
> at all due to the incorrect creation of it. So we aslo fix the incorrect 
> creation in mac_via first, then move the
> timer_new to mos6522_realize().
> 
> v1:
>- Delay timer_new() from init() to realize() to fix memleaks.
> v2:
>- Similarly to other cleanups, move timer_new into realize in 
> target/s390x/cpu.c (Suggested by Philippe Mathieu-Daudé).
>- Send these two patches as a series instead of send each as a single 
> patch but with wrong subject in v1.
> v3:
>- It's not valid in mos6522 if we move timer_new from init to realize, 
> because it's never called at all.
>  Thus, we remove null check in reset, and add calls to mos6522_realize() 
> in mac_via_realize to make this move to be valid.
>- split patch by device to make it more clear.
> v4:
>- Aslo do timer_free on the error path in realize() and fix some coding 
> style. Then use device_class_set_parent_unrealize to declare unrealize. 
>- split the mos6522 patch into two, one to fix incorrect creation of 
> mos6522, the other to fix memleak.
> 
> Pan Nengyuan (3):
>   s390x: fix memleaks in cpu_finalize
>   mac_via: fix incorrect creation of mos6522 device in mac_via
>   hw/misc/mos6522: move timer_new from init() into realize() to avoid
> memleaks
> 
>  hw/misc/mac_via.c  | 43 +-
>  hw/misc/mos6522.c  |  6 ++
>  target/s390x/cpu-qom.h |  1 +
>  target/s390x/cpu.c | 41 
>  4 files changed, 74 insertions(+), 17 deletions(-)

I just tried this patchset applied on top of git master and it causes 
qemu-system-ppc
to segfault on startup:

$ gdb --args ./qemu-system-ppc
...
...
Thread 1 "qemu-system-ppc" received signal SIGSEGV, Segmentation fault.
0x55e7e38c in timer_del (ts=0x0) at util/qemu-timer.c:429
429 QEMUTimerList *timer_list = ts->timer_list;
(gdb) bt
#0  0x55e7e38c in timer_del (ts=0x0) at util/qemu-timer.c:429
#1  0x55b5d2c1 in mos6522_reset (dev=0x56e0ac50) at 
hw/misc/mos6522.c:468
#2  0x55b63570 in mos6522_cuda_reset (dev=0x56e0ac50) at
hw/misc/macio/cuda.c:599
#3  0x55ad9dd5 in device_transitional_reset (obj=0x56e0ac50) at
hw/core/qdev.c:1136
#4  0x55ae0755 in resettable_phase_hold (obj=0x56e0ac50, opaque=0x0,
type=RESET_TYPE_COLD) at hw/core/resettable.c:182
#5  0x55add5f8 in bus_reset_child_foreach (obj=0x56a472a0,
cb=0x55ae0605 , opaque=0x0, type=RESET_TYPE_COLD) at
hw/core/bus.c:94
#6  0x55ae0418 in resettable_child_foreach (rc=0x5696af80,
obj=0x56a472a0, cb=0x55ae0605 , opaque=0x0,
type=RESET_TYPE_COLD) at hw/core/resettable.c:96
#7  0x55ae06db in resettable_phase_hold (obj=0x56a472a0, opaque=0x0,
type=RESET_TYPE_COLD) at hw/core/resettable.c:173
#8  0x55ae02ab in resettable_assert_reset (obj=0x56a472a0,
type=RESET_TYPE_COLD) at hw/core/resettable.c:60
#9  0x55ae01ef in resettable_reset (obj=0x56a472a0, 
type=RESET_TYPE_COLD)
at hw/core/resettable.c:45
#10 0x55ae0afa in resettable_cold_reset_fn (opaque=0x56a472a0) at
hw/core/resettable.c:269
#11 0x55ae13a0 in qemu_devices_reset () at hw/core/reset.c:69
#12 0x5597d54c in qemu_system_reset (reason=SHUTDOWN_CAUSE_NONE) at
/home/build/src/qemu/git/qemu/softmmu/vl.c:1393
#13 0x559855bb in qemu_init (argc=1, argv=0x7fffea78,
envp=0x7fffea88) at /home/build/src/qemu/git/qemu/softmmu/vl.c:4418
#14 0x55e1b646 in main (argc=1, argv=0x7fffea78, 
envp=0x7fffea88) at
/home/build/src/qemu/git/qemu/softmmu/main.c:48


Possibly related to some of the new reset changes?


ATB,

Mark.

Re: [PATCH 0/3] reference implementation of RSS

[Michael S. Tsirkin]

On Sun, Mar 08, 2020 at 11:56:38AM +0200, Yuri Benditovich wrote:
> On Sun, Mar 8, 2020 at 10:06 AM Michael S. Tsirkin  wrote:
> >
> > On Fri, Mar 06, 2020 at 11:50:30AM +0200, Yuri Benditovich wrote:
> > >
> > >
> > > On Fri, Mar 6, 2020 at 11:27 AM Jason Wang  wrote:
> > >
> > >
> > > On 2020/2/27 上午1:48, Yuri Benditovich wrote:
> > > > Support for VIRTIO_NET_F_RSS feature in QEMU for reference
> > > > purpose. Implements Toeplitz hash calculation for incoming
> > > > packets according to configuration provided by driver.
> > > >
> > > > This series requires previously submitted and accepted
> > > > patch to be applied:
> > > > https://lists.gnu.org/archive/html/qemu-devel/2020-01/msg06448.html
> > > >
> > > > Yuri Benditovich (3):
> > > >virtio-net: introduce RSS RX steering feature
> > > >virtio-net: implement RSS configuration command
> > > >virtio-net: implement RX RSS processing
> > > >
> > > >   hw/net/trace-events |   3 +
> > > >   hw/net/virtio-net.c | 234
> > > +++-VIRTIO_NET_F_RSS
> > > >   include/hw/virtio/virtio-net.h  |  12 +
> > > >   include/standard-headers/linux/virtio_net.h |  37 +++-
> > > >   4 files changed, 273 insertions(+), 13 deletions(-)
> > > >
> > >
> > > One question before the reviewing.
> > >
> > > Do we need to deal with migration (which I think yes)?
> > >
> > >
> > > I think we don't (yet) as this is a reference implementation and the main 
> > > goal
> > > is to provide the functional reference for hardware solution.
> >
> >
> > That's fine, but then we must block migration, and add appropriate code
> > comments. Just silently losing data isn't a good idea.
> 
> Note that there is no data lost, just the configuration of RSS is not 
> migrating.
> So, qemu just will not redirect the data to different queues after migration.

Right. Unlike auto-mq, the spec doesn't say the direction is best effort though,
so that would be a spec violation.

> I would add the migration prevention in the next series with
> implementation of hash delivery to prevent different packet sizes in
> driver and qemu.

And hopefully full migration support will follow.

One other thing to check is vhost - I didn't check
what happens with this patchset but
I think at a minimum we need to fail vhost init,
until the backend implements the feature biit.


> >
> > > I agree with the general direction that for complete support of RSS and 
> > > hash
> > > delivery the best way is to do that in kernel using bpf.
> > > For that, IMO, the bpf should be a part of the kernel (it uses skb 
> > > fields) and
> > > the tap should receive just RSS parameters for it.
> > > At this stage we definitely will need to add migration support and 
> > > propagation
> > > of RSS parameters.
> > >
> > >
> > >
> > > Thanks
> > >
> > >
> >

Re: [PATCH 0/3] reference implementation of RSS

[Yuri Benditovich]

On Sun, Mar 8, 2020 at 2:17 PM Michael S. Tsirkin  wrote:
>
> On Sun, Mar 08, 2020 at 11:56:38AM +0200, Yuri Benditovich wrote:
> > On Sun, Mar 8, 2020 at 10:06 AM Michael S. Tsirkin  wrote:
> > >
> > > On Fri, Mar 06, 2020 at 11:50:30AM +0200, Yuri Benditovich wrote:
> > > >
> > > >
> > > > On Fri, Mar 6, 2020 at 11:27 AM Jason Wang  wrote:
> > > >
> > > >
> > > > On 2020/2/27 上午1:48, Yuri Benditovich wrote:
> > > > > Support for VIRTIO_NET_F_RSS feature in QEMU for reference
> > > > > purpose. Implements Toeplitz hash calculation for incoming
> > > > > packets according to configuration provided by driver.
> > > > >
> > > > > This series requires previously submitted and accepted
> > > > > patch to be applied:
> > > > > 
> > > > https://lists.gnu.org/archive/html/qemu-devel/2020-01/msg06448.html
> > > > >
> > > > > Yuri Benditovich (3):
> > > > >virtio-net: introduce RSS RX steering feature
> > > > >virtio-net: implement RSS configuration command
> > > > >virtio-net: implement RX RSS processing
> > > > >
> > > > >   hw/net/trace-events |   3 +
> > > > >   hw/net/virtio-net.c | 234
> > > > +++-VIRTIO_NET_F_RSS
> > > > >   include/hw/virtio/virtio-net.h  |  12 +
> > > > >   include/standard-headers/linux/virtio_net.h |  37 +++-
> > > > >   4 files changed, 273 insertions(+), 13 deletions(-)
> > > > >
> > > >
> > > > One question before the reviewing.
> > > >
> > > > Do we need to deal with migration (which I think yes)?
> > > >
> > > >
> > > > I think we don't (yet) as this is a reference implementation and the 
> > > > main goal
> > > > is to provide the functional reference for hardware solution.
> > >
> > >
> > > That's fine, but then we must block migration, and add appropriate code
> > > comments. Just silently losing data isn't a good idea.
> >
> > Note that there is no data lost, just the configuration of RSS is not 
> > migrating.
> > So, qemu just will not redirect the data to different queues after 
> > migration.
>
> Right. Unlike auto-mq, the spec doesn't say the direction is best effort 
> though,
> so that would be a spec violation.
>
> > I would add the migration prevention in the next series with
> > implementation of hash delivery to prevent different packet sizes in
> > driver and qemu.
>
> And hopefully full migration support will follow.
>
> One other thing to check is vhost - I didn't check
> what happens with this patchset but
> I think at a minimum we need to fail vhost init,
> until the backend implements the feature biit.

RSS feature currently is not indicated in case vhost is on.
The same will be with hash report.

>
>
> > >
> > > > I agree with the general direction that for complete support of RSS and 
> > > > hash
> > > > delivery the best way is to do that in kernel using bpf.
> > > > For that, IMO, the bpf should be a part of the kernel (it uses skb 
> > > > fields) and
> > > > the tap should receive just RSS parameters for it.
> > > > At this stage we definitely will need to add migration support and 
> > > > propagation
> > > > of RSS parameters.
> > > >
> > > >
> > > >
> > > > Thanks
> > > >
> > > >
> > >
>

[PATCH] docs: Add RX target.

[Yoshinori Sato]

Add rx-virt target specificaion document.

Signed-off-by: Yoshinori Sato 
---
 docs/system/target-rx.rst | 35 +++
 docs/system/targets.rst   |  1 +
 2 files changed, 36 insertions(+)
 create mode 100644 docs/system/target-rx.rst

diff --git a/docs/system/target-rx.rst b/docs/system/target-rx.rst
new file mode 100644
index 00..8540fd5218
--- /dev/null
+++ b/docs/system/target-rx.rst
@@ -0,0 +1,35 @@
+.. _RX-System-emulator:
+
+RX System emulator
+
+
+Use the executable ``qemu-system-rx`` to simulate a Virtual RX target.
+This target emulated following devices.
+
+-  R5F562N8 MCU
+
+   -  On-chip memory (ROM 512KB, RAM 96KB)
+   -  Interrupt Control Unit (ICUa)
+   -  8Bit Timer x 1CH (TMR0,1)
+   -  Compare Match Timer x 2CH (CMT0,1)
+   -  Serial Communication Interface x 1CH (SCI0)
+
+-  External memory 16MByte
+
+Example of ``qemu-system-rx`` usage for RX is shown below:
+
+Download  from
+https://osdn.net/users/ysato/pf/qemu/dl/u-boot.bin.gz
+
+Start emulation of rx-virt::
+  qemu-system-rx -bios 
+
+Download ``kernel_image_file`` from
+https://osdn.net/users/ysato/pf/qemu/dl/zImage
+
+Download ``device_tree_blob`` from
+https://osdn.net/users/ysato/pf/qemu/dl/rx-virt.dtb
+
+Start emulation of rx-virt::
+  qemu-system-rx -kernel  -dtb  \
+  -append "earlycon"
diff --git a/docs/system/targets.rst b/docs/system/targets.rst
index eba3111247..946f513daa 100644
--- a/docs/system/targets.rst
+++ b/docs/system/targets.rst
@@ -17,3 +17,4 @@ Contents:
target-arm
target-m68k
target-xtensa
+   target-rx
-- 
2.20.1

Re: [PATCH 0/3] reference implementation of RSS

[Michael S. Tsirkin]

On Sun, Mar 08, 2020 at 02:44:59PM +0200, Yuri Benditovich wrote:
> On Sun, Mar 8, 2020 at 2:17 PM Michael S. Tsirkin  wrote:
> >
> > On Sun, Mar 08, 2020 at 11:56:38AM +0200, Yuri Benditovich wrote:
> > > On Sun, Mar 8, 2020 at 10:06 AM Michael S. Tsirkin  
> > > wrote:
> > > >
> > > > On Fri, Mar 06, 2020 at 11:50:30AM +0200, Yuri Benditovich wrote:
> > > > >
> > > > >
> > > > > On Fri, Mar 6, 2020 at 11:27 AM Jason Wang  
> > > > > wrote:
> > > > >
> > > > >
> > > > > On 2020/2/27 上午1:48, Yuri Benditovich wrote:
> > > > > > Support for VIRTIO_NET_F_RSS feature in QEMU for reference
> > > > > > purpose. Implements Toeplitz hash calculation for incoming
> > > > > > packets according to configuration provided by driver.
> > > > > >
> > > > > > This series requires previously submitted and accepted
> > > > > > patch to be applied:
> > > > > > 
> > > > > https://lists.gnu.org/archive/html/qemu-devel/2020-01/msg06448.html
> > > > > >
> > > > > > Yuri Benditovich (3):
> > > > > >virtio-net: introduce RSS RX steering feature
> > > > > >virtio-net: implement RSS configuration command
> > > > > >virtio-net: implement RX RSS processing
> > > > > >
> > > > > >   hw/net/trace-events |   3 +
> > > > > >   hw/net/virtio-net.c | 234
> > > > > +++-VIRTIO_NET_F_RSS
> > > > > >   include/hw/virtio/virtio-net.h  |  12 +
> > > > > >   include/standard-headers/linux/virtio_net.h |  37 +++-
> > > > > >   4 files changed, 273 insertions(+), 13 deletions(-)
> > > > > >
> > > > >
> > > > > One question before the reviewing.
> > > > >
> > > > > Do we need to deal with migration (which I think yes)?
> > > > >
> > > > >
> > > > > I think we don't (yet) as this is a reference implementation and the 
> > > > > main goal
> > > > > is to provide the functional reference for hardware solution.
> > > >
> > > >
> > > > That's fine, but then we must block migration, and add appropriate code
> > > > comments. Just silently losing data isn't a good idea.
> > >
> > > Note that there is no data lost, just the configuration of RSS is not 
> > > migrating.
> > > So, qemu just will not redirect the data to different queues after 
> > > migration.
> >
> > Right. Unlike auto-mq, the spec doesn't say the direction is best effort 
> > though,
> > so that would be a spec violation.
> >
> > > I would add the migration prevention in the next series with
> > > implementation of hash delivery to prevent different packet sizes in
> > > driver and qemu.
> >
> > And hopefully full migration support will follow.
> >
> > One other thing to check is vhost - I didn't check
> > what happens with this patchset but
> > I think at a minimum we need to fail vhost init,
> > until the backend implements the feature biit.
> 
> RSS feature currently is not indicated in case vhost is on.
> The same will be with hash report.

IIRC with vhost features not listed are assumed to be
implemented by qemu and not to need backend support.

Maybe we should change that to make things more robust
in the future ... Jason, Marc-André am I right? what do you think?



> >
> >
> > > >
> > > > > I agree with the general direction that for complete support of RSS 
> > > > > and hash
> > > > > delivery the best way is to do that in kernel using bpf.
> > > > > For that, IMO, the bpf should be a part of the kernel (it uses skb 
> > > > > fields) and
> > > > > the tap should receive just RSS parameters for it.
> > > > > At this stage we definitely will need to add migration support and 
> > > > > propagation
> > > > > of RSS parameters.
> > > > >
> > > > >
> > > > >
> > > > > Thanks
> > > > >
> > > > >
> > > >
> >

Re: [PATCH v5 00/16] APIC ID fixes for AMD EPYC CPU model

[Michael S. Tsirkin]

On Tue, Mar 03, 2020 at 01:56:51PM -0600, Babu Moger wrote:
> This series fixes APIC ID encoding problem reported on AMD EPYC cpu models.
> https://bugzilla.redhat.com/show_bug.cgi?id=1728166
> 
> Currently, the APIC ID is decoded based on the sequence
> sockets->dies->cores->threads. This works for most standard AMD and other
> vendors' configurations, but this decoding sequence does not follow that of
> AMD's APIC ID enumeration strictly. In some cases this can cause CPU topology
> inconsistency.  When booting a guest VM, the kernel tries to validate the
> topology, and finds it inconsistent with the enumeration of EPYC cpu models.
> 
> To fix the problem we need to build the topology as per the Processor
> Programming Reference (PPR) for AMD Family 17h Model 01h, Revision B1
> Processors. The documentation is available from the bugzilla Link below.
> 
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=206537
> 
> Here is the text from the PPR.
> Operating systems are expected to use Core::X86::Cpuid::SizeId[ApicIdSize], 
> the
> number of least significant bits in the Initial APIC ID that indicate core ID
> within a processor, in constructing per-core CPUID masks.
> Core::X86::Cpuid::SizeId[ApicIdSize] determines the maximum number of cores
> (MNC) that the processor could theoretically support, not the actual number of
> cores that are actually implemented or enabled on the processor, as indicated
> by Core::X86::Cpuid::SizeId[NC].
> Each Core::X86::Apic::ApicId[ApicId] register is preset as follows:
> • ApicId[6] = Socket ID.
> • ApicId[5:4] = Node ID.
> • ApicId[3] = Logical CCX L3 complex ID
> • ApicId[2:0]= (SMT) ? {LogicalCoreID[1:0],ThreadId} : 
> {1'b0,LogicalCoreID[1:0]}


Looks reasonable:

Acked-by: Michael S. Tsirkin 

belongs in Eduardo's tree I guess.

> v5:
>  Generated the patches on top of git://github.com/ehabkost/qemu.git 
> (x86-next).
>  Changes from v4.
>  1. Re-arranged the patches 2 and 4 as suggested by Igor.
>  2. Kept the apicid handler functions inside X86MachineState as discussed.
> These handlers are loaded from X86CPUDefinitions.
>  3. Removed unnecessary X86CPUstate initialization from x86_cpu_new. Suggested
> by Igor.
>  4. And other minor changes related to patch format.
> 
> v4:
>  
> https://lore.kernel.org/qemu-devel/158161767653.48948.10578064482878399556.st...@naples-babu.amd.com/
>  Changes from v3.
>  1. Moved the arch_id calculation inside the function x86_cpus_init. With 
> this change,
> we dont need to change common numa code.(suggested by Igor)
>  2. Introduced the model specific handlers inside X86CPUDefinitions.
> These handlers are loaded into X86MachineState during the init.
>  3. Removed llc_id from x86CPU.
>  4. Removed init_apicid_fn hanlder from MachineClass. Kept all the code 
> changes
> inside the x86.
>  5. Added new handler function apicid_pkg_offset for pkg_offset calculation.
>  6. And some Other minor changes.
> 
> v3:
>   
> https://lore.kernel.org/qemu-devel/157541968844.46157.17994918142533791313.st...@naples-babu.amd.com/
>  
>   1. Consolidated the topology information in structure X86CPUTopoInfo.
>   2. Changed the ccx_id to llc_id as commented by upstream.
>   3. Generalized the apic id decoding. It is mostly similar to current apic id
>  except that it adds new field llc_id when numa configured. Removes all 
> the
>  hardcoded values.
>   4. Removed the earlier parse_numa split. And moved the numa node 
> initialization
>  inside the numa_complete_configuration. This is bit cleaner as commented 
> by 
>  Eduardo.
>   5. Added new function init_apicid_fn inside machine_class structure. This
>  will be used to update the apic id handler specific to cpu model.
>   6. Updated the cpuid unit tests.
>   7. TODO : Need to figure out how to dynamically update the handlers using 
> cpu models.
>  I might some guidance on that.
> 
> v2:
>   
> https://lore.kernel.org/qemu-devel/156779689013.21957.1631551572950676212.stgit@localhost.localdomain/
>   1. Introduced the new property epyc to enable new epyc mode.
>   2. Separated the epyc mode and non epyc mode function.
>   3. Introduced function pointers in PCMachineState to handle the
>  differences.
>   4. Mildly tested different combinations to make things are working as 
> expected.
>   5. TODO : Setting the epyc feature bit needs to be worked out. This feature 
> is
>  supported only on AMD EPYC models. I may need some guidance on that.
> 
> v1:
>   
> https://lore.kernel.org/qemu-devel/20190731232032.51786-1-babu.mo...@amd.com/
> 
> ---
> 
> Babu Moger (16):
>   hw/i386: Rename X86CPUTopoInfo structure to X86CPUTopoIDs
>   hw/i386: Introduce X86CPUTopoInfo to contain topology info
>   hw/i386: Consolidate topology functions
>   machine: Add SMP Sockets in CpuTopology
>   hw/i386: Remove unnecessary initialization in x86_cpu_new
>   hw/i386: Update structures to save the number of nodes per package
>   hw/i386: R

Re: [PATCH v4 2/3] mac_via: fix incorrect creation of mos6522 device in mac_via

[Peter Maydell]

On Thu, 5 Mar 2020 at 06:39, Pan Nengyuan  wrote:
>
> This patch fix a bug in mac_via where it failed to actually realize devices 
> it was using.
> And move the init codes which inits the mos6522 objects and properties on 
> them from realize()
> into init(). However, we keep qdev_set_parent_bus in realize(), otherwise it 
> will cause
> device-introspect-test test fail. Then do the realize mos6522 device in the 
> mac_vir_realize.
>
> Signed-off-by: Pan Nengyuan 
> ---
> Cc: Laurent Vivier 
> Cc: Mark Cave-Ayland 
> ---

> diff --git a/hw/misc/mac_via.c b/hw/misc/mac_via.c
> index b7d0012794..4c5ad08805 100644
> --- a/hw/misc/mac_via.c
> +++ b/hw/misc/mac_via.c
> @@ -868,24 +868,24 @@ static void mac_via_reset(DeviceState *dev)
>  static void mac_via_realize(DeviceState *dev, Error **errp)
>  {
>  MacVIAState *m = MAC_VIA(dev);
> -MOS6522State *ms;
>  struct tm tm;
>  int ret;
> +Error *err = NULL;
>
> -/* Init VIAs 1 and 2 */
> -sysbus_init_child_obj(OBJECT(dev), "via1", &m->mos6522_via1,
> -  sizeof(m->mos6522_via1), TYPE_MOS6522_Q800_VIA1);
> +qdev_set_parent_bus(DEVICE(&m->mos6522_via1), sysbus_get_default());
> +qdev_set_parent_bus(DEVICE(&m->mos6522_via2), sysbus_get_default());

Rather than manually setting the parent bus, you can use
sysbus_init_child_obj() instead of object_initialize_child() --
it is a convenience function that does both object_initialize_child()
and qdev_set_parent_bus() for you.

> -sysbus_init_child_obj(OBJECT(dev), "via2", &m->mos6522_via2,
> -  sizeof(m->mos6522_via2), TYPE_MOS6522_Q800_VIA2);
> +object_property_set_bool(OBJECT(&m->mos6522_via1), true, "realized", 
> &err);
> +if (err != NULL) {
> +error_propagate(errp, err);
> +return;
> +}
>
> -/* Pass through mos6522 output IRQs */
> -ms = MOS6522(&m->mos6522_via1);
> -object_property_add_alias(OBJECT(dev), "irq[0]", OBJECT(ms),
> -  SYSBUS_DEVICE_GPIO_IRQ "[0]", &error_abort);
> -ms = MOS6522(&m->mos6522_via2);
> -object_property_add_alias(OBJECT(dev), "irq[1]", OBJECT(ms),
> -  SYSBUS_DEVICE_GPIO_IRQ "[0]", &error_abort);
> +object_property_set_bool(OBJECT(&m->mos6522_via2), true, "realized", 
> &err);
> +if (err != NULL) {
> +error_propagate(errp, err);
> +return;
> +}
>
>  /* Pass through mos6522 input IRQs */
>  qdev_pass_gpios(DEVICE(&m->mos6522_via1), dev, "via1-irq");
> @@ -932,6 +932,7 @@ static void mac_via_init(Object *obj)
>  {
>  SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
>  MacVIAState *m = MAC_VIA(obj);
> +MOS6522State *ms;
>
>  /* MMIO */
>  memory_region_init(&m->mmio, obj, "mac-via", 2 * VIA_SIZE);
> @@ -948,6 +949,22 @@ static void mac_via_init(Object *obj)
>  /* ADB */
>  qbus_create_inplace((BusState *)&m->adb_bus, sizeof(m->adb_bus),
>  TYPE_ADB_BUS, DEVICE(obj), "adb.0");
> +
> +/* Init VIAs 1 and 2 */
> +object_initialize_child(OBJECT(m), "via1", &m->mos6522_via1,
> +sizeof(m->mos6522_via1), TYPE_MOS6522_Q800_VIA1,
> +&error_abort, NULL);
> +object_initialize_child(OBJECT(m), "via2", &m->mos6522_via2,
> +sizeof(m->mos6522_via2), TYPE_MOS6522_Q800_VIA2,
> +&error_abort, NULL);
> +
> +/* Pass through mos6522 output IRQs */
> +ms = MOS6522(&m->mos6522_via1);
> +object_property_add_alias(OBJECT(m), "irq[0]", OBJECT(ms),
> +  SYSBUS_DEVICE_GPIO_IRQ "[0]", &error_abort);

There's no point in using the MOS6522() cast to get a MOS6522*,
because the only thing you do with it is immediately cast it
to an Object* with the OBJECT() cast. Just use the OBJECT cast directly.

> +ms = MOS6522(&m->mos6522_via2);
> +object_property_add_alias(OBJECT(m), "irq[1]", OBJECT(ms),
> +  SYSBUS_DEVICE_GPIO_IRQ "[0]", &error_abort);
>  }
>
>  static void postload_update_cb(void *opaque, int running, RunState state)

thanks
-- PMM

[PULL 0/4] virtio, pci, pc: fixes, cleanups, features

[Michael S. Tsirkin]

The following changes since commit 67f17e23baca5dd545fe98b01169cc351a70fe35:

  Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into staging 
(2020-03-06 17:15:36 +)

are available in the Git repository at:

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

for you to fetch changes up to a6f65f4fc217713ee2c78b99baae1cc31c761778:

  hw/i386/intel_iommu: Simplify vtd_find_as_from_bus_num() logic (2020-03-08 
09:27:09 -0400)


virtio, pci, pc: fixes, cleanups, features

Bugfixes, cleanups all over the place.
Ability to disable hotplug for pci express ports.

Signed-off-by: Michael S. Tsirkin 


Jason Wang (1):
  vhost: correctly turn on VIRTIO_F_IOMMU_PLATFORM

Julia Suvorova (1):
  pcie_root_port: Add hotplug disabling option

Nick Erdmann (1):
  vhost-vsock: fix error message output

Philippe Mathieu-Daudé (1):
  hw/i386/intel_iommu: Simplify vtd_find_as_from_bus_num() logic

 include/hw/pci/pcie.h  |  2 +-
 include/hw/pci/pcie_port.h |  3 +++
 hw/i386/intel_iommu.c  | 34 ++
 hw/pci-bridge/pcie_root_port.c |  2 +-
 hw/pci-bridge/xio3130_downstream.c |  2 +-
 hw/pci/pcie.c  | 11 +++
 hw/pci/pcie_port.c |  1 +
 hw/virtio/vhost-vsock.c|  2 +-
 hw/virtio/vhost.c  | 12 +++-
 9 files changed, 44 insertions(+), 25 deletions(-)

[PULL 1/4] pcie_root_port: Add hotplug disabling option

[Michael S. Tsirkin]

From: Julia Suvorova 

Make hot-plug/hot-unplug on PCIe Root Ports optional to allow libvirt
manage it and restrict unplug for the whole machine. This is going to
prevent user-initiated unplug in guests (Windows mostly).
Hotplug is enabled by default.
Usage:
-device pcie-root-port,hotplug=off,...

If you want to disable hot-unplug on some downstream ports of one
switch, disable hot-unplug on PCIe Root Port connected to the upstream
port as well as on the selected downstream ports.

Discussion related:
https://lists.gnu.org/archive/html/qemu-devel/2020-02/msg00530.html

Signed-off-by: Julia Suvorova 
Message-Id: <20200226174607.205941-1-jus...@redhat.com>
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
Reviewed-by: Ján Tomko 
---
 include/hw/pci/pcie.h  |  2 +-
 include/hw/pci/pcie_port.h |  3 +++
 hw/pci-bridge/pcie_root_port.c |  2 +-
 hw/pci-bridge/xio3130_downstream.c |  2 +-
 hw/pci/pcie.c  | 11 +++
 hw/pci/pcie_port.c |  1 +
 6 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/include/hw/pci/pcie.h b/include/hw/pci/pcie.h
index 7064875835..14c58ebdb6 100644
--- a/include/hw/pci/pcie.h
+++ b/include/hw/pci/pcie.h
@@ -104,7 +104,7 @@ void pcie_cap_deverr_reset(PCIDevice *dev);
 void pcie_cap_lnkctl_init(PCIDevice *dev);
 void pcie_cap_lnkctl_reset(PCIDevice *dev);
 
-void pcie_cap_slot_init(PCIDevice *dev, uint16_t slot);
+void pcie_cap_slot_init(PCIDevice *dev, PCIESlot *s);
 void pcie_cap_slot_reset(PCIDevice *dev);
 void pcie_cap_slot_get(PCIDevice *dev, uint16_t *slt_ctl, uint16_t *slt_sta);
 void pcie_cap_slot_write_config(PCIDevice *dev,
diff --git a/include/hw/pci/pcie_port.h b/include/hw/pci/pcie_port.h
index 4b3d254b08..caae57573b 100644
--- a/include/hw/pci/pcie_port.h
+++ b/include/hw/pci/pcie_port.h
@@ -55,6 +55,9 @@ struct PCIESlot {
 
 /* Disable ACS (really for a pcie_root_port) */
 booldisable_acs;
+
+/* Indicates whether hot-plug is enabled on the slot */
+boolhotplug;
 QLIST_ENTRY(PCIESlot) next;
 };
 
diff --git a/hw/pci-bridge/pcie_root_port.c b/hw/pci-bridge/pcie_root_port.c
index 0ba4e4dea4..f1cfe9d14a 100644
--- a/hw/pci-bridge/pcie_root_port.c
+++ b/hw/pci-bridge/pcie_root_port.c
@@ -94,7 +94,7 @@ static void rp_realize(PCIDevice *d, Error **errp)
 
 pcie_cap_arifwd_init(d);
 pcie_cap_deverr_init(d);
-pcie_cap_slot_init(d, s->slot);
+pcie_cap_slot_init(d, s);
 pcie_cap_root_init(d);
 
 pcie_chassis_create(s->chassis);
diff --git a/hw/pci-bridge/xio3130_downstream.c 
b/hw/pci-bridge/xio3130_downstream.c
index 153a4acad2..04aae72cd6 100644
--- a/hw/pci-bridge/xio3130_downstream.c
+++ b/hw/pci-bridge/xio3130_downstream.c
@@ -94,7 +94,7 @@ static void xio3130_downstream_realize(PCIDevice *d, Error 
**errp)
 }
 pcie_cap_flr_init(d);
 pcie_cap_deverr_init(d);
-pcie_cap_slot_init(d, s->slot);
+pcie_cap_slot_init(d, s);
 pcie_cap_arifwd_init(d);
 
 pcie_chassis_create(s->chassis);
diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
index 08718188bb..0eb3a2a5d2 100644
--- a/hw/pci/pcie.c
+++ b/hw/pci/pcie.c
@@ -495,7 +495,7 @@ void pcie_cap_slot_unplug_request_cb(HotplugHandler 
*hotplug_dev,
 
 /* pci express slot for pci express root/downstream port
PCI express capability slot registers */
-void pcie_cap_slot_init(PCIDevice *dev, uint16_t slot)
+void pcie_cap_slot_init(PCIDevice *dev, PCIESlot *s)
 {
 uint32_t pos = dev->exp.exp_cap;
 
@@ -505,13 +505,16 @@ void pcie_cap_slot_init(PCIDevice *dev, uint16_t slot)
 pci_long_test_and_clear_mask(dev->config + pos + PCI_EXP_SLTCAP,
  ~PCI_EXP_SLTCAP_PSN);
 pci_long_test_and_set_mask(dev->config + pos + PCI_EXP_SLTCAP,
-   (slot << PCI_EXP_SLTCAP_PSN_SHIFT) |
+   (s->slot << PCI_EXP_SLTCAP_PSN_SHIFT) |
PCI_EXP_SLTCAP_EIP |
-   PCI_EXP_SLTCAP_HPS |
-   PCI_EXP_SLTCAP_HPC |
PCI_EXP_SLTCAP_PIP |
PCI_EXP_SLTCAP_AIP |
PCI_EXP_SLTCAP_ABP);
+if (s->hotplug) {
+pci_long_test_and_set_mask(dev->config + pos + PCI_EXP_SLTCAP,
+   PCI_EXP_SLTCAP_HPS |
+   PCI_EXP_SLTCAP_HPC);
+}
 
 if (dev->cap_present & QEMU_PCIE_SLTCAP_PCP) {
 pci_long_test_and_set_mask(dev->config + pos + PCI_EXP_SLTCAP,
diff --git a/hw/pci/pcie_port.c b/hw/pci/pcie_port.c
index f8263cb306..eb563ad435 100644
--- a/hw/pci/pcie_port.c
+++ b/hw/pci/pcie_port.c
@@ -147,6 +147,7 @@ static const TypeInfo pcie_port_type_info = {
 static Property pcie_slot_props[] = {
 DEFINE_PROP_UINT8("chassis", PCIESlot, chassis, 0),
 DEFINE_PROP_UINT16("slot", PCIESlot, slot, 0),
+DEFINE_PROP_BOOL("hotplug", PCIESlot, hotplug, true),

[PULL 3/4] vhost-vsock: fix error message output

[Michael S. Tsirkin]

From: Nick Erdmann 

error_setg_errno takes a positive error number, so we should not invert
errno's sign.

Signed-off-by: Nick Erdmann 
Message-Id: <04df3f47-c93b-1d02-d250-f9bda8dbc...@nirf.de>
Reviewed-by: Ján Tomko 
Fixes: fc0b9b0e1cbb ("vhost-vsock: add virtio sockets device")
Reviewed-by: Laurent Vivier 
Reviewed-by: Stefano Garzarella 
Reviewed-by: Stefan Hajnoczi 
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
---
 hw/virtio/vhost-vsock.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/virtio/vhost-vsock.c b/hw/virtio/vhost-vsock.c
index 66da96583b..9f9093e196 100644
--- a/hw/virtio/vhost-vsock.c
+++ b/hw/virtio/vhost-vsock.c
@@ -325,7 +325,7 @@ static void vhost_vsock_device_realize(DeviceState *dev, 
Error **errp)
 } else {
 vhostfd = open("/dev/vhost-vsock", O_RDWR);
 if (vhostfd < 0) {
-error_setg_errno(errp, -errno,
+error_setg_errno(errp, errno,
  "vhost-vsock: failed to open vhost device");
 return;
 }
-- 
MST

[PULL 4/4] hw/i386/intel_iommu: Simplify vtd_find_as_from_bus_num() logic

[Michael S. Tsirkin]

From: Philippe Mathieu-Daudé 

The vtd_find_as_from_bus_num() function was introduced (in commit
dbaabb25f) in a code format that could return an incorrect pointer,
which was later fixed by commit a2e1cd41ccf.
We could have avoided this by writing the if() statement differently.
Do it now, in case this function is re-used. The code is easier to
review (harder to miss bugs).

Reviewed-by: Peter Xu 
Reviewed-by: Eric Auger 
Signed-off-by: Philippe Mathieu-Daudé 
Message-Id: <20200305102702.31512-1-phi...@redhat.com>
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
---
 hw/i386/intel_iommu.c | 34 ++
 1 file changed, 18 insertions(+), 16 deletions(-)

diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 6258c58ac9..204b6841ec 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -987,24 +987,26 @@ static bool vtd_slpte_nonzero_rsvd(uint64_t slpte, 
uint32_t level)
 static VTDBus *vtd_find_as_from_bus_num(IntelIOMMUState *s, uint8_t bus_num)
 {
 VTDBus *vtd_bus = s->vtd_as_by_bus_num[bus_num];
-if (!vtd_bus) {
-/*
- * Iterate over the registered buses to find the one which
- * currently hold this bus number, and update the bus_num
- * lookup table:
- */
-GHashTableIter iter;
+GHashTableIter iter;
 
-g_hash_table_iter_init(&iter, s->vtd_as_by_busptr);
-while (g_hash_table_iter_next(&iter, NULL, (void **)&vtd_bus)) {
-if (pci_bus_num(vtd_bus->bus) == bus_num) {
-s->vtd_as_by_bus_num[bus_num] = vtd_bus;
-return vtd_bus;
-}
-}
-vtd_bus = NULL;
+if (vtd_bus) {
+return vtd_bus;
 }
-return vtd_bus;
+
+/*
+ * Iterate over the registered buses to find the one which
+ * currently holds this bus number and update the bus_num
+ * lookup table.
+ */
+g_hash_table_iter_init(&iter, s->vtd_as_by_busptr);
+while (g_hash_table_iter_next(&iter, NULL, (void **)&vtd_bus)) {
+if (pci_bus_num(vtd_bus->bus) == bus_num) {
+s->vtd_as_by_bus_num[bus_num] = vtd_bus;
+return vtd_bus;
+}
+}
+
+return NULL;
 }
 
 /* Given the @iova, get relevant @slptep. @slpte_level will be the last level
-- 
MST

[PULL 2/4] vhost: correctly turn on VIRTIO_F_IOMMU_PLATFORM

[Michael S. Tsirkin]

From: Jason Wang 

We turn on device IOTLB via VIRTIO_F_IOMMU_PLATFORM unconditionally on
platform without IOMMU support. This can lead unnecessary IOTLB
transactions which will damage the performance.

Fixing this by check whether the device is backed by IOMMU and disable
device IOTLB.

Reported-by: Halil Pasic 
Tested-by: Halil Pasic 
Reviewed-by: Halil Pasic 
Signed-off-by: Jason Wang 
Message-Id: <20200302042454.24814-1-jasow...@redhat.com>
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
---
 hw/virtio/vhost.c | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 0d226dae10..01ebe12f28 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -290,7 +290,14 @@ static int vhost_dev_has_iommu(struct vhost_dev *dev)
 {
 VirtIODevice *vdev = dev->vdev;
 
-return virtio_host_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM);
+/*
+ * For vhost, VIRTIO_F_IOMMU_PLATFORM means the backend support
+ * incremental memory mapping API via IOTLB API. For platform that
+ * does not have IOMMU, there's no need to enable this feature
+ * which may cause unnecessary IOTLB miss/update trnasactions.
+ */
+return vdev->dma_as != &address_space_memory &&
+   virtio_host_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM);
 }
 
 static void *vhost_memory_map(struct vhost_dev *dev, hwaddr addr,
@@ -765,6 +772,9 @@ static int vhost_dev_set_features(struct vhost_dev *dev,
 if (enable_log) {
 features |= 0x1ULL << VHOST_F_LOG_ALL;
 }
+if (!vhost_dev_has_iommu(dev)) {
+features &= ~(0x1ULL << VIRTIO_F_IOMMU_PLATFORM);
+}
 r = dev->vhost_ops->vhost_set_features(dev, features);
 if (r < 0) {
 VHOST_OPS_DEBUG("vhost_set_features failed");
-- 
MST

Re: [PATCH v4 0/3] delay timer_new from init to realize to fix memleaks.

[Peter Maydell]

On Sun, 8 Mar 2020 at 11:58, Mark Cave-Ayland
 wrote:
> I just tried this patchset applied on top of git master and it causes 
> qemu-system-ppc
> to segfault on startup:
>
> $ gdb --args ./qemu-system-ppc
> ...
> ...
> Thread 1 "qemu-system-ppc" received signal SIGSEGV, Segmentation fault.
> 0x55e7e38c in timer_del (ts=0x0) at util/qemu-timer.c:429
> 429 QEMUTimerList *timer_list = ts->timer_list;
> (gdb) bt
> #0  0x55e7e38c in timer_del (ts=0x0) at util/qemu-timer.c:429
> #1  0x55b5d2c1 in mos6522_reset (dev=0x56e0ac50) at 
> hw/misc/mos6522.c:468
> #2  0x55b63570 in mos6522_cuda_reset (dev=0x56e0ac50) at
> hw/misc/macio/cuda.c:599

It looks like we haven't caught all the cases of "somebody created a
MOS6522 (or one of its subclasses) but forgot to realize it". This
particular one I think is the s->cuda which is inited in macio_oldworld_init()
but not realized in macio_oldworld_realize(). I think that pmu_init() in
hw/misc/macio/pmu.c also has this bug. We need to go through and
audit all the places where we create TYPE_MOS6522 or any of its
subclasses and make sure they are also realizing the devices they create.
(The presence of the new 3-phase reset infrastructure in the backtrace
is a red herring here -- this would have crashed the same way with the
old code too.)

We should probably find some generic place in Device code where we
can stick an assert "are we trying to reset an unrealized device?"
because I bet we have other instances of this bug which we haven't
noticed because the reset function happens to not misbehave on
an inited-but-not-realized device...

thanks
-- PMM

[PATCH v2 04/14] block/amend: separate amend and create options for qemu-img

[Maxim Levitsky]

Some options are only useful for creation
(or hard to be amended, like cluster size for qcow2), while some other
options are only useful for amend, like upcoming keyslot management
options for luks

Since currently only qcow2 supports amend, move all its options
to a common macro and then include it in each action option list.

In future it might be useful to remove some options which are
not supported anyway from amend list, which currently
cause an error message if amended.

Signed-off-by: Maxim Levitsky 
---
 block/qcow2.c | 160 +-
 include/block/block_int.h |   4 +
 qemu-img.c|  18 ++---
 3 files changed, 100 insertions(+), 82 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index b55e5b7c1f..9574085772 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -5440,83 +5440,96 @@ void qcow2_signal_corruption(BlockDriverState *bs, bool 
fatal, int64_t offset,
 s->signaled_corruption = true;
 }
 
+#define QCOW_COMMON_OPTIONS \
+{   \
+.name = BLOCK_OPT_SIZE, \
+.type = QEMU_OPT_SIZE,  \
+.help = "Virtual disk size" \
+},  \
+{   \
+.name = BLOCK_OPT_COMPAT_LEVEL, \
+.type = QEMU_OPT_STRING,\
+.help = "Compatibility level (v2 [0.10] or v3 [1.1])"   \
+},  \
+{   \
+.name = BLOCK_OPT_BACKING_FILE, \
+.type = QEMU_OPT_STRING,\
+.help = "File name of a base image" \
+},  \
+{   \
+.name = BLOCK_OPT_BACKING_FMT,  \
+.type = QEMU_OPT_STRING,\
+.help = "Image format of the base image"\
+},  \
+{   \
+.name = BLOCK_OPT_DATA_FILE,\
+.type = QEMU_OPT_STRING,\
+.help = "File name of an external data file"\
+},  \
+{   \
+.name = BLOCK_OPT_DATA_FILE_RAW,\
+.type = QEMU_OPT_BOOL,  \
+.help = "The external data file must stay valid "   \
+"as a raw image"\
+},  \
+{   \
+.name = BLOCK_OPT_ENCRYPT,  \
+.type = QEMU_OPT_BOOL,  \
+.help = "Encrypt the image with format 'aes'. (Deprecated " \
+"in favor of " BLOCK_OPT_ENCRYPT_FORMAT "=aes)",\
+},  \
+{   \
+.name = BLOCK_OPT_ENCRYPT_FORMAT,   \
+.type = QEMU_OPT_STRING,\
+.help = "Encrypt the image, format choices: 'aes', 'luks'", \
+},  \
+BLOCK_CRYPTO_OPT_DEF_KEY_SECRET("encrypt.", \
+"ID of secret providing qcow AES key or LUKS passphrase"),  \
+BLOCK_CRYPTO_OPT_DEF_LUKS_CIPHER_ALG("encrypt."),   \
+BLOCK_CRYPTO_OPT_DEF_LUKS_CIPHER_MODE("encrypt."),  \
+BLOCK_CRYPTO_OPT_DEF_LUKS_IVGEN_ALG("encrypt."),\
+BLOCK_CRYPTO_OPT_DEF_LUKS_IVGEN_HASH_ALG("encrypt."),   \
+BLOCK_CRYPTO_OPT_DEF_LUKS_HASH_ALG("encrypt."), \
+BLOCK_CRYPTO_OPT_DEF_LUKS_ITER_TIME("encrypt."),\
+{   \
+.name = BLOCK_OPT_CLUSTER_SIZE, \
+.type = QEMU_OPT_SIZE,  \
+.help = "qcow2 cluster size",   \
+.def_value_str = stringify(DEFAULT_CLUSTER_SIZE)\
+},   

[PATCH v2 00/14] LUKS: encryption slot management using amend interface

[Maxim Levitsky]

Hi!
Here is the updated series of my patches, incorporating all the feedback I 
received.

This implements the API interface that we agreed upon except that I merged the
LUKSKeyslotActive/LUKSKeyslotInactive union into a struct because otherwise
I need nested unions which are not supported currently by QAPI parser.
This didn't change the API and thus once support for nested unions is there,
it can always be implemented in backward compatible way.

I hope that this series will finally be considered for merging, since I am 
somewhat running
out of time to finish this task.

Patches are strictly divided by topic to 3 groups, and each group depends on 
former groups.

* Patches 1,2 implement qcrypto generic amend interface, including definition
  of structs used in crypto.json and implement this in luks crypto driver
  Nothing is exposed to the user at this stage

* Patches 3-9 use the code from patches 1,2 to implement qemu-img amend based 
encryption slot management
  for luks and for qcow2, and add a bunch of iotests to cover that.

* Patches 10-13 add x-blockdev-amend (I'll drop the -x prefix if you like), and 
wire it
  to luks and qcow2 driver to implement qmp based encryption slot management 
also using
  the code from patches 1,2, and also add a bunch of iotests to cover this.

Tested with -raw,-qcow2 and -luks iotests and 'make check'

Best regards,
Maxim Levitsky

clone of "luks-keymgmnt-v2"

Maxim Levitsky (14):
  qcrypto/core: add generic infrastructure for crypto options amendment
  qcrypto/luks: implement encryption key management
  block/amend: add 'force' option
  block/amend: separate amend and create options for qemu-img
  block/amend: refactor qcow2 amend options
  block/crypto: rename two functions
  block/crypto: implement the encryption key management
  block/qcow2: extend qemu-img amend interface with crypto options
  iotests: filter few more luks specific create options
  iotests: qemu-img tests for luks key management
  block/core: add generic infrastructure for x-blockdev-amend qmp
command
  block/crypto: implement blockdev-amend
  block/qcow2: implement blockdev-amend
  iotests: add tests for blockdev-amend

 block.c  |   4 +-
 block/Makefile.objs  |   2 +-
 block/amend.c| 108 +
 block/crypto.c   | 203 ++--
 block/crypto.h   |  47 +++-
 block/qcow2.c| 314 ++--
 crypto/block-luks.c  | 398 ++-
 crypto/block.c   |  31 +++
 crypto/blockpriv.h   |   8 +
 docs/tools/qemu-img.rst  |   5 +-
 include/block/block.h|   1 +
 include/block/block_int.h|  24 +-
 include/crypto/block.h   |  22 ++
 qapi/block-core.json |  68 ++
 qapi/crypto.json |  75 +-
 qapi/job.json|   4 +-
 qemu-img-cmds.hx |   4 +-
 qemu-img.c   |  44 +++-
 tests/qemu-iotests/049.out   | 102 
 tests/qemu-iotests/061.out   |  12 +-
 tests/qemu-iotests/079.out   |  18 +-
 tests/qemu-iotests/082.out   | 176 --
 tests/qemu-iotests/085.out   |  38 +--
 tests/qemu-iotests/087.out   |   6 +-
 tests/qemu-iotests/115.out   |   2 +-
 tests/qemu-iotests/121.out   |   4 +-
 tests/qemu-iotests/125.out   | 192 +++
 tests/qemu-iotests/134.out   |   2 +-
 tests/qemu-iotests/144.out   |   4 +-
 tests/qemu-iotests/158.out   |   4 +-
 tests/qemu-iotests/182.out   |   2 +-
 tests/qemu-iotests/185.out   |   8 +-
 tests/qemu-iotests/188.out   |   2 +-
 tests/qemu-iotests/189.out   |   4 +-
 tests/qemu-iotests/198.out   |   4 +-
 tests/qemu-iotests/243.out   |  16 +-
 tests/qemu-iotests/250.out   |   2 +-
 tests/qemu-iotests/255.out   |   8 +-
 tests/qemu-iotests/263.out   |   4 +-
 tests/qemu-iotests/280.out   |   2 +-
 tests/qemu-iotests/284.out   |   6 +-
 tests/qemu-iotests/300   | 207 
 tests/qemu-iotests/300.out   |  99 
 tests/qemu-iotests/301   |  90 +++
 tests/qemu-iotests/301.out   |  30 +++
 tests/qemu-iotests/302   | 278 +
 tests/qemu-iotests/302.out   |  40 
 tests/qemu-iotests/303   | 233 ++
 tests/qemu-iotests/303.out   |  33 +++
 tests/qemu-iotests/common.filter |   6 +-
 tests/qemu-iotests/group |   6 +
 51 files changed, 2486 insertions(+), 516 deletions(-)
 create mode 100644 block/amend.c
 create mode 100755 tests/qemu-iotests/300
 create mode 100644 tests/qemu-iotests/300.out
 create mode 100755 tests/qemu-iotests/301
 create mode 100644 tests/qemu-iotests/301.out
 create mode 100755 tests/qemu-iotests/302
 create mode 100644 tests/qemu-iotests/302.out
 create mode 100755 tests/qemu-iotests/303
 create mode 100644 tests/qemu-iotests/303.o

[PATCH v2 01/14] qcrypto/core: add generic infrastructure for crypto options amendment

[Maxim Levitsky]

This will be used first to implement luks keyslot management.

block_crypto_amend_opts_init will be used to convert
qemu-img cmdline to QCryptoBlockAmendOptions

Signed-off-by: Maxim Levitsky 
Reviewed-by: Daniel P. Berrangé 
---
 block/crypto.c | 17 +
 block/crypto.h |  3 +++
 crypto/block.c | 31 +++
 crypto/blockpriv.h |  8 
 include/crypto/block.h | 22 ++
 qapi/crypto.json   | 16 
 6 files changed, 97 insertions(+)

diff --git a/block/crypto.c b/block/crypto.c
index 24823835c1..ecf96a7a9b 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -184,6 +184,23 @@ block_crypto_create_opts_init(QDict *opts, Error **errp)
 return ret;
 }
 
+QCryptoBlockAmendOptions *
+block_crypto_amend_opts_init(QDict *opts, Error **errp)
+{
+Visitor *v;
+QCryptoBlockAmendOptions *ret;
+
+v = qobject_input_visitor_new_flat_confused(opts, errp);
+if (!v) {
+return NULL;
+}
+
+visit_type_QCryptoBlockAmendOptions(v, NULL, &ret, errp);
+
+visit_free(v);
+return ret;
+}
+
 
 static int block_crypto_open_generic(QCryptoBlockFormat format,
  QemuOptsList *opts_spec,
diff --git a/block/crypto.h b/block/crypto.h
index b935695e79..06e044c9be 100644
--- a/block/crypto.h
+++ b/block/crypto.h
@@ -91,6 +91,9 @@
 QCryptoBlockCreateOptions *
 block_crypto_create_opts_init(QDict *opts, Error **errp);
 
+QCryptoBlockAmendOptions *
+block_crypto_amend_opts_init(QDict *opts, Error **errp);
+
 QCryptoBlockOpenOptions *
 block_crypto_open_opts_init(QDict *opts, Error **errp);
 
diff --git a/crypto/block.c b/crypto/block.c
index 325752871c..0ce67db641 100644
--- a/crypto/block.c
+++ b/crypto/block.c
@@ -115,6 +115,37 @@ QCryptoBlock 
*qcrypto_block_create(QCryptoBlockCreateOptions *options,
 }
 
 
+int qcrypto_block_amend_options(QCryptoBlock *block,
+QCryptoBlockReadFunc readfunc,
+QCryptoBlockWriteFunc writefunc,
+void *opaque,
+QCryptoBlockAmendOptions *options,
+bool force,
+Error **errp)
+{
+if (options->format != block->format) {
+error_setg(errp,
+   "Cannot amend encryption format");
+return -1;
+}
+
+if (!block->driver->amend) {
+error_setg(errp,
+   "Crypto format %s doesn't support format options amendment",
+   QCryptoBlockFormat_str(block->format));
+return -1;
+}
+
+return block->driver->amend(block,
+readfunc,
+writefunc,
+opaque,
+options,
+force,
+errp);
+}
+
+
 QCryptoBlockInfo *qcrypto_block_get_info(QCryptoBlock *block,
  Error **errp)
 {
diff --git a/crypto/blockpriv.h b/crypto/blockpriv.h
index 71c59cb542..3c7ccea504 100644
--- a/crypto/blockpriv.h
+++ b/crypto/blockpriv.h
@@ -62,6 +62,14 @@ struct QCryptoBlockDriver {
   void *opaque,
   Error **errp);
 
+int (*amend)(QCryptoBlock *block,
+ QCryptoBlockReadFunc readfunc,
+ QCryptoBlockWriteFunc writefunc,
+ void *opaque,
+ QCryptoBlockAmendOptions *options,
+ bool force,
+ Error **errp);
+
 int (*get_info)(QCryptoBlock *block,
 QCryptoBlockInfo *info,
 Error **errp);
diff --git a/include/crypto/block.h b/include/crypto/block.h
index d49d2c2da9..e4553cf33d 100644
--- a/include/crypto/block.h
+++ b/include/crypto/block.h
@@ -144,6 +144,28 @@ QCryptoBlock 
*qcrypto_block_create(QCryptoBlockCreateOptions *options,
void *opaque,
Error **errp);
 
+/**
+ * qcrypto_block_amend_options:
+ * @block: the block encryption object
+ *
+ * @readfunc: callback for reading data from the volume header
+ * @writefunc: callback for writing data to the volume header
+ * @opaque: data to pass to @readfunc and @writefunc
+ * @options: the new/amended encryption options
+ * @force: hint for the driver to allow unsafe operation
+ * @errp: error pointer
+ *
+ * Changes the crypto options of the encryption format
+ *
+ */
+int qcrypto_block_amend_options(QCryptoBlock *block,
+QCryptoBlockReadFunc readfunc,
+QCryptoBlockWriteFunc writefunc,
+void *opaque,
+QCryptoBlockAmendOptions *options,
+bool force,
+Error **errp);
+
 
 /**
  * qcrypto_block_get_info:
diff --g

[PATCH v2 10/14] iotests: qemu-img tests for luks key management

[Maxim Levitsky]

This commit adds two tests, which test the new amend interface
of both luks raw images and qcow2 luks encrypted images.

Signed-off-by: Maxim Levitsky 
---
 tests/qemu-iotests/300 | 207 +
 tests/qemu-iotests/300.out |  99 ++
 tests/qemu-iotests/301 |  90 
 tests/qemu-iotests/301.out |  30 ++
 tests/qemu-iotests/group   |   3 +
 5 files changed, 429 insertions(+)
 create mode 100755 tests/qemu-iotests/300
 create mode 100644 tests/qemu-iotests/300.out
 create mode 100755 tests/qemu-iotests/301
 create mode 100644 tests/qemu-iotests/301.out

diff --git a/tests/qemu-iotests/300 b/tests/qemu-iotests/300
new file mode 100755
index 00..aa1a77690f
--- /dev/null
+++ b/tests/qemu-iotests/300
@@ -0,0 +1,207 @@
+#!/usr/bin/env bash
+#
+# Test encryption key management with luks
+# Based on 134
+#
+# Copyright (C) 2019 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+#
+
+# creator
+owner=mlevi...@redhat.com
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+status=1   # failure is the default!
+
+_cleanup()
+{
+   _cleanup_test_img
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+_supported_fmt qcow2 luks
+_supported_proto file #TODO
+
+QEMU_IO_OPTIONS=$QEMU_IO_OPTIONS_NO_FMT
+
+if [ "$IMGFMT" = "qcow2" ] ; then
+   PR="encrypt."
+   EXTRA_IMG_ARGS="-o encrypt.format=luks"
+fi
+
+
+# secrets: you are supposed to see the password as ***, see :-)
+S0="--object secret,id=sec0,data=hunter0"
+S1="--object secret,id=sec1,data=hunter1"
+S2="--object secret,id=sec2,data=hunter2"
+S3="--object secret,id=sec3,data=hunter3"
+S4="--object secret,id=sec4,data=hunter4"
+SECRETS="$S0 $S1 $S2 $S3 $S4"
+
+# image with given secret
+IMGS0="--image-opts 
driver=$IMGFMT,file.filename=$TEST_IMG,${PR}key-secret=sec0"
+IMGS1="--image-opts 
driver=$IMGFMT,file.filename=$TEST_IMG,${PR}key-secret=sec1"
+IMGS2="--image-opts 
driver=$IMGFMT,file.filename=$TEST_IMG,${PR}key-secret=sec2"
+IMGS3="--image-opts 
driver=$IMGFMT,file.filename=$TEST_IMG,${PR}key-secret=sec3"
+IMGS4="--image-opts 
driver=$IMGFMT,file.filename=$TEST_IMG,${PR}key-secret=sec4"
+
+
+echo "== creating a test image =="
+_make_test_img $S0 $EXTRA_IMG_ARGS -o ${PR}key-secret=sec0,${PR}iter-time=10 
32M
+
+echo
+echo "== test that key 0 opens the image =="
+$QEMU_IO $S0 -c "read 0 4096" $IMGS0 | _filter_qemu_io | _filter_testdir
+
+echo
+echo "== adding a password to slot 4 =="
+$QEMU_IMG amend $SECRETS $IMGS0 -o 
${PR}state=active,${PR}new-secret=sec4,${PR}iter-time=10,${PR}keyslot=4
+echo "== adding a password to slot 1 =="
+$QEMU_IMG amend $SECRETS $IMGS0 -o 
${PR}state=active,${PR}new-secret=sec1,${PR}iter-time=10
+echo "== adding a password to slot 3 =="
+$QEMU_IMG amend $SECRETS $IMGS1 -o 
${PR}state=active,${PR}new-secret=sec3,${PR}iter-time=10,${PR}keyslot=3
+
+echo "== adding a password to slot 2 =="
+$QEMU_IMG amend $SECRETS $IMGS3 -o 
${PR}state=active,${PR}new-secret=sec2,${PR}iter-time=10
+
+
+echo "== erase slot 4 =="
+$QEMU_IMG amend $SECRETS $IMGS1 -o ${PR}state=inactive,${PR}keyslot=4 | 
_filter_img_create
+
+
+echo
+echo "== all secrets should work =="
+for IMG in "$IMGS0" "$IMGS1" "$IMGS2" "$IMGS3"; do
+   $QEMU_IO $SECRETS -c "read 0 4096" $IMG | _filter_qemu_io | 
_filter_testdir
+done
+
+echo
+echo "== erase slot 0 and try it =="
+$QEMU_IMG amend $SECRETS $IMGS1 -o ${PR}state=inactive,${PR}old-secret=sec0 | 
_filter_img_create
+$QEMU_IO $SECRETS -c "read 0 4096" $IMGS0 | _filter_qemu_io | _filter_testdir
+
+echo
+echo "== erase slot 2 and try it =="
+$QEMU_IMG amend $SECRETS $IMGS1 -o ${PR}state=inactive,${PR}keyslot=2 | 
_filter_img_create
+$QEMU_IO $SECRETS -c "read 0 4096" $IMGS2 | _filter_qemu_io | _filter_testdir
+
+
+# at this point slots 1 and 3 should be active
+
+echo
+echo "== filling  4 slots with secret 2 =="
+for i in $(seq 0 3) ; do
+   $QEMU_IMG amend $SECRETS $IMGS3 -o 
${PR}state=active,${PR}new-secret=sec2,${PR}iter-time=10
+done
+
+echo
+echo "== adding secret 0 =="
+   $QEMU_IMG amend $SECRETS $IMGS3 -o 
${PR}state=active,${PR}new-secret=sec0,${PR}iter-time=10
+
+echo
+echo "== adding secret 3 (last slot) =="
+   $QEMU_IMG amend $SECRETS $IMGS3 -o 
${PR}state=active,${PR}new-secret=sec3,${PR}iter-time=10
+
+echo
+echo "== t

[PATCH v2 08/14] block/qcow2: extend qemu-img amend interface with crypto options

[Maxim Levitsky]

Now that we have all the infrastructure in place,
wire it in the qcow2 driver and expose this to the user.

Signed-off-by: Maxim Levitsky 
---
 block/qcow2.c  | 80 --
 tests/qemu-iotests/082.out | 45 +
 2 files changed, 114 insertions(+), 11 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 81e7895e7c..10b22544f2 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -176,6 +176,19 @@ static ssize_t qcow2_crypto_hdr_write_func(QCryptoBlock 
*block, size_t offset,
 return ret;
 }
 
+static QDict*
+qcow2_extract_crypto_opts(QemuOpts *opts, const char *fmt, Error **errp)
+{
+QDict *cryptoopts_qdict;
+QDict *opts_qdict;
+
+/* Extract "encrypt." options into a qdict */
+opts_qdict = qemu_opts_to_qdict(opts, NULL);
+qdict_extract_subqdict(opts_qdict, &cryptoopts_qdict, "encrypt.");
+qobject_unref(opts_qdict);
+qdict_put_str(cryptoopts_qdict, "format", "luks");
+return cryptoopts_qdict;
+}
 
 /* 
  * read qcow2 extension and fill bs
@@ -4615,20 +4628,18 @@ static ssize_t 
qcow2_measure_crypto_hdr_write_func(QCryptoBlock *block,
 static bool qcow2_measure_luks_headerlen(QemuOpts *opts, size_t *len,
  Error **errp)
 {
-QDict *opts_qdict;
-QDict *cryptoopts_qdict;
 QCryptoBlockCreateOptions *cryptoopts;
+QDict *crypto_opts_dict;
 QCryptoBlock *crypto;
 
-/* Extract "encrypt." options into a qdict */
-opts_qdict = qemu_opts_to_qdict(opts, NULL);
-qdict_extract_subqdict(opts_qdict, &cryptoopts_qdict, "encrypt.");
-qobject_unref(opts_qdict);
+crypto_opts_dict = qcow2_extract_crypto_opts(opts, "luks", errp);
+if (!crypto_opts_dict) {
+return false;
+}
+
+cryptoopts = block_crypto_create_opts_init(crypto_opts_dict, errp);
+qobject_unref(crypto_opts_dict);
 
-/* Build QCryptoBlockCreateOptions object from qdict */
-qdict_put_str(cryptoopts_qdict, "format", "luks");
-cryptoopts = block_crypto_create_opts_init(cryptoopts_qdict, errp);
-qobject_unref(cryptoopts_qdict);
 if (!cryptoopts) {
 return false;
 }
@@ -5067,6 +5078,7 @@ typedef enum Qcow2AmendOperation {
 QCOW2_NO_OPERATION = 0,
 
 QCOW2_UPGRADING,
+QCOW2_UPDATING_ENCRYPTION,
 QCOW2_CHANGING_REFCOUNT_ORDER,
 QCOW2_DOWNGRADING,
 } Qcow2AmendOperation;
@@ -5148,6 +5160,7 @@ static int qcow2_amend_options(BlockDriverState *bs, 
QemuOpts *opts,
 int ret;
 QemuOptDesc *desc = opts->list->desc;
 Qcow2AmendHelperCBInfo helper_cb_info;
+bool encryption_update = false;
 
 while (desc && desc->name) {
 if (!qemu_opt_find(opts, desc->name)) {
@@ -5174,6 +5187,18 @@ static int qcow2_amend_options(BlockDriverState *bs, 
QemuOpts *opts,
 backing_file = qemu_opt_get(opts, BLOCK_OPT_BACKING_FILE);
 } else if (!strcmp(desc->name, BLOCK_OPT_BACKING_FMT)) {
 backing_format = qemu_opt_get(opts, BLOCK_OPT_BACKING_FMT);
+} else if (g_str_has_prefix(desc->name, "encrypt.")) {
+if (!s->crypto) {
+error_setg(errp,
+   "Can't amend encryption options - encryption not 
present");
+return -EINVAL;
+}
+if (s->crypt_method_header != QCOW_CRYPT_LUKS) {
+error_setg(errp,
+   "Only LUKS encryption options can be amended");
+return -ENOTSUP;
+}
+encryption_update = true;
 } else if (!strcmp(desc->name, BLOCK_OPT_LAZY_REFCOUNTS)) {
 lazy_refcounts = qemu_opt_get_bool(opts, BLOCK_OPT_LAZY_REFCOUNTS,
lazy_refcounts);
@@ -5216,7 +5241,8 @@ static int qcow2_amend_options(BlockDriverState *bs, 
QemuOpts *opts,
 .original_status_cb = status_cb,
 .original_cb_opaque = cb_opaque,
 .total_operations = (new_version != old_version)
-  + (s->refcount_bits != refcount_bits)
+  + (s->refcount_bits != refcount_bits) +
+(encryption_update == true)
 };
 
 /* Upgrade first (some features may require compat=1.1) */
@@ -5229,6 +5255,33 @@ static int qcow2_amend_options(BlockDriverState *bs, 
QemuOpts *opts,
 }
 }
 
+if (encryption_update) {
+QDict *amend_opts_dict;
+QCryptoBlockAmendOptions *amend_opts;
+
+helper_cb_info.current_operation = QCOW2_UPDATING_ENCRYPTION;
+amend_opts_dict = qcow2_extract_crypto_opts(opts, "luks", errp);
+if (!amend_opts_dict) {
+return -EINVAL;
+}
+amend_opts = block_crypto_amend_opts_init(amend_opts_dict, errp);
+qobject_unref(amend_opts_dict);
+if (!amend_opts) {
+return -EINVAL;
+}
+ret = qcrypto_block_amend_options(s->crypto,
+  qcow2_crypto_hdr_read_fu

[PATCH v2 02/14] qcrypto/luks: implement encryption key management

[Maxim Levitsky]

Next few patches will expose that functionality
to the user.

Signed-off-by: Maxim Levitsky 
---
 crypto/block-luks.c | 398 +++-
 qapi/crypto.json|  61 ++-
 2 files changed, 455 insertions(+), 4 deletions(-)

diff --git a/crypto/block-luks.c b/crypto/block-luks.c
index 4861db810c..b11ee08c6d 100644
--- a/crypto/block-luks.c
+++ b/crypto/block-luks.c
@@ -32,6 +32,7 @@
 #include "qemu/uuid.h"
 
 #include "qemu/coroutine.h"
+#include "qemu/bitmap.h"
 
 /*
  * Reference for the LUKS format implemented here is
@@ -70,6 +71,9 @@ typedef struct QCryptoBlockLUKSKeySlot 
QCryptoBlockLUKSKeySlot;
 
 #define QCRYPTO_BLOCK_LUKS_SECTOR_SIZE 512LL
 
+#define QCRYPTO_BLOCK_LUKS_DEFAULT_ITER_TIME_MS 2000
+#define QCRYPTO_BLOCK_LUKS_ERASE_ITERATIONS 40
+
 static const char qcrypto_block_luks_magic[QCRYPTO_BLOCK_LUKS_MAGIC_LEN] = {
 'L', 'U', 'K', 'S', 0xBA, 0xBE
 };
@@ -219,6 +223,9 @@ struct QCryptoBlockLUKS {
 
 /* Hash algorithm used in pbkdf2 function */
 QCryptoHashAlgorithm hash_alg;
+
+/* Name of the secret that was used to open the image */
+char *secret;
 };
 
 
@@ -1069,6 +1076,108 @@ qcrypto_block_luks_find_key(QCryptoBlock *block,
 return -1;
 }
 
+/*
+ * Returns true if a slot i is marked as active
+ * (contains encrypted copy of the master key)
+ */
+static bool
+qcrypto_block_luks_slot_active(const QCryptoBlockLUKS *luks,
+   unsigned int slot_idx)
+{
+uint32_t val = luks->header.key_slots[slot_idx].active;
+return val ==  QCRYPTO_BLOCK_LUKS_KEY_SLOT_ENABLED;
+}
+
+/*
+ * Returns the number of slots that are marked as active
+ * (slots that contain encrypted copy of the master key)
+ */
+static unsigned int
+qcrypto_block_luks_count_active_slots(const QCryptoBlockLUKS *luks)
+{
+size_t i = 0;
+unsigned int ret = 0;
+
+for (i = 0; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS; i++) {
+if (qcrypto_block_luks_slot_active(luks, i)) {
+ret++;
+}
+}
+return ret;
+}
+
+/*
+ * Finds first key slot which is not active
+ * Returns the key slot index, or -1 if it doesn't exist
+ */
+static int
+qcrypto_block_luks_find_free_keyslot(const QCryptoBlockLUKS *luks)
+{
+size_t i;
+
+for (i = 0; i < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS; i++) {
+if (!qcrypto_block_luks_slot_active(luks, i)) {
+return i;
+}
+}
+return -1;
+}
+
+/*
+ * Erases an keyslot given its index
+ * Returns:
+ *0 if the keyslot was erased successfully
+ *   -1 if a error occurred while erasing the keyslot
+ *
+ */
+static int
+qcrypto_block_luks_erase_key(QCryptoBlock *block,
+ unsigned int slot_idx,
+ QCryptoBlockWriteFunc writefunc,
+ void *opaque,
+ Error **errp)
+{
+QCryptoBlockLUKS *luks = block->opaque;
+QCryptoBlockLUKSKeySlot *slot = &luks->header.key_slots[slot_idx];
+g_autofree uint8_t *garbagesplitkey = NULL;
+size_t splitkeylen = luks->header.master_key_len * slot->stripes;
+size_t i;
+
+assert(slot_idx < QCRYPTO_BLOCK_LUKS_NUM_KEY_SLOTS);
+assert(splitkeylen > 0);
+garbagesplitkey = g_new0(uint8_t, splitkeylen);
+
+/* Reset the key slot header */
+memset(slot->salt, 0, QCRYPTO_BLOCK_LUKS_SALT_LEN);
+slot->iterations = 0;
+slot->active = QCRYPTO_BLOCK_LUKS_KEY_SLOT_DISABLED;
+
+qcrypto_block_luks_store_header(block,  writefunc, opaque, errp);
+/*
+ * Now try to erase the key material, even if the header
+ * update failed
+ */
+for (i = 0; i < QCRYPTO_BLOCK_LUKS_ERASE_ITERATIONS; i++) {
+if (qcrypto_random_bytes(garbagesplitkey, splitkeylen, errp) < 0) {
+/*
+ * If we failed to get the random data, still write
+ * at least zeros to the key slot at least once
+ */
+if (i > 0) {
+return -1;
+}
+}
+if (writefunc(block,
+  slot->key_offset_sector * QCRYPTO_BLOCK_LUKS_SECTOR_SIZE,
+  garbagesplitkey,
+  splitkeylen,
+  opaque,
+  errp) != splitkeylen) {
+return -1;
+}
+}
+return 0;
+}
 
 static int
 qcrypto_block_luks_open(QCryptoBlock *block,
@@ -1099,6 +1208,7 @@ qcrypto_block_luks_open(QCryptoBlock *block,
 
 luks = g_new0(QCryptoBlockLUKS, 1);
 block->opaque = luks;
+luks->secret = g_strdup(options->u.luks.key_secret);
 
 if (qcrypto_block_luks_load_header(block, readfunc, opaque, errp) < 0) {
 goto fail;
@@ -1164,6 +1274,7 @@ qcrypto_block_luks_open(QCryptoBlock *block,
  fail:
 qcrypto_block_free_cipher(block);
 qcrypto_ivgen_free(block->ivgen);
+g_free(luks->secret);
 g_free(luks);
 return -1;
 }
@@ -1204,7 +1315,7 @@ qcrypto_block_luks_create(QCryptoBlock *block,
 
 memcpy(&luks_opts, &optio

[PATCH v2 07/14] block/crypto: implement the encryption key management

[Maxim Levitsky]

This implements the encryption key management using the generic code in
qcrypto layer and exposes it to the user via qemu-img

This code adds another 'write_func' because the initialization
write_func works directly on the underlying file, and amend
works on instance of luks device.

This commit also adds a 'hack/workaround' I and Kevin Wolf (thanks)
made to make the driver both support write sharing (to avoid breaking the 
users),
and be safe against concurrent  metadata update (the keyslots)

Eventually the write sharing for luks driver will be deprecated
and removed together with this hack.

The hack is that we ask (as a format driver) for BLK_PERM_CONSISTENT_READ
and then when we want to update the keys, we unshare that permission.
So if someone else has the image open, even readonly, encryption
key update will fail gracefully.

Also thanks to Daniel Berrange for the idea of
unsharing read, rather that write permission which allows
to avoid cases when the other user had opened the image read-only.

Signed-off-by: Maxim Levitsky 
---
 block/crypto.c | 127 +++--
 block/crypto.h |  44 +++--
 2 files changed, 163 insertions(+), 8 deletions(-)

diff --git a/block/crypto.c b/block/crypto.c
index 0b37dae564..727a3fde58 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -36,6 +36,7 @@ typedef struct BlockCrypto BlockCrypto;
 
 struct BlockCrypto {
 QCryptoBlock *block;
+bool updating_keys;
 };
 
 
@@ -70,6 +71,24 @@ static ssize_t block_crypto_read_func(QCryptoBlock *block,
 return ret;
 }
 
+static ssize_t block_crypto_write_func(QCryptoBlock *block,
+   size_t offset,
+   const uint8_t *buf,
+   size_t buflen,
+   void *opaque,
+   Error **errp)
+{
+BlockDriverState *bs = opaque;
+ssize_t ret;
+
+ret = bdrv_pwrite(bs->file, offset, buf, buflen);
+if (ret < 0) {
+error_setg_errno(errp, -ret, "Could not write encryption header");
+return ret;
+}
+return ret;
+}
+
 
 struct BlockCryptoCreateData {
 BlockBackend *blk;
@@ -148,6 +167,19 @@ static QemuOptsList block_crypto_create_opts_luks = {
 };
 
 
+static QemuOptsList block_crypto_amend_opts_luks = {
+.name = "crypto",
+.head = QTAILQ_HEAD_INITIALIZER(block_crypto_create_opts_luks.head),
+.desc = {
+BLOCK_CRYPTO_OPT_DEF_LUKS_STATE(""),
+BLOCK_CRYPTO_OPT_DEF_LUKS_KEYSLOT(""),
+BLOCK_CRYPTO_OPT_DEF_LUKS_OLD_SECRET(""),
+BLOCK_CRYPTO_OPT_DEF_LUKS_NEW_SECRET(""),
+BLOCK_CRYPTO_OPT_DEF_LUKS_ITER_TIME(""),
+{ /* end of list */ }
+},
+};
+
 QCryptoBlockOpenOptions *
 block_crypto_open_opts_init(QDict *opts, Error **errp)
 {
@@ -661,6 +693,95 @@ block_crypto_get_specific_info_luks(BlockDriverState *bs, 
Error **errp)
 return spec_info;
 }
 
+static int
+block_crypto_amend_options_luks(BlockDriverState *bs,
+   QemuOpts *opts,
+   BlockDriverAmendStatusCB *status_cb,
+   void *cb_opaque,
+   bool force,
+   Error **errp)
+{
+BlockCrypto *crypto = bs->opaque;
+QDict *cryptoopts = NULL;
+QCryptoBlockAmendOptions *amend_options = NULL;
+int ret;
+
+assert(crypto);
+assert(crypto->block);
+crypto->updating_keys = true;
+
+ret = bdrv_child_refresh_perms(bs, bs->file, errp);
+if (ret < 0) {
+goto cleanup;
+}
+
+cryptoopts = qemu_opts_to_qdict(opts, NULL);
+qdict_put_str(cryptoopts, "format", "luks");
+amend_options = block_crypto_amend_opts_init(cryptoopts, errp);
+if (!amend_options) {
+ret = -EINVAL;
+goto cleanup;
+}
+
+ret = qcrypto_block_amend_options(crypto->block,
+  block_crypto_read_func,
+  block_crypto_write_func,
+  bs,
+  amend_options,
+  force,
+  errp);
+cleanup:
+crypto->updating_keys = false;
+bdrv_child_refresh_perms(bs, bs->file, errp);
+qapi_free_QCryptoBlockAmendOptions(amend_options);
+qobject_unref(cryptoopts);
+return ret;
+}
+
+
+static void
+block_crypto_child_perms(BlockDriverState *bs, BdrvChild *c,
+ const BdrvChildRole *role,
+ BlockReopenQueue *reopen_queue,
+ uint64_t perm, uint64_t shared,
+ uint64_t *nperm, uint64_t *nshared)
+{
+
+BlockCrypto *crypto = bs->opaque;
+
+bdrv_filter_default_perms(bs, c, role, reopen_queue,
+perm, shared, nperm, nshared);
+/*
+ * Ask for consistent read permission so that if
+ * someone else trie

[PATCH v2 03/14] block/amend: add 'force' option

[Maxim Levitsky]

'force' option will be used for some unsafe amend operations.

This includes things like erasing last keyslot in luks based formats
(which destroys the data, unless the master key is backed up
by external means), but that _might_ be desired result.

Signed-off-by: Maxim Levitsky 
Reviewed-by: Daniel P. Berrangé 
---
 block.c   | 4 +++-
 block/qcow2.c | 1 +
 docs/tools/qemu-img.rst   | 5 -
 include/block/block.h | 1 +
 include/block/block_int.h | 1 +
 qemu-img-cmds.hx  | 4 ++--
 qemu-img.c| 8 +++-
 7 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/block.c b/block.c
index 1bdb9c679d..3b6f51aa3e 100644
--- a/block.c
+++ b/block.c
@@ -6320,6 +6320,7 @@ void bdrv_remove_aio_context_notifier(BlockDriverState 
*bs,
 
 int bdrv_amend_options(BlockDriverState *bs, QemuOpts *opts,
BlockDriverAmendStatusCB *status_cb, void *cb_opaque,
+   bool force,
Error **errp)
 {
 if (!bs->drv) {
@@ -6331,7 +6332,8 @@ int bdrv_amend_options(BlockDriverState *bs, QemuOpts 
*opts,
bs->drv->format_name);
 return -ENOTSUP;
 }
-return bs->drv->bdrv_amend_options(bs, opts, status_cb, cb_opaque, errp);
+return bs->drv->bdrv_amend_options(bs, opts, status_cb,
+   cb_opaque, force, errp);
 }
 
 /*
diff --git a/block/qcow2.c b/block/qcow2.c
index 3c754f616b..b55e5b7c1f 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -5145,6 +5145,7 @@ static void qcow2_amend_helper_cb(BlockDriverState *bs,
 static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
BlockDriverAmendStatusCB *status_cb,
void *cb_opaque,
+   bool force,
Error **errp)
 {
 BDRVQcow2State *s = bs->opaque;
diff --git a/docs/tools/qemu-img.rst b/docs/tools/qemu-img.rst
index 0080f83a76..fc2dca6649 100644
--- a/docs/tools/qemu-img.rst
+++ b/docs/tools/qemu-img.rst
@@ -249,11 +249,14 @@ Command description:
 
 .. program:: qemu-img-commands
 
-.. option:: amend [--object OBJECTDEF] [--image-opts] [-p] [-q] [-f FMT] [-t 
CACHE] -o OPTIONS FILENAME
+.. option:: amend [--object OBJECTDEF] [--image-opts] [-p] [-q] [-f FMT] [-t 
CACHE] [--force] -o OPTIONS FILENAME
 
   Amends the image format specific *OPTIONS* for the image file
   *FILENAME*. Not all file formats support this operation.
 
+  --force allows some unsafe operations. Currently for -f luks, it allows to
+  erase last encryption key, and to overwrite an active encryption key.
+
 .. option:: bench [-c COUNT] [-d DEPTH] [-f FMT] 
[--flush-interval=FLUSH_INTERVAL] [-i AIO] [-n] [--no-drain] [-o OFFSET] 
[--pattern=PATTERN] [-q] [-s BUFFER_SIZE] [-S STEP_SIZE] [-t CACHE] [-w] [-U] 
FILENAME
 
   Run a simple sequential I/O benchmark on the specified image. If ``-w`` is
diff --git a/include/block/block.h b/include/block/block.h
index cd6b5b95aa..dda18a3fa3 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -389,6 +389,7 @@ typedef void BlockDriverAmendStatusCB(BlockDriverState *bs, 
int64_t offset,
   int64_t total_work_size, void *opaque);
 int bdrv_amend_options(BlockDriverState *bs_new, QemuOpts *opts,
BlockDriverAmendStatusCB *status_cb, void *cb_opaque,
+   bool force,
Error **errp);
 
 /* check if a named node can be replaced when doing drive-mirror */
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 6f9fd5e20e..24d00fbf48 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -426,6 +426,7 @@ struct BlockDriver {
 int (*bdrv_amend_options)(BlockDriverState *bs, QemuOpts *opts,
   BlockDriverAmendStatusCB *status_cb,
   void *cb_opaque,
+  bool force,
   Error **errp);
 
 void (*bdrv_debug_event)(BlockDriverState *bs, BlkdebugEvent event);
diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
index c9c54de1df..9920f1f9d4 100644
--- a/qemu-img-cmds.hx
+++ b/qemu-img-cmds.hx
@@ -10,9 +10,9 @@ HXCOMM When amending the rST sections, please remember to 
copy the usage
 HXCOMM over to the per-command sections in qemu-img.texi.
 
 DEF("amend", img_amend,
-"amend [--object objectdef] [--image-opts] [-p] [-q] [-f fmt] [-t cache] 
-o options filename")
+"amend [--object objectdef] [--image-opts] [-p] [-q] [-f fmt] [-t cache] 
[--force] -o options filename")
 SRST
-.. option:: amend [--object OBJECTDEF] [--image-opts] [-p] [-q] [-f FMT] [-t 
CACHE] -o OPTIONS FILENAME
+.. option:: amend [--object OBJECTDEF] [--image-opts] [-p] [-q] [-f FMT] [-t 
CACHE] [--force] -o OPTIONS FILENAME
 ERST
 
 DEF("bench", img_bench,
diff --git a/qemu-img.c b/qemu-img.c
index 804630a368..551388676f 100644
--- a/qem

[PATCH v2 11/14] block/core: add generic infrastructure for x-blockdev-amend qmp command

[Maxim Levitsky]

blockdev-amend will be used similiar to blockdev-create
to allow on the fly changes of the structure of the format based block devices.

Current plan is to first support encryption keyslot management for luks
based formats (raw and embedded in qcow2)

Signed-off-by: Maxim Levitsky 
---
 block/Makefile.objs   |   2 +-
 block/amend.c | 108 ++
 include/block/block_int.h |  21 +---
 qapi/block-core.json  |  42 +++
 qapi/job.json |   4 +-
 5 files changed, 169 insertions(+), 8 deletions(-)
 create mode 100644 block/amend.c

diff --git a/block/Makefile.objs b/block/Makefile.objs
index 3bcb35c81d..5f0e60e7b4 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -19,7 +19,7 @@ block-obj-$(CONFIG_WIN32) += file-win32.o win32-aio.o
 block-obj-$(CONFIG_POSIX) += file-posix.o
 block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
 block-obj-$(CONFIG_LINUX_IO_URING) += io_uring.o
-block-obj-y += null.o mirror.o commit.o io.o create.o
+block-obj-y += null.o mirror.o commit.o io.o create.o amend.o
 block-obj-y += throttle-groups.o
 block-obj-$(CONFIG_LINUX) += nvme.o
 
diff --git a/block/amend.c b/block/amend.c
new file mode 100644
index 00..2db7b1eafc
--- /dev/null
+++ b/block/amend.c
@@ -0,0 +1,108 @@
+/*
+ * Block layer code related to image options amend
+ *
+ * Copyright (c) 2018 Kevin Wolf 
+ * Copyright (c) 2019 Maxim Levitsky 
+ *
+ * Heavily based on create.c
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu/osdep.h"
+#include "block/block_int.h"
+#include "qemu/job.h"
+#include "qemu/main-loop.h"
+#include "qapi/qapi-commands-block-core.h"
+#include "qapi/qapi-visit-block-core.h"
+#include "qapi/clone-visitor.h"
+#include "qapi/error.h"
+
+typedef struct BlockdevAmendJob {
+Job common;
+BlockdevAmendOptions *opts;
+BlockDriverState *bs;
+bool force;
+} BlockdevAmendJob;
+
+static int coroutine_fn blockdev_amend_run(Job *job, Error **errp)
+{
+BlockdevAmendJob *s = container_of(job, BlockdevAmendJob, common);
+int ret;
+
+job_progress_set_remaining(&s->common, 1);
+ret = s->bs->drv->bdrv_co_amend(s->bs, s->opts, s->force, errp);
+job_progress_update(&s->common, 1);
+qapi_free_BlockdevAmendOptions(s->opts);
+return ret;
+}
+
+static const JobDriver blockdev_amend_job_driver = {
+.instance_size = sizeof(BlockdevAmendJob),
+.job_type  = JOB_TYPE_AMEND,
+.run   = blockdev_amend_run,
+};
+
+void qmp_x_blockdev_amend(const char *job_id,
+  const char *node_name,
+  BlockdevAmendOptions *options,
+  bool has_force,
+  bool force,
+  Error **errp)
+{
+BlockdevAmendJob *s;
+const char *fmt = BlockdevDriver_str(options->driver);
+BlockDriver *drv = bdrv_find_format(fmt);
+BlockDriverState *bs = bdrv_find_node(node_name);
+
+/*
+ * If the driver is in the schema, we know that it exists. But it may not
+ * be whitelisted.
+ */
+assert(drv);
+if (bdrv_uses_whitelist() && !bdrv_is_whitelisted(drv, false)) {
+error_setg(errp, "Driver is not whitelisted");
+return;
+}
+
+if (bs->drv != drv) {
+error_setg(errp,
+   "x-blockdev-amend doesn't support changing the block 
driver");
+return;
+}
+
+/* Error out if the driver doesn't support .bdrv_co_amend */
+if (!drv->bdrv_co_amend) {
+error_setg(errp, "Driver does not support x-blockdev-amend");
+return;
+}
+
+/* Create the block job */
+s = job_create(job_id, &blockdev_amend_job_driver, NULL,
+   bdrv_get_aio_context(bs), JOB_DEFAULT | JOB_MANUAL_DISMISS,
+   NULL, NULL, errp);
+if (!s) {
+return;
+}
+
+s->bs = bs,
+s->opts = QAPI_CLONE(BlockdevAmendOptions, options),
+   

[PATCH v2 05/14] block/amend: refactor qcow2 amend options

[Maxim Levitsky]

Some qcow2 create options can't be used for amend.
Remove them from the qcow2 create options and add generic logic to detect
such options in qemu-img

Signed-off-by: Maxim Levitsky 
---
 block/qcow2.c  | 108 ++---
 qemu-img.c |  18 +++-
 tests/qemu-iotests/049.out | 102 ++--
 tests/qemu-iotests/061.out |  12 ++-
 tests/qemu-iotests/079.out |  18 ++--
 tests/qemu-iotests/082.out | 149 
 tests/qemu-iotests/085.out |  38 
 tests/qemu-iotests/087.out |   6 +-
 tests/qemu-iotests/115.out |   2 +-
 tests/qemu-iotests/121.out |   4 +-
 tests/qemu-iotests/125.out | 192 ++---
 tests/qemu-iotests/134.out |   2 +-
 tests/qemu-iotests/144.out |   4 +-
 tests/qemu-iotests/158.out |   4 +-
 tests/qemu-iotests/182.out |   2 +-
 tests/qemu-iotests/185.out |   8 +-
 tests/qemu-iotests/188.out |   2 +-
 tests/qemu-iotests/189.out |   4 +-
 tests/qemu-iotests/198.out |   4 +-
 tests/qemu-iotests/243.out |  16 ++--
 tests/qemu-iotests/250.out |   2 +-
 tests/qemu-iotests/255.out |   8 +-
 tests/qemu-iotests/263.out |   4 +-
 tests/qemu-iotests/280.out |   2 +-
 24 files changed, 283 insertions(+), 428 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 9574085772..81e7895e7c 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2946,17 +2946,6 @@ static int qcow2_change_backing_file(BlockDriverState 
*bs,
 return qcow2_update_header(bs);
 }
 
-static int qcow2_crypt_method_from_format(const char *encryptfmt)
-{
-if (g_str_equal(encryptfmt, "luks")) {
-return QCOW_CRYPT_LUKS;
-} else if (g_str_equal(encryptfmt, "aes")) {
-return QCOW_CRYPT_AES;
-} else {
-return -EINVAL;
-}
-}
-
 static int qcow2_set_up_encryption(BlockDriverState *bs,
QCryptoBlockCreateOptions *cryptoopts,
Error **errp)
@@ -5155,9 +5144,6 @@ static int qcow2_amend_options(BlockDriverState *bs, 
QemuOpts *opts,
 bool lazy_refcounts = s->use_lazy_refcounts;
 bool data_file_raw = data_file_is_raw(bs);
 const char *compat = NULL;
-uint64_t cluster_size = s->cluster_size;
-bool encrypt;
-int encformat;
 int refcount_bits = s->refcount_bits;
 int ret;
 QemuOptDesc *desc = opts->list->desc;
@@ -5182,44 +5168,12 @@ static int qcow2_amend_options(BlockDriverState *bs, 
QemuOpts *opts,
 error_setg(errp, "Unknown compatibility level %s", compat);
 return -EINVAL;
 }
-} else if (!strcmp(desc->name, BLOCK_OPT_PREALLOC)) {
-error_setg(errp, "Cannot change preallocation mode");
-return -ENOTSUP;
 } else if (!strcmp(desc->name, BLOCK_OPT_SIZE)) {
 new_size = qemu_opt_get_size(opts, BLOCK_OPT_SIZE, 0);
 } else if (!strcmp(desc->name, BLOCK_OPT_BACKING_FILE)) {
 backing_file = qemu_opt_get(opts, BLOCK_OPT_BACKING_FILE);
 } else if (!strcmp(desc->name, BLOCK_OPT_BACKING_FMT)) {
 backing_format = qemu_opt_get(opts, BLOCK_OPT_BACKING_FMT);
-} else if (!strcmp(desc->name, BLOCK_OPT_ENCRYPT)) {
-encrypt = qemu_opt_get_bool(opts, BLOCK_OPT_ENCRYPT,
-!!s->crypto);
-
-if (encrypt != !!s->crypto) {
-error_setg(errp,
-   "Changing the encryption flag is not supported");
-return -ENOTSUP;
-}
-} else if (!strcmp(desc->name, BLOCK_OPT_ENCRYPT_FORMAT)) {
-encformat = qcow2_crypt_method_from_format(
-qemu_opt_get(opts, BLOCK_OPT_ENCRYPT_FORMAT));
-
-if (encformat != s->crypt_method_header) {
-error_setg(errp,
-   "Changing the encryption format is not supported");
-return -ENOTSUP;
-}
-} else if (g_str_has_prefix(desc->name, "encrypt.")) {
-error_setg(errp,
-   "Changing the encryption parameters is not supported");
-return -ENOTSUP;
-} else if (!strcmp(desc->name, BLOCK_OPT_CLUSTER_SIZE)) {
-cluster_size = qemu_opt_get_size(opts, BLOCK_OPT_CLUSTER_SIZE,
- cluster_size);
-if (cluster_size != s->cluster_size) {
-error_setg(errp, "Changing the cluster size is not supported");
-return -ENOTSUP;
-}
 } else if (!strcmp(desc->name, BLOCK_OPT_LAZY_REFCOUNTS)) {
 lazy_refcounts = qemu_opt_get_bool(opts, BLOCK_OPT_LAZY_REFCOUNTS,
lazy_refcounts);
@@ -5472,37 +5426,6 @@ void qcow2_signal_corruption(BlockDriverState *bs, bool 
fatal, int64_t offset,
 .help = "The external data file must stay valid "   \
 "as a raw image"\
 },

[PATCH v2 14/14] iotests: add tests for blockdev-amend

[Maxim Levitsky]

This commit adds two tests that cover the
new blockdev-amend functionality of luks and qcow2 driver

Signed-off-by: Maxim Levitsky 
---
 tests/qemu-iotests/302 | 278 +
 tests/qemu-iotests/302.out |  40 ++
 tests/qemu-iotests/303 | 233 +++
 tests/qemu-iotests/303.out |  33 +
 tests/qemu-iotests/group   |   3 +
 5 files changed, 587 insertions(+)
 create mode 100755 tests/qemu-iotests/302
 create mode 100644 tests/qemu-iotests/302.out
 create mode 100755 tests/qemu-iotests/303
 create mode 100644 tests/qemu-iotests/303.out

diff --git a/tests/qemu-iotests/302 b/tests/qemu-iotests/302
new file mode 100755
index 00..a6b1155c33
--- /dev/null
+++ b/tests/qemu-iotests/302
@@ -0,0 +1,278 @@
+#!/usr/bin/env python3
+#
+# Test case QMP's encrypted key management
+#
+# Copyright (C) 2019 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 .
+#
+
+import iotests
+import os
+import time
+import json
+
+test_img = os.path.join(iotests.test_dir, 'test.img')
+
+class Secret:
+def __init__(self, index):
+self._id = "keysec" + str(index)
+# you are not supposed to see the password...
+self._secret = "hunter" + str(index)
+
+def id(self):
+return self._id
+
+def secret(self):
+return self._secret
+
+def to_cmdline_object(self):
+return  [ "secret,id=" + self._id + ",data=" + self._secret]
+
+def to_qmp_object(self):
+return { "qom_type" : "secret", "id": self.id(),
+ "props": { "data": self.secret() } }
+
+
+class EncryptionSetupTestCase(iotests.QMPTestCase):
+
+# test case startup
+def setUp(self):
+# start the VM
+self.vm = iotests.VM()
+self.vm.launch()
+
+# create the secrets and load 'em into the VM
+self.secrets = [ Secret(i) for i in range(0, 6) ]
+for secret in self.secrets:
+result = self.vm.qmp("object-add", **secret.to_qmp_object())
+self.assert_qmp(result, 'return', {})
+
+if iotests.imgfmt == "qcow2":
+self.pfx = "encrypt."
+self.img_opts = [ '-o', "encrypt.format=luks" ]
+else:
+self.pfx = ""
+self.img_opts = []
+
+# test case shutdown
+def tearDown(self):
+# stop the VM
+self.vm.shutdown()
+
+###
+# create the encrypted block device
+def createImg(self, file, secret):
+
+iotests.qemu_img(
+'create',
+'--object', *secret.to_cmdline_object(),
+'-f', iotests.imgfmt,
+'-o', self.pfx + 'key-secret=' + secret.id(),
+'-o', self.pfx + 'iter-time=10',
+*self.img_opts,
+file,
+'1M')
+
+###
+# open an encrypted block device
+def openImageQmp(self, id, file, secret, read_only = False):
+
+encrypt_options = {
+'key-secret' : secret.id()
+}
+
+if iotests.imgfmt == "qcow2":
+encrypt_options = {
+'encrypt': {
+'format':'luks',
+**encrypt_options
+}
+}
+
+result = self.vm.qmp('blockdev-add', **
+{
+'driver': iotests.imgfmt,
+'node-name': id,
+'read-only': read_only,
+
+**encrypt_options,
+
+'file': {
+'driver': 'file',
+'filename': test_img,
+}
+}
+)
+self.assert_qmp(result, 'return', {})
+
+# close the encrypted block device
+def closeImageQmp(self, id):
+result = self.vm.qmp('blockdev-del', **{ 'node-name': id })
+self.assert_qmp(result, 'return', {})
+
+###
+# add a key to an encrypted block device
+def addKeyQmp(self, id, secret, unlock_secret = None,
+  slot = None, force = False):
+
+crypt_options = {
+'state'  : 'active',
+'new-secret' : secret.id(),
+'iter-

[PATCH v2 06/14] block/crypto: rename two functions

[Maxim Levitsky]

rename the write_func to create_write_func, and init_func to create_init_func.
This is preparation for other write_func that will be used to update the 
encryption keys.

No functional changes

Signed-off-by: Maxim Levitsky 
Reviewed-by: Daniel P. Berrangé 
---
 block/crypto.c | 25 -
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/block/crypto.c b/block/crypto.c
index ecf96a7a9b..0b37dae564 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -78,12 +78,12 @@ struct BlockCryptoCreateData {
 };
 
 
-static ssize_t block_crypto_write_func(QCryptoBlock *block,
-   size_t offset,
-   const uint8_t *buf,
-   size_t buflen,
-   void *opaque,
-   Error **errp)
+static ssize_t block_crypto_create_write_func(QCryptoBlock *block,
+  size_t offset,
+  const uint8_t *buf,
+  size_t buflen,
+  void *opaque,
+  Error **errp)
 {
 struct BlockCryptoCreateData *data = opaque;
 ssize_t ret;
@@ -96,11 +96,10 @@ static ssize_t block_crypto_write_func(QCryptoBlock *block,
 return ret;
 }
 
-
-static ssize_t block_crypto_init_func(QCryptoBlock *block,
-  size_t headerlen,
-  void *opaque,
-  Error **errp)
+static ssize_t block_crypto_create_init_func(QCryptoBlock *block,
+ size_t headerlen,
+ void *opaque,
+ Error **errp)
 {
 struct BlockCryptoCreateData *data = opaque;
 
@@ -296,8 +295,8 @@ static int block_crypto_co_create_generic(BlockDriverState 
*bs,
 };
 
 crypto = qcrypto_block_create(opts, NULL,
-  block_crypto_init_func,
-  block_crypto_write_func,
+  block_crypto_create_init_func,
+  block_crypto_create_write_func,
   &data,
   errp);
 
-- 
2.17.2

[PATCH v2 09/14] iotests: filter few more luks specific create options

[Maxim Levitsky]

This allows more tests to be able to have same output on both qcow2 luks 
encrypted images
and raw luks images

Signed-off-by: Maxim Levitsky 
---
 tests/qemu-iotests/087.out   | 6 +++---
 tests/qemu-iotests/134.out   | 2 +-
 tests/qemu-iotests/158.out   | 4 ++--
 tests/qemu-iotests/188.out   | 2 +-
 tests/qemu-iotests/189.out   | 4 ++--
 tests/qemu-iotests/198.out   | 4 ++--
 tests/qemu-iotests/263.out   | 4 ++--
 tests/qemu-iotests/284.out   | 6 +++---
 tests/qemu-iotests/common.filter | 6 --
 9 files changed, 20 insertions(+), 18 deletions(-)

diff --git a/tests/qemu-iotests/087.out b/tests/qemu-iotests/087.out
index f23bffbbf1..d5ff53302e 100644
--- a/tests/qemu-iotests/087.out
+++ b/tests/qemu-iotests/087.out
@@ -34,7 +34,7 @@ QMP_VERSION
 
 === Encrypted image QCow ===
 
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT encryption=on 
encrypt.key-secret=sec0 size=134217728
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT encryption=on size=134217728
 Testing:
 QMP_VERSION
 {"return": {}}
@@ -46,7 +46,7 @@ QMP_VERSION
 
 === Encrypted image LUKS ===
 
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT encrypt.format=luks 
encrypt.key-secret=sec0 size=134217728
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
 Testing:
 QMP_VERSION
 {"return": {}}
@@ -58,7 +58,7 @@ QMP_VERSION
 
 === Missing driver ===
 
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT encryption=on 
encrypt.key-secret=sec0 size=134217728
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT encryption=on size=134217728
 Testing: -S
 QMP_VERSION
 {"return": {}}
diff --git a/tests/qemu-iotests/134.out b/tests/qemu-iotests/134.out
index f2878f5f3a..e4733c0b81 100644
--- a/tests/qemu-iotests/134.out
+++ b/tests/qemu-iotests/134.out
@@ -1,5 +1,5 @@
 QA output created by 134
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT encryption=on 
encrypt.key-secret=sec0 size=134217728
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT encryption=on size=134217728
 
 == reading whole image ==
 read 134217728/134217728 bytes at offset 0
diff --git a/tests/qemu-iotests/158.out b/tests/qemu-iotests/158.out
index fa2294bb85..52ea9a488f 100644
--- a/tests/qemu-iotests/158.out
+++ b/tests/qemu-iotests/158.out
@@ -1,6 +1,6 @@
 QA output created by 158
 == create base ==
-Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT encryption=on 
encrypt.key-secret=sec0 size=134217728
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT encryption=on size=134217728
 
 == writing whole image ==
 wrote 134217728/134217728 bytes at offset 0
@@ -10,7 +10,7 @@ wrote 134217728/134217728 bytes at offset 0
 read 134217728/134217728 bytes at offset 0
 128 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 == create overlay ==
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT encryption=on 
encrypt.key-secret=sec0 size=134217728 backing_file=TEST_DIR/t.IMGFMT.base
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT encryption=on size=134217728 
backing_file=TEST_DIR/t.IMGFMT.base
 
 == writing part of a cluster ==
 wrote 1024/1024 bytes at offset 0
diff --git a/tests/qemu-iotests/188.out b/tests/qemu-iotests/188.out
index 4b9aadd51c..5426861b18 100644
--- a/tests/qemu-iotests/188.out
+++ b/tests/qemu-iotests/188.out
@@ -1,5 +1,5 @@
 QA output created by 188
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT encrypt.format=luks 
encrypt.key-secret=sec0 encrypt.iter-time=10 size=16777216
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=16777216
 
 == reading whole image ==
 read 16777216/16777216 bytes at offset 0
diff --git a/tests/qemu-iotests/189.out b/tests/qemu-iotests/189.out
index e536d95d53..bc213cbe14 100644
--- a/tests/qemu-iotests/189.out
+++ b/tests/qemu-iotests/189.out
@@ -1,6 +1,6 @@
 QA output created by 189
 == create base ==
-Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT encrypt.format=luks 
encrypt.key-secret=sec0 encrypt.iter-time=10 size=16777216
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=16777216
 
 == writing whole image ==
 wrote 16777216/16777216 bytes at offset 0
@@ -10,7 +10,7 @@ wrote 16777216/16777216 bytes at offset 0
 read 16777216/16777216 bytes at offset 0
 16 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 == create overlay ==
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT encrypt.format=luks 
encrypt.key-secret=sec1 encrypt.iter-time=10 size=16777216 
backing_file=TEST_DIR/t.IMGFMT.base
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=16777216 
backing_file=TEST_DIR/t.IMGFMT.base
 
 == writing part of a cluster ==
 wrote 1024/1024 bytes at offset 0
diff --git a/tests/qemu-iotests/198.out b/tests/qemu-iotests/198.out
index b0f2d417af..acfdf96b0c 100644
--- a/tests/qemu-iotests/198.out
+++ b/tests/qemu-iotests/198.out
@@ -1,12 +1,12 @@
 QA output created by 198
 == create base ==
-Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT encrypt.format=luks 
encrypt.key-secret=sec0 encrypt.iter-time=10 size=16777216
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=16777216
 
 == writing whole image base ==
 wrote 16777216/16777216 bytes at offset 0
 16 MiB, X o

[PATCH v2 12/14] block/crypto: implement blockdev-amend

[Maxim Levitsky]

Signed-off-by: Maxim Levitsky 
Reviewed-by: Daniel P. Berrangé 
---
 block/crypto.c   | 72 
 qapi/block-core.json | 14 -
 2 files changed, 66 insertions(+), 20 deletions(-)

diff --git a/block/crypto.c b/block/crypto.c
index 727a3fde58..389586200f 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -694,32 +694,21 @@ block_crypto_get_specific_info_luks(BlockDriverState *bs, 
Error **errp)
 }
 
 static int
-block_crypto_amend_options_luks(BlockDriverState *bs,
-   QemuOpts *opts,
-   BlockDriverAmendStatusCB *status_cb,
-   void *cb_opaque,
-   bool force,
-   Error **errp)
+block_crypto_amend_options_generic_luks(BlockDriverState *bs,
+QCryptoBlockAmendOptions 
*amend_options,
+bool force,
+Error **errp)
 {
 BlockCrypto *crypto = bs->opaque;
-QDict *cryptoopts = NULL;
-QCryptoBlockAmendOptions *amend_options = NULL;
 int ret;
 
 assert(crypto);
 assert(crypto->block);
-crypto->updating_keys = true;
 
+/* apply for exclusive read/write permissions to the underlying file*/
+crypto->updating_keys = true;
 ret = bdrv_child_refresh_perms(bs, bs->file, errp);
-if (ret < 0) {
-goto cleanup;
-}
-
-cryptoopts = qemu_opts_to_qdict(opts, NULL);
-qdict_put_str(cryptoopts, "format", "luks");
-amend_options = block_crypto_amend_opts_init(cryptoopts, errp);
-if (!amend_options) {
-ret = -EINVAL;
+if (ret) {
 goto cleanup;
 }
 
@@ -731,13 +720,57 @@ block_crypto_amend_options_luks(BlockDriverState *bs,
   force,
   errp);
 cleanup:
+/* release exclusive read/write permissions to the underlying file*/
 crypto->updating_keys = false;
 bdrv_child_refresh_perms(bs, bs->file, errp);
-qapi_free_QCryptoBlockAmendOptions(amend_options);
+return ret;
+}
+
+static int
+block_crypto_amend_options_luks(BlockDriverState *bs,
+QemuOpts *opts,
+BlockDriverAmendStatusCB *status_cb,
+void *cb_opaque,
+bool force,
+Error **errp)
+{
+BlockCrypto *crypto = bs->opaque;
+QDict *cryptoopts = NULL;
+QCryptoBlockAmendOptions *amend_options = NULL;
+int ret = -EINVAL;
+
+assert(crypto);
+assert(crypto->block);
+
+cryptoopts = qemu_opts_to_qdict(opts, NULL);
+qdict_put_str(cryptoopts, "format", "luks");
+amend_options = block_crypto_amend_opts_init(cryptoopts, errp);
 qobject_unref(cryptoopts);
+if (!amend_options) {
+goto cleanup;
+}
+ret = block_crypto_amend_options_generic_luks(bs, amend_options,
+  force, errp);
+cleanup:
+qapi_free_QCryptoBlockAmendOptions(amend_options);
 return ret;
 }
 
+static int
+coroutine_fn block_crypto_co_amend_luks(BlockDriverState *bs,
+BlockdevAmendOptions *opts,
+bool force,
+Error **errp)
+{
+QCryptoBlockAmendOptions amend_opts;
+
+amend_opts = (QCryptoBlockAmendOptions) {
+.format = Q_CRYPTO_BLOCK_FORMAT_LUKS,
+.u.luks = *qapi_BlockdevAmendOptionsLUKS_base(&opts->u.luks),
+};
+return block_crypto_amend_options_generic_luks(bs, &amend_opts,
+   force, errp);
+}
 
 static void
 block_crypto_child_perms(BlockDriverState *bs, BdrvChild *c,
@@ -809,6 +842,7 @@ static BlockDriver bdrv_crypto_luks = {
 .bdrv_get_info  = block_crypto_get_info_luks,
 .bdrv_get_specific_info = block_crypto_get_specific_info_luks,
 .bdrv_amend_options = block_crypto_amend_options_luks,
+.bdrv_co_amend  = block_crypto_co_amend_luks,
 
 .strong_runtime_opts = block_crypto_strong_runtime_opts,
 };
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 192da75a10..967b5738c9 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -4757,6 +4757,18 @@
   'data': { 'job-id': 'str',
 'options': 'BlockdevCreateOptions' } }
 
+##
+# @BlockdevAmendOptionsLUKS:
+#
+# Driver specific image amend options for LUKS.
+#
+# Since: 5.0
+##
+{ 'struct': 'BlockdevAmendOptionsLUKS',
+  'base': 'QCryptoBlockAmendOptionsLUKS',
+  'data': { }
+}
+
 ##
 # @BlockdevAmendOptions:
 #
@@ -4771,7 +4783,7 @@
   'driver': 'BlockdevDriver' },
   'discriminator': 'driver',
   'data': {
-  } }
+  'luks':   'BlockdevAmendOptionsLUKS' } }
 
 ##
 # @x-blockdev-amend:
-- 
2.17.2

[PATCH v2 13/14] block/qcow2: implement blockdev-amend

[Maxim Levitsky]

Currently the implementation only supports amending the encryption
options, unlike the qemu-img version

Signed-off-by: Maxim Levitsky 
Reviewed-by: Daniel P. Berrangé 
---
 block/qcow2.c| 39 +++
 qapi/block-core.json | 16 +++-
 2 files changed, 54 insertions(+), 1 deletion(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 10b22544f2..8fde20344d 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -5397,6 +5397,44 @@ static int qcow2_amend_options(BlockDriverState *bs, 
QemuOpts *opts,
 return 0;
 }
 
+static int coroutine_fn qcow2_co_amend(BlockDriverState *bs,
+   BlockdevAmendOptions *opts,
+   bool force,
+   Error **errp)
+{
+BlockdevAmendOptionsQcow2 *qopts = &opts->u.qcow2;
+BDRVQcow2State *s = bs->opaque;
+int ret = 0;
+
+if (qopts->has_encrypt) {
+if (!s->crypto) {
+error_setg(errp, "image is not encrypted, can't amend");
+return -EOPNOTSUPP;
+}
+
+if (qopts->encrypt->format != Q_CRYPTO_BLOCK_FORMAT_LUKS) {
+error_setg(errp,
+   "Amend can't be used to change the qcow2 encryption 
format");
+return -EOPNOTSUPP;
+}
+
+if (s->crypt_method_header != QCOW_CRYPT_LUKS) {
+error_setg(errp,
+   "Only LUKS encryption options can be amended for qcow2 
with blockdev-amend");
+return -EOPNOTSUPP;
+}
+
+ret = qcrypto_block_amend_options(s->crypto,
+  qcow2_crypto_hdr_read_func,
+  qcow2_crypto_hdr_write_func,
+  bs,
+  qopts->encrypt,
+  force,
+  errp);
+}
+return ret;
+}
+
 /*
  * If offset or size are negative, respectively, they will not be included in
  * the BLOCK_IMAGE_CORRUPTED event emitted.
@@ -5606,6 +5644,7 @@ BlockDriver bdrv_qcow2 = {
 .mutable_opts= mutable_opts,
 .bdrv_co_check   = qcow2_co_check,
 .bdrv_amend_options  = qcow2_amend_options,
+.bdrv_co_amend   = qcow2_co_amend,
 
 .bdrv_detach_aio_context  = qcow2_detach_aio_context,
 .bdrv_attach_aio_context  = qcow2_attach_aio_context,
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 967b5738c9..4b69b0e195 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -4769,6 +4769,19 @@
   'data': { }
 }
 
+##
+# @BlockdevAmendOptionsQcow2:
+#
+# Driver specific image amend options for qcow2.
+# For now, only encryption options can be amended
+#
+# @encrypt  Encryption options to be amended
+#
+# Since: 5.0
+##
+{ 'struct': 'BlockdevAmendOptionsQcow2',
+  'data': { '*encrypt': 'QCryptoBlockAmendOptions' } }
+
 ##
 # @BlockdevAmendOptions:
 #
@@ -4783,7 +4796,8 @@
   'driver': 'BlockdevDriver' },
   'discriminator': 'driver',
   'data': {
-  'luks':   'BlockdevAmendOptionsLUKS' } }
+  'luks':   'BlockdevAmendOptionsLUKS',
+  'qcow2':  'BlockdevAmendOptionsQcow2' } }
 
 ##
 # @x-blockdev-amend:
-- 
2.17.2

Re: [PATCH v32 16/22] hw/char: RX62N serial communication interface (SCI)

[Philippe Mathieu-Daudé]

Hi Yoshinori,

On 2/24/20 3:19 PM, Yoshinori Sato wrote:

This module supported only non FIFO type.
Hardware manual.
https://www.renesas.com/us/en/doc/products/mpumcu/doc/rx_family/r01uh0033ej0140_rx62n.pdf

Signed-off-by: Yoshinori Sato 
Reviewed-by: Alex Bennée 
Reviewed-by: Philippe Mathieu-Daudé 
Message-Id: <20190607091116.49044-8-ys...@users.sourceforge.jp>
Tested-by: Philippe Mathieu-Daudé 
Signed-off-by: Richard Henderson 
---
  include/hw/char/renesas_sci.h |  45 +
  hw/char/renesas_sci.c | 342 ++
  hw/char/Kconfig   |   3 +
  hw/char/Makefile.objs |   1 +
  4 files changed, 391 insertions(+)
  create mode 100644 include/hw/char/renesas_sci.h
  create mode 100644 hw/char/renesas_sci.c

diff --git a/include/hw/char/renesas_sci.h b/include/hw/char/renesas_sci.h
new file mode 100644
index 00..50d1336944
--- /dev/null
+++ b/include/hw/char/renesas_sci.h
@@ -0,0 +1,45 @@
+/*
+ * Renesas Serial Communication Interface
+ *
+ * Copyright (c) 2018 Yoshinori Sato
+ *
+ * This code is licensed under the GPL version 2 or later.
+ *
+ */
+
+#include "chardev/char-fe.h"
+#include "qemu/timer.h"
+#include "hw/sysbus.h"
+
+#define TYPE_RENESAS_SCI "renesas-sci"
+#define RSCI(obj) OBJECT_CHECK(RSCIState, (obj), TYPE_RENESAS_SCI)
+
+enum {
+ERI = 0,
+RXI = 1,
+TXI = 2,
+TEI = 3,
+SCI_NR_IRQ = 4,
+};
+
+typedef struct {
+SysBusDevice parent_obj;
+MemoryRegion memory;
+
+uint8_t smr;
+uint8_t brr;
+uint8_t scr;
+uint8_t tdr;
+uint8_t ssr;
+uint8_t rdr;
+uint8_t scmr;
+uint8_t semr;
+
+uint8_t read_ssr;
+int64_t trtime;
+int64_t rx_next;
+QEMUTimer *timer;
+CharBackend chr;
+uint64_t input_freq;
+qemu_irq irq[SCI_NR_IRQ];
+} RSCIState;
diff --git a/hw/char/renesas_sci.c b/hw/char/renesas_sci.c
new file mode 100644
index 00..0760a51f43
--- /dev/null
+++ b/hw/char/renesas_sci.c
@@ -0,0 +1,342 @@
+/*
+ * Renesas Serial Communication Interface


Looking at this again, have you looked at the SH model 
(hw/char/sh_serial.c)? This seems the same.

(Similarly your timer model with hw/timer/sh_timer.c).


+ *
+ * Datasheet: RX62N Group, RX621 Group User's Manual: Hardware
+ * (Rev.1.40 R01UH0033EJ0140)
+ *
+ * Copyright (c) 2019 Yoshinori Sato
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2 or later, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see .
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/log.h"
+#include "qapi/error.h"
+#include "qemu-common.h"
+#include "hw/hw.h"
+#include "hw/irq.h"
+#include "hw/sysbus.h"
+#include "hw/registerfields.h"
+#include "hw/qdev-properties.h"
+#include "hw/char/renesas_sci.h"
+#include "migration/vmstate.h"
+#include "qemu/error-report.h"
+
+/* SCI register map */
+REG8(SMR, 0)
+  FIELD(SMR, CKS,  0, 2)
+  FIELD(SMR, MP,   2, 1)
+  FIELD(SMR, STOP, 3, 1)
+  FIELD(SMR, PM,   4, 1)
+  FIELD(SMR, PE,   5, 1)
+  FIELD(SMR, CHR,  6, 1)
+  FIELD(SMR, CM,   7, 1)
+REG8(BRR, 1)
+REG8(SCR, 2)
+  FIELD(SCR, CKE, 0, 2)
+  FIELD(SCR, TEIE, 2, 1)
+  FIELD(SCR, MPIE, 3, 1)
+  FIELD(SCR, RE,   4, 1)
+  FIELD(SCR, TE,   5, 1)
+  FIELD(SCR, RIE,  6, 1)
+  FIELD(SCR, TIE,  7, 1)
+REG8(TDR, 3)
+REG8(SSR, 4)
+  FIELD(SSR, MPBT, 0, 1)
+  FIELD(SSR, MPB,  1, 1)
+  FIELD(SSR, TEND, 2, 1)
+  FIELD(SSR, ERR, 3, 3)
+FIELD(SSR, PER,  3, 1)
+FIELD(SSR, FER,  4, 1)
+FIELD(SSR, ORER, 5, 1)
+  FIELD(SSR, RDRF, 6, 1)
+  FIELD(SSR, TDRE, 7, 1)
+REG8(RDR, 5)
+REG8(SCMR, 6)
+  FIELD(SCMR, SMIF, 0, 1)
+  FIELD(SCMR, SINV, 2, 1)
+  FIELD(SCMR, SDIR, 3, 1)
+  FIELD(SCMR, BCP2, 7, 1)
+REG8(SEMR, 7)
+  FIELD(SEMR, ACS0, 0, 1)
+  FIELD(SEMR, ABCS, 4, 1)
+
+static int can_receive(void *opaque)
+{
+RSCIState *sci = RSCI(opaque);
+if (sci->rx_next > qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL)) {
+return 0;
+} else {
+return FIELD_EX8(sci->scr, SCR, RE);
+}
+}
+
+static void receive(void *opaque, const uint8_t *buf, int size)
+{
+RSCIState *sci = RSCI(opaque);
+sci->rx_next = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + sci->trtime;
+if (FIELD_EX8(sci->ssr, SSR, RDRF) || size > 1) {
+sci->ssr = FIELD_DP8(sci->ssr, SSR, ORER, 1);
+if (FIELD_EX8(sci->scr, SCR, RIE)) {
+qemu_set_irq(sci->irq[ERI], 1);
+}
+} else {
+sci->rdr = buf[0];
+sci->ssr = FIELD_DP8(sci->ssr, SSR, RDRF, 1);
+if (FIELD_EX8(sci->scr, SCR, RIE)) {
+qemu_irq_pulse(sci->irq[RXI]);
+  

Re: [PATCH v32 21/22] BootLinuxConsoleTest: Test the RX-Virt machine

[Philippe Mathieu-Daudé]

On 2/24/20 3:19 PM, Yoshinori Sato wrote:

From: Philippe Mathieu-Daudé 

Add two tests for the rx-virt machine, based on the recommended test
setup from Yoshinori Sato:
https://lists.gnu.org/archive/html/qemu-devel/2019-05/msg03586.html

- U-Boot prompt
- Linux kernel with Sash shell

These are very quick tests:

   $ avocado run -t arch:rx tests/acceptance/boot_linux_console.py
   JOB ID : 84a6ef01c0b87975ecbfcb31a920afd735753ace
   JOB LOG: 
/home/phil/avocado/job-results/job-2019-05-24T05.02-84a6ef0/job.log
(1/2) 
tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_rx_uboot: PASS 
(0.11 s)
(2/2) 
tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_rx_linux: PASS 
(0.45 s)
   RESULTS: PASS 2 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | 
CANCEL 0

Tests can also be run with:

   $ avocado --show=console run -t arch:rx 
tests/acceptance/boot_linux_console.py
   console: U-Boot 2016.05-rc3-23705-ga1ef3c71cb-dirty (Feb 05 2019 - 21:56:06 
+0900)
   console: Linux version 4.19.0+ (yo-satoh@yo-satoh-debian) (gcc version 9.0.0 
20181105 (experimental) (GCC)) #137 Wed Feb 20 23:20:02 JST 2019
   console: Built 1 zonelists, mobility grouping on.  Total pages: 8128
   ...
   console: SuperH (H)SCI(F) driver initialized
   console: 88240.serial: ttySC0 at MMIO 0x88240 (irq = 215, base_baud = 0) is 
a sci
   console: console [ttySC0] enabled
   console: 88248.serial: ttySC1 at MMIO 0x88248 (irq = 219, base_baud = 0) is 
a sci

Signed-off-by: Philippe Mathieu-Daudé 
Based-on: 20190517045136.3509-1-richard.hender...@linaro.org
"RX architecture support"
Signed-off-by: Yoshinori Sato 
---
  tests/acceptance/boot_linux_console.py | 46 ++
  1 file changed, 46 insertions(+)

diff --git a/tests/acceptance/boot_linux_console.py 
b/tests/acceptance/boot_linux_console.py
index 34d37eba3b..367cf480a5 100644
--- a/tests/acceptance/boot_linux_console.py
+++ b/tests/acceptance/boot_linux_console.py
@@ -686,3 +686,49 @@ class BootLinuxConsole(Test):
  tar_hash = '49e88d9933742f0164b60839886c9739cb7a0d34'
  self.vm.add_args('-cpu', 'dc233c')
  self.do_test_advcal_2018('02', tar_hash, 'santas-sleigh-ride.elf')
+
+def test_rx_uboot(self):
+"""
+:avocado: tags=arch:rx
+:avocado: tags=machine:rx-virt
+:avocado: tags=endian:little
+"""
+uboot_url = ('https://acc.dl.osdn.jp/users/23/23888/u-boot.bin.gz')
+uboot_hash = '9b78dbd43b40b2526848c0b1ce9de02c24f4dcdb'
+uboot_path = self.fetch_asset(uboot_url, asset_hash=uboot_hash)
+uboot_path = archive.uncompress(uboot_path, self.workdir)
+
+self.vm.set_machine('rx-virt')
+self.vm.set_console()
+self.vm.add_args('-bios', uboot_path,
+ '-no-reboot')
+self.vm.launch()
+uboot_version = 'U-Boot 2016.05-rc3-23705-ga1ef3c71cb-dirty'
+self.wait_for_console_pattern(uboot_version)
+gcc_version = 'rx-unknown-linux-gcc (GCC) 9.0.0 20181105 
(experimental)'
+# FIXME limit baudrate on chardev, else we type too fast
+#self.exec_command_and_wait_for_pattern('version', gcc_version)
+
+def test_rx_linux(self):
+"""
+:avocado: tags=arch:rx
+:avocado: tags=machine:rx-virt
+:avocado: tags=endian:little
+"""
+dtb_url = ('https://acc.dl.osdn.jp/users/23/23887/rx-qemu.dtb')


Sourceforge URL are not very stable, I'm now getting:

HTTP request sent, awaiting response... 302 Found
Location: https://osdn.dl.osdn.net/users/23/23887/rx-qemu.dtb [following]
--2020-03-08 17:17:31--  https://osdn.dl.osdn.net/users/23/23887/rx-qemu.dtb
Resolving osdn.dl.osdn.net (osdn.dl.osdn.net)... 202.221.179.23
Connecting to osdn.dl.osdn.net (osdn.dl.osdn.net)|202.221.179.23|:443... 
connected.

HTTP request sent, awaiting response... 404 Not Found
2020-03-08 17:17:32 ERROR 404: Not Found.


+dtb_hash = '7b4e4e2c71905da44e86ce47adee2210b026ac18'
+dtb_path = self.fetch_asset(dtb_url, asset_hash=dtb_hash)
+kernel_url = ('http://acc.dl.osdn.jp/users/23/23845/zImage')
+kernel_hash = '39a81067f8d72faad90866ddfefa19165d68fc99'
+kernel_path = self.fetch_asset(kernel_url, asset_hash=kernel_hash)
+
+self.vm.set_machine('rx-virt')
+self.vm.set_console()
+kernel_command_line = self.KERNEL_COMMON_COMMAND_LINE + 'earlycon'
+self.vm.add_args('-kernel', kernel_path,
+ '-dtb', dtb_path,
+ '-no-reboot')
+self.vm.launch()
+self.wait_for_console_pattern('Sash command shell (version 1.1.1)')
+self.exec_command_and_wait_for_pattern('printenv',
+   'TERM=linux')

Regarding SESparse support in QEMU

[Tirthankar Saha]

Hi Sam,

Can you please share any notes that you have regarding the structure of the
SESparse journal? This will help in adding "read-write" support for
SESparse snapshots.

Thanks,

Tirthankar

Re: [PATCH v6 3/4] linux-user: Support futex_time64

[Laurent Vivier]

Le 06/03/2020 à 19:24, Alistair Francis a écrit :
> Add support for host and target futex_time64. If futex_time64 exists on
> the host we try that first before falling back to the standard futux
> syscall.

In fact there are two definitions for futex: one to use as system call
in TARGET_NR_exit (sys_futex) and one to use to translate
TARGET_NR_futex (see d509eeb13c9c ("linux-user: Use safe_syscall for
futex syscall")).

We also rely on the system timespec definition, so I'm not sure anymore
we can use both 32bit and 64bit host syscalls (it's more complicated
than I expected...)


> Signed-off-by: Alistair Francis 
> ---
>  linux-user/syscall.c | 98 
>  1 file changed, 80 insertions(+), 18 deletions(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 0f219b26c1..8a50e2d3dc 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -245,7 +245,12 @@ static type name (type1 arg1,type2 arg2,type3 arg3,type4 
> arg4,type5 arg5,\
>  #define __NR_sys_rt_sigqueueinfo __NR_rt_sigqueueinfo
>  #define __NR_sys_rt_tgsigqueueinfo __NR_rt_tgsigqueueinfo
>  #define __NR_sys_syslog __NR_syslog
> -#define __NR_sys_futex __NR_futex
> +#if defined(__NR_futex)
> +# define __NR_sys_futex __NR_futex
> +#endif
> +#if defined(__NR_futex_time64)
> +# define __NR_sys_futex_time64 __NR_futex_time64
> +#endif

As you don't use futex_time64() in TARGET_NR_exit, you don't need to
define it.

>  #define __NR_sys_inotify_init __NR_inotify_init
>  #define __NR_sys_inotify_add_watch __NR_inotify_add_watch
>  #define __NR_sys_inotify_rm_watch __NR_inotify_rm_watch
> @@ -295,10 +300,15 @@ _syscall1(int,exit_group,int,error_code)
>  #if defined(TARGET_NR_set_tid_address) && defined(__NR_set_tid_address)
>  _syscall1(int,set_tid_address,int *,tidptr)
>  #endif
> -#if defined(TARGET_NR_futex) && defined(__NR_futex)
> +#if (defined(TARGET_NR_futex) || defined(TARGET_NR_futex_time64)) && \
> +defined(__NR_futex)
>  _syscall6(int,sys_futex,int *,uaddr,int,op,int,val,
>const struct timespec *,timeout,int *,uaddr2,int,val3)
>  #endif
> +#if defined(TARGET_NR_futex_time64) && defined(__NR_futex_time64)
> +_syscall6(int,sys_futex_time64,int *,uaddr,int,op,int,val,
> +  const struct timespec *,timeout,int *,uaddr2,int,val3)
> +#endif

We don't need them as the sys_ version are not used in TARGET_NR_futex
and TARGET_NR_futex_time64...

>  #define __NR_sys_sched_getaffinity __NR_sched_getaffinity
>  _syscall3(int, sys_sched_getaffinity, pid_t, pid, unsigned int, len,
>unsigned long *, user_mask_ptr);
> @@ -762,10 +772,14 @@ safe_syscall5(int, ppoll, struct pollfd *, ufds, 
> unsigned int, nfds,
>  safe_syscall6(int, epoll_pwait, int, epfd, struct epoll_event *, events,
>int, maxevents, int, timeout, const sigset_t *, sigmask,
>size_t, sigsetsize)
> -#ifdef TARGET_NR_futex
> +#if defined(__NR_futex)
>  safe_syscall6(int,futex,int *,uaddr,int,op,int,val, \
>const struct timespec *,timeout,int *,uaddr2,int,val3)
>  #endif
> +#if defined(__NR_futex_time64)
> +safe_syscall6(int,futex_time64,int *,uaddr,int,op,int,val, \
> +  const struct timespec *,timeout,int *,uaddr2,int,val3)
> +#endif

... we use these ones, the safe_ version.

>  safe_syscall2(int, rt_sigsuspend, sigset_t *, newset, size_t, sigsetsize)
>  safe_syscall2(int, kill, pid_t, pid, int, sig)
>  safe_syscall2(int, tkill, int, tid, int, sig)
> @@ -1210,7 +1224,7 @@ static inline abi_long copy_to_user_timeval64(abi_ulong 
> target_tv_addr,
>  return 0;
>  }
>  
> -#if defined(TARGET_NR_futex) || \
> +#if defined(TARGET_NR_futex) || defined(TARGET_NR_futex_time64) || \
>  defined(TARGET_NR_rt_sigtimedwait) || \
>  defined(TARGET_NR_pselect6) || defined(TARGET_NR_pselect6) || \
>  defined(TARGET_NR_nanosleep) || defined(TARGET_NR_clock_settime) || \
> @@ -6898,12 +6912,12 @@ static inline abi_long host_to_target_statx(struct 
> target_statx *host_stx,
> futexes locally would make futexes shared between multiple processes
> tricky.  However they're probably useless because guest atomic
> operations won't work either.  */
> -#if defined(TARGET_NR_futex)
> +#if defined(TARGET_NR_futex) || defined(TARGET_NR_futex_time64)
>  static int do_futex(target_ulong uaddr, int op, int val, target_ulong 
> timeout,
>  target_ulong uaddr2, int val3)
>  {
>  struct timespec ts, *pts;
> -int base_op;
> +int base_op, err = -ENOSYS;
>  
>  /* ??? We assume FUTEX_* constants are the same on both host
> and target.  */
> @@ -6915,18 +6929,49 @@ static int do_futex(target_ulong uaddr, int op, int 
> val, target_ulong timeout,
>  switch (base_op) {
>  case FUTEX_WAIT:
>  case FUTEX_WAIT_BITSET:
> +#ifdef __NR_futex_time64
> +struct __kernel_timespec ts64, *pts64;
> +
>  if (timeout) {
> -pts = &ts;
> -target_to_host_timespec

Re: [PATCH v8 07/10] virtio-9p: introduce ERRP_AUTO_PROPAGATE

[Christian Schoenebeck]

On Freitag, 6. März 2020 06:15:33 CET Vladimir Sementsov-Ogievskiy wrote:
> If we want to add some info to errp (by error_prepend() or
> error_append_hint()), we must use the ERRP_AUTO_PROPAGATE macro.
> Otherwise, this info will not be added when errp == &error_fatal
> (the program will exit prior to the error_append_hint() or
> error_prepend() call).  Fix such cases.
> 
> If we want to check error after errp-function call, we need to
> introduce local_err and then propagate it to errp. Instead, use
> ERRP_AUTO_PROPAGATE macro, benefits are:
> 1. No need of explicit error_propagate call
> 2. No need of explicit local_err variable: use errp directly
> 3. ERRP_AUTO_PROPAGATE leaves errp as is if it's not NULL or
>&error_fatal, this means that we don't break error_abort
>(we'll abort on error_set, not on error_propagate)
> 
> This commit is generated by command
> 
> sed -n '/^virtio-9p$/,/^$/{s/^F: //p}' MAINTAINERS | \
> xargs git ls-files | grep '\.[hc]$' | \
> xargs spatch \
> --sp-file scripts/coccinelle/auto-propagated-errp.cocci \
> --macro-file scripts/cocci-macro-file.h \
> --in-place --no-show-diff --max-width 80
> 
> Reported-by: Kevin Wolf 
> Reported-by: Greg Kurz 
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> Acked-by: Greg Kurz 

Reviewed-by: Christian Schoenebeck 

> ---
>  hw/9pfs/9p-local.c | 12 +---
>  hw/9pfs/9p.c   |  1 +
>  2 files changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
> index 54e012e5b4..0361e0c0b4 100644
> --- a/hw/9pfs/9p-local.c
> +++ b/hw/9pfs/9p-local.c
> @@ -1479,10 +1479,10 @@ static void error_append_security_model_hint(Error
> *const *errp)
> 
>  static int local_parse_opts(QemuOpts *opts, FsDriverEntry *fse, Error
> **errp) {
> +ERRP_AUTO_PROPAGATE();
>  const char *sec_model = qemu_opt_get(opts, "security_model");
>  const char *path = qemu_opt_get(opts, "path");
>  const char *multidevs = qemu_opt_get(opts, "multidevs");
> -Error *local_err = NULL;
> 
>  if (!sec_model) {
>  error_setg(errp, "security_model property not set");
> @@ -1516,11 +1516,10 @@ static int local_parse_opts(QemuOpts *opts,
> FsDriverEntry *fse, Error **errp) fse->export_flags &=
> ~V9FS_FORBID_MULTIDEVS;
>  fse->export_flags &= ~V9FS_REMAP_INODES;
>  } else {
> -error_setg(&local_err, "invalid multidevs property '%s'",
> +error_setg(errp, "invalid multidevs property '%s'",
> multidevs);
> -error_append_hint(&local_err, "Valid options are: multidevs="
> +error_append_hint(errp, "Valid options are: multidevs="
>"[remap|forbid|warn]\n");
> -error_propagate(errp, local_err);
>  return -1;
>  }
>  }
> @@ -1530,9 +1529,8 @@ static int local_parse_opts(QemuOpts *opts,
> FsDriverEntry *fse, Error **errp) return -1;
>  }
> 
> -if (fsdev_throttle_parse_opts(opts, &fse->fst, &local_err)) {
> -error_propagate_prepend(errp, local_err,
> -"invalid throttle configuration: ");
> +if (fsdev_throttle_parse_opts(opts, &fse->fst, errp)) {
> +error_prepend(errp, "invalid throttle configuration: ");
>  return -1;
>  }
> 
> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> index 9e046f7acb..3aa6a57f3a 100644
> --- a/hw/9pfs/9p.c
> +++ b/hw/9pfs/9p.c
> @@ -4023,6 +4023,7 @@ void pdu_submit(V9fsPDU *pdu, P9MsgHeader *hdr)
>  int v9fs_device_realize_common(V9fsState *s, const V9fsTransport *t,
> Error **errp)
>  {
> +ERRP_AUTO_PROPAGATE();
>  int i, len;
>  struct stat stat;
>  FsDriverEntry *fse;

Re: [PATCH v8 02/10] scripts: add coccinelle script to use auto propagated errp

[Christian Schoenebeck]

On Freitag, 6. März 2020 06:15:28 CET Vladimir Sementsov-Ogievskiy wrote:
> diff --git a/scripts/coccinelle/auto-propagated-errp.cocci
> b/scripts/coccinelle/auto-propagated-errp.cocci new file mode 100644
> index 00..bff274bd6d
> --- /dev/null
> +++ b/scripts/coccinelle/auto-propagated-errp.cocci
> @@ -0,0 +1,231 @@
> +// Use ERRP_AUTO_PROPAGATE (see include/qapi/error.h)
> +//
> +// Copyright (c) 2020 Virtuozzo International GmbH.

Just in case:

WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?

Best regards,
Christian Schoenebeck

Re: [PATCH 0/5] mostly changes related to audio float samples

[Volker Rümelin]

Hi Howard,

thank you for testing the patches.

> Hi,
>
> I applied these to Mark's screamer branch to create a new OSX build. Testing 
> was done by playing system sounds, and an MP3/Internet radio with 
> QuickTime/iTunes. With or without 6dB, the max volume is way out of my 
> comfort zone. I hear no real difference in quality compared to a build from 
> the

It wasn't my intention to annoy your neighbours. On my computer I use an analog 
output for playback. Clipping happens before DA conversion, so I can use the 
volume control on the amplifier to have normal volume levels.

> current screamer branch. If any, it might sound a bit better. That means 
> there still is some minimal crackling (clipping?) in the high volume parts of 
> the

Crackling and clipping are two different things. Crackling is normally a timing 
problem where audio samples aren't delivered in time. Clipping produces 
harmonic distortions. For example a clipped sine wave will sound metallic 
because of the harmonic distortions. The difference between a clipped and a 
pure sine wave is clearly audible.

> audio I played with OSX guests, not Mac OS 9.x guests.  
>
> So as there is no regression,
> tested by: howard spoelstra mailto:hsp.c...@gmail.com>>

With best regards,
Volker

[PATCH v2 0/6] mostly changes related to audio float samples

[Volker Rümelin]

v2:
- "qapi/audio: add documentation for AudioFormat"
  Markus suggested to correct a spelling mistake.

- "audio: add audiodev format=f32 option documentation"
  New patch.

Volker Rümelin (6):
  qapi/audio: add documentation for AudioFormat
  audio: change naming scheme of FLOAT_CONV macros
  audio: consistency changes
  audio: change mixing engine float range to [-1.f, 1.f]
  audio: fix saturation nonlinearity in clip_* functions
  audio: add audiodev format=f32 option documentation

 audio/mixeng.c  | 26 +-
 audio/mixeng_template.h | 22 ++
 qapi/audio.json | 14 ++
 qemu-options.hx |  4 ++--
 4 files changed, 39 insertions(+), 27 deletions(-)

-- 
2.16.4

[PATCH v2 1/6] qapi/audio: add documentation for AudioFormat

[Volker Rümelin]

The review for patch ed2a4a7941 "audio: proper support for
float samples in mixeng" suggested this would be a good idea.

Acked-by: Markus Armbruster 
Signed-off-by: Volker Rümelin 
---
 qapi/audio.json | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/qapi/audio.json b/qapi/audio.json
index d8c507cced..c31251f45b 100644
--- a/qapi/audio.json
+++ b/qapi/audio.json
@@ -273,6 +273,20 @@
 #
 # An enumeration of possible audio formats.
 #
+# @u8: unsigned 8 bit integer
+#
+# @s8: signed 8 bit integer
+#
+# @u16: unsigned 16 bit integer
+#
+# @s16: signed 16 bit integer
+#
+# @u32: unsigned 32 bit integer
+#
+# @s32: signed 32 bit integer
+#
+# @f32: single precision floating-point (since 5.0)
+#
 # Since: 4.0
 ##
 { 'enum': 'AudioFormat',
-- 
2.16.4

[PATCH v2 2/6] audio: change naming scheme of FLOAT_CONV macros

[Volker Rümelin]

This patch changes the naming scheme of the FLOAT_CONV_TO and
FLOAT_CONV_FROM macros to the scheme used in mixeng_template.h.

Signed-off-by: Volker Rümelin 
---
 audio/mixeng.c | 22 +++---
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/audio/mixeng.c b/audio/mixeng.c
index c14b0d874c..b57fad83bf 100644
--- a/audio/mixeng.c
+++ b/audio/mixeng.c
@@ -268,17 +268,17 @@ f_sample *mixeng_clip[2][2][2][3] = {
 };
 
 #ifdef FLOAT_MIXENG
-#define FLOAT_CONV_TO(x) (x)
-#define FLOAT_CONV_FROM(x) (x)
+#define CONV_NATURAL_FLOAT(x) (x)
+#define CLIP_NATURAL_FLOAT(x) (x)
 #else
 static const float float_scale = UINT_MAX;
-#define FLOAT_CONV_TO(x) ((x) * float_scale)
+#define CONV_NATURAL_FLOAT(x) ((x) * float_scale)
 
 #ifdef RECIPROCAL
 static const float float_scale_reciprocal = 1.f / UINT_MAX;
-#define FLOAT_CONV_FROM(x) ((x) * float_scale_reciprocal)
+#define CLIP_NATURAL_FLOAT(x) ((x) * float_scale_reciprocal)
 #else
-#define FLOAT_CONV_FROM(x) ((x) / float_scale)
+#define CLIP_NATURAL_FLOAT(x) ((x) / float_scale)
 #endif
 #endif
 
@@ -288,7 +288,7 @@ static void conv_natural_float_to_mono(struct st_sample 
*dst, const void *src,
 float *in = (float *)src;
 
 while (samples--) {
-dst->r = dst->l = FLOAT_CONV_TO(*in++);
+dst->r = dst->l = CONV_NATURAL_FLOAT(*in++);
 dst++;
 }
 }
@@ -299,8 +299,8 @@ static void conv_natural_float_to_stereo(struct st_sample 
*dst, const void *src,
 float *in = (float *)src;
 
 while (samples--) {
-dst->l = FLOAT_CONV_TO(*in++);
-dst->r = FLOAT_CONV_TO(*in++);
+dst->l = CONV_NATURAL_FLOAT(*in++);
+dst->r = CONV_NATURAL_FLOAT(*in++);
 dst++;
 }
 }
@@ -316,7 +316,7 @@ static void clip_natural_float_from_mono(void *dst, const 
struct st_sample *src,
 float *out = (float *)dst;
 
 while (samples--) {
-*out++ = FLOAT_CONV_FROM(src->l) + FLOAT_CONV_FROM(src->r);
+*out++ = CLIP_NATURAL_FLOAT(src->l) + CLIP_NATURAL_FLOAT(src->r);
 src++;
 }
 }
@@ -327,8 +327,8 @@ static void clip_natural_float_from_stereo(
 float *out = (float *)dst;
 
 while (samples--) {
-*out++ = FLOAT_CONV_FROM(src->l);
-*out++ = FLOAT_CONV_FROM(src->r);
+*out++ = CLIP_NATURAL_FLOAT(src->l);
+*out++ = CLIP_NATURAL_FLOAT(src->r);
 src++;
 }
 }
-- 
2.16.4

[PATCH v2 4/6] audio: change mixing engine float range to [-1.f, 1.f]

[Volker Rümelin]

Currently the internal float range of the mixing engine is
[-.5f, .5f]. PulseAudio, SDL2 and libasound use a [-1.f, 1.f]
range. This means with float samples the audio playback volume
is 6dB too low and audio recording signals will be clipped in
most cases.

To avoid another scaling factor in the conv_natural_float_* and
clip_natural_float_* functions with FLOAT_MIXENG defined this
patch changes the mixing engine float range to [-1.f, 1.f].

Signed-off-by: Volker Rümelin 
---
 audio/mixeng.c  |  4 ++--
 audio/mixeng_template.h | 17 -
 2 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/audio/mixeng.c b/audio/mixeng.c
index 725b529be7..739a500449 100644
--- a/audio/mixeng.c
+++ b/audio/mixeng.c
@@ -271,11 +271,11 @@ f_sample *mixeng_clip[2][2][2][3] = {
 #define CONV_NATURAL_FLOAT(x) (x)
 #define CLIP_NATURAL_FLOAT(x) (x)
 #else
-static const float float_scale = UINT_MAX;
+static const float float_scale = UINT_MAX / 2.f;
 #define CONV_NATURAL_FLOAT(x) ((x) * float_scale)
 
 #ifdef RECIPROCAL
-static const float float_scale_reciprocal = 1.f / UINT_MAX;
+static const float float_scale_reciprocal = 2.f / UINT_MAX;
 #define CLIP_NATURAL_FLOAT(x) ((x) * float_scale_reciprocal)
 #else
 #define CLIP_NATURAL_FLOAT(x) ((x) / float_scale)
diff --git a/audio/mixeng_template.h b/audio/mixeng_template.h
index 77cc89b9e8..fc8e1d4d9e 100644
--- a/audio/mixeng_template.h
+++ b/audio/mixeng_template.h
@@ -41,32 +41,31 @@ static inline mixeng_real glue (conv_, ET) (IN_T v)
 
 #ifdef RECIPROCAL
 #ifdef SIGNED
-return nv * (1.f / (mixeng_real) (IN_MAX - IN_MIN));
+return nv * (2.f / ((mixeng_real)IN_MAX - IN_MIN));
 #else
-return (nv - HALF) * (1.f / (mixeng_real) IN_MAX);
+return (nv - HALF) * (2.f / (mixeng_real)IN_MAX);
 #endif
 #else  /* !RECIPROCAL */
 #ifdef SIGNED
-return nv / (mixeng_real) ((mixeng_real) IN_MAX - IN_MIN);
+return nv / (((mixeng_real)IN_MAX - IN_MIN) / 2.f);
 #else
-return (nv - HALF) / (mixeng_real) IN_MAX;
+return (nv - HALF) / ((mixeng_real)IN_MAX / 2.f);
 #endif
 #endif
 }
 
 static inline IN_T glue (clip_, ET) (mixeng_real v)
 {
-if (v >= 0.5) {
+if (v >= 1.f) {
 return IN_MAX;
-}
-else if (v < -0.5) {
+} else if (v < -1.f) {
 return IN_MIN;
 }
 
 #ifdef SIGNED
-return ENDIAN_CONVERT ((IN_T) (v * ((mixeng_real) IN_MAX - IN_MIN)));
+return ENDIAN_CONVERT((IN_T)(v * (((mixeng_real)IN_MAX - IN_MIN) / 2.f)));
 #else
-return ENDIAN_CONVERT ((IN_T) ((v * IN_MAX) + HALF));
+return ENDIAN_CONVERT((IN_T)((v * ((mixeng_real)IN_MAX / 2.f)) + HALF));
 #endif
 }
 
-- 
2.16.4

[PATCH v2 5/6] audio: fix saturation nonlinearity in clip_* functions

[Volker Rümelin]

The current positive limit for the saturation nonlinearity is
only correct if the type of the result has 8 bits or less.

Signed-off-by: Volker Rümelin 
---
 audio/mixeng_template.h | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/audio/mixeng_template.h b/audio/mixeng_template.h
index fc8e1d4d9e..bc8509e423 100644
--- a/audio/mixeng_template.h
+++ b/audio/mixeng_template.h
@@ -83,10 +83,9 @@ static inline int64_t glue (conv_, ET) (IN_T v)
 
 static inline IN_T glue (clip_, ET) (int64_t v)
 {
-if (v >= 0x7f00) {
+if (v >= 0x7fffLL) {
 return IN_MAX;
-}
-else if (v < -2147483648LL) {
+} else if (v < -2147483648LL) {
 return IN_MIN;
 }
 
-- 
2.16.4

[PATCH v2 3/6] audio: consistency changes

[Volker Rümelin]

Change the clip_natural_float_from_mono() function in
audio/mixeng.c to be consistent with the clip_*_from_mono()
functions in audio/mixeng_template.h.

Signed-off-by: Volker Rümelin 
---
 audio/mixeng.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/audio/mixeng.c b/audio/mixeng.c
index b57fad83bf..725b529be7 100644
--- a/audio/mixeng.c
+++ b/audio/mixeng.c
@@ -316,7 +316,7 @@ static void clip_natural_float_from_mono(void *dst, const 
struct st_sample *src,
 float *out = (float *)dst;
 
 while (samples--) {
-*out++ = CLIP_NATURAL_FLOAT(src->l) + CLIP_NATURAL_FLOAT(src->r);
+*out++ = CLIP_NATURAL_FLOAT(src->l + src->r);
 src++;
 }
 }
-- 
2.16.4

[PATCH v2 6/6] audio: add audiodev format=f32 option documentation

[Volker Rümelin]

The documentaion for -audiodev format=f32 option was missing.

Signed-off-by: Volker Rümelin 
---
 qemu-options.hx | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index f9fefd43be..2919eddf4d 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -551,7 +551,7 @@ DEF("audiodev", HAS_ARG, QEMU_OPTION_audiodev,
 "in|out.frequency= frequency to use with fixed settings\n"
 "in|out.channels= number of channels to use with fixed 
settings\n"
 "in|out.format= sample format to use with fixed settings\n"
-"valid values: s8, s16, s32, u8, u16, u32\n"
+"valid values: s8, s16, s32, u8, u16, u32, f32\n"
 "in|out.voices= number of voices to use\n"
 "in|out.buffer-length= length of buffer in microseconds\n"
 "-audiodev none,id=id,[,prop[=value][,...]]\n"
@@ -647,7 +647,7 @@ SRST
 ``in|out.format=format``
 Specify the sample format to use when using fixed-settings.
 Valid values are: ``s8``, ``s16``, ``s32``, ``u8``, ``u16``,
-``u32``. Default is ``s16``.
+``u32``, ``f32``. Default is ``s16``.
 
 ``in|out.voices=voices``
 Specify the number of voices to use. Default is 1.
-- 
2.16.4

Re: [PATCH v4 0/3] delay timer_new from init to realize to fix memleaks.

[Pan Nengyuan]

On 3/8/2020 9:39 PM, Peter Maydell wrote:
> On Sun, 8 Mar 2020 at 11:58, Mark Cave-Ayland
>  wrote:
>> I just tried this patchset applied on top of git master and it causes 
>> qemu-system-ppc
>> to segfault on startup:
>>
>> $ gdb --args ./qemu-system-ppc
>> ...
>> ...
>> Thread 1 "qemu-system-ppc" received signal SIGSEGV, Segmentation fault.
>> 0x55e7e38c in timer_del (ts=0x0) at util/qemu-timer.c:429
>> 429 QEMUTimerList *timer_list = ts->timer_list;
>> (gdb) bt
>> #0  0x55e7e38c in timer_del (ts=0x0) at util/qemu-timer.c:429
>> #1  0x55b5d2c1 in mos6522_reset (dev=0x56e0ac50) at 
>> hw/misc/mos6522.c:468
>> #2  0x55b63570 in mos6522_cuda_reset (dev=0x56e0ac50) at
>> hw/misc/macio/cuda.c:599
> 
> It looks like we haven't caught all the cases of "somebody created a
> MOS6522 (or one of its subclasses) but forgot to realize it". This
> particular one I think is the s->cuda which is inited in macio_oldworld_init()
> but not realized in macio_oldworld_realize(). I think that pmu_init() in
> hw/misc/macio/pmu.c also has this bug. We need to go through and
> audit all the places where we create TYPE_MOS6522 or any of its
> subclasses and make sure they are also realizing the devices they create.
> (The presence of the new 3-phase reset infrastructure in the backtrace
> is a red herring here -- this would have crashed the same way with the
> old code too.)
> 

Yes, that's right, this series miss other cases, I will search and fix all
the cases about MOS6522 device.

Thanks.

> We should probably find some generic place in Device code where we
> can stick an assert "are we trying to reset an unrealized device?"
> because I bet we have other instances of this bug which we haven't
> noticed because the reset function happens to not misbehave on
> an inited-but-not-realized device...
> 
> thanks
> -- PMM
> .
> 

Re: [PATCH v4 2/3] mac_via: fix incorrect creation of mos6522 device in mac_via

[Pan Nengyuan]

On 3/8/2020 9:29 PM, Peter Maydell wrote:
> On Thu, 5 Mar 2020 at 06:39, Pan Nengyuan  wrote:
>>
>> This patch fix a bug in mac_via where it failed to actually realize devices 
>> it was using.
>> And move the init codes which inits the mos6522 objects and properties on 
>> them from realize()
>> into init(). However, we keep qdev_set_parent_bus in realize(), otherwise it 
>> will cause
>> device-introspect-test test fail. Then do the realize mos6522 device in the 
>> mac_vir_realize.
>>
>> Signed-off-by: Pan Nengyuan 
>> ---
>> Cc: Laurent Vivier 
>> Cc: Mark Cave-Ayland 
>> ---
> 
>> diff --git a/hw/misc/mac_via.c b/hw/misc/mac_via.c
>> index b7d0012794..4c5ad08805 100644
>> --- a/hw/misc/mac_via.c
>> +++ b/hw/misc/mac_via.c
>> @@ -868,24 +868,24 @@ static void mac_via_reset(DeviceState *dev)
>>  static void mac_via_realize(DeviceState *dev, Error **errp)
>>  {
>>  MacVIAState *m = MAC_VIA(dev);
>> -MOS6522State *ms;
>>  struct tm tm;
>>  int ret;
>> +Error *err = NULL;
>>
>> -/* Init VIAs 1 and 2 */
>> -sysbus_init_child_obj(OBJECT(dev), "via1", &m->mos6522_via1,
>> -  sizeof(m->mos6522_via1), TYPE_MOS6522_Q800_VIA1);
>> +qdev_set_parent_bus(DEVICE(&m->mos6522_via1), sysbus_get_default());
>> +qdev_set_parent_bus(DEVICE(&m->mos6522_via2), sysbus_get_default());
> 
> Rather than manually setting the parent bus, you can use
> sysbus_init_child_obj() instead of object_initialize_child() --
> it is a convenience function that does both object_initialize_child()
> and qdev_set_parent_bus() for you.

Actually I used sysbus_init_child_obj() first, but it will fail to run 
device-introspect-test.
Because qdev_set_parent_bus() will change 'info qtree' after we call 
'device-list-properties'.
Thus, I do it in the realize.

> 
>> -sysbus_init_child_obj(OBJECT(dev), "via2", &m->mos6522_via2,
>> -  sizeof(m->mos6522_via2), TYPE_MOS6522_Q800_VIA2);
>> +object_property_set_bool(OBJECT(&m->mos6522_via1), true, "realized", 
>> &err);
>> +if (err != NULL) {
>> +error_propagate(errp, err);
>> +return;
>> +}
>>
>> -/* Pass through mos6522 output IRQs */
>> -ms = MOS6522(&m->mos6522_via1);
>> -object_property_add_alias(OBJECT(dev), "irq[0]", OBJECT(ms),
>> -  SYSBUS_DEVICE_GPIO_IRQ "[0]", &error_abort);
>> -ms = MOS6522(&m->mos6522_via2);
>> -object_property_add_alias(OBJECT(dev), "irq[1]", OBJECT(ms),
>> -  SYSBUS_DEVICE_GPIO_IRQ "[0]", &error_abort);
>> +object_property_set_bool(OBJECT(&m->mos6522_via2), true, "realized", 
>> &err);
>> +if (err != NULL) {
>> +error_propagate(errp, err);
>> +return;
>> +}
>>
>>  /* Pass through mos6522 input IRQs */
>>  qdev_pass_gpios(DEVICE(&m->mos6522_via1), dev, "via1-irq");
>> @@ -932,6 +932,7 @@ static void mac_via_init(Object *obj)
>>  {
>>  SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
>>  MacVIAState *m = MAC_VIA(obj);
>> +MOS6522State *ms;
>>
>>  /* MMIO */
>>  memory_region_init(&m->mmio, obj, "mac-via", 2 * VIA_SIZE);
>> @@ -948,6 +949,22 @@ static void mac_via_init(Object *obj)
>>  /* ADB */
>>  qbus_create_inplace((BusState *)&m->adb_bus, sizeof(m->adb_bus),
>>  TYPE_ADB_BUS, DEVICE(obj), "adb.0");
>> +
>> +/* Init VIAs 1 and 2 */
>> +object_initialize_child(OBJECT(m), "via1", &m->mos6522_via1,
>> +sizeof(m->mos6522_via1), TYPE_MOS6522_Q800_VIA1,
>> +&error_abort, NULL);
>> +object_initialize_child(OBJECT(m), "via2", &m->mos6522_via2,
>> +sizeof(m->mos6522_via2), TYPE_MOS6522_Q800_VIA2,
>> +&error_abort, NULL);
>> +
>> +/* Pass through mos6522 output IRQs */
>> +ms = MOS6522(&m->mos6522_via1);
>> +object_property_add_alias(OBJECT(m), "irq[0]", OBJECT(ms),
>> +  SYSBUS_DEVICE_GPIO_IRQ "[0]", &error_abort);
> 
> There's no point in using the MOS6522() cast to get a MOS6522*,
> because the only thing you do with it is immediately cast it
> to an Object* with the OBJECT() cast. Just use the OBJECT cast directly.

Ok, will do it.

Thanks.

> 
>> +ms = MOS6522(&m->mos6522_via2);
>> +object_property_add_alias(OBJECT(m), "irq[1]", OBJECT(ms),
>> +  SYSBUS_DEVICE_GPIO_IRQ "[0]", &error_abort);
>>  }
>>
>>  static void postload_update_cb(void *opaque, int running, RunState state)
> 
> thanks
> -- PMM
> .
> 

[Bug 1866577] [NEW] powerpc-none-eabi-gdb.exe GDB 9.1 with QEMU 4.2 gdb-stub comes with Reply contains invalid hex digit 79

[Yonggang Luo]

Public bug reported:

I am using powerpc-none-eabi-gdb with qemu 4.2, but it comes with 
the following error:

undefinedC:\CI-Tools\msys64\powerpc-none-eabi\usr\local\bin\powerpc-
none-eabi-gdb.exe: warning: Couldn't determine a path for the index
cache directory.

```Not implemented stop reason (assuming exception): undefined```
The target architecture is assumed to be powerpc:603

```
Reply contains invalid hex digit 79
```

** 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/1866577

Title:
  powerpc-none-eabi-gdb.exe GDB 9.1 with QEMU 4.2 gdb-stub comes with
  Reply contains invalid hex digit 79

Status in QEMU:
  New

Bug description:
  I am using powerpc-none-eabi-gdb with qemu 4.2, but it comes with 
  the following error:

  undefinedC:\CI-Tools\msys64\powerpc-none-eabi\usr\local\bin\powerpc-
  none-eabi-gdb.exe: warning: Couldn't determine a path for the index
  cache directory.

  ```Not implemented stop reason (assuming exception): undefined```
  The target architecture is assumed to be powerpc:603

  ```
  Reply contains invalid hex digit 79
  ```

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

[PATCH v3] virtio-serial-bus: Plug memory leak on realize() error paths

[Pan Nengyuan]

We neglect to free port->bh on the error paths.  Fix that.
Reproducer:
{'execute': 'device_add', 'arguments': {'id': 'virtio_serial_pci0', 
'driver': 'virtio-serial-pci', 'bus': 'pci.0', 'addr': '0x5'}, 'id': 'yVkZcGgV'}
{'execute': 'device_add', 'arguments': {'id': 'port1', 'driver': 
'virtserialport', 'name': 'port1', 'chardev': 'channel1', 'bus': 
'virtio_serial_pci0.0', 'nr': 1}, 'id': '3dXdUgJA'}
{'execute': 'device_add', 'arguments': {'id': 'port2', 'driver': 
'virtserialport', 'name': 'port2', 'chardev': 'channel2', 'bus': 
'virtio_serial_pci0.0', 'nr': 1}, 'id': 'qLzcCkob'}
{'execute': 'device_add', 'arguments': {'id': 'port2', 'driver': 
'virtserialport', 'name': 'port2', 'chardev': 'channel2', 'bus': 
'virtio_serial_pci0.0', 'nr': 2}, 'id': 'qLzcCkob'}

The leak stack:
Direct leak of 40 byte(s) in 1 object(s) allocated from:
#0 0x7f04a8008ae8 in __interceptor_malloc (/lib64/libasan.so.5+0xefae8)
#1 0x7f04a73cf1d5 in g_malloc (/lib64/libglib-2.0.so.0+0x531d5)
#2 0x56273eaee484 in aio_bh_new /mnt/sdb/backup/qemu/util/async.c:125
#3 0x56273eafe9a8 in qemu_bh_new /mnt/sdb/backup/qemu/util/main-loop.c:532
#4 0x56273d52e62e in virtser_port_device_realize 
/mnt/sdb/backup/qemu/hw/char/virtio-serial-bus.c:946
#5 0x56273dcc5040 in device_set_realized 
/mnt/sdb/backup/qemu/hw/core/qdev.c:891
#6 0x56273e5ebbce in property_set_bool 
/mnt/sdb/backup/qemu/qom/object.c:2238
#7 0x56273e5e5a9c in object_property_set 
/mnt/sdb/backup/qemu/qom/object.c:1324
#8 0x56273e5ef5f8 in object_property_set_qobject 
/mnt/sdb/backup/qemu/qom/qom-qobject.c:26
#9 0x56273e5e5e6a in object_property_set_bool 
/mnt/sdb/backup/qemu/qom/object.c:1390
#10 0x56273daa40de in qdev_device_add 
/mnt/sdb/backup/qemu/qdev-monitor.c:680
#11 0x56273daa53e9 in qmp_device_add /mnt/sdb/backup/qemu/qdev-monitor.c:805

Fixes: 199646d81522509ac2dba6d28c31e8c7d807bc93
Reported-by: Euler Robot 
Signed-off-by: Pan Nengyuan 
Reviewed-by: Markus Armbruster 
Reviewed-by: Amit Shah 
---
v1->v2:
- simply create port->bh last in virtser_port_device_realize() to fix 
memleaks.(Suggested by Markus Armbruster)
v3->v2:
- tidy up commit message
---
 hw/char/virtio-serial-bus.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
index 941ed5aca9..99a65bab7f 100644
--- a/hw/char/virtio-serial-bus.c
+++ b/hw/char/virtio-serial-bus.c
@@ -943,7 +943,6 @@ static void virtser_port_device_realize(DeviceState *dev, 
Error **errp)
 Error *err = NULL;
 
 port->vser = bus->vser;
-port->bh = qemu_bh_new(flush_queued_data_bh, port);
 
 assert(vsc->have_data);
 
@@ -992,6 +991,7 @@ static void virtser_port_device_realize(DeviceState *dev, 
Error **errp)
 return;
 }
 
+port->bh = qemu_bh_new(flush_queued_data_bh, port);
 port->elem = NULL;
 }
 
-- 
2.18.2

Re: [PATCH v2] hw/char/pl011: Enable TxFIFO and async transmission

[Gavin Shan]

On 3/6/20 10:15 PM, Marc-André Lureau wrote:


On Mon, Feb 24, 2020 at 4:13 AM Gavin Shan  wrote:


The depth of TxFIFO can be 1 or 16 depending on LCR[4]. The TxFIFO is
disabled when its depth is 1. It's nice to have TxFIFO enabled if
possible because more characters can be piled and transmitted at once,
which would have less overhead. Besides, we can be blocked because of
qemu_chr_fe_write_all(), which isn't nice.

This enables TxFIFO if possible. On ther other hand, the asynchronous
transmission is enabled if needed, as we did in hw/char/cadence_uart.c

Signed-off-by: Gavin Shan 
---
v2: Put write_{count,fifo} into migration subsection
 Don't start async IO handle if it has been started, to avoid race
 Update with PL011_FLAG_{TXFF,TXFE} on changing write_count
---
  hw/char/pl011.c | 105 +---
  include/hw/char/pl011.h |   3 ++
  2 files changed, 102 insertions(+), 6 deletions(-)

diff --git a/hw/char/pl011.c b/hw/char/pl011.c
index 13e784f9d9..de5c4254fe 100644
--- a/hw/char/pl011.c
+++ b/hw/char/pl011.c
@@ -169,6 +169,73 @@ static void pl011_set_read_trigger(PL011State *s)
  s->read_trigger = 1;
  }

+static gboolean pl011_xmit(GIOChannel *chan, GIOCondition cond, void *opaque)
+{
+PL011State *s = (PL011State *)opaque;


Useless casts, perhaps use PL011() instead?



Yes, PL011() makes the code a bit more safer. It will be used in next respin.


+int ret;
+
+/* Drain FIFO if there is no backend */
+if (!qemu_chr_fe_backend_connected(&s->chr)) {
+s->write_count = 0;
+s->flags &= ~PL011_FLAG_TXFF;
+s->flags |= PL011_FLAG_TXFE;
+return FALSE;


Perhaps use G_SOURCE_REMOVE ?



Yes, it's more precise even it's equal to false. It will be used in next respin.


+}
+
+/* Nothing to do */
+if (!s->write_count) {
+return FALSE;
+}
+
+ret = qemu_chr_fe_write(&s->chr, s->write_fifo, s->write_count);
+if (ret > 0) {
+s->write_count -= ret;
+memmove(s->write_fifo, s->write_fifo + ret, s->write_count);
+s->flags &= ~PL011_FLAG_TXFF;
+if (!s->write_count) {
+s->flags |= PL011_FLAG_TXFE;
+}
+}
+
+if (s->write_count) {
+s->watch_tag = qemu_chr_fe_add_watch(&s->chr, G_IO_OUT | G_IO_HUP,
+ pl011_xmit, s);
+if (!s->watch_tag) {
+s->write_count = 0;
+s->flags &= ~PL011_FLAG_TXFF;
+s->flags |= PL011_FLAG_TXFE;
+return FALSE;
+}
+}
+
+s->int_level |= PL011_INT_TX;
+pl011_update(s);
+return FALSE;
+}
+
+static void pl011_write_fifo(void *opaque, const unsigned char *buf, int size)
+{
+PL011State *s = (PL011State *)opaque;
+int depth = (s->lcr & 0x10) ? 16 : 1;
+
+if (size >= (depth - s->write_count)) {


parenthesis could be dropped



It's really personal taste, but it will be dropped in next respin :)


+size = depth - s->write_count;
+}
+


Why make a function that accepts size != 1 (and may silently drop
data), when the only caller is pl011_write_fifo(opaque, &ch, 1); ?


+if (size > 0) {


I don't think other cases are supposed to happen.



The function will be reused when loopback mode is enabled in future. The
received characters could be sent in batch mode in loopback mode. So we
have variable size here. The caller won't lose data by checking the amount
of characters are sent successfully. Also, the check on @size makes the code
looks complete and lets keep it.


+memcpy(s->write_fifo + s->write_count, buf, size);
+s->write_count += size;
+if (s->write_count >= depth) {
+s->flags |= PL011_FLAG_TXFF;
+}
+s->flags &= ~PL011_FLAG_TXFE;
+}
+
+if (!s->watch_tag) {
+pl011_xmit(NULL, G_IO_OUT, s);
+}
+}
+
  static void pl011_write(void *opaque, hwaddr offset,
  uint64_t value, unsigned size)
  {
@@ -179,13 +246,8 @@ static void pl011_write(void *opaque, hwaddr offset,

  switch (offset >> 2) {
  case 0: /* UARTDR */
-/* ??? Check if transmitter is enabled.  */
  ch = value;
-/* XXX this blocks entire thread. Rewrite to use
- * qemu_chr_fe_write and background I/O callbacks */
-qemu_chr_fe_write_all(&s->chr, &ch, 1);
-s->int_level |= PL011_INT_TX;
-pl011_update(s);
+pl011_write_fifo(opaque, &ch, 1);
  break;
  case 1: /* UARTRSR/UARTECR */
  s->rsr = 0;
@@ -207,7 +269,16 @@ static void pl011_write(void *opaque, hwaddr offset,
  if ((s->lcr ^ value) & 0x10) {
  s->read_count = 0;
  s->read_pos = 0;
+
+if (s->watch_tag) {
+g_source_remove(s->watch_tag);
+s->watch_tag = 0;
+}
+s->write_count = 0;
+s->flags &= ~PL011_FLAG_TXFF;
+s->flags |= PL011_FLAG_TXFE;
  }

[PATCH v2 0/2] Implement "non 100% native mode" in via-ide

[BALATON Zoltan]

This small series implements the quirky mode of via-ide found at least
on pegasos2 which is needed for guests that expect this and activate
work arounds on that platform and don't work unless this is emulated.
(Symptom is missing IDE interrupts.) We need a flag to turn this mode
on or off so the first patch repurposes the last remaining CMD646
specific field in PCIIDEState to allow more flags and make room for
the new legacy-irq flag there. (The CMD646 may need similar mode or
something else may need more flags in the future.) Boards using CMD646
and VIA IDE are updated for the above changes.

Tested with Linux and MorphOS on pegasos2 and a Gentoo live CD kernel
for mips_fulong2e that's the only one I could find but being beta not
sure if that fully works on real hardware. (The mips_fulong2e also
seems to have problems with pci devices so to boot Linux you need
-net none -vga none and use serial console otherwise the kernel panics.)

Regards,
BALATON Zoltan

BALATON Zoltan (2):
  ide: Make room for flags in PCIIDEState and add one for legacy IRQ
routing
  via-ide: Also emulate non 100% native mode

 hw/alpha/dp264.c|  2 +-
 hw/ide/cmd646.c | 12 -
 hw/ide/via.c| 57 +++--
 hw/mips/mips_fulong2e.c |  2 +-
 hw/sparc64/sun4u.c  |  9 ++-
 include/hw/ide.h|  7 ++---
 include/hw/ide/pci.h|  7 -
 7 files changed, 69 insertions(+), 27 deletions(-)

-- 
2.21.1

[PATCH v2 1/2] ide: Make room for flags in PCIIDEState and add one for legacy IRQ routing

[BALATON Zoltan]

We'll need a flag for implementing some device specific behaviour in
via-ide but we already have a currently CMD646 specific field that can
be repurposed for this and leave room for furhter flags if needed in
the future. This patch changes the "secondary" field to "flags" and
define the flags for CMD646 and via-ide and change CMD646 and its
users accordingly.

Signed-off-by: BALATON Zoltan 
---
 hw/alpha/dp264.c |  2 +-
 hw/ide/cmd646.c  | 12 ++--
 hw/sparc64/sun4u.c   |  9 ++---
 include/hw/ide.h |  4 ++--
 include/hw/ide/pci.h |  7 ++-
 5 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/hw/alpha/dp264.c b/hw/alpha/dp264.c
index d28f57199f..9c90b9355b 100644
--- a/hw/alpha/dp264.c
+++ b/hw/alpha/dp264.c
@@ -102,7 +102,7 @@ static void clipper_init(MachineState *machine)
 DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
 ide_drive_get(hd, ARRAY_SIZE(hd));
 
-pci_cmd646_ide_init(pci_bus, hd, 0);
+pci_cmd646_ide_init(pci_bus, hd, -1, false);
 }
 
 /* Load PALcode.  Given that this is not "real" cpu palcode,
diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c
index 335c060673..0be650791f 100644
--- a/hw/ide/cmd646.c
+++ b/hw/ide/cmd646.c
@@ -256,7 +256,7 @@ static void pci_cmd646_ide_realize(PCIDevice *dev, Error 
**errp)
 pci_conf[PCI_CLASS_PROG] = 0x8f;
 
 pci_conf[CNTRL] = CNTRL_EN_CH0; // enable IDE0
-if (d->secondary) {
+if (d->flags & BIT(PCI_IDE_SECONDARY)) {
 /* XXX: if not enabled, really disable the seconday IDE controller */
 pci_conf[CNTRL] |= CNTRL_EN_CH1; /* enable IDE1 */
 }
@@ -317,20 +317,20 @@ static void pci_cmd646_ide_exitfn(PCIDevice *dev)
 }
 }
 
-void pci_cmd646_ide_init(PCIBus *bus, DriveInfo **hd_table,
- int secondary_ide_enabled)
+void pci_cmd646_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn,
+ bool secondary_ide_enabled)
 {
 PCIDevice *dev;
 
-dev = pci_create(bus, -1, "cmd646-ide");
-qdev_prop_set_uint32(&dev->qdev, "secondary", secondary_ide_enabled);
+dev = pci_create(bus, devfn, "cmd646-ide");
+qdev_prop_set_bit(&dev->qdev, "secondary", secondary_ide_enabled);
 qdev_init_nofail(&dev->qdev);
 
 pci_ide_create_devs(dev, hd_table);
 }
 
 static Property cmd646_ide_properties[] = {
-DEFINE_PROP_UINT32("secondary", PCIIDEState, secondary, 0),
+DEFINE_PROP_BIT("secondary", PCIIDEState, flags, PCI_IDE_SECONDARY, false),
 DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index d33e84f831..fbe6790847 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -50,8 +50,7 @@
 #include "hw/sparc/sparc64.h"
 #include "hw/nvram/fw_cfg.h"
 #include "hw/sysbus.h"
-#include "hw/ide.h"
-#include "hw/ide/pci.h"
+#include "hw/ide/internal.h"
 #include "hw/loader.h"
 #include "hw/fw-path-provider.h"
 #include "elf.h"
@@ -664,11 +663,7 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
 }
 
 ide_drive_get(hd, ARRAY_SIZE(hd));
-
-pci_dev = pci_create(pci_busA, PCI_DEVFN(3, 0), "cmd646-ide");
-qdev_prop_set_uint32(&pci_dev->qdev, "secondary", 1);
-qdev_init_nofail(&pci_dev->qdev);
-pci_ide_create_devs(pci_dev, hd);
+pci_cmd646_ide_init(pci_busA, hd, PCI_DEVFN(3, 0), true);
 
 /* Map NVRAM into I/O (ebus) space */
 nvram = m48t59_init(NULL, 0, 0, NVRAM_SIZE, 1968, 59);
diff --git a/include/hw/ide.h b/include/hw/ide.h
index 28d8a06439..d88c5ee695 100644
--- a/include/hw/ide.h
+++ b/include/hw/ide.h
@@ -12,8 +12,8 @@ ISADevice *isa_ide_init(ISABus *bus, int iobase, int iobase2, 
int isairq,
 DriveInfo *hd0, DriveInfo *hd1);
 
 /* ide-pci.c */
-void pci_cmd646_ide_init(PCIBus *bus, DriveInfo **hd_table,
- int secondary_ide_enabled);
+void pci_cmd646_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn,
+ bool secondary_ide_enabled);
 PCIDevice *pci_piix3_xen_ide_init(PCIBus *bus, DriveInfo **hd_table, int 
devfn);
 PCIDevice *pci_piix3_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn);
 PCIDevice *pci_piix4_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn);
diff --git a/include/hw/ide/pci.h b/include/hw/ide/pci.h
index a9f2c33e68..21075edf16 100644
--- a/include/hw/ide/pci.h
+++ b/include/hw/ide/pci.h
@@ -40,6 +40,11 @@ typedef struct BMDMAState {
 #define TYPE_PCI_IDE "pci-ide"
 #define PCI_IDE(obj) OBJECT_CHECK(PCIIDEState, (obj), TYPE_PCI_IDE)
 
+enum {
+PCI_IDE_SECONDARY, /* used only for cmd646 */
+PCI_IDE_LEGACY_IRQ
+};
+
 typedef struct PCIIDEState {
 /*< private >*/
 PCIDevice parent_obj;
@@ -47,7 +52,7 @@ typedef struct PCIIDEState {
 
 IDEBus bus[2];
 BMDMAState bmdma[2];
-uint32_t secondary; /* used only for cmd646 */
+uint32_t flags;
 MemoryRegion bmdma_bar;
 MemoryRegion cmd_bar[2];
 MemoryRegion data_bar[2];
-- 
2.21.1

[PATCH v2 2/2] via-ide: Also emulate non 100% native mode

[BALATON Zoltan]

Some machines operate in "non 100% native mode" where interrupts are
fixed at legacy IDE interrupts and some guests expect this behaviour
without checking based on knowledge about hardware. Even Linux has
arch specific workarounds for this that are activated on such boards
so this needs to be emulated as well.

Signed-off-by: BALATON Zoltan 
---
v2: Don't use PCI_INTERRUPT_LINE in via_ide_set_irq()

 hw/ide/via.c| 57 +++--
 hw/mips/mips_fulong2e.c |  2 +-
 include/hw/ide.h|  3 ++-
 3 files changed, 52 insertions(+), 10 deletions(-)

diff --git a/hw/ide/via.c b/hw/ide/via.c
index 096de8dba0..44ecc2af29 100644
--- a/hw/ide/via.c
+++ b/hw/ide/via.c
@@ -1,9 +1,10 @@
 /*
- * QEMU IDE Emulation: PCI VIA82C686B support.
+ * QEMU VIA southbridge IDE emulation (VT82C686B, VT8231)
  *
  * Copyright (c) 2003 Fabrice Bellard
  * Copyright (c) 2006 Openedhand Ltd.
  * Copyright (c) 2010 Huacai Chen 
+ * Copyright (c) 2019-2020 BALATON Zoltan
  *
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the "Software"), to 
deal
@@ -25,6 +26,8 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/range.h"
+#include "hw/qdev-properties.h"
 #include "hw/pci/pci.h"
 #include "migration/vmstate.h"
 #include "qemu/module.h"
@@ -111,14 +114,40 @@ static void via_ide_set_irq(void *opaque, int n, int 
level)
 } else {
 d->config[0x70 + n * 8] &= ~0x80;
 }
-
 level = (d->config[0x70] & 0x80) || (d->config[0x78] & 0x80);
-n = pci_get_byte(d->config + PCI_INTERRUPT_LINE);
-if (n) {
-qemu_set_irq(isa_get_irq(NULL, n), level);
+
+/*
+ * Some machines operate in "non 100% native mode" where PCI_INTERRUPT_LINE
+ * is not used but IDE always uses ISA IRQ 14 and 15 even in native mode.
+ * Some guest drivers expect this, often without checking.
+ */
+if (!(pci_get_byte(d->config + PCI_CLASS_PROG) & (n ? 4 : 1)) ||
+PCI_IDE(d)->flags & BIT(PCI_IDE_LEGACY_IRQ)) {
+qemu_set_irq(isa_get_irq(NULL, (n ? 15 : 14)), level);
+} else {
+qemu_set_irq(isa_get_irq(NULL, 14), level);
 }
 }
 
+static uint32_t via_ide_config_read(PCIDevice *d, uint32_t address, int len)
+{
+/*
+ * The pegasos2 firmware writes to PCI_INTERRUPT_LINE but on real
+ * hardware it's fixed at 14 and won't change. Some guests also expect
+ * legacy interrupts, without reading PCI_INTERRUPT_LINE but Linux
+ * depends on this to read 14. We set it to 14 in the reset method and
+ * also set the wmask to 0 to emulate this but that turns out to be not
+ * enough. QEMU resets the PCI bus after this device and
+ * pci_do_device_reset() called from pci_device_reset() will zero
+ * PCI_INTERRUPT_LINE so this config_read function is to counter that and
+ * restore the correct value, otherwise this should not be needed.
+ */
+if (range_covers_byte(address, len, PCI_INTERRUPT_LINE)) {
+pci_set_byte(d->config + PCI_INTERRUPT_LINE, 14);
+}
+return pci_default_read_config(d, address, len);
+}
+
 static void via_ide_reset(DeviceState *dev)
 {
 PCIIDEState *d = PCI_IDE(dev);
@@ -169,7 +198,8 @@ static void via_ide_realize(PCIDevice *dev, Error **errp)
 
 pci_config_set_prog_interface(pci_conf, 0x8f); /* native PCI ATA mode */
 pci_set_long(pci_conf + PCI_CAPABILITY_LIST, 0x00c0);
-dev->wmask[PCI_INTERRUPT_LINE] = 0xf;
+dev->wmask[PCI_CLASS_PROG] = 5;
+dev->wmask[PCI_INTERRUPT_LINE] = 0;
 
 memory_region_init_io(&d->data_bar[0], OBJECT(d), &pci_ide_data_le_ops,
   &d->bus[0], "via-ide0-data", 8);
@@ -213,14 +243,23 @@ static void via_ide_exitfn(PCIDevice *dev)
 }
 }
 
-void via_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn)
+void via_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn,
+  bool legacy_irq)
 {
 PCIDevice *dev;
 
-dev = pci_create_simple(bus, devfn, "via-ide");
+dev = pci_create(bus, devfn, "via-ide");
+qdev_prop_set_bit(&dev->qdev, "legacy-irq", legacy_irq);
+qdev_init_nofail(&dev->qdev);
 pci_ide_create_devs(dev, hd_table);
 }
 
+static Property via_ide_properties[] = {
+DEFINE_PROP_BIT("legacy-irq", PCIIDEState, flags, PCI_IDE_LEGACY_IRQ,
+false),
+DEFINE_PROP_END_OF_LIST(),
+};
+
 static void via_ide_class_init(ObjectClass *klass, void *data)
 {
 DeviceClass *dc = DEVICE_CLASS(klass);
@@ -229,10 +268,12 @@ static void via_ide_class_init(ObjectClass *klass, void 
*data)
 dc->reset = via_ide_reset;
 k->realize = via_ide_realize;
 k->exit = via_ide_exitfn;
+k->config_read = via_ide_config_read;
 k->vendor_id = PCI_VENDOR_ID_VIA;
 k->device_id = PCI_DEVICE_ID_VIA_IDE;
 k->revision = 0x06;
 k->class_id = PCI_CLASS_STORAGE_IDE;
+device_class_set_props(dc, via_ide_properties);
 set_bit(DEVICE_CATEGORY_STORAGE, dc->categor

Re: [PATCH v3] virtio-serial-bus: Plug memory leak on realize() error paths

[no-reply]

Patchew URL: 
https://patchew.org/QEMU/20200309021738.30072-1-pannengy...@huawei.com/



Hi,

This series failed the docker-clang@ubuntu build test. Please find the testing 
commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-ubuntu V=1 NETWORK=1
time make docker-test-clang@ubuntu SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===

  LINKfsdev/virtfs-proxy-helper
  LINKscsi/qemu-pr-helper
  LINKqemu-bridge-helper
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of 
`rpc_createerr@@GLIBC_2.2.5' overridden by definition from 
/lib/x86_64-linux-gnu/libc.so.6
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of 
`rpc_createerr@@GLIBC_2.2.5' overridden by definition from 
/lib/x86_64-linux-gnu/libc.so.6
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of 
`rpc_createerr@@GLIBC_2.2.5' overridden by definition from 
/lib/x86_64-linux-gnu/libc.so.6
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of 
`rpc_createerr@@GLIBC_2.2.5' overridden by definition from 
/lib/x86_64-linux-gnu/libc.so.6
  LINKvirtiofsd
  GEN lm32-softmmu/hmp-commands.h
  GEN cris-softmmu/hmp-commands.h
---
  CC  arm-softmmu/hw/vfio/display.o
  CC  aarch64-softmmu/hw/vfio/pci.o
  CC  aarch64-softmmu/hw/vfio/pci-quirks.o
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of 
`rpc_createerr@@GLIBC_2.2.5' overridden by definition from 
/lib/x86_64-linux-gnu/libc.so.6
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of 
`rpc_createerr@@GLIBC_2.2.5' overridden by definition from 
/lib/x86_64-linux-gnu/libc.so.6
/usr/bin/ld/usr/bin/ld: : 
/lib/x86_64-linux-gnu/libtirpc.so.3/lib/x86_64-linux-gnu/libtirpc.so.3: 
warning: common of `: warning: common of 
`rpc_createerr@@GLIBC_2.2.5rpc_createerr@@GLIBC_2.2.5' overridden by definition 
from ' overridden by definition from 
/lib/x86_64-linux-gnu/libc.so.6/lib/x86_64-linux-gnu/libc.so.6

/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of 
`rpc_createerr@@GLIBC_2.2.5' overridden by definition from 
/lib/x86_64-linux-gnu/libc.so.6
  CC  i386-softmmu/hw/virtio/vhost-user-fs.o
  CC  mips-softmmu/hw/virtio/vhost-vsock-pci.o
  CC  alpha-softmmu/hw/virtio/vhost-user-scsi-pci.o
---
  CC  i386-softmmu/target/i386/hyperv.o
  CC  ppc-softmmu/hw/display/virtio-gpu-pci.o
  CC  ppc64-softmmu/hw/display/vhost-user-gpu.o
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of 
`rpc_createerr@@GLIBC_2.2.5' overridden by definition from 
/lib/x86_64-linux-gnu/libc.so.6
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of 
`rpc_createerr@@GLIBC_2.2.5' overridden by definition from 
/lib/x86_64-linux-gnu/libc.so.6
  CC  mipsel-softmmu/hw/virtio/virtio.o
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of 
`rpc_createerr@@GLIBC_2.2.5' overridden by definition from 
/lib/x86_64-linux-gnu/libc.so.6
  CC  aarch64-softmmu/hw/arm/mcimx7d-sabre.o
  CC  arm-softmmu/hw/arm/smmuv3.o
  CC  mips64el-softmmu/qapi/qapi-types-machine-target.o
  CC  or1k-softmmu/target/openrisc/exception_helper.o
  CC  nios2-softmmu/target/nios2/cpu.o
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of 
`rpc_createerr@@GLIBC_2.2.5' overridden by definition from 
/lib/x86_64-linux-gnu/libc.so.6
  CC  i386-softmmu/target/i386/sev.o
  CC  mips64el-softmmu/qapi/qapi-types-misc-target.o
  CC  ppc64-softmmu/hw/display/virtio-gpu-pci.o
  CC  mipsel-softmmu/hw/virtio/vhost.o
  CC  ppc-softmmu/hw/display/vhost-user-gpu-pci.o
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of 
`rpc_createerr@@GLIBC_2.2.5' overridden by definition from 
/lib/x86_64-linux-gnu/libc.so.6
  CC  mips64el-softmmu/qapi/qapi-types.o
  CC  aarch64-softmmu/hw/arm/smmu-common.o
  CC  arm-softmmu/hw/arm/fsl-imx6ul.o
---
  CC  mipsel-softmmu/hw/virtio/vhost-user-blk-pci.o
  CC  ppc-softmmu/hw/vfio/platform.o
  CC  riscv32-softmmu/tcg/tcg.o
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of 
`rpc_createerr@@GLIBC_2.2.5' overridden by definition from 
/lib/x86_64-linux-gnu/libc.so.6
  CC  mips64el-softmmu/target/mips/helper.o
  CC  mipsel-softmmu/hw/virtio/vhost-user-input-pci.o
  CC  aarch64-softmmu/qapi/qapi-events.o
---
  CC  aarch64-softmmu/qapi/qapi-commands-misc-target.o
  CC  mips64el-softmmu/target/mips/cp0_helper.o
  CC  ppc64-softmmu/hw/nvram/spapr_nvram.o
/usr/bin/ld: /lib/x86_64-linux-gnu/libtirpc.so.3: warning: common of 
`rpc_createerr@@GLIBC_2.2.5' overridden by definition from 
/lib/x86_64-linux-gnu/libc.so.6
  CC  arm-softmmu/softmmu/vl.o
  CC  aarch64-softmmu/qapi/qapi-commands.o
  CC  riscv32-softmmu/tcg/tcg-op.o
---
  CC  aarch64-softmmu/target/arm/neon_helper.o
  CC  arm-softmmu/target/arm/vec_he

Re: [PATCH] dp8393x: Mask EOL bit from descriptor addresses, take 2

[Jason Wang]

On 2020/3/8 下午5:52, Laurent Vivier wrote:

Le 04/03/2020 à 04:23, Finn Thain a écrit :

A portion of a recent patch got lost due to a merge snafu. That patch is
now commit 88f632fbb1 ("dp8393x: Mask EOL bit from descriptor addresses").
This patch restores the portion that got lost.

Signed-off-by: Finn Thain 
---
  hw/net/dp8393x.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
index 8a3504d962..81fc13ee9f 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -525,8 +525,8 @@ static void dp8393x_do_transmit_packets(dp8393xState *s)
   * (4 + 3 * s->regs[SONIC_TFC]),
 MEMTXATTRS_UNSPECIFIED, s->data,
 size);
-s->regs[SONIC_CTDA] = dp8393x_get(s, width, 0) & ~0x1;
-if (dp8393x_get(s, width, 0) & SONIC_DESC_EOL) {
+s->regs[SONIC_CTDA] = dp8393x_get(s, width, 0);
+if (s->regs[SONIC_CTDA] & SONIC_DESC_EOL) {
  /* EOL detected */
  break;
  }


Jason,

as it's a trivial bug fixes (only a diff between the commit and the
patch), will you merge it via the network queue or do you want I take it
via trivial queue?



Hi Laurent:

Please merge it.

Thanks




Thanks,
Laurent

I am trying to fixes a issue with QEMU with VxWorks.

[Yonggang Luo]

When I am running QEMU to simulating PowerPC.
And after running the following powerpc code:
00e2b5dc :
intUnlock():
  e2b5dc: 54 63 04 20 rlwinm r3,r3,0,16,16
  e2b5e0: 7c 80 00 a6 mfmsr r4
  e2b5e4: 7c 83 1b 78 or r3,r4,r3
  e2b5e8: 7c 60 01 24 mtmsr r3
  e2b5ec: 4c 00 01 2c isync
  e2b5f0: 4e 80 00 20 blr

The QEMU are getting stuck and can not running the following instructions,
What I need to do to inspect which instruction are getting stuck and how to
fix it?
Any means to debugging that.

-- 
 此致
礼
罗勇刚
Yours
sincerely,
Yonggang Luo

[Question] An issue when repeat reboot in guest during migration

[Longpeng (Mike, Cloud Infrastructure Service Product Dept.)]

Hi guys,

We find an issue when repeat reboot in guest during migration, it cause the
migration thread never be waken up again.

|
   |
main_loop_should_exit [BQL LOCK]   |
 pause_all_vcpus   |
  1. set all cpus ->stop=true  |
 and then kick |
  2. return if all cpus is paused  |
 (by '->stopped == true'), else|
  3. qemu_cond_wait [BQL UNLOCK]   |
   |LOCK BQL
   |...
   |do_vm_stop
   | pause_all_vcpus
   |  (A)set all cpus ->stop=true
   | and then kick
   |  (B)return if all cpus is paused
   | (by '->stopped == true'), else
   |  (C)qemu_cond_wait [BQL UNLOCK]
  4. be waken up and LOCK BQL  |  (D)be waken up BUT wait for  BQL
  5. goto 2.   |
 (BQL is still LOCKed) |
 resume_all_vcpus  |
  1. set all cpus ->stop=false |
 and ->stopped=false   |
...|
BQL UNLOCK |  (E)LOCK BQL
   |  (F)goto B. [but stopped is false now!]
   |Finally, sleep at step 3 forever.

I've not test the latest QEMU yet, but after take a quick look at codes, it
seems also has this issue.

Currently I just find two potential approaches to fix it:

1. Just retry. Check if all vcpus are paused before (F), if not then goto (A) to
send the stop request again. It's very simple and safe but it just reduce the
probability.

2. To support nest pause. Use a refcount to instead of the 'bool stop', switch
the 'stopped' to false only when the refcount reduce to zero. Maybe this can
completely solve the problem but it seems easy to introduce other bugs if
imprudence.

Any suggestions ?

Thanks.

---
Regards,
Longpeng(Mike)

Re: [PATCH v32 16/22] hw/char: RX62N serial communication interface (SCI)

[Yoshinori Sato]

On Mon, 09 Mar 2020 00:41:45 +0900,
Philippe Mathieu-Daudé wrote:
> 
> Hi Yoshinori,
> 
> On 2/24/20 3:19 PM, Yoshinori Sato wrote:
> > This module supported only non FIFO type.
> > Hardware manual.
> > https://www.renesas.com/us/en/doc/products/mpumcu/doc/rx_family/r01uh0033ej0140_rx62n.pdf
> > 
> > Signed-off-by: Yoshinori Sato 
> > Reviewed-by: Alex Bennée 
> > Reviewed-by: Philippe Mathieu-Daudé 
> > Message-Id: <20190607091116.49044-8-ys...@users.sourceforge.jp>
> > Tested-by: Philippe Mathieu-Daudé 
> > Signed-off-by: Richard Henderson 
> > ---
> >   include/hw/char/renesas_sci.h |  45 +
> >   hw/char/renesas_sci.c | 342 ++
> >   hw/char/Kconfig   |   3 +
> >   hw/char/Makefile.objs |   1 +
> >   4 files changed, 391 insertions(+)
> >   create mode 100644 include/hw/char/renesas_sci.h
> >   create mode 100644 hw/char/renesas_sci.c
> > 
> > diff --git a/include/hw/char/renesas_sci.h b/include/hw/char/renesas_sci.h
> > new file mode 100644
> > index 00..50d1336944
> > --- /dev/null
> > +++ b/include/hw/char/renesas_sci.h
> > @@ -0,0 +1,45 @@
> > +/*
> > + * Renesas Serial Communication Interface
> > + *
> > + * Copyright (c) 2018 Yoshinori Sato
> > + *
> > + * This code is licensed under the GPL version 2 or later.
> > + *
> > + */
> > +
> > +#include "chardev/char-fe.h"
> > +#include "qemu/timer.h"
> > +#include "hw/sysbus.h"
> > +
> > +#define TYPE_RENESAS_SCI "renesas-sci"
> > +#define RSCI(obj) OBJECT_CHECK(RSCIState, (obj), TYPE_RENESAS_SCI)
> > +
> > +enum {
> > +ERI = 0,
> > +RXI = 1,
> > +TXI = 2,
> > +TEI = 3,
> > +SCI_NR_IRQ = 4,
> > +};
> > +
> > +typedef struct {
> > +SysBusDevice parent_obj;
> > +MemoryRegion memory;
> > +
> > +uint8_t smr;
> > +uint8_t brr;
> > +uint8_t scr;
> > +uint8_t tdr;
> > +uint8_t ssr;
> > +uint8_t rdr;
> > +uint8_t scmr;
> > +uint8_t semr;
> > +
> > +uint8_t read_ssr;
> > +int64_t trtime;
> > +int64_t rx_next;
> > +QEMUTimer *timer;
> > +CharBackend chr;
> > +uint64_t input_freq;
> > +qemu_irq irq[SCI_NR_IRQ];
> > +} RSCIState;
> > diff --git a/hw/char/renesas_sci.c b/hw/char/renesas_sci.c
> > new file mode 100644
> > index 00..0760a51f43
> > --- /dev/null
> > +++ b/hw/char/renesas_sci.c
> > @@ -0,0 +1,342 @@
> > +/*
> > + * Renesas Serial Communication Interface
> 
> Looking at this again, have you looked at the SH model
> (hw/char/sh_serial.c)? This seems the same.
> (Similarly your timer model with hw/timer/sh_timer.c).
>

sh_serial has FIFO.
renesas_sci has no FIFO.
These are relationships like 8250 and 16550.
sh_serial is old,so I think it's better to implement
and integrate FIFO into renesas_sci.

> > + *
> > + * Datasheet: RX62N Group, RX621 Group User's Manual: Hardware
> > + * (Rev.1.40 R01UH0033EJ0140)
> > + *
> > + * Copyright (c) 2019 Yoshinori Sato
> > + *
> > + * This program is free software; you can redistribute it and/or modify it
> > + * under the terms and conditions of the GNU General Public License,
> > + * version 2 or later, as published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope it will be useful, but WITHOUT
> > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> > + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License 
> > for
> > + * more details.
> > + *
> > + * You should have received a copy of the GNU General Public License along 
> > with
> > + * this program.  If not, see .
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "qemu/log.h"
> > +#include "qapi/error.h"
> > +#include "qemu-common.h"
> > +#include "hw/hw.h"
> > +#include "hw/irq.h"
> > +#include "hw/sysbus.h"
> > +#include "hw/registerfields.h"
> > +#include "hw/qdev-properties.h"
> > +#include "hw/char/renesas_sci.h"
> > +#include "migration/vmstate.h"
> > +#include "qemu/error-report.h"
> > +
> > +/* SCI register map */
> > +REG8(SMR, 0)
> > +  FIELD(SMR, CKS,  0, 2)
> > +  FIELD(SMR, MP,   2, 1)
> > +  FIELD(SMR, STOP, 3, 1)
> > +  FIELD(SMR, PM,   4, 1)
> > +  FIELD(SMR, PE,   5, 1)
> > +  FIELD(SMR, CHR,  6, 1)
> > +  FIELD(SMR, CM,   7, 1)
> > +REG8(BRR, 1)
> > +REG8(SCR, 2)
> > +  FIELD(SCR, CKE, 0, 2)
> > +  FIELD(SCR, TEIE, 2, 1)
> > +  FIELD(SCR, MPIE, 3, 1)
> > +  FIELD(SCR, RE,   4, 1)
> > +  FIELD(SCR, TE,   5, 1)
> > +  FIELD(SCR, RIE,  6, 1)
> > +  FIELD(SCR, TIE,  7, 1)
> > +REG8(TDR, 3)
> > +REG8(SSR, 4)
> > +  FIELD(SSR, MPBT, 0, 1)
> > +  FIELD(SSR, MPB,  1, 1)
> > +  FIELD(SSR, TEND, 2, 1)
> > +  FIELD(SSR, ERR, 3, 3)
> > +FIELD(SSR, PER,  3, 1)
> > +FIELD(SSR, FER,  4, 1)
> > +FIELD(SSR, ORER, 5, 1)
> > +  FIELD(SSR, RDRF, 6, 1)
> > +  FIELD(SSR, TDRE, 7, 1)
> > +REG8(RDR, 5)
> > +REG8(SCMR, 6)
> > +  FIELD(SCMR, SMIF, 0, 1)
> > +  FIELD(SCMR, SINV, 2, 1)
> > +  FIELD(SCMR, SDIR, 3, 1)
> > +  FIELD(SCMR, BCP2, 7, 1)
> > +REG8(SEMR, 7)
> > +  FIELD(S

Re: [PATCH v32 21/22] BootLinuxConsoleTest: Test the RX-Virt machine

[Yoshinori Sato]

On Mon, 09 Mar 2020 01:20:05 +0900,
Philippe Mathieu-Daudé wrote:
> 
> On 2/24/20 3:19 PM, Yoshinori Sato wrote:
> > From: Philippe Mathieu-Daudé 
> > 
> > Add two tests for the rx-virt machine, based on the recommended test
> > setup from Yoshinori Sato:
> > https://lists.gnu.org/archive/html/qemu-devel/2019-05/msg03586.html
> > 
> > - U-Boot prompt
> > - Linux kernel with Sash shell
> > 
> > These are very quick tests:
> > 
> >$ avocado run -t arch:rx tests/acceptance/boot_linux_console.py
> >JOB ID : 84a6ef01c0b87975ecbfcb31a920afd735753ace
> >JOB LOG: 
> > /home/phil/avocado/job-results/job-2019-05-24T05.02-84a6ef0/job.log
> > (1/2) 
> > tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_rx_uboot: PASS 
> > (0.11 s)
> > (2/2) 
> > tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_rx_linux: PASS 
> > (0.45 s)
> >RESULTS: PASS 2 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | 
> > CANCEL 0
> > 
> > Tests can also be run with:
> > 
> >$ avocado --show=console run -t arch:rx 
> > tests/acceptance/boot_linux_console.py
> >console: U-Boot 2016.05-rc3-23705-ga1ef3c71cb-dirty (Feb 05 2019 - 
> > 21:56:06 +0900)
> >console: Linux version 4.19.0+ (yo-satoh@yo-satoh-debian) (gcc version 
> > 9.0.0 20181105 (experimental) (GCC)) #137 Wed Feb 20 23:20:02 JST 2019
> >console: Built 1 zonelists, mobility grouping on.  Total pages: 8128
> >...
> >console: SuperH (H)SCI(F) driver initialized
> >console: 88240.serial: ttySC0 at MMIO 0x88240 (irq = 215, base_baud = 0) 
> > is a sci
> >console: console [ttySC0] enabled
> >console: 88248.serial: ttySC1 at MMIO 0x88248 (irq = 219, base_baud = 0) 
> > is a sci
> > 
> > Signed-off-by: Philippe Mathieu-Daudé 
> > Based-on: 20190517045136.3509-1-richard.hender...@linaro.org
> > "RX architecture support"
> > Signed-off-by: Yoshinori Sato 
> > ---
> >   tests/acceptance/boot_linux_console.py | 46 ++
> >   1 file changed, 46 insertions(+)
> > 
> > diff --git a/tests/acceptance/boot_linux_console.py 
> > b/tests/acceptance/boot_linux_console.py
> > index 34d37eba3b..367cf480a5 100644
> > --- a/tests/acceptance/boot_linux_console.py
> > +++ b/tests/acceptance/boot_linux_console.py
> > @@ -686,3 +686,49 @@ class BootLinuxConsole(Test):
> >   tar_hash = '49e88d9933742f0164b60839886c9739cb7a0d34'
> >   self.vm.add_args('-cpu', 'dc233c')
> >   self.do_test_advcal_2018('02', tar_hash, 'santas-sleigh-ride.elf')
> > +
> > +def test_rx_uboot(self):
> > +"""
> > +:avocado: tags=arch:rx
> > +:avocado: tags=machine:rx-virt
> > +:avocado: tags=endian:little
> > +"""
> > +uboot_url = ('https://acc.dl.osdn.jp/users/23/23888/u-boot.bin.gz')
> > +uboot_hash = '9b78dbd43b40b2526848c0b1ce9de02c24f4dcdb'
> > +uboot_path = self.fetch_asset(uboot_url, asset_hash=uboot_hash)
> > +uboot_path = archive.uncompress(uboot_path, self.workdir)
> > +
> > +self.vm.set_machine('rx-virt')
> > +self.vm.set_console()
> > +self.vm.add_args('-bios', uboot_path,
> > + '-no-reboot')
> > +self.vm.launch()
> > +uboot_version = 'U-Boot 2016.05-rc3-23705-ga1ef3c71cb-dirty'
> > +self.wait_for_console_pattern(uboot_version)
> > +gcc_version = 'rx-unknown-linux-gcc (GCC) 9.0.0 20181105 
> > (experimental)'
> > +# FIXME limit baudrate on chardev, else we type too fast
> > +#self.exec_command_and_wait_for_pattern('version', gcc_version)
> > +
> > +def test_rx_linux(self):
> > +"""
> > +:avocado: tags=arch:rx
> > +:avocado: tags=machine:rx-virt
> > +:avocado: tags=endian:little
> > +"""
> > +dtb_url = ('https://acc.dl.osdn.jp/users/23/23887/rx-qemu.dtb')
> 
> Sourceforge URL are not very stable, I'm now getting:
> 
> HTTP request sent, awaiting response... 302 Found
> Location: https://osdn.dl.osdn.net/users/23/23887/rx-qemu.dtb [following]
> --2020-03-08 17:17:31--  https://osdn.dl.osdn.net/users/23/23887/rx-qemu.dtb
> Resolving osdn.dl.osdn.net (osdn.dl.osdn.net)... 202.221.179.23
> Connecting to osdn.dl.osdn.net
> (osdn.dl.osdn.net)|202.221.179.23|:443... connected.
> HTTP request sent, awaiting response... 404 Not Found
> 2020-03-08 17:17:32 ERROR 404: Not Found.

Permernet link is bellow.
https://osdn.net/users/ysato/pf/qemu/dl/u-boot.bin.gz
https://osdn.net/users/ysato/pf/qemu/dl/rx-virt.dtb
https://osdn.net/users/ysato/pf/qemu/dl/zImage

I was misunderstanding. sorry.

> > +dtb_hash = '7b4e4e2c71905da44e86ce47adee2210b026ac18'
> > +dtb_path = self.fetch_asset(dtb_url, asset_hash=dtb_hash)
> > +kernel_url = ('http://acc.dl.osdn.jp/users/23/23845/zImage')
> > +kernel_hash = '39a81067f8d72faad90866ddfefa19165d68fc99'
> > +kernel_path = self.fetch_asset(kernel_url, asset_hash=kernel_hash)
> > +
> > +self.vm.set_machin