Re: [Qemu-devel] [PATCH v6 01/12] qapi: Use predicate callback to determine visit filtering
Eric Blake writes:
> Previously, qapi-types and qapi-visit filtered out implicit
> objects during visit_object_type() by using 'info' (works since
> implicit objects do not [yet] have associated info); meanwhile
> qapi-introspect filtered out all schema types on the first pass
> by returning a python type from visit_begin(), which was then
> used in QAPISchema.visit(). Rather than keeping these ad hoc
> approaches, add a new visitor callback visit_predicate() which
> returns False to skip a given entity, and which defaults to
> True unless overridden. Use the new mechanism to simplify all
> three visitors that need filtering. No change to the generated
> code.
Let's call it visit_wanted().
> Suggested-by: Markus Armbruster
> Signed-off-by: Eric Blake
>
> ---
> v6: new patch
> ---
> scripts/qapi-introspect.py | 5 -
> scripts/qapi-types.py | 18 ++
> scripts/qapi-visit.py | 16 +---
> scripts/qapi.py| 11 +++
> 4 files changed, 30 insertions(+), 20 deletions(-)
>
> diff --git a/scripts/qapi-introspect.py b/scripts/qapi-introspect.py
> index 7d39320..f30fac3 100644
> --- a/scripts/qapi-introspect.py
> +++ b/scripts/qapi-introspect.py
> @@ -54,7 +54,6 @@ class QAPISchemaGenIntrospectVisitor(QAPISchemaVisitor):
> self._jsons = []
> self._used_types = []
> self._name_map = {}
> -return QAPISchemaType # don't visit types for now
>
> def visit_end(self):
> # visit the types that are actually used
> @@ -82,6 +81,10 @@ const char %(c_name)s[] = %(c_string)s;
> self._used_types = None
> self._name_map = None
>
> +def visit_predicate(self, entity):
> +# Ignore types on first pass; visit_end() will pick up used types
> +return not isinstance(entity, QAPISchemaType)
> +
> def _name(self, name):
> if self._unmask:
> return name
> diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
> index d405f8d..c5b71b0 100644
> --- a/scripts/qapi-types.py
> +++ b/scripts/qapi-types.py
> @@ -233,6 +233,9 @@ class QAPISchemaGenTypeVisitor(QAPISchemaVisitor):
> self.decl = self._btin + self.decl
> self._btin = None
>
> +def visit_predicate(self, entity):
> +return not isinstance(entity, QAPISchemaObjectType) or entity.info
> +
This is the faithful translation. But the left hand side is superfluous
because non-types laways have info, isn't it?
> def _gen_type_cleanup(self, name):
> self.decl += gen_type_cleanup_decl(name)
> self.defn += gen_type_cleanup(name)
> @@ -254,14 +257,13 @@ class QAPISchemaGenTypeVisitor(QAPISchemaVisitor):
> self._gen_type_cleanup(name)
>
> def visit_object_type(self, name, info, base, members, variants):
> -if info:
> -self._fwdecl += gen_fwd_object_or_array(name)
> -if variants:
> -assert not members # not implemented
> -self.decl += gen_union(name, base, variants)
> -else:
> -self.decl += gen_struct(name, base, members)
> -self._gen_type_cleanup(name)
> +self._fwdecl += gen_fwd_object_or_array(name)
> +if variants:
> +assert not members # not implemented
> +self.decl += gen_union(name, base, variants)
> +else:
> +self.decl += gen_struct(name, base, members)
> +self._gen_type_cleanup(name)
>
> def visit_alternate_type(self, name, info, variants):
> self._fwdecl += gen_fwd_object_or_array(name)
> diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
> index 4f97781..0f47614 100644
> --- a/scripts/qapi-visit.py
> +++ b/scripts/qapi-visit.py
> @@ -333,6 +333,9 @@ class QAPISchemaGenVisitVisitor(QAPISchemaVisitor):
> self.decl = self._btin + self.decl
> self._btin = None
>
> +def visit_predicate(self, entity):
> +return not isinstance(entity, QAPISchemaObjectType) or entity.info
> +
Likewise.
> def visit_enum_type(self, name, info, values, prefix):
> self.decl += gen_visit_decl(name, scalar=True)
> self.defn += gen_visit_enum(name)
> @@ -349,13 +352,12 @@ class QAPISchemaGenVisitVisitor(QAPISchemaVisitor):
> self.defn += defn
>
> def visit_object_type(self, name, info, base, members, variants):
> -if info:
> -self.decl += gen_visit_decl(name)
> -if variants:
> -assert not members # not implemented
> -self.defn += gen_visit_union(name, base, variants)
> -else:
> -self.defn += gen_visit_struct(name, base, members)
> +self.decl += gen_visit_decl(name)
> +if variants:
> +assert not members # not implemented
> +self.defn += gen_visit_union(name, base, variants)
> +else:
> +self.defn += gen_visit_struct(name, base, members)
>
[Qemu-devel] [PATCH qemu v4] monitor/target-ppc: Define target_get_monitor_def
At the moment get_monitor_def() returns only registers from statically
defined monitor_defs array. However there is a lot of BOOK3S SPRs
which are not in the list and cannot be printed from the monitor.
This adds a new target platform hook - target_get_monitor_def().
The hook is called if a register was not found in the static
array returned by the target_monitor_defs() hook.
The hook is only defined for POWERPC, it returns registered
SPRs and fails on unregistered ones providing the user with information
on what is actually supported on the running CPU. The register value is
saved as uint64_t as it is the biggest supported register size;
target_ulong cannot be used because of the stub - it is in a "common"
code and cannot include "cpu.h", etc; this is also why the hook prototype
is redefined in the stub instead of being included from some header.
This replaces static descriptors for GPRs, FPRs, SRs with a helper which
looks for a value in a corresponding array in the CPUPPCState.
The immediate effect is that all 32 SRs can be printed now (instead of 16);
later this can be reused for VSX or TM registers.
While we are here, this adds "cr" as a synonym of "ccr".
Signed-off-by: Alexey Kardashevskiy
---
Does it make sense to split it into two patches?
---
Changes:
v4:
* rebased on the current upstream which moved MonitorDef to target-ppc
* reverted the change for registers other than GPR/FPR/SPR, such as
FPSCR/PC/MSR/...
* removed CPU hook and made it a stub
v3:
* removed the check for endptr as strtoul() always initializes it
* check if there is any number after r/f/sr and fail if none
* added tolower()/strncasecmp() to support both r/f/sr and R/F/SR
v2:
* handles r**, f**, sr** if their numbers were parsed completely and correctly
* added "cr" as synonym of "ccr"
---
include/monitor/hmp-target.h | 1 +
monitor.c | 10 +-
stubs/Makefile.objs| 1 +
stubs/target-get-monitor-def.c | 31 ++
target-ppc/cpu-qom.h | 2 +
target-ppc/monitor.c | 219 -
6 files changed, 105 insertions(+), 159 deletions(-)
create mode 100644 stubs/target-get-monitor-def.c
diff --git a/include/monitor/hmp-target.h b/include/monitor/hmp-target.h
index 213566c..bc2c9c0 100644
--- a/include/monitor/hmp-target.h
+++ b/include/monitor/hmp-target.h
@@ -35,6 +35,7 @@ struct MonitorDef {
};
const MonitorDef *target_monitor_defs(void);
+int target_get_monitor_def(CPUState *cs, const char *name, uint64_t *pval);
CPUArchState *mon_get_cpu_env(void);
CPUState *mon_get_cpu(void);
diff --git a/monitor.c b/monitor.c
index 4f1ba2f..83e126a 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2104,6 +2104,8 @@ static int get_monitor_def(target_long *pval, const char
*name)
{
const MonitorDef *md = target_monitor_defs();
void *ptr;
+uint64_t tmp = 0;
+int ret;
if (md == NULL) {
return -1;
@@ -2131,7 +2133,13 @@ static int get_monitor_def(target_long *pval, const char
*name)
return 0;
}
}
-return -1;
+
+ret = target_get_monitor_def(mon_get_cpu(), name, &tmp);
+if (!ret) {
+*pval = (target_long) tmp;
+}
+
+return ret;
}
static void next(void)
diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index 85e4e81..91bcdbb 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -39,3 +39,4 @@ stub-obj-y += cpus.o
stub-obj-y += kvm.o
stub-obj-y += qmp_pc_dimm_device_list.o
stub-obj-y += target-monitor-defs.o
+stub-obj-y += target-get-monitor-def.o
diff --git a/stubs/target-get-monitor-def.c b/stubs/target-get-monitor-def.c
new file mode 100644
index 000..711a9ae
--- /dev/null
+++ b/stubs/target-get-monitor-def.c
@@ -0,0 +1,31 @@
+/*
+ * Stub for target_get_monitor_def.
+ *
+ * Copyright IBM Corp., 2015
+ *
+ * Author: Alexey Kardashevskiy
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License,
+ * or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see
Re: [Qemu-devel] [PATCH] target-tilegx: Call dest_gr() later when have to use it
On 10/2/15 03:30, Richard Henderson wrote: > On 10/01/2015 11:49 PM, gang.chen.5...@gmail.com wrote: >> From: Chen Gang >> >> When a nop instruction is generated, but the 'dest' is a valid (e.g. for >> any qemu skipped instructions, but still be useful in real machine), >> always allocate dest_gr() will cause issue for these nop instructions. >> >> After fix this issue, the temporary implementation of floating point >> instructions (which have skipped instructions) can work correctlly. > > Um, no. See the gen_rrr_opcode where we simply use two switch statements for > this. OK, thanks. -- Chen Gang (陈刚) Open, share, and attitude like air, water, and life which God blessed
Re: [Qemu-devel] [PATCH] target-tilegx: Call dest_gr() later when have to use it
On 10/01/2015 11:49 PM, gang.chen.5...@gmail.com wrote:
From: Chen Gang
When a nop instruction is generated, but the 'dest' is a valid (e.g. for
any qemu skipped instructions, but still be useful in real machine),
always allocate dest_gr() will cause issue for these nop instructions.
After fix this issue, the temporary implementation of floating point
instructions (which have skipped instructions) can work correctlly.
Um, no. See the gen_rrr_opcode where we simply use two switch statements for
this.
r~
Signed-off-by: Chen Gang
---
target-tilegx/translate.c | 214 +++---
1 file changed, 127 insertions(+), 87 deletions(-)
diff --git a/target-tilegx/translate.c b/target-tilegx/translate.c
index b7bb4f3..2ca5d43 100644
--- a/target-tilegx/translate.c
+++ b/target-tilegx/translate.c
@@ -30,6 +30,8 @@
#define FMT64X "%016" PRIx64
+#define TDEST dest_gr(dc, dest)
+
static TCGv_ptr cpu_env;
static TCGv cpu_pc;
static TCGv cpu_regs[TILEGX_R_COUNT];
@@ -547,18 +549,17 @@ static TileExcp gen_rr_opcode(DisasContext *dc, unsigned
opext,
return ret;
}
-tdest = dest_gr(dc, dest);
tsrca = load_gr(dc, srca);
switch (opext) {
case OE_RR_X0(CNTLZ):
case OE_RR_Y0(CNTLZ):
-gen_helper_cntlz(tdest, tsrca);
+gen_helper_cntlz(TDEST, tsrca);
mnemonic = "cntlz";
break;
case OE_RR_X0(CNTTZ):
case OE_RR_Y0(CNTTZ):
-gen_helper_cnttz(tdest, tsrca);
+gen_helper_cnttz(TDEST, tsrca);
mnemonic = "cnttz";
break;
case OE_RR_X0(FSINGLE_PACK1):
@@ -631,10 +632,11 @@ static TileExcp gen_rr_opcode(DisasContext *dc, unsigned
opext,
mnemonic = "ld";
do_load:
if (!prefetch_nofault) {
-tcg_gen_qemu_ld_tl(tdest, tsrca, dc->mmuidx, memop);
+tcg_gen_qemu_ld_tl(TDEST, tsrca, dc->mmuidx, memop);
}
break;
case OE_RR_X1(LDNA):
+tdest = dest_gr(dc, dest);
tcg_gen_andi_tl(tdest, tsrca, ~7);
tcg_gen_qemu_ld_tl(tdest, tdest, dc->mmuidx, MO_TEQ);
mnemonic = "ldna";
@@ -644,22 +646,22 @@ static TileExcp gen_rr_opcode(DisasContext *dc, unsigned
opext,
if (srca) {
return TILEGX_EXCP_OPCODE_UNIMPLEMENTED;
}
-tcg_gen_movi_tl(tdest, dc->pc + TILEGX_BUNDLE_SIZE_IN_BYTES);
+tcg_gen_movi_tl(TDEST, dc->pc + TILEGX_BUNDLE_SIZE_IN_BYTES);
mnemonic = "lnk";
break;
case OE_RR_X0(PCNT):
case OE_RR_Y0(PCNT):
-gen_helper_pcnt(tdest, tsrca);
+gen_helper_pcnt(TDEST, tsrca);
mnemonic = "pcnt";
break;
case OE_RR_X0(REVBITS):
case OE_RR_Y0(REVBITS):
-gen_helper_revbits(tdest, tsrca);
+gen_helper_revbits(TDEST, tsrca);
mnemonic = "revbits";
break;
case OE_RR_X0(REVBYTES):
case OE_RR_Y0(REVBYTES):
-tcg_gen_bswap64_tl(tdest, tsrca);
+tcg_gen_bswap64_tl(TDEST, tsrca);
mnemonic = "revbytes";
break;
case OE_RR_X0(TBLIDXB0):
@@ -682,7 +684,7 @@ static TileExcp gen_rr_opcode(DisasContext *dc, unsigned
opext,
static TileExcp gen_rrr_opcode(DisasContext *dc, unsigned opext,
unsigned dest, unsigned srca, unsigned srcb)
{
-TCGv tdest = dest_gr(dc, dest);
+TCGv tdest;
TCGv tsrca = load_gr(dc, srca);
TCGv tsrcb = load_gr(dc, srcb);
TCGv t0;
@@ -691,13 +693,14 @@ static TileExcp gen_rrr_opcode(DisasContext *dc, unsigned
opext,
switch (opext) {
case OE_RRR(ADDXSC, 0, X0):
case OE_RRR(ADDXSC, 0, X1):
-gen_saturate_op(tdest, tsrca, tsrcb, tcg_gen_add_tl);
+gen_saturate_op(TDEST, tsrca, tsrcb, tcg_gen_add_tl);
mnemonic = "addxsc";
break;
case OE_RRR(ADDX, 0, X0):
case OE_RRR(ADDX, 0, X1):
case OE_RRR(ADDX, 0, Y0):
case OE_RRR(ADDX, 0, Y1):
+tdest = dest_gr(dc, dest);
tcg_gen_add_tl(tdest, tsrca, tsrcb);
tcg_gen_ext32s_tl(tdest, tdest);
mnemonic = "addx";
@@ -706,25 +709,25 @@ static TileExcp gen_rrr_opcode(DisasContext *dc, unsigned
opext,
case OE_RRR(ADD, 0, X1):
case OE_RRR(ADD, 0, Y0):
case OE_RRR(ADD, 0, Y1):
-tcg_gen_add_tl(tdest, tsrca, tsrcb);
+tcg_gen_add_tl(TDEST, tsrca, tsrcb);
mnemonic = "add";
break;
case OE_RRR(AND, 0, X0):
case OE_RRR(AND, 0, X1):
case OE_RRR(AND, 5, Y0):
case OE_RRR(AND, 5, Y1):
-tcg_gen_and_tl(tdest, tsrca, tsrcb);
+tcg_gen_and_tl(TDEST, tsrca, tsrcb);
mnemonic = "and";
break;
case OE_RRR(CMOVEQZ, 0, X0):
case OE_RRR(CMOVEQZ, 4, Y0):
-tcg_gen_movcond_tl(TCG_COND_EQ, tdest, tsrca, load_zero(dc),
+tcg_gen_movcond_tl(TCG_COND_EQ, TDEST, tsrca, load_zero(dc),
[Qemu-devel] [PATCH v6 10/12] qapi: Correct error for union branch 'kind' clash
The error message when a simple union or alternate contains a
branch named 'kind' is ugly, because it is tied to the Schema
member named 'type'. A future patch will fix the generated C
to match QMP, but until that point, we can hack things with
a temporary subclass to make the error message reflect the
actually collision.
Rename alternate-clash to alternate-clash-members, and add a
new test alternate-clash-type. While similar to the earlier
addition of union-clash-type, we have one major difference: a
future patch will be simplifying alternates to not need an
implict AlternateKind enum, but we still need to detect the
collision with the resulting C 'qtype_code type;' tag.
No change to generated code.
Signed-off-by: Eric Blake
---
v6: New patch
---
scripts/qapi.py| 14 ++
tests/Makefile | 3 ++-
tests/qapi-schema/alternate-clash-members.err | 1 +
.../{alternate-clash.exit => alternate-clash-members.exit} | 0
.../{alternate-clash.json => alternate-clash-members.json} | 0
.../{alternate-clash.out => alternate-clash-members.out} | 0
tests/qapi-schema/alternate-clash-type.err | 1 +
tests/qapi-schema/alternate-clash-type.exit| 1 +
tests/qapi-schema/alternate-clash-type.json| 10 ++
tests/qapi-schema/alternate-clash-type.out | 0
tests/qapi-schema/alternate-clash.err | 1 -
tests/qapi-schema/union-clash-type.err | 2 +-
tests/qapi-schema/union-clash-type.json| 2 --
13 files changed, 30 insertions(+), 5 deletions(-)
create mode 100644 tests/qapi-schema/alternate-clash-members.err
rename tests/qapi-schema/{alternate-clash.exit =>
alternate-clash-members.exit} (100%)
rename tests/qapi-schema/{alternate-clash.json =>
alternate-clash-members.json} (100%)
rename tests/qapi-schema/{alternate-clash.out => alternate-clash-members.out}
(100%)
create mode 100644 tests/qapi-schema/alternate-clash-type.err
create mode 100644 tests/qapi-schema/alternate-clash-type.exit
create mode 100644 tests/qapi-schema/alternate-clash-type.json
create mode 100644 tests/qapi-schema/alternate-clash-type.out
delete mode 100644 tests/qapi-schema/alternate-clash.err
diff --git a/scripts/qapi.py b/scripts/qapi.py
index d90399a..6c224c6 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -1008,10 +1008,24 @@ class QAPISchemaObjectTypeVariants(object):
assert isinstance(self.tag_member.type, QAPISchemaEnumType)
for v in self.variants:
vseen = dict(seen)
+# TODO Ugly special case for simple unions, where the C member
+# is named 'kind' instead of 'type'.
+if not self.tag_name:
+vseen['kind'] = QAPISchemaKindHack('kind',
+ vseen.pop('type')._owner)
v.check(schema, info, self.tag_member.type, vseen,
self.tag_name is not None)
+# TODO Ugly hack for simple unions, where the C member is named 'kind'
+class QAPISchemaKindHack(QAPISchemaObjectTypeMember):
+def __init__(self, name, owner):
+QAPISchemaObjectTypeMember.__init__(self, name, '', False, owner)
+
+def describe(self):
+return "'%s' (implicit tag of %s)" % (self.name, self._owner)
+
+
class QAPISchemaObjectTypeVariant(QAPISchemaObjectTypeMember):
def __init__(self, name, typ, owner):
QAPISchemaObjectTypeMember.__init__(self, name, typ, False, owner)
diff --git a/tests/Makefile b/tests/Makefile
index 79fb154..db0f9a6 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -223,7 +223,8 @@ $(foreach target,$(SYSEMU_TARGET_LIST), \
qapi-schema += alternate-array.json
qapi-schema += alternate-base.json
-qapi-schema += alternate-clash.json
+qapi-schema += alternate-clash-members.json
+qapi-schema += alternate-clash-type.json
qapi-schema += alternate-conflict-dict.json
qapi-schema += alternate-conflict-string.json
qapi-schema += alternate-empty.json
diff --git a/tests/qapi-schema/alternate-clash-members.err
b/tests/qapi-schema/alternate-clash-members.err
new file mode 100644
index 000..0adf737
--- /dev/null
+++ b/tests/qapi-schema/alternate-clash-members.err
@@ -0,0 +1 @@
+tests/qapi-schema/alternate-clash-members.json:7: Alternate 'Alt1' branch
'a_b' clashes with 'a-b'
diff --git a/tests/qapi-schema/alternate-clash.exit
b/tests/qapi-schema/alternate-clash-members.exit
similarity index 100%
rename from tests/qapi-schema/alternate-clash.exit
rename to tests/qapi-schema/alternate-clash-members.exit
diff --git a/tests/qapi-schema/alternate-clash.json
b/tests/qapi-schema/alternate-clash-members.json
similarity index 100%
rename from tests/qapi-schema/alternate-clash.json
rename to tests/qapi-schema/alternate-clash-members.json
diff --git a/tests/qapi-schema/alternate-clash.out
b/tests/qapi-schema/alternate-cla
[Qemu-devel] [PATCH v6 09/12] qapi: Defer duplicate enum value checks to schema check()
Similar to the previous commit, move the detection of a collision
in enum values from parse time to QAPISchemaEnumType.check().
This happens to also detect collisions in union branch names,
so for a decent error message, we have to determine if the enum
is implicit (and if so where the real collision lies).
Testing this showed that the test union-bad-branch and
union-clash-branches were basically testing the same thing;
with the minor difference that the former clashes only in the
enum, while the latter also clashes in the C union member
names that would be generated. So delete the weaker test.
However, the union-clash-type test exposes one weakness in this
patch - it claims the collision is with 'type', even though the
C struct only has a duplicate 'kind'. We will eventually fix
the generated C to use 'type' like QMP already does, but in the
short term, the next patch will fix things to find the correct
collision.
No change to generated code.
Signed-off-by: Eric Blake
---
v6: new patch
---
scripts/qapi.py| 44 --
tests/Makefile | 1 -
tests/qapi-schema/alternate-clash.err | 2 +-
tests/qapi-schema/enum-clash-member.err| 2 +-
tests/qapi-schema/enum-max-member.err | 2 +-
tests/qapi-schema/union-bad-branch.err | 1 -
tests/qapi-schema/union-bad-branch.exit| 1 -
tests/qapi-schema/union-bad-branch.json| 8 --
tests/qapi-schema/union-bad-branch.out | 0
tests/qapi-schema/union-clash-branches.err | 2 +-
tests/qapi-schema/union-clash-type.err | 2 +-
tests/qapi-schema/union-clash-type.json| 2 ++
tests/qapi-schema/union-max.err| 2 +-
13 files changed, 25 insertions(+), 44 deletions(-)
delete mode 100644 tests/qapi-schema/union-bad-branch.err
delete mode 100644 tests/qapi-schema/union-bad-branch.exit
delete mode 100644 tests/qapi-schema/union-bad-branch.json
delete mode 100644 tests/qapi-schema/union-bad-branch.out
diff --git a/scripts/qapi.py b/scripts/qapi.py
index e58c5a8..d90399a 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -530,7 +530,6 @@ def check_union(expr, expr_info):
base = expr.get('base')
discriminator = expr.get('discriminator')
members = expr['data']
-values = {'MAX': '(automatic)', 'KIND': '(automatic)'}
# Two types of unions, determined by discriminator.
@@ -591,34 +590,16 @@ def check_union(expr, expr_info):
"enum '%s'" %
(key, enum_define["enum_name"]))
-# Otherwise, check for conflicts in the generated enum
-else:
-c_key = camel_to_upper(key)
-if c_key in values:
-raise QAPIExprError(expr_info,
-"Union '%s' member '%s' clashes with '%s'"
-% (name, key, values[c_key]))
-values[c_key] = key
-
def check_alternate(expr, expr_info):
name = expr['alternate']
members = expr['data']
-values = {'MAX': '(automatic)'}
types_seen = {}
# Check every branch
for (key, value) in members.items():
check_name(expr_info, "Member of alternate '%s'" % name, key)
-# Check for conflicts in the generated enum
-c_key = camel_to_upper(key)
-if c_key in values:
-raise QAPIExprError(expr_info,
-"Alternate '%s' member '%s' clashes with '%s'"
-% (name, key, values[c_key]))
-values[c_key] = key
-
# Ensure alternates have no type conflicts.
check_type(expr_info, "Member '%s' of alternate '%s'" % (key, name),
value,
@@ -637,7 +618,6 @@ def check_enum(expr, expr_info):
name = expr['enum']
members = expr.get('data')
prefix = expr.get('prefix')
-values = {'MAX': '(automatic)'}
if not isinstance(members, list):
raise QAPIExprError(expr_info,
@@ -648,12 +628,6 @@ def check_enum(expr, expr_info):
for member in members:
check_name(expr_info, "Member of enum '%s'" % name, member,
enum_member=True)
-key = camel_to_upper(member)
-if key in values:
-raise QAPIExprError(expr_info,
-"Enum '%s' member '%s' clashes with '%s'"
-% (name, member, values[key]))
-values[key] = member
def check_struct(expr, expr_info):
@@ -873,7 +847,23 @@ class QAPISchemaEnumType(QAPISchemaType):
self.prefix = prefix
def check(self, schema):
-assert len(set(self.values)) == len(self.values)
+seen = {c_enum_const(self.name, 'MAX'): '(automatic MAX)'}
+for value in self.values:
+c_value = c_enum_const(self.name, value)
+if c_value in seen:
+if self.name[-4:] == 'Kind':
+owner = schema.lookup_type(self.name[
[Qemu-devel] [PATCH v6 07/12] qapi: Detect collisions in C member names
Detect attempts to declare two object members that would result
in the same C member name, by keying the 'seen' dictionary off
of the C name rather than the qapi name. Doing this was made
easier by adding a member.c_name() helper function.
As this is the first error raised within the QAPISchema*.check()
methods, we also have to pass 'info' through the call stack, and
fix the overall 'try' to display errors detected during the
check() phase.
This fixes a couple of previously-broken tests, and the
resulting error messages demonstrate the utility of the
.describe() method added previously. No change to generated
code.
Signed-off-by: Eric Blake
---
v6: rebase to earlier testsuite and info improvements
---
scripts/qapi.py| 46 --
tests/qapi-schema/args-name-clash.err | 1 +
tests/qapi-schema/args-name-clash.exit | 2 +-
tests/qapi-schema/args-name-clash.json | 6 ++--
tests/qapi-schema/args-name-clash.out | 6
tests/qapi-schema/flat-union-clash-branch.err | 1 +
tests/qapi-schema/flat-union-clash-branch.exit | 2 +-
tests/qapi-schema/flat-union-clash-branch.json | 9 ++---
tests/qapi-schema/flat-union-clash-branch.out | 14
9 files changed, 40 insertions(+), 47 deletions(-)
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 880de94..1acf02b 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -103,6 +103,7 @@ class QAPISchemaError(Exception):
class QAPIExprError(Exception):
def __init__(self, expr_info, msg):
Exception.__init__(self)
+assert expr_info
self.info = expr_info
self.msg = msg
@@ -964,11 +965,12 @@ class QAPISchemaObjectType(QAPISchemaType):
members = []
seen = {}
for m in members:
-seen[m.name] = m
+assert m.c_name() not in seen
+seen[m.c_name()] = m
for m in self.local_members:
-m.check(schema, members, seen)
+m.check(schema, self.info, members, seen)
if self.variants:
-self.variants.check(schema, members, seen)
+self.variants.check(schema, self.info, members, seen)
self.members = members
def is_implicit(self):
@@ -1004,12 +1006,19 @@ class QAPISchemaObjectTypeMember(object):
self.optional = optional
self._owner = owner
-def check(self, schema, all_members, seen):
-assert self.name not in seen
+def check(self, schema, info, all_members, seen):
+if self.c_name() in seen:
+raise QAPIExprError(info,
+"%s collides with %s"
+% (self.describe(),
+ seen[self.c_name()].describe()))
self.type = schema.lookup_type(self._type_name)
assert self.type
all_members.append(self)
-seen[self.name] = self
+seen[self.c_name()] = self
+
+def c_name(self):
+return c_name(self.name)
def describe(self):
return "'%s' (member of %s)" % (self.name, self._owner)
@@ -1028,23 +1037,24 @@ class QAPISchemaObjectTypeVariants(object):
self.tag_member = tag_member
self.variants = variants
-def check(self, schema, members, seen):
+def check(self, schema, info, members, seen):
if self.tag_name:
-self.tag_member = seen[self.tag_name]
+self.tag_member = seen[c_name(self.tag_name)]
+assert self.tag_name == self.tag_member.name
else:
-self.tag_member.check(schema, members, seen)
+self.tag_member.check(schema, info, members, seen)
assert isinstance(self.tag_member.type, QAPISchemaEnumType)
for v in self.variants:
vseen = dict(seen)
-v.check(schema, self.tag_member.type, vseen)
+v.check(schema, info, self.tag_member.type, vseen)
class QAPISchemaObjectTypeVariant(QAPISchemaObjectTypeMember):
def __init__(self, name, typ, owner):
QAPISchemaObjectTypeMember.__init__(self, name, typ, False, owner)
-def check(self, schema, tag_type, seen):
-QAPISchemaObjectTypeMember.check(self, schema, [], seen)
+def check(self, schema, info, tag_type, seen):
+QAPISchemaObjectTypeMember.check(self, schema, info, [], seen)
assert self.name in tag_type.values
def describe(self):
@@ -1069,7 +1079,7 @@ class QAPISchemaAlternateType(QAPISchemaType):
self.variants = variants
def check(self, schema):
-self.variants.check(schema, [], {})
+self.variants.check(schema, self.info, [], {})
def json_type(self):
return 'value'
@@ -1128,14 +1138,14 @@ class QAPISchema(object):
def __init__(self, fname):
try:
self.exprs = check_exprs(QAPISchemaParser(open(fname, "r")).exprs)
+self._entity_dict = {}
+self._def_predefineds()
+QAPISch
[Qemu-devel] [PATCH v6 11/12] qapi: Detect base class loops
It should be fairly obvious that qapi base classes need to
form an acyclic graph, since QMP cannot specify the same
key more than once, while base classes are included as flat
members alongside other members added by the child. But prior
to Markus' introspection commits (such as commit 75ebcd7f),
the test in isolation would cause python to exit with a
complaint about unbounded nesting; and after his patches (in
particular ac88219a), it triggers an assertion failure. This
patch includes both the test and the fix, since the .err file
output for an assertion failure is too hard to rebase when
other patches cause line number changes.
Signed-off-by: Eric Blake
---
v6: rebase to earlier info changes
---
scripts/qapi.py | 6 +-
tests/Makefile| 1 +
tests/qapi-schema/base-cycle.err | 1 +
tests/qapi-schema/base-cycle.exit | 1 +
tests/qapi-schema/base-cycle.json | 3 +++
tests/qapi-schema/base-cycle.out | 0
6 files changed, 11 insertions(+), 1 deletion(-)
create mode 100644 tests/qapi-schema/base-cycle.err
create mode 100644 tests/qapi-schema/base-cycle.exit
create mode 100644 tests/qapi-schema/base-cycle.json
create mode 100644 tests/qapi-schema/base-cycle.out
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 6c224c6..c226cd9 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -913,7 +913,11 @@ class QAPISchemaObjectType(QAPISchemaType):
self.members = None
def check(self, schema):
-assert self.members is not False# not running in cycles
+if self.members is False: # check for cycles
+assert self._base_name
+raise QAPIExprError(self.info,
+"Object %s cyclically depends on %s"
+% (self.name, self._base_name))
if self.members:
return
self.members = False# mark as being checked
diff --git a/tests/Makefile b/tests/Makefile
index db0f9a6..8ae8140 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -249,6 +249,7 @@ qapi-schema += bad-ident.json
qapi-schema += bad-type-bool.json
qapi-schema += bad-type-dict.json
qapi-schema += bad-type-int.json
+qapi-schema += base-cycle.json
qapi-schema += command-int.json
qapi-schema += comments.json
qapi-schema += double-data.json
diff --git a/tests/qapi-schema/base-cycle.err b/tests/qapi-schema/base-cycle.err
new file mode 100644
index 000..e0221b5
--- /dev/null
+++ b/tests/qapi-schema/base-cycle.err
@@ -0,0 +1 @@
+tests/qapi-schema/base-cycle.json:2: Object Base1 cyclically depends on Base2
diff --git a/tests/qapi-schema/base-cycle.exit
b/tests/qapi-schema/base-cycle.exit
new file mode 100644
index 000..d00491f
--- /dev/null
+++ b/tests/qapi-schema/base-cycle.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/base-cycle.json
b/tests/qapi-schema/base-cycle.json
new file mode 100644
index 000..2866772
--- /dev/null
+++ b/tests/qapi-schema/base-cycle.json
@@ -0,0 +1,3 @@
+# we reject a loop in base classes
+{ 'struct': 'Base1', 'base': 'Base2', 'data': {} }
+{ 'struct': 'Base2', 'base': 'Base1', 'data': {} }
diff --git a/tests/qapi-schema/base-cycle.out b/tests/qapi-schema/base-cycle.out
new file mode 100644
index 000..e69de29
--
2.4.3
[Qemu-devel] [PATCH v6 06/12] qapi: Track owner of each object member
Future commits will migrate semantic checking away from parsing
and over to the various QAPISchema*.check() methods. But to
report an error message about an incorrect semantic use of a
member of an object type, we need to know which type, command,
or event owns the member. Rather than making all the check()
methods have to pass around additional information, it is easier
to have each member track who owns it in the first place.
The source information is intended for human consumption in
error messages, and a new describe() method is added to access
the resulting information. For example, given the qapi:
{ 'command': 'foo', 'data': { 'string': 'str' } }
an implementation of visit_command() that calls
arg_type.members[0].describe()
will see "'string' (member of foo arguments)".
Where implicit types are involved, the code intentionally tries
to pick the name of the owner of that implicit type, rather than
the type name itself (a user reading the error message should be
able to grep for the problem in their original file, but will not
be able to locate a generated implicit name).
No change to generated code.
Signed-off-by: Eric Blake
---
v6: rebase on new lazy array creation and simple union 'type'
motion; tweak commit message
---
scripts/qapi.py | 53 +
1 file changed, 33 insertions(+), 20 deletions(-)
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 19cca97..880de94 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -993,14 +993,16 @@ class QAPISchemaObjectType(QAPISchemaType):
class QAPISchemaObjectTypeMember(object):
-def __init__(self, name, typ, optional):
+def __init__(self, name, typ, optional, owner):
assert isinstance(name, str)
assert isinstance(typ, str)
assert isinstance(optional, bool)
+assert isinstance(owner, str) and ':' not in owner
self.name = name
self._type_name = typ
self.type = None
self.optional = optional
+self._owner = owner
def check(self, schema, all_members, seen):
assert self.name not in seen
@@ -1009,6 +1011,9 @@ class QAPISchemaObjectTypeMember(object):
all_members.append(self)
seen[self.name] = self
+def describe(self):
+return "'%s' (member of %s)" % (self.name, self._owner)
+
class QAPISchemaObjectTypeVariants(object):
def __init__(self, tag_name, tag_member, variants):
@@ -1035,13 +1040,16 @@ class QAPISchemaObjectTypeVariants(object):
class QAPISchemaObjectTypeVariant(QAPISchemaObjectTypeMember):
-def __init__(self, name, typ):
-QAPISchemaObjectTypeMember.__init__(self, name, typ, False)
+def __init__(self, name, typ, owner):
+QAPISchemaObjectTypeMember.__init__(self, name, typ, False, owner)
def check(self, schema, tag_type, seen):
QAPISchemaObjectTypeMember.check(self, schema, [], seen)
assert self.name in tag_type.values
+def describe(self):
+return "'%s' (branch of %s)" % (self.name, self._owner)
+
# This function exists to support ugly simple union special cases
# TODO get rid of them, and drop the function
def simple_union_type(self):
@@ -1193,7 +1201,7 @@ class QAPISchema(object):
prefix = expr.get('prefix')
self._def_entity(QAPISchemaEnumType(name, info, data, prefix))
-def _make_member(self, name, typ, info):
+def _make_member(self, name, typ, info, owner):
optional = False
if name.startswith('*'):
name = name[1:]
@@ -1201,10 +1209,10 @@ class QAPISchema(object):
if isinstance(typ, list):
assert len(typ) == 1
typ = self._make_array_type(typ[0], info)
-return QAPISchemaObjectTypeMember(name, typ, optional)
+return QAPISchemaObjectTypeMember(name, typ, optional, owner)
-def _make_members(self, data, info):
-return [self._make_member(key, value, info)
+def _make_members(self, data, info, owner):
+return [self._make_member(key, value, info, owner)
for (key, value) in data.iteritems()]
def _def_struct_type(self, expr, info):
@@ -1212,25 +1220,26 @@ class QAPISchema(object):
base = expr.get('base')
data = expr['data']
self._def_entity(QAPISchemaObjectType(name, info, base,
- self._make_members(data, info),
+ self._make_members(data, info,
+ name),
None))
-def _make_variant(self, case, typ):
-return QAPISchemaObjectTypeVariant(case, typ)
+def _make_variant(self, case, typ, owner):
+return QAPISchemaObjectTypeVariant(case, typ, owner)
-def _make_simple_variant(self, case, typ, info):
+def _make_simple_variant(self, case, typ, info, owner):
if isinstance(typ, list):
[Qemu-devel] [PATCH v6 02/12] qapi: Don't use info as witness of implicit object type
A future patch will enable error reporting from the various QAPISchema*.check() methods. But to report an error related to an implicit type, we'll need to associate a location with the type (the same location as the top-level entity that is causing the creation of the implicit type), and once we do that, keying off of whether foo.info exists is no longer a viable way to determine if foo is an implicit type. Instead, add an is_implicit() method to QAPISchemaObjectType, and use that function where needed. (Done at the ObjectType level, since we already know all builtins and arrays are implicit, no commands or events are implicit, and we don't have any differences in generated code for regular vs. implicit enums.) Signed-off-by: Eric Blake --- v6: split 11/46 into pieces; don't rename _info yet; rework atop nicer filtering mechanism, including no need to change visitor signature --- scripts/qapi-types.py | 3 ++- scripts/qapi-visit.py | 3 ++- scripts/qapi.py | 10 +++--- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py index c5b71b0..6bac5b3 100644 --- a/scripts/qapi-types.py +++ b/scripts/qapi-types.py @@ -234,7 +234,8 @@ class QAPISchemaGenTypeVisitor(QAPISchemaVisitor): self._btin = None def visit_predicate(self, entity): -return not isinstance(entity, QAPISchemaObjectType) or entity.info +return not (isinstance(entity, QAPISchemaObjectType) and +entity.is_implicit()) def _gen_type_cleanup(self, name): self.decl += gen_type_cleanup_decl(name) diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py index 0f47614..2957c85 100644 --- a/scripts/qapi-visit.py +++ b/scripts/qapi-visit.py @@ -334,7 +334,8 @@ class QAPISchemaGenVisitVisitor(QAPISchemaVisitor): self._btin = None def visit_predicate(self, entity): -return not isinstance(entity, QAPISchemaObjectType) or entity.info +return not (isinstance(entity, QAPISchemaObjectType) and +entity.is_implicit()) def visit_enum_type(self, name, info, values, prefix): self.decl += gen_visit_decl(name, scalar=True) diff --git a/scripts/qapi.py b/scripts/qapi.py index 7d359c8..8123ab3 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -970,12 +970,15 @@ class QAPISchemaObjectType(QAPISchemaType): self.variants.check(schema, members, seen) self.members = members +def is_implicit(self): +return self.name[0] == ':' + def c_name(self): -assert self.info +assert not self.is_implicit() return QAPISchemaType.c_name(self) def c_type(self, is_param=False): -assert self.info +assert not self.is_implicit() return QAPISchemaType.c_type(self) def json_type(self): @@ -1043,7 +1046,8 @@ class QAPISchemaObjectTypeVariant(QAPISchemaObjectTypeMember): # This function exists to support ugly simple union special cases # TODO get rid of them, and drop the function def simple_union_type(self): -if isinstance(self.type, QAPISchemaObjectType) and not self.type.info: +if isinstance(self.type, + QAPISchemaObjectType) and self.type.is_implicit(): assert len(self.type.members) == 1 assert not self.type.variants return self.type.members[0].type -- 2.4.3
[Qemu-devel] [PATCH v6 03/12] qapi: Lazy creation of array types
Commit ac88219a had several TODO markers about whether we needed
to automatically create the corresponding array type alongside
any other type. It turns out that most of the time, we don't!
As part of lazy creation of array types, this patch now assigns
an 'info' to array types at their point of first instantiation,
rather than leaving it None.
There are a few exceptions: 1) We have a few situations where we
use an array type in internal code but do not expose that type
through QMP; fix it by declaring a dummy type that forces the
generator to see that we want to use the array type.
2) The builtin arrays (such as intList for QAPI ['int']) must
always be generated, because of the way our QAPI_TYPES_BUILTIN
compile guard works: we have situations (at the very least
tests/test-qmp-output-visitor.c) that include both top-level
"qapi-types.h" (via "error.h") and a secondary
"test-qapi-types.h". If we were to only emit the builtin types
when used locally, then the first .h file would not include all
types, but the second .h does not declare anything at all because
the first .h set QAPI_TYPES_BUILTIN, and we would end up with
compilation error due to things like unknown type 'int8List'.
Actually, we may need to revisit how we do type guards, and
change from a single QAPI_TYPES_BUILTIN over to a different
usage pattern that does one #ifdef per qapi type - right now,
the only types that are declared multiple times between two qapi
.json files for inclusion by a single .c file happen to be the
builtin arrays. But now that we have QAPI 'include' statements,
it is logical to assume that we will soon reach a point where
we want to reuse non-builtin types (yes, I'm thinking about what
it will take to add introspection to QGA, where we will want to
reuse the SchemaInfo type and friends). One #ifdef per type
will help ensure that generating the same qapi type into more
than one qapi-types.h won't cause collisions when both are
included in the same .c file; but we also have to solve how to
avoid creating duplicate qapi-types.c entry points. So that
is a problem left for another day.
Interestingly, the introspection output is unchanged - this is
because we already cull all types that are not indirectly
reachable from a command or event, so introspection was already
using only a subset of array types. The subset of types
introspected is now a much larger percentage of the overall set
of array types emitted in qapi-types.h (since the larger set
shrunk), but still not 100% (proof that the array types emitted
for our new Dummy struct, and the new Dummy struct itself, don't
affect QMP). Of course, the rest of the generated output is
drastically shrunk (a number of unused array types are no longer
generated) [for best results, diff the generated files with
'git diff --patience --no-index pre post'].
Signed-off-by: Eric Blake
---
v6: new patch
---
qapi-schema.json| 10 +
scripts/qapi.py | 39 -
tests/qapi-schema/qapi-schema-test.json | 4
tests/qapi-schema/qapi-schema-test.out | 3 +++
4 files changed, 36 insertions(+), 20 deletions(-)
diff --git a/qapi-schema.json b/qapi-schema.json
index 8b0520c..98bf0b2 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3396,6 +3396,16 @@
'features': 'int' } }
##
+# @Dummy
+#
+# Not used by QMP; hack to let us use X86CPUFeatureWordInfoList internally
+#
+# Since 2.5
+##
+{ 'struct': 'Dummy', 'data': { 'unused': ['X86CPUFeatureWordInfo'] } }
+
+
+##
# @RxState:
#
# Packets receiving state
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 8123ab3..15640b6 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -1143,7 +1143,7 @@ class QAPISchema(object):
def _def_builtin_type(self, name, json_type, c_type, c_null):
self._def_entity(QAPISchemaBuiltinType(name, json_type,
c_type, c_null))
-self._make_array_type(name) # TODO really needed?
+self._make_array_type(name, None)
def _def_predefineds(self):
for t in [('str','string', 'char' + pointer_suffix, 'NULL'),
@@ -1170,10 +1170,10 @@ class QAPISchema(object):
self._def_entity(QAPISchemaEnumType(name, None, values, None))
return name
-def _make_array_type(self, element_type):
+def _make_array_type(self, element_type, info):
name = element_type + 'List'
if not self.lookup_type(name):
-self._def_entity(QAPISchemaArrayType(name, None, element_type))
+self._def_entity(QAPISchemaArrayType(name, info, element_type))
return name
def _make_implicit_object_type(self, name, role, members):
@@ -1190,20 +1190,19 @@ class QAPISchema(object):
data = expr['data']
prefix = expr.get('prefix')
self._def_entity(QAPISchemaEnumType(name, info, data, prefix))
-self._make_array_type(name) # TODO really needed?
-
[Qemu-devel] [PATCH v6 12/12] RFC: qapi: Hide _info member
Now that nothing but sub-classes are using QAPISchemaEntity.info,
rename it to _info to ensure that external users do not revert
to using it again.
Signed-off-by: Eric Blake
---
v6: split from 11/46; probably worth dropping
---
scripts/qapi.py | 28 ++--
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/scripts/qapi.py b/scripts/qapi.py
index c226cd9..7b8d976 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -740,7 +740,7 @@ class QAPISchemaEntity(object):
assert isinstance(name, str)
self.name = name
assert info or not QAPISchema.predefined_initialized
-self.info = info
+self._info = info
def c_name(self):
return c_name(self.name)
@@ -834,7 +834,7 @@ class QAPISchemaBuiltinType(QAPISchemaType):
return self._json_type_name
def visit(self, visitor):
-visitor.visit_builtin_type(self.name, self.info, self.json_type())
+visitor.visit_builtin_type(self.name, self._info, self.json_type())
class QAPISchemaEnumType(QAPISchemaType):
@@ -860,7 +860,7 @@ class QAPISchemaEnumType(QAPISchemaType):
description = "Union '%s' branch" % owner.name
else:
description = "Enum '%s' value" % self.name
-raise QAPIExprError(self.info,
+raise QAPIExprError(self._info,
"%s '%s' clashes with '%s'"
% (description, value, seen[c_value]))
seen[c_value] = value
@@ -876,7 +876,7 @@ class QAPISchemaEnumType(QAPISchemaType):
return 'string'
def visit(self, visitor):
-visitor.visit_enum_type(self.name, self.info,
+visitor.visit_enum_type(self.name, self._info,
self.values, self.prefix)
@@ -895,7 +895,7 @@ class QAPISchemaArrayType(QAPISchemaType):
return 'array'
def visit(self, visitor):
-visitor.visit_array_type(self.name, self.info, self.element_type)
+visitor.visit_array_type(self.name, self._info, self.element_type)
class QAPISchemaObjectType(QAPISchemaType):
@@ -915,7 +915,7 @@ class QAPISchemaObjectType(QAPISchemaType):
def check(self, schema):
if self.members is False: # check for cycles
assert self._base_name
-raise QAPIExprError(self.info,
+raise QAPIExprError(self._info,
"Object %s cyclically depends on %s"
% (self.name, self._base_name))
if self.members:
@@ -934,9 +934,9 @@ class QAPISchemaObjectType(QAPISchemaType):
assert m.c_name() not in seen
seen[m.c_name()] = m
for m in self.local_members:
-m.check(schema, self.info, members, seen)
+m.check(schema, self._info, members, seen)
if self.variants:
-self.variants.check(schema, self.info, members, seen)
+self.variants.check(schema, self._info, members, seen)
self.members = members
def is_implicit(self):
@@ -954,9 +954,9 @@ class QAPISchemaObjectType(QAPISchemaType):
return 'object'
def visit(self, visitor):
-visitor.visit_object_type(self.name, self.info,
+visitor.visit_object_type(self.name, self._info,
self.base, self.local_members, self.variants)
-visitor.visit_object_type_flat(self.name, self.info,
+visitor.visit_object_type_flat(self.name, self._info,
self.members, self.variants)
@@ -1071,13 +1071,13 @@ class QAPISchemaAlternateType(QAPISchemaType):
self.variants = variants
def check(self, schema):
-self.variants.check(schema, self.info, [], {})
+self.variants.check(schema, self._info, [], {})
def json_type(self):
return 'value'
def visit(self, visitor):
-visitor.visit_alternate_type(self.name, self.info, self.variants)
+visitor.visit_alternate_type(self.name, self._info, self.variants)
class QAPISchemaCommand(QAPISchemaEntity):
@@ -1102,7 +1102,7 @@ class QAPISchemaCommand(QAPISchemaEntity):
assert isinstance(self.ret_type, QAPISchemaType)
def visit(self, visitor):
-visitor.visit_command(self.name, self.info,
+visitor.visit_command(self.name, self._info,
self.arg_type, self.ret_type,
self.gen, self.success_response)
@@ -1121,7 +1121,7 @@ class QAPISchemaEvent(QAPISchemaEntity):
assert not self.arg_type.variants # not implemented
def visit(self, visitor):
-visitor.visit_event(self.name, self.info, self.arg_type)
+visitor.visit_event(self.name, self._info, self.arg_type)
class QAPISchema(object):
--
2.4.3
[Qemu-devel] [PATCH v6 05/12] qapi: Track location that created an implicit type
A future patch will enable deferred error detection in the
various QAPISchema*.check() methods (rather than the current
ad hoc parse checks). But that means the user can request
a QAPI entity that will only fail validation after it has
been initialized. Since all errors have to have an
associated 'info' location, we need a location to be
associated with all user-triggered implicit types. The
intuitive info to use is the location of the enclosing
entity that caused the creation of the implicit type.
Note that we do not anticipate builtin types being used in
an error message (as they are not part of the user's QAPI
input, the user can't cause a semantic error in their
behavior), so we exempt those types from requiring info, by
setting a flag to track the completion of _def_predefineds().
No change to the generated code.
RFC: I used a class-level static flag to track whether we expected
'info is None' when creating a QAPISchemaEntity. This is gross,
because the flag will only be set on the first QAPISchema() instance
(it works because none of our client scripts ever instantiate more
than one schema). But the only other thing I could think of would
be passing the QAPISchema instance into the constructor for each
QAPISchemaEntity, which is a lot of churn. Any better ideas on how
best to do the assertion, or should I just drop it?
Signed-off-by: Eric Blake
---
v6: improve commit message, track implicit enum info, rebase
on new lazy array handling
---
scripts/qapi.py | 26 +++---
1 file changed, 15 insertions(+), 11 deletions(-)
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 255001a..19cca97 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -792,6 +792,7 @@ class QAPISchemaEntity(object):
def __init__(self, name, info):
assert isinstance(name, str)
self.name = name
+assert info or not QAPISchema.predefined_initialized
self.info = info
def c_name(self):
@@ -1114,6 +1115,8 @@ class QAPISchemaEvent(QAPISchemaEntity):
class QAPISchema(object):
+predefined_initialized = False
+
def __init__(self, fname):
try:
self.exprs = check_exprs(QAPISchemaParser(open(fname, "r")).exprs)
@@ -1122,6 +1125,7 @@ class QAPISchema(object):
exit(1)
self._entity_dict = {}
self._def_predefineds()
+QAPISchema.predefined_initialized = True
self._def_exprs()
self.check()
@@ -1163,9 +1167,9 @@ class QAPISchema(object):
[], None)
self._def_entity(self.the_empty_object_type)
-def _make_implicit_enum_type(self, name, values):
+def _make_implicit_enum_type(self, name, info, values):
name = name + 'Kind'
-self._def_entity(QAPISchemaEnumType(name, None, values, None))
+self._def_entity(QAPISchemaEnumType(name, info, values, None))
return name
def _make_array_type(self, element_type, info):
@@ -1174,12 +1178,12 @@ class QAPISchema(object):
self._def_entity(QAPISchemaArrayType(name, info, element_type))
return name
-def _make_implicit_object_type(self, name, role, members):
+def _make_implicit_object_type(self, name, info, role, members):
if not members:
return None
name = ':obj-%s-%s' % (name, role)
if not self.lookup_entity(name, QAPISchemaObjectType):
-self._def_entity(QAPISchemaObjectType(name, None, None,
+self._def_entity(QAPISchemaObjectType(name, info, None,
members, None))
return name
@@ -1218,13 +1222,13 @@ class QAPISchema(object):
if isinstance(typ, list):
assert len(typ) == 1
typ = self._make_array_type(typ[0], info)
-typ = self._make_implicit_object_type(typ, 'wrapper',
+typ = self._make_implicit_object_type(typ, info, 'wrapper',
[self._make_member('data', typ,
info)])
return QAPISchemaObjectTypeVariant(case, typ)
-def _make_tag_enum(self, type_name, variants):
-typ = self._make_implicit_enum_type(type_name,
+def _make_tag_enum(self, type_name, info, variants):
+typ = self._make_implicit_enum_type(type_name, info,
[v.name for v in variants])
return QAPISchemaObjectTypeMember('type', typ, False)
@@ -1240,7 +1244,7 @@ class QAPISchema(object):
else:
variants = [self._make_simple_variant(key, value, info)
for (key, value) in data.iteritems()]
-tag_enum = self._make_tag_enum(name, variants)
+tag_enum = self._make_tag_enum(name, info, variants)
self._def_entity(
QAPISchemaObjectType(name, info, base,
self._make_members(Ord
[Qemu-devel] [PATCH v6 01/12] qapi: Use predicate callback to determine visit filtering
Previously, qapi-types and qapi-visit filtered out implicit
objects during visit_object_type() by using 'info' (works since
implicit objects do not [yet] have associated info); meanwhile
qapi-introspect filtered out all schema types on the first pass
by returning a python type from visit_begin(), which was then
used in QAPISchema.visit(). Rather than keeping these ad hoc
approaches, add a new visitor callback visit_predicate() which
returns False to skip a given entity, and which defaults to
True unless overridden. Use the new mechanism to simplify all
three visitors that need filtering. No change to the generated
code.
Suggested-by: Markus Armbruster
Signed-off-by: Eric Blake
---
v6: new patch
---
scripts/qapi-introspect.py | 5 -
scripts/qapi-types.py | 18 ++
scripts/qapi-visit.py | 16 +---
scripts/qapi.py| 11 +++
4 files changed, 30 insertions(+), 20 deletions(-)
diff --git a/scripts/qapi-introspect.py b/scripts/qapi-introspect.py
index 7d39320..f30fac3 100644
--- a/scripts/qapi-introspect.py
+++ b/scripts/qapi-introspect.py
@@ -54,7 +54,6 @@ class QAPISchemaGenIntrospectVisitor(QAPISchemaVisitor):
self._jsons = []
self._used_types = []
self._name_map = {}
-return QAPISchemaType # don't visit types for now
def visit_end(self):
# visit the types that are actually used
@@ -82,6 +81,10 @@ const char %(c_name)s[] = %(c_string)s;
self._used_types = None
self._name_map = None
+def visit_predicate(self, entity):
+# Ignore types on first pass; visit_end() will pick up used types
+return not isinstance(entity, QAPISchemaType)
+
def _name(self, name):
if self._unmask:
return name
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index d405f8d..c5b71b0 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -233,6 +233,9 @@ class QAPISchemaGenTypeVisitor(QAPISchemaVisitor):
self.decl = self._btin + self.decl
self._btin = None
+def visit_predicate(self, entity):
+return not isinstance(entity, QAPISchemaObjectType) or entity.info
+
def _gen_type_cleanup(self, name):
self.decl += gen_type_cleanup_decl(name)
self.defn += gen_type_cleanup(name)
@@ -254,14 +257,13 @@ class QAPISchemaGenTypeVisitor(QAPISchemaVisitor):
self._gen_type_cleanup(name)
def visit_object_type(self, name, info, base, members, variants):
-if info:
-self._fwdecl += gen_fwd_object_or_array(name)
-if variants:
-assert not members # not implemented
-self.decl += gen_union(name, base, variants)
-else:
-self.decl += gen_struct(name, base, members)
-self._gen_type_cleanup(name)
+self._fwdecl += gen_fwd_object_or_array(name)
+if variants:
+assert not members # not implemented
+self.decl += gen_union(name, base, variants)
+else:
+self.decl += gen_struct(name, base, members)
+self._gen_type_cleanup(name)
def visit_alternate_type(self, name, info, variants):
self._fwdecl += gen_fwd_object_or_array(name)
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 4f97781..0f47614 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -333,6 +333,9 @@ class QAPISchemaGenVisitVisitor(QAPISchemaVisitor):
self.decl = self._btin + self.decl
self._btin = None
+def visit_predicate(self, entity):
+return not isinstance(entity, QAPISchemaObjectType) or entity.info
+
def visit_enum_type(self, name, info, values, prefix):
self.decl += gen_visit_decl(name, scalar=True)
self.defn += gen_visit_enum(name)
@@ -349,13 +352,12 @@ class QAPISchemaGenVisitVisitor(QAPISchemaVisitor):
self.defn += defn
def visit_object_type(self, name, info, base, members, variants):
-if info:
-self.decl += gen_visit_decl(name)
-if variants:
-assert not members # not implemented
-self.defn += gen_visit_union(name, base, variants)
-else:
-self.defn += gen_visit_struct(name, base, members)
+self.decl += gen_visit_decl(name)
+if variants:
+assert not members # not implemented
+self.defn += gen_visit_union(name, base, variants)
+else:
+self.defn += gen_visit_struct(name, base, members)
def visit_alternate_type(self, name, info, variants):
self.decl += gen_visit_decl(name)
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 26cff3f..7d359c8 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -811,6 +811,9 @@ class QAPISchemaVisitor(object):
def visit_end(self):
pass
+def visit_predicate(self, entity):
+return True
+
def visit_builtin_
[Qemu-devel] [PATCH v6 00/12] post-introspection cleanups, subset B
Pending prerequisite: Markus' qapi-next branch (which has my subset A patches): git://repo.or.cz/qemu/armbru.git qapi-next http://thread.gmane.org/gmane.comp.emulators.qemu/365827/focus=366351 Also available as a tag at this location: git fetch git://repo.or.cz/qemu/ericb.git qapi-cleanupv6b and I plan to eventually forcefully update my branch with the rest of the v5 series, at: http://repo.or.cz/qemu/ericb.git/shortlog/refs/heads/qapi v6 notes: This is patches 11-16 of my v5 series; it has grown a bit with splitting some patches and adding some others. I suspect that 12/12 on this series will be discarded, but am including it because it was split from v5 content. Not much review comments other than on the original 11/46, but there is enough churn due to rebasing that it's now easier to review this version than plowing through v5. Subset C (and more?) will come later. 001/12:[down] 'qapi: Use predicate callback to determine visit filtering' 002/12:[0043] [FC] 'qapi: Don't use info as witness of implicit object type' 003/12:[down] 'qapi: Lazy creation of array types' 004/12:[down] 'qapi: Create simple union type member earlier' 005/12:[0028] [FC] 'qapi: Track location that created an implicit type' 006/12:[0051] [FC] 'qapi: Track owner of each object member' 007/12:[0031] [FC] 'qapi: Detect collisions in C member names' 008/12:[0035] [FC] 'qapi: Defer duplicate member checks to schema check()' 009/12:[down] 'qapi: Defer duplicate enum value checks to schema check()' 010/12:[down] 'qapi: Correct error for union branch 'kind' clash' 011/12:[0002] [FC] 'qapi: Detect base class loops' 012/12:[down] 'RFC: qapi: Hide _info member' In v5: https://lists.gnu.org/archive/html/qemu-devel/2015-09/msg05410.html I _did_ rearrange patches to try and group related features: 1-2: Groundwork cleanups 3-5: Add more test cases 6-16: Front-end cleanups 17-18: Introspection output cleanups 19-20: 'alternate' type cleanups 21-29: qapi visitor cleanups 30-45: qapi-ify netdev_add 46: add qapi shorthand for flat unions Lots of fixes based on additional testing, and rebased to track other changes that happened in the meantime. The series is huge; I can split off smaller portions as requested. In v4: https://lists.gnu.org/archive/html/qemu-devel/2015-09/msg02580.html add some more clean up patches rebase to Markus' recent work pull in part of Zoltán's work to make netdev_add a flat union, further enhancing it to be introspectible I might be able to rearrange some of these patches, or separate it into smaller independent series, if requested; but I'm posting now to get review started. In v3: https://lists.gnu.org/archive/html/qemu-devel/2015-08/msg02059.html redo cleanup of dealloc of partial struct add patches to make all visit_type_*() avoid leaks on failure add patches to allow boxed command arguments and events In v2: https://lists.gnu.org/archive/html/qemu-devel/2015-08/msg00900.html rebase to Markus' v3 series rework how comments are emitted for fields inherited from base additional patches added for deleting colliding 'void *data' documentation updates to match code changes v1 was here: https://lists.gnu.org/archive/html/qemu-devel/2015-07/msg05266.html https://lists.gnu.org/archive/html/qemu-devel/2015-07/msg05325.html Eric Blake (12): qapi: Use predicate callback to determine visit filtering qapi: Don't use info as witness of implicit object type qapi: Lazy creation of array types qapi: Create simple union type member earlier qapi: Track location that created an implicit type qapi: Track owner of each object member qapi: Detect collisions in C member names qapi: Defer duplicate member checks to schema check() qapi: Defer duplicate enum value checks to schema check() qapi: Correct error for union branch 'kind' clash qapi: Detect base class loops RFC: qapi: Hide _info member qapi-schema.json | 10 + scripts/qapi-introspect.py | 5 +- scripts/qapi-types.py | 19 +- scripts/qapi-visit.py | 17 +- scripts/qapi.py| 300 +++-- tests/Makefile | 5 +- tests/qapi-schema/alternate-clash-members.err | 1 + ...ad-branch.exit => alternate-clash-members.exit} | 0 ...ate-clash.json => alternate-clash-members.json} | 0 ...-bad-branch.out => alternate-clash-members.out} | 0 tests/qapi-schema/alternate-clash-type.err | 1 + ...ernate-clash.exit => alternate-clash-type.exit} | 0 tests/qapi-schema/alternate-clash-type.json| 10 + ...lternate-clash.out => alternate-clash-type.out} | 0 tests/qapi-schema/alternate-clash.err | 1 - tests/qapi-schema/args-name-clash.err | 1 + tests/qapi-schema/args-name-clash.exit | 2 +- tests/qapi-schema/args-name-clash.json | 6 +- tests/qapi-schema/args-name-clash.out
[Qemu-devel] [PATCH v6 04/12] qapi: Create simple union type member earlier
For simple unions, we were creating the implicit 'type' tag
member during the QAPISchemaObjectTypeVariants constructor.
This is different from every other implicit QAPISchemaEntity
object, which get created by QAPISchema methods. Hoist the
creation to the caller, and pass the entity rather than the
string name, so that we have the nice property that no
entities are created as a side effect within a different
entity. A later patch will then have an easier time of
associating location info with each entity creation.
No change to generated code.
Signed-off-by: Eric Blake
---
v6: New patch
---
scripts/qapi.py | 17 -
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 15640b6..255001a 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -1010,18 +1010,16 @@ class QAPISchemaObjectTypeMember(object):
class QAPISchemaObjectTypeVariants(object):
-def __init__(self, tag_name, tag_enum, variants):
+def __init__(self, tag_name, tag_member, variants):
assert tag_name is None or isinstance(tag_name, str)
-assert tag_enum is None or isinstance(tag_enum, str)
+assert (tag_member is None or
+isinstance(tag_member, QAPISchemaObjectTypeMember))
for v in variants:
assert isinstance(v, QAPISchemaObjectTypeVariant)
self.tag_name = tag_name
if tag_name:
-assert not tag_enum
-self.tag_member = None
-else:
-self.tag_member = QAPISchemaObjectTypeMember('type', tag_enum,
- False)
+assert tag_member is None
+self.tag_member = tag_member
self.variants = variants
def check(self, schema, members, seen):
@@ -1226,8 +1224,9 @@ class QAPISchema(object):
return QAPISchemaObjectTypeVariant(case, typ)
def _make_tag_enum(self, type_name, variants):
-return self._make_implicit_enum_type(type_name,
- [v.name for v in variants])
+typ = self._make_implicit_enum_type(type_name,
+[v.name for v in variants])
+return QAPISchemaObjectTypeMember('type', typ, False)
def _def_union_type(self, expr, info):
name = expr['union']
--
2.4.3
[Qemu-devel] [PATCH v6 08/12] qapi: Defer duplicate member checks to schema check()
With the previous commit, we have two different locations for
detecting member name clashes - one at parse time, and another
at QAPISchema*.check() time. Consolidate some of the checks
into a single place, which is also in line with our TODO to
eventually defer all of the parse time semantic checking into
the newer schema code. The check_member_clash() function is
no longer needed.
The wording of several error messages has changed, but in many
cases feels like an improvement rather than a regression. The
recent change to avoid an assertion failure when a flat union
branch name collides with its discriminator name is also
handled nicely by this code; but there is more work needed
before we can detect all collisions in simple union branch
names done by the old code.
No change to generated code.
Signed-off-by: Eric Blake
---
v6: rebase to earlier testsuite improvements, fold in cleanup
of flat-union-clash-type
---
scripts/qapi.py | 46 +--
tests/qapi-schema/flat-union-clash-member.err | 2 +-
tests/qapi-schema/flat-union-clash-type.err | 2 +-
tests/qapi-schema/struct-base-clash-deep.err | 2 +-
tests/qapi-schema/struct-base-clash.err | 2 +-
5 files changed, 19 insertions(+), 35 deletions(-)
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 1acf02b..e58c5a8 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -499,21 +499,6 @@ def check_type(expr_info, source, value, allow_array=False,
'enum'])
-def check_member_clash(expr_info, base_name, data, source=""):
-base = find_struct(base_name)
-assert base
-base_members = base['data']
-for key in data.keys():
-if key.startswith('*'):
-key = key[1:]
-if key in base_members or "*" + key in base_members:
-raise QAPIExprError(expr_info,
-"Member name '%s'%s clashes with base '%s'"
-% (key, source, base_name))
-if base.get('base'):
-check_member_clash(expr_info, base['base'], data, source)
-
-
def check_command(expr, expr_info):
name = expr['command']
@@ -592,15 +577,9 @@ def check_union(expr, expr_info):
for (key, value) in members.items():
check_name(expr_info, "Member of union '%s'" % name, key)
-# Each value must name a known type; furthermore, in flat unions,
-# branches must be a struct with no overlapping member names
+# Each value must name a known type
check_type(expr_info, "Member '%s' of union '%s'" % (key, name),
value, allow_array=not base, allow_metas=allow_metas)
-if base:
-branch_struct = find_struct(value)
-assert branch_struct
-check_member_clash(expr_info, base, branch_struct['data'],
- " of branch '%s'" % key)
# If the discriminator names an enum type, then all members
# of 'data' must also be members of the enum type, which in turn
@@ -611,11 +590,6 @@ def check_union(expr, expr_info):
"Discriminator value '%s' is not found in "
"enum '%s'" %
(key, enum_define["enum_name"]))
-if discriminator in enum_define['enum_values']:
-raise QAPIExprError(expr_info,
-"Discriminator name '%s' collides with "
-"enum value in '%s'" %
-(discriminator, enum_define["enum_name"]))
# Otherwise, check for conflicts in the generated enum
else:
@@ -690,8 +664,6 @@ def check_struct(expr, expr_info):
allow_dict=True, allow_optional=True)
check_type(expr_info, "'base' for struct '%s'" % name, expr.get('base'),
allow_metas=['struct'])
-if expr.get('base'):
-check_member_clash(expr_info, expr['base'], expr['data'])
def check_keys(expr_elem, meta, required, optional=[]):
@@ -1046,16 +1018,28 @@ class QAPISchemaObjectTypeVariants(object):
assert isinstance(self.tag_member.type, QAPISchemaEnumType)
for v in self.variants:
vseen = dict(seen)
-v.check(schema, info, self.tag_member.type, vseen)
+v.check(schema, info, self.tag_member.type, vseen,
+self.tag_name is not None)
class QAPISchemaObjectTypeVariant(QAPISchemaObjectTypeMember):
def __init__(self, name, typ, owner):
QAPISchemaObjectTypeMember.__init__(self, name, typ, False, owner)
-def check(self, schema, info, tag_type, seen):
+def check(self, schema, info, tag_type, seen, flat):
QAPISchemaObjectTypeMember.check(self, schema, info, [], seen)
assert self.name in tag_type.values
+if flat:
+self.type.check(schema)
+assert isinstance(self.type.members, list)
+
Re: [Qemu-devel] [Qemu-ppc] [PATCH] spapr: Add "slb-size" property to CPU device tree nodes
On Thu, Oct 01, 2015 at 03:30:07PM +0200, Thomas Huth wrote: > According to a commit message in the Linux kernel (see here > https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=b60c31d85a2a > for example), the name of the property that carries the information > about the number of SLB entries should be called "slb-size", and > not "ibm,slb-size". The Linux kernel can deal with both names, but > to be on the safe side we should support the official name, too. > > Signed-off-by: Thomas Huth Merged to spapr-next, thanks. -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson pgpnA4w0XHcmD.pgp Description: PGP signature
[Qemu-devel] [PATCH] target-tilegx: Implement v?int_* instructions.
From: Chen Gang
Signed-off-by: Chen Gang
---
target-tilegx/helper.h | 5
target-tilegx/simd_helper.c | 56 +
target-tilegx/translate.c | 14
3 files changed, 75 insertions(+)
diff --git a/target-tilegx/helper.h b/target-tilegx/helper.h
index dc865bb..3f4fa3c 100644
--- a/target-tilegx/helper.h
+++ b/target-tilegx/helper.h
@@ -10,6 +10,11 @@ DEF_HELPER_FLAGS_3(cmula, TCG_CALL_NO_RWG_SE, i64, i64, i64,
i64)
DEF_HELPER_FLAGS_3(cmulaf, TCG_CALL_NO_RWG_SE, i64, i64, i64, i64)
DEF_HELPER_FLAGS_4(cmul2, TCG_CALL_NO_RWG_SE, i64, i64, i64, int, int)
+DEF_HELPER_FLAGS_2(v1int_h, TCG_CALL_NO_RWG_SE, i64, i64, i64)
+DEF_HELPER_FLAGS_2(v1int_l, TCG_CALL_NO_RWG_SE, i64, i64, i64)
+DEF_HELPER_FLAGS_2(v2int_h, TCG_CALL_NO_RWG_SE, i64, i64, i64)
+DEF_HELPER_FLAGS_2(v2int_l, TCG_CALL_NO_RWG_SE, i64, i64, i64)
+
DEF_HELPER_FLAGS_2(v1multu, TCG_CALL_NO_RWG_SE, i64, i64, i64)
DEF_HELPER_FLAGS_2(v1shl, TCG_CALL_NO_RWG_SE, i64, i64, i64)
DEF_HELPER_FLAGS_2(v1shru, TCG_CALL_NO_RWG_SE, i64, i64, i64)
diff --git a/target-tilegx/simd_helper.c b/target-tilegx/simd_helper.c
index 23c20bd..6fa6318 100644
--- a/target-tilegx/simd_helper.c
+++ b/target-tilegx/simd_helper.c
@@ -102,3 +102,59 @@ uint64_t helper_v2shrs(uint64_t a, uint64_t b)
}
return r;
}
+
+uint64_t helper_v1int_h(uint64_t a, uint64_t b)
+{
+uint64_t r = 0, tmp;
+int i;
+
+for (i = 0; i < 32; i += 8) {
+tmp = (uint8_t)(a >> (i + 32));
+r |= tmp << (2 * i + 8);
+tmp = (uint8_t)(b >> (i + 32));
+r |= tmp << 2 * i;
+}
+return r;
+}
+
+uint64_t helper_v1int_l(uint64_t a, uint64_t b)
+{
+uint64_t r = 0, tmp;
+int i;
+
+for (i = 0; i < 32; i += 8) {
+tmp = (uint8_t)(a >> i);
+r |= tmp << (2 * i + 8);
+tmp = (uint8_t)(b >> i);
+r |= tmp << 2 * i;
+}
+return r;
+}
+
+uint64_t helper_v2int_h(uint64_t a, uint64_t b)
+{
+uint64_t r = 0, tmp;
+int i;
+
+for (i = 0; i < 32; i += 16) {
+tmp = (uint16_t)(a >> (i + 32));
+r |= tmp << (2 * i + 16);
+tmp = (uint16_t)(b >> (i + 32));
+r |= tmp << 2 * i;
+}
+return r;
+}
+
+uint64_t helper_v2int_l(uint64_t a, uint64_t b)
+{
+uint64_t r = 0, tmp;
+int i;
+
+for (i = 0; i < 32; i += 16) {
+tmp = (uint16_t)(a >> i);
+r |= tmp << (2 * i + 16);
+tmp = (uint16_t)(b >> i);
+r |= tmp << 2 * i;
+}
+return r;
+}
diff --git a/target-tilegx/translate.c b/target-tilegx/translate.c
index 03c8e76..6853628 100644
--- a/target-tilegx/translate.c
+++ b/target-tilegx/translate.c
@@ -1334,10 +1334,17 @@ static TileExcp gen_rrr_opcode(DisasContext *dc,
unsigned opext,
case OE_RRR(V1DOTPUS, 0, X0):
case OE_RRR(V1DOTPU, 0, X0):
case OE_RRR(V1DOTP, 0, X0):
+return TILEGX_EXCP_OPCODE_UNIMPLEMENTED;
case OE_RRR(V1INT_H, 0, X0):
case OE_RRR(V1INT_H, 0, X1):
+gen_helper_v1int_h(TDEST, tsrca, tsrcb);
+mnemonic = "v1int_h";
+break;
case OE_RRR(V1INT_L, 0, X0):
case OE_RRR(V1INT_L, 0, X1):
+gen_helper_v1int_l(TDEST, tsrca, tsrcb);
+mnemonic = "v1int_l";
+break;
case OE_RRR(V1MAXU, 0, X0):
case OE_RRR(V1MAXU, 0, X1):
case OE_RRR(V1MINU, 0, X0):
@@ -1403,10 +1410,17 @@ static TileExcp gen_rrr_opcode(DisasContext *dc,
unsigned opext,
case OE_RRR(V2CMPNE, 0, X1):
case OE_RRR(V2DOTPA, 0, X0):
case OE_RRR(V2DOTP, 0, X0):
+return TILEGX_EXCP_OPCODE_UNIMPLEMENTED;
case OE_RRR(V2INT_H, 0, X0):
case OE_RRR(V2INT_H, 0, X1):
+gen_helper_v2int_h(TDEST, tsrca, tsrcb);
+mnemonic = "v2int_h";
+break;
case OE_RRR(V2INT_L, 0, X0):
case OE_RRR(V2INT_L, 0, X1):
+gen_helper_v2int_l(TDEST, tsrca, tsrcb);
+mnemonic = "v2int_l";
+break;
case OE_RRR(V2MAXS, 0, X0):
case OE_RRR(V2MAXS, 0, X1):
case OE_RRR(V2MINS, 0, X0):
--
1.9.3
Re: [Qemu-devel] [PATCH v2] exec: factor out duplicate mmap code
On 10/01/2015 10:58 PM, Michael S. Tsirkin wrote:
Anonymous and file-backed RAM allocation are now almost exactly the same.
Reduce code duplication by moving RAM mmap code out of oslib-posix.c and
exec.c.
Reported-by: Marc-André Lureau
Signed-off-by: Michael S. Tsirkin
Reviewed-by: Paolo Bonzini
Acked-by: Paolo Bonzini
---
Changes from v1: add shared flag to get MAP_SHARED mappings
(for vhost-user), only set MAP_ANONYMOUS for anonymous RAM.
include/qemu/mmap-alloc.h | 10 +++
exec.c| 47 +++
util/mmap-alloc.c | 71 +++
util/oslib-posix.c| 28 +++
util/Makefile.objs| 2 +-
5 files changed, 96 insertions(+), 62 deletions(-)
create mode 100644 include/qemu/mmap-alloc.h
create mode 100644 util/mmap-alloc.c
diff --git a/include/qemu/mmap-alloc.h b/include/qemu/mmap-alloc.h
new file mode 100644
index 000..56388e6
--- /dev/null
+++ b/include/qemu/mmap-alloc.h
@@ -0,0 +1,10 @@
+#ifndef QEMU_MMAP_ALLOC
+#define QEMU_MMAP_ALLOC
+
+#include "qemu-common.h"
+
+void *qemu_ram_mmap(int fd, size_t size, size_t align, bool shared);
+
+void qemu_ram_munmap(void *ptr, size_t size);
+
+#endif
diff --git a/exec.c b/exec.c
index 7d90a52..4505dc7 100644
--- a/exec.c
+++ b/exec.c
@@ -55,6 +55,9 @@
#include "exec/ram_addr.h"
#include "qemu/range.h"
+#ifndef _WIN32
+#include "qemu/mmap-alloc.h"
+#endif
//#define DEBUG_SUBPAGE
@@ -84,9 +87,9 @@ static MemoryRegion io_mem_unassigned;
*/
#define RAM_RESIZEABLE (1 << 2)
-/* An extra page is mapped on top of this RAM.
+/* RAM is backed by an mmapped file.
*/
-#define RAM_EXTRA (1 << 3)
+#define RAM_FILE (1 << 3)
#endif
struct CPUTailQ cpus = QTAILQ_HEAD_INITIALIZER(cpus);
@@ -1188,13 +1191,10 @@ static void *file_ram_alloc(RAMBlock *block,
char *filename;
char *sanitized_name;
char *c;
-void *ptr;
-void *area = NULL;
+void *area;
int fd;
uint64_t hpagesize;
-uint64_t total;
Error *local_err = NULL;
-size_t offset;
hpagesize = gethugepagesize(path, &local_err);
if (local_err) {
@@ -1238,7 +1238,6 @@ static void *file_ram_alloc(RAMBlock *block,
g_free(filename);
memory = ROUND_UP(memory, hpagesize);
-total = memory + hpagesize;
/*
* ftruncate is not supported by hugetlbfs in older
@@ -1250,40 +1249,14 @@ static void *file_ram_alloc(RAMBlock *block,
perror("ftruncate");
}
-ptr = mmap(0, total, PROT_NONE, MAP_PRIVATE | MAP_ANONYMOUS,
--1, 0);
-if (ptr == MAP_FAILED) {
-error_setg_errno(errp, errno,
- "unable to allocate memory range for hugepages");
-close(fd);
-goto error;
-}
-
-offset = QEMU_ALIGN_UP((uintptr_t)ptr, hpagesize) - (uintptr_t)ptr;
-
-area = mmap(ptr + offset, memory, PROT_READ | PROT_WRITE,
-(block->flags & RAM_SHARED ? MAP_SHARED : MAP_PRIVATE) |
-MAP_FIXED,
-fd, 0);
+area = qemu_ram_mmap(fd, memory, hpagesize, block->flags & RAM_SHARED);
if (area == MAP_FAILED) {
error_setg_errno(errp, errno,
"unable to map backing store for hugepages");
-munmap(ptr, total);
close(fd);
goto error;
}
-if (offset > 0) {
-munmap(ptr, offset);
-}
-ptr += offset;
-total -= offset;
-
-if (total > memory + getpagesize()) {
-munmap(ptr + memory + getpagesize(),
- total - memory - getpagesize());
-}
-
if (mem_prealloc) {
os_mem_prealloc(fd, area, memory);
}
@@ -1601,7 +1574,7 @@ ram_addr_t qemu_ram_alloc_from_file(ram_addr_t size,
MemoryRegion *mr,
new_block->used_length = size;
new_block->max_length = size;
new_block->flags = share ? RAM_SHARED : 0;
-new_block->flags |= RAM_EXTRA;
+new_block->flags |= RAM_FILE;
new_block->host = file_ram_alloc(new_block, size,
mem_path, errp);
if (!new_block->host) {
@@ -1703,8 +1676,8 @@ static void reclaim_ramblock(RAMBlock *block)
xen_invalidate_map_cache_entry(block->host);
#ifndef _WIN32
} else if (block->fd >= 0) {
-if (block->flags & RAM_EXTRA) {
-munmap(block->host, block->max_length + getpagesize());
+if (block->flags & RAM_FILE) {
+qemu_ram_munmap(block->host, block->max_length);
} else {
munmap(block->host, block->max_length);
}
diff --git a/util/mmap-alloc.c b/util/mmap-alloc.c
new file mode 100644
index 000..e82cc94
--- /dev/null
+++ b/util/mmap-alloc.c
@@ -0,0 +1,71 @@
+/*
+ * Support for RAM backed by mmaped host memory.
+ *
+ * Copyright (c) 2015 Red Hat, Inc.
+ *
+ * Authors:
+ * Michael S. Tsirkin
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * later. See the
Re: [Qemu-devel] [PATCH v3] target-tilegx: Support iret instruction and related special registers
On 10/02/2015 11:31 AM, Chris Metcalf wrote: It disables interrupts from being delivered. This means asynchronous interrupts get deferred until ICS is set back to zero, and synchronous interrupts (page fault, etc) cause a double-fault instead. ICS is automatically set on entry to interrupt handlers, so the handler has time to acquire any information about the interrupt from SPRs, and it is expected that ICS is cleared as soon as possible. ICS can also be used before returning from interrupts if you need to do something like adjust the interrupt mask prior to returning. Which is all very well and good for supervisor mode... but what's it good for in user mode? I was about to quote you from 2012 (https://lkml.org/lkml/2012/3/30/994): In general we want to avoid ever touching memory while within an interrupt critical section, since the page fault path goes through a different path from the hypervisor when in an interrupt critical section, and we carefully decided with tilegx that we didn't need to support this path in the kernel. Which implies that tilegx userland does nothing at all with ICS, and is in fact unsupported? r~
Re: [Qemu-devel] [PATCH v1 1/1] target-microblaze: Set the PC in reset instead of realize
On Sat, Jul 25, 2015 at 12:45:08PM +1000, Edgar E. Iglesias wrote: > On Thu, Jul 23, 2015 at 08:13:56AM -0700, Alistair Francis wrote: > > Set the Microblaze CPU PC in the reset instead of setting it > > in the realize. This is required as the PC is zeroed in the > > reset function and causes problems in some situations. > > > > Signed-off-by: Alistair Francis > > Looks good: > > Reviewed-by: Edgar E. Iglesias > > I'll test and add this to my queue. Applied, thanks! (Sorry for the delay...) > > > > --- > > > > target-microblaze/cpu.c |4 ++-- > > 1 files changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/target-microblaze/cpu.c b/target-microblaze/cpu.c > > index 9ac509a..cbd84a2 100644 > > --- a/target-microblaze/cpu.c > > +++ b/target-microblaze/cpu.c > > @@ -107,6 +107,8 @@ static void mb_cpu_reset(CPUState *s) > > /* Disable stack protector. */ > > env->shr = ~0; > > > > +env->sregs[SR_PC] = cpu->cfg.base_vectors; > > + > > #if defined(CONFIG_USER_ONLY) > > /* start in user mode with interrupts enabled. */ > > env->sregs[SR_MSR] = MSR_EE | MSR_IE | MSR_VM | MSR_UM; > > @@ -183,8 +185,6 @@ static void mb_cpu_realizefn(DeviceState *dev, Error > > **errp) > > env->pvr.regs[10] = 0x0c00; /* Default to spartan 3a dsp family. > > */ > > env->pvr.regs[11] = PVR11_USE_MMU | (16 << 17); > > > > -env->sregs[SR_PC] = cpu->cfg.base_vectors; > > - > > mcc->parent_realize(dev, errp); > > } > > > > -- > > 1.7.1 > >
Re: [Qemu-devel] [PATCH v3] target-tilegx: Support iret instruction and related special registers
OK, thanks. I shall try to send patch v4 for it within 2 days.
On 10/2/15 09:31, Chris Metcalf wrote:
> On 10/1/2015 8:36 PM, Richard Henderson wrote:
>> On 10/01/2015 10:37 PM, gang.chen.5...@gmail.com wrote:
>>> {
>>>mtspr INTERRUPT_CRITICAL_SECTION, r3
>>>shli r2, r2, SPR_EX_CONTEXT_0_1__ICS_SHIFT
>>> }
>>>
>>> {
>>>mtspr EX_CONTEXT_0_0, lr
>>>ori r2, r2, RETURN_PL
>>> }
>>>
>>> {
>>>or r0, r1, r0
>>>mtspr EX_CONTEXT_0_1, r2
>>> }
>>>
>>> iret
>>>
>>> jrp lr
>>>
>>> Until now, EX_CONTEXT_0_0 and EX_CONTEXT_0_1 are only used in mtspr, so
>>> just skip them, at present. "jrp lr" in __longjmp is for historical
>>> reasons, and might get removed in the future.
>>
>> So, really, iret is supposed to branch to EX_CONTEXT_0_0, and (presumably)
>> validate the privilege level in EX_CONTEXT_0_1 continues to be user-mode.
>
> Yes, I gave the same feedback earlier today. EX_CONTEXT_0_1 should be either
> 0 or 1 to set INTERRUPT_CRITICAL_SECTION appropriately, and raise GPV for any
> other value. (Obviously it's more complex if you're really emulating system
> software, but for now that's out of scope, I think.)
>
>>
>>> +case OE_RR_X1(IRET):
>>> +if (srca) {
>>> +return TILEGX_EXCP_OPCODE_UNIMPLEMENTED;
>>> +}
>>> +srca = TILEGX_R_LR;
>>> +mnemonic = "iret";
>>> +goto do_jr;
>>
>> which means this is wrong, but just happens to work for __longjmp.
>>
>> It appears that the entire point of this iret path is to atomically branch
>> and set INTERRUPT_CRITICAL_SECTION at the same time. So, this isn't
>> complete.
>>
>> What INTERRUPT_CRITICAL_SECTION is supposed to *do* at user mode, I don't
>> know.
>
> It disables interrupts from being delivered. This means asynchronous
> interrupts get deferred until ICS is set back to zero, and synchronous
> interrupts (page fault, etc) cause a double-fault instead. ICS is
> automatically set on entry to interrupt handlers, so the handler has time to
> acquire any information about the interrupt from SPRs, and it is expected
> that ICS is cleared as soon as possible. ICS can also be used before
> returning from interrupts if you need to do something like adjust the
> interrupt mask prior to returning.
>
--
Chen Gang (陈刚)
Open, share, and attitude like air, water, and life which God blessed
Re: [Qemu-devel] [PATCH v3] target-tilegx: Support iret instruction and related special registers
On 10/1/2015 8:36 PM, Richard Henderson wrote:
On 10/01/2015 10:37 PM, gang.chen.5...@gmail.com wrote:
{
mtspr INTERRUPT_CRITICAL_SECTION, r3
shli r2, r2, SPR_EX_CONTEXT_0_1__ICS_SHIFT
}
{
mtspr EX_CONTEXT_0_0, lr
ori r2, r2, RETURN_PL
}
{
or r0, r1, r0
mtspr EX_CONTEXT_0_1, r2
}
iret
jrp lr
Until now, EX_CONTEXT_0_0 and EX_CONTEXT_0_1 are only used in mtspr, so
just skip them, at present. "jrp lr" in __longjmp is for historical
reasons, and might get removed in the future.
So, really, iret is supposed to branch to EX_CONTEXT_0_0, and (presumably)
validate the privilege level in EX_CONTEXT_0_1 continues to be user-mode.
Yes, I gave the same feedback earlier today. EX_CONTEXT_0_1 should be either 0
or 1 to set INTERRUPT_CRITICAL_SECTION appropriately, and raise GPV for any
other value. (Obviously it's more complex if you're really emulating system
software, but for now that's out of scope, I think.)
+case OE_RR_X1(IRET):
+if (srca) {
+return TILEGX_EXCP_OPCODE_UNIMPLEMENTED;
+}
+srca = TILEGX_R_LR;
+mnemonic = "iret";
+goto do_jr;
which means this is wrong, but just happens to work for __longjmp.
It appears that the entire point of this iret path is to atomically branch and
set INTERRUPT_CRITICAL_SECTION at the same time. So, this isn't complete.
What INTERRUPT_CRITICAL_SECTION is supposed to *do* at user mode, I don't know.
It disables interrupts from being delivered. This means asynchronous
interrupts get deferred until ICS is set back to zero, and synchronous
interrupts (page fault, etc) cause a double-fault instead. ICS is
automatically set on entry to interrupt handlers, so the handler has time to
acquire any information about the interrupt from SPRs, and it is expected that
ICS is cleared as soon as possible. ICS can also be used before returning from
interrupts if you need to do something like adjust the interrupt mask prior to
returning.
--
Chris Metcalf, EZChip Semiconductor
http://www.ezchip.com
[Qemu-devel] [PATCH] target-tilegx: Implement v2sh* instructions
From: Chen Gang It is just according to v1sh* instructions implementation. Signed-off-by: Chen Gang --- target-tilegx/translate.c | 20 +++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/target-tilegx/translate.c b/target-tilegx/translate.c index f711c18..03c8e76 100644 --- a/target-tilegx/translate.c +++ b/target-tilegx/translate.c @@ -1769,11 +1769,29 @@ static TileExcp gen_rri_opcode(DisasContext *dc, unsigned opext, break; case OE_SH(V2SHLI, X0): case OE_SH(V2SHLI, X1): +tdest = dest_gr(dc, dest); +i2 = imm & 15; +i3 = 0x >> i2; +tcg_gen_andi_tl(tdest, tsrca, V2_IMM(i3)); +tcg_gen_shli_tl(tdest, tdest, i2); +mnemonic = "v2shli"; +break; case OE_SH(V2SHRSI, X0): case OE_SH(V2SHRSI, X1): +t0 = tcg_const_tl(imm & 15); +gen_helper_v2shrs(TDEST, tsrca, t0); +tcg_temp_free(t0); +mnemonic = "v2shrsi"; +break; case OE_SH(V2SHRUI, X0): case OE_SH(V2SHRUI, X1): -return TILEGX_EXCP_OPCODE_UNIMPLEMENTED; +tdest = dest_gr(dc, dest); +i2 = imm & 15; +i3 = (0x << i2) & 0x; +tcg_gen_andi_tl(tdest, tsrca, V2_IMM(i3)); +tcg_gen_shri_tl(tdest, tdest, i2); +mnemonic = "v2shrui"; +break; case OE(ADDLI_OPCODE_X0, 0, X0): case OE(ADDLI_OPCODE_X1, 0, X1): -- 1.9.3
Re: [Qemu-devel] [PATCH v3] target-tilegx: Support iret instruction and related special registers
On 10/2/15 08:36, Richard Henderson wrote:
> On 10/01/2015 10:37 PM, gang.chen.5...@gmail.com wrote:
>> {
>> mtspr INTERRUPT_CRITICAL_SECTION, r3
>> shli r2, r2, SPR_EX_CONTEXT_0_1__ICS_SHIFT
>> }
>>
>> {
>> mtspr EX_CONTEXT_0_0, lr
>> ori r2, r2, RETURN_PL
>> }
>>
>> {
>> or r0, r1, r0
>> mtspr EX_CONTEXT_0_1, r2
>> }
>>
>> iret
>>
>> jrp lr
>>
>> Until now, EX_CONTEXT_0_0 and EX_CONTEXT_0_1 are only used in mtspr, so
>> just skip them, at present. "jrp lr" in __longjmp is for historical
>> reasons, and might get removed in the future.
>
> So, really, iret is supposed to branch to EX_CONTEXT_0_0, and (presumably)
> validate the privilege level in EX_CONTEXT_0_1 continues to be user-mode.
>
Oh, really.
>> + case OE_RR_X1(IRET):
>> + if (srca) {
>> + return TILEGX_EXCP_OPCODE_UNIMPLEMENTED;
>> + }
>> + srca = TILEGX_R_LR;
>> + mnemonic = "iret";
>> + goto do_jr;
>
> which means this is wrong, but just happens to work for __longjmp.
>
> It appears that the entire point of this iret path is to atomically branch
> and set INTERRUPT_CRITICAL_SECTION at the same time. So, this isn't complete.
>
OK, thanks.
> What INTERRUPT_CRITICAL_SECTION is supposed to *do* at user mode, I don't
> know.
>
Welcome any other members' ideas, suggestions and completions.
Thanks.
--
Chen Gang (陈刚)
Open, share, and attitude like air, water, and life which God blessed
[Qemu-devel] [PATCH v2 6/8] target-arm: Add S2 translation support for S1 PTW
From: "Edgar E. Iglesias"
Add support for applying S2 translation to S1 page-table walks.
Signed-off-by: Edgar E. Iglesias
---
target-arm/helper.c| 50 --
target-arm/op_helper.c | 4 ++--
2 files changed, 50 insertions(+), 4 deletions(-)
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 7f66e3c..2d1532a 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -21,6 +21,12 @@ static bool get_phys_addr(CPUARMState *env, target_ulong
address,
target_ulong *page_size, uint32_t *fsr,
ARMMMUFaultInfo *fi);
+static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address,
+ int access_type, ARMMMUIdx mmu_idx,
+ hwaddr *phys_ptr, MemTxAttrs *txattrs, int
*prot,
+ target_ulong *page_size_ptr, uint32_t *fsr,
+ ARMMMUFaultInfo *fi);
+
/* Definitions for the PMCCNTR and PMCR registers */
#define PMCRD 0x8
#define PMCRC 0x4
@@ -6143,6 +6149,32 @@ static bool get_level1_table_address(CPUARMState *env,
ARMMMUIdx mmu_idx,
return true;
}
+/* Translate a S1 pagetable walk through S2 if needed. */
+static hwaddr S1_ptw_translate(CPUARMState *env, ARMMMUIdx mmu_idx,
+ hwaddr addr, MemTxAttrs txattrs,
+ uint32_t *fsr,
+ ARMMMUFaultInfo *fi)
+{
+if ((mmu_idx == ARMMMUIdx_S1NSE0 || mmu_idx == ARMMMUIdx_S1NSE1) &&
+!regime_translation_disabled(env, ARMMMUIdx_S2NS)) {
+target_ulong s2size;
+hwaddr s2pa;
+int s2prot;
+int ret;
+
+ret = get_phys_addr_lpae(env, addr, 0, ARMMMUIdx_S2NS, &s2pa,
+ &txattrs, &s2prot, &s2size, fsr, fi);
+if (ret) {
+fi->s2addr = addr;
+fi->stage2 = true;
+fi->s1ptw = true;
+return ~0;
+}
+addr = s2pa;
+}
+return addr;
+}
+
/* All loads done in the course of a page table walk go through here.
* TODO: rather than ignoring errors from physical memory reads (which
* are external aborts in ARM terminology) we should propagate this
@@ -6158,11 +6190,19 @@ static uint32_t arm_ldl_ptw(CPUState *cs, hwaddr addr,
bool is_secure)
return address_space_ldl(cs->as, addr, attrs, NULL);
}
-static uint64_t arm_ldq_ptw(CPUState *cs, hwaddr addr, bool is_secure)
+static uint64_t arm_ldq_ptw(CPUState *cs, hwaddr addr, bool is_secure,
+ARMMMUIdx mmu_idx, uint32_t *fsr,
+ARMMMUFaultInfo *fi)
{
+ARMCPU *cpu = ARM_CPU(cs);
+CPUARMState *env = &cpu->env;
MemTxAttrs attrs = {};
attrs.secure = is_secure;
+addr = S1_ptw_translate(env, mmu_idx, addr, attrs, fsr, fi);
+if (fi->s1ptw) {
+return 0;
+}
return address_space_ldq(cs->as, addr, attrs, NULL);
}
@@ -6625,7 +6665,11 @@ static bool get_phys_addr_lpae(CPUARMState *env,
target_ulong address,
descaddr |= (address >> (granule_sz * (4 - level))) & descmask;
descaddr &= ~7ULL;
nstable = extract32(tableattrs, 4, 1);
-descriptor = arm_ldq_ptw(cs, descaddr, !nstable);
+descriptor = arm_ldq_ptw(cs, descaddr, !nstable, mmu_idx, fsr, fi);
+if (fi->s1ptw) {
+goto do_fault;
+}
+
if (!(descriptor & 1) ||
(!(descriptor & 2) && (level == 3))) {
/* Invalid, or the Reserved level 3 encoding */
@@ -6709,6 +6753,8 @@ static bool get_phys_addr_lpae(CPUARMState *env,
target_ulong address,
do_fault:
/* Long-descriptor format IFSR/DFSR value */
*fsr = (1 << 9) | (fault_type << 2) | level;
+/* Tag the error as S2 for failed S1 PTW at S2 or ordinary S2. */
+fi->stage2 = fi->s1ptw || (mmu_idx == ARMMMUIdx_S2NS);
return true;
}
diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
index 7ff3c61..d4715f4 100644
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -104,10 +104,10 @@ void tlb_fill(CPUState *cs, target_ulong addr, int
is_write, int mmu_idx,
* information; this is always true for exceptions reported to EL1.
*/
if (is_write == 2) {
-syn = syn_insn_abort(same_el, 0, 0, syn);
+syn = syn_insn_abort(same_el, 0, fi.s1ptw, syn);
exc = EXCP_PREFETCH_ABORT;
} else {
-syn = syn_data_abort(same_el, 0, 0, 0, is_write == 1, syn);
+syn = syn_data_abort(same_el, 0, 0, fi.s1ptw, is_write == 1, syn);
if (is_write == 1 && arm_feature(env, ARM_FEATURE_V6)) {
fsr |= (1 << 11);
}
--
1.9.1
[Qemu-devel] [PATCH v2 5/8] target-arm: Add ARMMMUFaultInfo
From: "Edgar E. Iglesias"
Introduce ARMMMUFaultInfo to propagate MMU Fault information
across the MMU translation code path. This is in preparation for
adding State-2 translation.
No functional changes.
Signed-off-by: Edgar E. Iglesias
---
target-arm/helper.c| 22 ++
target-arm/internals.h | 11 ++-
target-arm/op_helper.c | 3 ++-
3 files changed, 26 insertions(+), 10 deletions(-)
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 51b0e61..7f66e3c 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -18,7 +18,8 @@
static bool get_phys_addr(CPUARMState *env, target_ulong address,
int access_type, ARMMMUIdx mmu_idx,
hwaddr *phys_ptr, MemTxAttrs *attrs, int *prot,
- target_ulong *page_size, uint32_t *fsr);
+ target_ulong *page_size, uint32_t *fsr,
+ ARMMMUFaultInfo *fi);
/* Definitions for the PMCCNTR and PMCR registers */
#define PMCRD 0x8
@@ -1774,9 +1775,10 @@ static uint64_t do_ats_write(CPUARMState *env, uint64_t
value,
bool ret;
uint64_t par64;
MemTxAttrs attrs = {};
+ARMMMUFaultInfo fi = {};
ret = get_phys_addr(env, value, access_type, mmu_idx,
-&phys_addr, &attrs, &prot, &page_size, &fsr);
+&phys_addr, &attrs, &prot, &page_size, &fsr, &fi);
if (extended_addresses_enabled(env)) {
/* fsr is a DFSR/IFSR value for the long descriptor
* translation table format, but with WnR always clear.
@@ -6431,7 +6433,8 @@ typedef enum {
static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address,
int access_type, ARMMMUIdx mmu_idx,
hwaddr *phys_ptr, MemTxAttrs *txattrs, int
*prot,
- target_ulong *page_size_ptr, uint32_t *fsr)
+ target_ulong *page_size_ptr, uint32_t *fsr,
+ ARMMMUFaultInfo *fi)
{
CPUState *cs = CPU(arm_env_get_cpu(env));
/* Read an LPAE long-descriptor translation table. */
@@ -6971,7 +6974,8 @@ static bool get_phys_addr_pmsav5(CPUARMState *env,
uint32_t address,
static bool get_phys_addr(CPUARMState *env, target_ulong address,
int access_type, ARMMMUIdx mmu_idx,
hwaddr *phys_ptr, MemTxAttrs *attrs, int *prot,
- target_ulong *page_size, uint32_t *fsr)
+ target_ulong *page_size, uint32_t *fsr,
+ ARMMMUFaultInfo *fi)
{
if (mmu_idx == ARMMMUIdx_S12NSE0 || mmu_idx == ARMMMUIdx_S12NSE1) {
/* TODO: when we support EL2 we should here call ourselves recursively
@@ -7030,7 +7034,7 @@ static bool get_phys_addr(CPUARMState *env, target_ulong
address,
if (regime_using_lpae_format(env, mmu_idx)) {
return get_phys_addr_lpae(env, address, access_type, mmu_idx, phys_ptr,
- attrs, prot, page_size, fsr);
+ attrs, prot, page_size, fsr, fi);
} else if (regime_sctlr(env, mmu_idx) & SCTLR_XP) {
return get_phys_addr_v6(env, address, access_type, mmu_idx, phys_ptr,
attrs, prot, page_size, fsr);
@@ -7045,7 +7049,8 @@ static bool get_phys_addr(CPUARMState *env, target_ulong
address,
* fsr with ARM DFSR/IFSR fault register format value on failure.
*/
bool arm_tlb_fill(CPUState *cs, vaddr address,
- int access_type, int mmu_idx, uint32_t *fsr)
+ int access_type, int mmu_idx, uint32_t *fsr,
+ ARMMMUFaultInfo *fi)
{
ARMCPU *cpu = ARM_CPU(cs);
CPUARMState *env = &cpu->env;
@@ -7056,7 +7061,7 @@ bool arm_tlb_fill(CPUState *cs, vaddr address,
MemTxAttrs attrs = {};
ret = get_phys_addr(env, address, access_type, mmu_idx, &phys_addr,
-&attrs, &prot, &page_size, fsr);
+&attrs, &prot, &page_size, fsr, fi);
if (!ret) {
/* Map a single [sub]page. */
phys_addr &= TARGET_PAGE_MASK;
@@ -7079,9 +7084,10 @@ hwaddr arm_cpu_get_phys_page_debug(CPUState *cs, vaddr
addr)
bool ret;
uint32_t fsr;
MemTxAttrs attrs = {};
+ARMMMUFaultInfo fi = {};
ret = get_phys_addr(env, addr, 0, cpu_mmu_index(env, false), &phys_addr,
-&attrs, &prot, &page_size, &fsr);
+&attrs, &prot, &page_size, &fsr, &fi);
if (ret) {
return -1;
diff --git a/target-arm/internals.h b/target-arm/internals.h
index 36a56aa..6157a41 100644
--- a/target-arm/internals.h
+++ b/target-arm/internals.h
@@ -389,8 +389,17 @@ bool arm_is_psci_call(ARMCPU *cpu, int excp_type);
void arm_handle_psci_call(ARMCPU *cpu);
#endif
+typedef struct ARMMMUFaultInfo ARMMMUFaultInfo;
+
+struct ARMMMUFaultInfo {
+target_ulong s2addr;
[Qemu-devel] [PATCH v2 8/8] target-arm: Add support for S1 + S2 MMU translations
From: "Edgar E. Iglesias"
Signed-off-by: Edgar E. Iglesias
---
target-arm/helper.c | 44 +---
1 file changed, 37 insertions(+), 7 deletions(-)
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 2d1532a..113d6f3 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -7024,14 +7024,44 @@ static bool get_phys_addr(CPUARMState *env,
target_ulong address,
ARMMMUFaultInfo *fi)
{
if (mmu_idx == ARMMMUIdx_S12NSE0 || mmu_idx == ARMMMUIdx_S12NSE1) {
-/* TODO: when we support EL2 we should here call ourselves recursively
- * to do the stage 1 and then stage 2 translations. The arm_ld*_ptw
- * functions will also need changing to perform ARMMMUIdx_S2NS loads
- * rather than direct physical memory loads when appropriate.
- * For non-EL2 CPUs a stage1+stage2 translation is just stage 1.
+/* Call ourselves recursively to do the stage 1 and then stage 2
+ * translations.
*/
-assert(!arm_feature(env, ARM_FEATURE_EL2));
-mmu_idx += ARMMMUIdx_S1NSE0;
+if (arm_feature(env, ARM_FEATURE_EL2)) {
+hwaddr ipa;
+int s2_prot;
+int ret;
+
+ret = get_phys_addr(env, address, access_type,
+mmu_idx + ARMMMUIdx_S1NSE0, &ipa, attrs,
+prot, page_size, fsr, fi);
+
+/* If S1 fails or S2 is disabled, return early. */
+if (ret || regime_translation_disabled(env, ARMMMUIdx_S2NS)) {
+if (ret && fi->stage2) {
+/* This is a S2 error while doing S1 PTW. */
+env->cp15.hpfar_el2 = extract64(fi->s2addr, 12, 47) << 4;
+}
+*phys_ptr = ipa;
+return ret;
+}
+
+/* S1 is done. Now do S2 translation. */
+ret = get_phys_addr_lpae(env, ipa, access_type, ARMMMUIdx_S2NS,
+ phys_ptr, attrs, &s2_prot,
+ page_size, fsr, fi);
+if (ret) {
+env->cp15.hpfar_el2 = extract64(ipa, 12, 47) << 4;
+}
+/* Combine the S1 and S2 perms. */
+*prot &= s2_prot;
+return ret;
+} else {
+/*
+ * For non-EL2 CPUs a stage1+stage2 translation is just stage 1.
+ */
+mmu_idx += ARMMMUIdx_S1NSE0;
+}
}
/* The page table entries may downgrade secure to non-secure, but
--
1.9.1
[Qemu-devel] [PATCH v2 4/8] target-arm: Avoid inline for get_phys_addr
From: "Edgar E. Iglesias"
Avoid inline for get_phys_addr() to prepare for future recursive use.
Reviewed-by: Peter Maydell
Signed-off-by: Edgar E. Iglesias
---
target-arm/helper.c | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 2701788..51b0e61 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -15,10 +15,10 @@
#define ARM_CPU_FREQ 10 /* FIXME: 1 GHz, should be configurable */
#ifndef CONFIG_USER_ONLY
-static inline bool get_phys_addr(CPUARMState *env, target_ulong address,
- int access_type, ARMMMUIdx mmu_idx,
- hwaddr *phys_ptr, MemTxAttrs *attrs, int
*prot,
- target_ulong *page_size, uint32_t *fsr);
+static bool get_phys_addr(CPUARMState *env, target_ulong address,
+ int access_type, ARMMMUIdx mmu_idx,
+ hwaddr *phys_ptr, MemTxAttrs *attrs, int *prot,
+ target_ulong *page_size, uint32_t *fsr);
/* Definitions for the PMCCNTR and PMCR registers */
#define PMCRD 0x8
@@ -6968,10 +6968,10 @@ static bool get_phys_addr_pmsav5(CPUARMState *env,
uint32_t address,
* @page_size: set to the size of the page containing phys_ptr
* @fsr: set to the DFSR/IFSR value on failure
*/
-static inline bool get_phys_addr(CPUARMState *env, target_ulong address,
- int access_type, ARMMMUIdx mmu_idx,
- hwaddr *phys_ptr, MemTxAttrs *attrs, int
*prot,
- target_ulong *page_size, uint32_t *fsr)
+static bool get_phys_addr(CPUARMState *env, target_ulong address,
+ int access_type, ARMMMUIdx mmu_idx,
+ hwaddr *phys_ptr, MemTxAttrs *attrs, int *prot,
+ target_ulong *page_size, uint32_t *fsr)
{
if (mmu_idx == ARMMMUIdx_S12NSE0 || mmu_idx == ARMMMUIdx_S12NSE1) {
/* TODO: when we support EL2 we should here call ourselves recursively
--
1.9.1
[Qemu-devel] [PATCH v2 0/8] arm: Steps towards EL2 support round 5
From: "Edgar E. Iglesias" Hi, Another round of patches towards EL2 support. This one adds partial support for 2-stage MMU for AArch64. I've marked it RFC because I expect a few iterations. Once we can settle on the approach I'll add the AArch32 support (changes for arm_ldl_ptw etc). I've probably missed alot of details aswell. Some of the details of error reporting are intentionally missing, I was thinking to add those incrementally as they get quite involved (e.g the register target and memory access size). Some of the patches at the start of the series might be good already, please pick them up if you agree Peter! Comments welcome! Best regards, Edgar v1 -> v2: * Fix HPFAR_EL2 access checks * Prettify computation of starting level for S2 PTW * Improve description of ap argument to get_S2prot * Fix EXEC protection in get_S2prot * Improve comments on S2 PTW attribute extraction Edgar E. Iglesias (8): target-arm: Add HPFAR_EL2 target-arm: Add computation of starting level for S2 PTW target-arm: Add support for S2 page-table protection bits target-arm: Avoid inline for get_phys_addr target-arm: Add ARMMMUFaultInfo target-arm: Add S2 translation support for S1 PTW target-arm: Route S2 MMU faults to EL2 target-arm: Add support for S1 + S2 MMU translations target-arm/cpu.h | 1 + target-arm/helper.c| 216 - target-arm/internals.h | 11 ++- target-arm/op_helper.c | 17 ++-- 4 files changed, 200 insertions(+), 45 deletions(-) -- 1.9.1
[Qemu-devel] [PATCH v2 3/8] target-arm: Add support for S2 page-table protection bits
From: "Edgar E. Iglesias"
Signed-off-by: Edgar E. Iglesias
---
target-arm/helper.c | 41 +
1 file changed, 37 insertions(+), 4 deletions(-)
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 16a0701..2701788 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -6015,6 +6015,28 @@ simple_ap_to_rw_prot(CPUARMState *env, ARMMMUIdx
mmu_idx, int ap)
return simple_ap_to_rw_prot_is_user(ap, regime_is_user(env, mmu_idx));
}
+/* Translate S2 section/page access permissions to protection flags
+ *
+ * @env: CPUARMState
+ * @s2ap:The 2-bit stage2 access permissions (S2AP)
+ * @xn: XN (execute-never) bit
+ */
+static int get_S2prot(CPUARMState *env, int s2ap, int xn)
+{
+int prot = 0;
+
+if (s2ap & 1) {
+prot |= PAGE_READ;
+}
+if (s2ap & 2) {
+prot |= PAGE_WRITE;
+}
+if (!xn) {
+prot |= PAGE_EXEC;
+}
+return prot;
+}
+
/* Translate section/page access permissions to protection flags
*
* @env: CPUARMState
@@ -6624,9 +6646,15 @@ static bool get_phys_addr_lpae(CPUARMState *env,
target_ulong address,
*/
page_size = (1ULL << ((granule_sz * (4 - level)) + 3));
descaddr |= (address & (page_size - 1));
-/* Extract attributes from the descriptor and merge with table attrs */
+/* Extract attributes from the descriptor */
attrs = extract64(descriptor, 2, 10)
| (extract64(descriptor, 52, 12) << 10);
+
+if (mmu_idx == ARMMMUIdx_S2NS) {
+/* Stage 2 table descriptors do not include any attribute fields */
+break;
+}
+/* Merge in attributes from table descriptors */
attrs |= extract32(tableattrs, 0, 2) << 11; /* XN, PXN */
attrs |= extract32(tableattrs, 3, 1) << 5; /* APTable[1] => AP[2] */
/* The sense of AP[1] vs APTable[0] is reversed, as APTable[0] == 1
@@ -6648,11 +6676,16 @@ static bool get_phys_addr_lpae(CPUARMState *env,
target_ulong address,
}
ap = extract32(attrs, 4, 2);
-ns = extract32(attrs, 3, 1);
xn = extract32(attrs, 12, 1);
-pxn = extract32(attrs, 11, 1);
-*prot = get_S1prot(env, mmu_idx, va_size == 64, ap, ns, xn, pxn);
+if (mmu_idx == ARMMMUIdx_S2NS) {
+ns = true;
+*prot = get_S2prot(env, ap, xn);
+} else {
+ns = extract32(attrs, 3, 1);
+pxn = extract32(attrs, 11, 1);
+*prot = get_S1prot(env, mmu_idx, va_size == 64, ap, ns, xn, pxn);
+}
fault_type = permission_fault;
if (!(*prot & (1 << access_type))) {
--
1.9.1
[Qemu-devel] [PATCH v2 7/8] target-arm: Route S2 MMU faults to EL2
From: "Edgar E. Iglesias"
Signed-off-by: Edgar E. Iglesias
---
target-arm/op_helper.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
index d4715f4..2ccd1c9 100644
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -90,13 +90,19 @@ void tlb_fill(CPUState *cs, target_ulong addr, int
is_write, int mmu_idx,
ARMCPU *cpu = ARM_CPU(cs);
CPUARMState *env = &cpu->env;
uint32_t syn, exc;
-bool same_el = (arm_current_el(env) != 0);
+unsigned int target_el;
+bool same_el;
if (retaddr) {
/* now we have a real cpu fault */
cpu_restore_state(cs, retaddr);
}
+target_el = exception_target_el(env);
+if (fi.stage2) {
+target_el = 2;
+}
+same_el = arm_current_el(env) == target_el;
/* AArch64 syndrome does not have an LPAE bit */
syn = fsr & ~(1 << 9);
@@ -116,7 +122,7 @@ void tlb_fill(CPUState *cs, target_ulong addr, int
is_write, int mmu_idx,
env->exception.vaddress = addr;
env->exception.fsr = fsr;
-raise_exception(env, exc, syn, exception_target_el(env));
+raise_exception(env, exc, syn, target_el);
}
}
#endif
--
1.9.1
[Qemu-devel] [PATCH v2 1/8] target-arm: Add HPFAR_EL2
From: "Edgar E. Iglesias"
Signed-off-by: Edgar E. Iglesias
---
target-arm/cpu.h| 1 +
target-arm/helper.c | 12
2 files changed, 13 insertions(+)
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index cc1578c..895f2c2 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -278,6 +278,7 @@ typedef struct CPUARMState {
};
uint64_t far_el[4];
};
+uint64_t hpfar_el2;
union { /* Translation result. */
struct {
uint64_t _unused_par_0;
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 8367997..5a5e5f0 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -3223,6 +3223,10 @@ static const ARMCPRegInfo el3_no_el2_cp_reginfo[] = {
{ .name = "CNTHP_CTL_EL2", .state = ARM_CP_STATE_BOTH,
.opc0 = 3, .opc1 = 4, .crn = 14, .crm = 2, .opc2 = 1,
.access = PL2_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
+{ .name = "HPFAR_EL2", .state = ARM_CP_STATE_BOTH,
+ .opc0 = 3, .opc1 = 4, .crn = 6, .crm = 0, .opc2 = 4,
+ .access = PL2_RW, .accessfn = access_el3_aa32ns_aa64any,
+ .type = ARM_CP_CONST, .resetvalue = 0 },
REGINFO_SENTINEL
};
@@ -3444,6 +3448,14 @@ static const ARMCPRegInfo el2_cp_reginfo[] = {
.resetvalue = 0,
.writefn = gt_hyp_ctl_write, .raw_writefn = raw_write },
#endif
+{ .name = "HPFAR", .state = ARM_CP_STATE_AA32,
+ .cp = 15, .opc1 = 4, .crn = 6, .crm = 0, .opc2 = 4,
+ .access = PL2_RW, .accessfn = access_el3_aa32ns,
+ .fieldoffset = offsetof(CPUARMState, cp15.hpfar_el2) },
+{ .name = "HPFAR_EL2", .state = ARM_CP_STATE_AA64,
+ .opc0 = 3, .opc1 = 4, .crn = 6, .crm = 0, .opc2 = 4,
+ .access = PL2_RW,
+ .fieldoffset = offsetof(CPUARMState, cp15.hpfar_el2) },
REGINFO_SENTINEL
};
--
1.9.1
[Qemu-devel] [PATCH v2 2/8] target-arm: Add computation of starting level for S2 PTW
From: "Edgar E. Iglesias"
The starting level for S2 pagetable walks is computed
differently from the S1 starting level. Implement the S2
variant.
Signed-off-by: Edgar E. Iglesias
---
target-arm/helper.c | 35 +++
1 file changed, 23 insertions(+), 12 deletions(-)
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 5a5e5f0..16a0701 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -6549,18 +6549,29 @@ static bool get_phys_addr_lpae(CPUARMState *env,
target_ulong address,
goto do_fault;
}
-/* The starting level depends on the virtual address size (which can be
- * up to 48 bits) and the translation granule size. It indicates the number
- * of strides (granule_sz bits at a time) needed to consume the bits
- * of the input address. In the pseudocode this is:
- * level = 4 - RoundUp((inputsize - grainsize) / stride)
- * where their 'inputsize' is our 'va_size - tsz', 'grainsize' is
- * our 'granule_sz + 3' and 'stride' is our 'granule_sz'.
- * Applying the usual "rounded up m/n is (m+n-1)/n" and simplifying:
- * = 4 - (va_size - tsz - granule_sz - 3 + granule_sz - 1) / granule_sz
- * = 4 - (va_size - tsz - 4) / granule_sz;
- */
-level = 4 - (va_size - tsz - 4) / granule_sz;
+if (mmu_idx != ARMMMUIdx_S2NS) {
+/* The starting level depends on the virtual address size (which can
+ * be up to 48 bits) and the translation granule size. It indicates
+ * the number of strides (granule_sz bits at a time) needed to
+ * consume the bits of the input address. In the pseudocode this is:
+ * level = 4 - RoundUp((inputsize - grainsize) / stride)
+ * where their 'inputsize' is our 'va_size - tsz', 'grainsize' is
+ * our 'granule_sz + 3' and 'stride' is our 'granule_sz'.
+ * Applying the usual "rounded up m/n is (m+n-1)/n" and simplifying:
+ * = 4 - (va_size - tsz - granule_sz - 3 + granule_sz - 1) / granule_sz
+ * = 4 - (va_size - tsz - 4) / granule_sz;
+ */
+level = 4 - (va_size - tsz - 4) / granule_sz;
+} else {
+unsigned int startlevel = extract32(tcr->raw_tcr, 6, 2);
+if (granule_sz == 9) {
+/* 4K pages */
+level = 2 - startlevel;
+} else {
+/* 16K or 64K pages */
+level = 3 - startlevel;
+}
+}
/* Clear the vaddr bits which aren't part of the within-region address,
* so that we don't have to special case things when calculating the
--
1.9.1
Re: [Qemu-devel] [PATCH v3] target-tilegx: Support iret instruction and related special registers
On 10/01/2015 10:37 PM, gang.chen.5...@gmail.com wrote:
{
mtspr INTERRUPT_CRITICAL_SECTION, r3
shli r2, r2, SPR_EX_CONTEXT_0_1__ICS_SHIFT
}
{
mtspr EX_CONTEXT_0_0, lr
ori r2, r2, RETURN_PL
}
{
or r0, r1, r0
mtspr EX_CONTEXT_0_1, r2
}
iret
jrp lr
Until now, EX_CONTEXT_0_0 and EX_CONTEXT_0_1 are only used in mtspr, so
just skip them, at present. "jrp lr" in __longjmp is for historical
reasons, and might get removed in the future.
So, really, iret is supposed to branch to EX_CONTEXT_0_0, and (presumably)
validate the privilege level in EX_CONTEXT_0_1 continues to be user-mode.
+case OE_RR_X1(IRET):
+if (srca) {
+return TILEGX_EXCP_OPCODE_UNIMPLEMENTED;
+}
+srca = TILEGX_R_LR;
+mnemonic = "iret";
+goto do_jr;
which means this is wrong, but just happens to work for __longjmp.
It appears that the entire point of this iret path is to atomically branch and
set INTERRUPT_CRITICAL_SECTION at the same time. So, this isn't complete.
What INTERRUPT_CRITICAL_SECTION is supposed to *do* at user mode, I don't know.
r~
Re: [Qemu-devel] [PATCH] disas/cris: Fix typo in comment
On Fri, Sep 25, 2015 at 10:45:53PM +0200, Stefan Weil wrote:
> Signed-off-by: Stefan Weil
Reviewed-by: Edgar E. Iglesias
Applied, thanks!
> ---
> disas/cris.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/disas/cris.c b/disas/cris.c
> index 1b76a09..4482a41 100644
> --- a/disas/cris.c
> +++ b/disas/cris.c
> @@ -2492,7 +2492,7 @@ print_with_operands (const struct cris_opcode *opcodep,
> = spec_reg_info ((insn >> 12) & 15, disdata->distype);
>
> if (sregp->name == NULL)
> - /* Should have been caught as a non-match eariler. */
> + /* Should have been caught as a non-match earlier. */
> *tp++ = '?';
> else
> {
> --
> 2.1.4
>
>
Re: [Qemu-devel] [PATCH v2 4/4] tests: add a local test for guest agent
Quoting Marc-André Lureau (2015-10-01 17:24:53)
>
>
> - Original Message -
> > Quoting marcandre.lur...@redhat.com (2015-09-11 13:53:41)
> > > From: Marc-André Lureau
> > >
> > > Add some local guest agent tests (as it is better than nothing) only
> > > when CONFIG_LINUX.
> > >
> > > They can be run inside or outside a VM, when run inside a VM, they will
> > > do a bit more side effects, such as freezing/thawing the FS or changing
> > > the time.
> > >
> > > A better test would involve a managed VM (or container), but it might be
> > > better to leave that off to autotest/avocado.
> > >
> > > Signed-off-by: Marc-André Lureau
> >
> > Sorry for the delay in getting to this. Have some review comments but
> > will make sure to stay on top of any follow-ups as I'm trying to get
> > this into my next pull.
> >
> > > ---
> > > tests/Makefile | 3 +
> > > tests/test-qga.c | 791
> > > +++
> > > 2 files changed, 794 insertions(+)
> > > create mode 100644 tests/test-qga.c
> > >
> > > diff --git a/tests/Makefile b/tests/Makefile
> > > index 34c6136..d5837a4 100644
> > > --- a/tests/Makefile
> > > +++ b/tests/Makefile
> > > @@ -76,6 +76,7 @@ check-unit-y += tests/test-write-threshold$(EXESUF)
> > > gcov-files-test-write-threshold-y = block/write-threshold.c
> > > check-unit-$(CONFIG_GNUTLS_HASH) += tests/test-crypto-hash$(EXESUF)
> > > check-unit-y += tests/test-crypto-cipher$(EXESUF)
> > > +check-unit-$(CONFIG_LINUX) += tests/test-qga$(EXESUF)
> > >
> > > check-block-$(CONFIG_POSIX) += tests/qemu-iotests-quick.sh
> > >
> > > @@ -432,6 +433,8 @@ endif
> > > qtest-obj-y = tests/libqtest.o libqemuutil.a libqemustub.a
> > > $(check-qtest-y): $(qtest-obj-y)
> > >
> > > +tests/test-qga: tests/test-qga.o $(qtest-obj-y)
> > > +
> > > .PHONY: check-help
> > > check-help:
> > > @echo "Regression testing targets:"
> > > diff --git a/tests/test-qga.c b/tests/test-qga.c
> > > new file mode 100644
> > > index 000..cfee134
> > > --- /dev/null
> > > +++ b/tests/test-qga.c
> > > @@ -0,0 +1,791 @@
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +
> > > +#include "libqtest.h"
> > > +#include "config-host.h"
> > > +
> > > +typedef struct {
> > > +char *test_dir;
> > > +GMainLoop *loop;
> > > +int fd;
> > > +GPid pid;
> > > +} TestFixture;
> > > +
> > > +static int connect_qga(char *path)
> > > +{
> > > +int s, ret, len, i = 0;
> > > +struct sockaddr_un remote;
> > > +
> > > +s = socket(AF_UNIX, SOCK_STREAM, 0);
> > > +g_assert(s != -1);
> > > +
> > > +remote.sun_family = AF_UNIX;
> > > +do {
> > > +strcpy(remote.sun_path, path);
> > > +len = strlen(remote.sun_path) + sizeof(remote.sun_family);
> > > +ret = connect(s, (struct sockaddr *)&remote, len);
> > > +if (ret == -1) {
> > > +g_usleep(G_USEC_PER_SEC);
> > > +}
> > > +if (i++ == 5) {
> > > +return -1;
> > > +}
> > > +} while (ret == -1);
> > > +
> > > +return s;
> > > +}
> > > +
> > > +static void qga_watch(GPid pid, gint status, gpointer user_data)
> > > +{
> > > +TestFixture *fixture = user_data;
> > > +
> > > +g_assert_cmpint(status, ==, 0);
> > > +g_main_loop_quit(fixture->loop);
> > > +}
> > > +
> > > +static void
> > > +fixture_setup(TestFixture *fixture, gconstpointer data)
> > > +{
> > > +const gchar *extra_arg = data;
> > > +GError *error = NULL;
> > > +gchar *cwd, *path, *cmd, **argv = NULL;
> > > +
> > > +fixture->loop = g_main_loop_new(NULL, FALSE);
> > > +
> > > +fixture->test_dir = g_strdup("/tmp/qgatest.XX");
> > > +g_assert_nonnull(g_mkdtemp(fixture->test_dir));
> > > +
> > > +path = g_build_filename(fixture->test_dir, "sock", NULL);
> > > +cwd = g_get_current_dir();
> > > +cmd = g_strdup_printf("%s%cqemu-ga -m unix-listen -t %s -p %s %s %s",
> > > + cwd, G_DIR_SEPARATOR,
> > > + fixture->test_dir, path,
> > > + getenv("QTEST_LOG") ? "-v" : "",
> > > + extra_arg ?: "");
> > > +g_shell_parse_argv(cmd, NULL, &argv, &error);
> > > +g_assert_no_error(error);
> > > +
> > > +g_spawn_async(fixture->test_dir, argv, NULL,
> > > + G_SPAWN_SEARCH_PATH|G_SPAWN_DO_NOT_REAP_CHILD,
> > > + NULL, NULL, &fixture->pid, &error);
> > > +g_assert_no_error(error);
> > > +
> > > +g_child_watch_add(fixture->pid, qga_watch, fixture);
> > > +
> > > +fixture->fd = connect_qga(path);
> > > +
> > > +g_strfreev(argv);
> > > +g_free(cmd);
> > > +g_free(cwd);
> > > +g_free(path);
> > > +}
> > > +
> > > +static void
> > > +fixture_tear_down(TestFixture *fixture, gconstpointer data)
> > > +{
> > > +gchar *tm
Re: [Qemu-devel] [PATCH 1/5] qga: drop guest_file_init helper and replace it with static initializers
Quoting Denis V. Lunev (2015-10-01 02:37:59)
> This just makes code shorter and better.
Can't complain with that.
>
> Signed-off-by: Denis V. Lunev
> Signed-off-by: Yuri Pudgorodskiy
> CC: Michael Roth
Reviewed-by: Michael Roth
> ---
> qga/commands-posix.c | 10 +++---
> qga/commands-win32.c | 10 +++---
> 2 files changed, 6 insertions(+), 14 deletions(-)
>
> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> index b03c316..8989912 100644
> --- a/qga/commands-posix.c
> +++ b/qga/commands-posix.c
> @@ -223,7 +223,9 @@ typedef struct GuestFileHandle {
>
> static struct {
> QTAILQ_HEAD(, GuestFileHandle) filehandles;
> -} guest_file_state;
> +} guest_file_state = {
> +.filehandles = QTAILQ_HEAD_INITIALIZER(guest_file_state.filehandles),
> +};
>
> static int64_t guest_file_handle_add(FILE *fh, Error **errp)
> {
> @@ -586,11 +588,6 @@ void qmp_guest_file_flush(int64_t handle, Error **errp)
> }
> }
>
> -static void guest_file_init(void)
> -{
> -QTAILQ_INIT(&guest_file_state.filehandles);
> -}
> -
> /* linux-specific implementations. avoid this if at all possible. */
> #if defined(__linux__)
>
> @@ -2486,5 +2483,4 @@ void ga_command_state_init(GAState *s, GACommandState
> *cs)
> #if defined(CONFIG_FSFREEZE)
> ga_command_state_add(cs, NULL, guest_fsfreeze_cleanup);
> #endif
> -ga_command_state_add(cs, guest_file_init, NULL);
> }
> diff --git a/qga/commands-win32.c b/qga/commands-win32.c
> index 41bdd3f..3374678 100644
> --- a/qga/commands-win32.c
> +++ b/qga/commands-win32.c
> @@ -55,7 +55,9 @@ typedef struct GuestFileHandle {
>
> static struct {
> QTAILQ_HEAD(, GuestFileHandle) filehandles;
> -} guest_file_state;
> +} guest_file_state = {
> +.filehandles = QTAILQ_HEAD_INITIALIZER(guest_file_state.filehandles),
> +};
>
>
> typedef struct OpenFlags {
> @@ -390,11 +392,6 @@ void qmp_guest_file_flush(int64_t handle, Error **errp)
> }
> }
>
> -static void guest_file_init(void)
> -{
> -QTAILQ_INIT(&guest_file_state.filehandles);
> -}
> -
> #ifdef CONFIG_QGA_NTDDSCSI
>
> static STORAGE_BUS_TYPE win2qemu[] = {
> @@ -1330,5 +1327,4 @@ void ga_command_state_init(GAState *s, GACommandState
> *cs)
> if (!vss_initialized()) {
> ga_command_state_add(cs, NULL, guest_fsfreeze_cleanup);
> }
> -ga_command_state_add(cs, guest_file_init, NULL);
> }
> --
> 2.1.4
>
Re: [Qemu-devel] [PATCH 3/5] qga: guest exec functionality
Quoting Denis V. Lunev (2015-10-01 02:38:01)
> From: Yuri Pudgorodskiy
>
> Guest-exec rewriten in platform-independant style with glib spawn.
>
> Child process is spawn asynchroneously and exit status can later
> be picked up by guest-exec-status command.
>
> stdin/stdout/stderr of the child now is redirected to /dev/null
> Later we will add ability to specify stdin in guest-exec command
> and to get collected stdout/stderr with guest-exec-status.
>
> Signed-off-by: Yuri Pudgorodskiy
> Signed-off-by: Denis V. Lunev
> CC: Michael Roth
> ---
> qga/commands.c | 168
> +++
> qga/qapi-schema.json | 57 +
> 2 files changed, 225 insertions(+)
>
> diff --git a/qga/commands.c b/qga/commands.c
> index 7834967..6efd6aa 100644
> --- a/qga/commands.c
> +++ b/qga/commands.c
> @@ -70,3 +70,171 @@ struct GuestAgentInfo *qmp_guest_info(Error **errp)
> qmp_for_each_command(qmp_command_info, info);
> return info;
> }
> +
> +struct GuestExecInfo {
> +GPid pid;
> +gint status;
> +bool finished;
> +QTAILQ_ENTRY(GuestExecInfo) next;
> +};
> +typedef struct GuestExecInfo GuestExecInfo;
> +
> +static struct {
> +QTAILQ_HEAD(, GuestExecInfo) processes;
> +} guest_exec_state = {
> +.processes = QTAILQ_HEAD_INITIALIZER(guest_exec_state.processes),
> +};
> +
> +static GuestExecInfo *guest_exec_info_add(GPid pid)
> +{
> +GuestExecInfo *gei;
> +
> +gei = g_malloc0(sizeof(*gei));
gei = g_new0(GuestExecInfo, 1);
and same for all other g_malloc*(sizeof(...)) callers.
(Markus has been trying to get all prior g_malloc users converted over)
> +gei->pid = pid;
> +QTAILQ_INSERT_TAIL(&guest_exec_state.processes, gei, next);
> +
> +return gei;
> +}
> +
> +static GuestExecInfo *guest_exec_info_find(GPid pid)
> +{
> +GuestExecInfo *gei;
> +
> +QTAILQ_FOREACH(gei, &guest_exec_state.processes, next) {
> +if (gei->pid == pid) {
> +return gei;
> +}
> +}
> +
> +return NULL;
> +}
> +
> +GuestExecStatus *qmp_guest_exec_status(int64_t pid, Error **err)
> +{
> +GuestExecInfo *gei;
> +GuestExecStatus *ges;
> +
> +slog("guest-exec-status called, pid: %u", (uint32_t)pid);
> +
> +gei = guest_exec_info_find((GPid)pid);
> +if (gei == NULL) {
> +error_setg(err, QERR_INVALID_PARAMETER, "pid");
> +return NULL;
> +}
> +
> +ges = g_malloc0(sizeof(GuestExecStatus));
> +ges->exit = ges->signal = -1;
> +
> +if (gei->finished) {
> +/* glib has no platform independent way to parse exit status */
> +#ifdef G_OS_WIN32
> +if ((uint32_t)gei->status < 0xC000U) {
Can you add a comment with a link to the documentation you referenced
for this? I assume this is actually for exceptions rather than normal
signals?
> +ges->exit = gei->status;
> +} else {
> +ges->signal = gei->status;
> +}
> +#else
> +if (WIFEXITED(gei->status)) {
> +ges->exit = WEXITSTATUS(gei->status);
> +} else if (WIFSIGNALED(gei->status)) {
> +ges->signal = WTERMSIG(gei->status);
> +}
> +#endif
> +QTAILQ_REMOVE(&guest_exec_state.processes, gei, next);
> +g_free(gei);
> +}
> +
> +return ges;
> +}
> +
> +/* Get environment variables or arguments array for execve(). */
> +static char **guest_exec_get_args(const strList *entry, bool log)
> +{
> +const strList *it;
> +int count = 1, i = 0; /* reserve for NULL terminator */
> +char **args;
> +char *str; /* for logging array of arguments */
> +size_t str_size = 1;
> +
> +for (it = entry; it != NULL; it = it->next) {
> +count++;
> +str_size += 1 + strlen(it->value);
> +}
> +
> +str = g_malloc(str_size);
> +*str = 0;
> +args = g_malloc(count * sizeof(char *));
> +for (it = entry; it != NULL; it = it->next) {
> +args[i++] = it->value;
> +pstrcat(str, str_size, it->value);
> +if (it->next) {
> +pstrcat(str, str_size, " ");
> +}
> +}
> +args[i] = NULL;
> +
> +if (log) {
> +slog("guest-exec called: \"%s\"", str);
> +}
> +g_free(str);
> +
> +return args;
> +}
> +
> +static void guest_exec_child_watch(GPid pid, gint status, gpointer data)
> +{
> +GuestExecInfo *gei = (GuestExecInfo *)data;
> +
> +g_debug("guest_exec_child_watch called, pid: %u, status: %u",
> +(uint32_t)pid, (uint32_t)status);
> +
> +gei->status = status;
> +gei->finished = true;
> +
> +g_spawn_close_pid(pid);
> +}
> +
> +GuestExec *qmp_guest_exec(const char *path,
> + bool has_arg, strList *arg,
> + bool has_env, strList *env,
> + bool has_inp_data, const char *inp_data,
> + bool has_capture_output, bool capture_output,
> + Error **err)
> +{
> +GPid pid;
> +GuestExe
Re: [Qemu-devel] [PATCH 2/5] qga: handle G_IO_STATUS_AGAIN in ga_channel_write_all()
Quoting Denis V. Lunev (2015-10-01 02:38:00)
> From: Yuri Pudgorodskiy
>
> glib may return G_IO_STATUS_AGAIN which is actually not an error.
> Also fixed a bug when on incomplete write buf pointer was not adjusted.
>
> Signed-off-by: Yuri Pudgorodskiy
> Signed-off-by: Denis V. Lunev
> CC: Michael Roth
Reviewed-by: Michael Roth
> ---
> qga/channel-posix.c | 23 +++
> 1 file changed, 11 insertions(+), 12 deletions(-)
>
> diff --git a/qga/channel-posix.c b/qga/channel-posix.c
> index 8aad4fe..7be92cc 100644
> --- a/qga/channel-posix.c
> +++ b/qga/channel-posix.c
> @@ -217,25 +217,24 @@ GIOStatus ga_channel_write_all(GAChannel *c, const
> gchar *buf, gsize size)
> GIOStatus status = G_IO_STATUS_NORMAL;
>
> while (size) {
> +g_debug("sending data, count: %d", (int)size);
> status = g_io_channel_write_chars(c->client_channel, buf, size,
>&written, &err);
> -g_debug("sending data, count: %d", (int)size);
> -if (err != NULL) {
> +if (status == G_IO_STATUS_NORMAL) {
> +size -= written;
> +buf += written;
> +} else if (status != G_IO_STATUS_AGAIN) {
> g_warning("error writing to channel: %s", err->message);
> -return G_IO_STATUS_ERROR;
> -}
> -if (status != G_IO_STATUS_NORMAL) {
> -break;
> +return status;
> }
> -size -= written;
> }
>
> -if (status == G_IO_STATUS_NORMAL) {
> +do {
> status = g_io_channel_flush(c->client_channel, &err);
> -if (err != NULL) {
> -g_warning("error flushing channel: %s", err->message);
> -return G_IO_STATUS_ERROR;
> -}
> +} while (status == G_IO_STATUS_AGAIN);
> +
> +if (status != G_IO_STATUS_NORMAL) {
> +g_warning("error flushing channel: %s", err->message);
> }
>
> return status;
> --
> 2.1.4
>
Re: [Qemu-devel] [PATCH 4/5] qga: handle possible SIGPIPE in guest-file-write
Quoting Denis V. Lunev (2015-10-01 02:38:02)
> qemu-ga should not exit on guest-file-write to pipe without read end
> but proper error code should be returned. The behavior of the
> spawned process should be default thus SIGPIPE processing should be
> reset to default after fork() but before exec().
>
> Signed-off-by: Denis V. Lunev
> Signed-off-by: Yuri Pudgorodskiy
> CC: Michael Roth
Reviewed-by: Michael Roth
> ---
> qga/commands.c | 18 +-
> qga/main.c | 6 ++
> 2 files changed, 23 insertions(+), 1 deletion(-)
>
> diff --git a/qga/commands.c b/qga/commands.c
> index 6efd6aa..199c7c3 100644
> --- a/qga/commands.c
> +++ b/qga/commands.c
> @@ -194,6 +194,22 @@ static void guest_exec_child_watch(GPid pid, gint
> status, gpointer data)
> g_spawn_close_pid(pid);
> }
>
> +/** Reset ignored signals back to default. */
> +static void guest_exec_task_setup(gpointer data)
> +{
> +#if !defined(G_OS_WIN32)
> +struct sigaction sigact;
> +
> +memset(&sigact, 0, sizeof(struct sigaction));
> +sigact.sa_handler = SIG_DFL;
> +
> +if (sigaction(SIGPIPE, &sigact, NULL) != 0) {
> +slog("sigaction() failed to reset child process's SIGPIPE: %s",
> + strerror(errno));
> +}
> +#endif
> +}
> +
> GuestExec *qmp_guest_exec(const char *path,
> bool has_arg, strList *arg,
> bool has_env, strList *env,
> @@ -219,7 +235,7 @@ GuestExec *qmp_guest_exec(const char *path,
> G_SPAWN_SEARCH_PATH |
> G_SPAWN_DO_NOT_REAP_CHILD |
> G_SPAWN_STDOUT_TO_DEV_NULL | G_SPAWN_STDERR_TO_DEV_NULL,
> -NULL, NULL, &pid, NULL, NULL, NULL, &gerr);
> +guest_exec_task_setup, NULL, &pid, NULL, NULL, NULL, &gerr);
> if (!ret) {
> error_setg(err, QERR_QGA_COMMAND_FAILED, gerr->message);
> g_error_free(gerr);
> diff --git a/qga/main.c b/qga/main.c
> index d8e063a..07e3c1c 100644
> --- a/qga/main.c
> +++ b/qga/main.c
> @@ -161,6 +161,12 @@ static gboolean register_signal_handlers(void)
> g_error("error configuring signal handler: %s", strerror(errno));
> }
>
> +sigact.sa_handler = SIG_IGN;
> +if (sigaction(SIGPIPE, &sigact, NULL) != 0) {
> +g_error("error configuring SIGPIPE signal handler: %s",
> +strerror(errno));
> +}
> +
> return true;
> }
>
> --
> 2.1.4
>
Re: [Qemu-devel] [PATCH v2 4/4] tests: add a local test for guest agent
- Original Message -
> Quoting marcandre.lur...@redhat.com (2015-09-11 13:53:41)
> > From: Marc-André Lureau
> >
> > Add some local guest agent tests (as it is better than nothing) only
> > when CONFIG_LINUX.
> >
> > They can be run inside or outside a VM, when run inside a VM, they will
> > do a bit more side effects, such as freezing/thawing the FS or changing
> > the time.
> >
> > A better test would involve a managed VM (or container), but it might be
> > better to leave that off to autotest/avocado.
> >
> > Signed-off-by: Marc-André Lureau
>
> Sorry for the delay in getting to this. Have some review comments but
> will make sure to stay on top of any follow-ups as I'm trying to get
> this into my next pull.
>
> > ---
> > tests/Makefile | 3 +
> > tests/test-qga.c | 791
> > +++
> > 2 files changed, 794 insertions(+)
> > create mode 100644 tests/test-qga.c
> >
> > diff --git a/tests/Makefile b/tests/Makefile
> > index 34c6136..d5837a4 100644
> > --- a/tests/Makefile
> > +++ b/tests/Makefile
> > @@ -76,6 +76,7 @@ check-unit-y += tests/test-write-threshold$(EXESUF)
> > gcov-files-test-write-threshold-y = block/write-threshold.c
> > check-unit-$(CONFIG_GNUTLS_HASH) += tests/test-crypto-hash$(EXESUF)
> > check-unit-y += tests/test-crypto-cipher$(EXESUF)
> > +check-unit-$(CONFIG_LINUX) += tests/test-qga$(EXESUF)
> >
> > check-block-$(CONFIG_POSIX) += tests/qemu-iotests-quick.sh
> >
> > @@ -432,6 +433,8 @@ endif
> > qtest-obj-y = tests/libqtest.o libqemuutil.a libqemustub.a
> > $(check-qtest-y): $(qtest-obj-y)
> >
> > +tests/test-qga: tests/test-qga.o $(qtest-obj-y)
> > +
> > .PHONY: check-help
> > check-help:
> > @echo "Regression testing targets:"
> > diff --git a/tests/test-qga.c b/tests/test-qga.c
> > new file mode 100644
> > index 000..cfee134
> > --- /dev/null
> > +++ b/tests/test-qga.c
> > @@ -0,0 +1,791 @@
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +#include "libqtest.h"
> > +#include "config-host.h"
> > +
> > +typedef struct {
> > +char *test_dir;
> > +GMainLoop *loop;
> > +int fd;
> > +GPid pid;
> > +} TestFixture;
> > +
> > +static int connect_qga(char *path)
> > +{
> > +int s, ret, len, i = 0;
> > +struct sockaddr_un remote;
> > +
> > +s = socket(AF_UNIX, SOCK_STREAM, 0);
> > +g_assert(s != -1);
> > +
> > +remote.sun_family = AF_UNIX;
> > +do {
> > +strcpy(remote.sun_path, path);
> > +len = strlen(remote.sun_path) + sizeof(remote.sun_family);
> > +ret = connect(s, (struct sockaddr *)&remote, len);
> > +if (ret == -1) {
> > +g_usleep(G_USEC_PER_SEC);
> > +}
> > +if (i++ == 5) {
> > +return -1;
> > +}
> > +} while (ret == -1);
> > +
> > +return s;
> > +}
> > +
> > +static void qga_watch(GPid pid, gint status, gpointer user_data)
> > +{
> > +TestFixture *fixture = user_data;
> > +
> > +g_assert_cmpint(status, ==, 0);
> > +g_main_loop_quit(fixture->loop);
> > +}
> > +
> > +static void
> > +fixture_setup(TestFixture *fixture, gconstpointer data)
> > +{
> > +const gchar *extra_arg = data;
> > +GError *error = NULL;
> > +gchar *cwd, *path, *cmd, **argv = NULL;
> > +
> > +fixture->loop = g_main_loop_new(NULL, FALSE);
> > +
> > +fixture->test_dir = g_strdup("/tmp/qgatest.XX");
> > +g_assert_nonnull(g_mkdtemp(fixture->test_dir));
> > +
> > +path = g_build_filename(fixture->test_dir, "sock", NULL);
> > +cwd = g_get_current_dir();
> > +cmd = g_strdup_printf("%s%cqemu-ga -m unix-listen -t %s -p %s %s %s",
> > + cwd, G_DIR_SEPARATOR,
> > + fixture->test_dir, path,
> > + getenv("QTEST_LOG") ? "-v" : "",
> > + extra_arg ?: "");
> > +g_shell_parse_argv(cmd, NULL, &argv, &error);
> > +g_assert_no_error(error);
> > +
> > +g_spawn_async(fixture->test_dir, argv, NULL,
> > + G_SPAWN_SEARCH_PATH|G_SPAWN_DO_NOT_REAP_CHILD,
> > + NULL, NULL, &fixture->pid, &error);
> > +g_assert_no_error(error);
> > +
> > +g_child_watch_add(fixture->pid, qga_watch, fixture);
> > +
> > +fixture->fd = connect_qga(path);
> > +
> > +g_strfreev(argv);
> > +g_free(cmd);
> > +g_free(cwd);
> > +g_free(path);
> > +}
> > +
> > +static void
> > +fixture_tear_down(TestFixture *fixture, gconstpointer data)
> > +{
> > +gchar *tmp;
> > +
> > +kill(fixture->pid, SIGTERM);
> > +
> > +g_main_loop_run(fixture->loop);
> > +g_main_loop_unref(fixture->loop);
> > +
> > +g_spawn_close_pid(fixture->pid);
> > +
> > +tmp = g_build_filename(fixture->test_dir, "foo", NULL);
> > +g_unlink(tmp);
> > +g_free(tmp);
> > +
> > +tmp = g_build_filename(fixture->t
Re: [Qemu-devel] [PATCH v3 0/6] qemu: guest agent: implement guest-exec command
Quoting Vasiliy Tolstov (2015-10-01 03:17:34) > 2015-10-01 11:00 GMT+03:00 Denis V. Lunev : > > Subject: [PATCH 0/5] simplified QEMU guest exec > > Date: Thu, 1 Oct 2015 10:37:58 +0300 > > Message-ID: <1443685083-6242-1-git-send-email-...@openvz.org> > > > hm... i don't see it and google and > http://lists.nongnu.org/archive/html/qemu-devel/2015-10/threads.html > says nothing =( Not sure how regularly that archive is updated, but it's here now if you haven't gotten it yet: http://thread.gmane.org/gmane.comp.emulators.qemu/366140 > > -- > Vasiliy Tolstov, > e-mail: v.tols...@selfip.ru >
Re: [Qemu-devel] [PATCH v2 2/4] qga: do not override configuration verbosity
Quoting marcandre.lur...@redhat.com (2015-09-11 13:53:39)
> From: Marc-André Lureau
>
> Move the default verbosity settings before loading the configuration
> file, or it will overwrite it. Found thanks to writing qga tests :)
>
> Signed-off-by: Marc-André Lureau
> ---
> qga/main.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/qga/main.c b/qga/main.c
> index 18e1e1d..aa6a063 100644
> --- a/qga/main.c
> +++ b/qga/main.c
> @@ -1083,8 +1083,6 @@ static void config_parse(GAConfig *config, int argc,
> char **argv)
> { NULL, 0, NULL, 0 }
> };
>
> -config->log_level = G_LOG_LEVEL_ERROR | G_LOG_LEVEL_CRITICAL;
> -
> while ((ch = getopt_long(argc, argv, sopt, lopt, &opt_ind)) != -1) {
> switch (ch) {
> case 'm':
> @@ -1332,6 +1330,8 @@ int main(int argc, char **argv)
> GAState *s = g_new0(GAState, 1);
> GAConfig *config = g_new0(GAConfig, 1);
>
> +config->log_level = G_LOG_LEVEL_ERROR | G_LOG_LEVEL_CRITICAL;
> +
We currently handle config->state_dir/method/channel_path defaults
after config_parse via if (!config->blah) config->blah = ...; I'd
rather we do it for this too for consistency, but not sure we have
a reliable indicator for determining whether or not config->log_level
is unset (GLogLevelFlags enum doesn't have a 0 value so that would
work, but it's hacky to make such an assumption).
This avoids any hacks so it's worth the trade-off in terms of
consistency so:
Reviewed-by: Michael Roth
> module_call_init(MODULE_INIT_QAPI);
>
> init_dfl_pathnames();
> --
> 2.4.3
>
Re: [Qemu-devel] [PATCH v2 1/4] qga: add QGA_CONF environment variable
Quoting marcandre.lur...@redhat.com (2015-09-11 13:53:38)
> From: Marc-André Lureau
>
> Having a environment variable allows to override default configuration
> path, useful for testing. Note that this can't easily be an argument,
> since loading config is done before parsing the arguments.
I'd rather this be fixed by adding a config_parse_early() prior
config_load() to handle a command-line specification of config
file path, but I don't object to an environment variable to
specify it in lieu of that so:
>
> Signed-off-by: Marc-André Lureau
Reviewed-by: Michael Roth
> ---
> qga/main.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/qga/main.c b/qga/main.c
> index d8e063a..18e1e1d 100644
> --- a/qga/main.c
> +++ b/qga/main.c
> @@ -945,10 +945,11 @@ static void config_load(GAConfig *config)
> {
> GError *gerr = NULL;
> GKeyFile *keyfile;
> +const char *conf = g_getenv("QGA_CONF") ?: QGA_CONF_DEFAULT;
>
> /* read system config */
> keyfile = g_key_file_new();
> -if (!g_key_file_load_from_file(keyfile, QGA_CONF_DEFAULT, 0, &gerr)) {
> +if (!g_key_file_load_from_file(keyfile, conf, 0, &gerr)) {
> goto end;
> }
> if (g_key_file_has_key(keyfile, "general", "daemon", NULL)) {
> --
> 2.4.3
>
Re: [Qemu-devel] [PATCH v2 4/4] tests: add a local test for guest agent
Quoting marcandre.lur...@redhat.com (2015-09-11 13:53:41)
> From: Marc-André Lureau
>
> Add some local guest agent tests (as it is better than nothing) only
> when CONFIG_LINUX.
>
> They can be run inside or outside a VM, when run inside a VM, they will
> do a bit more side effects, such as freezing/thawing the FS or changing
> the time.
>
> A better test would involve a managed VM (or container), but it might be
> better to leave that off to autotest/avocado.
>
> Signed-off-by: Marc-André Lureau
Sorry for the delay in getting to this. Have some review comments but
will make sure to stay on top of any follow-ups as I'm trying to get
this into my next pull.
> ---
> tests/Makefile | 3 +
> tests/test-qga.c | 791
> +++
> 2 files changed, 794 insertions(+)
> create mode 100644 tests/test-qga.c
>
> diff --git a/tests/Makefile b/tests/Makefile
> index 34c6136..d5837a4 100644
> --- a/tests/Makefile
> +++ b/tests/Makefile
> @@ -76,6 +76,7 @@ check-unit-y += tests/test-write-threshold$(EXESUF)
> gcov-files-test-write-threshold-y = block/write-threshold.c
> check-unit-$(CONFIG_GNUTLS_HASH) += tests/test-crypto-hash$(EXESUF)
> check-unit-y += tests/test-crypto-cipher$(EXESUF)
> +check-unit-$(CONFIG_LINUX) += tests/test-qga$(EXESUF)
>
> check-block-$(CONFIG_POSIX) += tests/qemu-iotests-quick.sh
>
> @@ -432,6 +433,8 @@ endif
> qtest-obj-y = tests/libqtest.o libqemuutil.a libqemustub.a
> $(check-qtest-y): $(qtest-obj-y)
>
> +tests/test-qga: tests/test-qga.o $(qtest-obj-y)
> +
> .PHONY: check-help
> check-help:
> @echo "Regression testing targets:"
> diff --git a/tests/test-qga.c b/tests/test-qga.c
> new file mode 100644
> index 000..cfee134
> --- /dev/null
> +++ b/tests/test-qga.c
> @@ -0,0 +1,791 @@
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include "libqtest.h"
> +#include "config-host.h"
> +
> +typedef struct {
> +char *test_dir;
> +GMainLoop *loop;
> +int fd;
> +GPid pid;
> +} TestFixture;
> +
> +static int connect_qga(char *path)
> +{
> +int s, ret, len, i = 0;
> +struct sockaddr_un remote;
> +
> +s = socket(AF_UNIX, SOCK_STREAM, 0);
> +g_assert(s != -1);
> +
> +remote.sun_family = AF_UNIX;
> +do {
> +strcpy(remote.sun_path, path);
> +len = strlen(remote.sun_path) + sizeof(remote.sun_family);
> +ret = connect(s, (struct sockaddr *)&remote, len);
> +if (ret == -1) {
> +g_usleep(G_USEC_PER_SEC);
> +}
> +if (i++ == 5) {
> +return -1;
> +}
> +} while (ret == -1);
> +
> +return s;
> +}
> +
> +static void qga_watch(GPid pid, gint status, gpointer user_data)
> +{
> +TestFixture *fixture = user_data;
> +
> +g_assert_cmpint(status, ==, 0);
> +g_main_loop_quit(fixture->loop);
> +}
> +
> +static void
> +fixture_setup(TestFixture *fixture, gconstpointer data)
> +{
> +const gchar *extra_arg = data;
> +GError *error = NULL;
> +gchar *cwd, *path, *cmd, **argv = NULL;
> +
> +fixture->loop = g_main_loop_new(NULL, FALSE);
> +
> +fixture->test_dir = g_strdup("/tmp/qgatest.XX");
> +g_assert_nonnull(g_mkdtemp(fixture->test_dir));
> +
> +path = g_build_filename(fixture->test_dir, "sock", NULL);
> +cwd = g_get_current_dir();
> +cmd = g_strdup_printf("%s%cqemu-ga -m unix-listen -t %s -p %s %s %s",
> + cwd, G_DIR_SEPARATOR,
> + fixture->test_dir, path,
> + getenv("QTEST_LOG") ? "-v" : "",
> + extra_arg ?: "");
> +g_shell_parse_argv(cmd, NULL, &argv, &error);
> +g_assert_no_error(error);
> +
> +g_spawn_async(fixture->test_dir, argv, NULL,
> + G_SPAWN_SEARCH_PATH|G_SPAWN_DO_NOT_REAP_CHILD,
> + NULL, NULL, &fixture->pid, &error);
> +g_assert_no_error(error);
> +
> +g_child_watch_add(fixture->pid, qga_watch, fixture);
> +
> +fixture->fd = connect_qga(path);
> +
> +g_strfreev(argv);
> +g_free(cmd);
> +g_free(cwd);
> +g_free(path);
> +}
> +
> +static void
> +fixture_tear_down(TestFixture *fixture, gconstpointer data)
> +{
> +gchar *tmp;
> +
> +kill(fixture->pid, SIGTERM);
> +
> +g_main_loop_run(fixture->loop);
> +g_main_loop_unref(fixture->loop);
> +
> +g_spawn_close_pid(fixture->pid);
> +
> +tmp = g_build_filename(fixture->test_dir, "foo", NULL);
> +g_unlink(tmp);
> +g_free(tmp);
> +
> +tmp = g_build_filename(fixture->test_dir, "qga.state", NULL);
> +g_unlink(tmp);
> +g_free(tmp);
> +
> +tmp = g_build_filename(fixture->test_dir, "sock", NULL);
> +g_unlink(tmp);
> +g_free(tmp);
> +
> +g_rmdir(fixture->test_dir);
> +g_free(fixture->test_dir);
> +}
> +
> +static void qmp_assertion_message_error(const char *domain,
> +
Re: [Qemu-devel] [PATCH 3/3] target-xtensa: xtfpga: support noMMU cores
On Thu, Oct 1, 2015 at 1:13 PM, Max Filippov wrote: > On Thu, Oct 1, 2015 at 10:18 PM, Peter Crosthwaite > wrote: >> On Thu, Oct 1, 2015 at 11:25 AM, Max Filippov wrote: >>> On Thu, Oct 1, 2015 at 9:06 PM, Peter Crosthwaite >>> wrote: What is your intended user command lines? E.g. how do I boot with ml605 no-mmu and then with mmu. >>> >>> '-M ml605 -cpu de108' will boot noMMU variant (de108 is not in the mainline >>> yet, >>> can be seen in the >>> https://github.com/OSLL/qemu-xtensa/commits/xtensa-cores), >>> '-M ml605 -cpu dc233c' will boot full MMU variant, as de108 doesn't >>> have full MMU >>> and dc233c does. >> >> So where I was going with this, is due to the way your devtools do >> memory maps, -cpu is the wrong switch altogether as what your tools >> call a "CPU" is different to what QEMU does. I am thinking: >> >> -M ml605 -global ml605.bitstream=xtensa-dc233c >> >> dc233c is a SoC that selects the correct xtensa CPU (either via >> property or an target-extensa CPU also named "dc233c"). When you set >> the bitstream property on the machine, it instantiates the correct SoC >> which in turn already knows the correct memory map and selects the CPU >> for you. >> >> When we have multi-arch working, we can then code-share the FPGA board >> definitions between arches e.g. petalogix-ml605 can be rewritten so >> that this works: >> >> qemu-system-multi -M ml605 -global ml605.bitstream=petalogix-mmu >> qemu-system-multi -M ml605 -global ml605.bitstream=xtensa-dc233c >> >> Names may needs some fine tuning. > > I'm completely lost here. I have a problem at hand: noMMU cores > can't run on XTFPGA boards because of wrong device memory map. I would word it differently, - You have no support for your noMMU designs at all. The two designs may have the same peripheral set so they can and should code share (probably to the point where they just have two const tables just diffing the memory maps) but they are ultimately different. > The board is not meant to be used outside target-xtensa at all, and > even for target-xtensa it's just a demo board. > > What problem do I solve by implementing it in the way you suggest? > You change the interchangable component from just a CPU (QEMU definition of "CPU") to the whole FPGA bitstream which in reality is what happens (unless we open the FPGA partial reconfiguration can-of-worms). So the user interface is matched to what really happens on the boards. The code is also more easily patched in the future, should anyone want to reuse xtensa ml605 with total FPGA design replacement. Regards, Peter > -- > Thanks. > -- Max
Re: [Qemu-devel] [PATCH 1/2] target-i386: Use 1UL for bit shift
Peter Maydell writes: > On 1 October 2015 at 18:30, Paolo Bonzini wrote: >> >> >> On 01/10/2015 19:07, Laszlo Ersek wrote: >>> > In addition, C89 didn't say at all what the result was for signed data >>> > types, so technically we could compile QEMU with -std=gnu89 (the default >>> > until GCC5) and call it a day. >>> > >>> > Really the C standard should make this implementation-defined. >>> >>> Obligatory link: http://blog.regehr.org/archives/1180 >> >> Many ideas in there are good (e.g. mem*() being defined for invalid >> argument and zero lengths, and of course item 7 which is the issue at >> hand). In many cases it's also good to change undefined behavior to >> unspecified values, however I think that goes too far. >> >> For example I'm okay with signed integer overflow being undefined >> behavior, and I also disagree with "It is permissible to compute >> out-of-bounds pointer values including performing pointer arithmetic on >> the null pointer". Using uintptr_t is just fine. > > I bet you QEMU breaks the 'out of bounds pointer arithmetic' > rule all over the place. (set_prop_arraylen(), for a concrete > example off the top of my head.) > > Signed integer overflow being UB is a really terrible idea which > is one of the core cases for nailing down the UB -- everybody > expects signed integers to behave as 2s-complement, when in > fact what the compiler can and will do currently is just do totally > unpredictable things... > >> Also strict aliasing improves performance noticeably at least on some >> kind of code. The relaxation of strict aliasing that GCC does with >> unions would be a useful addition to the C standard, though. > > QEMU currently turns off strict-aliasing entirely, which I think > is entirely sensible of us. > > A lot of the underlying intention behind the proposal (as I > interpret it) is "consistency and predictability of behaviour > for the programmer trumps pure performance". That sounds like > a good idea to me. We do not have a raging "oh my god the compiler can't sufficiently optimize" crisis. We do have a raging "we can't get our software sufficiently reliable" crisis.
Re: [Qemu-devel] [PATCH v7 05/18] qapi: Test for various name collisions
Eric Blake writes: > On 10/01/2015 11:39 AM, Markus Armbruster wrote: > >>> I'm leaning towards A (calling self-inheritance a name collision is a >>> bit of a stretch in the first place; and leaving it untested until 16/46 >>> goes in doesn't hurt). >> >> Okay, A. it is. I pushed to branch qapi-next at >> http://repo.or.cz/qemu/armbru.git > > Branch looks good to go with one nit: 15/18 commit message (currently > e7462ff) has a typo in the text you massaged: > > The generated code has fewer blank line in qapi-event.c functions, > but has no semantic difference. > > s/line/lines/ Fixed and pushed. Thanks! > I'll rebase my remaining patches on top of your tip, and let you decide > how long to wait before sending the pull request. And with your rewrite > of 17/18, this part of the series no longer depends on the python 2.6 > configure patch. I'll wait a few days. We'll need the time to review the next batch of patches anyway.
Re: [Qemu-devel] [PATCH 3/3] target-xtensa: xtfpga: support noMMU cores
On Thu, Oct 1, 2015 at 10:18 PM, Peter Crosthwaite wrote: > On Thu, Oct 1, 2015 at 11:25 AM, Max Filippov wrote: >> On Thu, Oct 1, 2015 at 9:06 PM, Peter Crosthwaite >> wrote: >>> What is your intended user command lines? E.g. how do I boot with >>> ml605 no-mmu and then with mmu. >> >> '-M ml605 -cpu de108' will boot noMMU variant (de108 is not in the mainline >> yet, >> can be seen in the https://github.com/OSLL/qemu-xtensa/commits/xtensa-cores), >> '-M ml605 -cpu dc233c' will boot full MMU variant, as de108 doesn't >> have full MMU >> and dc233c does. > > So where I was going with this, is due to the way your devtools do > memory maps, -cpu is the wrong switch altogether as what your tools > call a "CPU" is different to what QEMU does. I am thinking: > > -M ml605 -global ml605.bitstream=xtensa-dc233c > > dc233c is a SoC that selects the correct xtensa CPU (either via > property or an target-extensa CPU also named "dc233c"). When you set > the bitstream property on the machine, it instantiates the correct SoC > which in turn already knows the correct memory map and selects the CPU > for you. > > When we have multi-arch working, we can then code-share the FPGA board > definitions between arches e.g. petalogix-ml605 can be rewritten so > that this works: > > qemu-system-multi -M ml605 -global ml605.bitstream=petalogix-mmu > qemu-system-multi -M ml605 -global ml605.bitstream=xtensa-dc233c > > Names may needs some fine tuning. I'm completely lost here. I have a problem at hand: noMMU cores can't run on XTFPGA boards because of wrong device memory map. The board is not meant to be used outside target-xtensa at all, and even for target-xtensa it's just a demo board. What problem do I solve by implementing it in the way you suggest? -- Thanks. -- Max
Re: [Qemu-devel] [PATCH RFC 3/8] target-arm: Add support for S2 page-table protection bits
On Thu, Oct 01, 2015 at 08:48:17PM +0100, Peter Maydell wrote:
> On 1 October 2015 at 19:44, Edgar E. Iglesias
> wrote:
> > On Wed, Sep 23, 2015 at 09:55:05AM -0700, Peter Maydell wrote:
> >> This isn't right -- the XN bit controls executability and the
> >> S2AP bits don't affect it at all. I think you want:
> >>
> >> int prot_rw = 0;
> >> if (s2ap & 1) {
> >> prot_rw |= PAGE_READ;
> >> }
> >> if (s2ap & 2) {
> >> prot_rw |= PAGE_WRITE;
> >> }
> >> if (!xn) {
> >> prot_rw |= PAGE_EXEC;
> >> }
> >
> >
> > Thanks, this was the stuff I was worried about when we talked about it
> > last time. I've got the following now which seems to be the same as
> > you suggest:
> >
> > static int get_S2prot(CPUARMState *env, int s2ap, int xn)
> > {
> > int prot;
> >
> > prot = s2ap & 1 ? PAGE_READ : 0;
> > prot |= s2ap & 2 ? PAGE_WRITE : 0;
> > if (!xn) {
> > prot |= PAGE_EXEC;
> > }
> > return prot;
> > }
>
> Yep, that's the right logic, though it seems a bit odd not
> to consistently use either 'if' or '?:' for all 3 bits.
OK, that's true, I'll change it to all ifs.
Thanks,
Edgar
Re: [Qemu-devel] [PATCH v2 6/6] tcg/mips: Support r6 SEL{NE, EQ}Z instead of MOVN/MOVZ
On 10/01/2015 08:58 PM, James Hogan wrote:
Extend MIPS movcond implementation to support the SELNEZ/SELEQZ
instructions introduced in MIPS r6 (where MOVN/MOVZ have been removed).
Whereas the "MOVN/MOVZ rd, rs, rt" instructions have the following
semantics:
rd = [!]rt ? rs : rd
The "SELNEZ/SELEQZ rd, rs, rt" instructions are slightly different:
rd = [!]rt ? rs : 0
First we ensure that if one of the movcond input values is zero that it
comes last (we can swap the input arguments if we invert the condition).
This is so that it can exactly match one of the SELNEZ/SELEQZ
instructions and avoid the need to emit the other one.
Otherwise we emit the opposite instruction first into a temporary
register, and OR that into the result:
SELNEZ/SELEQZ TMP1, v2, c1
SELEQZ/SELNEZ ret, v1, c1
OR ret, ret, TMP1
Which does the following:
ret = cond ? v1 : v2
Signed-off-by: James Hogan
Cc: Richard Henderson
Cc: Aurelien Jarno
---
Changes in v2:
- Combine with patch 6 from v1, and drop functional changes to movcond
implementation pre-r6. We now provide different constraints for
movcond depending on presence of r6. (thanks Richard for feedback).
---
tcg/mips/tcg-target.c | 40 ++--
1 file changed, 34 insertions(+), 6 deletions(-)
diff --git a/tcg/mips/tcg-target.c b/tcg/mips/tcg-target.c
index a937b1475d04..ef1e85a75e22 100644
--- a/tcg/mips/tcg-target.c
+++ b/tcg/mips/tcg-target.c
@@ -314,6 +314,8 @@ typedef enum {
OPC_NOR = OPC_SPECIAL | 0x27,
OPC_SLT = OPC_SPECIAL | 0x2A,
OPC_SLTU = OPC_SPECIAL | 0x2B,
+OPC_SELEQZ = OPC_SPECIAL | 0x35,
+OPC_SELNEZ = OPC_SPECIAL | 0x37,
OPC_REGIMM = 0x01 << 26,
OPC_BLTZ = OPC_REGIMM | (0x00 << 16),
@@ -858,13 +860,24 @@ static void tcg_out_brcond2(TCGContext *s, TCGCond cond,
TCGReg al, TCGReg ah,
}
static void tcg_out_movcond(TCGContext *s, TCGCond cond, TCGReg ret,
-TCGReg c1, TCGReg c2, TCGReg v)
+TCGReg c1, TCGReg c2, TCGReg v1, TCGReg v2)
{
-MIPSInsn m_opc = OPC_MOVN;
+MIPSInsn m_opc_t = use_mips32r6_instructions ? OPC_SELNEZ : OPC_MOVN;
+MIPSInsn m_opc_f = use_mips32r6_instructions ? OPC_SELEQZ : OPC_MOVZ;
+const MIPSInsn m_opc_t_inv = m_opc_f;
+const MIPSInsn m_opc_f_inv = m_opc_t;
+
+/* If one of the values is zero, put it last to match SEL*Z instructions */
+if (use_mips32r6_instructions && v1 == 0) {
+v1 = v2;
+v2 = 0;
+cond = tcg_invert_cond(cond);
+}
switch (cond) {
case TCG_COND_EQ:
-m_opc = OPC_MOVZ;
+m_opc_t = m_opc_t_inv;
+m_opc_f = m_opc_f_inv;
/* FALLTHRU */
case TCG_COND_NE:
if (c2 != 0) {
@@ -877,14 +890,25 @@ static void tcg_out_movcond(TCGContext *s, TCGCond cond,
TCGReg ret,
/* Minimize code size by preferring a compare not requiring INV. */
if (mips_cmp_map[cond] & MIPS_CMP_INV) {
cond = tcg_invert_cond(cond);
-m_opc = OPC_MOVZ;
+m_opc_t = m_opc_t_inv;
+m_opc_f = m_opc_f_inv;
}
tcg_out_setcond(s, cond, TCG_TMP0, c1, c2);
c1 = TCG_TMP0;
break;
}
-tcg_out_opc_reg(s, m_opc, ret, v, c1);
+if (use_mips32r6_instructions) {
+if (v2 != 0) {
+tcg_out_opc_reg(s, m_opc_f, TCG_TMP1, v2, c1);
+}
+tcg_out_opc_reg(s, m_opc_t, ret, v1, c1);
+if (v2 != 0) {
+tcg_out_opc_reg(s, OPC_OR, ret, ret, TCG_TMP1);
+}
+} else {
+tcg_out_opc_reg(s, m_opc_t, ret, v1, c1);
+}
}
I think it's worth just using on "bool ne" in the generic code. Then when you
get down to the r6 conditional selecting the insns.
E.g.
if (use_mips32r6_instructions) {
MIPSInsn m_opc_t = ne ? OPC_SELNEZ : OPC_SELEQZ;
MIPSInsn m_opf_f = ne ? OPC_SELEQZ : OPC_SELNEZ;
...
} else {
MIPSInsn m_opc = ne ? OPC_MOVN : OPC_MOVZ;
...
}
And maybe add either a tcg_debug_assert or a comment in the !r6 path to remind
the reader that we took care of ret == v2 via constraints.
r~
Re: [Qemu-devel] [PATCH RFC 3/8] target-arm: Add support for S2 page-table protection bits
On 1 October 2015 at 19:44, Edgar E. Iglesias wrote:
> On Wed, Sep 23, 2015 at 09:55:05AM -0700, Peter Maydell wrote:
>> This isn't right -- the XN bit controls executability and the
>> S2AP bits don't affect it at all. I think you want:
>>
>> int prot_rw = 0;
>> if (s2ap & 1) {
>> prot_rw |= PAGE_READ;
>> }
>> if (s2ap & 2) {
>> prot_rw |= PAGE_WRITE;
>> }
>> if (!xn) {
>> prot_rw |= PAGE_EXEC;
>> }
>
>
> Thanks, this was the stuff I was worried about when we talked about it
> last time. I've got the following now which seems to be the same as
> you suggest:
>
> static int get_S2prot(CPUARMState *env, int s2ap, int xn)
> {
> int prot;
>
> prot = s2ap & 1 ? PAGE_READ : 0;
> prot |= s2ap & 2 ? PAGE_WRITE : 0;
> if (!xn) {
> prot |= PAGE_EXEC;
> }
> return prot;
> }
Yep, that's the right logic, though it seems a bit odd not
to consistently use either 'if' or '?:' for all 3 bits.
thanks
-- PMM
[Qemu-devel] [PATCHv2] arm: Fail on unknown subtest
Signed-off-by: Christopher Covington
---
arm/selftest.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arm/selftest.c b/arm/selftest.c
index fc9ec60..f4a5030 100644
--- a/arm/selftest.c
+++ b/arm/selftest.c
@@ -376,6 +376,9 @@ int main(int argc, char **argv)
cpumask_set_cpu(0, &smp_reported);
while (!cpumask_full(&smp_reported))
cpu_relax();
+ } else {
+ printf("Unknown subtest\n");
+ abort();
}
return report_summary();
--
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Re: [Qemu-devel] [PATCH v2 5/6] tcg/mips: Support r6 multiply/divide encodings
On 10/01/2015 08:58 PM, James Hogan wrote: MIPSr6 adds several new integer multiply, divide, and modulo instructions, and removes several pre-r6 encodings, along with the HI/LO registers which were the implicit operands of some of those instructions. Update TCG to use the new instructions when built for r6. The new instructions actually map much more directly to the TCG ops, as they only provide a single 32-bit half of the result and in a normal general purpose register instead of HI or LO. The mulu2_i32 and muls2_i32 operations are no longer appropriate for r6, so they are removed from the TCG opcode table. This is because they would need to emit two separate host instructions anyway (for the high and low half of the result), which TCG can arrange automatically for us in the absense of mulu2_i32/muls2_i32 by splitting it into mul_i32 and mul*h_i32 TCG ops. Signed-off-by: James Hogan Cc: Richard Henderson Cc: Aurelien Jarno --- Changes in v2: - Use a common OPC_MUL definition. use_mips32_instructions will always be 1 for MIPS r6 builds (Richard) --- Reviewed-by: Richard Henderson r~
[Qemu-devel] [PATCH] arm: Add PMU test
Beginning with just a read of the control register, add plumbing
for testing the ARM Performance Monitors Unit (PMU).
Signed-off-by: Christopher Covington
---
arm/pmu.c| 31 +++
arm/unittests.cfg| 5 +
config/config-arm-common.mak | 4 +++-
3 files changed, 39 insertions(+), 1 deletion(-)
create mode 100644 arm/pmu.c
diff --git a/arm/pmu.c b/arm/pmu.c
new file mode 100644
index 000..b1e3c7a
--- /dev/null
+++ b/arm/pmu.c
@@ -0,0 +1,31 @@
+#include "libcflat.h"
+
+union pmcr_el0 {
+ struct {
+ unsigned int implementor:8;
+ unsigned int identification_code:8;
+ unsigned int num_counters:5;
+ unsigned int zeros:4;
+ unsigned int cycle_counter_long:1;
+ unsigned int cycle_counter_disable_when_prohibited:1;
+ unsigned int event_counter_export:1;
+ unsigned int cycle_counter_clock_divider:1;
+ unsigned int cycle_counter_reset:1;
+ unsigned int event_counter_reset:1;
+ unsigned int enable:1;
+ } split;
+ uint32_t full;
+};
+
+int main()
+{
+ union pmcr_el0 pmcr;
+
+ asm volatile("mrs %0, pmcr_el0\n" : "=r" (pmcr));
+
+ printf("PMU implementor: 0x%x\n", pmcr.split.implementor);
+ printf("Identification code: 0x%x\n", pmcr.split.identification_code);
+ printf("Event counters: %d\n", pmcr.split.num_counters);
+
+ return report_summary();
+}
diff --git a/arm/unittests.cfg b/arm/unittests.cfg
index e068a0c..d3deb6a 100644
--- a/arm/unittests.cfg
+++ b/arm/unittests.cfg
@@ -35,3 +35,8 @@ file = selftest.flat
smp = `getconf _NPROCESSORS_CONF`
extra_params = -append 'smp'
groups = selftest
+
+# Test PMU support
+[pmu]
+file = pmu.flat
+groups = pmu
diff --git a/config/config-arm-common.mak b/config/config-arm-common.mak
index 698555d..b34d04c 100644
--- a/config/config-arm-common.mak
+++ b/config/config-arm-common.mak
@@ -11,7 +11,8 @@ endif
tests-common = \
$(TEST_DIR)/selftest.flat \
- $(TEST_DIR)/spinlock-test.flat
+ $(TEST_DIR)/spinlock-test.flat \
+ $(TEST_DIR)/pmu.flat
all: test_cases
@@ -70,3 +71,4 @@ test_cases: $(generated_files) $(tests-common) $(tests)
$(TEST_DIR)/selftest.elf: $(cstart.o) $(TEST_DIR)/selftest.o
$(TEST_DIR)/spinlock-test.elf: $(cstart.o) $(TEST_DIR)/spinlock-test.o
+$(TEST_DIR)/pmu.elf: $(cstart.o) $(TEST_DIR)/pmu.o
--
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Re: [Qemu-devel] [PATCH v2 2/6] disas/mips: Add R6 jr/jr.hb to disassembler
On 10/01/2015 08:58 PM, James Hogan wrote: MIPS r6 encodes jr as jalr zero, and jr.hb as jalr.hb zero, so add these encodings to the MIPS disassembly table. Signed-off-by: James Hogan Cc: Aurelien Jarno Cc: Leon Alrae --- disas/mips.c | 2 ++ 1 file changed, 2 insertions(+) Reviewed-by: Richard Henderson r~
Re: [Qemu-devel] [RFC v5 3/6] softmmu: Add helpers for a new slowpath
On 10/01/2015 06:05 PM, alvise rigo wrote: On Wed, Sep 30, 2015 at 10:42 PM, Richard Henderson wrote: On 09/30/2015 07:46 PM, alvise rigo wrote: On Wed, Sep 30, 2015 at 5:58 AM, Richard Henderson wrote: Why would you need to indicate that another cpu has started an exclusive operation on this page? That seems definitely wrong. The cpu_physical_memory_clear_excl_dirty() sets the flag to generate the TLB entry with the EXCL flag. Yes, but surely the clearing of dirty on current_cpu is enough to cause the other cpus to see that they need to set TLB_EXCL when reloading their tlb entries. Why do you need to manipulate the *other* cpu's dirty bit? Because then we can assume that a cpu with the bit cleared has for sure the TLB entries with the EXCL flag set for that specific page. Moreover, knowing which cpus have the EXCL flag set allows to reduce the flushing requests whenever a new LL is issued on the same page. Does it actually help, or is that a guess without numbers? r~
[Qemu-devel] [Bug 1484925] Re: Segfault with custom vnc client
Hi,
Did you resolve your problem? Because I have the same issus..
Dubravko
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1484925
Title:
Segfault with custom vnc client
Status in QEMU:
New
Bug description:
Hey,
I'm using Citrix XenServer 6.5. I worte a script that uses noVNC to
connect to the rfb console via xapi. When I use GRML and try to boot
it, the QEMU process segfaults and kills my VM. This happens when the
screen resizes and the kernel is loading:
recvfrom(3, "\3\1\0\0\0\0\2\200\1\220\3\0\2\200\0\0\0P\1\220", 4096, 0, NULL,
NULL) = 20
--- SIGSEGV {si_signo=SIGSEGV, si_code=SEGV_MAPERR, si_addr=0xb28000} ---
I can see in the child process the following message, right before the parent
Segfaults:
read(4, "cirrus: blanking the screen line_offset=0 height=480\n", 53) = 53
This issue only happens, when I have my custom php/novnc-client
connected. I also tried the nodejs/novnc package from xen-orchestra -
same result. Using the stock client from Citrix XenCenter it works
just fine. So I think it is related to noVNC. I hope this is just a
bug and not exploitable to force a VM to crash or execute code.
XenServer launches the qemu with the following command line:
qemu-dm-25 --syslog -d 25 -m 2048 -boot dc -serial pty -vcpus 1
-videoram 4 -vncunused -k en-us -vnc 127.0.0.1:1 -usb -usbdevice
tablet -net nic,vlan=0,macaddr=8a:43:e2:b1:57:df,model=rtl8139 -net
tap,vlan=0,bridge=xenbr0,ifname=tap25.0 -acpi -monitor pty
XenServer 6.5 is using the following version:
# /usr/lib64/xen/bin/qemu-dm -help
QEMU PC emulator version 0.10.2, Copyright (c) 2003-2008 Fabrice Bellard
Greetings
Uli Stärk
To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1484925/+subscriptions
Re: [Qemu-devel] Enabling PMU in qemu arm64
On Thu, Oct 1, 2015 at 12:21 PM, Christopher Covington wrote: > > Are you using KVM or TCG (are you running on an x86 host or an arm64 host)? I am using TCG, aarch64-softmmu on x86 host. > > We have published some patches implementing the PMU registers and instruction > counting (but not any other events) for TCG mode [1], but more work is > required to get these changes into shape for inclusion upstream. > > 1. https://lists.nongnu.org/archive/html/qemu-devel/2015-08/msg00567.html Thanks for the pointer. From the patch series I can see that patches 7 and 9 are for enabling PMU in ARM virt. Do you plan on submitting them upstream? I will try these patches locally and see how it goes. > > To guide and justify the changes I'm currently trying to write kvm-unit-tests > that measure > > A) IPC using PMCCNTR_EL0 (implemented upstream, at least when not using > -icount) and code with known length in instructions; PMCCNTR_EL0 always returns 0 for me(in 2.4, will check tip). > B) CPU frequency using PMCCNTR_EL0 and CNTVCT_EL0; and > C) instructions event in the PMU for code with known length in instructions I am guessing these two are not upstream yet, would be great to see it there. Thanks! -- Pranith
Re: [Qemu-devel] [RFC v5 0/6] Slow-path for atomic instruction translation
On Wed, Sep 30, 2015 at 06:44:32 +0200, Paolo Bonzini wrote: > I have a doubt about your patches for ll/sc emulation, that I hope you > can clarify. > > From 1ft, both approaches rely on checking a flag during stores. > This is split between the TLB and the CPUState for Alvise's patches (in > order to exploit the existing fast-path checks), and entirely in the > radix tree for Emilio's. However, the idea is the same. Not quite the same idea, see below. > Now, the patch are okay for serial emulation, but I am not sure if it's > possible to do lock-free ll/sc emulation, because there is a race. > > If we check the flag before the store, the race is as follows: > >CPU0CPU1 >--- >check flag >load locked: > set flag > load value (normal load on CPU) >store >store conditional (normal store on CPU) > > where the sc doesn't fail. For completeness, if we check it afterwards > (which would be possible with Emilio's approach, though not for the > TLB-based one): > >CPU0CPU1 >-- >load locked > set bit > load value (normal load on CPU) >store >store conditional (normal store on CPU) >check flag > > and again the sc doesn't fail. (snip) > Tell me I'm wrong. :) The difference between Alvise's implementation and what I submitted is that in the radix tree a cache line that has *ever* had an atomic op performed on it, is marked as "slow path" for the rest of the execution, meaning that *all* subsequent stores to said cache line will take the cache line entry's lock. This does not fix the race completely (e.g. you could have a store and an atomic op concurrently executing on a line that hasn't yet had an atomic op on it), but it significantly closes it. My guess is that it would be very hard to trigger by practical programs. E.
Re: [Qemu-devel] [PATCH 3/3] target-xtensa: xtfpga: support noMMU cores
On Thu, Oct 1, 2015 at 11:25 AM, Max Filippov wrote: > On Thu, Oct 1, 2015 at 9:06 PM, Peter Crosthwaite > wrote: >> On Wed, Sep 30, 2015 at 3:42 PM, Max Filippov wrote: >>> On Thu, Oct 1, 2015 at 1:23 AM, Peter Crosthwaite >>> wrote: On Wed, Sep 30, 2015 at 3:07 PM, Max Filippov wrote: > On Thu, Oct 1, 2015 at 12:02 AM, Peter Crosthwaite > wrote: >> Which makes MMU vs noMMU a board level property, not a CPU property. >> It should appear in QEMU as such. > > Ok, one last try: can this property be queried from the selected CPU > and passed to SoC initialization code? No, what I am trying to say is go the other way round. The property is on the SoC, and it is used to select the CPU type. >>> >>> This is not what happens in reality. In reality the MMU type is the >>> CPU property. >>> A set of CPU properties comes from the outside along with the CPU name, and >>> that's the only possible set of properties for this name. >>> XTFPGA bitstream build scripts lay out its address space based on MMU >>> type of the selected CPU. >>> >>> So let's make XTFPGA an SoC with a property that selects its address space >>> layout. The machine (lx60/lx200/ml605/kc705) is configured with a CPU, >> >> So lx60/lx200 sound like isolated FPGA chips (Virtex 4 lx60?) while >> ml605/kc705 are commonly available fully-featured FPGA dev boards. > > Actually they're all fully-featured FPGA dev boards, from different times. > lx60 and lx200 boards were produced by Avnet (yes, Virtex 4 based), > ml605 and kc705 are from Xilinx. > >> Is lx60 short for a particular board featuring this part or is it more >> abstract? > > Less memory, smaller onboard FLASH, different audio subsystem (not > currently modeled). > >>> it >>> queries MMU type from it and instantiates XTFPGA SoC with proper address >>> space. That seems to be closest to what happens in reality. >> >> What is your intended user command lines? E.g. how do I boot with >> ml605 no-mmu and then with mmu. > > '-M ml605 -cpu de108' will boot noMMU variant (de108 is not in the mainline > yet, > can be seen in the https://github.com/OSLL/qemu-xtensa/commits/xtensa-cores), > '-M ml605 -cpu dc233c' will boot full MMU variant, as de108 doesn't > have full MMU > and dc233c does. > So where I was going with this, is due to the way your devtools do memory maps, -cpu is the wrong switch altogether as what your tools call a "CPU" is different to what QEMU does. I am thinking: -M ml605 -global ml605.bitstream=xtensa-dc233c dc233c is a SoC that selects the correct xtensa CPU (either via property or an target-extensa CPU also named "dc233c"). When you set the bitstream property on the machine, it instantiates the correct SoC which in turn already knows the correct memory map and selects the CPU for you. When we have multi-arch working, we can then code-share the FPGA board definitions between arches e.g. petalogix-ml605 can be rewritten so that this works: qemu-system-multi -M ml605 -global ml605.bitstream=petalogix-mmu qemu-system-multi -M ml605 -global ml605.bitstream=xtensa-dc233c Names may needs some fine tuning. Regards, Peter > -- > Thanks. > -- Max
Re: [Qemu-devel] [PATCH 1/2] target-i386: Use 1UL for bit shift
On 10/01/15 19:38, Peter Maydell wrote:
> On 1 October 2015 at 18:30, Paolo Bonzini wrote:
>>
>>
>> On 01/10/2015 19:07, Laszlo Ersek wrote:
In addition, C89 didn't say at all what the result was for signed data
types, so technically we could compile QEMU with -std=gnu89 (the default
until GCC5) and call it a day.
Really the C standard should make this implementation-defined.
>>>
>>> Obligatory link: http://blog.regehr.org/archives/1180
>>
>> Many ideas in there are good (e.g. mem*() being defined for invalid
>> argument and zero lengths, and of course item 7 which is the issue at
>> hand). In many cases it's also good to change undefined behavior to
>> unspecified values, however I think that goes too far.
>>
>> For example I'm okay with signed integer overflow being undefined
>> behavior, and I also disagree with "It is permissible to compute
>> out-of-bounds pointer values including performing pointer arithmetic on
>> the null pointer". Using uintptr_t is just fine.
>
> I bet you QEMU breaks the 'out of bounds pointer arithmetic'
> rule all over the place. (set_prop_arraylen(), for a concrete
> example off the top of my head.)
>
> Signed integer overflow being UB is a really terrible idea which
> is one of the core cases for nailing down the UB -- everybody
> expects signed integers to behave as 2s-complement, when in
> fact what the compiler can and will do currently is just do totally
> unpredictable things...
>
>> Also strict aliasing improves performance noticeably at least on some
>> kind of code. The relaxation of strict aliasing that GCC does with
>> unions would be a useful addition to the C standard, though.
>
> QEMU currently turns off strict-aliasing entirely, which I think
> is entirely sensible of us.
Hm, I didn't know that. Indeed it is part of QEMU_CFLAGS.
Another example: the kernel. In the top Makefile, KBUILD_CFLAGS gets
-fno-strict-aliasing. And according to
"Documentation/kbuild/makefiles.txt", "... the top Makefile owns the
variable $(KBUILD_CFLAGS) and uses it for compilation flags for the
entire tree".
Yet another example: edk2. (See "BaseTools/Conf/tools_def.template",
GCC_ALL_CC_FLAGS.)
> A lot of the underlying intention behind the proposal (as I
> interpret it) is "consistency and predictability of behaviour
> for the programmer trumps pure performance". That sounds like
> a good idea to me.
I once spent an afternoon interpreting the "effective type" paragraphs
in the C standard ("6.5 Expressions", paragraphs 6 and 7). They make
sense, and it is possible to write conformant code.
Here's an example:
- In the firmware, allocate an array of bytes, dynamically. This array
will have no declared type.
- Populate the array byte-wise, from fw_cfg. Because the stores happen
through character-typed lvalues, they do not "imbue" the target
object with any effective type, for further accesses that do not
modify the value. (I.e., for further reads.)
- Get a (uint8_t*) into the array somewhere, and cast it to
(struct acpi_table_hdr *). Read fields through the cast pointer.
Assuming no out-of-bounds situation (considering the entire
pointed to acpi_table_hdr struct), and assuming no alignment
violations for the fields (which is implementation-defined), these
accesses will be fine.
*However*. If in point 2 you populate the array with uint64_t accesses,
that *does* imbue the array elements with an effective type that is
binding for further read accesses. And, in step 3, because the ACPI
table header struct does not include uint64_t fields, those accesses
will be undefined behavior.
... I don't know who on earth has brain capacity for tracking this.
Effective type *does* propagate in a trackable manner, but it is one
order of magnitude harder to follow for humans than integer conversions
-- and resultant ranges -- are (and those are hard enough already!).
So, it would be nice and prudent to comply with the effective type /
strict aliasing rules, and allow the compiler to optimize "more", but
personally I think I wouldn't be able to track effective type
*realiably* (despite being fully conscious of integer promotions and
conversions, for example). Therefore, I embrace -fno-strict-aliasing.
Thanks
Laszlo
Re: [Qemu-devel] [PATCH v4 3/7] Implement fw_cfg DMA interface
On 10/01/15 19:18, Peter Maydell wrote: > On 1 October 2015 at 15:36, Laszlo Ersek wrote: >> I think I finally understand this difference now. It is all rooted in >> the difference between the internal APIs sysbus_add_io() and >> sysbus_init_mmio(). Both of these are called from the device realize >> functions, but the first (sysbus_add_io()) wants the IO port address at >> once, whereas the second (sysbus_init_mmio()) doesn't want the address >> -- the actual mapping (sysbus_mmio_map()) is delayed to board code; the >> device code doesn't want to be aware of it. > > Yes. The sysbus_add_io() API is firmly wedded to the x86 I/O port > concept and to the idea that devices are at fixed I/O port addresses > which don't depend on what board they're in (because the few > non-x86 systems that set up some kind of "IO port" abstraction > are generally doing it to look more x86-like). > > That said, sysbus_add_io() is one of those odd functions which > we use half a dozen times in the whole codebase and which leaves > me wondering if it ought to be refactored to work differently > (eg split into "declare IO ports" and "map IO ports into IO space" > like the mmio functions)... I had the same idea looming in the back of my mind, but I wouldn't know how to attack the refactoring (plus I wouldn't want to delay Marc's work with it -- after all the function is not being introduced by this series), so I didn't bring it up. :P Thanks! Laszlo
[Qemu-devel] [PATCH v3] linux-user/syscall.c: malloc()/calloc() to g_malloc()/g_try_malloc()/g_new0()
v1-> Convert malloc()/calloc() calls to g_malloc()/g_try_malloc()/g_new0()
in linux-user/syscall.c file
v1->v2 convert the free() call in host_to_target_semarray()
to g_free() and calls g_try_malloc(count) instead of
g_try_malloc(sizeof(count))
Signed-off-by: Harmandeep Kaur
---
v2->v3 used g_try_new() and friends to avoid overflow issues.
linux-user/syscall.c | 20 ++--
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index d1d3eb2..5cfb7dd 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -1554,7 +1554,7 @@ set_timeout:
}
fprog.len = tswap16(tfprog->len);
-filter = malloc(fprog.len * sizeof(*filter));
+filter = g_try_new(struct sock_filter, fprog.len);
if (filter == NULL) {
unlock_user_struct(tfilter, tfprog->filter, 1);
unlock_user_struct(tfprog, optval_addr, 1);
@@ -1570,7 +1570,7 @@ set_timeout:
ret = get_errno(setsockopt(sockfd, SOL_SOCKET,
SO_ATTACH_FILTER, &fprog, sizeof(fprog)));
-free(filter);
+g_free(filter);
unlock_user_struct(tfilter, tfprog->filter, 1);
unlock_user_struct(tfprog, optval_addr, 1);
@@ -1881,7 +1881,7 @@ static struct iovec *lock_iovec(int type, abi_ulong
target_addr,
return NULL;
}
-vec = calloc(count, sizeof(struct iovec));
+vec = g_try_new0(struct iovec, count);
if (vec == NULL) {
errno = ENOMEM;
return NULL;
@@ -1945,7 +1945,7 @@ static struct iovec *lock_iovec(int type, abi_ulong
target_addr,
}
unlock_user(target_vec, target_addr, 0);
fail2:
-free(vec);
+g_free(vec);
errno = err;
return NULL;
}
@@ -2672,14 +2672,14 @@ static inline abi_long target_to_host_semarray(int
semid, unsigned short **host_
nsems = semid_ds.sem_nsems;
-*host_array = malloc(nsems*sizeof(unsigned short));
+*host_array = g_try_new(unsigned short, nsems);
if (!*host_array) {
return -TARGET_ENOMEM;
}
array = lock_user(VERIFY_READ, target_addr,
nsems*sizeof(unsigned short), 1);
if (!array) {
-free(*host_array);
+g_free(*host_array);
return -TARGET_EFAULT;
}
@@ -2975,7 +2975,7 @@ static inline abi_long do_msgsnd(int msqid, abi_long
msgp,
if (!lock_user_struct(VERIFY_READ, target_mb, msgp, 0))
return -TARGET_EFAULT;
-host_mb = malloc(msgsz+sizeof(long));
+host_mb = g_try_malloc(msgsz + sizeof(long));
if (!host_mb) {
unlock_user_struct(target_mb, msgp, 0);
return -TARGET_ENOMEM;
@@ -2983,7 +2983,7 @@ static inline abi_long do_msgsnd(int msqid, abi_long
msgp,
host_mb->mtype = (abi_long) tswapal(target_mb->mtype);
memcpy(host_mb->mtext, target_mb->mtext, msgsz);
ret = get_errno(msgsnd(msqid, host_mb, msgsz, msgflg));
-free(host_mb);
+g_free(host_mb);
unlock_user_struct(target_mb, msgp, 0);
return ret;
@@ -7625,7 +7625,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long
arg1,
struct linux_dirent *dirp;
abi_long count = arg3;
-dirp = malloc(count);
+dirp = g_try_malloc(sizeof(count));
if (!dirp) {
ret = -TARGET_ENOMEM;
goto fail;
@@ -7662,7 +7662,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long
arg1,
ret = count1;
unlock_user(target_dirp, arg2, ret);
}
-free(dirp);
+g_free(dirp);
}
#else
{
--
1.9.1
[Qemu-devel] [PULL 0/1] Block job patches
The following changes since commit fa500928ad9da6dd570918e3dfca13c029af07a8: Merge remote-tracking branch 'remotes/juanquintela/tags/migration/20150930' into staging (2015-10-01 10:49:38 +0100) are available in the git repository at: g...@github.com:codyprime/qemu-kvm-jtc.git tags/block-pull-request for you to fetch changes up to 5279efebcf8f8fbf2ed2feed63cdb9d375c7cd07: block: mirror - fix full sync mode when target does not support zero init (2015-10-01 15:02:21 -0400) Block job patches Jeff Cody (1): block: mirror - fix full sync mode when target does not support zero init block/mirror.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) -- 1.9.3
[Qemu-devel] [PULL 1/1] block: mirror - fix full sync mode when target does not support zero init
During mirror, if the target device does not support zero init, a
mirror may result in a corrupted image for sync="full" mode.
This is due to how the initial dirty bitmap is set up prior to copying
data - we did not mark sectors as dirty that are unallocated. This
means those unallocated sectors are skipped over on the target, and for
a device without zero init, invalid data may reside in those holes.
If both of the following conditions are true, then we will explicitly
mark all sectors as dirty:
1.) sync = "full"
2.) bdrv_has_zero_init(target) == false
If the target does support zero init, but a target image is passed in
with data already present (i.e. an "existing" image), it is assumed the
data present in the existing image is valid data for those sectors.
Reviewed-by: Paolo Bonzini
Message-id:
91ed4bc5bda7e2b09eb508b07c83f4071fe0b3c9.1443705220.git.jc...@redhat.com
Signed-off-by: Jeff Cody
---
block/mirror.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/block/mirror.c b/block/mirror.c
index a258926..87928ab 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -455,6 +455,8 @@ static void coroutine_fn mirror_run(void *opaque)
if (!s->is_none_mode) {
/* First part, loop on the sectors and initialize the dirty bitmap. */
BlockDriverState *base = s->base;
+bool mark_all_dirty = s->base == NULL &&
!bdrv_has_zero_init(s->target);
+
for (sector_num = 0; sector_num < end; ) {
/* Just to make sure we are not exceeding int limit. */
int nb_sectors = MIN(INT_MAX >> BDRV_SECTOR_BITS,
@@ -477,7 +479,7 @@ static void coroutine_fn mirror_run(void *opaque)
}
assert(n > 0);
-if (ret == 1) {
+if (ret == 1 || mark_all_dirty) {
bdrv_set_dirty_bitmap(s->dirty_bitmap, sector_num, n);
}
sector_num += n;
--
1.9.3
Re: [Qemu-devel] [PATCH v3] block: mirror - fix full sync mode when target does not support zero init
On Thu, Oct 01, 2015 at 09:14:51AM -0400, Jeff Cody wrote:
> During mirror, if the target device does not support zero init, a
> mirror may result in a corrupted image for sync="full" mode.
>
> This is due to how the initial dirty bitmap is set up prior to copying
> data - we did not mark sectors as dirty that are unallocated. This
> means those unallocated sectors are skipped over on the target, and for
> a device without zero init, invalid data may reside in those holes.
>
> If both of the following conditions are true, then we will explicitly
> mark all sectors as dirty:
>
> 1.) sync = "full"
> 2.) bdrv_has_zero_init(target) == false
>
> If the target does support zero init, but a target image is passed in
> with data already present (i.e. an "existing" image), it is assumed the
> data present in the existing image is valid data for those sectors.
>
> Signed-off-by: Jeff Cody
> ---
> block/mirror.c | 8 +---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/block/mirror.c b/block/mirror.c
> index a258926..ce367e0 100644
> --- a/block/mirror.c
> +++ b/block/mirror.c
> @@ -455,6 +455,8 @@ static void coroutine_fn mirror_run(void *opaque)
> if (!s->is_none_mode) {
> /* First part, loop on the sectors and initialize the dirty bitmap.
> */
> BlockDriverState *base = s->base;
> +bool mark_all_dirty = s->base == NULL &&
> !bdrv_has_zero_init(s->target);
> +
> for (sector_num = 0; sector_num < end; ) {
> /* Just to make sure we are not exceeding int limit. */
> int nb_sectors = MIN(INT_MAX >> BDRV_SECTOR_BITS,
> @@ -477,7 +479,7 @@ static void coroutine_fn mirror_run(void *opaque)
> }
>
> assert(n > 0);
> -if (ret == 1) {
> +if (ret == 1 || mark_all_dirty) {
> bdrv_set_dirty_bitmap(s->dirty_bitmap, sector_num, n);
> }
> sector_num += n;
> @@ -767,8 +769,8 @@ void mirror_start(BlockDriverState *bs, BlockDriverState
> *target,
> base = mode == MIRROR_SYNC_MODE_TOP ? bs->backing_hd : NULL;
> mirror_start_job(bs, target, replaces,
> speed, granularity, buf_size,
> - on_source_error, on_target_error, unmap, cb, opaque,
> errp,
> - &mirror_job_driver, is_none_mode, base);
> + on_source_error, on_target_error, unmap,
> + cb, opaque, errp, &mirror_job_driver, is_none_mode,
> base);
> }
>
> void commit_active_start(BlockDriverState *bs, BlockDriverState *base,
> --
> 1.9.3
>
Applied to my block branch (with the last hunk removed):
git git://github.com/codyprime/qemu-kvm-jtc.git block
Thanks,
Jeff
Re: [Qemu-devel] [PATCH RFC 3/8] target-arm: Add support for S2 page-table protection bits
On Wed, Sep 23, 2015 at 09:55:05AM -0700, Peter Maydell wrote:
> On 19 September 2015 at 07:15, Edgar E. Iglesias
> wrote:
> > From: "Edgar E. Iglesias"
> >
> > Signed-off-by: Edgar E. Iglesias
> > ---
> > target-arm/helper.c | 48 +---
> > 1 file changed, 45 insertions(+), 3 deletions(-)
> >
> > diff --git a/target-arm/helper.c b/target-arm/helper.c
> > index 33be8c2..6f0ed51 100644
> > --- a/target-arm/helper.c
> > +++ b/target-arm/helper.c
> > @@ -6008,6 +6008,38 @@ simple_ap_to_rw_prot(CPUARMState *env, ARMMMUIdx
> > mmu_idx, int ap)
> > return simple_ap_to_rw_prot_is_user(ap, regime_is_user(env, mmu_idx));
> > }
> >
> > +/* Translate S2 section/page access permissions to protection flags
> > + *
> > + * @env: CPUARMState
> > + * @ap: The 2-bit simple AP (AP[2:1])
>
> I think this should read "The 2-bit stage2 access permissions (S2AP)".
> The interpretation of the field differs from the simple-AP bits.
> You could helpfully call the argument "s2ap" as well.
>
> > + * @xn: XN (execute-never) bit
> > + */
> > +static int get_S2prot(CPUARMState *env, int ap, int xn)
> > +{
> > +int prot_rw;
> > +
> > +switch (ap) {
> > +default:
> > +case 0:
> > +prot_rw = 0;
> > +break;
> > +case 1:
> > +prot_rw = PAGE_READ | PAGE_EXEC;
> > +break;
> > +case 2:
> > +prot_rw = PAGE_WRITE;
> > +break;
> > +case 3:
> > +prot_rw = PAGE_READ | PAGE_EXEC | PAGE_WRITE;
> > +break;
> > +}
> > +
> > +if (xn) {
> > +prot_rw &= ~PAGE_EXEC;
> > +}
>
> This isn't right -- the XN bit controls executability and the
> S2AP bits don't affect it at all. I think you want:
>
> int prot_rw = 0;
> if (s2ap & 1) {
> prot_rw |= PAGE_READ;
> }
> if (s2ap & 2) {
> prot_rw |= PAGE_WRITE;
> }
> if (!xn) {
> prot_rw |= PAGE_EXEC;
> }
Thanks, this was the stuff I was worried about when we talked about it
last time. I've got the following now which seems to be the same as
you suggest:
static int get_S2prot(CPUARMState *env, int s2ap, int xn)
{
int prot;
prot = s2ap & 1 ? PAGE_READ : 0;
prot |= s2ap & 2 ? PAGE_WRITE : 0;
if (!xn) {
prot |= PAGE_EXEC;
}
return prot;
}
I've also fixed up your other comments on this.
Thanks,
Edgar
>
>
> > +return prot_rw;
> > +}
> > +
> > /* Translate section/page access permissions to protection flags
> > *
> > * @env: CPUARMState
> > @@ -6617,6 +6649,11 @@ static bool get_phys_addr_lpae(CPUARMState *env,
> > target_ulong address,
> > /* Extract attributes from the descriptor and merge with table
> > attrs */
> > attrs = extract64(descriptor, 2, 10)
> > | (extract64(descriptor, 52, 12) << 10);
> > +
> > +if (mmu_idx == ARMMMUIdx_S2NS) {
> > +/* The following extractions do not apply to S2. */
>
> I think it would be clearer to say
> /* Stage 2 table descriptors do not include any attribute fields. */
>
> > +break;
> > +}
>
> and then here put
> /* Merge in attributes from table descriptors */
>
> and delete the now-redundant "and merge with table attrs" part from the
> comment at the start of this hunk.
>
> > attrs |= extract32(tableattrs, 0, 2) << 11; /* XN, PXN */
> > attrs |= extract32(tableattrs, 3, 1) << 5; /* APTable[1] => AP[2]
> > */
> > /* The sense of AP[1] vs APTable[0] is reversed, as APTable[0] == 1
> > @@ -6638,11 +6675,16 @@ static bool get_phys_addr_lpae(CPUARMState *env,
> > target_ulong address,
> > }
> >
> > ap = extract32(attrs, 4, 2);
> > -ns = extract32(attrs, 3, 1);
> > xn = extract32(attrs, 12, 1);
> > -pxn = extract32(attrs, 11, 1);
> >
> > -*prot = get_S1prot(env, mmu_idx, va_size == 64, ap, ns, xn, pxn);
> > +if (mmu_idx == ARMMMUIdx_S2NS) {
> > +ns = true;
> > +*prot = get_S2prot(env, ap, xn);
> > +} else {
> > +ns = extract32(attrs, 3, 1);
> > +pxn = extract32(attrs, 11, 1);
> > +*prot = get_S1prot(env, mmu_idx, va_size == 64, ap, ns, xn, pxn);
> > +}
> >
> > fault_type = permission_fault;
> > if (!(*prot & (1 << access_type))) {
> > --
> > 1.9.1
> >
>
> thanks
> -- PMM
Re: [Qemu-devel] [PATCH 1/2] target-i386: Use 1UL for bit shift
On 10/01/15 19:30, Paolo Bonzini wrote: > > > On 01/10/2015 19:07, Laszlo Ersek wrote: >>> In addition, C89 didn't say at all what the result was for signed data >>> types, so technically we could compile QEMU with -std=gnu89 (the default >>> until GCC5) and call it a day. >>> >>> Really the C standard should make this implementation-defined. >> >> Obligatory link: http://blog.regehr.org/archives/1180 > > Many ideas in there are good (e.g. mem*() being defined for invalid > argument and zero lengths, and of course item 7 which is the issue at > hand). In many cases it's also good to change undefined behavior to > unspecified values, however I think that goes too far. > > For example I'm okay with signed integer overflow being undefined > behavior, and I also disagree with "It is permissible to compute > out-of-bounds pointer values including performing pointer arithmetic on > the null pointer". Using uintptr_t is just fine. > > Also strict aliasing improves performance noticeably at least on some > kind of code. The relaxation of strict aliasing that GCC does with > unions would be a useful addition to the C standard, though. What do you mean under "relaxation of strict aliasing that GCC does with unions"? I believe I know how unions affect this (although for details I'd obviously have to consult the standard :)), but what are the gcc specific parts? Thanks! Laszlo
Re: [Qemu-devel] [PATCH v7 05/18] qapi: Test for various name collisions
On 10/01/2015 11:39 AM, Markus Armbruster wrote: >> I'm leaning towards A (calling self-inheritance a name collision is a >> bit of a stretch in the first place; and leaving it untested until 16/46 >> goes in doesn't hurt). > > Okay, A. it is. I pushed to branch qapi-next at > http://repo.or.cz/qemu/armbru.git Branch looks good to go with one nit: 15/18 commit message (currently e7462ff) has a typo in the text you massaged: The generated code has fewer blank line in qapi-event.c functions, but has no semantic difference. s/line/lines/ I'll rebase my remaining patches on top of your tip, and let you decide how long to wait before sending the pull request. And with your rewrite of 17/18, this part of the series no longer depends on the python 2.6 configure patch. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH RFC 4/8] target-arm: Avoid inline for get_phys_addr
On Wed, Sep 23, 2015 at 09:58:10AM -0700, Peter Maydell wrote:
> On 19 September 2015 at 07:15, Edgar E. Iglesias
> wrote:
> > From: "Edgar E. Iglesias"
> >
> > Avoid inline for get_phys_addr() to prepare for future recursive use.
>
> Does the compiler actually complain?
Sorry for the late replies Peter...
Yes the compiler complains if we keep the inline. I'll fix the indentation.
Thanks,
Edgar
>
> In any case this function is a lot more complex than it used to be so
> we might as well just rely on the compiler's discretion about whether
> to bother inlining it or not.
>
> > Signed-off-by: Edgar E. Iglesias
> > ---
> > target-arm/helper.c | 10 +-
> > 1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/target-arm/helper.c b/target-arm/helper.c
> > index 6f0ed51..7e7f29d 100644
> > --- a/target-arm/helper.c
> > +++ b/target-arm/helper.c
> > @@ -13,7 +13,7 @@
> > #include "exec/semihost.h"
> >
> > #ifndef CONFIG_USER_ONLY
> > -static inline bool get_phys_addr(CPUARMState *env, target_ulong address,
> > +static bool get_phys_addr(CPUARMState *env, target_ulong address,
> > int access_type, ARMMMUIdx mmu_idx,
> > hwaddr *phys_ptr, MemTxAttrs *attrs, int
> > *prot,
> > target_ulong *page_size, uint32_t *fsr);
>
> Might as well reindent these lines.
>
> > @@ -6967,10 +6967,10 @@ static bool get_phys_addr_pmsav5(CPUARMState *env,
> > uint32_t address,
> > * @page_size: set to the size of the page containing phys_ptr
> > * @fsr: set to the DFSR/IFSR value on failure
> > */
> > -static inline bool get_phys_addr(CPUARMState *env, target_ulong address,
> > - int access_type, ARMMMUIdx mmu_idx,
> > - hwaddr *phys_ptr, MemTxAttrs *attrs, int
> > *prot,
> > - target_ulong *page_size, uint32_t *fsr)
> > +static bool get_phys_addr(CPUARMState *env, target_ulong address,
> > + int access_type, ARMMMUIdx mmu_idx,
> > + hwaddr *phys_ptr, MemTxAttrs *attrs, int *prot,
> > + target_ulong *page_size, uint32_t *fsr)
> > {
> > if (mmu_idx == ARMMMUIdx_S12NSE0 || mmu_idx == ARMMMUIdx_S12NSE1) {
> > /* TODO: when we support EL2 we should here call ourselves
> > recursively
> > --
> > 1.9.1
> >
>
> Otherwise:
> Reviewed-by: Peter Maydell
>
> thanks
> -- PMM
Re: [Qemu-devel] [PATCH v4 6/7] Make the kernel image in the fw_cfg DMA interface bootable
On Thu, 1 Oct 2015 12:02:42 -0400
"Kevin O'Connor" wrote:
> On Thu, Oct 01, 2015 at 05:25:11PM +0200, Laszlo Ersek wrote:
> > On 10/01/15 14:16, Marc Marí wrote:
> > > Add an entry to the bootorder file with name "vmlinux".
> > > Give this entry more priority than the romfile.
> > >
> > > Signed-off-by: Marc Marí
> > > ---
> > > hw/i386/pc.c | 4 +++-
> > > 1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> > > index 81d93b4..c4c51f7 100644
> > > --- a/hw/i386/pc.c
> > > +++ b/hw/i386/pc.c
> > > @@ -1012,8 +1012,10 @@ static void load_linux(PCMachineState
> > > *pcms, fw_cfg_add_bytes(fw_cfg, FW_CFG_SETUP_DATA, setup,
> > > setup_size);
> > > option_rom[nb_option_roms].name = "linuxboot.bin";
> > > -option_rom[nb_option_roms].bootindex = 0;
> > > +option_rom[nb_option_roms].bootindex = 1;
> > > nb_option_roms++;
> > > +
> > > +add_boot_device_path(0, NULL, "vmlinux");
> > > }
> > >
> > > #define NE2000_NB_MAX 6
> > >
> >
> > Where does this idea come from?
> >
> > This will yet again break the invariant that the bootorder fw_cfg
> > file is a list of OpenFirmware device paths.
>
> I believe it came from a discussion between myself and Marc, because I
> did not like the way Marc's original SeaBIOS patches overloaded the
> meaning of "genroms/linuxboot.bin" in the bootorder file.
I didn't like it either.
> [...]
> > Given that direct kernel boot is always expected to take priority
> > over anything else (which is ensured by this QEMU patch too), can
> > bootprio_find_vmlinux() in SeaBIOS just look at the same fw_cfg key
> > (0x0008)?
>
> That's fine with me. Marc - I think qemu_vmlinux_setup() in SeaBIOS
> with the following would work:
>
> void qemu_vmlinux_setup(void)
> {
> u32 kernel_size;
> qemu_cfg_read_entry(&kernel_size, QEMU_CFG_KERNEL_SIZE,
> sizeof(kernel_size)); if (kernel_size)
> boot_add_qemu_vmlinux("QEMU Kernel image", 0);
> }
>
> Marc, if you're okay with the above, you don't have to keep respinning
> patches - I can fix it up upon commit.
I'm ok with it
Thanks
Marc
Re: [Qemu-devel] [PATCH 3/3] target-xtensa: xtfpga: support noMMU cores
On Thu, Oct 1, 2015 at 9:06 PM, Peter Crosthwaite wrote: > On Wed, Sep 30, 2015 at 3:42 PM, Max Filippov wrote: >> On Thu, Oct 1, 2015 at 1:23 AM, Peter Crosthwaite >> wrote: >>> On Wed, Sep 30, 2015 at 3:07 PM, Max Filippov wrote: On Thu, Oct 1, 2015 at 12:02 AM, Peter Crosthwaite wrote: > Which makes MMU vs noMMU a board level property, not a CPU property. > It should appear in QEMU as such. Ok, one last try: can this property be queried from the selected CPU and passed to SoC initialization code? >>> >>> No, what I am trying to say is go the other way round. The property is >>> on the SoC, and it is used to select the CPU type. >> >> This is not what happens in reality. In reality the MMU type is the >> CPU property. >> A set of CPU properties comes from the outside along with the CPU name, and >> that's the only possible set of properties for this name. >> XTFPGA bitstream build scripts lay out its address space based on MMU >> type of the selected CPU. >> >> So let's make XTFPGA an SoC with a property that selects its address space >> layout. The machine (lx60/lx200/ml605/kc705) is configured with a CPU, > > So lx60/lx200 sound like isolated FPGA chips (Virtex 4 lx60?) while > ml605/kc705 are commonly available fully-featured FPGA dev boards. Actually they're all fully-featured FPGA dev boards, from different times. lx60 and lx200 boards were produced by Avnet (yes, Virtex 4 based), ml605 and kc705 are from Xilinx. > Is lx60 short for a particular board featuring this part or is it more > abstract? Less memory, smaller onboard FLASH, different audio subsystem (not currently modeled). >> it >> queries MMU type from it and instantiates XTFPGA SoC with proper address >> space. That seems to be closest to what happens in reality. > > What is your intended user command lines? E.g. how do I boot with > ml605 no-mmu and then with mmu. '-M ml605 -cpu de108' will boot noMMU variant (de108 is not in the mainline yet, can be seen in the https://github.com/OSLL/qemu-xtensa/commits/xtensa-cores), '-M ml605 -cpu dc233c' will boot full MMU variant, as de108 doesn't have full MMU and dc233c does. -- Thanks. -- Max
Re: [Qemu-devel] [PATCH 3/3] block: prohibit migration during transactions
On 01/10/2015 18:34, John Snow wrote:
> +
> +error_setg(&blocker, "Block device(s) are in use by a Block
> Transaction");
s/Block Transaction/transaction command/
But how can migration start during a transaction?
> +ret = migrate_add_blocker(blocker, errp);
> +if (ret < 0) {
> +goto cleanup_mig;
> +}
>
> QSIMPLEQ_HEAD(snap_bdrv_states, BlkTransactionState) snap_bdrv_states;
> QSIMPLEQ_INIT(&snap_bdrv_states);
> @@ -1814,6 +1823,9 @@ exit:
> }
> g_free(state);
> }
> + cleanup_mig:
> +migrate_del_blocker(blocker);
> +error_free(blocker);
Re: [Qemu-devel] [PATCH 1/3] block: prohibit migration during BlockJobs
On 01/10/2015 18:34, John Snow wrote: > Unless we can prove this to be safe for specific cases, > the default should be to prohibit migration during BlockJobs. Block jobs do not affect the current block, only other block device, hence they *are* safe for migration. What you want, I think, is the target not to be garbage when migration ends. Based on this you can block specific cases, namely mirror which you already do allow (patch 2) and backup except for sync='none'. Paolo > In conjunction with > "migration: disallow_migrate_add_blocker during migration", > this should be sufficient to disallow the blockjob from starting > in the event of an in-progress migration.
Re: [Qemu-devel] [PATCH 3/3] target-xtensa: xtfpga: support noMMU cores
On Wed, Sep 30, 2015 at 3:42 PM, Max Filippov wrote: > On Thu, Oct 1, 2015 at 1:23 AM, Peter Crosthwaite > wrote: >> On Wed, Sep 30, 2015 at 3:07 PM, Max Filippov wrote: >>> On Thu, Oct 1, 2015 at 12:02 AM, Peter Crosthwaite >>> wrote: Which makes MMU vs noMMU a board level property, not a CPU property. It should appear in QEMU as such. >>> >>> Ok, one last try: can this property be queried from the selected CPU >>> and passed to SoC initialization code? >> >> No, what I am trying to say is go the other way round. The property is >> on the SoC, and it is used to select the CPU type. > > This is not what happens in reality. In reality the MMU type is the > CPU property. > A set of CPU properties comes from the outside along with the CPU name, and > that's the only possible set of properties for this name. > XTFPGA bitstream build scripts lay out its address space based on MMU > type of the selected CPU. > > So let's make XTFPGA an SoC with a property that selects its address space > layout. The machine (lx60/lx200/ml605/kc705) is configured with a CPU, So lx60/lx200 sound like isolated FPGA chips (Virtex 4 lx60?) while ml605/kc705 are commonly available fully-featured FPGA dev boards. Is lx60 short for a particular board featuring this part or is it more abstract? > it > queries MMU type from it and instantiates XTFPGA SoC with proper address > space. That seems to be closest to what happens in reality. > What is your intended user command lines? E.g. how do I boot with ml605 no-mmu and then with mmu. Regards, Peter > -- > Thanks. > -- Max
[Qemu-devel] [RFC PATCH] Add qemu .clang-format
From: Marc-André Lureau clang-format is awesome to reflow your code according to qemu coding style in an editor (in the region you modify). (note: clang-tidy should be able to add missing braces around statements, but I haven't tried it, it's quite recent) Signed-off-by: Marc-André Lureau --- .clang-format | 6 ++ 1 file changed, 6 insertions(+) create mode 100644 .clang-format diff --git a/.clang-format b/.clang-format new file mode 100644 index 000..6422547 --- /dev/null +++ b/.clang-format @@ -0,0 +1,6 @@ +BasedOnStyle: LLVM +IndentWidth: 4 +UseTab: Never +BreakBeforeBraces: Linux +AllowShortIfStatementsOnASingleLine: false +IndentCaseLabels: false -- 2.4.3
Re: [Qemu-devel] [PATCH v7 05/18] qapi: Test for various name collisions
Eric Blake writes: > On 10/01/2015 09:34 AM, Markus Armbruster wrote: > Do we have a test for the simpler case of a struct inheriting from itself? >>> >>> Not here, but in v5 16/46. That's because it asserts, but while it was >>> easy to fix up in the QAPISchema.check(), I did not find it worth the >>> churn to fix it up in the ad hoc parse code just to rip it back out >>> later, and the QAPISchema.check() code requires several scaffolding >>> patches (so it wasn't as easy as fixing the union 'type' clash asserts). >>> Tracking an assertion failure for any more than one patch at a time is >>> horrible (as any other change to qapi.py changes line numbers that >>> affect the assertion failure). >> >> Well, I'm happy to take a test for inheritance loops, or leave it >> uncovered for now, but I don't want to take a non-working test of an >> unimplemented obscure case "union" without a test for the implemented >> case "struct". >> >> I can: >> >> A. Drop this test case. >> >> B. Replace it with the test case from 16/46. >> >> C. Add the test case from 16/46 and keep this one. >> >> I very much prefer B. You? > > If we go with B, we'd have an assertion failure that does not get fixed > by 6/18, and therefore is subject to churn until the fix is present. > > I'm leaning towards A (calling self-inheritance a name collision is a > bit of a stretch in the first place; and leaving it untested until 16/46 > goes in doesn't hurt). Okay, A. it is. I pushed to branch qapi-next at http://repo.or.cz/qemu/armbru.git
[Qemu-devel] [PATCH v7 24/24] vhost-user-test: check ownership during migration
From: Marc-André Lureau
Check that backend source and destination do not have simultaneous
ownership during migration.
Signed-off-by: Marc-André Lureau
---
tests/vhost-user-test.c | 38 ++
1 file changed, 38 insertions(+)
diff --git a/tests/vhost-user-test.c b/tests/vhost-user-test.c
index ef22e3e..a74c934 100644
--- a/tests/vhost-user-test.c
+++ b/tests/vhost-user-test.c
@@ -307,6 +307,10 @@ static void chr_read(void *opaque, const uint8_t *buf, int
size)
g_cond_signal(&s->data_cond);
break;
+case VHOST_USER_RESET_DEVICE:
+s->fds_num = 0;
+break;
+
default:
break;
}
@@ -461,12 +465,37 @@ static guint64 get_log_size(TestServer *s)
return log_size;
}
+typedef struct TestMigrateSource {
+GSource source;
+TestServer *src;
+TestServer *dest;
+} TestMigrateSource;
+
+static gboolean
+test_migrate_source_check(GSource *source)
+{
+TestMigrateSource *t = (TestMigrateSource *)source;
+gboolean overlap = t->src->fds_num > 0 && t->dest->fds_num > 0;
+
+g_assert(!overlap);
+
+return FALSE;
+}
+
+GSourceFuncs test_migrate_source_funcs = {
+NULL,
+test_migrate_source_check,
+NULL,
+NULL
+};
+
static void test_migrate(void)
{
TestServer *s = test_server_new("src");
TestServer *dest = test_server_new("dest");
const char *uri = "tcp:127.0.0.1:1234";
QTestState *global = global_qtest, *from, *to;
+GSource *source;
gchar *cmd;
QDict *rsp;
guint8 *log;
@@ -484,6 +513,12 @@ static void test_migrate(void)
to = qtest_init(cmd);
g_free(cmd);
+source = g_source_new(&test_migrate_source_funcs,
+ sizeof(TestMigrateSource));
+((TestMigrateSource *)source)->src = s;
+((TestMigrateSource *)source)->dest = dest;
+g_source_attach(source, NULL);
+
/* slow down migration to have time to fiddle with log */
/* TODO: qtest could learn to break on some places */
rsp = qmp("{ 'execute': 'migrate_set_speed',"
@@ -522,6 +557,9 @@ static void test_migrate(void)
read_guest_mem(dest);
+g_source_destroy(source);
+g_source_unref(source);
+
qtest_quit(to);
test_server_free(dest);
qtest_quit(from);
--
2.4.3
[Qemu-devel] [PATCH v7 18/24] vhost: add migration block if memfd failed
From: Marc-André Lureau
Signed-off-by: Marc-André Lureau
---
hw/virtio/vhost.c| 3 +++
include/qemu/memfd.h | 2 ++
util/memfd.c | 22 ++
3 files changed, 27 insertions(+)
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 1e8ee76..eaf8117 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -994,6 +994,9 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
if (!(hdev->features & (0x1ULL << VHOST_F_LOG_ALL))) {
error_setg(&hdev->migration_blocker,
"Migration disabled: vhost lacks VHOST_F_LOG_ALL
feature.");
+} else if (!qemu_memfd_check()) {
+error_setg(&hdev->migration_blocker,
+ "Migration disabled: failed to allocate shared memory");
}
}
diff --git a/include/qemu/memfd.h b/include/qemu/memfd.h
index 950fb88..53858ed 100644
--- a/include/qemu/memfd.h
+++ b/include/qemu/memfd.h
@@ -2,6 +2,7 @@
#define QEMU_MEMFD_H
#include "config-host.h"
+#include
#ifndef F_LINUX_SPECIFIC_BASE
#define F_LINUX_SPECIFIC_BASE 1024
@@ -20,5 +21,6 @@
void *qemu_memfd_alloc(const char *name, size_t size, unsigned int seals,
int *fd);
void qemu_memfd_free(void *ptr, size_t size, int fd);
+bool qemu_memfd_check(void);
#endif /* QEMU_MEMFD_H */
diff --git a/util/memfd.c b/util/memfd.c
index ff577b5..2632436 100644
--- a/util/memfd.c
+++ b/util/memfd.c
@@ -141,3 +141,25 @@ void qemu_memfd_free(void *ptr, size_t size, int fd)
close(fd);
}
}
+
+enum {
+MEMFD_KO,
+MEMFD_OK,
+MEMFD_TODO
+};
+
+bool qemu_memfd_check(void)
+{
+static int memfd_check = MEMFD_TODO;
+
+if (memfd_check == MEMFD_TODO) {
+int fd;
+void *ptr;
+
+ptr = qemu_memfd_alloc("test", 4096, 0, &fd);
+memfd_check = ptr ? MEMFD_OK : MEMFD_KO;
+qemu_memfd_free(ptr, 4096, fd);
+}
+
+return memfd_check == MEMFD_OK;
+}
--
2.4.3
[Qemu-devel] [PATCH v7 21/24] vhost-user-test: wrap server in TestServer struct
From: Marc-André Lureau
In the coming patches, a test will use several servers
simultaneously. Wrap the server in a struct, out of the global scope.
Signed-off-by: Marc-André Lureau
---
tests/vhost-user-test.c | 139 +++-
1 file changed, 89 insertions(+), 50 deletions(-)
diff --git a/tests/vhost-user-test.c b/tests/vhost-user-test.c
index 4be5583..034d89b 100644
--- a/tests/vhost-user-test.c
+++ b/tests/vhost-user-test.c
@@ -108,10 +108,16 @@ static VhostUserMsg m __attribute__ ((unused));
#define VHOST_USER_VERSION(0x1)
/*/
-int fds_num = 0, fds[VHOST_MEMORY_MAX_NREGIONS];
-static VhostUserMemory memory;
-static CompatGMutex data_mutex;
-static CompatGCond data_cond;
+typedef struct TestServer {
+gchar *socket_path;
+gchar *chr_name;
+CharDriverState *chr;
+int fds_num;
+int fds[VHOST_MEMORY_MAX_NREGIONS];
+VhostUserMemory memory;
+GMutex data_mutex;
+GCond data_cond;
+} TestServer;
#if !GLIB_CHECK_VERSION(2, 32, 0)
static gboolean g_cond_wait_until(CompatGCond cond, CompatGMutex mutex,
@@ -126,67 +132,68 @@ static gboolean g_cond_wait_until(CompatGCond cond,
CompatGMutex mutex,
}
#endif
-static void wait_for_fds(void)
+static void wait_for_fds(TestServer *s)
{
gint64 end_time;
-g_mutex_lock(&data_mutex);
+g_mutex_lock(&s->data_mutex);
end_time = g_get_monotonic_time() + 5 * G_TIME_SPAN_SECOND;
-while (!fds_num) {
-if (!g_cond_wait_until(&data_cond, &data_mutex, end_time)) {
+while (!s->fds_num) {
+if (!g_cond_wait_until(&s->data_cond, &s->data_mutex, end_time)) {
/* timeout has passed */
-g_assert(fds_num);
+g_assert(s->fds_num);
break;
}
}
/* check for sanity */
-g_assert_cmpint(fds_num, >, 0);
-g_assert_cmpint(fds_num, ==, memory.nregions);
+g_assert_cmpint(s->fds_num, >, 0);
+g_assert_cmpint(s->fds_num, ==, s->memory.nregions);
-g_mutex_unlock(&data_mutex);
+g_mutex_unlock(&s->data_mutex);
}
-static void read_guest_mem(void)
+static void read_guest_mem(TestServer *s)
{
uint32_t *guest_mem;
int i, j;
size_t size;
-wait_for_fds();
+wait_for_fds(s);
-g_mutex_lock(&data_mutex);
+g_mutex_lock(&s->data_mutex);
/* iterate all regions */
-for (i = 0; i < fds_num; i++) {
+for (i = 0; i < s->fds_num; i++) {
/* We'll check only the region statring at 0x0*/
-if (memory.regions[i].guest_phys_addr != 0x0) {
+if (s->memory.regions[i].guest_phys_addr != 0x0) {
continue;
}
-g_assert_cmpint(memory.regions[i].memory_size, >, 1024);
+g_assert_cmpint(s->memory.regions[i].memory_size, >, 1024);
-size = memory.regions[i].memory_size + memory.regions[i].mmap_offset;
+size = s->memory.regions[i].memory_size +
+s->memory.regions[i].mmap_offset;
guest_mem = mmap(0, size, PROT_READ | PROT_WRITE,
- MAP_SHARED, fds[i], 0);
+ MAP_SHARED, s->fds[i], 0);
g_assert(guest_mem != MAP_FAILED);
-guest_mem += (memory.regions[i].mmap_offset / sizeof(*guest_mem));
+guest_mem += (s->memory.regions[i].mmap_offset / sizeof(*guest_mem));
for (j = 0; j < 256; j++) {
-uint32_t a = readl(memory.regions[i].guest_phys_addr + j*4);
+uint32_t a = readl(s->memory.regions[i].guest_phys_addr + j*4);
uint32_t b = guest_mem[j];
g_assert_cmpint(a, ==, b);
}
-munmap(guest_mem, memory.regions[i].memory_size);
+munmap(guest_mem, s->memory.regions[i].memory_size);
}
-g_mutex_unlock(&data_mutex);
+g_mutex_unlock(&s->data_mutex);
}
static void *thread_function(void *data)
@@ -204,7 +211,8 @@ static int chr_can_read(void *opaque)
static void chr_read(void *opaque, const uint8_t *buf, int size)
{
-CharDriverState *chr = opaque;
+TestServer *s = opaque;
+CharDriverState *chr = s->chr;
VhostUserMsg msg;
uint8_t *p = (uint8_t *) &msg;
int fd;
@@ -214,12 +222,12 @@ static void chr_read(void *opaque, const uint8_t *buf,
int size)
return;
}
-g_mutex_lock(&data_mutex);
+g_mutex_lock(&s->data_mutex);
memcpy(p, buf, VHOST_USER_HDR_SIZE);
if (msg.size) {
p += VHOST_USER_HDR_SIZE;
-qemu_chr_fe_read_all(chr, p, msg.size);
+g_assert_cmpint(qemu_chr_fe_read_all(chr, p, msg.size), ==, msg.size);
}
switch (msg.request) {
@@ -257,11 +265,11 @@ static void chr_read(void *opaque, const uint8_t *buf,
int size)
case VHOST_USER_SET_MEM_TABLE:
/* received the mem table */
-memcpy(&memory, &msg.memory, sizeof(msg.memory));
-fds_num = qemu_chr_fe_get_msgfds(chr, fds, sizeof(fds) / sizeof(int));
+
[Qemu-devel] [PATCH v7 16/24] vhost user: add rarp sending after live migration for legacy guest
From: Thibaut Collet
A new vhost user message is added to allow QEMU to ask to vhost user backend to
broadcast a fake RARP after live migration for guest without GUEST_ANNOUNCE
capability.
This new message is sent only if the backend supports the new
VHOST_USER_PROTOCOL_F_RARP protocol feature.
The payload of this new message is the MAC address of the guest (not known by
the backend). The MAC address is copied in the first 6 bytes of a u64 to avoid
to create a new payload message type.
This new message has no equivalent ioctl so a new callback is added in the
userOps structure to send the request.
Upon reception of this new message the vhost user backend must generate and
broadcast a fake RARP request to notify the migration is terminated.
Signed-off-by: Thibaut Collet
[Rebased and fixed checkpatch errors - Marc-André]
Signed-off-by: Marc-André Lureau
---
docs/specs/vhost-user.txt | 15 +++
hw/net/vhost_net.c| 17 +
hw/virtio/vhost-user.c| 32 +++-
include/hw/virtio/vhost-backend.h | 3 +++
include/net/vhost_net.h | 1 +
net/vhost-user.c | 24 ++--
6 files changed, 89 insertions(+), 3 deletions(-)
diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
index e0292a0..e0d71e2 100644
--- a/docs/specs/vhost-user.txt
+++ b/docs/specs/vhost-user.txt
@@ -194,6 +194,7 @@ Protocol features
#define VHOST_USER_PROTOCOL_F_MQ 0
#define VHOST_USER_PROTOCOL_F_LOG_SHMFD 1
+#define VHOST_USER_PROTOCOL_F_RARP 2
Message types
-
@@ -381,3 +382,17 @@ Message types
Master payload: vring state description
Signal slave to enable or disable corresponding vring.
+
+ * VHOST_USER_SEND_RARP
+
+ Id: 19
+ Equivalent ioctl: N/A
+ Master payload: u64
+
+ Ask vhost user backend to broadcast a fake RARP to notify the migration
+ is terminated for guest that does not support GUEST_ANNOUNCE.
+ Only legal if feature bit VHOST_USER_F_PROTOCOL_FEATURES is present in
+ VHOST_USER_GET_FEATURES and protocol feature bit
VHOST_USER_PROTOCOL_F_RARP
+ is present in VHOST_USER_GET_PROTOCOL_FEATURES.
+ The first 6 bytes of the payload contain the mac address of the guest to
+ allow the vhost user backend to construct and broadcast the fake RARP.
diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index 840f443..da66b64 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -388,6 +388,18 @@ void vhost_net_cleanup(struct vhost_net *net)
g_free(net);
}
+int vhost_net_notify_migration_done(struct vhost_net *net, char* mac_addr)
+{
+const VhostOps *vhost_ops = net->dev.vhost_ops;
+int r = -1;
+
+if (vhost_ops->vhost_migration_done) {
+r = vhost_ops->vhost_migration_done(&net->dev, mac_addr);
+}
+
+return r;
+}
+
bool vhost_net_virtqueue_pending(VHostNetState *net, int idx)
{
return vhost_virtqueue_pending(&net->dev, idx);
@@ -479,6 +491,11 @@ void vhost_net_virtqueue_mask(VHostNetState *net,
VirtIODevice *dev,
{
}
+int vhost_net_notify_migration_done(struct vhost_net *net)
+{
+return -1;
+}
+
VHostNetState *get_vhost_net(NetClientState *nc)
{
return 0;
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index cd84f0c..5ab888d 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -10,6 +10,7 @@
#include "hw/virtio/vhost.h"
#include "hw/virtio/vhost-backend.h"
+#include "hw/virtio/virtio-net.h"
#include "sysemu/char.h"
#include "sysemu/kvm.h"
#include "qemu/error-report.h"
@@ -27,9 +28,10 @@
#define VHOST_MEMORY_MAX_NREGIONS8
#define VHOST_USER_F_PROTOCOL_FEATURES 30
-#define VHOST_USER_PROTOCOL_FEATURE_MASK 0x3ULL
+#define VHOST_USER_PROTOCOL_FEATURE_MASK 0x7ULL
#define VHOST_USER_PROTOCOL_F_MQ 0
#define VHOST_USER_PROTOCOL_F_LOG_SHMFD 1
+#define VHOST_USER_PROTOCOL_F_RARP 2
typedef enum VhostUserRequest {
VHOST_USER_NONE = 0,
@@ -51,6 +53,7 @@ typedef enum VhostUserRequest {
VHOST_USER_SET_PROTOCOL_FEATURES = 16,
VHOST_USER_GET_QUEUE_NUM = 17,
VHOST_USER_SET_VRING_ENABLE = 18,
+VHOST_USER_SEND_RARP = 19,
VHOST_USER_MAX
} VhostUserRequest;
@@ -557,6 +560,32 @@ static bool vhost_user_requires_shm_log(struct vhost_dev
*dev)
VHOST_USER_PROTOCOL_F_LOG_SHMFD);
}
+static int vhost_user_migration_done(struct vhost_dev *dev, char* mac_addr)
+{
+VhostUserMsg msg = { 0 };
+int err;
+
+assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER);
+
+/* If guest supports GUEST_ANNOUNCE do nothing */
+if (virtio_has_feature(dev->acked_features, VIRTIO_NET_F_GUEST_ANNOUNCE)) {
+return 0;
+}
+
+/* if backend supports VHOST_USER_PROTOCOL_F_RARP ask it to send the RARP
*/
+if (virtio_has_feature(dev->protocol_features,
+ VHOST_USER_PROTOCOL_F_RARP)
[Qemu-devel] [PATCH v7 17/24] vhost-user: use an enum helper for features mask
From: Thibaut Collet
The VHOST_USER_PROTOCOL_FEATURE_MASK will be automatically updated when
adding new features to the enum.
Signed-off-by: Thibaut Collet
[Adapted from mailing list discussion - Marc-André]
Signed-off-by: Marc-André Lureau
---
hw/virtio/vhost-user.c | 13 +
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 5ab888d..bbf5eeb 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -28,10 +28,15 @@
#define VHOST_MEMORY_MAX_NREGIONS8
#define VHOST_USER_F_PROTOCOL_FEATURES 30
-#define VHOST_USER_PROTOCOL_FEATURE_MASK 0x7ULL
-#define VHOST_USER_PROTOCOL_F_MQ 0
-#define VHOST_USER_PROTOCOL_F_LOG_SHMFD 1
-#define VHOST_USER_PROTOCOL_F_RARP 2
+enum VhostUserProtocolFeature {
+VHOST_USER_PROTOCOL_F_MQ = 0,
+VHOST_USER_PROTOCOL_F_LOG_SHMFD = 1,
+VHOST_USER_PROTOCOL_F_RARP = 2,
+
+VHOST_USER_PROTOCOL_F_MAX
+};
+
+#define VHOST_USER_PROTOCOL_FEATURE_MASK ((1 << VHOST_USER_PROTOCOL_F_MAX) - 1)
typedef enum VhostUserRequest {
VHOST_USER_NONE = 0,
--
2.4.3
Re: [Qemu-devel] [PATCH v1 1/3] ssi: Move ssi.h into a seperate directory
On Thu, Oct 1, 2015 at 10:19 AM, Alistair Francis wrote: > On Wed, Sep 30, 2015 at 2:06 PM, Peter Crosthwaite > wrote: >> On Tue, Sep 29, 2015 at 4:03 PM, Alistair Francis >> wrote: >>> Move the ssi.h include file into the ssi directory. >>> >>> Signed-off-by: Alistair Francis >>> --- >>> >>> hw/arm/pxa2xx.c | 2 +- >>> hw/arm/spitz.c | 2 +- >>> hw/arm/stellaris.c | 2 +- >>> hw/arm/strongarm.c | 2 +- >>> hw/arm/tosa.c | 2 +- >>> hw/arm/xilinx_zynq.c| 2 +- >>> hw/arm/z2.c | 2 +- >>> hw/block/m25p80.c | 2 +- >>> hw/display/ads7846.c| 2 +- >>> hw/display/ssd0323.c| 2 +- >>> hw/microblaze/petalogix_ml605_mmu.c | 2 +- >>> hw/misc/max111x.c | 2 +- >>> hw/sd/ssi-sd.c | 2 +- >>> hw/ssi/pl022.c | 2 +- >>> hw/ssi/ssi.c| 2 +- >>> hw/ssi/xilinx_spi.c | 2 +- >>> hw/ssi/xilinx_spips.c | 2 +- >>> include/hw/ssi.h| 94 >>> >>> include/hw/ssi/ssi.h| 96 >>> + >> >> Curious as to why git didn't pick this up as a rename (I think it >> should handle small diffs in the renaming). Do you have rename >> detection turned on? > > I do have rename detection turned on. I think the reason it didn't is > because I tiddied up the code a little bit when I moved it. Checkpatch > was reproting errors on the struct typedeffs so I had to change that. > Can we do it in two steps and take the hit on the checkpatch fails on P1? Checkpatch fails are acceptable for pure mass-code movements. There is also probably some way to set your rename detction to higher level that should pick it up. Regards, Peter > Thanks, > > Alistair > >> >> Regards, >> Peter >> >>> 19 files changed, 113 insertions(+), 111 deletions(-) >>> delete mode 100644 include/hw/ssi.h >>> create mode 100644 include/hw/ssi/ssi.h >>> >>> diff --git a/hw/arm/pxa2xx.c b/hw/arm/pxa2xx.c >>> index 164260a..534c06f 100644 >>> --- a/hw/arm/pxa2xx.c >>> +++ b/hw/arm/pxa2xx.c >>> @@ -12,7 +12,7 @@ >>> #include "sysemu/sysemu.h" >>> #include "hw/char/serial.h" >>> #include "hw/i2c/i2c.h" >>> -#include "hw/ssi.h" >>> +#include "hw/ssi/ssi.h" >>> #include "sysemu/char.h" >>> #include "sysemu/block-backend.h" >>> #include "sysemu/blockdev.h" >>> diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c >>> index 2af03be..c9405af 100644 >>> --- a/hw/arm/spitz.c >>> +++ b/hw/arm/spitz.c >>> @@ -16,7 +16,7 @@ >>> #include "sysemu/sysemu.h" >>> #include "hw/pcmcia.h" >>> #include "hw/i2c/i2c.h" >>> -#include "hw/ssi.h" >>> +#include "hw/ssi/ssi.h" >>> #include "hw/block/flash.h" >>> #include "qemu/timer.h" >>> #include "hw/devices.h" >>> diff --git a/hw/arm/stellaris.c b/hw/arm/stellaris.c >>> index 3d6486f..c785e90 100644 >>> --- a/hw/arm/stellaris.c >>> +++ b/hw/arm/stellaris.c >>> @@ -8,7 +8,7 @@ >>> */ >>> >>> #include "hw/sysbus.h" >>> -#include "hw/ssi.h" >>> +#include "hw/ssi/ssi.h" >>> #include "hw/arm/arm.h" >>> #include "hw/devices.h" >>> #include "qemu/timer.h" >>> diff --git a/hw/arm/strongarm.c b/hw/arm/strongarm.c >>> index 9624ecb..4d2ba02 100644 >>> --- a/hw/arm/strongarm.c >>> +++ b/hw/arm/strongarm.c >>> @@ -34,7 +34,7 @@ >>> #include "hw/arm/arm.h" >>> #include "sysemu/char.h" >>> #include "sysemu/sysemu.h" >>> -#include "hw/ssi.h" >>> +#include "hw/ssi/ssi.h" >>> >>> //#define DEBUG >>> >>> diff --git a/hw/arm/tosa.c b/hw/arm/tosa.c >>> index 51d0b89..6b210a6 100644 >>> --- a/hw/arm/tosa.c >>> +++ b/hw/arm/tosa.c >>> @@ -19,7 +19,7 @@ >>> #include "hw/pcmcia.h" >>> #include "hw/boards.h" >>> #include "hw/i2c/i2c.h" >>> -#include "hw/ssi.h" >>> +#include "hw/ssi/ssi.h" >>> #include "sysemu/block-backend.h" >>> #include "hw/sysbus.h" >>> #include "exec/address-spaces.h" >>> diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c >>> index 9f89483..9db9602 100644 >>> --- a/hw/arm/xilinx_zynq.c >>> +++ b/hw/arm/xilinx_zynq.c >>> @@ -24,7 +24,7 @@ >>> #include "hw/block/flash.h" >>> #include "sysemu/block-backend.h" >>> #include "hw/loader.h" >>> -#include "hw/ssi.h" >>> +#include "hw/ssi/ssi.h" >>> #include "qemu/error-report.h" >>> >>> #define NUM_SPI_FLASHES 4 >>> diff --git a/hw/arm/z2.c b/hw/arm/z2.c >>> index b44eb76..c82fe2c 100644 >>> --- a/hw/arm/z2.c >>> +++ b/hw/arm/z2.c >>> @@ -16,7 +16,7 @@ >>> #include "hw/arm/arm.h" >>> #include "hw/devices.h" >>> #include "hw/i2c/i2c.h" >>> -#include "hw/ssi.h" >>> +#include "hw/ssi/ssi.h" >>> #include "hw/boards.h" >>> #include "sysemu/sysemu.h" >>> #include "hw/block/flash.h" >>> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c >>> index efc43dd..44830c7 100644 >>> --- a/hw/block/m25p80.c >>> +++ b/hw/block/m25p80.c >>> @@ -24,7 +24,7 @@ >>> #include "hw/h
Re: [Qemu-devel] [PATCH 1/2] target-i386: Use 1UL for bit shift
On 1 October 2015 at 18:30, Paolo Bonzini wrote: > > > On 01/10/2015 19:07, Laszlo Ersek wrote: >> > In addition, C89 didn't say at all what the result was for signed data >> > types, so technically we could compile QEMU with -std=gnu89 (the default >> > until GCC5) and call it a day. >> > >> > Really the C standard should make this implementation-defined. >> >> Obligatory link: http://blog.regehr.org/archives/1180 > > Many ideas in there are good (e.g. mem*() being defined for invalid > argument and zero lengths, and of course item 7 which is the issue at > hand). In many cases it's also good to change undefined behavior to > unspecified values, however I think that goes too far. > > For example I'm okay with signed integer overflow being undefined > behavior, and I also disagree with "It is permissible to compute > out-of-bounds pointer values including performing pointer arithmetic on > the null pointer". Using uintptr_t is just fine. I bet you QEMU breaks the 'out of bounds pointer arithmetic' rule all over the place. (set_prop_arraylen(), for a concrete example off the top of my head.) Signed integer overflow being UB is a really terrible idea which is one of the core cases for nailing down the UB -- everybody expects signed integers to behave as 2s-complement, when in fact what the compiler can and will do currently is just do totally unpredictable things... > Also strict aliasing improves performance noticeably at least on some > kind of code. The relaxation of strict aliasing that GCC does with > unions would be a useful addition to the C standard, though. QEMU currently turns off strict-aliasing entirely, which I think is entirely sensible of us. A lot of the underlying intention behind the proposal (as I interpret it) is "consistency and predictability of behaviour for the programmer trumps pure performance". That sounds like a good idea to me. thanks -- PMM
[Qemu-devel] [PATCH v7 12/24] vhost: use a function for each call
From: Marc-André Lureau
Replace the generic vhost_call() by specific functions for each
function call to help with type safety and changing arguments.
While doing this, I found that "unsigned long long" and "uint64_t" were
used interchangeably and causing compilation warnings, using uint64_t
instead, as the vhost & protocol specifies.
Signed-off-by: Marc-André Lureau
---
hw/net/vhost_net.c| 16 +-
hw/scsi/vhost-scsi.c | 7 +-
hw/virtio/vhost-backend.c | 124 -
hw/virtio/vhost-user.c| 518 ++
hw/virtio/vhost.c | 36 +--
include/hw/virtio/vhost-backend.h | 63 -
include/hw/virtio/vhost.h | 12 +-
7 files changed, 501 insertions(+), 275 deletions(-)
diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index 2bce891..1ab4133 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -252,8 +252,7 @@ static int vhost_net_start_one(struct vhost_net *net,
file.fd = net->backend;
for (file.index = 0; file.index < net->dev.nvqs; ++file.index) {
const VhostOps *vhost_ops = net->dev.vhost_ops;
-r = vhost_ops->vhost_call(&net->dev, VHOST_NET_SET_BACKEND,
- &file);
+r = vhost_ops->vhost_net_set_backend(&net->dev, &file);
if (r < 0) {
r = -errno;
goto fail;
@@ -266,8 +265,7 @@ fail:
if (net->nc->info->type == NET_CLIENT_OPTIONS_KIND_TAP) {
while (file.index-- > 0) {
const VhostOps *vhost_ops = net->dev.vhost_ops;
-int r = vhost_ops->vhost_call(&net->dev, VHOST_NET_SET_BACKEND,
- &file);
+int r = vhost_ops->vhost_net_set_backend(&net->dev, &file);
assert(r >= 0);
}
}
@@ -289,15 +287,13 @@ static void vhost_net_stop_one(struct vhost_net *net,
if (net->nc->info->type == NET_CLIENT_OPTIONS_KIND_TAP) {
for (file.index = 0; file.index < net->dev.nvqs; ++file.index) {
const VhostOps *vhost_ops = net->dev.vhost_ops;
-int r = vhost_ops->vhost_call(&net->dev, VHOST_NET_SET_BACKEND,
- &file);
+int r = vhost_ops->vhost_net_set_backend(&net->dev, &file);
assert(r >= 0);
}
} else if (net->nc->info->type == NET_CLIENT_OPTIONS_KIND_VHOST_USER) {
for (file.index = 0; file.index < net->dev.nvqs; ++file.index) {
const VhostOps *vhost_ops = net->dev.vhost_ops;
-int r = vhost_ops->vhost_call(&net->dev, VHOST_RESET_DEVICE,
- NULL);
+int r = vhost_ops->vhost_reset_device(&net->dev);
assert(r >= 0);
}
}
@@ -428,8 +424,8 @@ int vhost_set_vring_enable(NetClientState *nc, int enable)
VHostNetState *net = get_vhost_net(nc);
const VhostOps *vhost_ops = net->dev.vhost_ops;
-if (vhost_ops->vhost_backend_set_vring_enable) {
-return vhost_ops->vhost_backend_set_vring_enable(&net->dev, enable);
+if (vhost_ops->vhost_set_vring_enable) {
+return vhost_ops->vhost_set_vring_enable(&net->dev, enable);
}
return 0;
diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c
index fb7983d..00cdac6 100644
--- a/hw/scsi/vhost-scsi.c
+++ b/hw/scsi/vhost-scsi.c
@@ -46,7 +46,7 @@ static int vhost_scsi_set_endpoint(VHostSCSI *s)
memset(&backend, 0, sizeof(backend));
pstrcpy(backend.vhost_wwpn, sizeof(backend.vhost_wwpn), vs->conf.wwpn);
-ret = vhost_ops->vhost_call(&s->dev, VHOST_SCSI_SET_ENDPOINT, &backend);
+ret = vhost_ops->vhost_scsi_set_endpoint(&s->dev, &backend);
if (ret < 0) {
return -errno;
}
@@ -61,7 +61,7 @@ static void vhost_scsi_clear_endpoint(VHostSCSI *s)
memset(&backend, 0, sizeof(backend));
pstrcpy(backend.vhost_wwpn, sizeof(backend.vhost_wwpn), vs->conf.wwpn);
-vhost_ops->vhost_call(&s->dev, VHOST_SCSI_CLEAR_ENDPOINT, &backend);
+vhost_ops->vhost_scsi_clear_endpoint(&s->dev, &backend);
}
static int vhost_scsi_start(VHostSCSI *s)
@@ -77,8 +77,7 @@ static int vhost_scsi_start(VHostSCSI *s)
return -ENOSYS;
}
-ret = vhost_ops->vhost_call(&s->dev,
-VHOST_SCSI_GET_ABI_VERSION, &abi_version);
+ret = vhost_ops->vhost_scsi_get_abi_version(&s->dev, &abi_version);
if (ret < 0) {
return -errno;
}
diff --git a/hw/virtio/vhost-backend.c b/hw/virtio/vhost-backend.c
index 87ab028..3820af4 100644
--- a/hw/virtio/vhost-backend.c
+++ b/hw/virtio/vhost-backend.c
@@ -43,27 +43,135 @@ static int vhost_kernel_cleanup(struct vhost_dev *dev)
return close(fd);
}
-static int vhost_kernel_get_vq_index(struct vhost_dev *dev, int idx)
+static int vhost_kernel_net_set_backend(struct vhost_dev *dev,
+struct vhost_vring_file *file)
{
-a
[Qemu-devel] [PATCH v7 14/24] net: add trace_vhost_user_event
From: Marc-André Lureau
Replace error_report() and use tracing instead. It's not an error to get
a connection or a disconnection, so silence this and trace it instead.
Signed-off-by: Marc-André Lureau
---
net/vhost-user.c | 4 ++--
trace-events | 3 +++
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/net/vhost-user.c b/net/vhost-user.c
index 8f354eb..9b38431 100644
--- a/net/vhost-user.c
+++ b/net/vhost-user.c
@@ -15,6 +15,7 @@
#include "qemu/config-file.h"
#include "qemu/error-report.h"
#include "qmp-commands.h"
+#include "trace.h"
typedef struct VhostUserState {
NetClientState nc;
@@ -148,18 +149,17 @@ static void net_vhost_user_event(void *opaque, int event)
NET_CLIENT_OPTIONS_KIND_NIC,
MAX_QUEUE_NUM);
s = DO_UPCAST(VhostUserState, nc, ncs[0]);
+trace_vhost_user_event(s->chr->label, event);
switch (event) {
case CHR_EVENT_OPENED:
if (vhost_user_start(queues, ncs) < 0) {
exit(1);
}
qmp_set_link(name, true, &err);
-error_report("chardev \"%s\" went up", s->chr->label);
break;
case CHR_EVENT_CLOSED:
qmp_set_link(name, true, &err);
vhost_user_stop(queues, ncs);
-error_report("chardev \"%s\" went down", s->chr->label);
break;
}
diff --git a/trace-events b/trace-events
index 36db793..6ca9868 100644
--- a/trace-events
+++ b/trace-events
@@ -1697,3 +1697,6 @@ qcrypto_tls_creds_x509_load_cert_list(void *creds, const
char *file) "TLS creds
# crypto/tlssession.c
qcrypto_tls_session_new(void *session, void *creds, const char *hostname,
const char *aclname, int endpoint) "TLS session new session=%p creds=%p
hostname=%s aclname=%s endpoint=%d"
+
+# net/vhost-user.c
+vhost_user_event(const char *chr, int event) "chr: %s got event: %d"
--
2.4.3
[Qemu-devel] [PATCH v7 23/24] vhost-user-test: add live-migration test
From: Marc-André Lureau
This test checks that the log fd is given to the migration source, and
mark dirty pages during migration.
Signed-off-by: Marc-André Lureau
---
tests/vhost-user-test.c | 171 +++-
1 file changed, 169 insertions(+), 2 deletions(-)
diff --git a/tests/vhost-user-test.c b/tests/vhost-user-test.c
index 791d849..ef22e3e 100644
--- a/tests/vhost-user-test.c
+++ b/tests/vhost-user-test.c
@@ -12,6 +12,7 @@
#include "libqtest.h"
#include "qemu/option.h"
+#include "qemu/range.h"
#include "sysemu/char.h"
#include "sysemu/sysemu.h"
@@ -47,6 +48,9 @@
#define VHOST_MEMORY_MAX_NREGIONS8
#define VHOST_USER_F_PROTOCOL_FEATURES 30
+#define VHOST_USER_PROTOCOL_F_LOG_SHMFD 1
+
+#define VHOST_LOG_PAGE 0x1000
typedef enum VhostUserRequest {
VHOST_USER_NONE = 0,
@@ -117,6 +121,7 @@ typedef struct TestServer {
VhostUserMemory memory;
GMutex data_mutex;
GCond data_cond;
+int log_fd;
} TestServer;
#if !GLIB_CHECK_VERSION(2, 32, 0)
@@ -238,7 +243,8 @@ static void chr_read(void *opaque, const uint8_t *buf, int
size)
/* send back features to qemu */
msg.flags |= VHOST_USER_REPLY_MASK;
msg.size = sizeof(m.u64);
-msg.u64 = 0x1ULL << VHOST_USER_F_PROTOCOL_FEATURES;
+msg.u64 = 0x1ULL << VHOST_F_LOG_ALL |
+0x1ULL << VHOST_USER_F_PROTOCOL_FEATURES;
p = (uint8_t *) &msg;
qemu_chr_fe_write_all(chr, p, VHOST_USER_HDR_SIZE + msg.size);
break;
@@ -252,7 +258,7 @@ static void chr_read(void *opaque, const uint8_t *buf, int
size)
/* send back features to qemu */
msg.flags |= VHOST_USER_REPLY_MASK;
msg.size = sizeof(m.u64);
-msg.u64 = 0;
+msg.u64 = 1 << VHOST_USER_PROTOCOL_F_LOG_SHMFD;
p = (uint8_t *) &msg;
qemu_chr_fe_write_all(chr, p, VHOST_USER_HDR_SIZE + msg.size);
break;
@@ -286,6 +292,21 @@ static void chr_read(void *opaque, const uint8_t *buf, int
size)
*/
qemu_set_nonblock(fd);
break;
+
+case VHOST_USER_SET_LOG_BASE:
+if (s->log_fd != -1) {
+close(s->log_fd);
+s->log_fd = -1;
+}
+qemu_chr_fe_get_msgfds(chr, &s->log_fd, 1);
+msg.flags |= VHOST_USER_REPLY_MASK;
+msg.size = 0;
+p = (uint8_t *) &msg;
+qemu_chr_fe_write_all(chr, p, VHOST_USER_HDR_SIZE);
+
+g_cond_signal(&s->data_cond);
+break;
+
default:
break;
}
@@ -337,6 +358,8 @@ static TestServer *test_server_new(const gchar *name)
g_mutex_init(&server->data_mutex);
g_cond_init(&server->data_cond);
+server->log_fd = -1;
+
return server;
}
@@ -358,12 +381,155 @@ static void test_server_free(TestServer *server)
close(server->fds[i]);
}
+if (server->log_fd != -1) {
+close(server->log_fd);
+}
+
unlink(server->socket_path);
g_free(server->socket_path);
+
+g_free(server->chr_name);
g_free(server);
}
+static void wait_for_log_fd(TestServer *s)
+{
+gint64 end_time;
+
+g_mutex_lock(&s->data_mutex);
+end_time = g_get_monotonic_time() + 5 * G_TIME_SPAN_SECOND;
+while (s->log_fd == -1) {
+if (!g_cond_wait_until(&s->data_cond, &s->data_mutex, end_time)) {
+/* timeout has passed */
+g_assert(s->log_fd != -1);
+break;
+}
+}
+
+g_mutex_unlock(&s->data_mutex);
+}
+
+static void write_guest_mem(TestServer *s, uint32 seed)
+{
+uint32_t *guest_mem;
+int i, j;
+size_t size;
+
+wait_for_fds(s);
+
+/* iterate all regions */
+for (i = 0; i < s->fds_num; i++) {
+
+/* We'll write only the region statring at 0x0 */
+if (s->memory.regions[i].guest_phys_addr != 0x0) {
+continue;
+}
+
+g_assert_cmpint(s->memory.regions[i].memory_size, >, 1024);
+
+size = s->memory.regions[i].memory_size +
+s->memory.regions[i].mmap_offset;
+
+guest_mem = mmap(0, size, PROT_READ | PROT_WRITE,
+ MAP_SHARED, s->fds[i], 0);
+
+g_assert(guest_mem != MAP_FAILED);
+guest_mem += (s->memory.regions[i].mmap_offset / sizeof(*guest_mem));
+
+for (j = 0; j < 256; j++) {
+guest_mem[j] = seed + j;
+}
+
+munmap(guest_mem, s->memory.regions[i].memory_size);
+break;
+}
+}
+
+static guint64 get_log_size(TestServer *s)
+{
+guint64 log_size = 0;
+int i;
+
+for (i = 0; i < s->memory.nregions; ++i) {
+VhostUserMemoryRegion *reg = &s->memory.regions[i];
+guint64 last = range_get_last(reg->guest_phys_addr,
+ reg->memory_size);
+log_size = MAX(log_size, last / (8 * VHOST_LOG_PAGE) + 1);
+}
+
+return log_size;
+}
+
+static void test_migrate(void)
+{
+TestServer *s = test_server_new("src");
+TestServer *dest = test_server_new("dest
[Qemu-devel] [PATCH v7 13/24] vhost-user: document migration log
From: Marc-André Lureau Signed-off-by: Marc-André Lureau --- docs/specs/vhost-user.txt | 48 +-- 1 file changed, 46 insertions(+), 2 deletions(-) diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt index 4eadad1..e0292a0 100644 --- a/docs/specs/vhost-user.txt +++ b/docs/specs/vhost-user.txt @@ -115,11 +115,13 @@ the ones that do: * VHOST_GET_FEATURES * VHOST_GET_PROTOCOL_FEATURES * VHOST_GET_VRING_BASE + * VHOST_SET_LOG_BASE (if VHOST_USER_PROTOCOL_F_LOG_SHMFD) There are several messages that the master sends with file descriptors passed in the ancillary data: * VHOST_SET_MEM_TABLE + * VHOST_SET_LOG_BASE (if VHOST_USER_PROTOCOL_F_LOG_SHMFD) * VHOST_SET_LOG_FD * VHOST_SET_VRING_KICK * VHOST_SET_VRING_CALL @@ -140,8 +142,7 @@ Multiple queue support Multiple queue is treated as a protocol extension, hence the slave has to implement protocol features first. The multiple queues feature is supported -only when the protocol feature VHOST_USER_PROTOCOL_F_MQ (bit 0) is set: -#define VHOST_USER_PROTOCOL_F_MQ0 +only when the protocol feature VHOST_USER_PROTOCOL_F_MQ (bit 0) is set. The max number of queues the slave supports can be queried with message VHOST_USER_GET_PROTOCOL_FEATURES. Master should stop when the number of @@ -152,6 +153,48 @@ queue in the sent message to identify a specified queue. One queue pair is enabled initially. More queues are enabled dynamically, by sending message VHOST_USER_SET_VRING_ENABLE. +Migration +- + +During live migration, the master may need to track the modifications +the slave makes to the memory mapped regions. The client should mark +the dirty pages in a log. Once it complies to this logging, it may +declare the VHOST_F_LOG_ALL vhost feature. + +All the modifications to memory pointed by vring "descriptor" should +be marked. Modifications to "used" vring should be marked if +VHOST_VRING_F_LOG is part of ring's features. + +Dirty pages are of size: +#define VHOST_LOG_PAGE 0x1000 + +The log memory fd is provided in the ancillary data of +VHOST_USER_SET_LOG_BASE message when the slave has +VHOST_USER_PROTOCOL_F_LOG_SHMFD protocol feature. + +The size of the log may be computed by using all the known guest +addresses. The log covers from address 0 to the maximum of guest +regions. In pseudo-code, to mark page at "addr" as dirty: + +page = addr / VHOST_LOG_PAGE +log[page / 8] |= 1 << page % 8 + +Use atomic operations, as the log may be concurrently manipulated. + +VHOST_USER_SET_LOG_FD is an optional message with an eventfd in +ancillary data, it may be used to inform the master that the log has +been modified. + +Once the source has finished migration, VHOST_USER_RESET_OWNER message +will be sent by the source. No further update must be done before the +destination takes over with new regions & rings. + +Protocol features +- + +#define VHOST_USER_PROTOCOL_F_MQ 0 +#define VHOST_USER_PROTOCOL_F_LOG_SHMFD 1 + Message types - @@ -236,6 +279,7 @@ Message types Id: 6 Equivalent ioctl: VHOST_SET_LOG_BASE Master payload: u64 + Slave payload: N/A Sets the logging base address. -- 2.4.3
[Qemu-devel] [PATCH v7 05/24] util: add fallback for qemu_memfd_alloc()
From: Marc-André Lureau
Add an open/unlink/mmap fallback for system that do not support
memfd (only available since 3.17, ~1y ago).
This patch may require additional SELinux policies to work for enforced
systems, but should fail gracefully in this case.
Signed-off-by: Marc-André Lureau
---
util/memfd.c | 20 ++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/util/memfd.c b/util/memfd.c
index dd47552..ff577b5 100644
--- a/util/memfd.c
+++ b/util/memfd.c
@@ -100,8 +100,24 @@ void *qemu_memfd_alloc(const char *name, size_t size,
unsigned int seals,
return NULL;
}
} else {
-perror("memfd");
-return NULL;
+const char *tmpdir = g_get_tmp_dir();
+gchar *fname;
+
+fname = g_strdup_printf("%s/memfd-XX", tmpdir);
+mfd = mkstemp(fname);
+unlink(fname);
+g_free(fname);
+
+if (mfd == -1) {
+perror("mkstemp");
+return NULL;
+}
+
+if (ftruncate(mfd, size) == -1) {
+perror("ftruncate");
+close(mfd);
+return NULL;
+}
}
ptr = mmap(0, size, PROT_READ | PROT_WRITE, MAP_SHARED, mfd, 0);
--
2.4.3
[Qemu-devel] [PATCH v7 15/24] vhost user: add support of live migration
From: Thibaut Collet
Some vhost user backends are able to support live migration.
To provide this service the following features must be added:
1. Add the VIRTIO_NET_F_GUEST_ANNOUNCE capability to vhost-net when netdev
backend is vhost-user.
2. Provide a nop receive callback to vhost-user.
This callback is called by:
* qemu_announce_self after a migration to send fake RARP to avoid network
outage for peers talking to the migrated guest.
- For guest with GUEST_ANNOUNCE capabilities, guest already sends GARP
when the bit VIRTIO_NET_S_ANNOUNCE is set.
=> These packets must be discarded.
- For guest without GUEST_ANNOUNCE capabilities, migration termination
is notified when the guest sends packets.
=> These packets can be discarded.
* virtio_net_tx_bh with a dummy boot to send fake bootp/dhcp request.
BIOS guest manages virtio driver to send 4 bootp/dhcp request in case of
dummy boot.
=> These packets must be discarded.
Signed-off-by: Thibaut Collet
---
hw/net/vhost_net.c | 2 ++
net/vhost-user.c | 10 ++
2 files changed, 12 insertions(+)
diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index 1ab4133..840f443 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -85,6 +85,8 @@ static const int user_feature_bits[] = {
VIRTIO_NET_F_CTRL_MAC_ADDR,
VIRTIO_NET_F_CTRL_GUEST_OFFLOADS,
+VIRTIO_NET_F_GUEST_ANNOUNCE,
+
VIRTIO_NET_F_MQ,
VHOST_INVALID_FEATURE_BIT
diff --git a/net/vhost-user.c b/net/vhost-user.c
index 9b38431..87917a5 100644
--- a/net/vhost-user.c
+++ b/net/vhost-user.c
@@ -103,6 +103,15 @@ err:
return -1;
}
+static ssize_t vhost_user_receive(NetClientState *nc, const uint8_t *buf,
+ size_t size)
+{
+/* Discard the request that is received and managed by backend
+ * by an other way.
+ */
+return size;
+}
+
static void vhost_user_cleanup(NetClientState *nc)
{
VhostUserState *s = DO_UPCAST(VhostUserState, nc, nc);
@@ -132,6 +141,7 @@ static bool vhost_user_has_ufo(NetClientState *nc)
static NetClientInfo net_vhost_user_info = {
.type = NET_CLIENT_OPTIONS_KIND_VHOST_USER,
.size = sizeof(VhostUserState),
+.receive = vhost_user_receive,
.cleanup = vhost_user_cleanup,
.has_vnet_hdr = vhost_user_has_vnet_hdr,
.has_ufo = vhost_user_has_ufo,
--
2.4.3
[Qemu-devel] [PATCH v7 02/24] linux-headers: add unistd.h
From: Marc-André Lureau New syscalls are not yet widely distributed. Add them to qemu linux-headers include directory. Update based on v4.3-rc3 kernel headers. Exclude mips for now, which is more problematic due to extra header inclusion and probably unnecessary here. Signed-off-by: Marc-André Lureau --- linux-headers/asm-arm/unistd.h | 448 +++ linux-headers/asm-arm64/kvm.h | 37 +- linux-headers/asm-arm64/unistd.h | 16 + linux-headers/asm-mips/unistd.h| 1063 linux-headers/asm-powerpc/unistd.h | 392 + linux-headers/asm-s390/unistd.h| 404 ++ linux-headers/asm-x86/unistd.h | 15 + linux-headers/asm-x86/unistd_32.h | 377 + linux-headers/asm-x86/unistd_64.h | 330 +++ linux-headers/asm-x86/unistd_x32.h | 319 +++ scripts/update-linux-headers.sh|7 +- 11 files changed, 3404 insertions(+), 4 deletions(-) create mode 100644 linux-headers/asm-arm/unistd.h create mode 100644 linux-headers/asm-arm64/unistd.h create mode 100644 linux-headers/asm-mips/unistd.h create mode 100644 linux-headers/asm-powerpc/unistd.h create mode 100644 linux-headers/asm-s390/unistd.h create mode 100644 linux-headers/asm-x86/unistd.h create mode 100644 linux-headers/asm-x86/unistd_32.h create mode 100644 linux-headers/asm-x86/unistd_64.h create mode 100644 linux-headers/asm-x86/unistd_x32.h diff --git a/linux-headers/asm-arm/unistd.h b/linux-headers/asm-arm/unistd.h new file mode 100644 index 000..0a1376c --- /dev/null +++ b/linux-headers/asm-arm/unistd.h @@ -0,0 +1,448 @@ +/* + * arch/arm/include/asm/unistd.h + * + * Copyright (C) 2001-2005 Russell King + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * Please forward _all_ changes to this file to r...@arm.linux.org.uk, + * no matter what the change is. Thanks! + */ +#ifndef __ASM_ARM_UNISTD_H +#define __ASM_ARM_UNISTD_H + +#define __NR_OABI_SYSCALL_BASE 0x90 + +#if defined(__thumb__) || defined(__ARM_EABI__) +#define __NR_SYSCALL_BASE 0 +#else +#define __NR_SYSCALL_BASE __NR_OABI_SYSCALL_BASE +#endif + +/* + * This file contains the system call numbers. + */ + +#define __NR_restart_syscall (__NR_SYSCALL_BASE+ 0) +#define __NR_exit (__NR_SYSCALL_BASE+ 1) +#define __NR_fork (__NR_SYSCALL_BASE+ 2) +#define __NR_read (__NR_SYSCALL_BASE+ 3) +#define __NR_write (__NR_SYSCALL_BASE+ 4) +#define __NR_open (__NR_SYSCALL_BASE+ 5) +#define __NR_close (__NR_SYSCALL_BASE+ 6) + /* 7 was sys_waitpid */ +#define __NR_creat (__NR_SYSCALL_BASE+ 8) +#define __NR_link (__NR_SYSCALL_BASE+ 9) +#define __NR_unlink(__NR_SYSCALL_BASE+ 10) +#define __NR_execve(__NR_SYSCALL_BASE+ 11) +#define __NR_chdir (__NR_SYSCALL_BASE+ 12) +#define __NR_time (__NR_SYSCALL_BASE+ 13) +#define __NR_mknod (__NR_SYSCALL_BASE+ 14) +#define __NR_chmod (__NR_SYSCALL_BASE+ 15) +#define __NR_lchown(__NR_SYSCALL_BASE+ 16) + /* 17 was sys_break */ + /* 18 was sys_stat */ +#define __NR_lseek (__NR_SYSCALL_BASE+ 19) +#define __NR_getpid(__NR_SYSCALL_BASE+ 20) +#define __NR_mount (__NR_SYSCALL_BASE+ 21) +#define __NR_umount(__NR_SYSCALL_BASE+ 22) +#define __NR_setuid(__NR_SYSCALL_BASE+ 23) +#define __NR_getuid(__NR_SYSCALL_BASE+ 24) +#define __NR_stime (__NR_SYSCALL_BASE+ 25) +#define __NR_ptrace(__NR_SYSCALL_BASE+ 26) +#define __NR_alarm (__NR_SYSCALL_BASE+ 27) + /* 28 was sys_fstat */ +#define __NR_pause (__NR_SYSCALL_BASE+ 29) +#define __NR_utime (__NR_SYSCALL_BASE+ 30) + /* 31 was sys_stty */ + /* 32 was sys_gtty */ +#define __NR_access(__NR_SYSCALL_BASE+ 33) +#define __NR_nice (__NR_SYSCALL_BASE+ 34) + /* 35 was sys_ftime */ +#define __NR_sync (__NR_SYSCALL_BASE+ 36) +#define __NR_kill (__NR_SYSCALL_BASE+ 37) +#define __NR_rename(__NR_SYSCALL_BASE+ 38) +#define __NR_mkdir (__NR_SYSCALL_BASE+ 39) +#define __NR_rmdir (__NR_SYSCALL_BASE+ 40) +#define __NR_dup
[Qemu-devel] [PATCH v7 04/24] util: add memfd helpers
From: Marc-André Lureau
Add qemu_memfd_alloc/free() helpers.
The function helps to allocate and seal shared memory.
Signed-off-by: Marc-André Lureau
---
include/qemu/memfd.h | 4 +++
util/memfd.c | 75 ++--
2 files changed, 77 insertions(+), 2 deletions(-)
diff --git a/include/qemu/memfd.h b/include/qemu/memfd.h
index 8b1fe6a..950fb88 100644
--- a/include/qemu/memfd.h
+++ b/include/qemu/memfd.h
@@ -17,4 +17,8 @@
#define F_SEAL_WRITE0x0008 /* prevent writes */
#endif
+void *qemu_memfd_alloc(const char *name, size_t size, unsigned int seals,
+ int *fd);
+void qemu_memfd_free(void *ptr, size_t size, int fd);
+
#endif /* QEMU_MEMFD_H */
diff --git a/util/memfd.c b/util/memfd.c
index a98d57e..dd47552 100644
--- a/util/memfd.c
+++ b/util/memfd.c
@@ -27,6 +27,14 @@
#include "config-host.h"
+#include
+#include
+
+#include
+#include
+#include
+#include
+
#include "qemu/memfd.h"
#ifdef CONFIG_MEMFD
@@ -44,13 +52,76 @@
#define MFD_ALLOW_SEALING 0x0002U
#endif
-static inline int memfd_create(const char *name, unsigned int flags)
+static int memfd_create(const char *name, unsigned int flags)
{
return syscall(__NR_memfd_create, name, flags);
}
#else /* !LINUX */
-static inline int memfd_create(const char *name, unsigned int flags)
+static int memfd_create(const char *name, unsigned int flags)
{
return -1;
}
#endif
+
+/*
+ * This is a best-effort helper for shared memory allocation, with
+ * optional sealing. The helper will do his best to allocate using
+ * memfd with sealing, but may fallback on other methods without
+ * sealing.
+ */
+void *qemu_memfd_alloc(const char *name, size_t size, unsigned int seals,
+ int *fd)
+{
+void *ptr;
+int mfd = -1;
+
+*fd = -1;
+
+if (seals) {
+mfd = memfd_create(name, MFD_ALLOW_SEALING | MFD_CLOEXEC);
+}
+
+if (mfd == -1) {
+/* some systems have memfd without sealing */
+mfd = memfd_create(name, MFD_CLOEXEC);
+seals = 0;
+}
+
+if (mfd != -1) {
+if (ftruncate(mfd, size) == -1) {
+perror("ftruncate");
+close(mfd);
+return NULL;
+}
+
+if (seals && fcntl(mfd, F_ADD_SEALS, seals) == -1) {
+perror("fcntl");
+close(mfd);
+return NULL;
+}
+} else {
+perror("memfd");
+return NULL;
+}
+
+ptr = mmap(0, size, PROT_READ | PROT_WRITE, MAP_SHARED, mfd, 0);
+if (ptr == MAP_FAILED) {
+perror("mmap");
+close(mfd);
+return NULL;
+}
+
+*fd = mfd;
+return ptr;
+}
+
+void qemu_memfd_free(void *ptr, size_t size, int fd)
+{
+if (ptr) {
+munmap(ptr, size);
+}
+
+if (fd != -1) {
+close(fd);
+}
+}
--
2.4.3
Re: [Qemu-devel] [PATCH 1/2] target-i386: Use 1UL for bit shift
On 01/10/2015 19:07, Laszlo Ersek wrote: > > In addition, C89 didn't say at all what the result was for signed data > > types, so technically we could compile QEMU with -std=gnu89 (the default > > until GCC5) and call it a day. > > > > Really the C standard should make this implementation-defined. > > Obligatory link: http://blog.regehr.org/archives/1180 Many ideas in there are good (e.g. mem*() being defined for invalid argument and zero lengths, and of course item 7 which is the issue at hand). In many cases it's also good to change undefined behavior to unspecified values, however I think that goes too far. For example I'm okay with signed integer overflow being undefined behavior, and I also disagree with "It is permissible to compute out-of-bounds pointer values including performing pointer arithmetic on the null pointer". Using uintptr_t is just fine. Also strict aliasing improves performance noticeably at least on some kind of code. The relaxation of strict aliasing that GCC does with unions would be a useful addition to the C standard, though. Paolo
[Qemu-devel] [PATCH v7 11/24] vhost-user: add a migration blocker
From: Marc-André Lureau
If VHOST_USER_PROTOCOL_F_LOG_SHMFD is not announced, block vhost-user
migration. The blocker is removed in vhost_dev_cleanup().
Signed-off-by: Marc-André Lureau
---
hw/virtio/vhost-user.c | 9 +
hw/virtio/vhost.c | 16
2 files changed, 21 insertions(+), 4 deletions(-)
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index cf14e38..f1edd04 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -15,6 +15,7 @@
#include "qemu/error-report.h"
#include "qemu/sockets.h"
#include "exec/ram_addr.h"
+#include "migration/migration.h"
#include
#include
@@ -442,6 +443,14 @@ static int vhost_user_init(struct vhost_dev *dev, void
*opaque)
}
}
+if (dev->migration_blocker == NULL &&
+!virtio_has_feature(dev->protocol_features,
+VHOST_USER_PROTOCOL_F_LOG_SHMFD)) {
+error_setg(&dev->migration_blocker,
+ "Migration disabled: vhost-user backend lacks "
+ "VHOST_USER_PROTOCOL_F_LOG_SHMFD feature.");
+}
+
return 0;
}
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index d1c0367..554e49d 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -944,6 +944,8 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
uint64_t features;
int i, r;
+hdev->migration_blocker = NULL;
+
if (vhost_set_backend_type(hdev, backend_type) < 0) {
close((uintptr_t)opaque);
return -1;
@@ -987,12 +989,18 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
.eventfd_del = vhost_eventfd_del,
.priority = 10
};
-hdev->migration_blocker = NULL;
-if (!(hdev->features & (0x1ULL << VHOST_F_LOG_ALL))) {
-error_setg(&hdev->migration_blocker,
- "Migration disabled: vhost lacks VHOST_F_LOG_ALL feature.");
+
+if (hdev->migration_blocker == NULL) {
+if (!(hdev->features & (0x1ULL << VHOST_F_LOG_ALL))) {
+error_setg(&hdev->migration_blocker,
+ "Migration disabled: vhost lacks VHOST_F_LOG_ALL
feature.");
+}
+}
+
+if (hdev->migration_blocker != NULL) {
migrate_add_blocker(hdev->migration_blocker);
}
+
hdev->mem = g_malloc0(offsetof(struct vhost_memory, regions));
hdev->n_mem_sections = 0;
hdev->mem_sections = NULL;
--
2.4.3
[Qemu-devel] [PATCH v7 01/24] configure: probe for memfd
From: Marc-André Lureau
Check if memfd_create() is part of system libc.
Signed-off-by: Marc-André Lureau
---
configure | 19 +++
1 file changed, 19 insertions(+)
diff --git a/configure b/configure
index f14454e..5e77ed5 100755
--- a/configure
+++ b/configure
@@ -3486,6 +3486,22 @@ if compile_prog "" "" ; then
eventfd=yes
fi
+# check if memfd is supported
+memfd=no
+cat > $TMPC << EOF
+#include
+
+int main(void)
+{
+return memfd_create("foo", MFD_ALLOW_SEALING);
+}
+EOF
+if compile_prog "" "" ; then
+ memfd=yes
+fi
+
+
+
# check for fallocate
fallocate=no
cat > $TMPC << EOF
@@ -4857,6 +4873,9 @@ fi
if test "$eventfd" = "yes" ; then
echo "CONFIG_EVENTFD=y" >> $config_host_mak
fi
+if test "$memfd" = "yes" ; then
+ echo "CONFIG_MEMFD=y" >> $config_host_mak
+fi
if test "$fallocate" = "yes" ; then
echo "CONFIG_FALLOCATE=y" >> $config_host_mak
fi
--
2.4.3
[Qemu-devel] [PATCH v7 10/24] vhost-user: send log shm fd along with log_base
From: Marc-André Lureau
Send the shm for the dirty pages logging if the backend supports
VHOST_USER_PROTOCOL_F_LOG_SHMFD. Wait for a reply to make sure
the old log is no longer used.
Signed-off-by: Marc-André Lureau
---
hw/virtio/vhost-backend.c | 3 ++-
hw/virtio/vhost-user.c| 27 +--
hw/virtio/vhost.c | 5 +++--
include/hw/virtio/vhost-backend.h | 4 +++-
4 files changed, 33 insertions(+), 6 deletions(-)
diff --git a/hw/virtio/vhost-backend.c b/hw/virtio/vhost-backend.c
index 88d33fd..87ab028 100644
--- a/hw/virtio/vhost-backend.c
+++ b/hw/virtio/vhost-backend.c
@@ -50,7 +50,8 @@ static int vhost_kernel_get_vq_index(struct vhost_dev *dev,
int idx)
return idx - dev->vq_index;
}
-static int vhost_set_log_base(struct vhost_dev *dev, uint64_t base)
+static int vhost_set_log_base(struct vhost_dev *dev, uint64_t base,
+ struct vhost_log *log)
{
return vhost_kernel_call(dev, VHOST_SET_LOG_BASE, &base);
}
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 01e51a0..cf14e38 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -367,8 +367,13 @@ static int vhost_user_call(struct vhost_dev *dev, unsigned
long int request,
return 0;
}
-static int vhost_set_log_base(struct vhost_dev *dev, uint64_t base)
+static int vhost_set_log_base(struct vhost_dev *dev, uint64_t base,
+ struct vhost_log *log)
{
+int fds[VHOST_MEMORY_MAX_NREGIONS];
+size_t fd_num = 0;
+bool shmfd = virtio_has_feature(dev->protocol_features,
+VHOST_USER_PROTOCOL_F_LOG_SHMFD);
VhostUserMsg msg = {
.request = VHOST_USER_SET_LOG_BASE,
.flags = VHOST_USER_VERSION,
@@ -376,7 +381,25 @@ static int vhost_set_log_base(struct vhost_dev *dev,
uint64_t base)
.size = sizeof(m.u64),
};
-vhost_user_write(dev, &msg, NULL, 0);
+if (shmfd && log->fd != -1) {
+fds[fd_num++] = log->fd;
+}
+
+vhost_user_write(dev, &msg, fds, fd_num);
+
+if (shmfd) {
+msg.size = 0;
+if (vhost_user_read(dev, &msg) < 0) {
+return 0;
+}
+
+if (msg.request != VHOST_USER_SET_LOG_BASE) {
+error_report("Received unexpected msg type. "
+ "Expected %d received %d",
+ VHOST_USER_SET_LOG_BASE, msg.request);
+return -1;
+}
+}
return 0;
}
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 63fa2bf..d1c0367 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -372,7 +372,7 @@ static inline void vhost_dev_log_resize(struct vhost_dev
*dev, uint64_t size)
/* inform backend of log switching, this must be done before
releasing the current log, to ensure no logging is lost */
-r = dev->vhost_ops->vhost_set_log_base(dev, log_base);
+r = dev->vhost_ops->vhost_set_log_base(dev, log_base, log);
assert(r >= 0);
vhost_log_put(dev, true);
dev->log = log;
@@ -1178,7 +1178,8 @@ int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice
*vdev)
vhost_dev_log_is_shared(hdev));
log_base = (uintptr_t)hdev->log->log;
r = hdev->vhost_ops->vhost_set_log_base(hdev,
-hdev->log_size ? log_base : 0);
+hdev->log_size ? log_base : 0,
+hdev->log);
if (r < 0) {
r = -errno;
goto fail_log;
diff --git a/include/hw/virtio/vhost-backend.h
b/include/hw/virtio/vhost-backend.h
index 649766a..7064a12 100644
--- a/include/hw/virtio/vhost-backend.h
+++ b/include/hw/virtio/vhost-backend.h
@@ -21,6 +21,7 @@ typedef enum VhostBackendType {
} VhostBackendType;
struct vhost_dev;
+struct vhost_log;
typedef int (*vhost_call)(struct vhost_dev *dev, unsigned long int request,
void *arg);
@@ -29,7 +30,8 @@ typedef int (*vhost_backend_cleanup)(struct vhost_dev *dev);
typedef int (*vhost_backend_get_vq_index)(struct vhost_dev *dev, int idx);
typedef int (*vhost_backend_set_vring_enable)(struct vhost_dev *dev, int
enable);
-typedef int (*vhost_set_log_base_op)(struct vhost_dev *dev, uint64_t base);
+typedef int (*vhost_set_log_base_op)(struct vhost_dev *dev, uint64_t base,
+ struct vhost_log *log);
typedef bool (*vhost_requires_shm_log_op)(struct vhost_dev *dev);
typedef struct VhostOps {
--
2.4.3
[Qemu-devel] [PATCH v7 22/24] vhost-user-test: learn to tweak various qemu arguments
From: Marc-André Lureau
Add a new macro to make the qemu command line with other
values of memory size, and specific chardev id.
Signed-off-by: Marc-André Lureau
---
tests/vhost-user-test.c | 25 +++--
1 file changed, 15 insertions(+), 10 deletions(-)
diff --git a/tests/vhost-user-test.c b/tests/vhost-user-test.c
index 034d89b..791d849 100644
--- a/tests/vhost-user-test.c
+++ b/tests/vhost-user-test.c
@@ -30,10 +30,10 @@
#endif
#define QEMU_CMD_ACCEL " -machine accel=tcg"
-#define QEMU_CMD_MEM" -m 512 -object
memory-backend-file,id=mem,size=512M,"\
+#define QEMU_CMD_MEM" -m %d -object memory-backend-file,id=mem,size=%dM,"\
"mem-path=%s,share=on -numa node,memdev=mem"
-#define QEMU_CMD_CHR" -chardev socket,id=chr0,path=%s"
-#define QEMU_CMD_NETDEV " -netdev vhost-user,id=net0,chardev=chr0,vhostforce"
+#define QEMU_CMD_CHR" -chardev socket,id=%s,path=%s"
+#define QEMU_CMD_NETDEV " -netdev vhost-user,id=net0,chardev=%s,vhostforce"
#define QEMU_CMD_NET" -device virtio-net-pci,netdev=net0 "
#define QEMU_CMD_ROM" -option-rom ../pc-bios/pxe-virtio.rom"
@@ -132,6 +132,9 @@ static gboolean g_cond_wait_until(CompatGCond cond,
CompatGMutex mutex,
}
#endif
+static const char *tmpfs;
+static const char *root;
+
static void wait_for_fds(TestServer *s)
{
gint64 end_time;
@@ -317,7 +320,7 @@ static const char *init_hugepagefs(const char *path)
return path;
}
-static TestServer *test_server_new(const gchar *tmpfs, const gchar *name)
+static TestServer *test_server_new(const gchar *name)
{
TestServer *server = g_new0(TestServer, 1);
gchar *chr_path;
@@ -337,9 +340,13 @@ static TestServer *test_server_new(const gchar *tmpfs,
const gchar *name)
return server;
}
-#define GET_QEMU_CMD(s, root)\
-g_strdup_printf(QEMU_CMD, (root), (s)->socket_path)
+#define GET_QEMU_CMD(s)
\
+g_strdup_printf(QEMU_CMD, 512, 512, (root), (s)->chr_name,
\
+(s)->socket_path, (s)->chr_name)
+#define GET_QEMU_CMDE(s, mem, extra, ...)
\
+g_strdup_printf(QEMU_CMD extra, (mem), (mem), (root), (s)->chr_name,
\
+(s)->socket_path, (s)->chr_name, ##__VA_ARGS__)
static void test_server_free(TestServer *server)
{
@@ -365,8 +372,6 @@ int main(int argc, char **argv)
char *qemu_cmd = NULL;
int ret;
char template[] = "/tmp/vhost-test-XX";
-const char *tmpfs;
-const char *root;
g_test_init(&argc, &argv, NULL);
@@ -387,12 +392,12 @@ int main(int argc, char **argv)
root = tmpfs;
}
-server = test_server_new(tmpfs, "test");
+server = test_server_new("test");
/* run the main loop thread so the chardev may operate */
g_thread_new(NULL, thread_function, NULL);
-qemu_cmd = GET_QEMU_CMD(server, root);
+qemu_cmd = GET_QEMU_CMD(server);
s = qtest_start(qemu_cmd);
g_free(qemu_cmd);
--
2.4.3
