[Qemu-devel] [PATCH 0/3] target/ppc: floating point multiply-add fixes
Exception handling in fmadd/fmsub/fnmadd/fnmsub isnt correct as the order of checking could give wrong settings in FPSCR. For example, (x * y) + z, if x = infinity, y = zero and z = snan. After the execution of instruction VXNAN and VXIMZ both should be set. For this correct the ordering in the float64_maddsub_update_excp() as follows: * If x, y or z is an SNaN, vxsnan_flag is set to 1. * If x is a Zero and y, is an Infinity or x is an Infinity and y is an Zero, vximz_flag is set to 1. * If the product of x and y is an Infinity and z is an Infinity of the opposite sign, vxisi_flag is set to 1. Moreover, all vector multiply-add/substract and vector scalar multiply-add/sub had the bug. VXISI should be set only when (∞ - ∞), where as the instruction was doing it wrong, was just checking ((a == ∞ OR b == ∞) && (c == ∞)). There are two issues here: 1. infinity can be +ve or -ve, i.e. (∞ + (-∞)), should result in setting VXISI 2. Need to take care of the operation (add or sub). (∞ + ∞) should not set VXISI Patch: 01: Fixes the order of checking and makes them independent as per the ISA 02: Introduces the macro to be used by Vector Scalar and Vector Multiply-Add 03: Make Vector Scalar and Vector Multiply Add use the macro for updating exception flags Nikunj A Dadhania (3): target/ppc: fmadd check for excp independently target/ppc: fmadd: add macro for updating flags target/ppc: use helper for excp handling target/ppc/fpu_helper.c | 78 + 1 file changed, 33 insertions(+), 45 deletions(-) -- 2.7.4
[Qemu-devel] [PATCH 2/3] target/ppc: fmadd: add macro for updating flags
Adds FPU_MADDSUB_UPDATE macro, this will be used for other routines
having float32/16
Signed-off-by: Nikunj A Dadhania
---
target/ppc/fpu_helper.c | 62 -
1 file changed, 31 insertions(+), 31 deletions(-)
diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c
index a547f58..56dfa18 100644
--- a/target/ppc/fpu_helper.c
+++ b/target/ppc/fpu_helper.c
@@ -743,38 +743,38 @@ uint64_t helper_frim(CPUPPCState *env, uint64_t arg)
return do_fri(env, arg, float_round_down);
}
-static void float64_maddsub_update_excp(CPUPPCState *env, float64 arg1,
-float64 arg2, float64 arg3,
-unsigned int madd_flags)
-{
-if (unlikely(float64_is_signaling_nan(arg1, &env->fp_status) ||
- float64_is_signaling_nan(arg2, &env->fp_status) ||
- float64_is_signaling_nan(arg3, &env->fp_status))) {
-/* sNaN operation */
-float_invalid_op_excp(env, POWERPC_EXCP_FP_VXSNAN, 1);
-}
-
-if (unlikely((float64_is_infinity(arg1) && float64_is_zero(arg2)) ||
- (float64_is_zero(arg1) && float64_is_infinity(arg2 {
-/* Multiplication of zero by infinity */
-float_invalid_op_excp(env, POWERPC_EXCP_FP_VXIMZ, 1);
-}
-
-if ((float64_is_infinity(arg1) || float64_is_infinity(arg2)) &&
-float64_is_infinity(arg3)) {
-uint8_t aSign, bSign, cSign;
-
-aSign = float64_is_neg(arg1);
-bSign = float64_is_neg(arg2);
-cSign = float64_is_neg(arg3);
-if (madd_flags & float_muladd_negate_c) {
-cSign ^= 1;
-}
-if (aSign ^ bSign ^ cSign) {
-float_invalid_op_excp(env, POWERPC_EXCP_FP_VXISI, 1);
-}
-}
+#define FPU_MADDSUB_UPDATE(name, tp)\
+static void name(CPUPPCState *env, float64 arg1,\
+ float64 arg2, float64 arg3,\
+ unsigned int madd_flags) \
+{ \
+if (tp##_is_signaling_nan(arg1, &env->fp_status) || \
+tp##_is_signaling_nan(arg2, &env->fp_status) || \
+tp##_is_signaling_nan(arg3, &env->fp_status)) { \
+/* sNaN operation */\
+float_invalid_op_excp(env, POWERPC_EXCP_FP_VXSNAN, 1); \
+} \
+if ((tp##_is_infinity(arg1) && tp##_is_zero(arg2)) || \
+(tp##_is_zero(arg1) && tp##_is_infinity(arg2))) { \
+/* Multiplication of zero by infinity */\
+float_invalid_op_excp(env, POWERPC_EXCP_FP_VXIMZ, 1); \
+} \
+if ((tp##_is_infinity(arg1) || tp##_is_infinity(arg2)) && \
+tp##_is_infinity(arg3)) { \
+uint8_t aSign, bSign, cSign;\
+\
+aSign = tp##_is_neg(arg1); \
+bSign = tp##_is_neg(arg2); \
+cSign = tp##_is_neg(arg3); \
+if (madd_flags & float_muladd_negate_c) { \
+cSign ^= 1; \
+} \
+if (aSign ^ bSign ^ cSign) {\
+float_invalid_op_excp(env, POWERPC_EXCP_FP_VXISI, 1); \
+} \
+} \
}
+FPU_MADDSUB_UPDATE(float64_maddsub_update_excp, float64)
#define FPU_FMADD(op, madd_flags) \
uint64_t helper_##op(CPUPPCState *env, uint64_t arg1, \
--
2.7.4
[Qemu-devel] [PATCH 3/3] target/ppc: use helper for excp handling
Use the helper routine float[32,64]_maddsub_update_excp() in VSX_MADD
macro.
Signed-off-by: Nikunj A Dadhania
---
target/ppc/fpu_helper.c | 20 ++--
1 file changed, 2 insertions(+), 18 deletions(-)
diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c
index 56dfa18..e88071b 100644
--- a/target/ppc/fpu_helper.c
+++ b/target/ppc/fpu_helper.c
@@ -774,6 +774,7 @@ static void name(CPUPPCState *env, float64 arg1,
\
} \
} \
}
+FPU_MADDSUB_UPDATE(float32_maddsub_update_excp, float32)
FPU_MADDSUB_UPDATE(float64_maddsub_update_excp, float64)
#define FPU_FMADD(op, madd_flags) \
@@ -2240,24 +2241,7 @@ void helper_##op(CPUPPCState *env, uint32_t opcode)
\
env->fp_status.float_exception_flags |= tstat.float_exception_flags; \
\
if (unlikely(tstat.float_exception_flags & float_flag_invalid)) { \
-if (tp##_is_signaling_nan(xa.fld, &tstat) || \
-tp##_is_signaling_nan(b->fld, &tstat) || \
-tp##_is_signaling_nan(c->fld, &tstat)) { \
-float_invalid_op_excp(env, POWERPC_EXCP_FP_VXSNAN, sfprf);\
-tstat.float_exception_flags &= ~float_flag_invalid; \
-} \
-if ((tp##_is_infinity(xa.fld) && tp##_is_zero(b->fld)) || \
-(tp##_is_zero(xa.fld) && tp##_is_infinity(b->fld))) { \
-xt_out.fld = float64_to_##tp(float_invalid_op_excp(env, \
-POWERPC_EXCP_FP_VXIMZ, sfprf), &env->fp_status); \
-tstat.float_exception_flags &= ~float_flag_invalid; \
-} \
-if ((tstat.float_exception_flags & float_flag_invalid) && \
-((tp##_is_infinity(xa.fld) || \
- tp##_is_infinity(b->fld)) &&\
- tp##_is_infinity(c->fld))) {\
-float_invalid_op_excp(env, POWERPC_EXCP_FP_VXISI, sfprf); \
-} \
+tp##_maddsub_update_excp(env, xa.fld, b->fld, c->fld, maddflgs); \
} \
\
if (r2sp) { \
--
2.7.4
Re: [Qemu-devel] [Qemu-block] [PATCH v3 0/3] throttle: improve command-line parameter documentation
On Fri, 3 Mar 2017 10:11:09 +0800 Stefan Hajnoczi wrote: > On Wed, Mar 01, 2017 at 10:27:48PM +0100, Greg Kurz wrote: > > On Wed, 1 Mar 2017 11:50:23 + > > Stefan Hajnoczi wrote: > > > > > v3: > > > * Added Patch 2 to fix invalid test parameters > > > * Switched to nicer max < avg check [Berto] > > > v2: > > > * Fixed s/bps/iops/ copy-paste error in Patch 1 [Berto] > > > * Rephrased warning about guest hangs and errors [Berto] > > > * Added Patch 2 to hide the internal .max value from the monitor > > > > > > Patch 1 fleshes out the documentation for I/O throttling command-line > > > parameters. > > > > > > Patch 2 hides an internal value that was being exposed to users via the > > > monitor and caused confusion. > > > > > > I ended up not adding QMP-style throttling.* names to the command-line > > > documentation because the names are very long and unlikely to be used. I > > > couldn't see a nice way of adding them while still keeping the > > > documentation > > > readable. > > > > > > > I only see this series now and it's a bit unfortunate... throttling options > > are now also available for fsdev. Only QMP-style names were added though, > > and they appear in the documentation. I agree the names are long, and the > > result isn't pretty on 80 columns, but it is readable still. > > > > I don't really want to add code, just to have shorter names and a prettier > > output. But it would be a good thing for fsdev to benefit from this new > > documentation... any suggestion how to do that ? > > I think another patch series would be good to: > 1. Document QMP-style names for -drive > 2. Reference throttling parameter documention in a common place for both >-fsdev and -drive. > > I'll do that. This is QEMU 2.10 material anyway (it has missed the > freeze deadline), so there's no time pressure. > > Stefan Sure. Please Cc me when you do that. Thanks. -- Greg pgpeNyIYWgGnT.pgp Description: OpenPGP digital signature
Re: [Qemu-devel] [Qemu-block] [PATCH 12/15] gluster: Plug memory leaks in qemu_gluster_parse_json()
On Thu, Mar 02, 2017 at 10:44:03PM +0100, Markus Armbruster wrote:
> To reproduce, run
>
> $ valgrind qemu-system-x86_64 --nodefaults -S --drive
> driver=gluster,volume=testvol,path=/a/b/c,server.0.type=xxx
>
> Signed-off-by: Markus Armbruster
> ---
> block/gluster.c | 28 ++--
> 1 file changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/block/gluster.c b/block/gluster.c
> index 6fbcf9e..35a7abb 100644
> --- a/block/gluster.c
> +++ b/block/gluster.c
> @@ -480,7 +480,7 @@ static int qemu_gluster_parse_json(BlockdevOptionsGluster
> *gconf,
>QDict *options, Error **errp)
> {
> QemuOpts *opts;
> -GlusterServer *gsconf;
> +GlusterServer *gsconf = NULL;
> GlusterServerList *curr = NULL;
> QDict *backing_options = NULL;
> Error *local_err = NULL;
> @@ -529,17 +529,16 @@ static int
> qemu_gluster_parse_json(BlockdevOptionsGluster *gconf,
> }
>
> ptr = qemu_opt_get(opts, GLUSTER_OPT_TYPE);
> +if (!ptr) {
> +error_setg(&local_err, QERR_MISSING_PARAMETER, GLUSTER_OPT_TYPE);
> +error_append_hint(&local_err, GERR_INDEX_HINT, i);
> +goto out;
> +
> +}
> gsconf = g_new0(GlusterServer, 1);
> gsconf->type = qapi_enum_parse(GlusterTransport_lookup, ptr,
> - GLUSTER_TRANSPORT__MAX,
> - GLUSTER_TRANSPORT__MAX,
> + GLUSTER_TRANSPORT__MAX, 0,
What is the reason to set the default to 0 and not the more readable
GLUSTER_TRANSPORT_UNIX?
> &local_err);
> -if (!ptr) {
> -error_setg(&local_err, QERR_MISSING_PARAMETER, GLUSTER_OPT_TYPE);
> -error_append_hint(&local_err, GERR_INDEX_HINT, i);
> -goto out;
> -
> -}
> if (local_err) {
> error_append_hint(&local_err,
>"Parameter '%s' may be 'tcp' or 'unix'\n",
> @@ -626,8 +625,10 @@ static int
> qemu_gluster_parse_json(BlockdevOptionsGluster *gconf,
> curr->next->value = gsconf;
> curr = curr->next;
> }
> +gsconf = NULL;
>
> -qdict_del(backing_options, str);
> +QDECREF(backing_options);
> +backing_options = NULL;
> g_free(str);
> str = NULL;
> }
> @@ -636,11 +637,10 @@ static int
> qemu_gluster_parse_json(BlockdevOptionsGluster *gconf,
>
> out:
> error_propagate(errp, local_err);
> +qapi_free_GlusterServer(gsconf);
> qemu_opts_del(opts);
> -if (str) {
> -qdict_del(backing_options, str);
> -g_free(str);
> -}
> +g_free(str);
> +QDECREF(backing_options);
> errno = EINVAL;
> return -errno;
> }
> --
> 2.7.4
>
>
Re: [Qemu-devel] [Qemu-block] [PATCH 10/15] gluster: Drop assumptions on SocketTransport names
Niels de Vos writes:
> On Thu, Mar 02, 2017 at 10:44:01PM +0100, Markus Armbruster wrote:
>> qemu_gluster_glfs_init() passes the names of QAPI enumeration type
>> SocketTransport to glfs_set_volfile_server(). Works, because they
>> were chosen to match. But the coupling is artificial. Use the
>> appropriate literal strings instead.
>>
>> Signed-off-by: Markus Armbruster
>> ---
>> block/gluster.c | 6 ++
>> 1 file changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/block/gluster.c b/block/gluster.c
>> index 56b4abe..7236d59 100644
>> --- a/block/gluster.c
>> +++ b/block/gluster.c
>> @@ -412,8 +412,7 @@ static struct glfs
>> *qemu_gluster_glfs_init(BlockdevOptionsGluster *gconf,
>>
>> for (server = gconf->server; server; server = server->next) {
>> if (server->value->type == GLUSTER_TRANSPORT_UNIX) {
>> -ret = glfs_set_volfile_server(glfs,
>> -
>> GlusterTransport_lookup[server->value->type],
>> +ret = glfs_set_volfile_server(glfs, "unix",
>> server->value->u.q_unix.path, 0);
>> } else {
>> if (parse_uint_full(server->value->u.tcp.port, &port, 10) < 0 ||
>> @@ -423,8 +422,7 @@ static struct glfs
>> *qemu_gluster_glfs_init(BlockdevOptionsGluster *gconf,
>> errno = EINVAL;
>> goto out;
>> }
>> -ret = glfs_set_volfile_server(glfs,
>> -
>> GlusterTransport_lookup[server->value->type],
>> +ret = glfs_set_volfile_server(glfs, "tcp",
>> server->value->u.tcp.host,
>> (int)port);
>> }
>> --
>> 2.7.4
>
> Instead of the strings for "unix" and "tcp", I would have liked
> #define's. Unfortunately it seems that these are not available in public
> headers :-/
Symbols from glfs.h would be ideal, yes. But well-known string literals
aren't too bad.
> If this is easier to understand, I don't have any objections.
It's easier to read, but that's not my chief reason. My chief reason is
that the values glfs_set_volfile_server() accepts are glfs's business,
while the names of the QAPI enumeration constants are QMP's business.
Coupling the two is inappropriate.
Decoupling them enables the renaming of the enumeration constant in
PATCH 14.
> Reviewed-by: Niels de Vos
Thanks!
Re: [Qemu-devel] [Qemu-block] [PATCH 12/15] gluster: Plug memory leaks in qemu_gluster_parse_json()
Niels de Vos writes:
> On Thu, Mar 02, 2017 at 10:44:03PM +0100, Markus Armbruster wrote:
>> To reproduce, run
>>
>> $ valgrind qemu-system-x86_64 --nodefaults -S --drive
>> driver=gluster,volume=testvol,path=/a/b/c,server.0.type=xxx
>>
>> Signed-off-by: Markus Armbruster
>> ---
>> block/gluster.c | 28 ++--
>> 1 file changed, 14 insertions(+), 14 deletions(-)
>>
>> diff --git a/block/gluster.c b/block/gluster.c
>> index 6fbcf9e..35a7abb 100644
>> --- a/block/gluster.c
>> +++ b/block/gluster.c
>> @@ -480,7 +480,7 @@ static int
>> qemu_gluster_parse_json(BlockdevOptionsGluster *gconf,
>>QDict *options, Error **errp)
>> {
>> QemuOpts *opts;
>> -GlusterServer *gsconf;
>> +GlusterServer *gsconf = NULL;
>> GlusterServerList *curr = NULL;
>> QDict *backing_options = NULL;
>> Error *local_err = NULL;
>> @@ -529,17 +529,16 @@ static int
>> qemu_gluster_parse_json(BlockdevOptionsGluster *gconf,
>> }
>>
>> ptr = qemu_opt_get(opts, GLUSTER_OPT_TYPE);
>> +if (!ptr) {
>> +error_setg(&local_err, QERR_MISSING_PARAMETER,
>> GLUSTER_OPT_TYPE);
>> +error_append_hint(&local_err, GERR_INDEX_HINT, i);
>> +goto out;
>> +
>> +}
>> gsconf = g_new0(GlusterServer, 1);
>> gsconf->type = qapi_enum_parse(GlusterTransport_lookup, ptr,
>> - GLUSTER_TRANSPORT__MAX,
>> - GLUSTER_TRANSPORT__MAX,
>> + GLUSTER_TRANSPORT__MAX, 0,
>
> What is the reason to set the default to 0 and not the more readable
> GLUSTER_TRANSPORT_UNIX?
I chose 0 because the actual value must not matter: we don't want a
default here.
qapi_enum_parse() returns this argument when ptr isn't found. If ptr is
non-null, it additionally sets an error. Since ptr can't be null here,
the argument is only returned together with an error. But then we won't
use *gsconf.
Do you think -1 instead of 0 would be clearer?
>> &local_err);
>> -if (!ptr) {
>> -error_setg(&local_err, QERR_MISSING_PARAMETER,
>> GLUSTER_OPT_TYPE);
>> -error_append_hint(&local_err, GERR_INDEX_HINT, i);
>> -goto out;
>> -
>> -}
>> if (local_err) {
>> error_append_hint(&local_err,
>>"Parameter '%s' may be 'tcp' or 'unix'\n",
>> @@ -626,8 +625,10 @@ static int
>> qemu_gluster_parse_json(BlockdevOptionsGluster *gconf,
>> curr->next->value = gsconf;
>> curr = curr->next;
>> }
>> +gsconf = NULL;
>>
>> -qdict_del(backing_options, str);
>> +QDECREF(backing_options);
>> +backing_options = NULL;
>> g_free(str);
>> str = NULL;
>> }
>> @@ -636,11 +637,10 @@ static int
>> qemu_gluster_parse_json(BlockdevOptionsGluster *gconf,
>>
>> out:
>> error_propagate(errp, local_err);
>> +qapi_free_GlusterServer(gsconf);
>> qemu_opts_del(opts);
>> -if (str) {
>> -qdict_del(backing_options, str);
>> -g_free(str);
>> -}
>> +g_free(str);
>> +QDECREF(backing_options);
>> errno = EINVAL;
>> return -errno;
>> }
>> --
>> 2.7.4
>>
>>
Re: [Qemu-devel] iommu emulation
Peter Xu writes: > On Thu, Mar 02, 2017 at 05:20:19PM -0500, Bandan Das wrote: >> Jintack Lim writes: >> >> > [cc Bandan] >> > ... >> >> Jintack, any progress with this ? >> >> I am testing on a X540-AT2 and I see a different behavior. It appears >> config succeeds but the driver keeps resetting the device due to a Tx >> hang: >> >> [ 568.612391 ] ixgbe :00:03.0 enp0s3: tx hang 38 detected on queue 0, >> resetting adapter >> [ 568.612393 ] ixgbe :00:03.0 enp0s3: initiating reset due to tx >> timeout >> [ 568.612397 ] ixgbe :00:03.0 enp0s3: Reset adapter >> >> This may be device specific but I think the actual behavior you see is >> also dependent on the ixgbe driver in the guest. Are you on a recent >> kernel ? Also, can you point me to the hack (by Peter) that you have >> mentioned above ? > > Hi, Bandan, > > Are you using the vtd vfio v7 series or another branch? Thanks for the tip. Jintack pointed me to your repo and I am using v7 from there. > If it's the upstream one... just a note that we need to make sure > "-device intel-iommu" be the first device specified in QEMU command > line parameters (need to before "-device vfio-pci,..."). Thanks, > > -- peterx
[Qemu-devel] [Bug 1668556] Re: QEMU Guest Agent service fails to start on Windows 10 RS2 preview
Not a bug! I have forgotten to add the socket in the start script :( -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1668556 Title: QEMU Guest Agent service fails to start on Windows 10 RS2 preview Status in QEMU: New Bug description: The "QEMU Guest Agent" service cannot be started on Windows 10 RS2 preview build. After starting the service this error message is displayed: "Windows could not start QEMU Guest Agent service on Local Computer. Error 1053: The service did not respond to the start or control request in a timely fashion." Output from the Windows System event log: 700902000x80801406SystemLT-WIN10-643QEMU Guest Agent510045004D0055002D0047004100 The "QEMU Guest Agent VSS Provider" service is running successfully. It worked on Windows 10 RS1 (before the upgrade). QEMU Guest Agent version: Installed from virtio-win-0.1.126_amd64 which was built from master branch with latest commit (8aaf403) - https://github.com/virtio-win/kvm-guest-drivers-windows Windows version (upgraded from RS1): Windows 10 Enterprise Insider Preview Evaluation copy. Build 15019.rs_prerelease.170121-1513 To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1668556/+subscriptions
