Re: [Qemu-devel] [PATCH 00/15] tcg field extract primitives

[no-reply]

Hi,

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

Subject: [Qemu-devel] [PATCH 00/15] tcg field extract primitives
Type: series
Message-id: 1476589070-5792-1-git-send-email-...@twiddle.net

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

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

# Useful git options
git config --local diff.renamelimit 0
git config --local diff.renames True

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

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

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
4d07805 target-s390: Use tcg_gen_extract_i64
3c516d8 target-ppc: Use tcg_gen_extract_*
053b5eb target-mips: Use tcg_gen_extract_*
b32052a target-i386: Use tcg_gen_extract_tl
6387628 target-arm: Use tcg_gen_*extract
0214aa9 target-alpha: Use deposit and extract ops
74f3547 tcg/s390: Implement field extraction opcodes
500fe06 tcg/ppc: Implement field extraction opcodes
976f642 tcg/mips: Implement field extraction opcodes
5eaa14b tcg/i386: Implement field extraction opcodes
123a584 tcg/arm: Implement field extraction opcodes
4624eb9 tcg/arm: Move isa detection to tcg-target.h
7ef8856 tcg/aarch64: Implement field extraction opcodes
2e87ea6 tcg: Minor adjustments to deposit expanders
be676b7 tcg: Add field extraction primitives

=== OUTPUT BEGIN ===
Checking PATCH 1/15: tcg: Add field extraction primitives...
ERROR: spaces required around that ':' (ctx:VxE)
#105: FILE: tcg/optimize.c:881:
+CASE_OP_32_64(extract):
   ^

ERROR: spaces required around that ':' (ctx:VxE)
#111: FILE: tcg/optimize.c:887:
+CASE_OP_32_64(sextract):
^

ERROR: spaces required around that ':' (ctx:VxE)
#125: FILE: tcg/optimize.c:1064:
+CASE_OP_32_64(extract):
   ^

ERROR: spaces required around that ':' (ctx:VxE)
#133: FILE: tcg/optimize.c:1072:
+CASE_OP_32_64(sextract):
^

ERROR: space prohibited after that '&&' (ctx:ExW)
#232: FILE: tcg/tcg-op.c:587:
+&& TCG_TARGET_extract_i32_valid(ofs, len)) {
 ^

ERROR: space prohibited after that '&&' (ctx:ExW)
#286: FILE: tcg/tcg-op.c:641:
+&& TCG_TARGET_extract_i32_valid(ofs, len)) {
 ^

ERROR: space prohibited after that '&&' (ctx:ExW)
#384: FILE: tcg/tcg-op.c:1780:
+&& TCG_TARGET_extract_i64_valid(ofs, len)) {
 ^

ERROR: space prohibited after that '&&' (ctx:ExW)
#473: FILE: tcg/tcg-op.c:1869:
+&& TCG_TARGET_extract_i64_valid(ofs, len)) {
 ^

total: 8 errors, 0 warnings, 560 lines checked

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

Checking PATCH 2/15: tcg: Minor adjustments to deposit expanders...
Checking PATCH 3/15: tcg/aarch64: Implement field extraction opcodes...
Checking PATCH 4/15: tcg/arm: Move isa detection to tcg-target.h...
WARNING: architecture specific defines should be avoided
#20: FILE: tcg/arm/tcg-target.h:30:
+#ifndef __ARM_ARCH

WARNING: architecture specific defines should be avoided
#21: FILE: tcg/arm/tcg-target.h:31:
+# if defined(__ARM_ARCH_7__) || defined(__ARM_ARCH_7A__) \

WARNING: architecture specific defines should be avoided
#40: FILE: tcg/arm/tcg-target.h:50:
+#if defined(__ARM_ARCH_5T__) \

total: 0 errors, 3 warnings, 107 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 5/15: tcg/arm: Implement field extraction opcodes...
Checking PATCH 6/15: tcg/i386: Implement field extraction opcodes...
ERROR: spaces required around that ':' (ctx:VxE)
#51: FILE: tcg/i386/tcg-target.inc.c:2146:
+OP_32_64(extract):
  ^

total: 1 errors, 0 warnings, 75 lines checked

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

Checking PATCH 7/15: tcg/mips: Implement field extraction opcodes...
Checking PATCH 8/15: tcg/ppc: Implement field extraction opcodes...
Checking PATCH 9/15: tcg/s390: Implement field extraction opcodes...
ERROR: space required after that ',' (ctx:VxV)
#40: FILE: tcg/s390/tcg-target.h:111:
+#define TCG_TARGET_deposit_i32_valid(o,l)  tcg_target_have_gen_inst()
   ^

ERROR: space required after that ',' (ctx:VxV)
#41: FILE: tcg/s390/tcg-target.h:112:
+#define TCG_TARGET_deposit_i64_valid(o,l)  tcg_target_have_gen_inst()
   ^

ERROR: space required after that ',' (ctx:VxV)
#42: FILE: 

[Qemu-devel] [PATCH 13/15] target-mips: Use tcg_gen_extract_*

[Richard Henderson]

Use the new primitives for EXT and DEXT.

Cc: Yongbok Kim 
Signed-off-by: Richard Henderson 
---
 target-mips/translate.c | 12 +---
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/target-mips/translate.c b/target-mips/translate.c
index d8dde7a..cf79aa4 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -4484,11 +4484,12 @@ static void gen_bitops (DisasContext *ctx, uint32_t 
opc, int rt,
 if (lsb + msb > 31) {
 goto fail;
 }
-tcg_gen_shri_tl(t0, t1, lsb);
 if (msb != 31) {
-tcg_gen_andi_tl(t0, t0, (1U << (msb + 1)) - 1);
+tcg_gen_extract_tl(t0, t1, lsb, msb + 1);
 } else {
-tcg_gen_ext32s_tl(t0, t0);
+/* The two checks together imply that lsb == 0,
+   so this is a simple sign-extension.  */
+tcg_gen_ext32s_tl(t0, t1);
 }
 break;
 #if defined(TARGET_MIPS64)
@@ -4503,10 +4504,7 @@ static void gen_bitops (DisasContext *ctx, uint32_t opc, 
int rt,
 if (lsb + msb > 63) {
 goto fail;
 }
-tcg_gen_shri_tl(t0, t1, lsb);
-if (msb != 63) {
-tcg_gen_andi_tl(t0, t0, (1ULL << (msb + 1)) - 1);
-}
+tcg_gen_extract_tl(t0, t1, lsb, msb + 1);
 break;
 #endif
 case OPC_INS:
-- 
2.7.4

[Qemu-devel] [PATCH 11/15] target-arm: Use tcg_gen_*extract

[Richard Henderson]

Use the new primitives for UBFX and SBFX.

Cc: Peter Maydell 
Signed-off-by: Richard Henderson 
---
 target-arm/translate-a64.c | 48 ++
 target-arm/translate.c | 37 ---
 2 files changed, 31 insertions(+), 54 deletions(-)

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 2d5c1a2..7df4e84 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -3171,11 +3171,8 @@ static void disas_bitfield(DisasContext *s, uint32_t 
insn)
 goto done;
 }
 }
-if (si == 63 || (si == 31 && ri <= si)) { /* ASR */
-if (si == 31) {
-tcg_gen_ext32s_i64(tcg_tmp, tcg_tmp);
-}
-tcg_gen_sari_i64(tcg_rd, tcg_tmp, ri);
+if (ri <= si) { /* ASR, SBFX */
+tcg_gen_sextract_i64(tcg_rd, tcg_tmp, ri, (si - ri) + 1);
 goto done;
 }
 } else if (opc == 2) { /* UBFM */
@@ -3183,41 +3180,42 @@ static void disas_bitfield(DisasContext *s, uint32_t 
insn)
 tcg_gen_andi_i64(tcg_rd, tcg_tmp, bitmask64(si + 1));
 return;
 }
-if (si == 63 || (si == 31 && ri <= si)) { /* LSR */
-if (si == 31) {
-tcg_gen_ext32u_i64(tcg_tmp, tcg_tmp);
-}
-tcg_gen_shri_i64(tcg_rd, tcg_tmp, ri);
-return;
-}
 if (si + 1 == ri && si != bitsize - 1) { /* LSL */
 int shift = bitsize - 1 - si;
 tcg_gen_shli_i64(tcg_rd, tcg_tmp, shift);
 goto done;
 }
+if (ri <= si) { /* UBFX, LSR */
+tcg_gen_extract_i64(tcg_rd, tcg_tmp, ri, (si - ri) + 1);
+return;
+}
 }
 
 if (opc != 1) { /* SBFM or UBFM */
 tcg_gen_movi_i64(tcg_rd, 0);
 }
 
