[PATCH v5 3/3] hw/vfio: let read-only flag take effect for mmap'd regions
along side setting host page table to be read-only, the memory regions
are also required to be read-only, so that when guest writes to the
read-only & mmap'd regions, vmexits would happen and region write handlers
are called.
Reviewed-by: Philippe Mathieu-Daudé
Signed-off-by: Yan Zhao
Signed-off-by: Xin Zeng
---
hw/vfio/common.c | 4
1 file changed, 4 insertions(+)
diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 2a4fedfeaa..bf510e66c0 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -980,6 +980,10 @@ int vfio_region_mmap(VFIORegion *region)
name, region->mmaps[i].size,
region->mmaps[i].mmap);
g_free(name);
+
+if (!(region->flags & VFIO_REGION_INFO_FLAG_WRITE)) {
+memory_region_set_readonly(>mmaps[i].mem, true);
+}
memory_region_add_subregion(region->mem, region->mmaps[i].offset,
>mmaps[i].mem);
--
2.17.1
[PATCH v5 2/3] hw/vfio: drop guest writes to ro regions
for vfio regions that are without write permission,
drop guest writes to those regions.
Cc: Philippe Mathieu-Daudé
Reviewed-by: Philippe Mathieu-Daudé
Signed-off-by: Yan Zhao
Signed-off-by: Xin Zeng
---
hw/vfio/common.c | 13 +++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 0b3593b3c0..2a4fedfeaa 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -38,6 +38,7 @@
#include "sysemu/reset.h"
#include "trace.h"
#include "qapi/error.h"
+#include "qemu/log.h"
VFIOGroupList vfio_group_list =
QLIST_HEAD_INITIALIZER(vfio_group_list);
@@ -190,6 +191,16 @@ void vfio_region_write(void *opaque, hwaddr addr,
uint64_t qword;
} buf;
+trace_vfio_region_write(vbasedev->name, region->nr, addr, data, size);
+if (!(region->flags & VFIO_REGION_INFO_FLAG_WRITE)) {
+qemu_log_mask(LOG_GUEST_ERROR,
+ "Invalid write to read only vfio region (%s:region%d"
+ "+0x%"HWADDR_PRIx" size %d)\n", vbasedev->name,
+ region->nr, addr, size);
+
+return;
+}
+
switch (size) {
case 1:
buf.byte = data;
@@ -215,8 +226,6 @@ void vfio_region_write(void *opaque, hwaddr addr,
addr, data, size);
}
-trace_vfio_region_write(vbasedev->name, region->nr, addr, data, size);
-
/*
* A read or write to a BAR always signals an INTx EOI. This will
* do nothing if not pending (including not in INTx mode). We assume
--
2.17.1
[PULL 05/20] qemu-option: Clean up after the previous commit
Signed-off-by: Markus Armbruster
Message-Id: <20200415083048.14339-6-arm...@redhat.com>
Reviewed-by: Eric Blake
---
util/qemu-option.c | 43 +++
1 file changed, 15 insertions(+), 28 deletions(-)
diff --git a/util/qemu-option.c b/util/qemu-option.c
index 2784757ef5..0ebfd97a98 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -965,18 +965,16 @@ void qemu_opts_set_defaults(QemuOptsList *list, const
char *params,
assert(opts);
}
-typedef struct OptsFromQDictState {
-QemuOpts *opts;
-Error **errp;
-} OptsFromQDictState;
-
-static void qemu_opts_from_qdict_1(const char *key, QObject *obj, void *opaque)
+static void qemu_opts_from_qdict_entry(QemuOpts *opts,
+ const QDictEntry *entry,
+ Error **errp)
{
-OptsFromQDictState *state = opaque;
+const char *key = qdict_entry_key(entry);
+QObject *obj = qdict_entry_value(entry);
char buf[32], *tmp = NULL;
const char *value;
-if (!strcmp(key, "id") || *state->errp) {
+if (!strcmp(key, "id")) {
return;
}
@@ -997,7 +995,7 @@ static void qemu_opts_from_qdict_1(const char *key, QObject
*obj, void *opaque)
return;
}
-qemu_opt_set(state->opts, key, value, state->errp);
+qemu_opt_set(opts, key, value, errp);
g_free(tmp);
}
@@ -1010,7 +1008,6 @@ static void qemu_opts_from_qdict_1(const char *key,
QObject *obj, void *opaque)
QemuOpts *qemu_opts_from_qdict(QemuOptsList *list, const QDict *qdict,
Error **errp)
{
-OptsFromQDictState state;
Error *local_err = NULL;
QemuOpts *opts;
const QDictEntry *entry;
@@ -1024,20 +1021,15 @@ QemuOpts *qemu_opts_from_qdict(QemuOptsList *list,
const QDict *qdict,
assert(opts != NULL);
-state.errp = _err;
-state.opts = opts;
-
for (entry = qdict_first(qdict);
entry;
entry = qdict_next(qdict, entry)) {
-qemu_opts_from_qdict_1(qdict_entry_key(entry),
- qdict_entry_value(entry),
- );
-}
-if (local_err) {
-error_propagate(errp, local_err);
-qemu_opts_del(opts);
-return NULL;
+qemu_opts_from_qdict_entry(opts, entry, _err);
+if (local_err) {
+error_propagate(errp, local_err);
+qemu_opts_del(opts);
+return NULL;
+}
}
return opts;
@@ -1056,21 +1048,16 @@ void qemu_opts_absorb_qdict(QemuOpts *opts, QDict
*qdict, Error **errp)
while (entry != NULL) {
Error *local_err = NULL;
-OptsFromQDictState state = {
-.errp = _err,
-.opts = opts,
-};
next = qdict_next(qdict, entry);
if (find_desc_by_name(opts->list->desc, entry->key)) {
-qemu_opts_from_qdict_1(entry->key, entry->value, );
+qemu_opts_from_qdict_entry(opts, entry, _err);
if (local_err) {
error_propagate(errp, local_err);
return;
-} else {
-qdict_del(qdict, entry->key);
}
+qdict_del(qdict, entry->key);
}
entry = next;
--
2.21.1
[PULL 18/20] qom: Simplify object_property_get_enum()
Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Message-Id: <20200424084338.26803-14-arm...@redhat.com> --- qom/object.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/qom/object.c b/qom/object.c index 1812f79224..be700e831f 100644 --- a/qom/object.c +++ b/qom/object.c @@ -1550,11 +1550,9 @@ int object_property_get_enum(Object *obj, const char *name, } visit_complete(v, ); visit_free(v); -v = string_input_visitor_new(str); -visit_type_enum(v, name, , enumprop->lookup, errp); +ret = qapi_enum_parse(enumprop->lookup, str, -1, errp); g_free(str); -visit_free(v); return ret; } -- 2.21.1
[PULL 17/20] qapi: Only input visitors can actually fail
The previous few commits have made this more obvious, and removed the
one exception. Time to clarify the documentation, and drop dead error
checking.
Signed-off-by: Markus Armbruster
Reviewed-by: Eric Blake
Message-Id: <20200424084338.26803-13-arm...@redhat.com>
---
include/qapi/visitor-impl.h | 4
include/qapi/visitor.h | 40 ++---
block.c | 9 +
block/sheepdog.c| 9 +
blockdev.c | 16 ++-
hw/core/machine-hmp-cmds.c | 2 +-
monitor/hmp-cmds.c | 3 ++-
7 files changed, 35 insertions(+), 48 deletions(-)
diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h
index 252206dc0d..98dc533d39 100644
--- a/include/qapi/visitor-impl.h
+++ b/include/qapi/visitor-impl.h
@@ -43,6 +43,10 @@ typedef enum VisitorType {
struct Visitor
{
+/*
+ * Only input visitors may fail!
+ */
+
/* Must be set to visit structs */
void (*start_struct)(Visitor *v, const char *name, void **obj,
size_t size, Error **errp);
diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
index 2d40d2fe0f..5573906966 100644
--- a/include/qapi/visitor.h
+++ b/include/qapi/visitor.h
@@ -82,7 +82,7 @@
* Each function also takes the customary @errp argument (see
* qapi/error.h for details), for reporting any errors (such as if a
* member @name is not present, or is present but not the specified
- * type).
+ * type). Only input visitors can fail.
*
* If an error is detected during visit_type_FOO() with an input
* visitor, then *@obj will be set to NULL for pointer types, and left
@@ -164,19 +164,14 @@
*
*
* Foo *f = ...obtain populated object...
- * Error *err = NULL;
* Visitor *v;
* Type *result;
*
* v = FOO_visitor_new(..., );
- * visit_type_Foo(v, NULL, , );
- * if (err) {
- * ...handle error...
- * } else {
- * visit_complete(v, );
- * ...use result...
- * }
+ * visit_type_Foo(v, NULL, , _abort);
+ * visit_complete(v, );
* visit_free(v);
+ * ...use result...
*
*
* It is also possible to use the visitors to do a virtual walk, where
@@ -289,6 +284,7 @@ void visit_free(Visitor *v);
* case @size is ignored.
*
* On failure, set *@obj to NULL and store an error through @errp.
+ * Can happen only when @v is an input visitor.
*
* After visit_start_struct() succeeds, the caller may visit its
* members one after the other, passing the member's name and address
@@ -305,7 +301,8 @@ void visit_start_struct(Visitor *v, const char *name, void
**obj,
/*
* Prepare for completing an object visit.
*
- * On failure, store an error through @errp.
+ * On failure, store an error through @errp. Can happen only when @v
+ * is an input visitor.
*
* Should be called prior to visit_end_struct() if all other
* intermediate visit steps were successful, to allow the visitor one
@@ -342,6 +339,7 @@ void visit_end_struct(Visitor *v, void **obj);
* ignored.
*
* On failure, set *@list to NULL and store an error through @errp.
+ * Can happen only when @v is an input visitor.
*
* After visit_start_list() succeeds, the caller may visit its members
* one after the other. A real visit (where @list is non-NULL) uses
@@ -375,7 +373,8 @@ GenericList *visit_next_list(Visitor *v, GenericList *tail,
size_t size);
/*
* Prepare for completing a list visit.
*
- * On failure, store an error through @errp.
+ * On failure, store an error through @errp. Can happen only when @v
+ * is an input visitor.
*
* Should be called prior to visit_end_list() if all other
* intermediate visit steps were successful, to allow the visitor one
@@ -411,6 +410,7 @@ void visit_end_list(Visitor *v, void **list);
* (*@obj)->type. Other visitors leave @obj unchanged.
*
* On failure, set *@obj to NULL and store an error through @errp.
+ * Can happen only when @v is an input visitor.
*
* If successful, this must be paired with visit_end_alternate() with
* the same @obj to clean up, even if visiting the contents of the
@@ -465,11 +465,13 @@ bool visit_optional(Visitor *v, const char *name, bool
*present);
* visitors produce text output. The mapping between enumeration
* values and strings is done by the visitor core, using @lookup.
*
- * On failure, store an error through @errp.
+ * On failure, store an error through @errp. Can happen only when @v
+ * is an input visitor.
*
* May call visit_type_str() under the hood, and the enum visit may
* fail even if the corresponding string visit succeeded; this implies
- * that visit_type_str() must have no unwelcome side effects.
+ * that an input visitor's visit_type_str() must have no unwelcome
+ * side effects.
*/
void visit_type_enum(Visitor *v, const char *name, int *obj,
const QEnumLookup *lookup, Error **errp);
@@ -495,7 +497,8 @@ bool visit_is_dealloc(Visitor *v);
* @obj must be non-NULL. Input visitors
[PULL 04/20] qobject: Eliminate qdict_iter(), use qdict_first(), qdict_next()
qdict_iter() has just three uses and no test coverage. Replace by
qdict_first(), qdict_next() for more concise code and less type
punning.
Signed-off-by: Markus Armbruster
Message-Id: <20200415083048.14339-5-arm...@redhat.com>
Reviewed-by: Eric Blake
---
include/qapi/qmp/qdict.h | 3 --
qapi/qobject-input-visitor.c | 21 +++---
qobject/qdict.c | 19 -
qobject/qjson.c | 54 +---
util/qemu-option.c | 10 ++-
5 files changed, 40 insertions(+), 67 deletions(-)
diff --git a/include/qapi/qmp/qdict.h b/include/qapi/qmp/qdict.h
index 7f3ec10a10..da942347a7 100644
--- a/include/qapi/qmp/qdict.h
+++ b/include/qapi/qmp/qdict.h
@@ -40,9 +40,6 @@ void qdict_del(QDict *qdict, const char *key);
int qdict_haskey(const QDict *qdict, const char *key);
QObject *qdict_get(const QDict *qdict, const char *key);
bool qdict_is_equal(const QObject *x, const QObject *y);
-void qdict_iter(const QDict *qdict,
-void (*iter)(const char *key, QObject *obj, void *opaque),
-void *opaque);
const QDictEntry *qdict_first(const QDict *qdict);
const QDictEntry *qdict_next(const QDict *qdict, const QDictEntry *entry);
void qdict_destroy_obj(QObject *obj);
diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c
index 32236cbcb1..5ce3ec2e5f 100644
--- a/qapi/qobject-input-visitor.c
+++ b/qapi/qobject-input-visitor.c
@@ -203,31 +203,32 @@ static const char
*qobject_input_get_keyval(QObjectInputVisitor *qiv,
return qstring_get_str(qstr);
}
-static void qdict_add_key(const char *key, QObject *obj, void *opaque)
-{
-GHashTable *h = opaque;
-g_hash_table_insert(h, (gpointer) key, NULL);
-}
-
static const QListEntry *qobject_input_push(QObjectInputVisitor *qiv,
const char *name,
QObject *obj, void *qapi)
{
GHashTable *h;
StackObject *tos = g_new0(StackObject, 1);
+QDict *qdict = qobject_to(QDict, obj);
+QList *qlist = qobject_to(QList, obj);
+const QDictEntry *entry;
assert(obj);
tos->name = name;
tos->obj = obj;
tos->qapi = qapi;
-if (qobject_type(obj) == QTYPE_QDICT) {
+if (qdict) {
h = g_hash_table_new(g_str_hash, g_str_equal);
-qdict_iter(qobject_to(QDict, obj), qdict_add_key, h);
+for (entry = qdict_first(qdict);
+ entry;
+ entry = qdict_next(qdict, entry)) {
+g_hash_table_insert(h, (void *)qdict_entry_key(entry), NULL);
+}
tos->h = h;
} else {
-assert(qobject_type(obj) == QTYPE_QLIST);
-tos->entry = qlist_first(qobject_to(QList, obj));
+assert(qlist);
+tos->entry = qlist_first(qlist);
tos->index = -1;
}
diff --git a/qobject/qdict.c b/qobject/qdict.c
index 3d8c2f7bbc..526de54ceb 100644
--- a/qobject/qdict.c
+++ b/qobject/qdict.c
@@ -298,25 +298,6 @@ const char *qdict_get_try_str(const QDict *qdict, const
char *key)
return qstr ? qstring_get_str(qstr) : NULL;
}
-/**
- * qdict_iter(): Iterate over all the dictionary's stored values.
- *
- * This function allows the user to provide an iterator, which will be
- * called for each stored value in the dictionary.
- */
-void qdict_iter(const QDict *qdict,
-void (*iter)(const char *key, QObject *obj, void *opaque),
-void *opaque)
-{
-int i;
-QDictEntry *entry;
-
-for (i = 0; i < QDICT_BUCKET_MAX; i++) {
-QLIST_FOREACH(entry, >table[i], next)
-iter(entry->key, entry->value, opaque);
-}
-}
-
static QDictEntry *qdict_next_entry(const QDict *qdict, int first_bucket)
{
int i;
diff --git a/qobject/qjson.c b/qobject/qjson.c
index f0eebc5fda..f1f2c69704 100644
--- a/qobject/qjson.c
+++ b/qobject/qjson.c
@@ -149,14 +149,6 @@ QDict *qdict_from_jsonf_nofail(const char *string, ...)
return qdict;
}
-typedef struct ToJsonIterState
-{
-int indent;
-int pretty;
-int count;
-QString *str;
-} ToJsonIterState;
-
static void to_json(const QObject *obj, QString *str, int pretty, int indent);
static void json_pretty_newline(QString *str, bool pretty, int indent)
@@ -171,26 +163,6 @@ static void json_pretty_newline(QString *str, bool pretty,
int indent)
}
}
-static void to_json_dict_iter(const char *key, QObject *obj, void *opaque)
-{
-ToJsonIterState *s = opaque;
-QString *qkey;
-
-if (s->count) {
-qstring_append(s->str, s->pretty ? "," : ", ");
-}
-
-json_pretty_newline(s->str, s->pretty, s->indent);
-
-qkey = qstring_from_str(key);
-to_json(QOBJECT(qkey), s->str, s->pretty, s->indent);
-qobject_unref(qkey);
-
-qstring_append(s->str, ": ");
-to_json(obj, s->str, s->pretty, s->indent);
-s->count++;
-}
-
static void to_json(const QObject *obj, QString *str, int pretty, int indent)
{
switch
[PULL 20/20] qapi: Generate simpler marshalling code when no arguments
When command FOO has no arguments, its generated qmp_marshal_FOO() is
a bit confusing. Make it simpler:
visit_start_struct(v, NULL, NULL, 0, );
if (err) {
goto out;
}
-
-if (!err) {
-visit_check_struct(v, );
-}
+visit_check_struct(v, );
visit_end_struct(v, NULL);
if (err) {
goto out;
}
Signed-off-by: Markus Armbruster
Message-Id: <20200424084338.26803-16-arm...@redhat.com>
Reviewed-by: Eric Blake
---
scripts/qapi/commands.py | 40
1 file changed, 24 insertions(+), 16 deletions(-)
diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py
index f545903567..6809b0fb6e 100644
--- a/scripts/qapi/commands.py
+++ b/scripts/qapi/commands.py
@@ -115,14 +115,10 @@ def gen_marshal(name, arg_type, boxed, ret_type):
c_type=ret_type.c_type())
if have_args:
-visit_members = ('visit_type_%s_members(v, , );'
- % arg_type.c_name())
ret += mcgen('''
%(c_name)s arg = {0};
''',
c_name=arg_type.c_name())
-else:
-visit_members = ''
ret += mcgen('''
@@ -131,16 +127,27 @@ def gen_marshal(name, arg_type, boxed, ret_type):
if (err) {
goto out;
}
-%(visit_members)s
+''')
+
+if have_args:
+ret += mcgen('''
+visit_type_%(c_arg_type)s_members(v, , );
if (!err) {
visit_check_struct(v, );
}
+''',
+ c_arg_type=arg_type.c_name())
+else:
+ret += mcgen('''
+visit_check_struct(v, );
+''')
+
+ret += mcgen('''
visit_end_struct(v, NULL);
if (err) {
goto out;
}
-''',
- visit_members=visit_members)
+''')
ret += gen_call(name, arg_type, boxed, ret_type)
@@ -151,20 +158,21 @@ out:
visit_free(v);
''')
-if have_args:
-visit_members = ('visit_type_%s_members(v, , NULL);'
- % arg_type.c_name())
-else:
-visit_members = ''
-
ret += mcgen('''
v = qapi_dealloc_visitor_new();
visit_start_struct(v, NULL, NULL, 0, NULL);
-%(visit_members)s
+''')
+
+if have_args:
+ret += mcgen('''
+visit_type_%(c_arg_type)s_members(v, , NULL);
+''',
+ c_arg_type=arg_type.c_name())
+
+ret += mcgen('''
visit_end_struct(v, NULL);
visit_free(v);
-''',
- visit_members=visit_members)
+''')
ret += mcgen('''
}
--
2.21.1
[PULL 19/20] qapi: Disallow qmp_marshal_FOO(NULL, ...)
For QMP commands without arguments, gen_marshal() laboriously
generates a qmp_marshal_FOO() that copes with null @args. Turns
there's just one caller that passes null instead of an empty QDict.
Adjust that caller, and simplify gen_marshal().
Signed-off-by: Markus Armbruster
Message-Id: <20200424084338.26803-15-arm...@redhat.com>
Reviewed-by: Eric Blake
---
docs/devel/qapi-code-gen.txt | 2 +-
monitor/qmp.c| 5 -
scripts/qapi/commands.py | 26 ++
3 files changed, 7 insertions(+), 26 deletions(-)
diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt
index c6dd1891c3..a7794ef658 100644
--- a/docs/devel/qapi-code-gen.txt
+++ b/docs/devel/qapi-code-gen.txt
@@ -1579,8 +1579,8 @@ Example:
void qmp_marshal_my_command(QDict *args, QObject **ret, Error **errp)
{
Error *err = NULL;
-UserDefOne *retval;
Visitor *v;
+UserDefOne *retval;
q_obj_my_command_arg arg = {0};
v = qobject_input_visitor_new(QOBJECT(args));
diff --git a/monitor/qmp.c b/monitor/qmp.c
index f89e7daf27..d433ceae5b 100644
--- a/monitor/qmp.c
+++ b/monitor/qmp.c
@@ -322,9 +322,12 @@ static QDict *qmp_greeting(MonitorQMP *mon)
{
QList *cap_list = qlist_new();
QObject *ver = NULL;
+QDict *args;
QMPCapability cap;
-qmp_marshal_query_version(NULL, , NULL);
+args = qdict_new();
+qmp_marshal_query_version(args, , NULL);
+qobject_unref(args);
for (cap = 0; cap < QMP_CAPABILITY__MAX; cap++) {
if (mon->capab_offered[cap]) {
diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py
index bc30876c88..f545903567 100644
--- a/scripts/qapi/commands.py
+++ b/scripts/qapi/commands.py
@@ -104,6 +104,7 @@ def gen_marshal(name, arg_type, boxed, ret_type):
%(proto)s
{
Error *err = NULL;
+Visitor *v;
''',
proto=build_marshal_proto(name))
@@ -117,21 +118,14 @@ def gen_marshal(name, arg_type, boxed, ret_type):
visit_members = ('visit_type_%s_members(v, , );'
% arg_type.c_name())
ret += mcgen('''
-Visitor *v;
%(c_name)s arg = {0};
-
''',
c_name=arg_type.c_name())
else:
visit_members = ''
-ret += mcgen('''
-Visitor *v = NULL;
-
-if (args) {
-''')
-push_indent()
ret += mcgen('''
+
v = qobject_input_visitor_new(QOBJECT(args));
visit_start_struct(v, NULL, NULL, 0, );
if (err) {
@@ -148,12 +142,6 @@ def gen_marshal(name, arg_type, boxed, ret_type):
''',
visit_members=visit_members)
-if not have_args:
-pop_indent()
-ret += mcgen('''
-}
-''')
-
ret += gen_call(name, arg_type, boxed, ret_type)
ret += mcgen('''
@@ -168,10 +156,6 @@ out:
% arg_type.c_name())
else:
visit_members = ''
-ret += mcgen('''
-if (args) {
-''')
-push_indent()
ret += mcgen('''
v = qapi_dealloc_visitor_new();
@@ -182,12 +166,6 @@ out:
''',
visit_members=visit_members)
-if not have_args:
-pop_indent()
-ret += mcgen('''
-}
-''')
-
ret += mcgen('''
}
''')
--
2.21.1
Re: [PATCH v2 06/17] block/io: support int64_t bytes in bdrv_aligned_pwritev()
30.04.2020 8:30, Vladimir Sementsov-Ogievskiy wrote:
30.04.2020 8:25, Vladimir Sementsov-Ogievskiy wrote:
30.04.2020 1:04, Eric Blake wrote:
On 4/27/20 3:23 AM, Vladimir Sementsov-Ogievskiy wrote:
We are generally moving to int64_t for both offset and bytes parameters
on all io paths. Prepare bdrv_aligned_pwritev() now (and convert the
dependencies: bdrv_co_write_req_prepare() and
bdrv_co_write_req_finish() to signed type bytes)
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
block/io.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/block/io.c b/block/io.c
index c8c30e3699..fe19e09034 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1854,7 +1854,7 @@ fail:
}
static inline int coroutine_fn
-bdrv_co_write_req_prepare(BdrvChild *child, int64_t offset, uint64_t bytes,
+bdrv_co_write_req_prepare(BdrvChild *child, int64_t offset, int64_t bytes,
BdrvTrackedRequest *req, int flags)
{
No change in size. First, check usage within function:
int64_t end_sector = DIV_ROUND_UP(offset + bytes, BDRV_SECTOR_SIZE);
Changes computation from uint64_t to int64_t. This causes a borderline bug on
images between INT64_MAX-511 and INT64_MAX (nbdkit can produce such images over
NBD, although they are atypical on disk), where DIV_ROUND_UP() would give the
right answer as uint64_t but a negative answer with int64_t. As those images
are not sector-aligned, maybe we don't need to care?
all other uses appear to be within asserts related to offset+bytes being
positive, so that's what we should check for.
Callers:
bdrv_aligned_pwritev() - changed in this patch to 'int64_t', analyzed below [1]
bdrv_co_pdiscard() - already passes 'int64_t', also checks for offset+bytes
overflow - safe
bdrv_co_copy_range_internal() - 'uint64_t', but already analyzed for 3/17 how it
was capped < 2M - safe
bdrv_co_truncate() - already passes 'int64_t', passes new_bytes computed by
subtracting from a positive 'int64_t offset' - safe
[1] except I hit the end of my work day, so my analysis will have to continue
tomorrow...
Thanks for reviewing!
I'm very sorry, I just need to say once again: the series should be rebased on
"[PATCH for-5.0? 0/9] block/io: safer inc/dec in_flight sections", as it is
already mostly reviewed by Stefan. Seems, that your analysis will be still valid after
it, although patches will change. I'll do it now to see, can I keep your r-b's.
I mean "[PATCH v2 0/9] block/io: safer inc/dec in_flight sections" of course
https://lists.gnu.org/archive/html/qemu-devel/2020-04/msg04559.html
Cool! Exactly up to this patch (inclusive) it rebases without conflicts. And
the next patch (and may be further) are conflicting. I'll finish rebasing and
resend.
--
Best regards,
Vladimir
[PULL 06/20] qapi: Belatedly update visitor.h's big comment for QAPI modules
Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Message-Id: <20200424084338.26803-2-arm...@redhat.com> --- include/qapi/visitor.h | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h index c5b23851a1..f8a0fc1ea9 100644 --- a/include/qapi/visitor.h +++ b/include/qapi/visitor.h @@ -58,7 +58,7 @@ * * where T is FOO for scalar types, and FOO * otherwise. The scalar * visitors are declared here; the remaining visitors are generated in - * qapi-visit.h. + * qapi-visit-MODULE.h. * * The @name parameter of visit_type_FOO() describes the relation * between this QAPI value and its parent container. When visiting @@ -86,16 +86,16 @@ * by manual construction. * * For the QAPI object types (structs, unions, and alternates), there - * is an additional generated function in qapi-visit.h compatible - * with: + * is an additional generated function in qapi-visit-MODULE.h + * compatible with: * * void visit_type_FOO_members(Visitor *v, FOO *obj, Error **errp); * * for visiting the members of a type without also allocating the QAPI * struct. * - * Additionally, in qapi-types.h, all QAPI pointer types (structs, - * unions, alternates, and lists) have a generated function compatible + * Additionally, QAPI pointer types (structs, unions, alternates, and + * lists) have a generated function in qapi-types-MODULE.h compatible * with: * * void qapi_free_FOO(FOO *obj); -- 2.21.1
[PULL 07/20] qapi: Fix the virtual walk example in visitor.h's big comment
Call visit_check_list(). Missed in commit a4a1c70dc7 "qapi: Make
input visitors detect unvisited list tails".
Drop an irrelevant error_propagate() while there.
Signed-off-by: Markus Armbruster
Reviewed-by: Eric Blake
Message-Id: <20200424084338.26803-3-arm...@redhat.com>
---
include/qapi/visitor.h | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
index f8a0fc1ea9..7f63e4c381 100644
--- a/include/qapi/visitor.h
+++ b/include/qapi/visitor.h
@@ -215,6 +215,9 @@
* goto outlist;
* }
* outlist:
+ * if (!err) {
+ * visit_check_list(v, );
+ * }
* visit_end_list(v, NULL);
* if (!err) {
* visit_check_struct(v, );
@@ -222,7 +225,6 @@
* outobj:
* visit_end_struct(v, NULL);
* out:
- * error_propagate(errp, err);
* visit_free(v);
*
*/
--
2.21.1
[PULL 16/20] qapi: Assert non-input visitors see only valid alternate tags
An alternate type's visit_type_FOO() fails when it runs into an invalid ->type. This is appropriate with an input visitor: visit_start_alternate() sets ->type according to the input, and bad input can lead to bad ->type. It should never happen with an output, clone or dealloc visitor: if it did, the alternate being output, cloned or deallocated would be messed up beyond repair. Assert that. Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Message-Id: <20200424084338.26803-12-arm...@redhat.com> --- scripts/qapi/visit.py | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py index 678109dfb5..d5d7a1031f 100644 --- a/scripts/qapi/visit.py +++ b/scripts/qapi/visit.py @@ -232,6 +232,7 @@ void visit_type_%(c_name)s(Visitor *v, const char *name, %(c_name)s **obj, Error case QTYPE_NONE: abort(); default: +assert(visit_is_input(v)); error_setg(, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "%(name)s"); /* Avoid passing invalid *obj to qapi_free_%(c_name)s() */ -- 2.21.1
[PULL 15/20] qapi: Clean up visitor's recovery from input with invalid type
An alternate type's visit_type_FOO() fails when it runs into an invalid ->type. If it's an input visit, we then need to free the the object we got from visit_start_alternate(). We do that with qapi_free_FOO(), which uses the dealloc visitor. Trouble is that object is in a bad state: its ->type is invalid. So the dealloc visitor will run into the same error again, and the error recovery skips deallocating the alternate's (invalid) alternative. Works, because qapi_free_FOO() ignores the error. Avoid it instead: free the messed up object with by g_free(). Signed-off-by: Markus Armbruster Message-Id: <20200424084338.26803-11-arm...@redhat.com> Reviewed-by: Eric Blake --- scripts/qapi/visit.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py index e3467b770b..678109dfb5 100644 --- a/scripts/qapi/visit.py +++ b/scripts/qapi/visit.py @@ -234,6 +234,9 @@ void visit_type_%(c_name)s(Visitor *v, const char *name, %(c_name)s **obj, Error default: error_setg(, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "%(name)s"); +/* Avoid passing invalid *obj to qapi_free_%(c_name)s() */ +g_free(*obj); +*obj = NULL; } out_obj: visit_end_alternate(v, (void **)obj); -- 2.21.1
[PULL 08/20] qapi: Fix typo in visit_start_list()'s contract
Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Message-Id: <20200424084338.26803-4-arm...@redhat.com> --- include/qapi/visitor.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h index 7f63e4c381..c5d0ce9184 100644 --- a/include/qapi/visitor.h +++ b/include/qapi/visitor.h @@ -345,9 +345,9 @@ void visit_end_struct(Visitor *v, void **obj); * input visitors set *@list to NULL. * * After visit_start_list() succeeds, the caller may visit its members - * one after the other. A real visit (where @obj is non-NULL) uses + * one after the other. A real visit (where @list is non-NULL) uses * visit_next_list() for traversing the linked list, while a virtual - * visit (where @obj is NULL) uses other means. For each list + * visit (where @list is NULL) uses other means. For each list * element, call the appropriate visit_type_FOO() with name set to * NULL and obj set to the address of the value member of the list * element. Finally, visit_end_list() needs to be called with the -- 2.21.1
[PULL 11/20] qapi: Assert incomplete object occurs only in dealloc visitor
Signed-off-by: Markus Armbruster
Reviewed-by: Eric Blake
Message-Id: <20200424084338.26803-7-arm...@redhat.com>
---
docs/devel/qapi-code-gen.txt | 2 ++
include/qapi/visitor.h | 5 +
qapi/qapi-visit-core.c | 5 +
scripts/qapi/visit.py| 4
4 files changed, 16 insertions(+)
diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt
index 1967adfa92..c6dd1891c3 100644
--- a/docs/devel/qapi-code-gen.txt
+++ b/docs/devel/qapi-code-gen.txt
@@ -1446,6 +1446,8 @@ Example:
goto out;
}
if (!*obj) {
+/* incomplete */
+assert(visit_is_dealloc(v));
goto out_obj;
}
visit_type_UserDefOne_members(v, *obj, );
diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
index a425ea514c..2d40d2fe0f 100644
--- a/include/qapi/visitor.h
+++ b/include/qapi/visitor.h
@@ -479,6 +479,11 @@ void visit_type_enum(Visitor *v, const char *name, int
*obj,
*/
bool visit_is_input(Visitor *v);
+/*
+ * Check if visitor is a dealloc visitor.
+ */
+bool visit_is_dealloc(Visitor *v);
+
/*** Visiting built-in types ***/
/*
diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index 5365561b07..d4aac206cf 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -142,6 +142,11 @@ bool visit_is_input(Visitor *v)
return v->type == VISITOR_INPUT;
}
+bool visit_is_dealloc(Visitor *v)
+{
+return v->type == VISITOR_DEALLOC;
+}
+
void visit_type_int(Visitor *v, const char *name, int64_t *obj, Error **errp)
{
assert(obj);
diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py
index 23d9194aa4..e3467b770b 100644
--- a/scripts/qapi/visit.py
+++ b/scripts/qapi/visit.py
@@ -189,6 +189,8 @@ void visit_type_%(c_name)s(Visitor *v, const char *name,
%(c_name)s **obj, Error
goto out;
}
if (!*obj) {
+/* incomplete */
+assert(visit_is_dealloc(v));
goto out_obj;
}
switch ((*obj)->type) {
@@ -260,6 +262,8 @@ void visit_type_%(c_name)s(Visitor *v, const char *name,
%(c_name)s **obj, Error
goto out;
}
if (!*obj) {
+/* incomplete */
+assert(visit_is_dealloc(v));
goto out_obj;
}
visit_type_%(c_name)s_members(v, *obj, );
--
2.21.1
[PULL 14/20] qapi: Assert non-input visitors see only valid narrow integers
visit_type_intN() and visit_type_uintN() fail when the value is out of
bounds.
This is appropriate with an input visitor: the value comes from input,
and input may be bad.
It should never happen with the other visitors: the value comes from
the caller, and callers must keep it within bounds. Assert that.
Signed-off-by: Markus Armbruster
Reviewed-by: Eric Blake
Message-Id: <20200424084338.26803-10-arm...@redhat.com>
---
qapi/qapi-visit-core.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index 80ca83bcb9..74aa9c04bd 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -160,10 +160,13 @@ static void visit_type_uintN(Visitor *v, uint64_t *obj,
const char *name,
Error *err = NULL;
uint64_t value = *obj;
+assert(v->type == VISITOR_INPUT || value <= max);
+
v->type_uint64(v, name, , );
if (err) {
error_propagate(errp, err);
} else if (value > max) {
+assert(v->type == VISITOR_INPUT);
error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
name ? name : "null", type);
} else {
@@ -219,10 +222,13 @@ static void visit_type_intN(Visitor *v, int64_t *obj,
const char *name,
Error *err = NULL;
int64_t value = *obj;
+assert(v->type == VISITOR_INPUT || (value >= min && value <= max));
+
v->type_int64(v, name, , );
if (err) {
error_propagate(errp, err);
} else if (value < min || value > max) {
+assert(v->type == VISITOR_INPUT);
error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
name ? name : "null", type);
} else {
--
2.21.1
[PULL 13/20] qapi: Assert output visitors see only valid enum values
output_type_enum() fails when *obj is not a valid value of the enum
type. Should not happen. Drop the check, along with its unit tests.
This unmasks qapi_enum_lookup()'s assertion.
Signed-off-by: Markus Armbruster
Reviewed-by: Eric Blake
Message-Id: <20200424084338.26803-9-arm...@redhat.com>
[Commit message tweaked]
---
qapi/qapi-visit-core.c | 9 ---
tests/test-qobject-output-visitor.c | 39 -
tests/test-string-output-visitor.c | 19 --
3 files changed, 67 deletions(-)
diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index d4aac206cf..80ca83bcb9 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -341,15 +341,6 @@ static void output_type_enum(Visitor *v, const char *name,
int *obj,
int value = *obj;
char *enum_str;
-/*
- * TODO why is this an error, not an assertion? If assertion:
- * delete, and rely on qapi_enum_lookup()
- */
-if (value < 0 || value >= lookup->size) {
-error_setg(errp, QERR_INVALID_PARAMETER, name ? name : "null");
-return;
-}
-
enum_str = (char *)qapi_enum_lookup(lookup, value);
visit_type_str(v, name, _str, errp);
}
diff --git a/tests/test-qobject-output-visitor.c
b/tests/test-qobject-output-visitor.c
index d7761ebf84..1c856d9bd2 100644
--- a/tests/test-qobject-output-visitor.c
+++ b/tests/test-qobject-output-visitor.c
@@ -141,21 +141,6 @@ static void test_visitor_out_enum(TestOutputVisitorData
*data,
}
}
-static void test_visitor_out_enum_errors(TestOutputVisitorData *data,
- const void *unused)
-{
-EnumOne i, bad_values[] = { ENUM_ONE__MAX, -1 };
-
-for (i = 0; i < ARRAY_SIZE(bad_values) ; i++) {
-Error *err = NULL;
-
-visit_type_EnumOne(data->ov, "unused", _values[i], );
-error_free_or_abort();
-visitor_reset(data);
-}
-}
-
-
static void test_visitor_out_struct(TestOutputVisitorData *data,
const void *unused)
{
@@ -234,26 +219,6 @@ static void
test_visitor_out_struct_nested(TestOutputVisitorData *data,
qapi_free_UserDefTwo(ud2);
}
-static void test_visitor_out_struct_errors(TestOutputVisitorData *data,
- const void *unused)
-{
-EnumOne bad_values[] = { ENUM_ONE__MAX, -1 };
-UserDefOne u = {0};
-UserDefOne *pu =
-int i;
-
-for (i = 0; i < ARRAY_SIZE(bad_values) ; i++) {
-Error *err = NULL;
-
-u.has_enum1 = true;
-u.enum1 = bad_values[i];
-visit_type_UserDefOne(data->ov, "unused", , );
-error_free_or_abort();
-visitor_reset(data);
-}
-}
-
-
static void test_visitor_out_list(TestOutputVisitorData *data,
const void *unused)
{
@@ -821,14 +786,10 @@ int main(int argc, char **argv)
_visitor_data, test_visitor_out_no_string);
output_visitor_test_add("/visitor/output/enum",
_visitor_data, test_visitor_out_enum);
-output_visitor_test_add("/visitor/output/enum-errors",
-_visitor_data, test_visitor_out_enum_errors);
output_visitor_test_add("/visitor/output/struct",
_visitor_data, test_visitor_out_struct);
output_visitor_test_add("/visitor/output/struct-nested",
_visitor_data, test_visitor_out_struct_nested);
-output_visitor_test_add("/visitor/output/struct-errors",
-_visitor_data, test_visitor_out_struct_errors);
output_visitor_test_add("/visitor/output/list",
_visitor_data, test_visitor_out_list);
output_visitor_test_add("/visitor/output/any",
diff --git a/tests/test-string-output-visitor.c
b/tests/test-string-output-visitor.c
index 1be1540767..3bd73c 100644
--- a/tests/test-string-output-visitor.c
+++ b/tests/test-string-output-visitor.c
@@ -203,19 +203,6 @@ static void test_visitor_out_enum(TestOutputVisitorData
*data,
}
}
-static void test_visitor_out_enum_errors(TestOutputVisitorData *data,
- const void *unused)
-{
-EnumOne i, bad_values[] = { ENUM_ONE__MAX, -1 };
-
-for (i = 0; i < ARRAY_SIZE(bad_values) ; i++) {
-Error *err = NULL;
-
-visit_type_EnumOne(data->ov, "unused", _values[i], );
-error_free_or_abort();
-}
-}
-
static void
output_visitor_test_add(const char *testpath,
TestOutputVisitorData *data,
@@ -260,12 +247,6 @@ int main(int argc, char **argv)
_visitor_data, test_visitor_out_enum, false);
output_visitor_test_add("/string-visitor/output/enum-human",
_visitor_data, test_visitor_out_enum, true);
-output_visitor_test_add("/string-visitor/output/enum-errors",
-_visitor_data,
[PULL 02/20] qobject: Factor out helper json_pretty_newline()
Signed-off-by: Markus Armbruster
Message-Id: <20200415083048.14339-3-arm...@redhat.com>
Reviewed-by: Eric Blake
[Coding style in moved code tidied up]
---
qobject/qjson.c | 40
1 file changed, 16 insertions(+), 24 deletions(-)
diff --git a/qobject/qjson.c b/qobject/qjson.c
index db36101f3b..87422f600d 100644
--- a/qobject/qjson.c
+++ b/qobject/qjson.c
@@ -159,21 +159,28 @@ typedef struct ToJsonIterState
static void to_json(const QObject *obj, QString *str, int pretty, int indent);
+static void json_pretty_newline(QString *str, bool pretty, int indent)
+{
+int i;
+
+if (pretty) {
+qstring_append(str, "\n");
+for (i = 0; i < indent; i++) {
+qstring_append(str, "");
+}
+}
+}
+
static void to_json_dict_iter(const char *key, QObject *obj, void *opaque)
{
ToJsonIterState *s = opaque;
QString *qkey;
-int j;
if (s->count) {
qstring_append(s->str, s->pretty ? "," : ", ");
}
-if (s->pretty) {
-qstring_append(s->str, "\n");
-for (j = 0 ; j < s->indent ; j++)
-qstring_append(s->str, "");
-}
+json_pretty_newline(s->str, s->pretty, s->indent);
qkey = qstring_from_str(key);
to_json(QOBJECT(qkey), s->str, s->pretty, s->indent);
@@ -187,17 +194,12 @@ static void to_json_dict_iter(const char *key, QObject
*obj, void *opaque)
static void to_json_list_iter(QObject *obj, void *opaque)
{
ToJsonIterState *s = opaque;
-int j;
if (s->count) {
qstring_append(s->str, s->pretty ? "," : ", ");
}
-if (s->pretty) {
-qstring_append(s->str, "\n");
-for (j = 0 ; j < s->indent ; j++)
-qstring_append(s->str, "");
-}
+json_pretty_newline(s->str, s->pretty, s->indent);
to_json(obj, s->str, s->pretty, s->indent);
s->count++;
@@ -282,12 +284,7 @@ static void to_json(const QObject *obj, QString *str, int
pretty, int indent)
s.pretty = pretty;
qstring_append(str, "{");
qdict_iter(val, to_json_dict_iter, );
-if (pretty) {
-int j;
-qstring_append(str, "\n");
-for (j = 0 ; j < indent ; j++)
-qstring_append(str, "");
-}
+json_pretty_newline(str, pretty, indent);
qstring_append(str, "}");
break;
}
@@ -301,12 +298,7 @@ static void to_json(const QObject *obj, QString *str, int
pretty, int indent)
s.pretty = pretty;
qstring_append(str, "[");
qlist_iter(val, (void *)to_json_list_iter, );
-if (pretty) {
-int j;
-qstring_append(str, "\n");
-for (j = 0 ; j < indent ; j++)
-qstring_append(str, "");
-}
+json_pretty_newline(str, pretty, indent);
qstring_append(str, "]");
break;
}
--
2.21.1
[PULL 09/20] qapi: Document @errp usage more thoroughly in visitor.h
Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Message-Id: <20200424084338.26803-5-arm...@redhat.com> --- include/qapi/visitor.h | 37 +++-- 1 file changed, 23 insertions(+), 14 deletions(-) diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h index c5d0ce9184..09df7099c6 100644 --- a/include/qapi/visitor.h +++ b/include/qapi/visitor.h @@ -284,9 +284,7 @@ void visit_free(Visitor *v); * into *@obj. @obj may also be NULL for a virtual walk, in which * case @size is ignored. * - * @errp obeys typical error usage, and reports failures such as a - * member @name is not present, or present but not an object. On - * error, input visitors set *@obj to NULL. + * On failure, set *@obj to NULL and store an error through @errp. * * After visit_start_struct() succeeds, the caller may visit its * members one after the other, passing the member's name and address @@ -303,8 +301,7 @@ void visit_start_struct(Visitor *v, const char *name, void **obj, /* * Prepare for completing an object visit. * - * @errp obeys typical error usage, and reports failures such as - * unparsed keys remaining in the input stream. + * On failure, store an error through @errp. * * Should be called prior to visit_end_struct() if all other * intermediate visit steps were successful, to allow the visitor one @@ -340,9 +337,7 @@ void visit_end_struct(Visitor *v, void **obj); * allow @list to be NULL for a virtual walk, in which case @size is * ignored. * - * @errp obeys typical error usage, and reports failures such as a - * member @name is not present, or present but not a list. On error, - * input visitors set *@list to NULL. + * On failure, set *@list to NULL and store an error through @errp. * * After visit_start_list() succeeds, the caller may visit its members * one after the other. A real visit (where @list is non-NULL) uses @@ -376,8 +371,7 @@ GenericList *visit_next_list(Visitor *v, GenericList *tail, size_t size); /* * Prepare for completing a list visit. * - * @errp obeys typical error usage, and reports failures such as - * unvisited list tail remaining in the input stream. + * On failure, store an error through @errp. * * Should be called prior to visit_end_list() if all other * intermediate visit steps were successful, to allow the visitor one @@ -409,8 +403,10 @@ void visit_end_list(Visitor *v, void **list); * * @obj must not be NULL. Input and clone visitors use @size to * determine how much memory to allocate into *@obj, then determine - * the qtype of the next thing to be visited, stored in (*@obj)->type. - * Other visitors will leave @obj unchanged. + * the qtype of the next thing to be visited, and store it in + * (*@obj)->type. Other visitors leave @obj unchanged. + * + * On failure, set *@obj to NULL and store an error through @errp. * * If successful, this must be paired with visit_end_alternate() with * the same @obj to clean up, even if visiting the contents of the @@ -463,8 +459,9 @@ bool visit_optional(Visitor *v, const char *name, bool *present); * * Currently, all input visitors parse text input, and all output * visitors produce text output. The mapping between enumeration - * values and strings is done by the visitor core, using @strings; it - * should be the ENUM_lookup array from visit-types.h. + * values and strings is done by the visitor core, using @lookup. + * + * On failure, store an error through @errp. * * May call visit_type_str() under the hood, and the enum visit may * fail even if the corresponding string visit succeeded; this implies @@ -488,6 +485,8 @@ bool visit_is_input(Visitor *v); * * @obj must be non-NULL. Input visitors set *@obj to the value; * other visitors will leave *@obj unchanged. + * + * On failure, store an error through @errp. */ void visit_type_int(Visitor *v, const char *name, int64_t *obj, Error **errp); @@ -564,6 +563,8 @@ void visit_type_size(Visitor *v, const char *name, uint64_t *obj, * * @obj must be non-NULL. Input visitors set *@obj to the value; * other visitors will leave *@obj unchanged. + * + * On failure, store an error through @errp. */ void visit_type_bool(Visitor *v, const char *name, bool *obj, Error **errp); @@ -581,6 +582,8 @@ void visit_type_bool(Visitor *v, const char *name, bool *obj, Error **errp); * It is safe to cast away const when preparing a (const char *) value * into @obj for use by an output visitor. * + * On failure, set *@obj to NULL and store an error through @errp. + * * FIXME: Callers that try to output NULL *obj should not be allowed. */ void visit_type_str(Visitor *v, const char *name, char **obj, Error **errp); @@ -594,6 +597,8 @@ void visit_type_str(Visitor *v, const char *name, char **obj, Error **errp); * @obj must be non-NULL. Input visitors set *@obj to the value; * other visitors will leave *@obj unchanged. Visitors should * document if infinity or NaN are not
[PULL 12/20] qapi: Fix Visitor contract for start_alternate()
The contract demands v->start_alternate() for input and dealloc
visitors, but visit_start_alternate() actually requires it for input
and clone visitors. Fix the contract, and delete superfluous
qapi_dealloc_start_alternate().
Signed-off-by: Markus Armbruster
Reviewed-by: Eric Blake
Message-Id: <20200424084338.26803-8-arm...@redhat.com>
---
include/qapi/visitor-impl.h | 5 ++---
qapi/qapi-dealloc-visitor.c | 7 ---
2 files changed, 2 insertions(+), 10 deletions(-)
diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h
index 8ccb3b6c20..252206dc0d 100644
--- a/include/qapi/visitor-impl.h
+++ b/include/qapi/visitor-impl.h
@@ -67,13 +67,12 @@ struct Visitor
/* Must be set */
void (*end_list)(Visitor *v, void **list);
-/* Must be set by input and dealloc visitors to visit alternates;
- * optional for output visitors. */
+/* Must be set by input and clone visitors to visit alternates */
void (*start_alternate)(Visitor *v, const char *name,
GenericAlternate **obj, size_t size,
Error **errp);
-/* Optional, needed for dealloc visitor */
+/* Optional */
void (*end_alternate)(Visitor *v, void **obj);
/* Must be set */
diff --git a/qapi/qapi-dealloc-visitor.c b/qapi/qapi-dealloc-visitor.c
index d192724b13..2239fc6417 100644
--- a/qapi/qapi-dealloc-visitor.c
+++ b/qapi/qapi-dealloc-visitor.c
@@ -34,12 +34,6 @@ static void qapi_dealloc_end_struct(Visitor *v, void **obj)
}
}
-static void qapi_dealloc_start_alternate(Visitor *v, const char *name,
- GenericAlternate **obj, size_t size,
- Error **errp)
-{
-}
-
static void qapi_dealloc_end_alternate(Visitor *v, void **obj)
{
if (obj) {
@@ -123,7 +117,6 @@ Visitor *qapi_dealloc_visitor_new(void)
v->visitor.type = VISITOR_DEALLOC;
v->visitor.start_struct = qapi_dealloc_start_struct;
v->visitor.end_struct = qapi_dealloc_end_struct;
-v->visitor.start_alternate = qapi_dealloc_start_alternate;
v->visitor.end_alternate = qapi_dealloc_end_alternate;
v->visitor.start_list = qapi_dealloc_start_list;
v->visitor.next_list = qapi_dealloc_next_list;
--
2.21.1
[PULL 10/20] qapi: Polish prose in visitor.h
Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Message-Id: <20200424084338.26803-6-arm...@redhat.com> --- include/qapi/visitor.h | 104 + 1 file changed, 54 insertions(+), 50 deletions(-) diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h index 09df7099c6..a425ea514c 100644 --- a/include/qapi/visitor.h +++ b/include/qapi/visitor.h @@ -25,19 +25,21 @@ * for doing work at each node of a QAPI graph; it can also be used * for a virtual walk, where there is no actual QAPI C struct. * - * There are four kinds of visitor classes: input visitors (QObject, - * string, and QemuOpts) parse an external representation and build - * the corresponding QAPI graph, output visitors (QObject and string) take - * a completed QAPI graph and generate an external representation, the - * dealloc visitor can take a QAPI graph (possibly partially - * constructed) and recursively free its resources, and the clone - * visitor performs a deep clone of one QAPI object to another. While - * the dealloc and QObject input/output visitors are general, the string, - * QemuOpts, and clone visitors have some implementation limitations; - * see the documentation for each visitor for more details on what it - * supports. Also, see visitor-impl.h for the callback contracts - * implemented by each visitor, and docs/devel/qapi-code-gen.txt for more - * about the QAPI code generator. + * There are four kinds of visitors: input visitors (QObject, string, + * and QemuOpts) parse an external representation and build the + * corresponding QAPI object, output visitors (QObject and string) + * take a QAPI object and generate an external representation, the + * dealloc visitor takes a QAPI object (possibly partially + * constructed) and recursively frees it, and the clone visitor + * performs a deep clone of a QAPI object. + * + * While the dealloc and QObject input/output visitors are general, + * the string, QemuOpts, and clone visitors have some implementation + * limitations; see the documentation for each visitor for more + * details on what it supports. Also, see visitor-impl.h for the + * callback contracts implemented by each visitor, and + * docs/devel/qapi-code-gen.txt for more about the QAPI code + * generator. * * All of the visitors are created via: * @@ -45,11 +47,15 @@ * * A visitor should be used for exactly one top-level visit_type_FOO() * or virtual walk; if that is successful, the caller can optionally - * call visit_complete() (for now, useful only for output visits, but - * safe to call on all visits). Then, regardless of success or - * failure, the user should call visit_free() to clean up resources. - * It is okay to free the visitor without completing the visit, if - * some other error is detected in the meantime. + * call visit_complete() (useful only for output visits, but safe to + * call on all visits). Then, regardless of success or failure, the + * user should call visit_free() to clean up resources. It is okay to + * free the visitor without completing the visit, if some other error + * is detected in the meantime. + * + * The clone and dealloc visitor should not be used directly outside + * of QAPI code. Use the qapi_free_FOO() and QAPI_CLONE() instead, + * described below. * * All QAPI types have a corresponding function with a signature * roughly compatible with this: @@ -68,22 +74,26 @@ * alternate, @name should equal the name used for visiting the * alternate. * - * The visit_type_FOO() functions expect a non-null @obj argument; - * they allocate *@obj during input visits, leave it unchanged on - * output visits, and recursively free any resources during a dealloc - * visit. Each function also takes the customary @errp argument (see + * The visit_type_FOO() functions take a non-null @obj argument; they + * allocate *@obj during input visits, leave it unchanged during + * output and clone visits, and free it (recursively) during a dealloc + * visit. + * + * Each function also takes the customary @errp argument (see * qapi/error.h for details), for reporting any errors (such as if a * member @name is not present, or is present but not the specified * type). * * If an error is detected during visit_type_FOO() with an input - * visitor, then *@obj will be NULL for pointer types, and left - * unchanged for scalar types. Using an output or clone visitor with - * an incomplete object has undefined behavior (other than a special - * case for visit_type_str() treating NULL like ""), while the dealloc - * visitor safely handles incomplete objects. Since input visitors - * never produce an incomplete object, such an object is possible only - * by manual construction. + * visitor, then *@obj will be set to NULL for pointer types, and left + * unchanged for scalar types. + * + * Using an output or clone visitor with an incomplete object has + * undefined behavior (other than a special case for visit_type_str() + *
[PULL 00/20] QAPI patches for 2020-04-30
The following changes since commit 648db19685b7030aa558a4ddbd3a8e53d8c9a062: Merge remote-tracking branch 'remotes/armbru/tags/pull-misc-2020-04-29' into staging (2020-04-29 15:07:33 +0100) are available in the Git repository at: git://repo.or.cz/qemu/armbru.git tags/pull-qapi-2020-04-30 for you to fetch changes up to 89bf68f933393a1bc0de4d07b59ffa8920da130f: qapi: Generate simpler marshalling code when no arguments (2020-04-30 07:26:41 +0200) QAPI patches for 2020-04-30 Markus Armbruster (20): qobject: Clean up QLIST_FOREACH_ENTRY() qobject: Factor out helper json_pretty_newline() qobject: Eliminate qlist_iter(), use QLIST_FOREACH_ENTRY() instead qobject: Eliminate qdict_iter(), use qdict_first(), qdict_next() qemu-option: Clean up after the previous commit qapi: Belatedly update visitor.h's big comment for QAPI modules qapi: Fix the virtual walk example in visitor.h's big comment qapi: Fix typo in visit_start_list()'s contract qapi: Document @errp usage more thoroughly in visitor.h qapi: Polish prose in visitor.h qapi: Assert incomplete object occurs only in dealloc visitor qapi: Fix Visitor contract for start_alternate() qapi: Assert output visitors see only valid enum values qapi: Assert non-input visitors see only valid narrow integers qapi: Clean up visitor's recovery from input with invalid type qapi: Assert non-input visitors see only valid alternate tags qapi: Only input visitors can actually fail qom: Simplify object_property_get_enum() qapi: Disallow qmp_marshal_FOO(NULL, ...) qapi: Generate simpler marshalling code when no arguments docs/devel/qapi-code-gen.txt| 4 +- include/qapi/qmp/qdict.h| 3 - include/qapi/qmp/qlist.h| 10 +- include/qapi/visitor-impl.h | 9 +- include/qapi/visitor.h | 192 +--- block.c | 9 +- block/sheepdog.c| 9 +- blockdev.c | 16 +-- hw/core/machine-hmp-cmds.c | 2 +- monitor/hmp-cmds.c | 3 +- monitor/qmp.c | 5 +- qapi/qapi-dealloc-visitor.c | 7 -- qapi/qapi-visit-core.c | 20 ++-- qapi/qobject-input-visitor.c| 21 ++-- qobject/qdict.c | 19 qobject/qjson.c | 107 +++- qobject/qlist.c | 44 +++-- qom/object.c| 4 +- tests/check-qlist.c | 37 +++ tests/test-qobject-output-visitor.c | 39 tests/test-string-output-visitor.c | 19 util/qemu-option.c | 43 scripts/qapi/commands.py| 62 +--- scripts/qapi/visit.py | 8 ++ 24 files changed, 272 insertions(+), 420 deletions(-) -- 2.21.1
[PULL 03/20] qobject: Eliminate qlist_iter(), use QLIST_FOREACH_ENTRY() instead
qlist_iter() has just three uses outside tests/. Replace by
QLIST_FOREACH_ENTRY() for more concise code and less type punning.
Signed-off-by: Markus Armbruster
Message-Id: <20200415083048.14339-4-arm...@redhat.com>
Reviewed-by: Eric Blake
---
include/qapi/qmp/qlist.h | 2 --
qobject/qjson.c | 31 ++--
qobject/qlist.c | 44 +++-
tests/check-qlist.c | 37 +
4 files changed, 37 insertions(+), 77 deletions(-)
diff --git a/include/qapi/qmp/qlist.h b/include/qapi/qmp/qlist.h
index 07ecae81e4..595b7943e1 100644
--- a/include/qapi/qmp/qlist.h
+++ b/include/qapi/qmp/qlist.h
@@ -47,8 +47,6 @@ static inline QObject *qlist_entry_obj(const QListEntry
*entry)
QList *qlist_new(void);
QList *qlist_copy(QList *src);
void qlist_append_obj(QList *qlist, QObject *obj);
-void qlist_iter(const QList *qlist,
-void (*iter)(QObject *obj, void *opaque), void *opaque);
QObject *qlist_pop(QList *qlist);
QObject *qlist_peek(QList *qlist);
int qlist_empty(const QList *qlist);
diff --git a/qobject/qjson.c b/qobject/qjson.c
index 87422f600d..f0eebc5fda 100644
--- a/qobject/qjson.c
+++ b/qobject/qjson.c
@@ -191,20 +191,6 @@ static void to_json_dict_iter(const char *key, QObject
*obj, void *opaque)
s->count++;
}
-static void to_json_list_iter(QObject *obj, void *opaque)
-{
-ToJsonIterState *s = opaque;
-
-if (s->count) {
-qstring_append(s->str, s->pretty ? "," : ", ");
-}
-
-json_pretty_newline(s->str, s->pretty, s->indent);
-
-to_json(obj, s->str, s->pretty, s->indent);
-s->count++;
-}
-
static void to_json(const QObject *obj, QString *str, int pretty, int indent)
{
switch (qobject_type(obj)) {
@@ -289,15 +275,20 @@ static void to_json(const QObject *obj, QString *str, int
pretty, int indent)
break;
}
case QTYPE_QLIST: {
-ToJsonIterState s;
QList *val = qobject_to(QList, obj);
+const char *comma = pretty ? "," : ", ";
+const char *sep = "";
+QListEntry *entry;
-s.count = 0;
-s.str = str;
-s.indent = indent + 1;
-s.pretty = pretty;
qstring_append(str, "[");
-qlist_iter(val, (void *)to_json_list_iter, );
+
+QLIST_FOREACH_ENTRY(val, entry) {
+qstring_append(str, sep);
+json_pretty_newline(str, pretty, indent + 1);
+to_json(qlist_entry_obj(entry), str, pretty, indent + 1);
+sep = comma;
+}
+
json_pretty_newline(str, pretty, indent);
qstring_append(str, "]");
break;
diff --git a/qobject/qlist.c b/qobject/qlist.c
index b3274af88b..1be95367d1 100644
--- a/qobject/qlist.c
+++ b/qobject/qlist.c
@@ -34,20 +34,17 @@ QList *qlist_new(void)
return qlist;
}
-static void qlist_copy_elem(QObject *obj, void *opaque)
-{
-QList *dst = opaque;
-
-qobject_ref(obj);
-qlist_append_obj(dst, obj);
-}
-
QList *qlist_copy(QList *src)
{
QList *dst = qlist_new();
+QListEntry *entry;
+QObject *elt;
-qlist_iter(src, qlist_copy_elem, dst);
-
+QLIST_FOREACH_ENTRY(src, entry) {
+elt = qlist_entry_obj(entry);
+qobject_ref(elt);
+qlist_append_obj(dst, elt);
+}
return dst;
}
@@ -86,21 +83,6 @@ void qlist_append_null(QList *qlist)
qlist_append(qlist, qnull());
}
-/**
- * qlist_iter(): Iterate over all the list's stored values.
- *
- * This function allows the user to provide an iterator, which will be
- * called for each stored value in the list.
- */
-void qlist_iter(const QList *qlist,
-void (*iter)(QObject *obj, void *opaque), void *opaque)
-{
-QListEntry *entry;
-
-QTAILQ_FOREACH(entry, >head, next)
-iter(entry->value, opaque);
-}
-
QObject *qlist_pop(QList *qlist)
{
QListEntry *entry;
@@ -137,16 +119,14 @@ int qlist_empty(const QList *qlist)
return QTAILQ_EMPTY(>head);
}
-static void qlist_size_iter(QObject *obj, void *opaque)
-{
-size_t *count = opaque;
-(*count)++;
-}
-
size_t qlist_size(const QList *qlist)
{
size_t count = 0;
-qlist_iter(qlist, qlist_size_iter, );
+QListEntry *entry;
+
+QLIST_FOREACH_ENTRY(qlist, entry) {
+count++;
+}
return count;
}
diff --git a/tests/check-qlist.c b/tests/check-qlist.c
index ece83e293d..3cd0ccbf19 100644
--- a/tests/check-qlist.c
+++ b/tests/check-qlist.c
@@ -61,40 +61,31 @@ static void qobject_to_qlist_test(void)
qobject_unref(qlist);
}
-static int iter_called;
-static const int iter_max = 42;
-
-static void iter_func(QObject *obj, void *opaque)
-{
-QNum *qi;
-int64_t val;
-
-g_assert(opaque == NULL);
-
-qi = qobject_to(QNum, obj);
-g_assert(qi != NULL);
-
-g_assert(qnum_get_try_int(qi, ));
-g_assert_cmpint(val, >=, 0);
-g_assert_cmpint(val, <=, iter_max);
-
-iter_called++;
-}
-
static void qlist_iter_test(void)
{
+
[PULL 01/20] qobject: Clean up QLIST_FOREACH_ENTRY()
QLIST_FOREACH_ENTRY() traverses a tail queue manually. Use
QTAILQ_FIRST() and QTAILQ_NEXT() instead.
Signed-off-by: Markus Armbruster
Message-Id: <20200415083048.14339-2-arm...@redhat.com>
Reviewed-by: Eric Blake
---
include/qapi/qmp/qlist.h | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/include/qapi/qmp/qlist.h b/include/qapi/qmp/qlist.h
index 8d2c32ca28..07ecae81e4 100644
--- a/include/qapi/qmp/qlist.h
+++ b/include/qapi/qmp/qlist.h
@@ -34,10 +34,10 @@ void qlist_append_int(QList *qlist, int64_t value);
void qlist_append_null(QList *qlist);
void qlist_append_str(QList *qlist, const char *value);
-#define QLIST_FOREACH_ENTRY(qlist, var) \
-for ((var) = ((qlist)->head.tqh_first); \
-(var); \
-(var) = ((var)->next.tqe_next))
+#define QLIST_FOREACH_ENTRY(qlist, var) \
+for ((var) = QTAILQ_FIRST(&(qlist)->head); \
+ (var); \
+ (var) = QTAILQ_NEXT((var), next))
static inline QObject *qlist_entry_obj(const QListEntry *entry)
{
--
2.21.1
Re: [PATCH v2 06/17] block/io: support int64_t bytes in bdrv_aligned_pwritev()
30.04.2020 8:25, Vladimir Sementsov-Ogievskiy wrote:
30.04.2020 1:04, Eric Blake wrote:
On 4/27/20 3:23 AM, Vladimir Sementsov-Ogievskiy wrote:
We are generally moving to int64_t for both offset and bytes parameters
on all io paths. Prepare bdrv_aligned_pwritev() now (and convert the
dependencies: bdrv_co_write_req_prepare() and
bdrv_co_write_req_finish() to signed type bytes)
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
block/io.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/block/io.c b/block/io.c
index c8c30e3699..fe19e09034 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1854,7 +1854,7 @@ fail:
}
static inline int coroutine_fn
-bdrv_co_write_req_prepare(BdrvChild *child, int64_t offset, uint64_t bytes,
+bdrv_co_write_req_prepare(BdrvChild *child, int64_t offset, int64_t bytes,
BdrvTrackedRequest *req, int flags)
{
No change in size. First, check usage within function:
int64_t end_sector = DIV_ROUND_UP(offset + bytes, BDRV_SECTOR_SIZE);
Changes computation from uint64_t to int64_t. This causes a borderline bug on
images between INT64_MAX-511 and INT64_MAX (nbdkit can produce such images over
NBD, although they are atypical on disk), where DIV_ROUND_UP() would give the
right answer as uint64_t but a negative answer with int64_t. As those images
are not sector-aligned, maybe we don't need to care?
all other uses appear to be within asserts related to offset+bytes being
positive, so that's what we should check for.
Callers:
bdrv_aligned_pwritev() - changed in this patch to 'int64_t', analyzed below [1]
bdrv_co_pdiscard() - already passes 'int64_t', also checks for offset+bytes
overflow - safe
bdrv_co_copy_range_internal() - 'uint64_t', but already analyzed for 3/17 how it
was capped < 2M - safe
bdrv_co_truncate() - already passes 'int64_t', passes new_bytes computed by
subtracting from a positive 'int64_t offset' - safe
[1] except I hit the end of my work day, so my analysis will have to continue
tomorrow...
Thanks for reviewing!
I'm very sorry, I just need to say once again: the series should be rebased on
"[PATCH for-5.0? 0/9] block/io: safer inc/dec in_flight sections", as it is
already mostly reviewed by Stefan. Seems, that your analysis will be still valid after
it, although patches will change. I'll do it now to see, can I keep your r-b's.
I mean "[PATCH v2 0/9] block/io: safer inc/dec in_flight sections" of course
https://lists.gnu.org/archive/html/qemu-devel/2020-04/msg04559.html
--
Best regards,
Vladimir
[PATCH v5 1/3] memory: drop guest writes to read-only ram device regions
for ram device regions, drop guest writes if the regions is read-only.
Cc: Philippe Mathieu-Daudé
Signed-off-by: Yan Zhao
Signed-off-by: Xin Zeng
---
memory.c | 15 ---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/memory.c b/memory.c
index 601b749906..90a748912f 100644
--- a/memory.c
+++ b/memory.c
@@ -34,6 +34,7 @@
#include "sysemu/accel.h"
#include "hw/boards.h"
#include "migration/vmstate.h"
+#include "qemu/log.h"
//#define DEBUG_UNASSIGNED
@@ -1307,12 +1308,19 @@ static uint64_t memory_region_ram_device_read(void
*opaque,
return data;
}
-static void memory_region_ram_device_write(void *opaque, hwaddr addr,
- uint64_t data, unsigned size)
+static MemTxResult memory_region_ram_device_write(void *opaque, hwaddr addr,
+ uint64_t data, unsigned size,
+ MemTxAttrs attrs)
{
MemoryRegion *mr = opaque;
trace_memory_region_ram_device_write(get_cpu_index(), mr, addr, data,
size);
+if (mr->readonly) {
+qemu_log_mask(LOG_GUEST_ERROR,
+ "Invalid write to read only ram device region addr 0x%"
+ HWADDR_PRIx" size %u\n", addr, size);
+return MEMTX_ERROR;
+}
switch (size) {
case 1:
@@ -1328,11 +1336,12 @@ static void memory_region_ram_device_write(void
*opaque, hwaddr addr,
*(uint64_t *)(mr->ram_block->host + addr) = data;
break;
}
+return MEMTX_OK;
}
static const MemoryRegionOps ram_device_mem_ops = {
.read = memory_region_ram_device_read,
-.write = memory_region_ram_device_write,
+.write_with_attrs = memory_region_ram_device_write,
.endianness = DEVICE_HOST_ENDIAN,
.valid = {
.min_access_size = 1,
--
2.17.1
[PATCH v5 0/3] drop writes to read-only ram device & vfio regions
guest writes to read-only memory regions need to be dropped. patch 1 modifies handler of ram device memory regions to drop guest writes to read-only ram device memory regions patch 2 modifies handler of non-mmap'd read-only vfio regions to drop guest writes to those regions patch 3 set read-only flag to mmap'd read-only vfio regions, so that guest writes to those regions would be trapped. without patch 1, host qemu would then crash on guest write to those read-only regions. with patch 1, host qemu would drop the writes. Changelog: v5: -changed write handler of ram device memory region from .write to .write_with_attrs in patch 1 (Paolo) (for vfio region in patch 2, I still keep the operations as .read & .write. the reasons are: 1. vfio_region_ops are for mmio/pio regions. the top level read/write dispatcher in kvm just ignores their return values. (the return value of address_space_rw() is just ignored) 2. there are a lot of callers to vfio_region_read() and vfio_region_write(), who actually do not care about the return values ) -minor changes on text format in error logs. v4: -instead of modifying tracing log, added qemu_log_mask(LOG_GUEST_ERROR...) to log guest writes to read-only regions (Philippe) for v3: -refreshed and Cc Stefan for reviewing of tracing part v2: -split one big patches into smaller ones (Philippe) -modify existing trace to record guest writes to read-only memory (Alex) -modify vfio_region_write() to drop guest writes to non-mmap'd read-only region (Alex) Yan Zhao (3): memory: drop guest writes to read-only ram device regions hw/vfio: drop guest writes to ro regions hw/vfio: let read-only flag take effect for mmap'd regions hw/vfio/common.c | 17 +++-- memory.c | 15 --- 2 files changed, 27 insertions(+), 5 deletions(-) -- 2.17.1
Re: [PATCH v2 06/17] block/io: support int64_t bytes in bdrv_aligned_pwritev()
30.04.2020 1:04, Eric Blake wrote:
On 4/27/20 3:23 AM, Vladimir Sementsov-Ogievskiy wrote:
We are generally moving to int64_t for both offset and bytes parameters
on all io paths. Prepare bdrv_aligned_pwritev() now (and convert the
dependencies: bdrv_co_write_req_prepare() and
bdrv_co_write_req_finish() to signed type bytes)
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
block/io.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/block/io.c b/block/io.c
index c8c30e3699..fe19e09034 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1854,7 +1854,7 @@ fail:
}
static inline int coroutine_fn
-bdrv_co_write_req_prepare(BdrvChild *child, int64_t offset, uint64_t bytes,
+bdrv_co_write_req_prepare(BdrvChild *child, int64_t offset, int64_t bytes,
BdrvTrackedRequest *req, int flags)
{
No change in size. First, check usage within function:
int64_t end_sector = DIV_ROUND_UP(offset + bytes, BDRV_SECTOR_SIZE);
Changes computation from uint64_t to int64_t. This causes a borderline bug on
images between INT64_MAX-511 and INT64_MAX (nbdkit can produce such images over
NBD, although they are atypical on disk), where DIV_ROUND_UP() would give the
right answer as uint64_t but a negative answer with int64_t. As those images
are not sector-aligned, maybe we don't need to care?
all other uses appear to be within asserts related to offset+bytes being
positive, so that's what we should check for.
Callers:
bdrv_aligned_pwritev() - changed in this patch to 'int64_t', analyzed below [1]
bdrv_co_pdiscard() - already passes 'int64_t', also checks for offset+bytes
overflow - safe
bdrv_co_copy_range_internal() - 'uint64_t', but already analyzed for 3/17 how it
was capped < 2M - safe
bdrv_co_truncate() - already passes 'int64_t', passes new_bytes computed by
subtracting from a positive 'int64_t offset' - safe
[1] except I hit the end of my work day, so my analysis will have to continue
tomorrow...
Thanks for reviewing!
I'm very sorry, I just need to say once again: the series should be rebased on
"[PATCH for-5.0? 0/9] block/io: safer inc/dec in_flight sections", as it is
already mostly reviewed by Stefan. Seems, that your analysis will be still valid after
it, although patches will change. I'll do it now to see, can I keep your r-b's.
--
Best regards,
Vladimir
Re: [PATCH] tests: add a "check-flake8" test for validating python code style
Daniel P. Berrangé writes:
> The flake8 program is a standard tool used by Python projects for
> validating many commonly recommended style rules. It would be desirable
> for QEMU to come into alignment with normal Python coding style best
> practices.
>
> QEMU currently violates a huge number of the style rules, so we can't
> blindly turn it on. Instead this patch introduces use of flake8 with
> a huge ignore list to turn off everything that is currently violated.
>
> The following descriptions are mostly taken from:
>
> https://www.flake8rules.com/
>
> Indentation:
>
> E111 Indentation is not a multiple of four
> E114 Indentation is not a multiple of four (comment)
> E115 Expected an indented block (comment)
> E116 Unexpected indentation (comment)
> E117 Over-indented
> E121 Continuation line under-indented for hanging indent
> E122 Continuation line missing indentation or outdented
> E123 Closing bracket does not match indentation of opening bracket's
> line
> E124 Closing bracket does not match visual indentation
> E125 Continuation line with same indent as next logical line
> E126 Continuation line over-indented for hanging indent
> E127 Continuation line over-indented for visual indent
> E128 Continuation line under-indented for visual indent
> E129 Visually indented line with same indent as next logical line
> E131 Continuation line unaligned for hanging indent
>
> Whitespace:
>
> E201 Whitespace after '('
> E202 Whitespace before ')'
> E203 Whitespace before ':'
> E211 Whitespace before '('
> E221 Multiple spaces before operator
> E222 Multiple spaces after operator
> E225 Missing whitespace around operator
> E226 Missing whitespace around arithmetic operator
> E228 Missing whitespace around modulo operator
> E231 Missing whitespace after ',', ';', or ':'
> E241 Multiple spaces after ','
> E251 Unexpected spaces around keyword / parameter equals
> E261 At least two spaces before inline comment
> E265 Block comment should start with '# '
> E266 Too many leading '#' for block comment
>
> Blank lines:
>
> E301 Expected 1 blank line, found 0
> E302 Expected 2 blank lines, found 0
> E303 Too many blank lines (3)
> E305 Expected 2 blank lines after end of function or class
> E306 Expected 1 blank line before a nested definition
>
> Imports:
>
> E401 Multiple imports on one line
> E402 Module level import not at top of file
>
> Line length:
>
> E501 Line too long (82 > 79 characters)
> E502 The backslash is redundant between brackets
>
> Statements:
>
> E701 Multiple statements on one line (colon)
> E702 Multiple statements on one line (semicolon)
> E703 Statement ends with a semicolon
> E711 Comparison to none should be 'if cond is none:'
> E712 Comparison to true should be 'if cond is true:' or 'if cond:'
> E713 Test for membership should be 'not in'
> E714 Test for object identity should be 'is not'
> E722 Do not use bare 'except'
> E731 Do not assign a lambda expression, use a def
> E741 Do not use variables named 'I', 'O', or 'l'
>
> Errors:
>
> F401 Module imported but unused
> F403 'from module import *' used; unable to detect undefined names
> F405 Name may be undefined, or defined from star imports: module
> F811 Redefinition of unused name from line n
> F821 Undefined name name
> F841 Local variable name is assigned to but never used
>
> Warnings:
>
> W391 Blank line at end of file
> W503 Line break occurred before a binary operator
> W504 Line break occurred after a binary operator
> W605 Invalid escape sequence 'x'
>
> Over time code should be fixed and rules removed from the ignore list.
> A handful of style rules may not warrant fixing as the cure is arguably
> worse and very subjective. e.g.
>
> E501: Force breaking lines at < 80 characters results in
>some really unnatural code formatting which harms
>readability.
>
> W504: Knuth code style requires the operators "or" and "and" etc
>to be at the start of line in a multi-line conditional.
Ah, a bikeshed! I really dislike long lines, and I really like Knuth
style ;-P
> Signed-off-by: Daniel P. Berrangé
> ---
>
> On its own this patch doesn't really do much of use except try to stop the
> situation getting worse. To be valuable some motivated contributor(s)
> would need to go through fixing the code, and re-enabling each excluded
> warning category one at a time.
>
> I'm mostly proposing this patch as a starting point for discussion, to
> see if anyone is indeed motivated to take on the code cleanup challenge,
Re: [PATCH v2 03/17] block/io: use int64_t bytes parameter in bdrv_check_byte_request()
29.04.2020 22:27, Eric Blake wrote:
On 4/27/20 3:23 AM, Vladimir Sementsov-Ogievskiy wrote:
We are generally moving to int64_t for both offset and bytes parameters
on all io paths. Convert bdrv_check_byte_request() now.
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
block/io.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/block/io.c b/block/io.c
index 7cbb80bd24..1267918def 100644
--- a/block/io.c
+++ b/block/io.c
@@ -875,9 +875,9 @@ static bool coroutine_fn
bdrv_wait_serialising_requests(BdrvTrackedRequest *self
}
static int bdrv_check_byte_request(BlockDriverState *bs, int64_t offset,
- size_t size)
+ int64_t bytes)
{
This changes an unsigned to signed value on 64-bit machines, and additionally
widens the parameter on 32-bit machines. Existing callers:
bdrv_co_preadv_part() with 'unsigned int' - no impact
bdrv_co_pwritev_part() with 'unsigned int' - no impact
bdrv_co_copy_range_internal() with 'uint64_t' - potentially fixes a latent bug
on 32-bit machines. Requires a larger audit to see how bdrv_co_copy_range() and
friends are used:
block/block-backend.c:blk_co_copy_range() - passes 'int', thus < 2G
block/block-copy.c:block_copy_do_copy() - passes 'int64_t', but only after
assert(nbytes < INT_MAX); also it has a BLOCK_COPY_MAX_COPY_RANGE set to 16M
that factors into its calculations on how much to do per iteration
So it looks like at present we are safe, but the commit message should
definitely call out the fix for an unreachable latent bug.
I will also point out that on Linux, copy_file_range() is capped by a size_t len
parameter, so even if we DO have a 32-bit caller passing > 2G, it will
encounter further truncation bugs if the block layer does not fragment the request
internally. I don't see any fragmentation logic in the public
bdrv_co_copy_range() or in bdrv_co_copy_range_internal() - I suspect that needs to
be added (probably as a separate patch); maybe by moving the fragmentation loop
currently in block-copy.c to instead live in io.c?
fragmentation loop in io.c should have larger granularity than in block-copy.c,
isn't it? Fragmentation loop in block-copy will be async for performance
reasons, based on aio-task-pool. And it should cover both copy_range and simple
copy and write-zeroes cases.. So, I think it's simpler to keep separate
fragmentation loop in io.c. Still it's not really needed until block-copy is
the only user of the interface.
- if (size > BDRV_REQUEST_MAX_BYTES) {
+ if (bytes > BDRV_REQUEST_MAX_BYTES) {
return -EIO;
}
@@ -885,7 +885,7 @@ static int bdrv_check_byte_request(BlockDriverState *bs,
int64_t offset,
return -ENOMEDIUM;
}
- if (offset < 0) {
+ if (offset < 0 || bytes < 0) {
return -EIO;
}
Reviewed-by: Eric Blake
I don't know if we have any iotest coverage of copy_range with a 5G image to
prove that it doesn't misbehave on a 32-bit machine. That seems like it will
eventually be useful, but doesn't necessarily have to be at the same time as
this patch.
--
Best regards,
Vladimir
Re: [PATCH 4/4] scripts/qmp: Fix QEMU Python scripts path
John Snow writes:
> On 4/21/20 5:42 AM, Philippe Mathieu-Daudé wrote:
>> QEMU Python scripts have been moved in commit 8f8fd9edba4 ("Introduce
>> Python module structure"). Use the same sys.path modification used
>> in the referenced commit to be able to use these scripts again.
>>
>> Signed-off-by: Philippe Mathieu-Daudé
>> ---
>> scripts/qmp/qmp | 4 +++-
>> scripts/qmp/qom-fuse | 4 +++-
>> scripts/qmp/qom-get | 4 +++-
>> scripts/qmp/qom-list | 4 +++-
>> scripts/qmp/qom-set | 4 +++-
>> scripts/qmp/qom-tree | 4 +++-
>> 6 files changed, 18 insertions(+), 6 deletions(-)
>>
>> diff --git a/scripts/qmp/qmp b/scripts/qmp/qmp
>> index 0625fc2aba..8e52e4a54d 100755
>> --- a/scripts/qmp/qmp
>> +++ b/scripts/qmp/qmp
>> @@ -11,7 +11,9 @@
>> # See the COPYING file in the top-level directory.
>>
>> import sys, os
>> -from qmp import QEMUMonitorProtocol
>> +
>> +sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..',
>> 'python'))
>> +from qemu.qmp import QEMUMonitorProtocol
>>
>
> Try to avoid using sys.path hacks; they don't work in pylint or mypy and
> it provides an active barrier to CQA work here.
> (They also tend to be quite fragile.)
>
> We can discuss the right way to do this; one of those ways is to create
> an installable package that we can install locally in a virtual environment.
>
> Another way is perhaps to set PYTHONPATH in the calling environment so
> that standard "import" directives will work.
>
> Both ultimately involve changing the environment of the user to
> accommodate the script.
For what it's worth, tests/Makefile.involve does the latter for
tests/qapi-schema/test-qapi.py. Simple enough, but makes manual
invocation inconvenient.
Not necessary for scripts/qapi-gen.py, because its "import qmp.FOO"
finds qmp right in scripts/qmp/.
Re: dirty bitmap migration refactor
29.04.2020 16:29, John Snow wrote: Hi all, as you are probably aware I haven't been paying attention to dirty bitmap work very much for the past month. Around KVM Forum, we had a giant thread dedicated to discussing the problems with dirty bitmap migration, which in a nutshell, are that it migrates using the node name with no option for re-routing or re-naming nodes. IIRC, there was a patchset to fix this quickly sent by Virtuozzo, but the series stalled because it was quite close to a release and was deemed too risky. What is the status of those patches, if any? Do we need to start from scratch to implement the functionality that libvirt wants here? Hi! There are two series now in list: "[PATCH v2 0/5] fix migration with bitmaps and mirror" - the series you are saying about I made some changes to it downstream, to restrict migration by generated node names at all, as we had a bug. I can resend, if needed. What the series does? It just tries to migrate by blk name even for filtered nodes. This fixes migration of bitmaps during mirror. But it doesn't apply to blockdev-era (doesn't hurt still). So can someone analyze, do we need this fix in current Qemu? Or is it for downstream only? Or should we take it just to make downstreaming cleaner? === The second is "[PATCH v2 00/22] Fix error handling during bitmap postcopy" - it fixes, how postcopy bitmap migration behaves on errors, and we need it anyway. === What to do next? I have a plan to post series to implement new API, discussed on list, to make mapping from bitmaps on source to bitmaps on target by hand. -- Best regards, Vladimir
Re: [PATCH v25 00/10] Add ARMv8 RAS virtualization support in QEMU
On 2020/4/17 21:32, Peter Maydell wrote: > On Fri, 10 Apr 2020 at 12:46, Dongjiu Geng wrote: >> >> In the ARMv8 platform, the CPU error types includes synchronous external >> abort(SEA) >> and SError Interrupt (SEI). If exception happens in guest, host does not >> know the detailed >> information of guest, so it is expected that guest can do the recovery. For >> example, if an >> exception happens in a guest user-space application, host does not know >> which application >> encounters errors, only guest knows it. >> >> For the ARMv8 SEA/SEI, KVM or host kernel delivers SIGBUS to notify >> userspace. >> After user space gets the notification, it will record the CPER into guest >> GHES >> buffer and inject an exception or IRQ to guest. >> >> In the current implementation, if the type of SIGBUS is BUS_MCEERR_AR, we >> will >> treat it as a synchronous exception, and notify guest with ARMv8 SEA >> notification type after recording CPER into guest. > > Hi. I left a comment on patch 1. The other 3 patches unreviewed > are 5, 6 and 8, which are all ACPI core code, so that's for > MST, Igor or Shannon to review. Ping MST, Igor and Shannon, sorry for the noise. > > Once those have been reviewed, please ping me if you want this > to go via target-arm.next. > > thanks > -- PMM > > . >
Re: [PATCH 2/3] hw/acpi-build: account for NVDIMM numa nodes in SRAT
On 4/28/2020 9:28 AM, Verma, Vishal L wrote:
NVDIMMs can belong to their own proximity domains, as described by the
NFIT. In such cases, the SRAT needs to have Memory Affinity structures
in the SRAT for these NVDIMMs, otherwise Linux doesn't populate node
data structures properly during NUMA initialization. See the following
for an example failure case.
https://lore.kernel.org/linux-nvdimm/20200416225438.15208-1-vishal.l.ve...@intel.com/
Fix this by adding device address range and node information from
NVDIMMs to the SRAT in build_srat().
The relevant command line options to exercise this are below. Nodes 0-1
contain CPUs and regular memory, and nodes 2-3 are the NVDIMM address
space.
-numa node,nodeid=0,mem=2048M,
-numa node,nodeid=1,mem=2048M,
-numa node,nodeid=2,mem=0,
-object
memory-backend-file,id=nvmem0,share,mem-path=nvdimm-0,size=16384M,align=128M
-device nvdimm,memdev=nvmem0,id=nv0,label-size=2M,node=2
-numa node,nodeid=3,mem=0,
-object
memory-backend-file,id=nvmem1,share,mem-path=nvdimm-1,size=16384M,align=128M
-device nvdimm,memdev=nvmem1,id=nv1,label-size=2M,node=3
Cc: Jingqi Liu
Cc: Michael S. Tsirkin
Signed-off-by: Vishal Verma
---
hw/i386/acpi-build.c | 20
1 file changed, 20 insertions(+)
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 23c77eeb95..b0da67de0e 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -48,6 +48,7 @@
#include "migration/vmstate.h"
#include "hw/mem/memory-device.h"
#include "hw/mem/nvdimm.h"
+#include "qemu/nvdimm-utils.h"
#include "sysemu/numa.h"
#include "sysemu/reset.h"
@@ -2429,6 +2430,25 @@ build_srat(GArray *table_data, BIOSLinker *linker, MachineState *machine)
MEM_AFFINITY_ENABLED);
}
}
+
+if (machine->nvdimms_state->is_enabled) {
+GSList *device_list = nvdimm_get_device_list();
+
+for (; device_list; device_list = device_list->next) {
+DeviceState *dev = device_list->data;
+int node = object_property_get_int(OBJECT(dev), PC_DIMM_NODE_PROP,
+ NULL);
+uint64_t addr = object_property_get_uint(OBJECT(dev),
+ PC_DIMM_ADDR_PROP, NULL);
+uint64_t size = object_property_get_uint(OBJECT(dev),
+ PC_DIMM_SIZE_PROP, NULL);
+
+numamem = acpi_data_push(table_data, sizeof *numamem);
+build_srat_memory(numamem, addr, size, node,
+ MEM_AFFINITY_ENABLED |
MEM_AFFINITY_NON_VOLATILE);
+}
+}
+
Looks good for me.
Reviewed-by: Jingqi Liu
Thanks,
Jingqi
slots = (table_data->len - numa_start) / sizeof *numamem;
for (; slots < pcms->numa_nodes + 2; slots++) {
numamem = acpi_data_push(table_data, sizeof *numamem);
Re: R: R: About hardfloat in ppc
On 4/29/20 5:20 PM, 罗勇刚(Yonggang Luo) wrote:
> Question, in hard-float, if we don't want to read the fp register.
> for example: If we wanna compute c = a + b in fp32
> if c = a + b In hard float
> and if b1 = c - a in hard float
> if b1 != b at bitwise level, the we se the inexat to 1, otherwsie
> we set inexat bit to 0? are this valid?
>
> we can also do it for a * b, a - b, a / b.
>
That does seem plausible, for all of the normal values for which we would apply
the hard-float optimization anyway. But we already check for the exceptional
cases:
if (unlikely(f32_is_inf(ur))) {
s->float_exception_flags |= float_flag_overflow;
} else if (unlikely(fabsf(ur.h) <= FLT_MIN)) {
if (post == NULL || post(ua, ub)) {
goto soft;
}
}
r~
[PATCH v3 8/9] target/arm: Implement SVE2 crypto destructive binary operations
Signed-off-by: Richard Henderson
---
target/arm/cpu.h | 5 +
target/arm/sve.decode | 7 ++
target/arm/translate-sve.c | 45 ++
3 files changed, 57 insertions(+)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 332c67bbb0..4f09dd42ba 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -3870,6 +3870,11 @@ static inline bool isar_feature_aa64_sve2_bitperm(const
ARMISARegisters *id)
return FIELD_EX64(id->id_aa64zfr0, ID_AA64ZFR0, BITPERM) != 0;
}
+static inline bool isar_feature_aa64_sve2_sm4(const ARMISARegisters *id)
+{
+return FIELD_EX64(id->id_aa64zfr0, ID_AA64ZFR0, SM4) != 0;
+}
+
static inline bool isar_feature_aa64_sve2_i8mm(const ARMISARegisters *id)
{
return FIELD_EX64(id->id_aa64zfr0, ID_AA64ZFR0, I8MM) != 0;
diff --git a/target/arm/sve.decode b/target/arm/sve.decode
index 04985d2bb8..149de7949b 100644
--- a/target/arm/sve.decode
+++ b/target/arm/sve.decode
@@ -118,6 +118,8 @@
@pd_pn_pm esz:2 .. rm:4 ... rn:4 . rd:4 _esz
@rdn_rm esz:2 .. .. rm:5 rd:5 \
_esz rn=%reg_movprfx
+@rdn_rm_e0 .. .. .. rm:5 rd:5 \
+_esz rn=%reg_movprfx esz=0
@rdn_sh_i8u esz:2 .. .. . rd:5 \
_esz rn=%reg_movprfx imm=%sh8_i8u
@rdn_i8u esz:2 .. ... imm:8 rd:5 \
@@ -1557,3 +1559,8 @@ STNT1_zprz 1110010 .. 10 . 001 ... . . \
# SVE2 crypto unary operations
# AESMC and AESIMC
AESMC 01000101 00 1011100 decrypt:1 0 rd:5
+
+# SVE2 crypto destructive binary operations
+AESE01000101 00 10001 0 11100 0 . . @rdn_rm_e0
+AESD01000101 00 10001 0 11100 1 . . @rdn_rm_e0
+SM4E01000101 00 10001 1 11100 0 . . @rdn_rm_e0
diff --git a/target/arm/translate-sve.c b/target/arm/translate-sve.c
index 44dd1fe2b0..91e71882d6 100644
--- a/target/arm/translate-sve.c
+++ b/target/arm/translate-sve.c
@@ -8188,3 +8188,48 @@ static bool trans_AESMC(DisasContext *s, arg_AESMC *a)
}
return true;
}
+
+static bool do_aese(DisasContext *s, arg_rrr_esz *a, bool decrypt)
+{
+if (!dc_isar_feature(aa64_sve2_aes, s)) {
+return false;
+}
+if (sve_access_check(s)) {
+unsigned vsz = vec_full_reg_size(s);
+tcg_gen_gvec_3_ool(vec_full_reg_offset(s, a->rd),
+ vec_full_reg_offset(s, a->rn),
+ vec_full_reg_offset(s, a->rm),
+ vsz, vsz, decrypt, gen_helper_crypto_aese);
+}
+return true;
+}
+
+static bool trans_AESE(DisasContext *s, arg_rrr_esz *a)
+{
+return do_aese(s, a, false);
+}
+
+static bool trans_AESD(DisasContext *s, arg_rrr_esz *a)
+{
+return do_aese(s, a, true);
+}
+
+static bool do_sm4(DisasContext *s, arg_rrr_esz *a, gen_helper_gvec_3 *fn)
+{
+if (!dc_isar_feature(aa64_sve2_sm4, s)) {
+return false;
+}
+if (sve_access_check(s)) {
+unsigned vsz = vec_full_reg_size(s);
+tcg_gen_gvec_3_ool(vec_full_reg_offset(s, a->rd),
+ vec_full_reg_offset(s, a->rn),
+ vec_full_reg_offset(s, a->rm),
+ vsz, vsz, 0, fn);
+}
+return true;
+}
+
+static bool trans_SM4E(DisasContext *s, arg_rrr_esz *a)
+{
+return do_sm4(s, a, gen_helper_crypto_sm4e);
+}
--
2.20.1
[PATCH v3 7/9] target/arm: Implement SVE2 crypto unary operations
Signed-off-by: Richard Henderson
---
target/arm/sve.decode | 6 ++
target/arm/translate-sve.c | 14 ++
2 files changed, 20 insertions(+)
diff --git a/target/arm/sve.decode b/target/arm/sve.decode
index b73b64c3f2..04985d2bb8 100644
--- a/target/arm/sve.decode
+++ b/target/arm/sve.decode
@@ -1551,3 +1551,9 @@ STNT1_zprz 1110010 .. 00 . 001 ... . . \
# SVE2 32-bit scatter non-temporal store (vector plus scalar)
STNT1_zprz 1110010 .. 10 . 001 ... . . \
@rprr_scatter_store xs=0 esz=2 scale=0
+
+### SVE2 Crypto Extensions
+
+# SVE2 crypto unary operations
+# AESMC and AESIMC
+AESMC 01000101 00 1011100 decrypt:1 0 rd:5
diff --git a/target/arm/translate-sve.c b/target/arm/translate-sve.c
index 413000df3b..44dd1fe2b0 100644
--- a/target/arm/translate-sve.c
+++ b/target/arm/translate-sve.c
@@ -8174,3 +8174,17 @@ static bool trans_USDOT_(DisasContext *s,
arg_USDOT_ *a)
}
return true;
}
+
+static bool trans_AESMC(DisasContext *s, arg_AESMC *a)
+{
+if (!dc_isar_feature(aa64_sve2_aes, s)) {
+return false;
+}
+if (sve_access_check(s)) {
+unsigned vsz = vec_full_reg_size(s);
+unsigned rd = vec_full_reg_offset(s, a->rd);
+tcg_gen_gvec_2_ool(rd, rd, vsz, vsz, a->decrypt,
+ gen_helper_crypto_aesmc);
+}
+return true;
+}
--
2.20.1
[PATCH v3 6/9] target/arm: Split helper_crypto_sm3tt
Rather than passing an opcode to a helper, fully decode the
operation at translate time. Use clear_tail_16 to zap the
balance of the SVE register with the AdvSIMD write.
Signed-off-by: Richard Henderson
---
target/arm/helper.h| 5 -
target/arm/crypto_helper.c | 24 ++--
target/arm/translate-a64.c | 21 +
3 files changed, 27 insertions(+), 23 deletions(-)
diff --git a/target/arm/helper.h b/target/arm/helper.h
index 24aca28a05..e891f91e65 100644
--- a/target/arm/helper.h
+++ b/target/arm/helper.h
@@ -534,7 +534,10 @@ DEF_HELPER_FLAGS_3(crypto_sha512su0, TCG_CALL_NO_RWG,
void, ptr, ptr, i32)
DEF_HELPER_FLAGS_4(crypto_sha512su1, TCG_CALL_NO_RWG,
void, ptr, ptr, ptr, i32)
-DEF_HELPER_FLAGS_5(crypto_sm3tt, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32,
i32)
+DEF_HELPER_FLAGS_4(crypto_sm3tt1a, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
+DEF_HELPER_FLAGS_4(crypto_sm3tt1b, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
+DEF_HELPER_FLAGS_4(crypto_sm3tt2a, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
+DEF_HELPER_FLAGS_4(crypto_sm3tt2b, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
DEF_HELPER_FLAGS_4(crypto_sm3partw1, TCG_CALL_NO_RWG,
void, ptr, ptr, ptr, i32)
DEF_HELPER_FLAGS_4(crypto_sm3partw2, TCG_CALL_NO_RWG,
diff --git a/target/arm/crypto_helper.c b/target/arm/crypto_helper.c
index 636683d0f1..c76806dc8d 100644
--- a/target/arm/crypto_helper.c
+++ b/target/arm/crypto_helper.c
@@ -632,15 +632,14 @@ void HELPER(crypto_sm3partw2)(void *vd, void *vn, void
*vm, uint32_t desc)
clear_tail_16(vd, desc);
}
-void HELPER(crypto_sm3tt)(void *vd, void *vn, void *vm, uint32_t imm2,
- uint32_t opcode)
+static inline void QEMU_ALWAYS_INLINE
+crypto_sm3tt(uint64_t *rd, uint64_t *rn, uint64_t *rm,
+ uint32_t desc, uint32_t opcode)
{
-uint64_t *rd = vd;
-uint64_t *rn = vn;
-uint64_t *rm = vm;
union CRYPTO_STATE d = { .l = { rd[0], rd[1] } };
union CRYPTO_STATE n = { .l = { rn[0], rn[1] } };
union CRYPTO_STATE m = { .l = { rm[0], rm[1] } };
+uint32_t imm2 = simd_data(desc);
uint32_t t;
assert(imm2 < 4);
@@ -655,7 +654,7 @@ void HELPER(crypto_sm3tt)(void *vd, void *vn, void *vm,
uint32_t imm2,
/* SM3TT2B */
t = cho(CR_ST_WORD(d, 3), CR_ST_WORD(d, 2), CR_ST_WORD(d, 1));
} else {
-g_assert_not_reached();
+qemu_build_not_reached();
}
t += CR_ST_WORD(d, 0) + CR_ST_WORD(m, imm2);
@@ -680,8 +679,21 @@ void HELPER(crypto_sm3tt)(void *vd, void *vn, void *vm,
uint32_t imm2,
rd[0] = d.l[0];
rd[1] = d.l[1];
+
+clear_tail_16(rd, desc);
}
+#define DO_SM3TT(NAME, OPCODE) \
+void HELPER(NAME)(void *vd, void *vn, void *vm, uint32_t desc) \
+{ crypto_sm3tt(vd, vn, vm, desc, OPCODE); }
+
+DO_SM3TT(crypto_sm3tt1a, 0)
+DO_SM3TT(crypto_sm3tt1b, 1)
+DO_SM3TT(crypto_sm3tt2a, 2)
+DO_SM3TT(crypto_sm3tt2b, 3)
+
+#undef DO_SM3TT
+
static uint8_t const sm4_sbox[] = {
0xd6, 0x90, 0xe9, 0xfe, 0xcc, 0xe1, 0x3d, 0xb7,
0x16, 0xb6, 0x14, 0xc2, 0x28, 0xfb, 0x2c, 0x05,
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index 81ad287811..0fecc9b06f 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -14902,13 +14902,15 @@ static void disas_crypto_xar(DisasContext *s,
uint32_t insn)
*/
static void disas_crypto_three_reg_imm2(DisasContext *s, uint32_t insn)
{
+static gen_helper_gvec_3 * const fns[4] = {
+gen_helper_crypto_sm3tt1a, gen_helper_crypto_sm3tt1b,
+gen_helper_crypto_sm3tt2a, gen_helper_crypto_sm3tt2b,
+};
int opcode = extract32(insn, 10, 2);
int imm2 = extract32(insn, 12, 2);
int rm = extract32(insn, 16, 5);
int rn = extract32(insn, 5, 5);
int rd = extract32(insn, 0, 5);
-TCGv_ptr tcg_rd_ptr, tcg_rn_ptr, tcg_rm_ptr;
-TCGv_i32 tcg_imm2, tcg_opcode;
if (!dc_isar_feature(aa64_sm3, s)) {
unallocated_encoding(s);
@@ -14919,20 +14921,7 @@ static void disas_crypto_three_reg_imm2(DisasContext
*s, uint32_t insn)
return;
}
-tcg_rd_ptr = vec_full_reg_ptr(s, rd);
-tcg_rn_ptr = vec_full_reg_ptr(s, rn);
-tcg_rm_ptr = vec_full_reg_ptr(s, rm);
-tcg_imm2 = tcg_const_i32(imm2);
-tcg_opcode = tcg_const_i32(opcode);
-
-gen_helper_crypto_sm3tt(tcg_rd_ptr, tcg_rn_ptr, tcg_rm_ptr, tcg_imm2,
-tcg_opcode);
-
-tcg_temp_free_ptr(tcg_rd_ptr);
-tcg_temp_free_ptr(tcg_rn_ptr);
-tcg_temp_free_ptr(tcg_rm_ptr);
-tcg_temp_free_i32(tcg_imm2);
-tcg_temp_free_i32(tcg_opcode);
+gen_gvec_op3_ool(s, true, rd, rn, rm, imm2, fns[opcode]);
}
/* C3.6 Data processing - SIMD, inc Crypto
--
2.20.1
[PATCH v3 4/9] target/arm: Convert sha1 and sha256 to gvec helpers
Do not yet convert the helpers to loop over opr_sz, but the
descriptor allows the vector tail to be cleared. Which fixes
an existing bug.
Signed-off-by: Richard Henderson
---
target/arm/helper.h| 12 +--
target/arm/crypto_helper.c | 24 +++--
target/arm/translate-a64.c | 34 +++--
target/arm/translate.c | 44 +++---
4 files changed, 53 insertions(+), 61 deletions(-)
diff --git a/target/arm/helper.h b/target/arm/helper.h
index d564747808..07466ddc6c 100644
--- a/target/arm/helper.h
+++ b/target/arm/helper.h
@@ -517,13 +517,13 @@ DEF_HELPER_FLAGS_4(crypto_aese, TCG_CALL_NO_RWG, void,
ptr, ptr, ptr, i32)
DEF_HELPER_FLAGS_3(crypto_aesmc, TCG_CALL_NO_RWG, void, ptr, ptr, i32)
DEF_HELPER_FLAGS_4(crypto_sha1_3reg, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
-DEF_HELPER_FLAGS_2(crypto_sha1h, TCG_CALL_NO_RWG, void, ptr, ptr)
-DEF_HELPER_FLAGS_2(crypto_sha1su1, TCG_CALL_NO_RWG, void, ptr, ptr)
+DEF_HELPER_FLAGS_3(crypto_sha1h, TCG_CALL_NO_RWG, void, ptr, ptr, i32)
+DEF_HELPER_FLAGS_3(crypto_sha1su1, TCG_CALL_NO_RWG, void, ptr, ptr, i32)
-DEF_HELPER_FLAGS_3(crypto_sha256h, TCG_CALL_NO_RWG, void, ptr, ptr, ptr)
-DEF_HELPER_FLAGS_3(crypto_sha256h2, TCG_CALL_NO_RWG, void, ptr, ptr, ptr)
-DEF_HELPER_FLAGS_2(crypto_sha256su0, TCG_CALL_NO_RWG, void, ptr, ptr)
-DEF_HELPER_FLAGS_3(crypto_sha256su1, TCG_CALL_NO_RWG, void, ptr, ptr, ptr)
+DEF_HELPER_FLAGS_4(crypto_sha256h, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
+DEF_HELPER_FLAGS_4(crypto_sha256h2, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
+DEF_HELPER_FLAGS_3(crypto_sha256su0, TCG_CALL_NO_RWG, void, ptr, ptr, i32)
+DEF_HELPER_FLAGS_4(crypto_sha256su1, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
DEF_HELPER_FLAGS_4(crypto_sha512h, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
DEF_HELPER_FLAGS_4(crypto_sha512h2, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
diff --git a/target/arm/crypto_helper.c b/target/arm/crypto_helper.c
index 637e4c00bb..7124745c32 100644
--- a/target/arm/crypto_helper.c
+++ b/target/arm/crypto_helper.c
@@ -303,7 +303,7 @@ void HELPER(crypto_sha1_3reg)(void *vd, void *vn, void *vm,
uint32_t op)
rd[1] = d.l[1];
}
-void HELPER(crypto_sha1h)(void *vd, void *vm)
+void HELPER(crypto_sha1h)(void *vd, void *vm, uint32_t desc)
{
uint64_t *rd = vd;
uint64_t *rm = vm;
@@ -314,9 +314,11 @@ void HELPER(crypto_sha1h)(void *vd, void *vm)
rd[0] = m.l[0];
rd[1] = m.l[1];
+
+clear_tail_16(vd, desc);
}
-void HELPER(crypto_sha1su1)(void *vd, void *vm)
+void HELPER(crypto_sha1su1)(void *vd, void *vm, uint32_t desc)
{
uint64_t *rd = vd;
uint64_t *rm = vm;
@@ -330,6 +332,8 @@ void HELPER(crypto_sha1su1)(void *vd, void *vm)
rd[0] = d.l[0];
rd[1] = d.l[1];
+
+clear_tail_16(vd, desc);
}
/*
@@ -357,7 +361,7 @@ static uint32_t s1(uint32_t x)
return ror32(x, 17) ^ ror32(x, 19) ^ (x >> 10);
}
-void HELPER(crypto_sha256h)(void *vd, void *vn, void *vm)
+void HELPER(crypto_sha256h)(void *vd, void *vn, void *vm, uint32_t desc)
{
uint64_t *rd = vd;
uint64_t *rn = vn;
@@ -388,9 +392,11 @@ void HELPER(crypto_sha256h)(void *vd, void *vn, void *vm)
rd[0] = d.l[0];
rd[1] = d.l[1];
+
+clear_tail_16(vd, desc);
}
-void HELPER(crypto_sha256h2)(void *vd, void *vn, void *vm)
+void HELPER(crypto_sha256h2)(void *vd, void *vn, void *vm, uint32_t desc)
{
uint64_t *rd = vd;
uint64_t *rn = vn;
@@ -413,9 +419,11 @@ void HELPER(crypto_sha256h2)(void *vd, void *vn, void *vm)
rd[0] = d.l[0];
rd[1] = d.l[1];
+
+clear_tail_16(vd, desc);
}
-void HELPER(crypto_sha256su0)(void *vd, void *vm)
+void HELPER(crypto_sha256su0)(void *vd, void *vm, uint32_t desc)
{
uint64_t *rd = vd;
uint64_t *rm = vm;
@@ -429,9 +437,11 @@ void HELPER(crypto_sha256su0)(void *vd, void *vm)
rd[0] = d.l[0];
rd[1] = d.l[1];
+
+clear_tail_16(vd, desc);
}
-void HELPER(crypto_sha256su1)(void *vd, void *vn, void *vm)
+void HELPER(crypto_sha256su1)(void *vd, void *vn, void *vm, uint32_t desc)
{
uint64_t *rd = vd;
uint64_t *rn = vn;
@@ -447,6 +457,8 @@ void HELPER(crypto_sha256su1)(void *vd, void *vn, void *vm)
rd[0] = d.l[0];
rd[1] = d.l[1];
+
+clear_tail_16(vd, desc);
}
/*
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index c737a409d0..48f71e01e4 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -14517,8 +14517,7 @@ static void disas_crypto_three_reg_sha(DisasContext *s,
uint32_t insn)
int rm = extract32(insn, 16, 5);
int rn = extract32(insn, 5, 5);
int rd = extract32(insn, 0, 5);
-CryptoThreeOpFn *genfn;
-TCGv_ptr tcg_rd_ptr, tcg_rn_ptr, tcg_rm_ptr;
+gen_helper_gvec_3 *genfn;
bool feature;
if (size != 0) {
@@ -14560,23 +14559,22 @@ static void disas_crypto_three_reg_sha(DisasContext
*s, uint32_t insn)
return;
}
-tcg_rd_ptr =
[PATCH v3 5/9] target/arm: Split helper_crypto_sha1_3reg
Rather than passing an opcode to a helper, fully decode the
operation at translate time. Use clear_tail_16 to zap the
balance of the SVE register with the AdvSIMD write.
Signed-off-by: Richard Henderson
---
target/arm/helper.h| 5 +-
target/arm/crypto_helper.c | 99 --
target/arm/translate-a64.c | 29 +--
target/arm/translate.c | 70 +++
4 files changed, 116 insertions(+), 87 deletions(-)
diff --git a/target/arm/helper.h b/target/arm/helper.h
index 07466ddc6c..24aca28a05 100644
--- a/target/arm/helper.h
+++ b/target/arm/helper.h
@@ -516,7 +516,10 @@ DEF_HELPER_FLAGS_2(neon_qzip32, TCG_CALL_NO_RWG, void,
ptr, ptr)
DEF_HELPER_FLAGS_4(crypto_aese, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
DEF_HELPER_FLAGS_3(crypto_aesmc, TCG_CALL_NO_RWG, void, ptr, ptr, i32)
-DEF_HELPER_FLAGS_4(crypto_sha1_3reg, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
+DEF_HELPER_FLAGS_4(crypto_sha1su0, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
+DEF_HELPER_FLAGS_4(crypto_sha1c, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
+DEF_HELPER_FLAGS_4(crypto_sha1p, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
+DEF_HELPER_FLAGS_4(crypto_sha1m, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
DEF_HELPER_FLAGS_3(crypto_sha1h, TCG_CALL_NO_RWG, void, ptr, ptr, i32)
DEF_HELPER_FLAGS_3(crypto_sha1su1, TCG_CALL_NO_RWG, void, ptr, ptr, i32)
diff --git a/target/arm/crypto_helper.c b/target/arm/crypto_helper.c
index 7124745c32..636683d0f1 100644
--- a/target/arm/crypto_helper.c
+++ b/target/arm/crypto_helper.c
@@ -24,11 +24,11 @@ union CRYPTO_STATE {
};
#ifdef HOST_WORDS_BIGENDIAN
-#define CR_ST_BYTE(state, i) (state.bytes[(15 - (i)) ^ 8])
-#define CR_ST_WORD(state, i) (state.words[(3 - (i)) ^ 2])
+#define CR_ST_BYTE(state, i) ((state).bytes[(15 - (i)) ^ 8])
+#define CR_ST_WORD(state, i) ((state).words[(3 - (i)) ^ 2])
#else
-#define CR_ST_BYTE(state, i) (state.bytes[i])
-#define CR_ST_WORD(state, i) (state.words[i])
+#define CR_ST_BYTE(state, i) ((state).bytes[i])
+#define CR_ST_WORD(state, i) ((state).words[i])
#endif
/*
@@ -258,49 +258,74 @@ static uint32_t maj(uint32_t x, uint32_t y, uint32_t z)
return (x & y) | ((x | y) & z);
}
-void HELPER(crypto_sha1_3reg)(void *vd, void *vn, void *vm, uint32_t op)
+void HELPER(crypto_sha1su0)(void *vd, void *vn, void *vm, uint32_t desc)
+{
+uint64_t *d = vd, *n = vn, *m = vm;
+uint64_t d0, d1;
+
+d0 = d[1] ^ d[0] ^ m[0];
+d1 = n[0] ^ d[1] ^ m[1];
+d[0] = d0;
+d[1] = d1;
+
+clear_tail_16(vd, desc);
+}
+
+static inline void crypto_sha1_3reg(uint64_t *rd, uint64_t *rn,
+uint64_t *rm, uint32_t desc,
+uint32_t (*fn)(union CRYPTO_STATE *d))
{
-uint64_t *rd = vd;
-uint64_t *rn = vn;
-uint64_t *rm = vm;
union CRYPTO_STATE d = { .l = { rd[0], rd[1] } };
union CRYPTO_STATE n = { .l = { rn[0], rn[1] } };
union CRYPTO_STATE m = { .l = { rm[0], rm[1] } };
+int i;
-if (op == 3) { /* sha1su0 */
-d.l[0] ^= d.l[1] ^ m.l[0];
-d.l[1] ^= n.l[0] ^ m.l[1];
-} else {
-int i;
+for (i = 0; i < 4; i++) {
+uint32_t t = fn();
-for (i = 0; i < 4; i++) {
-uint32_t t;
+t += rol32(CR_ST_WORD(d, 0), 5) + CR_ST_WORD(n, 0)
+ + CR_ST_WORD(m, i);
-switch (op) {
-case 0: /* sha1c */
-t = cho(CR_ST_WORD(d, 1), CR_ST_WORD(d, 2), CR_ST_WORD(d, 3));
-break;
-case 1: /* sha1p */
-t = par(CR_ST_WORD(d, 1), CR_ST_WORD(d, 2), CR_ST_WORD(d, 3));
-break;
-case 2: /* sha1m */
-t = maj(CR_ST_WORD(d, 1), CR_ST_WORD(d, 2), CR_ST_WORD(d, 3));
-break;
-default:
-g_assert_not_reached();
-}
-t += rol32(CR_ST_WORD(d, 0), 5) + CR_ST_WORD(n, 0)
- + CR_ST_WORD(m, i);
-
-CR_ST_WORD(n, 0) = CR_ST_WORD(d, 3);
-CR_ST_WORD(d, 3) = CR_ST_WORD(d, 2);
-CR_ST_WORD(d, 2) = ror32(CR_ST_WORD(d, 1), 2);
-CR_ST_WORD(d, 1) = CR_ST_WORD(d, 0);
-CR_ST_WORD(d, 0) = t;
-}
+CR_ST_WORD(n, 0) = CR_ST_WORD(d, 3);
+CR_ST_WORD(d, 3) = CR_ST_WORD(d, 2);
+CR_ST_WORD(d, 2) = ror32(CR_ST_WORD(d, 1), 2);
+CR_ST_WORD(d, 1) = CR_ST_WORD(d, 0);
+CR_ST_WORD(d, 0) = t;
}
rd[0] = d.l[0];
rd[1] = d.l[1];
+
+clear_tail_16(rd, desc);
+}
+
+static uint32_t do_sha1c(union CRYPTO_STATE *d)
+{
+return cho(CR_ST_WORD(*d, 1), CR_ST_WORD(*d, 2), CR_ST_WORD(*d, 3));
+}
+
+void HELPER(crypto_sha1c)(void *vd, void *vn, void *vm, uint32_t desc)
+{
+crypto_sha1_3reg(vd, vn, vm, desc, do_sha1c);
+}
+
+static uint32_t do_sha1p(union CRYPTO_STATE *d)
+{
+return par(CR_ST_WORD(*d, 1), CR_ST_WORD(*d, 2), CR_ST_WORD(*d, 3));
+}
+
+void
[PATCH v3 2/9] target/arm: Convert rax1 to gvec helpers
With this conversion, we will be able to use the same helpers
with sve. This also fixes a bug in which we failed to clear
the high bits of the SVE register after an AdvSIMD operation.
Signed-off-by: Richard Henderson
---
target/arm/helper.h| 2 ++
target/arm/translate-a64.h | 2 ++
target/arm/crypto_helper.c | 11
target/arm/translate-a64.c | 53 ++
4 files changed, 40 insertions(+), 28 deletions(-)
diff --git a/target/arm/helper.h b/target/arm/helper.h
index 6623b6689a..96cf4464be 100644
--- a/target/arm/helper.h
+++ b/target/arm/helper.h
@@ -537,6 +537,8 @@ DEF_HELPER_FLAGS_3(crypto_sm3partw2, TCG_CALL_NO_RWG, void,
ptr, ptr, ptr)
DEF_HELPER_FLAGS_4(crypto_sm4e, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
DEF_HELPER_FLAGS_4(crypto_sm4ekey, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
+DEF_HELPER_FLAGS_4(crypto_rax1, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
+
DEF_HELPER_FLAGS_3(crc32, TCG_CALL_NO_RWG_SE, i32, i32, i32, i32)
DEF_HELPER_FLAGS_3(crc32c, TCG_CALL_NO_RWG_SE, i32, i32, i32, i32)
diff --git a/target/arm/translate-a64.h b/target/arm/translate-a64.h
index 84c40377bd..f2250a8dd1 100644
--- a/target/arm/translate-a64.h
+++ b/target/arm/translate-a64.h
@@ -141,4 +141,6 @@ void arm_gen_gvec_xar(unsigned vece, uint32_t rd_ofs,
uint32_t rn_ofs,
uint32_t rm_ofs, int64_t shift,
uint32_t opr_sz, uint32_t max_sz);
+extern const GVecGen3 rax1_op;
+
#endif /* TARGET_ARM_TRANSLATE_A64_H */
diff --git a/target/arm/crypto_helper.c b/target/arm/crypto_helper.c
index 6bd5a3d2d0..372d8350e4 100644
--- a/target/arm/crypto_helper.c
+++ b/target/arm/crypto_helper.c
@@ -725,3 +725,14 @@ void HELPER(crypto_sm4ekey)(void *vd, void *vn, void* vm,
uint32_t desc)
}
clear_tail(vd, opr_sz, simd_maxsz(desc));
}
+
+void HELPER(crypto_rax1)(void *vd, void *vn, void *vm, uint32_t desc)
+{
+intptr_t i, opr_sz = simd_oprsz(desc);
+uint64_t *d = vd, *n = vn, *m = vm;
+
+for (i = 0; i < opr_sz / 8; ++i) {
+d[i] = n[i] ^ rol64(m[i], 1);
+}
+clear_tail(vd, opr_sz, simd_maxsz(desc));
+}
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index b2adf3a39e..2eb4315b6d 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -14636,6 +14636,29 @@ static void disas_crypto_two_reg_sha(DisasContext *s,
uint32_t insn)
tcg_temp_free_ptr(tcg_rn_ptr);
}
+static void gen_rax1_i64(TCGv_i64 d, TCGv_i64 n, TCGv_i64 m)
+{
+tcg_gen_rotli_i64(d, m, 1);
+tcg_gen_xor_i64(d, d, n);
+}
+
+static void gen_rax1_vec(unsigned vece, TCGv_vec d, TCGv_vec n, TCGv_vec m)
+{
+tcg_gen_rotli_vec(vece, d, m, 1);
+tcg_gen_xor_vec(vece, d, d, n);
+}
+
+static const TCGOpcode rax1_opc[] = { INDEX_op_rotli_vec, 0 };
+
+const GVecGen3 rax1_op =
+{
+.fni8 = gen_rax1_i64,
+.fniv = gen_rax1_vec,
+.opt_opc = rax1_opc,
+.fno = gen_helper_crypto_rax1,
+.vece = MO_64,
+};
+
/* Crypto three-reg SHA512
* 31 21 20 16 15 14 13 12 11 10 95 40
* +---+--+---+---+-++--+--+
@@ -14670,7 +14693,7 @@ static void disas_crypto_three_reg_sha512(DisasContext
*s, uint32_t insn)
break;
case 3: /* RAX1 */
feature = dc_isar_feature(aa64_sha3, s);
-genfn = NULL;
+gvecop = _op;
break;
default:
g_assert_not_reached();
@@ -14706,10 +14729,7 @@ static void disas_crypto_three_reg_sha512(DisasContext
*s, uint32_t insn)
if (gvecop) {
gen_gvec_op3(s, true, rd, rn, rm, gvecop);
-return;
-}
-
-if (genfn) {
+} else {
TCGv_ptr tcg_rd_ptr, tcg_rn_ptr, tcg_rm_ptr;
tcg_rd_ptr = vec_full_reg_ptr(s, rd);
@@ -14721,29 +14741,6 @@ static void disas_crypto_three_reg_sha512(DisasContext
*s, uint32_t insn)
tcg_temp_free_ptr(tcg_rd_ptr);
tcg_temp_free_ptr(tcg_rn_ptr);
tcg_temp_free_ptr(tcg_rm_ptr);
-} else {
-TCGv_i64 tcg_op1, tcg_op2, tcg_res[2];
-int pass;
-
-tcg_op1 = tcg_temp_new_i64();
-tcg_op2 = tcg_temp_new_i64();
-tcg_res[0] = tcg_temp_new_i64();
-tcg_res[1] = tcg_temp_new_i64();
-
-for (pass = 0; pass < 2; pass++) {
-read_vec_element(s, tcg_op1, rn, pass, MO_64);
-read_vec_element(s, tcg_op2, rm, pass, MO_64);
-
-tcg_gen_rotli_i64(tcg_res[pass], tcg_op2, 1);
-tcg_gen_xor_i64(tcg_res[pass], tcg_res[pass], tcg_op1);
-}
-write_vec_element(s, tcg_res[0], rd, 0, MO_64);
-write_vec_element(s, tcg_res[1], rd, 1, MO_64);
-
-tcg_temp_free_i64(tcg_op1);
-tcg_temp_free_i64(tcg_op2);
-tcg_temp_free_i64(tcg_res[0]);
-tcg_temp_free_i64(tcg_res[1]);
}
}
--
2.20.1
[PATCH v3 9/9] target/arm: Implement SVE2 crypto constructive binary operations
Signed-off-by: Richard Henderson
---
target/arm/cpu.h | 5 +
target/arm/sve.decode | 4
target/arm/translate-sve.c | 20
3 files changed, 29 insertions(+)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 4f09dd42ba..0a7c68843b 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -3870,6 +3870,11 @@ static inline bool isar_feature_aa64_sve2_bitperm(const
ARMISARegisters *id)
return FIELD_EX64(id->id_aa64zfr0, ID_AA64ZFR0, BITPERM) != 0;
}
+static inline bool isar_feature_aa64_sve2_sha3(const ARMISARegisters *id)
+{
+return FIELD_EX64(id->id_aa64zfr0, ID_AA64ZFR0, SHA3) != 0;
+}
+
static inline bool isar_feature_aa64_sve2_sm4(const ARMISARegisters *id)
{
return FIELD_EX64(id->id_aa64zfr0, ID_AA64ZFR0, SM4) != 0;
diff --git a/target/arm/sve.decode b/target/arm/sve.decode
index 149de7949b..3cf824bac5 100644
--- a/target/arm/sve.decode
+++ b/target/arm/sve.decode
@@ -1564,3 +1564,7 @@ AESMC 01000101 00 1011100 decrypt:1 0
rd:5
AESE01000101 00 10001 0 11100 0 . . @rdn_rm_e0
AESD01000101 00 10001 0 11100 1 . . @rdn_rm_e0
SM4E01000101 00 10001 1 11100 0 . . @rdn_rm_e0
+
+# SVE2 crypto constructive binary operations
+SM4EKEY 01000101 00 1 . 0 0 . . @rd_rn_rm_e0
+RAX101000101 00 1 . 0 1 . . @rd_rn_rm_e0
diff --git a/target/arm/translate-sve.c b/target/arm/translate-sve.c
index 91e71882d6..a8e57ea5f4 100644
--- a/target/arm/translate-sve.c
+++ b/target/arm/translate-sve.c
@@ -8233,3 +8233,23 @@ static bool trans_SM4E(DisasContext *s, arg_rrr_esz *a)
{
return do_sm4(s, a, gen_helper_crypto_sm4e);
}
+
+static bool trans_SM4EKEY(DisasContext *s, arg_rrr_esz *a)
+{
+return do_sm4(s, a, gen_helper_crypto_sm4ekey);
+}
+
+static bool trans_RAX1(DisasContext *s, arg_rrr_esz *a)
+{
+if (!dc_isar_feature(aa64_sve2_sha3, s)) {
+return false;
+}
+if (sve_access_check(s)) {
+unsigned vsz = vec_full_reg_size(s);
+tcg_gen_gvec_3(vec_full_reg_offset(s, a->rd),
+ vec_full_reg_offset(s, a->rn),
+ vec_full_reg_offset(s, a->rm),
+ vsz, vsz, _op);
+}
+return true;
+}
--
2.20.1
[PATCH v3 3/9] target/arm: Convert sha512 and sm3 to gvec helpers
Do not yet convert the helpers to loop over opr_sz, but the
descriptor allows the vector tail to be cleared. Which fixes
an existing bug.
Signed-off-by: Richard Henderson
---
target/arm/helper.h| 15 +
target/arm/crypto_helper.c | 37 +++
target/arm/translate-a64.c | 62 +++---
3 files changed, 64 insertions(+), 50 deletions(-)
diff --git a/target/arm/helper.h b/target/arm/helper.h
index 96cf4464be..d564747808 100644
--- a/target/arm/helper.h
+++ b/target/arm/helper.h
@@ -525,14 +525,17 @@ DEF_HELPER_FLAGS_3(crypto_sha256h2, TCG_CALL_NO_RWG,
void, ptr, ptr, ptr)
DEF_HELPER_FLAGS_2(crypto_sha256su0, TCG_CALL_NO_RWG, void, ptr, ptr)
DEF_HELPER_FLAGS_3(crypto_sha256su1, TCG_CALL_NO_RWG, void, ptr, ptr, ptr)
-DEF_HELPER_FLAGS_3(crypto_sha512h, TCG_CALL_NO_RWG, void, ptr, ptr, ptr)
-DEF_HELPER_FLAGS_3(crypto_sha512h2, TCG_CALL_NO_RWG, void, ptr, ptr, ptr)
-DEF_HELPER_FLAGS_2(crypto_sha512su0, TCG_CALL_NO_RWG, void, ptr, ptr)
-DEF_HELPER_FLAGS_3(crypto_sha512su1, TCG_CALL_NO_RWG, void, ptr, ptr, ptr)
+DEF_HELPER_FLAGS_4(crypto_sha512h, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
+DEF_HELPER_FLAGS_4(crypto_sha512h2, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
+DEF_HELPER_FLAGS_3(crypto_sha512su0, TCG_CALL_NO_RWG, void, ptr, ptr, i32)
+DEF_HELPER_FLAGS_4(crypto_sha512su1, TCG_CALL_NO_RWG,
+ void, ptr, ptr, ptr, i32)
DEF_HELPER_FLAGS_5(crypto_sm3tt, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32,
i32)
-DEF_HELPER_FLAGS_3(crypto_sm3partw1, TCG_CALL_NO_RWG, void, ptr, ptr, ptr)
-DEF_HELPER_FLAGS_3(crypto_sm3partw2, TCG_CALL_NO_RWG, void, ptr, ptr, ptr)
+DEF_HELPER_FLAGS_4(crypto_sm3partw1, TCG_CALL_NO_RWG,
+ void, ptr, ptr, ptr, i32)
+DEF_HELPER_FLAGS_4(crypto_sm3partw2, TCG_CALL_NO_RWG,
+ void, ptr, ptr, ptr, i32)
DEF_HELPER_FLAGS_4(crypto_sm4e, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
DEF_HELPER_FLAGS_4(crypto_sm4ekey, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
diff --git a/target/arm/crypto_helper.c b/target/arm/crypto_helper.c
index 372d8350e4..637e4c00bb 100644
--- a/target/arm/crypto_helper.c
+++ b/target/arm/crypto_helper.c
@@ -31,6 +31,19 @@ union CRYPTO_STATE {
#define CR_ST_WORD(state, i) (state.words[i])
#endif
+/*
+ * The caller has not been converted to full gvec, and so only
+ * modifies the low 16 bytes of the vector register.
+ */
+static void clear_tail_16(void *vd, uint32_t desc)
+{
+int opr_sz = simd_oprsz(desc);
+int max_sz = simd_maxsz(desc);
+
+assert(opr_sz == 16);
+clear_tail(vd, opr_sz, max_sz);
+}
+
static void do_crypto_aese(uint64_t *rd, uint64_t *rn,
uint64_t *rm, bool decrypt)
{
@@ -470,7 +483,7 @@ static uint64_t s1_512(uint64_t x)
return ror64(x, 19) ^ ror64(x, 61) ^ (x >> 6);
}
-void HELPER(crypto_sha512h)(void *vd, void *vn, void *vm)
+void HELPER(crypto_sha512h)(void *vd, void *vn, void *vm, uint32_t desc)
{
uint64_t *rd = vd;
uint64_t *rn = vn;
@@ -483,9 +496,11 @@ void HELPER(crypto_sha512h)(void *vd, void *vn, void *vm)
rd[0] = d0;
rd[1] = d1;
+
+clear_tail_16(vd, desc);
}
-void HELPER(crypto_sha512h2)(void *vd, void *vn, void *vm)
+void HELPER(crypto_sha512h2)(void *vd, void *vn, void *vm, uint32_t desc)
{
uint64_t *rd = vd;
uint64_t *rn = vn;
@@ -498,9 +513,11 @@ void HELPER(crypto_sha512h2)(void *vd, void *vn, void *vm)
rd[0] = d0;
rd[1] = d1;
+
+clear_tail_16(vd, desc);
}
-void HELPER(crypto_sha512su0)(void *vd, void *vn)
+void HELPER(crypto_sha512su0)(void *vd, void *vn, uint32_t desc)
{
uint64_t *rd = vd;
uint64_t *rn = vn;
@@ -512,9 +529,11 @@ void HELPER(crypto_sha512su0)(void *vd, void *vn)
rd[0] = d0;
rd[1] = d1;
+
+clear_tail_16(vd, desc);
}
-void HELPER(crypto_sha512su1)(void *vd, void *vn, void *vm)
+void HELPER(crypto_sha512su1)(void *vd, void *vn, void *vm, uint32_t desc)
{
uint64_t *rd = vd;
uint64_t *rn = vn;
@@ -522,9 +541,11 @@ void HELPER(crypto_sha512su1)(void *vd, void *vn, void *vm)
rd[0] += s1_512(rn[0]) + rm[0];
rd[1] += s1_512(rn[1]) + rm[1];
+
+clear_tail_16(vd, desc);
}
-void HELPER(crypto_sm3partw1)(void *vd, void *vn, void *vm)
+void HELPER(crypto_sm3partw1)(void *vd, void *vn, void *vm, uint32_t desc)
{
uint64_t *rd = vd;
uint64_t *rn = vn;
@@ -548,9 +569,11 @@ void HELPER(crypto_sm3partw1)(void *vd, void *vn, void *vm)
rd[0] = d.l[0];
rd[1] = d.l[1];
+
+clear_tail_16(vd, desc);
}
-void HELPER(crypto_sm3partw2)(void *vd, void *vn, void *vm)
+void HELPER(crypto_sm3partw2)(void *vd, void *vn, void *vm, uint32_t desc)
{
uint64_t *rd = vd;
uint64_t *rn = vn;
@@ -568,6 +591,8 @@ void HELPER(crypto_sm3partw2)(void *vd, void *vn, void *vm)
rd[0] = d.l[0];
rd[1] = d.l[1];
+
+clear_tail_16(vd, desc);
}
void HELPER(crypto_sm3tt)(void *vd, void *vn, void *vm, uint32_t imm2,
diff --git
[PATCH v3 1/9] target/arm: Convert aes and sm4 to gvec helpers
With this conversion, we will be able to use the same helpers
with sve. This also fixes a bug in which we failed to clear
the high bits of the SVE register after an AdvSIMD operation.
Signed-off-by: Richard Henderson
---
target/arm/helper.h| 6 ++--
target/arm/vec_internal.h | 10 ++
target/arm/crypto_helper.c | 72 +++---
target/arm/translate-a64.c | 56 ++---
target/arm/translate.c | 27 +++---
target/arm/vec_helper.c| 10 --
6 files changed, 115 insertions(+), 66 deletions(-)
diff --git a/target/arm/helper.h b/target/arm/helper.h
index 032d5cdfbd..6623b6689a 100644
--- a/target/arm/helper.h
+++ b/target/arm/helper.h
@@ -513,7 +513,7 @@ DEF_HELPER_FLAGS_2(neon_qzip8, TCG_CALL_NO_RWG, void, ptr,
ptr)
DEF_HELPER_FLAGS_2(neon_qzip16, TCG_CALL_NO_RWG, void, ptr, ptr)
DEF_HELPER_FLAGS_2(neon_qzip32, TCG_CALL_NO_RWG, void, ptr, ptr)
-DEF_HELPER_FLAGS_3(crypto_aese, TCG_CALL_NO_RWG, void, ptr, ptr, i32)
+DEF_HELPER_FLAGS_4(crypto_aese, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
DEF_HELPER_FLAGS_3(crypto_aesmc, TCG_CALL_NO_RWG, void, ptr, ptr, i32)
DEF_HELPER_FLAGS_4(crypto_sha1_3reg, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
@@ -534,8 +534,8 @@ DEF_HELPER_FLAGS_5(crypto_sm3tt, TCG_CALL_NO_RWG, void,
ptr, ptr, ptr, i32, i32)
DEF_HELPER_FLAGS_3(crypto_sm3partw1, TCG_CALL_NO_RWG, void, ptr, ptr, ptr)
DEF_HELPER_FLAGS_3(crypto_sm3partw2, TCG_CALL_NO_RWG, void, ptr, ptr, ptr)
-DEF_HELPER_FLAGS_2(crypto_sm4e, TCG_CALL_NO_RWG, void, ptr, ptr)
-DEF_HELPER_FLAGS_3(crypto_sm4ekey, TCG_CALL_NO_RWG, void, ptr, ptr, ptr)
+DEF_HELPER_FLAGS_4(crypto_sm4e, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
+DEF_HELPER_FLAGS_4(crypto_sm4ekey, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
DEF_HELPER_FLAGS_3(crc32, TCG_CALL_NO_RWG_SE, i32, i32, i32, i32)
DEF_HELPER_FLAGS_3(crc32c, TCG_CALL_NO_RWG_SE, i32, i32, i32, i32)
diff --git a/target/arm/vec_internal.h b/target/arm/vec_internal.h
index ac365123c4..7eaffeb060 100644
--- a/target/arm/vec_internal.h
+++ b/target/arm/vec_internal.h
@@ -20,6 +20,16 @@
#ifndef TARGET_ARM_VEC_INTERNALS_H
#define TARGET_ARM_VEC_INTERNALS_H
+static inline void clear_tail(void *vd, uintptr_t opr_sz, uintptr_t max_sz)
+{
+uint64_t *d = vd + opr_sz;
+uintptr_t i;
+
+for (i = opr_sz; i < max_sz; i += 8) {
+*d++ = 0;
+}
+}
+
static inline int32_t do_sqrshl_bhs(int32_t src, int32_t shift, int bits,
bool round, uint32_t *sat)
{
diff --git a/target/arm/crypto_helper.c b/target/arm/crypto_helper.c
index f800266727..6bd5a3d2d0 100644
--- a/target/arm/crypto_helper.c
+++ b/target/arm/crypto_helper.c
@@ -13,7 +13,9 @@
#include "cpu.h"
#include "exec/helper-proto.h"
+#include "tcg/tcg-gvec-desc.h"
#include "crypto/aes.h"
+#include "vec_internal.h"
union CRYPTO_STATE {
uint8_tbytes[16];
@@ -29,18 +31,15 @@ union CRYPTO_STATE {
#define CR_ST_WORD(state, i) (state.words[i])
#endif
-void HELPER(crypto_aese)(void *vd, void *vm, uint32_t decrypt)
+static void do_crypto_aese(uint64_t *rd, uint64_t *rn,
+ uint64_t *rm, bool decrypt)
{
static uint8_t const * const sbox[2] = { AES_sbox, AES_isbox };
static uint8_t const * const shift[2] = { AES_shifts, AES_ishifts };
-uint64_t *rd = vd;
-uint64_t *rm = vm;
union CRYPTO_STATE rk = { .l = { rm[0], rm[1] } };
-union CRYPTO_STATE st = { .l = { rd[0], rd[1] } };
+union CRYPTO_STATE st = { .l = { rn[0], rn[1] } };
int i;
-assert(decrypt < 2);
-
/* xor state vector with round key */
rk.l[0] ^= st.l[0];
rk.l[1] ^= st.l[1];
@@ -54,7 +53,18 @@ void HELPER(crypto_aese)(void *vd, void *vm, uint32_t
decrypt)
rd[1] = st.l[1];
}
-void HELPER(crypto_aesmc)(void *vd, void *vm, uint32_t decrypt)
+void HELPER(crypto_aese)(void *vd, void *vn, void *vm, uint32_t desc)
+{
+intptr_t i, opr_sz = simd_oprsz(desc);
+bool decrypt = simd_data(desc);
+
+for (i = 0; i < opr_sz; i += 16) {
+do_crypto_aese(vd + i, vn + i, vm + i, decrypt);
+}
+clear_tail(vd, opr_sz, simd_maxsz(desc));
+}
+
+static void do_crypto_aesmc(uint64_t *rd, uint64_t *rm, bool decrypt)
{
static uint32_t const mc[][256] = { {
/* MixColumns lookup table */
@@ -190,13 +200,9 @@ void HELPER(crypto_aesmc)(void *vd, void *vm, uint32_t
decrypt)
0xbe805d9f, 0xb58d5491, 0xa89a4f83, 0xa397468d,
} };
-uint64_t *rd = vd;
-uint64_t *rm = vm;
union CRYPTO_STATE st = { .l = { rm[0], rm[1] } };
int i;
-assert(decrypt < 2);
-
for (i = 0; i < 16; i += 4) {
CR_ST_WORD(st, i >> 2) =
mc[decrypt][CR_ST_BYTE(st, i)] ^
@@ -209,6 +215,17 @@ void HELPER(crypto_aesmc)(void *vd, void *vm, uint32_t
decrypt)
rd[1] = st.l[1];
}
+void HELPER(crypto_aesmc)(void *vd, void *vm, uint32_t desc)
+{
+intptr_t i, opr_sz = simd_oprsz(desc);
+bool decrypt =
[PATCH v3 0/9] target/arm: Implement SVE2 Crypto Extensions
Stephen, what I was looking for when I talked about modifying the existing helpers is patch 1, which simplifies the final 3 patches for the new SVE2 insns. In the process I found that the existing implementation of the AdvSIMD insns is buggy wrt SVE. We need to clear the bits in the Zreg destination beyond the first 128. I could have done this via clear_vec_high in translate-a64.c, but since we already have a function call, we're better off doing the clearing out of line. This means adding a desc parameter so that we know the total vector length. r~ Based-on: https://github.com/rth7680/qemu/commit/8c3a91e4e487ef840193a489e5457165530666fc Richard Henderson (9): target/arm: Convert aes and sm4 to gvec helpers target/arm: Convert rax1 to gvec helpers target/arm: Convert sha512 and sm3 to gvec helpers target/arm: Convert sha1 and sha256 to gvec helpers target/arm: Split helper_crypto_sha1_3reg target/arm: Split helper_crypto_sm3tt target/arm: Implement SVE2 crypto unary operations target/arm: Implement SVE2 crypto destructive binary operations target/arm: Implement SVE2 crypto constructive binary operations target/arm/cpu.h | 10 ++ target/arm/helper.h| 45 --- target/arm/translate-a64.h | 2 + target/arm/vec_internal.h | 10 ++ target/arm/sve.decode | 17 +++ target/arm/crypto_helper.c | 267 ++--- target/arm/translate-a64.c | 211 + target/arm/translate-sve.c | 79 +++ target/arm/translate.c | 125 + target/arm/vec_helper.c| 10 -- 10 files changed, 491 insertions(+), 285 deletions(-) -- 2.20.1
[PATCH v3] migration/xbzrle: add encoding rate
Users may need to check the xbzrle encoding rate to know if the guest
memory is xbzrle encoding-friendly, and dynamically turn off the
encoding if the encoding rate is low.
Signed-off-by: Yi Sun
Signed-off-by: Wei Wang
---
migration/migration.c | 1 +
migration/ram.c | 39 +--
monitor/hmp-cmds.c| 2 ++
qapi/migration.json | 5 -
4 files changed, 44 insertions(+), 3 deletions(-)
diff --git a/migration/migration.c b/migration/migration.c
index 187ac0410c..e40421353c 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -930,6 +930,7 @@ static void populate_ram_info(MigrationInfo *info,
MigrationState *s)
info->xbzrle_cache->pages = xbzrle_counters.pages;
info->xbzrle_cache->cache_miss = xbzrle_counters.cache_miss;
info->xbzrle_cache->cache_miss_rate = xbzrle_counters.cache_miss_rate;
+info->xbzrle_cache->encoding_rate = xbzrle_counters.encoding_rate;
info->xbzrle_cache->overflow = xbzrle_counters.overflow;
}
diff --git a/migration/ram.c b/migration/ram.c
index 04f13feb2e..41b75a0a0f 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -327,6 +327,10 @@ struct RAMState {
uint64_t num_dirty_pages_period;
/* xbzrle misses since the beginning of the period */
uint64_t xbzrle_cache_miss_prev;
+/* Amount of xbzrle pages since the beginning of the period */
+uint64_t xbzrle_pages_prev;
+/* Amount of xbzrle encoded bytes since the beginning of the period */
+uint64_t xbzrle_bytes_prev;
/* compression statistics since the beginning of the period */
/* amount of count that no free thread to compress data */
@@ -696,6 +700,18 @@ static int save_xbzrle_page(RAMState *rs, uint8_t
**current_data,
return -1;
}
+/*
+ * Reaching here means the page has hit the xbzrle cache, no matter what
+ * encoding result it is (normal encoding, overflow or skipping the page),
+ * count the page as encoded. This is used to caculate the encoding rate.
+ *
+ * Example: 2 pages (8KB) being encoded, first page encoding generates 2KB,
+ * 2nd page turns out to be skipped (i.e. no new bytes written to the
+ * page), the overall encoding rate will be 8KB / 2KB = 4, which has the
+ * skipped page included. In this way, the encoding rate can tell if the
+ * guest page is good for xbzrle encoding.
+ */
+xbzrle_counters.pages++;
prev_cached_page = get_cached_data(XBZRLE.cache, current_addr);
/* save current buffer into memory */
@@ -726,6 +742,7 @@ static int save_xbzrle_page(RAMState *rs, uint8_t
**current_data,
} else if (encoded_len == -1) {
trace_save_xbzrle_page_overflow();
xbzrle_counters.overflow++;
+xbzrle_counters.bytes += TARGET_PAGE_SIZE;
return -1;
}
@@ -736,8 +753,12 @@ static int save_xbzrle_page(RAMState *rs, uint8_t
**current_data,
qemu_put_be16(rs->f, encoded_len);
qemu_put_buffer(rs->f, XBZRLE.encoded_buf, encoded_len);
bytes_xbzrle += encoded_len + 1 + 2;
-xbzrle_counters.pages++;
-xbzrle_counters.bytes += bytes_xbzrle;
+/*
+ * Like compressed_size (please see update_compress_thread_counts),
+ * the xbzrle encoded bytes don't count the 8 byte header with
+ * RAM_SAVE_FLAG_CONTINUE.
+ */
+xbzrle_counters.bytes += bytes_xbzrle - 8;
ram_counters.transferred += bytes_xbzrle;
return 1;
@@ -870,9 +891,23 @@ static void migration_update_rates(RAMState *rs, int64_t
end_time)
}
if (migrate_use_xbzrle()) {
+double encoded_size, unencoded_size;
+
xbzrle_counters.cache_miss_rate = (double)(xbzrle_counters.cache_miss -
rs->xbzrle_cache_miss_prev) / page_count;
rs->xbzrle_cache_miss_prev = xbzrle_counters.cache_miss;
+unencoded_size = (xbzrle_counters.pages - rs->xbzrle_pages_prev) *
+ TARGET_PAGE_SIZE;
+encoded_size = xbzrle_counters.bytes - rs->xbzrle_bytes_prev;
+if (xbzrle_counters.pages == rs->xbzrle_pages_prev) {
+xbzrle_counters.encoding_rate = 0;
+} else if (!encoded_size) {
+xbzrle_counters.encoding_rate = UINT64_MAX;
+} else {
+xbzrle_counters.encoding_rate = unencoded_size / encoded_size;
+}
+rs->xbzrle_pages_prev = xbzrle_counters.pages;
+rs->xbzrle_bytes_prev = xbzrle_counters.bytes;
}
if (migrate_use_compression()) {
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 9b94e67879..c2a3a667ae 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -303,6 +303,8 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict)
info->xbzrle_cache->cache_miss);
monitor_printf(mon, "xbzrle cache miss rate: %0.2f\n",
info->xbzrle_cache->cache_miss_rate);
+monitor_printf(mon, "xbzrle encoding rate: %0.2f\n",
+
Re: [RFC PATCH v2] plugins: new lockstep plugin for debugging TCG changes
Patchew URL: https://patchew.org/QEMU/20200429200754.18327-1-alex.ben...@linaro.org/ Hi, This series seems to have some coding style problems. See output below for more information: Message-id: 20200429200754.18327-1-alex.ben...@linaro.org Subject: [RFC PATCH v2] plugins: new lockstep plugin for debugging TCG changes Type: series === 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 === Switched to a new branch 'test' 0f98ef2 plugins: new lockstep plugin for debugging TCG changes === OUTPUT BEGIN === WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? #54: new file mode 100644 WARNING: line over 80 characters #175: FILE: tests/plugin/lockstep.c:117: +g_string_append_printf(out, "We are @ %#016lx vs %#016lx\n", us->pc, them->pc); WARNING: line over 80 characters #179: FILE: tests/plugin/lockstep.c:121: +g_string_append_printf(out, " previously @ %#016lx/%ld (%ld insn, %ld blocks)\n", ERROR: do not use C99 // comments #259: FILE: tests/plugin/lockstep.c:201: +// save a reference so we can free later total: 1 errors, 3 warnings, 322 lines checked Commit 0f98ef2b5ce2 (plugins: new lockstep plugin for debugging TCG changes) 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/20200429200754.18327-1-alex.ben...@linaro.org/testing.checkpatch/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
Re: [PATCH v5 0/4] introduction of migration_version attribute for VFIO live migration
On Wed, Apr 29, 2020 at 10:13:01PM +0800, Eric Blake wrote: > [meta-comment] > > On 4/29/20 4:35 AM, Yan Zhao wrote: > > On Wed, Apr 29, 2020 at 04:22:01PM +0800, Dr. David Alan Gilbert wrote: > [...] > > This patchset introduces a migration_version attribute > > under sysfs > >>> of VFIO > > Mediated devices. > > Hmm, several pages with up to 16 levels of quoting, with editors making > the lines ragged, all before I get to the real meat of the email. > Remember, it's okay to trim content,... > > >> So why don't we split the difference; lets say that it should start with > >> the hex PCI Vendor ID. > >> > > The problem is for mdev devices, if the parent devices are not PCI devices, > > they don't have PCI vendor IDs. > > ...to just what you are replying to. > sorry for that. next time I'll try to make a better balance between keeping conversation background and leaving the real meat of the email. Thanks for reminding. Yan
Re: [PATCH v5 0/4] introduction of migration_version attribute for VFIO live migration
On Wed, Apr 29, 2020 at 05:48:44PM +0800, Dr. David Alan Gilbert wrote: > > > > > > > > > > > > > An mdev type is meant to define a software compatible > > > > > > > > > > > > > interface, so in > > > > > > > > > > > > > the case of mdev->mdev migration, doesn't migrating > > > > > > > > > > > > > to a different type > > > > > > > > > > > > > fail the most basic of compatibility tests that we > > > > > > > > > > > > > expect userspace to > > > > > > > > > > > > > perform? IOW, if two mdev types are migration > > > > > > > > > > > > > compatible, it seems a > > > > > > > > > > > > > prerequisite to that is that they provide the same > > > > > > > > > > > > > software interface, > > > > > > > > > > > > > which means they should be the same mdev type. > > > > > > > > > > > > > > > > > > > > > > > > > > In the hybrid cases of mdev->phys or phys->mdev, how > > > > > > > > > > > > > does a > > > > > > > > > > > > management > > > > > > > > > > > > > tool begin to even guess what might be compatible? > > > > > > > > > > > > > Are we expecting > > > > > > > > > > > > > libvirt to probe ever device with this attribute in > > > > > > > > > > > > > the system? Is > > > > > > > > > > > > > there going to be a new class hierarchy created to > > > > > > > > > > > > > enumerate all > > > > > > > > > > > > > possible migrate-able devices? > > > > > > > > > > > > > > > > > > > > > > > > > yes, management tool needs to guess and test migration > > > > > > > > > > > > compatible > > > > > > > > > > > > between two devices. But I think it's not the problem > > > > > > > > > > > > only for > > > > > > > > > > > > mdev->phys or phys->mdev. even for mdev->mdev, > > > > > > > > > > > > management tool needs > > > > > > > > > > > > to > > > > > > > > > > > > first assume that the two mdevs have the same type of > > > > > > > > > > > > parent devices > > > > > > > > > > > > (e.g.their pciids are equal). otherwise, it's still > > > > > > > > > > > > enumerating > > > > > > > > > > > > possibilities. > > > > > > > > > > > > > > > > > > > > > > > > on the other hand, for two mdevs, > > > > > > > > > > > > mdev1 from pdev1, its mdev_type is 1/2 of pdev1; > > > > > > > > > > > > mdev2 from pdev2, its mdev_type is 1/4 of pdev2; > > > > > > > > > > > > if pdev2 is exactly 2 times of pdev1, why not allow > > > > > > > > > > > > migration between > > > > > > > > > > > > mdev1 <-> mdev2. > > > > > > > > > > > > > > > > > > > > > > How could the manage tool figure out that 1/2 of pdev1 is > > > > > > > > > > > equivalent > > > > > > > > > > > to 1/4 of pdev2? If we really want to allow such thing > > > > > > > > > > > happen, the best > > > > > > > > > > > choice is to report the same mdev type on both pdev1 and > > > > > > > > > > > pdev2. > > > > > > > > > > I think that's exactly the value of this migration_version > > > > > > > > > > interface. > > > > > > > > > > the management tool can take advantage of this interface to > > > > > > > > > > know if two > > > > > > > > > > devices are migration compatible, no matter they are mdevs, > > > > > > > > > > non-mdevs, > > > > > > > > > > or mix. > > > > > > > > > > > > > > > > > > > > as I know, (please correct me if not right), current > > > > > > > > > > libvirt still > > > > > > > > > > requires manually generating mdev devices, and it just > > > > > > > > > > duplicates src vm > > > > > > > > > > configuration to the target vm. > > > > > > > > > > for libvirt, currently it's always phys->phys and > > > > > > > > > > mdev->mdev (and of the > > > > > > > > > > same mdev type). > > > > > > > > > > But it does not justify that hybrid cases should not be > > > > > > > > > > allowed. otherwise, > > > > > > > > > > why do we need to introduce this migration_version > > > > > > > > > > interface and leave > > > > > > > > > > the judgement of migration compatibility to vendor driver? > > > > > > > > > > why not simply > > > > > > > > > > set the criteria to something like "pciids of parent > > > > > > > > > > devices are equal, > > > > > > > > > > and mdev types are equal" ? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > btw mdev<->phys just brings trouble to upper stack as > > > > > > > > > > > Alex pointed out. > > > > > > > > > > could you help me understand why it will bring trouble to > > > > > > > > > > upper stack? > > > > > > > > > > > > > > > > > > > > I think it just needs to read src migration_version under > > > > > > > > > > src dev node, > > > > > > > > > > and test it in target migration version under target dev > > > > > > > > > > node. > > > > > > > > > > > > > > > > > > > > after all, through this interface we just help the upper > > > > > > > > > > layer > > > > > > > > > > knowing available options through reading and testing, and > > > > > > > > > > they decide > > > > > > > > > > to use it or not. > > > > > > > > > > > > > > > > > > > > > Can we simplify the requirement by allowing only > > > > > > > > > > >
Re: R: R: About hardfloat in ppc
Question, in hard-float, if we don't want to read the fp register. for example: If we wanna compute c = a + b in fp32 if c = a + b In hard float and if b1 = c - a in hard float if b1 != b at bitwise level, the we se the inexat to 1, otherwsie we set inexat bit to 0? are this valid? we can also do it for a * b, a - b, a / b. On Thu, Apr 30, 2020 at 2:25 AM Alex Bennée wrote: > > Dino Papararo writes: > > > Hi Alex, > > > > > I leave to you TCG's experts how it works and how to implement it, I'm > > only tryng to explain a possible fast way to go (if ever possible) > > This is all a theoretical discussion unless someone cares enough to > improve the situation. While I have an interest in improving TCG > performance I'm afraid there are many more easier wins before tackling a > target specific hack for which I'm not familiar. No doubt this thread > will be referred to next time someone wants something done about it. > > > ..Large majority of software don't check for exceptions at all and if > > I really want to pursue max precision I'll go for a software > > multiprecision library like GMP or MPFR Libraries. > > However for QEMU we regard failure to correctly emulate the architecture > as a bug - we don't code to common software patterns because there is > plenty of software out there that doesn't follow it. > > > So the hardfloats 'should' be set as first choice and only if > > instruction requires precision/error check process it in softfloats. > > Sure but someone will have to do the work to support that. > > -- > Alex Bennée > -- 此致 礼 罗勇刚 Yours sincerely, Yonggang Luo
Re: [PATCH v2 0/4] block: Do not call BlockDriver.bdrv_make_empty() directly
Patchew URL: https://patchew.org/QEMU/20200429141126.85159-1-mre...@redhat.com/ Hi, This series failed the asan build test. Please find the testing commands and their output below. If you have Docker installed, you can probably reproduce it locally. === TEST SCRIPT BEGIN === #!/bin/bash export ARCH=x86_64 make docker-image-fedora V=1 NETWORK=1 time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1 === TEST SCRIPT END === LINKqemu-nbd LINKqemu-storage-daemon LINKqemu-io /tmp/qemu-test/src/qemu-img.c:1071:27: error: implicit declaration of function 'blk_new_with_bs' is invalid in C99 [-Werror,-Wimplicit-function-declaration] old_backing_blk = blk_new_with_bs(bs, BLK_PERM_WRITE, BLK_PERM_ALL, ^ /tmp/qemu-test/src/qemu-img.c:1071:27: error: this function declaration is not a prototype [-Werror,-Wstrict-prototypes] /tmp/qemu-test/src/qemu-img.c:1071:25: error: incompatible integer to pointer conversion assigning to 'BlockBackend *' (aka 'struct BlockBackend *') from 'int' [-Werror,-Wint-conversion] old_backing_blk = blk_new_with_bs(bs, BLK_PERM_WRITE, BLK_PERM_ALL, ^ ~ 3 errors generated. make: *** [/tmp/qemu-test/src/rules.mak:69: qemu-img.o] Error 1 make: *** Waiting for unfinished jobs Traceback (most recent call last): File "./tests/docker/docker.py", line 664, in --- raise CalledProcessError(retcode, cmd) subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=0c317bc0372e46baa5e530cb5bffad05', '-u', '1003', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=x86_64-softmmu', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-mhzkjux4/src/docker-src.2020-04-29-19.38.18.14298:/var/tmp/qemu:z,ro', 'qemu:fedora', '/var/tmp/qemu/run', 'test-debug']' returned non-zero exit status 2. filter=--filter=label=com.qemu.instance.uuid=0c317bc0372e46baa5e530cb5bffad05 make[1]: *** [docker-run] Error 1 make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-mhzkjux4/src' make: *** [docker-run-test-debug@fedora] Error 2 real3m19.927s user0m8.122s The full log is available at http://patchew.org/logs/20200429141126.85159-1-mre...@redhat.com/testing.asan/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
Re: [PATCH v2 0/4] block: Do not call BlockDriver.bdrv_make_empty() directly
Patchew URL: https://patchew.org/QEMU/20200429141126.85159-1-mre...@redhat.com/ Hi, This series failed the docker-quick@centos7 build test. Please find the testing commands and their output below. If you have Docker installed, you can probably reproduce it locally. === TEST SCRIPT BEGIN === #!/bin/bash make docker-image-centos7 V=1 NETWORK=1 time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1 === TEST SCRIPT END === BUILD pc-bios/optionrom/pvh.raw SIGNpc-bios/optionrom/pvh.bin /tmp/qemu-test/src/qemu-img.c: In function 'img_commit': /tmp/qemu-test/src/qemu-img.c:1071:9: error: implicit declaration of function 'blk_new_with_bs' [-Werror=implicit-function-declaration] old_backing_blk = blk_new_with_bs(bs, BLK_PERM_WRITE, BLK_PERM_ALL, ^ /tmp/qemu-test/src/qemu-img.c:1071:9: error: nested extern declaration of 'blk_new_with_bs' [-Werror=nested-externs] /tmp/qemu-test/src/qemu-img.c:1071:25: error: assignment makes pointer from integer without a cast [-Werror] old_backing_blk = blk_new_with_bs(bs, BLK_PERM_WRITE, BLK_PERM_ALL, ^ cc1: all warnings being treated as errors make: *** [qemu-img.o] Error 1 make: *** Waiting for unfinished jobs make: *** wait: No child processes. Stop. Traceback (most recent call last): --- raise CalledProcessError(retcode, cmd) subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=70ea0da52ca941ecafb5efd2e18dfe6a', '-u', '1003', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-2faaaxl7/src/docker-src.2020-04-29-19.35.24.5998:/var/tmp/qemu:z,ro', 'qemu:centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit status 2. filter=--filter=label=com.qemu.instance.uuid=70ea0da52ca941ecafb5efd2e18dfe6a make[1]: *** [docker-run] Error 1 make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-2faaaxl7/src' make: *** [docker-run-test-quick@centos7] Error 2 real2m24.697s user0m7.953s The full log is available at http://patchew.org/logs/20200429141126.85159-1-mre...@redhat.com/testing.docker-quick@centos7/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
Re: [RFC PATCH 0/1] hw/audio: Make 'soundhw' command line option a QOM interface
Patchew URL: https://patchew.org/QEMU/20200429173352.29442-1-phi...@redhat.com/ Hi, This series failed the docker-mingw@fedora build test. Please find the testing commands and their output below. If you have Docker installed, you can probably reproduce it locally. === TEST SCRIPT BEGIN === #! /bin/bash export ARCH=x86_64 make docker-image-fedora V=1 NETWORK=1 time make docker-test-mingw@fedora J=14 NETWORK=1 === TEST SCRIPT END === LINKaarch64-softmmu/qemu-system-aarch64w.exe ../hw/audio/soundhw.o: In function `soundhw_create_entry': /tmp/qemu-test/src/hw/audio/soundhw.c:136: undefined reference to `isa_create_simple' collect2: error: ld returned 1 exit status make[1]: *** [Makefile:208: qemu-system-aarch64w.exe] Error 1 make: *** [Makefile:527: aarch64-softmmu/all] Error 2 make: *** Waiting for unfinished jobs GEN x86_64-softmmu/qemu-system-x86_64.exe Traceback (most recent call last): --- raise CalledProcessError(retcode, cmd) subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=3da0c9d83a224670b19a0d47e227af57', '-u', '1003', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-_fqhuldy/src/docker-src.2020-04-29-19.16.21.27499:/var/tmp/qemu:z,ro', 'qemu:fedora', '/var/tmp/qemu/run', 'test-mingw']' returned non-zero exit status 2. filter=--filter=label=com.qemu.instance.uuid=3da0c9d83a224670b19a0d47e227af57 make[1]: *** [docker-run] Error 1 make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-_fqhuldy/src' make: *** [docker-run-test-mingw@fedora] Error 2 real2m44.609s user0m7.898s The full log is available at http://patchew.org/logs/20200429173352.29442-1-phi...@redhat.com/testing.docker-mingw@fedora/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
Re: R: About hardfloat in ppc
On Wed, Apr 29, 2020 at 7:57 PM Alex Bennée wrote: > > Dino Papararo writes: > > > Hello, > > about handling of PPC fpu exceptions and Hard Floats support we could > consider a different approach for different instructions. > > i.e. not all fpu instructions take care about inexact or exceptions > bits: if I take a simple fadd f0,f1,f2 I'll copy value derived from adding > f1+f2 into f1 register and no one will check about inexact or exception > bits raised into FPSCR register. > > Instead if I'll take fadd. f0,f1,f2 the dot following the add > instructions means I want take inexact or exceptions bits into account. > > So I could use hard floats for first case and softfloats for second case. > > Could this be a fast solution to start implement hard floats for PPC?? > > While it may be true that normal software practice is not to read the > exception registers for every operation we can't base our emulation on > that. We must always be able to re-create the state of the exception > registers whenever they may be read by the program. There are 3 cases > this may happen: > > - a direct read of the inexact register > - checking the sigcontext of a synchronous exception (e.g. fault) > - checking the sigcontext of an asynchronous exception (e.g. timer/IPI) > > Given the way the translator works we can simplify the asynchronous case > because we know they are only ever delivered at the start of translated > blocks. We must have a fully rectified system state at the end of every > block. So lets consider some cases: > > fpOpA > clear flags > fpOpB > clear flags > fpOpC > read flags > I am thinking about a new way to do optimize if InstCombine are possible in tcg, like InstCombine in LLVM suppose we have clearFlagsFpOpA clearFlagsFpOpB clearFlagsFpOpC clearFlagsFpOpD Then we can instCombine into FpOpA FpObB FpOpC clearFlagsFpOpD, Are this would be a possible idea? I think TCG have BasicBlock, and we can optimize TCG at the basic block level. > > Assuming we know the fpOps can't generate exceptions we can know that > only fpOpC will ever generate a user visible floating point flags so we > can indeed use hardfloat for fpOpA and fpOpB. However if we see the > pattern: > > fpOpA > ld/st > clear flags > fpOpB > read flags > > we must have the fully rectified version of the flags because the ld/st > may fault. However it's not guaranteed it will fault so we could defer > the flag calculation for fpOpA until such time as we need it. The > easiest way would be to save the values going into the operation and > then re-run it in softfloat when required (hopefully never ;-). > > A lot will depend on the behaviour of the architecture. For example: > > fpOpA > fpOpB > read flags > > whether or not we need to be able to calculate the flags for fpOpA will > depend on if fpOpB completely resets the flags visible or if the result > is additive. > > So in short I think there may be scope for using hardfloat but it will > require knowledge of front-end knowing if it is safe to skip flag > calculation in particular cases. We might even need support within TCG > for saving (and marking) temporaries over potentially faulting > boundaries so these lazy evaluations can be done. We can certainly add a > fp-status less set of primitives to softfloat which can use the > hardfloat path when we know we are using normal numbers. > > > > > A little of documentation here: > http://mirror.informatimago.com/next/developer.apple.com/documentation/mac/PPCNumerics/PPCNumerics-154.html > > > > Regards, > > Dino Papararo > > > > -Messaggio originale- > > Da: Qemu-devel Per > conto di Alex Bennée > > Inviato: martedì 28 aprile 2020 10:37 > > A: luoyongg...@gmail.com > > Cc: qemu-...@nongnu.org; qemu-devel@nongnu.org > > Oggetto: Re: About hardfloat in ppc > > > > > > 罗勇刚(Yonggang Luo) writes: > > > >> I am confusing why only inexact are set then we can use hard-float. > > > > The inexact behaviour of the host hardware may be different from the > guest architecture we are trying to emulate and the host hardware may not > be configurable to emulate the guest mode. > > > > Have a look in softfloat.c and see all the places where > float_flag_inexact is set. Can you convince yourself that the host hardware > will do the same? > > > >> And PPC always clearing inexact flag before calling to soft-float > >> funcitons. so we can not optimize it with hard-float. > >> I need some resouces about ineact flag and why always clearing inexcat > >> in PPC FP simualtion. > > > > Because that is the behaviour of the PPC floating point unit. The > inexact flag will represent the last operation done. > > > >> I am looking for two possible solution: > >> 1. do not clear inexact flag in PPC simulation 2. even the inexact are > >> cleared, we can still use alternative hard-float. > >> > >> But now I am the beginner, Have no clue about all the things. > > > > Well you'll need to learn about floating point because these are
Re: [RFC PATCH 0/1] hw/audio: Make 'soundhw' command line option a QOM interface
Patchew URL: https://patchew.org/QEMU/20200429173352.29442-1-phi...@redhat.com/ Hi, This series failed the docker-quick@centos7 build test. Please find the testing commands and their output below. If you have Docker installed, you can probably reproduce it locally. === TEST SCRIPT BEGIN === #!/bin/bash make docker-image-centos7 V=1 NETWORK=1 time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1 === TEST SCRIPT END === LINKaarch64-softmmu/qemu-system-aarch64 ../hw/audio/soundhw.o: In function `soundhw_create_entry': /tmp/qemu-test/src/hw/audio/soundhw.c:136: undefined reference to `isa_create_simple' collect2: error: ld returned 1 exit status make[1]: *** [qemu-system-aarch64] Error 1 make: *** [aarch64-softmmu/all] Error 2 Traceback (most recent call last): File "./tests/docker/docker.py", line 664, in sys.exit(main()) --- raise CalledProcessError(retcode, cmd) subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=5de0a0c769b044a1956d7b5dd71fa689', '-u', '1001', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-u4j0jrib/src/docker-src.2020-04-29-19.08.08.8026:/var/tmp/qemu:z,ro', 'qemu:centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit status 2. filter=--filter=label=com.qemu.instance.uuid=5de0a0c769b044a1956d7b5dd71fa689 make[1]: *** [docker-run] Error 1 make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-u4j0jrib/src' make: *** [docker-run-test-quick@centos7] Error 2 real2m38.775s user0m8.234s The full log is available at http://patchew.org/logs/20200429173352.29442-1-phi...@redhat.com/testing.docker-quick@centos7/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
Re: [PATCH v2 06/17] block/io: support int64_t bytes in bdrv_aligned_pwritev()
On 4/27/20 3:23 AM, Vladimir Sementsov-Ogievskiy wrote:
We are generally moving to int64_t for both offset and bytes parameters
on all io paths. Prepare bdrv_aligned_pwritev() now (and convert the
dependencies: bdrv_co_write_req_prepare() and
bdrv_co_write_req_finish() to signed type bytes)
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
block/io.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/block/io.c b/block/io.c
index c8c30e3699..fe19e09034 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1854,7 +1854,7 @@ fail:
}
static inline int coroutine_fn
-bdrv_co_write_req_prepare(BdrvChild *child, int64_t offset, uint64_t bytes,
+bdrv_co_write_req_prepare(BdrvChild *child, int64_t offset, int64_t bytes,
BdrvTrackedRequest *req, int flags)
{
No change in size. First, check usage within function:
int64_t end_sector = DIV_ROUND_UP(offset + bytes, BDRV_SECTOR_SIZE);
Changes computation from uint64_t to int64_t. This causes a borderline
bug on images between INT64_MAX-511 and INT64_MAX (nbdkit can produce
such images over NBD, although they are atypical on disk), where
DIV_ROUND_UP() would give the right answer as uint64_t but a negative
answer with int64_t. As those images are not sector-aligned, maybe we
don't need to care?
all other uses appear to be within asserts related to offset+bytes being
positive, so that's what we should check for.
Callers:
bdrv_aligned_pwritev() - changed in this patch to 'int64_t', analyzed
below [1]
bdrv_co_pdiscard() - already passes 'int64_t', also checks for
offset+bytes overflow - safe
bdrv_co_copy_range_internal() - 'uint64_t', but already analyzed for
3/17 how it was capped < 2M - safe
bdrv_co_truncate() - already passes 'int64_t', passes new_bytes computed
by subtracting from a positive 'int64_t offset' - safe
[1] except I hit the end of my work day, so my analysis will have to
continue tomorrow...
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
Re: [PATCHv2] add optee dts entry for secure machine
Kindly ping to merge this patch.
For more detail. This patch creates a dtb entry to load the optee
driver which is needed for secure boot (atf+optee+uboot+linux). Kernel
part is already there:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/tee/optee/core.c?h=v5.7-rc3#n705
.compatible = "linaro,optee-tz"
Thank you,
Maxim.
On Wed, 22 Apr 2020 at 16:07, Maxim Uvarov wrote:
>
> Add optee compatible string for dtb to force linux
> to boot optee module.
>
> Signed-off-by: Maxim Uvarov
> ---
> v2: added method.
> hw/arm/virt.c | 18 ++
> 1 file changed, 18 insertions(+)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 656b008..c937a82 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -1335,6 +1335,23 @@ static void create_platform_bus(VirtMachineState *vms)
> sysbus_mmio_get_region(s, 0));
> }
>
> +static void create_secure_tee(VirtMachineState *vms)
> +{
> +char *firmware;
> +char *optee;
> +
> +firmware = g_strdup_printf("/firmware");
> +qemu_fdt_add_subnode(vms->fdt, firmware);
> +
> +optee = g_strdup_printf("/firmware/optee");
> +qemu_fdt_add_subnode(vms->fdt, optee);
> +qemu_fdt_setprop_string(vms->fdt, optee, "compatible",
> "linaro,optee-tz");
> +qemu_fdt_setprop_string(vms->fdt, optee, "method", "smc");
> +
> +g_free(optee);
> +g_free(firmware);
> +}
> +
> static void create_secure_ram(VirtMachineState *vms,
>MemoryRegion *secure_sysmem)
> {
> @@ -1720,6 +1737,7 @@ static void machvirt_init(MachineState *machine)
> if (vms->secure) {
> create_secure_ram(vms, secure_sysmem);
> create_uart(vms, VIRT_SECURE_UART, secure_sysmem, serial_hd(1));
> +create_secure_tee(vms);
> }
>
> vms->highmem_ecam &= vms->highmem && (!firmware_loaded || aarch64);
> --
> 2.17.1
>
Re: [PATCH v2 05/17] block/io: support int64_t bytes in bdrv_co_do_pwrite_zeroes()
On 4/27/20 3:23 AM, Vladimir Sementsov-Ogievskiy wrote:
We are generally moving to int64_t for both offset and bytes parameters
on all io paths. Prepare bdrv_co_do_pwrite_zeroes() now.
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
block/io.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/block/io.c b/block/io.c
index 4796476835..c8c30e3699 100644
--- a/block/io.c
+++ b/block/io.c
@@ -42,7 +42,7 @@
static void bdrv_parent_cb_resize(BlockDriverState *bs);
static int coroutine_fn bdrv_co_do_pwrite_zeroes(BlockDriverState *bs,
-int64_t offset, int bytes, BdrvRequestFlags flags);
+int64_t offset, int64_t bytes, BdrvRequestFlags flags);
static void bdrv_parent_drained_begin(BlockDriverState *bs, BdrvChild *ignore,
bool ignore_bds_parents)
@@ -1743,7 +1743,7 @@ int coroutine_fn bdrv_co_preadv_part(BdrvChild *child,
}
static int coroutine_fn bdrv_co_do_pwrite_zeroes(BlockDriverState *bs,
-int64_t offset, int bytes, BdrvRequestFlags flags)
+int64_t offset, int64_t bytes, BdrvRequestFlags flags)
Widens from 32- to 64-bit. Callers (I'm looking at pre-series code, the
further I get into your series, the more likely that intermediate
changes may alter the analysis...):
bdrv_co_do_copy_on_readv() - passes 'int64_t pnum' bounded by
fragmenting loop limited to MAX_BOUNCE_BUFFER
bdrv_aligned_pwritev() - passes 'unsigned int bytes' - latent bug fix
for sizes between 2G and 4G, if any
to see if that bug could be tickled, look at callers of
bdrv_aligned_pwritev:
bdrv_co_do_zero_pwritev() - splits 'unsigned int bytes' into
head|body|tail; head and tail are safe but body could be > 2G
bdrv_co_pwritev_part() - gates with bdrv_check_byte_request()
continuing the audit, callers of bdrv_co_do_zero_pwritev:
bdrv_co_pwritev_part() - gates with bdrv_check_byte_request()
okay, all callers pass < 2G per our current code in
bdrv_check_byte_request(), so there is no actual bug. Still, the latent
fix would be nice to mention in the commit message.
{
BlockDriver *drv = bs->drv;
QEMUIOVector qiov;
@@ -1773,7 +1773,7 @@ static int coroutine_fn
bdrv_co_do_pwrite_zeroes(BlockDriverState *bs,
assert(max_write_zeroes >= bs->bl.request_alignment);
while (bytes > 0 && !ret) {
-int num = bytes;
+int64_t num = bytes;
Use of 'bytes' within the function:
compute 'int tail' via % 'int alignment' - safe
fragmentation loop 'int num' - still fragments with a cap on max_transfer
use of 'num' within the loop
compute 'int head' via % 'int alignment' - safe
clamp size by 'int max_write_zeroes' - safe
drv->bdrv_co_pwrite_zeroes(int) - safe because of clamping
clamp size by 'int max_transfer' - safe
qemu_iovec_init_buf(size_t) - safe because of clamping
bdrv_driver_pwritev(uint64_t) [well, int64_t after 4/17] - safe
So even with the wider type, we aren't exceeding the contract of
anything we pass it on to. Later patches may improve
drv->bdrv_co_pwrite_zeroes and qemu_iovec_init_buf to be 64-bit clean,
at which point we would want to revisit this function to use 64-bit
clamping rather than 32-bit clamping, but it does not have to happen here.
Reviewed-by: Eric Blake
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
Re: [PATCH 1/1] target/riscv: fix VS interrupts forwarding to HS
> Your change just made it true for whenever virtulisation is enabled > (in which case we don't need it). This is exactly my point. As I said in the commit message, the spec clearly tells us that "Interrupts for higher-privilege modes, y>x, are always globally enabled regardless of the setting of the global yIE bit for the higher-privilege mode.". HS is clearly a higher-privilege mode than either VS or VU. So, if virtualization is enabled, HS level interrupts must be considered enabled independently of the state of the actual sie bit in mstatus_hs.
Error "cannot bind memory to host NUMA nodes: Operation not permitted" running inside docker
Hi,
I encountered the following error on the QEMU 5.0.0 release, compiled and run
inside a docker image:
"cannot bind memory to host NUMA nodes: Operation not permitted"
The QEMU command line to reproduce this behavior:
qemu-system-i386 -m 64 -M pc -smp 1 -display none -monitor stdio -drive
file=mp-acpi/NOS.iso,media=cdrom,id=d -boot order=d -d cpu_reset
The docker image which shows the error is available here:
https://hub.docker.com/repository/docker/xenos1984/test-qemu
Built on Ubuntu 20.04, and including NUMA support with libnuma-dev package
installed, from the following sources:
https://github.com/xenos1984/cross-toolchain/tree/master/tools-qemu
https://github.com/xenos1984/cross-toolchain/tree/master/test-qemu
The iso image used can be obtained here, but should not be relevant:
https://github.com/xenos1984/NOS/releases/download/latest/nos-i686.iso.bz2
The command fails when the image is used in a CI environment:
https://circleci.com/gh/xenos1984/NOS/953
On recommendation by @imammedo I tried the following:
--- a/backends/hostmem.c
+++ b/backends/hostmem.c
@@ -384,3 +384,3 @@
if (mbind(ptr, sz, backend->policy,
- maxnode ? backend->host_nodes : NULL, maxnode + 1, flags)) {
+ maxnode ? backend->host_nodes : NULL, 0, flags)) {
if (backend->policy != MPOL_DEFAULT || errno != ENOSYS) {
But no success, the same error occurs.
Best regards,
xenos1984 / Manuel Hohmann
signature.asc
Description: OpenPGP digital signature
Re: [PATCH 1/2] softfloat: m68k: infinity is a valid encoding
Laurent Vivier writes: > Le 29/04/2020 à 11:26, Alex Bennée a écrit : >> >> Laurent Vivier writes: >> >>> Le 28/04/2020 à 20:43, Alex Bennée a écrit : KONRAD Frederic writes: > The MC68881 say about infinities (3.2.4): > > "*For the extended precision format, the most significant bit of the > mantissa (the integer bit) is a don't care." > > https://www.nxp.com/docs/en/reference-manual/MC68881UM.pdf > > The m68k extended format is implemented with the floatx80 and > floatx80_invalid_encoding currently treats 0x7fff as > an invalid encoding. This patch fixes floatx80_invalid_encoding so it > accepts that the most significant bit of the mantissa can be 0. > > This bug can be revealed with the following code which pushes extended > infinity on the stack as a double and then reloads it as a double. It > should normally be converted and read back as infinity and is currently > read back as nan: Do you have any real HW on which you could record some .ref files for the various multiarch float tests we have (float_convs/float_madds)? Does this different of invalid encoding show up when you add them? >>> >>> On my side, in the past when I started to implement m68k FPU, I used >>> TestFloat and SoftFloat I have ported to m68k and I compare the result >>> in QEMU and in a Quadra 800. >> >> Surely TestFloat and SoftFloat is all emulation though? >> >> Anyway if you have a Quadra 800 running Linux could you generate some >> .ref files for the float_convs and float_madds test cases. The binaries >> are static so you should just be able to copy them and run. >> >> > > Here are the files I have generated on Q800. So running those with: run-float_convs: QEMU_OPTS += -cpu m68040 run-float_madds: QEMU_OPTS += -cpu m68040 We see the m68k float needs a fair bit of work from the get go: Referenceqemu-m68k -cou m68040 ### Rounding to nearest ### Rounding to nearest > from single: f32(-nan:0xffbf) > to double: f64(-nan:0x00fff7e000) (OK) > to int32: 2147483647 (OK) > to int64: 9223372034707292159 (OK) > to uint32: 2147483647 (OK) > to uint64: 9223372034707292159 (OK) from single: f32(-nan:0x) from single: f32(-nan:0x) to double: f64(-nan:0x00e000) (OK)to double: f64(-nan:0x00e000) (OK) to int32: 2147483392 (INVALID) |to int32: 2147483647 (OK) to int64: 9223370939490631424 (INVALID) |to int64: 9223372034707292159 (OK) to uint32: 2147483392 (INVALID) | to uint32: 2147483647 (OK) to uint64: 9223370939490631424 (INVALID) | to uint64: 9223372034707292159 (OK) from single: f32(-nan:0x) | from single: f32(nan:0x7fff) to double: f64(-nan:0x00e000) (OK)| to double: f64(nan:0x007fffe000) (OK) to int32: 2147483392 (INVALID) |to int32: 2147483647 (OK) to int64: 9223370939490631424 (INVALID) |to int64: 9223372034707292159 (OK) to uint32: 2147483392 (INVALID) | to uint32: 2147483647 (OK) to uint64: 9223370939490631424 (INVALID) | to uint64: 9223372034707292159 (OK) from single: f32(-inf:0xff80) < to double: f64(-inf:0x00fff0) (OK)< to int32: -2147483648 (INVALID) < to int64: 1 (INVALID)< to uint32: -2147483648 (INVALID) < to uint64: -9223372034707292160 (INVALID) < from single: f32(-0x1.fe00p+127:0xff7f) from single: f32(-0x1.fe00p+127:0xff7f) to double: f64(-0x1.fe00p+127:0x00c7efe to double: f64(-0x1.fe00p+127:0x00c7efe to int32: -2147483648 (INVALID) |to int32: -2147483648 (OK) to int64: 1 (INVALID)|to int64: 1 (OK) to uint32: -2147483648 (INVALID) | to uint32: -2147483648 (OK) to uint64: -9223372034707292160 (INVALID) | to uint64: -9223372034707292160 (OK)
Error "cannot bind memory to host NUMA nodes: Operation not permitted" running inside docker
Hi,
I encountered the following error message on the QEMU 5.0.0 release, compiled
and run inside a docker image:
"cannot bind memory to host NUMA nodes: Operation not permitted"
The QEMU command line to reproduce this behavior (it happens also on -x86_64,
-arm, -aarch64 with similar command line):
qemu-system-i386 -m 64 -M pc -smp 1 -display none -monitor stdio -drive
file=mp-acpi/NOS.iso,media=cdrom,id=d -boot order=d -d cpu_reset
The docker image which shows the error is available here:
https://hub.docker.com/repository/docker/xenos1984/test-qemu
Built on Ubuntu 20.04, and including NUMA support with libnuma-dev package
installed, from the following sources:
https://github.com/xenos1984/cross-toolchain/tree/master/tools-qemu
https://github.com/xenos1984/cross-toolchain/tree/master/test-qemu
The iso image used can be obtained here, but should not be relevant:
https://github.com/xenos1984/NOS/releases/download/latest/nos-i686.iso.bz2
The command fails when the image is used in a CI environment:
https://circleci.com/gh/xenos1984/NOS/953
On recommendation by @imammedo I post the issue to qemu-devel, and also tried
the following patch:
--- a/backends/hostmem.c
+++ b/backends/hostmem.c
@@ -384,3 +384,3 @@
if (mbind(ptr, sz, backend->policy,
- maxnode ? backend->host_nodes : NULL, maxnode + 1, flags)) {
+ maxnode ? backend->host_nodes : NULL, 0, flags)) {
if (backend->policy != MPOL_DEFAULT || errno != ENOSYS) {
But no success, the same error occurs. It happens only within docker - the same
command runs fine on my desktop (also Ubuntu 20.04) system.
Best regards,
xenos1984 / Manuel Hohmann
PS: I apologize if this mail is sent / received more than once; there was a
problem with my outgoing mails.
signature.asc
Description: OpenPGP digital signature
Re: [PULL 00/14] RISC-V Patch Queue for 5.1
On Wed, Apr 29, 2020 at 1:04 PM Peter Maydell wrote: > > On Wed, 29 Apr 2020 at 19:37, Alistair Francis > wrote: > > > > The following changes since commit a7922a3c81f34f45b1ebc9670a7769edc4c42a43: > > > > Open 5.1 development tree (2020-04-29 15:07:10 +0100) > > > > are available in the Git repository at: > > > > g...@github.com:alistair23/qemu.git tags/pull-riscv-to-apply-20200429-1 > > > > for you to fetch changes up to 23766b6a35d5b1664ab782c02624bf2435c4ed5d: > > > > hw/riscv/spike: Allow more than one CPUs (2020-04-29 11:23:44 -0700) > > > > > > RISC-V pull request for 5.1 > > > > This is the first pull request for the 5.1 development period. It > > contains all of the patches that were sent during the 5.0 timeframe. > > > > This is an assortment of fixes for RISC-V, including fixes for the > > Hypervisor extension, the Spike machine and an update to OpenSBI. > > > > -- > > Hi; this doesn't apply to current master. The conflict looks like > it's probably pretty easy to fix up, but could you fix it and resend, > please? Done! There was a conflict with the other PR that went in after I sent the PR. Should be good to go now. Alistair > > thanks > -- PMM >
[PULL v2 10/14] linux-user/riscv: fix up struct target_ucontext definition
From: LIU Zhiwei
As struct target_ucontext will be transfered to signal handler, it
must keep pace with struct ucontext_t defined in Linux kernel.
Signed-off-by: LIU Zhiwei
Reviewed-by: Alistair Francis
Message-id: 20200412020830.607-1-zhiwei_...@c-sky.com
Message-Id: <20200412020830.607-1-zhiwei_...@c-sky.com>
Signed-off-by: Alistair Francis
---
linux-user/riscv/signal.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/linux-user/riscv/signal.c b/linux-user/riscv/signal.c
index 83ecc6f799..67a95dbc7b 100644
--- a/linux-user/riscv/signal.c
+++ b/linux-user/riscv/signal.c
@@ -40,8 +40,9 @@ struct target_ucontext {
unsigned long uc_flags;
struct target_ucontext *uc_link;
target_stack_t uc_stack;
-struct target_sigcontext uc_mcontext;
target_sigset_t uc_sigmask;
+uint8_t __unused[1024 / 8 - sizeof(target_sigset_t)];
+struct target_sigcontext uc_mcontext QEMU_ALIGNED(16);
};
struct target_rt_sigframe {
--
2.26.2
[PULL v2 12/14] hw/riscv: Add optional symbol callback ptr to riscv_load_firmware()
From: Anup Patel
This patch adds an optional function pointer, "sym_cb", to
riscv_load_firmware() which provides the possibility to access
the symbol table during kernel loading.
The pointer is ignored, if supplied with flat (non-elf) firmware image.
The Spike board requires it locate the HTIF symbols from firmware ELF
passed via "-bios" option.
Signed-off-by: Anup Patel
Reviewed-by: Alistair Francis
Message-id: 20200427080644.168461-2-anup.pa...@wdc.com
Message-Id: <20200427080644.168461-2-anup.pa...@wdc.com>
Signed-off-by: Alistair Francis
---
hw/riscv/boot.c | 13 -
hw/riscv/sifive_u.c | 2 +-
hw/riscv/virt.c | 2 +-
include/hw/riscv/boot.h | 6 --
4 files changed, 14 insertions(+), 9 deletions(-)
diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
index b8e765277d..726300a171 100644
--- a/hw/riscv/boot.c
+++ b/hw/riscv/boot.c
@@ -36,7 +36,8 @@
void riscv_find_and_load_firmware(MachineState *machine,
const char *default_machine_firmware,
- hwaddr firmware_load_addr)
+ hwaddr firmware_load_addr,
+ symbol_fn_t sym_cb)
{
char *firmware_filename = NULL;
@@ -76,7 +77,7 @@ void riscv_find_and_load_firmware(MachineState *machine,
if (firmware_filename) {
/* If not "none" load the firmware */
-riscv_load_firmware(firmware_filename, firmware_load_addr);
+riscv_load_firmware(firmware_filename, firmware_load_addr, sym_cb);
g_free(firmware_filename);
}
}
@@ -96,12 +97,14 @@ char *riscv_find_firmware(const char *firmware_filename)
}
target_ulong riscv_load_firmware(const char *firmware_filename,
- hwaddr firmware_load_addr)
+ hwaddr firmware_load_addr,
+ symbol_fn_t sym_cb)
{
uint64_t firmware_entry, firmware_start, firmware_end;
-if (load_elf(firmware_filename, NULL, NULL, NULL, _entry,
- _start, _end, NULL, 0, EM_RISCV, 1, 0) > 0)
{
+if (load_elf_ram_sym(firmware_filename, NULL, NULL, NULL,
+ _entry, _start, _end, NULL,
+ 0, EM_RISCV, 1, 0, NULL, true, sym_cb) > 0) {
return firmware_entry;
}
diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index e32355a691..bed10fcfa8 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -352,7 +352,7 @@ static void sifive_u_machine_init(MachineState *machine)
create_fdt(s, memmap, machine->ram_size, machine->kernel_cmdline);
riscv_find_and_load_firmware(machine, BIOS_FILENAME,
- memmap[SIFIVE_U_DRAM].base);
+ memmap[SIFIVE_U_DRAM].base, NULL);
if (machine->kernel_filename) {
uint64_t kernel_entry = riscv_load_kernel(machine->kernel_filename,
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index c621a970aa..daae3ebdbb 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -511,7 +511,7 @@ static void riscv_virt_board_init(MachineState *machine)
mask_rom);
riscv_find_and_load_firmware(machine, BIOS_FILENAME,
- memmap[VIRT_DRAM].base);
+ memmap[VIRT_DRAM].base, NULL);
if (machine->kernel_filename) {
uint64_t kernel_entry = riscv_load_kernel(machine->kernel_filename,
diff --git a/include/hw/riscv/boot.h b/include/hw/riscv/boot.h
index df80051fbc..474a940ad5 100644
--- a/include/hw/riscv/boot.h
+++ b/include/hw/riscv/boot.h
@@ -24,10 +24,12 @@
void riscv_find_and_load_firmware(MachineState *machine,
const char *default_machine_firmware,
- hwaddr firmware_load_addr);
+ hwaddr firmware_load_addr,
+ symbol_fn_t sym_cb);
char *riscv_find_firmware(const char *firmware_filename);
target_ulong riscv_load_firmware(const char *firmware_filename,
- hwaddr firmware_load_addr);
+ hwaddr firmware_load_addr,
+ symbol_fn_t sym_cb);
target_ulong riscv_load_kernel(const char *kernel_filename,
symbol_fn_t sym_cb);
hwaddr riscv_load_initrd(const char *filename, uint64_t mem_size,
--
2.26.2
[PULL v2 08/14] riscv: sifive_e: Support changing CPU type
From: Corey Wharton
Allows the CPU to be changed from the default via the -cpu command
line option.
Signed-off-by: Corey Wharton
Reviewed-by: Bin Meng
Reviewed-by: Alistair Francis
Reviewed-by: Philippe Mathieu-Daudé
Message-id: 20200313193429.8035-2-core...@fb.com
Message-Id: <20200313193429.8035-2-core...@fb.com>
[ Changes by AF:
- Set "cpu-type" from the machine and not SoC
]
Signed-off-by: Alistair Francis
---
hw/riscv/sifive_e.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/hw/riscv/sifive_e.c b/hw/riscv/sifive_e.c
index 646553a7c3..b53109521e 100644
--- a/hw/riscv/sifive_e.c
+++ b/hw/riscv/sifive_e.c
@@ -123,8 +123,6 @@ static void riscv_sifive_e_soc_init(Object *obj)
object_initialize_child(obj, "cpus", >cpus,
sizeof(s->cpus), TYPE_RISCV_HART_ARRAY,
_abort, NULL);
-object_property_set_str(OBJECT(>cpus), SIFIVE_E_CPU, "cpu-type",
-_abort);
object_property_set_int(OBJECT(>cpus), ms->smp.cpus, "num-harts",
_abort);
sysbus_init_child_obj(obj, "riscv.sifive.e.gpio0",
@@ -141,6 +139,8 @@ static void riscv_sifive_e_soc_realize(DeviceState *dev,
Error **errp)
SiFiveESoCState *s = RISCV_E_SOC(dev);
MemoryRegion *sys_mem = get_system_memory();
+object_property_set_str(OBJECT(>cpus), ms->cpu_type, "cpu-type",
+_abort);
object_property_set_bool(OBJECT(>cpus), true, "realized",
_abort);
@@ -219,6 +219,7 @@ static void riscv_sifive_e_machine_init(MachineClass *mc)
mc->desc = "RISC-V Board compatible with SiFive E SDK";
mc->init = riscv_sifive_e_init;
mc->max_cpus = 1;
+mc->default_cpu_type = SIFIVE_E_CPU;
}
DEFINE_MACHINE("sifive_e", riscv_sifive_e_machine_init)
--
2.26.2
[PULL v2 07/14] hw/riscv: Generate correct "mmu-type" for 32-bit machines
From: Bin Meng
32-bit machine should have its CPU's "mmu-type" set to "riscv,sv32".
Signed-off-by: Bin Meng
Reviewed-by: Alistair Francis
Message-id: 1583585319-26603-1-git-send-email-bmeng...@gmail.com
Message-Id: <1583585319-26603-1-git-send-email-bmeng...@gmail.com>
Signed-off-by: Alistair Francis
---
hw/riscv/sifive_u.c | 4
hw/riscv/spike.c| 4
hw/riscv/virt.c | 4
3 files changed, 12 insertions(+)
diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index 8d0ee8b9c4..e32355a691 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -160,7 +160,11 @@ static void create_fdt(SiFiveUState *s, const struct
MemmapEntry *memmap,
qemu_fdt_add_subnode(fdt, nodename);
/* cpu 0 is the management hart that does not have mmu */
if (cpu != 0) {
+#if defined(TARGET_RISCV32)
+qemu_fdt_setprop_string(fdt, nodename, "mmu-type", "riscv,sv32");
+#else
qemu_fdt_setprop_string(fdt, nodename, "mmu-type", "riscv,sv48");
+#endif
isa = riscv_isa_string(>soc.u_cpus.harts[cpu - 1]);
} else {
isa = riscv_isa_string(>soc.e_cpus.harts[0]);
diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
index 5053fe4590..98697a244e 100644
--- a/hw/riscv/spike.c
+++ b/hw/riscv/spike.c
@@ -102,7 +102,11 @@ static void create_fdt(SpikeState *s, const struct
MemmapEntry *memmap,
char *intc = g_strdup_printf("/cpus/cpu@%d/interrupt-controller", cpu);
char *isa = riscv_isa_string(>soc.harts[cpu]);
qemu_fdt_add_subnode(fdt, nodename);
+#if defined(TARGET_RISCV32)
+qemu_fdt_setprop_string(fdt, nodename, "mmu-type", "riscv,sv32");
+#else
qemu_fdt_setprop_string(fdt, nodename, "mmu-type", "riscv,sv48");
+#endif
qemu_fdt_setprop_string(fdt, nodename, "riscv,isa", isa);
qemu_fdt_setprop_string(fdt, nodename, "compatible", "riscv");
qemu_fdt_setprop_string(fdt, nodename, "status", "okay");
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index 85ec9e22aa..c621a970aa 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -229,7 +229,11 @@ static void create_fdt(RISCVVirtState *s, const struct
MemmapEntry *memmap,
char *intc = g_strdup_printf("/cpus/cpu@%d/interrupt-controller", cpu);
char *isa = riscv_isa_string(>soc.harts[cpu]);
qemu_fdt_add_subnode(fdt, nodename);
+#if defined(TARGET_RISCV32)
+qemu_fdt_setprop_string(fdt, nodename, "mmu-type", "riscv,sv32");
+#else
qemu_fdt_setprop_string(fdt, nodename, "mmu-type", "riscv,sv48");
+#endif
qemu_fdt_setprop_string(fdt, nodename, "riscv,isa", isa);
qemu_fdt_setprop_string(fdt, nodename, "compatible", "riscv");
qemu_fdt_setprop_string(fdt, nodename, "status", "okay");
--
2.26.2
[PULL v2 14/14] hw/riscv/spike: Allow more than one CPUs
From: Anup Patel
Currently, the upstream Spike ISA simulator allows more than
one CPUs so we update QEMU Spike machine on similar lines to
allow more than one CPUs.
The maximum number of CPUs for QEMU Spike machine is kept
same as QEMU Virt machine.
Signed-off-by: Anup Patel
Reviewed-by: Alistair Francis
Message-id: 20200427080644.168461-4-anup.pa...@wdc.com
Message-Id: <20200427080644.168461-4-anup.pa...@wdc.com>
Signed-off-by: Alistair Francis
---
hw/riscv/spike.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
index e7908b88fe..d0c4843712 100644
--- a/hw/riscv/spike.c
+++ b/hw/riscv/spike.c
@@ -476,7 +476,7 @@ static void spike_machine_init(MachineClass *mc)
{
mc->desc = "RISC-V Spike Board";
mc->init = spike_board_init;
-mc->max_cpus = 1;
+mc->max_cpus = 8;
mc->is_default = true;
mc->default_cpu_type = SPIKE_V1_10_0_CPU;
}
--
2.26.2
[PULL v2 05/14] riscv: AND stage-1 and stage-2 protection flags
Take the result of stage-1 and stage-2 page table walks and AND the two protection flags together. This way we require both to set permissions instead of just stage-2. Signed-off-by: Alistair Francis Reviewed-by: Richard Henderson Tested-by: Anup Patel Message-id: 846f1e18f5922d818bc464ec32c144ef314ec724.1585262586.git.alistair.fran...@wdc.com Message-Id: <846f1e18f5922d818bc464ec32c144ef314ec724.1585262586.git.alistair.fran...@wdc.com> --- target/riscv/cpu_helper.c | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c index f36d184b7b..50e13a064f 100644 --- a/target/riscv/cpu_helper.c +++ b/target/riscv/cpu_helper.c @@ -707,7 +707,7 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size, #ifndef CONFIG_USER_ONLY vaddr im_address; hwaddr pa = 0; -int prot; +int prot, prot2; bool pmp_violation = false; bool m_mode_two_stage = false; bool hs_mode_two_stage = false; @@ -757,13 +757,15 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size, /* Second stage lookup */ im_address = pa; -ret = get_physical_address(env, , , im_address, +ret = get_physical_address(env, , , im_address, access_type, mmu_idx, false, true); qemu_log_mask(CPU_LOG_MMU, "%s 2nd-stage address=%" VADDR_PRIx " ret %d physical " TARGET_FMT_plx " prot %d\n", -__func__, im_address, ret, pa, prot); +__func__, im_address, ret, pa, prot2); + +prot &= prot2; if (riscv_feature(env, RISCV_FEATURE_PMP) && (ret == TRANSLATE_SUCCESS) && -- 2.26.2
[PULL v2 13/14] hw/riscv/spike: Allow loading firmware separately using -bios option
From: Anup Patel
This patch extends Spike machine support to allow loading OpenSBI
firmware (fw_jump.elf) separately using -bios option.
Signed-off-by: Anup Patel
Reviewed-by: Alistair Francis
Message-id: 20200427080644.168461-3-anup.pa...@wdc.com
Message-Id: <20200427080644.168461-3-anup.pa...@wdc.com>
Signed-off-by: Alistair Francis
---
hw/riscv/spike.c | 24 +++-
1 file changed, 23 insertions(+), 1 deletion(-)
diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
index 98697a244e..e7908b88fe 100644
--- a/hw/riscv/spike.c
+++ b/hw/riscv/spike.c
@@ -45,6 +45,12 @@
#include
+#if defined(TARGET_RISCV32)
+# define BIOS_FILENAME "opensbi-riscv32-spike-fw_jump.elf"
+#else
+# define BIOS_FILENAME "opensbi-riscv64-spike-fw_jump.elf"
+#endif
+
static const struct MemmapEntry {
hwaddr base;
hwaddr size;
@@ -187,8 +193,24 @@ static void spike_board_init(MachineState *machine)
memory_region_add_subregion(system_memory, memmap[SPIKE_MROM].base,
mask_rom);
+riscv_find_and_load_firmware(machine, BIOS_FILENAME,
+ memmap[SPIKE_DRAM].base,
+ htif_symbol_callback);
+
if (machine->kernel_filename) {
-riscv_load_kernel(machine->kernel_filename, htif_symbol_callback);
+uint64_t kernel_entry = riscv_load_kernel(machine->kernel_filename,
+ htif_symbol_callback);
+
+if (machine->initrd_filename) {
+hwaddr start;
+hwaddr end = riscv_load_initrd(machine->initrd_filename,
+ machine->ram_size, kernel_entry,
+ );
+qemu_fdt_setprop_cell(s->fdt, "/chosen",
+ "linux,initrd-start", start);
+qemu_fdt_setprop_cell(s->fdt, "/chosen", "linux,initrd-end",
+ end);
+}
}
/* reset vector */
--
2.26.2
[PULL v2 02/14] riscv/sifive_u: Add a serial property to the sifive_u SoC
At present the board serial number is hard-coded to 1, and passed
to OTP model during initialization. Firmware (FSBL, U-Boot) uses
the serial number to generate a unique MAC address for the on-chip
ethernet controller. When multiple QEMU 'sifive_u' instances are
created and connected to the same subnet, they all have the same
MAC address hence it creates a unusable network.
A new "serial" property is introduced to the sifive_u SoC to specify
the board serial number. When not given, the default serial number
1 is used.
Suggested-by: Bin Meng
Signed-off-by: Alistair Francis
Reviewed-by: Bin Meng
Tested-by: Bin Meng
---
hw/riscv/sifive_u.c | 8 +++-
include/hw/riscv/sifive_u.h | 2 ++
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index 7f6a3c6c15..6e659e986f 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -491,7 +491,6 @@ static void riscv_sifive_u_soc_init(Object *obj)
TYPE_SIFIVE_U_PRCI);
sysbus_init_child_obj(obj, "otp", >otp, sizeof(s->otp),
TYPE_SIFIVE_U_OTP);
-qdev_prop_set_uint32(DEVICE(>otp), "serial", OTP_SERIAL);
sysbus_init_child_obj(obj, "gem", >gem, sizeof(s->gem),
TYPE_CADENCE_GEM);
}
@@ -584,6 +583,7 @@ static void riscv_sifive_u_soc_realize(DeviceState *dev,
Error **errp)
object_property_set_bool(OBJECT(>prci), true, "realized", );
sysbus_mmio_map(SYS_BUS_DEVICE(>prci), 0, memmap[SIFIVE_U_PRCI].base);
+qdev_prop_set_uint32(DEVICE(>otp), "serial", s->serial);
object_property_set_bool(OBJECT(>otp), true, "realized", );
sysbus_mmio_map(SYS_BUS_DEVICE(>otp), 0, memmap[SIFIVE_U_OTP].base);
@@ -610,10 +610,16 @@ static void riscv_sifive_u_soc_realize(DeviceState *dev,
Error **errp)
memmap[SIFIVE_U_GEM_MGMT].base, memmap[SIFIVE_U_GEM_MGMT].size);
}
+static Property riscv_sifive_u_soc_props[] = {
+DEFINE_PROP_UINT32("serial", SiFiveUSoCState, serial, OTP_SERIAL),
+DEFINE_PROP_END_OF_LIST()
+};
+
static void riscv_sifive_u_soc_class_init(ObjectClass *oc, void *data)
{
DeviceClass *dc = DEVICE_CLASS(oc);
+device_class_set_props(dc, riscv_sifive_u_soc_props);
dc->realize = riscv_sifive_u_soc_realize;
/* Reason: Uses serial_hds in realize function, thus can't be used twice */
dc->user_creatable = false;
diff --git a/include/hw/riscv/sifive_u.h b/include/hw/riscv/sifive_u.h
index 82667b5746..a2baa1de5f 100644
--- a/include/hw/riscv/sifive_u.h
+++ b/include/hw/riscv/sifive_u.h
@@ -42,6 +42,8 @@ typedef struct SiFiveUSoCState {
SiFiveUPRCIState prci;
SiFiveUOTPState otp;
CadenceGEMState gem;
+
+uint32_t serial;
} SiFiveUSoCState;
#define TYPE_RISCV_U_MACHINE MACHINE_TYPE_NAME("sifive_u")
--
2.26.2
[PULL v2 06/14] riscv: Fix Stage2 SV32 page table walk
From: Anup Patel
As-per RISC-V H-Extension v0.5 draft, the Stage2 SV32 page table has
12bits of VPN[1] and 10bits of VPN[0]. The additional 2bits in VPN[1]
is required to handle the 34bit intermediate physical address coming
from Stage1 SV32 page table. The 12bits of VPN[1] implies that Stage2
SV32 level-0 page table will be 16KB in size with total 4096 enteries
where each entry maps 4MB of memory (same as Stage1 SV32 page table).
The get_physical_address() function is broken for Stage2 SV32 level-0
page table because it incorrectly computes output physical address for
Stage2 SV32 level-0 page table entry.
The root cause of the issue is that get_physical_address() uses the
"widened" variable to compute level-0 physical address mapping which
changes level-0 mapping size (instead of 4MB). We should use the
"widened" variable only for computing index of Stage2 SV32 level-0
page table.
Signed-off-by: Anup Patel
Reviewed-by: Alistair Francis
Message-id: 20200330082724.120444-1-anup.pa...@wdc.com
Message-Id: <20200330082724.120444-1-anup.pa...@wdc.com>
Signed-off-by: Alistair Francis
---
target/riscv/cpu_helper.c | 7 +--
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 50e13a064f..bc80aa87cf 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -559,12 +559,7 @@ restart:
/* for superpage mappings, make a fake leaf PTE for the TLB's
benefit. */
target_ulong vpn = addr >> PGSHIFT;
-if (i == 0) {
-*physical = (ppn | (vpn & ((1L << (ptshift + widened)) - 1)))
<<
- PGSHIFT;
-} else {
-*physical = (ppn | (vpn & ((1L << ptshift) - 1))) << PGSHIFT;
-}
+*physical = (ppn | (vpn & ((1L << ptshift) - 1))) << PGSHIFT;
/* set permissions on the TLB entry */
if ((pte & PTE_R) || ((pte & PTE_X) && mxr)) {
--
2.26.2
[PULL v2 09/14] target/riscv: Add a sifive-e34 cpu type
From: Corey Wharton
The sifive-e34 cpu type is the same as the sifive-e31 with the
single precision floating-point extension enabled.
Signed-off-by: Corey Wharton
Reviewed-by: Alistair Francis
Reviewed-by: Bin Meng
Message-id: 20200313193429.8035-3-core...@fb.com
Message-Id: <20200313193429.8035-3-core...@fb.com>
Signed-off-by: Alistair Francis
---
target/riscv/cpu.c | 10 ++
target/riscv/cpu.h | 1 +
2 files changed, 11 insertions(+)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 4e578239d3..059d71f2c7 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -164,6 +164,15 @@ static void rv32imacu_nommu_cpu_init(Object *obj)
set_feature(env, RISCV_FEATURE_PMP);
}
+static void rv32imafcu_nommu_cpu_init(Object *obj)
+{
+CPURISCVState *env = _CPU(obj)->env;
+set_misa(env, RV32 | RVI | RVM | RVA | RVF | RVC | RVU);
+set_priv_version(env, PRIV_VERSION_1_10_0);
+set_resetvec(env, DEFAULT_RSTVEC);
+set_feature(env, RISCV_FEATURE_PMP);
+}
+
#elif defined(TARGET_RISCV64)
static void riscv_base64_cpu_init(Object *obj)
@@ -610,6 +619,7 @@ static const TypeInfo riscv_cpu_type_infos[] = {
#if defined(TARGET_RISCV32)
DEFINE_CPU(TYPE_RISCV_CPU_BASE32, riscv_base32_cpu_init),
DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_E31, rv32imacu_nommu_cpu_init),
+DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_E34, rv32imafcu_nommu_cpu_init),
DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_U34, rv32gcsu_priv1_10_0_cpu_init),
/* Depreacted */
DEFINE_CPU(TYPE_RISCV_CPU_RV32IMACU_NOMMU, rv32imacu_nommu_cpu_init),
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 7d21addbab..d0e7f5b9c5 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -36,6 +36,7 @@
#define TYPE_RISCV_CPU_BASE32 RISCV_CPU_TYPE_NAME("rv32")
#define TYPE_RISCV_CPU_BASE64 RISCV_CPU_TYPE_NAME("rv64")
#define TYPE_RISCV_CPU_SIFIVE_E31 RISCV_CPU_TYPE_NAME("sifive-e31")
+#define TYPE_RISCV_CPU_SIFIVE_E34 RISCV_CPU_TYPE_NAME("sifive-e34")
#define TYPE_RISCV_CPU_SIFIVE_E51 RISCV_CPU_TYPE_NAME("sifive-e51")
#define TYPE_RISCV_CPU_SIFIVE_U34 RISCV_CPU_TYPE_NAME("sifive-u34")
#define TYPE_RISCV_CPU_SIFIVE_U54 RISCV_CPU_TYPE_NAME("sifive-u54")
--
2.26.2
[PULL v2 01/14] riscv/sifive_u: Fix up file ordering
Split the file into clear machine and SoC sections.
Signed-off-by: Alistair Francis
Reviewed-by: Bin Meng
---
hw/riscv/sifive_u.c | 108 ++--
1 file changed, 54 insertions(+), 54 deletions(-)
diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index 998666c91f..7f6a3c6c15 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -312,7 +312,7 @@ static void create_fdt(SiFiveUState *s, const struct
MemmapEntry *memmap,
g_free(nodename);
}
-static void riscv_sifive_u_init(MachineState *machine)
+static void sifive_u_machine_init(MachineState *machine)
{
const struct MemmapEntry *memmap = sifive_u_memmap;
SiFiveUState *s = RISCV_U_MACHINE(machine);
@@ -403,6 +403,59 @@ static void riscv_sifive_u_init(MachineState *machine)
_space_memory);
}
+static bool sifive_u_machine_get_start_in_flash(Object *obj, Error **errp)
+{
+SiFiveUState *s = RISCV_U_MACHINE(obj);
+
+return s->start_in_flash;
+}
+
+static void sifive_u_machine_set_start_in_flash(Object *obj, bool value, Error
**errp)
+{
+SiFiveUState *s = RISCV_U_MACHINE(obj);
+
+s->start_in_flash = value;
+}
+
+static void sifive_u_machine_instance_init(Object *obj)
+{
+SiFiveUState *s = RISCV_U_MACHINE(obj);
+
+s->start_in_flash = false;
+object_property_add_bool(obj, "start-in-flash",
sifive_u_machine_get_start_in_flash,
+ sifive_u_machine_set_start_in_flash, NULL);
+object_property_set_description(obj, "start-in-flash",
+"Set on to tell QEMU's ROM to jump to "
+"flash. Otherwise QEMU will jump to DRAM",
+NULL);
+}
+
+static void sifive_u_machine_class_init(ObjectClass *oc, void *data)
+{
+MachineClass *mc = MACHINE_CLASS(oc);
+
+mc->desc = "RISC-V Board compatible with SiFive U SDK";
+mc->init = sifive_u_machine_init;
+mc->max_cpus = SIFIVE_U_MANAGEMENT_CPU_COUNT + SIFIVE_U_COMPUTE_CPU_COUNT;
+mc->min_cpus = SIFIVE_U_MANAGEMENT_CPU_COUNT + 1;
+mc->default_cpus = mc->min_cpus;
+}
+
+static const TypeInfo sifive_u_machine_typeinfo = {
+.name = MACHINE_TYPE_NAME("sifive_u"),
+.parent = TYPE_MACHINE,
+.class_init = sifive_u_machine_class_init,
+.instance_init = sifive_u_machine_instance_init,
+.instance_size = sizeof(SiFiveUState),
+};
+
+static void sifive_u_machine_init_register_types(void)
+{
+type_register_static(_u_machine_typeinfo);
+}
+
+type_init(sifive_u_machine_init_register_types)
+
static void riscv_sifive_u_soc_init(Object *obj)
{
MachineState *ms = MACHINE(qdev_get_machine());
@@ -443,33 +496,6 @@ static void riscv_sifive_u_soc_init(Object *obj)
TYPE_CADENCE_GEM);
}
-static bool sifive_u_get_start_in_flash(Object *obj, Error **errp)
-{
-SiFiveUState *s = RISCV_U_MACHINE(obj);
-
-return s->start_in_flash;
-}
-
-static void sifive_u_set_start_in_flash(Object *obj, bool value, Error **errp)
-{
-SiFiveUState *s = RISCV_U_MACHINE(obj);
-
-s->start_in_flash = value;
-}
-
-static void riscv_sifive_u_machine_instance_init(Object *obj)
-{
-SiFiveUState *s = RISCV_U_MACHINE(obj);
-
-s->start_in_flash = false;
-object_property_add_bool(obj, "start-in-flash",
sifive_u_get_start_in_flash,
- sifive_u_set_start_in_flash, NULL);
-object_property_set_description(obj, "start-in-flash",
-"Set on to tell QEMU's ROM to jump to "
-"flash. Otherwise QEMU will jump to DRAM",
-NULL);
-}
-
static void riscv_sifive_u_soc_realize(DeviceState *dev, Error **errp)
{
MachineState *ms = MACHINE(qdev_get_machine());
@@ -607,29 +633,3 @@ static void riscv_sifive_u_soc_register_types(void)
}
type_init(riscv_sifive_u_soc_register_types)
-
-static void riscv_sifive_u_machine_class_init(ObjectClass *oc, void *data)
-{
-MachineClass *mc = MACHINE_CLASS(oc);
-
-mc->desc = "RISC-V Board compatible with SiFive U SDK";
-mc->init = riscv_sifive_u_init;
-mc->max_cpus = SIFIVE_U_MANAGEMENT_CPU_COUNT + SIFIVE_U_COMPUTE_CPU_COUNT;
-mc->min_cpus = SIFIVE_U_MANAGEMENT_CPU_COUNT + 1;
-mc->default_cpus = mc->min_cpus;
-}
-
-static const TypeInfo riscv_sifive_u_machine_typeinfo = {
-.name = MACHINE_TYPE_NAME("sifive_u"),
-.parent = TYPE_MACHINE,
-.class_init = riscv_sifive_u_machine_class_init,
-.instance_init = riscv_sifive_u_machine_instance_init,
-.instance_size = sizeof(SiFiveUState),
-};
-
-static void riscv_sifive_u_machine_init_register_types(void)
-{
-type_register_static(_sifive_u_machine_typeinfo);
-}
-
-type_init(riscv_sifive_u_machine_init_register_types)
--
2.26.2
[PULL v2 03/14] riscv/sifive_u: Add a serial property to the sifive_u machine
From: Bin Meng
At present the board serial number is hard-coded to 1, and passed
to OTP model during initialization. Firmware (FSBL, U-Boot) uses
the serial number to generate a unique MAC address for the on-chip
ethernet controller. When multiple QEMU 'sifive_u' instances are
created and connected to the same subnet, they all have the same
MAC address hence it creates a unusable network.
A new "serial" property is introduced to specify the board serial
number. When not given, the default serial number 1 is used.
Signed-off-by: Bin Meng
Reviewed-by: Palmer Dabbelt
Reviewed-by: Alistair Francis
Message-Id: <1573916930-19068-1-git-send-email-bmeng...@gmail.com>
[ Changed by AF:
- Use the SoC's serial property to pass the info to the SoC
- Fixup commit title
- Rebase on file restructuring
]
Signed-off-by: Alistair Francis
---
hw/riscv/sifive_u.c | 20
include/hw/riscv/sifive_u.h | 1 +
2 files changed, 21 insertions(+)
diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index 6e659e986f..8d0ee8b9c4 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -34,6 +34,7 @@
#include "qemu/log.h"
#include "qemu/error-report.h"
#include "qapi/error.h"
+#include "qapi/visitor.h"
#include "hw/boards.h"
#include "hw/loader.h"
#include "hw/sysbus.h"
@@ -326,6 +327,8 @@ static void sifive_u_machine_init(MachineState *machine)
object_initialize_child(OBJECT(machine), "soc", >soc,
sizeof(s->soc), TYPE_RISCV_U_SOC,
_abort, NULL);
+object_property_set_uint(OBJECT(>soc), s->serial, "serial",
+_abort);
object_property_set_bool(OBJECT(>soc), true, "realized",
_abort);
@@ -417,6 +420,18 @@ static void sifive_u_machine_set_start_in_flash(Object
*obj, bool value, Error *
s->start_in_flash = value;
}
+static void sifive_u_machine_get_serial(Object *obj, Visitor *v, const char
*name,
+void *opaque, Error **errp)
+{
+visit_type_uint32(v, name, (uint32_t *)opaque, errp);
+}
+
+static void sifive_u_machine_set_serial(Object *obj, Visitor *v, const char
*name,
+void *opaque, Error **errp)
+{
+visit_type_uint32(v, name, (uint32_t *)opaque, errp);
+}
+
static void sifive_u_machine_instance_init(Object *obj)
{
SiFiveUState *s = RISCV_U_MACHINE(obj);
@@ -428,6 +443,11 @@ static void sifive_u_machine_instance_init(Object *obj)
"Set on to tell QEMU's ROM to jump to "
"flash. Otherwise QEMU will jump to DRAM",
NULL);
+
+s->serial = OTP_SERIAL;
+object_property_add(obj, "serial", "uint32", sifive_u_machine_get_serial,
+sifive_u_machine_set_serial, NULL, >serial, NULL);
+object_property_set_description(obj, "serial", "Board serial number",
NULL);
}
static void sifive_u_machine_class_init(ObjectClass *oc, void *data)
diff --git a/include/hw/riscv/sifive_u.h b/include/hw/riscv/sifive_u.h
index a2baa1de5f..16c297ec5f 100644
--- a/include/hw/riscv/sifive_u.h
+++ b/include/hw/riscv/sifive_u.h
@@ -61,6 +61,7 @@ typedef struct SiFiveUState {
int fdt_size;
bool start_in_flash;
+uint32_t serial;
} SiFiveUState;
enum {
--
2.26.2
[PULL v2 04/14] riscv: Don't use stage-2 PTE lookup protection flags
When doing the fist of a two stage lookup (Hypervisor extensions) don't
set the current protection flags from the second stage lookup of the
base address PTE.
Signed-off-by: Alistair Francis
Reviewed-by: Richard Henderson
Tested-by: Anup Patel
Message-id:
931db85d6890ed4bc2b527fd1011197cd28299aa.1585262586.git.alistair.fran...@wdc.com
Message-Id:
<931db85d6890ed4bc2b527fd1011197cd28299aa.1585262586.git.alistair.fran...@wdc.com>
---
target/riscv/cpu_helper.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index d3ba9efb02..f36d184b7b 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -452,10 +452,11 @@ restart:
hwaddr pte_addr;
if (two_stage && first_stage) {
+int vbase_prot;
hwaddr vbase;
/* Do the second stage translation on the base PTE address. */
-get_physical_address(env, , prot, base, access_type,
+get_physical_address(env, , _prot, base, access_type,
mmu_idx, false, true);
pte_addr = vbase + idx * ptesize;
--
2.26.2
[PULL v2 00/14] RISC-V Patch Queue for 5.1
The following changes since commit 648db19685b7030aa558a4ddbd3a8e53d8c9a062: Merge remote-tracking branch 'remotes/armbru/tags/pull-misc-2020-04-29' into staging (2020-04-29 15:07:33 +0100) are available in the Git repository at: g...@github.com:alistair23/qemu.git tags/pull-riscv-to-apply-20200429-2 for you to fetch changes up to 31e6d70485b1a719ca27e9a2d21f2a61ac497cdf: hw/riscv/spike: Allow more than one CPUs (2020-04-29 13:16:38 -0700) RISC-V pull request for 5.1 This is the first pull request for the 5.1 development period. It contains all of the patches that were sent during the 5.0 timeframe. This is an assortment of fixes for RISC-V, including fixes for the Hypervisor extension, the Spike machine and an update to OpenSBI. Alistair Francis (4): riscv/sifive_u: Fix up file ordering riscv/sifive_u: Add a serial property to the sifive_u SoC riscv: Don't use stage-2 PTE lookup protection flags riscv: AND stage-1 and stage-2 protection flags Anup Patel (4): riscv: Fix Stage2 SV32 page table walk hw/riscv: Add optional symbol callback ptr to riscv_load_firmware() hw/riscv/spike: Allow loading firmware separately using -bios option hw/riscv/spike: Allow more than one CPUs Bin Meng (3): riscv/sifive_u: Add a serial property to the sifive_u machine hw/riscv: Generate correct "mmu-type" for 32-bit machines roms: opensbi: Upgrade from v0.6 to v0.7 Corey Wharton (2): riscv: sifive_e: Support changing CPU type target/riscv: Add a sifive-e34 cpu type LIU Zhiwei (1): linux-user/riscv: fix up struct target_ucontext definition hw/riscv/boot.c | 13 ++- hw/riscv/sifive_e.c | 5 +- hw/riscv/sifive_u.c | 142 --- hw/riscv/spike.c | 30 +- hw/riscv/virt.c | 6 +- include/hw/riscv/boot.h | 6 +- include/hw/riscv/sifive_u.h | 3 + linux-user/riscv/signal.c| 3 +- pc-bios/opensbi-riscv32-sifive_u-fw_jump.bin | Bin 49472 -> 49520 bytes pc-bios/opensbi-riscv32-virt-fw_jump.bin | Bin 41280 -> 49504 bytes pc-bios/opensbi-riscv64-sifive_u-fw_jump.bin | Bin 53760 -> 57936 bytes pc-bios/opensbi-riscv64-virt-fw_jump.bin | Bin 49664 -> 57920 bytes roms/opensbi | 2 +- target/riscv/cpu.c | 10 ++ target/riscv/cpu.h | 1 + target/riscv/cpu_helper.c| 18 ++-- 16 files changed, 159 insertions(+), 80 deletions(-)
Re: [PATCH v2 04/17] block/io: use int64_t bytes in driver wrappers
On 4/27/20 3:23 AM, Vladimir Sementsov-Ogievskiy wrote:
We are generally moving to int64_t for both offset and bytes parameters
on all io paths. Convert driver wrappers parameters which are
already 64bit to signed type.
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
block/io.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/block/io.c b/block/io.c
index 1267918def..4796476835 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1086,7 +1086,7 @@ static void bdrv_co_io_em_complete(void *opaque, int ret)
}
static int coroutine_fn bdrv_driver_preadv(BlockDriverState *bs,
- uint64_t offset, uint64_t bytes,
+ int64_t offset, int64_t bytes,
Remains 64 bits, the question is whether any caller could pass in
something larger than 63 bits. Callers are:
bdrv_co_do_copy_on_readv() - passes 'int64_t pnum', but sets pnum in a
fragmenting loop, MAX_BOUNCE_BUFFER when copy-on-read is needed, or
max_transfer bounded by BDRV_REQUEST_MAX_BYTES otherwise
bdrv_aligned_preadv() - passes 'unsigned int bytes' further limited by
fragmenting loop on max_transfer <= INT_MAX
Input is bounded to < 2G, internal use of 'bytes' is:
drv->bdrv_co_preadv_part(uint64_t) - safe
qemu_iovec_init_slice(size_t) - potential truncation on 32-bit platform,
if you don't fix qemu_iovec
drv->bdrv_co_preadv(uint64_t) - safe
drv->bdrv_aio_preadv(uint64_t) - safe
drv->bdrv_co_readv(int) after assertion of <2G - safe
QEMUIOVector *qiov,
size_t qiov_offset, int flags)
{
@@ -1155,7 +1155,7 @@ out:
}
static int coroutine_fn bdrv_driver_pwritev(BlockDriverState *bs,
-uint64_t offset, uint64_t bytes,
+int64_t offset, int64_t bytes,
Remains 64 bits, the question is whether any caller could pass in
something larger than 63. Callers are:
bdrv_co_do_copy_on_readv() - passes 'int64_t pnum', but set in a
fragmenting loop bounded by MAX_BOUNCE_BUFFER
bdrv_co_do_pwrite_zeroes() - passes 'int num'
bdrv_aligned_pwritev() - passes 'unsigned int bytes' further limited by
fragmenting loop on max_transfer <= INT_MAX
Input is bounded to < 2G, internal use of 'bytes' is:
drv->bdrv_co_pwritev_part(uint64_t) - safe
qemu_iovec_init_slice(size_t) - potential truncation on 32-bit platform,
if you don't fix qemu_iovec
drv->bdrv_co_pwritev(uint64_t) - safe
drv->bdrv_aio_pwritev(uint64_t) - safe
drv->bdrv_co_writev(int) after assertion of <2G - safe
QEMUIOVector *qiov,
size_t qiov_offset, int flags)
{
@@ -1235,8 +1235,8 @@ emulate_flags:
}
static int coroutine_fn
-bdrv_driver_pwritev_compressed(BlockDriverState *bs, uint64_t offset,
- uint64_t bytes, QEMUIOVector *qiov,
+bdrv_driver_pwritev_compressed(BlockDriverState *bs, int64_t offset,
+ int64_t bytes, QEMUIOVector *qiov,
size_t qiov_offset)
Remains 64 bits, the question is whether any caller could pass in
something larger than 63. Callers are:
bdrv_aligned_pwritev() - passes 'unsigned int bytes'
Input is bounded to < 4G, internal use of 'bytes' is:
drv->bdrv_co_pwritev_compressed_part(uint64_t) - safe
drv->bdrv_co_pwritev_compressed(uint64_t) - safe
qemu_iovec_init_slice(size_t) - potential truncation on 32-bit platform,
if you don't fix qemu_iovec
Because our input is bounded, all of the changes made here have no
semantic impact, and I argue this patch is safe. However, I also
pointed out the spots where we have latent bugs (on 32-bit machines
only) if the callers are relaxed to pass larger than 2G or 4G. As long
as you are auditing things, it would be nice to either fix that in this
patch, or document in the commit message how a future patch will address
that latent problem (whether by enforcing max_transfer to be capped at
2G on 32-bit platforms, or else fixing qemu_iovec to use int64_t instead
of size_t bounds).
Reviewed-by: Eric Blake
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
[RFC PATCH v2] plugins: new lockstep plugin for debugging TCG changes
When we make changes to the TCG we sometimes cause regressions that
are deep into the execution cycle of the guest. Debugging this often
requires comparing large volumes of trace information to figure out
where behaviour has diverged.
The lockstep plugin utilises a shared socket so two QEMU's running
with the plugin will write their current execution position and wait
to receive the position of their partner process. When execution
diverges the plugins output where they were and the previous few
blocks before unloading themselves and letting execution continue.
Originally I planned for this to be most useful with -icount but it
turns out you can get divergence pretty quickly due to asynchronous
qemu_cpu_kick_rr_cpus() events causing one side to eventually run into
a short block a few cycles before the other side. For this reason I've
added a bit of tracking and I think the divergence reporting could be
finessed to report only if we really start to diverge in execution.
An example run would be:
qemu-system-sparc -monitor none -parallel none -net none \
-M SS-20 -m 256 -kernel day11/zImage.elf \
-plugin ./tests/plugin/liblockstep.so,arg=lockstep-sparc.sock \
-d plugin,nochain
with an identical command in another window in the same working
directory.
Signed-off-by: Alex Bennée
Cc: Richard Henderson
Cc: Mark Cave-Ayland
---
v2
- track BlockInfo in a separate list to avoid double-free
- cleaner abstraction between run/exec/log state
- exit cleanly when out of sync
- more verbiage and explanation in commit message
- skip liblockstep in check-tcg
---
tests/plugin/lockstep.c | 307 ++
tests/plugin/Makefile | 1 +
tests/tcg/Makefile.target | 2 +-
3 files changed, 309 insertions(+), 1 deletion(-)
create mode 100644 tests/plugin/lockstep.c
diff --git a/tests/plugin/lockstep.c b/tests/plugin/lockstep.c
new file mode 100644
index 00..b8a60cdc6b
--- /dev/null
+++ b/tests/plugin/lockstep.c
@@ -0,0 +1,307 @@
+/*
+ * Lockstep Execution Plugin
+ *
+ * Allows you to execute two QEMU instances in lockstep and report
+ * when their execution diverges. This is mainly useful for developers
+ * who want to see where a change to TCG code generation has
+ * introduced a subtle and hard to find bug.
+ *
+ * Caveats:
+ * - single-threaded linux-user apps only with non-deterministic syscalls
+ * - no MTTCG enabled system emulation (icount may help)
+ *
+ * While icount makes things more deterministic it doesn't mean a
+ * particular run may execute the exact same sequence of blocks. An
+ * asynchronous event (for example X11 graphics update) may cause a
+ * block to end early and a new partial block to start. This means
+ * serial only test cases are a better bet. -d nochain may also help.
+ *
+ * This code is not thread safe!
+ *
+ * Copyright (c) 2020 Linaro Ltd
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+
+QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
+
+/* saved so we can uninstall later */
+static qemu_plugin_id_t our_id;
+
+static unsigned long bb_count;
+static unsigned long insn_count;
+
+/* Information about a translated block */
+typedef struct {
+uint64_t pc;
+uint64_t insns;
+} BlockInfo;
+
+/* Information about an execution state in the log */
+typedef struct {
+BlockInfo *block;
+unsigned long insn_count;
+unsigned long block_count;
+} ExecInfo;
+
+/* The execution state we compare */
+typedef struct {
+uint64_t pc;
+unsigned long insn_count;
+} ExecState;
+
+/* list of translated block info */
+static GSList *blocks;
+
+/* execution log and points of divergence */
+static GSList *log, *divergence_log;
+
+static int divergence_count;
+static int max_divergence = 10;
+
+static int socket_fd;
+static char *path_to_unlink;
+
+
+static void plugin_cleanup(qemu_plugin_id_t id)
+{
+/* Free our block data */
+g_slist_free_full(blocks, _free);
+g_slist_free_full(log, _free);
+g_slist_free(divergence_log);
+
+close(socket_fd);
+if (path_to_unlink) {
+unlink(path_to_unlink);
+}
+}
+
+static void plugin_exit(qemu_plugin_id_t id, void *p)
+{
+g_autoptr(GString) out = g_string_new("No divergence :-)\n");
+g_string_append_printf(out, "Executed %ld/%d blocks\n",
+ bb_count, g_slist_length(log));
+g_string_append_printf(out, "Executed ~%ld instructions\n", insn_count);
+qemu_plugin_outs(out->str);
+
+plugin_cleanup(id);
+}
+
+static void report_divergance(ExecState *us, ExecState *them)
+{
+int i;
+GSList *entry = log;
+g_autoptr(GString) out = g_string_new("I feel the ");
+
+divergence_count++;
+
+g_string_append_printf(out,
+ "%d%s divergence in the force\n", divergence_count,
+ divergence_count > 3 ? "th" :
+
Re: [PULL 00/14] RISC-V Patch Queue for 5.1
On Wed, 29 Apr 2020 at 19:37, Alistair Francis wrote: > > The following changes since commit a7922a3c81f34f45b1ebc9670a7769edc4c42a43: > > Open 5.1 development tree (2020-04-29 15:07:10 +0100) > > are available in the Git repository at: > > g...@github.com:alistair23/qemu.git tags/pull-riscv-to-apply-20200429-1 > > for you to fetch changes up to 23766b6a35d5b1664ab782c02624bf2435c4ed5d: > > hw/riscv/spike: Allow more than one CPUs (2020-04-29 11:23:44 -0700) > > > RISC-V pull request for 5.1 > > This is the first pull request for the 5.1 development period. It > contains all of the patches that were sent during the 5.0 timeframe. > > This is an assortment of fixes for RISC-V, including fixes for the > Hypervisor extension, the Spike machine and an update to OpenSBI. > > -- Hi; this doesn't apply to current master. The conflict looks like it's probably pretty easy to fix up, but could you fix it and resend, please? thanks -- PMM
Re: [PULL 00/32] Miscellaneous patches for 2020-04-29
On Wed, 29 Apr 2020 at 08:23, Markus Armbruster wrote: > > The following changes since commit fdd76fecdde1ad444ff4deb7f1c4f7e4a1ef97d6: > > Update version for v5.0.0 release (2020-04-28 17:46:57 +0100) > > are available in the Git repository at: > > git://repo.or.cz/qemu/armbru.git tags/pull-misc-2020-04-29 > > for you to fetch changes up to 8ef3a4be27efccd791d05e74b7b17d918f511a76: > > qemu-option: pass NULL rather than 0 to the id of qemu_opts_set() > (2020-04-29 08:01:52 +0200) > > > Miscellaneous patches for 2020-04-29 > > * Fix CLI option parsing corner cases > * Fix bugs on (unlikely) error paths > * Fix undefined behavior for silly option arguments > * Tidy up bamboo and sam460ex reporting of funny memory sizes > * Clean up error API violations > * A bit of refactoring here and there Applied, thanks. Please update the changelog at https://wiki.qemu.org/ChangeLog/5.1 for any user-visible changes. -- PMM
[PATCH 3/4] configure: add flag to enable SafeStack
This patch adds a flag to enable the SafeStack instrumentation provided
by LLVM.
The checks make sure that the compiler supports the flags, and that we
are using the proper coroutine implementation (coroutine-ucontext).
While SafeStack is supported only on Linux, NetBSD, FreeBSD and macOS,
we are not checking for the O.S. since this is already done by LLVM.
Signed-off-by: Daniele Buono
---
configure | 29 +
1 file changed, 29 insertions(+)
diff --git a/configure b/configure
index 23b5e93752..f37e4ae0bd 100755
--- a/configure
+++ b/configure
@@ -302,6 +302,7 @@ audio_win_int=""
libs_qga=""
debug_info="yes"
stack_protector=""
+safe_stack="no"
use_containers="yes"
gdb_bin=$(command -v "gdb")
@@ -1275,6 +1276,8 @@ for opt do
;;
--disable-stack-protector) stack_protector="no"
;;
+ --enable-safe-stack) safe_stack="yes"
+ ;;
--disable-curses) curses="no"
;;
--enable-curses) curses="yes"
@@ -1774,6 +1777,8 @@ Advanced options (experts only):
--with-coroutine=BACKEND coroutine backend. Supported options:
ucontext, sigaltstack, windows
--enable-gcovenable test coverage analysis with gcov
+ --enable-safe-stack enable the SafeStack stack protection. Depends on
+ clang/llvm >= 3.7 and coroutine backend ucontext.
--gcov=GCOV use specified gcov [$gcov_tool]
--disable-blobs disable installing provided firmware blobs
--with-vss-sdk=SDK-path enable Windows VSS support in QEMU Guest Agent
@@ -5501,6 +5506,29 @@ if test "$debug_stack_usage" = "yes"; then
fi
fi
+##
+# Check if SafeStack is enabled and supported
+
+if test "$safe_stack" = "yes"; then
+ cat > $TMPC << EOF
+int main(int argc, char *argv[])
+{
+return 0;
+}
+EOF
+ flag="-fsanitize=safe-stack"
+ # Check that safe-stack is supported.
+ if compile_prog "-Werror $flag" ""; then
+# Flag needed both at compilation and at linking
+QEMU_CFLAGS="$QEMU_CFLAGS $flag"
+QEMU_LDFLAGS="$QEMU_LDFLAGS $flag"
+ else
+error_exit "SafeStack not supported by your compiler"
+ fi
+ if test "$coroutine" != "ucontext"; then
+error_exit "SafeStack is only supported by the coroutine backend ucontext"
+ fi
+fi
##
# check if we have open_by_handle_at
@@ -6595,6 +6623,7 @@ echo "sparse enabled$sparse"
echo "strip binaries$strip_opt"
echo "profiler $profiler"
echo "static build $static"
+echo "safe stack$safe_stack"
if test "$darwin" = "yes" ; then
echo "Cocoa support $cocoa"
fi
--
2.26.2
[PATCH 2/4] coroutine: Add check for SafeStack in sigalstack
LLVM's SafeStack instrumentation cannot be used inside signal handlers
that make use of sigaltstack().
Since coroutine-sigaltstack relies on sigaltstack(), it is not
compatible with SafeStack. The resulting binary is incorrect, with
different coroutines sharing the same unsafe stack and producing
undefined behavior at runtime.
To avoid this, we add a check in coroutine-sigaltstack that throws a
preprocessor #error and interrupt the compilation if SafeStack is
enabled.
Signed-off-by: Daniele Buono
---
util/coroutine-sigaltstack.c | 4
1 file changed, 4 insertions(+)
diff --git a/util/coroutine-sigaltstack.c b/util/coroutine-sigaltstack.c
index f6fc49a0e5..b7cdc959f8 100644
--- a/util/coroutine-sigaltstack.c
+++ b/util/coroutine-sigaltstack.c
@@ -30,6 +30,10 @@
#include "qemu-common.h"
#include "qemu/coroutine_int.h"
+#ifdef CONFIG_SAFESTACK
+#error "SafeStack does not work with sigaltstack's implementation"
+#endif
+
typedef struct {
Coroutine base;
void *stack;
--
2.26.2
[PATCH 4/4] check-block: Enable iotests with SafeStack
SafeStack is a stack protection technique implemented in llvm. It is
enabled with a -fsanitize flag.
iotests are currently disabled when any -fsanitize option is used.
Since SafeStack is useful on production environments, and its
implementation may break the binary, filter it out when the check is
performed, so that if SafeStack was the only -fsanitize option, iotests
are still performed.
Signed-off-by: Daniele Buono
---
tests/check-block.sh | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/tests/check-block.sh b/tests/check-block.sh
index ad320c21ba..8e29c868e5 100755
--- a/tests/check-block.sh
+++ b/tests/check-block.sh
@@ -21,7 +21,17 @@ if grep -q "CONFIG_GPROF=y" config-host.mak 2>/dev/null ;
then
exit 0
fi
-if grep -q "CFLAGS.*-fsanitize" config-host.mak 2>/dev/null ; then
+# Disable tests with any sanitizer except for SafeStack
+CFLAGS=$( grep "CFLAGS.*-fsanitize" config-host.mak 2>/dev/null )
+SANITIZE_FLAGS=""
+#Remove all occurrencies of -fsanitize=safe-stack
+for i in ${CFLAGS}; do
+if [ "${i}" != "-fsanitize=safe-stack" ]; then
+SANITIZE_FLAGS="${SANITIZE_FLAGS} ${i}"
+fi
+done
+if echo ${SANITIZE_FLAGS} | grep -q "\-fsanitize" 2>/dev/null; then
+# Have a sanitize flag that is not allowed, stop
echo "Sanitizers are enabled ==> Not running the qemu-iotests."
exit 0
fi
--
2.26.2
[PATCH 1/4] coroutine: support SafeStack in ucontext backend
LLVM's SafeStack instrumentation does not yet support programs that make
use of the APIs in ucontext.h
With the current implementation of coroutine-ucontext, the resulting
binary is incorrect, with different coroutines sharing the same unsafe
stack and producing undefined behavior at runtime.
This fix allocates an additional unsafe stack area for each coroutine,
and sets the new unsafe stack pointer before calling swapcontext() in
qemu_coroutine_new.
This is the only place where the pointer needs to be manually updated,
since sigsetjmp/siglongjmp are already instrumented by LLVM to properly
support SafeStack.
The additional stack is then freed in qemu_coroutine_delete.
Signed-off-by: Daniele Buono
---
include/qemu/coroutine_int.h | 6 ++
util/coroutine-ucontext.c| 25 +
2 files changed, 31 insertions(+)
diff --git a/include/qemu/coroutine_int.h b/include/qemu/coroutine_int.h
index bd6b0468e1..2ffd75ddbe 100644
--- a/include/qemu/coroutine_int.h
+++ b/include/qemu/coroutine_int.h
@@ -28,6 +28,12 @@
#include "qemu/queue.h"
#include "qemu/coroutine.h"
+#if defined(__has_feature) && __has_feature(safe_stack)
+#define CONFIG_SAFESTACK 1
+/* Pointer to the unsafe stack, defined by the compiler */
+extern __thread void *__safestack_unsafe_stack_ptr;
+#endif
+
#define COROUTINE_STACK_SIZE (1 << 20)
typedef enum {
diff --git a/util/coroutine-ucontext.c b/util/coroutine-ucontext.c
index bd593e61bc..b79e9df9eb 100644
--- a/util/coroutine-ucontext.c
+++ b/util/coroutine-ucontext.c
@@ -41,6 +41,11 @@ typedef struct {
Coroutine base;
void *stack;
size_t stack_size;
+#ifdef CONFIG_SAFESTACK
+/* Need an unsafe stack for each coroutine */
+void *unsafe_stack;
+size_t unsafe_stack_size;
+#endif
sigjmp_buf env;
#ifdef CONFIG_VALGRIND_H
@@ -140,6 +145,10 @@ Coroutine *qemu_coroutine_new(void)
co = g_malloc0(sizeof(*co));
co->stack_size = COROUTINE_STACK_SIZE;
co->stack = qemu_alloc_stack(>stack_size);
+#ifdef CONFIG_SAFESTACK
+co->unsafe_stack_size = COROUTINE_STACK_SIZE;
+co->unsafe_stack = qemu_alloc_stack(>unsafe_stack_size);
+#endif
co->base.entry_arg = _env; /* stash away our jmp_buf */
uc.uc_link = _uc;
@@ -160,6 +169,19 @@ Coroutine *qemu_coroutine_new(void)
/* swapcontext() in, siglongjmp() back out */
if (!sigsetjmp(old_env, 0)) {
start_switch_fiber(_stack_save, co->stack, co->stack_size);
+#ifdef CONFIG_SAFESTACK
+/*
+ * Before we swap the context, set the new unsafe stack
+ * The unsafe stack grows just like the normal stack, so start from
+ * the last usable location of the memory area.
+ * NOTE: we don't have to re-set it afterwards because sigsetjmp was
+ * called with the original usp. Since we are not coming back with a
+ * swapcontext, but with a siglongjmp, when we are back here we
+ * already have usp restored to the valid one for this function
+ */
+void *usp = co->unsafe_stack + co->unsafe_stack_size;
+__safestack_unsafe_stack_ptr = usp;
+#endif
swapcontext(_uc, );
}
@@ -192,6 +214,9 @@ void qemu_coroutine_delete(Coroutine *co_)
#endif
qemu_free_stack(co->stack, co->stack_size);
+#ifdef CONFIG_SAFESTACK
+qemu_free_stack(co->unsafe_stack, co->unsafe_stack_size);
+#endif
g_free(co);
}
--
2.26.2
[PATCH 0/4] Add support for SafeStack
LLVM supports SafeStack instrumentation to protect against stack buffer overflows, since version 3.7 >From https://clang.llvm.org/docs/SafeStack.html: "It works by separating the program stack into two distinct regions: the safe stack and the unsafe stack. The safe stack stores return addresses, register spills, and local variables that are always accessed in a safe way, while the unsafe stack stores everything else. This separation ensures that buffer overflows on the unsafe stack cannot be used to overwrite anything on the safe stack." Unfortunately, the use of two stack regions does not cope well with QEMU's coroutines. The second stack region is not properly set up with both ucontext and sigaltstack, so multiple coroutines end up sharing the same memory area for the unsafe stack, causing undefined behaviors at runtime (and most iochecks to fail). This patch series fixes the implementation of the ucontext backend and make sure that sigaltstack is never used if the compiler is applying the SafeStack instrumentation. It also adds a configure flag to enable SafeStack, and enables iotests when SafeStack is used. This is an RFC mainly because of the low-level use of the SafeStack runtime. When running swapcontext(), we have to manually set the unsafe stack pointer to the new area allocated for the coroutine. LLVM does not allow this by using builtin, so we have to use implementation details that may change in the future. This patch has been tested briefly ( make check on an x86 system ) with clang v3.9, v4.0, v5.0, v6.0 Heavier testing, with make check-acceptance has been performed with clang v7.0 Daniele Buono (4): coroutine: support SafeStack in ucontext backend coroutine: Add check for SafeStack in sigalstack configure: add flag to enable SafeStack check-block: Enable iotests with SafeStack configure| 29 + include/qemu/coroutine_int.h | 6 ++ tests/check-block.sh | 12 +++- util/coroutine-sigaltstack.c | 4 util/coroutine-ucontext.c| 25 + 5 files changed, 75 insertions(+), 1 deletion(-) -- 2.26.2
Re: [PATCH v2 5/6] ramfb: add sanity checks to ramfb_create_display_surface
On 04/29/20 13:52, Gerd Hoffmann wrote:
> Signed-off-by: Gerd Hoffmann
> ---
> hw/display/ramfb.c | 6 ++
> 1 file changed, 6 insertions(+)
>
> diff --git a/hw/display/ramfb.c b/hw/display/ramfb.c
> index eb8b4bc49a2f..be884c9ea837 100644
> --- a/hw/display/ramfb.c
> +++ b/hw/display/ramfb.c
> @@ -15,6 +15,7 @@
> #include "qapi/error.h"
> #include "hw/loader.h"
> #include "hw/display/ramfb.h"
> +#include "hw/display/bochs-vbe.h" /* for limits */
> #include "ui/console.h"
> #include "sysemu/reset.h"
>
> @@ -49,6 +50,11 @@ static DisplaySurface *ramfb_create_display_surface(int
> width, int height,
> hwaddr size;
> void *data;
>
> +if (width < 16 || width > VBE_DISPI_MAX_XRES ||
> +height < 16 || height > VBE_DISPI_MAX_YRES ||
> +format == 0 /* unknown format */)
> +return NULL;
> +
> if (linesize == 0) {
> linesize = width * PIXMAN_FORMAT_BPP(format) / 8;
> }
>
Reviewed-by: Laszlo Ersek
Re: [PATCH v2 03/17] block/io: use int64_t bytes parameter in bdrv_check_byte_request()
On 4/27/20 3:23 AM, Vladimir Sementsov-Ogievskiy wrote:
We are generally moving to int64_t for both offset and bytes parameters
on all io paths. Convert bdrv_check_byte_request() now.
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
block/io.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/block/io.c b/block/io.c
index 7cbb80bd24..1267918def 100644
--- a/block/io.c
+++ b/block/io.c
@@ -875,9 +875,9 @@ static bool coroutine_fn
bdrv_wait_serialising_requests(BdrvTrackedRequest *self
}
static int bdrv_check_byte_request(BlockDriverState *bs, int64_t offset,
- size_t size)
+ int64_t bytes)
{
This changes an unsigned to signed value on 64-bit machines, and
additionally widens the parameter on 32-bit machines. Existing callers:
bdrv_co_preadv_part() with 'unsigned int' - no impact
bdrv_co_pwritev_part() with 'unsigned int' - no impact
bdrv_co_copy_range_internal() with 'uint64_t' - potentially fixes a
latent bug on 32-bit machines. Requires a larger audit to see how
bdrv_co_copy_range() and friends are used:
block/block-backend.c:blk_co_copy_range() - passes 'int', thus < 2G
block/block-copy.c:block_copy_do_copy() - passes 'int64_t', but only
after assert(nbytes < INT_MAX); also it has a BLOCK_COPY_MAX_COPY_RANGE
set to 16M that factors into its calculations on how much to do per
iteration
So it looks like at present we are safe, but the commit message should
definitely call out the fix for an unreachable latent bug.
I will also point out that on Linux, copy_file_range() is capped by a
size_t len parameter, so even if we DO have a 32-bit caller passing >
2G, it will encounter further truncation bugs if the block layer does
not fragment the request internally. I don't see any fragmentation
logic in the public bdrv_co_copy_range() or in
bdrv_co_copy_range_internal() - I suspect that needs to be added
(probably as a separate patch); maybe by moving the fragmentation loop
currently in block-copy.c to instead live in io.c?
-if (size > BDRV_REQUEST_MAX_BYTES) {
+if (bytes > BDRV_REQUEST_MAX_BYTES) {
return -EIO;
}
@@ -885,7 +885,7 @@ static int bdrv_check_byte_request(BlockDriverState *bs, int64_t offset,
return -ENOMEDIUM;
}
-if (offset < 0) {
+if (offset < 0 || bytes < 0) {
return -EIO;
}
Reviewed-by: Eric Blake
I don't know if we have any iotest coverage of copy_range with a 5G
image to prove that it doesn't misbehave on a 32-bit machine. That
seems like it will eventually be useful, but doesn't necessarily have to
be at the same time as this patch.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
Re: [PATCH] tests: add a "check-flake8" test for validating python code style
On 4/29/20 11:36 AM, Daniel P. Berrangé wrote:
> The flake8 program is a standard tool used by Python projects for
> validating many commonly recommended style rules. It would be desirable
> for QEMU to come into alignment with normal Python coding style best
> practices.
>
> QEMU currently violates a huge number of the style rules, so we can't
> blindly turn it on. Instead this patch introduces use of flake8 with
> a huge ignore list to turn off everything that is currently violated.
>
> The following descriptions are mostly taken from:
>
> https://www.flake8rules.com/
>
> Indentation:
>
> E111 Indentation is not a multiple of four
> E114 Indentation is not a multiple of four (comment)
> E115 Expected an indented block (comment)
> E116 Unexpected indentation (comment)
> E117 Over-indented
> E121 Continuation line under-indented for hanging indent
> E122 Continuation line missing indentation or outdented
> E123 Closing bracket does not match indentation of opening bracket's
> line
> E124 Closing bracket does not match visual indentation
> E125 Continuation line with same indent as next logical line
> E126 Continuation line over-indented for hanging indent
> E127 Continuation line over-indented for visual indent
> E128 Continuation line under-indented for visual indent
> E129 Visually indented line with same indent as next logical line
> E131 Continuation line unaligned for hanging indent
>
> Whitespace:
>
> E201 Whitespace after '('
> E202 Whitespace before ')'
> E203 Whitespace before ':'
> E211 Whitespace before '('
> E221 Multiple spaces before operator
> E222 Multiple spaces after operator
> E225 Missing whitespace around operator
> E226 Missing whitespace around arithmetic operator
> E228 Missing whitespace around modulo operator
> E231 Missing whitespace after ',', ';', or ':'
> E241 Multiple spaces after ','
> E251 Unexpected spaces around keyword / parameter equals
> E261 At least two spaces before inline comment
> E265 Block comment should start with '# '
> E266 Too many leading '#' for block comment
>
> Blank lines:
>
> E301 Expected 1 blank line, found 0
> E302 Expected 2 blank lines, found 0
> E303 Too many blank lines (3)
> E305 Expected 2 blank lines after end of function or class
> E306 Expected 1 blank line before a nested definition
>
> Imports:
>
> E401 Multiple imports on one line
> E402 Module level import not at top of file
>
> Line length:
>
> E501 Line too long (82 > 79 characters)
> E502 The backslash is redundant between brackets
>
> Statements:
>
> E701 Multiple statements on one line (colon)
> E702 Multiple statements on one line (semicolon)
> E703 Statement ends with a semicolon
> E711 Comparison to none should be 'if cond is none:'
> E712 Comparison to true should be 'if cond is true:' or 'if cond:'
> E713 Test for membership should be 'not in'
> E714 Test for object identity should be 'is not'
> E722 Do not use bare 'except'
> E731 Do not assign a lambda expression, use a def
> E741 Do not use variables named 'I', 'O', or 'l'
>
> Errors:
>
> F401 Module imported but unused
> F403 'from module import *' used; unable to detect undefined names
> F405 Name may be undefined, or defined from star imports: module
> F811 Redefinition of unused name from line n
> F821 Undefined name name
> F841 Local variable name is assigned to but never used
>
> Warnings:
>
> W391 Blank line at end of file
> W503 Line break occurred before a binary operator
> W504 Line break occurred after a binary operator
> W605 Invalid escape sequence 'x'
>
> Over time code should be fixed and rules removed from the ignore list.
> A handful of style rules may not warrant fixing as the cure is arguably
> worse and very subjective. e.g.
>
> E501: Force breaking lines at < 80 characters results in
>some really unnatural code formatting which harms
>readability.
>
> W504: Knuth code style requires the operators "or" and "and" etc
>to be at the start of line in a multi-line conditional.
>
> Signed-off-by: Daniel P. Berrangé
> ---
>
> On its own this patch doesn't really do much of use except try to stop the
> situation getting worse. To be valuable some motivated contributor(s)
> would need to go through fixing the code, and re-enabling each excluded
> warning category one at a time.
>
> I'm mostly proposing this patch as a starting point for discussion, to
> see if anyone is indeed motivated to take on the code cleanup challenge,
> and feed the fixes in through
Re: [PATCH v2 0/4] block: Do not call BlockDriver.bdrv_make_empty() directly
Patchew URL: https://patchew.org/QEMU/20200429141126.85159-1-mre...@redhat.com/
Hi,
This series failed the docker-mingw@fedora build test. Please find the testing
commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.
=== TEST SCRIPT BEGIN ===
#! /bin/bash
export ARCH=x86_64
make docker-image-fedora V=1 NETWORK=1
time make docker-test-mingw@fedora J=14 NETWORK=1
=== TEST SCRIPT END ===
LINKqemu-edid.exe
LINKqemu-ga.exe
/tmp/qemu-test/src/qemu-img.c: In function 'img_commit':
/tmp/qemu-test/src/qemu-img.c:1071:27: error: implicit declaration of function
'blk_new_with_bs'; did you mean 'blk_get_stats'?
[-Werror=implicit-function-declaration]
old_backing_blk = blk_new_with_bs(bs, BLK_PERM_WRITE, BLK_PERM_ALL,
^~~
blk_get_stats
/tmp/qemu-test/src/qemu-img.c:1071:27: error: nested extern declaration of
'blk_new_with_bs' [-Werror=nested-externs]
/tmp/qemu-test/src/qemu-img.c:1071:25: error: assignment to 'BlockBackend *'
{aka 'struct BlockBackend *'} from 'int' makes pointer from integer without a
cast [-Werror=int-conversion]
old_backing_blk = blk_new_with_bs(bs, BLK_PERM_WRITE, BLK_PERM_ALL,
^
cc1: all warnings being treated as errors
make: *** [/tmp/qemu-test/src/rules.mak:69: qemu-img.o] Error 1
make: *** Waiting for unfinished jobs
GEN x86_64-softmmu/hmp-commands.h
GEN x86_64-softmmu/hmp-commands-info.h
---
raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run',
'--label', 'com.qemu.instance.uuid=e0a16b2399774bccbf7cf068081a9efd', '-u',
'1003', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=',
'-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e',
'SHOW_ENV=', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v',
'/home/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v',
'/var/tmp/patchew-tester-tmp-etm7bld6/src/docker-src.2020-04-29-15.12.17.3885:/var/tmp/qemu:z,ro',
'qemu:fedora', '/var/tmp/qemu/run', 'test-mingw']' returned non-zero exit
status 2.
filter=--filter=label=com.qemu.instance.uuid=e0a16b2399774bccbf7cf068081a9efd
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-etm7bld6/src'
make: *** [docker-run-test-mingw@fedora] Error 2
real2m47.092s
user0m7.752s
The full log is available at
http://patchew.org/logs/20200429141126.85159-1-mre...@redhat.com/testing.docker-mingw@fedora/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com
Re: [PATCH v4 1/7] raspi: add BCM2835 SOC MPHI emulation
Hi Phil,
On Tue, Apr 28, 2020 at 12:06 AM Philippe Mathieu-Daudé
wrote:
> Hi Paul,
>
> On 4/28/20 4:22 AM, Paul Zimmerman wrote:
> > Add BCM2835 SOC MPHI (Message-based Parallel Host Interface)
> > emulation. It is very basic, only providing the FIQ interrupt
> > needed to allow the dwc-otg USB host controller driver in the
> > Raspbian kernel to function.
> >
> > Signed-off-by: Paul Zimmerman
> > ---
> > hw/arm/bcm2835_peripherals.c | 17 +++
> > hw/misc/Makefile.objs| 1 +
> > hw/misc/bcm2835_mphi.c | 190 +++
> > include/hw/arm/bcm2835_peripherals.h | 2 +
> > include/hw/misc/bcm2835_mphi.h | 48 +++
> > 5 files changed, 258 insertions(+)
> > create mode 100644 hw/misc/bcm2835_mphi.c
> > create mode 100644 include/hw/misc/bcm2835_mphi.h
> >
> > diff --git a/hw/arm/bcm2835_peripherals.c b/hw/arm/bcm2835_peripherals.c
> > index edcaa4916d..5e2c832d95 100644
> > --- a/hw/arm/bcm2835_peripherals.c
> > +++ b/hw/arm/bcm2835_peripherals.c
> > @@ -124,6 +124,10 @@ static void bcm2835_peripherals_init(Object *obj)
> > sysbus_init_child_obj(obj, "gpio", >gpio, sizeof(s->gpio),
> >TYPE_BCM2835_GPIO);
> >
> > +/* Mphi */
> > +sysbus_init_child_obj(obj, "mphi", >mphi, sizeof(s->mphi),
> > + TYPE_BCM2835_MPHI);
> > +
> > object_property_add_const_link(OBJECT(>gpio), "sdbus-sdhci",
> > OBJECT(>sdhci.sdbus),
> _abort);
> > object_property_add_const_link(OBJECT(>gpio), "sdbus-sdhost",
> > @@ -368,6 +372,19 @@ static void bcm2835_peripherals_realize(DeviceState
> *dev, Error **errp)
> > return;
> > }
> >
> > +/* Mphi */
> > +object_property_set_bool(OBJECT(>mphi), true, "realized", );
> > +if (err) {
> > +error_propagate(errp, err);
> > +return;
> > +}
> > +
> > +memory_region_add_subregion(>peri_mr, MPHI_OFFSET,
> > +sysbus_mmio_get_region(SYS_BUS_DEVICE(>mphi), 0));
> > +sysbus_connect_irq(SYS_BUS_DEVICE(>mphi), 0,
> > +qdev_get_gpio_in_named(DEVICE(>ic), BCM2835_IC_GPU_IRQ,
> > + INTERRUPT_HOSTPORT));
> > +
> > create_unimp(s, >armtmr, "bcm2835-sp804",
> ARMCTRL_TIMER0_1_OFFSET, 0x40);
> > create_unimp(s, >cprman, "bcm2835-cprman", CPRMAN_OFFSET,
> 0x1000);
> > create_unimp(s, >a2w, "bcm2835-a2w", A2W_OFFSET, 0x1000);
> > diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
> > index 68aae2eabb..91085cc21b 100644
> > --- a/hw/misc/Makefile.objs
> > +++ b/hw/misc/Makefile.objs
> > @@ -57,6 +57,7 @@ common-obj-$(CONFIG_OMAP) += omap_l4.o
> > common-obj-$(CONFIG_OMAP) += omap_sdrc.o
> > common-obj-$(CONFIG_OMAP) += omap_tap.o
> > common-obj-$(CONFIG_RASPI) += bcm2835_mbox.o
> > +common-obj-$(CONFIG_RASPI) += bcm2835_mphi.o
> > common-obj-$(CONFIG_RASPI) += bcm2835_property.o
> > common-obj-$(CONFIG_RASPI) += bcm2835_rng.o
> > common-obj-$(CONFIG_RASPI) += bcm2835_thermal.o
> > diff --git a/hw/misc/bcm2835_mphi.c b/hw/misc/bcm2835_mphi.c
> > new file mode 100644
> > index 00..66fc4a9cd3
> > --- /dev/null
> > +++ b/hw/misc/bcm2835_mphi.c
> > @@ -0,0 +1,190 @@
> > +/*
> > + * BCM2835 SOC MPHI emulation
> > + *
> > + * Very basic emulation, only providing the FIQ interrupt needed to
> > + * allow the dwc-otg USB host controller driver in the Raspbian kernel
> > + * to function.
> > + *
> > + * Copyright (c) 2020 Paul Zimmerman
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details.
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "qapi/error.h"
> > +#include "hw/misc/bcm2835_mphi.h"
> > +#include "migration/vmstate.h"
> > +#include "qemu/error-report.h"
> > +#include "qemu/main-loop.h"
> > +
> > +static inline void mphi_raise_irq(BCM2835MphiState *s)
> > +{
> > +qemu_set_irq(s->irq, 1);
> > +}
> > +
> > +static inline void mphi_lower_irq(BCM2835MphiState *s)
> > +{
> > +qemu_set_irq(s->irq, 0);
> > +}
> > +
> > +static uint64_t mphi_reg_read(void *ptr, hwaddr addr, unsigned size)
> > +{
> > +BCM2835MphiState *s = ptr;
> > +uint32_t reg = s->regbase + addr;
> > +uint32_t val = 0;
> > +
> > +switch (reg) {
> > +case 0x28: /* outdda */
> > +val = s->outdda;
> > +break;
> > +case 0x2c: /* outddb */
> > +val = s->outddb;
> > +break;
> > +case 0x4c: /* ctrl */
> > +val = s->ctrl;
> > +val |= 1 << 17;
> > +
Re: [PATCH v2 0/4] block: Do not call BlockDriver.bdrv_make_empty() directly
Patchew URL: https://patchew.org/QEMU/20200429141126.85159-1-mre...@redhat.com/ Hi, This series failed the asan build test. Please find the testing commands and their output below. If you have Docker installed, you can probably reproduce it locally. === TEST SCRIPT BEGIN === #!/bin/bash export ARCH=x86_64 make docker-image-fedora V=1 NETWORK=1 time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1 === TEST SCRIPT END === BUILD pc-bios/optionrom/linuxboot_dma.raw BUILD pc-bios/optionrom/kvmvapic.raw LINKqemu-edid /tmp/qemu-test/src/qemu-img.c:1071:27: error: implicit declaration of function 'blk_new_with_bs' is invalid in C99 [-Werror,-Wimplicit-function-declaration] old_backing_blk = blk_new_with_bs(bs, BLK_PERM_WRITE, BLK_PERM_ALL, ^ /tmp/qemu-test/src/qemu-img.c:1071:27: error: this function declaration is not a prototype [-Werror,-Wstrict-prototypes] /tmp/qemu-test/src/qemu-img.c:1071:25: error: incompatible integer to pointer conversion assigning to 'BlockBackend *' (aka 'struct BlockBackend *') from 'int' [-Werror,-Wint-conversion] old_backing_blk = blk_new_with_bs(bs, BLK_PERM_WRITE, BLK_PERM_ALL, ^ ~ 3 errors generated. --- SIGNpc-bios/optionrom/linuxboot.bin SIGNpc-bios/optionrom/linuxboot_dma.bin SIGNpc-bios/optionrom/kvmvapic.bin make: *** [/tmp/qemu-test/src/rules.mak:69: qemu-img.o] Error 1 make: *** Waiting for unfinished jobs BUILD pc-bios/optionrom/pvh.img BUILD pc-bios/optionrom/pvh.raw --- raise CalledProcessError(retcode, cmd) subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=d13b024a2281417b9c8a863d2bbcfec3', '-u', '1003', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=x86_64-softmmu', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-j487_kuh/src/docker-src.2020-04-29-15.07.32.27231:/var/tmp/qemu:z,ro', 'qemu:fedora', '/var/tmp/qemu/run', 'test-debug']' returned non-zero exit status 2. filter=--filter=label=com.qemu.instance.uuid=d13b024a2281417b9c8a863d2bbcfec3 make[1]: *** [docker-run] Error 1 make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-j487_kuh/src' make: *** [docker-run-test-debug@fedora] Error 2 real3m51.019s user0m7.947s The full log is available at http://patchew.org/logs/20200429141126.85159-1-mre...@redhat.com/testing.asan/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
Re: [PATCH v2 0/4] block: Do not call BlockDriver.bdrv_make_empty() directly
Patchew URL: https://patchew.org/QEMU/20200429141126.85159-1-mre...@redhat.com/ Hi, This series failed the docker-quick@centos7 build test. Please find the testing commands and their output below. If you have Docker installed, you can probably reproduce it locally. === TEST SCRIPT BEGIN === #!/bin/bash make docker-image-centos7 V=1 NETWORK=1 time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1 === TEST SCRIPT END === LINKqemu-storage-daemon LINKqemu-io /tmp/qemu-test/src/qemu-img.c: In function 'img_commit': /tmp/qemu-test/src/qemu-img.c:1071:9: error: implicit declaration of function 'blk_new_with_bs' [-Werror=implicit-function-declaration] old_backing_blk = blk_new_with_bs(bs, BLK_PERM_WRITE, BLK_PERM_ALL, ^ /tmp/qemu-test/src/qemu-img.c:1071:9: error: nested extern declaration of 'blk_new_with_bs' [-Werror=nested-externs] /tmp/qemu-test/src/qemu-img.c:1071:25: error: assignment makes pointer from integer without a cast [-Werror] old_backing_blk = blk_new_with_bs(bs, BLK_PERM_WRITE, BLK_PERM_ALL, ^ cc1: all warnings being treated as errors make: *** [qemu-img.o] Error 1 make: *** Waiting for unfinished jobs make: *** wait: No child processes. Stop. Traceback (most recent call last): --- raise CalledProcessError(retcode, cmd) subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=54c1e8d7f38d491c879a9f7fd70b93fd', '-u', '1003', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-j98lpee6/src/docker-src.2020-04-29-15.04.21.16547:/var/tmp/qemu:z,ro', 'qemu:centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit status 2. filter=--filter=label=com.qemu.instance.uuid=54c1e8d7f38d491c879a9f7fd70b93fd make[1]: *** [docker-run] Error 1 make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-j98lpee6/src' make: *** [docker-run-test-quick@centos7] Error 2 real2m41.192s user0m8.095s The full log is available at http://patchew.org/logs/20200429141126.85159-1-mre...@redhat.com/testing.docker-quick@centos7/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
Re: [PATCH 1/1] target/riscv: fix VS interrupts forwarding to HS
On Wed, Apr 29, 2020 at 9:07 AM Jose Martins wrote:
>
> > If the Hypervisor sets the V* interrupts why does it then want to
> > receive the interrupt itself?
>
> I don't think this is a question of whether there is a use case for it
> or not (I agree with you, of the top of my head I don't see why would
> you forward v* interrupts to the hypervisor). However, from what I
> can understand, the spec allows for it. If you don't set the
> corresponding bits in hideleg, v* interrupts should be forwarded to HS
> (as I said, they are guaranteed not to be forwarded to m mode because
> these bits must be hardwired in mideleg). Otherwise, there would be no
> purpose for the hideleg register, as v* interrupts bits are the only
> ones that can be written in it (am I missing something?).
I think you are right.
"Among bits 15:0 of hideleg, only bits 10, 6, and 2 (corresponding to
the standard VS-level interrupts) shall be writable, and the others
shall be hardwired to zero."
Which means that it only controls the V* interrupts.
>
> > Isn't hs_sie only ever accessed if riscv_cpu_virt_enabled(env)?
> > Doesn't this just set hs_sie to always be 1?
>
> I don't understand if you don't agree that hs_sie should be always set
> when riscv_cpu_virt_enabled(env), or if you agree with it and don't
> see the need for the hs_sie variable at all. If it is the latter, I
> agree with you. So the patch would become:
Currently hs_sie is set:
- When we are in U-Mode
- If we are in S-Mode and hs_mstatus_sie is true
Then hs_sie is only accessed if virtulisation is enabled.
Your change just made it true for whenever virtulisation is enabled
(in which case we don't need it).
>
> Signed-off-by: Jose Martins
> ---
> target/riscv/cpu_helper.c | 8 ++--
> 1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index d3ba9efb02..a85eadb4fb 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -41,10 +41,8 @@ static int riscv_cpu_local_irq_pending(CPURISCVState *env)
>
> target_ulong mstatus_mie = get_field(env->mstatus, MSTATUS_MIE);
> target_ulong mstatus_sie = get_field(env->mstatus, MSTATUS_SIE);
> -target_ulong hs_mstatus_sie = get_field(env->mstatus_hs, MSTATUS_SIE);
>
> -target_ulong pending = env->mip & env->mie &
> - ~(MIP_VSSIP | MIP_VSTIP | MIP_VSEIP);
> +target_ulong pending = env->mip & env->mie;
This looks good
> target_ulong vspending = (env->mip & env->mie &
>(MIP_VSSIP | MIP_VSTIP | MIP_VSEIP));
>
> @@ -52,11 +50,9 @@ static int riscv_cpu_local_irq_pending(CPURISCVState *env)
>(env->priv == PRV_M && mstatus_mie);
> target_ulong sie= env->priv < PRV_S ||
>(env->priv == PRV_S && mstatus_sie);
> -target_ulong hs_sie = env->priv < PRV_S ||
> - (env->priv == PRV_S && hs_mstatus_sie);
>
> if (riscv_cpu_virt_enabled(env)) {
> -target_ulong pending_hs_irq = pending & -hs_sie;
> +target_ulong pending_hs_irq = pending & ~env->hideleg;
I don't see why we don't need to check the HS version of MSTATUS_SIE.
I think hs_sie should stay shouldn't it?
Alistair
>
> if (pending_hs_irq) {
> riscv_cpu_set_force_hs_excep(env, FORCE_HS_EXCEP);
> --
> 2.17.1
>
> Jose
[PATCH 0/1] Patch for keycodemapdb
Hi Daniel, here is a patch for keycodemapdb. With this patch I have a working '<' key (the key right of the left shift key on my german keyboard) in my qemu -display gtk guests on Windows. With best regards, Volker
Re: [PATCH 2/4] target/arm: Use enum constant in get_phys_addr_lpae() call
On 3/30/20 2:03 PM, Peter Maydell wrote: > The access_type argument to get_phys_addr_lpae() is an MMUAccessType; > use the enum constant MMU_DATA_LOAD rather than a literal 0 when we > call it in S1_ptw_translate(). > > Signed-off-by: Peter Maydell > --- > target/arm/helper.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH 3/4] target/arm: Add new 's1_is_el0' argument to get_phys_addr_lpae()
On 3/30/20 2:03 PM, Peter Maydell wrote: > For ARMv8.2-TTS2UXN, the stage 2 page table walk wants to know > whether the stage 1 access is for EL0 or not, because whether > exec permission is given can depend on whether this is an EL0 > or EL1 access. Add a new argument to get_phys_addr_lpae() so > the call sites can pass this information in. > > Since get_phys_addr_lpae() doesn't already have a doc comment, > add one so we have a place to put the documentation of the > semantics of the new s1_is_el0 argument. > > Signed-off-by: Peter Maydell > --- > target/arm/helper.c | 29 - > 1 file changed, 28 insertions(+), 1 deletion(-) Reviewed-by: Richard Henderson r~
[PATCH 1/1] Fix win32 keycode for VK_OEM_102
The win32 keycode for VK_OEM_102 is 0xe2. The Microsoft docu- mentation for virtual-key codes agrees with mingw32/winuser.h. Signed-off-by: Volker Rümelin --- data/keymaps.csv | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/data/keymaps.csv b/data/keymaps.csv index bc2376c..8111047 100644 --- a/data/keymaps.csv +++ b/data/keymaps.csv @@ -145,7 +145,7 @@ KEY_KPDOT,83,ANSI_KeypadDecimal,0x41,0x53,0x71,0x71,99,VK_DECIMAL,0x6e,83,83,XK_ KEY_KPDOT,83,ANSI_KeypadDecimal,0x41,0x53,0x71,0x71,99,VK_DECIMAL,0x6e,83,83,XK_KP_Decimal,0xffae,NumpadDecimal,KPDC,kp_decimal,0x32,0x41 ,84,,,0x54,, KEY_ZENKAKUHANKAKU,85,,,0x76,0x5f,,148,,,Lang5,HZTG,,, -KEY_102ND,86,,,0x56,0x61,0x13,100,VK_OEM_102,0xe1,86,86,,,IntlBackslash,LSGT,less,0x7c, +KEY_102ND,86,,,0x56,0x61,0x13,100,VK_OEM_102,0xe2,86,86,,,IntlBackslash,LSGT,less,0x7c, KEY_F11,87,F11,0x67,0x57,0x78,0x56,68,VK_F11,0x7a,87,87,XK_F11,0xffc8,F11,FK11,f11,0x09,0x67 KEY_F12,88,F12,0x6f,0x58,0x07,0x5e,69,VK_F12,0x7b,88,88,XK_F12,0xffc9,F12,FK12,f12,0x0b,0x6f KEY_RO,89,,,0x73,0x51,,135,,,IntlRo,AB11,ro,, -- 2.16.4
[PULL 14/14] hw/riscv/spike: Allow more than one CPUs
From: Anup Patel
Currently, the upstream Spike ISA simulator allows more than
one CPUs so we update QEMU Spike machine on similar lines to
allow more than one CPUs.
The maximum number of CPUs for QEMU Spike machine is kept
same as QEMU Virt machine.
Signed-off-by: Anup Patel
Reviewed-by: Alistair Francis
Message-id: 20200427080644.168461-4-anup.pa...@wdc.com
Message-Id: <20200427080644.168461-4-anup.pa...@wdc.com>
Signed-off-by: Alistair Francis
---
hw/riscv/spike.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
index e7908b88fe..d0c4843712 100644
--- a/hw/riscv/spike.c
+++ b/hw/riscv/spike.c
@@ -476,7 +476,7 @@ static void spike_machine_init(MachineClass *mc)
{
mc->desc = "RISC-V Spike Board";
mc->init = spike_board_init;
-mc->max_cpus = 1;
+mc->max_cpus = 8;
mc->is_default = true;
mc->default_cpu_type = SPIKE_V1_10_0_CPU;
}
--
2.26.2
Re: [PATCH 1/4] target/arm: Don't use a TLB for ARMMMUIdx_Stage2
On 3/30/20 2:03 PM, Peter Maydell wrote: > We define ARMMMUIdx_Stage2 as being an MMU index which uses a QEMU > TLB. However we never actually use the TLB -- all stage 2 lookups > are done by direct calls to get_phys_addr_lpae() followed by a > physical address load via address_space_ld*(). > > Remove Stage2 from the list of ARM MMU indexes which correspond to > real core MMU indexes, and instead put it in the set of "NOTLB" ARM > MMU indexes. > > This allows us to drop NB_MMU_MODES to 11. It also means we can > safely add support for the ARMv8.3-TTS2UXN extension, which adds > permission bits to the stage 2 descriptors which define execute > permission separatel for EL0 and EL1; supporting that while keeping > Stage2 in a QEMU TLB would require us to use separate TLBs for > "Stage2 for an EL0 access" and "Stage2 for an EL1 access", which is a > lot of extra complication given we aren't even using the QEMU TLB. > > In the process of updating the comment on our MMU index use, > fix a couple of other minor errors: > * NS EL2 EL2&0 was missing from the list in the comment > * some text hadn't been updated from when we bumped NB_MMU_MODES >above 8 > > Signed-off-by: Peter Maydell > --- > target/arm/cpu-param.h | 2 +- > target/arm/cpu.h | 21 +--- > target/arm/helper.c| 112 - > 3 files changed, 27 insertions(+), 108 deletions(-) Reviewed-by: Richard Henderson r~
