[Qemu-devel] [PATCH v3 5/6] tcg/i386: Add vector operations

[Richard Henderson]

Signed-off-by: Richard Henderson 
---
 tcg/i386/tcg-target.h |  36 +++-
 tcg/i386/tcg-target.inc.c | 423 +-
 2 files changed, 413 insertions(+), 46 deletions(-)

diff --git a/tcg/i386/tcg-target.h b/tcg/i386/tcg-target.h
index b89dababf4..df69f8db91 100644
--- a/tcg/i386/tcg-target.h
+++ b/tcg/i386/tcg-target.h
@@ -30,11 +30,10 @@
 
 #ifdef __x86_64__
 # define TCG_TARGET_REG_BITS  64
-# define TCG_TARGET_NB_REGS   16
 #else
 # define TCG_TARGET_REG_BITS  32
-# define TCG_TARGET_NB_REGS8
 #endif
+# define TCG_TARGET_NB_REGS   24
 
 typedef enum {
 TCG_REG_EAX = 0,
@@ -56,6 +55,19 @@ typedef enum {
 TCG_REG_R13,
 TCG_REG_R14,
 TCG_REG_R15,
+
+/* SSE registers; 64-bit has access to 8 more, but we won't
+   need more than a few and using only the first 8 minimizes
+   the need for a rex prefix on the sse instructions.  */
+TCG_REG_XMM0,
+TCG_REG_XMM1,
+TCG_REG_XMM2,
+TCG_REG_XMM3,
+TCG_REG_XMM4,
+TCG_REG_XMM5,
+TCG_REG_XMM6,
+TCG_REG_XMM7,
+
 TCG_REG_RAX = TCG_REG_EAX,
 TCG_REG_RCX = TCG_REG_ECX,
 TCG_REG_RDX = TCG_REG_EDX,
@@ -78,6 +90,17 @@ typedef enum {
 extern bool have_bmi1;
 extern bool have_popcnt;
 
+#ifdef __SSE2__
+#define have_sse2  true
+#else
+extern bool have_sse2;
+#endif
+#ifdef __AVX2__
+#define have_avx2  true
+#else
+extern bool have_avx2;
+#endif
+
 /* optional instructions */
 #define TCG_TARGET_HAS_div2_i32 1
 #define TCG_TARGET_HAS_rot_i32  1
@@ -146,6 +169,15 @@ extern bool have_popcnt;
 #define TCG_TARGET_HAS_mulsh_i640
 #endif
 
+#define TCG_TARGET_HAS_v64  have_sse2
+#define TCG_TARGET_HAS_v128 have_sse2
+#define TCG_TARGET_HAS_v256 have_avx2
+
+#define TCG_TARGET_HAS_andc_vec 1
+#define TCG_TARGET_HAS_orc_vec  0
+#define TCG_TARGET_HAS_not_vec  0
+#define TCG_TARGET_HAS_neg_vec  0
+
 #define TCG_TARGET_deposit_i32_valid(ofs, len) \
 (((ofs) == 0 && (len) == 8) || ((ofs) == 8 && (len) == 8) || \
  ((ofs) == 0 && (len) == 16))
diff --git a/tcg/i386/tcg-target.inc.c b/tcg/i386/tcg-target.inc.c
index 69e49c9f58..df3be932d5 100644
--- a/tcg/i386/tcg-target.inc.c
+++ b/tcg/i386/tcg-target.inc.c
@@ -28,10 +28,11 @@
 static const char * const tcg_target_reg_names[TCG_TARGET_NB_REGS] = {
 #if TCG_TARGET_REG_BITS == 64
 "%rax", "%rcx", "%rdx", "%rbx", "%rsp", "%rbp", "%rsi", "%rdi",
-"%r8",  "%r9",  "%r10", "%r11", "%r12", "%r13", "%r14", "%r15",
 #else
 "%eax", "%ecx", "%edx", "%ebx", "%esp", "%ebp", "%esi", "%edi",
 #endif
+"%r8",  "%r9",  "%r10", "%r11", "%r12", "%r13", "%r14", "%r15",
+"%xmm0", "%xmm1", "%xmm2", "%xmm3", "%xmm4", "%xmm5", "%xmm6", "%xmm7",
 };
 #endif
 
@@ -61,6 +62,14 @@ static const int tcg_target_reg_alloc_order[] = {
 TCG_REG_EDX,
 TCG_REG_EAX,
 #endif
+TCG_REG_XMM0,
+TCG_REG_XMM1,
+TCG_REG_XMM2,
+TCG_REG_XMM3,
+TCG_REG_XMM4,
+TCG_REG_XMM5,
+TCG_REG_XMM6,
+TCG_REG_XMM7,
 };
 
 static const int tcg_target_call_iarg_regs[] = {
@@ -94,7 +103,7 @@ static const int tcg_target_call_oarg_regs[] = {
 #define TCG_CT_CONST_I32 0x400
 #define TCG_CT_CONST_WSZ 0x800
 
-/* Registers used with L constraint, which are the first argument 
+/* Registers used with L constraint, which are the first argument
registers on x86_64, and two random call clobbered registers on
i386. */
 #if TCG_TARGET_REG_BITS == 64
@@ -126,6 +135,16 @@ static bool have_cmov;
 bool have_bmi1;
 bool have_popcnt;
 
+#ifndef have_sse2
+bool have_sse2;
+#endif
+#ifdef have_avx2
+#define have_avx1  have_avx2
+#else
+static bool have_avx1;
+bool have_avx2;
+#endif
+
 #ifdef CONFIG_CPUID_H
 static bool have_movbe;
 static bool have_bmi2;
@@ -192,14 +211,17 @@ static const char 
*target_parse_constraint(TCGArgConstraint *ct,
 tcg_regset_set_reg(ct->u.regs, TCG_REG_EDI);
 break;
 case 'q':
+/* A register that can be used as a byte operand.  */
 ct->ct |= TCG_CT_REG;
 ct->u.regs = TCG_TARGET_REG_BITS == 64 ? 0x : 0xf;
 break;
 case 'Q':
+/* A register with an addressable second byte (e.g. %ah).  */
 ct->ct |= TCG_CT_REG;
 ct->u.regs = 0xf;
 break;
 case 'r':
+/* A general register.  */
 ct->ct |= TCG_CT_REG;
 ct->u.regs = TCG_TARGET_REG_BITS == 64 ? 0x : 0xff;
 break;
@@ -207,6 +229,11 @@ static const char 
*target_parse_constraint(TCGArgConstraint *ct,
 /* With TZCNT/LZCNT, we can have operand-size as an input.  */
 ct->ct |= TCG_CT_CONST_WSZ;
 break;
+case 'x':
+/* A vector register.  */
+ct->ct |= TCG_CT_REG;
+ct->u.regs = 0xff;
+break;
 
 /* qemu_ld/st address constraint */
 case 'L':
@@ -277,8 +304,9 @@ static inline int tcg_target_const_match(tcg_target_long 
val, TCGType type,
 # define 

Re: [Qemu-devel] [PATCH v3 0/6] TCG vectorization and example conversion

[Richard Henderson]

On 09/15/2017 07:34 PM, Richard Henderson wrote:
> Now addressing the complex vector op issue.  I now expose TCGv_vec
> to target front-ends, but opaque wrt the vector size.  One can thus
> compose vector operations, as demonstrated in target/arm/.
> 
> The actual host vector length now becomes an argument to the *_vec
> opcodes.  It's a little awkward, but does prevent an explosion of
> opcode values.
> 
> All R-b dropped because all patches rewritten or heavily modified.

Bah.  Forgot to mention that this depends on tcg-next.  Full tree at

  git://github.com/rth7680/qemu.git native-vector-registers-3


r~

[Qemu-devel] [PATCH v3 2/6] tcg: Add vector expanders

[Richard Henderson]

Signed-off-by: Richard Henderson 
---
 Makefile.target  |   2 +-
 accel/tcg/tcg-runtime.h  |  24 ++
 tcg/tcg-gvec-desc.h  |  49 +++
 tcg/tcg-op-gvec.h| 143 
 accel/tcg/tcg-runtime-gvec.c | 255 +
 tcg/tcg-op-gvec.c| 853 +++
 accel/tcg/Makefile.objs  |   2 +-
 7 files changed, 1326 insertions(+), 2 deletions(-)
 create mode 100644 tcg/tcg-gvec-desc.h
 create mode 100644 tcg/tcg-op-gvec.h
 create mode 100644 accel/tcg/tcg-runtime-gvec.c
 create mode 100644 tcg/tcg-op-gvec.c

diff --git a/Makefile.target b/Makefile.target
index 6361f957fb..f9967feef5 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -94,7 +94,7 @@ all: $(PROGS) stap
 obj-y += exec.o
 obj-y += accel/
 obj-$(CONFIG_TCG) += tcg/tcg.o tcg/tcg-op.o tcg/optimize.o
-obj-$(CONFIG_TCG) += tcg/tcg-common.o
+obj-$(CONFIG_TCG) += tcg/tcg-common.o tcg/tcg-op-gvec.o
 obj-$(CONFIG_TCG_INTERPRETER) += tcg/tci.o
 obj-$(CONFIG_TCG_INTERPRETER) += disas/tci.o
 obj-y += fpu/softfloat.o
diff --git a/accel/tcg/tcg-runtime.h b/accel/tcg/tcg-runtime.h
index c41d38a557..61c0ce39d3 100644
--- a/accel/tcg/tcg-runtime.h
+++ b/accel/tcg/tcg-runtime.h
@@ -134,3 +134,27 @@ GEN_ATOMIC_HELPERS(xor_fetch)
 GEN_ATOMIC_HELPERS(xchg)
 
 #undef GEN_ATOMIC_HELPERS
+
+DEF_HELPER_FLAGS_3(gvec_mov, TCG_CALL_NO_RWG, void, ptr, ptr, i32)
+
+DEF_HELPER_FLAGS_4(gvec_add8, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
+DEF_HELPER_FLAGS_4(gvec_add16, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
+DEF_HELPER_FLAGS_4(gvec_add32, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
+DEF_HELPER_FLAGS_4(gvec_add64, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
+
+DEF_HELPER_FLAGS_4(gvec_sub8, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
+DEF_HELPER_FLAGS_4(gvec_sub16, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
+DEF_HELPER_FLAGS_4(gvec_sub32, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
+DEF_HELPER_FLAGS_4(gvec_sub64, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
+
+DEF_HELPER_FLAGS_3(gvec_neg8, TCG_CALL_NO_RWG, void, ptr, ptr, i32)
+DEF_HELPER_FLAGS_3(gvec_neg16, TCG_CALL_NO_RWG, void, ptr, ptr, i32)
+DEF_HELPER_FLAGS_3(gvec_neg32, TCG_CALL_NO_RWG, void, ptr, ptr, i32)
+DEF_HELPER_FLAGS_3(gvec_neg64, TCG_CALL_NO_RWG, void, ptr, ptr, i32)
+
+DEF_HELPER_FLAGS_3(gvec_not, TCG_CALL_NO_RWG, void, ptr, ptr, i32)
+DEF_HELPER_FLAGS_4(gvec_and, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
+DEF_HELPER_FLAGS_4(gvec_or, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
+DEF_HELPER_FLAGS_4(gvec_xor, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
+DEF_HELPER_FLAGS_4(gvec_andc, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
+DEF_HELPER_FLAGS_4(gvec_orc, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
diff --git a/tcg/tcg-gvec-desc.h b/tcg/tcg-gvec-desc.h
new file mode 100644
index 00..8ba9a8168d
--- /dev/null
+++ b/tcg/tcg-gvec-desc.h
@@ -0,0 +1,49 @@
+/*
+ *  Generic vector operation descriptor
+ *
+ *  Copyright (c) 2017 Linaro
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library 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
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see .
+ */
+
+/* ??? These bit widths are set for ARM SVE, maxing out at 256 byte vectors. */
+#define SIMD_OPRSZ_SHIFT   0
+#define SIMD_OPRSZ_BITS5
+
+#define SIMD_MAXSZ_SHIFT   (SIMD_OPRSZ_SHIFT + SIMD_OPRSZ_BITS)
+#define SIMD_MAXSZ_BITS5
+
+#define SIMD_DATA_SHIFT(SIMD_MAXSZ_SHIFT + SIMD_MAXSZ_BITS)
+#define SIMD_DATA_BITS (32 - SIMD_DATA_SHIFT)
+
+/* Create a descriptor from components.  */
+uint32_t simd_desc(uint32_t oprsz, uint32_t maxsz, int32_t data);
+
+/* Extract the operation size from a descriptor.  */
+static inline intptr_t simd_oprsz(uint32_t desc)
+{
+return (extract32(desc, SIMD_OPRSZ_SHIFT, SIMD_OPRSZ_BITS) + 1) * 8;
+}
+
+/* Extract the max vector size from a descriptor.  */
+static inline intptr_t simd_maxsz(uint32_t desc)
+{
+return (extract32(desc, SIMD_MAXSZ_SHIFT, SIMD_MAXSZ_BITS) + 1) * 8;
+}
+
+/* Extract the operation-specific data from a descriptor.  */
+static inline int32_t simd_data(uint32_t desc)
+{
+return sextract32(desc, SIMD_DATA_SHIFT, SIMD_DATA_BITS);
+}
diff --git a/tcg/tcg-op-gvec.h b/tcg/tcg-op-gvec.h
new file mode 100644
index 00..28bd77f1dc
--- /dev/null
+++ b/tcg/tcg-op-gvec.h
@@ -0,0 +1,143 @@
+/*
+ *  Generic vector operation expansion
+ *
+ *  Copyright (c) 2017 Linaro
+ *
+ * This library is free software; you can redistribute it 

[Qemu-devel] [PATCH v3 1/6] tcg: Add types and operations for host vectors

[Richard Henderson]

Nothing uses or enables them yet.

Signed-off-by: Richard Henderson 
---
 tcg/tcg-op.h  |  26 +++
 tcg/tcg-opc.h |  37 ++
 tcg/tcg.h |  34 +
 tcg/tcg-op.c  | 234 ++
 tcg/tcg.c |  77 ++-
 tcg/README|  46 
 6 files changed, 453 insertions(+), 1 deletion(-)

diff --git a/tcg/tcg-op.h b/tcg/tcg-op.h
index 5d3278f243..b9b0b9f46f 100644
--- a/tcg/tcg-op.h
+++ b/tcg/tcg-op.h
@@ -915,6 +915,32 @@ void tcg_gen_atomic_or_fetch_i64(TCGv_i64, TCGv, TCGv_i64, 
TCGArg, TCGMemOp);
 void tcg_gen_atomic_xor_fetch_i32(TCGv_i32, TCGv, TCGv_i32, TCGArg, TCGMemOp);
 void tcg_gen_atomic_xor_fetch_i64(TCGv_i64, TCGv, TCGv_i64, TCGArg, TCGMemOp);
 
+void tcg_gen_mov_vec(TCGv_vec, TCGv_vec);
+void tcg_gen_movi_vec(TCGv_vec, tcg_target_long);
+void tcg_gen_add8_vec(TCGv_vec r, TCGv_vec a, TCGv_vec b);
+void tcg_gen_add16_vec(TCGv_vec r, TCGv_vec a, TCGv_vec b);
+void tcg_gen_add32_vec(TCGv_vec r, TCGv_vec a, TCGv_vec b);
+void tcg_gen_add64_vec(TCGv_vec r, TCGv_vec a, TCGv_vec b);
+void tcg_gen_sub8_vec(TCGv_vec r, TCGv_vec a, TCGv_vec b);
+void tcg_gen_sub16_vec(TCGv_vec r, TCGv_vec a, TCGv_vec b);
+void tcg_gen_sub32_vec(TCGv_vec r, TCGv_vec a, TCGv_vec b);
+void tcg_gen_sub64_vec(TCGv_vec r, TCGv_vec a, TCGv_vec b);
+void tcg_gen_and_vec(TCGv_vec r, TCGv_vec a, TCGv_vec b);
+void tcg_gen_or_vec(TCGv_vec r, TCGv_vec a, TCGv_vec b);
+void tcg_gen_xor_vec(TCGv_vec r, TCGv_vec a, TCGv_vec b);
+void tcg_gen_andc_vec(TCGv_vec r, TCGv_vec a, TCGv_vec b);
+void tcg_gen_orc_vec(TCGv_vec r, TCGv_vec a, TCGv_vec b);
+void tcg_gen_not_vec(TCGv_vec r, TCGv_vec a);
+void tcg_gen_neg8_vec(TCGv_vec r, TCGv_vec a);
+void tcg_gen_neg16_vec(TCGv_vec r, TCGv_vec a);
+void tcg_gen_neg32_vec(TCGv_vec r, TCGv_vec a);
+void tcg_gen_neg64_vec(TCGv_vec r, TCGv_vec a);
+
+void tcg_gen_ld_vec(TCGv_vec r, TCGv_ptr base, TCGArg offset);
+void tcg_gen_st_vec(TCGv_vec r, TCGv_ptr base, TCGArg offset);
+void tcg_gen_ldz_vec(TCGv_vec r, TCGv_ptr base, TCGArg offset, TCGType sz);
+void tcg_gen_stl_vec(TCGv_vec r, TCGv_ptr base, TCGArg offset, TCGType sz);
+
 #if TARGET_LONG_BITS == 64
 #define tcg_gen_movi_tl tcg_gen_movi_i64
 #define tcg_gen_mov_tl tcg_gen_mov_i64
diff --git a/tcg/tcg-opc.h b/tcg/tcg-opc.h
index 956fb1e9f3..8200184fa9 100644
--- a/tcg/tcg-opc.h
+++ b/tcg/tcg-opc.h
@@ -204,8 +204,45 @@ DEF(qemu_ld_i64, DATA64_ARGS, TLADDR_ARGS, 1,
 DEF(qemu_st_i64, 0, TLADDR_ARGS + DATA64_ARGS, 1,
 TCG_OPF_CALL_CLOBBER | TCG_OPF_SIDE_EFFECTS | TCG_OPF_64BIT)
 
+/* Host vector support.  */
+
+#define IMPLVEC  \
+IMPL(TCG_TARGET_HAS_v64 | TCG_TARGET_HAS_v128 | TCG_TARGET_HAS_v256)
+
+DEF(mov_vec, 1, 1, 1, TCG_OPF_NOT_PRESENT)
+
+/* ??? Simple, but perhaps dupiN would be more descriptive.  */
+DEF(movi_vec, 1, 0, 2, TCG_OPF_NOT_PRESENT)
+
+DEF(ld_vec, 1, 1, 2, IMPLVEC)
+DEF(ldz_vec, 1, 1, 3, IMPLVEC)
+DEF(st_vec, 0, 2, 2, IMPLVEC)
+
+DEF(add8_vec, 1, 2, 1, IMPLVEC)
+DEF(add16_vec, 1, 2, 1, IMPLVEC)
+DEF(add32_vec, 1, 2, 1, IMPLVEC)
+DEF(add64_vec, 1, 2, 1, IMPLVEC)
+
+DEF(sub8_vec, 1, 2, 1, IMPLVEC)
+DEF(sub16_vec, 1, 2, 1, IMPLVEC)
+DEF(sub32_vec, 1, 2, 1, IMPLVEC)
+DEF(sub64_vec, 1, 2, 1, IMPLVEC)
+
+DEF(neg8_vec, 1, 1, 1, IMPLVEC | IMPL(TCG_TARGET_HAS_neg_vec))
+DEF(neg16_vec, 1, 1, 1, IMPLVEC | IMPL(TCG_TARGET_HAS_neg_vec))
+DEF(neg32_vec, 1, 1, 1, IMPLVEC | IMPL(TCG_TARGET_HAS_neg_vec))
+DEF(neg64_vec, 1, 1, 1, IMPLVEC | IMPL(TCG_TARGET_HAS_neg_vec))
+
+DEF(and_vec, 1, 2, 1, IMPLVEC)
+DEF(or_vec, 1, 2, 1, IMPLVEC)
+DEF(xor_vec, 1, 2, 1, IMPLVEC)
+DEF(andc_vec, 1, 2, 1, IMPLVEC | IMPL(TCG_TARGET_HAS_andc_vec))
+DEF(orc_vec, 1, 2, 1, IMPLVEC | IMPL(TCG_TARGET_HAS_orc_vec))
+DEF(not_vec, 1, 1, 1, IMPLVEC | IMPL(TCG_TARGET_HAS_not_vec))
+
 #undef TLADDR_ARGS
 #undef DATA64_ARGS
 #undef IMPL
 #undef IMPL64
+#undef IMPLVEC
 #undef DEF
diff --git a/tcg/tcg.h b/tcg/tcg.h
index 25662c36d4..7cd356e87f 100644
--- a/tcg/tcg.h
+++ b/tcg/tcg.h
@@ -173,6 +173,16 @@ typedef uint64_t TCGRegSet;
 # error "Missing unsigned widening multiply"
 #endif
 
+#ifndef TCG_TARGET_HAS_v64
+#define TCG_TARGET_HAS_v64  0
+#define TCG_TARGET_HAS_v128 0
+#define TCG_TARGET_HAS_v256 0
+#define TCG_TARGET_HAS_neg_vec  0
+#define TCG_TARGET_HAS_not_vec  0
+#define TCG_TARGET_HAS_andc_vec 0
+#define TCG_TARGET_HAS_orc_vec  0
+#endif
+
 #ifndef TARGET_INSN_START_EXTRA_WORDS
 # define TARGET_INSN_START_WORDS 1
 #else
@@ -249,6 +259,11 @@ typedef struct TCGPool {
 typedef enum TCGType {
 TCG_TYPE_I32,
 TCG_TYPE_I64,
+
+TCG_TYPE_V64,
+TCG_TYPE_V128,
+TCG_TYPE_V256,
+
 TCG_TYPE_COUNT, /* number of different types */
 
 /* An alias for the size of the host register.  */
@@ -399,6 +414,8 @@ typedef tcg_target_ulong TCGArg;
 * TCGv_i32 : 32 bit integer type
 * TCGv_i64 : 64 bit integer type
 * TCGv_ptr : a host pointer type
+* TCGv_vec : a 

[Qemu-devel] [PATCH v3 4/6] target/arm: Use vector infrastructure for aa64 add/sub/logic

[Richard Henderson]

Signed-off-by: Richard Henderson 
---
 target/arm/translate-a64.c | 216 ++---
 1 file changed, 143 insertions(+), 73 deletions(-)

diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index a3984c9a0d..4759cc9829 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -21,6 +21,7 @@
 #include "cpu.h"
 #include "exec/exec-all.h"
 #include "tcg-op.h"
+#include "tcg-op-gvec.h"
 #include "qemu/log.h"
 #include "arm_ldst.h"
 #include "translate.h"
@@ -82,6 +83,7 @@ typedef void NeonGenTwoDoubleOPFn(TCGv_i64, TCGv_i64, 
TCGv_i64, TCGv_ptr);
 typedef void NeonGenOneOpFn(TCGv_i64, TCGv_i64);
 typedef void CryptoTwoOpEnvFn(TCGv_ptr, TCGv_i32, TCGv_i32);
 typedef void CryptoThreeOpEnvFn(TCGv_ptr, TCGv_i32, TCGv_i32, TCGv_i32);
+typedef void GVecGenTwoFn(uint32_t, uint32_t, uint32_t, uint32_t, uint32_t);
 
 /* initialize TCG globals.  */
 void a64_translate_init(void)
@@ -537,6 +539,21 @@ static inline int vec_reg_offset(DisasContext *s, int 
regno,
 return offs;
 }
 
+/* Return the offset info CPUARMState of the "whole" vector register Qn.  */
+static inline int vec_full_reg_offset(DisasContext *s, int regno)
+{
+assert_fp_access_checked(s);
+return offsetof(CPUARMState, vfp.regs[regno * 2]);
+}
+
+/* Return the byte size of the "whole" vector register, VL / 8.  */
+static inline int vec_full_reg_size(DisasContext *s)
+{
+/* FIXME SVE: We should put the composite ZCR_EL* value into tb->flags.
+   In the meantime this is just the AdvSIMD length of 128.  */
+return 128 / 8;
+}
+
 /* Return the offset into CPUARMState of a slice (from
  * the least significant end) of FP register Qn (ie
  * Dn, Sn, Hn or Bn).
@@ -9036,85 +9053,125 @@ static void disas_simd_three_reg_diff(DisasContext *s, 
uint32_t insn)
 }
 }
 
+static void gen_bsl_i64(TCGv_i64 rd, TCGv_i64 rn, TCGv_i64 rm)
+{
+tcg_gen_xor_i64(rn, rn, rm);
+tcg_gen_and_i64(rn, rn, rd);
+tcg_gen_xor_i64(rd, rm, rn);
+}
+
+static void gen_bit_i64(TCGv_i64 rd, TCGv_i64 rn, TCGv_i64 rm)
+{
+tcg_gen_xor_i64(rn, rn, rd);
+tcg_gen_and_i64(rn, rn, rm);
+tcg_gen_xor_i64(rd, rd, rn);
+}
+
+static void gen_bif_i64(TCGv_i64 rd, TCGv_i64 rn, TCGv_i64 rm)
+{
+tcg_gen_xor_i64(rn, rn, rd);
+tcg_gen_andc_i64(rn, rn, rm);
+tcg_gen_xor_i64(rd, rd, rn);
+}
+
+static void gen_bsl_vec(TCGv_vec rd, TCGv_vec rn, TCGv_vec rm)
+{
+tcg_gen_xor_vec(rn, rn, rm);
+tcg_gen_and_vec(rn, rn, rd);
+tcg_gen_xor_vec(rd, rm, rn);
+}
+
+static void gen_bit_vec(TCGv_vec rd, TCGv_vec rn, TCGv_vec rm)
+{
+tcg_gen_xor_vec(rn, rn, rd);
+tcg_gen_and_vec(rn, rn, rm);
+tcg_gen_xor_vec(rd, rd, rn);
+}
+
+static void gen_bif_vec(TCGv_vec rd, TCGv_vec rn, TCGv_vec rm)
+{
+tcg_gen_xor_vec(rn, rn, rd);
+tcg_gen_andc_vec(rn, rn, rm);
+tcg_gen_xor_vec(rd, rd, rn);
+}
+
 /* Logic op (opcode == 3) subgroup of C3.6.16. */
 static void disas_simd_3same_logic(DisasContext *s, uint32_t insn)
 {
+static const GVecGen3 bsl_op = {
+.fni8 = gen_bsl_i64,
+.fniv = gen_bsl_vec,
+.prefer_i64 = TCG_TARGET_REG_BITS == 64,
+.load_dest = true
+};
+static const GVecGen3 bit_op = {
+.fni8 = gen_bit_i64,
+.fniv = gen_bit_vec,
+.prefer_i64 = TCG_TARGET_REG_BITS == 64,
+.load_dest = true
+};
+static const GVecGen3 bif_op = {
+.fni8 = gen_bif_i64,
+.fniv = gen_bif_vec,
+.prefer_i64 = TCG_TARGET_REG_BITS == 64,
+.load_dest = true
+};
+
 int rd = extract32(insn, 0, 5);
 int rn = extract32(insn, 5, 5);
 int rm = extract32(insn, 16, 5);
 int size = extract32(insn, 22, 2);
 bool is_u = extract32(insn, 29, 1);
 bool is_q = extract32(insn, 30, 1);
-TCGv_i64 tcg_op1, tcg_op2, tcg_res[2];
-int pass;
+GVecGenTwoFn *gvec_fn;
+const GVecGen3 *gvec_op;
 
 if (!fp_access_check(s)) {
 return;
 }
 
-tcg_op1 = tcg_temp_new_i64();
-tcg_op2 = tcg_temp_new_i64();
-tcg_res[0] = tcg_temp_new_i64();
-tcg_res[1] = tcg_temp_new_i64();
-
-for (pass = 0; pass < (is_q ? 2 : 1); pass++) {
-read_vec_element(s, tcg_op1, rn, pass, MO_64);
-read_vec_element(s, tcg_op2, rm, pass, MO_64);
-
-if (!is_u) {
-switch (size) {
-case 0: /* AND */
-tcg_gen_and_i64(tcg_res[pass], tcg_op1, tcg_op2);
-break;
-case 1: /* BIC */
-tcg_gen_andc_i64(tcg_res[pass], tcg_op1, tcg_op2);
-break;
-case 2: /* ORR */
-tcg_gen_or_i64(tcg_res[pass], tcg_op1, tcg_op2);
-break;
-case 3: /* ORN */
-tcg_gen_orc_i64(tcg_res[pass], tcg_op1, tcg_op2);
-break;
-}
-} else {
-if (size != 0) {
-/* B* ops need res loaded to operate on */
-

[Qemu-devel] [PATCH v3 6/6] tcg/aarch64: Add vector operations

[Richard Henderson]

Signed-off-by: Richard Henderson 
---
 tcg/aarch64/tcg-target.h |  20 ++-
 tcg/aarch64/tcg-target.inc.c | 340 +--
 2 files changed, 315 insertions(+), 45 deletions(-)

diff --git a/tcg/aarch64/tcg-target.h b/tcg/aarch64/tcg-target.h
index c2525066ab..c3e8c4480f 100644
--- a/tcg/aarch64/tcg-target.h
+++ b/tcg/aarch64/tcg-target.h
@@ -31,13 +31,22 @@ typedef enum {
 TCG_REG_SP = 31,
 TCG_REG_XZR = 31,
 
+TCG_REG_V0 = 32, TCG_REG_V1, TCG_REG_V2, TCG_REG_V3,
+TCG_REG_V4, TCG_REG_V5, TCG_REG_V6, TCG_REG_V7,
+TCG_REG_V8, TCG_REG_V9, TCG_REG_V10, TCG_REG_V11,
+TCG_REG_V12, TCG_REG_V13, TCG_REG_V14, TCG_REG_V15,
+TCG_REG_V16, TCG_REG_V17, TCG_REG_V18, TCG_REG_V19,
+TCG_REG_V20, TCG_REG_V21, TCG_REG_V22, TCG_REG_V23,
+TCG_REG_V24, TCG_REG_V25, TCG_REG_V26, TCG_REG_V27,
+TCG_REG_V28, TCG_REG_V29, TCG_REG_V30, TCG_REG_V31,
+
 /* Aliases.  */
 TCG_REG_FP = TCG_REG_X29,
 TCG_REG_LR = TCG_REG_X30,
 TCG_AREG0  = TCG_REG_X19,
 } TCGReg;
 
-#define TCG_TARGET_NB_REGS 32
+#define TCG_TARGET_NB_REGS 64
 
 /* used for function call generation */
 #define TCG_REG_CALL_STACK  TCG_REG_SP
@@ -113,6 +122,15 @@ typedef enum {
 #define TCG_TARGET_HAS_mulsh_i641
 #define TCG_TARGET_HAS_direct_jump  1
 
+#define TCG_TARGET_HAS_v64  1
+#define TCG_TARGET_HAS_v128 1
+#define TCG_TARGET_HAS_v256 0
+
+#define TCG_TARGET_HAS_andc_vec 1
+#define TCG_TARGET_HAS_orc_vec  1
+#define TCG_TARGET_HAS_not_vec  1
+#define TCG_TARGET_HAS_neg_vec  1
+
 #define TCG_TARGET_DEFAULT_MO (0)
 
 static inline void flush_icache_range(uintptr_t start, uintptr_t stop)
diff --git a/tcg/aarch64/tcg-target.inc.c b/tcg/aarch64/tcg-target.inc.c
index 150530f30e..4b401cfe6c 100644
--- a/tcg/aarch64/tcg-target.inc.c
+++ b/tcg/aarch64/tcg-target.inc.c
@@ -20,10 +20,15 @@ QEMU_BUILD_BUG_ON(TCG_TYPE_I32 != 0 || TCG_TYPE_I64 != 1);
 
 #ifdef CONFIG_DEBUG_TCG
 static const char * const tcg_target_reg_names[TCG_TARGET_NB_REGS] = {
-"%x0", "%x1", "%x2", "%x3", "%x4", "%x5", "%x6", "%x7",
-"%x8", "%x9", "%x10", "%x11", "%x12", "%x13", "%x14", "%x15",
-"%x16", "%x17", "%x18", "%x19", "%x20", "%x21", "%x22", "%x23",
-"%x24", "%x25", "%x26", "%x27", "%x28", "%fp", "%x30", "%sp",
+"x0", "x1", "x2", "x3", "x4", "x5", "x6", "x7",
+"x8", "x9", "x10", "x11", "x12", "x13", "x14", "x15",
+"x16", "x17", "x18", "x19", "x20", "x21", "x22", "x23",
+"x24", "x25", "x26", "x27", "x28", "fp", "x30", "sp",
+
+"v0", "v1", "v2", "v3", "v4", "v5", "v6", "v7",
+"v8", "v9", "v10", "v11", "v12", "v13", "v14", "v15",
+"v16", "v17", "v18", "v19", "v20", "v21", "v22", "v23",
+"v24", "v25", "v26", "v27", "v28", "fp", "v30", "v31",
 };
 #endif /* CONFIG_DEBUG_TCG */
 
@@ -43,6 +48,14 @@ static const int tcg_target_reg_alloc_order[] = {
 /* X19 reserved for AREG0 */
 /* X29 reserved as fp */
 /* X30 reserved as temporary */
+
+TCG_REG_V0, TCG_REG_V1, TCG_REG_V2, TCG_REG_V3,
+TCG_REG_V4, TCG_REG_V5, TCG_REG_V6, TCG_REG_V7,
+/* V8 - V15 are call-saved, and skipped.  */
+TCG_REG_V16, TCG_REG_V17, TCG_REG_V18, TCG_REG_V19,
+TCG_REG_V20, TCG_REG_V21, TCG_REG_V22, TCG_REG_V23,
+TCG_REG_V24, TCG_REG_V25, TCG_REG_V26, TCG_REG_V27,
+TCG_REG_V28, TCG_REG_V29, TCG_REG_V30, TCG_REG_V31,
 };
 
 static const int tcg_target_call_iarg_regs[8] = {
@@ -119,10 +132,14 @@ static const char 
*target_parse_constraint(TCGArgConstraint *ct,
const char *ct_str, TCGType type)
 {
 switch (*ct_str++) {
-case 'r':
+case 'r': /* general registers */
 ct->ct |= TCG_CT_REG;
 ct->u.regs = 0xu;
 break;
+case 'w': /* advsimd registers */
+ct->ct |= TCG_CT_REG;
+ct->u.regs = 0xull;
+break;
 case 'l': /* qemu_ld / qemu_st address, data_reg */
 ct->ct |= TCG_CT_REG;
 ct->u.regs = 0xu;
@@ -290,6 +307,12 @@ typedef enum {
 I3312_LDRSHX= 0x3800 | LDST_LD_S_X << 22 | MO_16 << 30,
 I3312_LDRSWX= 0x3800 | LDST_LD_S_X << 22 | MO_32 << 30,
 
+I3312_LDRVD = 0x3c00 | LDST_LD << 22 | MO_64 << 30,
+I3312_STRVD = 0x3c00 | LDST_ST << 22 | MO_64 << 30,
+
+I3312_LDRVQ = 0x3c00 | 3 << 22 | 0 << 30,
+I3312_STRVQ = 0x3c00 | 2 << 22 | 0 << 30,
+
 I3312_TO_I3310  = 0x00200800,
 I3312_TO_I3313  = 0x0100,
 
@@ -374,8 +397,33 @@ typedef enum {
 I3510_EON   = 0x4a20,
 I3510_ANDS  = 0x6a00,
 
-NOP = 0xd503201f,
+/* AdvSIMD modified immediate */
+I3606_MOVI  = 0x0f000400,
+
+/* AdvSIMD three same.  */
+I3616_ADD_B = 0x0e208400,
+I3616_ADD_H = 0x0e608400,
+I3616_ADD_S = 0x0ea08400,
+I3616_ADD_D = 0x4ee08400,
+I3616_AND   = 

[Qemu-devel] [PATCH v3 3/6] target/arm: Align vector registers

[Richard Henderson]

Signed-off-by: Richard Henderson 
---
 target/arm/cpu.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 98b9b26fd3..c346bd148f 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -486,7 +486,7 @@ typedef struct CPUARMState {
  * the two execution states, and means we do not need to explicitly
  * map these registers when changing states.
  */
-float64 regs[64];
+float64 regs[64] QEMU_ALIGNED(16);
 
 uint32_t xregs[16];
 /* We store these fpcsr fields separately for convenience.  */
-- 
2.13.5

[Qemu-devel] [PATCH v3 0/6] TCG vectorization and example conversion

[Richard Henderson]

Now addressing the complex vector op issue.  I now expose TCGv_vec
to target front-ends, but opaque wrt the vector size.  One can thus
compose vector operations, as demonstrated in target/arm/.

The actual host vector length now becomes an argument to the *_vec
opcodes.  It's a little awkward, but does prevent an explosion of
opcode values.

All R-b dropped because all patches rewritten or heavily modified.

Whacha think?


r~


Richard Henderson (6):
  tcg: Add types and operations for host vectors
  tcg: Add vector expanders
  target/arm: Align vector registers
  target/arm: Use vector infrastructure for aa64 add/sub/logic
  tcg/i386: Add vector operations
  tcg/aarch64: Add vector operations

 Makefile.target  |   2 +-
 accel/tcg/tcg-runtime.h  |  24 ++
 target/arm/cpu.h |   2 +-
 tcg/aarch64/tcg-target.h |  20 +-
 tcg/i386/tcg-target.h|  36 +-
 tcg/tcg-gvec-desc.h  |  49 +++
 tcg/tcg-op-gvec.h| 143 
 tcg/tcg-op.h |  26 ++
 tcg/tcg-opc.h|  37 ++
 tcg/tcg.h|  34 ++
 accel/tcg/tcg-runtime-gvec.c | 255 +
 target/arm/translate-a64.c   | 216 +++
 tcg/aarch64/tcg-target.inc.c | 340 ++---
 tcg/i386/tcg-target.inc.c| 423 ++---
 tcg/tcg-op-gvec.c| 853 +++
 tcg/tcg-op.c | 234 
 tcg/tcg.c|  77 +++-
 accel/tcg/Makefile.objs  |   2 +-
 tcg/README   |  46 +++
 19 files changed, 2651 insertions(+), 168 deletions(-)
 create mode 100644 tcg/tcg-gvec-desc.h
 create mode 100644 tcg/tcg-op-gvec.h
 create mode 100644 accel/tcg/tcg-runtime-gvec.c
 create mode 100644 tcg/tcg-op-gvec.c

-- 
2.13.5

Re: [Qemu-devel] [PULL 00/11] Ide patches

[no-reply]

Hi,

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

Subject: [Qemu-devel] [PULL 00/11] Ide patches
Message-id: 20170916010330.10435-1-js...@redhat.com
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
bc6172dc94 AHCI: remove DPRINTF macro
2ea9b926e3 AHCI: pretty-print FIS to buffer instead of stderr
d23d77942b AHCI: Rework IRQ constants
c3e3883d53 AHCI: Replace DPRINTF with trace-events
a3f8dc5d3c IDE: replace DEBUG_AIO with trace events
3c992d8a98 ATAPI: Replace DEBUG_IDE_ATAPI with tracing events
3beaa3f939 IDE: add tracing for data ports
1994e49cc7 IDE: Add register hints to tracing
a446948ea3 IDE: replace DEBUG_IDE with tracing system
8d2b13d3da hw/ide/microdrive: Mark the dscm1 device with user_creatable = 
false
9bc5607864 ide: ahci: unparent children buses before freeing their memory

=== OUTPUT BEGIN ===
Checking PATCH 1/11: ide: ahci: unparent children buses before freeing their 
memory...
Checking PATCH 2/11: hw/ide/microdrive: Mark the dscm1 device with 
user_creatable = false...
Checking PATCH 3/11: IDE: replace DEBUG_IDE with tracing system...
ERROR: spaces required around that '|' (ctx:VxV)
#146: FILE: hw/ide/core.c:1197:
+if (reg_num != 7 && (s->status & (BUSY_STAT|DRQ_STAT))) {
^

total: 1 errors, 0 warnings, 337 lines checked

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

Checking PATCH 4/11: IDE: Add register hints to tracing...
Checking PATCH 5/11: IDE: add tracing for data ports...
Checking PATCH 6/11: ATAPI: Replace DEBUG_IDE_ATAPI with tracing events...
Checking PATCH 7/11: IDE: replace DEBUG_AIO with trace events...
Checking PATCH 8/11: AHCI: Replace DPRINTF with trace-events...
ERROR: Hex numbers must be prefixed with '0x'
#548: FILE: hw/ide/trace-events:91:
+handle_reg_h2d_fis_pmp(void *s, int port, char b0, char b1, char b2) 
"ahci(%p)[%d]: Port Multiplier not supported, FIS: 0x%02x-%02x-%02x"

ERROR: Hex numbers must be prefixed with '0x'
#549: FILE: hw/ide/trace-events:92:
+handle_reg_h2d_fis_res(void *s, int port, char b0, char b1, char b2) 
"ahci(%p)[%d]: Reserved flags set in H2D Register FIS, FIS: 0x%02x-%02x-%02x"

ERROR: Hex numbers must be prefixed with '0x'
#555: FILE: hw/ide/trace-events:98:
+handle_cmd_unhandled_fis(void *s, int port, uint8_t b0, uint8_t b1, uint8_t 
b2) "ahci(%p)[%d]: unhandled FIS type. cmd_fis: 0x%02x-%02x-%02x"

total: 3 errors, 0 warnings, 496 lines checked

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

Checking PATCH 9/11: AHCI: Rework IRQ constants...
Checking PATCH 10/11: AHCI: pretty-print FIS to buffer instead of stderr...
WARNING: line over 80 characters
#60: FILE: hw/ide/ahci.c:1206:
+char *pretty_fis = ahci_pretty_buffer_fis(ide_state->io_buffer, 
0x10);

total: 0 errors, 1 warnings, 60 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 11/11: AHCI: remove DPRINTF macro...
=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-de...@freelists.org

[Qemu-devel] [PULL 08/11] AHCI: Replace DPRINTF with trace-events

[John Snow]

There are a few hangers-on that will be dealt with individually
in forthcoming patches.

Signed-off-by: John Snow 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Eric Blake 
Reviewed-by: Stefan Hajnoczi 
Message-id: 20170901001502.29915-7-js...@redhat.com
Signed-off-by: John Snow 
---
 hw/ide/ahci.c   | 157 +++-
 hw/ide/trace-events |  49 
 2 files changed, 117 insertions(+), 89 deletions(-)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index ccbe091..9d2c8ded 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -35,6 +35,7 @@
 #include "hw/ide/ahci_internal.h"
 
 #define DEBUG_AHCI 0
+#include "trace.h"
 
 #define DPRINTF(port, fmt, ...) \
 do { \
@@ -114,9 +115,9 @@ static uint32_t  ahci_port_read(AHCIState *s, int port, int 
offset)
 default:
 val = 0;
 }
-DPRINTF(port, "offset: 0x%x val: 0x%x\n", offset, val);
-return val;
 
+trace_ahci_port_read(s, port, offset, val);
+return val;
 }
 
 static void ahci_irq_raise(AHCIState *s, AHCIDevice *dev)
@@ -125,7 +126,7 @@ static void ahci_irq_raise(AHCIState *s, AHCIDevice *dev)
 PCIDevice *pci_dev = (PCIDevice *) object_dynamic_cast(OBJECT(dev_state),
TYPE_PCI_DEVICE);
 
-DPRINTF(0, "raise irq\n");
+trace_ahci_irq_raise(s);
 
 if (pci_dev && msi_enabled(pci_dev)) {
 msi_notify(pci_dev, 0);
@@ -140,7 +141,7 @@ static void ahci_irq_lower(AHCIState *s, AHCIDevice *dev)
 PCIDevice *pci_dev = (PCIDevice *) object_dynamic_cast(OBJECT(dev_state),
TYPE_PCI_DEVICE);
 
-DPRINTF(0, "lower irq\n");
+trace_ahci_irq_lower(s);
 
 if (!pci_dev || !msi_enabled(pci_dev)) {
 qemu_irq_lower(s->irq);
@@ -150,8 +151,7 @@ static void ahci_irq_lower(AHCIState *s, AHCIDevice *dev)
 static void ahci_check_irq(AHCIState *s)
 {
 int i;
-
-DPRINTF(-1, "check irq %#x\n", s->control_regs.irqstatus);
+uint32_t old_irq = s->control_regs.irqstatus;
 
 s->control_regs.irqstatus = 0;
 for (i = 0; i < s->ports; i++) {
@@ -160,7 +160,7 @@ static void ahci_check_irq(AHCIState *s)
 s->control_regs.irqstatus |= (1 << i);
 }
 }
-
+trace_ahci_check_irq(s, old_irq, s->control_regs.irqstatus);
 if (s->control_regs.irqstatus &&
 (s->control_regs.ghc & HOST_CTL_IRQ_EN)) {
 ahci_irq_raise(s, NULL);
@@ -240,7 +240,7 @@ static void  ahci_port_write(AHCIState *s, int port, int 
offset, uint32_t val)
 {
 AHCIPortRegs *pr = >dev[port].port_regs;
 
-DPRINTF(port, "offset: 0x%x val: 0x%x\n", offset, val);
+trace_ahci_port_write(s, port, offset, val);
 switch (offset) {
 case PORT_LST_ADDR:
 pr->lst_addr = val;
@@ -341,8 +341,6 @@ static uint64_t ahci_mem_read_32(void *opaque, hwaddr addr)
 val = s->control_regs.version;
 break;
 }
-
-DPRINTF(-1, "(addr 0x%08X), val 0x%08X\n", (unsigned) addr, val);
 } else if ((addr >= AHCI_PORT_REGS_START_ADDR) &&
(addr < (AHCI_PORT_REGS_START_ADDR +
 (s->ports * AHCI_PORT_ADDR_OFFSET_LEN {
@@ -350,6 +348,7 @@ static uint64_t ahci_mem_read_32(void *opaque, hwaddr addr)
  addr & AHCI_PORT_ADDR_OFFSET_MASK);
 }
 
+trace_ahci_mem_read_32(s, addr, val);
 return val;
 }
 
@@ -379,8 +378,7 @@ static uint64_t ahci_mem_read(void *opaque, hwaddr addr, 
unsigned size)
 val = (hi << 32 | lo) >> (ofst * 8);
 }
 
-DPRINTF(-1, "addr=0x%" HWADDR_PRIx " val=0x%" PRIx64 ", size=%d\n",
-addr, val, size);
+trace_ahci_mem_read(opaque, size, addr, val);
 return val;
 }
 
@@ -390,8 +388,7 @@ static void ahci_mem_write(void *opaque, hwaddr addr,
 {
 AHCIState *s = opaque;
 
-DPRINTF(-1, "addr=0x%" HWADDR_PRIx " val=0x%" PRIx64 ", size=%d\n",
-addr, val, size);
+trace_ahci_mem_write(s, size, addr, val);
 
 /* Only aligned reads are allowed on AHCI */
 if (addr & 3) {
@@ -401,15 +398,12 @@ static void ahci_mem_write(void *opaque, hwaddr addr,
 }
 
 if (addr < AHCI_GENERIC_HOST_CONTROL_REGS_MAX_ADDR) {
-DPRINTF(-1, "(addr 0x%08X), val 0x%08"PRIX64"\n", (unsigned) addr, 
val);
-
 switch (addr) {
 case HOST_CAP: /* R/WO, RO */
 /* FIXME handle R/WO */
 break;
 case HOST_CTL: /* R/W */
 if (val & HOST_CTL_RESET) {
-DPRINTF(-1, "HBA Reset\n");
 ahci_reset(s);
 } else {
 s->control_regs.ghc = (val & 0x3) | HOST_CTL_AHCI_EN;
@@ -427,7 +421,7 @@ static void ahci_mem_write(void *opaque, hwaddr addr,
 /* FIXME report write? */
 break;
 default:
- 

[Qemu-devel] [PULL 10/11] AHCI: pretty-print FIS to buffer instead of stderr

[John Snow]

The current FIS printing routines dump the FIS to screen. adjust this
such that it dumps to buffer instead, then use this ability to have
FIS dump mechanisms via trace-events instead of compiled defines.

Signed-off-by: John Snow 
Reviewed-by: Stefan Hajnoczi 
Reviewed-by: Eric Blake 
Reviewed-by: Philippe Mathieu-Daudé 
Message-id: 20170901001502.29915-9-js...@redhat.com
Signed-off-by: John Snow 
---
 hw/ide/ahci.c   | 28 ++--
 hw/ide/trace-events |  4 
 2 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 2dfcab9..dfc05be 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -644,20 +644,21 @@ static void ahci_reset_port(AHCIState *s, int port)
 ahci_init_d2h(d);
 }
 
-static void debug_print_fis(uint8_t *fis, int cmd_len)
+/* Buffer pretty output based on a raw FIS structure. */
+static char *ahci_pretty_buffer_fis(uint8_t *fis, int cmd_len)
 {
-#if DEBUG_AHCI
 int i;
+GString *s = g_string_new("FIS:");
 
-fprintf(stderr, "fis:");
 for (i = 0; i < cmd_len; i++) {
 if ((i & 0xf) == 0) {
-fprintf(stderr, "\n%02x:",i);
+g_string_append_printf(s, "\n0x%02x: ", i);
 }
-fprintf(stderr, "%02x ",fis[i]);
+g_string_append_printf(s, "%02x ", fis[i]);
 }
-fprintf(stderr, "\n");
-#endif
+g_string_append_c(s, '\n');
+
+return g_string_free(s, FALSE);
 }
 
 static bool ahci_map_fis_address(AHCIDevice *ad)
@@ -1201,7 +1202,11 @@ static void handle_reg_h2d_fis(AHCIState *s, int port,
  * table to ide_state->io_buffer */
 if (opts & AHCI_CMD_ATAPI) {
 memcpy(ide_state->io_buffer, _fis[AHCI_COMMAND_TABLE_ACMD], 0x10);
-debug_print_fis(ide_state->io_buffer, 0x10);
+if (trace_event_get_state_backends(TRACE_HANDLE_REG_H2D_FIS_DUMP)) {
+char *pretty_fis = ahci_pretty_buffer_fis(ide_state->io_buffer, 
0x10);
+trace_handle_reg_h2d_fis_dump(s, port, pretty_fis);
+g_free(pretty_fis);
+}
 s->dev[port].done_atapi_packet = false;
 /* XXX send PIO setup FIS */
 }
@@ -1256,8 +1261,11 @@ static int handle_cmd(AHCIState *s, int port, uint8_t 
slot)
 trace_handle_cmd_badmap(s, port, cmd_len);
 goto out;
 }
-debug_print_fis(cmd_fis, 0x80);
-
+if (trace_event_get_state_backends(TRACE_HANDLE_CMD_FIS_DUMP)) {
+char *pretty_fis = ahci_pretty_buffer_fis(cmd_fis, 0x80);
+trace_handle_cmd_fis_dump(s, port, pretty_fis);
+g_free(pretty_fis);
+}
 switch (cmd_fis[0]) {
 case SATA_FIS_TYPE_REGISTER_H2D:
 handle_reg_h2d_fis(s, port, slot, cmd_fis);
diff --git a/hw/ide/trace-events b/hw/ide/trace-events
index e15fd77..601bd97 100644
--- a/hw/ide/trace-events
+++ b/hw/ide/trace-events
@@ -105,3 +105,7 @@ ahci_cmd_done(void *s, int port) "ahci(%p)[%d]: cmd done"
 ahci_reset(void *s) "ahci(%p): HBA reset"
 allwinner_ahci_mem_read(void *s, void *a, uint64_t addr, uint64_t val, 
unsigned size) "ahci(%p): read a=%p addr=0x%"HWADDR_PRIx" val=0x%"PRIx64", 
size=%d"
 allwinner_ahci_mem_write(void *s, void *a, uint64_t addr, uint64_t val, 
unsigned size) "ahci(%p): write a=%p addr=0x%"HWADDR_PRIx" val=0x%"PRIx64", 
size=%d"
+
+# Warning: Verbose
+handle_reg_h2d_fis_dump(void *s, int port, const char *fis) "ahci(%p)[%d]: %s"
+handle_cmd_fis_dump(void *s, int port, const char *fis) "ahci(%p)[%d]: %s"
-- 
2.9.5

[Qemu-devel] [PULL 09/11] AHCI: Rework IRQ constants

[John Snow]

Create a new enum so that we can name the IRQ bits, which will make debugging
them a little nicer if we can print them out. Not handled in this patch, but
this will make it possible to get a nice debug printf detailing exactly which
status bits are set, as it can be multiple at any given time.

As a consequence of this patch, it is no longer possible to set multiple IRQ
codes at once, but nothing was utilizing this ability anyway.

Signed-off-by: John Snow 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Eric Blake 
Reviewed-by: Stefan Hajnoczi 
Message-id: 20170901001502.29915-8-js...@redhat.com
Signed-off-by: John Snow 
---
 hw/ide/ahci.c  | 49 ++---
 hw/ide/ahci_internal.h | 44 +++-
 hw/ide/trace-events|  2 +-
 3 files changed, 74 insertions(+), 21 deletions(-)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 9d2c8ded..2dfcab9 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -56,6 +56,27 @@ static bool ahci_map_fis_address(AHCIDevice *ad);
 static void ahci_unmap_clb_address(AHCIDevice *ad);
 static void ahci_unmap_fis_address(AHCIDevice *ad);
 
+static const char *AHCIPortIRQ_lookup[AHCI_PORT_IRQ__COUNT] = {
+[AHCI_PORT_IRQ_BIT_DHRS] = "DHRS",
+[AHCI_PORT_IRQ_BIT_PSS]  = "PSS",
+[AHCI_PORT_IRQ_BIT_DSS]  = "DSS",
+[AHCI_PORT_IRQ_BIT_SDBS] = "SDBS",
+[AHCI_PORT_IRQ_BIT_UFS]  = "UFS",
+[AHCI_PORT_IRQ_BIT_DPS]  = "DPS",
+[AHCI_PORT_IRQ_BIT_PCS]  = "PCS",
+[AHCI_PORT_IRQ_BIT_DMPS] = "DMPS",
+[8 ... 21]   = "RESERVED",
+[AHCI_PORT_IRQ_BIT_PRCS] = "PRCS",
+[AHCI_PORT_IRQ_BIT_IPMS] = "IPMS",
+[AHCI_PORT_IRQ_BIT_OFS]  = "OFS",
+[25] = "RESERVED",
+[AHCI_PORT_IRQ_BIT_INFS] = "INFS",
+[AHCI_PORT_IRQ_BIT_IFS]  = "IFS",
+[AHCI_PORT_IRQ_BIT_HBDS] = "HBDS",
+[AHCI_PORT_IRQ_BIT_HBFS] = "HBFS",
+[AHCI_PORT_IRQ_BIT_TFES] = "TFES",
+[AHCI_PORT_IRQ_BIT_CPDS] = "CPDS"
+};
 
 static uint32_t  ahci_port_read(AHCIState *s, int port, int offset)
 {
@@ -170,12 +191,18 @@ static void ahci_check_irq(AHCIState *s)
 }
 
 static void ahci_trigger_irq(AHCIState *s, AHCIDevice *d,
- int irq_type)
+ enum AHCIPortIRQ irqbit)
 {
-DPRINTF(d->port_no, "trigger irq %#x -> %x\n",
-irq_type, d->port_regs.irq_mask & irq_type);
+g_assert(irqbit >= 0 && irqbit < 32);
+uint32_t irq = 1U << irqbit;
+uint32_t irqstat = d->port_regs.irq_stat | irq;
 
-d->port_regs.irq_stat |= irq_type;
+trace_ahci_trigger_irq(s, d->port_no,
+   AHCIPortIRQ_lookup[irqbit], irq,
+   d->port_regs.irq_stat, irqstat,
+   irqstat & d->port_regs.irq_mask);
+
+d->port_regs.irq_stat = irqstat;
 ahci_check_irq(s);
 }
 
@@ -718,7 +745,7 @@ static void ahci_write_fis_sdb(AHCIState *s, 
NCQTransferState *ncq_tfs)
 
 /* Trigger IRQ if interrupt bit is set (which currently, it always is) */
 if (sdb_fis->flags & 0x40) {
-ahci_trigger_irq(s, ad, PORT_IRQ_SDB_FIS);
+ahci_trigger_irq(s, ad, AHCI_PORT_IRQ_BIT_SDBS);
 }
 }
 
@@ -761,10 +788,10 @@ static void ahci_write_fis_pio(AHCIDevice *ad, uint16_t 
len)
 ad->port.ifs[0].status;
 
 if (pio_fis[2] & ERR_STAT) {
-ahci_trigger_irq(ad->hba, ad, PORT_IRQ_TF_ERR);
+ahci_trigger_irq(ad->hba, ad, AHCI_PORT_IRQ_BIT_TFES);
 }
 
-ahci_trigger_irq(ad->hba, ad, PORT_IRQ_PIOS_FIS);
+ahci_trigger_irq(ad->hba, ad, AHCI_PORT_IRQ_BIT_PSS);
 }
 
 static bool ahci_write_fis_d2h(AHCIDevice *ad)
@@ -804,10 +831,10 @@ static bool ahci_write_fis_d2h(AHCIDevice *ad)
 ad->port.ifs[0].status;
 
 if (d2h_fis[2] & ERR_STAT) {
-ahci_trigger_irq(ad->hba, ad, PORT_IRQ_TF_ERR);
+ahci_trigger_irq(ad->hba, ad, AHCI_PORT_IRQ_BIT_TFES);
 }
 
-ahci_trigger_irq(ad->hba, ad, PORT_IRQ_D2H_REG_FIS);
+ahci_trigger_irq(ad->hba, ad, AHCI_PORT_IRQ_BIT_DHRS);
 return true;
 }
 
@@ -1082,7 +1109,7 @@ static void process_ncq_command(AHCIState *s, int port, 
uint8_t *cmd_fis,
  "is smaller than the requested size (0x%zx)",
  ncq_tfs->sglist.size, size);
 ncq_err(ncq_tfs);
-ahci_trigger_irq(ad->hba, ad, PORT_IRQ_OVERFLOW);
+ahci_trigger_irq(ad->hba, ad, AHCI_PORT_IRQ_BIT_OFS);
 return;
 } else if (ncq_tfs->sglist.size != size) {
 trace_process_ncq_command_large(s, port, tag,
@@ -1225,7 +1252,7 @@ static int handle_cmd(AHCIState *s, int port, uint8_t 
slot)
 trace_handle_cmd_badfis(s, port);
 return -1;
 } else if (cmd_len != 0x80) {
-ahci_trigger_irq(s, >dev[port], PORT_IRQ_HBUS_ERR);
+ahci_trigger_irq(s, >dev[port], AHCI_PORT_IRQ_BIT_HBFS);
 trace_handle_cmd_badmap(s, port, 

[Qemu-devel] [PULL 11/11] AHCI: remove DPRINTF macro

[John Snow]

Signed-off-by: John Snow 
Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 
Reviewed-by: Eric Blake 
Reviewed-by: Stefan Hajnoczi 
Message-id: 20170901001502.29915-10-js...@redhat.com
Signed-off-by: John Snow 
---
 hw/ide/ahci.c | 9 -
 1 file changed, 9 deletions(-)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index dfc05be..24c65df 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -34,17 +34,8 @@
 #include "hw/ide/pci.h"
 #include "hw/ide/ahci_internal.h"
 
-#define DEBUG_AHCI 0
 #include "trace.h"
 
-#define DPRINTF(port, fmt, ...) \
-do { \
-if (DEBUG_AHCI) { \
-fprintf(stderr, "ahci: %s: [%d] ", __func__, port); \
-fprintf(stderr, fmt, ## __VA_ARGS__); \
-} \
-} while (0)
-
 static void check_cmd(AHCIState *s, int port);
 static int handle_cmd(AHCIState *s, int port, uint8_t slot);
 static void ahci_reset_port(AHCIState *s, int port);
-- 
2.9.5

[Qemu-devel] [PULL 03/11] IDE: replace DEBUG_IDE with tracing system

[John Snow]

Remove the DEBUG_IDE preprocessor definition with something more
appropriately flexible, using the trace-events subsystem.

This will be less prone to bitrot and will more effectively allow
us to target just the functions we care about.

Signed-off-by: John Snow 
Reviewed-by: Eric Blake 
Reviewed-by: Stefan Hajnoczi 
Message-id: 20170901001502.29915-2-js...@redhat.com
Signed-off-by: John Snow 
---
 Makefile.objs |  1 +
 hw/ide/cmd646.c   | 10 +++-
 hw/ide/core.c | 65 +++
 hw/ide/pci.c  | 17 -
 hw/ide/piix.c | 11 
 hw/ide/trace-events   | 35 +
 hw/ide/via.c  | 10 +++-
 include/hw/ide/internal.h |  1 -
 8 files changed, 80 insertions(+), 70 deletions(-)
 create mode 100644 hw/ide/trace-events

diff --git a/Makefile.objs b/Makefile.objs
index 24a4ea0..967c092 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -153,6 +153,7 @@ trace-events-subdirs += hw/acpi
 trace-events-subdirs += hw/arm
 trace-events-subdirs += hw/alpha
 trace-events-subdirs += hw/xen
+trace-events-subdirs += hw/ide
 trace-events-subdirs += ui
 trace-events-subdirs += audio
 trace-events-subdirs += net
diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c
index 9ebb8d4..86b2a8f 100644
--- a/hw/ide/cmd646.c
+++ b/hw/ide/cmd646.c
@@ -32,6 +32,7 @@
 #include "sysemu/dma.h"
 
 #include "hw/ide/pci.h"
+#include "trace.h"
 
 /* CMD646 specific */
 #define CFR0x50
@@ -195,9 +196,8 @@ static uint64_t bmdma_read(void *opaque, hwaddr addr,
 val = 0xff;
 break;
 }
-#ifdef DEBUG_IDE
-printf("bmdma: readb " TARGET_FMT_plx " : 0x%02x\n", addr, val);
-#endif
+
+trace_bmdma_read_cmd646(addr, val);
 return val;
 }
 
@@ -211,9 +211,7 @@ static void bmdma_write(void *opaque, hwaddr addr,
 return;
 }
 
-#ifdef DEBUG_IDE
-printf("bmdma: writeb " TARGET_FMT_plx " : 0x%" PRIx64 "\n", addr, val);
-#endif
+trace_bmdma_write_cmd646(addr, val);
 switch(addr & 3) {
 case 0:
 bmdma_cmd_writeb(bm, val);
diff --git a/hw/ide/core.c b/hw/ide/core.c
index bea3953..31fd593 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -36,6 +36,7 @@
 #include "qemu/cutils.h"
 
 #include "hw/ide/internal.h"
+#include "trace.h"
 
 /* These values were based on a Seagate ST3500418AS but have been modified
to make more sense in QEMU */
@@ -656,10 +657,7 @@ void ide_cancel_dma_sync(IDEState *s)
  * write requests) pending and we can avoid to drain. */
 QLIST_FOREACH(req, >buffered_requests, list) {
 if (!req->orphaned) {
-#ifdef DEBUG_IDE
-printf("%s: invoking cb %p of buffered request %p with"
-   " -ECANCELED\n", __func__, req->original_cb, req);
-#endif
+trace_ide_cancel_dma_sync_buffered(req->original_cb, req);
 req->original_cb(req->original_opaque, -ECANCELED);
 }
 req->orphaned = true;
@@ -678,9 +676,7 @@ void ide_cancel_dma_sync(IDEState *s)
  * aio operation with preadv/pwritev.
  */
 if (s->bus->dma->aiocb) {
-#ifdef DEBUG_IDE
-printf("%s: draining all remaining requests", __func__);
-#endif
+trace_ide_cancel_dma_sync_remaining();
 blk_drain(s->blk);
 assert(s->bus->dma->aiocb == NULL);
 }
@@ -741,9 +737,7 @@ static void ide_sector_read(IDEState *s)
 n = s->req_nb_sectors;
 }
 
-#if defined(DEBUG_IDE)
-printf("sector=%" PRId64 "\n", sector_num);
-#endif
+trace_ide_sector_read(sector_num, n);
 
 if (!ide_sect_range_ok(s, sector_num, n)) {
 ide_rw_error(s);
@@ -1005,14 +999,14 @@ static void ide_sector_write(IDEState *s)
 
 s->status = READY_STAT | SEEK_STAT | BUSY_STAT;
 sector_num = ide_get_sector(s);
-#if defined(DEBUG_IDE)
-printf("sector=%" PRId64 "\n", sector_num);
-#endif
+
 n = s->nsector;
 if (n > s->req_nb_sectors) {
 n = s->req_nb_sectors;
 }
 
+trace_ide_sector_write(sector_num, n);
+
 if (!ide_sect_range_ok(s, sector_num, n)) {
 ide_rw_error(s);
 block_acct_invalid(blk_get_stats(s->blk), BLOCK_ACCT_WRITE);
@@ -1194,18 +1188,17 @@ static void ide_clear_hob(IDEBus *bus)
 void ide_ioport_write(void *opaque, uint32_t addr, uint32_t val)
 {
 IDEBus *bus = opaque;
+IDEState *s = idebus_active_if(bus);
+int reg_num = addr & 7;
 
-#ifdef DEBUG_IDE
-printf("IDE: write addr=0x%x val=0x%02x\n", addr, val);
-#endif
-
-addr &= 7;
+trace_ide_ioport_write(addr, val, bus, s);
 
 /* ignore writes to command block while busy with previous command */
-if (addr != 7 && (idebus_active_if(bus)->status & (BUSY_STAT|DRQ_STAT)))
+if (reg_num != 7 && (s->status & (BUSY_STAT|DRQ_STAT))) {
 return;
+}
 
-switch(addr) {
+switch (reg_num) {
 case 0:
 break;
 case 1:
@@ -1261,9 +1254,7 @@ void 

[Qemu-devel] [PULL 04/11] IDE: Add register hints to tracing

[John Snow]

Name the registers for tracing purposes.

Signed-off-by: John Snow 
Reviewed-by: Eric Blake 
Reviewed-by: Stefan Hajnoczi 
Message-id: 20170901001502.29915-3-js...@redhat.com
Signed-off-by: John Snow 
---
 hw/ide/core.c   | 88 +
 hw/ide/trace-events |  8 ++---
 2 files changed, 72 insertions(+), 24 deletions(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index 31fd593..cb250e6 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -1185,13 +1185,37 @@ static void ide_clear_hob(IDEBus *bus)
 bus->ifs[1].select &= ~(1 << 7);
 }
 
+/* IOport [W]rite [R]egisters */
+enum ATA_IOPORT_WR {
+ATA_IOPORT_WR_DATA = 0,
+ATA_IOPORT_WR_FEATURES = 1,
+ATA_IOPORT_WR_SECTOR_COUNT = 2,
+ATA_IOPORT_WR_SECTOR_NUMBER = 3,
+ATA_IOPORT_WR_CYLINDER_LOW = 4,
+ATA_IOPORT_WR_CYLINDER_HIGH = 5,
+ATA_IOPORT_WR_DEVICE_HEAD = 6,
+ATA_IOPORT_WR_COMMAND = 7,
+ATA_IOPORT_WR_NUM_REGISTERS,
+};
+
+const char *ATA_IOPORT_WR_lookup[ATA_IOPORT_WR_NUM_REGISTERS] = {
+[ATA_IOPORT_WR_DATA] = "Data",
+[ATA_IOPORT_WR_FEATURES] = "Features",
+[ATA_IOPORT_WR_SECTOR_COUNT] = "Sector Count",
+[ATA_IOPORT_WR_SECTOR_NUMBER] = "Sector Number",
+[ATA_IOPORT_WR_CYLINDER_LOW] = "Cylinder Low",
+[ATA_IOPORT_WR_CYLINDER_HIGH] = "Cylinder High",
+[ATA_IOPORT_WR_DEVICE_HEAD] = "Device/Head",
+[ATA_IOPORT_WR_COMMAND] = "Command"
+};
+
 void ide_ioport_write(void *opaque, uint32_t addr, uint32_t val)
 {
 IDEBus *bus = opaque;
 IDEState *s = idebus_active_if(bus);
 int reg_num = addr & 7;
 
-trace_ide_ioport_write(addr, val, bus, s);
+trace_ide_ioport_write(addr, ATA_IOPORT_WR_lookup[reg_num], val, bus, s);
 
 /* ignore writes to command block while busy with previous command */
 if (reg_num != 7 && (s->status & (BUSY_STAT|DRQ_STAT))) {
@@ -1201,43 +1225,43 @@ void ide_ioport_write(void *opaque, uint32_t addr, 
uint32_t val)
 switch (reg_num) {
 case 0:
 break;
-case 1:
-   ide_clear_hob(bus);
+case ATA_IOPORT_WR_FEATURES:
+ide_clear_hob(bus);
 /* NOTE: data is written to the two drives */
-   bus->ifs[0].hob_feature = bus->ifs[0].feature;
-   bus->ifs[1].hob_feature = bus->ifs[1].feature;
+bus->ifs[0].hob_feature = bus->ifs[0].feature;
+bus->ifs[1].hob_feature = bus->ifs[1].feature;
 bus->ifs[0].feature = val;
 bus->ifs[1].feature = val;
 break;
-case 2:
+case ATA_IOPORT_WR_SECTOR_COUNT:
ide_clear_hob(bus);
bus->ifs[0].hob_nsector = bus->ifs[0].nsector;
bus->ifs[1].hob_nsector = bus->ifs[1].nsector;
 bus->ifs[0].nsector = val;
 bus->ifs[1].nsector = val;
 break;
-case 3:
+case ATA_IOPORT_WR_SECTOR_NUMBER:
ide_clear_hob(bus);
bus->ifs[0].hob_sector = bus->ifs[0].sector;
bus->ifs[1].hob_sector = bus->ifs[1].sector;
 bus->ifs[0].sector = val;
 bus->ifs[1].sector = val;
 break;
-case 4:
+case ATA_IOPORT_WR_CYLINDER_LOW:
ide_clear_hob(bus);
bus->ifs[0].hob_lcyl = bus->ifs[0].lcyl;
bus->ifs[1].hob_lcyl = bus->ifs[1].lcyl;
 bus->ifs[0].lcyl = val;
 bus->ifs[1].lcyl = val;
 break;
-case 5:
+case ATA_IOPORT_WR_CYLINDER_HIGH:
ide_clear_hob(bus);
bus->ifs[0].hob_hcyl = bus->ifs[0].hcyl;
bus->ifs[1].hob_hcyl = bus->ifs[1].hcyl;
 bus->ifs[0].hcyl = val;
 bus->ifs[1].hcyl = val;
 break;
-case 6:
+case ATA_IOPORT_WR_DEVICE_HEAD:
/* FIXME: HOB readback uses bit 7 */
 bus->ifs[0].select = (val & ~0x10) | 0xa0;
 bus->ifs[1].select = (val | 0x10) | 0xa0;
@@ -1245,7 +1269,7 @@ void ide_ioport_write(void *opaque, uint32_t addr, 
uint32_t val)
 bus->unit = (val >> 4) & 1;
 break;
 default:
-case 7:
+case ATA_IOPORT_WR_COMMAND:
 /* command */
 ide_exec_cmd(bus, val);
 break;
@@ -2052,6 +2076,30 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
 }
 }
 
+/* IOport [R]ead [R]egisters */
+enum ATA_IOPORT_RR {
+ATA_IOPORT_RR_DATA = 0,
+ATA_IOPORT_RR_ERROR = 1,
+ATA_IOPORT_RR_SECTOR_COUNT = 2,
+ATA_IOPORT_RR_SECTOR_NUMBER = 3,
+ATA_IOPORT_RR_CYLINDER_LOW = 4,
+ATA_IOPORT_RR_CYLINDER_HIGH = 5,
+ATA_IOPORT_RR_DEVICE_HEAD = 6,
+ATA_IOPORT_RR_STATUS = 7,
+ATA_IOPORT_RR_NUM_REGISTERS,
+};
+
+const char *ATA_IOPORT_RR_lookup[ATA_IOPORT_RR_NUM_REGISTERS] = {
+[ATA_IOPORT_RR_DATA] = "Data",
+[ATA_IOPORT_RR_ERROR] = "Error",
+[ATA_IOPORT_RR_SECTOR_COUNT] = "Sector Count",
+[ATA_IOPORT_RR_SECTOR_NUMBER] = "Sector Number",
+[ATA_IOPORT_RR_CYLINDER_LOW] = "Cylinder Low",
+[ATA_IOPORT_RR_CYLINDER_HIGH] = "Cylinder High",
+[ATA_IOPORT_RR_DEVICE_HEAD] = "Device/Head",
+[ATA_IOPORT_RR_STATUS] = "Status"
+};
+
 uint32_t 

[Qemu-devel] [PULL 01/11] ide: ahci: unparent children buses before freeing their memory

[John Snow]

From: Igor Mammedov 

Fixes read after freeing error reported
  https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg04243.html
  Message-Id: <59a56959-ca12-ea75-33fa-ff07eba1b...@redhat.com>

ich9-ahci device creates ide buses and attaches them as QOM children
at realize time, however it forgets to properly clean them up
at unrealize time and frees memory containing these children,
with following call-chain:

   qdev_device_add()
 object_property_set_bool('realized', true)
   device_set_realized()
  ...
  pci_qdev_realize() -> pci_ich9_ahci_realize() -> ahci_realize()
   ...
   s->dev = g_new0(AHCIDevice, ports);
   ...
  AHCIDevice *ad = >dev[i];
  ide_bus_new(>port, sizeof(ad->port), qdev, i, 1);
  ^^^ creates bus in memory allocated by above gnew()
  and adds it as child propety to ahci device
  ...
  hotplug_handler_plug(); -> goto post_realize_fail;
  pci_qdev_unrealize() -> pci_ich9_uninit() -> ahci_uninit()
  ...
   g_free(s->dev);
   ^^^ free memory that holds children busses

  return with error from device_set_realized()

As result later when qdev_device_add() tries to unparent ich9-ahci
after failed device_set_realized(),
object_unparent() -> object_property_del_child()
iterates over existing QOM children including buses added by
ide_bus_new() and tries to unparent them, which causes access to
freed memory where they where located.

Reported-by: Thomas Huth 
Signed-off-by: Igor Mammedov 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Michael S. Tsirkin 
Tested-by: Thomas Huth 
Reviewed-by: John Snow 
Message-id: 1503938085-169486-1-git-send-email-imamm...@redhat.com
Signed-off-by: John Snow 
---
 hw/ide/ahci.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 406a1b5..ccbe091 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -1495,6 +1495,7 @@ void ahci_uninit(AHCIState *s)
 
 ide_exit(s);
 }
+object_unparent(OBJECT(>port));
 }
 
 g_free(s->dev);
-- 
2.9.5

[Qemu-devel] [PULL 07/11] IDE: replace DEBUG_AIO with trace events

[John Snow]

Signed-off-by: John Snow 
Reviewed-by: Stefan Hajnoczi 
Reviewed-by: Eric Blake 
Reviewed-by: Philippe Mathieu-Daudé 
Message-id: 20170901001502.29915-6-js...@redhat.com
Signed-off-by: John Snow 
---
 hw/ide/atapi.c|  5 +
 hw/ide/core.c | 24 +---
 hw/ide/trace-events   |  3 +++
 include/hw/ide/internal.h |  6 --
 4 files changed, 25 insertions(+), 13 deletions(-)

diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
index 9b84a1b..c0509c8 100644
--- a/hw/ide/atapi.c
+++ b/hw/ide/atapi.c
@@ -416,10 +416,7 @@ static void ide_atapi_cmd_read_dma_cb(void *opaque, int 
ret)
 s->io_buffer_size = n * 2048;
 data_offset = 0;
 }
-#ifdef DEBUG_AIO
-printf("aio_read_cd: lba=%u n=%d\n", s->lba, n);
-#endif
-
+trace_ide_atapi_cmd_read_dma_cb_aio(s, s->lba, n);
 s->bus->dma->iov.iov_base = (void *)(s->io_buffer + data_offset);
 s->bus->dma->iov.iov_len = n * ATAPI_SECTOR_SIZE;
 qemu_iovec_init_external(>bus->dma->qiov, >bus->dma->iov, 1);
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 82a19b1..2cc2a08 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -58,6 +58,21 @@ static const int smart_attributes[][12] = {
 { 190,  0x03, 0x00, 0x45, 0x45, 0x1f, 0x00, 0x1f, 0x1f, 0x00, 0x00, 0x32},
 };
 
+const char *IDE_DMA_CMD_lookup[IDE_DMA__COUNT] = {
+[IDE_DMA_READ] = "DMA READ",
+[IDE_DMA_WRITE] = "DMA WRITE",
+[IDE_DMA_TRIM] = "DMA TRIM",
+[IDE_DMA_ATAPI] = "DMA ATAPI"
+};
+
+static const char *IDE_DMA_CMD_str(enum ide_dma_cmd enval)
+{
+if (enval >= 0 && enval < IDE_DMA__COUNT) {
+return IDE_DMA_CMD_lookup[enval];
+}
+return "DMA UNKNOWN CMD";
+}
+
 static void ide_dummy_transfer_stop(IDEState *s);
 
 static void padstr(char *str, const char *src, int len)
@@ -860,10 +875,7 @@ static void ide_dma_cb(void *opaque, int ret)
 goto eot;
 }
 
-#ifdef DEBUG_AIO
-printf("ide_dma_cb: sector_num=%" PRId64 " n=%d, cmd_cmd=%d\n",
-   sector_num, n, s->dma_cmd);
-#endif
+trace_ide_dma_cb(s, sector_num, n, IDE_DMA_CMD_str(s->dma_cmd));
 
 if ((s->dma_cmd == IDE_DMA_READ || s->dma_cmd == IDE_DMA_WRITE) &&
 !ide_sect_range_ok(s, sector_num, n)) {
@@ -2391,9 +2403,7 @@ void ide_bus_reset(IDEBus *bus)
 
 /* pending async DMA */
 if (bus->dma->aiocb) {
-#ifdef DEBUG_AIO
-printf("aio_cancel\n");
-#endif
+trace_ide_bus_reset_aio();
 blk_aio_cancel(bus->dma->aiocb);
 bus->dma->aiocb = NULL;
 }
diff --git a/hw/ide/trace-events b/hw/ide/trace-events
index 8c79a6c..cc8949c 100644
--- a/hw/ide/trace-events
+++ b/hw/ide/trace-events
@@ -18,6 +18,8 @@ ide_cancel_dma_sync_remaining(void) "draining all remaining 
requests"
 ide_sector_read(int64_t sector_num, int nsectors) "sector=%"PRId64" 
nsectors=%d"
 ide_sector_write(int64_t sector_num, int nsectors) "sector=%"PRId64" 
nsectors=%d"
 ide_reset(void *s) "IDEstate %p"
+ide_bus_reset_aio(void) "aio_cancel"
+ide_dma_cb(void *s, int64_t sector_num, int n, const char *dma) "IDEState %p; 
sector_num=%"PRId64" n=%d cmd=%s"
 
 # BMDMA HBAs:
 
@@ -51,5 +53,6 @@ ide_atapi_cmd_reply_end_new(void *s, int status) "IDEState: 
%p; new transfer sta
 ide_atapi_cmd_check_status(void *s) "IDEState: %p"
 ide_atapi_cmd_read(void *s, const char *method, int lba, int nb_sectors) 
"IDEState: %p; read %s: LBA=%d nb_sectors=%d"
 ide_atapi_cmd(void *s, uint8_t cmd) "IDEState: %p; cmd: 0x%02x"
+ide_atapi_cmd_read_dma_cb_aio(void *s, int lba, int n) "IDEState: %p; aio 
read: lba=%d n=%d"
 # Warning: Verbose
 ide_atapi_cmd_packet(void *s, uint16_t limit, const char *packet) "IDEState: 
%p; limit=0x%x packet: %s"
diff --git a/include/hw/ide/internal.h b/include/hw/ide/internal.h
index 74efe8a..db9fde0 100644
--- a/include/hw/ide/internal.h
+++ b/include/hw/ide/internal.h
@@ -14,7 +14,6 @@
 #include "block/scsi.h"
 
 /* debug IDE devices */
-//#define DEBUG_AIO
 #define USE_DMA_CDROM
 
 typedef struct IDEBus IDEBus;
@@ -333,12 +332,15 @@ struct unreported_events {
 };
 
 enum ide_dma_cmd {
-IDE_DMA_READ,
+IDE_DMA_READ = 0,
 IDE_DMA_WRITE,
 IDE_DMA_TRIM,
 IDE_DMA_ATAPI,
+IDE_DMA__COUNT
 };
 
+extern const char *IDE_DMA_CMD_lookup[IDE_DMA__COUNT];
+
 #define ide_cmd_is_read(s) \
((s)->dma_cmd == IDE_DMA_READ)
 
-- 
2.9.5

[Qemu-devel] [PULL 05/11] IDE: add tracing for data ports

[John Snow]

To be used sparingly, but still interesting in the case of small
firmwares designed to reproduce bugs in QEMU IDE.

Signed-off-by: John Snow 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Eric Blake 
Reviewed-by: Stefan Hajnoczi 
Message-id: 20170901001502.29915-4-js...@redhat.com
Signed-off-by: John Snow 
---
 hw/ide/core.c   | 12 +++-
 hw/ide/trace-events |  5 +
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index cb250e6..82a19b1 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -2259,6 +2259,8 @@ void ide_data_writew(void *opaque, uint32_t addr, 
uint32_t val)
 IDEState *s = idebus_active_if(bus);
 uint8_t *p;
 
+trace_ide_data_writew(addr, val, bus, s);
+
 /* PIO data access allowed only when DRQ bit is set. The result of a write
  * during PIO out is indeterminate, just ignore it. */
 if (!(s->status & DRQ_STAT) || ide_is_pio_out(s)) {
@@ -2304,6 +2306,8 @@ uint32_t ide_data_readw(void *opaque, uint32_t addr)
 s->status &= ~DRQ_STAT;
 s->end_transfer_func(s);
 }
+
+trace_ide_data_readw(addr, ret, bus, s);
 return ret;
 }
 
@@ -2313,6 +2317,8 @@ void ide_data_writel(void *opaque, uint32_t addr, 
uint32_t val)
 IDEState *s = idebus_active_if(bus);
 uint8_t *p;
 
+trace_ide_data_writel(addr, val, bus, s);
+
 /* PIO data access allowed only when DRQ bit is set. The result of a write
  * during PIO out is indeterminate, just ignore it. */
 if (!(s->status & DRQ_STAT) || ide_is_pio_out(s)) {
@@ -2343,7 +2349,8 @@ uint32_t ide_data_readl(void *opaque, uint32_t addr)
 /* PIO data access allowed only when DRQ bit is set. The result of a read
  * during PIO in is indeterminate, return 0 and don't move forward. */
 if (!(s->status & DRQ_STAT) || !ide_is_pio_out(s)) {
-return 0;
+ret = 0;
+goto out;
 }
 
 p = s->data_ptr;
@@ -2358,6 +2365,9 @@ uint32_t ide_data_readl(void *opaque, uint32_t addr)
 s->status &= ~DRQ_STAT;
 s->end_transfer_func(s);
 }
+
+out:
+trace_ide_data_readl(addr, ret, bus, s);
 return ret;
 }
 
diff --git a/hw/ide/trace-events b/hw/ide/trace-events
index bff8f39..17bc6f1 100644
--- a/hw/ide/trace-events
+++ b/hw/ide/trace-events
@@ -6,6 +6,11 @@ ide_ioport_read(uint32_t addr, const char *reg, uint32_t val, 
void *bus, void *s
 ide_ioport_write(uint32_t addr, const char *reg, uint32_t val, void *bus, void 
*s) "IDE PIO wr @ 0x%"PRIx32" (%s); val 0x%02"PRIx32"; bus %p IDEState %p"
 ide_status_read(uint32_t addr, uint32_t val, void *bus, void *s)   
"IDE PIO rd @ 0x%"PRIx32" (Alt Status); val 0x%02"PRIx32"; bus %p; IDEState 
%p"
 ide_cmd_write(uint32_t addr, uint32_t val, void *bus)  
"IDE PIO wr @ 0x%"PRIx32" (Device Control); val 0x%02"PRIx32"; bus %p"
+# Warning: verbose
+ide_data_readw(uint32_t addr, uint32_t val, void *bus, void *s)
"IDE PIO rd @ 0x%"PRIx32" (Data: Word); val 0x%04"PRIx32"; bus %p; IDEState 
%p"
+ide_data_writew(uint32_t addr, uint32_t val, void *bus, void *s)   
"IDE PIO wr @ 0x%"PRIx32" (Data: Word); val 0x%04"PRIx32"; bus %p; IDEState 
%p"
+ide_data_readl(uint32_t addr, uint32_t val, void *bus, void *s)
"IDE PIO rd @ 0x%"PRIx32" (Data: Long); val 0x%08"PRIx32"; bus %p; IDEState 
%p"
+ide_data_writel(uint32_t addr, uint32_t val, void *bus, void *s)   
"IDE PIO wr @ 0x%"PRIx32" (Data: Long); val 0x%08"PRIx32"; bus %p; IDEState 
%p"
 # misc
 ide_exec_cmd(void *bus, void *state, uint32_t cmd) "IDE exec cmd: bus %p; 
state %p; cmd 0x%02x"
 ide_cancel_dma_sync_buffered(void *fn, void *req) "invoking cb %p of buffered 
request %p with -ECANCELED"
-- 
2.9.5

[Qemu-devel] [PULL 02/11] hw/ide/microdrive: Mark the dscm1xxxx device with user_creatable = false

[John Snow]

From: Thomas Huth 

QEMU currently aborts with an assertion message when the user is trying
to remove a dscm1 again:

$ aarch64-softmmu/qemu-system-aarch64 -S -M integratorcp -nographic
QEMU 2.9.93 monitor - type 'help' for more information
(qemu) device_add dscm1,id=xyz
(qemu) device_del xyz
**
ERROR:qemu/qdev-monitor.c:872:qdev_unplug: assertion failed: (hotplug_ctrl)
Aborted (core dumped)

Looks like this device has to be wired up in code and is not meant
to be hot-pluggable, so let's mark it with user_creatable = false.

Signed-off-by: Thomas Huth 
Reviewed-by: John Snow 
Message-id: 1503543783-17192-1-git-send-email-th...@redhat.com
Signed-off-by: John Snow 
---
 hw/ide/microdrive.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/ide/microdrive.c b/hw/ide/microdrive.c
index e3fd30e..17917c0 100644
--- a/hw/ide/microdrive.c
+++ b/hw/ide/microdrive.c
@@ -575,12 +575,15 @@ PCMCIACardState *dscm1_init(DriveInfo *dinfo)
 static void dscm1_class_init(ObjectClass *oc, void *data)
 {
 PCMCIACardClass *pcc = PCMCIA_CARD_CLASS(oc);
+DeviceClass *dc = DEVICE_CLASS(oc);
 
 pcc->cis = dscm1_cis;
 pcc->cis_len = sizeof(dscm1_cis);
 
 pcc->attach = dscm1_attach;
 pcc->detach = dscm1_detach;
+/* Reason: Needs to be wired-up in code, see dscm1_init() */
+dc->user_creatable = false;
 }
 
 static const TypeInfo dscm1_type_info = {
-- 
2.9.5

[Qemu-devel] [PULL 06/11] ATAPI: Replace DEBUG_IDE_ATAPI with tracing events

[John Snow]

As part of the ongoing effort to modernize the tracing facilities for
the IDE family of devices, remove PRINTFs in the ATAPI device with
actual tracing events.

Signed-off-by: John Snow 
Reviewed-by: Stefan Hajnoczi 
Reviewed-by: Eric Blake 
Reviewed-by: Philippe Mathieu-Daudé 
Message-id: 20170901001502.29915-5-js...@redhat.com
Signed-off-by: John Snow 
---
 hw/ide/atapi.c| 64 +--
 hw/ide/trace-events   | 15 +++
 include/hw/ide/internal.h |  1 -
 3 files changed, 38 insertions(+), 42 deletions(-)

diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
index fc1d19c..9b84a1b 100644
--- a/hw/ide/atapi.c
+++ b/hw/ide/atapi.c
@@ -27,6 +27,7 @@
 #include "hw/ide/internal.h"
 #include "hw/scsi/scsi.h"
 #include "sysemu/block-backend.h"
+#include "trace.h"
 
 #define ATAPI_SECTOR_BITS (2 + BDRV_SECTOR_BITS)
 #define ATAPI_SECTOR_SIZE (1 << ATAPI_SECTOR_BITS)
@@ -116,9 +117,7 @@ cd_read_sector_sync(IDEState *s)
 block_acct_start(blk_get_stats(s->blk), >acct,
  ATAPI_SECTOR_SIZE, BLOCK_ACCT_READ);
 
-#ifdef DEBUG_IDE_ATAPI
-printf("cd_read_sector_sync: lba=%d\n", s->lba);
-#endif
+trace_cd_read_sector_sync(s->lba);
 
 switch (s->cd_sector_size) {
 case 2048:
@@ -152,9 +151,7 @@ static void cd_read_sector_cb(void *opaque, int ret)
 {
 IDEState *s = opaque;
 
-#ifdef DEBUG_IDE_ATAPI
-printf("cd_read_sector_cb: lba=%d ret=%d\n", s->lba, ret);
-#endif
+trace_cd_read_sector_cb(s->lba, ret);
 
 if (ret < 0) {
 block_acct_failed(blk_get_stats(s->blk), >acct);
@@ -188,9 +185,7 @@ static int cd_read_sector(IDEState *s)
 s->iov.iov_len = ATAPI_SECTOR_SIZE;
 qemu_iovec_init_external(>qiov, >iov, 1);
 
-#ifdef DEBUG_IDE_ATAPI
-printf("cd_read_sector: lba=%d\n", s->lba);
-#endif
+trace_cd_read_sector(s->lba);
 
 block_acct_start(blk_get_stats(s->blk), >acct,
  ATAPI_SECTOR_SIZE, BLOCK_ACCT_READ);
@@ -213,9 +208,7 @@ void ide_atapi_cmd_ok(IDEState *s)
 
 void ide_atapi_cmd_error(IDEState *s, int sense_key, int asc)
 {
-#ifdef DEBUG_IDE_ATAPI
-printf("atapi_cmd_error: sense=0x%x asc=0x%x\n", sense_key, asc);
-#endif
+trace_ide_atapi_cmd_error(s, sense_key, asc);
 s->error = sense_key << 4;
 s->status = READY_STAT | ERR_STAT;
 s->nsector = (s->nsector & ~7) | ATAPI_INT_REASON_IO | ATAPI_INT_REASON_CD;
@@ -252,19 +245,14 @@ static uint16_t atapi_byte_count_limit(IDEState *s)
 void ide_atapi_cmd_reply_end(IDEState *s)
 {
 int byte_count_limit, size, ret;
-#ifdef DEBUG_IDE_ATAPI
-printf("reply: tx_size=%d elem_tx_size=%d index=%d\n",
-   s->packet_transfer_size,
-   s->elementary_transfer_size,
-   s->io_buffer_index);
-#endif
+trace_ide_atapi_cmd_reply_end(s, s->packet_transfer_size,
+  s->elementary_transfer_size,
+  s->io_buffer_index);
 if (s->packet_transfer_size <= 0) {
 /* end of transfer */
 ide_atapi_cmd_ok(s);
 ide_set_irq(s->bus);
-#ifdef DEBUG_IDE_ATAPI
-printf("end of transfer, status=0x%x\n", s->status);
-#endif
+trace_ide_atapi_cmd_reply_end_eot(s, s->status);
 } else {
 /* see if a new sector must be read */
 if (s->lba != -1 && s->io_buffer_index >= s->cd_sector_size) {
@@ -300,9 +288,7 @@ void ide_atapi_cmd_reply_end(IDEState *s)
 /* a new transfer is needed */
 s->nsector = (s->nsector & ~7) | ATAPI_INT_REASON_IO;
 byte_count_limit = atapi_byte_count_limit(s);
-#ifdef DEBUG_IDE_ATAPI
-printf("byte_count_limit=%d\n", byte_count_limit);
-#endif
+trace_ide_atapi_cmd_reply_end_bcl(s, byte_count_limit);
 size = s->packet_transfer_size;
 if (size > byte_count_limit) {
 /* byte count limit must be even if this case */
@@ -324,9 +310,7 @@ void ide_atapi_cmd_reply_end(IDEState *s)
 ide_transfer_start(s, s->io_buffer + s->io_buffer_index - size,
size, ide_atapi_cmd_reply_end);
 ide_set_irq(s->bus);
-#ifdef DEBUG_IDE_ATAPI
-printf("status=0x%x\n", s->status);
-#endif
+trace_ide_atapi_cmd_reply_end_new(s, s->status);
 }
 }
 }
@@ -368,9 +352,7 @@ static void ide_atapi_cmd_read_pio(IDEState *s, int lba, 
int nb_sectors,
 
 static void ide_atapi_cmd_check_status(IDEState *s)
 {
-#ifdef DEBUG_IDE_ATAPI
-printf("atapi_cmd_check_status\n");
-#endif
+trace_ide_atapi_cmd_check_status(s);
 s->error = MC_ERR | (UNIT_ATTENTION << 4);
 s->status = ERR_STAT;
 s->nsector = 0;
@@ -477,10 +459,8 @@ static void ide_atapi_cmd_read_dma(IDEState *s, int lba, 
int nb_sectors,
 static void ide_atapi_cmd_read(IDEState *s, int lba, int nb_sectors,
int sector_size)
 {
-#ifdef 

[Qemu-devel] [PULL 00/11] Ide patches

[John Snow]

The following changes since commit 5faf2d376af3cb4eb92da44c2580e08d39832caa:

  Merge remote-tracking branch 'remotes/huth/tags/check-20170915' into staging 
(2017-09-15 20:29:44 +0100)

are available in the git repository at:

  https://github.com/jnsnow/qemu.git tags/ide-pull-request

for you to fetch changes up to 2a94e34d3ecef91727f467cc012587c632099d40:

  AHCI: remove DPRINTF macro (2017-09-15 20:36:18 -0400)





Igor Mammedov (1):
  ide: ahci: unparent children buses before freeing their memory

John Snow (9):
  IDE: replace DEBUG_IDE with tracing system
  IDE: Add register hints to tracing
  IDE: add tracing for data ports
  ATAPI: Replace DEBUG_IDE_ATAPI with tracing events
  IDE: replace DEBUG_AIO with trace events
  AHCI: Replace DPRINTF with trace-events
  AHCI: Rework IRQ constants
  AHCI: pretty-print FIS to buffer instead of stderr
  AHCI: remove DPRINTF macro

Thomas Huth (1):
  hw/ide/microdrive: Mark the dscm1 device with user_creatable =
false

 Makefile.objs |   1 +
 hw/ide/ahci.c | 244 --
 hw/ide/ahci_internal.h|  44 +++--
 hw/ide/atapi.c|  69 +
 hw/ide/cmd646.c   |  10 +-
 hw/ide/core.c | 185 +++
 hw/ide/microdrive.c   |   3 +
 hw/ide/pci.c  |  17 +---
 hw/ide/piix.c |  11 +--
 hw/ide/trace-events   | 111 +
 hw/ide/via.c  |  10 +-
 include/hw/ide/internal.h |   8 +-
 12 files changed, 441 insertions(+), 272 deletions(-)
 create mode 100644 hw/ide/trace-events

-- 
2.9.5

Re: [Qemu-devel] [PATCH qemu v2 11/13] memory: Share FlatView's and dispatch trees between address spaces

[Alexey Kardashevskiy]

On 15/09/17 19:25, Paolo Bonzini wrote:
> On 15/09/2017 10:40, Alexey Kardashevskiy wrote:
>> +
>> +static bool flatview_can_share(FlatView *old_view, FlatView *new_view)
>> +{
>> +MemoryRegion *old_root = memory_region_unalias_entire(old_view->root);
>> +MemoryRegion *new_root = memory_region_unalias_entire(new_view->root);
>> +
>> +if (old_root == new_root) {
>> +return true;
>> +}
>> +
>> +if (!old_root->enabled && !new_root->enabled) {
>> +return true;
>> +}
>> +
>> +return false;
>> +}
>> +
> 
> I think you can improve this by keeping the root in the address space
> (removing the previous patch).  Instead, the FlatView's root can be the
> one with resolved aliases.  Then old_root is just old_view->root, and if
> an empty FlatView has a NULL root this just becomes
> 
>the_other_view->root == memory_region_unalias_entire(as->root);

Ok!


> 
>>
>> +
>> +/* Build list of unique FlatViews, FV::root is the key */
>> +QTAILQ_FOREACH(old_view, _views, flat_views_link) {
>> +found = false;
>> +QTAILQ_FOREACH(new_view, _tmp, flat_views_link) {
>> +if (flatview_can_share(old_view, new_view)) {
>> +found = true;
>> +break;
>> +}
>> +}
>> +if (found) {
>> +continue;
>> +}
>> +
>> +new_view = generate_memory_topology(old_view->root);
>> +flatview_render_new(old_view, new_view);
>> +QTAILQ_INSERT_TAIL(_tmp, new_view, flat_views_link);
>> +}
> 
> I don't understand the algorithm here.  Why is it visiting _views
> rather than the list of address spaces?  I would have thought it enough
> to do
> 
>for each address space
> if there is a (new) flat view that we can share
> add a reference
> else
> render a new flat view and add it to fvs_tmp
> 
>flat_views = fvs_tmp;
> 
>for each address space
> old_view = address_space_to_flatview(as);
> find the new flat view to use in fvs_tmp
> address_space_update_topology_pass(..., false);
> address_space_update_topology_pass(..., true);
> QTAILQ_REMOVE(_view->address_spaces, ...);
> atomic_rcu_set(>current_map, new_view);
> flatview_unref(old_view);
> QTAILQ_INSERT_TAIL(_view->address_spaces, ...);
> 
> 
> It's also not clear to me what you need the FlatView's address_spaces
> list for.  (It's probably something trivial that I'm missing, or maybe
> it goes away with the previous change).


It is trivial - the first version added a global list of FVs and shared
them among ASes. Every transactoion_commit would produce a new FV and
replace it in all ASes which old FV was shared among. The decision whether
to share FV or not was made in address_space_init() which a very different
place in code.

In v2 I moved the sharing decision to the commit part and nw having a
global list of FVs and ASes list in each FV seems redundant so I'll remove
it in v3, thanks for pointing out :)

And I'll probably drop FV allocation in address_space_init as it is going
to be rebuild at commit time anyway.


> 
>>
>>  AddressSpace *address_space_init_shareable(MemoryRegion *root, const char 
>> *name)
>>  {
>>  AddressSpace *as;
>>  
>> -QTAILQ_FOREACH(as, _spaces, address_spaces_link) {
>> -if (root == address_space_root(as) && as->malloced) {
>> -as->ref_count++;
>> -return as;
>> -}
>> -}
>> -
>>  as = g_malloc0(sizeof *as);
>>  address_space_init(as, root, name);
>> -as->malloced = true;
>> +
>>  return as;
>>  }
>>  
> 
> This belongs in the next patch;

By "this" you mean removal of "malloced", not the AS loop above, right?

> leaving as->malloced in
> do_address_space_destroy and the reference count in
> address_space_destroy is not a big complication.


But why would we want to leave them anyway?


thanks for the quick review!


-- 
Alexey

Re: [Qemu-devel] [PATCH] MAINTAINERS: Add Python scripts

[Eduardo Habkost]

On Fri, Sep 15, 2017 at 08:17:18PM -0400, John Snow wrote:
> 
> 
> On 09/15/2017 07:07 PM, Eduardo Habkost wrote:
> > Cleber and I are volunteering to review and queue patches for the
> > Python scripts and modules in scripts/.
> > 
> > I'm setting "M: Odd fixes" because not all scripts are actively
> > maintained.
> > 
> > Signed-off-by: Eduardo Habkost 
> > ---
> > Even before this patch is merged, I plan to send a pull request
> > including some patches for the Python code soon.
> > ---
> >  MAINTAINERS | 7 +++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 2c333aba21..2c3b8ecde7 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -1390,6 +1390,13 @@ S: Maintained
> >  F: include/sysemu/cryptodev*.h
> >  F: backends/cryptodev*.c
> >  
> > +Python scripts
> > +M: Eduardo Habkost 
> > +M: Cleber Rosa 
> > +S: Odd fixes
> > +F: scripts/qmp/*
> > +F: scripts/*.py
> > +
> >  QAPI
> >  M: Markus Armbruster 
> >  M: Michael Roth 
> > 
> 
> I rather like the thought of having dedicated Python maintainers who can
> review python code with an eye for what is idiomatic. I am fairly
> certain the python I write is functional, but I'm rarely sure it's what
> a python programmer would do.
> 
> I suppose you are intentionally omitting any python that exists as part
> of the test infrastructure, however?

Initially, yes.  I was trying to include only the stuff we were
actively maintaining.  But later I decided to use "S: Odd fixes"
instead of "S: Maintained", so I guess it won't hurt to include
this:

---
diff --git a/MAINTAINERS b/MAINTAINERS
index 2c3b8ecde7..876ac0df99 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1396,6 +1396,8 @@ M: Cleber Rosa 
 S: Odd fixes
 F: scripts/qmp/*
 F: scripts/*.py
+F: tests/*.py
+K: #!.*python
 
 QAPI
 M: Markus Armbruster 

-- 
Eduardo

Re: [Qemu-devel] [PATCH v2 7/7] mips: update mips_cpu_list() to use object_class_get_list()

[Eduardo Habkost]

On Wed, Aug 30, 2017 at 07:52:25PM -0300, Philippe Mathieu-Daudé wrote:
> while here, move it from translate_init.c to helper.c
> 
> Signed-off-by: Philippe Mathieu-Daudé 
> Tested-by: Igor Mammedov 
> Tested-by: James Hogan 
> ---
>  target/mips/helper.c | 46 
> 
>  target/mips/translate_init.c | 10 --
>  2 files changed, 46 insertions(+), 10 deletions(-)
> 
> diff --git a/target/mips/helper.c b/target/mips/helper.c
> index ea076261af..8d12b0088a 100644
> --- a/target/mips/helper.c
> +++ b/target/mips/helper.c
> @@ -1093,3 +1093,49 @@ void QEMU_NORETURN do_raise_exception_err(CPUMIPSState 
> *env,
>  
>  cpu_loop_exit_restore(cs, pc);
>  }
> +
> +/* Sort alphabetically by type name, except for "any". */
> +static gint mips_cpu_list_compare(gconstpointer a, gconstpointer b)
> +{
> +ObjectClass *class_a = (ObjectClass *)a;
> +ObjectClass *class_b = (ObjectClass *)b;
> +const char *name_a, *name_b;
> +
> +name_a = object_class_get_name(class_a);
> +name_b = object_class_get_name(class_b);
> +if (strcmp(name_a, "any-" TYPE_MIPS_CPU) == 0) {
> +return 1;
> +} else if (strcmp(name_b, "any-" TYPE_MIPS_CPU) == 0) {
> +return -1;
> +} else {
> +return strcmp(name_a, name_b);
> +}

This works, but I'd prefer to do like x86 and have a
MIPSCPUClass::ordering field.

(Or, even better: to add a generic CPUClass::ordering field so
all architectures can use the same method to order the model
list)

> +}
> +
> +static void mips_cpu_list_entry(gpointer data, gpointer user_data)
> +{
> +ObjectClass *oc = data;
> +CPUListState *s = user_data;
> +const char *typename;
> +char *name;
> +
> +typename = object_class_get_name(oc);
> +name = g_strndup(typename, strlen(typename) - strlen("-" TYPE_MIPS_CPU));
> +(*s->cpu_fprintf)(s->file, "  %s\n", name);
> +g_free(name);
> +}
> +
> +void mips_cpu_list(FILE *f, fprintf_function cpu_fprintf)
> +{
> +CPUListState s = {
> +.file = f,
> +.cpu_fprintf = cpu_fprintf,
> +};
> +GSList *list;
> +
> +list = object_class_get_list(TYPE_MIPS_CPU, false);
> +list = g_slist_sort(list, mips_cpu_list_compare);
> +(*cpu_fprintf)(f, "Available CPUs:\n");
> +g_slist_foreach(list, mips_cpu_list_entry, );
> +g_slist_free(list);
> +}

I think it's about time we implement a generic mechanism to list
CPU models using the QOM hierarchy, but this is out of the scope
of this series.


> diff --git a/target/mips/translate_init.c b/target/mips/translate_init.c
> index 8bbded46c4..b75f4c9065 100644
> --- a/target/mips/translate_init.c
> +++ b/target/mips/translate_init.c
> @@ -767,16 +767,6 @@ static const mips_def_t *cpu_mips_find_by_name (const 
> char *name)
>  return NULL;
>  }
>  
> -void mips_cpu_list (FILE *f, fprintf_function cpu_fprintf)
> -{
> -int i;
> -
> -for (i = 0; i < ARRAY_SIZE(mips_defs); i++) {
> -(*cpu_fprintf)(f, "MIPS '%s'\n",
> -   mips_defs[i].name);
> -}
> -}
> -
>  #ifndef CONFIG_USER_ONLY
>  static void no_mmu_init (CPUMIPSState *env, const mips_def_t *def)
>  {
> -- 
> 2.14.1
> 
> 

-- 
Eduardo

Re: [Qemu-devel] [PATCH] MAINTAINERS: Add Python scripts

[John Snow]

On 09/15/2017 07:07 PM, Eduardo Habkost wrote:
> Cleber and I are volunteering to review and queue patches for the
> Python scripts and modules in scripts/.
> 
> I'm setting "M: Odd fixes" because not all scripts are actively
> maintained.
> 
> Signed-off-by: Eduardo Habkost 
> ---
> Even before this patch is merged, I plan to send a pull request
> including some patches for the Python code soon.
> ---
>  MAINTAINERS | 7 +++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 2c333aba21..2c3b8ecde7 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1390,6 +1390,13 @@ S: Maintained
>  F: include/sysemu/cryptodev*.h
>  F: backends/cryptodev*.c
>  
> +Python scripts
> +M: Eduardo Habkost 
> +M: Cleber Rosa 
> +S: Odd fixes
> +F: scripts/qmp/*
> +F: scripts/*.py
> +
>  QAPI
>  M: Markus Armbruster 
>  M: Michael Roth 
> 

I rather like the thought of having dedicated Python maintainers who can
review python code with an eye for what is idiomatic. I am fairly
certain the python I write is functional, but I'm rarely sure it's what
a python programmer would do.

I suppose you are intentionally omitting any python that exists as part
of the test infrastructure, however?

John

Re: [Qemu-devel] [PATCH v2 6/7] mips: replace cpu_mips_init() with cpu_generic_init()

[Eduardo Habkost]

On Wed, Aug 30, 2017 at 07:52:24PM -0300, Philippe Mathieu-Daudé wrote:
> From: Igor Mammedov 
> 
> now cpu_mips_init() reimplements subset of cpu_generic_init()
> tasks, so just drop it and use cpu_generic_init() directly.
> 
> Signed-off-by: Igor Mammedov 
> Reviewed-by: Hervé Poussineau 
> Signed-off-by: Philippe Mathieu-Daudé 
> [PMD: use internal.h instead of cpu.h]
> Tested-by: James Hogan 

Reviewed-by: Eduardo Habkost 

-- 
Eduardo

Re: [Qemu-devel] [PATCH v2 5/7] mips: MIPSCPU model subclasses

[Eduardo Habkost]

On Wed, Aug 30, 2017 at 07:52:23PM -0300, Philippe Mathieu-Daudé wrote:
> From: Igor Mammedov 
> 
> Register separate QOM types for each mips cpu model,
> so it would be possible to reuse generic CPU creation
> routines.
> 
> Signed-off-by: Igor Mammedov 
> Signed-off-by: Philippe Mathieu-Daudé 
> [PMD: use internal.h, use void* to hold cpu_def in MIPSCPUClass,
>  mark MIPSCPU abstract]
> Tested-by: James Hogan 
> ---
>  target/mips/cpu-qom.h|  1 +
>  target/mips/internal.h   | 59 
> 
>  target/mips/cpu.c| 53 ++-
>  target/mips/translate.c  | 13 +-
>  target/mips/translate_init.c | 58 ++-
>  5 files changed, 120 insertions(+), 64 deletions(-)
> 
> diff --git a/target/mips/cpu-qom.h b/target/mips/cpu-qom.h
> index 3f5bf23823..085711d8f9 100644
> --- a/target/mips/cpu-qom.h
> +++ b/target/mips/cpu-qom.h
> @@ -49,6 +49,7 @@ typedef struct MIPSCPUClass {
>  
>  DeviceRealize parent_realize;
>  void (*parent_reset)(CPUState *cpu);
> +const void *cpu_def;

Why void*?  The following should be valid even if you don't
include internal.h:

const struct mips_def_t *cpu_def;


>  } MIPSCPUClass;
>  
>  typedef struct MIPSCPU MIPSCPU;
> diff --git a/target/mips/internal.h b/target/mips/internal.h
> index cf4c9db427..45ded3484c 100644
> --- a/target/mips/internal.h
> +++ b/target/mips/internal.h
> @@ -7,6 +7,65 @@
>  #ifndef MIPS_INTERNAL_H
>  #define MIPS_INTERNAL_H
>  
> +
> +/* MMU types, the first four entries have the same layout as the
> +   CP0C0_MT field.  */
> +enum mips_mmu_types {
> +MMU_TYPE_NONE,
> +MMU_TYPE_R4000,
> +MMU_TYPE_RESERVED,
> +MMU_TYPE_FMT,
> +MMU_TYPE_R3000,
> +MMU_TYPE_R6000,
> +MMU_TYPE_R8000
> +};
> +
> +struct mips_def_t {
> +const char *name;
> +int32_t CP0_PRid;
> +int32_t CP0_Config0;
> +int32_t CP0_Config1;
> +int32_t CP0_Config2;
> +int32_t CP0_Config3;
> +int32_t CP0_Config4;
> +int32_t CP0_Config4_rw_bitmask;
> +int32_t CP0_Config5;
> +int32_t CP0_Config5_rw_bitmask;
> +int32_t CP0_Config6;
> +int32_t CP0_Config7;
> +target_ulong CP0_LLAddr_rw_bitmask;
> +int CP0_LLAddr_shift;
> +int32_t SYNCI_Step;
> +int32_t CCRes;
> +int32_t CP0_Status_rw_bitmask;
> +int32_t CP0_TCStatus_rw_bitmask;
> +int32_t CP0_SRSCtl;
> +int32_t CP1_fcr0;
> +int32_t CP1_fcr31_rw_bitmask;
> +int32_t CP1_fcr31;
> +int32_t MSAIR;
> +int32_t SEGBITS;
> +int32_t PABITS;
> +int32_t CP0_SRSConf0_rw_bitmask;
> +int32_t CP0_SRSConf0;
> +int32_t CP0_SRSConf1_rw_bitmask;
> +int32_t CP0_SRSConf1;
> +int32_t CP0_SRSConf2_rw_bitmask;
> +int32_t CP0_SRSConf2;
> +int32_t CP0_SRSConf3_rw_bitmask;
> +int32_t CP0_SRSConf3;
> +int32_t CP0_SRSConf4_rw_bitmask;
> +int32_t CP0_SRSConf4;
> +int32_t CP0_PageGrain_rw_bitmask;
> +int32_t CP0_PageGrain;
> +target_ulong CP0_EBaseWG_rw_bitmask;
> +int insn_flags;
> +enum mips_mmu_types mmu_type;
> +};
> +
> +extern const struct mips_def_t mips_defs[];
> +extern const int mips_defs_number;
> +
>  enum CPUMIPSMSADataFormat {
>  DF_BYTE = 0,
>  DF_HALF,
> diff --git a/target/mips/cpu.c b/target/mips/cpu.c
> index e3ef835599..84b6f8bf68 100644
> --- a/target/mips/cpu.c
> +++ b/target/mips/cpu.c
> @@ -146,12 +146,37 @@ static void mips_cpu_initfn(Object *obj)
>  CPUState *cs = CPU(obj);
>  MIPSCPU *cpu = MIPS_CPU(obj);
>  CPUMIPSState *env = >env;
> +MIPSCPUClass *mcc = MIPS_CPU_GET_CLASS(obj);
>  
>  cs->env_ptr = env;
>  
>  if (tcg_enabled()) {
>  mips_tcg_init();
>  }
> +
> +if (mcc->cpu_def) {

Why do we need this conditional?  It looks harmless but
unnecessary.

The rest of the patch looks good to me, and if the MIPS
maintainers think the current code is good enough, they have my:

Reviewed-by: Eduardo Habkost 

(Additional questions/comments below)

> +env->cpu_model = mcc->cpu_def;
> +}
> +}
> +
> +static char *mips_cpu_type_name(const char *cpu_model)
> +{
> +return g_strdup_printf("%s-" TYPE_MIPS_CPU, cpu_model);
> +}
> +
> +static ObjectClass *mips_cpu_class_by_name(const char *cpu_model)
> +{
> +ObjectClass *oc;
> +char *typename;
> +
> +if (cpu_model == NULL) {
> +return NULL;
> +}

For later: isn't this check for cpu_model==NULL duplicated on
every single class_by_name implementation?  What about moving it
to cpu_class_by_name()?

> +
> +typename = mips_cpu_type_name(cpu_model);
> +oc = object_class_by_name(typename);
> +g_free(typename);
> +return oc;
>  }
>  
>  static void mips_cpu_class_init(ObjectClass *c, void *data)
> @@ -166,6 +191,7 @@ static void mips_cpu_class_init(ObjectClass *c, void 
> *data)
>  

[Qemu-devel] [PULL 15/15] qemu.py: include debug information on launch error

[Eduardo Habkost]

From: Amador Pahim 

When launching a VM, if an exception happens and the VM is not
initiated, it might be useful to see the qemu command line and
the qemu command output.

This patch creates that message. Notice that self._iolog needs to be
cleaned up in the beginning of the launch() to make sure we will not
expose the qemu log from a previous launch if the current one fails.

Signed-off-by: Amador Pahim 
Message-Id: <20170901112829.2571-6-apa...@redhat.com>
Reviewed-by: Fam Zheng 
Signed-off-by: Eduardo Habkost 
---
 scripts/qemu.py | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/scripts/qemu.py b/scripts/qemu.py
index 9440261ac3..8c67595ec8 100644
--- a/scripts/qemu.py
+++ b/scripts/qemu.py
@@ -187,6 +187,7 @@ class QEMUMachine(object):
 
 def launch(self):
 '''Launch the VM and establish a QMP connection'''
+self._iolog = None
 self._qemu_full_args = None
 devnull = open(os.path.devnull, 'rb')
 qemulog = open(self._qemu_log_path, 'wb')
@@ -206,6 +207,12 @@ class QEMUMachine(object):
 self._popen.wait()
 self._load_io_log()
 self._post_shutdown()
+
+LOG.debug('Error launching VM')
+if self._qemu_full_args:
+LOG.debug('Command: %r', ' '.join(self._qemu_full_args))
+if self._iolog:
+LOG.debug('Output: %r', self._iolog)
 raise
 
 def shutdown(self):
-- 
2.13.5

Re: [Qemu-devel] [PATCH v2 4/7] mips: call cpu_mips_realize_env() from mips_cpu_realizefn()

[Eduardo Habkost]

On Wed, Aug 30, 2017 at 07:52:22PM -0300, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé 
> Tested-by: Igor Mammedov 
> Tested-by: James Hogan 
> ---
>  target/mips/cpu.c   | 3 +++
>  target/mips/translate.c | 1 -
>  2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/target/mips/cpu.c b/target/mips/cpu.c
> index 68bf423e9d..e3ef835599 100644
> --- a/target/mips/cpu.c
> +++ b/target/mips/cpu.c
> @@ -123,6 +123,7 @@ static void mips_cpu_disas_set_info(CPUState *s, 
> disassemble_info *info) {
>  static void mips_cpu_realizefn(DeviceState *dev, Error **errp)
>  {
>  CPUState *cs = CPU(dev);
> +MIPSCPU *cpu = MIPS_CPU(dev);
>  MIPSCPUClass *mcc = MIPS_CPU_GET_CLASS(dev);
>  Error *local_err = NULL;
>  
> @@ -132,6 +133,8 @@ static void mips_cpu_realizefn(DeviceState *dev, Error 
> **errp)
>  return;
>  }
>  
> +cpu_mips_realize_env(>env);
> +

This changes the order between cpu_mips_realize_env() and
cpu_exec_initfn(), but cpu_exec_initfn() don't have anything that
depends on cpu_mips_realize_env() being called first.

Reviewed-by: Eduardo Habkost 

>  cpu_reset(cs);
>  qemu_init_vcpu(cs);
>  
> diff --git a/target/mips/translate.c b/target/mips/translate.c
> index 5fc7979ac5..94c38e8755 100644
> --- a/target/mips/translate.c
> +++ b/target/mips/translate.c
> @@ -20535,7 +20535,6 @@ MIPSCPU *cpu_mips_init(const char *cpu_model)
>  cpu = MIPS_CPU(object_new(TYPE_MIPS_CPU));
>  env = >env;
>  env->cpu_model = def;
> -cpu_mips_realize_env(env);
>  
>  object_property_set_bool(OBJECT(cpu), true, "realized", NULL);
>  
> -- 
> 2.14.1
> 
> 

-- 
Eduardo

[Qemu-devel] [PULL 14/15] qemu.py: improve message on negative exit code

[Eduardo Habkost]

From: Amador Pahim 

The current message shows 'self._args', which contains only part of the
options used in the Qemu command line.

This patch makes the qemu full args list an instance variable and then
uses it in the negative exit code message.

Message was moved outside the 'if is_running' block to make sure it will
be logged if the VM finishes before the call to shutdown().

Signed-off-by: Amador Pahim 
Message-Id: <20170901112829.2571-5-apa...@redhat.com>
[ehabkost: removed superfluous parenthesis]
Reviewed-by: Fam Zheng 
Signed-off-by: Eduardo Habkost 
---
 scripts/qemu.py | 24 +---
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/scripts/qemu.py b/scripts/qemu.py
index c9bcaafe41..9440261ac3 100644
--- a/scripts/qemu.py
+++ b/scripts/qemu.py
@@ -87,6 +87,7 @@ class QEMUMachine(object):
 self._socket_scm_helper = socket_scm_helper
 self._debug = debug
 self._qmp = None
+self._qemu_full_args = None
 
 def __enter__(self):
 return self
@@ -186,13 +187,16 @@ class QEMUMachine(object):
 
 def launch(self):
 '''Launch the VM and establish a QMP connection'''
+self._qemu_full_args = None
 devnull = open(os.path.devnull, 'rb')
 qemulog = open(self._qemu_log_path, 'wb')
 try:
 self._pre_launch()
-args = (self._wrapper + [self._binary] + self._base_args() +
-self._args)
-self._popen = subprocess.Popen(args, stdin=devnull, stdout=qemulog,
+self._qemu_full_args = self._wrapper + [self._binary] +
+self._base_args() + self._args
+self._popen = subprocess.Popen(self._qemu_full_args,
+   stdin=devnull,
+   stdout=qemulog,
stderr=subprocess.STDOUT,
shell=False)
 self._post_launch()
@@ -212,14 +216,20 @@ class QEMUMachine(object):
 self._qmp.close()
 except:
 self._popen.kill()
+self._popen.wait()
 
-exitcode = self._popen.wait()
-if exitcode < 0:
-LOG.warn('qemu received signal %i: %s', -exitcode,
-  ' '.join(self._args))
 self._load_io_log()
 self._post_shutdown()
 
+exitcode = self.exitcode()
+if exitcode is not None and exitcode < 0:
+msg = 'qemu received signal %i: %s'
+if self._qemu_full_args:
+command = ' '.join(self._qemu_full_args)
+else:
+command = ''
+LOG.warn(msg, exitcode, command)
+
 def qmp(self, cmd, conv_keys=True, **args):
 '''Invoke a QMP command and return the response dict'''
 qmp_args = dict()
-- 
2.13.5

[Qemu-devel] [PULL 13/15] qemu.py: use os.path.null instead of /dev/null

[Eduardo Habkost]

From: Amador Pahim 

For increased portability, let's use os.path.devnull.

Signed-off-by: Amador Pahim 
Message-Id: <20170901112829.2571-4-apa...@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Fam Zheng 
Signed-off-by: Eduardo Habkost 
---
 scripts/qemu.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/qemu.py b/scripts/qemu.py
index 8d3d24dd2b..c9bcaafe41 100644
--- a/scripts/qemu.py
+++ b/scripts/qemu.py
@@ -124,7 +124,7 @@ class QEMUMachine(object):
 fd_param = ["%s" % self._socket_scm_helper,
 "%d" % self._qmp.get_sock_fd(),
 "%s" % fd_file_path]
-devnull = open('/dev/null', 'rb')
+devnull = open(os.path.devnull, 'rb')
 proc = subprocess.Popen(fd_param, stdin=devnull, 
stdout=subprocess.PIPE,
 stderr=subprocess.STDOUT)
 output = proc.communicate()[0]
@@ -186,7 +186,7 @@ class QEMUMachine(object):
 
 def launch(self):
 '''Launch the VM and establish a QMP connection'''
-devnull = open('/dev/null', 'rb')
+devnull = open(os.path.devnull, 'rb')
 qemulog = open(self._qemu_log_path, 'wb')
 try:
 self._pre_launch()
-- 
2.13.5

[Qemu-devel] [PULL 08/15] qmp.py: Avoid "has_key" usage

[Eduardo Habkost]

From: Lukáš Doktor 

The "has_key" is deprecated in favor of "__in__" operator.

Signed-off-by: Lukáš Doktor 
Reviewed-by: Eduardo Habkost 
Reviewed-by: Philippe Mathieu-Daudé 
Message-Id: <20170818142613.32394-9-ldok...@redhat.com>
Signed-off-by: Eduardo Habkost 
---
 scripts/qmp/qmp.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/qmp/qmp.py b/scripts/qmp/qmp.py
index 95ff5cc39a..f2f5a9b296 100644
--- a/scripts/qmp/qmp.py
+++ b/scripts/qmp/qmp.py
@@ -197,7 +197,7 @@ class QEMUMonitorProtocol(object):
 Build and send a QMP command to the monitor, report errors if any
 """
 ret = self.cmd(cmd, kwds)
-if ret.has_key('error'):
+if "error" in ret:
 raise Exception(ret['error']['desc'])
 return ret['return']
 
-- 
2.13.5

[Qemu-devel] [PULL 07/15] qmp.py: Use object-based class for QEMUMonitorProtocol

[Eduardo Habkost]

From: Lukáš Doktor 

There is no need to define QEMUMonitorProtocol as old-style class.

Signed-off-by: Lukáš Doktor 
Reviewed-by: Eduardo Habkost 
Message-Id: <20170818142613.32394-8-ldok...@redhat.com>
Signed-off-by: Eduardo Habkost 
---
 scripts/qmp/qmp.py| 2 +-
 scripts/qmp/qmp-shell | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/scripts/qmp/qmp.py b/scripts/qmp/qmp.py
index 782d1ac5df..95ff5cc39a 100644
--- a/scripts/qmp/qmp.py
+++ b/scripts/qmp/qmp.py
@@ -30,7 +30,7 @@ class QMPTimeoutError(QMPError):
 pass
 
 
-class QEMUMonitorProtocol:
+class QEMUMonitorProtocol(object):
 
 #: Socket's error class
 error = socket.error
diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
index 860ffb27f2..be449de621 100755
--- a/scripts/qmp/qmp-shell
+++ b/scripts/qmp/qmp-shell
@@ -106,7 +106,7 @@ class FuzzyJSON(ast.NodeTransformer):
 #   _execute_cmd()). Let's design a better one.
 class QMPShell(qmp.QEMUMonitorProtocol):
 def __init__(self, address, pretty=False):
-qmp.QEMUMonitorProtocol.__init__(self, self.__get_address(address))
+super(QMPShell, self).__init__(self.__get_address(address))
 self._greeting = None
 self._completer = None
 self._pretty = pretty
@@ -281,7 +281,7 @@ class QMPShell(qmp.QEMUMonitorProtocol):
 return True
 
 def connect(self, negotiate):
-self._greeting = qmp.QEMUMonitorProtocol.connect(self, negotiate)
+self._greeting = super(QMPShell, self).connect(negotiate)
 self.__completer_setup()
 
 def show_banner(self, msg='Welcome to the QMP low-level shell!'):
-- 
2.13.5

[Qemu-devel] [PULL 10/15] qtest.py: Few pylint/style fixes

[Eduardo Habkost]

From: Lukáš Doktor 

No actual code changes, just few pylint/style fixes.

Signed-off-by: Lukáš Doktor 
Reviewed-by: John Snow 
Message-Id: <20170818142613.32394-11-ldok...@redhat.com>
Signed-off-by: Eduardo Habkost 
---
 scripts/qtest.py | 11 +--
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/scripts/qtest.py b/scripts/qtest.py
index ab183c0635..df0daf26ca 100644
--- a/scripts/qtest.py
+++ b/scripts/qtest.py
@@ -11,14 +11,11 @@
 # Based on qmp.py.
 #
 
-import errno
 import socket
-import string
 import os
-import subprocess
-import qmp.qmp
 import qemu
 
+
 class QEMUQtestProtocol(object):
 def __init__(self, address, server=False):
 """
@@ -83,8 +80,10 @@ class QEMUQtestMachine(qemu.QEMUMachine):
  socket_scm_helper=None):
 if name is None:
 name = "qemu-%d" % os.getpid()
-super(QEMUQtestMachine, self).__init__(binary, args, name=name, 
test_dir=test_dir,
-   
socket_scm_helper=socket_scm_helper)
+super(QEMUQtestMachine,
+  self).__init__(binary, args, name=name, test_dir=test_dir,
+ socket_scm_helper=socket_scm_helper)
+self._qtest = None
 self._qtest_path = os.path.join(test_dir, name + "-qtest.sock")
 
 def _base_args(self):
-- 
2.13.5

[Qemu-devel] [PULL 05/15] qemu.py: Use custom exceptions rather than Exception

[Eduardo Habkost]

From: Lukáš Doktor 

The naked Exception should not be widely used. It makes sense to be a
bit more specific and use better-suited custom exceptions. As a benefit
we can store the full reply in the exception in case someone needs it
when catching the exception.

Signed-off-by: Lukáš Doktor 
Reviewed-by: Eduardo Habkost 
Message-Id: <20170818142613.32394-6-ldok...@redhat.com>
Signed-off-by: Eduardo Habkost 
---
 scripts/qemu.py | 17 +++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/scripts/qemu.py b/scripts/qemu.py
index bde8da8fe7..7c6802609a 100644
--- a/scripts/qemu.py
+++ b/scripts/qemu.py
@@ -19,6 +19,19 @@ import subprocess
 import qmp.qmp
 
 
+class MonitorResponseError(qmp.qmp.QMPError):
+'''
+Represents erroneous QMP monitor reply
+'''
+def __init__(self, reply):
+try:
+desc = reply["error"]["desc"]
+except KeyError:
+desc = reply
+super(MonitorResponseError, self).__init__(desc)
+self.reply = reply
+
+
 class QEMUMachine(object):
 '''A QEMU VM
 
@@ -213,9 +226,9 @@ class QEMUMachine(object):
 '''
 reply = self.qmp(cmd, conv_keys, **args)
 if reply is None:
-raise Exception("Monitor is closed")
+raise qmp.qmp.QMPError("Monitor is closed")
 if "error" in reply:
-raise Exception(reply["error"]["desc"])
+raise MonitorResponseError(reply)
 return reply["return"]
 
 def get_qmp_event(self, wait=False):
-- 
2.13.5

[Qemu-devel] [PULL 09/15] qmp.py: Avoid overriding a builtin object

[Eduardo Habkost]

From: Lukáš Doktor 

The "id" is a builtin method to get object's identity and should not be
overridden. This might bring some issues in case someone was directly
calling "cmd(..., id=id)" but I haven't found such usage on brief search
for "cmd\(.*id=".

Signed-off-by: Lukáš Doktor 
Reviewed-by: Eduardo Habkost 
Message-Id: <20170818142613.32394-10-ldok...@redhat.com>
Signed-off-by: Eduardo Habkost 
---
 scripts/qmp/qmp.py | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/scripts/qmp/qmp.py b/scripts/qmp/qmp.py
index f2f5a9b296..ef12e8a1a0 100644
--- a/scripts/qmp/qmp.py
+++ b/scripts/qmp/qmp.py
@@ -177,19 +177,19 @@ class QEMUMonitorProtocol(object):
 print >>sys.stderr, "QMP:<<< %s" % resp
 return resp
 
-def cmd(self, name, args=None, id=None):
+def cmd(self, name, args=None, cmd_id=None):
 """
 Build a QMP command and send it to the QMP Monitor.
 
 @param name: command name (string)
 @param args: command arguments (dict)
-@param id: command id (dict, list, string or int)
+@param cmd_id: command id (dict, list, string or int)
 """
 qmp_cmd = {'execute': name}
 if args:
 qmp_cmd['arguments'] = args
-if id:
-qmp_cmd['id'] = id
+if cmd_id:
+qmp_cmd['id'] = cmd_id
 return self.cmd_obj(qmp_cmd)
 
 def command(self, cmd, **kwds):
-- 
2.13.5

[Qemu-devel] [PULL 04/15] qemu.py: Simplify QMP key-conversion

[Eduardo Habkost]

From: Lukáš Doktor 

The QMP key conversion consist of '_'s to be replaced with '-'s, which
can easily be done by a single `str.replace` method which is faster and
does not require `string` module import.

Signed-off-by: Lukáš Doktor 
Reviewed-by: Eduardo Habkost 
Message-Id: <20170818142613.32394-5-ldok...@redhat.com>
Reviewed-by: Cleber Rosa 
Signed-off-by: Eduardo Habkost 
---
 scripts/qemu.py | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/scripts/qemu.py b/scripts/qemu.py
index d8c169b31e..bde8da8fe7 100644
--- a/scripts/qemu.py
+++ b/scripts/qemu.py
@@ -13,7 +13,6 @@
 #
 
 import errno
-import string
 import os
 import sys
 import subprocess
@@ -195,14 +194,12 @@ class QEMUMachine(object):
 self._load_io_log()
 self._post_shutdown()
 
-underscore_to_dash = string.maketrans('_', '-')
-
 def qmp(self, cmd, conv_keys=True, **args):
 '''Invoke a QMP command and return the response dict'''
 qmp_args = dict()
 for key, value in args.iteritems():
 if conv_keys:
-qmp_args[key.translate(self.underscore_to_dash)] = value
+qmp_args[key.replace('_', '-')] = value
 else:
 qmp_args[key] = value
 
-- 
2.13.5

[Qemu-devel] [PULL 11/15] qemu.py: fix is_running() return before first launch()

[Eduardo Habkost]

From: Amador Pahim 

is_running() returns None when called before the first time we
call launch():

>>> import qemu
>>> vm = qemu.QEMUMachine('qemu-system-x86_64')
>>> vm.is_running()
>>>

It should return False instead. This patch fixes that.

For consistence, this patch removes the parenthesis from the
second clause as it's not really needed.

Signed-off-by: Amador Pahim 
Message-Id: <20170901112829.2571-2-apa...@redhat.com>
Reviewed-by: Fam Zheng 
Signed-off-by: Eduardo Habkost 
---
 scripts/qemu.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/qemu.py b/scripts/qemu.py
index 7c6802609a..f80b008f7f 100644
--- a/scripts/qemu.py
+++ b/scripts/qemu.py
@@ -131,7 +131,7 @@ class QEMUMachine(object):
 raise
 
 def is_running(self):
-return self._popen and (self._popen.returncode is None)
+return self._popen is not None and self._popen.returncode is None
 
 def exitcode(self):
 if self._popen is None:
-- 
2.13.5

[Qemu-devel] [PULL 06/15] qmp.py: Couple of pylint/style fixes

[Eduardo Habkost]

From: Lukáš Doktor 

No actual code changes, just initializing attributes earlier to avoid
AttributeError on early introspection, a few pylint/style fixes and
docstring clarifications.

Signed-off-by: Lukáš Doktor 
Reviewed-by: Philippe Mathieu-Daudé 
Message-Id: <20170818142613.32394-7-ldok...@redhat.com>
Signed-off-by: Eduardo Habkost 
---
 scripts/qmp/qmp.py | 37 -
 1 file changed, 24 insertions(+), 13 deletions(-)

diff --git a/scripts/qmp/qmp.py b/scripts/qmp/qmp.py
index 62d3651967..782d1ac5df 100644
--- a/scripts/qmp/qmp.py
+++ b/scripts/qmp/qmp.py
@@ -13,19 +13,30 @@ import errno
 import socket
 import sys
 
+
 class QMPError(Exception):
 pass
 
+
 class QMPConnectError(QMPError):
 pass
 
+
 class QMPCapabilitiesError(QMPError):
 pass
 
+
 class QMPTimeoutError(QMPError):
 pass
 
+
 class QEMUMonitorProtocol:
+
+#: Socket's error class
+error = socket.error
+#: Socket's timeout
+timeout = socket.timeout
+
 def __init__(self, address, server=False, debug=False):
 """
 Create a QEMUMonitorProtocol class.
@@ -42,6 +53,7 @@ class QEMUMonitorProtocol:
 self.__address = address
 self._debug = debug
 self.__sock = self.__get_sock()
+self.__sockfile = None
 if server:
 self.__sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
 self.__sock.bind(self.__address)
@@ -56,7 +68,7 @@ class QEMUMonitorProtocol:
 
 def __negotiate_capabilities(self):
 greeting = self.__json_read()
-if greeting is None or not greeting.has_key('QMP'):
+if greeting is None or "QMP" not in greeting:
 raise QMPConnectError
 # Greeting seems ok, negotiate capabilities
 resp = self.cmd('qmp_capabilities')
@@ -78,8 +90,6 @@ class QEMUMonitorProtocol:
 continue
 return resp
 
-error = socket.error
-
 def __get_events(self, wait=False):
 """
 Check for new events in the stream and cache them in __events.
@@ -89,8 +99,8 @@ class QEMUMonitorProtocol:
 
 @raise QMPTimeoutError: If a timeout float is provided and the timeout
 period elapses.
-@raise QMPConnectError: If wait is True but no events could be 
retrieved
-or if some other error occurred.
+@raise QMPConnectError: If wait is True but no events could be
+retrieved or if some other error occurred.
 """
 
 # Check for new events regardless and pull them into the cache:
@@ -175,7 +185,7 @@ class QEMUMonitorProtocol:
 @param args: command arguments (dict)
 @param id: command id (dict, list, string or int)
 """
-qmp_cmd = { 'execute': name }
+qmp_cmd = {'execute': name}
 if args:
 qmp_cmd['arguments'] = args
 if id:
@@ -183,6 +193,9 @@ class QEMUMonitorProtocol:
 return self.cmd_obj(qmp_cmd)
 
 def command(self, cmd, **kwds):
+"""
+Build and send a QMP command to the monitor, report errors if any
+"""
 ret = self.cmd(cmd, kwds)
 if ret.has_key('error'):
 raise Exception(ret['error']['desc'])
@@ -190,15 +203,15 @@ class QEMUMonitorProtocol:
 
 def pull_event(self, wait=False):
 """
-Get and delete the first available QMP event.
+Pulls a single event.
 
 @param wait (bool): block until an event is available.
 @param wait (float): If wait is a float, treat it as a timeout value.
 
 @raise QMPTimeoutError: If a timeout float is provided and the timeout
 period elapses.
-@raise QMPConnectError: If wait is True but no events could be 
retrieved
-or if some other error occurred.
+@raise QMPConnectError: If wait is True but no events could be
+retrieved or if some other error occurred.
 
 @return The first available QMP event, or None.
 """
@@ -217,8 +230,8 @@ class QEMUMonitorProtocol:
 
 @raise QMPTimeoutError: If a timeout float is provided and the timeout
 period elapses.
-@raise QMPConnectError: If wait is True but no events could be 
retrieved
-or if some other error occurred.
+@raise QMPConnectError: If wait is True but no events could be
+retrieved or if some other error occurred.
 
 @return The list of available QMP events.
 """
@@ -235,8 +248,6 @@ class QEMUMonitorProtocol:
 self.__sock.close()
 self.__sockfile.close()
 
-timeout = socket.timeout
-
 def settimeout(self, timeout):
 self.__sock.settimeout(timeout)
 
-- 
2.13.5

[Qemu-devel] [PULL 12/15] qemu.py: avoid writing to stdout/stderr

[Eduardo Habkost]

From: Amador Pahim 

This module should not write directly to stdout/stderr. Instead, it
should either raise exceptions or just log the messages and let the
callers handle them and decide what to do. For example, scripts could
choose to send the log messages stderr or/and write them to a file if
verbose or debugging mode is enabled.

This patch replaces the writes to stderr by an exception in the
send_fd_scm() when _socket_scm_helper is not set or not present. In the
same method, the subprocess Popen will now redirect the stdout/stderr to
logging.debug instead of writing to system stderr. As consequence, since
the Popen.communicate() is now used (in order to get the stdout), the
further call to wait() became redundant and was replaced by
Popen.returncode.

The shutdown() message on negative exit code will now be logged
to logging.warn instead of written to system stderr.

Signed-off-by: Amador Pahim 
Message-Id: <20170901112829.2571-3-apa...@redhat.com>
Reviewed-by: Fam Zheng 
Signed-off-by: Eduardo Habkost 
---
 scripts/qemu.py | 31 ++-
 1 file changed, 22 insertions(+), 9 deletions(-)

diff --git a/scripts/qemu.py b/scripts/qemu.py
index f80b008f7f..8d3d24dd2b 100644
--- a/scripts/qemu.py
+++ b/scripts/qemu.py
@@ -13,12 +13,22 @@
 #
 
 import errno
+import logging
 import os
 import sys
 import subprocess
 import qmp.qmp
 
 
+LOG = logging.getLogger(__name__)
+
+
+class QEMUMachineError(Exception):
+"""
+Exception called when an error in QEMUMachine happens.
+"""
+
+
 class MonitorResponseError(qmp.qmp.QMPError):
 '''
 Represents erroneous QMP monitor reply
@@ -107,18 +117,21 @@ class QEMUMachine(object):
 # In iotest.py, the qmp should always use unix socket.
 assert self._qmp.is_scm_available()
 if self._socket_scm_helper is None:
-print >>sys.stderr, "No path to socket_scm_helper set"
-return -1
+raise QEMUMachineError("No path to socket_scm_helper set")
 if not os.path.exists(self._socket_scm_helper):
-print >>sys.stderr, "%s does not exist" % self._socket_scm_helper
-return -1
+raise QEMUMachineError("%s does not exist" %
+   self._socket_scm_helper)
 fd_param = ["%s" % self._socket_scm_helper,
 "%d" % self._qmp.get_sock_fd(),
 "%s" % fd_file_path]
 devnull = open('/dev/null', 'rb')
-proc = subprocess.Popen(fd_param, stdin=devnull, stdout=sys.stdout,
-stderr=sys.stderr)
-return proc.wait()
+proc = subprocess.Popen(fd_param, stdin=devnull, 
stdout=subprocess.PIPE,
+stderr=subprocess.STDOUT)
+output = proc.communicate()[0]
+if output:
+LOG.debug(output)
+
+return proc.returncode
 
 @staticmethod
 def _remove_if_exists(path):
@@ -202,8 +215,8 @@ class QEMUMachine(object):
 
 exitcode = self._popen.wait()
 if exitcode < 0:
-sys.stderr.write('qemu received signal %i: %s\n'
- % (-exitcode, ' '.join(self._args)))
+LOG.warn('qemu received signal %i: %s', -exitcode,
+  ' '.join(self._args))
 self._load_io_log()
 self._post_shutdown()
 
-- 
2.13.5

[Qemu-devel] [PULL 02/15] qemu|qtest: Avoid dangerous arguments

[Eduardo Habkost]

From: Lukáš Doktor 

The list object is mutable in python and potentially might modify other
object's arguments when used as default argument. Reproducer:

>>> vm1 = QEMUMachine("qemu")
>>> vm2 = QEMUMachine("qemu")
>>> vm1._wrapper.append("foo")
>>> print vm2._wrapper
['foo']

In this case the `args` is actually copied so it would be safe to keep
it, but it's not a good practice to keep it. The same issue applies in
inherited qtest module.

Signed-off-by: Lukáš Doktor 
Reviewed-by: Eduardo Habkost 
Reviewed-by: John Snow 
Message-Id: <20170818142613.32394-3-ldok...@redhat.com>
Reviewed-by: Cleber Rosa 
Signed-off-by: Eduardo Habkost 
---
 scripts/qemu.py  | 6 +-
 scripts/qtest.py | 2 +-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/scripts/qemu.py b/scripts/qemu.py
index b45e691538..afd98a290e 100644
--- a/scripts/qemu.py
+++ b/scripts/qemu.py
@@ -30,7 +30,7 @@ class QEMUMachine(object):
 # vm is guaranteed to be shut down here
 '''
 
-def __init__(self, binary, args=[], wrapper=[], name=None,
+def __init__(self, binary, args=None, wrapper=None, name=None,
  test_dir="/var/tmp", monitor_address=None,
  socket_scm_helper=None, debug=False):
 '''
@@ -46,6 +46,10 @@ class QEMUMachine(object):
 @param debug: enable debug mode
 @note: Qemu process is not started until launch() is used.
 '''
+if args is None:
+args = []
+if wrapper is None:
+wrapper = []
 if name is None:
 name = "qemu-%d" % os.getpid()
 if monitor_address is None:
diff --git a/scripts/qtest.py b/scripts/qtest.py
index d5aecb5f49..ab183c0635 100644
--- a/scripts/qtest.py
+++ b/scripts/qtest.py
@@ -79,7 +79,7 @@ class QEMUQtestProtocol(object):
 class QEMUQtestMachine(qemu.QEMUMachine):
 '''A QEMU VM'''
 
-def __init__(self, binary, args=[], name=None, test_dir="/var/tmp",
+def __init__(self, binary, args=None, name=None, test_dir="/var/tmp",
  socket_scm_helper=None):
 if name is None:
 name = "qemu-%d" % os.getpid()
-- 
2.13.5

[Qemu-devel] [PULL 01/15] qemu.py: Pylint/style fixes

[Eduardo Habkost]

From: Lukáš Doktor 

No actual code changes, just several pylint/style fixes and docstring
clarifications.

Signed-off-by: Lukáš Doktor 
Reviewed-by: Stefan Hajnoczi 
Message-Id: <20170818142613.32394-2-ldok...@redhat.com>
Reviewed-by: Cleber Rosa 
Signed-off-by: Eduardo Habkost 
---
 scripts/qemu.py | 73 +++--
 1 file changed, 55 insertions(+), 18 deletions(-)

diff --git a/scripts/qemu.py b/scripts/qemu.py
index 4d8ee10943..b45e691538 100644
--- a/scripts/qemu.py
+++ b/scripts/qemu.py
@@ -30,8 +30,22 @@ class QEMUMachine(object):
 # vm is guaranteed to be shut down here
 '''
 
-def __init__(self, binary, args=[], wrapper=[], name=None, 
test_dir="/var/tmp",
- monitor_address=None, socket_scm_helper=None, debug=False):
+def __init__(self, binary, args=[], wrapper=[], name=None,
+ test_dir="/var/tmp", monitor_address=None,
+ socket_scm_helper=None, debug=False):
+'''
+Initialize a QEMUMachine
+
+@param binary: path to the qemu binary
+@param args: list of extra arguments
+@param wrapper: list of arguments used as prefix to qemu binary
+@param name: prefix for socket and log file names (default: qemu-PID)
+@param test_dir: where to create socket and log file
+@param monitor_address: address for QMP monitor
+@param socket_scm_helper: helper program, required for send_fd_scm()"
+@param debug: enable debug mode
+@note: Qemu process is not started until launch() is used.
+'''
 if name is None:
 name = "qemu-%d" % os.getpid()
 if monitor_address is None:
@@ -40,12 +54,13 @@ class QEMUMachine(object):
 self._qemu_log_path = os.path.join(test_dir, name + ".log")
 self._popen = None
 self._binary = binary
-self._args = list(args) # Force copy args in case we modify them
+self._args = list(args) # Force copy args in case we modify them
 self._wrapper = wrapper
 self._events = []
 self._iolog = None
 self._socket_scm_helper = socket_scm_helper
 self._debug = debug
+self._qmp = None
 
 def __enter__(self):
 return self
@@ -78,16 +93,16 @@ class QEMUMachine(object):
 if self._socket_scm_helper is None:
 print >>sys.stderr, "No path to socket_scm_helper set"
 return -1
-if os.path.exists(self._socket_scm_helper) == False:
+if not os.path.exists(self._socket_scm_helper):
 print >>sys.stderr, "%s does not exist" % self._socket_scm_helper
 return -1
 fd_param = ["%s" % self._socket_scm_helper,
 "%d" % self._qmp.get_sock_fd(),
 "%s" % fd_file_path]
 devnull = open('/dev/null', 'rb')
-p = subprocess.Popen(fd_param, stdin=devnull, stdout=sys.stdout,
- stderr=sys.stderr)
-return p.wait()
+proc = subprocess.Popen(fd_param, stdin=devnull, stdout=sys.stdout,
+stderr=sys.stderr)
+return proc.wait()
 
 @staticmethod
 def _remove_if_exists(path):
@@ -113,8 +128,8 @@ class QEMUMachine(object):
 return self._popen.pid
 
 def _load_io_log(self):
-with open(self._qemu_log_path, "r") as fh:
-self._iolog = fh.read()
+with open(self._qemu_log_path, "r") as iolog:
+self._iolog = iolog.read()
 
 def _base_args(self):
 if isinstance(self._monitor_address, tuple):
@@ -128,7 +143,8 @@ class QEMUMachine(object):
 '-display', 'none', '-vga', 'none']
 
 def _pre_launch(self):
-self._qmp = qmp.qmp.QEMUMonitorProtocol(self._monitor_address, 
server=True,
+self._qmp = qmp.qmp.QEMUMonitorProtocol(self._monitor_address,
+server=True,
 debug=self._debug)
 
 def _post_launch(self):
@@ -145,9 +161,11 @@ class QEMUMachine(object):
 qemulog = open(self._qemu_log_path, 'wb')
 try:
 self._pre_launch()
-args = self._wrapper + [self._binary] + self._base_args() + 
self._args
+args = (self._wrapper + [self._binary] + self._base_args() +
+self._args)
 self._popen = subprocess.Popen(args, stdin=devnull, stdout=qemulog,
-   stderr=subprocess.STDOUT, 
shell=False)
+   stderr=subprocess.STDOUT,
+   shell=False)
 self._post_launch()
 except:
 if self.is_running():
@@ -168,23 +186,30 @@ class QEMUMachine(object):
 
 exitcode = self._popen.wait()
 if 

[Qemu-devel] [PULL 03/15] qemu.py: Use iteritems rather than keys()

[Eduardo Habkost]

From: Lukáš Doktor 

Let's avoid creating an in-memory list of keys and query for each value
and use `iteritems` which is an iterator of key-value pairs.

Signed-off-by: Lukáš Doktor 
Reviewed-by: Eduardo Habkost 
Reviewed-by: Philippe Mathieu-Daudé 
Message-Id: <20170818142613.32394-4-ldok...@redhat.com>
Reviewed-by: Cleber Rosa 
Signed-off-by: Eduardo Habkost 
---
 scripts/qemu.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/scripts/qemu.py b/scripts/qemu.py
index afd98a290e..d8c169b31e 100644
--- a/scripts/qemu.py
+++ b/scripts/qemu.py
@@ -200,11 +200,11 @@ class QEMUMachine(object):
 def qmp(self, cmd, conv_keys=True, **args):
 '''Invoke a QMP command and return the response dict'''
 qmp_args = dict()
-for key in args.keys():
+for key, value in args.iteritems():
 if conv_keys:
-qmp_args[key.translate(self.underscore_to_dash)] = args[key]
+qmp_args[key.translate(self.underscore_to_dash)] = value
 else:
-qmp_args[key] = args[key]
+qmp_args[key] = value
 
 return self._qmp.cmd(cmd, args=qmp_args)
 
-- 
2.13.5

[Qemu-devel] [PULL 00/15] Python queue, 2017-09-15

[Eduardo Habkost]

I have been queueing some patches for Python modules and scripts,
recently, and submitted a MAINTAINERS patch to include Cleber and
me as maintainers.

However, I don't want to delay this pull request to avoid
conflicts if people send additional patches for those scripts, so
I'm sending this pull request before the MAINTAINERS patch is
applied.

The following changes since commit 5faf2d376af3cb4eb92da44c2580e08d39832caa:

  Merge remote-tracking branch 'remotes/huth/tags/check-20170915' into staging 
(2017-09-15 20:29:44 +0100)

are available in the git repository at:

  git://github.com/ehabkost/qemu.git tags/python-next-pull-request

for you to fetch changes up to b92a0011b1220aff549ae82c6104014d25e339ef:

  qemu.py: include debug information on launch error (2017-09-15 20:12:00 -0300)


Python queue, 2017-09-15



Amador Pahim (5):
  qemu.py: fix is_running() return before first launch()
  qemu.py: avoid writing to stdout/stderr
  qemu.py: use os.path.null instead of /dev/null
  qemu.py: improve message on negative exit code
  qemu.py: include debug information on launch error

Lukáš Doktor (10):
  qemu.py: Pylint/style fixes
  qemu|qtest: Avoid dangerous arguments
  qemu.py: Use iteritems rather than keys()
  qemu.py: Simplify QMP key-conversion
  qemu.py: Use custom exceptions rather than Exception
  qmp.py: Couple of pylint/style fixes
  qmp.py: Use object-based class for QEMUMonitorProtocol
  qmp.py: Avoid "has_key" usage
  qmp.py: Avoid overriding a builtin object
  qtest.py: Few pylint/style fixes

 scripts/qemu.py   | 145 +++---
 scripts/qmp/qmp.py|  49 ++---
 scripts/qtest.py  |  13 +++--
 scripts/qmp/qmp-shell |   4 +-
 4 files changed, 151 insertions(+), 60 deletions(-)

-- 
2.13.5

[Qemu-devel] [Bug 1594394] Re: Using setreuid / setegid crashes x86_64 user-mode target

[James Clarke]

And to confirm, while somewhat of a hack, that patch does indeed fix
aptitude as expected (as well as my minimal test case I wrote when
debugging this before stumbling upon this bug report).

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1594394

Title:
  Using setreuid / setegid crashes x86_64 user-mode target

Status in QEMU:
  New

Bug description:
  When setreuid() or setegid() are called from x86_64 target code in
  user mode, qemu crashes inside the NPTL signal handlers.  x86 targets
  do not directly use a syscall to handle setreuid() / setegid();
  instead the x86 NPTL implementation sets up a temporary data region in
  memory (__xidcmd) and issues a signal (SIGRT1) to all threads,
  allowing the handler for that signal to issue the syscall.  Under
  qemu, __xidcmd remains null (see variable display below backtrace).

  Backtrace:
  Program received signal SIGSEGV, Segmentation fault.
  [Switching to Thread 0x3fff85c74fc0 (LWP 74517)]
  0x6017491c in sighandler_setxid (sig=33, si=0x3fff85c72d08, 
ctx=0x3fff85c71f90) at nptl-init.c:263
  263 nptl-init.c: No such file or directory.
  (gdb) thread apply all bt

  Thread 3 (Thread 0x3fff87e8efc0 (LWP 74515)):
  #0  0x601cc430 in syscall ()
  #1  0x60109080 in futex_wait (val=, ev=) at /build/qemu/util/qemu-thread-posix.c:292
  #2  qemu_event_wait (ev=0x62367bb0 ) at 
/build/qemu/util/qemu-thread-posix.c:399
  #3  0x6010f73c in call_rcu_thread (opaque=) at 
/build/qemu/util/rcu.c:250
  #4  0x60176f8c in start_thread (arg=0x3fff87e8efc0) at 
pthread_create.c:336
  #5  0x601cebf4 in clone ()

  Thread 2 (Thread 0x3fff85c74fc0 (LWP 74517)):
  #0  0x6017491c in sighandler_setxid (sig=33, si=0x3fff85c72d08, 
ctx=0x3fff85c71f90) at nptl-init.c:263
  #1  
  #2  0x601cc42c in syscall ()
  #3  0x60044b08 in safe_futex (val3=, uaddr2=0x0, 
timeout=, val=, op=128, uaddr=) at 
/build/qemu/linux-user/syscall.c:748
  #4  do_futex (val3=, uaddr2=275186650880, timeout=0, val=1129, 
op=128, uaddr=275186651116) at /build/qemu/linux-user/syscall.c:6201
  #5  do_syscall (cpu_env=0x1000abfd350, num=, 
arg1=275186651116, arg2=, arg3=1129, arg4=0, arg5=275186650880, 
arg6=, arg7=0, arg8=0)
  at /build/qemu/linux-user/syscall.c:10651
  #6  0x600347b8 in cpu_loop (env=0x1000abfd350) at 
/build/qemu/linux-user/main.c:317
  #7  0x60036ae0 in clone_func (arg=0x3fffc4c2ca38) at 
/build/qemu/linux-user/syscall.c:5445
  #8  0x60176f8c in start_thread (arg=0x3fff85c74fc0) at 
pthread_create.c:336
  #9  0x601cebf4 in clone ()

  Thread 1 (Thread 0x1000aa05000 (LWP 74511)):
  #0  0x601cc430 in syscall ()
  #1  0x60044b08 in safe_futex (val3=, uaddr2=0x0, 
timeout=, val=, op=128, uaddr=) at 
/build/qemu/linux-user/syscall.c:748
  #2  do_futex (val3=, uaddr2=1, timeout=0, val=1, op=128, 
uaddr=275078324992) at /build/qemu/linux-user/syscall.c:6201
  #3  do_syscall (cpu_env=0x1000aa23890, num=, 
arg1=275078324992, arg2=, arg3=1, arg4=0, arg5=1, 
arg6=, arg7=0, arg8=0) at /build/qemu/linux-user/syscall.c:10651
  #4  0x600347b8 in cpu_loop (env=0x1000aa23890) at 
/build/qemu/linux-user/main.c:317
  #5  0x600020e4 in main (argc=, argv=, 
envp=) at /build/qemu/linux-user/main.c:4779
  (gdb) p __xidcmd
  $1 = (struct xid_command *) 0x0

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1594394/+subscriptions

[Qemu-devel] [Bug 1594394] Re: Using setreuid / setegid crashes x86_64 user-mode target

[James Clarke]

What's the status of this? This is causing aptitude in Debian chroots to
reliably segfault under qemu-arm-user.

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1594394

Title:
  Using setreuid / setegid crashes x86_64 user-mode target

Status in QEMU:
  New

Bug description:
  When setreuid() or setegid() are called from x86_64 target code in
  user mode, qemu crashes inside the NPTL signal handlers.  x86 targets
  do not directly use a syscall to handle setreuid() / setegid();
  instead the x86 NPTL implementation sets up a temporary data region in
  memory (__xidcmd) and issues a signal (SIGRT1) to all threads,
  allowing the handler for that signal to issue the syscall.  Under
  qemu, __xidcmd remains null (see variable display below backtrace).

  Backtrace:
  Program received signal SIGSEGV, Segmentation fault.
  [Switching to Thread 0x3fff85c74fc0 (LWP 74517)]
  0x6017491c in sighandler_setxid (sig=33, si=0x3fff85c72d08, 
ctx=0x3fff85c71f90) at nptl-init.c:263
  263 nptl-init.c: No such file or directory.
  (gdb) thread apply all bt

  Thread 3 (Thread 0x3fff87e8efc0 (LWP 74515)):
  #0  0x601cc430 in syscall ()
  #1  0x60109080 in futex_wait (val=, ev=) at /build/qemu/util/qemu-thread-posix.c:292
  #2  qemu_event_wait (ev=0x62367bb0 ) at 
/build/qemu/util/qemu-thread-posix.c:399
  #3  0x6010f73c in call_rcu_thread (opaque=) at 
/build/qemu/util/rcu.c:250
  #4  0x60176f8c in start_thread (arg=0x3fff87e8efc0) at 
pthread_create.c:336
  #5  0x601cebf4 in clone ()

  Thread 2 (Thread 0x3fff85c74fc0 (LWP 74517)):
  #0  0x6017491c in sighandler_setxid (sig=33, si=0x3fff85c72d08, 
ctx=0x3fff85c71f90) at nptl-init.c:263
  #1  
  #2  0x601cc42c in syscall ()
  #3  0x60044b08 in safe_futex (val3=, uaddr2=0x0, 
timeout=, val=, op=128, uaddr=) at 
/build/qemu/linux-user/syscall.c:748
  #4  do_futex (val3=, uaddr2=275186650880, timeout=0, val=1129, 
op=128, uaddr=275186651116) at /build/qemu/linux-user/syscall.c:6201
  #5  do_syscall (cpu_env=0x1000abfd350, num=, 
arg1=275186651116, arg2=, arg3=1129, arg4=0, arg5=275186650880, 
arg6=, arg7=0, arg8=0)
  at /build/qemu/linux-user/syscall.c:10651
  #6  0x600347b8 in cpu_loop (env=0x1000abfd350) at 
/build/qemu/linux-user/main.c:317
  #7  0x60036ae0 in clone_func (arg=0x3fffc4c2ca38) at 
/build/qemu/linux-user/syscall.c:5445
  #8  0x60176f8c in start_thread (arg=0x3fff85c74fc0) at 
pthread_create.c:336
  #9  0x601cebf4 in clone ()

  Thread 1 (Thread 0x1000aa05000 (LWP 74511)):
  #0  0x601cc430 in syscall ()
  #1  0x60044b08 in safe_futex (val3=, uaddr2=0x0, 
timeout=, val=, op=128, uaddr=) at 
/build/qemu/linux-user/syscall.c:748
  #2  do_futex (val3=, uaddr2=1, timeout=0, val=1, op=128, 
uaddr=275078324992) at /build/qemu/linux-user/syscall.c:6201
  #3  do_syscall (cpu_env=0x1000aa23890, num=, 
arg1=275078324992, arg2=, arg3=1, arg4=0, arg5=1, 
arg6=, arg7=0, arg8=0) at /build/qemu/linux-user/syscall.c:10651
  #4  0x600347b8 in cpu_loop (env=0x1000aa23890) at 
/build/qemu/linux-user/main.c:317
  #5  0x600020e4 in main (argc=, argv=, 
envp=) at /build/qemu/linux-user/main.c:4779
  (gdb) p __xidcmd
  $1 = (struct xid_command *) 0x0

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1594394/+subscriptions

[Qemu-devel] [PATCH] MAINTAINERS: Add Python scripts

[Eduardo Habkost]

Cleber and I are volunteering to review and queue patches for the
Python scripts and modules in scripts/.

I'm setting "M: Odd fixes" because not all scripts are actively
maintained.

Signed-off-by: Eduardo Habkost 
---
Even before this patch is merged, I plan to send a pull request
including some patches for the Python code soon.
---
 MAINTAINERS | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 2c333aba21..2c3b8ecde7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1390,6 +1390,13 @@ S: Maintained
 F: include/sysemu/cryptodev*.h
 F: backends/cryptodev*.c
 
+Python scripts
+M: Eduardo Habkost 
+M: Cleber Rosa 
+S: Odd fixes
+F: scripts/qmp/*
+F: scripts/*.py
+
 QAPI
 M: Markus Armbruster 
 M: Michael Roth 
-- 
2.13.5

Re: [Qemu-devel] [PATCH v2 1/1] target/xtensa: Use the pre-defined MEMTXATTRS_UNSPECIFIED macro

[Max Filippov]

On Fri, Sep 15, 2017 at 2:56 PM, Alistair Francis
 wrote:
> Instead of using the hardcoded (MemTxAttrs){0} for no memory attributes
> let's use the already defined MEMTXATTRS_UNSPECIFIED macro instead.
>
> This is technically a change of behaviour as MEMTXATTRS_UNSPECIFIED sets
> the unspecified field to 1, but it doesn't look like anything is
> checking this field.
>
> Signed-off-by: Alistair Francis 
> ---
> V2:
>  - Update commit message to indicate the change in behaviour
>
>  target/xtensa/op_helper.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Acked-by: Max Filippov 

-- 
Thanks.
-- Max

Re: [Qemu-devel] [PATCH v2 1/2] x86_iommu: Move machine check to x86_iommu_realize()

[Eduardo Habkost]

On Fri, Sep 15, 2017 at 05:04:36PM +0200, Mohammed Gamal wrote:
> Instead of having the same error checks in vtd_realize()
> and amdvi_realize(), move that over to the generic
> x86_iommu_realize().
> 
> Signed-off-by: Mohammed Gamal 
> ---
>  hw/i386/amd_iommu.c   | 10 +-
>  hw/i386/intel_iommu.c | 10 +-
>  hw/i386/x86-iommu.c   | 13 +
>  3 files changed, 15 insertions(+), 18 deletions(-)
> 
> diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
> index 334938a..839f01f 100644
> --- a/hw/i386/amd_iommu.c
> +++ b/hw/i386/amd_iommu.c
> @@ -1141,18 +1141,10 @@ static void amdvi_realize(DeviceState *dev, Error 
> **err)
>  AMDVIState *s = AMD_IOMMU_DEVICE(dev);
>  X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(dev);
>  MachineState *ms = MACHINE(qdev_get_machine());
> -MachineClass *mc = MACHINE_GET_CLASS(ms);
>  PCMachineState *pcms =
>  PC_MACHINE(object_dynamic_cast(OBJECT(ms), TYPE_PC_MACHINE));

This function now expects pcms to never be NULL, so using
object_dynamic_cast() does not make sense.  You can use
PC_MACHINE(ms) directly.


> -PCIBus *bus;
> +PCIBus *bus = pcms->bus;
>  
> -if (!pcms) {
> -error_setg(err, "Machine-type '%s' not supported by amd-iommu",
> -   mc->name);
> -return;
> -}
> -
> -bus = pcms->bus;
>  s->iotlb = g_hash_table_new_full(amdvi_uint64_hash,
>   amdvi_uint64_equal, g_free, g_free);
>  
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index 3a5bb0b..aa01812 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -3027,20 +3027,12 @@ static bool vtd_decide_config(IntelIOMMUState *s, 
> Error **errp)
>  static void vtd_realize(DeviceState *dev, Error **errp)
>  {
>  MachineState *ms = MACHINE(qdev_get_machine());
> -MachineClass *mc = MACHINE_GET_CLASS(ms);
>  PCMachineState *pcms =
>  PC_MACHINE(object_dynamic_cast(OBJECT(ms), TYPE_PC_MACHINE));

Same here.

> -PCIBus *bus;
> +PCIBus *bus = pcms->bus;
>  IntelIOMMUState *s = INTEL_IOMMU_DEVICE(dev);
>  X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(dev);
>  
> -if (!pcms) {
> -error_setg(errp, "Machine-type '%s' not supported by intel-iommu",
> -   mc->name);
> -return;
> -}
> -
> -bus = pcms->bus;
>  x86_iommu->type = TYPE_INTEL;
>  
>  if (!vtd_decide_config(s, errp)) {
> diff --git a/hw/i386/x86-iommu.c b/hw/i386/x86-iommu.c
> index 293caf8..51de519 100644
> --- a/hw/i386/x86-iommu.c
> +++ b/hw/i386/x86-iommu.c
> @@ -21,6 +21,8 @@
>  #include "hw/sysbus.h"
>  #include "hw/boards.h"
>  #include "hw/i386/x86-iommu.h"
> +#include "hw/i386/pc.h"
> +#include "qapi/error.h"
>  #include "qemu/error-report.h"
>  #include "trace.h"
>  
> @@ -80,7 +82,18 @@ static void x86_iommu_realize(DeviceState *dev, Error 
> **errp)
>  {
>  X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(dev);
>  X86IOMMUClass *x86_class = X86_IOMMU_GET_CLASS(dev);
> +MachineState *ms = MACHINE(qdev_get_machine());
> +MachineClass *mc = MACHINE_GET_CLASS(ms);
> +PCMachineState *pcms =
> +PC_MACHINE(object_dynamic_cast(OBJECT(ms), TYPE_PC_MACHINE));
>  QLIST_INIT(_iommu->iec_notifiers);
> +
> +if (!pcms) {
> +error_setg(errp, "Machine-type '%s' not supported by IOMMU",
> +   mc->name);
> +return;
> +}
> +
>  if (x86_class->realize) {
>  x86_class->realize(dev, errp);
>  }
> -- 
> 1.8.3.1
> 

-- 
Eduardo

Re: [Qemu-devel] [PATCH] dma/i82374: avoid double creation of i82374 device

[Eduardo Habkost]

On Fri, Sep 15, 2017 at 12:18:11PM +0200, Paolo Bonzini wrote:
> On 15/09/2017 11:06, Eduardo Otubo wrote:
> > QEMU fails when used with the following command line:
> > 
> >   ./ppc64-softmmu/qemu-system-ppc64 -S -machine 40p,accel=tcg -device i82374
> >   qemu-system-ppc64: hw/isa/isa-bus.c:110: isa_bus_dma: Assertion 
> > `!bus->dma[0] && !bus->dma[1]' failed.
> >   Aborted (core dumped)
> > 
> > The 40p machine type already creates the device i82374. If specified in the
> > command line, it will try to create it again, hence generating the error. 
> > The
> > function isa_bus_dma() isn't supposed to be called twice for the same bus. 
> > One
> > way to avoid this problem is to set user_creatable=false.
> > 
> > A possible fix in a near future would be making
> > isa_bus_dma()/DMA_init()/i82374_realize() return an error instead of 
> > asserting
> > as well.
> > 
> > Signed-off-by: Eduardo Otubo 
> > ---
> >  hw/dma/i82374.c | 5 +
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/hw/dma/i82374.c b/hw/dma/i82374.c
> > index 6c0f975df0..e76dea8dc7 100644
> > --- a/hw/dma/i82374.c
> > +++ b/hw/dma/i82374.c
> > @@ -139,6 +139,11 @@ static void i82374_class_init(ObjectClass *klass, void 
> > *data)
> >  dc->realize = i82374_realize;
> >  dc->vmsd = _i82374;
> >  dc->props = i82374_properties;
> > +dc->user_creatable = false;
> > +/*
> > + * Reason: i82374_realize() crashes (assertion failure inside 
> > isa_bus_dma()
> > + * if the device is instantiated twice.
> > + */
> >  }
> >  
> >  static const TypeInfo i82374_info = {
> > 
> 
> This breaks "make check", doesn't it?

Why would it?  I don't see any test code using -device i82374.
(endianness-test uses -device i82378).

> 
> v2 should be the one that returns an error instead of asserting.

I agree that returning an error is better.

-- 
Eduardo

Re: [Qemu-devel] [Qemu-ppc] [RFC PATCH] tests: Add a device_add/del HMP test

[Eduardo Habkost]

On Wed, Sep 13, 2017 at 07:45:11AM +0200, Thomas Huth wrote:
> On 12.09.2017 19:37, Eduardo Habkost wrote:
> > On Mon, Sep 11, 2017 at 08:13:21AM +0200, Thomas Huth wrote:
> >> On 09.09.2017 22:41, Eduardo Habkost wrote:
> >>> On Wed, Sep 06, 2017 at 08:59:32AM +0200, Markus Armbruster wrote:
>  Thomas Huth  writes:
> 
> > On 05.09.2017 18:48, Dr. David Alan Gilbert wrote:
> >> * Markus Armbruster (arm...@redhat.com) wrote:
> >>> Thomas Huth  writes:
> >>>
>  People tend to forget to mark internal devices with "user_creatable 
>  = false
>  or hotpluggable = false, and these devices can crash QEMU if added 
>  via the
>  HMP monitor. So let's add a test to run through all devices and that 
>  tries
>  to add them blindly (without arguments) to see whether this could 
>  crash the
>  QEMU instance.
> >> [...]
> >>> * The device supports only cold plug with -device, not hot plug with
> >>>   device_add.
> >
> > We've got Eduardo's scripts/device-crash-test script for that already,
> > so no need to cover that here.
> 
>  Point taken.  So this test is really about hot plug / unplug.  Suggest
>  to clarify the commit message: s/add them blindly/hotplug and unplug
>  them blindly/.
> >>>
> >>> We could extend device-crash-test to test device_add too, as it
> >>> already has extra code to deal with known crashes and testing
> >>> multiple machine-types.  Also, any additional code we write to
> >>> ensure we add mandatory arguments or plug only to valid buses
> >>> would apply to both -device and device_add.  I also think Python
> >>> test code is easier to maintain and extend, but that's just my
> >>> personal preference.
> >>
> >> Adding device_add/del support to device-crash-test is certainly an
> >> option. The problem is that nobody runs it by default, so this won't
> >> help to avoid that new problems are being committed to the repository.
> >>
> >> I think we really should have a test for "make check", too. So would my
> >> test be acceptable if I'd rewrite it to use QMP instead (I don't think I
> >> could do the full list that Markus mentioned, but at least a basic test
> >> via QMP as a start)?
> > 
> > We can run device-crash-test on "make check", we just need to
> > choose what's the subset of tests we want to run (because testing
> > all machine+device+target combinations would take too long).
> 
> Maybe we should just run it one time for every machine - and try to add
> all available devices at once?

Yes, it makes sense.  I will keep that in mind when trying to
implement device_add support on device-crash-test (but if anybody
wants to volunteer to implement it, be my guest).

-- 
Eduardo

Re: [Qemu-devel] [PATCH v2 2/6] hw/block/fdc: Convert to realize

[John Snow]

On 08/04/2017 06:26 AM, Mao Zhongyi wrote:
> Convert floppy_drive_init() to realize and rename it to
> floppy_drive_realize().
> 
> Cc: John Snow 
> Cc: Kevin Wolf 
> Cc: Max Reitz 
> Cc: Markus Armbruster 
> 
> Signed-off-by: Mao Zhongyi 

Similar to the first patch, you need to visit the test output for 172
and update it to match accordingly. Should be a trivial fix.

--js

Re: [Qemu-devel] [PATCH] rbd: Detect rbd image resizes and propagate them

[Eric Blake]

On 09/15/2017 07:33 AM, Kevin Wolf wrote:
> Am 13.09.2017 um 18:44 hat Adam Wolfe Gordon geschrieben:
>> Register a watcher with rbd so that we get notified when an image is
>> resized. Propagate resizes to parent block devices so that guest devices
>> get resized without user intervention.
>>
>> Signed-off-by: Adam Wolfe Gordon 
>> ---

> 
> There's more wrong about this than just making assumptions about the
> graph layout above our own node (which would be wrong enough already).
> 
> Other assumptions that you are making here and that don't hold true
> generally are that there is only one parent node (no reason why the
> image couldn't be used by two different users) and that we always
> inherit from a parent. If this node was created explicitly by the user
> with its own -blockdev (or -drive) option, it doesn't inherit options
> from any of its parents.
> 
> Basically, a block driver shouldn't care about its users and it can't
> access them in a clean way. This is intentional.
> 
>> +if (parent == NULL) {
>> +error_report("bs %s does not have parent", 
>> bdrv_get_device_or_node_name(bs));
>> +return;
>> +}
>> +
>> +/* Force parents to re-read our size. */
>> +was_variable_length = bs->drv->has_variable_length;
>> +bs->drv->has_variable_length = true;
>> +new_parent_len = bdrv_getlength(parent);
>> +if (new_parent_len < 0) {
>> +error_report("getlength failed on parent %s", 
>> bdrv_get_device_or_node_name(parent));
>> +bs->drv->has_variable_length = was_variable_length;
>> +return;
>> +}
>> +bs->drv->has_variable_length = was_variable_length;
>> +
>> +/* Notify block backends that that we have resized.
>> +   Copied from bdrv_parent_cb_resize. */
>> +QLIST_FOREACH(c, >parents, next_parent) {
>> +if (c->role->resize) {
>> +c->role->resize(c);
>> +}
>> +}
> 
> This is the right approach that you should use consistently instead.

Also, we need to fix existing bugs in block.c first; right now,
bdrv_truncate() forgets that refresh_total_sectors() can fail, and then
calls bdrv_dirty_bitmap_truncate() even though that will act on the
wrong length.  If we are going to allow devices to recognize that they
have been externally resized outside of qemu's control, then we need to
make sure that we react to size updates consistently.

I'm trying to fix that [1], but it may interact with what you are trying
to do here.

[1] https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg03776.html

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [Qemu-block] [PATCH v2 1/6] hw/ide: Convert DeviceClass init to realize

[John Snow]

On 09/15/2017 05:35 PM, John Snow wrote:
> 
> 
> On 08/04/2017 06:26 AM, Mao Zhongyi wrote:
>> Replace init with realize in IDEDeviceClass, which has errp
>> as a parameter. So all the implementations now use error_setg
>> instead of error_report for reporting error.
>>
>> Cc: John Snow 
>> Cc: Markus Armbruster 
>>
>> Signed-off-by: Mao Zhongyi 
>> ---
>>  hw/ide/core.c |  7 ++--
>>  hw/ide/qdev.c | 86 
>> +++
>>  include/hw/ide/internal.h |  5 +--
>>  3 files changed, 49 insertions(+), 49 deletions(-)
>>
>> diff --git a/hw/ide/core.c b/hw/ide/core.c
>> index 0b48b64..7c86fc7 100644
>> --- a/hw/ide/core.c
>> +++ b/hw/ide/core.c
>> @@ -34,6 +34,7 @@
>>  #include "hw/block/block.h"
>>  #include "sysemu/block-backend.h"
>>  #include "qemu/cutils.h"
>> +#include "qemu/error-report.h"
>>  
>>  #include "hw/ide/internal.h"
>>  
>> @@ -2398,7 +2399,7 @@ int ide_init_drive(IDEState *s, BlockBackend *blk, 
>> IDEDriveKind kind,
>> const char *version, const char *serial, const char 
>> *model,
>> uint64_t wwn,
>> uint32_t cylinders, uint32_t heads, uint32_t secs,
>> -   int chs_trans)
>> +   int chs_trans, Error **errp)
> 
> this function now requires an additional invariant, which is that we
> must return -1 AND set errp. Probably wisest to just get rid of the
> return code so that we don't accidentally goof this up in the future.
> 
> I think Markus has had some guidance on this in the past, but admittedly
> I can't remember his preference.
> 
>>  {
>>  uint64_t nb_sectors;
>>  
>> @@ -2423,11 +2424,11 @@ int ide_init_drive(IDEState *s, BlockBackend *blk, 
>> IDEDriveKind kind,
>>  blk_set_guest_block_size(blk, 2048);
>>  } else {
>>  if (!blk_is_inserted(s->blk)) {
>> -error_report("Device needs media, but drive is empty");
>> +error_setg(errp, "Device needs media, but drive is empty");
>>  return -1;
>>  }
>>  if (blk_is_read_only(blk)) {
>> -error_report("Can't use a read-only drive");
>> +error_setg(errp, "Can't use a read-only drive");
>>  return -1;
>>  }
>>  blk_set_dev_ops(blk, _hd_block_ops, s);
>> diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
>> index cc2f5bd..d60ac25 100644
>> --- a/hw/ide/qdev.c
>> +++ b/hw/ide/qdev.c
>> @@ -80,7 +80,7 @@ static char *idebus_get_fw_dev_path(DeviceState *dev)
>>  return g_strdup(path);
>>  }
>>  
>> -static int ide_qdev_init(DeviceState *qdev)
>> +static void ide_qdev_realize(DeviceState *qdev, Error **errp)
>>  {
>>  IDEDevice *dev = IDE_DEVICE(qdev);
>>  IDEDeviceClass *dc = IDE_DEVICE_GET_CLASS(dev);
>> @@ -91,34 +91,31 @@ static int ide_qdev_init(DeviceState *qdev)
>>  }
>>  
>>  if (dev->unit >= bus->max_units) {
>> -error_report("Can't create IDE unit %d, bus supports only %d units",
>> +error_setg(errp, "Can't create IDE unit %d, bus supports only %d 
>> units",
>>   dev->unit, bus->max_units);
>> -goto err;
>> +return;
>>  }
>>  
>>  switch (dev->unit) {
>>  case 0:
>>  if (bus->master) {
>> -error_report("IDE unit %d is in use", dev->unit);
>> -goto err;
>> +error_setg(errp, "IDE unit %d is in use", dev->unit);
>> +return;
>>  }
>>  bus->master = dev;
>>  break;
>>  case 1:
>>  if (bus->slave) {
>> -error_report("IDE unit %d is in use", dev->unit);
>> -goto err;
>> +error_setg(errp, "IDE unit %d is in use", dev->unit);
>> +return;
>>  }
>>  bus->slave = dev;
>>  break;
>>  default:
>> -error_report("Invalid IDE unit %d", dev->unit);
>> -goto err;
>> +error_setg(errp, "Invalid IDE unit %d", dev->unit);
>> +return;
>>  }
>> -return dc->init(dev);
>> -
>> -err:
>> -return -1;
>> +dc->realize(dev, errp);
>>  }
>>  
>>  IDEDevice *ide_create_drive(IDEBus *bus, int unit, DriveInfo *drive)
>> @@ -159,7 +156,7 @@ typedef struct IDEDrive {
>>  IDEDevice dev;
>>  } IDEDrive;
>>  
>> -static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind)
>> +static void ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind, Error **errp)
>>  {
>>  IDEBus *bus = DO_UPCAST(IDEBus, qbus, dev->qdev.parent_bus);
>>  IDEState *s = bus->ifs + dev->unit;
>> @@ -168,8 +165,8 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind 
>> kind)
>>  
>>  if (!dev->conf.blk) {
>>  if (kind != IDE_CD) {
>> -error_report("No drive specified");
>> -return -1;
>> +error_setg(errp, "No drive specified");
>> +return;
>>  } else {
>>  /* Anonymous BlockBackend for an empty 

[Qemu-devel] [PATCH v2 1/1] target/xtensa: Use the pre-defined MEMTXATTRS_UNSPECIFIED macro

[Alistair Francis]

Instead of using the hardcoded (MemTxAttrs){0} for no memory attributes
let's use the already defined MEMTXATTRS_UNSPECIFIED macro instead.

This is technically a change of behaviour as MEMTXATTRS_UNSPECIFIED sets
the unspecified field to 1, but it doesn't look like anything is
checking this field.

Signed-off-by: Alistair Francis 
---
V2:
 - Update commit message to indicate the change in behaviour

 target/xtensa/op_helper.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/xtensa/op_helper.c b/target/xtensa/op_helper.c
index 519fbeddd6..3d990c0caa 100644
--- a/target/xtensa/op_helper.c
+++ b/target/xtensa/op_helper.c
@@ -1025,11 +1025,11 @@ void HELPER(ule_s)(CPUXtensaState *env, uint32_t br, 
float32 a, float32 b)
 uint32_t HELPER(rer)(CPUXtensaState *env, uint32_t addr)
 {
 return address_space_ldl(env->address_space_er, addr,
- (MemTxAttrs){0}, NULL);
+ MEMTXATTRS_UNSPECIFIED, NULL);
 }
 
 void HELPER(wer)(CPUXtensaState *env, uint32_t data, uint32_t addr)
 {
 address_space_stl(env->address_space_er, addr, data,
-  (MemTxAttrs){0}, NULL);
+  MEMTXATTRS_UNSPECIFIED, NULL);
 }
-- 
2.11.0

Re: [Qemu-devel] [PATCH v2 1/6] hw/ide: Convert DeviceClass init to realize

[John Snow]

On 09/15/2017 05:42 PM, Eric Blake wrote:
> On 09/15/2017 04:35 PM, John Snow wrote:
>>
>>
>> On 08/04/2017 06:26 AM, Mao Zhongyi wrote:
>>> Replace init with realize in IDEDeviceClass, which has errp
>>> as a parameter. So all the implementations now use error_setg
>>> instead of error_report for reporting error.
>>>
> 
>>> @@ -2398,7 +2399,7 @@ int ide_init_drive(IDEState *s, BlockBackend *blk, 
>>> IDEDriveKind kind,
>>> const char *version, const char *serial, const char 
>>> *model,
>>> uint64_t wwn,
>>> uint32_t cylinders, uint32_t heads, uint32_t secs,
>>> -   int chs_trans)
>>> +   int chs_trans, Error **errp)
>>
>> this function now requires an additional invariant, which is that we
>> must return -1 AND set errp. Probably wisest to just get rid of the
>> return code so that we don't accidentally goof this up in the future.
>>
>> I think Markus has had some guidance on this in the past, but admittedly
>> I can't remember his preference.
> 

[...]

> So these days, the preference is to KEEP the redundancy rather than
> eliminate it, due to ease-of-use concerns.
> 

OK, that's fine. I only want consistency with whatever the
paradigm-du-jour is for error handling. Thanks for the quick rebuttal.

--js

Re: [Qemu-devel] [PATCH v2 1/6] hw/ide: Convert DeviceClass init to realize

[Eric Blake]

On 09/15/2017 04:35 PM, John Snow wrote:
> 
> 
> On 08/04/2017 06:26 AM, Mao Zhongyi wrote:
>> Replace init with realize in IDEDeviceClass, which has errp
>> as a parameter. So all the implementations now use error_setg
>> instead of error_report for reporting error.
>>

>> @@ -2398,7 +2399,7 @@ int ide_init_drive(IDEState *s, BlockBackend *blk, 
>> IDEDriveKind kind,
>> const char *version, const char *serial, const char 
>> *model,
>> uint64_t wwn,
>> uint32_t cylinders, uint32_t heads, uint32_t secs,
>> -   int chs_trans)
>> +   int chs_trans, Error **errp)
> 
> this function now requires an additional invariant, which is that we
> must return -1 AND set errp. Probably wisest to just get rid of the
> return code so that we don't accidentally goof this up in the future.
> 
> I think Markus has had some guidance on this in the past, but admittedly
> I can't remember his preference.

Returning void requires callers to use boilerplate:

bar(..., Error **errp)
{
Error *err = NULL;
foo(..., );
if (err) {
error_propagate(errp, err);
handle error ...;
return;
}
}

whereas keeping the invariant that a -1 return is synonymous with err
being set is simpler:

bar(..., Error **errp)
{
if (foo(..., errp) < 0) {
handle error ...;
return;
}
}

So these days, the preference is to KEEP the redundancy rather than
eliminate it, due to ease-of-use concerns.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v2 1/6] hw/ide: Convert DeviceClass init to realize

[John Snow]

On 08/04/2017 06:26 AM, Mao Zhongyi wrote:
> Replace init with realize in IDEDeviceClass, which has errp
> as a parameter. So all the implementations now use error_setg
> instead of error_report for reporting error.
> 
> Cc: John Snow 
> Cc: Markus Armbruster 
> 
> Signed-off-by: Mao Zhongyi 
> ---
>  hw/ide/core.c |  7 ++--
>  hw/ide/qdev.c | 86 
> +++
>  include/hw/ide/internal.h |  5 +--
>  3 files changed, 49 insertions(+), 49 deletions(-)
> 
> diff --git a/hw/ide/core.c b/hw/ide/core.c
> index 0b48b64..7c86fc7 100644
> --- a/hw/ide/core.c
> +++ b/hw/ide/core.c
> @@ -34,6 +34,7 @@
>  #include "hw/block/block.h"
>  #include "sysemu/block-backend.h"
>  #include "qemu/cutils.h"
> +#include "qemu/error-report.h"
>  
>  #include "hw/ide/internal.h"
>  
> @@ -2398,7 +2399,7 @@ int ide_init_drive(IDEState *s, BlockBackend *blk, 
> IDEDriveKind kind,
> const char *version, const char *serial, const char 
> *model,
> uint64_t wwn,
> uint32_t cylinders, uint32_t heads, uint32_t secs,
> -   int chs_trans)
> +   int chs_trans, Error **errp)

this function now requires an additional invariant, which is that we
must return -1 AND set errp. Probably wisest to just get rid of the
return code so that we don't accidentally goof this up in the future.

I think Markus has had some guidance on this in the past, but admittedly
I can't remember his preference.

>  {
>  uint64_t nb_sectors;
>  
> @@ -2423,11 +2424,11 @@ int ide_init_drive(IDEState *s, BlockBackend *blk, 
> IDEDriveKind kind,
>  blk_set_guest_block_size(blk, 2048);
>  } else {
>  if (!blk_is_inserted(s->blk)) {
> -error_report("Device needs media, but drive is empty");
> +error_setg(errp, "Device needs media, but drive is empty");
>  return -1;
>  }
>  if (blk_is_read_only(blk)) {
> -error_report("Can't use a read-only drive");
> +error_setg(errp, "Can't use a read-only drive");
>  return -1;
>  }
>  blk_set_dev_ops(blk, _hd_block_ops, s);
> diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
> index cc2f5bd..d60ac25 100644
> --- a/hw/ide/qdev.c
> +++ b/hw/ide/qdev.c
> @@ -80,7 +80,7 @@ static char *idebus_get_fw_dev_path(DeviceState *dev)
>  return g_strdup(path);
>  }
>  
> -static int ide_qdev_init(DeviceState *qdev)
> +static void ide_qdev_realize(DeviceState *qdev, Error **errp)
>  {
>  IDEDevice *dev = IDE_DEVICE(qdev);
>  IDEDeviceClass *dc = IDE_DEVICE_GET_CLASS(dev);
> @@ -91,34 +91,31 @@ static int ide_qdev_init(DeviceState *qdev)
>  }
>  
>  if (dev->unit >= bus->max_units) {
> -error_report("Can't create IDE unit %d, bus supports only %d units",
> +error_setg(errp, "Can't create IDE unit %d, bus supports only %d 
> units",
>   dev->unit, bus->max_units);
> -goto err;
> +return;
>  }
>  
>  switch (dev->unit) {
>  case 0:
>  if (bus->master) {
> -error_report("IDE unit %d is in use", dev->unit);
> -goto err;
> +error_setg(errp, "IDE unit %d is in use", dev->unit);
> +return;
>  }
>  bus->master = dev;
>  break;
>  case 1:
>  if (bus->slave) {
> -error_report("IDE unit %d is in use", dev->unit);
> -goto err;
> +error_setg(errp, "IDE unit %d is in use", dev->unit);
> +return;
>  }
>  bus->slave = dev;
>  break;
>  default:
> -error_report("Invalid IDE unit %d", dev->unit);
> -goto err;
> +error_setg(errp, "Invalid IDE unit %d", dev->unit);
> +return;
>  }
> -return dc->init(dev);
> -
> -err:
> -return -1;
> +dc->realize(dev, errp);
>  }
>  
>  IDEDevice *ide_create_drive(IDEBus *bus, int unit, DriveInfo *drive)
> @@ -159,7 +156,7 @@ typedef struct IDEDrive {
>  IDEDevice dev;
>  } IDEDrive;
>  
> -static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind)
> +static void ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind, Error **errp)
>  {
>  IDEBus *bus = DO_UPCAST(IDEBus, qbus, dev->qdev.parent_bus);
>  IDEState *s = bus->ifs + dev->unit;
> @@ -168,8 +165,8 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind 
> kind)
>  
>  if (!dev->conf.blk) {
>  if (kind != IDE_CD) {
> -error_report("No drive specified");
> -return -1;
> +error_setg(errp, "No drive specified");
> +return;
>  } else {
>  /* Anonymous BlockBackend for an empty drive */
>  dev->conf.blk = blk_new(0, BLK_PERM_ALL);
> @@ -182,36 +179,36 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind 
> kind)
>  dev->conf.discard_granularity 

Re: [Qemu-devel] [PULL 0/9] Fixes and improvements for various qtests

[Peter Maydell]

On 15 September 2017 at 09:02, Thomas Huth <th...@redhat.com> wrote:
> The following changes since commit 3dabde1128b671f36ac6cb36b97b273139964420:
>
>   Merge remote-tracking branch 'remotes/dgilbert/tags/pull-hmp-20170914' into 
> staging (2017-09-14 16:33:02 +0100)
>
> are available in the git repository at:
>
>   git://github.com/huth/qemu.git tags/check-20170915
>
> for you to fetch changes up to 7b899f4dd596dbb7d271f7fab36fbfffec84868e:
>
>   qtest: Avoid passing raw strings through hmp() (2017-09-15 09:05:19 +0200)
>
> 
> Some fixes and improvements for various qtests by Eric and me.
>
> 
> Eric Blake (5):
>   test-qga: Kill broken and dead QGA_TEST_SIDE_EFFECTING code
>   qtest: Don't perform side effects inside assertion
>   numa-test: Use hmp()
>   libqtest: Remove dead qtest_instances variable
>   qtest: Avoid passing raw strings through hmp()
>
> Thomas Huth (4):
>   tests: Introduce generic device hot-plug/hot-unplug functions
>   tests/test-hmp: Remove puv3 and tricore_testboard from the blacklist
>   tests/libqtest: Use a proper error message if QTEST_QEMU_BINARY is 
> missing
>   tests: Fix broken ivshmem-server-msi/-irq tests

Applied, thanks.

-- PMM

Re: [Qemu-devel] [PATCH v3] linux-user/syscall.c: Handle SH4's exceptional alignment for p{read, write}64

[Philippe Mathieu-Daudé]

On 09/15/2017 04:33 PM, James Clarke wrote:

Fixes: https://bugs.launchpad.net/qemu/+bug/1716767
Signed-off-by: James Clarke 


Reviewed-by: Philippe Mathieu-Daudé 


---

Changes since v2:
  * Fixed opening curly brace formatting, both for my new SH4-specific
regpairs_aligned function, as well as the Arm one I touched, to appease
checkpatch.pl

Changes since v1:
  * Removed all changes in v1 :)
  * Added syscall num argument to regpairs_aligned
  * Added SH4-specific implementation of regpairs_aligned to return 1 for
p{read,write}64

Re: [Qemu-devel] [PULL v4 00/38] Test and build patches

[Fam Zheng]

On Fri, 09/15 11:47, Philippe Mathieu-Daudé wrote:
> why not generate 1 key in
> tests/vm/ (or clever ~/.cache/qemu-vm/ already used by those scripts) once
> when the make vm-test rule is called, that would be 1 key per repository
> clone (or 1 per user using ~/.cache).

Like I explained elsewhere, the BSD images must be generated after the keys in
order for the pub key to be added to the guest authorized_keys. And there is no
automatic way to do so. That's why the keys are committed to the repo, not
generated (otherwise we'd just use ~/.ssh pub keys).

Fam

Re: [Qemu-devel] [PATCH v3] linux-user/syscall.c: Handle SH4's exceptional alignment for p{read, write}64

[John Paul Adrian Glaubitz]

(re-sent because GPG messed up the line endings)

On 09/15/2017 09:33 PM, James Clarke wrote:
> Fixes: https://bugs.launchpad.net/qemu/+bug/1716767
> Signed-off-by: James Clarke 
> ---
> 
> Changes since v2:
>  * Fixed opening curly brace formatting, both for my new SH4-specific
>regpairs_aligned function, as well as the Arm one I touched, to appease
>checkpatch.pl
> 
> Changes since v1:
>  * Removed all changes in v1 :)
>  * Added syscall num argument to regpairs_aligned
>  * Added SH4-specific implementation of regpairs_aligned to return 1 for
>p{read,write}64
> 
>  linux-user/syscall.c | 36 +---
>  1 file changed, 25 insertions(+), 11 deletions(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 9b6364a266..0c1bd80bed 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -667,18 +667,32 @@ static inline int next_free_host_timer(void)
>  
>  /* ARM EABI and MIPS expect 64bit types aligned even on pairs or registers */
>  #ifdef TARGET_ARM
> -static inline int regpairs_aligned(void *cpu_env) {
> +static inline int regpairs_aligned(void *cpu_env, int num)
> +{
>  return CPUARMState *)cpu_env)->eabi) == 1) ;
>  }
>  #elif defined(TARGET_MIPS) && (TARGET_ABI_BITS == 32)
> -static inline int regpairs_aligned(void *cpu_env) { return 1; }
> +static inline int regpairs_aligned(void *cpu_env, int num) { return 1; }
>  #elif defined(TARGET_PPC) && !defined(TARGET_PPC64)
>  /* SysV AVI for PPC32 expects 64bit parameters to be passed on odd/even pairs
>   * of registers which translates to the same as ARM/MIPS, because we start 
> with
>   * r3 as arg1 */
> -static inline int regpairs_aligned(void *cpu_env) { return 1; }
> +static inline int regpairs_aligned(void *cpu_env, int num) { return 1; }
> +#elif defined(TARGET_SH4)
> +/* SH4 doesn't align register pairs, except for p{read,write}64 */
> +static inline int regpairs_aligned(void *cpu_env, int num)
> +{
> +switch (num) {
> +case TARGET_NR_pread64:
> +case TARGET_NR_pwrite64:
> +return 1;
> +
> +default:
> +return 0;
> +}
> +}
>  #else
> -static inline int regpairs_aligned(void *cpu_env) { return 0; }
> +static inline int regpairs_aligned(void *cpu_env, int num) { return 0; }
>  #endif
>  
>  #define ERRNO_TABLE_SIZE 1200
> @@ -6857,7 +6871,7 @@ static inline abi_long target_truncate64(void *cpu_env, 
> const char *arg1,
>   abi_long arg3,
>   abi_long arg4)
>  {
> -if (regpairs_aligned(cpu_env)) {
> +if (regpairs_aligned(cpu_env, TARGET_NR_truncate64)) {
>  arg2 = arg3;
>  arg3 = arg4;
>  }
> @@ -6871,7 +6885,7 @@ static inline abi_long target_ftruncate64(void 
> *cpu_env, abi_long arg1,
>abi_long arg3,
>abi_long arg4)
>  {
> -if (regpairs_aligned(cpu_env)) {
> +if (regpairs_aligned(cpu_env, TARGET_NR_ftruncate64)) {
>  arg2 = arg3;
>  arg3 = arg4;
>  }
> @@ -10495,7 +10509,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
> arg1,
>  #endif
>  #ifdef TARGET_NR_pread64
>  case TARGET_NR_pread64:
> -if (regpairs_aligned(cpu_env)) {
> +if (regpairs_aligned(cpu_env, num)) {
>  arg4 = arg5;
>  arg5 = arg6;
>  }
> @@ -10505,7 +10519,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
> arg1,
>  unlock_user(p, arg2, ret);
>  break;
>  case TARGET_NR_pwrite64:
> -if (regpairs_aligned(cpu_env)) {
> +if (regpairs_aligned(cpu_env, num)) {
>  arg4 = arg5;
>  arg5 = arg6;
>  }
> @@ -11275,7 +11289,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
> arg1,
>  arg6 = ret;
>  #else
>  /* 6 args: fd, offset (high, low), len (high, low), advice */
> -if (regpairs_aligned(cpu_env)) {
> +if (regpairs_aligned(cpu_env, num)) {
>  /* offset is in (3,4), len in (5,6) and advice in 7 */
>  arg2 = arg3;
>  arg3 = arg4;
> @@ -11294,7 +11308,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
> arg1,
>  #ifdef TARGET_NR_fadvise64
>  case TARGET_NR_fadvise64:
>  /* 5 args: fd, offset (high, low), len, advice */
> -if (regpairs_aligned(cpu_env)) {
> +if (regpairs_aligned(cpu_env, num)) {
>  /* offset is in (3,4), len in 5 and advice in 6 */
>  arg2 = arg3;
>  arg3 = arg4;
> @@ -11407,7 +11421,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
> arg1,
>  #ifdef TARGET_NR_readahead
>  case TARGET_NR_readahead:
>  #if TARGET_ABI_BITS == 32
> -if (regpairs_aligned(cpu_env)) {
> +if (regpairs_aligned(cpu_env, num)) {
>  arg2 = arg3;
>  arg3 = arg4;
>  arg4 = arg5;
> 

Tested-By: John Paul 

Re: [Qemu-devel] [PATCH v3] linux-user/syscall.c: Handle SH4's exceptional alignment for p{read, write}64

[John Paul Adrian Glaubitz]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA256

On 09/15/2017 09:33 PM, James Clarke wrote:
> Fixes: https://bugs.launchpad.net/qemu/+bug/1716767 Signed-off-by: James 
> Clarke  ---
> 
> Changes since v2: * Fixed opening curly brace formatting, both for my new 
> SH4-specific regpairs_aligned function, as well as the Arm one I touched, to
> appease checkpatch.pl
> 
> Changes since v1: * Removed all changes in v1 :) * Added syscall num argument 
> to regpairs_aligned * Added SH4-specific implementation of regpairs_aligned
> to return 1 for p{read,write}64
> 
> linux-user/syscall.c | 36 +--- 1 file 
> changed, 25 insertions(+), 11 deletions(-)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c index 
> 9b6364a266..0c1bd80bed 100644 --- a/linux-user/syscall.c +++ 
> b/linux-user/syscall.c @@
> -667,18 +667,32 @@ static inline int next_free_host_timer(void)
> 
> /* ARM EABI and MIPS expect 64bit types aligned even on pairs or registers */ 
> #ifdef TARGET_ARM -static inline int regpairs_aligned(void *cpu_env) { 
> +static inline int regpairs_aligned(void *cpu_env, int num) +{ return 
> CPUARMState *)cpu_env)->eabi) == 1) ; } #elif defined(TARGET_MIPS) &&
> (TARGET_ABI_BITS == 32) -static inline int regpairs_aligned(void *cpu_env) { 
> return 1; } +static inline int regpairs_aligned(void *cpu_env, int num) {
> return 1; } #elif defined(TARGET_PPC) && !defined(TARGET_PPC64) /* SysV AVI 
> for PPC32 expects 64bit parameters to be passed on odd/even pairs * of
> registers which translates to the same as ARM/MIPS, because we start with * 
> r3 as arg1 */ -static inline int regpairs_aligned(void *cpu_env) { return 1; 
> } 
> +static inline int regpairs_aligned(void *cpu_env, int num) { return 1; } 
> +#elif defined(TARGET_SH4) +/* SH4 doesn't align register pairs, except for
> p{read,write}64 */ +static inline int regpairs_aligned(void *cpu_env, int 
> num) +{ +switch (num) { +case TARGET_NR_pread64: +case
> TARGET_NR_pwrite64: +return 1; + +default: +return 0; +   
>  } +} #else -static inline int regpairs_aligned(void *cpu_env) { return 0; } 
> +static inline int regpairs_aligned(void *cpu_env, int num) { return 0; } 
> #endif
> 
> #define ERRNO_TABLE_SIZE 1200 @@ -6857,7 +6871,7 @@ static inline abi_long 
> target_truncate64(void *cpu_env, const char *arg1, abi_long arg3, abi_long
> arg4) { -if (regpairs_aligned(cpu_env)) { +if 
> (regpairs_aligned(cpu_env, TARGET_NR_truncate64)) { arg2 = arg3; arg3 = arg4; 
> } @@ -6871,7 +6885,7 @@
> static inline abi_long target_ftruncate64(void *cpu_env, abi_long arg1, 
> abi_long arg3, abi_long arg4) { -if (regpairs_aligned(cpu_env)) { +if
> (regpairs_aligned(cpu_env, TARGET_NR_ftruncate64)) { arg2 = arg3; arg3 = 
> arg4; } @@ -10495,7 +10509,7 @@ abi_long do_syscall(void *cpu_env, int num,
> abi_long arg1, #endif #ifdef TARGET_NR_pread64 case TARGET_NR_pread64: -  
>   if (regpairs_aligned(cpu_env)) { +if (regpairs_aligned(cpu_env,
> num)) { arg4 = arg5; arg5 = arg6; } @@ -10505,7 +10519,7 @@ abi_long 
> do_syscall(void *cpu_env, int num, abi_long arg1, unlock_user(p, arg2, ret); 
> break; 
> case TARGET_NR_pwrite64: -if (regpairs_aligned(cpu_env)) { +
> if (regpairs_aligned(cpu_env, num)) { arg4 = arg5; arg5 = arg6; } @@ -11275,7
> +11289,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, arg6 = 
> ret; #else /* 6 args: fd, offset (high, low), len (high, low), advice */ -
> if (regpairs_aligned(cpu_env)) { +if (regpairs_aligned(cpu_env, num)) 
> { /* offset is in (3,4), len in (5,6) and advice in 7 */ arg2 = arg3; arg3 =
> arg4; @@ -11294,7 +11308,7 @@ abi_long do_syscall(void *cpu_env, int num, 
> abi_long arg1, #ifdef TARGET_NR_fadvise64 case TARGET_NR_fadvise64: /* 5 args:
> fd, offset (high, low), len, advice */ -if 
> (regpairs_aligned(cpu_env)) { +if (regpairs_aligned(cpu_env, num)) { 
> /* offset is in (3,4), len
> in 5 and advice in 6 */ arg2 = arg3; arg3 = arg4; @@ -11407,7 +11421,7 @@ 
> abi_long do_syscall(void *cpu_env, int num, abi_long arg1, #ifdef
> TARGET_NR_readahead case TARGET_NR_readahead: #if TARGET_ABI_BITS == 32 - 
>if (regpairs_aligned(cpu_env)) { +if (regpairs_aligned(cpu_env,
> num)) { arg2 = arg3; arg3 = arg4; arg4 = arg5;
> 

Tested-By: John Paul Adrian Glaubitz 

- -- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaub...@debian.org
`. `'   Freie Universitaet Berlin - glaub...@physik.fu-berlin.de
  `-GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913
-BEGIN PGP SIGNATURE-

iQIzBAEBCAAdFiEEYv+KdYTgKVaVRgAGdCY7N/W1+RMFAlm8OtoACgkQdCY7N/W1
+RNU1RAAjlDrSn79gJNhsCXBykjVU5i5GrOS5kGSxvs1P494ntprMyraMpcKfFlj
r2jbn9UjTshiHi9GZjsNKQF6FusNYNOPqKIIiY5Cd63yNjymTEvF+p9vlhNhr4TS
fjDZKKWJ9Xs3hzUqRTu6rfbLaG+56Yzd1pkE9iocfGT/r0zXHaSWPnKIBe0uPkkp

Re: [Qemu-devel] [PATCH v3] linux-user/syscall.c: Handle SH4's exceptional alignment for p{read, write}64

[Richard Henderson]

On 09/15/2017 12:33 PM, James Clarke wrote:
> Fixes: https://bugs.launchpad.net/qemu/+bug/1716767
> Signed-off-by: James Clarke 
> ---
> 
> Changes since v2:
>  * Fixed opening curly brace formatting, both for my new SH4-specific
>regpairs_aligned function, as well as the Arm one I touched, to appease
>checkpatch.pl
> 
> Changes since v1:
>  * Removed all changes in v1 :)
>  * Added syscall num argument to regpairs_aligned
>  * Added SH4-specific implementation of regpairs_aligned to return 1 for
>p{read,write}64
> 
>  linux-user/syscall.c | 36 +---
>  1 file changed, 25 insertions(+), 11 deletions(-)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v3] linux-user/syscall.c: Handle SH4's exceptional alignment for p{read, write}64

[Laurent Vivier]

Le 15/09/2017 à 21:33, James Clarke a écrit :
> Fixes: https://bugs.launchpad.net/qemu/+bug/1716767
> Signed-off-by: James Clarke 
> ---
> 
> Changes since v2:
>  * Fixed opening curly brace formatting, both for my new SH4-specific
>regpairs_aligned function, as well as the Arm one I touched, to appease
>checkpatch.pl
> 
> Changes since v1:
>  * Removed all changes in v1 :)
>  * Added syscall num argument to regpairs_aligned
>  * Added SH4-specific implementation of regpairs_aligned to return 1 for
>p{read,write}64
> 
>  linux-user/syscall.c | 36 +---
>  1 file changed, 25 insertions(+), 11 deletions(-)

Reviewed-by: Laurent Vivier 

Re: [Qemu-devel] [PATCH v3] linux-user/syscall.c: Handle SH4's exceptional alignment for p{read, write}64

[Eric Blake]

On 09/15/2017 02:33 PM, James Clarke wrote:
> Fixes: https://bugs.launchpad.net/qemu/+bug/1716767
> Signed-off-by: James Clarke 
> ---
> 
> Changes since v2:
>  * Fixed opening curly brace formatting, both for my new SH4-specific
>regpairs_aligned function, as well as the Arm one I touched, to appease
>checkpatch.pl

It's better to post your v3 as a top-level post rather than in-reply-to
v1 and v2; our automated tooling doesn't always find buried patches as
easily as new threads.  No need to resend for now, but food for thought
if you have to do a v4.

Other patch submission hints at http://wiki.qemu.org/Contribute/SubmitAPatch

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH v3] linux-user/syscall.c: Handle SH4's exceptional alignment for p{read, write}64

[James Clarke]

Fixes: https://bugs.launchpad.net/qemu/+bug/1716767
Signed-off-by: James Clarke 
---

Changes since v2:
 * Fixed opening curly brace formatting, both for my new SH4-specific
   regpairs_aligned function, as well as the Arm one I touched, to appease
   checkpatch.pl

Changes since v1:
 * Removed all changes in v1 :)
 * Added syscall num argument to regpairs_aligned
 * Added SH4-specific implementation of regpairs_aligned to return 1 for
   p{read,write}64

 linux-user/syscall.c | 36 +---
 1 file changed, 25 insertions(+), 11 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 9b6364a266..0c1bd80bed 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -667,18 +667,32 @@ static inline int next_free_host_timer(void)
 
 /* ARM EABI and MIPS expect 64bit types aligned even on pairs or registers */
 #ifdef TARGET_ARM
-static inline int regpairs_aligned(void *cpu_env) {
+static inline int regpairs_aligned(void *cpu_env, int num)
+{
 return CPUARMState *)cpu_env)->eabi) == 1) ;
 }
 #elif defined(TARGET_MIPS) && (TARGET_ABI_BITS == 32)
-static inline int regpairs_aligned(void *cpu_env) { return 1; }
+static inline int regpairs_aligned(void *cpu_env, int num) { return 1; }
 #elif defined(TARGET_PPC) && !defined(TARGET_PPC64)
 /* SysV AVI for PPC32 expects 64bit parameters to be passed on odd/even pairs
  * of registers which translates to the same as ARM/MIPS, because we start with
  * r3 as arg1 */
-static inline int regpairs_aligned(void *cpu_env) { return 1; }
+static inline int regpairs_aligned(void *cpu_env, int num) { return 1; }
+#elif defined(TARGET_SH4)
+/* SH4 doesn't align register pairs, except for p{read,write}64 */
+static inline int regpairs_aligned(void *cpu_env, int num)
+{
+switch (num) {
+case TARGET_NR_pread64:
+case TARGET_NR_pwrite64:
+return 1;
+
+default:
+return 0;
+}
+}
 #else
-static inline int regpairs_aligned(void *cpu_env) { return 0; }
+static inline int regpairs_aligned(void *cpu_env, int num) { return 0; }
 #endif
 
 #define ERRNO_TABLE_SIZE 1200
@@ -6857,7 +6871,7 @@ static inline abi_long target_truncate64(void *cpu_env, 
const char *arg1,
  abi_long arg3,
  abi_long arg4)
 {
-if (regpairs_aligned(cpu_env)) {
+if (regpairs_aligned(cpu_env, TARGET_NR_truncate64)) {
 arg2 = arg3;
 arg3 = arg4;
 }
@@ -6871,7 +6885,7 @@ static inline abi_long target_ftruncate64(void *cpu_env, 
abi_long arg1,
   abi_long arg3,
   abi_long arg4)
 {
-if (regpairs_aligned(cpu_env)) {
+if (regpairs_aligned(cpu_env, TARGET_NR_ftruncate64)) {
 arg2 = arg3;
 arg3 = arg4;
 }
@@ -10495,7 +10509,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
arg1,
 #endif
 #ifdef TARGET_NR_pread64
 case TARGET_NR_pread64:
-if (regpairs_aligned(cpu_env)) {
+if (regpairs_aligned(cpu_env, num)) {
 arg4 = arg5;
 arg5 = arg6;
 }
@@ -10505,7 +10519,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
arg1,
 unlock_user(p, arg2, ret);
 break;
 case TARGET_NR_pwrite64:
-if (regpairs_aligned(cpu_env)) {
+if (regpairs_aligned(cpu_env, num)) {
 arg4 = arg5;
 arg5 = arg6;
 }
@@ -11275,7 +11289,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
arg1,
 arg6 = ret;
 #else
 /* 6 args: fd, offset (high, low), len (high, low), advice */
-if (regpairs_aligned(cpu_env)) {
+if (regpairs_aligned(cpu_env, num)) {
 /* offset is in (3,4), len in (5,6) and advice in 7 */
 arg2 = arg3;
 arg3 = arg4;
@@ -11294,7 +11308,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
arg1,
 #ifdef TARGET_NR_fadvise64
 case TARGET_NR_fadvise64:
 /* 5 args: fd, offset (high, low), len, advice */
-if (regpairs_aligned(cpu_env)) {
+if (regpairs_aligned(cpu_env, num)) {
 /* offset is in (3,4), len in 5 and advice in 6 */
 arg2 = arg3;
 arg3 = arg4;
@@ -11407,7 +11421,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
arg1,
 #ifdef TARGET_NR_readahead
 case TARGET_NR_readahead:
 #if TARGET_ABI_BITS == 32
-if (regpairs_aligned(cpu_env)) {
+if (regpairs_aligned(cpu_env, num)) {
 arg2 = arg3;
 arg3 = arg4;
 arg4 = arg5;
-- 
2.13.2

Re: [Qemu-devel] [PULL 00/18] ppc-for-2.11 queue 20170915

[Peter Maydell]

On 15 September 2017 at 04:51, David Gibson <da...@gibson.dropbear.id.au> wrote:
> The following changes since commit 3dabde1128b671f36ac6cb36b97b273139964420:
>
>   Merge remote-tracking branch 'remotes/dgilbert/tags/pull-hmp-20170914' into 
> staging (2017-09-14 16:33:02 +0100)
>
> are available in the git repository at:
>
>   git://github.com/dgibson/qemu.git tags/ppc-for-2.11-20170915
>
> for you to fetch changes up to 70a0c19e83aa4c71c879d51e426e89e4b3d4e014:
>
>   ppc/kvm: use kvm_vm_check_extension() in kvmppc_is_pr() (2017-09-15 
> 10:29:48 +1000)
>
> 
> ppc patch queue 2017-09-15
>
> Here's the current batch of accumulated ppc patches.  These are all
> pretty simple bugfixes or cleanups, no big new features here.
>

Applied, thanks.

-- PMM

Re: [Qemu-devel] [PATCH v2] linux-user/syscall.c: Handle SH4's exceptional alignment for p{read, write}64

[no-reply]

Hi,

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

Subject: [Qemu-devel] [PATCH v2] linux-user/syscall.c: Handle SH4's exceptional 
alignment for p{read, write}64
Message-id: 20170915190748.82389-1-jrt...@jrtc27.com
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 t [tag update]
patchew/1504888905-22396-1-git-send-email-chugh.ish...@research.iiit.ac.in -> 
patchew/1504888905-22396-1-git-send-email-chugh.ish...@research.iiit.ac.in
Switched to a new branch 'test'
5c4a338e12 linux-user/syscall.c: Handle SH4's exceptional alignment for p{read, 
write}64

=== OUTPUT BEGIN ===
Checking PATCH 1/1: linux-user/syscall.c: Handle SH4's exceptional alignment 
for p{read, write}64...
ERROR: open brace '{' following function declarations go on the next line
#20: FILE: linux-user/syscall.c:670:
+static inline int regpairs_aligned(void *cpu_env, int num) {

ERROR: open brace '{' following function declarations go on the next line
#34: FILE: linux-user/syscall.c:682:
+static inline int regpairs_aligned(void *cpu_env, int num) {

total: 2 errors, 0 warnings, 90 lines checked

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

=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-de...@freelists.org

Re: [Qemu-devel] [PATCH 7/6] qemu-iotests: Test change-backing-file command

[Eric Blake]

On 09/15/2017 12:02 PM, Kevin Wolf wrote:
> This involves a temporary read-write reopen if the backing file link in
> the middle of a backing file chain should be changed and is therefore a
> good test for the latest bdrv_reopen() vs. op blockers fixes.
> 
> Signed-off-by: Kevin Wolf 
> ---
> 
> I actually managed to find a simple case that reproduces the bug that
> is fixed in this series, but outside of my commit job improvements that
> originally led me to this work.
> 
>  tests/qemu-iotests/195 | 92 
> ++
>  tests/qemu-iotests/195.out | 78 +++
>  tests/qemu-iotests/group   |  1 +

The usual collision in test numbering ;)


> +_cleanup()
> +{
> +_cleanup_test_img
> +rm -f "$TEST_IMG.mid"
> +}
> +trap "_cleanup; exit \$status" 0 1 2 3 15

Semantic conflict with Jeff's work to add './check -s' to save
intermediate files in a per-test temp directory.

> +
> +echo
> +echo "Change backing file of mid (opened read-only)"
> +echo
> +
> +run_qemu -drive if=none,file="$TEST_IMG",backing.node-name=mid < +{"execute":"qmp_capabilities"}
> +{"execute":"change-backing-file", 
> "arguments":{"device":"none0","image-node-name":"mid","backing-file":"/dev/null"}}

Since the images don't contain any content, I guess this works.  But
another possible change would be rewriting the backing file to alternate
between absolute and relative (or even between 'file' and './file') (so
that it's pointing to the same file at all times, just by different names)

> +{"execute":"quit"}
> +EOF
> +
> +TEST_IMG="$TEST_IMG.mid" _img_info
> +
> +echo
> +echo "Change backing file of top (opened writable)"
> +echo
> +
> +TEST_IMG="$TEST_IMG.mid" _make_test_img -b "$TEST_IMG.base"
> +
> +run_qemu -drive if=none,file="$TEST_IMG",node-name=top < +{"execute":"qmp_capabilities"}
> +{"execute":"change-backing-file", 
> "arguments":{"device":"none0","image-node-name":"top","backing-file":"/dev/null"}}
> +{"execute":"quit"}

You've quit qemu between runs.  Would we get any further coverage by doing:

change mid
change active
change mid

all within a single run? (That is, make SURE that actions on one part of
the graph don't interfere with later actions elsewhere)

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v4 2/3] backup: Adds Backup Tool

[John Snow]

On 09/08/2017 12:41 PM, Ishani Chugh wrote:
> qemu-backup will be a command-line tool for performing full and
> incremental disk backups on running VMs. It is intended as a
> reference implementation for management stack and backup developers
> to see QEMU's backup features in action. The tool writes details of
> guest in a configuration file and the data is retrieved from the file
> while creating a backup. The location of config file can be set as an
> environment variable QEMU_BACKUP_CONFIG. The usage is as follows:
> 
> Add a guest
> python qemu-backup.py guest add --guest  --qmp 
> 
> Remove a guest
> python qemu-backup.py guest remove --guest 
> 
> List all guest configs in configuration file:
> python qemu-backup.py guest list
> 
> Add a drive for backup in a specified guest
> python qemu-backup.py drive add --guest  --id  
> [--target ]
> 
> Create backup of the added drives:
> python qemu-backup.py backup --guest 
> 
> Restore operation
> python qemu-backup.py restore --guest 
> 
> 
> Signed-off-by: Ishani Chugh 
> ---
>  contrib/backup/qemu-backup.py | 373 
> ++
>  1 file changed, 373 insertions(+)
>  create mode 100755 contrib/backup/qemu-backup.py
> 
> diff --git a/contrib/backup/qemu-backup.py b/contrib/backup/qemu-backup.py
> new file mode 100755
> index 000..7077f68
> --- /dev/null
> +++ b/contrib/backup/qemu-backup.py
> @@ -0,0 +1,373 @@
> +#!/usr/bin/python
> +# -*- coding: utf-8 -*-
> +#
> +# Copyright (C) 2017 Ishani Chugh 
> +#
> +# 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 .
> +#
> +
> +"""
> +This file is an implementation of backup tool
> +"""
> +from __future__ import print_function
> +from argparse import ArgumentParser
> +import os
> +import errno
> +from socket import error as socket_error
> +try:
> +import configparser
> +except ImportError:
> +import ConfigParser as configparser
> +import sys
> +sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..',
> + 'scripts', 'qmp'))
> +from qmp import QEMUMonitorProtocol
> +
> +
> +class BackupTool(object):
> +"""BackupTool Class"""
> +def __init__(self, config_file=os.path.expanduser('~') +
> + '/.config/qemu/qemu-backup-config'):
> +if "QEMU_BACKUP_CONFIG" in os.environ:
> +self.config_file = os.environ["QEMU_BACKUP_CONFIG"]
> +
> +else:
> +self.config_file = config_file
> +try:
> +if not os.path.isdir(os.path.dirname(self.config_file)):
> +os.makedirs(os.path.dirname(self.config_file))
> +except:
> +print("Cannot create config directory", file=sys.stderr)
> +sys.exit(1)
> +self.config = configparser.ConfigParser()
> +self.config.read(self.config_file)
> +try:
> +if self.config.get('general', 'version') != '1.0':
> +print("Version Conflict in config file", file=sys.stderr)
> +sys.exit(1)
> +except:
> +self.config['general'] = {'version': '1.0'}
> +self.write_config()
> +
> +def write_config(self):
> +"""
> +Writes configuration to ini file.
> +"""
> +config_file = open(self.config_file + ".tmp", 'w')
> +self.config.write(config_file)
> +config_file.flush()
> +os.fsync(config_file.fileno())
> +config_file.close()
> +os.rename(self.config_file + ".tmp", self.config_file)
> +
> +def get_socket_address(self, socket_address):
> +"""
> +Return Socket address in form of string or tuple
> +"""
> +if socket_address.startswith('tcp'):
> +return (socket_address.split(':')[1],
> +int(socket_address.split(':')[2]))
> +return socket_address.split(':', 2)[1]
> +
> +def _full_backup(self, guest_name):
> +"""
> +Performs full backup of guest
> +"""
> +self.verify_guest_present(guest_name)
> +self.verify_guest_running(guest_name)
> +connection = QEMUMonitorProtocol(
> + self.get_socket_address(
> + self.config[guest_name]['qmp']))
> +

Re: [Qemu-devel] [PATCH v4 1/3] Add manpage for QEMU Backup Tool

[John Snow]

On 09/08/2017 12:41 PM, Ishani Chugh wrote:
> qemu-backup will be a command-line tool for performing full and
> incremental disk backups on running VMs. It is intended as a
> reference implementation for management stack and backup developers
> to see QEMU's backup features in action. This commit is an
> initial implementation of manpage listing the commands which the
> backup tool will support. The manpage will be built along with other
> docs when configure is provided with --enable-docs flag in the
> location contrib/backup in build directory.
> 
> Signed-off-by: Ishani Chugh 
> ---
>  Makefile|  14 ++--
>  contrib/backup/qemu-backup.texi | 142 
> 
>  2 files changed, 152 insertions(+), 4 deletions(-)
>  create mode 100644 contrib/backup/qemu-backup.texi
> 
> diff --git a/Makefile b/Makefile
> index 337a1f6..794cac5 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -209,6 +209,7 @@ ifdef BUILD_DOCS
>  DOCS=qemu-doc.html qemu-doc.txt qemu.1 qemu-img.1 qemu-nbd.8 qemu-ga.8
>  DOCS+=docs/interop/qemu-qmp-ref.html docs/interop/qemu-qmp-ref.txt 
> docs/interop/qemu-qmp-ref.7
>  DOCS+=docs/interop/qemu-ga-ref.html docs/interop/qemu-ga-ref.txt 
> docs/interop/qemu-ga-ref.7
> +DOCS+=contrib/backup/qemu-backup.html contrib/backup/qemu-backup.txt
>  ifdef CONFIG_VIRTFS
>  DOCS+=fsdev/virtfs-proxy-helper.1
>  endif
> @@ -517,6 +518,8 @@ VERSION ?= $(shell cat VERSION)
> 
>  dist: qemu-$(VERSION).tar.bz2
> 
> +qemu-backup.8: contrib/backup/qemu-backup.texi
> +
>  qemu-%.tar.bz2:
>   $(SRC_PATH)/scripts/make-release "$(SRC_PATH)" "$(patsubst 
> qemu-%.tar.bz2,%,$@)"
> 
> @@ -728,16 +731,19 @@ fsdev/virtfs-proxy-helper.1: 
> fsdev/virtfs-proxy-helper.texi
>  qemu-nbd.8: qemu-nbd.texi qemu-option-trace.texi
>  qemu-ga.8: qemu-ga.texi
> 
> -html: qemu-doc.html docs/interop/qemu-qmp-ref.html 
> docs/interop/qemu-ga-ref.html
> -info: qemu-doc.info docs/interop/qemu-qmp-ref.info 
> docs/interop/qemu-ga-ref.info
> -pdf: qemu-doc.pdf docs/interop/qemu-qmp-ref.pdf docs/interop/qemu-ga-ref.pdf
> -txt: qemu-doc.txt docs/interop/qemu-qmp-ref.txt docs/interop/qemu-ga-ref.txt
> +html: qemu-doc.html docs/interop/qemu-qmp-ref.html 
> docs/interop/qemu-ga-ref.html contrib/backup/qemu-backup.html
> +info: qemu-doc.info docs/interop/qemu-qmp-ref.info 
> docs/interop/qemu-ga-ref.info contrib/backup/qemu-backup.info
> +pdf: qemu-doc.pdf docs/interop/qemu-qmp-ref.pdf docs/interop/qemu-ga-ref.pdf 
> contrib/backup/qemu-backup.pdf
> +txt: qemu-doc.txt docs/interop/qemu-qmp-ref.txt docs/interop/qemu-ga-ref.txt 
> contrib/backup/qemu-backup.txt
> 
>  qemu-doc.html qemu-doc.info qemu-doc.pdf qemu-doc.txt: \
>   qemu-img.texi qemu-nbd.texi qemu-options.texi qemu-option-trace.texi \
>   qemu-monitor.texi qemu-img-cmds.texi qemu-ga.texi \
>   qemu-monitor-info.texi
> 
> +contrib/backup/qemu-backup.html contrib/backup/qemu-backup.pdf 
> contrib/backup/qemu-backup.txt contrib/backup/qemu-backup.info: \
> + contrib/backup/qemu-backup.texi
> +
>  docs/interop/qemu-ga-ref.dvi docs/interop/qemu-ga-ref.html \
>  docs/interop/qemu-ga-ref.info docs/interop/qemu-ga-ref.pdf \
>  docs/interop/qemu-ga-ref.txt docs/interop/qemu-ga-ref.7: \
> diff --git a/contrib/backup/qemu-backup.texi b/contrib/backup/qemu-backup.texi
> new file mode 100644
> index 000..7ad266c
> --- /dev/null
> +++ b/contrib/backup/qemu-backup.texi
> @@ -0,0 +1,142 @@
> +\input texinfo
> +@setfilename qemu-backup
> +
> +@documentlanguage en
> +@documentencoding UTF-8
> +
> +@settitle QEMU Backup Tool
> +@copying
> +
> +Copyright @copyright{} 2017 The QEMU Project developers
> +@end copying
> +@ifinfo
> +@direntry
> +* QEMU: (QEMU-backup).Man page for QEMU Backup Tool.
> +@end direntry
> +@end ifinfo
> +@iftex
> +@titlepage
> +@sp 7
> +@center @titlefont{QEMU Backup Tool}
> +@sp 1
> +@sp 3
> +@end titlepage
> +@end iftex
> +@ifnottex
> +@node Top
> +@top Short Sample
> +
> +@menu
> +* Name::
> +* Synopsis::
> +* List of Commands::
> +* Command Parameters::
> +* Command Descriptions::
> +* License::
> +@end menu
> +
> +@end ifnottex
> +
> +@node Name
> +@chapter Name
> +
> +QEMU disk backup tool.
> +
> +@node Synopsis
> +@chapter Synopsis
> +
> +qemu-backup command [command options].
> +
> +@node  List of Commands
> +@chapter  List of Commands
> +@itemize
> +@item qemu-backup guest add --guest guestname --qmp socketpath
> +@item qemu-backup guest list
> +@item qemu-backup drive add --id driveid --guest guestname --target target
> +@item qemu-backup drive add --all --guest guestname --target target
> +@item qemu-backup drive list --guest guestname
> +@item qemu-backup restore --guest guestname
> +@item qemu-backup guest remove --guest guestname
> +@item qemu-backup drive remove --guest guestname --id driveid
> +@end itemize
> +@node  Command Parameters
> +@chapter  Command Parameters
> +@itemize
> +@item --all: Add all the drives present in a guest which are 

[Qemu-devel] [PATCH v2] linux-user/syscall.c: Handle SH4's exceptional alignment for p{read, write}64

[James Clarke]

Fixes: https://bugs.launchpad.net/qemu/+bug/1716767
Signed-off-by: James Clarke 
---

Changes since v1:
 * Removed all changes in v1 :)
 * Added syscall num argument to regpairs_aligned
 * Added SH4-specific implementation of regpairs_aligned to return 1 for
   p{read,write}64

 linux-user/syscall.c | 34 +++---
 1 file changed, 23 insertions(+), 11 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 9b6364a266..492c654970 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -667,18 +667,30 @@ static inline int next_free_host_timer(void)
 
 /* ARM EABI and MIPS expect 64bit types aligned even on pairs or registers */
 #ifdef TARGET_ARM
-static inline int regpairs_aligned(void *cpu_env) {
+static inline int regpairs_aligned(void *cpu_env, int num) {
 return CPUARMState *)cpu_env)->eabi) == 1) ;
 }
 #elif defined(TARGET_MIPS) && (TARGET_ABI_BITS == 32)
-static inline int regpairs_aligned(void *cpu_env) { return 1; }
+static inline int regpairs_aligned(void *cpu_env, int num) { return 1; }
 #elif defined(TARGET_PPC) && !defined(TARGET_PPC64)
 /* SysV AVI for PPC32 expects 64bit parameters to be passed on odd/even pairs
  * of registers which translates to the same as ARM/MIPS, because we start with
  * r3 as arg1 */
-static inline int regpairs_aligned(void *cpu_env) { return 1; }
+static inline int regpairs_aligned(void *cpu_env, int num) { return 1; }
+#elif defined(TARGET_SH4)
+/* SH4 doesn't align register pairs, except for p{read,write}64 */
+static inline int regpairs_aligned(void *cpu_env, int num) {
+switch (num) {
+case TARGET_NR_pread64:
+case TARGET_NR_pwrite64:
+return 1;
+
+default:
+return 0;
+}
+}
 #else
-static inline int regpairs_aligned(void *cpu_env) { return 0; }
+static inline int regpairs_aligned(void *cpu_env, int num) { return 0; }
 #endif
 
 #define ERRNO_TABLE_SIZE 1200
@@ -6857,7 +6869,7 @@ static inline abi_long target_truncate64(void *cpu_env, 
const char *arg1,
  abi_long arg3,
  abi_long arg4)
 {
-if (regpairs_aligned(cpu_env)) {
+if (regpairs_aligned(cpu_env, TARGET_NR_truncate64)) {
 arg2 = arg3;
 arg3 = arg4;
 }
@@ -6871,7 +6883,7 @@ static inline abi_long target_ftruncate64(void *cpu_env, 
abi_long arg1,
   abi_long arg3,
   abi_long arg4)
 {
-if (regpairs_aligned(cpu_env)) {
+if (regpairs_aligned(cpu_env, TARGET_NR_ftruncate64)) {
 arg2 = arg3;
 arg3 = arg4;
 }
@@ -10495,7 +10507,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
arg1,
 #endif
 #ifdef TARGET_NR_pread64
 case TARGET_NR_pread64:
-if (regpairs_aligned(cpu_env)) {
+if (regpairs_aligned(cpu_env, num)) {
 arg4 = arg5;
 arg5 = arg6;
 }
@@ -10505,7 +10517,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
arg1,
 unlock_user(p, arg2, ret);
 break;
 case TARGET_NR_pwrite64:
-if (regpairs_aligned(cpu_env)) {
+if (regpairs_aligned(cpu_env, num)) {
 arg4 = arg5;
 arg5 = arg6;
 }
@@ -11275,7 +11287,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
arg1,
 arg6 = ret;
 #else
 /* 6 args: fd, offset (high, low), len (high, low), advice */
-if (regpairs_aligned(cpu_env)) {
+if (regpairs_aligned(cpu_env, num)) {
 /* offset is in (3,4), len in (5,6) and advice in 7 */
 arg2 = arg3;
 arg3 = arg4;
@@ -11294,7 +11306,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
arg1,
 #ifdef TARGET_NR_fadvise64
 case TARGET_NR_fadvise64:
 /* 5 args: fd, offset (high, low), len, advice */
-if (regpairs_aligned(cpu_env)) {
+if (regpairs_aligned(cpu_env, num)) {
 /* offset is in (3,4), len in 5 and advice in 6 */
 arg2 = arg3;
 arg3 = arg4;
@@ -11407,7 +11419,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
arg1,
 #ifdef TARGET_NR_readahead
 case TARGET_NR_readahead:
 #if TARGET_ABI_BITS == 32
-if (regpairs_aligned(cpu_env)) {
+if (regpairs_aligned(cpu_env, num)) {
 arg2 = arg3;
 arg3 = arg4;
 arg4 = arg5;
-- 
2.13.2

Re: [Qemu-devel] [PATCH 6/6] block: Fix permissions after bdrv_reopen()

[Eric Blake]

On 09/15/2017 05:10 AM, Kevin Wolf wrote:
> If we switch between read-only and read-write, the permissions that
> image format drivers need on bs->file change, too. Make sure to update
> the permissions during bdrv_reopen().
> 
> Signed-off-by: Kevin Wolf 
> ---
>  include/block/block.h |  1 +
>  block.c   | 64 
> +++
>  2 files changed, 65 insertions(+)
> 

> +static BlockReopenQueueEntry *find_parent_in_reopen_queue(BlockReopenQueue 
> *q,
> +  BdrvChild *c)
> +{
> +BlockReopenQueueEntry *entry;
> +
> +QSIMPLEQ_FOREACH(entry, q, entry) {
> +BlockDriverState *bs = entry->state.bs;
> +BdrvChild *child;
> +
> +QLIST_FOREACH(child, >children, next) {
> +if (child == c) {
> +return entry;

An O(n^2) loop. Is it going to bite us at any point in the future, or
are we generally dealing with a small enough queue size and BDS graph to
not worry about it?

Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 5/6] block: reopen: Queue children after their parents

[Eric Blake]

On 09/15/2017 05:10 AM, Kevin Wolf wrote:
> We will calculate the required new permissions in the prepare stage of a
> reopen. Required permissions of children can be influenced by the
> changes made to their parents, but parents are independent from their
> children. This means that permissions need to be calculated top-down. In
> order to achieve this, queue parents before their children rather than
> queuing the children first.
> 
> Signed-off-by: Kevin Wolf 
> ---
>  block.c | 26 +-
>  1 file changed, 13 insertions(+), 13 deletions(-)
> 

Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 4/6] block: Base permissions on rw state after reopen

[Eric Blake]

On 09/15/2017 05:10 AM, Kevin Wolf wrote:
> When new permissions are calculated during bdrv_reopen(), they need to
> be based on the state of the graph as it will be after the reopen has
> completed, not on the current state of the involved nodes.
> 
> This patch makes bdrv_is_writable() optionally accept a BlockReopenQueue
> from which the new flags are taken. This is then used for determining
> the new bs->file permissions of format drivers as soon as we add the
> code to actually pass a non-NULL reopen queue to the .bdrv_child_perm
> callbacks.
> 
> While moving bdrv_is_writable(), make it static. It isn't used outside
> block.c.
> 
> Signed-off-by: Kevin Wolf 
> ---
>  include/block/block.h |  1 -
>  block.c   | 52 
> ---
>  2 files changed, 37 insertions(+), 16 deletions(-)
> 

> + * Return the flags that @bs will have after the reopens in @q have
> + * successfully completed. If @q is NULL (or @bs is not contained in @q),
> + * return the current flags.
> + */
> +static int bdrv_reopen_get_flags(BlockReopenQueue *q, BlockDriverState *bs)

> +/* Returns whether the image file can be written to after the reopen queue @q
> + * has been successfully applied, or right now if @q is NULL. */
> +static bool bdrv_is_writable(BlockDriverState *bs, BlockReopenQueue *q)

Is it worth having both functions with arguments in the same order?

> +{
> +int flags = bdrv_reopen_get_flags(q, bs);
> +

No real semantic impact to leave it as is, but it would avoid the odd
swap of arguments here.  So either way,

Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH] linux-user/syscall.c: Handle SH4's exceptional alignment for p{read, write}64

[Laurent Vivier]

Le 15/09/2017 à 17:41, Philippe Mathieu-Daudé a écrit :
> On 09/15/2017 03:58 AM, James Clarke wrote:
>> Fixes: https://bugs.launchpad.net/qemu/+bug/1716767
>> Signed-off-by: James Clarke 
> 
> Congratulations! You have won yourself a R: entry (Designated reviewer)
> in the "Linux user" and "SH4" sections of MAINTAINERS!
> 
>> ---
>>   linux-user/syscall.c | 12 
>>   1 file changed, 12 insertions(+)
>>
>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>> index 9b6364a266..24d6a81c21 100644
>> --- a/linux-user/syscall.c
>> +++ b/linux-user/syscall.c
>> @@ -10495,20 +10495,32 @@ abi_long do_syscall(void *cpu_env, int num,
>> abi_long arg1,
>>   #endif
>>   #ifdef TARGET_NR_pread64
>>   case TARGET_NR_pread64:
>> +#if defined(TARGET_SH4)
>> +    /* SH4 doesn't align register pairs, except for
>> p{read,write}64 */
>> +    arg4 = arg5;
>> +    arg5 = arg6;
>> +#else
>>   if (regpairs_aligned(cpu_env)) {
>>   arg4 = arg5;
>>   arg5 = arg6;
>>   }
>> +#endif
> 
> I'd rather use arch_type from "sysemu/arch_init.h":
> 
>   case TARGET_NR_pwrite64:
>   /* SH4 doesn't align register pairs, except for p{read,write}64 */
>   if (arch_type == QEMU_ARCH_SH4 || regpairs_aligned(cpu_env)) {
>   arg4 = arg5;
>   arg5 = arg6;
>   }
> 
> What do you think?

For the moment, arch_type is only available for system targets.

Laurent

Re: [Qemu-devel] [PATCH 3/6] block: Add reopen queue to bdrv_check_perm()

[Eric Blake]

On 09/15/2017 05:10 AM, Kevin Wolf wrote:
> In the context of bdrv_reopen(), we'll have to look at the state of the
> graph as it will be after the reopen. This interface addition is in
> preparation for the change.
> 
> Signed-off-by: Kevin Wolf 
> ---
>  block.c | 34 +++---
>  1 file changed, 19 insertions(+), 15 deletions(-)
> 

Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [RFC 14/15] qmp: support out-of-band (oob) execution

[Eric Blake]

On 09/14/2017 09:59 PM, Peter Xu wrote:
> On Thu, Sep 14, 2017 at 04:33:34PM +0100, Stefan Hajnoczi wrote:
>> On Thu, Sep 14, 2017 at 03:50:35PM +0800, Peter Xu wrote:
>>> diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt
>>> index 61fa167..47d16bb 100644
>>> --- a/docs/devel/qapi-code-gen.txt
>>> +++ b/docs/devel/qapi-code-gen.txt

>>
>> Is there a more relevant place to document QMP run-oob behavior than the
>> "How to use the QAPI code generator document"?
> 
> I agree, but I don't really know it. :(
> 
> Markus, could you provide a hint?

I'm not Markus, but I suggest docs/interop/qmp-spec.txt

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v5 08/12] char: convert the escc device to keycodemapdb

[Programmingkid]

Sorry but I saw this error when I tried your patches:

  OBJCui/cocoa.o
  CC  ui/curses.o
ui/input-keymap.c:17:10: fatal error: 'ui/input-keymap-qcode-to-sun.c' file not
  found
#include "ui/input-keymap-qcode-to-sun.c"
 ^
1 error generated.
make: *** [ui/input-keymap.o] Error 1
make: *** Waiting for unfinished jobs


I could not find this file. 

Re: [Qemu-devel] [PATCH RFC 0/6] xen: xen-domid-restrict improvements

[no-reply]

Hi,

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

Subject: [Qemu-devel] [PATCH RFC 0/6] xen: xen-domid-restrict improvements
Message-id: 1505498999-17427-1-git-send-email-ian.jack...@eu.citrix.com
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
d0bf857e06 os-posix: Provide new -runasid option
569ba00e8f xen: destroy_hvm_domain: Try xendevicemodel_shutdown
1ef23a12d2 xen: destroy_hvm_domain: Move reason into a variable
af7aac4055 xen: restrict: use xentoolcore_restrict_all
b84c49e46a xen: defer call to xen_restrict until running
293b6b0146 xen: link against xentoolcore

=== OUTPUT BEGIN ===
Checking PATCH 1/6: xen: link against xentoolcore...
Checking PATCH 2/6: xen: defer call to xen_restrict until running...
Checking PATCH 3/6: xen: restrict: use xentoolcore_restrict_all...
Checking PATCH 4/6: xen: destroy_hvm_domain: Move reason into a variable...
Checking PATCH 5/6: xen: destroy_hvm_domain: Try xendevicemodel_shutdown...
ERROR: braces {} are necessary for all arms of this statement
#27: FILE: hw/i386/xen/xen-hvm.c:1395:
+if (!rc)
[...]

total: 1 errors, 0 warnings, 18 lines checked

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

Checking PATCH 6/6: os-posix: Provide new -runasid option...
ERROR: consider using qemu_strtoul in preference to strtoul
#41: FILE: os-posix.c:159:
+lv = strtoul(optarg, , 0);

ERROR: do not use assignment in if condition
#42: FILE: os-posix.c:160:
+if (errno || *ep != '.' || (user_uid = lv) != lv

ERROR: spaces required around that '+' (ctx:VxV)
#48: FILE: os-posix.c:166:
+lv = strtoul(ep+1, , 0);
^

ERROR: consider using qemu_strtoul in preference to strtoul
#48: FILE: os-posix.c:166:
+lv = strtoul(ep+1, , 0);

ERROR: do not use assignment in if condition
#49: FILE: os-posix.c:167:
+if (errno || *ep || (user_gid = lv) != lv

ERROR: space required after that ',' (ctx:WxV)
#51: FILE: os-posix.c:169:
+fprintf(stderr ,"Could not obtain gid from \"%s\"", optarg);
^

total: 6 errors, 0 warnings, 79 lines checked

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

=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-de...@freelists.org

Re: [Qemu-devel] [PATCH RFC 0/6] xen: xen-domid-restrict improvements

[no-reply]

Hi,

This series failed automatic build test. Please find the testing commands and
their output below. If you have docker installed, you can probably reproduce it
locally.

Subject: [Qemu-devel] [PATCH RFC 0/6] xen: xen-domid-restrict improvements
Message-id: 1505498999-17427-1-git-send-email-ian.jack...@eu.citrix.com
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash
set -e
git submodule update --init dtc
# Let docker tests dump environment info
export SHOW_ENV=1
export J=8
time make docker-test-quick@centos6
time make docker-test-build@min-glib
time make docker-test-mingw@fedora
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]   
patchew/1505498999-17427-1-git-send-email-ian.jack...@eu.citrix.com -> 
patchew/1505498999-17427-1-git-send-email-ian.jack...@eu.citrix.com
Switched to a new branch 'test'
d0bf857e06 os-posix: Provide new -runasid option
569ba00e8f xen: destroy_hvm_domain: Try xendevicemodel_shutdown
1ef23a12d2 xen: destroy_hvm_domain: Move reason into a variable
af7aac4055 xen: restrict: use xentoolcore_restrict_all
b84c49e46a xen: defer call to xen_restrict until running
293b6b0146 xen: link against xentoolcore

=== OUTPUT BEGIN ===
Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
Cloning into '/var/tmp/patchew-tester-tmp-b93ibrpf/src/dtc'...
Submodule path 'dtc': checked out '558cd81bdd432769b59bff01240c44f82cfb1a9d'
  BUILD   centos6
make[1]: Entering directory '/var/tmp/patchew-tester-tmp-b93ibrpf/src'
  ARCHIVE qemu.tgz
  ARCHIVE dtc.tgz
  COPYRUNNER
RUN test-quick in qemu:centos6 
Packages installed:
SDL-devel-1.2.14-7.el6_7.1.x86_64
bison-2.4.1-5.el6.x86_64
bzip2-devel-1.0.5-7.el6_0.x86_64
ccache-3.1.6-2.el6.x86_64
csnappy-devel-0-6.20150729gitd7bc683.el6.x86_64
flex-2.5.35-9.el6.x86_64
gcc-4.4.7-18.el6.x86_64
git-1.7.1-8.el6.x86_64
glib2-devel-2.28.8-9.el6.x86_64
libepoxy-devel-1.2-3.el6.x86_64
libfdt-devel-1.4.0-1.el6.x86_64
librdmacm-devel-1.0.21-0.el6.x86_64
lzo-devel-2.03-3.1.el6_5.1.x86_64
make-3.81-23.el6.x86_64
mesa-libEGL-devel-11.0.7-4.el6.x86_64
mesa-libgbm-devel-11.0.7-4.el6.x86_64
package g++ is not installed
pixman-devel-0.32.8-1.el6.x86_64
spice-glib-devel-0.26-8.el6.x86_64
spice-server-devel-0.12.4-16.el6.x86_64
tar-1.23-15.el6_8.x86_64
vte-devel-0.25.1-9.el6.x86_64
xen-devel-4.6.3-15.el6.x86_64
zlib-devel-1.2.3-29.el6.x86_64

Environment variables:
PACKAGES=bison bzip2-devel ccache csnappy-devel flex g++
 gcc git glib2-devel libepoxy-devel libfdt-devel 
librdmacm-devel lzo-devel make mesa-libEGL-devel 
mesa-libgbm-devel pixman-devel SDL-devel spice-glib-devel 
spice-server-devel tar vte-devel xen-devel zlib-devel
HOSTNAME=324cd3b04cae
TERM=xterm
MAKEFLAGS= -j8
HISTSIZE=1000
J=8
USER=root
LS_COLORS=rs=0:di=01;34:ln=01;36:mh=00:pi=40;33:so=01;35:do=01;35:bd=40;33;01:cd=40;33;01:or=40;31;01:mi=01;05;37;41:su=37;41:sg=30;43:ca=30;41:tw=30;42:ow=34;42:st=37;44:ex=01;32:*.tar=01;31:*.tgz=01;31:*.arj=01;31:*.taz=01;31:*.lzh=01;31:*.lzma=01;31:*.tlz=01;31:*.txz=01;31:*.zip=01;31:*.z=01;31:*.Z=01;31:*.dz=01;31:*.gz=01;31:*.lz=01;31:*.xz=01;31:*.bz2=01;31:*.tbz=01;31:*.tbz2=01;31:*.bz=01;31:*.tz=01;31:*.deb=01;31:*.rpm=01;31:*.jar=01;31:*.rar=01;31:*.ace=01;31:*.zoo=01;31:*.cpio=01;31:*.7z=01;31:*.rz=01;31:*.jpg=01;35:*.jpeg=01;35:*.gif=01;35:*.bmp=01;35:*.pbm=01;35:*.pgm=01;35:*.ppm=01;35:*.tga=01;35:*.xbm=01;35:*.xpm=01;35:*.tif=01;35:*.tiff=01;35:*.png=01;35:*.svg=01;35:*.svgz=01;35:*.mng=01;35:*.pcx=01;35:*.mov=01;35:*.mpg=01;35:*.mpeg=01;35:*.m2v=01;35:*.mkv=01;35:*.ogm=01;35:*.mp4=01;35:*.m4v=01;35:*.mp4v=01;35:*.vob=01;35:*.qt=01;35:*.nuv=01;35:*.wmv=01;35:*.asf=01;35:*.rm=01;35:*.rmvb=01;35:*.flc=01;35:*.avi=01;35:*.fli=01;35:*.flv=01;35:*.gl=01;35:*.dl=01;35:*.xcf=01;35:*.xwd=01;35:*.yuv=01;35:*.cgm=01;35:*.emf=01;35:*.axv=01;35:*.anx=01;35:*.ogv=01;35:*.ogx=01;35:*.aac=01;36:*.au=01;36:*.flac=01;36:*.mid=01;36:*.midi=01;36:*.mka=01;36:*.mp3=01;36:*.mpc=01;36:*.ogg=01;36:*.ra=01;36:*.wav=01;36:*.axa=01;36:*.oga=01;36:*.spx=01;36:*.xspf=01;36:
CCACHE_DIR=/var/tmp/ccache
EXTRA_CONFIGURE_OPTS=
V=
SHOW_ENV=1
MAIL=/var/spool/mail/root
PATH=/usr/lib/ccache:/usr/lib64/ccache:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
PWD=/
LANG=en_US.UTF-8
TARGET_LIST=
HISTCONTROL=ignoredups
SHLVL=1
HOME=/root
TEST_DIR=/tmp/qemu-test
LOGNAME=root
LESSOPEN=||/usr/bin/lesspipe.sh %s
FEATURES= dtc
DEBUG=
G_BROKEN_FILENAMES=1
CCACHE_HASHDIR=
_=/usr/bin/env

Configure options:
--enable-werror --target-list=x86_64-softmmu,aarch64-softmmu 
--prefix=/var/tmp/qemu-build/install
No C++ compiler available; disabling C++ specific optional code
Install prefix/var/tmp/qemu-build/install
BIOS directory/var/tmp/qemu-build/install/share/qemu
binary directory  /var/tmp/qemu-build/install/bin
library directory /var/tmp/qemu-build/install/lib
module directory  

Re: [Qemu-devel] [PATCH] iotests: Print full path of bad output if mismatch

[Eric Blake]

On 09/15/2017 12:45 AM, Fam Zheng wrote:
> So it is easier to copy paste the path.
> 
> Signed-off-by: Fam Zheng 
> ---
>  tests/qemu-iotests/check | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
> index d504b6e455..4583a0c269 100755
> --- a/tests/qemu-iotests/check
> +++ b/tests/qemu-iotests/check
> @@ -353,7 +353,7 @@ do
>  else
>  echo " - output mismatch (see $seq.out.bad)"
>  mv $tmp.out $seq.out.bad
> -$diff -w "$reference" $seq.out.bad
> +$diff -w "$reference" $(realpath $seq.out.bad)

Realpath collapses symlinks, and $() costs a process.  Also, your patch
fails miserably if the current working directory contains spaces; you
forgot to quote $().  It would be simpler, and more correct, to do:

$diff -w "$reference" "$PWD/$seq.out.bad"

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH RFC 3/6] xen: restrict: use xentoolcore_restrict_all

[Ian Jackson]

And insist that it works.

Signed-off-by: Ian Jackson 
---
 include/hw/xen/xen_common.h | 26 ++
 1 file changed, 2 insertions(+), 24 deletions(-)

diff --git a/include/hw/xen/xen_common.h b/include/hw/xen/xen_common.h
index 86c7f26..b6cb024 100644
--- a/include/hw/xen/xen_common.h
+++ b/include/hw/xen/xen_common.h
@@ -12,6 +12,7 @@
 
 #include 
 #include 
+#include 
 #include 
 
 #include "hw/hw.h"
@@ -289,30 +290,7 @@ static inline int xen_modified_memory(domid_t domid, 
uint64_t first_pfn,
 
 static inline int xen_restrict(domid_t domid)
 {
-int rc;
-
-/* Attempt to restrict devicemodel operations */
-rc = xendevicemodel_restrict(xen_dmod, domid);
-trace_xen_domid_restrict(rc ? errno : 0);
-
-if (rc < 0) {
-/*
- * If errno is ENOTTY then restriction is not implemented so
- * there's no point in trying to restrict other types of
- * operation, but it should not be treated as a failure.
- */
-if (errno == ENOTTY) {
-return 0;
-}
-
-return rc;
-}
-
-/* Restrict foreignmemory operations */
-rc = xenforeignmemory_restrict(xen_fmem, domid);
-trace_xen_domid_restrict(rc ? errno : 0);
-
-return rc;
+return xentoolcore_restrict_all(domid);
 }
 
 void destroy_hvm_domain(bool reboot);
-- 
2.1.4

[Qemu-devel] [PATCH RFC 6/6] os-posix: Provide new -runasid option

[Ian Jackson]

This allows the caller to specify a uid and gid to use, even if there
is no corresponding password entry.  This will be useful in certain
Xen configurations.

Signed-off-by: Ian Jackson 
---
 os-posix.c  | 30 ++
 qemu-options.hx | 12 
 2 files changed, 38 insertions(+), 4 deletions(-)

diff --git a/os-posix.c b/os-posix.c
index 92e9d85..b88995e 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -43,6 +43,8 @@
 #endif
 
 static struct passwd *user_pwd;
+static uid_t user_uid = (uid_t)-1;
+static gid_t user_gid = (gid_t)-1;
 static const char *chroot_dir;
 static int daemonize;
 static int daemon_pipe;
@@ -134,6 +136,8 @@ void os_set_proc_name(const char *s)
  */
 void os_parse_cmd_args(int index, const char *optarg)
 {
+unsigned long lv;
+char *ep;
 switch (index) {
 #ifdef CONFIG_SLIRP
 case QEMU_OPTION_smb:
@@ -150,6 +154,22 @@ void os_parse_cmd_args(int index, const char *optarg)
 exit(1);
 }
 break;
+case QEMU_OPTION_runasid:
+errno = 0;
+lv = strtoul(optarg, , 0);
+if (errno || *ep != '.' || (user_uid = lv) != lv
+|| (user_uid == (uid_t)-1)) {
+fprintf(stderr, "Could not obtain uid from \"%s\"", optarg);
+exit(1);
+}
+errno = 0;
+lv = strtoul(ep+1, , 0);
+if (errno || *ep || (user_gid = lv) != lv
+|| (user_gid == (gid_t)-1)) {
+fprintf(stderr ,"Could not obtain gid from \"%s\"", optarg);
+exit(1);
+}
+break;
 case QEMU_OPTION_chroot:
 chroot_dir = optarg;
 break;
@@ -166,17 +186,19 @@ void os_parse_cmd_args(int index, const char *optarg)
 
 static void change_process_uid(void)
 {
-if (user_pwd) {
-if (setgid(user_pwd->pw_gid) < 0) {
+if (user_pwd || user_uid != (uid_t)-1) {
+if (setgid(user_pwd ? user_pwd->pw_gid : user_gid) < 0) {
 fprintf(stderr, "Failed to setgid(%d)\n", user_pwd->pw_gid);
 exit(1);
 }
-if (initgroups(user_pwd->pw_name, user_pwd->pw_gid) < 0) {
+if ((user_pwd
+ ? initgroups(user_pwd->pw_name, user_pwd->pw_gid)
+ : setgroups(1, _gid)) < 0) {
 fprintf(stderr, "Failed to initgroups(\"%s\", %d)\n",
 user_pwd->pw_name, user_pwd->pw_gid);
 exit(1);
 }
-if (setuid(user_pwd->pw_uid) < 0) {
+if (setuid(user_pwd ? user_pwd->pw_uid : user_gid) < 0) {
 fprintf(stderr, "Failed to setuid(%d)\n", user_pwd->pw_uid);
 exit(1);
 }
diff --git a/qemu-options.hx b/qemu-options.hx
index 9f6e2ad..34a5329 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -3968,6 +3968,18 @@ Immediately before starting guest execution, drop root 
privileges, switching
 to the specified user.
 ETEXI
 
+#ifndef _WIN32
+DEF("runasid", HAS_ARG, QEMU_OPTION_runasid, \
+"-runasid uid.gid change to numeric uid and gid just before starting 
the VM\n",
+QEMU_ARCH_ALL)
+#endif
+STEXI
+@item -runasid @var{uid}.@var{gid}
+@findex -runasid
+Immediately before starting guest execution, drop root privileges, switching
+to the specified uid and gid.
+ETEXI
+
 DEF("prom-env", HAS_ARG, QEMU_OPTION_prom_env,
 "-prom-env variable=value\n"
 "set OpenBIOS nvram variables\n",
-- 
2.1.4

[Qemu-devel] [PATCH RFC 5/6] xen: destroy_hvm_domain: Try xendevicemodel_shutdown

[Ian Jackson]

xc_interface_open etc. is not going to work if we have dropped
privilege, but xendevicemodel_shutdown will if everything is new
enough.

Signed-off-by: Ian Jackson 
---
 hw/i386/xen/xen-hvm.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
index 83420cd..639425a 100644
--- a/hw/i386/xen/xen-hvm.c
+++ b/hw/i386/xen/xen-hvm.c
@@ -1386,9 +1386,18 @@ void destroy_hvm_domain(bool reboot)
 {
 xc_interface *xc_handle;
 int sts;
+int rc;
 
 unsigned int reason = reboot ? SHUTDOWN_reboot : SHUTDOWN_poweroff;
 
+if (xen_dmod) {
+rc = xendevicemodel_shutdown(xen_dmod, xen_domid, reason);
+if (!rc)
+return;
+perror("xendevicemodel_shutdown failed");
+/* well, try the old thing then */
+}
+
 xc_handle = xc_interface_open(0, 0, 0);
 if (xc_handle == NULL) {
 fprintf(stderr, "Cannot acquire xenctrl handle\n");
-- 
2.1.4

[Qemu-devel] [PATCH RFC 1/6] xen: link against xentoolcore

[Ian Jackson]

Signed-off-by: Ian Jackson 
---
 configure | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index fd7e3a5..c59a0c0 100755
--- a/configure
+++ b/configure
@@ -2072,14 +2072,14 @@ if test "$xen" != "no" ; then
   $($pkg_config --modversion xencontrol | sed 's/\./ /g') )"
 xen=yes
 xen_pc="xencontrol xenstore xenguest xenforeignmemory xengnttab"
-xen_pc="$xen_pc xenevtchn xendevicemodel"
+xen_pc="$xen_pc xenevtchn xendevicemodel xentoolcore"
 QEMU_CFLAGS="$QEMU_CFLAGS $($pkg_config --cflags $xen_pc)"
 libs_softmmu="$($pkg_config --libs $xen_pc) $libs_softmmu"
 LDFLAGS="$($pkg_config --libs $xen_pc) $LDFLAGS"
   else
 
 xen_libs="-lxenstore -lxenctrl -lxenguest"
-xen_stable_libs="-lxenforeignmemory -lxengnttab -lxenevtchn"
+xen_stable_libs="-lxenforeignmemory -lxengnttab -lxenevtchn -lxentoolcore"
 
 # First we test whether Xen headers and libraries are available.
 # If no, we are done and there is no Xen support.
-- 
2.1.4

[Qemu-devel] [PATCH RFC 2/6] xen: defer call to xen_restrict until running

[Ian Jackson]

Signed-off-by: Ian Jackson 
---
 hw/i386/xen/xen-hvm.c |  8 
 hw/xen/xen-common.c   | 10 ++
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
index d9ccd5d..7b60ec6 100644
--- a/hw/i386/xen/xen-hvm.c
+++ b/hw/i386/xen/xen-hvm.c
@@ -1254,14 +1254,6 @@ void xen_hvm_init(PCMachineState *pcms, MemoryRegion 
**ram_memory)
 goto err;
 }
 
-if (xen_domid_restrict) {
-rc = xen_restrict(xen_domid);
-if (rc < 0) {
-error_report("failed to restrict: error %d", errno);
-goto err;
-}
-}
-
 xen_create_ioreq_server(xen_domid, >ioservid);
 
 state->exit.notify = xen_exit_notifier;
diff --git a/hw/xen/xen-common.c b/hw/xen/xen-common.c
index 632a938..dfee53e 100644
--- a/hw/xen/xen-common.c
+++ b/hw/xen/xen-common.c
@@ -111,9 +111,19 @@ static void xenstore_record_dm_state(struct xs_handle *xs, 
const char *state)
 static void xen_change_state_handler(void *opaque, int running,
  RunState state)
 {
+int rc;
+
 if (running) {
 /* record state running */
 xenstore_record_dm_state(xenstore, "running");
+
+if (xen_domid_restrict) {
+rc = xen_restrict(xen_domid);
+if (rc < 0) {
+perror("xen: failed to restrict");
+exit(1);
+}
+}
 }
 }
 
-- 
2.1.4

[Qemu-devel] [PATCH RFC 4/6] xen: destroy_hvm_domain: Move reason into a variable

[Ian Jackson]

We are going to want to reuse this.

No functional change.

Signed-off-by: Ian Jackson 
---
 hw/i386/xen/xen-hvm.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
index 7b60ec6..83420cd 100644
--- a/hw/i386/xen/xen-hvm.c
+++ b/hw/i386/xen/xen-hvm.c
@@ -1387,12 +1387,13 @@ void destroy_hvm_domain(bool reboot)
 xc_interface *xc_handle;
 int sts;
 
+unsigned int reason = reboot ? SHUTDOWN_reboot : SHUTDOWN_poweroff;
+
 xc_handle = xc_interface_open(0, 0, 0);
 if (xc_handle == NULL) {
 fprintf(stderr, "Cannot acquire xenctrl handle\n");
 } else {
-sts = xc_domain_shutdown(xc_handle, xen_domid,
- reboot ? SHUTDOWN_reboot : SHUTDOWN_poweroff);
+sts = xc_domain_shutdown(xc_handle, xen_domid, reason);
 if (sts != 0) {
 fprintf(stderr, "xc_domain_shutdown failed to issue %s, "
 "sts %d, %s\n", reboot ? "reboot" : "poweroff",
-- 
2.1.4

[Qemu-devel] [PATCH RFC 0/6] xen: xen-domid-restrict improvements

[Ian Jackson]

I have been working on trying to get qemu, when running as a Xen
device model, to _actually_ not have power equivalent to root.

I think I have achieved this, with some limitations (which will be
discussed in my series against xen.git, which I am about to post).

However, there are changes to qemu needed.  In particular

 * The -xen-domid-restrict option does not work properly right now.
   It only restricts a small subset of the descriptors qemu has open.
   I am introducing a new library call in the Xen libraries for this,
   xentoolcore_restrict_all.

 * We need to call a different function on domain shutdown.

 * Additionally, in the future, we intend to be able to set aside
   a uid range for these qemus to run in, and that involves being
   able to tell qemu to drop privilege by numeric uid and gid.

This series is only an RFC because right now it won't compile against
older versions of Xen.  There is "configure" work needed.  I would
appreciate some help and/or advice and have CC'd some people who
touched this area recently...

Thanks for your attention.

Ian.

Re: [Qemu-devel] [PULL 00/18] target-arm queue

[Peter Maydell]

On 14 September 2017 at 18:52, Peter Maydell  wrote:
> ARM queue: nothing particularly exciting, but 18 patches
> is enough to send out.
>
> thanks
> -- PMM
>
> The following changes since commit 3dabde1128b671f36ac6cb36b97b273139964420:
>
>   Merge remote-tracking branch 'remotes/dgilbert/tags/pull-hmp-20170914' into 
> staging (2017-09-14 16:33:02 +0100)
>
> are available in the git repository at:
>
>   git://git.linaro.org/people/pmaydell/qemu-arm.git 
> tags/pull-target-arm-20170914
>
> for you to fetch changes up to ce3bc112cdb1d462e2d52eaa17a7314e7f3af504:
>
>   mps2-an511: Fix wiring of UART overflow interrupt lines (2017-09-14 
> 18:43:19 +0100)
>
> 
> target-arm queue:
>  * v7M: various code cleanups
>  * v7M: set correct BFSR bits on bus fault
>  * v7M: clear exclusive monitor on reset and exception entry/exit
>  * v7M: don't apply priority mask to negative priorities
>  * zcu102: support 'secure' and 'virtualization' machine properties
>  * aarch64: fix ERET single stepping
>  * gpex: implement PCI INTx routing
>  * mps2-an511: fix UART overflow interrupt line wiring
>

Applied, thanks.

-- PMM

Re: [Qemu-devel] [PATCH 2/6] block: Add reopen_queue to bdrv_child_perm()

[Eric Blake]

On 09/15/2017 05:10 AM, Kevin Wolf wrote:
> In the context of bdrv_reopen(), we'll have to look at the state of the
> graph as it will be after the reopen. This interface addition is in
> preparation for the change.
> 
> Signed-off-by: Kevin Wolf 
> ---
>  include/block/block_int.h |  7 +++
>  block.c   | 19 ---
>  block/commit.c|  1 +
>  block/mirror.c|  1 +
>  block/replication.c   |  1 +
>  block/vvfat.c |  1 +
>  6 files changed, 23 insertions(+), 7 deletions(-)
> 
Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 1/6] qemu-io: Reset qemuio_blk permissions before each command

[Eric Blake]

On 09/15/2017 05:10 AM, Kevin Wolf wrote:
> qemu-io provides a 'reopen' command that allows switching from writable
> to read-only access. We need to make sure that we don't try to keep
> write permissions to a BlockBackend that becomes read-only, otherwise
> things are going to fail.
> 
> command() already makes sure to request any additional permissions that
> a qemu-io command requires, so just resetting the permissions to values
> that are safe for read-only images is enough to fix this.
> 
> As a side effect, this makes the output of qemu-iotests case 187 more
> consistent.
> 
> Signed-off-by: Kevin Wolf 
> ---
>  qemu-io.c  | 13 +
>  tests/qemu-iotests/187.out |  2 +-
>  2 files changed, 14 insertions(+), 1 deletion(-)
> 

Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PULL 1/1] trace: Immediately apply per-vCPU state changes if a vCPU is being created

[Stefan Hajnoczi]

From: Lluís Vilanova 

Right now, function trace_event_set_vcpu_state_dynamic() asynchronously enables
events in the case a vCPU is executing TCG code. If the vCPU is being created
this makes some events like "guest_cpu_enter" to not be traced.

Signed-off-by: Lluís Vilanova 
Reviewed-by: Emilio G. Cota 
Message-id: 150525662577.19850.13767570977540117247.st...@frigg.lan
Signed-off-by: Stefan Hajnoczi 
---
 trace/control-target.c | 18 +++---
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/trace/control-target.c b/trace/control-target.c
index 4e36101997..706b2cee9d 100644
--- a/trace/control-target.c
+++ b/trace/control-target.c
@@ -88,13 +88,17 @@ void trace_event_set_vcpu_state_dynamic(CPUState *vcpu,
 clear_bit(vcpu_id, vcpu->trace_dstate_delayed);
 (*ev->dstate)--;
 }
-/*
- * Delay changes until next TB; we want all TBs to be built from a
- * single set of dstate values to ensure consistency of generated
- * tracing code.
- */
-async_run_on_cpu(vcpu, trace_event_synchronize_vcpu_state_dynamic,
- RUN_ON_CPU_NULL);
+if (vcpu->created) {
+/*
+ * Delay changes until next TB; we want all TBs to be built from a
+ * single set of dstate values to ensure consistency of generated
+ * tracing code.
+ */
+async_run_on_cpu(vcpu, trace_event_synchronize_vcpu_state_dynamic,
+ RUN_ON_CPU_NULL);
+} else {
+trace_event_synchronize_vcpu_state_dynamic(vcpu, RUN_ON_CPU_NULL);
+}
 }
 }
 
-- 
2.13.5

[Qemu-devel] [PULL 0/1] Tracing patches

[Stefan Hajnoczi]

The following changes since commit 3dabde1128b671f36ac6cb36b97b273139964420:

  Merge remote-tracking branch 'remotes/dgilbert/tags/pull-hmp-20170914' into 
staging (2017-09-14 16:33:02 +0100)

are available in the git repository at:

  git://github.com/stefanha/qemu.git tags/tracing-pull-request

for you to fetch changes up to f99b38fc7c1bdf2dcc5e316478281c35b8c60f79:

  trace: Immediately apply per-vCPU state changes if a vCPU is being created 
(2017-09-15 14:25:22 +0100)





Lluís Vilanova (1):
  trace: Immediately apply per-vCPU state changes if a vCPU is being
created

 trace/control-target.c | 18 +++---
 1 file changed, 11 insertions(+), 7 deletions(-)

-- 
2.13.5

Re: [Qemu-devel] [RFC v2 11/32] vhub: Open userfaultfd

[Dr. David Alan Gilbert]

* Peter Xu (pet...@redhat.com) wrote:
> On Thu, Aug 24, 2017 at 08:27:09PM +0100, Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert" 
> > 
> > Open a userfaultfd (on a postcopy_advise) and send it back in
> > the reply to the qemu for it to monitor.
> > 
> > Signed-off-by: Dr. David Alan Gilbert 
> > ---
> >  contrib/libvhost-user/libvhost-user.c | 26 +++---
> >  contrib/libvhost-user/libvhost-user.h |  3 +++
> >  2 files changed, 26 insertions(+), 3 deletions(-)
> > 
> > diff --git a/contrib/libvhost-user/libvhost-user.c 
> > b/contrib/libvhost-user/libvhost-user.c
> > index 47884c0a15..f9b5b12b28 100644
> > --- a/contrib/libvhost-user/libvhost-user.c
> > +++ b/contrib/libvhost-user/libvhost-user.c
> > @@ -15,6 +15,7 @@
> >  
> >  #include 
> >  #include 
> > +#include 
> >  #include 
> >  
> >  #include "qemu/atomic.h"
> > @@ -773,11 +774,30 @@ vu_set_vring_enable_exec(VuDev *dev, VhostUserMsg 
> > *vmsg)
> >  static bool
> >  vu_set_postcopy_advise(VuDev *dev, VhostUserMsg *vmsg)
> >  {
> > -/* TODO: Open ufd, pass it back in the request
> > - * TODO: Add addresses 
> > - */
> > +struct uffdio_api api_struct;
> > +
> > +dev->postcopy_ufd = syscall(__NR_userfaultfd, O_CLOEXEC | O_NONBLOCK);
> > +/* TODO: Add addresses */
> >  vmsg->payload.u64 = 0xcafe;
> >  vmsg->size = sizeof(vmsg->payload.u64);
> > +
> > +if (dev->postcopy_ufd == -1) {
> > +vu_panic(dev, "Userfaultfd not available: %s", strerror(errno));
> > +goto out;
> 
> We got error but still goto out?  I feel like we should reply with
> some kind of error code when any error happens.

See that the out: code returns the dev->postcopy_ufd to qemu
and in this case it's -1 so it is already flagged as an error.

> > +}
> > +api_struct.api = UFFD_API;
> > +api_struct.features = 0;
> > +if (ioctl(dev->postcopy_ufd, UFFDIO_API, _struct)) {
> > +vu_panic(dev, "Failed UFFDIO_API: %s", strerror(errno));
> > +close(dev->postcopy_ufd);
> > +dev->postcopy_ufd = -1;
> > +goto out;
> 
> Same here.

And there we explicitly set dev->postcopy_ufd = -1 before going to out
so it's sent as the error value.

> > +}
> > +/* TODO: Stash feature flags somewhere */
> > +out:
> > +/* Return a ufd to the QEMU */
> > +vmsg->fd_num = 1;
> > +vmsg->fds[0] = dev->postcopy_ufd;

^^^ see that's where the -1's end up.
> >  return true; /* = send a reply */
> >  }

Dave

> >  
> > diff --git a/contrib/libvhost-user/libvhost-user.h 
> > b/contrib/libvhost-user/libvhost-user.h
> > index 3987ce643d..3e8efdd919 100644
> > --- a/contrib/libvhost-user/libvhost-user.h
> > +++ b/contrib/libvhost-user/libvhost-user.h
> > @@ -234,6 +234,9 @@ struct VuDev {
> >   * re-initialize */
> >  vu_panic_cb panic;
> >  const VuDevIface *iface;
> > +
> > +/* Postcopy data */
> > +int postcopy_ufd;
> >  };
> >  
> >  typedef struct VuVirtqElement {
> > -- 
> > 2.13.5
> > 
> 
> -- 
> Peter Xu
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] [virtio-dev] Re: [RFC 0/8] virtio-crypto: add multiplexing mode support

[Halil Pasic]

On 09/14/2017 02:58 AM, Longpeng (Mike) wrote:
> 
> 
> On 2017/9/14 2:14, Halil Pasic wrote:
> 
>>
>>
>> On 09/11/2017 03:10 AM, Longpeng(Mike) wrote:
>>> *NOTE*
>>> The code realization is based on the latest virtio crypto spec:
>>>  [PATCH v19 0/2] virtio-crypto: virtio crypto device specification
>>>https://lists.nongnu.org/archive/html/qemu-devel/2017-08/msg05217.html
>>>
>>> In session mode, the process of create/close a session
>>> makes we have a least one full round-trip cost from guest to host to guest
>>> to be able to send any data for symmetric algorithms. It gets ourself into
>>> synchronization troubles in some scenarios like a web server handling lots
>>> of small requests whose algorithms and keys are different.
>>>
>>> We can support one-blob request (no sessions) as well for symmetric
>>> algorithms, including HASH, MAC services. The benefit is obvious for
>>> HASH service because it's usually a one-blob operation.
>>>
>>
>> Hi!
>>
>> I've just started looking at this. Patch #1 modifies linux/virtio_crypto.h
>> which if I compare with the (almost) latest linux master is different. Thus
>> I would expect a corresponding kernel patch set too, but I haven't received
>> one, nor did I find a reference in the cover letter.
>>
>> I think if I want to test the new features I need the kernel counter-part
>> too, or?
>>
>> Could you point me to the kernel counterpart?
>>
> 
> 
> Hi Halil,
> 
> We haven't implemented the kernel frontend part yet, but there's a testcase
> based on qtest, you can use it.
> 
> Please see the attachment.
> 

Thanks Longpeng! I have two problems with this: first I can't use this on s390x
and as you may have noticed I'm working mostly on s390x (that's what I'm payed
for). OK, my laptop is amd64 so I was able to try it out, and that leads to the
next problem. I can't test before/after and cross version stuff with this. That
hurts me because I have a feeling things can be done simpler but that feeling 
has
failed me before, so I tend to try out first and then start a discussion.

Is some kernel patch series already in the pipeline? 

Regards,
Halil

Re: [Qemu-devel] [PATCH 3/3] iotests: Add "quit during block migration" case 195

[Eric Blake]

On 09/15/2017 12:44 AM, Fam Zheng wrote:
> Signed-off-by: Fam Zheng 
> ---
>  tests/qemu-iotests/195 | 97 
> ++
>  tests/qemu-iotests/195.out | 19 +
>  tests/qemu-iotests/group   |  1 +
>  3 files changed, 117 insertions(+)
>  create mode 100755 tests/qemu-iotests/195
>  create mode 100644 tests/qemu-iotests/195.out

> +_cleanup()
> +{
> +rm -f "${MIG_SOCKET}"
> +rm -f "${TEST_IMG}.dest"
> +_cleanup_test_img
> +_cleanup_qemu
> +}
> +trap "_cleanup; exit \$status" 0 1 2 3 15

Semantic conflict with Jeff's patch series that adds './check -s' that
preserves files in the per-test temp directory.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH] linux-user/syscall.c: Handle SH4's exceptional alignment for p{read, write}64

[Richard Henderson]

On 09/15/2017 08:41 AM, Philippe Mathieu-Daudé wrote:
> On 09/15/2017 03:58 AM, James Clarke wrote:
>> Fixes: https://bugs.launchpad.net/qemu/+bug/1716767
>> Signed-off-by: James Clarke 
> 
> Congratulations! You have won yourself a R: entry (Designated reviewer) in the
> "Linux user" and "SH4" sections of MAINTAINERS!
> 
>> ---
>>   linux-user/syscall.c | 12 
>>   1 file changed, 12 insertions(+)
>>
>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>> index 9b6364a266..24d6a81c21 100644
>> --- a/linux-user/syscall.c
>> +++ b/linux-user/syscall.c
>> @@ -10495,20 +10495,32 @@ abi_long do_syscall(void *cpu_env, int num,
>> abi_long arg1,
>>   #endif
>>   #ifdef TARGET_NR_pread64
>>   case TARGET_NR_pread64:
>> +#if defined(TARGET_SH4)
>> +    /* SH4 doesn't align register pairs, except for p{read,write}64 */
>> +    arg4 = arg5;
>> +    arg5 = arg6;
>> +#else
>>   if (regpairs_aligned(cpu_env)) {
>>   arg4 = arg5;
>>   arg5 = arg6;
>>   }
>> +#endif
> 
> I'd rather use arch_type from "sysemu/arch_init.h":
> 
>   case TARGET_NR_pwrite64:
>   /* SH4 doesn't align register pairs, except for p{read,write}64 */
>   if (arch_type == QEMU_ARCH_SH4 || regpairs_aligned(cpu_env)) {
>   arg4 = arg5;
>   arg5 = arg6;
>   }
> 
> What do you think?

I'd rather change the interface of regpairs_aligned to take the syscall number,
and add an instance for SH4 above.


r~

Re: [Qemu-devel] [PATCH] virtio-serial: add enable_backend callback

[Paolo Bonzini]

On 07/07/2017 16:21, Pavel Butsykin wrote:
> We should guarantee that RAM will not be modified while VM has a stopped
> state, otherwise it can lead to negative consequences during post-copy
> migration. In RUN_STATE_FINISH_MIGRATE step, it's expected that RAM on
> source side will not be modified as this could lead to non-consistent vm state
> on the destination side. Also RAM access during postcopy-ram migration with
> enabled release-ram capability can lead to sad consequences.
> 
> Let's add enable_backend() callback to avoid undesirable virtioqueue changes
> in the guest memory.
> 
> Signed-off-by: Pavel Butsykin 

To understand the patch better this doesn't fix _all_ stopped states,
only migration, right?  That said it's a valid bugfix even independent
of the effects for stopped runstate.

Thanks,

Paolo

> ---
>  hw/char/virtio-console.c  | 21 +
>  hw/char/virtio-serial-bus.c   |  7 +++
>  include/hw/virtio/virtio-serial.h |  3 +++
>  3 files changed, 31 insertions(+)
> 
> diff --git a/hw/char/virtio-console.c b/hw/char/virtio-console.c
> index 0cb1668c8a..b55905892e 100644
> --- a/hw/char/virtio-console.c
> +++ b/hw/char/virtio-console.c
> @@ -163,6 +163,26 @@ static void chr_event(void *opaque, int event)
>  }
>  }
>  
> +static void virtconsole_enable_backend(VirtIOSerialPort *port, bool enable)
> +{
> +VirtConsole *vcon = VIRTIO_CONSOLE(port);
> +
> +if (!qemu_chr_fe_get_driver(>chr)) {
> +return;
> +}
> +
> +if (enable) {
> +VirtIOSerialPortClass *k = VIRTIO_SERIAL_PORT_GET_CLASS(port);
> +
> +qemu_chr_fe_set_handlers(>chr, chr_can_read, chr_read,
> + k->is_console ? NULL : chr_event,
> + vcon, NULL, false);
> +} else {
> +qemu_chr_fe_set_handlers(>chr, NULL, NULL,
> + NULL, NULL, NULL, false);
> +}
> +}
> +
>  static void virtconsole_realize(DeviceState *dev, Error **errp)
>  {
>  VirtIOSerialPort *port = VIRTIO_SERIAL_PORT(dev);
> @@ -233,6 +253,7 @@ static void virtserialport_class_init(ObjectClass *klass, 
> void *data)
>  k->unrealize = virtconsole_unrealize;
>  k->have_data = flush_buf;
>  k->set_guest_connected = set_guest_connected;
> +k->enable_backend = virtconsole_enable_backend;
>  k->guest_writable = guest_writable;
>  dc->props = virtserialport_properties;
>  }
> diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
> index f5bc173844..f0f18c8e7c 100644
> --- a/hw/char/virtio-serial-bus.c
> +++ b/hw/char/virtio-serial-bus.c
> @@ -637,6 +637,13 @@ static void set_status(VirtIODevice *vdev, uint8_t 
> status)
>  if (!(status & VIRTIO_CONFIG_S_DRIVER_OK)) {
>  guest_reset(vser);
>  }
> +
> +QTAILQ_FOREACH(port, >ports, next) {
> +VirtIOSerialPortClass *vsc = VIRTIO_SERIAL_PORT_GET_CLASS(port);
> +if (vsc->enable_backend) {
> +vsc->enable_backend(port, vdev->vm_running);
> +}
> +}
>  }
>  
>  static void vser_reset(VirtIODevice *vdev)
> diff --git a/include/hw/virtio/virtio-serial.h 
> b/include/hw/virtio/virtio-serial.h
> index b19c44727f..12657a9f39 100644
> --- a/include/hw/virtio/virtio-serial.h
> +++ b/include/hw/virtio/virtio-serial.h
> @@ -58,6 +58,9 @@ typedef struct VirtIOSerialPortClass {
>  /* Guest opened/closed device. */
>  void (*set_guest_connected)(VirtIOSerialPort *port, int guest_connected);
>  
> +/* Enable/disable backend for virtio serial port */
> +void (*enable_backend)(VirtIOSerialPort *port, bool enable);
> +
>  /* Guest is now ready to accept data (virtqueues set up). */
>  void (*guest_ready)(VirtIOSerialPort *port);
>  
>