-/* do the bit move operation */
-if (si >= ri) {
-/* Wd = Wn */
-tcg_gen_shri_i64(tcg_tmp, tcg_tmp, ri);
-pos = 0;
-len = (si - ri) + 1;
-} else {
-/* Wd<32+s-r,32-r> = Wn */
-pos = bitsize - ri;
-len = si + 1;
+/* Do the bit move operation.  Note that above we handled ri <= si,
+   Wd = Wn, via tcg_gen_*extract_i64.  Now we handle
+   the ri > si case, Wd<32+s-r,32-r> = Wn, via deposit.  */
+pos = bitsize - ri;
+len = si + 1;
+
+if (opc == 0 && len < ri) {
+/* SBFM - sign extend the destination field from len to fill
+   the balance of the word.  Let the deposit below insert all
+   of those sign bits.  */
+tcg_gen_sextract_i64(tcg_tmp, tcg_tmp, 0, len);
+len = ri;
 }
 
 tcg_gen_deposit_i64(tcg_rd, tcg_rd, tcg_tmp, pos, len);
 
-if (opc == 0) { /* SBFM - sign extend the destination field */
-tcg_gen_shli_i64(tcg_rd, tcg_rd, 64 - (pos + len));
-tcg_gen_sari_i64(tcg_rd, tcg_rd, 64 - (pos + len));
+if (opc != 1) {
+/* SBFM or UBFM: We started with zero above, and we haven't
+   modified any bits outside bitsize, therefore the zero-extension
+   below is unneeded.  */
+return;
 }
 
  done:
diff --git a/target-arm/translate.c b/target-arm/translate.c
index aaf6135..37ad61d 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -297,29 +297,6 @@ static void gen_revsh(TCGv_i32 var)
 tcg_gen_ext16s_i32(var, var);
 }
 
-/* Unsigned bitfield extract.  */
-static void gen_ubfx(TCGv_i32 var, int shift, uint32_t mask)
-{
-if (shift)
-tcg_gen_shri_i32(var, var, shift);
-tcg_gen_andi_i32(var, var, mask);
-}
-
-/* Signed bitfield extract.  */
-static void gen_sbfx(TCGv_i32 var, int shift, int width)
-{
-uint32_t signbit;
-
-if (shift)
-tcg_gen_sari_i32(var, var, shift);
-if (shift + width < 32) {
-signbit = 1u << (width - 1);
-tcg_gen_andi_i32(var, var, (1u << width) - 1);
-tcg_gen_xori_i32(var, var, signbit);
-tcg_gen_subi_i32(var, var, signbit);
-}
-}
-
 /* Return (b << 32) + a. Mark inputs as dead */
 static TCGv_i64 gen_addq_msw(TCGv_i64 a, TCGv_i32 b)
 {
@@ -9234,9 +9211,9 @@ static void disas_arm_insn(DisasContext *s, unsigned int 
insn)
 goto illegal_op;
 if (i < 32) {
 if (op1 & 0x20) {
-gen_ubfx(tmp, shift, (1u << i) - 1);
+tcg_gen_extract_i32(tmp, tmp, shift, i);
 } else {
-gen_sbfx(tmp, shift, i);
+tcg_gen_sextract_i32(tmp, tmp, shift, i);
 }
 }
 store_reg(s, rd, tmp);
@@ -10551,15 +10528,17 @@ static int disas_thumb2_insn(CPUARMState *env, 
DisasContext *s, uint16_t insn_hw
 imm++;
 if (shift + imm > 

[Qemu-devel] [PATCH 07/15] tcg/mips: Implement field extraction opcodes

[Richard Henderson]

Cc: Aurelien Jarno 
Signed-off-by: Richard Henderson 
---
 tcg/mips/tcg-target.h | 2 +-
 tcg/mips/tcg-target.inc.c | 4 
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/tcg/mips/tcg-target.h b/tcg/mips/tcg-target.h
index 1bcea3b..f1c3137 100644
--- a/tcg/mips/tcg-target.h
+++ b/tcg/mips/tcg-target.h
@@ -123,7 +123,7 @@ extern bool use_mips32r2_instructions;
 #define TCG_TARGET_HAS_bswap16_i32  use_mips32r2_instructions
 #define TCG_TARGET_HAS_bswap32_i32  use_mips32r2_instructions
 #define TCG_TARGET_HAS_deposit_i32  use_mips32r2_instructions
-#define TCG_TARGET_HAS_extract_i32  0
+#define TCG_TARGET_HAS_extract_i32  use_mips32r2_instructions
 #define TCG_TARGET_HAS_sextract_i32 0
 #define TCG_TARGET_HAS_ext8s_i32use_mips32r2_instructions
 #define TCG_TARGET_HAS_ext16s_i32   use_mips32r2_instructions
diff --git a/tcg/mips/tcg-target.inc.c b/tcg/mips/tcg-target.inc.c
index abce602..192dd49 100644
--- a/tcg/mips/tcg-target.inc.c
+++ b/tcg/mips/tcg-target.inc.c
@@ -1637,6 +1637,9 @@ static inline void tcg_out_op(TCGContext *s, TCGOpcode 
opc,
 case INDEX_op_deposit_i32:
 tcg_out_opc_bf(s, OPC_INS, a0, a2, args[3] + args[4] - 1, args[3]);
 break;
+case INDEX_op_extract_i32:
+tcg_out_opc_bf(s, OPC_EXT, a0, a1, a3 + args[3] - 1, a2);
+break;
 
 case INDEX_op_brcond_i32:
 tcg_out_brcond(s, a2, a0, a1, arg_label(args[3]));
@@ -1736,6 +1739,7 @@ static const TCGTargetOpDef mips_op_defs[] = {
 { INDEX_op_ext16s_i32, { "r", "rZ" } },
 
 { INDEX_op_deposit_i32, { "r", "0", "rZ" } },
+{ INDEX_op_extract_i32, { "r", "r" } },
 
 { INDEX_op_brcond_i32, { "rZ", "rZ" } },
 #if use_mips32r6_instructions
-- 
2.7.4

[Qemu-devel] [PATCH 05/15] tcg/arm: Implement field extraction opcodes

[Richard Henderson]

Cc: Andrzej Zaborowski 
Cc: Peter Maydell 
Signed-off-by: Richard Henderson 
---
 tcg/arm/tcg-target.h |  4 ++--
 tcg/arm/tcg-target.inc.c | 22 ++
 2 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/tcg/arm/tcg-target.h b/tcg/arm/tcg-target.h
index d1fe12b..4e30728 100644
--- a/tcg/arm/tcg-target.h
+++ b/tcg/arm/tcg-target.h
@@ -111,8 +111,8 @@ extern bool use_idiv_instructions;
 #define TCG_TARGET_HAS_nand_i32 0
 #define TCG_TARGET_HAS_nor_i32  0
 #define TCG_TARGET_HAS_deposit_i32  use_armv7_instructions
-#define TCG_TARGET_HAS_extract_i32  0
-#define TCG_TARGET_HAS_sextract_i32 0
+#define TCG_TARGET_HAS_extract_i32  use_armv7_instructions
+#define TCG_TARGET_HAS_sextract_i32 use_armv7_instructions
 #define TCG_TARGET_HAS_movcond_i32  1
 #define TCG_TARGET_HAS_mulu2_i321
 #define TCG_TARGET_HAS_muls2_i321
diff --git a/tcg/arm/tcg-target.inc.c b/tcg/arm/tcg-target.inc.c
index 1415c27..6765a9d 100644
--- a/tcg/arm/tcg-target.inc.c
+++ b/tcg/arm/tcg-target.inc.c
@@ -713,6 +713,20 @@ static inline void tcg_out_deposit(TCGContext *s, int 
cond, TCGReg rd,
   | (ofs << 7) | ((ofs + len - 1) << 16));
 }
 
+static inline void tcg_out_extract(TCGContext *s, int cond, TCGReg rd,
+   TCGArg a1, int ofs, int len)
+{
+tcg_out32(s, 0x07e00050 | (cond << 28) | (rd << 12) | a1
+  | (ofs << 7) | ((len - 1) << 16));
+}
+
+static inline void tcg_out_sextract(TCGContext *s, int cond, TCGReg rd,
+TCGArg a1, int ofs, int len)
+{
+tcg_out32(s, 0x07a00050 | (cond << 28) | (rd << 12) | a1
+  | (ofs << 7) | ((len - 1) << 16));
+}
+
 /* Note that this routine is used for both LDR and LDRH formats, so we do
not wish to include an immediate shift at this point.  */
 static void tcg_out_memop_r(TCGContext *s, int cond, ARMInsn opc, TCGReg rt,
@@ -1894,6 +1908,12 @@ static inline void tcg_out_op(TCGContext *s, TCGOpcode 
opc,
 tcg_out_deposit(s, COND_AL, args[0], args[2],
 args[3], args[4], const_args[2]);
 break;
+case INDEX_op_extract_i32:
+tcg_out_extract(s, COND_AL, args[0], args[1], args[2], args[3]);
+break;
+case INDEX_op_sextract_i32:
+tcg_out_sextract(s, COND_AL, args[0], args[1], args[2], args[3]);
+break;
 
 case INDEX_op_div_i32:
 tcg_out_sdiv(s, COND_AL, args[0], args[1], args[2]);
@@ -1976,6 +1996,8 @@ static const TCGTargetOpDef arm_op_defs[] = {
 { INDEX_op_ext16u_i32, { "r", "r" } },
 
 { INDEX_op_deposit_i32, { "r", "0", "rZ" } },
+{ INDEX_op_extract_i32, { "r", "r" } },
+{ INDEX_op_sextract_i32, { "r", "r" } },
 
 { INDEX_op_div_i32, { "r", "r", "r" } },
 { INDEX_op_divu_i32, { "r", "r", "r" } },
-- 
2.7.4

[Qemu-devel] [PATCH 10/15] target-alpha: Use deposit and extract ops

[Richard Henderson]

Signed-off-by: Richard Henderson 
---
 target-alpha/translate.c | 67 ++--
 1 file changed, 42 insertions(+), 25 deletions(-)

diff --git a/target-alpha/translate.c b/target-alpha/translate.c
index af717ca..a341729 100644
--- a/target-alpha/translate.c
+++ b/target-alpha/translate.c
@@ -953,7 +953,13 @@ static void gen_ext_h(DisasContext *ctx, TCGv vc, TCGv va, 
int rb, bool islit,
   uint8_t lit, uint8_t byte_mask)
 {
 if (islit) {
-tcg_gen_shli_i64(vc, va, (64 - lit * 8) & 0x3f);
+int pos = (64 - lit * 8) & 0x3f;
+int len = cto32(byte_mask) * 8;
+if (pos < len) {
+tcg_gen_deposit_i64(vc, load_zero(ctx), va, pos, len - pos);
+} else {
+tcg_gen_movi_i64(vc, 0);
+}
 } else {
 TCGv tmp = tcg_temp_new();
 tcg_gen_shli_i64(tmp, load_gpr(ctx, rb), 3);
@@ -970,38 +976,44 @@ static void gen_ext_l(DisasContext *ctx, TCGv vc, TCGv 
va, int rb, bool islit,
   uint8_t lit, uint8_t byte_mask)
 {
 if (islit) {
-tcg_gen_shri_i64(vc, va, (lit & 7) * 8);
+int pos = (lit & 7) * 8;
+int len = cto32(byte_mask) * 8;
+if (pos + len >= 64) {
+len = 64 - pos;
+}
+tcg_gen_extract_i64(vc, va, pos, len);
 } else {
 TCGv tmp = tcg_temp_new();
 tcg_gen_andi_i64(tmp, load_gpr(ctx, rb), 7);
 tcg_gen_shli_i64(tmp, tmp, 3);
 tcg_gen_shr_i64(vc, va, tmp);
 tcg_temp_free(tmp);
+gen_zapnoti(vc, vc, byte_mask);
 }
-gen_zapnoti(vc, vc, byte_mask);
 }
 
 /* INSWH, INSLH, INSQH */
 static void gen_ins_h(DisasContext *ctx, TCGv vc, TCGv va, int rb, bool islit,
   uint8_t lit, uint8_t byte_mask)
 {
-TCGv tmp = tcg_temp_new();
-
-/* The instruction description has us left-shift the byte mask and extract
-   bits <15:8> and apply that zap at the end.  This is equivalent to simply
-   performing the zap first and shifting afterward.  */
-gen_zapnoti(tmp, va, byte_mask);
-
 if (islit) {
-lit &= 7;
-if (unlikely(lit == 0)) {
-tcg_gen_movi_i64(vc, 0);
+int pos = 64 - (lit & 7) * 8;
+int len = cto32(byte_mask) * 8;
+if (pos < len) {
+tcg_gen_extract_i64(vc, va, pos, len - pos);
 } else {
-tcg_gen_shri_i64(vc, tmp, 64 - lit * 8);
+tcg_gen_movi_i64(vc, 0);
 }
 } else {
+TCGv tmp = tcg_temp_new();
 TCGv shift = tcg_temp_new();
 
+/* The instruction description has us left-shift the byte mask
+   and extract bits <15:8> and apply that zap at the end.  This
+   is equivalent to simply performing the zap first and shifting
+   afterward.  */
+gen_zapnoti(tmp, va, byte_mask);
+
 /* If (B & 7) == 0, we need to shift by 64 and leave a zero.  Do this
portably by splitting the shift into two parts: shift_count-1 and 1.
Arrange for the -1 by using ones-complement instead of
@@ -1014,32 +1026,37 @@ static void gen_ins_h(DisasContext *ctx, TCGv vc, TCGv 
va, int rb, bool islit,
 tcg_gen_shr_i64(vc, tmp, shift);
 tcg_gen_shri_i64(vc, vc, 1);
 tcg_temp_free(shift);
+tcg_temp_free(tmp);
 }
-tcg_temp_free(tmp);
 }
 
 /* INSBL, INSWL, INSLL, INSQL */
 static void gen_ins_l(DisasContext *ctx, TCGv vc, TCGv va, int rb, bool islit,
   uint8_t lit, uint8_t byte_mask)
 {
-TCGv tmp = tcg_temp_new();
-
-/* The instruction description has us left-shift the byte mask
-   the same number of byte slots as the data and apply the zap
-   at the end.  This is equivalent to simply performing the zap
-   first and shifting afterward.  */
-gen_zapnoti(tmp, va, byte_mask);
-
 if (islit) {
-tcg_gen_shli_i64(vc, tmp, (lit & 7) * 8);
+int pos = (lit & 7) * 8;
+int len = cto32(byte_mask) * 8;
+if (pos + len > 64) {
+len = 64 - pos;
+}
+tcg_gen_deposit_i64(vc, load_zero(ctx), va, pos, len);
 } else {
+TCGv tmp = tcg_temp_new();
 TCGv shift = tcg_temp_new();
+
+/* The instruction description has us left-shift the byte mask
+   and extract bits <15:8> and apply that zap at the end.  This
+   is equivalent to simply performing the zap first and shifting
+   afterward.  */
+gen_zapnoti(tmp, va, byte_mask);
+
 tcg_gen_andi_i64(shift, load_gpr(ctx, rb), 7);
 tcg_gen_shli_i64(shift, shift, 3);
 tcg_gen_shl_i64(vc, tmp, shift);
 tcg_temp_free(shift);
+tcg_temp_free(tmp);
 }
-tcg_temp_free(tmp);
 }
 
 /* MSKWH, MSKLH, MSKQH */
-- 
2.7.4

[Qemu-devel] [PATCH 04/15] tcg/arm: Move isa detection to tcg-target.h

[Richard Henderson]

Cc: Andrzej Zaborowski 
Cc: Peter Maydell 
Signed-off-by: Richard Henderson 
---
 tcg/arm/tcg-target.h | 36 
 tcg/arm/tcg-target.inc.c | 41 +
 2 files changed, 33 insertions(+), 44 deletions(-)

diff --git a/tcg/arm/tcg-target.h b/tcg/arm/tcg-target.h
index 8e724be..d1fe12b 100644
--- a/tcg/arm/tcg-target.h
+++ b/tcg/arm/tcg-target.h
@@ -26,6 +26,37 @@
 #ifndef ARM_TCG_TARGET_H
 #define ARM_TCG_TARGET_H
 
+/* The __ARM_ARCH define is provided by gcc 4.8.  Construct it otherwise.  */
+#ifndef __ARM_ARCH
+# if defined(__ARM_ARCH_7__) || defined(__ARM_ARCH_7A__) \
+ || defined(__ARM_ARCH_7R__) || defined(__ARM_ARCH_7M__) \
+ || defined(__ARM_ARCH_7EM__)
+#  define __ARM_ARCH 7
+# elif defined(__ARM_ARCH_6__) || defined(__ARM_ARCH_6J__) \
+   || defined(__ARM_ARCH_6Z__) || defined(__ARM_ARCH_6ZK__) \
+   || defined(__ARM_ARCH_6K__) || defined(__ARM_ARCH_6T2__)
+#  define __ARM_ARCH 6
+# elif defined(__ARM_ARCH_5__) || defined(__ARM_ARCH_5E__) \
+   || defined(__ARM_ARCH_5T__) || defined(__ARM_ARCH_5TE__) \
+   || defined(__ARM_ARCH_5TEJ__)
+#  define __ARM_ARCH 5
+# else
+#  define __ARM_ARCH 4
+# endif
+#endif
+
+extern int arm_arch;
+
+#if defined(__ARM_ARCH_5T__) \
+|| defined(__ARM_ARCH_5TE__) || defined(__ARM_ARCH_5TEJ__)
+# define use_armv5t_instructions 1
+#else
+# define use_armv5t_instructions use_armv6_instructions
+#endif
+
+#define use_armv6_instructions  (__ARM_ARCH >= 6 || arm_arch >= 6)
+#define use_armv7_instructions  (__ARM_ARCH >= 7 || arm_arch >= 7)
+
 #undef TCG_TARGET_STACK_GROWSUP
 #define TCG_TARGET_INSN_UNIT_SIZE 4
 #define TCG_TARGET_TLB_DISPLACEMENT_BITS 16
@@ -79,7 +110,7 @@ extern bool use_idiv_instructions;
 #define TCG_TARGET_HAS_eqv_i32  0
 #define TCG_TARGET_HAS_nand_i32 0
 #define TCG_TARGET_HAS_nor_i32  0
-#define TCG_TARGET_HAS_deposit_i32  1
+#define TCG_TARGET_HAS_deposit_i32  use_armv7_instructions
 #define TCG_TARGET_HAS_extract_i32  0
 #define TCG_TARGET_HAS_sextract_i32 0
 #define TCG_TARGET_HAS_movcond_i32  1
@@ -90,9 +121,6 @@ extern bool use_idiv_instructions;
 #define TCG_TARGET_HAS_div_i32  use_idiv_instructions
 #define TCG_TARGET_HAS_rem_i32  0
 
-extern bool tcg_target_deposit_valid(int ofs, int len);
-#define TCG_TARGET_deposit_i32_valid  tcg_target_deposit_valid
-
 enum {
 TCG_AREG0 = TCG_REG_R6,
 };
diff --git a/tcg/arm/tcg-target.inc.c b/tcg/arm/tcg-target.inc.c
index ffa0d40..1415c27 100644
--- a/tcg/arm/tcg-target.inc.c
+++ b/tcg/arm/tcg-target.inc.c
@@ -25,36 +25,7 @@
 #include "elf.h"
 #include "tcg-be-ldst.h"
 
-/* The __ARM_ARCH define is provided by gcc 4.8.  Construct it otherwise.  */
-#ifndef __ARM_ARCH
-# if defined(__ARM_ARCH_7__) || defined(__ARM_ARCH_7A__) \
- || defined(__ARM_ARCH_7R__) || defined(__ARM_ARCH_7M__) \
- || defined(__ARM_ARCH_7EM__)
-#  define __ARM_ARCH 7
-# elif defined(__ARM_ARCH_6__) || defined(__ARM_ARCH_6J__) \
-   || defined(__ARM_ARCH_6Z__) || defined(__ARM_ARCH_6ZK__) \
-   || defined(__ARM_ARCH_6K__) || defined(__ARM_ARCH_6T2__)
-#  define __ARM_ARCH 6
-# elif defined(__ARM_ARCH_5__) || defined(__ARM_ARCH_5E__) \
-   || defined(__ARM_ARCH_5T__) || defined(__ARM_ARCH_5TE__) \
-   || defined(__ARM_ARCH_5TEJ__)
-#  define __ARM_ARCH 5
-# else
-#  define __ARM_ARCH 4
-# endif
-#endif
-
-static int arm_arch = __ARM_ARCH;
-
-#if defined(__ARM_ARCH_5T__) \
-|| defined(__ARM_ARCH_5TE__) || defined(__ARM_ARCH_5TEJ__)
-# define use_armv5t_instructions 1
-#else
-# define use_armv5t_instructions use_armv6_instructions
-#endif
-
-#define use_armv6_instructions  (__ARM_ARCH >= 6 || arm_arch >= 6)
-#define use_armv7_instructions  (__ARM_ARCH >= 7 || arm_arch >= 7)
+int arm_arch = __ARM_ARCH;
 
 #ifndef use_idiv_instructions
 bool use_idiv_instructions;
@@ -730,16 +701,6 @@ static inline void tcg_out_bswap32(TCGContext *s, int 
cond, int rd, int rn)
 }
 }
 
-bool tcg_target_deposit_valid(int ofs, int len)
-{
-/* ??? Without bfi, we could improve over generic code by combining
-   the right-shift from a non-zero ofs with the orr.  We do run into
-   problems when rd == rs, and the mask generated from ofs+len doesn't
-   fit into an immediate.  We would have to be careful not to pessimize
-   wrt the optimizations performed on the expanded code.  */
-return use_armv7_instructions;
-}
-
 static inline void tcg_out_deposit(TCGContext *s, int cond, TCGReg rd,
TCGArg a1, int ofs, int len, bool const_a1)
 {
-- 
2.7.4

[Qemu-devel] [PATCH 09/15] tcg/s390: Implement field extraction opcodes

[Richard Henderson]

Cc: Alexander Graf 
Signed-off-by: Richard Henderson 
---
 tcg/s390/tcg-target.h | 12 +++-
 tcg/s390/tcg-target.inc.c | 13 -
 2 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/tcg/s390/tcg-target.h b/tcg/s390/tcg-target.h
index 9583df4..cf8fbfd 100644
--- a/tcg/s390/tcg-target.h
+++ b/tcg/s390/tcg-target.h
@@ -66,7 +66,7 @@ typedef enum TCGReg {
 #define TCG_TARGET_HAS_nand_i32 0
 #define TCG_TARGET_HAS_nor_i32  0
 #define TCG_TARGET_HAS_deposit_i32  1
-#define TCG_TARGET_HAS_extract_i32  0
+#define TCG_TARGET_HAS_extract_i32  1
 #define TCG_TARGET_HAS_sextract_i32 0
 #define TCG_TARGET_HAS_movcond_i32  1
 #define TCG_TARGET_HAS_add2_i32 1
@@ -97,7 +97,7 @@ typedef enum TCGReg {
 #define TCG_TARGET_HAS_nand_i64 0
 #define TCG_TARGET_HAS_nor_i64  0
 #define TCG_TARGET_HAS_deposit_i64  1
-#define TCG_TARGET_HAS_extract_i64  0
+#define TCG_TARGET_HAS_extract_i64  1
 #define TCG_TARGET_HAS_sextract_i64 0
 #define TCG_TARGET_HAS_movcond_i64  1
 #define TCG_TARGET_HAS_add2_i64 1
@@ -107,9 +107,11 @@ typedef enum TCGReg {
 #define TCG_TARGET_HAS_muluh_i640
 #define TCG_TARGET_HAS_mulsh_i640
 
-extern bool tcg_target_deposit_valid(int ofs, int len);
-#define TCG_TARGET_deposit_i32_valid  tcg_target_deposit_valid
-#define TCG_TARGET_deposit_i64_valid  tcg_target_deposit_valid
+extern bool tcg_target_have_gen_inst(void);
+#define TCG_TARGET_deposit_i32_valid(o,l)  tcg_target_have_gen_inst()
+#define TCG_TARGET_deposit_i64_valid(o,l)  tcg_target_have_gen_inst()
+#define TCG_TARGET_extract_i32_valid(o,l)  tcg_target_have_gen_inst()
+#define TCG_TARGET_extract_i64_valid(o,l)  tcg_target_have_gen_inst()
 
 /* used for function call generation */
 #define TCG_REG_CALL_STACK TCG_REG_R15
diff --git a/tcg/s390/tcg-target.inc.c b/tcg/s390/tcg-target.inc.c
index 253d4a0..fa9e144 100644
--- a/tcg/s390/tcg-target.inc.c
+++ b/tcg/s390/tcg-target.inc.c
@@ -1250,7 +1250,7 @@ static void tgen_movcond(TCGContext *s, TCGType type, 
TCGCond c, TCGReg dest,
 }
 }
 
-bool tcg_target_deposit_valid(int ofs, int len)
+bool tcg_target_have_gen_inst(void)
 {
 return (facilities & FACILITY_GEN_INST_EXT) != 0;
 }
@@ -1263,6 +1263,12 @@ static void tgen_deposit(TCGContext *s, TCGReg dest, 
TCGReg src,
 tcg_out_risbg(s, dest, src, msb, lsb, ofs, 0);
 }
 
+static void tgen_extract(TCGContext *s, TCGReg dest, TCGReg src,
+ int ofs, int len)
+{
+tcg_out_risbg(s, dest, src, 64 - len, 63, 64 - ofs, 1);
+}
+
 static void tgen_gotoi(TCGContext *s, int cc, tcg_insn_unit *dest)
 {
 ptrdiff_t off = dest - s->code_ptr;
@@ -2169,6 +2175,9 @@ static inline void tcg_out_op(TCGContext *s, TCGOpcode 
opc,
 OP_32_64(deposit):
 tgen_deposit(s, args[0], args[2], args[3], args[4]);
 break;
+OP_32_64(extract):
+tgen_extract(s, args[0], args[1], args[2], args[3]);
+break;
 
 case INDEX_op_mb:
 /* The host memory model is quite strong, we simply need to
@@ -2238,6 +2247,7 @@ static const TCGTargetOpDef s390_op_defs[] = {
 { INDEX_op_setcond_i32, { "r", "r", "rC" } },
 { INDEX_op_movcond_i32, { "r", "r", "rC", "r", "0" } },
 { INDEX_op_deposit_i32, { "r", "0", "r" } },
+{ INDEX_op_extract_i32, { "r", "r" } },
 
 { INDEX_op_qemu_ld_i32, { "r", "L" } },
 { INDEX_op_qemu_ld_i64, { "r", "L" } },
@@ -2299,6 +2309,7 @@ static const TCGTargetOpDef s390_op_defs[] = {
 { INDEX_op_setcond_i64, { "r", "r", "rC" } },
 { INDEX_op_movcond_i64, { "r", "r", "rC", "r", "0" } },
 { INDEX_op_deposit_i64, { "r", "0", "r" } },
+{ INDEX_op_extract_i64, { "r", "r" } },
 
 { INDEX_op_mb, { } },
 { -1 },
-- 
2.7.4

[Qemu-devel] [PATCH 08/15] tcg/ppc: Implement field extraction opcodes

[Richard Henderson]

Signed-off-by: Richard Henderson 
---
 tcg/ppc/tcg-target.h |  4 ++--
 tcg/ppc/tcg-target.inc.c | 10 ++
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/tcg/ppc/tcg-target.h b/tcg/ppc/tcg-target.h
index c765d3e..b42c57a 100644
--- a/tcg/ppc/tcg-target.h
+++ b/tcg/ppc/tcg-target.h
@@ -69,7 +69,7 @@ typedef enum {
 #define TCG_TARGET_HAS_nand_i32 1
 #define TCG_TARGET_HAS_nor_i32  1
 #define TCG_TARGET_HAS_deposit_i32  1
-#define TCG_TARGET_HAS_extract_i32  0
+#define TCG_TARGET_HAS_extract_i32  1
 #define TCG_TARGET_HAS_sextract_i32 0
 #define TCG_TARGET_HAS_movcond_i32  1
 #define TCG_TARGET_HAS_mulu2_i320
@@ -102,7 +102,7 @@ typedef enum {
 #define TCG_TARGET_HAS_nand_i64 1
 #define TCG_TARGET_HAS_nor_i64  1
 #define TCG_TARGET_HAS_deposit_i64  1
-#define TCG_TARGET_HAS_extract_i64  0
+#define TCG_TARGET_HAS_extract_i64  1
 #define TCG_TARGET_HAS_sextract_i64 0
 #define TCG_TARGET_HAS_movcond_i64  1
 #define TCG_TARGET_HAS_add2_i64 1
diff --git a/tcg/ppc/tcg-target.inc.c b/tcg/ppc/tcg-target.inc.c
index a3262cf..7ec54a2 100644
--- a/tcg/ppc/tcg-target.inc.c
+++ b/tcg/ppc/tcg-target.inc.c
@@ -2396,6 +2396,14 @@ static void tcg_out_op(TCGContext *s, TCGOpcode opc, 
const TCGArg *args,
 }
 break;
 
+case INDEX_op_extract_i32:
+tcg_out_rlw(s, RLWINM, args[0], args[1],
+32 - args[2], 32 - args[3], 31);
+break;
+case INDEX_op_extract_i64:
+tcg_out_rld(s, RLDICL, args[0], args[1], 64 - args[2], 64 - args[3]);
+break;
+
 case INDEX_op_movcond_i32:
 tcg_out_movcond(s, TCG_TYPE_I32, args[5], args[0], args[1], args[2],
 args[3], args[4], const_args[2]);
@@ -2530,6 +2538,7 @@ static const TCGTargetOpDef ppc_op_defs[] = {
 { INDEX_op_movcond_i32, { "r", "r", "ri", "rZ", "rZ" } },
 
 { INDEX_op_deposit_i32, { "r", "0", "rZ" } },
+{ INDEX_op_extract_i32, { "r", "r" } },
 
 { INDEX_op_muluh_i32, { "r", "r", "r" } },
 { INDEX_op_mulsh_i32, { "r", "r", "r" } },
@@ -2585,6 +2594,7 @@ static const TCGTargetOpDef ppc_op_defs[] = {
 { INDEX_op_movcond_i64, { "r", "r", "ri", "rZ", "rZ" } },
 
 { INDEX_op_deposit_i64, { "r", "0", "rZ" } },
+{ INDEX_op_extract_i64, { "r", "r" } },
 
 { INDEX_op_mulsh_i64, { "r", "r", "r" } },
 { INDEX_op_muluh_i64, { "r", "r", "r" } },
-- 
2.7.4

[Qemu-devel] [PATCH 01/15] tcg: Add field extraction primitives

[Richard Henderson]

Adds tcg_gen_extract_* and tcg_gen_sextract_* for extraction of
fixed position bitfields, much like we already have for deposit.

Signed-off-by: Richard Henderson 
---
 tcg/aarch64/tcg-target.h |   4 +
 tcg/arm/tcg-target.h |   2 +
 tcg/i386/tcg-target.h|   4 +
 tcg/ia64/tcg-target.h|   4 +
 tcg/mips/tcg-target.h|   2 +
 tcg/optimize.c   |  29 +
 tcg/ppc/tcg-target.h |   4 +
 tcg/s390/tcg-target.h|   4 +
 tcg/sparc/tcg-target.h   |   4 +
 tcg/tcg-op.c | 313 +++
 tcg/tcg-op.h |  12 ++
 tcg/tcg-opc.h|   4 +
 tcg/tcg.h|   8 ++
 tcg/tci/tcg-target.h |   4 +
 14 files changed, 398 insertions(+)

diff --git a/tcg/aarch64/tcg-target.h b/tcg/aarch64/tcg-target.h
index a1d101f..410c31b 100644
--- a/tcg/aarch64/tcg-target.h
+++ b/tcg/aarch64/tcg-target.h
@@ -63,6 +63,8 @@ typedef enum {
 #define TCG_TARGET_HAS_nand_i32 0
 #define TCG_TARGET_HAS_nor_i32  0
 #define TCG_TARGET_HAS_deposit_i32  1
+#define TCG_TARGET_HAS_extract_i32  0
+#define TCG_TARGET_HAS_sextract_i32 0
 #define TCG_TARGET_HAS_movcond_i32  1
 #define TCG_TARGET_HAS_add2_i32 1
 #define TCG_TARGET_HAS_sub2_i32 1
@@ -93,6 +95,8 @@ typedef enum {
 #define TCG_TARGET_HAS_nand_i64 0
 #define TCG_TARGET_HAS_nor_i64  0
 #define TCG_TARGET_HAS_deposit_i64  1
+#define TCG_TARGET_HAS_extract_i64  0
+#define TCG_TARGET_HAS_sextract_i64 0
 #define TCG_TARGET_HAS_movcond_i64  1
 #define TCG_TARGET_HAS_add2_i64 1
 #define TCG_TARGET_HAS_sub2_i64 1
diff --git a/tcg/arm/tcg-target.h b/tcg/arm/tcg-target.h
index a0e1acf..8e724be 100644
--- a/tcg/arm/tcg-target.h
+++ b/tcg/arm/tcg-target.h
@@ -80,6 +80,8 @@ extern bool use_idiv_instructions;
 #define TCG_TARGET_HAS_nand_i32 0
 #define TCG_TARGET_HAS_nor_i32  0
 #define TCG_TARGET_HAS_deposit_i32  1
+#define TCG_TARGET_HAS_extract_i32  0
+#define TCG_TARGET_HAS_sextract_i32 0
 #define TCG_TARGET_HAS_movcond_i32  1
 #define TCG_TARGET_HAS_mulu2_i321
 #define TCG_TARGET_HAS_muls2_i321
diff --git a/tcg/i386/tcg-target.h b/tcg/i386/tcg-target.h
index 524cfc6..7625188 100644
--- a/tcg/i386/tcg-target.h
+++ b/tcg/i386/tcg-target.h
@@ -94,6 +94,8 @@ extern bool have_bmi1;
 #define TCG_TARGET_HAS_nand_i32 0
 #define TCG_TARGET_HAS_nor_i32  0
 #define TCG_TARGET_HAS_deposit_i32  1
+#define TCG_TARGET_HAS_extract_i32  0
+#define TCG_TARGET_HAS_sextract_i32 0
 #define TCG_TARGET_HAS_movcond_i32  1
 #define TCG_TARGET_HAS_add2_i32 1
 #define TCG_TARGET_HAS_sub2_i32 1
@@ -124,6 +126,8 @@ extern bool have_bmi1;
 #define TCG_TARGET_HAS_nand_i64 0
 #define TCG_TARGET_HAS_nor_i64  0
 #define TCG_TARGET_HAS_deposit_i64  1
+#define TCG_TARGET_HAS_extract_i64  0
+#define TCG_TARGET_HAS_sextract_i64 0
 #define TCG_TARGET_HAS_movcond_i64  1
 #define TCG_TARGET_HAS_add2_i64 1
 #define TCG_TARGET_HAS_sub2_i64 1
diff --git a/tcg/ia64/tcg-target.h b/tcg/ia64/tcg-target.h
index 6dddb7f..8856dc8 100644
--- a/tcg/ia64/tcg-target.h
+++ b/tcg/ia64/tcg-target.h
@@ -149,6 +149,10 @@ typedef enum {
 #define TCG_TARGET_HAS_movcond_i64  1
 #define TCG_TARGET_HAS_deposit_i32  1
 #define TCG_TARGET_HAS_deposit_i64  1
+#define TCG_TARGET_HAS_extract_i32  0
+#define TCG_TARGET_HAS_extract_i64  0
+#define TCG_TARGET_HAS_sextract_i32 0
+#define TCG_TARGET_HAS_sextract_i64 0
 #define TCG_TARGET_HAS_add2_i32 0
 #define TCG_TARGET_HAS_add2_i64 0
 #define TCG_TARGET_HAS_sub2_i32 0
diff --git a/tcg/mips/tcg-target.h b/tcg/mips/tcg-target.h
index 3aeac87..1bcea3b 100644
--- a/tcg/mips/tcg-target.h
+++ b/tcg/mips/tcg-target.h
@@ -123,6 +123,8 @@ extern bool use_mips32r2_instructions;
 #define TCG_TARGET_HAS_bswap16_i32  use_mips32r2_instructions
 #define TCG_TARGET_HAS_bswap32_i32  use_mips32r2_instructions
 #define TCG_TARGET_HAS_deposit_i32  use_mips32r2_instructions
+#define TCG_TARGET_HAS_extract_i32  0
+#define TCG_TARGET_HAS_sextract_i32 0
 #define TCG_TARGET_HAS_ext8s_i32use_mips32r2_instructions
 #define TCG_TARGET_HAS_ext16s_i32   use_mips32r2_instructions
 #define TCG_TARGET_HAS_rot_i32  use_mips32r2_instructions
diff --git a/tcg/optimize.c b/tcg/optimize.c
index 0f13490..0be71f8 100644
--- a/tcg/optimize.c
+++ b/tcg/optimize.c
@@ -878,6 +878,19 @@ void tcg_optimize(TCGContext *s)
  temps[args[2]].mask);
 break;
 
+CASE_OP_32_64(extract):
+mask = extract64(temps[args[1]].mask, args[2], args[3]);
+if (args[2] == 0) {
+affected = temps[args[1]].mask & ~mask;
+}
+break;
+CASE_OP_32_64(sextract):
+mask = sextract64(temps[args[1]].mask, args[2], args[3]);

[Qemu-devel] [PATCH 06/15] tcg/i386: Implement field extraction opcodes

[Richard Henderson]

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

diff --git a/tcg/i386/tcg-target.h b/tcg/i386/tcg-target.h
index 7625188..302ca96 100644
--- a/tcg/i386/tcg-target.h
+++ b/tcg/i386/tcg-target.h
@@ -94,8 +94,8 @@ extern bool have_bmi1;
 #define TCG_TARGET_HAS_nand_i32 0
 #define TCG_TARGET_HAS_nor_i32  0
 #define TCG_TARGET_HAS_deposit_i32  1
-#define TCG_TARGET_HAS_extract_i32  0
-#define TCG_TARGET_HAS_sextract_i32 0
+#define TCG_TARGET_HAS_extract_i32  1
+#define TCG_TARGET_HAS_sextract_i32 1
 #define TCG_TARGET_HAS_movcond_i32  1
 #define TCG_TARGET_HAS_add2_i32 1
 #define TCG_TARGET_HAS_sub2_i32 1
@@ -126,7 +126,7 @@ extern bool have_bmi1;
 #define TCG_TARGET_HAS_nand_i64 0
 #define TCG_TARGET_HAS_nor_i64  0
 #define TCG_TARGET_HAS_deposit_i64  1
-#define TCG_TARGET_HAS_extract_i64  0
+#define TCG_TARGET_HAS_extract_i64  1
 #define TCG_TARGET_HAS_sextract_i64 0
 #define TCG_TARGET_HAS_movcond_i64  1
 #define TCG_TARGET_HAS_add2_i64 1
@@ -142,6 +142,9 @@ extern bool have_bmi1;
  ((ofs) == 0 && (len) == 16))
 #define TCG_TARGET_deposit_i64_validTCG_TARGET_deposit_i32_valid
 
+#define TCG_TARGET_extract_i32_valid(ofs, len) ((ofs) == 8 && (len) == 8)
+#define TCG_TARGET_extract_i64_validTCG_TARGET_extract_i32_valid
+
 #if TCG_TARGET_REG_BITS == 64
 # define TCG_AREG0 TCG_REG_R14
 #else
diff --git a/tcg/i386/tcg-target.inc.c b/tcg/i386/tcg-target.inc.c
index eeb1777..091c6ff 100644
--- a/tcg/i386/tcg-target.inc.c
+++ b/tcg/i386/tcg-target.inc.c
@@ -2143,6 +2143,32 @@ static inline void tcg_out_op(TCGContext *s, TCGOpcode 
opc,
 }
 break;
 
+OP_32_64(extract):
+/* Note that TCG_TARGET_extract_*_valid only allows pos=8, len=8,
+   on the off-chance that we can use the high-byte registers.
+   Otherwise we emit the same ext16 + shift pattern that we would
+   have gotten from the normal tcg-op.c expansion.  */
+if (args[1] < 4 && args[0] < 8) {
+/* Do not set P_REXB_RM, so that we do get the %[abcd]h regs.  */
+tcg_out_modrm(s, OPC_MOVZBL, args[0], args[1] + 4);
+} else {
+tcg_out_ext16u(s, args[0], args[1]);
+tcg_out_shifti(s, SHIFT_SHR, args[0], 8);
+}
+break;
+
+case INDEX_op_sextract_i32:
+/* Note that we don't implement sextract_i64, as we cannot
+   sign-extend to 64-bits without using the REX prefix that
+   explicitly excludes access to the high-byte registers.  */
+if (args[1] < 4 && args[0] < 8) {
+tcg_out_modrm(s, OPC_MOVSBL, args[0], args[1] + 4);
+} else {
+tcg_out_ext16s(s, args[0], args[1], 0);
+tcg_out_shifti(s, SHIFT_SAR + rexw, args[0], args[0]);
+}
+break;
+
 case INDEX_op_mb:
 tcg_out_mb(s, args[0]);
 break;
@@ -2204,6 +2230,9 @@ static const TCGTargetOpDef x86_op_defs[] = {
 { INDEX_op_setcond_i32, { "q", "r", "ri" } },
 
 { INDEX_op_deposit_i32, { "Q", "0", "Q" } },
+{ INDEX_op_extract_i32, { "r", "r" } },
+{ INDEX_op_sextract_i32, { "r", "r" } },
+
 { INDEX_op_movcond_i32, { "r", "r", "ri", "r", "0" } },
 
 { INDEX_op_mulu2_i32, { "a", "d", "a", "r" } },
@@ -2265,6 +2294,7 @@ static const TCGTargetOpDef x86_op_defs[] = {
 { INDEX_op_extu_i32_i64, { "r", "r" } },
 
 { INDEX_op_deposit_i64, { "Q", "0", "Q" } },
+{ INDEX_op_extract_i64, { "r", "r" } },
 { INDEX_op_movcond_i64, { "r", "r", "re", "r", "0" } },
 
 { INDEX_op_mulu2_i64, { "a", "d", "a", "r" } },
-- 
2.7.4

[Qemu-devel] [PATCH 14/15] target-ppc: Use tcg_gen_extract_*

[Richard Henderson]

Use the new primitives for RDWINM and RLDICL.

Cc: qemu-...@nongnu.org
Signed-off-by: Richard Henderson 
---
 target-ppc/translate.c | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index bfc1301..724d95c 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -1977,9 +1977,8 @@ static void gen_rlwinm(DisasContext *ctx)
 if (mb == 0 && me == (31 - sh)) {
 tcg_gen_shli_tl(t_ra, t_rs, sh);
 tcg_gen_ext32u_tl(t_ra, t_ra);
-} else if (sh != 0 && me == 31 && sh == (32 - mb)) {
-tcg_gen_ext32u_tl(t_ra, t_rs);
-tcg_gen_shri_tl(t_ra, t_ra, mb);
+} else if (me == 31 && (me - mb + 1) + sh <= 32) {
+tcg_gen_extract_tl(t_ra, t_rs, sh, me - mb + 1);
 } else {
 target_ulong mask;
 #if defined(TARGET_PPC64)
@@ -2094,8 +2093,8 @@ static void gen_rldinm(DisasContext *ctx, int mb, int me, 
int sh)
 
 if (sh != 0 && mb == 0 && me == (63 - sh)) {
 tcg_gen_shli_tl(t_ra, t_rs, sh);
-} else if (sh != 0 && me == 63 && sh == (64 - mb)) {
-tcg_gen_shri_tl(t_ra, t_rs, mb);
+} else if (me == 63 && (me - mb + 1) + sh <= 64) {
+tcg_gen_extract_tl(t_ra, t_rs, sh, me - mb + 1);
 } else {
 tcg_gen_rotli_tl(t_ra, t_rs, sh);
 tcg_gen_andi_tl(t_ra, t_ra, MASK(mb, me));
-- 
2.7.4

[Qemu-devel] [PATCH 02/15] tcg: Minor adjustments to deposit expanders

[Richard Henderson]

Assert that len is not 0.

Since we have asserted that ofs + len <= N, a later
check for len == N implies that ofs == 0.

Signed-off-by: Richard Henderson 
---
 tcg/tcg-op.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tcg/tcg-op.c b/tcg/tcg-op.c
index cc4a331..39bab98 100644
--- a/tcg/tcg-op.c
+++ b/tcg/tcg-op.c
@@ -543,10 +543,11 @@ void tcg_gen_deposit_i32(TCGv_i32 ret, TCGv_i32 arg1, 
TCGv_i32 arg2,
 TCGv_i32 t1;
 
 tcg_debug_assert(ofs < 32);
+tcg_debug_assert(len > 0);
 tcg_debug_assert(len <= 32);
 tcg_debug_assert(ofs + len <= 32);
 
-if (ofs == 0 && len == 32) {
+if (len == 32) {
 tcg_gen_mov_i32(ret, arg2);
 return;
 }
@@ -1693,10 +1694,11 @@ void tcg_gen_deposit_i64(TCGv_i64 ret, TCGv_i64 arg1, 
TCGv_i64 arg2,
 TCGv_i64 t1;
 
 tcg_debug_assert(ofs < 64);
+tcg_debug_assert(len > 0);
 tcg_debug_assert(len <= 64);
 tcg_debug_assert(ofs + len <= 64);
 
-if (ofs == 0 && len == 64) {
+if (len == 64) {
 tcg_gen_mov_i64(ret, arg2);
 return;
 }
-- 
2.7.4

[Qemu-devel] [PATCH 15/15] target-s390: Use tcg_gen_extract_i64

[Richard Henderson]

Use the new primitive for RISBG.

Cc: Alexander Graf 
Signed-off-by: Richard Henderson 
---
 target-s390x/translate.c | 24 +++-
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/target-s390x/translate.c b/target-s390x/translate.c
index 02bc705..477238d 100644
--- a/target-s390x/translate.c
+++ b/target-s390x/translate.c
@@ -3134,20 +3134,26 @@ static ExitStatus op_risbg(DisasContext *s, DisasOps *o)
 }
 }
 
-/* In some cases we can implement this with deposit, which can be more
-   efficient on some hosts.  */
-if (~mask == imask && i3 <= i4) {
+len = i4 - i3 + 1;
+pos = 63 - i4;
+rot = i5 & 63;
+
+/* In some cases we can implement this with extract.  */
+if (imask == 0 && pos == 0 && len > 0 && rot + len <= 64) {
+tcg_gen_extract_i64(o->out, o->in2, rot, len);
+return NO_EXIT;
+}
+
+/* In some cases we can implement this with deposit.  */
+if (~mask == imask && len > 0) {
 if (s->fields->op2 == 0x5d) {
-i3 += 32, i4 += 32;
+pos += 32;
 }
 /* Note that we rotate the bits to be inserted to the lsb, not to
the position as described in the PoO.  */
-len = i4 - i3 + 1;
-pos = 63 - i4;
-rot = (i5 - pos) & 63;
+rot = (rot - pos) & 63;
 } else {
-pos = len = -1;
-rot = i5 & 63;
+pos = -1;
 }
 
 /* Rotate the input as necessary.  */
-- 
2.7.4

[Qemu-devel] [PATCH 12/15] target-i386: Use tcg_gen_extract_tl

[Richard Henderson]

Three places where it was easy to identify a right-shift
followed by an extract or and-with-immediate.

Signed-off-by: Richard Henderson 
---
 target-i386/translate.c | 45 +++--
 1 file changed, 23 insertions(+), 22 deletions(-)

diff --git a/target-i386/translate.c b/target-i386/translate.c
index 2f60e9c..4a3014c 100644
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -383,8 +383,7 @@ static void gen_op_mov_reg_v(TCGMemOp ot, int reg, TCGv t0)
 static inline void gen_op_mov_v_reg(TCGMemOp ot, TCGv t0, int reg)
 {
 if (ot == MO_8 && byte_reg_is_xH(reg)) {
-tcg_gen_shri_tl(t0, cpu_regs[reg - 4], 8);
-tcg_gen_ext8u_tl(t0, t0);
+tcg_gen_extract_tl(t0, cpu_regs[reg - 4], 8, 8);
 } else {
 tcg_gen_mov_tl(t0, cpu_regs[reg]);
 }
@@ -3715,8 +3714,7 @@ static void gen_sse(CPUX86State *env, DisasContext *s, 
int b,
 
 /* Extract the LEN into a mask.  Lengths larger than
operand size get all ones.  */
-tcg_gen_shri_tl(cpu_A0, cpu_regs[s->vex_v], 8);
-tcg_gen_ext8u_tl(cpu_A0, cpu_A0);
+tcg_gen_extract_tl(cpu_A0, cpu_regs[s->vex_v], 8, 8);
 tcg_gen_movcond_tl(TCG_COND_LEU, cpu_A0, cpu_A0, bound,
cpu_A0, bound);
 tcg_temp_free(bound);
@@ -3867,9 +3865,8 @@ static void gen_sse(CPUX86State *env, DisasContext *s, 
int b,
 gen_compute_eflags(s);
 }
 carry_in = cpu_tmp0;
-tcg_gen_shri_tl(carry_in, cpu_cc_src,
-ctz32(b == 0x1f6 ? CC_C : CC_O));
-tcg_gen_andi_tl(carry_in, carry_in, 1);
+tcg_gen_extract_tl(carry_in, cpu_cc_src,
+   ctz32(b == 0x1f6 ? CC_C : CC_O), 1);
 }
 
 switch (ot) {
@@ -5340,21 +5337,25 @@ static target_ulong disas_insn(CPUX86State *env, 
DisasContext *s,
 rm = (modrm & 7) | REX_B(s);
 
 if (mod == 3) {
-gen_op_mov_v_reg(ot, cpu_T0, rm);
-switch (s_ot) {
-case MO_UB:
-tcg_gen_ext8u_tl(cpu_T0, cpu_T0);
-break;
-case MO_SB:
-tcg_gen_ext8s_tl(cpu_T0, cpu_T0);
-break;
-case MO_UW:
-tcg_gen_ext16u_tl(cpu_T0, cpu_T0);
-break;
-default:
-case MO_SW:
-tcg_gen_ext16s_tl(cpu_T0, cpu_T0);
-break;
+if (s_ot == MO_SB && byte_reg_is_xH(rm)) {
+tcg_gen_sextract_tl(cpu_T0, cpu_regs[rm - 4], 8, 8);
+} else {
+gen_op_mov_v_reg(ot, cpu_T0, rm);
+switch (s_ot) {
+case MO_UB:
+tcg_gen_ext8u_tl(cpu_T0, cpu_T0);
+break;
+case MO_SB:
+tcg_gen_ext8s_tl(cpu_T0, cpu_T0);
+break;
+case MO_UW:
+tcg_gen_ext16u_tl(cpu_T0, cpu_T0);
+break;
+default:
+case MO_SW:
+tcg_gen_ext16s_tl(cpu_T0, cpu_T0);
+break;
+}
 }
 gen_op_mov_reg_v(d_ot, reg, cpu_T0);
 } else {
-- 
2.7.4

[Qemu-devel] [PATCH 03/15] tcg/aarch64: Implement field extraction opcodes

[Richard Henderson]

Cc: Claudio Fontana 
Signed-off-by: Richard Henderson 
---
 tcg/aarch64/tcg-target.h |  8 
 tcg/aarch64/tcg-target.inc.c | 14 ++
 2 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/tcg/aarch64/tcg-target.h b/tcg/aarch64/tcg-target.h
index 410c31b..4a74bd8 100644
--- a/tcg/aarch64/tcg-target.h
+++ b/tcg/aarch64/tcg-target.h
@@ -63,8 +63,8 @@ typedef enum {
 #define TCG_TARGET_HAS_nand_i32 0
 #define TCG_TARGET_HAS_nor_i32  0
 #define TCG_TARGET_HAS_deposit_i32  1
-#define TCG_TARGET_HAS_extract_i32  0
-#define TCG_TARGET_HAS_sextract_i32 0
+#define TCG_TARGET_HAS_extract_i32  1
+#define TCG_TARGET_HAS_sextract_i32 1
 #define TCG_TARGET_HAS_movcond_i32  1
 #define TCG_TARGET_HAS_add2_i32 1
 #define TCG_TARGET_HAS_sub2_i32 1
@@ -95,8 +95,8 @@ typedef enum {
 #define TCG_TARGET_HAS_nand_i64 0
 #define TCG_TARGET_HAS_nor_i64  0
 #define TCG_TARGET_HAS_deposit_i64  1
-#define TCG_TARGET_HAS_extract_i64  0
-#define TCG_TARGET_HAS_sextract_i64 0
+#define TCG_TARGET_HAS_extract_i64  1
+#define TCG_TARGET_HAS_sextract_i64 1
 #define TCG_TARGET_HAS_movcond_i64  1
 #define TCG_TARGET_HAS_add2_i64 1
 #define TCG_TARGET_HAS_sub2_i64 1
diff --git a/tcg/aarch64/tcg-target.inc.c b/tcg/aarch64/tcg-target.inc.c
index 1939d35..a496b3b 100644
--- a/tcg/aarch64/tcg-target.inc.c
+++ b/tcg/aarch64/tcg-target.inc.c
@@ -1640,6 +1640,16 @@ static void tcg_out_op(TCGContext *s, TCGOpcode opc,
 tcg_out_dep(s, ext, a0, REG0(2), args[3], args[4]);
 break;
 
+case INDEX_op_extract_i64:
+case INDEX_op_extract_i32:
+tcg_out_ubfm(s, ext, a0, a1, a2, args[3]);
+break;
+
+case INDEX_op_sextract_i64:
+case INDEX_op_sextract_i32:
+tcg_out_sbfm(s, ext, a0, a1, a2, args[3]);
+break;
+
 case INDEX_op_add2_i32:
 tcg_out_addsub2(s, TCG_TYPE_I32, a0, a1, REG0(2), REG0(3),
 (int32_t)args[4], args[5], const_args[4],
@@ -1785,6 +1795,10 @@ static const TCGTargetOpDef aarch64_op_defs[] = {
 
 { INDEX_op_deposit_i32, { "r", "0", "rZ" } },
 { INDEX_op_deposit_i64, { "r", "0", "rZ" } },
+{ INDEX_op_extract_i32, { "r", "r" } },
+{ INDEX_op_extract_i64, { "r", "r" } },
+{ INDEX_op_sextract_i32, { "r", "r" } },
+{ INDEX_op_sextract_i64, { "r", "r" } },
 
 { INDEX_op_add2_i32, { "r", "r", "rZ", "rZ", "rA", "rMZ" } },
 { INDEX_op_add2_i64, { "r", "r", "rZ", "rZ", "rA", "rMZ" } },
-- 
2.7.4

[Qemu-devel] [PATCH 00/15] tcg field extract primitives

[Richard Henderson]

I was fooling around with a new target last weekend, and got myself
all turned around with its field extract instruction.  How much more
handy would it be, I told myself, if we had such a thing generically?

In addition, many hosts have this natively.  So it seems a shame to
not take advantage of it when we can.

Lightly tested on x86_64, ppc64le, arm32, and s390x hosts.


r~


Richard Henderson (15):
  tcg: Add field extraction primitives
  tcg: Minor adjustments to deposit expanders
  tcg/aarch64: Implement field extraction opcodes
  tcg/arm: Move isa detection to tcg-target.h
  tcg/arm: Implement field extraction opcodes
  tcg/i386: Implement field extraction opcodes
  tcg/mips: Implement field extraction opcodes
  tcg/ppc: Implement field extraction opcodes
  tcg/s390: Implement field extraction opcodes
  target-alpha: Use deposit and extract ops
  target-arm: Use tcg_gen_*extract
  target-i386: Use tcg_gen_extract_tl
  target-mips: Use tcg_gen_extract_*
  target-ppc: Use tcg_gen_extract_*
  target-s390: Use tcg_gen_extract_i64

 target-alpha/translate.c |  67 +
 target-arm/translate-a64.c   |  48 ---
 target-arm/translate.c   |  37 ++---
 target-i386/translate.c  |  45 +++---
 target-mips/translate.c  |  12 +-
 target-ppc/translate.c   |   9 +-
 target-s390x/translate.c |  24 ++--
 tcg/aarch64/tcg-target.h |   4 +
 tcg/aarch64/tcg-target.inc.c |  14 ++
 tcg/arm/tcg-target.h |  38 +-
 tcg/arm/tcg-target.inc.c |  63 -
 tcg/i386/tcg-target.h|   7 +
 tcg/i386/tcg-target.inc.c|  30 
 tcg/ia64/tcg-target.h|   4 +
 tcg/mips/tcg-target.h|   2 +
 tcg/mips/tcg-target.inc.c|   4 +
 tcg/optimize.c   |  29 
 tcg/ppc/tcg-target.h |   4 +
 tcg/ppc/tcg-target.inc.c |  10 ++
 tcg/s390/tcg-target.h|  12 +-
 tcg/s390/tcg-target.inc.c|  13 +-
 tcg/sparc/tcg-target.h   |   4 +
 tcg/tcg-op.c | 319 ++-
 tcg/tcg-op.h |  12 ++
 tcg/tcg-opc.h|   4 +
 tcg/tcg.h|   8 ++
 tcg/tci/tcg-target.h |   4 +
 27 files changed, 655 insertions(+), 172 deletions(-)

-- 
2.7.4

Re: [Qemu-devel] [PATCH 05/15] util: add a helper to mmap private anonymous memory

[Michael S. Tsirkin]

On Tue, Jun 28, 2016 at 11:01:29AM +0200, Peter Lieven wrote:
> Signed-off-by: Peter Lieven 
> ---
>  include/qemu/mmap-alloc.h |  6 ++
>  util/mmap-alloc.c | 17 +
>  2 files changed, 23 insertions(+)
> 
> diff --git a/include/qemu/mmap-alloc.h b/include/qemu/mmap-alloc.h
> index 0899b2f..a457721 100644
> --- a/include/qemu/mmap-alloc.h
> +++ b/include/qemu/mmap-alloc.h
> @@ -9,4 +9,10 @@ void *qemu_ram_mmap(int fd, size_t size, size_t align, bool 
> shared);
>  
>  void qemu_ram_munmap(void *ptr, size_t size);
>  
> +/* qemu_anon_ram_mmap maps private anonymous memory using mmap and
> + * aborts if the allocation fails. its meant to act as an replacement
> + * for g_malloc0 and friends. */

This needs better docs. When should one use g_malloc0 and when
qemu_anon_ram_munmap?



> +void *qemu_anon_ram_mmap(size_t size);
> +void qemu_anon_ram_munmap(void *ptr, size_t size);
> +

The names are confusing - this isn't guest RAM, this is
just internal QEMU memory, isn't it?

Just rename it qemu_malloc0 then ...

>  #endif
> diff --git a/util/mmap-alloc.c b/util/mmap-alloc.c
> index 629d97a..c099858 100644
> --- a/util/mmap-alloc.c
> +++ b/util/mmap-alloc.c
> @@ -107,3 +107,20 @@ void qemu_ram_munmap(void *ptr, size_t size)
>  munmap(ptr, size + getpagesize());
>  }
>  }
> +
> +void *qemu_anon_ram_mmap(size_t size)
> +{
> +void *ptr = mmap(NULL, size, PROT_READ | PROT_WRITE,
> + MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> +if (ptr == MAP_FAILED) {
> +abort();
> +}
> +return ptr;
> +}
> +
> +void qemu_anon_ram_munmap(void *ptr, size_t size)
> +{
> +if (ptr) {
> +munmap(ptr, size);
> +}
> +}
> -- 
> 1.9.1

Re: [Qemu-devel] [PATCHv3 7/7] spapr: Improved placement of PCI host bridges in guest memory map

[David Gibson]

On Fri, Oct 14, 2016 at 09:25:58AM +0200, Laurent Vivier wrote:
> On 14/10/2016 01:29, David Gibson wrote:
> > From 98f07c09c6d72218800d6cfbf44b973a88ece2aa Mon Sep 17 00:00:00 2001
> > From: David Gibson 
> > Date: Fri, 14 Oct 2016 10:21:00 +1100
> > Subject: [PATCH] spapr: Improved placement of PCI host bridges in guest 
> > memory
> >  map
> > 
> > Currently, the MMIO space for accessing PCI on pseries guests begins at
> > 1 TiB in guest address space.  Each PCI host bridge (PHB) has a 64 GiB
> > chunk of address space in which it places its outbound PIO and 32-bit and
> > 64-bit MMIO windows.
> > 
> > This scheme as several problems:
> >   - It limits guest RAM to 1 TiB (though we have a limited fix for this
> > now)
> >   - It limits the total MMIO window to 64 GiB.  This is not always enough
> > for some of the large nVidia GPGPU cards
> >   - Putting all the windows into a single 64 GiB area means that naturally
> > aligning things within there will waste more address space.
> > In addition there was a miscalculation in some of the defaults, which meant
> > that the MMIO windows for each PHB actually slightly overran the 64 GiB
> > region for that PHB.  We got away without nasty consequences because
> > the overrun fit within an unused area at the beginning of the next PHB's
> > region, but it's not pretty.
> > 
> > This patch implements a new scheme which addresses those problems, and is
> > also closer to what bare metal hardware and pHyp guests generally use.
> > 
> > Because some guest versions (including most current distro kernels) can't
> > access PCI MMIO above 64 TiB, we put all the PCI windows between 32 TiB and
> > 64 TiB.  This is broken into 1 TiB chunks.  The 1 TiB contains the PIO
> > (64 kiB) and 32-bit MMIO (2 GiB) windows for all of the PHBs.  Each
> > subsequent TiB chunk contains a naturally aligned 64-bit MMIO window for
> > one PHB each.
> > 
> > This reduces the number of allowed PHBs (without full manual configuration
> > of all the windows) from 256 to 31, but this should still be plenty in
> > practice.
> > 
> > We also change some of the default window sizes for manually configured
> > PHBs to saner values.
> > 
> > Finally we adjust some tests and libqos so that it correctly uses the new
> > default locations.  Ideally it would parse the device tree given to the
> > guest, but that's a more complex problem for another time.
> > 
> > Signed-off-by: David Gibson 
> 
> 
> I really like this new version.
> 
> Reviewed-by: Laurent Vivier 

Great!  I've merged it into ppc-for-2.8.

> 
> Laurent
> 
> > ---
> >  hw/ppc/spapr.c  | 121 
> > +---
> >  hw/ppc/spapr_pci.c  |   5 +-
> >  include/hw/pci-host/spapr.h |   8 ++-
> >  tests/endianness-test.c |   3 +-
> >  tests/libqos/pci-spapr.c|   9 ++--
> >  tests/spapr-phb-test.c  |   2 +-
> >  6 files changed, 108 insertions(+), 40 deletions(-)
> > 
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index 8db3657..4bdf15b 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -2375,31 +2375,37 @@ static void spapr_phb_placement(sPAPRMachineState 
> > *spapr, uint32_t index,
> >  hwaddr *mmio32, hwaddr *mmio64,
> >  unsigned n_dma, uint32_t *liobns, Error 
> > **errp)
> >  {
> > +/*
> > + * New-style PHB window placement.
> > + *
> > + * Goals: Gives large (1TiB), naturally aligned 64-bit MMIO window
> > + * for each PHB, in addition to 2GiB 32-bit MMIO and 64kiB PIO
> > + * windows.
> > + *
> > + * Some guest kernels can't work with MMIO windows above 1<<46
> > + * (64TiB), so we place up to 31 PHBs in the area 32TiB..64TiB
> > + *
> > + * 32TiB..33TiB contains the PIO and 32-bit MMIO windows for all
> > + * PHBs.  33..34TiB has the 64-bit MMIO window for PHB0, 34..35
> > + * has the 64-bit window for PHB1 and so forth.
> > + */
> >  const uint64_t base_buid = 0x8002000ULL;
> > -const hwaddr phb_spacing = 0x10ULL; /* 64 GiB */
> > -const hwaddr mmio_offset = 0xa000; /* 2 GiB + 512 MiB */
> > -const hwaddr pio_offset = 0x8000; /* 2 GiB */
> > -const uint32_t max_index = 255;
> > -const hwaddr phb0_alignment = 0x100ULL; /* 1 TiB */
> > -
> > -uint64_t ram_top = MACHINE(spapr)->ram_size;
> > -hwaddr phb0_base, phb_base;
> > +const int max_phbs =
> > +(SPAPR_PCI_LIMIT - SPAPR_PCI_BASE) / SPAPR_PCI_MEM64_WIN_SIZE - 1;
> >  int i;
> >  
> > -/* Do we have hotpluggable memory? */
> > -if (MACHINE(spapr)->maxram_size > ram_top) {
> > -/* Can't just use maxram_size, because there may be an
> > - * alignment gap between normal and hotpluggable memory
> > - * regions */
> > -ram_top = spapr->hotplug_memory.base +
> > -

[Qemu-devel] [PATCH 4/4] Added error checking for msix_init.

[Shreya Shrivastava]

Add checks for negative return value to uses of msix_init.
Signed-off-by: Shreya Shrivastava 

---
 hw/usb/hcd-xhci.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 4acf0c6..cb854aa 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -3705,10 +3705,11 @@ static void usb_xhci_realize(struct PCIDevice
*dev, Error **errp)

 if (xhci->msix != ON_OFF_AUTO_OFF) {
 /* TODO check for errors */
-msix_init(dev, xhci->numintrs,
+ret = msix_init(dev, xhci->numintrs,
   >mem, 0, OFF_MSIX_TABLE,
   >mem, 0, OFF_MSIX_PBA,
   0x90);
+assert(ret >= 0);
 }
 }

-- 
1.9.1

[Qemu-devel] [PATCH 3/4] Add error checking for event_notifier_init.

[Shreya Shrivastava]

Add checks for negative return value to uses of event_notifier_init.
Signed-off-by: Shreya Shrivastava 

---
 tests/test-aio.c | 59

 1 file changed, 30 insertions(+), 29 deletions(-)

diff --git a/tests/test-aio.c b/tests/test-aio.c
index 03aa846..49b99f6 100644
--- a/tests/test-aio.c
+++ b/tests/test-aio.c
@@ -137,8 +137,8 @@ static void test_acquire(void)
 AcquireTestData data;

 /* Dummy event notifier ensures aio_poll() will block */
-event_notifier_init(, false);
-set_event_notifier(ctx, , dummy_notifier_read);
+if (event_notifier_init(, false))
+set_event_notifier(ctx, , dummy_notifier_read);
 g_assert(!aio_poll(ctx, false)); /* consume aio_notify() */

 qemu_mutex_init(_lock);
@@ -314,8 +314,8 @@ static void test_bh_flush(void)
 static void test_set_event_notifier(void)
 {
 EventNotifierTestData data = { .n = 0, .active = 0 };
-event_notifier_init(, false);
-set_event_notifier(ctx, , event_ready_cb);
+if (event_notifier_init(, false))
+set_event_notifier(ctx, , event_ready_cb);
 g_assert(!aio_poll(ctx, false));
 g_assert_cmpint(data.n, ==, 0);

@@ -328,8 +328,8 @@ static void test_set_event_notifier(void)
 static void test_wait_event_notifier(void)
 {
 EventNotifierTestData data = { .n = 0, .active = 1 };
-event_notifier_init(, false);
-set_event_notifier(ctx, , event_ready_cb);
+if (event_notifier_init(, false))
+set_event_notifier(ctx, , event_ready_cb);
 while (aio_poll(ctx, false));
 g_assert_cmpint(data.n, ==, 0);
 g_assert_cmpint(data.active, ==, 1);
@@ -353,8 +353,8 @@ static void test_wait_event_notifier(void)
 static void test_flush_event_notifier(void)
 {
 EventNotifierTestData data = { .n = 0, .active = 10, .auto_set = true };
-event_notifier_init(, false);
-set_event_notifier(ctx, , event_ready_cb);
+if (event_notifier_init(, false))
+set_event_notifier(ctx, , event_ready_cb);
 while (aio_poll(ctx, false));
 g_assert_cmpint(data.n, ==, 0);
 g_assert_cmpint(data.active, ==, 10);
@@ -381,9 +381,10 @@ static void test_aio_external_client(void)

 for (i = 1; i < 3; i++) {
 EventNotifierTestData data = { .n = 0, .active = 10, .auto_set =
true };
-event_notifier_init(, false);
-aio_set_event_notifier(ctx, , true, event_ready_cb);
-event_notifier_set();
+if (event_notifier_init(, false) {
+aio_set_event_notifier(ctx, , true, event_ready_cb);
+event_notifier_set();
+}
 for (j = 0; j < i; j++) {
 aio_disable_external(ctx);
 }
@@ -404,8 +405,8 @@ static void test_wait_event_notifier_noflush(void)
 EventNotifierTestData data = { .n = 0 };
 EventNotifierTestData dummy = { .n = 0, .active = 1 };

-event_notifier_init(, false);
-set_event_notifier(ctx, , event_ready_cb);
+if (event_notifier_init(, false))
+set_event_notifier(ctx, , event_ready_cb);

 g_assert(!aio_poll(ctx, false));
 g_assert_cmpint(data.n, ==, 0);
@@ -417,8 +418,8 @@ static void test_wait_event_notifier_noflush(void)
 data.n = 0;

 /* An active event notifier forces aio_poll to look at
EventNotifiers.  */
-event_notifier_init(, false);
-set_event_notifier(ctx, , event_ready_cb);
+if (event_notifier_init(, false)
+set_event_notifier(ctx, , event_ready_cb);

 event_notifier_set();
 g_assert(aio_poll(ctx, false));
@@ -458,8 +459,8 @@ static void test_timer_schedule(void)
 /* aio_poll will not block to wait for timers to complete unless it has
  * an fd to wait on. Fixing this breaks other tests. So create a
dummy one.
  */
-event_notifier_init(, false);
-set_event_notifier(ctx, , dummy_io_handler_read);
+if (event_notifier_init(, false))
+set_event_notifier(ctx, , dummy_io_handler_read);
 aio_poll(ctx, false);

 aio_timer_init(ctx, , data.clock_type,
@@ -668,8 +669,8 @@ static void test_source_bh_flush(void)
 static void test_source_set_event_notifier(void)
 {
 EventNotifierTestData data = { .n = 0, .active = 0 };
-event_notifier_init(, false);
-set_event_notifier(ctx, , event_ready_cb);
+if (event_notifier_init(, false)
+set_event_notifier(ctx, , event_ready_cb);
 while (g_main_context_iteration(NULL, false));
 g_assert_cmpint(data.n, ==, 0);

@@ -682,8 +683,8 @@ static void test_source_set_event_notifier(void)
 static void test_source_wait_event_notifier(void)
 {
 EventNotifierTestData data = { .n = 0, .active = 1 };
-event_notifier_init(, false);
-set_event_notifier(ctx, , event_ready_cb);
+if (event_notifier_init(, false))
+set_event_notifier(ctx, , event_ready_cb);
 while (g_main_context_iteration(NULL, false));
 g_assert_cmpint(data.n, ==, 0);
 g_assert_cmpint(data.active, ==, 1);
@@ -707,8 +708,8 @@ static void 

[Qemu-devel] [PATCH 2/4] Add error checking for load_image_targphys.

[Shreya Shrivastava]

Add checks for negative return value to uses of load_image_targphys.
Signed-off-by: Shreya Shrivastava 

---
 hw/arm/nseries.c  |  9 +++--
 hw/lm32/milkymist.c   | 19 +++
 hw/ppc/virtex_ml507.c |  5 +
 3 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/hw/arm/nseries.c b/hw/arm/nseries.c
index c86cf80..e0b0ae5 100644
--- a/hw/arm/nseries.c
+++ b/hw/arm/nseries.c
@@ -1306,6 +1306,7 @@ static int n810_atag_setup(const struct
arm_boot_info *info, void *p)
 static void n8x0_init(MachineState *machine,
   struct arm_boot_info *binfo, int model)
 {
+int rom_size;
 MemoryRegion *sysmem = get_system_memory();
 struct n800_s *s = (struct n800_s *) g_malloc0(sizeof(*s));
 int sdram_size = binfo->ram_size;
@@ -1379,10 +1380,14 @@ static void n8x0_init(MachineState *machine,
  *
  * The code above is for loading the `zImage' file from Nokia
  * images.  */
-load_image_targphys(option_rom[0].name,
+ rom_size = load_image_targphys(option_rom[0].name,
 OMAP2_Q2_BASE + 0x40,
 sdram_size - 0x40);
-
+ if (rom_size < 0) {
+   fprintf(stderr, "qemu: could not load rom file '%s'\n",
+   option_rom[0].name);
+   exit(1);
+}
 n800_setup_nolo_tags(nolo_tags);
 cpu_physical_memory_write(OMAP2_SRAM_BASE, nolo_tags, 0x1);
 g_free(nolo_tags);
diff --git a/hw/lm32/milkymist.c b/hw/lm32/milkymist.c
index 5cae0f1..2930f0b 100644
--- a/hw/lm32/milkymist.c
+++ b/hw/lm32/milkymist.c
@@ -86,7 +86,7 @@ milkymist_init(MachineState *machine)
 const char *initrd_filename = machine->initrd_filename;
 LM32CPU *cpu;
 CPULM32State *env;
-int kernel_size;
+int bios_size, kernel_size;
 DriveInfo *dinfo;
 MemoryRegion *address_space_mem = get_system_memory();
 MemoryRegion *phys_sdram = g_new(MemoryRegion, 1);
@@ -146,9 +146,14 @@ milkymist_init(MachineState *machine)
 bios_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);

 if (bios_filename) {
-load_image_targphys(bios_filename, BIOS_OFFSET, BIOS_SIZE);
-}
+bios_size = load_image_targphys(bios_filename, BIOS_OFFSET,
BIOS_SIZE);

+if (bios_size < 0) {
+fprintf(stderr, "qemu: could not load bios '%s'\n",
+bios_filename);
+exit(1);
+}
+ }
 reset_info->bootstrap_pc = BIOS_OFFSET;

 /* if no kernel is given no valid bios rom is a fatal error */
@@ -205,9 +210,15 @@ milkymist_init(MachineState *machine)
 }

 if (initrd_filename) {
-size_t initrd_size;
+int initrd_size;
 initrd_size = load_image_targphys(initrd_filename, initrd_base,
 initrd_max);
+
+if (initrd_size < 0) {
+fprintf(stderr, "qemu: could not load ram disk '%s'\n",
+initrd_filename);
+exit(1);
+}
 reset_info->initrd_base = (uint32_t)initrd_base;
 reset_info->initrd_size = (uint32_t)initrd_size;
 }
diff --git a/hw/ppc/virtex_ml507.c b/hw/ppc/virtex_ml507.c
index b97d966..eec7778 100644
--- a/hw/ppc/virtex_ml507.c
+++ b/hw/ppc/virtex_ml507.c
@@ -269,6 +269,11 @@ static void virtex_init(MachineState *machine)
 kernel_size = load_image_targphys(kernel_filename,
   boot_offset,
   ram_size);
+if (kernel_size < 0) {
+error_report("couldn't load kernel '%s'",
+  kernel_filename);
+exit(1);
+}
 boot_info.bootstrap_pc = boot_offset;
 high = boot_info.bootstrap_pc + kernel_size + 8192;
 }
-- 
1.9.1

[Qemu-devel] [PATCH 1/4] Add error checking for qemu_find_file.

[Shreya Shrivastava]

This patch adds check for NULL return value to uses of qemu_find_file.
 Signed-off-by: Shreya Shrivastava 

---
 hw/microblaze/boot.c  | 5 +
 hw/ppc/e500.c | 5 -
 hw/ppc/mac_newworld.c | 5 +
 hw/ppc/mac_oldworld.c | 4 
 4 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/hw/microblaze/boot.c b/hw/microblaze/boot.c
index 9eebb1a..ba06bf5 100644
--- a/hw/microblaze/boot.c
+++ b/hw/microblaze/boot.c
@@ -127,6 +127,11 @@ void microblaze_load_kernel(MicroBlazeCPU *cpu,
hwaddr ddr_base,
 /* default to pcbios dtb as passed by machine_init */
 if (!dtb_arg) {
 filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, dtb_filename);
+if (!filename) {
+error_report("qemu: could not load dtb file '%s'",
+ dtb_filename);
+exit(EXIT_FAILURE);
+}
 }

 boot_info.machine_cpu_reset = machine_cpu_reset;
diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index cf8b122..3fce082 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -1014,7 +1014,10 @@ void ppce500_init(MachineState *machine,
PPCE500Params *params)
 }
 }
 filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
-
+if (!filename) {
+fprintf(stderr, "Couldn't open bios file %s\n", bios_name);
+exit(1);
+}
 bios_size = load_elf(filename, NULL, NULL, _entry, , NULL,
  1, PPC_ELF_MACHINE, 0, 0);
 if (bios_size < 0) {
diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index 7d25106..9923f00 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -217,6 +217,11 @@ static void ppc_core99_init(MachineState *machine)
 if (bios_name == NULL)
 bios_name = PROM_FILENAME;
 filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
+if (!filename) {
+fprintf(stderr, "Couldn't open bios file %s\n",
+bios_name);
+exit(1);
+}
 memory_region_set_readonly(bios, true);
 memory_region_add_subregion(get_system_memory(), PROM_ADDR, bios);

diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index 4479487..3e44c41 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -145,6 +145,10 @@ static void ppc_heathrow_init(MachineState *machine)
 if (bios_name == NULL)
 bios_name = PROM_FILENAME;
 filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
+if (!filename) {
+fprintf(stderr, "Couldn't open bios file %s\n", bios_name);
+exit(1);
+}
 memory_region_set_readonly(bios, true);
 memory_region_add_subregion(sysmem, PROM_ADDR, bios);

-- 
1.9.1

[Qemu-devel] [PATCH 0/4] Error checking for NULL or negative return value.

[Shreya Shrivastava]

Hello,

This series adds basic error checking for negative and null value return.

Shreya.

Shreya Shrivastava (4):
  Add error checking for qemu_find_file.
  Add error checking for load_image_targphys.
  Add error checking for event_notifier_init.
  Add error checking for msix_init.

 hw/arm/nseries.c  |  9 ++--
 hw/lm32/milkymist.c   | 19 +
 hw/microblaze/boot.c  |  5 +
 hw/ppc/e500.c |  5 -
 hw/ppc/mac_newworld.c |  5 +
 hw/ppc/mac_oldworld.c |  4 
 hw/ppc/virtex_ml507.c |  5 +
 hw/usb/hcd-xhci.c |  3 ++-
 tests/test-aio.c  | 59
++-
 9 files changed, 77 insertions(+), 37 deletions(-)

-- 
1.9.1

Re: [Qemu-devel] [v2 5/5] qapi: allow blockdev-add for ssh

[Max Reitz]

On 15.10.2016 11:04, Ashijeet Acharya wrote:
> Introduce new object 'BlockdevOptionsSsh' in qapi/block-core.json to
> support blockdev-add for SSH network protocol driver. Use only 'struct
> InetSocketAddress' since SSH only supports connection over TCP.
> 
> Signed-off-by: Ashijeet Acharya 
> ---
>  qapi/block-core.json | 24 ++--
>  1 file changed, 22 insertions(+), 2 deletions(-)
> 
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 9d797b8..2e8a390 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -1716,7 +1716,8 @@
>  'dmg', 'file', 'ftp', 'ftps', 'gluster', 'host_cdrom',
>  'host_device', 'http', 'https', 'luks', 'null-aio', 'null-co',
>  'parallels', 'qcow', 'qcow2', 'qed', 'quorum', 'raw',
> - 'replication', 'tftp', 'vdi', 'vhdx', 'vmdk', 'vpc', 'vvfat' ] }
> +'replication', 'ssh', 'tftp', 'vdi', 'vhdx', 'vmdk', 'vpc',
> +'vvfat' ] }
>  
>  ##
>  # @BlockdevOptionsFile
> @@ -1953,6 +1954,25 @@
>  '*vport': 'int',
>  '*segment': 'str' } }
>  
> +##
> +# @BlockdevoptionsSsh

Should be *BlockdevOptionsSsh.

> +#
> +# @server:  host address and port number

It could be argued that the port number is part of the host address. I'd
therefore just describe it as "host address" since you can specify some
other options to, like @ipv6.

> +#
> +# @path:path to the image on the host
> +#
> +# @user:user as which to connect

This can actually be an optional argument, and I'd make it one (it
defaults to the current local user name).

> +#
> +# @host_key_check   defines how and what to check the host key against

As you can see from other descriptions, we normally put an "#optional"
in front of descriptions of optional parameters, and it's also a good
idea to specify the default behavior or value, which in this case is
"yes" - intuitively I'd have expected "no", so you should probably
indeed make a note of that.

Max

> +#
> +# Since 2.8
> +##
> +{ 'struct': 'BlockdevoptionsSsh',
> +  'data': { 'server': 'InetSocketAddress',
> +'path': 'str',
> +'user': 'str',
> +'*host_key_check': 'str' } }
> +
>  
>  ##
>  # @BlkdebugEvent
> @@ -2281,7 +2301,7 @@
>  # TODO rbd: Wait for structured options
>'replication':'BlockdevOptionsReplication',
>  # TODO sheepdog: Wait for structured options
> -# TODO ssh: Should take InetSocketAddress for 'host'?
> +  'ssh':'BlockdevoptionsSsh',
>'tftp':   'BlockdevOptionsCurl',
>'vdi':'BlockdevOptionsGenericFormat',
>'vhdx':   'BlockdevOptionsGenericFormat',
> 




signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [v2 4/5] block/ssh: Use InetSocketAddress options

[Max Reitz]

On 15.10.2016 11:04, Ashijeet Acharya wrote:
> Drop the use of legacy options in favour of the InetSocketAddress
> options.
> 
> Signed-off-by: Ashijeet Acharya 
> ---
>  block/ssh.c | 9 +
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/block/ssh.c b/block/ssh.c
> index 6420359..7fec0e1 100644
> --- a/block/ssh.c
> +++ b/block/ssh.c
> @@ -199,6 +199,7 @@ static int parse_uri(const char *filename, QDict 
> *options, Error **errp)
>  {
>  URI *uri = NULL;
>  QueryParams *qp;
> +char *port_str;
>  int i;
>  
>  uri = uri_parse(filename);
> @@ -231,11 +232,10 @@ static int parse_uri(const char *filename, QDict 
> *options, Error **errp)
>  qdict_put(options, "user", qstring_from_str(uri->user));
>  }
>  
> -qdict_put(options, "host", qstring_from_str(uri->server));
> +qdict_put(options, "server.host", qstring_from_str(uri->server));
>  
> -if (uri->port) {
> -qdict_put(options, "port", qint_from_int(uri->port));
> -}
> +port_str = g_strdup_printf("%d", uri->port ?: 22);
> +qdict_put(options, "server.port", qstring_from_str(port_str));
>  
>  qdict_put(options, "path", qstring_from_str(uri->path));
>  
> @@ -251,6 +251,7 @@ static int parse_uri(const char *filename, QDict 
> *options, Error **errp)
>  
>  query_params_free(qp);
>  uri_free(uri);
> +g_free(port_str);

I'd put this right after qdict_put(..., qstring_from_str(port_str));.
But that's up to you, either way:

Reviewed-by: Max Reitz 

>  return 0;
>  
>   err:
> 




signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [v2 3/5] block/ssh: Use inet_connect_saddr() to establish socket connection

[Max Reitz]

On 15.10.2016 11:04, Ashijeet Acharya wrote:
> Make inet_connect_saddr() in util/qemu-socktets.c public and use it

*util/qemu-sockets.c

> instead of inet_connect() because this directly takes the
> InetSocketAddress to establish a socket connection for the SSH
> block driver.
> 
> Signed-off-by: Ashijeet Acharya 
> ---
>  block/ssh.c| 5 +
>  include/qemu/sockets.h | 2 ++
>  util/qemu-sockets.c| 2 +-
>  3 files changed, 4 insertions(+), 5 deletions(-)

As I said for patch 3, I'd split this one and pull the part of it that
is making inet_connect_saddr() public in front of patch 2 and squash the
rest into patch 2.

> 
> diff --git a/block/ssh.c b/block/ssh.c
> index 3b18907..6420359 100644
> --- a/block/ssh.c
> +++ b/block/ssh.c
> @@ -666,13 +666,10 @@ static int connect_to_ssh(BDRVSSHState *s, QDict 
> *options,
>  goto err;
>  }
>  
> -/* Construct the host:port name for inet_connect. */
> -g_free(s->hostport);
>  port = atoi(s->inet->port);
> -s->hostport = g_strdup_printf("%s:%d", s->inet->host, port);
>  
>  /* Open the socket and connect. */
> -s->sock = inet_connect(s->hostport, errp);
> +s->sock = inet_connect_saddr(s->inet, errp, NULL, NULL);
>  if (s->sock < 0) {
>  ret = -EIO;
>  goto err;
> diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h
> index 9eb2470..5589e68 100644
> --- a/include/qemu/sockets.h
> +++ b/include/qemu/sockets.h
> @@ -34,6 +34,8 @@ typedef void NonBlockingConnectHandler(int fd, Error *err, 
> void *opaque);
>  
>  InetSocketAddress *inet_parse(const char *str, Error **errp);
>  int inet_connect(const char *str, Error **errp);
> +int inet_connect_saddr(InetSocketAddress *saddr, Error **errp,
> +   NonBlockingConnectHandler *callback, void *opaque);
>  
>  NetworkAddressFamily inet_netfamily(int family);
>  
> diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
> index 4cef549..3411888 100644
> --- a/util/qemu-sockets.c
> +++ b/util/qemu-sockets.c
> @@ -412,7 +412,7 @@ static struct addrinfo 
> *inet_parse_connect_saddr(InetSocketAddress *saddr,
>   * function succeeds, callback will be called when the connection
>   * completes, with the file descriptor on success, or -1 on error.
>   */
> -static int inet_connect_saddr(InetSocketAddress *saddr, Error **errp,
> +int inet_connect_saddr(InetSocketAddress *saddr, Error **errp,
>NonBlockingConnectHandler *callback, void 
> *opaque)

You should keep the second line aligned to the opening parenthesis.

Max

>  {
>  Error *local_err = NULL;
> 




signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [v2 2/5] block/ssh: Add InetSocketAddress and accept it

[Max Reitz]

On 15.10.2016 11:04, Ashijeet Acharya wrote:
> Add InetSocketAddress compatibility to SSH driver.
> 
> Add a new option "server" to the SSH block driver which then accepts
> a InetSocketAddress.
> 
> "host" and "port" are supported as legacy options and are mapped to
> their InetSocketAddress representation.
> 
> Signed-off-by: Ashijeet Acharya 
> ---
>  block/ssh.c | 83 
> ++---
>  1 file changed, 74 insertions(+), 9 deletions(-)
> 
> diff --git a/block/ssh.c b/block/ssh.c
> index 75cb7bc..3b18907 100644
> --- a/block/ssh.c
> +++ b/block/ssh.c
> @@ -30,10 +30,14 @@
>  #include "block/block_int.h"
>  #include "qapi/error.h"
>  #include "qemu/error-report.h"
> +#include "qemu/cutils.h"
>  #include "qemu/sockets.h"
>  #include "qemu/uri.h"
> +#include "qapi-visit.h"
>  #include "qapi/qmp/qint.h"
>  #include "qapi/qmp/qstring.h"
> +#include "qapi/qobject-input-visitor.h"
> +#include "qapi/qobject-output-visitor.h"
>  
>  /* DEBUG_SSH=1 enables the DPRINTF (debugging printf) statements in
>   * this block driver code.
> @@ -74,6 +78,8 @@ typedef struct BDRVSSHState {
>   */
>  LIBSSH2_SFTP_ATTRIBUTES attrs;
>  
> +InetSocketAddress *inet;
> +
>  /* Used to warn if 'flush' is not supported. */
>  char *hostport;
>  bool unsafe_flush_warning;
> @@ -263,7 +269,9 @@ static bool ssh_has_filename_options_conflict(QDict 
> *options, Error **errp)
>  !strcmp(qe->key, "port") ||
>  !strcmp(qe->key, "path") ||
>  !strcmp(qe->key, "user") ||
> -!strcmp(qe->key, "host_key_check"))
> +!strcmp(qe->key, "host_key_check") ||
> +!strcmp(qe->key, "server") ||

I know I've done the same in my series, but I'll actually drop this
condition from the next version; I'm not actually handling the case
where the destination address is not flattened, and neither are you
(we're both using qdict_extract_subqdict() instead of testing whether
"server" is an object on its own), so I think you should drop it as well
and just test for the prefix.

It doesn't harm to test for "server" itself, but I think it's a bit
confusing still, since you (we) are not dealing with that possibility
anywhere else.

> +!strstart(qe->key, "server.", NULL))

It should be just "strstart", not "!strstart", because the function
returns 1 if the prefix matches.

>  {
>  error_setg(errp, "Option '%s' cannot be used with a file name",
> qe->key);
> @@ -555,13 +563,66 @@ static QemuOptsList ssh_runtime_opts = {
>  },
>  };
>  
> +static bool ssh_process_legacy_socket_options(QDict *output_opts,
> +  QemuOpts *legacy_opts,
> +  Error **errp)
> +{
> +const char *host = qemu_opt_get(legacy_opts, "host");
> +const char *port = qemu_opt_get(legacy_opts, "port");
> +
> +if (!host && port) {
> +error_setg(errp, "port may not be used without host");
> +return false;
> +} else {
> +qdict_put(output_opts, "server.host", qstring_from_str(host));

This will segfault if host is NULL. You shouldn't go into this branch at
all if host is not set.

> +qdict_put(output_opts, "server.port",
> +  qstring_from_str(port ?: stringify(22)));
> +}
> +
> +return true;
> +}
> +
> +static InetSocketAddress *ssh_config(BDRVSSHState *s, QDict *options,
> + Error **errp)
> +{
> +InetSocketAddress *inet = NULL;
> +QDict *addr = NULL;
> +QObject *crumpled_addr = NULL;
> +Visitor *iv = NULL;
> +Error *local_error = NULL;
> +
> +qdict_extract_subqdict(options, , "server.");
> +if (!qdict_size(addr)) {
> +error_setg(errp, "SSH server address missing");
> +goto out;
> +}
> +
> +crumpled_addr = qdict_crumple(addr, true, errp);
> +if (!crumpled_addr) {
> +goto out;
> +}
> +
> +iv = qobject_input_visitor_new_autocast(crumpled_addr, true, 1, true);

In contrast to what Kevin said in v1, I think you do not want to use
autocast here.

Or, to be more specific, it's difficult. The thing is that the autocast
documentation says: "Any scalar values in the @obj input data structure
should always be represented as strings".

So if you do use the autocast version, command line works great because
from there everything comes as a string. But blockdev-add no longer
works because from there everything comes with the correct type (and you
cannot give it the wrong type). Case in point, this happens if you try
to create an SSH BDS with "'ipv4': true":

{"error": {"class": "GenericError", "desc": "Invalid parameter type for
'ipv4', expected: string"}}

OK, let's try "'ipv4': 'true'" then:

{"error": {"class": "GenericError", "desc": "Invalid parameter type for
'ipv4', expected: boolean"}}

Too bad. The former is a message from 

Re: [Qemu-devel] [PATCH v2] tap-bsd: OpenBSD uses tap(4) now

[Brad Smith]

On 10/06/16 21:28, Brad Smith wrote:

Update the tap-bsd code now that OpenBSD uses tap(4).

Signed-off-by: Brad Smith 
---
v2: Allow the code to deal with newer vs older OpenBSD releases


ping.


diff --git a/net/tap-bsd.c b/net/tap-bsd.c
index c506ac3..6c96922 100644
--- a/net/tap-bsd.c
+++ b/net/tap-bsd.c
@@ -35,6 +35,10 @@
 #include 
 #endif

+#if defined(__OpenBSD__)
+#include 
+#endif
+
 #ifndef __FreeBSD__
 int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
  int vnet_hdr_required, int mq_required, Error **errp)
@@ -55,7 +59,7 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
 if (*ifname) {
 snprintf(dname, sizeof dname, "/dev/%s", ifname);
 } else {
-#if defined(__OpenBSD__)
+#if defined(__OpenBSD__) && OpenBSD < 201605
 snprintf(dname, sizeof dname, "/dev/tun%d", i);
 #else
 snprintf(dname, sizeof dname, "/dev/tap%d", i);

Re: [Qemu-devel] [libvirt] inconsistent handling of "qemu64" CPU model

[Divan Santana]

> In your case, you should be able to use
>
> 
> qemu64
> 
> 
>
> to get the same CPU model you'd get by default (if not, you may need to
> also add ).
>
> Alternatively
>
> 
> qemu64
> 
> 
>
> should work too (and it would be better in case you use it on an AMD
> host).

Does anyone know how one can one make the above the default behavior on
a system, using libvirt?

The reason, is that I make use on some vagrant boxes, which have this
issue. I understand that the boxes have incorrectly configured and
hence I have this issue.

I'd prefer to set the above as default on my system, rather then editing
all the vagrant boxes I'm using

Re: [Qemu-devel] [v2 1/5] block/ssh: Add ssh_has_filename_options_conflict()

[Max Reitz]

On 15.10.2016 11:04, Ashijeet Acharya wrote:
> We have 5 options plus ("server") option which is added in the next
> patch that conflict with specifying a SSH filename. We need to iterate
> over all the options to check whether its key has an "server." prefix.
> 
> This iteration will help us adding the new option "server" easily.
> 
> Signed-off-by: Ashijeet Acharya 
> ---
>  block/ssh.c | 27 +--
>  1 file changed, 21 insertions(+), 6 deletions(-)

Reviewed-by: Max Reitz 



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 4/5] curses: add option to specify VGA font encoding

[Paolo Bonzini]

> +#ifdef CONFIG_ICONV
> +if (font_charset) {
> +unsigned char ch;
> +wchar_t wch;
> +char *pch, *pwch;
> +size_t sch, swch;
> +iconv_t conv;
> +
> +conv = iconv_open("WCHAR_T", font_charset);

Is this portable?  It seems safer to convert to UTF-8 and parse the
resulting character (assuming that wchar_t is Unicode).  There is
already mod_utf8_codepoint for this task.

Paolo

[Qemu-devel] [PATCH 0/5] curses: wide character support

[Samuel Thibault]

Hello,

This patch series adds wide character support to the curses frontend of qemu,
thus allowing to fix a lot of input and output issues with e.g. accented letters
and semi-graphic glyphs. Since qemu can't know the encoding of the VGA font, the
user has to specify it (just like he has to specify the keyboard layout with
-k). I used option -f to make it simple for now, but I welcome any other idea :)

Samuel

Samuel Thibault (5):
  curses: fix left/right arrow translation
  curses: Use cursesw instead of curses
  curses: use wide output functions
  curses: add option to specify VGA font encoding
  curses: support wide input

 configure   |  71 ++--
 hw/display/vga.c|   4 +-
 include/sysemu/sysemu.h |   1 +
 include/ui/console.h|  16 +-
 qemu-options.hx |   5 +-
 ui/curses.c | 436 +---
 ui/curses_keys.h| 113 +++--
 vl.c|  21 ++-
 8 files changed, 543 insertions(+), 124 deletions(-)

-- 
2.9.3

[Qemu-devel] [PATCH 5/5] curses: support wide input

[Samuel Thibault]

This makes use of wide curses functions instead of 8bit functions. This
allows to type e.g. accented letters.

Unfortunately, key codes are then returned with values that could be
confused with wide characters by ncurses, so we need to add a maybe_keycode
variable to know whether the returned value is a key code or a character
(wide support), or possibly both (non-wide support).

The translation tables thus also need to be separated into key code
translation and character translation.  The curses2foo helper makes it easier
to use them.

Signed-off-by: Samuel Thibault 
---
 ui/curses.c  |  76 ++---
 ui/curses_keys.h | 113 ---
 2 files changed, 127 insertions(+), 62 deletions(-)

diff --git a/ui/curses.c b/ui/curses.c
index f839a9d..441cc93 100644
--- a/ui/curses.c
+++ b/ui/curses.c
@@ -43,6 +43,12 @@
 #define FONT_HEIGHT 16
 #define FONT_WIDTH 8
 
+enum maybe_keycode {
+CURSES_KEYCODE,
+CURSES_CHAR,
+CURSES_CHAR_OR_KEYCODE,
+};
+
 static DisplayChangeListener *dcl;
 static console_ch_t screen[160 * 100];
 static WINDOW *screenpad = NULL;
@@ -51,6 +57,23 @@ static int px, py, sminx, sminy, smaxx, smaxy;
 
 console_ch_t vga_to_curses[256];
 
+static wint_t console_getch(int *maybe_keycode)
+{
+wint_t ret;
+switch (get_wch()) {
+case KEY_CODE_YES:
+*maybe_keycode = CURSES_KEYCODE;
+break;
+case OK:
+*maybe_keycode = CURSES_CHAR;
+break;
+case ERR:
+ret = -1;
+break;
+}
+return ret;
+}
+
 static void curses_update(DisplayChangeListener *dcl,
   int x, int y, int w, int h)
 {
@@ -186,9 +209,37 @@ static void curses_cursor_position(DisplayChangeListener 
*dcl,
 
 static kbd_layout_t *kbd_layout = NULL;
 
+static int curses2foo(const int _curses2foo[], const int _curseskey2foo[],
+  int chr, int maybe_keycode)
+{
+int ret = -1;
+if (maybe_keycode == CURSES_CHAR) {
+if (chr < CURSES_CHARS) {
+ret = _curses2foo[chr];
+}
+} else {
+if (chr < CURSES_KEYS) {
+ret = _curseskey2foo[chr];
+}
+if (ret == -1 && maybe_keycode == CURSES_CHAR_OR_KEYCODE &&
+chr < CURSES_CHARS) {
+ret = _curses2foo[chr];
+}
+}
+return ret;
+}
+
+#define curses2keycode(chr, maybe_keycode) \
+curses2foo(_curses2keycode, _curseskey2keycode, chr, maybe_keycode)
+#define curses2keysym(chr, maybe_keycode) \
+curses2foo(_curses2keysym, _curseskey2keysym, chr, maybe_keycode)
+#define curses2qemu(chr, maybe_keycode) \
+curses2foo(_curses2qemu, _curseskey2qemu, chr, maybe_keycode)
+
 static void curses_refresh(DisplayChangeListener *dcl)
 {
 int chr, keysym, keycode, keycode_alt;
+int maybe_keycode;
 
 curses_winch_check();
 
@@ -204,14 +255,14 @@ static void curses_refresh(DisplayChangeListener *dcl)
 
 while (1) {
 /* while there are any pending key strokes to process */
-chr = getch();
+chr = console_getch(_keycode);
 
-if (chr == ERR)
+if (chr == -1)
 break;
 
 #ifdef KEY_RESIZE
 /* this shouldn't occur when we use a custom SIGWINCH handler */
-if (chr == KEY_RESIZE) {
+if (maybe_keycode != 0 && chr == KEY_RESIZE) {
 clear();
 refresh();
 curses_calc_pad();
@@ -220,17 +271,19 @@ static void curses_refresh(DisplayChangeListener *dcl)
 }
 #endif
 
-keycode = curses2keycode[chr];
+keycode = curses2keycode(chr, maybe_keycode);
 keycode_alt = 0;
 
 /* alt or esc key */
 if (keycode == 1) {
-int nextchr = getch();
+int next_maybe_keycode;
+int nextchr = console_getch(_maybe_keycode);
 
-if (nextchr != ERR) {
+if (nextchr != -1) {
 chr = nextchr;
+maybe_keycode = next_maybe_keycode;
 keycode_alt = ALT;
-keycode = curses2keycode[chr];
+keycode = curses2keycode(chr, maybe_keycode);
 
 if (keycode != -1) {
 keycode |= ALT;
@@ -250,9 +303,7 @@ static void curses_refresh(DisplayChangeListener *dcl)
 }
 
 if (kbd_layout) {
-keysym = -1;
-if (chr < CURSES_KEYS)
-keysym = curses2keysym[chr];
+keysym = curses2keysym(chr, maybe_keycode);
 
 if (keysym == -1) {
 if (chr < ' ') {
@@ -317,10 +368,7 @@ static void curses_refresh(DisplayChangeListener *dcl)
 qemu_input_event_send_key_delay(0);
 }
 } else {
-keysym = -1;
-if (chr < CURSES_KEYS) {
-keysym = curses2qemu[chr];
-}
+keysym = curses2qemu(chr, maybe_keycode);
 if (keysym == -1)
   

[Qemu-devel] [PATCH 3/5] curses: use wide output functions

[Samuel Thibault]

This makes use of cchar_t instead of chtype when using ncursesw, which
allows to store a wide char as well as the WACS values.

This also allows to complete the printable glyphs list beyond ascii and
the ACS values.

Signed-off-by: Samuel Thibault 
---
 configure|   5 +-
 hw/display/vga.c |   4 +-
 include/ui/console.h |  16 ++-
 ui/curses.c  | 321 ---
 4 files changed, 294 insertions(+), 52 deletions(-)

diff --git a/configure b/configure
index 29649d9..ebe0599 100755
--- a/configure
+++ b/configure
@@ -2901,14 +2901,17 @@ if test "$curses" != "no" ; then
 #include 
 #include 
 #include 
+#include 
 int main(void) {
   const char *s = curses_version();
+  const char *codeset;
   wchar_t wch = L'w';
   setlocale(LC_ALL, "");
   resize_term(0, 0);
   addwstr(L"wide chars\n");
   addnwstr(, 1);
-  return s != 0;
+  codeset = nl_langinfo(CODESET);
+  return s != 0 && codeset != 0;
 }
 EOF
   IFS=:
diff --git a/hw/display/vga.c b/hw/display/vga.c
index 2a88b3c..ce225c9 100644
--- a/hw/display/vga.c
+++ b/hw/display/vga.c
@@ -1966,7 +1966,7 @@ static void vga_update_text(void *opaque, console_ch_t 
*chardata)
 
 for (i = 0; i < size; src ++, dst ++, i ++) {
 console_write_ch(, VMEM2CHTYPE(le32_to_cpu(*src)));
-if (*dst != val) {
+if (memcmp(dst, , sizeof(val))) {
 *dst = val;
 c_max = i;
 break;
@@ -1975,7 +1975,7 @@ static void vga_update_text(void *opaque, console_ch_t 
*chardata)
 c_min = i;
 for (; i < size; src ++, dst ++, i ++) {
 console_write_ch(, VMEM2CHTYPE(le32_to_cpu(*src)));
-if (*dst != val) {
+if (memcmp(dst, , sizeof(val))) {
 *dst = val;
 c_max = i;
 }
diff --git a/include/ui/console.h b/include/ui/console.h
index d9c13d2..94e3133 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -338,8 +338,8 @@ static inline pixman_format_code_t 
surface_format(DisplaySurface *s)
 
 #ifdef CONFIG_CURSES
 #include 
-typedef chtype console_ch_t;
-extern chtype vga_to_curses[];
+typedef cchar_t console_ch_t;
+extern console_ch_t vga_to_curses[];
 #else
 typedef unsigned long console_ch_t;
 #endif
@@ -347,16 +347,20 @@ static inline void console_write_ch(console_ch_t *dest, 
uint32_t ch)
 {
 uint8_t c = ch;
 #ifdef CONFIG_CURSES
-if (vga_to_curses[c]) {
-ch &= ~(console_ch_t)0xff;
-ch |= vga_to_curses[c];
+if (vga_to_curses[c].chars[0]) {
+*dest = vga_to_curses[c];
+} else {
+dest->chars[0] = c;
+dest->chars[1] = 0;
+dest->attr = 0;
 }
+dest->attr |= ch & ~0xff;
 #else
 if (c == '\0') {
 ch |= ' ';
 }
-#endif
 *dest = ch;
+#endif
 }
 
 typedef struct GraphicHwOps {
diff --git a/ui/curses.c b/ui/curses.c
index 3599295..cd8ed5f 100644
--- a/ui/curses.c
+++ b/ui/curses.c
@@ -28,6 +28,9 @@
 #include 
 #include 
 #endif
+#include 
+#include 
+#include 
 
 #include "qemu-common.h"
 #include "ui/console.h"
@@ -43,16 +46,17 @@ static WINDOW *screenpad = NULL;
 static int width, height, gwidth, gheight, invalidate;
 static int px, py, sminx, sminy, smaxx, smaxy;
 
-chtype vga_to_curses[256];
+console_ch_t vga_to_curses[256];
 
 static void curses_update(DisplayChangeListener *dcl,
   int x, int y, int w, int h)
 {
-chtype *line;
+console_ch_t *line;
 
-line = ((chtype *) screen) + y * width;
-for (h += y; y < h; y ++, line += width)
-mvwaddchnstr(screenpad, y, 0, line, width);
+line = ((console_ch_t *) screen) + y * width;
+for (h += y; y < h; y ++, line += width) {
+mvwadd_wchnstr(screenpad, y, 0, line, width);
+}
 
 pnoutrefresh(screenpad, py, px, sminy, sminx, smaxy - 1, smaxx - 1);
 refresh();
@@ -358,45 +362,275 @@ static void curses_setup(void)
 
 /*
  * Setup mapping for vga to curses line graphics.
- * FIXME: for better font, have to use ncursesw and setlocale()
  */
-#if 0
-/* FIXME: map from where? */
-ACS_S1;
-ACS_S3;
-ACS_S7;
-ACS_S9;
-#endif
-/* ACS_* is not constant. So, we can't initialize statically. */
-vga_to_curses['\0'] = ' ';
-vga_to_curses[0x04] = ACS_DIAMOND;
-vga_to_curses[0x18] = ACS_UARROW;
-vga_to_curses[0x19] = ACS_DARROW;
-vga_to_curses[0x1a] = ACS_RARROW;
-vga_to_curses[0x1b] = ACS_LARROW;
-vga_to_curses[0x9c] = ACS_STERLING;
-vga_to_curses[0xb0] = ACS_BOARD;
-vga_to_curses[0xb1] = ACS_CKBOARD;
-vga_to_curses[0xb3] = ACS_VLINE;
-vga_to_curses[0xb4] = ACS_RTEE;
-vga_to_curses[0xbf] = ACS_URCORNER;
-vga_to_curses[0xc0] = ACS_LLCORNER;
-vga_to_curses[0xc1] = ACS_BTEE;
-vga_to_curses[0xc2] = ACS_TTEE;
-vga_to_curses[0xc3] = ACS_LTEE;
-vga_to_curses[0xc4] = ACS_HLINE;

[Qemu-devel] [PATCH 2/5] curses: Use cursesw instead of curses

[Samuel Thibault]

Use ncursesw package instead of curses on non-mingw, and check a few
functions.
Also take cflags from pkg-config, since cursesw headers may be in a
separate, non-default directory.

Signed-off-by: Samuel Thibault 
---
 configure | 29 -
 1 file changed, 20 insertions(+), 9 deletions(-)

diff --git a/configure b/configure
index 7d083bd..29649d9 100755
--- a/configure
+++ b/configure
@@ -2890,27 +2890,38 @@ fi
 # curses probe
 if test "$curses" != "no" ; then
   if test "$mingw32" = "yes" ; then
-curses_list="$($pkg_config --libs ncurses 2>/dev/null):-lpdcurses"
+curses_inc_list="$($pkg_config --cflags ncurses 2>/dev/null):"
+curses_lib_list="$($pkg_config --libs ncurses 2>/dev/null):-lpdcurses"
   else
-curses_list="$($pkg_config --libs ncurses 2>/dev/null):-lncurses:-lcurses"
+curses_inc_list="$($pkg_config --cflags ncursesw 2>/dev/null):"
+curses_lib_list="$($pkg_config --libs ncursesw 
2>/dev/null):-lncursesw:-lcursesw"
   fi
   curses_found=no
   cat > $TMPC << EOF
+#include 
 #include 
+#include 
 int main(void) {
   const char *s = curses_version();
+  wchar_t wch = L'w';
+  setlocale(LC_ALL, "");
   resize_term(0, 0);
+  addwstr(L"wide chars\n");
+  addnwstr(, 1);
   return s != 0;
 }
 EOF
   IFS=:
-  for curses_lib in $curses_list; do
-unset IFS
-if compile_prog "" "$curses_lib" ; then
-  curses_found=yes
-  libs_softmmu="$curses_lib $libs_softmmu"
-  break
-fi
+  for curses_inc in $curses_inc_list; do
+for curses_lib in $curses_lib_list; do
+  unset IFS
+  if compile_prog "$curses_inc" "$curses_lib" ; then
+curses_found=yes
+QEMU_CFLAGS="$curses_inc $QEMU_CFLAGS"
+libs_softmmu="$curses_lib $libs_softmmu"
+break
+  fi
+done
   done
   unset IFS
   if test "$curses_found" = "yes" ; then
-- 
2.9.3

[Qemu-devel] [PATCH 1/5] curses: fix left/right arrow translation

[Samuel Thibault]

In default VGA font, left/right arrow are glyphs 0x1a and 0x1b, not 0x0a and
0x0b.

Signed-off-by: Samuel Thibault 
---
 ui/curses.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ui/curses.c b/ui/curses.c
index c8be4b7..3599295 100644
--- a/ui/curses.c
+++ b/ui/curses.c
@@ -370,10 +370,10 @@ static void curses_setup(void)
 /* ACS_* is not constant. So, we can't initialize statically. */
 vga_to_curses['\0'] = ' ';
 vga_to_curses[0x04] = ACS_DIAMOND;
-vga_to_curses[0x0a] = ACS_RARROW;
-vga_to_curses[0x0b] = ACS_LARROW;
 vga_to_curses[0x18] = ACS_UARROW;
 vga_to_curses[0x19] = ACS_DARROW;
+vga_to_curses[0x1a] = ACS_RARROW;
+vga_to_curses[0x1b] = ACS_LARROW;
 vga_to_curses[0x9c] = ACS_STERLING;
 vga_to_curses[0xb0] = ACS_BOARD;
 vga_to_curses[0xb1] = ACS_CKBOARD;
-- 
2.9.3

[Qemu-devel] [PATCH 4/5] curses: add option to specify VGA font encoding

[Samuel Thibault]

This detects and uses iconv to convert glyphs from the specified VGA font
encoding to unicode.

Signed-off-by: Samuel Thibault 
---
 configure   | 37 +
 include/sysemu/sysemu.h |  1 +
 qemu-options.hx |  5 -
 ui/curses.c | 41 +
 vl.c| 21 -
 5 files changed, 103 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index ebe0599..2ecd03a 100755
--- a/configure
+++ b/configure
@@ -203,6 +203,7 @@ bluez=""
 brlapi=""
 curl=""
 curses=""
+cursesw=""
 docs=""
 fdt=""
 netmap="no"
@@ -984,6 +985,10 @@ for opt do
   ;;
   --enable-curses) curses="yes"
   ;;
+  --disable-iconv) iconv="no"
+  ;;
+  --enable-iconv) iconv="yes"
+  ;;
   --disable-curl) curl="no"
   ;;
   --enable-curl) curl="yes"
@@ -1351,6 +1356,7 @@ disabled with --disable-FEATURE, default is enabled if 
available:
   --with-gtkabi select preferred GTK ABI 2.0 or 3.0
   vte vte support for the gtk UI
   curses  curses UI
+  iconv   font glyph conversion support
   vnc VNC UI support
   vnc-saslSASL encryption for VNC server
   vnc-jpegJPEG lossy compression for VNC server
@@ -2938,6 +2944,33 @@ EOF
 fi
 
 ##
+# iconv probe
+if test "$iconv" != "no" ; then
+  cat > $TMPC << EOF
+#include 
+int main(void) {
+  iconv_t conv = iconv_open("ISO-8859-1", "ISO-8859-1");
+  return conv != (iconv_t) -1;
+}
+EOF
+  for iconv_lib in '' -liconv; do
+if compile_prog "" "$iconv_lib" ; then
+  iconv_found=yes
+  libs_softmmu="$iconv_lib $libs_softmmu"
+  break
+fi
+  done
+  if test "$iconv_found" = "yes" ; then
+iconv=yes
+  else
+if test "$iconv" = "yes" ; then
+  feature_not_found "iconv" "Install iconv devel"
+fi
+iconv=no
+  fi
+fi
+
+##
 # curl probe
 if test "$curl" != "no" ; then
   if $pkg_config libcurl --exists; then
@@ -4860,6 +4893,7 @@ echo "nettle$nettle $(echo_version $nettle 
$nettle_version)"
 echo "nettle kdf$nettle_kdf"
 echo "libtasn1  $tasn1"
 echo "curses support$curses"
+echo "iconv support $iconv"
 echo "virgl support $virglrenderer"
 echo "curl support  $curl"
 echo "mingw32 support   $mingw32"
@@ -5120,6 +5154,9 @@ fi
 if test "$curses" = "yes" ; then
   echo "CONFIG_CURSES=y" >> $config_host_mak
 fi
+if test "$iconv" = "yes" ; then
+  echo "CONFIG_ICONV=y" >> $config_host_mak
+fi
 if test "$utimens" = "yes" ; then
   echo "CONFIG_UTIMENSAT=y" >> $config_host_mak
 fi
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index ee7c760..b90b9b1 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -148,6 +148,7 @@ extern int graphic_height;
 extern int graphic_depth;
 extern int display_opengl;
 extern const char *keyboard_layout;
+extern const char *font_charset;
 extern int win2k_install_hack;
 extern int alt_grab;
 extern int ctrl_grab;
diff --git a/qemu-options.hx b/qemu-options.hx
index 0b621bb..c82a26e 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -930,7 +930,7 @@ DEF("display", HAS_ARG, QEMU_OPTION_display,
 "[,window_close=on|off][,gl=on|off]|curses|none|\n"
 "-display gtk[,grab_on_hover=on|off][,gl=on|off]|\n"
 "-display vnc=[,]\n"
-"-display curses\n"
+"-display curses[,charset=]\n"
 "-display none"
 "select display type\n"
 "The default display is equivalent to\n"
@@ -961,6 +961,9 @@ support a text mode, QEMU can display this output using a
 curses/ncurses interface. Nothing is displayed when the graphics
 device is in graphical mode or if the graphics device does not support
 a text mode. Generally only the VGA device models support text mode.
+The font charset used by the guest can be specified with the
+@code{charset} option, for example @code{charset=CP850} for IBM CP850
+encoding. The default is @code{CP437}.
 @item none
 Do not display video output. The guest will still see an emulated
 graphics card, but its output will not be displayed to the QEMU
diff --git a/ui/curses.c b/ui/curses.c
index cd8ed5f..f839a9d 100644
--- a/ui/curses.c
+++ b/ui/curses.c
@@ -31,6 +31,9 @@
 #include 
 #include 
 #include 
+#ifdef CONFIG_ICONV
+#include 
+#endif
 
 #include "qemu-common.h"
 #include "ui/console.h"
@@ -397,6 +400,44 @@ static void curses_setup(void)
 vga_to_curses[0x1e].chars[0] = L'\u25b2';
 vga_to_curses[0x1f].chars[0] = L'\u25bc';
 
+#ifdef CONFIG_ICONV
+if (font_charset) {
+unsigned char ch;
+wchar_t wch;
+char *pch, *pwch;
+size_t sch, swch;
+iconv_t conv;
+
+conv = iconv_open("WCHAR_T", font_charset);
+if (conv == (iconv_t) -1) {
+fprintf(stderr, "Could not convert font glyphs from %s: '%s'\n",
+font_charset, 

Re: [Qemu-devel] [Qemu-block] [PATCH v4 0/3] iotests: Fix test 162

[Max Reitz]

On 13.10.2016 07:20, Hao QingFeng wrote:
> 
> 
> 在 2016-10-13 3:46, Max Reitz 写道:
>> On 12.10.2016 10:55, Hao QingFeng wrote:
>>> Max,
>>>
>>> Just a common question for this case, if sshx block driver wasn't built
>>> into qemu-img, this case would fail as below:
>> Good point, and thanks for bringing it up, but it's not directly linked
>> to this series other than by its subject, of course, so I'd rather add a
>> fix on top.
> Thanks and sorry for sending to the improper mail series.
>>> exec /home/haoqf/KVMonz/qemu/tests/qemu-iotests/../../qemu-img info
>>> --image-opts driver=ssh,host=localhost,port=0.42,path=/foo
>>> qemu-img: Could not open
>>> 'driver=ssh,host=localhost,port=0.42,path=/foo': Unknown driver 'ssh'
>>>
>>> Adding 162.notrun can bypass this case but it would skip it even if
>>> qemu-img has sshx block driver, in which case I think it should be run.
>>>
>>> So How about adding a script to dynamically check at runtime if the
>>> current env qemu-img can meet the requirement to run the test or not?
>> Unfortunately, the list of block drivers listed by will not contain ssh
>> if ssh is built as a module, which is possible.
> Actually I am not sure if I understood it. Do you mean
> "CONFIG_LIBSSH2=m" set
> rather than "CONFIG_LIBSSH2=y" in config-host.mak? But in the configure
> it's
> set to be "CONFIG_LIBSSH2=y":
> if test "$libssh2" = "yes" ; then
>   echo "CONFIG_LIBSSH2=y" >> $config_host_mak
>   echo "LIBSSH2_CFLAGS=$libssh2_cflags" >> $config_host_mak
>   echo "LIBSSH2_LIBS=$libssh2_libs" >> $config_host_mak
> fi

I don't know which version of qemu you are looking at, but on master it
says "m" instead of "y" there:

http://git.qemu.org/?p=qemu.git;a=blob;f=configure;h=dd9e6792bbe04411d81eb5438d58eb1999d4dcd2;hb=HEAD#l5477

> Meanwhile I changed it to be "CONFIG_LIBSSH2=m" and reconfig, make the
> qemu,
> qemu-img --help can still prompt ssh.

Have you tried building master with --enable-modules specified for
configure?

Max

>> This is a bug that should be fixed, but I'd rather do so in a separate
>> series from this one.
>>
>> In any case, once it is fixed I'd rather just take the approach quorum
>> tests take already (e.g. test 081), which is something like:
>>
>> test_ssh=$($QEMU_IMG --help | grep '^Supported formats:.* ssh\( \|$\)')
>> [ "$test_ssh" = "" ] && _notrun "ssh support required"
> Cool. Agree with this like what was done in 081.  thanks
>> Max
>>
> 




signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH] Reduce curses escdelay from 1s to 0.2s

[Samuel Thibault]

Hello,

Peter Maydell, on Wed 22 Jun 2016 21:49:04 +0100, wrote:
> On 22 June 2016 at 16:44, Samuel Thibault  
> wrote:
> > By default, curses will only report single ESC key event after 1s delay,
> > since ESC is also used for keypad escape sequences. This however makes users
> > believe that ESC is not working. Reducing to 0.2s provides good enough user
> > experience, while still allowing 200ms for keypad sequences to get in, which
> > should be more than enough.
> 
> That the default delay is this massive seems like a bug in curses,
> but I don't suppose it's likely to be changed at this point :-(

So, could this be applied?  Otherwise e.g. DOS applications ESC actions
are very slugguish and thus look bogus in the ncurses UI.

Samuel

Re: [Qemu-devel] [PATCH 07/22] qcow2-bitmap: introduce auto-loading bitmaps

[Vladimir Sementsov-Ogievskiy]

On 15.10.2016 20:03, Max Reitz wrote:

On 14.10.2016 20:44, Vladimir Sementsov-Ogievskiy wrote:

On 01.10.2016 19:26, Max Reitz wrote:

On 30.09.2016 12:53, Vladimir Sementsov-Ogievskiy wrote:

Auto loading bitmaps are bitmaps in Qcow2, with AUTO flag set. They are

...


+goto fail;
+}
+
+/* loop is safe because next entry offset is calculated after
conversion to
Should it be s/safe/unsafe/?

loop is safe. _unsafe is related to absence of assert in a loop, as it
loops through BE data. Bad wording, I'll change it somehow..

Yes, I know that the loop is safe in the sense of the word "safe", but I
meant that it's kind of confusing that the loop uses
"for_each_bitmap_dir_entry_unsafe" and thus is "unsafe", too.

Another idea would be to write "This is actually safe" instead of "loop
is safe".


Finally I've decided to introduce normal list of normal structures like 
in snapshots..





+ * cpu format */
+for_each_bitmap_dir_entry_unsafe(e, dir, size) {
+if ((uint8_t *)(e + 1) > dir_end) {
+goto broken_dir;
+}
+
+bitmap_dir_entry_to_cpu(e);
+
+if ((uint8_t *)next_dir_entry(e) > dir_end) {
+goto broken_dir;
+}
+
+if (e->extra_data_size != 0) {
+error_setg(errp, "Can't load bitmap '%.*s' from '%s':"
+   "extra data not supported.", e->name_size,

Full stop again.

Also, I'm not quite sure why you give the device/node name here. You
don't do that anywhere else and I think if we want to emit the
information where something failed, it should be added at some previous
point in the call chain.

For example, how? As I understand, I can emit it only by error_setg, so
actually it would be better to add node and bitmap name to all
error_setg in the code.. or create helper function for it.

error_prepend() would be the function.

This code will be invoked by any code that is opening an image. There
are of course a couple of places where that can be the case: For
external tools such as qemu-img, it's normally pretty clear which image
is meant (and it additionally uses error_reportf_err() to give you more
information).

For -drive, every error message will at least be prepended by the
corresponding -drive parameter, so that will help a bit.

blockdev-add, unfortunately, doesn't do anything like this. But the user
can of course choose to construct the BDS graph node by node and thus
always know which node an error originates from.

Anyway, if you want to add this information to every error message, you
should probably do so in bdrv_open_inherit(). The issue I'd take with
this is that most users probably won't set the node name themselves
(either they don't at all or it's some management tool that does), so
reporting the node name doesn't actually help them at all (and
management tools are not supposed to parse error messages, so it won't
help in that case either).

Max



Thanks for explanations, and for the whole review, it's great! Sorry for 
my laziness and for spelling(

O! I've discovered vim spell, so one problem less, I hope.

--
Best regards,
Vladimir

Re: [Qemu-devel] [PATCH v4 12/12] iotests: Add test for NBD's blockdev-add interface

[Max Reitz]

On 13.10.2016 15:26, Kevin Wolf wrote:
> Am 28.09.2016 um 22:56 hat Max Reitz geschrieben:
>> Signed-off-by: Max Reitz 
>> ---
>>  tests/qemu-iotests/147 | 201 
>> +
>>  tests/qemu-iotests/147.out |   5 ++
>>  tests/qemu-iotests/group   |   1 +
>>  3 files changed, 207 insertions(+)
>>  create mode 100755 tests/qemu-iotests/147
>>  create mode 100644 tests/qemu-iotests/147.out
>>
>> diff --git a/tests/qemu-iotests/147 b/tests/qemu-iotests/147
>> new file mode 100755
>> index 000..61e1e6f
>> --- /dev/null
>> +++ b/tests/qemu-iotests/147
>> @@ -0,0 +1,201 @@
>> +#!/usr/bin/env python
>> +#
>> +# Test case for NBD's blockdev-add interface
>> +#
>> +# Copyright (C) 2016 Red Hat, Inc.
>> +#
>> +# This program is free software; you can redistribute it and/or modify
>> +# it under the terms of the GNU General Public License as published by
>> +# the Free Software Foundation; either version 2 of the License, or
>> +# (at your option) any later version.
>> +#
>> +# This program is distributed in the hope that it will be useful,
>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> +# GNU General Public License for more details.
>> +#
>> +# You should have received a copy of the GNU General Public License
>> +# along with this program.  If not, see .
>> +#
>> +
>> +import os
>> +import socket
>> +import stat
>> +import time
>> +import iotests
>> +from iotests import cachemode, imgfmt, qemu_img, qemu_nbd
>> +
>> +NBD_PORT = 10811
>> +
>> +test_img = os.path.join(iotests.test_dir, 'test.img')
>> +
>> +class NBDBlockdevAddBase(iotests.QMPTestCase):
>> +def blockdev_add_options(self, address, export=None):
>> +options = { 'node-name': 'nbd-blockdev',
>> +'driver': 'raw',
>> +'file': {
>> +'driver': 'nbd',
>> +'address': address
>> +} }
>> +if export is not None:
>> +options['file']['export'] = export
>> +return options
>> +
>> +def client_test(self, filename, address, export=None):
>> +bao = self.blockdev_add_options(address, export)
>> +result = self.vm.qmp('blockdev-add', options=bao)
> 
> This needs a rebase (**bao instead of options=bao).
> 
>> +self.assert_qmp(result, 'return', {})
>> +
>> +result = self.vm.qmp('query-named-block-nodes')
>> +for node in result['return']:
>> +if node['node-name'] == 'nbd-blockdev':
>> +self.assert_qmp(node, 'image/filename', filename)
>> +break
>> +
>> +result = self.vm.qmp('x-blockdev-del', node_name='nbd-blockdev')
>> +self.assert_qmp(result, 'return', {})
>> +
>> +
>> +class QemuNBD(NBDBlockdevAddBase):
>> +def setUp(self):
>> +qemu_img('create', '-f', iotests.imgfmt, test_img, '64k')
>> +self.vm = iotests.VM()
>> +self.vm.launch()
>> +
>> +def tearDown(self):
>> +self.vm.shutdown()
>> +os.remove(test_img)
>> +
>> +def _server_up(self, *args):
>> +self.assertEqual(qemu_nbd('-f', imgfmt, test_img, *args), 0)
>> +
>> +def test_inet(self):
>> +self._server_up('-p', str(NBD_PORT))
>> +address = { 'type': 'inet',
>> +'data': {
>> +'host': 'localhost',
>> +'port': str(NBD_PORT)
>> +} }
>> +self.client_test('nbd://localhost:%i' % NBD_PORT, address)
>> +
>> +def test_unix(self):
>> +socket = os.path.join(iotests.test_dir, 'qemu-nbd.socket')
>> +self._server_up('-k', socket)
>> +address = { 'type': 'unix',
>> +'data': { 'path': socket } }
>> +self.client_test('nbd+unix://?socket=' + socket, address)
>> +try:
>> +os.remove(socket)
>> +except OSError:
>> +pass
> 
> If the test case fails, the socket file is leaked. We should probably
> either create and remove it in setUp()/tearDown(), or use a try/finally
> block around the test_unix code.

Yes, will do, thanks.

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v4 09/12] iotests.py: Add qemu_nbd function

[Max Reitz]

On 13.10.2016 15:11, Kevin Wolf wrote:
> Am 28.09.2016 um 22:55 hat Max Reitz geschrieben:
>> Signed-off-by: Max Reitz 
>> ---
>>  tests/qemu-iotests/iotests.py | 8 
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
>> index 3329bc1..5a2678f 100644
>> --- a/tests/qemu-iotests/iotests.py
>> +++ b/tests/qemu-iotests/iotests.py
>> @@ -39,6 +39,10 @@ qemu_io_args = [os.environ.get('QEMU_IO_PROG', 'qemu-io')]
>>  if os.environ.get('QEMU_IO_OPTIONS'):
>>  qemu_io_args += os.environ['QEMU_IO_OPTIONS'].strip().split(' ')
>>  
>> +qemu_nbd_args = [os.environ.get('QEMU_NBD_PROG', 'qemu-nbd')]
>> +if os.environ.get('QEMU_NBD_OPTIONS'):
>> +qemu_nbd_args += os.environ['QEMU_NBD_OPTIONS'].strip().split(' ')
>> +
>>  qemu_prog = os.environ.get('QEMU_PROG', 'qemu')
>>  qemu_opts = os.environ.get('QEMU_OPTIONS', '').strip().split(' ')
>>  
>> @@ -87,6 +91,10 @@ def qemu_io(*args):
>>  sys.stderr.write('qemu-io received signal %i: %s\n' % (-exitcode, ' 
>> '.join(args)))
>>  return subp.communicate()[0]
>>  
>> +def qemu_nbd(*args):
>> +'''Run qemu-nbd in daemon mode and return the parent's exit code'''
>> +return subprocess.call(qemu_nbd_args + ['--fork'] + list(args))
> 
> Wouldn't it be better to always use -t, track the PID and shut it down
> explicitly when the test exits?

Probably. It's a lot more complicated, though. I'll see what I can do
but I'm not sure if I can do a lot before 2.8.

Max

> The way you're using qemu-nbd here is fine if the test case passes, but
> if it fails before we access the NBD server, the server keeps running in
> the background.
> 
> Kevin



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v4 06/12] block/nbd: Accept SocketAddress

[Max Reitz]

On 13.10.2016 13:42, Kevin Wolf wrote:
> Am 28.09.2016 um 22:55 hat Max Reitz geschrieben:
>> Add a new option "address" to the NBD block driver which accepts a
>> SocketAddress.
>>
>> "path", "host" and "port" are still supported as legacy options and are
>> mapped to their corresponding SocketAddress representation.
>>
>> Signed-off-by: Max Reitz 
> 
> Not opposed in principle to your change, but we should try to keep the
> naming consistent between NBD and the other block drivers, notably the
> SSH work that is currently going on.
> 
> This patch uses 'address' for the SockAddress, the proposed SSH patch
> uses 'server'. I don't mind too much which one we choose, though I like
> 'server' a bit better. Anyway, we should choose one and stick to it in
> all drivers.

OK, I'll use "server" then. I don't mind either way, so even a weak
opinion is enough to persuade me. ;-)

>>  block/nbd.c   | 166 
>> ++
>>  tests/qemu-iotests/051.out|   4 +-
>>  tests/qemu-iotests/051.pc.out |   4 +-
>>  3 files changed, 106 insertions(+), 68 deletions(-)
>>
>> diff --git a/block/nbd.c b/block/nbd.c
>> index cdab20f..449f94e 100644
>> --- a/block/nbd.c
>> +++ b/block/nbd.c
>> @@ -32,6 +32,9 @@
>>  #include "qemu/uri.h"
>>  #include "block/block_int.h"
>>  #include "qemu/module.h"
>> +#include "qapi-visit.h"
>> +#include "qapi/qmp-input-visitor.h"
>> +#include "qapi/qmp-output-visitor.h"
>>  #include "qapi/qmp/qdict.h"
>>  #include "qapi/qmp/qjson.h"
>>  #include "qapi/qmp/qint.h"
>> @@ -44,7 +47,8 @@ typedef struct BDRVNBDState {
>>  NbdClientSession client;
>>  
>>  /* For nbd_refresh_filename() */
>> -char *path, *host, *port, *export, *tlscredsid;
>> +SocketAddress *saddr;
>> +char *export, *tlscredsid;
>>  } BDRVNBDState;
>>  
>>  static int nbd_parse_uri(const char *filename, QDict *options)
>> @@ -131,7 +135,9 @@ static bool nbd_has_filename_options_conflict(QDict 
>> *options, Error **errp)
>>  if (!strcmp(e->key, "host") ||
>>  !strcmp(e->key, "port") ||
>>  !strcmp(e->key, "path") ||
>> -!strcmp(e->key, "export"))
>> +!strcmp(e->key, "export") ||
>> +!strcmp(e->key, "address") ||
>> +!strncmp(e->key, "address.", 8))
> 
> strstart()?

Yep, will use.

>>  {
>>  error_setg(errp, "Option '%s' cannot be used with a file name",
>> e->key);
>> @@ -205,50 +211,67 @@ out:
>>  g_free(file);
>>  }
>>  
>> -static SocketAddress *nbd_config(BDRVNBDState *s, QemuOpts *opts, Error 
>> **errp)
>> +static bool nbd_process_legacy_socket_options(QDict *output_options,
>> +  QemuOpts *legacy_opts,
>> +  Error **errp)
>>  {
>> -SocketAddress *saddr;
>> -
>> -s->path = g_strdup(qemu_opt_get(opts, "path"));
>> -s->host = g_strdup(qemu_opt_get(opts, "host"));
>> -s->port = g_strdup(qemu_opt_get(opts, "port"));
>> -
>> -if (!s->path == !s->host) {
>> -if (s->path) {
>> -error_setg(errp, "path and host may not be used at the same 
>> time");
>> -} else {
>> -error_setg(errp, "one of path and host must be specified");
>> +const char *path = qemu_opt_get(legacy_opts, "path");
>> +const char *host = qemu_opt_get(legacy_opts, "host");
>> +const char *port = qemu_opt_get(legacy_opts, "port");
>> +
>> +if (path && host) {
>> +error_setg(errp, "path and host may not be used at the same time");
>> +return false;
>> +} else if (path) {
>> +if (port) {
>> +error_setg(errp, "port may not be used without host");
>> +return false;
>>  }
>> -return NULL;
>> +
>> +qdict_put(output_options, "address.type", qstring_from_str("unix"));
>> +qdict_put(output_options, "address.data.path", 
>> qstring_from_str(path));
>> +} else if (host) {
>> +qdict_put(output_options, "address.type", qstring_from_str("inet"));
>> +qdict_put(output_options, "address.data.host", 
>> qstring_from_str(host));
>> +qdict_put(output_options, "address.data.port",
>> +  qstring_from_str(port ?: stringify(NBD_DEFAULT_PORT)));
>>  }
>> -if (s->port && !s->host) {
>> -error_setg(errp, "port may not be used without host");
>> -return NULL;
>> +
>> +return true;
>> +}
> 
> If both the legacy option and the new one are given, the legacy one
> takes precedence. Intentional?

No. I think it'll be easiest to just return an error when a user is
trying to use both.

Thanks,

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v3] block: Remove "options" indirection from blockdev-add

[Max Reitz]

On 11.10.2016 15:27, Kevin Wolf wrote:
> Now that QAPI supports boxed types, we can have unions at the top level
> of a command, so let's put our real options directly there for
> blockdev-add instead of having a single "options" dict that contains the
> real arguments.
> 
> blockdev-add is still experimental and we already made substantial
> changes to the API recently, so we're free to make changes like this
> one, too.
> 
> Signed-off-by: Kevin Wolf 
> Reviewed-by: Eric Blake 
> Reviewed-by: Markus Armbruster 
> ---
>  docs/qmp-commands.txt  |  84 ---
>  qapi/block-core.json   |   4 +-
>  tests/qemu-iotests/041 |  11 +++--
>  tests/qemu-iotests/067 |  12 +++--
>  tests/qemu-iotests/071 | 118 
> +
>  tests/qemu-iotests/081 |  52 ++
>  tests/qemu-iotests/085 |   9 ++--
>  tests/qemu-iotests/087 |  76 +--
>  tests/qemu-iotests/117 |  12 ++---
>  tests/qemu-iotests/118 |  42 +-
>  tests/qemu-iotests/124 |  20 -
>  tests/qemu-iotests/139 |  10 ++---
>  tests/qemu-iotests/141 |  13 +++---
>  tests/qemu-iotests/155 |  10 ++---
>  14 files changed, 214 insertions(+), 259 deletions(-)

Reviewed-by: Max Reitz 



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 07/22] qcow2-bitmap: introduce auto-loading bitmaps

[Max Reitz]

On 14.10.2016 20:44, Vladimir Sementsov-Ogievskiy wrote:
> On 01.10.2016 19:26, Max Reitz wrote:
>> On 30.09.2016 12:53, Vladimir Sementsov-Ogievskiy wrote:
>>> Auto loading bitmaps are bitmaps in Qcow2, with AUTO flag set. They are
> 
> ...
> 
>> +goto fail;
>> +}
>> +
>> +/* loop is safe because next entry offset is calculated after
>> conversion to
>> Should it be s/safe/unsafe/?
> 
> loop is safe. _unsafe is related to absence of assert in a loop, as it
> loops through BE data. Bad wording, I'll change it somehow..

Yes, I know that the loop is safe in the sense of the word "safe", but I
meant that it's kind of confusing that the loop uses
"for_each_bitmap_dir_entry_unsafe" and thus is "unsafe", too.

Another idea would be to write "This is actually safe" instead of "loop
is safe".

>>> + * cpu format */
>>> +for_each_bitmap_dir_entry_unsafe(e, dir, size) {
>>> +if ((uint8_t *)(e + 1) > dir_end) {
>>> +goto broken_dir;
>>> +}
>>> +
>>> +bitmap_dir_entry_to_cpu(e);
>>> +
>>> +if ((uint8_t *)next_dir_entry(e) > dir_end) {
>>> +goto broken_dir;
>>> +}
>>> +
>>> +if (e->extra_data_size != 0) {
>>> +error_setg(errp, "Can't load bitmap '%.*s' from '%s':"
>>> +   "extra data not supported.", e->name_size,
>> Full stop again.
>>
>> Also, I'm not quite sure why you give the device/node name here. You
>> don't do that anywhere else and I think if we want to emit the
>> information where something failed, it should be added at some previous
>> point in the call chain.
> 
> For example, how? As I understand, I can emit it only by error_setg, so
> actually it would be better to add node and bitmap name to all
> error_setg in the code.. or create helper function for it.

error_prepend() would be the function.

This code will be invoked by any code that is opening an image. There
are of course a couple of places where that can be the case: For
external tools such as qemu-img, it's normally pretty clear which image
is meant (and it additionally uses error_reportf_err() to give you more
information).

For -drive, every error message will at least be prepended by the
corresponding -drive parameter, so that will help a bit.

blockdev-add, unfortunately, doesn't do anything like this. But the user
can of course choose to construct the BDS graph node by node and thus
always know which node an error originates from.

Anyway, if you want to add this information to every error message, you
should probably do so in bdrv_open_inherit(). The issue I'd take with
this is that most users probably won't set the node name themselves
(either they don't at all or it's some management tool that does), so
reporting the node name doesn't actually help them at all (and
management tools are not supposed to parse error messages, so it won't
help in that case either).

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 1/1] slirp: VMState conversion; tcpcb

[Samuel Thibault]

Dr. David Alan Gilbert (git), on Thu 13 Oct 2016 18:37:46 +0100, wrote:
> Convert the migration of the struct tcpcb to use a VMStateDescription,
> the rest of it will come later.
> 
> Mostly mechanical, except for conversion of some 'char' to uint8_t
> to ensure portability.
> 
> Tested with
>-netdev user,id=unet,guestfwd=tcp:10.0.2.50:-tcp:localhost:
> 
>   with a nc hung off port  and having the guest spew 'seq'
> to the port.
> 
> Signed-off-by: Dr. David Alan Gilbert 

Reviewed-by: Samuel Thibault 

Thanks,
Samuel

Re: [Qemu-devel] [PATCH 11/22] qcow2-bitmap: add qcow2_store_persistent_bitmaps()

[Max Reitz]

On 13.10.2016 18:48, Vladimir Sementsov-Ogievskiy wrote:
> On 07.10.2016 22:24, Max Reitz wrote:
>> On 30.09.2016 12:53, Vladimir Sementsov-Ogievskiy wrote:
>>> Realize block bitmap stroing interface, to allow qcow2 images store
> 
> [snip]
> 
>>> +uint64_t end = MIN(bm_size, sector + dsc);
>>> +uint64_t write_size =
>>> +bdrv_dirty_bitmap_serialization_size(bitmap, sector, end
>>> - sector);
>>> +
>>> +int64_t off = qcow2_alloc_clusters(bs, cl_size);
>>> +if (off < 0) {
>>> +ret = off;
>>> +goto finish;
>>> +}
>>> +bitmap_table[cluster] = off;
>>> +
>>> +bdrv_dirty_bitmap_serialize_part(bitmap, buf, sector, end);
>> s/end/end - sector/?
> 
> o_0 terrible mistake, thank you.
> 
>>
>>> +if (write_size < cl_size) {
>>> +memset(buf + write_size, 0, cl_size - write_size);
>>> +}
>>> +
>> I guess there should be a metadata overlap check here.
> 
> What is the general rule of checking it? Should I check it before all my
> extension related writes?

The general rule is supposed to be "One check before every write to
bs->file".

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH] linux-user: Fix do_store_exclusive for shared memory of interprocess.

[no-reply]

Hi,

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

Subject: [Qemu-devel] [PATCH] linux-user: Fix do_store_exclusive for shared 
memory of interprocess.
Type: series
Message-id: 20161015155348.26834-...@hev.cc

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

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

# Useful git options
git config --local diff.renamelimit 0
git config --local diff.renames True

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

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

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
03ca3dc linux-user: Fix do_store_exclusive for shared memory of interprocess.

=== OUTPUT BEGIN ===
Checking PATCH 1/1: linux-user: Fix do_store_exclusive for shared memory of 
interprocess
ERROR: code indent should never use tabs
#20: FILE: linux-user/main.c:2315:
+#define cmpxchg_user(old, new, gaddr, target_type)^I^I^I\$

ERROR: code indent should never use tabs
#21: FILE: linux-user/main.c:2316:
+({^I^I^I^I^I^I^I^I^I\$

ERROR: code indent should never use tabs
#22: FILE: linux-user/main.c:2317:
+abi_ulong __gaddr = (gaddr);^I^I^I^I^I\$

ERROR: code indent should never use tabs
#23: FILE: linux-user/main.c:2318:
+target_type *__hptr;^I^I^I^I^I^I\$

ERROR: code indent should never use tabs
#24: FILE: linux-user/main.c:2319:
+abi_long __ret = 0;^I^I^I^I^I^I^I\$

ERROR: do not use assignment in if condition
#25: FILE: linux-user/main.c:2320:
+if ((__hptr = lock_user(VERIFY_WRITE, __gaddr, sizeof(target_type), 0))) { 
\

ERROR: braces {} are necessary for all arms of this statement
#25: FILE: linux-user/main.c:2320:
+if ((__hptr = lock_user(VERIFY_WRITE, __gaddr, sizeof(target_type), 0))) { 
\
[...]
+} else \
[...]

ERROR: code indent should never use tabs
#26: FILE: linux-user/main.c:2321:
+if ((old) != atomic_cmpxchg(__hptr, (old), (new)))^I^I\$

ERROR: braces {} are necessary for all arms of this statement
#26: FILE: linux-user/main.c:2321:
+if ((old) != atomic_cmpxchg(__hptr, (old), (new))) \
[...]

ERROR: code indent should never use tabs
#27: FILE: linux-user/main.c:2322:
+__ret = -TARGET_EAGAIN;^I^I^I^I^I\$

ERROR: code indent should never use tabs
#28: FILE: linux-user/main.c:2323:
+unlock_user(__hptr, __gaddr, sizeof(target_type));^I^I\$

ERROR: code indent should never use tabs
#29: FILE: linux-user/main.c:2324:
+} else^I^I^I^I^I^I^I^I\$

ERROR: code indent should never use tabs
#30: FILE: linux-user/main.c:2325:
+__ret = -TARGET_EFAULT;^I^I^I^I^I^I\$

ERROR: code indent should never use tabs
#31: FILE: linux-user/main.c:2326:
+__ret;^I^I^I^I^I^I^I^I\$

WARNING: line over 80 characters
#34: FILE: linux-user/main.c:2329:
+#define cmpxchg_user_u32(old, new, gaddr) cmpxchg_user((old), (new), (gaddr), 
uint32_t)

WARNING: line over 80 characters
#35: FILE: linux-user/main.c:2330:
+#define cmpxchg_user_u64(old, new, gaddr) cmpxchg_user((old), (new), (gaddr), 
uint64_t)

total: 14 errors, 2 warnings, 40 lines checked

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

=== OUTPUT END ===

Test command exited with code: 1


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

[Qemu-devel] [PATCH] linux-user: Fix do_store_exclusive for shared memory of interprocess.

[Heiher]

From: Heiher 

test case: http://pastebin.com/raw/x2GW4xNW

Signed-off-by: Heiher 
---
 linux-user/main.c | 24 ++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/linux-user/main.c b/linux-user/main.c
index 0e31dad..81b0a49 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -2312,6 +2312,23 @@ static const uint8_t mips_syscall_args[] = {
 #  undef MIPS_SYS
 # endif /* O32 */
 
+#define cmpxchg_user(old, new, gaddr, target_type) \
+({ \
+abi_ulong __gaddr = (gaddr);   \
+target_type *__hptr;   \
+abi_long __ret = 0;
\
+if ((__hptr = lock_user(VERIFY_WRITE, __gaddr, sizeof(target_type), 0))) { 
\
+if ((old) != atomic_cmpxchg(__hptr, (old), (new))) \
+__ret = -TARGET_EAGAIN;\
+unlock_user(__hptr, __gaddr, sizeof(target_type)); \
+} else \
+__ret = -TARGET_EFAULT;
\
+__ret; \
+})
+
+#define cmpxchg_user_u32(old, new, gaddr) cmpxchg_user((old), (new), (gaddr), 
uint32_t)
+#define cmpxchg_user_u64(old, new, gaddr) cmpxchg_user((old), (new), (gaddr), 
uint64_t)
+
 static int do_store_exclusive(CPUMIPSState *env)
 {
 target_ulong addr;
@@ -2342,12 +2359,15 @@ static int do_store_exclusive(CPUMIPSState *env)
 env->active_tc.gpr[reg] = 0;
 } else {
 if (d) {
-segv = put_user_u64(env->llnewval, addr);
+segv = cmpxchg_user_u64(env->llval, env->llnewval, addr);
 } else {
-segv = put_user_u32(env->llnewval, addr);
+segv = cmpxchg_user_u32(env->llval, env->llnewval, addr);
 }
 if (!segv) {
 env->active_tc.gpr[reg] = 1;
+} else if (-TARGET_EAGAIN == segv) {
+segv = 0;
+env->active_tc.gpr[reg] = 0;
 }
 }
 }
-- 
2.10.0

Re: [Qemu-devel] [PULL 4/4] tests/docker/Makefile.include: add a generic docker-run target

[Alex Bennée]

Paolo Bonzini  writes:

> On 14/10/2016 17:29, Fam Zheng wrote:
>> From: Alex Bennée 
>>
>> This re-factors the docker makefile to include a docker-run target which
>> can be controlled entirely from environment variables specified on the
>> make command line. This allows us to run against any given docker image
>> we may have in our repository, for example:
>>
>> make docker-run TEST="test-quick" IMAGE="debian:arm64" \
>>  EXECUTABLE=./aarch64-linux-user/qemu-aarch64
>>
>> The existing docker-foo@bar targets still work but the inline
>> verification has been dropped because we already don't hit that due to
>> other pattern rules in rules.mak.
>>
>> Signed-off-by: Alex Bennée 
>>
>> Message-Id: <20161011161625.9070-5-alex.ben...@linaro.org>
>> Message-Id: <20161011161625.9070-6-alex.ben...@linaro.org>
>> [Squash in the verification removal patch. - Fam]
>> Signed-off-by: Fam Zheng 
>> ---
>>  tests/docker/Makefile.include | 61 
>> +++
>>  1 file changed, 38 insertions(+), 23 deletions(-)
>>
>> diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
>> index b44daab..925f711 100644
>> --- a/tests/docker/Makefile.include
>> +++ b/tests/docker/Makefile.include
>> @@ -78,6 +78,7 @@ docker:
>>  @echo ' "IMAGE" is one of the listed container 
>> name."'
>>  @echo 'docker-image:Build all images.'
>>  @echo 'docker-image-IMAGE:  Build image "IMAGE".'
>> +@echo 'docker-run:  For manually running a "TEST" with 
>> "IMAGE"'
>>  @echo
>>  @echo 'Available container images:'
>>  @echo '$(DOCKER_IMAGES)'
>> @@ -101,31 +102,45 @@ docker:
>>  @echo 'NOCACHE=1Ignore cache when build images.'
>>  @echo 'EXECUTABLE=Include executable in image.'
>>
>> +# This rule if for directly running against an arbitrary docker target.
>> +# It is called by the expanded docker targets (e.g. make
>> +# docker-test-foo@bar) which will do additional verification.
>> +#
>> +# For example: make docker-run TEST="test-quick" IMAGE="debian:arm64" 
>> EXECUTABLE=./aarch64-linux-user/qemu-aarch64
>> +#
>> +docker-run: docker-qemu-src
>> +@mkdir -p "$(DOCKER_CCACHE_DIR)"
>> +@if test -z "$(IMAGE)" || test -z "$(TEST)"; \
>> +then echo "Invalid target $(IMAGE)/$(TEST)"; exit 1; \
>> +fi
>> +$(if $(EXECUTABLE), \
>> +$(call quiet-command,   \
>> +$(SRC_PATH)/tests/docker/docker.py update   \
>> +$(IMAGE) $(EXECUTABLE), \
>> +"  COPYING $(EXECUTABLE) to $(IMAGE)"))
>> +$(call quiet-command,   \
>> +$(SRC_PATH)/tests/docker/docker.py run  \
>> +-t  \
>> +$(if $V,,--rm)  \
>> +$(if $(DEBUG),-i,--net=none)\
>> +-e TARGET_LIST=$(TARGET_LIST)   \
>> +-e EXTRA_CONFIGURE_OPTS="$(EXTRA_CONFIGURE_OPTS)" \
>> +-e V=$V -e J=$J -e DEBUG=$(DEBUG)   \
>> +-e SHOW_ENV=$(SHOW_ENV) \
>> +-e CCACHE_DIR=/var/tmp/ccache   \
>> +-v $$(readlink -e 
>> $(DOCKER_SRC_COPY)):/var/tmp/qemu:z$(COMMA)ro \
>> +-v $(DOCKER_CCACHE_DIR):/var/tmp/ccache:z   \
>> +$(IMAGE)\
>> +/var/tmp/qemu/run   \
>> +$(TEST), "  RUN $(TEST) in ${IMAGE}")
>> +
>> +# Run targets:
>> +#
>> +# Of the form docker-TEST-FOO@IMAGE-BAR which will then be expanded into a 
>> call to "make docker-run"
>>  docker-run-%: CMD = $(shell echo '$@' | sed -e 
>> 's/docker-run-\([^@]*\)@\(.*\)/\1/')
>>  docker-run-%: IMAGE = $(shell echo '$@' | sed -e 
>> 's/docker-run-\([^@]*\)@\(.*\)/\2/')
>> -docker-run-%: docker-qemu-src
>> -@mkdir -p "$(DOCKER_CCACHE_DIR)"
>> -@if test -z "$(IMAGE)" || test -z "$(CMD)"; \
>> -then echo "Invalid target"; exit 1; \
>> -fi
>> -$(if $(filter $(TESTS),$(CMD)),$(if $(filter $(IMAGES),$(IMAGE)), \
>> -$(call quiet-command,\
>> -if $(SRC_PATH)/tests/docker/docker.py images | \
>> -awk '$$1=="qemu" && $$2=="$(IMAGE)"{found=1} 
>> END{exit(!found)}'; then \
>> -$(SRC_PATH)/tests/docker/docker.py run $(if 
>> $V,,--rm) \
>> --t \
>> -$(if $(DEBUG),-i,--net=none) \
>> -

Re: [Qemu-devel] [PULL 4/4] tests/docker/Makefile.include: add a generic docker-run target

[Paolo Bonzini]

On 14/10/2016 17:29, Fam Zheng wrote:
> From: Alex Bennée 
> 
> This re-factors the docker makefile to include a docker-run target which
> can be controlled entirely from environment variables specified on the
> make command line. This allows us to run against any given docker image
> we may have in our repository, for example:
> 
> make docker-run TEST="test-quick" IMAGE="debian:arm64" \
>  EXECUTABLE=./aarch64-linux-user/qemu-aarch64
> 
> The existing docker-foo@bar targets still work but the inline
> verification has been dropped because we already don't hit that due to
> other pattern rules in rules.mak.
> 
> Signed-off-by: Alex Bennée 
> 
> Message-Id: <20161011161625.9070-5-alex.ben...@linaro.org>
> Message-Id: <20161011161625.9070-6-alex.ben...@linaro.org>
> [Squash in the verification removal patch. - Fam]
> Signed-off-by: Fam Zheng 
> ---
>  tests/docker/Makefile.include | 61 
> +++
>  1 file changed, 38 insertions(+), 23 deletions(-)
> 
> diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
> index b44daab..925f711 100644
> --- a/tests/docker/Makefile.include
> +++ b/tests/docker/Makefile.include
> @@ -78,6 +78,7 @@ docker:
>   @echo ' "IMAGE" is one of the listed container 
> name."'
>   @echo 'docker-image:Build all images.'
>   @echo 'docker-image-IMAGE:  Build image "IMAGE".'
> + @echo 'docker-run:  For manually running a "TEST" with 
> "IMAGE"'
>   @echo
>   @echo 'Available container images:'
>   @echo '$(DOCKER_IMAGES)'
> @@ -101,31 +102,45 @@ docker:
>   @echo 'NOCACHE=1Ignore cache when build images.'
>   @echo 'EXECUTABLE=Include executable in image.'
>  
> +# This rule if for directly running against an arbitrary docker target.
> +# It is called by the expanded docker targets (e.g. make
> +# docker-test-foo@bar) which will do additional verification.
> +#
> +# For example: make docker-run TEST="test-quick" IMAGE="debian:arm64" 
> EXECUTABLE=./aarch64-linux-user/qemu-aarch64
> +#
> +docker-run: docker-qemu-src
> + @mkdir -p "$(DOCKER_CCACHE_DIR)"
> + @if test -z "$(IMAGE)" || test -z "$(TEST)"; \
> + then echo "Invalid target $(IMAGE)/$(TEST)"; exit 1; \
> + fi
> + $(if $(EXECUTABLE), \
> + $(call quiet-command,   \
> + $(SRC_PATH)/tests/docker/docker.py update   \
> + $(IMAGE) $(EXECUTABLE), \
> + "  COPYING $(EXECUTABLE) to $(IMAGE)"))
> + $(call quiet-command,   \
> + $(SRC_PATH)/tests/docker/docker.py run  \
> + -t  \
> + $(if $V,,--rm)  \
> + $(if $(DEBUG),-i,--net=none)\
> + -e TARGET_LIST=$(TARGET_LIST)   \
> + -e EXTRA_CONFIGURE_OPTS="$(EXTRA_CONFIGURE_OPTS)" \
> + -e V=$V -e J=$J -e DEBUG=$(DEBUG)   \
> + -e SHOW_ENV=$(SHOW_ENV) \
> + -e CCACHE_DIR=/var/tmp/ccache   \
> + -v $$(readlink -e 
> $(DOCKER_SRC_COPY)):/var/tmp/qemu:z$(COMMA)ro \
> + -v $(DOCKER_CCACHE_DIR):/var/tmp/ccache:z   \
> + $(IMAGE)\
> + /var/tmp/qemu/run   \
> + $(TEST), "  RUN $(TEST) in ${IMAGE}")
> +
> +# Run targets:
> +#
> +# Of the form docker-TEST-FOO@IMAGE-BAR which will then be expanded into a 
> call to "make docker-run"
>  docker-run-%: CMD = $(shell echo '$@' | sed -e 
> 's/docker-run-\([^@]*\)@\(.*\)/\1/')
>  docker-run-%: IMAGE = $(shell echo '$@' | sed -e 
> 's/docker-run-\([^@]*\)@\(.*\)/\2/')
> -docker-run-%: docker-qemu-src
> - @mkdir -p "$(DOCKER_CCACHE_DIR)"
> - @if test -z "$(IMAGE)" || test -z "$(CMD)"; \
> - then echo "Invalid target"; exit 1; \
> - fi
> - $(if $(filter $(TESTS),$(CMD)),$(if $(filter $(IMAGES),$(IMAGE)), \
> - $(call quiet-command,\
> - if $(SRC_PATH)/tests/docker/docker.py images | \
> - awk '$$1=="qemu" && $$2=="$(IMAGE)"{found=1} 
> END{exit(!found)}'; then \
> - $(SRC_PATH)/tests/docker/docker.py run $(if 
> $V,,--rm) \
> - -t \
> - $(if $(DEBUG),-i,--net=none) \
> - -e TARGET_LIST=$(TARGET_LIST) \
> - -e 

Re: [Qemu-devel] [V2,1/7] nios2: Add disas entries

[Romain Naour]

Hi Marek,

Le 28/09/2016 à 01:30, Marek Vasut a écrit :
> Add nios2 disassembler support. This patch is composed from binutils files
> from commit "Opcodes and assembler support for Nios II R2". The files from
> binutils used in this patch are:
> 
> include/opcode/nios2.h
> include/opcode/nios2r1.h
> include/opcode/nios2r2.h
> opcodes/nios2-opc.c
> opcodes/nios2-dis.c

With Waldemar Brodkorb and I, we tested this series using 10m50 kernel defconfig
with Buildroot generated system. In order to ease the test, we added the device
tree and a initramfs to the kernel image.

Here is the result:

Welcome to Buildroot
# cat /proc/cpuinfo
CPU:Nios II/fast
MMU:present
FPU:none
Clocking:   75.00 MHz
BogoMips:   150.00
Calibration:7500 loops
HW:
 MUL:   yes
 MULX:  no
 DIV:   yes
Icache: 32kB, line length: 32
Dcache: 32kB, line length: 32
TLB:16 ways, 256 entries, 8 PID bits
# uname -a
Linux buildroot 4.8.1 #2 Fri Oct 14 19:10:18 CEST 2016 nios2 GNU/Linux

When this series will be accepted in Qemu, I'll add a demo defconfig in
Buildroot in order to ease runtime testing.

Tested-by: Romain Naour 

Best regards,
Romain

> Signed-off-by: Marek Vasut 
> Cc: Chris Wulff 
> Cc: Jeff Da Silva 
> Cc: Ley Foon Tan 
> Cc: Sandra Loosemore 
> Cc: Yves Vandervennet 
> ---
> V2: Replace the nios2.c with GPL2 licensed version
> ---

Re: [Qemu-devel] [PATCH v2] MAINTAINERS: qemu-trivial information

[Michael Tokarev]

08.10.2016 13:00, Laurent Vivier wrote:

Information about "qemu-trivial" ML can be found in the wiki:

http://wiki.qemu.org/Contribute/TrivialPatches

But the first place where a developer looks is the file MAINTAINERS.

This also allows the get_maintainer.pl script to display
the qemu-trivial ML address when the mail subject contains "trivial".


Applied to -trivial, thank you!

/mjt

Re: [Qemu-devel] [PATCH] colo-compare: remove unused struct CompareChardevProps and 'props' variable

[Michael Tokarev]

13.10.2016 09:57, zhanghailiang wrote:

After commit 0a73336d, 'props' variable in find_and_check_chardev()
is unused. Remove it, togther with struct CompareChardevProps.


Applied to -trivial, thanks!

/mjt

Re: [Qemu-devel] [PATCH] milkymist-pfpu: fix potential integer overflow

[Michael Tokarev]

14.10.2016 12:51, Michael Walle wrote:

Since the lm32 is a 32 bit architecture, just return a 32 bit value which
is then converted to a 64 bit value.

Spotted by coverity, CID 1005506.


Applied to -trivial, thank you!

/mjt

Re: [Qemu-devel] [Qemu-ppc] [QEMU PATCH v6 2/2] migration: migrate QTAILQ

[Halil Pasic]

On 10/14/2016 07:18 PM, Jianjun Duan wrote:
> +/*
>  + * Offsets of layout of a tail queue head.
>  + */
>  +#define QTAILQ_FIRST_OFFSET 0
>  +#define QTAILQ_LAST_OFFSET (sizeof(void *))
>  +
>  +/*
>  + * Offsets of layout of a tail queue element.
>  + */
>  +#define QTAILQ_NEXT_OFFSET 0
>  +#define QTAILQ_PREV_OFFSET (sizeof(void *))
>  +
>  +/*
>  + * Tail queue tranversal using pointer arithmetic.
>  + */
>  +#define QTAILQ_RAW_FOREACH(elm, head, entry)
> \
>  +for ((elm) = *((void **) ((char *) (head) + 
>  QTAILQ_FIRST_OFFSET)); \
>  + (elm); 
> \
>  + (elm) =
> \
>  + *((void **) ((char *) (elm) + (entry) + 
>  QTAILQ_NEXT_OFFSET)))
>  +/*
>  + * Tail queue insertion using pointer arithmetic.
>  + */
>  +#define QTAILQ_RAW_INSERT_TAIL(head, elm, entry) do {   
> \
>  +*((void **) ((char *) (elm) + (entry) + 
>  QTAILQ_NEXT_OFFSET)) = NULL;   \
>  +*((void **) ((char *) (elm) + (entry) + 
>  QTAILQ_PREV_OFFSET)) = \
>  +*((void **) ((char *) (head) + QTAILQ_LAST_OFFSET));
> \
>  +**((void ***)((char *) (head) + QTAILQ_LAST_OFFSET)) = 
>  (elm);  \
>  +*((void **) ((char *) (head) + QTAILQ_LAST_OFFSET)) =   
> \
>  +(void *) ((char *) (elm) + (entry) + 
>  QTAILQ_NEXT_OFFSET);  \
>  +} while (/*CONSTCOND*/0)
 >>>
 >>> I wonder if there's a simpler way to do this; I'm not sure this works, 
 >>> but something like:
 >>>
 >>> struct QTAILQDummy {
 >>> char dummy;
 >>> };
 >>>
 >>> QTAILQ_HEAD(QTAILQRawHead, struct QTAILQDummy)
 >>> typedef QTAILQ_ENTRY(struct QTAILQDummy) QTAILQRawEntry;
 >>>
 >>> #define QTAILQ_RAW_FOREACH(elm, head, entry)   
 >>> \
 >>> for ((elm) = ((struct QTAILQRawHead *)head)->tqh_first)
 >>> \
 >>>  (elm);
 >>> \
 >>>  (elm) =   
 >>> \
 >>>  (elm) = ((QTAILQRawEntry *)((char *) (elm) + 
 >>> (entry)))->tqh_next
 >>>
 >>> and then I think elm gets declared as a struct QTAILQDummy.
 >>> But it does avoid those 
 >>> FIRST_OFFSET/LAST_OFFSET/NEXT_OFFSET/PREV_OFFSET calculations.
 >>>
 >>> Would that work?
 >>>
>>> >> It is intended for QTAILQ of any type. So type is not available.
>> > 
>> > I think it might be possible to do it generally.
>> > 
> If we have type, then we can use what is there already, and don't need a
> pointer arithmetic based approach. Inside put/get, we only get type
> layout info from vmsd, which is all about size and offset. This macro
> is used inside put/get, so I am not sure how we can directly use type
> here.
> 

Dave's approach seems perfectly sane to me. 

Jianjun have you actually tried to make it work before writing this?
Your argument does not work, because what you need from vmsd for
QTAILQ_RAW_FOREACH is only .start which corresponds to the entry
parameter of the macro. Dave still does the pointer arithmetic to
get a  pointer (char*) to the anonymous struct holding tqe_next
and tqe_prev. Now since no arithmetic is done wit tqe_next
and tqe_prev, only dereferencing, their pointer type does not matter
all that much so we can do the and follow the pointer. Same goes
for the head.

Actually the QTAILQDummy is not necessary in my opinion since we can
probably (did not try it out myself) do:

Q_TAILQ_HEAD(QTAILQRawHead, void,)
typedef Q_TAILQ_ENTRY(void,) QTAILQRawEntry;

Cheers,
Halil

Re: [Qemu-devel] [PATCH] hw/block/nvme: Simplify if-statements a little bit

[Michael Tokarev]

12.10.2016 18:18, Thomas Huth wrote:

The condition  '!A || (A && B)' is equivalent to '!A || B'.


Applied to -trivial, thanks!

/mjt

Re: [Qemu-devel] [PATCH] char.h: misc doc fix

[Michael Tokarev]

Applied to -trivial, thanks!

/mjt

Re: [Qemu-devel] [PATCH 1/4] target-lm32: swap operand of wcsr in LOG_DIS()

[Michael Tokarev]

Applied all 4 to -trivial. With some digging around :)

Thanks,

/mjt

Re: [Qemu-devel] [PATCH] target-lm32: fix LOG_DIS operand order

[Michael Tokarev]

12.10.2016 20:15, Michael Walle wrote:

The order of most opcodes with immediates was wrong (according to the
reference manual) in the (debug) logging. Additionally, one operand for the
andhi instruction was completly wrong. Fix these.


Applied to -trivial, thank you!

/mjt

Re: [Qemu-devel] [PATCH] target-lm32: fix style issue

[Michael Tokarev]

12.10.2016 19:23, Michael Walle wrote:

Both branches of the ternary operator have the same expressions. Drop the
operator.


Applied to -trivial, thank you!

/mjt

Re: [Qemu-devel] [PATCH] hw/tpm/tpm_passthrough: Simplify if-statements a little bit

[Michael Tokarev]

Applied to -trivial, thank you!

/mjt

Re: [Qemu-devel] [PATCH] filter-dump: add missing "["

[Michael Tokarev]

Applied to -trivial, thanks!

/mjt

Re: [Qemu-devel] [PATCH] trace: fix group name generation

[Peter Maydell]

On 14 October 2016 at 23:06, Greg Kurz  wrote:
> On Fri, 14 Oct 2016 16:31:01 -0500
> Eric Blake  wrote:
>
>> On 10/14/2016 04:26 PM, Greg Kurz wrote:
>> > Since commit "80dd5c4918ab trace: introduce a formal group name for trace
>> > events", tracetool generates C variable names and macro definitions out
>> > of the path to the trace-events-all file.

>> > diff --git a/scripts/tracetool.py b/scripts/tracetool.py
>> > index 629b2593c846..b81b834db924 100755
>> > --- a/scripts/tracetool.py
>> > +++ b/scripts/tracetool.py
>> > @@ -70,7 +70,7 @@ def make_group_name(filename):
>> >
>> >  if dirname == "":
>> >  return "common"
>> > -return re.sub(r"/|-", "_", dirname)
>> > +return "_" + re.sub(r"[^\w]", "_", dirname)
>>
>> This STILL doesn't solve the complaint that the build is now dependent
>> on the location.  Why can't we STRIP off any prefix prior to the in-tree
>> portion of the naming that we know is sane, instead of munging the
>> prefix but in the process creating source code that generates with
>> different lengths?
>>
>
> Heh, because the complaint was about the build break :)
>
>> Ideally, compiling twice, once in directory 'a', and the second time in
>> directory '', should not make a noticeable
>> difference in the final size of the executable due to the difference in
>> lengths of the debugging symbols used to record the longer name of the
>> second directory being encoded into lots of macro names.
>>
>
> You're right. I'm okay to look for a way to fix the symbol variable size
> issue, but I think this patch still makes sense as it makes tracetool
> less fragile in case we add a directory with an illegal character to
> the QEMU tree... and it fixes annoying build breaks (Paolo also hit
> this and asked to revert 80dd5c4918ab in another mail).

I agree with Eric that we should just not be putting
the build directory name in these variables -- this
looks like it's simply a bug to me, and we should fix it.

I think the chances of us adding a directory to the QEMU
tree itself with a silly name are quite low (not least because
if you do that then you get a handy build failure to tell
you not to do that ;-))

thanks
-- PMM

Re: [Qemu-devel] how to get the default cpu for a softmmu

[Dennis Luehring]

Am 15.10.2016 um 12:29 schrieb Alex Bennée:

>i can get a list of all supported CPUs by using the -cpu ? commandline
>
>but which CPU is selected if i don't give the -cpu parameter on normal
>qemu start of a softmmu?

IIRC it is dependant on the machine model you select


and which cpu is seletec if i you for example 
i386-softmmu/qemu-system-i386 -machine pc


i only get the complete list of available cpus if i add -cpu ? to the 
machine parameter

Re: [Qemu-devel] how to get the default cpu for a softmmu

[Alex Bennée]

Dennis Luehring  writes:

> i can get a list of all supported CPUs by using the -cpu ? commandline
>
> but which CPU is selected if i don't give the -cpu parameter on normal
> qemu start of a softmmu?

IIRC it is dependant on the machine model you select.

--
Alex Bennée

Re: [Qemu-devel] [PATCH v4 0/3] IOMMU: intel_iommu support map and unmap notifications

[no-reply]

Hi,

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

Type: series
Message-id: 1476526326-32363-1-git-send-email-bd.a...@gmail.com
Subject: [Qemu-devel] [PATCH v4 0/3] IOMMU: intel_iommu support map and unmap 
notifications

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

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

# Useful git options
git config --local diff.renamelimit 0
git config --local diff.renames True

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

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

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
2bf1af5 IOMMU: enable intel_iommu map and unmap notifiers
859d070 IOMMU: change iommu_op->translate's is_write to flags, add support to 
NO_FAIL flag mode
45f4bb2 IOMMU: add option to enable VTD_CAP_CM to vIOMMU capility exposoed to 
guest

=== OUTPUT BEGIN ===
Checking PATCH 1/3: IOMMU: add option to enable VTD_CAP_CM to vIOMMU capility 
exposoed to guest...
ERROR: space required before the open brace '{'
#31: FILE: hw/i386/intel_iommu.c:2389:
+if (s->cache_mode_enabled){

total: 1 errors, 0 warnings, 32 lines checked

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

Checking PATCH 2/3: IOMMU: change iommu_op->translate's is_write to flags, add 
support to NO_FAIL flag mode...
WARNING: line over 80 characters
#22: FILE: exec.c:435:
+iotlb = mr->iommu_ops->translate(mr, addr, is_write ? IOMMU_WO : 
IOMMU_RO);

WARNING: line over 80 characters
#102: FILE: hw/i386/intel_iommu.c:632:
+static int vtd_gpa_to_slpte(VTDContextEntry *ce, uint64_t gpa, 
IOMMUAccessFlags flags,

ERROR: switch and case should be at the same indent
#111: FILE: hw/i386/intel_iommu.c:652:
+switch(flags){
+case IOMMU_WO:
[...]
+case IOMMU_RO:
[...]
+case IOMMU_RW: /* passthrow */
+case IOMMU_NO_FAIL:
[...]
+default:

ERROR: space required before the open brace '{'
#111: FILE: hw/i386/intel_iommu.c:652:
+switch(flags){

ERROR: space required before the open parenthesis '('
#111: FILE: hw/i386/intel_iommu.c:652:
+switch(flags){

ERROR: line over 90 characters
#135: FILE: hw/i386/intel_iommu.c:688:
+return (flags == IOMMU_RW || flags == IOMMU_WO) ? -VTD_FR_WRITE : 
-VTD_FR_READ;

WARNING: line over 80 characters
#149: FILE: hw/i386/intel_iommu.c:809:
+   uint8_t devfn, hwaddr addr, 
IOMMUAccessFlags flags,

ERROR: space required before the open brace '{'
#177: FILE: hw/i386/intel_iommu.c:872:
+if (flags != IOMMU_NO_FAIL){

ERROR: space required before the open brace '{'
#201: FILE: hw/i386/intel_iommu.c:896:
+if (flags != IOMMU_NO_FAIL){

WARNING: line over 80 characters
#203: FILE: hw/i386/intel_iommu.c:898:
+VTD_DPRINTF(FLOG, "fault processing is disabled for DMA 
requests "

WARNING: line over 80 characters
#251: FILE: include/exec/memory.h:174:
+IOMMUTLBEntry (*translate)(MemoryRegion *iommu, hwaddr addr, 
IOMMUAccessFlags flags);

WARNING: line over 80 characters
#264: FILE: memory.c:1566:
+iotlb = mr->iommu_ops->translate(mr, addr, is_write ? IOMMU_WO : 
IOMMU_RO);

total: 6 errors, 6 warnings, 214 lines checked

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

Checking PATCH 3/3: IOMMU: enable intel_iommu map and unmap notifiers...
ERROR: trailing whitespace
#33: FILE: hw/i386/intel_iommu.c:148:
+static int vtd_get_did_dev(IntelIOMMUState *s, uint8_t bus_num, uint8_t devfn, 
$

ERROR: "foo * bar" should be "foo *bar"
#34: FILE: hw/i386/intel_iommu.c:149:
+   uint16_t * domain_id)

ERROR: space required before the open brace '{'
#42: FILE: hw/i386/intel_iommu.c:157:
+if (ret_fr){

ERROR: trailing whitespace
#77: FILE: hw/i386/intel_iommu.c:1079:
+static void vtd_iotlb_page_invalidate_notify(IntelIOMMUState *s, $

ERROR: trailing whitespace
#78: FILE: hw/i386/intel_iommu.c:1080:
+   uint16_t domain_id, hwaddr addr, $

ERROR: "foo * bar" should be "foo *bar"
#81: FILE: hw/i386/intel_iommu.c:1083:
+IntelIOMMUNotifierNode * node;

ERROR: space required before the open brace '{'
#83: FILE: hw/i386/intel_iommu.c:1085:
+QLIST_FOREACH(node, &(s->notifiers_list), next){

ERROR: trailing whitespace
#84: FILE: hw/i386/intel_iommu.c:1086:
+VTDAddressSpace *vtd_as = node->vtd_as; $

ERROR: trailing whitespace
#85: FILE: hw/i386/intel_iommu.c:1087:
+uint16_t vfio_domain_id; $

ERROR: trailing whitespace
#86: FILE: 

[Qemu-devel] [PATCH v4 2/3] IOMMU: change iommu_op->translate's is_write to flags, add support to NO_FAIL flag mode

[Aviv B.D]

From: "Aviv Ben-David" 

Supports translation trials without reporting error to guest on
translation failure.

Signed-off-by: Aviv Ben-David 
---
 exec.c|  2 +-
 hw/i386/amd_iommu.c   |  4 ++--
 hw/i386/intel_iommu.c | 65 ---
 include/exec/memory.h |  4 ++--
 memory.c  |  2 +-
 5 files changed, 47 insertions(+), 30 deletions(-)

diff --git a/exec.c b/exec.c
index 374c364..1b16fe4 100644
--- a/exec.c
+++ b/exec.c
@@ -432,7 +432,7 @@ MemoryRegion *address_space_translate(AddressSpace *as, 
hwaddr addr,
 break;
 }
 
-iotlb = mr->iommu_ops->translate(mr, addr, is_write);
+iotlb = mr->iommu_ops->translate(mr, addr, is_write ? IOMMU_WO : 
IOMMU_RO);
 addr = ((iotlb.translated_addr & ~iotlb.addr_mask)
 | (addr & iotlb.addr_mask));
 *plen = MIN(*plen, (addr | iotlb.addr_mask) - addr + 1);
diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index 47b79d9..1f0d76b 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -988,7 +988,7 @@ static inline bool amdvi_is_interrupt_addr(hwaddr addr)
 }
 
 static IOMMUTLBEntry amdvi_translate(MemoryRegion *iommu, hwaddr addr,
- bool is_write)
+ IOMMUAccessFlags flags)
 {
 AMDVIAddressSpace *as = container_of(iommu, AMDVIAddressSpace, iommu);
 AMDVIState *s = as->iommu_state;
@@ -1017,7 +1017,7 @@ static IOMMUTLBEntry amdvi_translate(MemoryRegion *iommu, 
hwaddr addr,
 return ret;
 }
 
-amdvi_do_translate(as, addr, is_write, );
+amdvi_do_translate(as, addr, flags & IOMMU_WO, );
 trace_amdvi_translation_result(as->bus_num, PCI_SLOT(as->devfn),
 PCI_FUNC(as->devfn), addr, ret.translated_addr);
 return ret;
diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 6e5ad7b..9ffa45f 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -364,7 +364,7 @@ static void vtd_set_frcd_and_update_ppf(IntelIOMMUState *s, 
uint16_t index)
 /* Must not update F field now, should be done later */
 static void vtd_record_frcd(IntelIOMMUState *s, uint16_t index,
 uint16_t source_id, hwaddr addr,
-VTDFaultReason fault, bool is_write)
+VTDFaultReason fault, IOMMUAccessFlags flags)
 {
 uint64_t hi = 0, lo;
 hwaddr frcd_reg_addr = DMAR_FRCD_REG_OFFSET + (((uint64_t)index) << 4);
@@ -373,7 +373,7 @@ static void vtd_record_frcd(IntelIOMMUState *s, uint16_t 
index,
 
 lo = VTD_FRCD_FI(addr);
 hi = VTD_FRCD_SID(source_id) | VTD_FRCD_FR(fault);
-if (!is_write) {
+if (!(flags == IOMMU_WO || flags == IOMMU_RW)) {
 hi |= VTD_FRCD_T;
 }
 vtd_set_quad_raw(s, frcd_reg_addr, lo);
@@ -404,7 +404,7 @@ static bool vtd_try_collapse_fault(IntelIOMMUState *s, 
uint16_t source_id)
 /* Log and report an DMAR (address translation) fault to software */
 static void vtd_report_dmar_fault(IntelIOMMUState *s, uint16_t source_id,
   hwaddr addr, VTDFaultReason fault,
-  bool is_write)
+  IOMMUAccessFlags flags)
 {
 uint32_t fsts_reg = vtd_get_long_raw(s, DMAR_FSTS_REG);
 
@@ -415,7 +415,7 @@ static void vtd_report_dmar_fault(IntelIOMMUState *s, 
uint16_t source_id,
 return;
 }
 VTD_DPRINTF(FLOG, "sid 0x%"PRIx16 ", fault %d, addr 0x%"PRIx64
-", is_write %d", source_id, fault, addr, is_write);
+", flags %d", source_id, fault, addr, flags);
 if (fsts_reg & VTD_FSTS_PFO) {
 VTD_DPRINTF(FLOG, "new fault is not recorded due to "
 "Primary Fault Overflow");
@@ -433,7 +433,7 @@ static void vtd_report_dmar_fault(IntelIOMMUState *s, 
uint16_t source_id,
 return;
 }
 
-vtd_record_frcd(s, s->next_frcd_reg, source_id, addr, fault, is_write);
+vtd_record_frcd(s, s->next_frcd_reg, source_id, addr, fault, flags);
 
 if (fsts_reg & VTD_FSTS_PPF) {
 VTD_DPRINTF(FLOG, "there are pending faults already, "
@@ -629,7 +629,7 @@ static bool vtd_slpte_nonzero_rsvd(uint64_t slpte, uint32_t 
level)
 /* Given the @gpa, get relevant @slptep. @slpte_level will be the last level
  * of the translation, can be used for deciding the size of large page.
  */
-static int vtd_gpa_to_slpte(VTDContextEntry *ce, uint64_t gpa, bool is_write,
+static int vtd_gpa_to_slpte(VTDContextEntry *ce, uint64_t gpa, 
IOMMUAccessFlags flags,
 uint64_t *slptep, uint32_t *slpte_level,
 bool *reads, bool *writes)
 {
@@ -649,7 +649,20 @@ static int vtd_gpa_to_slpte(VTDContextEntry *ce, uint64_t 
gpa, bool is_write,
 }
 
 /* FIXME: what is the Atomics request here? */
-access_right_check = is_write ? VTD_SL_W : VTD_SL_R;
+switch(flags){
+case 

[Qemu-devel] [PATCH v4 1/3] IOMMU: add option to enable VTD_CAP_CM to vIOMMU capility exposoed to guest

[Aviv B.D]

From: "Aviv Ben-David" 

This capability asks the guest to invalidate cache before each map operation.
We can use this invalidation to trap map operations in the hypervisor.

This capability only enabled when cache-mode property of the device is true.

Signed-off-by: Aviv Ben-David 
---
 hw/i386/intel_iommu.c  | 5 +
 hw/i386/intel_iommu_internal.h | 1 +
 include/hw/i386/intel_iommu.h  | 2 ++
 3 files changed, 8 insertions(+)

diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 2efd69b..6e5ad7b 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -2012,6 +2012,7 @@ static const MemoryRegionOps vtd_mem_ops = {
 
 static Property vtd_properties[] = {
 DEFINE_PROP_UINT32("version", IntelIOMMUState, version, 0),
+DEFINE_PROP_BOOL("cache-mode", IntelIOMMUState, cache_mode_enabled, FALSE),
 DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -2385,6 +2386,10 @@ static void vtd_init(IntelIOMMUState *s)
 s->ecap |= VTD_ECAP_IR | VTD_ECAP_EIM | VTD_ECAP_MHMV;
 }
 
+if (s->cache_mode_enabled){
+s->cap |= VTD_CAP_CM;
+}
+
 vtd_reset_context_cache(s);
 vtd_reset_iotlb(s);
 
diff --git a/hw/i386/intel_iommu_internal.h b/hw/i386/intel_iommu_internal.h
index 0829a50..35d9f3a 100644
--- a/hw/i386/intel_iommu_internal.h
+++ b/hw/i386/intel_iommu_internal.h
@@ -201,6 +201,7 @@
 #define VTD_CAP_MAMV(VTD_MAMV << 48)
 #define VTD_CAP_PSI (1ULL << 39)
 #define VTD_CAP_SLLPS   ((1ULL << 34) | (1ULL << 35))
+#define VTD_CAP_CM  (1ULL << 7)
 
 /* Supported Adjusted Guest Address Widths */
 #define VTD_CAP_SAGAW_SHIFT 8
diff --git a/include/hw/i386/intel_iommu.h b/include/hw/i386/intel_iommu.h
index a42dbd7..7a94f16 100644
--- a/include/hw/i386/intel_iommu.h
+++ b/include/hw/i386/intel_iommu.h
@@ -258,6 +258,8 @@ struct IntelIOMMUState {
 uint8_t womask[DMAR_REG_SIZE];  /* WO (write only - read returns 0) */
 uint32_t version;
 
+bool cache_mode_enabled;/* RO - is cap CM enabled? */
+
 dma_addr_t root;/* Current root table pointer */
 bool root_extended; /* Type of root table (extended or not) */
 bool dmar_enabled;  /* Set if DMA remapping is enabled */
-- 
1.9.1

[Qemu-devel] [PATCH v4 0/3] IOMMU: intel_iommu support map and unmap notifications

[Aviv B.D]

From: "Aviv Ben-David" 

* Advertize Cache Mode capability in iommu cap register. 
  This capability is controlled by "cache-mode" property of intel-iommu device.
  To enable this option call QEMU with "-device intel-iommu,cache-mode=true".

* On page cache invalidation in intel vIOMMU, check if the domain belong to
  registered notifier, and notify accordingly.

Currently this patch still doesn't enabling VFIO devices support with vIOMMU 
present. Current problems:
* vfio_iommu_map_notify is not aware about memory range belong to specific 
  VFIOGuestIOMMU.
* memory_region_iommu_replay hangs QEMU on start up while it itterate over 
  64bit address space. Commenting out the call to this function enables 
  workable VFIO device while vIOMMU present.

Changes from v1 to v2:
* remove assumption that the cache do not clears
* fix lockup on high load.

Changes from v2 to v3:
* remove debug leftovers
* split to sepearate commits
* change is_write to flags in vtd_do_iommu_translate, add IOMMU_NO_FAIL 
  to suppress error propagating to guest.

Changes from v3 to v4:
* Add property to intel_iommu device to control the CM capability, 
  default to False.
* Use s->iommu_ops.notify_flag_changed to register notifiers.

Aviv Ben-David (3):
  IOMMU: add option to enable VTD_CAP_CM to vIOMMU capility exposoed to
guest
  IOMMU: change iommu_op->translate's is_write to flags, add support to
NO_FAIL flag mode
  IOMMU: enable intel_iommu map and unmap notifiers

 exec.c |   2 +-
 hw/i386/amd_iommu.c|   4 +-
 hw/i386/intel_iommu.c  | 170 +
 hw/i386/intel_iommu_internal.h |   3 +
 include/exec/memory.h  |   4 +-
 include/hw/i386/intel_iommu.h  |  10 +++
 memory.c   |   2 +-
 7 files changed, 159 insertions(+), 36 deletions(-)

-- 
1.9.1

[Qemu-devel] [PATCH v4 3/3] IOMMU: enable intel_iommu map and unmap notifiers

[Aviv B.D]

From: "Aviv Ben-David" 

* register notifiers when vtd_iommu_notify_flag_changed is called.
* Notify on IOTLB page invalidation about unmap and map operations.

Adds a list of registered vtd_as's to intel iommu state to save
iterations over each PCI device in a search of the corrosponding domain.

Signed-off-by: Aviv Ben-David 
---
 hw/i386/intel_iommu.c  | 102 ++---
 hw/i386/intel_iommu_internal.h |   2 +
 include/hw/i386/intel_iommu.h  |   8 
 3 files changed, 105 insertions(+), 7 deletions(-)

diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 9ffa45f..0560e3c 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -51,6 +51,9 @@ static int vtd_dbgflags = VTD_DBGBIT(GENERAL) | 
VTD_DBGBIT(CSR);
 #define VTD_DPRINTF(what, fmt, ...) do {} while (0)
 #endif
 
+static int vtd_dev_to_context_entry(IntelIOMMUState *s, uint8_t bus_num,
+uint8_t devfn, VTDContextEntry *ce);
+
 static void vtd_define_quad(IntelIOMMUState *s, hwaddr addr, uint64_t val,
 uint64_t wmask, uint64_t w1cmask)
 {
@@ -142,6 +145,23 @@ static uint64_t vtd_set_clear_mask_quad(IntelIOMMUState 
*s, hwaddr addr,
 return new_val;
 }
 
+static int vtd_get_did_dev(IntelIOMMUState *s, uint8_t bus_num, uint8_t devfn, 
+   uint16_t * domain_id)
+{
+VTDContextEntry ce;
+int ret_fr;
+
+assert(domain_id);
+
+ret_fr = vtd_dev_to_context_entry(s, bus_num, devfn, );
+if (ret_fr){
+return -1;
+}
+
+*domain_id =  VTD_CONTEXT_ENTRY_DID(ce.hi);
+return 0;
+}
+
 /* GHashTable functions */
 static gboolean vtd_uint64_equal(gconstpointer v1, gconstpointer v2)
 {
@@ -682,9 +702,6 @@ static int vtd_gpa_to_slpte(VTDContextEntry *ce, uint64_t 
gpa, IOMMUAccessFlags
 *reads = (*reads) && (slpte & VTD_SL_R);
 *writes = (*writes) && (slpte & VTD_SL_W);
 if (!(slpte & access_right_check)) {
-VTD_DPRINTF(GENERAL, "error: lack of %s permission for "
-"gpa 0x%"PRIx64 " slpte 0x%"PRIx64,
-(flags == IOMMU_WO ? "write" : "read"), gpa, slpte);
 return (flags == IOMMU_RW || flags == IOMMU_WO) ? -VTD_FR_WRITE : 
-VTD_FR_READ;
 }
 if (vtd_slpte_nonzero_rsvd(slpte, level)) {
@@ -732,9 +749,6 @@ static int vtd_dev_to_context_entry(IntelIOMMUState *s, 
uint8_t bus_num,
 }
 
 if (!vtd_context_entry_present(ce)) {
-VTD_DPRINTF(GENERAL,
-"error: context-entry #%"PRIu8 "(bus #%"PRIu8 ") "
-"is not present", devfn, bus_num);
 return -VTD_FR_CONTEXT_ENTRY_P;
 } else if ((ce->hi & VTD_CONTEXT_ENTRY_RSVD_HI) ||
(ce->lo & VTD_CONTEXT_ENTRY_RSVD_LO)) {
@@ -1062,6 +1076,55 @@ static void vtd_iotlb_domain_invalidate(IntelIOMMUState 
*s, uint16_t domain_id)
 _id);
 }
 
+static void vtd_iotlb_page_invalidate_notify(IntelIOMMUState *s, 
+   uint16_t domain_id, hwaddr addr, 
+   uint8_t am)
+{
+IntelIOMMUNotifierNode * node;
+
+QLIST_FOREACH(node, &(s->notifiers_list), next){
+VTDAddressSpace *vtd_as = node->vtd_as; 
+uint16_t vfio_domain_id; 
+int ret = vtd_get_did_dev(s, pci_bus_num(vtd_as->bus), vtd_as->devfn, 
+  _domain_id);
+//int i=0;
+if (!ret && domain_id == vfio_domain_id){
+IOMMUTLBEntry entry; 
+
+/* notify unmap */
+if (node->notifier_flag & IOMMU_NOTIFIER_UNMAP){
+VTD_DPRINTF(GENERAL, "Remove addr 0x%"PRIx64 " mask %d", addr, 
am);
+entry.target_as = _space_memory;
+entry.iova = addr & VTD_PAGE_MASK_4K;
+entry.translated_addr = 0;
+entry.addr_mask = ~VTD_PAGE_MASK(VTD_PAGE_SHIFT_4K + am);
+entry.perm = IOMMU_NONE;
+memory_region_notify_iommu(>vtd_as->iommu, entry);
+}
+   
+/* notify map */
+if (node->notifier_flag & IOMMU_NOTIFIER_MAP){
+hwaddr original_addr = addr;
+VTD_DPRINTF(GENERAL, "add addr 0x%"PRIx64 " mask %d", addr, 
am);
+while (addr < original_addr + (1<iommu_ops.translate(
+
>vtd_as->iommu,
+addr, 
+IOMMU_NO_FAIL); 
+if (entry.perm != IOMMU_NONE){
+addr += entry.addr_mask + 1; 
+memory_region_notify_iommu(>vtd_as->iommu, 
entry);
+ 

Re: [Qemu-devel] [PATCH] target-arm/translate.c: fix movs pc, lr exception return on ARMv7

[Alex Bennée]

Peter Maydell  writes:

> On 14 October 2016 at 16:13, Alex Bennée  wrote:
>> This was broken by the fix for 9b6a3ea7a699594162ed3d11e4e04b98568dc5c0.
>> Specifically a movs pc,lr in the kernels ret_fast_syscall returning to
>> some thumb mode user space code but store_reg unconditionally aligned
>> the return PC instead of treating the return as an "interworking"
>> branch.
>>
>> I suspect we need to audit all calls to store_reg that might involve the
>> PC to ensure "interworking" branches are correctly handled. Also I'm not
>> quite sure how the code worked before 9b6a3e as the store_reg path
>> wouldn't have triggered the store_cpu_field(var, thumb) to set the
>> processor mode back to thumb.
>>
>> Signed-off-by: Alex Bennée 
>
> I think this is the wrong fix to the problem -- see the
> patch I sent a few days back.

Well at least my analysis of the problem was correct even if the
solution was too hacky. Your patch is obviously the better solution ;-)

For ref:

  [PATCH] Fix masking of PC lower bits when doing exception returns

>
> thanks
> -- PMM


--
Alex Bennée

Re: [Qemu-devel] [v2 0/5] Allow blockdev-add for SSH

[no-reply]

Hi,

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

Subject: [Qemu-devel] [v2 0/5] Allow blockdev-add for SSH
Type: series
Message-id: 1476522280-23211-1-git-send-email-ashijeetacha...@gmail.com

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

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag] 
patchew/1476522280-23211-1-git-send-email-ashijeetacha...@gmail.com -> 
patchew/1476522280-23211-1-git-send-email-ashijeetacha...@gmail.com
Switched to a new branch 'test'
ff808eb qapi: allow blockdev-add for ssh
7c6e759 block/ssh: Use InetSocketAddress options
a30c804 block/ssh: Use inet_connect_saddr() to establish socket connection
5b5cfe0 block/ssh: Add InetSocketAddress and accept it
fd4fb5d block/ssh: Add ssh_has_filename_options_conflict()

=== OUTPUT BEGIN ===
Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
Cloning into 'dtc'...
Submodule path 'dtc': checked out '65cc4d2748a2c2e6f27f1cf39e07a5dbabd80ebf'
  BUILD   centos6
  ARCHIVE qemu.tgz
  ARCHIVE dtc.tgz
  COPYRUNNER
  RUN test-quick in centos6
Packages installed:
SDL-devel-1.2.14-7.el6_7.1.x86_64
ccache-3.1.6-2.el6.x86_64
epel-release-6-8.noarch
gcc-4.4.7-17.el6.x86_64
git-1.7.1-4.el6_7.1.x86_64
glib2-devel-2.28.8-5.el6.x86_64
libfdt-devel-1.4.0-1.el6.x86_64
make-3.81-23.el6.x86_64
package g++ is not installed
pixman-devel-0.32.8-1.el6.x86_64
tar-1.23-15.el6_8.x86_64
zlib-devel-1.2.3-29.el6.x86_64

Environment variables:
PACKAGES=libfdt-devel ccache tar git make gcc g++ zlib-devel 
glib2-devel SDL-devel pixman-devel epel-release
HOSTNAME=05347c7f0feb
TERM=xterm
MAKEFLAGS= -j16
HISTSIZE=1000
J=16
USER=root
CCACHE_DIR=/var/tmp/ccache
EXTRA_CONFIGURE_OPTS=
V=
SHOW_ENV=1
MAIL=/var/spool/mail/root
PATH=/usr/lib/ccache:/usr/lib64/ccache:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
PWD=/
LANG=en_US.UTF-8
TARGET_LIST=
HISTCONTROL=ignoredups
SHLVL=1
HOME=/root
TEST_DIR=/tmp/qemu-test
LOGNAME=root
LESSOPEN=||/usr/bin/lesspipe.sh %s
FEATURES= dtc
DEBUG=
G_BROKEN_FILENAMES=1
CCACHE_HASHDIR=
_=/usr/bin/env

Configure options:
--enable-werror --target-list=x86_64-softmmu,aarch64-softmmu 
--prefix=/var/tmp/qemu-build/install
No C++ compiler available; disabling C++ specific optional code
Install prefix/var/tmp/qemu-build/install
BIOS directory/var/tmp/qemu-build/install/share/qemu
binary directory  /var/tmp/qemu-build/install/bin
library directory /var/tmp/qemu-build/install/lib
module directory  /var/tmp/qemu-build/install/lib/qemu
libexec directory /var/tmp/qemu-build/install/libexec
include directory /var/tmp/qemu-build/install/include
config directory  /var/tmp/qemu-build/install/etc
local state directory   /var/tmp/qemu-build/install/var
Manual directory  /var/tmp/qemu-build/install/share/man
ELF interp prefix /usr/gnemul/qemu-%M
Source path   /tmp/qemu-test/src
C compilercc
Host C compiler   cc
C++ compiler  
Objective-C compiler cc
ARFLAGS   rv
CFLAGS-O2 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -g 
QEMU_CFLAGS   -I/usr/include/pixman-1-pthread -I/usr/include/glib-2.0 
-I/usr/lib64/glib-2.0/include   -fPIE -DPIE -m64 -D_GNU_SOURCE 
-D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes 
-Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes 
-fno-strict-aliasing -fno-common -fwrapv  -Wendif-labels -Wmissing-include-dirs 
-Wempty-body -Wnested-externs -Wformat-security -Wformat-y2k -Winit-self 
-Wignored-qualifiers -Wold-style-declaration -Wold-style-definition 
-Wtype-limits -fstack-protector-all
LDFLAGS   -Wl,--warn-common -Wl,-z,relro -Wl,-z,now -pie -m64 -g 
make  make
install   install
pythonpython -B
smbd  /usr/sbin/smbd
module supportno
host CPU  x86_64
host big endian   no
target list   x86_64-softmmu aarch64-softmmu
tcg debug enabled no
gprof enabled no
sparse enabledno
strip binariesyes
profiler  no
static build  no
pixmansystem
SDL support   yes (1.2.14)
GTK support   no 
GTK GL supportno
VTE support   no 
TLS priority  NORMAL
GNUTLS supportno
GNUTLS rndno
libgcrypt no
libgcrypt kdf no
nettleno 
nettle kdfno
libtasn1  no
curses supportno
virgl support no
curl support  no
mingw32 support   no
Audio drivers oss
Block whitelist (rw) 
Block whitelist (ro) 
VirtFS supportno
VNC support   yes
VNC SASL support  no
VNC JPEG support  no
VNC PNG support   no
xen support   no
brlapi supportno
bluez  supportno
Documentation no
PIE   yes

[Qemu-devel] [v2 4/5] block/ssh: Use InetSocketAddress options

[Ashijeet Acharya]

Drop the use of legacy options in favour of the InetSocketAddress
options.

Signed-off-by: Ashijeet Acharya 
---
 block/ssh.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/block/ssh.c b/block/ssh.c
index 6420359..7fec0e1 100644
--- a/block/ssh.c
+++ b/block/ssh.c
@@ -199,6 +199,7 @@ static int parse_uri(const char *filename, QDict *options, 
Error **errp)
 {
 URI *uri = NULL;
 QueryParams *qp;
+char *port_str;
 int i;
 
 uri = uri_parse(filename);
@@ -231,11 +232,10 @@ static int parse_uri(const char *filename, QDict 
*options, Error **errp)
 qdict_put(options, "user", qstring_from_str(uri->user));
 }
 
-qdict_put(options, "host", qstring_from_str(uri->server));
+qdict_put(options, "server.host", qstring_from_str(uri->server));
 
-if (uri->port) {
-qdict_put(options, "port", qint_from_int(uri->port));
-}
+port_str = g_strdup_printf("%d", uri->port ?: 22);
+qdict_put(options, "server.port", qstring_from_str(port_str));
 
 qdict_put(options, "path", qstring_from_str(uri->path));
 
@@ -251,6 +251,7 @@ static int parse_uri(const char *filename, QDict *options, 
Error **errp)
 
 query_params_free(qp);
 uri_free(uri);
+g_free(port_str);
 return 0;
 
  err:
-- 
2.6.2

[Qemu-devel] [v2 3/5] block/ssh: Use inet_connect_saddr() to establish socket connection

[Ashijeet Acharya]

Make inet_connect_saddr() in util/qemu-socktets.c public and use it
instead of inet_connect() because this directly takes the
InetSocketAddress to establish a socket connection for the SSH
block driver.

Signed-off-by: Ashijeet Acharya 
---
 block/ssh.c| 5 +
 include/qemu/sockets.h | 2 ++
 util/qemu-sockets.c| 2 +-
 3 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/block/ssh.c b/block/ssh.c
index 3b18907..6420359 100644
--- a/block/ssh.c
+++ b/block/ssh.c
@@ -666,13 +666,10 @@ static int connect_to_ssh(BDRVSSHState *s, QDict *options,
 goto err;
 }
 
-/* Construct the host:port name for inet_connect. */
-g_free(s->hostport);
 port = atoi(s->inet->port);
-s->hostport = g_strdup_printf("%s:%d", s->inet->host, port);
 
 /* Open the socket and connect. */
-s->sock = inet_connect(s->hostport, errp);
+s->sock = inet_connect_saddr(s->inet, errp, NULL, NULL);
 if (s->sock < 0) {
 ret = -EIO;
 goto err;
diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h
index 9eb2470..5589e68 100644
--- a/include/qemu/sockets.h
+++ b/include/qemu/sockets.h
@@ -34,6 +34,8 @@ typedef void NonBlockingConnectHandler(int fd, Error *err, 
void *opaque);
 
 InetSocketAddress *inet_parse(const char *str, Error **errp);
 int inet_connect(const char *str, Error **errp);
+int inet_connect_saddr(InetSocketAddress *saddr, Error **errp,
+   NonBlockingConnectHandler *callback, void *opaque);
 
 NetworkAddressFamily inet_netfamily(int family);
 
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 4cef549..3411888 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -412,7 +412,7 @@ static struct addrinfo 
*inet_parse_connect_saddr(InetSocketAddress *saddr,
  * function succeeds, callback will be called when the connection
  * completes, with the file descriptor on success, or -1 on error.
  */
-static int inet_connect_saddr(InetSocketAddress *saddr, Error **errp,
+int inet_connect_saddr(InetSocketAddress *saddr, Error **errp,
   NonBlockingConnectHandler *callback, void 
*opaque)
 {
 Error *local_err = NULL;
-- 
2.6.2

[Qemu-devel] [v2 1/5] block/ssh: Add ssh_has_filename_options_conflict()

[Ashijeet Acharya]

We have 5 options plus ("server") option which is added in the next
patch that conflict with specifying a SSH filename. We need to iterate
over all the options to check whether its key has an "server." prefix.

This iteration will help us adding the new option "server" easily.

Signed-off-by: Ashijeet Acharya 
---
 block/ssh.c | 27 +--
 1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/block/ssh.c b/block/ssh.c
index 5ce12b6..75cb7bc 100644
--- a/block/ssh.c
+++ b/block/ssh.c
@@ -254,15 +254,30 @@ static int parse_uri(const char *filename, QDict 
*options, Error **errp)
 return -EINVAL;
 }
 
+static bool ssh_has_filename_options_conflict(QDict *options, Error **errp)
+{
+const QDictEntry *qe;
+
+for (qe = qdict_first(options); qe; qe = qdict_next(options, qe)) {
+if (!strcmp(qe->key, "host") ||
+!strcmp(qe->key, "port") ||
+!strcmp(qe->key, "path") ||
+!strcmp(qe->key, "user") ||
+!strcmp(qe->key, "host_key_check"))
+{
+error_setg(errp, "Option '%s' cannot be used with a file name",
+   qe->key);
+return true;
+}
+}
+
+return false;
+}
+
 static void ssh_parse_filename(const char *filename, QDict *options,
Error **errp)
 {
-if (qdict_haskey(options, "user") ||
-qdict_haskey(options, "host") ||
-qdict_haskey(options, "port") ||
-qdict_haskey(options, "path") ||
-qdict_haskey(options, "host_key_check")) {
-error_setg(errp, "user, host, port, path, host_key_check cannot be 
used at the same time as a file option");
+if (ssh_has_filename_options_conflict(options, errp)) {
 return;
 }
 
-- 
2.6.2

[Qemu-devel] [v2 2/5] block/ssh: Add InetSocketAddress and accept it

[Ashijeet Acharya]

Add InetSocketAddress compatibility to SSH driver.

Add a new option "server" to the SSH block driver which then accepts
a InetSocketAddress.

"host" and "port" are supported as legacy options and are mapped to
their InetSocketAddress representation.

Signed-off-by: Ashijeet Acharya 
---
 block/ssh.c | 83 ++---
 1 file changed, 74 insertions(+), 9 deletions(-)

diff --git a/block/ssh.c b/block/ssh.c
index 75cb7bc..3b18907 100644
--- a/block/ssh.c
+++ b/block/ssh.c
@@ -30,10 +30,14 @@
 #include "block/block_int.h"
 #include "qapi/error.h"
 #include "qemu/error-report.h"
+#include "qemu/cutils.h"
 #include "qemu/sockets.h"
 #include "qemu/uri.h"
+#include "qapi-visit.h"
 #include "qapi/qmp/qint.h"
 #include "qapi/qmp/qstring.h"
+#include "qapi/qobject-input-visitor.h"
+#include "qapi/qobject-output-visitor.h"
 
 /* DEBUG_SSH=1 enables the DPRINTF (debugging printf) statements in
  * this block driver code.
@@ -74,6 +78,8 @@ typedef struct BDRVSSHState {
  */
 LIBSSH2_SFTP_ATTRIBUTES attrs;
 
+InetSocketAddress *inet;
+
 /* Used to warn if 'flush' is not supported. */
 char *hostport;
 bool unsafe_flush_warning;
@@ -263,7 +269,9 @@ static bool ssh_has_filename_options_conflict(QDict 
*options, Error **errp)
 !strcmp(qe->key, "port") ||
 !strcmp(qe->key, "path") ||
 !strcmp(qe->key, "user") ||
-!strcmp(qe->key, "host_key_check"))
+!strcmp(qe->key, "host_key_check") ||
+!strcmp(qe->key, "server") ||
+!strstart(qe->key, "server.", NULL))
 {
 error_setg(errp, "Option '%s' cannot be used with a file name",
qe->key);
@@ -555,13 +563,66 @@ static QemuOptsList ssh_runtime_opts = {
 },
 };
 
+static bool ssh_process_legacy_socket_options(QDict *output_opts,
+  QemuOpts *legacy_opts,
+  Error **errp)
+{
+const char *host = qemu_opt_get(legacy_opts, "host");
+const char *port = qemu_opt_get(legacy_opts, "port");
+
+if (!host && port) {
+error_setg(errp, "port may not be used without host");
+return false;
+} else {
+qdict_put(output_opts, "server.host", qstring_from_str(host));
+qdict_put(output_opts, "server.port",
+  qstring_from_str(port ?: stringify(22)));
+}
+
+return true;
+}
+
+static InetSocketAddress *ssh_config(BDRVSSHState *s, QDict *options,
+ Error **errp)
+{
+InetSocketAddress *inet = NULL;
+QDict *addr = NULL;
+QObject *crumpled_addr = NULL;
+Visitor *iv = NULL;
+Error *local_error = NULL;
+
+qdict_extract_subqdict(options, , "server.");
+if (!qdict_size(addr)) {
+error_setg(errp, "SSH server address missing");
+goto out;
+}
+
+crumpled_addr = qdict_crumple(addr, true, errp);
+if (!crumpled_addr) {
+goto out;
+}
+
+iv = qobject_input_visitor_new_autocast(crumpled_addr, true, 1, true);
+visit_type_InetSocketAddress(iv, NULL, , _error);
+if (local_error) {
+error_propagate(errp, local_error);
+goto out;
+}
+
+out:
+QDECREF(addr);
+qobject_decref(crumpled_addr);
+visit_free(iv);
+return inet;
+}
+
 static int connect_to_ssh(BDRVSSHState *s, QDict *options,
   int ssh_flags, int creat_mode, Error **errp)
 {
 int r, ret;
 QemuOpts *opts = NULL;
 Error *local_err = NULL;
-const char *host, *user, *path, *host_key_check;
+const char *user, *path, *host_key_check;
 int port;
 
 opts = qemu_opts_create(_runtime_opts, NULL, 0, _abort);
@@ -572,15 +633,11 @@ static int connect_to_ssh(BDRVSSHState *s, QDict *options,
 goto err;
 }
 
-host = qemu_opt_get(opts, "host");
-if (!host) {
+if (!ssh_process_legacy_socket_options(options, opts, errp)) {
 ret = -EINVAL;
-error_setg(errp, "No hostname was specified");
 goto err;
 }
 
-port = qemu_opt_get_number(opts, "port", 22);
-
 path = qemu_opt_get(opts, "path");
 if (!path) {
 ret = -EINVAL;
@@ -603,9 +660,16 @@ static int connect_to_ssh(BDRVSSHState *s, QDict *options,
 host_key_check = "yes";
 }
 
+/* Pop the config into our state object, Exit if invalid */
+s->inet = ssh_config(s, options, errp);
+if (!s->inet) {
+goto err;
+}
+
 /* Construct the host:port name for inet_connect. */
 g_free(s->hostport);
-s->hostport = g_strdup_printf("%s:%d", host, port);
+port = atoi(s->inet->port);
+s->hostport = g_strdup_printf("%s:%d", s->inet->host, port);
 
 /* Open the socket and connect. */
 s->sock = inet_connect(s->hostport, errp);
@@ -634,7 +698,8 @@ static int connect_to_ssh(BDRVSSHState *s, QDict *options,
 }
 
 /* Check the remote host's 

[Qemu-devel] [v2 5/5] qapi: allow blockdev-add for ssh

[Ashijeet Acharya]

Introduce new object 'BlockdevOptionsSsh' in qapi/block-core.json to
support blockdev-add for SSH network protocol driver. Use only 'struct
InetSocketAddress' since SSH only supports connection over TCP.

Signed-off-by: Ashijeet Acharya 
---
 qapi/block-core.json | 24 ++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 9d797b8..2e8a390 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1716,7 +1716,8 @@
 'dmg', 'file', 'ftp', 'ftps', 'gluster', 'host_cdrom',
 'host_device', 'http', 'https', 'luks', 'null-aio', 'null-co',
 'parallels', 'qcow', 'qcow2', 'qed', 'quorum', 'raw',
-   'replication', 'tftp', 'vdi', 'vhdx', 'vmdk', 'vpc', 'vvfat' ] }
+'replication', 'ssh', 'tftp', 'vdi', 'vhdx', 'vmdk', 'vpc',
+'vvfat' ] }
 
 ##
 # @BlockdevOptionsFile
@@ -1953,6 +1954,25 @@
 '*vport': 'int',
 '*segment': 'str' } }
 
+##
+# @BlockdevoptionsSsh
+#
+# @server:  host address and port number
+#
+# @path:path to the image on the host
+#
+# @user:user as which to connect
+#
+# @host_key_check   defines how and what to check the host key against
+#
+# Since 2.8
+##
+{ 'struct': 'BlockdevoptionsSsh',
+  'data': { 'server': 'InetSocketAddress',
+'path': 'str',
+'user': 'str',
+'*host_key_check': 'str' } }
+
 
 ##
 # @BlkdebugEvent
@@ -2281,7 +2301,7 @@
 # TODO rbd: Wait for structured options
   'replication':'BlockdevOptionsReplication',
 # TODO sheepdog: Wait for structured options
-# TODO ssh: Should take InetSocketAddress for 'host'?
+  'ssh':'BlockdevoptionsSsh',
   'tftp':   'BlockdevOptionsCurl',
   'vdi':'BlockdevOptionsGenericFormat',
   'vhdx':   'BlockdevOptionsGenericFormat',
-- 
2.6.2

[Qemu-devel] [v2 0/5] Allow blockdev-add for SSH

[Ashijeet Acharya]

Previously posted series patches:
v1: http://lists.nongnu.org/archive/html/qemu-devel/2016-10/msg02137.html

This series adds blockdev-add support for SSH block driver.

This patch series has an additional patch 'Patch 3'.

Patch 1 prepares the code for the addition of a new option prefix,
which is "server.". This is accomplished by adding a
ssh_has_filename_options_conflict() function which helps to iterate
over the various options and check for conflict.

Patch 2 first adds InetSocketAddress compatibility to SSH block driver
and then makes it accept a InetSocketAddress under the "server" option.
The old options "host" and "port" are supported as legacy options and
then translated to the respective InetSocketAddress representation.

Patch 3 makes use of inet_connect_saddr() instead of inet_connect()
to establish socket connection.

Patch 4 drops the usage of "host" and "port" outside of
ssh_has_filename_options_conflict() and
ssh_process_legacy_socket_options() functions in order to make them
legacy options completely.

Patch 5 helps to allow blockdev-add support for the SSH block driver
by making the SSH option available.


*** This series depends on the following patch: ***
"qdict: implement a qdict_crumple method for un-flattening a dict"
from Daniel's "QAPI/QOM work for non-scalar object properties"
series.

Changes in v2:
- Use strstart() instead of strcmp() (Kevin)
- Use qobject_input_visitor_new_autocast() instead of
  qmp_input_visitor_new() and change header files accordingly (Kevin)
- Use inet_connect_saddr() instead of inet_connect() (Kevin)
- Drop the contruction of : string (Kevin)
- Fix the bug in ssh_process_legacy_socket_options()

Ashijeet Acharya (5):
  block/ssh: Add ssh_has_filename_options_conflict()
  block/ssh: Add InetSocketAddress and accept it
  block/ssh: Use inet_connect_saddr() to establish socket connection
  block/ssh: Use InetSocketAddress options
  qapi: allow blockdev-add for ssh

 block/ssh.c| 120 -
 include/qemu/sockets.h |   2 +
 qapi/block-core.json   |  24 +-
 util/qemu-sockets.c|   2 +-
 4 files changed, 124 insertions(+), 24 deletions(-)

-- 
2.6.2

[Qemu-devel] [Bug 1593605] Re: windows2008r2 boot failed with uefi

[Laszlo Ersek (Red Hat)]

@alex3kov -- I think you mean the question as

"""
Since this behaviour will not change in future versions of Windows 7 / Windows 
Server 2008 R2, ...
"""

because, again, the problem is not in OVMF but in the Windows guest.

QEMU cannot be expected to recognize (guest OS, guest firmware, hw
config) combinations that might not work. That's not QEMU's business.

Such (automatic or semi-automatic) config tweaks belong to the
management layer. If you use virt-manager or virt-install to create your
guest, and select the guest OS type correctly (or let virt-manager /
virt-install recognize the guest type from a signature of the installer
ISO, which I believe is implemented with the help of libosinfo), then
virt-manager / virt-install will automatically disable Hyper-V
enlightments for you. This is what
https://bugzilla.redhat.com/show_bug.cgi?id=1185253 was about, which I
referenced here earlier.

The virt-manager change is .

So, the answer to your question is to use libvirt and its tools (which
is recommended anyway). Thanks.

(In general, I have no idea why large groups of users keep struggling
with QEMU command lines, when the interface that libvirt provides is
just so much better and easier for production use. The reason I always
recommend libvirt is not because I'm an RH zealot, the reason I
recommend it is that libvirt adds *actual value*, even for the
individual, interactive user.)

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

Title:
  windows2008r2 boot failed with uefi

Status in QEMU:
  Invalid

Bug description:
  I want to run my win2008r2 with uefi. Hypervisor is ubuntu16.04 and my
  qemu command line show below:

  qemu-system-x86_64 -enable-kvm -name win2008r2 -S -machine pc-
  i440fx-2.5,accel=kvm,usb=off -cpu
  host,hv_time,hv_relaxed,hv_spinlocks=0x2000 -drive
  file=/usr/share/qemu/OVMF.fd,if=pflash,format=raw,unit=0,readonly=on
  -drive
  file=/var/lib/libvirt/qemu/nvram/win2008r2_VARS.fd,if=pflash,format=raw,unit=1
  -m size=8388608k,slots=10,maxmem=1073741824k -realtime mlock=off -smp
  8,maxcpus=96,sockets=24,cores=4,threads=1 -numa
  node,nodeid=0,cpus=0-7,mem=8192 -uuid 030638c5-c6aa-
  4f06-82f8-dd2d04fd5705 -no-user-config -nodefaults -chardev
  socket,id=charmonitor,path=/var/lib/libvirt/qemu/domain-
  win2008r2/monitor.sock,server,nowait -mon
  chardev=charmonitor,id=monitor,mode=control -rtc
  base=localtime,clock=vm,driftfix=slew -no-hpet -no-shutdown -boot
  strict=on -device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 -device
  usb-ehci,id=usb1,bus=pci.0,addr=0x4 -device nec-usb-
  xhci,id=usb2,bus=pci.0,addr=0x5 -device
  lsi,id=scsi0,bus=pci.0,addr=0x6 -device virtio-scsi-
  pci,id=scsi1,bus=pci.0,addr=0x7 -device virtio-serial-pci,id=virtio-
  serial0,bus=pci.0,addr=0x8 -drive
  file=/vms/images/win2008r2,format=qcow2,if=none,id=drive-
  ide0-0-0,cache=directsync -device ide-hd,bus=ide.0,unit=0,drive=drive-
  ide0-0-0,id=ide0-0-0,bootindex=1 -drive
  
file=/vms/isos/cn_windows_server_2008_r2_standard_enterprise_datacenter_and_web_with_sp1_x64_dvd_617598.iso,format=raw,if=none,id
  =drive-ide0-1-1,readonly=on -device ide-cd,bus=ide.1,unit=1,drive
  =drive-ide0-1-1,id=ide0-1-1,bootindex=2 -chardev pty,id=charserial0
  -device isa-serial,chardev=charserial0,id=serial0 -chardev
  
socket,id=charchannel0,path=/var/lib/libvirt/qemu/win2008r2.agent,server,nowait
  -device virtserialport,bus=virtio-
  serial0.0,nr=1,chardev=charchannel0,id=channel0,name=org.qemu.guest_agent.0
  -device usb-tablet,id=input0 -vnc 0.0.0.0:0 -device
  VGA,id=video0,vgamem_mb=16,bus=pci.0,addr=0x2 -device virtio-balloon-
  pci,id=balloon0,bus=pci.0,addr=0xa -msg timestamp=on

  
  OVMF.fd is download from http://sourceforge.net/projects/edk2/files/OVMF/ 
OVMF-X64-r15214.zip.

  When I boot my domain with windows2008 iso, the kvm was caught in
  endless interrupt. I enable trace on my host and I got this.


  1. echo 1 > /sys/kernel/debug/tracing/events/kvm/enable
  2. cat /sys/kernel/debug/tracing/trace_pipe 
  qemu-system-x86-1969  [006]   2093.019588: kvm_exit: reason 
EXTERNAL_INTERRUPT rip 0xf8001080ae8e info 0 80fd
   qemu-system-x86-1969  [006] d...  2093.019590: kvm_entry: vcpu 0
   qemu-system-x86-1966  [017]   2093.021424: kvm_set_irq: gsi 8 level 1 
source 0
   qemu-system-x86-1966  [017]   2093.021429: kvm_pic_set_irq: chip 1 pin 0 
(edge|masked)
   qemu-system-x86-1966  [017]   2093.021430: kvm_ioapic_set_irq: pin 8 dst 
1 vec=209 (Fixed|logical|edge) (coalesced)
   qemu-system-x86-1969  [006]   2093.021683: kvm_exit: reason 
EXTERNAL_INTERRUPT rip 0xf8001080ae78 info 0 80fd
   qemu-system-x86-1969  [006] d...  2093.021686: kvm_entry: vcpu 0
   qemu-system-x86-1969  [006]   2093.022592: kvm_exit: reason 
EXTERNAL_INTERRUPT rip 0xf8001080ae8e info 0 

[Qemu-devel] [PATCH v2] rbd: make the code more readable

[Xiubo Li]

Make it a bit clearer and more readable.

Signed-off-by: Xiubo Li 
CC: John Snow 
---

V2:
- Advice from John Snow. Thanks.


 block/rbd.c | 25 -
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/block/rbd.c b/block/rbd.c
index 0a5840d..d0d4b39 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -366,45 +366,44 @@ static int qemu_rbd_create(const char *filename, QemuOpts 
*opts, Error **errp)
 rados_conf_read_file(cluster, NULL);
 } else if (conf[0] != '\0' &&
qemu_rbd_set_conf(cluster, conf, true, _err) < 0) {
-rados_shutdown(cluster);
 error_propagate(errp, local_err);
-return -EIO;
+ret = -EIO;
+goto shutdown;
 }
 
 if (conf[0] != '\0' &&
 qemu_rbd_set_conf(cluster, conf, false, _err) < 0) {
-rados_shutdown(cluster);
 error_propagate(errp, local_err);
-return -EIO;
+ret = -EIO;
+goto shutdown;
 }
 
 if (qemu_rbd_set_auth(cluster, secretid, errp) < 0) {
-rados_shutdown(cluster);
-return -EIO;
+ret = -EIO;
+goto shutdown;
 }
 
 ret = rados_connect(cluster);
 if (ret < 0) {
 error_setg_errno(errp, -ret, "error connecting");
-rados_shutdown(cluster);
-return ret;
+goto shutdown;
 }
 
 ret = rados_ioctx_create(cluster, pool, _ctx);
 if (ret < 0) {
 error_setg_errno(errp, -ret, "error opening pool %s", pool);
-rados_shutdown(cluster);
-return ret;
+goto shutdown;
 }
 
 ret = rbd_create(io_ctx, name, bytes, _order);
-rados_ioctx_destroy(io_ctx);
-rados_shutdown(cluster);
 if (ret < 0) {
 error_setg_errno(errp, -ret, "error rbd create");
-return ret;
 }
 
+rados_ioctx_destroy(io_ctx);
+
+shutdown:
+rados_shutdown(cluster);
 return ret;
 }
 
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 00/16] target-sparc improvements

[Mark Cave-Ayland]

On 12/10/16 02:42, Richard Henderson wrote:

> On 10/11/2016 04:42 PM, Mark Cave-Ayland wrote:
>> I'm fairly sure that I've tested an earlier version of this patchset,
>> however just to confirm is it just that you want a Tested-by from me of
>> this branch based upon the v6 atomics patch? If so I can run it against
>> all of my SPARC/SPARC64 test images over the next day or so.
> 
> I remember having posted bits and pieces that are in here, but not the
> whole thing at once.  Please do test wrt the atomics.

Hi Richard,

I found some time this morning to run your tgt-sparc-6 branch through my
OpenBIOS boot tests, and while SPARC32 seems to work fine, I noticed a
couple of regressions on SPARC64 as below:


$ ./qemu-system-sparc64 -cdrom debian-7.8.0-sparc-netinst.iso -nographic
-boot d
OpenBIOS for Sparc64
Configuration device id QEMU version 1 machine id 0
kernel cmdline
CPUs: 1 x SUNW,UltraSPARC-IIi
UUID: ----
Welcome to OpenBIOS v1.1 built on Sep 12 2016 07:11
  Type 'help' for detailed information
Trying cdrom:f...
Not a bootable ELF image
Loading a.out image...
Loaded 7680 bytes
entry point is 0x4000

Jumping to entry point 4000 for type 0005...
switching to new context: entry point 0x4000 stack 0xffe8dd31
SILO Version 1.4.14
EXT2 superblock magic is wrong
EXT2 superblock magic is wrong
\


  Welcome to Debian GNU/Linux wheezy!

This is a Debian installation CDROM, built on 20150110-20:41.
Keep it once you have installed your system, as you can boot from it
to repair the system on your hard disk if that ever becomes necessary.

WARNING: You should completely back up all of your hard disks before
  proceeding. The installation procedure can completely and irreversibly
  erase them! If you haven't made backups yet, remove the rescue CD from
  the drive and press L1-A to get back to the OpenBoot prompt.

Debian GNU/Linux comes with ABSOLUTELY NO WARRANTY, to the extent permitted
by applicable law.

[ ENTER - Boot install ]   [ Type "expert" - Boot into expert mode ]
   [ Type "rescue" - Boot into rescue mode ]
boot:
Allocated 64 Megs of memory at 0x4000 for kernel
EXT2 superblock magic is wrong
Loaded kernel version 3.2.65
EXT2 superblock magic is wrong
Loading initial ramdisk (5047556 bytes at 0x440 phys, 0x40C0
virt)...
-
[0.00] PROMLIB: Sun IEEE Boot Prom 'OBP 3.10.24 1999/01/01 01:01'
[0.00] PROMLIB: Root node compatible: sun4u
[0.00] Initializing cgroup subsys cpuset
[0.00] Initializing cgroup subsys cpu
[0.00] Linux version 3.2.0-4-sparc64
(debian-ker...@lists.debian.org) (gcc version 4.6.3 (Debian 4.6.3-14) )
#1 Debian 3.2.65-1
[0.00] bootconsole [earlyprom0] enabled
[0.00] ARCH: SUN4U
[0.00] Ethernet address: 52:54:00:12:34:56
[0.00] Kernel: Using 2 locked TLB entries for main kernel image.
[0.00] Remapping the kernel... done.
[0.00] OF stdout device is: /pci@1fe,0/ebus@3/su
[0.00] PROM: Built device tree with 33902 bytes of memory.
[0.00] Top of RAM: 0x7e8, Total RAM: 0x7e8
[0.00] Memory hole size: 0MB
[0.00] Zone PFN ranges:
[0.00]   Normal   0x -> 0x3f40
[0.00] Movable zone start PFN for each node
[0.00] early_node_map[1] active PFN ranges
[0.00] 0: 0x -> 0x3f40
[0.00] Booting Linux...
[0.00] CPU CAPS: [flush,stbar,swap,muldiv,v9,mul32,div32,v8plus]
[0.00] CPU CAPS: [vis]
[0.00] Built 1 zonelists in Zone order, mobility grouping on.
Total pages: 16065
[0.00] Kernel command line:
[0.00] PID hash table entries: 512 (order: -1, 4096 bytes)
[0.00] Dentry cache hash table entries: 16384 (order: 4, 131072
bytes)
[0.00] Inode-cache hash table entries: 8192 (order: 3, 65536 bytes)
[0.00] Memory: 110256k available (3440k kernel code, 1432k data,
192k init) [f800,07e8]
[0.00] NR_IRQS:255
[0.00] clocksource: mult[a00] shift[24]
[0.00] clockevent: mult[199a] shift[32]
[0.00] Console: colour dummy device 80x25
[0.00] console [tty0] enabled, bootconsole disabled
[0.00] PROMLIB: Sun IEEE Boot Prom 'OBP 3.10.24 1999/01/01 01:01'
[0.00] PROMLIB: Root node compatible: sun4u
[0.00] Initializing cgroup subsys cpuset
[0.00] Initializing cgroup subsys cpu
[0.00] Linux version 3.2.0-4-sparc64
(debian-ker...@lists.debian.org) (gcc version 4.6.3 (Debian 4.6.3-14) )
#1 Debian 3.2.65-1
[0.00] bootconsole [earlyprom0] enabled
[0.00] ARCH: SUN4U
[0.00] Ethernet address: 52:54:00:12:34:56
[0.00] Kernel: Using 2 locked TLB entries for main kernel image.
[0.00] Remapping the kernel... done.
[0.00] OF stdout device is: /pci@1fe,0/ebus@3/su
[0.00] PROM: Built device tree with 33902 bytes of 

Re: [Qemu-devel] [PATCH v11 01/19] block: Add bdrv_drain_all_{begin, end}()

[Paolo Bonzini]

On 14/10/2016 15:08, Alberto Garcia wrote:
> bdrv_drain_all() doesn't allow the caller to do anything after all
> pending requests have been completed but before block jobs are
> resumed.
> 
> This patch splits bdrv_drain_all() into _begin() and _end() for that
> purpose. It also adds aio_{disable,enable}_external() calls to disable
> external clients in the meantime.
> 
> Signed-off-by: Alberto Garcia 
> ---
>  block/io.c| 24 +---
>  include/block/block.h |  2 ++
>  2 files changed, 23 insertions(+), 3 deletions(-)
> 
> diff --git a/block/io.c b/block/io.c
> index b136c89..9418f8b 100644
> --- a/block/io.c
> +++ b/block/io.c
> @@ -275,8 +275,11 @@ void bdrv_drain(BlockDriverState *bs)
>   *
>   * This function does not flush data to disk, use bdrv_flush_all() for that
>   * after calling this function.
> + *
> + * This pauses all block jobs and disables external clients. It must
> + * be paired with bdrv_drain_all_end().
>   */
> -void bdrv_drain_all(void)
> +void bdrv_drain_all_begin(void)
>  {
>  /* Always run first iteration so any pending completion BHs run */
>  bool busy = true;
> @@ -300,6 +303,7 @@ void bdrv_drain_all(void)
>  bdrv_parent_drained_begin(bs);
>  bdrv_io_unplugged_begin(bs);
>  bdrv_drain_recurse(bs);
> +aio_disable_external(aio_context);
>  aio_context_release(aio_context);
>  
>  if (!g_slist_find(aio_ctxs, aio_context)) {
> @@ -333,17 +337,25 @@ void bdrv_drain_all(void)
>  }
>  }
>  
> +g_slist_free(aio_ctxs);
> +}
> +
> +void bdrv_drain_all_end(void)
> +{
> +BlockDriverState *bs;
> +BdrvNextIterator it;
> +BlockJob *job = NULL;
> +
>  for (bs = bdrv_first(); bs; bs = bdrv_next()) {
>  AioContext *aio_context = bdrv_get_aio_context(bs);
>  
>  aio_context_acquire(aio_context);
> +aio_enable_external(aio_context);
>  bdrv_io_unplugged_end(bs);
>  bdrv_parent_drained_end(bs);
>  aio_context_release(aio_context);
>  }
> -g_slist_free(aio_ctxs);
>  
> -job = NULL;
>  while ((job = block_job_next(job))) {
>  AioContext *aio_context = blk_get_aio_context(job->blk);
>  
> @@ -353,6 +365,12 @@ void bdrv_drain_all(void)
>  }
>  }
>  
> +void bdrv_drain_all(void)
> +{
> +bdrv_drain_all_begin();
> +bdrv_drain_all_end();
> +}
> +
>  /**
>   * Remove an active request from the tracked requests list
>   *
> diff --git a/include/block/block.h b/include/block/block.h
> index 107c603..301d713 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -338,6 +338,8 @@ int bdrv_flush_all(void);
>  void bdrv_close_all(void);
>  void bdrv_drain(BlockDriverState *bs);
>  void coroutine_fn bdrv_co_drain(BlockDriverState *bs);
> +void bdrv_drain_all_begin(void);
> +void bdrv_drain_all_end(void);
>  void bdrv_drain_all(void);
>  
>  int bdrv_pdiscard(BlockDriverState *bs, int64_t offset, int count);
> 

Reviewed-by: Paolo Bonzini 

Re: [Qemu-devel] [PATCH v11 02/19] block: Pause all jobs during bdrv_reopen_multiple()

[Paolo Bonzini]

On 14/10/2016 15:08, Alberto Garcia wrote:
> When a BlockDriverState is about to be reopened it can trigger certain
> operations that need to write to disk. During this process a different
> block job can be woken up. If that block job completes and also needs
> to call bdrv_reopen() it can happen that it needs to do it on the same
> BlockDriverState that is still in the process of being reopened.
> 
> This can have fatal consequences, like in this example:
> 
>   1) Block job A starts and sleeps after a while.
>   2) Block job B starts and tries to reopen node1 (a qcow2 file).
>   3) Reopening node1 means flushing and replacing its qcow2 cache.
>   4) While the qcow2 cache is being flushed, job A wakes up.
>   5) Job A completes and reopens node1, replacing its cache.
>   6) Job B resumes, but the cache that was being flushed no longer
>  exists.
> 
> This patch splits the bdrv_drain_all() call to keep all block jobs
> paused during bdrv_reopen_multiple(), so that step 4 can never happen
> and the operation is safe.
> 
> Note that this scenario can only happen if both bdrv_reopen() calls
> are made by block jobs on the same backing chain. Otherwise there's no
> chance that the same BlockDriverState appears in both reopen queues.
> 
> Signed-off-by: Alberto Garcia 
> ---
>  block.c | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/block.c b/block.c
> index 7f3e7bc..adbecd0 100644
> --- a/block.c
> +++ b/block.c
> @@ -2090,7 +2090,7 @@ int bdrv_reopen_multiple(BlockReopenQueue *bs_queue, 
> Error **errp)
>  
>  assert(bs_queue != NULL);
>  
> -bdrv_drain_all();
> +bdrv_drain_all_begin();
>  
>  QSIMPLEQ_FOREACH(bs_entry, bs_queue, entry) {
>  if (bdrv_reopen_prepare(_entry->state, bs_queue, _err)) {
> @@ -2120,6 +2120,9 @@ cleanup:
>  g_free(bs_entry);
>  }
>  g_free(bs_queue);
> +
> +bdrv_drain_all_end();
> +
>  return ret;
>  }
>  
> 

Reviewed-by: Paolo Bonzini