[PATCH v3 15/33] target/arm: Convert SQRSHL, UQRSHL to decodetree
Reviewed-by: Peter Maydell
Signed-off-by: Richard Henderson
---
target/arm/tcg/a64.decode | 4 +++
target/arm/tcg/translate-a64.c | 48 --
2 files changed, 26 insertions(+), 26 deletions(-)
diff --git a/target/arm/tcg/a64.decode b/target/arm/tcg/a64.decode
index 85caf37948..96ce35ad40 100644
--- a/target/arm/tcg/a64.decode
+++ b/target/arm/tcg/a64.decode
@@ -762,6 +762,8 @@ SRSHL_s 0101 1110 111 . 01010 1 . .
@rrr_d
URSHL_s 0111 1110 111 . 01010 1 . . @rrr_d
SQSHL_s 0101 1110 ..1 . 01001 1 . . @rrr_e
UQSHL_s 0111 1110 ..1 . 01001 1 . . @rrr_e
+SQRSHL_s0101 1110 ..1 . 01011 1 . . @rrr_e
+UQRSHL_s0111 1110 ..1 . 01011 1 . . @rrr_e
### Advanced SIMD scalar pairwise
@@ -890,6 +892,8 @@ SRSHL_v 0.00 1110 ..1 . 01010 1 . .
@qrrr_e
URSHL_v 0.10 1110 ..1 . 01010 1 . . @qrrr_e
SQSHL_v 0.00 1110 ..1 . 01001 1 . . @qrrr_e
UQSHL_v 0.10 1110 ..1 . 01001 1 . . @qrrr_e
+SQRSHL_v0.00 1110 ..1 . 01011 1 . . @qrrr_e
+UQRSHL_v0.10 1110 ..1 . 01011 1 . . @qrrr_e
### Advanced SIMD scalar x indexed element
diff --git a/target/arm/tcg/translate-a64.c b/target/arm/tcg/translate-a64.c
index b9d577f620..2424c6d314 100644
--- a/target/arm/tcg/translate-a64.c
+++ b/target/arm/tcg/translate-a64.c
@@ -5162,6 +5162,22 @@ static const ENVScalar2 f_scalar_uqshl = {
};
TRANS(UQSHL_s, do_env_scalar2, a, &f_scalar_uqshl)
+static const ENVScalar2 f_scalar_sqrshl = {
+{ gen_helper_neon_qrshl_s8,
+ gen_helper_neon_qrshl_s16,
+ gen_helper_neon_qrshl_s32 },
+gen_helper_neon_qrshl_s64,
+};
+TRANS(SQRSHL_s, do_env_scalar2, a, &f_scalar_sqrshl)
+
+static const ENVScalar2 f_scalar_uqrshl = {
+{ gen_helper_neon_qrshl_u8,
+ gen_helper_neon_qrshl_u16,
+ gen_helper_neon_qrshl_u32 },
+gen_helper_neon_qrshl_u64,
+};
+TRANS(UQRSHL_s, do_env_scalar2, a, &f_scalar_uqrshl)
+
static bool do_fp3_vector(DisasContext *s, arg_qrrr_e *a,
gen_helper_gvec_3_ptr * const fns[3])
{
@@ -5413,6 +5429,8 @@ TRANS(SRSHL_v, do_gvec_fn3, a, gen_gvec_srshl)
TRANS(URSHL_v, do_gvec_fn3, a, gen_gvec_urshl)
TRANS(SQSHL_v, do_gvec_fn3, a, gen_neon_sqshl)
TRANS(UQSHL_v, do_gvec_fn3, a, gen_neon_uqshl)
+TRANS(SQRSHL_v, do_gvec_fn3, a, gen_neon_sqrshl)
+TRANS(UQRSHL_v, do_gvec_fn3, a, gen_neon_uqrshl)
/*
@@ -9426,13 +9444,6 @@ static void handle_3same_64(DisasContext *s, int opcode,
bool u,
}
gen_cmtst_i64(tcg_rd, tcg_rn, tcg_rm);
break;
-case 0xb: /* SQRSHL, UQRSHL */
-if (u) {
-gen_helper_neon_qrshl_u64(tcg_rd, tcg_env, tcg_rn, tcg_rm);
-} else {
-gen_helper_neon_qrshl_s64(tcg_rd, tcg_env, tcg_rn, tcg_rm);
-}
-break;
case 0x10: /* ADD, SUB */
if (u) {
tcg_gen_sub_i64(tcg_rd, tcg_rn, tcg_rm);
@@ -9446,6 +9457,7 @@ static void handle_3same_64(DisasContext *s, int opcode,
bool u,
case 0x8: /* SSHL, USHL */
case 0x9: /* SQSHL, UQSHL */
case 0xa: /* SRSHL, URSHL */
+case 0xb: /* SQRSHL, UQRSHL */
g_assert_not_reached();
}
}
@@ -9467,8 +9479,6 @@ static void disas_simd_scalar_three_reg_same(DisasContext
*s, uint32_t insn)
TCGv_i64 tcg_rd;
switch (opcode) {
-case 0xb: /* SQRSHL, UQRSHL */
-break;
case 0x6: /* CMGT, CMHI */
case 0x7: /* CMGE, CMHS */
case 0x11: /* CMTST, CMEQ */
@@ -9490,6 +9500,7 @@ static void disas_simd_scalar_three_reg_same(DisasContext
*s, uint32_t insn)
case 0x8: /* SSHL, USHL */
case 0x9: /* SQSHL, UQSHL */
case 0xa: /* SRSHL, URSHL */
+case 0xb: /* SQRSHL, UQRSHL */
unallocated_encoding(s);
return;
}
@@ -9516,16 +9527,6 @@ static void
disas_simd_scalar_three_reg_same(DisasContext *s, uint32_t insn)
void (*genfn)(TCGv_i64, TCGv_i64, TCGv_i64, TCGv_i64, MemOp) = NULL;
switch (opcode) {
-case 0xb: /* SQRSHL, UQRSHL */
-{
-static NeonGenTwoOpEnvFn * const fns[3][2] = {
-{ gen_helper_neon_qrshl_s8, gen_helper_neon_qrshl_u8 },
-{ gen_helper_neon_qrshl_s16, gen_helper_neon_qrshl_u16 },
-{ gen_helper_neon_qrshl_s32, gen_helper_neon_qrshl_u32 },
-};
-genenvfn = fns[size][u];
-break;
-}
case 0x16: /* SQDMULH, SQRDMULH */
{
static NeonGenTwoOpEnvFn * const fns[2][2] = {
@@ -9540,6 +9541,7 @@ static void disas_simd_scalar_three_reg_same(DisasContext
*s, uint32_t insn)
case 0x1: /* SQADD, UQADD */
case 0x5: /* SQSUB, UQSUB */
case 0x9: /* SQSHL, UQSHL */
+case 0xb: /* SQRSHL, UQRSHL */
g_assert_not_reached();
}
@@ -10959,6 +10961,7 @@ static void disas_si
[PATCH v3 17/33] target/arm: Convert CMGT, CMHI, CMGE, CMHS, CMTST, CMEQ to decodetree
Reviewed-by: Peter Maydell
Signed-off-by: Richard Henderson
---
target/arm/tcg/a64.decode | 12 +++
target/arm/tcg/translate-a64.c | 132 -
2 files changed, 60 insertions(+), 84 deletions(-)
diff --git a/target/arm/tcg/a64.decode b/target/arm/tcg/a64.decode
index 44383b4fc7..3061e26242 100644
--- a/target/arm/tcg/a64.decode
+++ b/target/arm/tcg/a64.decode
@@ -767,6 +767,12 @@ UQRSHL_s0111 1110 ..1 . 01011 1 . .
@rrr_e
ADD_s 0101 1110 111 . 1 1 . . @rrr_d
SUB_s 0111 1110 111 . 1 1 . . @rrr_d
+CMGT_s 0101 1110 111 . 00110 1 . . @rrr_d
+CMHI_s 0111 1110 111 . 00110 1 . . @rrr_d
+CMGE_s 0101 1110 111 . 00111 1 . . @rrr_d
+CMHS_s 0111 1110 111 . 00111 1 . . @rrr_d
+CMTST_s 0101 1110 111 . 10001 1 . . @rrr_d
+CMEQ_s 0111 1110 111 . 10001 1 . . @rrr_d
### Advanced SIMD scalar pairwise
@@ -900,6 +906,12 @@ UQRSHL_v0.10 1110 ..1 . 01011 1 . .
@qrrr_e
ADD_v 0.00 1110 ..1 . 1 1 . . @qrrr_e
SUB_v 0.10 1110 ..1 . 1 1 . . @qrrr_e
+CMGT_v 0.00 1110 ..1 . 00110 1 . . @qrrr_e
+CMHI_v 0.10 1110 ..1 . 00110 1 . . @qrrr_e
+CMGE_v 0.00 1110 ..1 . 00111 1 . . @qrrr_e
+CMHS_v 0.10 1110 ..1 . 00111 1 . . @qrrr_e
+CMTST_v 0.00 1110 ..1 . 10001 1 . . @qrrr_e
+CMEQ_v 0.10 1110 ..1 . 10001 1 . . @qrrr_e
### Advanced SIMD scalar x indexed element
diff --git a/target/arm/tcg/translate-a64.c b/target/arm/tcg/translate-a64.c
index 77a64923e7..3c6cfc2952 100644
--- a/target/arm/tcg/translate-a64.c
+++ b/target/arm/tcg/translate-a64.c
@@ -5180,6 +5180,24 @@ static const ENVScalar2 f_scalar_uqrshl = {
};
TRANS(UQRSHL_s, do_env_scalar2, a, &f_scalar_uqrshl)
+static bool do_cmop_d(DisasContext *s, arg_rrr_e *a, TCGCond cond)
+{
+if (fp_access_check(s)) {
+TCGv_i64 t0 = read_fp_dreg(s, a->rn);
+TCGv_i64 t1 = read_fp_dreg(s, a->rm);
+tcg_gen_negsetcond_i64(cond, t0, t0, t1);
+write_fp_dreg(s, a->rd, t0);
+}
+return true;
+}
+
+TRANS(CMGT_s, do_cmop_d, a, TCG_COND_GT)
+TRANS(CMHI_s, do_cmop_d, a, TCG_COND_GTU)
+TRANS(CMGE_s, do_cmop_d, a, TCG_COND_GE)
+TRANS(CMHS_s, do_cmop_d, a, TCG_COND_GEU)
+TRANS(CMEQ_s, do_cmop_d, a, TCG_COND_EQ)
+TRANS(CMTST_s, do_cmop_d, a, TCG_COND_TSTNE)
+
static bool do_fp3_vector(DisasContext *s, arg_qrrr_e *a,
gen_helper_gvec_3_ptr * const fns[3])
{
@@ -5437,6 +5455,28 @@ TRANS(UQRSHL_v, do_gvec_fn3, a, gen_neon_uqrshl)
TRANS(ADD_v, do_gvec_fn3, a, tcg_gen_gvec_add)
TRANS(SUB_v, do_gvec_fn3, a, tcg_gen_gvec_sub)
+static bool do_cmop_v(DisasContext *s, arg_qrrr_e *a, TCGCond cond)
+{
+if (a->esz == MO_64 && !a->q) {
+return false;
+}
+if (fp_access_check(s)) {
+tcg_gen_gvec_cmp(cond, a->esz,
+ vec_full_reg_offset(s, a->rd),
+ vec_full_reg_offset(s, a->rn),
+ vec_full_reg_offset(s, a->rm),
+ a->q ? 16 : 8, vec_full_reg_size(s));
+}
+return true;
+}
+
+TRANS(CMGT_v, do_cmop_v, a, TCG_COND_GT)
+TRANS(CMHI_v, do_cmop_v, a, TCG_COND_GTU)
+TRANS(CMGE_v, do_cmop_v, a, TCG_COND_GE)
+TRANS(CMHS_v, do_cmop_v, a, TCG_COND_GEU)
+TRANS(CMEQ_v, do_cmop_v, a, TCG_COND_EQ)
+TRANS(CMTST_v, do_gvec_fn3, a, gen_gvec_cmtst)
+
/*
* Advanced SIMD scalar/vector x indexed element
*/
@@ -9421,45 +9461,6 @@ static void
disas_simd_scalar_three_reg_diff(DisasContext *s, uint32_t insn)
}
}
-static void handle_3same_64(DisasContext *s, int opcode, bool u,
-TCGv_i64 tcg_rd, TCGv_i64 tcg_rn, TCGv_i64 tcg_rm)
-{
-/* Handle 64x64->64 opcodes which are shared between the scalar
- * and vector 3-same groups. We cover every opcode where size == 3
- * is valid in either the three-reg-same (integer, not pairwise)
- * or scalar-three-reg-same groups.
- */
-TCGCond cond;
-
-switch (opcode) {
-case 0x6: /* CMGT, CMHI */
-cond = u ? TCG_COND_GTU : TCG_COND_GT;
-do_cmop:
-/* 64 bit integer comparison, result = test ? -1 : 0. */
-tcg_gen_negsetcond_i64(cond, tcg_rd, tcg_rn, tcg_rm);
-break;
-case 0x7: /* CMGE, CMHS */
-cond = u ? TCG_COND_GEU : TCG_COND_GE;
-goto do_cmop;
-case 0x11: /* CMTST, CMEQ */
-if (u) {
-cond = TCG_COND_EQ;
-goto do_cmop;
-}
-gen_cmtst_i64(tcg_rd, tcg_rn, tcg_rm);
-break;
-default:
-case 0x1: /* SQADD / UQADD */
-case 0x5: /* SQSUB / UQSUB */
-case 0x8: /* SSHL, USHL */
-case 0x9: /* SQSHL, UQSHL */
-case 0xa: /* SRSHL, URSHL */
-case 0xb: /* SQRSHL, UQRSHL */
-ca
[PATCH v3 08/33] target/arm: Convert SUQADD, USQADD to decodetree
These are faux 2-operand instructions, reading from rd.
Sort them next to the other three-operand same insns for clarity.
Reviewed-by: Peter Maydell
Signed-off-by: Richard Henderson
---
target/arm/tcg/a64.decode | 8 +
target/arm/tcg/translate-a64.c | 64 --
2 files changed, 14 insertions(+), 58 deletions(-)
diff --git a/target/arm/tcg/a64.decode b/target/arm/tcg/a64.decode
index 19010af03b..7c350ba833 100644
--- a/target/arm/tcg/a64.decode
+++ b/target/arm/tcg/a64.decode
@@ -45,6 +45,7 @@
@rrr_sd ... rm:5 .. rn:5 rd:5 &rrr_e esz=%esz_sd
@rrr_hsd ... rm:5 .. rn:5 rd:5 &rrr_e esz=%esz_hsd
@rrr_e esz:2 . rm:5 .. rn:5 rd:5 &rrr_e
+@r2r_e esz:2 . . .. rm:5 rd:5 &rrr_e rn=%rd
@rrx_h .. .. rm:4 . . rn:5 rd:5 &rrx_e esz=1 idx=%hlm
@rrx_s .. . rm:5 . . rn:5 rd:5 &rrx_e esz=2 idx=%hl
@@ -60,6 +61,7 @@
@qrrr_h . q:1 .. ... rm:5 .. rn:5 rd:5 &qrrr_e esz=1
@qrrr_sd. q:1 .. ... rm:5 .. rn:5 rd:5 &qrrr_e esz=%esz_sd
@qrrr_e . q:1 .. esz:2 . rm:5 .. rn:5 rd:5 &qrrr_e
+@qr2r_e . q:1 .. esz:2 . . .. rm:5 rd:5 &qrrr_e rn=%rd
@qrrx_h . q:1 .. .. .. rm:4 . . rn:5 rd:5 \
&qrrx_e esz=1 idx=%hlm
@@ -750,6 +752,9 @@ UQADD_s 0111 1110 ..1 . 1 1 . .
@rrr_e
SQSUB_s 0101 1110 ..1 . 00101 1 . . @rrr_e
UQSUB_s 0111 1110 ..1 . 00101 1 . . @rrr_e
+SUQADD_s0101 1110 ..1 0 00111 0 . . @r2r_e
+USQADD_s0111 1110 ..1 0 00111 0 . . @r2r_e
+
### Advanced SIMD scalar pairwise
FADDP_s 0101 1110 0011 1101 10 . . @rr_h
@@ -868,6 +873,9 @@ UQADD_v 0.10 1110 ..1 . 1 1 . .
@qrrr_e
SQSUB_v 0.00 1110 ..1 . 00101 1 . . @qrrr_e
UQSUB_v 0.10 1110 ..1 . 00101 1 . . @qrrr_e
+SUQADD_v0.00 1110 ..1 0 00111 0 . . @qr2r_e
+USQADD_v0.10 1110 ..1 0 00111 0 . . @qr2r_e
+
### Advanced SIMD scalar x indexed element
FMUL_si 0101 00 .. 1001 . 0 . . @rrx_h
diff --git a/target/arm/tcg/translate-a64.c b/target/arm/tcg/translate-a64.c
index 3956c41543..c0637bda0f 100644
--- a/target/arm/tcg/translate-a64.c
+++ b/target/arm/tcg/translate-a64.c
@@ -5096,6 +5096,8 @@ TRANS(SQADD_s, do_satacc_s, a, MO_SIGN, MO_SIGN,
gen_sqadd_bhs, gen_sqadd_d)
TRANS(SQSUB_s, do_satacc_s, a, MO_SIGN, MO_SIGN, gen_sqsub_bhs, gen_sqsub_d)
TRANS(UQADD_s, do_satacc_s, a, 0, 0, gen_uqadd_bhs, gen_uqadd_d)
TRANS(UQSUB_s, do_satacc_s, a, 0, 0, gen_uqsub_bhs, gen_uqsub_d)
+TRANS(SUQADD_s, do_satacc_s, a, MO_SIGN, 0, gen_suqadd_bhs, gen_suqadd_d)
+TRANS(USQADD_s, do_satacc_s, a, 0, MO_SIGN, gen_usqadd_bhs, gen_usqadd_d)
static bool do_fp3_vector(DisasContext *s, arg_qrrr_e *a,
gen_helper_gvec_3_ptr * const fns[3])
@@ -5339,6 +5341,8 @@ TRANS(SQADD_v, do_gvec_fn3, a, gen_gvec_sqadd_qc)
TRANS(UQADD_v, do_gvec_fn3, a, gen_gvec_uqadd_qc)
TRANS(SQSUB_v, do_gvec_fn3, a, gen_gvec_sqsub_qc)
TRANS(UQSUB_v, do_gvec_fn3, a, gen_gvec_uqsub_qc)
+TRANS(SUQADD_v, do_gvec_fn3, a, gen_gvec_suqadd_qc)
+TRANS(USQADD_v, do_gvec_fn3, a, gen_gvec_usqadd_qc)
/*
* Advanced SIMD scalar/vector x indexed element
@@ -10009,48 +10013,6 @@ static void handle_2misc_narrow(DisasContext *s, bool
scalar,
clear_vec_high(s, is_q, rd);
}
-/* Remaining saturating accumulating ops */
-static void handle_2misc_satacc(DisasContext *s, bool is_scalar, bool is_u,
-bool is_q, unsigned size, int rn, int rd)
-{
-TCGv_i64 res, qc, a, b;
-
-if (!is_scalar) {
-gen_gvec_fn3(s, is_q, rd, rd, rn,
- is_u ? gen_gvec_usqadd_qc : gen_gvec_suqadd_qc, size);
-return;
-}
-
-res = tcg_temp_new_i64();
-qc = tcg_temp_new_i64();
-a = tcg_temp_new_i64();
-b = tcg_temp_new_i64();
-
-/* Read and extend scalar inputs to 64-bits. */
-read_vec_element(s, a, rd, 0, size | (is_u ? 0 : MO_SIGN));
-read_vec_element(s, b, rn, 0, size | (is_u ? MO_SIGN : 0));
-tcg_gen_ld_i64(qc, tcg_env, offsetof(CPUARMState, vfp.qc));
-
-if (size == MO_64) {
-if (is_u) {
-gen_usqadd_d(res, qc, a, b);
-} else {
-gen_suqadd_d(res, qc, a, b);
-}
-} else {
-if (is_u) {
-gen_usqadd_bhs(res, qc, a, b, size);
-} else {
-gen_suqadd_bhs(res, qc, a, b, size);
-/* Truncate signed 64-bit result for writeback. */
-tcg_gen_ext_i64(res, res, size);
-}
-}
-
-write_fp_dreg(s, rd, res);
-tcg_gen_st_i64(qc, tcg_env, offsetof(CPUARMState, vfp.qc));
-}
-
/* AdvSIMD scalar two reg misc
* 31 30 29 28 24 23 22 21 17 16
[PATCH v3 07/33] target/arm: Convert SQADD, SQSUB, UQADD, UQSUB to decodetree
Reviewed-by: Peter Maydell
Signed-off-by: Richard Henderson
---
target/arm/tcg/a64.decode | 11
target/arm/tcg/translate-a64.c | 96 +++---
2 files changed, 64 insertions(+), 43 deletions(-)
diff --git a/target/arm/tcg/a64.decode b/target/arm/tcg/a64.decode
index f48adef5bb..19010af03b 100644
--- a/target/arm/tcg/a64.decode
+++ b/target/arm/tcg/a64.decode
@@ -44,6 +44,7 @@
@rrr_h ... rm:5 .. rn:5 rd:5 &rrr_e esz=1
@rrr_sd ... rm:5 .. rn:5 rd:5 &rrr_e esz=%esz_sd
@rrr_hsd ... rm:5 .. rn:5 rd:5 &rrr_e esz=%esz_hsd
+@rrr_e esz:2 . rm:5 .. rn:5 rd:5 &rrr_e
@rrx_h .. .. rm:4 . . rn:5 rd:5 &rrx_e esz=1 idx=%hlm
@rrx_s .. . rm:5 . . rn:5 rd:5 &rrx_e esz=2 idx=%hl
@@ -744,6 +745,11 @@ FRECPS_s0101 1110 0.1 . 1 1 . .
@rrr_sd
FRSQRTS_s 0101 1110 110 . 00111 1 . . @rrr_h
FRSQRTS_s 0101 1110 1.1 . 1 1 . . @rrr_sd
+SQADD_s 0101 1110 ..1 . 1 1 . . @rrr_e
+UQADD_s 0111 1110 ..1 . 1 1 . . @rrr_e
+SQSUB_s 0101 1110 ..1 . 00101 1 . . @rrr_e
+UQSUB_s 0111 1110 ..1 . 00101 1 . . @rrr_e
+
### Advanced SIMD scalar pairwise
FADDP_s 0101 1110 0011 1101 10 . . @rr_h
@@ -857,6 +863,11 @@ BSL_v 0.10 1110 011 . 00011 1 . .
@qrrr_b
BIT_v 0.10 1110 101 . 00011 1 . . @qrrr_b
BIF_v 0.10 1110 111 . 00011 1 . . @qrrr_b
+SQADD_v 0.00 1110 ..1 . 1 1 . . @qrrr_e
+UQADD_v 0.10 1110 ..1 . 1 1 . . @qrrr_e
+SQSUB_v 0.00 1110 ..1 . 00101 1 . . @qrrr_e
+UQSUB_v 0.10 1110 ..1 . 00101 1 . . @qrrr_e
+
### Advanced SIMD scalar x indexed element
FMUL_si 0101 00 .. 1001 . 0 . . @rrx_h
diff --git a/target/arm/tcg/translate-a64.c b/target/arm/tcg/translate-a64.c
index ca7ba6b1e8..3956c41543 100644
--- a/target/arm/tcg/translate-a64.c
+++ b/target/arm/tcg/translate-a64.c
@@ -5060,6 +5060,43 @@ static const FPScalar f_scalar_frsqrts = {
};
TRANS(FRSQRTS_s, do_fp3_scalar, a, &f_scalar_frsqrts)
+static bool do_satacc_s(DisasContext *s, arg_rrr_e *a,
+MemOp sgn_n, MemOp sgn_m,
+void (*gen_bhs)(TCGv_i64, TCGv_i64, TCGv_i64, TCGv_i64, MemOp),
+void (*gen_d)(TCGv_i64, TCGv_i64, TCGv_i64, TCGv_i64))
+{
+TCGv_i64 t0, t1, t2, qc;
+MemOp esz = a->esz;
+
+if (!fp_access_check(s)) {
+return true;
+}
+
+t0 = tcg_temp_new_i64();
+t1 = tcg_temp_new_i64();
+t2 = tcg_temp_new_i64();
+qc = tcg_temp_new_i64();
+read_vec_element(s, t1, a->rn, 0, esz | sgn_n);
+read_vec_element(s, t2, a->rm, 0, esz | sgn_m);
+tcg_gen_ld_i64(qc, tcg_env, offsetof(CPUARMState, vfp.qc));
+
+if (esz == MO_64) {
+gen_d(t0, qc, t1, t2);
+} else {
+gen_bhs(t0, qc, t1, t2, esz);
+tcg_gen_ext_i64(t0, t0, esz);
+}
+
+write_fp_dreg(s, a->rd, t0);
+tcg_gen_st_i64(qc, tcg_env, offsetof(CPUARMState, vfp.qc));
+return true;
+}
+
+TRANS(SQADD_s, do_satacc_s, a, MO_SIGN, MO_SIGN, gen_sqadd_bhs, gen_sqadd_d)
+TRANS(SQSUB_s, do_satacc_s, a, MO_SIGN, MO_SIGN, gen_sqsub_bhs, gen_sqsub_d)
+TRANS(UQADD_s, do_satacc_s, a, 0, 0, gen_uqadd_bhs, gen_uqadd_d)
+TRANS(UQSUB_s, do_satacc_s, a, 0, 0, gen_uqsub_bhs, gen_uqsub_d)
+
static bool do_fp3_vector(DisasContext *s, arg_qrrr_e *a,
gen_helper_gvec_3_ptr * const fns[3])
{
@@ -5298,6 +5335,11 @@ TRANS(BSL_v, do_bitsel, a->q, a->rd, a->rd, a->rn, a->rm)
TRANS(BIT_v, do_bitsel, a->q, a->rd, a->rm, a->rn, a->rd)
TRANS(BIF_v, do_bitsel, a->q, a->rd, a->rm, a->rd, a->rn)
+TRANS(SQADD_v, do_gvec_fn3, a, gen_gvec_sqadd_qc)
+TRANS(UQADD_v, do_gvec_fn3, a, gen_gvec_uqadd_qc)
+TRANS(SQSUB_v, do_gvec_fn3, a, gen_gvec_sqsub_qc)
+TRANS(UQSUB_v, do_gvec_fn3, a, gen_gvec_uqsub_qc)
+
/*
* Advanced SIMD scalar/vector x indexed element
*/
@@ -9291,29 +9333,8 @@ static void handle_3same_64(DisasContext *s, int opcode,
bool u,
* or scalar-three-reg-same groups.
*/
TCGCond cond;
-TCGv_i64 qc;
switch (opcode) {
-case 0x1: /* SQADD */
-qc = tcg_temp_new_i64();
-tcg_gen_ld_i64(qc, tcg_env, offsetof(CPUARMState, vfp.qc));
-if (u) {
-gen_uqadd_d(tcg_rd, qc, tcg_rn, tcg_rm);
-} else {
-gen_sqadd_d(tcg_rd, qc, tcg_rn, tcg_rm);
-}
-tcg_gen_st_i64(qc, tcg_env, offsetof(CPUARMState, vfp.qc));
-break;
-case 0x5: /* SQSUB */
-qc = tcg_temp_new_i64();
-tcg_gen_ld_i64(qc, tcg_env, offsetof(CPUARMState, vfp.qc));
-if (u) {
-gen_uqsub_d(tcg_rd, qc, tcg_rn, tcg_rm);
-} else {
-gen_sqsub_d(tcg_r
[PATCH v3 25/33] target/arm: Convert SRHADD, URHADD to decodetree
Reviewed-by: Peter Maydell
Signed-off-by: Richard Henderson
---
target/arm/tcg/a64.decode | 2 ++
target/arm/tcg/translate-a64.c | 11 +++
2 files changed, 5 insertions(+), 8 deletions(-)
diff --git a/target/arm/tcg/a64.decode b/target/arm/tcg/a64.decode
index b1bbcb144e..1c448b4f7c 100644
--- a/target/arm/tcg/a64.decode
+++ b/target/arm/tcg/a64.decode
@@ -916,6 +916,8 @@ SHADD_v 0.00 1110 ..1 . 0 1 . .
@qrrr_e
UHADD_v 0.10 1110 ..1 . 0 1 . . @qrrr_e
SHSUB_v 0.00 1110 ..1 . 00100 1 . . @qrrr_e
UHSUB_v 0.10 1110 ..1 . 00100 1 . . @qrrr_e
+SRHADD_v0.00 1110 ..1 . 00010 1 . . @qrrr_e
+URHADD_v0.10 1110 ..1 . 00010 1 . . @qrrr_e
### Advanced SIMD scalar x indexed element
diff --git a/target/arm/tcg/translate-a64.c b/target/arm/tcg/translate-a64.c
index 40aa7a9d57..9ef5de6755 100644
--- a/target/arm/tcg/translate-a64.c
+++ b/target/arm/tcg/translate-a64.c
@@ -5458,6 +5458,8 @@ TRANS(SHADD_v, do_gvec_fn3_no64, a, gen_gvec_shadd)
TRANS(UHADD_v, do_gvec_fn3_no64, a, gen_gvec_uhadd)
TRANS(SHSUB_v, do_gvec_fn3_no64, a, gen_gvec_shsub)
TRANS(UHSUB_v, do_gvec_fn3_no64, a, gen_gvec_uhsub)
+TRANS(SRHADD_v, do_gvec_fn3_no64, a, gen_gvec_srhadd)
+TRANS(URHADD_v, do_gvec_fn3_no64, a, gen_gvec_urhadd)
static bool do_cmop_v(DisasContext *s, arg_qrrr_e *a, TCGCond cond)
{
@@ -10923,7 +10925,6 @@ static void disas_simd_3same_int(DisasContext *s,
uint32_t insn)
return;
}
/* fall through */
-case 0x2: /* SRHADD, URHADD */
case 0xc: /* SMAX, UMAX */
case 0xd: /* SMIN, UMIN */
case 0xe: /* SABD, UABD */
@@ -10949,6 +10950,7 @@ static void disas_simd_3same_int(DisasContext *s,
uint32_t insn)
case 0x0: /* SHADD, UHADD */
case 0x01: /* SQADD, UQADD */
+case 0x02: /* SRHADD, URHADD */
case 0x04: /* SHSUB, UHSUB */
case 0x05: /* SQSUB, UQSUB */
case 0x06: /* CMGT, CMHI */
@@ -10968,13 +10970,6 @@ static void disas_simd_3same_int(DisasContext *s,
uint32_t insn)
}
switch (opcode) {
-case 0x02: /* SRHADD, URHADD */
-if (u) {
-gen_gvec_fn3(s, is_q, rd, rn, rm, gen_gvec_urhadd, size);
-} else {
-gen_gvec_fn3(s, is_q, rd, rn, rm, gen_gvec_srhadd, size);
-}
-return;
case 0x0c: /* SMAX, UMAX */
if (u) {
gen_gvec_fn3(s, is_q, rd, rn, rm, tcg_gen_gvec_umax, size);
--
2.34.1
[PATCH v3 20/33] target/arm: Convert SHADD, UHADD to gvec
Reviewed-by: Peter Maydell
Signed-off-by: Richard Henderson
---
target/arm/helper.h | 6 --
target/arm/tcg/translate.h | 5 ++
target/arm/tcg/gengvec.c| 144
target/arm/tcg/neon_helper.c| 27 --
target/arm/tcg/translate-a64.c | 17 ++--
target/arm/tcg/translate-neon.c | 4 +-
6 files changed, 158 insertions(+), 45 deletions(-)
diff --git a/target/arm/helper.h b/target/arm/helper.h
index 9a89c9cea7..b26bfcb079 100644
--- a/target/arm/helper.h
+++ b/target/arm/helper.h
@@ -268,12 +268,6 @@ DEF_HELPER_FLAGS_2(fjcvtzs, TCG_CALL_NO_RWG, i64, f64, ptr)
DEF_HELPER_FLAGS_3(check_hcr_el2_trap, TCG_CALL_NO_WG, void, env, i32, i32)
/* neon_helper.c */
-DEF_HELPER_2(neon_hadd_s8, i32, i32, i32)
-DEF_HELPER_2(neon_hadd_u8, i32, i32, i32)
-DEF_HELPER_2(neon_hadd_s16, i32, i32, i32)
-DEF_HELPER_2(neon_hadd_u16, i32, i32, i32)
-DEF_HELPER_2(neon_hadd_s32, s32, s32, s32)
-DEF_HELPER_2(neon_hadd_u32, i32, i32, i32)
DEF_HELPER_2(neon_rhadd_s8, i32, i32, i32)
DEF_HELPER_2(neon_rhadd_u8, i32, i32, i32)
DEF_HELPER_2(neon_rhadd_s16, i32, i32, i32)
diff --git a/target/arm/tcg/translate.h b/target/arm/tcg/translate.h
index 048cb45ebe..dd99d76bf2 100644
--- a/target/arm/tcg/translate.h
+++ b/target/arm/tcg/translate.h
@@ -472,6 +472,11 @@ void gen_neon_sqrshl(unsigned vece, uint32_t rd_ofs,
uint32_t rn_ofs,
void gen_neon_uqrshl(unsigned vece, uint32_t rd_ofs, uint32_t rn_ofs,
uint32_t rm_ofs, uint32_t opr_sz, uint32_t max_sz);
+void gen_gvec_shadd(unsigned vece, uint32_t rd_ofs, uint32_t rn_ofs,
+uint32_t rm_ofs, uint32_t opr_sz, uint32_t max_sz);
+void gen_gvec_uhadd(unsigned vece, uint32_t rd_ofs, uint32_t rn_ofs,
+uint32_t rm_ofs, uint32_t opr_sz, uint32_t max_sz);
+
void gen_cmtst_i64(TCGv_i64 d, TCGv_i64 a, TCGv_i64 b);
void gen_ushl_i32(TCGv_i32 d, TCGv_i32 a, TCGv_i32 b);
void gen_sshl_i32(TCGv_i32 d, TCGv_i32 a, TCGv_i32 b);
diff --git a/target/arm/tcg/gengvec.c b/target/arm/tcg/gengvec.c
index 2451d23823..c0627a787b 100644
--- a/target/arm/tcg/gengvec.c
+++ b/target/arm/tcg/gengvec.c
@@ -1861,3 +1861,147 @@ void gen_gvec_uminp(unsigned vece, uint32_t rd_ofs,
uint32_t rn_ofs,
tcg_debug_assert(vece <= MO_32);
tcg_gen_gvec_3_ool(rd_ofs, rn_ofs, rm_ofs, opr_sz, max_sz, 0, fns[vece]);
}
+
+static void gen_shadd8_i64(TCGv_i64 d, TCGv_i64 a, TCGv_i64 b)
+{
+TCGv_i64 t = tcg_temp_new_i64();
+
+tcg_gen_and_i64(t, a, b);
+tcg_gen_vec_sar8i_i64(a, a, 1);
+tcg_gen_vec_sar8i_i64(b, b, 1);
+tcg_gen_andi_i64(t, t, dup_const(MO_8, 1));
+tcg_gen_vec_add8_i64(d, a, b);
+tcg_gen_vec_add8_i64(d, d, t);
+}
+
+static void gen_shadd16_i64(TCGv_i64 d, TCGv_i64 a, TCGv_i64 b)
+{
+TCGv_i64 t = tcg_temp_new_i64();
+
+tcg_gen_and_i64(t, a, b);
+tcg_gen_vec_sar16i_i64(a, a, 1);
+tcg_gen_vec_sar16i_i64(b, b, 1);
+tcg_gen_andi_i64(t, t, dup_const(MO_16, 1));
+tcg_gen_vec_add16_i64(d, a, b);
+tcg_gen_vec_add16_i64(d, d, t);
+}
+
+static void gen_shadd_i32(TCGv_i32 d, TCGv_i32 a, TCGv_i32 b)
+{
+TCGv_i32 t = tcg_temp_new_i32();
+
+tcg_gen_and_i32(t, a, b);
+tcg_gen_sari_i32(a, a, 1);
+tcg_gen_sari_i32(b, b, 1);
+tcg_gen_andi_i32(t, t, 1);
+tcg_gen_add_i32(d, a, b);
+tcg_gen_add_i32(d, d, t);
+}
+
+static void gen_shadd_vec(unsigned vece, TCGv_vec d, TCGv_vec a, TCGv_vec b)
+{
+TCGv_vec t = tcg_temp_new_vec_matching(d);
+
+tcg_gen_and_vec(vece, t, a, b);
+tcg_gen_sari_vec(vece, a, a, 1);
+tcg_gen_sari_vec(vece, b, b, 1);
+tcg_gen_and_vec(vece, t, t, tcg_constant_vec_matching(d, vece, 1));
+tcg_gen_add_vec(vece, d, a, b);
+tcg_gen_add_vec(vece, d, d, t);
+}
+
+void gen_gvec_shadd(unsigned vece, uint32_t rd_ofs, uint32_t rn_ofs,
+uint32_t rm_ofs, uint32_t opr_sz, uint32_t max_sz)
+{
+static const TCGOpcode vecop_list[] = {
+INDEX_op_sari_vec, INDEX_op_add_vec, 0
+};
+static const GVecGen3 g[] = {
+{ .fni8 = gen_shadd8_i64,
+ .fniv = gen_shadd_vec,
+ .opt_opc = vecop_list,
+ .vece = MO_8 },
+{ .fni8 = gen_shadd16_i64,
+ .fniv = gen_shadd_vec,
+ .opt_opc = vecop_list,
+ .vece = MO_16 },
+{ .fni4 = gen_shadd_i32,
+ .fniv = gen_shadd_vec,
+ .opt_opc = vecop_list,
+ .vece = MO_32 },
+};
+tcg_debug_assert(vece <= MO_32);
+tcg_gen_gvec_3(rd_ofs, rn_ofs, rm_ofs, opr_sz, max_sz, &g[vece]);
+}
+
+static void gen_uhadd8_i64(TCGv_i64 d, TCGv_i64 a, TCGv_i64 b)
+{
+TCGv_i64 t = tcg_temp_new_i64();
+
+tcg_gen_and_i64(t, a, b);
+tcg_gen_vec_shr8i_i64(a, a, 1);
+tcg_gen_vec_shr8i_i64(b, b, 1);
+tcg_gen_andi_i64(t, t, dup_const(MO_8, 1));
+tcg_gen_vec_add8_i64(d, a, b);
+tcg_gen_vec_add8_i64(d, d, t);
+}
+
+static void gen_uhadd16_i64(TCGv_i64 d, TCGv_i64 a, TCGv_i64 b)
+{
+TCGv_i64 t = tcg_temp_new_i64();
+
+tc
[PATCH v3 14/33] target/arm: Convert SQRSHL and UQRSHL (register) to gvec
Reviewed-by: Peter Maydell
Signed-off-by: Richard Henderson
---
target/arm/helper.h | 8 ++
target/arm/tcg/translate.h | 4 +++
target/arm/tcg/neon-dp.decode | 17 ++--
target/arm/tcg/gengvec.c| 24
target/arm/tcg/neon_helper.c| 24
target/arm/tcg/translate-a64.c | 17 +---
target/arm/tcg/translate-neon.c | 49 ++---
7 files changed, 71 insertions(+), 72 deletions(-)
diff --git a/target/arm/helper.h b/target/arm/helper.h
index f345087ddb..9a89c9cea7 100644
--- a/target/arm/helper.h
+++ b/target/arm/helper.h
@@ -334,6 +334,14 @@ DEF_HELPER_FLAGS_5(neon_uqshl_b, TCG_CALL_NO_RWG, void,
ptr, ptr, ptr, ptr, i32)
DEF_HELPER_FLAGS_5(neon_uqshl_h, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, ptr,
i32)
DEF_HELPER_FLAGS_5(neon_uqshl_s, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, ptr,
i32)
DEF_HELPER_FLAGS_5(neon_uqshl_d, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, ptr,
i32)
+DEF_HELPER_FLAGS_5(neon_sqrshl_b, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, ptr,
i32)
+DEF_HELPER_FLAGS_5(neon_sqrshl_h, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, ptr,
i32)
+DEF_HELPER_FLAGS_5(neon_sqrshl_s, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, ptr,
i32)
+DEF_HELPER_FLAGS_5(neon_sqrshl_d, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, ptr,
i32)
+DEF_HELPER_FLAGS_5(neon_uqrshl_b, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, ptr,
i32)
+DEF_HELPER_FLAGS_5(neon_uqrshl_h, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, ptr,
i32)
+DEF_HELPER_FLAGS_5(neon_uqrshl_s, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, ptr,
i32)
+DEF_HELPER_FLAGS_5(neon_uqrshl_d, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, ptr,
i32)
DEF_HELPER_FLAGS_4(gvec_srshl_b, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
DEF_HELPER_FLAGS_4(gvec_srshl_h, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, i32)
diff --git a/target/arm/tcg/translate.h b/target/arm/tcg/translate.h
index 6c6d4d49e7..048cb45ebe 100644
--- a/target/arm/tcg/translate.h
+++ b/target/arm/tcg/translate.h
@@ -467,6 +467,10 @@ void gen_neon_sqshl(unsigned vece, uint32_t rd_ofs,
uint32_t rn_ofs,
uint32_t rm_ofs, uint32_t opr_sz, uint32_t max_sz);
void gen_neon_uqshl(unsigned vece, uint32_t rd_ofs, uint32_t rn_ofs,
uint32_t rm_ofs, uint32_t opr_sz, uint32_t max_sz);
+void gen_neon_sqrshl(unsigned vece, uint32_t rd_ofs, uint32_t rn_ofs,
+ uint32_t rm_ofs, uint32_t opr_sz, uint32_t max_sz);
+void gen_neon_uqrshl(unsigned vece, uint32_t rd_ofs, uint32_t rn_ofs,
+ uint32_t rm_ofs, uint32_t opr_sz, uint32_t max_sz);
void gen_cmtst_i64(TCGv_i64 d, TCGv_i64 a, TCGv_i64 b);
void gen_ushl_i32(TCGv_i32 d, TCGv_i32 a, TCGv_i32 b);
diff --git a/target/arm/tcg/neon-dp.decode b/target/arm/tcg/neon-dp.decode
index 6d4996b8d8..788578c8fa 100644
--- a/target/arm/tcg/neon-dp.decode
+++ b/target/arm/tcg/neon-dp.decode
@@ -102,25 +102,12 @@ VCGE_U_3s 001 1 0 . .. 0011 . . . 1
@3same
VSHL_S_3s 001 0 0 . .. 0100 . . . 0 @3same_rev
VSHL_U_3s 001 1 0 . .. 0100 . . . 0 @3same_rev
-
-# Insns operating on 64-bit elements (size!=0b11 handled elsewhere)
-# The _rev suffix indicates that Vn and Vm are reversed (as explained
-# by the comment for the @3same_rev format).
-@3same_64_rev ... . . . 11 . q:1 . . \
- &3same vm=%vn_dp vn=%vm_dp vd=%vd_dp size=3
-
VQSHL_S_3s 001 0 0 . .. 0100 . . . 1 @3same_rev
VQSHL_U_3s 001 1 0 . .. 0100 . . . 1 @3same_rev
VRSHL_S_3s 001 0 0 . .. 0101 . . . 0 @3same_rev
VRSHL_U_3s 001 1 0 . .. 0101 . . . 0 @3same_rev
-{
- VQRSHL_S64_3s 001 0 0 . .. 0101 . . . 1 @3same_64_rev
- VQRSHL_S_3s 001 0 0 . .. 0101 . . . 1 @3same_rev
-}
-{
- VQRSHL_U64_3s 001 1 0 . .. 0101 . . . 1 @3same_64_rev
- VQRSHL_U_3s 001 1 0 . .. 0101 . . . 1 @3same_rev
-}
+VQRSHL_S_3s 001 0 0 . .. 0101 . . . 1 @3same_rev
+VQRSHL_U_3s 001 1 0 . .. 0101 . . . 1 @3same_rev
VMAX_S_3s 001 0 0 . .. 0110 . . . 0 @3same
VMAX_U_3s 001 1 0 . .. 0110 . . . 0 @3same
diff --git a/target/arm/tcg/gengvec.c b/target/arm/tcg/gengvec.c
index 63c3ec2e73..6dc96269d5 100644
--- a/target/arm/tcg/gengvec.c
+++ b/target/arm/tcg/gengvec.c
@@ -1264,6 +1264,30 @@ void gen_neon_uqshl(unsigned vece, uint32_t rd_ofs,
uint32_t rn_ofs,
opr_sz, max_sz, 0, fns[vece]);
}
+void gen_neon_sqrshl(unsigned vece, uint32_t rd_ofs, uint32_t rn_ofs,
+ uint32_t rm_ofs, uint32_t opr_sz, uint32_t max_sz)
+{
+static gen_helper_gvec_3_ptr * const fns[] = {
+gen_helper_neon_sqrshl_b, gen_helper_neon_sqrshl_h,
+gen_helper_neon_sqrshl_s, gen_helper_neon_sqrshl_d,
+}
[PATCH v3 24/33] target/arm: Convert SRHADD, URHADD to gvec
Reviewed-by: Peter Maydell
Signed-off-by: Richard Henderson
---
target/arm/helper.h | 7 --
target/arm/tcg/translate.h | 4 +
target/arm/tcg/gengvec.c| 144
target/arm/tcg/neon_helper.c| 27 --
target/arm/tcg/translate-a64.c | 48 ++-
target/arm/tcg/translate-neon.c | 26 +-
6 files changed, 158 insertions(+), 98 deletions(-)
diff --git a/target/arm/helper.h b/target/arm/helper.h
index b95f24ed0a..85f9302563 100644
--- a/target/arm/helper.h
+++ b/target/arm/helper.h
@@ -268,13 +268,6 @@ DEF_HELPER_FLAGS_2(fjcvtzs, TCG_CALL_NO_RWG, i64, f64, ptr)
DEF_HELPER_FLAGS_3(check_hcr_el2_trap, TCG_CALL_NO_WG, void, env, i32, i32)
/* neon_helper.c */
-DEF_HELPER_2(neon_rhadd_s8, i32, i32, i32)
-DEF_HELPER_2(neon_rhadd_u8, i32, i32, i32)
-DEF_HELPER_2(neon_rhadd_s16, i32, i32, i32)
-DEF_HELPER_2(neon_rhadd_u16, i32, i32, i32)
-DEF_HELPER_2(neon_rhadd_s32, s32, s32, s32)
-DEF_HELPER_2(neon_rhadd_u32, i32, i32, i32)
-
DEF_HELPER_2(neon_pmin_u8, i32, i32, i32)
DEF_HELPER_2(neon_pmin_s8, i32, i32, i32)
DEF_HELPER_2(neon_pmin_u16, i32, i32, i32)
diff --git a/target/arm/tcg/translate.h b/target/arm/tcg/translate.h
index 315e0afd04..3b1e68b779 100644
--- a/target/arm/tcg/translate.h
+++ b/target/arm/tcg/translate.h
@@ -480,6 +480,10 @@ void gen_gvec_shsub(unsigned vece, uint32_t rd_ofs,
uint32_t rn_ofs,
uint32_t rm_ofs, uint32_t opr_sz, uint32_t max_sz);
void gen_gvec_uhsub(unsigned vece, uint32_t rd_ofs, uint32_t rn_ofs,
uint32_t rm_ofs, uint32_t opr_sz, uint32_t max_sz);
+void gen_gvec_srhadd(unsigned vece, uint32_t rd_ofs, uint32_t rn_ofs,
+ uint32_t rm_ofs, uint32_t opr_sz, uint32_t max_sz);
+void gen_gvec_urhadd(unsigned vece, uint32_t rd_ofs, uint32_t rn_ofs,
+ uint32_t rm_ofs, uint32_t opr_sz, uint32_t max_sz);
void gen_cmtst_i64(TCGv_i64 d, TCGv_i64 a, TCGv_i64 b);
void gen_ushl_i32(TCGv_i32 d, TCGv_i32 a, TCGv_i32 b);
diff --git a/target/arm/tcg/gengvec.c b/target/arm/tcg/gengvec.c
index c46365c3a6..119826bf28 100644
--- a/target/arm/tcg/gengvec.c
+++ b/target/arm/tcg/gengvec.c
@@ -2149,3 +2149,147 @@ void gen_gvec_uhsub(unsigned vece, uint32_t rd_ofs,
uint32_t rn_ofs,
assert(vece <= MO_32);
tcg_gen_gvec_3(rd_ofs, rn_ofs, rm_ofs, opr_sz, max_sz, &g[vece]);
}
+
+static void gen_srhadd8_i64(TCGv_i64 d, TCGv_i64 a, TCGv_i64 b)
+{
+TCGv_i64 t = tcg_temp_new_i64();
+
+tcg_gen_or_i64(t, a, b);
+tcg_gen_vec_sar8i_i64(a, a, 1);
+tcg_gen_vec_sar8i_i64(b, b, 1);
+tcg_gen_andi_i64(t, t, dup_const(MO_8, 1));
+tcg_gen_vec_add8_i64(d, a, b);
+tcg_gen_vec_add8_i64(d, d, t);
+}
+
+static void gen_srhadd16_i64(TCGv_i64 d, TCGv_i64 a, TCGv_i64 b)
+{
+TCGv_i64 t = tcg_temp_new_i64();
+
+tcg_gen_or_i64(t, a, b);
+tcg_gen_vec_sar16i_i64(a, a, 1);
+tcg_gen_vec_sar16i_i64(b, b, 1);
+tcg_gen_andi_i64(t, t, dup_const(MO_16, 1));
+tcg_gen_vec_add16_i64(d, a, b);
+tcg_gen_vec_add16_i64(d, d, t);
+}
+
+static void gen_srhadd_i32(TCGv_i32 d, TCGv_i32 a, TCGv_i32 b)
+{
+TCGv_i32 t = tcg_temp_new_i32();
+
+tcg_gen_or_i32(t, a, b);
+tcg_gen_sari_i32(a, a, 1);
+tcg_gen_sari_i32(b, b, 1);
+tcg_gen_andi_i32(t, t, 1);
+tcg_gen_add_i32(d, a, b);
+tcg_gen_add_i32(d, d, t);
+}
+
+static void gen_srhadd_vec(unsigned vece, TCGv_vec d, TCGv_vec a, TCGv_vec b)
+{
+TCGv_vec t = tcg_temp_new_vec_matching(d);
+
+tcg_gen_or_vec(vece, t, a, b);
+tcg_gen_sari_vec(vece, a, a, 1);
+tcg_gen_sari_vec(vece, b, b, 1);
+tcg_gen_and_vec(vece, t, t, tcg_constant_vec_matching(d, vece, 1));
+tcg_gen_add_vec(vece, d, a, b);
+tcg_gen_add_vec(vece, d, d, t);
+}
+
+void gen_gvec_srhadd(unsigned vece, uint32_t rd_ofs, uint32_t rn_ofs,
+ uint32_t rm_ofs, uint32_t opr_sz, uint32_t max_sz)
+{
+static const TCGOpcode vecop_list[] = {
+INDEX_op_sari_vec, INDEX_op_add_vec, 0
+};
+static const GVecGen3 g[] = {
+{ .fni8 = gen_srhadd8_i64,
+ .fniv = gen_srhadd_vec,
+ .opt_opc = vecop_list,
+ .vece = MO_8 },
+{ .fni8 = gen_srhadd16_i64,
+ .fniv = gen_srhadd_vec,
+ .opt_opc = vecop_list,
+ .vece = MO_16 },
+{ .fni4 = gen_srhadd_i32,
+ .fniv = gen_srhadd_vec,
+ .opt_opc = vecop_list,
+ .vece = MO_32 },
+};
+assert(vece <= MO_32);
+tcg_gen_gvec_3(rd_ofs, rn_ofs, rm_ofs, opr_sz, max_sz, &g[vece]);
+}
+
+static void gen_urhadd8_i64(TCGv_i64 d, TCGv_i64 a, TCGv_i64 b)
+{
+TCGv_i64 t = tcg_temp_new_i64();
+
+tcg_gen_or_i64(t, a, b);
+tcg_gen_vec_shr8i_i64(a, a, 1);
+tcg_gen_vec_shr8i_i64(b, b, 1);
+tcg_gen_andi_i64(t, t, dup_const(MO_8, 1));
+tcg_gen_vec_add8_i64(d, a, b);
+tcg_gen_vec_add8_i64(d, d, t);
+}
+
+static void gen_urhadd16_i64(TCGv_i64 d, TCGv_i64 a, TCGv_i64 b)
+{
+TCGv_i64 t = tcg_temp_new_i64(
[PATCH v3 00/33] target/arm: Convert a64 advsimd to decodetree (part 1b)
Changes for v3:
* Reword prefetch unpredictable patch.
* Validate vector length when qc is an implied operand.
* Adjust some legacy decode based on review.
* Apply r-b.
Patches needing review:
01-target-arm-Diagnose-UNPREDICTABLE-operands-to-PLD.patch
03-target-arm-Assert-oprsz-in-range-when-using-vfp.q.patch
04-target-arm-Convert-SUQADD-and-USQADD-to-gvec.patch
10-target-arm-Convert-SRSHL-and-URSHL-register-to-gv.patch
12-target-arm-Convert-SQSHL-and-UQSHL-register-to-gv.patch
31-target-arm-Convert-SQDMULH-SQRDMULH-to-decodetree.patch
32-target-arm-Convert-FMADD-FMSUB-FNMADD-FNMSUB-to-d.patch
r~
Richard Henderson (33):
target/arm: Diagnose UNPREDICTABLE operands to PLD, PLDW, PLI
target/arm: Improve vector UQADD, UQSUB, SQADD, SQSUB
target/arm: Assert oprsz in range when using vfp.qc
target/arm: Convert SUQADD and USQADD to gvec
target/arm: Inline scalar SUQADD and USQADD
target/arm: Inline scalar SQADD, UQADD, SQSUB, UQSUB
target/arm: Convert SQADD, SQSUB, UQADD, UQSUB to decodetree
target/arm: Convert SUQADD, USQADD to decodetree
target/arm: Convert SSHL, USHL to decodetree
target/arm: Convert SRSHL and URSHL (register) to gvec
target/arm: Convert SRSHL, URSHL to decodetree
target/arm: Convert SQSHL and UQSHL (register) to gvec
target/arm: Convert SQSHL, UQSHL to decodetree
target/arm: Convert SQRSHL and UQRSHL (register) to gvec
target/arm: Convert SQRSHL, UQRSHL to decodetree
target/arm: Convert ADD, SUB (vector) to decodetree
target/arm: Convert CMGT, CMHI, CMGE, CMHS, CMTST, CMEQ to decodetree
target/arm: Use TCG_COND_TSTNE in gen_cmtst_{i32,i64}
target/arm: Use TCG_COND_TSTNE in gen_cmtst_vec
target/arm: Convert SHADD, UHADD to gvec
target/arm: Convert SHADD, UHADD to decodetree
target/arm: Convert SHSUB, UHSUB to gvec
target/arm: Convert SHSUB, UHSUB to decodetree
target/arm: Convert SRHADD, URHADD to gvec
target/arm: Convert SRHADD, URHADD to decodetree
target/arm: Convert SMAX, SMIN, UMAX, UMIN to decodetree
target/arm: Convert SABA, SABD, UABA, UABD to decodetree
target/arm: Convert MUL, PMUL to decodetree
target/arm: Convert MLA, MLS to decodetree
target/arm: Tidy SQDMULH, SQRDMULH (vector)
target/arm: Convert SQDMULH, SQRDMULH to decodetree
target/arm: Convert FMADD, FMSUB, FNMADD, FNMSUB to decodetree
target/arm: Convert FCSEL to decodetree
target/arm/helper.h | 96 ++-
target/arm/tcg/translate-a64.h | 14 +
target/arm/tcg/translate.h | 44 +
target/arm/tcg/a32-uncond.decode |8 +-
target/arm/tcg/a64.decode| 115 +++
target/arm/tcg/neon-dp.decode| 37 +-
target/arm/tcg/t32.decode|7 +-
target/arm/tcg/gengvec.c | 689 +++-
target/arm/tcg/gengvec64.c | 181
target/arm/tcg/neon_helper.c | 506 +++-
target/arm/tcg/translate-a64.c | 1321 ++
target/arm/tcg/translate-neon.c | 118 +--
target/arm/tcg/translate.c | 58 ++
target/arm/tcg/vec_helper.c | 128 +++
14 files changed, 1829 insertions(+), 1493 deletions(-)
--
2.34.1
[PATCH v3 05/33] target/arm: Inline scalar SUQADD and USQADD
This eliminates the last uses of these neon helpers.
Incorporate the MO_64 expanders as an option to the vector expander.
Reviewed-by: Peter Maydell
Signed-off-by: Richard Henderson
---
target/arm/helper.h| 8 --
target/arm/tcg/translate-a64.h | 8 ++
target/arm/tcg/gengvec64.c | 71 ++
target/arm/tcg/neon_helper.c | 165 -
target/arm/tcg/translate-a64.c | 73 +--
5 files changed, 103 insertions(+), 222 deletions(-)
diff --git a/target/arm/helper.h b/target/arm/helper.h
index de2c5c9aef..c76158d6d3 100644
--- a/target/arm/helper.h
+++ b/target/arm/helper.h
@@ -274,14 +274,6 @@ DEF_HELPER_FLAGS_3(neon_qadd_u16, TCG_CALL_NO_RWG, i32,
env, i32, i32)
DEF_HELPER_FLAGS_3(neon_qadd_s16, TCG_CALL_NO_RWG, i32, env, i32, i32)
DEF_HELPER_FLAGS_3(neon_qadd_u32, TCG_CALL_NO_RWG, i32, env, i32, i32)
DEF_HELPER_FLAGS_3(neon_qadd_s32, TCG_CALL_NO_RWG, i32, env, i32, i32)
-DEF_HELPER_FLAGS_3(neon_uqadd_s8, TCG_CALL_NO_RWG, i32, env, i32, i32)
-DEF_HELPER_FLAGS_3(neon_uqadd_s16, TCG_CALL_NO_RWG, i32, env, i32, i32)
-DEF_HELPER_FLAGS_3(neon_uqadd_s32, TCG_CALL_NO_RWG, i32, env, i32, i32)
-DEF_HELPER_FLAGS_3(neon_uqadd_s64, TCG_CALL_NO_RWG, i64, env, i64, i64)
-DEF_HELPER_FLAGS_3(neon_sqadd_u8, TCG_CALL_NO_RWG, i32, env, i32, i32)
-DEF_HELPER_FLAGS_3(neon_sqadd_u16, TCG_CALL_NO_RWG, i32, env, i32, i32)
-DEF_HELPER_FLAGS_3(neon_sqadd_u32, TCG_CALL_NO_RWG, i32, env, i32, i32)
-DEF_HELPER_FLAGS_3(neon_sqadd_u64, TCG_CALL_NO_RWG, i64, env, i64, i64)
DEF_HELPER_3(neon_qsub_u8, i32, env, i32, i32)
DEF_HELPER_3(neon_qsub_s8, i32, env, i32, i32)
DEF_HELPER_3(neon_qsub_u16, i32, env, i32, i32)
diff --git a/target/arm/tcg/translate-a64.h b/target/arm/tcg/translate-a64.h
index b5cb26f8a2..0fcf7cb63a 100644
--- a/target/arm/tcg/translate-a64.h
+++ b/target/arm/tcg/translate-a64.h
@@ -197,9 +197,17 @@ void gen_gvec_eor3(unsigned vece, uint32_t d, uint32_t n,
uint32_t m,
uint32_t a, uint32_t oprsz, uint32_t maxsz);
void gen_gvec_bcax(unsigned vece, uint32_t d, uint32_t n, uint32_t m,
uint32_t a, uint32_t oprsz, uint32_t maxsz);
+
+void gen_suqadd_bhs(TCGv_i64 res, TCGv_i64 qc,
+TCGv_i64 a, TCGv_i64 b, MemOp esz);
+void gen_suqadd_d(TCGv_i64 res, TCGv_i64 qc, TCGv_i64 a, TCGv_i64 b);
void gen_gvec_suqadd_qc(unsigned vece, uint32_t rd_ofs,
uint32_t rn_ofs, uint32_t rm_ofs,
uint32_t opr_sz, uint32_t max_sz);
+
+void gen_usqadd_bhs(TCGv_i64 res, TCGv_i64 qc,
+TCGv_i64 a, TCGv_i64 b, MemOp esz);
+void gen_usqadd_d(TCGv_i64 res, TCGv_i64 qc, TCGv_i64 a, TCGv_i64 b);
void gen_gvec_usqadd_qc(unsigned vece, uint32_t rd_ofs,
uint32_t rn_ofs, uint32_t rm_ofs,
uint32_t opr_sz, uint32_t max_sz);
diff --git a/target/arm/tcg/gengvec64.c b/target/arm/tcg/gengvec64.c
index b3afabd38b..2617cde0a5 100644
--- a/target/arm/tcg/gengvec64.c
+++ b/target/arm/tcg/gengvec64.c
@@ -188,6 +188,38 @@ void gen_gvec_bcax(unsigned vece, uint32_t d, uint32_t n,
uint32_t m,
tcg_gen_gvec_4(d, n, m, a, oprsz, maxsz, &op);
}
+/*
+ * Set @res to the correctly saturated result.
+ * Set @qc non-zero if saturation occured.
+ */
+void gen_suqadd_bhs(TCGv_i64 res, TCGv_i64 qc,
+TCGv_i64 a, TCGv_i64 b, MemOp esz)
+{
+TCGv_i64 max = tcg_constant_i64((1ull << ((8 << esz) - 1)) - 1);
+TCGv_i64 t = tcg_temp_new_i64();
+
+tcg_gen_add_i64(t, a, b);
+tcg_gen_smin_i64(res, t, max);
+tcg_gen_xor_i64(t, t, res);
+tcg_gen_or_i64(qc, qc, t);
+}
+
+void gen_suqadd_d(TCGv_i64 res, TCGv_i64 qc, TCGv_i64 a, TCGv_i64 b)
+{
+TCGv_i64 max = tcg_constant_i64(INT64_MAX);
+TCGv_i64 t = tcg_temp_new_i64();
+
+/* Maximum value that can be added to @a without overflow. */
+tcg_gen_sub_i64(t, max, a);
+
+/* Constrain addend so that the next addition never overflows. */
+tcg_gen_umin_i64(t, t, b);
+tcg_gen_add_i64(res, a, t);
+
+tcg_gen_xor_i64(t, t, b);
+tcg_gen_or_i64(qc, qc, t);
+}
+
static void gen_suqadd_vec(unsigned vece, TCGv_vec t, TCGv_vec qc,
TCGv_vec a, TCGv_vec b)
{
@@ -231,6 +263,7 @@ void gen_gvec_suqadd_qc(unsigned vece, uint32_t rd_ofs,
.write_aofs = true,
.vece = MO_32 },
{ .fniv = gen_suqadd_vec,
+ .fni8 = gen_suqadd_d,
.fno = gen_helper_gvec_suqadd_d,
.opt_opc = vecop_list,
.write_aofs = true,
@@ -242,6 +275,43 @@ void gen_gvec_suqadd_qc(unsigned vece, uint32_t rd_ofs,
rn_ofs, rm_ofs, opr_sz, max_sz, &ops[vece]);
}
+void gen_usqadd_bhs(TCGv_i64 res, TCGv_i64 qc,
+TCGv_i64 a, TCGv_i64 b, MemOp esz)
+{
+TCGv_i64 max = tcg_constant_i64(MAKE_64BIT_MASK(0, 8 << esz));
+TCGv_i64 zero = tcg_constant_i64(0);
+TCGv_i64 tmp = tcg_temp_new_i64();
+
+tcg_gen_add_i64(tmp, a, b);
+tcg
[PATCH v3 04/33] target/arm: Convert SUQADD and USQADD to gvec
Signed-off-by: Richard Henderson
---
target/arm/helper.h| 16 +
target/arm/tcg/translate-a64.h | 6 ++
target/arm/tcg/gengvec64.c | 110
target/arm/tcg/translate-a64.c | 113 ++---
target/arm/tcg/vec_helper.c| 64 +++
5 files changed, 245 insertions(+), 64 deletions(-)
diff --git a/target/arm/helper.h b/target/arm/helper.h
index f830531dd3..de2c5c9aef 100644
--- a/target/arm/helper.h
+++ b/target/arm/helper.h
@@ -836,6 +836,22 @@ DEF_HELPER_FLAGS_5(gvec_sqsub_s, TCG_CALL_NO_RWG,
void, ptr, ptr, ptr, ptr, i32)
DEF_HELPER_FLAGS_5(gvec_sqsub_d, TCG_CALL_NO_RWG,
void, ptr, ptr, ptr, ptr, i32)
+DEF_HELPER_FLAGS_5(gvec_usqadd_b, TCG_CALL_NO_RWG,
+ void, ptr, ptr, ptr, ptr, i32)
+DEF_HELPER_FLAGS_5(gvec_usqadd_h, TCG_CALL_NO_RWG,
+ void, ptr, ptr, ptr, ptr, i32)
+DEF_HELPER_FLAGS_5(gvec_usqadd_s, TCG_CALL_NO_RWG,
+ void, ptr, ptr, ptr, ptr, i32)
+DEF_HELPER_FLAGS_5(gvec_usqadd_d, TCG_CALL_NO_RWG,
+ void, ptr, ptr, ptr, ptr, i32)
+DEF_HELPER_FLAGS_5(gvec_suqadd_b, TCG_CALL_NO_RWG,
+ void, ptr, ptr, ptr, ptr, i32)
+DEF_HELPER_FLAGS_5(gvec_suqadd_h, TCG_CALL_NO_RWG,
+ void, ptr, ptr, ptr, ptr, i32)
+DEF_HELPER_FLAGS_5(gvec_suqadd_s, TCG_CALL_NO_RWG,
+ void, ptr, ptr, ptr, ptr, i32)
+DEF_HELPER_FLAGS_5(gvec_suqadd_d, TCG_CALL_NO_RWG,
+ void, ptr, ptr, ptr, ptr, i32)
DEF_HELPER_FLAGS_5(gvec_fmlal_a32, TCG_CALL_NO_RWG,
void, ptr, ptr, ptr, ptr, i32)
diff --git a/target/arm/tcg/translate-a64.h b/target/arm/tcg/translate-a64.h
index 91750f0ca9..b5cb26f8a2 100644
--- a/target/arm/tcg/translate-a64.h
+++ b/target/arm/tcg/translate-a64.h
@@ -197,6 +197,12 @@ void gen_gvec_eor3(unsigned vece, uint32_t d, uint32_t n,
uint32_t m,
uint32_t a, uint32_t oprsz, uint32_t maxsz);
void gen_gvec_bcax(unsigned vece, uint32_t d, uint32_t n, uint32_t m,
uint32_t a, uint32_t oprsz, uint32_t maxsz);
+void gen_gvec_suqadd_qc(unsigned vece, uint32_t rd_ofs,
+uint32_t rn_ofs, uint32_t rm_ofs,
+uint32_t opr_sz, uint32_t max_sz);
+void gen_gvec_usqadd_qc(unsigned vece, uint32_t rd_ofs,
+uint32_t rn_ofs, uint32_t rm_ofs,
+uint32_t opr_sz, uint32_t max_sz);
void gen_sve_ldr(DisasContext *s, TCGv_ptr, int vofs, int len, int rn, int
imm);
void gen_sve_str(DisasContext *s, TCGv_ptr, int vofs, int len, int rn, int
imm);
diff --git a/target/arm/tcg/gengvec64.c b/target/arm/tcg/gengvec64.c
index 093b498b13..b3afabd38b 100644
--- a/target/arm/tcg/gengvec64.c
+++ b/target/arm/tcg/gengvec64.c
@@ -188,3 +188,113 @@ void gen_gvec_bcax(unsigned vece, uint32_t d, uint32_t n,
uint32_t m,
tcg_gen_gvec_4(d, n, m, a, oprsz, maxsz, &op);
}
+static void gen_suqadd_vec(unsigned vece, TCGv_vec t, TCGv_vec qc,
+ TCGv_vec a, TCGv_vec b)
+{
+TCGv_vec max =
+tcg_constant_vec_matching(t, vece, (1ull << ((8 << vece) - 1)) - 1);
+TCGv_vec u = tcg_temp_new_vec_matching(t);
+
+/* Maximum value that can be added to @a without overflow. */
+tcg_gen_sub_vec(vece, u, max, a);
+
+/* Constrain addend so that the next addition never overflows. */
+tcg_gen_umin_vec(vece, u, u, b);
+tcg_gen_add_vec(vece, t, u, a);
+
+/* Compute QC by comparing the adjusted @b. */
+tcg_gen_xor_vec(vece, u, u, b);
+tcg_gen_or_vec(vece, qc, qc, u);
+}
+
+void gen_gvec_suqadd_qc(unsigned vece, uint32_t rd_ofs,
+uint32_t rn_ofs, uint32_t rm_ofs,
+uint32_t opr_sz, uint32_t max_sz)
+{
+static const TCGOpcode vecop_list[] = {
+INDEX_op_add_vec, INDEX_op_sub_vec, INDEX_op_umin_vec, 0
+};
+static const GVecGen4 ops[4] = {
+{ .fniv = gen_suqadd_vec,
+ .fno = gen_helper_gvec_suqadd_b,
+ .opt_opc = vecop_list,
+ .write_aofs = true,
+ .vece = MO_8 },
+{ .fniv = gen_suqadd_vec,
+ .fno = gen_helper_gvec_suqadd_h,
+ .opt_opc = vecop_list,
+ .write_aofs = true,
+ .vece = MO_16 },
+{ .fniv = gen_suqadd_vec,
+ .fno = gen_helper_gvec_suqadd_s,
+ .opt_opc = vecop_list,
+ .write_aofs = true,
+ .vece = MO_32 },
+{ .fniv = gen_suqadd_vec,
+ .fno = gen_helper_gvec_suqadd_d,
+ .opt_opc = vecop_list,
+ .write_aofs = true,
+ .vece = MO_64 },
+};
+
+tcg_debug_assert(opr_sz <= sizeof_field(CPUARMState, vfp.qc));
+tcg_gen_gvec_4(rd_ofs, offsetof(CPUARMState, vfp.qc),
+ rn_ofs, rm_ofs, opr_sz, max_sz, &ops[vece]);
+}
+
+static void gen_usqadd_vec(unsigned vece, TCGv_vec t, TCGv_vec qc,
+ TCGv_vec a, TCGv_ve
Re: [PATCH 5/5] contrib/plugins: add ips plugin example for cost modeling
On 5/28/24 12:57, Alex Bennée wrote:
Pierrick Bouvier writes:
On 5/28/24 12:14, Alex Bennée wrote:
Pierrick Bouvier writes:
This plugin uses the new time control interface to make decisions
about the state of time during the emulation. The algorithm is
currently very simple. The user specifies an ips rate which applies
per core. If the core runs ahead of its allocated execution time the
plugin sleeps for a bit to let real time catch up. Either way time is
updated for the emulation as a function of total executed instructions
with some adjustments for cores that idle.
Examples
Slow down execution of /bin/true:
$ num_insn=$(./build/qemu-x86_64 -plugin ./build/tests/plugin/libinsn.so -d plugin
/bin/true |& grep total | sed -e 's/.*: //')
$ time ./build/qemu-x86_64 -plugin
./build/contrib/plugins/libips.so,ips=$(($num_insn/4)) /bin/true
real 4.000s
Boot a Linux kernel simulating a 250MHz cpu:
$ /build/qemu-system-x86_64 -kernel /boot/vmlinuz-6.1.0-21-amd64 -append
"console=ttyS0" -plugin
./build/contrib/plugins/libips.so,ips=$((250*1000*1000)) -smp 1 -m 512
check time until kernel panic on serial0
Signed-off-by: Pierrick Bouvier
---
contrib/plugins/ips.c| 239 +++
contrib/plugins/Makefile | 1 +
2 files changed, 240 insertions(+)
create mode 100644 contrib/plugins/ips.c
diff --git a/contrib/plugins/ips.c b/contrib/plugins/ips.c
new file mode 100644
index 000..cf3159df391
--- /dev/null
+++ b/contrib/plugins/ips.c
@@ -0,0 +1,239 @@
+/*
+ * ips rate limiting plugin.
+ *
+ * This plugin can be used to restrict the execution of a system to a
+ * particular number of Instructions Per Second (ips). This controls
+ * time as seen by the guest so while wall-clock time may be longer
+ * from the guests point of view time will pass at the normal rate.
+ *
+ * This uses the new plugin API which allows the plugin to control
+ * system time.
+ *
+ * Copyright (c) 2023 Linaro Ltd
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include
+#include
+#include
+
+QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
+
+/* how many times do we update time per sec */
+#define NUM_TIME_UPDATE_PER_SEC 10
+#define NSEC_IN_ONE_SEC (1000 * 1000 * 1000)
+
+static GMutex global_state_lock;
+
+static uint64_t insn_per_second = 1000 * 1000; /* ips per core, per second */
+static uint64_t insn_quantum; /* trap every N instructions */
+static bool precise_execution; /* count every instruction */
+static int64_t start_time_ns; /* time (ns since epoch) first vCPU started */
+static int64_t virtual_time_ns; /* last set virtual time */
+
+static const void *time_handle;
+
+typedef enum {
+UNKNOWN = 0,
+EXECUTING,
+IDLE,
+FINISHED
+} vCPUState;
+
+typedef struct {
+uint64_t counter;
+uint64_t track_insn;
+vCPUState state;
+/* timestamp when vCPU entered state */
+int64_t last_state_time;
+} vCPUTime;
+
+struct qemu_plugin_scoreboard *vcpus;
+
+/* return epoch time in ns */
+static int64_t now_ns(void)
+{
+return g_get_real_time() * 1000;
+}
+
+static uint64_t num_insn_during(int64_t elapsed_ns)
+{
+double num_secs = elapsed_ns / (double) NSEC_IN_ONE_SEC;
+return num_secs * (double) insn_per_second;
+}
+
+static int64_t time_for_insn(uint64_t num_insn)
+{
+double num_secs = (double) num_insn / (double) insn_per_second;
+return num_secs * (double) NSEC_IN_ONE_SEC;
+}
+
+static int64_t uptime_ns(void)
+{
+int64_t now = now_ns();
+g_assert(now >= start_time_ns);
+return now - start_time_ns;
+}
+
+static void vcpu_set_state(vCPUTime *vcpu, vCPUState new_state)
+{
+vcpu->last_state_time = now_ns();
+vcpu->state = new_state;
+}
+
+static void update_system_time(vCPUTime *vcpu)
+{
+/* flush remaining instructions */
+vcpu->counter += vcpu->track_insn;
+vcpu->track_insn = 0;
+
+int64_t uptime = uptime_ns();
+uint64_t expected_insn = num_insn_during(uptime);
+
+if (vcpu->counter >= expected_insn) {
+/* this vcpu ran faster than expected, so it has to sleep */
+uint64_t insn_advance = vcpu->counter - expected_insn;
+uint64_t time_advance_ns = time_for_insn(insn_advance);
+int64_t sleep_us = time_advance_ns / 1000;
+g_usleep(sleep_us);
+}
+
+/* based on number of instructions, what should be the new time? */
+int64_t new_virtual_time = time_for_insn(vcpu->counter);
+
+g_mutex_lock(&global_state_lock);
+
+/* Time only moves forward. Another vcpu might have updated it already. */
+if (new_virtual_time > virtual_time_ns) {
+qemu_plugin_update_ns(time_handle, new_virtual_time);
+virtual_time_ns = new_virtual_time;
+}
+
+g_mutex_unlock(&global_state_lock);
+}
+
+static void set_start_time()
+{
+g_mutex_lock(&global_state_lock);
+if (!start_time_ns) {
+start_time_ns = now_ns();
+}
+g_mutex_unlock(&global_state_lock);
+}
+
+static void vcpu_init(q
Re: [PULL v2 0/7] Block jobs patches for 2024-04-29
On 5/28/24 06:57, Vladimir Sementsov-Ogievskiy wrote: The following changes since commit ad10b4badc1dd5b28305f9b9f1168cf0aa3ae946: Merge tag 'pull-error-2024-05-27' ofhttps://repo.or.cz/qemu/armbru into staging (2024-05-27 06:40:42 -0700) are available in the Git repository at: https://gitlab.com/vsementsov/qemu.git tags/pull-block-jobs-2024-04-29-v2 for you to fetch changes up to a149401048481247bcbaf6035a7a1308974fb464: iotests/pylintrc: allow up to 10 similar lines (2024-05-28 15:52:15 +0300) Block jobs patches for 2024-04-29 v2: add "iotests/pylintrc: allow up to 10 similar lines" to fix check-python-minreqs - backup: discard-source parameter - blockcommit: Reopen base image as RO after abort Applied, thanks. Please update https://wiki.qemu.org/ChangeLog/9.1 as appropriate. r~
Re: [PATCH 5/5] contrib/plugins: add ips plugin example for cost modeling
Pierrick Bouvier writes:
> On 5/28/24 12:14, Alex Bennée wrote:
>> Pierrick Bouvier writes:
>>
>>> This plugin uses the new time control interface to make decisions
>>> about the state of time during the emulation. The algorithm is
>>> currently very simple. The user specifies an ips rate which applies
>>> per core. If the core runs ahead of its allocated execution time the
>>> plugin sleeps for a bit to let real time catch up. Either way time is
>>> updated for the emulation as a function of total executed instructions
>>> with some adjustments for cores that idle.
>>>
>>> Examples
>>>
>>>
>>> Slow down execution of /bin/true:
>>> $ num_insn=$(./build/qemu-x86_64 -plugin ./build/tests/plugin/libinsn.so -d
>>> plugin /bin/true |& grep total | sed -e 's/.*: //')
>>> $ time ./build/qemu-x86_64 -plugin
>>> ./build/contrib/plugins/libips.so,ips=$(($num_insn/4)) /bin/true
>>> real 4.000s
>>>
>>> Boot a Linux kernel simulating a 250MHz cpu:
>>> $ /build/qemu-system-x86_64 -kernel /boot/vmlinuz-6.1.0-21-amd64 -append
>>> "console=ttyS0" -plugin
>>> ./build/contrib/plugins/libips.so,ips=$((250*1000*1000)) -smp 1 -m 512
>>> check time until kernel panic on serial0
>>>
>>> Signed-off-by: Pierrick Bouvier
>>> ---
>>> contrib/plugins/ips.c| 239 +++
>>> contrib/plugins/Makefile | 1 +
>>> 2 files changed, 240 insertions(+)
>>> create mode 100644 contrib/plugins/ips.c
>>>
>>> diff --git a/contrib/plugins/ips.c b/contrib/plugins/ips.c
>>> new file mode 100644
>>> index 000..cf3159df391
>>> --- /dev/null
>>> +++ b/contrib/plugins/ips.c
>>> @@ -0,0 +1,239 @@
>>> +/*
>>> + * ips rate limiting plugin.
>>> + *
>>> + * This plugin can be used to restrict the execution of a system to a
>>> + * particular number of Instructions Per Second (ips). This controls
>>> + * time as seen by the guest so while wall-clock time may be longer
>>> + * from the guests point of view time will pass at the normal rate.
>>> + *
>>> + * This uses the new plugin API which allows the plugin to control
>>> + * system time.
>>> + *
>>> + * Copyright (c) 2023 Linaro Ltd
>>> + *
>>> + * SPDX-License-Identifier: GPL-2.0-or-later
>>> + */
>>> +
>>> +#include
>>> +#include
>>> +#include
>>> +
>>> +QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
>>> +
>>> +/* how many times do we update time per sec */
>>> +#define NUM_TIME_UPDATE_PER_SEC 10
>>> +#define NSEC_IN_ONE_SEC (1000 * 1000 * 1000)
>>> +
>>> +static GMutex global_state_lock;
>>> +
>>> +static uint64_t insn_per_second = 1000 * 1000; /* ips per core, per second
>>> */
>>> +static uint64_t insn_quantum; /* trap every N instructions */
>>> +static bool precise_execution; /* count every instruction */
>>> +static int64_t start_time_ns; /* time (ns since epoch) first vCPU started
>>> */
>>> +static int64_t virtual_time_ns; /* last set virtual time */
>>> +
>>> +static const void *time_handle;
>>> +
>>> +typedef enum {
>>> +UNKNOWN = 0,
>>> +EXECUTING,
>>> +IDLE,
>>> +FINISHED
>>> +} vCPUState;
>>> +
>>> +typedef struct {
>>> +uint64_t counter;
>>> +uint64_t track_insn;
>>> +vCPUState state;
>>> +/* timestamp when vCPU entered state */
>>> +int64_t last_state_time;
>>> +} vCPUTime;
>>> +
>>> +struct qemu_plugin_scoreboard *vcpus;
>>> +
>>> +/* return epoch time in ns */
>>> +static int64_t now_ns(void)
>>> +{
>>> +return g_get_real_time() * 1000;
>>> +}
>>> +
>>> +static uint64_t num_insn_during(int64_t elapsed_ns)
>>> +{
>>> +double num_secs = elapsed_ns / (double) NSEC_IN_ONE_SEC;
>>> +return num_secs * (double) insn_per_second;
>>> +}
>>> +
>>> +static int64_t time_for_insn(uint64_t num_insn)
>>> +{
>>> +double num_secs = (double) num_insn / (double) insn_per_second;
>>> +return num_secs * (double) NSEC_IN_ONE_SEC;
>>> +}
>>> +
>>> +static int64_t uptime_ns(void)
>>> +{
>>> +int64_t now = now_ns();
>>> +g_assert(now >= start_time_ns);
>>> +return now - start_time_ns;
>>> +}
>>> +
>>> +static void vcpu_set_state(vCPUTime *vcpu, vCPUState new_state)
>>> +{
>>> +vcpu->last_state_time = now_ns();
>>> +vcpu->state = new_state;
>>> +}
>>> +
>>> +static void update_system_time(vCPUTime *vcpu)
>>> +{
>>> +/* flush remaining instructions */
>>> +vcpu->counter += vcpu->track_insn;
>>> +vcpu->track_insn = 0;
>>> +
>>> +int64_t uptime = uptime_ns();
>>> +uint64_t expected_insn = num_insn_during(uptime);
>>> +
>>> +if (vcpu->counter >= expected_insn) {
>>> +/* this vcpu ran faster than expected, so it has to sleep */
>>> +uint64_t insn_advance = vcpu->counter - expected_insn;
>>> +uint64_t time_advance_ns = time_for_insn(insn_advance);
>>> +int64_t sleep_us = time_advance_ns / 1000;
>>> +g_usleep(sleep_us);
>>> +}
>>> +
>>> +/* based on number of instructions, what should be the new time? */
>>> +int64_t new_virtual_time = time_for_insn(vcpu->counter);
>>> +
>>> +g_
Re: [PATCH 5/5] contrib/plugins: add ips plugin example for cost modeling
On 5/28/24 12:14, Alex Bennée wrote:
Pierrick Bouvier writes:
This plugin uses the new time control interface to make decisions
about the state of time during the emulation. The algorithm is
currently very simple. The user specifies an ips rate which applies
per core. If the core runs ahead of its allocated execution time the
plugin sleeps for a bit to let real time catch up. Either way time is
updated for the emulation as a function of total executed instructions
with some adjustments for cores that idle.
Examples
Slow down execution of /bin/true:
$ num_insn=$(./build/qemu-x86_64 -plugin ./build/tests/plugin/libinsn.so -d plugin
/bin/true |& grep total | sed -e 's/.*: //')
$ time ./build/qemu-x86_64 -plugin
./build/contrib/plugins/libips.so,ips=$(($num_insn/4)) /bin/true
real 4.000s
Boot a Linux kernel simulating a 250MHz cpu:
$ /build/qemu-system-x86_64 -kernel /boot/vmlinuz-6.1.0-21-amd64 -append
"console=ttyS0" -plugin
./build/contrib/plugins/libips.so,ips=$((250*1000*1000)) -smp 1 -m 512
check time until kernel panic on serial0
Signed-off-by: Pierrick Bouvier
---
contrib/plugins/ips.c| 239 +++
contrib/plugins/Makefile | 1 +
2 files changed, 240 insertions(+)
create mode 100644 contrib/plugins/ips.c
diff --git a/contrib/plugins/ips.c b/contrib/plugins/ips.c
new file mode 100644
index 000..cf3159df391
--- /dev/null
+++ b/contrib/plugins/ips.c
@@ -0,0 +1,239 @@
+/*
+ * ips rate limiting plugin.
+ *
+ * This plugin can be used to restrict the execution of a system to a
+ * particular number of Instructions Per Second (ips). This controls
+ * time as seen by the guest so while wall-clock time may be longer
+ * from the guests point of view time will pass at the normal rate.
+ *
+ * This uses the new plugin API which allows the plugin to control
+ * system time.
+ *
+ * Copyright (c) 2023 Linaro Ltd
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include
+#include
+#include
+
+QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
+
+/* how many times do we update time per sec */
+#define NUM_TIME_UPDATE_PER_SEC 10
+#define NSEC_IN_ONE_SEC (1000 * 1000 * 1000)
+
+static GMutex global_state_lock;
+
+static uint64_t insn_per_second = 1000 * 1000; /* ips per core, per second */
+static uint64_t insn_quantum; /* trap every N instructions */
+static bool precise_execution; /* count every instruction */
+static int64_t start_time_ns; /* time (ns since epoch) first vCPU started */
+static int64_t virtual_time_ns; /* last set virtual time */
+
+static const void *time_handle;
+
+typedef enum {
+UNKNOWN = 0,
+EXECUTING,
+IDLE,
+FINISHED
+} vCPUState;
+
+typedef struct {
+uint64_t counter;
+uint64_t track_insn;
+vCPUState state;
+/* timestamp when vCPU entered state */
+int64_t last_state_time;
+} vCPUTime;
+
+struct qemu_plugin_scoreboard *vcpus;
+
+/* return epoch time in ns */
+static int64_t now_ns(void)
+{
+return g_get_real_time() * 1000;
+}
+
+static uint64_t num_insn_during(int64_t elapsed_ns)
+{
+double num_secs = elapsed_ns / (double) NSEC_IN_ONE_SEC;
+return num_secs * (double) insn_per_second;
+}
+
+static int64_t time_for_insn(uint64_t num_insn)
+{
+double num_secs = (double) num_insn / (double) insn_per_second;
+return num_secs * (double) NSEC_IN_ONE_SEC;
+}
+
+static int64_t uptime_ns(void)
+{
+int64_t now = now_ns();
+g_assert(now >= start_time_ns);
+return now - start_time_ns;
+}
+
+static void vcpu_set_state(vCPUTime *vcpu, vCPUState new_state)
+{
+vcpu->last_state_time = now_ns();
+vcpu->state = new_state;
+}
+
+static void update_system_time(vCPUTime *vcpu)
+{
+/* flush remaining instructions */
+vcpu->counter += vcpu->track_insn;
+vcpu->track_insn = 0;
+
+int64_t uptime = uptime_ns();
+uint64_t expected_insn = num_insn_during(uptime);
+
+if (vcpu->counter >= expected_insn) {
+/* this vcpu ran faster than expected, so it has to sleep */
+uint64_t insn_advance = vcpu->counter - expected_insn;
+uint64_t time_advance_ns = time_for_insn(insn_advance);
+int64_t sleep_us = time_advance_ns / 1000;
+g_usleep(sleep_us);
+}
+
+/* based on number of instructions, what should be the new time? */
+int64_t new_virtual_time = time_for_insn(vcpu->counter);
+
+g_mutex_lock(&global_state_lock);
+
+/* Time only moves forward. Another vcpu might have updated it already. */
+if (new_virtual_time > virtual_time_ns) {
+qemu_plugin_update_ns(time_handle, new_virtual_time);
+virtual_time_ns = new_virtual_time;
+}
+
+g_mutex_unlock(&global_state_lock);
+}
+
+static void set_start_time()
+{
+g_mutex_lock(&global_state_lock);
+if (!start_time_ns) {
+start_time_ns = now_ns();
+}
+g_mutex_unlock(&global_state_lock);
+}
+
+static void vcpu_init(qemu_plugin_id_t id, unsigned int cpu_index)
+{
+vCPUTime *vcpu = q
Re: [PATCH v2 2/6] tests/qtest/migration-test: Fix and enable test_ignore_shared
On Tue, May 28, 2024 at 10:42:06AM +1000, Nicholas Piggin wrote:
> This test is already starting to bitrot, so first remove it from ifdef
> and fix compile issues. ppc64 transfers about 2MB, so bump the size
> threshold too.
>
> It was said to be broken on aarch64 but it may have been the limited shm
> size under gitlab CI. The test is now excluded from running on CI so it
> shouldn't cause too much annoyance.
>
> So let's try enable it.
>
> Cc: Yury Kotov
> Cc: Dr. David Alan Gilbert
Dave's new email is:
d...@treblig.org
Please feel free to use it in a repost.
Thanks,
> Signed-off-by: Nicholas Piggin
> ---
> tests/qtest/migration-test.c | 14 --
> 1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> index 04bf1c0092..8247ed98f2 100644
> --- a/tests/qtest/migration-test.c
> +++ b/tests/qtest/migration-test.c
> @@ -1893,14 +1893,15 @@ static void
> test_precopy_unix_tls_x509_override_host(void)
> #endif /* CONFIG_TASN1 */
> #endif /* CONFIG_GNUTLS */
>
> -#if 0
> -/* Currently upset on aarch64 TCG */
> static void test_ignore_shared(void)
> {
> g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
> QTestState *from, *to;
> +MigrateStart args = {
> +.use_shmem = true,
> +};
>
> -if (test_migrate_start(&from, &to, uri, false, true, NULL, NULL)) {
> +if (test_migrate_start(&from, &to, uri, &args)) {
> return;
> }
>
> @@ -1925,11 +1926,11 @@ static void test_ignore_shared(void)
> wait_for_migration_complete(from);
>
> /* Check whether shared RAM has been really skipped */
> -g_assert_cmpint(read_ram_property_int(from, "transferred"), <, 1024 *
> 1024);
> +g_assert_cmpint(read_ram_property_int(from, "transferred"), <,
> + 4 * 1024 * 1024);
>
> test_migrate_end(from, to, true);
> }
> -#endif
>
> static void *
> test_migrate_xbzrle_start(QTestState *from,
> @@ -3580,7 +3581,8 @@ int main(int argc, char **argv)
> #endif /* CONFIG_TASN1 */
> #endif /* CONFIG_GNUTLS */
>
> -/* migration_test_add("/migration/ignore_shared", test_ignore_shared); */
> +migration_test_add("/migration/ignore_shared", test_ignore_shared);
> +
> #ifndef _WIN32
> migration_test_add("/migration/precopy/fd/tcp",
> test_migrate_precopy_fd_socket);
> --
> 2.43.0
>
--
Peter Xu
Re: [PATCH 5/5] contrib/plugins: add ips plugin example for cost modeling
Pierrick Bouvier writes:
> This plugin uses the new time control interface to make decisions
> about the state of time during the emulation. The algorithm is
> currently very simple. The user specifies an ips rate which applies
> per core. If the core runs ahead of its allocated execution time the
> plugin sleeps for a bit to let real time catch up. Either way time is
> updated for the emulation as a function of total executed instructions
> with some adjustments for cores that idle.
>
> Examples
>
>
> Slow down execution of /bin/true:
> $ num_insn=$(./build/qemu-x86_64 -plugin ./build/tests/plugin/libinsn.so -d
> plugin /bin/true |& grep total | sed -e 's/.*: //')
> $ time ./build/qemu-x86_64 -plugin
> ./build/contrib/plugins/libips.so,ips=$(($num_insn/4)) /bin/true
> real 4.000s
>
> Boot a Linux kernel simulating a 250MHz cpu:
> $ /build/qemu-system-x86_64 -kernel /boot/vmlinuz-6.1.0-21-amd64 -append
> "console=ttyS0" -plugin
> ./build/contrib/plugins/libips.so,ips=$((250*1000*1000)) -smp 1 -m 512
> check time until kernel panic on serial0
>
> Signed-off-by: Pierrick Bouvier
> ---
> contrib/plugins/ips.c| 239 +++
> contrib/plugins/Makefile | 1 +
> 2 files changed, 240 insertions(+)
> create mode 100644 contrib/plugins/ips.c
>
> diff --git a/contrib/plugins/ips.c b/contrib/plugins/ips.c
> new file mode 100644
> index 000..cf3159df391
> --- /dev/null
> +++ b/contrib/plugins/ips.c
> @@ -0,0 +1,239 @@
> +/*
> + * ips rate limiting plugin.
> + *
> + * This plugin can be used to restrict the execution of a system to a
> + * particular number of Instructions Per Second (ips). This controls
> + * time as seen by the guest so while wall-clock time may be longer
> + * from the guests point of view time will pass at the normal rate.
> + *
> + * This uses the new plugin API which allows the plugin to control
> + * system time.
> + *
> + * Copyright (c) 2023 Linaro Ltd
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#include
> +#include
> +#include
> +
> +QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
> +
> +/* how many times do we update time per sec */
> +#define NUM_TIME_UPDATE_PER_SEC 10
> +#define NSEC_IN_ONE_SEC (1000 * 1000 * 1000)
> +
> +static GMutex global_state_lock;
> +
> +static uint64_t insn_per_second = 1000 * 1000; /* ips per core, per second */
> +static uint64_t insn_quantum; /* trap every N instructions */
> +static bool precise_execution; /* count every instruction */
> +static int64_t start_time_ns; /* time (ns since epoch) first vCPU started */
> +static int64_t virtual_time_ns; /* last set virtual time */
> +
> +static const void *time_handle;
> +
> +typedef enum {
> +UNKNOWN = 0,
> +EXECUTING,
> +IDLE,
> +FINISHED
> +} vCPUState;
> +
> +typedef struct {
> +uint64_t counter;
> +uint64_t track_insn;
> +vCPUState state;
> +/* timestamp when vCPU entered state */
> +int64_t last_state_time;
> +} vCPUTime;
> +
> +struct qemu_plugin_scoreboard *vcpus;
> +
> +/* return epoch time in ns */
> +static int64_t now_ns(void)
> +{
> +return g_get_real_time() * 1000;
> +}
> +
> +static uint64_t num_insn_during(int64_t elapsed_ns)
> +{
> +double num_secs = elapsed_ns / (double) NSEC_IN_ONE_SEC;
> +return num_secs * (double) insn_per_second;
> +}
> +
> +static int64_t time_for_insn(uint64_t num_insn)
> +{
> +double num_secs = (double) num_insn / (double) insn_per_second;
> +return num_secs * (double) NSEC_IN_ONE_SEC;
> +}
> +
> +static int64_t uptime_ns(void)
> +{
> +int64_t now = now_ns();
> +g_assert(now >= start_time_ns);
> +return now - start_time_ns;
> +}
> +
> +static void vcpu_set_state(vCPUTime *vcpu, vCPUState new_state)
> +{
> +vcpu->last_state_time = now_ns();
> +vcpu->state = new_state;
> +}
> +
> +static void update_system_time(vCPUTime *vcpu)
> +{
> +/* flush remaining instructions */
> +vcpu->counter += vcpu->track_insn;
> +vcpu->track_insn = 0;
> +
> +int64_t uptime = uptime_ns();
> +uint64_t expected_insn = num_insn_during(uptime);
> +
> +if (vcpu->counter >= expected_insn) {
> +/* this vcpu ran faster than expected, so it has to sleep */
> +uint64_t insn_advance = vcpu->counter - expected_insn;
> +uint64_t time_advance_ns = time_for_insn(insn_advance);
> +int64_t sleep_us = time_advance_ns / 1000;
> +g_usleep(sleep_us);
> +}
> +
> +/* based on number of instructions, what should be the new time? */
> +int64_t new_virtual_time = time_for_insn(vcpu->counter);
> +
> +g_mutex_lock(&global_state_lock);
> +
> +/* Time only moves forward. Another vcpu might have updated it already.
> */
> +if (new_virtual_time > virtual_time_ns) {
> +qemu_plugin_update_ns(time_handle, new_virtual_time);
> +virtual_time_ns = new_virtual_time;
> +}
> +
> +g_mutex_unlock(&global_state_lock);
> +}
> +
> +static void set_star
Re: [PATCH 1/5] sysemu: add set_virtual_time to accel ops
On 5/28/24 10:11, Alex Bennée wrote: Pierrick Bouvier writes: From: Alex Bennée We are about to remove direct calls to individual accelerators for this information and will need a central point for plugins to hook into time changes. From: Alex Bennée Signed-off-by: Alex Bennée Reviewed-by: Philippe Mathieu-Daudé Just a note, when patches written by other people come via your tree you should add your s-o-b tag to indicate: "I'm legally okay to contribute this and happy for it to go into QEMU" Thanks for clarifying, I didn't know it was needed as a committer (vs author), and checkpatch.pl does not seem to check for this. I'll add this when reposting the series, if we have some comments.
Re: [PATCH 0/6] accel: Restrict TCG plugin (un)registration to TCG accel
On 5/28/24 07:59, Philippe Mathieu-Daudé wrote: > Philippe Mathieu-Daudé (6): system/runstate: Remove unused 'qemu/plugin.h' header accel/tcg: Move common declarations to 'internal-common.h' accel: Clarify accel_cpu_common_[un]realize() use unassigned vCPU accel: Introduce accel_cpu_common_[un]realize_assigned() handlers accel: Restrict TCG plugin (un)registration to TCG accel accel/tcg: Move qemu_plugin_vcpu_init__async() to plugins/ Reviewed-by: Pierrick Bouvier
Re: [RFC PATCH 4/4] ci: Add the new migration device tests
On Tue, May 28, 2024 at 03:10:48PM -0300, Fabiano Rosas wrote:
> Peter Xu writes:
>
> > On Mon, May 27, 2024 at 08:59:00PM -0300, Fabiano Rosas wrote:
> >> Peter Xu writes:
> >>
> >> > On Thu, May 23, 2024 at 05:19:22PM -0300, Fabiano Rosas wrote:
> >> >> We have two new migration tests that check cross version
> >> >> compatibility. One uses the vmstate-static-checker.py script to
> >> >> compare the vmstate structures from two different QEMU versions. The
> >> >> other runs a simple migration with a few devices present in the VM, to
> >> >> catch obvious breakages.
> >> >>
> >> >> Add both tests to the migration-compat-common job.
> >> >>
> >> >> Signed-off-by: Fabiano Rosas
> >> >> ---
> >> >> .gitlab-ci.d/buildtest.yml | 43 +++---
> >> >> 1 file changed, 36 insertions(+), 7 deletions(-)
> >> >>
> >> >> diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
> >> >> index 91c57efded..bc7ac35983 100644
> >> >> --- a/.gitlab-ci.d/buildtest.yml
> >> >> +++ b/.gitlab-ci.d/buildtest.yml
> >> >> @@ -202,18 +202,47 @@ build-previous-qemu:
> >> >>needs:
> >> >> - job: build-previous-qemu
> >> >> - job: build-system-opensuse
> >> >> - # The old QEMU could have bugs unrelated to migration that are
> >> >> - # already fixed in the current development branch, so this test
> >> >> - # might fail.
> >> >> + # This test is allowed to fail because:
> >> >> + #
> >> >> + # - The old QEMU could have bugs unrelated to migration that are
> >> >> + # already fixed in the current development branch.
> >> >
> >> > Did you ever hit a real failure with this? I'm wondering whether we can
> >> > remove this allow_failure thing.
> >> >
> >>
> >> I haven't. But when it fails we'll go through an entire release cycle
> >> with this thing showing red for every person that runs the CI. Remember,
> >> this is a CI failure to which there's no fix aside from waiting for the
> >> release to happen. Even if we're quick to react and disable the job, I
> >> feel it might create some confusion already.
> >
> > My imagination was if needed we'll get complains and we add that until
> > then for that broken release only, and remove in the next release again.
> >
> >>
> >> >> + #
> >> >> + # - The vmstate-static-checker script trips on renames and other
> >> >> + # backward-compatible changes to the vmstate structs.
> >> >
> >> > I think I keep my preference per last time we talked on this. :)
> >>
> >> Sorry, I'm not trying to force this in any way, I just wrote these to
> >> use in the pull-request and thought I'd put it out there. At the very
> >> least we can have your concerns documented. =)
> >
> > Yep that's fine. I think we should keep such discussion on the list,
> > especially we have different opinions, while none of us got convinced yet
> > so far. :)
> >
> >>
> >> > I still think it's too early to involve a test that can report false
> >> > negative.
> >>
> >> (1)
> >> Well, we haven't seen any false negatives, we've seen fields being
> >> renamed. If that happens, then we'll ask the person to update the
> >> script. Is that not acceptable to you? Or are you thinking about other
> >> sorts of issues?
> >
> > Then question is how to update the script. So far it's treated as failure
> > on rename, even if it's benign. Right now we have this:
> >
> > print("Section \"" + sec + "\",", end=' ')
> > print("Description \"" + desc + "\":", end=' ')
> > print("expected field \"" + s_item["field"] + "\",", end=' ')
> > print("got \"" + d_item["field"] + "\"; skipping rest")
> > bump_taint()
> > break
> >
> > Do you want to introduce a list of renamed vmsd fields in this script and
> > maintain that? IMHO it's an overkill and unnecessary burden to other
> > developers.
> >
>
> That's not _my_ idea, we already have that (see below). There's not much
> reason to rename fields like that, the vmstate is obviously something
> that should be kept stable, so having to do a rename in a script is way
> better than having to figure out the fix for the compatibility break.
>
> def check_fields_match(name, s_field, d_field):
> if s_field == d_field:
> return True
>
> # Some fields changed names between qemu versions. This list
> # is used to allow such changes in each section / description.
> changed_names = {
> 'apic': ['timer', 'timer_expiry'],
> 'e1000': ['dev', 'parent_obj'],
> 'ehci': ['dev', 'pcidev'],
> 'I440FX': ['dev', 'parent_obj'],
> 'ich9_ahci': ['card', 'parent_obj'],
> 'ich9-ahci': ['ahci', 'ich9_ahci'],
> 'ioh3420': ['PCIDevice', 'PCIEDevice'],
> 'ioh-3240-express-root-port': ['port.br.dev',
>'parent_obj.parent_obj.parent_obj',
>'port.br.dev.exp.aer_log',
>
> 'parent_obj.parent_obj
[PATCH 1/1] tests/avocado: update sbsa-ref firmware
Partial support for NUMA setup: - cpu nodes - memory nodes Used versions: - Trusted Firmware v2.11.0 - Tianocore EDK2 stable202405 - Tianocore EDK2 Platforms code commit 4bbd0ed Firmware is built using Debian 'bookworm' cross toolchain (gcc 12.2.0). --- tests/avocado/machine_aarch64_sbsaref.py | 20 ++-- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/tests/avocado/machine_aarch64_sbsaref.py b/tests/avocado/machine_aarch64_sbsaref.py index 98c76c1ff7..6bb82f2a03 100644 --- a/tests/avocado/machine_aarch64_sbsaref.py +++ b/tests/avocado/machine_aarch64_sbsaref.py @@ -37,18 +37,18 @@ def fetch_firmware(self): Used components: -- Trusted Firmware 2.10.2 -- Tianocore EDK2 stable202402 -- Tianocore EDK2-platforms commit 085c2fb +- Trusted Firmware 2.11.0 +- Tianocore EDK2 stable202405 +- Tianocore EDK2-platforms commit 4bbd0ed """ # Secure BootRom (TF-A code) fs0_xz_url = ( "https://artifacts.codelinaro.org/artifactory/linaro-419-sbsa-ref/"; -"20240313-116475/edk2/SBSA_FLASH0.fd.xz" +"20240528-140808/edk2/SBSA_FLASH0.fd.xz" ) -fs0_xz_hash = "637593749cc307dea7dc13265c32e5d020267552f22b18a31850b8429fc5e159" +fs0_xz_hash = "fa6004900b67172914c908b78557fec4d36a5f784f4c3dd08f49adb75e1892a9" tar_xz_path = self.fetch_asset(fs0_xz_url, asset_hash=fs0_xz_hash, algorithm='sha256') archive.extract(tar_xz_path, self.workdir) @@ -57,9 +57,9 @@ def fetch_firmware(self): # Non-secure rom (UEFI and EFI variables) fs1_xz_url = ( "https://artifacts.codelinaro.org/artifactory/linaro-419-sbsa-ref/"; -"20240313-116475/edk2/SBSA_FLASH1.fd.xz" +"20240528-140808/edk2/SBSA_FLASH1.fd.xz" ) -fs1_xz_hash = "cb0a5e8cf5e303c5d3dc106cfd5943ffe9714b86afddee7164c69ee1dd41991c" +fs1_xz_hash = "5f3747d4000bc416d9641e33ff4ac60c3cc8cb74ca51b6e932e58531c62eb6f7" tar_xz_path = self.fetch_asset(fs1_xz_url, asset_hash=fs1_xz_hash, algorithm='sha256') archive.extract(tar_xz_path, self.workdir) @@ -98,15 +98,15 @@ def test_sbsaref_edk2_firmware(self): # AP Trusted ROM wait_for_console_pattern(self, "Booting Trusted Firmware") -wait_for_console_pattern(self, "BL1: v2.10.2(release):") +wait_for_console_pattern(self, "BL1: v2.11.0(release):") wait_for_console_pattern(self, "BL1: Booting BL2") # Trusted Boot Firmware -wait_for_console_pattern(self, "BL2: v2.10.2(release)") +wait_for_console_pattern(self, "BL2: v2.11.0(release)") wait_for_console_pattern(self, "Booting BL31") # EL3 Runtime Software -wait_for_console_pattern(self, "BL31: v2.10.2(release)") +wait_for_console_pattern(self, "BL31: v2.11.0(release)") # Non-trusted Firmware wait_for_console_pattern(self, "UEFI firmware (version 1.0") -- 2.45.1
Re: [PULL v2 00/42] target-arm queue
On 5/28/24 07:07, Peter Maydell wrote: Hi; most of this is the first half of the A64 simd decodetree conversion; the rest is a mix of fixes from the last couple of weeks. v2 uses patches from the v2 decodetree series to avoid a few regressions in some A32 insns. (Richard: I'm still planning to review the second half of the v2 decodetree series; I just wanted to get the respin of this pullreq out today...) thanks -- PMM The following changes since commit ad10b4badc1dd5b28305f9b9f1168cf0aa3ae946: Merge tag 'pull-error-2024-05-27' ofhttps://repo.or.cz/qemu/armbru into staging (2024-05-27 06:40:42 -0700) are available in the Git repository at: https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20240528 for you to fetch changes up to f240df3c31b40e4cf1af1f156a88efc1a1df406c: target/arm: Convert disas_simd_3same_logic to decodetree (2024-05-28 14:29:01 +0100) target-arm queue: * xlnx_dpdma: fix descriptor endianness bug * hvf: arm: Fix encodings for ID_AA64PFR1_EL1 and debug System registers * hw/arm/npcm7xx: remove setting of mp-affinity * hw/char: Correct STM32L4x5 usart register CR2 field ADD_0 size * hw/intc/arm_gic: Fix handling of NS view of GICC_APR * hw/input/tsc2005: Fix -Wchar-subscripts warning in tsc2005_txrx() * hw: arm: Remove use of tabs in some source files * docs/system: Remove ADC from raspi documentation * target/arm: Start of the conversion of A64 SIMD to decodetree Applied, thanks. Please update https://wiki.qemu.org/ChangeLog/9.1 as appropriate. r~
Re: [PATCH V1 08/26] migration: vmstate_info_void_ptr
On Tue, May 28, 2024 at 11:10:16AM -0400, Steven Sistare wrote: > On 5/27/2024 2:31 PM, Peter Xu wrote: > > On Mon, Apr 29, 2024 at 08:55:17AM -0700, Steve Sistare wrote: > > > Define VMSTATE_VOID_PTR so the value of a pointer (but not its target) > > > can be saved in the migration stream. This will be needed for CPR. > > > > > > Signed-off-by: Steve Sistare > > > > This is really tricky. > > > > From a first glance, I don't think migrating a VA is valid at all for > > migration even if with exec.. and looks insane to me for a cross-process > > migration, which seems to be allowed to use as a generic VMSD helper.. as > > VA is the address space barrier for different processes and I think it > > normally even apply to generic execve(), and we're trying to jailbreak for > > some reason.. > > > > It definitely won't work for any generic migration as sizeof(void*) can be > > different afaict between hosts, e.g. 32bit -> 64bit migrations. > > > > Some description would be really helpful in this commit message, > > e.g. explain the users and why. Do we need to poison that for generic VMSD > > use (perhaps with prefixed underscores)? I think I'll need to read on the > > rest to tell.. > > Short answer: we never dereference the void* in the new process. And must > not. > > Longer answer: > > During CPR for vfio, each mapped DMA region is re-registered in the new > process using the new VA. The ioctl to re-register identifies the mapping > by IOVA and length. > > The same requirement holds for CPR of iommufd devices. However, in the > iommufd framework, IOVA does not uniquely identify a dma mapping, and we > need to use the old VA as the unique identifier. The new process > re-registers each mapping, passing the old VA and new VA to the kernel. > The old VA is never dereferenced in the new process, we just need its value. > > I suspected that the void* which must not be dereferenced might make people > uncomfortable. I have an older version of my code which adds a uint64_t > field to RAMBlock for recording and migrating the old VA. The saving and > loading code is slightly less elegant, but no big deal. Would you prefer > that? I see, thanks for explaining. Yes that sounds better to me. Re the ugliness: is that about a pre_save() plus one extra uint64_t field? In that case it looks better comparing to migrating "void*". I'm trying to read some context on the vaddr remap thing from you, and I found this: https://lore.kernel.org/all/y90bvbnrvraceq%2f...@nvidia.com/ So it will work with iommufd now? Meanwhile, what's the status for mdev? Looks like it isn't supported yet for both. Thanks, -- Peter Xu
Re: block snapshot issue with RBD
Hi Ilya On 5/28/24 11:13 AM, Ilya Dryomov wrote: On Mon, May 27, 2024 at 9:06 PM Jin Cao wrote: Supplementary info: VM is paused after "migrate" command. After being resumed with "cont", snapshot_delete_blkdev_internal works again, which is confusing, as disk snapshot generally recommend I/O is paused, and a frozen VM satisfy this requirement. Hi Jin, This doesn't seem to be related to RBD. Given that the same error is observed when using the RBD driver with the raw format, I would dig in the direction of migration somehow "installing" the raw format (which is on-disk compatible with the rbd format). Thanks for the hint. Also, did you mean to say "snapshot_blkdev_internal" instead of "snapshot_delete_blkdev_internal" in both instances? Sorry for my copy-and-paste mistake. Yes, it's snapshot_blkdev_internal. -- Sincerely, Jin Cao Thanks, Ilya -- Sincerely Jin Cao On 5/27/24 10:56 AM, Jin Cao wrote: CC block and migration related address. On 5/27/24 12:03 AM, Jin Cao wrote: Hi, I encountered RBD block snapshot issue after doing migration. Steps - 1. Start QEMU with: ./qemu-system-x86_64 -name VM -machine q35 -accel kvm -cpu host,migratable=on -m 2G -boot menu=on,strict=on rbd:image/ubuntu-22.04-server-cloudimg-amd64.raw -net nic -net user -cdrom /home/my/path/of/cloud-init.iso -monitor stdio 2. Do block snapshot in monitor cmd: snapshot_delete_blkdev_internal. It works as expected: the snapshot is visable with command`rbd snap ls pool_name/image_name`. 3. Do pseudo migration with monitor cmd: migrate -d exec:cat>/tmp/vm.out 4. Do block snapshot again with snapshot_delete_blkdev_internal, then I get: Error: Block format 'raw' used by device 'ide0-hd0' does not support internal snapshots I was hoping to do the second block snapshot successfully, and it feels abnormal the RBD block snapshot function is disrupted after migration. BTW, I get the same block snapshot error when I start QEMU with: "-drive format=raw,file=rbd:pool_name/image_name" My questions is: how could I proceed with RBD block snapshot after the pseudo migration?
Re: [PATCH V2 0/3] improve -overcommit cpu-pm=on|off
On 5/28/2024 2:23 AM, Igor Mammedov wrote: > On Fri, 24 May 2024 13:00:14 -0700 > Zide Chen wrote: > >> Currently, if running "-overcommit cpu-pm=on" on hosts that don't >> have MWAIT support, the MWAIT/MONITOR feature is advertised to the >> guest and executing MWAIT/MONITOR on the guest triggers #UD. > > this is missing proper description how do you trigger issue > with reproducer and detailed description why guest sees MWAIT > when it's not supported by host. If "overcommit cpu-pm=on" and "-cpu hpst" are present, as shown in the following, CPUID_EXT_MONITOR is set after x86_cpu_filter_features(), so that it doesn't have a chance to check MWAIT against host features and will be advertised to the guest regardless of whether it's supported by the host or not. x86_cpu_realizefn() x86_cpu_filter_features() cpu_exec_realizefn() kvm_cpu_realizefn host_cpu_realizefn host_cpu_enable_cpu_pm env->features[FEAT_1_ECX] |= CPUID_EXT_MONITOR; If it's not supported by the host, executing MONITOR or MWAIT instructions from the guest triggers #UD, no matter MWAIT_EXITING control is set or not.
Re: [PATCH v7 00/12] Enabling DCD emulation support in Qemu
On Thu, May 16, 2024 at 10:05:33AM -0700, fan wrote: > On Fri, Apr 19, 2024 at 02:24:36PM -0400, Gregory Price wrote: > > On Thu, Apr 18, 2024 at 04:10:51PM -0700, nifan@gmail.com wrote: > > > A git tree of this series can be found here (with one extra commit on top > > > for printing out accepted/pending extent list): > > > https://github.com/moking/qemu/tree/dcd-v7 > > > > > > v6->v7: > > > > > > 1. Fixed the dvsec range register issue mentioned in the the cover letter > > > in v6. > > >Only relevant bits are set to mark the device ready (Patch 6). > > > (Jonathan) > > > 2. Moved the if statement in cxl_setup_memory from Patch 6 to Patch 4. > > > (Jonathan) > > > 3. Used MIN instead of if statement to get record_count in Patch 7. > > > (Jonathan) > > > 4. Added "Reviewed-by" tag to Patch 7. > > > 5. Modified cxl_dc_extent_release_dry_run so the updated extent list can > > > be > > >reused in cmd_dcd_release_dyn_cap to simplify the process in Patch 8. > > > (Jørgen) > > > 6. Added comments to indicate further "TODO" items in > > > cmd_dcd_add_dyn_cap_rsp. > > > (Jonathan) > > > 7. Avoided irrelevant code reformat in Patch 8. (Jonathan) > > > 8. Modified QMP interfaces for adding/releasing DC extents to allow > > > passing > > >tags, selection policy, flags in the interface. (Jonathan, Gregory) > > > 9. Redesigned the pending list so extents in the same requests are grouped > > > together. A new data structure is introduced to represent "extent > > > group" > > > in pending list. (Jonathan) > > > 10. Added support in QMP interface for "More" flag. > > > 11. Check "Forced removal" flag for release request and not let it pass > > > through. > > > 12. Removed the dynamic capacity log type from CxlEventLog definition in > > > cxl.json > > >to avoid the side effect it may introduce to inject error to DC event > > > log. > > >(Jonathan) > > > 13. Hard coded the event log type to dynamic capacity event log in QMP > > > interfaces. (Jonathan) > > > 14. Adding space in between "-1]". (Jonathan) > > > 15. Some minor comment fixes. > > > > > > The code is tested with similar setup and has passed similar tests as > > > listed > > > in the cover letter of v5[1] and v6[2]. > > > Also, the code is tested with the latest DCD kernel patchset[3]. > > > > > > [1] Qemu DCD patchset v5: > > > https://lore.kernel.org/linux-cxl/20240304194331.1586191-1-nifan@gmail.com/T/#t > > > [2] Qemu DCD patchset v6: > > > https://lore.kernel.org/linux-cxl/20240325190339.696686-1-nifan@gmail.com/T/#t > > > [3] DCD kernel patches: > > > https://lore.kernel.org/linux-cxl/20240324-dcd-type2-upstream-v1-0-b7b00d623...@intel.com/T/#m11c571e21c4fe17c7d04ec5c2c7bc7cbf2cd07e3 > > > > > > > added review to all patches, will hopefully be able to add a Tested-by > > tag early next week, along with a v1 RFC for MHD bit-tracking. > > > > We've been testing v5/v6 for a bit, so I expect as soon as we get the > > MHD code ported over to v7 i'll ship a tested-by tag pretty quick. > > > > The super-set release will complicate a few things but this doesn't > > look like a blocker on our end, just a change to how we track bits in a > > shared bit/bytemap. > > > > Hi Gregory, > I am planning to address all the concerns in this series and send out v8 > next week. Jonathan mentioned you have few related patches built on top > of this series, can you point me to the latest version so I can look > into it? Also, would you like me to carry them over to send together > with my series in next version? It could be easier for you to avoid the > potential rebase needed for your patches? > > Let me know. > > Thanks, > Fan > I apologize for missing this, I was out of the country for a few weeks. I'm still catching up on the work history. I think i saw in passing that you picked up the CCI changes, and those were the ones causing conflicts - so that's perfect. I can always rebase on that. ~ Gregory
Re: [PATCH] tests/qtest/migrate-test: Use regular file file for shared-memory tests
Peter Xu writes: > On Tue, May 28, 2024 at 09:35:22AM -0400, Peter Xu wrote: >> On Tue, May 28, 2024 at 02:27:57PM +1000, Nicholas Piggin wrote: >> > There is no need to use /dev/shm for file-backed memory devices, and >> > it is too small to be usable in gitlab CI. Switch to using a regular >> > file in /tmp/ which will usually have more space available. >> > >> > Signed-off-by: Nicholas Piggin >> > --- >> > Am I missing something? AFAIKS there is not even any point using >> > /dev/shm aka tmpfs anyway, there is not much special about it as a >> > filesystem. This applies on top of the series just sent, and passes >> > gitlab CI qtests including aarch64. >> >> I think it's just that /dev/shm guarantees shmem usage, while the var >> "tmpfs" implies g_dir_make_tmp() which may be another non-ram based file >> system, while that'll be slightly different comparing to what a real user >> would use - we don't suggest user to put guest RAM on things like btrfs. >> >> One real implication is if we add a postcopy test it'll fail with >> g_dir_make_tmp() when it is not pointing to a shmem mount, as >> UFFDIO_REGISTER will fail there. But that test doesn't yet exist as the >> QEMU paths should be the same even if Linux will trigger different paths >> when different types of mem is used (anonymous v.s. shmem). >> >> If the goal here is to properly handle the case where tmpfs doesn't have >> enough space, how about what I suggested in the other email? >> >> https://lore.kernel.org/r/ZlSppKDE6wzjCF--@x1n >> >> IOW, try populate the shmem region before starting the guest, skip if >> population failed. Would that work? > > Let me append some more info here.. > > I think madvise() isn't needed as fallocate() should do the population work > already, afaiu, then it means we pass the shmem path to QEMU and QEMU > should notice this memory-backend-file existed, open() directly. > > I quicked walk the QEMU memory code and so far it looks all applicable, so > that QEMU should just start the guest with the pre-populated shmem page > caches. > > There's one trick where qemu_ram_mmap() will map some extra pages, on x86 > 4k, and I don't yet know why we did that.. > > /* > * Note: this always allocates at least one extra page of virtual address > * space, even if size is already aligned. > */ > total = size + align; At the end of the function: /* * Leave a single PROT_NONE page allocated after the RAM block, to serve as * a guard page guarding against potential buffer overflows. */
Re: [PATCH 0/6] accel: Restrict TCG plugin (un)registration to TCG accel
On 5/28/24 07:59, Philippe Mathieu-Daudé wrote: Philippe Mathieu-Daudé (6): system/runstate: Remove unused 'qemu/plugin.h' header accel/tcg: Move common declarations to 'internal-common.h' accel: Clarify accel_cpu_common_[un]realize() use unassigned vCPU accel: Introduce accel_cpu_common_[un]realize_assigned() handlers accel: Restrict TCG plugin (un)registration to TCG accel accel/tcg: Move qemu_plugin_vcpu_init__async() to plugins/ Reviewed-by: Richard Henderson r~
Re: block snapshot issue with RBD
On Mon, May 27, 2024 at 9:06 PM Jin Cao wrote: > > Supplementary info: VM is paused after "migrate" command. After being > resumed with "cont", snapshot_delete_blkdev_internal works again, which > is confusing, as disk snapshot generally recommend I/O is paused, and a > frozen VM satisfy this requirement. Hi Jin, This doesn't seem to be related to RBD. Given that the same error is observed when using the RBD driver with the raw format, I would dig in the direction of migration somehow "installing" the raw format (which is on-disk compatible with the rbd format). Also, did you mean to say "snapshot_blkdev_internal" instead of "snapshot_delete_blkdev_internal" in both instances? Thanks, Ilya > > -- > Sincerely > Jin Cao > > On 5/27/24 10:56 AM, Jin Cao wrote: > > CC block and migration related address. > > > > On 5/27/24 12:03 AM, Jin Cao wrote: > >> Hi, > >> > >> I encountered RBD block snapshot issue after doing migration. > >> > >> Steps > >> - > >> > >> 1. Start QEMU with: > >> ./qemu-system-x86_64 -name VM -machine q35 -accel kvm -cpu > >> host,migratable=on -m 2G -boot menu=on,strict=on > >> rbd:image/ubuntu-22.04-server-cloudimg-amd64.raw -net nic -net user > >> -cdrom /home/my/path/of/cloud-init.iso -monitor stdio > >> > >> 2. Do block snapshot in monitor cmd: snapshot_delete_blkdev_internal. > >> It works as expected: the snapshot is visable with command`rbd snap ls > >> pool_name/image_name`. > >> > >> 3. Do pseudo migration with monitor cmd: migrate -d exec:cat>/tmp/vm.out > >> > >> 4. Do block snapshot again with snapshot_delete_blkdev_internal, then > >> I get: > >> Error: Block format 'raw' used by device 'ide0-hd0' does not > >> support internal snapshots > >> > >> I was hoping to do the second block snapshot successfully, and it > >> feels abnormal the RBD block snapshot function is disrupted after > >> migration. > >> > >> BTW, I get the same block snapshot error when I start QEMU with: > >> "-drive format=raw,file=rbd:pool_name/image_name" > >> > >> My questions is: how could I proceed with RBD block snapshot after the > >> pseudo migration?
Re: [RFC PATCH 4/4] ci: Add the new migration device tests
Peter Xu writes:
> On Mon, May 27, 2024 at 08:59:00PM -0300, Fabiano Rosas wrote:
>> Peter Xu writes:
>>
>> > On Thu, May 23, 2024 at 05:19:22PM -0300, Fabiano Rosas wrote:
>> >> We have two new migration tests that check cross version
>> >> compatibility. One uses the vmstate-static-checker.py script to
>> >> compare the vmstate structures from two different QEMU versions. The
>> >> other runs a simple migration with a few devices present in the VM, to
>> >> catch obvious breakages.
>> >>
>> >> Add both tests to the migration-compat-common job.
>> >>
>> >> Signed-off-by: Fabiano Rosas
>> >> ---
>> >> .gitlab-ci.d/buildtest.yml | 43 +++---
>> >> 1 file changed, 36 insertions(+), 7 deletions(-)
>> >>
>> >> diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
>> >> index 91c57efded..bc7ac35983 100644
>> >> --- a/.gitlab-ci.d/buildtest.yml
>> >> +++ b/.gitlab-ci.d/buildtest.yml
>> >> @@ -202,18 +202,47 @@ build-previous-qemu:
>> >>needs:
>> >> - job: build-previous-qemu
>> >> - job: build-system-opensuse
>> >> - # The old QEMU could have bugs unrelated to migration that are
>> >> - # already fixed in the current development branch, so this test
>> >> - # might fail.
>> >> + # This test is allowed to fail because:
>> >> + #
>> >> + # - The old QEMU could have bugs unrelated to migration that are
>> >> + # already fixed in the current development branch.
>> >
>> > Did you ever hit a real failure with this? I'm wondering whether we can
>> > remove this allow_failure thing.
>> >
>>
>> I haven't. But when it fails we'll go through an entire release cycle
>> with this thing showing red for every person that runs the CI. Remember,
>> this is a CI failure to which there's no fix aside from waiting for the
>> release to happen. Even if we're quick to react and disable the job, I
>> feel it might create some confusion already.
>
> My imagination was if needed we'll get complains and we add that until
> then for that broken release only, and remove in the next release again.
>
>>
>> >> + #
>> >> + # - The vmstate-static-checker script trips on renames and other
>> >> + # backward-compatible changes to the vmstate structs.
>> >
>> > I think I keep my preference per last time we talked on this. :)
>>
>> Sorry, I'm not trying to force this in any way, I just wrote these to
>> use in the pull-request and thought I'd put it out there. At the very
>> least we can have your concerns documented. =)
>
> Yep that's fine. I think we should keep such discussion on the list,
> especially we have different opinions, while none of us got convinced yet
> so far. :)
>
>>
>> > I still think it's too early to involve a test that can report false
>> > negative.
>>
>> (1)
>> Well, we haven't seen any false negatives, we've seen fields being
>> renamed. If that happens, then we'll ask the person to update the
>> script. Is that not acceptable to you? Or are you thinking about other
>> sorts of issues?
>
> Then question is how to update the script. So far it's treated as failure
> on rename, even if it's benign. Right now we have this:
>
> print("Section \"" + sec + "\",", end=' ')
> print("Description \"" + desc + "\":", end=' ')
> print("expected field \"" + s_item["field"] + "\",", end=' ')
> print("got \"" + d_item["field"] + "\"; skipping rest")
> bump_taint()
> break
>
> Do you want to introduce a list of renamed vmsd fields in this script and
> maintain that? IMHO it's an overkill and unnecessary burden to other
> developers.
>
That's not _my_ idea, we already have that (see below). There's not much
reason to rename fields like that, the vmstate is obviously something
that should be kept stable, so having to do a rename in a script is way
better than having to figure out the fix for the compatibility break.
def check_fields_match(name, s_field, d_field):
if s_field == d_field:
return True
# Some fields changed names between qemu versions. This list
# is used to allow such changes in each section / description.
changed_names = {
'apic': ['timer', 'timer_expiry'],
'e1000': ['dev', 'parent_obj'],
'ehci': ['dev', 'pcidev'],
'I440FX': ['dev', 'parent_obj'],
'ich9_ahci': ['card', 'parent_obj'],
'ich9-ahci': ['ahci', 'ich9_ahci'],
'ioh3420': ['PCIDevice', 'PCIEDevice'],
'ioh-3240-express-root-port': ['port.br.dev',
'parent_obj.parent_obj.parent_obj',
'port.br.dev.exp.aer_log',
'parent_obj.parent_obj.parent_obj.exp.aer_log'],
'cirrus_vga': ['hw_cursor_x', 'vga.hw_cursor_x',
'hw_cursor_y', 'vga.hw_cursor_y'],
'lsiscsi': ['dev', 'parent_obj'],
'mch': ['d', 'parent_obj'],
'pci_bridge': ['bridge.dev', 'parent_obj', 'bridge.dev.shpc'
Re: [PATCH V1 07/26] migration: VMStateId
On Tue, May 28, 2024 at 11:10:03AM -0400, Steven Sistare via wrote:
> On 5/27/2024 2:20 PM, Peter Xu wrote:
> > On Mon, Apr 29, 2024 at 08:55:16AM -0700, Steve Sistare wrote:
> > > Define a type for the 256 byte id string to guarantee the same length is
> > > used and enforced everywhere.
> > >
> > > Signed-off-by: Steve Sistare
> > > ---
> > > include/exec/ramblock.h | 3 ++-
> > > include/migration/vmstate.h | 2 ++
> > > migration/savevm.c | 8
> > > migration/vmstate.c | 3 ++-
> > > 4 files changed, 10 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/include/exec/ramblock.h b/include/exec/ramblock.h
> > > index 0babd10..61deefe 100644
> > > --- a/include/exec/ramblock.h
> > > +++ b/include/exec/ramblock.h
> > > @@ -23,6 +23,7 @@
> > > #include "cpu-common.h"
> > > #include "qemu/rcu.h"
> > > #include "exec/ramlist.h"
> > > +#include "migration/vmstate.h"
> > > struct RAMBlock {
> > > struct rcu_head rcu;
> > > @@ -35,7 +36,7 @@ struct RAMBlock {
> > > void (*resized)(const char*, uint64_t length, void *host);
> > > uint32_t flags;
> > > /* Protected by the BQL. */
> > > -char idstr[256];
> > > +VMStateId idstr;
> > > /* RCU-enabled, writes protected by the ramlist lock */
> > > QLIST_ENTRY(RAMBlock) next;
> > > QLIST_HEAD(, RAMBlockNotifier) ramblock_notifiers;
> >
> > Hmm.. Don't look like a good idea to include a migration header in
> > ramblock.h? Is this ramblock change needed for this work?
>
> Well, entities that are migrated include migration headers, and now that
> includes RAMBlock. There is precedent:
>
> 0 include/exec/ramblock.h 26 #include "migration/vmstate.h"
> 1 include/hw/acpi/ich9_tco. 14 #include "migration/vmstate.h"
> 2 include/hw/display/ramfb. 4 #include "migration/vmstate.h"
> 3 include/hw/hyperv/vmbus.h 16 #include "migration/vmstate.h"
> 4 include/hw/input/pl050.h 14 #include "migration/vmstate.h"
> 5 include/hw/pci/shpc.h 7 #include "migration/vmstate.h"
> 6 include/hw/virtio/virtio. 20 #include "migration/vmstate.h"
> 7 include/migration/cpu.h8 #include "migration/vmstate.h"
>
> Granted, only some of the C files that include ramblock.h need all of
> vmstate.h.
> I could define VMStateId in a smaller file such as migration/misc.h, or a
> new file migration/vmstateid.h, and include that in ramblock.h.
> Any preference?
One issue here is currently the idstr[] of ramblock is a verbose name of
the ramblock, and logically it doesn't need to have anything to do with
vmstate.
I'll continue to read to see why we need VMStateID here for a ramblock. So
if it is necessary, maybe the name VMStateID to be used here is misleading?
It'll also be nice to separate the changes, as ramblock.h doesn't belong to
migration subsystem but memory api.
Thanks,
--
Peter Xu
Re: [PATCH v2 38/67] target/arm: Convert SUQADD and USQADD to gvec
On 5/28/24 08:37, Peter Maydell wrote:
On Sat, 25 May 2024 at 00:32, Richard Henderson
wrote:
Signed-off-by: Richard Henderson
---
target/arm/helper.h| 16 +
target/arm/tcg/translate-a64.h | 6 ++
target/arm/tcg/gengvec64.c | 106 +++
target/arm/tcg/translate-a64.c | 113 ++---
target/arm/tcg/vec_helper.c| 64 +++
5 files changed, 241 insertions(+), 64 deletions(-)
diff --git a/target/arm/tcg/gengvec64.c b/target/arm/tcg/gengvec64.c
index 093b498b13..4b76e476a0 100644
--- a/target/arm/tcg/gengvec64.c
+++ b/target/arm/tcg/gengvec64.c
@@ -188,3 +188,109 @@ void gen_gvec_bcax(unsigned vece, uint32_t d, uint32_t n,
uint32_t m,
tcg_gen_gvec_4(d, n, m, a, oprsz, maxsz, &op);
}
+static void gen_suqadd_vec(unsigned vece, TCGv_vec t, TCGv_vec qc,
+ TCGv_vec a, TCGv_vec b)
+{
+TCGv_vec max =
+tcg_constant_vec_matching(t, vece, (1ull << ((8 << vece) - 1)) - 1);
+TCGv_vec u = tcg_temp_new_vec_matching(t);
+
+/* Maximum value that can be added to @a without overflow. */
+tcg_gen_sub_vec(vece, u, max, a);
+
+/* Constrain addend so that the next addition never overflows. */
+tcg_gen_umin_vec(vece, u, u, b);
+tcg_gen_add_vec(vece, t, u, a);
+
+/* Compute QC by comparing the adjusted @b. */
+tcg_gen_xor_vec(vece, u, u, b);
+tcg_gen_or_vec(vece, qc, qc, u);
With this kind of code where we wind up doing a vector op
into vfp.qc, is there anything somewhere that asserts that
we don't try to do it with a vector length bigger than
sizeof(vfp.qc) (i.e. 128) ?
No, but I could add an assert to the top-level expander below.
(In this case gen_gvec_usqadd_qc.)
r~
Re: [PATCH] scripts/update-linux-headers.sh: Remove temporary directory inbetween
On 28/05/2024 17.56, Michael S. Tsirkin wrote: On Mon, May 27, 2024 at 08:02:43AM +0200, Thomas Huth wrote: We are reusing the same temporary directory for installing the headers of all targets, so there could be stale files here when switching from one target to another. Make sure to delete the folder before installing a new set of target headers into it. Signed-off-by: Thomas Huth Reviewed-by: Michael S. Tsirkin Thanks! who's merging this? I don't mind ... if nobody objects, I can put it into my next pull request. Thomas
Re: [PATCH v2 03/67] target/arm: Reject incorrect operands to PLD, PLDW, PLI
On 5/28/24 06:18, Peter Maydell wrote: On Sat, 25 May 2024 at 00:25, Richard Henderson wrote: For all, rm == 15 is invalid. Prior to v8, thumb with rm == 13 is invalid. For PLDW, rn == 15 is invalid. Fixes a RISU mismatch for the HINTSPACE pattern in t32.risu compared to a neoverse-n1 host. These are UNPREDICTABLE cases, not invalid. In general we don't try to match a specific implementation's UNPREDICTABLE choices. I think we're better off avoiding the mismatch by improving the risu patterns to avoid the UNPREDICTABLE cases. We do plenty of other treatments of UNPREDICTABLE as UNDEF (e.g. STREX). Why is this case any different? r~
Re: [PATCH v2 66/67] target/arm: Convert FMADD, FMSUB, FNMADD, FNMSUB to decodetree
On 5/28/24 09:15, Peter Maydell wrote:
On Sat, 25 May 2024 at 00:27, Richard Henderson
wrote:
These are the only instructions in the 3 source scalar class.
Signed-off-by: Richard Henderson
---
target/arm/tcg/a64.decode | 10 ++
target/arm/tcg/translate-a64.c | 233 -
2 files changed, 93 insertions(+), 150 deletions(-)
static void disas_data_proc_fp(DisasContext *s, uint32_t insn)
{
-if (extract32(insn, 24, 1)) {
-/* Floating point data-processing (3 source) */
-disas_fp_3src(s, insn);
-} else if (extract32(insn, 21, 1) == 0) {
+if (extract32(insn, 21, 1) == 0) {
/* Floating point to fixed point conversions */
disas_fp_fixed_conv(s, insn);
} else {
Doesn't this result in the unallocated-encodings in the
fp-3src class now falling into the "else" clause and
being mis-decoded as the wrong thing? I think this
needs to be
if (extract32(insn, 24, 1)) {
unallocated_encoding();
} else if (extract32(insn, 21, 1) == 0) {
[etc]
Agreed.
r~
Re: [PATCH v2 46/67] target/arm: Convert SQSHL and UQSHL (register) to gvec
On 5/28/24 08:53, Peter Maydell wrote:
On Sat, 25 May 2024 at 00:28, Richard Henderson
wrote:
Signed-off-by: Richard Henderson
---
target/arm/helper.h | 8
target/arm/tcg/translate.h | 4
target/arm/tcg/neon-dp.decode | 10 ++---
target/arm/tcg/gengvec.c| 24 ++
target/arm/tcg/neon_helper.c| 36 +
target/arm/tcg/translate-a64.c | 17 +++-
target/arm/tcg/translate-neon.c | 6 ++
7 files changed, 83 insertions(+), 22 deletions(-)
+#define NEON_GVEC_VOP2_ENV(name, vtype) \
+void HELPER(name)(void *vd, void *vn, void *vm, void *venv, uint32_t desc) \
+{ \
+intptr_t i, opr_sz = simd_oprsz(desc); \
+vtype *d = vd, *n = vn, *m = vm;\
+CPUARMState *env = venv;\
+for (i = 0; i < opr_sz / sizeof(vtype); i++) { \
+NEON_FN(d[i], n[i], m[i]); \
+} \
+clear_tail(d, opr_sz, simd_maxsz(desc));\
+}
+
Same question about H macros as for patch 44.
No H macros needed.
r~
Re: [PATCH v2 44/67] target/arm: Convert SRSHL and URSHL (register) to gvec
On 5/28/24 08:51, Peter Maydell wrote:
On Sat, 25 May 2024 at 00:27, Richard Henderson
wrote:
Signed-off-by: Richard Henderson
---
target/arm/helper.h | 10 +
target/arm/tcg/translate.h | 4
target/arm/tcg/neon-dp.decode | 10 ++---
target/arm/tcg/gengvec.c| 22 +++
target/arm/tcg/neon_helper.c| 38 -
target/arm/tcg/translate-a64.c | 17 ++-
target/arm/tcg/translate-neon.c | 6 ++
7 files changed, 84 insertions(+), 23 deletions(-)
uint32_t HELPER(glue(neon_,name))(CPUARMState *env, uint32_t arg1, uint32_t
arg2) \
NEON_VOP_BODY(vtype, n)
+#define NEON_GVEC_VOP2(name, vtype) \
+void HELPER(name)(void *vd, void *vn, void *vm, uint32_t desc) \
+{ \
+intptr_t i, opr_sz = simd_oprsz(desc); \
+vtype *d = vd, *n = vn, *m = vm;\
+for (i = 0; i < opr_sz / sizeof(vtype); i++) { \
+NEON_FN(d[i], n[i], m[i]); \
Does this need H macros for the bigendian case ? It looks
like we use it for smaller-than-64-bit element cases.
The same operation happens in each lane, and order of evaluation does not matter. So, no
H macros needed.
r~
Re: [PATCH v2 65/67] target/arm: Convert SQDMULH, SQRDMULH to decodetree
On 5/28/24 09:10, Peter Maydell wrote:
+void HELPER(neon_sqdmulh_idx_s)(void *vd, void *vn, void *vm,
+void *vq, uint32_t desc)
+{
+intptr_t i, j, opr_sz = simd_oprsz(desc);
+int idx = simd_data(desc);
+int32_t *d = vd, *n = vn, *m = (int32_t *)vm + H4(idx);
+
+for (i = 0; i < opr_sz / 4; i += 16 / 4) {
+int32_t mm = m[i];
+for (j = 0; j < 16 / 4; ++j) {
+d[i + j] = do_sqrdmlah_s(n[i + j], mm, 0, false, false, vq);
+}
+}
+clear_tail(d, opr_sz, simd_maxsz(desc));
+}
+
+void HELPER(neon_sqrdmulh_idx_s)(void *vd, void *vn, void *vm,
+ void *vq, uint32_t desc)
+{
+intptr_t i, j, opr_sz = simd_oprsz(desc);
+int idx = simd_data(desc);
+int32_t *d = vd, *n = vn, *m = (int32_t *)vm + H4(idx);
+
+for (i = 0; i < opr_sz / 4; i += 16 / 4) {
+int32_t mm = m[i];
+for (j = 0; j < 16 / 4; ++j) {
+d[i + j] = do_sqrdmlah_s(n[i + j], mm, 0, false, true, vq);
+}
+}
+clear_tail(d, opr_sz, simd_maxsz(desc));
+}
Missing H macros in these helpers ?
No. The only index that's not identical across the vector is M, and H is handled once at
the beginning. Otherwise n[] and d[] have the same operation applied to all indicies, and
it doesn't matter which order in which these happen.
r~
Re: [PATCH 3/3] backends/hostmem: Round up memory size for qemu_madvise() and mbind()
On 5/28/24 09:15, Michal Privoznik wrote: +sz = ROUND_UP(sz, qemu_ram_pagesize(backend->mr.ram_block)); Second argument is evaluated twice. You probably don't want that to be a function call. r~
Re: [PATCH 1/5] sysemu: add set_virtual_time to accel ops
Pierrick Bouvier writes: > From: Alex Bennée > > We are about to remove direct calls to individual accelerators for > this information and will need a central point for plugins to hook > into time changes. > > From: Alex Bennée > Signed-off-by: Alex Bennée > Reviewed-by: Philippe Mathieu-Daudé Just a note, when patches written by other people come via your tree you should add your s-o-b tag to indicate: "I'm legally okay to contribute this and happy for it to go into QEMU" -- Alex Bennée Virtualisation Tech Lead @ Linaro
Re: [PULL 2/2] hw/ufs: Add support MCQ of UFSHCI 4.0
On 5/27/24 23:12, Jeuk Kim wrote:
From: Minwoo Im
This patch adds support for MCQ defined in UFSHCI 4.0. This patch
utilized the legacy I/O codes as much as possible to support MCQ.
MCQ operation & runtime register is placed at 0x1000 offset of UFSHCI
register statically with no spare space among four registers (48B):
UfsMcqSqReg, UfsMcqSqIntReg, UfsMcqCqReg, UfsMcqCqIntReg
The maxinum number of queue is 32 as per spec, and the default
MAC(Multiple Active Commands) are 32 in the device.
Example:
-device ufs,serial=foo,id=ufs0,mcq=true,mcq-maxq=8
Signed-off-by: Minwoo Im
Reviewed-by: Jeuk Kim
Message-Id: <20240528023106.856777-3-minwoo...@samsung.com>
Signed-off-by: Jeuk Kim
---
hw/ufs/trace-events | 17 ++
hw/ufs/ufs.c| 475 ++--
hw/ufs/ufs.h| 98 -
include/block/ufs.h | 23 ++-
4 files changed, 593 insertions(+), 20 deletions(-)
Fails build:
https://gitlab.com/qemu-project/qemu/-/jobs/6960270722
In file included from trace/trace-hw_ufs.c:5:
../hw/ufs/trace-events:28:24: error: format specifies type 'unsigned char' but the
argument has type 'uint32_t' (aka 'unsigned int') [-Werror,-Wformat]
, cqid, addr);
^~~~
../hw/ufs/trace-events:25:112: error: format specifies type 'unsigned char' but the
argument has type 'uint32_t' (aka 'unsigned int') [-Werror,-Wformat]
qemu_log("ufs_err_dma_write_cq " "failed to write cq entry. cqid %"PRIu8",
hwaddr %"PRIu64"" "\n", cqid, addr);
~~~
^~~~
2 errors generated.
r~
Re: [PATCH 1/3] osdep: Make qemu_madvise() to set errno in all cases
Am 28.05.24 um 18:15 schrieb Michal Privoznik:
The unspoken premise of qemu_madvise() is that errno is set on
error. And it is mostly the case except for posix_madvise() which
is documented to return either zero (on success) or a positive
error number. This means, we must set errno ourselves. And while
at it, make the function return a negative value on error, just
like other error paths do.
Signed-off-by: Michal Privoznik
---
util/osdep.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/util/osdep.c b/util/osdep.c
index e996c4744a..e42f4e8121 100644
--- a/util/osdep.c
+++ b/util/osdep.c
@@ -57,7 +57,12 @@ int qemu_madvise(void *addr, size_t len, int advice)
#if defined(CONFIG_MADVISE)
return madvise(addr, len, advice);
#elif defined(CONFIG_POSIX_MADVISE)
-return posix_madvise(addr, len, advice);
+int rc = posix_madvise(addr, len, advice);
+if (rc) {
+errno = rc;
+return -1;
+}
+return 0;
#else
errno = EINVAL;
return -1;
Interesting, seems to be correct
Reviewed-by: David Hildenbrand
--
Thanks,
David / dhildenb
Re: [PATCH 3/3] backends/hostmem: Round up memory size for qemu_madvise() and mbind()
Am 28.05.24 um 18:15 schrieb Michal Privoznik:
./build/qemu-system-x86_64 \ -m size=8389632k,slots=16,maxmem=2560k \
-object
'{"qom-type":"memory-backend-file","id":"ram-node0","mem-path":"hugepages2M","prealloc":true,"size":8590983168,"host-nodes":[0],"policy":"bind"}' \ -numa node,nodeid=0,cpus=0,memdev=ram-node0
For DIMMs and friends we now (again) enforce that the size must be aligned to
the page size:
commit 540a1abbf0b243e4cfb4333c5d30a041f7080ba4
Author: David Hildenbrand
Date: Wed Jan 17 14:55:54 2024 +0100
memory-device: reintroduce memory region size check
We used to check that the memory region size is multiples of the overall
requested address alignment for the device memory address.
We removed that check, because there are cases (i.e., hv-balloon) where
devices unconditionally request an address alignment that has a very large
alignment (i.e., 32 GiB), but the actual memory device size might not be
multiples of that alignment.
However, this change:
(a) allows for some practically impossible DIMM sizes, like "1GB+1 byte".
(b) allows for DIMMs that partially cover hugetlb pages, previously
reported in [1].
...
Partial hugetlb pages do not particularly make sense; wasting memory. Do we
expect people to actually ave working setup that we might break when disallowing
such configurations? Or would we actually help them identify that something
non-sensical is happening?
When using memory-backend-memfd, we already do get a proper error:
./build/qemu-system-x86_64 -m 2047m -object
memory-backend-memfd,id=ram-node0,hugetlb=on,size=2047m,reserve=off -numa
node,nodeid=0,cpus=0,memdev=ram-node0 -S
qemu-system-x86_64: failed to resize memfd to 2146435072: Invalid argument
So this only applies to memory-backend-file, where we maybe should fail in a
similar way?
--
Thanks,
David / dhildenb
Re: [PATCH V1 00/26] Live update: cpr-exec
On Tue, May 28, 2024 at 11:10:27AM -0400, Steven Sistare wrote: > On 5/27/2024 1:45 PM, Peter Xu wrote: > > On Tue, May 21, 2024 at 07:46:12AM -0400, Steven Sistare wrote: > > > I understand, thanks. If I can help with any of your todo list, > > > just ask - steve > > > > Thanks for offering the help, Steve. Started looking at this today, then I > > found that I miss something high-level. Let me ask here, and let me > > apologize already for starting to throw multiple questions.. > > > > IIUC the whole idea of this patchset is to allow efficient QEMU upgrade, in > > this case not host kernel but QEMU-only, and/or upper. > > > > Is there any justification on why the complexity is needed here? It looks > > to me this one is more involved than cpr-reboot, so I'm thinking how much > > we can get from the complexity, and whether it's worthwhile. 1000+ LOC is > > the min support, and if we even expect more to come, that's really > > important, IMHO. > > > > For example, what's the major motivation of this whole work? Is that more > > on performance, or is it more for supporting the special devices like VFIO > > which we used to not support, or something else? I can't find them in > > whatever cover letter I can find, including this one. > > > > Firstly, regarding performance, IMHO it'll be always nice to share even > > some very fundamental downtime measurement comparisons using the new exec > > mode v.s. the old migration ways to upgrade QEMU binary. Do you perhaps > > have some number on hand when you started working on this feature years > > ago? Or maybe some old links on the list would help too, as I didn't > > follow this work since the start. > > > > On VFIO, IIUC you started out this project without VFIO migration being > > there. Now we have VFIO migration so not sure how much it would work for > > the upgrade use case. Even with current VFIO migration, we may not want to > > migrate device states for a local upgrade I suppose, as that can be a lot > > depending on the type of device assigned. However it'll be nice to discuss > > this too if this is the major purpose of the series. > > > > I think one other challenge on QEMU upgrade with VFIO devices is that the > > dest QEMU won't be able to open the VFIO device when the src QEMU is still > > using it as the owner. IIUC this is a similar condition where QEMU wants > > to have proper ownership transfer of a shared block device, and AFAIR right > > now we resolved that issue using some form of file lock on the image file. > > In this case it won't easily apply to a VFIO dev fd, but maybe we still > > have other approaches, not sure whether you investigated any. E.g. could > > the VFIO handle be passed over using unix scm rights? I think this might > > remove one dependency of using exec which can cause quite some difference > > v.s. a generic migration (from which regard, cpr-reboot is still a pretty > > generic migration). > > > > You also mentioned vhost/tap, is that also a major goal of this series in > > the follow up patchsets? Is this a problem only because this solution will > > do exec? Can it work if either the exec()ed qemu or dst qemu create the > > vhost/tap fds when boot? > > > > Meanwhile, could you elaborate a bit on the implication on chardevs? From > > what I read in the doc update it looks like a major part of work in the > > future, but I don't yet understand the issue.. Is it also relevant to the > > exec() approach? > > > > In all cases, some of such discussion would be really appreciated. And if > > you used to consider other approaches to solve this problem it'll be great > > to mention how you chose this way. Considering this work contains too many > > things, it'll be nice if such discussion can start with the fundamentals, > > e.g. on why exec() is a must. > > The main goal of cpr-exec is providing a fast and reliable way to update > qemu. cpr-reboot is not fast enough or general enough. It requires the > guest to support suspend and resume for all devices, and that takes seconds. > If one actually reboots the host, that adds more seconds, depending on > system services. cpr-exec takes 0.1 secs, and works every time, unlike > like migration which can fail to converge on a busy system. Live migration > also consumes more system and network resources. Right, but note that when I was thinking of a comparison between cpr-exec v.s. normal migration, I didn't mean a "normal live migration". I think it's more of the case whether exec() can be avoided. I had a feeling that this exec() will cause a major part of work elsewhere but maybe I am wrong as I didn't see the whole branch. AFAIU, "cpr-exec takes 0.1 secs" is a conditional result. I think it at least should be relevant to what devices are attached to the VM, right? E.g., I observed at least two things that can drastically enlarge the blackout window: 1) vcpu save/load sometimes can take ridiculously long time, even if 99% of them are fi
Re: [PATCH 2/4] gdbstub: Add support for MTE in user mode
Gustavo Romero writes: > This commit implements the stubs to handle the qIsAddressTagged, > qMemTag, and QMemTag GDB packets, allowing all GDB 'memory-tag' > subcommands to work with QEMU gdbstub on aarch64 user mode. It also > implements the get/set function for the special GDB MTE register > 'tag_ctl', used to control the MTE fault type at runtime. > > Signed-off-by: Gustavo Romero > --- > configs/targets/aarch64-linux-user.mak | 2 +- > target/arm/cpu.c | 1 + > target/arm/gdbstub.c | 321 + > target/arm/internals.h | 2 + > 4 files changed, 325 insertions(+), 1 deletion(-) > > diff --git a/configs/targets/aarch64-linux-user.mak > b/configs/targets/aarch64-linux-user.mak > index ba8bc5fe3f..8f0ed21d76 100644 > --- a/configs/targets/aarch64-linux-user.mak > +++ b/configs/targets/aarch64-linux-user.mak > @@ -1,6 +1,6 @@ > TARGET_ARCH=aarch64 > TARGET_BASE_ARCH=arm > -TARGET_XML_FILES= gdb-xml/aarch64-core.xml gdb-xml/aarch64-fpu.xml > gdb-xml/aarch64-pauth.xml > +TARGET_XML_FILES= gdb-xml/aarch64-core.xml gdb-xml/aarch64-fpu.xml > gdb-xml/aarch64-pauth.xml gdb-xml/aarch64-mte.xml Ahh there it is, this got missed from the commit -- Alex Bennée Virtualisation Tech Lead @ Linaro
Re: [PATCH 1/4] gdbstub: Add support for target-specific stubs
Gustavo Romero writes:
> Currently, it's not possible to have stubs specific to a given target,
> even though there are GDB features which are target-specific, like, for
> instance, memory tagging.
>
> This commit introduces set_query_supported_arch,
> set_gdb_gen_query_table_arch, and set_gdb_gen_set_table_arch functions
> as interfaces to extend the qSupported string, the query handler table,
> and set handler table per target, so allowing target-specific stub
> implementation.
Subsystem functions exposed to the wider QEMU should be prefixed by the
subsystem name (e.g. gdb_*).
> Besides that, it moves GdbCmdParseEntry struct, its related types, and
> gdb_put_packet to include/exec/gdbstub.h so they are also available in
> the target-specific stubs.
Generally I'm trying to reduce the amount of stuff that gets dumped in
here because it is included widely and if your not careful you start
winding yourself into knots.
Could we put the command specific stuff into include/gdbstub/commands.h
and only include it when needed?
In general I think we could make the series a little more granular:
- move GdbCmdParseEntry into headers
- clean-up process_string_cmd
- introduce new gdb_ APIs
That will reduce the size of the individual patches and make review a
bit easier.
>
> Signed-off-by: Gustavo Romero
> ---
> gdbstub/gdbstub.c | 108 +++--
> gdbstub/internals.h| 22 -
> gdbstub/syscalls.c | 1 +
> include/exec/gdbstub.h | 86 +++-
> 4 files changed, 147 insertions(+), 70 deletions(-)
>
> diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
> index b3574997ea..4996530fde 100644
> --- a/gdbstub/gdbstub.c
> +++ b/gdbstub/gdbstub.c
> @@ -920,43 +920,6 @@ static int cmd_parse_params(const char *data, const char
> *schema,
> return 0;
> }
>
> -typedef void (*GdbCmdHandler)(GArray *params, void *user_ctx);
> -
> -/*
> - * cmd_startswith -> cmd is compared using startswith
> - *
> - * allow_stop_reply -> true iff the gdbstub can respond to this command with
> a
> - * "stop reply" packet. The list of commands that accept such response is
> - * defined at the GDB Remote Serial Protocol documentation. see:
> - *
> https://sourceware.org/gdb/onlinedocs/gdb/Stop-Reply-Packets.html#Stop-Reply-Packets.
> - *
> - * schema definitions:
> - * Each schema parameter entry consists of 2 chars,
> - * the first char represents the parameter type handling
> - * the second char represents the delimiter for the next parameter
> - *
> - * Currently supported schema types:
> - * 'l' -> unsigned long (stored in .val_ul)
> - * 'L' -> unsigned long long (stored in .val_ull)
> - * 's' -> string (stored in .data)
> - * 'o' -> single char (stored in .opcode)
> - * 't' -> thread id (stored in .thread_id)
> - * '?' -> skip according to delimiter
> - *
> - * Currently supported delimiters:
> - * '?' -> Stop at any delimiter (",;:=\0")
> - * '0' -> Stop at "\0"
> - * '.' -> Skip 1 char unless reached "\0"
> - * Any other value is treated as the delimiter value itself
> - */
> -typedef struct GdbCmdParseEntry {
> -GdbCmdHandler handler;
> -const char *cmd;
> -bool cmd_startswith;
> -const char *schema;
> -bool allow_stop_reply;
> -} GdbCmdParseEntry;
> -
> static inline int startswith(const char *string, const char *pattern)
> {
>return !strncmp(string, pattern, strlen(pattern));
> @@ -1645,6 +1608,13 @@ static void handle_query_thread_extra(GArray *params,
> void *user_ctx)
> gdb_put_strbuf();
> }
>
> +/* Arch-specific qSupported */
> +char *query_supported_arch = NULL;
This should be static.
> +void set_query_supported_arch(char *query_supported)
> +{
> +query_supported_arch = query_supported;
> +}
> +
Maybe gdb_extended_qsupported_features?
> static void handle_query_supported(GArray *params, void *user_ctx)
> {
> CPUClass *cc;
> @@ -1684,6 +1654,11 @@ static void handle_query_supported(GArray *params,
> void *user_ctx)
> }
>
> g_string_append(gdbserver_state.str_buf,
> ";vContSupported+;multiprocess+");
> +
> +if (query_supported_arch) {
> +g_string_append(gdbserver_state.str_buf, query_supported_arch);
> +}
> +
> gdb_put_strbuf();
> }
>
> @@ -1765,6 +1740,16 @@ static const GdbCmdParseEntry
> gdb_gen_query_set_common_table[] = {
> },
> };
>
> +
> +/* Arch-specific query table */
> +static GdbCmdParseEntry *gdb_gen_query_table_arch = NULL ;
> +static int gdb_gen_query_table_arch_size = 0;
For local statics you don't need prefixes if this saves on variables
getting too large. Also you don't need init to NULL/0 for statics.
> +void set_gdb_gen_query_table_arch(GdbCmdParseEntry *table, int size)
> +{
> +gdb_gen_query_table_arch = table;
> +gdb_gen_query_table_arch_size = size;
> +}
> +
> static const GdbCmdParseEntry gdb_gen_query_table[] = {
> {
> .handler = handle_query_curr_tid,
> @@ -1857,6 +1842,15 @@ static
Re: hw/usb/hcd-ohci: Fix #1510, #303: pid not IN or OUT
On Tue, 28 May 2024 at 16:37, Cord Amfmgm wrote: > > > > On Tue, May 28, 2024 at 9:03 AM Peter Maydell > wrote: >> >> On Mon, 20 May 2024 at 23:24, Cord Amfmgm wrote: >> > On Mon, May 20, 2024 at 12:05 PM Peter Maydell >> > wrote: >> >> For the "zero length buffer" case, do you have a more detailed >> >> pointer to the bit of the spec that says that "cbp = be + 1" is a >> >> valid way to specify a zero length buffer? Table 4-2 in the copy I >> >> have says for CurrentBufferPointer "a value of 0 indicates >> >> a zero-length data packet or that all bytes have been transferred", >> >> and the sample host OS driver function QueueGeneralRequest() >> >> later in the spec handles a 0 length packet by setting >> >> TD->HcTD.CBP = TD->HcTD.BE = 0; >> >> (which our emulation's code does handle). >> > >> > >> > Reading the spec carefully, a CBP set to 0 should always mean the >> > zero-length buffer case (or that all bytes have been transferred, so the >> > buffer is now zero-length - the same thing). >> >> Yeah, I can see the argument for "we should treat all cbp == 0 as >> zero-length buffer, not just cbp == be == 0". >> >> > Table 4-2 is the correct reference, and this part is clear. It's the part >> > you quoted. "Contains the physical address of the next memory location >> > that will be accessed for transfer to/from the endpoint. A value of 0 >> > indicates a zero-length data packet or that all bytes have been >> > transferred." >> > >> > Table 4-2 has this additional nugget that may be confusingly worded, for >> > BufferEnd: "Contains physical address of the last byte in the buffer for >> > this TD" >> >> Mmm, but for a zero length buffer there is no "last byte" to >> have this be the physical address of. >> >> > And here's an example buffer of length 0 -- you probably already know what >> > I'm going to do here: >> > >> > char buf[0]; >> > char * CurrentBufferPointer = &buf[0]; >> > char * BufferEnd = &buf[-1]; // "address of the last byte in the buffer" >> > // The OHCI Host Controller than advances CurrentBufferPointer like this: >> > CurrentBufferPointer += 0 >> > // After the transfer: >> > // CurrentBufferPointer = &buf[0]; >> > // BufferEnd = &buf[-1]; >> >> Right, but why do you think this is valid, rather than >> being a guest software bug? My reading of the spec is that it's >> pretty clear about how to say "zero length buffer", and this >> isn't it. >> >> Is there some real-world guest OS that programs the OHCI >> controller this way that we're trying to accommodate? > > > qemu versions 4.2 and before allowed this behavior. So? That might just mean we had a bug and we fixed it. 4.2 is a very old version of QEMU and nobody seems to have complained in the four years since we released 5.0 about this, which suggests that generally guest OS drivers don't try to send zero-length buffers in this way. > I don't think it's valid to ask for a *popular* guest OS as a > proof-of-concept because I'm not an expert on those. I didn't ask for "popular"; I asked for "real-world". What is the actual guest code you're running that falls over because of the behaviour change? More generally, why do you want this behaviour to be changed? Reasonable reasons might include: * we're out of spec based on reading the documentation * you're trying to run some old Windows VM/QNX/etc image, and it doesn't work any more * all the real hardware we tested behaves this way But don't necessarily include: * something somebody wrote and only tested on QEMU happens to assume the old behaviour rather than following the hw spec QEMU occasionally works around guest OS bugs, but only as when we really have to. It's usually better to fix the bug in the guest. thanks -- PMM
Re: [PATCH v2 00/67] target/arm: Convert a64 advsimd to decodetree (part 1)
On Sat, 25 May 2024 at 00:22, Richard Henderson wrote: > > In the process, convert more code to gvec as well -- I will need > the gvec code for implementing SME2. I guess this is about 1/3 > of the job done, but there's no reason to wait until the patch > set is completely unwieldy. > > Changes for v2: > * Fix existing RISU failures vs neoverse-n1. > * Introduce vfp_load_reg16, fixing a regression wrt VNEG (scalar, hp). > * Fix typo in SUQADD vectorization. > * Two more conversions. I've now finished review for this. I pulled 2 and 4-36 into my target-arm pullreq currently on list. To save you having to go through a lot of replies that are just me giving r-by tags, the patches I had comments/questions on are: 3, 38, 44, 46, 50, 65, 66. thanks -- PMM
Re: [PATCH v2 67/67] target/arm: Convert FCSEL to decodetree
On Sat, 25 May 2024 at 00:32, Richard Henderson wrote: > > Signed-off-by: Richard Henderson > --- Reviewed-by: Peter Maydell thanks -- PMM
[PATCH 1/3] osdep: Make qemu_madvise() to set errno in all cases
The unspoken premise of qemu_madvise() is that errno is set on
error. And it is mostly the case except for posix_madvise() which
is documented to return either zero (on success) or a positive
error number. This means, we must set errno ourselves. And while
at it, make the function return a negative value on error, just
like other error paths do.
Signed-off-by: Michal Privoznik
---
util/osdep.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/util/osdep.c b/util/osdep.c
index e996c4744a..e42f4e8121 100644
--- a/util/osdep.c
+++ b/util/osdep.c
@@ -57,7 +57,12 @@ int qemu_madvise(void *addr, size_t len, int advice)
#if defined(CONFIG_MADVISE)
return madvise(addr, len, advice);
#elif defined(CONFIG_POSIX_MADVISE)
-return posix_madvise(addr, len, advice);
+int rc = posix_madvise(addr, len, advice);
+if (rc) {
+errno = rc;
+return -1;
+}
+return 0;
#else
errno = EINVAL;
return -1;
--
2.44.1
Re: [PATCH v2 66/67] target/arm: Convert FMADD, FMSUB, FNMADD, FNMSUB to decodetree
On Sat, 25 May 2024 at 00:27, Richard Henderson
wrote:
>
> These are the only instructions in the 3 source scalar class.
>
> Signed-off-by: Richard Henderson
> ---
> target/arm/tcg/a64.decode | 10 ++
> target/arm/tcg/translate-a64.c | 233 -
> 2 files changed, 93 insertions(+), 150 deletions(-)
>
> static void disas_data_proc_fp(DisasContext *s, uint32_t insn)
> {
> -if (extract32(insn, 24, 1)) {
> -/* Floating point data-processing (3 source) */
> -disas_fp_3src(s, insn);
> -} else if (extract32(insn, 21, 1) == 0) {
> +if (extract32(insn, 21, 1) == 0) {
> /* Floating point to fixed point conversions */
> disas_fp_fixed_conv(s, insn);
> } else {
Doesn't this result in the unallocated-encodings in the
fp-3src class now falling into the "else" clause and
being mis-decoded as the wrong thing? I think this
needs to be
if (extract32(insn, 24, 1)) {
unallocated_encoding();
} else if (extract32(insn, 21, 1) == 0) {
[etc]
thanks
-- PMM
[PATCH 0/3] backends/hostmem: Round up memory size for qemu_madvise() and mbind()
This is a resurrection of the following old patch of mine: https://lists.gnu.org/archive/html/qemu-devel/2023-11/msg05536.html You could say it's a v2 and here's what changed since v1: - qemu_madvise() is fixed in case posix_madvise() is used - Warnings are reported in case of qemu_madvise() failure - Size is rounded up in case of live change of corresponding attributes too Michal Privoznik (3): osdep: Make qemu_madvise() to set errno in all cases backends/hostmem: Warn on qemu_madvise() failures backends/hostmem: Round up memory size for qemu_madvise() and mbind() backends/hostmem.c | 38 ++ util/osdep.c | 7 ++- 2 files changed, 36 insertions(+), 9 deletions(-) -- 2.44.1
[PATCH 2/3] backends/hostmem: Warn on qemu_madvise() failures
If user sets .merge or .dump attributes qemu_madvise() is called
with corresponding advice. But it is never checked for failure
which may mislead users into thinking the attribute is set
correctly.I believe at this point it's too late to report errors
in that case but let's report a warning at least.
Signed-off-by: Michal Privoznik
---
backends/hostmem.c | 27 +++
1 file changed, 19 insertions(+), 8 deletions(-)
diff --git a/backends/hostmem.c b/backends/hostmem.c
index eb9682b4a8..1a6fd1c714 100644
--- a/backends/hostmem.c
+++ b/backends/hostmem.c
@@ -14,6 +14,7 @@
#include "sysemu/hostmem.h"
#include "hw/boards.h"
#include "qapi/error.h"
+#include "qemu/error-report.h"
#include "qapi/qapi-builtin-visit.h"
#include "qapi/visitor.h"
#include "qemu/config-file.h"
@@ -178,8 +179,11 @@ static void host_memory_backend_set_merge(Object *obj,
bool value, Error **errp)
void *ptr = memory_region_get_ram_ptr(&backend->mr);
uint64_t sz = memory_region_size(&backend->mr);
-qemu_madvise(ptr, sz,
- value ? QEMU_MADV_MERGEABLE : QEMU_MADV_UNMERGEABLE);
+if (qemu_madvise(ptr, sz,
+ value ? QEMU_MADV_MERGEABLE : QEMU_MADV_UNMERGEABLE))
{
+warn_report("Couldn't change property 'merge' on '%s': %s",
+object_get_typename(obj), strerror(errno));
+}
backend->merge = value;
}
}
@@ -204,8 +208,11 @@ static void host_memory_backend_set_dump(Object *obj, bool
value, Error **errp)
void *ptr = memory_region_get_ram_ptr(&backend->mr);
uint64_t sz = memory_region_size(&backend->mr);
-qemu_madvise(ptr, sz,
- value ? QEMU_MADV_DODUMP : QEMU_MADV_DONTDUMP);
+if (qemu_madvise(ptr, sz,
+ value ? QEMU_MADV_DODUMP : QEMU_MADV_DONTDUMP)) {
+warn_report("Couldn't change property 'dump' on '%s': %s",
+object_get_typename(obj), strerror(errno));
+}
backend->dump = value;
}
}
@@ -337,11 +344,15 @@ host_memory_backend_memory_complete(UserCreatable *uc,
Error **errp)
ptr = memory_region_get_ram_ptr(&backend->mr);
sz = memory_region_size(&backend->mr);
-if (backend->merge) {
-qemu_madvise(ptr, sz, QEMU_MADV_MERGEABLE);
+if (backend->merge &&
+qemu_madvise(ptr, sz, QEMU_MADV_MERGEABLE)) {
+warn_report("Couldn't set property 'merge' on '%s': %s",
+object_get_typename(OBJECT(uc)), strerror(errno));
}
-if (!backend->dump) {
-qemu_madvise(ptr, sz, QEMU_MADV_DONTDUMP);
+if (!backend->dump &&
+qemu_madvise(ptr, sz, QEMU_MADV_DONTDUMP)) {
+warn_report("Couldn't set property 'dump' on '%s': %s",
+object_get_typename(OBJECT(uc)), strerror(errno));
}
#ifdef CONFIG_NUMA
unsigned long lastbit = find_last_bit(backend->host_nodes, MAX_NODES);
--
2.44.1
[PATCH 3/3] backends/hostmem: Round up memory size for qemu_madvise() and mbind()
Simple reproducer:
qemu.git $ ./build/qemu-system-x86_64 \
-m size=8389632k,slots=16,maxmem=2560k \
-object
'{"qom-type":"memory-backend-file","id":"ram-node0","mem-path":"/hugepages2M/","prealloc":true,"size":8590983168,"host-nodes":[0],"policy":"bind"}'
\
-numa node,nodeid=0,cpus=0,memdev=ram-node0
With current master I get:
qemu-system-x86_64: cannot bind memory to host NUMA nodes: Invalid argument
The problem is that memory size (8193MiB) is not an integer
multiple of underlying pagesize (2MiB) which triggers a check
inside of mbind(), since we can't really set policy just to a
fraction of a page. As qemu_madvise() has the same expectation,
round size passed to underlying pagesize.
Signed-off-by: Michal Privoznik
---
backends/hostmem.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/backends/hostmem.c b/backends/hostmem.c
index 1a6fd1c714..9b727699f6 100644
--- a/backends/hostmem.c
+++ b/backends/hostmem.c
@@ -179,6 +179,8 @@ static void host_memory_backend_set_merge(Object *obj, bool
value, Error **errp)
void *ptr = memory_region_get_ram_ptr(&backend->mr);
uint64_t sz = memory_region_size(&backend->mr);
+sz = ROUND_UP(sz, qemu_ram_pagesize(backend->mr.ram_block));
+
if (qemu_madvise(ptr, sz,
value ? QEMU_MADV_MERGEABLE : QEMU_MADV_UNMERGEABLE))
{
warn_report("Couldn't change property 'merge' on '%s': %s",
@@ -208,6 +210,8 @@ static void host_memory_backend_set_dump(Object *obj, bool
value, Error **errp)
void *ptr = memory_region_get_ram_ptr(&backend->mr);
uint64_t sz = memory_region_size(&backend->mr);
+sz = ROUND_UP(sz, qemu_ram_pagesize(backend->mr.ram_block));
+
if (qemu_madvise(ptr, sz,
value ? QEMU_MADV_DODUMP : QEMU_MADV_DONTDUMP)) {
warn_report("Couldn't change property 'dump' on '%s': %s",
@@ -344,6 +348,13 @@ host_memory_backend_memory_complete(UserCreatable *uc,
Error **errp)
ptr = memory_region_get_ram_ptr(&backend->mr);
sz = memory_region_size(&backend->mr);
+/*
+ * Round up size to be an integer multiple of pagesize, because
+ * both madvise() and mbind() does not really like setting
+ * advice/policy to just a fraction of a page.
+ */
+sz = ROUND_UP(sz, qemu_ram_pagesize(backend->mr.ram_block));
+
if (backend->merge &&
qemu_madvise(ptr, sz, QEMU_MADV_MERGEABLE)) {
warn_report("Couldn't set property 'merge' on '%s': %s",
--
2.44.1
Re: [PATCH v2 65/67] target/arm: Convert SQDMULH, SQRDMULH to decodetree
On Sat, 25 May 2024 at 00:27, Richard Henderson
wrote:
>
> These are the last instructions within disas_simd_three_reg_same
> and disas_simd_scalar_three_reg_same, so remove them.
>
> Signed-off-by: Richard Henderson
> diff --git a/target/arm/tcg/vec_helper.c b/target/arm/tcg/vec_helper.c
> index d8e96386be..b05922b425 100644
> --- a/target/arm/tcg/vec_helper.c
> +++ b/target/arm/tcg/vec_helper.c
> @@ -311,6 +311,38 @@ void HELPER(neon_sqrdmulh_h)(void *vd, void *vn, void
> *vm,
> clear_tail(d, opr_sz, simd_maxsz(desc));
> }
>
> +void HELPER(neon_sqdmulh_idx_h)(void *vd, void *vn, void *vm,
> +void *vq, uint32_t desc)
> +{
> +intptr_t i, j, opr_sz = simd_oprsz(desc);
> +int idx = simd_data(desc);
> +int16_t *d = vd, *n = vn, *m = (int16_t *)vm + H2(idx);
> +
> +for (i = 0; i < opr_sz / 2; i += 16 / 2) {
> +int16_t mm = m[i];
> +for (j = 0; j < 16 / 2; ++j) {
> +d[i + j] = do_sqrdmlah_h(n[i + j], mm, 0, false, false, vq);
> +}
> +}
> +clear_tail(d, opr_sz, simd_maxsz(desc));
> +}
> +
> +void HELPER(neon_sqrdmulh_idx_h)(void *vd, void *vn, void *vm,
> + void *vq, uint32_t desc)
> +{
> +intptr_t i, j, opr_sz = simd_oprsz(desc);
> +int idx = simd_data(desc);
> +int16_t *d = vd, *n = vn, *m = (int16_t *)vm + H2(idx);
> +
> +for (i = 0; i < opr_sz / 2; i += 16 / 2) {
> +int16_t mm = m[i];
> +for (j = 0; j < 16 / 2; ++j) {
> +d[i + j] = do_sqrdmlah_h(n[i + j], mm, 0, false, true, vq);
> +}
> +}
> +clear_tail(d, opr_sz, simd_maxsz(desc));
> +}
> +
> void HELPER(sve2_sqrdmlah_h)(void *vd, void *vn, void *vm,
> void *va, uint32_t desc)
> {
> @@ -474,6 +506,38 @@ void HELPER(neon_sqrdmulh_s)(void *vd, void *vn, void
> *vm,
> clear_tail(d, opr_sz, simd_maxsz(desc));
> }
>
> +void HELPER(neon_sqdmulh_idx_s)(void *vd, void *vn, void *vm,
> +void *vq, uint32_t desc)
> +{
> +intptr_t i, j, opr_sz = simd_oprsz(desc);
> +int idx = simd_data(desc);
> +int32_t *d = vd, *n = vn, *m = (int32_t *)vm + H4(idx);
> +
> +for (i = 0; i < opr_sz / 4; i += 16 / 4) {
> +int32_t mm = m[i];
> +for (j = 0; j < 16 / 4; ++j) {
> +d[i + j] = do_sqrdmlah_s(n[i + j], mm, 0, false, false, vq);
> +}
> +}
> +clear_tail(d, opr_sz, simd_maxsz(desc));
> +}
> +
> +void HELPER(neon_sqrdmulh_idx_s)(void *vd, void *vn, void *vm,
> + void *vq, uint32_t desc)
> +{
> +intptr_t i, j, opr_sz = simd_oprsz(desc);
> +int idx = simd_data(desc);
> +int32_t *d = vd, *n = vn, *m = (int32_t *)vm + H4(idx);
> +
> +for (i = 0; i < opr_sz / 4; i += 16 / 4) {
> +int32_t mm = m[i];
> +for (j = 0; j < 16 / 4; ++j) {
> +d[i + j] = do_sqrdmlah_s(n[i + j], mm, 0, false, true, vq);
> +}
> +}
> +clear_tail(d, opr_sz, simd_maxsz(desc));
> +}
Missing H macros in these helpers ?
thanks
-- PMM
Re: [PATCH v5] linux-aio: add IO_CMD_FDSYNC command support
Am 25.04.2024 um 09:04 hat Prasad Pandit geschrieben: > From: Prasad Pandit > > Libaio defines IO_CMD_FDSYNC command to sync all outstanding > asynchronous I/O operations, by flushing out file data to the > disk storage. Enable linux-aio to submit such aio request. > > When using aio=native without fdsync() support, QEMU creates > pthreads, and destroying these pthreads results in TLB flushes. > In a real-time guest environment, TLB flushes cause a latency > spike. This patch helps to avoid such spikes. > > Reviewed-by: Stefan Hajnoczi > Signed-off-by: Prasad Pandit Thanks, applied to the block branch. Kevin
Re: [PATCH v2 64/67] target/arm: Tidy SQDMULH, SQRDMULH (vector)
On Sat, 25 May 2024 at 00:30, Richard Henderson wrote: > > We already have a gvec helper for the operations, but we aren't > using it on the aa32 neon side. Create a unified expander for > use by both aa32 and aa64 translators. > > Signed-off-by: Richard Henderson > --- Reviewed-by: Peter Maydell thanks -- PMM
Re: [PATCH v2 63/67] target/arm: Convert MLA, MLS to decodetree
On Sat, 25 May 2024 at 00:27, Richard Henderson wrote: > > Signed-off-by: Richard Henderson > --- > target/arm/tcg/a64.decode | 8 > target/arm/tcg/translate-a64.c | 77 ++ > 2 files changed, 31 insertions(+), 54 deletions(-) > Reviewed-by: Peter Maydell thanks -- PMM
Re: [PATCH 0/4] Add MTE stubs for aarch64 user mode
Gustavo Romero writes: > This patchset adds the stubs necessary to support GDB memory tagging > commands on QEMU aarch64 user mode. On application I'm getting the following failure on configure which makes me think something is missing: Program scripts/undefsym.py found: YES (/home/alex/lsrc/qemu.git/builds/all/pyvenv/bin/python3 /home/alex/lsrc/qemu.git/scripts/undefsym.py) Program scripts/feature_to_c.py found: YES (/home/alex/lsrc/qemu.git/builds/all/pyvenv/bin/python3 /home/alex/lsrc/qemu.git/scripts/feature_to_c.py) ../../meson.build:3851:4: ERROR: File gdb-xml/aarch64-mte.xml does not exist. A full log can be found at /home/alex/lsrc/qemu.git/builds/all/meson-logs/meson-log.txt ninja: error: rebuilding 'build.ninja': subcommand failed FAILED: build.ninja /home/alex/lsrc/qemu.git/builds/all/pyvenv/bin/meson --internal regenerate /home/alex/lsrc/qemu.git /home/alex/lsrc/qemu.git/builds/all make: *** [Makefile:167: run-ninja] Error 1 Compilation exited abnormally with code 2 at Tue May 28 16:59:05 -- Alex Bennée Virtualisation Tech Lead @ Linaro
Re: [PATCH v2 62/67] target/arm: Convert MUL, PMUL to decodetree
On Sat, 25 May 2024 at 00:26, Richard Henderson wrote: > > Signed-off-by: Richard Henderson > --- > target/arm/tcg/a64.decode | 5 > target/arm/tcg/translate-a64.c | 51 +- > 2 files changed, 25 insertions(+), 31 deletions(-) Reviewed-by: Peter Maydell thanks -- PMM
Re: [PATCH] block/copy-before-write: use uint64_t for timeout in nanoseconds
Am 29.04.2024 um 16:19 hat Fiona Ebner geschrieben:
> rather than the uint32_t for which the maximum is slightly more than 4
> seconds and larger values would overflow. The QAPI interface allows
> specifying the number of seconds, so only values 0 to 4 are safe right
> now, other values lead to a much lower timeout than a user expects.
>
> The block_copy() call where this is used already takes a uint64_t for
> the timeout, so no change required there.
>
> Fixes: 6db7fd1ca9 ("block/copy-before-write: implement cbw-timeout option")
> Reported-by: Friedrich Weber
> Signed-off-by: Fiona Ebner
Thanks, applied to the block branch.
But I don't think our job is done yet with this. Increasing the limit is
good and useful, but even if it's now unlikely to hit with sane values,
we should still catch integer overflows in cbw_open() and return an
error on too big values instead of silently wrapping around.
Kevin
Re: [PATCH] tests/qtest/migrate-test: Use regular file file for shared-memory tests
On Tue, May 28, 2024 at 09:35:22AM -0400, Peter Xu wrote: > On Tue, May 28, 2024 at 02:27:57PM +1000, Nicholas Piggin wrote: > > There is no need to use /dev/shm for file-backed memory devices, and > > it is too small to be usable in gitlab CI. Switch to using a regular > > file in /tmp/ which will usually have more space available. > > > > Signed-off-by: Nicholas Piggin > > --- > > Am I missing something? AFAIKS there is not even any point using > > /dev/shm aka tmpfs anyway, there is not much special about it as a > > filesystem. This applies on top of the series just sent, and passes > > gitlab CI qtests including aarch64. > > I think it's just that /dev/shm guarantees shmem usage, while the var > "tmpfs" implies g_dir_make_tmp() which may be another non-ram based file > system, while that'll be slightly different comparing to what a real user > would use - we don't suggest user to put guest RAM on things like btrfs. > > One real implication is if we add a postcopy test it'll fail with > g_dir_make_tmp() when it is not pointing to a shmem mount, as > UFFDIO_REGISTER will fail there. But that test doesn't yet exist as the > QEMU paths should be the same even if Linux will trigger different paths > when different types of mem is used (anonymous v.s. shmem). > > If the goal here is to properly handle the case where tmpfs doesn't have > enough space, how about what I suggested in the other email? > > https://lore.kernel.org/r/ZlSppKDE6wzjCF--@x1n > > IOW, try populate the shmem region before starting the guest, skip if > population failed. Would that work? Let me append some more info here.. I think madvise() isn't needed as fallocate() should do the population work already, afaiu, then it means we pass the shmem path to QEMU and QEMU should notice this memory-backend-file existed, open() directly. I quicked walk the QEMU memory code and so far it looks all applicable, so that QEMU should just start the guest with the pre-populated shmem page caches. There's one trick where qemu_ram_mmap() will map some extra pages, on x86 4k, and I don't yet know why we did that.. /* * Note: this always allocates at least one extra page of virtual address * space, even if size is already aligned. */ total = size + align; But that was only used in mmap_reserve() not mmap_activate(), so the real mmap() should still be exactly what we fallocate()ed. Thanks, -- Peter Xu
Re: [PATCH v2 61/67] target/arm: Convert SABA, SABD, UABA, UABD to decodetree
On Sat, 25 May 2024 at 00:29, Richard Henderson wrote: > > Signed-off-by: Richard Henderson > --- > target/arm/tcg/a64.decode | 4 > target/arm/tcg/translate-a64.c | 22 ++ > 2 files changed, 10 insertions(+), 16 deletions(-) Reviewed-by: Peter Maydell thanks -- PMM
Re: [PATCH v2 60/67] target/arm: Convert SMAX, SMIN, UMAX, UMIN to decodetree
On Sat, 25 May 2024 at 00:27, Richard Henderson wrote: > > Signed-off-by: Richard Henderson > --- > target/arm/tcg/a64.decode | 4 > target/arm/tcg/translate-a64.c | 22 ++ > 2 files changed, 10 insertions(+), 16 deletions(-) > Reviewed-by: Peter Maydell thanks -- PMM
Re: [PATCH v2 59/67] target/arm: Convert SRHADD, URHADD to decodetree
On Sat, 25 May 2024 at 00:31, Richard Henderson wrote: > > Signed-off-by: Richard Henderson > --- Reviewed-by: Peter Maydell thanks -- PMM
Re: [PATCH v2 58/67] target/arm: Convert SRHADD, URHADD to gvec
On Sat, 25 May 2024 at 00:27, Richard Henderson wrote: > > Signed-off-by: Richard Henderson > --- Reviewed-by: Peter Maydell thanks -- PMM
Re: [PATCH v2 57/67] target/arm: Convert SHSUB, UHSUB to decodetree
On Sat, 25 May 2024 at 00:27, Richard Henderson wrote: > > Signed-off-by: Richard Henderson > --- > target/arm/tcg/a64.decode | 2 ++ > target/arm/tcg/translate-a64.c | 11 +++ > 2 files changed, 5 insertions(+), 8 deletions(-) > Reviewed-by: Peter Maydell thanks -- PMM
Re: [PATCH v2 56/67] target/arm: Convert SHSUB, UHSUB to gvec
On Sat, 25 May 2024 at 00:29, Richard Henderson wrote: > > Signed-off-by: Richard Henderson > --- > target/arm/helper.h | 6 -- > target/arm/tcg/translate.h | 4 + > target/arm/tcg/gengvec.c| 144 > target/arm/tcg/neon_helper.c| 27 -- > target/arm/tcg/translate-a64.c | 17 ++-- > target/arm/tcg/translate-neon.c | 4 +- > 6 files changed, 157 insertions(+), 45 deletions(-) > Reviewed-by: Peter Maydell thanks -- PMM
Re: [PATCH v2 54/67] target/arm: Convert SHADD, UHADD to gvec
On Sat, 25 May 2024 at 00:26, Richard Henderson wrote: > > Signed-off-by: Richard Henderson > --- > target/arm/helper.h | 6 -- > target/arm/tcg/translate.h | 5 ++ > target/arm/tcg/gengvec.c| 144 > target/arm/tcg/neon_helper.c| 27 -- > target/arm/tcg/translate-a64.c | 17 ++-- > target/arm/tcg/translate-neon.c | 4 +- > 6 files changed, 158 insertions(+), 45 deletions(-) Reviewed-by: Peter Maydell thanks -- PMM
Re: [PATCH v2 55/67] target/arm: Convert SHADD, UHADD to decodetree
On Sat, 25 May 2024 at 00:27, Richard Henderson wrote: > > Signed-off-by: Richard Henderson > --- > target/arm/tcg/a64.decode | 2 ++ > target/arm/tcg/translate-a64.c | 11 +++ > 2 files changed, 5 insertions(+), 8 deletions(-) > Reviewed-by: Peter Maydell thanks -- PMM
Re: [PATCH v2 53/67] target/arm: Use TCG_COND_TSTNE in gen_cmtst_vec
On Sat, 25 May 2024 at 00:29, Richard Henderson wrote: > > Signed-off-by: Richard Henderson > --- > target/arm/tcg/gengvec.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) Reviewed-by: Peter Maydell thanks -- PMM
Re: [PATCH v2 52/67] target/arm: Use TCG_COND_TSTNE in gen_cmtst_{i32, i64}
On Sat, 25 May 2024 at 00:30, Richard Henderson wrote: > > Signed-off-by: Richard Henderson > --- > target/arm/tcg/gengvec.c | 6 ++ > 1 file changed, 2 insertions(+), 4 deletions(-) Reviewed-by: Peter Maydell thanks -- PMM
Re: [PATCH v2 51/67] target/arm: Convert CMGT, CMHI, CMGE, CMHS, CMTST, CMEQ to decodetree
On Sat, 25 May 2024 at 00:26, Richard Henderson wrote: > > Signed-off-by: Richard Henderson > --- > target/arm/tcg/a64.decode | 12 +++ > target/arm/tcg/translate-a64.c | 132 - > 2 files changed, 60 insertions(+), 84 deletions(-) Reviewed-by: Peter Maydell thanks -- PMM
Re: [PATCH] qemu-io: add cvtnum() error handling for zone commands
Am 07.05.2024 um 20:05 hat Stefan Hajnoczi geschrieben: > cvtnum() parses positive int64_t values and returns a negative errno on > failure. Print errors and return early when cvtnum() fails. > > While we're at it, also reject nr_zones values greater or equal to 2^32 > since they cannot be represented. > > Reported-by: Peter Maydell > Cc: Sam Li > Signed-off-by: Stefan Hajnoczi Thanks, applied to the block branch. Kevin
Re: [PATCH] scripts/update-linux-headers.sh: Remove temporary directory inbetween
On Mon, May 27, 2024 at 08:02:43AM +0200, Thomas Huth wrote: > We are reusing the same temporary directory for installing the headers > of all targets, so there could be stale files here when switching from > one target to another. Make sure to delete the folder before installing > a new set of target headers into it. > > Signed-off-by: Thomas Huth Reviewed-by: Michael S. Tsirkin who's merging this? > --- > scripts/update-linux-headers.sh | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/scripts/update-linux-headers.sh b/scripts/update-linux-headers.sh > index 8963c39189..fbf7e119bc 100755 > --- a/scripts/update-linux-headers.sh > +++ b/scripts/update-linux-headers.sh > @@ -112,6 +112,7 @@ for arch in $ARCHLIST; do > arch_var=ARCH > fi > > +rm -rf "$hdrdir" > make -C "$linux" O="$blddir" INSTALL_HDR_PATH="$hdrdir" $arch_var=$arch > headers_install > > rm -rf "$output/linux-headers/asm-$arch" > -- > 2.45.1
Re: [PATCH v2 50/67] target/arm: Convert ADD, SUB (vector) to decodetree
On Sat, 25 May 2024 at 00:29, Richard Henderson wrote: > > Signed-off-by: Richard Henderson > --- > target/arm/tcg/a64.decode | 6 ++ > target/arm/tcg/translate-a64.c | 34 +++--- > 2 files changed, 17 insertions(+), 23 deletions(-) > @@ -10958,6 +10956,11 @@ static void disas_simd_3same_int(DisasContext *s, > uint32_t insn) > > case 0x01: /* SQADD, UQADD */ > case 0x05: /* SQSUB, UQSUB */ > +case 0x08: /* SSHL, USHL */ > +case 0x09: /* SQSHL, UQSHL */ > +case 0x0a: /* SRSHL, URSHL */ > +case 0x0b: /* SQRSHL, UQRSHL */ > +case 0x10: /* ADD, SUB */ > unallocated_encoding(s); > return; > } > @@ -11044,14 +11040,6 @@ static void disas_simd_3same_int(DisasContext *s, > uint32_t insn) > vec_full_reg_offset(s, rm), > is_q ? 16 : 8, vec_full_reg_size(s)); > return; > - > -case 0x01: /* SQADD, UQADD */ > -case 0x05: /* SQSUB, UQSUB */ > -case 0x08: /* SSHL, USHL */ > -case 0x09: /* SQSHL, UQSHL */ > -case 0x0a: /* SRSHL, URSHL */ > -case 0x0b: /* SQRSHL, UQRSHL */ > -g_assert_not_reached(); > } Shouldn't the parts of these that aren't ADD,SUB have been in some previous patches rather than this one? Otherwise Reviewed-by: Peter Maydell thanks -- PMM
Re: [PATCH v2 48/67] target/arm: Convert SQRSHL and UQRSHL (register) to gvec
On Sat, 25 May 2024 at 00:27, Richard Henderson wrote: > > Signed-off-by: Richard Henderson > --- Reviewed-by: Peter Maydell thanks -- PMM
Re: [PATCH v2 49/67] target/arm: Convert SQRSHL, UQRSHL to decodetree
On Sat, 25 May 2024 at 00:27, Richard Henderson wrote: > > Signed-off-by: Richard Henderson > --- > target/arm/tcg/a64.decode | 4 +++ > target/arm/tcg/translate-a64.c | 48 -- > 2 files changed, 26 insertions(+), 26 deletions(-) Reviewed-by: Peter Maydell thanks -- PMM
Re: [PATCH-for-9.1 v2 2/3] migration: Remove RDMA protocol handling
On Tue, May 28, 2024 at 09:06:04AM +, Gonglei (Arei) wrote: > Hi Peter, > > > -Original Message- > > From: Peter Xu [mailto:pet...@redhat.com] > > Sent: Wednesday, May 22, 2024 6:15 AM > > To: Yu Zhang > > Cc: Michael Galaxy ; Jinpu Wang > > ; Elmar Gerdes ; > > zhengchuan ; Gonglei (Arei) > > ; Daniel P. Berrangé ; > > Markus Armbruster ; Zhijian Li (Fujitsu) > > ; qemu-devel@nongnu.org; Yuval Shaia > > ; Kevin Wolf ; Prasanna > > Kumar Kalever ; Cornelia Huck > > ; Michael Roth ; Prasanna > > Kumar Kalever ; Paolo Bonzini > > ; qemu-bl...@nongnu.org; de...@lists.libvirt.org; > > Hanna Reitz ; Michael S. Tsirkin ; > > Thomas Huth ; Eric Blake ; Song > > Gao ; Marc-André Lureau > > ; Alex Bennée ; > > Wainer dos Santos Moschetta ; Beraldo Leal > > ; Pannengyuan ; > > Xiexiangyou ; Fabiano Rosas > > Subject: Re: [PATCH-for-9.1 v2 2/3] migration: Remove RDMA protocol handling > > > > On Fri, May 17, 2024 at 03:01:59PM +0200, Yu Zhang wrote: > > > Hello Michael and Peter, > > > > Hi, > > > > > > > > Exactly, not so compelling, as I did it first only on servers widely > > > used for production in our data center. The network adapters are > > > > > > Ethernet controller: Broadcom Inc. and subsidiaries NetXtreme BCM5720 > > > 2-port Gigabit Ethernet PCIe > > > > Hmm... I definitely thinks Jinpu's Mellanox ConnectX-6 looks more > > reasonable. > > > > https://lore.kernel.org/qemu-devel/CAMGffEn-DKpMZ4tA71MJYdyemg0Zda15 > > wvaqk81vxtkzx-l...@mail.gmail.com/ > > > > Appreciate a lot for everyone helping on the testings. > > > > > InfiniBand controller: Mellanox Technologies MT27800 Family > > > [ConnectX-5] > > > > > > which doesn't meet our purpose. I can choose RDMA or TCP for VM > > > migration. RDMA traffic is through InfiniBand and TCP through Ethernet > > > on these two hosts. One is standby while the other is active. > > > > > > Now I'll try on a server with more recent Ethernet and InfiniBand > > > network adapters. One of them has: > > > BCM57414 NetXtreme-E 10Gb/25Gb RDMA Ethernet Controller (rev 01) > > > > > > The comparison between RDMA and TCP on the same NIC could make more > > sense. > > > > It looks to me NICs are powerful now, but again as I mentioned I don't > > think it's > > a reason we need to deprecate rdma, especially if QEMU's rdma migration has > > the chance to be refactored using rsocket. > > > > Is there anyone who started looking into that direction? Would it make > > sense > > we start some PoC now? > > > > My team has finished the PoC refactoring which works well. > > Progress: > 1. Implement io/channel-rdma.c, > 2. Add unit test tests/unit/test-io-channel-rdma.c and verifying it is > successful, > 3. Remove the original code from migration/rdma.c, > 4. Rewrite the rdma_start_outgoing_migration and > rdma_start_incoming_migration logic, > 5. Remove all rdma_xxx functions from migration/ram.c. (to prevent RDMA live > migration from polluting the core logic of live migration), > 6. The soft-RoCE implemented by software is used to test the RDMA live > migration. It's successful. > > We will be submit the patchset later. That's great news, thank you! -- Peter Xu
Re: [PATCH 0/2] Revert "monitor: use aio_co_reschedule_self()"
Am 06.05.2024 um 21:06 hat Stefan Hajnoczi geschrieben:
> This series fixes RHEL-34618 "qemu crash on Assertion `luringcb->co->ctx ==
> s->aio_context' failed when do block_resize on hotplug disk with
> aio=io_uring":
> https://issues.redhat.com/browse/RHEL-34618
>
> Kevin identified commit 1f25c172f837 ("monitor: use aio_co_reschedule_self()")
> as the root cause. There is a subtlety regarding how
> qemu_get_current_aio_context() returns qemu_aio_context even though we may be
> running in iohandler_ctx.
>
> Revert commit 1f25c172f837, it was just intended as a code cleanup.
>
> Stefan Hajnoczi (2):
> Revert "monitor: use aio_co_reschedule_self()"
> aio: warn about iohandler_ctx special casing
Thanks, applied to the block branch.
Kevin
Re: [PATCH v2 47/67] target/arm: Convert SQSHL, UQSHL to decodetree
On Sat, 25 May 2024 at 00:32, Richard Henderson wrote: > > Signed-off-by: Richard Henderson > --- > target/arm/tcg/a64.decode | 4 ++ > target/arm/tcg/translate-a64.c | 74 ++ > 2 files changed, 53 insertions(+), 25 deletions(-) > Reviewed-by: Peter Maydell thanks -- PMM
Re: [PATCH v2 46/67] target/arm: Convert SQSHL and UQSHL (register) to gvec
On Sat, 25 May 2024 at 00:28, Richard Henderson
wrote:
>
> Signed-off-by: Richard Henderson
> ---
> target/arm/helper.h | 8
> target/arm/tcg/translate.h | 4
> target/arm/tcg/neon-dp.decode | 10 ++---
> target/arm/tcg/gengvec.c| 24 ++
> target/arm/tcg/neon_helper.c| 36 +
> target/arm/tcg/translate-a64.c | 17 +++-
> target/arm/tcg/translate-neon.c | 6 ++
> 7 files changed, 83 insertions(+), 22 deletions(-)
> +#define NEON_GVEC_VOP2_ENV(name, vtype) \
> +void HELPER(name)(void *vd, void *vn, void *vm, void *venv, uint32_t desc) \
> +{ \
> +intptr_t i, opr_sz = simd_oprsz(desc); \
> +vtype *d = vd, *n = vn, *m = vm;\
> +CPUARMState *env = venv;\
> +for (i = 0; i < opr_sz / sizeof(vtype); i++) { \
> +NEON_FN(d[i], n[i], m[i]); \
> +} \
> +clear_tail(d, opr_sz, simd_maxsz(desc));\
> +}
> +
Same question about H macros as for patch 44.
-- PMM
Re: [RFC PATCH 2/4] tests/qtest/migration: Add a test that runs vmstate-static-checker
On Mon, May 27, 2024 at 07:52:28PM -0300, Fabiano Rosas wrote:
> Peter Xu writes:
>
> > On Thu, May 23, 2024 at 05:19:20PM -0300, Fabiano Rosas wrote:
> >> We have the vmstate-static-checker script that takes the output of:
> >> '$QEMU -M $machine -dump-vmstate' for two different QEMU versions and
> >> compares them to check for compatibility breakages. This is just too
> >> simple and useful for us to pass on it. Add a test that runs the
> >> script.
> >>
> >> Since this needs to use two different QEMU versions, the test is
> >> skipped if only one QEMU is provided. The infrastructure for passing
> >> more than one binary is already in place:
> >>
> >> $ PYTHON=$(which python3.11) \
> >> QTEST_QEMU_BINARY_SRC=../build-previous/qemu-system-x86_64 \
> >> QTEST_QEMU_BINARY=./qemu-system-x86_64 \
> >> ./tests/qtest/migration-test -p /x86_64/migration/vmstate-checker-script
> >>
> >> Signed-off-by: Fabiano Rosas
> >> ---
> >> some code duplication for now, just so we can reason about this
> >> without too much noise
> >> ---
> >> tests/qtest/migration-test.c | 82
> >> 1 file changed, 82 insertions(+)
> >>
> >> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> >> index e8d3555f56..2253e0fc5b 100644
> >> --- a/tests/qtest/migration-test.c
> >> +++ b/tests/qtest/migration-test.c
> >> @@ -63,6 +63,7 @@ static QTestMigrationState dst_state;
> >> #define DIRTYLIMIT_TOLERANCE_RANGE 25 /* MB/s */
> >>
> >> #define ANALYZE_SCRIPT "scripts/analyze-migration.py"
> >> +#define VMSTATE_CHECKER_SCRIPT "scripts/vmstate-static-checker.py"
> >>
> >> #define QEMU_VM_FILE_MAGIC 0x5145564d
> >> #define FILE_TEST_FILENAME "migfile"
> >> @@ -1611,6 +1612,85 @@ static void test_analyze_script(void)
> >> test_migrate_end(from, to, false);
> >> cleanup("migfile");
> >> }
> >> +
> >> +static void test_vmstate_checker_script(void)
> >> +{
> >> +g_autofree gchar *cmd_src = NULL;
> >> +g_autofree gchar *cmd_dst = NULL;
> >> +g_autofree gchar *vmstate_src = NULL;
> >> +g_autofree gchar *vmstate_dst = NULL;
> >> +const char *machine_alias, *machine_opts = "";
> >> +g_autofree char *machine = NULL;
> >> +const char *arch = qtest_get_arch();
> >> +int pid, wstatus;
> >> +const char *python = g_getenv("PYTHON");
> >> +
> >> +if (!getenv(QEMU_ENV_SRC) && !getenv(QEMU_ENV_DST)) {
> >> +g_test_skip("Test needs two different QEMU versions");
> >> +return;
> >> +}
> >> +
> >> +if (!python) {
> >> +g_test_skip("PYTHON variable not set");
> >> +return;
> >> +}
> >> +
> >> +if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
> >> +if (g_str_equal(arch, "i386")) {
> >> +machine_alias = "pc";
> >> +} else {
> >> +machine_alias = "q35";
> >> +}
> >> +} else if (g_str_equal(arch, "s390x")) {
> >> +machine_alias = "s390-ccw-virtio";
> >> +} else if (strcmp(arch, "ppc64") == 0) {
> >> +machine_alias = "pseries";
> >> +} else if (strcmp(arch, "aarch64") == 0) {
> >> +machine_alias = "virt";
> >> +} else {
> >> +g_assert_not_reached();
> >> +}
> >> +
> >> +if (!qtest_has_machine(machine_alias)) {
> >> +g_autofree char *msg = g_strdup_printf("machine %s not
> >> supported", machine_alias);
> >> +g_test_skip(msg);
> >> +return;
> >> +}
> >> +
> >> +machine = resolve_machine_version(machine_alias, QEMU_ENV_SRC,
> >> + QEMU_ENV_DST);
> >> +
> >> +vmstate_src = g_strdup_printf("%s/vmstate-src", tmpfs);
> >> +vmstate_dst = g_strdup_printf("%s/vmstate-dst", tmpfs);
> >> +
> >> +cmd_dst = g_strdup_printf("-machine %s,%s -dump-vmstate %s",
> >> + machine, machine_opts, vmstate_dst);
> >> +cmd_src = g_strdup_printf("-machine %s,%s -dump-vmstate %s",
> >> + machine, machine_opts, vmstate_src);
> >> +
> >> +qtest_init_with_env_no_handshake(QEMU_ENV_SRC, cmd_src);
> >> +qtest_init_with_env_no_handshake(QEMU_ENV_DST, cmd_dst);
> >> +
> >> +pid = fork();
> >> +if (!pid) {
> >> +close(1);
> >> +open("/dev/null", O_WRONLY);
> >> +execl(python, python, VMSTATE_CHECKER_SCRIPT,
> >> + "-s", vmstate_src,
> >> + "-d", vmstate_dst,
> >> + NULL);
> >> +g_assert_not_reached();
> >> +}
> >> +
> >> +g_assert(waitpid(pid, &wstatus, 0) == pid);
> >> +if (!WIFEXITED(wstatus) || WEXITSTATUS(wstatus) != 0) {
> >> +g_test_message("Failed to run vmstate-static-checker.py");
> >> +g_test_fail();
> >> +}
> >> +
> >> +cleanup("vmstate-src");
> >> +cleanup("vmstate-dst");
> >> +}
> >
> > Did I ask before on whether this can be written without C?
>
> If you did I forgot about it, sorry.
Nah it's really about me forgetting things sometim
Re: [PATCH v2 45/67] target/arm: Convert SRSHL, URSHL to decodetree
On Sat, 25 May 2024 at 00:29, Richard Henderson wrote: > > Signed-off-by: Richard Henderson > --- > target/arm/tcg/a64.decode | 4 > target/arm/tcg/translate-a64.c | 22 +++--- > 2 files changed, 11 insertions(+), 15 deletions(-) > Reviewed-by: Peter Maydell thanks -- PMM
Re: [PATCH v2 44/67] target/arm: Convert SRSHL and URSHL (register) to gvec
On Sat, 25 May 2024 at 00:27, Richard Henderson
wrote:
>
> Signed-off-by: Richard Henderson
> ---
> target/arm/helper.h | 10 +
> target/arm/tcg/translate.h | 4
> target/arm/tcg/neon-dp.decode | 10 ++---
> target/arm/tcg/gengvec.c| 22 +++
> target/arm/tcg/neon_helper.c| 38 -
> target/arm/tcg/translate-a64.c | 17 ++-
> target/arm/tcg/translate-neon.c | 6 ++
> 7 files changed, 84 insertions(+), 23 deletions(-)
> uint32_t HELPER(glue(neon_,name))(CPUARMState *env, uint32_t arg1, uint32_t
> arg2) \
> NEON_VOP_BODY(vtype, n)
>
> +#define NEON_GVEC_VOP2(name, vtype) \
> +void HELPER(name)(void *vd, void *vn, void *vm, uint32_t desc) \
> +{ \
> +intptr_t i, opr_sz = simd_oprsz(desc); \
> +vtype *d = vd, *n = vn, *m = vm;\
> +for (i = 0; i < opr_sz / sizeof(vtype); i++) { \
> +NEON_FN(d[i], n[i], m[i]); \
Does this need H macros for the bigendian case ? It looks
like we use it for smaller-than-64-bit element cases.
> +} \
> +clear_tail(d, opr_sz, simd_maxsz(desc));\
> +}
> +
thanks
-- PMM
Re: [PATCH V1 05/26] migration: precreate vmstate
On 5/27/2024 2:16 PM, Peter Xu wrote:
On Mon, Apr 29, 2024 at 08:55:14AM -0700, Steve Sistare wrote:
Provide the VMStateDescription precreate field to mark objects that must
be loaded on the incoming side before devices have been created, because
they provide properties that will be needed at creation time. They will
be saved to and loaded from their own QEMUFile, via
qemu_savevm_precreate_save and qemu_savevm_precreate_load, but these
functions are not yet called in this patch. Allow them to be called
before or after normal migration is active, when current_migration and
current_incoming are not valid.
Signed-off-by: Steve Sistare
---
include/migration/vmstate.h | 6
migration/savevm.c | 69 +
migration/savevm.h | 3 ++
3 files changed, 73 insertions(+), 5 deletions(-)
diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index 294d2d8..4691334 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -198,6 +198,12 @@ struct VMStateDescription {
* a QEMU_VM_SECTION_START section.
*/
bool early_setup;
+
+/*
+ * Send/receive this object in the precreate migration stream.
+ */
+bool precreate;
+
int version_id;
int minimum_version_id;
MigrationPriority priority;
diff --git a/migration/savevm.c b/migration/savevm.c
index 9789823..a30bcd9 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -239,6 +239,7 @@ static SaveState savevm_state = {
#define SAVEVM_FOREACH(se, entry)\
QTAILQ_FOREACH(se, &savevm_state.handlers, entry)\
+if (!se->vmsd || !se->vmsd->precreate)
#define SAVEVM_FOREACH_ALL(se, entry)\
QTAILQ_FOREACH(se, &savevm_state.handlers, entry)
@@ -1006,13 +1007,19 @@ static void save_section_header(QEMUFile *f,
SaveStateEntry *se,
}
}
+static bool send_section_footer(SaveStateEntry *se)
+{
+return (se->vmsd && se->vmsd->precreate) ||
+ migrate_get_current()->send_section_footer;
+}
Does the precreate vmsd "require" the footer? Or it should also work?
IMHO it's less optimal to bind features without good reasons.
It is not required. However, IMO we should not treat send-section-footer as
a fungible feature. It is strictly an improvement, as was added to catch
misformated sections. It is only registered as a feature for backwards
compatibility with qemu 2.3 and xen.
For a brand new data stream such as precreate, where we are not constrained
by backwards compatibility, we should unconditionally use the better protocol,
and always send the footer.
- Steve
Re: [RFC PATCH 4/4] ci: Add the new migration device tests
On Mon, May 27, 2024 at 08:59:00PM -0300, Fabiano Rosas wrote:
> Peter Xu writes:
>
> > On Thu, May 23, 2024 at 05:19:22PM -0300, Fabiano Rosas wrote:
> >> We have two new migration tests that check cross version
> >> compatibility. One uses the vmstate-static-checker.py script to
> >> compare the vmstate structures from two different QEMU versions. The
> >> other runs a simple migration with a few devices present in the VM, to
> >> catch obvious breakages.
> >>
> >> Add both tests to the migration-compat-common job.
> >>
> >> Signed-off-by: Fabiano Rosas
> >> ---
> >> .gitlab-ci.d/buildtest.yml | 43 +++---
> >> 1 file changed, 36 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
> >> index 91c57efded..bc7ac35983 100644
> >> --- a/.gitlab-ci.d/buildtest.yml
> >> +++ b/.gitlab-ci.d/buildtest.yml
> >> @@ -202,18 +202,47 @@ build-previous-qemu:
> >>needs:
> >> - job: build-previous-qemu
> >> - job: build-system-opensuse
> >> - # The old QEMU could have bugs unrelated to migration that are
> >> - # already fixed in the current development branch, so this test
> >> - # might fail.
> >> + # This test is allowed to fail because:
> >> + #
> >> + # - The old QEMU could have bugs unrelated to migration that are
> >> + # already fixed in the current development branch.
> >
> > Did you ever hit a real failure with this? I'm wondering whether we can
> > remove this allow_failure thing.
> >
>
> I haven't. But when it fails we'll go through an entire release cycle
> with this thing showing red for every person that runs the CI. Remember,
> this is a CI failure to which there's no fix aside from waiting for the
> release to happen. Even if we're quick to react and disable the job, I
> feel it might create some confusion already.
My imagination was if needed we'll get complains and we add that until
then for that broken release only, and remove in the next release again.
>
> >> + #
> >> + # - The vmstate-static-checker script trips on renames and other
> >> + # backward-compatible changes to the vmstate structs.
> >
> > I think I keep my preference per last time we talked on this. :)
>
> Sorry, I'm not trying to force this in any way, I just wrote these to
> use in the pull-request and thought I'd put it out there. At the very
> least we can have your concerns documented. =)
Yep that's fine. I think we should keep such discussion on the list,
especially we have different opinions, while none of us got convinced yet
so far. :)
>
> > I still think it's too early to involve a test that can report false
> > negative.
>
> (1)
> Well, we haven't seen any false negatives, we've seen fields being
> renamed. If that happens, then we'll ask the person to update the
> script. Is that not acceptable to you? Or are you thinking about other
> sorts of issues?
Then question is how to update the script. So far it's treated as failure
on rename, even if it's benign. Right now we have this:
print("Section \"" + sec + "\",", end=' ')
print("Description \"" + desc + "\":", end=' ')
print("expected field \"" + s_item["field"] + "\",", end=' ')
print("got \"" + d_item["field"] + "\"; skipping rest")
bump_taint()
break
Do you want to introduce a list of renamed vmsd fields in this script and
maintain that? IMHO it's an overkill and unnecessary burden to other
developers.
>
> > I'd still keep running this before soft-freeze like I used to
> > do, throw issues to others and urge them to fix before release.
>
> Having hidden procedures that maintainers run before a release is bad
> IMHO, it just delays the catching of bugs and frustrates
> contributors. Imagine working on a series, everything goes well with
> reviews, CI passes, patch gets queued and merged and a month later you
> get a ping about something you should have done to avoid breaking
> migration. Right during freeze.
I understand your point, however I don't yet see a way CI could cover
everything. CI won't cover performance test, and I still ran multifd tests
at least smoke it too before soft-freeze.
If there's something done wrong, we notify the sooner the better. Now it
looks to me the best trade-off is like that - we notify at soft-freeze once
per release considering that's pretty rare too, e.g. 9.0 has no such outlier.
Again I'm happy if we have a solution to not report false negatives; that's
the only concern I have.
>
> > Per my
> > previous experience that doesn't consume me a lot of time, and it's not
> > common to see issues either.
> >
> > So I want people to really pay attention when someone sees a migration CI
> > test failed, rather than we help people form the habit in "oh migration CI
> > failed again? I think that's fine, it allows failing anyway".
>
> That's a good point. I don't think it applies here though. See my point
> in (1).
Yes, if you hav
Re: [PATCH v2 43/67] target/arm: Convert SSHL, USHL to decodetree
On Sat, 25 May 2024 at 00:29, Richard Henderson wrote: > > Signed-off-by: Richard Henderson > --- > target/arm/tcg/a64.decode | 7 ++ > target/arm/tcg/translate-a64.c | 40 +- > 2 files changed, 32 insertions(+), 15 deletions(-) Reviewed-by: Peter Maydell thanks -- PMM
Re: [PULL 00/28] riscv-to-apply queue
On 5/27/24 19:43, Alistair Francis wrote: The following changes since commit ad10b4badc1dd5b28305f9b9f1168cf0aa3ae946: Merge tag 'pull-error-2024-05-27' ofhttps://repo.or.cz/qemu/armbru into staging (2024-05-27 06:40:42 -0700) are available in the Git repository at: https://github.com/alistair23/qemu.git tags/pull-riscv-to-apply-20240528 for you to fetch changes up to 1806da76cb81088ea026ca3441551782b850e393: target/riscv: raise an exception when CSRRS/CSRRC writes a read-only CSR (2024-05-28 12:20:27 +1000) RISC-V PR for 9.1 * APLICs add child earlier than realize * Fix exposure of Zkr * Raise exceptions on wrs.nto * Implement SBI debug console (DBCN) calls for KVM * Support 64-bit addresses for initrd * Change RISCV_EXCP_SEMIHOST exception number to 63 * Tolerate KVM disable ext errors * Set tval in breakpoints * Add support for Zve32x extension * Add support for Zve64x extension * Relax vector register check in RISCV gdbstub * Fix the element agnostic Vector function problem * Fix Zvkb extension config * Implement dynamic establishment of custom decoder * Add th.sxstatus CSR emulation * Fix Zvfhmin checking for vfwcvt.f.f.v and vfncvt.f.f.w instructions * Check single width operator for vector fp widen instructions * Check single width operator for vfncvt.rod.f.f.w * Remove redudant SEW checking for vector fp narrow/widen instructions * Prioritize pmp errors in raise_mmu_exception() * Do not set mtval2 for non guest-page faults * Remove experimental prefix from "B" extension * Fixup CBO extension register calculation * Fix the hart bit setting of AIA * Fix reg_width in ricsv_gen_dynamic_vector_feature() * Decode all of the pmpcfg and pmpaddr CSRs * Raise an exception when CSRRS/CSRRC writes a read-only CSR Fails testing: https://gitlab.com/qemu-project/qemu/-/jobs/6953349448 ERROR:../tests/plugin/insn.c:58:vcpu_init: assertion failed: (count > 0) timeout: the monitored command dumped core Aborted make[1]: *** [Makefile:178: run-plugin-catch-syscalls-with-libinsn.so] Error 134 make: *** [/builds/qemu-project/qemu/tests/Makefile.include:56: run-tcg-tests-riscv64-linux-user] Error 2 #0 riscv_gdb_get_csr (cs=, buf=0x558e7f50, n=3072) at ../src/target/riscv/gdbstub.c:183 #1 0x77fb7841 in vcpu_init (id=, vcpu_index=) at ../src/tests/plugin/insn.c:57 #2 0x5569ef1a in plugin_vcpu_cb__simple (cpu=0x558fb820, ev=) at ../src/plugins/core.c:111 After 182 result = riscv_csrrw_debug(env, n, &val, 0, 0); result == 2. r~
Re: [PATCH v2 42/67] target/arm: Convert SUQADD, USQADD to decodetree
On Sat, 25 May 2024 at 00:26, Richard Henderson wrote: > > These are faux 2-operand instructions, reading from rd. > Sort them next to the other three-operand same insns for clarity. > > Signed-off-by: Richard Henderson > --- > target/arm/tcg/a64.decode | 8 + > target/arm/tcg/translate-a64.c | 64 -- > 2 files changed, 14 insertions(+), 58 deletions(-) Reviewed-by: Peter Maydell thanks -- PMM
Re: [PATCH v2 41/67] target/arm: Convert SQADD, SQSUB, UQADD, UQSUB to decodetree
On Sat, 25 May 2024 at 00:28, Richard Henderson wrote: > > Signed-off-by: Richard Henderson > --- > target/arm/tcg/a64.decode | 11 > target/arm/tcg/translate-a64.c | 100 +++-- > 2 files changed, 68 insertions(+), 43 deletions(-) > Reviewed-by: Peter Maydell thanks -- PMM
Re: [PATCH v2 40/67] target/arm: Inline scalar SQADD, UQADD, SQSUB, UQSUB
On Sat, 25 May 2024 at 00:27, Richard Henderson wrote: > > This eliminates the last uses of these neon helpers. > Incorporate the MO_64 expanders as an option to the vector expander. > > Signed-off-by: Richard Henderson > --- Reviewed-by: Peter Maydell thanks -- PMM
Re: [PATCH v2 0/3] docs: define policy forbidding use of "AI" / LLM code generators
Am 16.05.2024 um 18:22 hat Daniel P. Berrangé geschrieben: > This patch kicks the hornet's nest of AI / LLM code generators. > > With the increasing interest in code generators in recent times, > it is inevitable that QEMU contributions will include AI generated > code. Thus far we have remained silent on the matter. Given that > everyone knows these tools exist, our current position has to be > considered tacit acceptance of the use of AI generated code in QEMU. > > The question for the project is whether that is a good position for > QEMU to take or not ? > > IANAL, but I like to think I'm reasonably proficient at understanding > open source licensing. I am not inherantly against the use of AI tools, > rather I am anti-risk. I also want to see OSS licenses respected and > complied with. > > AFAICT at its current state of (im)maturity the question of licensing > of AI code generator output does not have a broadly accepted / settled > legal position. This is an inherant bias/self-interest from the vendors > promoting their usage, who tend to minimize/dismiss the legal questions. > From my POV, this puts such tools in a position of elevated legal risk. > > Given the fuzziness over the legal position of generated code from > such tools, I don't consider it credible (today) for a contributor > to assert compliance with the DCO terms (b) or (c) (which is a stated > pre-requisite for QEMU accepting patches) when a patch includes (or is > derived from) AI generated code. > > By implication, I think that QEMU must (for now) explicitly decline > to (knowingly) accept AI generated code. > > Perhaps a few years down the line the legal uncertainty will have > reduced and we can re-evaluate this policy. > > Discuss... > > Changes in v2: > > * Fix a huge number of typos in docs > * Clarify that maintainers should still add R-b where relevant, even >if they are already adding their own S-oB. > * Clarify situation when contributor re-starts previously abandoned >work from another contributor. > * Add info about Suggested-by tag > * Add new docs section dealing with the broad topic of "generated >files" (whether code generators or compilers) > * Simplify the section related to prohibition of AI generated files >and give further examples of tools considered covered > * Remove repeated references to "LLM" as a specific technology, just >use the broad "AI" term, except for one use of LLM as an example. > * Add note that the policy may evolve if the legal clarity improves > * Add note that exceptions can be requested on case-by-case basis >if contributor thinks they can demonstrate a credible copyright >and licensing status > > Daniel P. Berrangé (3): > docs: introduce dedicated page about code provenance / sign-off > docs: define policy limiting the inclusion of generated files > docs: define policy forbidding use of AI code generators > > docs/devel/code-provenance.rst| 315 ++ > docs/devel/index-process.rst | 1 + > docs/devel/submitting-a-patch.rst | 19 +- > 3 files changed, 318 insertions(+), 17 deletions(-) > create mode 100644 docs/devel/code-provenance.rst Reviewed-by: Kevin Wolf
Re: [PATCH v2 2/3] docs: define policy limiting the inclusion of generated files
Am 16.05.2024 um 18:22 hat Daniel P. Berrangé geschrieben: > Files contributed to QEMU are generally expected to be provided in the > preferred format for manipulation. IOW, we generally don't expect to > have generated / compiled code included in the tree, rather, we expect > to run the code generator / compiler as part of the build process. > > There are some obvious exceptions to this seen in our existing tree, the > biggest one being the inclusion of many binary firmware ROMs. A more > niche example is the inclusion of a generated eBPF program. Or the CI > dockerfiles which are mostly auto-generated. In these cases, however, > the preferred format source code is still required to be included, > alongside the generated output. > > Tools which perform user defined algorithmic transformations on code are > not considered to be "code generators". ie, we permit use of coccinelle, > spell checkers, and sed/awk/etc to manipulate code. Such use of automated > manipulation should still be declared in the commit message. > > One off generators which create a boilerplate file which the author then > fills in, are acceptable if their output has clear copyright and license > status. This could be where a contributor writes a throwaway python > script to automate creation of some mundane piece of code for example. > > Signed-off-by: Daniel P. Berrangé > --- > docs/devel/code-provenance.rst | 55 ++ > 1 file changed, 55 insertions(+) > > diff --git a/docs/devel/code-provenance.rst b/docs/devel/code-provenance.rst > index 7c42fae571..eabb3e7c08 100644 > --- a/docs/devel/code-provenance.rst > +++ b/docs/devel/code-provenance.rst > @@ -210,3 +210,58 @@ mailing list. > It is also recommended to attempt to contact the original author to let them > know you are interested in taking over their work, in case they still > intended > to return to the work, or had any suggestions about the best way to continue. > + > +Inclusion of generated files > + > + > +Files in patches contributed to QEMU are generally expected to be provided > +only in the preferred format for making modifications. The implication of > +this is that the output of code generators or compilers is usually not > +appropriate to contribute to QEMU. > + > +For reasons of practicality there are some exceptions to this rule, where > +generated code is permitted, provided it is also accompanied by the > +corresponding preferred source format. This is done where it is impractical > +to expect those building QEMU to run the code generation or compilation > +process. A non-exhustive list of examples is: > + > + * Images: where an bitmap image is created from a vector file it is common > + to include the rendered bitmaps at desired resolution(s), since subtle > + changes in the rasterization process / tools may affect quality. The > + original vector file is expected to accompany any generated bitmaps. > + > + * Firmware: QEMU includes pre-compiled binary ROMs for a variety of guest > + firmwares. When such binary ROMs are contributed, the corresponding source > + must also be provided, either directly, or through a git submodule link. > + > + * Dockerfiles: the majority of the dockerfiles are automatically generated > + from a canonical list of build dependencies maintained in tree, together > + with the libvirt-ci git submodule link. The generated dockerfiles are > + included in tree because it is desirable to be able to directly build > + container images from a clean git checkout. > + > + * EBPF: QEMU includes some generated EBPF machine code, since the required > + eBPF compilation tools are not broadly available on all targetted OS > + distributions. The corresponding eBPF C code for the binary is also > + provided. This is a time limited exception until the eBPF toolchain is > + sufficiently broadly available in distros. This paragraph is inconsistent with the spelling of "EBPF"/"eBPF". Kevin
Re: [PATCH v2 39/67] target/arm: Inline scalar SUQADD and USQADD
On Sat, 25 May 2024 at 00:23, Richard Henderson wrote: > > This eliminates the last uses of these neon helpers. > Incorporate the MO_64 expanders as an option to the vector expander. > > Signed-off-by: Richard Henderson > --- > target/arm/helper.h| 8 -- > target/arm/tcg/translate-a64.h | 8 ++ > target/arm/tcg/gengvec64.c | 71 ++ > target/arm/tcg/neon_helper.c | 165 - > target/arm/tcg/translate-a64.c | 73 +-- > 5 files changed, 103 insertions(+), 222 deletions(-) Reviewed-by: Peter Maydell thanks -- PMM
Re: [PATCH v2 38/67] target/arm: Convert SUQADD and USQADD to gvec
On Sat, 25 May 2024 at 00:32, Richard Henderson
wrote:
>
> Signed-off-by: Richard Henderson
> ---
> target/arm/helper.h| 16 +
> target/arm/tcg/translate-a64.h | 6 ++
> target/arm/tcg/gengvec64.c | 106 +++
> target/arm/tcg/translate-a64.c | 113 ++---
> target/arm/tcg/vec_helper.c| 64 +++
> 5 files changed, 241 insertions(+), 64 deletions(-)
> diff --git a/target/arm/tcg/gengvec64.c b/target/arm/tcg/gengvec64.c
> index 093b498b13..4b76e476a0 100644
> --- a/target/arm/tcg/gengvec64.c
> +++ b/target/arm/tcg/gengvec64.c
> @@ -188,3 +188,109 @@ void gen_gvec_bcax(unsigned vece, uint32_t d, uint32_t
> n, uint32_t m,
> tcg_gen_gvec_4(d, n, m, a, oprsz, maxsz, &op);
> }
>
> +static void gen_suqadd_vec(unsigned vece, TCGv_vec t, TCGv_vec qc,
> + TCGv_vec a, TCGv_vec b)
> +{
> +TCGv_vec max =
> +tcg_constant_vec_matching(t, vece, (1ull << ((8 << vece) - 1)) - 1);
> +TCGv_vec u = tcg_temp_new_vec_matching(t);
> +
> +/* Maximum value that can be added to @a without overflow. */
> +tcg_gen_sub_vec(vece, u, max, a);
> +
> +/* Constrain addend so that the next addition never overflows. */
> +tcg_gen_umin_vec(vece, u, u, b);
> +tcg_gen_add_vec(vece, t, u, a);
> +
> +/* Compute QC by comparing the adjusted @b. */
> +tcg_gen_xor_vec(vece, u, u, b);
> +tcg_gen_or_vec(vece, qc, qc, u);
With this kind of code where we wind up doing a vector op
into vfp.qc, is there anything somewhere that asserts that
we don't try to do it with a vector length bigger than
sizeof(vfp.qc) (i.e. 128) ?
thanks
-- PMM
Re: hw/usb/hcd-ohci: Fix #1510, #303: pid not IN or OUT
On Tue, May 28, 2024 at 9:03 AM Peter Maydell wrote: > On Mon, 20 May 2024 at 23:24, Cord Amfmgm wrote: > > On Mon, May 20, 2024 at 12:05 PM Peter Maydell > wrote: > >> For the "zero length buffer" case, do you have a more detailed > >> pointer to the bit of the spec that says that "cbp = be + 1" is a > >> valid way to specify a zero length buffer? Table 4-2 in the copy I > >> have says for CurrentBufferPointer "a value of 0 indicates > >> a zero-length data packet or that all bytes have been transferred", > >> and the sample host OS driver function QueueGeneralRequest() > >> later in the spec handles a 0 length packet by setting > >> TD->HcTD.CBP = TD->HcTD.BE = 0; > >> (which our emulation's code does handle). > > > > > > Reading the spec carefully, a CBP set to 0 should always mean the > zero-length buffer case (or that all bytes have been transferred, so the > buffer is now zero-length - the same thing). > > Yeah, I can see the argument for "we should treat all cbp == 0 as > zero-length buffer, not just cbp == be == 0". > > > Table 4-2 is the correct reference, and this part is clear. It's the > part you quoted. "Contains the physical address of the next memory location > that will be accessed for transfer to/from the endpoint. A value of 0 > indicates a zero-length data packet or that all bytes have been > transferred." > > > > Table 4-2 has this additional nugget that may be confusingly worded, for > BufferEnd: "Contains physical address of the last byte in the buffer for > this TD" > > Mmm, but for a zero length buffer there is no "last byte" to > have this be the physical address of. > > > And here's an example buffer of length 0 -- you probably already know > what I'm going to do here: > > > > char buf[0]; > > char * CurrentBufferPointer = &buf[0]; > > char * BufferEnd = &buf[-1]; // "address of the last byte in the buffer" > > // The OHCI Host Controller than advances CurrentBufferPointer like > this: CurrentBufferPointer += 0 > > // After the transfer: > > // CurrentBufferPointer = &buf[0]; > > // BufferEnd = &buf[-1]; > > Right, but why do you think this is valid, rather than > being a guest software bug? My reading of the spec is that it's > pretty clear about how to say "zero length buffer", and this > isn't it. > > Is there some real-world guest OS that programs the OHCI > controller this way that we're trying to accommodate? > qemu versions 4.2 and before allowed this behavior. I don't think it's valid to ask for a *popular* guest OS as a proof-of-concept because I'm not an expert on those. The spec says "last byte" which can (not "should" just "can") be interpreted as "cbp - 1". Therefore, I raised this patch request to re-enable the previous qemu behavior, in the sense of ""be conservative in what you do, be liberal in what you accept from others" - https://en.wikipedia.org/wiki/Robustness_principle It is *not* valid to say the spec disallows "cbp - 1" to mean "empty buffer." That is what I am arguing :) > > thanks > -- PMM >
Re: [RFC PATCH 3/4] tests/qtest/migration: Add support for simple device tests
On Mon, May 27, 2024 at 07:59:50PM -0300, Fabiano Rosas wrote: > Peter Xu writes: > > > On Thu, May 23, 2024 at 05:19:21PM -0300, Fabiano Rosas wrote: > >> The current migration-tests are almost entirely focused on catching > >> bugs on the migration code itself, not on the device migration > >> infrastructure (vmstate). That means we miss catching some low hanging > >> fruits that would show up immediately if only we had the device in > >> question present in the VM. > >> > >> Add a list of devices to include by default in the migration-tests, > >> starting with one that recently had issues, virtio-gpu. Also add an > >> environment variable QTEST_DEVICE_OPTS to allow test users to > >> experiment with different devices or device options. > >> > >> Do not run every migration test with the devices because that would > >> increase the complexity of the command lines and, as mentioned, the > >> migration-tests are mostly used to test the core migration code, not > >> the device migration. Add a special value QTEST_DEVICE_OPTS=all that > >> enables testing with devices. > >> > >> Notes on usage: > >> > >> For this new testing mode, it's not useful to run all the migration > >> tests, a single test would probably suffice to catch any issues, so > >> provide the -p option to migration-test and the test of your choice. > >> > >> Like with the cross-version compatibility tests in CI and the recently > >> introduced vmstate-static-checker test, to be of any use, a test with > >> devices needs to be run against a different QEMU version, like so: > >> > >> $ cd build > >> $ QTEST_DEVICE_OPTS=all \ > >> QTEST_QEMU_BINARY=./qemu-system-x86_64 \ > >> QTEST_QEMU_BINARY_DST=../build-previous/qemu-system-x86_64 \ > >> ./tests/qtest/migration-test -p /x86_64/migration/precopy/tcp/plain > >> > >> $ cd build > >> $ QTEST_DEVICE_OPTS='-device virtio-net' \ > >> QTEST_QEMU_BINARY=./qemu-system-x86_64 \ > >> QTEST_QEMU_BINARY_DST=../build-previous/qemu-system-x86_64 \ > >> ./tests/qtest/migration-test -p /x86_64/migration/precopy/tcp/plain > >> > >> Signed-off-by: Fabiano Rosas > >> --- > >> tests/qtest/migration-test.c | 19 +-- > >> 1 file changed, 17 insertions(+), 2 deletions(-) > >> > >> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c > >> index 2253e0fc5b..35bb224d18 100644 > >> --- a/tests/qtest/migration-test.c > >> +++ b/tests/qtest/migration-test.c > >> @@ -71,6 +71,13 @@ static QTestMigrationState dst_state; > >> #define QEMU_ENV_SRC "QTEST_QEMU_BINARY_SRC" > >> #define QEMU_ENV_DST "QTEST_QEMU_BINARY_DST" > >> > >> +/* > >> + * The tests using DEFAULT_DEVICES need a special invocation and > >> + * cannot be reached from make check, so don't bother with the > >> + * --without-default-devices build. > > > > What's this "--without-default-devices"? > > A configure option. It removes from the build any devices that are > marked as default. It's an endless source of bugs because it is supposed > to be paired with a config file that adds back some of the removed > devices, but there's nothing enforcing that so we always run it as is > and generate a broken QEMU binary. > > So anything in the tests that refer to devices should first check if > that QEMU binary even has the device present. I'm saying here that we're > not going to do that because this test cannot be accidentally reached > via make check. Realistically, most people will consume this test > through the CI job only. Ah I didn't expect that is an existing configure option.. then it is all fine. Reviewed-by: Peter Xu Thanks, -- Peter Xu
Re: [PATCH v6 2/7] migration/multifd: put IOV initialization into compression method
On Tue, May 28, 2024 at 01:36:38PM +, Liu, Yuan1 wrote:
> > -Original Message-
> > From: Peter Xu
> > Sent: Tuesday, May 28, 2024 4:51 AM
> > To: Liu, Yuan1
> > Cc: faro...@suse.de; qemu-devel@nongnu.org; Zou, Nanhai
> >
> > Subject: Re: [PATCH v6 2/7] migration/multifd: put IOV initialization into
> > compression method
> >
> > On Mon, May 06, 2024 at 12:57:46AM +0800, Yuan Liu wrote:
> > > Different compression methods may require different numbers of IOVs.
> > > Based on streaming compression of zlib and zstd, all pages will be
> > > compressed to a data block, so two IOVs are needed for packet header
> > > and compressed data block.
> > >
> > > Signed-off-by: Yuan Liu
> > > Reviewed-by: Nanhai Zou
> > > ---
> > > migration/multifd-zlib.c | 7 +++
> > > migration/multifd-zstd.c | 8 +++-
> > > migration/multifd.c | 22 --
> > > 3 files changed, 26 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/migration/multifd-zlib.c b/migration/multifd-zlib.c
> > > index 737a9645d2..2ced69487e 100644
> > > --- a/migration/multifd-zlib.c
> > > +++ b/migration/multifd-zlib.c
> > > @@ -70,6 +70,10 @@ static int zlib_send_setup(MultiFDSendParams *p,
> > Error **errp)
> > > goto err_free_zbuff;
> > > }
> > > p->compress_data = z;
> > > +
> > > +/* Needs 2 IOVs, one for packet header and one for compressed data
> > */
> > > +p->iov = g_new0(struct iovec, 2);
> > > +
> > > return 0;
> > >
> > > err_free_zbuff:
> > > @@ -101,6 +105,9 @@ static void zlib_send_cleanup(MultiFDSendParams *p,
> > Error **errp)
> > > z->buf = NULL;
> > > g_free(p->compress_data);
> > > p->compress_data = NULL;
> > > +
> > > +g_free(p->iov);
> > > +p->iov = NULL;
> > > }
> > >
> > > /**
> > > diff --git a/migration/multifd-zstd.c b/migration/multifd-zstd.c
> > > index 256858df0a..ca17b7e310 100644
> > > --- a/migration/multifd-zstd.c
> > > +++ b/migration/multifd-zstd.c
> > > @@ -52,7 +52,6 @@ static int zstd_send_setup(MultiFDSendParams *p, Error
> > **errp)
> > > struct zstd_data *z = g_new0(struct zstd_data, 1);
> > > int res;
> > >
> > > -p->compress_data = z;
> > > z->zcs = ZSTD_createCStream();
> > > if (!z->zcs) {
> > > g_free(z);
> > > @@ -77,6 +76,10 @@ static int zstd_send_setup(MultiFDSendParams *p,
> > Error **errp)
> > > error_setg(errp, "multifd %u: out of memory for zbuff", p->id);
> > > return -1;
> > > }
> > > +p->compress_data = z;
> > > +
> > > +/* Needs 2 IOVs, one for packet header and one for compressed data
> > */
> > > +p->iov = g_new0(struct iovec, 2);
> > > return 0;
> > > }
> > >
> > > @@ -98,6 +101,9 @@ static void zstd_send_cleanup(MultiFDSendParams *p,
> > Error **errp)
> > > z->zbuff = NULL;
> > > g_free(p->compress_data);
> > > p->compress_data = NULL;
> > > +
> > > +g_free(p->iov);
> > > +p->iov = NULL;
> > > }
> > >
> > > /**
> > > diff --git a/migration/multifd.c b/migration/multifd.c
> > > index f317bff077..d82885fdbb 100644
> > > --- a/migration/multifd.c
> > > +++ b/migration/multifd.c
> > > @@ -137,6 +137,13 @@ static int nocomp_send_setup(MultiFDSendParams *p,
> > Error **errp)
> > > p->write_flags |= QIO_CHANNEL_WRITE_FLAG_ZERO_COPY;
> > > }
> > >
> > > +if (multifd_use_packets()) {
> > > +/* We need one extra place for the packet header */
> > > +p->iov = g_new0(struct iovec, p->page_count + 1);
> > > +} else {
> > > +p->iov = g_new0(struct iovec, p->page_count);
> > > +}
> > > +
> > > return 0;
> > > }
> > >
> > > @@ -150,6 +157,8 @@ static int nocomp_send_setup(MultiFDSendParams *p,
> > Error **errp)
> > > */
> > > static void nocomp_send_cleanup(MultiFDSendParams *p, Error **errp)
> > > {
> > > +g_free(p->iov);
> > > +p->iov = NULL;
> > > return;
> > > }
> > >
> > > @@ -228,6 +237,7 @@ static int nocomp_send_prepare(MultiFDSendParams *p,
> > Error **errp)
> > > */
> > > static int nocomp_recv_setup(MultiFDRecvParams *p, Error **errp)
> > > {
> > > +p->iov = g_new0(struct iovec, p->page_count);
> > > return 0;
> > > }
> > >
> > > @@ -240,6 +250,8 @@ static int nocomp_recv_setup(MultiFDRecvParams *p,
> > Error **errp)
> > > */
> > > static void nocomp_recv_cleanup(MultiFDRecvParams *p)
> > > {
> > > +g_free(p->iov);
> > > +p->iov = NULL;
> > > }
> >
> > Are recv_setup()/recv_cleanup() for zstd/zlib missing?
>
> Zstd/zlib does not request the IOVs on the receiving side.
> The zstd/zlib reads the compressed data into the buffer directly
>
> qio_channel_read_all(p->c, (void *)z->zbuff, in_size, errp);
Hmm indeed, thanks.
>
> > If the iov will be managed by the compressors, I'm wondering whether we
> > should start assert(p->iov) after send|recv_setup(), and assert(!p->iov)
> > after send|recv_cleanup(). That'll guard all these.
>
> Yes, I agree with adding a check of IOV in multifd.c sin
Re: [PATCH 1/1] vhost-vsock: add VIRTIO_F_RING_PACKED to feaure_bits
On Mon, May 27, 2024 at 01:27:10PM GMT, Halil Pasic wrote:
On Thu, 16 May 2024 10:39:42 +0200
Stefano Garzarella wrote:
[..]
>---
>
>This is a minimal fix, that follows the current patterns in the
>codebase, and not necessarily the best one.
Yeah, I did something similar with commit 562a7d23bf ("vhost: mask
VIRTIO_F_RING_RESET for vhost and vhost-user devices") so I think for
now is the right approach.
I suggest to check also other devices like we did in that commit (e.g.
hw/scsi/vhost-scsi.c, hw/scsi/vhost-user-scsi.c, etc. )
Hi Stefano!
Thank you for chiming in, and sorry for the late response. I was hoping
that Michael is going to chime in and that I can base my reply on his
take. Anyway here I go.
A very valid observation! I do agree that we need this for
basically every vhost device, and since:
* net/vhost-vdpa.c
* hw/net/vhost_net.c
* hw/virtio/vhost-user-fs.c
already have it, that translates to shotgun it to the rest. Which
isn't nice in my opinion, which is why I am hoping for a discussion
on this topic, and a better solution (even if it turns out to be
something like a common macro).
Yeah, I see your point and I agree on a better solution.
[..]
>
>The documentation however does kind of state, that feature_bits is
>supposed to contain the supported features. And under the assumption
>that feature bit not in feature_bits implies that the corresponding bit
>must not be set in the 3rd argument (features), then even with the
>current implementation we do end up with the intersection of the three
>as stated. And then vsock would be at fault for violating that
>assumption, and my fix would be the best thing to do -- I guess.
>
>Is the implementation the way it is for a good reason, I can't judge
>that with certainty for myself.
Yes, I think we should fix the documentation, and after a few years of
not looking at it I'm confused again about what it does.
I would prefer to fix the algorithm and make whole thing less fragile.
But re-reading my commit for VIRTIO_F_RING_RESET, it seems that I had
interpreted `feature_bits` (2nd argument) as a list of features that
QEMU doesn't know how to emulate and therefore are required by the
backend (vhost/vhost-user/vdpa). Because the problem is that `features`
(3rd argument) is a set of features required by the driver that can be
provided by both QEMU and the backend.
Hm. I would say, this does sound like the sanest explanation, that might
justify the current code, but I will argue that for me, it isn't sane
enough.
Here comes my argument.
1) The uses is explicitly asking for a vhost device and giving the user
a non vhost device is not an option.
I didn't get this point :-( can you elaborate?
2) The whole purpose of vhost is that at least the data plane is
implemented outside of QEMU (I am maybe a little sloppy here with
dataplane). That means a rather substantial portion of the device
implementation is not in QEMU, while QEMU remains in charge of the
setup.
Yep
3) Thus I would argue, that all the "transport feature bits" from 24 to
40 should have a corresponding vhost feature because the vhost part needs
some sort of a support.
What do we have there in bits from 24 to 40 according to the spec?
* VIRTIO_F_INDIRECT_DESC
* VIRTIO_F_EVENT_IDX
* VIRTIO_F_VERSION_1
* VIRTIO_F_ACCESS_PLATFORM
* VIRTIO_F_RING_PACKED
* VIRTIO_F_IN_ORDER
* VIRTIO_F_ORDER_PLATFORM
* VIRTIO_F_SR_IOV
* VIRTIO_F_NOTIFICATION_DATA
* VIRTIO_F_NOTIF_CONFIG_DATA
* VIRTIO_F_RING_RESET
and for transitional:
* VIRTIO_F_NOTIFY_ON_EMPTY
* VIRTIO_F_ANY_LAYOUT
* UNUSED
I would say, form these only VIRTIO_F_SR_IOV and
VIRTIO_F_NOTIF_CONFIG_DATA look iffy in a sense things may work out
for vhost devices without the vhost part doing something for it. And
even there, I don't think it would hurt to make vhost part of the
negotiation (I don't think those are supported by QEMU at this point).
I would very much prefer having a consolidated and safe handling for
these.
I completely agree on this!
4) I would also argue that a bunch of the device specific feature bits
should have vhost feature bits as well for the same reason:
features are also such that for a vhost device, the vhost part needs
some sort of a support.
Looking through all of these would require a lot of time, so instead
of that, let me use SCSI as an example. The features are:
* VIRTIO_SCSI_F_INOUT
* VIRTIO_SCSI_F_HOTPLUG
* VIRTIO_SCSI_F_CHANGE
* VIRTIO_SCSI_F_T10_PI
The in the Linux kernel we have
VHOST_SCSI_FEATURES = VHOST_FEATURES | (1ULL << VIRTIO_SCSI_F_HOTPLUG) |
(1ULL << VIRTIO_SCSI_F_T10_PI)
but in QEMU kernel_feature_bits does not have
VIRTIO_SCSI_F_T10_PI which together does not make much sense to me. And I would
also expect VIRTIO_SCSI_F_INOUT to be a part of the negotiation, because
to me that the side that is processing the queue is the one that should
care about it, but I don't think it is supported by QEMU at all, and
then not negotiating i
