Re: [Qemu-devel] [PATCH] target-i386: Preserve the Z bit for bt/bts/btr/btc
Il 09/04/2014 16:56, Richard Henderson ha scritto:
Older Intel manuals (pre-2010) describe Z as undefined, but AMD and
newer Intel manuals describe Z as unchanged.
Signed-off-by: Richard Henderson
---
target-i386/translate.c | 40 +++-
1 file changed, 31 insertions(+), 9 deletions(-)
---
Clemens, your patch fails to fix flags computation for bts/btr/btc,
which should be done similarly to bt.
And to answer your question, no, QEMU does not make any assumptions
about undefined flags. We often set them to zero, just because that
is easier than any other setting.
r~
diff --git a/target-i386/translate.c b/target-i386/translate.c
index 02625e3..032b0fd 100644
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -6708,41 +6708,63 @@ static target_ulong disas_insn(CPUX86State *env,
DisasContext *s,
}
bt_op:
tcg_gen_andi_tl(cpu_T[1], cpu_T[1], (1 << (3 + ot)) - 1);
+tcg_gen_shr_tl(cpu_tmp4, cpu_T[0], cpu_T[1]);
switch(op) {
case 0:
-tcg_gen_shr_tl(cpu_cc_src, cpu_T[0], cpu_T[1]);
-tcg_gen_movi_tl(cpu_cc_dst, 0);
break;
case 1:
-tcg_gen_shr_tl(cpu_tmp4, cpu_T[0], cpu_T[1]);
tcg_gen_movi_tl(cpu_tmp0, 1);
tcg_gen_shl_tl(cpu_tmp0, cpu_tmp0, cpu_T[1]);
tcg_gen_or_tl(cpu_T[0], cpu_T[0], cpu_tmp0);
break;
case 2:
-tcg_gen_shr_tl(cpu_tmp4, cpu_T[0], cpu_T[1]);
tcg_gen_movi_tl(cpu_tmp0, 1);
tcg_gen_shl_tl(cpu_tmp0, cpu_tmp0, cpu_T[1]);
-tcg_gen_not_tl(cpu_tmp0, cpu_tmp0);
-tcg_gen_and_tl(cpu_T[0], cpu_T[0], cpu_tmp0);
+tcg_gen_andc_tl(cpu_T[0], cpu_T[0], cpu_tmp0);
break;
default:
case 3:
-tcg_gen_shr_tl(cpu_tmp4, cpu_T[0], cpu_T[1]);
tcg_gen_movi_tl(cpu_tmp0, 1);
tcg_gen_shl_tl(cpu_tmp0, cpu_tmp0, cpu_T[1]);
tcg_gen_xor_tl(cpu_T[0], cpu_T[0], cpu_tmp0);
break;
}
-set_cc_op(s, CC_OP_SARB + ot);
if (op != 0) {
if (mod != 3) {
gen_op_st_v(s, ot, cpu_T[0], cpu_A0);
} else {
gen_op_mov_reg_v(ot, rm, cpu_T[0]);
}
+}
+
+/* Delay all CC updates until after the store above. Note that
+ C is the result of the test, Z is unchanged, and the others
+ are all undefined. */
+switch (s->cc_op) {
+case CC_OP_MULB ... CC_OP_MULQ:
+case CC_OP_ADDB ... CC_OP_ADDQ:
+case CC_OP_ADCB ... CC_OP_ADCQ:
+case CC_OP_SUBB ... CC_OP_SUBQ:
+case CC_OP_SBBB ... CC_OP_SBBQ:
+case CC_OP_LOGICB ... CC_OP_LOGICQ:
+case CC_OP_INCB ... CC_OP_INCQ:
+case CC_OP_DECB ... CC_OP_DECQ:
+case CC_OP_SHLB ... CC_OP_SHLQ:
+case CC_OP_SARB ... CC_OP_SARQ:
+case CC_OP_BMILGB ... CC_OP_BMILGQ:
+/* Z was going to be computed from the non-zero status of CC_DST.
+ We can get that same Z value (and the new C value) by leaving
+ CC_DST alone, setting CC_SRC, and using a CC_OP_SAR of the
+ same width. */
tcg_gen_mov_tl(cpu_cc_src, cpu_tmp4);
-tcg_gen_movi_tl(cpu_cc_dst, 0);
+set_cc_op(s, ((s->cc_op - CC_OP_MULB) & 3) + CC_OP_SARB);
+break;
+default:
+/* Otherwise, generate EFLAGS and replace the C bit. */
+gen_compute_eflags(s);
+tcg_gen_deposit_tl(cpu_cc_src, cpu_cc_src, cpu_tmp4,
+ ctz32(CC_C), 1);
+break;
}
break;
case 0x1bc: /* bsf / tzcnt */
Cc: qemu-sta...@nongnu.org
Reviewed-by: Paolo Bonzini
Hmm, actually the comments aren't following the common style of
asterisks-on-every-line. Just fix it up without posting v2.
Paolo
Re: [Qemu-devel] TCG x86-64 'bt' insn
On 19 April 2014 23:41, Clemens Kolbitsch wrote: > Thanks guys, awesome feedback and glad to see it was picked up in QEMU 2.0 > :) This didn't go into 2.0 -- it arrived somewhat late for that. It'll get into 2.1 (and perhaps 2.0.1, if anybody cares enough to cc stable on it). thanks -- PMM
Re: [Qemu-devel] TCG x86-64 'bt' insn
Thanks guys, awesome feedback and glad to see it was picked up in QEMU 2.0 :) On Sat, Apr 19, 2014 at 3:21 PM, Peter Maydell wrote: > On 19 April 2014 23:04, Paolo Bonzini wrote: > > The new code should apply to btc/btr/bts too. > > ...see also RTH's patch: > > https://lists.gnu.org/archive/html/qemu-devel/2014-04/msg01455.html > > thanks > -- PMM > -- Clemens Kolbitsch Security Researcher kolbit...@lastline.com Mobile +1 (206) 356-7745 Land +1 (805) 456-7076 Lastline, Inc. 6950 Hollister Avenue, Suite 101 Goleta, CA 93117 www.lastline.com
Re: [Qemu-devel] [PATCH] target-i386: Preserve the Z bit for bt/bts/btr/btc
Great, thanks for the feedback and the fix Richard!
On Wed, Apr 9, 2014 at 1:56 PM, Richard Henderson wrote:
> Older Intel manuals (pre-2010) describe Z as undefined, but AMD and
> newer Intel manuals describe Z as unchanged.
>
> Signed-off-by: Richard Henderson
> ---
> target-i386/translate.c | 40 +++-
> 1 file changed, 31 insertions(+), 9 deletions(-)
>
> ---
> Clemens, your patch fails to fix flags computation for bts/btr/btc,
> which should be done similarly to bt.
>
> And to answer your question, no, QEMU does not make any assumptions
> about undefined flags. We often set them to zero, just because that
> is easier than any other setting.
>
>
> r~
>
>
>
> diff --git a/target-i386/translate.c b/target-i386/translate.c
> index 02625e3..032b0fd 100644
> --- a/target-i386/translate.c
> +++ b/target-i386/translate.c
> @@ -6708,41 +6708,63 @@ static target_ulong disas_insn(CPUX86State *env,
> DisasContext *s,
> }
> bt_op:
> tcg_gen_andi_tl(cpu_T[1], cpu_T[1], (1 << (3 + ot)) - 1);
> +tcg_gen_shr_tl(cpu_tmp4, cpu_T[0], cpu_T[1]);
> switch(op) {
> case 0:
> -tcg_gen_shr_tl(cpu_cc_src, cpu_T[0], cpu_T[1]);
> -tcg_gen_movi_tl(cpu_cc_dst, 0);
> break;
> case 1:
> -tcg_gen_shr_tl(cpu_tmp4, cpu_T[0], cpu_T[1]);
> tcg_gen_movi_tl(cpu_tmp0, 1);
> tcg_gen_shl_tl(cpu_tmp0, cpu_tmp0, cpu_T[1]);
> tcg_gen_or_tl(cpu_T[0], cpu_T[0], cpu_tmp0);
> break;
> case 2:
> -tcg_gen_shr_tl(cpu_tmp4, cpu_T[0], cpu_T[1]);
> tcg_gen_movi_tl(cpu_tmp0, 1);
> tcg_gen_shl_tl(cpu_tmp0, cpu_tmp0, cpu_T[1]);
> -tcg_gen_not_tl(cpu_tmp0, cpu_tmp0);
> -tcg_gen_and_tl(cpu_T[0], cpu_T[0], cpu_tmp0);
> +tcg_gen_andc_tl(cpu_T[0], cpu_T[0], cpu_tmp0);
> break;
> default:
> case 3:
> -tcg_gen_shr_tl(cpu_tmp4, cpu_T[0], cpu_T[1]);
> tcg_gen_movi_tl(cpu_tmp0, 1);
> tcg_gen_shl_tl(cpu_tmp0, cpu_tmp0, cpu_T[1]);
> tcg_gen_xor_tl(cpu_T[0], cpu_T[0], cpu_tmp0);
> break;
> }
> -set_cc_op(s, CC_OP_SARB + ot);
> if (op != 0) {
> if (mod != 3) {
> gen_op_st_v(s, ot, cpu_T[0], cpu_A0);
> } else {
> gen_op_mov_reg_v(ot, rm, cpu_T[0]);
> }
> +}
> +
> +/* Delay all CC updates until after the store above. Note that
> + C is the result of the test, Z is unchanged, and the others
> + are all undefined. */
> +switch (s->cc_op) {
> +case CC_OP_MULB ... CC_OP_MULQ:
> +case CC_OP_ADDB ... CC_OP_ADDQ:
> +case CC_OP_ADCB ... CC_OP_ADCQ:
> +case CC_OP_SUBB ... CC_OP_SUBQ:
> +case CC_OP_SBBB ... CC_OP_SBBQ:
> +case CC_OP_LOGICB ... CC_OP_LOGICQ:
> +case CC_OP_INCB ... CC_OP_INCQ:
> +case CC_OP_DECB ... CC_OP_DECQ:
> +case CC_OP_SHLB ... CC_OP_SHLQ:
> +case CC_OP_SARB ... CC_OP_SARQ:
> +case CC_OP_BMILGB ... CC_OP_BMILGQ:
> +/* Z was going to be computed from the non-zero status of
> CC_DST.
> + We can get that same Z value (and the new C value) by
> leaving
> + CC_DST alone, setting CC_SRC, and using a CC_OP_SAR of the
> + same width. */
> tcg_gen_mov_tl(cpu_cc_src, cpu_tmp4);
> -tcg_gen_movi_tl(cpu_cc_dst, 0);
> +set_cc_op(s, ((s->cc_op - CC_OP_MULB) & 3) + CC_OP_SARB);
> +break;
> +default:
> +/* Otherwise, generate EFLAGS and replace the C bit. */
> +gen_compute_eflags(s);
> +tcg_gen_deposit_tl(cpu_cc_src, cpu_cc_src, cpu_tmp4,
> + ctz32(CC_C), 1);
> +break;
> }
> break;
> case 0x1bc: /* bsf / tzcnt */
> --
> 1.9.0
>
>
--
Clemens Kolbitsch
Security Researcher
kolbit...@lastline.com
Mobile +1 (206) 356-7745
Land +1 (805) 456-7076
Lastline, Inc.
6950 Hollister Avenue, Suite 101
Goleta, CA 93117
www.lastline.com
Re: [Qemu-devel] TCG x86-64 'bt' insn
On 19 April 2014 23:04, Paolo Bonzini wrote: > The new code should apply to btc/btr/bts too. ...see also RTH's patch: https://lists.gnu.org/archive/html/qemu-devel/2014-04/msg01455.html thanks -- PMM
Re: [Qemu-devel] TCG x86-64 'bt' insn
Il 09/04/2014 13:55, Clemens Kolbitsch ha scritto:
Hi,
quick follow-up. *As always* you find a problem right after asking for
help :). The updated patch does not cause BSOD on Windows guests, but
neither does it fix the actual problem (of the program seg-faulting)
I would really appreciate any feedback on the proposed patch below - the
difference to the previous patch is that we clear out undefined flags
and only keep the Z-flag (and update the C-flag)
The new code should apply to btc/btr/bts too. Basically the "switch"
statement keeps just the shr, and everything else moves below.
Or even simpler, you could exploit the undefinedness of non-CF, non-ZF
flags and keep using CC_OP_SARB. Because CC_OP_SARB uses "cc_dst == 0"
to compute ZF, if ZF=1 you want to set it to 0, and if ZF=0 you want to
set it to non-zero. So just compute !ZF into CC_DST, and place the
tested bit (CF) into CC_SRC. Basically:
gen_setcc1(s, (JCC_Z << 1) | 1, cpu_cc_dst);
switch(op) {
...
}
tcg_gen_andi_tl(cpu_cc_src, cpu_tmp4, CC_C);
set_cc_op(s, CC_OP_SARB + ot);
Last another general question: Does TCG make any assumptions that
undefined flags are set to 0? I see that most flag-computations set
undefined flags to 0 - is this just a convention or really a requirement?
Just a convention.
Paolo
Thanks guys!
-Clemens
On Wed, Apr 9, 2014 at 10:33 AM, Clemens Kolbitsch
mailto:kolbit...@lastline.com>> wrote:
Hi guys,
I have to revive a rather old thread [1,2]. A quick summary of the
issue:
TCG emulates the BT (bit-test) instruction using a SAR to re-compute
eflags. While SAR affects all flags, BT only affects the C-flag and
leaves the Z-flag unaffected/unchanged [3].
According to the x86 manual, the current behavior is correct
(although not perfect performance-wise), but NOT correct according
to newer Intel CPU instruction sets (such as x86-64). What has
caused some confusion in the past it seems is that AMD's manual
mentions that all flags other than the C-flag are "undefined" (just
like in Intel's old x86 manual).
A patch has been suggested (although not applied) before [2] and I
was trying to update and try the patch to the QEMU 2.0 RC2, but it
seems not to be working (causes a BSOD on Windows guests).
BTW: I have a program that seg-faults on Windows clients (32 as well
as 64 bit guest OSes) in vanilla QEMU 2.0 RC2 (just like in previous
versions), so this is not just of theoretical importance :)
Can somebody say if there is something that I'm doing obviously wrong?
*NON-FUNCTIONAL PATCH!*
--- ../orig/qemu-2.0.0-rc2/target-i386/translate.c 2014-04-08
12:38:58.0 -0700
+++ target-i386/translate.c 2014-04-09 10:08:43.252084947 -0700
@@ -6710,8 +6710,14 @@
tcg_gen_andi_tl(cpu_T[1], cpu_T[1], (1 << (3 + ot)) - 1);
switch(op) {
case 0:
-tcg_gen_shr_tl(cpu_cc_src, cpu_T[0], cpu_T[1]);
-tcg_gen_movi_tl(cpu_cc_dst, 0);
+/* whatever the last CC-op is - recompute now so we can
OR-in
+ * updated results */
+gen_update_cc_op(s);
+gen_compute_eflags(s);
+tcg_gen_shr_tl(cpu_tmp4, cpu_T[0], cpu_T[1]);
+tcg_gen_andi_tl(cpu_tmp4, cpu_tmp4, CC_C);
+tcg_gen_or_tl(cpu_cc_src, cpu_cc_src, cpu_tmp4);
+set_cc_op(s, CC_OP_EFLAGS);
break;
case 1:
tcg_gen_shr_tl(cpu_tmp4, cpu_T[0], cpu_T[1]);
@@ -6734,8 +6740,8 @@
tcg_gen_xor_tl(cpu_T[0], cpu_T[0], cpu_tmp0);
break;
}
-set_cc_op(s, CC_OP_SARB + ot);
if (op != 0) {
+set_cc_op(s, CC_OP_SARB + ot);
if (mod != 3) {
gen_op_st_v(s, ot, cpu_T[0], cpu_A0);
} else {
Thanks, I'd really appreciate any input you can provide.
-Clemens
[1] http://lists.gnu.org/archive/html/qemu-devel/2010-11/msg00442.html
[2] http://lists.gnu.org/archive/html/qemu-devel/2010-11/msg00764.html
[3] page
148
http://www.intel.com/content/dam/www/public/us/en/documents/manuals/64-ia-32-architectures-software-developer-instruction-set-reference-manual-325383.pdf
Re: [Qemu-devel] [PATCH] qapi: generate space in c_type() to fix coding style
On 04/18/2014 09:56 PM, Amos Kong wrote:
> Currently we always add a space after c_type in mcgen(), there is
> some redundant space in generated code. The space isn't needed for
> points by the coding style.
Second sentence is awkward; maybe:
Avoiding the space when appropriate makes us match the coding style.
>
> char * value;
> ^
> qapi_free_NameInfo(NameInfo * obj)
>^
> It's fussy to add checking in each mcgen(), this patch just addes
s/addes/adds/
> the necessary space in c_type(), and remove original space in mcgen().
s/remove/removes the/
>
> Signed-off-by: Amos Kong
> ---
> scripts/qapi-types.py | 10 +-
> scripts/qapi-visit.py | 20 ++--
> scripts/qapi.py | 14 +++---
> 3 files changed, 22 insertions(+), 22 deletions(-)
> union {
> -%(type)s value;
> +%(type)svalue;
Code itself does what it claims. It's a bit harder to read the
generator without the space, and I might have added a comment to
c_type() explaining that the output string includes the space except for
pointer types. I'll let others decide whether to take the patch, but
I'm comfortable if the commit message is fixed and you want to add:
Reviewed-by: Eric Blake
--
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] qapi: generate space in c_type() to fix coding style
Il 18/04/2014 23:56, Amos Kong ha scritto:
Currently we always add a space after c_type in mcgen(), there is
some redundant space in generated code. The space isn't needed for
points by the coding style.
char * value;
^
qapi_free_NameInfo(NameInfo * obj)
^
It's fussy to add checking in each mcgen(), this patch just addes
the necessary space in c_type(), and remove original space in mcgen().
It also makes the generator harder to read to though, and the
improvement in the generated code is minor enough that I think the
change is overall negative. But I won't oppose the patch if the
maintainers are fine with it.
Paolo
Signed-off-by: Amos Kong
---
scripts/qapi-types.py | 10 +-
scripts/qapi-visit.py | 20 ++--
scripts/qapi.py | 14 +++---
3 files changed, 22 insertions(+), 22 deletions(-)
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 10864ef..54a9b89 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -23,7 +23,7 @@ def generate_fwd_struct(name, members, builtin_type=False):
typedef struct %(name)sList
{
union {
-%(type)s value;
+%(type)svalue;
uint64_t padding;
};
struct %(name)sList *next;
@@ -75,7 +75,7 @@ def generate_struct_fields(members):
pop_indent()
else:
ret += mcgen('''
-%(c_type)s %(c_name)s;
+%(c_type)s%(c_name)s;
''',
c_type=c_type(argentry), c_name=c_var(argname))
@@ -219,7 +219,7 @@ struct %(name)s
for key in typeinfo:
ret += mcgen('''
-%(c_type)s %(c_name)s;
+%(c_type)s%(c_name)s;
''',
c_type=c_type(typeinfo[key]),
c_name=c_fun(key))
@@ -251,7 +251,7 @@ extern const int %(name)s_qtypes[];
def generate_type_cleanup_decl(name):
ret = mcgen('''
-void qapi_free_%(type)s(%(c_type)s obj);
+void qapi_free_%(type)s(%(c_type)sobj);
''',
c_type=c_type(name),type=name)
return ret
@@ -259,7 +259,7 @@ void qapi_free_%(type)s(%(c_type)s obj);
def generate_type_cleanup(name):
ret = mcgen('''
-void qapi_free_%(type)s(%(c_type)s obj)
+void qapi_free_%(type)s(%(c_type)sobj)
{
QapiDeallocVisitor *md;
Visitor *v;
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 45ce3a9..8ffed3d 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -38,7 +38,7 @@ def generate_visit_struct_fields(name, field_prefix,
fn_prefix, members, base =
ret += mcgen('''
-static void visit_type_%(full_name)s_fields(Visitor *m, %(name)s ** obj, Error
**errp)
+static void visit_type_%(full_name)s_fields(Visitor *m, %(name)s **obj, Error
**errp)
{
Error *err = NULL;
''',
@@ -149,7 +149,7 @@ def generate_visit_struct(expr):
ret += mcgen('''
-void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error
**errp)
+void visit_type_%(name)s(Visitor *m, %(name)s **obj, const char *name, Error
**errp)
{
''',
name=name)
@@ -166,7 +166,7 @@ void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const
char *name, Error **
def generate_visit_list(name, members):
return mcgen('''
-void visit_type_%(name)sList(Visitor *m, %(name)sList ** obj, const char
*name, Error **errp)
+void visit_type_%(name)sList(Visitor *m, %(name)sList **obj, const char *name,
Error **errp)
{
GenericList *i, **prev = (GenericList **)obj;
Error *err = NULL;
@@ -193,7 +193,7 @@ void visit_type_%(name)sList(Visitor *m, %(name)sList **
obj, const char *name,
def generate_visit_enum(name, members):
return mcgen('''
-void visit_type_%(name)s(Visitor *m, %(name)s * obj, const char *name, Error
**errp)
+void visit_type_%(name)s(Visitor *m, %(name)s *obj, const char *name, Error
**errp)
{
visit_type_enum(m, (int *)obj, %(name)s_lookup, "%(name)s", name, errp);
}
@@ -203,7 +203,7 @@ void visit_type_%(name)s(Visitor *m, %(name)s * obj, const
char *name, Error **e
def generate_visit_anon_union(name, members):
ret = mcgen('''
-void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error
**errp)
+void visit_type_%(name)s(Visitor *m, %(name)s **obj, const char *name, Error
**errp)
{
Error *err = NULL;
@@ -279,7 +279,7 @@ def generate_visit_union(expr):
ret += mcgen('''
-void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error
**errp)
+void visit_type_%(name)s(Visitor *m, %(name)s **obj, const char *name, Error
**errp)
{
Error *err = NULL;
@@ -367,13 +367,13 @@ def generate_declaration(name, members, genlist=True,
builtin_type=False):
if not builtin_type:
ret += mcgen('''
-void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error
**errp);
+void visit_type_%(name)s(Visitor *m, %(name)s **obj, const char *name, Error
**errp);
''',
name=name)
if genlist:
ret += mcgen('''
-void vis
Re: [Qemu-devel] [PATCH] virtfs-proxy-helper: fix call to accept
Il 19/04/2014 13:39, Mike Frysinger ha scritto:
From: Tim Comer
The current code calls accept() without initializing the size parameter
which means the accept call might write too much to the stack.
URL: https://bugs.gentoo.org/486714
Signed-off-by: Tim Comer
Signed-off-by: Mike Frysinger
---
fsdev/virtfs-proxy-helper.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/fsdev/virtfs-proxy-helper.c b/fsdev/virtfs-proxy-helper.c
index bfecb87..cd291d3 100644
--- a/fsdev/virtfs-proxy-helper.c
+++ b/fsdev/virtfs-proxy-helper.c
@@ -760,6 +760,7 @@ static int proxy_socket(const char *path, uid_t uid, gid_t
gid)
return -1;
}
+size = sizeof(qemu);
client = accept(sock, (struct sockaddr *)&qemu, &size);
if (client < 0) {
do_perror("accept");
Reviewed-by: Paolo Bonzini
Re: [Qemu-devel] [PATCH v3 2/4] GlobalProperty: Display warning about unused -global
Il 18/04/2014 11:21, Andreas Färber ha scritto: Improving this is greatly appreciated, thanks. Now, I can see two ways things can go wrong: a) Mistyping or later refactoring devices, or b) user typos or thinkos. And four ways to set globals: -global, config file (I hope?), legacy command line options (vl.c) and machine .compat_props. If a property does not exist on the instance of an existing type, object_property_parse() will raise an Error and we will abort in device_post_init(). What we are silently missing is if a type is misspelled; for that we can probably add an Error **errp to the two qdev_prop_register_global*() functions - assuming QOM types are already registered at that point. qom-test would help us catch any such mistake by instantiating each machine. Even then, I suspect sooner or later machines other than PC and Q35 will be versioned. At that point we'll probably want QEMU-global compat_props that automatically apply to all machines, even if a type is not missing. I think we should already approximate this behavior by allowing machine .compat_props where the type doesn't exist. Paolo I note that your proposed check is not failing, but still, with hot-add of e.g. PCI devices we might well get a global property default for a type that is not instantiated immediately but possibly used later on.
Re: [Qemu-devel] [PATCH 4/4] PortioList: fix PortioList uses so they do not leak memory
Il 18/04/2014 12:36, Andreas Färber ha scritto:
Am 18.04.2014 15:41, schrieb Kirill Batuzov:
PortioList is an abstraction used for construction of MemoryRegionPortioList
from MemoryRegionPortio. It is not needed later, so there is no need to
allocate it dynamically. Also portio_list_destroy should be called to free
memory allocated in portio_list_init.
This change spans several target platforms. The following testcases cover all
changed lines:
qemu-system-ppc -M prep
qemu-system-i386 -vga qxl
qemu-system-i386 -M isapc -soundhw adlib -device ib700,id=watchdog0,bus=isa.0
Signed-off-by: Kirill Batuzov
Your changes look correct, but I'm not sure about our future plans with
this API. CC'ing some people to help review this.
Indeed, it looks strange to have a portio_list_destroy without a
portio_list_del before. You can add the PortioLists to AdlibState,
PCIQXLDevice etc. instead.
Paolo
Regards,
Andreas
---
hw/audio/adlib.c|7 ---
hw/display/qxl.c|9 +
hw/display/vga.c| 16 +---
hw/dma/i82374.c |7 ---
hw/isa/isa-bus.c|7 ---
hw/ppc/prep.c |7 ---
hw/watchdog/wdt_ib700.c |7 ---
7 files changed, 34 insertions(+), 26 deletions(-)
Coding style in adlib.c slightly differs from QEMU coding style.
Particularly there are spaces between function name and parenthesis. I decided
to not mix coding styles, but as a result checkpatch.pl complains on this
patch.
diff --git a/hw/audio/adlib.c b/hw/audio/adlib.c
index 28eed81..e43d990 100644
--- a/hw/audio/adlib.c
+++ b/hw/audio/adlib.c
@@ -293,7 +293,7 @@ static MemoryRegionPortio adlib_portio_list[] = {
static void adlib_realizefn (DeviceState *dev, Error **errp)
{
AdlibState *s = ADLIB(dev);
-PortioList *port_list = g_new(PortioList, 1);
+PortioList port_list;
struct audsettings as;
if (glob_adlib) {
@@ -349,8 +349,9 @@ static void adlib_realizefn (DeviceState *dev, Error **errp)
adlib_portio_list[0].offset = s->port;
adlib_portio_list[1].offset = s->port + 8;
-portio_list_init (port_list, OBJECT(s), adlib_portio_list, s, "adlib");
-portio_list_add (port_list, isa_address_space_io(&s->parent_obj), 0);
+portio_list_init (&port_list, OBJECT(s), adlib_portio_list, s, "adlib");
+portio_list_add (&port_list, isa_address_space_io(&s->parent_obj), 0);
+portio_list_destroy (&port_list);
}
static Property adlib_properties[] = {
diff --git a/hw/display/qxl.c b/hw/display/qxl.c
index 47bbf1f..7a361c7 100644
--- a/hw/display/qxl.c
+++ b/hw/display/qxl.c
@@ -2055,7 +2055,7 @@ static int qxl_init_primary(PCIDevice *dev)
{
PCIQXLDevice *qxl = DO_UPCAST(PCIQXLDevice, pci, dev);
VGACommonState *vga = &qxl->vga;
-PortioList *qxl_vga_port_list = g_new(PortioList, 1);
+PortioList qxl_vga_port_list;
int rc;
qxl->id = 0;
@@ -2064,10 +2064,11 @@ static int qxl_init_primary(PCIDevice *dev)
vga_common_init(vga, OBJECT(dev));
vga_init(vga, OBJECT(dev),
pci_address_space(dev), pci_address_space_io(dev), false);
-portio_list_init(qxl_vga_port_list, OBJECT(dev), qxl_vga_portio_list,
+portio_list_init(&qxl_vga_port_list, OBJECT(dev), qxl_vga_portio_list,
vga, "vga");
-portio_list_set_flush_coalesced(qxl_vga_port_list);
-portio_list_add(qxl_vga_port_list, pci_address_space_io(dev), 0x3b0);
+portio_list_set_flush_coalesced(&qxl_vga_port_list);
+portio_list_add(&qxl_vga_port_list, pci_address_space_io(dev), 0x3b0);
+portio_list_destroy(&qxl_vga_port_list);
vga->con = graphic_console_init(DEVICE(dev), 0, &qxl_ops, qxl);
qemu_spice_display_init_common(&qxl->ssd);
diff --git a/hw/display/vga.c b/hw/display/vga.c
index 063319d..72feafd 100644
--- a/hw/display/vga.c
+++ b/hw/display/vga.c
@@ -2351,8 +2351,8 @@ void vga_init(VGACommonState *s, Object *obj,
MemoryRegion *address_space,
{
MemoryRegion *vga_io_memory;
const MemoryRegionPortio *vga_ports, *vbe_ports;
-PortioList *vga_port_list = g_new(PortioList, 1);
-PortioList *vbe_port_list = g_new(PortioList, 1);
+PortioList vga_port_list;
+PortioList vbe_port_list;
qemu_register_reset(vga_reset, s);
@@ -2367,13 +2367,15 @@ void vga_init(VGACommonState *s, Object *obj,
MemoryRegion *address_space,
1);
memory_region_set_coalescing(vga_io_memory);
if (init_vga_ports) {
-portio_list_init(vga_port_list, obj, vga_ports, s, "vga");
-portio_list_set_flush_coalesced(vga_port_list);
-portio_list_add(vga_port_list, address_space_io, 0x3b0);
+portio_list_init(&vga_port_list, obj, vga_ports, s, "vga");
+portio_list_set_flush_coalesced(&vga_port_list);
+portio_list_add(&vga_port_list, address_space_io, 0x3b0);
+portio_list_destroy(&vga_port_list);
}
if (vbe_ports) {
-portio_list_init(vbe_port_list, obj
Re: [Qemu-devel] memory access trace from qemu
Pete Stevenson writes: > Hi All - > I would like to generate a trace of all memory accesses (i.e. read or write, > physical address, and data content/payload). The end goal is to use this trace > to drive a separate memory system simulator. Ideally, the trace would also > provide core-id and a timestamp (but I am not as optimistic that qemu will > give > me these). > I have noted that several previous threads address this topic, so perhaps the > question becomes can I get in contact with those who have successfully done > this > before? I'd like to do as little as possible here :) to get what I want, and > I'm > hoping that either this has been rolled into the new qemu release or that a > previously existing patch does most of what I want (i.e. which patch?). > I would be happy to hack the qemu source code if there is only one or two > places > where I need to do invasive surgery. You could try with [1], although it only provides information on virtual addresses. You can get the physical address and the value by calling other functions once you know the virtual address (qi_mem_virt_to_phys, qi_mem_read_phys). The little documentation available on the patches might be outdated, though. I was planning on cleaning it up after polishing the TCG tracing patches. [1] https://projects.gso.ac.upc.edu/projects/qemu-dbi/wiki Lluis -- "And it's much the same thing with knowledge, for whenever you learn something new, the whole world becomes that much richer." -- The Princess of Pure Reason, as told by Norton Juster in The Phantom Tollbooth
[Qemu-devel] [PATCH] virtfs-proxy-helper: fix call to accept
From: Tim Comer
The current code calls accept() without initializing the size parameter
which means the accept call might write too much to the stack.
URL: https://bugs.gentoo.org/486714
Signed-off-by: Tim Comer
Signed-off-by: Mike Frysinger
---
fsdev/virtfs-proxy-helper.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/fsdev/virtfs-proxy-helper.c b/fsdev/virtfs-proxy-helper.c
index bfecb87..cd291d3 100644
--- a/fsdev/virtfs-proxy-helper.c
+++ b/fsdev/virtfs-proxy-helper.c
@@ -760,6 +760,7 @@ static int proxy_socket(const char *path, uid_t uid, gid_t
gid)
return -1;
}
+size = sizeof(qemu);
client = accept(sock, (struct sockaddr *)&qemu, &size);
if (client < 0) {
do_perror("accept");
--
1.9.2
Re: [Qemu-devel] segfault while booting from saved snapshot
Thankyou for your reply. Live migration does seem like an interesting option to explore. I will look into it and get back if I get stuck. On Wed, Apr 16, 2014 at 6:16 PM, Kevin Wolf wrote: > Am 15.04.2014 um 16:55 hat Shehbaz Jaffer geschrieben: > > Thankyou for your reply. I do not face the error using qemu.1.7.1 > version. > > > > 1 quick question: > > > > I want to create a qcow2 image file after my VM has completed booting; > i.e., > > when I boot from this new snapshot, I should directly get to my VMs login > > prompt. How can i create such a qcow2 snapshot? > > > > I tried stopped the VM using QMP after the VM had completed booting, and > then > > using host CLI to create a qcow2 image : > > > > qemu-img create -b ubuntu.qcow2 -f qcow2 newsnap.qcow2 > > > > This created a new qcow2 file - newsnap.qcow2 on top of ubuntu.qcow2, > however > > when I boot using newsnap.qcow2, It takes me again through the complete > boot > > process - till the login prompt. > > > > Is there a way to create the qcow2 snapshot after VM has completed > booting? > > The reason for this is that you only create a snapshot of the disk this > way, not of the VM state (i.e. RAM contents, device states, etc.) > > The most convenient way when you're using qemu directly is probably > using internal snapshots. This is what the savevm/loadvm HMP commands > provide (not sure if there is a QMP equivalent); you can also use the > -loadvm command line parameter to start a VM from a snapshot. > > Another option is that you take an external snapshot in a new qcow2 file > like you did, but you additionally do a live migration into a file. When > restarting the VM, you need to use the -incoming option and feed that > file back to qemu. As this is a bit more cumbersome, this is probably > more an option for management tools than for manual use. > > Kevin > -- Shehbaz Jaffer MTS | Advanced Technology Group | NetApp
Re: [Qemu-devel] building for an arm host on an x86_64 machine [was: qemu builds on arm hosts]
Am 19.04.2014 11:53, schrieb Peter Maydell: > It's almost always much simpler just to build on the host > system. Since there's nothing you can do with the cross > compiled binaries unless you already have a host system, > I think what you need to do first is get and set up the ARM > hardware you're going to use. Then just build natively on that. ... or just use your distro's (or self-built without --cpu=) qemu-system-arm (with appropriate -machine argument) on the x86_64 machine to emulate an ARM machine, and within that use the gcc of the distro for building arm packages natively. A chroot using linux-user is another more advanced way to do the same. Cheers, Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [Xen-devel] [PATCH v3 2/4] GlobalProperty: Display warning about unused -global
Il 18/04/2014 18:54, Fabio Fantoni ha scritto:
Il 18/04/2014 17:59, Andreas Färber ha scritto:
Am 18.04.2014 17:36, schrieb Fabio Fantoni:
2014-04-18 17:21 GMT+02:00 Andreas Färber mailto:afaer...@suse.de>>:
Hi Don,
Am 25.03.2014 00 :55, schrieb Don Slutz:
> This can help a user understand why -global was ignored.
>
> For example: with "-vga cirrus"; "-global vga.vgamem_mb=16"
is just
> ignored when "-global cirrus-vga.vgamem_mb=16" is not.
>
> This is currently clear when the wrong property is provided:
>
> out/x86_64-softmmu/qemu-system-x86_64 -global
cirrus-vga.vram_size_mb=16 -monitor pty -vga cirrus
> char device redirected to /dev/pts/20 (label compat_monitor0)
> qemu-system-x86_64: Property '.vram_size_mb' not found
> Aborted (core dumped)
>
> vs
>
> out/x86_64-softmmu/qemu-system-x86_64 -global
vga.vram_size_mb=16
-monitor pty -vga cirrus
> char device redirected to /dev/pts/20 (label compat_monitor0)
> VNC server running on `::1:5900'
> ^Cqemu: terminating on signal 2
I added the cirrus video memory setting in libxl time ago (using
-global
vga.vram_size_mb), testing it with qemu 1.3 and also on qemu 1.6 when I
changed from -vga to -device if I remember good.
Has been changed in recent versions or something was not right even
though it seemed right to me?
There are multiple graphics cards to choose from. When using -vga std or
-device vga, then -global vga.foo=bar gets used; if -vga cirrus or
-device cirrus-vga then it needs to be -global cirrus-vga.foo=bar and
any -global vga.foo=bar gets ignored - unless you manage to add it as
secondary (PCI) graphics card.
Thanks for your reply.
Can you tell me if also -device cirrus-vga,vram_size_mb=N is correct
and working?
I probably found the correct values settable:
in
in hw/display/cirrus_vga.c:
2988 static Property pci_vga_cirrus_properties[] = {
2989 DEFINE_PROP_UINT32("vgamem_mb", struct PCICirrusVGAState,
2990cirrus_vga.vga.vram_size_mb, 8),
2991 DEFINE_PROP_END_OF_LIST(),
2992 };
Than I "-device cirrus-vga,vgamem_mb=N", not show errors and should be
correct, right?
in hw/display/vga-pci.c:
182 static Property vga_pci_properties[] = {
183 DEFINE_PROP_UINT32("vgamem_mb", PCIVGAState, vga.vram_size_mb,
16),
184 DEFINE_PROP_BIT("mmio", PCIVGAState, flags,
PCI_VGA_FLAG_ENABLE_MMIO, true),
185 DEFINE_PROP_END_OF_LIST(),
186 };
I tried time ago the videoram setting of stdvga on xen but seems was not
worked (no error but performance remain bad on medium/large resolution,
trying kvm with same parameters is better), I not know if is vgabios
problem or low level xen changes are needed.
The mmio option seems new to me, what it is in practice, may need to
disable it in xen?
Thanks for any reply and sorry for my bad english.
Thanks for any reply.
Regards,
Andreas
P.S. Please remember to use text format mails.
[Qemu-devel] spice bug: QXLInterface::client_monitors_config failed/missing unexpectedly
Testing latest xen with qemu 2.0.0-rc2 and spice-server 0.12.4 I found this bug on qemu logs of domU using emulated vga different from qxl (for now not working on xen, already reported in the past but not yet resolved): ... main_channel_link: add main channel client main_channel_handle_parsed: net test: latency 142.148000 ms, bitrate 416109 bps (0.396832 Mbps) LOW BANDWIDTH inputs_connect: inputs channel client create red_dispatcher_set_cursor_peer: main_channel_handle_parsed: agent start (/usr/sbin/xl:3581): Spice-Warning **: red_dispatcher.c:334:red_dispatcher_client_monitors_config: spice bug: QXLInterface::client_monitors_config failed/missing unexpectedly main_channel_handle_parsed: agent start (/usr/sbin/xl:3581): Spice-Warning **: red_dispatcher.c:334:red_dispatcher_client_monitors_config: spice bug: QXLInterface::client_monitors_config failed/missing unexpectedly (/usr/sbin/xl:3581): Spice-Warning **: red_dispatcher.c:334:red_dispatcher_client_monitors_config: spice bug: QXLInterface::client_monitors_config failed/missing unexpectedly main_channel_handle_parsed: agent start (/usr/sbin/xl:3581): Spice-Warning **: red_dispatcher.c:334:red_dispatcher_client_monitors_config: spice bug: QXLInterface::client_monitors_config failed/missing unexpectedly It is normal not using qxl and does not create problems or needs to be placed? If you need more informations or test tell me and I'll post them. Thanks for any reply and sorry for my bad english.
Re: [Qemu-devel] building for an arm host on an x86_64 machine [was: qemu builds on arm hosts]
On 19 April 2014 00:28, New B wrote: > I just realized that the original subject of my question was not accurate. > > I am trying to compile qemu to run on an arm host. I don't have an arm host > yet. Until I get one, I am just trying to build and link it on an x86_64 > ubuntu machine. (If I am already out of bounds at this point as I would need > a different toolchain, I would appreciate pointers) > Qemu configuration > ./configure --cpu=arm --target-list=arm-softmmu --disable-vnc --disable-sdl > --disable-virtfs --disable-brlapi --disable-rdma --disable-libusb > --disable-usb-redir --enable-pie This configure line is garbage: you're forcing it to try to build as if for an ARM host but you're not telling it to use an ARM cross compiler so it's building with the x86 compiler and this will never work. Don't try to specify --cpu manually. You seem to be confusing several things here. There are three different systems you need to care about in the cross compilation setup you're trying to do: (1) the build system, where you do your compilation (2) the host system, where the QEMU binaries you build are going to run (3) the guest system, what you want to actually emulate For you the build system is x86-64, and the host system is ARM. I'm not sure what you want the guest system to be. --target-list is where you configure which guest system; it seems unlikely you wanted to run ARM guests on an ARM host, but maybe you do. For cross compile you need to pass configure the --cross-prefix argument to tell it what your compiler is, for instance: --cross-prefix=arm-linux-gnueabihf- The correct argument depends on what your cross toolchain's command names are. If you don't already have a working cross compilation environment (including cross versions of the zlib, glib and other libraries QEMU needs) you're going to have to go and set that up first. That's a complicated process; maybe your Linux distribution will have a setup to do it, maybe not. In any case it's not something that I can really advise on. It also needs to be a cross setup that targets the library versions that you're going to be running on your host system. It's almost always much simpler just to build on the host system. Since there's nothing you can do with the cross compiled binaries unless you already have a host system, I think what you need to do first is get and set up the ARM hardware you're going to use. Then just build natively on that. thanks -- PMM
