Re: Why guest physical addresses are not the same as the corresponding host virtual addresses in QEMU/KVM? Thanks!

[Maxim Levitsky]

On Sun, 2020-10-11 at 01:26 -0400, harry harry wrote:
> Hi QEMU/KVM developers,
> 
> I am sorry if my email disturbs you. I did an experiment and found the
> guest physical addresses (GPAs) are not the same as the corresponding
> host virtual addresses (HVAs). I am curious about why; I think they
> should be the same. I am very appreciated if you can give some
> comments and suggestions about 1) why GPAs and HVAs are not the same
> in the following experiment; 2) are there any better experiments to
> look into the reasons? Any other comments/suggestions are also very
> welcome. Thanks!
> 
> The experiment is like this: in a single vCPU VM, I ran a program
> allocating and referencing lots of pages (e.g., 100*1024) and didn't
> let the program terminate. Then, I checked the program's guest virtual
> addresses (GVAs) and GPAs through parsing its pagemap and maps files
> located at /proc/pid/pagemap and /proc/pid/maps, respectively. At
> last, in the host OS, I checked the vCPU's pagemap and maps files to
> find the program's HVAs and host physical addresses (HPAs); I actually
> checked the new allocated physical pages in the host OS after the
> program was executed in the guest OS.
> 
> With the above experiment, I found GPAs of the program are different
> from its corresponding HVAs. BTW, Intel EPT and other related Intel
> virtualization techniques were enabled.
> 
> Thanks,
> Harry
> 
The fundemental reason is that some HVAs (e.g. QEMU's virtual memory addresses) 
are already allocated
for qemu's own use (e.g qemu code/heap/etc) prior to the guest starting up. 

KVM does though use quite effiecient way of mapping HVA's to GPA. It uses an 
array of arbitrary sized HVA areas
(which we call memslots) and for each such area/memslot you specify the GPA to 
map to. In theory QEMU 
could allocate the whole guest's memory in one contiguous area and map it as 
single memslot to the guest. 
In practice there are MMIO holes, and various other reasons why there will be 
more that 1 memslot.
 
Best regards,
Maxim Levitsky

[PATCH v4 6/7] qom: Add user_creatable_print_help_from_qdict()

[Markus Armbruster]

From: Kevin Wolf 

This adds a function that, given a QDict of non-help options, prints
help for user creatable objects.

Signed-off-by: Kevin Wolf 
Reviewed-by: Eric Blake 
Reviewed-by: Markus Armbruster 
---
 include/qom/object_interfaces.h | 21 ++---
 qom/object_interfaces.c |  9 +
 2 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/include/qom/object_interfaces.h b/include/qom/object_interfaces.h
index f118fb516b..07d5cc8832 100644
--- a/include/qom/object_interfaces.h
+++ b/include/qom/object_interfaces.h
@@ -154,13 +154,28 @@ int user_creatable_add_opts_foreach(void *opaque,
  * @type: the QOM type to be added
  * @opts: options to create
  *
- * Prints help if requested in @opts.
+ * Prints help if requested in @type or @opts. Note that if @type is neither
+ * "help"/"?" nor a valid user creatable type, no help will be printed
+ * regardless of @opts.
  *
- * Returns: true if @opts contained a help option and help was printed, false
- * if no help option was found.
+ * Returns: true if a help option was found and help was printed, false
+ * otherwise.
  */
 bool user_creatable_print_help(const char *type, QemuOpts *opts);
 
+/**
+ * user_creatable_print_help_from_qdict:
+ * @args: options to create
+ *
+ * Prints help considering the other options given in @args (if "qom-type" is
+ * given and valid, print properties for the type, otherwise print valid types)
+ *
+ * In contrast to user_creatable_print_help(), this function can't return that
+ * no help was requested. It should only be called if we know that help is
+ * requested and it will always print some help.
+ */
+void user_creatable_print_help_from_qdict(QDict *args);
+
 /**
  * user_creatable_del:
  * @id: the unique ID for the object
diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
index 3fd1da157e..ed896fe764 100644
--- a/qom/object_interfaces.c
+++ b/qom/object_interfaces.c
@@ -279,6 +279,15 @@ bool user_creatable_print_help(const char *type, QemuOpts 
*opts)
 return false;
 }
 
+void user_creatable_print_help_from_qdict(QDict *args)
+{
+const char *type = qdict_get_try_str(args, "qom-type");
+
+if (!type || !user_creatable_print_type_properites(type)) {
+user_creatable_print_types();
+}
+}
+
 bool user_creatable_del(const char *id, Error **errp)
 {
 Object *container;
-- 
2.26.2

[PATCH v4 7/7] qemu-storage-daemon: Remove QemuOpts from --object parser

[Markus Armbruster]

From: Kevin Wolf 

The command line parser for --object parses the input twice: Once into
QemuOpts just for detecting help options, and then again into a QDict
using the keyval parser for actually creating the object.

Now that the keyval parser can also detect help options, we can simplify
this and remove the QemuOpts part.

Signed-off-by: Kevin Wolf 
Reviewed-by: Eric Blake 
Reviewed-by: Markus Armbruster 
---
 storage-daemon/qemu-storage-daemon.c | 15 ---
 1 file changed, 4 insertions(+), 11 deletions(-)

diff --git a/storage-daemon/qemu-storage-daemon.c 
b/storage-daemon/qemu-storage-daemon.c
index 6f0e0cfb36..e419ba9f19 100644
--- a/storage-daemon/qemu-storage-daemon.c
+++ b/storage-daemon/qemu-storage-daemon.c
@@ -264,21 +264,14 @@ static void process_options(int argc, char *argv[])
 }
 case OPTION_OBJECT:
 {
-QemuOpts *opts;
-const char *type;
 QDict *args;
+bool help;
 
-/* FIXME The keyval parser rejects 'help' arguments, so we must
- * unconditionall try QemuOpts first. */
-opts = qemu_opts_parse(&qemu_object_opts,
-   optarg, true, &error_fatal);
-type = qemu_opt_get(opts, "qom-type");
-if (type && user_creatable_print_help(type, opts)) {
+args = keyval_parse(optarg, "qom-type", &help, &error_fatal);
+if (help) {
+user_creatable_print_help_from_qdict(args);
 exit(EXIT_SUCCESS);
 }
-qemu_opts_del(opts);
-
-args = keyval_parse(optarg, "qom-type", NULL, &error_fatal);
 user_creatable_add_dict(args, true, &error_fatal);
 qobject_unref(args);
 break;
-- 
2.26.2

[PATCH v4 5/7] qom: Factor out helpers from user_creatable_print_help()

[Markus Armbruster]

From: Kevin Wolf 

This creates separate helper functions for printing a list of user
creatable object types and for printing a list of properties of a given
type. This will allow using these parts without having a QemuOpts.

Signed-off-by: Kevin Wolf 
Reviewed-by: Eric Blake 
Reviewed-by: Markus Armbruster 
---
 qom/object_interfaces.c | 90 -
 1 file changed, 52 insertions(+), 38 deletions(-)

diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
index e8e1523960..3fd1da157e 100644
--- a/qom/object_interfaces.c
+++ b/qom/object_interfaces.c
@@ -214,52 +214,66 @@ char *object_property_help(const char *name, const char 
*type,
 return g_string_free(str, false);
 }
 
-bool user_creatable_print_help(const char *type, QemuOpts *opts)
+static void user_creatable_print_types(void)
+{
+GSList *l, *list;
+
+printf("List of user creatable objects:\n");
+list = object_class_get_list_sorted(TYPE_USER_CREATABLE, false);
+for (l = list; l != NULL; l = l->next) {
+ObjectClass *oc = OBJECT_CLASS(l->data);
+printf("  %s\n", object_class_get_name(oc));
+}
+g_slist_free(list);
+}
+
+static bool user_creatable_print_type_properites(const char *type)
 {
 ObjectClass *klass;
+ObjectPropertyIterator iter;
+ObjectProperty *prop;
+GPtrArray *array;
+int i;
 
-if (is_help_option(type)) {
-GSList *l, *list;
+klass = object_class_by_name(type);
+if (!klass) {
+return false;
+}
 
-printf("List of user creatable objects:\n");
-list = object_class_get_list_sorted(TYPE_USER_CREATABLE, false);
-for (l = list; l != NULL; l = l->next) {
-ObjectClass *oc = OBJECT_CLASS(l->data);
-printf("  %s\n", object_class_get_name(oc));
+array = g_ptr_array_new();
+object_class_property_iter_init(&iter, klass);
+while ((prop = object_property_iter_next(&iter))) {
+if (!prop->set) {
+continue;
 }
-g_slist_free(list);
+
+g_ptr_array_add(array,
+object_property_help(prop->name, prop->type,
+ prop->defval, prop->description));
+}
+g_ptr_array_sort(array, (GCompareFunc)qemu_pstrcmp0);
+if (array->len > 0) {
+printf("%s options:\n", type);
+} else {
+printf("There are no options for %s.\n", type);
+}
+for (i = 0; i < array->len; i++) {
+printf("%s\n", (char *)array->pdata[i]);
+}
+g_ptr_array_set_free_func(array, g_free);
+g_ptr_array_free(array, true);
+return true;
+}
+
+bool user_creatable_print_help(const char *type, QemuOpts *opts)
+{
+if (is_help_option(type)) {
+user_creatable_print_types();
 return true;
 }
 
-klass = object_class_by_name(type);
-if (klass && qemu_opt_has_help_opt(opts)) {
-ObjectPropertyIterator iter;
-ObjectProperty *prop;
-GPtrArray *array = g_ptr_array_new();
-int i;
-
-object_class_property_iter_init(&iter, klass);
-while ((prop = object_property_iter_next(&iter))) {
-if (!prop->set) {
-continue;
-}
-
-g_ptr_array_add(array,
-object_property_help(prop->name, prop->type,
- prop->defval, 
prop->description));
-}
-g_ptr_array_sort(array, (GCompareFunc)qemu_pstrcmp0);
-if (array->len > 0) {
-printf("%s options:\n", type);
-} else {
-printf("There are no options for %s.\n", type);
-}
-for (i = 0; i < array->len; i++) {
-printf("%s\n", (char *)array->pdata[i]);
-}
-g_ptr_array_set_free_func(array, g_free);
-g_ptr_array_free(array, true);
-return true;
+if (qemu_opt_has_help_opt(opts)) {
+return user_creatable_print_type_properites(type);
 }
 
 return false;
-- 
2.26.2

[PATCH v4 2/7] test-keyval: Demonstrate misparse of ', ' with implied key

[Markus Armbruster]

Add a test for "val,,ue" with implied key.  Documentation says this
should parse as implied key with value "val", then fail.  The code
parses it as implied key with value "val,ue", then succeeds.  The next
commit will fix it.

Signed-off-by: Markus Armbruster 
---
 tests/test-keyval.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/tests/test-keyval.c b/tests/test-keyval.c
index e331a84149..f02bdf7029 100644
--- a/tests/test-keyval.c
+++ b/tests/test-keyval.c
@@ -182,6 +182,13 @@ static void test_keyval_parse(void)
 error_free_or_abort(&err);
 g_assert(!qdict);
 
+/* Implied key's value can't have comma (qemu_opts_parse(): it can) */
+/* BUG: it can */
+qdict = keyval_parse("val,,ue", "implied", &error_abort);
+g_assert_cmpuint(qdict_size(qdict), ==, 1);
+g_assert_cmpstr(qdict_get_try_str(qdict, "implied"), ==, "val,ue");
+qobject_unref(qdict);
+
 /* Empty key is not an implied key */
 qdict = keyval_parse("=val", "implied", &err);
 error_free_or_abort(&err);
-- 
2.26.2

[PATCH v4 0/7] qemu-storage-daemon: Remove QemuOpts from --object

[Markus Armbruster]

This replaces the QemuOpts-based help code for --object in the storage
daemon with code based on the keyval parser.

Review of v3 led me to preexisting issues.  Instead of posting my
fixes separately, then working with Kevin to rebase his work on top of
mine, I did the rebase myself and am posting it together with my fixes
as v4.  Hope that's okay.

Note: to test qemu-storage-daemon --object help, you need Philippe's
"[PATCH v3] hw/nvram: Always register FW_CFG_DATA_GENERATOR_INTERFACE"
and its prerequisites (first 10 patches of Paolo's "[PULL 00/39] SCSI,
qdev, qtest, meson patches for 2020-10-10"), so it doesn't crash with
"missing interface 'fw_cfg-data-generator' for object 'tls-creds'".

v4:
- PATCH 1-3: New
- PATCH 4:
  * Rebased
  * Commit message typos [Eric]
  * Replace flawed is_help_option_n() by starts_with_help_option()
[me]
  * Update grammar and accompanying prose [me]
  * Update keyval_parse_one()'s contract, tweak keyval_parse()'s [me]
  * Revert the keyval_parse_one() change to a rebased version of v1,
because it's simpler and the edge case that led to the more
complicated version no longer exists [me].
  * Rearrange tests so the simple cases come first
- PATCH 5-7: Unchanged

v3:
- Always parse help options, no matter if the caller implements help or
  not. If it doesn't, return an error. [Markus]
- Document changes to the keyval parser grammar [Markus]
- Support both 'help' and '?' [Eric]
- Test case fixes [Eric]
- Improved documentation of user_creatable_print_help(_from_qdict)
  [Markus]

v2:
- Fixed double comma by reusing the existing key and value parsers [Eric]
- More tests to cover the additional cases

Kevin Wolf (4):
  keyval: Parse help options
  qom: Factor out helpers from user_creatable_print_help()
  qom: Add user_creatable_print_help_from_qdict()
  qemu-storage-daemon: Remove QemuOpts from --object parser

Markus Armbruster (3):
  keyval: Fix and clarify grammar
  test-keyval: Demonstrate misparse of ',' with implied key
  keyval: Fix parsing of ',' in value of implied key

 include/qemu/help_option.h   |  11 ++
 include/qemu/option.h|   2 +-
 include/qom/object_interfaces.h  |  21 ++-
 qapi/qobject-input-visitor.c |   2 +-
 qom/object_interfaces.c  |  99 --
 storage-daemon/qemu-storage-daemon.c |  15 +--
 tests/test-keyval.c  | 187 ++-
 util/keyval.c| 103 +++
 8 files changed, 297 insertions(+), 143 deletions(-)

-- 
2.26.2

[PATCH v4 4/7] keyval: Parse help options

[Markus Armbruster]

From: Kevin Wolf 

This adds a special meaning for 'help' and '?' as options to the keyval
parser. Instead of being an error (because of a missing value) or a
value for an implied key, they now request help, which is a new boolean
output of the parser in addition to the QDict.

A new parameter 'p_help' is added to keyval_parse() that contains on
return whether help was requested. If NULL is passed, requesting help
results in an error and all other cases work like before.

Turning previous error cases into help is a compatible extension. The
behaviour potentially changes for implied keys: They could previously
get 'help' as their value, which is now interpreted as requesting help.

This is not a problem in practice because 'help' and '?' are not a valid
values for the implied key of any option parsed with keyval_parse():

* audiodev: union Audiodev, implied key "driver" is enum AudiodevDriver,
  "help" and "?" are not among its values

* display: union DisplayOptions, implied key "type" is enum
  DisplayType, "help" and "?" are not among its values

* blockdev: union BlockdevOptions, implied key "driver is enum
  BlockdevDriver, "help" and "?" are not among its values

* export: union BlockExport, implied key "type" is enum BlockExportType,
  "help" and "?" are not among its values

* monitor: struct MonitorOptions, implied key "mode" is enum MonitorMode,
  "help" and "?" are not among its values

* nbd-server: struct NbdServerOptions, no implied key.

Signed-off-by: Kevin Wolf 
Signed-off-by: Markus Armbruster 
---
 include/qemu/help_option.h   |  11 ++
 include/qemu/option.h|   2 +-
 qapi/qobject-input-visitor.c |   2 +-
 storage-daemon/qemu-storage-daemon.c |   2 +-
 tests/test-keyval.c  | 184 ++-
 util/keyval.c|  63 +++--
 6 files changed, 186 insertions(+), 78 deletions(-)

diff --git a/include/qemu/help_option.h b/include/qemu/help_option.h
index 328d2a89fd..ca6389a154 100644
--- a/include/qemu/help_option.h
+++ b/include/qemu/help_option.h
@@ -19,4 +19,15 @@ static inline bool is_help_option(const char *s)
 return !strcmp(s, "?") || !strcmp(s, "help");
 }
 
+static inline int starts_with_help_option(const char *s)
+{
+if (*s == '?') {
+return 1;
+}
+if (g_str_has_prefix(s, "help")) {
+return 4;
+}
+return 0;
+}
+
 #endif
diff --git a/include/qemu/option.h b/include/qemu/option.h
index 05e8a15c73..ac69352e0e 100644
--- a/include/qemu/option.h
+++ b/include/qemu/option.h
@@ -149,6 +149,6 @@ void qemu_opts_free(QemuOptsList *list);
 QemuOptsList *qemu_opts_append(QemuOptsList *dst, QemuOptsList *list);
 
 QDict *keyval_parse(const char *params, const char *implied_key,
-Error **errp);
+bool *help, Error **errp);
 
 #endif
diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c
index f918a05e5f..7b184b50a7 100644
--- a/qapi/qobject-input-visitor.c
+++ b/qapi/qobject-input-visitor.c
@@ -757,7 +757,7 @@ Visitor *qobject_input_visitor_new_str(const char *str,
 assert(args);
 v = qobject_input_visitor_new(QOBJECT(args));
 } else {
-args = keyval_parse(str, implied_key, errp);
+args = keyval_parse(str, implied_key, NULL, errp);
 if (!args) {
 return NULL;
 }
diff --git a/storage-daemon/qemu-storage-daemon.c 
b/storage-daemon/qemu-storage-daemon.c
index 1ae1cda481..6f0e0cfb36 100644
--- a/storage-daemon/qemu-storage-daemon.c
+++ b/storage-daemon/qemu-storage-daemon.c
@@ -278,7 +278,7 @@ static void process_options(int argc, char *argv[])
 }
 qemu_opts_del(opts);
 
-args = keyval_parse(optarg, "qom-type", &error_fatal);
+args = keyval_parse(optarg, "qom-type", NULL, &error_fatal);
 user_creatable_add_dict(args, true, &error_fatal);
 qobject_unref(args);
 break;
diff --git a/tests/test-keyval.c b/tests/test-keyval.c
index 04c62cf045..7b459900af 100644
--- a/tests/test-keyval.c
+++ b/tests/test-keyval.c
@@ -1,3 +1,4 @@
+
 /*
  * Unit tests for parsing of KEY=VALUE,... strings
  *
@@ -27,27 +28,28 @@ static void test_keyval_parse(void)
 QDict *qdict, *sub_qdict;
 char long_key[129];
 char *params;
+bool help;
 
 /* Nothing */
-qdict = keyval_parse("", NULL, &error_abort);
+qdict = keyval_parse("", NULL, NULL, &error_abort);
 g_assert_cmpuint(qdict_size(qdict), ==, 0);
 qobject_unref(qdict);
 
 /* Empty key (qemu_opts_parse() accepts this) */
-qdict = keyval_parse("=val", NULL, &err);
+qdict = keyval_parse("=val", NULL, NULL, &err);
 error_free_or_abort(&err);
 g_assert(!qdict);
 
 /* Empty key fragment */
-qdict = keyval_parse(".", NULL, &err);
+qdict = keyval_parse(".", NULL, NULL, &err);
 error_free_or_abort(&err);
 g_assert(!qdict);
-qdict = keyval_parse("key.", NULL

[PATCH v4 1/7] keyval: Fix and clarify grammar

[Markus Armbruster]

The grammar has a few issues:

* key-fragment = / [^=,.]* /

  Prose restricts key fragments: they "must be valid QAPI names or
  consist only of decimal digits".  Technically, '' consists only of
  decimal digits.  The code rejects that.  Fix the grammar.

* val  = { / [^,]* / | ',,' }

  Use + instead of *.  Accepts the same language.

* val-no-key   = / [^=,]* /

  The code rejects an empty value.  Fix the grammar.

* Section "Additional syntax for use with an implied key" is
  confusing.  Rewrite it.

Signed-off-by: Markus Armbruster 
---
 util/keyval.c | 16 ++--
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/util/keyval.c b/util/keyval.c
index 13def4af54..82d8497c71 100644
--- a/util/keyval.c
+++ b/util/keyval.c
@@ -16,8 +16,8 @@
  *   key-vals = [ key-val { ',' key-val } [ ',' ] ]
  *   key-val  = key '=' val
  *   key  = key-fragment { '.' key-fragment }
- *   key-fragment = / [^=,.]* /
- *   val  = { / [^,]* / | ',,' }
+ *   key-fragment = / [^=,.]+ /
+ *   val  = { / [^,]+ / | ',,' }
  *
  * Semantics defined by reduction to JSON:
  *
@@ -71,12 +71,16 @@
  * Awkward.  Note that we carefully restrict alternate types to avoid
  * similar ambiguity.
  *
- * Additional syntax for use with an implied key:
+ * Alternative syntax for use with an implied key:
  *
- *   key-vals-ik  = val-no-key [ ',' key-vals ]
- *   val-no-key   = / [^=,]* /
+ *   key-vals = [ key-val-1st { ',' key-val } [ ',' ] ]
+ *   key-val-1st  = val-no-key | key-val
+ *   val-no-key   = / [^=,]+ /
  *
- * where no-key is syntactic sugar for implied-key=val-no-key.
+ * where val-no-key is syntactic sugar for implied-key=val-no-key.
+ *
+ * Note that you can't use the sugared form when the value contains
+ * '=' or ','.
  */
 
 #include "qemu/osdep.h"
-- 
2.26.2

[PATCH v4 3/7] keyval: Fix parsing of ',' in value of implied key

[Markus Armbruster]

The previous commit demonstrated documentation and code disagree on
parsing of ',' in the value of an implied key.  Fix the code to match
the documentation.

This breaks uses of keyval_parse() that pass an implied key and accept
a value containing ','.  None of the existing uses does:

* audiodev: implied key "driver" is enum AudiodevDriver, none of the
  values contains ','

* display: implied key "type" is enum DisplayType, none of the values
  contains ','

* blockdev: implied key "driver is enum BlockdevDriver, none of the
  values contains ','

* export: implied key "type" is enum BlockExportType, none of the
  values contains ','

* monitor: implied key "mode" is enum MonitorMode, none of the values
  contains ','

* nbd-server: no implied key.

Signed-off-by: Markus Armbruster 
---
 tests/test-keyval.c |  8 +++-
 util/keyval.c   | 28 +---
 2 files changed, 20 insertions(+), 16 deletions(-)

diff --git a/tests/test-keyval.c b/tests/test-keyval.c
index f02bdf7029..04c62cf045 100644
--- a/tests/test-keyval.c
+++ b/tests/test-keyval.c
@@ -183,11 +183,9 @@ static void test_keyval_parse(void)
 g_assert(!qdict);
 
 /* Implied key's value can't have comma (qemu_opts_parse(): it can) */
-/* BUG: it can */
-qdict = keyval_parse("val,,ue", "implied", &error_abort);
-g_assert_cmpuint(qdict_size(qdict), ==, 1);
-g_assert_cmpstr(qdict_get_try_str(qdict, "implied"), ==, "val,ue");
-qobject_unref(qdict);
+qdict = keyval_parse("val,,ue", "implied", &err);
+error_free_or_abort(&err);
+g_assert(!qdict);
 
 /* Empty key is not an implied key */
 qdict = keyval_parse("=val", "implied", &err);
diff --git a/util/keyval.c b/util/keyval.c
index 82d8497c71..8f33a36a7c 100644
--- a/util/keyval.c
+++ b/util/keyval.c
@@ -173,7 +173,7 @@ static const char *keyval_parse_one(QDict *qdict, const 
char *params,
 const char *implied_key,
 Error **errp)
 {
-const char *key, *key_end, *s, *end;
+const char *key, *key_end, *val_end, *s, *end;
 size_t len;
 char key_in_cur[128];
 QDict *cur;
@@ -182,10 +182,12 @@ static const char *keyval_parse_one(QDict *qdict, const 
char *params,
 QString *val;
 
 key = params;
+val_end = NULL;
 len = strcspn(params, "=,");
 if (implied_key && len && key[len] != '=') {
 /* Desugar implied key */
 key = implied_key;
+val_end = params + len;
 len = strlen(implied_key);
 }
 key_end = key + len;
@@ -241,7 +243,11 @@ static const char *keyval_parse_one(QDict *qdict, const 
char *params,
 
 if (key == implied_key) {
 assert(!*s);
-s = params;
+val = qstring_from_substr(params, 0, val_end - params);
+s = val_end;
+if (*s == ',') {
+s++;
+}
 } else {
 if (*s != '=') {
 error_setg(errp, "Expected '=' after parameter '%.*s'",
@@ -249,19 +255,19 @@ static const char *keyval_parse_one(QDict *qdict, const 
char *params,
 return NULL;
 }
 s++;
-}
 
-val = qstring_new();
-for (;;) {
-if (!*s) {
-break;
-} else if (*s == ',') {
-s++;
-if (*s != ',') {
+val = qstring_new();
+for (;;) {
+if (!*s) {
 break;
+} else if (*s == ',') {
+s++;
+if (*s != ',') {
+break;
+}
 }
+qstring_append_chr(val, *s++);
 }
-qstring_append_chr(val, *s++);
 }
 
 if (!keyval_parse_put(cur, key_in_cur, val, key, key_end, errp)) {
-- 
2.26.2

[PATCH] sabre: increase number of PCI bus IRQs from 32 to 64

[Mark Cave-Ayland]

The rework of the sabre IRQs in commit 6864fa3897 "sun4u: update PCI topology to
include simba PCI bridges" changed the IRQ routing so that both PCI and legacy
OBIO IRQs are routed through the sabre PCI host bridge to the CPU.

Unfortunately this commit failed to increase the number of PCI bus IRQs
accordingly meaning that access to the legacy IRQs OBIO (irqnum >= 0x20) would
overflow the PCI bus IRQ array causing strange failures running 
qemu-system-sparc64
in NetBSD.

Reported-by: Harold Gutch 
Fixes: https://bugs.launchpad.net/qemu/+bug/1838658
Fixes: 6864fa3897 ("sun4u: update PCI topology to include simba PCI bridges")
Signed-off-by: Mark Cave-Ayland 
---
 hw/pci-host/sabre.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/pci-host/sabre.c b/hw/pci-host/sabre.c
index 5ac6283623..ffdba1d865 100644
--- a/hw/pci-host/sabre.c
+++ b/hw/pci-host/sabre.c
@@ -396,7 +396,7 @@ static void sabre_realize(DeviceState *dev, Error **errp)
  pci_sabre_set_irq, pci_sabre_map_irq, s,
  &s->pci_mmio,
  &s->pci_ioport,
- 0, 32, TYPE_PCI_BUS);
+ 0, 0x40, TYPE_PCI_BUS);
 
 pci_create_simple(phb->bus, 0, TYPE_SABRE_PCI_DEVICE);
 
-- 
2.20.1

[PATCH] pci: assert that irqnum is between 0 and bus->nirqs in pci_change_irq_level()

[Mark Cave-Ayland]

These assertions similar to those in the adjacent pci_bus_get_irq_level() 
function
ensure that irqnum lies within the valid PCI bus IRQ range.

Signed-off-by: Mark Cave-Ayland 
---

This would have immediately picked up on the sabre PCI bus IRQ overflow fixed by
the patch I just posted.

---
 hw/pci/pci.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 3c8f10b461..b1484b3747 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -258,6 +258,8 @@ static void pci_change_irq_level(PCIDevice *pci_dev, int 
irq_num, int change)
 break;
 pci_dev = bus->parent_dev;
 }
+assert(irq_num >= 0);
+assert(irq_num < bus->nirq);
 bus->irq_count[irq_num] += change;
 bus->set_irq(bus->irq_opaque, irq_num, bus->irq_count[irq_num] != 0);
 }
-- 
2.20.1

Re: [PATCH] pci: assert that irqnum is between 0 and bus->nirqs in pci_change_irq_level()

[Mark Cave-Ayland]

On 11/10/2020 09:20, Mark Cave-Ayland wrote:

> These assertions similar to those in the adjacent pci_bus_get_irq_level() 
> function
> ensure that irqnum lies within the valid PCI bus IRQ range.
> 
> Signed-off-by: Mark Cave-Ayland 
> ---
> 
> This would have immediately picked up on the sabre PCI bus IRQ overflow fixed 
> by
> the patch I just posted.
> 
> ---
>  hw/pci/pci.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 3c8f10b461..b1484b3747 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -258,6 +258,8 @@ static void pci_change_irq_level(PCIDevice *pci_dev, int 
> irq_num, int change)
>  break;
>  pci_dev = bus->parent_dev;
>  }
> +assert(irq_num >= 0);
> +assert(irq_num < bus->nirq);
>  bus->irq_count[irq_num] += change;
>  bus->set_irq(bus->irq_opaque, irq_num, bus->irq_count[irq_num] != 0);
>  }

Actually something else is odd here: I've just done a quick check on the 
callers to
pci_change_irq_level() and it appears that both pci_update_irq_disabled() and
pci_irq_handler() assume that irqnum is a PCI device IRQ i.e between 0 and 3, 
whereas
pci_change_irq_level() assumes it is working with a PCI bus IRQ between 0 and 
bus->nirqs.

It feels like pci_change_irq_level() should be renamed to 
pci_bus_change_irq_level()
similar to pci_bus_get_irq_level() but in that case are 
pci_update_irq_disabled() and
pci_irq_handler() both incorrect?


ATB,

Mark.

Re: [PATCH] qcow2: cleanup created file when qcow2_co_create

[Maxim Levitsky]

On Tue, 2020-09-01 at 15:30 +0200, Alberto Garcia wrote:
> On Thu 16 Jul 2020 01:33:59 PM CEST, Maxim Levitsky wrote:
> >  if (ret < 0) {
> > +
> > +Error *local_delete_err = NULL;
> > +int r_del = bdrv_co_delete_file(bs, &local_delete_err);
> > +/*
> > + * ENOTSUP will happen if the block driver doesn't support
> > + * the 'bdrv_co_delete_file' interface. This is a predictable
> > + * scenario and shouldn't be reported back to the user.
> > + */
> > +if ((r_del < 0) && (r_del != -ENOTSUP)) {
> > +error_report_err(local_delete_err);
> > +}
> 
> I think you're leaking the error on ENOTSUP.

Indeed, just like in the original code from where I copy&pasta'ed this from.
I'll send a patch to fix this and update the patch as well.

Thanks,
Best regards,
Maxim Levitsky

> 
> Berto
> 

[PATCH v2 0/2] qcow2: don't leave partially initialized file on image creation

[Maxim Levitsky]

Use the bdrv_co_delete_file interface to delete the underlying
file if qcow2 initilization fails (e.g due to bad encryption secret)

This gives the qcow2 the same treatment as to luks.

V2: added a patch to fix a memory leak.

Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1845353

Maxim Levitsky (2):
  crypto: luks: fix tiny memory leak
  block: qcow2: remove the created file on initialization error

 block/crypto.c |  1 +
 block/qcow2.c  | 12 
 2 files changed, 13 insertions(+)

-- 
2.26.2

[PATCH v2 1/2] crypto: luks: fix tiny memory leak

[Maxim Levitsky]

In the case when underlying block device doesn't support the
bdrv_co_delete_file interface, an 'Error' wasn't freed.

Signed-off-by: Maxim Levitsky 
---
 block/crypto.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/block/crypto.c b/block/crypto.c
index 0807557763..9b61fd4aa8 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -736,6 +736,7 @@ fail:
 if ((r_del < 0) && (r_del != -ENOTSUP)) {
 error_report_err(local_delete_err);
 }
+error_free(local_delete_err);
 }
 
 bdrv_unref(bs);
-- 
2.26.2

[PATCH v2 2/2] block: qcow2: remove the created file on initialization error

[Maxim Levitsky]

If the qcow initialization fails after we created the storage file,
we should remove it to avoid leaving stale files around.

We already do this for luks raw images.

Signed-off-by: Maxim Levitsky 
---
 block/qcow2.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/block/qcow2.c b/block/qcow2.c
index b05512718c..4dc6102df8 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -3834,6 +3834,18 @@ static int coroutine_fn qcow2_co_create_opts(BlockDriver 
*drv,
 /* Create the qcow2 image (format layer) */
 ret = qcow2_co_create(create_options, errp);
 if (ret < 0) {
+
+Error *local_delete_err = NULL;
+int r_del = bdrv_co_delete_file(bs, &local_delete_err);
+/*
+ * ENOTSUP will happen if the block driver doesn't support
+ * the 'bdrv_co_delete_file' interface. This is a predictable
+ * scenario and shouldn't be reported back to the user.
+ */
+if ((r_del < 0) && (r_del != -ENOTSUP)) {
+error_report_err(local_delete_err);
+}
+error_free(local_delete_err);
 goto finish;
 }
 
-- 
2.26.2

Re: [PATCH V13 6/9] target/mips: Add loongson-ext lsdc2 group of instructions

[Philippe Mathieu-Daudé]

On 10/11/20 5:02 AM, Huacai Chen wrote:

Hi, Philippe,

On Sat, Oct 10, 2020 at 9:07 PM Philippe Mathieu-Daudé  wrote:


On 10/7/20 10:39 AM, Huacai Chen wrote:

From: Jiaxun Yang 

LDC2/SDC2 opcodes have been rewritten as "load & store with offset"
group of instructions by loongson-ext ASE.

This patch add implementation of these instructions:
gslbx: load 1 bytes to GPR
gslhx: load 2 bytes to GPR
gslwx: load 4 bytes to GPR
gsldx: load 8 bytes to GPR
gslwxc1: load 4 bytes to FPR
gsldxc1: load 8 bytes to FPR
gssbx: store 1 bytes from GPR
gsshx: store 2 bytes from GPR
gsswx: store 4 bytes from GPR
gssdx: store 8 bytes from GPR
gsswxc1: store 4 bytes from FPR
gssdxc1: store 8 bytes from FPR

Details of Loongson-EXT is here:
https://github.com/FlyGoat/loongson-insn/blob/master/loongson-ext.md

Signed-off-by: Huacai Chen 
Signed-off-by: Jiaxun Yang 


If this patch is from Jiaxun, Huacai's S-o-b should come *after*.

OK, I will do that.




---
   target/mips/translate.c | 179 

   1 file changed, 179 insertions(+)

diff --git a/target/mips/translate.c b/target/mips/translate.c
index 916b57f..4d42cfc 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -484,6 +484,24 @@ enum {
   OPC_GSSDRC1 = 0x7 | OPC_GSSHFS,
   };

+/* Loongson EXT LDC2/SDC2 opcodes */
+#define MASK_LOONGSON_LSDC2(op)   (MASK_OP_MAJOR(op) | (op & 0x7))
+
+enum {
+OPC_GSLBX  = 0x0 | OPC_LDC2,
+OPC_GSLHX  = 0x1 | OPC_LDC2,
+OPC_GSLWX  = 0x2 | OPC_LDC2,
+OPC_GSLDX  = 0x3 | OPC_LDC2,
+OPC_GSLWXC1= 0x6 | OPC_LDC2,
+OPC_GSLDXC1= 0x7 | OPC_LDC2,
+OPC_GSSBX  = 0x0 | OPC_SDC2,
+OPC_GSSHX  = 0x1 | OPC_SDC2,
+OPC_GSSWX  = 0x2 | OPC_SDC2,
+OPC_GSSDX  = 0x3 | OPC_SDC2,
+OPC_GSSWXC1= 0x6 | OPC_SDC2,
+OPC_GSSDXC1= 0x7 | OPC_SDC2,
+};
+
   /* BSHFL opcodes */
   #define MASK_BSHFL(op)  (MASK_SPECIAL3(op) | (op & (0x1F << 6)))

@@ -6172,6 +6190,165 @@ static void gen_loongson_lswc2(DisasContext *ctx, int 
rt,
   tcg_temp_free(t0);
   }

+/* Loongson EXT LDC2/SDC2 */
+static void gen_loongson_lsdc2(DisasContext *ctx, int rt,
+int rs, int rd)


Alignment off (various occurences in this series).

OK, thanks.




+{
+int offset = (int8_t)(ctx->opcode >> 3);


Please use sextract32() which is easier to read:

 int32_t offset = sextract32(ctx->opcode, 3, 8);

OK, thanks.




+uint32_t opc = MASK_LOONGSON_LSDC2(ctx->opcode);
+TCGv t0, t1;
+TCGv_i32 fp0;
+
+/* Pre-conditions */
+switch (opc) {
+case OPC_GSLBX:
+case OPC_GSLHX:
+case OPC_GSLWX:
+case OPC_GSLDX:
+/* prefetch, implement as NOP */
+if (rt == 0) {
+return;
+}
+break;
+case OPC_GSSBX:
+case OPC_GSSHX:
+case OPC_GSSWX:
+case OPC_GSSDX:
+break;
+case OPC_GSLWXC1:
+#if defined(TARGET_MIPS64)
+case OPC_GSLDXC1:
+#endif
+check_cp1_enabled(ctx);
+/* prefetch, implement as NOP */
+if (rt == 0) {
+return;
+}
+break;
+case OPC_GSSWXC1:
+#if defined(TARGET_MIPS64)
+case OPC_GSSDXC1:
+#endif
+check_cp1_enabled(ctx);
+break;
+default:
+MIPS_INVAL("loongson_lsdc2");
+generate_exception_end(ctx, EXCP_RI);
+return;
+break;
+}
+
+t0 = tcg_temp_new();
+
+gen_base_offset_addr(ctx, t0, rs, offset);
+gen_op_addr_add(ctx, t0, cpu_gpr[rd], t0);
+
+switch (opc) {
+case OPC_GSLBX:
+tcg_gen_qemu_ld_tl(t0, t0, ctx->mem_idx, MO_SB);
+gen_store_gpr(t0, rt);
+break;
+case OPC_GSLHX:
+tcg_gen_qemu_ld_tl(t0, t0, ctx->mem_idx, MO_TESW |
+ctx->default_tcg_memop_mask);


Do Loongson EXT plan to support unaligned accesses?

Not support in hardware, and Linux kernel emulate the unaligned cases.


OK, that was my understanding. So we don't need to use
default_tcg_memop_mask, we can directly use MO_ALIGN in
place instead.

Regards,

Phil.

Re: [PULL v3 0/8] NBD patches through 2020-10-08

[Peter Maydell]

On Fri, 9 Oct 2020 at 21:15, Eric Blake  wrote:
>
> The following changes since commit 4a7c0bd9dcb08798c6f82e55b5a3423f7ee669f1:
>
>   Merge remote-tracking branch 'remotes/dgibson/tags/ppc-for-5.2-20201009' 
> into staging (2020-10-09 15:48:04 +0100)
>
> are available in the Git repository at:
>
>   https://repo.or.cz/qemu/ericb.git tags/pull-nbd-2020-10-08-v3
>
> for you to fetch changes up to ebd57062a1957307a175a810441af87259d7dbe9:
>
>   nbd: Simplify meta-context parsing (2020-10-09 15:05:08 -0500)
>
> v2: fix BSD build
> v3: fix mingw build (only the affected patch resent)
>
> 
> nbd patches for 2020-10-08
>
> - silence compilation warnings
> - more fixes to prevent reconnect hangs
> - improve 'qemu-nbd' termination behavior
> - cleaner NBD protocol compliance on string handling
>

Applied, thanks.

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

-- PMM

[PATCH v1 3/8] migration: Add spaces around operator

[Bihong Yu]

Signed-off-by:Bihong Yu 
Reviewed-by: Chuan Zheng 
Signed-off-by: Bihong Yu 
---
 migration/migration.c|  4 ++--
 migration/postcopy-ram.c |  2 +-
 migration/ram.c  |  2 +-
 migration/savevm.c   |  2 +-
 migration/vmstate.c  | 10 +-
 5 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index 0575ecb..e050f57 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2478,8 +2478,8 @@ static void migrate_handle_rp_req_pages(MigrationState 
*ms, const char* rbname,
  * Since we currently insist on matching page sizes, just sanity check
  * we're being asked for whole host pages.
  */
-if (start & (our_host_ps-1) ||
-   (len & (our_host_ps-1))) {
+if (start & (our_host_ps - 1) ||
+   (len & (our_host_ps - 1))) {
 error_report("%s: Misaligned page request, start: " RAM_ADDR_FMT
  " len: %zd", __func__, start, len);
 mark_source_rp_bad(ms);
diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index 0a2f88a8..eea92bb 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -403,7 +403,7 @@ bool postcopy_ram_supported_by_host(MigrationIncomingState 
*mis)
  strerror(errno));
 goto out;
 }
-g_assert(((size_t)testarea & (pagesize-1)) == 0);
+g_assert(((size_t)testarea & (pagesize - 1)) == 0);
 
 reg_struct.range.start = (uintptr_t)testarea;
 reg_struct.range.len = pagesize;
diff --git a/migration/ram.c b/migration/ram.c
index 59bdd15..90b277b 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1563,7 +1563,7 @@ int ram_save_queue_pages(const char *rbname, ram_addr_t 
start, ram_addr_t len)
 rs->last_req_rb = ramblock;
 }
 trace_ram_save_queue_pages(ramblock->idstr, start, len);
-if (start+len > ramblock->used_length) {
+if (start + len > ramblock->used_length) {
 error_report("%s request overrun start=" RAM_ADDR_FMT " len="
  RAM_ADDR_FMT " blocklen=" RAM_ADDR_FMT,
  __func__, start, len, ramblock->used_length);
diff --git a/migration/savevm.c b/migration/savevm.c
index d2e141f..9e95df1 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -521,7 +521,7 @@ static const VMStateDescription vmstate_configuration = {
 VMSTATE_VBUFFER_ALLOC_UINT32(name, SaveState, 0, NULL, len),
 VMSTATE_END_OF_LIST()
 },
-.subsections = (const VMStateDescription*[]) {
+.subsections = (const VMStateDescription * []) {
 &vmstate_target_page_bits,
 &vmstate_capabilites,
 &vmstate_uuid,
diff --git a/migration/vmstate.c b/migration/vmstate.c
index bafa890..e9d2aef 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -32,13 +32,13 @@ static int vmstate_n_elems(void *opaque, const VMStateField 
*field)
 if (field->flags & VMS_ARRAY) {
 n_elems = field->num;
 } else if (field->flags & VMS_VARRAY_INT32) {
-n_elems = *(int32_t *)(opaque+field->num_offset);
+n_elems = *(int32_t *)(opaque + field->num_offset);
 } else if (field->flags & VMS_VARRAY_UINT32) {
-n_elems = *(uint32_t *)(opaque+field->num_offset);
+n_elems = *(uint32_t *)(opaque + field->num_offset);
 } else if (field->flags & VMS_VARRAY_UINT16) {
-n_elems = *(uint16_t *)(opaque+field->num_offset);
+n_elems = *(uint16_t *)(opaque + field->num_offset);
 } else if (field->flags & VMS_VARRAY_UINT8) {
-n_elems = *(uint8_t *)(opaque+field->num_offset);
+n_elems = *(uint8_t *)(opaque + field->num_offset);
 }
 
 if (field->flags & VMS_MULTIPLY_ELEMENTS) {
@@ -54,7 +54,7 @@ static int vmstate_size(void *opaque, const VMStateField 
*field)
 int size = field->size;
 
 if (field->flags & VMS_VBUFFER) {
-size = *(int32_t *)(opaque+field->size_offset);
+size = *(int32_t *)(opaque + field->size_offset);
 if (field->flags & VMS_MULTIPLY) {
 size *= field->size;
 }
-- 
1.8.3.1

[PATCH v1 3/8] migration: Add spaces around operator

[Bihong Yu]

Signed-off-by:Bihong Yu 
Reviewed-by: Chuan Zheng 
Signed-off-by: Bihong Yu 
---
 migration/migration.c|  4 ++--
 migration/postcopy-ram.c |  2 +-
 migration/ram.c  |  2 +-
 migration/savevm.c   |  2 +-
 migration/vmstate.c  | 10 +-
 5 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index 0575ecb..e050f57 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2478,8 +2478,8 @@ static void migrate_handle_rp_req_pages(MigrationState 
*ms, const char* rbname,
  * Since we currently insist on matching page sizes, just sanity check
  * we're being asked for whole host pages.
  */
-if (start & (our_host_ps-1) ||
-   (len & (our_host_ps-1))) {
+if (start & (our_host_ps - 1) ||
+   (len & (our_host_ps - 1))) {
 error_report("%s: Misaligned page request, start: " RAM_ADDR_FMT
  " len: %zd", __func__, start, len);
 mark_source_rp_bad(ms);
diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index 0a2f88a8..eea92bb 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -403,7 +403,7 @@ bool postcopy_ram_supported_by_host(MigrationIncomingState 
*mis)
  strerror(errno));
 goto out;
 }
-g_assert(((size_t)testarea & (pagesize-1)) == 0);
+g_assert(((size_t)testarea & (pagesize - 1)) == 0);
 
 reg_struct.range.start = (uintptr_t)testarea;
 reg_struct.range.len = pagesize;
diff --git a/migration/ram.c b/migration/ram.c
index 59bdd15..90b277b 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1563,7 +1563,7 @@ int ram_save_queue_pages(const char *rbname, ram_addr_t 
start, ram_addr_t len)
 rs->last_req_rb = ramblock;
 }
 trace_ram_save_queue_pages(ramblock->idstr, start, len);
-if (start+len > ramblock->used_length) {
+if (start + len > ramblock->used_length) {
 error_report("%s request overrun start=" RAM_ADDR_FMT " len="
  RAM_ADDR_FMT " blocklen=" RAM_ADDR_FMT,
  __func__, start, len, ramblock->used_length);
diff --git a/migration/savevm.c b/migration/savevm.c
index d2e141f..9e95df1 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -521,7 +521,7 @@ static const VMStateDescription vmstate_configuration = {
 VMSTATE_VBUFFER_ALLOC_UINT32(name, SaveState, 0, NULL, len),
 VMSTATE_END_OF_LIST()
 },
-.subsections = (const VMStateDescription*[]) {
+.subsections = (const VMStateDescription * []) {
 &vmstate_target_page_bits,
 &vmstate_capabilites,
 &vmstate_uuid,
diff --git a/migration/vmstate.c b/migration/vmstate.c
index bafa890..e9d2aef 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -32,13 +32,13 @@ static int vmstate_n_elems(void *opaque, const VMStateField 
*field)
 if (field->flags & VMS_ARRAY) {
 n_elems = field->num;
 } else if (field->flags & VMS_VARRAY_INT32) {
-n_elems = *(int32_t *)(opaque+field->num_offset);
+n_elems = *(int32_t *)(opaque + field->num_offset);
 } else if (field->flags & VMS_VARRAY_UINT32) {
-n_elems = *(uint32_t *)(opaque+field->num_offset);
+n_elems = *(uint32_t *)(opaque + field->num_offset);
 } else if (field->flags & VMS_VARRAY_UINT16) {
-n_elems = *(uint16_t *)(opaque+field->num_offset);
+n_elems = *(uint16_t *)(opaque + field->num_offset);
 } else if (field->flags & VMS_VARRAY_UINT8) {
-n_elems = *(uint8_t *)(opaque+field->num_offset);
+n_elems = *(uint8_t *)(opaque + field->num_offset);
 }
 
 if (field->flags & VMS_MULTIPLY_ELEMENTS) {
@@ -54,7 +54,7 @@ static int vmstate_size(void *opaque, const VMStateField 
*field)
 int size = field->size;
 
 if (field->flags & VMS_VBUFFER) {
-size = *(int32_t *)(opaque+field->size_offset);
+size = *(int32_t *)(opaque + field->size_offset);
 if (field->flags & VMS_MULTIPLY) {
 size *= field->size;
 }
-- 
1.8.3.1

[PATCH v1 7/8] migration: Open brace '{' following function declarations go on the next line

[Bihong Yu]

Signed-off-by:Bihong Yu 
Reviewed-by: Chuan Zheng 
Signed-off-by: Bihong Yu 
---
 migration/rdma.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/migration/rdma.c b/migration/rdma.c
index 0eb42b7..ca4d315 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -273,7 +273,8 @@ static uint64_t htonll(uint64_t v)
 return u.llv;
 }
 
-static uint64_t ntohll(uint64_t v) {
+static uint64_t ntohll(uint64_t v)
+{
 union { uint32_t lv[2]; uint64_t llv; } u;
 u.llv = v;
 return ((uint64_t)ntohl(u.lv[0]) << 32) | (uint64_t) ntohl(u.lv[1]);
-- 
1.8.3.1

[PATCH v1 5/8] migration: Add braces {} for if statement

[Bihong Yu]

Signed-off-by:Bihong Yu 
Reviewed-by: Chuan Zheng 
Signed-off-by: Bihong Yu 
---
 migration/ram.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index 90b277b..12e7296 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -101,14 +101,16 @@ static struct {
 
 static void XBZRLE_cache_lock(void)
 {
-if (migrate_use_xbzrle())
+if (migrate_use_xbzrle()) {
 qemu_mutex_lock(&XBZRLE.lock);
+}
 }
 
 static void XBZRLE_cache_unlock(void)
 {
-if (migrate_use_xbzrle())
+if (migrate_use_xbzrle()) {
 qemu_mutex_unlock(&XBZRLE.lock);
+}
 }
 
 /**
-- 
1.8.3.1

[PATCH v1 6/8] migration: Do not initialise statics and globals to 0 or NULL

[Bihong Yu]

Signed-off-by:Bihong Yu 
Reviewed-by: Chuan Zheng 
Signed-off-by: Bihong Yu 
---
 migration/ram.c| 2 +-
 migration/savevm.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index 12e7296..f71ff2b 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -2743,7 +2743,7 @@ static int load_xbzrle(QEMUFile *f, ram_addr_t addr, void 
*host)
  */
 static inline RAMBlock *ram_block_from_stream(QEMUFile *f, int flags)
 {
-static RAMBlock *block = NULL;
+static RAMBlock *block;
 char id[256];
 uint8_t len;
 
diff --git a/migration/savevm.c b/migration/savevm.c
index 9e95df1..f808bc2 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -64,7 +64,7 @@
 #include "qemu/bitmap.h"
 #include "net/announce.h"
 
-const unsigned int postcopy_ram_discard_version = 0;
+const unsigned int postcopy_ram_discard_version;
 
 /* Subcommands for QEMU_VM_COMMAND */
 enum qemu_vm_cmd {
-- 
1.8.3.1

[PATCH v1 8/8] migration: Delete redundant spaces

[Bihong Yu]

Signed-off-by:Bihong Yu 
Reviewed-by: Chuan Zheng 
Signed-off-by: Bihong Yu 
---
 migration/rdma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/migration/rdma.c b/migration/rdma.c
index ca4d315..00eac34 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -855,7 +855,7 @@ static int qemu_rdma_broken_ipv6_kernel(struct ibv_context 
*verbs, Error **errp)
  */
 if (!verbs) {
 int num_devices, x;
-struct ibv_device ** dev_list = ibv_get_device_list(&num_devices);
+struct ibv_device **dev_list = ibv_get_device_list(&num_devices);
 bool roce_found = false;
 bool ib_found = false;
 
-- 
1.8.3.1

[PATCH v1 0/8] Fix some style problems in migration

[Bihong Yu]

Recently I am reading migration related code, find some style problems in
migration directory while using checkpatch.pl to check migration code. Fix the
error style problems.

Bihong Yu (8):
  migration: Do not use C99 // comments
  migration: Don't use '#' flag of printf format
  migration: Add spaces around operator
  migration: Open brace '{' following struct go on the same line
  migration: Add braces {} for if statement
  migration: Do not initialise statics and globals to 0 or NULL
  migration: Open brace '{' following function declarations go on the
next line
  migration: Delete redundant spaces

 migration/block.c|  4 ++--
 migration/migration.c|  4 ++--
 migration/migration.h|  3 +--
 migration/postcopy-ram.c |  2 +-
 migration/ram.c  | 14 --
 migration/rdma.c |  7 ---
 migration/savevm.c   |  4 ++--
 migration/vmstate.c  | 10 +-
 8 files changed, 25 insertions(+), 23 deletions(-)

-- 
1.8.3.1

[PATCH v1 8/8] migration: Delete redundant spaces

[Bihong Yu]

Signed-off-by:Bihong Yu 
Reviewed-by: Chuan Zheng 
Signed-off-by: Bihong Yu 
---
 migration/rdma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/migration/rdma.c b/migration/rdma.c
index ca4d315..00eac34 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -855,7 +855,7 @@ static int qemu_rdma_broken_ipv6_kernel(struct ibv_context 
*verbs, Error **errp)
  */
 if (!verbs) {
 int num_devices, x;
-struct ibv_device ** dev_list = ibv_get_device_list(&num_devices);
+struct ibv_device **dev_list = ibv_get_device_list(&num_devices);
 bool roce_found = false;
 bool ib_found = false;
 
-- 
1.8.3.1

[PATCH v1 2/8] migration: Don't use '#' flag of printf format

[Bihong Yu]

Signed-off-by:Bihong Yu 
Reviewed-by: Chuan Zheng 
Signed-off-by: Bihong Yu 
---
 migration/block.c | 2 +-
 migration/ram.c   | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/migration/block.c b/migration/block.c
index 4b8576b..399dfb8 100644
--- a/migration/block.c
+++ b/migration/block.c
@@ -998,7 +998,7 @@ static int block_load(QEMUFile *f, void *opaque, int 
version_id)
(addr == 100) ? '\n' : '\r');
 fflush(stdout);
 } else if (!(flags & BLK_MIG_FLAG_EOS)) {
-fprintf(stderr, "Unknown block migration flags: %#x\n", flags);
+fprintf(stderr, "Unknown block migration flags: %0x\n", flags);
 return -EINVAL;
 }
 ret = qemu_file_get_error(f);
diff --git a/migration/ram.c b/migration/ram.c
index 433489d..59bdd15 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -3298,7 +3298,7 @@ static int ram_load_postcopy(QEMUFile *f)
 multifd_recv_sync_main();
 break;
 default:
-error_report("Unknown combination of migration flags: %#x"
+error_report("Unknown combination of migration flags: %0x"
  " (postcopy mode)", flags);
 ret = -EINVAL;
 break;
@@ -3576,7 +3576,7 @@ static int ram_load_precopy(QEMUFile *f)
 if (flags & RAM_SAVE_FLAG_HOOK) {
 ram_control_load_hook(f, RAM_CONTROL_HOOK, NULL);
 } else {
-error_report("Unknown combination of migration flags: %#x",
+error_report("Unknown combination of migration flags: %0x",
  flags);
 ret = -EINVAL;
 }
-- 
1.8.3.1

[PATCH v1 7/8] migration: Open brace '{' following function declarations go on the next line

[Bihong Yu]

Signed-off-by:Bihong Yu 
Reviewed-by: Chuan Zheng 
Signed-off-by: Bihong Yu 
---
 migration/rdma.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/migration/rdma.c b/migration/rdma.c
index 0eb42b7..ca4d315 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -273,7 +273,8 @@ static uint64_t htonll(uint64_t v)
 return u.llv;
 }
 
-static uint64_t ntohll(uint64_t v) {
+static uint64_t ntohll(uint64_t v)
+{
 union { uint32_t lv[2]; uint64_t llv; } u;
 u.llv = v;
 return ((uint64_t)ntohl(u.lv[0]) << 32) | (uint64_t) ntohl(u.lv[1]);
-- 
1.8.3.1

[PATCH v1 2/8] migration: Don't use '#' flag of printf format

[Bihong Yu]

Signed-off-by:Bihong Yu 
Reviewed-by: Chuan Zheng 
Signed-off-by: Bihong Yu 
---
 migration/block.c | 2 +-
 migration/ram.c   | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/migration/block.c b/migration/block.c
index 4b8576b..399dfb8 100644
--- a/migration/block.c
+++ b/migration/block.c
@@ -998,7 +998,7 @@ static int block_load(QEMUFile *f, void *opaque, int 
version_id)
(addr == 100) ? '\n' : '\r');
 fflush(stdout);
 } else if (!(flags & BLK_MIG_FLAG_EOS)) {
-fprintf(stderr, "Unknown block migration flags: %#x\n", flags);
+fprintf(stderr, "Unknown block migration flags: %0x\n", flags);
 return -EINVAL;
 }
 ret = qemu_file_get_error(f);
diff --git a/migration/ram.c b/migration/ram.c
index 433489d..59bdd15 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -3298,7 +3298,7 @@ static int ram_load_postcopy(QEMUFile *f)
 multifd_recv_sync_main();
 break;
 default:
-error_report("Unknown combination of migration flags: %#x"
+error_report("Unknown combination of migration flags: %0x"
  " (postcopy mode)", flags);
 ret = -EINVAL;
 break;
@@ -3576,7 +3576,7 @@ static int ram_load_precopy(QEMUFile *f)
 if (flags & RAM_SAVE_FLAG_HOOK) {
 ram_control_load_hook(f, RAM_CONTROL_HOOK, NULL);
 } else {
-error_report("Unknown combination of migration flags: %#x",
+error_report("Unknown combination of migration flags: %0x",
  flags);
 ret = -EINVAL;
 }
-- 
1.8.3.1

[PATCH v1 1/8] migration: Do not use C99 // comments

[Bihong Yu]

Signed-off-by:Bihong Yu 
Reviewed-by: Chuan Zheng 
Signed-off-by: Bihong Yu 
---
 migration/block.c | 2 +-
 migration/rdma.c  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/migration/block.c b/migration/block.c
index 737b649..4b8576b 100644
--- a/migration/block.c
+++ b/migration/block.c
@@ -40,7 +40,7 @@
 #define MAX_IO_BUFFERS 512
 #define MAX_PARALLEL_IO 16
 
-//#define DEBUG_BLK_MIGRATION
+/* #define DEBUG_BLK_MIGRATION */
 
 #ifdef DEBUG_BLK_MIGRATION
 #define DPRINTF(fmt, ...) \
diff --git a/migration/rdma.c b/migration/rdma.c
index 0340841..0eb42b7 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -1288,7 +1288,7 @@ const char *print_wrid(int wrid)
  * workload information or LRU information is available, do not attempt to use
  * this feature except for basic testing.
  */
-//#define RDMA_UNREGISTRATION_EXAMPLE
+/* #define RDMA_UNREGISTRATION_EXAMPLE */
 
 /*
  * Perform a non-optimized memory unregistration after every transfer
-- 
1.8.3.1

[PATCH v1 4/8] migration: Open brace '{' following struct go on the same line

[Bihong Yu]

Signed-off-by:Bihong Yu 
Reviewed-by: Chuan Zheng 
Signed-off-by: Bihong Yu 
---
 migration/migration.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/migration/migration.h b/migration/migration.h
index deb411a..99784b4 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -124,8 +124,7 @@ struct MigrationClass {
 DeviceClass parent_class;
 };
 
-struct MigrationState
-{
+struct MigrationState {
 /*< private >*/
 DeviceState parent_obj;
 
-- 
1.8.3.1

[PATCH v1 4/8] migration: Open brace '{' following struct go on the same line

[Bihong Yu]

Signed-off-by:Bihong Yu 
Reviewed-by: Chuan Zheng 
Signed-off-by: Bihong Yu 
---
 migration/migration.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/migration/migration.h b/migration/migration.h
index deb411a..99784b4 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -124,8 +124,7 @@ struct MigrationClass {
 DeviceClass parent_class;
 };
 
-struct MigrationState
-{
+struct MigrationState {
 /*< private >*/
 DeviceState parent_obj;
 
-- 
1.8.3.1

[PATCH v1 1/8] migration: Do not use C99 // comments

[Bihong Yu]

Signed-off-by:Bihong Yu 
Reviewed-by: Chuan Zheng 
Signed-off-by: Bihong Yu 
---
 migration/block.c | 2 +-
 migration/rdma.c  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/migration/block.c b/migration/block.c
index 737b649..4b8576b 100644
--- a/migration/block.c
+++ b/migration/block.c
@@ -40,7 +40,7 @@
 #define MAX_IO_BUFFERS 512
 #define MAX_PARALLEL_IO 16
 
-//#define DEBUG_BLK_MIGRATION
+/* #define DEBUG_BLK_MIGRATION */
 
 #ifdef DEBUG_BLK_MIGRATION
 #define DPRINTF(fmt, ...) \
diff --git a/migration/rdma.c b/migration/rdma.c
index 0340841..0eb42b7 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -1288,7 +1288,7 @@ const char *print_wrid(int wrid)
  * workload information or LRU information is available, do not attempt to use
  * this feature except for basic testing.
  */
-//#define RDMA_UNREGISTRATION_EXAMPLE
+/* #define RDMA_UNREGISTRATION_EXAMPLE */
 
 /*
  * Perform a non-optimized memory unregistration after every transfer
-- 
1.8.3.1

[PATCH v1 7/8] migration: Open brace '{' following function declarations go on the next line

[Bihong Yu]

Signed-off-by:Bihong Yu 
Reviewed-by: Chuan Zheng 
Signed-off-by: Bihong Yu 
---
 migration/rdma.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/migration/rdma.c b/migration/rdma.c
index 0eb42b7..ca4d315 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -273,7 +273,8 @@ static uint64_t htonll(uint64_t v)
 return u.llv;
 }
 
-static uint64_t ntohll(uint64_t v) {
+static uint64_t ntohll(uint64_t v)
+{
 union { uint32_t lv[2]; uint64_t llv; } u;
 u.llv = v;
 return ((uint64_t)ntohl(u.lv[0]) << 32) | (uint64_t) ntohl(u.lv[1]);
-- 
1.8.3.1

[PATCH v1 8/8] migration: Delete redundant spaces

[Bihong Yu]

Signed-off-by:Bihong Yu 
Reviewed-by: Chuan Zheng 
Signed-off-by: Bihong Yu 
---
 migration/rdma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/migration/rdma.c b/migration/rdma.c
index ca4d315..00eac34 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -855,7 +855,7 @@ static int qemu_rdma_broken_ipv6_kernel(struct ibv_context 
*verbs, Error **errp)
  */
 if (!verbs) {
 int num_devices, x;
-struct ibv_device ** dev_list = ibv_get_device_list(&num_devices);
+struct ibv_device **dev_list = ibv_get_device_list(&num_devices);
 bool roce_found = false;
 bool ib_found = false;
 
-- 
1.8.3.1

[PATCH v1 0/8] Fix some style problems in migration

[Bihong Yu]

Recently I am reading migration related code, find some style problems in
migration directory while using checkpatch.pl to check migration code. Fix the
error style problems.

Bihong Yu (8):
  migration: Do not use C99 // comments
  migration: Don't use '#' flag of printf format
  migration: Add spaces around operator
  migration: Open brace '{' following struct go on the same line
  migration: Add braces {} for if statement
  migration: Do not initialise statics and globals to 0 or NULL
  migration: Open brace '{' following function declarations go on the
next line
  migration: Delete redundant spaces

 migration/block.c|  4 ++--
 migration/migration.c|  4 ++--
 migration/migration.h|  3 +--
 migration/postcopy-ram.c |  2 +-
 migration/ram.c  | 14 --
 migration/rdma.c |  7 ---
 migration/savevm.c   |  4 ++--
 migration/vmstate.c  | 10 +-
 8 files changed, 25 insertions(+), 23 deletions(-)

-- 
1.8.3.1

[PATCH v1 6/8] migration: Do not initialise statics and globals to 0 or NULL

[Bihong Yu]

Signed-off-by:Bihong Yu 
Reviewed-by: Chuan Zheng 
Signed-off-by: Bihong Yu 
---
 migration/ram.c| 2 +-
 migration/savevm.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index 12e7296..f71ff2b 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -2743,7 +2743,7 @@ static int load_xbzrle(QEMUFile *f, ram_addr_t addr, void 
*host)
  */
 static inline RAMBlock *ram_block_from_stream(QEMUFile *f, int flags)
 {
-static RAMBlock *block = NULL;
+static RAMBlock *block;
 char id[256];
 uint8_t len;
 
diff --git a/migration/savevm.c b/migration/savevm.c
index 9e95df1..f808bc2 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -64,7 +64,7 @@
 #include "qemu/bitmap.h"
 #include "net/announce.h"
 
-const unsigned int postcopy_ram_discard_version = 0;
+const unsigned int postcopy_ram_discard_version;
 
 /* Subcommands for QEMU_VM_COMMAND */
 enum qemu_vm_cmd {
-- 
1.8.3.1

[PATCH v1 5/8] migration: Add braces {} for if statement

[Bihong Yu]

Signed-off-by:Bihong Yu 
Reviewed-by: Chuan Zheng 
Signed-off-by: Bihong Yu 
---
 migration/ram.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index 90b277b..12e7296 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -101,14 +101,16 @@ static struct {
 
 static void XBZRLE_cache_lock(void)
 {
-if (migrate_use_xbzrle())
+if (migrate_use_xbzrle()) {
 qemu_mutex_lock(&XBZRLE.lock);
+}
 }
 
 static void XBZRLE_cache_unlock(void)
 {
-if (migrate_use_xbzrle())
+if (migrate_use_xbzrle()) {
 qemu_mutex_unlock(&XBZRLE.lock);
+}
 }
 
 /**
-- 
1.8.3.1

[PATCH v1 0/8] Fix some style problems in migration

[Bihong Yu]

Recently I am reading migration related code, find some style problems in
migration directory while using checkpatch.pl to check migration code. Fix the
error style problems.

Bihong Yu (8):
  migration: Do not use C99 // comments
  migration: Don't use '#' flag of printf format
  migration: Add spaces around operator
  migration: Open brace '{' following struct go on the same line
  migration: Add braces {} for if statement
  migration: Do not initialise statics and globals to 0 or NULL
  migration: Open brace '{' following function declarations go on the
next line
  migration: Delete redundant spaces

 migration/block.c|  4 ++--
 migration/migration.c|  4 ++--
 migration/migration.h|  3 +--
 migration/postcopy-ram.c |  2 +-
 migration/ram.c  | 14 --
 migration/rdma.c |  7 ---
 migration/savevm.c   |  4 ++--
 migration/vmstate.c  | 10 +-
 8 files changed, 25 insertions(+), 23 deletions(-)

-- 
1.8.3.1

[PATCH v1 1/8] migration: Do not use C99 // comments

[Bihong Yu]

Signed-off-by:Bihong Yu 
Reviewed-by: Chuan Zheng 
Signed-off-by: Bihong Yu 
---
 migration/block.c | 2 +-
 migration/rdma.c  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/migration/block.c b/migration/block.c
index 737b649..4b8576b 100644
--- a/migration/block.c
+++ b/migration/block.c
@@ -40,7 +40,7 @@
 #define MAX_IO_BUFFERS 512
 #define MAX_PARALLEL_IO 16
 
-//#define DEBUG_BLK_MIGRATION
+/* #define DEBUG_BLK_MIGRATION */
 
 #ifdef DEBUG_BLK_MIGRATION
 #define DPRINTF(fmt, ...) \
diff --git a/migration/rdma.c b/migration/rdma.c
index 0340841..0eb42b7 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -1288,7 +1288,7 @@ const char *print_wrid(int wrid)
  * workload information or LRU information is available, do not attempt to use
  * this feature except for basic testing.
  */
-//#define RDMA_UNREGISTRATION_EXAMPLE
+/* #define RDMA_UNREGISTRATION_EXAMPLE */
 
 /*
  * Perform a non-optimized memory unregistration after every transfer
-- 
1.8.3.1

[PATCH v1 3/8] migration: Add spaces around operator

[Bihong Yu]

Signed-off-by:Bihong Yu 
Reviewed-by: Chuan Zheng 
Signed-off-by: Bihong Yu 
---
 migration/migration.c|  4 ++--
 migration/postcopy-ram.c |  2 +-
 migration/ram.c  |  2 +-
 migration/savevm.c   |  2 +-
 migration/vmstate.c  | 10 +-
 5 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index 0575ecb..e050f57 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2478,8 +2478,8 @@ static void migrate_handle_rp_req_pages(MigrationState 
*ms, const char* rbname,
  * Since we currently insist on matching page sizes, just sanity check
  * we're being asked for whole host pages.
  */
-if (start & (our_host_ps-1) ||
-   (len & (our_host_ps-1))) {
+if (start & (our_host_ps - 1) ||
+   (len & (our_host_ps - 1))) {
 error_report("%s: Misaligned page request, start: " RAM_ADDR_FMT
  " len: %zd", __func__, start, len);
 mark_source_rp_bad(ms);
diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index 0a2f88a8..eea92bb 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -403,7 +403,7 @@ bool postcopy_ram_supported_by_host(MigrationIncomingState 
*mis)
  strerror(errno));
 goto out;
 }
-g_assert(((size_t)testarea & (pagesize-1)) == 0);
+g_assert(((size_t)testarea & (pagesize - 1)) == 0);
 
 reg_struct.range.start = (uintptr_t)testarea;
 reg_struct.range.len = pagesize;
diff --git a/migration/ram.c b/migration/ram.c
index 59bdd15..90b277b 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1563,7 +1563,7 @@ int ram_save_queue_pages(const char *rbname, ram_addr_t 
start, ram_addr_t len)
 rs->last_req_rb = ramblock;
 }
 trace_ram_save_queue_pages(ramblock->idstr, start, len);
-if (start+len > ramblock->used_length) {
+if (start + len > ramblock->used_length) {
 error_report("%s request overrun start=" RAM_ADDR_FMT " len="
  RAM_ADDR_FMT " blocklen=" RAM_ADDR_FMT,
  __func__, start, len, ramblock->used_length);
diff --git a/migration/savevm.c b/migration/savevm.c
index d2e141f..9e95df1 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -521,7 +521,7 @@ static const VMStateDescription vmstate_configuration = {
 VMSTATE_VBUFFER_ALLOC_UINT32(name, SaveState, 0, NULL, len),
 VMSTATE_END_OF_LIST()
 },
-.subsections = (const VMStateDescription*[]) {
+.subsections = (const VMStateDescription * []) {
 &vmstate_target_page_bits,
 &vmstate_capabilites,
 &vmstate_uuid,
diff --git a/migration/vmstate.c b/migration/vmstate.c
index bafa890..e9d2aef 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -32,13 +32,13 @@ static int vmstate_n_elems(void *opaque, const VMStateField 
*field)
 if (field->flags & VMS_ARRAY) {
 n_elems = field->num;
 } else if (field->flags & VMS_VARRAY_INT32) {
-n_elems = *(int32_t *)(opaque+field->num_offset);
+n_elems = *(int32_t *)(opaque + field->num_offset);
 } else if (field->flags & VMS_VARRAY_UINT32) {
-n_elems = *(uint32_t *)(opaque+field->num_offset);
+n_elems = *(uint32_t *)(opaque + field->num_offset);
 } else if (field->flags & VMS_VARRAY_UINT16) {
-n_elems = *(uint16_t *)(opaque+field->num_offset);
+n_elems = *(uint16_t *)(opaque + field->num_offset);
 } else if (field->flags & VMS_VARRAY_UINT8) {
-n_elems = *(uint8_t *)(opaque+field->num_offset);
+n_elems = *(uint8_t *)(opaque + field->num_offset);
 }
 
 if (field->flags & VMS_MULTIPLY_ELEMENTS) {
@@ -54,7 +54,7 @@ static int vmstate_size(void *opaque, const VMStateField 
*field)
 int size = field->size;
 
 if (field->flags & VMS_VBUFFER) {
-size = *(int32_t *)(opaque+field->size_offset);
+size = *(int32_t *)(opaque + field->size_offset);
 if (field->flags & VMS_MULTIPLY) {
 size *= field->size;
 }
-- 
1.8.3.1

[PATCH v1 2/8] migration: Don't use '#' flag of printf format

[Bihong Yu]

Signed-off-by:Bihong Yu 
Reviewed-by: Chuan Zheng 
Signed-off-by: Bihong Yu 
---
 migration/block.c | 2 +-
 migration/ram.c   | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/migration/block.c b/migration/block.c
index 4b8576b..399dfb8 100644
--- a/migration/block.c
+++ b/migration/block.c
@@ -998,7 +998,7 @@ static int block_load(QEMUFile *f, void *opaque, int 
version_id)
(addr == 100) ? '\n' : '\r');
 fflush(stdout);
 } else if (!(flags & BLK_MIG_FLAG_EOS)) {
-fprintf(stderr, "Unknown block migration flags: %#x\n", flags);
+fprintf(stderr, "Unknown block migration flags: %0x\n", flags);
 return -EINVAL;
 }
 ret = qemu_file_get_error(f);
diff --git a/migration/ram.c b/migration/ram.c
index 433489d..59bdd15 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -3298,7 +3298,7 @@ static int ram_load_postcopy(QEMUFile *f)
 multifd_recv_sync_main();
 break;
 default:
-error_report("Unknown combination of migration flags: %#x"
+error_report("Unknown combination of migration flags: %0x"
  " (postcopy mode)", flags);
 ret = -EINVAL;
 break;
@@ -3576,7 +3576,7 @@ static int ram_load_precopy(QEMUFile *f)
 if (flags & RAM_SAVE_FLAG_HOOK) {
 ram_control_load_hook(f, RAM_CONTROL_HOOK, NULL);
 } else {
-error_report("Unknown combination of migration flags: %#x",
+error_report("Unknown combination of migration flags: %0x",
  flags);
 ret = -EINVAL;
 }
-- 
1.8.3.1

[PATCH v1 6/8] migration: Do not initialise statics and globals to 0 or NULL

[Bihong Yu]

Signed-off-by:Bihong Yu 
Reviewed-by: Chuan Zheng 
Signed-off-by: Bihong Yu 
---
 migration/ram.c| 2 +-
 migration/savevm.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index 12e7296..f71ff2b 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -2743,7 +2743,7 @@ static int load_xbzrle(QEMUFile *f, ram_addr_t addr, void 
*host)
  */
 static inline RAMBlock *ram_block_from_stream(QEMUFile *f, int flags)
 {
-static RAMBlock *block = NULL;
+static RAMBlock *block;
 char id[256];
 uint8_t len;
 
diff --git a/migration/savevm.c b/migration/savevm.c
index 9e95df1..f808bc2 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -64,7 +64,7 @@
 #include "qemu/bitmap.h"
 #include "net/announce.h"
 
-const unsigned int postcopy_ram_discard_version = 0;
+const unsigned int postcopy_ram_discard_version;
 
 /* Subcommands for QEMU_VM_COMMAND */
 enum qemu_vm_cmd {
-- 
1.8.3.1

[PATCH v1 4/8] migration: Open brace '{' following struct go on the same line

[Bihong Yu]

Signed-off-by:Bihong Yu 
Reviewed-by: Chuan Zheng 
Signed-off-by: Bihong Yu 
---
 migration/migration.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/migration/migration.h b/migration/migration.h
index deb411a..99784b4 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -124,8 +124,7 @@ struct MigrationClass {
 DeviceClass parent_class;
 };
 
-struct MigrationState
-{
+struct MigrationState {
 /*< private >*/
 DeviceState parent_obj;
 
-- 
1.8.3.1

Re: [PATCH v1 0/8] Fix some style problems in migration

[no-reply]

Patchew URL: 
https://patchew.org/QEMU/1602411429-12043-1-git-send-email-yubih...@huawei.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 1602411429-12043-1-git-send-email-yubih...@huawei.com
Subject: [PATCH v1 0/8] Fix some style problems in migration

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
   4a7c0bd..b433f2c  master -> master
 * [new tag] 
patchew/1602411429-12043-1-git-send-email-yubih...@huawei.com -> 
patchew/1602411429-12043-1-git-send-email-yubih...@huawei.com
Switched to a new branch 'test'
1a213e1 migration: Delete redundant spaces
515c82f migration: Open brace '{' following function declarations go on the 
next line
d49e458 migration: Do not initialise statics and globals to 0 or NULL
74a25de migration: Add braces {} for if statement
7dbb0ff migration: Open brace '{' following struct go on the same line
730c9a26 migration: Add spaces around operator
62c3395 migration: Don't use '#' flag of printf format
05c1af5 migration: Do not use C99 // comments

=== OUTPUT BEGIN ===
1/8 Checking commit 05c1af5dbfcc (migration: Do not use C99 // comments)
ERROR: space required after Signed-off-by:
#7: 
Signed-off-by:Bihong Yu 

total: 1 errors, 0 warnings, 16 lines checked

Patch 1/8 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

2/8 Checking commit 62c339591b16 (migration: Don't use '#' flag of printf 
format)
ERROR: space required after Signed-off-by:
#7: 
Signed-off-by:Bihong Yu 

total: 1 errors, 0 warnings, 24 lines checked

Patch 2/8 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

3/8 Checking commit 730c9a264343 (migration: Add spaces around operator)
ERROR: space required after Signed-off-by:
#7: 
Signed-off-by:Bihong Yu 

total: 1 errors, 0 warnings, 59 lines checked

Patch 3/8 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

4/8 Checking commit 7dbb0ff26f66 (migration: Open brace '{' following struct go 
on the same line)
ERROR: space required after Signed-off-by:
#7: 
Signed-off-by:Bihong Yu 

total: 1 errors, 0 warnings, 9 lines checked

Patch 4/8 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

5/8 Checking commit 74a25de0eed8 (migration: Add braces {} for if statement)
ERROR: space required after Signed-off-by:
#7: 
Signed-off-by:Bihong Yu 

total: 1 errors, 0 warnings, 18 lines checked

Patch 5/8 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

6/8 Checking commit d49e4586bb79 (migration: Do not initialise statics and 
globals to 0 or NULL)
ERROR: space required after Signed-off-by:
#7: 
Signed-off-by:Bihong Yu 

total: 1 errors, 0 warnings, 16 lines checked

Patch 6/8 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

7/8 Checking commit 515c82ff06b4 (migration: Open brace '{' following function 
declarations go on the next line)
ERROR: space required after Signed-off-by:
#7: 
Signed-off-by:Bihong Yu 

total: 1 errors, 0 warnings, 9 lines checked

Patch 7/8 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

8/8 Checking commit 1a213e145493 (migration: Delete redundant spaces)
ERROR: space required after Signed-off-by:
#7: 
Signed-off-by:Bihong Yu 

total: 1 errors, 0 warnings, 8 lines checked

Patch 8/8 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/1602411429-12043-1-git-send-email-yubih...@huawei.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com

[PATCH v1 5/8] migration: Add braces {} for if statement

[Bihong Yu]

Signed-off-by:Bihong Yu 
Reviewed-by: Chuan Zheng 
Signed-off-by: Bihong Yu 
---
 migration/ram.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index 90b277b..12e7296 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -101,14 +101,16 @@ static struct {
 
 static void XBZRLE_cache_lock(void)
 {
-if (migrate_use_xbzrle())
+if (migrate_use_xbzrle()) {
 qemu_mutex_lock(&XBZRLE.lock);
+}
 }
 
 static void XBZRLE_cache_unlock(void)
 {
-if (migrate_use_xbzrle())
+if (migrate_use_xbzrle()) {
 qemu_mutex_unlock(&XBZRLE.lock);
+}
 }
 
 /**
-- 
1.8.3.1

Re: Why guest physical addresses are not the same as the corresponding host virtual addresses in QEMU/KVM? Thanks!

[harry harry]

Hi Maxim,

Thanks much for your reply.

On Sun, Oct 11, 2020 at 3:29 AM Maxim Levitsky  wrote:
>
> On Sun, 2020-10-11 at 01:26 -0400, harry harry wrote:
> > Hi QEMU/KVM developers,
> >
> > I am sorry if my email disturbs you. I did an experiment and found the
> > guest physical addresses (GPAs) are not the same as the corresponding
> > host virtual addresses (HVAs). I am curious about why; I think they
> > should be the same. I am very appreciated if you can give some
> > comments and suggestions about 1) why GPAs and HVAs are not the same
> > in the following experiment; 2) are there any better experiments to
> > look into the reasons? Any other comments/suggestions are also very
> > welcome. Thanks!
> >
> > The experiment is like this: in a single vCPU VM, I ran a program
> > allocating and referencing lots of pages (e.g., 100*1024) and didn't
> > let the program terminate. Then, I checked the program's guest virtual
> > addresses (GVAs) and GPAs through parsing its pagemap and maps files
> > located at /proc/pid/pagemap and /proc/pid/maps, respectively. At
> > last, in the host OS, I checked the vCPU's pagemap and maps files to
> > find the program's HVAs and host physical addresses (HPAs); I actually
> > checked the new allocated physical pages in the host OS after the
> > program was executed in the guest OS.
> >
> > With the above experiment, I found GPAs of the program are different
> > from its corresponding HVAs. BTW, Intel EPT and other related Intel
> > virtualization techniques were enabled.
> >
> > Thanks,
> > Harry
> >
> The fundemental reason is that some HVAs (e.g. QEMU's virtual memory 
> addresses) are already allocated
> for qemu's own use (e.g qemu code/heap/etc) prior to the guest starting up.
>
> KVM does though use quite effiecient way of mapping HVA's to GPA. It uses an 
> array of arbitrary sized HVA areas
> (which we call memslots) and for each such area/memslot you specify the GPA 
> to map to. In theory QEMU
> could allocate the whole guest's memory in one contiguous area and map it as 
> single memslot to the guest.
> In practice there are MMIO holes, and various other reasons why there will be 
> more that 1 memslot.

It is still not clear to me why GPAs are not the same as the
corresponding HVAs in my experiment. Since two-dimensional paging
(Intel EPT) is used, GPAs should be the same as their corresponding
HVAs. Otherwise, I think EPT may not work correctly. What do you
think?

Thanks,
Harry

Re: [PATCH v1 0/8] Fix some style problems in migration

[no-reply]

Patchew URL: 
https://patchew.org/QEMU/1602413321-22252-1-git-send-email-yubih...@huawei.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 1602413321-22252-1-git-send-email-yubih...@huawei.com
Subject: [PATCH v1 0/8] Fix some style problems in migration

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag] 
patchew/1602413321-22252-1-git-send-email-yubih...@huawei.com -> 
patchew/1602413321-22252-1-git-send-email-yubih...@huawei.com
Switched to a new branch 'test'
cc410ad migration: Delete redundant spaces
0b36f1a migration: Open brace '{' following function declarations go on the 
next line
0c0be70 migration: Do not initialise statics and globals to 0 or NULL
4e3b241 migration: Add braces {} for if statement
b3c9e08 migration: Open brace '{' following struct go on the same line
d311759 migration: Add spaces around operator
0e05186 migration: Don't use '#' flag of printf format
1da292c migration: Do not use C99 // comments

=== OUTPUT BEGIN ===
1/8 Checking commit 1da292c2a6f4 (migration: Do not use C99 // comments)
ERROR: space required after Signed-off-by:
#7: 
Signed-off-by:Bihong Yu 

total: 1 errors, 0 warnings, 16 lines checked

Patch 1/8 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

2/8 Checking commit 0e051862e53e (migration: Don't use '#' flag of printf 
format)
ERROR: space required after Signed-off-by:
#7: 
Signed-off-by:Bihong Yu 

total: 1 errors, 0 warnings, 24 lines checked

Patch 2/8 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

3/8 Checking commit d311759fac6c (migration: Add spaces around operator)
ERROR: space required after Signed-off-by:
#7: 
Signed-off-by:Bihong Yu 

total: 1 errors, 0 warnings, 59 lines checked

Patch 3/8 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

4/8 Checking commit b3c9e08f76fa (migration: Open brace '{' following struct go 
on the same line)
ERROR: space required after Signed-off-by:
#7: 
Signed-off-by:Bihong Yu 

total: 1 errors, 0 warnings, 9 lines checked

Patch 4/8 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

5/8 Checking commit 4e3b2411656a (migration: Add braces {} for if statement)
ERROR: space required after Signed-off-by:
#7: 
Signed-off-by:Bihong Yu 

total: 1 errors, 0 warnings, 18 lines checked

Patch 5/8 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

6/8 Checking commit 0c0be70f265c (migration: Do not initialise statics and 
globals to 0 or NULL)
ERROR: space required after Signed-off-by:
#7: 
Signed-off-by:Bihong Yu 

total: 1 errors, 0 warnings, 16 lines checked

Patch 6/8 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

7/8 Checking commit 0b36f1af928d (migration: Open brace '{' following function 
declarations go on the next line)
ERROR: space required after Signed-off-by:
#7: 
Signed-off-by:Bihong Yu 

total: 1 errors, 0 warnings, 9 lines checked

Patch 7/8 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

8/8 Checking commit cc410adff527 (migration: Delete redundant spaces)
ERROR: space required after Signed-off-by:
#7: 
Signed-off-by:Bihong Yu 

total: 1 errors, 0 warnings, 8 lines checked

Patch 8/8 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/1602413321-22252-1-git-send-email-yubih...@huawei.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com

Re: [PATCH v1 0/8] Fix some style problems in migration

[no-reply]

Patchew URL: 
https://patchew.org/QEMU/1602413863-19513-1-git-send-email-yubih...@huawei.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 1602413863-19513-1-git-send-email-yubih...@huawei.com
Subject: [PATCH v1 0/8] Fix some style problems in migration

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 - [tag update]  
patchew/1602411429-12043-1-git-send-email-yubih...@huawei.com -> 
patchew/1602411429-12043-1-git-send-email-yubih...@huawei.com
 - [tag update]  
patchew/1602413321-22252-1-git-send-email-yubih...@huawei.com -> 
patchew/1602413321-22252-1-git-send-email-yubih...@huawei.com
 * [new tag] 
patchew/1602413863-19513-1-git-send-email-yubih...@huawei.com -> 
patchew/1602413863-19513-1-git-send-email-yubih...@huawei.com
Switched to a new branch 'test'
e540883 migration: Delete redundant spaces
ba991ab migration: Open brace '{' following function declarations go on the 
next line
33cc992 migration: Do not initialise statics and globals to 0 or NULL
ca4a625 migration: Add braces {} for if statement
4f4c641 migration: Open brace '{' following struct go on the same line
abbc199 migration: Add spaces around operator
3395e18 migration: Don't use '#' flag of printf format
528909a migration: Do not use C99 // comments

=== OUTPUT BEGIN ===
1/8 Checking commit 528909ad70b3 (migration: Do not use C99 // comments)
ERROR: space required after Signed-off-by:
#7: 
Signed-off-by:Bihong Yu 

total: 1 errors, 0 warnings, 16 lines checked

Patch 1/8 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

2/8 Checking commit 3395e181c39f (migration: Don't use '#' flag of printf 
format)
ERROR: space required after Signed-off-by:
#7: 
Signed-off-by:Bihong Yu 

total: 1 errors, 0 warnings, 24 lines checked

Patch 2/8 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

3/8 Checking commit abbc1992ac91 (migration: Add spaces around operator)
ERROR: space required after Signed-off-by:
#7: 
Signed-off-by:Bihong Yu 

total: 1 errors, 0 warnings, 59 lines checked

Patch 3/8 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

4/8 Checking commit 4f4c64176326 (migration: Open brace '{' following struct go 
on the same line)
ERROR: space required after Signed-off-by:
#7: 
Signed-off-by:Bihong Yu 

total: 1 errors, 0 warnings, 9 lines checked

Patch 4/8 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

5/8 Checking commit ca4a625ffe25 (migration: Add braces {} for if statement)
ERROR: space required after Signed-off-by:
#7: 
Signed-off-by:Bihong Yu 

total: 1 errors, 0 warnings, 18 lines checked

Patch 5/8 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

6/8 Checking commit 33cc992647bd (migration: Do not initialise statics and 
globals to 0 or NULL)
ERROR: space required after Signed-off-by:
#7: 
Signed-off-by:Bihong Yu 

total: 1 errors, 0 warnings, 16 lines checked

Patch 6/8 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

7/8 Checking commit ba991ab5ea63 (migration: Open brace '{' following function 
declarations go on the next line)
ERROR: space required after Signed-off-by:
#7: 
Signed-off-by:Bihong Yu 

total: 1 errors, 0 warnings, 9 lines checked

Patch 7/8 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

8/8 Checking commit e540883aba9c (migration: Delete redundant spaces)
ERROR: space required after Signed-off-by:
#7: 
Signed-off-by:Bihong Yu 

total: 1 errors, 0 warnings, 8 lines checked

Patch 8/8 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/1602413863-19513-1-git-send-email-yubih...@huawei.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com

Re: [PATCH v3 02/16] fuzz: Add general virtual-device fuzzer

[Alexander Bulekov]

On 201008 0903, Paolo Bonzini wrote:
> On 21/09/20 16:34, Alexander Bulekov wrote:
> >> Can you fuzz writing "FUZZ" in memory? Like:
> >> OP_WRITE(0x10, "UsingLibFUZZerString")?
> > No.. Hopefully that's not a huge problem.
> > 
> 
> Instead of always looking for a separator, can you:
> 
> 1) skip over it if you find it naturally at the end of a command (that
> is, "FUZZ" is like a comment command)
> 
> 2) actively search for it only if you stumble upon an unrecognized command?
> 

What is the end goal? Is it to be able to use the "FUZZ" bytes to fuzz
devices?
My concern is that we want to keep the "stability" added by the FUZZ
separators (ie removing a single byte shouldn't completely change the
sequence of operations).

> In that case, if you have
> 
>   AbcFUZZD0x10UsingLibFUZZerFUZZ
> 
> The first and third instances would be ignored, while the second would
> be part of the input.  On the other hand if you have
> 
>   bcFUZZD0x10UsingLibFUZZerFUZZ
> 
> "b" is an invalid command and therefore you'd skip directly to "D".

There aren't any invalid OPCodes, since we interpret the opcode modulo
the size of the OPcode table. We only have invalid/skipped commands when
there isn't enough data after the opcode to figure out what we should do.

> 
> Paolo
> 

Re: [PATCH v3 05/16] fuzz: Declare DMA Read callback function

[Alexander Bulekov]

On 201008 0939, Paolo Bonzini wrote:
> On 21/09/20 04:24, Alexander Bulekov wrote:
> > This patch declares the fuzz_dma_read_cb function and uses the
> > preprocessor and linker(weak symbols) to handle these cases:
> > 
> > When we build softmmu/all with --enable-fuzzing, there should be no
> > strong symbol defined for fuzz_dma_read_cb, and we link against a weak
> > stub function.
> > 
> > When we build softmmu/fuzz with --enable-fuzzing, we link against the
> > strong symbol in general_fuzz.c
> > 
> > When we build softmmu/all without --enable-fuzzing, fuzz_dma_read_cb is
> > an empty, inlined function. As long as we don't call any other functions
> > when building the arguments, there should be no overhead.
> 
> Can you move the weak function somewhere in tests/qtest/fuzz instead?
> Then you don't need an #ifdef because you can add it to specific_fuzz_ss.
> 
> Paolo
> 

If I understand correctly, specific_fuzz_ss is only used to build
qemu-fuzz targets. The goal here was to support building qemu-system
with --enable-fuzzing (ie CONFIG_FUZZ=y), where specific_fuzz isn't
used. If its too ugly, we could make a stub file under tests/qtest/fuzz
and add it to specific_ss when: 'CONFIG_FUZZ'.
-Alex

Re: [PULL 00/22] acceptance regressions, testing, gitdm, plugins

[Peter Maydell]

On Fri, 9 Oct 2020 at 17:31, Alex Bennée  wrote:
>
> The following changes since commit 4a7c0bd9dcb08798c6f82e55b5a3423f7ee669f1:
>
>   Merge remote-tracking branch 'remotes/dgibson/tags/ppc-for-5.2-20201009' 
> into staging (2020-10-09 15:48:04 +0100)
>
> are available in the Git repository at:
>
>   https://github.com/stsquad/qemu.git tags/pull-various-091020-1
>
> for you to fetch changes up to e5d402b28f1a325d46b7b0f08d04257f618e6d03:
>
>   tests/acceptance: disable machine_rx_gdbsim on GitLab (2020-10-09 17:27:55 
> +0100)
>
> 
> Testing, gitdm and plugin fixes:
>
>   - fix acceptance regressions in MIPS and IDE
>   - speed up cirrus msys2/mingw builds
>   - add genisoimage to more docker images
>   - slew of gitdb updates
>   - fix some windows compile issues for plugins
>   - add V=1 to cirrus output
>   - disable rxsim in gitlab CI


Applied, thanks.

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

-- PMM

Re: [PATCH v1 2/8] migration: Don't use '#' flag of printf format

[Peter Maydell]

On Sun, 11 Oct 2020 at 14:52, Bihong Yu  wrote:
> @@ -998,7 +998,7 @@ static int block_load(QEMUFile *f, void *opaque, int 
> version_id)
> (addr == 100) ? '\n' : '\r');
>  fflush(stdout);
>  } else if (!(flags & BLK_MIG_FLAG_EOS)) {
> -fprintf(stderr, "Unknown block migration flags: %#x\n", flags);
> +fprintf(stderr, "Unknown block migration flags: %0x\n", flags);

This doesn't look right. "%#x" will print a number in hex with a leading '0x'.
To get the same effect without using "#" you need "0x%x" (that is,
the format string provides the 0x characters literally).
What you've written is '%0x", which is a format string where the '0' is
a request to print with zero padding (which is ignored since there's no
field width given), so the result is the same as if you'd just said '%x',
and there is no '0x' in the output.

$ cat /tmp/zz9.c
#include 
int main(void) {
  printf("%#x\n", 42);
  printf("%0x\n", 42);
  printf("0x%x\n", 42);
  return 0;
}
$ gcc -g -Wall -o /tmp/zz9 /tmp/zz9.c
$ /tmp/zz9
0x2a
2a
0x2a

>  default:
> -error_report("Unknown combination of migration flags: %#x"
> +error_report("Unknown combination of migration flags: %0x"
>   " (postcopy mode)", flags);
>  ret = -EINVAL;
>  break;
> @@ -3576,7 +3576,7 @@ static int ram_load_precopy(QEMUFile *f)
>  if (flags & RAM_SAVE_FLAG_HOOK) {
>  ram_control_load_hook(f, RAM_CONTROL_HOOK, NULL);
>  } else {
> -error_report("Unknown combination of migration flags: %#x",
> +error_report("Unknown combination of migration flags: %0x",
>   flags);
>  ret = -EINVAL;
>  }

These two similarly should be "0x%x".

thanks
-- PMM

Re: [PATCH v3 13/15] hw/misc/bcm2835_cprman: add sane reset values to the registers

[Luc Michel]

On 18:18 Sat 10 Oct , Philippe Mathieu-Daudé wrote:
> On 10/10/20 3:57 PM, Luc Michel wrote:
> > Those reset values have been extracted from a Raspberry Pi 3 model B
> > v1.2, using the 2020-08-20 version of raspios. The dump was done using
> > the debugfs interface of the CPRMAN driver in Linux (under
> > '/sys/kernel/debug/clk'). Each exposed clock tree stage (PLLs, channels
> > and muxes) can be observed by reading the 'regdump' file (e.g.
> > 'plla/regdump').
> > 
> > Those values are set by the Raspberry Pi firmware at boot time (Linux
> > expects them to be set when it boots up).
> > 
> > Some stages are not exposed by the Linux driver (e.g. the PLL B). For
> > those, the reset values are unknown and left to 0 which implies a
> > disabled output.
> > 
> > Once booted in QEMU, the final clock tree is very similar to the one
> > visible on real hardware. The differences come from some unimplemented
> > devices for which the driver simply disable the corresponding clock.
> > 
> > Tested-by: Philippe Mathieu-Daudé 
> > Signed-off-by: Luc Michel 
> > ---
> >   include/hw/misc/bcm2835_cprman_internals.h | 269 +
> >   hw/misc/bcm2835_cprman.c   |  31 +++
> >   2 files changed, 300 insertions(+)
> > 
> > diff --git a/include/hw/misc/bcm2835_cprman_internals.h 
> > b/include/hw/misc/bcm2835_cprman_internals.h
> > index a6e799075f..339759b307 100644
> > --- a/include/hw/misc/bcm2835_cprman_internals.h
> > +++ b/include/hw/misc/bcm2835_cprman_internals.h
> > @@ -745,6 +745,275 @@ static inline void 
> > set_clock_mux_init_info(BCM2835CprmanState *s,
> >   mux->reg_div = &s->regs[CLOCK_MUX_INIT_INFO[id].cm_offset + 1];
> >   mux->int_bits = CLOCK_MUX_INIT_INFO[id].int_bits;
> >   mux->frac_bits = CLOCK_MUX_INIT_INFO[id].frac_bits;
> >   }
> > +
> > +/*
> > + * Object reset info
> > + * Those values have been dumped from a Raspberry Pi 3 Model B v1.2 using 
> > the
> > + * clk debugfs interface in Linux.
> > + */
> > +typedef struct PLLResetInfo {
> > +uint32_t cm;
> > +uint32_t a2w_ctrl;
> > +uint32_t a2w_ana[4];
> > +uint32_t a2w_frac;
> > +} PLLResetInfo;
> > +
> > +static const PLLResetInfo PLL_RESET_INFO[] = {
> > +[CPRMAN_PLLA] = {
> > +.cm = 0x008a,
> > +.a2w_ctrl = 0x0002103a,
> > +.a2w_frac = 0x00098000,
> > +.a2w_ana = { 0x, 0x00144000, 0x, 0x0100 }
> > +},
> > +
> > +[CPRMAN_PLLC] = {
> > +.cm = 0x0228,
> > +.a2w_ctrl = 0x0002103e,
> > +.a2w_frac = 0x0008,
> > +.a2w_ana = { 0x, 0x00144000, 0x, 0x0100 }
> > +},
> > +
> > +[CPRMAN_PLLD] = {
> > +.cm = 0x020a,
> > +.a2w_ctrl = 0x00021034,
> > +.a2w_frac = 0x00015556,
> > +.a2w_ana = { 0x, 0x00144000, 0x, 0x0100 }
> > +},
> > +
> > +[CPRMAN_PLLH] = {
> > +.cm = 0x,
> > +.a2w_ctrl = 0x0002102d,
> > +.a2w_frac = 0x,
> > +.a2w_ana = { 0x0090, 0x000c, 0x, 0x }
> > +},
> > +
> > +[CPRMAN_PLLB] = {
> > +/* unknown */
> > +.cm = 0x,
> > +.a2w_ctrl = 0x,
> > +.a2w_frac = 0x,
> > +.a2w_ana = { 0x, 0x, 0x, 0x }
> > +}
> > +};
> > +
> > +typedef struct PLLChannelResetInfo {
> > +/*
> > + * Even though a PLL channel has a CM register, it shares it with its
> > + * parent PLL. The parent already takes care of the reset value.
> > + */
> > +uint32_t a2w_ctrl;
> > +} PLLChannelResetInfo;
> > +
> > +static const PLLChannelResetInfo PLL_CHANNEL_RESET_INFO[] = {
> > +[CPRMAN_PLLA_CHANNEL_DSI0] = { .a2w_ctrl = 0x0100 },
> > +[CPRMAN_PLLA_CHANNEL_CORE] = { .a2w_ctrl = 0x0003 },
> > +[CPRMAN_PLLA_CHANNEL_PER] = { .a2w_ctrl = 0x }, /* unknown */
> > +[CPRMAN_PLLA_CHANNEL_CCP2] = { .a2w_ctrl = 0x0100 },
> > +
> > +[CPRMAN_PLLC_CHANNEL_CORE2] = { .a2w_ctrl = 0x0100 },
> > +[CPRMAN_PLLC_CHANNEL_CORE1] = { .a2w_ctrl = 0x0100 },
> > +[CPRMAN_PLLC_CHANNEL_PER] = { .a2w_ctrl = 0x0002 },
> > +[CPRMAN_PLLC_CHANNEL_CORE0] = { .a2w_ctrl = 0x0002 },
> > +
> > +[CPRMAN_PLLD_CHANNEL_DSI0] = { .a2w_ctrl = 0x0100 },
> > +[CPRMAN_PLLD_CHANNEL_CORE] = { .a2w_ctrl = 0x0004 },
> > +[CPRMAN_PLLD_CHANNEL_PER] = { .a2w_ctrl = 0x0004 },
> > +[CPRMAN_PLLD_CHANNEL_DSI1] = { .a2w_ctrl = 0x0100 },
> > +
> > +[CPRMAN_PLLH_CHANNEL_AUX] = { .a2w_ctrl = 0x0004 },
> > +[CPRMAN_PLLH_CHANNEL_RCAL] = { .a2w_ctrl = 0x },
> > +[CPRMAN_PLLH_CHANNEL_PIX] = { .a2w_ctrl = 0x },
> > +
> > +[CPRMAN_PLLB_CHANNEL_ARM] = { .a2w_ctrl = 0x }, /* unknown */
> > +};
> > +
> > +typedef struct ClockMuxResetInfo {
> > +uint32_t cm_ctl;
> > +uint32_t cm_div;
> > +} ClockMuxResetInfo;
> > +
> > +static const ClockMuxResetInf

Re: [PULL 00/10] migration queue

[Peter Maydell]

On Thu, 8 Oct 2020 at 20:13, Dr. David Alan Gilbert (git)
 wrote:
>
> From: "Dr. David Alan Gilbert" 
>
> The following changes since commit e64cf4d569f6461d6b9072e00d6e78d0ab8bd4a7:
>
>   Merge remote-tracking branch 'remotes/rth/tags/pull-tcg-20201008' into 
> staging (2020-10-08 17:18:46 +0100)
>
> are available in the Git repository at:
>
>   git://github.com/dagrh/qemu.git tags/pull-migration-20201008a
>
> for you to fetch changes up to ee02b58c82749adb486ef2ae7efdc8e05b093cfd:
>
>   migration/dirtyrate: present dirty rate only when querying the rate has 
> completed (2020-10-08 19:57:00 +0100)
>
> 
> Migration and virtiofs pull 2020-10-08
>
> v2 (from yesterdays)
>   Updated types in comparison to fix mingw build
>   rebased
>
> -
> Migration:
>   Dirtyrate measurement API cleanup
>   Postcopy recovery fixes
>
> Virtiofsd:
>   Missing qemu_init_exec_dir call
>   Support for setting the group on socket creation
>   Stop a gcc warning
>   Avoid tempdir in sandboxing

This seems to hang in 'make check' trying to run
tests/qtest/migration-test on s390x and ppc, ie
the big-endian hosts.

thanks
-- PMM

Re: [PULL 00/30] Block patches

[Peter Maydell]

On Fri, 9 Oct 2020 at 20:35, Stefan Hajnoczi  wrote:
>
> The following changes since commit 497d415d76b9f59fcae27f22df1ca2c3fa4df64e:
>
>   Merge remote-tracking branch 
> 'remotes/pmaydell/tags/pull-target-arm-20201008-1' into staging (2020-10-08 
> 21:41:20 +0100)
>
> are available in the Git repository at:
>
>   https://gitlab.com/stefanha/qemu.git tags/block-pull-request
>
> for you to fetch changes up to e969c7b045c90368bc3a5db3479e70b6f0ecb828:
>
>   iotests: add commit top->base cases to 274 (2020-10-09 14:32:24 +0100)
>
> 
> Pull request
>
> This pull request includes the vhost-user-blk server by Coiby Xu, the block
> coroutine code generator by Vladimir Sementsov-Ogievskiy, nvme block driver
> statistics by Philippe Mathieu-Daudé, and cleanups/fixes/additions to the
> vhost-user-blk server by me.
>

Hi; this seems to have a conflict in qemu-nbd.c with something
that landed in the latest nbd pullreq. Could you fix up and
resend, please?

thanks
-- PMM

[PATCH] hw/pci-host/grackle: Verify PIC link is properly set

[Philippe Mathieu-Daudé]

The Grackle PCI host model expects the interrupt controller
being set, but does not verify it is present. Add a check to
help developers using this model.

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/pci-host/grackle.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/hw/pci-host/grackle.c b/hw/pci-host/grackle.c
index 57c29b20afb..20361d215ca 100644
--- a/hw/pci-host/grackle.c
+++ b/hw/pci-host/grackle.c
@@ -76,6 +76,10 @@ static void grackle_realize(DeviceState *dev, Error **errp)
 GrackleState *s = GRACKLE_PCI_HOST_BRIDGE(dev);
 PCIHostState *phb = PCI_HOST_BRIDGE(dev);
 
+if (!s->pic) {
+error_setg(errp, TYPE_GRACKLE_PCI_HOST_BRIDGE ": 'pic' link not set");
+return;
+}
 phb->bus = pci_register_root_bus(dev, NULL,
  pci_grackle_set_irq,
  pci_grackle_map_irq,
-- 
2.26.2

[PATCH 02/10] hw/isa: Add the ISA_IRQ_KBD_DEFAULT definition

[Philippe Mathieu-Daudé]

The PS2 keyboard uses IRQ #1 by default. Add this
default definition to the IsaIrqNumber enum.

Avoid magic values in the code, replace them by the
newly introduced definition.

Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/isa/isa.h | 1 +
 hw/sparc64/sun4u.c   | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/hw/isa/isa.h b/include/hw/isa/isa.h
index 2a052ffa025..e139b88c992 100644
--- a/include/hw/isa/isa.h
+++ b/include/hw/isa/isa.h
@@ -9,6 +9,7 @@
 #include "qom/object.h"
 
 enum IsaIrqNumber {
+ISA_IRQ_KBD_DEFAULT =  1,
 ISA_NUM_IRQS= 16
 };
 
diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index ad5ca2472a4..d4c39490cd9 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -615,7 +615,7 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
 qdev_get_gpio_in_named(DEVICE(sabre), "pbm-irq", OBIO_LPT_IRQ));
 qdev_connect_gpio_out_named(DEVICE(ebus), "isa-irq", 6,
 qdev_get_gpio_in_named(DEVICE(sabre), "pbm-irq", OBIO_FDD_IRQ));
-qdev_connect_gpio_out_named(DEVICE(ebus), "isa-irq", 1,
+qdev_connect_gpio_out_named(DEVICE(ebus), "isa-irq", ISA_IRQ_KBD_DEFAULT,
 qdev_get_gpio_in_named(DEVICE(sabre), "pbm-irq", OBIO_KBD_IRQ));
 qdev_connect_gpio_out_named(DEVICE(ebus), "isa-irq", 12,
 qdev_get_gpio_in_named(DEVICE(sabre), "pbm-irq", OBIO_MSE_IRQ));
-- 
2.26.2

[PATCH 00/10] hw/isa: Introduce definitions for default IRQ values

[Philippe Mathieu-Daudé]

Replace various magic values by definitions to make
the code easier to read.

This probably makes sense to merge this series via
the 'PC chipset' tree, rather than qemu-trivial@.

Regards,

Phil.

Philippe Mathieu-Daudé (10):
  hw/isa: Introduce IsaIrqNumber enum
  hw/isa: Add the ISA_IRQ_KBD_DEFAULT definition
  hw/isa: Add the ISA_IRQ_SER_DEFAULT definition
  hw/isa: Add the ISA_IRQ_TPM_DEFAULT definition
  hw/isa: Add the ISA_IRQ_FDC_DEFAULT definition
  hw/isa: Add the ISA_IRQ_PAR_DEFAULT definition
  hw/isa: Add the ISA_IRQ_RTC_DEFAULT definition
  hw/isa: Add the ISA_IRQ_NET_DEFAULT definition
  hw/isa: Add the ISA_IRQ_MOU_DEFAULT definition
  hw/isa: Add the ISA_IRQ_IDE_DEFAULT definition

 include/hw/isa/isa.h | 13 -
 include/hw/rtc/mc146818rtc.h |  1 -
 hw/block/fdc.c   |  4 ++--
 hw/char/parallel.c   |  2 +-
 hw/i386/acpi-build.c |  2 +-
 hw/ide/isa.c |  2 +-
 hw/input/pckbd.c |  2 +-
 hw/ipmi/isa_ipmi_bt.c|  2 +-
 hw/ipmi/isa_ipmi_kcs.c   |  2 +-
 hw/isa/piix4.c   |  2 +-
 hw/net/ne2000-isa.c  |  2 +-
 hw/rtc/m48t59-isa.c  |  2 +-
 hw/rtc/mc146818rtc.c |  4 ++--
 hw/sparc64/sun4u.c   | 10 +-
 hw/timer/hpet.c  |  8 
 hw/tpm/tpm_tis_isa.c |  2 +-
 tests/qtest/rtc-test.c   |  8 
 17 files changed, 39 insertions(+), 29 deletions(-)

-- 
2.26.2

[PATCH 04/10] hw/isa: Add the ISA_IRQ_TPM_DEFAULT definition

[Philippe Mathieu-Daudé]

The TPM TIS device uses IRQ #5 by default. Add this
default definition to the IsaIrqNumber enum.

Avoid magic values in the code, replace them by the
newly introduced definition.

Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/isa/isa.h   | 1 +
 hw/i386/acpi-build.c   | 2 +-
 hw/ipmi/isa_ipmi_bt.c  | 2 +-
 hw/ipmi/isa_ipmi_kcs.c | 2 +-
 hw/tpm/tpm_tis_isa.c   | 2 +-
 5 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/include/hw/isa/isa.h b/include/hw/isa/isa.h
index 519296d5823..e4f2aed004f 100644
--- a/include/hw/isa/isa.h
+++ b/include/hw/isa/isa.h
@@ -11,6 +11,7 @@
 enum IsaIrqNumber {
 ISA_IRQ_KBD_DEFAULT =  1,
 ISA_IRQ_SER_DEFAULT =  4,
+ISA_IRQ_TPM_DEFAULT =  5,
 ISA_NUM_IRQS= 16
 };
 
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 45ad2f95334..2b6038ab015 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1886,7 +1886,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
 Rewrite to take IRQ from TPM device model and
 fix default IRQ value there to use some unused IRQ
  */
-/* aml_append(crs, aml_irq_no_flags(TPM_TIS_IRQ)); */
+/* aml_append(crs, aml_irq_no_flags(ISA_IRQ_TPM_DEFAULT)); */
 aml_append(dev, aml_name_decl("_CRS", crs));
 
 tpm_build_ppi_acpi(tpm, dev);
diff --git a/hw/ipmi/isa_ipmi_bt.c b/hw/ipmi/isa_ipmi_bt.c
index b7c2ad557b2..13a92bd2c44 100644
--- a/hw/ipmi/isa_ipmi_bt.c
+++ b/hw/ipmi/isa_ipmi_bt.c
@@ -137,7 +137,7 @@ static void *isa_ipmi_bt_get_backend_data(IPMIInterface *ii)
 
 static Property ipmi_isa_properties[] = {
 DEFINE_PROP_UINT32("ioport", ISAIPMIBTDevice, bt.io_base,  0xe4),
-DEFINE_PROP_INT32("irq",   ISAIPMIBTDevice, isairq,  5),
+DEFINE_PROP_INT32("irq", ISAIPMIBTDevice, isairq, ISA_IRQ_TPM_DEFAULT),
 DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/ipmi/isa_ipmi_kcs.c b/hw/ipmi/isa_ipmi_kcs.c
index 7dd6bf0040a..c956b539688 100644
--- a/hw/ipmi/isa_ipmi_kcs.c
+++ b/hw/ipmi/isa_ipmi_kcs.c
@@ -144,7 +144,7 @@ static void *isa_ipmi_kcs_get_backend_data(IPMIInterface 
*ii)
 
 static Property ipmi_isa_properties[] = {
 DEFINE_PROP_UINT32("ioport", ISAIPMIKCSDevice, kcs.io_base,  0xca2),
-DEFINE_PROP_INT32("irq",   ISAIPMIKCSDevice, isairq,  5),
+DEFINE_PROP_INT32("irq", ISAIPMIKCSDevice, isairq, ISA_IRQ_TPM_DEFAULT),
 DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/tpm/tpm_tis_isa.c b/hw/tpm/tpm_tis_isa.c
index 6fd876eebf1..5a4afda42df 100644
--- a/hw/tpm/tpm_tis_isa.c
+++ b/hw/tpm/tpm_tis_isa.c
@@ -91,7 +91,7 @@ static void tpm_tis_isa_reset(DeviceState *dev)
 }
 
 static Property tpm_tis_isa_properties[] = {
-DEFINE_PROP_UINT32("irq", TPMStateISA, state.irq_num, TPM_TIS_IRQ),
+DEFINE_PROP_UINT32("irq", TPMStateISA, state.irq_num, ISA_IRQ_TPM_DEFAULT),
 DEFINE_PROP_TPMBE("tpmdev", TPMStateISA, state.be_driver),
 DEFINE_PROP_BOOL("ppi", TPMStateISA, state.ppi_enabled, true),
 DEFINE_PROP_END_OF_LIST(),
-- 
2.26.2

[PATCH 07/10] hw/isa: Add the ISA_IRQ_RTC_DEFAULT definition

[Philippe Mathieu-Daudé]

The RTC time keep clock ses IRQ #8 by default. Add this
default definition to the IsaIrqNumber enum.

Avoid magic values in the code, replace them by the
newly introduced definition.

Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/isa/isa.h | 1 +
 include/hw/rtc/mc146818rtc.h | 1 -
 hw/isa/piix4.c   | 2 +-
 hw/rtc/m48t59-isa.c  | 2 +-
 hw/rtc/mc146818rtc.c | 4 ++--
 hw/timer/hpet.c  | 8 
 tests/qtest/rtc-test.c   | 8 
 7 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/include/hw/isa/isa.h b/include/hw/isa/isa.h
index 081fa446152..9f78ff11246 100644
--- a/include/hw/isa/isa.h
+++ b/include/hw/isa/isa.h
@@ -14,6 +14,7 @@ enum IsaIrqNumber {
 ISA_IRQ_TPM_DEFAULT =  5,
 ISA_IRQ_FDC_DEFAULT =  6,
 ISA_IRQ_PAR_DEFAULT =  7,
+ISA_IRQ_RTC_DEFAULT =  8,
 ISA_NUM_IRQS= 16
 };
 
diff --git a/include/hw/rtc/mc146818rtc.h b/include/hw/rtc/mc146818rtc.h
index 5b45b229244..1cca26399ce 100644
--- a/include/hw/rtc/mc146818rtc.h
+++ b/include/hw/rtc/mc146818rtc.h
@@ -47,7 +47,6 @@ struct RTCState {
 QLIST_ENTRY(RTCState) link;
 };
 
-#define RTC_ISA_IRQ 8
 #define RTC_ISA_BASE 0x70
 
 ISADevice *mc146818_rtc_init(ISABus *bus, int base_year,
diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index a50d97834c7..d9cceff9c84 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -185,7 +185,7 @@ static void piix4_realize(PCIDevice *dev, Error **errp)
 if (!qdev_realize(DEVICE(&s->rtc), BUS(isa_bus), errp)) {
 return;
 }
-isa_init_irq(ISA_DEVICE(&s->rtc), &s->rtc.irq, RTC_ISA_IRQ);
+isa_init_irq(ISA_DEVICE(&s->rtc), &s->rtc.irq, ISA_IRQ_RTC_DEFAULT);
 
 piix4_dev = dev;
 }
diff --git a/hw/rtc/m48t59-isa.c b/hw/rtc/m48t59-isa.c
index cae315e4885..bdde427a945 100644
--- a/hw/rtc/m48t59-isa.c
+++ b/hw/rtc/m48t59-isa.c
@@ -124,7 +124,7 @@ static void m48t59_isa_realize(DeviceState *dev, Error 
**errp)
 
 s->model = u->info.model;
 s->size = u->info.size;
-isa_init_irq(isadev, &s->IRQ, 8);
+isa_init_irq(isadev, &s->IRQ, ISA_IRQ_RTC_DEFAULT);
 m48t59_realize_common(s, errp);
 memory_region_init_io(&d->io, OBJECT(dev), &m48t59_io_ops, s, "m48t59", 4);
 if (d->io_base != 0) {
diff --git a/hw/rtc/mc146818rtc.c b/hw/rtc/mc146818rtc.c
index 7a38540cb9d..ba156b9a0bd 100644
--- a/hw/rtc/mc146818rtc.c
+++ b/hw/rtc/mc146818rtc.c
@@ -981,7 +981,7 @@ ISADevice *mc146818_rtc_init(ISABus *bus, int base_year, 
qemu_irq intercept_irq)
 if (intercept_irq) {
 qdev_connect_gpio_out(dev, 0, intercept_irq);
 } else {
-isa_connect_gpio_out(isadev, 0, RTC_ISA_IRQ);
+isa_connect_gpio_out(isadev, 0, ISA_IRQ_RTC_DEFAULT);
 }
 
 object_property_add_alias(qdev_get_machine(), "rtc-time", OBJECT(isadev),
@@ -1020,7 +1020,7 @@ static void rtc_build_aml(ISADevice *isadev, Aml *scope)
 crs = aml_resource_template();
 aml_append(crs, aml_io(AML_DECODE16, RTC_ISA_BASE, RTC_ISA_BASE,
0x01, 0x08));
-aml_append(crs, aml_irq_no_flags(RTC_ISA_IRQ));
+aml_append(crs, aml_irq_no_flags(ISA_IRQ_RTC_DEFAULT));
 
 dev = aml_device("RTC");
 aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0B00")));
diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c
index 05fd86af817..579a9faecf3 100644
--- a/hw/timer/hpet.c
+++ b/hw/timer/hpet.c
@@ -196,7 +196,7 @@ static void update_irq(struct HPETTimer *timer, int set)
  * timer0 be routed to IRQ0 in NON-APIC or IRQ2 in the I/O APIC,
  * timer1 be routed to IRQ8 in NON-APIC or IRQ8 in the I/O APIC.
  */
-route = (timer->tn == 0) ? 0 : RTC_ISA_IRQ;
+route = (timer->tn == 0) ? 0 : ISA_IRQ_RTC_DEFAULT;
 } else {
 route = timer_int_route(timer);
 }
@@ -615,11 +615,11 @@ static void hpet_ram_write(void *opaque, hwaddr addr,
 if (activating_bit(old_val, new_val, HPET_CFG_LEGACY)) {
 qemu_set_irq(s->pit_enabled, 0);
 qemu_irq_lower(s->irqs[0]);
-qemu_irq_lower(s->irqs[RTC_ISA_IRQ]);
+qemu_irq_lower(s->irqs[ISA_IRQ_RTC_DEFAULT]);
 } else if (deactivating_bit(old_val, new_val, HPET_CFG_LEGACY)) {
 qemu_irq_lower(s->irqs[0]);
 qemu_set_irq(s->pit_enabled, 1);
-qemu_set_irq(s->irqs[RTC_ISA_IRQ], s->rtc_irq_level);
+qemu_set_irq(s->irqs[ISA_IRQ_RTC_DEFAULT], s->rtc_irq_level);
 }
 break;
 case HPET_CFG + 4:
@@ -711,7 +711,7 @@ static void hpet_handle_legacy_irq(void *opaque, int n, int 
level)
 } else {
 s->rtc_irq_level = level;
 if (!hpet_in_legacy_mode(s)) {
-qemu_set_irq(s->irqs[RTC_ISA_IRQ], level);
+qemu_set_irq(s->irqs[ISA_IRQ_RTC_DEFAULT], level);
 }
 }
 }
diff --git a/tests/qtest/rtc-test.c b/tests/qtest/rtc-test.c
index c7af34f6b1b..9ae90d4925c 100644
--- a/tests/qtest/rtc-test.c
+++ b/tests/qtest/r

[PATCH 01/10] hw/isa: Introduce IsaIrqNumber enum

[Philippe Mathieu-Daudé]

We are going to list all default ISA IRQs. As all the definitions
are related, introduce the IsaIrqNumber type to enumerate them.

Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/isa/isa.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/hw/isa/isa.h b/include/hw/isa/isa.h
index ddaae89a853..2a052ffa025 100644
--- a/include/hw/isa/isa.h
+++ b/include/hw/isa/isa.h
@@ -8,7 +8,9 @@
 #include "hw/qdev-core.h"
 #include "qom/object.h"
 
-#define ISA_NUM_IRQS 16
+enum IsaIrqNumber {
+ISA_NUM_IRQS= 16
+};
 
 #define TYPE_ISA_DEVICE "isa-device"
 OBJECT_DECLARE_TYPE(ISADevice, ISADeviceClass, ISA_DEVICE)
-- 
2.26.2

[PATCH 06/10] hw/isa: Add the ISA_IRQ_PAR_DEFAULT definition

[Philippe Mathieu-Daudé]

The parallel port uses IRQ #7 by default. Add this
default definition to the IsaIrqNumber enum.

Avoid magic values in the code, replace them by the
newly introduced definition.

Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/isa/isa.h | 1 +
 hw/char/parallel.c   | 2 +-
 hw/sparc64/sun4u.c   | 2 +-
 3 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/include/hw/isa/isa.h b/include/hw/isa/isa.h
index 214a6730598..081fa446152 100644
--- a/include/hw/isa/isa.h
+++ b/include/hw/isa/isa.h
@@ -13,6 +13,7 @@ enum IsaIrqNumber {
 ISA_IRQ_SER_DEFAULT =  4,
 ISA_IRQ_TPM_DEFAULT =  5,
 ISA_IRQ_FDC_DEFAULT =  6,
+ISA_IRQ_PAR_DEFAULT =  7,
 ISA_NUM_IRQS= 16
 };
 
diff --git a/hw/char/parallel.c b/hw/char/parallel.c
index 8b418abf719..9e0d80ec0d0 100644
--- a/hw/char/parallel.c
+++ b/hw/char/parallel.c
@@ -636,7 +636,7 @@ bool parallel_mm_init(MemoryRegion *address_space,
 static Property parallel_isa_properties[] = {
 DEFINE_PROP_UINT32("index", ISAParallelState, index,   -1),
 DEFINE_PROP_UINT32("iobase", ISAParallelState, iobase,  -1),
-DEFINE_PROP_UINT32("irq",   ISAParallelState, isairq,  7),
+DEFINE_PROP_UINT32("irq",   ISAParallelState, isairq,  
ISA_IRQ_PAR_DEFAULT),
 DEFINE_PROP_CHR("chardev",  ISAParallelState, state.chr),
 DEFINE_PROP_END_OF_LIST(),
 };
diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index 458dc215e6f..c5b3e838ac2 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -611,7 +611,7 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
 pci_realize_and_unref(ebus, pci_busA, &error_fatal);
 
 /* Wire up "well-known" ISA IRQs to PBM legacy obio IRQs */
-qdev_connect_gpio_out_named(DEVICE(ebus), "isa-irq", 7,
+qdev_connect_gpio_out_named(DEVICE(ebus), "isa-irq", ISA_IRQ_PAR_DEFAULT,
 qdev_get_gpio_in_named(DEVICE(sabre), "pbm-irq", OBIO_LPT_IRQ));
 qdev_connect_gpio_out_named(DEVICE(ebus), "isa-irq", ISA_IRQ_FDC_DEFAULT,
 qdev_get_gpio_in_named(DEVICE(sabre), "pbm-irq", OBIO_FDD_IRQ));
-- 
2.26.2

[PATCH 05/10] hw/isa: Add the ISA_IRQ_FDC_DEFAULT definition

[Philippe Mathieu-Daudé]

The floppy disk controller uses IRQ #6 by default. Add this
default definition to the IsaIrqNumber enum.

Avoid magic values in the code, replace them by the
newly introduced definition.

Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/isa/isa.h | 1 +
 hw/block/fdc.c   | 4 ++--
 hw/sparc64/sun4u.c   | 2 +-
 3 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/include/hw/isa/isa.h b/include/hw/isa/isa.h
index e4f2aed004f..214a6730598 100644
--- a/include/hw/isa/isa.h
+++ b/include/hw/isa/isa.h
@@ -12,6 +12,7 @@ enum IsaIrqNumber {
 ISA_IRQ_KBD_DEFAULT =  1,
 ISA_IRQ_SER_DEFAULT =  4,
 ISA_IRQ_TPM_DEFAULT =  5,
+ISA_IRQ_FDC_DEFAULT =  6,
 ISA_NUM_IRQS= 16
 };
 
diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index 4c2c35e223a..531fc4c0b72 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -2855,7 +2855,7 @@ static void fdc_isa_build_aml(ISADevice *isadev, Aml 
*scope)
 crs = aml_resource_template();
 aml_append(crs, aml_io(AML_DECODE16, 0x03F2, 0x03F2, 0x00, 0x04));
 aml_append(crs, aml_io(AML_DECODE16, 0x03F7, 0x03F7, 0x00, 0x01));
-aml_append(crs, aml_irq_no_flags(6));
+aml_append(crs, aml_irq_no_flags(ISA_IRQ_FDC_DEFAULT));
 aml_append(crs,
 aml_dma(AML_COMPATIBILITY, AML_NOTBUSMASTER, AML_TRANSFER8, 2));
 
@@ -2889,7 +2889,7 @@ static const VMStateDescription vmstate_isa_fdc ={
 
 static Property isa_fdc_properties[] = {
 DEFINE_PROP_UINT32("iobase", FDCtrlISABus, iobase, 0x3f0),
-DEFINE_PROP_UINT32("irq", FDCtrlISABus, irq, 6),
+DEFINE_PROP_UINT32("irq", FDCtrlISABus, irq, ISA_IRQ_FDC_DEFAULT),
 DEFINE_PROP_UINT32("dma", FDCtrlISABus, dma, 2),
 DEFINE_PROP_DRIVE("driveA", FDCtrlISABus, state.qdev_for_drives[0].blk),
 DEFINE_PROP_DRIVE("driveB", FDCtrlISABus, state.qdev_for_drives[1].blk),
diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index 6e42467d5cc..458dc215e6f 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -613,7 +613,7 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
 /* Wire up "well-known" ISA IRQs to PBM legacy obio IRQs */
 qdev_connect_gpio_out_named(DEVICE(ebus), "isa-irq", 7,
 qdev_get_gpio_in_named(DEVICE(sabre), "pbm-irq", OBIO_LPT_IRQ));
-qdev_connect_gpio_out_named(DEVICE(ebus), "isa-irq", 6,
+qdev_connect_gpio_out_named(DEVICE(ebus), "isa-irq", ISA_IRQ_FDC_DEFAULT,
 qdev_get_gpio_in_named(DEVICE(sabre), "pbm-irq", OBIO_FDD_IRQ));
 qdev_connect_gpio_out_named(DEVICE(ebus), "isa-irq", ISA_IRQ_KBD_DEFAULT,
 qdev_get_gpio_in_named(DEVICE(sabre), "pbm-irq", OBIO_KBD_IRQ));
-- 
2.26.2

[PATCH 09/10] hw/isa: Add the ISA_IRQ_MOU_DEFAULT definition

[Philippe Mathieu-Daudé]

The PS2 mouse uses IRQ #12 by default. Add this
default definition to the IsaIrqNumber enum.

Avoid magic values in the code, replace them by the
newly introduced definition.

Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/isa/isa.h | 1 +
 hw/input/pckbd.c | 2 +-
 hw/sparc64/sun4u.c   | 2 +-
 3 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/include/hw/isa/isa.h b/include/hw/isa/isa.h
index 11166592246..43cdc3c47b6 100644
--- a/include/hw/isa/isa.h
+++ b/include/hw/isa/isa.h
@@ -16,6 +16,7 @@ enum IsaIrqNumber {
 ISA_IRQ_PAR_DEFAULT =  7,
 ISA_IRQ_RTC_DEFAULT =  8,
 ISA_IRQ_NET_DEFAULT =  9,
+ISA_IRQ_MOU_DEFAULT = 12,
 ISA_NUM_IRQS= 16
 };
 
diff --git a/hw/input/pckbd.c b/hw/input/pckbd.c
index dde85ba6c68..140c992b03b 100644
--- a/hw/input/pckbd.c
+++ b/hw/input/pckbd.c
@@ -577,7 +577,7 @@ static void i8042_build_aml(ISADevice *isadev, Aml *scope)
 aml_append(kbd, aml_name_decl("_CRS", crs));
 
 crs = aml_resource_template();
-aml_append(crs, aml_irq_no_flags(12));
+aml_append(crs, aml_irq_no_flags(ISA_IRQ_MOU_DEFAULT));
 
 mou = aml_device("MOU");
 aml_append(mou, aml_name_decl("_HID", aml_eisaid("PNP0F13")));
diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index c5b3e838ac2..ddd51c7cbbe 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -617,7 +617,7 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
 qdev_get_gpio_in_named(DEVICE(sabre), "pbm-irq", OBIO_FDD_IRQ));
 qdev_connect_gpio_out_named(DEVICE(ebus), "isa-irq", ISA_IRQ_KBD_DEFAULT,
 qdev_get_gpio_in_named(DEVICE(sabre), "pbm-irq", OBIO_KBD_IRQ));
-qdev_connect_gpio_out_named(DEVICE(ebus), "isa-irq", 12,
+qdev_connect_gpio_out_named(DEVICE(ebus), "isa-irq", ISA_IRQ_MOU_DEFAULT,
 qdev_get_gpio_in_named(DEVICE(sabre), "pbm-irq", OBIO_MSE_IRQ));
 qdev_connect_gpio_out_named(DEVICE(ebus), "isa-irq", ISA_IRQ_SER_DEFAULT,
 qdev_get_gpio_in_named(DEVICE(sabre), "pbm-irq", OBIO_SER_IRQ));
-- 
2.26.2

[PATCH 03/10] hw/isa: Add the ISA_IRQ_SER_DEFAULT definition

[Philippe Mathieu-Daudé]

The first serial port uses IRQ #4 by default. Add this
default definition to the IsaIrqNumber enum.

Avoid magic values in the code, replace them by the
newly introduced definition.

Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/isa/isa.h | 1 +
 hw/sparc64/sun4u.c   | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/hw/isa/isa.h b/include/hw/isa/isa.h
index e139b88c992..519296d5823 100644
--- a/include/hw/isa/isa.h
+++ b/include/hw/isa/isa.h
@@ -10,6 +10,7 @@
 
 enum IsaIrqNumber {
 ISA_IRQ_KBD_DEFAULT =  1,
+ISA_IRQ_SER_DEFAULT =  4,
 ISA_NUM_IRQS= 16
 };
 
diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index d4c39490cd9..6e42467d5cc 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -619,7 +619,7 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
 qdev_get_gpio_in_named(DEVICE(sabre), "pbm-irq", OBIO_KBD_IRQ));
 qdev_connect_gpio_out_named(DEVICE(ebus), "isa-irq", 12,
 qdev_get_gpio_in_named(DEVICE(sabre), "pbm-irq", OBIO_MSE_IRQ));
-qdev_connect_gpio_out_named(DEVICE(ebus), "isa-irq", 4,
+qdev_connect_gpio_out_named(DEVICE(ebus), "isa-irq", ISA_IRQ_SER_DEFAULT,
 qdev_get_gpio_in_named(DEVICE(sabre), "pbm-irq", OBIO_SER_IRQ));
 
 switch (vga_interface_type) {
-- 
2.26.2

[PATCH 10/10] hw/isa: Add the ISA_IRQ_IDE_DEFAULT definition

[Philippe Mathieu-Daudé]

The IDE controller uses IRQ #14 by default. Add this
default definition to the IsaIrqNumber enum.

Avoid magic values in the code, replace them by the
newly introduced definition.

Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/isa/isa.h | 1 +
 hw/ide/isa.c | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/hw/isa/isa.h b/include/hw/isa/isa.h
index 43cdc3c47b6..05622ee11e2 100644
--- a/include/hw/isa/isa.h
+++ b/include/hw/isa/isa.h
@@ -17,6 +17,7 @@ enum IsaIrqNumber {
 ISA_IRQ_RTC_DEFAULT =  8,
 ISA_IRQ_NET_DEFAULT =  9,
 ISA_IRQ_MOU_DEFAULT = 12,
+ISA_IRQ_IDE_DEFAULT = 14,
 ISA_NUM_IRQS= 16
 };
 
diff --git a/hw/ide/isa.c b/hw/ide/isa.c
index 6bc19de2265..2412d568937 100644
--- a/hw/ide/isa.c
+++ b/hw/ide/isa.c
@@ -108,7 +108,7 @@ ISADevice *isa_ide_init(ISABus *bus, int iobase, int 
iobase2, int isairq,
 static Property isa_ide_properties[] = {
 DEFINE_PROP_UINT32("iobase",  ISAIDEState, iobase,  0x1f0),
 DEFINE_PROP_UINT32("iobase2", ISAIDEState, iobase2, 0x3f6),
-DEFINE_PROP_UINT32("irq",ISAIDEState, isairq,  14),
+DEFINE_PROP_UINT32("irq", ISAIDEState, isairq, ISA_IRQ_IDE_DEFAULT),
 DEFINE_PROP_END_OF_LIST(),
 };
 
-- 
2.26.2

[PATCH 08/10] hw/isa: Add the ISA_IRQ_NET_DEFAULT definition

[Philippe Mathieu-Daudé]

The network devices use IRQ #9 by default. Add this
default definition to the IsaIrqNumber enum.

Avoid magic values in the code, replace them by the
newly introduced definition.

Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/isa/isa.h | 1 +
 hw/net/ne2000-isa.c  | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/hw/isa/isa.h b/include/hw/isa/isa.h
index 9f78ff11246..11166592246 100644
--- a/include/hw/isa/isa.h
+++ b/include/hw/isa/isa.h
@@ -15,6 +15,7 @@ enum IsaIrqNumber {
 ISA_IRQ_FDC_DEFAULT =  6,
 ISA_IRQ_PAR_DEFAULT =  7,
 ISA_IRQ_RTC_DEFAULT =  8,
+ISA_IRQ_NET_DEFAULT =  9,
 ISA_NUM_IRQS= 16
 };
 
diff --git a/hw/net/ne2000-isa.c b/hw/net/ne2000-isa.c
index dd6f6e34d3c..e31e86c14af 100644
--- a/hw/net/ne2000-isa.c
+++ b/hw/net/ne2000-isa.c
@@ -80,7 +80,7 @@ static void isa_ne2000_realizefn(DeviceState *dev, Error 
**errp)
 
 static Property ne2000_isa_properties[] = {
 DEFINE_PROP_UINT32("iobase", ISANE2000State, iobase, 0x300),
-DEFINE_PROP_UINT32("irq",   ISANE2000State, isairq, 9),
+DEFINE_PROP_UINT32("irq", ISANE2000State, isairq, ISA_IRQ_NET_DEFAULT),
 DEFINE_NIC_PROPERTIES(ISANE2000State, ne2000.c),
 DEFINE_PROP_END_OF_LIST(),
 };
-- 
2.26.2

[PATCH 1/4] hw: Replace magic value by PCI_NUM_PINS definition

[Philippe Mathieu-Daudé]

Use self-explicit PCI_NUM_PINS definition instead of magic value.

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/arm/virt.c   | 4 ++--
 hw/mips/gt64xxx_pci.c   | 2 +-
 hw/pci-host/versatile.c | 6 +++---
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index e465a988d68..ddad9621f79 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1117,11 +1117,11 @@ static void create_pcie_irq_map(const VirtMachineState 
*vms,
 int first_irq, const char *nodename)
 {
 int devfn, pin;
-uint32_t full_irq_map[4 * 4 * 10] = { 0 };
+uint32_t full_irq_map[4 * PCI_NUM_PINS * 10] = { 0 };
 uint32_t *irq_map = full_irq_map;
 
 for (devfn = 0; devfn <= 0x18; devfn += 0x8) {
-for (pin = 0; pin < 4; pin++) {
+for (pin = 0; pin < PCI_NUM_PINS; pin++) {
 int irq_type = GIC_FDT_IRQ_TYPE_SPI;
 int irq_nr = first_irq + ((pin + PCI_SLOT(devfn)) % PCI_NUM_PINS);
 int irq_level = GIC_FDT_IRQ_FLAGS_LEVEL_HI;
diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
index e091bc4ed55..ff1a35755f6 100644
--- a/hw/mips/gt64xxx_pci.c
+++ b/hw/mips/gt64xxx_pci.c
@@ -1018,7 +1018,7 @@ static void gt64120_pci_set_irq(void *opaque, int 
irq_num, int level)
 if (pic_irq < 16) {
 /* The pic level is the logical OR of all the PCI irqs mapped to it. */
 pic_level = 0;
-for (i = 0; i < 4; i++) {
+for (i = 0; i < PCI_NUM_PINS; i++) {
 if (pic_irq == piix4_dev->config[PIIX_PIRQCA + i]) {
 pic_level |= pci_irq_levels[i];
 }
diff --git a/hw/pci-host/versatile.c b/hw/pci-host/versatile.c
index 3553277f941..b4951023f4e 100644
--- a/hw/pci-host/versatile.c
+++ b/hw/pci-host/versatile.c
@@ -75,7 +75,7 @@ enum {
 struct PCIVPBState {
 PCIHostState parent_obj;
 
-qemu_irq irq[4];
+qemu_irq irq[PCI_NUM_PINS];
 MemoryRegion controlregs;
 MemoryRegion mem_config;
 MemoryRegion mem_config2;
@@ -412,7 +412,7 @@ static void pci_vpb_realize(DeviceState *dev, Error **errp)
 
 object_initialize(&s->pci_dev, sizeof(s->pci_dev), 
TYPE_VERSATILE_PCI_HOST);
 
-for (i = 0; i < 4; i++) {
+for (i = 0; i < PCI_NUM_PINS; i++) {
 sysbus_init_irq(sbd, &s->irq[i]);
 }
 
@@ -422,7 +422,7 @@ static void pci_vpb_realize(DeviceState *dev, Error **errp)
 mapfn = pci_vpb_map_irq;
 }
 
-pci_bus_irqs(&s->pci_bus, pci_vpb_set_irq, mapfn, s->irq, 4);
+pci_bus_irqs(&s->pci_bus, pci_vpb_set_irq, mapfn, s->irq, PCI_NUM_PINS);
 
 /* Our memory regions are:
  * 0 : our control registers
-- 
2.26.2

[PATCH] hw/scsi/megasas: Remove pointless parenthesis

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/scsi/megasas.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
index e24c12d7eed..d57402c9b09 100644
--- a/hw/scsi/megasas.c
+++ b/hw/scsi/megasas.c
@@ -2384,8 +2384,8 @@ static void megasas_scsi_realize(PCIDevice *dev, Error 
**errp)
 if (!s->sas_addr) {
 s->sas_addr = ((NAA_LOCALLY_ASSIGNED_ID << 24) |
IEEE_COMPANY_LOCALLY_ASSIGNED) << 36;
-s->sas_addr |= (pci_dev_bus_num(dev) << 16);
-s->sas_addr |= (PCI_SLOT(dev->devfn) << 8);
+s->sas_addr |= pci_dev_bus_num(dev) << 16;
+s->sas_addr |= PCI_SLOT(dev->devfn) << 8;
 s->sas_addr |= PCI_FUNC(dev->devfn);
 }
 if (!s->hba_serial) {
-- 
2.26.2

[PATCH 0/4] hw: Replace some magic by definitions

[Philippe Mathieu-Daudé]

A bunch of trivial cleanups, replacing magic
values by definitions to make the code easier
to review.

Expected to be merged via qemu-trivial@.

Regards,

Phil.

Philippe Mathieu-Daudé (4):
  hw: Replace magic value by PCI_NUM_PINS definition
  hw/pci-host/pam: Use ARRAY_SIZE() instead of magic value
  hw/pci-host/versatile: Add WINDOW_COUNT definition to replace magic
'3'
  tests/qtest: Replace magic value by NANOSECONDS_PER_SECOND definition

 hw/arm/virt.c   |  4 ++--
 hw/mips/gt64xxx_pci.c   |  2 +-
 hw/pci-host/pam.c   |  2 +-
 hw/pci-host/versatile.c | 34 +-
 tests/qtest/rtc-test.c  |  2 +-
 5 files changed, 22 insertions(+), 22 deletions(-)

-- 
2.26.2

[PATCH 2/4] hw/pci-host/pam: Use ARRAY_SIZE() instead of magic value

[Philippe Mathieu-Daudé]

Replace the magic '4' by ARRAY_SIZE(mem->alias) which is more explicit.

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/pci-host/pam.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/pci-host/pam.c b/hw/pci-host/pam.c
index a4962057833..4712260025a 100644
--- a/hw/pci-host/pam.c
+++ b/hw/pci-host/pam.c
@@ -51,7 +51,7 @@ void init_pam(DeviceState *dev, MemoryRegion *ram_memory,
  start, size);
 
 memory_region_transaction_begin();
-for (i = 0; i < 4; ++i) {
+for (i = 0; i < ARRAY_SIZE(mem->alias); ++i) {
 memory_region_set_enabled(&mem->alias[i], false);
 memory_region_add_subregion_overlap(system_memory, start,
 &mem->alias[i], 1);
-- 
2.26.2

[PATCH 3/4] hw/pci-host/versatile: Add WINDOW_COUNT definition to replace magic '3'

[Philippe Mathieu-Daudé]

Use self-explicit WINDOW_COUNT definition instead of a magic value.

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/pci-host/versatile.c | 28 ++--
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/hw/pci-host/versatile.c b/hw/pci-host/versatile.c
index b4951023f4e..4d9565de4b1 100644
--- a/hw/pci-host/versatile.c
+++ b/hw/pci-host/versatile.c
@@ -72,6 +72,8 @@ enum {
 PCI_VPB_IRQMAP_FORCE_OK,
 };
 
+#define WINDOW_COUNT 3
+
 struct PCIVPBState {
 PCIHostState parent_obj;
 
@@ -86,18 +88,18 @@ struct PCIVPBState {
  * The offsets into pci_mem_space are controlled by the imap registers.
  */
 MemoryRegion pci_io_window;
-MemoryRegion pci_mem_window[3];
+MemoryRegion pci_mem_window[WINDOW_COUNT];
 PCIBus pci_bus;
 PCIDevice pci_dev;
 
 /* Constant for life of device: */
 int realview;
-uint32_t mem_win_size[3];
+uint32_t mem_win_size[WINDOW_COUNT];
 uint8_t irq_mapping_prop;
 
 /* Variable state: */
-uint32_t imap[3];
-uint32_t smap[3];
+uint32_t imap[WINDOW_COUNT];
+uint32_t smap[WINDOW_COUNT];
 uint32_t selfid;
 uint32_t flags;
 uint8_t irq_mapping;
@@ -130,7 +132,7 @@ static void pci_vpb_update_all_windows(PCIVPBState *s)
 /* Update all alias windows based on the current register state */
 int i;
 
-for (i = 0; i < 3; i++) {
+for (i = 0; i < WINDOW_COUNT; i++) {
 pci_vpb_update_window(s, i);
 }
 }
@@ -148,8 +150,8 @@ static const VMStateDescription pci_vpb_vmstate = {
 .minimum_version_id = 1,
 .post_load = pci_vpb_post_load,
 .fields = (VMStateField[]) {
-VMSTATE_UINT32_ARRAY(imap, PCIVPBState, 3),
-VMSTATE_UINT32_ARRAY(smap, PCIVPBState, 3),
+VMSTATE_UINT32_ARRAY(imap, PCIVPBState, WINDOW_COUNT),
+VMSTATE_UINT32_ARRAY(smap, PCIVPBState, WINDOW_COUNT),
 VMSTATE_UINT32(selfid, PCIVPBState),
 VMSTATE_UINT32(flags, PCIVPBState),
 VMSTATE_UINT8(irq_mapping, PCIVPBState),
@@ -371,12 +373,10 @@ static void pci_vpb_reset(DeviceState *d)
 {
 PCIVPBState *s = PCI_VPB(d);
 
-s->imap[0] = 0;
-s->imap[1] = 0;
-s->imap[2] = 0;
-s->smap[0] = 0;
-s->smap[1] = 0;
-s->smap[2] = 0;
+for (int i = 0; i < WINDOW_COUNT; i++) {
+s->imap[i] = 0;
+s->smap[i] = 0;
+}
 s->selfid = 0;
 s->flags = 0;
 s->irq_mapping = s->irq_mapping_prop;
@@ -453,7 +453,7 @@ static void pci_vpb_realize(DeviceState *dev, Error **errp)
  * PCI memory space. The sizes vary from board to board; the base
  * offsets are guest controllable via the IMAP registers.
  */
-for (i = 0; i < 3; i++) {
+for (i = 0; i < WINDOW_COUNT; i++) {
 memory_region_init_alias(&s->pci_mem_window[i], OBJECT(s), 
"pci-vbp-window",
  &s->pci_mem_space, 0, s->mem_win_size[i]);
 sysbus_init_mmio(sbd, &s->pci_mem_window[i]);
-- 
2.26.2

[PATCH 4/4] tests/qtest: Replace magic value by NANOSECONDS_PER_SECOND definition

[Philippe Mathieu-Daudé]

Use self-explicit NANOSECONDS_PER_SECOND definition instead
of a magic value.

Signed-off-by: Philippe Mathieu-Daudé 
---
 tests/qtest/rtc-test.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qtest/rtc-test.c b/tests/qtest/rtc-test.c
index c7af34f6b1b..402ce2c6090 100644
--- a/tests/qtest/rtc-test.c
+++ b/tests/qtest/rtc-test.c
@@ -292,7 +292,7 @@ static void alarm_time(void)
 break;
 }
 
-clock_step(10);
+clock_step(NANOSECONDS_PER_SECOND);
 }
 
 g_assert(get_irq(RTC_ISA_IRQ));
-- 
2.26.2

[PATCH] target/sparc/int32_helper: Remove duplicated 'Tag Overflow' entry

[Philippe Mathieu-Daudé]

Commit 0b09be2b2f ("Nicer debug output for exceptions") added
twice the same "Tag Overflow" entry, remove the extra one.

Signed-off-by: Philippe Mathieu-Daudé 
---
 target/sparc/int32_helper.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/target/sparc/int32_helper.c b/target/sparc/int32_helper.c
index 9a71e1abd87..ba63c739c1e 100644
--- a/target/sparc/int32_helper.c
+++ b/target/sparc/int32_helper.c
@@ -50,7 +50,6 @@ static const char * const excp_names[0x80] = {
 [TT_EXTINT | 0xd] = "External Interrupt 13",
 [TT_EXTINT | 0xe] = "External Interrupt 14",
 [TT_EXTINT | 0xf] = "External Interrupt 15",
-[TT_TOVF] = "Tag Overflow",
 [TT_CODE_ACCESS] = "Instruction Access Error",
 [TT_DATA_ACCESS] = "Data Access Error",
 [TT_DIV_ZERO] = "Division By Zero",
-- 
2.26.2

Re: [PATCH 04/10] hw/isa: Add the ISA_IRQ_TPM_DEFAULT definition

[Stefan Berger]

On 10/11/20 3:32 PM, Philippe Mathieu-Daudé wrote:

The TPM TIS device uses IRQ #5 by default. Add this
default definition to the IsaIrqNumber enum.

Avoid magic values in the code, replace them by the
newly introduced definition.

Signed-off-by: Philippe Mathieu-Daudé 
---
  include/hw/isa/isa.h   | 1 +
  hw/i386/acpi-build.c   | 2 +-
  hw/ipmi/isa_ipmi_bt.c  | 2 +-
  hw/ipmi/isa_ipmi_kcs.c | 2 +-
  hw/tpm/tpm_tis_isa.c   | 2 +-
  5 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/include/hw/isa/isa.h b/include/hw/isa/isa.h
index 519296d5823..e4f2aed004f 100644
--- a/include/hw/isa/isa.h
+++ b/include/hw/isa/isa.h
@@ -11,6 +11,7 @@
  enum IsaIrqNumber {
  ISA_IRQ_KBD_DEFAULT =  1,
  ISA_IRQ_SER_DEFAULT =  4,
+ISA_IRQ_TPM_DEFAULT =  5,
  ISA_NUM_IRQS= 16
  };

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 45ad2f95334..2b6038ab015 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1886,7 +1886,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
  Rewrite to take IRQ from TPM device model and
  fix default IRQ value there to use some unused IRQ
   */
-/* aml_append(crs, aml_irq_no_flags(TPM_TIS_IRQ)); */
+/* aml_append(crs, aml_irq_no_flags(ISA_IRQ_TPM_DEFAULT)); */
  aml_append(dev, aml_name_decl("_CRS", crs));

  tpm_build_ppi_acpi(tpm, dev);
diff --git a/hw/ipmi/isa_ipmi_bt.c b/hw/ipmi/isa_ipmi_bt.c
index b7c2ad557b2..13a92bd2c44 100644
--- a/hw/ipmi/isa_ipmi_bt.c
+++ b/hw/ipmi/isa_ipmi_bt.c
@@ -137,7 +137,7 @@ static void *isa_ipmi_bt_get_backend_data(IPMIInterface *ii)

  static Property ipmi_isa_properties[] = {
  DEFINE_PROP_UINT32("ioport", ISAIPMIBTDevice, bt.io_base,  0xe4),
-DEFINE_PROP_INT32("irq",   ISAIPMIBTDevice, isairq,  5),
+DEFINE_PROP_INT32("irq", ISAIPMIBTDevice, isairq, ISA_IRQ_TPM_DEFAULT),
  DEFINE_PROP_END_OF_LIST(),
  };

diff --git a/hw/ipmi/isa_ipmi_kcs.c b/hw/ipmi/isa_ipmi_kcs.c
index 7dd6bf0040a..c956b539688 100644
--- a/hw/ipmi/isa_ipmi_kcs.c
+++ b/hw/ipmi/isa_ipmi_kcs.c
@@ -144,7 +144,7 @@ static void *isa_ipmi_kcs_get_backend_data(IPMIInterface 
*ii)

  static Property ipmi_isa_properties[] = {
  DEFINE_PROP_UINT32("ioport", ISAIPMIKCSDevice, kcs.io_base,  0xca2),
-DEFINE_PROP_INT32("irq",   ISAIPMIKCSDevice, isairq,  5),
+DEFINE_PROP_INT32("irq", ISAIPMIKCSDevice, isairq, ISA_IRQ_TPM_DEFAULT),
  DEFINE_PROP_END_OF_LIST(),
  };



I don't know what these devices do but this looks like an unwanted clash.



diff --git a/hw/tpm/tpm_tis_isa.c b/hw/tpm/tpm_tis_isa.c
index 6fd876eebf1..5a4afda42df 100644
--- a/hw/tpm/tpm_tis_isa.c
+++ b/hw/tpm/tpm_tis_isa.c
@@ -91,7 +91,7 @@ static void tpm_tis_isa_reset(DeviceState *dev)
  }

  static Property tpm_tis_isa_properties[] = {
-DEFINE_PROP_UINT32("irq", TPMStateISA, state.irq_num, TPM_TIS_IRQ),
+DEFINE_PROP_UINT32("irq", TPMStateISA, state.irq_num, ISA_IRQ_TPM_DEFAULT),
  DEFINE_PROP_TPMBE("tpmdev", TPMStateISA, state.be_driver),
  DEFINE_PROP_BOOL("ppi", TPMStateISA, state.ppi_enabled, true),
  DEFINE_PROP_END_OF_LIST(),

[PoCv2 01/15] mingw: fix error __USE_MINGW_ANSI_STDIO redefined

[marcandre . lureau]

From: Marc-André Lureau 

Always put osdep.h first, and remove redundant stdlib.h include.

Signed-off-by: Marc-André Lureau 
---
 migration/dirtyrate.c | 3 ++-
 tests/test-bitmap.c   | 1 -
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
index 68577ef250..47f761e67a 100644
--- a/migration/dirtyrate.c
+++ b/migration/dirtyrate.c
@@ -10,8 +10,9 @@
  * See the COPYING file in the top-level directory.
  */
 
-#include 
 #include "qemu/osdep.h"
+
+#include 
 #include "qapi/error.h"
 #include "cpu.h"
 #include "qemu/config-file.h"
diff --git a/tests/test-bitmap.c b/tests/test-bitmap.c
index 2f5b71458a..8db4f67883 100644
--- a/tests/test-bitmap.c
+++ b/tests/test-bitmap.c
@@ -8,7 +8,6 @@
  * Author: Peter Xu 
  */
 
-#include 
 #include "qemu/osdep.h"
 #include "qemu/bitmap.h"
 
-- 
2.28.0

[PoCv2 00/15] Rust binding for QAPI (qemu-ga only, for now)

[marcandre . lureau]

From: Marc-André Lureau 

Hi,

Among the QEMU developers, there is a desire to use Rust. (see previous
thread from Stefan "Why QEMU should move from C to Rust", the rust-vmm
related projects and other experiments).

Thanks to our QAPI type system and the associate code generator, it is
relatively straightforward to create Rust bindings for the generated C
types (also called sys/ffi binding) and functions. (rust-bindgen could
probably do a similar job, but it would probably bring other issues).
This provides an important internal API already.

Slightly more complicated is to expose a Rust API for those, and provide
convenient conversions C<->Rust. Taking inspiration from glib-rs
binding, I implemented a simplified version of the FromGlib/ToGlib
traits, with simpler ownership model, sufficient for QAPI needs.

The usage is relatively simple:

- from_qemu_none(ptr: *const sys::P) -> T
  Return a Rust type T for a const ffi pointer P.

- from_qemu_full(ptr: *mut sys::P) -> T
  Return a Rust type T for a ffi pointer P, taking ownership.

- T::to_qemu_none() -> Stash
  Returns a borrowed ffi pointer P (using a Stash to destroy "glue"
  storage data, if any).

- T::to_qemu_full() -> P
  Returns a ffi pointer P. (P resources are leaked/passed to C/ffi)

With those traits, it's relatively easy to implement the QMP callbacks. With
enough interest, we could eventually add new commands in Rust, and start
rewriting QGA in Rust, as it is a simple service. See qga/qmp/ for some
examples. QEMU would be the next obvious target.

My biggest pain-point right now is the handling of 'if' conditions. I tried
different approaches, but none of them are satisfying. I am planning to tackle
this next again, along with full QEMU API schema support and hopefully some t=
est
integration.

v2:
 - split the original patch in smaller patches and more digestable form
 - dropped the DBus interface experiment from this series
 - various build-sys improvements, new configure options --with-rust(-target)
 - various attempts at better meson integration, finally satisfied enough wit=
h the
   current solution, which handles getting link flags from Rust sanely.
   (more meson stuff to come to handle config-host/features mapping etc).
 - rebased, QGA QMP now uses unions, added support for it.
 - start a common crate (which re-surfaced issues with foreign types and trai=
ts,
   worked around with a NewPtr wrapper)
 - explicit errors when ifcond are presents (after various unsucessful attemp=
ts,
   I will try to tackle it in v3)
 - mingw cross compilation support
 - some attempts to add it to CI
 - actually implement {get,set}-vcpus
 - vendor the external crates

Marc-Andr=C3=A9 Lureau (15):
  mingw: fix error __USE_MINGW_ANSI_STDIO redefined
  scripts/qapi: teach c_param_type() to return const argument type
  build-sys: add --with-rust{-target} & basic build infrastructure
  build-sys: add a cargo-wrapper script
  qga/rust: build and link an empty static library
  rust: provide a common crate for QEMU
  scripts/qapi: add Rust sys bindings generation
  qga/rust: generate QGA QAPI sys bindings
  scripts/qapi: add generation of Rust bindings for types
  qga/rust: build Rust types
  qga: add qmp! macro helper
  qga: implement get-host-name in Rust
  qga: implement {get,set}-vcpus in Rust
  travis: add Rust
  rust: use vendored-sources

 .cargo/config|   5 +
 .gitmodules  |   3 +
 .travis.yml  |  18 +-
 Cargo.toml   |   5 +
 configure|  26 ++
 include/qemu/osdep.h |  10 -
 meson.build  |  29 ++-
 migration/dirtyrate.c|   3 +-
 qga/Cargo.toml   |  20 ++
 qga/commands-posix.c | 159 -
 qga/commands-win32.c |  76 --
 qga/commands.c   |  34 +--
 qga/lib.rs   |   5 +
 qga/meson.build  |  30 ++-
 qga/qapi.rs  |   6 +
 qga/qapi_sys.rs  |   5 +
 qga/qmp/hostname.rs  |   9 +
 qga/qmp/mod.rs   |  61 +
 qga/qmp/vcpus.rs | 161 +
 rust/common/Cargo.toml   |  11 +
 rust/common/src/error.rs | 109 +
 rust/common/src/lib.rs   |  10 +
 rust/common/src/qemu.rs  |  30 +++
 rust/common/src/sys.rs   |  58 +
 rust/common/src/translate.rs | 309 
 rust/vendored|   1 +
 scripts/cargo_wrapper.py | 102 
 scripts/qapi-gen.py  |  18 +-
 scripts/qapi/rs.py   | 204 
 scripts/qapi/rs_sys.py   | 254 
 scripts/qapi/rs_types.py | 447 +++
 scripts/qapi/schema.py   |  14 +-
 tests/test-bitmap.c  |   1 -
 tests/test-qga.c |   4 +
 util/oslib-posix.c   |  35 ---
 util/oslib-win32.c   |  13 -
 36 files changed, 1962 insertions(+), 323 deletions(-)
 create mode 100644 .cargo/config
 create mode 100644 Cargo.toml

[PoCv2 02/15] scripts/qapi: teach c_param_type() to return const argument type

[marcandre . lureau]

From: Marc-André Lureau 

As it should be, since the argument isn't owned by the callee,
but a lot of code in QEMU rely on non-const arguments to tweak it.

Since Rust types / bindings are derived from the C version, we have to
be more accurate there to do correct ownership conversions.

Signed-off-by: Marc-André Lureau 
---
 scripts/qapi/schema.py | 14 ++
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index d1307ec661..ca85c7273a 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -167,8 +167,14 @@ class QAPISchemaType(QAPISchemaEntity):
 pass
 
 # Return the C type to be used in a parameter list.
-def c_param_type(self):
-return self.c_type()
+#
+# The argument should be considered const, since no ownership is given to 
the callee,
+# but qemu C code frequently tweaks it. Set const=True for a stricter 
declaration.
+def c_param_type(self, const=False):
+c_type = self.c_type()
+if const and c_type.endswith(pointer_suffix):
+c_type = 'const ' + c_type
+return c_type
 
 # Return the C type to be used where we suppress boxing.
 def c_unboxed_type(self):
@@ -221,10 +227,10 @@ class QAPISchemaBuiltinType(QAPISchemaType):
 def c_type(self):
 return self._c_type_name
 
-def c_param_type(self):
+def c_param_type(self, const=False):
 if self.name == 'str':
 return 'const ' + self._c_type_name
-return self._c_type_name
+return super().c_param_type(const)
 
 def json_type(self):
 return self._json_type_name
-- 
2.28.0

[PoCv2 03/15] build-sys: add --with-rust{-target} & basic build infrastructure

[marcandre . lureau]

From: Marc-André Lureau 

Add the build-sys infrastructure to build Rust code. Introduce a
top-level workspace, so various sub-projects (libraries, executables
etc) can be developed together, sharing the dependencies and output
directory.

If not Tier 1, many of the platforms QEMU supports are considered Tier
2: https://doc.rust-lang.org/nightly/rustc/platform-support.html

Rust is generally available on various distributions (thanks to Firefox,
I suppose). If not, it can usually be installed with rustup by
developpers.

configure will enable Rust support automatically if cargo is present.
Rust support can be disabled --without-rust. When detecting windows
cross building, it will use the $cpu-pc-windows-gnu target by
default (more default mappings could be added over time). This can be
changed with --with-rust-target=RTT.

Signed-off-by: Marc-André Lureau 
---
 Cargo.toml  |  2 ++
 configure   | 18 ++
 meson.build | 23 +++
 3 files changed, 43 insertions(+)
 create mode 100644 Cargo.toml

diff --git a/Cargo.toml b/Cargo.toml
new file mode 100644
index 00..c4b464ff15
--- /dev/null
+++ b/Cargo.toml
@@ -0,0 +1,2 @@
+[workspace]
+members = []
diff --git a/configure b/configure
index b553288c5e..7945ceac63 100755
--- a/configure
+++ b/configure
@@ -446,6 +446,8 @@ meson=""
 ninja=""
 skip_meson=no
 gettext=""
+with_rust="auto"
+with_rust_target=""
 
 bogus_os="no"
 malloc_trim="auto"
@@ -1519,6 +1521,12 @@ for opt do
   ;;
   --disable-libdaxctl) libdaxctl=no
   ;;
+  --with-rust) with_rust=yes
+  ;;
+  --without-rust) with_rust=no
+  ;;
+  --with-rust-target=*) with_rust_target="$optarg"
+  ;;
   *)
   echo "ERROR: unknown option $opt"
   echo "Try '$0 --help' for more information"
@@ -1666,6 +1674,8 @@ Advanced options (experts only):
   --extra-ldflags=LDFLAGS  append extra linker flags LDFLAGS
   --cross-cc-ARCH=CC   use compiler when building ARCH guest test cases
   --cross-cc-flags-ARCH=   use compiler flags when building ARCH guest tests
+  --with-rust  enable Rust compilation
+  --with-rust-target=RTT   use the given Rust target triple
   --make=MAKE  use specified make [$make]
   --python=PYTHON  use specified python [$python]
   --sphinx-build=SPHINXuse specified sphinx-build [$sphinx_build]
@@ -1918,6 +1928,10 @@ if test -z "$ninja"; then
 done
 fi
 
+if test "$with_rust" = auto && has cargo; then
+with_rust=yes
+fi
+
 # Check that the C compiler works. Doing this here before testing
 # the host CPU ensures that we had a valid CC to autodetect the
 # $cpu var (and we should bail right here if that's not the case).
@@ -7046,6 +7060,10 @@ fi
 if test "$safe_stack" = "yes"; then
   echo "CONFIG_SAFESTACK=y" >> $config_host_mak
 fi
+if test "$with_rust" = "yes" ; then
+  echo "CONFIG_WITH_RUST=y" >> $config_host_mak
+  echo "CONFIG_WITH_RUST_TARGET=$with_rust_target" >> $config_host_mak
+fi
 
 # If we're using a separate build tree, set it up now.
 # DIRS are directories which we simply mkdir in the build tree;
diff --git a/meson.build b/meson.build
index 17c89c87c6..d8526dc999 100644
--- a/meson.build
+++ b/meson.build
@@ -73,6 +73,28 @@ if cpu in ['x86', 'x86_64']
   }
 endif
 
+with_rust = 'CONFIG_WITH_RUST' in config_host
+cargo = find_program('cargo', required: with_rust)
+
+if with_rust
+  rs_target_triple = config_host['CONFIG_WITH_RUST_TARGET']
+  if meson.is_cross_build()
+# more default target mappings may be added over time
+if rs_target_triple == '' and targetos == 'windows'
+  rs_target_triple = host_machine.cpu() + '-pc-windows-gnu'
+endif
+if rs_target_triple == ''
+  error('cross-compiling, but no Rust target-triple defined.')
+endif
+  endif
+endif
+
+if get_option('optimization') in ['0', '1', 'g']
+  rs_build_type = 'debug'
+else
+  rs_build_type = 'release'
+endif
+
 ##
 # Compiler flags #
 ##
@@ -1770,6 +1792,7 @@ endif
 if targetos == 'darwin'
   summary_info += {'Objective-C compiler': 
meson.get_compiler('objc').cmd_array()[0]}
 endif
+summary_info += {'Rust support':  with_rust}
 summary_info += {'ARFLAGS':   config_host['ARFLAGS']}
 summary_info += {'CFLAGS':' '.join(get_option('c_args')
+ ['-O' + 
get_option('optimization')]
-- 
2.28.0

[PoCv2 08/15] qga/rust: generate QGA QAPI sys bindings

[marcandre . lureau]

From: Marc-André Lureau 

Use qapi-gen to generate low-level C sys bindings for QAPI types,
include them to the build.

Signed-off-by: Marc-André Lureau 
---
 qga/Cargo.toml  |  4 
 qga/lib.rs  |  1 +
 qga/meson.build | 11 +++
 qga/qapi_sys.rs |  5 +
 4 files changed, 21 insertions(+)
 create mode 100644 qga/qapi_sys.rs

diff --git a/qga/Cargo.toml b/qga/Cargo.toml
index 50c3415ab2..9966057594 100644
--- a/qga/Cargo.toml
+++ b/qga/Cargo.toml
@@ -4,6 +4,10 @@ version = "0.1.0"
 edition = "2018"
 license = "GPLv2"
 
+[dependencies]
+common = { path = "../rust/common" }
+libc = "^0.2.76"
+
 [lib]
 path = "lib.rs"
 crate-type = ["staticlib"]
diff --git a/qga/lib.rs b/qga/lib.rs
index e69de29bb2..050a47e2a3 100644
--- a/qga/lib.rs
+++ b/qga/lib.rs
@@ -0,0 +1 @@
+mod qapi_sys;
diff --git a/qga/meson.build b/qga/meson.build
index 62e13a11b3..dbc8f1623b 100644
--- a/qga/meson.build
+++ b/qga/meson.build
@@ -47,10 +47,21 @@ qga_ss = qga_ss.apply(config_host, strict: false)
 
 qga_rs = declare_dependency()
 if with_rust
+  qga_qapi_rs_outputs = [
+'qga-qapi-sys-types.rs',
+  ]
+
+  qapi_gen_rs_files = custom_target('QGA QAPI Rust bindings',
+output: qga_qapi_rs_outputs,
+input: 'qapi-schema.json',
+command: [ qapi_gen, '-r', '-o', 'qga', 
'-p', 'qga-', '@INPUT0@' ],
+depend_files: qapi_gen_depends)
+
   cargo_qga = custom_target('cargo-qga',
 build_by_default: true,
 output: ['libqga.args', 'libqga.a'],
 build_always_stale: true,
+depends: [qapi_gen_rs_files],
 command: [cargo_wrapper,
   'build-lib',
   meson.current_build_dir(),
diff --git a/qga/qapi_sys.rs b/qga/qapi_sys.rs
new file mode 100644
index 00..06fc49b826
--- /dev/null
+++ b/qga/qapi_sys.rs
@@ -0,0 +1,5 @@
+#![allow(dead_code)]
+include!(concat!(
+env!("MESON_BUILD_ROOT"),
+"/qga/qga-qapi-sys-types.rs"
+));
-- 
2.28.0

[PoCv2 04/15] build-sys: add a cargo-wrapper script

[marcandre . lureau]

From: Marc-André Lureau 

Introduce a script to help calling cargo from Rust.

Cargo is the most convenient way to build Rust code, with various
crates, and has many features that meson lacks in general for Rust.
Trying to convert projects to meson automatically is an option I
considered (see for ex https://github.com/badboy/bygge for ninja
conversion), but the complexity of the task and the need of build.rs
mechanism in various crates makes this endeavour out of scope at this
point.

This script will help to invoke cargo in different ways. For now, it
supports building static libraries. It sets up a common environment, run
the compiler and grep its output for the linker flags. Finally it copies
the built library to the expected meson output directory, and create a
file with the linker arguments.

Signed-off-by: Marc-André Lureau 
---
 meson.build  |   1 +
 scripts/cargo_wrapper.py | 101 +++
 2 files changed, 102 insertions(+)
 create mode 100644 scripts/cargo_wrapper.py

diff --git a/meson.build b/meson.build
index d8526dc999..c30bb290c5 100644
--- a/meson.build
+++ b/meson.build
@@ -75,6 +75,7 @@ endif
 
 with_rust = 'CONFIG_WITH_RUST' in config_host
 cargo = find_program('cargo', required: with_rust)
+cargo_wrapper = find_program('scripts/cargo_wrapper.py')
 
 if with_rust
   rs_target_triple = config_host['CONFIG_WITH_RUST_TARGET']
diff --git a/scripts/cargo_wrapper.py b/scripts/cargo_wrapper.py
new file mode 100644
index 00..164fad5123
--- /dev/null
+++ b/scripts/cargo_wrapper.py
@@ -0,0 +1,101 @@
+#!/usr/bin/env python3
+# Copyright (c) 2020 Red Hat, Inc.
+#
+# Author:
+#  Marc-André Lureau 
+#
+# This work is licensed under the terms of the GNU GPL, version 2 or
+# later.  See the COPYING file in the top-level directory.
+
+import argparse
+import configparser
+import distutils.file_util
+import glob
+import os
+import os.path
+import re
+import subprocess
+import sys
+from typing import List
+
+
+def get_cargo_target_dir(args: argparse.Namespace) -> str:
+# avoid conflict with qemu "target" directory
+return os.path.join(args.build_dir, "rs-target")
+
+
+def get_manifest_path(args: argparse.Namespace) -> str:
+return os.path.join(args.src_dir, "Cargo.toml")
+
+
+def build_lib(args: argparse.Namespace) -> None:
+target_dir = get_cargo_target_dir(args)
+manifest_path = get_manifest_path(args)
+# let's pretend it's an INI file to avoid extra dependency
+config = configparser.ConfigParser()
+config.read(manifest_path)
+package_name = config["package"]["name"].strip('"')
+liba = os.path.join(
+target_dir, args.target_triple, args.build_type, "lib" + package_name 
+ ".a"
+)
+libargs = os.path.join(args.build_dir, "lib" + package_name + ".args")
+
+env = {}
+env["MESON_CURRENT_BUILD_DIR"] = args.build_dir
+env["MESON_BUILD_ROOT"] = args.build_root
+env["WINAPI_NO_BUNDLED_LIBRARIES"] = "1"
+cargo_cmd = [
+"cargo",
+"rustc",
+"--target-dir",
+target_dir,
+"--manifest-path",
+manifest_path,
+]
+if args.target_triple:
+cargo_cmd += ["--target", args.target_triple]
+if args.build_type == "release":
+cargo_cmd += ["--release"]
+cargo_cmd += ["--", "--print", "native-static-libs"]
+cargo_cmd += args.EXTRA
+try:
+out = subprocess.check_output(
+cargo_cmd,
+env=dict(os.environ, **env),
+stderr=subprocess.STDOUT,
+universal_newlines=True,
+)
+native_static_libs = re.search(r"native-static-libs:(.*)", out)
+link_args = native_static_libs.group(1)
+with open(libargs, "w") as f:
+print(link_args, file=f)
+
+distutils.file_util.copy_file(liba, args.build_dir, update=True)
+except subprocess.CalledProcessError as e:
+print(
+"Environment: " + " ".join(["{}={}".format(k, v) for k, v in 
env.items()])
+)
+print("Command: " + " ".join(cargo_cmd))
+print(e.output)
+sys.exit(1)
+
+
+def main() -> None:
+parser = argparse.ArgumentParser()
+parser.add_argument("command")
+parser.add_argument("build_dir")
+parser.add_argument("src_dir")
+parser.add_argument("build_root")
+parser.add_argument("build_type")
+parser.add_argument("target_triple")
+parser.add_argument("EXTRA", nargs="*")
+args = parser.parse_args()
+
+if args.command == "build-lib":
+build_lib(args)
+else:
+raise argparse.ArgumentTypeError("Unknown command: %s" % args.command)
+
+
+if __name__ == "__main__":
+main()
-- 
2.28.0

[PoCv2 05/15] qga/rust: build and link an empty static library

[marcandre . lureau]

From: Marc-André Lureau 

Meson doesn't integrate very smoothly with Cargo. Use the cargo-wrapper
script as a custom_target() always stale to build the Rust code. The
"build-lib" command will produce a static library in the meson expected
output directory, as well as link flags that must be employed to do the
final link.

Those link flags can't be queried during configure time (Cargo doesn't
have a user-queriable configure step), so we pass them to the linker
thanks to @file argument support at build time.

Signed-off-by: Marc-André Lureau 
---
 Cargo.toml  |  2 +-
 qga/Cargo.toml  |  9 +
 qga/lib.rs  |  0
 qga/meson.build | 18 +-
 4 files changed, 27 insertions(+), 2 deletions(-)
 create mode 100644 qga/Cargo.toml
 create mode 100644 qga/lib.rs

diff --git a/Cargo.toml b/Cargo.toml
index c4b464ff15..e69b04200f 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -1,2 +1,2 @@
 [workspace]
-members = []
+members = ["qga"]
diff --git a/qga/Cargo.toml b/qga/Cargo.toml
new file mode 100644
index 00..50c3415ab2
--- /dev/null
+++ b/qga/Cargo.toml
@@ -0,0 +1,9 @@
+[package]
+name = "qga"
+version = "0.1.0"
+edition = "2018"
+license = "GPLv2"
+
+[lib]
+path = "lib.rs"
+crate-type = ["staticlib"]
diff --git a/qga/lib.rs b/qga/lib.rs
new file mode 100644
index 00..e69de29bb2
diff --git a/qga/meson.build b/qga/meson.build
index cd08bd953a..62e13a11b3 100644
--- a/qga/meson.build
+++ b/qga/meson.build
@@ -45,9 +45,25 @@ qga_ss.add(when: 'CONFIG_WIN32', if_true: files(
 
 qga_ss = qga_ss.apply(config_host, strict: false)
 
+qga_rs = declare_dependency()
+if with_rust
+  cargo_qga = custom_target('cargo-qga',
+build_by_default: true,
+output: ['libqga.args', 'libqga.a'],
+build_always_stale: true,
+command: [cargo_wrapper,
+  'build-lib',
+  meson.current_build_dir(),
+  meson.current_source_dir(),
+  meson.build_root(),
+  rs_build_type,
+  rs_target_triple])
+  qga_rs = declare_dependency(link_args: '@' + cargo_qga.full_path(), sources: 
cargo_qga)
+endif
+
 qga = executable('qemu-ga', qga_ss.sources(),
  link_args: config_host['LIBS_QGA'].split(),
- dependencies: [qemuutil, libudev],
+ dependencies: [qemuutil, libudev, qga_rs],
  install: true)
 all_qga = [qga]
 
-- 
2.28.0

[PoCv2 14/15] travis: add Rust

[marcandre . lureau]

From: Marc-André Lureau 

Signed-off-by: Marc-André Lureau 
---
 .travis.yml | 18 +-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/.travis.yml b/.travis.yml
index 1054ec5d29..b2835316bc 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -23,6 +23,8 @@ addons:
   apt:
 packages:
   # Build dependencies
+  - cargo
+  - rustc
   - libaio-dev
   - libattr1-dev
   - libbrlapi-dev
@@ -71,7 +73,7 @@ env:
   global:
 - SRC_DIR=".."
 - BUILD_DIR="build"
-- BASE_CONFIG="--disable-docs --disable-tools"
+- BASE_CONFIG="--disable-docs --disable-tools --with-rust"
 - TEST_BUILD_CMD=""
 - TEST_CMD="make check V=1"
 # This is broadly a list of "mainline" softmmu targets which have support 
across the major distros
@@ -258,6 +260,8 @@ jobs:
 # Extra toolchains
 - gcc-9
 - g++-9
+- cargo
+- rustc
 # Build dependencies
 - libaio-dev
 - libattr1-dev
@@ -325,6 +329,8 @@ jobs:
   dist: focal
   addons:
 apt_packages:
+  - cargo
+  - rustc
   - libaio-dev
   - libattr1-dev
   - libbrlapi-dev
@@ -358,6 +364,8 @@ jobs:
   dist: focal
   addons:
 apt_packages:
+  - cargo
+  - rustc
   - libaio-dev
   - libattr1-dev
   - libbrlapi-dev
@@ -390,6 +398,8 @@ jobs:
   dist: bionic
   addons:
 apt_packages:
+  - cargo
+  - rustc
   - libaio-dev
   - libattr1-dev
   - libbrlapi-dev
@@ -432,6 +442,8 @@ jobs:
   dist: bionic
   addons:
 apt_packages:
+  - cargo
+  - rustc
   - libaio-dev
   - libattr1-dev
   - libcap-ng-dev
@@ -461,6 +473,8 @@ jobs:
   dist: bionic
   addons:
 apt_packages:
+  - cargo
+  - rustc
   - libgcrypt20-dev
   - libgnutls28-dev
   env:
@@ -472,6 +486,8 @@ jobs:
   compiler: clang
   addons:
 apt_packages:
+  - cargo
+  - rustc
   - libaio-dev
   - libattr1-dev
   - libbrlapi-dev
-- 
2.28.0

[PoCv2 10/15] qga/rust: build Rust types

[marcandre . lureau]

From: Marc-André Lureau 

Signed-off-by: Marc-André Lureau 
---
 qga/lib.rs  | 1 +
 qga/meson.build | 1 +
 qga/qapi.rs | 6 ++
 3 files changed, 8 insertions(+)
 create mode 100644 qga/qapi.rs

diff --git a/qga/lib.rs b/qga/lib.rs
index 050a47e2a3..bff4107569 100644
--- a/qga/lib.rs
+++ b/qga/lib.rs
@@ -1 +1,2 @@
+mod qapi;
 mod qapi_sys;
diff --git a/qga/meson.build b/qga/meson.build
index dbc8f1623b..aedbd07a04 100644
--- a/qga/meson.build
+++ b/qga/meson.build
@@ -49,6 +49,7 @@ qga_rs = declare_dependency()
 if with_rust
   qga_qapi_rs_outputs = [
 'qga-qapi-sys-types.rs',
+'qga-qapi-types.rs',
   ]
 
   qapi_gen_rs_files = custom_target('QGA QAPI Rust bindings',
diff --git a/qga/qapi.rs b/qga/qapi.rs
new file mode 100644
index 00..e4b9113300
--- /dev/null
+++ b/qga/qapi.rs
@@ -0,0 +1,6 @@
+#![allow(dead_code)]
+use common::*;
+
+new_ptr!();
+
+include!(concat!(env!("MESON_BUILD_ROOT"), "/qga/qga-qapi-types.rs"));
-- 
2.28.0

[PoCv2 06/15] rust: provide a common crate for QEMU

[marcandre . lureau]

From: Marc-André Lureau 

This crates provides common bindings and facilities for QEMU C API
shared by various projects.

Most importantly, it defines the conversion traits used to convert from
C to Rust types. Those traits are largely adapted from glib-rs, since
those have prooven to be very flexible, and should guide us to bind
further QEMU types such as QOM. If glib-rs becomes a dependency, we
should consider adopting glib translate traits. For QAPI, we need a
smaller subset.

Signed-off-by: Marc-André Lureau 
---
 Cargo.toml   |   5 +-
 rust/common/Cargo.toml   |  11 ++
 rust/common/src/error.rs | 109 
 rust/common/src/lib.rs   |  10 ++
 rust/common/src/qemu.rs  |  30 
 rust/common/src/sys.rs   |  58 +++
 rust/common/src/translate.rs | 309 +++
 7 files changed, 531 insertions(+), 1 deletion(-)
 create mode 100644 rust/common/Cargo.toml
 create mode 100644 rust/common/src/error.rs
 create mode 100644 rust/common/src/lib.rs
 create mode 100644 rust/common/src/qemu.rs
 create mode 100644 rust/common/src/sys.rs
 create mode 100644 rust/common/src/translate.rs

diff --git a/Cargo.toml b/Cargo.toml
index e69b04200f..26bd083f79 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -1,2 +1,5 @@
 [workspace]
-members = ["qga"]
+members = [
+  "qga",
+  "rust/common",
+]
diff --git a/rust/common/Cargo.toml b/rust/common/Cargo.toml
new file mode 100644
index 00..f0584dcc83
--- /dev/null
+++ b/rust/common/Cargo.toml
@@ -0,0 +1,11 @@
+[package]
+name = "common"
+version = "0.1.0"
+edition = "2018"
+license = "GPLv2"
+
+[dependencies]
+libc = "^0.2.76"
+
+[target."cfg(unix)".dependencies]
+nix = "^0.18.0"
diff --git a/rust/common/src/error.rs b/rust/common/src/error.rs
new file mode 100644
index 00..b89f788833
--- /dev/null
+++ b/rust/common/src/error.rs
@@ -0,0 +1,109 @@
+use std::{self, ffi, fmt, io, ptr};
+
+use crate::translate::*;
+use crate::{qemu, sys};
+
+/// Common error type for QEMU and related projects.
+#[derive(Debug)]
+pub enum Error {
+FailedAt(String, &'static str, u32),
+Io(io::Error),
+#[cfg(unix)]
+Nix(nix::Error),
+}
+
+/// Alias for a `Result` with the error type for QEMU.
+pub type Result = std::result::Result;
+
+impl Error {
+fn message(&self) -> String {
+use Error::*;
+match self {
+FailedAt(msg, _, _) => msg.into(),
+Io(io) => format!("IO error: {}", io),
+#[cfg(unix)]
+Nix(nix) => format!("Nix error: {}", nix),
+}
+}
+
+fn location(&self) -> Option<(&'static str, u32)> {
+use Error::*;
+match self {
+FailedAt(_, file, line) => Some((file, *line)),
+Io(_) => None,
+#[cfg(unix)]
+Nix(_) => None,
+}
+}
+}
+
+impl fmt::Display for Error {
+fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+use Error::*;
+match self {
+FailedAt(msg, file, line) => write!(f, "{} ({}:{})", msg, file, 
line),
+_ => write!(f, "{}", self.message()),
+}
+}
+}
+
+impl From for Error {
+fn from(val: io::Error) -> Self {
+Error::Io(val)
+}
+}
+
+#[cfg(unix)]
+impl From for Error {
+fn from(val: nix::Error) -> Self {
+Error::Nix(val)
+}
+}
+
+impl QemuPtrDefault for Error {
+type QemuType = *mut sys::Error;
+}
+
+impl<'a> ToQemuPtr<'a, *mut sys::Error> for Error {
+type Storage = qemu::CError;
+
+fn to_qemu_none(&'a self) -> Stash<'a, *mut sys::Error, Self> {
+let err = self.to_qemu_full();
+
+Stash(err, unsafe { from_qemu_full(err) })
+}
+
+fn to_qemu_full(&self) -> *mut sys::Error {
+let cmsg =
+ffi::CString::new(self.message()).expect("ToQemuPtr: 
unexpected '\0' character");
+let mut csrc = ffi::CString::new("").unwrap();
+let (src, line) = self.location().map_or((ptr::null(), 0 as i32), 
|loc| {
+csrc = ffi::CString::new(loc.0).expect("ToQemuPtr:: 
unexpected '\0' character");
+(csrc.as_ptr() as *const libc::c_char, loc.1 as i32)
+});
+let func = ptr::null();
+
+let mut err: *mut sys::Error = ptr::null_mut();
+unsafe {
+sys::error_setg_internal(
+&mut err as *mut *mut _,
+src,
+line,
+func,
+cmsg.as_ptr() as *const libc::c_char,
+);
+err
+}
+}
+}
+
+/// Convenience macro to build a `Error::FailedAt` error.
+///
+/// (this error type can be nicely converted to a QEMU `sys::Error`)
+#[allow(unused_macros)]
+#[macro_export]
+macro_rules! err {
+($err:expr) => {
+Err(Error::FailedAt($err.into(), file!(), line!()))
+};
+}
diff --git a/rust/common/src/lib.rs b/rust/common/src/lib.rs
new file mode 100644
index 00..2632a2b92b
--- /dev/null
+++ b/rust/common/src/lib.rs
@@ -0,0 +1,10 @@
+mod e

[PoCv2 09/15] scripts/qapi: add generation of Rust bindings for types

[marcandre . lureau]

From: Marc-André Lureau 

Generate high-level idiomatic Rust code for the QAPI types, with to/from
translations for C FFI.

- no conditional support yet

- C FFI types are mapped to higher-level types (char*->String,
  Foo*->Foo, has_foo/foo->Option etc)

- C enums are simply aliased

- C structures have corresponding Rust structures

- alternate are represented as Rust enum (TODO: to/from conversion)

- variants A are represented as Rust AEnum enums. A AEnum::kind() method
  allows to easily get the variant C kind enum value. The translation code
  also uses a helper enum ACEnum to hold the C pointer variant. There
  might be better ways to achieve the translation, but that one works
  for now.

- unions are represented in a similar way as C, as a struct S with a "u"
  member (since S may have extra 'base' fields). However, the discriminant
  isn't a member of S, as Rust enum variant don't need that.

- lists are represented as Vec and translated thanks to vec_to/from
  translate macros

Signed-off-by: Marc-André Lureau 
---
 meson.build  |   1 +
 scripts/qapi-gen.py  |   2 +
 scripts/qapi/rs.py   |  78 +++
 scripts/qapi/rs_types.py | 447 +++
 4 files changed, 528 insertions(+)
 create mode 100644 scripts/qapi/rs_types.py

diff --git a/meson.build b/meson.build
index b6b8330b97..666e38033e 100644
--- a/meson.build
+++ b/meson.build
@@ -1149,6 +1149,7 @@ qapi_gen_depends = [ meson.source_root() / 
'scripts/qapi/__init__.py',
  meson.source_root() / 'scripts/qapi/common.py',
  meson.source_root() / 'scripts/qapi/rs.py',
  meson.source_root() / 'scripts/qapi/rs_sys.py',
+ meson.source_root() / 'scripts/qapi/rs_types.py',
  meson.source_root() / 'scripts/qapi-gen.py',
 ]
 
diff --git a/scripts/qapi-gen.py b/scripts/qapi-gen.py
index 5bfe9c8cd1..556bfe9e7b 100644
--- a/scripts/qapi-gen.py
+++ b/scripts/qapi-gen.py
@@ -17,6 +17,7 @@ from qapi.types import gen_types
 from qapi.visit import gen_visit
 
 from qapi.rs_sys import gen_rs_systypes
+from qapi.rs_types import gen_rs_types
 
 def main(argv):
 parser = argparse.ArgumentParser(
@@ -50,6 +51,7 @@ def main(argv):
 
 if args.rust:
 gen_rs_systypes(schema, args.output_dir, args.prefix, args.builtins)
+gen_rs_types(schema, args.output_dir, args.prefix, args.builtins)
 else:
 gen_types(schema, args.output_dir, args.prefix, args.builtins)
 gen_visit(schema, args.output_dir, args.prefix, args.builtins)
diff --git a/scripts/qapi/rs.py b/scripts/qapi/rs.py
index daa946580b..cab5bb9b72 100644
--- a/scripts/qapi/rs.py
+++ b/scripts/qapi/rs.py
@@ -11,6 +11,8 @@ from qapi.common import *
 from qapi.gen import QAPIGen, QAPISchemaVisitor
 
 
+rs_lists = set()
+
 rs_name_trans = str.maketrans('.-', '__')
 
 # Map @name to a valid Rust identifier.
@@ -42,12 +44,55 @@ def rs_name(name, protect=True):
 return name
 
 
+def rs_type(c_type, ns='qapi::', optional=False):
+vec = False
+to_rs = {
+'char': 'i8',
+'int8_t': 'i8',
+'uint8_t': 'u8',
+'int16_t': 'i16',
+'uint16_t': 'u16',
+'int32_t': 'i32',
+'uint32_t': 'u32',
+'int64_t': 'i64',
+'uint64_t': 'u64',
+'double': 'f64',
+'bool': 'bool',
+'str': 'String',
+}
+if c_type.startswith('const '):
+c_type = c_type[6:]
+if c_type.endswith(pointer_suffix):
+c_type = c_type.rstrip(pointer_suffix).strip()
+if c_type.endswith('List'):
+c_type = c_type[:-4]
+vec = True
+else:
+to_rs = {
+'char': 'String',
+}
+
+if c_type in to_rs:
+ret = to_rs[c_type]
+else:
+ret = ns + c_type
+
+if vec:
+ret = 'Vec<%s>' % ret
+if optional:
+return 'Option<%s>' % ret
+else:
+return ret
+
+
 def rs_ctype_parse(c_type):
 is_pointer = False
 if c_type.endswith(pointer_suffix):
 is_pointer = True
 c_type = c_type.rstrip(pointer_suffix).strip()
 is_list = c_type.endswith('List')
+if is_list:
+rs_lists.add(c_type)
 is_const = False
 if c_type.startswith('const '):
 is_const = True
@@ -103,6 +148,39 @@ def to_camel_case(value):
 return value
 
 
+def to_qemu_none(c_type, name):
+(is_pointer, _, is_list, _) = rs_ctype_parse(c_type)
+
+if is_pointer:
+if c_type == 'char':
+return mcgen('''
+let %(name)s_ = CString::new(%(name)s).unwrap();
+let %(name)s = %(name)s_.as_ptr();
+''', name=name)
+elif is_list:
+return mcgen('''
+let %(name)s_ = NewPtr(%(name)s).to_qemu_none();
+let %(name)s = %(name)s_.0.0;
+''', name=name)
+else:
+return mcgen('''
+let %(name)s_ = %(name)s.to_qemu_none();
+let %(name)s = %(name)s_.0;
+''', name=name)
+ret

[PoCv2 11/15] qga: add qmp! macro helper

[marcandre . lureau]

From: Marc-André Lureau 

Add a macro to help wrapping higher-level qmp handlers, by taking care
of errors and return value pointer translation.

Signed-off-by: Marc-André Lureau 
---
 qga/lib.rs |  1 +
 qga/qmp/mod.rs | 36 
 2 files changed, 37 insertions(+)
 create mode 100644 qga/qmp/mod.rs

diff --git a/qga/lib.rs b/qga/lib.rs
index bff4107569..5fe08c25a3 100644
--- a/qga/lib.rs
+++ b/qga/lib.rs
@@ -1,2 +1,3 @@
 mod qapi;
 mod qapi_sys;
+mod qmp;
diff --git a/qga/qmp/mod.rs b/qga/qmp/mod.rs
new file mode 100644
index 00..38060100af
--- /dev/null
+++ b/qga/qmp/mod.rs
@@ -0,0 +1,36 @@
+use common::*;
+
+use crate::*;
+
+macro_rules! qmp {
+// the basic return value variant
+($e:expr, $errp:ident, $errval:expr) => {{
+assert!(!$errp.is_null());
+unsafe {
+*$errp = std::ptr::null_mut();
+}
+
+match $e {
+Ok(val) => val,
+Err(err) => unsafe {
+*$errp = err.to_qemu_full();
+$errval
+},
+}
+}};
+// the ptr return value variant
+($e:expr, $errp:ident) => {{
+assert!(!$errp.is_null());
+unsafe {
+*$errp = std::ptr::null_mut();
+}
+
+match $e {
+Ok(val) => val.to_qemu_full().into(),
+Err(err) => unsafe {
+*$errp = err.to_qemu_full();
+std::ptr::null_mut()
+},
+}
+}};
+}
-- 
2.28.0

[PoCv2 07/15] scripts/qapi: add Rust sys bindings generation

[marcandre . lureau]

From: Marc-André Lureau 

Generate the C QAPI types in Rust, with a few common niceties to
Debug/Clone/Copy the Rust type.

An important question that remains unsolved to be usable with the QEMU
schema in this version, is the handling of the 'if' compilation
conditions. Since the 'if' value is a C pre-processor condition, it is
hard to evaluate from Rust (we could implement a minimalistic CPP
evaluator, or invoke CPP and somehow parse the output...).

The normal Rust way of handling conditional compilation is via #[cfg]
features, which rely on feature arguments being passed to rustc from
Cargo. This would require a long Rust feature list, and new 'if-cfg'
conditions in the schema (an idea would be for Cargo to read features
from meson in the future?)

Signed-off-by: Marc-André Lureau 
---
 meson.build|   4 +-
 scripts/qapi-gen.py|  16 ++-
 scripts/qapi/rs.py | 126 
 scripts/qapi/rs_sys.py | 254 +
 4 files changed, 394 insertions(+), 6 deletions(-)
 create mode 100644 scripts/qapi/rs.py
 create mode 100644 scripts/qapi/rs_sys.py

diff --git a/meson.build b/meson.build
index c30bb290c5..b6b8330b97 100644
--- a/meson.build
+++ b/meson.build
@@ -1147,7 +1147,9 @@ qapi_gen_depends = [ meson.source_root() / 
'scripts/qapi/__init__.py',
  meson.source_root() / 'scripts/qapi/types.py',
  meson.source_root() / 'scripts/qapi/visit.py',
  meson.source_root() / 'scripts/qapi/common.py',
- meson.source_root() / 'scripts/qapi-gen.py'
+ meson.source_root() / 'scripts/qapi/rs.py',
+ meson.source_root() / 'scripts/qapi/rs_sys.py',
+ meson.source_root() / 'scripts/qapi-gen.py',
 ]
 
 tracetool = [
diff --git a/scripts/qapi-gen.py b/scripts/qapi-gen.py
index 541e8c1f55..5bfe9c8cd1 100644
--- a/scripts/qapi-gen.py
+++ b/scripts/qapi-gen.py
@@ -16,10 +16,13 @@ from qapi.schema import QAPIError, QAPISchema
 from qapi.types import gen_types
 from qapi.visit import gen_visit
 
+from qapi.rs_sys import gen_rs_systypes
 
 def main(argv):
 parser = argparse.ArgumentParser(
 description='Generate code from a QAPI schema')
+parser.add_argument('-r', '--rust', action='store_true',
+help="generate Rust code")
 parser.add_argument('-b', '--builtins', action='store_true',
 help="generate code for built-in types")
 parser.add_argument('-o', '--output-dir', action='store', default='',
@@ -45,11 +48,14 @@ def main(argv):
 print(err, file=sys.stderr)
 exit(1)
 
-gen_types(schema, args.output_dir, args.prefix, args.builtins)
-gen_visit(schema, args.output_dir, args.prefix, args.builtins)
-gen_commands(schema, args.output_dir, args.prefix)
-gen_events(schema, args.output_dir, args.prefix)
-gen_introspect(schema, args.output_dir, args.prefix, args.unmask)
+if args.rust:
+gen_rs_systypes(schema, args.output_dir, args.prefix, args.builtins)
+else:
+gen_types(schema, args.output_dir, args.prefix, args.builtins)
+gen_visit(schema, args.output_dir, args.prefix, args.builtins)
+gen_commands(schema, args.output_dir, args.prefix)
+gen_events(schema, args.output_dir, args.prefix)
+gen_introspect(schema, args.output_dir, args.prefix, args.unmask)
 
 
 if __name__ == '__main__':
diff --git a/scripts/qapi/rs.py b/scripts/qapi/rs.py
new file mode 100644
index 00..daa946580b
--- /dev/null
+++ b/scripts/qapi/rs.py
@@ -0,0 +1,126 @@
+# This work is licensed under the terms of the GNU GPL, version 2.
+# See the COPYING file in the top-level directory.
+"""
+QAPI Rust generator
+"""
+
+import os
+import subprocess
+
+from qapi.common import *
+from qapi.gen import QAPIGen, QAPISchemaVisitor
+
+
+rs_name_trans = str.maketrans('.-', '__')
+
+# Map @name to a valid Rust identifier.
+# If @protect, avoid returning certain ticklish identifiers (like
+# keywords) by prepending raw identifier prefix 'r#'.
+def rs_name(name, protect=True):
+name = name.translate(rs_name_trans)
+if name[0].isnumeric():
+name = '_' + name
+if not protect:
+return name
+# based from the list:
+# https://doc.rust-lang.org/reference/keywords.html
+if name in ('Self', 'abstract', 'as', 'async',
+'await','become', 'box', 'break',
+'const', 'continue', 'crate', 'do',
+'dyn', 'else', 'enum', 'extern',
+'false', 'final', 'fn', 'for',
+'if', 'impl', 'in', 'let',
+'loop', 'macro', 'match', 'mod',
+'move', 'mut', 'override', 'priv',
+'pub', 'ref', 'return', 'self',
+'static', 'struct', 'super', 'trait',
+'true', 'try', 'type', 'typeof',
+'union', 'unsafe', 'unsized', 'use',
+'virtual', 'w

[PoCv2 13/15] qga: implement {get,set}-vcpus in Rust

[marcandre . lureau]

From: Marc-André Lureau 

This is a rewrite of the C version (using the nix & winapi crates).

The main difference is that Rust doesn't let you mix const/mut logic,
the way transfer_vcpu in C does. The Rust version does introduce some
duplication, but is also more strict and can prevent mistakes.

Signed-off-by: Marc-André Lureau 
---
 qga/Cargo.toml   |   6 ++
 qga/commands-posix.c | 159 --
 qga/commands-win32.c |  76 
 qga/commands.c   |  14 
 qga/qmp/mod.rs   |  18 +
 qga/qmp/vcpus.rs | 161 +++
 tests/test-qga.c |   2 +
 7 files changed, 201 insertions(+), 235 deletions(-)
 create mode 100644 qga/qmp/vcpus.rs

diff --git a/qga/Cargo.toml b/qga/Cargo.toml
index 63a419255d..bb86fc543d 100644
--- a/qga/Cargo.toml
+++ b/qga/Cargo.toml
@@ -9,6 +9,12 @@ common = { path = "../rust/common" }
 libc = "^0.2.76"
 hostname = "^0.3.1"
 
+[target."cfg(unix)".dependencies]
+nix = "^0.18.0"
+
+[target."cfg(windows)".dependencies]
+winapi = { version = "^0.3.9", features = ["sysinfoapi", "winnt"] }
+
 [lib]
 path = "lib.rs"
 crate-type = ["staticlib"]
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 3bffee99d4..2c2c97fbca 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -2092,165 +2092,6 @@ error:
 return NULL;
 }
 
-#define SYSCONF_EXACT(name, errp) sysconf_exact((name), #name, (errp))
-
-static long sysconf_exact(int name, const char *name_str, Error **errp)
-{
-long ret;
-
-errno = 0;
-ret = sysconf(name);
-if (ret == -1) {
-if (errno == 0) {
-error_setg(errp, "sysconf(%s): value indefinite", name_str);
-} else {
-error_setg_errno(errp, errno, "sysconf(%s)", name_str);
-}
-}
-return ret;
-}
-
-/* Transfer online/offline status between @vcpu and the guest system.
- *
- * On input either @errp or *@errp must be NULL.
- *
- * In system-to-@vcpu direction, the following @vcpu fields are accessed:
- * - R: vcpu->logical_id
- * - W: vcpu->online
- * - W: vcpu->can_offline
- *
- * In @vcpu-to-system direction, the following @vcpu fields are accessed:
- * - R: vcpu->logical_id
- * - R: vcpu->online
- *
- * Written members remain unmodified on error.
- */
-static void transfer_vcpu(GuestLogicalProcessor *vcpu, bool sys2vcpu,
-  char *dirpath, Error **errp)
-{
-int fd;
-int res;
-int dirfd;
-static const char fn[] = "online";
-
-dirfd = open(dirpath, O_RDONLY | O_DIRECTORY);
-if (dirfd == -1) {
-error_setg_errno(errp, errno, "open(\"%s\")", dirpath);
-return;
-}
-
-fd = openat(dirfd, fn, sys2vcpu ? O_RDONLY : O_RDWR);
-if (fd == -1) {
-if (errno != ENOENT) {
-error_setg_errno(errp, errno, "open(\"%s/%s\")", dirpath, fn);
-} else if (sys2vcpu) {
-vcpu->online = true;
-vcpu->can_offline = false;
-} else if (!vcpu->online) {
-error_setg(errp, "logical processor #%" PRId64 " can't be "
-   "offlined", vcpu->logical_id);
-} /* otherwise pretend successful re-onlining */
-} else {
-unsigned char status;
-
-res = pread(fd, &status, 1, 0);
-if (res == -1) {
-error_setg_errno(errp, errno, "pread(\"%s/%s\")", dirpath, fn);
-} else if (res == 0) {
-error_setg(errp, "pread(\"%s/%s\"): unexpected EOF", dirpath,
-   fn);
-} else if (sys2vcpu) {
-vcpu->online = (status != '0');
-vcpu->can_offline = true;
-} else if (vcpu->online != (status != '0')) {
-status = '0' + vcpu->online;
-if (pwrite(fd, &status, 1, 0) == -1) {
-error_setg_errno(errp, errno, "pwrite(\"%s/%s\")", dirpath,
- fn);
-}
-} /* otherwise pretend successful re-(on|off)-lining */
-
-res = close(fd);
-g_assert(res == 0);
-}
-
-res = close(dirfd);
-g_assert(res == 0);
-}
-
-GuestLogicalProcessorList *qmp_guest_get_vcpus(Error **errp)
-{
-int64_t current;
-GuestLogicalProcessorList *head, **link;
-long sc_max;
-Error *local_err = NULL;
-
-current = 0;
-head = NULL;
-link = &head;
-sc_max = SYSCONF_EXACT(_SC_NPROCESSORS_CONF, &local_err);
-
-while (local_err == NULL && current < sc_max) {
-GuestLogicalProcessor *vcpu;
-GuestLogicalProcessorList *entry;
-int64_t id = current++;
-char *path = g_strdup_printf("/sys/devices/system/cpu/cpu%" PRId64 "/",
- id);
-
-if (g_file_test(path, G_FILE_TEST_EXISTS)) {
-vcpu = g_malloc0(sizeof *vcpu);
-vcpu->logical_id = id;
-vcpu->has_can_offline = true; /* lolspeak ftw */
-transfer_vcpu(vcpu, true, path, &local_err);
-entry = g_malloc0(size

[PoCv2 12/15] qga: implement get-host-name in Rust

[marcandre . lureau]

From: Marc-André Lureau 

Use the "hostname" crate (https://github.com/svartalf/hostname)

(notice the wrong error message in our win32 implementation)

Signed-off-by: Marc-André Lureau 
---
 include/qemu/osdep.h | 10 --
 qga/Cargo.toml   |  1 +
 qga/commands.c   | 20 
 qga/lib.rs   |  2 ++
 qga/qmp/hostname.rs  |  9 +
 qga/qmp/mod.rs   |  7 +++
 tests/test-qga.c |  2 ++
 util/oslib-posix.c   | 35 ---
 util/oslib-win32.c   | 13 -
 9 files changed, 25 insertions(+), 74 deletions(-)
 create mode 100644 qga/qmp/hostname.rs

diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index f9ec8c84e9..1ea244fc06 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -664,16 +664,6 @@ static inline void qemu_reset_optind(void)
 #endif
 }
 
-/**
- * qemu_get_host_name:
- * @errp: Error object
- *
- * Operating system agnostic way of querying host name.
- *
- * Returns allocated hostname (caller should free), NULL on failure.
- */
-char *qemu_get_host_name(Error **errp);
-
 /**
  * qemu_get_host_physmem:
  *
diff --git a/qga/Cargo.toml b/qga/Cargo.toml
index 9966057594..63a419255d 100644
--- a/qga/Cargo.toml
+++ b/qga/Cargo.toml
@@ -7,6 +7,7 @@ license = "GPLv2"
 [dependencies]
 common = { path = "../rust/common" }
 libc = "^0.2.76"
+hostname = "^0.3.1"
 
 [lib]
 path = "lib.rs"
diff --git a/qga/commands.c b/qga/commands.c
index 3dcd5fbe5c..15478a16e7 100644
--- a/qga/commands.c
+++ b/qga/commands.c
@@ -512,25 +512,13 @@ int ga_parse_whence(GuestFileWhence *whence, Error **errp)
 return -1;
 }
 
+#ifndef CONFIG_WITH_RUST
 GuestHostName *qmp_guest_get_host_name(Error **errp)
 {
-GuestHostName *result = NULL;
-g_autofree char *hostname = qemu_get_host_name(errp);
-
-/*
- * We want to avoid using g_get_host_name() because that
- * caches the result and we wouldn't reflect changes in the
- * host name.
- */
-
-if (!hostname) {
-hostname = g_strdup("localhost");
-}
-
-result = g_new0(GuestHostName, 1);
-result->host_name = g_steal_pointer(&hostname);
-return result;
+error_setg(errp, QERR_UNSUPPORTED);
+return NULL;
 }
+#endif
 
 GuestTimezone *qmp_guest_get_timezone(Error **errp)
 {
diff --git a/qga/lib.rs b/qga/lib.rs
index 5fe08c25a3..f4967f59e5 100644
--- a/qga/lib.rs
+++ b/qga/lib.rs
@@ -1,3 +1,5 @@
+pub use common::{err, Error, Result};
+
 mod qapi;
 mod qapi_sys;
 mod qmp;
diff --git a/qga/qmp/hostname.rs b/qga/qmp/hostname.rs
new file mode 100644
index 00..c3eb1f6fd2
--- /dev/null
+++ b/qga/qmp/hostname.rs
@@ -0,0 +1,9 @@
+use crate::*;
+
+pub(crate) fn get() -> Result {
+let host_name = hostname::get()?
+.into_string()
+.or_else(|_| err!("Invalid hostname"))?;
+
+Ok(qapi::GuestHostName { host_name })
+}
diff --git a/qga/qmp/mod.rs b/qga/qmp/mod.rs
index 38060100af..e855aa4bd7 100644
--- a/qga/qmp/mod.rs
+++ b/qga/qmp/mod.rs
@@ -34,3 +34,10 @@ macro_rules! qmp {
 }
 }};
 }
+
+mod hostname;
+
+#[no_mangle]
+extern "C" fn qmp_guest_get_host_name(errp: *mut *mut sys::Error) -> *mut 
qapi_sys::GuestHostName {
+qmp!(hostname::get(), errp)
+}
diff --git a/tests/test-qga.c b/tests/test-qga.c
index c1b173b3cb..6190e93e0e 100644
--- a/tests/test-qga.c
+++ b/tests/test-qga.c
@@ -863,6 +863,7 @@ static void test_qga_guest_exec_invalid(gconstpointer fix)
 
 static void test_qga_guest_get_host_name(gconstpointer fix)
 {
+#ifdef CONFIG_WITH_RUST
 const TestFixture *fixture = fix;
 QDict *ret, *val;
 
@@ -874,6 +875,7 @@ static void test_qga_guest_get_host_name(gconstpointer fix)
 g_assert(qdict_haskey(val, "host-name"));
 
 qobject_unref(ret);
+#endif
 }
 
 static void test_qga_guest_get_timezone(gconstpointer fix)
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index f15234b5c0..1722c269b8 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -804,41 +804,6 @@ void sigaction_invoke(struct sigaction *action,
 action->sa_sigaction(info->ssi_signo, &si, NULL);
 }
 
-#ifndef HOST_NAME_MAX
-# ifdef _POSIX_HOST_NAME_MAX
-#  define HOST_NAME_MAX _POSIX_HOST_NAME_MAX
-# else
-#  define HOST_NAME_MAX 255
-# endif
-#endif
-
-char *qemu_get_host_name(Error **errp)
-{
-long len = -1;
-g_autofree char *hostname = NULL;
-
-#ifdef _SC_HOST_NAME_MAX
-len = sysconf(_SC_HOST_NAME_MAX);
-#endif /* _SC_HOST_NAME_MAX */
-
-if (len < 0) {
-len = HOST_NAME_MAX;
-}
-
-/* Unfortunately, gethostname() below does not guarantee a
- * NULL terminated string. Therefore, allocate one byte more
- * to be sure. */
-hostname = g_new0(char, len + 1);
-
-if (gethostname(hostname, len) < 0) {
-error_setg_errno(errp, errno,
- "cannot get hostname");
-return NULL;
-}
-
-return g_steal_pointer(&hostname);
-}
-
 size_t qemu_get_host_physmem(void)
 {
 #ifdef _SC_PHYS_PAGES
diff --git a/util/oslib-win32.c b/util/os

Re: [PATCH 3/4] hw/pci-host/versatile: Add WINDOW_COUNT definition to replace magic '3'

[Peter Maydell]

On Sun, 11 Oct 2020 at 20:49, Philippe Mathieu-Daudé  wrote:
>
> Use self-explicit WINDOW_COUNT definition instead of a magic value.
>
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  hw/pci-host/versatile.c | 28 ++--
>  1 file changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/hw/pci-host/versatile.c b/hw/pci-host/versatile.c
> index b4951023f4e..4d9565de4b1 100644
> --- a/hw/pci-host/versatile.c
> +++ b/hw/pci-host/versatile.c
> @@ -72,6 +72,8 @@ enum {
>  PCI_VPB_IRQMAP_FORCE_OK,
>  };
>
> +#define WINDOW_COUNT 3
> +
>  struct PCIVPBState {
>  PCIHostState parent_obj;
>
> @@ -86,18 +88,18 @@ struct PCIVPBState {
>   * The offsets into pci_mem_space are controlled by the imap registers.
>   */
>  MemoryRegion pci_io_window;
> -MemoryRegion pci_mem_window[3];
> +MemoryRegion pci_mem_window[WINDOW_COUNT];
>  PCIBus pci_bus;
>  PCIDevice pci_dev;
>
>  /* Constant for life of device: */
>  int realview;
> -uint32_t mem_win_size[3];
> +uint32_t mem_win_size[WINDOW_COUNT];
>  uint8_t irq_mapping_prop;
>
>  /* Variable state: */
> -uint32_t imap[3];
> -uint32_t smap[3];
> +uint32_t imap[WINDOW_COUNT];
> +uint32_t smap[WINDOW_COUNT];

Strictly speaking, this is conflating two separate
things which both happen to be equal to three.

The versatile/realview PCI controller has:
 * three memory windows in the system address space
   - those are represented by the pci_mem_window[] array
   - mem_win_size[] holds the size of each window
 (which is fixed but varies between the different
 implementations of this controller on different boards)
   - the device IMAPn registers allow the guest to
 configure the mapping from "a CPU access to an
 address in window n" to "a PCI address on the PCI bus,
 and our imap[] array holds those register values
 * three PCI BARs which represent memory windows on the
 PCI bus which bus-master PCI devices can use to
 write back into the system address space
   - the device SMAPn registers allow the guest to configure
 the mapping from "a bus-master access to an address
 on the PCI bus wherever the guest mapped BAR n"
 to "a system address", and the smap[] array holds
 those register values
There's no inherent reason why the number of PCI BARs
needs to be the same as the number of system address
space memory windows, so they shouldn't really share
the same constant.

(We don't actually implement the behaviour of the SMAP
registers and the BARs, because Linux always configures
the PCI controller to a 1:1 mapping of PCI space to
system address space. So we get away with just having
our implementation be "always do direct accesses".)

thanks
-- PMM

[PoCv2 15/15] rust: use vendored-sources

[marcandre . lureau]

From: Marc-André Lureau 

Most likely, QEMU will want tighter control over the sources, rather
than relying on crates.io downloading, use a git submodule with all the
dependencies.

"cargo vendor" makes that simple.

Signed-off-by: Marc-André Lureau 
---
 .cargo/config| 5 +
 .gitmodules  | 3 +++
 configure| 8 
 rust/vendored| 1 +
 scripts/cargo_wrapper.py | 1 +
 5 files changed, 18 insertions(+)
 create mode 100644 .cargo/config
 create mode 16 rust/vendored

diff --git a/.cargo/config b/.cargo/config
new file mode 100644
index 00..a8c55940d5
--- /dev/null
+++ b/.cargo/config
@@ -0,0 +1,5 @@
+[source.crates-io]
+replace-with = "vendored-sources"
+
+[source.vendored-sources]
+directory = "rust/vendored"
diff --git a/.gitmodules b/.gitmodules
index 2bdeeacef8..62a2be12b9 100644
--- a/.gitmodules
+++ b/.gitmodules
@@ -64,3 +64,6 @@
 [submodule "roms/vbootrom"]
path = roms/vbootrom
url = https://git.qemu.org/git/vbootrom.git
+[submodule "rust/vendored"]
+   path = rust/vendored
+   url = https://github.com/elmarco/qemu-rust-vendored.git
diff --git a/configure b/configure
index 7945ceac63..702546d87f 100755
--- a/configure
+++ b/configure
@@ -1932,6 +1932,14 @@ if test "$with_rust" = auto && has cargo; then
 with_rust=yes
 fi
 
+case "$with_rust" in
+  yes)
+if test -e "${source_path}/.git" && test $git_update = 'yes' ; then
+  git_submodules="${git_submodules} rust/vendored"
+fi
+;;
+esac
+
 # Check that the C compiler works. Doing this here before testing
 # the host CPU ensures that we had a valid CC to autodetect the
 # $cpu var (and we should bail right here if that's not the case).
diff --git a/rust/vendored b/rust/vendored
new file mode 16
index 00..71ee65d042
--- /dev/null
+++ b/rust/vendored
@@ -0,0 +1 @@
+Subproject commit 71ee65d042606d18de3175d67f9e4e4b78a1f865
diff --git a/scripts/cargo_wrapper.py b/scripts/cargo_wrapper.py
index 164fad5123..4a0673407b 100644
--- a/scripts/cargo_wrapper.py
+++ b/scripts/cargo_wrapper.py
@@ -51,6 +51,7 @@ def build_lib(args: argparse.Namespace) -> None:
 target_dir,
 "--manifest-path",
 manifest_path,
+"--offline",
 ]
 if args.target_triple:
 cargo_cmd += ["--target", args.target_triple]
-- 
2.28.0

Re: [PoCv2 01/15] mingw: fix error __USE_MINGW_ANSI_STDIO redefined

[Marc-André Lureau]

Hi

On Mon, Oct 12, 2020 at 12:35 AM  wrote:
>
> From: Marc-André Lureau 
>
> Always put osdep.h first, and remove redundant stdlib.h include.
>
> Signed-off-by: Marc-André Lureau 

(ignore this patch, which was already sent earlier)

> ---
>  migration/dirtyrate.c | 3 ++-
>  tests/test-bitmap.c   | 1 -
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
> index 68577ef250..47f761e67a 100644
> --- a/migration/dirtyrate.c
> +++ b/migration/dirtyrate.c
> @@ -10,8 +10,9 @@
>   * See the COPYING file in the top-level directory.
>   */
>
> -#include 
>  #include "qemu/osdep.h"
> +
> +#include 
>  #include "qapi/error.h"
>  #include "cpu.h"
>  #include "qemu/config-file.h"
> diff --git a/tests/test-bitmap.c b/tests/test-bitmap.c
> index 2f5b71458a..8db4f67883 100644
> --- a/tests/test-bitmap.c
> +++ b/tests/test-bitmap.c
> @@ -8,7 +8,6 @@
>   * Author: Peter Xu 
>   */
>
> -#include 
>  #include "qemu/osdep.h"
>  #include "qemu/bitmap.h"
>
> --
> 2.28.0
>

Re: [PoCv2 00/15] Rust binding for QAPI (qemu-ga only, for now)

[no-reply]

Patchew URL: 
https://patchew.org/QEMU/20201011203513.1621355-1-marcandre.lur...@redhat.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20201011203513.1621355-1-marcandre.lur...@redhat.com
Subject: [PoCv2 00/15] Rust binding for QAPI (qemu-ga only, for now)

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag] 
patchew/20201011203513.1621355-1-marcandre.lur...@redhat.com -> 
patchew/20201011203513.1621355-1-marcandre.lur...@redhat.com
Switched to a new branch 'test'
e5abacf rust: use vendored-sources
f8125dc travis: add Rust
0bb6095b qga: implement {get,set}-vcpus in Rust
6f1cb07 qga: implement get-host-name in Rust
6cc3f63 qga: add qmp! macro helper
abc816c qga/rust: build Rust types
8273b34 scripts/qapi: add generation of Rust bindings for types
3068424 qga/rust: generate QGA QAPI sys bindings
8d23431 scripts/qapi: add Rust sys bindings generation
eb4bb2d rust: provide a common crate for QEMU
f19d443 qga/rust: build and link an empty static library
03097d9 build-sys: add a cargo-wrapper script
d8ec366 build-sys: add --with-rust{-target} & basic build infrastructure
e524eda scripts/qapi: teach c_param_type() to return const argument type
e68868d mingw: fix error __USE_MINGW_ANSI_STDIO redefined

=== OUTPUT BEGIN ===
1/15 Checking commit e68868d9d68b (mingw: fix error __USE_MINGW_ANSI_STDIO 
redefined)
2/15 Checking commit e524eda108f7 (scripts/qapi: teach c_param_type() to return 
const argument type)
WARNING: line over 80 characters
#27: FILE: scripts/qapi/schema.py:171:
+# The argument should be considered const, since no ownership is given to 
the callee,

WARNING: line over 80 characters
#28: FILE: scripts/qapi/schema.py:172:
+# but qemu C code frequently tweaks it. Set const=True for a stricter 
declaration.

total: 0 errors, 2 warnings, 28 lines checked

Patch 2/15 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
3/15 Checking commit d8ec36655d29 (build-sys: add --with-rust{-target} & basic 
build infrastructure)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#29: 
new file mode 100644

total: 0 errors, 1 warnings, 85 lines checked

Patch 3/15 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
4/15 Checking commit 03097d99222b (build-sys: add a cargo-wrapper script)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#39: 
new file mode 100644

WARNING: line over 80 characters
#82: FILE: scripts/cargo_wrapper.py:39:
+target_dir, args.target_triple, args.build_type, "lib" + package_name 
+ ".a"

WARNING: line over 80 characters
#119: FILE: scripts/cargo_wrapper.py:76:
+"Environment: " + " ".join(["{}={}".format(k, v) for k, v in 
env.items()])

total: 0 errors, 3 warnings, 108 lines checked

Patch 4/15 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
5/15 Checking commit f19d443c4992 (qga/rust: build and link an empty static 
library)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#29: 
new file mode 100644

total: 0 errors, 1 warnings, 38 lines checked

Patch 5/15 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
6/15 Checking commit eb4bb2d04ac5 (rust: provide a common crate for QEMU)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#32: 
new file mode 100644

total: 0 errors, 1 warnings, 533 lines checked

Patch 6/15 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
7/15 Checking commit 8d23431862a8 (scripts/qapi: add Rust sys bindings 
generation)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#79: 
new file mode 100644

WARNING: line over 80 characters
#182: FILE: scripts/qapi/rs.py:99:
+value = ''.join(word.title() for word in filter(None, re.split("[-_]+", 
value)))

ERROR: line over 90 characters
#270: FILE: scripts/qapi/rs_sys.py:55:
+ rs_systype=rs_systype(memb.type.c_type(), ''), 
rs_name=rs_name(memb.name))

WARNING: line over 80 characters
#332: FILE: scripts/qapi/rs_sys.py:117:
+ret += gen_rs_sys_object(v.type.name, v.type.ifcond, 
v.type.base,

WARNING: line over 80 characters
#429: FILE: scripts/qapi/rs_sys.py:214:
+ rs_name=

[PATCH 0/3] hw/ssi: Rename SSI 'slave' as 'peripheral'

[Philippe Mathieu-Daudé]

In order to use inclusive terminology, rename SSI 'slave' as
'peripheral', following the resolution Paolo pointed in [*]:
https://www.oshwa.org/a-resolution-to-redefine-spi-signal-names/

Candidate to be merged via the ARM or Trivial trees.

[*] https://www.mail-archive.com/qemu-devel@nongnu.org/msg739108.html

Philippe Mathieu-Daudé (3):
  hw/ssi/aspeed_smc: Rename max_slaves as max_devices
  hw/ssi/ssi: Update coding style to make checkpatch.pl happy
  hw/ssi: Rename SSI 'slave' as 'peripheral'

 include/hw/misc/max111x.h   |  2 +-
 include/hw/ssi/aspeed_smc.h |  2 +-
 include/hw/ssi/ssi.h| 56 +++--
 hw/arm/spitz.c  | 32 ++---
 hw/arm/stellaris.c  |  4 +--
 hw/arm/tosa.c   | 12 
 hw/arm/z2.c | 14 +-
 hw/block/m25p80.c   | 14 +-
 hw/display/ads7846.c| 12 
 hw/display/ssd0323.c| 12 
 hw/misc/max111x.c   | 18 ++--
 hw/sd/ssi-sd.c  | 12 
 hw/ssi/aspeed_smc.c | 53 ++-
 hw/ssi/pl022.c  |  2 +-
 hw/ssi/ssi.c| 48 +++
 hw/ssi/xilinx_spips.c   |  7 +++--
 16 files changed, 152 insertions(+), 148 deletions(-)

-- 
2.26.2

[PATCH 1/3] hw/ssi/aspeed_smc: Rename max_slaves as max_devices

[Philippe Mathieu-Daudé]

From: Philippe Mathieu-Daudé 

In order to use inclusive terminology, rename max_slaves
as max_peripherals.

Patch generated using:

  $ sed -i s/slave/peripheral/ \
hw/ssi/aspeed_smc.c include/hw/ssi/aspeed_smc.h

One line in aspeed_smc_read() has been manually tweaked
to pass checkpatch.

Reviewed-by: Thomas Huth 
Reviewed-by: Cédric Le Goater 
Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/ssi/aspeed_smc.h |  2 +-
 hw/ssi/aspeed_smc.c | 53 +++--
 2 files changed, 28 insertions(+), 27 deletions(-)

diff --git a/include/hw/ssi/aspeed_smc.h b/include/hw/ssi/aspeed_smc.h
index 3dd354b52ec..16c03fe64f3 100644
--- a/include/hw/ssi/aspeed_smc.h
+++ b/include/hw/ssi/aspeed_smc.h
@@ -43,7 +43,7 @@ typedef struct AspeedSMCController {
 uint8_t r_timings;
 uint8_t nregs_timings;
 uint8_t conf_enable_w0;
-uint8_t max_slaves;
+uint8_t max_peripherals;
 const AspeedSegments *segments;
 hwaddr flash_window_base;
 uint32_t flash_window_size;
diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
index 795784e5f36..2780cac71d2 100644
--- a/hw/ssi/aspeed_smc.c
+++ b/hw/ssi/aspeed_smc.c
@@ -181,7 +181,7 @@
 #define SNOOP_START   0x0
 
 /*
- * Default segments mapping addresses and size for each slave per
+ * Default segments mapping addresses and size for each peripheral per
  * controller. These can be changed when board is initialized with the
  * Segment Address Registers.
  */
@@ -259,7 +259,7 @@ static const AspeedSMCController controllers[] = {
 .r_timings = R_TIMINGS,
 .nregs_timings = 1,
 .conf_enable_w0= CONF_ENABLE_W0,
-.max_slaves= 1,
+.max_peripherals   = 1,
 .segments  = aspeed_segments_legacy,
 .flash_window_base = ASPEED_SOC_SMC_FLASH_BASE,
 .flash_window_size = 0x600,
@@ -275,7 +275,7 @@ static const AspeedSMCController controllers[] = {
 .r_timings = R_TIMINGS,
 .nregs_timings = 1,
 .conf_enable_w0= CONF_ENABLE_W0,
-.max_slaves= 5,
+.max_peripherals   = 5,
 .segments  = aspeed_segments_fmc,
 .flash_window_base = ASPEED_SOC_FMC_FLASH_BASE,
 .flash_window_size = 0x1000,
@@ -293,7 +293,7 @@ static const AspeedSMCController controllers[] = {
 .r_timings = R_SPI_TIMINGS,
 .nregs_timings = 1,
 .conf_enable_w0= SPI_CONF_ENABLE_W0,
-.max_slaves= 1,
+.max_peripherals   = 1,
 .segments  = aspeed_segments_spi,
 .flash_window_base = ASPEED_SOC_SPI_FLASH_BASE,
 .flash_window_size = 0x1000,
@@ -309,7 +309,7 @@ static const AspeedSMCController controllers[] = {
 .r_timings = R_TIMINGS,
 .nregs_timings = 1,
 .conf_enable_w0= CONF_ENABLE_W0,
-.max_slaves= 3,
+.max_peripherals   = 3,
 .segments  = aspeed_segments_ast2500_fmc,
 .flash_window_base = ASPEED_SOC_FMC_FLASH_BASE,
 .flash_window_size = 0x1000,
@@ -327,7 +327,7 @@ static const AspeedSMCController controllers[] = {
 .r_timings = R_TIMINGS,
 .nregs_timings = 1,
 .conf_enable_w0= CONF_ENABLE_W0,
-.max_slaves= 2,
+.max_peripherals   = 2,
 .segments  = aspeed_segments_ast2500_spi1,
 .flash_window_base = ASPEED_SOC_SPI_FLASH_BASE,
 .flash_window_size = 0x800,
@@ -343,7 +343,7 @@ static const AspeedSMCController controllers[] = {
 .r_timings = R_TIMINGS,
 .nregs_timings = 1,
 .conf_enable_w0= CONF_ENABLE_W0,
-.max_slaves= 2,
+.max_peripherals   = 2,
 .segments  = aspeed_segments_ast2500_spi2,
 .flash_window_base = ASPEED_SOC_SPI2_FLASH_BASE,
 .flash_window_size = 0x800,
@@ -359,7 +359,7 @@ static const AspeedSMCController controllers[] = {
 .r_timings = R_TIMINGS,
 .nregs_timings = 1,
 .conf_enable_w0= CONF_ENABLE_W0,
-.max_slaves= 3,
+.max_peripherals   = 3,
 .segments  = aspeed_segments_ast2600_fmc,
 .flash_window_base = ASPEED26_SOC_FMC_FLASH_BASE,
 .flash_window_size = 0x1000,
@@ -377,7 +377,7 @@ static const AspeedSMCController controllers[] = {
 .r_timings = R_TIMINGS,
 .nregs_timings = 2,
 .conf_enable_w0= CONF_ENABLE_W0,
-.max_slaves= 2,
+.max_peripherals   = 2,
 .segments  = aspeed_segments_ast2600_spi1,
 .flash_window_base = ASPEED26_SOC_SPI_FLASH_BASE,
 .flash_window_size = 0x1000,
@@ -395,7 +395,7 @@ static const AspeedSMCController controllers[] = {
 .r_timings = R_TIMINGS,
 .nregs_timings = 3,
 .conf_enable_w0= CONF_ENABLE_W0,
-.max_sl

Re: [PATCH 04/10] hw/isa: Add the ISA_IRQ_TPM_DEFAULT definition

[Philippe Mathieu-Daudé]

On 10/11/20 10:28 PM, Stefan Berger wrote:

On 10/11/20 3:32 PM, Philippe Mathieu-Daudé wrote:

The TPM TIS device uses IRQ #5 by default. Add this
default definition to the IsaIrqNumber enum.

Avoid magic values in the code, replace them by the
newly introduced definition.

Signed-off-by: Philippe Mathieu-Daudé 
---
  include/hw/isa/isa.h   | 1 +
  hw/i386/acpi-build.c   | 2 +-
  hw/ipmi/isa_ipmi_bt.c  | 2 +-
  hw/ipmi/isa_ipmi_kcs.c | 2 +-
  hw/tpm/tpm_tis_isa.c   | 2 +-
  5 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/include/hw/isa/isa.h b/include/hw/isa/isa.h
index 519296d5823..e4f2aed004f 100644
--- a/include/hw/isa/isa.h
+++ b/include/hw/isa/isa.h
@@ -11,6 +11,7 @@
  enum IsaIrqNumber {
  ISA_IRQ_KBD_DEFAULT =  1,
  ISA_IRQ_SER_DEFAULT =  4,
+    ISA_IRQ_TPM_DEFAULT =  5,
  ISA_NUM_IRQS    = 16
  };

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 45ad2f95334..2b6038ab015 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1886,7 +1886,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
  Rewrite to take IRQ from TPM device model and
  fix default IRQ value there to use some unused IRQ
   */
-    /* aml_append(crs, aml_irq_no_flags(TPM_TIS_IRQ)); */
+    /* aml_append(crs, 
aml_irq_no_flags(ISA_IRQ_TPM_DEFAULT)); */

  aml_append(dev, aml_name_decl("_CRS", crs));

  tpm_build_ppi_acpi(tpm, dev);
diff --git a/hw/ipmi/isa_ipmi_bt.c b/hw/ipmi/isa_ipmi_bt.c
index b7c2ad557b2..13a92bd2c44 100644
--- a/hw/ipmi/isa_ipmi_bt.c
+++ b/hw/ipmi/isa_ipmi_bt.c
@@ -137,7 +137,7 @@ static void 
*isa_ipmi_bt_get_backend_data(IPMIInterface *ii)


  static Property ipmi_isa_properties[] = {
  DEFINE_PROP_UINT32("ioport", ISAIPMIBTDevice, bt.io_base,  0xe4),
-    DEFINE_PROP_INT32("irq",   ISAIPMIBTDevice, isairq,  5),
+    DEFINE_PROP_INT32("irq", ISAIPMIBTDevice, isairq, 
ISA_IRQ_TPM_DEFAULT),

  DEFINE_PROP_END_OF_LIST(),
  };

diff --git a/hw/ipmi/isa_ipmi_kcs.c b/hw/ipmi/isa_ipmi_kcs.c
index 7dd6bf0040a..c956b539688 100644
--- a/hw/ipmi/isa_ipmi_kcs.c
+++ b/hw/ipmi/isa_ipmi_kcs.c
@@ -144,7 +144,7 @@ static void 
*isa_ipmi_kcs_get_backend_data(IPMIInterface *ii)


  static Property ipmi_isa_properties[] = {
  DEFINE_PROP_UINT32("ioport", ISAIPMIKCSDevice, kcs.io_base,  
0xca2),

-    DEFINE_PROP_INT32("irq",   ISAIPMIKCSDevice, isairq,  5),
+    DEFINE_PROP_INT32("irq", ISAIPMIKCSDevice, isairq, 
ISA_IRQ_TPM_DEFAULT),

  DEFINE_PROP_END_OF_LIST(),
  };



I don't know what these devices do but this looks like an unwanted clash.


Yes, sorry :( Maybe better to drop this patch.




diff --git a/hw/tpm/tpm_tis_isa.c b/hw/tpm/tpm_tis_isa.c
index 6fd876eebf1..5a4afda42df 100644
--- a/hw/tpm/tpm_tis_isa.c
+++ b/hw/tpm/tpm_tis_isa.c
@@ -91,7 +91,7 @@ static void tpm_tis_isa_reset(DeviceState *dev)
  }

  static Property tpm_tis_isa_properties[] = {
-    DEFINE_PROP_UINT32("irq", TPMStateISA, state.irq_num, TPM_TIS_IRQ),
+    DEFINE_PROP_UINT32("irq", TPMStateISA, state.irq_num, 
ISA_IRQ_TPM_DEFAULT),

  DEFINE_PROP_TPMBE("tpmdev", TPMStateISA, state.be_driver),
  DEFINE_PROP_BOOL("ppi", TPMStateISA, state.ppi_enabled, true),
  DEFINE_PROP_END_OF_LIST(),

[PATCH 2/3] hw/ssi/ssi: Update coding style to make checkpatch.pl happy

[Philippe Mathieu-Daudé]

To make the next commit easier to review, clean this code first.

Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/ssi/ssi.h | 16 +---
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/include/hw/ssi/ssi.h b/include/hw/ssi/ssi.h
index fe3028c39dc..c15548425a3 100644
--- a/include/hw/ssi/ssi.h
+++ b/include/hw/ssi/ssi.h
@@ -1,12 +1,14 @@
 /* QEMU Synchronous Serial Interface support.  */
 
-/* In principle SSI is a point-point interface.  As such the qemu
-   implementation has a single slave device on a "bus".
-   However it is fairly common for boards to have multiple slaves
-   connected to a single master, and select devices with an external
-   chip select.  This is implemented in qemu by having an explicit mux device.
-   It is assumed that master and slave are both using the same transfer width.
-   */
+/*
+ * In principle SSI is a point-point interface.  As such the qemu
+ * implementation has a single slave device on a "bus".
+ * However it is fairly common for boards to have multiple slaves
+ * connected to a single master, and select devices with an external
+ * chip select.  This is implemented in qemu by having an explicit mux device.
+ * It is assumed that master and slave are both using the same transfer
+ * width.
+ */
 
 #ifndef QEMU_SSI_H
 #define QEMU_SSI_H
-- 
2.26.2

[PATCH 3/3] hw/ssi: Rename SSI 'slave' as 'peripheral'

[Philippe Mathieu-Daudé]

In order to use inclusive terminology, rename SSI 'slave' as
'peripheral', following the specification resolution:
https://www.oshwa.org/a-resolution-to-redefine-spi-signal-names/

Patch created mechanically using:

  $ sed -i s/SSISlave/SSIPeripheral/ $(git grep -l SSISlave)
  $ sed -i s/SSI_SLAVE/SSI_PERIPHERAL/ $(git grep -l SSI_SLAVE)
  $ sed -i s/ssi-slave/ssi-peripheral/ $(git grep -l ssi-slave)
  $ sed -i s/ssi_slave/ssi_peripheral/ $(git grep -l ssi_slave)
  $ sed -i s/ssi_create_slave/ssi_create_peripheral/ \
$(git grep -l ssi_create_slave)

Then in VMStateDescription vmstate_ssi_peripheral we restored
the "SSISlave" migration stream name (to avoid breaking migration).

Finally the following files have been manually tweaked:
 - hw/ssi/pl022.c
 - hw/ssi/xilinx_spips.c

Suggested-by: Paolo Bonzini 
Signed-off-by: Philippe Mathieu-Daudé 
---
 include/hw/misc/max111x.h |  2 +-
 include/hw/ssi/ssi.h  | 46 ++---
 hw/arm/spitz.c| 32 +-
 hw/arm/stellaris.c|  4 ++--
 hw/arm/tosa.c | 12 +-
 hw/arm/z2.c   | 14 ++--
 hw/block/m25p80.c | 14 ++--
 hw/display/ads7846.c  | 12 +-
 hw/display/ssd0323.c  | 12 +-
 hw/misc/max111x.c | 18 +++
 hw/sd/ssi-sd.c| 12 +-
 hw/ssi/pl022.c|  2 +-
 hw/ssi/ssi.c  | 48 +++
 hw/ssi/xilinx_spips.c |  7 +++---
 14 files changed, 118 insertions(+), 117 deletions(-)

diff --git a/include/hw/misc/max111x.h b/include/hw/misc/max111x.h
index 606cf1e0a2a..beff59c815d 100644
--- a/include/hw/misc/max111x.h
+++ b/include/hw/misc/max111x.h
@@ -33,7 +33,7 @@
  *be lowered once it has been asserted.
  */
 struct MAX111xState {
-SSISlave parent_obj;
+SSIPeripheral parent_obj;
 
 qemu_irq interrupt;
 /* Values of inputs at system reset (settable by QOM property) */
diff --git a/include/hw/ssi/ssi.h b/include/hw/ssi/ssi.h
index c15548425a3..f411858ab08 100644
--- a/include/hw/ssi/ssi.h
+++ b/include/hw/ssi/ssi.h
@@ -2,11 +2,11 @@
 
 /*
  * In principle SSI is a point-point interface.  As such the qemu
- * implementation has a single slave device on a "bus".
- * However it is fairly common for boards to have multiple slaves
+ * implementation has a single peripheral on a "bus".
+ * However it is fairly common for boards to have multiple peripherals
  * connected to a single master, and select devices with an external
  * chip select.  This is implemented in qemu by having an explicit mux device.
- * It is assumed that master and slave are both using the same transfer
+ * It is assumed that master and peripheral are both using the same transfer
  * width.
  */
 
@@ -18,9 +18,9 @@
 
 typedef enum SSICSMode SSICSMode;
 
-#define TYPE_SSI_SLAVE "ssi-slave"
-OBJECT_DECLARE_TYPE(SSISlave, SSISlaveClass,
-SSI_SLAVE)
+#define TYPE_SSI_PERIPHERAL "ssi-peripheral"
+OBJECT_DECLARE_TYPE(SSIPeripheral, SSIPeripheralClass,
+SSI_PERIPHERAL)
 
 #define SSI_GPIO_CS "ssi-gpio-cs"
 
@@ -30,21 +30,21 @@ enum SSICSMode {
 SSI_CS_HIGH,
 };
 
-/* Slave devices.  */
-struct SSISlaveClass {
+/* Peripherals.  */
+struct SSIPeripheralClass {
 DeviceClass parent_class;
 
-void (*realize)(SSISlave *dev, Error **errp);
+void (*realize)(SSIPeripheral *dev, Error **errp);
 
 /* if you have standard or no CS behaviour, just override transfer.
  * This is called when the device cs is active (true by default).
  */
-uint32_t (*transfer)(SSISlave *dev, uint32_t val);
+uint32_t (*transfer)(SSIPeripheral *dev, uint32_t val);
 /* called when the CS line changes. Optional, devices only need to 
implement
  * this if they have side effects associated with the cs line (beyond
  * tristating the txrx lines).
  */
-int (*set_cs)(SSISlave *dev, bool select);
+int (*set_cs)(SSIPeripheral *dev, bool select);
 /* define whether or not CS exists and is active low/high */
 SSICSMode cs_polarity;
 
@@ -53,30 +53,30 @@ struct SSISlaveClass {
  * cs_polarity are unused if this is overwritten. Transfer_raw will
  * always be called for the device for every txrx access to the parent bus
  */
-uint32_t (*transfer_raw)(SSISlave *dev, uint32_t val);
+uint32_t (*transfer_raw)(SSIPeripheral *dev, uint32_t val);
 };
 
-struct SSISlave {
+struct SSIPeripheral {
 DeviceState parent_obj;
 
 /* Chip select state */
 bool cs;
 };
 
-extern const VMStateDescription vmstate_ssi_slave;
+extern const VMStateDescription vmstate_ssi_peripheral;
 
-#define VMSTATE_SSI_SLAVE(_field, _state) {  \
+#define VMSTATE_SSI_PERIPHERAL(_field, _state) { \
 .name   = (stringify(_field)),   \
-.size   = sizeof(SSISlave),