[Qemu-devel] [PATCH v3 06/31] tcg/arm: Use tcg_out_mov_reg in tcg_out_mov

[Richard Henderson]

We have a function that takes an additional condition parameter
over the standard backend interface.  It already takes care of
eliding no-op moves.

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

diff --git a/tcg/arm/tcg-target.inc.c b/tcg/arm/tcg-target.inc.c
index abf0c444b4..130b6bef1e 100644
--- a/tcg/arm/tcg-target.inc.c
+++ b/tcg/arm/tcg-target.inc.c
@@ -2267,7 +2267,7 @@ static inline bool tcg_out_sti(TCGContext *s, TCGType 
type, TCGArg val,
 static inline void tcg_out_mov(TCGContext *s, TCGType type,
TCGReg ret, TCGReg arg)
 {
-tcg_out_dat_reg(s, COND_AL, ARITH_MOV, ret, 0, arg, SHIFT_IMM_LSL(0));
+tcg_out_mov_reg(s, COND_AL, ret, arg);
 }
 
 static inline void tcg_out_movi(TCGContext *s, TCGType type,
-- 
2.17.1

[Qemu-devel] [PATCH v3 07/31] tcg: Return bool success from tcg_out_mov

[Richard Henderson]

This patch merely changes the interface, aborting on all failures,
of which there are currently none.

Reviewed-by: Alex Bennée 
Reviewed-by: David Hildenbrand 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: David Gibson 
Signed-off-by: Richard Henderson 
---
 tcg/aarch64/tcg-target.inc.c |  5 +++--
 tcg/arm/tcg-target.inc.c |  3 ++-
 tcg/i386/tcg-target.inc.c|  5 +++--
 tcg/mips/tcg-target.inc.c|  3 ++-
 tcg/ppc/tcg-target.inc.c |  3 ++-
 tcg/riscv/tcg-target.inc.c   |  5 +++--
 tcg/s390/tcg-target.inc.c|  3 ++-
 tcg/sparc/tcg-target.inc.c   |  3 ++-
 tcg/tcg.c| 14 ++
 tcg/tci/tcg-target.inc.c |  3 ++-
 10 files changed, 31 insertions(+), 16 deletions(-)

diff --git a/tcg/aarch64/tcg-target.inc.c b/tcg/aarch64/tcg-target.inc.c
index eefa929948..ee89734318 100644
--- a/tcg/aarch64/tcg-target.inc.c
+++ b/tcg/aarch64/tcg-target.inc.c
@@ -938,10 +938,10 @@ static void tcg_out_ldst(TCGContext *s, AArch64Insn insn, 
TCGReg rd,
 tcg_out_ldst_r(s, insn, rd, rn, TCG_TYPE_I64, TCG_REG_TMP);
 }
 
-static void tcg_out_mov(TCGContext *s, TCGType type, TCGReg ret, TCGReg arg)
+static bool tcg_out_mov(TCGContext *s, TCGType type, TCGReg ret, TCGReg arg)
 {
 if (ret == arg) {
-return;
+return true;
 }
 switch (type) {
 case TCG_TYPE_I32:
@@ -970,6 +970,7 @@ static void tcg_out_mov(TCGContext *s, TCGType type, TCGReg 
ret, TCGReg arg)
 default:
 g_assert_not_reached();
 }
+return true;
 }
 
 static void tcg_out_ld(TCGContext *s, TCGType type, TCGReg ret,
diff --git a/tcg/arm/tcg-target.inc.c b/tcg/arm/tcg-target.inc.c
index 130b6bef1e..7316504c9d 100644
--- a/tcg/arm/tcg-target.inc.c
+++ b/tcg/arm/tcg-target.inc.c
@@ -2264,10 +2264,11 @@ static inline bool tcg_out_sti(TCGContext *s, TCGType 
type, TCGArg val,
 return false;
 }
 
-static inline void tcg_out_mov(TCGContext *s, TCGType type,
+static inline bool tcg_out_mov(TCGContext *s, TCGType type,
TCGReg ret, TCGReg arg)
 {
 tcg_out_mov_reg(s, COND_AL, ret, arg);
+return true;
 }
 
 static inline void tcg_out_movi(TCGContext *s, TCGType type,
diff --git a/tcg/i386/tcg-target.inc.c b/tcg/i386/tcg-target.inc.c
index d5ed9f1ffd..1198c76392 100644
--- a/tcg/i386/tcg-target.inc.c
+++ b/tcg/i386/tcg-target.inc.c
@@ -809,12 +809,12 @@ static inline void tgen_arithr(TCGContext *s, int subop, 
int dest, int src)
 tcg_out_modrm(s, OPC_ARITH_GvEv + (subop << 3) + ext, dest, src);
 }
 
-static void tcg_out_mov(TCGContext *s, TCGType type, TCGReg ret, TCGReg arg)
+static bool tcg_out_mov(TCGContext *s, TCGType type, TCGReg ret, TCGReg arg)
 {
 int rexw = 0;
 
 if (arg == ret) {
-return;
+return true;
 }
 switch (type) {
 case TCG_TYPE_I64:
@@ -852,6 +852,7 @@ static void tcg_out_mov(TCGContext *s, TCGType type, TCGReg 
ret, TCGReg arg)
 default:
 g_assert_not_reached();
 }
+return true;
 }
 
 static void tcg_out_dup_vec(TCGContext *s, TCGType type, unsigned vece,
diff --git a/tcg/mips/tcg-target.inc.c b/tcg/mips/tcg-target.inc.c
index 412cacdcb9..7cafd4a790 100644
--- a/tcg/mips/tcg-target.inc.c
+++ b/tcg/mips/tcg-target.inc.c
@@ -558,13 +558,14 @@ static inline void tcg_out_dsra(TCGContext *s, TCGReg rd, 
TCGReg rt, TCGArg sa)
 tcg_out_opc_sa64(s, OPC_DSRA, OPC_DSRA32, rd, rt, sa);
 }
 
-static inline void tcg_out_mov(TCGContext *s, TCGType type,
+static inline bool tcg_out_mov(TCGContext *s, TCGType type,
TCGReg ret, TCGReg arg)
 {
 /* Simple reg-reg move, optimising out the 'do nothing' case */
 if (ret != arg) {
 tcg_out_opc_reg(s, OPC_OR, ret, arg, TCG_REG_ZERO);
 }
+return true;
 }
 
 static void tcg_out_movi(TCGContext *s, TCGType type,
diff --git a/tcg/ppc/tcg-target.inc.c b/tcg/ppc/tcg-target.inc.c
index 36b4791707..30c095d3d5 100644
--- a/tcg/ppc/tcg-target.inc.c
+++ b/tcg/ppc/tcg-target.inc.c
@@ -559,12 +559,13 @@ static bool patch_reloc(tcg_insn_unit *code_ptr, int type,
 static void tcg_out_mem_long(TCGContext *s, int opi, int opx, TCGReg rt,
  TCGReg base, tcg_target_long offset);
 
-static void tcg_out_mov(TCGContext *s, TCGType type, TCGReg ret, TCGReg arg)
+static bool tcg_out_mov(TCGContext *s, TCGType type, TCGReg ret, TCGReg arg)
 {
 tcg_debug_assert(TCG_TARGET_REG_BITS == 64 || type == TCG_TYPE_I32);
 if (ret != arg) {
 tcg_out32(s, OR | SAB(arg, ret, arg));
 }
+return true;
 }
 
 static inline void tcg_out_rld(TCGContext *s, int op, TCGReg ra, TCGReg rs,
diff --git a/tcg/riscv/tcg-target.inc.c b/tcg/riscv/tcg-target.inc.c
index 2932505094..6497a4dab2 100644
--- a/tcg/riscv/tcg-target.inc.c
+++ b/tcg/riscv/tcg-target.inc.c
@@ -515,10 +515,10 @@ static bool patch_reloc(tcg_insn_unit *code_ptr, int type,
  * TCG intrinsics
  */
 
-static void tcg_out_mov(TCGContext *s, TCGType type, TCGReg ret, TCGReg arg)
+static bool 

[Qemu-devel] [PATCH v3 15/31] tcg: Add gvec expanders for variable shift

[Richard Henderson]

The gvec expanders perform a modulo on the shift count.  If the target
requires alternate behaviour, then it cannot use the generic gvec
expanders anyway, and will have to have its own custom code.

Reviewed-by: Alex Bennée 
Signed-off-by: Richard Henderson 
---
 accel/tcg/tcg-runtime.h  |  15 +++
 tcg/tcg-op-gvec.h|  11 ++
 tcg/tcg-op.h |   4 +
 accel/tcg/tcg-runtime-gvec.c | 144 ++
 tcg/tcg-op-gvec.c| 195 +++
 tcg/tcg-op-vec.c |  15 +++
 6 files changed, 384 insertions(+)

diff --git a/accel/tcg/tcg-runtime.h b/accel/tcg/tcg-runtime.h
index dfe325625c..ed3ce5fd91 100644
--- a/accel/tcg/tcg-runtime.h
+++ b/accel/tcg/tcg-runtime.h
@@ -254,6 +254,21 @@ DEF_HELPER_FLAGS_3(gvec_sar16i, TCG_CALL_NO_RWG, void, 
ptr, ptr, i32)
 DEF_HELPER_FLAGS_3(gvec_sar32i, TCG_CALL_NO_RWG, void, ptr, ptr, i32)
 DEF_HELPER_FLAGS_3(gvec_sar64i, TCG_CALL_NO_RWG, void, ptr, ptr, i32)
 
+DEF_HELPER_FLAGS_4(gvec_shl8v, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
+DEF_HELPER_FLAGS_4(gvec_shl16v, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
+DEF_HELPER_FLAGS_4(gvec_shl32v, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
+DEF_HELPER_FLAGS_4(gvec_shl64v, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
+
+DEF_HELPER_FLAGS_4(gvec_shr8v, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
+DEF_HELPER_FLAGS_4(gvec_shr16v, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
+DEF_HELPER_FLAGS_4(gvec_shr32v, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
+DEF_HELPER_FLAGS_4(gvec_shr64v, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
+
+DEF_HELPER_FLAGS_4(gvec_sar8v, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
+DEF_HELPER_FLAGS_4(gvec_sar16v, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
+DEF_HELPER_FLAGS_4(gvec_sar32v, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
+DEF_HELPER_FLAGS_4(gvec_sar64v, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
+
 DEF_HELPER_FLAGS_4(gvec_eq8, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
 DEF_HELPER_FLAGS_4(gvec_eq16, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
 DEF_HELPER_FLAGS_4(gvec_eq32, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
diff --git a/tcg/tcg-op-gvec.h b/tcg/tcg-op-gvec.h
index ac744ff7c9..84a6247b16 100644
--- a/tcg/tcg-op-gvec.h
+++ b/tcg/tcg-op-gvec.h
@@ -318,6 +318,17 @@ void tcg_gen_gvec_shri(unsigned vece, uint32_t dofs, 
uint32_t aofs,
 void tcg_gen_gvec_sari(unsigned vece, uint32_t dofs, uint32_t aofs,
int64_t shift, uint32_t oprsz, uint32_t maxsz);
 
+/*
+ * Perform vector shift by vector element, modulo the element size.
+ * E.g.  D[i] = A[i] << (B[i] % (8 << vece)).
+ */
+void tcg_gen_gvec_shlv(unsigned vece, uint32_t dofs, uint32_t aofs,
+   uint32_t bofs, uint32_t oprsz, uint32_t maxsz);
+void tcg_gen_gvec_shrv(unsigned vece, uint32_t dofs, uint32_t aofs,
+   uint32_t bofs, uint32_t oprsz, uint32_t maxsz);
+void tcg_gen_gvec_sarv(unsigned vece, uint32_t dofs, uint32_t aofs,
+   uint32_t bofs, uint32_t oprsz, uint32_t maxsz);
+
 void tcg_gen_gvec_cmp(TCGCond cond, unsigned vece, uint32_t dofs,
   uint32_t aofs, uint32_t bofs,
   uint32_t oprsz, uint32_t maxsz);
diff --git a/tcg/tcg-op.h b/tcg/tcg-op.h
index 9fff9864f6..833c6330b5 100644
--- a/tcg/tcg-op.h
+++ b/tcg/tcg-op.h
@@ -986,6 +986,10 @@ void tcg_gen_shli_vec(unsigned vece, TCGv_vec r, TCGv_vec 
a, int64_t i);
 void tcg_gen_shri_vec(unsigned vece, TCGv_vec r, TCGv_vec a, int64_t i);
 void tcg_gen_sari_vec(unsigned vece, TCGv_vec r, TCGv_vec a, int64_t i);
 
+void tcg_gen_shlv_vec(unsigned vece, TCGv_vec r, TCGv_vec a, TCGv_vec s);
+void tcg_gen_shrv_vec(unsigned vece, TCGv_vec r, TCGv_vec a, TCGv_vec s);
+void tcg_gen_sarv_vec(unsigned vece, TCGv_vec r, TCGv_vec a, TCGv_vec s);
+
 void tcg_gen_cmp_vec(TCGCond cond, unsigned vece, TCGv_vec r,
  TCGv_vec a, TCGv_vec b);
 
diff --git a/accel/tcg/tcg-runtime-gvec.c b/accel/tcg/tcg-runtime-gvec.c
index e2c6f24262..2152fb6903 100644
--- a/accel/tcg/tcg-runtime-gvec.c
+++ b/accel/tcg/tcg-runtime-gvec.c
@@ -725,6 +725,150 @@ void HELPER(gvec_sar64i)(void *d, void *a, uint32_t desc)
 clear_high(d, oprsz, desc);
 }
 
+void HELPER(gvec_shl8v)(void *d, void *a, void *b, uint32_t desc)
+{
+intptr_t oprsz = simd_oprsz(desc);
+intptr_t i;
+
+for (i = 0; i < oprsz; i += sizeof(uint8_t)) {
+uint8_t sh = *(uint8_t *)(b + i) & 7;
+*(uint8_t *)(d + i) = *(uint8_t *)(a + i) << sh;
+}
+clear_high(d, oprsz, desc);
+}
+
+void HELPER(gvec_shl16v)(void *d, void *a, void *b, uint32_t desc)
+{
+intptr_t oprsz = simd_oprsz(desc);
+intptr_t i;
+
+for (i = 0; i < oprsz; i += sizeof(uint16_t)) {
+uint8_t sh = *(uint16_t *)(b + i) & 15;
+*(uint16_t *)(d + i) = *(uint16_t *)(a + i) << sh;
+}
+clear_high(d, oprsz, desc);
+}
+
+void HELPER(gvec_shl32v)(void *d, void *a, void *b, uint32_t desc)
+{
+intptr_t oprsz = simd_oprsz(desc);
+

[Qemu-devel] [PATCH v3 09/31] tcg: Promote tcg_out_{dup, dupi}_vec to backend interface

[Richard Henderson]

The i386 backend already has these functions, and the aarch64 backend
could easily split out one.  Nothing is done with these functions yet,
but this will aid register allocation of INDEX_op_dup_vec in a later patch.

Adjust the aarch64 tcg_out_dupi_vec signature to match the new interface.

Reviewed-by: Alex Bennée 
Signed-off-by: Richard Henderson 
---
 tcg/aarch64/tcg-target.inc.c | 12 ++--
 tcg/i386/tcg-target.inc.c|  3 ++-
 tcg/tcg.c| 14 ++
 3 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/tcg/aarch64/tcg-target.inc.c b/tcg/aarch64/tcg-target.inc.c
index ee89734318..e443b5df23 100644
--- a/tcg/aarch64/tcg-target.inc.c
+++ b/tcg/aarch64/tcg-target.inc.c
@@ -799,7 +799,7 @@ static void tcg_out_logicali(TCGContext *s, AArch64Insn 
insn, TCGType ext,
 }
 
 static void tcg_out_dupi_vec(TCGContext *s, TCGType type,
- TCGReg rd, uint64_t v64)
+ TCGReg rd, tcg_target_long v64)
 {
 int op, cmode, imm8;
 
@@ -814,6 +814,14 @@ static void tcg_out_dupi_vec(TCGContext *s, TCGType type,
 }
 }
 
+static bool tcg_out_dup_vec(TCGContext *s, TCGType type, unsigned vece,
+TCGReg rd, TCGReg rs)
+{
+int is_q = type - TCG_TYPE_V64;
+tcg_out_insn(s, 3605, DUP, is_q, rd, rs, 1 << vece, 0);
+return true;
+}
+
 static void tcg_out_movi(TCGContext *s, TCGType type, TCGReg rd,
  tcg_target_long value)
 {
@@ -2201,7 +2209,7 @@ static void tcg_out_vec_op(TCGContext *s, TCGOpcode opc,
 tcg_out_insn(s, 3617, NOT, is_q, 0, a0, a1);
 break;
 case INDEX_op_dup_vec:
-tcg_out_insn(s, 3605, DUP, is_q, a0, a1, 1 << vece, 0);
+tcg_out_dup_vec(s, type, vece, a0, a1);
 break;
 case INDEX_op_shli_vec:
 tcg_out_insn(s, 3614, SHL, is_q, a0, a1, a2 + (8 << vece));
diff --git a/tcg/i386/tcg-target.inc.c b/tcg/i386/tcg-target.inc.c
index 1198c76392..0d621670c7 100644
--- a/tcg/i386/tcg-target.inc.c
+++ b/tcg/i386/tcg-target.inc.c
@@ -855,7 +855,7 @@ static bool tcg_out_mov(TCGContext *s, TCGType type, TCGReg 
ret, TCGReg arg)
 return true;
 }
 
-static void tcg_out_dup_vec(TCGContext *s, TCGType type, unsigned vece,
+static bool tcg_out_dup_vec(TCGContext *s, TCGType type, unsigned vece,
 TCGReg r, TCGReg a)
 {
 if (have_avx2) {
@@ -888,6 +888,7 @@ static void tcg_out_dup_vec(TCGContext *s, TCGType type, 
unsigned vece,
 g_assert_not_reached();
 }
 }
+return true;
 }
 
 static void tcg_out_dupi_vec(TCGContext *s, TCGType type,
diff --git a/tcg/tcg.c b/tcg/tcg.c
index 68d86361e2..3ef4d3478d 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -109,10 +109,24 @@ static void tcg_out_movi(TCGContext *s, TCGType type,
 static void tcg_out_op(TCGContext *s, TCGOpcode opc, const TCGArg *args,
const int *const_args);
 #if TCG_TARGET_MAYBE_vec
+static bool tcg_out_dup_vec(TCGContext *s, TCGType type, unsigned vece,
+TCGReg dst, TCGReg src);
+static void tcg_out_dupi_vec(TCGContext *s, TCGType type,
+ TCGReg dst, tcg_target_long arg);
 static void tcg_out_vec_op(TCGContext *s, TCGOpcode opc, unsigned vecl,
unsigned vece, const TCGArg *args,
const int *const_args);
 #else
+static inline bool tcg_out_dup_vec(TCGContext *s, TCGType type, unsigned vece,
+   TCGReg dst, TCGReg src)
+{
+g_assert_not_reached();
+}
+static inline void tcg_out_dupi_vec(TCGContext *s, TCGType type,
+TCGReg dst, tcg_target_long arg)
+{
+g_assert_not_reached();
+}
 static inline void tcg_out_vec_op(TCGContext *s, TCGOpcode opc, unsigned vecl,
   unsigned vece, const TCGArg *args,
   const int *const_args)
-- 
2.17.1

[Qemu-devel] [PATCH v3 05/31] tcg: Assert fixed_reg is read-only

[Richard Henderson]

The only fixed_reg is cpu_env, and it should not be modified
during any TB.  Therefore code that tries to special-case moves
into a fixed_reg is dead.  Remove it.

Reviewed-by: Alex Bennée 
Reviewed-by: David Hildenbrand 
Signed-off-by: Richard Henderson 
---
 tcg/tcg.c | 87 +--
 1 file changed, 40 insertions(+), 47 deletions(-)

diff --git a/tcg/tcg.c b/tcg/tcg.c
index f7bef51de8..70ca113c26 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -3274,11 +3274,8 @@ static void tcg_reg_alloc_do_movi(TCGContext *s, TCGTemp 
*ots,
   tcg_target_ulong val, TCGLifeData arg_life,
   TCGRegSet preferred_regs)
 {
-if (ots->fixed_reg) {
-/* For fixed registers, we do not do any constant propagation.  */
-tcg_out_movi(s, ots->type, ots->reg, val);
-return;
-}
+/* ENV should not be modified.  */
+tcg_debug_assert(!ots->fixed_reg);
 
 /* The movi is not explicitly generated here.  */
 if (ots->val_type == TEMP_VAL_REG) {
@@ -3314,6 +3311,9 @@ static void tcg_reg_alloc_mov(TCGContext *s, const TCGOp 
*op)
 ots = arg_temp(op->args[0]);
 ts = arg_temp(op->args[1]);
 
+/* ENV should not be modified.  */
+tcg_debug_assert(!ots->fixed_reg);
+
 /* Note that otype != itype for no-op truncation.  */
 otype = ots->type;
 itype = ts->type;
@@ -3338,7 +3338,7 @@ static void tcg_reg_alloc_mov(TCGContext *s, const TCGOp 
*op)
 }
 
 tcg_debug_assert(ts->val_type == TEMP_VAL_REG);
-if (IS_DEAD_ARG(0) && !ots->fixed_reg) {
+if (IS_DEAD_ARG(0)) {
 /* mov to a non-saved dead register makes no sense (even with
liveness analysis disabled). */
 tcg_debug_assert(NEED_SYNC_ARG(0));
@@ -3351,7 +3351,7 @@ static void tcg_reg_alloc_mov(TCGContext *s, const TCGOp 
*op)
 }
 temp_dead(s, ots);
 } else {
-if (IS_DEAD_ARG(1) && !ts->fixed_reg && !ots->fixed_reg) {
+if (IS_DEAD_ARG(1) && !ts->fixed_reg) {
 /* the mov can be suppressed */
 if (ots->val_type == TEMP_VAL_REG) {
 s->reg_to_temp[ots->reg] = NULL;
@@ -3504,6 +3504,10 @@ static void tcg_reg_alloc_op(TCGContext *s, const TCGOp 
*op)
 arg = op->args[i];
 arg_ct = >args_ct[i];
 ts = arg_temp(arg);
+
+/* ENV should not be modified.  */
+tcg_debug_assert(!ts->fixed_reg);
+
 if ((arg_ct->ct & TCG_CT_ALIAS)
 && !const_args[arg_ct->alias_index]) {
 reg = new_args[arg_ct->alias_index];
@@ -3512,29 +3516,21 @@ static void tcg_reg_alloc_op(TCGContext *s, const TCGOp 
*op)
 i_allocated_regs | o_allocated_regs,
 op->output_pref[k], ts->indirect_base);
 } else {
-/* if fixed register, we try to use it */
-reg = ts->reg;
-if (ts->fixed_reg &&
-tcg_regset_test_reg(arg_ct->u.regs, reg)) {
-goto oarg_end;
-}
 reg = tcg_reg_alloc(s, arg_ct->u.regs, o_allocated_regs,
 op->output_pref[k], ts->indirect_base);
 }
 tcg_regset_set_reg(o_allocated_regs, reg);
-/* if a fixed register is used, then a move will be done 
afterwards */
-if (!ts->fixed_reg) {
-if (ts->val_type == TEMP_VAL_REG) {
-s->reg_to_temp[ts->reg] = NULL;
-}
-ts->val_type = TEMP_VAL_REG;
-ts->reg = reg;
-/* temp value is modified, so the value kept in memory is
-   potentially not the same */
-ts->mem_coherent = 0;
-s->reg_to_temp[reg] = ts;
+if (ts->val_type == TEMP_VAL_REG) {
+s->reg_to_temp[ts->reg] = NULL;
 }
-oarg_end:
+ts->val_type = TEMP_VAL_REG;
+ts->reg = reg;
+/*
+ * Temp value is modified, so the value kept in memory is
+ * potentially not the same.
+ */
+ts->mem_coherent = 0;
+s->reg_to_temp[reg] = ts;
 new_args[i] = reg;
 }
 }
@@ -3550,10 +3546,10 @@ static void tcg_reg_alloc_op(TCGContext *s, const TCGOp 
*op)
 /* move the outputs in the correct register if needed */
 for(i = 0; i < nb_oargs; i++) {
 ts = arg_temp(op->args[i]);
-reg = new_args[i];
-if (ts->fixed_reg && ts->reg != reg) {
-tcg_out_mov(s, ts->type, ts->reg, reg);
-}
+
+/* ENV should not be modified.  */
+tcg_debug_assert(!ts->fixed_reg);
+
 if (NEED_SYNC_ARG(i)) {
 temp_sync(s, ts, o_allocated_regs, 0, IS_DEAD_ARG(i));
 } else if (IS_DEAD_ARG(i)) {
@@ -3674,26 +3670,23 @@ static void 

[Qemu-devel] [PATCH v3 14/31] tcg: Add INDEX_op_dupm_vec

[Richard Henderson]

Allow the backend to expand dup from memory directly, instead of
forcing the value into a temp first.  This is especially important
if integer/vector register moves do not exist.

Note that officially tcg_out_dupm_vec is allowed to fail.
If it did, we could fix this up relatively easily:

  VECE == 32/64:
Load the value into a vector register, then dup.
Both of these must work.

  VECE == 8/16:
If the value happens to be at an offset such that an aligned
load would place the desired value in the least significant
end of the register, go ahead and load w/garbage in high bits.

Load the value w/INDEX_op_ld{8,16}_i32.
Attempt a move directly to vector reg, which may fail.
Store the value into the backing store for OTS.
Load the value into the vector reg w/TCG_TYPE_I32, which must work.
Duplicate from the vector reg into itself, which must work.

All of which is well and good, except that all supported
hosts can support dupm for all vece, so all of the failure
paths would be dead code and untestable.

Reviewed-by: Alex Bennée 
Signed-off-by: Richard Henderson 
---
 tcg/tcg-op.h |  1 +
 tcg/tcg-opc.h|  1 +
 tcg/aarch64/tcg-target.inc.c |  4 ++
 tcg/i386/tcg-target.inc.c|  4 ++
 tcg/tcg-op-gvec.c| 89 +++-
 tcg/tcg-op-vec.c | 11 +
 tcg/tcg.c|  1 +
 7 files changed, 70 insertions(+), 41 deletions(-)

diff --git a/tcg/tcg-op.h b/tcg/tcg-op.h
index 1f1824c30a..9fff9864f6 100644
--- a/tcg/tcg-op.h
+++ b/tcg/tcg-op.h
@@ -954,6 +954,7 @@ void tcg_gen_atomic_umax_fetch_i64(TCGv_i64, TCGv, 
TCGv_i64, TCGArg, TCGMemOp);
 void tcg_gen_mov_vec(TCGv_vec, TCGv_vec);
 void tcg_gen_dup_i32_vec(unsigned vece, TCGv_vec, TCGv_i32);
 void tcg_gen_dup_i64_vec(unsigned vece, TCGv_vec, TCGv_i64);
+void tcg_gen_dup_mem_vec(unsigned vece, TCGv_vec, TCGv_ptr, tcg_target_long);
 void tcg_gen_dup8i_vec(TCGv_vec, uint32_t);
 void tcg_gen_dup16i_vec(TCGv_vec, uint32_t);
 void tcg_gen_dup32i_vec(TCGv_vec, uint32_t);
diff --git a/tcg/tcg-opc.h b/tcg/tcg-opc.h
index 1bad6e4208..4bf71f261f 100644
--- a/tcg/tcg-opc.h
+++ b/tcg/tcg-opc.h
@@ -219,6 +219,7 @@ DEF(dup2_vec, 1, 2, 0, IMPLVEC | IMPL(TCG_TARGET_REG_BITS 
== 32))
 
 DEF(ld_vec, 1, 1, 1, IMPLVEC)
 DEF(st_vec, 0, 2, 1, IMPLVEC)
+DEF(dupm_vec, 1, 1, 1, IMPLVEC)
 
 DEF(add_vec, 1, 2, 0, IMPLVEC)
 DEF(sub_vec, 1, 2, 0, IMPLVEC)
diff --git a/tcg/aarch64/tcg-target.inc.c b/tcg/aarch64/tcg-target.inc.c
index e8cf4e4044..15ab35adf7 100644
--- a/tcg/aarch64/tcg-target.inc.c
+++ b/tcg/aarch64/tcg-target.inc.c
@@ -2190,6 +2190,9 @@ static void tcg_out_vec_op(TCGContext *s, TCGOpcode opc,
 case INDEX_op_st_vec:
 tcg_out_st(s, type, a0, a1, a2);
 break;
+case INDEX_op_dupm_vec:
+tcg_out_dupm_vec(s, type, vece, a0, a1, a2);
+break;
 case INDEX_op_add_vec:
 tcg_out_insn(s, 3616, ADD, is_q, vece, a0, a1, a2);
 break;
@@ -2522,6 +2525,7 @@ static const TCGTargetOpDef *tcg_target_op_def(TCGOpcode 
op)
 return _w;
 case INDEX_op_ld_vec:
 case INDEX_op_st_vec:
+case INDEX_op_dupm_vec:
 return _r;
 case INDEX_op_dup_vec:
 return _wr;
diff --git a/tcg/i386/tcg-target.inc.c b/tcg/i386/tcg-target.inc.c
index f4bd00e24f..5b33bbd99b 100644
--- a/tcg/i386/tcg-target.inc.c
+++ b/tcg/i386/tcg-target.inc.c
@@ -2829,6 +2829,9 @@ static void tcg_out_vec_op(TCGContext *s, TCGOpcode opc,
 case INDEX_op_st_vec:
 tcg_out_st(s, type, a0, a1, a2);
 break;
+case INDEX_op_dupm_vec:
+tcg_out_dupm_vec(s, type, vece, a0, a1, a2);
+break;
 
 case INDEX_op_x86_shufps_vec:
 insn = OPC_SHUFPS;
@@ -3115,6 +3118,7 @@ static const TCGTargetOpDef *tcg_target_op_def(TCGOpcode 
op)
 
 case INDEX_op_ld_vec:
 case INDEX_op_st_vec:
+case INDEX_op_dupm_vec:
 return _r;
 
 case INDEX_op_add_vec:
diff --git a/tcg/tcg-op-gvec.c b/tcg/tcg-op-gvec.c
index 3fcb2352d9..35ebc5a201 100644
--- a/tcg/tcg-op-gvec.c
+++ b/tcg/tcg-op-gvec.c
@@ -395,6 +395,41 @@ static TCGType choose_vector_type(const TCGOpcode *list, 
unsigned vece,
 return 0;
 }
 
+static void do_dup_store(TCGType type, uint32_t dofs, uint32_t oprsz,
+ uint32_t maxsz, TCGv_vec t_vec)
+{
+uint32_t i = 0;
+
+switch (type) {
+case TCG_TYPE_V256:
+/*
+ * Recall that ARM SVE allows vector sizes that are not a
+ * power of 2, but always a multiple of 16.  The intent is
+ * that e.g. size == 80 would be expanded with 2x32 + 1x16.
+ */
+for (; i + 32 <= oprsz; i += 32) {
+tcg_gen_stl_vec(t_vec, cpu_env, dofs + i, TCG_TYPE_V256);
+}
+/* fallthru */
+case TCG_TYPE_V128:
+for (; i + 16 <= oprsz; i += 16) {
+tcg_gen_stl_vec(t_vec, cpu_env, dofs + i, TCG_TYPE_V128);
+}
+break;
+case TCG_TYPE_V64:
+for (; i < 

[Qemu-devel] [PATCH v3 02/31] tcg: Do not recreate INDEX_op_neg_vec unless supported

[Richard Henderson]

Use tcg_can_emit_vec_op instead of just TCG_TARGET_HAS_neg_vec,
so that we check the type and vece for the actual operation.

Reviewed-by: Alex Bennée 
Signed-off-by: Richard Henderson 
---
 tcg/optimize.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/tcg/optimize.c b/tcg/optimize.c
index 5150c38a25..24faa06260 100644
--- a/tcg/optimize.c
+++ b/tcg/optimize.c
@@ -734,9 +734,13 @@ void tcg_optimize(TCGContext *s)
 } else if (opc == INDEX_op_sub_i64) {
 neg_op = INDEX_op_neg_i64;
 have_neg = TCG_TARGET_HAS_neg_i64;
-} else {
+} else if (TCG_TARGET_HAS_neg_vec) {
+TCGType type = TCGOP_VECL(op) + TCG_TYPE_V64;
+unsigned vece = TCGOP_VECE(op);
 neg_op = INDEX_op_neg_vec;
-have_neg = TCG_TARGET_HAS_neg_vec;
+have_neg = tcg_can_emit_vec_op(neg_op, type, vece) > 0;
+} else {
+break;
 }
 if (!have_neg) {
 break;
-- 
2.17.1

[Qemu-devel] [PATCH v3 08/31] tcg: Support cross-class moves without instruction support

[Richard Henderson]

PowerPC Altivec does not support direct moves between vector registers
and general registers.  So when tcg_out_mov fails, we can use the
backing memory for the temporary to perform the move.

Acked-by: David Hildenbrand 
Signed-off-by: Richard Henderson 
---
 tcg/tcg.c | 31 ---
 1 file changed, 28 insertions(+), 3 deletions(-)

diff --git a/tcg/tcg.c b/tcg/tcg.c
index 8ed7cb8654..68d86361e2 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -3368,7 +3368,20 @@ static void tcg_reg_alloc_mov(TCGContext *s, const TCGOp 
*op)
  ots->indirect_base);
 }
 if (!tcg_out_mov(s, otype, ots->reg, ts->reg)) {
-abort();
+/*
+ * Cross register class move not supported.
+ * Store the source register into the destination slot
+ * and leave the destination temp as TEMP_VAL_MEM.
+ */
+assert(!ots->fixed_reg);
+if (!ts->mem_allocated) {
+temp_allocate_frame(s, ots);
+}
+tcg_out_st(s, ts->type, ts->reg,
+   ots->mem_base->reg, ots->mem_offset);
+ots->mem_coherent = 1;
+temp_free_or_dead(s, ots, -1);
+return;
 }
 }
 ots->val_type = TEMP_VAL_REG;
@@ -3470,7 +3483,13 @@ static void tcg_reg_alloc_op(TCGContext *s, const TCGOp 
*op)
 reg = tcg_reg_alloc(s, arg_ct->u.regs, i_allocated_regs,
 o_preferred_regs, ts->indirect_base);
 if (!tcg_out_mov(s, ts->type, reg, ts->reg)) {
-abort();
+/*
+ * Cross register class move not supported.  Sync the
+ * temp back to its slot and load from there.
+ */
+temp_sync(s, ts, i_allocated_regs, 0, 0);
+tcg_out_ld(s, ts->type, reg,
+   ts->mem_base->reg, ts->mem_offset);
 }
 }
 new_args[i] = reg;
@@ -3631,7 +3650,13 @@ static void tcg_reg_alloc_call(TCGContext *s, TCGOp *op)
 if (ts->reg != reg) {
 tcg_reg_free(s, reg, allocated_regs);
 if (!tcg_out_mov(s, ts->type, reg, ts->reg)) {
-abort();
+/*
+ * Cross register class move not supported.  Sync the
+ * temp back to its slot and load from there.
+ */
+temp_sync(s, ts, allocated_regs, 0, 0);
+tcg_out_ld(s, ts->type, reg,
+   ts->mem_base->reg, ts->mem_offset);
 }
 }
 } else {
-- 
2.17.1

[Qemu-devel] [PATCH v3 10/31] tcg: Manually expand INDEX_op_dup_vec

[Richard Henderson]

This case is similar to INDEX_op_mov_* in that we need to do
different things depending on the current location of the source.

Signed-off-by: Richard Henderson 
---
v3: Added some commentary to the tcg_reg_alloc_* functions.
---
 tcg/aarch64/tcg-target.inc.c |   9 ++-
 tcg/i386/tcg-target.inc.c|   8 +--
 tcg/tcg.c| 111 +++
 3 files changed, 118 insertions(+), 10 deletions(-)

diff --git a/tcg/aarch64/tcg-target.inc.c b/tcg/aarch64/tcg-target.inc.c
index e443b5df23..3cefdd1e43 100644
--- a/tcg/aarch64/tcg-target.inc.c
+++ b/tcg/aarch64/tcg-target.inc.c
@@ -2108,10 +2108,8 @@ static void tcg_out_op(TCGContext *s, TCGOpcode opc,
 
 case INDEX_op_mov_i32:  /* Always emitted via tcg_out_mov.  */
 case INDEX_op_mov_i64:
-case INDEX_op_mov_vec:
 case INDEX_op_movi_i32: /* Always emitted via tcg_out_movi.  */
 case INDEX_op_movi_i64:
-case INDEX_op_dupi_vec:
 case INDEX_op_call: /* Always emitted via tcg_out_call.  */
 default:
 g_assert_not_reached();
@@ -2208,9 +2206,6 @@ static void tcg_out_vec_op(TCGContext *s, TCGOpcode opc,
 case INDEX_op_not_vec:
 tcg_out_insn(s, 3617, NOT, is_q, 0, a0, a1);
 break;
-case INDEX_op_dup_vec:
-tcg_out_dup_vec(s, type, vece, a0, a1);
-break;
 case INDEX_op_shli_vec:
 tcg_out_insn(s, 3614, SHL, is_q, a0, a1, a2 + (8 << vece));
 break;
@@ -2254,6 +2249,10 @@ static void tcg_out_vec_op(TCGContext *s, TCGOpcode opc,
 }
 }
 break;
+
+case INDEX_op_mov_vec:  /* Always emitted via tcg_out_mov.  */
+case INDEX_op_dupi_vec: /* Always emitted via tcg_out_movi.  */
+case INDEX_op_dup_vec:  /* Always emitted via tcg_out_dup_vec.  */
 default:
 g_assert_not_reached();
 }
diff --git a/tcg/i386/tcg-target.inc.c b/tcg/i386/tcg-target.inc.c
index 0d621670c7..3c8229d413 100644
--- a/tcg/i386/tcg-target.inc.c
+++ b/tcg/i386/tcg-target.inc.c
@@ -2603,10 +2603,8 @@ static inline void tcg_out_op(TCGContext *s, TCGOpcode 
opc,
 break;
 case INDEX_op_mov_i32:  /* Always emitted via tcg_out_mov.  */
 case INDEX_op_mov_i64:
-case INDEX_op_mov_vec:
 case INDEX_op_movi_i32: /* Always emitted via tcg_out_movi.  */
 case INDEX_op_movi_i64:
-case INDEX_op_dupi_vec:
 case INDEX_op_call: /* Always emitted via tcg_out_call.  */
 default:
 tcg_abort();
@@ -2795,9 +2793,6 @@ static void tcg_out_vec_op(TCGContext *s, TCGOpcode opc,
 case INDEX_op_st_vec:
 tcg_out_st(s, type, a0, a1, a2);
 break;
-case INDEX_op_dup_vec:
-tcg_out_dup_vec(s, type, vece, a0, a1);
-break;
 
 case INDEX_op_x86_shufps_vec:
 insn = OPC_SHUFPS;
@@ -2839,6 +2834,9 @@ static void tcg_out_vec_op(TCGContext *s, TCGOpcode opc,
 tcg_out8(s, a2);
 break;
 
+case INDEX_op_mov_vec:  /* Always emitted via tcg_out_mov.  */
+case INDEX_op_dupi_vec: /* Always emitted via tcg_out_movi.  */
+case INDEX_op_dup_vec:  /* Always emitted via tcg_out_dup_vec.  */
 default:
 g_assert_not_reached();
 }
diff --git a/tcg/tcg.c b/tcg/tcg.c
index 3ef4d3478d..2b715bf099 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -3284,6 +3284,9 @@ static void tcg_reg_alloc_bb_end(TCGContext *s, TCGRegSet 
allocated_regs)
 save_globals(s, allocated_regs);
 }
 
+/*
+ * Specialized code generation for INDEX_op_movi_*.
+ */
 static void tcg_reg_alloc_do_movi(TCGContext *s, TCGTemp *ots,
   tcg_target_ulong val, TCGLifeData arg_life,
   TCGRegSet preferred_regs)
@@ -3313,6 +3316,9 @@ static void tcg_reg_alloc_movi(TCGContext *s, const TCGOp 
*op)
 tcg_reg_alloc_do_movi(s, ots, val, op->life, op->output_pref[0]);
 }
 
+/*
+ * Specialized code generation for INDEX_op_mov_*.
+ */
 static void tcg_reg_alloc_mov(TCGContext *s, const TCGOp *op)
 {
 const TCGLifeData arg_life = op->life;
@@ -3407,6 +3413,108 @@ static void tcg_reg_alloc_mov(TCGContext *s, const 
TCGOp *op)
 }
 }
 
+/*
+ * Specialized code generation for INDEX_op_dup_vec.
+ */
+static void tcg_reg_alloc_dup(TCGContext *s, const TCGOp *op)
+{
+const TCGLifeData arg_life = op->life;
+TCGRegSet dup_out_regs, dup_in_regs;
+TCGTemp *its, *ots;
+TCGType itype, vtype;
+unsigned vece;
+bool ok;
+
+ots = arg_temp(op->args[0]);
+its = arg_temp(op->args[1]);
+
+/* ENV should not be modified.  */
+tcg_debug_assert(!ots->fixed_reg);
+
+itype = its->type;
+vece = TCGOP_VECE(op);
+vtype = TCGOP_VECL(op) + TCG_TYPE_V64;
+
+if (its->val_type == TEMP_VAL_CONST) {
+/* Propagate constant via movi -> dupi.  */
+tcg_target_ulong val = its->val;
+if (IS_DEAD_ARG(1)) {
+temp_dead(s, its);
+}
+tcg_reg_alloc_do_movi(s, ots, val, arg_life, op->output_pref[0]);
+return;
+}
+
+dup_out_regs = 

[Qemu-devel] [PATCH v3 01/31] tcg: Implement tcg_gen_gvec_3i()

[Richard Henderson]

From: David Hildenbrand 

Let's add tcg_gen_gvec_3i(), similar to tcg_gen_gvec_2i(), however
without introducing "gen_helper_gvec_3i *fnoi", as it isn't needed
for now.

Reviewed-by: Alex Bennée 
Signed-off-by: David Hildenbrand 
Message-Id: <20190416185301.25344-2-da...@redhat.com>
Signed-off-by: Richard Henderson 
---
 tcg/tcg-op-gvec.h |  24 
 tcg/tcg-op-gvec.c | 139 ++
 2 files changed, 163 insertions(+)

diff --git a/tcg/tcg-op-gvec.h b/tcg/tcg-op-gvec.h
index 850da32ded..c093243c4c 100644
--- a/tcg/tcg-op-gvec.h
+++ b/tcg/tcg-op-gvec.h
@@ -164,6 +164,27 @@ typedef struct {
 bool load_dest;
 } GVecGen3;
 
+typedef struct {
+/*
+ * Expand inline as a 64-bit or 32-bit integer. Only one of these will be
+ * non-NULL.
+ */
+void (*fni8)(TCGv_i64, TCGv_i64, TCGv_i64, int64_t);
+void (*fni4)(TCGv_i32, TCGv_i32, TCGv_i32, int32_t);
+/* Expand inline with a host vector type.  */
+void (*fniv)(unsigned, TCGv_vec, TCGv_vec, TCGv_vec, int64_t);
+/* Expand out-of-line helper w/descriptor, data in descriptor.  */
+gen_helper_gvec_3 *fno;
+/* The opcode, if any, to which this corresponds.  */
+TCGOpcode opc;
+/* The vector element size, if applicable.  */
+uint8_t vece;
+/* Prefer i64 to v64.  */
+bool prefer_i64;
+/* Load dest as a 3rd source operand.  */
+bool load_dest;
+} GVecGen3i;
+
 typedef struct {
 /* Expand inline as a 64-bit or 32-bit integer.
Only one of these will be non-NULL.  */
@@ -193,6 +214,9 @@ void tcg_gen_gvec_2s(uint32_t dofs, uint32_t aofs, uint32_t 
oprsz,
  uint32_t maxsz, TCGv_i64 c, const GVecGen2s *);
 void tcg_gen_gvec_3(uint32_t dofs, uint32_t aofs, uint32_t bofs,
 uint32_t oprsz, uint32_t maxsz, const GVecGen3 *);
+void tcg_gen_gvec_3i(uint32_t dofs, uint32_t aofs, uint32_t bofs,
+ uint32_t oprsz, uint32_t maxsz, int64_t c,
+ const GVecGen3i *);
 void tcg_gen_gvec_4(uint32_t dofs, uint32_t aofs, uint32_t bofs, uint32_t cofs,
 uint32_t oprsz, uint32_t maxsz, const GVecGen4 *);
 
diff --git a/tcg/tcg-op-gvec.c b/tcg/tcg-op-gvec.c
index 0996ef0812..f831adb4e7 100644
--- a/tcg/tcg-op-gvec.c
+++ b/tcg/tcg-op-gvec.c
@@ -663,6 +663,29 @@ static void expand_3_i32(uint32_t dofs, uint32_t aofs,
 tcg_temp_free_i32(t0);
 }
 
+static void expand_3i_i32(uint32_t dofs, uint32_t aofs, uint32_t bofs,
+  uint32_t oprsz, int32_t c, bool load_dest,
+  void (*fni)(TCGv_i32, TCGv_i32, TCGv_i32, int32_t))
+{
+TCGv_i32 t0 = tcg_temp_new_i32();
+TCGv_i32 t1 = tcg_temp_new_i32();
+TCGv_i32 t2 = tcg_temp_new_i32();
+uint32_t i;
+
+for (i = 0; i < oprsz; i += 4) {
+tcg_gen_ld_i32(t0, cpu_env, aofs + i);
+tcg_gen_ld_i32(t1, cpu_env, bofs + i);
+if (load_dest) {
+tcg_gen_ld_i32(t2, cpu_env, dofs + i);
+}
+fni(t2, t0, t1, c);
+tcg_gen_st_i32(t2, cpu_env, dofs + i);
+}
+tcg_temp_free_i32(t0);
+tcg_temp_free_i32(t1);
+tcg_temp_free_i32(t2);
+}
+
 /* Expand OPSZ bytes worth of three-operand operations using i32 elements.  */
 static void expand_4_i32(uint32_t dofs, uint32_t aofs, uint32_t bofs,
  uint32_t cofs, uint32_t oprsz, bool write_aofs,
@@ -770,6 +793,29 @@ static void expand_3_i64(uint32_t dofs, uint32_t aofs,
 tcg_temp_free_i64(t0);
 }
 
+static void expand_3i_i64(uint32_t dofs, uint32_t aofs, uint32_t bofs,
+  uint32_t oprsz, int64_t c, bool load_dest,
+  void (*fni)(TCGv_i64, TCGv_i64, TCGv_i64, int64_t))
+{
+TCGv_i64 t0 = tcg_temp_new_i64();
+TCGv_i64 t1 = tcg_temp_new_i64();
+TCGv_i64 t2 = tcg_temp_new_i64();
+uint32_t i;
+
+for (i = 0; i < oprsz; i += 8) {
+tcg_gen_ld_i64(t0, cpu_env, aofs + i);
+tcg_gen_ld_i64(t1, cpu_env, bofs + i);
+if (load_dest) {
+tcg_gen_ld_i64(t2, cpu_env, dofs + i);
+}
+fni(t2, t0, t1, c);
+tcg_gen_st_i64(t2, cpu_env, dofs + i);
+}
+tcg_temp_free_i64(t0);
+tcg_temp_free_i64(t1);
+tcg_temp_free_i64(t2);
+}
+
 /* Expand OPSZ bytes worth of three-operand operations using i64 elements.  */
 static void expand_4_i64(uint32_t dofs, uint32_t aofs, uint32_t bofs,
  uint32_t cofs, uint32_t oprsz, bool write_aofs,
@@ -883,6 +929,35 @@ static void expand_3_vec(unsigned vece, uint32_t dofs, 
uint32_t aofs,
 tcg_temp_free_vec(t0);
 }
 
+/*
+ * Expand OPSZ bytes worth of three-vector operands and an immediate operand
+ * using host vectors.
+ */
+static void expand_3i_vec(unsigned vece, uint32_t dofs, uint32_t aofs,
+  uint32_t bofs, uint32_t oprsz, uint32_t tysz,
+  TCGType type, int64_t c, bool load_dest,
+  void (*fni)(unsigned, TCGv_vec, 

[Qemu-devel] [PATCH v3 03/31] tcg: Allow add_vec, sub_vec, neg_vec, not_vec to be expanded

[Richard Henderson]

Reviewed-by: Alex Bennée 
Reviewed-by: David Hildenbrand 
Signed-off-by: Richard Henderson 
---
 tcg/tcg-op-vec.c | 49 
 1 file changed, 33 insertions(+), 16 deletions(-)

diff --git a/tcg/tcg-op-vec.c b/tcg/tcg-op-vec.c
index 27f65600c3..cfb18682b1 100644
--- a/tcg/tcg-op-vec.c
+++ b/tcg/tcg-op-vec.c
@@ -226,16 +226,6 @@ void tcg_gen_stl_vec(TCGv_vec r, TCGv_ptr b, TCGArg o, 
TCGType low_type)
 vec_gen_3(INDEX_op_st_vec, low_type, 0, ri, bi, o);
 }
 
-void tcg_gen_add_vec(unsigned vece, TCGv_vec r, TCGv_vec a, TCGv_vec b)
-{
-vec_gen_op3(INDEX_op_add_vec, vece, r, a, b);
-}
-
-void tcg_gen_sub_vec(unsigned vece, TCGv_vec r, TCGv_vec a, TCGv_vec b)
-{
-vec_gen_op3(INDEX_op_sub_vec, vece, r, a, b);
-}
-
 void tcg_gen_and_vec(unsigned vece, TCGv_vec r, TCGv_vec a, TCGv_vec b)
 {
 vec_gen_op3(INDEX_op_and_vec, 0, r, a, b);
@@ -296,11 +286,30 @@ void tcg_gen_eqv_vec(unsigned vece, TCGv_vec r, TCGv_vec 
a, TCGv_vec b)
 tcg_gen_not_vec(0, r, r);
 }
 
+static bool do_op2(unsigned vece, TCGv_vec r, TCGv_vec a, TCGOpcode opc)
+{
+TCGTemp *rt = tcgv_vec_temp(r);
+TCGTemp *at = tcgv_vec_temp(a);
+TCGArg ri = temp_arg(rt);
+TCGArg ai = temp_arg(at);
+TCGType type = rt->base_type;
+int can;
+
+tcg_debug_assert(at->base_type >= type);
+can = tcg_can_emit_vec_op(opc, type, vece);
+if (can > 0) {
+vec_gen_2(opc, type, vece, ri, ai);
+} else if (can < 0) {
+tcg_expand_vec_op(opc, type, vece, ri, ai);
+} else {
+return false;
+}
+return true;
+}
+
 void tcg_gen_not_vec(unsigned vece, TCGv_vec r, TCGv_vec a)
 {
-if (TCG_TARGET_HAS_not_vec) {
-vec_gen_op2(INDEX_op_not_vec, 0, r, a);
-} else {
+if (!TCG_TARGET_HAS_not_vec || !do_op2(vece, r, a, INDEX_op_not_vec)) {
 TCGv_vec t = tcg_const_ones_vec_matching(r);
 tcg_gen_xor_vec(0, r, a, t);
 tcg_temp_free_vec(t);
@@ -309,9 +318,7 @@ void tcg_gen_not_vec(unsigned vece, TCGv_vec r, TCGv_vec a)
 
 void tcg_gen_neg_vec(unsigned vece, TCGv_vec r, TCGv_vec a)
 {
-if (TCG_TARGET_HAS_neg_vec) {
-vec_gen_op2(INDEX_op_neg_vec, vece, r, a);
-} else {
+if (!TCG_TARGET_HAS_neg_vec || !do_op2(vece, r, a, INDEX_op_neg_vec)) {
 TCGv_vec t = tcg_const_zeros_vec_matching(r);
 tcg_gen_sub_vec(vece, r, t, a);
 tcg_temp_free_vec(t);
@@ -409,6 +416,16 @@ static void do_op3(unsigned vece, TCGv_vec r, TCGv_vec a,
 }
 }
 
+void tcg_gen_add_vec(unsigned vece, TCGv_vec r, TCGv_vec a, TCGv_vec b)
+{
+do_op3(vece, r, a, b, INDEX_op_add_vec);
+}
+
+void tcg_gen_sub_vec(unsigned vece, TCGv_vec r, TCGv_vec a, TCGv_vec b)
+{
+do_op3(vece, r, a, b, INDEX_op_sub_vec);
+}
+
 void tcg_gen_mul_vec(unsigned vece, TCGv_vec r, TCGv_vec a, TCGv_vec b)
 {
 do_op3(vece, r, a, b, INDEX_op_mul_vec);
-- 
2.17.1

[Qemu-devel] [PATCH v3 00/31] tcg vector improvements

[Richard Henderson]

Changes since v2 (stsquad review):
  * Split out a tcg/arm/ change to tcg_out_mov.
  * Add some additional commentary for tcg_reg_alloc_foo.

Patches missing ack/review:

0006-tcg-arm-Use-tcg_out_mov_reg-in-tcg_out_mov.patch (new)
0010-tcg-Manually-expand-INDEX_op_dup_vec.patch
0011-tcg-Add-tcg_out_dupm_vec-to-the-backend-interface.patch
0012-tcg-i386-Implement-tcg_out_dupm_vec.patch
0013-tcg-aarch64-Implement-tcg_out_dupm_vec.patch
0016-tcg-i386-Support-vector-variable-shift-opcodes.patch
0018-tcg-Add-gvec-expanders-for-vector-shift-by-scalar.patch
0019-tcg-i386-Support-vector-scalar-shift-opcodes.patch
0022-tcg-i386-Support-vector-absolute-value.patch
0025-target-cris-Use-tcg_gen_abs_tl.patch
0027-target-ppc-Use-tcg_gen_abs_tl.patch
0031-tcg-aarch64-Do-not-advertise-minmax-for-MO_64.patch (new)


r~


David Hildenbrand (1):
  tcg: Implement tcg_gen_gvec_3i()

Philippe Mathieu-Daudé (2):
  target/ppc: Use tcg_gen_abs_i32
  target/tricore: Use tcg_gen_abs_tl

Richard Henderson (28):
  tcg: Do not recreate INDEX_op_neg_vec unless supported
  tcg: Allow add_vec, sub_vec, neg_vec, not_vec to be expanded
  tcg: Specify optional vector requirements with a list
  tcg: Assert fixed_reg is read-only
  tcg/arm: Use tcg_out_mov_reg in tcg_out_mov
  tcg: Return bool success from tcg_out_mov
  tcg: Support cross-class moves without instruction support
  tcg: Promote tcg_out_{dup,dupi}_vec to backend interface
  tcg: Manually expand INDEX_op_dup_vec
  tcg: Add tcg_out_dupm_vec to the backend interface
  tcg/i386: Implement tcg_out_dupm_vec
  tcg/aarch64: Implement tcg_out_dupm_vec
  tcg: Add INDEX_op_dupm_vec
  tcg: Add gvec expanders for variable shift
  tcg/i386: Support vector variable shift opcodes
  tcg/aarch64: Support vector variable shift opcodes
  tcg: Add gvec expanders for vector shift by scalar
  tcg/i386: Support vector scalar shift opcodes
  tcg: Add support for integer absolute value
  tcg: Add support for vector absolute value
  tcg/i386: Support vector absolute value
  tcg/aarch64: Support vector absolute value
  target/arm: Use tcg_gen_abs_i64 and tcg_gen_gvec_abs
  target/cris: Use tcg_gen_abs_tl
  target/ppc: Use tcg_gen_abs_tl
  target/s390x: Use tcg_gen_abs_i64
  target/xtensa: Use tcg_gen_abs_i32
  tcg/aarch64: Do not advertise minmax for MO_64

 accel/tcg/tcg-runtime.h |  20 +
 target/arm/helper.h |   2 -
 tcg/aarch64/tcg-target.h|   3 +-
 tcg/aarch64/tcg-target.opc.h|   2 +
 tcg/i386/tcg-target.h   |   5 +-
 tcg/tcg-op-gvec.h   |  64 +-
 tcg/tcg-op.h|  14 +
 tcg/tcg-opc.h   |   2 +
 tcg/tcg.h   |  21 +
 accel/tcg/tcg-runtime-gvec.c| 192 ++
 target/arm/neon_helper.c|   5 -
 target/arm/translate-a64.c  |  41 +-
 target/arm/translate-sve.c  |   9 +-
 target/arm/translate.c  | 144 +++--
 target/cris/translate.c |   9 +-
 target/ppc/translate.c  |  68 +-
 target/ppc/translate/spe-impl.inc.c |  14 +-
 target/ppc/translate/vmx-impl.inc.c |   7 +-
 target/s390x/translate.c|   8 +-
 target/tricore/translate.c  |  27 +-
 target/xtensa/translate.c   |   9 +-
 tcg/aarch64/tcg-target.inc.c| 120 +++-
 tcg/arm/tcg-target.inc.c|   5 +-
 tcg/i386/tcg-target.inc.c   | 163 -
 tcg/mips/tcg-target.inc.c   |   3 +-
 tcg/optimize.c  |   8 +-
 tcg/ppc/tcg-target.inc.c|   3 +-
 tcg/riscv/tcg-target.inc.c  |   5 +-
 tcg/s390/tcg-target.inc.c   |   3 +-
 tcg/sparc/tcg-target.inc.c  |   3 +-
 tcg/tcg-op-gvec.c   | 945 +++-
 tcg/tcg-op-vec.c| 270 +++-
 tcg/tcg-op.c|  20 +
 tcg/tcg.c   | 271 ++--
 tcg/tci/tcg-target.inc.c|   3 +-
 tcg/README  |   4 +
 36 files changed, 2019 insertions(+), 473 deletions(-)

-- 
2.17.1

[Qemu-devel] [PATCH] migration: don't set MIGRATION dirty range for ignored block

[Wei Yang]

The ignored blocks are not migrated and those ranges are not used.

Signed-off-by: Wei Yang 
---
 exec.c  | 4 +++-
 include/exec/ram_addr.h | 2 ++
 migration/ram.c | 2 +-
 3 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/exec.c b/exec.c
index 86a38d3b3b..97da155c12 100644
--- a/exec.c
+++ b/exec.c
@@ -2192,6 +2192,8 @@ static void ram_block_add(RAMBlock *new_block, Error 
**errp, bool shared)
 RAMBlock *last_block = NULL;
 ram_addr_t old_ram_size, new_ram_size;
 Error *err = NULL;
+uint8_t dirty_memory_clients = ramblock_is_ignored(new_block) ?
+ DIRTY_CLIENTS_NOMIG : DIRTY_CLIENTS_ALL;
 
 old_ram_size = last_ram_page();
 
@@ -2252,7 +2254,7 @@ static void ram_block_add(RAMBlock *new_block, Error 
**errp, bool shared)
 
 cpu_physical_memory_set_dirty_range(new_block->offset,
 new_block->used_length,
-DIRTY_CLIENTS_ALL);
+dirty_memory_clients);
 
 if (new_block->host) {
 qemu_ram_setup_dump(new_block->host, new_block->max_length);
diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
index a7c81bdb32..4765435fb8 100644
--- a/include/exec/ram_addr.h
+++ b/include/exec/ram_addr.h
@@ -72,6 +72,7 @@ static inline unsigned long int 
ramblock_recv_bitmap_offset(void *host_addr,
 }
 
 bool ramblock_is_pmem(RAMBlock *rb);
+bool ramblock_is_ignored(RAMBlock *rb);
 
 long qemu_getrampagesize(void);
 
@@ -117,6 +118,7 @@ int qemu_ram_resize(RAMBlock *block, ram_addr_t newsize, 
Error **errp);
 
 #define DIRTY_CLIENTS_ALL ((1 << DIRTY_MEMORY_NUM) - 1)
 #define DIRTY_CLIENTS_NOCODE  (DIRTY_CLIENTS_ALL & ~(1 << DIRTY_MEMORY_CODE))
+#define DIRTY_CLIENTS_NOMIG   (DIRTY_CLIENTS_ALL & ~(1 << 
DIRTY_MEMORY_MIGRATION))
 
 void tb_invalidate_phys_range(ram_addr_t start, ram_addr_t end);
 
diff --git a/migration/ram.c b/migration/ram.c
index 1def8122e9..44525e3816 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -159,7 +159,7 @@ out:
 return ret;
 }
 
-static bool ramblock_is_ignored(RAMBlock *block)
+bool ramblock_is_ignored(RAMBlock *block)
 {
 return !qemu_ram_is_migratable(block) ||
(migrate_ignore_shared() && qemu_ram_is_shared(block));
-- 
2.19.1

Re: [Qemu-devel] [PATCH v1 5/5] hw/arm: Add the Netduino Plus 2

[Alistair Francis]

On Fri, May 3, 2019 at 9:26 PM Alistair  wrote:
>
> On Thu, May 2, 2019, at 3:06 AM, Peter Maydell wrote:
> > On Tue, 30 Apr 2019 at 21:29, Alistair Francis  wrote:
> > >
> > > On Tue, Apr 30, 2019 at 9:02 AM Peter Maydell  
> > > wrote:
> > > > Can you explain the purpose of the reset code? None of the other
> > > > v7m boards seem to need to do a manual qemu_register_reset().
> > >
> > > The reset code allows the machine to work with the -kernel option.
> > > Without the reset override using -kernel results in the guest starting
> > > at the wrong address. We can use the -device loader option without the
> > > reset code though.
> >
> > That sounds in line with how -kernel works on the other armv7m
> > boards -- the expectation is that your image file includes a
> > full vector table and the CPU will read the PC and SP from it
> > when it resets. If you want "honour the entry point" you can
> > use -device loader, as you say.
> >
> > Ignoring the entry point for -kernel ELF files is certainly
> > a bit confusing, but I think if we want to change this we should
> > do it globally, rather than having one board which behaves
> > differently to the rest. Changing it does have some awkwardness:
>
> Hmm... That is a good point. It is confusing having something just for one 
> board. I'll drop this part and we can re-evaluate later.
>
> > * possibility of breaking previously working images
>
> I have no way to test the other boards, so this might be difficult to change.
>
> > * we can get the initial PC from the ELF entrypoint, but if
> >  we do this what do we do about the initial SP value ?
>
> Not sure about this one either. I'm guessing it changes between the different 
> M cores.

Ah, it seems like -device loader doesn't work, it looks like not
setting the thumb register causes this core dump:

qemu: fatal: Lockup: can't escalate 3 to HardFault (current priority -1)

R00=2000 R01=0574 R02=200015d0 R03=200015d0
R04= R05= R06= R07=
R08= R09= R10= R11=
R12= R13=ffe0 R14=fff9 R15=0800cba4
XPSR=6103 -ZC- T handler
s00= s01= d00=
s02= s03= d01=
s04= s05= d02=
s06= s07= d03=
s08= s09= d04=
s10= s11= d05=
s12= s13= d06=
s14= s15= d07=
s16= s17= d08=
s18= s19= d09=
s20= s21= d10=
s22= s23= d11=
s24= s25= d12=
s26= s27= d13=
s28= s29= d14=
s30= s31= d15=
s32= s33= d16=
s34= s35= d17=
s36= s37= d18=
s38= s39= d19=
s40= s41= d20=
s42= s43= d21=
s44= s45= d22=
s46= s47= d23=
s48= s49= d24=
s50= s51= d25=
s52= s53= d26=
s54= s55= d27=
s56= s57= d28=
s58= s59= d29=
s60= s61= d30=
s62= s63= d31=
FPSCR: 
Aborted (core dumped)

Alistair

>
> Alistair
>
> >
> > thanks
> > -- PMM
> >
> >
>

Re: [Qemu-devel] [PATCH v2] target/arm: Stop using variable length array in dc_zva

[Philippe Mathieu-Daudé]

On 5/3/19 2:04 PM, Peter Maydell wrote:
> Currently the dc_zva helper function uses a variable length
> array. In fact we know (as the comment above remarks) that
> the length of this array is bounded because the architecture
> limits the block size and QEMU limits the target page size.
> Use a fixed array size and assert that we don't run off it.
> 
> Signed-off-by: Peter Maydell 
> ---
> Changes v1->v2:
>  * use ARRAY_SIZE() instead of sizeof()

Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 

>  * add a comment to make it a bit clearer that the
>expected size of hostaddr[] is only 2 entries
> ---
>  target/arm/helper.c | 8 ++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 81a92ab4911..10444d12b18 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -1,4 +1,5 @@
>  #include "qemu/osdep.h"
> +#include "qemu/units.h"
>  #include "target/arm/idau.h"
>  #include "trace.h"
>  #include "cpu.h"
> @@ -13099,14 +13100,17 @@ void HELPER(dc_zva)(CPUARMState *env, uint64_t 
> vaddr_in)
>   * We know that in fact for any v8 CPU the page size is at least 4K
>   * and the block size must be 2K or less, but TARGET_PAGE_SIZE is 
> only
>   * 1K as an artefact of legacy v5 subpage support being present in 
> the
> - * same QEMU executable.
> + * same QEMU executable. So in practice the hostaddr[] array has
> + * two entries, given the current setting of TARGET_PAGE_BITS_MIN.
>   */
>  int maxidx = DIV_ROUND_UP(blocklen, TARGET_PAGE_SIZE);
> -void *hostaddr[maxidx];
> +void *hostaddr[DIV_ROUND_UP(2 * KiB, 1 << TARGET_PAGE_BITS_MIN)];
>  int try, i;
>  unsigned mmu_idx = cpu_mmu_index(env, false);
>  TCGMemOpIdx oi = make_memop_idx(MO_UB, mmu_idx);
>  
> +assert(maxidx <= ARRAY_SIZE(hostaddr));
> +
>  for (try = 0; try < 2; try++) {
>  
>  for (i = 0; i < maxidx; i++) {
> 

Re: [Qemu-devel] [PATCH v1 5/5] hw/arm: Add the Netduino Plus 2

[Alistair]

On Thu, May 2, 2019, at 3:06 AM, Peter Maydell wrote:
> On Tue, 30 Apr 2019 at 21:29, Alistair Francis  wrote:
> >
> > On Tue, Apr 30, 2019 at 9:02 AM Peter Maydell  
> > wrote:
> > > Can you explain the purpose of the reset code? None of the other
> > > v7m boards seem to need to do a manual qemu_register_reset().
> >
> > The reset code allows the machine to work with the -kernel option.
> > Without the reset override using -kernel results in the guest starting
> > at the wrong address. We can use the -device loader option without the
> > reset code though.
> 
> That sounds in line with how -kernel works on the other armv7m
> boards -- the expectation is that your image file includes a
> full vector table and the CPU will read the PC and SP from it
> when it resets. If you want "honour the entry point" you can
> use -device loader, as you say.
> 
> Ignoring the entry point for -kernel ELF files is certainly
> a bit confusing, but I think if we want to change this we should
> do it globally, rather than having one board which behaves
> differently to the rest. Changing it does have some awkwardness:

Hmm... That is a good point. It is confusing having something just for one 
board. I'll drop this part and we can re-evaluate later.

> * possibility of breaking previously working images

I have no way to test the other boards, so this might be difficult to change.

> * we can get the initial PC from the ELF entrypoint, but if
>  we do this what do we do about the initial SP value ?

Not sure about this one either. I'm guessing it changes between the different M 
cores.

Alistair

> 
> thanks
> -- PMM
> 
> 

Re: [Qemu-devel] [PATCH 2/2] hw/arm/boot: Honour image size field in AArch64 Image format kernels

[Richard Henderson]

On 5/3/19 10:13 AM, Peter Maydell wrote:
> Since Linux v3.17, the kernel's Image header includes a field image_size,
> which gives the total size of the kernel including unpopulated data
> sections such as the BSS). If this is present, then return it from
> load_aarch64_image() as the true size of the kernel rather than
> just using the size of the Image file itself. This allows the code
> which calculates where to put the initrd to avoid putting it in
> the kernel's BSS area.
> 
> This means that we should be able to reliably load kernel images
> which are larger than 128MB without accidentally putting the
> initrd or dtb in locations that clash with the kernel itself.
> 
> Fixes: https://bugs.launchpad.net/qemu/+bug/1823998
> Signed-off-by: Peter Maydell 
> ---
>  hw/arm/boot.c | 17 +++--
>  1 file changed, 15 insertions(+), 2 deletions(-)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH for-4.1] block: Use bdrv_unref_child() for all children in bdrv_close()

[Max Reitz]

On 31.03.19 13:17, Alberto Garcia wrote:
> bdrv_unref_child() does the following things:
> 
>   - Updates the child->bs->inherits_from pointer.
>   - Calls bdrv_detach_child() to remove the BdrvChild from bs->children.
>   - Calls bdrv_unref() to unref the child BlockDriverState.
> 
> When bdrv_unref_child() was introduced in commit 33a604075c it was not
> used in bdrv_close() because the drivers that had additional children
> (like quorum or blkverify) had already called bdrv_unref() on their
> children during their own close functions.
> 
> This was changed later (in 0bd6e91a7e for quorum, in 3e586be0b2 for
> blkverify) so there's no reason not to use bdrv_unref_child() in
> bdrv_close() anymore.
> 
> After this there's also no need to remove bs->backing and bs->file
> separately from the rest of the children, so bdrv_close() can be
> simplified.
> 
> Signed-off-by: Alberto Garcia 
> ---
>  block.c | 16 +++-
>  1 file changed, 3 insertions(+), 13 deletions(-)

Hm, I’m afraid this breaks make check:

$ make -j 4 && make tests/test-bdrv-drain && gtester tests/test-bdrv-drain

TEST: tests/test-bdrv-drain... (pid=22321)
test-bdrv-drain: block.c:5397: bdrv_unref: Assertion `bs->refcnt > 0'
failed.

(gdb) bt
#0  0x7f15c7ffc57f in raise () from /lib64/libc.so.6
#1  0x7f15c7fe6895 in abort () from /lib64/libc.so.6
#2  0x7f15c7fe6769 in __assert_fail_base.cold.0 () from /lib64/libc.so.6
#3  0x7f15c7ff4a26 in __assert_fail () from /lib64/libc.so.6
#4  0x5562a11b0a00 in bdrv_unref (bs=0x5562a3370c30) at block.c:5397
#5  bdrv_unref (bs=0x5562a3370c30) at block.c:5392
#6  0x5562a11a7f8f in test_detach_indirect (by_parent_cb=) at tests/test-bdrv-drain.c:1442
#7  0x7f15c8766fca in ?? () from /lib64/libglib-2.0.so.0
#8  0x7f15c8766e84 in ?? () from /lib64/libglib-2.0.so.0
#9  0x7f15c8766e84 in ?? () from /lib64/libglib-2.0.so.0
#10 0x7f15c8767282 in g_test_run_suite () from /lib64/libglib-2.0.so.0
#11 0x7f15c87672a5 in g_test_run () from /lib64/libglib-2.0.so.0
#12 0x5562a11a5b77 in main (argc=, argv=) at tests/test-bdrv-drain.c:1617

So I’ll dequeue this patch for now.

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH] chardev/char-i2c: Implement Linux I2C character device

[Ernest Esene]

On Fri, May 03, 2019 at 03:24:06PM -0500, Eric Blake wrote:
> On 5/3/19 2:31 PM, Ernest Esene wrote:
> > Add support for Linux I2C character device for I2C device passthrough
> > For example:
> > -chardev linux-i2c,address=0x46,path=/dev/i2c-N,id=i2c-chardev
> > 
> > Signed-off-by: Ernest Esene 
> > ---
> 
> Just an interface review:
> 
> > +++ b/qapi/char.json
> > @@ -240,6 +240,21 @@
> >'data': { 'device': 'str' },
> >'base': 'ChardevCommon' }
> >  
> Missing a 'Since: 4.1' line.
4.1? Oh! I couldn't guess this number, I had to deliberately omit it.
> 
> > +{ 'struct': 'ChardevI2c',
> > +  'data': { 'device': 'str',
> > +'address': 'int16'},
> > +  'base': 'ChardevCommon'}
> 
> 'if': 'defined(CONFIG_LINUX)'
> 
> as part of the usage of this struct, so that introspection will only see
> the struct where it can be used.
> 
> > +
> >  ##
> >  # @ChardevSocket:
> >  #
> > @@ -398,6 +413,7 @@
> >'data': { 'file': 'ChardevFile',
> >  'serial': 'ChardevHostdev',
> >  'parallel': 'ChardevHostdev',
> > +'i2c': 'ChardevI2c',
> >  'pipe': 'ChardevHostdev',
> >  'socket': 'ChardevSocket',
> >  'udp': 'ChardevUdp',
> > 
> 
> -- 
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.   +1-919-301-3226
> Virtualization:  qemu.org | libvirt.org
> 
Thanks so much for the useful review. I have applied the changes and
will soon send v2 of the patch.

I hope it is OK to update the "MAINTAINERS" file this

Character Devices (Linux I2C)
M: Ernest Esene 
S: Maintained
F: chardev/char-i2c.c


-Ernest Esene


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PULL 19/19] configure: automatically pick python3 is available

[Eduardo Habkost]

On Fri, May 03, 2019 at 06:00:11PM -0300, Eduardo Habkost wrote:
> On Fri, May 03, 2019 at 06:41:43PM +0200, Thomas Huth wrote:
> > On 03/05/2019 02.41, Eduardo Habkost wrote:
> > > From: Daniel P. Berrangé 
> > > 
> > > Unless overridden via an env var or configure arg, QEMU will only look
> > > for the 'python' binary in $PATH. This is unhelpful on distros which
> > > are only shipping Python 3.x (eg Fedora) in their default install as,
> > > if they comply with PEP 394, the bare 'python' binary won't exist.
> > > 
> > > This changes configure so that by default it will search for all three
> > > common python binaries, preferring to find Python 3.x versions.
> > > 
> > > Signed-off-by: Daniel P. Berrangé 
> > > Message-Id: <20190327170701.23798-1-berra...@redhat.com>
> > > Signed-off-by: Eduardo Habkost 
> > > ---
> > >  configure | 18 +++---
> > >  1 file changed, 15 insertions(+), 3 deletions(-)
> > 
> > I haven't bisected it, but I think this patch here broke the gitlab-ci 
> > tests:
> > 
> >  https://gitlab.com/huth/qemu/-/jobs/206806257
> > 
> > Seems like the test is now failing when you don't have an UTF-8 locale:
> > 
> >  LANG=C make check-qapi-schema
> 
> I couldn't reproduce it this way, probably because I'm running Python 3.7 
> which
> implements PEP 538 ("Coercing the legacy C locale to a UTF-8 based locale").
> 
> But I can force it to break using:
> 
>   PYTHONIOENCODING=ascii make check-qapi-schema
> 
> >  [...]
> >  TESTtests/qapi-schema/union-base-empty.out
> >  --- /builds/huth/qemu/tests/qapi-schema/unicode-str.err2019-05-03 
> > 15:21:39.0 +
> >  +++ -  2019-05-03 15:42:01.561762978 +
> >  @@ -1 +1 @@
> >  -tests/qapi-schema/unicode-str.json:2: 'command' uses invalid name 'é'
> >  +tests/qapi-schema/unicode-str.json:2: 'command' uses invalid name '\xe9'
> >  /builds/huth/qemu/tests/Makefile.include:1105: recipe for target 
> > 'check-tests/qapi-schema/unicode-str.json' failed
> >  make: *** [check-tests/qapi-schema/unicode-str.json] Error 1
> > 
> > Any ideas how to fix this?
> 
> Probably we just need to specify an explicit encoding at the statement that
> prints the error message to stderr.  I will give it a try.

Forcing a specific encoding inside test-qapi.py would very easy
on Python 3.7+ (sys.stderr.reconfigure(...)), but tricky on older
versions.  I believe this is the simplest way to fix the problem
on Python 3.5 and 3.6.

Can somebody confirm this really fixes the problem on gitlab-ci?

---
diff --git a/tests/Makefile.include b/tests/Makefile.include
index 7c8b9c84b2..af88ab6f8b 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -1103,7 +1103,7 @@ check-tests/qemu-iotests-quick.sh: 
tests/qemu-iotests-quick.sh qemu-img$(EXESUF)
 .PHONY: $(patsubst %, check-%, $(check-qapi-schema-y))
 $(patsubst %, check-%, $(check-qapi-schema-y)): check-%.json: 
$(SRC_PATH)/%.json
$(call quiet-command, PYTHONPATH=$(SRC_PATH)/scripts \
-   $(PYTHON) $(SRC_PATH)/tests/qapi-schema/test-qapi.py \
+   PYTHONIOENCODING=utf-8 $(PYTHON) 
$(SRC_PATH)/tests/qapi-schema/test-qapi.py \
$^ >$*.test.out 2>$*.test.err; \
echo $$? >$*.test.exit, \
"TEST","$*.out")

-- 
Eduardo

Re: [Qemu-devel] Failing QEMU iotest 175

[Nir Soffer]

On Fri, May 3, 2019, 23:21 Eric Blake  wrote:

> On 5/2/19 11:37 PM, Thomas Huth wrote:
> > On 02/05/2019 23.56, Eric Blake wrote:
> >> On 4/28/19 10:18 AM, Thomas Huth wrote:
> >>> QEMU iotest 175 is failing for me when I run it with -raw:
> >>>
> >>
> >>>  == creating image with default preallocation ==
> >>>  Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576
> >>> -size=1048576, blocks=0
> >>> +size=1048576, blocks=2
> >>
> >> What filesystem?
> >
> > ext4
> >
>
> Hmm, it's passing for me on ext4, but that probably means we have
> different configuration parameters. I'm not sure how to easily show what
> parameters a particular ext4 partition uses to compare the differences
> between your setup and mine (mine is tuned to whatever defaults Fedora's
> installer chose on my behalf), so maybe someone else can chime in.
>
> >> It should be fairly obvious that 'stat -c blocks=%b' is
> >> file-system dependent (some allocate slightly more or less space, based
> >> on granularities and on predictions of future use), so we may need to
> >> update the test to apply a filter or otherwise allow a bit of fuzz in
> >> the answer. But 0/2 is definitely different than...
> >>>
> >>>  == creating image with preallocation off ==
> >>>  Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576
> preallocation=off
> >>> -size=1048576, blocks=0
> >>> +size=1048576, blocks=2
> >>>
> >>>  == creating image with preallocation full ==
> >>>  Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576
> preallocation=full
> >>> -size=1048576, blocks=2048
> >>> +size=1048576, blocks=2050
> >>
> >> 2048/2050, so we DO have some indication of whether the file is sparse
> >> or fully allocated.
> >
> > Maybe we could check that the value after "blocks=" is a single digit in
> > the first case, and matches "blocks=20.." in the second case?
>
> I wonder if 'qemu-img map --output=json $TEST_IMG' might be any more
> reliable (at least for ignoring any extra block allocations associated
> with the file, if it is some journaling option or xattr or other reason
> why your files seem to occupy more disk sectors than just the size of
> the file would imply).
>

I think it should work better and is more correct, testing actual sparsness
instead of underlying file system implementation.

I can send a fix next week.

Nir


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

Re: [Qemu-devel] [PATCH] qcow2: Assert that host cluster offsets fit in L2 table entries

[Eric Blake]

On 5/3/19 4:38 AM, Alberto Garcia wrote:
> The standard cluster descriptor in L2 table entries has a field to
> store the host cluster offset. When we need to get that offset from an
> entry we use L2E_OFFSET_MASK to ensure that we only use the bits that
> belong to that field.
> 
> But while that mask is used every time we read from an L2 entry, it
> is never used when we write to it. Due to the QCOW_MAX_CLUSTER_OFFSET
> limit set in the cluster allocation code QEMU can never produce
> offsets that don't fit in that field so any such offset would indicate
> a bug in QEMU.

Yeah, I'm not seeing where this one could ever overflow.

> 
> Compressed cluster descriptors contain two fields (host cluster offset
> and size of the compressed data) and the situation with them is
> similar. In this case the masks are not constant but are stored in the
> csize_mask and cluster_offset_mask fields of BDRVQcow2State.

For this one, we did have a bug in the past where we were overflowing,
as evidenced by iotest 220 shortly after we patched the bug (77d6a215).

> 
> Signed-off-by: Alberto Garcia 
> ---
>  block/qcow2-cluster.c | 14 --
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 

Adding more assertions shouldn't hurt.
Reviewed-by: Eric Blake 

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PULL 19/19] configure: automatically pick python3 is available

[Eduardo Habkost]

On Fri, May 03, 2019 at 06:41:43PM +0200, Thomas Huth wrote:
> On 03/05/2019 02.41, Eduardo Habkost wrote:
> > From: Daniel P. Berrangé 
> > 
> > Unless overridden via an env var or configure arg, QEMU will only look
> > for the 'python' binary in $PATH. This is unhelpful on distros which
> > are only shipping Python 3.x (eg Fedora) in their default install as,
> > if they comply with PEP 394, the bare 'python' binary won't exist.
> > 
> > This changes configure so that by default it will search for all three
> > common python binaries, preferring to find Python 3.x versions.
> > 
> > Signed-off-by: Daniel P. Berrangé 
> > Message-Id: <20190327170701.23798-1-berra...@redhat.com>
> > Signed-off-by: Eduardo Habkost 
> > ---
> >  configure | 18 +++---
> >  1 file changed, 15 insertions(+), 3 deletions(-)
> 
> I haven't bisected it, but I think this patch here broke the gitlab-ci tests:
> 
>  https://gitlab.com/huth/qemu/-/jobs/206806257
> 
> Seems like the test is now failing when you don't have an UTF-8 locale:
> 
>  LANG=C make check-qapi-schema

I couldn't reproduce it this way, probably because I'm running Python 3.7 which
implements PEP 538 ("Coercing the legacy C locale to a UTF-8 based locale").

But I can force it to break using:

  PYTHONIOENCODING=ascii make check-qapi-schema

>  [...]
>  TESTtests/qapi-schema/union-base-empty.out
>  --- /builds/huth/qemu/tests/qapi-schema/unicode-str.err  2019-05-03 
> 15:21:39.0 +
>  +++ -2019-05-03 15:42:01.561762978 +
>  @@ -1 +1 @@
>  -tests/qapi-schema/unicode-str.json:2: 'command' uses invalid name 'é'
>  +tests/qapi-schema/unicode-str.json:2: 'command' uses invalid name '\xe9'
>  /builds/huth/qemu/tests/Makefile.include:1105: recipe for target 
> 'check-tests/qapi-schema/unicode-str.json' failed
>  make: *** [check-tests/qapi-schema/unicode-str.json] Error 1
> 
> Any ideas how to fix this?

Probably we just need to specify an explicit encoding at the statement that
prints the error message to stderr.  I will give it a try.

-- 
Eduardo

Re: [Qemu-devel] Failing QEMU iotest 221

[Eric Blake]

On 5/2/19 11:43 PM, Thomas Huth wrote:
> On 03/05/2019 00.02, Eric Blake wrote:
>> On 4/28/19 10:21 AM, Thomas Huth wrote:
>>> QEMU iotest 221 is failing for me, too, when I run it with -raw:
>>
>> Which filesystem?
> 
> ext4 again.
> 
> $ stat -f /home/thuth/tmp/qemu-build/tests/qemu-iotests/
>   File: "/home/thuth/tmp/qemu-build/tests/qemu-iotests/"
> ID: 1e68b4a412e09716 Namelen: 255 Type: ext2/ext3
> Block size: 1024   Fundamental block size: 1024

Odd that it is so small; these days, most ext4 systems have a block size
of 4k.

> 
> Maybe the "check" script should report the output of "stat -f", too?

Wouldn't hurt, although that doesn't tell us all of the file system
tuning parameters that might be important to reproducing a problem.


>>> +++ /home/thuth/tmp/qemu-build/tests/qemu-iotests/221.out.bad   
>>> 2019-04-28 17:18:52.0 +0200
>>> @@ -7,10 +7,10 @@
>>>  [{ "start": 0, "length": 43520, "depth": 0, "zero": true, "data": false, 
>>> "offset": OFFSET}]
>>>  wrote 1/1 bytes at offset 43008
>>>  1 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>>> -[{ "start": 0, "length": 40960, "depth": 0, "zero": true, "data": false, 
>>> "offset": OFFSET},
>>> -{ "start": 40960, "length": 2049, "depth": 0, "zero": false, "data": true, 
>>> "offset": OFFSET},
>>> +[{ "start": 0, "length": 43008, "depth": 0, "zero": true, "data": false, 
>>> "offset": OFFSET},
>>> +{ "start": 43008, "length": 1, "depth": 0, "zero": false, "data": true, 
>>> "offset": OFFSET},
>>
>> Ugh. Hole granularities are file-system specific, so we need to figure
>> out some way to fuzz the input. It might also be possible to pick nicer
>> size numbers - perhaps if the test image is sized at 64k+1 instead of
>> 43009 (84*512, but NOT evenly divisible by 4k), the +1 byte is more
>> likely to be directly one a hole boundary, rather than being somewhere
>> that causes rounding the hole boundary 2k earlier because of 4k or 64k
>> sizing constraints.
> 
> Ok ... sounds like that's definitely something I'd like to leave to you
> or one of the block guys to fix.

I can certainly prepare a patch that widens the file to 64k+1 instead of
43008+1, but since I can't (yet) reproduce the failure, I'd be relying
on you to verify that it makes a difference.

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH] VirtIO-RNG: Update default entropy source to `/dev/urandom`

[Kashyap Chamarthy]

On Fri, May 03, 2019 at 04:49:05PM +0100, Daniel P. Berrangé wrote:
> On Fri, May 03, 2019 at 05:46:13PM +0200, Kashyap Chamarthy wrote:
> > When QEMU exposes a VirtIO-RNG device to the guest, that device needs a
> > source of entropy, and that source needs to be "non-blocking", like
> > `/dev/urandom`.  However, currently QEMU defaults to the problematic
> > `/dev/random`, which is "blocking" (as in, it waits until sufficient
> > entropy is available).
> > 
> > So change the entropy source to the recommended `/dev/urandom`.
> > 
> > Related discussion in these[1][2] past threads.
> > 
> > [1] https://lists.nongnu.org/archive/html/qemu-devel/2018-06/msg08335.html
> > -- "RNG: Any reason QEMU doesn't default to `/dev/urandom`?"
> > [2] https://lists.nongnu.org/archive/html/qemu-devel/2018-09/msg02724.html
> > -- "[RFC] Virtio RNG: Consider changing the default entropy source to
> >/dev/urandom"
> > 
> > Signed-off-by: Kashyap Chamarthy 
> > ---
> >  backends/rng-random.c | 2 +-
> >  qemu-options.hx   | 2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> Reviewed-by: Daniel P. Berrangé 

I'm wondering if this needs to be mentioned on a Release Notes wiki
somewhere -- because we're changing the default.

-- 
/kashyap

Re: [Qemu-devel] [PATCH] chardev/char-i2c: Implement Linux I2C character device

[Eric Blake]

On 5/3/19 2:31 PM, Ernest Esene wrote:
> Add support for Linux I2C character device for I2C device passthrough
> For example:
> -chardev linux-i2c,address=0x46,path=/dev/i2c-N,id=i2c-chardev
> 
> Signed-off-by: Ernest Esene 
> ---

Just an interface review:

> +++ b/qapi/char.json
> @@ -240,6 +240,21 @@
>'data': { 'device': 'str' },
>'base': 'ChardevCommon' }
>  
> +##
> +# @ChardevI2c:
> +#
> +# Configuration info for i2c chardev (only on linux).

Rather than a parenthetical comment, you could use:

> +#
> +# @device: The name of the special file for the device,
> +#  i.e. /dev/i2c-0 on linux
> +# @address: The address of the i2c device on the host.
> +#
> +##

Missing a 'Since: 4.1' line.

> +{ 'struct': 'ChardevI2c',
> +  'data': { 'device': 'str',
> +'address': 'int16'},
> +  'base': 'ChardevCommon'}

'if': 'defined(CONFIG_LINUX)'

as part of the usage of this struct, so that introspection will only see
the struct where it can be used.

> +
>  ##
>  # @ChardevSocket:
>  #
> @@ -398,6 +413,7 @@
>'data': { 'file': 'ChardevFile',
>  'serial': 'ChardevHostdev',
>  'parallel': 'ChardevHostdev',
> +'i2c': 'ChardevI2c',
>  'pipe': 'ChardevHostdev',
>  'socket': 'ChardevSocket',
>  'udp': 'ChardevUdp',
> 

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v2 1/3] q35: set split kernel irqchip as default

[Eduardo Habkost]

On Fri, May 03, 2019 at 04:00:33PM -0400, Michael S. Tsirkin wrote:
> On Mon, Apr 29, 2019 at 11:55:56AM -0300, Eduardo Habkost wrote:
> > irqchip=split and irqchip=kernel aren't guest ABI compatible, are
> > they?
> 
> Can you remind me why they aren't?

We have the code introduced by patch 3/3 from this series, but I
don't know if it's the only difference:

hw/i386/x86-iommu.c=static void x86_iommu_realize(DeviceState *dev, Error 
**errp)
[...]
hw/i386/x86-iommu.c:bool irq_all_kernel = kvm_irqchip_in_kernel() && 
!kvm_irqchip_is_split();
[...]
hw/i386/x86-iommu.c-/* If the user didn't specify IR, choose a default 
value for it */
hw/i386/x86-iommu.c-if (x86_iommu->intr_supported == ON_OFF_AUTO_AUTO) {
hw/i386/x86-iommu.c-x86_iommu->intr_supported = irq_all_kernel ?
hw/i386/x86-iommu.c-ON_OFF_AUTO_OFF : ON_OFF_AUTO_ON;
hw/i386/x86-iommu.c-}

-- 
Eduardo

Re: [Qemu-devel] Failing QEMU iotest 175

[Eric Blake]

On 5/2/19 11:37 PM, Thomas Huth wrote:
> On 02/05/2019 23.56, Eric Blake wrote:
>> On 4/28/19 10:18 AM, Thomas Huth wrote:
>>> QEMU iotest 175 is failing for me when I run it with -raw:
>>>
>>
>>>  == creating image with default preallocation ==
>>>  Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576
>>> -size=1048576, blocks=0
>>> +size=1048576, blocks=2
>>
>> What filesystem?
> 
> ext4
> 

Hmm, it's passing for me on ext4, but that probably means we have
different configuration parameters. I'm not sure how to easily show what
parameters a particular ext4 partition uses to compare the differences
between your setup and mine (mine is tuned to whatever defaults Fedora's
installer chose on my behalf), so maybe someone else can chime in.

>> It should be fairly obvious that 'stat -c blocks=%b' is
>> file-system dependent (some allocate slightly more or less space, based
>> on granularities and on predictions of future use), so we may need to
>> update the test to apply a filter or otherwise allow a bit of fuzz in
>> the answer. But 0/2 is definitely different than...
>>>
>>>  == creating image with preallocation off ==
>>>  Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 preallocation=off
>>> -size=1048576, blocks=0
>>> +size=1048576, blocks=2
>>>
>>>  == creating image with preallocation full ==
>>>  Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 preallocation=full
>>> -size=1048576, blocks=2048
>>> +size=1048576, blocks=2050
>>
>> 2048/2050, so we DO have some indication of whether the file is sparse
>> or fully allocated.
> 
> Maybe we could check that the value after "blocks=" is a single digit in
> the first case, and matches "blocks=20.." in the second case?

I wonder if 'qemu-img map --output=json $TEST_IMG' might be any more
reliable (at least for ignoring any extra block allocations associated
with the file, if it is some journaling option or xattr or other reason
why your files seem to occupy more disk sectors than just the size of
the file would imply).

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v2 1/3] q35: set split kernel irqchip as default

[Michael S. Tsirkin]

On Mon, Apr 29, 2019 at 11:55:56AM -0300, Eduardo Habkost wrote:
> irqchip=split and irqchip=kernel aren't guest ABI compatible, are
> they?

Can you remind me why they aren't?

> -- 
> Eduardo

[Qemu-devel] [PATCH] Deprecate Python 2 support

[Eduardo Habkost]

Python 2 will reach end of life in January 1 2020.  Declare it as
deprecated.

Signed-off-by: Eduardo Habkost 
---
 configure| 8 
 qemu-deprecated.texi | 8 
 2 files changed, 16 insertions(+)

diff --git a/configure b/configure
index 5b183c2e39..50385061ed 100755
--- a/configure
+++ b/configure
@@ -6461,6 +6461,14 @@ if test "$supported_os" = "no"; then
 echo "us upstream at qemu-devel@nongnu.org."
 fi
 
+# Note that if the Python conditional here evaluates True we will exit
+# with status 1 which is a shell 'false' value.
+if ! $python -c 'import sys; sys.exit(sys.version_info < (3,0))'; then
+  echo
+  echo "WARNING: Python 2 support is deprecated" >&2
+  echo "WARNING: Python 3 will be required for building future versions of 
QEMU" >&2
+fi
+
 config_host_mak="config-host.mak"
 
 echo "# Automatically generated by configure - do not modify" 
>config-all-disas.mak
diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi
index 842e71b11d..2f2d9a3e95 100644
--- a/qemu-deprecated.texi
+++ b/qemu-deprecated.texi
@@ -206,3 +206,11 @@ Note that if you are exposing the export via /dev/nbd0, it 
is easier
 to just export the entire image and then mount only /dev/nbd0p1 than
 it is to reinvoke @command{qemu-nbd -c /dev/nbd0} limited to just a
 subset of the image.
+
+@section Build system
+
+@subsection Python 2 support (since 4.1.0)
+
+In the future, QEMU will require Python 3 to be available at
+build time.  Support for Python 2 in scripts shipped with QEMU
+is deprecated.
-- 
2.18.0.rc1.1.g3f1ff2140

Re: [Qemu-devel] [PATCH v5 00/15] demacro softmmu (plus tests/coverage)

[Alex Bennée]

Richard Henderson  writes:

> On 4/30/19 9:52 AM, Alex Bennée wrote:
>> I've also moved the existing system memory test and made it multiarch
>> and added the bootstrapping for aarch64 system tests. I would like to
>> add support for Big Endian as well but I didn't want to delay the
>> posting of the series. It would also be nice to exercise the
>> ioread/write paths and other handling but I leave this as an exercise
>> for later.
>
> Somewhere in there you're adding
>
>   -chardev file,path=hello.out,id=output

It's in the default runner config in tests/tcg/Makefile

> but there's no corresponding use of the chardev.
> Which, somehow doesn't seem to matter to your aarch64
> testcase,

Argh.. it's because -semihosting is different from -serial and doesn't
allow the usual redirection rules you get with a chardev..

> but when I try this for alpha I truly get
> no output at all.  I needed
>
>   -serial chardev:output

or -serial chadev,id=output?
>
> to populate the file.
>
>
> r~


--
Alex Bennée

[Qemu-devel] [PATCH] chardev/char-i2c: Implement Linux I2C character device

[Ernest Esene]

Add support for Linux I2C character device for I2C device passthrough
For example:
-chardev linux-i2c,address=0x46,path=/dev/i2c-N,id=i2c-chardev

Signed-off-by: Ernest Esene 
---
 chardev/Makefile.objs  |   1 +
 chardev/char-i2c.c | 142 +
 chardev/char.c |   3 ++
 include/chardev/char.h |   1 +
 qapi/char.json |  16 ++
 5 files changed, 163 insertions(+)
 create mode 100644 chardev/char-i2c.c

diff --git a/chardev/Makefile.objs b/chardev/Makefile.objs
index d68e1347f9..6c96b9a353 100644
--- a/chardev/Makefile.objs
+++ b/chardev/Makefile.objs
@@ -16,6 +16,7 @@ chardev-obj-y += char-stdio.o
 chardev-obj-y += char-udp.o
 chardev-obj-$(CONFIG_WIN32) += char-win.o
 chardev-obj-$(CONFIG_WIN32) += char-win-stdio.o
+chardev-obj-$(CONFIG_POSIX) +=char-i2c.o
 
 common-obj-y += msmouse.o wctablet.o testdev.o
 common-obj-$(CONFIG_BRLAPI) += baum.o
diff --git a/chardev/char-i2c.c b/chardev/char-i2c.c
new file mode 100644
index 00..78cf973bd7
--- /dev/null
+++ b/chardev/char-i2c.c
@@ -0,0 +1,142 @@
+/*
+ * QEMU System Emulator
+ *
+ * Copyright (c) 2019 Ernest Esene 
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "qemu/option.h"
+#include "qemu-common.h"
+#include "io/channel-file.h"
+
+#include "chardev/char-fd.h"
+#include "chardev/char.h"
+
+#include 
+#include 
+
+#define CHR_IOCTL_I2C_SET_ADDR 1
+
+#define CHR_I2C_ADDR_10BIT_MAX 1023
+#define CHR_I2C_ADDR_7BIT_MAX 127
+
+void qemu_set_block(int fd);
+
+static int i2c_ioctl(Chardev *chr, int cmd, void *arg)
+{
+FDChardev *fd_chr = FD_CHARDEV(chr);
+QIOChannelFile *floc = QIO_CHANNEL_FILE(fd_chr->ioc_in);
+int fd = floc->fd;
+int addr;
+
+switch (cmd) {
+case CHR_IOCTL_I2C_SET_ADDR:
+addr = (int) (long) arg;
+
+if (addr > CHR_I2C_ADDR_7BIT_MAX) {
+/*TODO: check if adapter support 10-bit addr
+I2C_FUNC_10BIT_ADDR */
+if (ioctl(fd, I2C_TENBIT, addr) < 0) {
+goto err;
+}
+}
+else {
+if (ioctl(fd, I2C_SLAVE, addr) < 0) {
+goto err;
+}
+}
+break;
+
+default:
+return -ENOTSUP;
+
+}
+return 0;
+err:
+return -ENOTSUP;
+}
+
+static void qmp_chardev_open_i2c(Chardev *chr, ChardevBackend *backend,
+ bool *be_opened, Error **errp)
+{
+ChardevI2c *i2c = backend->u.i2c.data;
+void *addr;
+int fd;
+
+fd = qmp_chardev_open_file_source(i2c->device, O_RDWR | O_NONBLOCK,
+  errp);
+if (fd < 0) {
+   return;
+}
+qemu_set_block(fd);
+qemu_chr_open_fd(chr, fd, fd);
+addr = (void *) (long) i2c->address;
+i2c_ioctl(chr, CHR_IOCTL_I2C_SET_ADDR, addr);
+}
+
+static void qemu_chr_parse_i2c(QemuOpts *opts, ChardevBackend *backend, Error 
**errp)
+{
+const char *device = qemu_opt_get(opts, "path");
+const char *addr = qemu_opt_get(opts, "address");
+long address;
+ChardevI2c *i2c;
+
+if (device == NULL) {
+error_setg(errp, "chardev: linux-i2c: no device path given");
+return;
+}
+if (addr == NULL) {
+error_setg(errp, "chardev: linux-i2c: no device address given");
+return;
+}
+address = strtol(addr, NULL, 0);
+if (address < 0 || address > CHR_I2C_ADDR_10BIT_MAX) {
+error_setg(errp, "chardev: linux-i2c: invalid device address given");
+return;
+}
+backend->type = CHARDEV_BACKEND_KIND_I2C;
+i2c = backend->u.i2c.data = g_new0(ChardevI2c, 1);
+qemu_chr_parse_common(opts, qapi_ChardevI2c_base(i2c));
+i2c->device = g_strdup(device);
+i2c->address = (int16_t) address;
+}
+
+static void char_i2c_class_init(ObjectClass *oc, void 

Re: [Qemu-devel] [PATCH RFC v8 01/12] target/rx: TCG translation

[Richard Henderson]

On 5/2/19 7:33 AM, Yoshinori Sato wrote:
> +/* conditional branch helper */
> +static void rx_bcnd_main(DisasContext *ctx, int cd, int dst)
> +{
> +DisasCompare dc;
> +TCGLabel *t, *done;
> +
> +switch (cd) {
> +case 0 ... 13:
> +dc.temp = tcg_temp_new();
> +psw_cond(, cd);
> +t = gen_new_label();
> +done = gen_new_label();
> +tcg_gen_brcondi_i32(dc.cond, dc.value, 0, t);
> +gen_goto_tb(ctx, 0, ctx->base.pc_next);
> +tcg_gen_br(done);
> +gen_set_label(t);
> +gen_goto_tb(ctx, 1, ctx->pc + dst);
> +gen_set_label(done);
> +tcg_temp_free(dc.temp);
> +break;
> +case 14:
> +/* always true case */
> +gen_goto_tb(ctx, 0, ctx->pc + dst);
> +break;
> +case 15:
> +/* always false case */
> +/* Nothing do */
> +break;
> +}
> +ctx->base.is_jmp = DISAS_JUMP;
> +}

Do not set is_jmp to DISAS_JUMP here.  We have already set is_jmp to
DISAS_NORETURN in gen_goto_tb.  For case 15, we do not need to exit the TB in
order to treat the never-taken branch as a nop.

This assignment means that we will emit *another* exit from the TB in
rx_tr_tb_stop, which will be unreachable code.

This is the only bug I see in this revision.  Thanks for your patience!


r~

Re: [Qemu-devel] [PATCH v2] mips: Decide to map PAGE_EXEC in map_address

[Aleksandar Markovic]

> (ping)
> 
> Is there anything else I can do to help to get this merged?
> 
> https://patchew.org/QEMU/20190423110034.1260142-1-jakub.jer...@kernkonzept.com/

Hello, Jakub.

I will be reviewing your patch next week, please be patient. In any case, 
thanks for
your involving in solving this issue!

Aleksandar

Re: [Qemu-devel] [PATCH v2 1/3] q35: set split kernel irqchip as default

[Eduardo Habkost]

On Mon, Apr 29, 2019 at 09:22:12AM -0600, Alex Williamson wrote:
[...]
> > > What's a good 4.0.1 strategy to resolve this?  Re-instate KVM irqchip
> > > as the Q35 default?  I can't see that simply switching to current QEMU
> > > handling is a viable option for performance?  What about 4.1?  We could
> > > certainly improve EOI support in QEMU, there's essentially no support
> > > currently, but it seems like an uphill battle for an iothread based
> > > userspace ioapic to ever compare to KVM handling?  Thanks,  
> > 
> > irqchip=split and irqchip=kernel aren't guest ABI compatible, are
> > they?  That would make it impossible to fix this in pc-q35-4.0
> > for a 4.0.1 update.
> 
> I suppose it would require a pc-q35-4.0.1 machine type :-\  Thanks,

I wonder if it's possible to untangle this and make the irqchip
option stop affecting guest ABI on 4.1+ machine-types?  This way
QEMU could choose smarter defaults in the future without the
compatibility code hassle.

-- 
Eduardo

Re: [Qemu-devel] [PATCH RFC v8 12/12] hw/registerfields.h: Add 8bit and 16bit register macros.

[Richard Henderson]

On 5/3/19 8:27 AM, Alex Bennée wrote:
> 
> Yoshinori Sato  writes:
> 
>> Some RX peripheral using 8bit and 16bit registers.
>> Added 8bit and 16bit APIs.
> 
> Doesn't this mean the build breaks at some point? Features used by other
> patches should be introduced first so the build remains bisectable.

The only bug I would fix in the ordering is to make the change to configure
last, so that the target/rx is not enabled while the patches are staging.


r~

Re: [Qemu-devel] [PATCH RFC v8 04/12] target/rx: RX disassembler

[Richard Henderson]

On 5/2/19 7:34 AM, Yoshinori Sato wrote:
> +static int32_t li(DisasContext *ctx, int sz)
> +{
> +int32_t addr;
> +bfd_byte buf[4];
> +addr = ctx->addr;
> +
> +switch (sz) {
> +case 1:
> +ctx->addr += 1;
> +ctx->dis->read_memory_func(addr, buf, 1, ctx->dis);
> +return buf[0];
> +case 2:
> +ctx->addr += 2;
> +ctx->dis->read_memory_func(addr, buf, 2, ctx->dis);
> +return buf[1] << 8 | buf[0];
> +case 3:
> +ctx->addr += 3;
> +ctx->dis->read_memory_func(addr, buf, 3, ctx->dis);
> +return buf[2] << 16 | buf[1] << 8 | buf[0];
> +case 0:
> +ctx->addr += 4;
> +ctx->dis->read_memory_func(addr, buf, 4, ctx->dis);
> +return buf[3] << 24 | buf[2] << 16 | buf[1] << 8 | buf[0];
> +default:
> +g_assert_not_reached();
> +}
> +}

These should be computing signed values.  This is already correct over in
translate.c.  Also, we can make use of some endian-specific unaligned load
functions from .  So for the 4 cases:

  return (int8_t)buf[0];

  return ldsw_le_p(buf);

  return (int8_t)buf[2] << 16 | buf[1] << 8 | buf[0];

  return ldl_le_p(buf);


r~

Re: [Qemu-devel] [PATCH] qom/object: Display more helpful message when an object type is missing

[Eduardo Habkost]

On Sat, Apr 27, 2019 at 03:56:42PM +0200, Philippe Mathieu-Daudé wrote:
> When writing a new board, adding device which uses other devices
> (container) or simply refactoring, one can discover the hard way
> his machine misses some devices. In the case of containers, the
> error is not obvious:
> 
>   $ qemu-system-microblaze -M xlnx-zynqmp-pmu
>   **
>   ERROR:/source/qemu/qom/object.c:454:object_initialize_with_type: assertion 
> failed: (type != NULL)
>   Aborted (core dumped)
> 
> And we have to look at the coredump to figure the error:
> 
>   (gdb) bt
>   #1  0x7f84773cf895 in abort () at /lib64/libc.so.6
>   #2  0x7f847961fb53 in  () at /lib64/libglib-2.0.so.0
>   #3  0x7f847967a4de in g_assertion_message_expr () at 
> /lib64/libglib-2.0.so.0
>   #4  0x55c4bcac6c11 in object_initialize_with_type 
> (data=data@entry=0x55c4bdf239e0, size=size@entry=2464, type=) 
> at /source/qemu/qom/object.c:454
>   #5  0x55c4bcac6e6d in object_initialize 
> (data=data@entry=0x55c4bdf239e0, size=size@entry=2464, 
> typename=typename@entry=0x55c4bcc7c643 "xlnx.zynqmp_ipi") at 
> /source/qemu/qom/object.c:474
>   #6  0x55c4bc9ea474 in xlnx_zynqmp_pmu_init (machine=0x55c4bdd46000) at 
> /source/qemu/hw/microblaze/xlnx-zynqmp-pmu.c:176
>   #7  0x55c4bca3b6cb in machine_run_board_init (machine=0x55c4bdd46000) 
> at /source/qemu/hw/core/machine.c:1030
>   #8  0x55c4bc95f6d2 in main (argc=, argv=, 
> envp=) at /source/qemu/vl.c:4479
> 
> Since the caller knows the type name requested, we can simply display it
> to ease development.
> 
> With this patch applied we get:
> 
>   $ qemu-system-microblaze -M xlnx-zynqmp-pmu
>   qemu-system-microblaze: missing object type 'xlnx.zynqmp_ipi'
>   Aborted (core dumped)
> 
> Since the assert(type) check in object_initialize_with_type() is
> now impossible, remove it.
> 
> Signed-off-by: Philippe Mathieu-Daudé 

Queued, thanks!

-- 
Eduardo

Re: [Qemu-devel] [PATCH 1/3] ram-encrypted-notifier: Introduce a RAM block encrypted notifier

[Eduardo Habkost]

On Fri, Apr 26, 2019 at 02:32:51PM +, Janakarajan Natarajan wrote:
> On 4/26/19 7:29 AM, Igor Mammedov wrote:
[...]
> >> diff --git a/numa.c b/numa.c
> >> index 3875e1efda..08601366c5 100644
> >> --- a/numa.c
> >> +++ b/numa.c
> > looks like wrong file to put RAMBlock code in. I though that we should put 
> > it in exec.c
> 
> 
> I placed the RAMBlockEncrypted Notifier code along with the RAMBlock 
> Notifier code.

Paolo, Fam, do you remember why was the ram block notifier code
added to numa.c instead of memory.c or exec.c?

-- 
Eduardo

Re: [Qemu-devel] [PATCH v3 36/40] s390x/tcg: Implement VECTOR SUBTRACT WITH BORROW COMPUTE BORROW INDICATION

[Richard Henderson]

On 5/2/19 7:10 AM, David Hildenbrand wrote:
> Mostly courtesy of Richard H.
> 
> Signed-off-by: David Hildenbrand 
> ---
>  target/s390x/insn-data.def  |  2 ++
>  target/s390x/translate_vx.inc.c | 34 +
>  2 files changed, 36 insertions(+)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] Following up questions related to QEMU and I/O Thread

[Wei Li]

Hi Paolo,

That will be great, I would like to hear more details about the design and 
implementation once you get those ready. 

Thanks a lot,
Wei

On 5/3/19, 11:05 AM, "Paolo Bonzini"  wrote:

On 03/05/19 10:21, Wei Li wrote:
> Got it, thanks Stefan for your clarification!

Hi Wei,

Stefan and I should be posting a patch to add Linux SCSI driver
batching, and an implementation for virtio-scsi.

Paolo

> Wei
> 
> On 5/1/19, 9:36 AM, "Stefan Hajnoczi"  wrote:
> 
> On Mon, Apr 29, 2019 at 10:56:31AM -0700, Wei Li wrote:
> >Does this mean the performance could be improved via adding Batch 
I/O submission support in Guest driver side which will be able to reduce the 
number of virtqueue kicks?
> 
> Yes, I think so.  It's not obvious to me how a Linux SCSI driver is
> supposed to implement batching though.  The .queuecommand API doesn't
> seem to include information relevant to batching.
> 
> Stefan
> 
> 
> 
> 

Re: [Qemu-devel] Following up questions related to QEMU and I/O Thread

[Paolo Bonzini]

On 03/05/19 10:21, Wei Li wrote:
> Got it, thanks Stefan for your clarification!

Hi Wei,

Stefan and I should be posting a patch to add Linux SCSI driver
batching, and an implementation for virtio-scsi.

Paolo

> Wei
> 
> On 5/1/19, 9:36 AM, "Stefan Hajnoczi"  wrote:
> 
> On Mon, Apr 29, 2019 at 10:56:31AM -0700, Wei Li wrote:
> >Does this mean the performance could be improved via adding Batch I/O 
> submission support in Guest driver side which will be able to reduce the 
> number of virtqueue kicks?
> 
> Yes, I think so.  It's not obvious to me how a Linux SCSI driver is
> supposed to implement batching though.  The .queuecommand API doesn't
> seem to include information relevant to batching.
> 
> Stefan
> 
> 
> 
> 

[Qemu-devel] [PATCH v4 3/5] linux-user: Add support the SIOCIFPFLAGS ioctls for all targets

[Aleksandar Markovic]

From: Neng Chen 

Add support for getting and setting extended private flags of a
network device via SIOCSIFPFLAGS and SIOCGIFPFLAGS ioctls.

The ioctl numeric values are platform-independent and determined by
the file include/uapi/linux/sockios.h in Linux kernel source code:

  #define SIOCSIFPFLAGS 0x8934
  #define SIOCGIFPFLAGS 0x8935

These ioctls get (or set) the field ifr_flags of type short in the
structure ifreq. Such functionality is achieved in QEMU by using
MK_STRUCT() and MK_PTR() macros with an appropriate argument, as
it was done for existing similar cases.

Signed-off-by: Neng Chen 
Signed-off-by: Aleksandar Markovic 
Message-Id: <1554839486-3527-1-git-send-email-aleksandar.marko...@rt-rk.com>
---
 linux-user/ioctls.h   | 2 ++
 linux-user/syscall_defs.h | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/linux-user/ioctls.h b/linux-user/ioctls.h
index c37adc5..76375df 100644
--- a/linux-user/ioctls.h
+++ b/linux-user/ioctls.h
@@ -206,6 +206,8 @@
   IOCTL(SIOCADDMULTI, IOC_W, MK_PTR(MK_STRUCT(STRUCT_sockaddr_ifreq)))
   IOCTL(SIOCDELMULTI, IOC_W, MK_PTR(MK_STRUCT(STRUCT_sockaddr_ifreq)))
   IOCTL(SIOCGIFINDEX, IOC_W | IOC_R, MK_PTR(MK_STRUCT(STRUCT_int_ifreq)))
+  IOCTL(SIOCSIFPFLAGS, IOC_W, MK_PTR(MK_STRUCT(STRUCT_short_ifreq)))
+  IOCTL(SIOCGIFPFLAGS, IOC_W | IOC_R, MK_PTR(MK_STRUCT(STRUCT_short_ifreq)))
   IOCTL(SIOCSIFLINK, 0, TYPE_NULL)
   IOCTL_SPECIAL(SIOCGIFCONF, IOC_W | IOC_R, do_ioctl_ifconf,
 MK_PTR(MK_STRUCT(STRUCT_ifconf)))
diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index 2941231..8904d35 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -781,6 +781,8 @@ struct target_pollfd {
 #define TARGET_SIOCADDMULTI0x8931  /* Multicast address lists  
*/
 #define TARGET_SIOCDELMULTI0x8932
 #define TARGET_SIOCGIFINDEX0x8933
+#define TARGET_SIOCSIFPFLAGS   0x8934  /* set extended flags  
*/
+#define TARGET_SIOCGIFPFLAGS   0x8935  /* get extended flags  
*/
 
 /* Bridging control calls */
 #define TARGET_SIOCGIFBR   0x8940  /* Bridging support 
*/
-- 
2.7.4

[Qemu-devel] [PATCH v4 4/5] linux-user: Add support for setsockopt() options IPV6__MEMBERSHIP

[Aleksandar Markovic]

From: Neng Chen 

Add support for options IPV6_ADD_MEMBERSHIP and IPV6_DROP_MEMPEMBERSHIP
of the syscall setsockopt(). These options control membership in
multicast groups. Their argument is a pointer to a struct ipv6_mreq,
which is in turn defined as:

struct ipv6_mreq {
/* IPv6 multicast address of group */
struct in6_addr  ipv6mr_multiaddr;
/* local IPv6 address of interface */
int  ipv6mr_interface;
};

The in6_addr structure consists of fields that are always big-endian
(on host of any endian), so the ipv6_mreq's field ipv6mr_multiaddr
doesn't need any endian conversion, whereas ipv6mr_interface does.

Signed-off-by: Neng Chen 
Signed-off-by: Aleksandar Markovic 
---
 linux-user/syscall.c | 19 +++
 1 file changed, 19 insertions(+)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 96cd4bf..b7eb4b7 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -1892,6 +1892,25 @@ static abi_long do_setsockopt(int sockfd, int level, int 
optname,
, sizeof(pki)));
 break;
 }
+case IPV6_ADD_MEMBERSHIP:
+case IPV6_DROP_MEMBERSHIP:
+{
+struct ipv6_mreq ipv6mreq;
+
+if (optlen < sizeof(ipv6mreq)) {
+return -TARGET_EINVAL;
+}
+
+if (copy_from_user(, optval_addr, sizeof(ipv6mreq))) {
+return -TARGET_EFAULT;
+}
+
+ipv6mreq.ipv6mr_interface = tswap32(ipv6mreq.ipv6mr_interface);
+
+ret = get_errno(setsockopt(sockfd, level, optname,
+   , sizeof(ipv6mreq)));
+break;
+}
 default:
 goto unimplemented;
 }
-- 
2.7.4

[Qemu-devel] [PATCH v4 5/5] linux-user: Sanitize interp_info and, for mips only, init field fp_abi

[Aleksandar Markovic]

From: Daniel Santos 

Sanitize interp_info structure in load_elf_binary() and, for MIPS only,
init its field fp_abi to MIPS_ABI_FP_UNKNOWN. This fixes appearances of
"Unexpected FPU mode" message in some MIPS use cases. Currently, this
bug is a complete stopper for some MIPS binaries.

In load_elf_binary(), struct image_info interp_info is used without
being properly initialized. One result is that when the ELF's program
header doesn't contain an entry for the ABI flags, then the value of
the struct image_info's fp_abi field is set to whatever happened to
be in stack memory at the time.

Backporting to 4.0 and, if possible, to 3.1 is recommended.

Fixes: https://bugs.launchpad.net/qemu/+bug/1825002

Signed-off-by: Daniel Santos 
Signed-off-by: Aleksandar Markovic 
Reviewed-by: Philippe Mathieu-Daudé 
---
 linux-user/elfload.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index c1a2602..7f09d57 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -2698,6 +2698,11 @@ int load_elf_binary(struct linux_binprm *bprm, struct 
image_info *info)
 char *elf_interpreter = NULL;
 char *scratch;
 
+memset(_info, 0, sizeof(interp_info));
+#ifdef TARGET_MIPS
+interp_info.fp_abi = MIPS_ABI_FP_UNKNOWN;
+#endif
+
 info->start_mmap = (abi_ulong)ELF_START_MMAP;
 
 load_elf_image(bprm->filename, bprm->fd, info,
-- 
2.7.4

[Qemu-devel] [PATCH v4 1/5] linux-user: Fix support for the SIOCATMARK and SIOCGPGRP ioctls for xtensa

[Aleksandar Markovic]

From: Aleksandar Markovic 

Fix support for the SIOCATMARK and SIOCGPGRP ioctls for xtensa by
correcting corresponding macro definition.

Values for TARGET_SIOCATMARK and TARGET_SIOCGPGRP are determined by
Linux kernel. Following relevant lines (obtained by grep) are from
the kernel source tree:

arch/ia64/include/uapi/asm/sockios.h:#define SIOCATMARK0x8905
arch/mips/include/uapi/asm/sockios.h:#define SIOCATMARK_IOR('s', 7, int)
arch/parisc/include/uapi/asm/sockios.h:#define SIOCATMARK  0x8905
arch/sh/include/uapi/asm/sockios.h:#define SIOCATMARK  _IOR('s', 7, int)
arch/xtensa/include/uapi/asm/sockios.h:#define SIOCATMARK  _IOR('s', 7, int)
arch/alpha/include/uapi/asm/sockios.h:#define SIOCATMARK   _IOR('s', 7, int)
arch/sparc/include/uapi/asm/sockios.h:#define SIOCATMARK   0x8905
include/uapi/asm-generic/sockios.h:#define SIOCATMARK  0x8905

arch/ia64/include/uapi/asm/sockios.h:#define SIOCGPGRP 0x8904
arch/mips/include/uapi/asm/sockios.h:#define SIOCGPGRP _IOR('s', 9, pid_t)
arch/parisc/include/uapi/asm/sockios.h:#define SIOCGPGRP   0x8904
arch/sh/include/uapi/asm/sockios.h:#define SIOCGPGRP   _IOR('s', 9, pid_t)
arch/xtensa/include/uapi/asm/sockios.h:#define SIOCGPGRP   _IOR('s', 9, pid_t)
arch/alpha/include/uapi/asm/sockios.h:#define SIOCGPGRP_IOR('s', 9, pid_t)
arch/sparc/include/uapi/asm/sockios.h:#define SIOCGPGRP0x8904
include/uapi/asm-generic/sockios.h:#define SIOCGPGRP   0x8904

It is visible from above that xtensa should have the same definitions
as alpha, mips and sh4 already do. This patch brings QEMU to the accurate
state wrt these two ioctls.

Acked-by: Max Filippov 
Signed-off-by: Aleksandar Markovic 
---
 linux-user/syscall_defs.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index 12c8407..1e86fb9 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -736,7 +736,8 @@ struct target_pollfd {
 #define TARGET_KDSETLED0x4B32  /* set led state [lights, not flags] */
 #define TARGET_KDSIGACCEPT 0x4B4E
 
-#if defined(TARGET_ALPHA) || defined(TARGET_MIPS) || defined(TARGET_SH4)
+#if defined(TARGET_ALPHA) || defined(TARGET_MIPS) || defined(TARGET_SH4) ||
\
+   defined(TARGET_XTENSA)
 #define TARGET_SIOCATMARK  TARGET_IOR('s', 7, int)
 #define TARGET_SIOCGPGRP   TARGET_IOR('s', 9, pid_t)
 #else
-- 
2.7.4

[Qemu-devel] [PATCH v4 2/5] linux-user: Add support for the SIOCSPGRP ioctl for all targets

[Aleksandar Markovic]

From: Aleksandar Markovic 

Add support for setting the process (or process group) to receive SIGIO
or SIGURG signals when I/O becomes possible or urgent data is available,
using SIOCSPGRP ioctl.

The ioctl numeric values for SIOCSPGRP are platform-dependent and are
determined by following files in Linux kernel source tree:

arch/ia64/include/uapi/asm/sockios.h:#define SIOCSPGRP0x8902
arch/mips/include/uapi/asm/sockios.h:#define SIOCSPGRP_IOW('s', 8, pid_t)
arch/parisc/include/uapi/asm/sockios.h:#define SIOCSPGRP  0x8902
arch/sh/include/uapi/asm/sockios.h:#define SIOCSPGRP  _IOW('s', 8, pid_t)
arch/xtensa/include/uapi/asm/sockios.h:#define SIOCSPGRP  _IOW('s', 8, pid_t)
arch/alpha/include/uapi/asm/sockios.h:#define SIOCSPGRP   _IOW('s', 8, pid_t)
arch/sparc/include/uapi/asm/sockios.h:#define SIOCSPGRP   0x8902
include/uapi/asm-generic/sockios.h:#define SIOCSPGRP  0x8902

Hence the different definition for alpha, mips, sh4, and xtensa.

Reviewed-by: Max Filippov 
Signed-off-by: Aleksandar Markovic 
---
 linux-user/ioctls.h   | 1 +
 linux-user/syscall_defs.h | 3 +++
 2 files changed, 4 insertions(+)

diff --git a/linux-user/ioctls.h b/linux-user/ioctls.h
index ae89516..c37adc5 100644
--- a/linux-user/ioctls.h
+++ b/linux-user/ioctls.h
@@ -218,6 +218,7 @@
   IOCTL(SIOCSRARP, IOC_W, MK_PTR(MK_STRUCT(STRUCT_arpreq)))
   IOCTL(SIOCGRARP, IOC_R, MK_PTR(MK_STRUCT(STRUCT_arpreq)))
   IOCTL(SIOCGIWNAME, IOC_W | IOC_R, MK_PTR(MK_STRUCT(STRUCT_char_ifreq)))
+  IOCTL(SIOCSPGRP, IOC_W, MK_PTR(TYPE_INT)) /* pid_t */
   IOCTL(SIOCGPGRP, IOC_R, MK_PTR(TYPE_INT)) /* pid_t */
   IOCTL(SIOCGSTAMP, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timeval)))
   IOCTL(SIOCGSTAMPNS, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timespec)))
diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index 1e86fb9..2941231 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -739,11 +739,14 @@ struct target_pollfd {
 #if defined(TARGET_ALPHA) || defined(TARGET_MIPS) || defined(TARGET_SH4) ||
\
defined(TARGET_XTENSA)
 #define TARGET_SIOCATMARK  TARGET_IOR('s', 7, int)
+#define TARGET_SIOCSPGRP   TARGET_IOW('s', 8, pid_t)
 #define TARGET_SIOCGPGRP   TARGET_IOR('s', 9, pid_t)
 #else
 #define TARGET_SIOCATMARK  0x8905
+#define TARGET_SIOCSPGRP   0x8902
 #define TARGET_SIOCGPGRP   0x8904
 #endif
+
 #define TARGET_SIOCGSTAMP  0x8906  /* Get stamp (timeval) */
 #define TARGET_SIOCGSTAMPNS0x8907  /* Get stamp (timespec) */
 
-- 
2.7.4

[Qemu-devel] [PATCH v4 0/5] linux-user: A set of miscellaneous patches

[Aleksandar Markovic]

From: Aleksandar Markovic 

This is a collection of misc patches for Linux user that I recently
accumulated from variuous sources. All of them originate from problems
observed on mips target. However, these changes actually affect and fix
problems on multiple targets.

v3->v4:

  - improved commit messages (fixed some typos, improved relevance)

v2->v3:

  - updated and improved commit messages
  - added IPV6_DROP_MEMBERSHIP support to the patch on setsockopt()'s
option

v1->v2:

  - added the patch on setsockopt()'s option IPV6_ADD_MEMBERSHIP
  - improved the commit message of interp_info sanitizing patch

Aleksandar Markovic (2):
  linux-user: Fix support for the SIOCATMARK and SIOCGPGRP ioctls for
xtensa
  linux-user: Add support for the SIOCSPGRP ioctl for all targets

Daniel Santos (1):
  linux-user: Sanitize interp_info and, for mips only, init field fp_abi

Neng Chen (2):
  linux-user: Add support the SIOCIFPFLAGS ioctls for all targets
  linux-user: Add support for setsockopt() options
IPV6__MEMBERSHIP

 linux-user/elfload.c  |  5 +
 linux-user/ioctls.h   |  3 +++
 linux-user/syscall.c  | 19 +++
 linux-user/syscall_defs.h |  8 +++-
 4 files changed, 34 insertions(+), 1 deletion(-)

-- 
2.7.4

[Qemu-devel] [Bug 1817345] Re: configure script breaks when $source_path contains white spaces

[Peter Maydell]

Antonio has submitted a patchset here:
https://patchew.org/QEMU/20190503082728.16485-1-...@ao2.it/


** Changed in: qemu
   Status: New => In Progress

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

Title:
  configure script breaks when $source_path contains white spaces

Status in QEMU:
  In Progress

Bug description:
  Hi,

  I noticed that the configure script breaks when the qemu source
  directory is in a path containing white spaces, in particular the list
  of targets is not correctly generated when calling "./configure
  --help".

  Steps to reproduce the problem:

  $ mkdir "dir with spaces"
  $ cd dir\ with\ spaces/
  $ git clone https://git.qemu.org/git/qemu.git
  $ cd qemu/
  $ ./configure --help | grep -A3 target-list

  
  Actual result:

--target-list=LIST   set target list (default: build everything)
 Available targets: dir with *-softmmu dir with 
 *-linux-user

  
  Expected result:

--target-list=LIST   set target list (default: build everything)
 Available targets: aarch64-softmmu alpha-softmmu 
 arm-softmmu cris-softmmu hppa-softmmu i386-softmmu 
 lm32-softmmu m68k-softmmu microblaze-softmmu 

  
  This happens because the $mak_wilds variable uses spaces to separate 
different paths, maybe newlines may be used, which are less likely to be in 
directory names.

  BTW "shellcheck" may help finding some other problems.

  Qemu version:

  $ git describe 
  v3.1.0-1960-ga05838cb2a

  Thanks,
 Antonio

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

Re: [Qemu-devel] [PATCH 1/2] hw/arm/boot: Avoid placing the initrd on top of the kernel

[Peter Maydell]

On Fri, 3 May 2019 at 18:13, Peter Maydell  wrote:
>
> We currently put the initrd at the smaller of:
>  * 128MB into RAM
>  * halfway into the RAM
> (with the dtb following it).
>
> However for large kernels this might mean that the kernel
> overlaps the initrd. For some kinds of kernel (self-decompressing
> 32-bit kernels, and ELF images with a BSS section at the end)
> we don't know the exact size, but even there we have a
> minimum size. Put the initrd at least further into RAM than
> that. For image formats that can give us an exact kernel size, this
> will mean that we definitely avoid overlaying kernel and initrd.
>
> Signed-off-by: Peter Maydell 
> ---
>  hw/arm/boot.c | 34 --
>  1 file changed, 20 insertions(+), 14 deletions(-)
>
> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
> index a830655e1af..7c978fedde4 100644
> --- a/hw/arm/boot.c
> +++ b/hw/arm/boot.c
> @@ -998,20 +998,6 @@ static void arm_setup_direct_kernel_boot(ARMCPU *cpu,
>  if (info->nb_cpus == 0)
>  info->nb_cpus = 1;
>
> -/*
> - * We want to put the initrd far enough into RAM that when the
> - * kernel is uncompressed it will not clobber the initrd. However
> - * on boards without much RAM we must ensure that we still leave
> - * enough room for a decent sized initrd, and on boards with large
> - * amounts of RAM we must avoid the initrd being so far up in RAM
> - * that it is outside lowmem and inaccessible to the kernel.
> - * So for boards with less  than 256MB of RAM we put the initrd
> - * halfway into RAM, and for boards with 256MB of RAM or more we put
> - * the initrd at 128MB.
> - */
> -info->initrd_start = info->loader_start +
> -MIN(info->ram_size / 2, 128 * 1024 * 1024);
> -
>  /* Assume that raw images are linux kernels, and ELF images are not.  */
>  kernel_size = arm_load_elf(info, _entry, _low_addr,
> _high_addr, elf_machine, as);
> @@ -1056,6 +1042,26 @@ static void arm_setup_direct_kernel_boot(ARMCPU *cpu,
>  exit(1);
>  }
>  info->entry = entry;
> +
> +/*
> + * We want to put the initrd far enough into RAM that when the
> + * kernel is uncompressed it will not clobber the initrd. However
> + * on boards without much RAM we must ensure that we still leave
> + * enough room for a decent sized initrd, and on boards with large
> + * amounts of RAM we must avoid the initrd being so far up in RAM
> + * that it is outside lowmem and inaccessible to the kernel.
> + * So for boards with less  than 256MB of RAM we put the initrd
> + * halfway into RAM, and for boards with 256MB of RAM or more we put
> + * the initrd at 128MB.
> + * We also refuse to put the initrd somewhere that will definitely
> + * overlay the kernel we just loaded, though for kernel formats which
> + * don't tell us their exact size (eg self-decompressing 32-bit kernels)
> + * we might still make a bad choice here.
> + */
> +info->initrd_start = info->loader_start +
> +MAX(MIN(info->ram_size / 2, 128 * 1024 * 1024), kernel_size);
> +info->initrd_start = TARGET_PAGE_ALIGN(info->initrd_start);
> +

I belatedly realized that we should probably check here whether
this is off the end of the ram, as otherwise following code
will get an underflow in "info->ram_size - info->initrd_start"
which is unlikely to result in useful behaviour.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2] security.rst: add Security Guide to developer docs

[Stefan Hajnoczi]

On Fri, May 03, 2019 at 10:04:10AM +0100, Alex Bennée wrote:
> Stefan Hajnoczi  writes:
> > +Isolation mechanisms
> > +
> > +Several isolation mechanisms are available to realize this architecture of
> > +guest isolation and the principle of least privilege.  With the exception 
> > of
> > +Linux seccomp, these mechanisms are all deployed by management tools that
> > +launch QEMU, such as libvirt.  They are also platform-specific so they are 
> > only
> > +described briefly for Linux here.
> > +
> > +The fundamental isolation mechanism is that QEMU processes must run as
> > +**unprivileged users**.  Sometimes it seems more convenient to launch QEMU 
> > as
> > +root to give it access to host devices (e.g. ``/dev/net/tun``) but this 
> > poses a
> > +huge security risk.  File descriptor passing can be used to give an 
> > otherwise
> > +unprivileged QEMU process access to host devices without running QEMU
> > as root.
> 
> Should we mention that you can still maintain running as a user and just
> make the devices you need available to the user/group rather than
> becoming root? For example I generally make /dev/kvm group accessible to
> my user account.

Sure.  I checked that /dev/vhost-* device nodes are root:root on Fedora
so at least the distro doesn't expect you to do that.  The /dev/kvm
device node is root:kvm so it's easy to do it by joining the kvm group
there.

Stefan


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v2] security.rst: add Security Guide to developer docs

[Stefan Hajnoczi]

On Fri, May 03, 2019 at 11:35:29AM +0100, Daniel P. Berrangé wrote:
> On Fri, May 03, 2019 at 11:28:53AM +0100, Peter Maydell wrote:
> > On Fri, 3 May 2019 at 11:19, Daniel P. Berrangé  wrote:
> > > Everything above here is useful to QEMU devs, app devs & end users and
> > > should be made part of the main QEMU doc - convert it to texi and @include
> > > it from qemu-doc.texi, as we do for other stuff under docs/
> > 
> > If we convert it to texi we'll have to convert it back again
> > as/when we migrate properly from texi to sphinx... (I would
> > like to make further moves in that direction during this
> > release cycle -- just need to find the time to work on it.)
> 
> Yes, but we're only talking about 100-150 lines of simple text with
> minimal markup needs. Won't be a noticable extra burden compared to
> the pre-existing 4700 lines of texi markup for qemu-doc.texi and its
> includes.

I'm happy to split as suggested and do it in texi for now.

I am also happy to convert the file back to rst again later.

Stefan


signature.asc
Description: PGP signature

[Qemu-devel] [Bug 1823998] Re: qemu-system-aarch64: support kernels bigger than 128MiB

[Peter Maydell]

I've submitted a patchset which I think should fix this, but if you
could test that it actually does handle large images correctly that
would be great.

https://patchew.org/QEMU/20190503171347.13747-1-peter.mayd...@linaro.org/


** Changed in: qemu
   Status: New => In Progress

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

Title:
  qemu-system-aarch64: support kernels bigger than 128MiB

Status in QEMU:
  In Progress

Bug description:
  Presently QEMU reserves up to 128MiB of space for an arm64 Linux
  kernel, placing the initrd following this, and the dtb following the
  initrd.

  This is not sufficient for some debug configurations of the kernel,
  which can be larger than 128MiB. Depending on the relative size of the
  kernel Image and unpopulated BSS, the dtb (or kernel) will be
  clobbered by the other, resulting in a silent boot failure.

  Since v3.17, the kernel Image header exposes a field called
  image_size, which describes the entire size of the kernel (including
  unpopulated sections such as the BSS) as a 64-bit little-endian value.
  For kernels prior to v3.17, this field is zero. This is documented at:

  
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/arm64/booting.txt?h=v5.0#n68

  It would be great if QEMU could take the image_size field into account
  when placing the initrd and dtb to avoid overlap with the kernel.

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

Re: [Qemu-devel] [PATCH] target/arm: Implement NSACR gating of floating point

[Richard Henderson]

On 5/3/19 6:13 AM, Peter Maydell wrote:
> On Sat, 13 Apr 2019 at 08:07, Richard Henderson
>> This one does do the right thing, but better to clear the bits on write to
>> NSACR.  This lets you avoid the change to fp_exception_el, and the missing
>> change to sve_exception_el.
> 
> Hi Richard -- I was just going through the review comments on this
> patchset, and I saw this bit. Could you clarify what you mean by
> "the missing change to sve_exception_el" ? Since SVE is AArch64 only,
> there can't be any configs where we have SVE and EL3 is AArch32,
> so I don't think these two features should be able to interact.

You're right.  I'm going to assume I had been insufficiently caffeinated at the
time.


r~

Re: [Qemu-devel] [PATCH v2] block/rbd: increase dynamically the image size

[Jason Dillaman]

On Fri, May 3, 2019 at 12:30 PM Stefano Garzarella  wrote:
>
> RBD APIs don't allow us to write more than the size set with
> rbd_create() or rbd_resize().
> In order to support growing images (eg. qcow2), we resize the
> image before write operations that exceed the current size.
>
> Signed-off-by: Stefano Garzarella 
> ---
> v2:
>   - use bs->total_sectors instead of adding a new field [Kevin]
>   - resize the image only during write operation [Kevin]
> for read operation, the bdrv_aligned_preadv() already handles reads
> that exceed the length returned by bdrv_getlength(), so IMHO we can
> avoid to handle it in the rbd driver
> ---
>  block/rbd.c | 14 +-
>  1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/block/rbd.c b/block/rbd.c
> index 0c549c9935..613e8f4982 100644
> --- a/block/rbd.c
> +++ b/block/rbd.c
> @@ -934,13 +934,25 @@ static BlockAIOCB *rbd_start_aio(BlockDriverState *bs,
>  }
>
>  switch (cmd) {
> -case RBD_AIO_WRITE:
> +case RBD_AIO_WRITE: {
> +/*
> + * RBD APIs don't allow us to write more than actual size, so in 
> order
> + * to support growing images, we resize the image before write
> + * operations that exceed the current size.
> + */
> +if (off + size > bs->total_sectors * BDRV_SECTOR_SIZE) {

When will "bs->total_sectors" be refreshed to represent the correct
current size? You wouldn't want a future write whose extent was
greater than the original image size but less then a previous IO that
expanded the image to attempt to shrink the image.

> +r = rbd_resize(s->image, off + size);
> +if (r < 0) {
> +goto failed_completion;
> +}
> +}
>  #ifdef LIBRBD_SUPPORTS_IOVEC
>  r = rbd_aio_writev(s->image, qiov->iov, qiov->niov, off, c);
>  #else
>  r = rbd_aio_write(s->image, off, size, rcb->buf, c);
>  #endif
>  break;
> +}
>  case RBD_AIO_READ:
>  #ifdef LIBRBD_SUPPORTS_IOVEC
>  r = rbd_aio_readv(s->image, qiov->iov, qiov->niov, off, c);
> --
> 2.20.1
>
>


-- 
Jason

[Qemu-devel] [PATCH v3 1/5] linux-user: Fix support for the SIOCATMARK and SIOCGPGRP ioctls for xtensa

[Aleksandar Markovic]

From: Aleksandar Markovic 

Fix support for the SIOCATMARK and SIOCGPGRP ioctls for xtensa by
correcting corresponding macro definition.

Values for TARGET_SIOCATMARK and TARGET_SIOCGPGRP are determined by
Linux kernel. Following relevant lines are from kernel source tree:

arch/ia64/include/uapi/asm/sockios.h:#define SIOCATMARK0x8905
arch/mips/include/uapi/asm/sockios.h:#define SIOCATMARK_IOR('s', 7, int)
arch/parisc/include/uapi/asm/sockios.h:#define SIOCATMARK  0x8905
arch/sh/include/uapi/asm/sockios.h:#define SIOCATMARK  _IOR('s', 7, int)
arch/xtensa/include/uapi/asm/sockios.h:#define SIOCATMARK  _IOR('s', 7, int)
arch/alpha/include/uapi/asm/sockios.h:#define SIOCATMARK   _IOR('s', 7, int)
arch/sparc/include/uapi/asm/sockios.h:#define SIOCATMARK   0x8905
include/uapi/asm-generic/sockios.h:#define SIOCATMARK  0x8905

arch/ia64/include/uapi/asm/sockios.h:#define SIOCGPGRP 0x8904
arch/mips/include/uapi/asm/sockios.h:#define SIOCGPGRP _IOR('s', 9, pid_t)
arch/parisc/include/uapi/asm/sockios.h:#define SIOCGPGRP   0x8904
arch/sh/include/uapi/asm/sockios.h:#define SIOCGPGRP   _IOR('s', 9, pid_t)
arch/xtensa/include/uapi/asm/sockios.h:#define SIOCGPGRP   _IOR('s', 9, pid_t)
arch/alpha/include/uapi/asm/sockios.h:#define SIOCGPGRP_IOR('s', 9, pid_t)
arch/sparc/include/uapi/asm/sockios.h:#define SIOCGPGRP0x8904
include/uapi/asm-generic/sockios.h:#define SIOCGPGRP   0x8904

It is visible that xtensa should have the same definitions as
alpha, mips and sh4 already do. This patch brings that to the
accurate state.

Acked-by: Max Filippov 
Signed-off-by: Aleksandar Markovic 
---
 linux-user/syscall_defs.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index 12c8407..1e86fb9 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -736,7 +736,8 @@ struct target_pollfd {
 #define TARGET_KDSETLED0x4B32  /* set led state [lights, not flags] */
 #define TARGET_KDSIGACCEPT 0x4B4E
 
-#if defined(TARGET_ALPHA) || defined(TARGET_MIPS) || defined(TARGET_SH4)
+#if defined(TARGET_ALPHA) || defined(TARGET_MIPS) || defined(TARGET_SH4) ||
\
+   defined(TARGET_XTENSA)
 #define TARGET_SIOCATMARK  TARGET_IOR('s', 7, int)
 #define TARGET_SIOCGPGRP   TARGET_IOR('s', 9, pid_t)
 #else
-- 
2.7.4

[Qemu-devel] [PATCH 2/2] hw/arm/boot: Honour image size field in AArch64 Image format kernels

[Peter Maydell]

Since Linux v3.17, the kernel's Image header includes a field image_size,
which gives the total size of the kernel including unpopulated data
sections such as the BSS). If this is present, then return it from
load_aarch64_image() as the true size of the kernel rather than
just using the size of the Image file itself. This allows the code
which calculates where to put the initrd to avoid putting it in
the kernel's BSS area.

This means that we should be able to reliably load kernel images
which are larger than 128MB without accidentally putting the
initrd or dtb in locations that clash with the kernel itself.

Fixes: https://bugs.launchpad.net/qemu/+bug/1823998
Signed-off-by: Peter Maydell 
---
 hw/arm/boot.c | 17 +++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index 7c978fedde4..34bdd151df8 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -910,6 +910,7 @@ static uint64_t load_aarch64_image(const char *filename, 
hwaddr mem_base,
hwaddr *entry, AddressSpace *as)
 {
 hwaddr kernel_load_offset = KERNEL64_LOAD_ADDR;
+uint64_t kernel_size = 0;
 uint8_t *buffer;
 int size;
 
@@ -937,7 +938,10 @@ static uint64_t load_aarch64_image(const char *filename, 
hwaddr mem_base,
  * is only valid if the image_size is non-zero.
  */
 memcpy(, buffer + ARM64_TEXT_OFFSET_OFFSET, sizeof(hdrvals));
-if (hdrvals[1] != 0) {
+
+kernel_size = le64_to_cpu(hdrvals[1]);
+
+if (kernel_size != 0) {
 kernel_load_offset = le64_to_cpu(hdrvals[0]);
 
 /*
@@ -955,12 +959,21 @@ static uint64_t load_aarch64_image(const char *filename, 
hwaddr mem_base,
 }
 }
 
+/*
+ * Kernels before v3.17 don't populate the image_size field, and
+ * raw images have no header. For those our best guess at the size
+ * is the size of the Image file itself.
+ */
+if (kernel_size == 0) {
+kernel_size = size;
+}
+
 *entry = mem_base + kernel_load_offset;
 rom_add_blob_fixed_as(filename, buffer, size, *entry, as);
 
 g_free(buffer);
 
-return size;
+return kernel_size;
 }
 
 static void arm_setup_direct_kernel_boot(ARMCPU *cpu,
-- 
2.20.1

Re: [Qemu-devel] [PATCH v4] hw/virtio/virtio-mmio: Convert DPRINTF to trace and log

[LI, BO XUAN]

Gotcha, thanks for the tip!

Best regards,
Boxuan Li

On Sat, May 4, 2019 at 1:00 AM Alex Bennée  wrote:

>
> LI, BO XUAN  writes:
>
> > Hi Alex,
> >
> > Sorry about that, I am still trying to get familiar with the patch
> > submission process. Since my patch has been changed from your last
> review,
> > I thought it would be safe to not include the r-b tag from last time.
> Will
> > take care next time!
>
> That's ok. As a general rule as long as you haven't substantially
> changed a patch it's safe to keep previous r-b tags. You can always
> mention it in your cover letter if you are unsure.
>
> --
> Alex Bennée
>

[Qemu-devel] [PATCH 1/2] blockjob: Fix coroutine thread after AioContext change

[Kevin Wolf]

Commit 463e0be10 ('blockjob: add AioContext attached callback') tried to
make block jobs robust against AioContext changes of their main node,
but it never made sure that the job coroutine actually runs in the new
thread.

Instead of waking up the job coroutine in whatever thread it ran before,
let's always pass the AioContext where it should be running now.

Signed-off-by: Kevin Wolf 
---
 job.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/job.c b/job.c
index da8e4b7bf2..2167d53717 100644
--- a/job.c
+++ b/job.c
@@ -432,7 +432,7 @@ void job_enter_cond(Job *job, bool(*fn)(Job *job))
 timer_del(>sleep_timer);
 job->busy = true;
 job_unlock();
-aio_co_wake(job->co);
+aio_co_enter(job->aio_context, job->co);
 }
 
 void job_enter(Job *job)
-- 
2.20.1

[Qemu-devel] [PATCH v3 4/5] linux-user: Add support for setsockopt() options IPV6__MEMBERSHIP

[Aleksandar Markovic]

From: Neng Chen 

Add support for options IPV6_ADD_MEMBERSHIP and IPV6_ADD_MEMBERSHIP
of the syscall setsockopt(). These options control membership in
multicast groups. Their argument is a pointer to a struct ipv6_mreq,
which is in turn defined as:

struct ipv6_mreq {
/* IPv6 multicast address of group */
struct in6_addr  ipv6mr_multiaddr;
/* local IPv6 address of interface */
int  ipv6mr_interface;
};

The in6_addr structure consists of fields that are always big-endian
(on any host), so the ipv6_mreq's field ipv6mr_multiaddr doesn't need
any endian conversion, whereas ipv6mr_interface does.

Signed-off-by: Neng Chen 
Signed-off-by: Aleksandar Markovic 
---
 linux-user/syscall.c | 19 +++
 1 file changed, 19 insertions(+)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 96cd4bf..b7eb4b7 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -1892,6 +1892,25 @@ static abi_long do_setsockopt(int sockfd, int level, int 
optname,
, sizeof(pki)));
 break;
 }
+case IPV6_ADD_MEMBERSHIP:
+case IPV6_DROP_MEMBERSHIP:
+{
+struct ipv6_mreq ipv6mreq;
+
+if (optlen < sizeof(ipv6mreq)) {
+return -TARGET_EINVAL;
+}
+
+if (copy_from_user(, optval_addr, sizeof(ipv6mreq))) {
+return -TARGET_EFAULT;
+}
+
+ipv6mreq.ipv6mr_interface = tswap32(ipv6mreq.ipv6mr_interface);
+
+ret = get_errno(setsockopt(sockfd, level, optname,
+   , sizeof(ipv6mreq)));
+break;
+}
 default:
 goto unimplemented;
 }
-- 
2.7.4

[Qemu-devel] [PATCH 0/2] blockjob: Fix coroutine thread after AioContext change

[Kevin Wolf]

Kevin Wolf (2):
  blockjob: Fix coroutine thread after AioContext change
  test-block-iothread: Job coroutine thread after AioContext switch

 job.c   |   2 +-
 tests/test-block-iothread.c | 107 
 2 files changed, 108 insertions(+), 1 deletion(-)

-- 
2.20.1

[Qemu-devel] [PATCH 2/2] test-block-iothread: Job coroutine thread after AioContext switch

[Kevin Wolf]

This tests that a job coroutine always runs in the right iothread after
the AioContext of its main node has changed.

Signed-off-by: Kevin Wolf 
---
 tests/test-block-iothread.c | 107 
 1 file changed, 107 insertions(+)

diff --git a/tests/test-block-iothread.c b/tests/test-block-iothread.c
index 97ac0b159d..036ed9a3b3 100644
--- a/tests/test-block-iothread.c
+++ b/tests/test-block-iothread.c
@@ -354,6 +354,111 @@ static void test_sync_op(const void *opaque)
 blk_unref(blk);
 }
 
+typedef struct TestBlockJob {
+BlockJob common;
+bool should_complete;
+int n;
+} TestBlockJob;
+
+static int test_job_prepare(Job *job)
+{
+g_assert(qemu_get_current_aio_context() == qemu_get_aio_context());
+return 0;
+}
+
+static int coroutine_fn test_job_run(Job *job, Error **errp)
+{
+TestBlockJob *s = container_of(job, TestBlockJob, common.job);
+
+job_transition_to_ready(>common.job);
+while (!s->should_complete) {
+s->n++;
+g_assert(qemu_get_current_aio_context() == job->aio_context);
+
+/* Avoid job_sleep_ns() because it marks the job as !busy. We want to
+ * emulate some actual activity (probably some I/O) here so that the
+ * drain involved in AioContext switches has to wait for this activity
+ * to stop. */
+qemu_co_sleep_ns(QEMU_CLOCK_REALTIME, 100);
+
+job_pause_point(>common.job);
+}
+
+g_assert(qemu_get_current_aio_context() == job->aio_context);
+return 0;
+}
+
+static void test_job_complete(Job *job, Error **errp)
+{
+TestBlockJob *s = container_of(job, TestBlockJob, common.job);
+s->should_complete = true;
+}
+
+BlockJobDriver test_job_driver = {
+.job_driver = {
+.instance_size  = sizeof(TestBlockJob),
+.free   = block_job_free,
+.user_resume= block_job_user_resume,
+.drain  = block_job_drain,
+.run= test_job_run,
+.complete   = test_job_complete,
+.prepare= test_job_prepare,
+},
+};
+
+static void test_attach_blockjob(void)
+{
+IOThread *iothread = iothread_new();
+AioContext *ctx = iothread_get_aio_context(iothread);
+BlockBackend *blk;
+BlockDriverState *bs;
+TestBlockJob *tjob;
+
+blk = blk_new(BLK_PERM_ALL, BLK_PERM_ALL);
+bs = bdrv_new_open_driver(_test, "base", BDRV_O_RDWR, _abort);
+blk_insert_bs(blk, bs, _abort);
+
+tjob = block_job_create("job0", _job_driver, NULL, bs,
+0, BLK_PERM_ALL,
+0, 0, NULL, NULL, _abort);
+job_start(>common.job);
+
+while (tjob->n == 0) {
+aio_poll(qemu_get_aio_context(), false);
+}
+
+blk_set_aio_context(blk, ctx);
+
+tjob->n = 0;
+while (tjob->n == 0) {
+aio_poll(qemu_get_aio_context(), false);
+}
+
+aio_context_acquire(ctx);
+blk_set_aio_context(blk, qemu_get_aio_context());
+aio_context_release(ctx);
+
+tjob->n = 0;
+while (tjob->n == 0) {
+aio_poll(qemu_get_aio_context(), false);
+}
+
+blk_set_aio_context(blk, ctx);
+
+tjob->n = 0;
+while (tjob->n == 0) {
+aio_poll(qemu_get_aio_context(), false);
+}
+
+aio_context_acquire(ctx);
+job_complete_sync(>common.job, _abort);
+blk_set_aio_context(blk, qemu_get_aio_context());
+aio_context_release(ctx);
+
+bdrv_unref(bs);
+blk_unref(blk);
+}
+
 int main(int argc, char **argv)
 {
 int i;
@@ -368,5 +473,7 @@ int main(int argc, char **argv)
 g_test_add_data_func(t->name, t, test_sync_op);
 }
 
+g_test_add_func("/attach/blockjob", test_attach_blockjob);
+
 return g_test_run();
 }
-- 
2.20.1

[Qemu-devel] [PATCH 0/2] hw/arm/boot: handle large Images more gracefully

[Peter Maydell]

This patchset attempts to fix https://bugs.launchpad.net/qemu/+bug/1823998
which reports that we don't handle kernels larger than 128MB
correctly, because we allow the initrd to be placed over the
tail end of the kernel. AArch64 kernel Image files (since v3.17)
report the total size they require (including any BSS area that
isn't in the Image itself), so we can use that to be sure we
place the initrd sufficiently far into the RAM.

Patch 1 in this series adjusts our "where do we put the initrd"
heuristic so that it always places it at least after whatever
our best guess at the kernel size is. (This might still not
be right for images like self-decompressing 32-bit kernels, where
there's no way to know how big the kernel will be after
decompression.) Patch 2 makes load_aarch64_image() return the
kernel size as indicated in the Image file header, so that for
the specific case of AArch64 Image files we will definitely not
put the initrd on top of them.

I've given this a quick smoke test but I don't have a very large
Image kernel to hand, so testing appreciated.

thanks
-- PMM

Peter Maydell (2):
  hw/arm/boot: Avoid placing the initrd on top of the kernel
  hw/arm/boot: Honour image size field in AArch64 Image format kernels

 hw/arm/boot.c | 51 +++
 1 file changed, 35 insertions(+), 16 deletions(-)

-- 
2.20.1

[Qemu-devel] [PATCH v3 2/5] linux-user: Add support for the SIOCSPGRP ioctl

[Aleksandar Markovic]

From: Aleksandar Markovic 

Add support for setting the process (or process group) to receive SIGIO
or SIGURG signals when I/O becomes possible or urgent data is available,
using SIOCSPGRP ioctl.

The ioctl numeric values for SIOCSPGRP are platform-dependent and are
determined by following files in Linux kernel source tree:

arch/ia64/include/uapi/asm/sockios.h:#define SIOCSPGRP0x8902
arch/mips/include/uapi/asm/sockios.h:#define SIOCSPGRP_IOW('s', 8, pid_t)
arch/parisc/include/uapi/asm/sockios.h:#define SIOCSPGRP  0x8902
arch/sh/include/uapi/asm/sockios.h:#define SIOCSPGRP  _IOW('s', 8, pid_t)
arch/xtensa/include/uapi/asm/sockios.h:#define SIOCSPGRP  _IOW('s', 8, pid_t)
arch/alpha/include/uapi/asm/sockios.h:#define SIOCSPGRP   _IOW('s', 8, pid_t)
arch/sparc/include/uapi/asm/sockios.h:#define SIOCSPGRP   0x8902
include/uapi/asm-generic/sockios.h:#define SIOCSPGRP  0x8902

Hence the different definition for alpha, mips, sh4, and xtensa.

Reviewed-by: Max Filippov 
Signed-off-by: Aleksandar Markovic 
---
 linux-user/ioctls.h   | 1 +
 linux-user/syscall_defs.h | 3 +++
 2 files changed, 4 insertions(+)

diff --git a/linux-user/ioctls.h b/linux-user/ioctls.h
index ae89516..c37adc5 100644
--- a/linux-user/ioctls.h
+++ b/linux-user/ioctls.h
@@ -218,6 +218,7 @@
   IOCTL(SIOCSRARP, IOC_W, MK_PTR(MK_STRUCT(STRUCT_arpreq)))
   IOCTL(SIOCGRARP, IOC_R, MK_PTR(MK_STRUCT(STRUCT_arpreq)))
   IOCTL(SIOCGIWNAME, IOC_W | IOC_R, MK_PTR(MK_STRUCT(STRUCT_char_ifreq)))
+  IOCTL(SIOCSPGRP, IOC_W, MK_PTR(TYPE_INT)) /* pid_t */
   IOCTL(SIOCGPGRP, IOC_R, MK_PTR(TYPE_INT)) /* pid_t */
   IOCTL(SIOCGSTAMP, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timeval)))
   IOCTL(SIOCGSTAMPNS, IOC_R, MK_PTR(MK_STRUCT(STRUCT_timespec)))
diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index 1e86fb9..2941231 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -739,11 +739,14 @@ struct target_pollfd {
 #if defined(TARGET_ALPHA) || defined(TARGET_MIPS) || defined(TARGET_SH4) ||
\
defined(TARGET_XTENSA)
 #define TARGET_SIOCATMARK  TARGET_IOR('s', 7, int)
+#define TARGET_SIOCSPGRP   TARGET_IOW('s', 8, pid_t)
 #define TARGET_SIOCGPGRP   TARGET_IOR('s', 9, pid_t)
 #else
 #define TARGET_SIOCATMARK  0x8905
+#define TARGET_SIOCSPGRP   0x8902
 #define TARGET_SIOCGPGRP   0x8904
 #endif
+
 #define TARGET_SIOCGSTAMP  0x8906  /* Get stamp (timeval) */
 #define TARGET_SIOCGSTAMPNS0x8907  /* Get stamp (timespec) */
 
-- 
2.7.4

Re: [Qemu-devel] [RFC 0/3] target/m68k: convert to transaction_failed hook

[Laurent Vivier]

On 10/12/2018 17:56, Peter Maydell wrote:

This patchset converts the m68k target from the deprecated
unassigned_access hook to the new transaction_failed hook.
It's RFC for a couple of reasons:
  * it's untested, since I don't have an m68k test image
  * the second patch just makes "bus error while trying to
read page tables" be treated as a page fault, when it
should probably cause a fault reporting it as a bus error
of some kind
  * I don't understand why the old unassigned_access hook
set the ATC bit in the MMU SSW, since the docs I have say
this should be set if the fault happened during a table
search, but cleared if it's just an ordinary bus-errored
data or insn access. Probably this is a pre-existing bug?


I think you're right. It must be cleared on bus error.

Thanks,
Laurent

[Qemu-devel] [PATCH 1/2] hw/arm/boot: Avoid placing the initrd on top of the kernel

[Peter Maydell]

We currently put the initrd at the smaller of:
 * 128MB into RAM
 * halfway into the RAM
(with the dtb following it).

However for large kernels this might mean that the kernel
overlaps the initrd. For some kinds of kernel (self-decompressing
32-bit kernels, and ELF images with a BSS section at the end)
we don't know the exact size, but even there we have a
minimum size. Put the initrd at least further into RAM than
that. For image formats that can give us an exact kernel size, this
will mean that we definitely avoid overlaying kernel and initrd.

Signed-off-by: Peter Maydell 
---
 hw/arm/boot.c | 34 --
 1 file changed, 20 insertions(+), 14 deletions(-)

diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index a830655e1af..7c978fedde4 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -998,20 +998,6 @@ static void arm_setup_direct_kernel_boot(ARMCPU *cpu,
 if (info->nb_cpus == 0)
 info->nb_cpus = 1;
 
-/*
- * We want to put the initrd far enough into RAM that when the
- * kernel is uncompressed it will not clobber the initrd. However
- * on boards without much RAM we must ensure that we still leave
- * enough room for a decent sized initrd, and on boards with large
- * amounts of RAM we must avoid the initrd being so far up in RAM
- * that it is outside lowmem and inaccessible to the kernel.
- * So for boards with less  than 256MB of RAM we put the initrd
- * halfway into RAM, and for boards with 256MB of RAM or more we put
- * the initrd at 128MB.
- */
-info->initrd_start = info->loader_start +
-MIN(info->ram_size / 2, 128 * 1024 * 1024);
-
 /* Assume that raw images are linux kernels, and ELF images are not.  */
 kernel_size = arm_load_elf(info, _entry, _low_addr,
_high_addr, elf_machine, as);
@@ -1056,6 +1042,26 @@ static void arm_setup_direct_kernel_boot(ARMCPU *cpu,
 exit(1);
 }
 info->entry = entry;
+
+/*
+ * We want to put the initrd far enough into RAM that when the
+ * kernel is uncompressed it will not clobber the initrd. However
+ * on boards without much RAM we must ensure that we still leave
+ * enough room for a decent sized initrd, and on boards with large
+ * amounts of RAM we must avoid the initrd being so far up in RAM
+ * that it is outside lowmem and inaccessible to the kernel.
+ * So for boards with less  than 256MB of RAM we put the initrd
+ * halfway into RAM, and for boards with 256MB of RAM or more we put
+ * the initrd at 128MB.
+ * We also refuse to put the initrd somewhere that will definitely
+ * overlay the kernel we just loaded, though for kernel formats which
+ * don't tell us their exact size (eg self-decompressing 32-bit kernels)
+ * we might still make a bad choice here.
+ */
+info->initrd_start = info->loader_start +
+MAX(MIN(info->ram_size / 2, 128 * 1024 * 1024), kernel_size);
+info->initrd_start = TARGET_PAGE_ALIGN(info->initrd_start);
+
 if (is_linux) {
 uint32_t fixupcontext[FIXUP_MAX];
 
-- 
2.20.1

[Qemu-devel] [PATCH v3 3/5] linux-user: Add support the SIOCSIFPFLAGS and SIOCGIFPFLAGS ioctls

[Aleksandar Markovic]

From: Neng Chen 

Add support for setting and getting extended (private) flags of a
network device via SIOCSIFPFLAGS and SIOCGIFPFLAGS ioctls.

The ioctl numeric value is platform-independent and determined by
the file include/uapi/linux/sockios.h in Linux kernel source code:

  #define SIOCSIFPFLAGS 0x8934

The ioctls set and get field ifr_flags of type short in the structure
ifreq. Such functionality in QEMU is achieved using MK_STRUCT() and
MK_PTR() macros with an appropriate argument.

Signed-off-by: Neng Chen 
Signed-off-by: Aleksandar Markovic 
Message-Id: <1554839486-3527-1-git-send-email-aleksandar.marko...@rt-rk.com>
---
 linux-user/ioctls.h   | 2 ++
 linux-user/syscall_defs.h | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/linux-user/ioctls.h b/linux-user/ioctls.h
index c37adc5..76375df 100644
--- a/linux-user/ioctls.h
+++ b/linux-user/ioctls.h
@@ -206,6 +206,8 @@
   IOCTL(SIOCADDMULTI, IOC_W, MK_PTR(MK_STRUCT(STRUCT_sockaddr_ifreq)))
   IOCTL(SIOCDELMULTI, IOC_W, MK_PTR(MK_STRUCT(STRUCT_sockaddr_ifreq)))
   IOCTL(SIOCGIFINDEX, IOC_W | IOC_R, MK_PTR(MK_STRUCT(STRUCT_int_ifreq)))
+  IOCTL(SIOCSIFPFLAGS, IOC_W, MK_PTR(MK_STRUCT(STRUCT_short_ifreq)))
+  IOCTL(SIOCGIFPFLAGS, IOC_W | IOC_R, MK_PTR(MK_STRUCT(STRUCT_short_ifreq)))
   IOCTL(SIOCSIFLINK, 0, TYPE_NULL)
   IOCTL_SPECIAL(SIOCGIFCONF, IOC_W | IOC_R, do_ioctl_ifconf,
 MK_PTR(MK_STRUCT(STRUCT_ifconf)))
diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index 2941231..8904d35 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -781,6 +781,8 @@ struct target_pollfd {
 #define TARGET_SIOCADDMULTI0x8931  /* Multicast address lists  
*/
 #define TARGET_SIOCDELMULTI0x8932
 #define TARGET_SIOCGIFINDEX0x8933
+#define TARGET_SIOCSIFPFLAGS   0x8934  /* set extended flags  
*/
+#define TARGET_SIOCGIFPFLAGS   0x8935  /* get extended flags  
*/
 
 /* Bridging control calls */
 #define TARGET_SIOCGIFBR   0x8940  /* Bridging support 
*/
-- 
2.7.4

[Qemu-devel] [PATCH v3 0/5] linux-user: A set of miscellaneous patches

[Aleksandar Markovic]

From: Aleksandar Markovic 

This is a collection of misc patches for Linux user that I recently
accumulated from variuous sources. All of them originate from problems
observed on mips target. However, these changes actually affect and fix
problems on multiple targets.

v1->v2:

  - updated and improved commit messages
  - added IPV6_DROP_MEMBERSHIP support to the patch on setsockopt()'s
option

v1->v2:

  - added the patch on setsockopt()'s option IPV6_ADD_MEMBERSHIP
  - improved the commit message of interp_info sanitizing patch

Aleksandar Markovic (2):
  linux-user: Fix support for the SIOCATMARK and SIOCGPGRP ioctls for
xtensa
  linux-user: Add support for the SIOCSPGRP ioctl

Daniel Santos (1):
  linux-user: Sanitize interp_info and, for mips only, init field fp_abi

Neng Chen (2):
  linux-user: Add support the SIOCSIFPFLAGS and SIOCGIFPFLAGS ioctls
  linux-user: Add support for setsockopt() options
IPV6__MEMBERSHIP

 linux-user/elfload.c  |  5 +
 linux-user/ioctls.h   |  3 +++
 linux-user/syscall.c  | 19 +++
 linux-user/syscall_defs.h |  8 +++-
 4 files changed, 34 insertions(+), 1 deletion(-)

-- 
2.7.4

[Qemu-devel] [PATCH v3 5/5] linux-user: Sanitize interp_info and, for mips only, init field fp_abi

[Aleksandar Markovic]

From: Daniel Santos 

Sanitize interp_info structure in load_elf_binary() and, for MIPS only,
init its field fp_abi to MIPS_ABI_FP_UNKNOWN. This fixes appearances of
"Unexpected FPU mode" message in some MIPS use cases. Currently, this
bug is a complete stopper for some MIPS binaries.

In load_elf_binary(), struct image_info interp_info is used without
being properly initialized. One result is that when the ELF's program
header doesn't contain an entry for the ABI flags, then the value of
the struct image_info's fp_abi field is set to whatever happened to
be in stack memory at the time.

Backporting to 4.0 and, if possible, to 3.1 is recommended.

Fixes: https://bugs.launchpad.net/qemu/+bug/1825002

Signed-off-by: Daniel Santos 
Signed-off-by: Aleksandar Markovic 
Reviewed-by: Philippe Mathieu-Daudé 
---
 linux-user/elfload.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index c1a2602..7f09d57 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -2698,6 +2698,11 @@ int load_elf_binary(struct linux_binprm *bprm, struct 
image_info *info)
 char *elf_interpreter = NULL;
 char *scratch;
 
+memset(_info, 0, sizeof(interp_info));
+#ifdef TARGET_MIPS
+interp_info.fp_abi = MIPS_ABI_FP_UNKNOWN;
+#endif
+
 info->start_mmap = (abi_ulong)ELF_START_MMAP;
 
 load_elf_image(bprm->filename, bprm->fd, info,
-- 
2.7.4

Re: [Qemu-devel] [PATCH] tests/tcg/alpha: add system boot.S

[Alex Bennée]

Richard Henderson  writes:

> This provides the bootstrap and low level helper functions for an
> alpha kernel.  We use direct access to the DP264 serial port for
> test output, and hard machine halt to exit the emulation.

Queued to testing/next, thanks.

I've also added tests/tcg/alpha/system/ to MAINTAINERS

>
> Signed-off-by: Richard Henderson 
> ---
>  tests/tcg/alpha/Makefile.softmmu-target |  32 ++
>  tests/tcg/alpha/system/boot.S   | 511 
>  tests/tcg/alpha/system/kernel.ld|  30 ++
>  3 files changed, 573 insertions(+)
>  create mode 100644 tests/tcg/alpha/Makefile.softmmu-target
>  create mode 100644 tests/tcg/alpha/system/boot.S
>  create mode 100644 tests/tcg/alpha/system/kernel.ld
>
> diff --git a/tests/tcg/alpha/Makefile.softmmu-target 
> b/tests/tcg/alpha/Makefile.softmmu-target
> new file mode 100644
> index 00..9f4b199258
> --- /dev/null
> +++ b/tests/tcg/alpha/Makefile.softmmu-target
> @@ -0,0 +1,32 @@
> +#
> +# Alpha system tests
> +#
> +
> +ALPHA_SYSTEM_SRC=$(SRC_PATH)/tests/tcg/alpha/system
> +VPATH+=$(ALPHA_SYSTEM_SRC)
> +
> +# These objects provide the basic boot code and helper functions for all 
> tests
> +CRT_OBJS=boot.o
> +
> +ALPHA_TEST_SRCS=$(wildcard $(ALPHA_SYSTEM_SRC)/*.c)
> +ALPHA_TESTS = $(patsubst $(ALPHA_SYSTEM_SRC)/%.c, %, $(ALPHA_TEST_SRCS))
> +
> +CRT_PATH=$(ALPHA_SYSTEM_SRC)
> +LINK_SCRIPT=$(ALPHA_SYSTEM_SRC)/kernel.ld
> +LDFLAGS=-Wl,-T$(LINK_SCRIPT)
> +TESTS+=$(ALPHA_TESTS) $(MULTIARCH_TESTS)
> +CFLAGS+=-nostdlib -g -O1 -mcpu=ev6 $(MINILIB_INC)
> +LDFLAGS+=-static -nostdlib $(CRT_OBJS) $(MINILIB_OBJS) -lgcc
> +
> +# building head blobs
> +.PRECIOUS: $(CRT_OBJS)
> +
> +%.o: $(CRT_PATH)/%.S
> + $(CC) $(CFLAGS) -x assembler-with-cpp -c $< -o $@
> +
> +# Build and link the tests
> +%: %.c $(LINK_SCRIPT) $(CRT_OBJS) $(MINILIB_OBJS)
> + $(CC) $(CFLAGS) $< -o $@ $(LDFLAGS)
> +
> +# Running
> +QEMU_OPTS+=-serial chardev:output -kernel
> diff --git a/tests/tcg/alpha/system/boot.S b/tests/tcg/alpha/system/boot.S
> new file mode 100644
> index 00..6d7fb36e17
> --- /dev/null
> +++ b/tests/tcg/alpha/system/boot.S
> @@ -0,0 +1,511 @@
> +/*
> + * Minimal Alpha system boot code.
> + *
> + * Copyright Linaro Ltd 2019
> + */
> +
> + .setnoat
> + .setnomacro
> + .arch   ev6
> + .text
> +
> +.macro load_pci_io reg
> + /* For typhoon, this is
> +  *   0xfc00  -- kseg identity map
> +  * +  0x100  -- typhoon pio base
> +  * +0x1fc00  -- typhoon pchip0 pci base
> +  * = 0xfd01fc00
> +  */
> + ldah\reg, -3/* ff..fd */
> + lda \reg, 0x1fc(\reg)   /* ff..fd01fc */
> + sll \reg, 24, \reg
> +.endm
> +
> +#define com1Rbr 0x3f8
> +#define com1Thr 0x3f8
> +#define com1Ier 0x3f9
> +#define com1Iir 0x3fa
> +#define com1Lcr 0x3fb
> +#define com1Mcr 0x3fc
> +#define com1Lsr 0x3fd
> +#define com1Msr 0x3fe
> +#define com1Scr 0x3ff
> +#define com1Dll 0x3f8
> +#define com1Dlm 0x3f9
> +
> +#define PAL_halt0
> +#define PAL_wrent  52
> +#define PAL_wrkgp  55
> +
> + .text
> + .p2align 4
> + .globl  _start
> + .ent_start
> +_start:
> + br  $gp, .+4
> + ldah$gp, 0($gp) !gpdisp!1
> + lda $gp, 0($gp) !gpdisp!1
> +
> + ldah$sp, $stack_end($gp)!gprelhigh
> + lda $sp, $stack_end($gp)!gprellow
> +
> + /* Install kernel gp for exception handlers.  */
> + mov $gp, $16
> + call_pal PAL_wrkgp
> +
> + /* Install exception handlers.  */
> + ldah$16, entInt($gp)!gprelhigh
> + lda $16, entInt($16)!gprellow
> + lda $17, 0
> + call_pal PAL_wrent
> +
> + ldah$16, entArith($gp)  !gprelhigh
> + lda $16, entArith($16)  !gprellow
> + lda $17, 1
> + call_pal PAL_wrent
> +
> + ldah$16, entMM($gp) !gprelhigh
> + lda $16, entMM($16) !gprellow
> + lda $17, 2
> + call_pal PAL_wrent
> +
> + ldah$16, entIF($gp) !gprelhigh
> + lda $16, entIF($16) !gprellow
> + lda $17, 3
> + call_pal PAL_wrent
> +
> + ldah$16, entUna($gp)!gprelhigh
> + lda $16, entUna($16)!gprellow
> + lda $17, 4
> + call_pal PAL_wrent
> +
> + ldah$16, entSys($gp)!gprelhigh
> + lda $16, entSys($16)!gprellow
> + lda $17, 5
> + call_pal PAL_wrent
> +
> + /*
> +  * Initialize COM1.
> +  */
> + load_pci_io $1
> + lda $2, 0x87/* outb(0x87, com1Lcr); */
> + stb $2, com1Lcr($1)
> + stb $31, com1Dlm($1)/* outb(0, com1Dlm); */
> + lda $2, 3   /* baudconst 3 => 56000 */
> + stb $2, com1Dll($1) /* outb(baudconst, com1Dll); */
> + lda $2, 0x07
> + stb $2, com1Lcr($1) /* outb(0x07, com1Lcr) 

Re: [Qemu-devel] [PULL 19/19] configure: automatically pick python3 is available

[Philippe Mathieu-Daudé]

On 5/3/19 6:41 PM, Thomas Huth wrote:
> On 03/05/2019 02.41, Eduardo Habkost wrote:
>> From: Daniel P. Berrangé 
>>
>> Unless overridden via an env var or configure arg, QEMU will only look
>> for the 'python' binary in $PATH. This is unhelpful on distros which
>> are only shipping Python 3.x (eg Fedora) in their default install as,
>> if they comply with PEP 394, the bare 'python' binary won't exist.
>>
>> This changes configure so that by default it will search for all three
>> common python binaries, preferring to find Python 3.x versions.
>>
>> Signed-off-by: Daniel P. Berrangé 
>> Message-Id: <20190327170701.23798-1-berra...@redhat.com>
>> Signed-off-by: Eduardo Habkost 
>> ---
>>  configure | 18 +++---
>>  1 file changed, 15 insertions(+), 3 deletions(-)
> 
> I haven't bisected it, but I think this patch here broke the gitlab-ci tests:
> 
>  https://gitlab.com/huth/qemu/-/jobs/206806257

What's the easier way to notice that automatically?

The quicker fix I have is Peter setup'ing a GitLab account mirroring his
repo:staging branch, and warn him, but that won't scale much.

> Seems like the test is now failing when you don't have an UTF-8 locale:
> 
>  LANG=C make check-qapi-schema
>  [...]
>  TESTtests/qapi-schema/union-base-empty.out
>  --- /builds/huth/qemu/tests/qapi-schema/unicode-str.err  2019-05-03 
> 15:21:39.0 +
>  +++ -2019-05-03 15:42:01.561762978 +
>  @@ -1 +1 @@
>  -tests/qapi-schema/unicode-str.json:2: 'command' uses invalid name 'é'
>  +tests/qapi-schema/unicode-str.json:2: 'command' uses invalid name '\xe9'
>  /builds/huth/qemu/tests/Makefile.include:1105: recipe for target 
> 'check-tests/qapi-schema/unicode-str.json' failed
>  make: *** [check-tests/qapi-schema/unicode-str.json] Error 1
> 
> Any ideas how to fix this?
> 
>  Thomas
> 

[Qemu-devel] [PATCH] net: avoid to use variable length array in net_client_init()

[Stefano Garzarella]

net_client_init() uses a variable length array to store the prefix
of 'ipv6-net' parameter (e.g. if ipv6-net=fec0::0/64, the prefix
is 'fec0::0').
Since the IPv6 prefix can be at most as long as an IPv6 address,
we can use an array with fixed size equals to INET6_ADDRSTRLEN.

Signed-off-by: Stefano Garzarella 
---
 net/net.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/net.c b/net/net.c
index f3a3c5444c..2e5f27e121 100644
--- a/net/net.c
+++ b/net/net.c
@@ -1118,7 +1118,7 @@ static int net_client_init(QemuOpts *opts, bool 
is_netdev, Error **errp)
 const char *ip6_net = qemu_opt_get(opts, "ipv6-net");
 
 if (ip6_net) {
-char buf[strlen(ip6_net) + 1];
+char buf[INET6_ADDRSTRLEN];
 
 if (get_str_sep(buf, sizeof(buf), _net, '/') < 0) {
 /* Default 64bit prefix length.  */
-- 
2.20.1

Re: [Qemu-devel] [PATCH v4] hw/virtio/virtio-mmio: Convert DPRINTF to trace and log

[Alex Bennée]

LI, BO XUAN  writes:

> Hi Alex,
>
> Sorry about that, I am still trying to get familiar with the patch
> submission process. Since my patch has been changed from your last review,
> I thought it would be safe to not include the r-b tag from last time. Will
> take care next time!

That's ok. As a general rule as long as you haven't substantially
changed a patch it's safe to keep previous r-b tags. You can always
mention it in your cover letter if you are unsure.

--
Alex Bennée

Re: [Qemu-devel] [PATCH] tests/docker: add ubuntu 18.04

[Philippe Mathieu-Daudé]

On 5/3/19 6:42 PM, Alex Bennée wrote:
> 
> Gerd Hoffmann  writes:
> 
>> Based on the ubuntu.docker file.
>> Used to reproduce the build failure Peter was seeing.
>> Others might find this useful too ;)
>>
>> Signed-off-by: Gerd Hoffmann 
>> ---
>>  tests/docker/dockerfiles/ubuntu1804.docker | 57 ++
>>  1 file changed, 57 insertions(+)
>>  create mode 100644 tests/docker/dockerfiles/ubuntu1804.docker
>>
>> diff --git a/tests/docker/dockerfiles/ubuntu1804.docker 
>> b/tests/docker/dockerfiles/ubuntu1804.docker
>> new file mode 100644
>> index ..2e2900150b09
>> --- /dev/null
>> +++ b/tests/docker/dockerfiles/ubuntu1804.docker
>> @@ -0,0 +1,57 @@
>> +FROM ubuntu:18.04
>> +ENV PACKAGES flex bison \
>> +ccache \
>> +clang \
>> +gcc \
>> +gettext \
>> +git \
>> +glusterfs-common \
>> +libaio-dev \
>> +libattr1-dev \
>> +libbluetooth-dev \
>> +libbrlapi-dev \
>> +libbz2-dev \
>> +libcacard-dev \
>> +libcap-dev \
>> +libcap-ng-dev \
>> +libcurl4-gnutls-dev \
>> +libdrm-dev \
>> +libepoxy-dev \
>> +libfdt-dev \
>> +libgbm-dev \
>> +libgtk-3-dev \
>> +libibverbs-dev \
>> +libiscsi-dev \
>> +libjemalloc-dev \
>> +libjpeg-turbo8-dev \
>> +liblzo2-dev \
>> +libncurses5-dev \
>> +libncursesw5-dev \
>> +libnfs-dev \
>> +libnss3-dev \
>> +libnuma-dev \
>> +libpixman-1-dev \
>> +librados-dev \
>> +librbd-dev \
>> +librdmacm-dev \
>> +libsasl2-dev \
>> +libsdl2-dev \
>> +libseccomp-dev \
>> +libsnappy-dev \
>> +libspice-protocol-dev \
>> +libspice-server-dev \
>> +libssh2-1-dev \
>> +libusb-1.0-0-dev \
>> +libusbredirhost-dev \
>> +libvdeplug-dev \
>> +libvte-2.91-dev \
>> +libxen-dev \
>> +make \
>> +python-yaml \
>> +sparse \
>> +texinfo \
>> +xfslibs-dev
>> +RUN apt-get update && \
>> +apt-get -y install $PACKAGES
>> +RUN dpkg -l $PACKAGES | sort > /packages.txt
>> +ENV FEATURES clang pyyaml sdl2
> 
> Queued to testing/next, thanks.

Just finished builds, so if it's not too late:

Tested-by: Philippe Mathieu-Daudé 

I'm not sure whichever {ubuntu1804/ubuntu18.04}.docker is better,
anyway:
Reviewed-by: Philippe Mathieu-Daudé 

> 
> --
> Alex Bennée
> 

Re: [Qemu-devel] [PULL 19/19] configure: automatically pick python3 is available

[Daniel P . Berrangé]

On Fri, May 03, 2019 at 05:54:35PM +0100, Daniel P. Berrangé wrote:
> On Fri, May 03, 2019 at 06:41:43PM +0200, Thomas Huth wrote:
> > On 03/05/2019 02.41, Eduardo Habkost wrote:
> > > From: Daniel P. Berrangé 
> > > 
> > > Unless overridden via an env var or configure arg, QEMU will only look
> > > for the 'python' binary in $PATH. This is unhelpful on distros which
> > > are only shipping Python 3.x (eg Fedora) in their default install as,
> > > if they comply with PEP 394, the bare 'python' binary won't exist.
> > > 
> > > This changes configure so that by default it will search for all three
> > > common python binaries, preferring to find Python 3.x versions.
> > > 
> > > Signed-off-by: Daniel P. Berrangé 
> > > Message-Id: <20190327170701.23798-1-berra...@redhat.com>
> > > Signed-off-by: Eduardo Habkost 
> > > ---
> > >  configure | 18 +++---
> > >  1 file changed, 15 insertions(+), 3 deletions(-)
> > 
> > I haven't bisected it, but I think this patch here broke the gitlab-ci 
> > tests:
> > 
> >  https://gitlab.com/huth/qemu/-/jobs/206806257
> > 
> > Seems like the test is now failing when you don't have an UTF-8 locale:
> > 
> >  LANG=C make check-qapi-schema
> >  [...]
> >  TESTtests/qapi-schema/union-base-empty.out
> >  --- /builds/huth/qemu/tests/qapi-schema/unicode-str.err2019-05-03 
> > 15:21:39.0 +
> >  +++ -  2019-05-03 15:42:01.561762978 +
> >  @@ -1 +1 @@
> >  -tests/qapi-schema/unicode-str.json:2: 'command' uses invalid name 'é'
> >  +tests/qapi-schema/unicode-str.json:2: 'command' uses invalid name '\xe9'
> >  /builds/huth/qemu/tests/Makefile.include:1105: recipe for target 
> > 'check-tests/qapi-schema/unicode-str.json' failed
> >  make: *** [check-tests/qapi-schema/unicode-str.json] Error 1
> > 
> > Any ideas how to fix this?
> 
> python3 is basically doomed if you use the C locale for LC_CTYPE, as
> it is not 8-bit clean.
> 
> If a python3 program is liable to see UTF-8 input data, the following
> env should generally be set when running python:
> 
>LC_ALL= LANG=C LC_CTYPE=en_US.UTF-8

Oh, actually I forgot we did that and then changed approach in QEMU,
see these:

commit 0d6b93deeeb3cc190692d629f5927befdc8b1fb8
Author: Matthias Maier 
Date:   Mon Jun 18 19:59:58 2018 +0200

Revert commit d4e5ec877ca

This commit removes the PYTHON_UTF8 workaround. The problem with setting

  LC_ALL= LANG=C LC_CTYPE=en_US.UTF-8

is that the en_US.UTF-8 locale might not be available. In this case
setting above locales results in build errors even though another UTF-8
locale was originally set [1]. The only stable way of fixing the
encoding problem is by specifying the encoding in Python, like the
previous commit does.

[1] https://bugs.gentoo.org/657766

Signed-off-by: Arfrever Frehtes Taifersar Arahesis 
Signed-off-by: Matthias Maier 
Message-Id: <20180618175958.29073-3-arm...@redhat.com>
Reviewed-by: Eduardo Habkost 
Reviewed-by: Eric Blake 
[Commit message tweaked]
Signed-off-by: Markus Armbruster 

commit de685ae5e9a4b523513033bd6cadc8187a227170
Author: Markus Armbruster 
Date:   Mon Jun 18 19:59:57 2018 +0200

qapi: Open files with encoding='utf-8'

Python 2 happily reads UTF-8 files in text mode, but Python 3 requires
either UTF-8 locale or an explicit encoding passed to open().  Commit
d4e5ec877ca fixed this by setting the en_US.UTF-8 locale.  Falls apart
when the locale isn't be available.

Matthias Maier and Arfrever Frehtes Taifersar Arahesis proposed to use
binary mode instead, with manual conversion from bytes to str.  Works,
but opening with an explicit encoding is simpler, so do that.

Since Python 2's open() doesn't support the encoding parameter, we
need to suppress it with a version check.

Reported-by: Arfrever Frehtes Taifersar Arahesis 
Reported-by: Matthias Maier 
Signed-off-by: Markus Armbruster 
Message-Id: <20180618175958.29073-2-arm...@redhat.com>
Reviewed-by: Eduardo Habkost 
Reviewed-by: Eric Blake 


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [RFC 3/3] target/m68k: Switch to transaction_failed hook

[Laurent Vivier]

On 10/12/2018 17:56, Peter Maydell wrote:

Switch the m68k target from the old unassigned_access hook
to the transaction_failed hook.

The notable difference is that rather than it being called
for all physical memory accesses which fail (including
those made by DMA devices or by the gdbstub), it is only
called for those made by the CPU via its MMU. (In previous
commits we put in explicit checks for the direct physical
loads made by the target/m68k code which will no longer
be handled by calling the unassigned_access hook.)

Signed-off-by: Peter Maydell 
---
  target/m68k/cpu.h   |  7 ---
  target/m68k/cpu.c   |  2 +-
  target/m68k/op_helper.c | 20 
  3 files changed, 13 insertions(+), 16 deletions(-)

diff --git a/target/m68k/cpu.h b/target/m68k/cpu.h
index b288a3864e0..08828b0581b 100644
--- a/target/m68k/cpu.h
+++ b/target/m68k/cpu.h
@@ -545,9 +545,10 @@ static inline int cpu_mmu_index (CPUM68KState *env, bool 
ifetch)
  
  int m68k_cpu_handle_mmu_fault(CPUState *cpu, vaddr address, int size, int rw,

int mmu_idx);
-void m68k_cpu_unassigned_access(CPUState *cs, hwaddr addr,
-bool is_write, bool is_exec, int is_asi,
-unsigned size);
+void m68k_cpu_transaction_failed(CPUState *cs, hwaddr physaddr, vaddr addr,
+ unsigned size, MMUAccessType access_type,
+ int mmu_idx, MemTxAttrs attrs,
+ MemTxResult response, uintptr_t retaddr);
  
  #include "exec/cpu-all.h"
  
diff --git a/target/m68k/cpu.c b/target/m68k/cpu.c

index 582e3a73b37..6d09c630b0e 100644
--- a/target/m68k/cpu.c
+++ b/target/m68k/cpu.c
@@ -271,7 +271,7 @@ static void m68k_cpu_class_init(ObjectClass *c, void *data)
  cc->gdb_write_register = m68k_cpu_gdb_write_register;
  cc->handle_mmu_fault = m68k_cpu_handle_mmu_fault;
  #if defined(CONFIG_SOFTMMU)
-cc->do_unassigned_access = m68k_cpu_unassigned_access;
+cc->do_transaction_failed = m68k_cpu_transaction_failed;
  cc->get_phys_page_debug = m68k_cpu_get_phys_page_debug;
  #endif
  cc->disas_set_info = m68k_cpu_disas_set_info;
diff --git a/target/m68k/op_helper.c b/target/m68k/op_helper.c
index 8d09ed91c49..6739ab8e436 100644
--- a/target/m68k/op_helper.c
+++ b/target/m68k/op_helper.c
@@ -454,19 +454,15 @@ static inline void do_interrupt_m68k_hardirq(CPUM68KState 
*env)
  do_interrupt_all(env, 1);
  }
  
-void m68k_cpu_unassigned_access(CPUState *cs, hwaddr addr, bool is_write,

-bool is_exec, int is_asi, unsigned size)
+void m68k_cpu_transaction_failed(CPUState *cs, hwaddr physaddr, vaddr addr,
+ unsigned size, MMUAccessType access_type,
+ int mmu_idx, MemTxAttrs attrs,
+ MemTxResult response, uintptr_t retaddr)
  {
  M68kCPU *cpu = M68K_CPU(cs);
  CPUM68KState *env = >env;
-#ifdef DEBUG_UNASSIGNED
-qemu_log_mask(CPU_LOG_INT, "Unassigned " TARGET_FMT_plx " wr=%d exe=%d\n",
- addr, is_write, is_exec);
-#endif
-if (env == NULL) {
-/* when called from gdb, env is NULL */
-return;
-}
+
+cpu_restore_state(cs, retaddr, true);
  
  if (m68k_feature(env, M68K_FEATURE_M68040)) {

  env->mmu.mmusr = 0;
@@ -476,7 +472,7 @@ void m68k_cpu_unassigned_access(CPUState *cs, hwaddr addr, 
bool is_write,
  if (env->sr & SR_S) { /* SUPERVISOR */
  env->mmu.ssw |= M68K_TM_040_SUPER;
  }
-if (is_exec) { /* instruction or data */
+if (access_type == MMU_INST_FETCH) { /* instruction or data */
  env->mmu.ssw |= M68K_TM_040_CODE;
  } else {
  env->mmu.ssw |= M68K_TM_040_DATA;
@@ -494,7 +490,7 @@ void m68k_cpu_unassigned_access(CPUState *cs, hwaddr addr, 
bool is_write,
  break;
  }
  
-if (!is_write) {

+if (access_type != MMU_DATA_STORE) {
  env->mmu.ssw |= M68K_RW_040;
  }
  



Reviewed-by: Laurent Vivier 

Re: [Qemu-devel] [PULL 19/19] configure: automatically pick python3 is available

[Daniel P . Berrangé]

On Fri, May 03, 2019 at 06:41:43PM +0200, Thomas Huth wrote:
> On 03/05/2019 02.41, Eduardo Habkost wrote:
> > From: Daniel P. Berrangé 
> > 
> > Unless overridden via an env var or configure arg, QEMU will only look
> > for the 'python' binary in $PATH. This is unhelpful on distros which
> > are only shipping Python 3.x (eg Fedora) in their default install as,
> > if they comply with PEP 394, the bare 'python' binary won't exist.
> > 
> > This changes configure so that by default it will search for all three
> > common python binaries, preferring to find Python 3.x versions.
> > 
> > Signed-off-by: Daniel P. Berrangé 
> > Message-Id: <20190327170701.23798-1-berra...@redhat.com>
> > Signed-off-by: Eduardo Habkost 
> > ---
> >  configure | 18 +++---
> >  1 file changed, 15 insertions(+), 3 deletions(-)
> 
> I haven't bisected it, but I think this patch here broke the gitlab-ci tests:
> 
>  https://gitlab.com/huth/qemu/-/jobs/206806257
> 
> Seems like the test is now failing when you don't have an UTF-8 locale:
> 
>  LANG=C make check-qapi-schema
>  [...]
>  TESTtests/qapi-schema/union-base-empty.out
>  --- /builds/huth/qemu/tests/qapi-schema/unicode-str.err  2019-05-03 
> 15:21:39.0 +
>  +++ -2019-05-03 15:42:01.561762978 +
>  @@ -1 +1 @@
>  -tests/qapi-schema/unicode-str.json:2: 'command' uses invalid name 'é'
>  +tests/qapi-schema/unicode-str.json:2: 'command' uses invalid name '\xe9'
>  /builds/huth/qemu/tests/Makefile.include:1105: recipe for target 
> 'check-tests/qapi-schema/unicode-str.json' failed
>  make: *** [check-tests/qapi-schema/unicode-str.json] Error 1
> 
> Any ideas how to fix this?

python3 is basically doomed if you use the C locale for LC_CTYPE, as
it is not 8-bit clean.

If a python3 program is liable to see UTF-8 input data, the following
env should generally be set when running python:

   LC_ALL= LANG=C LC_CTYPE=en_US.UTF-8  

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

[Qemu-devel] [Bug 1823998] Re: qemu-system-aarch64: support kernels bigger than 128MiB

[Peter Maydell]

** Tags added: arm

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

Title:
  qemu-system-aarch64: support kernels bigger than 128MiB

Status in QEMU:
  New

Bug description:
  Presently QEMU reserves up to 128MiB of space for an arm64 Linux
  kernel, placing the initrd following this, and the dtb following the
  initrd.

  This is not sufficient for some debug configurations of the kernel,
  which can be larger than 128MiB. Depending on the relative size of the
  kernel Image and unpopulated BSS, the dtb (or kernel) will be
  clobbered by the other, resulting in a silent boot failure.

  Since v3.17, the kernel Image header exposes a field called
  image_size, which describes the entire size of the kernel (including
  unpopulated sections such as the BSS) as a 64-bit little-endian value.
  For kernels prior to v3.17, this field is zero. This is documented at:

  
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/arm64/booting.txt?h=v5.0#n68

  It would be great if QEMU could take the image_size field into account
  when placing the initrd and dtb to avoid overlap with the kernel.

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

Re: [Qemu-devel] [RFC 2/3] target/m68k: In get_physical_address() check for memory access failures

[Laurent Vivier]

On 10/12/2018 17:56, Peter Maydell wrote:

In get_physical_address(), use address_space_ldl() and
address_space_stl() instead of ldl_phys() and stl_phys().
This allows us to check whether the memory access failed.
For the moment, we simply return -1 in this case;
add a TODO comment that we should ideally generate the
appropriate kind of fault.

Signed-off-by: Peter Maydell 
---
  target/m68k/helper.c | 62 +---
  1 file changed, 52 insertions(+), 10 deletions(-)

diff --git a/target/m68k/helper.c b/target/m68k/helper.c
index 374e4861886..b5fa2f8056d 100644
--- a/target/m68k/helper.c
+++ b/target/m68k/helper.c
@@ -660,6 +660,7 @@ static int get_physical_address(CPUM68KState *env, hwaddr 
*physical,
  bool debug = access_type & ACCESS_DEBUG;
  int page_bits;
  int i;
+MemTxResult txres;
  
  /* Transparent Translation (physical = logical) */

  for (i = 0; i < M68K_MAX_TTR; i++) {
@@ -689,12 +690,19 @@ static int get_physical_address(CPUM68KState *env, hwaddr 
*physical,
  /* Root Index */
  entry = M68K_POINTER_BASE(next) | M68K_ROOT_INDEX(address);
  
-next = ldl_phys(cs->as, entry);

+next = address_space_ldl(cs->as, entry, MEMTXATTRS_UNSPECIFIED, );
+if (txres != MEMTX_OK) {
+goto txfail;
+}
  if (!M68K_UDT_VALID(next)) {
  return -1;
  }
  if (!(next & M68K_DESC_USED) && !debug) {
-stl_phys(cs->as, entry, next | M68K_DESC_USED);
+address_space_stl(cs->as, entry, next | M68K_DESC_USED,
+  MEMTXATTRS_UNSPECIFIED, );
+if (txres != MEMTX_OK) {
+goto txfail;
+}
  }
  if (next & M68K_DESC_WRITEPROT) {
  if (access_type & ACCESS_PTEST) {
@@ -709,12 +717,19 @@ static int get_physical_address(CPUM68KState *env, hwaddr 
*physical,
  /* Pointer Index */
  entry = M68K_POINTER_BASE(next) | M68K_POINTER_INDEX(address);
  
-next = ldl_phys(cs->as, entry);

+next = address_space_ldl(cs->as, entry, MEMTXATTRS_UNSPECIFIED, );
+if (txres != MEMTX_OK) {
+goto txfail;
+}
  if (!M68K_UDT_VALID(next)) {
  return -1;
  }
  if (!(next & M68K_DESC_USED) && !debug) {
-stl_phys(cs->as, entry, next | M68K_DESC_USED);
+address_space_stl(cs->as, entry, next | M68K_DESC_USED,
+  MEMTXATTRS_UNSPECIFIED, );
+if (txres != MEMTX_OK) {
+goto txfail;
+}
  }
  if (next & M68K_DESC_WRITEPROT) {
  if (access_type & ACCESS_PTEST) {
@@ -733,27 +748,46 @@ static int get_physical_address(CPUM68KState *env, hwaddr 
*physical,
  entry = M68K_4K_PAGE_BASE(next) | M68K_4K_PAGE_INDEX(address);
  }
  
-next = ldl_phys(cs->as, entry);

+next = address_space_ldl(cs->as, entry, MEMTXATTRS_UNSPECIFIED, );
+if (txres != MEMTX_OK) {
+goto txfail;
+}
  
  if (!M68K_PDT_VALID(next)) {

  return -1;
  }
  if (M68K_PDT_INDIRECT(next)) {
-next = ldl_phys(cs->as, M68K_INDIRECT_POINTER(next));
+next = address_space_ldl(cs->as, M68K_INDIRECT_POINTER(next),
+ MEMTXATTRS_UNSPECIFIED, );
+if (txres != MEMTX_OK) {
+goto txfail;
+}
  }
  if (access_type & ACCESS_STORE) {
  if (next & M68K_DESC_WRITEPROT) {
  if (!(next & M68K_DESC_USED) && !debug) {
-stl_phys(cs->as, entry, next | M68K_DESC_USED);
+address_space_stl(cs->as, entry, next | M68K_DESC_USED,
+  MEMTXATTRS_UNSPECIFIED, );
+if (txres != MEMTX_OK) {
+goto txfail;
+}
  }
  } else if ((next & (M68K_DESC_MODIFIED | M68K_DESC_USED)) !=
 (M68K_DESC_MODIFIED | M68K_DESC_USED) && !debug) {
-stl_phys(cs->as, entry,
- next | (M68K_DESC_MODIFIED | M68K_DESC_USED));
+address_space_stl(cs->as, entry,
+  next | (M68K_DESC_MODIFIED | M68K_DESC_USED),
+  MEMTXATTRS_UNSPECIFIED, );
+if (txres != MEMTX_OK) {
+goto txfail;
+}
  }
  } else {
  if (!(next & M68K_DESC_USED) && !debug) {
-stl_phys(cs->as, entry, next | M68K_DESC_USED);
+address_space_stl(cs->as, entry, next | M68K_DESC_USED,
+  MEMTXATTRS_UNSPECIFIED, );
+if (txres != MEMTX_OK) {
+goto txfail;
+}
  }
  }
  
@@ -785,6 +819,14 @@ static int get_physical_address(CPUM68KState *env, hwaddr *physical,

  }
  
  return 0;

+
+txfail:
+/*
+ * A page table load/store failed. TODO: we should really raise a
+ * suitable guest fault here if this is not a debug access.
+ * For now just return that the translation failed.
+ */
+return 

Re: [Qemu-devel] [PATCH] tests/docker: add ubuntu 18.04

[Alex Bennée]

Gerd Hoffmann  writes:

> Based on the ubuntu.docker file.
> Used to reproduce the build failure Peter was seeing.
> Others might find this useful too ;)
>
> Signed-off-by: Gerd Hoffmann 
> ---
>  tests/docker/dockerfiles/ubuntu1804.docker | 57 ++
>  1 file changed, 57 insertions(+)
>  create mode 100644 tests/docker/dockerfiles/ubuntu1804.docker
>
> diff --git a/tests/docker/dockerfiles/ubuntu1804.docker 
> b/tests/docker/dockerfiles/ubuntu1804.docker
> new file mode 100644
> index ..2e2900150b09
> --- /dev/null
> +++ b/tests/docker/dockerfiles/ubuntu1804.docker
> @@ -0,0 +1,57 @@
> +FROM ubuntu:18.04
> +ENV PACKAGES flex bison \
> +ccache \
> +clang \
> +gcc \
> +gettext \
> +git \
> +glusterfs-common \
> +libaio-dev \
> +libattr1-dev \
> +libbluetooth-dev \
> +libbrlapi-dev \
> +libbz2-dev \
> +libcacard-dev \
> +libcap-dev \
> +libcap-ng-dev \
> +libcurl4-gnutls-dev \
> +libdrm-dev \
> +libepoxy-dev \
> +libfdt-dev \
> +libgbm-dev \
> +libgtk-3-dev \
> +libibverbs-dev \
> +libiscsi-dev \
> +libjemalloc-dev \
> +libjpeg-turbo8-dev \
> +liblzo2-dev \
> +libncurses5-dev \
> +libncursesw5-dev \
> +libnfs-dev \
> +libnss3-dev \
> +libnuma-dev \
> +libpixman-1-dev \
> +librados-dev \
> +librbd-dev \
> +librdmacm-dev \
> +libsasl2-dev \
> +libsdl2-dev \
> +libseccomp-dev \
> +libsnappy-dev \
> +libspice-protocol-dev \
> +libspice-server-dev \
> +libssh2-1-dev \
> +libusb-1.0-0-dev \
> +libusbredirhost-dev \
> +libvdeplug-dev \
> +libvte-2.91-dev \
> +libxen-dev \
> +make \
> +python-yaml \
> +sparse \
> +texinfo \
> +xfslibs-dev
> +RUN apt-get update && \
> +apt-get -y install $PACKAGES
> +RUN dpkg -l $PACKAGES | sort > /packages.txt
> +ENV FEATURES clang pyyaml sdl2

Queued to testing/next, thanks.

--
Alex Bennée

Re: [Qemu-devel] [PULL 19/19] configure: automatically pick python3 is available

[Thomas Huth]

On 03/05/2019 02.41, Eduardo Habkost wrote:
> From: Daniel P. Berrangé 
> 
> Unless overridden via an env var or configure arg, QEMU will only look
> for the 'python' binary in $PATH. This is unhelpful on distros which
> are only shipping Python 3.x (eg Fedora) in their default install as,
> if they comply with PEP 394, the bare 'python' binary won't exist.
> 
> This changes configure so that by default it will search for all three
> common python binaries, preferring to find Python 3.x versions.
> 
> Signed-off-by: Daniel P. Berrangé 
> Message-Id: <20190327170701.23798-1-berra...@redhat.com>
> Signed-off-by: Eduardo Habkost 
> ---
>  configure | 18 +++---
>  1 file changed, 15 insertions(+), 3 deletions(-)

I haven't bisected it, but I think this patch here broke the gitlab-ci tests:

 https://gitlab.com/huth/qemu/-/jobs/206806257

Seems like the test is now failing when you don't have an UTF-8 locale:

 LANG=C make check-qapi-schema
 [...]
 TESTtests/qapi-schema/union-base-empty.out
 --- /builds/huth/qemu/tests/qapi-schema/unicode-str.err2019-05-03 
15:21:39.0 +
 +++ -  2019-05-03 15:42:01.561762978 +
 @@ -1 +1 @@
 -tests/qapi-schema/unicode-str.json:2: 'command' uses invalid name 'é'
 +tests/qapi-schema/unicode-str.json:2: 'command' uses invalid name '\xe9'
 /builds/huth/qemu/tests/Makefile.include:1105: recipe for target 
'check-tests/qapi-schema/unicode-str.json' failed
 make: *** [check-tests/qapi-schema/unicode-str.json] Error 1

Any ideas how to fix this?

 Thomas

[Qemu-devel] [Bug 1825359] Re: cpu_ld*_code() triggers MMU_DATA_LOAD i.s.o. MMU_INST_FETCH

[Peter Maydell]

Your patch is now in git master as commit ef5dae6805cce7b59d129 --
thanks!


** Changed in: qemu
   Status: Confirmed => Fix Committed

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

Title:
  cpu_ld*_code() triggers MMU_DATA_LOAD i.s.o. MMU_INST_FETCH

Status in QEMU:
  Fix Committed

Bug description:
  commit 377b155bde451d5ac545fbdcdfbf6ca17a4228f5
  Merge: c876180938 328eb60dc1
  Author: Peter Maydell ; masked for anti-spamming purposes
  Date:   Mon Mar 11 18:26:37 2019 +
  https://github.com/qemu/qemu/commit/377b155bde451d5ac545fbdcdfbf6ca17a4228f5
  --

  cpu_ld*_code() is used for loading code data as the name suggests. Although, 
it begins accessing memory with MMU_INST_FETCH access type, somewhere down
  the road, when the "io_readx(..., access_type=MMU_INST_FETCH, ...)" is
  called, it is ignoring this "access_type" while calling the "tlb_fill()"
  with a _hardcoded_ MMU_DATA_LOAD:

  cputlb.c
  
  static uint64_t io_readx(..., MMUAccessType access_type, ...)
  {

  if (recheck) {
  CPUTLBEntry *entry;
  target_ulong tlb_addr;

  tlb_fill(cpu, addr, size, MMU_DATA_LOAD, mmu_idx, retaddr);
  ...
  }
  

  This is an issue, because there can exist _small_ regions of memory (smaller
  than the TARGET_PAGE_SIZE) that are only executable and not readable.

  TL;DR

  What happens is at first, a "tlb_fill(..., access_type=MMU_INST_FETCH, ...)"
  is triggered by "tb_lookup_cpu_state()". To be precise, this is the call
  stack which is good behavior:
  ---
  #0  tlb_fill (cs=..., vaddr=684, size=0, access_type=MMU_INST_FETCH, 
mmu_idx=0, retaddr=0) at target/arc/mmu.c:602
  #1  get_page_addr_code (env=..., addr=684) at accel/tcg/cputlb.c:1045
  #2  tb_htable_lookup (cpu=..., pc=684, cs_base=0, flags=0, 
cf_mask=4278190080) at accel/tcg/cpu-exec.c:337
  #3  tb_lookup__cpu_state (cpu=..., pc=..., cs_base=..., flags=..., 
cf_mask=4278190080) at include/exec/tb-lookup.h:43
  #4  tb_find (cpu=..., last_tb=... , tb_exit=0, 
cf_mask=0) at accel/tcg/cpu-exec.c:404
  #5  cpu_exec (cpu=...) at accel/tcg/cpu-exec.c:729
  #6  tcg_cpu_exec (cpu=...) at cpus.c:1430
  #7  qemu_tcg_rr_cpu_thread_fn (arg=...) at cpus.c:1531
  #8  qemu_thread_start (args=...) at util/qemu-thread-posix.c:502
  ---

  After this call, TLB is filled with an entry that its size field is small,
  say 32 bytes. This causes a TLB_RECHECK for consequent memory accesses, which 
  is logical. However, in our decoder, we use cpu_lduw_code() to read the
  instructions and decode them. As mentioned, in the beginning, the
  access_type=MMU_INST_FETCH is lost in "io_readx()" while calling tlb_fill()",
  and now THIS CAUSES A GUEST EXCEPTION BECAUSE THAT REGION IS NOT ALLOWED TO
  BE READ. Here, comes that trace call of the _bad_ behavior:
  ---
  #0  tlb_fill (..., access_type=MMU_DATA_LOAD, ...) at target/arc/mmu.c:605
  #1  io_readx (..., access_type=MMU_INST_FETCH, size=2) at 
accel/tcg/cputlb.c:881
  #2  io_readw (..., access_type=MMU_INST_FETCH) at 
accel/tcg/softmmu_template.h:106
  #3  helper_le_ldw_cmmu (..., oi=16, retaddr=0) at 
accel/tcg/softmmu_template.h:146
  #4  cpu_lduw_code_ra (env=..., ptr=684, retaddr=0) at 
include/exec/cpu_ldst_template.h:102
  #5  cpu_lduw_code (env=..., ptr=684) at include/exec/cpu_ldst_template.h:114
  #6  read_and_decode_context (ctx=..., opcode_p=...) at 
target/arc/arc-decoder.c:1479
  #7  arc_decode (ctx=...) at target/arc/arc-decoder.c:1736
  #8  decode_opc (env=..., ctx=...) at target/arc/translate.c:313
  #9  arc_tr_translate_insn (dcbase=..., cpu=...) at target/arc/translate.c:335
  #10 translator_loop (.. ) at accel/tcg/translator.c:107
  #11 gen_intermediate_code (cpu=..., tb=... ) at 
target/arc/translate.c:413
  #12 tb_gen_code (cpu=..., pc=684, cs_base=0, flags=0, cflags=-16711679) at 
accel/tcg/translate-all.c:1723
  #13 tb_find (cpu=..., last_tb=... , tb_exit=0, 
cf_mask=0) at accel/tcg/cpu-exec.c:407
  #14 cpu_exec (cpu=...) at accel/tcg/cpu-exec.c:729
  #15 tcg_cpu_exec (cpu=...) at cpus.c:1430

  ---

  Do you confirm if this is an issue? Maybe there are other ways to read
  an instruction with MMU_INST_FETCH access that I don't know about.

  Last but not least, although this is not a security issue for QEMU per
  se, but it is hindering a security feature for the guest.

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

[Qemu-devel] [Bug 1824853] Re: 4.0.0-rc3 crashes with tcg/tcg.c:3952: tcg_gen_code: Assertion `s->gen_insn_end_off[num_insns] == off' failed

[Peter Maydell]

The fix should now be in git master (commits 8b86d6d25807e13a6 and
6e6c4efed995d9ec), so it will be in the 4.1 release.


** Changed in: qemu
   Status: In Progress => Fix Committed

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

Title:
  4.0.0-rc3 crashes with tcg/tcg.c:3952: tcg_gen_code: Assertion
  `s->gen_insn_end_off[num_insns] == off' failed

Status in QEMU:
  Fix Committed

Bug description:
  I tried to bootstrap and regtested gcc trunk (gcc svn rev 270278,
  datestamp 20190411) inside my arm64-gentoo installation under qemu-
  system-aarch64.

  Qemu version was 4.0.0-rc3 and -cpu cortex-a57. Qemu configured with
  only --target-list=aarch64-softmmu,aarch64-linux-user and compiled
  using gcc "version 5.5.0 20171010 (Ubuntu 5.5.0-12ubuntu1~16.04)".

  Executable created from gcc/testsuite/gcc.target/aarch64/advsimd-
  intrinsics/vldX.c compiled with -O2 crashed the whole qemu-system.

  To investigate a bit I also manually run
  ~/gcc/inst/trunk/bin/gcc 
~/gcc/src/trunk/gcc/testsuite/gcc.target/aarch64/advsimd-intrinsics/vldX.c
  with different options like:
  -O0 -lm -o d0.exe
  -O1 -lm -o d1.exe
  -O2 -lm -o d2.exe
  -O0 -static -lm -o s0.exe
  -O1 -static -lm -o s1.exe
  -O2 -static -lm -o s2.exe

  So, now I have 6 different arm64 executables created with different 
optimization levels. O0 and O1 versions run ok.
  Three sN.exe static executables I've also tried in qemu user mode (with same 
-cpu), no issue in user mode.

  And inside qemu-system I can see that
  running "d2.exe" (attached) gives:
  tcg/tcg.c:3952: tcg_gen_code: Assertion `s->gen_insn_end_off[num_insns] == 
off' failed.

  And running "s2.exe" gives:
  tcg/tcg.c:320: set_jmp_reset_offset: Assertion `s->tb_jmp_reset_offset[which] 
== off' failed.

  It seems like this test is an counter-example for logic that
  "tcg_ctx->nb_ops < 4000" implies tcg will fit into 16-bit signed size
  (see tcg_op_buf_full comments).

  Richard's changes in abebf92597186 and 9f754620651d were not enough, 
translation block must be smaller, or we have to find some proper way to bail 
out when buffer overflows.
  I don't know why this situation is not caught by code_gen_highwater logic in 
tcg.c

  I've also tried this "bail out" patch

  diff --git a/tcg/tcg.c b/tcg/tcg.c
  --- a/tcg/tcg.c
  +++ b/tcg/tcg.c
  @@ -3949,7 +3949,8 @@ int tcg_gen_code(TCGContext *s, TranslationBlock *tb)
   size_t off = tcg_current_code_size(s);
   s->gen_insn_end_off[num_insns] = off;
   /* Assert that we do not overflow our stored offset.  */
  -assert(s->gen_insn_end_off[num_insns] == off);
  +if (s->gen_insn_end_off[num_insns] != off)
  +  return -1;
   }
   num_insns++;
   for (i = 0; i < TARGET_INSN_START_WORDS; ++i) {

  But then running "d2.exe" just hangs the whole qemu-system. It seems
  that when tcg_gen_code return -1 (like in highwater logic mentioned
  before), we just re-call it again and again.

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

Re: [Qemu-devel] [PATCH v3] hw/usb/hcd-xhci: Fix GCC 9 build warning

[Alistair Francis]

On Fri, May 3, 2019 at 2:24 AM Daniel P. Berrangé  wrote:
>
> On Fri, May 03, 2019 at 12:42:04AM +, Alistair Francis wrote:
> > Fix this build warning with GCC 9 on Fedora 30:
> > hw/usb/hcd-xhci.c:3339:66: error: ‘%d’ directive output may be truncated 
> > writing between 1 and 10 bytes into a region of size 5 
> > [-Werror=format-truncation=]
> >  3339 | snprintf(port->name, sizeof(port->name), "usb2 port 
> > #%d", i+1);
> >   |  ^~
> > hw/usb/hcd-xhci.c:3339:54: note: directive argument in the range [1, 
> > 2147483647]
> >  3339 | snprintf(port->name, sizeof(port->name), "usb2 port 
> > #%d", i+1);
> >   |  ^~~
> > In file included from /usr/include/stdio.h:867,
> >  from /home/alistair/qemu/include/qemu/osdep.h:99,
> >  from hw/usb/hcd-xhci.c:21:
> > /usr/include/bits/stdio2.h:67:10: note: ‘__builtin___snprintf_chk’ output 
> > between 13 and 22 bytes into a destination of size 16
> >67 |   return __builtin___snprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 
> > 1,
> >   |  
> > ^~~~
> >68 |__bos (__s), __fmt, __va_arg_pack ());
> >   |~
> >
> > Signed-off-by: Alistair Francis 
> > Reviewed-by: Laurent Vivier 
> > Reviewed-by: Daniel P. Berrangé 
> > Reviewed-by: Philippe Mathieu-Daudé 
> > ---
> > This is the onl patch left if my original series "Fix some GCC 9 build
> > warnings" that hasn't either been accepeted into a maintainers tree or
> > fixed by someone else.
> >
> >  hw/usb/hcd-xhci.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
> > index ec28bee319..a15b103b65 100644
> > --- a/hw/usb/hcd-xhci.c
> > +++ b/hw/usb/hcd-xhci.c
> > @@ -3321,6 +3321,8 @@ static void usb_xhci_init(XHCIState *xhci)
> >
> >  usb_bus_new(>bus, sizeof(xhci->bus), _bus_ops, dev);
> >
> > +g_assert(usbports <= MAX(MAXPORTS_2, MAXPORTS_3));
> > +
> >  for (i = 0; i < usbports; i++) {
> >  speedmask = 0;
> >  if (i < xhci->numports_2) {
>
> Reviewed-by: Daniel P. Berrangé 
>
> but it looks like Gerd already sent a pull request with my patch for
> this from a few weeks back
>
> https://lists.gnu.org/archive/html/qemu-devel/2019-05/msg00543.html

No worries, I'm just glad it's fixed.

Alistair

>
> Regards,
> Daniel
> --
> |: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o-https://fstop138.berrange.com :|
> |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [RFC 1/3] target/m68k: In dump_address_map() check for memory access failures

[Laurent Vivier]

On 10/12/2018 17:56, Peter Maydell wrote:

In dump_address_map(), use address_space_ldl() instead of ldl_phys().
This allows us to check whether the memory access failed.

Signed-off-by: Peter Maydell 
---
  target/m68k/helper.c | 22 +++---
  1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/target/m68k/helper.c b/target/m68k/helper.c
index 917d46efcc3..374e4861886 100644
--- a/target/m68k/helper.c
+++ b/target/m68k/helper.c
@@ -411,6 +411,7 @@ static void dump_address_map(FILE *f, fprintf_function 
cpu_fprintf,
  int last_attr = -1, attr = -1;
  M68kCPU *cpu = m68k_env_get_cpu(env);
  CPUState *cs = CPU(cpu);
+MemTxResult txres;
  
  if (env->mmu.tcr & M68K_TCR_PAGE_8K) {

  /* 8k page */
@@ -424,22 +425,29 @@ static void dump_address_map(FILE *f, fprintf_function 
cpu_fprintf,
  tib_mask = M68K_4K_PAGE_MASK;
  }
  for (i = 0; i < M68K_ROOT_POINTER_ENTRIES; i++) {
-tia = ldl_phys(cs->as, M68K_POINTER_BASE(root_pointer) + i * 4);
-if (!M68K_UDT_VALID(tia)) {
+tia = address_space_ldl(cs->as, M68K_POINTER_BASE(root_pointer) + i * 
4,
+MEMTXATTRS_UNSPECIFIED, );
+if (txres != MEMTX_OK || !M68K_UDT_VALID(tia)) {
  continue;
  }
  for (j = 0; j < M68K_ROOT_POINTER_ENTRIES; j++) {
-tib = ldl_phys(cs->as, M68K_POINTER_BASE(tia) + j * 4);
-if (!M68K_UDT_VALID(tib)) {
+tib = address_space_ldl(cs->as, M68K_POINTER_BASE(tia) + j * 4,
+MEMTXATTRS_UNSPECIFIED, );
+if (txres != MEMTX_OK || !M68K_UDT_VALID(tib)) {
  continue;
  }
  for (k = 0; k < tic_size; k++) {
-tic = ldl_phys(cs->as, (tib & tib_mask) + k * 4);
-if (!M68K_PDT_VALID(tic)) {
+tic = address_space_ldl(cs->as, (tib & tib_mask) + k * 4,
+MEMTXATTRS_UNSPECIFIED, );
+if (txres != MEMTX_OK || !M68K_PDT_VALID(tic)) {
  continue;
  }
  if (M68K_PDT_INDIRECT(tic)) {
-tic = ldl_phys(cs->as, M68K_INDIRECT_POINTER(tic));
+tic = address_space_ldl(cs->as, M68K_INDIRECT_POINTER(tic),
+MEMTXATTRS_UNSPECIFIED, );
+if (txres != MEMTX_OK) {
+continue;
+}
  }
  
  last_logical = logical;




Reviewed-by: Laurent Vivier 

[Qemu-devel] [PATCH v2 4/4] 9p: use variable length suffixes for inode mapping

[Christian Schoenebeck via Qemu-devel]

This patch introduces variable length suffixes for being used for inode mapping
instead of the fixed 16 bit size prefixes of patch 1. With this patch the inode
numbers on guest will typically be much smaller (e.g. around >2^1 .. >2^7
instead of >2^48 with patch 1).

I preserved both solutions in this patch though, so you can switch between
them by adjusting P9_VARI_LENGTH_INODE_SUFFIXES, which probably also makes
code comparison between the two more easy for now.

Motivation: with patch 1 the inode numbers were so high that some network
services had issues. Especially SMB seems to be a problem again due to varying
implementations regarding SMB file IDs. I don't bother you with the details.

Additionally this solution should be more efficient, since inode numbers in
practice can take almost their entire 64 bit range on guest as well. Which
might also be beneficial for nested virtualization.

The "Exponential Golomb" algorithm is used as basis for generating the
variable length suffixes. The algorithm has a paramter k which controls the
distribution of bits on increasing indeces (minimum bits at low index vs.
maximum bits at high index). With k=0 the generated suffixes look like:

Index Dec/Bin -> Generated Suffix Bin
1 [1] -> [1] (1 bits)
2 [10] -> [010] (3 bits)
3 [11] -> [110] (3 bits)
4 [100] -> [00100] (5 bits)
5 [101] -> [10100] (5 bits)
6 [110] -> [01100] (5 bits)
7 [111] -> [11100] (5 bits)
8 [1000] -> [0001000] (7 bits)
9 [1001] -> [1001000] (7 bits)
10 [1010] -> [0101000] (7 bits)
11 [1011] -> [1101000] (7 bits)
12 [1100] -> [0011000] (7 bits)
...
65533 [1101] ->  [1011000] (31 bits)
65534 [1110] ->  [0111000] (31 bits)
65535 [] ->  [000] (31 bits)
Hence minBits=1 maxBits=31

And with k=5 they would look like:

Index Dec/Bin -> Generated Suffix Bin
1 [1] -> [01] (6 bits)
2 [10] -> [11] (6 bits)
3 [11] -> [010001] (6 bits)
4 [100] -> [110001] (6 bits)
5 [101] -> [001001] (6 bits)
6 [110] -> [101001] (6 bits)
7 [111] -> [011001] (6 bits)
8 [1000] -> [111001] (6 bits)
9 [1001] -> [000101] (6 bits)
10 [1010] -> [100101] (6 bits)
11 [1011] -> [010101] (6 bits)
12 [1100] -> [110101] (6 bits)
...
65533 [1101] -> [001110001000] (28 bits)
65534 [1110] -> [101110001000] (28 bits)
65535 [] -> [00001000] (28 bits)
Hence minBits=6 maxBits=28

If somebody wants to play around with the details of the suffix generation, let
me know and I will send you a small C source for you to play with.

Additionally this patch disables persistency of file IDs (that I introduced
with patch 3) at compile time by default for now. You can still enable it by
setting QPP_TABLE_PERSISTENCY_LIMIT to some reasonable high value (e.g. 127).
Reason: I am still unresolved what to do with this feature. The original
motivation was to avoid file ID collisions with network services if the machine
was interrupted for a short period. But what I did not consider was that
device IDs might change on host, so after loading the tables from fs the qp
tables would grow with irrelevant entries.

Signed-off-by: Christian Schoenebeck 
---
 hw/9pfs/9p.c | 744 ++-
 hw/9pfs/9p.h | 121 +-
 2 files changed, 805 insertions(+), 60 deletions(-)

diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 29c6dfc68a..003901a1e0 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -30,6 +30,10 @@
 #if defined(__linux__) /* TODO: This should probably go into osdep.h instead */
 # include  /* for XATTR_SIZE_MAX */
 #endif
+#include 
+#include 
+#include 
+#include 
 
 /*
  * How many bytes may we store to fs per extended attribute value?
@@ -585,6 +589,123 @@ static void coroutine_fn virtfs_reset(V9fsPDU *pdu)
 P9_STAT_MODE_NAMED_PIPE |   \
 P9_STAT_MODE_SOCKET)
 
+/* Mirrors all bits of a byte. So e.g. binary 1010 would become 0101. 
*/
+static inline uint8_t mirror8bit(uint8_t byte) {
+return (byte * 0x0202020202ULL & 0x010884422010ULL) % 1023;
+}
+
+/* Same as mirror8bit() just for a 64 bit data type instead for a byte. */
+static inline uint64_t mirror64bit(uint64_t value) {
+return ((uint64_t)mirror8bit( value& 0xff) << 56) |
+   ((uint64_t)mirror8bit((value >> 8)  & 0xff) << 48) |
+   ((uint64_t)mirror8bit((value >> 16) & 0xff) << 40) |
+   ((uint64_t)mirror8bit((value >> 24) & 0xff) << 32) |
+   ((uint64_t)mirror8bit((value >> 32) & 0xff) << 24) |
+   ((uint64_t)mirror8bit((value >> 40) & 0xff) << 16) |
+   ((uint64_t)mirror8bit((value >> 48) & 0xff) << 8 ) |
+   ((uint64_t)mirror8bit((value >> 56) & 0xff)  ) ;
+}
+
+#if P9_VARI_LENGTH_INODE_SUFFIXES
+
+/* Parameter k for the Exponential Golomb algorihm to be used.
+ *
+ * The smaller this value, the 

Re: [Qemu-devel] [PATCH v4] hw/virtio/virtio-mmio: Convert DPRINTF to trace and log

[LI, BO XUAN]

Hi Alex,

Sorry about that, I am still trying to get familiar with the patch
submission process. Since my patch has been changed from your last review,
I thought it would be safe to not include the r-b tag from last time. Will
take care next time!

Best regards,
Boxuan Li

On Sat, May 4, 2019 at 12:18 AM Alex Bennée  wrote:

>
> Boxuan Li  writes:
>
> > Use traces for debug message and qemu_log_mask for errors.
> >
> > Signed-off-by: Boxuan Li 
>
> You didn't add my r-b tags from last time. Anyway:
>
> Reviewed-by: Alex Bennée 
>
> > ---
> > v1:
> https://patchew.org/QEMU/20190428110258.86681-1-libox...@connect.hku.hk/
> > v2:
> https://patchew.org/QEMU/20190501081039.58938-1-libox...@connect.hku.hk/
> > v3:
> https://patchew.org/QEMU/20190503084654.18413-1-libox...@connect.hku.hk/
> > v4: Fix indentation and do not convert uint64_t to int
> > ---
> >  hw/virtio/trace-events  |  7 +++
> >  hw/virtio/virtio-mmio.c | 44
> +---
> >  2 files changed, 28 insertions(+), 23 deletions(-)
> >
> > diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
> > index 60c649c4bc..e28ba48da6 100644
> > --- a/hw/virtio/trace-events
> > +++ b/hw/virtio/trace-events
> > @@ -46,3 +46,10 @@ virtio_balloon_handle_output(const char *name,
> uint64_t gpa) "section name: %s g
> >  virtio_balloon_get_config(uint32_t num_pages, uint32_t actual)
> "num_pages: %d actual: %d"
> >  virtio_balloon_set_config(uint32_t actual, uint32_t oldactual) "actual:
> %d oldactual: %d"
> >  virtio_balloon_to_target(uint64_t target, uint32_t num_pages) "balloon
> target: 0x%"PRIx64" num_pages: %d"
> > +
> > +# virtio-mmio.c
> > +virtio_mmio_read(uint64_t offset) "virtio_mmio_read offset 0x%" PRIx64
> > +virtio_mmio_write_offset(uint64_t offset, uint64_t value)
> "virtio_mmio_write offset 0x%" PRIx64 " value 0x%" PRIx64
> > +virtio_mmio_guest_page(uint64_t size, int shift) "guest page size 0x%"
> PRIx64 " shift %d"
> > +virtio_mmio_queue_write(uint64_t value, int max_size) "mmio_queue write
> 0x%" PRIx64 " max %d"
> > +virtio_mmio_setting_irq(int level) "virtio_mmio setting IRQ %d"
> > diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c
> > index 5807aa87fe..96c762f0bf 100644
> > --- a/hw/virtio/virtio-mmio.c
> > +++ b/hw/virtio/virtio-mmio.c
> > @@ -27,16 +27,8 @@
> >  #include "sysemu/kvm.h"
> >  #include "hw/virtio/virtio-bus.h"
> >  #include "qemu/error-report.h"
> > -
> > -/* #define DEBUG_VIRTIO_MMIO */
> > -
> > -#ifdef DEBUG_VIRTIO_MMIO
> > -
> > -#define DPRINTF(fmt, ...) \
> > -do { printf("virtio_mmio: " fmt , ## __VA_ARGS__); } while (0)
> > -#else
> > -#define DPRINTF(fmt, ...) do {} while (0)
> > -#endif
> > +#include "qemu/log.h"
> > +#include "trace.h"
> >
> >  /* QOM macros */
> >  /* virtio-mmio-bus */
> > @@ -107,7 +99,7 @@ static uint64_t virtio_mmio_read(void *opaque, hwaddr
> offset, unsigned size)
> >  VirtIOMMIOProxy *proxy = (VirtIOMMIOProxy *)opaque;
> >  VirtIODevice *vdev = virtio_bus_get_device(>bus);
> >
> > -DPRINTF("virtio_mmio_read offset 0x%x\n", (int)offset);
> > +trace_virtio_mmio_read(offset);
> >
> >  if (!vdev) {
> >  /* If no backend is present, we treat most registers as
> > @@ -144,7 +136,9 @@ static uint64_t virtio_mmio_read(void *opaque,
> hwaddr offset, unsigned size)
> >  }
> >  }
> >  if (size != 4) {
> > -DPRINTF("wrong size access to register!\n");
> > +qemu_log_mask(LOG_GUEST_ERROR,
> > +  "%s: wrong size access to register!\n",
> > +  __func__);
> >  return 0;
> >  }
> >  switch (offset) {
> > @@ -182,10 +176,12 @@ static uint64_t virtio_mmio_read(void *opaque,
> hwaddr offset, unsigned size)
> >  case VIRTIO_MMIO_QUEUE_ALIGN:
> >  case VIRTIO_MMIO_QUEUE_NOTIFY:
> >  case VIRTIO_MMIO_INTERRUPT_ACK:
> > -DPRINTF("read of write-only register\n");
> > +qemu_log_mask(LOG_GUEST_ERROR,
> > +  "%s: read of write-only register\n",
> > +  __func__);
> >  return 0;
> >  default:
> > -DPRINTF("bad register offset\n");
> > +qemu_log_mask(LOG_GUEST_ERROR, "%s: bad register offset\n",
> __func__);
> >  return 0;
> >  }
> >  return 0;
> > @@ -197,8 +193,7 @@ static void virtio_mmio_write(void *opaque, hwaddr
> offset, uint64_t value,
> >  VirtIOMMIOProxy *proxy = (VirtIOMMIOProxy *)opaque;
> >  VirtIODevice *vdev = virtio_bus_get_device(>bus);
> >
> > -DPRINTF("virtio_mmio_write offset 0x%x value 0x%" PRIx64 "\n",
> > -(int)offset, value);
> > +trace_virtio_mmio_write_offset(offset, value);
> >
> >  if (!vdev) {
> >  /* If no backend is present, we just make all registers
> > @@ -226,7 +221,9 @@ static void virtio_mmio_write(void *opaque, hwaddr
> offset, uint64_t value,
> >  return;
> >  }
> >  if (size != 4) {
> > -DPRINTF("wrong size access to register!\n");
> > +  

[Qemu-devel] [PATCH v2] block/rbd: increase dynamically the image size

[Stefano Garzarella]

RBD APIs don't allow us to write more than the size set with
rbd_create() or rbd_resize().
In order to support growing images (eg. qcow2), we resize the
image before write operations that exceed the current size.

Signed-off-by: Stefano Garzarella 
---
v2:
  - use bs->total_sectors instead of adding a new field [Kevin]
  - resize the image only during write operation [Kevin]
for read operation, the bdrv_aligned_preadv() already handles reads
that exceed the length returned by bdrv_getlength(), so IMHO we can
avoid to handle it in the rbd driver
---
 block/rbd.c | 14 +-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/block/rbd.c b/block/rbd.c
index 0c549c9935..613e8f4982 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -934,13 +934,25 @@ static BlockAIOCB *rbd_start_aio(BlockDriverState *bs,
 }
 
 switch (cmd) {
-case RBD_AIO_WRITE:
+case RBD_AIO_WRITE: {
+/*
+ * RBD APIs don't allow us to write more than actual size, so in order
+ * to support growing images, we resize the image before write
+ * operations that exceed the current size.
+ */
+if (off + size > bs->total_sectors * BDRV_SECTOR_SIZE) {
+r = rbd_resize(s->image, off + size);
+if (r < 0) {
+goto failed_completion;
+}
+}
 #ifdef LIBRBD_SUPPORTS_IOVEC
 r = rbd_aio_writev(s->image, qiov->iov, qiov->niov, off, c);
 #else
 r = rbd_aio_write(s->image, off, size, rcb->buf, c);
 #endif
 break;
+}
 case RBD_AIO_READ:
 #ifdef LIBRBD_SUPPORTS_IOVEC
 r = rbd_aio_readv(s->image, qiov->iov, qiov->niov, off, c);
-- 
2.20.1

Re: [Qemu-devel] [PATCH v3 1/4] QEMU_PACKED: Remove gcc_struct attribute in Windows non x86 targets

[Philippe Mathieu-Daudé]

On 5/3/19 5:56 PM, Marc-André Lureau wrote:
> Hi
> 
> Le ven. 3 mai 2019 à 17:23, Peter Maydell  a
> écrit :
> 
>> On Fri, 3 May 2019 at 06:07, Thomas Huth  wrote:
>>>
>>> On 03/05/2019 02.36, Cao Jiaxi wrote:
 gcc_struct is for x86 only, and it generates an warning on ARM64
>> Clang/MinGW targets.

 Signed-off-by: Cao Jiaxi 
 ---
  contrib/libvhost-user/libvhost-user.h | 2 +-
  include/qemu/compiler.h   | 2 +-
  scripts/cocci-macro-file.h| 7 ++-
  slirp/src/util.h  | 2 +-
  4 files changed, 9 insertions(+), 4 deletions(-)
>>
 diff --git a/slirp/src/util.h b/slirp/src/util.h
 index 01f1e0e068..278828fe3f 100644
 --- a/slirp/src/util.h
 +++ b/slirp/src/util.h
 @@ -43,7 +43,7 @@
  #include 
  #endif

 -#if defined(_WIN32)
 +#if defined(_WIN32) && (defined(__x86_64__) || defined(__i386__))
  # define SLIRP_PACKED __attribute__((gcc_struct, packed))
  #else
  # define SLIRP_PACKED __attribute__((packed))

>>>
>>> The slirp code is currently on its way into a separate module, so you
>>> might need to provide that hunk to the libslirp folks again... I'm
>>> putting the slirp maintainers on CC:, maybe they can pick it up from
>> here.
>>
>> Yes, the slirp module has now landed in master, so this patch
>> definitely needs to be split into two. I've kept in my
>> target-arm.next tree the parts which are applicable to
>> the QEMU repo itself (ie everything except the slirp/ change),
>> so we just need a new patch for the slirp submodule part.
>>
>> Marc-André, Samuel -- what's the process for submitting and
>> getting reviewed changes to the slirp submodule now it's a
>> separate project ?
>>
> 
> It's hosted on gitlab.freedesktop.org, with some CI. It's fine to send
> patches on qemu devel, as long as Samuel or I are in CC. But in the long
> term, I think gitlab MR will be favoured (after a while using it, I think
> gitlab is better than ML for patches & bug tracking tbh)

Except when working offline (or with poor intermittent link).

Re: [Qemu-devel] Following up questions related to QEMU and I/O Thread

[Wei Li]

Got it, thanks Stefan for your clarification!

Wei

On 5/1/19, 9:36 AM, "Stefan Hajnoczi"  wrote:

On Mon, Apr 29, 2019 at 10:56:31AM -0700, Wei Li wrote:
>Does this mean the performance could be improved via adding Batch I/O 
submission support in Guest driver side which will be able to reduce the number 
of virtqueue kicks?

Yes, I think so.  It's not obvious to me how a Linux SCSI driver is
supposed to implement batching though.  The .queuecommand API doesn't
seem to include information relevant to batching.

Stefan

[Qemu-devel] [PATCH v2 3/4] 9p: persistency of QID path beyond reboots / suspensions

[Christian Schoenebeck via Qemu-devel]

This patch aims to keep QID path identical beyond the scope of reboots and
guest suspensions. With the 1st patch alone the QID path of the same files
might change after reboots / suspensions, since 9p would restart with
empty qpp_table and the resulting QID path depends on the precise sequence
of files being accessed on guest.

The first patch should already avoid the vast majority of potential QID
path collisions. However especially network services running on guest
would still be prone to QID path issues when just using the 1st patch.
For instance Samba is exporting file IDs to clients in the network and
SMB cliens in the network will use those IDs to access and request
changes on the file server. If guest is now interrupted in between, like
it commonly happens on maintenance, e.g. software updates on host, then
SMB clients in the network will continue working with old file IDs, which
in turn leads to data corruption and data loss on the file server.
Furthermore on SMB client side I also encountered severe misbehaviours in
this case, for instance Macs accessing the file server would either
start to hang or die with a kernel panic within seconds, since the smbx
implementation on macOS heavily relies on file IDs being unique (within
the context of a connection that is).

So this patch here mitigates the remaining problem described above by
storing the qpp_table persistently as extended attribute(s) on the
exported root of the file system and automatically tries to restore the
qpp_table i.e. after reboots / resumptions.

This patch is aimed at real world scenarios, in which qpp_table will only
ever get few dozens of entries (and none ever in qpf_table). So it is e.g.
intentionally limited to only store qpp_table, not qpf_table; and so far
I have not made optimizations, since in practice the qpf_table is really
just tiny.

Since there is currently no callback in qemu yet that would reliably be
called on guest shutdowns, the table is stored on every new insertion for
now.

Signed-off-by: Christian Schoenebeck 
---
 hw/9pfs/9p.c | 315 ++-
 hw/9pfs/9p.h |  33 +++
 2 files changed, 343 insertions(+), 5 deletions(-)

diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 2b893e25a1..29c6dfc68a 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -26,6 +26,19 @@
 #include "migration/blocker.h"
 #include "sysemu/qtest.h"
 #include "qemu/xxhash.h"
+#include "qemu/crc32c.h"
+#if defined(__linux__) /* TODO: This should probably go into osdep.h instead */
+# include  /* for XATTR_SIZE_MAX */
+#endif
+
+/*
+ * How many bytes may we store to fs per extended attribute value?
+ */
+#ifdef XATTR_SIZE_MAX
+# define ATTR_MAX_SIZE XATTR_SIZE_MAX /* Linux only: 64kB limit in kernel */
+#else
+# define ATTR_MAX_SIZE 65536 /* Most systems allow a bit more, so we take this 
as basis.  */
+#endif
 
 int open_fd_hw;
 int total_open_fd;
@@ -642,6 +655,285 @@ static int qid_path_fullmap(V9fsPDU *pdu, const struct 
stat *stbuf,
 return 0;
 }
 
+static inline bool is_ro_export(FsContext *ctx)
+{
+return ctx->export_flags & V9FS_RDONLY;
+}
+
+/*
+ * Once qpp_table size exceeds this value, we no longer save
+ * the table persistently. See comment in v9fs_store_qpp_table()
+ */
+#define QPP_TABLE_PERSISTENCY_LIMIT 32768
+
+/* Remove all user.virtfs.system.qidp.* xattrs from export root. */
+static void remove_qidp_xattr(FsContext *ctx)
+{
+V9fsString name;
+int i;
+
+/* just for a paranoid endless recursion sanity check */
+const ssize_t max_size =
+sizeof(QppSrlzHeader) +
+QPP_TABLE_PERSISTENCY_LIMIT * sizeof(QppEntryS);
+
+v9fs_string_init();
+for (i = 0; i * ATTR_MAX_SIZE < max_size; ++i) {
+v9fs_string_sprintf(, "user.virtfs.system.qidp.%d", i);
+if (lremovexattr(ctx->fs_root, name.data) < 0)
+break;
+}
+v9fs_string_free();
+}
+
+/* Used to convert qpp hash table into continuous stream. */
+static void qpp_table_serialize(void *p, uint32_t h, void *up)
+{
+const QppEntry *entry = (const QppEntry*) p;
+QppSerialize *ser = (QppSerialize*) up;
+
+if (ser->error)
+return;
+
+/* safety first */
+if (entry->qp_prefix - 1 >= ser->count) {
+ser->error = -1;
+return;
+}
+
+ser->elements[entry->qp_prefix - 1] = (QppEntryS) {
+.dev = entry->dev,
+.ino_prefix = entry->ino_prefix
+};
+ser->done++;
+}
+
+/*
+ * Tries to store the current qpp_table as extended attribute(s) on the
+ * exported file system root with the goal to preserve identical qids
+ * beyond the scope of reboots.
+ */
+static void v9fs_store_qpp_table(V9fsState *s)
+{
+FsContext *ctx = >ctx;
+V9fsString name;
+int i, res;
+size_t size;
+QppSrlzStream* stream;
+QppSerialize ser;
+
+if (is_ro_export(ctx))
+return;
+
+/*
+ * Whenever we exceeded some certain (arbitrary) high qpp_table size we
+ * delete the stored table from the file 

[Qemu-devel] [PATCH v2 2/4] 9P: trivial cleanup of QID path collision mitigation

[Christian Schoenebeck via Qemu-devel]

Addresses trivial changes regarding the previous patch as requested
on the mailing list a while ago.

* Removed unneccessary parantheses:
  https://lists.gnu.org/archive/html/qemu-devel/2018-02/msg02661.html

* Removed unneccessary g_malloc() result checks:
  https://lists.gnu.org/archive/html/qemu-devel/2018-02/msg02814.html

* Unsigned type changes:
  https://lists.gnu.org/archive/html/qemu-devel/2018-02/msg02581.html

Signed-off-by: Christian Schoenebeck 
---
 fsdev/9p-marshal.h   |  2 +-
 hw/9pfs/9p.c | 16 +---
 hw/9pfs/trace-events | 14 +++---
 3 files changed, 13 insertions(+), 19 deletions(-)

diff --git a/fsdev/9p-marshal.h b/fsdev/9p-marshal.h
index d1ad3645c4..8f3babb60a 100644
--- a/fsdev/9p-marshal.h
+++ b/fsdev/9p-marshal.h
@@ -9,7 +9,7 @@ typedef struct V9fsString
 
 typedef struct V9fsQID
 {
-int8_t type;
+uint8_t type;
 uint32_t version;
 uint64_t path;
 } V9fsQID;
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index b9bbdcbaee..2b893e25a1 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -587,13 +587,13 @@ static uint32_t qpf_hash(QpfEntry e)
 static bool qpp_cmp_func(const void *obj, const void *userp)
 {
 const QppEntry *e1 = obj, *e2 = userp;
-return (e1->dev == e2->dev) && (e1->ino_prefix == e2->ino_prefix);
+return e1->dev == e2->dev && e1->ino_prefix == e2->ino_prefix;
 }
 
 static bool qpf_cmp_func(const void *obj, const void *userp)
 {
 const QpfEntry *e1 = obj, *e2 = userp;
-return (e1->dev == e2->dev) && (e1->ino == e2->ino);
+return e1->dev == e2->dev && e1->ino == e2->ino;
 }
 
 static void qp_table_remove(void *p, uint32_t h, void *up)
@@ -630,9 +630,6 @@ static int qid_path_fullmap(V9fsPDU *pdu, const struct stat 
*stbuf,
 }
 
 val = g_malloc0(sizeof(QppEntry));
-if (!val) {
-return -ENOMEM;
-}
 *val = lookup;
 
 /* new unique inode and device combo */
@@ -673,9 +670,6 @@ static int qid_path_prefixmap(V9fsPDU *pdu, const struct 
stat *stbuf,
 }
 
 val = g_malloc0(sizeof(QppEntry));
-if (!val) {
-return -ENOMEM;
-}
 *val = lookup;
 
 /* new unique inode prefix and device combo */
@@ -870,9 +864,9 @@ static int donttouch_stat(V9fsStat *stat)
 {
 if (stat->type == -1 &&
 stat->dev == -1 &&
-stat->qid.type == -1 &&
-stat->qid.version == -1 &&
-stat->qid.path == -1 &&
+stat->qid.type == 0xff &&
+stat->qid.version == (uint32_t) -1 &&
+stat->qid.path == (uint64_t) -1 &&
 stat->mode == -1 &&
 stat->atime == -1 &&
 stat->mtime == -1 &&
diff --git a/hw/9pfs/trace-events b/hw/9pfs/trace-events
index c0a0a4ab5d..6964756922 100644
--- a/hw/9pfs/trace-events
+++ b/hw/9pfs/trace-events
@@ -6,7 +6,7 @@ v9fs_rerror(uint16_t tag, uint8_t id, int err) "tag %d id %d 
err %d"
 v9fs_version(uint16_t tag, uint8_t id, int32_t msize, char* version) "tag %d 
id %d msize %d version %s"
 v9fs_version_return(uint16_t tag, uint8_t id, int32_t msize, char* version) 
"tag %d id %d msize %d version %s"
 v9fs_attach(uint16_t tag, uint8_t id, int32_t fid, int32_t afid, char* uname, 
char* aname) "tag %u id %u fid %d afid %d uname %s aname %s"
-v9fs_attach_return(uint16_t tag, uint8_t id, int8_t type, int32_t version, 
int64_t path) "tag %d id %d type %d version %d path %"PRId64
+v9fs_attach_return(uint16_t tag, uint8_t id, uint8_t type, uint32_t version, 
uint64_t path) "tag %d id %d type %d version %d path %"PRId64
 v9fs_stat(uint16_t tag, uint8_t id, int32_t fid) "tag %d id %d fid %d"
 v9fs_stat_return(uint16_t tag, uint8_t id, int32_t mode, int32_t atime, 
int32_t mtime, int64_t length) "tag %d id %d stat={mode %d atime %d mtime %d 
length %"PRId64"}"
 v9fs_getattr(uint16_t tag, uint8_t id, int32_t fid, uint64_t request_mask) 
"tag %d id %d fid %d request_mask %"PRIu64
@@ -14,9 +14,9 @@ v9fs_getattr_return(uint16_t tag, uint8_t id, uint64_t 
result_mask, uint32_t mod
 v9fs_walk(uint16_t tag, uint8_t id, int32_t fid, int32_t newfid, uint16_t 
nwnames) "tag %d id %d fid %d newfid %d nwnames %d"
 v9fs_walk_return(uint16_t tag, uint8_t id, uint16_t nwnames, void* qids) "tag 
%d id %d nwnames %d qids %p"
 v9fs_open(uint16_t tag, uint8_t id, int32_t fid, int32_t mode) "tag %d id %d 
fid %d mode %d"
-v9fs_open_return(uint16_t tag, uint8_t id, int8_t type, int32_t version, 
int64_t path, int iounit) "tag %d id %d qid={type %d version %d path %"PRId64"} 
iounit %d"
+v9fs_open_return(uint16_t tag, uint8_t id, uint8_t type, uint32_t version, 
uint64_t path, int iounit) "tag %d id %d qid={type %d version %d path 
%"PRId64"} iounit %d"
 v9fs_lcreate(uint16_t tag, uint8_t id, int32_t dfid, int32_t flags, int32_t 
mode, uint32_t gid) "tag %d id %d dfid %d flags %d mode %d gid %u"
-v9fs_lcreate_return(uint16_t tag, uint8_t id, int8_t type, int32_t version, 
int64_t path, int32_t iounit) "tag %d id %d qid={type %d version %d path 
%"PRId64"} iounit %d"

[Qemu-devel] [PATCH v2 0/4] 9p: Fix file ID collisions

[Christian Schoenebeck via Qemu-devel]

Hi!

This is v2 of a proposed patch set for fixing file ID collisions with 9pfs.

Patch 1 to 3 are identical to the previous version. New in this v2 is patch 4
which introduces variable length suffixes for inode mapping instead of fixed
length prefixes.

Also: patch 4 disables file ID persistency at compile time by default for now,
since I am yet unresolved about details of that persistency.

Christian Schoenebeck (4):
  9p: mitigates most QID path collisions
  9P: trivial cleanup of QID path collision mitigation
  9p: persistency of QID path beyond reboots / suspensions
  9p: use variable length suffixes for inode mapping

 fsdev/9p-marshal.h   |6 +-
 hw/9pfs/9p.c | 1145 --
 hw/9pfs/9p.h |  167 
 hw/9pfs/trace-events |   14 +-
 4 files changed, 1296 insertions(+), 36 deletions(-)

-- 
2.11.0

[Qemu-devel] [PATCH v2 1/4] 9p: mitigates most QID path collisions

[Christian Schoenebeck via Qemu-devel]

This first patch here is an updated version of Antonios Motakis'
original 4-patch set, merged to one patch:

https://lists.gnu.org/archive/html/qemu-devel/2018-02/msg02283.html

* Updated to latest git master, specifically to new qht interface.

* Merged the original 4 patches to this single patch.

Signed-off-by: Christian Schoenebeck 
---
 fsdev/9p-marshal.h |   4 +-
 hw/9pfs/9p.c   | 200 -
 hw/9pfs/9p.h   |  21 ++
 3 files changed, 204 insertions(+), 21 deletions(-)

diff --git a/fsdev/9p-marshal.h b/fsdev/9p-marshal.h
index c8823d878f..d1ad3645c4 100644
--- a/fsdev/9p-marshal.h
+++ b/fsdev/9p-marshal.h
@@ -10,8 +10,8 @@ typedef struct V9fsString
 typedef struct V9fsQID
 {
 int8_t type;
-int32_t version;
-int64_t path;
+uint32_t version;
+uint64_t path;
 } V9fsQID;
 
 typedef struct V9fsStat
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 55821343e5..b9bbdcbaee 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -25,6 +25,7 @@
 #include "trace.h"
 #include "migration/blocker.h"
 #include "sysemu/qtest.h"
+#include "qemu/xxhash.h"
 
 int open_fd_hw;
 int total_open_fd;
@@ -571,14 +572,135 @@ static void coroutine_fn virtfs_reset(V9fsPDU *pdu)
 P9_STAT_MODE_NAMED_PIPE |   \
 P9_STAT_MODE_SOCKET)
 
-/* This is the algorithm from ufs in spfs */
-static void stat_to_qid(const struct stat *stbuf, V9fsQID *qidp)
+
+/* creative abuse of qemu_xxhash7, which is based on xxhash */
+static uint32_t qpp_hash(QppEntry e)
 {
-size_t size;
+return qemu_xxhash7(e.ino_prefix, e.dev, 0, 0, 0);
+}
+
+static uint32_t qpf_hash(QpfEntry e)
+{
+return qemu_xxhash7(e.ino, e.dev, 0, 0, 0);
+}
+
+static bool qpp_cmp_func(const void *obj, const void *userp)
+{
+const QppEntry *e1 = obj, *e2 = userp;
+return (e1->dev == e2->dev) && (e1->ino_prefix == e2->ino_prefix);
+}
+
+static bool qpf_cmp_func(const void *obj, const void *userp)
+{
+const QpfEntry *e1 = obj, *e2 = userp;
+return (e1->dev == e2->dev) && (e1->ino == e2->ino);
+}
+
+static void qp_table_remove(void *p, uint32_t h, void *up)
+{
+g_free(p);
+}
+
+static void qp_table_destroy(struct qht *ht)
+{
+qht_iter(ht, qp_table_remove, NULL);
+qht_destroy(ht);
+}
+
+static int qid_path_fullmap(V9fsPDU *pdu, const struct stat *stbuf,
+uint64_t *path)
+{
+QpfEntry lookup = {
+.dev = stbuf->st_dev,
+.ino = stbuf->st_ino
+}, *val;
+uint32_t hash = qpf_hash(lookup);
+
+/* most users won't need the fullmap, so init the table lazily */
+if (!pdu->s->qpf_table.map) {
+qht_init(>s->qpf_table, qpf_cmp_func, 1 << 16, 
QHT_MODE_AUTO_RESIZE);
+}
+
+val = qht_lookup(>s->qpf_table, , hash);
+
+if (!val) {
+if (pdu->s->qp_fullpath_next == 0) {
+/* no more files can be mapped :'( */
+return -ENFILE;
+}
+
+val = g_malloc0(sizeof(QppEntry));
+if (!val) {
+return -ENOMEM;
+}
+*val = lookup;
+
+/* new unique inode and device combo */
+val->path = pdu->s->qp_fullpath_next++;
+pdu->s->qp_fullpath_next &= QPATH_INO_MASK;
+qht_insert(>s->qpf_table, val, hash, NULL);
+}
+
+*path = val->path;
+return 0;
+}
+
+/* stat_to_qid needs to map inode number (64 bits) and device id (32 bits)
+ * to a unique QID path (64 bits). To avoid having to map and keep track
+ * of up to 2^64 objects, we map only the 16 highest bits of the inode plus
+ * the device id to the 16 highest bits of the QID path. The 48 lowest bits
+ * of the QID path equal to the lowest bits of the inode number.
+ *
+ * This takes advantage of the fact that inode number are usually not
+ * random but allocated sequentially, so we have fewer items to keep
+ * track of.
+ */
+static int qid_path_prefixmap(V9fsPDU *pdu, const struct stat *stbuf,
+uint64_t *path)
+{
+QppEntry lookup = {
+.dev = stbuf->st_dev,
+.ino_prefix = (uint16_t) (stbuf->st_ino >> 48)
+}, *val;
+uint32_t hash = qpp_hash(lookup);
+
+val = qht_lookup(>s->qpp_table, , hash);
+
+if (!val) {
+if (pdu->s->qp_prefix_next == 0) {
+/* we ran out of prefixes */
+return -ENFILE;
+}
+
+val = g_malloc0(sizeof(QppEntry));
+if (!val) {
+return -ENOMEM;
+}
+*val = lookup;
+
+/* new unique inode prefix and device combo */
+val->qp_prefix = pdu->s->qp_prefix_next++;
+qht_insert(>s->qpp_table, val, hash, NULL);
+}
+
+*path = ((uint64_t)val->qp_prefix << 48) | (stbuf->st_ino & 
QPATH_INO_MASK);
+return 0;
+}
+
+static int stat_to_qid(V9fsPDU *pdu, const struct stat *stbuf, V9fsQID *qidp)
+{
+int err;
+
+/* map inode+device to qid path (fast path) */
+err = qid_path_prefixmap(pdu, stbuf, >path);
+if (err == 

Re: [Qemu-devel] [PATCH] qxl: fix -Waddress-of-packed-member

[Philippe Mathieu-Daudé]

On 4/8/19 10:12 PM, Marc-André Lureau wrote:
> The GCC9 compiler complains about QXL code that takes the address of
> members of the 'struct QXLReleaseRing' which is marked packed:
> 
>   CC  hw/display/qxl.o
> /home/elmarco/src/qemu/hw/display/qxl.c: In function ‘init_qxl_ram’:
> /home/elmarco/src/qemu/hw/display/qxl.c:50:19: warning: taking address of 
> packed member of ‘struct QXLReleaseRing_ring_el’ may result in an unaligned 
> pointer value [-Waddress-of-packed-member]
>50 | ret = &(r)->items[prod].el;   
>   \
>   |   ^~~~
> /home/elmarco/src/qemu/hw/display/qxl.c:429:5: note: in expansion of macro 
> ‘SPICE_RING_PROD_ITEM’
>   429 | SPICE_RING_PROD_ITEM(d, >ram->release_ring, item);
>   | ^~~~
> /home/elmarco/src/qemu/hw/display/qxl.c: In function ‘qxl_push_free_res’:
> /home/elmarco/src/qemu/hw/display/qxl.c:50:19: warning: taking address of 
> packed member of ‘struct QXLReleaseRing_ring_el’ may result in an unaligned 
> pointer value [-Waddress-of-packed-member]
>50 | ret = &(r)->items[prod].el;   
>   \
>   |   ^~~~
> /home/elmarco/src/qemu/hw/display/qxl.c:762:5: note: in expansion of macro 
> ‘SPICE_RING_PROD_ITEM’
>   762 | SPICE_RING_PROD_ITEM(d, ring, item);
>   | ^~~~
> /home/elmarco/src/qemu/hw/display/qxl.c: In function 
> ‘interface_release_resource’:
> /home/elmarco/src/qemu/hw/display/qxl.c:50:19: warning: taking address of 
> packed member of ‘struct QXLReleaseRing_ring_el’ may result in an unaligned 
> pointer value [-Waddress-of-packed-member]
>50 | ret = &(r)->items[prod].el;   
>   \
>   |   ^~~~
> /home/elmarco/src/qemu/hw/display/qxl.c:795:5: note: in expansion of macro 
> ‘SPICE_RING_PROD_ITEM’
>   795 | SPICE_RING_PROD_ITEM(qxl, ring, item);
>   | ^~~~
> 
> Replace pointer usage by direct structure/array access instead.
> 
> Signed-off-by: Marc-André Lureau 

Tested-by: Philippe Mathieu-Daudé 

> ---
>  hw/display/qxl.c | 83 +---
>  1 file changed, 50 insertions(+), 33 deletions(-)
> 
> diff --git a/hw/display/qxl.c b/hw/display/qxl.c
> index c8ce5781e0..12d83dd6f1 100644
> --- a/hw/display/qxl.c
> +++ b/hw/display/qxl.c
> @@ -39,29 +39,49 @@
>   * abort we just qxl_set_guest_bug and set the return to NULL. Still
>   * it may happen as a result of emulator bug as well.
>   */
> -#undef SPICE_RING_PROD_ITEM
> -#define SPICE_RING_PROD_ITEM(qxl, r, ret) { \
> -uint32_t prod = (r)->prod & SPICE_RING_INDEX_MASK(r);   \
> -if (prod >= ARRAY_SIZE((r)->items)) {   \
> -qxl_set_guest_bug(qxl, "SPICE_RING_PROD_ITEM indices mismatch " \
> -  "%u >= %zu", prod, ARRAY_SIZE((r)->items));   \
> -ret = NULL; \
> -} else {\
> -ret = &(r)->items[prod].el; \
> -}   \
> +#define SPICE_RING_GET_CHECK(qxl, r, field) ({  \
> +field = (r)->field & SPICE_RING_INDEX_MASK(r);  \
> +bool mismatch = field >= ARRAY_SIZE((r)->items);\
> +if (mismatch) { \
> +qxl_set_guest_bug(qxl, "SPICE_RING_GET %s indices mismatch "\
> +  "%u >= %zu", stringify(field), field, \
> +  ARRAY_SIZE((r)->items));  \
> +}   \
> +!mismatch;  \
> +})
> +
> +static inline uint64_t
> +qxl_release_ring_get_prod(PCIQXLDevice *qxl)
> +{
> +struct QXLReleaseRing *ring = >ram->release_ring;
> +uint32_t prod;
> +bool ok = SPICE_RING_GET_CHECK(qxl, ring, prod);
> +assert(ok);
> +
> +return ring->items[prod].el;
> +}
> +
> +static inline bool
> +qxl_release_ring_set_prod(PCIQXLDevice *qxl, uint64_t val)
> +{
> +struct QXLReleaseRing *ring = >ram->release_ring;
> +uint32_t prod;
> +bool ok = SPICE_RING_GET_CHECK(qxl, ring, prod);
> +if (ok) {
> +ring->items[prod].el = val;
>  }
> +return ok;
> +}
>  
>  #undef SPICE_RING_CONS_ITEM
> -#define SPICE_RING_CONS_ITEM(qxl, r, ret) { \
> -uint32_t cons = (r)->cons & SPICE_RING_INDEX_MASK(r);   \
> -if (cons >= ARRAY_SIZE((r)->items)) {   \
> -qxl_set_guest_bug(qxl, "SPICE_RING_CONS_ITEM indices mismatch " \
> -

Re: [Qemu-devel] [PATCH v4] hw/virtio/virtio-mmio: Convert DPRINTF to trace and log

[Alex Bennée]

Boxuan Li  writes:

> Use traces for debug message and qemu_log_mask for errors.
>
> Signed-off-by: Boxuan Li 

You didn't add my r-b tags from last time. Anyway:

Reviewed-by: Alex Bennée 

> ---
> v1: https://patchew.org/QEMU/20190428110258.86681-1-libox...@connect.hku.hk/
> v2: https://patchew.org/QEMU/20190501081039.58938-1-libox...@connect.hku.hk/
> v3: https://patchew.org/QEMU/20190503084654.18413-1-libox...@connect.hku.hk/
> v4: Fix indentation and do not convert uint64_t to int
> ---
>  hw/virtio/trace-events  |  7 +++
>  hw/virtio/virtio-mmio.c | 44 +---
>  2 files changed, 28 insertions(+), 23 deletions(-)
>
> diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
> index 60c649c4bc..e28ba48da6 100644
> --- a/hw/virtio/trace-events
> +++ b/hw/virtio/trace-events
> @@ -46,3 +46,10 @@ virtio_balloon_handle_output(const char *name, uint64_t 
> gpa) "section name: %s g
>  virtio_balloon_get_config(uint32_t num_pages, uint32_t actual) "num_pages: 
> %d actual: %d"
>  virtio_balloon_set_config(uint32_t actual, uint32_t oldactual) "actual: %d 
> oldactual: %d"
>  virtio_balloon_to_target(uint64_t target, uint32_t num_pages) "balloon 
> target: 0x%"PRIx64" num_pages: %d"
> +
> +# virtio-mmio.c
> +virtio_mmio_read(uint64_t offset) "virtio_mmio_read offset 0x%" PRIx64
> +virtio_mmio_write_offset(uint64_t offset, uint64_t value) "virtio_mmio_write 
> offset 0x%" PRIx64 " value 0x%" PRIx64
> +virtio_mmio_guest_page(uint64_t size, int shift) "guest page size 0x%" 
> PRIx64 " shift %d"
> +virtio_mmio_queue_write(uint64_t value, int max_size) "mmio_queue write 0x%" 
> PRIx64 " max %d"
> +virtio_mmio_setting_irq(int level) "virtio_mmio setting IRQ %d"
> diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c
> index 5807aa87fe..96c762f0bf 100644
> --- a/hw/virtio/virtio-mmio.c
> +++ b/hw/virtio/virtio-mmio.c
> @@ -27,16 +27,8 @@
>  #include "sysemu/kvm.h"
>  #include "hw/virtio/virtio-bus.h"
>  #include "qemu/error-report.h"
> -
> -/* #define DEBUG_VIRTIO_MMIO */
> -
> -#ifdef DEBUG_VIRTIO_MMIO
> -
> -#define DPRINTF(fmt, ...) \
> -do { printf("virtio_mmio: " fmt , ## __VA_ARGS__); } while (0)
> -#else
> -#define DPRINTF(fmt, ...) do {} while (0)
> -#endif
> +#include "qemu/log.h"
> +#include "trace.h"
>
>  /* QOM macros */
>  /* virtio-mmio-bus */
> @@ -107,7 +99,7 @@ static uint64_t virtio_mmio_read(void *opaque, hwaddr 
> offset, unsigned size)
>  VirtIOMMIOProxy *proxy = (VirtIOMMIOProxy *)opaque;
>  VirtIODevice *vdev = virtio_bus_get_device(>bus);
>
> -DPRINTF("virtio_mmio_read offset 0x%x\n", (int)offset);
> +trace_virtio_mmio_read(offset);
>
>  if (!vdev) {
>  /* If no backend is present, we treat most registers as
> @@ -144,7 +136,9 @@ static uint64_t virtio_mmio_read(void *opaque, hwaddr 
> offset, unsigned size)
>  }
>  }
>  if (size != 4) {
> -DPRINTF("wrong size access to register!\n");
> +qemu_log_mask(LOG_GUEST_ERROR,
> +  "%s: wrong size access to register!\n",
> +  __func__);
>  return 0;
>  }
>  switch (offset) {
> @@ -182,10 +176,12 @@ static uint64_t virtio_mmio_read(void *opaque, hwaddr 
> offset, unsigned size)
>  case VIRTIO_MMIO_QUEUE_ALIGN:
>  case VIRTIO_MMIO_QUEUE_NOTIFY:
>  case VIRTIO_MMIO_INTERRUPT_ACK:
> -DPRINTF("read of write-only register\n");
> +qemu_log_mask(LOG_GUEST_ERROR,
> +  "%s: read of write-only register\n",
> +  __func__);
>  return 0;
>  default:
> -DPRINTF("bad register offset\n");
> +qemu_log_mask(LOG_GUEST_ERROR, "%s: bad register offset\n", 
> __func__);
>  return 0;
>  }
>  return 0;
> @@ -197,8 +193,7 @@ static void virtio_mmio_write(void *opaque, hwaddr 
> offset, uint64_t value,
>  VirtIOMMIOProxy *proxy = (VirtIOMMIOProxy *)opaque;
>  VirtIODevice *vdev = virtio_bus_get_device(>bus);
>
> -DPRINTF("virtio_mmio_write offset 0x%x value 0x%" PRIx64 "\n",
> -(int)offset, value);
> +trace_virtio_mmio_write_offset(offset, value);
>
>  if (!vdev) {
>  /* If no backend is present, we just make all registers
> @@ -226,7 +221,9 @@ static void virtio_mmio_write(void *opaque, hwaddr 
> offset, uint64_t value,
>  return;
>  }
>  if (size != 4) {
> -DPRINTF("wrong size access to register!\n");
> +qemu_log_mask(LOG_GUEST_ERROR,
> +  "%s: wrong size access to register!\n",
> +  __func__);
>  return;
>  }
>  switch (offset) {
> @@ -246,8 +243,7 @@ static void virtio_mmio_write(void *opaque, hwaddr 
> offset, uint64_t value,
>  if (proxy->guest_page_shift > 31) {
>  proxy->guest_page_shift = 0;
>  }
> -DPRINTF("guest page size %" PRIx64 " shift %d\n", value,
> -proxy->guest_page_shift);
> +

Re: [Qemu-devel] [RFC PATCH] tests/qemu-iotests: re-format output to for make check-block

[Alex Bennée]

Thomas Huth  writes:

> On 03/05/2019 16.39, Alex Bennée wrote:
>> This attempts to clean-up the output to better match the output of the
>> rest of the QEMU check system. This includes:
>>
>>   - formatting as "  TESTiotest: nnn"
>>   - calculating time diff at the end
>>   - only dumping config on failure
>>
>> Signed-off-by: Alex Bennée 
>> ---
>>  tests/qemu-iotests/check | 71 +++-
>>  1 file changed, 34 insertions(+), 37 deletions(-)
>
> Thanks for tackling this! The output now looks nicer indeed if you run
> "make check-qtest check-block -j8". However, if you add a "V=1" at the
> end of the command line, the outputs look quite different again...
>
> That's why I thought that having a TAP mode for the check script could
> be a good idea, too. Then we could pipe the output through the
> tap-driver.pl script, too, so we get uniform output for all tests...?

That would probably be a cleaner approach. What would be even better is
somehow expanding the list of tests at make time so you could run your
tests in parallel.

I did wonder how useful the timing stuff was to developers.

>
>  Thomas


--
Alex Bennée

Re: [Qemu-devel] [PATCH RFC v8 00/12] Add RX archtecture support

[Alex Bennée]

Yoshinori Sato  writes:

> Hello.
> This patch series is added Renesas RX target emulation.

I think the series is almost there - it's mostly just nits and clean
build fixes to sort out now. If you run the branch through CI you will
see where things fail to build.

However once that is all sorted it should be good for merging. I guess
Richard would merge it until we can get your key signed so you can
submit your own PRs:

  https://wiki.qemu.org/KeySigningParty

--
Alex Bennée

Re: [Qemu-devel] [PATCH] VirtIO-RNG: Update default entropy source to `/dev/urandom`

[Richard W.M. Jones]

On Fri, May 03, 2019 at 05:46:13PM +0200, Kashyap Chamarthy wrote:
> When QEMU exposes a VirtIO-RNG device to the guest, that device needs a
> source of entropy, and that source needs to be "non-blocking", like
> `/dev/urandom`.  However, currently QEMU defaults to the problematic
> `/dev/random`, which is "blocking" (as in, it waits until sufficient
> entropy is available).
> 
> So change the entropy source to the recommended `/dev/urandom`.
> 
> Related discussion in these[1][2] past threads.
> 
> [1] https://lists.nongnu.org/archive/html/qemu-devel/2018-06/msg08335.html
> -- "RNG: Any reason QEMU doesn't default to `/dev/urandom`?"
> [2] https://lists.nongnu.org/archive/html/qemu-devel/2018-09/msg02724.html
> -- "[RFC] Virtio RNG: Consider changing the default entropy source to
>/dev/urandom"
> 
> Signed-off-by: Kashyap Chamarthy 
> ---
>  backends/rng-random.c | 2 +-
>  qemu-options.hx   | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/backends/rng-random.c b/backends/rng-random.c
> index e2a49b0571..eff36ef140 100644
> --- a/backends/rng-random.c
> +++ b/backends/rng-random.c
> @@ -112,7 +112,7 @@ static void rng_random_init(Object *obj)
>  rng_random_set_filename,
>  NULL);
>  
> -s->filename = g_strdup("/dev/random");
> +s->filename = g_strdup("/dev/urandom");
>  s->fd = -1;
>  }
>  
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 51802cbb26..a525609149 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -4276,7 +4276,7 @@ Creates a random number generator backend which obtains 
> entropy from
>  a device on the host. The @option{id} parameter is a unique ID that
>  will be used to reference this entropy backend from the @option{virtio-rng}
>  device. The @option{filename} parameter specifies which file to obtain
> -entropy from and if omitted defaults to @option{/dev/random}.
> +entropy from and if omitted defaults to @option{/dev/urandom}.
>  
>  @item -object rng-egd,id=@var{id},chardev=@var{chardevid}

I think this is a very sensible change, removing a bit of superstition
about randomness.

Reviewed-by: Richard W.M. Jones 

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW

Re: [Qemu-devel] [PATCH RFC v8 05/12] target/rx: Miscellaneous files

[Alex Bennée]

Yoshinori Sato  writes:

> Signed-off-by: Yoshinori Sato 
> ---
>  target/rx/gdbstub.c | 112 
> 
>  target/rx/monitor.c |  38 
>  target/rx/Makefile.objs |  11 +
>  3 files changed, 161 insertions(+)
>  create mode 100644 target/rx/gdbstub.c
>  create mode 100644 target/rx/monitor.c
>  create mode 100644 target/rx/Makefile.objs
>
> diff --git a/target/rx/gdbstub.c b/target/rx/gdbstub.c
> new file mode 100644
> index 00..d76ca52e82
> --- /dev/null
> +++ b/target/rx/gdbstub.c
> @@ -0,0 +1,112 @@
> +/*
> + * RX gdb server stub
> + *
> + * Copyright (c) 2019 Yoshinori Sato
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2 or later, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope 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 .
> + */
> +#include "qemu/osdep.h"
> +#include "qemu-common.h"
> +#include "cpu.h"
> +#include "exec/gdbstub.h"
> +
> +int rx_cpu_gdb_read_register(CPUState *cs, uint8_t *mem_buf, int n)
> +{
> +RXCPU *cpu = RXCPU(cs);
> +CPURXState *env = >env;
> +
> +switch (n) {
> +case 0 ... 15:
> +return gdb_get_regl(mem_buf, env->regs[n]);
> +case 16:
> +return gdb_get_regl(mem_buf, (env->psw_u) ? env->regs[0] : env->usp);
> +case 17:
> +return gdb_get_regl(mem_buf, (!env->psw_u) ? env->regs[0] : 
> env->isp);
> +case 18:
> +return gdb_get_regl(mem_buf, rx_cpu_pack_psw(env));
> +case 19:
> +return gdb_get_regl(mem_buf, env->pc);
> +case 20:
> +return gdb_get_regl(mem_buf, env->intb);
> +case 21:
> +return gdb_get_regl(mem_buf, env->bpsw);
> +case 22:
> +return gdb_get_regl(mem_buf, env->bpc);
> +case 23:
> +return gdb_get_regl(mem_buf, env->fintv);
> +case 24:
> +return gdb_get_regl(mem_buf, env->fpsw);
> +case 25:
> +return 0;
> +}
> +return 0;
> +}
> +
> +int rx_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
> +{
> +RXCPU *cpu = RXCPU(cs);
> +CPURXState *env = >env;
> +uint32_t psw;
> +switch (n) {
> +case 0 ... 15:
> +env->regs[n] = ldl_p(mem_buf);
> +if (n == 0) {
> +if (env->psw_u) {
> +env->usp = env->regs[0];
> +} else {
> +env->isp = env->regs[0];
> +}
> +}
> +break;
> +case 16:
> +env->usp = ldl_p(mem_buf);
> +if (env->psw_u) {
> +env->regs[0] = ldl_p(mem_buf);
> +}
> +break;
> +case 17:
> +env->isp = ldl_p(mem_buf);
> +if (!env->psw_u) {
> +env->regs[0] = ldl_p(mem_buf);
> +}
> +break;
> +case 18:
> +psw = ldl_p(mem_buf);
> +rx_cpu_unpack_psw(env, psw, 1);
> +break;
> +case 19:
> +env->pc = ldl_p(mem_buf);
> +break;
> +case 20:
> +env->intb = ldl_p(mem_buf);
> +break;
> +case 21:
> +env->bpsw = ldl_p(mem_buf);
> +break;
> +case 22:
> +env->bpc = ldl_p(mem_buf);
> +break;
> +case 23:
> +env->fintv = ldl_p(mem_buf);
> +break;
> +case 24:
> +env->fpsw = ldl_p(mem_buf);
> +break;
> +case 25:
> +return 8;
> +default:
> +return 0;
> +}
> +
> +return 4;
> +}
> diff --git a/target/rx/monitor.c b/target/rx/monitor.c
> new file mode 100644
> index 00..5d7a1e58b5
> --- /dev/null
> +++ b/target/rx/monitor.c
> @@ -0,0 +1,38 @@
> +/*
> + * QEMU monitor
> + *
> + * Copyright (c) 2003-2004 Fabrice Bellard
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a 
> copy
> + * of this software and associated documentation files (the "Software"), to 
> deal
> + * in the Software without restriction, including without limitation the 
> rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR