[Qemu-devel] [PATCH 3/5] target/s390x: change PSW_SHIFT_KEY

[Richard Henderson]

From: David Hildenbrand 

Such shifts are usually used to easily extract the PSW KEY from the PSW
mask, so let's avoid the confusing offset of 4.

Reviewed-by: Thomas Huth 
Signed-off-by: David Hildenbrand 
Message-Id: <20170614133819.18480-2-da...@redhat.com>
Signed-off-by: Richard Henderson 
---
 target/s390x/cpu.h   | 2 +-
 target/s390x/translate.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index a4028fb..532a4a0 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -315,7 +315,7 @@ void s390x_cpu_debug_excp_handler(CPUState *cs);
 #define PSW_MASK_IO 0x0200ULL
 #define PSW_MASK_EXT0x0100ULL
 #define PSW_MASK_KEY0x00F0ULL
-#define PSW_SHIFT_KEY   56
+#define PSW_SHIFT_KEY   52
 #define PSW_MASK_MCHECK 0x0004ULL
 #define PSW_MASK_WAIT   0x0002ULL
 #define PSW_MASK_PSTATE 0x0001ULL
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 48cee25..4773a10 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -3752,7 +3752,7 @@ static ExitStatus op_spka(DisasContext *s, DisasOps *o)
 {
 check_privileged(s);
 tcg_gen_shri_i64(o->in2, o->in2, 4);
-tcg_gen_deposit_i64(psw_mask, psw_mask, o->in2, PSW_SHIFT_KEY - 4, 4);
+tcg_gen_deposit_i64(psw_mask, psw_mask, o->in2, PSW_SHIFT_KEY, 4);
 return NO_EXIT;
 }
 
-- 
2.9.4

[Qemu-devel] [PATCH 1/5] target/s390x: Map existing FAC_* names to S390_FEAT_* names

[Richard Henderson]

The FAC_ names were placeholders prior to the introduction
of the current facility modeling.

Signed-off-by: Richard Henderson 
---
 target/s390x/translate.c | 59 
 1 file changed, 29 insertions(+), 30 deletions(-)

diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 8c055b7..af18ffb 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -1178,39 +1178,10 @@ typedef enum {
 EXIT_NORETURN,
 } ExitStatus;
 
-typedef enum DisasFacility {
-FAC_Z,  /* zarch (default) */
-FAC_CASS,   /* compare and swap and store */
-FAC_CASS2,  /* compare and swap and store 2*/
-FAC_DFP,/* decimal floating point */
-FAC_DFPR,   /* decimal floating point rounding */
-FAC_DO, /* distinct operands */
-FAC_EE, /* execute extensions */
-FAC_EI, /* extended immediate */
-FAC_FPE,/* floating point extension */
-FAC_FPSSH,  /* floating point support sign handling */
-FAC_FPRGR,  /* FPR-GR transfer */
-FAC_GIE,/* general instructions extension */
-FAC_HFP_MA, /* HFP multiply-and-add/subtract */
-FAC_HW, /* high-word */
-FAC_I_SIM,  /* IEEE exception sumilation */
-FAC_MIE,/* miscellaneous-instruction-extensions */
-FAC_LAT,/* load-and-trap */
-FAC_LOC,/* load/store on condition */
-FAC_LD, /* long displacement */
-FAC_PC, /* population count */
-FAC_SCF,/* store clock fast */
-FAC_SFLE,   /* store facility list extended */
-FAC_ILA,/* interlocked access facility 1 */
-FAC_LPP,/* load-program-parameter */
-FAC_DAT_ENH,/* DAT-enhancement */
-FAC_E2, /* extended-translation facility 2 */
-} DisasFacility;
-
 struct DisasInsn {
 unsigned opc:16;
 DisasFormat fmt:8;
-DisasFacility fac:8;
+unsigned fac:8;
 unsigned spec:8;
 
 const char *name;
@@ -5413,6 +5384,34 @@ enum DisasInsnEnum {
 #define SPEC_prep_0 0
 #define SPEC_wout_0 0
 
+/* Give smaller names to the various facilities.  */
+#define FAC_Z   S390_FEAT_ZARCH
+#define FAC_CASSS390_FEAT_COMPARE_AND_SWAP_AND_STORE
+#define FAC_CASS2   S390_FEAT_COMPARE_AND_SWAP_AND_STORE_2
+#define FAC_DFP S390_FEAT_DFP
+#define FAC_DFPRS390_FEAT_FLOATING_POINT_SUPPPORT_ENH /* DFP-rounding 
*/
+#define FAC_DO  S390_FEAT_STFLE_45 /* distinct-operands */
+#define FAC_EE  S390_FEAT_EXECUTE_EXT
+#define FAC_EI  S390_FEAT_EXTENDED_IMMEDIATE
+#define FAC_FPE S390_FEAT_FLOATING_POINT_EXT
+#define FAC_FPSSH   S390_FEAT_FLOATING_POINT_SUPPPORT_ENH /* 
FPS-sign-handling */
+#define FAC_FPRGR   S390_FEAT_FLOATING_POINT_SUPPPORT_ENH /* 
FPR-GR-transfer */
+#define FAC_GIE S390_FEAT_GENERAL_INSTRUCTIONS_EXT
+#define FAC_HFP_MA  S390_FEAT_HFP_MADDSUB
+#define FAC_HW  S390_FEAT_STFLE_45 /* high-word */
+#define FAC_I_SIM   S390_FEAT_FLOATING_POINT_SUPPPORT_ENH /* 
IEEE-exception-simulation */
+#define FAC_MIE S390_FEAT_STFLE_49 /* misc-instruction-extensions */
+#define FAC_LAT S390_FEAT_STFLE_49 /* load-and-trap */
+#define FAC_LOC S390_FEAT_STFLE_45 /* load/store on condition 1 */
+#define FAC_LD  S390_FEAT_LONG_DISPLACEMENT
+#define FAC_PC  S390_FEAT_STFLE_45 /* population count */
+#define FAC_SCF S390_FEAT_STORE_CLOCK_FAST
+#define FAC_SFLES390_FEAT_STFLE
+#define FAC_ILA S390_FEAT_STFLE_45 /* interlocked-access-facility 1 */
+#define FAC_LPP S390_FEAT_SET_PROGRAM_PARAMETERS /* 
load-program-parameter */
+#define FAC_DAT_ENH S390_FEAT_DAT_ENH
+#define FAC_E2  S390_FEAT_EXTENDED_TRANSLATION_2
+
 static const DisasInsn insn_info[] = {
 #include "insn-data.def"
 };
-- 
2.9.4

[Qemu-devel] [PATCH 2/5] target/s390x: Enforce instruction features

[Richard Henderson]

Signed-off-by: Richard Henderson 
---
 target/s390x/translate.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index af18ffb..48cee25 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -55,6 +55,7 @@ typedef struct DisasFields DisasFields;
 
 struct DisasContext {
 struct TranslationBlock *tb;
+const unsigned long *features;
 const DisasInsn *insn;
 DisasFields *fields;
 uint64_t ex_value;
@@ -5600,6 +5601,12 @@ static ExitStatus translate_one(CPUS390XState *env, 
DisasContext *s)
 }
 #endif
 
+/* Check for insn feature enabled.  */
+if (!test_bit(insn->fac, s->features)) {
+gen_program_exception(s, PGM_OPERATION);
+return EXIT_NORETURN;
+}
+
 /* Check for insn specification exceptions.  */
 if (insn->spec) {
 int spec = insn->spec, excp = 0, r;
@@ -5726,6 +5733,7 @@ void gen_intermediate_code(CPUS390XState *env, struct 
TranslationBlock *tb)
 }
 
 dc.tb = tb;
+dc.features = cpu->model->features;
 dc.pc = pc_start;
 dc.cc_op = CC_OP_DYNAMIC;
 dc.ex_value = tb->cs_base;
-- 
2.9.4

[Qemu-devel] [PATCH 4/5] target/s390x: implement mvcos instruction

[Richard Henderson]

From: David Hildenbrand 

This adds support for the MOVE WITH OPTIONAL SPECIFICATIONS (MVCOS)
instruction. Allow to enable it for the qemu cpu model using

qemu-system-s390x ... -cpu qemu,mvcos=on ...

This allows to boot linux kernel that uses it for uacccess.

We are missing (as for most other part) low address protection checks,
PSW key / storage key checks and support for AR-mode.

We fake an ADDRESSING exception when called from problem state (which
seems to rely on PSW key checks to be in place) and if AR-mode is used.
user mode will always see a PRIVILEDGED exception.

This patch is based on an original patch by Miroslav Benes (thanks!).

Signed-off-by: David Hildenbrand 
Message-Id: <20170614133819.18480-3-da...@redhat.com>
Signed-off-by: Richard Henderson 
---
 target/s390x/cpu.h |  22 +-
 target/s390x/cpu_models.c  |   1 +
 target/s390x/helper.h  |   1 +
 target/s390x/insn-data.def |   2 +
 target/s390x/mem_helper.c  | 181 +
 target/s390x/translate.c   |   9 +++
 6 files changed, 201 insertions(+), 15 deletions(-)

diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 532a4a0..5b94ace 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -304,6 +304,7 @@ void s390x_cpu_debug_excp_handler(CPUState *cs);
 #undef PSW_MASK_WAIT
 #undef PSW_MASK_PSTATE
 #undef PSW_MASK_ASC
+#undef PSW_SHIFT_ASC
 #undef PSW_MASK_CC
 #undef PSW_MASK_PM
 #undef PSW_MASK_64
@@ -320,6 +321,7 @@ void s390x_cpu_debug_excp_handler(CPUState *cs);
 #define PSW_MASK_WAIT   0x0002ULL
 #define PSW_MASK_PSTATE 0x0001ULL
 #define PSW_MASK_ASC0xC000ULL
+#define PSW_SHIFT_ASC   46
 #define PSW_MASK_CC 0x3000ULL
 #define PSW_MASK_PM 0x0F00ULL
 #define PSW_MASK_64 0x0001ULL
@@ -336,6 +338,12 @@ void s390x_cpu_debug_excp_handler(CPUState *cs);
 #define PSW_ASC_SECONDARY   0x8000ULL
 #define PSW_ASC_HOME0xC000ULL
 
+/* the address space values shifted */
+#define AS_PRIMARY  0
+#define AS_ACCREG   1
+#define AS_SECONDARY2
+#define AS_HOME 3
+
 /* tb flags */
 
 #define FLAG_MASK_PER   (PSW_MASK_PER>> 32)
@@ -354,6 +362,7 @@ void s390x_cpu_debug_excp_handler(CPUState *cs);
 
 /* Control register 0 bits */
 #define CR0_LOWPROT 0x1000ULL
+#define CR0_SECONDARY   0x0400ULL
 #define CR0_EDAT0x0080ULL
 
 /* MMU */
@@ -361,7 +370,18 @@ void s390x_cpu_debug_excp_handler(CPUState *cs);
 #define MMU_SECONDARY_IDX   1
 #define MMU_HOME_IDX2
 
-static inline int cpu_mmu_index (CPUS390XState *env, bool ifetch)
+static inline bool psw_key_valid(CPUS390XState *env, uint8_t psw_key)
+{
+uint16_t pkm = env->cregs[3] >> 16;
+
+if (env->psw.mask & PSW_MASK_PSTATE) {
+/* PSW key has range 0..15, it is valid if the bit is 1 in the PKM */
+return pkm & (0x80 >> psw_key);
+}
+return true;
+}
+
+static inline int cpu_mmu_index(CPUS390XState *env, bool ifetch)
 {
 switch (env->psw.mask & PSW_MASK_ASC) {
 case PSW_ASC_PRIMARY:
diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
index 478bcc6..c3a4ce6 100644
--- a/target/s390x/cpu_models.c
+++ b/target/s390x/cpu_models.c
@@ -682,6 +682,7 @@ static void add_qemu_cpu_model_features(S390FeatBitmap fbm)
 S390_FEAT_LONG_DISPLACEMENT_FAST,
 S390_FEAT_ETF2_ENH,
 S390_FEAT_STORE_CLOCK_FAST,
+S390_FEAT_MOVE_WITH_OPTIONAL_SPEC,
 S390_FEAT_GENERAL_INSTRUCTIONS_EXT,
 S390_FEAT_EXECUTE_EXT,
 S390_FEAT_STFLE_45,
diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index 69249a5..b268367 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -105,6 +105,7 @@ DEF_HELPER_FLAGS_1(stfl, TCG_CALL_NO_RWG, void, env)
 DEF_HELPER_2(stfle, i32, env, i64)
 DEF_HELPER_FLAGS_2(lpq, TCG_CALL_NO_WG, i64, env, i64)
 DEF_HELPER_FLAGS_4(stpq, TCG_CALL_NO_WG, void, env, i64, i64, i64)
+DEF_HELPER_4(mvcos, i32, env, i64, i64, i64)
 
 #ifndef CONFIG_USER_ONLY
 DEF_HELPER_3(servc, i32, env, i64, i64)
diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index d089707..aa4c5b2 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -590,6 +590,8 @@
 C(0xb254, MVPG,RRE,   Z,   r1_o, r2_o, 0, 0, mvpg, 0)
 /* MOVE STRING */
 C(0xb255, MVST,RRE,   Z,   r1_o, r2_o, 0, 0, mvst, 0)
+/* MOVE WITH OPTIONAL SPECIFICATION */
+C(0xc800, MVCOS,   SSF,   MVCOS, la1, a2, 0, 0, mvcos, 0)
 /* MOVE WITH OFFSET */
 /* Really format SS_b, but we pack both lengths into one argument
for the helper call, so we might as well leave one 8-bit field.  */
diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 

[Qemu-devel] [PATCH 0/5] More s390x improvements

[Richard Henderson]

David, in his first mvcos patch, points out that we're not enforcing
the facilties on translation.  This takes care of that.  I also went
through and see that we fully implement 3 other facilities bits.


r~


David Hildenbrand (2):
  target/s390x: change PSW_SHIFT_KEY
  target/s390x: implement mvcos instruction

Richard Henderson (3):
  target/s390x: Map existing FAC_* names to S390_FEAT_* names
  target/s390x: Enforce instruction features
  target/s390x: mark CSST, CSST2, FPSEH facilities as available

 target/s390x/cpu.h |  24 +-
 target/s390x/cpu_models.c  |   4 +
 target/s390x/helper.h  |   1 +
 target/s390x/insn-data.def |   2 +
 target/s390x/mem_helper.c  | 181 +
 target/s390x/translate.c   |  78 +++
 6 files changed, 243 insertions(+), 47 deletions(-)

-- 
2.9.4

[Qemu-devel] [PATCH 5/5] target/s390x: mark CSST, CSST2, FPSEH facilities as available

[Richard Henderson]

Signed-off-by: Richard Henderson 
---
 target/s390x/cpu_models.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
index c3a4ce6..703feca 100644
--- a/target/s390x/cpu_models.c
+++ b/target/s390x/cpu_models.c
@@ -683,8 +683,11 @@ static void add_qemu_cpu_model_features(S390FeatBitmap fbm)
 S390_FEAT_ETF2_ENH,
 S390_FEAT_STORE_CLOCK_FAST,
 S390_FEAT_MOVE_WITH_OPTIONAL_SPEC,
+S390_FEAT_COMPARE_AND_SWAP_AND_STORE,
+S390_FEAT_COMPARE_AND_SWAP_AND_STORE_2,
 S390_FEAT_GENERAL_INSTRUCTIONS_EXT,
 S390_FEAT_EXECUTE_EXT,
+S390_FEAT_FLOATING_POINT_SUPPPORT_ENH,
 S390_FEAT_STFLE_45,
 };
 int i;
-- 
2.9.4

[Qemu-devel] [FIX PATCH] target/ppc: Proper cleanup when ppc_cpu_realizefn fails

[Bharata B Rao]

If ppc_cpu_realizefn() fails after cpu_exec_realizefn() has been
called, we will have to undo whatever cpu_exec_realizefn() did
by explicitly calling cpu_exec_unrealizeffn() which is currently
missing. Failure to do this proper cleanup will result in CPU
which was never fully realized to linger on the cpus list causing
SIGSEGV later (for eg when running "info cpus").

Signed-off-by: Bharata B Rao 
---
 target/ppc/translate_init.c | 12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/target/ppc/translate_init.c b/target/ppc/translate_init.c
index e837cd2..53aff5a 100644
--- a/target/ppc/translate_init.c
+++ b/target/ppc/translate_init.c
@@ -9825,14 +9825,14 @@ static void ppc_cpu_realizefn(DeviceState *dev, Error 
**errp)
 error_append_hint(errp, "Adjust the number of cpus to %d "
   "or try to raise the number of threads per core\n",
   cpu->cpu_dt_id * smp_threads / max_smt);
-return;
+goto unrealize;
 }
 #endif
 
 if (tcg_enabled()) {
 if (ppc_fixup_cpu(cpu) != 0) {
 error_setg(errp, "Unable to emulate selected CPU with TCG");
-return;
+goto unrealize;
 }
 }
 
@@ -9841,14 +9841,14 @@ static void ppc_cpu_realizefn(DeviceState *dev, Error 
**errp)
 error_setg(errp, "CPU does not possess a BookE or 4xx MMU. "
"Please use qemu-system-ppc or qemu-system-ppc64 instead "
"or choose another CPU model.");
-return;
+goto unrealize;
 }
 #endif
 
 create_ppc_opcodes(cpu, _err);
 if (local_err != NULL) {
 error_propagate(errp, local_err);
-return;
+goto unrealize;
 }
 init_ppc_proc(cpu);
 
@@ -10033,6 +10033,10 @@ static void ppc_cpu_realizefn(DeviceState *dev, Error 
**errp)
 fflush(stdout);
 }
 #endif
+return;
+
+unrealize:
+cpu_exec_unrealizefn(cs);
 }
 
 static void ppc_cpu_unrealizefn(DeviceState *dev, Error **errp)
-- 
2.7.4

Re: [Qemu-devel] [PATCH] target/aarch64: exit to main loop after 'msr daifclr'

[Emilio G. Cota]

On Wed, Jun 14, 2017 at 18:20:29 -0700, Richard Henderson wrote:
> On 06/14/2017 01:33 PM, Emilio G. Cota wrote:
> >On Wed, Jun 14, 2017 at 12:48:21 -0700, Richard Henderson wrote:
> >>Exit to cpu loop so we reevaluate cpu_arm_hw_interrupts.
> >>
> >>Cc: qemu-...@nongnu.org
> >>Cc: Peter Maydell 
> >>Signed-off-by: Richard Henderson 
> >>---
> >>  target/arm/translate-a64.c | 7 ++-
> >>  1 file changed, 6 insertions(+), 1 deletion(-)
> >>
> >>diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
> >>index 860e279..e55547d 100644
> >>--- a/target/arm/translate-a64.c
> >>+++ b/target/arm/translate-a64.c
> >>@@ -1422,7 +1422,9 @@ static void handle_msr_i(DisasContext *s, uint32_t 
> >>insn,
> >>  gen_helper_msr_i_pstate(cpu_env, tcg_op, tcg_imm);
> >>  tcg_temp_free_i32(tcg_imm);
> >>  tcg_temp_free_i32(tcg_op);
> >>-s->is_jmp = DISAS_UPDATE;
> >>+/* For DAIFClear, exit the cpu loop to re-evaluate pending IRQs.  
> >>*/
> >>+gen_a64_set_pc_im(s->pc);
> >
> >For op != 0x1f we end up setting the pc twice (first here, then in
> >the switch statement). It's still correct though.
> 
> No, that's why I switched to DISAS_JUMP.
> 
(snip)
> >+case DISAS_EXIT:
> >+gen_a64_set_pc_im(dc->pc);
> >+tcg_gen_exit_tb(0);
> >+break;
> 
> This gives translate-a64.c and translate.c different semantics for
> DISAS_EXIT. I considered that to be a bad thing.

Agreed with the above two. Sorry I missed this in my first read
of the patch, it seems that my writing of my version of this patch
impaired my ability to review another version :-)

Thanks for the clarifications!

Reviewed-by: Emilio G. Cota 
Tested-by: Emilio G. Cota 

E.

[Qemu-devel] [PATCH v2] vhost-user: support cross-endianess negatiation

[Felipe Franciosi]

Currently, vhost-user does not implement any means for notifying the
backend about guest endianess. This commit introduces a new message
called VHOST_USER_SET_VRING_ENDIAN which is analogous to the ioctl()
called VHOST_SET_VRING_ENDIAN used for kernel vhost backends. Such
message is necessary for backends supporting legacy (pre-1.0) virtio
devices running in big-endian guests.

Signed-off-by: Felipe Franciosi 
Signed-off-by: Mike Cui 
---
 v1->v2:
 * Elaborated message usage on the documentation.
 * Fixed Id typo (22->23) on the documentation.

 docs/specs/vhost-user.txt | 16 
 hw/virtio/vhost-user.c| 23 +--
 2 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
index 481ab56..954771d 100644
--- a/docs/specs/vhost-user.txt
+++ b/docs/specs/vhost-user.txt
@@ -326,6 +326,7 @@ Protocol features
 #define VHOST_USER_PROTOCOL_F_REPLY_ACK  3
 #define VHOST_USER_PROTOCOL_F_MTU4
 #define VHOST_USER_PROTOCOL_F_SLAVE_REQ  5
+#define VHOST_USER_PROTOCOL_F_CROSS_ENDIAN   6
 
 Master message types
 
@@ -580,6 +581,21 @@ Master message types
   This request should be send only when VIRTIO_F_IOMMU_PLATFORM feature
   has been successfully negotiated.
 
+ * VHOST_USER_SET_VRING_ENDIAN
+
+  Id: 23
+  Equivalent ioctl: VHOST_SET_VRING_ENDIAN
+  Master payload: vring state description
+
+  Set the endianess of a VQ for legacy devices. Little-endian is indicated
+  with state.num set to 0 and big-endian is indicated with state.num set
+  to 1. Other values are invalid.
+  This request should be sent only when VHOST_USER_PROTOCOL_F_CROSS_ENDIAN
+  has been negotiated.
+  Backends that negotiated this feature should handle both endianesses
+  and expect this message once (per VQ) during device configuration
+  (ie. before the master starts the VQ).
+
 Slave message types
 ---
 
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 958ee09..006af1c 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -33,6 +33,7 @@ enum VhostUserProtocolFeature {
 VHOST_USER_PROTOCOL_F_REPLY_ACK = 3,
 VHOST_USER_PROTOCOL_F_NET_MTU = 4,
 VHOST_USER_PROTOCOL_F_SLAVE_REQ = 5,
+VHOST_USER_PROTOCOL_F_CROSS_ENDIAN = 6,
 
 VHOST_USER_PROTOCOL_F_MAX
 };
@@ -63,6 +64,7 @@ typedef enum VhostUserRequest {
 VHOST_USER_NET_SET_MTU = 20,
 VHOST_USER_SET_SLAVE_REQ_FD = 21,
 VHOST_USER_IOTLB_MSG = 22,
+VHOST_USER_SET_VRING_ENDIAN = 23,
 VHOST_USER_MAX
 } VhostUserRequest;
 
@@ -367,8 +369,25 @@ static int vhost_user_set_vring_addr(struct vhost_dev *dev,
 static int vhost_user_set_vring_endian(struct vhost_dev *dev,
struct vhost_vring_state *ring)
 {
-error_report("vhost-user trying to send unhandled ioctl");
-return -1;
+bool cross_endian = virtio_has_feature(dev->protocol_features,
+   VHOST_USER_PROTOCOL_F_CROSS_ENDIAN);
+VhostUserMsg msg = {
+.request = VHOST_USER_SET_VRING_ENDIAN,
+.flags = VHOST_USER_VERSION,
+.payload.state = *ring,
+.size = sizeof(msg.payload.state),
+};
+
+if (!cross_endian) {
+error_report("vhost-user trying to send unhandled ioctl");
+return -1;
+}
+
+if (vhost_user_write(dev, , NULL, 0) < 0) {
+return -1;
+}
+
+return 0;
 }
 
 static int vhost_set_vring(struct vhost_dev *dev,
-- 
1.9.5

Re: [Qemu-devel] [PATCH V6 02/10] net/filter-mirror.c: Make filter mirror support vnet support.

[Jason Wang]

On 2017年06月14日 16:04, Zhang Chen wrote:



On 06/13/2017 05:14 PM, Jason Wang wrote:



On 2017年06月12日 17:27, Zhang Chen wrote:



+if (nf->direction == NET_FILTER_DIRECTION_RX ||
+nf->direction == NET_FILTER_DIRECTION_ALL) {
+vnet_hdr_len = nf->netdev->vnet_hdr_len;


This can only work if e.g virtio-net set its own vnet_hdr_len. But 
looks like it use guest_hdr_len instead.


I see in hw/net/virtio-net.c use the 
"qemu_set_vnet_hdr_len(nc->peer, n->guest_hdr_len);" to
set the nf->netdev->vnet_hdr_len, any case not include here? 


You mean:

if (peer_has_vnet_hdr(n) &&
qemu_has_vnet_hdr_len(nc->peer, n->guest_hdr_len)) {
qemu_set_vnet_hdr_len(nc->peer, n->guest_hdr_len);
n->host_hdr_len = n->guest_hdr_len;
}

?

From the code, it only set peer's vnet header when peer supports 
vnet_hdr up to n->guest_hdr_len. If peer can't, virtio-net will strip 
the header.


So, We should get the n->guest_hdr_len in another way? like set a new 
parameter in NetClientState?

Any suggestion about it?

Thanks
Zhang Chen


Rethink about this, a question is do we really care guest vnet header 
len? During guest transmission, when packet reaches netfilter. The 
vnet_header should have been stripped to netdev->vnet_hdr_len.


I still think use nf->netdev->vnet_hdr_len is sufficient.

Thanks





Thanks

Re: [Qemu-devel] [PULL 0/2] Block patches

[Jeff Cody]

On Wed, Jun 14, 2017 at 03:26:52PM -0700, no-re...@patchew.org wrote:
> Hi,
> 
> This series failed automatic build test. Please find the testing commands and
> their output below. If you have docker installed, you can probably reproduce 
> it
> locally.
> 
> Type: series
> Subject: [Qemu-devel] [PULL 0/2] Block patches
> Message-id: 20170614215526.9218-1-jc...@redhat.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=8
> # master is broken, skip centos6...
> #time make docker-test-quick@centos6
> time make docker-test-mingw@fedora
> time make docker-test-build@min-glib
> === TEST SCRIPT END ===
> 
> Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
> From https://github.com/patchew-project/qemu
>  * [new tag] patchew/20170614215526.9218-1-jc...@redhat.com -> 
> patchew/20170614215526.9218-1-jc...@redhat.com
> Switched to a new branch 'test'
> 2f0938b block/iscsi: enable filename option and parsing
> e6b28ad block/rbd: enable filename option and parsing
>

[...]

>   CC  hw/pci-bridge/pcie_root_port.o
>   CC  hw/pci-bridge/gen_pcie_root_port.o
>   CC  hw/pci-bridge/pci_expander_bridge.o
> In file included from /tmp/qemu-test/src/hw/net/vmxnet3.c:30:
> /tmp/qemu-test/src/include/migration/register.h:18: error: redefinition of 
> typedef ‘LoadStateHandler’
> /tmp/qemu-test/src/include/migration/vmstate.h:32: note: previous declaration 
> of ‘LoadStateHandler’ was here

Just an FYI, this appears to be unrelated to the actual patches in the pull
req.

> make: *** [hw/net/vmxnet3.o] Error 1
> make: *** Waiting for unfinished jobs
> tests/docker/Makefile.include:118: recipe for target 'docker-run' failed
> make[1]: *** [docker-run] Error 2
> make[1]: Leaving directory '/var/tmp/patchew-tester-tmp-aceb4nxr/src'
> tests/docker/Makefile.include:149: recipe for target 
> 'docker-run-test-build@min-glib' failed
> make: *** [docker-run-test-build@min-glib] Error 2
> === OUTPUT END ===
> 
> Test command exited with code: 2
> 
> 
> ---
> Email generated automatically by Patchew [http://patchew.org/].
> Please send your feedback to patchew-de...@freelists.org

Re: [Qemu-devel] 答复: Re: 答复: Re: [PATCHv2 02/04] colo-compare: Process pactkets in the IOThread ofthe primary

[Jason Wang]

On 2017年06月13日 19:24, wang.yong...@zte.com.cn wrote:


>>Char-fe.c for sure which means frontend of chardev.


>>> These API can only watch events in the qemu main thread, not in the

>>> IOThread.

>>>

>>> I had to use the qio_channel_socket_set_aio_fd_handler function to

>>>

>>> monitor the char event in the IOThread,so the io channel is used her

>>>


>>The point is not touching the internal structure of chardev like ioc,

>>instead extend its helper like e.g qemu_chr_fe_set_handlers() and let it

>>set aio handlers,


>Currently character devices are tied to the GSource API. However,I'll 
try to submit a patch first.



Hi Jason,


I have investigated the change, which involves a great deal.

We have to convert the user of those APIs (the external API is

qemu_chr_fe_add_watch) from GSource/QIOChannel to AioContext.


Can we join this series first?  and replace GSource with AioContext in 
a future patch.





Touching internal chardev member out of its scope is bad, we need to 
seek a solution for this.


Paolo, Marc and Stefan:

We want let chardev front-end run in colo comparing IOThread. This looks 
not supported by current chardev frontend API. Any idea/suggestion on 
how to achieve this?


Thanks

Re: [Qemu-devel] [virtio-dev] Re: [virtio-dev] Re: [PATCH v1] virtio-net: enable configurable tx queue size

[Jason Wang]

On 2017年06月14日 23:22, Michael S. Tsirkin wrote:

On Wed, Jun 14, 2017 at 07:26:54PM +0800, Jason Wang wrote:


On 2017年06月13日 18:46, Jason Wang wrote:


On 2017年06月13日 17:50, Wei Wang wrote:

On 06/13/2017 05:04 PM, Jason Wang wrote:


On 2017年06月13日 15:17, Wei Wang wrote:

On 06/13/2017 02:29 PM, Jason Wang wrote:

The issue is what if there's a mismatch of max #sgs between qemu and

When the vhost backend is used, QEMU is not
involved in the data path.
The vhost backend
directly gets what is offered by the guest from the vq. Why would
there be a mismatch of
max #sgs between QEMU and vhost, and what is
the QEMU side max #sgs
used for? Thanks.

You need query the backend max #sgs in this case
at least. no? If not
how do you know the value is supported by the backend?

Thanks


Here is my thought: vhost backend has already been
supporting 1024 sgs,
so I think it might not be necessary to query the
max sgs that the vhost
backend supports. In the setup phase, when QEMU
detects the backend is
vhost, it assumes 1024 max sgs is supported, instead
of giving an extra
call to query.

We can probably assume vhost kernel supports up to 1024
sgs. But how about for other vhost-user backends?


So far, I haven't seen any vhost backend implementation
supporting less than 1024 sgs.

Since vhost-user is an open protocol we can not check each
implementation (some may be even close sourced). For safety, we
need an explicit clarification on this.




And what you said here makes me ask one of my questions in the past:

Do we have plan to extend 1024 to a larger value or 1024
looks good for the future years? If we only care about
1024, there's even no need for a new config filed, a
feature flag is more than enough. If we want to extend
it to e.g 2048, we definitely need to query vhost
backend's limit (even for vhost-kernel).


According to virtio spec (e.g. 2.4.4), unreasonably large
descriptors are
not encouraged to be used by the guest. If possible, I would
suggest to use
1024 as the largest number of descriptors that the guest can
chain, even when
we have larger queue size in the future. That is,
if (backend == QEMU backend)
 config.max_chain_size = 1023 (defined by the qemu
backend implementation);
else if (backend == vhost)
 config.max_chain_size = 1024;

It is transparent to the guest. From the guest's point of
view, all it knows is a value
given to him via reading config.max_chain_size.

So not transparent actually, guest at least guest need to see
and check for this. So the question still, since you only care
about two cases in fact:

- backend supports 1024
- backend supports <1024 (qemu or whatever other backends)

So it looks like a new feature flag is more than enough. If
device(backends) support this feature, it can make sure 1024 sgs
is supported?


That wouldn't be enough. For example, QEMU3.0 backend supports
max_chain_size=1023,
while QEMU4.0 backend supports max_chain_size=1021. How would the
guest know
the max size with the same feature flag? Would it still chain 1023
descriptors with QEMU4.0?

Best,
Wei

I believe we won't go back to less than 1024 in the future. It may be
worth to add a unittest for this to catch regression early.

Thanks

I think I disagree with that. Smaller pipes a better (e.g. less cache
pressure) and you only need huge pipes because host thread gets
scheduled out for too long. With more CPUs there's less of a chance of
an overcommit so we'll be able to get by with smaller pipes in the
future.


Agree, but we are talking about the upper limit. Even if 1024 is 
supported, small number of #sgs is still encouraged.





Consider the queue size is 256 now, I think maybe we can first make tx queue
size configurable up to 1024, and then do the #sg stuffs on top.

What's your opinion, Michael?

Thanks

With a kernel backend, 1024 is problematic since we are then unable
to add any entries or handle cases where an entry crosses an MR region
boundary. We could support up to 512 with a kernel backend but no one
seems to want that :)


Then I see issues with indirect descriptors.

We try to allow up 1024 chained descriptors implicitly since 
e0e9b406470b ("vhost: max s/g to match qemu"). If guest can submit 
crossing MR descs, I'm afraid we've already had this bug since this 
commit. And actually this seems conflict to what spec said in 2.4.5:


"""
The number of descriptors in the table is defined by the queue size for 
this virtqueue: this is the maximum possible descriptor chain length.

"""

Technically, we had the same issue for rx since we allow 1024 queue size 
now.


So actually, allowing the size to 1024 does not introduce any new trouble?


With vhost-user the backend might be able to handle that. So an
acceptable option would be to allow 1K with vhost-user backends
only, trim it back with other backends.



I believe the idea is to clarify the maximum chain size instead of 
having any assumption.


Thanks

Re: [Qemu-devel] [PATCH] fix: avoid infinite loop when blockjob encountering failure

[sochin.jiang]

I realized blockjob is freed after completed unless we call block_job_ref()

before run_block_job is called.


On 2017/6/15 10:38, sochin.jiang wrote:
> Thanks for your kindly reply.
>
> I do have made a mistake that ignoring the AIOContext lock.
>
> About the patch, firstly, if job->ret comes to be non-zero(also means 
> job->completed to be true) , blockjob 'callback'(common_block_job_cb) will be 
> called, blockjob error will be put into errp. It won't report success.
>
> Secondly, blockjob fails with 'ret < 0' and without calling 
> block_job_complete_sync(), we won't have segfault because bdrv_reopen won't 
> be called. Also, with the use-after-free problems.
>
> So, skip the block_job_complete_sync() call if job->completed(job->ret to be 
> non-zero) is true can avoid all the problems, am I right ?
>
> Thank you again.
>
>
> Best Regard.
>
> Sochin
>
>
>
>
>
>
>
> On 2017/6/14 21:12, Max Reitz wrote:
>> Thanks for your patch! The issue can be reproduced as follows:
>>
>> $ qemu-img create -f qcow2 -b \
>> "json:{'driver':'raw','file':{
>> 'driver':'blkdebug','inject-error':[{'event':'write_aio'}],
>> 'image':{'driver':'null-co'}}}" \
>>  overlay.qcow2
>> $ qemu-io -c 'write 0 64k' overlay.qcow2
>> $ qemu-img commit overlay.qcow2
>>
>> While your patch fixes that issue, I still have some comments:
>>
>> On 2017-06-14 08:22, sochin.jiang wrote:
>>> From: "sochin.jiang" 
>>>
>>> img_commit could fall into infinite loop if it's blockjob
>> This should be "into an infinite loop" and "its" instead if "it's".
>>
>> This empty line should be omitted.
>>
>>> fail encountering any I/O error. Try to fix it.
>> Should be "fails on any I/O error" or "fails on encountering any I/O
>> error". Also, you're not trying to fix it but let's all hope you really
>> are fixing it. :-)
>>
>> (So "Fix it." instead of "Try to fix it.")
>>
>>> Signed-off-by: sochin.jiang 
>>> ---
>>>  qemu-img.c | 5 -
>>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/qemu-img.c b/qemu-img.c
>>> index 0ad698d..6ba565d 100644
>>> --- a/qemu-img.c
>>> +++ b/qemu-img.c
>>> @@ -895,8 +895,11 @@ static void run_block_job(BlockJob *job, Error **errp)
>>>  aio_poll(aio_context, true);
>>>  qemu_progress_print(job->len ?
>>>  ((float)job->offset / job->len * 100.f) : 
>>> 0.0f, 0);
>>> -} while (!job->ready);
>>> +} while (!job->ready && !job->ret);
>> I think it would be better to test job->completed instead of job->ret.
>>
>>>  
>>> +if (job->ret) {
>>> +return;
>>> +}
>> We shouldn't just return here but still do all the deinitialization like
>> call aio_context_release(). I guess the best would be to just skip the
>> block_job_complete_sync() call if job->completed is true.
>>
>>>  block_job_complete_sync(job, errp);
>>>  aio_context_release(aio_context);
>> Then, there are three more issues I found while reviewing this patch:
>>
>> First, if the block job is completed before block_job_complete_sync() is
>> called (i.e. if an error occurred), it is automatically freed. This is
>> bad because this means we'll have some instances of use-after-free here.
>> Therefore, we need to invoke block_job_ref() before run_block_job() and
>> block_job_unref() afterwards. (And since these functions are currenctly
>> static in blockjob.c, we'll have to make them global.)
>>
>> Secondly, run_block_job() doesn't evaluate job->ret. Therefore it will
>> report success even if the commit failed (it is expecting
>> block_job_complete_sync() to put an error into errp, but it will not do
>> that). So we'll have to do that (manually check job->ret and if it's
>> negative, put an error message into errp; also, assert that
>> job->cancelled is false).
>>
>> Thirdly, we have segfault in bdrv_reopen_prepare() if the image has
>> non-string options... I'll handle this one.
>>
>> I can also handle the other two issues, if you'd like me to.
>>
>>
>> Finally, an iotest would be nice (see my reproducer above). But I can
>> handle that as well, if you decide not to write one.
>>
>> Max
>>

Re: [Qemu-devel] [PATCH] util: remove the obsolete non-blocking connect

[no-reply]

Hi,

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

Type: series
Subject: [Qemu-devel] [PATCH] util: remove the obsolete non-blocking connect
Message-id: 20170615030801.6260-1-maozy.f...@cn.fujitsu.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=8
# master is broken, skip centos6...
#time make docker-test-quick@centos6
time make docker-test-mingw@fedora
time make docker-test-build@min-glib
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag] patchew/20170615030801.6260-1-maozy.f...@cn.fujitsu.com -> 
patchew/20170615030801.6260-1-maozy.f...@cn.fujitsu.com
Switched to a new branch 'test'
1498946 util: remove the obsolete non-blocking connect

=== OUTPUT BEGIN ===
Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
Cloning into '/var/tmp/patchew-tester-tmp-j3x1ptzn/src/dtc'...
Submodule path 'dtc': checked out '558cd81bdd432769b59bff01240c44f82cfb1a9d'
  BUILD   fedora
make[1]: Entering directory '/var/tmp/patchew-tester-tmp-j3x1ptzn/src'
  ARCHIVE qemu.tgz
  ARCHIVE dtc.tgz
  COPYRUNNER
RUN test-mingw in qemu:fedora 
Packages installed:
PyYAML-3.11-13.fc25.x86_64
SDL-devel-1.2.15-21.fc24.x86_64
bc-1.06.95-16.fc24.x86_64
bison-3.0.4-4.fc24.x86_64
ccache-3.3.4-1.fc25.x86_64
clang-3.9.1-2.fc25.x86_64
findutils-4.6.0-8.fc25.x86_64
flex-2.6.0-3.fc25.x86_64
gcc-6.3.1-1.fc25.x86_64
gcc-c++-6.3.1-1.fc25.x86_64
git-2.9.3-2.fc25.x86_64
glib2-devel-2.50.3-1.fc25.x86_64
libfdt-devel-1.4.2-1.fc25.x86_64
make-4.1-5.fc24.x86_64
mingw32-SDL-1.2.15-7.fc24.noarch
mingw32-bzip2-1.0.6-7.fc24.noarch
mingw32-curl-7.47.0-1.fc24.noarch
mingw32-glib2-2.50.1-1.fc25.noarch
mingw32-gmp-6.1.1-1.fc25.noarch
mingw32-gnutls-3.5.5-2.fc25.noarch
mingw32-gtk2-2.24.31-2.fc25.noarch
mingw32-gtk3-3.22.2-1.fc25.noarch
mingw32-libjpeg-turbo-1.5.1-1.fc25.noarch
mingw32-libpng-1.6.27-1.fc25.noarch
mingw32-libssh2-1.4.3-5.fc24.noarch
mingw32-libtasn1-4.9-1.fc25.noarch
mingw32-nettle-3.3-1.fc25.noarch
mingw32-pixman-0.34.0-1.fc25.noarch
mingw32-pkg-config-0.28-6.fc24.x86_64
mingw64-SDL-1.2.15-7.fc24.noarch
mingw64-bzip2-1.0.6-7.fc24.noarch
mingw64-curl-7.47.0-1.fc24.noarch
mingw64-glib2-2.50.1-1.fc25.noarch
mingw64-gmp-6.1.1-1.fc25.noarch
mingw64-gnutls-3.5.5-2.fc25.noarch
mingw64-gtk2-2.24.31-2.fc25.noarch
mingw64-gtk3-3.22.2-1.fc25.noarch
mingw64-libjpeg-turbo-1.5.1-1.fc25.noarch
mingw64-libpng-1.6.27-1.fc25.noarch
mingw64-libssh2-1.4.3-5.fc24.noarch
mingw64-libtasn1-4.9-1.fc25.noarch
mingw64-nettle-3.3-1.fc25.noarch
mingw64-pixman-0.34.0-1.fc25.noarch
mingw64-pkg-config-0.28-6.fc24.x86_64
package python2 is not installed
perl-5.24.1-385.fc25.x86_64
pixman-devel-0.34.0-2.fc24.x86_64
sparse-0.5.0-10.fc25.x86_64
tar-1.29-3.fc25.x86_64
which-2.21-1.fc25.x86_64
zlib-devel-1.2.8-10.fc24.x86_64

Environment variables:
FBR=f25
PACKAGES=ccache git tar PyYAML sparse flex bison python2 glib2-devel 
pixman-devel zlib-devel SDL-devel libfdt-devel gcc gcc-c++ clang make perl 
which bc findutils mingw32-pixman mingw32-glib2 mingw32-gmp mingw32-SDL 
mingw32-pkg-config mingw32-gtk2 mingw32-gtk3 mingw32-gnutls mingw32-nettle 
mingw32-libtasn1 mingw32-libjpeg-turbo mingw32-libpng mingw32-curl 
mingw32-libssh2 mingw32-bzip2 mingw64-pixman mingw64-glib2 mingw64-gmp 
mingw64-SDL mingw64-pkg-config mingw64-gtk2 mingw64-gtk3 mingw64-gnutls 
mingw64-nettle mingw64-libtasn1 mingw64-libjpeg-turbo mingw64-libpng 
mingw64-curl mingw64-libssh2 mingw64-bzip2
HOSTNAME=
TERM=xterm
MAKEFLAGS= -j8
HISTSIZE=1000
J=8
USER=root

[Qemu-devel] [PATCH] util: remove the obsolete non-blocking connect

[Mao Zhongyi]

From: Cao jin 

The non-blocking connect mechanism is obsolete, and it doesn't
work well in inet connection, because it will call getaddrinfo
first and getaddrinfo will blocks on DNS lookups. Since commit
e65c67e4 & d984464e, the non-blocking connect of migration goes
through QIOChannel in a different manner(using a thread), and
nobody use this old non-blocking connect anymore.

Any newly written code which needs a non-blocking connect should
use the QIOChannel code, so we can drop NonBlockingConnectHandler
as a concept entirely.

Cc: mitake.hito...@lab.ntt.co.jp
Cc: namei.u...@gmail.com
Cc: jc...@redhat.com
Cc: kw...@redhat.com
Cc: mre...@redhat.com
Cc: berra...@redhat.com
Cc: kra...@redhat.com
Cc: pbonz...@redhat.com
Cc: qemu-bl...@nongnu.org
Cc: sheep...@lists.wpkg.org
Suggested-by: Daniel P. Berrange 
Signed-off-by: Cao jin 
Signed-off-by: Mao Zhongyi 
---
This patch was reviewed by Daniel about a years ago, but it has never
been merged just since socket_connect() called by net_socket_connect_init()
where NonBlockingConnectHandler was passed to socket_connect(). it's broken.

Now this problem was worked around by Daniel's patch(commit 6701e551 'Revert
"Change net/socket.c to use socket_*() functions" again'). Therefore, resend
it, of course, part of related code has changed over the years, so changed
the patch accordingly.

The reviewed patch listed on:
https://lists.gnu.org/archive/html/qemu-devel/2016-07/msg06373.html

 block/sheepdog.c   |   2 +-
 include/qemu/sockets.h |   9 +--
 io/channel-socket.c|   2 +-
 util/qemu-sockets.c| 198 ++---
 4 files changed, 26 insertions(+), 185 deletions(-)

diff --git a/block/sheepdog.c b/block/sheepdog.c
index a18315a..97e6c96 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -589,7 +589,7 @@ static int connect_to_sdog(BDRVSheepdogState *s, Error 
**errp)
 {
 int fd;
 
-fd = socket_connect(s->addr, NULL, NULL, errp);
+fd = socket_connect(s->addr, errp);
 
 if (s->addr->type == SOCKET_ADDRESS_TYPE_INET && fd >= 0) {
 int ret = socket_set_nodelay(fd);
diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h
index 5c326db..b2f0478 100644
--- a/include/qemu/sockets.h
+++ b/include/qemu/sockets.h
@@ -27,17 +27,11 @@ int socket_set_fast_reuse(int fd);
 #define SHUT_RDWR 2
 #endif
 
-/* callback function for nonblocking connect
- * valid fd on success, negative error code on failure
- */
-typedef void NonBlockingConnectHandler(int fd, Error *err, void *opaque);
-
 int inet_ai_family_from_address(InetSocketAddress *addr,
 Error **errp);
 int inet_parse(InetSocketAddress *addr, const char *str, Error **errp);
 int inet_connect(const char *str, Error **errp);
 int inet_connect_saddr(InetSocketAddress *saddr,
-   NonBlockingConnectHandler *callback, void *opaque,
Error **errp);
 
 NetworkAddressFamily inet_netfamily(int family);
@@ -46,8 +40,7 @@ int unix_listen(const char *path, char *ostr, int olen, Error 
**errp);
 int unix_connect(const char *path, Error **errp);
 
 SocketAddress *socket_parse(const char *str, Error **errp);
-int socket_connect(SocketAddress *addr, NonBlockingConnectHandler *callback,
-   void *opaque, Error **errp);
+int socket_connect(SocketAddress *addr, Error **errp);
 int socket_listen(SocketAddress *addr, Error **errp);
 void socket_listen_cleanup(int fd, Error **errp);
 int socket_dgram(SocketAddress *remote, SocketAddress *local, Error **errp);
diff --git a/io/channel-socket.c b/io/channel-socket.c
index 53386b7..05ea120 100644
--- a/io/channel-socket.c
+++ b/io/channel-socket.c
@@ -140,7 +140,7 @@ int qio_channel_socket_connect_sync(QIOChannelSocket *ioc,
 int fd;
 
 trace_qio_channel_socket_connect_sync(ioc, addr);
-fd = socket_connect(addr, NULL, NULL, errp);
+fd = socket_connect(addr, errp);
 if (fd < 0) {
 trace_qio_channel_socket_connect_fail(ioc);
 return -1;
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 82290cb..e086497 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -263,88 +263,21 @@ listen:
 ((rc) == -EINPROGRESS)
 #endif
 
-/* Struct to store connect state for non blocking connect */
-typedef struct ConnectState {
-int fd;
-struct addrinfo *addr_list;
-struct addrinfo *current_addr;
-NonBlockingConnectHandler *callback;
-void *opaque;
-} ConnectState;
-
-static int inet_connect_addr(struct addrinfo *addr, bool *in_progress,
- ConnectState *connect_state, Error **errp);
-
-static void wait_for_connect(void *opaque)
-{
-ConnectState *s = opaque;
-int val = 0, rc = 0;
-socklen_t valsize = sizeof(val);
-bool in_progress;
-Error *err = NULL;
-
-qemu_set_fd_handler(s->fd, NULL, NULL, NULL);
-
-do {
-rc = 

Re: [Qemu-devel] [PATCH 2/3] exec: simplify address_space_get_iotlb_entry

[Peter Xu]

On Thu, Jun 15, 2017 at 10:31:11AM +0800, Peter Xu wrote:
> On Wed, Jun 14, 2017 at 09:34:52PM +0300, Michael S. Tsirkin wrote:
> > On Mon, Jun 12, 2017 at 12:04:58PM +0800, Peter Xu wrote:
> > > On Mon, Jun 12, 2017 at 06:07:04AM +0300, Michael S. Tsirkin wrote:
> > > > On Mon, Jun 12, 2017 at 10:34:43AM +0800, Peter Xu wrote:
> > > > > On Sun, Jun 11, 2017 at 08:10:15PM +0800, David Gibson wrote:
> > > > > > On Sun, Jun 11, 2017 at 01:09:26PM +0300, Michael S. Tsirkin wrote:
> > > > > > > On Fri, Jun 09, 2017 at 09:58:47AM +0800, Peter Xu wrote:
> > > > > > > > > > The problem is that when I was fixing the problem that 
> > > > > > > > > > vhost had with
> > > > > > > > > > PT (a764040, "exec: abstract 
> > > > > > > > > > address_space_do_translate()"), I did
> > > > > > > > > > broke the IOTLB translation a bit (it was using page 
> > > > > > > > > > masks). IMHO we
> > > > > > > > > > need to fix it first for correctness (patch 1/2).
> > > > > > > > > > 
> > > > > > > > > > For patch 3, if we can have Jason's patch to allow dynamic
> > > > > > > > > > iommu_platform switching, that'll be the best, then I can 
> > > > > > > > > > rewrite
> > > > > > > > > > patch 3 with the switching logic rather than caching 
> > > > > > > > > > anything. But
> > > > > > > > > > IMHO that can be separated from patch 1/2 if you like.
> > > > > > > > > > 
> > > > > > > > > > Or do you have better suggestion on how should we fix it?
> > > > > > > > > > 
> > > > > > > > > > Thanks,
> > > > > > > > > 
> > > > > > > > > Can we drop masks completely and replace with length? I think 
> > > > > > > > > we
> > > > > > > > > should do that instead of trying to fix masks.
> > > > > > > > 
> > > > > > > > Do you mean to modify IOMMUTLBEntry.addr_mask into length?
> > > > > > > 
> > > > > > > I think it's better than alternatives.
> > > > > > > 
> > > > > > > > Again, I am not sure this is good... At least we need to get 
> > > > > > > > ack from
> > > > > > > > David since spapr should be the initial user of it, and 
> > > > > > > > possibly also
> > > > > > > > Alex since vfio should be assuming that (IIUC both in QEMU and 
> > > > > > > > kernel)
> > > > > > > > addr_mask is page masks rather than arbirary length.
> > > > > > > > 
> > > > > > > > (CC Alex)
> > > > > > > > 
> > > > > > > > Thanks,
> > > > > > > 
> > > > > > > Callbacks that need powers of two can easily split up the range.
> > > > > > 
> > > > > > I think I missed part of the thread.  What's the original use case 
> > > > > > for
> > > > > > non-power-of-two IOTLB entries?  It certainly won't happen on Power.
> > > > > 
> > > > > Currently address_space_get_iotlb_entry() didn't really follow the
> > > > > rule, addr_mask can be arbitary length. This series tried to fix it,
> > > > > while Michael was questioning about whether we should really fix that
> > > > > at all.
> > > > > 
> > > > > Michael,
> > > > > 
> > > > > Even if for performance's sake, I should still think we should fix it.
> > > > > Let's consider a most simple worst case: we have a single page mapped
> > > > > with IOVA range (2M page):
> > > > > 
> > > > >  [0x0, 0x20)
> > > > > 
> > > > > And if guest access IOVA using the following patern:
> > > > > 
> > > > >  0x1f, 0x1e, 0x1d, ...
> > > > > 
> > > > > Then now we'll get this:
> > > > > 
> > > > > - request 0x1f, cache miss, will get iotlb [0x1f, 0x20)
> > > > > - request 0x1e, cache miss, will get iotlb [0x1e, 0x20)
> > > > > - request 0x1d, cache miss, will get iotlb [0x1d, 0x20)
> > > > > - ...
> > > > 
> > > > We pass an offset too, do we not? So callee can figure out
> > > > the region starts at 0x0 and avoid 2nd and 3rd misses.
> > > 
> > > Here when you say "offset", do you mean the offset in
> > > MemoryRegionSection?
> > > 
> > > In address_space_get_iotlb_entry() we have this:
> > > 
> > > section = address_space_do_translate(as, addr, , ,
> > >  is_write, false);
> > > 
> > > One thing to mention is that, imho we cannot really assume the xlat is
> > > valid on the whole "section" range - the section can be a huge GPA
> > > range, while the xlat may only be valid on a single 4K page. The only
> > > safe region we can use here is (xlat, xlat+plen). Outside that, we
> > > should know nothing valid.
> 
> [1]
> 
> > > 
> > > Please correct me if I didn't really catch the point..
> > 
> > IIUC section is the translation result. If so all of it is valid,
> > not just one page.
> 
> It should be only one page (I think that depends on how we implemented
> the translation logic). Please check this (gdb session on current
> master):
> 
> (gdb) b address_space_get_iotlb_entry 
> Breakpoint 2 at 0x5576803a: file /root/git/qemu/exec.c, line 512.
> ... (until break)
> (gdb) bt
> #0  0x5576803a in address_space_get_iotlb_entry (as=0x5841bbb0, 
> addr=4294959104, is_write=false) at /root/git/qemu/exec.c:512
> #1  0x558322dd in 

[Qemu-devel] [FIX PATCH] spapr: prevent QEMU crash when CPU realization fails

[Bharata B Rao]

ICPState objects were being allocated before CPU thread realization.
However commit 9ed656631d73 (xics: setup cpu at realize time) reversed it
by allocating ICPState objects after CPU thread is realized. But it
didn't take care to fix the error path because of which we observe
a SIGSEGV when CPU thread realization fails during cold/hotplug.

Fix this by ensuring that we do object_unparent() of ICPState object
only in case when is was created earlier.

Signed-off-by: Bharata B Rao 
---
NOTE: There is another SIGSEGV failure that I am investigating which happens
when CPU realization fails. It appears that that the CPU object isn't getting
fully cleaned up.

 hw/ppc/spapr_cpu_core.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index d6719d5..0d0e959 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -198,13 +198,14 @@ static void spapr_cpu_core_realize_child(Object *child, 
Error **errp)
 object_property_add_const_link(obj, ICP_PROP_CPU, child, _abort);
 object_property_set_bool(obj, true, "realized", _err);
 if (local_err) {
-goto error;
+goto free_icp;
 }
 
 return;
 
-error:
+free_icp:
 object_unparent(obj);
+error:
 error_propagate(errp, local_err);
 }
 
-- 
2.7.4

Re: [Qemu-devel] [PATCH] fix: avoid infinite loop when blockjob encountering failure

[sochin.jiang]

Thanks for your kindly reply.

I do have made a mistake that ignoring the AIOContext lock.

About the patch, firstly, if job->ret comes to be non-zero(also means 
job->completed to be true) , blockjob 'callback'(common_block_job_cb) will be 
called, blockjob error will be put into errp. It won't report success.

Secondly, blockjob fails with 'ret < 0' and without calling 
block_job_complete_sync(), we won't have segfault because bdrv_reopen won't be 
called. Also, with the use-after-free problems.

So, skip the block_job_complete_sync() call if job->completed(job->ret to be 
non-zero) is true can avoid all the problems, am I right ?

Thank you again.


Best Regard.

Sochin







On 2017/6/14 21:12, Max Reitz wrote:
> Thanks for your patch! The issue can be reproduced as follows:
>
> $ qemu-img create -f qcow2 -b \
> "json:{'driver':'raw','file':{
> 'driver':'blkdebug','inject-error':[{'event':'write_aio'}],
> 'image':{'driver':'null-co'}}}" \
>  overlay.qcow2
> $ qemu-io -c 'write 0 64k' overlay.qcow2
> $ qemu-img commit overlay.qcow2
>
> While your patch fixes that issue, I still have some comments:
>
> On 2017-06-14 08:22, sochin.jiang wrote:
>> From: "sochin.jiang" 
>>
>> img_commit could fall into infinite loop if it's blockjob
> This should be "into an infinite loop" and "its" instead if "it's".
>
> This empty line should be omitted.
>
>> fail encountering any I/O error. Try to fix it.
> Should be "fails on any I/O error" or "fails on encountering any I/O
> error". Also, you're not trying to fix it but let's all hope you really
> are fixing it. :-)
>
> (So "Fix it." instead of "Try to fix it.")
>
>> Signed-off-by: sochin.jiang 
>> ---
>>  qemu-img.c | 5 -
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/qemu-img.c b/qemu-img.c
>> index 0ad698d..6ba565d 100644
>> --- a/qemu-img.c
>> +++ b/qemu-img.c
>> @@ -895,8 +895,11 @@ static void run_block_job(BlockJob *job, Error **errp)
>>  aio_poll(aio_context, true);
>>  qemu_progress_print(job->len ?
>>  ((float)job->offset / job->len * 100.f) : 0.0f, 
>> 0);
>> -} while (!job->ready);
>> +} while (!job->ready && !job->ret);
> I think it would be better to test job->completed instead of job->ret.
>
>>  
>> +if (job->ret) {
>> +return;
>> +}
> We shouldn't just return here but still do all the deinitialization like
> call aio_context_release(). I guess the best would be to just skip the
> block_job_complete_sync() call if job->completed is true.
>
>>  block_job_complete_sync(job, errp);
>>  aio_context_release(aio_context);
> Then, there are three more issues I found while reviewing this patch:
>
> First, if the block job is completed before block_job_complete_sync() is
> called (i.e. if an error occurred), it is automatically freed. This is
> bad because this means we'll have some instances of use-after-free here.
> Therefore, we need to invoke block_job_ref() before run_block_job() and
> block_job_unref() afterwards. (And since these functions are currenctly
> static in blockjob.c, we'll have to make them global.)
>
> Secondly, run_block_job() doesn't evaluate job->ret. Therefore it will
> report success even if the commit failed (it is expecting
> block_job_complete_sync() to put an error into errp, but it will not do
> that). So we'll have to do that (manually check job->ret and if it's
> negative, put an error message into errp; also, assert that
> job->cancelled is false).
>
> Thirdly, we have segfault in bdrv_reopen_prepare() if the image has
> non-string options... I'll handle this one.
>
> I can also handle the other two issues, if you'd like me to.
>
>
> Finally, an iotest would be nice (see my reproducer above). But I can
> handle that as well, if you decide not to write one.
>
> Max
>

Re: [Qemu-devel] [Qemu-ppc] [PATCH] target/ppc/excp_helper: Take BQL before calling cpu_interrupt()

[Nikunj A Dadhania]

Alex Bennée  writes:

> Thomas Huth  writes:
>
>> Since the introduction of MTTCG, using the msgsnd instruction
>> abort()s if being called without holding the BQL. So let's protect
>> that part of the code now with qemu_mutex_lock_iothread().
>>
>> Buglink: https://bugs.launchpad.net/qemu/+bug/1694998
>> Signed-off-by: Thomas Huth 
>
> Reviewed-by: Alex Bennée 
>
> p.s. I was checking the ppc code for other CPU_FOREACH patterns and I
> noticed the tlb_flush calls could probably use the tlb_flush_all_cpus
> API instead of manually looping themselves.

Will that be synchronous call? In PPC, we do lazy tlb flush, the tlb
flushes are batched until a synchronization point (for optimization).
The batching is achieved using a tlb_need_flush (global/local) and when
there is isync/ptesync or an exception, the actual flush is done. At
this point we need to make sure that the flush is synchronous.

> You should also double check the semantics to make sure none of them
> need to use the _synced variant and a cpu_exit if the flush needs to
> complete w.r.t the originating CPU.

Regards,
Nikunj

Re: [Qemu-devel] [PATCH 2/3] exec: simplify address_space_get_iotlb_entry

[Peter Xu]

On Wed, Jun 14, 2017 at 09:34:52PM +0300, Michael S. Tsirkin wrote:
> On Mon, Jun 12, 2017 at 12:04:58PM +0800, Peter Xu wrote:
> > On Mon, Jun 12, 2017 at 06:07:04AM +0300, Michael S. Tsirkin wrote:
> > > On Mon, Jun 12, 2017 at 10:34:43AM +0800, Peter Xu wrote:
> > > > On Sun, Jun 11, 2017 at 08:10:15PM +0800, David Gibson wrote:
> > > > > On Sun, Jun 11, 2017 at 01:09:26PM +0300, Michael S. Tsirkin wrote:
> > > > > > On Fri, Jun 09, 2017 at 09:58:47AM +0800, Peter Xu wrote:
> > > > > > > > > The problem is that when I was fixing the problem that vhost 
> > > > > > > > > had with
> > > > > > > > > PT (a764040, "exec: abstract address_space_do_translate()"), 
> > > > > > > > > I did
> > > > > > > > > broke the IOTLB translation a bit (it was using page masks). 
> > > > > > > > > IMHO we
> > > > > > > > > need to fix it first for correctness (patch 1/2).
> > > > > > > > > 
> > > > > > > > > For patch 3, if we can have Jason's patch to allow dynamic
> > > > > > > > > iommu_platform switching, that'll be the best, then I can 
> > > > > > > > > rewrite
> > > > > > > > > patch 3 with the switching logic rather than caching 
> > > > > > > > > anything. But
> > > > > > > > > IMHO that can be separated from patch 1/2 if you like.
> > > > > > > > > 
> > > > > > > > > Or do you have better suggestion on how should we fix it?
> > > > > > > > > 
> > > > > > > > > Thanks,
> > > > > > > > 
> > > > > > > > Can we drop masks completely and replace with length? I think we
> > > > > > > > should do that instead of trying to fix masks.
> > > > > > > 
> > > > > > > Do you mean to modify IOMMUTLBEntry.addr_mask into length?
> > > > > > 
> > > > > > I think it's better than alternatives.
> > > > > > 
> > > > > > > Again, I am not sure this is good... At least we need to get ack 
> > > > > > > from
> > > > > > > David since spapr should be the initial user of it, and possibly 
> > > > > > > also
> > > > > > > Alex since vfio should be assuming that (IIUC both in QEMU and 
> > > > > > > kernel)
> > > > > > > addr_mask is page masks rather than arbirary length.
> > > > > > > 
> > > > > > > (CC Alex)
> > > > > > > 
> > > > > > > Thanks,
> > > > > > 
> > > > > > Callbacks that need powers of two can easily split up the range.
> > > > > 
> > > > > I think I missed part of the thread.  What's the original use case for
> > > > > non-power-of-two IOTLB entries?  It certainly won't happen on Power.
> > > > 
> > > > Currently address_space_get_iotlb_entry() didn't really follow the
> > > > rule, addr_mask can be arbitary length. This series tried to fix it,
> > > > while Michael was questioning about whether we should really fix that
> > > > at all.
> > > > 
> > > > Michael,
> > > > 
> > > > Even if for performance's sake, I should still think we should fix it.
> > > > Let's consider a most simple worst case: we have a single page mapped
> > > > with IOVA range (2M page):
> > > > 
> > > >  [0x0, 0x20)
> > > > 
> > > > And if guest access IOVA using the following patern:
> > > > 
> > > >  0x1f, 0x1e, 0x1d, ...
> > > > 
> > > > Then now we'll get this:
> > > > 
> > > > - request 0x1f, cache miss, will get iotlb [0x1f, 0x20)
> > > > - request 0x1e, cache miss, will get iotlb [0x1e, 0x20)
> > > > - request 0x1d, cache miss, will get iotlb [0x1d, 0x20)
> > > > - ...
> > > 
> > > We pass an offset too, do we not? So callee can figure out
> > > the region starts at 0x0 and avoid 2nd and 3rd misses.
> > 
> > Here when you say "offset", do you mean the offset in
> > MemoryRegionSection?
> > 
> > In address_space_get_iotlb_entry() we have this:
> > 
> > section = address_space_do_translate(as, addr, , ,
> >  is_write, false);
> > 
> > One thing to mention is that, imho we cannot really assume the xlat is
> > valid on the whole "section" range - the section can be a huge GPA
> > range, while the xlat may only be valid on a single 4K page. The only
> > safe region we can use here is (xlat, xlat+plen). Outside that, we
> > should know nothing valid.

[1]

> > 
> > Please correct me if I didn't really catch the point..
> 
> IIUC section is the translation result. If so all of it is valid,
> not just one page.

It should be only one page (I think that depends on how we implemented
the translation logic). Please check this (gdb session on current
master):

(gdb) b address_space_get_iotlb_entry 
Breakpoint 2 at 0x5576803a: file /root/git/qemu/exec.c, line 512.
... (until break)
(gdb) bt
#0  0x5576803a in address_space_get_iotlb_entry (as=0x5841bbb0, 
addr=4294959104, is_write=false) at /root/git/qemu/exec.c:512
#1  0x558322dd in vhost_device_iotlb_miss (dev=0x568b03a0, 
iova=4294959104, write=0) at /root/git/qemu/hw/virtio/vhost.c:982
#2  0x55834ad9 in vhost_backend_handle_iotlb_msg (dev=0x568b03a0, 
imsg=0x7fffd428) at /root/git/qemu/hw/virtio/vhost-backend.c:334
#3  0x558347be in vhost_kernel_iotlb_read 

Re: [Qemu-devel] [PATCH] tcg-runtime: increase hit rate of lookup_tb_ptr

[Richard Henderson]

On 06/14/2017 01:27 PM, Emilio G. Cota wrote:

On Wed, Jun 14, 2017 at 12:48:17 -0700, Richard Henderson wrote:

We can call tb_htable_lookup even when the tb_jmp_cache
is completely empty.  Therefore, un-nest most of the code
dependent on tb != NULL from the read from the cache.

Signed-off-by: Richard Henderson 


I just wrote this alternative patch, which does the same thing
as yours. I also measured what the effect of this change
has on the hit rate of lookup_tb_ptr. Feel free to reuse parts
of the patch and/or the commit message!


Thanks.  I'll adjust the commit.


r~

Re: [Qemu-devel] [PATCH v2 2/5] target/alpha: Use tcg_gen_lookup_and_goto_ptr

[Richard Henderson]

On 06/14/2017 01:37 PM, Emilio G. Cota wrote:

On Wed, Jun 14, 2017 at 12:48:18 -0700, Richard Henderson wrote:

Signed-off-by: Richard Henderson 

(snip)

@@ -1198,7 +1205,10 @@ static ExitStatus gen_call_pal(DisasContext *ctx, int 
palcode)
  tcg_gen_andi_i64(tmp, ctx->ir[IR_A0], PS_INT_MASK);
  tcg_gen_st8_i64(tmp, cpu_env, offsetof(CPUAlphaState, ps));
  tcg_temp_free(tmp);
-break;
+
+/* Allow interrupts to be recognized right away.  */
+tcg_gen_movi_i64(cpu_pc, ctx.pc);


ctx->pc though


Bah.  I knew I'd fixed that, but it got folded into the s390 patch.


r~

Re: [Qemu-devel] [PATCH] target/aarch64: exit to main loop after 'msr daifclr'

[Richard Henderson]

On 06/14/2017 01:33 PM, Emilio G. Cota wrote:

On Wed, Jun 14, 2017 at 12:48:21 -0700, Richard Henderson wrote:

Exit to cpu loop so we reevaluate cpu_arm_hw_interrupts.

Cc: qemu-...@nongnu.org
Cc: Peter Maydell 
Signed-off-by: Richard Henderson 
---
  target/arm/translate-a64.c | 7 ++-
  1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index 860e279..e55547d 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -1422,7 +1422,9 @@ static void handle_msr_i(DisasContext *s, uint32_t insn,
  gen_helper_msr_i_pstate(cpu_env, tcg_op, tcg_imm);
  tcg_temp_free_i32(tcg_imm);
  tcg_temp_free_i32(tcg_op);
-s->is_jmp = DISAS_UPDATE;
+/* For DAIFClear, exit the cpu loop to re-evaluate pending IRQs.  */
+gen_a64_set_pc_im(s->pc);


For op != 0x1f we end up setting the pc twice (first here, then in
the switch statement). It's still correct though.


No, that's why I switched to DISAS_JUMP.




+s->is_jmp = (op == 0x1f ? DISAS_EXIT : DISAS_JUMP);


Could do without the parens.


I think it's clearer.


+case DISAS_EXIT:
+gen_a64_set_pc_im(dc->pc);
+tcg_gen_exit_tb(0);
+break;


This gives translate-a64.c and translate.c different semantics for DISAS_EXIT. 
I considered that to be a bad thing.



r~

Re: [Qemu-devel] [PATCH] target/m68k: fix V flag for CC_OP_SUBx

[Richard Henderson]

On 06/14/2017 01:39 PM, Laurent Vivier wrote:

V flag for subtraction is:

v = (res ^ src1) & (src1 ^ src2)

(see COMPUTE_CCR() in target/m68k/helper.c)

But gen_flush_flags() uses:

v = (res ^ src2) & (src1 ^ src2)

The problem has been found with the following program:

 .global _start
_start:
 move.l  #-2147483648,%d0
 subq.l  #1,%d0
 jvc 1f
 move.l #1,%d1
 move.l #1,%d0
 trap #0
1:
 move.l #0,%d1
 move.l #1,%d0
 trap #0

It works fine (exit(1)) on real hardware, and with "-singlestep".

"-singlestep" uses gen_helper_flush_flags(), whereas
without "-singlestep", V flag is computed directly in
gen_flush_flags().

This patch updates gen_flush_flags() to have the same result
as with gen_helper_flush_flags().

Signed-off-by: Laurent Vivier
---
  target/m68k/translate.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)


Reviewed-by: Richard Henderson 


r~

[Qemu-devel] [Bug 823733] Re: Soloaris can't be poweroff

[Thomas Huth]

Triaging old bug tickets ... can you still reproduce this problem with
the latest version of QEMU (currently version 2.9.0)?

** Changed in: qemu
   Status: New => Incomplete

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

Title:
  Soloaris can't be poweroff

Status in QEMU:
  Incomplete

Bug description:
  Thank you forgive my poor English.

  It seems KVM can’t poweroff solairs 10 or sloalrs 11 VM.
  I have created solaris 10 and 11 as usual. Everything in VM is running OK, 
but finally I use shell command ‘poweroff’ or ‘init 5’, the solaris VM (both 10 
& 11) system could’t be poweroff but with promoting me the message: perss any 
key to reboot …..  ,I pressed any key in vnc client, solaris VM reboot 
immediately. Endless reboot loop above.

  the solaris 10 & 11 from oracle iso file name :
  sol-10-u9-ga-x86-dvd.iso
  sol-11-exp-201011-text-x86.iso

  the solaris 10 & 11 from oracle iso file name :
  sol-10-u9-ga-x86-dvd.iso
  sol-11-exp-201011-text-x86.iso

  1. On my real physical machine,the solaris can be poweroff
  2. On vmware ,the solaris can be poweroff
  3. On my real physical machine,I have try to disbale the ACPI opiton in BOIS, 
then the solaris can't be poweroff,Like the problem I have described above
  so ,I doubt the KVM has a little problem in ACPI 

  I have try the suggestion as follows, but I can’t solve the problem.
  7.2 Solaris reboot all the time on grub menu
  • Run through the installer as usual 
  • On completion and reboot, the VM will perpetually reboot. "Stop" the 
VM. 
  • Start it up again, and immediately open a vnc console and select the 
Safe Boot from the options screen 
  • When prompted if you want to try and recover the boot block, say yes 
  • You should now have a Bourne terminal with your existing filesystem 
mounted on /a 
  • Run /a/usr/bin/bash (my preferred shell) 
  • export TERM=xterm 
  • vi /a/boot/grub/menu.1st (editing the bootloader on your mounted 
filesystem), to add "kernel/unix" to the kernel options for the non-safe-mode 
boot. Ex : 
  Config File : /a/boot/grub/menu.lst 
  kernel$ /platform/i86pc/multiboot -B $ZFS-BOOTFS kernel/unix

  According to KVM requirements, I collected the following information:
  CPU model name
  model name  : Intel(R) Xeon(R) CPU   X3450  @ 2.67GHz

  kvm -version
  QEMU PC emulator version 0.12.3 (qemu-kvm-0.12.3), Copyright (c) 2003-2008 
Fabrice Bellard

  Host kernel version
  Ubuntu 10.04.1 LTS   2.6.32-25-server 

  What host kernel arch you are using (i386 or x86_64)
  X86_64

  Guest OS
  Solaris 10 and Solaris 11，both can not shutdown

  The qemu command line you are using to start the guest

  First, I used the command line as follows:
  kvm -m 1024 -drive file=solaris10.img,cache=writeback -net nic -net user 
-nographic -vnc :1
  then I try to use -no-kvm-irqchip or -no-kvm ,but the problem also appears!

  Secondly, have created and run solaris 10&11 by using Virsh, still solaris 
can't be poweroff, the XML file content is :
  
  solairs
  85badf15-244d-4719-a2da-8c3de064137d
  1677721
  1677721
  1
  
  hvm

 
 
  
  
 

 destroy
restart
 destroy
 
   /usr/bin/kvm
   

 
 
   
  



   
   
   
  

  


  libvirt-f36f5289-692e-6f1c-fe71-c6ed19453e2f
  libvirt-f36f5289-692e-6f1c-fe71-c6ed19453e2f

   

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

[Qemu-devel] [PATCH] tcg: consistently access cpu->tb_jmp_cache atomically

[Emilio G. Cota]

Some code paths can lead to atomic accesses racing with memset()
on cpu->tb_jmp_cache, which can result in torn reads/writes
and is undefined behaviour in C11.

These torn accesses are unlikely to show up as bugs, but from code
inspection they seem possible. For example, tb_phys_invalidate does:
/* remove the TB from the hash list */
h = tb_jmp_cache_hash_func(tb->pc);
CPU_FOREACH(cpu) {
if (atomic_read(>tb_jmp_cache[h]) == tb) {
atomic_set(>tb_jmp_cache[h], NULL);
}
}
Here atomic_set might race with a concurrent memset (such as the
ones scheduled via "unsafe" async work, e.g. tlb_flush_page) and
therefore we might end up with a torn pointer (or who knows what,
because we are under undefined behaviour).

This patch converts parallel accesses to cpu->tb_jmp_cache to use
atomic primitives, thereby bringing these accesses back to defined
behaviour. The price to pay is to potentially execute more instructions
when clearing cpu->tb_jmp_cache, but given how infrequently they happen
and the small size of the cache, the performance impact I have measured
is within noise range when booting debian-arm.

Note that under "safe async" work (e.g. do_tb_flush) we could use memset
because no other vcpus are running. However I'm keeping these accesses
atomic as well to keep things simple and to avoid confusing analysis
tools such as ThreadSanitizer.

Signed-off-by: Emilio G. Cota 
---
 cputlb.c  |  4 ++--
 include/qom/cpu.h | 11 ++-
 qom/cpu.c |  5 +
 translate-all.c   | 25 -
 4 files changed, 25 insertions(+), 20 deletions(-)

diff --git a/cputlb.c b/cputlb.c
index 743776a..e85b6d3 100644
--- a/cputlb.c
+++ b/cputlb.c
@@ -118,7 +118,7 @@ static void tlb_flush_nocheck(CPUState *cpu)
 
 memset(env->tlb_table, -1, sizeof(env->tlb_table));
 memset(env->tlb_v_table, -1, sizeof(env->tlb_v_table));
-memset(cpu->tb_jmp_cache, 0, sizeof(cpu->tb_jmp_cache));
+cpu_tb_jmp_cache_clear(cpu);
 
 env->vtlb_index = 0;
 env->tlb_flush_addr = -1;
@@ -183,7 +183,7 @@ static void tlb_flush_by_mmuidx_async_work(CPUState *cpu, 
run_on_cpu_data data)
 }
 }
 
-memset(cpu->tb_jmp_cache, 0, sizeof(cpu->tb_jmp_cache));
+cpu_tb_jmp_cache_clear(cpu);
 
 tlb_debug("done\n");
 
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 89ddb68..2fe7cff 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -346,7 +346,7 @@ struct CPUState {
 
 void *env_ptr; /* CPUArchState */
 
-/* Writes protected by tb_lock, reads not thread-safe  */
+/* Accessed in parallel; all accesses must be atomic */
 struct TranslationBlock *tb_jmp_cache[TB_JMP_CACHE_SIZE];
 
 struct GDBRegisterState *gdb_regs;
@@ -422,6 +422,15 @@ extern struct CPUTailQ cpus;
 
 extern __thread CPUState *current_cpu;
 
+static inline void cpu_tb_jmp_cache_clear(CPUState *cpu)
+{
+unsigned int i;
+
+for (i = 0; i < TB_JMP_CACHE_SIZE; i++) {
+atomic_set(>tb_jmp_cache[i], NULL);
+}
+}
+
 /**
  * qemu_tcg_mttcg_enabled:
  * Check whether we are running MultiThread TCG or not.
diff --git a/qom/cpu.c b/qom/cpu.c
index 5069876..585419b 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -274,7 +274,6 @@ void cpu_reset(CPUState *cpu)
 static void cpu_common_reset(CPUState *cpu)
 {
 CPUClass *cc = CPU_GET_CLASS(cpu);
-int i;
 
 if (qemu_loglevel_mask(CPU_LOG_RESET)) {
 qemu_log("CPU Reset (CPU %d)\n", cpu->cpu_index);
@@ -292,9 +291,7 @@ static void cpu_common_reset(CPUState *cpu)
 cpu->crash_occurred = false;
 
 if (tcg_enabled()) {
-for (i = 0; i < TB_JMP_CACHE_SIZE; ++i) {
-atomic_set(>tb_jmp_cache[i], NULL);
-}
+cpu_tb_jmp_cache_clear(cpu);
 
 #ifdef CONFIG_SOFTMMU
 tlb_flush(cpu, 0);
diff --git a/translate-all.c b/translate-all.c
index b3ee876..af3c4df 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -923,11 +923,7 @@ static void do_tb_flush(CPUState *cpu, run_on_cpu_data 
tb_flush_count)
 }
 
 CPU_FOREACH(cpu) {
-int i;
-
-for (i = 0; i < TB_JMP_CACHE_SIZE; ++i) {
-atomic_set(>tb_jmp_cache[i], NULL);
-}
+cpu_tb_jmp_cache_clear(cpu);
 }
 
 tcg_ctx.tb_ctx.nb_tbs = 0;
@@ -1806,19 +1802,22 @@ void cpu_io_recompile(CPUState *cpu, uintptr_t retaddr)
 cpu_loop_exit_noexc(cpu);
 }
 
-void tb_flush_jmp_cache(CPUState *cpu, target_ulong addr)
+static void tb_jmp_cache_clear_page(CPUState *cpu, target_ulong page_addr)
 {
+unsigned int i0 = tb_jmp_cache_hash_page(page_addr);
 unsigned int i;
 
+for (i = 0; i < TB_JMP_PAGE_SIZE; i++) {
+atomic_set(>tb_jmp_cache[i0 + i], NULL);
+}
+}
+
+void tb_flush_jmp_cache(CPUState *cpu, target_ulong addr)
+{
 /* Discard jump cache entries for any tb which might potentially
overlap the flushed page.  */
-i = tb_jmp_cache_hash_page(addr - TARGET_PAGE_SIZE);
-memset(>tb_jmp_cache[i], 0,
-   

Re: [Qemu-devel] [PATCH] spapr: manage hotplugged devices while the VM is not started

[Michael Roth]

Quoting Igor Mammedov (2017-06-14 04:00:01)
> On Tue, 13 Jun 2017 16:42:45 -0500
> Michael Roth  wrote:
> 
> > Quoting Igor Mammedov (2017-06-09 03:27:33)
> > > On Thu, 08 Jun 2017 15:00:53 -0500
> > > Michael Roth  wrote:
> > >   
> > > > Quoting David Gibson (2017-05-30 23:35:57)  
> > > > > On Tue, May 30, 2017 at 06:04:45PM +0200, Laurent Vivier wrote:
> > > > > > For QEMU, a hotlugged device is a device added using the HMP/QMP
> > > > > > interface.
> > > > > > For SPAPR, a hotplugged device is a device added while the
> > > > > > machine is running. In this case QEMU doesn't update internal
> > > > > > state but relies on the OS for this part
> > > > > > 
> > > > > > In the case of migration, when we (libvirt) hotplug a device
> > > > > > on the source guest, we (libvirt) generally hotplug the same
> > > > > > device on the destination guest. But in this case, the machine
> > > > > > is stopped (RUN_STATE_INMIGRATE) and QEMU must not expect
> > > > > > the OS will manage it as an hotplugged device as it will
> > > > > > be "imported" by the migration.
> > > > > > 
> > > > > > This patch changes the meaning of "hotplugged" in spapr.c
> > > > > > to manage a QEMU hotplugged device like a "coldplugged" one
> > > > > > when the machine is awaiting an incoming migration.
> > > > > > 
> > > > > > Signed-off-by: Laurent Vivier 
> > > > > 
> > > > > So, I think this is a reasonable concept, at least in terms of
> > > > > cleanliness and not doing unnecessary work.  However, if it's fixing
> > > > > bugs, I suspect that means we still have problems elsewhere.
> > > > 
> > > > I was hoping a lot of these issues would go away once we default
> > > > the initial/reset DRC states to "coldplugged". I think your pending
> > > > patch:
> > > > 
> > > >   "spapr: Make DRC reset force DRC into known state"
> > > > 
> > > > But I didn't consider the fact that libvirt will be issuing these
> > > > hotplugs *after* reset, so those states would indeed need to
> > > > be fixed up again to reflect boot-time,attached as opposed to
> > > > boot-time,unattached before starting the target.
> > > > 
> > > > So I do think this patch addresses a specific bug that isn't
> > > > obviously fixable elsewhere.
> > > > 
> > > > To me it seems like the only way to avoid doing something like
> > > > what this patch does is to migrate all attached DRCs from the
> > > > source in all cases.
> > > > 
> > > > This would break backward-migration though, unless we switch from
> > > > using subregions for DRCs to explicitly disabling DRC migration
> > > > based on machine type.  
> > > we could leave old machines broken and fix only new machine types,
> > > then it would be easy ot migrate 'additional' DRC state as subsection
> > > only on new for new machines.  
> > 
> > That's an option, but subsections were only really used for backward
> > compatibility. Not sure how much we have to gain from using both.
> If I remember correctly subsections could be/are used for forward compat stuff
> i.e. subsection is generated on source side when .needed callback returns
> true and destinations will just consume whatever data were sent
> without looking at .need callback. So source could generate extra
> DRC subsection when cpu hotplug is enabled for new machine types,
> ex: f816a62daa

Well, what I was thinking was that if we dropped the approach of basing
.needed around "is this DRC in a transitional state?" (which can only
be determined on the source) in favor of "if (dev->hotplugged)", we
could possible get away with a non-subsection VMSD. But you're right,
unless we can ensure dev->hotplugged is sychronized on both source
and dest, we might still need to use a subsection regardless. *Unless*
we just migrate all DRCs indiscriminately...

> 
> adding David/Juan to CC list to correct me if I'm wrong.

Thanks Juan and David for clarifying.

> 
> > > > That approach seems to similar to what x86 does, e.g.
> > > > hw/acpi/ich9.c and hw/acpi/piix.c migrate vmstate_memhp_state
> > > > (corresponding to all DIMMs' slot status) in all cases where
> > > > memory hotplug is enabled. If they were to do this using
> > > > subregions for DIMMs in a transitional state I think similar
> > > > issues would pop up in that code as well.
> > > > 
> > > > Even if we take this route, we still need to explicitly suppress
> > > > hotplug events during INMIGRATE to avoid extra events going on
> > > > the queue. *Unless* we similarly rely purely on the ones sent by
> > > > the source.  
> > > pc/q35 might also lose events if device is hotplugged during migration,
> > > in addition migration would fail anyway since dst qemu
> > > should be launched with all devices that are present on src.
> > > 
> > > ex: consider if one hotplugs DIMM during migration, it creates
> > > RAM region mapped into guest and that region might be transferred
> > > as part of VMState (not sure if it even works)
> > 

[Qemu-devel] [PATCH 1/2] powerpc: Fix emulation of mcrf in emulate_step()

[Anton Blanchard]

From: Anton Blanchard 

The mcrf emulation code was looking at the CR fields in the reverse
order. It also relied on reserved fields being zero which is somewhat
fragile, so fix that too.

Cc: sta...@vger.kernel.org
Signed-off-by: Anton Blanchard 
---
 arch/powerpc/lib/sstep.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
index 33117f8a0882..fb84f51b1f0b 100644
--- a/arch/powerpc/lib/sstep.c
+++ b/arch/powerpc/lib/sstep.c
@@ -683,8 +683,10 @@ int analyse_instr(struct instruction_op *op, struct 
pt_regs *regs,
case 19:
switch ((instr >> 1) & 0x3ff) {
case 0: /* mcrf */
-   rd = (instr >> 21) & 0x1c;
-   ra = (instr >> 16) & 0x1c;
+   rd = 7 - ((instr >> 23) & 0x7);
+   ra = 7 - ((instr >> 18) & 0x7);
+   rd *= 4;
+   ra *= 4;
val = (regs->ccr >> ra) & 0xf;
regs->ccr = (regs->ccr & ~(0xfUL << rd)) | (val << rd);
goto instr_done;
-- 
2.11.0

Re: [Qemu-devel] [PATCH v1 2/3] target/s390x: implement mvcos instruction

[Richard Henderson]

On 06/14/2017 01:00 PM, Thomas Huth wrote:

On 14.06.2017 09:56, David Hildenbrand wrote:
[...]

I think you should also mask the length with 0x if the PSW was
not in 64-bit mode? Or is this done automagically by the generated TCG
code already?


I was asking myself the same question, but it shouldn't really matter as
was we will be using a maximum of 4096, no?


Question is whether we can end up here somehow in 32-bit mode and the
upper part of the register is still != 0 ... something like 0x10010
for example. Can we be sure that the upper half is always cleared if we
switch from 64-bit mode to the 32-bit mode before?


We don't currently have any length masking at the translate level.
Within mem_helper.c, we have wrap_length.


r~

Re: [Qemu-devel] [PATCH v6 1/6] Pass generic CPUState to gen_intermediate_code()

[Laurent Vivier]

Le 12/06/2017 à 16:53, Lluís Vilanova a écrit :
> Needed to implement a target-agnostic gen_intermediate_code() in the
> future.
> 
> Signed-off-by: Lluís Vilanova 
> Reviewed-by: David Gibson 
> Reviewed-by: Richard Henderson 
> ---
>  include/exec/exec-all.h   |2 +-
>  target/alpha/translate.c  |   11 +--
>  target/arm/translate.c|   20 ++--
>  target/cris/translate.c   |   17 -
>  target/i386/translate.c   |   13 ++---
>  target/lm32/translate.c   |   22 +++---
>  target/m68k/translate.c   |   15 +++
>  target/microblaze/translate.c |   22 +++---
>  target/mips/translate.c   |   15 +++
>  target/moxie/translate.c  |   14 +++---
>  target/openrisc/translate.c   |   19 ++-
>  target/ppc/translate.c|   15 +++
>  target/s390x/translate.c  |   13 ++---
>  target/sh4/translate.c|   15 +++
>  target/sparc/translate.c  |   11 +--
>  target/tilegx/translate.c |7 +++
>  target/tricore/translate.c|9 -
>  target/unicore32/translate.c  |   17 -
>  target/xtensa/translate.c |   13 ++---
>  translate-all.c   |2 +-
>  20 files changed, 130 insertions(+), 142 deletions(-)

for m68k part:

Acked-by: Laurent Vivier 

[Qemu-devel] [Bug 1093691] Re: QEMU build fails on OpenBSD/mips64

[Thomas Huth]

Triaging old bug tickets ... does this problem still persist with the
latest version of QEMU (currently version 2.9.0)?

** Changed in: qemu
   Status: New => Incomplete

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

Title:
  QEMU build fails on OpenBSD/mips64

Status in QEMU:
  Incomplete

Bug description:
  Building QEMU 1.2.1 on OpenBSD/mips64 fails as follows although I
  believe QEMU was also broken with 1.1.x as well..

  cc -I/usr/obj/ports/qemu-1.2.1/qemu-1.2.1/slirp -I. 
-I/usr/obj/ports/qemu-1.2.1/qemu-1.2.1 
-I/usr/obj/ports/qemu-1.2.1/qemu-1.2.1/fpu 
-I/usr/obj/ports/qemu-1.2.1/qemu-1.2.1/tcg -
  I/usr/obj/ports/qemu-1.2.1/qemu-1.2.1/tcg/mips  -D_GNU_SOURCE 
-D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes 
-Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmis
  sing-prototypes -fno-strict-aliasing -I/usr/local/include 
-I/usr/X11R6/include -Wno-redundant-decls -DTIME_MAX=INT_MAX  -Wendif-labels 
-Wmissing-include-dirs -Wnested-externs -Wf
  ormat-security -Wformat-y2k -Winit-self -Wold-style-definition 
-I/usr/local/include/libpng -DHAS_AUDIO -DHAS_AUDIO_CHOICE  
-DTARGET_PHYS_ADDR_BITS=64 -I.. -I/usr/obj/ports/qemu-1
  .2.1/qemu-1.2.1/target-i386 -DNEED_CPU_H 
-I/usr/obj/ports/qemu-1.2.1/qemu-1.2.1/include-I/usr/local/include/libpng 
-pthread -I/usr/local/include/glib-2.0 -I/usr/local/lib/gli
  b-2.0/include -I/usr/local/include -MMD -MP -MT tcg/tcg.o -MF tcg/tcg.d -O2 
-pipe -c -o tcg/tcg.o /usr/obj/ports/qemu-1.2.1/qemu-1.2.1/tcg/tcg.c
  In file included from /usr/obj/ports/qemu-1.2.1/qemu-1.2.1/tcg/tcg.c:50:
  /usr/obj/ports/qemu-1.2.1/qemu-1.2.1/tcg/tcg-op.h: In function 
'tcg_gen_div_i64':
  /usr/obj/ports/qemu-1.2.1/qemu-1.2.1/tcg/tcg-op.h:1229: error: 
'TCG_TARGET_HAS_div_i64' undeclared (first use in this function)
  /usr/obj/ports/qemu-1.2.1/qemu-1.2.1/tcg/tcg-op.h:1229: error: (Each 
undeclared identifier is reported only once
  /usr/obj/ports/qemu-1.2.1/qemu-1.2.1/tcg/tcg-op.h:1229: error: for each 
function it appears in.)
  /usr/obj/ports/qemu-1.2.1/qemu-1.2.1/tcg/tcg-op.h:1231: error: 
'TCG_TARGET_HAS_div2_i64' undeclared (first use in this function)
  /usr/obj/ports/qemu-1.2.1/qemu-1.2.1/tcg/tcg-op.h: In function 
'tcg_gen_rem_i64':
  /usr/obj/ports/qemu-1.2.1/qemu-1.2.1/tcg/tcg-op.h:1248: error: 
'TCG_TARGET_HAS_div_i64' undeclared (first use in this function)
  /usr/obj/ports/qemu-1.2.1/qemu-1.2.1/tcg/tcg-op.h:1250: error: 
'TCG_TARGET_HAS_div2_i64' undeclared (first use in this function)
  /usr/obj/ports/qemu-1.2.1/qemu-1.2.1/tcg/tcg-op.h: In function 
'tcg_gen_divu_i64':
  /usr/obj/ports/qemu-1.2.1/qemu-1.2.1/tcg/tcg-op.h:1267: error: 
'TCG_TARGET_HAS_div_i64' undeclared (first use in this function)
   
  /usr/obj/ports/qemu-1.2.1/qemu-1.2.1/tcg/tcg-op.h:1269: error: 
'TCG_TARGET_HAS_div2_i64' undeclared (first use in this function)
  /usr/obj/ports/qemu-1.2.1/qemu-1.2.1/tcg/tcg-op.h: In function 
'tcg_gen_remu_i64':
  /usr/obj/ports/qemu-1.2.1/qemu-1.2.1/tcg/tcg-op.h:1286: error: 
'TCG_TARGET_HAS_div_i64' undeclared (first use in this function)
   
  /usr/obj/ports/qemu-1.2.1/qemu-1.2.1/tcg/tcg-op.h:1288: error: 
'TCG_TARGET_HAS_div2_i64' undeclared (first use in this function)
  /usr/obj/ports/qemu-1.2.1/qemu-1.2.1/tcg/tcg-op.h: In function 
'tcg_gen_ext8s_i64':
  /usr/obj/ports/qemu-1.2.1/qemu-1.2.1/tcg/tcg-op.h:1526: error: 
'TCG_TARGET_HAS_ext8s_i64' undeclared (first use in this function)
  /usr/obj/ports/qemu-1.2.1/qemu-1.2.1/tcg/tcg-op.h: In function 
'tcg_gen_ext16s_i64':
  /usr/obj/ports/qemu-1.2.1/qemu-1.2.1/tcg/tcg-op.h:1536: error: 
'TCG_TARGET_HAS_ext16s_i64' undeclared (first use in this function)
  ...

  Attached is the full build log.

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

[Qemu-devel] [PULL 2/2] block/iscsi: enable filename option and parsing

[Jeff Cody]

When enabling option parsing and blockdev-add for iscsi, we removed the
'filename' option.  Unfortunately, this was a bit optimistic, as
previous versions of QEMU allowed the use of the option in backing
filenames via json.  This means that without parsing this option, we
cannot open existing images that used to work fine.

See bug: https://bugzilla.redhat.com/show_bug.cgi?id=1457088

Tested-by: Richard W.M. Jones 
Signed-off-by: Jeff Cody 
Message-id: 
0789ab6c32814ab4b6896707d378804bd4424c65.1497444637.git.jc...@redhat.com
Signed-off-by: Jeff Cody 
---
 block/iscsi.c | 22 +-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/block/iscsi.c b/block/iscsi.c
index 5daa201..b5f7a22 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -1732,6 +1732,10 @@ static QemuOptsList runtime_opts = {
 .name = "timeout",
 .type = QEMU_OPT_NUMBER,
 },
+{
+.name = "filename",
+.type = QEMU_OPT_STRING,
+},
 { /* end of list */ }
 },
 };
@@ -1747,12 +1751,27 @@ static int iscsi_open(BlockDriverState *bs, QDict 
*options, int flags,
 char *initiator_name = NULL;
 QemuOpts *opts;
 Error *local_err = NULL;
-const char *transport_name, *portal, *target;
+const char *transport_name, *portal, *target, *filename;
 #if LIBISCSI_API_VERSION >= (20160603)
 enum iscsi_transport_type transport;
 #endif
 int i, ret = 0, timeout = 0, lun;
 
+/* If we are given a filename, parse the filename, with precedence given to
+ * filename encoded options */
+filename = qdict_get_try_str(options, "filename");
+if (filename) {
+error_report("Warning: 'filename' option specified. "
+  "This is an unsupported option, and may be deprecated "
+  "in the future");
+iscsi_parse_filename(filename, options, _err);
+if (local_err) {
+ret = -EINVAL;
+error_propagate(errp, local_err);
+goto exit;
+}
+}
+
 opts = qemu_opts_create(_opts, NULL, 0, _abort);
 qemu_opts_absorb_qdict(opts, options, _err);
 if (local_err) {
@@ -1967,6 +1986,7 @@ out:
 }
 memset(iscsilun, 0, sizeof(IscsiLun));
 }
+exit:
 return ret;
 }
 
-- 
2.9.3

[Qemu-devel] [PULL 0/2] Block patches

[Jeff Cody]

The following changes since commit 3f0602927b120a480b35dcf58cf6f95435b3ae91:

  Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20170613' 
into staging (2017-06-13 15:49:07 +0100)

are available in the git repository at:

  git://github.com/codyprime/qemu-kvm-jtc.git tags/block-pull-request

for you to fetch changes up to 5c3ad1a6a8fa041c57403dbe1fc5927eec0be66b:

  block/iscsi: enable filename option and parsing (2017-06-14 17:39:46 -0400)


Block patches


Jeff Cody (2):
  block/rbd: enable filename option and parsing
  block/iscsi: enable filename option and parsing

 block/iscsi.c | 22 +-
 block/rbd.c   | 22 +-
 2 files changed, 42 insertions(+), 2 deletions(-)

-- 
2.9.3

[Qemu-devel] [PULL 1/2] block/rbd: enable filename option and parsing

[Jeff Cody]

When enabling option parsing and blockdev-add for rbd, we removed the
'filename' option.  Unfortunately, this was a bit optimistic, as
previous versions of QEMU allowed the use of the option in backing
filenames via json.  This means that without parsing this option, we
cannot open existing images that used to work fine.

See bug: https://bugzilla.redhat.com/show_bug.cgi?id=1457088

Tested-by: Richard W.M. Jones 
Signed-off-by: Jeff Cody 
Message-id: 
937dc9fde348d13311eb8e23444df3bc3190b612.1497444637.git.jc...@redhat.com
Signed-off-by: Jeff Cody 
---
 block/rbd.c | 22 +-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/block/rbd.c b/block/rbd.c
index e551639..ff44e5f 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -340,6 +340,10 @@ static QemuOptsList runtime_opts = {
 .type = QEMU_OPT_STRING,
 .help = "Legacy rados key/value option parameters",
 },
+{
+.name = "filename",
+.type = QEMU_OPT_STRING,
+},
 { /* end of list */ }
 },
 };
@@ -541,12 +545,27 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict 
*options, int flags,
 {
 BDRVRBDState *s = bs->opaque;
 const char *pool, *snap, *conf, *user, *image_name, *keypairs;
-const char *secretid;
+const char *secretid, *filename;
 QemuOpts *opts;
 Error *local_err = NULL;
 char *mon_host = NULL;
 int r;
 
+/* If we are given a filename, parse the filename, with precedence given to
+ * filename encoded options */
+filename = qdict_get_try_str(options, "filename");
+if (filename) {
+error_report("Warning: 'filename' option specified. "
+  "This is an unsupported option, and may be deprecated "
+  "in the future");
+qemu_rbd_parse_filename(filename, options, _err);
+if (local_err) {
+r = -EINVAL;
+error_propagate(errp, local_err);
+goto exit;
+}
+}
+
 opts = qemu_opts_create(_opts, NULL, 0, _abort);
 qemu_opts_absorb_qdict(opts, options, _err);
 if (local_err) {
@@ -665,6 +684,7 @@ failed_shutdown:
 failed_opts:
 qemu_opts_del(opts);
 g_free(mon_host);
+exit:
 return r;
 }
 
-- 
2.9.3

Re: [Qemu-devel] [PATCH v2 0/2] Parse 'filename' option for RBD/iSCSI

[Jeff Cody]

On Wed, Jun 14, 2017 at 08:53:18AM -0400, Jeff Cody wrote:
> Change from v2:
> Add warning message that this is an unsupported option that may
> be deprecated in the future.
> 
> We need to be able to parse the 'filename' option for rbd and iscsi, because
> there may exist images in the wild that have json backing files, that specify
> the filename argument.
> 
> Jeff Cody (2):
>   block/rbd: enable filename option and parsing
>   block/iscsi: enable filename option and parsing
> 
>  block/iscsi.c | 22 +-
>  block/rbd.c   | 22 +-
>  2 files changed, 42 insertions(+), 2 deletions(-)
> 
> -- 
> 2.9.3
> 

Thanks,

Applied to my block branch:

git://github.com/codyprime/qemu-kvm-jtc block

-Jeff

Re: [Qemu-devel] [PATCH v6 1/6] Pass generic CPUState to gen_intermediate_code()

[Eduardo Habkost]

On Mon, Jun 12, 2017 at 05:53:55PM +0300, Lluís Vilanova wrote:
> Needed to implement a target-agnostic gen_intermediate_code() in the
> future.
> 
> Signed-off-by: Lluís Vilanova 
> Reviewed-by: David Gibson 
> Reviewed-by: Richard Henderson 

Acked-by: Eduardo Habkost 

For i386 parts:

Reviewed-by: Eduardo Habkost 

-- 
Eduardo

[Qemu-devel] [Bug 1096713] Re: qemu 1.3.0: Windows XP crashes when reconizing the USB keyboard

[Thomas Huth]

Triaging old bug tickets ... can you still reproduce this problem with
the latest version of QEMU (currently version 2.9.0)?

** Changed in: qemu
   Status: New => Incomplete

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

Title:
  qemu 1.3.0: Windows XP crashes when reconizing the USB keyboard

Status in QEMU:
  Incomplete

Bug description:
  I'm trying to use the usb tablet and the usb keyboard as follows:
  ./qemu-system-i386  -device pci-ohci -device usb-tablet -device usb-kbd
  or
  ./qemu-system-i386  -device ich9-usb-ehci1 -device ich9-usb-uhci1 -device 
usb-tablet -device ich9-usb-uhci2 -device usb-kbd

  While Windows XP works fine if I only use the tablet but not the
  keyboard, it crashed with a BSOD when I use both keyboard and tablet.
  It crashed during the detection of the keyboard.

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

Re: [Qemu-devel] [PATCH 4/4] include/exec/poison: Mark CONFIG_KVM as poisoned, too

[Paolo Bonzini]

- Original Message -
> From: "Thomas Huth" 
> To: qemu-devel@nongnu.org, "Paolo Bonzini" 
> Sent: Wednesday, June 14, 2017 9:21:53 PM
> Subject: [PATCH 4/4] include/exec/poison: Mark CONFIG_KVM as poisoned, too
> 
> We unfortunately need some additional "#ifndef NEED_CPU_H" fuzz in
> include/sysemu/kvm.h for this, so that the header can still be included
> from common code (which is done all over the place), but now we can
> finally be sure that nobody uses this define in a wrong place anymore.
> 
> Signed-off-by: Thomas Huth 
> ---
>  hw/acpi/ich9.c|  1 -
>  include/exec/poison.h |  1 +
>  include/sysemu/kvm.h  | 37 +++--
>  3 files changed, 36 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
> index 5c279bb..c5d8646 100644
> --- a/hw/acpi/ich9.c
> +++ b/hw/acpi/ich9.c
> @@ -33,7 +33,6 @@
>  #include "sysemu/sysemu.h"
>  #include "hw/acpi/acpi.h"
>  #include "hw/acpi/tco.h"
> -#include "sysemu/kvm.h"
>  #include "exec/address-spaces.h"
>  
>  #include "hw/i386/ich9.h"
> diff --git a/include/exec/poison.h b/include/exec/poison.h
> index 5ffed4d..540fc70 100644
> --- a/include/exec/poison.h
> +++ b/include/exec/poison.h
> @@ -80,6 +80,7 @@
>  
>  #pragma GCC poison CONFIG_LINUX_USER
>  #pragma GCC poison CONFIG_VHOST_NET
> +#pragma GCC poison CONFIG_KVM
>  
>  #endif
>  #endif
> diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
> index 1e91613..8cc57e4 100644
> --- a/include/sysemu/kvm.h
> +++ b/include/sysemu/kvm.h
> @@ -19,6 +19,8 @@
>  #include "exec/memattrs.h"
>  #include "hw/irq.h"
>  
> +#ifdef NEED_CPU_H
> +
>  #ifdef CONFIG_KVM
>  #include 
>  #include 
> @@ -39,6 +41,8 @@
>  #define KVM_FEATURE_CLOCKSOURCE_STABLE_BIT 0

These defines can be moved to kvm_i386.h too.

>  #endif
>  
> +#endif /* NEED_CPU_H */
> +
>  extern bool kvm_allowed;
>  extern bool kvm_kernel_irqchip;
>  extern bool kvm_split_irqchip;
> @@ -55,7 +59,8 @@ extern bool kvm_direct_msi_allowed;
>  extern bool kvm_ioeventfd_any_length_allowed;
>  extern bool kvm_msi_use_devid;
>  
> -#if defined CONFIG_KVM || !defined NEED_CPU_H
> +#if !defined(NEED_CPU_H)
> +
>  #define kvm_enabled()   (kvm_allowed)
>  /**
>   * kvm_irqchip_in_kernel:
> @@ -178,6 +183,31 @@ extern bool kvm_msi_use_devid;
>  #define kvm_msi_devid_required() (kvm_msi_use_devid)
>  
>  #else
> +
> +#ifdef CONFIG_KVM
> +
> +/*
> + * Same definitions again, but we need to keep them separate
> + * since CONFIG_KVM is poisoned without NEED_CPU_H
> + */

Maybe

#ifndef NEED_CPU_H
# define CONFIG_KVM_POSSIBLE
#elif defined CONFIG_KVM
# define CONFIG_KVM_POSSIBLE
#endif

#ifdef CONFIG_KVM_POSSIBLE
#define kvm_enabled()   (kvm_allowed)
...
#else
#define kvm_enabled()   0
...
#endif

?

> +#define kvm_enabled()   (kvm_allowed)
> +#define kvm_irqchip_in_kernel() (kvm_kernel_irqchip)
> +#define kvm_irqchip_is_split() (kvm_split_irqchip)
> +#define kvm_async_interrupts_enabled() (kvm_async_interrupts_allowed)
> +#define kvm_halt_in_kernel() (kvm_halt_in_kernel_allowed)
> +#define kvm_eventfds_enabled() (kvm_eventfds_allowed)
> +#define kvm_irqfds_enabled() (kvm_irqfds_allowed)
> +#define kvm_resamplefds_enabled() (kvm_resamplefds_allowed)
> +#define kvm_msi_via_irqfd_enabled() (kvm_msi_via_irqfd_allowed)
> +#define kvm_gsi_routing_enabled() (kvm_gsi_routing_allowed)
> +#define kvm_gsi_direct_mapping() (kvm_gsi_direct_mapping)
> +#define kvm_readonly_mem_enabled() (kvm_readonly_mem_allowed)
> +#define kvm_direct_msi_enabled() (kvm_direct_msi_allowed)
> +#define kvm_ioeventfd_any_length_enabled()
> (kvm_ioeventfd_any_length_allowed)
> +#define kvm_msi_devid_required() (kvm_msi_use_devid)
> +
> +#else
> +
>  #define kvm_enabled()   (0)
>  #define kvm_irqchip_in_kernel() (false)
>  #define kvm_irqchip_is_split() (false)
> @@ -193,7 +223,10 @@ extern bool kvm_msi_use_devid;
>  #define kvm_direct_msi_enabled() (false)
>  #define kvm_ioeventfd_any_length_enabled() (false)
>  #define kvm_msi_devid_required() (false)
> -#endif
> +
> +#endif  /* CONFIG_KVM */
> +
> +#endif  /* NEED_CPU_H */
>  
>  struct kvm_run;
>  struct kvm_lapic_state;
> --
> 1.8.3.1
> 
> 

[Qemu-devel] [PATCH] target/m68k: fix V flag for CC_OP_SUBx

[Laurent Vivier]

V flag for subtraction is:

   v = (res ^ src1) & (src1 ^ src2)

(see COMPUTE_CCR() in target/m68k/helper.c)

But gen_flush_flags() uses:

   v = (res ^ src2) & (src1 ^ src2)

The problem has been found with the following program:

.global _start
_start:
move.l  #-2147483648,%d0
subq.l  #1,%d0
jvc 1f
move.l #1,%d1
move.l #1,%d0
trap #0
1:
move.l #0,%d1
move.l #1,%d0
trap #0

It works fine (exit(1)) on real hardware, and with "-singlestep".

"-singlestep" uses gen_helper_flush_flags(), whereas
without "-singlestep", V flag is computed directly in
gen_flush_flags().

This patch updates gen_flush_flags() to have the same result
as with gen_helper_flush_flags().

Signed-off-by: Laurent Vivier 
---
 target/m68k/translate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/m68k/translate.c b/target/m68k/translate.c
index ad4d4ef..52653f6 100644
--- a/target/m68k/translate.c
+++ b/target/m68k/translate.c
@@ -565,7 +565,7 @@ static void gen_flush_flags(DisasContext *s)
 t1 = tcg_temp_new();
 tcg_gen_add_i32(t0, QREG_CC_N, QREG_CC_V);
 gen_ext(t0, t0, s->cc_op - CC_OP_SUBB, 1);
-tcg_gen_xor_i32(t1, QREG_CC_N, QREG_CC_V);
+tcg_gen_xor_i32(t1, QREG_CC_N, t0);
 tcg_gen_xor_i32(QREG_CC_V, QREG_CC_V, t0);
 tcg_temp_free(t0);
 tcg_gen_and_i32(QREG_CC_V, QREG_CC_V, t1);
-- 
2.9.4

Re: [Qemu-devel] [PATCH v2 2/5] target/alpha: Use tcg_gen_lookup_and_goto_ptr

[Emilio G. Cota]

On Wed, Jun 14, 2017 at 12:48:18 -0700, Richard Henderson wrote:
> Signed-off-by: Richard Henderson 
(snip)
> @@ -1198,7 +1205,10 @@ static ExitStatus gen_call_pal(DisasContext *ctx, int 
> palcode)
>  tcg_gen_andi_i64(tmp, ctx->ir[IR_A0], PS_INT_MASK);
>  tcg_gen_st8_i64(tmp, cpu_env, offsetof(CPUAlphaState, ps));
>  tcg_temp_free(tmp);
> -break;
> +
> +/* Allow interrupts to be recognized right away.  */
> +tcg_gen_movi_i64(cpu_pc, ctx.pc);

ctx->pc though

Tested-by: Emilio G. Cota 

Thanks!

E.

[Qemu-devel] [PATCH 5/5] hostmem-file: Add "persistent" option

[Eduardo Habkost]

The new option can be used to indicate that the memory block contents
can be safely discarded and don't need to be flushed to the filesystem
when the memory backend is destroyed (including when QEMU exits).

Internally, it will trigger a madvise(MADV_REMOVE) or
fallocate(FALLOC_FL_PUNCH_HOLE) call when the memory backend is
destroyed.

Signed-off-by: Eduardo Habkost 
---
 backends/hostmem-file.c | 35 ++-
 qemu-options.hx |  9 -
 2 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
index d078775b4b..e6cb232349 100644
--- a/backends/hostmem-file.c
+++ b/backends/hostmem-file.c
@@ -32,6 +32,7 @@ struct HostMemoryBackendFile {
 HostMemoryBackend parent_obj;
 
 bool share;
+bool persistent;
 char *mem_path;
 };
 
@@ -57,7 +58,7 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error 
**errp)
 path = object_get_canonical_path(OBJECT(backend));
 memory_region_init_ram_from_file(>mr, OBJECT(backend),
  path,
- backend->size, fb->share, true,
+ backend->size, fb->share, fb->persistent,
  fb->mem_path, errp);
 g_free(path);
 }
@@ -103,6 +104,26 @@ static void file_memory_backend_set_share(Object *o, bool 
value, Error **errp)
 fb->share = value;
 }
 
+static bool file_memory_backend_get_persistent(Object *o, Error **errp)
+{
+HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(o);
+
+return fb->persistent;
+}
+
+static void file_memory_backend_set_persistent(Object *o, bool value,
+   Error **errp)
+{
+HostMemoryBackend *backend = MEMORY_BACKEND(o);
+HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(o);
+
+if (host_memory_backend_mr_inited(backend)) {
+error_setg(errp, "cannot change property value");
+return;
+}
+fb->persistent = value;
+}
+
 static void
 file_backend_class_init(ObjectClass *oc, void *data)
 {
@@ -110,14 +131,25 @@ file_backend_class_init(ObjectClass *oc, void *data)
 
 bc->alloc = file_backend_memory_alloc;
 
+
 object_class_property_add_bool(oc, "share",
 file_memory_backend_get_share, file_memory_backend_set_share,
 _abort);
+object_class_property_add_bool(oc, "persistent",
+file_memory_backend_get_persistent, file_memory_backend_set_persistent,
+_abort);
 object_class_property_add_str(oc, "mem-path",
 get_mem_path, set_mem_path,
 _abort);
 }
 
+static void file_backend_instance_init(Object *o)
+{
+HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(o);
+
+fb->persistent = true;
+}
+
 static void file_backend_instance_finalize(Object *o)
 {
 HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(o);
@@ -129,6 +161,7 @@ static const TypeInfo file_backend_info = {
 .name = TYPE_MEMORY_BACKEND_FILE,
 .parent = TYPE_MEMORY_BACKEND,
 .class_init = file_backend_class_init,
+.instance_init = file_backend_instance_init,
 .instance_finalize = file_backend_instance_finalize,
 .instance_size = sizeof(HostMemoryBackendFile),
 };
diff --git a/qemu-options.hx b/qemu-options.hx
index 30c4f9850f..7fb4feec9b 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -3961,7 +3961,7 @@ property must be set.  These objects are placed in the
 
 @table @option
 
-@item -object 
memory-backend-file,id=@var{id},size=@var{size},mem-path=@var{dir},share=@var{on|off}
+@item -object 
memory-backend-file,id=@var{id},size=@var{size},mem-path=@var{dir},share=@var{on|off},persistent=@var{on|off}
 
 Creates a memory file backend object, which can be used to back
 the guest RAM with huge pages. The @option{id} parameter is a
@@ -3973,6 +3973,13 @@ the path to either a shared memory or huge page 
filesystem mount.
 The @option{share} boolean option determines whether the memory
 region is marked as private to QEMU, or shared. The latter allows
 a co-operating external process to access the QEMU memory region.
+Setting the @option{persistent} boolean option to @var{off}
+indicates that memory contents can be safely discarded and not
+flushed to disk when the backend object is destroyed or QEMU
+exits.  @option{persistent} is @var{on} by default.  It is valid
+to set @option{persistent} to @var{off} if @option{share} is
+@var{on}. It is redundant to set @option{persistent} to @var{off}
+if @option{share} is @var{off}.
 
 @item -object rng-random,id=@var{id},filename=@var{/dev/random}
 
-- 
2.11.0.259.g40922b1

[Qemu-devel] [PATCH] target/aarch64: exit to main loop after 'msr daifclr'

[Emilio G. Cota]

On Wed, Jun 14, 2017 at 12:48:21 -0700, Richard Henderson wrote:
> Exit to cpu loop so we reevaluate cpu_arm_hw_interrupts.
> 
> Cc: qemu-...@nongnu.org
> Cc: Peter Maydell 
> Signed-off-by: Richard Henderson 
> ---
>  target/arm/translate-a64.c | 7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
> index 860e279..e55547d 100644
> --- a/target/arm/translate-a64.c
> +++ b/target/arm/translate-a64.c
> @@ -1422,7 +1422,9 @@ static void handle_msr_i(DisasContext *s, uint32_t insn,
>  gen_helper_msr_i_pstate(cpu_env, tcg_op, tcg_imm);
>  tcg_temp_free_i32(tcg_imm);
>  tcg_temp_free_i32(tcg_op);
> -s->is_jmp = DISAS_UPDATE;
> +/* For DAIFClear, exit the cpu loop to re-evaluate pending IRQs.  */
> +gen_a64_set_pc_im(s->pc);

For op != 0x1f we end up setting the pc twice (first here, then in
the switch statement). It's still correct though.

> +s->is_jmp = (op == 0x1f ? DISAS_EXIT : DISAS_JUMP);

Could do without the parens.

>  break;
>  }
>  default:
> @@ -11369,6 +11371,9 @@ void gen_intermediate_code_a64(ARMCPU *cpu, 
> TranslationBlock *tb)
>  case DISAS_JUMP:
>  tcg_gen_lookup_and_goto_ptr(cpu_pc);
>  break;
> +case DISAS_EXIT:
> +tcg_gen_exit_tb(0);
> +break;

We could also mention the regression in the commit log. In fact
I just did that :-) feel free to reuse parts of the below.

Thanks,

E.

--- 8< ---

Commit e75449a3 ("target/aarch64: optimize indirect branches") causes
a regression by which -smp > 1 freezes under TCG, even with
`-accel accel=tcg,thread=single' (i.e. MTTCG disabled).

The cause of the regression is well described here by Alex:
On Wed, Jun 14, 2017 at 15:02:06 +0100, Alex Bennée wrote:
> Fundamentally the problem was that an interrupt was pending
> (interrupt_request was set) but the "msr daifclr" operations when the
> kernel did local_irq/fiq_enable() never got handled because the
> cpu_idle loop was being very efficiently chained. As a result we never
> got around to exiting the TCG code and calling arm_cpu_do_interrupt
> which would then raise the IRQ to move things on
> [ Message-Id: <20170614140209.29847-1-alex.ben...@linaro.org> ]

Fix it by enforcing an exit to the main loop right after 'msr daifclr'
is executed.

Signed-off-by: Emilio G. Cota 
---
 target/arm/translate-a64.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index 860e279..323e419 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -1422,7 +1422,8 @@ static void handle_msr_i(DisasContext *s, uint32_t insn,
 gen_helper_msr_i_pstate(cpu_env, tcg_op, tcg_imm);
 tcg_temp_free_i32(tcg_imm);
 tcg_temp_free_i32(tcg_op);
-s->is_jmp = DISAS_UPDATE;
+/* force exit to the main loop for DAIFClear */
+s->is_jmp = op == 0x1f ? DISAS_EXIT : DISAS_UPDATE;
 break;
 }
 default:
@@ -11362,6 +11363,10 @@ void gen_intermediate_code_a64(ARMCPU *cpu, 
TranslationBlock *tb)
 case DISAS_NEXT:
 gen_goto_tb(dc, 1, dc->pc);
 break;
+case DISAS_EXIT:
+gen_a64_set_pc_im(dc->pc);
+tcg_gen_exit_tb(0);
+break;
 default:
 case DISAS_UPDATE:
 gen_a64_set_pc_im(dc->pc);
-- 
2.7.4

[Qemu-devel] [PATCH 1/5] vl: Clean up user-creatable objects when exiting

[Eduardo Habkost]

Delete all user-creatable objects in /objects when exiting QEMU, so they
can perform cleanup actions.

Signed-off-by: Eduardo Habkost 
---
 include/qom/object_interfaces.h | 8 
 qom/object_interfaces.c | 5 +
 vl.c| 1 +
 3 files changed, 14 insertions(+)

diff --git a/include/qom/object_interfaces.h b/include/qom/object_interfaces.h
index fdd7603c84..3f5f206921 100644
--- a/include/qom/object_interfaces.h
+++ b/include/qom/object_interfaces.h
@@ -148,4 +148,12 @@ int user_creatable_add_opts_foreach(void *opaque,
  */
 void user_creatable_del(const char *id, Error **errp);
 
+/**
+ * user_creatable_cleanup:
+ *
+ * Delete all user-creatable objects and the user-creatable
+ * objects container.
+ */
+void user_creatable_cleanup(void);
+
 #endif
diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
index ff27e0669e..dbf8878322 100644
--- a/qom/object_interfaces.c
+++ b/qom/object_interfaces.c
@@ -193,6 +193,11 @@ void user_creatable_del(const char *id, Error **errp)
 object_unparent(obj);
 }
 
+void user_creatable_cleanup(void)
+{
+object_unparent(object_get_objects_root());
+}
+
 static void register_types(void)
 {
 static const TypeInfo uc_interface_info = {
diff --git a/vl.c b/vl.c
index 32db19e3b9..4e9a7fd05a 100644
--- a/vl.c
+++ b/vl.c
@@ -4762,6 +4762,7 @@ int main(int argc, char **argv, char **envp)
 audio_cleanup();
 monitor_cleanup();
 qemu_chr_cleanup();
+user_creatable_cleanup();
 /* TODO: unref root container, check all devices are ok */
 
 return 0;
-- 
2.11.0.259.g40922b1

[Qemu-devel] [PATCH 4/5] memory: Add 'persistent' parameter to memory_region_init_ram_from_file()

[Eduardo Habkost]

Make it possible to set the RAM_NONPERSISTENT flag on the RAMBlock when
mapping a file.

Signed-off-by: Eduardo Habkost 
---
 include/exec/memory.h   | 4 
 include/exec/ram_addr.h | 4 ++--
 backends/hostmem-file.c | 2 +-
 exec.c  | 7 +--
 memory.c| 4 +++-
 numa.c  | 2 +-
 6 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/include/exec/memory.h b/include/exec/memory.h
index 80e605a96a..f47c534179 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -446,6 +446,9 @@ void memory_region_init_resizeable_ram(MemoryRegion *mr,
  *must be unique within any device
  * @size: size of the region.
  * @share: %true if memory must be mmaped with the MAP_SHARED flag
+ * @persistent: %false if RAM contents can be discarded and don't
+ *  need to be flushed to disk when the memory region
+ *  is freed.
  * @path: the path in which to allocate the RAM.
  * @errp: pointer to Error*, to store an error if it happens.
  */
@@ -454,6 +457,7 @@ void memory_region_init_ram_from_file(MemoryRegion *mr,
   const char *name,
   uint64_t size,
   bool share,
+  bool persistent,
   const char *path,
   Error **errp);
 #endif
diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
index 140efa840c..67dc099355 100644
--- a/include/exec/ram_addr.h
+++ b/include/exec/ram_addr.h
@@ -63,8 +63,8 @@ static inline void *ramblock_ptr(RAMBlock *block, ram_addr_t 
offset)
 long qemu_getrampagesize(void);
 unsigned long last_ram_page(void);
 RAMBlock *qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr,
-   bool share, const char *mem_path,
-   Error **errp);
+   bool share, bool persistent,
+   const char *mem_path, Error **errp);
 RAMBlock *qemu_ram_alloc_from_ptr(ram_addr_t size, void *host,
   MemoryRegion *mr, Error **errp);
 RAMBlock *qemu_ram_alloc(ram_addr_t size, MemoryRegion *mr, Error **errp);
diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
index fc4ef46d11..d078775b4b 100644
--- a/backends/hostmem-file.c
+++ b/backends/hostmem-file.c
@@ -57,7 +57,7 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error 
**errp)
 path = object_get_canonical_path(OBJECT(backend));
 memory_region_init_ram_from_file(>mr, OBJECT(backend),
  path,
- backend->size, fb->share,
+ backend->size, fb->share, true,
  fb->mem_path, errp);
 g_free(path);
 }
diff --git a/exec.c b/exec.c
index a6e9ed4ece..77734198d0 100644
--- a/exec.c
+++ b/exec.c
@@ -1937,8 +1937,8 @@ static void ram_block_add(RAMBlock *new_block, Error 
**errp)
 
 #ifdef __linux__
 RAMBlock *qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr,
-   bool share, const char *mem_path,
-   Error **errp)
+   bool share, bool persistent,
+   const char *mem_path, Error **errp)
 {
 RAMBlock *new_block;
 Error *local_err = NULL;
@@ -1965,6 +1965,9 @@ RAMBlock *qemu_ram_alloc_from_file(ram_addr_t size, 
MemoryRegion *mr,
 new_block->used_length = size;
 new_block->max_length = size;
 new_block->flags = share ? RAM_SHARED : 0;
+if (!persistent) {
+new_block->flags |= RAM_NONPERSISTENT;
+}
 new_block->host = file_ram_alloc(new_block, size,
  mem_path, errp);
 if (!new_block->host) {
diff --git a/memory.c b/memory.c
index 0ddc4cc28d..3c0c0ff141 100644
--- a/memory.c
+++ b/memory.c
@@ -1387,6 +1387,7 @@ void memory_region_init_ram_from_file(MemoryRegion *mr,
   const char *name,
   uint64_t size,
   bool share,
+  bool persistent,
   const char *path,
   Error **errp)
 {
@@ -1394,7 +1395,8 @@ void memory_region_init_ram_from_file(MemoryRegion *mr,
 mr->ram = true;
 mr->terminates = true;
 mr->destructor = memory_region_destructor_ram;
-mr->ram_block = qemu_ram_alloc_from_file(size, mr, share, path, errp);
+mr->ram_block = qemu_ram_alloc_from_file(size, mr, share, persistent,
+ path, errp);
 mr->dirty_log_mask = tcg_enabled() ? (1 << DIRTY_MEMORY_CODE) : 0;
 }
 #endif
diff --git a/numa.c b/numa.c
index 65701cb6c8..825d5933ca 100644

[Qemu-devel] [PATCH 0/5] hostmem-file: Add "persistent" option

[Eduardo Habkost]

This series adds a new "persistent" option to
memory-backend-file.  The new option it will be useful if
somebody is sharing RAM contents on a file using share=on, but
don't need it to be flushed to disk when QEMU exits.

Internally, it will trigger a madvise(MADV_REMOVE) or
fallocate(FALLOC_FL_PUNCH_HOLE) call when the memory backend is
destroyed.

To make we actually trigger the new code when QEMU exits, the
first patch in the series ensures we destroy all user-created
objects when exiting QEMU.

Eduardo Habkost (5):
  vl: Clean up user-creatable objects when exiting
  memory: Allow RAM up to block->max_length to be discarded
  memory: Add RAM_NONPERSISTENT flag
  memory: Add 'persistent' parameter to
memory_region_init_ram_from_file()
  hostmem-file: Add "persistent" option

 include/exec/memory.h   |  4 
 include/exec/ram_addr.h |  4 ++--
 include/qom/object_interfaces.h |  8 
 backends/hostmem-file.c | 35 ++-
 exec.c  | 26 ++
 memory.c|  4 +++-
 numa.c  |  2 +-
 qom/object_interfaces.c |  5 +
 vl.c|  1 +
 qemu-options.hx |  9 -
 10 files changed, 88 insertions(+), 10 deletions(-)

-- 
2.11.0.259.g40922b1

[Qemu-devel] [PATCH 3/5] memory: Add RAM_NONPERSISTENT flag

[Eduardo Habkost]

The new flag will make qemu_ram_free() discard the contents of the
block.  It will be used to let QEMU be configured to avoid flushing file
contents to disk when exiting.  As MADV_REMOVE is not always supported,
the new code will try MADV_NOTNEEDED in case MADV_REMOVE fails.

The new flag will also indicate that ram_block_discard_range() can use
MADV_REMOVE when discarding memory pages.  I have considered calling
MADV_REMOVE unconditionally (as destroying the RAM contents seems to be
OK every time ram_block_discard_range() is called), but for safety I
decided to restrict the new code to blocks having RAM_NONPERSISTENT set.

Signed-off-by: Eduardo Habkost 
---
 exec.c | 17 -
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/exec.c b/exec.c
index 585d6ed6d7..a6e9ed4ece 100644
--- a/exec.c
+++ b/exec.c
@@ -102,6 +102,11 @@ static MemoryRegion io_mem_unassigned;
  */
 #define RAM_RESIZEABLE (1 << 2)
 
+/* RAMBlock contents are not persistent, and we can discard memory contents
+ * when freeing the memory block.
+ */
+#define RAM_NONPERSISTENT (1 << 3)
+
 #endif
 
 #ifdef TARGET_PAGE_BITS_VARY
@@ -2061,6 +2066,10 @@ void qemu_ram_free(RAMBlock *block)
 ram_block_notify_remove(block->host, block->max_length);
 }
 
+if (block->flags & RAM_NONPERSISTENT) {
+ram_block_discard_range(block, 0, block->max_length);
+}
+
 qemu_mutex_lock_ramlist();
 QLIST_REMOVE_RCU(block, next);
 ram_list.mru_block = NULL;
@@ -3537,7 +3546,13 @@ int ram_block_discard_range(RAMBlock *rb, uint64_t 
start, size_t length)
 /* Note: We need the madvise MADV_DONTNEED behaviour of definitely
  * freeing the page.
  */
-ret = madvise(host_startaddr, length, MADV_DONTNEED);
+if (rb->flags & RAM_NONPERSISTENT) {
+ret = madvise(host_startaddr, length, MADV_REMOVE);
+}
+/* Fallback to MADV_DONTNEED if MADV_REMOVE fails */
+if (ret || !(rb->flags & RAM_NONPERSISTENT)) {
+ret = madvise(host_startaddr, length, MADV_DONTNEED);
+}
 #endif
 } else {
 /* Huge page case  - unfortunately it can't do DONTNEED, but
-- 
2.11.0.259.g40922b1

[Qemu-devel] [PATCH 2/5] memory: Allow RAM up to block->max_length to be discarded

[Eduardo Habkost]

Currently ram_block_discard_range() is called only by the postcopy code,
using length=block->used_length.  However, new code will use
ram_block_discard_range() to discard the contents of the entire
RAMBlock, so change the limit check to use max_length instead of
used_length.

Signed-off-by: Eduardo Habkost 
---
 exec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/exec.c b/exec.c
index a93e209625..585d6ed6d7 100644
--- a/exec.c
+++ b/exec.c
@@ -3522,7 +3522,7 @@ int ram_block_discard_range(RAMBlock *rb, uint64_t start, 
size_t length)
 goto err;
 }
 
-if ((start + length) <= rb->used_length) {
+if ((start + length) <= rb->max_length) {
 uint8_t *host_endaddr = host_startaddr + length;
 if ((uintptr_t)host_endaddr & (rb->page_size - 1)) {
 error_report("ram_block_discard_range: Unaligned end address: %p",
-- 
2.11.0.259.g40922b1

[Qemu-devel] [PATCH] tcg-runtime: increase hit rate of lookup_tb_ptr

[Emilio G. Cota]

On Wed, Jun 14, 2017 at 12:48:17 -0700, Richard Henderson wrote:
> We can call tb_htable_lookup even when the tb_jmp_cache
> is completely empty.  Therefore, un-nest most of the code
> dependent on tb != NULL from the read from the cache.
> 
> Signed-off-by: Richard Henderson 

I just wrote this alternative patch, which does the same thing
as yours. I also measured what the effect of this change
has on the hit rate of lookup_tb_ptr. Feel free to reuse parts
of the patch and/or the commit message!

Thanks,

E.

--- 8< ---

Strangely, we do not look up the tb in the global hash table
when we get NULL from tb_jmp_cache.

Fix it, which improves the hit rate of lookup_tb_ptr; for instance,
when booting and immediately shutting down debian-arm, the hit
rate improves from
93.150742% (before this patch)
to
99.451323 % (after).

While at it, use a variable for the tb_jmp_cache hash and get rid
of the goto's.

Suggested-by: Richard Henderson 
Suggested-by: Alex Bennée 
Signed-off-by: Emilio G. Cota 
---
 tcg-runtime.c | 20 
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/tcg-runtime.c b/tcg-runtime.c
index 7fa90ce..09324b9 100644
--- a/tcg-runtime.c
+++ b/tcg-runtime.c
@@ -149,23 +149,19 @@ void *HELPER(lookup_tb_ptr)(CPUArchState *env, 
target_ulong addr)
 CPUState *cpu = ENV_GET_CPU(env);
 TranslationBlock *tb;
 target_ulong cs_base, pc;
+unsigned int hash = tb_jmp_cache_hash_func(addr);
 uint32_t flags;
 
-tb = atomic_rcu_read(>tb_jmp_cache[tb_jmp_cache_hash_func(addr)]);
-if (likely(tb)) {
-cpu_get_tb_cpu_state(env, , _base, );
-if (likely(tb->pc == addr && tb->cs_base == cs_base &&
-   tb->flags == flags)) {
-goto found;
-}
+tb = atomic_rcu_read(>tb_jmp_cache[hash]);
+cpu_get_tb_cpu_state(env, , _base, );
+if (unlikely(tb == NULL || tb->pc != addr || tb->cs_base != cs_base ||
+   tb->flags != flags)) {
 tb = tb_htable_lookup(cpu, addr, cs_base, flags);
-if (likely(tb)) {
-atomic_set(>tb_jmp_cache[tb_jmp_cache_hash_func(addr)], tb);
-goto found;
+if (unlikely(tb == NULL)) {
+return tcg_ctx.code_gen_epilogue;
 }
+atomic_set(>tb_jmp_cache[hash], tb);
 }
-return tcg_ctx.code_gen_epilogue;
- found:
 qemu_log_mask_and_addr(CPU_LOG_EXEC, addr,
"Chain %p [%d: " TARGET_FMT_lx "] %s\n",
tb->tc_ptr, cpu->cpu_index, addr,
-- 
2.7.4

Re: [Qemu-devel] [PATCH v2 0/5] Fixes for TCG hangs

[no-reply]

Hi,

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

Message-id: 20170614194821.8754-1-...@twiddle.net
Subject: [Qemu-devel] [PATCH v2 0/5] Fixes for TCG hangs
Type: series

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

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

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

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

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

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
43fb9b8 target/arm: Exit after clearing interrupt mask
11693ca target/s390x: Exit after changing PSW mask
0a2e4e4 target/mips: Exit after enabling interrupts
c147794 target/alpha: Use tcg_gen_lookup_and_goto_ptr
53499be tcg: Refactor helper_lookup_tb_ptr

=== OUTPUT BEGIN ===
Checking PATCH 1/5: tcg: Refactor helper_lookup_tb_ptr...
Checking PATCH 2/5: target/alpha: Use tcg_gen_lookup_and_goto_ptr...
ERROR: return is not a function, parentheses are not required
#27: FILE: target/alpha/translate.c:464:
+return ((ctx->tb->cflags & CF_LAST_IO)

total: 1 errors, 0 warnings, 66 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/5: target/mips: Exit after enabling interrupts...
Checking PATCH 4/5: target/s390x: Exit after changing PSW mask...
Checking PATCH 5/5: target/arm: Exit after clearing interrupt mask...
=== OUTPUT END ===

Test command exited with code: 1


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

Re: [Qemu-devel] [PATCH v2 2/2] target/s390x: implement mvcos instruction

[Thomas Huth]

On 14.06.2017 15:38, David Hildenbrand wrote:
> This adds support for the MOVE WITH OPTIONAL SPECIFICATIONS (MVCOS)
> instruction. Allow to enable it for the qemu cpu model using
> 
> qemu-system-s390x ... -cpu qemu,mvcos=on ...
> 
> This allows to boot linux kernel that uses it for uacccess.
> 
> We are missing (as for most other part) low address protection checks,
> PSW key / storage key checks and support for AR-mode.
> 
> We fake an ADDRESSING exception when called from problem state (which
> seems to rely on PSW key checks to be in place) and if AR-mode is used.
> user mode will always see a PRIVILEDGED exception.
> 
> This patch is based on an original patch by Miroslav Benes (thanks!).
> 
> Signed-off-by: David Hildenbrand 
> ---
[...]
> index 80caab9..5f76dae 100644
> --- a/target/s390x/mem_helper.c
> +++ b/target/s390x/mem_helper.c
> @@ -110,6 +110,20 @@ static inline void cpu_stsize_data_ra(CPUS390XState 
> *env, uint64_t addr,
>  }
>  }
>  
> +static inline uint64_t wrap_address(CPUS390XState *env, uint64_t a)
> +{
> +if (!(env->psw.mask & PSW_MASK_64)) {
> +if (!(env->psw.mask & PSW_MASK_32)) {
> +/* 24-Bit mode */
> +a &= 0x00ff;
> +} else {
> +/* 31-Bit mode */
> +a &= 0x7fff;
> +}
> +}
> +return a;
> +}
> +
>  static void fast_memset(CPUS390XState *env, uint64_t dest, uint8_t byte,
>  uint32_t l, uintptr_t ra)
>  {
> @@ -118,7 +132,7 @@ static void fast_memset(CPUS390XState *env, uint64_t 
> dest, uint8_t byte,
>  while (l > 0) {
>  void *p = tlb_vaddr_to_host(env, dest, MMU_DATA_STORE, mmu_idx);
>  if (p) {
> -/* Access to the whole page in write mode granted.  */
> +/* Access to the whole page in write mode granted  */

Unrelated change ;-)

>  uint32_t l_adj = adj_len_to_page(l, dest);
>  memset(p, byte, l_adj);
>  dest += l_adj;
> @@ -133,6 +147,68 @@ static void fast_memset(CPUS390XState *env, uint64_t 
> dest, uint8_t byte,
>  }
>  }
>  
> +#ifndef CONFIG_USER_ONLY
> +static void fast_memmove_idx(CPUS390XState *env, uint64_t dest, uint64_t src,
> + uint32_t len, int dest_idx, int src_idx,
> + uintptr_t ra)
> +{
> +TCGMemOpIdx oi_dest = make_memop_idx(MO_UB, dest_idx);
> +TCGMemOpIdx oi_src = make_memop_idx(MO_UB, src_idx);
> +uint32_t len_adj;
> +void *src_p;
> +void *dest_p;
> +uint8_t x;
> +
> +while (len > 0) {
> +src = wrap_address(env, src);
> +dest = wrap_address(env, dest);
> +src_p = tlb_vaddr_to_host(env, src, MMU_DATA_LOAD, src_idx);
> +dest_p = tlb_vaddr_to_host(env, dest, MMU_DATA_STORE, dest_idx);
> +
> +if (src_p && dest_p) {
> +/* Access to both whole pages granted.  */
> +len_adj = adj_len_to_page(adj_len_to_page(len, src), dest);
> +memmove(dest_p, src_p, len_adj);
> +} else {
> +/* We failed to get access to one or both whole pages. The next
> +   read or write access will likely fill the QEMU TLB for the
> +   next iteration.  */
> +len_adj = 1;
> +x = helper_ret_ldub_mmu(env, src, oi_src, ra);
> +helper_ret_stb_mmu(env, dest, x, oi_dest, ra);
> +}
> +src += len_adj;
> +dest += len_adj;
> +len -= len_adj;
> +}
> +}

The code is pretty similar to fast_memmove() ... I wonder whether it makes
sense to change fast_memmove() to call fast_memmove_idx(), too?

Something like:

static void fast_memmove(CPUS390XState *env, uint64_t dest, uint64_t src,
 uint32_t l, uintptr_t ra)
{
int mmu_idx = cpu_mmu_index(env, false);

fast_memmove_idx(env, dest, src, l, mmu_idx, mmu_idx, ra);
}

Or could that result in some speed penalty here?
Anyway, this should likely be done in a separate patch.

> +static int mmu_idx_from_as(uint8_t as)
> +{
> +switch (as) {
> +case AS_PRIMARY:
> +return MMU_PRIMARY_IDX;
> +case AS_SECONDARY:
> +return MMU_SECONDARY_IDX;
> +case AS_HOME:
> +return MMU_HOME_IDX;
> +}
> +/* FIXME AS_ACCREG */
> +assert(false);
> +return -1;
> +}

Hmm, maybe it would make sense to change the MMU_*_IDX defines to match
the AS value directly?

> +static void fast_memmove_as(CPUS390XState *env, uint64_t dest, uint64_t src,
> +uint32_t len, uint8_t dest_as, uint8_t src_as,
> +uintptr_t ra)
> +{
> +int src_idx = mmu_idx_from_as(src_as);
> +int dest_idx = mmu_idx_from_as(dest_as);
> +
> +fast_memmove_idx(env, dest, src, len, dest_idx, src_idx, ra);
> +}
> +#endif
> +
>  static void fast_memmove(CPUS390XState *env, uint64_t dest, uint64_t src,
>   uint32_t l, uintptr_t ra)
>  {
> @@ -408,20 +484,6 @@ uint32_t 

Re: [Qemu-devel] [PATCH v1 2/3] target/s390x: implement mvcos instruction

[Thomas Huth]

On 14.06.2017 09:56, David Hildenbrand wrote:
[...]
>> I think you should also mask the length with 0x if the PSW was
>> not in 64-bit mode? Or is this done automagically by the generated TCG
>> code already?
> 
> I was asking myself the same question, but it shouldn't really matter as
> was we will be using a maximum of 4096, no?

Question is whether we can end up here somehow in 32-bit mode and the
upper part of the register is still != 0 ... something like 0x10010
for example. Can we be sure that the upper half is always cleared if we
switch from 64-bit mode to the 32-bit mode before?

 Thomas

[Qemu-devel] [PATCH v2 5/5] target/arm: Exit after clearing interrupt mask

[Richard Henderson]

Exit to cpu loop so we reevaluate cpu_arm_hw_interrupts.

Cc: qemu-...@nongnu.org
Cc: Peter Maydell 
Signed-off-by: Richard Henderson 
---
 target/arm/translate-a64.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index 860e279..e55547d 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -1422,7 +1422,9 @@ static void handle_msr_i(DisasContext *s, uint32_t insn,
 gen_helper_msr_i_pstate(cpu_env, tcg_op, tcg_imm);
 tcg_temp_free_i32(tcg_imm);
 tcg_temp_free_i32(tcg_op);
-s->is_jmp = DISAS_UPDATE;
+/* For DAIFClear, exit the cpu loop to re-evaluate pending IRQs.  */
+gen_a64_set_pc_im(s->pc);
+s->is_jmp = (op == 0x1f ? DISAS_EXIT : DISAS_JUMP);
 break;
 }
 default:
@@ -11369,6 +11371,9 @@ void gen_intermediate_code_a64(ARMCPU *cpu, 
TranslationBlock *tb)
 case DISAS_JUMP:
 tcg_gen_lookup_and_goto_ptr(cpu_pc);
 break;
+case DISAS_EXIT:
+tcg_gen_exit_tb(0);
+break;
 case DISAS_TB_JUMP:
 case DISAS_EXC:
 case DISAS_SWI:
-- 
2.9.4

[Qemu-devel] [PATCH v2 4/5] target/s390x: Exit after changing PSW mask

[Richard Henderson]

Exit to cpu loop so we reevaluate cpu_s390x_hw_interrupts.

Signed-off-by: Richard Henderson 
---
 target/alpha/translate.c |  2 +-
 target/s390x/translate.c | 14 ++
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/target/alpha/translate.c b/target/alpha/translate.c
index a48e451..232af9e 100644
--- a/target/alpha/translate.c
+++ b/target/alpha/translate.c
@@ -1207,7 +1207,7 @@ static ExitStatus gen_call_pal(DisasContext *ctx, int 
palcode)
 tcg_temp_free(tmp);
 
 /* Allow interrupts to be recognized right away.  */
-tcg_gen_movi_i64(cpu_pc, ctx.pc);
+tcg_gen_movi_i64(cpu_pc, ctx->pc);
 return EXIT_PC_UPDATED_NOCHAIN;
 
 case 0x36:
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 95f91d4..c9e3b81 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -1174,6 +1174,8 @@ typedef enum {
 /* We are exiting the TB, but have neither emitted a goto_tb, nor
updated the PC for the next instruction to be executed.  */
 EXIT_PC_STALE,
+/* We are exiting the TB to the main loop.  */
+EXIT_PC_STALE_NOCHAIN,
 /* We are ending the TB with a noreturn function call, e.g. longjmp.
No following code will be executed.  */
 EXIT_NORETURN,
@@ -3796,7 +3798,8 @@ static ExitStatus op_ssm(DisasContext *s, DisasOps *o)
 {
 check_privileged(s);
 tcg_gen_deposit_i64(psw_mask, psw_mask, o->in2, 56, 8);
-return NO_EXIT;
+/* Exit to main loop to reevaluate s390_cpu_exec_interrupt.  */
+return EXIT_PC_STALE_NOCHAIN;
 }
 
 static ExitStatus op_stap(DisasContext *s, DisasOps *o)
@@ -4044,7 +4047,9 @@ static ExitStatus op_stnosm(DisasContext *s, DisasOps *o)
 } else {
 tcg_gen_ori_i64(psw_mask, psw_mask, i2 << 56);
 }
-return NO_EXIT;
+
+/* Exit to main loop to reevaluate s390_cpu_exec_interrupt.  */
+return EXIT_PC_STALE_NOCHAIN;
 }
 
 static ExitStatus op_stura(DisasContext *s, DisasOps *o)
@@ -5794,6 +5799,7 @@ void gen_intermediate_code(CPUS390XState *env, struct 
TranslationBlock *tb)
 case EXIT_NORETURN:
 break;
 case EXIT_PC_STALE:
+case EXIT_PC_STALE_NOCHAIN:
 update_psw_addr();
 /* FALLTHRU */
 case EXIT_PC_UPDATED:
@@ -5805,14 +5811,14 @@ void gen_intermediate_code(CPUS390XState *env, struct 
TranslationBlock *tb)
 /* Exit the TB, either by raising a debug exception or by return.  */
 if (do_debug) {
 gen_exception(EXCP_DEBUG);
-} else if (use_exit_tb()) {
+} else if (use_exit_tb() || status == EXIT_PC_STALE_NOCHAIN) {
 tcg_gen_exit_tb(0);
 } else {
 tcg_gen_lookup_and_goto_ptr(psw_addr);
 }
 break;
 default:
-abort();
+g_assert_not_reached();
 }
 
 gen_tb_end(tb, num_insns);
-- 
2.9.4

[Qemu-devel] [PATCH v2 3/5] target/mips: Exit after enabling interrupts

[Richard Henderson]

From: Paolo Bonzini 

Exit to cpu loop so we reevaluate cpu_mips_hw_interrupts.

Cc: Aurelien Jarno 
Cc: Yongbok Kim 
Signed-off-by: Richard Henderson 
---
 target/mips/translate.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/target/mips/translate.c b/target/mips/translate.c
index 559f8fe..891f14b 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -13403,9 +13403,11 @@ static void gen_pool32axf (CPUMIPSState *env, 
DisasContext *ctx, int rt, int rs)
 save_cpu_state(ctx, 1);
 gen_helper_ei(t0, cpu_env);
 gen_store_gpr(t0, rs);
-/* Stop translation as we may have switched the execution mode 
*/
-ctx->bstate = BS_STOP;
 tcg_temp_free(t0);
+/* BS_STOP isn't good enough here;
+   reevaluate cpu_mips_hw_interrupts_enabled.  */
+gen_save_pc(ctx->pc + 4);
+ctx->bstate = BS_EXCP;
 }
 break;
 default:
-- 
2.9.4

[Qemu-devel] [PATCH v2 0/5] Fixes for TCG hangs

[Richard Henderson]

Some good detective work by all involved.  This is attemping to
get all of the feedback from this morning.


r~


Paolo Bonzini (1):
  target/mips: Exit after enabling interrupts

Richard Henderson (4):
  tcg: Refactor helper_lookup_tb_ptr
  target/alpha: Use tcg_gen_lookup_and_goto_ptr
  target/s390x: Exit after changing PSW mask
  target/arm: Exit after clearing interrupt mask

 target/alpha/translate.c   | 27 ++-
 target/arm/translate-a64.c |  7 ++-
 target/mips/translate.c|  6 --
 target/s390x/translate.c   | 14 ++
 tcg-runtime.c  | 34 ++
 5 files changed, 60 insertions(+), 28 deletions(-)

-- 
2.9.4

[Qemu-devel] [PATCH v2 1/5] tcg: Refactor helper_lookup_tb_ptr

[Richard Henderson]

We can call tb_htable_lookup even when the tb_jmp_cache
is completely empty.  Therefore, un-nest most of the code
dependent on tb != NULL from the read from the cache.

Signed-off-by: Richard Henderson 
---
 tcg-runtime.c | 34 ++
 1 file changed, 18 insertions(+), 16 deletions(-)

diff --git a/tcg-runtime.c b/tcg-runtime.c
index 7fa90ce..a35c725 100644
--- a/tcg-runtime.c
+++ b/tcg-runtime.c
@@ -147,30 +147,32 @@ uint64_t HELPER(ctpop_i64)(uint64_t arg)
 void *HELPER(lookup_tb_ptr)(CPUArchState *env, target_ulong addr)
 {
 CPUState *cpu = ENV_GET_CPU(env);
+void *ret = tcg_ctx.code_gen_epilogue;
 TranslationBlock *tb;
 target_ulong cs_base, pc;
-uint32_t flags;
-
-tb = atomic_rcu_read(>tb_jmp_cache[tb_jmp_cache_hash_func(addr)]);
-if (likely(tb)) {
-cpu_get_tb_cpu_state(env, , _base, );
-if (likely(tb->pc == addr && tb->cs_base == cs_base &&
-   tb->flags == flags)) {
-goto found;
-}
+uint32_t flags, addr_hash;
+
+addr_hash = tb_jmp_cache_hash_func(addr);
+tb = atomic_rcu_read(>tb_jmp_cache[addr_hash]);
+cpu_get_tb_cpu_state(env, , _base, );
+
+if (unlikely(!(tb
+   && tb->pc == addr
+   && tb->cs_base == cs_base
+   && tb->flags == flags))) {
 tb = tb_htable_lookup(cpu, addr, cs_base, flags);
-if (likely(tb)) {
-atomic_set(>tb_jmp_cache[tb_jmp_cache_hash_func(addr)], tb);
-goto found;
+if (!tb) {
+return ret;
 }
+atomic_set(>tb_jmp_cache[addr_hash], tb);
 }
-return tcg_ctx.code_gen_epilogue;
- found:
+
+ret = tb->tc_ptr;
 qemu_log_mask_and_addr(CPU_LOG_EXEC, addr,
"Chain %p [%d: " TARGET_FMT_lx "] %s\n",
-   tb->tc_ptr, cpu->cpu_index, addr,
+   ret, cpu->cpu_index, addr,
lookup_symbol(addr));
-return tb->tc_ptr;
+return ret;
 }
 
 void HELPER(exit_atomic)(CPUArchState *env)
-- 
2.9.4

[Qemu-devel] [PATCH v2 2/5] target/alpha: Use tcg_gen_lookup_and_goto_ptr

[Richard Henderson]

Signed-off-by: Richard Henderson 
---
 target/alpha/translate.c | 27 ++-
 1 file changed, 22 insertions(+), 5 deletions(-)

diff --git a/target/alpha/translate.c b/target/alpha/translate.c
index 7c45ae3..a48e451 100644
--- a/target/alpha/translate.c
+++ b/target/alpha/translate.c
@@ -84,6 +84,7 @@ typedef enum {
the PC (for whatever reason), so there's no need to do it again on
exiting the TB.  */
 EXIT_PC_UPDATED,
+EXIT_PC_UPDATED_NOCHAIN,
 
 /* We are exiting the TB, but have neither emitted a goto_tb, nor
updated the PC for the next instruction to be executed.  */
@@ -458,11 +459,17 @@ static bool in_superpage(DisasContext *ctx, int64_t addr)
 #endif
 }
 
+static bool use_exit_tb(DisasContext *ctx)
+{
+return ((ctx->tb->cflags & CF_LAST_IO)
+|| ctx->singlestep_enabled
+|| singlestep);
+}
+
 static bool use_goto_tb(DisasContext *ctx, uint64_t dest)
 {
 /* Suppress goto_tb in the case of single-steping and IO.  */
-if ((ctx->tb->cflags & CF_LAST_IO)
-|| ctx->singlestep_enabled || singlestep) {
+if (unlikely(use_exit_tb(ctx))) {
 return false;
 }
 #ifndef CONFIG_USER_ONLY
@@ -1198,7 +1205,10 @@ static ExitStatus gen_call_pal(DisasContext *ctx, int 
palcode)
 tcg_gen_andi_i64(tmp, ctx->ir[IR_A0], PS_INT_MASK);
 tcg_gen_st8_i64(tmp, cpu_env, offsetof(CPUAlphaState, ps));
 tcg_temp_free(tmp);
-break;
+
+/* Allow interrupts to be recognized right away.  */
+tcg_gen_movi_i64(cpu_pc, ctx.pc);
+return EXIT_PC_UPDATED_NOCHAIN;
 
 case 0x36:
 /* RDPS */
@@ -1266,7 +1276,7 @@ static ExitStatus gen_call_pal(DisasContext *ctx, int 
palcode)
need the page permissions check.  We'll see the existence of
the page when we create the TB, and we'll flush all TBs if
we change the PAL base register.  */
-if (!ctx->singlestep_enabled && !(ctx->tb->cflags & CF_LAST_IO)) {
+if (!use_exit_tb(ctx)) {
 tcg_gen_goto_tb(0);
 tcg_gen_movi_i64(cpu_pc, entry);
 tcg_gen_exit_tb((uintptr_t)ctx->tb);
@@ -2686,7 +2696,8 @@ static ExitStatus translate_one(DisasContext *ctx, 
uint32_t insn)
 tcg_gen_andi_i64(tmp, vb, 1);
 tcg_gen_st8_i64(tmp, cpu_env, offsetof(CPUAlphaState, pal_mode));
 tcg_gen_andi_i64(cpu_pc, vb, ~3);
-ret = EXIT_PC_UPDATED;
+/* Allow interrupts to be recognized right away.  */
+ret = EXIT_PC_UPDATED_NOCHAIN;
 break;
 #else
 goto invalid_opc;
@@ -3010,6 +3021,12 @@ void gen_intermediate_code(CPUAlphaState *env, struct 
TranslationBlock *tb)
 tcg_gen_movi_i64(cpu_pc, ctx.pc);
 /* FALLTHRU */
 case EXIT_PC_UPDATED:
+if (!use_exit_tb()) {
+tcg_gen_lookup_and_goto_ptr(cpu_pc);
+break;
+}
+/* FALLTHRU */
+case EXIT_PC_UPDATED_NOCHAIN:
 if (ctx.singlestep_enabled) {
 gen_excp_1(EXCP_DEBUG, 0);
 } else {
-- 
2.9.4

Re: [Qemu-devel] [PATCH v1 3/3] tcg-runtime: short-circuit lookup_tb_ptr on IRQs

[Richard Henderson]

On 06/14/2017 12:07 PM, Alex Bennée wrote:


Richard Henderson  writes:


On 06/14/2017 10:08 AM, Paolo Bonzini wrote:

And MIPS:

diff --git a/target/mips/translate.c b/target/mips/translate.c
index 559f8fed89..244f3cb9ab 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -13403,8 +13403,9 @@ static void gen_pool32axf (CPUMIPSState *env, 
DisasContext *ctx, int rt, int rs)
   save_cpu_state(ctx, 1);
   gen_helper_ei(t0, cpu_env);
   gen_store_gpr(t0, rs);
-/* Stop translation as we may have switched the execution mode 
*/
-ctx->bstate = BS_STOP;
+/* BS_STOP isn't good enough here, reevaluate 
cpu_mips_hw_interrupts_enabled. */
+gen_save_pc(ctx->pc + 4);
+ctx->bstate = BS_EXCP;
   tcg_temp_free(t0);
   }
   break;

The others seem okay.


Thanks for this bit.  We also need to fix SSM for s390x.


If your rolling a series for all these can you also pick up Thomas
Huth's fix for --accel?


Will do.


r~

[Qemu-devel] [PATCH 3/4] include/hw/i386/pc.h: Move CONFIG_KVM related definitions to kvm_i386.h

[Thomas Huth]

pc.h is included from common code (where is CONFIG_KVM is not available),
so the #defines that depend on CONFIG_KVM should not be declared here
to avoid that anybody is using them in a wrong way.

Signed-off-by: Thomas Huth 
---
 hw/i386/pc_q35.c   |  1 +
 include/hw/i386/pc.h   | 13 -
 target/i386/kvm_i386.h | 13 +
 3 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 1523ef3..8f696b7 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -36,6 +36,7 @@
 #include "hw/timer/mc146818rtc.h"
 #include "hw/xen/xen.h"
 #include "sysemu/kvm.h"
+#include "kvm_i386.h"
 #include "hw/kvm/clock.h"
 #include "hw/pci-host/q35.h"
 #include "exec/address-spaces.h"
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index d071c9c..a31f7aa 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -20,19 +20,6 @@
 
 #define HPET_INTCAP "hpet-intcap"
 
-#ifdef CONFIG_KVM
-#define kvm_pit_in_kernel() \
-(kvm_irqchip_in_kernel() && !kvm_irqchip_is_split())
-#define kvm_pic_in_kernel()  \
-(kvm_irqchip_in_kernel() && !kvm_irqchip_is_split())
-#define kvm_ioapic_in_kernel() \
-(kvm_irqchip_in_kernel() && !kvm_irqchip_is_split())
-#else
-#define kvm_pit_in_kernel()  0
-#define kvm_pic_in_kernel()  0
-#define kvm_ioapic_in_kernel()   0
-#endif
-
 /**
  * PCMachineState:
  * @acpi_dev: link to ACPI PM device that performs ACPI hotplug handling
diff --git a/target/i386/kvm_i386.h b/target/i386/kvm_i386.h
index bfce427..ac33f39 100644
--- a/target/i386/kvm_i386.h
+++ b/target/i386/kvm_i386.h
@@ -15,6 +15,19 @@
 
 #define kvm_apic_in_kernel() (kvm_irqchip_in_kernel())
 
+#ifdef CONFIG_KVM
+#define kvm_pit_in_kernel() \
+(kvm_irqchip_in_kernel() && !kvm_irqchip_is_split())
+#define kvm_pic_in_kernel()  \
+(kvm_irqchip_in_kernel() && !kvm_irqchip_is_split())
+#define kvm_ioapic_in_kernel() \
+(kvm_irqchip_in_kernel() && !kvm_irqchip_is_split())
+#else
+#define kvm_pit_in_kernel()  0
+#define kvm_pic_in_kernel()  0
+#define kvm_ioapic_in_kernel()   0
+#endif
+
 bool kvm_allows_irq0_override(void);
 bool kvm_has_smm(void);
 bool kvm_has_adjust_clock_stable(void);
-- 
1.8.3.1

[Qemu-devel] [PATCH 1/4] include/exec/poison: Add missing TARGET defines

[Thomas Huth]

Since we've got some new CPU targets in QEMU during the last months
and years, we've got some new TARGET_xxx defines now which should
be marked as poisoned for common code.

Signed-off-by: Thomas Huth 
---
 include/exec/poison.h | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/include/exec/poison.h b/include/exec/poison.h
index 3ca7929..9356d5f 100644
--- a/include/exec/poison.h
+++ b/include/exec/poison.h
@@ -12,17 +12,28 @@
 #pragma GCC poison TARGET_CRIS
 #pragma GCC poison TARGET_LM32
 #pragma GCC poison TARGET_M68K
+#pragma GCC poison TARGET_MICROBLAZE
 #pragma GCC poison TARGET_MIPS
+#pragma GCC poison TARGET_ABI_MIPSO32
 #pragma GCC poison TARGET_MIPS64
+#pragma GCC poison TARGET_ABI_MIPSN64
+#pragma GCC poison TARGET_MOXIE
+#pragma GCC poison TARGET_NIOS2
 #pragma GCC poison TARGET_OPENRISC
 #pragma GCC poison TARGET_PPC
 #pragma GCC poison TARGET_PPCEMB
 #pragma GCC poison TARGET_PPC64
 #pragma GCC poison TARGET_ABI32
+#pragma GCC poison TARGET_S390X
 #pragma GCC poison TARGET_SH4
 #pragma GCC poison TARGET_SPARC
 #pragma GCC poison TARGET_SPARC64
+#pragma GCC poison TARGET_TRICORE
+#pragma GCC poison TARGET_UNICORE32
+#pragma GCC poison TARGET_XTENSA
 
+#pragma GCC poison TARGET_NAME
+#pragma GCC poison TARGET_SUPPORTS_MTTCG
 #pragma GCC poison TARGET_WORDS_BIGENDIAN
 #pragma GCC poison BSWAP_NEEDED
 
-- 
1.8.3.1

[Qemu-devel] [PATCH 4/4] include/exec/poison: Mark CONFIG_KVM as poisoned, too

[Thomas Huth]

We unfortunately need some additional "#ifndef NEED_CPU_H" fuzz in
include/sysemu/kvm.h for this, so that the header can still be included
from common code (which is done all over the place), but now we can
finally be sure that nobody uses this define in a wrong place anymore.

Signed-off-by: Thomas Huth 
---
 hw/acpi/ich9.c|  1 -
 include/exec/poison.h |  1 +
 include/sysemu/kvm.h  | 37 +++--
 3 files changed, 36 insertions(+), 3 deletions(-)

diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
index 5c279bb..c5d8646 100644
--- a/hw/acpi/ich9.c
+++ b/hw/acpi/ich9.c
@@ -33,7 +33,6 @@
 #include "sysemu/sysemu.h"
 #include "hw/acpi/acpi.h"
 #include "hw/acpi/tco.h"
-#include "sysemu/kvm.h"
 #include "exec/address-spaces.h"
 
 #include "hw/i386/ich9.h"
diff --git a/include/exec/poison.h b/include/exec/poison.h
index 5ffed4d..540fc70 100644
--- a/include/exec/poison.h
+++ b/include/exec/poison.h
@@ -80,6 +80,7 @@
 
 #pragma GCC poison CONFIG_LINUX_USER
 #pragma GCC poison CONFIG_VHOST_NET
+#pragma GCC poison CONFIG_KVM
 
 #endif
 #endif
diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index 1e91613..8cc57e4 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -19,6 +19,8 @@
 #include "exec/memattrs.h"
 #include "hw/irq.h"
 
+#ifdef NEED_CPU_H
+
 #ifdef CONFIG_KVM
 #include 
 #include 
@@ -39,6 +41,8 @@
 #define KVM_FEATURE_CLOCKSOURCE_STABLE_BIT 0
 #endif
 
+#endif /* NEED_CPU_H */
+
 extern bool kvm_allowed;
 extern bool kvm_kernel_irqchip;
 extern bool kvm_split_irqchip;
@@ -55,7 +59,8 @@ extern bool kvm_direct_msi_allowed;
 extern bool kvm_ioeventfd_any_length_allowed;
 extern bool kvm_msi_use_devid;
 
-#if defined CONFIG_KVM || !defined NEED_CPU_H
+#if !defined(NEED_CPU_H)
+
 #define kvm_enabled()   (kvm_allowed)
 /**
  * kvm_irqchip_in_kernel:
@@ -178,6 +183,31 @@ extern bool kvm_msi_use_devid;
 #define kvm_msi_devid_required() (kvm_msi_use_devid)
 
 #else
+
+#ifdef CONFIG_KVM
+
+/*
+ * Same definitions again, but we need to keep them separate
+ * since CONFIG_KVM is poisoned without NEED_CPU_H
+ */
+#define kvm_enabled()   (kvm_allowed)
+#define kvm_irqchip_in_kernel() (kvm_kernel_irqchip)
+#define kvm_irqchip_is_split() (kvm_split_irqchip)
+#define kvm_async_interrupts_enabled() (kvm_async_interrupts_allowed)
+#define kvm_halt_in_kernel() (kvm_halt_in_kernel_allowed)
+#define kvm_eventfds_enabled() (kvm_eventfds_allowed)
+#define kvm_irqfds_enabled() (kvm_irqfds_allowed)
+#define kvm_resamplefds_enabled() (kvm_resamplefds_allowed)
+#define kvm_msi_via_irqfd_enabled() (kvm_msi_via_irqfd_allowed)
+#define kvm_gsi_routing_enabled() (kvm_gsi_routing_allowed)
+#define kvm_gsi_direct_mapping() (kvm_gsi_direct_mapping)
+#define kvm_readonly_mem_enabled() (kvm_readonly_mem_allowed)
+#define kvm_direct_msi_enabled() (kvm_direct_msi_allowed)
+#define kvm_ioeventfd_any_length_enabled() (kvm_ioeventfd_any_length_allowed)
+#define kvm_msi_devid_required() (kvm_msi_use_devid)
+
+#else
+
 #define kvm_enabled()   (0)
 #define kvm_irqchip_in_kernel() (false)
 #define kvm_irqchip_is_split() (false)
@@ -193,7 +223,10 @@ extern bool kvm_msi_use_devid;
 #define kvm_direct_msi_enabled() (false)
 #define kvm_ioeventfd_any_length_enabled() (false)
 #define kvm_msi_devid_required() (false)
-#endif
+
+#endif  /* CONFIG_KVM */
+
+#endif  /* NEED_CPU_H */
 
 struct kvm_run;
 struct kvm_lapic_state;
-- 
1.8.3.1

[Qemu-devel] [PATCH 2/4] include/exec/poison: Mark some CONFIG defines as poisoned, too

[Thomas Huth]

These are defined in config-target.h and thus should never be
used in common code.

Signed-off-by: Thomas Huth 
---
 include/exec/poison.h | 20 
 1 file changed, 20 insertions(+)

diff --git a/include/exec/poison.h b/include/exec/poison.h
index 9356d5f..5ffed4d 100644
--- a/include/exec/poison.h
+++ b/include/exec/poison.h
@@ -61,5 +61,25 @@
 #pragma GCC poison CPU_INTERRUPT_TGT_INT_1
 #pragma GCC poison CPU_INTERRUPT_TGT_INT_2
 
+#pragma GCC poison CONFIG_ALPHA_DIS
+#pragma GCC poison CONFIG_ARM_A64_DIS
+#pragma GCC poison CONFIG_ARM_DIS
+#pragma GCC poison CONFIG_CRIS_DIS
+#pragma GCC poison CONFIG_I386_DIS
+#pragma GCC poison CONFIG_LM32_DIS
+#pragma GCC poison CONFIG_M68K_DIS
+#pragma GCC poison CONFIG_MICROBLAZE_DIS
+#pragma GCC poison CONFIG_MIPS_DIS
+#pragma GCC poison CONFIG_MOXIE_DIS
+#pragma GCC poison CONFIG_NIOS2_DIS
+#pragma GCC poison CONFIG_PPC_DIS
+#pragma GCC poison CONFIG_S390_DIS
+#pragma GCC poison CONFIG_SH4_DIS
+#pragma GCC poison CONFIG_SPARC_DIS
+#pragma GCC poison CONFIG_XTENSA_DIS
+
+#pragma GCC poison CONFIG_LINUX_USER
+#pragma GCC poison CONFIG_VHOST_NET
+
 #endif
 #endif
-- 
1.8.3.1

[Qemu-devel] [PATCH 0/4] Poison some more target-specific defines

[Thomas Huth]

This series marks some more #defines as poisoned, which are
target-specific (declared in config-target.h) and thus must
not be used in common code.

Note that these are just the easy cases - we should later also add
CONFIG_SOFTMMU and CONFIG_USER_ONLY, but they require some other
additional clean-up first.

Thomas Huth (4):
  include/exec/poison: Add missing TARGET defines
  include/exec/poison: Mark some CONFIG defines as poisoned, too
  include/hw/i386/pc.h: Move CONFIG_KVM related definitions to
kvm_i386.h
  include/exec/poison: Mark CONFIG_KVM as poisoned, too

 hw/acpi/ich9.c |  1 -
 hw/i386/pc_q35.c   |  1 +
 include/exec/poison.h  | 32 
 include/hw/i386/pc.h   | 13 -
 include/sysemu/kvm.h   | 37 +++--
 target/i386/kvm_i386.h | 13 +
 6 files changed, 81 insertions(+), 16 deletions(-)

-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v1 3/3] tcg-runtime: short-circuit lookup_tb_ptr on IRQs

[Richard Henderson]

On 06/14/2017 12:11 PM, Peter Maydell wrote:

On 14 June 2017 at 18:49, Alex Bennée  wrote:

diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c
index 2a85666579..7e67bb3db2 100644
--- a/target/arm/op_helper.c
+++ b/target/arm/op_helper.c
@@ -835,6 +835,9 @@ void HELPER(msr_i_pstate)(CPUARMState *env, uint32_t op, 
uint32_t imm)
  break;
  case 0x1f: /* DAIFClear */
  env->daif &= ~((imm << 6) & PSTATE_DAIF);
+/* This may result in pending IRQs being unmasked so ensure we
+   exit the loop */
+cpu_exit(ENV_GET_CPU(env));
  break;
  default:
  g_assert_not_reached();


The 'op' field we're switching on here is just a constant
from the instruction encoding, so I'd rather see us
identify that in translate-a64.c and end the TB or
whatever when we need to, rather than doing the
longjump-out-of-here that cpu_exit() does at runtime.


cpu_exit isn't the longjmp; this is just a set of exit_request and icount_decr.

That said, you're right that we can do this more directly from the translator.


r~

Re: [Qemu-devel] [PATCH v1 3/3] tcg-runtime: short-circuit lookup_tb_ptr on IRQs

[Peter Maydell]

On 14 June 2017 at 18:49, Alex Bennée  wrote:
> diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c
> index 2a85666579..7e67bb3db2 100644
> --- a/target/arm/op_helper.c
> +++ b/target/arm/op_helper.c
> @@ -835,6 +835,9 @@ void HELPER(msr_i_pstate)(CPUARMState *env, uint32_t op, 
> uint32_t imm)
>  break;
>  case 0x1f: /* DAIFClear */
>  env->daif &= ~((imm << 6) & PSTATE_DAIF);
> +/* This may result in pending IRQs being unmasked so ensure we
> +   exit the loop */
> +cpu_exit(ENV_GET_CPU(env));
>  break;
>  default:
>  g_assert_not_reached();

The 'op' field we're switching on here is just a constant
from the instruction encoding, so I'd rather see us
identify that in translate-a64.c and end the TB or
whatever when we need to, rather than doing the
longjump-out-of-here that cpu_exit() does at runtime.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v1 3/3] tcg-runtime: short-circuit lookup_tb_ptr on IRQs

[Alex Bennée]

Richard Henderson  writes:

> On 06/14/2017 10:08 AM, Paolo Bonzini wrote:
>> And MIPS:
>>
>> diff --git a/target/mips/translate.c b/target/mips/translate.c
>> index 559f8fed89..244f3cb9ab 100644
>> --- a/target/mips/translate.c
>> +++ b/target/mips/translate.c
>> @@ -13403,8 +13403,9 @@ static void gen_pool32axf (CPUMIPSState *env, 
>> DisasContext *ctx, int rt, int rs)
>>   save_cpu_state(ctx, 1);
>>   gen_helper_ei(t0, cpu_env);
>>   gen_store_gpr(t0, rs);
>> -/* Stop translation as we may have switched the execution 
>> mode */
>> -ctx->bstate = BS_STOP;
>> +/* BS_STOP isn't good enough here, reevaluate 
>> cpu_mips_hw_interrupts_enabled. */
>> +gen_save_pc(ctx->pc + 4);
>> +ctx->bstate = BS_EXCP;
>>   tcg_temp_free(t0);
>>   }
>>   break;
>>
>> The others seem okay.
>
> Thanks for this bit.  We also need to fix SSM for s390x.

If your rolling a series for all these can you also pick up Thomas
Huth's fix for --accel?

--
Alex Bennée

Re: [Qemu-devel] [PATCH 05/23] hyperv: ensure VP index equal to QEMU cpu_index

[Eduardo Habkost]

On Wed, Jun 14, 2017 at 09:40:37PM +0300, Roman Kagan wrote:
> On Wed, Jun 14, 2017 at 10:45:23AM -0300, Eduardo Habkost wrote:
> > On Wed, Jun 14, 2017 at 03:38:59PM +0200, Igor Mammedov wrote:
> > > On Wed, 14 Jun 2017 10:22:16 -0300
> > > Eduardo Habkost  wrote:
> > > 
> > > > On Wed, Jun 14, 2017 at 03:17:54PM +0200, Paolo Bonzini wrote:
> > > > > 
> > > > > 
> > > > > On 14/06/2017 15:11, Igor Mammedov wrote:  
> > > > > >> No, KVM really uses the VCPU _index_ for HV_X64_MSR_VP_INDEX:  
> > > > > >
> > > > > > and as you pointed out that works just by luck,
> > > > > > as soon as we there would be out of order created CPUs
> > > > > > returned value won't match cpu_index.
> > > > > > 
> > > > > > So instead of spreading this nonsense out to QEMU, is it possible
> > > > > > to fix kernel(kvm+guest) to use apic_id instead?  
> > > > > 
> > > > > Yes, definitely.  At this point let's add a new 
> > > > > "KVM_CAP_HYPERV_SYNIC2"
> > > > > capability and declare the old one broken, that will make things 
> > > > > easier.  
> > > > 
> > > > What do we need a capability for?
> > > > Can't we just call
> > > > KVM_SET_MSRS using arch_id (== apic_id) and warn the user if it
> > > > doesn't work?
> > > warn won't solve issue, and it's called rather late in vcpu creation
> > > chain without any error propagation so it's not possible (hard) to
> > > gracefully fail cpu hotplug.
> > 
> > The issue is unsolvable on old kernels (and I don't think we want
> > to prevent the VM from starting), hence the warning.  But the
> > ability to report errors gracefully on CPU hotplug is a very good
> > reason.  Good point.
> > 
> > > 
> > > with KVM_CAP_HYPERV_SYNIC2 we could easily turn on compat property for
> > > SYNC device (needs QOMification) that would turn on broken compat
> > > logic if capability is not present/old machine type.
> > 
> > I was going to propose enabling the compat logic if KVM_SET_MSRS
> > failed, but you have a good point about KVM_SET_MSRS being called
> > very late.
> 
> Thanks for this discussion, I didn't realize all the implications
> (including that hotplug issue too).
> 
> One more data point is that until now there was no use for vp_index in
> QEMU, so it didn't care how KVM managed it.  In KVM the only
> vp_index-aware path that the guests could trigger was exactly reading of
> HV_X64_MSR_VP_INDEX.
> 
> So let me try to sum up (to make sure I understand it right);
> 
> 1) we add KVM_CAP_HYPERV_SYNIC2 to KVM; when QEMU enables it KVM
>switches to using vcpu_id as vp_index and stops zeroing synic pages

If we want to keep KVM code simpler, we could make QEMU
explicitly initialize vp_index using vcpu_id (== arch_id == apic_id)
if KVM_CAP_HYPERV_SYNIC2 is reported as supported.

> 
> 2) new QEMU refuses to start in non-compat mode when
>KVM_CAP_HYPERV_SYNIC2 is not supported

It depends on which cases are considered "in non-compat mode".
Getting a VM not runnable just because the machine-type was
updated is not desirable, especially considering that on most
cases we will create the VCPUs on the same order and things would
keep working.  Probably the best we can do on this case is to
automatically enable compat mode, but print a warning saying
future QEMU versions might break if the kernel is not upgraded.

> 
> 3) old QEMU or new QEMU in compat mode enables KVM_CAP_HYPERV_SYNIC
>making KVM keep using internal cpu index as vp_index and zeroing
>synic pages

Maybe we need a warning on this case too, because QEMU won't
guarantee it will always create VCPUs in the same order in the
future.

> 
> 4) new QEMU in compat mode refuses vmbus or sint route creation

Also: new QEMU in compat mode refuses CPU hotplug.

> 
> Is this what is proposed?  My only problem here is that KVM will have to
> somehow guarantee stable numbering in the old synic mode.  How can this
> be ensured?  And should it be at all?

As far as I can see, KVM can only guarantee stable numbering in
the old mode if QEMU creates the VCPUs in the same order.  QEMU
can do a reasonable effort to not break that unnecessarily, but
users should be aware that old mode is deprecated and can break.

-- 
Eduardo

Re: [Qemu-devel] [PATCH 05/23] hyperv: ensure VP index equal to QEMU cpu_index

[Roman Kagan]

On Wed, Jun 14, 2017 at 10:45:23AM -0300, Eduardo Habkost wrote:
> On Wed, Jun 14, 2017 at 03:38:59PM +0200, Igor Mammedov wrote:
> > On Wed, 14 Jun 2017 10:22:16 -0300
> > Eduardo Habkost  wrote:
> > 
> > > On Wed, Jun 14, 2017 at 03:17:54PM +0200, Paolo Bonzini wrote:
> > > > 
> > > > 
> > > > On 14/06/2017 15:11, Igor Mammedov wrote:  
> > > > >> No, KVM really uses the VCPU _index_ for HV_X64_MSR_VP_INDEX:  
> > > > >
> > > > > and as you pointed out that works just by luck,
> > > > > as soon as we there would be out of order created CPUs
> > > > > returned value won't match cpu_index.
> > > > > 
> > > > > So instead of spreading this nonsense out to QEMU, is it possible
> > > > > to fix kernel(kvm+guest) to use apic_id instead?  
> > > > 
> > > > Yes, definitely.  At this point let's add a new "KVM_CAP_HYPERV_SYNIC2"
> > > > capability and declare the old one broken, that will make things 
> > > > easier.  
> > > 
> > > What do we need a capability for?
> > > Can't we just call
> > > KVM_SET_MSRS using arch_id (== apic_id) and warn the user if it
> > > doesn't work?
> > warn won't solve issue, and it's called rather late in vcpu creation
> > chain without any error propagation so it's not possible (hard) to
> > gracefully fail cpu hotplug.
> 
> The issue is unsolvable on old kernels (and I don't think we want
> to prevent the VM from starting), hence the warning.  But the
> ability to report errors gracefully on CPU hotplug is a very good
> reason.  Good point.
> 
> > 
> > with KVM_CAP_HYPERV_SYNIC2 we could easily turn on compat property for
> > SYNC device (needs QOMification) that would turn on broken compat
> > logic if capability is not present/old machine type.
> 
> I was going to propose enabling the compat logic if KVM_SET_MSRS
> failed, but you have a good point about KVM_SET_MSRS being called
> very late.

Thanks for this discussion, I didn't realize all the implications
(including that hotplug issue too).

One more data point is that until now there was no use for vp_index in
QEMU, so it didn't care how KVM managed it.  In KVM the only
vp_index-aware path that the guests could trigger was exactly reading of
HV_X64_MSR_VP_INDEX.

So let me try to sum up (to make sure I understand it right);

1) we add KVM_CAP_HYPERV_SYNIC2 to KVM; when QEMU enables it KVM
   switches to using vcpu_id as vp_index and stops zeroing synic pages

2) new QEMU refuses to start in non-compat mode when
   KVM_CAP_HYPERV_SYNIC2 is not supported

3) old QEMU or new QEMU in compat mode enables KVM_CAP_HYPERV_SYNIC
   making KVM keep using internal cpu index as vp_index and zeroing
   synic pages

4) new QEMU in compat mode refuses vmbus or sint route creation

Is this what is proposed?  My only problem here is that KVM will have to
somehow guarantee stable numbering in the old synic mode.  How can this
be ensured?  And should it be at all?

Thanks,
Roman.

Re: [Qemu-devel] [PATCH 2/3] exec: simplify address_space_get_iotlb_entry

[Michael S. Tsirkin]

On Mon, Jun 12, 2017 at 12:04:58PM +0800, Peter Xu wrote:
> On Mon, Jun 12, 2017 at 06:07:04AM +0300, Michael S. Tsirkin wrote:
> > On Mon, Jun 12, 2017 at 10:34:43AM +0800, Peter Xu wrote:
> > > On Sun, Jun 11, 2017 at 08:10:15PM +0800, David Gibson wrote:
> > > > On Sun, Jun 11, 2017 at 01:09:26PM +0300, Michael S. Tsirkin wrote:
> > > > > On Fri, Jun 09, 2017 at 09:58:47AM +0800, Peter Xu wrote:
> > > > > > > > The problem is that when I was fixing the problem that vhost 
> > > > > > > > had with
> > > > > > > > PT (a764040, "exec: abstract address_space_do_translate()"), I 
> > > > > > > > did
> > > > > > > > broke the IOTLB translation a bit (it was using page masks). 
> > > > > > > > IMHO we
> > > > > > > > need to fix it first for correctness (patch 1/2).
> > > > > > > > 
> > > > > > > > For patch 3, if we can have Jason's patch to allow dynamic
> > > > > > > > iommu_platform switching, that'll be the best, then I can 
> > > > > > > > rewrite
> > > > > > > > patch 3 with the switching logic rather than caching anything. 
> > > > > > > > But
> > > > > > > > IMHO that can be separated from patch 1/2 if you like.
> > > > > > > > 
> > > > > > > > Or do you have better suggestion on how should we fix it?
> > > > > > > > 
> > > > > > > > Thanks,
> > > > > > > 
> > > > > > > Can we drop masks completely and replace with length? I think we
> > > > > > > should do that instead of trying to fix masks.
> > > > > > 
> > > > > > Do you mean to modify IOMMUTLBEntry.addr_mask into length?
> > > > > 
> > > > > I think it's better than alternatives.
> > > > > 
> > > > > > Again, I am not sure this is good... At least we need to get ack 
> > > > > > from
> > > > > > David since spapr should be the initial user of it, and possibly 
> > > > > > also
> > > > > > Alex since vfio should be assuming that (IIUC both in QEMU and 
> > > > > > kernel)
> > > > > > addr_mask is page masks rather than arbirary length.
> > > > > > 
> > > > > > (CC Alex)
> > > > > > 
> > > > > > Thanks,
> > > > > 
> > > > > Callbacks that need powers of two can easily split up the range.
> > > > 
> > > > I think I missed part of the thread.  What's the original use case for
> > > > non-power-of-two IOTLB entries?  It certainly won't happen on Power.
> > > 
> > > Currently address_space_get_iotlb_entry() didn't really follow the
> > > rule, addr_mask can be arbitary length. This series tried to fix it,
> > > while Michael was questioning about whether we should really fix that
> > > at all.
> > > 
> > > Michael,
> > > 
> > > Even if for performance's sake, I should still think we should fix it.
> > > Let's consider a most simple worst case: we have a single page mapped
> > > with IOVA range (2M page):
> > > 
> > >  [0x0, 0x20)
> > > 
> > > And if guest access IOVA using the following patern:
> > > 
> > >  0x1f, 0x1e, 0x1d, ...
> > > 
> > > Then now we'll get this:
> > > 
> > > - request 0x1f, cache miss, will get iotlb [0x1f, 0x20)
> > > - request 0x1e, cache miss, will get iotlb [0x1e, 0x20)
> > > - request 0x1d, cache miss, will get iotlb [0x1d, 0x20)
> > > - ...
> > 
> > We pass an offset too, do we not? So callee can figure out
> > the region starts at 0x0 and avoid 2nd and 3rd misses.
> 
> Here when you say "offset", do you mean the offset in
> MemoryRegionSection?
> 
> In address_space_get_iotlb_entry() we have this:
> 
> section = address_space_do_translate(as, addr, , ,
>  is_write, false);
> 
> One thing to mention is that, imho we cannot really assume the xlat is
> valid on the whole "section" range - the section can be a huge GPA
> range, while the xlat may only be valid on a single 4K page. The only
> safe region we can use here is (xlat, xlat+plen). Outside that, we
> should know nothing valid.
> 
> Please correct me if I didn't really catch the point..

IIUC section is the translation result. If so all of it is valid,
not just one page.

> > 
> > 
> > > We'll all cache miss along the way until we access 0x0. While if we
> > > are with page mask, we'll get:
> > > 
> > > - request 0x1f, cache miss, will get iotlb [0x0, 0x20)
> > > - request 0x1e, cache hit
> > > - request 0x1d, cache hit
> > > - ...
> > > 
> > > We'll only miss at the first IO.
> > 
> > I think we should send as much info as we can.
> > There should be a way to find full region start and length.
> 
> Thanks,
> 
> -- 
> Peter Xu

Re: [Qemu-devel] [PATCH v1 3/3] tcg-runtime: short-circuit lookup_tb_ptr on IRQs

[Richard Henderson]

On 06/14/2017 10:08 AM, Paolo Bonzini wrote:

And MIPS:

diff --git a/target/mips/translate.c b/target/mips/translate.c
index 559f8fed89..244f3cb9ab 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -13403,8 +13403,9 @@ static void gen_pool32axf (CPUMIPSState *env, 
DisasContext *ctx, int rt, int rs)
  save_cpu_state(ctx, 1);
  gen_helper_ei(t0, cpu_env);
  gen_store_gpr(t0, rs);
-/* Stop translation as we may have switched the execution mode 
*/
-ctx->bstate = BS_STOP;
+/* BS_STOP isn't good enough here, reevaluate 
cpu_mips_hw_interrupts_enabled. */
+gen_save_pc(ctx->pc + 4);
+ctx->bstate = BS_EXCP;
  tcg_temp_free(t0);
  }
  break;

The others seem okay.


Thanks for this bit.  We also need to fix SSM for s390x.


r~

Re: [Qemu-devel] [PATCH] q35/mch: implement extended TSEG sizes

[Michael S. Tsirkin]

On Fri, Jun 09, 2017 at 10:01:18PM +0200, Gerd Hoffmann wrote:
> On Fri, 2017-06-09 at 13:40 +0200, Paolo Bonzini wrote:
> > 
> > On 08/06/2017 21:55, Michael S. Tsirkin wrote:
> > > We don't have room anywhere in PCI config space. Laszlo makes
> > > argument
> > > why it's safe for this device based on spec but it's anyone's guess
> > > whether current and future software will follow spec.  In short,
> > > going
> > > anywhere near the emulated device has a potential to break some
> > > drivers.
> > 
> > There are no such drivers.  The MCH and PCH are only touched by the
> > firmware, not by the OS.
> 
> Yea.  That is *exactly* the reason why I think simply using the 0x50
> offset probably works fine in practice even though I suspect on
> physical hardware it might be some undocumented register.  Much of the
> stuff in the host bridge pci config space is firmware territory, and we
> run qemu specific firmware *anyway*.
> 
> cheers,
>   Gerd

To be specific, what I meant is a bit that tells guest that a
config space register is available, and lets host find out
that guest is going to use it.

This to ensure full forward and backward compatibility.

I agree a fw cfg file for a single bit seems like an overkill, that's
why I thought sharing feature files with SMI would be a good idea.

Do you see an issue with that?

-- 
MST

[Qemu-devel] tlb_flush() in qom/cpu.c

[Thomas Huth]

 Hi Alex,

I'm currently trying to poison some more target-specific defines and
noticed something fishy:

In commit  1f5c00cfdb8114c ("move tlb_flush to cpu_common_reset") you
moved the call to tlb_flush() to qom/cpu.c and guarded it with a #ifdef
CONFIG_SOFTMMU. However, qom/cpu.c is common code (common-obj-y in the
Makefile), so CONFIG_SOFTMMU is *never* defined here, i.e. the
tlb_flush() is never called anymore! (this is also quite obvious since
you've changed the prototype of tlb_flush() in d10eb08f5d83 later
without adapting qom/cpu.c).

Not sure how to fix this in a nice way, though ... shall we move the
tlb_flush() back to the target-specific reset handlers?

 Thomas

Re: [Qemu-devel] DragonFly BSD support

[Kamil Rytarowski]

On 14.06.2017 12:55, Antonio Huete Jiménez wrote:
> 
> Hi all,
> 
> According to 2.9 changelog page, DragonFly BSD will be listed as
> unsupported with the possibility of dropping support completely in the
> future:
> 
> http://wiki.qemu.org/ChangeLog/2.9
> 
> I'd like to volunteer so that qemu can keep DragonFly BSD as a supported
> platform.
> Could you please let me know the requirements needed to do so?
> 
> Best regards,
> Antonio Huete
> 
> 

Are there any local patches for qemu in your ports tree?

Please add yourself to MAINTAINERS and set DragonFly as maintained in
"configure".

There is an option to establish a BSD pull-request queue (as a public
group on GitHub?) and share it for BSD-specific patches. I work on
NetBSD and we still need someone at least for FreeBSD/OpenBSD.



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v1 3/3] tcg-runtime: short-circuit lookup_tb_ptr on IRQs

[Richard Henderson]

On 06/14/2017 10:49 AM, Alex Bennée wrote:

I think this is a band-aid, and would rather fix the front-ends as in
Emilio's patch.


It seems a shame to cause all msr accesses to trigger and exit when we
only care about the unmasking case. How about:


Author: Alex Bennée 
Date:   Wed Jun 14 18:46:01 2017 +0100

 target/arm/op_helper: ensure we exit the run-loop

 When IRQs are un-masked we need to ensure the run-loop is exited so we
 can evaluate arm_cpu_do_interrupt.

 Signed-off-by: Alex Bennée 

diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c
index 2a85666579..7e67bb3db2 100644
--- a/target/arm/op_helper.c
+++ b/target/arm/op_helper.c
@@ -835,6 +835,9 @@ void HELPER(msr_i_pstate)(CPUARMState *env, uint32_t op, 
uint32_t imm)
  break;
  case 0x1f: /* DAIFClear */
  env->daif &= ~((imm << 6) & PSTATE_DAIF);
+/* This may result in pending IRQs being unmasked so ensure we
+   exit the loop */
+cpu_exit(ENV_GET_CPU(env));


That works for me.  And I guess that takes care of any potential problems with 
A32 as well?



r~

Re: [Qemu-devel] [PATCH v1 3/3] tcg-runtime: short-circuit lookup_tb_ptr on IRQs

[Alex Bennée]

Paolo Bonzini  writes:

> On 14/06/2017 17:45, Richard Henderson wrote:
>> While the next TB would detect the exit flag has been set there is no
>> point if we can exit sooner. We also check cpu->interrupt_request as
>> some front-ends can set it rather than using the cpu_interrupt() API
>> call and would normally be expecting the IRQ to get picked up on the
>> previously fairly regular exits from the run loop.
>
> This is not what happens actually; it's not about front-ends setting
> cpu->interrupt_request, it's about front-ends doing exit_tb when they
> wanted to re-evaluate cpu_handle_interrupt.
>
> cpu_exit is used when device code causes a rising edge in
> cpu->interrupt_request.  What we have here is that the MSR write causes
> cc->cpu_exec_interrupt's return value to change from false to true.
>
> I think this is a band-aid, and would rather fix the front-ends as in
> Emilio's patch.

It seems a shame to cause all msr accesses to trigger and exit when we
only care about the unmasking case. How about:


Author: Alex Bennée 
Date:   Wed Jun 14 18:46:01 2017 +0100

target/arm/op_helper: ensure we exit the run-loop

When IRQs are un-masked we need to ensure the run-loop is exited so we
can evaluate arm_cpu_do_interrupt.

Signed-off-by: Alex Bennée 

diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c
index 2a85666579..7e67bb3db2 100644
--- a/target/arm/op_helper.c
+++ b/target/arm/op_helper.c
@@ -835,6 +835,9 @@ void HELPER(msr_i_pstate)(CPUARMState *env, uint32_t op, 
uint32_t imm)
 break;
 case 0x1f: /* DAIFClear */
 env->daif &= ~((imm << 6) & PSTATE_DAIF);
+/* This may result in pending IRQs being unmasked so ensure we
+   exit the loop */
+cpu_exit(ENV_GET_CPU(env));
 break;
 default:
 g_assert_not_reached();

[Qemu-devel] [PULL v1 6/7] exec: allow to get a pointer for some mmio memory region

[Edgar E. Iglesias]

From: KONRAD Frederic 

This introduces a special callback which allows to run code from some MMIO
devices.

SysBusDevice with a MemoryRegion which implements the request_ptr callback will
be notified when the guest try to execute code from their offset. Then it will
be able to eg: pre-load some code from an SPI device or ask a pointer from an
external simulator, etc..

When the pointer or the data in it are no longer valid the device has to
invalidate it.

Signed-off-by: KONRAD Frederic 

V2 -> V3:
  * Put the invalidate in an async work.
  * Clear the dirty flag before dropping the mmio interface.
  * Rebase against recent cpu_handle_unaligned changes.
RFC -> V1:
  * Use mmio-interface instead of directly creating the subregion.
---
 cputlb.c  |  10 +
 include/exec/memory.h |  35 
 memory.c  | 111 ++
 3 files changed, 156 insertions(+)

diff --git a/cputlb.c b/cputlb.c
index 95265a0..1900936 100644
--- a/cputlb.c
+++ b/cputlb.c
@@ -858,6 +858,16 @@ tb_page_addr_t get_page_addr_code(CPUArchState *env, 
target_ulong addr)
 pd = iotlbentry->addr & ~TARGET_PAGE_MASK;
 mr = iotlb_to_region(cpu, pd, iotlbentry->attrs);
 if (memory_region_is_unassigned(mr)) {
+qemu_mutex_lock_iothread();
+if (memory_region_request_mmio_ptr(mr, addr)) {
+qemu_mutex_unlock_iothread();
+/* A MemoryRegion is potentially added so re-run the
+ * get_page_addr_code.
+ */
+return get_page_addr_code(env, addr);
+}
+qemu_mutex_unlock_iothread();
+
 cpu_unassigned_access(cpu, addr, false, true, 0, 4);
 /* The CPU's unassigned access hook might have longjumped out
  * with an exception. If it didn't (or there was no hook) then
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 80e605a..137339a 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -137,6 +137,15 @@ struct MemoryRegionOps {
 uint64_t data,
 unsigned size,
 MemTxAttrs attrs);
+/* Instruction execution pre-callback:
+ * @addr is the address of the access relative to the @mr.
+ * @size is the size of the area returned by the callback.
+ * @offset is the location of the pointer inside @mr.
+ *
+ * Returns a pointer to a location which contains guest code.
+ */
+void *(*request_ptr)(void *opaque, hwaddr addr, unsigned *size,
+ unsigned *offset);
 
 enum device_endian endianness;
 /* Guest-visible constraints: */
@@ -1354,6 +1363,32 @@ void memory_global_dirty_log_stop(void);
 void mtree_info(fprintf_function mon_printf, void *f, bool flatview);
 
 /**
+ * memory_region_request_mmio_ptr: request a pointer to an mmio
+ * MemoryRegion. If it is possible map a RAM MemoryRegion with this pointer.
+ * When the device wants to invalidate the pointer it will call
+ * memory_region_invalidate_mmio_ptr.
+ *
+ * @mr: #MemoryRegion to check
+ * @addr: address within that region
+ *
+ * Returns true on success, false otherwise.
+ */
+bool memory_region_request_mmio_ptr(MemoryRegion *mr, hwaddr addr);
+
+/**
+ * memory_region_invalidate_mmio_ptr: invalidate the pointer to an mmio
+ * previously requested.
+ * In the end that means that if something wants to execute from this area it
+ * will need to request the pointer again.
+ *
+ * @mr: #MemoryRegion associated to the pointer.
+ * @addr: address within that region
+ * @size: size of that area.
+ */
+void memory_region_invalidate_mmio_ptr(MemoryRegion *mr, hwaddr offset,
+   unsigned size);
+
+/**
  * memory_region_dispatch_read: perform a read directly to the specified
  * MemoryRegion.
  *
diff --git a/memory.c b/memory.c
index 0ddc4cc..9c89dd4 100644
--- a/memory.c
+++ b/memory.c
@@ -30,6 +30,8 @@
 #include "exec/ram_addr.h"
 #include "sysemu/kvm.h"
 #include "sysemu/sysemu.h"
+#include "hw/misc/mmio_interface.h"
+#include "hw/qdev-properties.h"
 
 //#define DEBUG_UNASSIGNED
 
@@ -2424,6 +2426,115 @@ void memory_listener_unregister(MemoryListener 
*listener)
 listener->address_space = NULL;
 }
 
+bool memory_region_request_mmio_ptr(MemoryRegion *mr, hwaddr addr)
+{
+void *host;
+unsigned size = 0;
+unsigned offset = 0;
+Object *new_interface;
+
+if (!mr || !mr->ops->request_ptr) {
+return false;
+}
+
+/*
+ * Avoid an update if the request_ptr call
+ * memory_region_invalidate_mmio_ptr which seems to be likely when we use
+ * a cache.
+ */
+memory_region_transaction_begin();
+
+host = mr->ops->request_ptr(mr->opaque, addr - mr->addr, , );
+
+if (!host || !size) {
+memory_region_transaction_commit();
+return false;
+}
+
+new_interface = 

[Qemu-devel] [PULL v1 5/7] introduce mmio_interface

[Edgar E. Iglesias]

From: KONRAD Frederic 

This introduces mmio_interface object which contains a MemoryRegion
and can be hotplugged/hotunplugged.

Reviewed-by: Edgar E. Iglesias 
Signed-off-by: KONRAD Frederic 

V1 -> V2:
  * Fix the qemu_log format.
---
 hw/misc/Makefile.objs|   1 +
 hw/misc/mmio_interface.c | 128 +++
 include/hw/misc/mmio_interface.h |  49 +++
 3 files changed, 178 insertions(+)
 create mode 100644 hw/misc/mmio_interface.c
 create mode 100644 include/hw/misc/mmio_interface.h

diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
index 2019846..08a79c3 100644
--- a/hw/misc/Makefile.objs
+++ b/hw/misc/Makefile.objs
@@ -57,3 +57,4 @@ obj-$(CONFIG_EDU) += edu.o
 obj-$(CONFIG_HYPERV_TESTDEV) += hyperv_testdev.o
 obj-$(CONFIG_AUX) += auxbus.o
 obj-$(CONFIG_ASPEED_SOC) += aspeed_scu.o aspeed_sdmc.o
+obj-y += mmio_interface.o
diff --git a/hw/misc/mmio_interface.c b/hw/misc/mmio_interface.c
new file mode 100644
index 000..6f004d2
--- /dev/null
+++ b/hw/misc/mmio_interface.c
@@ -0,0 +1,128 @@
+/*
+ * mmio_interface.c
+ *
+ *  Copyright (C) 2017 : GreenSocs
+ *  http://www.greensocs.com/ , email: i...@greensocs.com
+ *
+ *  Developed by :
+ *  Frederic Konrad   
+ *
+ * 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 .
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/log.h"
+#include "trace.h"
+#include "hw/qdev-properties.h"
+#include "hw/misc/mmio_interface.h"
+#include "qapi/error.h"
+
+#ifndef DEBUG_MMIO_INTERFACE
+#define DEBUG_MMIO_INTERFACE 0
+#endif
+
+static uint64_t mmio_interface_counter;
+
+#define DPRINTF(fmt, ...) do { 
\
+if (DEBUG_MMIO_INTERFACE) {
\
+qemu_log("mmio_interface: 0x%" PRIX64 ": " fmt, s->id, ## 
__VA_ARGS__);\
+}  
\
+} while (0);
+
+static void mmio_interface_init(Object *obj)
+{
+MMIOInterface *s = MMIO_INTERFACE(obj);
+
+if (DEBUG_MMIO_INTERFACE) {
+s->id = mmio_interface_counter++;
+}
+
+DPRINTF("interface created\n");
+s->host_ptr = 0;
+s->subregion = 0;
+}
+
+static void mmio_interface_realize(DeviceState *dev, Error **errp)
+{
+MMIOInterface *s = MMIO_INTERFACE(dev);
+
+DPRINTF("realize from 0x%" PRIX64 " to 0x%" PRIX64 " map host pointer"
+" %p\n", s->start, s->end, s->host_ptr);
+
+if (!s->host_ptr) {
+error_setg(errp, "host_ptr property must be set");
+}
+
+if (!s->subregion) {
+error_setg(errp, "subregion property must be set");
+}
+
+memory_region_init_ram_ptr(>ram_mem, OBJECT(s), "ram",
+   s->end - s->start + 1, s->host_ptr);
+memory_region_set_readonly(>ram_mem, s->ro);
+memory_region_add_subregion(s->subregion, s->start, >ram_mem);
+}
+
+static void mmio_interface_unrealize(DeviceState *dev, Error **errp)
+{
+MMIOInterface *s = MMIO_INTERFACE(dev);
+
+DPRINTF("unrealize from 0x%" PRIX64 " to 0x%" PRIX64 " map host pointer"
+" %p\n", s->start, s->end, s->host_ptr);
+memory_region_del_subregion(s->subregion, >ram_mem);
+}
+
+static void mmio_interface_finalize(Object *obj)
+{
+MMIOInterface *s = MMIO_INTERFACE(obj);
+
+DPRINTF("finalize from 0x%" PRIX64 " to 0x%" PRIX64 " map host pointer"
+" %p\n", s->start, s->end, s->host_ptr);
+object_unparent(OBJECT(>ram_mem));
+}
+
+static Property mmio_interface_properties[] = {
+DEFINE_PROP_UINT64("start", MMIOInterface, start, 0),
+DEFINE_PROP_UINT64("end", MMIOInterface, end, 0),
+DEFINE_PROP_PTR("host_ptr", MMIOInterface, host_ptr),
+DEFINE_PROP_BOOL("ro", MMIOInterface, ro, false),
+DEFINE_PROP_MEMORY_REGION("subregion", MMIOInterface, subregion),
+DEFINE_PROP_END_OF_LIST(),
+};
+
+static void mmio_interface_class_init(ObjectClass *oc, void *data)
+{
+DeviceClass *dc = DEVICE_CLASS(oc);
+
+dc->realize = mmio_interface_realize;
+dc->unrealize = mmio_interface_unrealize;
+dc->props = mmio_interface_properties;
+}
+
+static const TypeInfo mmio_interface_info = {
+.name  = TYPE_MMIO_INTERFACE,
+.parent= TYPE_DEVICE,
+.instance_size = 

[Qemu-devel] [PULL v1 7/7] xilinx_spips: allow mmio execution

[Edgar E. Iglesias]

From: KONRAD Frederic 

This allows to execute from the lqspi area.

When the request_ptr is called the device loads 1024bytes from the SPI device.
Then this code can be executed by the guest.

Tested-by: Edgar E. Iglesias 
Reviewed-by: Edgar E. Iglesias 
Signed-off-by: KONRAD Frederic 
---
 hw/ssi/xilinx_spips.c | 74 ++-
 1 file changed, 55 insertions(+), 19 deletions(-)

diff --git a/hw/ssi/xilinx_spips.c b/hw/ssi/xilinx_spips.c
index da8adfa..e833028 100644
--- a/hw/ssi/xilinx_spips.c
+++ b/hw/ssi/xilinx_spips.c
@@ -496,6 +496,18 @@ static const MemoryRegionOps spips_ops = {
 .endianness = DEVICE_LITTLE_ENDIAN,
 };
 
+static void xilinx_qspips_invalidate_mmio_ptr(XilinxQSPIPS *q)
+{
+XilinxSPIPS *s = >parent_obj;
+
+if (q->lqspi_cached_addr != ~0ULL) {
+/* Invalidate the current mapped mmio */
+memory_region_invalidate_mmio_ptr(>mmlqspi, q->lqspi_cached_addr,
+  LQSPI_CACHE_SIZE);
+q->lqspi_cached_addr = ~0ULL;
+}
+}
+
 static void xilinx_qspips_write(void *opaque, hwaddr addr,
 uint64_t value, unsigned size)
 {
@@ -505,7 +517,7 @@ static void xilinx_qspips_write(void *opaque, hwaddr addr,
 addr >>= 2;
 
 if (addr == R_LQSPI_CFG) {
-q->lqspi_cached_addr = ~0ULL;
+xilinx_qspips_invalidate_mmio_ptr(q);
 }
 }
 
@@ -517,27 +529,20 @@ static const MemoryRegionOps qspips_ops = {
 
 #define LQSPI_CACHE_SIZE 1024
 
-static uint64_t
-lqspi_read(void *opaque, hwaddr addr, unsigned int size)
+static void lqspi_load_cache(void *opaque, hwaddr addr)
 {
-int i;
 XilinxQSPIPS *q = opaque;
 XilinxSPIPS *s = opaque;
-uint32_t ret;
-
-if (addr >= q->lqspi_cached_addr &&
-addr <= q->lqspi_cached_addr + LQSPI_CACHE_SIZE - 4) {
-uint8_t *retp = >lqspi_buf[addr - q->lqspi_cached_addr];
-ret = cpu_to_le32(*(uint32_t *)retp);
-DB_PRINT_L(1, "addr: %08x, data: %08x\n", (unsigned)addr,
-   (unsigned)ret);
-return ret;
-} else {
-int flash_addr = (addr / num_effective_busses(s));
-int slave = flash_addr >> LQSPI_ADDRESS_BITS;
-int cache_entry = 0;
-uint32_t u_page_save = s->regs[R_LQSPI_STS] & ~LQSPI_CFG_U_PAGE;
-
+int i;
+int flash_addr = ((addr & ~(LQSPI_CACHE_SIZE - 1))
+   / num_effective_busses(s));
+int slave = flash_addr >> LQSPI_ADDRESS_BITS;
+int cache_entry = 0;
+uint32_t u_page_save = s->regs[R_LQSPI_STS] & ~LQSPI_CFG_U_PAGE;
+
+if (addr < q->lqspi_cached_addr ||
+addr > q->lqspi_cached_addr + LQSPI_CACHE_SIZE - 4) {
+xilinx_qspips_invalidate_mmio_ptr(q);
 s->regs[R_LQSPI_STS] &= ~LQSPI_CFG_U_PAGE;
 s->regs[R_LQSPI_STS] |= slave ? LQSPI_CFG_U_PAGE : 0;
 
@@ -589,12 +594,43 @@ lqspi_read(void *opaque, hwaddr addr, unsigned int size)
 xilinx_spips_update_cs_lines(s);
 
 q->lqspi_cached_addr = flash_addr * num_effective_busses(s);
+}
+}
+
+static void *lqspi_request_mmio_ptr(void *opaque, hwaddr addr, unsigned *size,
+unsigned *offset)
+{
+XilinxQSPIPS *q = opaque;
+hwaddr offset_within_the_region = addr & ~(LQSPI_CACHE_SIZE - 1);
+
+lqspi_load_cache(opaque, offset_within_the_region);
+*size = LQSPI_CACHE_SIZE;
+*offset = offset_within_the_region;
+return q->lqspi_buf;
+}
+
+static uint64_t
+lqspi_read(void *opaque, hwaddr addr, unsigned int size)
+{
+XilinxQSPIPS *q = opaque;
+uint32_t ret;
+
+if (addr >= q->lqspi_cached_addr &&
+addr <= q->lqspi_cached_addr + LQSPI_CACHE_SIZE - 4) {
+uint8_t *retp = >lqspi_buf[addr - q->lqspi_cached_addr];
+ret = cpu_to_le32(*(uint32_t *)retp);
+DB_PRINT_L(1, "addr: %08x, data: %08x\n", (unsigned)addr,
+   (unsigned)ret);
+return ret;
+} else {
+lqspi_load_cache(opaque, addr);
 return lqspi_read(opaque, addr, size);
 }
 }
 
 static const MemoryRegionOps lqspi_ops = {
 .read = lqspi_read,
+.request_ptr = lqspi_request_mmio_ptr,
 .endianness = DEVICE_NATIVE_ENDIAN,
 .valid = {
 .min_access_size = 1,
-- 
2.7.4

[Qemu-devel] [PULL v1 4/7] qdev: add MemoryRegion property

[Edgar E. Iglesias]

From: KONRAD Frederic 

We need to pass a pointer to a MemoryRegion for mmio_interface.
So this just adds that.

Reviewed-by: Edgar E. Iglesias 
Signed-off-by: KONRAD Frederic 
---
 include/hw/qdev-properties.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
index d206fc9..0e9b492 100644
--- a/include/hw/qdev-properties.h
+++ b/include/hw/qdev-properties.h
@@ -171,6 +171,8 @@ extern PropertyInfo qdev_prop_arraylen;
 DEFINE_PROP_DEFAULT(_n, _s, _f, 0, qdev_prop_blocksize, uint16_t)
 #define DEFINE_PROP_PCI_HOST_DEVADDR(_n, _s, _f) \
 DEFINE_PROP(_n, _s, _f, qdev_prop_pci_host_devaddr, PCIHostDeviceAddress)
+#define DEFINE_PROP_MEMORY_REGION(_n, _s, _f) \
+DEFINE_PROP(_n, _s, _f, qdev_prop_ptr, MemoryRegion *)
 
 #define DEFINE_PROP_END_OF_LIST()   \
 {}
-- 
2.7.4

[Qemu-devel] [PULL v1 3/7] cputlb: fix the way get_page_addr_code fills the tlb

[Edgar E. Iglesias]

From: KONRAD Frederic 

get_page_addr_code(..) does a cpu_ldub_code to fill the tlb:
This can lead to some side effects if a device is mapped at this address.

So this patch replaces the cpu_memory_ld by a tlb_fill.

Reviewed-by: Richard Henderson 
Reviewed-by: Edgar E. Iglesias 
Signed-off-by: KONRAD Frederic 
---
 cputlb.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/cputlb.c b/cputlb.c
index 5d6c755..95265a0 100644
--- a/cputlb.c
+++ b/cputlb.c
@@ -849,8 +849,10 @@ tb_page_addr_t get_page_addr_code(CPUArchState *env, 
target_ulong addr)
 index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1);
 mmu_idx = cpu_mmu_index(env, true);
 if (unlikely(env->tlb_table[mmu_idx][index].addr_code !=
- (addr & TARGET_PAGE_MASK))) {
-cpu_ldub_code(env, addr);
+ (addr & (TARGET_PAGE_MASK | TLB_INVALID_MASK {
+if (!VICTIM_TLB_HIT(addr_read, addr)) {
+tlb_fill(ENV_GET_CPU(env), addr, MMU_INST_FETCH, mmu_idx, 0);
+}
 }
 iotlbentry = >iotlb[mmu_idx][index];
 pd = iotlbentry->addr & ~TARGET_PAGE_MASK;
-- 
2.7.4

[Qemu-devel] [PULL v1 2/7] cputlb: move get_page_addr_code

[Edgar E. Iglesias]

From: KONRAD Frederic 

This just moves the code before VICTIM_TLB_HIT macro definition
so we can use it.

Reviewed-by: Richard Henderson 
Reviewed-by: Edgar E. Iglesias 
Signed-off-by: KONRAD Frederic 

V2 -> V3:
  * Rebase against cpu_unaligned access recent change.
---
 cputlb.c | 70 
 1 file changed, 35 insertions(+), 35 deletions(-)

diff --git a/cputlb.c b/cputlb.c
index 1cc382d..5d6c755 100644
--- a/cputlb.c
+++ b/cputlb.c
@@ -746,41 +746,6 @@ static inline ram_addr_t 
qemu_ram_addr_from_host_nofail(void *ptr)
 return ram_addr;
 }
 
-/* NOTE: this function can trigger an exception */
-/* NOTE2: the returned address is not exactly the physical address: it
- * is actually a ram_addr_t (in system mode; the user mode emulation
- * version of this function returns a guest virtual address).
- */
-tb_page_addr_t get_page_addr_code(CPUArchState *env, target_ulong addr)
-{
-int mmu_idx, index, pd;
-void *p;
-MemoryRegion *mr;
-CPUState *cpu = ENV_GET_CPU(env);
-CPUIOTLBEntry *iotlbentry;
-
-index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1);
-mmu_idx = cpu_mmu_index(env, true);
-if (unlikely(env->tlb_table[mmu_idx][index].addr_code !=
- (addr & TARGET_PAGE_MASK))) {
-cpu_ldub_code(env, addr);
-}
-iotlbentry = >iotlb[mmu_idx][index];
-pd = iotlbentry->addr & ~TARGET_PAGE_MASK;
-mr = iotlb_to_region(cpu, pd, iotlbentry->attrs);
-if (memory_region_is_unassigned(mr)) {
-cpu_unassigned_access(cpu, addr, false, true, 0, 4);
-/* The CPU's unassigned access hook might have longjumped out
- * with an exception. If it didn't (or there was no hook) then
- * we can't proceed further.
- */
-report_bad_exec(cpu, addr);
-exit(1);
-}
-p = (void *)((uintptr_t)addr + env->tlb_table[mmu_idx][index].addend);
-return qemu_ram_addr_from_host_nofail(p);
-}
-
 static uint64_t io_readx(CPUArchState *env, CPUIOTLBEntry *iotlbentry,
  target_ulong addr, uintptr_t retaddr, int size)
 {
@@ -868,6 +833,41 @@ static bool victim_tlb_hit(CPUArchState *env, size_t 
mmu_idx, size_t index,
   victim_tlb_hit(env, mmu_idx, index, offsetof(CPUTLBEntry, TY), \
  (ADDR) & TARGET_PAGE_MASK)
 
+/* NOTE: this function can trigger an exception */
+/* NOTE2: the returned address is not exactly the physical address: it
+ * is actually a ram_addr_t (in system mode; the user mode emulation
+ * version of this function returns a guest virtual address).
+ */
+tb_page_addr_t get_page_addr_code(CPUArchState *env, target_ulong addr)
+{
+int mmu_idx, index, pd;
+void *p;
+MemoryRegion *mr;
+CPUState *cpu = ENV_GET_CPU(env);
+CPUIOTLBEntry *iotlbentry;
+
+index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1);
+mmu_idx = cpu_mmu_index(env, true);
+if (unlikely(env->tlb_table[mmu_idx][index].addr_code !=
+ (addr & TARGET_PAGE_MASK))) {
+cpu_ldub_code(env, addr);
+}
+iotlbentry = >iotlb[mmu_idx][index];
+pd = iotlbentry->addr & ~TARGET_PAGE_MASK;
+mr = iotlb_to_region(cpu, pd, iotlbentry->attrs);
+if (memory_region_is_unassigned(mr)) {
+cpu_unassigned_access(cpu, addr, false, true, 0, 4);
+/* The CPU's unassigned access hook might have longjumped out
+ * with an exception. If it didn't (or there was no hook) then
+ * we can't proceed further.
+ */
+report_bad_exec(cpu, addr);
+exit(1);
+}
+p = (void *)((uintptr_t)addr + env->tlb_table[mmu_idx][index].addend);
+return qemu_ram_addr_from_host_nofail(p);
+}
+
 /* Probe for whether the specified guest write access is permitted.
  * If it is not permitted then an exception will be taken in the same
  * way as if this were a real write access (and we will not return).
-- 
2.7.4

[Qemu-devel] [PULL v1 0/7] MMIO Exec pull request

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

Hi,

Paolo suggested offline that we send a pull request for this series.
Here it is, I've run it through my testsuite + tested the LQSPI testcase
on Zynq.

Cheers,
Edgar

The following changes since commit 3f0602927b120a480b35dcf58cf6f95435b3ae91:

  Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20170613' 
into staging (2017-06-13 15:49:07 +0100)

are available in the git repository at:

  g...@github.com:edgarigl/qemu.git tags/edgar/mmio-exec.for-upstream

for you to fetch changes up to 63ef40dd6bc6cfdae5fa67ccac1cb11e7a5161b0:

  xilinx_spips: allow mmio execution (2017-06-14 17:31:08 +0200)


mmio-exec.for-upstream


KONRAD Frederic (7):
  cputlb: cleanup get_page_addr_code to use VICTIM_TLB_HIT
  cputlb: move get_page_addr_code
  cputlb: fix the way get_page_addr_code fills the tlb
  qdev: add MemoryRegion property
  introduce mmio_interface
  exec: allow to get a pointer for some mmio memory region
  xilinx_spips: allow mmio execution

 cputlb.c |  82 ++---
 hw/misc/Makefile.objs|   1 +
 hw/misc/mmio_interface.c | 128 +++
 hw/ssi/xilinx_spips.c|  74 --
 include/exec/memory.h|  35 +++
 include/hw/misc/mmio_interface.h |  49 +++
 include/hw/qdev-properties.h |   2 +
 memory.c | 111 +
 8 files changed, 428 insertions(+), 54 deletions(-)
 create mode 100644 hw/misc/mmio_interface.c
 create mode 100644 include/hw/misc/mmio_interface.h

[Qemu-devel] [PULL v1 1/7] cputlb: cleanup get_page_addr_code to use VICTIM_TLB_HIT

[Edgar E. Iglesias]

From: KONRAD Frederic 

This replaces env1 and page_index variables by env and index
so we can use VICTIM_TLB_HIT macro later.

Reviewed-by: Richard Henderson 
Reviewed-by: Edgar E. Iglesias 
Signed-off-by: KONRAD Frederic 
---
 cputlb.c | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/cputlb.c b/cputlb.c
index 743776a..1cc382d 100644
--- a/cputlb.c
+++ b/cputlb.c
@@ -751,21 +751,21 @@ static inline ram_addr_t 
qemu_ram_addr_from_host_nofail(void *ptr)
  * is actually a ram_addr_t (in system mode; the user mode emulation
  * version of this function returns a guest virtual address).
  */
-tb_page_addr_t get_page_addr_code(CPUArchState *env1, target_ulong addr)
+tb_page_addr_t get_page_addr_code(CPUArchState *env, target_ulong addr)
 {
-int mmu_idx, page_index, pd;
+int mmu_idx, index, pd;
 void *p;
 MemoryRegion *mr;
-CPUState *cpu = ENV_GET_CPU(env1);
+CPUState *cpu = ENV_GET_CPU(env);
 CPUIOTLBEntry *iotlbentry;
 
-page_index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1);
-mmu_idx = cpu_mmu_index(env1, true);
-if (unlikely(env1->tlb_table[mmu_idx][page_index].addr_code !=
+index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1);
+mmu_idx = cpu_mmu_index(env, true);
+if (unlikely(env->tlb_table[mmu_idx][index].addr_code !=
  (addr & TARGET_PAGE_MASK))) {
-cpu_ldub_code(env1, addr);
+cpu_ldub_code(env, addr);
 }
-iotlbentry = >iotlb[mmu_idx][page_index];
+iotlbentry = >iotlb[mmu_idx][index];
 pd = iotlbentry->addr & ~TARGET_PAGE_MASK;
 mr = iotlb_to_region(cpu, pd, iotlbentry->attrs);
 if (memory_region_is_unassigned(mr)) {
@@ -777,7 +777,7 @@ tb_page_addr_t get_page_addr_code(CPUArchState *env1, 
target_ulong addr)
 report_bad_exec(cpu, addr);
 exit(1);
 }
-p = (void *)((uintptr_t)addr + 
env1->tlb_table[mmu_idx][page_index].addend);
+p = (void *)((uintptr_t)addr + env->tlb_table[mmu_idx][index].addend);
 return qemu_ram_addr_from_host_nofail(p);
 }
 
-- 
2.7.4

Re: [Qemu-devel] [PATCH] vhost-user: support cross-endianess negatiation

[Felipe Franciosi]

Hello!

On 14 Jun 2017, at 18:17, Marc-André Lureau 
> wrote:

H

On Wed, Jun 14, 2017 at 8:42 PM Felipe Franciosi 
> wrote:

...
@@ -580,6 +581,19 @@ Master message types
   This request should be send only when VIRTIO_F_IOMMU_PLATFORM feature
   has been successfully negotiated.

+ * VHOST_USER_SET_VRING_ENDIAN
+
+  Id: 22
+  Equivalent ioctl: VHOST_SET_VRING_ENDIAN
+  Master payload: vring state description
+
+  Set the endianess of a VQ for legacy devices. Big-endian is indicated
+  with state.num set to true, and little-endian is indicated with state.num
+  set to false.
+  This request should be sent only when VHOST_USER_PROTOCOL_F_CROSS_ENDIAN
+  has been negotiated.

And it can be sent at any time? multiple times? Do we expect the backend to 
handle any endianess?

Thanks for the feedback. I tried to write the docs to the same level of detail 
of the other messages, but agree that we should start to make an effort to have 
more elaborate documentation. I'll address your questions on the spec and send 
a v2.

I also noticed the "id" above (on the doc) should be 23.

Thanks,
Felipe

Re: [Qemu-devel] [PATCH] vhost-user: support cross-endianess negatiation

[Juan Quintela]

Paolo Bonzini  wrote:
> On 14/06/2017 19:02, Felipe Franciosi wrote:
>> 
>>> On 14 Jun 2017, at 17:59, no-re...@patchew.org wrote:
>>>
>>> Hi,
>>>
>>> This series failed automatic build test. Please find the testing commands 
>>> and
>>> their output below. If you have docker installed, you can probably 
>>> reproduce it
>>> locally.
>>>
>>> Message-id: 1497458486-15673-1-git-send-email-fel...@nutanix.com
>>> Type: series
>>> Subject: [Qemu-devel] [PATCH] vhost-user: support cross-endianess 
>>> negatiation
>>>
>> ...
>>>
>>>  CC  hw/pci-bridge/pci_expander_bridge.o
>>> In file included from /tmp/qemu-test/src/hw/net/vmxnet3.c:30:
>>> /tmp/qemu-test/src/include/migration/register.h:18: error:
>>> redefinition of typedef ‘LoadStateHandler’
>>> /tmp/qemu-test/src/include/migration/vmstate.h:32: note: previous
>>> declaration of ‘LoadStateHandler’ was here
>>> make: *** [hw/net/vmxnet3.o] Error 1
>>> make: *** Waiting for unfinished jobs
>>> tests/docker/Makefile.include:118: recipe for target 'docker-run' failed
>>> make[1]: *** [docker-run] Error 2
>>> make[1]: Leaving directory '/var/tmp/patchew-tester-tmp-enl3ixyr/src'
>>> tests/docker/Makefile.include:149: recipe for target
>>> 'docker-run-test-build@min-glib' failed
>>> make: *** [docker-run-test-build@min-glib] Error 2
>>> === OUTPUT END ===
>>>
>>> Test command exited with code: 2
>> 
>> That seems completely unrelated to what our patch touched... I'm
>> gonna guess this is a known issue and I can safely ignore it?
>
> Yes.
>
> Juan, can you take a look?  Just removing the typedef from
> VMStateDescription, and instead just using int(*load_state_old)(etc...)
> should do it.

There is a patch already on list, reviewed, and on a PULL request.

Problem is that new complires don't complain about a repeated typedef.
Older ones do.  I compile on F25, and there it don't warn/error.

Sorry for the noise.

Later, Juan.

Re: [Qemu-devel] [PATCH 1/3] travis: install more library dependencies

[Paolo Bonzini]

On 14/06/2017 19:04, Peter Maydell wrote:
> On 14 June 2017 at 17:49, Paolo Bonzini  wrote:
>> Well, trusty is 3 years old by now... I wouldn't call that bleeding
>> edge, and it seems like Travis is suggesting using Docker images for
>> those who want to use a newer distro.  This patch and patch 2 are
>> useful, but I think I'd rather get full coverage, either with Shippable
>> or by keeping on doing manual builds, than to rush things and switch to
>> CI when it's not ready.
> 
> Yes, I overall agree that we maybe don't want to use Travis
> for this, but I would like us to automate it somehow.
> (I was about 50/50 on whether to tag the patchset as RFC.)
> 
>> First, I don't think it's accurate to say that scans have been often
>> weeks or months apart:
>>
>> #days   #commits
>> 2017-06-05  4   123
>> 2017-06-01  14  214
>> 2017-05-18  3   108
>> 2017-05-15  8   262
>> 2017-05-07  12  149
>> 2017-04-25  24  317
> 
> Yes, but this one (I think) only happened because I got fed
> up enough of the build being out of date to go and find out
> how to rebuild it and do an upload. I think I also did the
> 1st June one by hand, maybe?

Yes, that one I was super-busy (and travelling until April 16).  On June
1 and June 12 we crossed, you did one at the same time as me but you
must have faster internet uplink (not hard :)).

> I'm more likely to look at coverity during freeze periods
> than less, because bugs coverity notices are more likely
> than not to be candidates for being worth fixing before
> releases, and I don't have my plate full with feature work.
> So I'd rather have the build be as up to date as possible
> during a release so we can catch any bugs that snuck in
> before we hit the last release candidate.

Understood, on the other hand during freeze periods it's easier to look
at what went in and see how safe it is.

> Conversely, if we don't do scans very frequently then the
> "outstanding defects" view gets hard to use because it's
> still showing things we've already fixed and isn't showing
> new things we've introduced but not scanned yet.
The beauty of doing manual scans is that you can do them when that pull
request with lots of Coverity fixes has just gone in. :)  Seriously, I
didn't think frequency was a problem and we must have different
workflows.  I rely more on "all newly detected"/"all newly fixed" than
on the "new" state, because that can more easily show problems in the
build environment, though admittedly a more reproducible scan recipe
makes that less relevant.

Paolo

Re: [Qemu-devel] [PATCH] vhost-user: support cross-endianess negatiation

[Marc-André Lureau]

H

On Wed, Jun 14, 2017 at 8:42 PM Felipe Franciosi  wrote:

> Currently, vhost-user does not implement any means for notifying the
> backend about guest endianess. This commit introduces a new message
> called VHOST_USER_SET_VRING_ENDIAN which is analogous to the ioctl()
> called VHOST_SET_VRING_ENDIAN used for kernel vhost backends. Such
> message is necessary for backends supporting legacy (pre-1.0) virtio
> devices running in big-endian guests.
>
> Signed-off-by: Felipe Franciosi 
> Signed-off-by: Mike Cui 
> ---
>  docs/specs/vhost-user.txt | 14 ++
>  hw/virtio/vhost-user.c| 23 +--
>  2 files changed, 35 insertions(+), 2 deletions(-)
>
> diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
> index 481ab56..eafbed9 100644
> --- a/docs/specs/vhost-user.txt
> +++ b/docs/specs/vhost-user.txt
> @@ -326,6 +326,7 @@ Protocol features
>  #define VHOST_USER_PROTOCOL_F_REPLY_ACK  3
>  #define VHOST_USER_PROTOCOL_F_MTU4
>  #define VHOST_USER_PROTOCOL_F_SLAVE_REQ  5
> +#define VHOST_USER_PROTOCOL_F_CROSS_ENDIAN   6
>
>  Master message types
>  
> @@ -580,6 +581,19 @@ Master message types
>This request should be send only when VIRTIO_F_IOMMU_PLATFORM
> feature
>has been successfully negotiated.
>
> + * VHOST_USER_SET_VRING_ENDIAN
> +
> +  Id: 22
> +  Equivalent ioctl: VHOST_SET_VRING_ENDIAN
> +  Master payload: vring state description
> +
> +  Set the endianess of a VQ for legacy devices. Big-endian is
> indicated
> +  with state.num set to true, and little-endian is indicated with
> state.num
> +  set to false.
> +  This request should be sent only when
> VHOST_USER_PROTOCOL_F_CROSS_ENDIAN
> +  has been negotiated.
>

And it can be sent at any time? multiple times? Do we expect the backend to
handle any endianess?


> +
> +
>  Slave message types
>  ---
>
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index 958ee09..006af1c 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -33,6 +33,7 @@ enum VhostUserProtocolFeature {
>  VHOST_USER_PROTOCOL_F_REPLY_ACK = 3,
>  VHOST_USER_PROTOCOL_F_NET_MTU = 4,
>  VHOST_USER_PROTOCOL_F_SLAVE_REQ = 5,
> +VHOST_USER_PROTOCOL_F_CROSS_ENDIAN = 6,
>
>  VHOST_USER_PROTOCOL_F_MAX
>  };
> @@ -63,6 +64,7 @@ typedef enum VhostUserRequest {
>  VHOST_USER_NET_SET_MTU = 20,
>  VHOST_USER_SET_SLAVE_REQ_FD = 21,
>  VHOST_USER_IOTLB_MSG = 22,
> +VHOST_USER_SET_VRING_ENDIAN = 23,
>  VHOST_USER_MAX
>  } VhostUserRequest;
>
> @@ -367,8 +369,25 @@ static int vhost_user_set_vring_addr(struct vhost_dev
> *dev,
>  static int vhost_user_set_vring_endian(struct vhost_dev *dev,
> struct vhost_vring_state *ring)
>  {
> -error_report("vhost-user trying to send unhandled ioctl");
> -return -1;
> +bool cross_endian = virtio_has_feature(dev->protocol_features,
> +
>  VHOST_USER_PROTOCOL_F_CROSS_ENDIAN);
> +VhostUserMsg msg = {
> +.request = VHOST_USER_SET_VRING_ENDIAN,
> +.flags = VHOST_USER_VERSION,
> +.payload.state = *ring,
> +.size = sizeof(msg.payload.state),
> +};
> +
> +if (!cross_endian) {
> +error_report("vhost-user trying to send unhandled ioctl");
> +return -1;
> +}
> +
> +if (vhost_user_write(dev, , NULL, 0) < 0) {
> +return -1;
> +}
> +
> +return 0;
>  }
>
>  static int vhost_set_vring(struct vhost_dev *dev,
> --
> 1.9.5
>
>
> --
Marc-André Lureau

Re: [Qemu-devel] [PATCH] vhost-user: support cross-endianess negatiation

[Paolo Bonzini]

On 14/06/2017 19:02, Felipe Franciosi wrote:
> 
>> On 14 Jun 2017, at 17:59, no-re...@patchew.org wrote:
>>
>> Hi,
>>
>> This series failed automatic build test. Please find the testing commands and
>> their output below. If you have docker installed, you can probably reproduce 
>> it
>> locally.
>>
>> Message-id: 1497458486-15673-1-git-send-email-fel...@nutanix.com
>> Type: series
>> Subject: [Qemu-devel] [PATCH] vhost-user: support cross-endianess negatiation
>>
> ...
>>
>>  CC  hw/pci-bridge/pci_expander_bridge.o
>> In file included from /tmp/qemu-test/src/hw/net/vmxnet3.c:30:
>> /tmp/qemu-test/src/include/migration/register.h:18: error: redefinition of 
>> typedef ‘LoadStateHandler’
>> /tmp/qemu-test/src/include/migration/vmstate.h:32: note: previous 
>> declaration of ‘LoadStateHandler’ was here
>> make: *** [hw/net/vmxnet3.o] Error 1
>> make: *** Waiting for unfinished jobs
>> tests/docker/Makefile.include:118: recipe for target 'docker-run' failed
>> make[1]: *** [docker-run] Error 2
>> make[1]: Leaving directory '/var/tmp/patchew-tester-tmp-enl3ixyr/src'
>> tests/docker/Makefile.include:149: recipe for target 
>> 'docker-run-test-build@min-glib' failed
>> make: *** [docker-run-test-build@min-glib] Error 2
>> === OUTPUT END ===
>>
>> Test command exited with code: 2
> 
> That seems completely unrelated to what our patch touched... I'm gonna guess 
> this is a known issue and I can safely ignore it?

Yes.

Juan, can you take a look?  Just removing the typedef from
VMStateDescription, and instead just using int(*load_state_old)(etc...)
should do it.

Paolo

Re: [Qemu-devel] [PATCH v1 2/3] tcg-runtime: light re-factor of lookup_tb_ptr

[Pranith Kumar]

Hi Alex,

On Wed, Jun 14, 2017 at 10:02 AM, Alex Bennée  wrote:
> Just a little precursor re-factoring before I was going to add a trace
> point:
>
>   - single return point, defaulting to tcg_ctx.code_gen_epilogue
>   - move cs_base, pc and flags inside the jump cache hit scope
>   - calculate the tb_jmp_cache hash once
>
> Signed-off-by: Alex Bennée 
> ---
>  tcg-runtime.c | 35 +++
>  1 file changed, 19 insertions(+), 16 deletions(-)

Phew, glad you got this figured out! I tested it on the images I have
and it works. Please add:

Tested-by: Pranith Kumar 


>
> diff --git a/tcg-runtime.c b/tcg-runtime.c
> index 7fa90ce508..f4bfa9cea6 100644
> --- a/tcg-runtime.c
> +++ b/tcg-runtime.c
> @@ -147,30 +147,33 @@ uint64_t HELPER(ctpop_i64)(uint64_t arg)
>  void *HELPER(lookup_tb_ptr)(CPUArchState *env, target_ulong addr)
>  {
>  CPUState *cpu = ENV_GET_CPU(env);
> +unsigned int addr_hash = tb_jmp_cache_hash_func(addr);
> +void *code_ptr = NULL;
>  TranslationBlock *tb;
> -target_ulong cs_base, pc;
> -uint32_t flags;
>
> -tb = atomic_rcu_read(>tb_jmp_cache[tb_jmp_cache_hash_func(addr)]);
> +tb = atomic_rcu_read(>tb_jmp_cache[addr_hash]);
>  if (likely(tb)) {
> +target_ulong cs_base, pc;
> +uint32_t flags;
> +
>  cpu_get_tb_cpu_state(env, , _base, );
> +
>  if (likely(tb->pc == addr && tb->cs_base == cs_base &&
> tb->flags == flags)) {
> -goto found;
> -}
> -tb = tb_htable_lookup(cpu, addr, cs_base, flags);
> -if (likely(tb)) {
> -atomic_set(>tb_jmp_cache[tb_jmp_cache_hash_func(addr)], tb);
> -goto found;
> +code_ptr = tb->tc_ptr;
> +} else {
> +/* If we didn't find it in the jmp_cache we still might
> + * find it in the global tb_htable
> + */
> +tb = tb_htable_lookup(cpu, addr, cs_base, flags);
> +if (likely(tb)) {
> +atomic_set(>tb_jmp_cache[addr_hash], tb);
> +code_ptr = tb->tc_ptr;
> +}
>  }
>  }
> -return tcg_ctx.code_gen_epilogue;
> - found:
> -qemu_log_mask_and_addr(CPU_LOG_EXEC, addr,
> -   "Chain %p [%d: " TARGET_FMT_lx "] %s\n",
> -   tb->tc_ptr, cpu->cpu_index, addr,
> -   lookup_symbol(addr));
> -return tb->tc_ptr;
> +
> +return code_ptr ? code_ptr : tcg_ctx.code_gen_epilogue;
>  }
>
>  void HELPER(exit_atomic)(CPUArchState *env)
> --
> 2.13.0
>
>

Re: [Qemu-devel] [PATCH v1 3/3] tcg-runtime: short-circuit lookup_tb_ptr on IRQs

[Paolo Bonzini]

On 14/06/2017 18:51, Richard Henderson wrote:
> On 06/14/2017 09:08 AM, Paolo Bonzini wrote:
>> I think this is a band-aid, and would rather fix the front-ends as in
>> Emilio's patch.  For Alpha my guess would be:
>>
>> diff --git a/target/alpha/translate.c b/target/alpha/translate.c
>> index 7c45ae360c..6e2ee3f958 100644
>> --- a/target/alpha/translate.c
>> +++ b/target/alpha/translate.c
>> @@ -1198,7 +1198,9 @@ static ExitStatus gen_call_pal(DisasContext
>> *ctx, int palcode)
>>   tcg_gen_andi_i64(tmp, ctx->ir[IR_A0], PS_INT_MASK);
>>tcg_gen_st8_i64(tmp, cpu_env, offsetof(CPUAlphaState,
>> ps));
>>   tcg_temp_free(tmp);
>> -break;
>> +
>> +/* Reevaluate interrupts */
>> +return EXIT_PC_STALE;
>> case 0x36:
>>   /* RDPS */
> 
> Thanks!
> 
> You're right that adjusting SWPIPL along these lines does fix the
> problem for Alpha.  Given that Alpha would typically hang in arch_idle,
> I'd been focusing primarily on WTINT.

And MIPS:

diff --git a/target/mips/translate.c b/target/mips/translate.c
index 559f8fed89..244f3cb9ab 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -13403,8 +13403,9 @@ static void gen_pool32axf (CPUMIPSState *env, 
DisasContext *ctx, int rt, int rs)
 save_cpu_state(ctx, 1);
 gen_helper_ei(t0, cpu_env);
 gen_store_gpr(t0, rs);
-/* Stop translation as we may have switched the execution mode 
*/
-ctx->bstate = BS_STOP;
+/* BS_STOP isn't good enough here, reevaluate 
cpu_mips_hw_interrupts_enabled. */
+gen_save_pc(ctx->pc + 4);
+ctx->bstate = BS_EXCP;
 tcg_temp_free(t0);
 }
 break;

The others seem okay.

Paolo

Re: [Qemu-devel] [PATCH v7 6/9] qcow2: add bdrv_measure() support

[Stefan Hajnoczi]

On Tue, Jun 13, 2017 at 05:07:13PM +0200, Alberto Garcia wrote:
> On Tue 13 Jun 2017 03:33:26 PM CEST, Stefan Hajnoczi  
> wrote:
> > Use qcow2_calc_prealloc_size() to get the required file size.
> >
> > Signed-off-by: Stefan Hajnoczi 
> > Reviewed-by: Alberto Garcia 
> 
> You kept my R-b here but one of the changes was in this patch:

Sorry, I forgot to remove it when making the change.

> > +info = g_new(BlockMeasureInfo, 1);
> > +info->fully_allocated =
> > +qcow2_calc_prealloc_size(virtual_size, cluster_size,
> > + ctz32(refcount_bits));
> > +if (DIV_ROUND_UP(info->fully_allocated, cluster_size) > INT_MAX) {
> > +g_free(info);
> > +error_setg(_err, "The image size is too large "
> > +   "(try using a larger cluster size)");
> > +goto err;
> > +}
> 
> This has the opposite problem than the previous version: valid image
> sizes are now rejected by the 'measure' command.
> 
> $ qemu-img create -f qcow2 img.qcow2 1P
> Formatting 'img.qcow2', fmt=qcow2 size=1125899906842624 encryption=off 
> cluster_size=65536 lazy_refcounts=off refcount_bits=16
> 
> $ build/qemu-img measure -O qcow2 --size 1P
> qemu-img: The image size is too large (try using a larger cluster size)

Hmm...if host file size (not virtual disk size) is 1P then qemu-img
check fails:

int qcow2_check_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
  BdrvCheckMode fix)
{
int64_t size, highest_cluster, nb_clusters;
...

size = bdrv_getlength(bs->file->bs);
if (size < 0) {
res->check_errors++;
return size;
}

nb_clusters = size_to_clusters(s, size);
if (nb_clusters > INT_MAX) {
res->check_errors++;
return -EFBIG;
}

This is also where I got the limit from.  It was introduced in:

commit 0abe740f1de899737242bcba1fb4a9857f7a3087
Author: Kevin Wolf 
Date:   Wed Mar 26 13:05:52 2014 +0100

qcow2: Protect against some integer overflows in bdrv_check

At that point the code used ints so the overflow check was necessary.
Now the code uses 64-bit types so INT_MAX is obsolete.

I will send a separate patch to fix qemu-img check.

> The actual limit is:
> 
> #define QCOW_MAX_L1_SIZE 0x200
> 
> That's 4194304 entries, each one can address cluster_size^2 / 8 bytes
> 
> So using that formula, here is the maximum virtual size depending on the
> cluster size:
> 
> |--+--|
> | Cluster size | Max virtual size |
> |--+--|
> | 512 bytes| 128 GB   |
> | 1 KB | 512 GB   |
> | 2 KB | 2 TB |
> | 4 KB | 8 TB |
> | 8 KB | 32 TB|
> | 16 KB| 128 TB   |
> | 32 KB| 512 TB   |
> | 64 KB| 2 PB |
> | 128 KB   | 8 PB |
> | 256 KB   | 32 PB|
> | 512 KB   | 128 PB   |
> | 1 MB | 512 PB   |
> | 2 MB | 2 EB |
> |--+--|
> 
> I just created a 2 EB image and it works fine, Linux can detect it
> without problems, I can create a file system, etc.
> 
> If you specify a larger size, qcow2_grow_l1_table() fails with -EFIB.

Thanks, will fix!

Stefan


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v1 2/3] target/s390x: implement mvcos instruction

[David Hildenbrand]

>> Would it makes sense to
>>
>> a) move cpu_restore_state() into program_interrupt()
>> b) make all callers forward ra from GETPC() (problem with kvm code that
>> share handlers?)
>> c) fixup callers that already do the cpu_restore_state()
>> d) drop potential_page_fault() completely
> 
> Yes, that makes sense.  For B, kvm can pass 0 for RA and nothing will happen. 
> For C, that project is on-going but not complete; D is indeed the ultimate 
> goal.
> 
>> Two questions:
>> a) Could we avoid having to forward the ra by doing GETPC directly in
>> program_interrupt()? In mem_helper, I can see that we do GETPC on
>> several places and pass it around, so I assume GETPC() has to be called
>> in the first handler?
> 
> You must use GETPC in the first handler.  We're looking for the address of 
> the 
> TCG generated code from where we were called.  So, no, you can't use GETPC 
> from 
> program_interrupt.
> 
>> b) With cpu_restore_state(), there is no need for update_psw_addr() +
>> update_cc_op(), correct?
> 
> Correct.

Thanks for the clarification!

> 
 +potential_page_fault(s);
 +gen_helper_mvcos(cc_op, cpu_env, o->addr1, o->in2, regs[r3]);
>>>
>>> ... the potential_page_fault.
>>
>> I would suggest to leave it in this patch as it and then clean it up all
>> together in one shot (adding 5 cpu_restore_state() vs. one
>> potential_page_fault() temporarily looks better to me).
> 
> I would say the opposite, since the code generated by potential_page_fault is 
> always executed, whereas the cpu_restore_state is on an error path which for 
> a 
> well-behaved guest will never be executed.

By temporary I meant:
I will be looking into cleaning this all up and getting rid of
potential_page_fault() soon :)

However, in v2 I avoided potential_page_fault().

> 
> 
> r~
> 

Thanks!

-- 

Thanks,

David

Re: [Qemu-devel] [PATCH 1/3] travis: install more library dependencies

[Peter Maydell]

On 14 June 2017 at 17:49, Paolo Bonzini  wrote:
> Well, trusty is 3 years old by now... I wouldn't call that bleeding
> edge, and it seems like Travis is suggesting using Docker images for
> those who want to use a newer distro.  This patch and patch 2 are
> useful, but I think I'd rather get full coverage, either with Shippable
> or by keeping on doing manual builds, than to rush things and switch to
> CI when it's not ready.

Yes, I overall agree that we maybe don't want to use Travis
for this, but I would like us to automate it somehow.
(I was about 50/50 on whether to tag the patchset as RFC.)

> First, I don't think it's accurate to say that scans have been often
> weeks or months apart:
>
> #days   #commits
> 2017-06-05  4   123
> 2017-06-01  14  214
> 2017-05-18  3   108
> 2017-05-15  8   262
> 2017-05-07  12  149
> 2017-04-25  24  317

Yes, but this one (I think) only happened because I got fed
up enough of the build being out of date to go and find out
how to rebuild it and do an upload. I think I also did the
1st June one by hand, maybe?

The reason I put this patch set together is because I didn't
want to have to do a manual build a third time :-)

> In the last eight months, there was exactly one case where the builds
> were more than one month apart and one more case where the builds were
> more than two weeks apart.  Both of them coincided with the two most
> recent hard freeze periods (2.8 and 2.9).

I'm more likely to look at coverity during freeze periods
than less, because bugs coverity notices are more likely
than not to be candidates for being worth fixing before
releases, and I don't have my plate full with feature work.
So I'd rather have the build be as up to date as possible
during a release so we can catch any bugs that snuck in
before we hit the last release candidate.

> Second, I don't even think that CI is particularly useful when someone
> must actively consume those scans: triage newly-reporte defects, inform
> the authors of the patch, and so on.  Too many Coverity reports can be a
> burden because you cannot use e.g. the "All newly detected" view.

You can do triage at any frequency you want, because bugs stay
in the "new" state until you move them, whether they were
detected in the most recent scan or not.

Conversely, if we don't do scans very frequently then the
"outstanding defects" view gets hard to use because it's
still showing things we've already fixed and isn't showing
new things we've introduced but not scanned yet.

thanks
-- PMM

Re: [Qemu-devel] [PATCH] vhost-user: support cross-endianess negatiation

[Felipe Franciosi]

> On 14 Jun 2017, at 17:59, no-re...@patchew.org wrote:
> 
> Hi,
> 
> This series failed automatic build test. Please find the testing commands and
> their output below. If you have docker installed, you can probably reproduce 
> it
> locally.
> 
> Message-id: 1497458486-15673-1-git-send-email-fel...@nutanix.com
> Type: series
> Subject: [Qemu-devel] [PATCH] vhost-user: support cross-endianess negatiation
> 
...
> 
>  CC  hw/pci-bridge/pci_expander_bridge.o
> In file included from /tmp/qemu-test/src/hw/net/vmxnet3.c:30:
> /tmp/qemu-test/src/include/migration/register.h:18: error: redefinition of 
> typedef ‘LoadStateHandler’
> /tmp/qemu-test/src/include/migration/vmstate.h:32: note: previous declaration 
> of ‘LoadStateHandler’ was here
> make: *** [hw/net/vmxnet3.o] Error 1
> make: *** Waiting for unfinished jobs
> tests/docker/Makefile.include:118: recipe for target 'docker-run' failed
> make[1]: *** [docker-run] Error 2
> make[1]: Leaving directory '/var/tmp/patchew-tester-tmp-enl3ixyr/src'
> tests/docker/Makefile.include:149: recipe for target 
> 'docker-run-test-build@min-glib' failed
> make: *** [docker-run-test-build@min-glib] Error 2
> === OUTPUT END ===
> 
> Test command exited with code: 2

That seems completely unrelated to what our patch touched... I'm gonna guess 
this is a known issue and I can safely ignore it?

Thanks,
F.

Re: [Qemu-devel] [PATCH v3] block: change variable names in BlockDriverState

[Max Reitz]

On 2017-06-09 12:18, Manos Pitsidianakis wrote:
> Change the 'int count' parameter in *pwrite_zeros, *pdiscard related
> functions (and some others) to 'int bytes', as they both refer to bytes.
> This helps with code legibility.
> 
> Signed-off-by: Manos Pitsidianakis 
> ---
>  block/blkdebug.c   | 36 +++
>  block/blkreplay.c  |  8 +++
>  block/block-backend.c  | 22 +--
>  block/file-posix.c | 34 +++---
>  block/io.c | 48 
> +-
>  block/iscsi.c  | 20 +-
>  block/mirror.c |  8 +++
>  block/nbd-client.c |  8 +++
>  block/nbd-client.h |  4 ++--
>  block/qcow2.c  | 28 
>  block/qed.c|  8 +++
>  block/raw-format.c |  8 +++
>  block/rbd.c|  4 ++--
>  block/sheepdog.c   |  6 +++---
>  include/block/block.h  |  8 +++
>  include/block/block_int.h  |  6 +++---
>  include/sysemu/block-backend.h | 20 +-
>  qemu-io-cmds.c | 46 
>  18 files changed, 161 insertions(+), 161 deletions(-)

Thank you, I've applied the patch to my block branch:

https://github.com/XanClic/qemu/commits/block

Max



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH v3 1/2] tests: Add test-listen - a stress test for QEMU socket listen

[Knut Omang]

There's a potential race condition between multiple bind()'s
attempting to bind to the same port, which occasionally
allows more than one bind to succeed against the same port.

When a subsequent listen() call is made with the same socket
only one will succeed.

The current QEMU code does however not take this situation into account
and the listen will cause the code to break out and fail even
when there are actually available ports to use.

This test exposes two subtests:

/socket/listen-serial
/socket/listen-compete

The "compete" subtest creates a number of threads and have them all trying to 
bind
to the same port with a large enough offset input to
allow all threads to get it's own port.
The "serial" subtest just does the same, except in series in a
single thread.

The serial version passes, probably in most versions of QEMU.

The parallel version exposes the problem in a relatively reliable way,
eg. it fails a majority of times, but not with a 100% rate, occasional
passes can be seen. Nevertheless this is quite good given that
the bug was tricky to reproduce and has been left undetected for
a while.

The problem seems to be present in all versions of QEMU.

The original failure scenario occurred with VNC port allocation
in a traditional Xen based build, in different code
but with similar functionality.

Reported-by: Bhavesh Davda 
Signed-off-by: Knut Omang 
Reviewed-by: Yuval Shaia 
Reviewed-by: Bhavesh Davda 
Reviewed-by: Girish Moodalbail 
---
 tests/Makefile.include |   2 +-
 tests/test-listen.c| 141 ++-
 2 files changed, 143 insertions(+)
 create mode 100644 tests/test-listen.c

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 7180fe4..22bb97e 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -127,6 +127,7 @@ check-unit-y += tests/test-bufferiszero$(EXESUF)
 gcov-files-check-bufferiszero-y = util/bufferiszero.c
 check-unit-y += tests/test-uuid$(EXESUF)
 check-unit-y += tests/ptimer-test$(EXESUF)
+#check-unit-y += tests/test-listen$(EXESUF)
 gcov-files-ptimer-test-y = hw/core/ptimer.c
 check-unit-y += tests/test-qapi-util$(EXESUF)
 gcov-files-test-qapi-util-y = qapi/qapi-util.c
@@ -764,6 +765,7 @@ tests/test-uuid$(EXESUF): tests/test-uuid.o 
$(test-util-obj-y)
 tests/test-arm-mptimer$(EXESUF): tests/test-arm-mptimer.o
 tests/test-qapi-util$(EXESUF): tests/test-qapi-util.o $(test-util-obj-y)
 tests/numa-test$(EXESUF): tests/numa-test.o
+tests/test-listen$(EXESUF): tests/test-listen.o $(test-util-obj-y)
 
 tests/migration/stress$(EXESUF): tests/migration/stress.o
$(call quiet-command, $(LINKPROG) -static -O3 $(PTHREAD_LIB) -o $@ $< 
,"LINK","$(TARGET_DIR)$@")
diff --git a/tests/test-listen.c b/tests/test-listen.c
new file mode 100644
index 000..45fe9a8
--- /dev/null
+++ b/tests/test-listen.c
@@ -0,0 +1,141 @@
+/*
+ * Test parallel port listen configuration with
+ * dynamic port allocation
+ */
+
+#include "qemu/osdep.h"
+#include "libqtest.h"
+#include "qemu-common.h"
+#include "qemu/thread.h"
+#include "qemu/sockets.h"
+#include "qapi/error.h"
+
+#define NAME_LEN 1024
+#define PORT_LEN 16
+
+struct thr_info {
+QemuThread thread;
+int to_port;
+int got_port;
+int eno;
+int fd;
+const char *errstr;
+};
+
+static char hostname[NAME_LEN + 1];
+static char port[PORT_LEN + 1];
+
+static void *listener_thread(void *arg)
+{
+struct thr_info *thr = (struct thr_info *)arg;
+SocketAddress addr = {
+.type = SOCKET_ADDRESS_TYPE_INET,
+.u = {
+.inet = {
+.host = hostname,
+.port = port,
+.ipv4 = true,
+.has_to = true,
+.to = thr->to_port,
+},
+},
+};
+Error *err = NULL;
+int fd;
+
+fd = socket_listen(, );
+if (fd < 0) {
+thr->eno = errno;
+thr->errstr = error_get_pretty(err);
+} else {
+struct sockaddr_in a;
+socklen_t a_len = sizeof(a);
+g_assert_cmpint(getsockname(fd, (struct sockaddr *), _len), ==, 0);
+thr->got_port = ntohs(a.sin_port);
+thr->fd = fd;
+}
+return arg;
+}
+
+
+static void listen_compete_nthr(bool threaded, int nthreads,
+int start_port, int max_offset)
+{
+int i;
+int failed_listens = 0;
+size_t alloc_sz = sizeof(struct thr_info) * nthreads;
+struct thr_info *thr = g_malloc(alloc_sz);
+int used[max_offset + 1];
+memset(used, 0, sizeof(used));
+g_assert_nonnull(thr);
+g_assert_cmpint(gethostname(hostname, NAME_LEN), == , 0);
+snprintf(port, PORT_LEN, "%d", start_port);
+memset(thr, 0, alloc_sz);
+
+for (i = 0; i < nthreads; i++) {
+thr[i].to_port = start_port + max_offset;
+if (threaded) {
+qemu_thread_create([i].thread, 

[Qemu-devel] [PATCH v3 0/2] Unit test+fix for problem with QEMU handling of multiple bind()s to the same port

[Knut Omang]

This series contains:
* a unit test that exposes a race condition which causes QEMU to fail
  to find a port even when there is plenty of available ports.
* a refactor of the qemu-sockets inet_listen_saddr() function
  to better handle this situation.

Changes from v2:
* Non-trivial rebase + further abstraction
  on top of 7ad9af343c7f1c70c8015c7c519c312d8c5f9fa1
  'tests: add functional test validating ipv4/ipv6 address flag handling'

Changes from v1:
* Fix potential uninitialized variable only detected by optimize.
* Improve unexpected error detection in test-listen to give more
  details about why the test fails unexpectedly.
* Fix some line length style issues.

Thanks,
Knut

Knut Omang (2):
  tests: Add test-listen - a stress test for QEMU socket listen
  sockets: Handle race condition between binds to the same port

 tests/Makefile.include |   2 +-
 tests/test-listen.c| 141 +-
 util/qemu-sockets.c| 159 --
 3 files changed, 250 insertions(+), 52 deletions(-)
 create mode 100644 tests/test-listen.c

base-commit: 7ad9af343c7f1c70c8015c7c519c312d8c5f9fa1
-- 
git-series 0.9.1

[Qemu-devel] [PATCH v3 2/2] sockets: Handle race condition between binds to the same port

[Knut Omang]

If an offset of ports is specified to the inet_listen_saddr function(),
and two or more processes tries to bind from these ports at the same time,
occasionally more than one process may be able to bind to the same
port. The condition is detected by listen() but too late to avoid a failure.

This function is called by socket_listen() and used
by all socket listening code in QEMU, so all cases where any form of dynamic
port selection is used should be subject to this issue.

Add code to close and re-establish the socket when this
condition is observed, hiding the race condition from the user.

This has been developed and tested by means of the
test-listen unit test in the previous commit.
Enable the test for make check now that it passes.

Signed-off-by: Knut Omang 
Reviewed-by: Bhavesh Davda 
Reviewed-by: Yuval Shaia 
Reviewed-by: Girish Moodalbail 
---
 tests/Makefile.include |   2 +-
 util/qemu-sockets.c| 159 --
 2 files changed, 108 insertions(+), 53 deletions(-)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 22bb97e..c38f94e 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -127,7 +127,7 @@ check-unit-y += tests/test-bufferiszero$(EXESUF)
 gcov-files-check-bufferiszero-y = util/bufferiszero.c
 check-unit-y += tests/test-uuid$(EXESUF)
 check-unit-y += tests/ptimer-test$(EXESUF)
-#check-unit-y += tests/test-listen$(EXESUF)
+check-unit-y += tests/test-listen$(EXESUF)
 gcov-files-ptimer-test-y = hw/core/ptimer.c
 check-unit-y += tests/test-qapi-util$(EXESUF)
 gcov-files-test-qapi-util-y = qapi/qapi-util.c
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 852773d..7b118b4 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -149,6 +149,94 @@ int inet_ai_family_from_address(InetSocketAddress *addr,
 return PF_UNSPEC;
 }
 
+static int create_fast_reuse_socket(struct addrinfo *e, Error **errp)
+{
+int slisten = qemu_socket(e->ai_family, e->ai_socktype, e->ai_protocol);
+if (slisten < 0) {
+if (!e->ai_next) {
+error_setg_errno(errp, errno, "Failed to create socket");
+}
+return -1;
+}
+
+socket_set_fast_reuse(slisten);
+return slisten;
+}
+
+static int try_bind(int socket, InetSocketAddress *saddr, struct addrinfo *e)
+{
+#ifndef IPV6_V6ONLY
+return bind(socket, e->ai_addr, e->ai_addrlen);
+#else
+/*
+ * Deals with first & last cases in matrix in comment
+ * for inet_ai_family_from_address().
+ */
+int v6only =
+((!saddr->has_ipv4 && !saddr->has_ipv6) ||
+ (saddr->has_ipv4 && saddr->ipv4 &&
+  saddr->has_ipv6 && saddr->ipv6)) ? 0 : 1;
+int stat;
+
+ rebind:
+if (e->ai_family == PF_INET6) {
+qemu_setsockopt(socket, IPPROTO_IPV6, IPV6_V6ONLY, ,
+sizeof(v6only));
+}
+
+stat = bind(socket, e->ai_addr, e->ai_addrlen);
+if (!stat) {
+return 0;
+}
+
+/* If we got EADDRINUSE from an IPv6 bind & v6only is unset,
+ * it could be that the IPv4 port is already claimed, so retry
+ * with v6only set
+ */
+if (e->ai_family == PF_INET6 && errno == EADDRINUSE && !v6only) {
+v6only = 1;
+goto rebind;
+}
+return stat;
+#endif
+}
+
+static int try_bind_listen(int *socket, InetSocketAddress *saddr,
+   struct addrinfo *e, int port, Error **errp)
+{
+int s = *socket;
+int ret;
+
+inet_setport(e, port);
+ret = try_bind(s, saddr, e);
+if (ret) {
+if (errno != EADDRINUSE) {
+error_setg_errno(errp, errno, "Failed to bind socket");
+}
+return errno;
+}
+if (listen(s, 1) == 0) {
+return 0;
+}
+if (errno == EADDRINUSE) {
+/* We got to bind the socket to a port but someone else managed
+ * to bind to the same port and beat us to listen on it!
+ * Recreate the socket and return EADDRINUSE to preserve the
+ * expected state by the caller:
+ */
+closesocket(s);
+s = create_fast_reuse_socket(e, errp);
+if (s < 0) {
+return errno;
+}
+*socket = s;
+errno = EADDRINUSE;
+return errno;
+}
+error_setg_errno(errp, errno, "Failed to listen on socket");
+return errno;
+}
+
 static int inet_listen_saddr(InetSocketAddress *saddr,
  int port_offset,
  bool update_addr,
@@ -158,7 +246,9 @@ static int inet_listen_saddr(InetSocketAddress *saddr,
 char port[33];
 char uaddr[INET6_ADDRSTRLEN+1];
 char uport[33];
-int slisten, rc, port_min, port_max, p;
+int rc, port_min, port_max, p;
+int slisten = 0;
+int saved_errno = 0;
 Error *err = NULL;
 
 memset(,0, sizeof(ai));
@@ -210,75 +300,40 @@ static int 

Re: [Qemu-devel] [PATCH v1 3/3] tcg-runtime: short-circuit lookup_tb_ptr on IRQs

[Richard Henderson]

On 06/14/2017 09:08 AM, Paolo Bonzini wrote:

I think this is a band-aid, and would rather fix the front-ends as in
Emilio's patch.  For Alpha my guess would be:

diff --git a/target/alpha/translate.c b/target/alpha/translate.c
index 7c45ae360c..6e2ee3f958 100644
--- a/target/alpha/translate.c
+++ b/target/alpha/translate.c
@@ -1198,7 +1198,9 @@ static ExitStatus gen_call_pal(DisasContext *ctx, int 
palcode)
  tcg_gen_andi_i64(tmp, ctx->ir[IR_A0], PS_INT_MASK);
   tcg_gen_st8_i64(tmp, cpu_env, offsetof(CPUAlphaState, ps));
  tcg_temp_free(tmp);
-break;
+
+/* Reevaluate interrupts */
+return EXIT_PC_STALE;
  
  case 0x36:

  /* RDPS */


Thanks!

You're right that adjusting SWPIPL along these lines does fix the problem for 
Alpha.  Given that Alpha would typically hang in arch_idle, I'd been focusing 
primarily on WTINT.



r~